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

["Rafał Miłecki" <zajec5@gmail.com>]

On 14.12.2021 20:59, Rob Herring wrote:
> 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).

OK, thanks for sharing that. Given a pretty limited optimism on this
approach I'll simply drop it and do things the old good way.

I thought it's a better desing but I probably was wrong. It was still
worth a try :)

Thanks to everyone involved in this discussion.


>> 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'?

It could probably be handled with sth like

functions {
	bar {
		pins = <&foo>;
	}
}

but my binding didn't cover that indeed.


>> 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 thought my design description & BCM5301X DTS patch may be a proof of
that but apparently it wasn't enough ;)