linux-kernel - Re: [RFC PATCH] pinctrl: add helper to expose pinctrl state in debugfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75Vf-=nM-M2K-v_8iyME4t6ZF-gvSZ5ePsxQFhObJ_0YHsw@mail.gmail.com>
Date:   Mon, 14 Dec 2020 19:55:12 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Drew Fustini <drew@...gleboard.org>
Cc:     "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Tony Lindgren <tony@...mide.com>
Subject: Re: [RFC PATCH] pinctrl: add helper to expose pinctrl state in debugfs

On Sat, Dec 12, 2020 at 1:43 AM Drew Fustini <drew@...gleboard.org> wrote:
> On Fri, Dec 11, 2020 at 11:15:21PM +0200, Andy Shevchenko wrote:
> > On Fri, Dec 11, 2020 at 1:54 PM Drew Fustini <drew@...gleboard.org> wrote:
> > >
> > > BeagleBoard.org [0] currently uses an out-of-tree driver called
> > > bone-pinmux-helper [1] developed by Pantelis Antoniou [2] back in 2013.
> >
> > And it looks like it's still using APIs from 2013.
> > Needs quite a clean up.
>
> Thanks for taking a look at my RFC and responding. It is good to know
> that it is using out-dated APIs. Would you be able to elaborate?
>
> It interacts with pinctrl core through devm_pinctrl_get(),
> pinctrl_lookup_state() and pinctrl_select_state(). Is there newer way of
> doing that?

No. I'm talking mostly about FS callbacks where some relatively old
new APIs can be used, such as kasprintf().

...

> > > I used the compatible string "pinctrl,state-helper" but would appreciate
> > > advice on how to best name this. Should I create a new vendor prefix?
> >
> > Since it's BB specific, it should have file name and compatible string
> > accordingly.
>
> At first, I was thinking about this as a beaglebone specific solution
> and had bone in the driver name and compatible string. But then I
> realized it could used in other situations where it is beneficial to
> to read and select a pinctrl state through debugfs.
>
> I'm happy to rebrand the naming as beaglebone if that would be more
> acceptable.

See below.

> > But I'm wondering, why it requires this kind of thing and can't be
> > simply always part of the kernel based on configuration option?
>
> Do you mean not having a new CONFIG option for this driver and just have
> it be enabled by CONFIG_PINCTRL?

No, configuration option stays, but no compatible strings no nothing
like that. Just probed always when loaded.
Actually not even sure we want to have it as a module.

...

> > > The P9_14_pinmux entry would cause pinctrl-state-helper to be probed.
> > > The driver would create the corresponding pinctrl state file in debugfs
> > > for the pin.  Here is an example of how the state can be read and
> > > written from userspace:
> > >
> > > root@...glebone:~# cat /sys/kernel/debug/ocp\:P9_14_pinmux/state
> > > default
> > > root@...glebone:~# echo pwm > /sys/kernel/debug/ocp\:P9_14_pinmux/state
> > > root@...glebone:~# cat /sys/kernel/debug/ocp\:P9_14_pinmux/state
> > > pwm
> >
> > Shouldn't it be rather a part of a certain pin control folder:
> > debug/pinctrl/.../mux/...
> > ?
>
> Yes, I think that would make sense, but I was struggling to figure out
> how to do that. pinctrl_init_debugfs() in pinctrl/core.c does create the
> "pinctrl" directory, but I could not figure out how to use this as the
> parent dir when calling debugfs_create_dir() in this driver's probe().
>
> I thought there might be a way in debugfs API to use existing directory
> path as a parent but I couldn't figure anything like that. I would
> appreciate any advice.

If the option is boolean from the beginning then you just call it from
the corresponding pin control instantiation chain.

-- 
With Best Regards,
Andy Shevchenko