2020-11-23 17:04:54

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 0/6] STM32 USBPHYC PLL management rework

STM32 USBPHYC controls the USB PLL. PLL requires to be powered with 1v1 and 1v8
supplies. To ensure a good behavior of the PLL, during boot, runtime and
suspend/resume sequences, this series reworks its management to fix regulators
issues and improve PLL status reliability.

Amelie Delaunay (6):
dt-bindings: phy: phy-stm32-usbphyc: move PLL supplies to parent node
phy: stm32: manage 1v1 and 1v8 supplies at pll activation/deactivation
phy: stm32: replace regulator_bulk* by multiple regulator_*
phy: stm32: ensure pll is disabled before phys creation
phy: stm32: ensure phy are no more active when removing the driver
phy: stm32: rework PLL Lock detection

.../bindings/phy/phy-stm32-usbphyc.yaml | 22 +-
drivers/phy/st/phy-stm32-usbphyc.c | 222 +++++++++++-------
2 files changed, 153 insertions(+), 91 deletions(-)

--
2.17.1


2020-11-23 17:04:58

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 3/6] phy: stm32: replace regulator_bulk* by multiple regulator_*

Due to async_schedule_domain call in regulator_bulk_enable,
scheduling while atomic bug can raise if regulator_bulk_enable is called
under atomic context.
To avoid this issue, this patch replaces all regulator_bulk* by regulator_
per regulators.

Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/phy/st/phy-stm32-usbphyc.c | 69 ++++++++++++++++++++++--------
1 file changed, 52 insertions(+), 17 deletions(-)

diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
index c78a2c7947ce..8ef97c8806ff 100644
--- a/drivers/phy/st/phy-stm32-usbphyc.c
+++ b/drivers/phy/st/phy-stm32-usbphyc.c
@@ -36,13 +36,6 @@
#define MINREV GENMASK(3, 0)
#define MAJREV GENMASK(7, 4)

-static const char * const supplies_names[] = {
- "vdda1v1", /* 1V1 */
- "vdda1v8", /* 1V8 */
-};
-
-#define NUM_SUPPLIES ARRAY_SIZE(supplies_names)
-
#define PLL_LOCK_TIME_US 100
#define PLL_PWR_DOWN_TIME_US 5
#define PLL_FVCO_MHZ 2880
@@ -69,7 +62,8 @@ struct stm32_usbphyc {
struct reset_control *rst;
struct stm32_usbphyc_phy **phys;
int nphys;
- struct regulator_bulk_data supplies[NUM_SUPPLIES];
+ struct regulator *vdda1v1;
+ struct regulator *vdda1v8;
int switch_setup;
};

@@ -83,6 +77,41 @@ static inline void stm32_usbphyc_clr_bits(void __iomem *reg, u32 bits)
writel_relaxed(readl_relaxed(reg) & ~bits, reg);
}

+static int stm32_usbphyc_regulators_enable(struct stm32_usbphyc *usbphyc)
+{
+ int ret;
+
+ ret = regulator_enable(usbphyc->vdda1v1);
+ if (ret)
+ return ret;
+
+ ret = regulator_enable(usbphyc->vdda1v8);
+ if (ret)
+ goto vdda1v1_disable;
+
+ return 0;
+
+vdda1v1_disable:
+ regulator_disable(usbphyc->vdda1v1);
+
+ return ret;
+}
+
+static int stm32_usbphyc_regulators_disable(struct stm32_usbphyc *usbphyc)
+{
+ int ret;
+
+ ret = regulator_disable(usbphyc->vdda1v8);
+ if (ret)
+ return ret;
+
+ ret = regulator_disable(usbphyc->vdda1v1);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static void stm32_usbphyc_get_pll_params(u32 clk_rate,
struct pll_params *pll_params)
{
@@ -170,7 +199,7 @@ static int stm32_usbphyc_pll_disable(struct stm32_usbphyc *usbphyc)
return -EIO;
}

- return regulator_bulk_disable(NUM_SUPPLIES, usbphyc->supplies);
+ return stm32_usbphyc_regulators_disable(usbphyc);
}

