Re: [PATCH 15/15] arm64: dts: marvell: Convert 7k/8k usb-phy properties to phy-supply

[Martin Blumenstingl <martin.blumenstingl@googlemail.com>]

Hi Miquel,

On Mon, Apr 8, 2019 at 10:51 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Rob,
>
> Gentle ping on the questions below, if you have some time to check.
>
> Thank you very much,
> Miquèl
>
> Miquel Raynal <miquel.raynal@bootlin.com> wrote on Wed, 3 Apr 2019
> 10:06:42 +0200:
>
> > Hi Martin,
> >
> > Rob, a few questions for you below :)
> >
> > Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote on Tue,
> > 2 Apr 2019 21:35:26 +0200:
> >
> > > Hi Miquel,
> > >
> > > thank you for keeping me in the loop!
> > >
> > > On Mon, Apr 1, 2019 at 6:52 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >
> > > > Update Aramda 7k/8k DTs to use the phy-supply property of the (recent)
> > > > generic PHY framework instead of the (legacy) usb-phy preperty. Both
> > > > enable the supply when the PHY is enabled.
> > > >
> > > > The COMPHY nodes only provide SERDES lanes configuration. The power
> > > > supply that is represented by the phy-supply property is just a
> > > > regulator wired to the USB connector, hence the creation of connector
> > > > nodes as child of the COMPHY nodes and the supply attached to it.
> > > shouldn't this also be reflected in the dt-bindings?
> >
> > I don't think it deserves an update in the bindings as this is just an
> > update for a better hardware representation. The COMPHY block is
> > already documented, the connector one too. I thought adding a connector
> > node in the COMPHY node was okay when I read Rob's answer to your
> > series as this has nothing to do with the IP itself but is just
> > describing how the board is built. Maybe Rob can confirm this?
> >
> > >
> > > > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  .../arm64/boot/dts/marvell/armada-7040-db.dts | 26 ++++++++++---------
> > > >  .../marvell/armada-8040-clearfog-gt-8k.dts    | 13 +++++-----
> > > >  .../arm64/boot/dts/marvell/armada-8040-db.dts | 13 +++++-----
> > > >  .../boot/dts/marvell/armada-8040-mcbin.dtsi   | 13 +++++-----
> > > >  4 files changed, 35 insertions(+), 30 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/marvell/armada-7040-db.dts b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> > > > index 131ce4229db0..4b8df359b1cc 100644
> > > > --- a/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> > > > +++ b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> > > > @@ -45,16 +45,6 @@
> > > >                 enable-active-high;
> > > >                 gpio = <&expander0 1 GPIO_ACTIVE_HIGH>;
> > > >         };
> > > > -
> > > > -       cp0_usb3_0_phy: cp0-usb3-0-phy {
> > > > -               compatible = "usb-nop-xceiv";
> > > > -               vcc-supply = <&cp0_reg_usb3_0_vbus>;
> > > > -       };
> > > > -
> > > > -       cp0_usb3_1_phy: cp0-usb3-1-phy {
> > > > -               compatible = "usb-nop-xceiv";
> > > > -               vcc-supply = <&cp0_reg_usb3_1_vbus>;
> > > > -       };
> > > >  };
> > > >
> > > >  &i2c0 {
> > > > @@ -200,15 +190,27 @@
> > > >         };
> > > >  };
> > > >
> > > > +&cp0_comphy1 {
> > > > +       cp0_usbh0_con: connector {
> > > > +               compatible = "usb-a-connector";
> > > > +               phy-supply = <&cp0_reg_usb3_0_vbus>;
> > > > +       };
> > > > +};
> > > (disclaimer: I don't have any board with a marvell SoC, so I don't
> > > understand how it works and I can't debug it)
> > > I know about the "phy-supply" property inside the PHY node itself
> > > (that would be cp0_comphy1 in this case).
> > > The connector binding does not mention a phy-supply property:
> > > Documentation/devicetree/bindings/connector/usb-connector.txt
> >
> > That's right, can Rob confirm that this is valid? If yes I can add an
> > optional "phy-supply" property there.
Icenowy (here: [0]) and Chunfeng (here: [1]) are working on this as
far as I can tell
Icenowy is parsing the "vbus-supply" regulator manually from within
the PHY driver, see [2]

the additional info from their patches doesn't fully answer my questions yet.
I CC'ed both to this mail hoping that they can provide additional
insight what the greater picture is (at least in terms of which driver
is controlling the USB VBUS signal) on Allwinner and Mediatek SoCs.


Regards
Martin


[0] https://patchwork.kernel.org/patch/10887839/
[1] https://patchwork.kernel.org/patch/10844251/
[2] https://patchwork.kernel.org/patch/10887841/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel