* [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).