[0/2] Apple M1 pinctrl DT bindings

This small series adds bindings for the pin and GPIO controller found
on the Apple M1 SoC.

At this point, the primary consumer for these bindings is U-Boot.
With the help of these bindings U-Boot can bring up the PCIe root
complex into a state where the OS use it without having to deal with
the GPIO pins.  A Linux driver may be implemented later to support
power management and/or other devices that need GPIO control.  Since
the binding is based on standard pinmux node bindings this should be
relatively easy.


Mark Kettenis (2):
  dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
  arm64: apple: Add pinctrl nodes

 .../bindings/pinctrl/apple,pinctrl.yaml       | 103 ++++++++++++++++++
 MAINTAINERS                                   |   2 +
 arch/arm64/boot/dts/apple/t8103.dtsi          |  83 ++++++++++++++
 include/dt-bindings/pinctrl/apple.h           |  13 +++
 4 files changed, 201 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
 create mode 100644 include/dt-bindings/pinctrl/apple.h

On Sat, May 8, 2021 at 4:20 PM Mark Kettenis <kettenis@openbsd.org> wrote:

> The Apple GPIO controller is a simple combined pin and GPIO conroller
> present on Apple ARM SoC platforms, including various iPhone and iPad
> devices and the "Apple Silicon" Macs.
>
> Signed-off-by: Mark Kettenis <kettenis@openbsd.org>

I knew this was coming! I saw an earlier version of the Linux
pin control driver in some tree somewhere.

I see we're only discussing bindings right now, but it would be
great to also take a look at the U-Boot driver and scratch Linux
driver (which I bet both exist) for a deeper understanding.
Git tree web links are fine.

> +description: |
> +  The Apple GPIO controller is a simple combined pin and GPIO controller

spelling

> +  present on Apple ARM SoC platforms, including various iPhone and iPad
> +  devices and the "Apple Silicon" Macs.

> +properties:
> +  compatible:
> +    items:
> +      - const: apple,t8103-pinctrl
> +      - const: apple,pinctrl

So is this an entirely Apple thing now, and not based on some Samsung
block from S3C like what we have seen before?

It'd be great if Krzysztof or Tomasz who have experience with the
Samsung hardware could have a look at the registers etc in the
drivers and confirm or clear any relationship to Samsung hardware.

This would partly involve trying to keep the pin control bindings
similar to Samsungs if there is a relationship.

If there is no relationship, then we invent something new.

All looks pretty good, but I am suspicious about this:

> +  interrupts:
> +    minItems: 1
> +    maxItems: 7

Which is used like that.

> +        interrupt-controller;
> +        interrupt-parent = <&aic>;
> +        interrupts = <AIC_IRQ 16 IRQ_TYPE_LEVEL_HIGH>,
> +                     <AIC_IRQ 17 IRQ_TYPE_LEVEL_HIGH>,
> +                     <AIC_IRQ 18 IRQ_TYPE_LEVEL_HIGH>,
> +                     <AIC_IRQ 19 IRQ_TYPE_LEVEL_HIGH>,
> +                     <AIC_IRQ 20 IRQ_TYPE_LEVEL_HIGH>,
> +                     <AIC_IRQ 21 IRQ_TYPE_LEVEL_HIGH>,
> +                     <AIC_IRQ 22 IRQ_TYPE_LEVEL_HIGH>;

First it is really odd with 7 of something as in all computer
science but I guess there is some explanation for that.

What I am really wondering is if these interrupts are hierarchical,
i.e. that they match 1-to-1 to a GPIO line.

We only (ideally) define the interrupts when it is used by the
GPIO block itself, such as when it spawns a cascaded interrupt
controller (i.e. you need to read status bits inside the GPIO
controller to figure out which line was fired).

If the interrupt has a 1-to-1 mapping between GPIO lines and
the parent interrupt controller we usually do not define these
interrupts in the device tree at all.

In those cases the interrupt is considered hierarchical and we
rely on the compatible for the block to define how the
interrupt lines are routed to the parent interrupt controller
(in this case AIC).

In the Linux case, the GPIO driver has a hardcoded table
of mappings from the GPIO irq line offset and the corresponding
index on the parent interrupt controller (AIC).

This is reflected in this IRQ routing information missing
from the bindings.

Marc Zyngier can probably tell the story of why it is handled
like this,

There is some info on hierarchical IRQ handling in the
Linux GPIO driver docs:
https://www.kernel.org/doc/html/latest/driver-api/gpio/driver.html
Section "GPIO drivers providing IRQs"

Yours,
Linus Walleij

Hi Mark,

here is a second note on pin mux layout:

On Sat, May 8, 2021 at 4:20 PM Mark Kettenis <kettenis@openbsd.org> wrote:

