lkml.org 
[lkml]   [2021]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v2 18/20] extcon: intel-cht-wc: Refactor cht_wc_extcon_get_charger()
    From
    Hi,

    On 11/17/21 08:15, Chanwoo Choi wrote:
    > 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.

    The setting of 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.

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

    Having this refactoring in a separate patch makes it easier
    to see what is going on in patch 21. So I'm going to keep this
    as a separate patch for v3 of this series.

    Thanks & Regards,

    Hans



    > 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@redhat.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;
    >>
    >
    >

    \
     
     \ /
      Last update: 2021-11-17 23:31    [W:3.432 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site