| Message ID | 20241126-z2-v1-3-c43c4cc6200d@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Driver for Apple Z2 touchscreens. | expand |
On Tue, Nov 26, 2024 at 09:48:01PM +0100, Sasha Finkelstein via B4 Relay wrote: > From: Sasha Finkelstein <fnkl.kernel@gmail.com> > > Adds device tree entries for the touchbar digitizer > > Co-developed-by: Janne Grunau <j@jannau.net> > Signed-off-by: Janne Grunau <j@jannau.net> > Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com> > --- > arch/arm64/boot/dts/apple/t8103-j293.dts | 24 ++++++++++++++++++++++++ > arch/arm64/boot/dts/apple/t8103.dtsi | 19 +++++++++++++++++++ > arch/arm64/boot/dts/apple/t8112-j493.dts | 20 ++++++++++++++++++++ > arch/arm64/boot/dts/apple/t8112.dtsi | 14 ++++++++++++++ The changes in t8103.dtsi and t8112.dtsi conflict with my "Add Apple SPI controller and spi-nor dt nodes" in https://lore.kernel.org/asahi/20241102-asahi-spi-dt-v1-0-7ac44c0a88f9@jannau.net/ I think it makes more sense to add the spi controller nodes in one go instead of piece by piece based on device support. Janne
On Tue, Nov 26, 2024 at 09:48:01PM +0100, Sasha Finkelstein wrote: > Adds device tree entries for the touchbar digitizer > > Co-developed-by: Janne Grunau <j@jannau.net> > Signed-off-by: Janne Grunau <j@jannau.net> > Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com> > --- > arch/arm64/boot/dts/apple/t8103-j293.dts | 24 ++++++++++++++++++++++++ > arch/arm64/boot/dts/apple/t8103.dtsi | 19 +++++++++++++++++++ > arch/arm64/boot/dts/apple/t8112-j493.dts | 20 ++++++++++++++++++++ > arch/arm64/boot/dts/apple/t8112.dtsi | 14 ++++++++++++++ > 4 files changed, 77 insertions(+) > > diff --git a/arch/arm64/boot/dts/apple/t8103-j293.dts b/arch/arm64/boot/dts/apple/t8103-j293.dts > index 56b0c67bfcda321b60c621de092643017693ff91..a1c4e5731f2147121a9845bc9f34d224025fb145 100644 > --- a/arch/arm64/boot/dts/apple/t8103-j293.dts > +++ b/arch/arm64/boot/dts/apple/t8103-j293.dts > @@ -28,6 +28,10 @@ led-0 { > default-state = "keep"; > }; > }; > + > + aliases { Do not add nodes to the end, but in appropriate place. Either ordered by name, as DTS coding style asks, or in logical place matching existing convention (convention: aliases are always the first node). > + touchbar0 = &touchbar0; Not used, drop. > + }; > }; > > &bluetooth0 { > @@ -38,6 +42,26 @@ &wifi0 { > brcm,board-type = "apple,honshu"; > }; > > +&spi0 { Also unusual placement - between 'w' and 'i'... unless you keep here the second style of sorting (matching DTSI)? > + status = "okay"; > + > + touchbar0: touchbar@0 { > + compatible = "apple,j293-touchbar", > + "apple,z2-touchbar", "apple,z2-multitouch"; > + reg = <0>; > + spi-max-frequency = <11500000>; > + spi-cs-setup-delay-ns = <2000>; > + spi-cs-hold-delay-ns = <2000>; > + reset-gpios = <&pinctrl_ap 139 GPIO_ACTIVE_LOW>; > + cs-gpios = <&pinctrl_ap 109 0>; Use proper GPIO flag define. > + interrupts-extended = <&pinctrl_ap 194 IRQ_TYPE_EDGE_FALLING>; > + firmware-name = "apple/dfrmtfw-j293.bin"; > + touchscreen-size-x = <23045>; > + touchscreen-size-y = <640>; > + label = "MacBookPro17,1 Touch Bar"; > + }; > +}; > + > &i2c2 { > status = "okay"; > }; > diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi > index 9b0dad6b618444ac6b1c9735c50cccfc3965f947..dc72aae3844bf33579f623f0b01abc7de4033af4 100644 > --- a/arch/arm64/boot/dts/apple/t8103.dtsi > +++ b/arch/arm64/boot/dts/apple/t8103.dtsi > @@ -326,6 +326,13 @@ clkref: clock-ref { > clock-output-names = "clkref"; > }; > > + clk_200m: clock-200m { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <200000000>; > + clock-output-names = "clk_200m"; > + }; > + > /* > * This is a fabulated representation of the input clock > * to NCO since we don't know the true clock tree. > @@ -441,6 +448,18 @@ fpwm1: pwm@235044000 { > status = "disabled"; > }; > > + spi0: spi@235100000 { > + compatible = "apple,t8103-spi", "apple,spi"; > + reg = <0x2 0x35100000 0x0 0x4000>; > + interrupt-parent = <&aic>; > + interrupts = <AIC_IRQ 614 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk_200m>; > + power-domains = <&ps_spi0>; > + #address-cells = <1>; > + #size-cells = <0>; > + status = "disabled"; /* only used in J293 */ > + }; > + > serial0: serial@235200000 { > compatible = "apple,s5l-uart"; > reg = <0x2 0x35200000 0x0 0x1000>; > diff --git a/arch/arm64/boot/dts/apple/t8112-j493.dts b/arch/arm64/boot/dts/apple/t8112-j493.dts > index 0ad908349f55406783942735a2e9dad54cda00ec..03fb711b3a1fa767ba70807a6d3404e4d52eb783 100644 > --- a/arch/arm64/boot/dts/apple/t8112-j493.dts > +++ b/arch/arm64/boot/dts/apple/t8112-j493.dts > @@ -20,6 +20,7 @@ / { > aliases { > bluetooth0 = &bluetooth0; > wifi0 = &wifi0; > + touchbar0 = &touchbar0; Do not add to the end of lists/properties/nodes etc, but keep order, usually alphabetical. This avoids conflicts or allows conflicting series to be still merged. That's a general rule for most of development (Makefiles, DTS, Kconfigs, lists in DT bindings). Best regards, Krzysztof
On Wed, 27 Nov 2024 at 09:55, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > + touchbar0 = &touchbar0; > > Not used, drop. Used by the bootloader to forward calibration data to the correct node.
On 27/11/2024 11:31, Sasha Finkelstein wrote: > On Wed, 27 Nov 2024 at 09:55, Krzysztof Kozlowski <krzk@kernel.org> wrote: >>> + touchbar0 = &touchbar0; >> >> Not used, drop. > > Used by the bootloader to forward calibration data to the correct node. I suggest document it with a comment, otherwise each undocumented alias without in-kernel user is a subject of removal later during cleanup. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/apple/t8103-j293.dts b/arch/arm64/boot/dts/apple/t8103-j293.dts index 56b0c67bfcda321b60c621de092643017693ff91..a1c4e5731f2147121a9845bc9f34d224025fb145 100644 --- a/arch/arm64/boot/dts/apple/t8103-j293.dts +++ b/arch/arm64/boot/dts/apple/t8103-j293.dts @@ -28,6 +28,10 @@ led-0 { default-state = "keep"; }; }; + + aliases { + touchbar0 = &touchbar0; + }; }; &bluetooth0 { @@ -38,6 +42,26 @@ &wifi0 { brcm,board-type = "apple,honshu"; }; +&spi0 { + status = "okay"; + + touchbar0: touchbar@0 { + compatible = "apple,j293-touchbar", + "apple,z2-touchbar", "apple,z2-multitouch"; + reg = <0>; + spi-max-frequency = <11500000>; + spi-cs-setup-delay-ns = <2000>; + spi-cs-hold-delay-ns = <2000>; + reset-gpios = <&pinctrl_ap 139 GPIO_ACTIVE_LOW>; + cs-gpios = <&pinctrl_ap 109 0>; + interrupts-extended = <&pinctrl_ap 194 IRQ_TYPE_EDGE_FALLING>; + firmware-name = "apple/dfrmtfw-j293.bin"; + touchscreen-size-x = <23045>; + touchscreen-size-y = <640>; + label = "MacBookPro17,1 Touch Bar"; + }; +}; + &i2c2 { status = "okay"; }; diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi index 9b0dad6b618444ac6b1c9735c50cccfc3965f947..dc72aae3844bf33579f623f0b01abc7de4033af4 100644 --- a/arch/arm64/boot/dts/apple/t8103.dtsi +++ b/arch/arm64/boot/dts/apple/t8103.dtsi @@ -326,6 +326,13 @@ clkref: clock-ref { clock-output-names = "clkref"; }; + clk_200m: clock-200m { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <200000000>; + clock-output-names = "clk_200m"; + }; + /* * This is a fabulated representation of the input clock * to NCO since we don't know the true clock tree. @@ -441,6 +448,18 @@ fpwm1: pwm@235044000 { status = "disabled"; }; + spi0: spi@235100000 { + compatible = "apple,t8103-spi", "apple,spi"; + reg = <0x2 0x35100000 0x0 0x4000>; + interrupt-parent = <&aic>; + interrupts = <AIC_IRQ 614 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk_200m>; + power-domains = <&ps_spi0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; /* only used in J293 */ + }; + serial0: serial@235200000 { compatible = "apple,s5l-uart"; reg = <0x2 0x35200000 0x0 0x1000>; diff --git a/arch/arm64/boot/dts/apple/t8112-j493.dts b/arch/arm64/boot/dts/apple/t8112-j493.dts index 0ad908349f55406783942735a2e9dad54cda00ec..03fb711b3a1fa767ba70807a6d3404e4d52eb783 100644 --- a/arch/arm64/boot/dts/apple/t8112-j493.dts +++ b/arch/arm64/boot/dts/apple/t8112-j493.dts @@ -20,6 +20,7 @@ / { aliases { bluetooth0 = &bluetooth0; wifi0 = &wifi0; + touchbar0 = &touchbar0; }; led-controller { @@ -67,3 +68,22 @@ &i2c4 { &fpwm1 { status = "okay"; }; + +&spi3 { + status = "okay"; + + touchbar0: touchbar@0 { + compatible = "apple,j493-touchbar", "apple,z2-touchbar", "apple,z2-multitouch"; + reg = <0>; + label = "Mac14,7 Touch Bar"; + spi-max-frequency = <8000000>; + spi-cs-setup-delay-ns = <2000>; + spi-cs-hold-delay-ns = <2000>; + + reset-gpios = <&pinctrl_ap 170 GPIO_ACTIVE_LOW>; + interrupts-extended = <&pinctrl_ap 174 IRQ_TYPE_EDGE_FALLING>; + firmware-name = "apple/dfrmtfw-j493.bin"; + touchscreen-size-x = <23045>; + touchscreen-size-y = <640>; + }; +}; diff --git a/arch/arm64/boot/dts/apple/t8112.dtsi b/arch/arm64/boot/dts/apple/t8112.dtsi index 1666e6ab250bc0be9b8318e3c8fc903ccd3f3760..977c1ca5e8c1b566bb3876b6619ea8812b98e072 100644 --- a/arch/arm64/boot/dts/apple/t8112.dtsi +++ b/arch/arm64/boot/dts/apple/t8112.dtsi @@ -467,6 +467,20 @@ fpwm1: pwm@235044000 { status = "disabled"; }; + spi3: spi@23510c000 { + compatible = "apple,t8112-spi", "apple,spi"; + reg = <0x2 0x3510c000 0x0 0x4000>; + interrupt-parent = <&aic>; + interrupts = <AIC_IRQ 751 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clkref>; + pinctrl-0 = <&spi3_pins>; + pinctrl-names = "default"; + power-domains = <&ps_spi3>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + }; + serial0: serial@235200000 { compatible = "apple,s5l-uart"; reg = <0x2 0x35200000 0x0 0x1000>;