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;
IronPort-SDR: RJGCzVcAxNlQricLjz/DfdP+9ItWBjQ4wrobbJRprvRGrx6Laaip7Yanyb3ViB2JHe6VI87AF7
 xvzf09HF7KOw==
IronPort-SDR: ZLLqWybu4KNlwT0nEra2KW9AAi9SfnFfqpqzXJdn+3aobKyzkNBm4nS7myaqwoGh6IiPV0zit4
 L+35aKnD+L+Q==
Date:   Tue, 8 Dec 2020 11:37:34 +0200
From:   Heikki Krogerus <heikki.krogerus@linux.intel.com>
To:     Prashant Malani <pmalani@chromium.org>
Cc:     linux-usb@vger.kernel.org, gregkh@linuxfoundation.org,
        bleung@chromium.org, Jonathan Corbet <corbet@lwn.net>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] usb: typec: Add bus type for plug alt modes
Message-ID: <20201208093734.GD680328@kuha.fi.intel.com>
References: <20201203030846.51669-1-pmalani@chromium.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201203030846.51669-1-pmalani@chromium.org>
Precedence: bulk

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@chromium.org>
> Cc: Heikki Krogerus <heikki.krogerus@linux.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