| Message ID | 1463092552-60696-1-git-send-email-briannorris@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
? 2016/5/13 6:35, Brian Norris ??: > Add description for the SDHCI v5.1 eMMC controller on rk3399. Fix it to > 200 MHz, to support all supported timing modes. > > Note that 'rockchip,rk3399-sdhci-5.1' is not documented; we presumably > have a compliant Arasan controller, but let's have a rockchip property > as the canonical backup/precautionary measure. Per Heiko's previous > suggestion, let's not clutter the arasan doc with it. > > Signed-off-by: Brian Norris <briannorris@chromium.org> Except for adding "rockchip,rk3399-sdhci-5.1", it looks nice and keeps consistent with our local kernel-4.4 tree. Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com> > --- > v2: > > * improved commit message > * assign eMMC clock to 200 MHz > > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > index 46f325a143b0..9980c2eab4e9 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > @@ -215,6 +215,19 @@ > status = "disabled"; > }; > > + sdhci: sdhci@fe330000 { > + compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1"; > + reg = <0x0 0xfe330000 0x0 0x10000>; > + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>; > + clock-names = "clk_xin", "clk_ahb"; > + assigned-clocks = <&cru SCLK_EMMC>; > + assigned-clock-rates = <200000000>; > + phys = <&emmc_phy>; > + phy-names = "phy_arasan"; > + status = "disabled"; > + }; > + > usb_host0_ehci: usb@fe380000 { > compatible = "generic-ehci"; > reg = <0x0 0xfe380000 0x0 0x20000>; > @@ -481,8 +494,18 @@ > }; > > grf: syscon@ff770000 { > - compatible = "rockchip,rk3399-grf", "syscon"; > + compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd"; > reg = <0x0 0xff770000 0x0 0x10000>; > + > + #address-cells = <1>; > + #size-cells = <1>; > + > + emmc_phy: phy@f780 { > + compatible = "rockchip,rk3399-emmc-phy"; > + reg = <0xf780 0x20>; > + #phy-cells = <0>; > + status = "disabled"; > + }; > }; > > watchdog@ff840000 { >
Hi, On Thu, May 12, 2016 at 3:35 PM, Brian Norris <briannorris@chromium.org> wrote: > Add description for the SDHCI v5.1 eMMC controller on rk3399. Fix it to > 200 MHz, to support all supported timing modes. > > Note that 'rockchip,rk3399-sdhci-5.1' is not documented; we presumably > have a compliant Arasan controller, but let's have a rockchip property > as the canonical backup/precautionary measure. Per Heiko's previous > suggestion, let's not clutter the arasan doc with it. > > Signed-off-by: Brian Norris <briannorris@chromium.org> > --- > v2: > > * improved commit message > * assign eMMC clock to 200 MHz > > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > index 46f325a143b0..9980c2eab4e9 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > @@ -215,6 +215,19 @@ > status = "disabled"; > }; > > + sdhci: sdhci@fe330000 { > + compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1"; > + reg = <0x0 0xfe330000 0x0 0x10000>; > + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>; > + clock-names = "clk_xin", "clk_ahb"; > + assigned-clocks = <&cru SCLK_EMMC>; > + assigned-clock-rates = <200000000>; > + phys = <&emmc_phy>; > + phy-names = "phy_arasan"; > + status = "disabled"; > + }; > + > usb_host0_ehci: usb@fe380000 { > compatible = "generic-ehci"; > reg = <0x0 0xfe380000 0x0 0x20000>; > @@ -481,8 +494,18 @@ > }; > > grf: syscon@ff770000 { > - compatible = "rockchip,rk3399-grf", "syscon"; > + compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd"; > reg = <0x0 0xff770000 0x0 0x10000>; > + > + #address-cells = <1>; > + #size-cells = <1>; > + > + emmc_phy: phy@f780 { > + compatible = "rockchip,rk3399-emmc-phy"; > + reg = <0xf780 0x20>; This is slightly wrong. It should be: reg = <0xf780 0x24>; The status register is at an offset of 0x20 and is 4 bytes big, so we need room for it. After that is fixed, feel free to add my Reviewed-by tag. -Doug
Am Donnerstag, 12. Mai 2016, 15:35:51 schrieb Brian Norris: > Add description for the SDHCI v5.1 eMMC controller on rk3399. Fix it to > 200 MHz, to support all supported timing modes. > > Note that 'rockchip,rk3399-sdhci-5.1' is not documented; we presumably > have a compliant Arasan controller, but let's have a rockchip property > as the canonical backup/precautionary measure. Per Heiko's previous > suggestion, let's not clutter the arasan doc with it. > > Signed-off-by: Brian Norris <briannorris@chromium.org> At least one split is necessary. So please at least split out the simple-mfd addition into a separate patch (I should've seen that in v1 already, but sadly didn't) I'm undecided if the emmc-phy addition also should get its own patch, but I guess it can stay together with the emmc controller. > --- > v2: > > * improved commit message > * assign eMMC clock to 200 MHz > > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index > 46f325a143b0..9980c2eab4e9 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > @@ -215,6 +215,19 @@ > status = "disabled"; > }; > > + sdhci: sdhci@fe330000 { > + compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1"; > + reg = <0x0 0xfe330000 0x0 0x10000>; > + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>; > + clock-names = "clk_xin", "clk_ahb"; > + assigned-clocks = <&cru SCLK_EMMC>; > + assigned-clock-rates = <200000000>; > + phys = <&emmc_phy>; > + phy-names = "phy_arasan"; > + status = "disabled"; > + }; > + > usb_host0_ehci: usb@fe380000 { > compatible = "generic-ehci"; > reg = <0x0 0xfe380000 0x0 0x20000>; > @@ -481,8 +494,18 @@ > }; > > grf: syscon@ff770000 { > - compatible = "rockchip,rk3399-grf", "syscon"; > + compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd"; > reg = <0x0 0xff770000 0x0 0x10000>; > + > + #address-cells = <1>; > + #size-cells = <1>; > + > + emmc_phy: phy@f780 { > + compatible = "rockchip,rk3399-emmc-phy"; > + reg = <0xf780 0x20>; > + #phy-cells = <0>; > + status = "disabled"; > + }; > }; > > watchdog@ff840000 {
On Fri, May 13, 2016 at 02:42:06PM -0700, Doug Anderson wrote: > On Thu, May 12, 2016 at 3:35 PM, Brian Norris <briannorris@chromium.org> wrote: > > + emmc_phy: phy@f780 { > > + compatible = "rockchip,rk3399-emmc-phy"; > > + reg = <0xf780 0x20>; > > This is slightly wrong. It should be: > > reg = <0xf780 0x24>; > > The status register is at an offset of 0x20 and is 4 bytes big, so we > need room for it. Fixed. > After that is fixed, feel free to add my Reviewed-by tag. Done. Thanks for the review. Brian
On Fri, May 13, 2016 at 11:47:57PM +0200, Heiko Stuebner wrote: > Am Donnerstag, 12. Mai 2016, 15:35:51 schrieb Brian Norris: > > Add description for the SDHCI v5.1 eMMC controller on rk3399. Fix it to > > 200 MHz, to support all supported timing modes. > > > > Note that 'rockchip,rk3399-sdhci-5.1' is not documented; we presumably > > have a compliant Arasan controller, but let's have a rockchip property > > as the canonical backup/precautionary measure. Per Heiko's previous > > suggestion, let's not clutter the arasan doc with it. > > > > Signed-off-by: Brian Norris <briannorris@chromium.org> > > At least one split is necessary. > So please at least split out the simple-mfd addition into a separate patch Will do. BTW, should this be noted in Documentation/devicetree/bindings/soc/rockchip/grf.txt now? > (I should've seen that in v1 already, but sadly didn't) No problem. > I'm undecided if the emmc-phy addition also should get its own patch, but I > guess it can stay together with the emmc controller. I think it makes sense for them to stay together. What's a phy without a controller to use it? :) Brian
Am Freitag, 13. Mai 2016, 14:57:24 schrieb Brian Norris: > On Fri, May 13, 2016 at 11:47:57PM +0200, Heiko Stuebner wrote: > > Am Donnerstag, 12. Mai 2016, 15:35:51 schrieb Brian Norris: > > > Add description for the SDHCI v5.1 eMMC controller on rk3399. Fix it > > > to > > > 200 MHz, to support all supported timing modes. > > > > > > Note that 'rockchip,rk3399-sdhci-5.1' is not documented; we presumably > > > have a compliant Arasan controller, but let's have a rockchip property > > > as the canonical backup/precautionary measure. Per Heiko's previous > > > suggestion, let's not clutter the arasan doc with it. > > > > > > Signed-off-by: Brian Norris <briannorris@chromium.org> > > > > At least one split is necessary. > > So please at least split out the simple-mfd addition into a separate > > patch > Will do. > > BTW, should this be noted in > Documentation/devicetree/bindings/soc/rockchip/grf.txt now? The simple-mfd is already documented in mfd/mfd.txt in a general way, so I don't think every instance needs to have that, especially as it mainly affects sub-nodes alone.
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index 46f325a143b0..9980c2eab4e9 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi @@ -215,6 +215,19 @@ status = "disabled"; }; + sdhci: sdhci@fe330000 { + compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1"; + reg = <0x0 0xfe330000 0x0 0x10000>; + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>; + clock-names = "clk_xin", "clk_ahb"; + assigned-clocks = <&cru SCLK_EMMC>; + assigned-clock-rates = <200000000>; + phys = <&emmc_phy>; + phy-names = "phy_arasan"; + status = "disabled"; + }; + usb_host0_ehci: usb@fe380000 { compatible = "generic-ehci"; reg = <0x0 0xfe380000 0x0 0x20000>; @@ -481,8 +494,18 @@ }; grf: syscon@ff770000 { - compatible = "rockchip,rk3399-grf", "syscon"; + compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd"; reg = <0x0 0xff770000 0x0 0x10000>; + + #address-cells = <1>; + #size-cells = <1>; + + emmc_phy: phy@f780 { + compatible = "rockchip,rk3399-emmc-phy"; + reg = <0xf780 0x20>; + #phy-cells = <0>; + status = "disabled"; + }; }; watchdog@ff840000 {
Add description for the SDHCI v5.1 eMMC controller on rk3399. Fix it to 200 MHz, to support all supported timing modes. Note that 'rockchip,rk3399-sdhci-5.1' is not documented; we presumably have a compliant Arasan controller, but let's have a rockchip property as the canonical backup/precautionary measure. Per Heiko's previous suggestion, let's not clutter the arasan doc with it. Signed-off-by: Brian Norris <briannorris@chromium.org> --- v2: * improved commit message * assign eMMC clock to 200 MHz arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)