static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
@@ -189,7 +218,7 @@ static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
return ret;
}

- ret = regulator_bulk_enable(NUM_SUPPLIES, usbphyc->supplies);
+ ret = stm32_usbphyc_regulators_enable(usbphyc);
if (ret)
return ret;

@@ -210,7 +239,7 @@ static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
return 0;

reg_disable:
- regulator_bulk_disable(NUM_SUPPLIES, usbphyc->supplies);
+ stm32_usbphyc_regulators_disable(usbphyc);

return ret;
}
@@ -306,7 +335,7 @@ static int stm32_usbphyc_probe(struct platform_device *pdev)
struct device_node *child, *np = dev->of_node;
struct phy_provider *phy_provider;
u32 version;
- int ret, i, port = 0;
+ int ret, port = 0;

usbphyc = devm_kzalloc(dev, sizeof(*usbphyc), GFP_KERNEL);
if (!usbphyc)
@@ -348,13 +377,19 @@ static int stm32_usbphyc_probe(struct platform_device *pdev)
goto clk_disable;
}

- for (i = 0; i < NUM_SUPPLIES; i++)
- usbphyc->supplies[i].supply = supplies_names[i];
+ usbphyc->vdda1v1 = devm_regulator_get(dev, "vdda1v1");
+ if (IS_ERR(usbphyc->vdda1v1)) {
+ ret = PTR_ERR(usbphyc->vdda1v1);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "failed to get vdda1v1 supply: %d\n", ret);
+ goto clk_disable;
+ }

- ret = devm_regulator_bulk_get(dev, NUM_SUPPLIES, usbphyc->supplies);
- if (ret) {
+ usbphyc->vdda1v8 = devm_regulator_get(dev, "vdda1v8");
+ if (IS_ERR(usbphyc->vdda1v8)) {
+ ret = PTR_ERR(usbphyc->vdda1v8);
if (ret != -EPROBE_DEFER)
- dev_err(dev, "failed to get regulators: %d\n", ret);
+ dev_err(dev, "failed to get vdda1v8 supply: %d\n", ret);
goto clk_disable;
}

--
2.17.1

2020-11-23 17:05:01

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 2/6] phy: stm32: manage 1v1 and 1v8 supplies at pll activation/deactivation

PLL block requires to be powered with 1v1 and 1v8 supplies to catch
ENABLE signal.
Currently, supplies are managed through phy_ops .power_on/off, and PLL
activation/deactivation is managed through phy_ops .init/exit.
The sequence of phy_ops .power_on/.phy_init, .power_off/.exit is USB
drivers dependent.
To ensure a good behavior of the PLL, supplies have to be managed at PLL
activation/deactivation. That means the supplies need to be put in usbphyc
node and not in phy children nodes.

Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/phy/st/phy-stm32-usbphyc.c | 102 +++++++++++++----------------
1 file changed, 46 insertions(+), 56 deletions(-)

diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
index a54317e96c41..c78a2c7947ce 100644
--- a/drivers/phy/st/phy-stm32-usbphyc.c
+++ b/drivers/phy/st/phy-stm32-usbphyc.c
@@ -58,7 +58,6 @@ struct pll_params {
struct stm32_usbphyc_phy {
struct phy *phy;
struct stm32_usbphyc *usbphyc;
- struct regulator_bulk_data supplies[NUM_SUPPLIES];
u32 index;
bool active;
};
@@ -70,6 +69,7 @@ struct stm32_usbphyc {
struct reset_control *rst;
struct stm32_usbphyc_phy **phys;
int nphys;
+ struct regulator_bulk_data supplies[NUM_SUPPLIES];
int switch_setup;
};

@@ -153,10 +153,30 @@ static bool stm32_usbphyc_has_one_phy_active(struct stm32_usbphyc *usbphyc)
return false;
}

