Re: [PATCH V3 1/2] dt-bindings: pinctrl: support specifying pins, groups & functions

[Rob Herring <robh@kernel.org>]

On Sat, Dec 11, 2021 at 12:16:25PM +0100, Rafał Miłecki wrote:
> Rob: please kindly comment on this idea of storing pins/groups/functions
> in DT.

I was never a fan of stuffing pin mux/ctrl into DT for what's mostly a 
one time stuffing of register values. And given how many things run 
before getting to the kernel, doing proper pin configuration in the 
kernel is much too late (or redundant because it was actually already 
done).

> 
> For a sample Linux implementation you can check (incomplete):
> [PATCH V2 4/6] pinctrl: support reading pins, groups & functions from DT
> https://patchwork.ozlabs.org/project/linux-gpio/patch/20211124230439.17531-5-zajec5@gmail.com/
> 
> For a real life DT usage you can check:
> [PATCH V2 6/6] ARM: dts: BCM5301X: add pinctrl pins, groups & functions
> https://patchwork.ozlabs.org/project/linux-gpio/patch/20211124230439.17531-7-zajec5@gmail.com/

What about h/w with no concept of 'groups'?


> Also see below inline comments.
> 
> 
> On 11.12.2021 00:26, Linus Walleij wrote:
> > On Fri, Dec 10, 2021 at 12:42 PM Rafał Miłecki <zajec5@gmail.com> wrote:
> > 
> > > This binding change is meant to introduce a generic way of describing
> > > pinctrl blocks details. Every pinmux block is expected to have:
> > > 1. Named pins
> > > 2. Named groups containing one or more pins
> > > 3. Named functions referencing one or more groups
> > > 
> > > It doesn't describe how hw should be programmed. That remains binding
> > > and driver specific.
> > 
> > So what this does is to take a large chunk of data that we known to be
> > associated with the compatible string (names of pins, groups and functions,
> > etc) and put it into the device tree instead of the alternative, which is
> > what most drivers do, and that is to compile in the data into the
> > operating system and just look it up by using a compatible
> > string.
> 
> Correct. It changes the place of storing platform specific data.
> 
> 
> > The DT maintainers have already indicated that this is not desirable
> > and I don't see it getting merged before it has a Reviewed-by
> > tag from one of the DT binding maintainers.
> 
> Tony pointed out that it was back in 2011. It's worth reconsidering.
> https://patchwork.ozlabs.org/comment/2786915/
> 
> Rob said it depends on whether "data be static (complete) and correct"
> https://patchwork.ozlabs.org/comment/2786688/

I haven't seen an answer for that question...

That and working for multiple platforms (from different vendors) are the 
main things that matter to me. 

> I find it absolutely required to get Rob's Reviewed-by before we merge
> it. I hope we can get Rob's opinion on this.
> 
> 
> > I think we need to know what the USP (unique selling point) is?
> > 
> > Would it be something like not having to duplicate work across some
> > boot loaders and operating systems? (Well they all need to parse this
> > type of description but that can be put into a library.)
> > 
> > Or something else?
> 
> There are two reasons for me to work on this binding:
> 
> 
> 1. I think it's cleaner to keep pinctrl details in DT
> 
> DT seems more natural (than C code) for:
> a) Translating info from datasheets
> b) Storing pin/group/function custom values
> c) Defining relations (phandles)
> d) Handling chip differences (adding new pins in newer revisions)
> 
> Last time I learnt that pins don't always/usually have numbers (in
> datasheets) but are rather named. Still in the "pinctrl_pin_desc" we
> have "unsigned number" just to enumerate them and reference in groups.
> 
> Adding or removing pins/groups/functions in DT is as simple as
> adding/deleting nodes. That also means less logic in C code.
> 
> 
> 2. It avoids data duplication
> 
> It allows me to keep pins/groups/functions data in one place (DT) and
> use it in both: Linux and U-Boot. No duplication & easier maintenance.
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel