Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753566AbcKUJHy (ORCPT ); Mon, 21 Nov 2016 04:07:54 -0500 Received: from mail-qt0-f171.google.com ([209.85.216.171]:34700 "EHLO mail-qt0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752316AbcKUJHx (ORCPT ); Mon, 21 Nov 2016 04:07:53 -0500 MIME-Version: 1.0 In-Reply-To: <75e1a8a4-062b-28db-3743-b16fd8c98fa0@lechnology.com> References: <20161114144103.12120-2-ahaslam@baylibre.com> <75e1a8a4-062b-28db-3743-b16fd8c98fa0@lechnology.com> From: Axel Haslam Date: Mon, 21 Nov 2016 10:07:11 +0100 Message-ID: Subject: Re: [v5,1/5] USB: ohci: da8xx: use ohci priv data instead of globals To: David Lechner Cc: Greg KH , Alan Stern , Kevin Hilman , kishon@ti.com, linux-usb@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 Content-Length: 2872 Lines: 84 On Sun, Nov 20, 2016 at 3:58 AM, David Lechner wrote: > On 11/14/2016 08:40 AM, ahaslam@baylibre.com wrote: >> >> Instead of global variables, use the extra_priv_size of >> the ohci driver. >> >> We cannot yet move the ocic mask because this is used on >> the interrupt handler which is registerded through platform >> data and does not have an hcd pointer. This will be moved >> on a later patch. >> >> Signed-off-by: Axel Haslam >> --- >> drivers/usb/host/ohci-da8xx.c | 73 >> +++++++++++++++++++++++++------------------ >> 1 file changed, 43 insertions(+), 30 deletions(-) >> >> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c >> index b3de8bc..438970b 100644 >> --- a/drivers/usb/host/ohci-da8xx.c >> +++ b/drivers/usb/host/ohci-da8xx.c > > ... >> >> @@ -238,25 +246,29 @@ static int ohci_da8xx_probe(struct platform_device >> *pdev) >> if (hub == NULL) >> return -ENODEV; >> >> - usb11_clk = devm_clk_get(&pdev->dev, "usb11"); >> - if (IS_ERR(usb11_clk)) { >> - if (PTR_ERR(usb11_clk) != -EPROBE_DEFER) >> + hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev, >> + dev_name(&pdev->dev)); >> + if (!hcd) >> + return -ENOMEM; >> + >> + da8xx_ohci = to_da8xx_ohci(hcd); >> + >> + da8xx_ohci->usb11_clk = devm_clk_get(&pdev->dev, "usb11"); >> + if (IS_ERR(da8xx_ohci->usb11_clk)) { >> + if (PTR_ERR(da8xx_ohci->usb11_clk) != -EPROBE_DEFER) >> dev_err(&pdev->dev, "Failed to get clock.\n"); >> - return PTR_ERR(usb11_clk); >> + error = PTR_ERR(da8xx_ohci->usb11_clk); >> + goto err; >> } > > > Small thing, but this could be written slightly cleaner as: > > da8xx_ohci->usb11_clk = devm_clk_get(&pdev->dev, "usb11"); > if (IS_ERR(da8xx_ohci->usb11_clk)) { > error = PTR_ERR(da8xx_ohci->usb11_clk); > if (error != -EPROBE_DEFER) > dev_err(&pdev->dev, "Failed to get clock.\n"); > goto err; > } > Yes, right, i will change it, Thanks. >> >> - usb11_phy = devm_phy_get(&pdev->dev, "usb-phy"); >> - if (IS_ERR(usb11_phy)) { >> - if (PTR_ERR(usb11_phy) != -EPROBE_DEFER) >> + da8xx_ohci->usb11_phy = devm_phy_get(&pdev->dev, "usb-phy"); >> + if (IS_ERR(da8xx_ohci->usb11_phy)) { >> + if (PTR_ERR(da8xx_ohci->usb11_phy) != -EPROBE_DEFER) >> dev_err(&pdev->dev, "Failed to get phy.\n"); >> - return PTR_ERR(usb11_phy); >> + error = PTR_ERR(da8xx_ohci->usb11_phy); >> + goto err; >> } > > > same here > > ... > > > Tested-by: David Lechner >