linux-kernel - Re: [PATCH] usb: typec: altmodes/displayport: Update active state

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: <CACeCKafPzxYWh5a4xmeggc+4zRou73kHnwV-G5xMfQDheGgGdg@mail.gmail.com>
Date:   Wed, 18 Jan 2023 10:26:21 -0800
From:   Prashant Malani <pmalani@...omium.org>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        bleung@...omium.org, stable@...r.kernel.org
Subject: Re: [PATCH] usb: typec: altmodes/displayport: Update active state

Hi Heikki,

Thanks for reviewing the patch.

On Wed, Jan 18, 2023 at 1:39 AM Heikki Krogerus
<heikki.krogerus@...ux.intel.com> wrote:
>
> On Wed, Jan 18, 2023 at 03:15:15AM +0000, Prashant Malani wrote:
> > Update the altmode "active" state when we receive Acks for Enter and
> > Exit Mode commands. Having the right state is necessary to change Pin
> > Assignments using the 'pin_assignment" sysfs file.
>
> The idea was that the port drivers take care of this, not the altmode
> drivers.

For the port's typec_altmode struct, that makes sense.
Should the port driver be taking care of the state for the partner's altmode
too, i.e "/sys/class/typec/port1-partner/port1-partner.0/active" ?

It seemed like the port driver should be forwarding the VDMs without snooping
the header, or IOW, it should let the altmode driver parse the VDMs (which it's
doing in this case) and manage the partner altmode state.

"pin_assignment_store" seems to only work if the partner's altmode
"active" bit is set to active [1]

FWIW, I think we can make the typec_altmode_update_active() calls from
our (cros-ec-typec) port driver too, but displayport.c is parsing the header
anyway, so it seemed repetitive. Just wanted to clarify the intention here.

BR,

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/typec/altmodes/displayport.c#n474