Hi Patrick, On Wed, 23 Sep 2020 at 09:06, Patrick DELAUNAY <patrick.delau...@st.com> wrote: > > Hi Simon, > > > From: Simon Glass <s...@chromium.org> > > Sent: mardi 22 septembre 2020 20:49 > > > > On Thu, 10 Sep 2020 at 10:44, Patrick Delaunay <patrick.delau...@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.delau...@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 <s...@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