Messages in this thread |  | | Subject | Re: USB Type-C Port Manager API concern | From | Mats Karrman <> | Date | Sun, 9 Apr 2017 23:05:28 +0200 |
| |
On 04/09/2017 05:16 PM, Guenter Roeck wrote:
> Hi Mats, > > On Sun, Apr 09, 2017 at 01:09:57AM +0200, Mats Karrman wrote: >> 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 ?
The problem I had was that I was trying to pull the same trick that you do ;) I.e. the probe function calls tcpm_register_port() that is calling back to the driver that was trying to call back to tcpm, just that the call to tcpm_register_port() has not yet returned so the pointer to tcpm_port in the driver data structure was still null.
I was able to fix the issue by commenting out the call to tcpm_init() at the end of tcpm_register_port() and instead call ("your") tcpm_tcpc_reset(), that currently does nothing but calling tcpm_init(), after I'm done.
Even though I'm not overly excited about the tcpm register function calling back to the driver I don't think my fix is much better. I should live by my own words and refrain from calling back to tcpm until registration has finished...
// Mats
|  |