* [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
@ 2021-05-08 14:19 ` Mark Kettenis
0 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-08 14:19 UTC (permalink / raw)
To: devicetree
Cc: Mark Kettenis, Hector Martin, Linus Walleij, Rob Herring,
linux-arm-kernel, linux-gpio, linux-kernel
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>
---
.../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
MAINTAINERS | 2 +
include/dt-bindings/pinctrl/apple.h | 13 +++
3 files changed, 118 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
create mode 100644 include/dt-bindings/pinctrl/apple.h
diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
new file mode 100644
index 000000000000..cc7805ca6ba1
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
@@ -0,0 +1,103 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/apple,pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple GPIO controller
+
+maintainers:
+ - Mark Kettenis <kettenis@openbsd.org>
+
+description: |
+ 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.
+
+properties:
+ compatible:
+ items:
+ - const: apple,t8103-pinctrl
+ - const: apple,pinctrl
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
+ gpio-ranges:
+ maxItems: 1
+
+ interrupts:
+ minItems: 1
+ maxItems: 7
+
+ interrupt-controller: true
+
+patternProperties:
+ '-pins$':
+ type: object
+ $ref: pinmux-node.yaml#
+
+ properties:
+ pinmux:
+ description:
+ Values are constructed from pin number and alternate function
+ configuration number using the APPLE_PINMUX() helper macro
+ defined in include/dt-bindings/pinctrl/apple.h.
+
+ required:
+ - pinmux
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - gpio-controller
+ - '#gpio-cells'
+ - gpio-ranges
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/apple-aic.h>
+ #include <dt-bindings/pinctrl/apple.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ pinctrl: pinctrl@23c100000 {
+ compatible = "apple,t8103-pinctrl", "apple,pinctrl";
+ reg = <0x2 0x3c100000 0x0 0x100000>;
+ clocks = <&gpio_clk>;
+
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&pinctrl 0 0 212>;
+
+ 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>;
+
+ pcie_pins: pcie-pins {
+ pinmux = <APPLE_PINMUX(150, 1)>,
+ <APPLE_PINMUX(151, 1)>,
+ <APPLE_PINMUX(32, 1)>;
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index ad0e9be66885..7327c9b778f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1654,9 +1654,11 @@ C: irc://chat.freenode.net/asahi-dev
T: git https://github.com/AsahiLinux/linux.git
F: Documentation/devicetree/bindings/arm/apple.yaml
F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
+F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
F: arch/arm64/boot/dts/apple/
F: drivers/irqchip/irq-apple-aic.c
F: include/dt-bindings/interrupt-controller/apple-aic.h
+F: include/dt-bindings/pinctrl/apple.h
ARM/ARTPEC MACHINE SUPPORT
M: Jesper Nilsson <jesper.nilsson@axis.com>
diff --git a/include/dt-bindings/pinctrl/apple.h b/include/dt-bindings/pinctrl/apple.h
new file mode 100644
index 000000000000..ea0a6f466592
--- /dev/null
+++ b/include/dt-bindings/pinctrl/apple.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0+ OR MIT */
+/*
+ * This header provides constants for Apple pinctrl bindings.
+ */
+
+#ifndef _DT_BINDINGS_PINCTRL_APPLE_H
+#define _DT_BINDINGS_PINCTRL_APPLE_H
+
+#define APPLE_PINMUX(pin, func) ((pin) | ((func) << 16))
+#define APPLE_PIN(pinmux) ((pinmux) & 0xffff)
+#define APPLE_FUNC(pinmux) ((pinmux) >> 16)
+
+#endif /* _DT_BINDINGS_PINCTRL_APPLE_H */
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-08 14:19 ` Mark Kettenis
@ 2021-05-08 21:09 ` Linus Walleij
-1 siblings, 0 replies; 38+ messages in thread
From: Linus Walleij @ 2021-05-08 21:09 UTC (permalink / raw)
To: kettenis, Krzysztof Kozlowski, Tomasz Figa, Marc Zyngier
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel, Bartosz Golaszewski
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-08 21:09 ` Linus Walleij
0 siblings, 0 replies; 38+ messages in thread
From: Linus Walleij @ 2021-05-08 21:09 UTC (permalink / raw)
To: kettenis, Krzysztof Kozlowski, Tomasz Figa, Marc Zyngier
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel, Bartosz Golaszewski
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-08 21:09 ` [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl Linus Walleij
@ 2021-05-08 23:02 ` Mark Kettenis
-1 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-08 23:02 UTC (permalink / raw)
To: Linus Walleij
Cc: kettenis, krzk, tomasz.figa, maz, devicetree, marcan, robh+dt,
linux-arm-kernel, linux-gpio, linux-kernel, bgolaszewski, sven
> From: Linus Walleij <linus.walleij@linaro.org>
> Date: Sat, 8 May 2021 23:09:55 +0200
>
> 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>
Hi Linus,
Thanks for taking a look.
> I knew this was coming! I saw an earlier version of the Linux
> pin control driver in some tree somewhere.
That may have been the initial Corellium port. I'm working with the
Asahi Linux folks to get something that is a bit more upstreamable and
something that works for U-Boot and other OSes too. So these proposed
DT bindings deviate a little bit from what Corellium did.
> 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.
My U-Boot driver is here:
https://github.com/kettenis/u-boot/blob/apple-m1-m1n1-gpio/drivers/pinctrl/pinctrl-apple.c
Not yet submitted upstream since that is a bit pointless without an
agreed upon binding; typically U-Boot requires a binding that is
accepted by the Linux devlopers before accepting patches.
Sven Peter may have a draft Linux driver somewhere as he has been
working on support for the type-C USB ports, but I haven't seen that
part of his code yet.
> > +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?
>
> > + 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?
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.
In the Apple device tree this controller has a "gpio,t8101" compatible
property which suggests that this particular incarnation was
introduced with the the T8101 SoC (marketed as A14 and found in the
iPhone 12). Of course that's just an educated guess.
> 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.
No kidding. But all 4 controllers present on the SoC have 7
interrupts assigned to them in the Apple device tree.
> What I am really wondering is if these interrupts are hierarchical,
> i.e. that they match 1-to-1 to a GPIO line.
They don't match 1-1. The GPIOs can be assigned to one of
(apparently) 7 interrupt groups. I haven't looked to closely at this
yet since U-Boot doesn't need/use the interrupt capability. But I
suspect that pins don't have to be assigned to a interrupt group and
that explains why there are only 7 interrupt groups as the 8th state
is reserved for "unasigned". The number of pins per controller
varies, but one of them has 212 pins.
> 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).
Multiple pins can be assigned to the same interrupt group as far as I
can tell. So in that case the driver will have to look at status
bits.
> 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,
Ok, hopefully Marc can say something sensible here, but I'd say the
interrupts on this hardware are cascaded.
> 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
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-08 23:02 ` Mark Kettenis
0 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-08 23:02 UTC (permalink / raw)
To: Linus Walleij
Cc: kettenis, krzk, tomasz.figa, maz, devicetree, marcan, robh+dt,
linux-arm-kernel, linux-gpio, linux-kernel, bgolaszewski, sven
> From: Linus Walleij <linus.walleij@linaro.org>
> Date: Sat, 8 May 2021 23:09:55 +0200
>
> 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>
Hi Linus,
Thanks for taking a look.
> I knew this was coming! I saw an earlier version of the Linux
> pin control driver in some tree somewhere.
That may have been the initial Corellium port. I'm working with the
Asahi Linux folks to get something that is a bit more upstreamable and
something that works for U-Boot and other OSes too. So these proposed
DT bindings deviate a little bit from what Corellium did.
> 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.
My U-Boot driver is here:
https://github.com/kettenis/u-boot/blob/apple-m1-m1n1-gpio/drivers/pinctrl/pinctrl-apple.c
Not yet submitted upstream since that is a bit pointless without an
agreed upon binding; typically U-Boot requires a binding that is
accepted by the Linux devlopers before accepting patches.
Sven Peter may have a draft Linux driver somewhere as he has been
working on support for the type-C USB ports, but I haven't seen that
part of his code yet.
> > +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?
>
> > + 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?
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.
In the Apple device tree this controller has a "gpio,t8101" compatible
property which suggests that this particular incarnation was
introduced with the the T8101 SoC (marketed as A14 and found in the
iPhone 12). Of course that's just an educated guess.
> 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.
No kidding. But all 4 controllers present on the SoC have 7
interrupts assigned to them in the Apple device tree.
> What I am really wondering is if these interrupts are hierarchical,
> i.e. that they match 1-to-1 to a GPIO line.
They don't match 1-1. The GPIOs can be assigned to one of
(apparently) 7 interrupt groups. I haven't looked to closely at this
yet since U-Boot doesn't need/use the interrupt capability. But I
suspect that pins don't have to be assigned to a interrupt group and
that explains why there are only 7 interrupt groups as the 8th state
is reserved for "unasigned". The number of pins per controller
varies, but one of them has 212 pins.
> 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).
Multiple pins can be assigned to the same interrupt group as far as I
can tell. So in that case the driver will have to look at status
bits.
> 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,
Ok, hopefully Marc can say something sensible here, but I'd say the
interrupts on this hardware are cascaded.
> 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
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-08 23:02 ` [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl Mark Kettenis
@ 2021-05-09 0:27 ` Linus Walleij
-1 siblings, 0 replies; 38+ messages in thread
From: Linus Walleij @ 2021-05-09 0:27 UTC (permalink / raw)
To: Mark Kettenis
Cc: kettenis, Krzysztof Kozlowski, Tomasz Figa, Marc Zyngier,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel, Bartosz Golaszewski, sven
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.
> > What I am really wondering is if these interrupts are hierarchical,
> > i.e. that they match 1-to-1 to a GPIO line.
>
> They don't match 1-1. The GPIOs can be assigned to one of
> (apparently) 7 interrupt groups.
Aha so it is a 1-to-1..* thing. How delicate.
> I haven't looked to closely at this
> yet since U-Boot doesn't need/use the interrupt capability. But I
> suspect that pins don't have to be assigned to a interrupt group and
> that explains why there are only 7 interrupt groups as the 8th state
> is reserved for "unasigned". The number of pins per controller
> varies, but one of them has 212 pins.
Wow.
> Multiple pins can be assigned to the same interrupt group as far as I
> can tell. So in that case the driver will have to look at status
> bits.
OK then this is not hierarchical.
> > Marc Zyngier can probably tell the story of why it is handled
> > like this,
>
> Ok, hopefully Marc can say something sensible here, but I'd say the
> interrupts on this hardware are cascaded.
Yes looks like so, it will be an interesting interrupt driver when
you get to that.
I have only the question in my second mail (just sent) but in any
case you are not doing anything out of the ordinary (it looks very
similar to the STM32) so I'm pleased with this binding.
I wanna give the DT reviewers some time to look at it as well
but I imagine we can soon merge this.
Yours.
Linus Walleij
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-09 0:27 ` Linus Walleij
0 siblings, 0 replies; 38+ messages in thread
From: Linus Walleij @ 2021-05-09 0:27 UTC (permalink / raw)
To: Mark Kettenis
Cc: kettenis, Krzysztof Kozlowski, Tomasz Figa, Marc Zyngier,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel, Bartosz Golaszewski, sven
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.
> > What I am really wondering is if these interrupts are hierarchical,
> > i.e. that they match 1-to-1 to a GPIO line.
>
> They don't match 1-1. The GPIOs can be assigned to one of
> (apparently) 7 interrupt groups.
Aha so it is a 1-to-1..* thing. How delicate.
> I haven't looked to closely at this
> yet since U-Boot doesn't need/use the interrupt capability. But I
> suspect that pins don't have to be assigned to a interrupt group and
> that explains why there are only 7 interrupt groups as the 8th state
> is reserved for "unasigned". The number of pins per controller
> varies, but one of them has 212 pins.
Wow.
> Multiple pins can be assigned to the same interrupt group as far as I
> can tell. So in that case the driver will have to look at status
> bits.
OK then this is not hierarchical.
> > Marc Zyngier can probably tell the story of why it is handled
> > like this,
>
> Ok, hopefully Marc can say something sensible here, but I'd say the
> interrupts on this hardware are cascaded.
Yes looks like so, it will be an interesting interrupt driver when
you get to that.
I have only the question in my second mail (just sent) but in any
case you are not doing anything out of the ordinary (it looks very
similar to the STM32) so I'm pleased with this binding.
I wanna give the DT reviewers some time to look at it as well
but I imagine we can soon merge this.
Yours.
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-09 0:27 ` [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl Linus Walleij
@ 2021-05-09 9:50 ` Tomasz Figa
-1 siblings, 0 replies; 38+ messages in thread
From: Tomasz Figa @ 2021-05-09 9:50 UTC (permalink / raw)
To: Linus Walleij
Cc: Mark Kettenis, kettenis, Krzysztof Kozlowski, Marc Zyngier,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel, Bartosz Golaszewski, sven
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-09 9:50 ` Tomasz Figa
0 siblings, 0 replies; 38+ messages in thread
From: Tomasz Figa @ 2021-05-09 9:50 UTC (permalink / raw)
To: Linus Walleij
Cc: Mark Kettenis, kettenis, Krzysztof Kozlowski, Marc Zyngier,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel, Bartosz Golaszewski, sven
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-09 9:50 ` [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl Tomasz Figa
@ 2021-05-10 13:09 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 38+ messages in thread
From: Krzysztof Kozlowski @ 2021-05-10 13:09 UTC (permalink / raw)
To: Tomasz Figa, Linus Walleij
Cc: Mark Kettenis, kettenis, Marc Zyngier,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel, Bartosz Golaszewski, sven
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
@ 2021-05-10 13:09 ` Krzysztof Kozlowski
0 siblings, 0 replies; 38+ messages in thread
From: Krzysztof Kozlowski @ 2021-05-10 13:09 UTC (permalink / raw)
To: Tomasz Figa, Linus Walleij
Cc: Mark Kettenis, kettenis, Marc Zyngier,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel, Bartosz Golaszewski, sven
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-08 14:19 ` Mark Kettenis
@ 2021-05-09 0:18 ` Linus Walleij
-1 siblings, 0 replies; 38+ messages in thread
From: Linus Walleij @ 2021-05-09 0:18 UTC (permalink / raw)
To: kettenis
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel
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
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-09 0:18 ` Linus Walleij
0 siblings, 0 replies; 38+ messages in thread
From: Linus Walleij @ 2021-05-09 0:18 UTC (permalink / raw)
To: kettenis
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-09 0:18 ` [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl Linus Walleij
@ 2021-05-09 9:46 ` Mark Kettenis
-1 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-09 9:46 UTC (permalink / raw)
To: Linus Walleij
Cc: kettenis, devicetree, marcan, robh+dt, linux-arm-kernel,
linux-gpio, linux-kernel
> From: Linus Walleij <linus.walleij@linaro.org>
> Date: Sun, 9 May 2021 02:18:52 +0200
Hi Linus,
> 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).
Right. So far it seems that there are just two functions: GPIO and a
peripheral mode. But there are some unused bits in the register right
next to the bit that controls the function, so maybe we'll see other
function numbers in the future.
> 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.
Since we will probably never have a data sheet or board schematics for
the Apple devices giving meaningful names to the pins isn't really
possible. So you'd end up with generic "gpioNNN" names like in the
example above.
> 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.
The memory thing is certainly a consideration for U-Boot, although not
particularly for this platform. With my OpenBSD hat on I must say
that we try to avoid string parsing in the kernel as much as possible,
which is why I would prefer pin numbers over strings. Interestingly
enough the pinmux-node.yaml schema allows the use of numbers instead
of strings for the "pins" property. But neither U-Boot nor Linux
seems to implement support for this in its generic pinctrl/pinmux
code.
However, the main reason for me to pick the "pinmux" scheme used by
STM32 is to make things a bit more future proof. We expect Apple to
come out with new models and new SoCs on a regular basis and there is
some evidence that Apple tends to re-uses hardware blocks between SoC
generations. So we hope that it will be possible to add support for
new Apple hardware by simply providing new device trees in m1n1 (the
bootloader developed by Asahi). Then all we have to do is update m1n1
with a new device tree and people can simply use an existing distro
without having to wait until the latest kernel becomes available for
their distro. My feeling is that this is a bit easier to achieve
using the bitstuffing/fiddling approach, although with some wisely
chosen names for the alternate functions, the string-based approach
might work as well.
> 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.
I do see the elegance of this approach.
> 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.
Right. So far it seems we don't need to set the electrical config on
any of the pins and I don't think we've identified any bits in the
register that change the electrical configuration. Although there are
some hints in the Apple device tree that there are pins that need some
additional configuration. But I also see some evidence that the Apple
firmware on these devices already sets up most of the pins for us.
> 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.
Agreed. The plan was that when we get at the point where we need to
fiddle with the electrical configuration of pins, we simply add a
reference to pincfg-node.yaml to the schema.
> Sadly we have something like three different ways to do
> pin control device tree, as a result of failure to find consensus.
Thank you for giving quite an extensive explanation of the options. I
think all things considered, I am still happy with using the "pinmux"
property, but if Hector or Sven think the "pin"/"function" approach
with strings is better, I can change it. Implementing that approach
in U-Boot isn't a huge burden either. In fact that was pretty much
what my initial implementation based on the bindings used by Corellium
did.
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-09 9:46 ` Mark Kettenis
0 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-09 9:46 UTC (permalink / raw)
To: Linus Walleij
Cc: kettenis, devicetree, marcan, robh+dt, linux-arm-kernel,
linux-gpio, linux-kernel
> From: Linus Walleij <linus.walleij@linaro.org>
> Date: Sun, 9 May 2021 02:18:52 +0200
Hi Linus,
> 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).
Right. So far it seems that there are just two functions: GPIO and a
peripheral mode. But there are some unused bits in the register right
next to the bit that controls the function, so maybe we'll see other
function numbers in the future.
> 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.
Since we will probably never have a data sheet or board schematics for
the Apple devices giving meaningful names to the pins isn't really
possible. So you'd end up with generic "gpioNNN" names like in the
example above.
> 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.
The memory thing is certainly a consideration for U-Boot, although not
particularly for this platform. With my OpenBSD hat on I must say
that we try to avoid string parsing in the kernel as much as possible,
which is why I would prefer pin numbers over strings. Interestingly
enough the pinmux-node.yaml schema allows the use of numbers instead
of strings for the "pins" property. But neither U-Boot nor Linux
seems to implement support for this in its generic pinctrl/pinmux
code.
However, the main reason for me to pick the "pinmux" scheme used by
STM32 is to make things a bit more future proof. We expect Apple to
come out with new models and new SoCs on a regular basis and there is
some evidence that Apple tends to re-uses hardware blocks between SoC
generations. So we hope that it will be possible to add support for
new Apple hardware by simply providing new device trees in m1n1 (the
bootloader developed by Asahi). Then all we have to do is update m1n1
with a new device tree and people can simply use an existing distro
without having to wait until the latest kernel becomes available for
their distro. My feeling is that this is a bit easier to achieve
using the bitstuffing/fiddling approach, although with some wisely
chosen names for the alternate functions, the string-based approach
might work as well.
> 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.
I do see the elegance of this approach.
> 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.
Right. So far it seems we don't need to set the electrical config on
any of the pins and I don't think we've identified any bits in the
register that change the electrical configuration. Although there are
some hints in the Apple device tree that there are pins that need some
additional configuration. But I also see some evidence that the Apple
firmware on these devices already sets up most of the pins for us.
> 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.
Agreed. The plan was that when we get at the point where we need to
fiddle with the electrical configuration of pins, we simply add a
reference to pincfg-node.yaml to the schema.
> Sadly we have something like three different ways to do
> pin control device tree, as a result of failure to find consensus.
Thank you for giving quite an extensive explanation of the options. I
think all things considered, I am still happy with using the "pinmux"
property, but if Hector or Sven think the "pin"/"function" approach
with strings is better, I can change it. Implementing that approach
in U-Boot isn't a huge burden either. In fact that was pretty much
what my initial implementation based on the bindings used by Corellium
did.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-09 9:46 ` [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl Mark Kettenis
@ 2021-05-09 10:49 ` Linus Walleij
-1 siblings, 0 replies; 38+ messages in thread
From: Linus Walleij @ 2021-05-09 10:49 UTC (permalink / raw)
To: Mark Kettenis
Cc: kettenis,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel
Hi Mark,
I think we settle with this scheme you choose, so the following
is just some hints for the future.
On Sun, May 9, 2021 at 11:46 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> Right. So far it seems we don't need to set the electrical config on
> any of the pins and I don't think we've identified any bits in the
> register that change the electrical configuration. Although there are
> some hints in the Apple device tree that there are pins that need some
> additional configuration. But I also see some evidence that the Apple
> firmware on these devices already sets up most of the pins for us.
This is something I think you want to look closer at soon.
It will *probably* be necessary to obtain optimal power management
for the devices, but I am not sure.
What operating systems (especially Android devices under Linux)
tend to do is to reconfigure pins at runtime in order to conserve
power. When the device goes to deep sleep just waiting for an
external interrupt (such as opening the lid on a laptop or pressing
the power button on a phone) the OS tend to reconfigure the
pins into a low power state for the duration of the sleep.
A typical example is to set a bunch of lines as "floating" (tristate)
in pin control terminology "bias-high-impedance". In some cases
pins may be connected to ground depending on use case,
but in each case this is done to avoid leak currents when
sleeping.
I don't know how noticeable impact this will have on standby times,
but the lower micron silicon technology, the bigger the impact.
If possible I'd recommend that you try to intercept what MacOS
is doing with these registers when the system goes in/out of
sleep mode. I think it could be key to some power savings you
would otherwise miss.
These two modes are standardized in pin control terms as
"default" and "sleep" and each affected device driver need
to actively put the pin control state to "sleep" when the
device is going to sleep, so the philosophy is entirely
distributed, in difference from some more sledgehammer-type
OS approaches where the OS is just slamming a number
of values into the pin registers at sleep (suspend) time.
The upside to doing this fine-grained and per device is that
the sleep modes can in best case be used at runtime
to e.g. save power on an unused USB port that isn't plugged
to anything (I don't know if this is a good example).
In Linux we have the runtime PM concept to deal with
this and there are some drivers actively saving power in
this way.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-09 10:49 ` Linus Walleij
0 siblings, 0 replies; 38+ messages in thread
From: Linus Walleij @ 2021-05-09 10:49 UTC (permalink / raw)
To: Mark Kettenis
Cc: kettenis,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Rob Herring, Linux ARM, open list:GPIO SUBSYSTEM,
linux-kernel
Hi Mark,
I think we settle with this scheme you choose, so the following
is just some hints for the future.
On Sun, May 9, 2021 at 11:46 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> Right. So far it seems we don't need to set the electrical config on
> any of the pins and I don't think we've identified any bits in the
> register that change the electrical configuration. Although there are
> some hints in the Apple device tree that there are pins that need some
> additional configuration. But I also see some evidence that the Apple
> firmware on these devices already sets up most of the pins for us.
This is something I think you want to look closer at soon.
It will *probably* be necessary to obtain optimal power management
for the devices, but I am not sure.
What operating systems (especially Android devices under Linux)
tend to do is to reconfigure pins at runtime in order to conserve
power. When the device goes to deep sleep just waiting for an
external interrupt (such as opening the lid on a laptop or pressing
the power button on a phone) the OS tend to reconfigure the
pins into a low power state for the duration of the sleep.
A typical example is to set a bunch of lines as "floating" (tristate)
in pin control terminology "bias-high-impedance". In some cases
pins may be connected to ground depending on use case,
but in each case this is done to avoid leak currents when
sleeping.
I don't know how noticeable impact this will have on standby times,
but the lower micron silicon technology, the bigger the impact.
If possible I'd recommend that you try to intercept what MacOS
is doing with these registers when the system goes in/out of
sleep mode. I think it could be key to some power savings you
would otherwise miss.
These two modes are standardized in pin control terms as
"default" and "sleep" and each affected device driver need
to actively put the pin control state to "sleep" when the
device is going to sleep, so the philosophy is entirely
distributed, in difference from some more sledgehammer-type
OS approaches where the OS is just slamming a number
of values into the pin registers at sleep (suspend) time.
The upside to doing this fine-grained and per device is that
the sleep modes can in best case be used at runtime
to e.g. save power on an unused USB port that isn't plugged
to anything (I don't know if this is a good example).
In Linux we have the runtime PM concept to deal with
this and there are some drivers actively saving power in
this way.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-08 14:19 ` Mark Kettenis
@ 2021-05-10 13:03 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 38+ messages in thread
From: Krzysztof Kozlowski @ 2021-05-10 13:03 UTC (permalink / raw)
To: Mark Kettenis, devicetree
Cc: Hector Martin, Linus Walleij, Rob Herring, linux-arm-kernel,
linux-gpio, linux-kernel
On 08/05/2021 10:19, Mark Kettenis 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>
> ---
> .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/pinctrl/apple.h | 13 +++
> 3 files changed, 118 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> create mode 100644 include/dt-bindings/pinctrl/apple.h
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> new file mode 100644
> index 000000000000..cc7805ca6ba1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/apple,pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple GPIO controller
> +
> +maintainers:
> + - Mark Kettenis <kettenis@openbsd.org>
> +
> +description: |
> + 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.
> +
> +properties:
> + compatible:
> + items:
> + - const: apple,t8103-pinctrl
> + - const: apple,pinctrl
What is the point of having very generic final compatible in the binding
which does not relate to actual hardware?
Let's say next SoC will be
apple,x-abcd-foo-2323-whatever-nothing-in-common and you still have to
use generic "apple,pinctrl" even though HW is not at all compatible?
This looks like wildcard, not HW description.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
@ 2021-05-10 13:03 ` Krzysztof Kozlowski
0 siblings, 0 replies; 38+ messages in thread
From: Krzysztof Kozlowski @ 2021-05-10 13:03 UTC (permalink / raw)
To: Mark Kettenis, devicetree
Cc: Hector Martin, Linus Walleij, Rob Herring, linux-arm-kernel,
linux-gpio, linux-kernel
On 08/05/2021 10:19, Mark Kettenis 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>
> ---
> .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/pinctrl/apple.h | 13 +++
> 3 files changed, 118 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> create mode 100644 include/dt-bindings/pinctrl/apple.h
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> new file mode 100644
> index 000000000000..cc7805ca6ba1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/apple,pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple GPIO controller
> +
> +maintainers:
> + - Mark Kettenis <kettenis@openbsd.org>
> +
> +description: |
> + 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.
> +
> +properties:
> + compatible:
> + items:
> + - const: apple,t8103-pinctrl
> + - const: apple,pinctrl
What is the point of having very generic final compatible in the binding
which does not relate to actual hardware?
Let's say next SoC will be
apple,x-abcd-foo-2323-whatever-nothing-in-common and you still have to
use generic "apple,pinctrl" even though HW is not at all compatible?
This looks like wildcard, not HW description.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-10 13:03 ` Krzysztof Kozlowski
@ 2021-05-10 17:09 ` Mark Kettenis
-1 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-10 17:09 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: kettenis, devicetree, marcan, linus.walleij, robh+dt,
linux-arm-kernel, linux-gpio, linux-kernel
> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Date: Mon, 10 May 2021 09:03:13 -0400
>
> On 08/05/2021 10:19, Mark Kettenis 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>
> > ---
> > .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> > MAINTAINERS | 2 +
> > include/dt-bindings/pinctrl/apple.h | 13 +++
> > 3 files changed, 118 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > create mode 100644 include/dt-bindings/pinctrl/apple.h
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > new file mode 100644
> > index 000000000000..cc7805ca6ba1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > @@ -0,0 +1,103 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/apple,pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Apple GPIO controller
> > +
> > +maintainers:
> > + - Mark Kettenis <kettenis@openbsd.org>
> > +
> > +description: |
> > + 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.
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - const: apple,t8103-pinctrl
> > + - const: apple,pinctrl
>
> What is the point of having very generic final compatible in the binding
> which does not relate to actual hardware?
>
> Let's say next SoC will be
> apple,x-abcd-foo-2323-whatever-nothing-in-common and you still have to
> use generic "apple,pinctrl" even though HW is not at all compatible?
> This looks like wildcard, not HW description.
Hi Krzysztof,
See my reply to Rob's mail. We have some confidence that Apple isn't
changing their GPIO block very often. If they were to change it in an
incompatible way in a future SoC, we'd drop the "apple,pinctrl"
compatible of course.
Thanks,
Mark
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
@ 2021-05-10 17:09 ` Mark Kettenis
0 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-10 17:09 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: kettenis, devicetree, marcan, linus.walleij, robh+dt,
linux-arm-kernel, linux-gpio, linux-kernel
> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Date: Mon, 10 May 2021 09:03:13 -0400
>
> On 08/05/2021 10:19, Mark Kettenis 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>
> > ---
> > .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> > MAINTAINERS | 2 +
> > include/dt-bindings/pinctrl/apple.h | 13 +++
> > 3 files changed, 118 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > create mode 100644 include/dt-bindings/pinctrl/apple.h
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > new file mode 100644
> > index 000000000000..cc7805ca6ba1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > @@ -0,0 +1,103 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/apple,pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Apple GPIO controller
> > +
> > +maintainers:
> > + - Mark Kettenis <kettenis@openbsd.org>
> > +
> > +description: |
> > + 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.
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - const: apple,t8103-pinctrl
> > + - const: apple,pinctrl
>
> What is the point of having very generic final compatible in the binding
> which does not relate to actual hardware?
>
> Let's say next SoC will be
> apple,x-abcd-foo-2323-whatever-nothing-in-common and you still have to
> use generic "apple,pinctrl" even though HW is not at all compatible?
> This looks like wildcard, not HW description.
Hi Krzysztof,
See my reply to Rob's mail. We have some confidence that Apple isn't
changing their GPIO block very often. If they were to change it in an
incompatible way in a future SoC, we'd drop the "apple,pinctrl"
compatible of course.
Thanks,
Mark
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-08 14:19 ` Mark Kettenis
@ 2021-05-10 14:01 ` Rob Herring
-1 siblings, 0 replies; 38+ messages in thread
From: Rob Herring @ 2021-05-10 14:01 UTC (permalink / raw)
To: Mark Kettenis
Cc: linux-arm-kernel, Linus Walleij, linux-gpio, Hector Martin,
linux-kernel, Rob Herring, devicetree
On Sat, 08 May 2021 16:19:55 +0200, Mark Kettenis 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>
> ---
> .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/pinctrl/apple.h | 13 +++
> 3 files changed, 118 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> create mode 100644 include/dt-bindings/pinctrl/apple.h
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pinctrl/apple,pinctrl.example.dts:19:18: fatal error: dt-bindings/interrupt-controller/apple-aic.h: No such file or directory
19 | #include <dt-bindings/interrupt-controller/apple-aic.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:377: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1414: dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1475875
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-10 14:01 ` Rob Herring
0 siblings, 0 replies; 38+ messages in thread
From: Rob Herring @ 2021-05-10 14:01 UTC (permalink / raw)
To: Mark Kettenis
Cc: linux-arm-kernel, Linus Walleij, linux-gpio, Hector Martin,
linux-kernel, Rob Herring, devicetree
On Sat, 08 May 2021 16:19:55 +0200, Mark Kettenis 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>
> ---
> .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/pinctrl/apple.h | 13 +++
> 3 files changed, 118 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> create mode 100644 include/dt-bindings/pinctrl/apple.h
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pinctrl/apple,pinctrl.example.dts:19:18: fatal error: dt-bindings/interrupt-controller/apple-aic.h: No such file or directory
19 | #include <dt-bindings/interrupt-controller/apple-aic.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:377: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1414: dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1475875
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-10 14:01 ` [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl Rob Herring
@ 2021-05-10 18:18 ` Rob Herring
-1 siblings, 0 replies; 38+ messages in thread
From: Rob Herring @ 2021-05-10 18:18 UTC (permalink / raw)
To: Mark Kettenis
Cc: linux-arm-kernel, Linus Walleij, open list:GPIO SUBSYSTEM,
Hector Martin, linux-kernel, devicetree
On Mon, May 10, 2021 at 9:01 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sat, 08 May 2021 16:19:55 +0200, Mark Kettenis 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>
> > ---
> > .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> > MAINTAINERS | 2 +
> > include/dt-bindings/pinctrl/apple.h | 13 +++
> > 3 files changed, 118 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > create mode 100644 include/dt-bindings/pinctrl/apple.h
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/pinctrl/apple,pinctrl.example.dts:19:18: fatal error: dt-bindings/interrupt-controller/apple-aic.h: No such file or directory
> 19 | #include <dt-bindings/interrupt-controller/apple-aic.h>
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[1]: *** [scripts/Makefile.lib:377: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.example.dt.yaml] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1414: dt_binding_check] Error 2
Ignore this. I've now updated the base to 5.13-rc1.
Rob
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-10 18:18 ` Rob Herring
0 siblings, 0 replies; 38+ messages in thread
From: Rob Herring @ 2021-05-10 18:18 UTC (permalink / raw)
To: Mark Kettenis
Cc: linux-arm-kernel, Linus Walleij, open list:GPIO SUBSYSTEM,
Hector Martin, linux-kernel, devicetree
On Mon, May 10, 2021 at 9:01 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sat, 08 May 2021 16:19:55 +0200, Mark Kettenis 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>
> > ---
> > .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> > MAINTAINERS | 2 +
> > include/dt-bindings/pinctrl/apple.h | 13 +++
> > 3 files changed, 118 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > create mode 100644 include/dt-bindings/pinctrl/apple.h
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/pinctrl/apple,pinctrl.example.dts:19:18: fatal error: dt-bindings/interrupt-controller/apple-aic.h: No such file or directory
> 19 | #include <dt-bindings/interrupt-controller/apple-aic.h>
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[1]: *** [scripts/Makefile.lib:377: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.example.dt.yaml] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1414: dt_binding_check] Error 2
Ignore this. I've now updated the base to 5.13-rc1.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-08 14:19 ` Mark Kettenis
@ 2021-05-10 14:19 ` Rob Herring
-1 siblings, 0 replies; 38+ messages in thread
From: Rob Herring @ 2021-05-10 14:19 UTC (permalink / raw)
To: Mark Kettenis
Cc: devicetree, Hector Martin, Linus Walleij, linux-arm-kernel,
linux-gpio, linux-kernel
On Sat, May 08, 2021 at 04:19:55PM +0200, Mark Kettenis 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>
> ---
> .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/pinctrl/apple.h | 13 +++
> 3 files changed, 118 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> create mode 100644 include/dt-bindings/pinctrl/apple.h
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> new file mode 100644
> index 000000000000..cc7805ca6ba1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/apple,pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple GPIO controller
> +
> +maintainers:
> + - Mark Kettenis <kettenis@openbsd.org>
> +
> +description: |
> + 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.
> +
> +properties:
> + compatible:
> + items:
> + - const: apple,t8103-pinctrl
> + - const: apple,pinctrl
A genericish fallback is maybe questionable for pinctrl. That's not
often the same from one SoC to the next.
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + gpio-controller: true
> +
> + '#gpio-cells':
> + const: 2
> +
> + gpio-ranges:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 7
> +
> + interrupt-controller: true
> +
> +patternProperties:
> + '-pins$':
> + type: object
> + $ref: pinmux-node.yaml#
> +
> + properties:
> + pinmux:
> + description:
> + Values are constructed from pin number and alternate function
> + configuration number using the APPLE_PINMUX() helper macro
> + defined in include/dt-bindings/pinctrl/apple.h.
> +
> + required:
> + - pinmux
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - gpio-controller
> + - '#gpio-cells'
> + - gpio-ranges
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/apple-aic.h>
> + #include <dt-bindings/pinctrl/apple.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + pinctrl: pinctrl@23c100000 {
> + compatible = "apple,t8103-pinctrl", "apple,pinctrl";
> + reg = <0x2 0x3c100000 0x0 0x100000>;
> + clocks = <&gpio_clk>;
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 0 212>;
> +
> + 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>;
> +
> + pcie_pins: pcie-pins {
> + pinmux = <APPLE_PINMUX(150, 1)>,
> + <APPLE_PINMUX(151, 1)>,
> + <APPLE_PINMUX(32, 1)>;
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ad0e9be66885..7327c9b778f1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1654,9 +1654,11 @@ C: irc://chat.freenode.net/asahi-dev
> T: git https://github.com/AsahiLinux/linux.git
> F: Documentation/devicetree/bindings/arm/apple.yaml
> F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> +F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> F: arch/arm64/boot/dts/apple/
> F: drivers/irqchip/irq-apple-aic.c
> F: include/dt-bindings/interrupt-controller/apple-aic.h
> +F: include/dt-bindings/pinctrl/apple.h
>
> ARM/ARTPEC MACHINE SUPPORT
> M: Jesper Nilsson <jesper.nilsson@axis.com>
> diff --git a/include/dt-bindings/pinctrl/apple.h b/include/dt-bindings/pinctrl/apple.h
> new file mode 100644
> index 000000000000..ea0a6f466592
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/apple.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0+ OR MIT */
> +/*
> + * This header provides constants for Apple pinctrl bindings.
> + */
> +
> +#ifndef _DT_BINDINGS_PINCTRL_APPLE_H
> +#define _DT_BINDINGS_PINCTRL_APPLE_H
> +
> +#define APPLE_PINMUX(pin, func) ((pin) | ((func) << 16))
> +#define APPLE_PIN(pinmux) ((pinmux) & 0xffff)
> +#define APPLE_FUNC(pinmux) ((pinmux) >> 16)
> +
> +#endif /* _DT_BINDINGS_PINCTRL_APPLE_H */
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
@ 2021-05-10 14:19 ` Rob Herring
0 siblings, 0 replies; 38+ messages in thread
From: Rob Herring @ 2021-05-10 14:19 UTC (permalink / raw)
To: Mark Kettenis
Cc: devicetree, Hector Martin, Linus Walleij, linux-arm-kernel,
linux-gpio, linux-kernel
On Sat, May 08, 2021 at 04:19:55PM +0200, Mark Kettenis 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>
> ---
> .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/pinctrl/apple.h | 13 +++
> 3 files changed, 118 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> create mode 100644 include/dt-bindings/pinctrl/apple.h
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> new file mode 100644
> index 000000000000..cc7805ca6ba1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/apple,pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple GPIO controller
> +
> +maintainers:
> + - Mark Kettenis <kettenis@openbsd.org>
> +
> +description: |
> + 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.
> +
> +properties:
> + compatible:
> + items:
> + - const: apple,t8103-pinctrl
> + - const: apple,pinctrl
A genericish fallback is maybe questionable for pinctrl. That's not
often the same from one SoC to the next.
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + gpio-controller: true
> +
> + '#gpio-cells':
> + const: 2
> +
> + gpio-ranges:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 7
> +
> + interrupt-controller: true
> +
> +patternProperties:
> + '-pins$':
> + type: object
> + $ref: pinmux-node.yaml#
> +
> + properties:
> + pinmux:
> + description:
> + Values are constructed from pin number and alternate function
> + configuration number using the APPLE_PINMUX() helper macro
> + defined in include/dt-bindings/pinctrl/apple.h.
> +
> + required:
> + - pinmux
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - gpio-controller
> + - '#gpio-cells'
> + - gpio-ranges
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/apple-aic.h>
> + #include <dt-bindings/pinctrl/apple.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + pinctrl: pinctrl@23c100000 {
> + compatible = "apple,t8103-pinctrl", "apple,pinctrl";
> + reg = <0x2 0x3c100000 0x0 0x100000>;
> + clocks = <&gpio_clk>;
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 0 212>;
> +
> + 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>;
> +
> + pcie_pins: pcie-pins {
> + pinmux = <APPLE_PINMUX(150, 1)>,
> + <APPLE_PINMUX(151, 1)>,
> + <APPLE_PINMUX(32, 1)>;
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ad0e9be66885..7327c9b778f1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1654,9 +1654,11 @@ C: irc://chat.freenode.net/asahi-dev
> T: git https://github.com/AsahiLinux/linux.git
> F: Documentation/devicetree/bindings/arm/apple.yaml
> F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> +F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> F: arch/arm64/boot/dts/apple/
> F: drivers/irqchip/irq-apple-aic.c
> F: include/dt-bindings/interrupt-controller/apple-aic.h
> +F: include/dt-bindings/pinctrl/apple.h
>
> ARM/ARTPEC MACHINE SUPPORT
> M: Jesper Nilsson <jesper.nilsson@axis.com>
> diff --git a/include/dt-bindings/pinctrl/apple.h b/include/dt-bindings/pinctrl/apple.h
> new file mode 100644
> index 000000000000..ea0a6f466592
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/apple.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0+ OR MIT */
> +/*
> + * This header provides constants for Apple pinctrl bindings.
> + */
> +
> +#ifndef _DT_BINDINGS_PINCTRL_APPLE_H
> +#define _DT_BINDINGS_PINCTRL_APPLE_H
> +
> +#define APPLE_PINMUX(pin, func) ((pin) | ((func) << 16))
> +#define APPLE_PIN(pinmux) ((pinmux) & 0xffff)
> +#define APPLE_FUNC(pinmux) ((pinmux) >> 16)
> +
> +#endif /* _DT_BINDINGS_PINCTRL_APPLE_H */
> --
> 2.31.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-10 14:19 ` Rob Herring
@ 2021-05-10 17:06 ` Mark Kettenis
-1 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-10 17:06 UTC (permalink / raw)
To: Rob Herring
Cc: kettenis, devicetree, marcan, linus.walleij, linux-arm-kernel,
linux-gpio, linux-kernel
> Date: Mon, 10 May 2021 09:19:55 -0500
> From: Rob Herring <robh@kernel.org>
Hi Rob,
> On Sat, May 08, 2021 at 04:19:55PM +0200, Mark Kettenis 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>
> > ---
> > .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> > MAINTAINERS | 2 +
> > include/dt-bindings/pinctrl/apple.h | 13 +++
> > 3 files changed, 118 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > create mode 100644 include/dt-bindings/pinctrl/apple.h
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > new file mode 100644
> > index 000000000000..cc7805ca6ba1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > @@ -0,0 +1,103 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/apple,pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Apple GPIO controller
> > +
> > +maintainers:
> > + - Mark Kettenis <kettenis@openbsd.org>
> > +
> > +description: |
> > + 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.
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - const: apple,t8103-pinctrl
> > + - const: apple,pinctrl
>
> A genericish fallback is maybe questionable for pinctrl. That's not
> often the same from one SoC to the next.
Krzysztof raised a similar point. It seems that Apple isn't in the
habit of changing this aspect of their SoCs. Judging from
https://github.com/corellium/linux-sandcastle/blob/sandcastle-5.4/drivers/pinctrl/pinctrl-hx-gpio.c
Apple has been using a controller with the same register layout and
the same known bits in those registers since the T8010 SoC (the A10
used in the iPhone 7). So there is some confidence that a single
driver will work for many generations of the SoC. Of course we don't
have any authoritative documentaton on these SoCs so I can't be 100%
sure that there aren't any subtle differences in the behaviour of the
hardware. That's why I also include a more specific compatible for
the T8103 SoC (the M1 used in the current Macs).
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + gpio-controller: true
> > +
> > + '#gpio-cells':
> > + const: 2
> > +
> > + gpio-ranges:
> > + maxItems: 1
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 7
> > +
> > + interrupt-controller: true
> > +
> > +patternProperties:
> > + '-pins$':
> > + type: object
> > + $ref: pinmux-node.yaml#
> > +
> > + properties:
> > + pinmux:
> > + description:
> > + Values are constructed from pin number and alternate function
> > + configuration number using the APPLE_PINMUX() helper macro
> > + defined in include/dt-bindings/pinctrl/apple.h.
> > +
> > + required:
> > + - pinmux
> > +
> > + additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - gpio-controller
> > + - '#gpio-cells'
> > + - gpio-ranges
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/apple-aic.h>
> > + #include <dt-bindings/pinctrl/apple.h>
> > +
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + pinctrl: pinctrl@23c100000 {
> > + compatible = "apple,t8103-pinctrl", "apple,pinctrl";
> > + reg = <0x2 0x3c100000 0x0 0x100000>;
> > + clocks = <&gpio_clk>;
> > +
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + gpio-ranges = <&pinctrl 0 0 212>;
> > +
> > + 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>;
> > +
> > + pcie_pins: pcie-pins {
> > + pinmux = <APPLE_PINMUX(150, 1)>,
> > + <APPLE_PINMUX(151, 1)>,
> > + <APPLE_PINMUX(32, 1)>;
> > + };
> > + };
> > + };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ad0e9be66885..7327c9b778f1 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1654,9 +1654,11 @@ C: irc://chat.freenode.net/asahi-dev
> > T: git https://github.com/AsahiLinux/linux.git
> > F: Documentation/devicetree/bindings/arm/apple.yaml
> > F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> > +F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > F: arch/arm64/boot/dts/apple/
> > F: drivers/irqchip/irq-apple-aic.c
> > F: include/dt-bindings/interrupt-controller/apple-aic.h
> > +F: include/dt-bindings/pinctrl/apple.h
> >
> > ARM/ARTPEC MACHINE SUPPORT
> > M: Jesper Nilsson <jesper.nilsson@axis.com>
> > diff --git a/include/dt-bindings/pinctrl/apple.h b/include/dt-bindings/pinctrl/apple.h
> > new file mode 100644
> > index 000000000000..ea0a6f466592
> > --- /dev/null
> > +++ b/include/dt-bindings/pinctrl/apple.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ OR MIT */
> > +/*
> > + * This header provides constants for Apple pinctrl bindings.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_PINCTRL_APPLE_H
> > +#define _DT_BINDINGS_PINCTRL_APPLE_H
> > +
> > +#define APPLE_PINMUX(pin, func) ((pin) | ((func) << 16))
> > +#define APPLE_PIN(pinmux) ((pinmux) & 0xffff)
> > +#define APPLE_FUNC(pinmux) ((pinmux) >> 16)
> > +
> > +#endif /* _DT_BINDINGS_PINCTRL_APPLE_H */
> > --
> > 2.31.1
> >
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
@ 2021-05-10 17:06 ` Mark Kettenis
0 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-10 17:06 UTC (permalink / raw)
To: Rob Herring
Cc: kettenis, devicetree, marcan, linus.walleij, linux-arm-kernel,
linux-gpio, linux-kernel
> Date: Mon, 10 May 2021 09:19:55 -0500
> From: Rob Herring <robh@kernel.org>
Hi Rob,
> On Sat, May 08, 2021 at 04:19:55PM +0200, Mark Kettenis 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>
> > ---
> > .../bindings/pinctrl/apple,pinctrl.yaml | 103 ++++++++++++++++++
> > MAINTAINERS | 2 +
> > include/dt-bindings/pinctrl/apple.h | 13 +++
> > 3 files changed, 118 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > create mode 100644 include/dt-bindings/pinctrl/apple.h
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > new file mode 100644
> > index 000000000000..cc7805ca6ba1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > @@ -0,0 +1,103 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/apple,pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Apple GPIO controller
> > +
> > +maintainers:
> > + - Mark Kettenis <kettenis@openbsd.org>
> > +
> > +description: |
> > + 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.
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - const: apple,t8103-pinctrl
> > + - const: apple,pinctrl
>
> A genericish fallback is maybe questionable for pinctrl. That's not
> often the same from one SoC to the next.
Krzysztof raised a similar point. It seems that Apple isn't in the
habit of changing this aspect of their SoCs. Judging from
https://github.com/corellium/linux-sandcastle/blob/sandcastle-5.4/drivers/pinctrl/pinctrl-hx-gpio.c
Apple has been using a controller with the same register layout and
the same known bits in those registers since the T8010 SoC (the A10
used in the iPhone 7). So there is some confidence that a single
driver will work for many generations of the SoC. Of course we don't
have any authoritative documentaton on these SoCs so I can't be 100%
sure that there aren't any subtle differences in the behaviour of the
hardware. That's why I also include a more specific compatible for
the T8103 SoC (the M1 used in the current Macs).
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + gpio-controller: true
> > +
> > + '#gpio-cells':
> > + const: 2
> > +
> > + gpio-ranges:
> > + maxItems: 1
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 7
> > +
> > + interrupt-controller: true
> > +
> > +patternProperties:
> > + '-pins$':
> > + type: object
> > + $ref: pinmux-node.yaml#
> > +
> > + properties:
> > + pinmux:
> > + description:
> > + Values are constructed from pin number and alternate function
> > + configuration number using the APPLE_PINMUX() helper macro
> > + defined in include/dt-bindings/pinctrl/apple.h.
> > +
> > + required:
> > + - pinmux
> > +
> > + additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - gpio-controller
> > + - '#gpio-cells'
> > + - gpio-ranges
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/apple-aic.h>
> > + #include <dt-bindings/pinctrl/apple.h>
> > +
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + pinctrl: pinctrl@23c100000 {
> > + compatible = "apple,t8103-pinctrl", "apple,pinctrl";
> > + reg = <0x2 0x3c100000 0x0 0x100000>;
> > + clocks = <&gpio_clk>;
> > +
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + gpio-ranges = <&pinctrl 0 0 212>;
> > +
> > + 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>;
> > +
> > + pcie_pins: pcie-pins {
> > + pinmux = <APPLE_PINMUX(150, 1)>,
> > + <APPLE_PINMUX(151, 1)>,
> > + <APPLE_PINMUX(32, 1)>;
> > + };
> > + };
> > + };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ad0e9be66885..7327c9b778f1 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1654,9 +1654,11 @@ C: irc://chat.freenode.net/asahi-dev
> > T: git https://github.com/AsahiLinux/linux.git
> > F: Documentation/devicetree/bindings/arm/apple.yaml
> > F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> > +F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> > F: arch/arm64/boot/dts/apple/
> > F: drivers/irqchip/irq-apple-aic.c
> > F: include/dt-bindings/interrupt-controller/apple-aic.h
> > +F: include/dt-bindings/pinctrl/apple.h
> >
> > ARM/ARTPEC MACHINE SUPPORT
> > M: Jesper Nilsson <jesper.nilsson@axis.com>
> > diff --git a/include/dt-bindings/pinctrl/apple.h b/include/dt-bindings/pinctrl/apple.h
> > new file mode 100644
> > index 000000000000..ea0a6f466592
> > --- /dev/null
> > +++ b/include/dt-bindings/pinctrl/apple.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ OR MIT */
> > +/*
> > + * This header provides constants for Apple pinctrl bindings.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_PINCTRL_APPLE_H
> > +#define _DT_BINDINGS_PINCTRL_APPLE_H
> > +
> > +#define APPLE_PINMUX(pin, func) ((pin) | ((func) << 16))
> > +#define APPLE_PIN(pinmux) ((pinmux) & 0xffff)
> > +#define APPLE_FUNC(pinmux) ((pinmux) >> 16)
> > +
> > +#endif /* _DT_BINDINGS_PINCTRL_APPLE_H */
> > --
> > 2.31.1
> >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-10 17:06 ` Mark Kettenis
@ 2021-05-19 23:27 ` Linus Walleij
-1 siblings, 0 replies; 38+ messages in thread
From: Linus Walleij @ 2021-05-19 23:27 UTC (permalink / raw)
To: Mark Kettenis
Cc: Rob Herring, kettenis,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Linux ARM, open list:GPIO SUBSYSTEM, linux-kernel
On Mon, May 10, 2021 at 7:06 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > From: Rob Herring <robh@kernel.org>
> > > +properties:
> > > + compatible:
> > > + items:
> > > + - const: apple,t8103-pinctrl
> > > + - const: apple,pinctrl
> >
> > A genericish fallback is maybe questionable for pinctrl. That's not
> > often the same from one SoC to the next.
>
> Krzysztof raised a similar point. It seems that Apple isn't in the
> habit of changing this aspect of their SoCs.
Rob what's your stance on this? Does it need to be changed?
Else I'll apply the patch.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-19 23:27 ` Linus Walleij
0 siblings, 0 replies; 38+ messages in thread
From: Linus Walleij @ 2021-05-19 23:27 UTC (permalink / raw)
To: Mark Kettenis
Cc: Rob Herring, kettenis,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hector Martin, Linux ARM, open list:GPIO SUBSYSTEM, linux-kernel
On Mon, May 10, 2021 at 7:06 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > From: Rob Herring <robh@kernel.org>
> > > +properties:
> > > + compatible:
> > > + items:
> > > + - const: apple,t8103-pinctrl
> > > + - const: apple,pinctrl
> >
> > A genericish fallback is maybe questionable for pinctrl. That's not
> > often the same from one SoC to the next.
>
> Krzysztof raised a similar point. It seems that Apple isn't in the
> habit of changing this aspect of their SoCs.
Rob what's your stance on this? Does it need to be changed?
Else I'll apply the patch.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple,pinctrl
2021-05-19 23:27 ` [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl Linus Walleij
@ 2021-05-20 11:22 ` Mark Kettenis
-1 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-20 11:22 UTC (permalink / raw)
To: Linus Walleij
Cc: robh, kettenis, devicetree, marcan, linux-arm-kernel, linux-gpio,
linux-kernel
> From: Linus Walleij <linus.walleij@linaro.org>
> Date: Thu, 20 May 2021 01:27:37 +0200
>
> On Mon, May 10, 2021 at 7:06 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > > From: Rob Herring <robh@kernel.org>
>
> > > > +properties:
> > > > + compatible:
> > > > + items:
> > > > + - const: apple,t8103-pinctrl
> > > > + - const: apple,pinctrl
> > >
> > > A genericish fallback is maybe questionable for pinctrl. That's not
> > > often the same from one SoC to the next.
> >
> > Krzysztof raised a similar point. It seems that Apple isn't in the
> > habit of changing this aspect of their SoCs.
>
> Rob what's your stance on this? Does it need to be changed?
> Else I'll apply the patch.
Hi Linus,
Rob asked me to provide a description for the interrupts in response
to the v2 I sent a few days ago:
http://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210516183221.93686-2-mark.kettenis@xs4all.nl/
I'll roll a v3 later today for that.
Cheers,
Mark
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add DT bindings for apple, pinctrl
@ 2021-05-20 11:22 ` Mark Kettenis
0 siblings, 0 replies; 38+ messages in thread
From: Mark Kettenis @ 2021-05-20 11:22 UTC (permalink / raw)
To: Linus Walleij
Cc: robh, kettenis, devicetree, marcan, linux-arm-kernel, linux-gpio,
linux-kernel
> From: Linus Walleij <linus.walleij@linaro.org>
> Date: Thu, 20 May 2021 01:27:37 +0200
>
> On Mon, May 10, 2021 at 7:06 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > > From: Rob Herring <robh@kernel.org>
>
> > > > +properties:
> > > > + compatible:
> > > > + items:
> > > > + - const: apple,t8103-pinctrl
> > > > + - const: apple,pinctrl
> > >
> > > A genericish fallback is maybe questionable for pinctrl. That's not
> > > often the same from one SoC to the next.
> >
> > Krzysztof raised a similar point. It seems that Apple isn't in the
> > habit of changing this aspect of their SoCs.
>
> Rob what's your stance on this? Does it need to be changed?
> Else I'll apply the patch.
Hi Linus,
Rob asked me to provide a description for the interrupts in response
to the v2 I sent a few days ago:
http://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210516183221.93686-2-mark.kettenis@xs4all.nl/
I'll roll a v3 later today for that.
Cheers,
Mark
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 38+ messages in thread