[PATCH] ata:ahci_xgene:use devm_platform_ioremap_resource() to simplify code

[PATCH] ata:ahci_xgene:use devm_platform_ioremap_resource() to simplify code

[Tang Bin]

In this function, devm_platform_ioremap_resource() should be suitable
to simplify code.

Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
---
 drivers/ata/ahci_xgene.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 16246c843..061209275 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -739,7 +739,6 @@ static int xgene_ahci_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
 	struct xgene_ahci_context *ctx;
-	struct resource *res;
 	const struct of_device_id *of_devid;
 	enum xgene_ahci_version version = XGENE_AHCI_V1;
 	const struct ata_port_info *ppi[] = { &xgene_ahci_v1_port_info,
@@ -759,32 +758,24 @@ static int xgene_ahci_probe(struct platform_device *pdev)
 	ctx->dev = dev;
 
 	/* Retrieve the IP core resource */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	ctx->csr_core = devm_ioremap_resource(dev, res);
+	ctx->csr_core = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(ctx->csr_core))
 		return PTR_ERR(ctx->csr_core);
 
 	/* Retrieve the IP diagnostic resource */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
-	ctx->csr_diag = devm_ioremap_resource(dev, res);
+	ctx->csr_diag = devm_platform_ioremap_resource(pdev, 2);
 	if (IS_ERR(ctx->csr_diag))
 		return PTR_ERR(ctx->csr_diag);
 
 	/* Retrieve the IP AXI resource */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
-	ctx->csr_axi = devm_ioremap_resource(dev, res);
+	ctx->csr_axi = devm_platform_ioremap_resource(pdev, 3);
 	if (IS_ERR(ctx->csr_axi))
 		return PTR_ERR(ctx->csr_axi);
 
 	/* Retrieve the optional IP mux resource */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
-	if (res) {
-		void __iomem *csr = devm_ioremap_resource(dev, res);
-		if (IS_ERR(csr))
-			return PTR_ERR(csr);
-
-		ctx->csr_mux = csr;
-	}
+	ctx->csr_mux = devm_platform_ioremap_resource(pdev, 4);
+	if (IS_ERR(ctx->csr_mux))
+		return PTR_ERR(ctx->csr_mux);
 
 	of_devid = of_match_device(xgene_ahci_of_match, dev);
 	if (of_devid) {
-- 
2.20.1.windows.1

Re: [PATCH] ata:ahci_xgene:use devm_platform_ioremap_resource() to simplify code

[Sergei Shtylyov]

Hello!

On 01.04.2020 11:49, Tang Bin wrote:

> In this function, devm_platform_ioremap_resource() should be suitable
> to simplify code.
> 
> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
> ---
>   drivers/ata/ahci_xgene.c | 21 ++++++---------------
>   1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
> index 16246c843..061209275 100644
> --- a/drivers/ata/ahci_xgene.c
> +++ b/drivers/ata/ahci_xgene.c
[...]
>   	/* Retrieve the optional IP mux resource */
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
> -	if (res) {
> -		void __iomem *csr = devm_ioremap_resource(dev, res);
> -		if (IS_ERR(csr))
> -			return PTR_ERR(csr);
> -
> -		ctx->csr_mux = csr;
> -	}
> +	ctx->csr_mux = devm_platform_ioremap_resource(pdev, 4);
> +	if (IS_ERR(ctx->csr_mux))
> +		return PTR_ERR(ctx->csr_mux);

    The previous code allowed the memory resource #4 to be absent, the new
code makes it mandatory? Is that intentional?

[...]

MBR, Sergei

Re: [PATCH] ata:ahci_xgene:use devm_platform_ioremap_resource() to simplify code

[Sergei Shtylyov]

Hello!

On 04/01/2020 12:44 PM, 唐彬 wrote:

>         I think the previous code in memory resource #4 to use 'csr' maybe useless，if
> devm_ioremap_resource() failed，the function will return, the 'ctx->csr_mux = csr'  will 
> become useless。So I do the same way as the other three。This's my idea, please criticize
> and correct it。Thank you very much!

   I was unable to understand what you mean here.
   My point is that the driver happily works if the MUX registers do not exist, and your
patch makes the driver fail the probe in this case. Even if this was correct, it's usually
a bad idea to do several thing in the same patch. So, you'd need a patch changing the probing logic before you convert things to devm_platfrom_ioremap_resource()...

[...]

MBR, Sergei