RE: [PATCH v6 03/13] pinctrl: Add sysfs support

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

Hi Greg,

> Subject: Re: [PATCH v6 03/13] pinctrl: Add sysfs support
> 
> On Tue, Mar 07, 2023 at 02:43:54PM +0000, Biju Das wrote:
> > Hi Linus Walleij,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH v6 03/13] pinctrl: Add sysfs support
> > >
> > > On Mon, Mar 6, 2023 at 10:00 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > >
> > > > Add a simple sysfs interface to the generic pinctrl framework for
> > > > configuring pins for output disable operation.
> > > >
> > > > /sys/class/pinctrl/
> > > >   `-- output-disable/
> > > >       |-- configure    (w/o) ask the kernel to configure a pin group
> > > >                              for output disable operation.
> > > >
> > > >   echo "<group-name function-name 0 1>" > configure
> > > >
> > > > The existing "pinmux-functions" debugfs file lists the pin
> > > > functions registered for the pin controller. For example:
> > > >
> > > >   function 0: usb0, groups = [ usb0 ]
> > > >   function 1: usb1, groups = [ usb1 ]
> > > >   function 2: gpt4-pins, groups = [ gpt4-pins ]
> > > >   function 3: scif0, groups = [ scif0 ]
> > > >   function 4: scif2, groups = [ scif2 ]
> > > >   function 5: spi1, groups = [ spi1 ]
> > > >
> > > > To configure gpt4-pins for output disable activation by user:
> > > >
> > > >   echo "gpt4-pins gpt4-pins 0 1" > configure
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > >
> > >
> > > You have to run things like this by Greg K-H (adde on To)
> > >
> > > > +static struct class pinctrl_class = {
> > > > +       .name = "pinctrl",
> > > > +       .owner = THIS_MODULE,
> > > > +       .dev_groups = pinctrl_groups, };
> > >
> > > Why are you adding a new device class?
> > >
> > > IIRC Greg don't like them, we should probably just deal with the
> > > devices directly on the bus where they are, which also implies some
> > > topology search etc which is a feature.
> >
> > I am ok for both,
> >
> > I thought adding new device class will be more generic and people can
> > use simple sysfs[1] like pwm for output disable operation rather than
> > the SoC specific operation[2].
> >
> > [1]
> > /sys/class/pinctrl/output-disable/configure
> 
> That's fine, but you don't need a class for it, use configfs for configuring
> things like this, that is what it is there for.

As per discussion with Linus W and Geert, it is agreed to use configure this
settings in device tree as DT is the best place for system config like these use
cases.

Cheers,
Biju