Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210407200723.1914388-1-badhri@google.com> <20210407200723.1914388-5-badhri@google.com>
 <YG67d/YOuoBwK+bF@kuha.fi.intel.com> <YG69UKTSgyHQHeFd@kuha.fi.intel.com>
In-Reply-To: <YG69UKTSgyHQHeFd@kuha.fi.intel.com>
From:   Badhri Jagan Sridharan <badhri@google.com>
Date:   Tue, 13 Apr 2021 17:59:52 -0700
Message-ID: <CAPTae5J2K_=VaL2mQOZxBgaSEGDavW=Rm8XNNDM4n=JPgqbjaw@mail.gmail.com>
Subject: Re: [PATCH v2 5/6] usb: typec: tcpm: Allow slow charging loops to
 comply to pSnkStby
To:     Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc:     Guenter Roeck <linux@roeck-us.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Rob Herring <robh+dt@kernel.org>,
        Adam Thomson <Adam.Thomson.Opensource@diasemi.com>,
        USB <linux-usb@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>, Kyle Tso <kyletso@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Apr 8, 2021 at 1:22 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> > > @@ -4047,9 +4053,12 @@ static void run_state_machine(struct tcpm_port *port)
> > >             break;
> > >     case SNK_DISCOVERY:
> > >             if (port->vbus_present) {
> > > -                   tcpm_set_current_limit(port,
> > > -                                          tcpm_get_current_limit(port),
> > > -                                          5000);
> > > +                   u32 current_lim = (!port->slow_charger_loop ||
> > > +                                      (tcpm_get_current_limit(port) <=
> > > +                                       PD_P_SNK_STDBY_MW / 5)) ?
> > > +                           tcpm_get_current_limit(port) :
> > > +                           PD_P_SNK_STDBY_MW / 5;
> >
> > Here the use of the ternary operator is not appropriate. Please try to
> > clean that up somehow. Maybe something like this would be better?
> >
> >                         u32 current_lim = tcpm_get_current_limit(port);
> >
> >                       if (port->slow_charger_loop || (current_lim < PD_P_SNK_STDBY_MW / 5))
> >                               current_lim = PD_P_SNK_STDBY_MW / 5;
>
> Sorry, I mean:
>
>                         if (port->slow_charger_loop || (current_lim > PD_P_SNK_STDBY_MW / 5))
>                                 current_lim = PD_P_SNK_STDBY_MW / 5;

Ack. Updating in my next version: V3.

Thanks,
Badhri

>
> thanks,
>
> --
> heikki