We don't provide defaults for the regmap, we shouldn't provide
the number of them either.

Remove incorrectly set fields in regmap configuration.

Fixes: 8670de9fae49 ("pinctrl: cy8c95x0: Use regmap ranges")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-cy8c95x0.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index a94fade0ebc2..e98eba17cd13 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -464,7 +464,6 @@ static const struct regmap_config cy8c9520_i2c_regmap = {
 	.ranges	= NULL,			/* Updated at runtime */
 	.num_ranges = 1,
 	.max_register = 0,		/* Updated at runtime */
-	.num_reg_defaults_raw = 0,	/* Updated at runtime */
 	.use_single_read = true,	/* Workaround for regcache bug */
 #if IS_ENABLED(CONFIG_DEBUG_PINCTRL)
 	.disable_locking = false,
@@ -1475,7 +1474,6 @@ static int cy8c95x0_probe(struct i2c_client *client)
 	memcpy(&regmap_conf, &cy8c9520_i2c_regmap, sizeof(regmap_conf));
 	regmap_conf.ranges = &regmap_range_conf;
 	regmap_conf.max_register = regmap_range_conf.range_max;
-	regmap_conf.num_reg_defaults_raw = regmap_range_conf.range_max;
 
 	chip->regmap = devm_regmap_init_i2c(client, &regmap_conf);
 	if (IS_ERR(chip->regmap))
-- 
2.43.0.rc1.1336.g36b5255a03ac

Hi Andy,
On Fri, Jan 17, 2025 at 3:23 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> We don't provide defaults for the regmap, we shouldn't provide
> the number of them either.
The intention is to read back the defaults from HW to initialize the
regmap cache.
The defaults are applied at each POR from the device's internal EEPROM.
See regcache_hw_init() for details.
>
> Remove incorrectly set fields in regmap configuration.
>
> Fixes: 8670de9fae49 ("pinctrl: cy8c95x0: Use regmap ranges")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/pinctrl-cy8c95x0.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
> index a94fade0ebc2..e98eba17cd13 100644
> --- a/drivers/pinctrl/pinctrl-cy8c95x0.c
> +++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
> @@ -464,7 +464,6 @@ static const struct regmap_config cy8c9520_i2c_regmap = {
>         .ranges = NULL,                 /* Updated at runtime */
>         .num_ranges = 1,
>         .max_register = 0,              /* Updated at runtime */
> -       .num_reg_defaults_raw = 0,      /* Updated at runtime */
>         .use_single_read = true,        /* Workaround for regcache bug */
>  #if IS_ENABLED(CONFIG_DEBUG_PINCTRL)
>         .disable_locking = false,
> @@ -1475,7 +1474,6 @@ static int cy8c95x0_probe(struct i2c_client *client)
>         memcpy(&regmap_conf, &cy8c9520_i2c_regmap, sizeof(regmap_conf));
>         regmap_conf.ranges = &regmap_range_conf;
>         regmap_conf.max_register = regmap_range_conf.range_max;
> -       regmap_conf.num_reg_defaults_raw = regmap_range_conf.range_max;
>
>         chip->regmap = devm_regmap_init_i2c(client, &regmap_conf);
>         if (IS_ERR(chip->regmap))
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>

On Fri, Jan 17, 2025 at 04:01:43PM +0100, Patrick Rudolph wrote:
> Hi Andy,
> On Fri, Jan 17, 2025 at 3:23 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > We don't provide defaults for the regmap, we shouldn't provide
> > the number of them either.
> The intention is to read back the defaults from HW to initialize the
> regmap cache.
> The defaults are applied at each POR from the device's internal EEPROM.
> See regcache_hw_init() for details.

Yes, I was looking a lot and that code and it doesn't work as intended.
I can reproduce the issue, but it's real issue and I don't think we need
to read back anything from the HW forcibly. It will be done naturally, no?

-- 
With Best Regards,
Andy Shevchenko

On Fri, Jan 17, 2025 at 05:13:36PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 17, 2025 at 04:01:43PM +0100, Patrick Rudolph wrote:
> > Hi Andy,
> > On Fri, Jan 17, 2025 at 3:23 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > We don't provide defaults for the regmap, we shouldn't provide
> > > the number of them either.
> > The intention is to read back the defaults from HW to initialize the
> > regmap cache.
> > The defaults are applied at each POR from the device's internal EEPROM.
> > See regcache_hw_init() for details.
> 
> Yes, I was looking a lot and that code and it doesn't work as intended.
> I can reproduce the issue, but it's real issue and I don't think we need
> to read back anything from the HW forcibly. It will be done naturally, no?

I think I now remember, it has a NULL pointer dereference.

-- 
With Best Regards,
Andy Shevchenko

On Fri, Jan 17, 2025 at 05:15:06PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 17, 2025 at 05:13:36PM +0200, Andy Shevchenko wrote:
> > On Fri, Jan 17, 2025 at 04:01:43PM +0100, Patrick Rudolph wrote:
> > > On Fri, Jan 17, 2025 at 3:23 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > We don't provide defaults for the regmap, we shouldn't provide
> > > > the number of them either.
> > > The intention is to read back the defaults from HW to initialize the
> > > regmap cache.
> > > The defaults are applied at each POR from the device's internal EEPROM.
> > > See regcache_hw_init() for details.
> > 
> > Yes, I was looking a lot and that code and it doesn't work as intended.
> > I can reproduce the issue, but it's real issue and I don't think we need
> > to read back anything from the HW forcibly. It will be done naturally, no?
> 
> I think I now remember, it has a NULL pointer dereference.

FWIW, okay, it was induced by a fix in regmap core, so this NULL pointer
dereference is for real, but it's a regression in the regmap core.

For the reference read this thread:
20250116124303.3941583-1-andriy.shevchenko@linux.intel.com.

-- 
With Best Regards,
Andy Shevchenko

On Fri, Jan 17, 2025 at 05:15:06PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 17, 2025 at 05:13:36PM +0200, Andy Shevchenko wrote:
> > On Fri, Jan 17, 2025 at 04:01:43PM +0100, Patrick Rudolph wrote:
> > > Hi Andy,
> > > On Fri, Jan 17, 2025 at 3:23 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > We don't provide defaults for the regmap, we shouldn't provide
> > > > the number of them either.
> > > The intention is to read back the defaults from HW to initialize the
> > > regmap cache.
> > > The defaults are applied at each POR from the device's internal EEPROM.
> > > See regcache_hw_init() for details.
> > 
> > Yes, I was looking a lot and that code and it doesn't work as intended.
> > I can reproduce the issue, but it's real issue and I don't think we need
> > to read back anything from the HW forcibly. It will be done naturally, no?
> 
> I think I now remember, it has a NULL pointer dereference.

Hmm... I have just tested and no issues found so far. I'm now puzzled what was
that when I tested this the first time a few months ago...

Okay, let's say I will drop this patch in v2. Can you look at the rest of
the series?

Thanks for review!

-- 
With Best Regards,
Andy Shevchenko