Re: [PATCH v4 3/5] phy: core: Add phy bulk data helper functions

[Inochi Amaoto <inochiama@gmail.com>]

On Mon, Oct 05, 2026 at 12:12:51PM +0200, Vinod Koul wrote:
> On 29-09-26, 16:52, Inochi Amaoto wrote:
> > 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.
> 
> Do we have many such examples? Phy is not a many resource like
> clock/regulators... Do we really need this. How many in kernel users
> will benefit from this API?
> 


I guess the following driver may benefit from bulk api. Most of them
are PCIe drivers....

drivers/ata/ahci_imx.c
drivers/pci/controller/cadence/pcie-cadence.c
drivers/pci/controller/dwc/pci-dra7xx.c
drivers/pci/controller/dwc/pcie-tegra194.c
drivers/pci/controller/pcie-rockchip.c
drivers/pci/controller/dwc/pcie-qcom.c

Note: I mean the bulk api, not just the things I added. I guess many
may want devm_phy_bulk_get() or devm_of_phy_bulk_get() instead of
devm_phy_bulk_get_all().

Regards,
Inochi

> > 
> > 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  | 253 ++++++++++++++++++++++++++++++++++++++++
> >  include/linux/phy/phy.h |  55 +++++++++
> >  2 files changed, 308 insertions(+)
> > 
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 8fbaea003b90..5b76c006575b 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -1004,6 +1004,259 @@ 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
> > + * @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.
> > + */
> > +static 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;
> > +	}
> > +}
> > +
> > +/**
> > + * of_phy_bulk_put() - release a set of 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.
> > + */
> > +static 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;
> > +	}
> > +}
> > +
> > +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.
> > + *
> > + * Return: the number of PHYs on success, %0 if no PHYs are found, or a
> > + * negative error code otherwise
> > + */
> > +static 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;
> > +}
> > +
> > +/**
> > + * phy_bulk_put_all() - release and free PHYs
> > + * @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
> > + */
> > +static 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);
> > +}
> > +
> > +/**
> > + * 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..77d9d74682b7 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -81,6 +81,18 @@ union phy_configure_opts {
> >  	struct phy_configure_opts_hdmi		hdmi;
> >  };
> >  
> > +/**
> > + * struct phy_bulk_data - Data used for bulk phy operations.
> > + *
> > + * @id: phy consumer ID
> > + */
> > +struct phy_bulk_data {
> > +	const char	*id;
> > +
> > +	/* private: Internal use */
> > +	struct phy	*phy;
> > +};
> > +
> >  /**
> >   * struct phy_ops - set of function pointers for performing phy operations
> >   * @init: operation to be performed for initializing phy
> > @@ -309,6 +321,12 @@ 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_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 +511,43 @@ static inline struct phy *devm_of_phy_get_by_index(struct device *dev,
> >  	return ERR_PTR(-ENOSYS);
> >  }
> >  
> > +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
> 
> -- 
> ~Vinod