Hi,
On 28/05/2020 20:12:03-0400, Kevin P. Fleming wrote:
> When the user provides an invalid value for tc-diode or
> tc-resistor generate an error message instead of silently
> ignoring it.
>
> Signed-off-by: Kevin P. Fleming <kevin+linux@xxxxxxx>
> ---
> drivers/rtc/rtc-abx80x.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index 3521d8e8dc38..dae046e3484a 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
> @@ -554,7 +554,8 @@ static const struct rtc_class_ops abx80x_rtc_ops = {
> .ioctl = abx80x_ioctl,
> };
>
> -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.
> {
> const char *diode;
> int trickle_cfg = 0;
> @@ -565,12 +566,14 @@ static int abx80x_dt_trickle_cfg(struct device_node *np)
> if (ret)
> return ret;
>
> - if (!strcmp(diode, "standard"))
> + if (!strcmp(diode, "standard")) {
> trickle_cfg |= ABX8XX_TRICKLE_STANDARD_DIODE;
> - else if (!strcmp(diode, "schottky"))
> + } else if (!strcmp(diode, "schottky")) {
> trickle_cfg |= ABX8XX_TRICKLE_SCHOTTKY_DIODE;
> - else
> + } else {
> + 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.
> return -EINVAL;
> + }
>
> ret = of_property_read_u32(np, "abracon,tc-resistor", &tmp);
> if (ret)
> @@ -580,8 +583,10 @@ static int abx80x_dt_trickle_cfg(struct device_node *np)
> if (trickle_resistors[i] == tmp)
> break;
>
> - if (i == sizeof(trickle_resistors))
> + if (i == sizeof(trickle_resistors)) {
> + dev_err(&client->dev, "Invalid tc-resistor value: %u\n", tmp);
> return -EINVAL;
> + }
>
> return (trickle_cfg | i);
> }
> @@ -793,7 +798,7 @@ static int abx80x_probe(struct i2c_client *client,
> }
>
> if (np && abx80x_caps[part].has_tc)
> - trickle_cfg = abx80x_dt_trickle_cfg(np);
> + trickle_cfg = abx80x_dt_trickle_cfg(client, np);
>
> if (trickle_cfg > 0) {
> dev_info(&client->dev, "Enabling trickle charger: %02x\n",
> --
> 2.26.2
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
