[RFC PATCH] dm: add cells_count parameter in *_count_phandle_with_args

[Simon Glass]

Hi Patrick,

On Wed, 23 Sep 2020 at 09:06, Patrick DELAUNAY <patrick.delaunay at st.com> wrote:
>
> Hi Simon,
>
> > From: Simon Glass <sjg at chromium.org>
> > Sent: mardi 22 septembre 2020 20:49
> >
> > On Thu, 10 Sep 2020 at 10:44, Patrick Delaunay <patrick.delaunay at st.com>
> > wrote:
> > >
> > > The cell_count argument is required when cells_name is NULL.
> > >
> > > This patch adds this parameter in live tree API
> > > - of_count_phandle_with_args
> > > - ofnode_count_phandle_with_args
> > > - dev_count_phandle_with_args
> > >
> > > This parameter solves issue when these API is used to count the number
> > > of element of a cell without cell name. This parameter allow to force
> > > the size cell.
> > >
> > > For example:
> > >   count = dev_count_phandle_with_args(dev, "array", NULL, 3);
> > >
> > > Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> > > ---
> > > I push today this RFC.
> > >
> > > It is linked to previous serie [1] but it is not a blocking point
> > > today as no user use this API with cells_name = NULL
> > > + dev_count_phandle_with_args
> > > + ofnode_count_phandle_with_args
> > >
> > > But I think it is the good time to modify these functions as they are
> > > not hugely used: it is the proposition in this RFC.
> > >
> > > It is just a RFC because I don't sure if I can modify the existing API
> > > even if parameters are aligned with *_parse_phandle_with_args.
> > >
> > > I can also to add new APIs to use when cells_name is NULL:
> > > + dev_count_phandle_with_cell_count(node, list_name, cell_count);
> > > + ofnode_count_phandle_with_cell_count(node, list_name, cell_count);
> > >
> > > and raise a error if cells_name == NULL in existing function
> > > + dev_count_phandle_with_args
> > > + ofnode_count_phandle_with_args
> > >
> > > [1] http://patchwork.ozlabs.org/project/uboot/list/?series=200899
> > >     "dm: add cells_count parameter in live DT APIs of_parse_phandle_with_args"
> > >
> > >
> > >  board/st/stm32mp1/stm32mp1.c    | 2 +-
> > >  drivers/clk/clk-uclass.c        | 4 ++--
> > >  drivers/core/of_access.c        | 7 ++++---
> > >  drivers/core/ofnode.c           | 6 +++---
> > >  drivers/core/read.c             | 5 +++--
> > >  drivers/phy/phy-uclass.c        | 2 +-
> > >  drivers/reset/reset-uclass.c    | 2 +-
> > >  drivers/usb/host/ehci-generic.c | 4 ++--
> > >  include/dm/of_access.h          | 4 +++-
> > >  include/dm/ofnode.h             | 3 ++-
> > >  include/dm/read.h               | 8 +++++---
> > >  11 files changed, 27 insertions(+), 20 deletions(-)'
> >
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> >
> > A test would go a long way here.
>
> Sure, I will add a test in the real patch...
>
> I send RFC without test just to be sure that adding parameter in  *_count_phandle_with_args()
> is a better solution than adding a new API.
>
> Proposition 1 (it is the RFC content): add argument in current API

I think that is best. It's only one more parameter onto a call that
already has lots of parameters. It reduced the number of separate
functions.


>
> Example:
>
> of_count_phandle_with_args(node, "clocks", "#clock-cells", 0);
> ofnode_count_phandle_with_args(node, "resets", "#reset-cells", 0);
> dev_count_phandle_with_args(node, "phys", "#phy-cells", 0);
>
> dev_count_phandle_with_args(node, "test", NULL, 3);
> ofnode_count_phandle_with_args(node, "test", NULL, 3);
>
>
> Proposition 2: new API *count_phandle_with_cell_count
>
> of_count_phandle_with_args(node, "clocks", "#clock-cells");
> ofnode_count_phandle_with_args(node, "resets", "#reset-cells");
> dev_count_phandle_with_args(node, "phys", "#phy-cells");
>
> dev_count_phandle_with_fixed_args(node, "test", 3);
> ofnode_count_phandle_with_fixed_args(node, "test", 3);
>
> I think that Proposition 1 (this RFC) is more clear because parameters are aligned
> with other API *read_phandle_with_args
>
> But proposition 2 is align with Linux API
> - of_count_phandle_with_args
> - of_parse_phandle_with_fixed_args
> And avoid to change all the current users of *count_phandle_with_args
>
> Patrick
>

- Simon