I noticed that my earlier reply was rejected by some mail servers, resending from @kernel.org to make sure everyone has it.
On Tue, Oct 27, 2020 at 10:57 PM Arnd Bergmann <[email protected]> wrote: > > On Tue, Oct 27, 2020 at 10:11 PM Enric Balletbo i Serra > <[email protected]> wrote: > > > > This adds syscon_regmap_lookup_by_phandle_optional() function to get an > > optional regmap. > > > > It behaves the same as syscon_regmap_lookup_by_phandle() except where > > there is no regmap phandle. In this case, instead of returning -ENODEV, > > the function returns NULL. This makes error checking simpler when the > > regmap phandle is optional. > > > > Suggested-by: Nicolas Boichat <[email protected]> > > Signed-off-by: Enric Balletbo i Serra <[email protected]> > > Looks good in principle. > > > @@ -255,6 +255,19 @@ struct regmap > > *syscon_regmap_lookup_by_phandle_args(struct device_node *np, > > } > > EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_phandle_args); > > > > +struct regmap *syscon_regmap_lookup_by_phandle_optional(struct device_node > > *np, > > + const char *property) > > +{ > > + struct regmap *regmap; > > + > > + regmap = syscon_regmap_lookup_by_phandle(np, property); > > + if (IS_ERR(regmap) && PTR_ERR(regmap) == -ENODEV) > > + return NULL; > > + > > + return regmap; > > +} > > I think the explanation from the patch description is needed here as well, > as an interface that might either return an error code or NULL is generally > a really bad idea. I understand what this is for, but it's easy to misread. > > > static inline struct regmap *device_node_to_regmap(struct device_node *np) > > { > > @@ -59,6 +62,14 @@ static inline struct regmap > > *syscon_regmap_lookup_by_phandle_args( > > { > > return ERR_PTR(-ENOTSUPP); > > } > > + > > +static inline struct regmap *syscon_regmap_lookup_by_phandle_optional( > > + struct device_node *np, > > + const char *property) > > +{ > > + return ERR_PTR(-ENOTSUPP); > > +} > > Shouldn't this also return NULL then? > > Arnd
