linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources"
@ 2021-03-02 13:56 Andy Shevchenko
  2021-03-02 13:56 ` [PATCH v1 2/5] mfd: intel_quark_i2c_gpio: Unregister resources in reversed order Andy Shevchenko
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Andy Shevchenko @ 2021-03-02 13:56 UTC (permalink / raw)
  To: Andy Shevchenko, linux-kernel; +Cc: Lee Jones, Rikard Falkeborn

The structures are used as place holders, so they are modified at run-time.
Obviously they may not be constants.

  BUG: unable to handle page fault for address: d0643220
  ...
  CPU: 0 PID: 110 Comm: modprobe Not tainted 5.11.0+ #1
  Hardware name: Intel Corp. QUARK/GalileoGen2, BIOS 0x01000200 01/01/2014
  EIP: intel_quark_mfd_probe+0x93/0x1c0 [intel_quark_i2c_gpio]

This partially reverts the commit c4a164f41554d2899bed94bdcc499263f41787b4.

While at it, add a comment to avoid similar changes in the future.

Fixes: c4a164f41554 ("mfd: Constify static struct resources")
Cc: Rikard Falkeborn <rikard.falkeborn@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 55a9e017edee..124c0ee84169 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -72,7 +72,8 @@ static const struct dmi_system_id dmi_platform_info[] = {
 	{}
 };
 
-static const struct resource intel_quark_i2c_res[] = {
+/* This is used as a place holder and will be modified at run-time */
+static struct resource intel_quark_i2c_res[] = {
 	[INTEL_QUARK_IORES_MEM] = {
 		.flags = IORESOURCE_MEM,
 	},
@@ -85,7 +86,8 @@ static struct mfd_cell_acpi_match intel_quark_acpi_match_i2c = {
 	.adr = MFD_ACPI_MATCH_I2C,
 };
 
-static const struct resource intel_quark_gpio_res[] = {
+/* This is used as a place holder and will be modified at run-time */
+static struct resource intel_quark_gpio_res[] = {
 	[INTEL_QUARK_IORES_MEM] = {
 		.flags = IORESOURCE_MEM,
 	},
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 2/5] mfd: intel_quark_i2c_gpio: Unregister resources in reversed order
  2021-03-02 13:56 [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Andy Shevchenko
@ 2021-03-02 13:56 ` Andy Shevchenko
  2021-03-23  9:21   ` Lee Jones
  2021-03-02 13:56 ` [PATCH v1 3/5] mfd: intel_quark_i2c_gpio: Remove unused struct device member Andy Shevchenko
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2021-03-02 13:56 UTC (permalink / raw)
  To: Andy Shevchenko, linux-kernel; +Cc: Lee Jones