+static int stm32_usbphyc_pll_disable(struct stm32_usbphyc *usbphyc)
+{
+ void __iomem *pll_reg = usbphyc->base + STM32_USBPHYC_PLL;
+
+ /* Check if other phy port active */
+ if (stm32_usbphyc_has_one_phy_active(usbphyc))
+ return 0;
+
+ stm32_usbphyc_clr_bits(pll_reg, PLLEN);
+ /* Wait for minimum width of powerdown pulse (ENABLE = Low) */
+ udelay(PLL_PWR_DOWN_TIME_US);
+
+ if (readl_relaxed(pll_reg) & PLLEN) {
+ dev_err(usbphyc->dev, "PLL not reset\n");
+ return -EIO;
+ }
+
+ return regulator_bulk_disable(NUM_SUPPLIES, usbphyc->supplies);
+}
+
static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
{
void __iomem *pll_reg = usbphyc->base + STM32_USBPHYC_PLL;
- bool pllen = (readl_relaxed(pll_reg) & PLLEN);
+ bool pllen = readl_relaxed(pll_reg) & PLLEN;
int ret;

/* Check if one phy port has already configured the pll */
@@ -164,46 +184,35 @@ static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
return 0;

if (pllen) {
- stm32_usbphyc_clr_bits(pll_reg, PLLEN);
- /* Wait for minimum width of powerdown pulse (ENABLE = Low) */
- udelay(PLL_PWR_DOWN_TIME_US);
+ ret = stm32_usbphyc_pll_disable(usbphyc);
+ if (ret)
+ return ret;
}

- ret = stm32_usbphyc_pll_init(usbphyc);
+ ret = regulator_bulk_enable(NUM_SUPPLIES, usbphyc->supplies);
if (ret)
return ret;

- stm32_usbphyc_set_bits(pll_reg, PLLEN);
+ ret = stm32_usbphyc_pll_init(usbphyc);
+ if (ret)
+ goto reg_disable;

+ stm32_usbphyc_set_bits(pll_reg, PLLEN);
/* Wait for maximum lock time */
udelay(PLL_LOCK_TIME_US);

if (!(readl_relaxed(pll_reg) & PLLEN)) {
dev_err(usbphyc->dev, "PLLEN not set\n");
- return -EIO;
+ ret = -EIO;
+ goto reg_disable;
}

return 0;
-}
-
-static int stm32_usbphyc_pll_disable(struct stm32_usbphyc *usbphyc)
-{
- void __iomem *pll_reg = usbphyc->base + STM32_USBPHYC_PLL;
-
- /* Check if other phy port active */
- if (stm32_usbphyc_has_one_phy_active(usbphyc))
- return 0;

- stm32_usbphyc_clr_bits(pll_reg, PLLEN);
- /* Wait for minimum width of powerdown pulse (ENABLE = Low) */
- udelay(PLL_PWR_DOWN_TIME_US);
+reg_disable:
+ regulator_bulk_disable(NUM_SUPPLIES, usbphyc->supplies);

- if (readl_relaxed(pll_reg) & PLLEN) {
- dev_err(usbphyc->dev, "PLL not reset\n");
- return -EIO;
- }
-
- return 0;
+ return ret;
}

static int stm32_usbphyc_phy_init(struct phy *phy)
@@ -231,25 +240,9 @@ static int stm32_usbphyc_phy_exit(struct phy *phy)
return stm32_usbphyc_pll_disable(usbphyc);
}

