[PATCH 1/3] mmc: omap_hsmmc: release correct resource

[PATCH 1/3] mmc: omap_hsmmc: release correct resource

[Venkatraman S]

From: Russell King - ARM Linux <linux@arm.linux.org.uk>

res can be one of several resources, as this variable is re-used several
times during probe.  This can cause the wrong resource parameters to be
passed to release_mem_region().

Get the original memory resource before calling release_mem_region().

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4254b6f..d15b149 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2042,7 +2042,9 @@ err1:
 err_alloc:
 	omap_hsmmc_gpio_free(pdata);
 err:
-	release_mem_region(res->start, resource_size(res));
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res)
+		release_mem_region(res->start, resource_size(res));
 	return ret;
 }
 
-- 
1.7.10.rc2

[PATCH 2/3] mmc: omap_hsmmc: Add fclk frequency to debufs

[Venkatraman S]

From: Viswanath Puttagunta <vishp@ti.com>

Add mmc functional clock frequency to list of
internal state variables to display for debug.

Signed-off-by: Viswanath Puttagunta <vishp@ti.com>
Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index d15b149..d046ba2 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1683,8 +1683,9 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
 	if (host->pdata->get_context_loss_count)
 		context_loss = host->pdata->get_context_loss_count(host->dev);
 
-	seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n\nregs:\n",
-			mmc->index, host->context_loss, context_loss);
+	seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n fclk:\t\t%lu Hz\n\nregs:\n",
+		mmc->index, host->context_loss, context_loss,
+			clk_get_rate(host->fclk));
 
 	if (host->suspended) {
 		seq_printf(s, "host suspended, can't read registers\n");
-- 
1.7.10.rc2

[PATCH 3/3] mmc: omap_hsmmc: If probe fails, give out error messages

[Venkatraman S]

From: Rajendra Nayak <rnayak@ti.com>

Giving out debug messages even in case of probe failure seems
not very useful. Make them error messages instead.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Venkatraman S <svenkatr@ti.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: <linux-mmc@vger.kernel.org>
---
 drivers/mmc/host/omap_hsmmc.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index d046ba2..6d3ceab 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1956,13 +1956,13 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 	ret = request_irq(host->irq, omap_hsmmc_irq, 0,
 			mmc_hostname(mmc), host);
 	if (ret) {
-		dev_dbg(mmc_dev(host->mmc), "Unable to grab HSMMC IRQ\n");
+		dev_err(mmc_dev(host->mmc), "Unable to grab HSMMC IRQ\n");
 		goto err_irq;
 	}
 
 	if (pdata->init != NULL) {
 		if (pdata->init(&pdev->dev) != 0) {
-			dev_dbg(mmc_dev(host->mmc),
+			dev_err(mmc_dev(host->mmc),
 				"Unable to configure MMC IRQs\n");
 			goto err_irq_cd_init;
 		}
@@ -1985,7 +1985,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 					   IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
 					   mmc_hostname(mmc), host);
 		if (ret) {
-			dev_dbg(mmc_dev(host->mmc),
+			dev_err(mmc_dev(host->mmc),
 				"Unable to grab MMC CD IRQ\n");
 			goto err_irq_cd;
 		}
-- 
1.7.10.rc2

Re: [PATCH 1/3] mmc: omap_hsmmc: release correct resource

[Russell King - ARM Linux]

On Wed, Apr 18, 2012 at 03:19:11PM +0530, Venkatraman S wrote:
> From: Russell King - ARM Linux <linux@arm.linux.org.uk>

Please correct this to "Russell King <rmk+kernel@arm.linux.org.uk>"

> 
> res can be one of several resources, as this variable is re-used several
> times during probe.  This can cause the wrong resource parameters to be
> passed to release_mem_region().
> 
> Get the original memory resource before calling release_mem_region().
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 4254b6f..d15b149 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -2042,7 +2042,9 @@ err1:
>  err_alloc:
>  	omap_hsmmc_gpio_free(pdata);
>  err:
> -	release_mem_region(res->start, resource_size(res));
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (res)
> +		release_mem_region(res->start, resource_size(res));
>  	return ret;
>  }
>  
> -- 
> 1.7.10.rc2
> 

Re: [PATCH 1/3] mmc: omap_hsmmc: release correct resource

[S, Venkatraman]

On Wed, Apr 18, 2012 at 3:28 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Apr 18, 2012 at 03:19:11PM +0530, Venkatraman S wrote:
>> From: Russell King - ARM Linux <linux@arm.linux.org.uk>
>
> Please correct this to "Russell King <rmk+kernel@arm.linux.org.uk>"
>
Ok.

[PATCH] mmc: omap_hsmmc: release correct resource

[Venkatraman S]

From: Russell King <rmk+kernel@arm.linux.org.uk>

res can be one of several resources, as this variable is re-used several
times during probe.  This can cause the wrong resource parameters to be
passed to release_mem_region().

Get the original memory resource before calling release_mem_region().

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4254b6f..d15b149 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2042,7 +2042,9 @@ err1:
 err_alloc:
 	omap_hsmmc_gpio_free(pdata);
 err:
-	release_mem_region(res->start, resource_size(res));
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res)
+		release_mem_region(res->start, resource_size(res));
 	return ret;
 }
 
-- 
1.7.10.rc2

