Re: [PATCH v1 1/1] pinctrl: add pinctrl/GPIO driver for Apple SoCs

["Sven Peter" <sven@xxxxxxxxxxxxx>]

On Sun, Sep 26, 2021, at 15:10, Linus Walleij wrote:
> On Sun, Sep 26, 2021 at 2:56 PM Sven Peter <sven@xxxxxxxxxxxxx> wrote:
>> On Sun, Sep 26, 2021, at 14:48, Linus Walleij wrote:
>
>> > I think npins should be known from the compatible (we know that
>> > this version of the SoC has so and so many pins) and the base
>> > should always be 0? It's not like we have several pin controllers
>> > of this type in the SoC is it?
>>
>> I've just checked: Looks like there are four different pin controllers of
>> this type with a different number of pins each in Apple's device tree for
>> the M1.
>
> So we need to understand this hardware: is this something like
> south/north/east/west, so the pins are oriented around the chip?
>
> I would suspect they should then be compatibles:
> apple,t8103-pinctrl-north, apple,t8103-pinctrl
> apple,t8103-pinctrl-south, apple,t8103-pinctrl
> apple,t8103-pinctrl-west, apple,t8103-pinctrl
> apple,t8103-pinctrl-east, apple,t8103-pinctrl
>
> going from specific to general signifying that we know which one
> we are dealing with and then we know the npins etc.

Apple calls those four controllers "gpio", "nub-gpio", "smc-gpio"
and "aop-gpio". SMC is their system management controller and AOP
is their "always-on processor". No idea what "nub-gpio" is.

>
> This also gives a normal grouping of functions associated with
> pins and the portion of the SoC, see for
> example drivers/pinctrl/intel/pinctrl-broxton.c.
>
> This shows that it might have been a bad idea to define the
> pins as opaque, because now that is hiding the fact that
> these pins are grouped in four distinct sets.
> APPLE_PINMUX(pin, func)
>
> Should rather have been APPLE_PINMUX(cardinal, pin, func)
> where cardinal would be 0..3 for north, south, west, east.
>
> This is a bit of guessing actually, but we should definitely
> try to make the driver reflect the reality and not over-generalize.
> If these four blocks in the SoC are different, they should have
> different compatible strings, because they are not, by
> definition, compatible.

I'd prefer to have a single compatible and get the npins from some
property and I don't think that's necessarily over-generalizing.
AFAICT Apple has been using the exact same MMIO interface for years
and I'd expect them to continue using it in the future. The only thing
that seems to change is the number of pins available and their assignment.
If we just have a single compatible we can likely support the M1X/2 or
however Apple calls the next SoCs with just a simple DTB change without
touching any driver code.



Best,

Sven