-static int stm32_usbphyc_phy_power_on(struct phy *phy)
-{
- struct stm32_usbphyc_phy *usbphyc_phy = phy_get_drvdata(phy);
-
- return regulator_bulk_enable(NUM_SUPPLIES, usbphyc_phy->supplies);
-}
-
-static int stm32_usbphyc_phy_power_off(struct phy *phy)
-{
- struct stm32_usbphyc_phy *usbphyc_phy = phy_get_drvdata(phy);
-
- return regulator_bulk_disable(NUM_SUPPLIES, usbphyc_phy->supplies);
-}
-
static const struct phy_ops stm32_usbphyc_phy_ops = {
.init = stm32_usbphyc_phy_init,
.exit = stm32_usbphyc_phy_exit,
- .power_on = stm32_usbphyc_phy_power_on,
- .power_off = stm32_usbphyc_phy_power_off,
.owner = THIS_MODULE,
};

@@ -313,7 +306,7 @@ static int stm32_usbphyc_probe(struct platform_device *pdev)
struct device_node *child, *np = dev->of_node;
struct phy_provider *phy_provider;
u32 version;
- int ret, port = 0;
+ int ret, i, port = 0;

usbphyc = devm_kzalloc(dev, sizeof(*usbphyc), GFP_KERNEL);
if (!usbphyc)
@@ -355,11 +348,20 @@ static int stm32_usbphyc_probe(struct platform_device *pdev)
goto clk_disable;
}

+ for (i = 0; i < NUM_SUPPLIES; i++)
+ usbphyc->supplies[i].supply = supplies_names[i];
+
+ ret = devm_regulator_bulk_get(dev, NUM_SUPPLIES, usbphyc->supplies);
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "failed to get regulators: %d\n", ret);
+ goto clk_disable;
+ }
+
for_each_child_of_node(np, child) {
struct stm32_usbphyc_phy *usbphyc_phy;
struct phy *phy;
u32 index;
- int i;

phy = devm_phy_create(dev, child, &stm32_usbphyc_phy_ops);
if (IS_ERR(phy)) {
@@ -377,18 +379,6 @@ static int stm32_usbphyc_probe(struct platform_device *pdev)
goto put_child;
}

- for (i = 0; i < NUM_SUPPLIES; i++)
- usbphyc_phy->supplies[i].supply = supplies_names[i];
-
- ret = devm_regulator_bulk_get(&phy->dev, NUM_SUPPLIES,
- usbphyc_phy->supplies);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- dev_err(&phy->dev,
- "failed to get regulators: %d\n", ret);
- goto put_child;
- }
-
ret = of_property_read_u32(child, "reg", &index);
if (ret || index > usbphyc->nphys) {
dev_err(&phy->dev, "invalid reg property: %d\n", ret);
--
2.17.1

2020-11-23 17:05:37

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 1/6] dt-bindings: phy: phy-stm32-usbphyc: move PLL supplies to parent node

PLL block requires to be powered with 1v1 and 1v8 supplies to catch ENABLE
signal.
Currently, supplies are managed through phy_ops .power_on/off, and PLL
activation/deactivation is managed through phy_ops .init/exit.
The sequence of phy_ops .power_on/.phy_init, .power_off/.exit is USB
drivers dependent.
To ensure a good behavior of the PLL, supplies have to be managed at PLL
activation/deactivation. That means the supplies need to be put in usbphyc
parent node and not in phy children nodes.

Signed-off-by: Amelie Delaunay <[email protected]>
---
Note that even with bindings change, it doesn't break the backward
compatibility: old device trees are still compatible, USB is still
functional. Device trees will be updated with this new bindings
when approved.
---
.../bindings/phy/phy-stm32-usbphyc.yaml | 22 +++++++++----------
1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml b/Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml
index 0ba61979b970..46df6786727a 100644
--- a/Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml
+++ b/Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml
@@ -45,6 +45,12 @@ properties:
"#size-cells":
const: 0

+ vdda1v1-supply:
+ description: regulator providing 1V1 power supply to the PLL block
+
+ vdda1v8-supply:
+ description: regulator providing 1V8 power supply to the PLL block
+
#Required child nodes:

patternProperties:
@@ -61,12 +67,6 @@ patternProperties:
phy-supply:
description: regulator providing 3V3 power supply to the PHY.

- vdda1v1-supply:
- description: regulator providing 1V1 power supply to the PLL block
-
- vdda1v8-supply:
- description: regulator providing 1V8 power supply to the PLL block
-
"#phy-cells":
enum: [ 0x0, 0x1 ]

@@ -90,8 +90,6 @@ patternProperties:
required:
- reg
- phy-supply
- - vdda1v1-supply
- - vdda1v8-supply
- "#phy-cells"

additionalProperties: false
@@ -102,6 +100,8 @@ required:
- clocks
- "#address-cells"
- "#size-cells"
+ - vdda1v1-supply
+ - vdda1v8-supply
- usb-phy@0
- usb-phy@1

@@ -116,22 +116,20 @@ examples:
reg = <0x5a006000 0x1000>;
clocks = <&rcc USBPHY_K>;
resets = <&rcc USBPHY_R>;
+ vdda1v1-supply = <&reg11>;
+ vdda1v8-supply = <&reg18>;
#address-cells = <1>;
#size-cells = <0>;

usbphyc_port0: usb-phy@0 {
reg = <0>;
phy-supply = <&vdd_usb>;
- vdda1v1-supply = <&reg11>;
- vdda1v8-supply = <&reg18>;
#phy-cells = <0>;
};

usbphyc_port1: usb-phy@1 {
reg = <1>;
phy-supply = <&vdd_usb>;
- vdda1v1-supply = <&reg11>;
- vdda1v8-supply = <&reg18>;
#phy-cells = <1>;
};
};
--
2.17.1

2020-11-23 17:08:12

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 6/6] phy: stm32: rework PLL Lock detection

USBPHYC has a register per phy to control and monitor the debug interface
of the HS PHY through a digital debug access.
With this register, it is possible to know if PLL Lock input to phy is
high. That means the PLL is ready for HS operation.
Instead of using an hard-coded delay after PLL enable and PLL disable, use
this bit to ensure good operating of the HS PHY.
Also use an atomic counter (n_pll_cons) to count the actual number of PLL
consumers and get rid of stm32_usbphyc_has_one_phy_active.
The boolean active in the usbphyc_phy structure is kept, because we need to
know in remove if a phy_exit is required to properly disable the PLL.

Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/phy/st/phy-stm32-usbphyc.c | 88 ++++++++++++++++++------------
1 file changed, 54 insertions(+), 34 deletions(-)

diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
index 8b11d95b2c20..d08fbb180e43 100644
--- a/drivers/phy/st/phy-stm32-usbphyc.c
+++ b/drivers/phy/st/phy-stm32-usbphyc.c
@@ -17,6 +17,7 @@

#define STM32_USBPHYC_PLL 0x0
#define STM32_USBPHYC_MISC 0x8
+#define STM32_USBPHYC_MONITOR(X) (0x108 + ((X) * 0x100))
#define STM32_USBPHYC_VERSION 0x3F4

/* STM32_USBPHYC_PLL bit fields */
@@ -32,12 +33,16 @@
/* STM32_USBPHYC_MISC bit fields */
#define SWITHOST BIT(0)

+/* STM32_USBPHYC_MONITOR bit fields */
+#define STM32_USBPHYC_MON_OUT GENMASK(3, 0)
+#define STM32_USBPHYC_MON_SEL GENMASK(8, 4)
+#define STM32_USBPHYC_MON_SEL_LOCKP 0x1F
+#define STM32_USBPHYC_MON_OUT_LOCKP BIT(3)
+
/* STM32_USBPHYC_VERSION bit fields */
#define MINREV GENMASK(3, 0)
#define MAJREV GENMASK(7, 4)

-#define PLL_LOCK_TIME_US 100
-#define PLL_PWR_DOWN_TIME_US 5
#define PLL_FVCO_MHZ 2880
#define PLL_INFF_MIN_RATE_HZ 19200000
#define PLL_INFF_MAX_RATE_HZ 38400000
@@ -64,6 +69,7 @@ struct stm32_usbphyc {
int nphys;
struct regulator *vdda1v1;
struct regulator *vdda1v8;
+ atomic_t n_pll_cons;
int switch_setup;
};

