Re: [flashrom] [PATCH 3/3] [RFC] Unify usbdev_status and pcidev_status into dev_status.

[Carl-Daniel Hailfinger]

Am 17.11.2012 20:09 schrieb Stefan Tauner:
> Signed-off-by: Stefan Tauner

Is there a reason you introduce a .devices struct member inside struct
programmer_entry in patch 1 just to rename it to .dev in patch 3?

Regards,
Carl-Daniel
> diff --git a/flashrom.c b/flashrom.c
> index d873276..ae8f70c 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -65,7 +65,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "internal",
>    	.type			= OTHER,
> .devices.note		= NULL,
> .devs.note		= NULL,
> .init			= internal_init,
> .map_flash_region	= physmap,
> .unmap_flash_region	= physunmap,
>
> @@ -78,7 +78,7 @@ const struct programmer_entry programmer_table[] = {
>    	.name			= "dummy",
>    	.type			= OTHER,
>    				/* FIXME */
> .devices.note		= "Dummy device, does nothing and logs all accesses\n",
> .devs.note		= "Dummy device, does nothing and logs all accesses\n",
> .init			= dummy_init,
> .map_flash_region	= dummy_map,
> .unmap_flash_region	= dummy_unmap,
>
> @@ -90,7 +90,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "nic3com",
>    	.type			= PCI,
> .devices.pci		= nics_3com,
> .devs.status		= nics_3com,
> .init			= nic3com_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -103,7 +103,7 @@ const struct programmer_entry programmer_table[] = {
>    	/* This programmer works for Realtek RTL8139 and SMC 1211. */
>    	.name			= "nicrealtek",
>    	.type			= PCI,
> .devices.pci		= nics_realtek,
> .devs.status		= nics_realtek,
> .init			= nicrealtek_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -115,7 +115,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "nicnatsemi",
>    	.type			= PCI,
> .devices.pci		= nics_natsemi,
> .devs.status		= nics_natsemi,
> .init			= nicnatsemi_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -127,7 +127,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "gfxnvidia",
>    	.type			= PCI,
> .devices.pci		= gfx_nvidia,
> .devs.status		= gfx_nvidia,
> .init			= gfxnvidia_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -139,7 +139,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "drkaiser",
>    	.type			= PCI,
> .devices.pci		= drkaiser_pcidev,
> .devs.status		= drkaiser_pcidev,
> .init			= drkaiser_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -151,7 +151,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "satasii",
>    	.type			= PCI,
> .devices.pci		= satas_sii,
> .devs.status		= satas_sii,
> .init			= satasii_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -163,7 +163,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "atahpt",
>    	.type			= PCI,
> .devices.pci		= ata_hpt,
> .devs.status		= ata_hpt,
> .init			= atahpt_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -175,7 +175,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "ft2232_spi",
>    	.type			= USB,
> .devices.usb		= devs_ft2232spi,
> .devs.status		= devs_ft2232spi,
> .init			= ft2232_spi_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -188,7 +188,7 @@ const struct programmer_entry programmer_table[] = {
>    	.name			= "serprog",
>    	.type			= OTHER,
>    				/* FIXME */
> .devices.note		= "All programmer devices speaking the serprog protocol\n",
> .devs.note		= "All programmer devices speaking the serprog protocol\n",
> .init			= serprog_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -201,7 +201,7 @@ const struct programmer_entry programmer_table[] = {
>    	.name			= "buspirate_spi",
>    	.type			= OTHER,
>    				/* FIXME */
> .devices.note		= "Dangerous Prototypes Bus Pirate\n",
> .devs.note		= "Dangerous Prototypes Bus Pirate\n",
> .init			= buspirate_spi_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -214,7 +214,7 @@ const struct programmer_entry programmer_table[] = {
>    	.name			= "dediprog",
>    	.type			= OTHER,
>    				/* FIXME */
> .devices.note		= "Dediprog SF100\n",
> .devs.note		= "Dediprog SF100\n",
> .init			= dediprog_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -227,7 +227,7 @@ const struct programmer_entry programmer_table[] = {
>    	.name			= "rayer_spi",
>    	.type			= OTHER,
>    				/* FIXME */
> .devices.note		= "RayeR parallel port programmer\n",
> .devs.note		= "RayeR parallel port programmer\n",
> .init			= rayer_spi_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -240,7 +240,7 @@ const struct programmer_entry programmer_table[] = {
>    	.name			= "pony_spi",
>    	.type			= OTHER,
>    				/* FIXME */
> .devices.note		= "Programmers compatible with SI-Prog, serbang or AJAWe\n",
> .devs.note		= "Programmers compatible with SI-Prog, serbang or AJAWe\n",
> .init			= pony_spi_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -252,7 +252,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "nicintel",
>    	.type			= PCI,
> .devices.pci		= nics_intel,
> .devs.status		= nics_intel,
> .init			= nicintel_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -264,7 +264,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "nicintel_spi",
>    	.type			= PCI,
> .devices.pci		= nics_intel_spi,
> .devs.status		= nics_intel_spi,
> .init			= nicintel_spi_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -276,7 +276,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "ogp_spi",
>    	.type			= PCI,
> .devices.pci		= ogp_spi,
> .devs.status		= ogp_spi,
> .init			= ogp_spi_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -288,7 +288,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "satamv",
>    	.type			= PCI,
> .devices.pci		= satas_mv,
> .devs.status		= satas_mv,
> .init			= satamv_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> @@ -300,7 +300,7 @@ const struct programmer_entry programmer_table[] = {
>    {
>    	.name			= "linux_spi",
>    	.type			= OTHER,
> .devices.note		= "Device files /dev/spidev*.*\n",
> .devs.note		= "Device files /dev/spidev*.*\n",
> .init			= linux_spi_init,
> .map_flash_region	= fallback_map,
> .unmap_flash_region	= fallback_unmap,
>
> diff --git a/programmer.h b/programmer.h
> index 21fa707..90033a9 100644
> --- a/programmer.h
> +++ b/programmer.h
> @@ -96,15 +96,22 @@ enum programmer_type {
>  struct programmer_entry {
>    const char *vendor;
>    const char *name;
>    const enum programmer_type type;
>    union {
>    	const struct pcidev_status *const pci;
>    	const struct usbdev_status *const usb;
>    	const char *const note;
> } devices;
>
> } devs;
>
> int (*init) (void);

[PATCH 3/3] [RFC] Unify usbdev_status and pcidev_status into dev_status.

[Stefan Tauner]

On Sun, 18 Nov 2012 20:53:43 +0100
Carl-Daniel Hailfinger  wrote:
> Is there a reason you introduce a .devices struct member inside struct
> programmer_entry in patch 1 just to rename it to .dev in patch 3?

not really (of course). probably some column limit made me do it :P
which one do you prefer - devices or devs? i'll change the first patch
accordingly if needed. if you (or anybody else for that matter) do not
respond till i am ready with the patch set, then i will use 'devs'.

[PATCH 3/3] [RFC] Unify usbdev_status and pcidev_status into dev_status.

[Stefan Tauner]

On Wed, 12 Dec 2012 04:00:27 +0100
Stefan Tauner  wrote:
> On Sun, 18 Nov 2012 20:53:43 +0100
> Carl-Daniel Hailfinger  wrote:
> > Is there a reason you introduce a .devices struct member inside struct
> > programmer_entry in patch 1 just to rename it to .dev in patch 3?
>
> not really (of course). probably some column limit made me do it :P
> which one do you prefer - devices or devs? i'll change the first patch
> accordingly if needed. if you (or anybody else for that matter) do not
> respond till i am ready with the patch set, then i will use 'devs'.

and more importantly: is that the only thing you want to discus about
this patch? :)

[PATCH 3/3] [RFC] Unify usbdev_status and pcidev_status into dev_status.

[Carl-Daniel Hailfinger]

Am 12.12.2012 04:00 schrieb Stefan Tauner:
> On Sun, 18 Nov 2012 20:53:43 +0100
> Carl-Daniel Hailfinger  wrote:
> > Is there a reason you introduce a .devices struct member inside struct
> > programmer_entry in patch 1 just to rename it to .dev in patch 3?
>
> not really (of course). probably some column limit made me do it :P
> which one do you prefer - devices or devs? i'll change the first patch
> accordingly if needed. if you (or anybody else for that matter) do not
> respond till i am ready with the patch set, then i will use 'devs'.

No preference, devs sounds fine to me.

Regards,
Carl-Daniel