Re: [PATCH v2 3/7] usb: typec: Separate the operations vector

[Guenter Roeck <linux@roeck-us.net>]

On 10/4/19 8:08 AM, Heikki Krogerus wrote:
> Introducing struct typec_operations which has the same
> callbacks as struct typec_capability. The old callbacks are
> kept for now, but after all users have been converted, they
> will be removed.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>   drivers/usb/typec/class.c | 39 +++++++++++++++++++++++++++++----------
>   include/linux/usb/typec.h | 20 ++++++++++++++++++++
>   2 files changed, 49 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 89ffe370e426..f4972b7ee022 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -54,6 +54,7 @@ struct typec_port {
>   
>   	const struct typec_capability	*orig_cap; /* to be removed */
>   	const struct typec_capability	*cap;
> +	const struct typec_operations   *ops;
>   };
>   
>   #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> @@ -956,7 +957,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
>   		return -EOPNOTSUPP;
>   	}
>   
> -	if (!port->cap->try_role) {
> +	if (!port->cap->try_role || (!port->ops && !port->ops->try_role)) {

Even though it is only temporary, this should be
	if (!port->cap->try_role && (!port->ops || !port->ops->try_role)) {

otherwise both cap->try_role and ops->try_role must exist. Also, there would
be a crash if port->cap->try_role and port->ops are both NULL. Same pretty
much everywhere below.

Guenter