Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1231060 > unrolled thread

[PATCH v2] mfd: lpc_ich: Separate device cells for clarity

Started byAaron Sierra <asierra@xes-inc.com>
First post2015-09-23 02:10 +0200
Last post2015-09-23 16:20 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2] mfd: lpc_ich: Separate device cells for clarity Aaron Sierra <asierra@xes-inc.com> - 2015-09-23 02:10 +0200
    Re: [PATCH v2] mfd: lpc_ich: Separate device cells for clarity Lee Jones <lee.jones@linaro.org> - 2015-09-23 02:40 +0200
      Re: [PATCH v2] mfd: lpc_ich: Separate device cells for clarity Aaron Sierra <asierra@xes-inc.com> - 2015-09-23 16:20 +0200

#1231060 — [PATCH v2] mfd: lpc_ich: Separate device cells for clarity

FromAaron Sierra <asierra@xes-inc.com>
Date2015-09-23 02:10 +0200
Subject[PATCH v2] mfd: lpc_ich: Separate device cells for clarity
Message-ID<qbFNg-4eK-17@gated-at.bofh.it>
The lpc_ich_cells array gives the wrong impression about the
relationship between the watchdog and GPIO devices. They are
completely distinct devices, so this patch separates the
array into distinct mfd_cell structs per device.

A side effect of removing the array, is that the lpc_cells enum
is no longer needed.

Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
---
 v2 - rebase onto 4.3-rc2

 drivers/mfd/lpc_ich.c | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index c5a9a08..b514f3c 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -132,24 +132,18 @@ static struct resource gpio_ich_res[] = {
 	},
 };
 
-enum lpc_cells {
-	LPC_WDT = 0,
-	LPC_GPIO,
+static struct mfd_cell lpc_ich_wdt_cell = {
+	.name = "iTCO_wdt",
+	.num_resources = ARRAY_SIZE(wdt_ich_res),
+	.resources = wdt_ich_res,
+	.ignore_resource_conflicts = true,
 };
 
-static struct mfd_cell lpc_ich_cells[] = {
-	[LPC_WDT] = {
-		.name = "iTCO_wdt",
-		.num_resources = ARRAY_SIZE(wdt_ich_res),
-		.resources = wdt_ich_res,
-		.ignore_resource_conflicts = true,
-	},
-	[LPC_GPIO] = {
-		.name = "gpio_ich",
-		.num_resources = ARRAY_SIZE(gpio_ich_res),
-		.resources = gpio_ich_res,
-		.ignore_resource_conflicts = true,
-	},
+static struct mfd_cell lpc_ich_gpio_cell = {
+	.name = "gpio_ich",
+	.num_resources = ARRAY_SIZE(gpio_ich_res),
+	.resources = gpio_ich_res,
+	.ignore_resource_conflicts = true,
 };
 
 /* chipset related info */
@@ -841,7 +835,7 @@ static int lpc_ich_finalize_wdt_cell(struct pci_dev *dev)
 	struct itco_wdt_platform_data *pdata;
 	struct lpc_ich_priv *priv = pci_get_drvdata(dev);
 	struct lpc_ich_info *info;
-	struct mfd_cell *cell = &lpc_ich_cells[LPC_WDT];
+	struct mfd_cell *cell = &lpc_ich_wdt_cell;
 
 	pdata = devm_kzalloc(&dev->dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
@@ -860,7 +854,7 @@ static int lpc_ich_finalize_wdt_cell(struct pci_dev *dev)
 static void lpc_ich_finalize_gpio_cell(struct pci_dev *dev)
 {
 	struct lpc_ich_priv *priv = pci_get_drvdata(dev);
-	struct mfd_cell *cell = &lpc_ich_cells[LPC_GPIO];
+	struct mfd_cell *cell = &lpc_ich_gpio_cell;
 
 	cell->platform_data = &lpc_chipset_info[priv->chipset];
 	cell->pdata_size = sizeof(struct lpc_ich_info);
@@ -904,7 +898,7 @@ static int lpc_ich_init_gpio(struct pci_dev *dev)
 	base_addr = base_addr_cfg & 0x0000ff80;
 	if (!base_addr) {
 		dev_notice(&dev->dev, "I/O space for ACPI uninitialized\n");
-		lpc_ich_cells[LPC_GPIO].num_resources--;
+		lpc_ich_gpio_cell.num_resources--;
 		goto gpe0_done;
 	}
 
@@ -918,7 +912,7 @@ static int lpc_ich_init_gpio(struct pci_dev *dev)
 		 * the platform_device subsystem doesn't see this resource
 		 * or it will register an invalid region.
 		 */
-		lpc_ich_cells[LPC_GPIO].num_resources--;
+		lpc_ich_gpio_cell.num_resources--;
 		acpi_conflict = true;
 	} else {
 		lpc_ich_enable_acpi_space(dev);
@@ -958,12 +952,12 @@ gpe0_done:
 
 	lpc_ich_finalize_gpio_cell(dev);
 	ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
-			      &lpc_ich_cells[LPC_GPIO], 1, NULL, 0, NULL);
+			      &lpc_ich_gpio_cell, 1, NULL, 0, NULL);
 
 gpio_done:
 	if (acpi_conflict)
 		pr_warn("Resource conflict(s) found affecting %s\n",
-				lpc_ich_cells[LPC_GPIO].name);
+				lpc_ich_gpio_cell.name);
 	return ret;
 }
 
