Re: [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration

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

On Wed, Oct 02, 2019 at 08:45:28PM -0700, Guenter Roeck wrote:
> On 10/2/19 11:29 AM, Heikki Krogerus wrote:
> > On Wed, Oct 02, 2019 at 09:36:39AM -0700, Guenter Roeck wrote:
> > > port->cap->type used to be the role provided by the low level driver.
> > > That was static, and it was not possible to override it. It did not
> > > resemble the current port type, or a configured port type, it resembled
> > > port capabilities.
> > > 
> > > Your code changes that, meaning even if the low level driver (effectively
> > > the hardware) states that it can not support DRP, it is now configurable
> > > anyway. That seems to me like a substantial change to the original meaning
> > > of this attribute.
> > > 
> > > Effectiv ely there are now three values,
> > > - port->port_type	the current port tyle
> > > - port->fixed_type	the type selected by the user
> > > - port->cap->type	the type provided by low level code,
> > > 			now no longer available / used
> > > 
> > > Even if the low level driver (hardware) says that it can not support
> > > TYPEC_PORT_DRP, that can be overwritten by the user. Maybe there is a
> > > good reason for that, but I don't see it, sorry.
> > 
> > No, that was not my intention. Clearly there is a bug in my code.
> > 
> > > Maybe it would make sense to introduce port->fixed_type in a separate patch.
> > > As part of that patch you could explain why port->cap->type, ie a reflection
> > > of the port capabilities, is no longer needed.
> > 
> > Or, I could make this really simple. I could just copy the content of
> > the cap as is to another struct typec_capability during port
> > registration. My goal is really just remove the need for the device
> > drivers keep the struct typec_capability instance if they don't need
> > it, and I don't need to remove the cap member to achieve that.
> > 
> 
> Maybe just use devm_kmemdup() ?

For example.

thanks,

-- 
heikki