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
