From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753163AbdEPOiO (ORCPT ); Tue, 16 May 2017 10:38:14 -0400 Received: from mail.kernel.org ([198.145.29.99]:46160 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752316AbdEPOiK (ORCPT ); Tue, 16 May 2017 10:38:10 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 01CAB239D4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=robh+dt@kernel.org MIME-Version: 1.0 In-Reply-To: <20170516133838.GA20659@hc> References: <20170516093655.17746-1-jglauber@cavium.com> <20170516093655.17746-6-jglauber@cavium.com> <20170516133838.GA20659@hc> From: Rob Herring Date: Tue, 16 May 2017 09:37:48 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 5/5] mmc: cavium: Fix probing race with regulator To: Jan Glauber Cc: Ulf Hansson , David Daney , Frank Rowand , "Steven J . Hill" , "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 16, 2017 at 8:38 AM, Jan Glauber wrote: > On Tue, May 16, 2017 at 08:07:50AM -0500, Rob Herring wrote: >> On Tue, May 16, 2017 at 4:36 AM, Jan Glauber wrote: >> > If the regulator probing is not yet finished this driver >> > might catch a -EPROBE_DEFER. Returning after this condition >> > did not remove the created platform device. On a repeated >> > call to the probe function the of_platform_device_create >> > fails. >> > >> > Calling of_platform_device_destroy after EPROBE_DEFER resolves >> > this bug. >> > >> > Signed-off-by: Jan Glauber >> > --- >> > drivers/mmc/host/cavium-thunderx.c | 4 +++- >> > 1 file changed, 3 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/mmc/host/cavium-thunderx.c b/drivers/mmc/host/cavium-thunderx.c >> > index fe3d772..257535e 100644 >> > --- a/drivers/mmc/host/cavium-thunderx.c >> > +++ b/drivers/mmc/host/cavium-thunderx.c >> > @@ -137,8 +137,10 @@ static int thunder_mmc_probe(struct pci_dev *pdev, >> > continue; >> > >> > ret = cvm_mmc_of_slot_probe(&host->slot_pdev[i]->dev, host); >> > - if (ret) >> > + if (ret) { >> > + of_platform_device_destroy(&host->slot_pdev[i]->dev, NULL); >> >> What if this fails after the 1st iteration of the loop. It's only >> cleaning up the current device. > > The platform device is just a dummy device created directly before > cvm_mmc_of_slot_probe(). So there is no need to cleanup anything else. So if you have 2 slots, the first slot probes successfully and the 2nd slot defers, then you only need to clean-up the 2nd device/slot? Looks to me like you are leaking the 1st device you alloc. > > As far as I've seen it the platform code 'tags' the nodes it already > used, but I need the same node to be parsed again on -EPROBE_DEFER. > >> Use devm_of_platform_populate or >> of_platform_populate/of_platform_depopulate instead. > > I'm not sure one of these will work here. Those functions loop over child nodes and create devices. You are doing the same thing. You'd just need to create all the devices first and then probe them all. The whole structure here with the dummy devices and how you are initializing things is screwy. IMO, you should be creating actual drivers for the dummy devices. Rob