Received-SPF: pass (google.com: domain of linux-kernel+bounces-185271-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240521152602.1097764-1-patrick.rudolph@9elements.com> <20240521152602.1097764-2-patrick.rudolph@9elements.com>
In-Reply-To: <20240521152602.1097764-2-patrick.rudolph@9elements.com>
From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Tue, 21 May 2024 20:34:18 +0300
Message-ID: <CAHp75VcYZCr=N_3YNe4kO1Vm95mJcxV4dKkBEz-NqXzumzXa1w@mail.gmail.com>
Subject: Re: [PATCH 2/3] pinctrl: cy8c95x0: Use regmap ranges
To: Patrick Rudolph <patrick.rudolph@9elements.com>
Cc: Linus Walleij <linus.walleij@linaro.org>, naresh.solanki@9elements.com, 
	broonie@kernel.org, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, May 21, 2024 at 6:26=E2=80=AFPM 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[] =3D {
> +       {
> +               .range_min =3D CY8C95X0_VIRTUAL,
> +               .range_max =3D 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 =3D CY8C95X0_PORTSEL,
> +               .selector_mask =3D 0x07,
> +               .selector_shift =3D 0x0,
> +               .window_start =3D CY8C95X0_INTMASK,
> +               .window_len =3D 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_ran=
ge_conf));

       memcpy(&regmap_range_conf, cy8c95x0_ranges, sizeof(regmap_range_conf=
));

..

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.

--=20
With Best Regards,
Andy Shevchenko