* [PATCH] phy: core: Add consumer device link support
@ 2019-11-04 14:37 Alexandre Torgue
2019-12-04 15:33 ` Alexandre Torgue
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Alexandre Torgue @ 2019-11-04 14:37 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Greg Kroah-Hartman
Cc: Yoshihiro Shimoda, Alexandre Torgue, linux-kernel, linux-usb,
linux-stm32
In order to enforce suspend/resume ordering, this commit creates link
between phy consumers and phy devices. This link avoids to suspend phy
before phy consumers.
Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
---
Hi,
To manage device_link in phy-core I had to "balance" get and put APIs a bit
more. Fot this reason, you'll find updates in Renesas usbhs rcar and rza drivers
as phy API changes.
Regards
Alex
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index b04f4fe85ac2..8dfb4868c8c3 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -29,7 +29,7 @@ static void devm_phy_release(struct device *dev, void *res)
{
struct phy *phy = *(struct phy **)res;
- phy_put(phy);
+ phy_put(dev, phy);
}
static void devm_phy_provider_release(struct device *dev, void *res)
@@ -566,12 +566,12 @@ struct phy *of_phy_get(struct device_node *np, const char *con_id)
EXPORT_SYMBOL_GPL(of_phy_get);
/**
- * phy_put() - release the PHY
- * @phy: the phy returned by phy_get()
+ * of_phy_put() - release the PHY
+ * @phy: the phy returned by of_phy_get()
*
- * Releases a refcount the caller received from phy_get().
+ * Releases a refcount the caller received from of_phy_get().
*/
-void phy_put(struct phy *phy)
+void of_phy_put(struct phy *phy)
{
if (!phy || IS_ERR(phy))
return;
@@ -584,6 +584,20 @@ void phy_put(struct phy *phy)
module_put(phy->ops->owner);
put_device(&phy->dev);
}
+EXPORT_SYMBOL_GPL(of_phy_put);
+
+/**
+ * phy_put() - release the PHY
+ * @dev: device that wants to release this phy
+ * @phy: the phy returned by phy_get()
+ *
+ * Releases a refcount the caller received from phy_get().
+ */
+void phy_put(struct device *dev, struct phy *phy)
+{
+ device_link_remove(dev, &phy->dev);
+ of_phy_put(phy);
+}
EXPORT_SYMBOL_GPL(phy_put);
/**
@@ -651,6 +665,7 @@ struct phy *phy_get(struct device *dev, const char *string)
{
int index = 0;
struct phy *phy;
+ struct device_link *link;
if (string == NULL) {
dev_WARN(dev, "missing string\n");
@@ -672,6 +687,13 @@ struct phy *phy_get(struct device *dev, const char *string)
get_device(&phy->dev);
+ link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
+ if (!link) {
+ dev_err(dev, "failed to create device link to %s\n",
+ dev_name(phy->dev.parent));
+ return ERR_PTR(-EINVAL);
+ }
+
return phy;
}
EXPORT_SYMBOL_GPL(phy_get);
@@ -765,6 +787,7 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
const char *con_id)
{
struct phy **ptr, *phy;
+ struct device_link *link;
ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
if (!ptr)
@@ -778,6 +801,13 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
devres_free(ptr);
}
+ link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
+ if (!link) {
+ dev_err(dev, "failed to create device link to %s\n",
+ dev_name(phy->dev.parent));
+ return ERR_PTR(-EINVAL);
+ }
+
return phy;
}
EXPORT_SYMBOL_GPL(devm_of_phy_get);
@@ -798,6 +828,7 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
int index)
{
struct phy **ptr, *phy;
+ struct device_link *link;
ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
if (!ptr)
@@ -819,6 +850,13 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
*ptr = phy;
devres_add(dev, ptr);
+ link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
+ if (!link) {
+ dev_err(dev, "failed to create device link to %s\n",
+ dev_name(phy->dev.parent));
+ return ERR_PTR(-EINVAL);
+ }
+
return phy;
}
EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index);
diff --git a/drivers/usb/renesas_usbhs/rcar2.c b/drivers/usb/renesas_usbhs/rcar2.c
index 440d213e1749..791908f8cf73 100644
--- a/drivers/usb/renesas_usbhs/rcar2.c
+++ b/drivers/usb/renesas_usbhs/rcar2.c
@@ -34,7 +34,7 @@ static int usbhs_rcar2_hardware_exit(struct platform_device *pdev)
struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
if (priv->phy) {
- phy_put(priv->phy);
+ phy_put(&pdev->dev, priv->phy);
priv->phy = NULL;
}
diff --git a/drivers/usb/renesas_usbhs/rza2.c b/drivers/usb/renesas_usbhs/rza2.c
index 021749594389..3eed3334a17f 100644
--- a/drivers/usb/renesas_usbhs/rza2.c
+++ b/drivers/usb/renesas_usbhs/rza2.c
@@ -29,7 +29,7 @@ static int usbhs_rza2_hardware_exit(struct platform_device *pdev)
{
struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
- phy_put(priv->phy);
+ phy_put(&pdev->dev, priv->phy);
priv->phy = NULL;
return 0;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 56d3a100006a..19eddd64c8f6 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -234,7 +234,8 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
const char *con_id);
struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
int index);
-void phy_put(struct phy *phy);
+void of_phy_put(struct phy *phy);
+void phy_put(struct device *dev, struct phy *phy);
void devm_phy_put(struct device *dev, struct phy *phy);
struct phy *of_phy_get(struct device_node *np, const char *con_id);
struct phy *of_phy_simple_xlate(struct device *dev,
@@ -419,7 +420,11 @@ static inline struct phy *devm_of_phy_get_by_index(struct device *dev,
return ERR_PTR(-ENOSYS);
}
-static inline void phy_put(struct phy *phy)
+static inline void of_phy_put(struct phy *phy)
+{
+}
+
+static inline void phy_put(struct device *dev, struct phy *phy)
{
}
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2019-11-04 14:37 [PATCH] phy: core: Add consumer device link support Alexandre Torgue
@ 2019-12-04 15:33 ` Alexandre Torgue
2019-12-04 17:19 ` Greg Kroah-Hartman
2020-01-07 11:51 ` Jon Hunter
2020-02-06 13:39 ` youling257
2 siblings, 1 reply; 16+ messages in thread
From: Alexandre Torgue @ 2019-12-04 15:33 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Greg Kroah-Hartman
Cc: Yoshihiro Shimoda, linux-kernel, linux-usb, linux-stm32
Hi,
Gentle ping, in case you missed this.
Regards
Alex
On 11/4/19 3:37 PM, Alexandre Torgue wrote:
> In order to enforce suspend/resume ordering, this commit creates link
> between phy consumers and phy devices. This link avoids to suspend phy
> before phy consumers.
>
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>
> ---
>
> Hi,
>
> To manage device_link in phy-core I had to "balance" get and put APIs a bit
> more. Fot this reason, you'll find updates in Renesas usbhs rcar and rza drivers
> as phy API changes.
>
> Regards
> Alex
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index b04f4fe85ac2..8dfb4868c8c3 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -29,7 +29,7 @@ static void devm_phy_release(struct device *dev, void *res)
> {
> struct phy *phy = *(struct phy **)res;
>
> - phy_put(phy);
> + phy_put(dev, phy);
> }
>
> static void devm_phy_provider_release(struct device *dev, void *res)
> @@ -566,12 +566,12 @@ struct phy *of_phy_get(struct device_node *np, const char *con_id)
> EXPORT_SYMBOL_GPL(of_phy_get);
>
> /**
> - * phy_put() - release the PHY
> - * @phy: the phy returned by phy_get()
> + * of_phy_put() - release the PHY
> + * @phy: the phy returned by of_phy_get()
> *
> - * Releases a refcount the caller received from phy_get().
> + * Releases a refcount the caller received from of_phy_get().
> */
> -void phy_put(struct phy *phy)
> +void of_phy_put(struct phy *phy)
> {
> if (!phy || IS_ERR(phy))
> return;
> @@ -584,6 +584,20 @@ void phy_put(struct phy *phy)
> module_put(phy->ops->owner);
> put_device(&phy->dev);
> }
> +EXPORT_SYMBOL_GPL(of_phy_put);
> +
> +/**
> + * phy_put() - release the PHY
> + * @dev: device that wants to release this phy
> + * @phy: the phy returned by phy_get()
> + *
> + * Releases a refcount the caller received from phy_get().
> + */
> +void phy_put(struct device *dev, struct phy *phy)
> +{
> + device_link_remove(dev, &phy->dev);
> + of_phy_put(phy);
> +}
> EXPORT_SYMBOL_GPL(phy_put);
>
> /**
> @@ -651,6 +665,7 @@ struct phy *phy_get(struct device *dev, const char *string)
> {
> int index = 0;
> struct phy *phy;
> + struct device_link *link;
>
> if (string == NULL) {
> dev_WARN(dev, "missing string\n");
> @@ -672,6 +687,13 @@ struct phy *phy_get(struct device *dev, const char *string)
>
> get_device(&phy->dev);
>
> + link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> + if (!link) {
> + dev_err(dev, "failed to create device link to %s\n",
> + dev_name(phy->dev.parent));
> + return ERR_PTR(-EINVAL);
> + }
> +
> return phy;
> }
> EXPORT_SYMBOL_GPL(phy_get);
> @@ -765,6 +787,7 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
> const char *con_id)
> {
> struct phy **ptr, *phy;
> + struct device_link *link;
>
> ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
> if (!ptr)
> @@ -778,6 +801,13 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
> devres_free(ptr);
> }
>
> + link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> + if (!link) {
> + dev_err(dev, "failed to create device link to %s\n",
> + dev_name(phy->dev.parent));
> + return ERR_PTR(-EINVAL);
> + }
> +
> return phy;
> }
> EXPORT_SYMBOL_GPL(devm_of_phy_get);
> @@ -798,6 +828,7 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
> int index)
> {
> struct phy **ptr, *phy;
> + struct device_link *link;
>
> ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
> if (!ptr)
> @@ -819,6 +850,13 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
> *ptr = phy;
> devres_add(dev, ptr);
>
> + link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> + if (!link) {
> + dev_err(dev, "failed to create device link to %s\n",
> + dev_name(phy->dev.parent));
> + return ERR_PTR(-EINVAL);
> + }
> +
> return phy;
> }
> EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index);
> diff --git a/drivers/usb/renesas_usbhs/rcar2.c b/drivers/usb/renesas_usbhs/rcar2.c
> index 440d213e1749..791908f8cf73 100644
> --- a/drivers/usb/renesas_usbhs/rcar2.c
> +++ b/drivers/usb/renesas_usbhs/rcar2.c
> @@ -34,7 +34,7 @@ static int usbhs_rcar2_hardware_exit(struct platform_device *pdev)
> struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
>
> if (priv->phy) {
> - phy_put(priv->phy);
> + phy_put(&pdev->dev, priv->phy);
> priv->phy = NULL;
> }
>
> diff --git a/drivers/usb/renesas_usbhs/rza2.c b/drivers/usb/renesas_usbhs/rza2.c
> index 021749594389..3eed3334a17f 100644
> --- a/drivers/usb/renesas_usbhs/rza2.c
> +++ b/drivers/usb/renesas_usbhs/rza2.c
> @@ -29,7 +29,7 @@ static int usbhs_rza2_hardware_exit(struct platform_device *pdev)
> {
> struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
>
> - phy_put(priv->phy);
> + phy_put(&pdev->dev, priv->phy);
> priv->phy = NULL;
>
> return 0;
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 56d3a100006a..19eddd64c8f6 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -234,7 +234,8 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
> const char *con_id);
> struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
> int index);
> -void phy_put(struct phy *phy);
> +void of_phy_put(struct phy *phy);
> +void phy_put(struct device *dev, struct phy *phy);
> void devm_phy_put(struct device *dev, struct phy *phy);
> struct phy *of_phy_get(struct device_node *np, const char *con_id);
> struct phy *of_phy_simple_xlate(struct device *dev,
> @@ -419,7 +420,11 @@ static inline struct phy *devm_of_phy_get_by_index(struct device *dev,
> return ERR_PTR(-ENOSYS);
> }
>
> -static inline void phy_put(struct phy *phy)
> +static inline void of_phy_put(struct phy *phy)
> +{
> +}
> +
> +static inline void phy_put(struct device *dev, struct phy *phy)
> {
> }
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2019-12-04 15:33 ` Alexandre Torgue
@ 2019-12-04 17:19 ` Greg Kroah-Hartman
0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2019-12-04 17:19 UTC (permalink / raw)
To: Alexandre Torgue
Cc: Kishon Vijay Abraham I, Yoshihiro Shimoda, linux-kernel,
linux-usb, linux-stm32
On Wed, Dec 04, 2019 at 04:33:14PM +0100, Alexandre Torgue wrote:
> Hi,
>
> Gentle ping, in case you missed this.
It's the middle of the merge window, we can't add anything to our trees
at this point in time, sorry.
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] phy: core: Add consumer device link support
2019-11-04 14:37 [PATCH] phy: core: Add consumer device link support Alexandre Torgue
2019-12-04 15:33 ` Alexandre Torgue
@ 2020-01-07 11:51 ` Jon Hunter
2020-01-08 7:29 ` Kishon Vijay Abraham I
2020-02-06 13:39 ` youling257
2 siblings, 1 reply; 16+ messages in thread
From: Jon Hunter @ 2020-01-07 11:51 UTC (permalink / raw)
To: Alexandre Torgue, Kishon Vijay Abraham I, Greg Kroah-Hartman
Cc: Yoshihiro Shimoda, linux-kernel, linux-usb, linux-stm32, linux-tegra
On 04/11/2019 14:37, Alexandre Torgue wrote:
> In order to enforce suspend/resume ordering, this commit creates link
> between phy consumers and phy devices. This link avoids to suspend phy
> before phy consumers.
>
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
With next-20200106 we are seeing a boot regression on Tegra124 Jetson
TK1 board. Bisect is pointing to this commit and reverting this on top
of -next fixes the problem.
The bootlog is showing the following crash on boot ...
[ 1.730024] 8<--- cut here ---
[ 1.733079] Unable to handle kernel paging request at virtual address fffffe7f
[ 1.740318] pgd = (ptrval)
[ 1.743021] [fffffe7f] *pgd=affff841, *pte=00000000, *ppte=00000000
[ 1.749304] Internal error: Oops: 27 [#1] SMP ARM
[ 1.754001] Modules linked in:
[ 1.757057] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.5.0-rc4-next-20200106-g9eb1b48ca4ce #1
[ 1.765654] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[ 1.771919] PC is at device_link_add+0x68/0x4d4
[ 1.776444] LR is at device_link_add+0x68/0x4d4
[ 1.780967] pc : [<c09832e4>] lr : [<c09832e4>] psr: 60000013
[ 1.787223] sp : ee0e1d60 ip : 60000013 fp : 00000005
[ 1.792439] r10: 00000000 r9 : 00000000 r8 : eefedd88
[ 1.797657] r7 : ee269c10 r6 : fffffdfb r5 : 00000001 r4 : 00000001
[ 1.804173] r3 : ee0d8000 r2 : 00000000 r1 : 00000000 r0 : c1858f88
[ 1.810691] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 1.817815] Control: 10c5387d Table: 8020406a DAC: 00000051
[ 1.823552] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[ 1.829549] Stack: (0xee0e1d60 to 0xee0e2000)
[ 1.833904] 1d60: eefedd88 00000040 c07087a0 fffffdfb ee269c10 ee737640 00000000 eefedd88
[ 1.842073] 1d80: 00000000 00000000 00000005 c0707d34 00000000 ee3c8a00 ee7375c0 ee269c10
[ 1.850242] 1da0: eefedd88 c0a0bd2c c1704e48 ee269c10 ee269c00 ee3c8a00 00000000 c0a0c4a8
[ 1.858409] 1dc0: ee269c10 c1704e48 c186603c 00000000 c186603c 00000000 00000000 bc98ab22
[ 1.866577] 1de0: ffffffff ee269c10 00000000 c186603c ee269c00 c186603c 00000000 00000000
[ 1.874744] 1e00: c1656690 c0a0ffe0 00000000 bc98ab22 ee269c10 ee269c10 00000000 c186603c
[ 1.882913] 1e20: 00000000 c186603c 00000000 c09887e0 c18ff9dc ee269c10 c18ff9e0 c0986860
[ 1.891082] 1e40: ee269c10 c186603c c186603c c1704e48 00000000 c15003f0 c15c3854 c0986af0
[ 1.899249] 1e60: c15c3854 c0d128b4 c10e48ec ee269c10 00000000 c186603c c1704e48 00000000
[ 1.907416] 1e80: c15003f0 c15c3854 c1656690 c0986da0 00000000 c186603c ee269c10 c0986e28
[ 1.915583] 1ea0: 00000000 c186603c c0986da8 c0984ba0 c15003f0 ee20c058 ee242334 bc98ab22
[ 1.923752] 1ec0: c18588c8 c186603c ee737200 c18588c8 00000000 c0985b94 c133ef10 ffffe000
[ 1.931919] 1ee0: c186603c c186603c c18aaf80 ffffe000 c158b72c c09878ac c1704e48 c18aaf80
[ 1.940088] 1f00: ffffe000 c0302f80 00000168 c0367d84 c143e5b4 c1371000 00000000 00000006
[ 1.948255] 1f20: 00000006 c125b1b4 00000000 c1704e48 c126f324 c125b228 00000000 efffec88
[ 1.956424] 1f40: 00000000 bc98ab22 00000000 c18b6bc0 c18b6bc0 bc98ab22 c18b6bc0 c18b6bc0
[ 1.964591] 1f60: 00000007 c15c3834 00000169 c1500f28 00000006 00000006 00000000 c15003f0
[ 1.972758] 1f80: 00000000 00000000 c0ef1cdc 00000000 00000000 00000000 00000000 00000000
[ 1.980924] 1fa0: 00000000 c0ef1ce4 00000000 c03010e8 00000000 00000000 00000000 00000000
[ 1.989092] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 1.997260] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 2.005440] [<c09832e4>] (device_link_add) from [<c0707d34>] (devm_of_phy_get+0x6c/0xb0)
[ 2.013528] [<c0707d34>] (devm_of_phy_get) from [<c0a0bd2c>] (ahci_platform_get_phy+0x28/0xd0)
[ 2.022134] [<c0a0bd2c>] (ahci_platform_get_phy) from [<c0a0c4a8>] (ahci_platform_get_resources+0x384/0x468)
[ 2.031952] [<c0a0c4a8>] (ahci_platform_get_resources) from [<c0a0ffe0>] (tegra_ahci_probe+0x14/0x650)
[ 2.041254] [<c0a0ffe0>] (tegra_ahci_probe) from [<c09887e0>] (platform_drv_probe+0x48/0x98)
[ 2.049686] [<c09887e0>] (platform_drv_probe) from [<c0986860>] (really_probe+0x234/0x34c)
[ 2.057944] [<c0986860>] (really_probe) from [<c0986af0>] (driver_probe_device+0x60/0x168)
[ 2.066202] [<c0986af0>] (driver_probe_device) from [<c0986da0>] (device_driver_attach+0x58/0x60)
[ 2.075064] [<c0986da0>] (device_driver_attach) from [<c0986e28>] (__driver_attach+0x80/0xbc)
[ 2.083582] [<c0986e28>] (__driver_attach) from [<c0984ba0>] (bus_for_each_dev+0x74/0xb4)
[ 2.091751] [<c0984ba0>] (bus_for_each_dev) from [<c0985b94>] (bus_add_driver+0x164/0x1e8)
[ 2.100008] [<c0985b94>] (bus_add_driver) from [<c09878ac>] (driver_register+0x7c/0x114)
[ 2.108094] [<c09878ac>] (driver_register) from [<c0302f80>] (do_one_initcall+0x54/0x22c)
[ 2.116271] [<c0302f80>] (do_one_initcall) from [<c1500f28>] (kernel_init_freeable+0x14c/0x1b0)
[ 2.124967] [<c1500f28>] (kernel_init_freeable) from [<c0ef1ce4>] (kernel_init+0x8/0x10c)
[ 2.133139] [<c0ef1ce4>] (kernel_init) from [<c03010e8>] (ret_from_fork+0x14/0x2c)
[ 2.140697] Exception stack(0xee0e1fb0 to 0xee0e1ff8)
[ 2.145743] 1fa0: 00000000 00000000 00000000 00000000
[ 2.153910] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 2.162076] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 2.168686] Code: e59f0470 03844040 eb15cb16 eb004c8a (e5d63084)
[ 2.174824] ---[ end trace fddbf111e88ec722 ]---
I believe that there is a bug in this patch and the following fixed it for me ...
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 8dfb4868c8c3..2eb28cc2d2dc 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -799,6 +799,7 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
devres_add(dev, ptr);
} else {
devres_free(ptr);
+ return phy;
}
link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
Cheers
Jon
--
nvpublic
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2020-01-07 11:51 ` Jon Hunter
@ 2020-01-08 7:29 ` Kishon Vijay Abraham I
2020-01-08 8:37 ` Alexandre Torgue
0 siblings, 1 reply; 16+ messages in thread
From: Kishon Vijay Abraham I @ 2020-01-08 7:29 UTC (permalink / raw)
To: Jon Hunter, Alexandre Torgue, Greg Kroah-Hartman
Cc: Yoshihiro Shimoda, linux-kernel, linux-usb, linux-stm32, linux-tegra
Hi Jon,
On 07/01/20 5:21 PM, Jon Hunter wrote:
>
> On 04/11/2019 14:37, Alexandre Torgue wrote:
>> In order to enforce suspend/resume ordering, this commit creates link
>> between phy consumers and phy devices. This link avoids to suspend phy
>> before phy consumers.
>>
>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>
> With next-20200106 we are seeing a boot regression on Tegra124 Jetson
> TK1 board. Bisect is pointing to this commit and reverting this on top
> of -next fixes the problem.
>
> The bootlog is showing the following crash on boot ...
>
> [ 1.730024] 8<--- cut here ---
> [ 1.733079] Unable to handle kernel paging request at virtual address fffffe7f
> [ 1.740318] pgd = (ptrval)
> [ 1.743021] [fffffe7f] *pgd=affff841, *pte=00000000, *ppte=00000000
> [ 1.749304] Internal error: Oops: 27 [#1] SMP ARM
> [ 1.754001] Modules linked in:
> [ 1.757057] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.5.0-rc4-next-20200106-g9eb1b48ca4ce #1
> [ 1.765654] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> [ 1.771919] PC is at device_link_add+0x68/0x4d4
> [ 1.776444] LR is at device_link_add+0x68/0x4d4
> [ 1.780967] pc : [<c09832e4>] lr : [<c09832e4>] psr: 60000013
> [ 1.787223] sp : ee0e1d60 ip : 60000013 fp : 00000005
> [ 1.792439] r10: 00000000 r9 : 00000000 r8 : eefedd88
> [ 1.797657] r7 : ee269c10 r6 : fffffdfb r5 : 00000001 r4 : 00000001
> [ 1.804173] r3 : ee0d8000 r2 : 00000000 r1 : 00000000 r0 : c1858f88
> [ 1.810691] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> [ 1.817815] Control: 10c5387d Table: 8020406a DAC: 00000051
> [ 1.823552] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
> [ 1.829549] Stack: (0xee0e1d60 to 0xee0e2000)
> [ 1.833904] 1d60: eefedd88 00000040 c07087a0 fffffdfb ee269c10 ee737640 00000000 eefedd88
> [ 1.842073] 1d80: 00000000 00000000 00000005 c0707d34 00000000 ee3c8a00 ee7375c0 ee269c10
> [ 1.850242] 1da0: eefedd88 c0a0bd2c c1704e48 ee269c10 ee269c00 ee3c8a00 00000000 c0a0c4a8
> [ 1.858409] 1dc0: ee269c10 c1704e48 c186603c 00000000 c186603c 00000000 00000000 bc98ab22
> [ 1.866577] 1de0: ffffffff ee269c10 00000000 c186603c ee269c00 c186603c 00000000 00000000
> [ 1.874744] 1e00: c1656690 c0a0ffe0 00000000 bc98ab22 ee269c10 ee269c10 00000000 c186603c
> [ 1.882913] 1e20: 00000000 c186603c 00000000 c09887e0 c18ff9dc ee269c10 c18ff9e0 c0986860
> [ 1.891082] 1e40: ee269c10 c186603c c186603c c1704e48 00000000 c15003f0 c15c3854 c0986af0
> [ 1.899249] 1e60: c15c3854 c0d128b4 c10e48ec ee269c10 00000000 c186603c c1704e48 00000000
> [ 1.907416] 1e80: c15003f0 c15c3854 c1656690 c0986da0 00000000 c186603c ee269c10 c0986e28
> [ 1.915583] 1ea0: 00000000 c186603c c0986da8 c0984ba0 c15003f0 ee20c058 ee242334 bc98ab22
> [ 1.923752] 1ec0: c18588c8 c186603c ee737200 c18588c8 00000000 c0985b94 c133ef10 ffffe000
> [ 1.931919] 1ee0: c186603c c186603c c18aaf80 ffffe000 c158b72c c09878ac c1704e48 c18aaf80
> [ 1.940088] 1f00: ffffe000 c0302f80 00000168 c0367d84 c143e5b4 c1371000 00000000 00000006
> [ 1.948255] 1f20: 00000006 c125b1b4 00000000 c1704e48 c126f324 c125b228 00000000 efffec88
> [ 1.956424] 1f40: 00000000 bc98ab22 00000000 c18b6bc0 c18b6bc0 bc98ab22 c18b6bc0 c18b6bc0
> [ 1.964591] 1f60: 00000007 c15c3834 00000169 c1500f28 00000006 00000006 00000000 c15003f0
> [ 1.972758] 1f80: 00000000 00000000 c0ef1cdc 00000000 00000000 00000000 00000000 00000000
> [ 1.980924] 1fa0: 00000000 c0ef1ce4 00000000 c03010e8 00000000 00000000 00000000 00000000
> [ 1.989092] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 1.997260] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
> [ 2.005440] [<c09832e4>] (device_link_add) from [<c0707d34>] (devm_of_phy_get+0x6c/0xb0)
> [ 2.013528] [<c0707d34>] (devm_of_phy_get) from [<c0a0bd2c>] (ahci_platform_get_phy+0x28/0xd0)
> [ 2.022134] [<c0a0bd2c>] (ahci_platform_get_phy) from [<c0a0c4a8>] (ahci_platform_get_resources+0x384/0x468)
> [ 2.031952] [<c0a0c4a8>] (ahci_platform_get_resources) from [<c0a0ffe0>] (tegra_ahci_probe+0x14/0x650)
> [ 2.041254] [<c0a0ffe0>] (tegra_ahci_probe) from [<c09887e0>] (platform_drv_probe+0x48/0x98)
> [ 2.049686] [<c09887e0>] (platform_drv_probe) from [<c0986860>] (really_probe+0x234/0x34c)
> [ 2.057944] [<c0986860>] (really_probe) from [<c0986af0>] (driver_probe_device+0x60/0x168)
> [ 2.066202] [<c0986af0>] (driver_probe_device) from [<c0986da0>] (device_driver_attach+0x58/0x60)
> [ 2.075064] [<c0986da0>] (device_driver_attach) from [<c0986e28>] (__driver_attach+0x80/0xbc)
> [ 2.083582] [<c0986e28>] (__driver_attach) from [<c0984ba0>] (bus_for_each_dev+0x74/0xb4)
> [ 2.091751] [<c0984ba0>] (bus_for_each_dev) from [<c0985b94>] (bus_add_driver+0x164/0x1e8)
> [ 2.100008] [<c0985b94>] (bus_add_driver) from [<c09878ac>] (driver_register+0x7c/0x114)
> [ 2.108094] [<c09878ac>] (driver_register) from [<c0302f80>] (do_one_initcall+0x54/0x22c)
> [ 2.116271] [<c0302f80>] (do_one_initcall) from [<c1500f28>] (kernel_init_freeable+0x14c/0x1b0)
> [ 2.124967] [<c1500f28>] (kernel_init_freeable) from [<c0ef1ce4>] (kernel_init+0x8/0x10c)
> [ 2.133139] [<c0ef1ce4>] (kernel_init) from [<c03010e8>] (ret_from_fork+0x14/0x2c)
> [ 2.140697] Exception stack(0xee0e1fb0 to 0xee0e1ff8)
> [ 2.145743] 1fa0: 00000000 00000000 00000000 00000000
> [ 2.153910] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 2.162076] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [ 2.168686] Code: e59f0470 03844040 eb15cb16 eb004c8a (e5d63084)
> [ 2.174824] ---[ end trace fddbf111e88ec722 ]---
>
>
> I believe that there is a bug in this patch and the following fixed it for me ...
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 8dfb4868c8c3..2eb28cc2d2dc 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -799,6 +799,7 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
> devres_add(dev, ptr);
> } else {
> devres_free(ptr);
> + return phy;
> }
>
> link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
Thank you for spotting this. I've included the fix now.
Thanks
Kishon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2020-01-08 7:29 ` Kishon Vijay Abraham I
@ 2020-01-08 8:37 ` Alexandre Torgue
0 siblings, 0 replies; 16+ messages in thread
From: Alexandre Torgue @ 2020-01-08 8:37 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Jon Hunter, Greg Kroah-Hartman
Cc: Yoshihiro Shimoda, linux-kernel, linux-usb, linux-stm32, linux-tegra
On 1/8/20 8:29 AM, Kishon Vijay Abraham I wrote:
> Hi Jon,
>
> On 07/01/20 5:21 PM, Jon Hunter wrote:
>>
>> On 04/11/2019 14:37, Alexandre Torgue wrote:
>>> In order to enforce suspend/resume ordering, this commit creates link
>>> between phy consumers and phy devices. This link avoids to suspend phy
>>> before phy consumers.
>>>
>>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>
>> With next-20200106 we are seeing a boot regression on Tegra124 Jetson
>> TK1 board. Bisect is pointing to this commit and reverting this on top
>> of -next fixes the problem.
>>
>> The bootlog is showing the following crash on boot ...
>>
>> [ 1.730024] 8<--- cut here ---
>> [ 1.733079] Unable to handle kernel paging request at virtual address fffffe7f
>> [ 1.740318] pgd = (ptrval)
>> [ 1.743021] [fffffe7f] *pgd=affff841, *pte=00000000, *ppte=00000000
>> [ 1.749304] Internal error: Oops: 27 [#1] SMP ARM
>> [ 1.754001] Modules linked in:
>> [ 1.757057] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.5.0-rc4-next-20200106-g9eb1b48ca4ce #1
>> [ 1.765654] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
>> [ 1.771919] PC is at device_link_add+0x68/0x4d4
>> [ 1.776444] LR is at device_link_add+0x68/0x4d4
>> [ 1.780967] pc : [<c09832e4>] lr : [<c09832e4>] psr: 60000013
>> [ 1.787223] sp : ee0e1d60 ip : 60000013 fp : 00000005
>> [ 1.792439] r10: 00000000 r9 : 00000000 r8 : eefedd88
>> [ 1.797657] r7 : ee269c10 r6 : fffffdfb r5 : 00000001 r4 : 00000001
>> [ 1.804173] r3 : ee0d8000 r2 : 00000000 r1 : 00000000 r0 : c1858f88
>> [ 1.810691] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
>> [ 1.817815] Control: 10c5387d Table: 8020406a DAC: 00000051
>> [ 1.823552] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
>> [ 1.829549] Stack: (0xee0e1d60 to 0xee0e2000)
>> [ 1.833904] 1d60: eefedd88 00000040 c07087a0 fffffdfb ee269c10 ee737640 00000000 eefedd88
>> [ 1.842073] 1d80: 00000000 00000000 00000005 c0707d34 00000000 ee3c8a00 ee7375c0 ee269c10
>> [ 1.850242] 1da0: eefedd88 c0a0bd2c c1704e48 ee269c10 ee269c00 ee3c8a00 00000000 c0a0c4a8
>> [ 1.858409] 1dc0: ee269c10 c1704e48 c186603c 00000000 c186603c 00000000 00000000 bc98ab22
>> [ 1.866577] 1de0: ffffffff ee269c10 00000000 c186603c ee269c00 c186603c 00000000 00000000
>> [ 1.874744] 1e00: c1656690 c0a0ffe0 00000000 bc98ab22 ee269c10 ee269c10 00000000 c186603c
>> [ 1.882913] 1e20: 00000000 c186603c 00000000 c09887e0 c18ff9dc ee269c10 c18ff9e0 c0986860
>> [ 1.891082] 1e40: ee269c10 c186603c c186603c c1704e48 00000000 c15003f0 c15c3854 c0986af0
>> [ 1.899249] 1e60: c15c3854 c0d128b4 c10e48ec ee269c10 00000000 c186603c c1704e48 00000000
>> [ 1.907416] 1e80: c15003f0 c15c3854 c1656690 c0986da0 00000000 c186603c ee269c10 c0986e28
>> [ 1.915583] 1ea0: 00000000 c186603c c0986da8 c0984ba0 c15003f0 ee20c058 ee242334 bc98ab22
>> [ 1.923752] 1ec0: c18588c8 c186603c ee737200 c18588c8 00000000 c0985b94 c133ef10 ffffe000
>> [ 1.931919] 1ee0: c186603c c186603c c18aaf80 ffffe000 c158b72c c09878ac c1704e48 c18aaf80
>> [ 1.940088] 1f00: ffffe000 c0302f80 00000168 c0367d84 c143e5b4 c1371000 00000000 00000006
>> [ 1.948255] 1f20: 00000006 c125b1b4 00000000 c1704e48 c126f324 c125b228 00000000 efffec88
>> [ 1.956424] 1f40: 00000000 bc98ab22 00000000 c18b6bc0 c18b6bc0 bc98ab22 c18b6bc0 c18b6bc0
>> [ 1.964591] 1f60: 00000007 c15c3834 00000169 c1500f28 00000006 00000006 00000000 c15003f0
>> [ 1.972758] 1f80: 00000000 00000000 c0ef1cdc 00000000 00000000 00000000 00000000 00000000
>> [ 1.980924] 1fa0: 00000000 c0ef1ce4 00000000 c03010e8 00000000 00000000 00000000 00000000
>> [ 1.989092] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> [ 1.997260] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
>> [ 2.005440] [<c09832e4>] (device_link_add) from [<c0707d34>] (devm_of_phy_get+0x6c/0xb0)
>> [ 2.013528] [<c0707d34>] (devm_of_phy_get) from [<c0a0bd2c>] (ahci_platform_get_phy+0x28/0xd0)
>> [ 2.022134] [<c0a0bd2c>] (ahci_platform_get_phy) from [<c0a0c4a8>] (ahci_platform_get_resources+0x384/0x468)
>> [ 2.031952] [<c0a0c4a8>] (ahci_platform_get_resources) from [<c0a0ffe0>] (tegra_ahci_probe+0x14/0x650)
>> [ 2.041254] [<c0a0ffe0>] (tegra_ahci_probe) from [<c09887e0>] (platform_drv_probe+0x48/0x98)
>> [ 2.049686] [<c09887e0>] (platform_drv_probe) from [<c0986860>] (really_probe+0x234/0x34c)
>> [ 2.057944] [<c0986860>] (really_probe) from [<c0986af0>] (driver_probe_device+0x60/0x168)
>> [ 2.066202] [<c0986af0>] (driver_probe_device) from [<c0986da0>] (device_driver_attach+0x58/0x60)
>> [ 2.075064] [<c0986da0>] (device_driver_attach) from [<c0986e28>] (__driver_attach+0x80/0xbc)
>> [ 2.083582] [<c0986e28>] (__driver_attach) from [<c0984ba0>] (bus_for_each_dev+0x74/0xb4)
>> [ 2.091751] [<c0984ba0>] (bus_for_each_dev) from [<c0985b94>] (bus_add_driver+0x164/0x1e8)
>> [ 2.100008] [<c0985b94>] (bus_add_driver) from [<c09878ac>] (driver_register+0x7c/0x114)
>> [ 2.108094] [<c09878ac>] (driver_register) from [<c0302f80>] (do_one_initcall+0x54/0x22c)
>> [ 2.116271] [<c0302f80>] (do_one_initcall) from [<c1500f28>] (kernel_init_freeable+0x14c/0x1b0)
>> [ 2.124967] [<c1500f28>] (kernel_init_freeable) from [<c0ef1ce4>] (kernel_init+0x8/0x10c)
>> [ 2.133139] [<c0ef1ce4>] (kernel_init) from [<c03010e8>] (ret_from_fork+0x14/0x2c)
>> [ 2.140697] Exception stack(0xee0e1fb0 to 0xee0e1ff8)
>> [ 2.145743] 1fa0: 00000000 00000000 00000000 00000000
>> [ 2.153910] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> [ 2.162076] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>> [ 2.168686] Code: e59f0470 03844040 eb15cb16 eb004c8a (e5d63084)
>> [ 2.174824] ---[ end trace fddbf111e88ec722 ]---
>>
>>
>> I believe that there is a bug in this patch and the following fixed it for me ...
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index 8dfb4868c8c3..2eb28cc2d2dc 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -799,6 +799,7 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
>> devres_add(dev, ptr);
>> } else {
>> devres_free(ptr);
>> + return phy;
>> }
>>
>> link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>
> Thank you for spotting this. I've included the fix now.
Thanks guys, and sorry for this regression.
alex
>
> Thanks
> Kishon
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] phy: core: Add consumer device link support
2019-11-04 14:37 [PATCH] phy: core: Add consumer device link support Alexandre Torgue
2019-12-04 15:33 ` Alexandre Torgue
2020-01-07 11:51 ` Jon Hunter
@ 2020-02-06 13:39 ` youling257
2020-02-07 5:16 ` Kishon Vijay Abraham I
2 siblings, 1 reply; 16+ messages in thread
From: youling257 @ 2020-02-06 13:39 UTC (permalink / raw)
To: alexandre.torgue
Cc: kishon, yoshihiro.shimoda.uh, gregkh, linux-kernel, linux-usb,
youling257
This patch cause "dwc3 dwc3.3.auto: failed to create device link to dwc3.3.auto.ulpi" problem.
https://bugzilla.kernel.org/show_bug.cgi?id=206435
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] phy: core: Add consumer device link support
2020-02-06 13:39 ` youling257
@ 2020-02-07 5:16 ` Kishon Vijay Abraham I
2020-02-07 6:57 ` youling 257
0 siblings, 1 reply; 16+ messages in thread
From: Kishon Vijay Abraham I @ 2020-02-07 5:16 UTC (permalink / raw)
To: youling257, alexandre.torgue
Cc: yoshihiro.shimoda.uh, gregkh, linux-kernel, linux-usb
Hi,
On 06/02/20 7:09 PM, youling257 wrote:
> This patch cause "dwc3 dwc3.3.auto: failed to create device link to dwc3.3.auto.ulpi" problem.
> https://bugzilla.kernel.org/show_bug.cgi?id=206435
I'm suspecting there is some sort of reverse dependency with dwc3 ULPI.
Can you try the following diff?
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 2eb28cc2d2dc..397311dcb116 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -687,7 +687,7 @@ struct phy *phy_get(struct device *dev, const char
*string)
get_device(&phy->dev);
- link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
+ link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
if (!link) {
dev_err(dev, "failed to create device link to %s\n",
dev_name(phy->dev.parent));
@@ -802,7 +802,7 @@ struct phy *devm_of_phy_get(struct device *dev,
struct device_node *np,
return phy;
}
- link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
+ link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
if (!link) {
dev_err(dev, "failed to create device link to %s\n",
dev_name(phy->dev.parent));
@@ -851,7 +851,7 @@ struct phy *devm_of_phy_get_by_index(struct device
*dev, struct device_node *np,
*ptr = phy;
devres_add(dev, ptr);
- link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
+ link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
if (!link) {
dev_err(dev, "failed to create device link to %s\n",
dev_name(phy->dev.parent));
Thanks
Kishon
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2020-02-07 5:16 ` Kishon Vijay Abraham I
@ 2020-02-07 6:57 ` youling 257
2020-02-10 8:08 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 16+ messages in thread
From: youling 257 @ 2020-02-07 6:57 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: alexandre.torgue, yoshihiro.shimoda.uh, gregkh, linux-kernel, linux-usb
test this diff, dwc3 work for my device, thanks.
2020-02-07 13:16 GMT+08:00, Kishon Vijay Abraham I <kishon@ti.com>:
> Hi,
>
> On 06/02/20 7:09 PM, youling257 wrote:
>> This patch cause "dwc3 dwc3.3.auto: failed to create device link to
>> dwc3.3.auto.ulpi" problem.
>> https://bugzilla.kernel.org/show_bug.cgi?id=206435
>
> I'm suspecting there is some sort of reverse dependency with dwc3 ULPI.
> Can you try the following diff?
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 2eb28cc2d2dc..397311dcb116 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -687,7 +687,7 @@ struct phy *phy_get(struct device *dev, const char
> *string)
>
> get_device(&phy->dev);
>
> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
> if (!link) {
> dev_err(dev, "failed to create device link to %s\n",
> dev_name(phy->dev.parent));
> @@ -802,7 +802,7 @@ struct phy *devm_of_phy_get(struct device *dev,
> struct device_node *np,
> return phy;
> }
>
> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
> if (!link) {
> dev_err(dev, "failed to create device link to %s\n",
> dev_name(phy->dev.parent));
> @@ -851,7 +851,7 @@ struct phy *devm_of_phy_get_by_index(struct device
> *dev, struct device_node *np,
> *ptr = phy;
> devres_add(dev, ptr);
>
> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
> if (!link) {
> dev_err(dev, "failed to create device link to %s\n",
> dev_name(phy->dev.parent));
>
> Thanks
> Kishon
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2020-02-07 6:57 ` youling 257
@ 2020-02-10 8:08 ` Kishon Vijay Abraham I
2020-02-10 11:30 ` Alexandre Torgue
0 siblings, 1 reply; 16+ messages in thread
From: Kishon Vijay Abraham I @ 2020-02-10 8:08 UTC (permalink / raw)
To: youling 257
Cc: alexandre.torgue, yoshihiro.shimoda.uh, gregkh, linux-kernel, linux-usb
Hi Alexandre,
On 07/02/20 12:27 PM, youling 257 wrote:
> test this diff, dwc3 work for my device, thanks.
>
> 2020-02-07 13:16 GMT+08:00, Kishon Vijay Abraham I <kishon@ti.com>:
>> Hi,
>>
>> On 06/02/20 7:09 PM, youling257 wrote:
>>> This patch cause "dwc3 dwc3.3.auto: failed to create device link to
>>> dwc3.3.auto.ulpi" problem.
>>> https://bugzilla.kernel.org/show_bug.cgi?id=206435
>>
>> I'm suspecting there is some sort of reverse dependency with dwc3 ULPI.
>> Can you try the following diff?
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index 2eb28cc2d2dc..397311dcb116 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -687,7 +687,7 @@ struct phy *phy_get(struct device *dev, const char
>> *string)
>>
>> get_device(&phy->dev);
>>
>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
>> if (!link) {
>> dev_err(dev, "failed to create device link to %s\n",
>> dev_name(phy->dev.parent));
>> @@ -802,7 +802,7 @@ struct phy *devm_of_phy_get(struct device *dev,
>> struct device_node *np,
>> return phy;
>> }
>>
>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
>> if (!link) {
>> dev_err(dev, "failed to create device link to %s\n",
>> dev_name(phy->dev.parent));
>> @@ -851,7 +851,7 @@ struct phy *devm_of_phy_get_by_index(struct device
>> *dev, struct device_node *np,
>> *ptr = phy;
>> devres_add(dev, ptr);
>>
>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
>> if (!link) {
>> dev_err(dev, "failed to create device link to %s\n",
>> dev_name(phy->dev.parent));Parent
Can you check if this doesn't affect the suspend/resume ordering?
Thanks
Kishon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2020-02-10 8:08 ` Kishon Vijay Abraham I
@ 2020-02-10 11:30 ` Alexandre Torgue
2020-02-10 12:18 ` Kishon Vijay Abraham I
2020-02-14 18:46 ` Andy Shevchenko
0 siblings, 2 replies; 16+ messages in thread
From: Alexandre Torgue @ 2020-02-10 11:30 UTC (permalink / raw)
To: Kishon Vijay Abraham I, youling 257
Cc: yoshihiro.shimoda.uh, gregkh, linux-kernel, linux-usb, saravanak
Hi Kishon,
On 2/10/20 9:08 AM, Kishon Vijay Abraham I wrote:
> Hi Alexandre,
>
> On 07/02/20 12:27 PM, youling 257 wrote:
>> test this diff, dwc3 work for my device, thanks.
>>
>> 2020-02-07 13:16 GMT+08:00, Kishon Vijay Abraham I <kishon@ti.com>:
>>> Hi,
>>>
>>> On 06/02/20 7:09 PM, youling257 wrote:
>>>> This patch cause "dwc3 dwc3.3.auto: failed to create device link to
>>>> dwc3.3.auto.ulpi" problem.
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=206435
>>>
>>> I'm suspecting there is some sort of reverse dependency with dwc3 ULPI.
>>> Can you try the following diff?
>>>
>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>> index 2eb28cc2d2dc..397311dcb116 100644
>>> --- a/drivers/phy/phy-core.c
>>> +++ b/drivers/phy/phy-core.c
>>> @@ -687,7 +687,7 @@ struct phy *phy_get(struct device *dev, const char
>>> *string)
>>>
>>> get_device(&phy->dev);
>>>
>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
>>> if (!link) {
>>> dev_err(dev, "failed to create device link to %s\n",
>>> dev_name(phy->dev.parent));
>>> @@ -802,7 +802,7 @@ struct phy *devm_of_phy_get(struct device *dev,
>>> struct device_node *np,
>>> return phy;
>>> }
>>>
>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
>>> if (!link) {
>>> dev_err(dev, "failed to create device link to %s\n",
>>> dev_name(phy->dev.parent));
>>> @@ -851,7 +851,7 @@ struct phy *devm_of_phy_get_by_index(struct device
>>> *dev, struct device_node *np,
>>> *ptr = phy;
>>> devres_add(dev, ptr);
>>>
>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
>>> if (!link) {
>>> dev_err(dev, "failed to create device link to %s\n",
>>> dev_name(phy->dev.parent));Parent
>
> Can you check if this doesn't affect the suspend/resume ordering?
With this fix, suspend/resume ordering is broken on my side. What do you
think to keep the STATELESS flag and to only display a warn if
"device_link_add" returns an error ? It's not "smart" but it could
solved our issue.
As a lot of improvements have been recently done on device link topic by
Saravana, we could check with him what is the way to follow.
Regards
Alex
>
> Thanks
> Kishon
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2020-02-10 11:30 ` Alexandre Torgue
@ 2020-02-10 12:18 ` Kishon Vijay Abraham I
2020-02-11 6:17 ` Saravana Kannan
2020-02-14 18:46 ` Andy Shevchenko
1 sibling, 1 reply; 16+ messages in thread
From: Kishon Vijay Abraham I @ 2020-02-10 12:18 UTC (permalink / raw)
To: Alexandre Torgue, youling 257
Cc: yoshihiro.shimoda.uh, gregkh, linux-kernel, linux-usb, saravanak
Hi,
On 10/02/20 5:00 PM, Alexandre Torgue wrote:
> Hi Kishon,
>
> On 2/10/20 9:08 AM, Kishon Vijay Abraham I wrote:
>> Hi Alexandre,
>>
>> On 07/02/20 12:27 PM, youling 257 wrote:
>>> test this diff, dwc3 work for my device, thanks.
>>>
>>> 2020-02-07 13:16 GMT+08:00, Kishon Vijay Abraham I <kishon@ti.com>:
>>>> Hi,
>>>>
>>>> On 06/02/20 7:09 PM, youling257 wrote:
>>>>> This patch cause "dwc3 dwc3.3.auto: failed to create device link to
>>>>> dwc3.3.auto.ulpi" problem.
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=206435
>>>>
>>>> I'm suspecting there is some sort of reverse dependency with dwc3 ULPI.
>>>> Can you try the following diff?
>>>>
>>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>>> index 2eb28cc2d2dc..397311dcb116 100644
>>>> --- a/drivers/phy/phy-core.c
>>>> +++ b/drivers/phy/phy-core.c
>>>> @@ -687,7 +687,7 @@ struct phy *phy_get(struct device *dev, const char
>>>> *string)
>>>>
>>>> get_device(&phy->dev);
>>>>
>>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>>> + link = device_link_add(dev, &phy->dev,
>>>> DL_FLAG_SYNC_STATE_ONLY);
>>>> if (!link) {
>>>> dev_err(dev, "failed to create device link to %s\n",
>>>> dev_name(phy->dev.parent));
>>>> @@ -802,7 +802,7 @@ struct phy *devm_of_phy_get(struct device *dev,
>>>> struct device_node *np,
>>>> return phy;
>>>> }
>>>>
>>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>>> + link = device_link_add(dev, &phy->dev,
>>>> DL_FLAG_SYNC_STATE_ONLY);
>>>> if (!link) {
>>>> dev_err(dev, "failed to create device link to %s\n",
>>>> dev_name(phy->dev.parent));
>>>> @@ -851,7 +851,7 @@ struct phy *devm_of_phy_get_by_index(struct device
>>>> *dev, struct device_node *np,
>>>> *ptr = phy;
>>>> devres_add(dev, ptr);
>>>>
>>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>>> + link = device_link_add(dev, &phy->dev,
>>>> DL_FLAG_SYNC_STATE_ONLY);
>>>> if (!link) {
>>>> dev_err(dev, "failed to create device link to %s\n",
>>>> dev_name(phy->dev.parent));Parent
>>
>> Can you check if this doesn't affect the suspend/resume ordering?
>
> With this fix, suspend/resume ordering is broken on my side. What do you
> think to keep the STATELESS flag and to only display a warn if
> "device_link_add" returns an error ? It's not "smart" but it could
> solved our issue.
yeah, that sounds reasonable for now. Can you find out the dependencies
between dwc3 and ulpi and what exactly breaks. That would help Saravana
to suggest a fix?
>
> As a lot of improvements have been recently done on device link topic by
> Saravana, we could check with him what is the way to follow.
Thanks
Kishon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2020-02-10 12:18 ` Kishon Vijay Abraham I
@ 2020-02-11 6:17 ` Saravana Kannan
0 siblings, 0 replies; 16+ messages in thread
From: Saravana Kannan @ 2020-02-11 6:17 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Alexandre Torgue, youling 257, yoshihiro.shimoda.uh,
Greg Kroah-Hartman, LKML, linux-usb
On Mon, Feb 10, 2020 at 4:14 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Hi,
>
> On 10/02/20 5:00 PM, Alexandre Torgue wrote:
> > Hi Kishon,
> >
> > On 2/10/20 9:08 AM, Kishon Vijay Abraham I wrote:
> >> Hi Alexandre,
> >>
> >> On 07/02/20 12:27 PM, youling 257 wrote:
> >>> test this diff, dwc3 work for my device, thanks.
> >>>
> >>> 2020-02-07 13:16 GMT+08:00, Kishon Vijay Abraham I <kishon@ti.com>:
> >>>> Hi,
> >>>>
> >>>> On 06/02/20 7:09 PM, youling257 wrote:
> >>>>> This patch cause "dwc3 dwc3.3.auto: failed to create device link to
> >>>>> dwc3.3.auto.ulpi" problem.
> >>>>> https://bugzilla.kernel.org/show_bug.cgi?id=206435
> >>>>
> >>>> I'm suspecting there is some sort of reverse dependency with dwc3 ULPI.
> >>>> Can you try the following diff?
> >>>>
> >>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> >>>> index 2eb28cc2d2dc..397311dcb116 100644
> >>>> --- a/drivers/phy/phy-core.c
> >>>> +++ b/drivers/phy/phy-core.c
> >>>> @@ -687,7 +687,7 @@ struct phy *phy_get(struct device *dev, const char
> >>>> *string)
> >>>>
> >>>> get_device(&phy->dev);
> >>>>
> >>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> >>>> + link = device_link_add(dev, &phy->dev,
> >>>> DL_FLAG_SYNC_STATE_ONLY);
> >>>> if (!link) {
> >>>> dev_err(dev, "failed to create device link to %s\n",
> >>>> dev_name(phy->dev.parent));
> >>>> @@ -802,7 +802,7 @@ struct phy *devm_of_phy_get(struct device *dev,
> >>>> struct device_node *np,
> >>>> return phy;
> >>>> }
> >>>>
> >>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> >>>> + link = device_link_add(dev, &phy->dev,
> >>>> DL_FLAG_SYNC_STATE_ONLY);
Definitely don't use SYNC_STATE_ONLY.
To give some context, drivers themselves are only expected to use
STATELESS links. Only the driver core is supposed to use MANAGED (if
you don't use STATELESS, it's MANAGED by default) links. And
SYNC_STATE_ONLY makes sense only for MANAGED links.
Also, as the SYNC_STATE_ONLY documentation says, it only affect
sync_state() behavior and doesn't affect suspend/resume in any way.
> >>>> if (!link) {
> >>>> dev_err(dev, "failed to create device link to %s\n",
> >>>> dev_name(phy->dev.parent));
> >>>> @@ -851,7 +851,7 @@ struct phy *devm_of_phy_get_by_index(struct device
> >>>> *dev, struct device_node *np,
> >>>> *ptr = phy;
> >>>> devres_add(dev, ptr);
> >>>>
> >>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> >>>> + link = device_link_add(dev, &phy->dev,
> >>>> DL_FLAG_SYNC_STATE_ONLY);
> >>>> if (!link) {
> >>>> dev_err(dev, "failed to create device link to %s\n",
> >>>> dev_name(phy->dev.parent));Parent
> >>
> >> Can you check if this doesn't affect the suspend/resume ordering?
> >
> > With this fix, suspend/resume ordering is broken on my side. What do you
> > think to keep the STATELESS flag and to only display a warn if
> > "device_link_add" returns an error ? It's not "smart" but it could
> > solved our issue.
>
> yeah, that sounds reasonable for now. Can you find out the dependencies
> between dwc3 and ulpi and what exactly breaks. That would help Saravana
> to suggest a fix?
Yup, I don't have enough context of the dependencies here to suggest a
good fix. But if device_link_add() is failing with STATELESS and not
failing with SYNC_STATE_ONLY, I'm pretty sure you have a cyclic
dependency between these devices when you create this link. Keep in
mind that it can be a cycle involving more than 2 devices -- A -> B ->
C -> A. And cycles don't make sense when you are trying to order
suspend/resume. Looks like the new link is wrong or an existing link
is wrong. If the dependencies are actually correct in hardware, then
maybe your hardware device needs to be split into multiple devices so
that you don't have cycles and can suspend in a meaningful way?
Hope that helps.
Thanks,
Saravana
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] phy: core: Add consumer device link support
2020-02-10 11:30 ` Alexandre Torgue
2020-02-10 12:18 ` Kishon Vijay Abraham I
@ 2020-02-14 18:46 ` Andy Shevchenko
2020-02-17 8:35 ` Alexandre Torgue
1 sibling, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2020-02-14 18:46 UTC (permalink / raw)
To: Alexandre Torgue
Cc: Kishon Vijay Abraham I, youling 257, Yoshihiro Shimoda,
Greg Kroah-Hartman, Linux Kernel Mailing List, USB, saravanak
On Mon, Feb 10, 2020 at 1:32 PM Alexandre Torgue
<alexandre.torgue@st.com> wrote:
> On 2/10/20 9:08 AM, Kishon Vijay Abraham I wrote:
> > On 07/02/20 12:27 PM, youling 257 wrote:
> >> test this diff, dwc3 work for my device, thanks.
> >>
> >> 2020-02-07 13:16 GMT+08:00, Kishon Vijay Abraham I <kishon@ti.com>:
> >>> On 06/02/20 7:09 PM, youling257 wrote:
> >>>> This patch cause "dwc3 dwc3.3.auto: failed to create device link to
> >>>> dwc3.3.auto.ulpi" problem.
> >>>> https://bugzilla.kernel.org/show_bug.cgi?id=206435
+1 to the report.
Please revert for v5.6 or provide a fix ASAP!
> >>>
> >>> I'm suspecting there is some sort of reverse dependency with dwc3 ULPI.
> >>> Can you try the following diff?
> >>>
> >>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> >>> index 2eb28cc2d2dc..397311dcb116 100644
> >>> --- a/drivers/phy/phy-core.c
> >>> +++ b/drivers/phy/phy-core.c
> >>> @@ -687,7 +687,7 @@ struct phy *phy_get(struct device *dev, const char
> >>> *string)
> >>>
> >>> get_device(&phy->dev);
> >>>
> >>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> >>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
> >>> if (!link) {
> >>> dev_err(dev, "failed to create device link to %s\n",
> >>> dev_name(phy->dev.parent));
> >>> @@ -802,7 +802,7 @@ struct phy *devm_of_phy_get(struct device *dev,
> >>> struct device_node *np,
> >>> return phy;
> >>> }
> >>>
> >>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> >>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
> >>> if (!link) {
> >>> dev_err(dev, "failed to create device link to %s\n",
> >>> dev_name(phy->dev.parent));
> >>> @@ -851,7 +851,7 @@ struct phy *devm_of_phy_get_by_index(struct device
> >>> *dev, struct device_node *np,
> >>> *ptr = phy;
> >>> devres_add(dev, ptr);
> >>>
> >>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> >>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
> >>> if (!link) {
> >>> dev_err(dev, "failed to create device link to %s\n",
> >>> dev_name(phy->dev.parent));Parent
> >
> > Can you check if this doesn't affect the suspend/resume ordering?
>
> With this fix, suspend/resume ordering is broken on my side. What do you
> think to keep the STATELESS flag and to only display a warn if
> "device_link_add" returns an error ? It's not "smart" but it could
> solved our issue.
>
> As a lot of improvements have been recently done on device link topic by
> Saravana, we could check with him what is the way to follow.
>
> Regards
> Alex
>
> >
> > Thanks
> > Kishon
> >
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2020-02-14 18:46 ` Andy Shevchenko
@ 2020-02-17 8:35 ` Alexandre Torgue
2020-02-17 8:40 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 16+ messages in thread
From: Alexandre Torgue @ 2020-02-17 8:35 UTC (permalink / raw)
To: Andy Shevchenko, Kishon Vijay Abraham I
Cc: youling 257, Yoshihiro Shimoda, Greg Kroah-Hartman,
Linux Kernel Mailing List, USB, saravanak
Hi,
On 2/14/20 7:46 PM, Andy Shevchenko wrote:
> On Mon, Feb 10, 2020 at 1:32 PM Alexandre Torgue
> <alexandre.torgue@st.com> wrote:
>> On 2/10/20 9:08 AM, Kishon Vijay Abraham I wrote:
>>> On 07/02/20 12:27 PM, youling 257 wrote:
>>>> test this diff, dwc3 work for my device, thanks.
>>>>
>>>> 2020-02-07 13:16 GMT+08:00, Kishon Vijay Abraham I <kishon@ti.com>:
>>>>> On 06/02/20 7:09 PM, youling257 wrote:
>>>>>> This patch cause "dwc3 dwc3.3.auto: failed to create device link to
>>>>>> dwc3.3.auto.ulpi" problem.
>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=206435
>
> +1 to the report.
> Please revert for v5.6 or provide a fix ASAP!
>
Kishon, do you plan to do the fix? If not, I'll send it quickly.
Thanks
Alex
>>>>>
>>>>> I'm suspecting there is some sort of reverse dependency with dwc3 ULPI.
>>>>> Can you try the following diff?
>>>>>
>>>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>>>> index 2eb28cc2d2dc..397311dcb116 100644
>>>>> --- a/drivers/phy/phy-core.c
>>>>> +++ b/drivers/phy/phy-core.c
>>>>> @@ -687,7 +687,7 @@ struct phy *phy_get(struct device *dev, const char
>>>>> *string)
>>>>>
>>>>> get_device(&phy->dev);
>>>>>
>>>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>>>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
>>>>> if (!link) {
>>>>> dev_err(dev, "failed to create device link to %s\n",
>>>>> dev_name(phy->dev.parent));
>>>>> @@ -802,7 +802,7 @@ struct phy *devm_of_phy_get(struct device *dev,
>>>>> struct device_node *np,
>>>>> return phy;
>>>>> }
>>>>>
>>>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>>>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
>>>>> if (!link) {
>>>>> dev_err(dev, "failed to create device link to %s\n",
>>>>> dev_name(phy->dev.parent));
>>>>> @@ -851,7 +851,7 @@ struct phy *devm_of_phy_get_by_index(struct device
>>>>> *dev, struct device_node *np,
>>>>> *ptr = phy;
>>>>> devres_add(dev, ptr);
>>>>>
>>>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>>>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
>>>>> if (!link) {
>>>>> dev_err(dev, "failed to create device link to %s\n",
>>>>> dev_name(phy->dev.parent));Parent
>>>
>>> Can you check if this doesn't affect the suspend/resume ordering?
>>
>> With this fix, suspend/resume ordering is broken on my side. What do you
>> think to keep the STATELESS flag and to only display a warn if
>> "device_link_add" returns an error ? It's not "smart" but it could
>> solved our issue.
>>
>> As a lot of improvements have been recently done on device link topic by
>> Saravana, we could check with him what is the way to follow.
>>
>> Regards
>> Alex
>>
>>>
>>> Thanks
>>> Kishon
>>>
>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] phy: core: Add consumer device link support
2020-02-17 8:35 ` Alexandre Torgue
@ 2020-02-17 8:40 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 16+ messages in thread
From: Kishon Vijay Abraham I @ 2020-02-17 8:40 UTC (permalink / raw)
To: Alexandre Torgue, Andy Shevchenko
Cc: youling 257, Yoshihiro Shimoda, Greg Kroah-Hartman,
Linux Kernel Mailing List, USB, saravanak
Hi,
On 17/02/20 2:05 pm, Alexandre Torgue wrote:
> Hi,
>
> On 2/14/20 7:46 PM, Andy Shevchenko wrote:
>> On Mon, Feb 10, 2020 at 1:32 PM Alexandre Torgue
>> <alexandre.torgue@st.com> wrote:
>>> On 2/10/20 9:08 AM, Kishon Vijay Abraham I wrote:
>>>> On 07/02/20 12:27 PM, youling 257 wrote:
>>>>> test this diff, dwc3 work for my device, thanks.
>>>>>
>>>>> 2020-02-07 13:16 GMT+08:00, Kishon Vijay Abraham I <kishon@ti.com>:
>>>>>> On 06/02/20 7:09 PM, youling257 wrote:
>>>>>>> This patch cause "dwc3 dwc3.3.auto: failed to create device link to
>>>>>>> dwc3.3.auto.ulpi" problem.
>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=206435
>>
>> +1 to the report.
>> Please revert for v5.6 or provide a fix ASAP!
>>
>
> Kishon, do you plan to do the fix? If not, I'll send it quickly.
Please send a fix for it.
Thanks
Kishon
>
> Thanks
> Alex
>
>>>>>>
>>>>>> I'm suspecting there is some sort of reverse dependency with dwc3
>>>>>> ULPI.
>>>>>> Can you try the following diff?
>>>>>>
>>>>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>>>>> index 2eb28cc2d2dc..397311dcb116 100644
>>>>>> --- a/drivers/phy/phy-core.c
>>>>>> +++ b/drivers/phy/phy-core.c
>>>>>> @@ -687,7 +687,7 @@ struct phy *phy_get(struct device *dev, const
>>>>>> char
>>>>>> *string)
>>>>>>
>>>>>> get_device(&phy->dev);
>>>>>>
>>>>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>>>>> + link = device_link_add(dev, &phy->dev,
>>>>>> DL_FLAG_SYNC_STATE_ONLY);
>>>>>> if (!link) {
>>>>>> dev_err(dev, "failed to create device link to
>>>>>> %s\n",
>>>>>> dev_name(phy->dev.parent));
>>>>>> @@ -802,7 +802,7 @@ struct phy *devm_of_phy_get(struct device *dev,
>>>>>> struct device_node *np,
>>>>>> return phy;
>>>>>> }
>>>>>>
>>>>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>>>>> + link = device_link_add(dev, &phy->dev,
>>>>>> DL_FLAG_SYNC_STATE_ONLY);
>>>>>> if (!link) {
>>>>>> dev_err(dev, "failed to create device link to
>>>>>> %s\n",
>>>>>> dev_name(phy->dev.parent));
>>>>>> @@ -851,7 +851,7 @@ struct phy *devm_of_phy_get_by_index(struct
>>>>>> device
>>>>>> *dev, struct device_node *np,
>>>>>> *ptr = phy;
>>>>>> devres_add(dev, ptr);
>>>>>>
>>>>>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>>>>>> + link = device_link_add(dev, &phy->dev,
>>>>>> DL_FLAG_SYNC_STATE_ONLY);
>>>>>> if (!link) {
>>>>>> dev_err(dev, "failed to create device link to
>>>>>> %s\n",
>>>>>> dev_name(phy->dev.parent));Parent
>>>>
>>>> Can you check if this doesn't affect the suspend/resume ordering?
>>>
>>> With this fix, suspend/resume ordering is broken on my side. What do you
>>> think to keep the STATELESS flag and to only display a warn if
>>> "device_link_add" returns an error ? It's not "smart" but it could
>>> solved our issue.
>>>
>>> As a lot of improvements have been recently done on device link topic by
>>> Saravana, we could check with him what is the way to follow.
>>>
>>> Regards
>>> Alex
>>>
>>>>
>>>> Thanks
>>>> Kishon
>>>>
>>
>>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-02-17 8:37 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04 14:37 [PATCH] phy: core: Add consumer device link support Alexandre Torgue
2019-12-04 15:33 ` Alexandre Torgue
2019-12-04 17:19 ` Greg Kroah-Hartman
2020-01-07 11:51 ` Jon Hunter
2020-01-08 7:29 ` Kishon Vijay Abraham I
2020-01-08 8:37 ` Alexandre Torgue
2020-02-06 13:39 ` youling257
2020-02-07 5:16 ` Kishon Vijay Abraham I
2020-02-07 6:57 ` youling 257
2020-02-10 8:08 ` Kishon Vijay Abraham I
2020-02-10 11:30 ` Alexandre Torgue
2020-02-10 12:18 ` Kishon Vijay Abraham I
2020-02-11 6:17 ` Saravana Kannan
2020-02-14 18:46 ` Andy Shevchenko
2020-02-17 8:35 ` Alexandre Torgue
2020-02-17 8:40 ` Kishon Vijay Abraham I
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).