diff mbox series

[RFCv1,1/3] phy: meson8b-usb2: Enable otg phy controller on Meson8b SoCs

Message ID 20190123151209.2080-2-linux.amoon@gmail.com (mailing list archive)
State Not Applicable
Headers show
Series Odroid c1+ usb fixs | expand

Commit Message

Anand Moon Jan. 23, 2019, 3:12 p.m. UTC
Meson8b support two USB phy controller, one with otg port and
other usb host controller. USB OTG controller ie. otg port can
be used to power on board or host port for usb device.
Changes enable usb otg host controller when dr_mode is set to otg.
Currenly otg port can be used to power on the board.

Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
New patch in this series.
--TOTO enable OTG port to work as host.
--Currently we can power on the board from this OTG port.

/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M
    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
        |__ Port 1: Dev 4, If 0, Class=Mass Storage, Driver=usb-storage, 480M
        |__ Port 3: Dev 3, If 0, Class=Video, Driver=uvcvideo, 480M
        |__ Port 3: Dev 3, If 1, Class=Video, Driver=uvcvideo, 480M
        |__ Port 3: Dev 3, If 2, Class=Audio, Driver=snd-usb-audio, 480M
        |__ Port 3: Dev 3, If 3, Class=Audio, Driver=snd-usb-audio, 480M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M
---
 drivers/phy/amlogic/phy-meson8b-usb2.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Martin Blumenstingl Feb. 4, 2019, 1:20 p.m. UTC | #1
Hi Anand,

On Wed, Jan 23, 2019 at 4:12 PM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Meson8b support two USB phy controller, one with otg port and
> other usb host controller. USB OTG controller ie. otg port can
> be used to power on board or host port for usb device.
> Changes enable usb otg host controller when dr_mode is set to otg.
can you please add a sentence explaining *why* this allows enabling USB OTG?

> Currenly otg port can be used to power on the board.
typo, should be "Currently"

> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> New patch in this series.
> --TOTO enable OTG port to work as host.
> --Currently we can power on the board from this OTG port.
>
> /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
>         |__ Port 1: Dev 4, If 0, Class=Mass Storage, Driver=usb-storage, 480M
>         |__ Port 3: Dev 3, If 0, Class=Video, Driver=uvcvideo, 480M
>         |__ Port 3: Dev 3, If 1, Class=Video, Driver=uvcvideo, 480M
>         |__ Port 3: Dev 3, If 2, Class=Audio, Driver=snd-usb-audio, 480M
>         |__ Port 3: Dev 3, If 3, Class=Audio, Driver=snd-usb-audio, 480M
> /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M
> ---
>  drivers/phy/amlogic/phy-meson8b-usb2.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
> index 9c01b7e19b06..1333c77e9172 100644
> --- a/drivers/phy/amlogic/phy-meson8b-usb2.c
> +++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
> @@ -109,6 +109,7 @@
>
>  #define RESET_COMPLETE_TIME                            500
>  #define ACA_ENABLE_COMPLETE_TIME                       50
> +#define DCD_ENABLE_COMPLETE_TIME                       10
>
>  struct phy_meson8b_usb2_priv {
>         void __iomem            *regs;
> @@ -188,6 +189,20 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
>
>                 udelay(ACA_ENABLE_COMPLETE_TIME);
>
> +               if (phy_meson8b_usb2_read(priv, REG_ADP_BC) &
> +                       REG_ADP_BC_ACA_PIN_FLOAT) {
> +                       dev_warn(&phy->dev, "USB ID detect failed!\n");
> +                       clk_disable_unprepare(priv->clk_usb);
> +                       clk_disable_unprepare(priv->clk_usb_general);
> +                       return -EINVAL;
> +               }
> +       } else if (priv->dr_mode == USB_DR_MODE_OTG) {
> +               phy_meson8b_usb2_mask_bits(priv, REG_ADP_BC,
> +                                          REG_ADP_BC_DCD_ENABLE,
> +                                          REG_ADP_BC_DCD_ENABLE);
where did you take this from?
$ grep -iR DCD arm-src-kernel-2015-01-15-321cfb5a46/drivers/amlogic/usb/dwc_otg/310/
-> I only find code which clears REG_ADP_BC_DCD_ENABLE (your patch sets it):
  adp_bc.b.dcd_enable = 0;

> +               udelay(DCD_ENABLE_COMPLETE_TIME);
from Amlogic's code: DWC_MDELAY(T_DCD_TIMEOUT);
while you are also using a delay of "10" the unit is different (your
patch: us, Amlogic: ms)


Regards
Martin
Anand Moon Feb. 4, 2019, 8:02 p.m. UTC | #2
Hi Martin,

Thanks for your review comment.
Please find my comments below.

On Mon, 4 Feb 2019 at 18:51, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Wed, Jan 23, 2019 at 4:12 PM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Meson8b support two USB phy controller, one with otg port and
> > other usb host controller. USB OTG controller ie. otg port can
> > be used to power on board or host port for usb device.
> > Changes enable usb otg host controller when dr_mode is set to otg.
> can you please add a sentence explaining *why* this allows enabling USB OTG?
>
> > Currenly otg port can be used to power on the board.
> typo, should be "Currently"

Ok I will fix this next version.

>
> > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > Cc: Jerome Brunet <jbrunet@baylibre.com>
> > Cc: Neil Armstrong <narmstrong@baylibre.com>
> > Cc: Kevin Hilman <khilman@baylibre.com>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > New patch in this series.
> > --TOTO enable OTG port to work as host.
> > --Currently we can power on the board from this OTG port.
> >
> > /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M
> >     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
> >         |__ Port 1: Dev 4, If 0, Class=Mass Storage, Driver=usb-storage, 480M
> >         |__ Port 3: Dev 3, If 0, Class=Video, Driver=uvcvideo, 480M
> >         |__ Port 3: Dev 3, If 1, Class=Video, Driver=uvcvideo, 480M
> >         |__ Port 3: Dev 3, If 2, Class=Audio, Driver=snd-usb-audio, 480M
> >         |__ Port 3: Dev 3, If 3, Class=Audio, Driver=snd-usb-audio, 480M
> > /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M
> > ---
> >  drivers/phy/amlogic/phy-meson8b-usb2.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
> > index 9c01b7e19b06..1333c77e9172 100644
> > --- a/drivers/phy/amlogic/phy-meson8b-usb2.c
> > +++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
> > @@ -109,6 +109,7 @@
> >
> >  #define RESET_COMPLETE_TIME                            500
> >  #define ACA_ENABLE_COMPLETE_TIME                       50
> > +#define DCD_ENABLE_COMPLETE_TIME                       10
> >
> >  struct phy_meson8b_usb2_priv {
> >         void __iomem            *regs;
> > @@ -188,6 +189,20 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
> >
> >                 udelay(ACA_ENABLE_COMPLETE_TIME);
> >
> > +               if (phy_meson8b_usb2_read(priv, REG_ADP_BC) &
> > +                       REG_ADP_BC_ACA_PIN_FLOAT) {
> > +                       dev_warn(&phy->dev, "USB ID detect failed!\n");
> > +                       clk_disable_unprepare(priv->clk_usb);
> > +                       clk_disable_unprepare(priv->clk_usb_general);
> > +                       return -EINVAL;
> > +               }
> > +       } else if (priv->dr_mode == USB_DR_MODE_OTG) {
> > +               phy_meson8b_usb2_mask_bits(priv, REG_ADP_BC,
> > +                                          REG_ADP_BC_DCD_ENABLE,
> > +                                          REG_ADP_BC_DCD_ENABLE);
> where did you take this from?

As per the schematic it's otg phy,
I study the code to enable "host" mode for phy1 and on similar line I
tried to enable otg phy.
this changes will come into effect when we set dr_mode="otg" in dts.
but it do not enable this as host port.

*Second thought this enable DCD which seem the be wrong. Thanks for porting out*
We could set this to 0 at initial state for now and it will work as it should.
I will send the correct fix later.

> $ grep -iR DCD arm-src-kernel-2015-01-15-321cfb5a46/drivers/amlogic/usb/dwc_otg/310/
> -> I only find code which clears REG_ADP_BC_DCD_ENABLE (your patch sets it):
>   adp_bc.b.dcd_enable = 0;

This usb port act as a changer port but it also depend on the how this
port is tuned to various states as shown below.

Standard Downstream Port (SDP) is a downstream port that complies
              with the USB 2.0 or USB 3.0 definition of a host or hub.
Charging Downstream Port (CDP) is a downstream port that complies
              with the USB 2.0 or USB 3.0 definition of a host or a hub.
Dedicated Charging Port (DCP)  is a downstream port on a device
              that outputs power through a USB connector.

We cannot observe this now because this behave like a states machine which
is possible handled both in kernel driver and u-boot of Amlogic.
As per schematics this (page 7)  we have a external jumper set to
control the power path of the port.

Odroid Wiki also says so.
[0] https://wiki.odroid.com/odroid-c1/hardware/otg_power_enable

>
> > +               udelay(DCD_ENABLE_COMPLETE_TIME);
> from Amlogic's code: DWC_MDELAY(T_DCD_TIMEOUT);
> while you are also using a delay of "10" the unit is different (your
> patch: us, Amlogic: ms)
>

This timeout is basically used in the state machine in the kernel and
u-boot to tune the phy
Not sure this changes will affect the phy power state.

>
> Regards
> Martin

Best Regards


-Anand
Martin Blumenstingl Feb. 4, 2019, 8:26 p.m. UTC | #3
Hi Anand,

On Mon, Feb 4, 2019 at 9:03 PM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Martin,
>
> Thanks for your review comment.
> Please find my comments below.
>
> On Mon, 4 Feb 2019 at 18:51, Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> >
> > Hi Anand,
> >
> > On Wed, Jan 23, 2019 at 4:12 PM Anand Moon <linux.amoon@gmail.com> wrote:
> > >
> > > Meson8b support two USB phy controller, one with otg port and
> > > other usb host controller. USB OTG controller ie. otg port can
> > > be used to power on board or host port for usb device.
> > > Changes enable usb otg host controller when dr_mode is set to otg.
> > can you please add a sentence explaining *why* this allows enabling USB OTG?
> >
> > > Currenly otg port can be used to power on the board.
> > typo, should be "Currently"
>
> Ok I will fix this next version.
>
> >
> > > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > > Cc: Jerome Brunet <jbrunet@baylibre.com>
> > > Cc: Neil Armstrong <narmstrong@baylibre.com>
> > > Cc: Kevin Hilman <khilman@baylibre.com>
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > ---
> > > New patch in this series.
> > > --TOTO enable OTG port to work as host.
> > > --Currently we can power on the board from this OTG port.
> > >
> > > /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M
> > >     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
> > >         |__ Port 1: Dev 4, If 0, Class=Mass Storage, Driver=usb-storage, 480M
> > >         |__ Port 3: Dev 3, If 0, Class=Video, Driver=uvcvideo, 480M
> > >         |__ Port 3: Dev 3, If 1, Class=Video, Driver=uvcvideo, 480M
> > >         |__ Port 3: Dev 3, If 2, Class=Audio, Driver=snd-usb-audio, 480M
> > >         |__ Port 3: Dev 3, If 3, Class=Audio, Driver=snd-usb-audio, 480M
> > > /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M
> > > ---
> > >  drivers/phy/amlogic/phy-meson8b-usb2.c | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
> > > index 9c01b7e19b06..1333c77e9172 100644
> > > --- a/drivers/phy/amlogic/phy-meson8b-usb2.c
> > > +++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
> > > @@ -109,6 +109,7 @@
> > >
> > >  #define RESET_COMPLETE_TIME                            500
> > >  #define ACA_ENABLE_COMPLETE_TIME                       50
> > > +#define DCD_ENABLE_COMPLETE_TIME                       10
> > >
> > >  struct phy_meson8b_usb2_priv {
> > >         void __iomem            *regs;
> > > @@ -188,6 +189,20 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
> > >
> > >                 udelay(ACA_ENABLE_COMPLETE_TIME);
> > >
> > > +               if (phy_meson8b_usb2_read(priv, REG_ADP_BC) &
> > > +                       REG_ADP_BC_ACA_PIN_FLOAT) {
> > > +                       dev_warn(&phy->dev, "USB ID detect failed!\n");
> > > +                       clk_disable_unprepare(priv->clk_usb);
> > > +                       clk_disable_unprepare(priv->clk_usb_general);
> > > +                       return -EINVAL;
> > > +               }
> > > +       } else if (priv->dr_mode == USB_DR_MODE_OTG) {
> > > +               phy_meson8b_usb2_mask_bits(priv, REG_ADP_BC,
> > > +                                          REG_ADP_BC_DCD_ENABLE,
> > > +                                          REG_ADP_BC_DCD_ENABLE);
> > where did you take this from?
>
> As per the schematic it's otg phy,
> I study the code to enable "host" mode for phy1 and on similar line I
> tried to enable otg phy.
dragons ahead!
if you want to work on OTG support then I'm fine with this (from time
to time people ask about this on IRC)!
But beware that this needs more than a few changes to the PHY driver.

Amlogic's code uses a timer to poll whether the port is in HOST or
DEVICE mode and switches the mode accordingly, see [0]
however, this is not just limited to the PHY but it also requires
switching the dwc2 registers

personally I would start with getting device / peripheral mode working for usb0.
if you're lucky the PHY driver will work without any changes and you
only need to set the correct dr_mode along with the g-* properties.

> this changes will come into effect when we set dr_mode="otg" in dts.
> but it do not enable this as host port.
>
> *Second thought this enable DCD which seem the be wrong. Thanks for porting out*
> We could set this to 0 at initial state for now and it will work as it should.
> I will send the correct fix later.
>
> > $ grep -iR DCD arm-src-kernel-2015-01-15-321cfb5a46/drivers/amlogic/usb/dwc_otg/310/
> > -> I only find code which clears REG_ADP_BC_DCD_ENABLE (your patch sets it):
> >   adp_bc.b.dcd_enable = 0;
>
> This usb port act as a changer port but it also depend on the how this
> port is tuned to various states as shown below.
>
> Standard Downstream Port (SDP) is a downstream port that complies
>               with the USB 2.0 or USB 3.0 definition of a host or hub.
> Charging Downstream Port (CDP) is a downstream port that complies
>               with the USB 2.0 or USB 3.0 definition of a host or a hub.
> Dedicated Charging Port (DCP)  is a downstream port on a device
>               that outputs power through a USB connector.
>
> We cannot observe this now because this behave like a states machine which
> is possible handled both in kernel driver and u-boot of Amlogic.
> As per schematics this (page 7)  we have a external jumper set to
> control the power path of the port.
>
> Odroid Wiki also says so.
> [0] https://wiki.odroid.com/odroid-c1/hardware/otg_power_enable
this seems hard to implement, because the same wiki page also states:
"If your C1+ PCB is previous version, you need to desolder the R94 as
described in this link."


Regards
Martin


[0] https://github.com/endlessm/linux-meson/blob/0672f0b61eb92ba63c91d858a678d2c3a0bba06a/drivers/amlogic/usb/dwc_otg/310/dwc_otg_driver.c#L716
Anand Moon Feb. 5, 2019, 7:52 p.m. UTC | #4
Hi Martin,

Thanks for your views.

On Tue, 5 Feb 2019 at 01:56, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Mon, Feb 4, 2019 at 9:03 PM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Hi Martin,
> >
> > Thanks for your review comment.
> > Please find my comments below.
> >
> > On Mon, 4 Feb 2019 at 18:51, Martin Blumenstingl
> > <martin.blumenstingl@googlemail.com> wrote:
> > >
> > > Hi Anand,
> > >
> > > On Wed, Jan 23, 2019 at 4:12 PM Anand Moon <linux.amoon@gmail.com> wrote:
> > > >
> > > > Meson8b support two USB phy controller, one with otg port and
> > > > other usb host controller. USB OTG controller ie. otg port can
> > > > be used to power on board or host port for usb device.
> > > > Changes enable usb otg host controller when dr_mode is set to otg.
> > > can you please add a sentence explaining *why* this allows enabling USB OTG?
> > >
> > > > Currenly otg port can be used to power on the board.
> > > typo, should be "Currently"
> >
> > Ok I will fix this next version.
> >
> > >
> > > > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > > > Cc: Jerome Brunet <jbrunet@baylibre.com>
> > > > Cc: Neil Armstrong <narmstrong@baylibre.com>
> > > > Cc: Kevin Hilman <khilman@baylibre.com>
> > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > > ---
> > > > New patch in this series.
> > > > --TOTO enable OTG port to work as host.
> > > > --Currently we can power on the board from this OTG port.
> > > >
> > > > /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M
> > > >     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
> > > >         |__ Port 1: Dev 4, If 0, Class=Mass Storage, Driver=usb-storage, 480M
> > > >         |__ Port 3: Dev 3, If 0, Class=Video, Driver=uvcvideo, 480M
> > > >         |__ Port 3: Dev 3, If 1, Class=Video, Driver=uvcvideo, 480M
> > > >         |__ Port 3: Dev 3, If 2, Class=Audio, Driver=snd-usb-audio, 480M
> > > >         |__ Port 3: Dev 3, If 3, Class=Audio, Driver=snd-usb-audio, 480M
> > > > /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M
> > > > ---
> > > >  drivers/phy/amlogic/phy-meson8b-usb2.c | 15 +++++++++++++++
> > > >  1 file changed, 15 insertions(+)
> > > >
> > > > diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
> > > > index 9c01b7e19b06..1333c77e9172 100644
> > > > --- a/drivers/phy/amlogic/phy-meson8b-usb2.c
> > > > +++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
> > > > @@ -109,6 +109,7 @@
> > > >
> > > >  #define RESET_COMPLETE_TIME                            500
> > > >  #define ACA_ENABLE_COMPLETE_TIME                       50
> > > > +#define DCD_ENABLE_COMPLETE_TIME                       10
> > > >
> > > >  struct phy_meson8b_usb2_priv {
> > > >         void __iomem            *regs;
> > > > @@ -188,6 +189,20 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
> > > >
> > > >                 udelay(ACA_ENABLE_COMPLETE_TIME);
> > > >
> > > > +               if (phy_meson8b_usb2_read(priv, REG_ADP_BC) &
> > > > +                       REG_ADP_BC_ACA_PIN_FLOAT) {
> > > > +                       dev_warn(&phy->dev, "USB ID detect failed!\n");
> > > > +                       clk_disable_unprepare(priv->clk_usb);
> > > > +                       clk_disable_unprepare(priv->clk_usb_general);
> > > > +                       return -EINVAL;
> > > > +               }
> > > > +       } else if (priv->dr_mode == USB_DR_MODE_OTG) {
> > > > +               phy_meson8b_usb2_mask_bits(priv, REG_ADP_BC,
> > > > +                                          REG_ADP_BC_DCD_ENABLE,
> > > > +                                          REG_ADP_BC_DCD_ENABLE);
> > > where did you take this from?
> >
> > As per the schematic it's otg phy,
> > I study the code to enable "host" mode for phy1 and on similar line I
> > tried to enable otg phy.
> dragons ahead!
> if you want to work on OTG support then I'm fine with this (from time
> to time people ask about this on IRC)!
> But beware that this needs more than a few changes to the PHY driver.
>
> Amlogic's code uses a timer to poll whether the port is in HOST or
> DEVICE mode and switches the mode accordingly, see [0]
> however, this is not just limited to the PHY but it also requires
> switching the dwc2 registers

Ok thanks for the input.

>
> personally I would start with getting device / peripheral mode working for usb0.
> if you're lucky the PHY driver will work without any changes and you
> only need to set the correct dr_mode along with the g-* properties.
>

Yes that should be the goal on getting working usb0 phy and otg port.
So for now we will drop this patch and disable the usb_phy0 mode in
dts for Odroid c2
in order to get ride of the power failed WARNING.

When we get the usb0 node to work again with good phy configuration and
tuning we can re-enable the nodes. I will try to look into this in the future.

Can you share your thought on this.



> > this changes will come into effect when we set dr_mode="otg" in dts.
> > but it do not enable this as host port.
> >
> > *Second thought this enable DCD which seem the be wrong. Thanks for porting out*
> > We could set this to 0 at initial state for now and it will work as it should.
> > I will send the correct fix later.
> >
> > > $ grep -iR DCD arm-src-kernel-2015-01-15-321cfb5a46/drivers/amlogic/usb/dwc_otg/310/
> > > -> I only find code which clears REG_ADP_BC_DCD_ENABLE (your patch sets it):
> > >   adp_bc.b.dcd_enable = 0;
> >
> > This usb port act as a changer port but it also depend on the how this
> > port is tuned to various states as shown below.
> >
> > Standard Downstream Port (SDP) is a downstream port that complies
> >               with the USB 2.0 or USB 3.0 definition of a host or hub.
> > Charging Downstream Port (CDP) is a downstream port that complies
> >               with the USB 2.0 or USB 3.0 definition of a host or a hub.
> > Dedicated Charging Port (DCP)  is a downstream port on a device
> >               that outputs power through a USB connector.
> >
> > We cannot observe this now because this behave like a states machine which
> > is possible handled both in kernel driver and u-boot of Amlogic.
> > As per schematics this (page 7)  we have a external jumper set to
> > control the power path of the port.
> >
> > Odroid Wiki also says so.
> > [0] https://wiki.odroid.com/odroid-c1/hardware/otg_power_enable
> this seems hard to implement, because the same wiki page also states:
> "If your C1+ PCB is previous version, you need to desolder the R94 as
> described in this link."
>
>
> Regards
> Martin
>
>
> [0] https://github.com/endlessm/linux-meson/blob/0672f0b61eb92ba63c91d858a678d2c3a0bba06a/drivers/amlogic/usb/dwc_otg/310/dwc_otg_driver.c#L716
Martin Blumenstingl Feb. 6, 2019, 11:48 a.m. UTC | #5
Hi Anand,

On Tue, Feb 5, 2019 at 8:52 PM Anand Moon <linux.amoon@gmail.com> wrote:
[...]
> > personally I would start with getting device / peripheral mode working for usb0.
> > if you're lucky the PHY driver will work without any changes and you
> > only need to set the correct dr_mode along with the g-* properties.
> >
>
> Yes that should be the goal on getting working usb0 phy and otg port.
> So for now we will drop this patch and disable the usb_phy0 mode in
> dts for Odroid c2
> in order to get ride of the power failed WARNING.
I checked Kevin's Odroid-C2 (GXBB) on Kernel CI and I can't see any
power failed warning: [0]

> When we get the usb0 node to work again with good phy configuration and
> tuning we can re-enable the nodes. I will try to look into this in the future.
>
> Can you share your thought on this.
sounds good to me! especially because it would give us OTG support on
Meson8, Meson8b, Meson8m2 and GXBB (because these SoCs are sharing the
same USB implementation)!

I see four challenges:
1. getting host mode working on the micro USB port. you have already
proven that you can do this :)
2. getting peripheral mode working on the micro USB port. my hope is
that it's only about setting the right dr_mode and the g-* properties
on the usb0 node (but I haven't tried this)
3. make OTG detection work and integrate it with dwc2. I would consult
the dwc2 maintainers to see what can be done
4. bring everything together, making sure that one feature doesn't break another


Regards
Martin


[0] https://storage.kernelci.org/next/master/next-20190130/arm64/defconfig/lab-baylibre-seattle/boot-meson-gxbb-odroidc2.html
Anand Moon Feb. 7, 2019, 6:41 a.m. UTC | #6
Hi Martin,

On Wed, 6 Feb 2019 at 17:18, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Tue, Feb 5, 2019 at 8:52 PM Anand Moon <linux.amoon@gmail.com> wrote:
> [...]
> > > personally I would start with getting device / peripheral mode working for usb0.
> > > if you're lucky the PHY driver will work without any changes and you
> > > only need to set the correct dr_mode along with the g-* properties.
> > >
> >
> > Yes that should be the goal on getting working usb0 phy and otg port.
> > So for now we will drop this patch and disable the usb_phy0 mode in
> > dts for Odroid c2
> > in order to get ride of the power failed WARNING.
> I checked Kevin's Odroid-C2 (GXBB) on Kernel CI and I can't see any
> power failed warning: [0]
>
> > When we get the usb0 node to work again with good phy configuration and
> > tuning we can re-enable the nodes. I will try to look into this in the future.
> >
> > Can you share your thought on this.
> sounds good to me! especially because it would give us OTG support on
> Meson8, Meson8b, Meson8m2 and GXBB (because these SoCs are sharing the
> same USB implementation)!
>
> I see four challenges:
> 1. getting host mode working on the micro USB port. you have already
> proven that you can do this :)
> 2. getting peripheral mode working on the micro USB port. my hope is
> that it's only about setting the right dr_mode and the g-* properties
> on the usb0 node (but I haven't tried this)
> 3. make OTG detection work and integrate it with dwc2. I would consult
> the dwc2 maintainers to see what can be done
> 4. bring everything together, making sure that one feature doesn't break another
>

Thanks for the feedback!
I will give this a try, In case I am stuck I will ask to you help.

>
> Regards
> Martin
>
>
> [0] https://storage.kernelci.org/next/master/next-20190130/arm64/defconfig/lab-baylibre-seattle/boot-meson-gxbb-odroidc2.html

Not a issue If I could reproduce my  kernel warning consistently again
I will share the boot log and the step I followed.
I will be dropping my patch for the initialization of usb0_phy untill
correct solution is-found.

Best Regards
-Anand
diff mbox series

Patch

diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
index 9c01b7e19b06..1333c77e9172 100644
--- a/drivers/phy/amlogic/phy-meson8b-usb2.c
+++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
@@ -109,6 +109,7 @@ 
 
 #define RESET_COMPLETE_TIME				500
 #define ACA_ENABLE_COMPLETE_TIME			50
+#define DCD_ENABLE_COMPLETE_TIME			10
 
 struct phy_meson8b_usb2_priv {
 	void __iomem		*regs;
@@ -188,6 +189,20 @@  static int phy_meson8b_usb2_power_on(struct phy *phy)
 
 		udelay(ACA_ENABLE_COMPLETE_TIME);
 
+		if (phy_meson8b_usb2_read(priv, REG_ADP_BC) &
+			REG_ADP_BC_ACA_PIN_FLOAT) {
+			dev_warn(&phy->dev, "USB ID detect failed!\n");
+			clk_disable_unprepare(priv->clk_usb);
+			clk_disable_unprepare(priv->clk_usb_general);
+			return -EINVAL;
+		}
+	} else if (priv->dr_mode == USB_DR_MODE_OTG) {
+		phy_meson8b_usb2_mask_bits(priv, REG_ADP_BC,
+					   REG_ADP_BC_DCD_ENABLE,
+					   REG_ADP_BC_DCD_ENABLE);
+
+		udelay(DCD_ENABLE_COMPLETE_TIME);
+
 		if (phy_meson8b_usb2_read(priv, REG_ADP_BC) &
 			REG_ADP_BC_ACA_PIN_FLOAT) {
 			dev_warn(&phy->dev, "USB ID detect failed!\n");