| Message ID | 20241101-asahi-spi-v3-1-3b411c5fb8e5@jannau.net |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Apple SPI controller driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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
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
> 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.
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
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.
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 --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>; + }; + };