[PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Suman Tripathi]

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---

Suman Tripathi (1):
  ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC
    AHCI SATA Host Controller driver.

 drivers/ata/ahci_xgene.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--
1.8.2.1

[PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Suman Tripathi]

This patch fixes the error print invalid resource for the APM X-Gene
SoC AHCI SATA Host Controller driver. This print was due to the fact
that the controller 3 don't have a mux resource. This didn't result
in any errors but the print seems like meaningless.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---
 drivers/ata/ahci_xgene.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 40d0a76..9404db0c 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -434,7 +434,7 @@ static int xgene_ahci_mux_select(struct xgene_ahci_context *ctx)
 	u32 val;

 	/* Check for optional MUX resource */
-	if (IS_ERR(ctx->csr_mux))
+	if (!ctx->csr_mux)
 		return 0;

 	val = readl(ctx->csr_mux + SATA_ENET_CONFIG_REG);
@@ -485,7 +485,7 @@ static int xgene_ahci_probe(struct platform_device *pdev)

 	/* Retrieve the optional IP mux resource */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
-	ctx->csr_mux = devm_ioremap_resource(dev, res);
+	ctx->csr_mux = res ? devm_ioremap_resource(dev, res) : NULL;

 	dev_dbg(dev, "VAddr 0x%p Mmio VAddr 0x%p\n", ctx->csr_core,
 		hpriv->mmio);
--
1.8.2.1

Re: [PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Tejun Heo]

On Fri, Sep 12, 2014 at 01:24:08PM +0530, Suman Tripathi wrote:
> This patch fixes the error print invalid resource for the APM X-Gene
> SoC AHCI SATA Host Controller driver. This print was due to the fact
> that the controller 3 don't have a mux resource. This didn't result
> in any errors but the print seems like meaningless.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Suman Tripathi <stripathi@apm.com>
> ---
>  drivers/ata/ahci_xgene.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
> index 40d0a76..9404db0c 100644
> --- a/drivers/ata/ahci_xgene.c
> +++ b/drivers/ata/ahci_xgene.c
> @@ -434,7 +434,7 @@ static int xgene_ahci_mux_select(struct xgene_ahci_context *ctx)
>  	u32 val;
> 
>  	/* Check for optional MUX resource */
> -	if (IS_ERR(ctx->csr_mux))
> +	if (!ctx->csr_mux)
>  		return 0;

Hmmm?  So, if devm_ioremap_resource() call actually fails, now the
function tries to operation on ERR_PTR() value as a real pointer?

Thanks.

-- 
tejun

[PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Tejun Heo]

On Fri, Sep 12, 2014 at 01:24:08PM +0530, Suman Tripathi wrote:
> This patch fixes the error print invalid resource for the APM X-Gene
> SoC AHCI SATA Host Controller driver. This print was due to the fact
> that the controller 3 don't have a mux resource. This didn't result
> in any errors but the print seems like meaningless.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Suman Tripathi <stripathi@apm.com>
> ---
>  drivers/ata/ahci_xgene.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
> index 40d0a76..9404db0c 100644
> --- a/drivers/ata/ahci_xgene.c
> +++ b/drivers/ata/ahci_xgene.c
> @@ -434,7 +434,7 @@ static int xgene_ahci_mux_select(struct xgene_ahci_context *ctx)
>  	u32 val;
> 
>  	/* Check for optional MUX resource */
> -	if (IS_ERR(ctx->csr_mux))
> +	if (!ctx->csr_mux)
>  		return 0;

Hmmm?  So, if devm_ioremap_resource() call actually fails, now the
function tries to operation on ERR_PTR() value as a real pointer?

Thanks.

-- 
tejun

Re: [PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Tejun Heo]

Hello,

On Sun, Sep 14, 2014 at 11:36:51AM +0530, Suman Tripathi wrote:
> We can  maintain same piece (IS_ERR(ctx->csr_mux)), then we can do the
> below instead of NULL ??
> 
> ctx->csr_mux = res ? devm_ioremap_resource(dev, res) : ERR_PTR(-EINVAL);

Setting it to NULL on failure would probably make more sense.  No need
to carry around ERR_PTR() value around.

Thanks.

-- 
tejun

[PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Tejun Heo]

Hello,

On Sun, Sep 14, 2014 at 11:36:51AM +0530, Suman Tripathi wrote:
> We can  maintain same piece (IS_ERR(ctx->csr_mux)), then we can do the
> below instead of NULL ??
> 
> ctx->csr_mux = res ? devm_ioremap_resource(dev, res) : ERR_PTR(-EINVAL);

Setting it to NULL on failure would probably make more sense.  No need
to carry around ERR_PTR() value around.

Thanks.

-- 
tejun

Re: [PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Tejun Heo]

On Mon, Sep 15, 2014 at 01:10:01PM +0530, Suman Tripathi wrote:
> [suman] : So the posted version is acceptable ? Any others comments on this
> patch ?

I'm suggesting setting ctx->cs_mux to NULL on failure.  IOW,

	if (res) {
		ctx->csr_mux = devm_ioremap_resources();
		if (IS_ERR(ctx->csr_mux)) {
			print warning or something;
			ctx->csr_mux = NULL;
		}
	}

Thanks.

-- 
tejun

[PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Tejun Heo]

On Mon, Sep 15, 2014 at 01:10:01PM +0530, Suman Tripathi wrote:
> [suman] : So the posted version is acceptable ? Any others comments on this
> patch ?

I'm suggesting setting ctx->cs_mux to NULL on failure.  IOW,

	if (res) {
		ctx->csr_mux = devm_ioremap_resources();
		if (IS_ERR(ctx->csr_mux)) {
			print warning or something;
			ctx->csr_mux = NULL;
		}
	}

Thanks.

-- 
tejun

Re: [PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Russell King - ARM Linux]

On Mon, Sep 15, 2014 at 04:44:52PM +0900, Tejun Heo wrote:
> On Mon, Sep 15, 2014 at 01:10:01PM +0530, Suman Tripathi wrote:
> > [suman] : So the posted version is acceptable ? Any others comments on this
> > patch ?
> 
> I'm suggesting setting ctx->cs_mux to NULL on failure.  IOW,
> 
> 	if (res) {
> 		ctx->csr_mux = devm_ioremap_resources();
> 		if (IS_ERR(ctx->csr_mux)) {
> 			print warning or something;
> 			ctx->csr_mux = NULL;
> 		}
> 	}

A much better approach is:

	if (res) {
		void __iomem *csr = devm_ioremap_resources();
		if (IS_ERR(csr)) {
			ret = ERR_PTR(csr);
			dev_xxx();
			goto err;
		}

		ctx->csr_mux = csr;
	}

Then you never end up in the situation where csr_mux contains an error
pointer value - and is much more obvious that is the case.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Russell King - ARM Linux]

On Mon, Sep 15, 2014 at 04:44:52PM +0900, Tejun Heo wrote:
> On Mon, Sep 15, 2014 at 01:10:01PM +0530, Suman Tripathi wrote:
> > [suman] : So the posted version is acceptable ? Any others comments on this
> > patch ?
> 
> I'm suggesting setting ctx->cs_mux to NULL on failure.  IOW,
> 
> 	if (res) {
> 		ctx->csr_mux = devm_ioremap_resources();
> 		if (IS_ERR(ctx->csr_mux)) {
> 			print warning or something;
> 			ctx->csr_mux = NULL;
> 		}
> 	}

A much better approach is:

	if (res) {
		void __iomem *csr = devm_ioremap_resources();
		if (IS_ERR(csr)) {
			ret = ERR_PTR(csr);
			dev_xxx();
			goto err;
		}

		ctx->csr_mux = csr;
	}

Then you never end up in the situation where csr_mux contains an error
pointer value - and is much more obvious that is the case.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Tejun Heo]

On Mon, Sep 22, 2014 at 06:31:33PM +0530, Suman Tripathi wrote:
> This patch fixes the error print invalid resource for the APM X-Gene
> SoC AHCI SATA Host Controller driver. This print was due to the fact
> that the controller 3 don't have a mux resource. This didn't result
> in any errors but the print seems like meaningless.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Suman Tripathi <stripathi@apm.com>

Applied to libata/for-3.18.

Thanks.

-- 
tejun

[PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Tejun Heo]

On Mon, Sep 22, 2014 at 06:31:33PM +0530, Suman Tripathi wrote:
> This patch fixes the error print invalid resource for the APM X-Gene
> SoC AHCI SATA Host Controller driver. This print was due to the fact
> that the controller 3 don't have a mux resource. This didn't result
> in any errors but the print seems like meaningless.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Suman Tripathi <stripathi@apm.com>

Applied to libata/for-3.18.

Thanks.

-- 
tejun

[PATCH] ahci_xgene: Fix the error print invalid resource for APM X-Gene SoC AHCI SATA Host Controller driver.

[Suman Tripathi]

This patch fixes the error print invalid resource for the APM X-Gene
SoC AHCI SATA Host Controller driver. This print was due to the fact
that the controller 3 don't have a mux resource. This didn't result
in any errors but the print seems like meaningless.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---
 drivers/ata/ahci_xgene.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 40d0a76..26f1f78 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -434,7 +434,7 @@ static int xgene_ahci_mux_select(struct xgene_ahci_context *ctx)
 	u32 val;

 	/* Check for optional MUX resource */
-	if (IS_ERR(ctx->csr_mux))
+	if (!ctx->csr_mux)
 		return 0;

 	val = readl(ctx->csr_mux + SATA_ENET_CONFIG_REG);
@@ -485,7 +485,13 @@ static int xgene_ahci_probe(struct platform_device *pdev)

 	/* Retrieve the optional IP mux resource */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
-	ctx->csr_mux = devm_ioremap_resource(dev, res);
+	if (res) {
+		void __iomem *csr = devm_ioremap_resource(dev, res);
+		if (IS_ERR(csr))
+			return PTR_ERR(csr);
+
+		ctx->csr_mux = csr;
+	}

 	dev_dbg(dev, "VAddr 0x%p Mmio VAddr 0x%p\n", ctx->csr_core,
 		hpriv->mmio);
--
1.8.2.1