[PATCH 1/3] extcon: intel-cht-wc: Make error messages consistent

[PATCH 1/3] extcon: intel-cht-wc: Make error messages consistent

[Hans de Goede]

Before this commit the error messages were a mix of "Failed to ..." and
"Error ...ing ...".

This commit makes all the error messages consistently use "Error ...ing".

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-intel-cht-wc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 0a05cc3..f1c43af 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -252,7 +252,7 @@ static int cht_wc_extcon_sw_control(struct cht_wc_extcon_data *ext, bool enable)
 	val = enable ? mask : 0;
 	ret = regmap_update_bits(ext->regmap, CHT_WC_CHGRCTRL0, mask, val);
 	if (ret)
-		dev_err(ext->dev, "Failed setting sw control: %d\n", ret);
+		dev_err(ext->dev, "Error setting sw control: %d\n", ret);
 
 	return ret;
 }
@@ -288,7 +288,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
 	/* Register extcon device */
 	ret = devm_extcon_dev_register(ext->dev, ext->edev);
 	if (ret) {
-		dev_err(ext->dev, "Failed to register extcon device\n");
+		dev_err(ext->dev, "Error registering extcon device: %d\n", ret);
 		goto disable_sw_control;
 	}
 
@@ -301,7 +301,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
 	ret = devm_request_threaded_irq(ext->dev, irq, NULL, cht_wc_extcon_isr,
 					IRQF_ONESHOT, pdev->name, ext);
 	if (ret) {
-		dev_err(ext->dev, "Failed to request interrupt\n");
+		dev_err(ext->dev, "Error requesting interrupt: %d\n", ret);
 		goto disable_sw_control;
 	}
 
@@ -310,7 +310,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
 			   (int)~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_ID_GND |
 				  CHT_WC_PWRSRC_ID_FLOAT));
 	if (ret) {
-		dev_err(ext->dev, "Failed to write the irq-mask: %d\n", ret);
+		dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
 		goto disable_sw_control;
 	}
 
-- 
2.9.3

[PATCH 2/3] extcon: intel-cht-wc: Disable external 5v boost converter on probe

[Hans de Goede]

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 <hdegoede@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)
+
+#define CHT_WC_VBUS_ENABLE_GPIO		2
+
 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)
-- 
2.9.3

[PATCH 3/3] extcon: intel-cht-wc: Ignore failure to detect charger-type on host mode exit

[Hans de Goede]

When we leave host-mode because the id-pin is no longer connected to
ground, the 5v boost converter is normally still on, so we will see
Vbus, but it is not from a charger (normally) so the charger-type
detection will fail.

This commit silences the cht_wc_extcon_get_charger() false-positive
errors when we're leaving host mode.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-intel-cht-wc.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 8651f63..0e1f2c7 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -98,6 +98,7 @@ struct cht_wc_extcon_data {
 	struct regmap *regmap;
 	struct extcon_dev *edev;
 	unsigned int previous_cable;
+	bool usb_host;
 };
 
 static int cht_wc_extcon_get_id(struct cht_wc_extcon_data *ext, int pwrsrc_sts)
@@ -114,7 +115,8 @@ static int cht_wc_extcon_get_id(struct cht_wc_extcon_data *ext, int pwrsrc_sts)
 	return USB_ID_FLOAT;
 }
 
-static int cht_wc_extcon_get_charger(struct cht_wc_extcon_data *ext)
+static int cht_wc_extcon_get_charger(struct cht_wc_extcon_data *ext,
+				     bool ignore_errors)
 {
 	int ret, usbsrc, status;
 	unsigned long timeout;
@@ -137,6 +139,9 @@ static int cht_wc_extcon_get_charger(struct cht_wc_extcon_data *ext)
 	} while (time_before(jiffies, timeout));
 
 	if (status != CHT_WC_USBSRC_STS_SUCCESS) {
+		if (ignore_errors)
+			return EXTCON_CHG_USB_SDP; /* Save fallback */
+
 		if (status == CHT_WC_USBSRC_STS_FAIL)
 			dev_warn(ext->dev, "Could not detect charger type\n");
 		else
@@ -206,6 +211,8 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
 {
 	int ret, pwrsrc_sts, id;
 	unsigned int cable = EXTCON_NONE;
+	/* Ignore errors in host mode, as the 5v boost converter is on then */
+	bool ignore_get_charger_errors = ext->usb_host;
 
 	ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts);
 	if (ret) {
@@ -226,7 +233,7 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
 		goto set_state;
 	}
 
-	ret = cht_wc_extcon_get_charger(ext);
+	ret = cht_wc_extcon_get_charger(ext, ignore_get_charger_errors);
 	if (ret >= 0)
 		cable = ret;
 
@@ -241,8 +248,8 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
 		ext->previous_cable = cable;
 	}
 
