Re: [PATCH v2 1/4] mfd: lpc_sch: reduce duplicate code and improve manageability
On Tue, 02 Sep 2014, Andy Shevchenko wrote:
> This patch refactors the driver to use helper functions instead of
> copy'n'pasted pieces of code.
>
> It also introduces an additional struct to hold a chipset info. The chipset
> info will be used to store features that are supported by specific processor
> or
> chipset. LPC_SCH supports SMBUS, GPIO and WDT features. As this code base
> might
> expand further to support more processors, this implementation will help to
> keep code base clean and manageable.
>
> The patch is partially based on the work done by Chang Rebecca Swee Fun.
>
> Tested-by: Chang Rebecca Swee Fun
> Signed-off-by: Andy Shevchenko
> ---
> drivers/mfd/lpc_sch.c | 181
> +++---
> 1 file changed, 99 insertions(+), 82 deletions(-)
Applied, thanks.
> diff --git a/drivers/mfd/lpc_sch.c b/drivers/mfd/lpc_sch.c
> index 4ee7550..bde070a 100644
> --- a/drivers/mfd/lpc_sch.c
> +++ b/drivers/mfd/lpc_sch.c
> @@ -40,120 +40,137 @@
> #define WDTBASE 0x84
> #define WDT_IO_SIZE 64
>
> -static struct resource smbus_sch_resource = {
> - .flags = IORESOURCE_IO,
> +enum sch_chipsets {
> + LPC_SCH = 0,/* Intel Poulsbo SCH */
> + LPC_ITC,/* Intel Tunnel Creek */
> + LPC_CENTERTON, /* Intel Centerton */
> };
>
> -static struct resource gpio_sch_resource = {
> - .flags = IORESOURCE_IO,
> +struct lpc_sch_info {
> + unsigned int io_size_smbus;
> + unsigned int io_size_gpio;
> + unsigned int io_size_wdt;
> };
>
> -static struct resource wdt_sch_resource = {
> - .flags = IORESOURCE_IO,
> -};
> -
> -static struct mfd_cell lpc_sch_cells[3];
> -
> -static struct mfd_cell isch_smbus_cell = {
> - .name = "isch_smbus",
> - .num_resources = 1,
> - .resources = _sch_resource,
> - .ignore_resource_conflicts = true,
> -};
> -
> -static struct mfd_cell sch_gpio_cell = {
> - .name = "sch_gpio",
> - .num_resources = 1,
> - .resources = _sch_resource,
> - .ignore_resource_conflicts = true,
> -};
> -
> -static struct mfd_cell wdt_sch_cell = {
> - .name = "ie6xx_wdt",
> - .num_resources = 1,
> - .resources = _sch_resource,
> - .ignore_resource_conflicts = true,
> +static struct lpc_sch_info sch_chipset_info[] = {
> + [LPC_SCH] = {
> + .io_size_smbus = SMBUS_IO_SIZE,
> + .io_size_gpio = GPIO_IO_SIZE,
> + },
> + [LPC_ITC] = {
> + .io_size_smbus = SMBUS_IO_SIZE,
> + .io_size_gpio = GPIO_IO_SIZE,
> + .io_size_wdt = WDT_IO_SIZE,
> + },
> + [LPC_CENTERTON] = {
> + .io_size_smbus = SMBUS_IO_SIZE,
> + .io_size_gpio = GPIO_IO_SIZE_CENTERTON,
> + .io_size_wdt = WDT_IO_SIZE,
> + },
> };
>
> static const struct pci_device_id lpc_sch_ids[] = {
> - { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SCH_LPC) },
> - { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ITC_LPC) },
> - { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CENTERTON_ILB) },
> + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SCH_LPC), LPC_SCH },
> + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ITC_LPC), LPC_ITC },
> + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_CENTERTON_ILB), LPC_CENTERTON
> },
> { 0, }
> };
> MODULE_DEVICE_TABLE(pci, lpc_sch_ids);
>
> -static int lpc_sch_probe(struct pci_dev *dev,
> - const struct pci_device_id *id)
> +#define LPC_NO_RESOURCE 1
> +#define LPC_SKIP_RESOURCE2
> +
> +static int lpc_sch_get_io(struct pci_dev *pdev, int where, const char *name,
> + struct resource *res, int size)
> {
> unsigned int base_addr_cfg;
> unsigned short base_addr;
> - int i, cells = 0;
> - int ret;
>
> - pci_read_config_dword(dev, SMBASE, _addr_cfg);
> + if (size == 0)
> + return LPC_NO_RESOURCE;
> +
> + pci_read_config_dword(pdev, where, _addr_cfg);
> base_addr = 0;
> if (!(base_addr_cfg & (1 << 31)))
> - dev_warn(>dev, "Decode of the SMBus I/O range disabled\n");
> + dev_warn(>dev, "Decode of the %s I/O range disabled\n",
> + name);
> else
> base_addr = (unsigned short)base_addr_cfg;
>
> if (base_addr == 0) {
> - dev_warn(>dev, "I/O space for SMBus uninitialized\n");
> - } else {
> - lpc_sch_cells[cells++] = isch_smbus_cell;
> - smbus_sch_resource.start = base_addr;
> - smbus_sch_resource.end = base_addr + SMBUS_IO_SIZE - 1;
> + dev_warn(>dev, "I/O space for %s uninitialized\n", name);
> + return LPC_SKIP_RESOURCE;
> }
>
> - pci_read_config_dword(dev, GPIOBASE, _addr_cfg);
> - base_addr = 0;
> - if (!(base_addr_cfg & (1 << 31)))
> - dev_warn(>dev, "Decode of the GPIO I/O range
[PATCH v2 1/4] mfd: lpc_sch: reduce duplicate code and improve manageability
This patch refactors the driver to use helper functions instead of
copy'n'pasted pieces of code.
It also introduces an additional struct to hold a chipset info. The chipset
info will be used to store features that are supported by specific processor or
chipset. LPC_SCH supports SMBUS, GPIO and WDT features. As this code base might
expand further to support more processors, this implementation will help to
keep code base clean and manageable.
The patch is partially based on the work done by Chang Rebecca Swee Fun.
Tested-by: Chang Rebecca Swee Fun
Signed-off-by: Andy Shevchenko
---
drivers/mfd/lpc_sch.c | 181 +++---
1 file changed, 99 insertions(+), 82 deletions(-)
diff --git a/drivers/mfd/lpc_sch.c b/drivers/mfd/lpc_sch.c
index 4ee7550..bde070a 100644
--- a/drivers/mfd/lpc_sch.c
+++ b/drivers/mfd/lpc_sch.c
@@ -40,120 +40,137 @@
#define WDTBASE0x84
#define WDT_IO_SIZE64
-static struct resource smbus_sch_resource = {
- .flags = IORESOURCE_IO,
+enum sch_chipsets {
+ LPC_SCH = 0,/* Intel Poulsbo SCH */
+ LPC_ITC,/* Intel Tunnel Creek */
+ LPC_CENTERTON, /* Intel Centerton */
};
-static struct resource gpio_sch_resource = {
- .flags = IORESOURCE_IO,
+struct lpc_sch_info {
+ unsigned int io_size_smbus;
+ unsigned int io_size_gpio;
+ unsigned int io_size_wdt;
};
-static struct resource wdt_sch_resource = {
- .flags = IORESOURCE_IO,
-};
-
-static struct mfd_cell lpc_sch_cells[3];
-
-static struct mfd_cell isch_smbus_cell = {
- .name = "isch_smbus",
- .num_resources = 1,
- .resources = _sch_resource,
- .ignore_resource_conflicts = true,
-};
-
-static struct mfd_cell sch_gpio_cell = {
- .name = "sch_gpio",
- .num_resources = 1,
- .resources = _sch_resource,
- .ignore_resource_conflicts = true,
-};
-
-static struct mfd_cell wdt_sch_cell = {
- .name = "ie6xx_wdt",
- .num_resources = 1,
- .resources = _sch_resource,
- .ignore_resource_conflicts = true,
+static struct lpc_sch_info sch_chipset_info[] = {
+ [LPC_SCH] = {
+ .io_size_smbus = SMBUS_IO_SIZE,
+ .io_size_gpio = GPIO_IO_SIZE,
+ },
+ [LPC_ITC] = {
+ .io_size_smbus = SMBUS_IO_SIZE,
+ .io_size_gpio = GPIO_IO_SIZE,
+ .io_size_wdt = WDT_IO_SIZE,
+ },
+ [LPC_CENTERTON] = {
+ .io_size_smbus = SMBUS_IO_SIZE,
+ .io_size_gpio = GPIO_IO_SIZE_CENTERTON,
+ .io_size_wdt = WDT_IO_SIZE,
+ },
};
static const struct pci_device_id lpc_sch_ids[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SCH_LPC) },
- { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ITC_LPC) },
- { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CENTERTON_ILB) },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SCH_LPC), LPC_SCH },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ITC_LPC), LPC_ITC },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_CENTERTON_ILB), LPC_CENTERTON
},
{ 0, }
};
MODULE_DEVICE_TABLE(pci, lpc_sch_ids);
-static int lpc_sch_probe(struct pci_dev *dev,
- const struct pci_device_id *id)
+#define LPC_NO_RESOURCE1
+#define LPC_SKIP_RESOURCE 2
+
+static int lpc_sch_get_io(struct pci_dev *pdev, int where, const char *name,
+ struct resource *res, int size)
{
unsigned int base_addr_cfg;
unsigned short base_addr;
- int i, cells = 0;
- int ret;
- pci_read_config_dword(dev, SMBASE, _addr_cfg);
+ if (size == 0)
+ return LPC_NO_RESOURCE;
+
+ pci_read_config_dword(pdev, where, _addr_cfg);
base_addr = 0;
if (!(base_addr_cfg & (1 << 31)))
- dev_warn(>dev, "Decode of the SMBus I/O range disabled\n");
+ dev_warn(>dev, "Decode of the %s I/O range disabled\n",
+name);
else
base_addr = (unsigned short)base_addr_cfg;
if (base_addr == 0) {
- dev_warn(>dev, "I/O space for SMBus uninitialized\n");
- } else {
- lpc_sch_cells[cells++] = isch_smbus_cell;
- smbus_sch_resource.start = base_addr;
- smbus_sch_resource.end = base_addr + SMBUS_IO_SIZE - 1;
+ dev_warn(>dev, "I/O space for %s uninitialized\n", name);
+ return LPC_SKIP_RESOURCE;
}
- pci_read_config_dword(dev, GPIOBASE, _addr_cfg);
- base_addr = 0;
- if (!(base_addr_cfg & (1 << 31)))
- dev_warn(>dev, "Decode of the GPIO I/O range disabled\n");
- else
- base_addr = (unsigned short)base_addr_cfg;
+ res->start = base_addr;
+ res->end = base_addr + size - 1;
+ res->flags = IORESOURCE_IO;
-
[PATCH v2 1/4] mfd: lpc_sch: reduce duplicate code and improve manageability
This patch refactors the driver to use helper functions instead of
copy'n'pasted pieces of code.
It also introduces an additional struct to hold a chipset info. The chipset
info will be used to store features that are supported by specific processor or
chipset. LPC_SCH supports SMBUS, GPIO and WDT features. As this code base might
expand further to support more processors, this implementation will help to
keep code base clean and manageable.
The patch is partially based on the work done by Chang Rebecca Swee Fun.
Tested-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
drivers/mfd/lpc_sch.c | 181 +++---
1 file changed, 99 insertions(+), 82 deletions(-)
diff --git a/drivers/mfd/lpc_sch.c b/drivers/mfd/lpc_sch.c
index 4ee7550..bde070a 100644
--- a/drivers/mfd/lpc_sch.c
+++ b/drivers/mfd/lpc_sch.c
@@ -40,120 +40,137 @@
#define WDTBASE0x84
#define WDT_IO_SIZE64
-static struct resource smbus_sch_resource = {
- .flags = IORESOURCE_IO,
+enum sch_chipsets {
+ LPC_SCH = 0,/* Intel Poulsbo SCH */
+ LPC_ITC,/* Intel Tunnel Creek */
+ LPC_CENTERTON, /* Intel Centerton */
};
-static struct resource gpio_sch_resource = {
- .flags = IORESOURCE_IO,
+struct lpc_sch_info {
+ unsigned int io_size_smbus;
+ unsigned int io_size_gpio;
+ unsigned int io_size_wdt;
};
-static struct resource wdt_sch_resource = {
- .flags = IORESOURCE_IO,
-};
-
-static struct mfd_cell lpc_sch_cells[3];
-
-static struct mfd_cell isch_smbus_cell = {
- .name = isch_smbus,
- .num_resources = 1,
- .resources = smbus_sch_resource,
- .ignore_resource_conflicts = true,
-};
-
-static struct mfd_cell sch_gpio_cell = {
- .name = sch_gpio,
- .num_resources = 1,
- .resources = gpio_sch_resource,
- .ignore_resource_conflicts = true,
-};
-
-static struct mfd_cell wdt_sch_cell = {
- .name = ie6xx_wdt,
- .num_resources = 1,
- .resources = wdt_sch_resource,
- .ignore_resource_conflicts = true,
+static struct lpc_sch_info sch_chipset_info[] = {
+ [LPC_SCH] = {
+ .io_size_smbus = SMBUS_IO_SIZE,
+ .io_size_gpio = GPIO_IO_SIZE,
+ },
+ [LPC_ITC] = {
+ .io_size_smbus = SMBUS_IO_SIZE,
+ .io_size_gpio = GPIO_IO_SIZE,
+ .io_size_wdt = WDT_IO_SIZE,
+ },
+ [LPC_CENTERTON] = {
+ .io_size_smbus = SMBUS_IO_SIZE,
+ .io_size_gpio = GPIO_IO_SIZE_CENTERTON,
+ .io_size_wdt = WDT_IO_SIZE,
+ },
};
static const struct pci_device_id lpc_sch_ids[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SCH_LPC) },
- { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ITC_LPC) },
- { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CENTERTON_ILB) },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SCH_LPC), LPC_SCH },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ITC_LPC), LPC_ITC },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_CENTERTON_ILB), LPC_CENTERTON
},
{ 0, }
};
MODULE_DEVICE_TABLE(pci, lpc_sch_ids);
-static int lpc_sch_probe(struct pci_dev *dev,
- const struct pci_device_id *id)
+#define LPC_NO_RESOURCE1
+#define LPC_SKIP_RESOURCE 2
+
+static int lpc_sch_get_io(struct pci_dev *pdev, int where, const char *name,
+ struct resource *res, int size)
{
unsigned int base_addr_cfg;
unsigned short base_addr;
- int i, cells = 0;
- int ret;
- pci_read_config_dword(dev, SMBASE, base_addr_cfg);
+ if (size == 0)
+ return LPC_NO_RESOURCE;
+
+ pci_read_config_dword(pdev, where, base_addr_cfg);
base_addr = 0;
if (!(base_addr_cfg (1 31)))
- dev_warn(dev-dev, Decode of the SMBus I/O range disabled\n);
+ dev_warn(pdev-dev, Decode of the %s I/O range disabled\n,
+name);
else
base_addr = (unsigned short)base_addr_cfg;
if (base_addr == 0) {
- dev_warn(dev-dev, I/O space for SMBus uninitialized\n);
- } else {
- lpc_sch_cells[cells++] = isch_smbus_cell;
- smbus_sch_resource.start = base_addr;
- smbus_sch_resource.end = base_addr + SMBUS_IO_SIZE - 1;
+ dev_warn(pdev-dev, I/O space for %s uninitialized\n, name);
+ return LPC_SKIP_RESOURCE;
}
- pci_read_config_dword(dev, GPIOBASE, base_addr_cfg);
- base_addr = 0;
- if (!(base_addr_cfg (1 31)))
- dev_warn(dev-dev, Decode of the GPIO I/O range disabled\n);
- else
- base_addr = (unsigned short)base_addr_cfg;
+ res-start = base_addr;
Re: [PATCH v2 1/4] mfd: lpc_sch: reduce duplicate code and improve manageability
On Tue, 02 Sep 2014, Andy Shevchenko wrote:
This patch refactors the driver to use helper functions instead of
copy'n'pasted pieces of code.
It also introduces an additional struct to hold a chipset info. The chipset
info will be used to store features that are supported by specific processor
or
chipset. LPC_SCH supports SMBUS, GPIO and WDT features. As this code base
might
expand further to support more processors, this implementation will help to
keep code base clean and manageable.
The patch is partially based on the work done by Chang Rebecca Swee Fun.
Tested-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
drivers/mfd/lpc_sch.c | 181
+++---
1 file changed, 99 insertions(+), 82 deletions(-)
Applied, thanks.
diff --git a/drivers/mfd/lpc_sch.c b/drivers/mfd/lpc_sch.c
index 4ee7550..bde070a 100644
--- a/drivers/mfd/lpc_sch.c
+++ b/drivers/mfd/lpc_sch.c
@@ -40,120 +40,137 @@
#define WDTBASE 0x84
#define WDT_IO_SIZE 64
-static struct resource smbus_sch_resource = {
- .flags = IORESOURCE_IO,
+enum sch_chipsets {
+ LPC_SCH = 0,/* Intel Poulsbo SCH */
+ LPC_ITC,/* Intel Tunnel Creek */
+ LPC_CENTERTON, /* Intel Centerton */
};
-static struct resource gpio_sch_resource = {
- .flags = IORESOURCE_IO,
+struct lpc_sch_info {
+ unsigned int io_size_smbus;
+ unsigned int io_size_gpio;
+ unsigned int io_size_wdt;
};
-static struct resource wdt_sch_resource = {
- .flags = IORESOURCE_IO,
-};
-
-static struct mfd_cell lpc_sch_cells[3];
-
-static struct mfd_cell isch_smbus_cell = {
- .name = isch_smbus,
- .num_resources = 1,
- .resources = smbus_sch_resource,
- .ignore_resource_conflicts = true,
-};
-
-static struct mfd_cell sch_gpio_cell = {
- .name = sch_gpio,
- .num_resources = 1,
- .resources = gpio_sch_resource,
- .ignore_resource_conflicts = true,
-};
-
-static struct mfd_cell wdt_sch_cell = {
- .name = ie6xx_wdt,
- .num_resources = 1,
- .resources = wdt_sch_resource,
- .ignore_resource_conflicts = true,
+static struct lpc_sch_info sch_chipset_info[] = {
+ [LPC_SCH] = {
+ .io_size_smbus = SMBUS_IO_SIZE,
+ .io_size_gpio = GPIO_IO_SIZE,
+ },
+ [LPC_ITC] = {
+ .io_size_smbus = SMBUS_IO_SIZE,
+ .io_size_gpio = GPIO_IO_SIZE,
+ .io_size_wdt = WDT_IO_SIZE,
+ },
+ [LPC_CENTERTON] = {
+ .io_size_smbus = SMBUS_IO_SIZE,
+ .io_size_gpio = GPIO_IO_SIZE_CENTERTON,
+ .io_size_wdt = WDT_IO_SIZE,
+ },
};
static const struct pci_device_id lpc_sch_ids[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SCH_LPC) },
- { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ITC_LPC) },
- { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CENTERTON_ILB) },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SCH_LPC), LPC_SCH },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ITC_LPC), LPC_ITC },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_CENTERTON_ILB), LPC_CENTERTON
},
{ 0, }
};
MODULE_DEVICE_TABLE(pci, lpc_sch_ids);
-static int lpc_sch_probe(struct pci_dev *dev,
- const struct pci_device_id *id)
+#define LPC_NO_RESOURCE 1
+#define LPC_SKIP_RESOURCE2
+
+static int lpc_sch_get_io(struct pci_dev *pdev, int where, const char *name,
+ struct resource *res, int size)
{
unsigned int base_addr_cfg;
unsigned short base_addr;
- int i, cells = 0;
- int ret;
- pci_read_config_dword(dev, SMBASE, base_addr_cfg);
+ if (size == 0)
+ return LPC_NO_RESOURCE;
+
+ pci_read_config_dword(pdev, where, base_addr_cfg);
base_addr = 0;
if (!(base_addr_cfg (1 31)))
- dev_warn(dev-dev, Decode of the SMBus I/O range disabled\n);
+ dev_warn(pdev-dev, Decode of the %s I/O range disabled\n,
+ name);
else
base_addr = (unsigned short)base_addr_cfg;
if (base_addr == 0) {
- dev_warn(dev-dev, I/O space for SMBus uninitialized\n);
- } else {
- lpc_sch_cells[cells++] = isch_smbus_cell;
- smbus_sch_resource.start = base_addr;
- smbus_sch_resource.end = base_addr + SMBUS_IO_SIZE - 1;
+ dev_warn(pdev-dev, I/O space for %s uninitialized\n, name);
+ return LPC_SKIP_RESOURCE;
}
- pci_read_config_dword(dev, GPIOBASE, base_addr_cfg);
- base_addr = 0;
- if (!(base_addr_cfg (1 31)))
- dev_warn(dev-dev, Decode of the GPIO I/O range disabled\n);
- else
- base_addr = (unsigned
