Re: [PATCH 1/2] usb: dwc3: use local copy of resource to fix-up register offset

From: Masami Hiramatsu <mhiramat@kernel.org>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>,
	linux-usb@vger.kernel.org, Masami Hiramatsu <mhiramat@kernel.org>,
	Jassi Brar <jaswinder.singh@linaro.org>,
	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Felipe Balbi <balbi@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] usb: dwc3: use local copy of resource to fix-up register offset
Date: Fri, 16 Mar 2018 10:46:04 +0900	[thread overview]
Message-ID: <20180316104604.1a88e9854cd9fb5809808eda@kernel.org> (raw)
In-Reply-To: <1521113998-25052-1-git-send-email-yamada.masahiro@socionext.com>

On Thu, 15 Mar 2018 20:39:57 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> It is not a good idea to modify the resource from the platform device.
> Modify its local copy to pass it to devm_ioremap_resource() so that we
> do not need to restore it in the failure path and the remove hook.
> 

This looks good to me.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks,

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  drivers/usb/dwc3/core.c | 32 ++++++++------------------------
>  1 file changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index f1d838a..e9083a3 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1164,7 +1164,7 @@ static void dwc3_check_params(struct dwc3 *dwc)
>  static int dwc3_probe(struct platform_device *pdev)
>  {
>  	struct device		*dev = &pdev->dev;
> -	struct resource		*res;
> +	struct resource		*res, dwc_res;
>  	struct dwc3		*dwc;
>  
>  	int			ret;
> @@ -1189,20 +1189,19 @@ static int dwc3_probe(struct platform_device *pdev)
>  	dwc->xhci_resources[0].flags = res->flags;
>  	dwc->xhci_resources[0].name = res->name;
>  
> -	res->start += DWC3_GLOBALS_REGS_START;
> -
>  	/*
>  	 * Request memory region but exclude xHCI regs,
>  	 * since it will be requested by the xhci-plat driver.
>  	 */
> -	regs = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(regs)) {
> -		ret = PTR_ERR(regs);
> -		goto err0;
> -	}
> +	dwc_res = *res;
> +	dwc_res.start += DWC3_GLOBALS_REGS_START;
> +
> +	regs = devm_ioremap_resource(dev, &dwc_res);
> +	if (IS_ERR(regs))
> +		return PTR_ERR(regs);
>  
>  	dwc->regs	= regs;
> -	dwc->regs_size	= resource_size(res);
> +	dwc->regs_size	= resource_size(&dwc_res);
>  
>  	dwc3_get_properties(dwc);
>  
> @@ -1269,29 +1268,14 @@ static int dwc3_probe(struct platform_device *pdev)
>  	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  
> -err0:
> -	/*
> -	 * restore res->start back to its original value so that, in case the
> -	 * probe is deferred, we don't end up getting error in request the
> -	 * memory region the next time probe is called.
> -	 */
> -	res->start -= DWC3_GLOBALS_REGS_START;
> -
>  	return ret;
>  }
>  
>  static int dwc3_remove(struct platform_device *pdev)
>  {
>  	struct dwc3	*dwc = platform_get_drvdata(pdev);
> -	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  
>  	pm_runtime_get_sync(&pdev->dev);
> -	/*
> -	 * restore res->start back to its original value so that, in case the
> -	 * probe is deferred, we don't end up getting error in request the
> -	 * memory region the next time probe is called.
> -	 */
> -	res->start -= DWC3_GLOBALS_REGS_START;
>  
>  	dwc3_debugfs_exit(dwc);
>  	dwc3_core_exit_mode(dwc);
> -- 
> 2.7.4
> 

-- 
Masami Hiramatsu <mhiramat@kernel.org>