Re: [PATCH] rtc: abx80x: Provide feedback for invalid dt properties

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

On 29/05/2020 06:28:56-0400, Kevin P. Fleming wrote:
> On Fri, May 29, 2020 at 4:54 AM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> > > -static int abx80x_dt_trickle_cfg(struct device_node *np)
> > > +static int abx80x_dt_trickle_cfg(struct i2c_client *client,
> > > +                              struct device_node *np)
> >
> > I would remove np from the parameters and use
> > struct device_node *np = client->dev.of_node;
> > in the function.
> 
> Will do.
> 
> > > +             dev_err(&client->dev, "Invalid tc-diode value: %s\n", diode);
> >
> > Can you make that dev_dbg? This is only ever needed at board bring up/
> > development time, so it is not necessary to bloat the kernel with more
> > strings.
> 
> I'm using this driver via the Raspberry Pi device tree 'overlay'
> mechanism, so I'm setting these parameters in a configuration file and
> they are applied by the board's firmware before the kernel is booted.
> As a result this is essentially 'runtime' configuration, it's not a
> static device tree for the board, so end users like me could run into
> this problem.
> 

This is still board bringup, once it is correct, you will never need the
message anymore. If the issue is device tree validation (i.e. typo in
the string), then maybe you should convert the doc to yaml so you device
tree and overlay could be checked.


> I'd be fine with changing it to dev_info though, and indicating that
> the trickle charger won't be enabled in addition.

dev_info is worse, it is still bloating the kernel and also always
printing in the kernel logs.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com