diff mbox series

[v4,18/20] extcon: intel-cht-wc: Refactor cht_wc_extcon_get_charger()

Message ID 20220130204557.15662-19-hdegoede@redhat.com
State Superseded
Headers show
Series power-suppy/i2c/extcon: Fix charger setup on Xiaomi Mi Pad 2 and Lenovo Yogabook | expand

Commit Message

Hans de Goede Jan. 30, 2022, 8:45 p.m. UTC 
This is a preparation patch for adding support for registering
a power_supply class device.

Setting usbsrc to "CHT_WC_USBSRC_TYPE_SDP << CHT_WC_USBSRC_TYPE_SHIFT"
will make the following switch-case return EXTCON_CHG_USB_SDP
just as before, so there is no functional change.

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- Reword the commit message
---
 drivers/extcon/extcon-intel-cht-wc.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

Andy Shevchenko Jan. 31, 2022, 1:56 p.m. UTC | #1 
On Sun, Jan 30, 2022 at 09:45:55PM +0100, Hans de Goede wrote:
> This is a preparation patch for adding support for registering
> a power_supply class device.
> 
> Setting usbsrc to "CHT_WC_USBSRC_TYPE_SDP << CHT_WC_USBSRC_TYPE_SHIFT"
> will make the following switch-case return EXTCON_CHG_USB_SDP
> just as before, so there is no functional change.

...

> -		return EXTCON_CHG_USB_SDP; /* Save fallback */

> +		/* Save fallback */

I see it's in the previous code, but what does it mean?
I would read it as "Safe fallback", bit I have no clue.
Hans de Goede Jan. 31, 2022, 3:22 p.m. UTC | #2 
Hi,

On 1/31/22 14:56, Andy Shevchenko wrote:
> On Sun, Jan 30, 2022 at 09:45:55PM +0100, Hans de Goede wrote:
>> This is a preparation patch for adding support for registering
>> a power_supply class device.
>>
>> Setting usbsrc to "CHT_WC_USBSRC_TYPE_SDP << CHT_WC_USBSRC_TYPE_SHIFT"
>> will make the following switch-case return EXTCON_CHG_USB_SDP
>> just as before, so there is no functional change.
> 
> ...
> 
>> -		return EXTCON_CHG_USB_SDP; /* Save fallback */
> 
>> +		/* Save fallback */
> 
> I see it's in the previous code, but what does it mean?
> I would read it as "Safe fallback", bit I have no clue.

Ah yes that should be safe not save, sorry will fix for v5.

Regards,

Hans
diff mbox series

Patch

diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index edc386937dee..150637bea417 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -153,14 +153,15 @@  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 (!ignore_errors) {
+			if (status == CHT_WC_USBSRC_STS_FAIL)
+				dev_warn(ext->dev, "Could not detect charger type\n");
+			else
+				dev_warn(ext->dev, "Timeout detecting charger type\n");
+		}
 
-		if (status == CHT_WC_USBSRC_STS_FAIL)
-			dev_warn(ext->dev, "Could not detect charger type\n");
-		else
-			dev_warn(ext->dev, "Timeout detecting charger type\n");
-		return EXTCON_CHG_USB_SDP; /* Save fallback */
+		/* Save fallback */
+		usbsrc = CHT_WC_USBSRC_TYPE_SDP << CHT_WC_USBSRC_TYPE_SHIFT;
 	}
 
 	usbsrc = (usbsrc & CHT_WC_USBSRC_TYPE_MASK) >> CHT_WC_USBSRC_TYPE_SHIFT;