> +        pcie_pins: pcie-pins {
> +          pinmux = <APPLE_PINMUX(150, 1)>,
> +                   <APPLE_PINMUX(151, 1)>,
> +                   <APPLE_PINMUX(32, 1)>;
(...)
> +#define APPLE_PINMUX(pin, func) ((pin) | ((func) << 16))
> +#define APPLE_PIN(pinmux) ((pinmux) & 0xffff)
> +#define APPLE_FUNC(pinmux) ((pinmux) >> 16)

Since the word altfunction is used, I suppose that this is
one of those pin controllers where each pin can be
muxed individually (usually with one register or one group
of bits per pin).

So this is one way to do it, which

Another way is what Qualcomm is doing and looks for
example like this:

pinctrl@800000 {
         /* eMMMC pins, all 8 data lines connected */
         dragon_sdcc1_pins: sdcc1 {
                 mux {
                         pins = "gpio159", "gpio160", "gpio161",
                              "gpio162", "gpio163", "gpio164",
                              "gpio165", "gpio166", "gpio167",
                              "gpio168";
                         function = "sdc1";
                 };
(...)

Here all pins have a name and they get assigned as a group
to a function. Each pin is referenced by name.

Some people don't like this because they like bitstuffing and
bitfiddling and are worried that the DTB file strings will take
up too much memory, and they have to include all these
strings in their operating system driver.

However there are clear upsides to it, when you later on
come to set up the electrical pin config:

                cmd {
                         pins = "gpio168"; /* SDC1 CMD */
                         drive-strength = <12>;
                         bias-pull-up;
                 };
                 data {
                         /* SDC1 D0 to D7 */
                         pins = "gpio159", "gpio160", "gpio161", "gpio162",
                               "gpio163", "gpio164", "gpio165", "gpio166";
                         drive-strength = <8>;
                         bias-pull-none;
                  };

As you can see this becomes quite readable. It is clear and
crisp which pins are set up for pull-up and not, and what
drive strength is used on each pin.

But notice first and foremost this: the muxing is done in
one node, and the electrical config is done in two separate
nodes, breaking muxing and config into two different
categories in the device tree.

The problem with the magic number approach to muxing
is that the magic numbers will fall through to the
electrical pin config later and indeed it looks like in the STM32
device trees:

sdmmc1_b4_od_pins_a: sdmmc1-b4-od-0 {
        pins1 {
                pinmux = <STM32_PINMUX('C', 8, AF12)>, /* SDMMC1_D0 */
                                <STM32_PINMUX('C', 9, AF12)>, /* SDMMC1_D1 */
                                <STM32_PINMUX('C', 10, AF12)>, /* SDMMC1_D2 */
                                <STM32_PINMUX('C', 11, AF12)>, /* SDMMC1_D3 */
                                <STM32_PINMUX('C', 12, AF12)>; /* SDMMC1_CK */
                slew-rate = <3>;
                drive-push-pull;
                bias-disable;
        };
        pins2 {
                pinmux = <STM32_PINMUX('D', 2, AF12)>; /* SDMMC1_CMD */
                slew-rate = <3>;
                drive-open-drain;
                bias-disable;
         };
};

Notice here how the pins need to be separated into two subnodes
in order to set different electrical configuration on them, and how
muxing and configuration are mixed up. This is a side effect of
using the "pinmux" attribute rather than "pins" and "function".

So make sure you really like this rather than the other approach
in your device trees.

I will definately insist that you electrical config be done similar
to how STM32 does it when you implement that later, for
example any magic numbers for electrical config is not
acceptable, you will have to find a way to use drive-open-drain;
and such flags in the device tree.

Sadly we have something like three different ways to do
pin control device tree, as a result of failure to find consensus.

Yours,
Linus Walleij

2021年5月9日(日) 9:27 Linus Walleij <linus.walleij@linaro.org>:
>
> On Sun, May 9, 2021 at 1:02 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > [Me]
> > > On Sat, May 8, 2021 at 4:20 PM Mark Kettenis <kettenis@openbsd.org> wrote:
>
> > My U-Boot driver is here:
>
> Thanks! Looks nice.
>
> > > > +description: |
> > > > +  The Apple GPIO controller is a simple combined pin and GPIO controller
> > >
> > > spelling
> >
> > Not sure I'm seeing a spelling mistake here.  Do you want a comma
> > inserted somewhere?
>
> Your original mail says "conroller" but the helpful google mail
> editor autocorrected the mistake when I hit enter after it.
>
> > > So is this an entirely Apple thing now, and not based on some Samsung
> > > block from S3C like what we have seen before?
> >
> > As far as I can tell, yes.  This Apple controller has a single
> > register per pin that controls the muxing and gpio functions, whereas
> > the S3C controller seems to have 4 registers per pin.
>
> Fair enough.
>

Right, doesn't sound like any Samsung pin controller I'm familiar
with, although I haven't followed new hardware developments since I
left Samsung a few years ago. I've stayed as a maintainer mostly to
help with the legacy SoCs I had worked with, e.g. s3c6410. :)

Best regards,
Tomasz

On 09/05/2021 05:50, Tomasz Figa wrote:
>>>> So is this an entirely Apple thing now, and not based on some Samsung

>>>> block from S3C like what we have seen before?

>>>

>>> As far as I can tell, yes.  This Apple controller has a single

>>> register per pin that controls the muxing and gpio functions, whereas

>>> the S3C controller seems to have 4 registers per pin.

>>

>> Fair enough.

>>

> 

> Right, doesn't sound like any Samsung pin controller I'm familiar

> with, although I haven't followed new hardware developments since I

> left Samsung a few years ago. I've stayed as a maintainer mostly to

> help with the legacy SoCs I had worked with, e.g. s3c6410. :)


I can confirm that it looks different than Samsung designs.


Best regards,
Krzysztof