On 15/12/2021 06:26, Simon Glass wrote: > On Sat, 11 Dec 2021 at 02:02, Alper Nebi Yasak <alpernebiya...@gmail.com> > wrote: >> diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c >> index 59683a080cd7..48b4b24db674 100644 >> --- a/drivers/phy/phy-uclass.c >> +++ b/drivers/phy/phy-uclass.c >> @@ -11,12 +11,65 @@ >> #include <dm/device_compat.h> >> #include <dm/devres.h> >> #include <generic-phy.h> >> +#include <linux/list.h> >> + >> +struct phy_id_priv { >> + unsigned long id; >> + int power_on_count; >> + int init_count; >> + struct list_head list; >> +}; >> >> static inline struct phy_ops *phy_dev_ops(struct udevice *dev) >> { >> return (struct phy_ops *)dev->driver->ops; >> } >> >> +static struct phy_id_priv *phy_get_uclass_priv(struct phy *phy) > > This name is very confusing as it is similar to one used by DM, but > does something different.
What I thought is: if we replaced these "struct phy" handles with child udevices, the counts would be their uclass_priv, and this function would be just dev_get_uclass_priv() on those child udevices. (Or maybe it would be parent_priv?). So I tried to pick names similar to those. > Do you think this function should be split into something that adds > the data and something that reads it? I can split and rename things into: - struct phy_counts - phy_alloc_counts(struct phy *phy) - phy_get_counts(struct phy *phy) >> +{ >> + struct list_head *uc_priv; >> + struct phy_id_priv *id_priv; >> + >> + if (!generic_phy_valid(phy)) >> + return NULL; >> + >> + uc_priv = dev_get_uclass_priv(phy->dev); >> + list_for_each_entry(id_priv, uc_priv, list) >> + if (id_priv->id == phy->id) >> + return id_priv; >> + >> + id_priv = kzalloc(sizeof(*id_priv), GFP_KERNEL); >> + if (!id_priv) >> + return NULL; >> + >> + id_priv->id = phy->id; >> + id_priv->power_on_count = 0; >> + id_priv->init_count = 0; >> + list_add(&id_priv->list, uc_priv); >> + >> + return id_priv; >> +}