linux-kernel - Re: [PATCH] usb: typec: Add bus type for plug alt modes

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: <20201208093734.GD680328@kuha.fi.intel.com>
Date:   Tue, 8 Dec 2020 11:37:34 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Prashant Malani <pmalani@...omium.org>
Cc:     linux-usb@...r.kernel.org, gregkh@...uxfoundation.org,
        bleung@...omium.org, Jonathan Corbet <corbet@....net>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: typec: Add bus type for plug alt modes

On Wed, Dec 02, 2020 at 07:08:47PM -0800, Prashant Malani wrote:
> Add the Type C bus for plug alternate modes which are being
> registered via the Type C connector class. This ensures that udev events
> get generated when plug alternate modes are registered (and not just for
> partner/port alternate modes), even though the Type C bus doesn't link
> plug alternate mode devices to alternate mode drivers.

I still don't understand how is the uevent related to the bus? If you
check the device_add() function, on line 2917, kobject_uevent() is
called unconditionally. The device does not need a bus for that event
to be generated.

Also, I don't understand how are the cable plug alt modes now
prevented from being bind to the alt mode drivers?

> Update the Type C bus documentation to mention that there
> are alternate mode devices for plugs as well.
> 
> Signed-off-by: Prashant Malani <pmalani@...omium.org>
> Cc: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> ---
>  Documentation/driver-api/usb/typec_bus.rst | 6 +++---
>  drivers/usb/typec/class.c                  | 8 ++++++--
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/driver-api/usb/typec_bus.rst b/Documentation/driver-api/usb/typec_bus.rst
> index 21c890ae17e5..7874d2f37d9f 100644
> --- a/Documentation/driver-api/usb/typec_bus.rst
> +++ b/Documentation/driver-api/usb/typec_bus.rst
> @@ -15,9 +15,9 @@ modes by using the SVID and the mode number.
>  
>  :ref:`USB Type-C Connector Class <typec>` provides a device for every alternate
>  mode a port supports, and separate device for every alternate mode the partner
> -supports. The drivers for the alternate modes are bound to the partner alternate
> -mode devices, and the port alternate mode devices must be handled by the port
> -drivers.
> +or cable plug supports. The drivers for the alternate modes are bound to the
> +partner alternate mode devices, and the port alternate mode devices must be
> +handled by the port drivers.
>  
>  When a new partner alternate mode device is registered, it is linked to the
>  alternate mode device of the port that the partner is attached to, that has
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 35eec707cb51..74061a699f16 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -478,8 +478,12 @@ typec_register_altmode(struct device *parent,
>  	if (!is_port)
>  		typec_altmode_set_partner(alt);
>  
> -	/* The partners are bind to drivers */
> -	if (is_typec_partner(parent))
> +	/*
> +	 * The partners are bind to drivers.
> +	 * Also set the bus field for plug alt modes so that the udev event occurs on device
> +	 * registration.
> +	 */
> +	if (is_typec_partner(parent) || is_typec_plug(parent))
>  		alt->adev.dev.bus = &typec_bus;
>  
>  	ret = device_register(&alt->adev.dev);
> -- 
> 2.29.2.454.gaff20da3a2-goog

thanks,

-- 
heikki