Re: [PATCH 1/2] pinctrl: core: configure pinmux from pins debug file

[Drew Fustini <drew@xxxxxxxxxxxxxxx>]

On Tue, May 18, 2021 at 2:38 AM Dario Binacchi <dariobin@xxxxxxxxx> wrote:
>
> Hi,
>
> > Il 18/05/2021 00:57 Drew Fustini <drew@xxxxxxxxxxxxxxx> ha scritto:
> >
> >
> > On Mon, May 17, 2021 at 11:02:00PM +0300, Andy Shevchenko wrote:
> > > On Sun, May 16, 2021 at 7:43 PM Dario Binacchi <dariobin@xxxxxxxxx> wrote:
> > > >
> > > > The MPUs of some architectures (e.g AM335x) must be in privileged
> > > > operating mode to write on the pinmux
> > >
> > > pinmux is not pin configuration. You need to rethink the approach.
> > >
> > > > registers. In such cases, where
> > > > writes will not work from user space, now it can be done from the pins
> > > > debug file if the platform driver exports the pin_dbg_set() helper among
> > > > the registered operations.
> > >
> > > Drew, is it similar to what you are trying to achieve?
> >
> > Yes, I would say this similar to what I was trying to accomplish: being
> > able to change contents of conf_<module>_<pin> register [table 9-60]
> > from userspace.
> >
> > However, I was specifically looking to change bits 2:0 which is mux
> > mode. My motivation was to allow BeagleBone users to easily switch
> > between pin functions on the expansion headers during runtime to make
> > rapid prototyping with a breadboard easier (such as changing header pin
> > from GPIO to SPI mode). Most of the header pins have 7 different modes.
> >
> > Ultimately, the solution I settled on with feedback from this list was
> > to create pinmux-select debugfs file that can activate desired fucntion:
> > 6199f6becc86 ("pinctrl: pinmux: Add pinmux-select debugfs file")
> >
> > Bits 6:3 are related to what this subsystem would refer to as pin conf
> > such as slew, input enable and bias. Thus it might make sense to expose
> > something like a select-pinconf file to activate pin conf settings from
> > userspace. This would require using 'pinconf-single' compatible.
> >
> > I fixed pinctrl-single bug regarding pinconf last year so it should be
> > possible to use 'pinconf-single' compatible for the am33xx_pinmux node:
> > f46fe79ff1b6 ("pinctrl-single: fix pcs_parse_pinconf() return value")
> >
>
> In the kernel version 4.1.6 that I am using on my custom board, I have fixed
> the commit f07512e615dd ("pinctrl/pinconfig: add debug interface"). However,
> this feature was later removed (https://lore.kernel.org/patchwork/patch/1033755/).
> Do you think it is better to bring that functionality back to life or the submitted
> patch could be fine too?

Wow, I had no idea there used to be a pinconf-config debugfs file.  I
would have been interested in using it if it had still existed.

Regarding your patch, I think it could be helpful to be able to set
the conf_<module>_<pin> registers directly through debugfs as I can
imagine situations where it would be useful for testing.  It is a bit
dangerous as the person using it has to be careful not to change the
wrong bits, but they would need to have debugfs mounted and
permissions to write to it.   I suppose it depends on what others
maintainers like Linus and Tony think about whether that is an
acceptable solution.

thanks,
drew