On Fri, 2017-03-17 at 10:55 +0100, Hans de Goede wrote:
> Add support for monitoring an extcon device with SDP/CDP/DCP and HOST
> cables and adjust ilimit and enable/disable the 5v boost converter
> accordingly. This is necessary on systems where the PSEL pin is
> hardwired
> high and ILIM needs to be set by software based on the detected
> charger
> type.
>
> config CHARGER_BQ24190
> tristate "TI BQ24190 battery charger driver"
> depends on I2C
> + depends on EXTCON
I dunno what is preferred here, but if we would like to keep
compatibility with previous configurations "select" should be used over
"depends on".
> +static void bq24190_extcon_work(struct work_struct *work)
> +{
> + struct bq24190_dev_info *bdi =
> + container_of(work, struct bq24190_dev_info,
> extcon_work);
> + int ret, iinlim = 0;
> +
> + if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_SDP) == 1)
> + iinlim = 500000;
> + else if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_CDP) ==
> 1 ||
> + extcon_get_state(bdi->extcon, EXTCON_CHG_USB_ACA) ==
> 1)
> + iinlim = 1500000;
> + else if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_DCP) ==
> 1)
> + iinlim = 2000000;
> +
> + if (iinlim) {
Could be possible to call below unconditionally here (use 0)?
> + ret = bq24190_set_field_val(bdi, BQ24190_REG_ISC,
> + BQ24190_REG_ISC_IINLIM_MASK,
> + BQ24190_REG_ISC_IINLIM_SHIFT,
> + bq24190_iinlim_values,
> + ARRAY_SIZE(bq24190_iinlim_values),
> + iinlim);
> + if (ret)
> + dev_err(bdi->dev, "Can't set IINLIM: %d\n",
> ret);
> + }
Perhaps make above as a helper?
In that case no need for "if (iinlim)" and perhaps switch-case might be
used instead of if-else-if (latter is up to you).
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
