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: <20210317181249.1062995-1-badhri@google.com> <PR3PR10MB41420951E2867C2E0E5272B980699@PR3PR10MB4142.EURPRD10.PROD.OUTLOOK.COM>
 <CAPTae5J_GdHsGQvNxgpffk5otyGhY8D48vddvin3A4fkz3KWUA@mail.gmail.com>
 <PR3PR10MB4142450638A8E07A33E475B080689@PR3PR10MB4142.EURPRD10.PROD.OUTLOOK.COM>
 <CAPTae5KX_+_4TN3DAptP-ra=Lzxnf7LgBDwxsv5JWxg4y4hQqQ@mail.gmail.com>
In-Reply-To: <CAPTae5KX_+_4TN3DAptP-ra=Lzxnf7LgBDwxsv5JWxg4y4hQqQ@mail.gmail.com>
From:   Badhri Jagan Sridharan <badhri@google.com>
Date:   Mon, 5 Apr 2021 18:43:58 -0700
Message-ID: <CAPTae5+kTcDE_gqr_Jy1VO-vfKED4vtc0yL6jjYSC0LZMwzc9g@mail.gmail.com>
Subject: Re: [PATCH v2] usb: typec: tcpm: Invoke power_supply_changed for tcpm-source-psy-
To:     Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Cc:     Guenter Roeck <linux@roeck-us.net>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Adam,

Just sent out a patch stack
https://patchwork.kernel.org/project/linux-usb/list/?series=461087
to address the issue that you mentioned here.

Thanks,
Badhri


On Mon, Apr 5, 2021 at 6:43 PM Badhri Jagan Sridharan <badhri@google.com> wrote:
>
> Hi Adam,
>
> Just sent out a patch stack https://patchwork.kernel.org/project/linux-usb/list/?series=461087
> to address the issue that you mentioned here.
>
> Thanks,
> Badhri
>
> On Fri, Mar 19, 2021 at 9:32 AM Adam Thomson <Adam.Thomson.Opensource@diasemi.com> wrote:
>>
>> On 18 March 2021 20:40, Badhri Jagan Sridharan wrote:
>>
>> > > Regarding selecting PDOs or PPS APDOs, surely we should only notify of a
>> > change
>> > > when we reach SNK_READY which means a new contract has been established?
>> > Until
>> > > that point it's possible any requested change could be rejected so why inform
>> > > clients before we know the settings have taken effect? I could be missing
>> > > something here as it's been a little while since I delved into this, but this
>> > > doesn't seem to make sense to me.
>> >
>> > I was trying to keep the power_supply_changed call close to the
>> > variables which are used to infer the power supply property values.
>> > Since port->pps_data.max_curr is already updated here and that's used
>> > to infer the CURRENT_MAX a client could still read this before the
>> > request goes through right ?
>>
>> Actually that's fair but I think the problem here relates to 'max_curr' not
>> being reset if the SRC rejects our request when we're swapping between one PPS
>> APDO and another PPS APDO. I think the 'max_curr' value should be reverted back
>> to the value for the existing PPS APDO we were already using. I suspect the same
>> might be true of 'min_volt' and 'max_volt' as well, now I look at it. It might
>> actually be prudent to have pending PPS data based on a request, which is only
>> committed as active once ACCEPT has been received.
>>
>> Regarding power_supply_changed() though, I still think we should only notify of
>> a change when the requested change has been accepted by the source, in relation
>> to these values as they should reflect the real, in-use voltage and current
>> values.
>>