[PATCH v2 1/1] mfd: intel_quark_i2c_gpio: Don't play dirty trick with const

[PATCH v2 1/1] mfd: intel_quark_i2c_gpio: Don't play dirty trick with const

[Andy Shevchenko]

As Linus rightfully noticed, the driver plays dirty trick with const,
i.e. it assigns a place holder data structure to the const field
in the MFD cell and then drops the const by explicit casting. This is
not how it should be.

Assign local pointers of the cell and resource to the respective
non-const place holders in the intel_quark_i2c_setup() and
intel_quark_gpio_setup().

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: eliminated bar parameter (Lee)
 drivers/mfd/intel_quark_i2c_gpio.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 7f90e6f022ba..9a5f84b93776 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -157,17 +157,16 @@ static void intel_quark_unregister_i2c_clk(struct device *dev)
 	clk_unregister(quark_mfd->i2c_clk);
 }
 
-static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
+static int intel_quark_i2c_setup(struct pci_dev *pdev)
 {
+	struct mfd_cell *cell = &intel_quark_mfd_cells[MFD_I2C_BAR];
+	struct resource *res = intel_quark_i2c_res;
 	const struct dmi_system_id *dmi_id;
 	struct dw_i2c_platform_data *pdata;
-	struct resource *res = (struct resource *)cell->resources;
 	struct device *dev = &pdev->dev;
 
-	res[INTEL_QUARK_IORES_MEM].start =
-		pci_resource_start(pdev, MFD_I2C_BAR);
-	res[INTEL_QUARK_IORES_MEM].end =
-		pci_resource_end(pdev, MFD_I2C_BAR);
+	res[INTEL_QUARK_IORES_MEM].start = pci_resource_start(pdev, MFD_I2C_BAR);
+	res[INTEL_QUARK_IORES_MEM].end = pci_resource_end(pdev, MFD_I2C_BAR);
 
 	res[INTEL_QUARK_IORES_IRQ].start = pci_irq_vector(pdev, 0);
 	res[INTEL_QUARK_IORES_IRQ].end = pci_irq_vector(pdev, 0);
@@ -189,16 +188,15 @@ static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 	return 0;
 }
 
-static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell)
+static int intel_quark_gpio_setup(struct pci_dev *pdev)
 {
+	struct mfd_cell *cell = &intel_quark_mfd_cells[MFD_GPIO_BAR];
+	struct resource *res = intel_quark_gpio_res;
 	struct dwapb_platform_data *pdata;
-	struct resource *res = (struct resource *)cell->resources;
 	struct device *dev = &pdev->dev;
 
-	res[INTEL_QUARK_IORES_MEM].start =
-		pci_resource_start(pdev, MFD_GPIO_BAR);
-	res[INTEL_QUARK_IORES_MEM].end =
-		pci_resource_end(pdev, MFD_GPIO_BAR);
+	res[INTEL_QUARK_IORES_MEM].start = pci_resource_start(pdev, MFD_GPIO_BAR);
+	res[INTEL_QUARK_IORES_MEM].end = pci_resource_end(pdev, MFD_GPIO_BAR);
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
@@ -252,11 +250,11 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
 	if (ret < 0)
 		goto err_unregister_i2c_clk;
 
-	ret = intel_quark_i2c_setup(pdev, &intel_quark_mfd_cells[MFD_I2C_BAR]);
+	ret = intel_quark_i2c_setup(pdev);
 	if (ret)
 		goto err_free_irq_vectors;
 
-	ret = intel_quark_gpio_setup(pdev, &intel_quark_mfd_cells[MFD_GPIO_BAR]);
+	ret = intel_quark_gpio_setup(pdev);
 	if (ret)
 		goto err_free_irq_vectors;
 
-- 
2.30.2

Re: [PATCH v2 1/1] mfd: intel_quark_i2c_gpio: Don't play dirty trick with const

[Lee Jones]

On Fri, 26 Mar 2021, Andy Shevchenko wrote:

> As Linus rightfully noticed, the driver plays dirty trick with const,
> i.e. it assigns a place holder data structure to the const field
> in the MFD cell and then drops the const by explicit casting. This is
> not how it should be.
> 
> Assign local pointers of the cell and resource to the respective
> non-const place holders in the intel_quark_i2c_setup() and
> intel_quark_gpio_setup().
> 
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: eliminated bar parameter (Lee)
>  drivers/mfd/intel_quark_i2c_gpio.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)

Neat.

Applied, thanks.

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

Re: [PATCH v2 1/1] mfd: intel_quark_i2c_gpio: Don't play dirty trick with const

[Lee Jones]

On Fri, 26 Mar 2021, Lee Jones wrote:

> On Fri, 26 Mar 2021, Andy Shevchenko wrote:
> 
> > As Linus rightfully noticed, the driver plays dirty trick with const,
> > i.e. it assigns a place holder data structure to the const field
> > in the MFD cell and then drops the const by explicit casting. This is
> > not how it should be.
> > 
> > Assign local pointers of the cell and resource to the respective
> > non-const place holders in the intel_quark_i2c_setup() and
> > intel_quark_gpio_setup().
> > 
> > Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > v2: eliminated bar parameter (Lee)
> >  drivers/mfd/intel_quark_i2c_gpio.c | 26 ++++++++++++--------------
> >  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> Neat.
> 
> Applied, thanks.

Am I still missing patches from you Andy?

I get:

 make --silent --keep-going --jobs=8 O=/home/tuxbuild/.cache/tuxmake/builds/1/tmp ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu- 'CC=sccache x86_64-linux-gnu-gcc' 'HOSTCC=sccache gcc' allmodconfig
 make --silent --keep-going --jobs=8 O=/home/tuxbuild/.cache/tuxmake/builds/1/tmp ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu- 'CC=sccache x86_64-linux-gnu-gcc' 'HOSTCC=sccache gcc'
 /builds/linux/drivers/mfd/intel_quark_i2c_gpio.c: In function 'intel_quark_i2c_setup':
 /builds/linux/drivers/mfd/intel_quark_i2c_gpio.c:181:25: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
   struct resource *res = intel_quark_i2c_res;
                          ^~~~~~~~~~~~~~~~~~~
 /builds/linux/drivers/mfd/intel_quark_i2c_gpio.c: In function 'intel_quark_gpio_setup':
 /builds/linux/drivers/mfd/intel_quark_i2c_gpio.c:203:25: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
   struct resource *res = intel_quark_gpio_res;
                          ^~~~~~~~~~~~~~~~~~~~

Link to the build (see: build.log):

  https://builds.tuxbuild.com/1rAMovpd041jvsjfQ538kW3nvYK/

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

Re: [PATCH v2 1/1] mfd: intel_quark_i2c_gpio: Don't play dirty trick with const

[Andy Shevchenko]

On Wed, Apr 14, 2021 at 06:13:19PM +0100, Lee Jones wrote:
> On Fri, 26 Mar 2021, Lee Jones wrote:
> 
> > On Fri, 26 Mar 2021, Andy Shevchenko wrote:
> > 
> > > As Linus rightfully noticed, the driver plays dirty trick with const,
> > > i.e. it assigns a place holder data structure to the const field
> > > in the MFD cell and then drops the const by explicit casting. This is
> > > not how it should be.
> > > 
> > > Assign local pointers of the cell and resource to the respective
> > > non-const place holders in the intel_quark_i2c_setup() and
> > > intel_quark_gpio_setup().
> > > 
> > > Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > > v2: eliminated bar parameter (Lee)
> > >  drivers/mfd/intel_quark_i2c_gpio.c | 26 ++++++++++++--------------
> > >  1 file changed, 12 insertions(+), 14 deletions(-)
> > 
> > Neat.
> > 
> > Applied, thanks.
> 
> Am I still missing patches from you Andy?

Patches for fixes should be applied to for-next as well. I don't know why with
my patches it diverged.

I have already commented on this when kbuild bot complained.

So,

	git checkout for-mfd-next
	git merge for-mfd-fixes

or equivalent (cherry-pick) will fix that.

