linux-kernel - Re: [PATCH v2 18/20] extcon: intel-cht-wc: Refactor cht_wc_extcon_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f84e2060-f6b7-64f9-78cd-e8ad8776ab2d@gmail.com>
Date:   Wed, 17 Nov 2021 16:15:58 +0900
From:   Chanwoo Choi <cwchoi00@...il.com>
To:     Hans de Goede <hdegoede@...hat.com>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Mark Gross <markgross@...nel.org>,
        Andy Shevchenko <andy@...radead.org>,
        Wolfram Sang <wsa@...-dreams.de>,
        Sebastian Reichel <sre@...nel.org>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Ard Biesheuvel <ardb@...nel.org>
Cc:     Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org,
        Yauhen Kharuzhy <jekhor@...il.com>,
        Tsuchiya Yuto <kitakar@...il.com>,
        platform-driver-x86@...r.kernel.org, linux-i2c@...r.kernel.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-efi@...r.kernel.org
Subject: Re: [PATCH v2 18/20] extcon: intel-cht-wc: Refactor
 cht_wc_extcon_get_charger()

Hello,

I think that you need to squash it with patch21
I'm not sure that this patch is either atomic or not because
you remove the 'return EXTCON_CHG_USB_SDP/EXTCON_CHG_USB_SDP'
without explaining why it is no problem. Just mention that
pass the role to next 'switch' cases. But, before this change,
there were any reason to return the type of charger cable
before switch statement.

According to your patch description, you don't need
to make the separate patch of it. Please squash it with patch21.

On 21. 11. 15. 오전 2:03, Hans de Goede wrote:
> Refactor cht_wc_extcon_get_charger() to have all the returns are in
> the "switch (usbsrc)" cases.
> 
> This is a preparation patch for adding support for registering
> a power_supply class device.
> 
> Signed-off-by: Hans de Goede <hdegoede@...hat.com>
> ---
>   drivers/extcon/extcon-intel-cht-wc.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
> index 119b83793123..f2b93a99a625 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;
> 


-- 
Best Regards,
Samsung Electronics
Chanwoo Choi