[PATCH treewide v2 0/9] phy: Add devm_of_phy_optional_get() helper

[PATCH treewide v2 0/9] phy: Add devm_of_phy_optional_get() helper

[Geert Uytterhoeven]

	Hi Vinod et al,

While there exist several optional_get() PHY helper functions, there is
no optional variant of devm_of_phy_get(), leading to several drivers
implementing this theirselves, sometimes in buggy ways.

Hence this series, after two cleanup patches, introduces a
devm_of_phy_optional_get() helper(), and converts existing users of
devm_of_phy_get() where appropriate.

Changes compared to v1[1]:
  - Incorporate "[PATCH v2 1/9] phy: Remove unused phy_optional_get()",
    as it touches the same documentation,
  - New patch "[PATCH v2 2/9] doc: phy: Document devm_of_phy_get()",
  - Print an error message in case of failure, as requested by RobH,
  - Update Documentation,
  - Clarify removed checks for -ENODEV and -ENOSYS,
  - Remove error printing in case of real failures from callers,
  - Rebase am65-cpsw change on top of commit 854617f52ab42418 ("net:
    ethernet: ti: am65-cpsw: Handle -EPROBE_DEFER for Serdes PHY") in
    net-next (next-20230123 and later),
  - Add Reviewed-by, Acked-by.

Most of this series been compile-tested only, but the new helper itself
has been tested with a new user[2].

Thanks for your comments!

[1] "[PATCH treewide 0/7] phy: Add devm_of_phy_optional_get() helper"
    https://lore.kernel.org/r/cover.1674036164.git.geert+renesas@glider.be
[2] "[PATCH 12/12] can: rcar_canfd: Add transceiver support"
    https://lore.kernel.org/r/e825b50a843ffe40e33f34e4d858c07c1b2ff259.1674499048.git.geert+renesas@glider.be

Geert Uytterhoeven (9):
  phy: Remove unused phy_optional_get()
  doc: phy: Document devm_of_phy_get()
  phy: Add devm_of_phy_optional_get() helper
  net: fman: memac: Convert to devm_of_phy_optional_get()
  net: lan966x: Convert to devm_of_phy_optional_get()
  net: ethernet: ti: am65-cpsw: Convert to devm_of_phy_optional_get()
  PCI: tegra: Convert to devm_of_phy_optional_get()
  usb: host: ehci-exynos: Convert to devm_of_phy_optional_get()
  usb: host: ohci-exynos: Convert to devm_of_phy_optional_get()

 Documentation/driver-api/phy/phy.rst          | 24 +++++----
 .../net/ethernet/freescale/fman/fman_memac.c  |  9 ++--
 .../ethernet/microchip/lan966x/lan966x_main.c |  5 +-
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      |  8 ++-
 drivers/pci/controller/pci-tegra.c            |  5 +-
 drivers/phy/phy-core.c                        | 51 +++++++++++--------
 drivers/usb/host/ehci-exynos.c                | 23 +++------
 drivers/usb/host/ohci-exynos.c                | 23 +++------
 include/linux/phy/phy.h                       | 16 +++---
 9 files changed, 75 insertions(+), 89 deletions(-)

-- 
2.34.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH v2 1/9] phy: Remove unused phy_optional_get()

[Geert Uytterhoeven]

There were never any upstream users of this function since its
introduction almost 10 years ago.

Besides, the dummy for phy_optional_get() should have returned NULL
instead of an error code.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This is v2 of "phy: Return NULL if the PHY is optional (part two)".
---
 Documentation/driver-api/phy/phy.rst | 11 +++++------
 drivers/phy/phy-core.c               | 21 ---------------------
 include/linux/phy/phy.h              |  7 -------
 3 files changed, 5 insertions(+), 34 deletions(-)

diff --git a/Documentation/driver-api/phy/phy.rst b/Documentation/driver-api/phy/phy.rst
index 8e8b3e8f95238d18..26467dd4f291505e 100644
--- a/Documentation/driver-api/phy/phy.rst
+++ b/Documentation/driver-api/phy/phy.rst
@@ -103,7 +103,6 @@ it. This framework provides the following APIs to get a reference to the PHY.
 ::
 
 	struct phy *phy_get(struct device *dev, const char *string);
-	struct phy *phy_optional_get(struct device *dev, const char *string);
 	struct phy *devm_phy_get(struct device *dev, const char *string);
 	struct phy *devm_phy_optional_get(struct device *dev,
 					  const char *string);
@@ -111,15 +110,15 @@ it. This framework provides the following APIs to get a reference to the PHY.
 					     struct device_node *np,
 					     int index);
 
-phy_get, phy_optional_get, devm_phy_get and devm_phy_optional_get can
-be used to get the PHY. In the case of dt boot, the string arguments
+phy_get, devm_phy_get and devm_phy_optional_get can be used to get the PHY.
+In the case of dt boot, the string arguments
 should contain the phy name as given in the dt data and in the case of
 non-dt boot, it should contain the label of the PHY.  The two
 devm_phy_get associates the device with the PHY using devres on
 successful PHY get. On driver detach, release function is invoked on