@@ -1007,7 +1001,7 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
 	 */
 	if (lpc_chipset_info[priv->chipset].iTCO_version == 1) {
 		/* Don't register iomem for TCO ver 1 */
-		lpc_ich_cells[LPC_WDT].num_resources--;
+		lpc_ich_wdt_cell.num_resources--;
 	} else if (lpc_chipset_info[priv->chipset].iTCO_version == 2) {
 		pci_read_config_dword(dev, RCBABASE, &base_addr_cfg);
 		base_addr = base_addr_cfg & 0xffffc000;
@@ -1035,7 +1029,7 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
 		goto wdt_done;
 
 	ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
-			      &lpc_ich_cells[LPC_WDT], 1, NULL, 0, NULL);
+			      &lpc_ich_wdt_cell, 1, NULL, 0, NULL);
 
 wdt_done:
 	return ret;
-- 
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1231070

FromLee Jones <lee.jones@linaro.org>
Date2015-09-23 02:40 +0200
Message-ID<qbGgh-4Nq-11@gated-at.bofh.it>
In reply to#1231060
On Tue, 22 Sep 2015, Aaron Sierra wrote:

> The lpc_ich_cells array gives the wrong impression about the
> relationship between the watchdog and GPIO devices. They are
> completely distinct devices, so this patch separates the
> array into distinct mfd_cell structs per device.
> 
> A side effect of removing the array, is that the lpc_cells enum
> is no longer needed.
> 
> Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
> ---
>  v2 - rebase onto 4.3-rc2
> 
>  drivers/mfd/lpc_ich.c | 42 ++++++++++++++++++------------------------
>  1 file changed, 18 insertions(+), 24 deletions(-)

Looks like you didn't apply Andy's Ack!

Applied with Andy's Ack.

> diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> index c5a9a08..b514f3c 100644
> --- a/drivers/mfd/lpc_ich.c
> +++ b/drivers/mfd/lpc_ich.c
> @@ -132,24 +132,18 @@ static struct resource gpio_ich_res[] = {
>  	},
>  };
>  
> -enum lpc_cells {
> -	LPC_WDT = 0,
> -	LPC_GPIO,
> +static struct mfd_cell lpc_ich_wdt_cell = {
> +	.name = "iTCO_wdt",
> +	.num_resources = ARRAY_SIZE(wdt_ich_res),
> +	.resources = wdt_ich_res,
> +	.ignore_resource_conflicts = true,
>  };
>  
> -static struct mfd_cell lpc_ich_cells[] = {
> -	[LPC_WDT] = {
> -		.name = "iTCO_wdt",
> -		.num_resources = ARRAY_SIZE(wdt_ich_res),
> -		.resources = wdt_ich_res,
> -		.ignore_resource_conflicts = true,
> -	},
> -	[LPC_GPIO] = {
> -		.name = "gpio_ich",
> -		.num_resources = ARRAY_SIZE(gpio_ich_res),
> -		.resources = gpio_ich_res,
> -		.ignore_resource_conflicts = true,
> -	},
> +static struct mfd_cell lpc_ich_gpio_cell = {
> +	.name = "gpio_ich",
> +	.num_resources = ARRAY_SIZE(gpio_ich_res),
> +	.resources = gpio_ich_res,
> +	.ignore_resource_conflicts = true,
>  };
>  
>  /* chipset related info */
> @@ -841,7 +835,7 @@ static int lpc_ich_finalize_wdt_cell(struct pci_dev *dev)
>  	struct itco_wdt_platform_data *pdata;
>  	struct lpc_ich_priv *priv = pci_get_drvdata(dev);
>  	struct lpc_ich_info *info;
> -	struct mfd_cell *cell = &lpc_ich_cells[LPC_WDT];
> +	struct mfd_cell *cell = &lpc_ich_wdt_cell;
>  
>  	pdata = devm_kzalloc(&dev->dev, sizeof(*pdata), GFP_KERNEL);
>  	if (!pdata)
> @@ -860,7 +854,7 @@ static int lpc_ich_finalize_wdt_cell(struct pci_dev *dev)
>  static void lpc_ich_finalize_gpio_cell(struct pci_dev *dev)
>  {
>  	struct lpc_ich_priv *priv = pci_get_drvdata(dev);
> -	struct mfd_cell *cell = &lpc_ich_cells[LPC_GPIO];
> +	struct mfd_cell *cell = &lpc_ich_gpio_cell;
>  
>  	cell->platform_data = &lpc_chipset_info[priv->chipset];
>  	cell->pdata_size = sizeof(struct lpc_ich_info);
> @@ -904,7 +898,7 @@ static int lpc_ich_init_gpio(struct pci_dev *dev)
>  	base_addr = base_addr_cfg & 0x0000ff80;
>  	if (!base_addr) {
>  		dev_notice(&dev->dev, "I/O space for ACPI uninitialized\n");
> -		lpc_ich_cells[LPC_GPIO].num_resources--;
> +		lpc_ich_gpio_cell.num_resources--;
>  		goto gpe0_done;
>  	}
>  
> @@ -918,7 +912,7 @@ static int lpc_ich_init_gpio(struct pci_dev *dev)
>  		 * the platform_device subsystem doesn't see this resource
>  		 * or it will register an invalid region.
>  		 */
> -		lpc_ich_cells[LPC_GPIO].num_resources--;
> +		lpc_ich_gpio_cell.num_resources--;
>  		acpi_conflict = true;
>  	} else {
>  		lpc_ich_enable_acpi_space(dev);
> @@ -958,12 +952,12 @@ gpe0_done:
>  
>  	lpc_ich_finalize_gpio_cell(dev);
>  	ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
> -			      &lpc_ich_cells[LPC_GPIO], 1, NULL, 0, NULL);
> +			      &lpc_ich_gpio_cell, 1, NULL, 0, NULL);
>  
>  gpio_done:
>  	if (acpi_conflict)
>  		pr_warn("Resource conflict(s) found affecting %s\n",
> -				lpc_ich_cells[LPC_GPIO].name);
> +				lpc_ich_gpio_cell.name);
>  	return ret;
>  }
>  
> @@ -1007,7 +1001,7 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
>  	 */
>  	if (lpc_chipset_info[priv->chipset].iTCO_version == 1) {
>  		/* Don't register iomem for TCO ver 1 */
> -		lpc_ich_cells[LPC_WDT].num_resources--;
> +		lpc_ich_wdt_cell.num_resources--;
>  	} else if (lpc_chipset_info[priv->chipset].iTCO_version == 2) {
>  		pci_read_config_dword(dev, RCBABASE, &base_addr_cfg);
>  		base_addr = base_addr_cfg & 0xffffc000;
> @@ -1035,7 +1029,7 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
>  		goto wdt_done;
>  
>  	ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
> -			      &lpc_ich_cells[LPC_WDT], 1, NULL, 0, NULL);
> +			      &lpc_ich_wdt_cell, 1, NULL, 0, NULL);
>  
>  wdt_done:
>  	return ret;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1231466

FromAaron Sierra <asierra@xes-inc.com>
Date2015-09-23 16:20 +0200
Message-ID<qbT3Q-6As-5@gated-at.bofh.it>
In reply to#1231070
----- Original Message -----
> From: "Lee Jones" <lee.jones@linaro.org>
> Sent: Tuesday, September 22, 2015 7:31:19 PM
> 
> On Tue, 22 Sep 2015, Aaron Sierra wrote:
> 
> > The lpc_ich_cells array gives the wrong impression about the
> > relationship between the watchdog and GPIO devices. They are
> > completely distinct devices, so this patch separates the
> > array into distinct mfd_cell structs per device.
> > 
> > A side effect of removing the array, is that the lpc_cells enum
> > is no longer needed.
> > 
> > Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
> > ---
> >  v2 - rebase onto 4.3-rc2
> > 
> >  drivers/mfd/lpc_ich.c | 42 ++++++++++++++++++------------------------
> >  1 file changed, 18 insertions(+), 24 deletions(-)
> 
> Looks like you didn't apply Andy's Ack!

Sorry, Andy! I erred on the side of caution. I wasn't sure if
the Ack applied to the concept or the iteration of the patch.

> Applied with Andy's Ack.

Thanks

-Aaron
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web