Re: [PATCH v2 3/4] pinctrl: renesas: Initial R8A779G0 (V4H) PFC support

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Morimoto-san,

On Mon, Jun 13, 2022 at 1:51 AM Kuninori Morimoto
<kuninori.morimoto.gx@xxxxxxxxxxx> wrote:
> > > From: LUU HOAI <hoai.luu.ub@xxxxxxxxxxx>
> > >
> > > This patch adds initial pinctrl support for the R8A779G0 (V4H) SoC,
> > > including bias, drive strength and voltage control.
> > >
> > > [Morimoto merged Kihara-san's MODSEL8 fixup patch,
> > > and cleanuped white space, care reserved bit on each configs,
> > > fixup setting miss]
> > > Signed-off-by: LUU HOAI <hoai.luu.ub@xxxxxxxxxxx>
> > > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@xxxxxxxxxxx>
> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> >
> > Thanks for your patch!
> >
> > Below is a first set of comments.
> > You can expect a second set of comments after the weekend.
>
> May I suggest ?
>
> v1 patch was almost as-is of working implementation,
> I did was just cleanup.
> v2 patch was tidyup implementation which found via runtime.
> So far, I think we can say "v2 patch is cleanup version of working implementation".
>
> But, v3 needs too many tidyups, and it is related to very deep parts.
> I'm very afraid it breaks working implementation for some reasons.
> In such case, it is very difficult to find and/or fix the issue
> because PFC file is too big and too complicated.
>
> I'm happy to fix all of you pointed, but I want to make it as additional patch.
> But what do you think ?

For missing features (e.g. missing alternative functions, missing
support for using I2C-capable pins as GPIOs, ...) I agree: these can
be added incrementally.

For descriptions that are wrong, I disagree: please try to fix them
in the patch that introduces them, if possible.
This is especially true for descriptions that become user-visible:
e.g. set names (no suffix vs. "_A" vs. "_B" suffixes) become DT ABI
once their pin groups have been defined.

Unfortunately some of the (lack of) set names need confirmation or
clarification from the hardware documentation team.

BTW, do you know which devices and pin groups have been tested on
actual hardware?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds