[PATCH v2 0/4] phy: core: Add phy bulk helpers support

[PATCH v2 0/4] phy: core: Add phy bulk helpers support

[Inochi Amaoto]

As discussed in [1], some drivers may do not care the difference between
phys. Instead, they only need to treat them as a group and operate them
together. This means a bulk operation is needed.

Add some bulk helper functions for phy core by referencing the design of
clock/reset subsystem. This can relieve the driver owners' life who needs
to handle many phys, as well as each phy error reporting.

[1] https://lore.kernel.org/linux-pci/ak9KzNFF26B0Kttz@ashevche-desk.local/

Changed from v1:
- https://lore.kernel.org/linux-phy/20260831025319.94886-1-inochiama@gmail.com/
patch 1:
1. Update document.
patch 2:
1. Fix document format.
2. Fix several document description.
patch 3:
1. Split into two patches: one for non devm helpers, one for devm helpers.
2. All the bulk helper now use unsigned int for counts.
3. Fix multiple the while loop statement.
4. Rename of_phy_get_parent_count() to of_phy_get_count().
5. Fix some 80 line function problems.
6. Use PTR_ERR_OR_ZERO() to simplify the error handle.
7. Fix zero value in the document
8. Use two separate release function for devm helpers.

Inochi Amaoto (4):
  phy: core: Add common helper to add phy phandle device link
  phy: core: Add common helper for get phy phandle by index
  phy: core: Add phy bulk data helper functions
  phy: core: Add managed phy bulk data helper functions

 drivers/phy/phy-core.c  | 700 +++++++++++++++++++++++++++++++++++++---
 include/linux/phy/phy.h | 186 +++++++++++
 2 files changed, 850 insertions(+), 36 deletions(-)

--
2.55.0

[PATCH v2 1/4] phy: core: Add common helper to add phy phandle device link

[Inochi Amaoto]

It is very common for adding a device link for phy phandle
for device managed phy helper functions. So add a common
helper for future reuse.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/phy/phy-core.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 21aaf2f76e53..ea79913fbb87 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -606,6 +606,26 @@ int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
 }
 EXPORT_SYMBOL_GPL(phy_validate);
 
+/**
+ * phy_add_device_link() - Associate the phy with the device
+ * @dev: the device to link the phy
+ * @phy: the phy to associate
+ *
+ * Associate the phy with the device by using device link.
+ */
+static void phy_add_device_link(struct device *dev, struct phy *phy)
+{
+	struct device_link *link;
+
+	if (!phy)
+		return;
+
+	link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
+	if (!link)
+		dev_dbg(dev, "failed to create device link to %s\n",
+			dev_name(phy->dev.parent));
+}
+
 /**
  * _of_phy_get() - lookup and obtain a reference to a phy by phandle
  * @np: device_node for which to get the phy
@@ -784,7 +804,6 @@ struct phy *phy_get(struct device *dev, const char *string)
 {
 	int index = 0;
 	struct phy *phy;
-	struct device_link *link;
 
 	if (dev->of_node) {
 		if (string)
@@ -808,10 +827,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);
-	if (!link)
-		dev_dbg(dev, "failed to create device link to %s\n",
-			dev_name(phy->dev.parent));
+	phy_add_device_link(dev, phy);
 
 	return phy;
 }
@@ -885,7 +901,6 @@ 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)
@@ -900,10 +915,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);
-	if (!link)
-		dev_dbg(dev, "failed to create device link to %s\n",
-			dev_name(phy->dev.parent));
+	phy_add_device_link(dev, phy);
 
 	return phy;
 }
@@ -955,7 +967,6 @@ 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)
@@ -977,10 +988,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);
-	if (!link)
-		dev_dbg(dev, "failed to create device link to %s\n",
-			dev_name(phy->dev.parent));
+	phy_add_device_link(dev, phy);
 
 	return phy;
 }
-- 
2.55.0

Re: [PATCH v2 1/4] phy: core: Add common helper to add phy phandle device link

[Andy Shevchenko]

On Fri, Sep 04, 2026 at 04:37:05PM +0800, Inochi Amaoto wrote:
> It is very common for adding a device link for phy phandle
> for device managed phy helper functions. So add a common
> helper for future reuse.

Makes sense,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/4] phy: core: Add common helper to add phy phandle device link

[Vladimir Oltean]

On Fri, Sep 04, 2026 at 04:37:05PM +0800, Inochi Amaoto wrote:
> It is very common for adding a device link for phy phandle
> for device managed phy helper functions. So add a common
> helper for future reuse.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  drivers/phy/phy-core.c | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 21aaf2f76e53..ea79913fbb87 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -606,6 +606,26 @@ int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
>  }
>  EXPORT_SYMBOL_GPL(phy_validate);
>  
> +/**
> + * phy_add_device_link() - Associate the phy with the device
> + * @dev: the device to link the phy
> + * @phy: the phy to associate
> + *
> + * Associate the phy with the device by using device link.

I guess it would be good to specify somewhere "consumer" device, because
there are multiple devices involved.

> + */
> +static void phy_add_device_link(struct device *dev, struct phy *phy)
> +{
> +	struct device_link *link;
> +
> +	if (!phy)
> +		return;
> +
> +	link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> +	if (!link)
> +		dev_dbg(dev, "failed to create device link to %s\n",
> +			dev_name(phy->dev.parent));
> +}
> +
>  /**
>   * _of_phy_get() - lookup and obtain a reference to a phy by phandle
>   * @np: device_node for which to get the phy

Re: [PATCH v2 1/4] phy: core: Add common helper to add phy phandle device link

[Inochi Amaoto]

On Mon, Sep 07, 2026 at 02:01:21PM +0300, Vladimir Oltean wrote:
> On Fri, Sep 04, 2026 at 04:37:05PM +0800, Inochi Amaoto wrote:
> > It is very common for adding a device link for phy phandle
> > for device managed phy helper functions. So add a common
> > helper for future reuse.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  drivers/phy/phy-core.c | 38 +++++++++++++++++++++++---------------
> >  1 file changed, 23 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 21aaf2f76e53..ea79913fbb87 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -606,6 +606,26 @@ int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
> >  }
> >  EXPORT_SYMBOL_GPL(phy_validate);
> >  
> > +/**
> > + * phy_add_device_link() - Associate the phy with the device
> > + * @dev: the device to link the phy
> > + * @phy: the phy to associate
> > + *
> > + * Associate the phy with the device by using device link.
> 
> I guess it would be good to specify somewhere "consumer" device, because
> there are multiple devices involved.
> 

Reasonable. I will update this comment. Thanks.

Regards,
Inochi

> > + */
> > +static void phy_add_device_link(struct device *dev, struct phy *phy)
> > +{
> > +	struct device_link *link;
> > +
> > +	if (!phy)
> > +		return;
> > +
> > +	link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> > +	if (!link)
> > +		dev_dbg(dev, "failed to create device link to %s\n",
> > +			dev_name(phy->dev.parent));
> > +}
> > +
> >  /**
> >   * _of_phy_get() - lookup and obtain a reference to a phy by phandle
> >   * @np: device_node for which to get the phy

[PATCH v2 2/4] phy: core: Add common helper for get phy phandle by index

[Inochi Amaoto]

Several phy helper use index to get phy phandle of a device node,
add a common function for the future reuse.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/phy/phy-core.c | 51 +++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index ea79913fbb87..a7d7acb5d3c2 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -681,33 +681,49 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
 	return phy;
 }
 
+/**
+ * of_phy_get_by_index() - lookup and obtain a reference to a phy using a
+ * device_node by index.
+ * @np: device_node for which to get the phy
+ * @index: index of the phy from device's point of view
+ *
+ * Returns: the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy. The caller is responsible for
+ * calling of_phy_put() to release that count.
+ */
+static struct phy *of_phy_get_by_index(struct device_node *np, int index)
+{
+	struct phy *phy;
+
+	phy = _of_phy_get(np, index);
+	if (IS_ERR(phy))
+		return phy;
+
+	if (!try_module_get(phy->ops->owner))
+		return ERR_PTR(-EPROBE_DEFER);
+
+	get_device(&phy->dev);
+
+	return phy;
+}
+
 /**
  * of_phy_get() - lookup and obtain a reference to a phy using a device_node.
  * @np: device_node for which to get the phy
  * @con_id: name of the phy from device's point of view
  *
- * Returns the phy driver, after getting a refcount to it; or
+ * Returns: the phy driver, after getting a refcount to it; or
  * -ENODEV if there is no such phy. The caller is responsible for
  * calling of_phy_put() to release that count.
  */
 struct phy *of_phy_get(struct device_node *np, const char *con_id)
 {
-	struct phy *phy = NULL;
 	int index = 0;
 
 	if (con_id)
 		index = of_property_match_string(np, "phy-names", con_id);
 
-	phy = _of_phy_get(np, index);
-	if (IS_ERR(phy))
-		return phy;
-
-	if (!try_module_get(phy->ops->owner))
-		return ERR_PTR(-EPROBE_DEFER);
-
-	get_device(&phy->dev);
-
-	return phy;
+	return of_phy_get_by_index(np, index);
 }
 EXPORT_SYMBOL_GPL(of_phy_get);
 
@@ -957,7 +973,7 @@ EXPORT_SYMBOL_GPL(devm_of_phy_optional_get);
  * @np: node containing the phy
  * @index: index of the phy
  *
- * Gets the phy using _of_phy_get(), then gets a refcount to it,
+ * Gets the phy using of_phy_get_by_index(), then gets a refcount to it,
  * and associates a device with it using devres. On driver detach,
  * release function is invoked on the devres data,
  * then, devres data is freed.
@@ -972,19 +988,12 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	phy = _of_phy_get(np, index);
+	phy = of_phy_get_by_index(np, index);
 	if (IS_ERR(phy)) {
 		devres_free(ptr);
 		return phy;
 	}
 
-	if (!try_module_get(phy->ops->owner)) {
-		devres_free(ptr);
-		return ERR_PTR(-EPROBE_DEFER);
-	}
-
-	get_device(&phy->dev);
-
 	*ptr = phy;
 	devres_add(dev, ptr);
 
-- 
2.55.0

Re: [PATCH v2 2/4] phy: core: Add common helper for get phy phandle by index

[Andy Shevchenko]

On Fri, Sep 04, 2026 at 04:37:06PM +0800, Inochi Amaoto wrote:
> Several phy helper use index to get phy phandle of a device node,

"Several" is used when you start with 3+. I see only a couple of.

> add a common function for the future reuse.

Code wise makes sense.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/4] phy: core: Add common helper for get phy phandle by index

[Vladimir Oltean]

On Fri, Sep 04, 2026 at 04:37:06PM +0800, Inochi Amaoto wrote:
> Several phy helper use index to get phy phandle of a device node,
> add a common function for the future reuse.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  drivers/phy/phy-core.c | 51 +++++++++++++++++++++++++-----------------
>  1 file changed, 30 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index ea79913fbb87..a7d7acb5d3c2 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -681,33 +681,49 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
>  	return phy;
>  }
>  
> +/**
> + * of_phy_get_by_index() - lookup and obtain a reference to a phy using a
> + * device_node by index.
> + * @np: device_node for which to get the phy
> + * @index: index of the phy from device's point of view
> + *
> + * Returns: the phy driver, after getting a refcount to it; or

Not driver, but device.

> + * -ENODEV if there is no such phy. The caller is responsible for
> + * calling of_phy_put() to release that count.
> + */
> +static struct phy *of_phy_get_by_index(struct device_node *np, int index)
> +{
> +	struct phy *phy;
> +
> +	phy = _of_phy_get(np, index);
> +	if (IS_ERR(phy))
> +		return phy;
> +
> +	if (!try_module_get(phy->ops->owner))
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	get_device(&phy->dev);
> +
> +	return phy;
> +}
> +
>  /**
>   * of_phy_get() - lookup and obtain a reference to a phy using a device_node.
>   * @np: device_node for which to get the phy
>   * @con_id: name of the phy from device's point of view
>   *
> - * Returns the phy driver, after getting a refcount to it; or
> + * Returns: the phy driver, after getting a refcount to it; or

Same comment here. You are changing this line anyway.

>   * -ENODEV if there is no such phy. The caller is responsible for
>   * calling of_phy_put() to release that count.
>   */
>  struct phy *of_phy_get(struct device_node *np, const char *con_id)
>  {
> -	struct phy *phy = NULL;
>  	int index = 0;
>  
>  	if (con_id)
>  		index = of_property_match_string(np, "phy-names", con_id);
>  
> -	phy = _of_phy_get(np, index);
> -	if (IS_ERR(phy))
> -		return phy;
> -
> -	if (!try_module_get(phy->ops->owner))
> -		return ERR_PTR(-EPROBE_DEFER);
> -
> -	get_device(&phy->dev);
> -
> -	return phy;
> +	return of_phy_get_by_index(np, index);
>  }
>  EXPORT_SYMBOL_GPL(of_phy_get);
>  
> @@ -957,7 +973,7 @@ EXPORT_SYMBOL_GPL(devm_of_phy_optional_get);
>   * @np: node containing the phy
>   * @index: index of the phy
>   *
> - * Gets the phy using _of_phy_get(), then gets a refcount to it,
> + * Gets the phy using of_phy_get_by_index(), then gets a refcount to it,
>   * and associates a device with it using devres. On driver detach,
>   * release function is invoked on the devres data,
>   * then, devres data is freed.
> @@ -972,19 +988,12 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
>  	if (!ptr)
>  		return ERR_PTR(-ENOMEM);
>  
> -	phy = _of_phy_get(np, index);
> +	phy = of_phy_get_by_index(np, index);
>  	if (IS_ERR(phy)) {
>  		devres_free(ptr);
>  		return phy;
>  	}
>  
> -	if (!try_module_get(phy->ops->owner)) {
> -		devres_free(ptr);
> -		return ERR_PTR(-EPROBE_DEFER);
> -	}
> -
> -	get_device(&phy->dev);
> -
>  	*ptr = phy;
>  	devres_add(dev, ptr);
>  
> -- 
> 2.55.0
> 
> 

Re: [PATCH v2 2/4] phy: core: Add common helper for get phy phandle by index

[Inochi Amaoto]

On Mon, Sep 07, 2026 at 02:03:08PM +0300, Vladimir Oltean wrote:
> On Fri, Sep 04, 2026 at 04:37:06PM +0800, Inochi Amaoto wrote:
> > Several phy helper use index to get phy phandle of a device node,
> > add a common function for the future reuse.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  drivers/phy/phy-core.c | 51 +++++++++++++++++++++++++-----------------
> >  1 file changed, 30 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index ea79913fbb87..a7d7acb5d3c2 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -681,33 +681,49 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
> >  	return phy;
> >  }
> >  
> > +/**
> > + * of_phy_get_by_index() - lookup and obtain a reference to a phy using a
> > + * device_node by index.
> > + * @np: device_node for which to get the phy
> > + * @index: index of the phy from device's point of view
> > + *
> > + * Returns: the phy driver, after getting a refcount to it; or
> 
> Not driver, but device.
> 
> > + * -ENODEV if there is no such phy. The caller is responsible for
> > + * calling of_phy_put() to release that count.
> > + */
> > +static struct phy *of_phy_get_by_index(struct device_node *np, int index)
> > +{
> > +	struct phy *phy;
> > +
> > +	phy = _of_phy_get(np, index);
> > +	if (IS_ERR(phy))
> > +		return phy;
> > +
> > +	if (!try_module_get(phy->ops->owner))
> > +		return ERR_PTR(-EPROBE_DEFER);
> > +
> > +	get_device(&phy->dev);
> > +
> > +	return phy;
> > +}
> > +
> >  /**
> >   * of_phy_get() - lookup and obtain a reference to a phy using a device_node.
> >   * @np: device_node for which to get the phy
> >   * @con_id: name of the phy from device's point of view
> >   *
> > - * Returns the phy driver, after getting a refcount to it; or
> > + * Returns: the phy driver, after getting a refcount to it; or
> 
> Same comment here. You are changing this line anyway.
> 
> >   * -ENODEV if there is no such phy. The caller is responsible for
> >   * calling of_phy_put() to release that count.
> >   */
> >  struct phy *of_phy_get(struct device_node *np, const char *con_id)
> >  {
> > -	struct phy *phy = NULL;
> >  	int index = 0;
> >  
> >  	if (con_id)
> >  		index = of_property_match_string(np, "phy-names", con_id);
> >  
> > -	phy = _of_phy_get(np, index);
> > -	if (IS_ERR(phy))
> > -		return phy;
> > -
> > -	if (!try_module_get(phy->ops->owner))
> > -		return ERR_PTR(-EPROBE_DEFER);
> > -
> > -	get_device(&phy->dev);
> > -
> > -	return phy;
> > +	return of_phy_get_by_index(np, index);
> >  }
> >  EXPORT_SYMBOL_GPL(of_phy_get);
> >  
> > @@ -957,7 +973,7 @@ EXPORT_SYMBOL_GPL(devm_of_phy_optional_get);
> >   * @np: node containing the phy
> >   * @index: index of the phy
> >   *
> > - * Gets the phy using _of_phy_get(), then gets a refcount to it,
> > + * Gets the phy using of_phy_get_by_index(), then gets a refcount to it,
> >   * and associates a device with it using devres. On driver detach,
> >   * release function is invoked on the devres data,
> >   * then, devres data is freed.
> > @@ -972,19 +988,12 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
> >  	if (!ptr)
> >  		return ERR_PTR(-ENOMEM);
> >  
> > -	phy = _of_phy_get(np, index);
> > +	phy = of_phy_get_by_index(np, index);
> >  	if (IS_ERR(phy)) {
> >  		devres_free(ptr);
> >  		return phy;
> >  	}
> >  
> > -	if (!try_module_get(phy->ops->owner)) {
> > -		devres_free(ptr);
> > -		return ERR_PTR(-EPROBE_DEFER);
> > -	}
> > -
> > -	get_device(&phy->dev);
> > -
> >  	*ptr = phy;
> >  	devres_add(dev, ptr);
> >  
> > -- 
> > 2.55.0
> > 
> > 
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

Ok, I will fix them, thanks.

Regards,
Inochi

[PATCH v2 3/4] phy: core: Add phy bulk data helper functions

[Inochi Amaoto]

Add several helper functions that allow drivers to get several phy
consumers in one operation. If any of the phy cannot be acquired then
any phys that were got will be put before returning to the caller.

This can relieve the driver owners' life who needs to handle many phys,
as well as each phy error reporting.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/phy/phy-core.c  | 413 ++++++++++++++++++++++++++++++++++++++++
 include/linux/phy/phy.h | 143 ++++++++++++++
 2 files changed, 556 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index a7d7acb5d3c2..f5596b8dfcfc 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -1003,6 +1003,419 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index);
 
