Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>]

Hi Linus,

On Tue, Dec 8, 2020 at 12:00 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> Hi Serigio,
>
> I dug around some to try to understand the patch I think I get
> it now :)
>
> Squash this with the third patch so it becomes a
> "move" of this file, preserving history. With that:
> Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>

Ok, will squash those two if you prefer that way with your 'Acked-by'.

>
> I have ideas, but it is better to move the driver out
> of staging and improve it in pinctrl.
>
> Since there might be many sub-SoCs for this pin
> controller, what about creating
> drivers/pinctrl/ralink/* for this at the same time?

Ok, I will put this inside a ralink subdirectory in pinctrl.

>
> On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
> <sergio.paracuellos@xxxxxxxxx> wrote:
> >
> > These Socs have 1-3 banks of 8-32 gpios. Rather then setting the muxing of each
> > pin individually, these socs have mux groups that when set will effect 1-N pins.
> > Pin groups have a 2, 4 or 8 different muxes.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> (...)
> > +#include <asm/mach-ralink/ralink_regs.h>
> > +#include <asm/mach-ralink/pinmux.h>
> > +#include <asm/mach-ralink/mt7620.h>
>
> I think in the next step we should move the contents of
> rt2880_pinmux_data into this driver, then we can drop these
> mach-headers and show the way for the rest of the ralink
> chips to push their data down into this driver (or subdrivers)
> and depopulate mach-ralink a bit.

Agree. Doing that no arch dependencies are included and we can cleanly
enable the driver also for COMPILE_TEST without adding special flags
in pinctrl Makefile.

>
> > +       p->groups = rt2880_pinmux_data;
>
> So this is where the driver actually gets a pointer to all
> groups and functions, and these groups and functions all
> come from arch/mips/ralink/mt7621.c right?

Yes, all of that is defined there.

>
> I think after this first step we should move mt7621.c
> to pinctrl and become a subdriver for this pin controller
> and then we can hopefully move the rest as well once
> you set the pattern for how we do this.

I see. Thanks for advices.

>
> Yours,
> Linus Walleij

Best regards,
    Sergio Paracuellos