| Message ID | 20211122225807.8105-1-j@jannau.net |
|---|---|
| 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 Mon, Nov 22, 2021, at 23:58, Janne Grunau wrote: > All M1 Mac devices have 2 SoC connected USB-C ports and use cd321x USB > type C port switch and power deliver controllers. I2c bus and adresses > configuration are for all devices equal. > The iMac (24-inch, 2021) has a configuration with 2 additional USB-C > ports (j456) using two addiotional cd321x controllers. > > Signed-off-by: Janne Grunau <j@jannau.net> > --- > arch/arm64/boot/dts/apple/t8103-j456.dts | 18 ++++++++++++++++++ > arch/arm64/boot/dts/apple/t8103-jxxx.dtsi | 18 ++++++++++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/arch/arm64/boot/dts/apple/t8103-j456.dts > b/arch/arm64/boot/dts/apple/t8103-j456.dts > index 7267e069355b..c6b8c28455f5 100644 > --- a/arch/arm64/boot/dts/apple/t8103-j456.dts > +++ b/arch/arm64/boot/dts/apple/t8103-j456.dts > @@ -21,6 +21,24 @@ aliases { > }; > }; > > +&i2c0 { > + hpm2: tps6598x@3b { I know that the tps example in the binding uses tps6598x as the node name but I think that these names are usually more general. Maybe usb-pd@3b instead which is used in arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi and reflects the function of this device. With that fixed: Reviewed-by: Sven Peter <sven@svenpeter.dev> Thanks, Sven
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.
On 23/11/2021 07.58, Janne Grunau wrote: > All M1 Mac devices have 2 SoC connected USB-C ports and use cd321x USB > type C port switch and power deliver controllers. I2c bus and adresses > configuration are for all devices equal. > The iMac (24-inch, 2021) has a configuration with 2 additional USB-C > ports (j456) using two addiotional cd321x controllers. > > Signed-off-by: Janne Grunau <j@jannau.net> > --- > arch/arm64/boot/dts/apple/t8103-j456.dts | 18 ++++++++++++++++++ > arch/arm64/boot/dts/apple/t8103-jxxx.dtsi | 18 ++++++++++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/arch/arm64/boot/dts/apple/t8103-j456.dts b/arch/arm64/boot/dts/apple/t8103-j456.dts > index 7267e069355b..c6b8c28455f5 100644 > --- a/arch/arm64/boot/dts/apple/t8103-j456.dts > +++ b/arch/arm64/boot/dts/apple/t8103-j456.dts > @@ -21,6 +21,24 @@ aliases { > }; > }; > > +&i2c0 { > + hpm2: tps6598x@3b { > + compatible = "apple,cd321x"; > + reg = <0x3b>; > + interrupt-parent = <&pinctrl_ap>; > + interrupts = <106 IRQ_TYPE_LEVEL_LOW>; > + interrupt-names = "irq"; > + }; > + > + hpm3: tps6598x@3c { > + compatible = "apple,cd321x"; > + reg = <0x3c>; > + interrupt-parent = <&pinctrl_ap>; > + interrupts = <106 IRQ_TYPE_LEVEL_LOW>; > + interrupt-names = "irq"; > + }; > +}; > + > /* > * Force the bus number assignments so that we can declare some of the > * on-board devices and properties that are populated by the bootloader > diff --git a/arch/arm64/boot/dts/apple/t8103-jxxx.dtsi b/arch/arm64/boot/dts/apple/t8103-jxxx.dtsi > index 53d22b434d76..5f748f3f60d2 100644 > --- a/arch/arm64/boot/dts/apple/t8103-jxxx.dtsi > +++ b/arch/arm64/boot/dts/apple/t8103-jxxx.dtsi > @@ -39,6 +39,24 @@ &serial0 { > status = "okay"; > }; > > +&i2c0 { > + hpm0: tps6598x@38 { > + compatible = "apple,cd321x"; > + reg = <0x38>; > + interrupt-parent = <&pinctrl_ap>; > + interrupts = <106 IRQ_TYPE_LEVEL_LOW>; > + interrupt-names = "irq"; > + }; > + > + hpm1: tps6598x@3f { > + compatible = "apple,cd321x"; > + reg = <0x3f>; > + interrupt-parent = <&pinctrl_ap>; > + interrupts = <106 IRQ_TYPE_LEVEL_LOW>; > + interrupt-names = "irq"; > + }; > +}; > + > /* > * Force the bus number assignments so that we can declare some of the > * on-board devices and properties that are populated by the bootloader > With sven's comments addressed, Acked-by: Hector Martin <marcan@marcan.st>
On 23/11/2021 07.58, Janne Grunau wrote: > Hej All, > > this series extends the device tree files and bindings to all current > Apple M1 devices. Specifically it adds DTs for following devices: > - MacBook Air (M1, 2020) > - Macbook Pro (13-inch, M1, 2020) > - iMac (24-inch, M1, 2021) > > It also adds i2c and cd321x devices nodes. Bindings and code changes for > those were merged for 5.16 without adding devices to the device tree. > Patches are include in a single series for dependencies > > Changes since v2: > - split i2c and cd321x changes into 2 commit > - add i2c4 used by MacBooc Pros (j293, 13-inch, M1, 2020) > - add hpm2 and hpm3 for the additional USB-C ports on j456 iMac > (24-inch, M1, 2021) models > > Series depends for functionality and dtbs verification on > "[PATCH 0/3] Apple Arm patform device tree and bindings fixes". > > thanks, > Janne > > The series is available as branch from: > https://github.com/jannau/linux/tree/apple_m1/dt-for-5.17_v3 > > Janne Grunau (4): > dt-bindings: arm: apple: Add iMac (24-inch 2021) to Apple bindings > arm64: dts: apple: Add missing M1 (t8103) devices > arm64: dts: apple: t8103: Add i2c nodes > arm64: dts: apple: t8103: Add cd321x nodes > > .../devicetree/bindings/arm/apple.yaml | 6 +- > arch/arm64/boot/dts/apple/Makefile | 4 + > arch/arm64/boot/dts/apple/t8103-j274.dts | 33 +------ > arch/arm64/boot/dts/apple/t8103-j293.dts | 41 +++++++++ > arch/arm64/boot/dts/apple/t8103-j313.dts | 33 +++++++ > arch/arm64/boot/dts/apple/t8103-j456.dts | 59 ++++++++++++ > arch/arm64/boot/dts/apple/t8103-j457.dts | 47 ++++++++++ > arch/arm64/boot/dts/apple/t8103-jxxx.dtsi | 67 ++++++++++++++ > arch/arm64/boot/dts/apple/t8103.dtsi | 92 +++++++++++++++++++ > 9 files changed, 352 insertions(+), 30 deletions(-) > create mode 100644 arch/arm64/boot/dts/apple/t8103-j293.dts > create mode 100644 arch/arm64/boot/dts/apple/t8103-j313.dts > create mode 100644 arch/arm64/boot/dts/apple/t8103-j456.dts > create mode 100644 arch/arm64/boot/dts/apple/t8103-j457.dts > create mode 100644 arch/arm64/boot/dts/apple/t8103-jxxx.dtsi > Whole series is: Tested-by: Hector Martin <marcan@marcan.st> on j274, j313, j456. Thanks! If you spin up a v4 with the minor comments addressed, I'll be happy to merge it :)
Hej, On 2021-11-26 14:39:16 +0900, Hector Martin wrote: > On 23/11/2021 07.58, Janne Grunau wrote: > > > > Janne Grunau (4): > > dt-bindings: arm: apple: Add iMac (24-inch 2021) to Apple bindings > > arm64: dts: apple: Add missing M1 (t8103) devices > > arm64: dts: apple: t8103: Add i2c nodes > > arm64: dts: apple: t8103: Add cd321x nodes > > > > .../devicetree/bindings/arm/apple.yaml | 6 +- > > arch/arm64/boot/dts/apple/Makefile | 4 + > > arch/arm64/boot/dts/apple/t8103-j274.dts | 33 +------ > > arch/arm64/boot/dts/apple/t8103-j293.dts | 41 +++++++++ > > arch/arm64/boot/dts/apple/t8103-j313.dts | 33 +++++++ > > arch/arm64/boot/dts/apple/t8103-j456.dts | 59 ++++++++++++ > > arch/arm64/boot/dts/apple/t8103-j457.dts | 47 ++++++++++ > > arch/arm64/boot/dts/apple/t8103-jxxx.dtsi | 67 ++++++++++++++ > > arch/arm64/boot/dts/apple/t8103.dtsi | 92 +++++++++++++++++++ > > 9 files changed, 352 insertions(+), 30 deletions(-) > > create mode 100644 arch/arm64/boot/dts/apple/t8103-j293.dts > > create mode 100644 arch/arm64/boot/dts/apple/t8103-j313.dts > > create mode 100644 arch/arm64/boot/dts/apple/t8103-j456.dts > > create mode 100644 arch/arm64/boot/dts/apple/t8103-j457.dts > > create mode 100644 arch/arm64/boot/dts/apple/t8103-jxxx.dtsi > > > > Whole series is: > > Tested-by: Hector Martin <marcan@marcan.st> > > on j274, j313, j456. > > Thanks! If you spin up a v4 with the minor comments addressed, I'll be happy > to merge it :) there is already a v4 with Sven's comments addressed at https://lore.kernel.org/linux-arm-kernel/20211123224926.7722-1-j@jannau.net/ Jean-Marc Ranger notified me off-list of a typo in "arm64: dts: apple: t8103: Add cd321x nodes" commit messages: 'addiotional'. I don't think it's worth spinning up a v5 for fixing that typo. thanks, Janne
On 26/11/2021 15.33, Janne Grunau wrote: > Hej, > > On 2021-11-26 14:39:16 +0900, Hector Martin wrote: >> On 23/11/2021 07.58, Janne Grunau wrote: >>> >>> Janne Grunau (4): >>> dt-bindings: arm: apple: Add iMac (24-inch 2021) to Apple bindings >>> arm64: dts: apple: Add missing M1 (t8103) devices >>> arm64: dts: apple: t8103: Add i2c nodes >>> arm64: dts: apple: t8103: Add cd321x nodes >>> >>> .../devicetree/bindings/arm/apple.yaml | 6 +- >>> arch/arm64/boot/dts/apple/Makefile | 4 + >>> arch/arm64/boot/dts/apple/t8103-j274.dts | 33 +------ >>> arch/arm64/boot/dts/apple/t8103-j293.dts | 41 +++++++++ >>> arch/arm64/boot/dts/apple/t8103-j313.dts | 33 +++++++ >>> arch/arm64/boot/dts/apple/t8103-j456.dts | 59 ++++++++++++ >>> arch/arm64/boot/dts/apple/t8103-j457.dts | 47 ++++++++++ >>> arch/arm64/boot/dts/apple/t8103-jxxx.dtsi | 67 ++++++++++++++ >>> arch/arm64/boot/dts/apple/t8103.dtsi | 92 +++++++++++++++++++ >>> 9 files changed, 352 insertions(+), 30 deletions(-) >>> create mode 100644 arch/arm64/boot/dts/apple/t8103-j293.dts >>> create mode 100644 arch/arm64/boot/dts/apple/t8103-j313.dts >>> create mode 100644 arch/arm64/boot/dts/apple/t8103-j456.dts >>> create mode 100644 arch/arm64/boot/dts/apple/t8103-j457.dts >>> create mode 100644 arch/arm64/boot/dts/apple/t8103-jxxx.dtsi >>> >> >> Whole series is: >> >> Tested-by: Hector Martin <marcan@marcan.st> >> >> on j274, j313, j456. >> >> Thanks! If you spin up a v4 with the minor comments addressed, I'll be happy >> to merge it :) > > there is already a v4 with Sven's comments addressed at > > https://lore.kernel.org/linux-arm-kernel/20211123224926.7722-1-j@jannau.net/ > > Jean-Marc Ranger notified me off-list of a typo in "arm64: dts: apple: > t8103: Add cd321x nodes" commit messages: 'addiotional'. > > I don't think it's worth spinning up a v5 for fixing that typo. Oh whoops, I think I mixed that one up with this one when scrolling through email and thought they were the same series. Was just catching up on email and it hadn't hit me yet... thanks for pointing it out.
Hej All, this series extends the device tree files and bindings to all current Apple M1 devices. Specifically it adds DTs for following devices: - MacBook Air (M1, 2020) - Macbook Pro (13-inch, M1, 2020) - iMac (24-inch, M1, 2021) It also adds i2c and cd321x devices nodes. Bindings and code changes for those were merged for 5.16 without adding devices to the device tree. Patches are include in a single series for dependencies Changes since v2: - split i2c and cd321x changes into 2 commit - add i2c4 used by MacBooc Pros (j293, 13-inch, M1, 2020) - add hpm2 and hpm3 for the additional USB-C ports on j456 iMac (24-inch, M1, 2021) models Series depends for functionality and dtbs verification on "[PATCH 0/3] Apple Arm patform device tree and bindings fixes". thanks, Janne The series is available as branch from: https://github.com/jannau/linux/tree/apple_m1/dt-for-5.17_v3 Janne Grunau (4): dt-bindings: arm: apple: Add iMac (24-inch 2021) to Apple bindings arm64: dts: apple: Add missing M1 (t8103) devices arm64: dts: apple: t8103: Add i2c nodes arm64: dts: apple: t8103: Add cd321x nodes .../devicetree/bindings/arm/apple.yaml | 6 +- arch/arm64/boot/dts/apple/Makefile | 4 + arch/arm64/boot/dts/apple/t8103-j274.dts | 33 +------ arch/arm64/boot/dts/apple/t8103-j293.dts | 41 +++++++++ arch/arm64/boot/dts/apple/t8103-j313.dts | 33 +++++++ arch/arm64/boot/dts/apple/t8103-j456.dts | 59 ++++++++++++ arch/arm64/boot/dts/apple/t8103-j457.dts | 47 ++++++++++ arch/arm64/boot/dts/apple/t8103-jxxx.dtsi | 67 ++++++++++++++ arch/arm64/boot/dts/apple/t8103.dtsi | 92 +++++++++++++++++++ 9 files changed, 352 insertions(+), 30 deletions(-) create mode 100644 arch/arm64/boot/dts/apple/t8103-j293.dts create mode 100644 arch/arm64/boot/dts/apple/t8103-j313.dts create mode 100644 arch/arm64/boot/dts/apple/t8103-j456.dts create mode 100644 arch/arm64/boot/dts/apple/t8103-j457.dts create mode 100644 arch/arm64/boot/dts/apple/t8103-jxxx.dtsi