+/**
+ * of_phy_get_count() - Get the number of phys of a device node
+ * @np: device_node for which to get the phy
+ *
+ * Return: the phy count if successful, %0 if no phy handle is found,
+ * negative error value if error occurs.
+ */
+static int of_phy_get_count(const struct device_node *np)
+{
+	int count;
+
+	count = of_count_phandle_with_args(np, "phys", "#phy-cells");
+
+	if (count == -ENOENT)
+		return 0;
+
+	return count;
+}
+
+/**
+ * phy_bulk_put() - release a set of PHYs obtained with phy_bulk_get()
+ * @dev: device that acquired the PHYs
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data with PHYs set
+ *
+ * Releases the PHY references in reverse order and clears the PHY pointer in
+ * each entry. The caller owns the phys array and is responsible for freeing it
+ * if necessary.
+ */
+void phy_bulk_put(struct device *dev, unsigned int num_phys,
+		  struct phy_bulk_data *phys)
+{
+	while (num_phys--) {
+		if (!IS_ERR_OR_NULL(phys[num_phys].phy))
+			phy_put(dev, phys[num_phys].phy);
+		phys[num_phys].phy = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(phy_bulk_put);
+
+/**
+ * of_phy_bulk_put() - release a set of PHYs obtained with of_phy_bulk_get()
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data with PHYs set
+ *
+ * Releases the PHY references in reverse order and clears the PHY pointer in
+ * each entry. The caller owns the phys array and is responsible for freeing it
+ * if necessary.
+ */
+void of_phy_bulk_put(unsigned int num_phys, struct phy_bulk_data *phys)
+{
+	while (num_phys--) {
+		of_phy_put(phys[num_phys].phy);
+		phys[num_phys].phy = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(of_phy_bulk_put);
+
+static int __phy_bulk_get(struct device *dev, unsigned int num_phys,
+			  struct phy_bulk_data *phys, bool optional)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < num_phys; i++)
+		phys[i].phy = NULL;
+
+	for (i = 0; i < num_phys; i++) {
+		phys[i].phy = phy_get(dev, phys[i].id);
+
+		ret = PTR_ERR_OR_ZERO(phys[i].phy);
+		if (ret) {
+			phys[i].phy = NULL;
+
+			if (ret == -ENODEV && optional)
+				continue;
+
+			dev_err_probe(dev, ret, "Failed to get phy: (%s)\n",
+				      phys[i].id);
+			goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	phy_bulk_put(dev, i, phys);
+
+	return ret;
+}
+
+/**
+ * phy_bulk_get() - lookup and obtain references to multiple PHYs
+ * @dev: device that requests the PHYs
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data with PHY names set
+ *
+ * Gets each PHY using phy_get(). This supports both device tree lookups and
+ * non-device-tree lookups registered with phy_create_lookup(). The caller must
+ * call phy_bulk_put() to release the PHY references.
+ *
+ * Return: %0 if successful, a negative error code otherwise
+ */
+int phy_bulk_get(struct device *dev, unsigned int num_phys,
+		 struct phy_bulk_data *phys)
+{
+	return __phy_bulk_get(dev, num_phys, phys, false);
+}
+EXPORT_SYMBOL_GPL(phy_bulk_get);
+
+/**
+ * phy_bulk_get_optional() - obtain references to multiple optional PHYs
+ * @dev: device that requests the PHYs
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data with PHY names set
+ *
+ * Gets each PHY using phy_get(). A PHY that is not present is stored as NULL
+ * instead of causing the operation to fail. The caller must call
+ * phy_bulk_put() to release the PHY references.
+ *
+ * Return: %0 if successful, a negative error code otherwise
+ */
+int phy_bulk_get_optional(struct device *dev, unsigned int num_phys,
+			  struct phy_bulk_data *phys)
+{
+	return __phy_bulk_get(dev, num_phys, phys, true);
+}
+EXPORT_SYMBOL_GPL(phy_bulk_get_optional);
+
+/**
+ * of_phy_bulk_get() - obtain references to multiple PHYs from a device node
+ * @np: device node containing the PHY references
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data with PHY names set
+ *
+ * Gets each PHY using of_phy_get() and the specified device node. The caller
+ * must call of_phy_bulk_put() to release the PHY references.
+ *
+ * Return: %0 if successful, a negative error code otherwise
+ */
+int of_phy_bulk_get(struct device_node *np, unsigned int num_phys,
+		    struct phy_bulk_data *phys)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < num_phys; i++)
+		phys[i].phy = NULL;
+
+	for (i = 0; i < num_phys; i++) {
+		phys[i].phy = of_phy_get(np, phys[i].id);
+		ret = PTR_ERR_OR_ZERO(phys[i].phy);
+		if (ret) {
+			phys[i].phy = NULL;
+			goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	of_phy_bulk_put(i, phys);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(of_phy_bulk_get);
+
+static int of_phy_bulk_get_by_index(struct device_node *np,
+				    unsigned int num_phys,
+				    struct phy_bulk_data *phys)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < num_phys; i++) {
+		phys[i].id = NULL;
+		phys[i].phy = NULL;
+	}
+
+	for (i = 0; i < num_phys; i++) {
+		of_property_read_string_index(np, "phy-names", i, &phys[i].id);
+
+		phys[i].phy = of_phy_get_by_index(np, i);
+
+		ret = PTR_ERR_OR_ZERO(phys[i].phy);
+		if (ret) {
+			phys[i].phy = NULL;
+			goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	of_phy_bulk_put(i, phys);
+
+	return ret;
+}
+
+/**
+ * of_phy_bulk_get_all() - obtain all PHYs from a device node
+ * @np: device node containing the PHY references
+ * @phys: pointer to store the allocated array of struct phy_bulk_data
+ *
+ * Gets every PHY referenced by the phys property in index order. PHY names are
+ * read from phy-names when present. The caller must call
+ * of_phy_bulk_put_all() to release the PHY references and free the array.
+ *
+ * Return: the number of PHYs on success, %0 if no PHYs are found, or a
+ * negative error code otherwise
+ */
+int of_phy_bulk_get_all(struct device_node *np, struct phy_bulk_data **phys)
+{
+	struct phy_bulk_data *phy_bulk;
+	int num_phys;
+	int ret;
+
+	num_phys = of_phy_get_count(np);
+	if (num_phys <= 0)
+		return num_phys;
+
+	phy_bulk = kmalloc_objs(*phy_bulk, num_phys);
+	if (!phy_bulk)
+		return -ENOMEM;
+
+	ret = of_phy_bulk_get_by_index(np, num_phys, phy_bulk);
+	if (ret) {
+		kfree(phy_bulk);
+		return ret;
+	}
+
+	*phys = phy_bulk;
+
+	return num_phys;
+}
+EXPORT_SYMBOL_GPL(of_phy_bulk_get_all);
+
+/**
+ * of_phy_bulk_put_all() - release and free PHYs obtained by
+ * of_phy_bulk_get_all()
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data to release and free
+ */
+void of_phy_bulk_put_all(unsigned int num_phys, struct phy_bulk_data *phys)
+{
+	if (IS_ERR_OR_NULL(phys))
+		return;
+
+	of_phy_bulk_put(num_phys, phys);
+	kfree(phys);
+}
+EXPORT_SYMBOL_GPL(of_phy_bulk_put_all);
+
+/**
+ * phy_bulk_get_all() - obtain all PHYs requested by a device
+ * @dev: device that requests the PHYs
+ * @phys: pointer to store the allocated array of struct phy_bulk_data
+ *
+ * Gets every PHY referenced by the device's device tree node and creates a
+ * device link for each PHY. The caller must call phy_bulk_put_all() to release
+ * the PHY references and free the array.
+ *
+ * Return: the number of PHYs on success, %0 if no PHYs are found, or a
+ * negative error code otherwise
+ */
+int phy_bulk_get_all(struct device *dev, struct phy_bulk_data **phys)
+{
+	struct device_node *np = dev_of_node(dev);
+	int ret;
+
+	*phys = NULL;
+
+	if (!np)
+		return 0;
+
+	ret = of_phy_bulk_get_all(np, phys);
+	if (ret > 0)
+		for (int i = 0; i < ret; i++)
+			phy_add_device_link(dev, (*phys)[i].phy);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_bulk_get_all);
+
+/**
+ * phy_bulk_put_all() - release and free PHYs obtained by phy_bulk_get_all()
+ * @dev: device that acquired the PHYs
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data to release and free
+ */
+void phy_bulk_put_all(struct device *dev, unsigned int num_phys,
+		      struct phy_bulk_data *phys)
+{
+	if (IS_ERR_OR_NULL(phys))
+		return;
+
+	phy_bulk_put(dev, num_phys, phys);
+	kfree(phys);
+}
+EXPORT_SYMBOL_GPL(phy_bulk_put_all);
+
+/**
+ * phy_bulk_init() - initialize multiple PHYs
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data to initialize
+ *
+ * Initializes the PHYs in array order. If an initialization fails, all PHYs
+ * initialized by this call are exited in reverse order.
+ *
+ * Return: %0 if successful, a negative error code otherwise
+ */
+int phy_bulk_init(unsigned int num_phys, struct phy_bulk_data *phys)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < num_phys; i++) {
+		ret = phy_init(phys[i].phy);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	while (i--)
+		phy_exit(phys[i].phy);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_bulk_init);
+
+/**
+ * phy_bulk_exit() - exit multiple PHYs
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data to exit
+ *
+ * Exits the PHYs in reverse array order. All PHYs are processed even if an
+ * error occurs.
+ *
+ * Return: %0 if successful, the first negative error code otherwise
+ */
+int phy_bulk_exit(unsigned int num_phys, struct phy_bulk_data *phys)
+{
+	int ret = 0;
+	int err;
+
+	while (num_phys--) {
+		err = phy_exit(phys[num_phys].phy);
+		if (err && !ret)
+			ret = err;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_bulk_exit);
+
+/**
+ * phy_bulk_power_on() - power on multiple PHYs
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data to power on
+ *
+ * Powers on the PHYs in array order. If a power-on operation fails, all PHYs
+ * powered on by this call are powered off in reverse order.
+ *
+ * Return: %0 if successful, a negative error code otherwise
+ */
+int phy_bulk_power_on(unsigned int num_phys, struct phy_bulk_data *phys)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < num_phys; i++) {
+		ret = phy_power_on(phys[i].phy);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	while (i--)
+		phy_power_off(phys[i].phy);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_bulk_power_on);
+
+/**
+ * phy_bulk_power_off() - power off multiple PHYs
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data to power off
+ *
+ * Powers off the PHYs in reverse array order. All PHYs are processed even if
+ * an error occurs.
+ *
+ * Return: %0 if successful, the first negative error code otherwise
+ */
+int phy_bulk_power_off(unsigned int num_phys, struct phy_bulk_data *phys)
+{
+	int ret = 0;
+	int err;
+
+	while (num_phys--) {
+		err = phy_power_off(phys[num_phys].phy);
+		if (err && !ret)
+			ret = err;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_bulk_power_off);
+
 /**
  * phy_create() - create a new phy
  * @dev: device that is creating the new phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index ea47975e288a..13f9f030122f 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -81,6 +81,17 @@ union phy_configure_opts {
 	struct phy_configure_opts_hdmi		hdmi;
 };
 
+/**
+ * struct phy_bulk_data - Data used for bulk phy operations.
+ *
+ * @id: phy consumer ID
+ * @phy: struct phy * to store the associated phy
+ */
+struct phy_bulk_data {
+	const char	*id;
+	struct phy	*phy;
+};
+
 /**
  * struct phy_ops - set of function pointers for performing phy operations
  * @init: operation to be performed for initializing phy
@@ -309,6 +320,26 @@ void devm_of_phy_provider_unregister(struct device *dev,
 	struct phy_provider *phy_provider);
 int phy_create_lookup(struct phy *phy, const char *con_id, const char *dev_id);
 void phy_remove_lookup(struct phy *phy, const char *con_id, const char *dev_id);
+
+int phy_bulk_get(struct device *dev, unsigned int num_phys,
+		 struct phy_bulk_data *phys);
+int phy_bulk_get_optional(struct device *dev, unsigned int num_phys,
+			  struct phy_bulk_data *phys);
+int of_phy_bulk_get(struct device_node *np, unsigned int num_phys,
+		    struct phy_bulk_data *phys);
+int phy_bulk_get_all(struct device *dev, struct phy_bulk_data **phys);
+int of_phy_bulk_get_all(struct device_node *np, struct phy_bulk_data **phys);
+void phy_bulk_put(struct device *dev, unsigned int num_phys,
+		  struct phy_bulk_data *phys);
+void of_phy_bulk_put(unsigned int num_phys, struct phy_bulk_data *phys);
+void phy_bulk_put_all(struct device *dev, unsigned int num_phys,
+		      struct phy_bulk_data *phys);
+void of_phy_bulk_put_all(unsigned int num_phys, struct phy_bulk_data *phys);
+int phy_bulk_init(unsigned int num_phys, struct phy_bulk_data *phys);
+int phy_bulk_exit(unsigned int num_phys, struct phy_bulk_data *phys);
+int phy_bulk_power_on(unsigned int num_phys, struct phy_bulk_data *phys);
+int phy_bulk_power_off(unsigned int num_phys, struct phy_bulk_data *phys);
+
 #else
 static inline int phy_pm_runtime_get(struct phy *phy)
 {
@@ -493,6 +524,118 @@ static inline struct phy *devm_of_phy_get_by_index(struct device *dev,
 	return ERR_PTR(-ENOSYS);
 }
 
+static inline int phy_bulk_get(struct device *dev, unsigned int num_phys,
+			       struct phy_bulk_data *phys)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int phy_bulk_get_optional(struct device *dev,
+					unsigned int num_phys,
+					struct phy_bulk_data *phys)
+{
+	if (!phys)
+		return 0;
+
+	for (unsigned int i = 0; i < num_phys; i++)
+		phys[i].phy = NULL;
+
+	return 0;
+}
+
+static inline int of_phy_bulk_get(struct device_node *np, unsigned int num_phys,
+				  struct phy_bulk_data *phys)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int phy_bulk_get_all(struct device *dev,
+				   struct phy_bulk_data **phys)
+{
+	if (phys)
+		*phys = NULL;
+
+	return -EOPNOTSUPP;
+}
+
+static inline int of_phy_bulk_get_all(struct device_node *np,
+				      struct phy_bulk_data **phys)
+{
+	if (phys)
+		*phys = NULL;
+
+	return -EOPNOTSUPP;
+}
+
+static inline void phy_bulk_put(struct device *dev, unsigned int num_phys,
+				struct phy_bulk_data *phys)
+{
+	if (!phys)
+		return;
+
+	while (num_phys--)
+		phys[num_phys].phy = NULL;
+}
+
+static inline void of_phy_bulk_put(unsigned int num_phys,
+				   struct phy_bulk_data *phys)
+{
+	if (!phys)
+		return;
+
+	while (num_phys--)
+		phys[num_phys].phy = NULL;
+}
+
+static inline void phy_bulk_put_all(struct device *dev, unsigned int num_phys,
+				    struct phy_bulk_data *phys)
+{
+	phy_bulk_put(dev, num_phys, phys);
+}
+
+static inline void of_phy_bulk_put_all(unsigned int num_phys,
+				       struct phy_bulk_data *phys)
+{
+	of_phy_bulk_put(num_phys, phys);
+}
+
+static inline int phy_bulk_check_disabled(unsigned int num_phys,
+					  struct phy_bulk_data *phys)
+{
+	if (!phys)
+		return 0;
+
+	for (unsigned int i = 0; i < num_phys; i++)
+		if (phys[i].phy)
+			return -EOPNOTSUPP;
+
+	return 0;
+}
+
+static inline int phy_bulk_init(unsigned int num_phys,
+				struct phy_bulk_data *phys)
+{
+	return phy_bulk_check_disabled(num_phys, phys);
+}
+
+static inline int phy_bulk_exit(unsigned int num_phys,
+				struct phy_bulk_data *phys)
+{
+	return phy_bulk_check_disabled(num_phys, phys);
+}
+
+static inline int phy_bulk_power_on(unsigned int num_phys,
+				    struct phy_bulk_data *phys)
+{
+	return phy_bulk_check_disabled(num_phys, phys);
+}
+
+static inline int phy_bulk_power_off(unsigned int num_phys,
+				     struct phy_bulk_data *phys)
+{
+	return phy_bulk_check_disabled(num_phys, phys);
+}
+
 static inline void of_phy_put(struct phy *phy)
 {
 }
-- 
2.55.0

Re: [PATCH v2 3/4] phy: core: Add phy bulk data helper functions

[Vladimir Oltean]

On Fri, Sep 04, 2026 at 04:37:07PM +0800, Inochi Amaoto wrote:
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index ea47975e288a..13f9f030122f 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -81,6 +81,17 @@ union phy_configure_opts {
>  	struct phy_configure_opts_hdmi		hdmi;
>  };
>  
> +/**
> + * struct phy_bulk_data - Data used for bulk phy operations.
> + *
> + * @id: phy consumer ID
> + * @phy: struct phy * to store the associated phy
> + */
> +struct phy_bulk_data {
> +	const char	*id;
> +	struct phy	*phy;

It's not clear from your design intention if struct phy_bulk_data :: phy
is permitted to be accessed individually by consumers. Personally I see
no reason to. In that case, can you explicitly mark "struct phy *phy" as
"/* private: Internal use */" (like struct regulator_bulk_data does,
albeit for "ret"), or simply say in the kernel-doc description that it
is managed internally by the PHY framework (as is done for both struct
regulator_bulk_data and for struct icc_bulk_data :: path)?

This should help at least LLM review flag abusive users.

> +};

Re: [PATCH v2 3/4] phy: core: Add phy bulk data helper functions

[Inochi Amaoto]

On Mon, Sep 07, 2026 at 01:56:11PM +0300, Vladimir Oltean wrote:
> On Fri, Sep 04, 2026 at 04:37:07PM +0800, Inochi Amaoto wrote:
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index ea47975e288a..13f9f030122f 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -81,6 +81,17 @@ union phy_configure_opts {
> >  	struct phy_configure_opts_hdmi		hdmi;
> >  };
> >  
> > +/**
> > + * struct phy_bulk_data - Data used for bulk phy operations.
> > + *
> > + * @id: phy consumer ID
> > + * @phy: struct phy * to store the associated phy
> > + */
> > +struct phy_bulk_data {
> > +	const char	*id;
> > +	struct phy	*phy;
> 
> It's not clear from your design intention if struct phy_bulk_data :: phy
> is permitted to be accessed individually by consumers. Personally I see
> no reason to. In that case, can you explicitly mark "struct phy *phy" as
> "/* private: Internal use */" (like struct regulator_bulk_data does,
> albeit for "ret"), or simply say in the kernel-doc description that it
> is managed internally by the PHY framework (as is done for both struct
> regulator_bulk_data and for struct icc_bulk_data :: path)?
> 
> This should help at least LLM review flag abusive users.
> 
> > +};

Yes, in my design it does not allow and I have no reason for this too. 
Because the bulk data helpers are for ones that does not care a specific
phy. So this is fine for me. Thanks for the thing I have missed.

Regards
Inochi

Re: [PATCH v2 3/4] phy: core: Add phy bulk data helper functions

[Vladimir Oltean]

On Fri, Sep 04, 2026 at 04:37:07PM +0800, Inochi Amaoto wrote:
> +static inline int phy_bulk_get_all(struct device *dev,
> +				   struct phy_bulk_data **phys)
> +{
> +	if (phys)
> +		*phys = NULL;
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int of_phy_bulk_get_all(struct device_node *np,
> +				      struct phy_bulk_data **phys)
> +{
> +	if (phys)
> +		*phys = NULL;
> +
> +	return -EOPNOTSUPP;
> +}

Why do the stub definitions of *_get_all() return an error?
I would expect these to have optional semantics, i.e. 0 PHYs are not an
error to the consumer.

For reference, I am comparing with clk_bulk_get_all() which returns 0.

> +
> +static inline void phy_bulk_put(struct device *dev, unsigned int num_phys,
> +				struct phy_bulk_data *phys)
> +{
> +	if (!phys)
> +		return;
> +
> +	while (num_phys--)
> +		phys[num_phys].phy = NULL;
> +}
> +
> +static inline void of_phy_bulk_put(unsigned int num_phys,
> +				   struct phy_bulk_data *phys)
> +{
> +	if (!phys)
> +		return;
> +
> +	while (num_phys--)
> +		phys[num_phys].phy = NULL;
> +}
> +
> +static inline void phy_bulk_put_all(struct device *dev, unsigned int num_phys,
> +				    struct phy_bulk_data *phys)
> +{
> +	phy_bulk_put(dev, num_phys, phys);
> +}
> +
> +static inline void of_phy_bulk_put_all(unsigned int num_phys,
> +				       struct phy_bulk_data *phys)
> +{
> +	of_phy_bulk_put(num_phys, phys);
> +}
> +
> +static inline int phy_bulk_check_disabled(unsigned int num_phys,
> +					  struct phy_bulk_data *phys)
> +{
> +	if (!phys)
> +		return 0;
> +
> +	for (unsigned int i = 0; i < num_phys; i++)
> +		if (phys[i].phy)
> +			return -EOPNOTSUPP;

For consistency with the individual API, I believe this should be
-ENOSYS (not that I know why we would be using this error code).

> +
> +	return 0;
> +}

Can you update Documentation/driver-api/phy/phy.rst with some terse
references to the bulk API and its intended use? Not much, just say
what it's for (like multi-lane protocols, and why some operations are
missing: phy_validate(), phy_set_mode_ext() etc). I guess they are
missing because currently they have no user, which is OK, but the rest
of the world should be on the same page w.r.t. the future of this API.

Thanks!

Re: [PATCH v2 3/4] phy: core: Add phy bulk data helper functions

[Inochi Amaoto]

On Mon, Sep 07, 2026 at 02:48:37PM +0300, Vladimir Oltean wrote:
> On Fri, Sep 04, 2026 at 04:37:07PM +0800, Inochi Amaoto wrote:
> > +static inline int phy_bulk_get_all(struct device *dev,
> > +				   struct phy_bulk_data **phys)
> > +{
> > +	if (phys)
> > +		*phys = NULL;
> > +
> > +	return -EOPNOTSUPP;
> > +}
> > +
> > +static inline int of_phy_bulk_get_all(struct device_node *np,
> > +				      struct phy_bulk_data **phys)
> > +{
> > +	if (phys)
> > +		*phys = NULL;
> > +
> > +	return -EOPNOTSUPP;
> > +}
> 
> Why do the stub definitions of *_get_all() return an error?
> I would expect these to have optional semantics, i.e. 0 PHYs are not an
> error to the consumer.
> 
> For reference, I am comparing with clk_bulk_get_all() which returns 0.
> 

This is the thing I am not very clear to. I found the clk_bulk_get_all()
return 0. But something in the reset return -EOPNOTSUPP for non optional
get (I reference __reset_control_bulk_get, as reset does not have an
API that is the same as this). I am not very sure whether it is best. 

Since you think we should follow this optional semantics, I think it is
fine for me to change this to 0.

> > +
> > +static inline void phy_bulk_put(struct device *dev, unsigned int num_phys,
> > +				struct phy_bulk_data *phys)
> > +{
> > +	if (!phys)
> > +		return;
> > +
> > +	while (num_phys--)
> > +		phys[num_phys].phy = NULL;
> > +}
> > +
> > +static inline void of_phy_bulk_put(unsigned int num_phys,
> > +				   struct phy_bulk_data *phys)
> > +{
> > +	if (!phys)
> > +		return;
> > +
> > +	while (num_phys--)
> > +		phys[num_phys].phy = NULL;
> > +}
> > +
> > +static inline void phy_bulk_put_all(struct device *dev, unsigned int num_phys,
> > +				    struct phy_bulk_data *phys)
> > +{
> > +	phy_bulk_put(dev, num_phys, phys);
> > +}
> > +
> > +static inline void of_phy_bulk_put_all(unsigned int num_phys,
> > +				       struct phy_bulk_data *phys)
> > +{
> > +	of_phy_bulk_put(num_phys, phys);
> > +}
> > +
> > +static inline int phy_bulk_check_disabled(unsigned int num_phys,
> > +					  struct phy_bulk_data *phys)
> > +{
> > +	if (!phys)
> > +		return 0;
> > +
> > +	for (unsigned int i = 0; i < num_phys; i++)
> > +		if (phys[i].phy)
> > +			return -EOPNOTSUPP;
> 
> For consistency with the individual API, I believe this should be
> -ENOSYS (not that I know why we would be using this error code).
> 

In fact I think -ENOSYS is more suitable, but I found almost every
subsystem use -EOPNOTSUPP for such a blob. So I think it will be
good to follow a generic -EOPNOTSUPP. In fact I found nothing about
why the phy subsystem use -ENOSYS for this, maybe someone can
answer it.

Instead of switching to -ENOSYS, I think it could be more proper to
change the existing blobs to -EOPNOTSUPP?

> > +
> > +	return 0;
> > +}
> 
> Can you update Documentation/driver-api/phy/phy.rst with some terse
> references to the bulk API and its intended use? Not much, just say
> what it's for (like multi-lane protocols, and why some operations are
> missing: phy_validate(), phy_set_mode_ext() etc). I guess they are
> missing because currently they have no user, which is OK, but the rest
> of the world should be on the same page w.r.t. the future of this API.
> 
> Thanks!
> 

Yes, I will, this is something I missed. Thanks.

Regards,
Inochi

Re: [PATCH v2 3/4] phy: core: Add phy bulk data helper functions

[Vladimir Oltean]

On Mon, Sep 07, 2026 at 08:15:04PM +0800, Inochi Amaoto wrote:
> On Mon, Sep 07, 2026 at 02:48:37PM +0300, Vladimir Oltean wrote:
> > On Fri, Sep 04, 2026 at 04:37:07PM +0800, Inochi Amaoto wrote:
> > > +static inline int phy_bulk_get_all(struct device *dev,
> > > +				   struct phy_bulk_data **phys)
> > > +{
> > > +	if (phys)
> > > +		*phys = NULL;
> > > +
> > > +	return -EOPNOTSUPP;
> > > +}
> > > +
> > > +static inline int of_phy_bulk_get_all(struct device_node *np,
> > > +				      struct phy_bulk_data **phys)
> > > +{
> > > +	if (phys)
> > > +		*phys = NULL;
> > > +
> > > +	return -EOPNOTSUPP;
> > > +}
> > 
> > Why do the stub definitions of *_get_all() return an error?
> > I would expect these to have optional semantics, i.e. 0 PHYs are not an
> > error to the consumer.
> > 
> > For reference, I am comparing with clk_bulk_get_all() which returns 0.
> 
> This is the thing I am not very clear to. I found the clk_bulk_get_all()
> return 0. But something in the reset return -EOPNOTSUPP for non optional
> get (I reference __reset_control_bulk_get, as reset does not have an
> API that is the same as this). I am not very sure whether it is best. 
> 
> Since you think we should follow this optional semantics, I think it is
> fine for me to change this to 0.

I am only talking about the *phy_bulk_get_all() functions, which have no
num_phys argument, and which as you said, have no reset_control equivalent.

The optional nature of *_get_all() should come specifically from the
fact that the consumer isn't specifically asking for "this many" PHYs
(num_phys). So the core can return 0 and say "that's all", and technically
not lie. Not a fatal error to the consumer; the bulk API supports other
calls with num_phys=0.

> > > +
> > > +static inline void phy_bulk_put(struct device *dev, unsigned int num_phys,
> > > +				struct phy_bulk_data *phys)
> > > +{
> > > +	if (!phys)
> > > +		return;
> > > +
> > > +	while (num_phys--)
> > > +		phys[num_phys].phy = NULL;
> > > +}
> > > +
> > > +static inline void of_phy_bulk_put(unsigned int num_phys,
> > > +				   struct phy_bulk_data *phys)
> > > +{
> > > +	if (!phys)
> > > +		return;
> > > +
> > > +	while (num_phys--)
> > > +		phys[num_phys].phy = NULL;
> > > +}
> > > +
> > > +static inline void phy_bulk_put_all(struct device *dev, unsigned int num_phys,
> > > +				    struct phy_bulk_data *phys)
> > > +{
> > > +	phy_bulk_put(dev, num_phys, phys);
> > > +}
> > > +
> > > +static inline void of_phy_bulk_put_all(unsigned int num_phys,
> > > +				       struct phy_bulk_data *phys)
> > > +{
> > > +	of_phy_bulk_put(num_phys, phys);
> > > +}
> > > +
> > > +static inline int phy_bulk_check_disabled(unsigned int num_phys,
> > > +					  struct phy_bulk_data *phys)
> > > +{
> > > +	if (!phys)
> > > +		return 0;
> > > +
> > > +	for (unsigned int i = 0; i < num_phys; i++)
> > > +		if (phys[i].phy)
> > > +			return -EOPNOTSUPP;
> > 
> > For consistency with the individual API, I believe this should be
> > -ENOSYS (not that I know why we would be using this error code).
> > 
> 
> In fact I think -ENOSYS is more suitable, but I found almost every
> subsystem use -EOPNOTSUPP for such a blob.

Why do you consider -ENOSYS to be more suitable? In include/uapi/asm-generic/errno.h
it says "/* Invalid system call number */" which makes it pretty use
case specific.

> So I think it will be
> good to follow a generic -EOPNOTSUPP. In fact I found nothing about
> why the phy subsystem use -ENOSYS for this, maybe someone can
> answer it.

It's been that way since initial commit ff764963479a ("drivers: phy: add
generic PHY framework") with no explanation.

> 
> Instead of switching to -ENOSYS, I think it could be more proper to
> change the existing blobs to -EOPNOTSUPP?

Personally I have nothing against this, though it depends on how deeply
you want to go in.

If you want to make this change, watch out for the following callers
which explicitly check for -ENOSYS:
- drivers/ata/libahci_platform.c:374
- drivers/usb/dwc2/platform.c:246
- drivers/usb/dwc3/core.c:1591,1608
- drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1363

Only devm_phy_get() / devm_of_phy_get() return codes get parsed this way.
For the runtime consumer functions, all consumers seem -ENOSYS-unaware.

Though after seeing how some drivers treat -ENODEV (for an absent PHY)
and -ENOSYS (for the disabled Generic PHY framework) the same, I think
it might make more sense to return -ENODEV from the stubs.

Re: [PATCH v2 3/4] phy: core: Add phy bulk data helper functions

[Inochi Amaoto]

On Mon, Sep 07, 2026 at 03:43:18PM +0300, Vladimir Oltean wrote:
> On Mon, Sep 07, 2026 at 08:15:04PM +0800, Inochi Amaoto wrote:
> > On Mon, Sep 07, 2026 at 02:48:37PM +0300, Vladimir Oltean wrote:
> > > On Fri, Sep 04, 2026 at 04:37:07PM +0800, Inochi Amaoto wrote:
> > > > +static inline int phy_bulk_get_all(struct device *dev,
> > > > +				   struct phy_bulk_data **phys)
> > > > +{
> > > > +	if (phys)
> > > > +		*phys = NULL;
> > > > +
> > > > +	return -EOPNOTSUPP;
> > > > +}
> > > > +
> > > > +static inline int of_phy_bulk_get_all(struct device_node *np,
> > > > +				      struct phy_bulk_data **phys)
> > > > +{
> > > > +	if (phys)
> > > > +		*phys = NULL;
> > > > +
> > > > +	return -EOPNOTSUPP;
> > > > +}
> > > 
> > > Why do the stub definitions of *_get_all() return an error?
> > > I would expect these to have optional semantics, i.e. 0 PHYs are not an
> > > error to the consumer.
> > > 
> > > For reference, I am comparing with clk_bulk_get_all() which returns 0.
> > 
> > This is the thing I am not very clear to. I found the clk_bulk_get_all()
> > return 0. But something in the reset return -EOPNOTSUPP for non optional
> > get (I reference __reset_control_bulk_get, as reset does not have an
> > API that is the same as this). I am not very sure whether it is best. 
> > 
> > Since you think we should follow this optional semantics, I think it is
> > fine for me to change this to 0.
> 
> I am only talking about the *phy_bulk_get_all() functions, which have no
> num_phys argument, and which as you said, have no reset_control equivalent.
> 
> The optional nature of *_get_all() should come specifically from the
> fact that the consumer isn't specifically asking for "this many" PHYs
> (num_phys). So the core can return 0 and say "that's all", and technically
> not lie. Not a fatal error to the consumer; the bulk API supports other
> calls with num_phys=0.
> 
> > > > +
> > > > +static inline void phy_bulk_put(struct device *dev, unsigned int num_phys,
> > > > +				struct phy_bulk_data *phys)
> > > > +{
> > > > +	if (!phys)
> > > > +		return;
> > > > +
> > > > +	while (num_phys--)
> > > > +		phys[num_phys].phy = NULL;
> > > > +}
> > > > +
> > > > +static inline void of_phy_bulk_put(unsigned int num_phys,
> > > > +				   struct phy_bulk_data *phys)
> > > > +{
> > > > +	if (!phys)
> > > > +		return;
> > > > +
> > > > +	while (num_phys--)
> > > > +		phys[num_phys].phy = NULL;
> > > > +}
> > > > +
> > > > +static inline void phy_bulk_put_all(struct device *dev, unsigned int num_phys,
> > > > +				    struct phy_bulk_data *phys)
> > > > +{
> > > > +	phy_bulk_put(dev, num_phys, phys);
> > > > +}
> > > > +
> > > > +static inline void of_phy_bulk_put_all(unsigned int num_phys,
> > > > +				       struct phy_bulk_data *phys)
> > > > +{
> > > > +	of_phy_bulk_put(num_phys, phys);
> > > > +}
> > > > +
> > > > +static inline int phy_bulk_check_disabled(unsigned int num_phys,
> > > > +					  struct phy_bulk_data *phys)
> > > > +{
> > > > +	if (!phys)
> > > > +		return 0;
> > > > +
> > > > +	for (unsigned int i = 0; i < num_phys; i++)
> > > > +		if (phys[i].phy)
> > > > +			return -EOPNOTSUPP;
> > > 
> > > For consistency with the individual API, I believe this should be
> > > -ENOSYS (not that I know why we would be using this error code).
> > > 
> > 
> > In fact I think -ENOSYS is more suitable, but I found almost every
> > subsystem use -EOPNOTSUPP for such a blob.
> 
> Why do you consider -ENOSYS to be more suitable? In include/uapi/asm-generic/errno.h
> it says "/* Invalid system call number */" which makes it pretty use
> case specific.
> 

I said it is more suitable as the phy subsystem already uses this, so
use "-ENOSYS" will have the same view of the existing code. But in fact
I think -EOPNOTSUPP is a better option as it provide the right 
information.

> > So I think it will be
> > good to follow a generic -EOPNOTSUPP. In fact I found nothing about
> > why the phy subsystem use -ENOSYS for this, maybe someone can
> > answer it.
> 
> It's been that way since initial commit ff764963479a ("drivers: phy: add
> generic PHY framework") with no explanation.
> 

Yes, this is something confused me. I see nothing for this.

> > 
> > Instead of switching to -ENOSYS, I think it could be more proper to
> > change the existing blobs to -EOPNOTSUPP?
> 
> Personally I have nothing against this, though it depends on how deeply
> you want to go in.
> 
> If you want to make this change, watch out for the following callers
> which explicitly check for -ENOSYS:
> - drivers/ata/libahci_platform.c:374
> - drivers/usb/dwc2/platform.c:246
> - drivers/usb/dwc3/core.c:1591,1608
> - drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1363
> 
> Only devm_phy_get() / devm_of_phy_get() return codes get parsed this way.
> For the runtime consumer functions, all consumers seem -ENOSYS-unaware.
> 

I think I can have a try by sending another patch. I think I should do
some search before doing a change. Thanks for this reminder.

> Though after seeing how some drivers treat -ENODEV (for an absent PHY)
> and -ENOSYS (for the disabled Generic PHY framework) the same, I think
> it might make more sense to return -ENODEV from the stubs.
> 

Actually, I think this could be aligned with non stub code. I found some
of them return -ENODEV in some case. This could be make sense in some
case.

Regards
Inochi

[PATCH v2 4/4] phy: core: Add managed phy bulk data helper functions

[Inochi Amaoto]

Add device managed variants of the phy bulk helper functions. So
the driver can benefit from automatically managed phy handles.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/phy/phy-core.c  | 198 ++++++++++++++++++++++++++++++++++++++++
 include/linux/phy/phy.h |  43 +++++++++
 2 files changed, 241 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index f5596b8dfcfc..e28206f8dc89 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -1304,6 +1304,204 @@ void phy_bulk_put_all(struct device *dev, unsigned int num_phys,
 }
 EXPORT_SYMBOL_GPL(phy_bulk_put_all);
 
+struct phy_bulk_devres {
+	struct phy_bulk_data *phys;
+	unsigned int num_phys;
+};
+
+static void devm_phy_bulk_release(struct device *dev, void *res)
+{
+	struct phy_bulk_devres *devres = res;
+
+	phy_bulk_put(dev, devres->num_phys, devres->phys);
+}
+
+static int __devm_phy_bulk_get(struct device *dev, unsigned int num_phys,
+			       struct phy_bulk_data *phys, bool optional)
+{
+	struct phy_bulk_devres *devres;
+	int ret;
+
+	devres = devres_alloc(devm_phy_bulk_release, sizeof(*devres),
+			      GFP_KERNEL);
+	if (!devres)
+		return -ENOMEM;
+
+	ret = __phy_bulk_get(dev, num_phys, phys, optional);
+	if (ret) {
+		devres_free(devres);
+		return ret;
+	}
+
+	devres->phys = phys;
+	devres->num_phys = num_phys;
+	devres_add(dev, devres);
+
+	return 0;
+}
+
+/**
+ * devm_phy_bulk_get() - managed lookup of multiple PHYs
+ * @dev: device that requests the PHYs
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data with PHY names set
+ *
+ * Gets the PHYs using phy_bulk_get() and associates the references with @dev.
+ * The references are automatically released on driver detach.
+ *
+ * Return: %0 if successful, a negative error code otherwise
+ */
+int devm_phy_bulk_get(struct device *dev, unsigned int num_phys,
+		      struct phy_bulk_data *phys)
+{
+	return __devm_phy_bulk_get(dev, num_phys, phys, false);
+}
+EXPORT_SYMBOL_GPL(devm_phy_bulk_get);
+
+/**
+ * devm_phy_bulk_get_optional() - managed lookup of multiple optional PHYs
+ * @dev: device that requests the PHYs
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data with PHY names set
+ *
+ * Gets the PHYs using phy_bulk_get_optional() and associates the references
+ * with @dev. Missing PHYs are stored as NULL. The references are automatically
+ * released on driver detach.
+ *
+ * Return: %0 if successful, a negative error code otherwise
+ */
+int devm_phy_bulk_get_optional(struct device *dev, unsigned int num_phys,
+			       struct phy_bulk_data *phys)
+{
+	return __devm_phy_bulk_get(dev, num_phys, phys, true);
+}
+EXPORT_SYMBOL_GPL(devm_phy_bulk_get_optional);
+
+/**
+ * devm_of_phy_bulk_get() - managed lookup of multiple PHYs from a device node
+ * @dev: device that requests the PHYs
+ * @np: device node containing the PHY references
+ * @num_phys: number of entries in the phys array
+ * @phys: array of struct phy_bulk_data with PHY names set
+ *
+ * Gets the PHYs using of_phy_bulk_get() from the specified device node,
+ * associates the references with @dev, and creates a device link for each PHY.
+ * The references are automatically released on driver detach.
+ *
+ * Return: %0 if successful, a negative error code otherwise
+ */
+int devm_of_phy_bulk_get(struct device *dev, struct device_node *np,
+			 unsigned int num_phys, struct phy_bulk_data *phys)
+{
+	struct phy_bulk_devres *devres;
+	int ret;
+
+	devres = devres_alloc(devm_phy_bulk_release, sizeof(*devres),
+			      GFP_KERNEL);
+	if (!devres)
+		return -ENOMEM;
+
+	ret = of_phy_bulk_get(np, num_phys, phys);
+	if (ret) {
+		devres_free(devres);
+		return ret;
+	}
+
+	for (unsigned int i = 0; i < num_phys; i++)
+		phy_add_device_link(dev, phys[i].phy);
+
+	devres->phys = phys;
+	devres->num_phys = num_phys;
+	devres_add(dev, devres);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_bulk_get);
+
+static void devm_phy_bulk_release_all(struct device *dev, void *res)
+{
+	struct phy_bulk_devres *devres = res;
+
+	phy_bulk_put_all(dev, devres->num_phys, devres->phys);
+}
+
+/**
+ * devm_phy_bulk_get_all() - managed lookup of all PHYs requested by a device
+ * @dev: device that requests the PHYs
+ * @phys: pointer to store the allocated array of struct phy_bulk_data
+ *
+ * Gets all PHYs using phy_bulk_get_all() and associates the allocated array and
+ * PHY references with @dev. They are automatically released on driver detach.
+ *
+ * Return: the number of PHYs on success, %0 if no PHYs are found, or a
+ * negative error code otherwise
+ */
+int devm_phy_bulk_get_all(struct device *dev, struct phy_bulk_data **phys)
+{
+	struct phy_bulk_devres *devres;
+	int ret;
+
+	*phys = NULL;
+
+	devres = devres_alloc(devm_phy_bulk_release_all, sizeof(*devres),
+			      GFP_KERNEL);
+	if (!devres)
+		return -ENOMEM;
+
+	ret = phy_bulk_get_all(dev, &devres->phys);
+	if (ret > 0) {
+		*phys = devres->phys;
+		devres->num_phys = ret;
+		devres_add(dev, devres);
+	} else {
+		devres_free(devres);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_phy_bulk_get_all);
+
+/**
+ * devm_of_phy_bulk_get_all() - managed lookup of all PHYs from a device node
+ * @dev: device that requests the PHYs
+ * @np: device node containing the PHY references
+ * @phys: pointer to store the allocated array of struct phy_bulk_data
+ *
+ * Gets all PHYs from the specified device node, associates the allocated array
+ * and PHY references with @dev, and creates a device link for each PHY. They
+ * are automatically released on driver detach.
+ *
+ * Return: the number of PHYs on success, %0 if no PHYs are found, or a
+ * negative error code otherwise
+ */
+int devm_of_phy_bulk_get_all(struct device *dev, struct device_node *np,
+			     struct phy_bulk_data **phys)
+{
+	struct phy_bulk_devres *devres;
+	int ret;
+
+	*phys = NULL;
+
+	devres = devres_alloc(devm_phy_bulk_release_all, sizeof(*devres),
+			      GFP_KERNEL);
+	if (!devres)
+		return -ENOMEM;
+
+	ret = of_phy_bulk_get_all(np, &devres->phys);
+	if (ret > 0) {
+		for (int i = 0; i < ret; i++)
+			phy_add_device_link(dev, devres->phys[i].phy);
+		*phys = devres->phys;
+		devres->num_phys = ret;
+		devres_add(dev, devres);
+	} else {
+		devres_free(devres);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_bulk_get_all);
+
 /**
  * phy_bulk_init() - initialize multiple PHYs
  * @num_phys: number of entries in the phys array
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 13f9f030122f..2af240f4b257 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -327,8 +327,17 @@ int phy_bulk_get_optional(struct device *dev, unsigned int num_phys,
 			  struct phy_bulk_data *phys);
 int of_phy_bulk_get(struct device_node *np, unsigned int num_phys,
 		    struct phy_bulk_data *phys);
+int devm_phy_bulk_get(struct device *dev, unsigned int num_phys,
+		      struct phy_bulk_data *phys);
+int devm_phy_bulk_get_optional(struct device *dev, unsigned int num_phys,
+			       struct phy_bulk_data *phys);
+int devm_of_phy_bulk_get(struct device *dev, struct device_node *np,
+			 unsigned int num_phys, struct phy_bulk_data *phys);
 int phy_bulk_get_all(struct device *dev, struct phy_bulk_data **phys);
 int of_phy_bulk_get_all(struct device_node *np, struct phy_bulk_data **phys);
+int devm_phy_bulk_get_all(struct device *dev, struct phy_bulk_data **phys);
+int devm_of_phy_bulk_get_all(struct device *dev, struct device_node *np,
+			     struct phy_bulk_data **phys);
 void phy_bulk_put(struct device *dev, unsigned int num_phys,
 		  struct phy_bulk_data *phys);
 void of_phy_bulk_put(unsigned int num_phys, struct phy_bulk_data *phys);
@@ -549,6 +558,27 @@ static inline int of_phy_bulk_get(struct device_node *np, unsigned int num_phys,
 	return -EOPNOTSUPP;
 }
 
+static inline int devm_phy_bulk_get(struct device *dev, unsigned int num_phys,
+				    struct phy_bulk_data *phys)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int devm_phy_bulk_get_optional(struct device *dev,
+					     unsigned int num_phys,
+					     struct phy_bulk_data *phys)
+{
+	return phy_bulk_get_optional(dev, num_phys, phys);
+}
+
+static inline int devm_of_phy_bulk_get(struct device *dev,
+				       struct device_node *np,
+				       unsigned int num_phys,
+				       struct phy_bulk_data *phys)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int phy_bulk_get_all(struct device *dev,
 				   struct phy_bulk_data **phys)
 {
@@ -567,6 +597,19 @@ static inline int of_phy_bulk_get_all(struct device_node *np,
 	return -EOPNOTSUPP;
 }
 
+static inline int devm_phy_bulk_get_all(struct device *dev,
+					struct phy_bulk_data **phys)
+{
+	return phy_bulk_get_all(dev, phys);
+}
+
+static inline int devm_of_phy_bulk_get_all(struct device *dev,
+					   struct device_node *np,
+					   struct phy_bulk_data **phys)
+{
+	return of_phy_bulk_get_all(np, phys);
+}
+
 static inline void phy_bulk_put(struct device *dev, unsigned int num_phys,
 				struct phy_bulk_data *phys)
 {
-- 
2.55.0

Re: [PATCH v2 0/4] phy: core: Add phy bulk helpers support

[Vladimir Oltean]

Hi Inochi,

On Fri, Sep 04, 2026 at 04:37:04PM +0800, Inochi Amaoto wrote:
> As discussed in [1], some drivers may do not care the difference between
> phys. Instead, they only need to treat them as a group and operate them
> together. This means a bulk operation is needed.
> 
> Add some bulk helper functions for phy core by referencing the design of
> clock/reset subsystem. This can relieve the driver owners' life who needs
> to handle many phys, as well as each phy error reporting.
> 
> [1] https://lore.kernel.org/linux-pci/ak9KzNFF26B0Kttz@ashevche-desk.local/
> 
> Changed from v1:
> - https://lore.kernel.org/linux-phy/20260831025319.94886-1-inochiama@gmail.com/
> patch 1:
> 1. Update document.
> patch 2:
> 1. Fix document format.
> 2. Fix several document description.
> patch 3:
> 1. Split into two patches: one for non devm helpers, one for devm helpers.
> 2. All the bulk helper now use unsigned int for counts.
> 3. Fix multiple the while loop statement.
> 4. Rename of_phy_get_parent_count() to of_phy_get_count().
> 5. Fix some 80 line function problems.
> 6. Use PTR_ERR_OR_ZERO() to simplify the error handle.
> 7. Fix zero value in the document
> 8. Use two separate release function for devm helpers.
> 
> Inochi Amaoto (4):
>   phy: core: Add common helper to add phy phandle device link
>   phy: core: Add common helper for get phy phandle by index
>   phy: core: Add phy bulk data helper functions
>   phy: core: Add managed phy bulk data helper functions
> 
>  drivers/phy/phy-core.c  | 700 +++++++++++++++++++++++++++++++++++++---
>  include/linux/phy/phy.h | 186 +++++++++++
>  2 files changed, 850 insertions(+), 36 deletions(-)
> 
> --
> 2.55.0

Please include a user of this API in the next patch submission.
We should avoid, as much as possible, to introduce unused code.

Also, please address Sashiko's feedback (excluding the pre-existing
issues).

Re: [PATCH v2 0/4] phy: core: Add phy bulk helpers support

[Inochi Amaoto]

On Mon, Sep 07, 2026 at 01:28:49PM +0300, Vladimir Oltean wrote:
> Hi Inochi,
> 
> On Fri, Sep 04, 2026 at 04:37:04PM +0800, Inochi Amaoto wrote:
> > As discussed in [1], some drivers may do not care the difference between
> > phys. Instead, they only need to treat them as a group and operate them
> > together. This means a bulk operation is needed.
> > 
> > Add some bulk helper functions for phy core by referencing the design of
> > clock/reset subsystem. This can relieve the driver owners' life who needs
> > to handle many phys, as well as each phy error reporting.
> > 
> > [1] https://lore.kernel.org/linux-pci/ak9KzNFF26B0Kttz@ashevche-desk.local/
> > 
> > Changed from v1:
> > - https://lore.kernel.org/linux-phy/20260831025319.94886-1-inochiama@gmail.com/
> > patch 1:
> > 1. Update document.
> > patch 2:
> > 1. Fix document format.
> > 2. Fix several document description.
> > patch 3:
> > 1. Split into two patches: one for non devm helpers, one for devm helpers.
> > 2. All the bulk helper now use unsigned int for counts.
> > 3. Fix multiple the while loop statement.
> > 4. Rename of_phy_get_parent_count() to of_phy_get_count().
> > 5. Fix some 80 line function problems.
> > 6. Use PTR_ERR_OR_ZERO() to simplify the error handle.
> > 7. Fix zero value in the document
> > 8. Use two separate release function for devm helpers.
> > 
> > Inochi Amaoto (4):
> >   phy: core: Add common helper to add phy phandle device link
> >   phy: core: Add common helper for get phy phandle by index
> >   phy: core: Add phy bulk data helper functions
> >   phy: core: Add managed phy bulk data helper functions
> > 
> >  drivers/phy/phy-core.c  | 700 +++++++++++++++++++++++++++++++++++++---
> >  include/linux/phy/phy.h | 186 +++++++++++
> >  2 files changed, 850 insertions(+), 36 deletions(-)
> > 
> > --
> > 2.55.0
> 
> Please include a user of this API in the next patch submission.
> We should avoid, as much as possible, to introduce unused code.
> 

Yes, I have one for it. This is the spacemit pcie driver.
https://lore.kernel.org/linux-pci/20260907112606.465778-1-inochiama@gmail.com/

But it only uses a few of these helpers. So should I remove all the
things that are not directly used, and let others add these if they
need?

> Also, please address Sashiko's feedback (excluding the pre-existing
> issues).
> 

Yes, I will, thanks.

Regards,
Inochi

Re: [PATCH v2 0/4] phy: core: Add phy bulk helpers support

[Vladimir Oltean]

On Mon, Sep 07, 2026 at 07:29:49PM +0800, Inochi Amaoto wrote:
> Yes, I have one for it. This is the spacemit pcie driver.
> https://lore.kernel.org/linux-pci/20260907112606.465778-1-inochiama@gmail.com/
> 
> But it only uses a few of these helpers. So should I remove all the
> things that are not directly used, and let others add these if they
> need?

How many functions would go away, and how difficult would they be to
reconstruct, if we only were to add them on an "as-needed" basis?

If the answer is "many", I suppose you could also leave a note in
phy.rst that links to v2 and mentions unimplemented functions because
they lack a user.

Re: [PATCH v2 0/4] phy: core: Add phy bulk helpers support

[Inochi Amaoto]

On Mon, Sep 07, 2026 at 02:57:12PM +0300, Vladimir Oltean wrote:
> On Mon, Sep 07, 2026 at 07:29:49PM +0800, Inochi Amaoto wrote:
> > Yes, I have one for it. This is the spacemit pcie driver.
> > https://lore.kernel.org/linux-pci/20260907112606.465778-1-inochiama@gmail.com/
> > 
> > But it only uses a few of these helpers. So should I remove all the
> > things that are not directly used, and let others add these if they
> > need?
> 
> How many functions would go away, and how difficult would they be to
> reconstruct, if we only were to add them on an "as-needed" basis?
> 
> If the answer is "many", I suppose you could also leave a note in
> phy.rst that links to v2 and mentions unimplemented functions because
> they lack a user.
> 

In fact I think it is happy. This PCIe driver only use three APIs:
- devm_of_phy_bulk_get_all
- phy_bulk_init
- phy_bulk_exit

So most of them will be removed as only a few are on the path.
I will follow you suggestion. Thanks.

Regards,
Inochi

Re: [PATCH v2 0/4] phy: core: Add phy bulk helpers support

[Vladimir Oltean]

On Mon, Sep 07, 2026 at 08:05:57PM +0800, Inochi Amaoto wrote:
> On Mon, Sep 07, 2026 at 02:57:12PM +0300, Vladimir Oltean wrote:
> > On Mon, Sep 07, 2026 at 07:29:49PM +0800, Inochi Amaoto wrote:
> > > Yes, I have one for it. This is the spacemit pcie driver.
> > > https://lore.kernel.org/linux-pci/20260907112606.465778-1-inochiama@gmail.com/
> > > 
> > > But it only uses a few of these helpers. So should I remove all the
> > > things that are not directly used, and let others add these if they
> > > need?
> > 
> > How many functions would go away, and how difficult would they be to
> > reconstruct, if we only were to add them on an "as-needed" basis?
> > 
> > If the answer is "many", I suppose you could also leave a note in
> > phy.rst that links to v2 and mentions unimplemented functions because
> > they lack a user.
> > 
> 
> In fact I think it is happy. This PCIe driver only use three APIs:
> - devm_of_phy_bulk_get_all
> - phy_bulk_init
> - phy_bulk_exit
> 
> So most of them will be removed as only a few are on the path.
> I will follow you suggestion. Thanks.
> 
> Regards,
> Inochi

I think you should call phy_bulk_power_on()/phy_bulk_power_off() as
well from the PCI controller driver, even if the PHY provider driver
doesn't (currently) implement them. The Generic PHY API is about
offering some degree of mutual consumer<->provider intercompatibility,
not just a pretentious indirect function call k1_pcie_init() ->
k1_pcie_phy_init().

So those 2 functions should also make it to your v3 patch set.

BTW, do you need a stable tag to be provided by linux-phy, to continue
your work on PCI in this development cycle? You should mention this in
your cover letter, because it does require a bit of maintainer
preparation beforehand.

Re: [PATCH v2 0/4] phy: core: Add phy bulk helpers support

[Vladimir Oltean]

On Mon, Sep 07, 2026 at 03:16:53PM +0300, Vladimir Oltean wrote:
> On Mon, Sep 07, 2026 at 08:05:57PM +0800, Inochi Amaoto wrote:
> > On Mon, Sep 07, 2026 at 02:57:12PM +0300, Vladimir Oltean wrote:
> > > On Mon, Sep 07, 2026 at 07:29:49PM +0800, Inochi Amaoto wrote:
> > > > Yes, I have one for it. This is the spacemit pcie driver.
> > > > https://lore.kernel.org/linux-pci/20260907112606.465778-1-inochiama@gmail.com/
> > > > 
> > > > But it only uses a few of these helpers. So should I remove all the
> > > > things that are not directly used, and let others add these if they
> > > > need?
> > > 
> > > How many functions would go away, and how difficult would they be to
> > > reconstruct, if we only were to add them on an "as-needed" basis?
> > > 
> > > If the answer is "many", I suppose you could also leave a note in
> > > phy.rst that links to v2 and mentions unimplemented functions because
> > > they lack a user.
> > > 
> > 
> > In fact I think it is happy. This PCIe driver only use three APIs:
> > - devm_of_phy_bulk_get_all
> > - phy_bulk_init
> > - phy_bulk_exit
> > 
> > So most of them will be removed as only a few are on the path.
> > I will follow you suggestion. Thanks.
> > 
> > Regards,
> > Inochi
> 
> I think you should call phy_bulk_power_on()/phy_bulk_power_off() as
> well from the PCI controller driver, even if the PHY provider driver
> doesn't (currently) implement them. The Generic PHY API is about
> offering some degree of mutual consumer<->provider intercompatibility,
> not just a pretentious indirect function call k1_pcie_init() ->
> k1_pcie_phy_init().
> 
> So those 2 functions should also make it to your v3 patch set.
> 
> BTW, do you need a stable tag to be provided by linux-phy, to continue
> your work on PCI in this development cycle? You should mention this in
> your cover letter, because it does require a bit of maintainer
> preparation beforehand.

Since we were talking about drivers/ata/libahci_platform.c on the other
patch - you could consider also converting this to the PHY bulk API and
this would give you some more coverage so that you don't remove unused
functions from your submission.

Re: [PATCH v2 0/4] phy: core: Add phy bulk helpers support

[Inochi Amaoto]

On Mon, Sep 07, 2026 at 03:16:53PM +0300, Vladimir Oltean wrote:
> On Mon, Sep 07, 2026 at 08:05:57PM +0800, Inochi Amaoto wrote:
> > On Mon, Sep 07, 2026 at 02:57:12PM +0300, Vladimir Oltean wrote:
> > > On Mon, Sep 07, 2026 at 07:29:49PM +0800, Inochi Amaoto wrote:
> > > > Yes, I have one for it. This is the spacemit pcie driver.
> > > > https://lore.kernel.org/linux-pci/20260907112606.465778-1-inochiama@gmail.com/
> > > > 
> > > > But it only uses a few of these helpers. So should I remove all the
> > > > things that are not directly used, and let others add these if they
> > > > need?
> > > 
> > > How many functions would go away, and how difficult would they be to
> > > reconstruct, if we only were to add them on an "as-needed" basis?
> > > 
> > > If the answer is "many", I suppose you could also leave a note in
> > > phy.rst that links to v2 and mentions unimplemented functions because
> > > they lack a user.
> > > 
> > 
> > In fact I think it is happy. This PCIe driver only use three APIs:
> > - devm_of_phy_bulk_get_all
> > - phy_bulk_init
> > - phy_bulk_exit
> > 
> > So most of them will be removed as only a few are on the path.
> > I will follow you suggestion. Thanks.
> > 
> > Regards,
> > Inochi
> 
> I think you should call phy_bulk_power_on()/phy_bulk_power_off() as
> well from the PCI controller driver, even if the PHY provider driver
> doesn't (currently) implement them. The Generic PHY API is about
> offering some degree of mutual consumer<->provider intercompatibility,
> not just a pretentious indirect function call k1_pcie_init() ->
> k1_pcie_phy_init().
> 
> So those 2 functions should also make it to your v3 patch set.
> 

Good. That's the point I have missed. I will add them, thanks.

> BTW, do you need a stable tag to be provided by linux-phy, to continue
> your work on PCI in this development cycle? You should mention this in
> your cover letter, because it does require a bit of maintainer
> preparation beforehand.
> 

I think I need. The most problem of PCI driver is already fixed so I
think there is a chance for it to get merged.

Regards,
Inochi