[PATCH 0/5] usb: meson: switch to set_mode callback and other cleanup
Switch to set_mode callback now it's available and in the same time make public functions static and drop useless mach-meson headers that are no more needed.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- Neil Armstrong (5): phy: meson-gxl-usb2: add set_mode callback usb: dwc3: meson-gxl: switch to generic_phy_set_mode phy: meson-gxl-usb2: remove phy_meson_gxl_usb2_set_mode usb: dwc3: meson-gxl: drop usb-gx.h and make dwc3_meson_gxl_force_mode static usb: dwc3: meson-g12a: drop usb.h and make dwc3_meson_g12a_force_mode static
arch/arm/include/asm/arch-meson/usb-gx.h | 17 ----------------- arch/arm/include/asm/arch-meson/usb.h | 12 ------------ drivers/phy/meson-gxl-usb2.c | 30 ++++++++++++++++++------------ drivers/usb/dwc3/dwc3-meson-g12a.c | 2 +- drivers/usb/dwc3/dwc3-meson-gxl.c | 18 ++++++++++-------- 5 files changed, 29 insertions(+), 50 deletions(-) --- base-commit: f1de28e67aa9b66bfca0fad3dc18446a6ec0b504 change-id: 20240618-u-boot-usb-gxl-phy-set-mode-c3991c5f1da2
Best regards,
Implement set_mode callback by calling the current public function.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- drivers/phy/meson-gxl-usb2.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/meson-gxl-usb2.c b/drivers/phy/meson-gxl-usb2.c index 92c285103c..e051e66224 100644 --- a/drivers/phy/meson-gxl-usb2.c +++ b/drivers/phy/meson-gxl-usb2.c @@ -150,6 +150,28 @@ void phy_meson_gxl_usb2_set_mode(struct phy *phy, enum usb_dr_mode mode) phy_meson_gxl_usb2_reset(priv); }
+static int _phy_meson_gxl_usb2_set_mode(struct phy *phy, enum phy_mode mode, int submode) +{ + if (submode) + return -EOPNOTSUPP; + + switch (mode) { + case PHY_MODE_USB_DEVICE: + phy_meson_gxl_usb2_set_mode(phy, USB_DR_MODE_PERIPHERAL); + break; + + case PHY_MODE_USB_HOST: + case PHY_MODE_USB_OTG: + phy_meson_gxl_usb2_set_mode(phy, USB_DR_MODE_HOST); + break; + + default: + return -EINVAL; + } + + return 0; +} + static int phy_meson_gxl_usb2_power_on(struct phy *phy) { struct udevice *dev = phy->dev; @@ -161,7 +183,7 @@ static int phy_meson_gxl_usb2_power_on(struct phy *phy) val &= ~U2P_R0_POWER_ON_RESET; regmap_write(priv->regmap, U2P_R0, val);
- phy_meson_gxl_usb2_set_mode(phy, USB_DR_MODE_HOST); + _phy_meson_gxl_usb2_set_mode(phy, PHY_MODE_USB_HOST, 0);
return 0; } @@ -183,6 +205,7 @@ static int phy_meson_gxl_usb2_power_off(struct phy *phy) struct phy_ops meson_gxl_usb2_phy_ops = { .power_on = phy_meson_gxl_usb2_power_on, .power_off = phy_meson_gxl_usb2_power_off, + .set_mode = _phy_meson_gxl_usb2_set_mode, };
int meson_gxl_usb2_phy_probe(struct udevice *dev)
On 6/18/24 9:55 AM, Neil Armstrong wrote:
Implement set_mode callback by calling the current public function.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org
drivers/phy/meson-gxl-usb2.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/meson-gxl-usb2.c b/drivers/phy/meson-gxl-usb2.c index 92c285103c..e051e66224 100644 --- a/drivers/phy/meson-gxl-usb2.c +++ b/drivers/phy/meson-gxl-usb2.c @@ -150,6 +150,28 @@ void phy_meson_gxl_usb2_set_mode(struct phy *phy, enum usb_dr_mode mode) phy_meson_gxl_usb2_reset(priv); }
+static int _phy_meson_gxl_usb2_set_mode(struct phy *phy, enum phy_mode mode, int submode) +{
- if (submode)
return -EOPNOTSUPP;- switch (mode) {
- case PHY_MODE_USB_DEVICE:
phy_meson_gxl_usb2_set_mode(phy, USB_DR_MODE_PERIPHERAL);break;- case PHY_MODE_USB_HOST:
- case PHY_MODE_USB_OTG:
phy_meson_gxl_usb2_set_mode(phy, USB_DR_MODE_HOST);break;- default:
return -EINVAL;- }
- return 0;
+}
- static int phy_meson_gxl_usb2_power_on(struct phy *phy) { struct udevice *dev = phy->dev;
@@ -161,7 +183,7 @@ static int phy_meson_gxl_usb2_power_on(struct phy *phy) val &= ~U2P_R0_POWER_ON_RESET; regmap_write(priv->regmap, U2P_R0, val);
- phy_meson_gxl_usb2_set_mode(phy, USB_DR_MODE_HOST);
Could you rename this ^ to some phy_meson_gxl_usb2_set_mode_inner() and ...
- _phy_meson_gxl_usb2_set_mode(phy, PHY_MODE_USB_HOST, 0);
... remove the leading underscore here.
return 0; } @@ -183,6 +205,7 @@ static int phy_meson_gxl_usb2_power_off(struct phy *phy) struct phy_ops meson_gxl_usb2_phy_ops = { .power_on = phy_meson_gxl_usb2_power_on, .power_off = phy_meson_gxl_usb2_power_off,
- .set_mode = _phy_meson_gxl_usb2_set_mode,
... and here ?
[...]
Use the PHY set_mode call instead of calling directly in the PHY shared function.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- drivers/usb/dwc3/dwc3-meson-gxl.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-meson-gxl.c b/drivers/usb/dwc3/dwc3-meson-gxl.c index 3e693c5ff3..290ac03a37 100644 --- a/drivers/usb/dwc3/dwc3-meson-gxl.c +++ b/drivers/usb/dwc3/dwc3-meson-gxl.c @@ -158,9 +158,9 @@ static int dwc3_meson_gxl_usb2_init(struct dwc3_meson_gxl *priv) if (!priv->phys[i].dev) continue;
- phy_meson_gxl_usb2_set_mode(&priv->phys[i], - (i == USB2_OTG_PHY) ? USB_DR_MODE_PERIPHERAL - : USB_DR_MODE_HOST); + generic_phy_set_mode(&priv->phys[i], + (i == USB2_OTG_PHY) ? PHY_MODE_USB_DEVICE + : PHY_MODE_USB_HOST, 0); }
return 0; @@ -224,7 +224,9 @@ int dwc3_meson_gxl_force_mode(struct udevice *dev, enum usb_dr_mode mode) #endif priv->otg_phy_mode = mode;
- phy_meson_gxl_usb2_set_mode(&priv->phys[USB2_OTG_PHY], mode); + generic_phy_set_mode(&priv->phys[USB2_OTG_PHY], + mode == USB_DR_MODE_PERIPHERAL ? PHY_MODE_USB_DEVICE + : PHY_MODE_USB_HOST, 0);
dwc3_meson_gxl_usb2_set_mode(priv, mode);
@@ -361,8 +363,9 @@ static int dwc3_meson_gxl_probe(struct udevice *dev) }
if (priv->phys[USB2_OTG_PHY].dev) - phy_meson_gxl_usb2_set_mode(&priv->phys[USB2_OTG_PHY], - priv->otg_phy_mode); + generic_phy_set_mode(&priv->phys[USB2_OTG_PHY], + priv->otg_phy_mode == USB_DR_MODE_PERIPHERAL ? PHY_MODE_USB_DEVICE + : PHY_MODE_USB_HOST, 0);
dwc3_meson_gxl_usb2_set_mode(priv, priv->otg_phy_mode);
Remove the public phy_meson_gxl_usb2_set_mode and move the implementation in the the set_mode callback.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- arch/arm/include/asm/arch-meson/usb-gx.h | 3 --- drivers/phy/meson-gxl-usb2.c | 45 ++++++++++---------------------- 2 files changed, 14 insertions(+), 34 deletions(-)
diff --git a/arch/arm/include/asm/arch-meson/usb-gx.h b/arch/arm/include/asm/arch-meson/usb-gx.h index 61f1809df9..966d401730 100644 --- a/arch/arm/include/asm/arch-meson/usb-gx.h +++ b/arch/arm/include/asm/arch-meson/usb-gx.h @@ -9,9 +9,6 @@ #include <generic-phy.h> #include <linux/usb/otg.h>
-/* TOFIX add set_mode to struct phy_ops */ -void phy_meson_gxl_usb2_set_mode(struct phy *phy, enum usb_dr_mode mode); - int dwc3_meson_gxl_force_mode(struct udevice *dev, enum usb_dr_mode mode);
#endif diff --git a/drivers/phy/meson-gxl-usb2.c b/drivers/phy/meson-gxl-usb2.c index e051e66224..140e936b47 100644 --- a/drivers/phy/meson-gxl-usb2.c +++ b/drivers/phy/meson-gxl-usb2.c @@ -19,8 +19,6 @@ #include <linux/printk.h> #include <linux/usb/otg.h>
-#include <asm/arch/usb-gx.h> - #include <linux/bitops.h> #include <linux/compat.h>
@@ -121,54 +119,39 @@ static void phy_meson_gxl_usb2_reset(struct phy_meson_gxl_usb2_priv *priv) udelay(RESET_COMPLETE_TIME); }
-void phy_meson_gxl_usb2_set_mode(struct phy *phy, enum usb_dr_mode mode) +static int phy_meson_gxl_usb2_set_mode(struct phy *phy, enum phy_mode mode, int submode) { struct udevice *dev = phy->dev; struct phy_meson_gxl_usb2_priv *priv = dev_get_priv(dev); uint val;
+ if (submode) + return -EOPNOTSUPP; + regmap_read(priv->regmap, U2P_R0, &val);
switch (mode) { - case USB_DR_MODE_UNKNOWN: - case USB_DR_MODE_HOST: - case USB_DR_MODE_OTG: - val |= U2P_R0_DM_PULLDOWN; - val |= U2P_R0_DP_PULLDOWN; - val &= ~U2P_R0_ID_PULLUP; - break; - - case USB_DR_MODE_PERIPHERAL: + case PHY_MODE_USB_DEVICE: val &= ~U2P_R0_DM_PULLDOWN; val &= ~U2P_R0_DP_PULLDOWN; val |= U2P_R0_ID_PULLUP; break; - } - - regmap_write(priv->regmap, U2P_R0, val); - - phy_meson_gxl_usb2_reset(priv); -} - -static int _phy_meson_gxl_usb2_set_mode(struct phy *phy, enum phy_mode mode, int submode) -{ - if (submode) - return -EOPNOTSUPP; - - switch (mode) { - case PHY_MODE_USB_DEVICE: - phy_meson_gxl_usb2_set_mode(phy, USB_DR_MODE_PERIPHERAL); - break;
case PHY_MODE_USB_HOST: case PHY_MODE_USB_OTG: - phy_meson_gxl_usb2_set_mode(phy, USB_DR_MODE_HOST); + val |= U2P_R0_DM_PULLDOWN; + val |= U2P_R0_DP_PULLDOWN; + val &= ~U2P_R0_ID_PULLUP; break;
default: return -EINVAL; }
+ regmap_write(priv->regmap, U2P_R0, val); + + phy_meson_gxl_usb2_reset(priv); + return 0; }
@@ -183,7 +166,7 @@ static int phy_meson_gxl_usb2_power_on(struct phy *phy) val &= ~U2P_R0_POWER_ON_RESET; regmap_write(priv->regmap, U2P_R0, val);
- _phy_meson_gxl_usb2_set_mode(phy, PHY_MODE_USB_HOST, 0); + phy_meson_gxl_usb2_set_mode(phy, PHY_MODE_USB_HOST, 0);
return 0; } @@ -205,7 +188,7 @@ static int phy_meson_gxl_usb2_power_off(struct phy *phy) struct phy_ops meson_gxl_usb2_phy_ops = { .power_on = phy_meson_gxl_usb2_power_on, .power_off = phy_meson_gxl_usb2_power_off, - .set_mode = _phy_meson_gxl_usb2_set_mode, + .set_mode = phy_meson_gxl_usb2_set_mode, };
int meson_gxl_usb2_phy_probe(struct udevice *dev)
On 6/18/24 9:55 AM, Neil Armstrong wrote:
[...]
@@ -205,7 +188,7 @@ static int phy_meson_gxl_usb2_power_off(struct phy *phy) struct phy_ops meson_gxl_usb2_phy_ops = { .power_on = phy_meson_gxl_usb2_power_on, .power_off = phy_meson_gxl_usb2_power_off,
- .set_mode = _phy_meson_gxl_usb2_set_mode,
- .set_mode = phy_meson_gxl_usb2_set_mode,
Oh, I see you did rename it here, so please ignore my comment on 1/5 . Maybe just mention in the 1/5 commit message that the rename is happening in follow up patch , if you are doing V2 at all .
On 18/06/2024 16:56, Marek Vasut wrote:
On 6/18/24 9:55 AM, Neil Armstrong wrote:
[...]
@@ -205,7 +188,7 @@ static int phy_meson_gxl_usb2_power_off(struct phy *phy) struct phy_ops meson_gxl_usb2_phy_ops = { .power_on = phy_meson_gxl_usb2_power_on, .power_off = phy_meson_gxl_usb2_power_off, - .set_mode = _phy_meson_gxl_usb2_set_mode, + .set_mode = phy_meson_gxl_usb2_set_mode,
Oh, I see you did rename it here, so please ignore my comment on 1/5 . Maybe just mention in the 1/5 commit message that the rename is happening in follow up patch , if you are doing V2 at all .
Sure, let me update the commit message,
Thanks, Neil
On 6/20/24 9:39 AM, Neil Armstrong wrote:
On 18/06/2024 16:56, Marek Vasut wrote:
On 6/18/24 9:55 AM, Neil Armstrong wrote:
[...]
@@ -205,7 +188,7 @@ static int phy_meson_gxl_usb2_power_off(struct phy *phy) struct phy_ops meson_gxl_usb2_phy_ops = { .power_on = phy_meson_gxl_usb2_power_on, .power_off = phy_meson_gxl_usb2_power_off, - .set_mode = _phy_meson_gxl_usb2_set_mode, + .set_mode = phy_meson_gxl_usb2_set_mode,
Oh, I see you did rename it here, so please ignore my comment on 1/5 . Maybe just mention in the 1/5 commit message that the rename is happening in follow up patch , if you are doing V2 at all .
Sure, let me update the commit message,
Thank you
Drop this useless usb-gx.h and now make dwc3_meson_gxl_force_mode static since only used in the dwc3-meson-gxl.c file.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- arch/arm/include/asm/arch-meson/usb-gx.h | 14 -------------- drivers/usb/dwc3/dwc3-meson-gxl.c | 3 +-- 2 files changed, 1 insertion(+), 16 deletions(-)
diff --git a/arch/arm/include/asm/arch-meson/usb-gx.h b/arch/arm/include/asm/arch-meson/usb-gx.h deleted file mode 100644 index 966d401730..0000000000 --- a/arch/arm/include/asm/arch-meson/usb-gx.h +++ /dev/null @@ -1,14 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0+ */ -/* - * Copyright 2019 BayLibre SAS - * Author: Neil Armstrong narmstrong@baylibre.com - */ -#ifndef _ARCH_MESON_USB_GX_H_ -#define _ARCH_MESON_USB_GX_H_ - -#include <generic-phy.h> -#include <linux/usb/otg.h> - -int dwc3_meson_gxl_force_mode(struct udevice *dev, enum usb_dr_mode mode); - -#endif diff --git a/drivers/usb/dwc3/dwc3-meson-gxl.c b/drivers/usb/dwc3/dwc3-meson-gxl.c index 290ac03a37..5fb9b477ad 100644 --- a/drivers/usb/dwc3/dwc3-meson-gxl.c +++ b/drivers/usb/dwc3/dwc3-meson-gxl.c @@ -26,7 +26,6 @@ #include <linux/bitfield.h> #include <linux/bitops.h> #include <linux/compat.h> -#include <asm/arch/usb-gx.h>
/* USB Glue Control Registers */
@@ -193,7 +192,7 @@ static int dwc3_meson_gxl_usb_init(struct dwc3_meson_gxl *priv) return 0; }
-int dwc3_meson_gxl_force_mode(struct udevice *dev, enum usb_dr_mode mode) +static int dwc3_meson_gxl_force_mode(struct udevice *dev, enum usb_dr_mode mode) { struct dwc3_meson_gxl *priv = dev_get_plat(dev);
Drop this useless usb.h and now make dwc3_meson_g12a_force_mode static since only used in the dwc3-meson-g12a.c file.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- arch/arm/include/asm/arch-meson/usb.h | 12 ------------ drivers/usb/dwc3/dwc3-meson-g12a.c | 2 +- 2 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/arch/arm/include/asm/arch-meson/usb.h b/arch/arm/include/asm/arch-meson/usb.h deleted file mode 100644 index b794b5ce77..0000000000 --- a/arch/arm/include/asm/arch-meson/usb.h +++ /dev/null @@ -1,12 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0+ */ -/* - * Copyright (C) 2019 BayLibre, SAS - * Author: Neil Armstrong narmstrong@baylibre.com - */ - -#ifndef __MESON_USB_H__ -#define __MESON_USB_H__ - -int dwc3_meson_g12a_force_mode(struct udevice *dev, enum usb_dr_mode mode); - -#endif /* __MESON_USB_H__ */ diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c index 21e4f637bb..41d15996e5 100644 --- a/drivers/usb/dwc3/dwc3-meson-g12a.c +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c @@ -270,7 +270,7 @@ static int dwc3_meson_g12a_usb_init(struct dwc3_meson_g12a *priv) return 0; }
-int dwc3_meson_g12a_force_mode(struct udevice *dev, enum usb_dr_mode mode) +static int dwc3_meson_g12a_force_mode(struct udevice *dev, enum usb_dr_mode mode) { struct dwc3_meson_g12a *priv = dev_get_plat(dev);
participants (2)
-
Marek Vasut -
Neil Armstrong