[PATCH v2 1/2] pinctrl: mcp23s08: Use full chunk of memory for regmap configuration

[PATCH v2 1/2] pinctrl: mcp23s08: Use full chunk of memory for regmap configuration

[Andy Shevchenko]

It appears that simplification of mcp23s08_spi_regmap_init() made
a regression due to wrong size calculation for dev_kmemdup() call.
It mises the fact that config variable is already a pointer, thus
the sizeof() calculation is wrong and only 4 or 8 bytes were copied.

Fix the parameters to devm_kmemdup() to copy full chunk of memory.

Fixes: 0874758ecb2b ("pinctrl: mcp23s08: Refactor mcp23s08_spi_regmap_init()")
Reported-by: Martin Hundebøll <martin@geanix.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: fixed few typos in the commit message
 drivers/pinctrl/pinctrl-mcp23s08_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08_spi.c b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
index 1f47a661b0a7..7c72cffe1412 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08_spi.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
@@ -119,7 +119,7 @@ static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
 		return -EINVAL;
 	}
 
-	copy = devm_kmemdup(dev, &config, sizeof(config), GFP_KERNEL);
+	copy = devm_kmemdup(dev, config, sizeof(*config), GFP_KERNEL);
 	if (!copy)
 		return -ENOMEM;
 
-- 
2.28.0

[PATCH v2 2/2] pinctrl: mcp23s08: Print error message when regmap init fails

[Andy Shevchenko]

It is useful for debugging to have the error message printed
when regmap initialisation fails. Add it to the driver.

Cc: Martin Hundebøll <martin@geanix.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: no changes
 drivers/pinctrl/pinctrl-mcp23s08_spi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08_spi.c b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
index 7c72cffe1412..9ae10318f6f3 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08_spi.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
@@ -126,6 +126,8 @@ static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
 	copy->name = name;
 
 	mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp, copy);
+	if (IS_ERR(mcp->regmap))
+		dev_err(dev, "regmap init failed for %s\n", mcp->chip.label);
 	return PTR_ERR_OR_ZERO(mcp->regmap);
 }
 
-- 
2.28.0

Re: [PATCH v2 1/2] pinctrl: mcp23s08: Use full chunk of memory for regmap configuration

[Martin Hundebøll]

Hi Linus,

You can add my
Tested-by: Martin Hundebøll <martin@geanix.com>

Thanks,
Martin

On 09/10/2020 20.08, Andy Shevchenko wrote:
> It appears that simplification of mcp23s08_spi_regmap_init() made
> a regression due to wrong size calculation for dev_kmemdup() call.
> It mises the fact that config variable is already a pointer, thus
> the sizeof() calculation is wrong and only 4 or 8 bytes were copied.
> 
> Fix the parameters to devm_kmemdup() to copy full chunk of memory.
> 
> Fixes: 0874758ecb2b ("pinctrl: mcp23s08: Refactor mcp23s08_spi_regmap_init()")
> Reported-by: Martin Hundebøll <martin@geanix.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: fixed few typos in the commit message
>   drivers/pinctrl/pinctrl-mcp23s08_spi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08_spi.c b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
> index 1f47a661b0a7..7c72cffe1412 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08_spi.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
> @@ -119,7 +119,7 @@ static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
>   		return -EINVAL;
>   	}
>   
> -	copy = devm_kmemdup(dev, &config, sizeof(config), GFP_KERNEL);
> +	copy = devm_kmemdup(dev, config, sizeof(*config), GFP_KERNEL);
>   	if (!copy)
>   		return -ENOMEM;
>   
> 

Re: [PATCH v2 1/2] pinctrl: mcp23s08: Use full chunk of memory for regmap configuration

[Andy Shevchenko]

On Mon, Oct 12, 2020 at 2:56 PM Martin Hundebøll <martin@geanix.com> wrote:
>
> Hi Linus,
>
> You can add my
> Tested-by: Martin Hundebøll <martin@geanix.com>

Thank you!
Linus, below more typo fixes, but I'm not going to send a new version
(until you explicitly require), so please take them into consideration
when applying.

> Thanks,
> Martin
>
> On 09/10/2020 20.08, Andy Shevchenko wrote:
> > It appears that simplification of mcp23s08_spi_regmap_init() made
> > a regression due to wrong size calculation for dev_kmemdup() call.

> > It mises the fact that config variable is already a pointer, thus

mises -> misses

> > the sizeof() calculation is wrong and only 4 or 8 bytes were copied.
> >
> > Fix the parameters to devm_kmemdup() to copy full chunk of memory.

copy full -> copy a full

> >
> > Fixes: 0874758ecb2b ("pinctrl: mcp23s08: Refactor mcp23s08_spi_regmap_init()")
> > Reported-by: Martin Hundebøll <martin@geanix.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > v2: fixed few typos in the commit message
> >   drivers/pinctrl/pinctrl-mcp23s08_spi.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-mcp23s08_spi.c b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
> > index 1f47a661b0a7..7c72cffe1412 100644
> > --- a/drivers/pinctrl/pinctrl-mcp23s08_spi.c
> > +++ b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
> > @@ -119,7 +119,7 @@ static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
> >               return -EINVAL;
> >       }
> >
> > -     copy = devm_kmemdup(dev, &config, sizeof(config), GFP_KERNEL);
> > +     copy = devm_kmemdup(dev, config, sizeof(*config), GFP_KERNEL);
> >       if (!copy)
> >               return -ENOMEM;
> >
> >



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] pinctrl: mcp23s08: Use full chunk of memory for regmap configuration

[Linus Walleij]

On Fri, Oct 9, 2020 at 8:09 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> It appears that simplification of mcp23s08_spi_regmap_init() made
> a regression due to wrong size calculation for dev_kmemdup() call.
> It mises the fact that config variable is already a pointer, thus
> the sizeof() calculation is wrong and only 4 or 8 bytes were copied.
>
> Fix the parameters to devm_kmemdup() to copy full chunk of memory.
>
> Fixes: 0874758ecb2b ("pinctrl: mcp23s08: Refactor mcp23s08_spi_regmap_init()")
> Reported-by: Martin Hundebøll <martin@geanix.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: fixed few typos in the commit message

Patches applied, fixed the extra typos in the process,
thanks!

Linus Walleij

Re: [PATCH v2 1/2] pinctrl: mcp23s08: Use full chunk of memory for regmap configuration

[Jan Kundrát]

> Patches applied, fixed the extra typos in the process,
> thanks!

Hi Linus,
are these patches already pushed? I've checked the gpio and pinctrl trees, 
and I don't think you've pushed them.

Anyway, they fix a boot-time oops with 5.9.0 for me, so:

Tested-by: Jan Kundrát <jan.kundrat@cesnet.cz>

With kind regards,
Jan

Re: [PATCH v2 1/2] pinctrl: mcp23s08: Use full chunk of memory for regmap configuration

[Linus Walleij]

On Mon, Oct 26, 2020 at 5:40 PM Jan Kundrát <jan.kundrat@cesnet.cz> wrote:

> > Patches applied, fixed the extra typos in the process,
> > thanks!
>
> Hi Linus,
> are these patches already pushed? I've checked the gpio and pinctrl trees,
> and I don't think you've pushed them.
>
> Anyway, they fix a boot-time oops with 5.9.0 for me, so:
>
> Tested-by: Jan Kundrát <jan.kundrat@cesnet.cz>

I applied them as non-critical fixes but now I will move them
to fixes, add your tested-by and merge ASAP.

Yours,
Linus Walleij