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 <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
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
The device pointer in the custom structure is not used anywhere,
remove it for good.
Signed-off-by: Andy Shevchenko <[email protected]>
---
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
I²C header provides a descriptive definitions for standard bus speeds.
Use them instead of plain numbers.
Signed-off-by: Andy Shevchenko <[email protected]>
---
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
In ->remove() unregister resources in reversed order, i.e. MFD devices first
followed by I²C clock.
Signed-off-by: Andy Shevchenko <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
> > Signed-off-by: Andy Shevchenko <[email protected]>
> > ---
> > 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 <[email protected]>
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
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 <[email protected]>
> > > Signed-off-by: Andy Shevchenko <[email protected]>
> > > ---
> > > 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 <[email protected]>
>
> 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
On Fri, Mar 19, 2021 at 10:41 AM Lee Jones <[email protected]> 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 <[email protected]>
> >
> > 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
Tested-by: Tong Zhang <[email protected]>
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 <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> > Signed-off-by: Andy Shevchenko <[email protected]>
> > ---
> > 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