* [PATCH v2 2/3] phy: rcar: Split init and set_mode operations
2024-06-17 17:36 [PATCH v2 1/3] phy: Extend generic_setup_phy() with PHY mode and submode Marek Vasut
@ 2024-06-17 17:36 ` Marek Vasut
2024-07-09 9:24 ` Mattijs Korpershoek
2024-06-17 17:36 ` [PATCH v2 3/3] phy: test: Implement sandbox PHY .set_mode and DM test Marek Vasut
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2024-06-17 17:36 UTC (permalink / raw)
To: u-boot
Cc: Marek Vasut, Caleb Connolly, Fabio Estevam, Fabrice Gasnier,
Jonas Karlman, Mathieu Othacehe, Mattijs Korpershoek,
Neil Armstrong, Nishanth Menon, Nobuhiro Iwamatsu, Sean Anderson,
Simon Glass, Sumit Garg, Tim Harvey, Tom Rini,
Xavier Drudis Ferran, u-boot-qcom
The current init operation also sets the PHY into USB host mode.
Split the mode configuration into set_mode callback instead and
implement support for device and OTG modes as well.
The OTG mode performs auto-detection and selects either host or
device mode. In case the OTG mode is configured, submode field
can be used to select full PHY (re)initialization or only mode
auto-detection. The full (re)initialization is only necessary
once, on start up.
Since the OTG mode may enable IRQ generation in the PHY, disable
that IRQ generation in the exit callback again.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Caleb Connolly <caleb.connolly@linaro.org>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Mathieu Othacehe <othacehe@gnu.org>
Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Nishanth Menon <nm@ti.com>
Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Xavier Drudis Ferran <xdrudis@tinet.cat>
Cc: u-boot-qcom@groups.io
Cc: u-boot@lists.denx.de
---
V2: No change
---
drivers/phy/phy-rcar-gen3.c | 90 ++++++++++++++++++++++++++++++++++---
1 file changed, 85 insertions(+), 5 deletions(-)
diff --git a/drivers/phy/phy-rcar-gen3.c b/drivers/phy/phy-rcar-gen3.c
index 7c292cae0e2..b278f995f37 100644
--- a/drivers/phy/phy-rcar-gen3.c
+++ b/drivers/phy/phy-rcar-gen3.c
@@ -8,6 +8,7 @@
#include <clk.h>
#include <div64.h>
#include <dm.h>
+#include <dm/device_compat.h>
#include <fdtdec.h>
#include <generic-phy.h>
#include <malloc.h>
@@ -31,8 +32,13 @@
#define USB2_LINECTRL1 0x610
#define USB2_ADPCTRL 0x630
+/* INT_ENABLE */
+#define USB2_INT_ENABLE_UCOM_INTEN BIT(3)
+#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2)
+#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1)
+
/* USBCTR */
-#define USB2_USBCTR_PLL_RST BIT(1)
+#define USB2_USBCTR_PLL_RST BIT(1)
/* SPD_RSM_TIMSET */
#define USB2_SPD_RSM_TIMSET_INIT 0x014e029b
@@ -43,11 +49,23 @@
/* COMMCTRL */
#define USB2_COMMCTRL_OTG_PERI BIT(31) /* 1 = Peripheral mode */
+/* OBINTSTA and OBINTEN */
+#define USB2_OBINT_SESSVLDCHG BIT(12)
+#define USB2_OBINT_IDDIGCHG BIT(11)
+
+/* VBCTRL */
+#define USB2_VBCTRL_DRVVBUSSEL BIT(8)
+
/* LINECTRL1 */
+#define USB2_LINECTRL1_DPRPD_EN BIT(19)
#define USB2_LINECTRL1_DP_RPD BIT(18)
+#define USB2_LINECTRL1_DMRPD_EN BIT(17)
#define USB2_LINECTRL1_DM_RPD BIT(16)
/* ADPCTRL */
+#define USB2_ADPCTRL_OTGSESSVLD BIT(20)
+#define USB2_ADPCTRL_IDDIG BIT(19)
+#define USB2_ADPCTRL_IDPULLUP BIT(5) /* 1 = ID sampling is enabled */
#define USB2_ADPCTRL_DRVVBUS BIT(4)
struct rcar_gen3_phy {
@@ -65,12 +83,14 @@ static int rcar_gen3_phy_phy_init(struct phy *phy)
writel(USB2_SPD_RSM_TIMSET_INIT, priv->regs + USB2_SPD_RSM_TIMSET);
writel(USB2_OC_TIMSET_INIT, priv->regs + USB2_OC_TIMSET);
- setbits_le32(priv->regs + USB2_LINECTRL1,
- USB2_LINECTRL1_DP_RPD | USB2_LINECTRL1_DM_RPD);
+ return 0;
+}
- clrbits_le32(priv->regs + USB2_COMMCTRL, USB2_COMMCTRL_OTG_PERI);
+static int rcar_gen3_phy_phy_exit(struct phy *phy)
+{
+ struct rcar_gen3_phy *priv = dev_get_priv(phy->dev);
- setbits_le32(priv->regs + USB2_ADPCTRL, USB2_ADPCTRL_DRVVBUS);
+ writel(0, priv->regs + USB2_INT_ENABLE);
return 0;
}
@@ -102,10 +122,70 @@ static int rcar_gen3_phy_phy_power_off(struct phy *phy)
return regulator_set_enable(priv->vbus_supply, false);
}
+static int rcar_gen3_phy_phy_set_mode(struct phy *phy, enum phy_mode mode,
+ int submode)
+{
+ const u32 adpdevmask = USB2_ADPCTRL_IDDIG | USB2_ADPCTRL_OTGSESSVLD;
+ struct rcar_gen3_phy *priv = dev_get_priv(phy->dev);
+ u32 adpctrl;
+
+ if (mode == PHY_MODE_USB_OTG) {
+ if (submode) {
+ /* OTG submode is used as initialization indicator */
+ writel(USB2_INT_ENABLE_UCOM_INTEN |
+ USB2_INT_ENABLE_USBH_INTB_EN |
+ USB2_INT_ENABLE_USBH_INTA_EN,
+ priv->regs + USB2_INT_ENABLE);
+ setbits_le32(priv->regs + USB2_VBCTRL,
+ USB2_VBCTRL_DRVVBUSSEL);
+ writel(USB2_OBINT_SESSVLDCHG | USB2_OBINT_IDDIGCHG,
+ priv->regs + USB2_OBINTSTA);
+ setbits_le32(priv->regs + USB2_OBINTEN,
+ USB2_OBINT_SESSVLDCHG |
+ USB2_OBINT_IDDIGCHG);
+ setbits_le32(priv->regs + USB2_ADPCTRL,
+ USB2_ADPCTRL_IDPULLUP);
+ clrsetbits_le32(priv->regs + USB2_LINECTRL1,
+ USB2_LINECTRL1_DP_RPD |
+ USB2_LINECTRL1_DM_RPD |
+ USB2_LINECTRL1_DPRPD_EN |
+ USB2_LINECTRL1_DMRPD_EN,
+ USB2_LINECTRL1_DPRPD_EN |
+ USB2_LINECTRL1_DMRPD_EN);
+ }
+
+ adpctrl = readl(priv->regs + USB2_ADPCTRL);
+ if ((adpctrl & adpdevmask) == adpdevmask)
+ mode = PHY_MODE_USB_DEVICE;
+ else
+ mode = PHY_MODE_USB_HOST;
+ }
+
+ if (mode == PHY_MODE_USB_HOST) {
+ clrbits_le32(priv->regs + USB2_COMMCTRL, USB2_COMMCTRL_OTG_PERI);
+ setbits_le32(priv->regs + USB2_LINECTRL1,
+ USB2_LINECTRL1_DP_RPD | USB2_LINECTRL1_DM_RPD);
+ setbits_le32(priv->regs + USB2_ADPCTRL, USB2_ADPCTRL_DRVVBUS);
+ } else if (mode == PHY_MODE_USB_DEVICE) {
+ setbits_le32(priv->regs + USB2_COMMCTRL, USB2_COMMCTRL_OTG_PERI);
+ clrsetbits_le32(priv->regs + USB2_LINECTRL1,
+ USB2_LINECTRL1_DP_RPD | USB2_LINECTRL1_DM_RPD,
+ USB2_LINECTRL1_DM_RPD);
+ clrbits_le32(priv->regs + USB2_ADPCTRL, USB2_ADPCTRL_DRVVBUS);
+ } else {
+ dev_err(phy->dev, "Unknown mode %d\n", mode);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static const struct phy_ops rcar_gen3_phy_phy_ops = {
.init = rcar_gen3_phy_phy_init,
+ .exit = rcar_gen3_phy_phy_exit,
.power_on = rcar_gen3_phy_phy_power_on,
.power_off = rcar_gen3_phy_phy_power_off,
+ .set_mode = rcar_gen3_phy_phy_set_mode,
};
static int rcar_gen3_phy_probe(struct udevice *dev)
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 2/3] phy: rcar: Split init and set_mode operations
2024-06-17 17:36 ` [PATCH v2 2/3] phy: rcar: Split init and set_mode operations Marek Vasut
@ 2024-07-09 9:24 ` Mattijs Korpershoek
0 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2024-07-09 9:24 UTC (permalink / raw)
To: Marek Vasut, u-boot
Cc: Marek Vasut, Caleb Connolly, Fabio Estevam, Fabrice Gasnier,
Jonas Karlman, Mathieu Othacehe, Neil Armstrong, Nishanth Menon,
Nobuhiro Iwamatsu, Sean Anderson, Simon Glass, Sumit Garg,
Tim Harvey, Tom Rini, Xavier Drudis Ferran, u-boot-qcom
Hi Marek,
Thank you for the patch.
On lun., juin 17, 2024 at 19:36, Marek Vasut <marek.vasut+renesas@mailbox.org> wrote:
> The current init operation also sets the PHY into USB host mode.
> Split the mode configuration into set_mode callback instead and
> implement support for device and OTG modes as well.
>
> The OTG mode performs auto-detection and selects either host or
> device mode. In case the OTG mode is configured, submode field
> can be used to select full PHY (re)initialization or only mode
> auto-detection. The full (re)initialization is only necessary
> once, on start up.
>
> Since the OTG mode may enable IRQ generation in the PHY, disable
> that IRQ generation in the exit callback again.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
> Cc: Caleb Connolly <caleb.connolly@linaro.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Mathieu Othacehe <othacehe@gnu.org>
> Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Cc: Sean Anderson <seanga2@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Xavier Drudis Ferran <xdrudis@tinet.cat>
> Cc: u-boot-qcom@groups.io
> Cc: u-boot@lists.denx.de
> ---
> V2: No change
> ---
> drivers/phy/phy-rcar-gen3.c | 90 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 85 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/phy/phy-rcar-gen3.c b/drivers/phy/phy-rcar-gen3.c
> index 7c292cae0e2..b278f995f37 100644
> --- a/drivers/phy/phy-rcar-gen3.c
> +++ b/drivers/phy/phy-rcar-gen3.c
> @@ -8,6 +8,7 @@
> #include <clk.h>
> #include <div64.h>
> #include <dm.h>
> +#include <dm/device_compat.h>
> #include <fdtdec.h>
> #include <generic-phy.h>
> #include <malloc.h>
> @@ -31,8 +32,13 @@
> #define USB2_LINECTRL1 0x610
> #define USB2_ADPCTRL 0x630
>
> +/* INT_ENABLE */
> +#define USB2_INT_ENABLE_UCOM_INTEN BIT(3)
> +#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2)
> +#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1)
> +
> /* USBCTR */
> -#define USB2_USBCTR_PLL_RST BIT(1)
> +#define USB2_USBCTR_PLL_RST BIT(1)
>
> /* SPD_RSM_TIMSET */
> #define USB2_SPD_RSM_TIMSET_INIT 0x014e029b
> @@ -43,11 +49,23 @@
> /* COMMCTRL */
> #define USB2_COMMCTRL_OTG_PERI BIT(31) /* 1 = Peripheral mode */
>
> +/* OBINTSTA and OBINTEN */
> +#define USB2_OBINT_SESSVLDCHG BIT(12)
> +#define USB2_OBINT_IDDIGCHG BIT(11)
> +
> +/* VBCTRL */
> +#define USB2_VBCTRL_DRVVBUSSEL BIT(8)
> +
> /* LINECTRL1 */
> +#define USB2_LINECTRL1_DPRPD_EN BIT(19)
> #define USB2_LINECTRL1_DP_RPD BIT(18)
> +#define USB2_LINECTRL1_DMRPD_EN BIT(17)
> #define USB2_LINECTRL1_DM_RPD BIT(16)
>
> /* ADPCTRL */
> +#define USB2_ADPCTRL_OTGSESSVLD BIT(20)
> +#define USB2_ADPCTRL_IDDIG BIT(19)
> +#define USB2_ADPCTRL_IDPULLUP BIT(5) /* 1 = ID sampling is enabled */
> #define USB2_ADPCTRL_DRVVBUS BIT(4)
>
> struct rcar_gen3_phy {
> @@ -65,12 +83,14 @@ static int rcar_gen3_phy_phy_init(struct phy *phy)
> writel(USB2_SPD_RSM_TIMSET_INIT, priv->regs + USB2_SPD_RSM_TIMSET);
> writel(USB2_OC_TIMSET_INIT, priv->regs + USB2_OC_TIMSET);
>
> - setbits_le32(priv->regs + USB2_LINECTRL1,
> - USB2_LINECTRL1_DP_RPD | USB2_LINECTRL1_DM_RPD);
> + return 0;
> +}
>
> - clrbits_le32(priv->regs + USB2_COMMCTRL, USB2_COMMCTRL_OTG_PERI);
> +static int rcar_gen3_phy_phy_exit(struct phy *phy)
> +{
> + struct rcar_gen3_phy *priv = dev_get_priv(phy->dev);
>
> - setbits_le32(priv->regs + USB2_ADPCTRL, USB2_ADPCTRL_DRVVBUS);
> + writel(0, priv->regs + USB2_INT_ENABLE);
>
> return 0;
> }
> @@ -102,10 +122,70 @@ static int rcar_gen3_phy_phy_power_off(struct phy *phy)
> return regulator_set_enable(priv->vbus_supply, false);
> }
>
> +static int rcar_gen3_phy_phy_set_mode(struct phy *phy, enum phy_mode mode,
> + int submode)
> +{
> + const u32 adpdevmask = USB2_ADPCTRL_IDDIG | USB2_ADPCTRL_OTGSESSVLD;
> + struct rcar_gen3_phy *priv = dev_get_priv(phy->dev);
> + u32 adpctrl;
> +
> + if (mode == PHY_MODE_USB_OTG) {
> + if (submode) {
> + /* OTG submode is used as initialization indicator */
> + writel(USB2_INT_ENABLE_UCOM_INTEN |
> + USB2_INT_ENABLE_USBH_INTB_EN |
> + USB2_INT_ENABLE_USBH_INTA_EN,
> + priv->regs + USB2_INT_ENABLE);
> + setbits_le32(priv->regs + USB2_VBCTRL,
> + USB2_VBCTRL_DRVVBUSSEL);
> + writel(USB2_OBINT_SESSVLDCHG | USB2_OBINT_IDDIGCHG,
> + priv->regs + USB2_OBINTSTA);
> + setbits_le32(priv->regs + USB2_OBINTEN,
> + USB2_OBINT_SESSVLDCHG |
> + USB2_OBINT_IDDIGCHG);
> + setbits_le32(priv->regs + USB2_ADPCTRL,
> + USB2_ADPCTRL_IDPULLUP);
> + clrsetbits_le32(priv->regs + USB2_LINECTRL1,
> + USB2_LINECTRL1_DP_RPD |
> + USB2_LINECTRL1_DM_RPD |
> + USB2_LINECTRL1_DPRPD_EN |
> + USB2_LINECTRL1_DMRPD_EN,
> + USB2_LINECTRL1_DPRPD_EN |
> + USB2_LINECTRL1_DMRPD_EN);
> + }
> +
> + adpctrl = readl(priv->regs + USB2_ADPCTRL);
> + if ((adpctrl & adpdevmask) == adpdevmask)
> + mode = PHY_MODE_USB_DEVICE;
> + else
> + mode = PHY_MODE_USB_HOST;
> + }
> +
> + if (mode == PHY_MODE_USB_HOST) {
> + clrbits_le32(priv->regs + USB2_COMMCTRL, USB2_COMMCTRL_OTG_PERI);
> + setbits_le32(priv->regs + USB2_LINECTRL1,
> + USB2_LINECTRL1_DP_RPD | USB2_LINECTRL1_DM_RPD);
> + setbits_le32(priv->regs + USB2_ADPCTRL, USB2_ADPCTRL_DRVVBUS);
> + } else if (mode == PHY_MODE_USB_DEVICE) {
> + setbits_le32(priv->regs + USB2_COMMCTRL, USB2_COMMCTRL_OTG_PERI);
> + clrsetbits_le32(priv->regs + USB2_LINECTRL1,
> + USB2_LINECTRL1_DP_RPD | USB2_LINECTRL1_DM_RPD,
> + USB2_LINECTRL1_DM_RPD);
> + clrbits_le32(priv->regs + USB2_ADPCTRL, USB2_ADPCTRL_DRVVBUS);
> + } else {
> + dev_err(phy->dev, "Unknown mode %d\n", mode);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static const struct phy_ops rcar_gen3_phy_phy_ops = {
> .init = rcar_gen3_phy_phy_init,
> + .exit = rcar_gen3_phy_phy_exit,
> .power_on = rcar_gen3_phy_phy_power_on,
> .power_off = rcar_gen3_phy_phy_power_off,
> + .set_mode = rcar_gen3_phy_phy_set_mode,
> };
>
> static int rcar_gen3_phy_probe(struct udevice *dev)
> --
> 2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] phy: test: Implement sandbox PHY .set_mode and DM test
2024-06-17 17:36 [PATCH v2 1/3] phy: Extend generic_setup_phy() with PHY mode and submode Marek Vasut
2024-06-17 17:36 ` [PATCH v2 2/3] phy: rcar: Split init and set_mode operations Marek Vasut
@ 2024-06-17 17:36 ` Marek Vasut
2024-07-09 9:33 ` Mattijs Korpershoek
2024-07-09 9:20 ` [PATCH v2 1/3] phy: Extend generic_setup_phy() with PHY mode and submode Mattijs Korpershoek
2024-09-08 18:11 ` Marek Vasut
3 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2024-06-17 17:36 UTC (permalink / raw)
To: u-boot
Cc: Marek Vasut, Caleb Connolly, Fabio Estevam, Fabrice Gasnier,
Jonas Karlman, Mathieu Othacehe, Mattijs Korpershoek,
Neil Armstrong, Nishanth Menon, Nobuhiro Iwamatsu, Sean Anderson,
Simon Glass, Sumit Garg, Tim Harvey, Tom Rini,
Xavier Drudis Ferran, u-boot-qcom
Implement trivial extension to the sandbox PHY, which makes it pretend
to support selecting USB Host mode and nothing else. Any other mode is
rejected with -EINVAL. Any submode except for default submode 0 is
rejected with -EOPNOTSUPP . The implementation behaves in this trivial
way to permit easy unit testing using test which is also added in this
commit.
To run the test, use e.g. sandbox64_defconfig and run U-Boot as follows:
$ ./u-boot -Tc 'ut dm phy_setup'
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Caleb Connolly <caleb.connolly@linaro.org>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Mathieu Othacehe <othacehe@gnu.org>
Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Nishanth Menon <nm@ti.com>
Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Xavier Drudis Ferran <xdrudis@tinet.cat>
Cc: u-boot-qcom@groups.io
Cc: u-boot@lists.denx.de
---
V2: New patch
---
drivers/phy/sandbox-phy.c | 13 +++++++++++++
test/dm/phy.c | 7 +++++++
2 files changed, 20 insertions(+)
diff --git a/drivers/phy/sandbox-phy.c b/drivers/phy/sandbox-phy.c
index b159147a765..e70d20432e0 100644
--- a/drivers/phy/sandbox-phy.c
+++ b/drivers/phy/sandbox-phy.c
@@ -72,6 +72,18 @@ static int sandbox_phy_exit(struct phy *phy)
return 0;
}
+static int
+sandbox_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
+{
+ if (submode)
+ return -EOPNOTSUPP;
+
+ if (mode != PHY_MODE_USB_HOST)
+ return -EINVAL;
+
+ return 0;
+}
+
static int sandbox_phy_bind(struct udevice *dev)
{
if (dev_get_driver_data(dev) != DRIVER_DATA)
@@ -96,6 +108,7 @@ static struct phy_ops sandbox_phy_ops = {
.power_off = sandbox_phy_power_off,
.init = sandbox_phy_init,
.exit = sandbox_phy_exit,
+ .set_mode = sandbox_phy_set_mode,
};
static const struct udevice_id sandbox_phy_ids[] = {
diff --git a/test/dm/phy.c b/test/dm/phy.c
index a90881b12ab..a93aa83ab10 100644
--- a/test/dm/phy.c
+++ b/test/dm/phy.c
@@ -246,6 +246,13 @@ static int dm_test_phy_setup(struct unit_test_state *uts)
ut_assertok(generic_setup_phy(parent, &phy, 0, PHY_MODE_USB_HOST, 0));
ut_assertok(generic_shutdown_phy(&phy));
+ /* set_mode as USB Host passes, anything else is not supported */
+ ut_assertok(generic_setup_phy(parent, &phy, 0, PHY_MODE_USB_HOST, 0));
+ ut_assertok(generic_phy_set_mode(&phy, PHY_MODE_USB_HOST, 0));
+ ut_asserteq(-EOPNOTSUPP, generic_phy_set_mode(&phy, PHY_MODE_USB_HOST, 1));
+ ut_asserteq(-EINVAL, generic_phy_set_mode(&phy, PHY_MODE_USB_DEVICE, 0));
+ ut_assertok(generic_shutdown_phy(&phy));
+
/* power_off fail with -EIO */
ut_assertok(generic_setup_phy(parent, &phy, 1, PHY_MODE_USB_HOST, 0));
ut_asserteq(-EIO, generic_shutdown_phy(&phy));
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 3/3] phy: test: Implement sandbox PHY .set_mode and DM test
2024-06-17 17:36 ` [PATCH v2 3/3] phy: test: Implement sandbox PHY .set_mode and DM test Marek Vasut
@ 2024-07-09 9:33 ` Mattijs Korpershoek
0 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2024-07-09 9:33 UTC (permalink / raw)
To: Marek Vasut, u-boot
Cc: Marek Vasut, Caleb Connolly, Fabio Estevam, Fabrice Gasnier,
Jonas Karlman, Mathieu Othacehe, Neil Armstrong, Nishanth Menon,
Nobuhiro Iwamatsu, Sean Anderson, Simon Glass, Sumit Garg,
Tim Harvey, Tom Rini, Xavier Drudis Ferran, u-boot-qcom
Hi Marek,
Thank you for the patch.
On lun., juin 17, 2024 at 19:36, Marek Vasut <marek.vasut+renesas@mailbox.org> wrote:
> Implement trivial extension to the sandbox PHY, which makes it pretend
> to support selecting USB Host mode and nothing else. Any other mode is
> rejected with -EINVAL. Any submode except for default submode 0 is
> rejected with -EOPNOTSUPP . The implementation behaves in this trivial
> way to permit easy unit testing using test which is also added in this
> commit.
>
> To run the test, use e.g. sandbox64_defconfig and run U-Boot as follows:
> $ ./u-boot -Tc 'ut dm phy_setup'
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
> Cc: Caleb Connolly <caleb.connolly@linaro.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Mathieu Othacehe <othacehe@gnu.org>
> Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Cc: Sean Anderson <seanga2@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Xavier Drudis Ferran <xdrudis@tinet.cat>
> Cc: u-boot-qcom@groups.io
> Cc: u-boot@lists.denx.de
> ---
> V2: New patch
> ---
> drivers/phy/sandbox-phy.c | 13 +++++++++++++
> test/dm/phy.c | 7 +++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/drivers/phy/sandbox-phy.c b/drivers/phy/sandbox-phy.c
> index b159147a765..e70d20432e0 100644
> --- a/drivers/phy/sandbox-phy.c
> +++ b/drivers/phy/sandbox-phy.c
> @@ -72,6 +72,18 @@ static int sandbox_phy_exit(struct phy *phy)
> return 0;
> }
>
> +static int
> +sandbox_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
> +{
> + if (submode)
> + return -EOPNOTSUPP;
> +
> + if (mode != PHY_MODE_USB_HOST)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int sandbox_phy_bind(struct udevice *dev)
> {
> if (dev_get_driver_data(dev) != DRIVER_DATA)
> @@ -96,6 +108,7 @@ static struct phy_ops sandbox_phy_ops = {
> .power_off = sandbox_phy_power_off,
> .init = sandbox_phy_init,
> .exit = sandbox_phy_exit,
> + .set_mode = sandbox_phy_set_mode,
> };
>
> static const struct udevice_id sandbox_phy_ids[] = {
> diff --git a/test/dm/phy.c b/test/dm/phy.c
> index a90881b12ab..a93aa83ab10 100644
> --- a/test/dm/phy.c
> +++ b/test/dm/phy.c
> @@ -246,6 +246,13 @@ static int dm_test_phy_setup(struct unit_test_state *uts)
> ut_assertok(generic_setup_phy(parent, &phy, 0, PHY_MODE_USB_HOST, 0));
> ut_assertok(generic_shutdown_phy(&phy));
>
> + /* set_mode as USB Host passes, anything else is not supported */
> + ut_assertok(generic_setup_phy(parent, &phy, 0, PHY_MODE_USB_HOST, 0));
> + ut_assertok(generic_phy_set_mode(&phy, PHY_MODE_USB_HOST, 0));
> + ut_asserteq(-EOPNOTSUPP, generic_phy_set_mode(&phy, PHY_MODE_USB_HOST, 1));
> + ut_asserteq(-EINVAL, generic_phy_set_mode(&phy, PHY_MODE_USB_DEVICE, 0));
> + ut_assertok(generic_shutdown_phy(&phy));
> +
> /* power_off fail with -EIO */
> ut_assertok(generic_setup_phy(parent, &phy, 1, PHY_MODE_USB_HOST, 0));
> ut_asserteq(-EIO, generic_shutdown_phy(&phy));
> --
> 2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] phy: Extend generic_setup_phy() with PHY mode and submode
2024-06-17 17:36 [PATCH v2 1/3] phy: Extend generic_setup_phy() with PHY mode and submode Marek Vasut
2024-06-17 17:36 ` [PATCH v2 2/3] phy: rcar: Split init and set_mode operations Marek Vasut
2024-06-17 17:36 ` [PATCH v2 3/3] phy: test: Implement sandbox PHY .set_mode and DM test Marek Vasut
@ 2024-07-09 9:20 ` Mattijs Korpershoek
2024-09-08 18:11 ` Marek Vasut
3 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2024-07-09 9:20 UTC (permalink / raw)
To: Marek Vasut, u-boot
Cc: Marek Vasut, Caleb Connolly, Fabio Estevam, Fabrice Gasnier,
Jonas Karlman, Mathieu Othacehe, Neil Armstrong, Nishanth Menon,
Nobuhiro Iwamatsu, Sean Anderson, Simon Glass, Sumit Garg,
Tim Harvey, Tom Rini, Xavier Drudis Ferran, u-boot-qcom
Hi Marek,
Thank you for the patch.
On lun., juin 17, 2024 at 19:36, Marek Vasut <marek.vasut+renesas@mailbox.org> wrote:
> Extend generic_setup_phy() parameter list with PHY mode and submode and
> call generic_phy_set_mode() in generic_setup_phy(), so the generic PHY
> setup function can configure the PHY into correct mode before powering
> the PHY up.
>
> Update all call sites of generic_setup_phy() as well, all of which are
> USB host related, except for DM test which now behaves as a USB host
> test.
>
> Note that if the PHY driver does not implement the .set_mode callback,
> generic_phy_set_mode() call returns 0 and does not error out, so this
> should not break any existing systems.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
> Cc: Caleb Connolly <caleb.connolly@linaro.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Mathieu Othacehe <othacehe@gnu.org>
> Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Cc: Sean Anderson <seanga2@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Xavier Drudis Ferran <xdrudis@tinet.cat>
> Cc: u-boot-qcom@groups.io
> Cc: u-boot@lists.denx.de
> ---
> V2: Add failpath to return errno from generic_phy_set_mode()
> ---
> drivers/phy/phy-uclass.c | 13 +++++++++++--
> drivers/usb/host/ehci-generic.c | 2 +-
> drivers/usb/host/ehci-msm.c | 2 +-
> drivers/usb/host/ehci-mx6.c | 2 +-
> drivers/usb/host/ehci-pci.c | 2 +-
> drivers/usb/host/ohci-generic.c | 2 +-
> include/generic-phy.h | 5 ++++-
> test/dm/phy.c | 8 ++++----
> 8 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
> index acdcda15b5b..777d952b041 100644
> --- a/drivers/phy/phy-uclass.c
> +++ b/drivers/phy/phy-uclass.c
> @@ -508,7 +508,8 @@ int generic_phy_power_off_bulk(struct phy_bulk *bulk)
> return ret;
> }
>
> -int generic_setup_phy(struct udevice *dev, struct phy *phy, int index)
> +int generic_setup_phy(struct udevice *dev, struct phy *phy, int index,
> + enum phy_mode mode, int submode)
> {
> int ret;
>
> @@ -520,10 +521,18 @@ int generic_setup_phy(struct udevice *dev, struct phy *phy, int index)
> if (ret)
> return ret;
>
> + ret = generic_phy_set_mode(phy, mode, submode);
> + if (ret)
> + goto phys_mode_err;
> +
> ret = generic_phy_power_on(phy);
> if (ret)
> - generic_phy_exit(phy);
> + goto phys_mode_err;
> +
> + return 0;
>
> +phys_mode_err:
> + generic_phy_exit(phy);
> return ret;
> }
>
> diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c
> index 23c3ed25554..1ae3619ce25 100644
> --- a/drivers/usb/host/ehci-generic.c
> +++ b/drivers/usb/host/ehci-generic.c
> @@ -94,7 +94,7 @@ static int ehci_usb_probe(struct udevice *dev)
> if (err)
> goto reset_err;
>
> - err = generic_setup_phy(dev, &priv->phy, 0);
> + err = generic_setup_phy(dev, &priv->phy, 0, PHY_MODE_USB_HOST, 0);
> if (err)
> goto regulator_err;
>
> diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
> index a081f71b187..b0c060b8173 100644
> --- a/drivers/usb/host/ehci-msm.c
> +++ b/drivers/usb/host/ehci-msm.c
> @@ -56,7 +56,7 @@ static int ehci_usb_probe(struct udevice *dev)
> hcor = (struct ehci_hcor *)((phys_addr_t)hccr +
> HC_LENGTH(ehci_readl(&(hccr)->cr_capbase)));
>
> - ret = generic_setup_phy(dev, &p->phy, 0);
> + ret = generic_setup_phy(dev, &p->phy, 0, PHY_MODE_USB_HOST, 0);
> if (ret)
> return ret;
>
> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> index 31cd8a50f4a..a93fa5d5455 100644
> --- a/drivers/usb/host/ehci-mx6.c
> +++ b/drivers/usb/host/ehci-mx6.c
> @@ -703,7 +703,7 @@ static int ehci_usb_probe(struct udevice *dev)
> usb_phy_enable(ehci, priv->phy_addr);
> #endif
> #else
> - ret = generic_setup_phy(dev, &priv->phy, 0);
> + ret = generic_setup_phy(dev, &priv->phy, 0, PHY_MODE_USB_HOST, 0);
> if (ret)
> goto err_regulator;
> #endif
> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index 572686580cd..8d05b14e898 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -30,7 +30,7 @@ static int ehci_pci_init(struct udevice *dev, struct ehci_hccr **ret_hccr,
> int ret;
> u32 cmd;
>
> - ret = generic_setup_phy(dev, &priv->phy, 0);
> + ret = generic_setup_phy(dev, &priv->phy, 0, PHY_MODE_USB_HOST, 0);
> if (ret)
> return ret;
>
> diff --git a/drivers/usb/host/ohci-generic.c b/drivers/usb/host/ohci-generic.c
> index f1325cd4953..cc44226f5e0 100644
> --- a/drivers/usb/host/ohci-generic.c
> +++ b/drivers/usb/host/ohci-generic.c
> @@ -50,7 +50,7 @@ static int ohci_usb_probe(struct udevice *dev)
> goto reset_err;
> }
>
> - err = generic_setup_phy(dev, &priv->phy, 0);
> + err = generic_setup_phy(dev, &priv->phy, 0, PHY_MODE_USB_HOST, 0);
> if (err)
> goto reset_err;
>
> diff --git a/include/generic-phy.h b/include/generic-phy.h
> index eaab7491660..47962023236 100644
> --- a/include/generic-phy.h
> +++ b/include/generic-phy.h
> @@ -415,10 +415,13 @@ int generic_phy_power_off_bulk(struct phy_bulk *bulk);
> * @dev: The consumer device.
> * @phy: A pointer to the PHY port
> * @index: The index in the list of available PHYs
> + * @mode: PHY mode
> + * @submode: PHY submode
> *
> * Return: 0 if OK, or negative error code.
> */
> -int generic_setup_phy(struct udevice *dev, struct phy *phy, int index);
> +int generic_setup_phy(struct udevice *dev, struct phy *phy, int index,
> + enum phy_mode mode, int submode);
>
> /**
> * generic_shutdown_phy() - Power off and de-initialize phy.
> diff --git a/test/dm/phy.c b/test/dm/phy.c
> index d14117f6f7a..a90881b12ab 100644
> --- a/test/dm/phy.c
> +++ b/test/dm/phy.c
> @@ -243,20 +243,20 @@ static int dm_test_phy_setup(struct unit_test_state *uts)
> "gen_phy_user", &parent));
>
> /* normal */
> - ut_assertok(generic_setup_phy(parent, &phy, 0));
> + ut_assertok(generic_setup_phy(parent, &phy, 0, PHY_MODE_USB_HOST, 0));
> ut_assertok(generic_shutdown_phy(&phy));
>
> /* power_off fail with -EIO */
> - ut_assertok(generic_setup_phy(parent, &phy, 1));
> + ut_assertok(generic_setup_phy(parent, &phy, 1, PHY_MODE_USB_HOST, 0));
> ut_asserteq(-EIO, generic_shutdown_phy(&phy));
>
> /* power_on fail with -EIO */
> - ut_asserteq(-EIO, generic_setup_phy(parent, &phy, 2));
> + ut_asserteq(-EIO, generic_setup_phy(parent, &phy, 2, PHY_MODE_USB_HOST, 0));
> ut_assertok(generic_shutdown_phy(&phy));
>
> /* generic_phy_get_by_index fail with -ENOENT */
> ut_asserteq(-ENOENT, generic_phy_get_by_index(parent, 3, &phy));
> - ut_assertok(generic_setup_phy(parent, &phy, 3));
> + ut_assertok(generic_setup_phy(parent, &phy, 3, PHY_MODE_USB_HOST, 0));
> ut_assertok(generic_shutdown_phy(&phy));
>
> return 0;
> --
> 2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/3] phy: Extend generic_setup_phy() with PHY mode and submode
2024-06-17 17:36 [PATCH v2 1/3] phy: Extend generic_setup_phy() with PHY mode and submode Marek Vasut
` (2 preceding siblings ...)
2024-07-09 9:20 ` [PATCH v2 1/3] phy: Extend generic_setup_phy() with PHY mode and submode Mattijs Korpershoek
@ 2024-09-08 18:11 ` Marek Vasut
2024-09-09 7:16 ` Mattijs Korpershoek
3 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2024-09-08 18:11 UTC (permalink / raw)
To: Marek Vasut, u-boot, Tom Rini
Cc: Caleb Connolly, Fabio Estevam, Fabrice Gasnier, Jonas Karlman,
Mathieu Othacehe, Mattijs Korpershoek, Neil Armstrong,
Nishanth Menon, Nobuhiro Iwamatsu, Sean Anderson, Simon Glass,
Sumit Garg, Tim Harvey, Xavier Drudis Ferran, u-boot-qcom
On 6/17/24 7:36 PM, Marek Vasut wrote:
> Extend generic_setup_phy() parameter list with PHY mode and submode and
> call generic_phy_set_mode() in generic_setup_phy(), so the generic PHY
> setup function can configure the PHY into correct mode before powering
> the PHY up.
>
> Update all call sites of generic_setup_phy() as well, all of which are
> USB host related, except for DM test which now behaves as a USB host
> test.
>
> Note that if the PHY driver does not implement the .set_mode callback,
> generic_phy_set_mode() call returns 0 and does not error out, so this
> should not break any existing systems.
Any news here ?
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/3] phy: Extend generic_setup_phy() with PHY mode and submode
2024-09-08 18:11 ` Marek Vasut
@ 2024-09-09 7:16 ` Mattijs Korpershoek
0 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2024-09-09 7:16 UTC (permalink / raw)
To: Marek Vasut, Marek Vasut, u-boot, Tom Rini
Cc: Caleb Connolly, Fabio Estevam, Fabrice Gasnier, Jonas Karlman,
Mathieu Othacehe, Neil Armstrong, Nishanth Menon,
Nobuhiro Iwamatsu, Sean Anderson, Simon Glass, Sumit Garg,
Tim Harvey, Xavier Drudis Ferran, u-boot-qcom
On dim., sept. 08, 2024 at 20:11, Marek Vasut <marek.vasut@mailbox.org> wrote:
> On 6/17/24 7:36 PM, Marek Vasut wrote:
>> Extend generic_setup_phy() parameter list with PHY mode and submode and
>> call generic_phy_set_mode() in generic_setup_phy(), so the generic PHY
>> setup function can configure the PHY into correct mode before powering
>> the PHY up.
>>
>> Update all call sites of generic_setup_phy() as well, all of which are
>> USB host related, except for DM test which now behaves as a USB host
>> test.
>>
>> Note that if the PHY driver does not implement the .set_mode callback,
>> generic_phy_set_mode() call returns 0 and does not error out, so this
>> should not break any existing systems.
> Any news here ?
According to patchwork, this is assigned to you:
https://patchwork.ozlabs.org/project/uboot/patch/20240617173740.80822-1-marek.vasut+renesas@mailbox.org/
^ permalink raw reply [flat|nested] 8+ messages in thread