| Message ID | 20211122225807.8105-4-j@jannau.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add DTs for all Apple M1 (t8103) devices | expand |
On 23/11/2021 07.58, Janne Grunau wrote: > Apple M1 has at least 5 i2c controllers. i2c0, i2c1 and i2c3 are used > on all M1 Mac devices. The 2020 Mac Mini uses i2c2 and the 13-inch > MacBook Pro uses i2c2 and i2c4. > > Signed-off-by: Janne Grunau <j@jannau.net> > --- > arch/arm64/boot/dts/apple/t8103-j274.dts | 4 ++ > arch/arm64/boot/dts/apple/t8103-j293.dts | 8 +++ > arch/arm64/boot/dts/apple/t8103.dtsi | 92 ++++++++++++++++++++++++ > 3 files changed, 104 insertions(+) > > diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi > index c320c8baeb41..15fec48f943a 100644 > --- a/arch/arm64/boot/dts/apple/t8103.dtsi > +++ b/arch/arm64/boot/dts/apple/t8103.dtsi > @@ -126,6 +126,73 @@ serial0: serial@235200000 { > status = "disabled"; > }; > > + i2c0: i2c@235010000 { > + compatible = "apple,t8103-i2c", "apple,i2c"; > + reg = <0x2 0x35010000 0x0 0x4000>; > + clocks = <&clk24>; [...] Please put these before serial0; I want to keep the nodes sorted by address. With that fixed: Acked-by: Hector Martin <marcan@marcan.st>
On 23/11/2021 07.58, Janne Grunau wrote: > Apple M1 has at least 5 i2c controllers. i2c0, i2c1 and i2c3 are used > on all M1 Mac devices. The 2020 Mac Mini uses i2c2 and the 13-inch > MacBook Pro uses i2c2 and i2c4. On further testing: i2c3 is not used on the 1GbE variant of j274. iBoot actually kills the node entirely. The interesting thing is it doesn't work; it times out probe transactions. I suspect iBoot does not enable its clock or something like that. I'll poke around this on IRC, but a priori we probably need m1n1 code to kill this node when the ADT doesn't have it. Maybe I should generalize the dwc3 killing code...
On Mon, Nov 22, 2021, at 23:58, Janne Grunau wrote: > Apple M1 has at least 5 i2c controllers. i2c0, i2c1 and i2c3 are used > on all M1 Mac devices. The 2020 Mac Mini uses i2c2 and the 13-inch > MacBook Pro uses i2c2 and i2c4. > > Signed-off-by: Janne Grunau <j@jannau.net> > --- > arch/arm64/boot/dts/apple/t8103-j274.dts | 4 ++ > arch/arm64/boot/dts/apple/t8103-j293.dts | 8 +++ > arch/arm64/boot/dts/apple/t8103.dtsi | 92 ++++++++++++++++++++++++ > 3 files changed, 104 insertions(+) > > diff --git a/arch/arm64/boot/dts/apple/t8103-j274.dts > b/arch/arm64/boot/dts/apple/t8103-j274.dts > index 9e01ef70039d..2cd429efba5b 100644 > --- a/arch/arm64/boot/dts/apple/t8103-j274.dts > +++ b/arch/arm64/boot/dts/apple/t8103-j274.dts > @@ -39,3 +39,7 @@ ethernet0: ethernet@0,0 { > local-mac-address = [00 10 18 00 00 00]; > }; > }; > + > +&i2c2 { > + status = "okay"; > +}; > diff --git a/arch/arm64/boot/dts/apple/t8103-j293.dts > b/arch/arm64/boot/dts/apple/t8103-j293.dts > index 466035f00b69..49cdf4b560a3 100644 > --- a/arch/arm64/boot/dts/apple/t8103-j293.dts > +++ b/arch/arm64/boot/dts/apple/t8103-j293.dts > @@ -31,3 +31,11 @@ &pcie0_dart_2 { > > /delete-node/ &port01; > /delete-node/ &port02; > + > +&i2c2 { > + status = "okay"; > +}; > + > +&i2c4 { > + status = "okay"; > +}; > diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi > b/arch/arm64/boot/dts/apple/t8103.dtsi > index c320c8baeb41..15fec48f943a 100644 > --- a/arch/arm64/boot/dts/apple/t8103.dtsi > +++ b/arch/arm64/boot/dts/apple/t8103.dtsi > @@ -126,6 +126,73 @@ serial0: serial@235200000 { > status = "disabled"; > }; > > + i2c0: i2c@235010000 { > + compatible = "apple,t8103-i2c", "apple,i2c"; > + reg = <0x2 0x35010000 0x0 0x4000>; > + clocks = <&clk24>; > + clock-names = "ref"; No need for clock-names in these nodes. The schema only allows a single clock anyway. With that removed and Hector's comments addressed: Reviewed-by: Sven Peter <sven@svenpeter.dev>
On 23/11/2021 14.41, Hector Martin wrote: > On 23/11/2021 07.58, Janne Grunau wrote: >> Apple M1 has at least 5 i2c controllers. i2c0, i2c1 and i2c3 are used >> on all M1 Mac devices. The 2020 Mac Mini uses i2c2 and the 13-inch >> MacBook Pro uses i2c2 and i2c4. > > On further testing: i2c3 is not used on the 1GbE variant of j274. iBoot > actually kills the node entirely. The interesting thing is it doesn't > work; it times out probe transactions. I suspect iBoot does not enable > its clock or something like that. > > I'll poke around this on IRC, but a priori we probably need m1n1 code to > kill this node when the ADT doesn't have it. Maybe I should generalize > the dwc3 killing code... Did that as of m1n1 be7ff3a, so we're good now :) For those following along in the list: the reason why i2c3 was getting stuck is because it seems the unused bus is weakly pulled low on these machines, which jams it. Setting the GPIO pull mode to pull-up makes it work as an empty bus, but let's just not instantiate at all in this case. m1n1 now checks the Apple DT and sets any FDT i2c devices that are not present in it to disabled.
On Tue, Nov 23, 2021 at 3:43 PM Hector Martin <marcan@marcan.st> wrote: > For those following along in the list: the reason why i2c3 was getting > stuck is because it seems the unused bus is weakly pulled low on these > machines, which jams it. That looks like some power saving attempt. I suppose that means that even i2c buses that are in use could be weakly pulled low when suspending the system and maybe even inbetween transactions to save some leak current. Yours, Linus Walleij
On 24/11/2021 08.26, Linus Walleij wrote: > On Tue, Nov 23, 2021 at 3:43 PM Hector Martin <marcan@marcan.st> wrote: > >> For those following along in the list: the reason why i2c3 was getting >> stuck is because it seems the unused bus is weakly pulled low on these >> machines, which jams it. > > That looks like some power saving attempt. > > I suppose that means that even i2c buses that are in use > could be weakly pulled low when suspending the system > and maybe even inbetween transactions to save some > leak current. Pulled up vs. down dosn't really result in better power savings; neither state will necessarily have more leakage. I think it's just that the pins are completely disconnected, and there's some very minor leakage to ground (megaohms will do) that ends up pulling them down. Pulling down an I2C bus between transactions is not legal; the idle state has to be high. Apple are actually not very good at configuring GPIOs for power saving; e.g. the I/Os for that unused i2c bus still have their input buffers turned on, which is a waste of power. If they wanted to save the smallest drop of power they'd turn that off. But the effect of this is so trivial it probably makes no difference in the context of a laptop, nevermind a desktop like the Mac Mini.
On Wed, Nov 24, 2021 at 6:42 AM Hector Martin <marcan@marcan.st> wrote: > Pulling down an I2C bus between transactions is not legal; the idle > state has to be high. Oh right. > Apple are actually not very good at configuring GPIOs for power saving; > e.g. the I/Os for that unused i2c bus still have their input buffers > turned on, which is a waste of power. If they wanted to save the > smallest drop of power they'd turn that off. But the effect of this is > so trivial it probably makes no difference in the context of a laptop, > nevermind a desktop like the Mac Mini. Hm that's true. The saying is that 99% is the backlight, the remaining 1% isn't so significant after that. Yours, Linus Walleij
On 2021年11月26日 0:50:28 JST, Linus Walleij <linus.walleij@linaro.org> wrote: >On Wed, Nov 24, 2021 at 6:42 AM Hector Martin <marcan@marcan.st> wrote: > >> Pulling down an I2C bus between transactions is not legal; the idle >> state has to be high. > >Oh right. > >> Apple are actually not very good at configuring GPIOs for power saving; >> e.g. the I/Os for that unused i2c bus still have their input buffers >> turned on, which is a waste of power. If they wanted to save the >> smallest drop of power they'd turn that off. But the effect of this is >> so trivial it probably makes no difference in the context of a laptop, >> nevermind a desktop like the Mac Mini. > >Hm that's true. The saying is that 99% is the backlight, the remaining 1% isn't >so significant after that. From watching a USB power meter plugged into the MacBook Air, I can say the backlight is about 70% :-) Then again, the new MBPs have LED matrix backlit screens... so suddenly dark themes are more efficient again! I should measure that one, see how much power changing themes saves.
diff --git a/arch/arm64/boot/dts/apple/t8103-j274.dts b/arch/arm64/boot/dts/apple/t8103-j274.dts index 9e01ef70039d..2cd429efba5b 100644 --- a/arch/arm64/boot/dts/apple/t8103-j274.dts +++ b/arch/arm64/boot/dts/apple/t8103-j274.dts @@ -39,3 +39,7 @@ ethernet0: ethernet@0,0 { local-mac-address = [00 10 18 00 00 00]; }; }; + +&i2c2 { + status = "okay"; +}; diff --git a/arch/arm64/boot/dts/apple/t8103-j293.dts b/arch/arm64/boot/dts/apple/t8103-j293.dts index 466035f00b69..49cdf4b560a3 100644 --- a/arch/arm64/boot/dts/apple/t8103-j293.dts +++ b/arch/arm64/boot/dts/apple/t8103-j293.dts @@ -31,3 +31,11 @@ &pcie0_dart_2 { /delete-node/ &port01; /delete-node/ &port02; + +&i2c2 { + status = "okay"; +}; + +&i2c4 { + status = "okay"; +}; diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi index c320c8baeb41..15fec48f943a 100644 --- a/arch/arm64/boot/dts/apple/t8103.dtsi +++ b/arch/arm64/boot/dts/apple/t8103.dtsi @@ -126,6 +126,73 @@ serial0: serial@235200000 { status = "disabled"; }; + i2c0: i2c@235010000 { + compatible = "apple,t8103-i2c", "apple,i2c"; + reg = <0x2 0x35010000 0x0 0x4000>; + clocks = <&clk24>; + clock-names = "ref"; + interrupt-parent = <&aic>; + interrupts = <AIC_IRQ 627 IRQ_TYPE_LEVEL_HIGH>; + pinctrl-0 = <&i2c0_pins>; + pinctrl-names = "default"; + #address-cells = <0x1>; + #size-cells = <0x0>; + }; + + i2c1: i2c@235014000 { + compatible = "apple,t8103-i2c", "apple,i2c"; + reg = <0x2 0x35014000 0x0 0x4000>; + clocks = <&clk24>; + clock-names = "ref"; + interrupt-parent = <&aic>; + interrupts = <AIC_IRQ 628 IRQ_TYPE_LEVEL_HIGH>; + pinctrl-0 = <&i2c1_pins>; + pinctrl-names = "default"; + #address-cells = <0x1>; + #size-cells = <0x0>; + }; + + i2c2: i2c@235018000 { + compatible = "apple,t8103-i2c", "apple,i2c"; + reg = <0x2 0x35018000 0x0 0x4000>; + clocks = <&clk24>; + clock-names = "ref"; + interrupt-parent = <&aic>; + interrupts = <AIC_IRQ 629 IRQ_TYPE_LEVEL_HIGH>; + pinctrl-0 = <&i2c2_pins>; + pinctrl-names = "default"; + #address-cells = <0x1>; + #size-cells = <0x0>; + status = "disabled"; /* not used in all devices */ + }; + + i2c3: i2c@23501c000 { + compatible = "apple,t8103-i2c", "apple,i2c"; + reg = <0x2 0x3501c000 0x0 0x4000>; + clocks = <&clk24>; + clock-names = "ref"; + interrupt-parent = <&aic>; + interrupts = <AIC_IRQ 630 IRQ_TYPE_LEVEL_HIGH>; + pinctrl-0 = <&i2c3_pins>; + pinctrl-names = "default"; + #address-cells = <0x1>; + #size-cells = <0x0>; + }; + + i2c4: i2c@235020000 { + compatible = "apple,t8103-i2c", "apple,i2c"; + reg = <0x2 0x35020000 0x0 0x4000>; + clocks = <&clk24>; + clock-names = "ref"; + interrupt-parent = <&aic>; + interrupts = <AIC_IRQ 631 IRQ_TYPE_LEVEL_HIGH>; + pinctrl-0 = <&i2c4_pins>; + pinctrl-names = "default"; + #address-cells = <0x1>; + #size-cells = <0x0>; + status = "disabled"; /* only used in J293 */ + }; + aic: interrupt-controller@23b100000 { compatible = "apple,t8103-aic", "apple,aic"; #interrupt-cells = <3>; @@ -153,6 +220,31 @@ pinctrl_ap: pinctrl@23c100000 { <AIC_IRQ 195 IRQ_TYPE_LEVEL_HIGH>, <AIC_IRQ 196 IRQ_TYPE_LEVEL_HIGH>; + i2c0_pins: i2c0-pins { + pinmux = <APPLE_PINMUX(192, 1)>, + <APPLE_PINMUX(188, 1)>; + }; + + i2c1_pins: i2c1-pins { + pinmux = <APPLE_PINMUX(201, 1)>, + <APPLE_PINMUX(199, 1)>; + }; + + i2c2_pins: i2c2-pins { + pinmux = <APPLE_PINMUX(163, 1)>, + <APPLE_PINMUX(162, 1)>; + }; + + i2c3_pins: i2c3-pins { + pinmux = <APPLE_PINMUX(73, 1)>, + <APPLE_PINMUX(72, 1)>; + }; + + i2c4_pins: i2c4-pins { + pinmux = <APPLE_PINMUX(135, 1)>, + <APPLE_PINMUX(134, 1)>; + }; + pcie_pins: pcie-pins { pinmux = <APPLE_PINMUX(150, 1)>, <APPLE_PINMUX(151, 1)>,
Apple M1 has at least 5 i2c controllers. i2c0, i2c1 and i2c3 are used on all M1 Mac devices. The 2020 Mac Mini uses i2c2 and the 13-inch MacBook Pro uses i2c2 and i2c4. Signed-off-by: Janne Grunau <j@jannau.net> --- arch/arm64/boot/dts/apple/t8103-j274.dts | 4 ++ arch/arm64/boot/dts/apple/t8103-j293.dts | 8 +++ arch/arm64/boot/dts/apple/t8103.dtsi | 92 ++++++++++++++++++++++++ 3 files changed, 104 insertions(+)