@@ -171,35 +177,27 @@ static int stm32_usbphyc_pll_init(struct stm32_usbphyc *usbphyc)
return 0;
}

-static bool stm32_usbphyc_has_one_phy_active(struct stm32_usbphyc *usbphyc)
+static int __stm32_usbphyc_pll_disable(struct stm32_usbphyc *usbphyc)
{
- int i;
+ void __iomem *pll_reg = usbphyc->base + STM32_USBPHYC_PLL;
+ u32 pllen;
+
+ stm32_usbphyc_clr_bits(pll_reg, PLLEN);

- for (i = 0; i < usbphyc->nphys; i++)
- if (usbphyc->phys[i]->active)
- return true;
+ /* Wait for minimum width of powerdown pulse (ENABLE = Low) */
+ if (readl_relaxed_poll_timeout(pll_reg, pllen, !(pllen & PLLEN), 5, 50))
+ dev_err(usbphyc->dev, "PLL not reset\n");

- return false;
+ return stm32_usbphyc_regulators_disable(usbphyc);
}

static int stm32_usbphyc_pll_disable(struct stm32_usbphyc *usbphyc)
{
- void __iomem *pll_reg = usbphyc->base + STM32_USBPHYC_PLL;
-
- /* Check if other phy port active */
- if (stm32_usbphyc_has_one_phy_active(usbphyc))
+ /* Check if a phy port is still active or clk48 in use */
+ if (atomic_dec_return(&usbphyc->n_pll_cons) > 0)
return 0;

- stm32_usbphyc_clr_bits(pll_reg, PLLEN);
- /* Wait for minimum width of powerdown pulse (ENABLE = Low) */
- udelay(PLL_PWR_DOWN_TIME_US);
-
- if (readl_relaxed(pll_reg) & PLLEN) {
- dev_err(usbphyc->dev, "PLL not reset\n");
- return -EIO;
- }
-
- return stm32_usbphyc_regulators_disable(usbphyc);
+ return __stm32_usbphyc_pll_disable(usbphyc);
}

static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
@@ -208,39 +206,43 @@ static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
bool pllen = readl_relaxed(pll_reg) & PLLEN;
int ret;

- /* Check if one phy port has already configured the pll */
- if (pllen && stm32_usbphyc_has_one_phy_active(usbphyc))
+ /*
+ * Check if a phy port or clk48 prepare has configured the pll
+ * and ensure the PLL is enabled
+ */
+ if (atomic_inc_return(&usbphyc->n_pll_cons) > 1 && pllen)
return 0;

if (pllen) {
- ret = stm32_usbphyc_pll_disable(usbphyc);
+ /*
+ * PLL shouldn't be enabled without known consumer,
+ * disable it and reinit n_pll_cons
+ */
+ dev_warn(usbphyc->dev, "PLL enabled without known consumers\n");
+
+ ret = __stm32_usbphyc_pll_disable(usbphyc);
if (ret)
return ret;
}

ret = stm32_usbphyc_regulators_enable(usbphyc);
if (ret)
- return ret;
+ goto dec_n_pll_cons;

ret = stm32_usbphyc_pll_init(usbphyc);
if (ret)
goto reg_disable;

stm32_usbphyc_set_bits(pll_reg, PLLEN);
- /* Wait for maximum lock time */
- udelay(PLL_LOCK_TIME_US);
-
- if (!(readl_relaxed(pll_reg) & PLLEN)) {
- dev_err(usbphyc->dev, "PLLEN not set\n");
- ret = -EIO;
- goto reg_disable;
- }

return 0;

reg_disable:
stm32_usbphyc_regulators_disable(usbphyc);

