| Message ID | 20210709142731.23418-3-bruno.meneguele@smartgreen.net |
|---|---|
| State | New |
| Headers | show |
| Series | add Watchdog Timer delay support for BQ24735 | expand |
Hi, On Fri, Jul 09, 2021 at 11:27:31AM -0300, Bruno Meneguele wrote: > The BQ24735 charger allows the user to set the watchdog timer delay between > two consecutives ChargeCurrent or ChargeVoltage command writes, if the IC > doesn't receive any command before the timeout happens, the charge is turned > off. > > This patch adds the support to the user to change the default/POR value with > four discrete values: > > 0 - disabled > 1 - enabled, 44 sec > 2 - enabled, 88 sec > 3 - enabled, 175 sec (default at POR) > > These are the options supported in the ChargeOptions register bits 13&14. > > Also, this patch make one additional check when poll-interval is set by the > user: if the interval set is greater than the WDT timeout it'll fail during > the probe stage, preventing the user to set non-compatible values between > the two options. > > Signed-off-by: Bruno Meneguele <bruno.meneguele@smartgreen.net> > --- > .../bindings/power/supply/bq24735.yaml | 13 +++++ Patches for the DT bindings needs to be CC'd to the DT binding maintainers and should be in their own patch. > drivers/power/supply/bq24735-charger.c | 48 +++++++++++++++++++ > include/linux/power/bq24735-charger.h | 1 + > 3 files changed, 62 insertions(+) > > diff --git a/Documentation/devicetree/bindings/power/supply/bq24735.yaml b/Documentation/devicetree/bindings/power/supply/bq24735.yaml > index 131be6782c4b..62399efab467 100644 > --- a/Documentation/devicetree/bindings/power/supply/bq24735.yaml > +++ b/Documentation/devicetree/bindings/power/supply/bq24735.yaml > @@ -56,6 +56,19 @@ properties: > The POR value is 0x1000h. This number is in mA (e.g. 8064). > See the spec for more information about the InputCurrent (0x3fh) register. > > + ti,wdt-timeout: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + Used to control and set the charger watchdog delay between consecutive > + charge voltage and charge current commands. > + This value must be: > + 0 - disabled > + 1 - 44 seconds > + 2 - 88 seconds > + 3 - 175 seconds > + The POR value is 0x11 (3). > + See the spec for more information about the ChargeOptions(0x12h) register. > + This is missing minimum: 0 maximum: 3 > ti,external-control: > type: boolean > description: | > diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c > index 3ce36d09c017..88f1cb1e9fee 100644 > --- a/drivers/power/supply/bq24735-charger.c > +++ b/drivers/power/supply/bq24735-charger.c > @@ -45,6 +45,8 @@ > /* ChargeOptions bits of interest */ > #define BQ24735_CHARGE_OPT_CHG_DISABLE (1 << 0) > #define BQ24735_CHARGE_OPT_AC_PRESENT (1 << 4) > +#define BQ24735_CHARGE_OPT_WDT_OFFSET 13 > +#define BQ24735_CHARGE_OPT_WDT (3 << BQ24735_CHARGE_OPT_WDT_OFFSET) > > struct bq24735 { > struct power_supply *charger; > @@ -156,6 +158,20 @@ static int bq24735_config_charger(struct bq24735 *charger) > } > } > > + if (pdata->wdt_timeout) { > + value = pdata->wdt_timeout; > + > + ret = bq24735_update_word(charger->client, BQ24735_CHARGE_OPT, > + BQ24735_CHARGE_OPT_WDT, > + (value << BQ24735_CHARGE_OPT_WDT_OFFSET)); > + if (ret < 0) { > + dev_err(&charger->client->dev, > + "Failed to write watchdog timer: %d\n", > + ret); > + return ret; > + } > + } binding says '0' = disabled, but code implements '0' = do not change. -- Sebastian > + > return 0; > } > > @@ -347,6 +363,17 @@ static struct bq24735_platform *bq24735_parse_dt_data(struct i2c_client *client) > if (!ret) > pdata->input_current = val; > > + ret = of_property_read_u32(np, "ti,wdt-timeout", &val); > + if (!ret) { > + if (val <= 3) { > + pdata->wdt_timeout = val; > + } else { > + dev_warn(&client->dev, > + "Invalid value for ti,wdt-timeout: %d", > + val); > + } > + } > + > pdata->ext_control = of_property_read_bool(np, "ti,external-control"); > > return pdata; > @@ -476,6 +503,27 @@ static int bq24735_charger_probe(struct i2c_client *client, > return 0; > if (!charger->poll_interval) > return 0; > + if (charger->pdata->wdt_timeout) { > + int wdt_ms; > + > + switch (charger->pdata->wdt_timeout) { > + case 1: > + wdt_ms = 44000; > + break; > + case 2: > + wdt_ms = 88000; > + break; > + case 3: > + wdt_ms = 175000; > + break; > + } > + > + if (charger->poll_interval > wdt_ms) { > + dev_err(&client->dev, > + "Poll interval greater than WDT timeout\n"); > + return -EINVAL; > + } > + } > > ret = devm_delayed_work_autocancel(&client->dev, &charger->poll, > bq24735_poll); > diff --git a/include/linux/power/bq24735-charger.h b/include/linux/power/bq24735-charger.h > index 321dd009ce66..ce5a030ca111 100644 > --- a/include/linux/power/bq24735-charger.h > +++ b/include/linux/power/bq24735-charger.h > @@ -12,6 +12,7 @@ struct bq24735_platform { > uint32_t charge_current; > uint32_t charge_voltage; > uint32_t input_current; > + uint32_t wdt_timeout; > > const char *name; > > -- > 2.31.1 >
On Fri, Aug 13, 2021 at 06:29:14PM +0200, Sebastian Reichel wrote: > Hi, > > On Fri, Jul 09, 2021 at 11:27:31AM -0300, Bruno Meneguele wrote: > > The BQ24735 charger allows the user to set the watchdog timer delay between > > two consecutives ChargeCurrent or ChargeVoltage command writes, if the IC > > doesn't receive any command before the timeout happens, the charge is turned > > off. > > > > This patch adds the support to the user to change the default/POR value with > > four discrete values: > > > > 0 - disabled > > 1 - enabled, 44 sec > > 2 - enabled, 88 sec > > 3 - enabled, 175 sec (default at POR) > > > > These are the options supported in the ChargeOptions register bits 13&14. > > > > Also, this patch make one additional check when poll-interval is set by the > > user: if the interval set is greater than the WDT timeout it'll fail during > > the probe stage, preventing the user to set non-compatible values between > > the two options. > > > > Signed-off-by: Bruno Meneguele <bruno.meneguele@smartgreen.net> > > --- > > .../bindings/power/supply/bq24735.yaml | 13 +++++ > > Patches for the DT bindings needs to be CC'd to the DT binding > maintainers and should be in their own patch. > Sorry about that, didn't know. Will do that in v4. > > drivers/power/supply/bq24735-charger.c | 48 +++++++++++++++++++ > > include/linux/power/bq24735-charger.h | 1 + > > 3 files changed, 62 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/power/supply/bq24735.yaml b/Documentation/devicetree/bindings/power/supply/bq24735.yaml > > index 131be6782c4b..62399efab467 100644 > > --- a/Documentation/devicetree/bindings/power/supply/bq24735.yaml > > +++ b/Documentation/devicetree/bindings/power/supply/bq24735.yaml > > @@ -56,6 +56,19 @@ properties: > > The POR value is 0x1000h. This number is in mA (e.g. 8064). > > See the spec for more information about the InputCurrent (0x3fh) register. > > > > + ti,wdt-timeout: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + Used to control and set the charger watchdog delay between consecutive > > + charge voltage and charge current commands. > > + This value must be: > > + 0 - disabled > > + 1 - 44 seconds > > + 2 - 88 seconds > > + 3 - 175 seconds > > + The POR value is 0x11 (3). > > + See the spec for more information about the ChargeOptions(0x12h) register. > > + > > This is missing > > minimum: 0 > maximum: 3 > Indeed. Will fix in v4. > > ti,external-control: > > type: boolean > > description: | > > diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c > > index 3ce36d09c017..88f1cb1e9fee 100644 > > --- a/drivers/power/supply/bq24735-charger.c > > +++ b/drivers/power/supply/bq24735-charger.c > > @@ -45,6 +45,8 @@ > > /* ChargeOptions bits of interest */ > > #define BQ24735_CHARGE_OPT_CHG_DISABLE (1 << 0) > > #define BQ24735_CHARGE_OPT_AC_PRESENT (1 << 4) > > +#define BQ24735_CHARGE_OPT_WDT_OFFSET 13 > > +#define BQ24735_CHARGE_OPT_WDT (3 << BQ24735_CHARGE_OPT_WDT_OFFSET) > > > > struct bq24735 { > > struct power_supply *charger; > > @@ -156,6 +158,20 @@ static int bq24735_config_charger(struct bq24735 *charger) > > } > > } > > > > + if (pdata->wdt_timeout) { > > + value = pdata->wdt_timeout; > > + > > + ret = bq24735_update_word(charger->client, BQ24735_CHARGE_OPT, > > + BQ24735_CHARGE_OPT_WDT, > > + (value << BQ24735_CHARGE_OPT_WDT_OFFSET)); > > + if (ret < 0) { > > + dev_err(&charger->client->dev, > > + "Failed to write watchdog timer: %d\n", > > + ret); > > + return ret; > > + } > > + } > > binding says '0' = disabled, but code implements '0' = do not > change. > Well noticed. Will fix in v4. I'm going to send a v4 without patch 1/2 because I noticed you already queued for-next, so the next version should have only 1/2 heandling the code and 2/2 specifically for dt-bindings. -- Bruno Meneguele PGP Key: http://bmeneg.com/pubkey.txt
diff --git a/Documentation/devicetree/bindings/power/supply/bq24735.yaml b/Documentation/devicetree/bindings/power/supply/bq24735.yaml index 131be6782c4b..62399efab467 100644 --- a/Documentation/devicetree/bindings/power/supply/bq24735.yaml +++ b/Documentation/devicetree/bindings/power/supply/bq24735.yaml @@ -56,6 +56,19 @@ properties: The POR value is 0x1000h. This number is in mA (e.g. 8064). See the spec for more information about the InputCurrent (0x3fh) register. + ti,wdt-timeout: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + Used to control and set the charger watchdog delay between consecutive + charge voltage and charge current commands. + This value must be: + 0 - disabled + 1 - 44 seconds + 2 - 88 seconds + 3 - 175 seconds + The POR value is 0x11 (3). + See the spec for more information about the ChargeOptions(0x12h) register. + ti,external-control: type: boolean description: | diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c index 3ce36d09c017..88f1cb1e9fee 100644 --- a/drivers/power/supply/bq24735-charger.c +++ b/drivers/power/supply/bq24735-charger.c @@ -45,6 +45,8 @@ /* ChargeOptions bits of interest */ #define BQ24735_CHARGE_OPT_CHG_DISABLE (1 << 0) #define BQ24735_CHARGE_OPT_AC_PRESENT (1 << 4) +#define BQ24735_CHARGE_OPT_WDT_OFFSET 13 +#define BQ24735_CHARGE_OPT_WDT (3 << BQ24735_CHARGE_OPT_WDT_OFFSET) struct bq24735 { struct power_supply *charger; @@ -156,6 +158,20 @@ static int bq24735_config_charger(struct bq24735 *charger) } } + if (pdata->wdt_timeout) { + value = pdata->wdt_timeout; + + ret = bq24735_update_word(charger->client, BQ24735_CHARGE_OPT, + BQ24735_CHARGE_OPT_WDT, + (value << BQ24735_CHARGE_OPT_WDT_OFFSET)); + if (ret < 0) { + dev_err(&charger->client->dev, + "Failed to write watchdog timer: %d\n", + ret); + return ret; + } + } + return 0; } @@ -347,6 +363,17 @@ static struct bq24735_platform *bq24735_parse_dt_data(struct i2c_client *client) if (!ret) pdata->input_current = val; + ret = of_property_read_u32(np, "ti,wdt-timeout", &val); + if (!ret) { + if (val <= 3) { + pdata->wdt_timeout = val; + } else { + dev_warn(&client->dev, + "Invalid value for ti,wdt-timeout: %d", + val); + } + } + pdata->ext_control = of_property_read_bool(np, "ti,external-control"); return pdata; @@ -476,6 +503,27 @@ static int bq24735_charger_probe(struct i2c_client *client, return 0; if (!charger->poll_interval) return 0; + if (charger->pdata->wdt_timeout) { + int wdt_ms; + + switch (charger->pdata->wdt_timeout) { + case 1: + wdt_ms = 44000; + break; + case 2: + wdt_ms = 88000; + break; + case 3: + wdt_ms = 175000; + break; + } + + if (charger->poll_interval > wdt_ms) { + dev_err(&client->dev, + "Poll interval greater than WDT timeout\n"); + return -EINVAL; + } + } ret = devm_delayed_work_autocancel(&client->dev, &charger->poll, bq24735_poll); diff --git a/include/linux/power/bq24735-charger.h b/include/linux/power/bq24735-charger.h index 321dd009ce66..ce5a030ca111 100644 --- a/include/linux/power/bq24735-charger.h +++ b/include/linux/power/bq24735-charger.h @@ -12,6 +12,7 @@ struct bq24735_platform { uint32_t charge_current; uint32_t charge_voltage; uint32_t input_current; + uint32_t wdt_timeout; const char *name;
The BQ24735 charger allows the user to set the watchdog timer delay between two consecutives ChargeCurrent or ChargeVoltage command writes, if the IC doesn't receive any command before the timeout happens, the charge is turned off. This patch adds the support to the user to change the default/POR value with four discrete values: 0 - disabled 1 - enabled, 44 sec 2 - enabled, 88 sec 3 - enabled, 175 sec (default at POR) These are the options supported in the ChargeOptions register bits 13&14. Also, this patch make one additional check when poll-interval is set by the user: if the interval set is greater than the WDT timeout it'll fail during the probe stage, preventing the user to set non-compatible values between the two options. Signed-off-by: Bruno Meneguele <bruno.meneguele@smartgreen.net> --- .../bindings/power/supply/bq24735.yaml | 13 +++++ drivers/power/supply/bq24735-charger.c | 48 +++++++++++++++++++ include/linux/power/bq24735-charger.h | 1 + 3 files changed, 62 insertions(+)