| Message ID | 20200324102030.31000-3-narmstrong@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc3: meson: add OTG support for GXL/GXM | expand |
Hi Neil, On Tue, Mar 24, 2020 at 11:20 AM Neil Armstrong <narmstrong@baylibre.com> wrote: [...] > +static const char *meson_a1_phy_names[] = { > + "usb2-phy0", "usb2-phy1" > +}; my understanding is that the A1 SoC only has usb2-phy1. +Cc Hanjie Lin to confirm this apart from that: Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Martin
On 24/03/2020 15:13, Martin Blumenstingl wrote: > Hi Neil, > > On Tue, Mar 24, 2020 at 11:20 AM Neil Armstrong <narmstrong@baylibre.com> wrote: > [...] >> +static const char *meson_a1_phy_names[] = { >> + "usb2-phy0", "usb2-phy1" >> +}; > my understanding is that the A1 SoC only has usb2-phy1. > +Cc Hanjie Lin to confirm this I forgot a comment here, it only has a single PHY, but still the 2 U2 PHY controls slots... it would need a large amount of code to handle this tweak, and leaving 2 PHYs keeps the behavior before the patchset. Neil > > apart from that: > Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > > > Martin >
On Tue, Mar 24, 2020 at 3:17 PM Neil Armstrong <narmstrong@baylibre.com> wrote: > > On 24/03/2020 15:13, Martin Blumenstingl wrote: > > Hi Neil, > > > > On Tue, Mar 24, 2020 at 11:20 AM Neil Armstrong <narmstrong@baylibre.com> wrote: > > [...] > >> +static const char *meson_a1_phy_names[] = { > >> + "usb2-phy0", "usb2-phy1" > >> +}; > > my understanding is that the A1 SoC only has usb2-phy1. > > +Cc Hanjie Lin to confirm this > > I forgot a comment here, it only has a single PHY, but still the 2 U2 PHY > controls slots... it would need a large amount of code to handle this > tweak, and leaving 2 PHYs keeps the behavior before the patchset. I just reviewed patch #3 and I now understand that it would require a large code-change I'm fine with a comment Martin
On 2020/3/24 22:19, Martin Blumenstingl wrote: > On Tue, Mar 24, 2020 at 3:17 PM Neil Armstrong <narmstrong@baylibre.com> wrote: >> >> On 24/03/2020 15:13, Martin Blumenstingl wrote: >>> Hi Neil, >>> >>> On Tue, Mar 24, 2020 at 11:20 AM Neil Armstrong <narmstrong@baylibre.com> wrote: >>> [...] >>>> +static const char *meson_a1_phy_names[] = { >>>> + "usb2-phy0", "usb2-phy1" >>>> +}; >>> my understanding is that the A1 SoC only has usb2-phy1. >>> +Cc Hanjie Lin to confirm this >> >> I forgot a comment here, it only has a single PHY, but still the 2 U2 PHY >> controls slots... it would need a large amount of code to handle this >> tweak, and leaving 2 PHYs keeps the behavior before the patchset. > I just reviewed patch #3 and I now understand that it would require a > large code-change > I'm fine with a comment > > > Martin > > . > I see, A comment here will be more comprehensible. Acked-by: Hanjie Lin <hanjie.lin@amlogic.com> Hanjie
diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c index 902553f39889..7ba5fb3d0e22 100644 --- a/drivers/usb/dwc3/dwc3-meson-g12a.c +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c @@ -96,16 +96,8 @@ #define USB_R5_ID_DIG_TH_MASK GENMASK(15, 8) #define USB_R5_ID_DIG_CNT_MASK GENMASK(23, 16) -enum { - USB2_HOST_PHY = 0, - USB2_OTG_PHY, - USB3_HOST_PHY, - PHY_COUNT, -}; - -static const char *phy_names[PHY_COUNT] = { - "usb2-phy0", "usb2-phy1", "usb3-phy0", -}; +#define PHY_COUNT 3 +#define USB2_OTG_PHY 1 static struct clk_bulk_data meson_g12a_clocks[] = { { .id = NULL }, @@ -117,22 +109,36 @@ static struct clk_bulk_data meson_a1_clocks[] = { { .id = "xtal_usb_ctrl" }, }; +static const char *meson_g12a_phy_names[] = { + "usb2-phy0", "usb2-phy1", "usb3-phy0", +}; + +static const char *meson_a1_phy_names[] = { + "usb2-phy0", "usb2-phy1" +}; + struct dwc3_meson_g12a_drvdata { bool otg_switch_supported; struct clk_bulk_data *clks; int num_clks; + const char **phy_names; + int num_phys; }; static struct dwc3_meson_g12a_drvdata g12a_drvdata = { .otg_switch_supported = true, .clks = meson_g12a_clocks, .num_clks = ARRAY_SIZE(meson_g12a_clocks), + .phy_names = meson_g12a_phy_names, + .num_phys = ARRAY_SIZE(meson_g12a_phy_names), }; static struct dwc3_meson_g12a_drvdata a1_drvdata = { .otg_switch_supported = false, .clks = meson_a1_clocks, .num_clks = ARRAY_SIZE(meson_a1_clocks), + .phy_names = meson_a1_phy_names, + .num_phys = ARRAY_SIZE(meson_a1_phy_names), }; struct dwc3_meson_g12a { @@ -171,10 +177,13 @@ static int dwc3_meson_g12a_usb2_init(struct dwc3_meson_g12a *priv) else priv->otg_phy_mode = PHY_MODE_USB_HOST; - for (i = 0 ; i < USB3_HOST_PHY ; ++i) { + for (i = 0; i < priv->drvdata->num_phys; ++i) { if (!priv->phys[i]) continue; + if (!strstr(priv->drvdata->phy_names[i], "usb2")) + continue; + regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i), U2P_R0_POWER_ON_RESET, U2P_R0_POWER_ON_RESET); @@ -284,17 +293,19 @@ static const struct regmap_config phy_meson_g12a_usb3_regmap_conf = { static int dwc3_meson_g12a_get_phys(struct dwc3_meson_g12a *priv) { + const char *phy_name; int i; - for (i = 0 ; i < PHY_COUNT ; ++i) { - priv->phys[i] = devm_phy_optional_get(priv->dev, phy_names[i]); + for (i = 0 ; i < priv->drvdata->num_phys ; ++i) { + phy_name = priv->drvdata->phy_names[i]; + priv->phys[i] = devm_phy_optional_get(priv->dev, phy_name); if (!priv->phys[i]) continue; if (IS_ERR(priv->phys[i])) return PTR_ERR(priv->phys[i]); - if (i == USB3_HOST_PHY) + if (strstr(phy_name, "usb3")) priv->usb3_ports++; else priv->usb2_ports++;
To handle the variable USB2 PHY counts on GXL and GXM SoCs, add the possible PHY names for each SoC in the compatible match data. Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/usb/dwc3/dwc3-meson-g12a.c | 39 +++++++++++++++++++----------- 1 file changed, 25 insertions(+), 14 deletions(-)