Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752781AbdDIVFg (ORCPT ); Sun, 9 Apr 2017 17:05:36 -0400 Received: from mail-lf0-f68.google.com ([209.85.215.68]:36506 "EHLO mail-lf0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752717AbdDIVF0 (ORCPT ); Sun, 9 Apr 2017 17:05:26 -0400 Subject: Re: USB Type-C Port Manager API concern To: Guenter Roeck References: <20170221142405.76299-3-heikki.krogerus@linux.intel.com> <4b4bbffc-db02-3b54-04bc-e7de79b2d9ed@roeck-us.net> <07618170-d561-e7fe-08e0-91316c53d832@gmail.com> <20170303125940.GA6999@kuha.fi.intel.com> <6ddb2eac-03d5-127e-df1e-ad189968e6b2@gmail.com> <20170306131442.GC6999@kuha.fi.intel.com> <696552a7-c36a-1d73-9517-543907e9da39@gmail.com> <9b32fabf-3ff5-112f-4249-a7024f808b20@roeck-us.net> <4fe455d8-30d9-0c21-46aa-6d273cd24d50@gmail.com> <20170409151614.GA23984@roeck-us.net> Cc: Heikki Krogerus , Greg KH , Felipe Balbi , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org From: Mats Karrman Message-ID: <949bc063-7002-c12c-ba56-8b46f9ea178b@gmail.com> Date: Sun, 9 Apr 2017 23:05:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170409151614.GA23984@roeck-us.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2554 Lines: 47 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