In ->remove() unregister resources in reversed order, i.e. MFD devices first
followed by I²C clock.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 124c0ee84169..db756b74195a 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -269,8 +269,8 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
 
 static void intel_quark_mfd_remove(struct pci_dev *pdev)
 {
-	intel_quark_unregister_i2c_clk(&pdev->dev);
 	mfd_remove_devices(&pdev->dev);
+	intel_quark_unregister_i2c_clk(&pdev->dev);
 }
 
 static struct pci_driver intel_quark_mfd_driver = {
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 3/5] mfd: intel_quark_i2c_gpio: Remove unused struct device member
  2021-03-02 13:56 [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Andy Shevchenko
  2021-03-02 13:56 ` [PATCH v1 2/5] mfd: intel_quark_i2c_gpio: Unregister resources in reversed order Andy Shevchenko
@ 2021-03-02 13:56 ` Andy Shevchenko
  2021-03-23  9:22   ` Lee Jones
  2021-03-02 13:56 ` [PATCH v1 4/5] mfd: intel_quark_i2c_gpio: Replace I²C speeds with descriptive definitions Andy Shevchenko
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2021-03-02 13:56 UTC (permalink / raw)
  To: Andy Shevchenko, linux-kernel; +Cc: Lee Jones

The device pointer in the custom structure is not used anywhere,
remove it for good.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index db756b74195a..053780539175 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -45,7 +45,6 @@
 #define INTEL_QUARK_I2C_CLK_HZ	33000000
 
 struct intel_quark_mfd {
-	struct device		*dev;
 	struct clk		*i2c_clk;
 	struct clk_lookup	*i2c_clk_lookup;
 };
@@ -239,7 +238,6 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
 	if (!quark_mfd)
 		return -ENOMEM;
 
-	quark_mfd->dev = &pdev->dev;
 	dev_set_drvdata(&pdev->dev, quark_mfd);
 
 	ret = intel_quark_register_i2c_clk(&pdev->dev);
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 4/5] mfd: intel_quark_i2c_gpio: Replace I²C speeds with descriptive definitions
  2021-03-02 13:56 [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Andy Shevchenko
  2021-03-02 13:56 ` [PATCH v1 2/5] mfd: intel_quark_i2c_gpio: Unregister resources in reversed order Andy Shevchenko
  2021-03-02 13:56 ` [PATCH v1 3/5] mfd: intel_quark_i2c_gpio: Remove unused struct device member Andy Shevchenko
@ 2021-03-02 13:56 ` Andy Shevchenko
  2021-03-23  9:22   ` Lee Jones
  2021-03-02 13:56 ` [PATCH v1 5/5] mfd: intel_quark_i2c_gpio: Reuse BAR definitions for MFD cell indexing Andy Shevchenko
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2021-03-02 13:56 UTC (permalink / raw)
  To: Andy Shevchenko, linux-kernel; +Cc: Lee Jones

I²C header provides a descriptive definitions for standard bus speeds.
Use them instead of plain numbers.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 053780539175..7af22d1399e1 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -16,6 +16,7 @@
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/dmi.h>
+#include <linux/i2c.h>
 #include <linux/platform_data/gpio-dwapb.h>
 #include <linux/platform_data/i2c-designware.h>
 
@@ -54,19 +55,19 @@ static const struct dmi_system_id dmi_platform_info[] = {
 		.matches = {
 			DMI_EXACT_MATCH(DMI_BOARD_NAME, "Galileo"),
 		},
-		.driver_data = (void *)100000,
+		.driver_data = (void *)I2C_MAX_STANDARD_MODE_FREQ,
 	},
 	{
 		.matches = {
 			DMI_EXACT_MATCH(DMI_BOARD_NAME, "GalileoGen2"),
 		},
-		.driver_data = (void *)400000,
+		.driver_data = (void *)I2C_MAX_FAST_MODE_FREQ,
 	},
 	{
 		.matches = {
 			DMI_EXACT_MATCH(DMI_BOARD_NAME, "SIMATIC IOT2000"),
 		},
-		.driver_data = (void *)400000,
+		.driver_data = (void *)I2C_MAX_FAST_MODE_FREQ,
 	},
 	{}
 };
@@ -176,7 +177,7 @@ static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 		return -ENOMEM;
 
 	/* Normal mode by default */
-	pdata->i2c_scl_freq = 100000;
+	pdata->i2c_scl_freq = I2C_MAX_STANDARD_MODE_FREQ;
 
 	dmi_id = dmi_first_match(dmi_platform_info);
 	if (dmi_id)
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 5/5] mfd: intel_quark_i2c_gpio: Reuse BAR definitions for MFD cell indexing
  2021-03-02 13:56 [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Andy Shevchenko
                   ` (2 preceding siblings ...)
  2021-03-02 13:56 ` [PATCH v1 4/5] mfd: intel_quark_i2c_gpio: Replace I²C speeds with descriptive definitions Andy Shevchenko
@ 2021-03-02 13:56 ` Andy Shevchenko
  2021-03-23  9:20   ` Lee Jones
  2021-03-02 20:59 ` [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Rikard Falkeborn
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2021-03-02 13:56 UTC (permalink / raw)
  To: Andy Shevchenko, linux-kernel; +Cc: Lee Jones

It's convenient and less error prone to use definitions to address
different cells in an array. For this purpose we may reuse existing
BAR definitions.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 7af22d1399e1..40871ce34e82 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -98,14 +98,6 @@ static struct mfd_cell_acpi_match intel_quark_acpi_match_gpio = {
 };
 
 static struct mfd_cell intel_quark_mfd_cells[] = {
-	{
-		.id = MFD_GPIO_BAR,
-		.name = "gpio-dwapb",
-		.acpi_match = &intel_quark_acpi_match_gpio,
-		.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
-		.resources = intel_quark_gpio_res,
-		.ignore_resource_conflicts = true,
-	},
 	{
 		.id = MFD_I2C_BAR,
 		.name = "i2c_designware",
@@ -114,6 +106,14 @@ static struct mfd_cell intel_quark_mfd_cells[] = {
 		.resources = intel_quark_i2c_res,
 		.ignore_resource_conflicts = true,
 	},
+	{
+		.id = MFD_GPIO_BAR,
+		.name = "gpio-dwapb",
+		.acpi_match = &intel_quark_acpi_match_gpio,
+		.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
+		.resources = intel_quark_gpio_res,
+		.ignore_resource_conflicts = true,
+	},
 };
 
 static const struct pci_device_id intel_quark_mfd_ids[] = {
@@ -245,11 +245,11 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	ret = intel_quark_i2c_setup(pdev, &intel_quark_mfd_cells[1]);
+	ret = intel_quark_i2c_setup(pdev, &intel_quark_mfd_cells[MFD_I2C_BAR]);
 	if (ret)
 		goto err_unregister_i2c_clk;
 
-	ret = intel_quark_gpio_setup(pdev, &intel_quark_mfd_cells[0]);
+	ret = intel_quark_gpio_setup(pdev, &intel_quark_mfd_cells[MFD_GPIO_BAR]);
 	if (ret)
 		goto err_unregister_i2c_clk;
 
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources"
  2021-03-02 13:56 [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Andy Shevchenko
                   ` (3 preceding siblings ...)
  2021-03-02 13:56 ` [PATCH v1 5/5] mfd: intel_quark_i2c_gpio: Reuse BAR definitions for MFD cell indexing Andy Shevchenko
@ 2021-03-02 20:59 ` Rikard Falkeborn
  2021-03-18 12:53   ` Andy Shevchenko
  2021-03-20 20:59 ` Tong Zhang
  2021-03-23  9:14 ` Lee Jones
  6 siblings, 1 reply; 17+ messages in thread
From: Rikard Falkeborn @ 2021-03-02 20:59 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel, Lee Jones, Rikard Falkeborn

On Tue, Mar 02, 2021 at 03:56:16PM +0200, Andy Shevchenko wrote:
> The structures are used as place holders, so they are modified at run-time.
> Obviously they may not be constants.
> 
>   BUG: unable to handle page fault for address: d0643220
>   ...
>   CPU: 0 PID: 110 Comm: modprobe Not tainted 5.11.0+ #1
>   Hardware name: Intel Corp. QUARK/GalileoGen2, BIOS 0x01000200 01/01/2014
>   EIP: intel_quark_mfd_probe+0x93/0x1c0 [intel_quark_i2c_gpio]
> 
> This partially reverts the commit c4a164f41554d2899bed94bdcc499263f41787b4.
> 
> While at it, add a comment to avoid similar changes in the future.
> 
> Fixes: c4a164f41554 ("mfd: Constify static struct resources")
> Cc: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_quark_i2c_gpio.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index 55a9e017edee..124c0ee84169 100644
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -72,7 +72,8 @@ static const struct dmi_system_id dmi_platform_info[] = {
>  	{}
>  };
>  
> -static const struct resource intel_quark_i2c_res[] = {
> +/* This is used as a place holder and will be modified at run-time */
> +static struct resource intel_quark_i2c_res[] = {
>  	[INTEL_QUARK_IORES_MEM] = {
>  		.flags = IORESOURCE_MEM,
>  	},
> @@ -85,7 +86,8 @@ static struct mfd_cell_acpi_match intel_quark_acpi_match_i2c = {
>  	.adr = MFD_ACPI_MATCH_I2C,
>  };
>  
> -static const struct resource intel_quark_gpio_res[] = {
> +/* This is used as a place holder and will be modified at run-time */
> +static struct resource intel_quark_gpio_res[] = {
>  	[INTEL_QUARK_IORES_MEM] = {
>  		.flags = IORESOURCE_MEM,
>  	},
> -- 
> 2.30.1
> 

Sorry about that :(

Reviewed-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources"
  2021-03-02 20:59 ` [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Rikard Falkeborn
@ 2021-03-18 12:53   ` Andy Shevchenko
  2021-03-19  8:39     ` Lee Jones
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2021-03-18 12:53 UTC (permalink / raw)
  To: Rikard Falkeborn; +Cc: linux-kernel, Lee Jones

On Tue, Mar 02, 2021 at 09:59:03PM +0100, Rikard Falkeborn wrote:
> On Tue, Mar 02, 2021 at 03:56:16PM +0200, Andy Shevchenko wrote:
> > The structures are used as place holders, so they are modified at run-time.
> > Obviously they may not be constants.
> > 
> >   BUG: unable to handle page fault for address: d0643220
> >   ...
> >   CPU: 0 PID: 110 Comm: modprobe Not tainted 5.11.0+ #1
> >   Hardware name: Intel Corp. QUARK/GalileoGen2, BIOS 0x01000200 01/01/2014
> >   EIP: intel_quark_mfd_probe+0x93/0x1c0 [intel_quark_i2c_gpio]
> > 
> > This partially reverts the commit c4a164f41554d2899bed94bdcc499263f41787b4.
> > 
> > While at it, add a comment to avoid similar changes in the future.
> > 
> > Fixes: c4a164f41554 ("mfd: Constify static struct resources")
> > Cc: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/mfd/intel_quark_i2c_gpio.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> > index 55a9e017edee..124c0ee84169 100644
> > --- a/drivers/mfd/intel_quark_i2c_gpio.c
> > +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> > @@ -72,7 +72,8 @@ static const struct dmi_system_id dmi_platform_info[] = {
> >  	{}
> >  };
> >  
> > -static const struct resource intel_quark_i2c_res[] = {
> > +/* This is used as a place holder and will be modified at run-time */
> > +static struct resource intel_quark_i2c_res[] = {
> >  	[INTEL_QUARK_IORES_MEM] = {
> >  		.flags = IORESOURCE_MEM,
> >  	},
> > @@ -85,7 +86,8 @@ static struct mfd_cell_acpi_match intel_quark_acpi_match_i2c = {
> >  	.adr = MFD_ACPI_MATCH_I2C,
> >  };
> >  
> > -static const struct resource intel_quark_gpio_res[] = {
> > +/* This is used as a place holder and will be modified at run-time */
> > +static struct resource intel_quark_gpio_res[] = {
> >  	[INTEL_QUARK_IORES_MEM] = {
> >  		.flags = IORESOURCE_MEM,
> >  	},
> > -- 
> > 2.30.1
> > 
> 
> Sorry about that :(
> 
> Reviewed-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>

Thanks for review!

Lee, this series has a critical bug fix, should I do something or you is going
to apply this soon?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources"
  2021-03-18 12:53   ` Andy Shevchenko
@ 2021-03-19  8:39     ` Lee Jones
  2021-03-19 10:01       ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Lee Jones @ 2021-03-19  8:39 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Rikard Falkeborn, linux-kernel

On Thu, 18 Mar 2021, Andy Shevchenko wrote:

> On Tue, Mar 02, 2021 at 09:59:03PM +0100, Rikard Falkeborn wrote:
> > On Tue, Mar 02, 2021 at 03:56:16PM +0200, Andy Shevchenko wrote:
> > > The structures are used as place holders, so they are modified at run-time.
> > > Obviously they may not be constants.
> > > 
> > >   BUG: unable to handle page fault for address: d0643220
> > >   ...
> > >   CPU: 0 PID: 110 Comm: modprobe Not tainted 5.11.0+ #1
> > >   Hardware name: Intel Corp. QUARK/GalileoGen2, BIOS 0x01000200 01/01/2014
> > >   EIP: intel_quark_mfd_probe+0x93/0x1c0 [intel_quark_i2c_gpio]
> > > 
> > > This partially reverts the commit c4a164f41554d2899bed94bdcc499263f41787b4.
> > > 
> > > While at it, add a comment to avoid similar changes in the future.
> > > 
> > > Fixes: c4a164f41554 ("mfd: Constify static struct resources")
> > > Cc: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  drivers/mfd/intel_quark_i2c_gpio.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> > > index 55a9e017edee..124c0ee84169 100644
> > > --- a/drivers/mfd/intel_quark_i2c_gpio.c
> > > +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> > > @@ -72,7 +72,8 @@ static const struct dmi_system_id dmi_platform_info[] = {
> > >  	{}
> > >  };
> > >  
> > > -static const struct resource intel_quark_i2c_res[] = {
> > > +/* This is used as a place holder and will be modified at run-time */
> > > +static struct resource intel_quark_i2c_res[] = {
> > >  	[INTEL_QUARK_IORES_MEM] = {
> > >  		.flags = IORESOURCE_MEM,
> > >  	},
> > > @@ -85,7 +86,8 @@ static struct mfd_cell_acpi_match intel_quark_acpi_match_i2c = {
> > >  	.adr = MFD_ACPI_MATCH_I2C,
> > >  };
> > >  
> > > -static const struct resource intel_quark_gpio_res[] = {
> > > +/* This is used as a place holder and will be modified at run-time */
> > > +static struct resource intel_quark_gpio_res[] = {
> > >  	[INTEL_QUARK_IORES_MEM] = {
> > >  		.flags = IORESOURCE_MEM,
> > >  	},
> > 
> > Sorry about that :(
> > 
> > Reviewed-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> 
> Thanks for review!
> 
> Lee, this series has a critical bug fix, should I do something or you is going
> to apply this soon?

It's on my to-review list.

I can prioritise bug fixes though - can it be applied by itself?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources"
  2021-03-19  8:39     ` Lee Jones
@ 2021-03-19 10:01       ` Andy Shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2021-03-19 10:01 UTC (permalink / raw)
  To: Lee Jones; +Cc: Andy Shevchenko, Rikard Falkeborn, Linux Kernel Mailing List

On Fri, Mar 19, 2021 at 10:41 AM Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 18 Mar 2021, Andy Shevchenko wrote:
> > On Tue, Mar 02, 2021 at 09:59:03PM +0100, Rikard Falkeborn wrote:
> > > On Tue, Mar 02, 2021 at 03:56:16PM +0200, Andy Shevchenko wrote:

...

> > > Sorry about that :(
> > >
> > > Reviewed-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> >
> > Thanks for review!
> >
> > Lee, this series has a critical bug fix, should I do something or you is going
> > to apply this soon?
>
> It's on my to-review list.
>
> I can prioritise bug fixes though - can it be applied by itself?

Sire, that's why it's at the beginning of the series.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources"
  2021-03-02 13:56 [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Andy Shevchenko
                   ` (4 preceding siblings ...)
  2021-03-02 20:59 ` [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Rikard Falkeborn
@ 2021-03-20 20:59 ` Tong Zhang
  2021-03-23  9:14 ` Lee Jones
  6 siblings, 0 replies; 17+ messages in thread
From: Tong Zhang @ 2021-03-20 20:59 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: open list, Lee Jones, Rikard Falkeborn

Tested-by: Tong Zhang <ztong0001@gmail.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources"
  2021-03-02 13:56 [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Andy Shevchenko
                   ` (5 preceding siblings ...)
  2021-03-20 20:59 ` Tong Zhang
@ 2021-03-23  9:14 ` Lee Jones
  2021-03-23 10:31   ` Andy Shevchenko
  6 siblings, 1 reply; 17+ messages in thread
From: Lee Jones @ 2021-03-23  9:14 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel, Rikard Falkeborn

On Tue, 02 Mar 2021, Andy Shevchenko wrote:

> The structures are used as place holders, so they are modified at run-time.
> Obviously they may not be constants.
> 
>   BUG: unable to handle page fault for address: d0643220
>   ...
>   CPU: 0 PID: 110 Comm: modprobe Not tainted 5.11.0+ #1
>   Hardware name: Intel Corp. QUARK/GalileoGen2, BIOS 0x01000200 01/01/2014
>   EIP: intel_quark_mfd_probe+0x93/0x1c0 [intel_quark_i2c_gpio]
> 
> This partially reverts the commit c4a164f41554d2899bed94bdcc499263f41787b4.
> 
> While at it, add a comment to avoid similar changes in the future.
> 
> Fixes: c4a164f41554 ("mfd: Constify static struct resources")
> Cc: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_quark_i2c_gpio.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Applied to -fixes for testing, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 5/5] mfd: intel_quark_i2c_gpio: Reuse BAR definitions for MFD cell indexing
  2021-03-02 13:56 ` [PATCH v1 5/5] mfd: intel_quark_i2c_gpio: Reuse BAR definitions for MFD cell indexing Andy Shevchenko
@ 2021-03-23  9:20   ` Lee Jones
  2021-03-23 10:31     ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Lee Jones @ 2021-03-23  9:20 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel

On Tue, 02 Mar 2021, Andy Shevchenko wrote:

> It's convenient and less error prone to use definitions to address
> different cells in an array. For this purpose we may reuse existing
> BAR definitions.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_quark_i2c_gpio.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index 7af22d1399e1..40871ce34e82 100644
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -98,14 +98,6 @@ static struct mfd_cell_acpi_match intel_quark_acpi_match_gpio = {
>  };
>  
>  static struct mfd_cell intel_quark_mfd_cells[] = {
> -	{
> -		.id = MFD_GPIO_BAR,
> -		.name = "gpio-dwapb",
> -		.acpi_match = &intel_quark_acpi_match_gpio,
> -		.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
> -		.resources = intel_quark_gpio_res,
> -		.ignore_resource_conflicts = true,
> -	},
>  	{
>  		.id = MFD_I2C_BAR,
>  		.name = "i2c_designware",
> @@ -114,6 +106,14 @@ static struct mfd_cell intel_quark_mfd_cells[] = {
>  		.resources = intel_quark_i2c_res,
>  		.ignore_resource_conflicts = true,
>  	},
> +	{
> +		.id = MFD_GPIO_BAR,
> +		.name = "gpio-dwapb",
> +		.acpi_match = &intel_quark_acpi_match_gpio,
> +		.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
> +		.resources = intel_quark_gpio_res,
> +		.ignore_resource_conflicts = true,
> +	},
>  };

I would make this more explicit.

[MFD_I2C_BAR] = { }

If someone comes along and re-reorders these, it will break.

>  static const struct pci_device_id intel_quark_mfd_ids[] = {
> @@ -245,11 +245,11 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
>  	if (ret)
>  		return ret;
>  
> -	ret = intel_quark_i2c_setup(pdev, &intel_quark_mfd_cells[1]);
> +	ret = intel_quark_i2c_setup(pdev, &intel_quark_mfd_cells[MFD_I2C_BAR]);
>  	if (ret)
>  		goto err_unregister_i2c_clk;
>  
> -	ret = intel_quark_gpio_setup(pdev, &intel_quark_mfd_cells[0]);
> +	ret = intel_quark_gpio_setup(pdev, &intel_quark_mfd_cells[MFD_GPIO_BAR]);
>  	if (ret)
>  		goto err_unregister_i2c_clk;
>  

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/5] mfd: intel_quark_i2c_gpio: Unregister resources in reversed order
  2021-03-02 13:56 ` [PATCH v1 2/5] mfd: intel_quark_i2c_gpio: Unregister resources in reversed order Andy Shevchenko
@ 2021-03-23  9:21   ` Lee Jones
  0 siblings, 0 replies; 17+ messages in thread
From: Lee Jones @ 2021-03-23  9:21 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel

On Tue, 02 Mar 2021, Andy Shevchenko wrote:

> In ->remove() unregister resources in reversed order, i.e. MFD devices first
> followed by I²C clock.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_quark_i2c_gpio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 3/5] mfd: intel_quark_i2c_gpio: Remove unused struct device member
  2021-03-02 13:56 ` [PATCH v1 3/5] mfd: intel_quark_i2c_gpio: Remove unused struct device member Andy Shevchenko
@ 2021-03-23  9:22   ` Lee Jones
  0 siblings, 0 replies; 17+ messages in thread
From: Lee Jones @ 2021-03-23  9:22 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel

On Tue, 02 Mar 2021, Andy Shevchenko wrote:

> The device pointer in the custom structure is not used anywhere,
> remove it for good.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_quark_i2c_gpio.c | 2 --
>  1 file changed, 2 deletions(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 4/5] mfd: intel_quark_i2c_gpio: Replace I²C speeds with descriptive definitions
  2021-03-02 13:56 ` [PATCH v1 4/5] mfd: intel_quark_i2c_gpio: Replace I²C speeds with descriptive definitions Andy Shevchenko
@ 2021-03-23  9:22   ` Lee Jones
  0 siblings, 0 replies; 17+ messages in thread
From: Lee Jones @ 2021-03-23  9:22 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel

On Tue, 02 Mar 2021, Andy Shevchenko wrote:

> I²C header provides a descriptive definitions for standard bus speeds.
> Use them instead of plain numbers.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_quark_i2c_gpio.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 5/5] mfd: intel_quark_i2c_gpio: Reuse BAR definitions for MFD cell indexing
  2021-03-23  9:20   ` Lee Jones
@ 2021-03-23 10:31     ` Andy Shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2021-03-23 10:31 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-kernel

On Tue, Mar 23, 2021 at 09:20:57AM +0000, Lee Jones wrote:
> On Tue, 02 Mar 2021, Andy Shevchenko wrote:
> 
> > It's convenient and less error prone to use definitions to address
> > different cells in an array. For this purpose we may reuse existing
> > BAR definitions.

...

> > +	{
> > +		.id = MFD_GPIO_BAR,
> > +		.name = "gpio-dwapb",
> > +		.acpi_match = &intel_quark_acpi_match_gpio,
> > +		.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
> > +		.resources = intel_quark_gpio_res,
> > +		.ignore_resource_conflicts = true,
> > +	},
> >  };
> 
> I would make this more explicit.
> 
> [MFD_I2C_BAR] = { }
> 
> If someone comes along and re-reorders these, it will break.

Agree.
Will do for v2.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources"
  2021-03-23  9:14 ` Lee Jones
@ 2021-03-23 10:31   ` Andy Shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2021-03-23 10:31 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-kernel, Rikard Falkeborn

On Tue, Mar 23, 2021 at 09:14:53AM +0000, Lee Jones wrote:
> On Tue, 02 Mar 2021, Andy Shevchenko wrote:
> 
> > The structures are used as place holders, so they are modified at run-time.
> > Obviously they may not be constants.
> > 
> >   BUG: unable to handle page fault for address: d0643220
> >   ...
> >   CPU: 0 PID: 110 Comm: modprobe Not tainted 5.11.0+ #1
> >   Hardware name: Intel Corp. QUARK/GalileoGen2, BIOS 0x01000200 01/01/2014
> >   EIP: intel_quark_mfd_probe+0x93/0x1c0 [intel_quark_i2c_gpio]
> > 
> > This partially reverts the commit c4a164f41554d2899bed94bdcc499263f41787b4.
> > 
> > While at it, add a comment to avoid similar changes in the future.
> > 
> > Fixes: c4a164f41554 ("mfd: Constify static struct resources")
> > Cc: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/mfd/intel_quark_i2c_gpio.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Applied to -fixes for testing, thanks.

Thanks!

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-03-23 10:32 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 13:56 [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Andy Shevchenko
2021-03-02 13:56 ` [PATCH v1 2/5] mfd: intel_quark_i2c_gpio: Unregister resources in reversed order Andy Shevchenko
2021-03-23  9:21   ` Lee Jones
2021-03-02 13:56 ` [PATCH v1 3/5] mfd: intel_quark_i2c_gpio: Remove unused struct device member Andy Shevchenko
2021-03-23  9:22   ` Lee Jones
2021-03-02 13:56 ` [PATCH v1 4/5] mfd: intel_quark_i2c_gpio: Replace I²C speeds with descriptive definitions Andy Shevchenko
2021-03-23  9:22   ` Lee Jones
2021-03-02 13:56 ` [PATCH v1 5/5] mfd: intel_quark_i2c_gpio: Reuse BAR definitions for MFD cell indexing Andy Shevchenko
2021-03-23  9:20   ` Lee Jones
2021-03-23 10:31     ` Andy Shevchenko
2021-03-02 20:59 ` [PATCH v1 1/5] mfd: intel_quark_i2c_gpio: revert "Constify static struct resources" Rikard Falkeborn
2021-03-18 12:53   ` Andy Shevchenko
2021-03-19  8:39     ` Lee Jones
2021-03-19 10:01       ` Andy Shevchenko
2021-03-20 20:59 ` Tong Zhang
2021-03-23  9:14 ` Lee Jones
2021-03-23 10:31   ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).