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

Reply via email to