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

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Sat, Oct 05, 2019 at 10:36:02AM -0700, Guenter Roeck wrote:
> 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.

Yes, this series is broken. I'll prepare v3.

I'm going to add two more patches to this series where I'll drop a few
more unused members from struct typec_capability.

thanks,

-- 
heikki