Hi,
On 2017년 04월 03일 20:26, Hans de Goede wrote:
> Disable the 5v boost converter on probe in case it was left on by
> the BIOS, this fixes 2 problems:
>
> 1) This gets seen by the external battery charger as a valid Vbus
> supply and it then tries to feed Vsys from this creating a
> feedback loop which causes aprox. 300 mA extra battery drain
> (and unless we drive the external-charger-disable pin high it
> also tries to charge the battery causing even more feedback).
> 2) This gets seen by the pwrsrc block as a SDP USB Vbus supply
>
> Since the external battery charger has its own 5v boost converter
> which does not have these issues, we simply turn the separate
> external 5v boost converter off and leave it off entirely.
>
> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> ---
> drivers/extcon/extcon-intel-cht-wc.c | 38
> ++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/drivers/extcon/extcon-intel-cht-wc.c
> b/drivers/extcon/extcon-intel-cht-wc.c
> index f1c43af..8651f63 100644
> --- a/drivers/extcon/extcon-intel-cht-wc.c
> +++ b/drivers/extcon/extcon-intel-cht-wc.c
> @@ -64,6 +64,11 @@
> #define CHT_WC_PWRSRC_ID_GND BIT(3)
> #define CHT_WC_PWRSRC_ID_FLOAT BIT(4)
>
> +#define CHT_WC_GPIO0_CTLO 0x6e2b
> +#define CHT_WC_GPIO_CTLO_OUTPUT BIT(0)
CHT_WC_GPIO0_CTLO or CHT_WC_GPIO_CTLO ?
Because CHT_WC_GPIO_CTLO_OUTPUT defintion use just 'GPIO' instead of 'GPIO0'.
I prefer to change the definition name as following with '_MASK' postfix.
- CHT_WC_GPIO_CTLO_OUTPUT -> CHT_WC_GPIO_CTLO_OUTPUT_MASK
> +#define CHT_WC_GPIO0_CTLO 0x6e2b
> +
> +#define CHT_WC_VBUS_ENABLE_GPIO 2
I think that 'CHT_WC_GPIO0_CTLO + CHT_WC_VBUS_ENABLE_GPIO' is a separate
register address. Maybe you can define the separate register offset
instead of the separate the CHT_WC_VBUS_ENABLE_GPIO offset.
> +
> enum cht_wc_usb_id {
> USB_ID_OTG,
> USB_ID_GND,
> @@ -170,6 +175,24 @@ static void cht_wc_extcon_set_phymux(struct
> cht_wc_extcon_data *ext, u8 state)
> dev_err(ext->dev, "Error writing phyctrl: %d\n", ret);
> }
>
> +static void cht_wc_extcon_set_5v_boost(struct cht_wc_extcon_data *ext,
> + bool enable)
> +{
> + int ret, val;
> +
> + val = enable ? CHT_WC_GPIO_CTLO_OUTPUT : 0;
> +
> + /*
> + * The 5V boost converter is enabled through a gpio on the PMIC, since
> + * there currently is no gpio driver we access the gpio reg directly.
> + */
> + ret = regmap_update_bits(ext->regmap,
> + CHT_WC_GPIO0_CTLO + CHT_WC_VBUS_ENABLE_GPIO,
> + CHT_WC_GPIO_CTLO_OUTPUT, val);
> + if (ret)
> + dev_err(ext->dev, "Error writing Vbus GPIO CTLO: %d\n", ret);
> +}
> +
> /* Small helper to sync EXTCON_CHG_USB_SDP and EXTCON_USB state */
> static void cht_wc_extcon_set_state(struct cht_wc_extcon_data *ext,
> unsigned int cable, bool state)
> @@ -280,6 +303,21 @@ static int cht_wc_extcon_probe(struct platform_device
> *pdev)
> if (IS_ERR(ext->edev))
> return PTR_ERR(ext->edev);
>
> + /*
> + * When a host-cable is detected the BIOS enables an external 5v boost
> + * converter to power connected devices there are 2 problems with this:
> + * 1) This gets seen by the external battery charger as a valid Vbus
> + * supply and it then tries to feed Vsys from this creating a
> + * feedback loop which causes aprox. 300 mA extra battery drain
> + * (and unless we drive the external-charger-disable pin high it
> + * also tries to charge the battery causing even more feedback).
> + * 2) This gets seen by the pwrsrc block as a SDP USB Vbus supply
> + * Since the external battery charger has its own 5v boost converter
> + * which does not have these issues, we simply turn the separate
> + * external 5v boost converter off and leave it off entirely.
> + */
> + cht_wc_extcon_set_5v_boost(ext, false);
> +
> /* Enable sw control */
> ret = cht_wc_extcon_sw_control(ext, true);
> if (ret)
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