-the devres data and devres data is freed. phy_optional_get and
-devm_phy_optional_get should be used when the phy is optional. These
-two functions will never return -ENODEV, but instead returns NULL when
+the devres data and devres data is freed.
+devm_phy_optional_get should be used when the phy is optional. This
+function will never return -ENODEV, but instead returns NULL when
 the phy cannot be found.Some generic drivers, such as ehci, may use multiple
 phys and for such drivers referencing phy(s) by name(s) does not make sense. In
 this case, devm_of_phy_get_by_index can be used to get a phy reference based on
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index d93ddf1262c5178b..672f5c86588609f3 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -766,27 +766,6 @@ struct phy *phy_get(struct device *dev, const char *string)
 }
 EXPORT_SYMBOL_GPL(phy_get);
 
-/**
- * phy_optional_get() - lookup and obtain a reference to an optional phy.
- * @dev: device that requests this phy
- * @string: the phy name as given in the dt data or the name of the controller
- * port for non-dt case
- *
- * Returns the phy driver, after getting a refcount to it; or
- * NULL if there is no such phy.  The caller is responsible for
- * calling phy_put() to release that count.
- */
-struct phy *phy_optional_get(struct device *dev, const char *string)
-{
-	struct phy *phy = phy_get(dev, string);
-
-	if (PTR_ERR(phy) == -ENODEV)
-		phy = NULL;
-
-	return phy;
-}
-EXPORT_SYMBOL_GPL(phy_optional_get);
-
 /**
  * devm_phy_get() - lookup and obtain a reference to a phy.
  * @dev: device that requests this phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index b1413757fcc3b89b..1b4f9be21e01f4c7 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -250,7 +250,6 @@ static inline void phy_set_bus_width(struct phy *phy, int bus_width)
 	phy->attrs.bus_width = bus_width;
 }
 struct phy *phy_get(struct device *dev, const char *string);
-struct phy *phy_optional_get(struct device *dev, const char *string);
 struct phy *devm_phy_get(struct device *dev, const char *string);
 struct phy *devm_phy_optional_get(struct device *dev, const char *string);
 struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
@@ -426,12 +425,6 @@ static inline struct phy *phy_get(struct device *dev, const char *string)
 	return ERR_PTR(-ENOSYS);
 }
 
-static inline struct phy *phy_optional_get(struct device *dev,
-					   const char *string)
-{
-	return ERR_PTR(-ENOSYS);
-}
-
 static inline struct phy *devm_phy_get(struct device *dev, const char *string)
 {
 	return ERR_PTR(-ENOSYS);
-- 
2.34.1

[PATCH v2 2/9] doc: phy: Document devm_of_phy_get()

[Geert Uytterhoeven]

Add the missing documentation for devm_of_phy_get(), which was forgotten
when the function was introduced.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 Documentation/driver-api/phy/phy.rst | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/driver-api/phy/phy.rst b/Documentation/driver-api/phy/phy.rst
index 26467dd4f291505e..6cadc58f4ce07ce4 100644
--- a/Documentation/driver-api/phy/phy.rst
+++ b/Documentation/driver-api/phy/phy.rst
@@ -106,6 +106,8 @@ it. This framework provides the following APIs to get a reference to the PHY.
 	struct phy *devm_phy_get(struct device *dev, const char *string);
 	struct phy *devm_phy_optional_get(struct device *dev,
 					  const char *string);
+	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);
@@ -119,10 +121,10 @@ successful PHY get. On driver detach, release function is invoked on
 the devres data and devres data is freed.
 devm_phy_optional_get should be used when the phy is optional. This
 function will never return -ENODEV, but instead returns NULL when
-the phy cannot be found.Some generic drivers, such as ehci, may use multiple
-phys and for such drivers referencing phy(s) by name(s) does not make sense. In
-this case, devm_of_phy_get_by_index can be used to get a phy reference based on
-the index.
+the phy cannot be found.
+Some generic drivers, such as ehci, may use multiple phys. In this case,
+devm_of_phy_get or devm_of_phy_get_by_index can be used to get a phy
+reference based on name or index.
 
 It should be noted that NULL is a valid phy reference. All phy
 consumer calls on the NULL phy become NOPs. That is the release calls,
-- 
2.34.1

[PATCH v2 3/9] phy: Add devm_of_phy_optional_get() helper

[Geert Uytterhoeven]

Add an optional variant of devm_of_phy_get() that also takes care of
printing real errors, so drivers no longer have to open-code this
operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Print an error message in case of failure, as requested by RobH,
  - Update Documentation.
---
 Documentation/driver-api/phy/phy.rst |  7 +++++--
 drivers/phy/phy-core.c               | 30 ++++++++++++++++++++++++++++
 include/linux/phy/phy.h              |  9 +++++++++
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/phy/phy.rst b/Documentation/driver-api/phy/phy.rst
index 6cadc58f4ce07ce4..81785c084f3ec2dd 100644
--- a/Documentation/driver-api/phy/phy.rst
+++ b/Documentation/driver-api/phy/phy.rst
@@ -108,6 +108,9 @@ it. This framework provides the following APIs to get a reference to the PHY.
 					  const char *string);
 	struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
 				    const char *con_id);
+	struct phy *devm_of_phy_optional_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);
@@ -119,8 +122,8 @@ non-dt boot, it should contain the label of the PHY.  The two
 devm_phy_get associates the device with the PHY using devres on
 successful PHY get. On driver detach, release function is invoked on
 the devres data and devres data is freed.
-devm_phy_optional_get should be used when the phy is optional. This
-function will never return -ENODEV, but instead returns NULL when
+The _optional_get variants should be used when the phy is optional. These
+functions will never return -ENODEV, but instead return NULL when
 the phy cannot be found.
 Some generic drivers, such as ehci, may use multiple phys. In this case,
 devm_of_phy_get or devm_of_phy_get_by_index can be used to get a phy
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 672f5c86588609f3..9951efc03eaaf842 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -858,6 +858,36 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(devm_of_phy_get);
 
+/**
+ * devm_of_phy_optional_get() - lookup and obtain a reference to an optional
+ * phy.
+ * @dev: device that requests this phy
+ * @np: node containing the phy
+ * @con_id: name of the phy from device's point of view
+ *
+ * Gets the phy using of_phy_get(), and associates a device with it using
+ * devres. On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.  This differs to devm_of_phy_get() in
+ * that if the phy does not exist, it is not considered an error and
+ * -ENODEV will not be returned. Instead the NULL phy is returned,
+ * which can be passed to all other phy consumer calls.
+ */
+struct phy *devm_of_phy_optional_get(struct device *dev, struct device_node *np,
+				     const char *con_id)
+{
+	struct phy *phy = devm_of_phy_get(dev, np, con_id);
+
+	if (PTR_ERR(phy) == -ENODEV)
+		phy = NULL;
+
+	if (IS_ERR(phy))
+		dev_err_probe(dev, PTR_ERR(phy), "failed to get PHY %pOF:%s",
+			      np, con_id);
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_optional_get);
+
 /**
  * devm_of_phy_get_by_index() - lookup and obtain a reference to a phy by index.
  * @dev: device that requests this phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 1b4f9be21e01f4c7..3a570bc59fc7f4a1 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -254,6 +254,8 @@ struct phy *devm_phy_get(struct device *dev, const char *string);
 struct phy *devm_phy_optional_get(struct device *dev, const char *string);
 struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
 			    const char *con_id);
+struct phy *devm_of_phy_optional_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 of_phy_put(struct phy *phy);
@@ -443,6 +445,13 @@ static inline struct phy *devm_of_phy_get(struct device *dev,
 	return ERR_PTR(-ENOSYS);
 }
 
+static inline struct phy *devm_of_phy_optional_get(struct device *dev,
+						   struct device_node *np,
+						   const char *con_id)
+{
+	return NULL;
+}
+
 static inline struct phy *devm_of_phy_get_by_index(struct device *dev,
 						   struct device_node *np,
 						   int index)
-- 
2.34.1

[PATCH v2 4/9] net: fman: memac: Convert to devm_of_phy_optional_get()

[Geert Uytterhoeven]

Use the new devm_of_phy_optional_get() helper instead of open-coding the
same operation.

As devm_of_phy_optional_get() returns NULL if either the PHY cannot be
found, or if support for the PHY framework is not enabled, it is no
longer needed to check for -ENODEV or -ENOSYS.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Sean Anderson <sean.anderson@seco.com>
---
v2:
  - Add Reviewed-by,
  - Clarify removed checks for -ENODEV and -ENOSYS,
  - Remove error printing in case of real failures.
---
 drivers/net/ethernet/freescale/fman/fman_memac.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
index 9349f841bd0645a0..ddd9d13f1166e120 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -1152,13 +1152,12 @@ int memac_initialization(struct mac_device *mac_dev,
 	else
 		memac->sgmii_pcs = pcs;
 
-	memac->serdes = devm_of_phy_get(mac_dev->dev, mac_node, "serdes");
-	err = PTR_ERR(memac->serdes);
-	if (err == -ENODEV || err == -ENOSYS) {
+	memac->serdes = devm_of_phy_optional_get(mac_dev->dev, mac_node,
+						 "serdes");
+	if (!memac->serdes) {
 		dev_dbg(mac_dev->dev, "could not get (optional) serdes\n");
-		memac->serdes = NULL;
 	} else if (IS_ERR(memac->serdes)) {
-		dev_err_probe(mac_dev->dev, err, "could not get serdes\n");
+		err = PTR_ERR(memac->serdes);
 		goto _return_fm_mac_free;
 	}
 
-- 
2.34.1

[PATCH v2 5/9] net: lan966x: Convert to devm_of_phy_optional_get()

[Geert Uytterhoeven]

Use the new devm_of_phy_optional_get() helper instead of open-coding the
same operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
v2:
  - Add Reviewed-by.
---
 drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
index 580c91d24a5284e7..f2670d6d84d7893a 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
@@ -1147,9 +1147,8 @@ static int lan966x_probe(struct platform_device *pdev)
 		lan966x->ports[p]->config.portmode = phy_mode;
 		lan966x->ports[p]->fwnode = fwnode_handle_get(portnp);
 
-		serdes = devm_of_phy_get(lan966x->dev, to_of_node(portnp), NULL);
-		if (PTR_ERR(serdes) == -ENODEV)
-			serdes = NULL;
+		serdes = devm_of_phy_optional_get(lan966x->dev,
+						  to_of_node(portnp), NULL);
 		if (IS_ERR(serdes)) {
 			err = PTR_ERR(serdes);
 			goto cleanup_ports;
-- 
2.34.1

[PATCH v2 6/9] net: ethernet: ti: am65-cpsw: Convert to devm_of_phy_optional_get()

[Geert Uytterhoeven]

Use the new devm_of_phy_optional_get() helper instead of open-coding the
same operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Rebase on top of commit 854617f52ab42418 ("net: ethernet: ti:
    am65-cpsw: Handle -EPROBE_DEFER for Serdes PHY") in net-next
    (next-20230123 and later).
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index c696da89962f1ae3..794f228c8d632f7a 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1460,11 +1460,9 @@ static int am65_cpsw_init_serdes_phy(struct device *dev, struct device_node *por
 	struct phy *phy;
 	int ret;
 
-	phy = devm_of_phy_get(dev, port_np, name);
-	if (PTR_ERR(phy) == -ENODEV)
-		return 0;
-	if (IS_ERR(phy))
-		return PTR_ERR(phy);
+	phy = devm_of_phy_optional_get(dev, port_np, name);
+	if (IS_ERR_OR_NULL(phy))
+		return PTR_ERR_OR_ZERO(phy);
 
 	/* Serdes PHY exists. Store it. */
 	port->slave.serdes_phy = phy;
-- 
2.34.1

[PATCH v2 7/9] PCI: tegra: Convert to devm_of_phy_optional_get()

[Geert Uytterhoeven]

Use the new devm_of_phy_optional_get() helper instead of open-coding the
same operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
v2:
  - Add Acked-by.
---
 drivers/pci/controller/pci-tegra.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 929f9363e94bec71..5b8907c663e516ad 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -1330,12 +1330,9 @@ static struct phy *devm_of_phy_optional_get_index(struct device *dev,
 	if (!name)
 		return ERR_PTR(-ENOMEM);
 
-	phy = devm_of_phy_get(dev, np, name);
+	phy = devm_of_phy_optional_get(dev, np, name);
 	kfree(name);
 
-	if (PTR_ERR(phy) == -ENODEV)
-		phy = NULL;
-
 	return phy;
 }
 
-- 
2.34.1

[PATCH v2 8/9] usb: host: ehci-exynos: Convert to devm_of_phy_optional_get()

[Geert Uytterhoeven]

Use the new devm_of_phy_optional_get() helper instead of open-coding the
same operation.

As devm_of_phy_optional_get() returns NULL if either the PHY cannot be
found, or if support for the PHY framework is not enabled, it is no
longer needed to check for -ENODEV or -ENOSYS.

This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
handle NULL parameters fine.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2:
  - Add Reviewed-by,
  - Clarify removed checks for -ENODEV and -ENOSYS,
  - Remove error printing in case of real failures.
---
 drivers/usb/host/ehci-exynos.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index a333231616f437b8..47c9f06c3d843db3 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -80,19 +80,11 @@ static int exynos_ehci_get_phy(struct device *dev,
 			return -EINVAL;
 		}
 
-		phy = devm_of_phy_get(dev, child, NULL);
+		phy = devm_of_phy_optional_get(dev, child, NULL);
 		exynos_ehci->phy[phy_number] = phy;
 		if (IS_ERR(phy)) {
-			ret = PTR_ERR(phy);
-			if (ret == -EPROBE_DEFER) {
-				of_node_put(child);
-				return ret;
-			} else if (ret != -ENOSYS && ret != -ENODEV) {
-				dev_err(dev,
-					"Error retrieving usb2 phy: %d\n", ret);
-				of_node_put(child);
-				return ret;
-			}
+			of_node_put(child);
+			return PTR_ERR(phy);
 		}
 	}
 
@@ -108,12 +100,10 @@ static int exynos_ehci_phy_enable(struct device *dev)
 	int ret = 0;
 
 	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ehci->phy[i]))
-			ret = phy_power_on(exynos_ehci->phy[i]);
+		ret = phy_power_on(exynos_ehci->phy[i]);
 	if (ret)
 		for (i--; i >= 0; i--)
-			if (!IS_ERR(exynos_ehci->phy[i]))
-				phy_power_off(exynos_ehci->phy[i]);
+			phy_power_off(exynos_ehci->phy[i]);
 
 	return ret;
 }
@@ -125,8 +115,7 @@ static void exynos_ehci_phy_disable(struct device *dev)
 	int i;
 
 	for (i = 0; i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ehci->phy[i]))
-			phy_power_off(exynos_ehci->phy[i]);
+		phy_power_off(exynos_ehci->phy[i]);
 }
 
 static void exynos_setup_vbus_gpio(struct device *dev)
-- 
2.34.1

[PATCH v2 9/9] usb: host: ohci-exynos: Convert to devm_of_phy_optional_get()

[Geert Uytterhoeven]

Use the new devm_of_phy_optional_get() helper instead of open-coding the
same operation.

As devm_of_phy_optional_get() returns NULL if either the PHY cannot be
found, or if support for the PHY framework is not enabled, it is no
longer needed to check for -ENODEV or -ENOSYS.

This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
handle NULL parameters fine.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
v2:
  - Add Acked-by,
  - Clarify removed checks for -ENODEV and -ENOSYS,
  - Remove error printing in case of real failures.
---
 drivers/usb/host/ohci-exynos.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 8d7977fd5d3bd502..8af17c1ee5cc8f1e 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -69,19 +69,11 @@ static int exynos_ohci_get_phy(struct device *dev,
 			return -EINVAL;
 		}
 
-		phy = devm_of_phy_get(dev, child, NULL);
+		phy = devm_of_phy_optional_get(dev, child, NULL);
 		exynos_ohci->phy[phy_number] = phy;
 		if (IS_ERR(phy)) {
-			ret = PTR_ERR(phy);
-			if (ret == -EPROBE_DEFER) {
-				of_node_put(child);
-				return ret;
-			} else if (ret != -ENOSYS && ret != -ENODEV) {
-				dev_err(dev,
-					"Error retrieving usb2 phy: %d\n", ret);
-				of_node_put(child);
-				return ret;
-			}
+			of_node_put(child);
+			return PTR_ERR(phy);
 		}
 	}
 
@@ -97,12 +89,10 @@ static int exynos_ohci_phy_enable(struct device *dev)
 	int ret = 0;
 
 	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ohci->phy[i]))
-			ret = phy_power_on(exynos_ohci->phy[i]);
+		ret = phy_power_on(exynos_ohci->phy[i]);
 	if (ret)
 		for (i--; i >= 0; i--)
-			if (!IS_ERR(exynos_ohci->phy[i]))
-				phy_power_off(exynos_ohci->phy[i]);
+			phy_power_off(exynos_ohci->phy[i]);
 
 	return ret;
 }
@@ -114,8 +104,7 @@ static void exynos_ohci_phy_disable(struct device *dev)
 	int i;
 
 	for (i = 0; i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ohci->phy[i]))
-			phy_power_off(exynos_ohci->phy[i]);
+		phy_power_off(exynos_ohci->phy[i]);
 }
 
 static int exynos_ohci_probe(struct platform_device *pdev)
-- 
2.34.1

Re: [PATCH v2 6/9] net: ethernet: ti: am65-cpsw: Convert to devm_of_phy_optional_get()

[Siddharth Vadapalli]

On 25/01/23 00:07, Geert Uytterhoeven wrote:
> Use the new devm_of_phy_optional_get() helper instead of open-coding the
> same operation.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - Rebase on top of commit 854617f52ab42418 ("net: ethernet: ti:
>     am65-cpsw: Handle -EPROBE_DEFER for Serdes PHY") in net-next
>     (next-20230123 and later).
> ---
>  drivers/net/ethernet/ti/am65-cpsw-nuss.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index c696da89962f1ae3..794f228c8d632f7a 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -1460,11 +1460,9 @@ static int am65_cpsw_init_serdes_phy(struct device *dev, struct device_node *por
>  	struct phy *phy;
>  	int ret;
>  
> -	phy = devm_of_phy_get(dev, port_np, name);
> -	if (PTR_ERR(phy) == -ENODEV)
> -		return 0;
> -	if (IS_ERR(phy))
> -		return PTR_ERR(phy);
> +	phy = devm_of_phy_optional_get(dev, port_np, name);
> +	if (IS_ERR_OR_NULL(phy))
> +		return PTR_ERR_OR_ZERO(phy);

Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>

Regards,
Siddharth.

Re: [PATCH v2 9/9] usb: host: ohci-exynos: Convert to devm_of_phy_optional_get()

[Greg Kroah-Hartman]

On Tue, Jan 24, 2023 at 07:37:28PM +0100, Geert Uytterhoeven wrote:
> Use the new devm_of_phy_optional_get() helper instead of open-coding the
> same operation.
> 
> As devm_of_phy_optional_get() returns NULL if either the PHY cannot be
> found, or if support for the PHY framework is not enabled, it is no
> longer needed to check for -ENODEV or -ENOSYS.
> 
> This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
> handle NULL parameters fine.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v2 8/9] usb: host: ehci-exynos: Convert to devm_of_phy_optional_get()

[Greg Kroah-Hartman]

On Tue, Jan 24, 2023 at 07:37:27PM +0100, Geert Uytterhoeven wrote:
> Use the new devm_of_phy_optional_get() helper instead of open-coding the
> same operation.
> 
> As devm_of_phy_optional_get() returns NULL if either the PHY cannot be
> found, or if support for the PHY framework is not enabled, it is no
> longer needed to check for -ENODEV or -ENOSYS.
> 
> This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
> handle NULL parameters fine.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH treewide v2 0/9] phy: Add devm_of_phy_optional_get() helper

[Geert Uytterhoeven]

Hi Vinod,

On Tue, Jan 24, 2023 at 7:37 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> While there exist several optional_get() PHY helper functions, there is
> no optional variant of devm_of_phy_get(), leading to several drivers
> implementing this theirselves, sometimes in buggy ways.
>
> Hence this series, after two cleanup patches, introduces a
> devm_of_phy_optional_get() helper(), and converts existing users of
> devm_of_phy_get() where appropriate.
>
> Changes compared to v1[1]:
>   - Incorporate "[PATCH v2 1/9] phy: Remove unused phy_optional_get()",
>     as it touches the same documentation,
>   - New patch "[PATCH v2 2/9] doc: phy: Document devm_of_phy_get()",
>   - Print an error message in case of failure, as requested by RobH,
>   - Update Documentation,
>   - Clarify removed checks for -ENODEV and -ENOSYS,
>   - Remove error printing in case of real failures from callers,
>   - Rebase am65-cpsw change on top of commit 854617f52ab42418 ("net:
>     ethernet: ti: am65-cpsw: Handle -EPROBE_DEFER for Serdes PHY") in
>     net-next (next-20230123 and later),
>   - Add Reviewed-by, Acked-by.
>
> Most of this series been compile-tested only, but the new helper itself
> has been tested with a new user[2].

Are you happy with this?  This series (at least patch 3) is a dependency
for [2], and in [3] you said you could apply it and create an immutable
branch.

Thanks!

> [1] "[PATCH treewide 0/7] phy: Add devm_of_phy_optional_get() helper"
>     https://lore.kernel.org/r/cover.1674036164.git.geert+renesas@glider.be
> [2] "[PATCH 12/12] can: rcar_canfd: Add transceiver support"
>     https://lore.kernel.org/r/e825b50a843ffe40e33f34e4d858c07c1b2ff259.1674499048.git.geert+renesas@glider.be

[3] https://lore.kernel.org/all/Y8kmG+jB%2Fs7stebA@matsya

>
> Geert Uytterhoeven (9):
>   phy: Remove unused phy_optional_get()
>   doc: phy: Document devm_of_phy_get()
>   phy: Add devm_of_phy_optional_get() helper
>   net: fman: memac: Convert to devm_of_phy_optional_get()
>   net: lan966x: Convert to devm_of_phy_optional_get()
>   net: ethernet: ti: am65-cpsw: Convert to devm_of_phy_optional_get()
>   PCI: tegra: Convert to devm_of_phy_optional_get()
>   usb: host: ehci-exynos: Convert to devm_of_phy_optional_get()
>   usb: host: ohci-exynos: Convert to devm_of_phy_optional_get()

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 6/9] net: ethernet: ti: am65-cpsw: Convert to devm_of_phy_optional_get()

[Vinod Koul]

On 24-01-23, 19:37, Geert Uytterhoeven wrote:
> Use the new devm_of_phy_optional_get() helper instead of open-coding the
> same operation.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - Rebase on top of commit 854617f52ab42418 ("net: ethernet: ti:
>     am65-cpsw: Handle -EPROBE_DEFER for Serdes PHY") in net-next
>     (next-20230123 and later).

I was trying to apply this on rc1, so ofcourse this fails for me? How do
we resolve this?

I can skip this patch, provide a tag for this to be pulled into -net
tree

> ---
>  drivers/net/ethernet/ti/am65-cpsw-nuss.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index c696da89962f1ae3..794f228c8d632f7a 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -1460,11 +1460,9 @@ static int am65_cpsw_init_serdes_phy(struct device *dev, struct device_node *por
>  	struct phy *phy;
>  	int ret;
>  
> -	phy = devm_of_phy_get(dev, port_np, name);
> -	if (PTR_ERR(phy) == -ENODEV)
> -		return 0;
> -	if (IS_ERR(phy))
> -		return PTR_ERR(phy);
> +	phy = devm_of_phy_optional_get(dev, port_np, name);
> +	if (IS_ERR_OR_NULL(phy))
> +		return PTR_ERR_OR_ZERO(phy);
>  
>  	/* Serdes PHY exists. Store it. */
>  	port->slave.serdes_phy = phy;
> -- 
> 2.34.1

-- 
~Vinod

Re: [PATCH v2 6/9] net: ethernet: ti: am65-cpsw: Convert to devm_of_phy_optional_get()

[Geert Uytterhoeven]

Hi Vinod,

On Fri, Feb 3, 2023 at 6:27 AM Vinod Koul <vkoul@kernel.org> wrote:
> On 24-01-23, 19:37, Geert Uytterhoeven wrote:
> > Use the new devm_of_phy_optional_get() helper instead of open-coding the
> > same operation.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > v2:
> >   - Rebase on top of commit 854617f52ab42418 ("net: ethernet: ti:
> >     am65-cpsw: Handle -EPROBE_DEFER for Serdes PHY") in net-next
> >     (next-20230123 and later).
>
> I was trying to apply this on rc1, so ofcourse this fails for me? How do
> we resolve this?
>
> I can skip this patch, provide a tag for this to be pulled into -net
> tree

Thanks, that's one option.
The other option is to postpone this patch, and apply it after v6.3-rc1.

Thanks!

> > --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > @@ -1460,11 +1460,9 @@ static int am65_cpsw_init_serdes_phy(struct device *dev, struct device_node *por
> >       struct phy *phy;
> >       int ret;
> >
> > -     phy = devm_of_phy_get(dev, port_np, name);
> > -     if (PTR_ERR(phy) == -ENODEV)
> > -             return 0;
> > -     if (IS_ERR(phy))
> > -             return PTR_ERR(phy);
> > +     phy = devm_of_phy_optional_get(dev, port_np, name);
> > +     if (IS_ERR_OR_NULL(phy))
> > +             return PTR_ERR_OR_ZERO(phy);
> >
> >       /* Serdes PHY exists. Store it. */
> >       port->slave.serdes_phy = phy;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 6/9] net: ethernet: ti: am65-cpsw: Convert to devm_of_phy_optional_get()

[Vinod Koul]

On 03-02-23, 09:04, Geert Uytterhoeven wrote:
> Hi Vinod,
> 
> On Fri, Feb 3, 2023 at 6:27 AM Vinod Koul <vkoul@kernel.org> wrote:
> > On 24-01-23, 19:37, Geert Uytterhoeven wrote:
> > > Use the new devm_of_phy_optional_get() helper instead of open-coding the
> > > same operation.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > > v2:
> > >   - Rebase on top of commit 854617f52ab42418 ("net: ethernet: ti:
> > >     am65-cpsw: Handle -EPROBE_DEFER for Serdes PHY") in net-next
> > >     (next-20230123 and later).
> >
> > I was trying to apply this on rc1, so ofcourse this fails for me? How do
> > we resolve this?
> >
> > I can skip this patch, provide a tag for this to be pulled into -net
> > tree
> 
> Thanks, that's one option.
> The other option is to postpone this patch, and apply it after v6.3-rc1.

Okay, done

-- 
~Vinod

Re: [PATCH treewide v2 0/9] phy: Add devm_of_phy_optional_get() helper

[Vinod Koul]

On 24-01-23, 19:37, Geert Uytterhoeven wrote:
> 	Hi Vinod et al,
> 
> While there exist several optional_get() PHY helper functions, there is
> no optional variant of devm_of_phy_get(), leading to several drivers
> implementing this theirselves, sometimes in buggy ways.
> 
> Hence this series, after two cleanup patches, introduces a
> devm_of_phy_optional_get() helper(), and converts existing users of
> devm_of_phy_get() where appropriate.

Applied and pushed to tag phy-devm_of_phy_optional_get

The following changes since commit 1b929c02afd37871d5afb9d498426f83432e71c2:

  Linux 6.2-rc1 (2022-12-25 13:41:39 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git tags/phy-devm_of_phy_optional_get

for you to fetch changes up to 41a435e30eb007ca2c8f71db734af6ec3509af4d:

  usb: host: ohci-exynos: Convert to devm_of_phy_optional_get() (2023-02-03 11:19:35 +0530)

----------------------------------------------------------------
Phy tag for new devm_of_phy_optional_get() API

----------------------------------------------------------------
Geert Uytterhoeven (8):
      phy: Remove unused phy_optional_get()
      doc: phy: Document devm_of_phy_get()
      phy: Add devm_of_phy_optional_get() helper
      net: fman: memac: Convert to devm_of_phy_optional_get()
      net: lan966x: Convert to devm_of_phy_optional_get()
      PCI: tegra: Convert to devm_of_phy_optional_get()
      usb: host: ehci-exynos: Convert to devm_of_phy_optional_get()
      usb: host: ohci-exynos: Convert to devm_of_phy_optional_get()

 Documentation/driver-api/phy/phy.rst                  | 24 ++++++++++++++----------
 drivers/net/ethernet/freescale/fman/fman_memac.c      |  9 ++++-----
 drivers/net/ethernet/microchip/lan966x/lan966x_main.c |  5 ++---
 drivers/pci/controller/pci-tegra.c                    |  5 +----
 drivers/phy/phy-core.c                                | 51 ++++++++++++++++++++++++++++++---------------------
 drivers/usb/host/ehci-exynos.c                        | 23 ++++++-----------------
 drivers/usb/host/ohci-exynos.c                        | 23 ++++++-----------------
 include/linux/phy/phy.h                               | 16 +++++++++-------
 8 files changed, 72 insertions(+), 84 deletions(-)


> Thanks!
> 
> > > --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > > +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > > @@ -1460,11 +1460,9 @@ static int am65_cpsw_init_serdes_phy(struct device *dev, struct device_node *por
> > >       struct phy *phy;
> > >       int ret;
> > >
> > > -     phy = devm_of_phy_get(dev, port_np, name);
> > > -     if (PTR_ERR(phy) == -ENODEV)
> > > -             return 0;
> > > -     if (IS_ERR(phy))
> > > -             return PTR_ERR(phy);
> > > +     phy = devm_of_phy_optional_get(dev, port_np, name);
> > > +     if (IS_ERR_OR_NULL(phy))
> > > +             return PTR_ERR_OR_ZERO(phy);
> > >
> > >       /* Serdes PHY exists. Store it. */
> > >       port->slave.serdes_phy = phy;
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds


> 
> Changes compared to v1[1]:
>   - Incorporate "[PATCH v2 1/9] phy: Remove unused phy_optional_get()",
>     as it touches the same documentation,
>   - New patch "[PATCH v2 2/9] doc: phy: Document devm_of_phy_get()",
>   - Print an error message in case of failure, as requested by RobH,
>   - Update Documentation,
>   - Clarify removed checks for -ENODEV and -ENOSYS,
>   - Remove error printing in case of real failures from callers,
>   - Rebase am65-cpsw change on top of commit 854617f52ab42418 ("net:
>     ethernet: ti: am65-cpsw: Handle -EPROBE_DEFER for Serdes PHY") in
>     net-next (next-20230123 and later),
>   - Add Reviewed-by, Acked-by.
> 
> Most of this series been compile-tested only, but the new helper itself
> has been tested with a new user[2].
> 
> Thanks for your comments!
> 
> [1] "[PATCH treewide 0/7] phy: Add devm_of_phy_optional_get() helper"
>     https://lore.kernel.org/r/cover.1674036164.git.geert+renesas@glider.be
> [2] "[PATCH 12/12] can: rcar_canfd: Add transceiver support"
>     https://lore.kernel.org/r/e825b50a843ffe40e33f34e4d858c07c1b2ff259.1674499048.git.geert+renesas@glider.be
> 
> Geert Uytterhoeven (9):
>   phy: Remove unused phy_optional_get()
>   doc: phy: Document devm_of_phy_get()
>   phy: Add devm_of_phy_optional_get() helper
>   net: fman: memac: Convert to devm_of_phy_optional_get()
>   net: lan966x: Convert to devm_of_phy_optional_get()
>   net: ethernet: ti: am65-cpsw: Convert to devm_of_phy_optional_get()
>   PCI: tegra: Convert to devm_of_phy_optional_get()
>   usb: host: ehci-exynos: Convert to devm_of_phy_optional_get()
>   usb: host: ohci-exynos: Convert to devm_of_phy_optional_get()
> 
>  Documentation/driver-api/phy/phy.rst          | 24 +++++----
>  .../net/ethernet/freescale/fman/fman_memac.c  |  9 ++--
>  .../ethernet/microchip/lan966x/lan966x_main.c |  5 +-
>  drivers/net/ethernet/ti/am65-cpsw-nuss.c      |  8 ++-
>  drivers/pci/controller/pci-tegra.c            |  5 +-
>  drivers/phy/phy-core.c                        | 51 +++++++++++--------
>  drivers/usb/host/ehci-exynos.c                | 23 +++------
>  drivers/usb/host/ohci-exynos.c                | 23 +++------
>  include/linux/phy/phy.h                       | 16 +++---
>  9 files changed, 75 insertions(+), 89 deletions(-)
> 
> -- 
> 2.34.1
> 
> Gr{oetje,eeting}s,
> 
> 						Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> 							    -- Linus Torvalds

-- 
~Vinod