lkml.org 
[lkml]   [2016]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCHv2] usb: USB Type-C Connector Class
    On Tue, May 24, 2016 at 12:28:26PM -0700, Guenter Roeck wrote:
    > On Thu, May 19, 2016 at 03:44:54PM +0300, Heikki Krogerus wrote:
    > > The purpose of this class is to provide unified interface for user
    > > space to get the status and basic information about USB Type-C
    > > Connectors in the system, control data role swapping, and when USB PD
    > > is available, also power role swapping and Alternate Modes.
    > >
    > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
    >
    > [ ... ]
    >
    > > +
    > > +static void typec_remove_partner(struct typec_port *port)
    > > +{
    > > + sysfs_remove_link(&port->dev.kobj, "partner");
    > > + typec_unregister_altmodes(port->partner->alt_modes);
    >
    > This only unregisters alternate modes registered through typec_add_partner(),
    > but not alternate modes registered separately. Or is the calling code expected
    > to set port->partner->alt_modes when calling typec_register_altmodes()
    > directly ?

    The altmodes for the partner are not meant to be registered
    separately. With the partners and also cable plugs the class is in
    control of registering and unregistering of the altmode devices after
    typec_connect() is called.

    The idea was that only the ports will register the alternate modes
    they support separately, but I think we have to change that too. So I
    don't think we'll export the typec_un/register_altmodes() at all.

    We will have to prevent any drivers from being bound to the port
    alternate mode devices when we add the alternate mode bus, and I had
    some idea where by making the port drivers themselves in charge of
    registering the port alternate modes, we could prevent it easily. But
    it's probable easier to just handle those in the class driver as well.

    > > +
    > > +void typec_unregister_altmodes(struct typec_altmode *alt_modes)
    > > +{
    > > + struct typec_altmode *alt;
    > > +
    > This will crash if alt_modes is NULL, which will happen if
    > partner->alt_modes is NULL at connection time. Semantically
    > this is different to typec_register_altmodes(), which does
    > have a NULL check.

    Yes, need to fix that.


    Thanks Guenter,

    --
    heikki

    \
     
     \ /
      Last update: 2016-05-25 14:41    [W:4.595 / U:0.444 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site