Hello Richard, Nice to see you upstreaming this! Thumbs up! Just few remarks to pmic node from me: On Thu, Jun 20, 2019 at 04:32:52PM +0300, Andra Danciu wrote: > From: Richard Hu <richard.hu@xxxxxxxxxxxxxx> > > The current level of support yields a working console and is able to boot > userspace from an initial ramdisk copied via u-boot in RAM. > > Additional subsystems that are active : > - Ethernet > - USB > > Cc: Daniel Baluta <daniel.baluta@xxxxxxx> > Signed-off-by: Richard Hu <richard.hu@xxxxxxxxxxxxxx> > Signed-off-by: Andra Danciu <andradanciu1997@xxxxxxxxx> > --- > I am using pico-pi-8mxm board to work on my project for Google Summer of Code. > This is based on patches from https://github.com/wandboard-org. > > arch/arm64/boot/dts/freescale/Makefile | 1 + > arch/arm64/boot/dts/freescale/wand-pi-8m.dts | 590 +++++++++++++++++++++++++++ > 2 files changed, 591 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/wand-pi-8m.dts > > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile > index 984554343c83..5904d6a8a033 100644 > --- a/arch/arm64/boot/dts/freescale/Makefile > +++ b/arch/arm64/boot/dts/freescale/Makefile > @@ -23,3 +23,4 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-rdb.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb > +dtb-$(CONFIG_ARCH_MXC) += wand-pi-8m.dtb > diff --git a/arch/arm64/boot/dts/freescale/wand-pi-8m.dts b/arch/arm64/boot/dts/freescale/wand-pi-8m.dts > new file mode 100644 > index 000000000000..9f7121014722 > --- /dev/null > +++ b/arch/arm64/boot/dts/freescale/wand-pi-8m.dts > @@ -0,0 +1,590 @@ // snip > + > +&i2c1 { > + clock-frequency = <100000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_i2c1>; > + status = "okay"; > + > + typec_tusb320:tusb320@47 { > + compatible = "ti,tusb320"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_tusb320_irq &pinctrl_typec_ss_sel>; > + reg = <0x47>; > + vbus-supply = <®_usb_otg_vbus>; > + ss-sel-gpios = <&gpio3 5 GPIO_ACTIVE_HIGH>; > + tusb320,int-gpio = <&gpio3 6 GPIO_ACTIVE_LOW>; > + tusb320,select-mode = <0>; > + tusb320,dfp-power = <0>; > + }; > + > + pmic: bd71837@4b { I was once told the node names should be generic :] So, I'd suggest using "pmic@4b". > + reg = <0x4b>; > + compatible = "rohm,bd71837"; > + /* PMIC BD71837 PMIC_nINT GPIO1_IO12 */ > + pinctrl-0 = <&pinctrl_pmic>; > + gpio_intr = <&gpio1 3 GPIO_ACTIVE_LOW>; > + > + bd71837,pmic-buck1-uses-i2c-dvs; > + bd71837,pmic-buck1-dvs-voltage = <900000>, <850000>, <800000>; /* VDD_SOC: Run-Idle-Suspend */ > + bd71837,pmic-buck2-uses-i2c-dvs; > + bd71837,pmic-buck2-dvs-voltage = <1000000>, <900000>, <0>; /* VDD_ARM: Run-Idle */ > + bd71837,pmic-buck3-uses-i2c-dvs; > + bd71837,pmic-buck3-dvs-voltage = <1000000>, <0>, <0>; /* VDD_GPU: Run */ > + bd71837,pmic-buck4-uses-i2c-dvs; > + bd71837,pmic-buck4-dvs-voltage = <1000000>, <0>, <0>; /* VDD_VPU: Run */ These entries should be replaced by proper properties for run-level voltage configuration. Please see the Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt and Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.txt. I think you wish to use rohm,dvs-run-voltage, rohm,dvs-idle-voltage, and rohm,dvs-suspend-voltage instead. Furthermore, I see you are not specifying rohm,reset-snvs-powered. I wonder if it is intentional to not use SNVS as reset target. Seeing you use i.MX8 and seeing used those unsupported run-level configuration properties which were present only in some very first proprietary driver draft - I expect this may not be intentional. I think that early driver defaulted to SNVS while it also failed to provide any regulator enable/disable control. > + > + gpo { > + rohm,drv = <0x0C>; /* 0b0000_1100 all gpos with cmos output mode */ > + }; What is this? > + > + regulators { > + #address-cells = <1>; > + #size-cells = <0>; > + > + buck1_reg: regulator@0 { I don't think the node names are correct. As far as I know the regulator core uses node names - please see the valid names from documentation. > + reg = <0>; > + regulator-compatible = "buck1"; I think you shouldn't use regulator-compatible. On the other hand, I think you should use regulator-name. > + regulator-min-microvolt = <700000>; > + regulator-max-microvolt = <1300000>; > + regulator-boot-on; > + regulator-always-on; > + regulator-ramp-delay = <1250>; > + }; > + > + buck2_reg: regulator@1 { > + reg = <1>; > + regulator-compatible = "buck2"; > + regulator-min-microvolt = <700000>; > + regulator-max-microvolt = <1300000>; > + regulator-boot-on; > + regulator-always-on; > + regulator-ramp-delay = <1250>; > + }; > + > + buck3_reg: regulator@2 { > + reg = <2>; > + regulator-compatible = "buck3"; > + regulator-min-microvolt = <700000>; > + regulator-max-microvolt = <1300000>; > + regulator-boot-on; > + regulator-always-on; In typical BD71837 use-cases the buck 3 is used to power graphichs accelerator. I wonder if enable/disable control should be allowed to help thermal issues and power saving? (This comment can be ignored if not applicaple to your board) > + }; > + > + buck4_reg: regulator@3 { > + reg = <3>; > + regulator-compatible = "buck4"; > + regulator-min-microvolt = <700000>; > + regulator-max-microvolt = <1300000>; > + regulator-boot-on; > + regulator-always-on; In typical BD71837 use-cases the buck 4 is used to power VPU. I wonder if enable/disable control should be allowed to help thermal issues and power saving? (This comment can be ignored if not applicaple to your board) > + }; > + > + buck5_reg: regulator@4 { > + reg = <4>; > + regulator-compatible = "buck5"; > + regulator-min-microvolt = <700000>; > + regulator-max-microvolt = <1350000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + buck6_reg: regulator@5 { > + reg = <5>; > + regulator-compatible = "buck6"; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3300000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + buck7_reg: regulator@6 { > + reg = <6>; > + regulator-compatible = "buck7"; > + regulator-min-microvolt = <1605000>; > + regulator-max-microvolt = <1995000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + buck8_reg: regulator@7 { > + reg = <7>; > + regulator-compatible = "buck8"; > + regulator-min-microvolt = <800000>; > + regulator-max-microvolt = <1400000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + ldo1_reg: regulator@8 { > + reg = <8>; > + regulator-compatible = "ldo1"; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3300000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + ldo2_reg: regulator@9 { > + reg = <9>; > + regulator-compatible = "ldo2"; > + regulator-min-microvolt = <900000>; > + regulator-max-microvolt = <900000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + ldo3_reg: regulator@10 { > + reg = <10>; > + regulator-compatible = "ldo3"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + ldo4_reg: regulator@11 { > + reg = <11>; > + regulator-compatible = "ldo4"; > + regulator-min-microvolt = <900000>; > + regulator-max-microvolt = <1800000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + ldo5_reg: regulator@12 { > + reg = <12>; > + regulator-compatible = "ldo5"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + regulator-boot-on; > + regulator-always-on; You may want to mark the BUCK6 as a supply for LDO5. > + }; > + > + ldo6_reg: regulator@13 { > + reg = <13>; > + regulator-compatible = "ldo6"; > + regulator-min-microvolt = <900000>; > + regulator-max-microvolt = <1800000>; > + regulator-boot-on; > + regulator-always-on; You may want to mark the BUCK7 as a supply for LDO6. > + }; > + > + ldo7_reg: regulator@14 { > + reg = <14>; > + regulator-compatible = "ldo7"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + }; > + }; > +}; > + Best Regards Matti Vaittinen -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =]
