| Message ID | 20180830190120.722-10-clabbe.montjoie@gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | ata: ahci_platform: support allwinner R40 AHCI | expand |
On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote: > R40 have a sata controller which is the same as A20. > This patch adds a DT node for it. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > --- > arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi > index 852c2ccc3268..d6b5820da850 100644 > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > @@ -550,6 +550,29 @@ > #size-cells = <0>; > }; > > + ahci: sata@1c18000 { > + compatible = "allwinner,sun8i-r40-ahci"; > + reg = <0x01c18000 0x1000>; > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>; > + resets = <&ccu RST_BUS_SATA>; > + resets-name = "ahci"; > + #address-cells = <1>; > + #size-cells = <0>; > + status = "disabled"; > + > + sata_port: sata-port@0 { > + reg = <0>; > + phys = <&sata_phy>; > + }; > + }; > + > + sata_phy: sata-phy@1c180c0 { > + compatible = "allwinner,sun8i-r40-sata-phy"; > + reg = <0x1c180c0 0x200>; Overlapping devices in the DTS is not ok. Maxime
On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote: > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote: > > R40 have a sata controller which is the same as A20. > > This patch adds a DT node for it. > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > --- > > arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi > > index 852c2ccc3268..d6b5820da850 100644 > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > > @@ -550,6 +550,29 @@ > > #size-cells = <0>; > > }; > > > > + ahci: sata@1c18000 { > > + compatible = "allwinner,sun8i-r40-ahci"; > > + reg = <0x01c18000 0x1000>; > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>; > > + resets = <&ccu RST_BUS_SATA>; > > + resets-name = "ahci"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + status = "disabled"; > > + > > + sata_port: sata-port@0 { > > + reg = <0>; > > + phys = <&sata_phy>; > > + }; > > + }; > > + > > + sata_phy: sata-phy@1c180c0 { > > + compatible = "allwinner,sun8i-r40-sata-phy"; > > + reg = <0x1c180c0 0x200>; > > Overlapping devices in the DTS is not ok. > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 phy@e900a0) But since it is not a good justification, it seems that regmap is my only solution ? Regards
On Fri, Aug 31, 2018 at 3:56 PM Corentin Labbe <clabbe.montjoie@gmail.com> wrote: > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote: > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote: > > > R40 have a sata controller which is the same as A20. > > > This patch adds a DT node for it. > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > --- > > > arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++ > > > 1 file changed, 23 insertions(+) > > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi > > > index 852c2ccc3268..d6b5820da850 100644 > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > > > @@ -550,6 +550,29 @@ > > > #size-cells = <0>; > > > }; > > > > > > + ahci: sata@1c18000 { > > > + compatible = "allwinner,sun8i-r40-ahci"; > > > + reg = <0x01c18000 0x1000>; > > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>; > > > + resets = <&ccu RST_BUS_SATA>; > > > + resets-name = "ahci"; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + status = "disabled"; > > > + > > > + sata_port: sata-port@0 { > > > + reg = <0>; > > > + phys = <&sata_phy>; > > > + }; > > > + }; > > > + > > > + sata_phy: sata-phy@1c180c0 { > > > + compatible = "allwinner,sun8i-r40-sata-phy"; > > > + reg = <0x1c180c0 0x200>; > > > > Overlapping devices in the DTS is not ok. > > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 phy@e900a0) > But since it is not a good justification, it seems that regmap is my only solution ? Since you are effectively splitting one device node into two, you should adjust the original (ahci) device nodes register range. ChenYu
On Fri, Aug 31, 2018 at 03:58:34PM +0800, Chen-Yu Tsai wrote: > On Fri, Aug 31, 2018 at 3:56 PM Corentin Labbe > <clabbe.montjoie@gmail.com> wrote: > > > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote: > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote: > > > > R40 have a sata controller which is the same as A20. > > > > This patch adds a DT node for it. > > > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > > --- > > > > arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++ > > > > 1 file changed, 23 insertions(+) > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > index 852c2ccc3268..d6b5820da850 100644 > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > @@ -550,6 +550,29 @@ > > > > #size-cells = <0>; > > > > }; > > > > > > > > + ahci: sata@1c18000 { > > > > + compatible = "allwinner,sun8i-r40-ahci"; > > > > + reg = <0x01c18000 0x1000>; > > > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>; > > > > + resets = <&ccu RST_BUS_SATA>; > > > > + resets-name = "ahci"; > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + status = "disabled"; > > > > + > > > > + sata_port: sata-port@0 { > > > > + reg = <0>; > > > > + phys = <&sata_phy>; > > > > + }; > > > > + }; > > > > + > > > > + sata_phy: sata-phy@1c180c0 { > > > > + compatible = "allwinner,sun8i-r40-sata-phy"; > > > > + reg = <0x1c180c0 0x200>; > > > > > > Overlapping devices in the DTS is not ok. > > > > > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 phy@e900a0) > > But since it is not a good justification, it seems that regmap is my only solution ? > > Since you are effectively splitting one device node into two, you should > adjust the original (ahci) device nodes register range. > I cannot do that if I remove patch 13, iow If I keep phy init code in both place as you requested.
On Fri, Aug 31, 2018 at 09:56:31AM +0200, Corentin Labbe wrote: > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote: > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote: > > > R40 have a sata controller which is the same as A20. > > > This patch adds a DT node for it. > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > --- > > > arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++ > > > 1 file changed, 23 insertions(+) > > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi > > > index 852c2ccc3268..d6b5820da850 100644 > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > > > @@ -550,6 +550,29 @@ > > > #size-cells = <0>; > > > }; > > > > > > + ahci: sata@1c18000 { > > > + compatible = "allwinner,sun8i-r40-ahci"; > > > + reg = <0x01c18000 0x1000>; > > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>; > > > + resets = <&ccu RST_BUS_SATA>; > > > + resets-name = "ahci"; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + status = "disabled"; > > > + > > > + sata_port: sata-port@0 { > > > + reg = <0>; > > > + phys = <&sata_phy>; > > > + }; > > > + }; > > > + > > > + sata_phy: sata-phy@1c180c0 { > > > + compatible = "allwinner,sun8i-r40-sata-phy"; > > > + reg = <0x1c180c0 0x200>; > > > > Overlapping devices in the DTS is not ok. > > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 > phy@e900a0) > > But since it is not a good justification, it seems that regmap is my > only solution ? I'm not even sure why you are moving the phy out of its original node (and driver). Maxime
On Fri, Aug 31, 2018 at 12:20:21PM +0200, maxime.ripard@bootlin.com wrote: > On Fri, Aug 31, 2018 at 09:56:31AM +0200, Corentin Labbe wrote: > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote: > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote: > > > > R40 have a sata controller which is the same as A20. > > > > This patch adds a DT node for it. > > > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > > --- > > > > arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++ > > > > 1 file changed, 23 insertions(+) > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > index 852c2ccc3268..d6b5820da850 100644 > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > @@ -550,6 +550,29 @@ > > > > #size-cells = <0>; > > > > }; > > > > > > > > + ahci: sata@1c18000 { > > > > + compatible = "allwinner,sun8i-r40-ahci"; > > > > + reg = <0x01c18000 0x1000>; > > > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>; > > > > + resets = <&ccu RST_BUS_SATA>; > > > > + resets-name = "ahci"; > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + status = "disabled"; > > > > + > > > > + sata_port: sata-port@0 { > > > > + reg = <0>; > > > > + phys = <&sata_phy>; > > > > + }; > > > > + }; > > > > + > > > > + sata_phy: sata-phy@1c180c0 { > > > > + compatible = "allwinner,sun8i-r40-sata-phy"; > > > > + reg = <0x1c180c0 0x200>; > > > > > > Overlapping devices in the DTS is not ok. > > > > > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 > > phy@e900a0) > > > > But since it is not a good justification, it seems that regmap is my > > only solution ? > > I'm not even sure why you are moving the phy out of its original node > (and driver). > For using the phy-supply already handled by the code. The other choice is to add another xxx-supply to ahci_platform. Or to use hackily port_regulator for this regulator.
On Fri, Aug 31, 2018 at 5:29 PM Corentin Labbe <clabbe.montjoie@gmail.com> wrote: > > On Fri, Aug 31, 2018 at 03:58:34PM +0800, Chen-Yu Tsai wrote: > > On Fri, Aug 31, 2018 at 3:56 PM Corentin Labbe > > <clabbe.montjoie@gmail.com> wrote: > > > > > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote: > > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote: > > > > > R40 have a sata controller which is the same as A20. > > > > > This patch adds a DT node for it. > > > > > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > > > --- > > > > > arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++ > > > > > 1 file changed, 23 insertions(+) > > > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > index 852c2ccc3268..d6b5820da850 100644 > > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > @@ -550,6 +550,29 @@ > > > > > #size-cells = <0>; > > > > > }; > > > > > > > > > > + ahci: sata@1c18000 { > > > > > + compatible = "allwinner,sun8i-r40-ahci"; > > > > > + reg = <0x01c18000 0x1000>; > > > > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>; > > > > > + resets = <&ccu RST_BUS_SATA>; > > > > > + resets-name = "ahci"; > > > > > + #address-cells = <1>; > > > > > + #size-cells = <0>; > > > > > + status = "disabled"; > > > > > + > > > > > + sata_port: sata-port@0 { > > > > > + reg = <0>; > > > > > + phys = <&sata_phy>; > > > > > + }; > > > > > + }; > > > > > + > > > > > + sata_phy: sata-phy@1c180c0 { > > > > > + compatible = "allwinner,sun8i-r40-sata-phy"; > > > > > + reg = <0x1c180c0 0x200>; > > > > > > > > Overlapping devices in the DTS is not ok. > > > > > > > > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 phy@e900a0) > > > But since it is not a good justification, it seems that regmap is my only solution ? > > > > Since you are effectively splitting one device node into two, you should > > adjust the original (ahci) device nodes register range. > > > > I cannot do that if I remove patch 13, iow If I keep phy init code in both place as you requested. Then you need to split the phy handling between a10 and r40. A10 doesn't need phy-supply at the moment anyway. And migrating A10 to newer binding is only causing you problems anyway. Just drop that part, and handle the R40. ChenYu
On Fri, Aug 31, 2018 at 6:54 PM Corentin Labbe <clabbe.montjoie@gmail.com> wrote: > > On Fri, Aug 31, 2018 at 12:20:21PM +0200, maxime.ripard@bootlin.com wrote: > > On Fri, Aug 31, 2018 at 09:56:31AM +0200, Corentin Labbe wrote: > > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote: > > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote: > > > > > R40 have a sata controller which is the same as A20. > > > > > This patch adds a DT node for it. > > > > > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > > > --- > > > > > arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++ > > > > > 1 file changed, 23 insertions(+) > > > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > index 852c2ccc3268..d6b5820da850 100644 > > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > @@ -550,6 +550,29 @@ > > > > > #size-cells = <0>; > > > > > }; > > > > > > > > > > + ahci: sata@1c18000 { > > > > > + compatible = "allwinner,sun8i-r40-ahci"; > > > > > + reg = <0x01c18000 0x1000>; > > > > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>; > > > > > + resets = <&ccu RST_BUS_SATA>; > > > > > + resets-name = "ahci"; > > > > > + #address-cells = <1>; > > > > > + #size-cells = <0>; > > > > > + status = "disabled"; > > > > > + > > > > > + sata_port: sata-port@0 { > > > > > + reg = <0>; > > > > > + phys = <&sata_phy>; > > > > > + }; > > > > > + }; > > > > > + > > > > > + sata_phy: sata-phy@1c180c0 { > > > > > + compatible = "allwinner,sun8i-r40-sata-phy"; > > > > > + reg = <0x1c180c0 0x200>; > > > > > > > > Overlapping devices in the DTS is not ok. > > > > > > > > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 > > > phy@e900a0) > > > > > > But since it is not a good justification, it seems that regmap is my > > > only solution ? > > > > I'm not even sure why you are moving the phy out of its original node > > (and driver). > > > > For using the phy-supply already handled by the code. > The other choice is to add another xxx-supply to ahci_platform. > Or to use hackily port_regulator for this regulator. The PHY registers are in the AHCI's "vendor specific registers" region. Following that are the per-port registers, which the ahci driver will need access to. This doesn't look like it should deserve a separate device node. What's wrong with handling the regulator directly in the ahci-sunxi PHY init code? ChenYu
On Fri, Aug 31, 2018 at 07:31:37PM +0800, Chen-Yu Tsai wrote: > On Fri, Aug 31, 2018 at 6:54 PM Corentin Labbe > <clabbe.montjoie@gmail.com> wrote: > > > > On Fri, Aug 31, 2018 at 12:20:21PM +0200, maxime.ripard@bootlin.com wrote: > > > On Fri, Aug 31, 2018 at 09:56:31AM +0200, Corentin Labbe wrote: > > > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote: > > > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote: > > > > > > R40 have a sata controller which is the same as A20. > > > > > > This patch adds a DT node for it. > > > > > > > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > > > > --- > > > > > > arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++ > > > > > > 1 file changed, 23 insertions(+) > > > > > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > index 852c2ccc3268..d6b5820da850 100644 > > > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > @@ -550,6 +550,29 @@ > > > > > > #size-cells = <0>; > > > > > > }; > > > > > > > > > > > > + ahci: sata@1c18000 { > > > > > > + compatible = "allwinner,sun8i-r40-ahci"; > > > > > > + reg = <0x01c18000 0x1000>; > > > > > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > > > > > + clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>; > > > > > > + resets = <&ccu RST_BUS_SATA>; > > > > > > + resets-name = "ahci"; > > > > > > + #address-cells = <1>; > > > > > > + #size-cells = <0>; > > > > > > + status = "disabled"; > > > > > > + > > > > > > + sata_port: sata-port@0 { > > > > > > + reg = <0>; > > > > > > + phys = <&sata_phy>; > > > > > > + }; > > > > > > + }; > > > > > > + > > > > > > + sata_phy: sata-phy@1c180c0 { > > > > > > + compatible = "allwinner,sun8i-r40-sata-phy"; > > > > > > + reg = <0x1c180c0 0x200>; > > > > > > > > > > Overlapping devices in the DTS is not ok. > > > > > > > > > > > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 > > > > phy@e900a0) > > > > > > > > But since it is not a good justification, it seems that regmap is my > > > > only solution ? > > > > > > I'm not even sure why you are moving the phy out of its original node > > > (and driver). > > > > > > > For using the phy-supply already handled by the code. > > The other choice is to add another xxx-supply to ahci_platform. > > Or to use hackily port_regulator for this regulator. > > The PHY registers are in the AHCI's "vendor specific registers" > region. Following that are the per-port registers, which the ahci > driver will need access to. This doesn't look like it should > deserve a separate device node. > > What's wrong with handling the regulator directly in the ahci-sunxi > PHY init code? > The reason are that I didnt wanted to use the port-regulator, and I didnt want to add new code to ahci_platform. I tried to place a phy-supply in the ahci node, but it doesnt work (with or without a phy subnode). Moving PHy code in a dedicated driver seemed to be good, but with all your comments and Maxime's one, it seems not. I will keep ahci_sunxi as-is and will try to found how to add the needed phy-supply. Regards
在 2018-08-31五的 19:10 +0800，Chen-Yu Tsai写道： > On Fri, Aug 31, 2018 at 5:29 PM Corentin Labbe > <clabbe.montjoie@gmail.com> wrote: > > > > On Fri, Aug 31, 2018 at 03:58:34PM +0800, Chen-Yu Tsai wrote: > > > On Fri, Aug 31, 2018 at 3:56 PM Corentin Labbe > > > <clabbe.montjoie@gmail.com> wrote: > > > > > > > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote: > > > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe > > > > > wrote: > > > > > > R40 have a sata controller which is the same as A20. > > > > > > This patch adds a DT node for it. > > > > > > > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > > > > --- > > > > > > arch/arm/boot/dts/sun8i-r40.dtsi | 23 > > > > > > +++++++++++++++++++++++ > > > > > > 1 file changed, 23 insertions(+) > > > > > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > index 852c2ccc3268..d6b5820da850 100644 > > > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > @@ -550,6 +550,29 @@ > > > > > > #size-cells = <0>; > > > > > > }; > > > > > > > > > > > > + ahci: sata@1c18000 { > > > > > > + compatible = "allwinner,sun8i-r40- > > > > > > ahci"; > > > > > > + reg = <0x01c18000 0x1000>; > > > > > > + interrupts = <GIC_SPI 56 > > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > > + clocks = <&ccu CLK_BUS_SATA>, <&ccu > > > > > > CLK_SATA>; > > > > > > + resets = <&ccu RST_BUS_SATA>; > > > > > > + resets-name = "ahci"; > > > > > > + #address-cells = <1>; > > > > > > + #size-cells = <0>; > > > > > > + status = "disabled"; > > > > > > + > > > > > > + sata_port: sata-port@0 { > > > > > > + reg = <0>; > > > > > > + phys = <&sata_phy>; > > > > > > + }; > > > > > > + }; > > > > > > + > > > > > > + sata_phy: sata-phy@1c180c0 { > > > > > > + compatible = "allwinner,sun8i-r40-sata- > > > > > > phy"; > > > > > > + reg = <0x1c180c0 0x200>; > > > > > > > > > > Overlapping devices in the DTS is not ok. > > > > > > > > > > > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 > > > > phy@e900a0) > > > > But since it is not a good justification, it seems that regmap > > > > is my only solution ? > > > > > > Since you are effectively splitting one device node into two, you > > > should > > > adjust the original (ahci) device nodes register range. > > > > > > > I cannot do that if I remove patch 13, iow If I keep phy init code > > in both place as you requested. > > Then you need to split the phy handling between a10 and r40. A10 > doesn't > need phy-supply at the moment anyway. And migrating A10 to newer > binding > is only causing you problems anyway. Just drop that part, and handle > the > R40. From the hardware perspective, they're the same. The A10/A20 has also two dedicated VDD pins for SATA, although on boards they're usually always on. > > ChenYu > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
在 2018-08-31五的 19:31 +0800，Chen-Yu Tsai写道： > On Fri, Aug 31, 2018 at 6:54 PM Corentin Labbe > <clabbe.montjoie@gmail.com> wrote: > > > > On Fri, Aug 31, 2018 at 12:20:21PM +0200, maxime.ripard@bootlin.com > > wrote: > > > On Fri, Aug 31, 2018 at 09:56:31AM +0200, Corentin Labbe wrote: > > > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote: > > > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe > > > > > wrote: > > > > > > R40 have a sata controller which is the same as A20. > > > > > > This patch adds a DT node for it. > > > > > > > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > > > > --- > > > > > > arch/arm/boot/dts/sun8i-r40.dtsi | 23 > > > > > > +++++++++++++++++++++++ > > > > > > 1 file changed, 23 insertions(+) > > > > > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > index 852c2ccc3268..d6b5820da850 100644 > > > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi > > > > > > @@ -550,6 +550,29 @@ > > > > > > #size-cells = <0>; > > > > > > }; > > > > > > > > > > > > + ahci: sata@1c18000 { > > > > > > + compatible = "allwinner,sun8i-r40- > > > > > > ahci"; > > > > > > + reg = <0x01c18000 0x1000>; > > > > > > + interrupts = <GIC_SPI 56 > > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > > + clocks = <&ccu CLK_BUS_SATA>, <&ccu > > > > > > CLK_SATA>; > > > > > > + resets = <&ccu RST_BUS_SATA>; > > > > > > + resets-name = "ahci"; > > > > > > + #address-cells = <1>; > > > > > > + #size-cells = <0>; > > > > > > + status = "disabled"; > > > > > > + > > > > > > + sata_port: sata-port@0 { > > > > > > + reg = <0>; > > > > > > + phys = <&sata_phy>; > > > > > > + }; > > > > > > + }; > > > > > > + > > > > > > + sata_phy: sata-phy@1c180c0 { > > > > > > + compatible = "allwinner,sun8i-r40- > > > > > > sata-phy"; > > > > > > + reg = <0x1c180c0 0x200>; > > > > > > > > > > Overlapping devices in the DTS is not ok. > > > > > > > > > > > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 > > > > phy@e900a0) > > > > > > > > But since it is not a good justification, it seems that regmap > > > > is my > > > > only solution ? > > > > > > I'm not even sure why you are moving the phy out of its original > > > node > > > (and driver). > > > > > > > For using the phy-supply already handled by the code. > > The other choice is to add another xxx-supply to ahci_platform. > > Or to use hackily port_regulator for this regulator. > > The PHY registers are in the AHCI's "vendor specific registers" > region. Following that are the per-port registers, which the ahci > driver will need access to. This doesn't look like it should > deserve a separate device node. > > What's wrong with handling the regulator directly in the ahci-sunxi > PHY init code? I remember I sent a patch that did this some times ago, and gets rejected. > > ChenYu
diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi index 852c2ccc3268..d6b5820da850 100644 --- a/arch/arm/boot/dts/sun8i-r40.dtsi +++ b/arch/arm/boot/dts/sun8i-r40.dtsi @@ -550,6 +550,29 @@ #size-cells = <0>; }; + ahci: sata@1c18000 { + compatible = "allwinner,sun8i-r40-ahci"; + reg = <0x01c18000 0x1000>; + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>; + resets = <&ccu RST_BUS_SATA>; + resets-name = "ahci"; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + + sata_port: sata-port@0 { + reg = <0>; + phys = <&sata_phy>; + }; + }; + + sata_phy: sata-phy@1c180c0 { + compatible = "allwinner,sun8i-r40-sata-phy"; + reg = <0x1c180c0 0x200>; + #phy-cells = <0>; + }; + gmac: ethernet@1c50000 { compatible = "allwinner,sun8i-r40-gmac"; syscon = <&ccu>;