Re: [PATCH] rtc: rv3028: Improve trickle charger logic

Alexandre Belloni
2023-06-08T09:32:33Z

Hello,

On 08/06/2023 11:04:46+0200, Andrej Picej wrote:
> + ret = regmap_read(rv3028->regmap, RV3028_BACKUP, &val_old);
> + if (ret < 0)
> + return ret;
> +
> + /* mask out only trickle charger bits */
> + val_old = val_old & (RV3028_BACKUP_TCE | RV3028_BACKUP_TCR_MASK);
> +
> /* setup trickle charger */
> - if (!device_property_read_u32(&client->dev, "trickle-resistor-ohms",
> - &ohms)) {
> + if (device_property_read_u32(&client->dev, "trickle-resistor-ohms", &ohms)) {
> + /* disable the trickle charger */
> + val = 0;

You can't do that, this will break existing users that may set the
trickle charger from their bootloader for example.

> + } else {
> int i;
>
> for (i = 0; i < ARRAY_SIZE(rv3028_trickle_resistors); i++)
> @@ -947,15 +957,21 @@ static int rv3028_probe(struct i2c_client *client)
> break;
>
> if (i < ARRAY_SIZE(rv3028_trickle_resistors)) {
> - ret = rv3028_update_cfg(rv3028, RV3028_BACKUP, RV3028_BACKUP_TCE |
> - RV3028_BACKUP_TCR_MASK, RV3028_BACKUP_TCE | i);
> - if (ret)
> - return ret;
> + /* enable the trickle charger and setup its resistor accordingly */
> + val = RV3028_BACKUP_TCE | i;
> } else {
> dev_warn(&client->dev, "invalid trickle resistor value\n");
> }
> }
>
> + /* only update EEPROM if changes are necessary */
> + if (val_old != val) {
> + ret = rv3028_update_cfg(rv3028, RV3028_BACKUP, RV3028_BACKUP_TCE |
> + RV3028_BACKUP_TCR_MASK, val);
> + if (ret)
> + return ret;
> + }
> +
> ret = rtc_add_group(rv3028->rtc, &rv3028_attr_group);
> if (ret)
> return ret;
> --
> 2.25.1
>

--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering