From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jagan Teki <jagan@amarulasolutions.com>
Date: Wed, 29 Apr 2020 01:15:35 +0530
Subject: [PATCH v6 03/14] phy: Add get/enable/disable for a bulk of phys
In-Reply-To: <1587953814.8249.15.camel@mhfsdcap03>
References: <1587352883-8641-1-git-send-email-chunfeng.yun@mediatek.com>
 <1587352883-8641-4-git-send-email-chunfeng.yun@mediatek.com>
 <CAMty3ZDU0Ze3W4gQc_z8-C16jNZ1B1nXOYUb_8WmqfargBF9RQ@mail.gmail.com>
 <1587953814.8249.15.camel@mhfsdcap03>
Message-ID: <CAMty3ZC-1eUK4u=pvbhVhAn+T9NsyPjw5ZO2EUOhG1=rgA-_nQ@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Mon, Apr 27, 2020 at 7:47 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Sat, 2020-04-25 at 18:58 +0530, Jagan Teki wrote:
> > On Mon, Apr 20, 2020 at 8:52 AM Chunfeng Yun <chunfeng.yun@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.yun@mediatek.com>
> > > Reviewed-by: Weijie Gao <weijie.gao@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?

I think the best way is to create bulk init, bulk power on separately
and use them with your bulk enable. This way it would satisfy both the
use cases.

Jagan.