Received-SPF: pass (google.com: domain of linux-kernel+bounces-206707-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Fri, 7 Jun 2024 23:29:10 +0300
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Patrick Rudolph <patrick.rudolph@9elements.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	naresh.solanki@9elements.com, broonie@kernel.org,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] pinctrl: cy8c95x0: Use regmap ranges
Message-ID: <ZmNtllmSoOGiikel@surfacebook.localdomain>
References: <20240521152602.1097764-1-patrick.rudolph@9elements.com>
 <20240521152602.1097764-2-patrick.rudolph@9elements.com>
 <CAHp75VcYZCr=N_3YNe4kO1Vm95mJcxV4dKkBEz-NqXzumzXa1w@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAHp75VcYZCr=N_3YNe4kO1Vm95mJcxV4dKkBEz-NqXzumzXa1w@mail.gmail.com>

Tue, May 21, 2024 at 08:34:18PM +0300, Andy Shevchenko kirjoitti:
> On Tue, May 21, 2024 at 6:26 PM Patrick Rudolph
> <patrick.rudolph@9elements.com> wrote:
> >
> > Instead of implementing a custom register paging mechanism in
> > the driver use the existing regmap ranges feature.
> 
> driver, use

...

> > +static const struct regmap_range_cfg cy8c95x0_ranges[] = {
> > +       {
> > +               .range_min = CY8C95X0_VIRTUAL,
> > +               .range_max = 0,         /* Updated at runtime */
> 
> This and similar comments are misleading since the data is defined as
> const. Updated --> Filled or alike here and elsewhere.
> 
> > +               .selector_reg = CY8C95X0_PORTSEL,
> > +               .selector_mask = 0x07,
> > +               .selector_shift = 0x0,
> > +               .window_start = CY8C95X0_INTMASK,
> > +               .window_len = MUXED_STRIDE,
> > +       }
> >  };

...

> > +                       regcache_cache_only(chip->regmap, true);
> > +                       regmap_update_bits(chip->regmap, off, mask & val, 0);
> > +                       regcache_cache_only(chip->regmap, false);
> 
> A side note: It's strange to see mask & val, 0 in the parameters of
> regmap calls. Perhaps refactor this (in a separate patch) to use
> standard approach?

...

> > +       memcpy(&regmap_range_conf, &cy8c95x0_ranges[0], sizeof(regmap_range_conf));
> 
>        memcpy(&regmap_range_conf, cy8c95x0_ranges, sizeof(regmap_range_conf));

I hope the all above can be tweaked by Linus when applying.

...

> Note, if you are not using --histogram diff algo, please start using
> it from the next version of this series.
> 
> P.S. I will test the next version on Intel Galileo Gen1 as currently I
> have some issues with the HW I need to fix first.

Unfortunately I wasn't able to ressurect the HW and now I'm going to be off for
a while. Let's go with this and if any problem appears, I probably can try to
fix myself.

Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko