[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170409151614.GA23984@roeck-us.net>
Date: Sun, 9 Apr 2017 08:16:14 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Mats Karrman <mats.dev.list@...il.com>
Cc: Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Greg KH <gregkh@...uxfoundation.org>,
Felipe Balbi <felipe.balbi@...ux.intel.com>,
linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: USB Type-C Port Manager API concern
Hi Mats,
On Sun, Apr 09, 2017 at 01:09:57AM +0200, Mats Karrman wrote:
> Hi Guenter,
>
> From a previous thread:
>
> On 03/08/2017 02:38 AM, Guenter Roeck wrote:
> >On 03/07/2017 02:30 PM, Mats Karrman wrote:
> >[ ... ]
> >
> >>
> >>I'm still struggling to catch up on what you guys have been up to during
> >>the
> >>last year or so :-) and came across some patches of Guenter from last
> >>October:
> >>
> >>http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1243527.html
> >>
> >>What happened to them? Has there been any progress since then?
> >>
> >
> >Updates to keep in sync with API changes, bug fixes, and minor
> >improvements,
> >for the most part. I can post a current version if there is interest.
> >The latest version is also available from
> >https://chromium-review.googlesource.com/#/c/389917/
>
> I'm working on a tcpi driver and have some concern about the tcpm api.
> The tcpm_register_port() is typically called from the probe function of
> tcpi driver where the tcpm_port reference returned is stored in the
> driver private data. The problem I ran into is that tcpm_register_port()
> calls back to the not yet fully initialized tcpi driver, causing null-
> pointer dereferences. This could of course be solved by extra logic in
> the tcpi driver but I think it would be more elegant if the registration
> of a port could be free of premature callbacks. E.g. it could be required
> that the tcpi driver probe called tcpm_tcpc_reset() once it's done
> initializing or the necessary data could be supplied in the call to
> tcpm_register_port().
> What do you think?
Let me think about it. In theory it should be possible to avoid callbacks into
the underlying driver until after the return from the registration code, but
that would still be racy if the underlying driver is not ready.
Basic problem seems to be that an API in general assumes that the caller is
ready to serve it once it registers itself. It is kind of unusual to have two
calls, one to register the driver and one to tell the infrastructure that it
is ready (which I assume your reset call would do). Ultimately this means
that the tcpm driver would have to have additional logic to identify if the
underlying driver is ready to handle callbacks.
Can you delay tcpm registration until after the underlying driver is ready,
ie typically to the end of its probe function ? Or am I misunderstanding
your problem ?
Thanks,
Guenter
Powered by blists - more mailing lists