+dec_n_pll_cons:
+ atomic_dec(&usbphyc->n_pll_cons);
+
return ret;
}

@@ -248,15 +250,33 @@ static int stm32_usbphyc_phy_init(struct phy *phy)
{
struct stm32_usbphyc_phy *usbphyc_phy = phy_get_drvdata(phy);
struct stm32_usbphyc *usbphyc = usbphyc_phy->usbphyc;
+ u32 reg_mon = STM32_USBPHYC_MONITOR(usbphyc_phy->index);
+ u32 monsel = FIELD_PREP(STM32_USBPHYC_MON_SEL,
+ STM32_USBPHYC_MON_SEL_LOCKP);
+ u32 monout;
int ret;

ret = stm32_usbphyc_pll_enable(usbphyc);
if (ret)
return ret;

+ /* Check that PLL Lock input to PHY is High */
+ writel_relaxed(monsel, usbphyc->base + reg_mon);
+ ret = readl_relaxed_poll_timeout(usbphyc->base + reg_mon, monout,
+ (monout & STM32_USBPHYC_MON_OUT_LOCKP),
+ 100, 1000);
+ if (ret) {
+ dev_err(usbphyc->dev, "PLL Lock input to PHY is Low (val=%x)\n",
+ (u32)(monout & STM32_USBPHYC_MON_OUT));
+ goto pll_disable;
+ }
+
usbphyc_phy->active = true;

return 0;
+
+pll_disable:
+ return stm32_usbphyc_pll_disable(usbphyc);
}

static int stm32_usbphyc_phy_exit(struct phy *phy)
--
2.17.1

2020-11-24 00:34:00

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 5/6] phy: stm32: ensure phy are no more active when removing the driver

To ensure a good balancing of regulators, and allow PLL disabling when the
driver is removed, call stm32_usbphyc_phy_exit on each ports to set phys
inactive and disable PLL.

Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/phy/st/phy-stm32-usbphyc.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
index 33367a325612..8b11d95b2c20 100644
--- a/drivers/phy/st/phy-stm32-usbphyc.c
+++ b/drivers/phy/st/phy-stm32-usbphyc.c
@@ -470,6 +470,12 @@ static int stm32_usbphyc_probe(struct platform_device *pdev)
static int stm32_usbphyc_remove(struct platform_device *pdev)
{
struct stm32_usbphyc *usbphyc = dev_get_drvdata(&pdev->dev);
+ int port;
+
+ /* Ensure PHYs are not active, to allow PLL disabling */
+ for (port = 0; port < usbphyc->nphys; port++)
+ if (usbphyc->phys[port]->active)
+ stm32_usbphyc_phy_exit(usbphyc->phys[port]->phy);

clk_disable_unprepare(usbphyc->clk);

--
2.17.1

2020-12-07 22:53:10

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/6] dt-bindings: phy: phy-stm32-usbphyc: move PLL supplies to parent node

On Mon, 23 Nov 2020 18:01:27 +0100, Amelie Delaunay wrote:
> PLL block requires to be powered with 1v1 and 1v8 supplies to catch ENABLE
> signal.
> Currently, supplies are managed through phy_ops .power_on/off, and PLL
> activation/deactivation is managed through phy_ops .init/exit.
> The sequence of phy_ops .power_on/.phy_init, .power_off/.exit is USB
> drivers dependent.
> To ensure a good behavior of the PLL, supplies have to be managed at PLL
> activation/deactivation. That means the supplies need to be put in usbphyc
> parent node and not in phy children nodes.
>
> Signed-off-by: Amelie Delaunay <[email protected]>
> ---
> Note that even with bindings change, it doesn't break the backward
> compatibility: old device trees are still compatible, USB is still
> functional. Device trees will be updated with this new bindings
> when approved.
> ---
> .../bindings/phy/phy-stm32-usbphyc.yaml | 22 +++++++++----------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>

Reviewed-by: Rob Herring <[email protected]>