diff mbox series

[v3,1/3] dt-bindings: spi: apple,spi: Add binding for Apple SPI controllers

Message ID 20241101-asahi-spi-v3-1-3b411c5fb8e5@jannau.net (mailing list archive)
State Superseded
Headers show
Series Apple SPI controller driver | expand

Commit Message

Janne Grunau via B4 Relay Nov. 1, 2024, 7:26 p.m. UTC
From: Hector Martin <marcan@marcan.st>

The Apple SPI controller is present in SoCs such as the M1 (t8103) and
M1 Pro/Max (t600x). This controller uses one IRQ and one clock, and
doesn't need any special properties, so the binding is trivial.

Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Janne Grunau <j@jannau.net>
---
 .../devicetree/bindings/spi/apple,spi.yaml         | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)

Comments

Nick Chan Nov. 2, 2024, 2:36 a.m. UTC | #1
On 2/11/2024 03:26, Janne Grunau via B4 Relay wrote:

[...]
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - apple,t8103-spi
> +          - apple,t8112-spi
> +          - apple,t6000-spi
> +      - const: apple,spi
Apple A7-A11 SoCs seems to use a Samsung SPI block, so apple,spi is too
generic. Fallback to something like apple,t8103-spi instead.

[...]

Nick Chan
Janne Grunau Nov. 2, 2024, 8:36 a.m. UTC | #2
On Sat, Nov 02, 2024 at 10:36:56AM +0800, Nick Chan wrote:
> 
> 
> On 2/11/2024 03:26, Janne Grunau via B4 Relay wrote:
> 
> [...]
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - apple,t8103-spi
> > +          - apple,t8112-spi
> > +          - apple,t6000-spi
> > +      - const: apple,spi
> Apple A7-A11 SoCs seems to use a Samsung SPI block, so apple,spi is too
> generic. Fallback to something like apple,t8103-spi instead.

Have you looked at it? This one still seems to be somehow Samsung
related. See the previous discussion at
https://lore.kernel.org/linux-devicetree/d87ae109-4b58-7465-b16e-3bf7c9d60f1f@marcan.st/

Even the A7-A11 SPI controllers are not compatible "apple,spi" doesn't
prevent us from using something like "apple,s5l-spi" for those.

Janne
Mark Kettenis Nov. 2, 2024, 8:39 a.m. UTC | #3
> Date: Sat, 2 Nov 2024 10:36:56 +0800
> Content-Language: en-MW
> 
> On 2/11/2024 03:26, Janne Grunau via B4 Relay wrote:
> 
> [...]
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - apple,t8103-spi
> > +          - apple,t8112-spi
> > +          - apple,t6000-spi
> > +      - const: apple,spi
> Apple A7-A11 SoCs seems to use a Samsung SPI block, so apple,spi is too
> generic. Fallback to something like apple,t8103-spi instead.

Well, if that is a Samsung SPI block it probably should have a
"generic" compatible that starts with "samsung,".  The M1/M2
controllers have a different SPI block (presumably) designed by Apple
themselves.  So I think it is (still) appropriate that that one is
"apple,spi".

Also, (upstream) U-Boot already uses the "apple,spi" compatible.  So
changing it for purity sake just causes pain.
Nick Chan Nov. 2, 2024, 1:15 p.m. UTC | #4
On 2/11/2024 16:39, Mark Kettenis wrote:
>> Date: Sat, 2 Nov 2024 10:36:56 +0800
>> Content-Language: en-MW
>>
>> On 2/11/2024 03:26, Janne Grunau via B4 Relay wrote:
>>
>> [...]
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - apple,t8103-spi
>>> +          - apple,t8112-spi
>>> +          - apple,t6000-spi
>>> +      - const: apple,spi
>> Apple A7-A11 SoCs seems to use a Samsung SPI block, so apple,spi is too
>> generic. Fallback to something like apple,t8103-spi instead.
> 
> Well, if that is a Samsung SPI block it probably should have a
> "generic" compatible that starts with "samsung,".  The M1/M2
> controllers have a different SPI block (presumably) designed by Apple
> themselves.  So I think it is (still) appropriate that that one is
> "apple,spi".
I just looked into the SPI on A7-A11 SoC in more detail instead of just
going off the ADT compatible. It seems a very big chunk of the registers
offsets and bits seems to be the same as the ones in M1. So, feel free to
ignore my comment above.

Acked-by: Nick Chan <towinchenmi@gmail.com>

> 
> Also, (upstream) U-Boot already uses the "apple,spi" compatible.  So
> changing it for purity sake just causes pain.
Well, if upstream U-Boot is using it, then I agree that "apple,spi"
should continue to be used.

> 

Nick Chan
Conor Dooley Nov. 4, 2024, 6:56 p.m. UTC | #5
On Fri, Nov 01, 2024 at 08:26:12PM +0100, Janne Grunau via B4 Relay wrote:
> +      spi: spi@39b104000 {

nit: the label here serves no purpose, and could be dropped. No need to
respin for that obviously.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.
Janne Grunau Nov. 5, 2024, 7:46 a.m. UTC | #6
On Mon, Nov 04, 2024 at 06:56:59PM +0000, Conor Dooley wrote:
> On Fri, Nov 01, 2024 at 08:26:12PM +0100, Janne Grunau via B4 Relay wrote:
> > +      spi: spi@39b104000 {
> 
> nit: the label here serves no purpose, and could be dropped. No need to
> respin for that obviously.

removed since there will be a respin for minor driver changes

> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

thanks
Janne
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/apple,spi.yaml b/Documentation/devicetree/bindings/spi/apple,spi.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..8b4f974faa23bcf9a1ea0a0eb52ad17ca196b341
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/apple,spi.yaml
@@ -0,0 +1,62 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/apple,spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple ARM SoC SPI controller
+
+allOf:
+  - $ref: spi-controller.yaml#
+
+maintainers:
+  - Hector Martin <marcan@marcan.st>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - apple,t8103-spi
+          - apple,t8112-spi
+          - apple,t6000-spi
+      - const: apple,spi
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/apple-aic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      spi: spi@39b104000 {
+        compatible = "apple,t6000-spi", "apple,spi";
+        reg = <0x3 0x9b104000 0x0 0x4000>;
+        interrupt-parent = <&aic>;
+        interrupts = <AIC_IRQ 0 1107 IRQ_TYPE_LEVEL_HIGH>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        clocks = <&clk>;
+      };
+    };