-	extcon_set_state_sync(ext->edev, EXTCON_USB_HOST,
-			      id == USB_ID_GND || id == USB_RID_A);
+	ext->usb_host = id == USB_ID_GND || id == USB_RID_A;
+	extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
 }
 
 static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
-- 
2.9.3

Re: [PATCH 1/3] extcon: intel-cht-wc: Make error messages consistent

[Chanwoo Choi]

Hi,

The intel-cht-wc.c[1] was merged on only extcon-next branch.
I think that this patch better to be squashed with patch[1].
[1] commit 6786e42f31637 ("extcon: intel-cht-wc: Add Intel Cherry Trail Whiskey Cove PMIC extcon driver")

How about it?

On 2017년 04월 03일 20:26, Hans de Goede wrote:
> Before this commit the error messages were a mix of "Failed to ..." and
> "Error ...ing ...".
>
> This commit makes all the error messages consistently use "Error ...ing".
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-intel-cht-wc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
> index 0a05cc3..f1c43af 100644
> --- a/drivers/extcon/extcon-intel-cht-wc.c
> +++ b/drivers/extcon/extcon-intel-cht-wc.c
> @@ -252,7 +252,7 @@ static int cht_wc_extcon_sw_control(struct cht_wc_extcon_data *ext, bool enable)
>  	val = enable ? mask : 0;
>  	ret = regmap_update_bits(ext->regmap, CHT_WC_CHGRCTRL0, mask, val);
>  	if (ret)
> -		dev_err(ext->dev, "Failed setting sw control: %d\n", ret);
> +		dev_err(ext->dev, "Error setting sw control: %d\n", ret);
>  
>  	return ret;
>  }
> @@ -288,7 +288,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>  	/* Register extcon device */
>  	ret = devm_extcon_dev_register(ext->dev, ext->edev);
>  	if (ret) {
> -		dev_err(ext->dev, "Failed to register extcon device\n");
> +		dev_err(ext->dev, "Error registering extcon device: %d\n", ret);
>  		goto disable_sw_control;
>  	}
>  
> @@ -301,7 +301,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>  	ret = devm_request_threaded_irq(ext->dev, irq, NULL, cht_wc_extcon_isr,
>  					IRQF_ONESHOT, pdev->name, ext);
>  	if (ret) {
> -		dev_err(ext->dev, "Failed to request interrupt\n");
> +		dev_err(ext->dev, "Error requesting interrupt: %d\n", ret);
>  		goto disable_sw_control;
>  	}
>  
> @@ -310,7 +310,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>  			   (int)~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_ID_GND |
>  				  CHT_WC_PWRSRC_ID_FLOAT));
>  	if (ret) {
> -		dev_err(ext->dev, "Failed to write the irq-mask: %d\n", ret);
> +		dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
>  		goto disable_sw_control;
>  	}
>  


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 2/3] extcon: intel-cht-wc: Disable external 5v boost converter on probe

[Chanwoo Choi]

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 <hdegoede@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

Re: [PATCH 1/3] extcon: intel-cht-wc: Make error messages consistent

[Hans de Goede]

Hi,

On 04-04-17 08:19, Chanwoo Choi wrote:
> Hi,
>
> The intel-cht-wc.c[1] was merged on only extcon-next branch.
> I think that this patch better to be squashed with patch[1].
> [1] commit 6786e42f31637 ("extcon: intel-cht-wc: Add Intel Cherry Trail Whiskey Cove PMIC extcon driver")
>
> How about it?

That is fine with me.

Regards,

Hans



>
> On 2017년 04월 03일 20:26, Hans de Goede wrote:
>> Before this commit the error messages were a mix of "Failed to ..." and
>> "Error ...ing ...".
>>
>> This commit makes all the error messages consistently use "Error ...ing".
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/extcon/extcon-intel-cht-wc.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
>> index 0a05cc3..f1c43af 100644
>> --- a/drivers/extcon/extcon-intel-cht-wc.c
>> +++ b/drivers/extcon/extcon-intel-cht-wc.c
>> @@ -252,7 +252,7 @@ static int cht_wc_extcon_sw_control(struct cht_wc_extcon_data *ext, bool enable)
>>  	val = enable ? mask : 0;
>>  	ret = regmap_update_bits(ext->regmap, CHT_WC_CHGRCTRL0, mask, val);
>>  	if (ret)
>> -		dev_err(ext->dev, "Failed setting sw control: %d\n", ret);
>> +		dev_err(ext->dev, "Error setting sw control: %d\n", ret);
>>
>>  	return ret;
>>  }
>> @@ -288,7 +288,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>>  	/* Register extcon device */
>>  	ret = devm_extcon_dev_register(ext->dev, ext->edev);
>>  	if (ret) {
>> -		dev_err(ext->dev, "Failed to register extcon device\n");
>> +		dev_err(ext->dev, "Error registering extcon device: %d\n", ret);
>>  		goto disable_sw_control;
>>  	}
>>
>> @@ -301,7 +301,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>>  	ret = devm_request_threaded_irq(ext->dev, irq, NULL, cht_wc_extcon_isr,
>>  					IRQF_ONESHOT, pdev->name, ext);
>>  	if (ret) {
>> -		dev_err(ext->dev, "Failed to request interrupt\n");
>> +		dev_err(ext->dev, "Error requesting interrupt: %d\n", ret);
>>  		goto disable_sw_control;
>>  	}
>>
>> @@ -310,7 +310,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>>  			   (int)~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_ID_GND |
>>  				  CHT_WC_PWRSRC_ID_FLOAT));
>>  	if (ret) {
>> -		dev_err(ext->dev, "Failed to write the irq-mask: %d\n", ret);
>> +		dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
>>  		goto disable_sw_control;
>>  	}
>>
>
>

