Received: by 2002:a25:6193:0:0:0:0:0 with SMTP id v141csp578860ybb; Wed, 1 Apr 2020 05:58:37 -0700 (PDT) X-Google-Smtp-Source: APiQypLxaOAgZ8mgtswa9nMzEVYcOfYzy28d7CPsLeqjSE1ybylJjigcLdLM18FH533D6DRKFet1 X-Received: by 2002:aca:f491:: with SMTP id s139mr2698673oih.128.1585745917736; Wed, 01 Apr 2020 05:58:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1585745917; cv=none; d=google.com; s=arc-20160816; b=Q5TFiDJwT89fromhzf2KyrIVy5v7fUtcgOLJMebefBGxrjZaSeB2eOsb7aZQgXK7LD GQ4pAFFMEZkB7T6gXI1uaTD4NqhefN2/DqKbu9LHumG9HfvgTnXKAhFup21QpvteSb23 J3Esl1+6De5d0xWRARULPXsnUigBvPXCnztofEUo+9Z8qlst+exOXnTNOqXT6QMV2I64 7a2R1C+KAi7NAEA+rvta2CHzPuSK5tMwza4Tb2H2RfJClhts8n35wzxmcGP2mUBJXFMI KDJ5+FPHLC+sD3epRCjzN/s3Pyz7FtT4if/mhH6RqdqtwwCaNSO9rtbLTmUTtTHdLKba IzmQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature; bh=FWzPy1PyCW/ZztUL3W8kHuieFwk92pfoEytt2EYdRMU=; b=JjcNoDCPNopfC5cEeU7D5zDBeB7Ohyd1XS1txgoZoeUAefC2R2NyIAU8OTNrN/acUr 6869Jwde0d6Bh8s33YGQq08644Gq/AJStRVPZWPwa1L61kzEjAaAwJkxjV4PWP1EbbQm uJSTKSNfYy8ZBWDO7iK6nvqgr4mxTYchjWXRoAJLAoY99ulzSgD4U6MwU7q2gwKVvLlz ZkbzXqSs9q/fZbHaTh7HQCKCppK3ASI4qq5VVVOQt7X9SLJth73FcTnmP5x4iySRjIz/ G+iNo2/WWQiFFXymhvEINWprSPrOCY4gg7mNaOCn2zaB65dRNyhLXjUUsMZsVFxfoaF1 +R+w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=tt1PrdNJ; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id k10si780243otr.174.2020.04.01.05.58.25; Wed, 01 Apr 2020 05:58:37 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=tt1PrdNJ; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732570AbgDAM5z (ORCPT + 99 others); Wed, 1 Apr 2020 08:57:55 -0400 Received: from mail.kernel.org ([198.145.29.99]:50518 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732507AbgDAM5y (ORCPT ); Wed, 1 Apr 2020 08:57:54 -0400 Received: from localhost (unknown [122.167.76.164]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 932C920658; Wed, 1 Apr 2020 12:57:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1585745873; bh=/7CdyfHR1LqxImi2jBxcvtSv+PPjOxapBSIg4fyZ97w=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=tt1PrdNJE62LxSxlqYUgjLQIfVkELT30zWfC2MfSXjWjYTpZqAkrPeDF7RffkyPit H1Ptvid/MNDvzvBsXUAbCbXcWSvzTL4vR0+1gWz4EJ92c4RxdCCYIGB9zv0YoyUEEw f3V1AoywopTN2aXyHjbEaFWPMizhhX4AZAtltrls= Date: Wed, 1 Apr 2020 18:27:48 +0530 From: Vinod Koul To: Mathias Nyman Cc: Mathias Nyman , Greg Kroah-Hartman , linux-arm-msm@vger.kernel.org, Bjorn Andersson , Yoshihiro Shimoda , Christian Lamparter , John Stultz , Alan Stern , Andreas =?iso-8859-1?Q?B=F6hler?= , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v8 3/5] usb: xhci: Add support for Renesas controller with memory Message-ID: <20200401125748.GC72691@vkoul-mobl> References: <20200323170601.419809-1-vkoul@kernel.org> <20200323170601.419809-4-vkoul@kernel.org> <6ea778a7-6d58-6dae-bd65-3a63a945fb97@linux.intel.com> <20200326115117.GZ72691@vkoul-mobl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200326115117.GZ72691@vkoul-mobl> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 26-03-20, 17:21, Vinod Koul wrote: > On 26-03-20, 13:29, Mathias Nyman wrote: > > Hi Vinod > > > > On 23.3.2020 19.05, Vinod Koul wrote: > > > Some rensas controller like uPD720201 and uPD720202 need firmware to be > > > loaded. Add these devices in table and invoke renesas firmware loader > > > functions to check and load the firmware into device memory when > > > required. > > > > > > Signed-off-by: Vinod Koul > > > --- > > > drivers/usb/host/xhci-pci-renesas.c | 1 + > > > drivers/usb/host/xhci-pci.c | 29 ++++++++++++++++++++++++++++- > > > drivers/usb/host/xhci-pci.h | 3 +++ > > > 3 files changed, 32 insertions(+), 1 deletion(-) > > > > > > > It's unfortunate if firmware loading couldn't be initiated in a PCI fixup hook > > for this Renesas controller. What was the reason it failed? > > > > Nicolas Saenz Julienne just submitted a solution like that for Raspberry Pi 4 > > where firmware loading is initiated in pci-quirks.c quirk_usb_early_handoff() > > > > https://lore.kernel.org/lkml/20200324182812.20420-1-nsaenzjulienne@suse.de > > > > Is he doing something different than what was done for the Renesas controller? > > I tried and everytime ended up not getting firmware. Though I did not > investigate a lot. Christian seemed to have tested sometime back as > well. > > Another problem is that we dont get driver_data in the quirk and there > didnt seem a way to find the firmware name. > > > > diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c > > > index c588277ac9b8..d413d53df94b 100644 > > > --- a/drivers/usb/host/xhci-pci-renesas.c > > > +++ b/drivers/usb/host/xhci-pci-renesas.c > > > @@ -336,6 +336,7 @@ static void renesas_fw_callback(const struct firmware *fw, > > > goto cleanup; > > > } > > > > > > + xhci_pci_probe(pdev, ctx->id); > > > return; > > > > I haven't looked into this but instead of calling xhci_pci_probe() here in the async fw > > loading callback could we just return -EPROBE_DEFER until firmware is loaded when > > xhci_pci_probe() is originally called? > > Hmm, initially my thinking was how to tell device core to probe again, > and then digging up I saw wait_for_device_probe() which can be used, let > me try that Sorry to report back that it doesn't work as planned :( I modified the code to invoke the request_firmware_nowait() which will load the firmware and provide the firmware in callback. Meanwhile return -EPROBE_DEFER. After a bit, the core invokes the driver probe again and we hit the roadblock. The request_firmware uses devres and allocates resources for loading the firmware. The problem is that device core checks for this: bus: 'pci': really_probe: probing driver xhci_hcd_pci with device 0000:01:00.0 pci 0000:01:00.0: Resources present before probing And here the probe fails. In some cases the firmware_callback finishes before this and we can probe again, but that is not very reliable. I tested another way to use request_firmware() (sync version) and then load the firmware in probe and load. The request is done only for renesas devices if they dont have firmware already running. So rest of the devices wont have any impact. Now should we continue this way in the patchset or move to sync version. Am okay either way. -- ~Vinod