On Sat, 2020-04-25 at 18:58 +0530, Jagan Teki wrote:
> On Mon, Apr 20, 2020 at 8:52 AM Chunfeng Yun <chunfeng....@mediatek.com>
> wrote:
> >
> > This patch adds a "bulk" API to the phy API in order to
> > get/enable/disable a group of phys associated with a device.
> >
> > The bulk API will avoid adding a copy of the same code to
> > manage a group of phys in drivers.
> >
> > Signed-off-by: Chunfeng Yun <chunfeng....@mediatek.com>
> > Reviewed-by: Weijie Gao <weijie....@mediatek.com>
> > ---
> > v6: add Reviewed-by Weijie
> >
> > v5: no changes
> >
> > v4: new patch
> > ---
> > drivers/phy/phy-uclass.c | 80 ++++++++++++++++++++++++++++++++++++++++
> > include/generic-phy.h | 66 +++++++++++++++++++++++++++++++++
> > 2 files changed, 146 insertions(+)
> >
> > diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
> > index e201a90c8c..62580520ad 100644
> > --- a/drivers/phy/phy-uclass.c
> > +++ b/drivers/phy/phy-uclass.c
> > @@ -6,6 +6,7 @@
> >
> > #include <common.h>
> > #include <dm.h>
> > +#include <dm/devres.h>
> > #include <generic-phy.h>
> >
> > static inline struct phy_ops *phy_dev_ops(struct udevice *dev)
> > @@ -161,6 +162,85 @@ int generic_phy_power_off(struct phy *phy)
> > return ops->power_off ? ops->power_off(phy) : 0;
> > }
> >
> > +int generic_phy_get_bulk(struct udevice *dev, struct phy_bulk *bulk)
> > +{
> > + int i, ret, count;
> > +
> > + bulk->count = 0;
> > +
> > + /* Return if no phy declared */
> > + if (!dev_read_prop(dev, "phys", NULL))
> > + return 0;
> > +
> > + count = dev_count_phandle_with_args(dev, "phys", "#phy-cells");
> > + if (count < 1)
> > + return count;
> > +
> > + bulk->phys = devm_kcalloc(dev, count, sizeof(struct phy),
> > GFP_KERNEL);
> > + if (!bulk->phys)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < count; i++) {
> > + ret = generic_phy_get_by_index(dev, i, &bulk->phys[i]);
> > + if (ret) {
> > + pr_err("Failed to get PHY%d for %s\n", i,
> > dev->name);
> > + return ret;
> > + }
> > + bulk->count++;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int generic_phy_enable_bulk(struct phy_bulk *bulk)
> > +{
> > + struct phy *phys = bulk->phys;
> > + int count = bulk->count;
> > + int i, ret;
> > +
> > + for (i = 0; i < count; i++) {
> > + ret = generic_phy_init(&phys[i]);
> > + if (ret) {
> > + pr_err("Can't init PHY%d\n", i);
> > + goto phys_init_err;
> > + }
> > + }
> > +
> > + for (i = 0; i < count; i++) {
> > + ret = generic_phy_power_on(&phys[i]);
> > + if (ret) {
> > + pr_err("Can't power PHY%d\n", i);
> > + goto phys_poweron_err;
> > + }
> > + }
>
> Better to have bulk init, bulk power on separately since not all phy
> consumers will init, power on sequentially.
Yes, It's will be flexible when provide bulk init/power-on, but
currently a bulk-enable API is simpler way due to all cases use
multi-phys do initialization and power-on sequentially, how about
separating it until we really need it?
>
> > +
> > + return 0;
> > +
> > +phys_poweron_err:
> > + for (; i > 0; i--)
> > + generic_phy_power_off(&phys[i - 1]);
> > +
> > + i = count;
> > +phys_init_err:
> > + for (; i > 0; i--)
> > + generic_phy_exit(&phys[i - 1]);
> > +
> > + return ret;
> > +}
> > +
> > +int generic_phy_disable_bulk(struct phy_bulk *bulk)
> > +{
> > + struct phy *phys = bulk->phys;
> > + int i, ret = 0;
> > +
> > + for (i = 0; i < bulk->count; i++) {
> > + ret |= generic_phy_power_off(&phys[i]);
> > + ret |= generic_phy_exit(&phys[i]);
> > + }
>
> Same, like above.
>
> Jagan.