Re: [PATCH 2/3] mmc: omap_hsmmc: Add fclk frequency to debufs

[Chris Ball]

Hi,

On Wed, Apr 18 2012, Venkatraman S wrote:
> From: Viswanath Puttagunta <vishp@ti.com>
>
> Add mmc functional clock frequency to list of
> internal state variables to display for debug.
>
> Signed-off-by: Viswanath Puttagunta <vishp@ti.com>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index d15b149..d046ba2 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1683,8 +1683,9 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
>  	if (host->pdata->get_context_loss_count)
>  		context_loss = host->pdata->get_context_loss_count(host->dev);
>  
> -	seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n\nregs:\n",
> -			mmc->index, host->context_loss, context_loss);
> +	seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n fclk:\t\t%lu Hz\n\nregs:\n",
> +		mmc->index, host->context_loss, context_loss,

Minor nit, but it looks like this line breaks indentation.

> +			clk_get_rate(host->fclk));
>  
>  	if (host->suspended) {
>  		seq_printf(s, "host suspended, can't read registers\n");

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 2/3] mmc: omap_hsmmc: Add fclk frequency to debufs

[S, Venkatraman]

On Thu, Apr 19, 2012 at 5:34 AM, Chris Ball <cjb@laptop.org> wrote:
> Hi,
>
> On Wed, Apr 18 2012, Venkatraman S wrote:
>> From: Viswanath Puttagunta <vishp@ti.com>
>>
>> Add mmc functional clock frequency to list of
>> internal state variables to display for debug.
>>
>> Signed-off-by: Viswanath Puttagunta <vishp@ti.com>
>> Signed-off-by: Venkatraman S <svenkatr@ti.com>
>> ---
>>  drivers/mmc/host/omap_hsmmc.c |    5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index d15b149..d046ba2 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -1683,8 +1683,9 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
>>       if (host->pdata->get_context_loss_count)
>>               context_loss = host->pdata->get_context_loss_count(host->dev);
>>
>> -     seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n\nregs:\n",
>> -                     mmc->index, host->context_loss, context_loss);
>> +     seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n fclk:\t\t%lu Hz\n\nregs:\n",
>> +             mmc->index, host->context_loss, context_loss,
>
> Minor nit, but it looks like this line breaks indentation.

It does ? I thought I actually fixed the indentation of the original
patch by removing a tabstop, as it was going
too far into the right.
checkpatch.pl doesn't complain about it.

>
>> +                     clk_get_rate(host->fclk));
>>
>>       if (host->suspended) {
>>               seq_printf(s, "host suspended, can't read registers\n");
>
> Thanks,
>
> - Chris.
> --
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] mmc: omap_hsmmc: Add fclk frequency to debufs

[Chris Ball]

Hi,

On Thu, Apr 19 2012, S, Venkatraman wrote:
>>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>>> index d15b149..d046ba2 100644
>>> --- a/drivers/mmc/host/omap_hsmmc.c
>>> +++ b/drivers/mmc/host/omap_hsmmc.c
>>> @@ -1683,8 +1683,9 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
>>>       if (host->pdata->get_context_loss_count)
>>>               context_loss = host->pdata->get_context_loss_count(host->dev);
>>>
>>> -     seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n\nregs:\n",
>>> -                     mmc->index, host->context_loss, context_loss);
>>> +     seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n fclk:\t\t%lu Hz\n\nregs:\n",
>>> +             mmc->index, host->context_loss, context_loss,
>>
>> Minor nit, but it looks like this line breaks indentation.
>
> It does ? I thought I actually fixed the indentation of the original
> patch by removing a tabstop, as it was going
> too far into the right.
> checkpatch.pl doesn't complain about it.

After applying it, omap_hsmmc.c looks like this here:

        seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n fclk:\t\t%lu Hz\n\nregs:\n",
                mmc->index, host->context_loss, context_loss,
                        clk_get_rate(host->fclk));

(That's with tabs converted to spaces for e-mail.)

So you're right that the original lines are indented a character too far
to the right, but after your patch there are different indentations used
for lines 2 and 3 above, so that's worse than it was before.

All of the seq_printf() calls in omap_hsmmc.c have the same indentation
one space too far to the right.  I guess we should just stick with it.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH 1/2] mmc: omap_hsmmc: Add fclk frequency to debufs

[Venkatraman S]

From: Viswanath Puttagunta <vishp@ti.com>

Add mmc functional clock frequency to list of
internal state variables to display for debug.

Signed-off-by: Viswanath Puttagunta <vishp@ti.com>
Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
	v1 -> v2: Restored indentation to be consistent
		  as the rest of the file.
 drivers/mmc/host/omap_hsmmc.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index d15b149..5d0ce20 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1683,8 +1683,9 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
 	if (host->pdata->get_context_loss_count)
 		context_loss = host->pdata->get_context_loss_count(host->dev);
 
-	seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n\nregs:\n",
-			mmc->index, host->context_loss, context_loss);
+	seq_printf(s, "mmc%d:\n ctx_loss:\t%d:%d\n fclk:\t\t%lu Hz\n\nregs:\n",
+			mmc->index, host->context_loss, context_loss,
+			clk_get_rate(host->fclk));
 
 	if (host->suspended) {
 		seq_printf(s, "host suspended, can't read registers\n");
-- 
1.7.10.rc2