[PATCH] sdhci: fix undue iomem warning

[PATCH] sdhci: fix undue iomem warning

[Daniel J Blueman]

Some newer SDHCI controllers have memory mapped I/O regions of 512
bytes, so accept these without warning.

Signed-off-by: Daniel J Blueman <daniel.blueman@gmail.com>
---
 drivers/mmc/host/sdhci-pci.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index 936bbca..ae948b0 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -918,8 +918,9 @@ static struct sdhci_pci_slot * __devinit sdhci_pci_probe_slot(
 		return ERR_PTR(-ENODEV);
 	}
 
-	if (pci_resource_len(pdev, bar) != 0x100) {
-		dev_err(&pdev->dev, "Invalid iomem size. You may "
+	int len = pci_resource_len(pdev, bar);
+	if (len != 0x100 && len != 0x200) {
+		dev_warn(&pdev->dev, "Invalid iomem size. You may "
 			"experience problems.\n");
 	}
 
-- 
1.7.4.1

Re: [PATCH] sdhci: fix undue iomem warning

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

Hi Daniel,

> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
> index 936bbca..ae948b0 100644
> --- a/drivers/mmc/host/sdhci-pci.c
> +++ b/drivers/mmc/host/sdhci-pci.c
> @@ -918,8 +918,9 @@ static struct sdhci_pci_slot * __devinit sdhci_pci_probe_slot(
>  		return ERR_PTR(-ENODEV);
>  	}
>  
> -	if (pci_resource_len(pdev, bar) != 0x100) {
> -		dev_err(&pdev->dev, "Invalid iomem size. You may "
> +	int len = pci_resource_len(pdev, bar);
> +	if (len != 0x100 && len != 0x200) {

Hmmm,

a) SDHC Specs (even v3) only mention 0x100, so this _is_ the standard.
   Do the new cards (which ones?) have anything located in the extra
   area?
b) your approach won't scale very well

so, I'd say it is better to keep the old way.

> +		dev_warn(&pdev->dev, "Invalid iomem size. You may "
>  			"experience problems.\n");

I second turning the message into a warning, though.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] sdhci: fix undue iomem warning

[Daniel J Blueman]

Hi Wolfram,

On 30 May 2011 20:58, Wolfram Sang <w.sang@pengutronix.de> wrote:
> Hi Daniel,
>
>> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
>> index 936bbca..ae948b0 100644
>> --- a/drivers/mmc/host/sdhci-pci.c
>> +++ b/drivers/mmc/host/sdhci-pci.c
>> @@ -918,8 +918,9 @@ static struct sdhci_pci_slot * __devinit sdhci_pci_probe_slot(
>>               return ERR_PTR(-ENODEV);
>>       }
>>
>> -     if (pci_resource_len(pdev, bar) != 0x100) {
>> -             dev_err(&pdev->dev, "Invalid iomem size. You may "
>> +     int len = pci_resource_len(pdev, bar);
>> +     if (len != 0x100 && len != 0x200) {
>
> Hmmm,
>
> a) SDHC Specs (even v3) only mention 0x100, so this _is_ the standard.
>   Do the new cards (which ones?) have anything located in the extra
>   area?

This controller is a dual-slot one, so has two register sets (though
one set of pins aren't wired to a socket).

> b) your approach won't scale very well

True - a more scalable test would be to check for non-zero length and
a multiple of 256 bytes, would you say?

This would be in-tune with page 2 of:
http://www.sdcard.org/developers/tech/host_controller/simple_spec/Simplified_SD_Host_Controller_Spec.pdf

> so, I'd say it is better to keep the old way.
>
>> +             dev_warn(&pdev->dev, "Invalid iomem size. You may "
>>                       "experience problems.\n");
>
> I second turning the message into a warning, though.

If the latter method is preferred, I'll adjust the patch and resend.

Thanks,
  Daniel
-- 
Daniel J Blueman

Re: [PATCH] sdhci: fix undue iomem warning

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]


> > a) SDHC Specs (even v3) only mention 0x100, so this _is_ the standard.
> >   Do the new cards (which ones?) have anything located in the extra
> >   area?
> 
> This controller is a dual-slot one, so has two register sets (though
> one set of pins aren't wired to a socket).

There are two controllers and they are packed into one PCI-bar? :( I guess this
needs refactoring of the probe_slot routine then. Just silencing the warning
will just hide the problem.

> > b) your approach won't scale very well
> 
> True - a more scalable test would be to check for non-zero length and
> a multiple of 256 bytes, would you say?

That wouldn't alarm for 0x10000 or the like, so no gain as well.

> >> +             dev_warn(&pdev->dev, "Invalid iomem size. You may "
> >>                       "experience problems.\n");
> >
> > I second turning the message into a warning, though.
> 
> If the latter method is preferred, I'll adjust the patch and resend.

Reconsidering: Given the current situation, an error message is maybe not a
that bad idea, until the code can handle two controllers in one bar.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] sdhci: fix undue iomem warning

[Arnd Bergmann]

On Monday 30 May 2011 20:21:03 Wolfram Sang wrote:
>   Show Details
>   > > a) SDHC Specs (even v3) only mention 0x100, so this is the standard.
> > >   Do the new cards (which ones?) have anything located in the extra
> > >   area?
> > 
> > This controller is a dual-slot one, so has two register sets (though
> > one set of pins aren't wired to a socket).
> 
> There are two controllers and they are packed into one PCI-bar? :( I guess this
> needs refactoring of the probe_slot routine then. Just silencing the warning
> will just hide the problem.

Right. Presumably someone has already built a different system with the
same chip and both slots in use. This probably also means we need a
way to figure out which of the slots are in fact connected.

> > > b) your approach won't scale very well
> > 
> > True - a more scalable test would be to check for non-zero length and
> > a multiple of 256 bytes, would you say?
> 
> That wouldn't alarm for 0x10000 or the like, so no gain as well.

In fact, all PCI resources are by definition power-of-two numbers,
so the check would not work at all.

> > >> +             dev_warn(&pdev->dev, "Invalid iomem size. You may "
> > >>                       "experience problems.\n");
> > >
> > > I second turning the message into a warning, though.
> > 
> > If the latter method is preferred, I'll adjust the patch and resend.
> 
> Reconsidering: Given the current situation, an error message is maybe not a
> that bad idea, until the code can handle two controllers in one bar.

Agreed.

	Arnd