Re: [PATCH 2/3] extcon: intel-cht-wc: Disable external 5v boost converter on probe

[Hans de Goede]

Hi,

On 04-04-17 08:31, Chanwoo Choi wrote:
> 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 <hdegoede@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.

All the other BIT definitions don't use _MASK either so
that would be inconsistent.

> - 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.

Ok, I will send you a version 2 using:

#define CHT_WC_VBUS_GPIO_CTLO			0x6e2d
#define CHT_WC_VBUS_GPIO_CTLO_OUTPUT		BIT(0)

Regards,

Hans



>
>> +
>>  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)
>>
>
>

Re: [PATCH 1/3] extcon: intel-cht-wc: Make error messages consistent

[Chanwoo Choi]

On 2017년 04월 04일 17:53, Hans de Goede wrote:
> Hi,
> 
> On 04-04-17 08:19, Chanwoo Choi wrote:
>> Hi,
>>
>> The intel-cht-wc.c[1] was merged on only extcon-next branch.
>> I think that this patch better to be squashed with patch[1].
>> [1] commit 6786e42f31637 ("extcon: intel-cht-wc: Add Intel Cherry Trail Whiskey Cove PMIC extcon driver")
>>
>> How about it?
> 
> That is fine with me.

OK. Applied it.

> 
> Regards,
> 
> Hans
> 
> 
> 
>>
>> On 2017년 04월 03일 20:26, Hans de Goede wrote:
>>> Before this commit the error messages were a mix of "Failed to ..." and
>>> "Error ...ing ...".
>>>
>>> This commit makes all the error messages consistently use "Error ...ing".
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>  drivers/extcon/extcon-intel-cht-wc.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
>>> index 0a05cc3..f1c43af 100644
>>> --- a/drivers/extcon/extcon-intel-cht-wc.c
>>> +++ b/drivers/extcon/extcon-intel-cht-wc.c
>>> @@ -252,7 +252,7 @@ static int cht_wc_extcon_sw_control(struct cht_wc_extcon_data *ext, bool enable)
>>>      val = enable ? mask : 0;
>>>      ret = regmap_update_bits(ext->regmap, CHT_WC_CHGRCTRL0, mask, val);
>>>      if (ret)
>>> -        dev_err(ext->dev, "Failed setting sw control: %d\n", ret);
>>> +        dev_err(ext->dev, "Error setting sw control: %d\n", ret);
>>>
>>>      return ret;
>>>  }
>>> @@ -288,7 +288,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>>>      /* Register extcon device */
>>>      ret = devm_extcon_dev_register(ext->dev, ext->edev);
>>>      if (ret) {
>>> -        dev_err(ext->dev, "Failed to register extcon device\n");
>>> +        dev_err(ext->dev, "Error registering extcon device: %d\n", ret);
>>>          goto disable_sw_control;
>>>      }
>>>
>>> @@ -301,7 +301,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>>>      ret = devm_request_threaded_irq(ext->dev, irq, NULL, cht_wc_extcon_isr,
>>>                      IRQF_ONESHOT, pdev->name, ext);
>>>      if (ret) {
>>> -        dev_err(ext->dev, "Failed to request interrupt\n");
>>> +        dev_err(ext->dev, "Error requesting interrupt: %d\n", ret);
>>>          goto disable_sw_control;
>>>      }
>>>
>>> @@ -310,7 +310,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>>>                 (int)~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_ID_GND |
>>>                    CHT_WC_PWRSRC_ID_FLOAT));
>>>      if (ret) {
>>> -        dev_err(ext->dev, "Failed to write the irq-mask: %d\n", ret);
>>> +        dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
>>>          goto disable_sw_control;
>>>      }
>>>
>>
>>
> 
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics