| Message ID | 20210804130625.15449-1-michael.riesch@wolfvision.net |
|---|---|
| Headers | show |
| Series | arm64: dts: rockchip: rk3568-evb1-v10: add sd card support | expand |
Hi Michael, Could you add a commit message to all patches in this serie? On 8/4/21 3:06 PM, Michael Riesch wrote: > Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net> > --- > v2: > - rename alias to match convention > > arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > index ed96f27c64a3..c4da6436059d 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > @@ -17,6 +17,7 @@ > ethernet0 = &gmac0; > ethernet1 = &gmac1; > mmc0 = &sdhci; > + mmc1 = &sdmmc0; mmc aliases are sort on reg address based on availability without number gap. sdmmc0: mmc@fe2b0000 {} sdhci: mmc@fe310000 {} > }; > > chosen: chosen { > @@ -353,6 +354,20 @@ > status = "okay"; > }; > > +&sdmmc0 { > + bus-width = <4>; > + cap-sd-highspeed; > + cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>; > + disable-wp; > + pinctrl-names = "default"; > + pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>; pinctrl-names below pinctrl-0 like the rest of rk356x.dtsi > + sd-uhs-sdr104; > + supports-sd; Check mmc-controller.yaml, rockchip-dw-mshc.yaml and synopsys-dw-mshc-common.yaml for properties. > + vmmc-supply = <&vcc3v3_sd>; > + vqmmc-supply = <&vccio_sd>; > + status = "okay"; > +}; > + > &uart2 { > status = "okay"; > }; >
Hi Michael, Missing commit message. On 8/4/21 3:06 PM, Michael Riesch wrote: > Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net> > --- > .../boot/dts/rockchip/rk3568-evb1-v10.dts | 206 ++++++++++++++++++ > 1 file changed, 206 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > index 65e536c78d2e..f682144a1892 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > @@ -104,6 +104,203 @@ > status = "okay"; > }; > > +&i2c0 { > + status = "okay"; > + > + rk809: pmic@20 { > + compatible = "rockchip,rk809"; > + reg = <0x20>; > + interrupt-parent = <&gpio0>; > + interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>; > + #clock-cells = <1>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pmic_int>; pinctrl-names below pinctrl-0 like the rest of rk356x.dtsi > + rockchip,system-power-controller; > + wakeup-source; > + > + vcc1-supply = <&vcc3v3_sys>; > + vcc2-supply = <&vcc3v3_sys>; > + vcc3-supply = <&vcc3v3_sys>; > + vcc4-supply = <&vcc3v3_sys>; > + vcc5-supply = <&vcc3v3_sys>; > + vcc6-supply = <&vcc3v3_sys>; > + vcc7-supply = <&vcc3v3_sys>; > + vcc8-supply = <&vcc3v3_sys>; > + vcc9-supply = <&vcc3v3_sys>; > + > + regulators { > + vdd_logic: DCDC_REG1 { > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <500000>; > + regulator-max-microvolt = <1350000>; > + regulator-init-microvolt = <900000>; > + regulator-ramp-delay = <6001>; > + regulator-initial-mode = <0x2>; > + regulator-name = "vdd_logic"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vdd_gpu: DCDC_REG2 { > + regulator-min-microvolt = <500000>; > + regulator-max-microvolt = <1350000>; > + regulator-init-microvolt = <900000>; > + regulator-ramp-delay = <6001>; > + regulator-initial-mode = <0x2>; > + regulator-name = "vdd_gpu"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc_ddr: DCDC_REG3 { > + regulator-always-on; > + regulator-boot-on; > + regulator-initial-mode = <0x2>; > + regulator-name = "vcc_ddr"; > + regulator-state-mem { > + regulator-on-in-suspend; > + }; > + }; > + > + vdd_npu: DCDC_REG4 { > + regulator-min-microvolt = <500000>; > + regulator-max-microvolt = <1350000>; Exception to the sort rule: 1: regulator-min-microvolt above regulator-max-microvolt 2: regulator-name above all other regulator properties. The rest in alphabetical order. Fix them all. > + regulator-init-microvolt = <900000>; > + regulator-ramp-delay = <6001>; > + regulator-initial-mode = <0x2>; > + regulator-name = "vdd_npu"; Add empty line between properties and a node. > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc_1v8: DCDC_REG5 { > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-name = "vcc_1v8"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vdda0v9_image: LDO_REG1 { > + regulator-min-microvolt = <900000>; > + regulator-max-microvolt = <900000>; > + regulator-name = "vdda0v9_image"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vdda_0v9: LDO_REG2 { > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <900000>; > + regulator-max-microvolt = <900000>; > + regulator-name = "vdda_0v9"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vdda0v9_pmu: LDO_REG3 { > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <900000>; > + regulator-max-microvolt = <900000>; > + regulator-name = "vdda0v9_pmu"; > + regulator-state-mem { > + regulator-on-in-suspend; > + regulator-suspend-microvolt = <900000>; > + }; > + }; > + > + vccio_acodec: LDO_REG4 { > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-name = "vccio_acodec"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vccio_sd: LDO_REG5 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + regulator-name = "vccio_sd"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc3v3_pmu: LDO_REG6 { > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-name = "vcc3v3_pmu"; > + regulator-state-mem { > + regulator-on-in-suspend; > + regulator-suspend-microvolt = <3300000>; > + }; > + }; > + > + vcca_1v8: LDO_REG7 { > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-name = "vcca_1v8"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcca1v8_pmu: LDO_REG8 { > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-name = "vcca1v8_pmu"; > + regulator-state-mem { > + regulator-on-in-suspend; > + regulator-suspend-microvolt = <1800000>; > + }; > + }; > + > + vcca1v8_image: LDO_REG9 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-name = "vcca1v8_image"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc_3v3: SWITCH_REG1 { > + regulator-always-on; > + regulator-boot-on; > + regulator-name = "vcc_3v3"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc3v3_sd: SWITCH_REG2 { > + regulator-name = "vcc3v3_sd"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + }; > + }; > +}; > + > &mdio0 { > rgmii_phy0: ethernet-phy@0 { > compatible = "ethernet-phy-ieee802.3-c22"; > @@ -124,6 +321,15 @@ > }; > }; > > +&pinctrl { > + pmic { > + pmic_int: pmic_int { > + rockchip,pins = > + <0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>; > + }; > + }; > +}; > + > &sdhci { > bus-width = <8>; > max-frequency = <200000000>; >
Hi Michael,
pmu_io_domains is a sub node of pmugrf, so add it to grf.yaml as well in
a separate patch. Place document changes before driver patches for
checkpatch scripts (undocumented compatible string):
./scripts/checkpatch.pl --strict <patch1> <patch2>
- if:
properties:
compatible:
contains:
enum:
- rockchip,px30-pmugrf
- rockchip,px30-grf
- rockchip,rk3228-grf
- rockchip,rk3288-grf
- rockchip,rk3328-grf
- rockchip,rk3368-pmugrf
- rockchip,rk3368-grf
- rockchip,rk3399-pmugrf
- rockchip,rk3399-grf
==>
- rockchip,rk3568-pmugrf
then:
properties:
io-domains:
type: object
$ref: "/schemas/power/rockchip-io-domain.yaml#"
On 8/4/21 3:06 PM, Michael Riesch wrote:
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
> arch/arm64/boot/dts/rockchip/rk356x.dtsi | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 3e90a8832bb9..834863940eba 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -203,6 +203,11 @@
> pmugrf: syscon@fdc20000 {
> compatible = "rockchip,rk3568-pmugrf", "syscon", "simple-mfd";
> reg = <0x0 0xfdc20000 0x0 0x10000>;
> +
> + pmu_io_domains: io-domains {
> + compatible = "rockchip,rk3568-pmu-io-voltage-domain";
> + status = "disabled";
> + };
> };
>
> grf: syscon@fdc60000 {
>
Hi Johan, Thanks for your comments, I'll try to implement the requested changes and prepare a v3 tomorrow. On 8/4/21 4:30 PM, Johan Jonker wrote: > Hi Michael, > > Could you add a commit message to all patches in this serie? Well the short commit message (i.e., the subject line) pretty much wraps it all up in my opinion, hence no need for an extended commit message. Is there anything in particular you would like to see or have explained that requires an extended message? I would like to refrain from adding an extended commit message just for the sake of having one. > On 8/4/21 3:06 PM, Michael Riesch wrote: >> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net> >> --- >> v2: >> - rename alias to match convention >> >> arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts >> index ed96f27c64a3..c4da6436059d 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts >> +++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts >> @@ -17,6 +17,7 @@ >> ethernet0 = &gmac0; >> ethernet1 = &gmac1; > >> mmc0 = &sdhci; >> + mmc1 = &sdmmc0; > > mmc aliases are sort on reg address based on availability without number > gap. > > sdmmc0: mmc@fe2b0000 {} > sdhci: mmc@fe310000 {} I'll turn these around. >> }; >> >> chosen: chosen { >> @@ -353,6 +354,20 @@ >> status = "okay"; >> }; >> >> +&sdmmc0 { >> + bus-width = <4>; >> + cap-sd-highspeed; >> + cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>; >> + disable-wp; > >> + pinctrl-names = "default"; >> + pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>; > > pinctrl-names below pinctrl-0 like the rest of rk356x.dtsi OK! >> + sd-uhs-sdr104; > >> + supports-sd; > > Check mmc-controller.yaml, rockchip-dw-mshc.yaml and > synopsys-dw-mshc-common.yaml for properties. I am afraid I don't quite follow. What exactly should I check? I am pretty sure that the properties I used are described in the mentioned yaml files. Regards, Michael > >> + vmmc-supply = <&vcc3v3_sd>; >> + vqmmc-supply = <&vccio_sd>; >> + status = "okay"; >> +}; >> + >> &uart2 { >> status = "okay"; >> }; >>
Hi Michael, Am Mittwoch, 4. August 2021, 20:49:45 CEST schrieb Michael Riesch: > Hi Johan, > > Thanks for your comments, I'll try to implement the requested changes > and prepare a v3 tomorrow. > > On 8/4/21 4:30 PM, Johan Jonker wrote: > > Hi Michael, > > > > Could you add a commit message to all patches in this serie? > > Well the short commit message (i.e., the subject line) pretty much wraps > it all up in my opinion, hence no need for an extended commit message. > Is there anything in particular you would like to see or have explained > that requires an extended message? I would like to refrain from adding > an extended commit message just for the sake of having one. it's just a matter of style, and yes having a non-empty commit message is preferred in most parts of the kernel. Even if it's just a simple one-liner ;-), for example "Enable the sdmmc node on the rk3568-evb1 with the 4 lanes connected on it" Heiko > > > On 8/4/21 3:06 PM, Michael Riesch wrote: > >> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net> > >> --- > >> v2: > >> - rename alias to match convention > >> > >> arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts | 15 +++++++++++++++ > >> 1 file changed, 15 insertions(+) > >> > >> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > >> index ed96f27c64a3..c4da6436059d 100644 > >> --- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > >> +++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > >> @@ -17,6 +17,7 @@ > >> ethernet0 = &gmac0; > >> ethernet1 = &gmac1; > > > >> mmc0 = &sdhci; > >> + mmc1 = &sdmmc0; > > > > mmc aliases are sort on reg address based on availability without number > > gap. > > > > sdmmc0: mmc@fe2b0000 {} > > sdhci: mmc@fe310000 {} > > I'll turn these around. > > >> }; > >> > >> chosen: chosen { > >> @@ -353,6 +354,20 @@ > >> status = "okay"; > >> }; > >> > >> +&sdmmc0 { > >> + bus-width = <4>; > >> + cap-sd-highspeed; > >> + cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>; > >> + disable-wp; > > > >> + pinctrl-names = "default"; > >> + pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>; > > > > pinctrl-names below pinctrl-0 like the rest of rk356x.dtsi > > OK! > > >> + sd-uhs-sdr104; > > > >> + supports-sd; > > > > Check mmc-controller.yaml, rockchip-dw-mshc.yaml and > > synopsys-dw-mshc-common.yaml for properties. > > I am afraid I don't quite follow. What exactly should I check? I am > pretty sure that the properties I used are described in the mentioned > yaml files. > > Regards, Michael > > > > >> + vmmc-supply = <&vcc3v3_sd>; > >> + vqmmc-supply = <&vccio_sd>; > >> + status = "okay"; > >> +}; > >> + > >> &uart2 { > >> status = "okay"; > >> }; > >> >
Hi all, This series enables the SD card reader on the RK3568 EVB1 and completes the support for the on-board eMMC. As the PMU IO domains are required, the patch series that introduces support for the RK3568 [1] has been integrated in this series to bring this mainline. Best regards, Michael [1] https://patchwork.kernel.org/project/linux-rockchip/list/?series=489383 v2: - rename alias to match convention - add support for rk3568 io domains Jianqun Xu (1): soc: rockchip: io-domain: add rk3568 support Michael Riesch (6): dt-bindings: power: add rk3568-pmu-io-domain support arm64: dts: rockchip: rk3568-evb1-v10: add regulators of rk809 pmic arm64: dts: rockchip: enable io domains for rk356x arm64: dts: rockchip: rk3568-evb1-v10: enable io domains arm64: dts: rockchip: rk3568-evb1-v10: add pinctrl and alias to emmc node arm64: dts: rockchip: rk3568-evb1-v10: add node for sd card .../bindings/power/rockchip-io-domain.yaml | 30 +++ .../boot/dts/rockchip/rk3568-evb1-v10.dts | 237 ++++++++++++++++++ arch/arm64/boot/dts/rockchip/rk356x.dtsi | 5 + drivers/soc/rockchip/io-domain.c | 88 ++++++- 4 files changed, 352 insertions(+), 8 deletions(-)