[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 <***@apm.com>
Signed-off-by: Suman Tripathi <***@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

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Suman Tripathi]

Signed-off-by: Loc Ho <***@apm.com>
Signed-off-by: Suman Tripathi <***@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

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tejun Heo]

Post by 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.
---
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 Heo]

Hello,

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 Heo]

[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.

[Russell King - ARM Linux]

Post by Tejun Heo

[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.