> I get:
> 
>  make --silent --keep-going --jobs=8 O=/home/tuxbuild/.cache/tuxmake/builds/1/tmp ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu- 'CC=sccache x86_64-linux-gnu-gcc' 'HOSTCC=sccache gcc' allmodconfig
>  make --silent --keep-going --jobs=8 O=/home/tuxbuild/.cache/tuxmake/builds/1/tmp ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu- 'CC=sccache x86_64-linux-gnu-gcc' 'HOSTCC=sccache gcc'
>  /builds/linux/drivers/mfd/intel_quark_i2c_gpio.c: In function 'intel_quark_i2c_setup':
>  /builds/linux/drivers/mfd/intel_quark_i2c_gpio.c:181:25: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
>    struct resource *res = intel_quark_i2c_res;
>                           ^~~~~~~~~~~~~~~~~~~
>  /builds/linux/drivers/mfd/intel_quark_i2c_gpio.c: In function 'intel_quark_gpio_setup':
>  /builds/linux/drivers/mfd/intel_quark_i2c_gpio.c:203:25: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
>    struct resource *res = intel_quark_gpio_res;
>                           ^~~~~~~~~~~~~~~~~~~~
> 
> Link to the build (see: build.log):
> 
>   https://builds.tuxbuild.com/1rAMovpd041jvsjfQ538kW3nvYK/
> 
> -- 
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/1] mfd: intel_quark_i2c_gpio: Don't play dirty trick with const

[Lee Jones]

On Wed, 14 Apr 2021, Andy Shevchenko wrote:

> On Wed, Apr 14, 2021 at 06:13:19PM +0100, Lee Jones wrote:
> > On Fri, 26 Mar 2021, Lee Jones wrote:
> > 
> > > On Fri, 26 Mar 2021, Andy Shevchenko wrote:
> > > 
> > > > As Linus rightfully noticed, the driver plays dirty trick with const,
> > > > i.e. it assigns a place holder data structure to the const field
> > > > in the MFD cell and then drops the const by explicit casting. This is
> > > > not how it should be.
> > > > 
> > > > Assign local pointers of the cell and resource to the respective
> > > > non-const place holders in the intel_quark_i2c_setup() and
> > > > intel_quark_gpio_setup().
> > > > 
> > > > Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > ---
> > > > v2: eliminated bar parameter (Lee)
> > > >  drivers/mfd/intel_quark_i2c_gpio.c | 26 ++++++++++++--------------
> > > >  1 file changed, 12 insertions(+), 14 deletions(-)
> > > 
> > > Neat.
> > > 
> > > Applied, thanks.
> > 
> > Am I still missing patches from you Andy?
> 
> Patches for fixes should be applied to for-next as well. I don't know why with
> my patches it diverged.
> 
> I have already commented on this when kbuild bot complained.
> 
> So,
> 
> 	git checkout for-mfd-next
> 	git merge for-mfd-fixes
> 
> or equivalent (cherry-pick) will fix that.

Ah, it's in -rc5.  Very good.  Thanks for the explanation.

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

Re: [PATCH v2 1/1] mfd: intel_quark_i2c_gpio: Don't play dirty trick with const

[Andy Shevchenko]

On Wed, Apr 14, 2021 at 10:19:16PM +0100, Lee Jones wrote:
> On Wed, 14 Apr 2021, Andy Shevchenko wrote:
> > On Wed, Apr 14, 2021 at 06:13:19PM +0100, Lee Jones wrote:
> > > On Fri, 26 Mar 2021, Lee Jones wrote:

...

> > > Am I still missing patches from you Andy?
> > 
> > Patches for fixes should be applied to for-next as well. I don't know why with
> > my patches it diverged.
> > 
> > I have already commented on this when kbuild bot complained.
> > 
> > So,
> > 
> > 	git checkout for-mfd-next
> > 	git merge for-mfd-fixes
> > 
> > or equivalent (cherry-pick) will fix that.
> 
> Ah, it's in -rc5.  Very good.  Thanks for the explanation.

You are welcome!

-- 
With Best Regards,
Andy Shevchenko