Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
Hi Marco,
On 27.05.20 11:35, Marco Felsch wrote:
> Hi Martin,
>
> On 20-05-14 17:57, Martin Kepplinger wrote:
>> From: "Angus Ainslie (Purism)"
>>
>> Add a devicetree description for the Librem 5 phone. The early batches
>> that have been sold are supported as well as the mass-produced device
>> available later this year, see https://puri.sm/products/librem-5/
>>
>> This boots to a working console with working WWAN modem, wifi usdhc,
>> IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs.
>>
>> Signed-off-by: Martin Kepplinger
>> Signed-off-by: Angus Ainslie (Purism)
>> Signed-off-by: Guido Günther
>> ---
>> arch/arm64/boot/dts/freescale/Makefile|1 +
>> .../boot/dts/freescale/imx8mq-librem5.dts | 1174 +
>> 2 files changed, 1175 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>>
>> diff --git a/arch/arm64/boot/dts/freescale/Makefile
>> b/arch/arm64/boot/dts/freescale/Makefile
>> index cd38d04da5a7..342579121f98 100644
>> --- a/arch/arm64/boot/dts/freescale/Makefile
>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>> @@ -34,6 +34,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
>> +dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mq-nitrogen.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mq-phanbell.dtb
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>> b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>> new file mode 100644
>> index ..95c105b4c120
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>> @@ -0,0 +1,1174 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2018-2020 Purism SPC
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "dt-bindings/input/input.h"
>> +#include "dt-bindings/pwm/pwm.h"
>> +#include "dt-bindings/usb/pd.h"
>> +#include "imx8mq.dtsi"
>> +
>> +/ {
>> +model = "Purism Librem 5";
>> +compatible = "purism,librem5", "fsl,imx8mq";
>> +
>> +backlight_dsi: backlight-dsi {
>> +compatible = "led-backlight";
>> +leds = <_backlight>;
>> +brightness-levels = <255>;
>> +default-brightness-level = <100>;
>> +};
>> +
>> +bm818_codec: sound-wwan-codec {
>> +compatible = "broadmobi,bm818", "option,gtm601";
>> +#sound-dai-cells = <0>;
>> +};
>
> Please sort the node names alpabetical.
>
>> +
>> +chosen {
>> +stdout-path =
>> +};
>> +
>> +gpio-keys {
>> +compatible = "gpio-keys";
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_keys>, <_hp>;
>> +
>> +hp-det {
>> +label = "HP_DET";
>> +gpios = < 9 GPIO_ACTIVE_HIGH>;
>> +wakeup-source;
>> +linux,code = ;
>
> Nit: I would add the wakeup-source behind the linux,code.
>
>> +};
>> +
>> +vol-down {
>> +label = "VOL_DOWN";
>> +gpios = < 17 GPIO_ACTIVE_LOW>;
>> +linux,code = ;
>> +};
>> +
>> +vol-up {
>> +label = "VOL_UP";
>> +gpios = < 16 GPIO_ACTIVE_LOW>;
>> +linux,code = ;
>> +};
>> +};
>> +
>> +pwmleds {
>> +compatible = "pwm-leds";
>> +
>> +blue {
>> +label = "phone:blue:front";
>> +max-brightness = <248>;
>> +pwms = < 0 5>;
>> +};
>> +
>> +green {
>> +label = "phone:green:front";
>> +max-brightness = <248>;
>> +pwms = < 0 5>;
>> +};
>> +
>> +red {
>> +label = "phone:red:front";
>> +max-brightness = <248>;
>> +pwms = < 0 5>;
>> +};
>> +};
>> +
>> +pmic_osc: clock-pmic {
>> +compatible = "fixed-clock";
>> +#clock-cells = <0>;
>> +clock-frequency = <32768>;
>> +clock-output-names = "pmic_osc";
>> +};
>
> Please sort nodes alphabetical.
>
>> +
>> +reg_audio_pwr_en: regulator-audio-pwr-en {
>> +compatible = "regulator-fixed";
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_audiopwr>;
>> +regulator-name = "AUDIO_PWR_EN";
>> +regulator-min-microvolt = <180>;
>> +regulator-max-microvolt = <180>;
>> +gpio = < 4 GPIO_ACTIVE_HIGH>;
>> +enable-active-high;
>> +regulator-always-on;
>
> Why should this regulator be always on? The wm8962.c driver can handle
> the regualtor enable/disable.
Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
Hi,
On Fri, May 29, 2020 at 06:28:50PM +0200, Pavel Machek wrote:
> Hi!
>
> > From: "Angus Ainslie (Purism)"
> >
> > Add a devicetree description for the Librem 5 phone. The early batches
> > that have been sold are supported as well as the mass-produced device
> > available later this year, see https://puri.sm/products/librem-5/
> >
> > This boots to a working console with working WWAN modem, wifi usdhc,
> > IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs.
> >
> > Signed-off-by: Martin Kepplinger
> > Signed-off-by: Angus Ainslie (Purism)
> > Signed-off-by: Guido Günther
>
>
> > + blue {
> > + label = "phone:blue:front";
> > + label = "phone:green:front";
> > + label = "phone:red:front";
>
> Droid 4 uses "status-led:{red,green,blue}". Could this use same
> naming?
Looking at leds-class.rst we don't have a useful devicename so
should that just be omitted and s.th. like
label = "blue:status";
label = "green:status";
label = "red:status";
be used instead. If we want to map to current usage
label = "blue:status";
label = "green:boot";
label = "red:charging";
would be even closer but since that is bound to change just going with
"status" would make sense.
Cheers,
-- Guido
>
> > + label = "lm3560:flash";
> > + label = "lm3560:torch";
>
> This is one LED, right? I'm pretty sure we don't want lm3560 in the
> name... "main-camera:flash" would be better. Even better would be
> something that's already in use.
>
> > + label = "white:backlight_cluster";
>
> Make this ":backlight", please. Again, we want something that's
> already used.
>
> Best regards,
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
On 29.05.20 20:07, Pavel Machek wrote: > Hi! > > Plus, do we need calibration matrix for magnetometer? I guess so. It's not a calibration matrix, it's the mount matrix that tells you how the chip is placed on the PCB relative to a "natural" orientation, see https://github.com/torvalds/linux/blob/master/Documentation/ABI/testing/sysfs-bus-iio#L1638 > > Best regards, > Pavel >
Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
Hi! Plus, do we need calibration matrix for magnetometer? Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
Hi!
> From: "Angus Ainslie (Purism)"
>
> Add a devicetree description for the Librem 5 phone. The early batches
> that have been sold are supported as well as the mass-produced device
> available later this year, see https://puri.sm/products/librem-5/
>
> This boots to a working console with working WWAN modem, wifi usdhc,
> IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs.
>
> Signed-off-by: Martin Kepplinger
> Signed-off-by: Angus Ainslie (Purism)
> Signed-off-by: Guido Günther
> + blue {
> + label = "phone:blue:front";
> + label = "phone:green:front";
> + label = "phone:red:front";
Droid 4 uses "status-led:{red,green,blue}". Could this use same
naming?
> + label = "lm3560:flash";
> + label = "lm3560:torch";
This is one LED, right? I'm pretty sure we don't want lm3560 in the
name... "main-camera:flash" would be better. Even better would be
something that's already in use.
> + label = "white:backlight_cluster";
Make this ":backlight", please. Again, we want something that's
already used.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
Hi Martin,
On 20-05-14 17:57, Martin Kepplinger wrote:
> From: "Angus Ainslie (Purism)"
>
> Add a devicetree description for the Librem 5 phone. The early batches
> that have been sold are supported as well as the mass-produced device
> available later this year, see https://puri.sm/products/librem-5/
>
> This boots to a working console with working WWAN modem, wifi usdhc,
> IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs.
>
> Signed-off-by: Martin Kepplinger
> Signed-off-by: Angus Ainslie (Purism)
> Signed-off-by: Guido Günther
> ---
> arch/arm64/boot/dts/freescale/Makefile|1 +
> .../boot/dts/freescale/imx8mq-librem5.dts | 1174 +
> 2 files changed, 1175 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile
> b/arch/arm64/boot/dts/freescale/Makefile
> index cd38d04da5a7..342579121f98 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -34,6 +34,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-nitrogen.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-phanbell.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
> b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
> new file mode 100644
> index ..95c105b4c120
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
> @@ -0,0 +1,1174 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018-2020 Purism SPC
> + */
> +
> +/dts-v1/;
> +
> +#include "dt-bindings/input/input.h"
> +#include "dt-bindings/pwm/pwm.h"
> +#include "dt-bindings/usb/pd.h"
> +#include "imx8mq.dtsi"
> +
> +/ {
> + model = "Purism Librem 5";
> + compatible = "purism,librem5", "fsl,imx8mq";
> +
> + backlight_dsi: backlight-dsi {
> + compatible = "led-backlight";
> + leds = <_backlight>;
> + brightness-levels = <255>;
> + default-brightness-level = <100>;
> + };
> +
> + bm818_codec: sound-wwan-codec {
> + compatible = "broadmobi,bm818", "option,gtm601";
> + #sound-dai-cells = <0>;
> + };
Please sort the node names alpabetical.
> +
> + chosen {
> + stdout-path =
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> + pinctrl-names = "default";
> + pinctrl-0 = <_keys>, <_hp>;
> +
> + hp-det {
> + label = "HP_DET";
> + gpios = < 9 GPIO_ACTIVE_HIGH>;
> + wakeup-source;
> + linux,code = ;
Nit: I would add the wakeup-source behind the linux,code.
> + };
> +
> + vol-down {
> + label = "VOL_DOWN";
> + gpios = < 17 GPIO_ACTIVE_LOW>;
> + linux,code = ;
> + };
> +
> + vol-up {
> + label = "VOL_UP";
> + gpios = < 16 GPIO_ACTIVE_LOW>;
> + linux,code = ;
> + };
> + };
> +
> + pwmleds {
> + compatible = "pwm-leds";
> +
> + blue {
> + label = "phone:blue:front";
> + max-brightness = <248>;
> + pwms = < 0 5>;
> + };
> +
> + green {
> + label = "phone:green:front";
> + max-brightness = <248>;
> + pwms = < 0 5>;
> + };
> +
> + red {
> + label = "phone:red:front";
> + max-brightness = <248>;
> + pwms = < 0 5>;
> + };
> + };
> +
> + pmic_osc: clock-pmic {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32768>;
> + clock-output-names = "pmic_osc";
> + };
Please sort nodes alphabetical.
> +
> + reg_audio_pwr_en: regulator-audio-pwr-en {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <_audiopwr>;
> + regulator-name = "AUDIO_PWR_EN";
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> + gpio = < 4 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + regulator-always-on;
Why should this regulator be always on? The wm8962.c driver can handle
the regualtor enable/disable.
> + };
> +
> + reg_aud_1v8: regulator-audio-v1v8 {
^
regulator-audio-1v8?
> +
Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
On Thu, May 14, 2020 at 7:02 PM Martin Kepplinger wrote: > > From: "Angus Ainslie (Purism)" > > Add a devicetree description for the Librem 5 phone. The early batches > that have been sold are supported as well as the mass-produced device > available later this year, see https://puri.sm/products/librem-5/ > > This boots to a working console with working WWAN modem, wifi usdhc, > IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs. > > Signed-off-by: Martin Kepplinger > Signed-off-by: Angus Ainslie (Purism) > Signed-off-by: Guido Günther For audio related part: Reviewed-by: Daniel Baluta
Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
On 14.05.20 17:57, Martin Kepplinger wrote: > From: "Angus Ainslie (Purism)" > > Add a devicetree description for the Librem 5 phone. The early batches > that have been sold are supported as well as the mass-produced device > available later this year, see https://puri.sm/products/librem-5/ > > This boots to a working console with working WWAN modem, wifi usdhc, > IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs. > > Signed-off-by: Martin Kepplinger > Signed-off-by: Angus Ainslie (Purism) > Signed-off-by: Guido Günther > --- > arch/arm64/boot/dts/freescale/Makefile|1 + > .../boot/dts/freescale/imx8mq-librem5.dts | 1174 + > 2 files changed, 1175 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-librem5.dts > hi, I highly appreciate it in case you can take time to review. It's not the smallest board, but this "base" support isn't huge either. For devicetree people this is certainly easy to read and I'll be happy for any opinions, objections or Acks you might have. thanks, martin
[PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
From: "Angus Ainslie (Purism)"
Add a devicetree description for the Librem 5 phone. The early batches
that have been sold are supported as well as the mass-produced device
available later this year, see https://puri.sm/products/librem-5/
This boots to a working console with working WWAN modem, wifi usdhc,
IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs.
Signed-off-by: Martin Kepplinger
Signed-off-by: Angus Ainslie (Purism)
Signed-off-by: Guido Günther
---
arch/arm64/boot/dts/freescale/Makefile|1 +
.../boot/dts/freescale/imx8mq-librem5.dts | 1174 +
2 files changed, 1175 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
diff --git a/arch/arm64/boot/dts/freescale/Makefile
b/arch/arm64/boot/dts/freescale/Makefile
index cd38d04da5a7..342579121f98 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -34,6 +34,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
+dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mq-nitrogen.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mq-phanbell.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
new file mode 100644
index ..95c105b4c120
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
@@ -0,0 +1,1174 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2018-2020 Purism SPC
+ */
+
+/dts-v1/;
+
+#include "dt-bindings/input/input.h"
+#include "dt-bindings/pwm/pwm.h"
+#include "dt-bindings/usb/pd.h"
+#include "imx8mq.dtsi"
+
+/ {
+ model = "Purism Librem 5";
+ compatible = "purism,librem5", "fsl,imx8mq";
+
+ backlight_dsi: backlight-dsi {
+ compatible = "led-backlight";
+ leds = <_backlight>;
+ brightness-levels = <255>;
+ default-brightness-level = <100>;
+ };
+
+ bm818_codec: sound-wwan-codec {
+ compatible = "broadmobi,bm818", "option,gtm601";
+ #sound-dai-cells = <0>;
+ };
+
+ chosen {
+ stdout-path =
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+ pinctrl-names = "default";
+ pinctrl-0 = <_keys>, <_hp>;
+
+ hp-det {
+ label = "HP_DET";
+ gpios = < 9 GPIO_ACTIVE_HIGH>;
+ wakeup-source;
+ linux,code = ;
+ };
+
+ vol-down {
+ label = "VOL_DOWN";
+ gpios = < 17 GPIO_ACTIVE_LOW>;
+ linux,code = ;
+ };
+
+ vol-up {
+ label = "VOL_UP";
+ gpios = < 16 GPIO_ACTIVE_LOW>;
+ linux,code = ;
+ };
+ };
+
+ pwmleds {
+ compatible = "pwm-leds";
+
+ blue {
+ label = "phone:blue:front";
+ max-brightness = <248>;
+ pwms = < 0 5>;
+ };
+
+ green {
+ label = "phone:green:front";
+ max-brightness = <248>;
+ pwms = < 0 5>;
+ };
+
+ red {
+ label = "phone:red:front";
+ max-brightness = <248>;
+ pwms = < 0 5>;
+ };
+ };
+
+ pmic_osc: clock-pmic {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <32768>;
+ clock-output-names = "pmic_osc";
+ };
+
+ reg_audio_pwr_en: regulator-audio-pwr-en {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <_audiopwr>;
+ regulator-name = "AUDIO_PWR_EN";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <180>;
+ gpio = < 4 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-always-on;
+ };
+
+ reg_aud_1v8: regulator-audio-v1v8 {
+ compatible = "regulator-fixed";
+ regulator-name = "aud_1v8";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <180>;
+ vin-supply = <_audio_pwr_en>;
+ };
+
+ reg_gnss: regulator-gnss {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <_gnsspwr>;
+ regulator-name = "GNSS";
+ regulator-min-microvolt =
