[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Alexander Kurz]

With commit 7b91369b DMA access got disabled for device drivers with zero
DMA mask property. sdhci-esdhc-imx got blocked from DMA access by this.
Hence: initialize the dma mask to enable access again.

Signed-off-by: Alexander Kurz <akurz@blala.de>
---
 arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c b/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
index a5edd7d..3d039ef 100644
--- a/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
+++ b/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
@@ -71,6 +71,7 @@ struct platform_device *__init imx_add_sdhci_esdhc_imx(
 	if (!pdata)
 		pdata = &default_esdhc_pdata;
 
-	return imx_add_platform_device(data->devid, data->id, res,
-			ARRAY_SIZE(res), pdata, sizeof(*pdata));
+	return imx_add_platform_device_dmamask(data->devid, data->id, res,
+			ARRAY_SIZE(res), pdata, sizeof(*pdata),
+			DMA_BIT_MASK(32));
 }
-- 
2.1.4

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Uwe Kleine-König]

Hello,

I added the people involved in 7b91369b4655 to Cc.

On Mon, Apr 11, 2016 at 10:20:46AM +0200, Alexander Kurz wrote:
> With commit 7b91369b DMA access got disabled for device drivers with zero

Please reference commits like:

	With commit 7b91369b4655 ("mmc: sdhci: Set DMA mask when adding
	host") DMA access ...


> DMA mask property. sdhci-esdhc-imx got blocked from DMA access by this.
> Hence: initialize the dma mask to enable access again.
> 
> Signed-off-by: Alexander Kurz <akurz@blala.de>
> ---
>  arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c b/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
> index a5edd7d..3d039ef 100644
> --- a/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
> +++ b/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
> @@ -71,6 +71,7 @@ struct platform_device *__init imx_add_sdhci_esdhc_imx(
>  	if (!pdata)
>  		pdata = &default_esdhc_pdata;
>  
> -	return imx_add_platform_device(data->devid, data->id, res,
> -			ARRAY_SIZE(res), pdata, sizeof(*pdata));
> +	return imx_add_platform_device_dmamask(data->devid, data->id, res,
> +			ARRAY_SIZE(res), pdata, sizeof(*pdata),
> +			DMA_BIT_MASK(32));
>  }
> -- 
> 2.1.4
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Adrian Hunter]

On 11/04/16 11:35, Uwe Kleine-K?nig wrote:
> Hello,
> 
> I added the people involved in 7b91369b4655 to Cc.
> 
> On Mon, Apr 11, 2016 at 10:20:46AM +0200, Alexander Kurz wrote:
>> With commit 7b91369b DMA access got disabled for device drivers with zero

Is that because dma_set_mask_and_coherent() fails?

> 
> Please reference commits like:
> 
> 	With commit 7b91369b4655 ("mmc: sdhci: Set DMA mask when adding
> 	host") DMA access ...
> 
> 
>> DMA mask property. sdhci-esdhc-imx got blocked from DMA access by this.
>> Hence: initialize the dma mask to enable access again.
>>
>> Signed-off-by: Alexander Kurz <akurz@blala.de>
>> ---
>>  arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c b/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
>> index a5edd7d..3d039ef 100644
>> --- a/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
>> +++ b/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
>> @@ -71,6 +71,7 @@ struct platform_device *__init imx_add_sdhci_esdhc_imx(
>>  	if (!pdata)
>>  		pdata = &default_esdhc_pdata;
>>  
>> -	return imx_add_platform_device(data->devid, data->id, res,
>> -			ARRAY_SIZE(res), pdata, sizeof(*pdata));
>> +	return imx_add_platform_device_dmamask(data->devid, data->id, res,
>> +			ARRAY_SIZE(res), pdata, sizeof(*pdata),
>> +			DMA_BIT_MASK(32));
>>  }
>> -- 
>> 2.1.4
>>
>>
> 

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Alexander Kurz]

Hi Adrian,
On Mon, 11 Apr 2016, Adrian Hunter wrote:

> On 11/04/16 11:35, Uwe Kleine-K?nig wrote:
> > Hello,
> > 
> > I added the people involved in 7b91369b4655 to Cc.
> > 
> > On Mon, Apr 11, 2016 at 10:20:46AM +0200, Alexander Kurz wrote:
> >> With commit 7b91369b DMA access got disabled for device drivers with zero
> 
> Is that because dma_set_mask_and_coherent() fails?
right, dma_set_mask_and_coherent() fails, thats the only reason for
this patch. This popped up on a Kindle3 (IMX35 with eMMC based root fs).

Sorry for the checkpatch / improper commit citation issue.

> 
> > 
> > Please reference commits like:
> > 
> > 	With commit 7b91369b4655 ("mmc: sdhci: Set DMA mask when adding
> > 	host") DMA access ...
> > 
> > 
> >> DMA mask property. sdhci-esdhc-imx got blocked from DMA access by this.
> >> Hence: initialize the dma mask to enable access again.
> >>
> >> Signed-off-by: Alexander Kurz <akurz@blala.de>
> >> ---
> >>  arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c b/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
> >> index a5edd7d..3d039ef 100644
> >> --- a/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
> >> +++ b/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
> >> @@ -71,6 +71,7 @@ struct platform_device *__init imx_add_sdhci_esdhc_imx(
> >>  	if (!pdata)
> >>  		pdata = &default_esdhc_pdata;
> >>  
> >> -	return imx_add_platform_device(data->devid, data->id, res,
> >> -			ARRAY_SIZE(res), pdata, sizeof(*pdata));
> >> +	return imx_add_platform_device_dmamask(data->devid, data->id, res,
> >> +			ARRAY_SIZE(res), pdata, sizeof(*pdata),
> >> +			DMA_BIT_MASK(32));
> >>  }
> >> -- 
> >> 2.1.4
> >>
> >>
> > 
> 

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Adrian Hunter]

On 11/04/16 19:13, Alexander Kurz wrote:
> Hi Adrian,
> On Mon, 11 Apr 2016, Adrian Hunter wrote:
> 
>> On 11/04/16 11:35, Uwe Kleine-K?nig wrote:
>>> Hello,
>>>
>>> I added the people involved in 7b91369b4655 to Cc.
>>>
>>> On Mon, Apr 11, 2016 at 10:20:46AM +0200, Alexander Kurz wrote:
>>>> With commit 7b91369b DMA access got disabled for device drivers with zero
>>
>> Is that because dma_set_mask_and_coherent() fails?
> right, dma_set_mask_and_coherent() fails, thats the only reason for
> this patch. This popped up on a Kindle3 (IMX35 with eMMC based root fs).

Arnd, Alexandre : Why should dma_set_mask_and_coherent() fail in this case
when DMA apparently doesn't need a dma_mask anyway?

> 
> Sorry for the checkpatch / improper commit citation issue.
> 
>>
>>>
>>> Please reference commits like:
>>>
>>> 	With commit 7b91369b4655 ("mmc: sdhci: Set DMA mask when adding
>>> 	host") DMA access ...
>>>
>>>
>>>> DMA mask property. sdhci-esdhc-imx got blocked from DMA access by this.
>>>> Hence: initialize the dma mask to enable access again.
>>>>
>>>> Signed-off-by: Alexander Kurz <akurz@blala.de>
>>>> ---
>>>>  arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c | 5 +++--
>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c b/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
>>>> index a5edd7d..3d039ef 100644
>>>> --- a/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
>>>> +++ b/arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c
>>>> @@ -71,6 +71,7 @@ struct platform_device *__init imx_add_sdhci_esdhc_imx(
>>>>  	if (!pdata)
>>>>  		pdata = &default_esdhc_pdata;
>>>>  
>>>> -	return imx_add_platform_device(data->devid, data->id, res,
>>>> -			ARRAY_SIZE(res), pdata, sizeof(*pdata));
>>>> +	return imx_add_platform_device_dmamask(data->devid, data->id, res,
>>>> +			ARRAY_SIZE(res), pdata, sizeof(*pdata),
>>>> +			DMA_BIT_MASK(32));
>>>>  }
>>>> -- 
>>>> 2.1.4
>>>>
>>>>
>>>

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Russell King - ARM Linux]

On Tue, Apr 12, 2016 at 09:05:34AM +0300, Adrian Hunter wrote:
> On 11/04/16 19:13, Alexander Kurz wrote:
> > Hi Adrian,
> > On Mon, 11 Apr 2016, Adrian Hunter wrote:
> > 
> >> On 11/04/16 11:35, Uwe Kleine-K?nig wrote:
> >>> Hello,
> >>>
> >>> I added the people involved in 7b91369b4655 to Cc.
> >>>
> >>> On Mon, Apr 11, 2016 at 10:20:46AM +0200, Alexander Kurz wrote:
> >>>> With commit 7b91369b DMA access got disabled for device drivers with zero
> >>
> >> Is that because dma_set_mask_and_coherent() fails?
> > right, dma_set_mask_and_coherent() fails, thats the only reason for
> > this patch. This popped up on a Kindle3 (IMX35 with eMMC based root fs).
> 
> Arnd, Alexandre : Why should dma_set_mask_and_coherent() fail in this case
> when DMA apparently doesn't need a dma_mask anyway?

What do you mean "doesn't need a dma_mask" ?  DMA _always_ requires a
DMA mask.  The DMA mask defines how many address bits are capable of
being used on the bus.

If there's no DMA mask, then there's no usable address bits, and so
the device is not DMA capable.  Hence, dma_set_mask_and_coherent()
will fail because its not possible to negotiate a non-zero number of
address bits.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Adrian Hunter]

On 12/04/16 11:40, Russell King - ARM Linux wrote:
> On Tue, Apr 12, 2016 at 09:05:34AM +0300, Adrian Hunter wrote:
>> On 11/04/16 19:13, Alexander Kurz wrote:
>>> Hi Adrian,
>>> On Mon, 11 Apr 2016, Adrian Hunter wrote:
>>>
>>>> On 11/04/16 11:35, Uwe Kleine-K?nig wrote:
>>>>> Hello,
>>>>>
>>>>> I added the people involved in 7b91369b4655 to Cc.
>>>>>
>>>>> On Mon, Apr 11, 2016 at 10:20:46AM +0200, Alexander Kurz wrote:
>>>>>> With commit 7b91369b DMA access got disabled for device drivers with zero
>>>>
>>>> Is that because dma_set_mask_and_coherent() fails?
>>> right, dma_set_mask_and_coherent() fails, thats the only reason for
>>> this patch. This popped up on a Kindle3 (IMX35 with eMMC based root fs).
>>
>> Arnd, Alexandre : Why should dma_set_mask_and_coherent() fail in this case
>> when DMA apparently doesn't need a dma_mask anyway?
> 
> What do you mean "doesn't need a dma_mask" ?  DMA _always_ requires a
> DMA mask.  The DMA mask defines how many address bits are capable of
> being used on the bus.
> 
> If there's no DMA mask, then there's no usable address bits, and so
> the device is not DMA capable.  Hence, dma_set_mask_and_coherent()
> will fail because its not possible to negotiate a non-zero number of
> address bits.
> 

The point is, now we valid dma_set_mask_and_coherent(), DMA will stop
working for any other SDHCI device that hasn't allocated dev.dma_mask.  Is
that OK?

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Alexandre Courbot]

On 04/12/2016 06:59 PM, Adrian Hunter wrote:
> On 12/04/16 11:40, Russell King - ARM Linux wrote:
>> On Tue, Apr 12, 2016 at 09:05:34AM +0300, Adrian Hunter wrote:
>>> On 11/04/16 19:13, Alexander Kurz wrote:
>>>> Hi Adrian,
>>>> On Mon, 11 Apr 2016, Adrian Hunter wrote:
>>>>
>>>>> On 11/04/16 11:35, Uwe Kleine-K?nig wrote:
>>>>>> Hello,
>>>>>>
>>>>>> I added the people involved in 7b91369b4655 to Cc.
>>>>>>
>>>>>> On Mon, Apr 11, 2016 at 10:20:46AM +0200, Alexander Kurz wrote:
>>>>>>> With commit 7b91369b DMA access got disabled for device drivers with zero
>>>>>
>>>>> Is that because dma_set_mask_and_coherent() fails?
>>>> right, dma_set_mask_and_coherent() fails, thats the only reason for
>>>> this patch. This popped up on a Kindle3 (IMX35 with eMMC based root fs).
>>>
>>> Arnd, Alexandre : Why should dma_set_mask_and_coherent() fail in this case
>>> when DMA apparently doesn't need a dma_mask anyway?
>>
>> What do you mean "doesn't need a dma_mask" ?  DMA _always_ requires a
>> DMA mask.  The DMA mask defines how many address bits are capable of
>> being used on the bus.
>>
>> If there's no DMA mask, then there's no usable address bits, and so
>> the device is not DMA capable.  Hence, dma_set_mask_and_coherent()
>> will fail because its not possible to negotiate a non-zero number of
>> address bits.
>>
>
> The point is, now we valid dma_set_mask_and_coherent(), DMA will stop
> working for any other SDHCI device that hasn't allocated dev.dma_mask.  Is
> that OK?

Clearly these devices need to be fixed. If we want to give them a grace 
period, we could also (temporarily) not propagate the return value of 
dma_set_mask_and_coherent() and limit ourselves to emitting a big 
warning about the inconsistency of having SDHCI_USE_SDMA/SDHCI_USE_ADMA 
in the host flags while not setting a dma mask.

But in the past, how prompt have people been to react to such warnings 
vs. their device not working as expected?

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Russell King - ARM Linux]

On Tue, Apr 12, 2016 at 09:25:04PM +0900, Alexandre Courbot wrote:
> Clearly these devices need to be fixed. If we want to give them a grace
> period, we could also (temporarily) not propagate the return value of
> dma_set_mask_and_coherent() and limit ourselves to emitting a big warning
> about the inconsistency of having SDHCI_USE_SDMA/SDHCI_USE_ADMA in the host
> flags while not setting a dma mask.

Isn't it just easier to fix the root problem?  It'll be one patch.

There's no need to add churn by changing sdhci stuff, then fixing the
imx stuff, and then reverting the sdhci changes.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Alexandre Courbot]

On 04/13/2016 12:31 AM, Russell King - ARM Linux wrote:
> On Tue, Apr 12, 2016 at 09:25:04PM +0900, Alexandre Courbot wrote:
>> Clearly these devices need to be fixed. If we want to give them a grace
>> period, we could also (temporarily) not propagate the return value of
>> dma_set_mask_and_coherent() and limit ourselves to emitting a big warning
>> about the inconsistency of having SDHCI_USE_SDMA/SDHCI_USE_ADMA in the host
>> flags while not setting a dma mask.
>
> Isn't it just easier to fix the root problem?  It'll be one patch.
>
> There's no need to add churn by changing sdhci stuff, then fixing the
> imx stuff, and then reverting the sdhci changes.

I think Adrian was concerned that other SDHCI drivers might unknowingly 
be in the same case. That being said I am also in favor of fixing the 
root issue instead of adding temporary glue that we might eventually 
forget to remove...

How long it will take for everyone to fix their drivers is another 
question, since the device doesn't clearly break, but falls back to a 
degraded mode with a warning.

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Adrian Hunter]

On 13/04/16 05:02, Alexandre Courbot wrote:
> On 04/13/2016 12:31 AM, Russell King - ARM Linux wrote:
>> On Tue, Apr 12, 2016 at 09:25:04PM +0900, Alexandre Courbot wrote:
>>> Clearly these devices need to be fixed. If we want to give them a grace
>>> period, we could also (temporarily) not propagate the return value of
>>> dma_set_mask_and_coherent() and limit ourselves to emitting a big warning
>>> about the inconsistency of having SDHCI_USE_SDMA/SDHCI_USE_ADMA in the host
>>> flags while not setting a dma mask.
>>
>> Isn't it just easier to fix the root problem?  It'll be one patch.
>>
>> There's no need to add churn by changing sdhci stuff, then fixing the
>> imx stuff, and then reverting the sdhci changes.
> 
> I think Adrian was concerned that other SDHCI drivers might unknowingly be
> in the same case.

Yes, that is the concern.

>                   That being said I am also in favor of fixing the root
> issue instead of adding temporary glue that we might eventually forget to
> remove...

Yes, temporary glue just defers the problem.

> 
> How long it will take for everyone to fix their drivers is another question,
> since the device doesn't clearly break, but falls back to a degraded mode
> with a warning.
> 

It looks like DeviceTree, PCI and ACPI enumerated devices always set up a
dma_mask.  I guess that just leaves devices enumerated from hard-coded
platform data.  I will have a look for any more of those, and also send an
email for affected people to check their device setup and drivers.

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Arnd Bergmann]

On Wednesday 13 April 2016 11:07:16 Adrian Hunter wrote:
> On 13/04/16 05:02, Alexandre Courbot wrote:
> > On 04/13/2016 12:31 AM, Russell King - ARM Linux wrote:
> >> On Tue, Apr 12, 2016 at 09:25:04PM +0900, Alexandre Courbot wrote:

> > How long it will take for everyone to fix their drivers is another question,
> > since the device doesn't clearly break, but falls back to a degraded mode
> > with a warning.
> > 
> 
> It looks like DeviceTree, PCI and ACPI enumerated devices always set up a
> dma_mask.  I guess that just leaves devices enumerated from hard-coded
> platform data.  I will have a look for any more of those, and also send an
> email for affected people to check their device setup and drivers.
> 

I've had a look now too and found only these three device definitions
for any sdhci variant, everything else is either unused or DT-only:

arch/arm/mach-dove/common.c:    .name           = "sdhci-dove",
arch/arm/plat-samsung/devs.c:   .name           = "s3c-sdhci",
arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c:   imx_sdhci_esdhc_imx_data_entry(MX25, "sdhci-esdhc-imx25", _id, _hwid)
arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c:   imx_sdhci_esdhc_imx_data_entry(MX35, "sdhci-esdhc-imx35", _id, _hwid)

Out of these, the s3c and dove variants set a 32-bit DMA mask, so as
far as I can tell, only imx has the problem.

	Arnd

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Adrian Hunter]

On 17/04/16 00:48, Arnd Bergmann wrote:
> On Wednesday 13 April 2016 11:07:16 Adrian Hunter wrote:
>> On 13/04/16 05:02, Alexandre Courbot wrote:
>>> On 04/13/2016 12:31 AM, Russell King - ARM Linux wrote:
>>>> On Tue, Apr 12, 2016 at 09:25:04PM +0900, Alexandre Courbot wrote:
> 
>>> How long it will take for everyone to fix their drivers is another question,
>>> since the device doesn't clearly break, but falls back to a degraded mode
>>> with a warning.
>>>
>>
>> It looks like DeviceTree, PCI and ACPI enumerated devices always set up a
>> dma_mask.  I guess that just leaves devices enumerated from hard-coded
>> platform data.  I will have a look for any more of those, and also send an
>> email for affected people to check their device setup and drivers.
>>
> 
> I've had a look now too and found only these three device definitions
> for any sdhci variant, everything else is either unused or DT-only:
> 
> arch/arm/mach-dove/common.c:    .name           = "sdhci-dove",
> arch/arm/plat-samsung/devs.c:   .name           = "s3c-sdhci",
> arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c:   imx_sdhci_esdhc_imx_data_entry(MX25, "sdhci-esdhc-imx25", _id, _hwid)
> arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c:   imx_sdhci_esdhc_imx_data_entry(MX35, "sdhci-esdhc-imx35", _id, _hwid)
> 
> Out of these, the s3c and dove variants set a 32-bit DMA mask, so as
> far as I can tell, only imx has the problem.
> 

Thanks for looking.

It looked to me like sdhci-pxav3 devices created by mmp2_add_sdhost() might
also be candidates.

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Arnd Bergmann]

On Monday 18 April 2016 09:58:04 Adrian Hunter wrote:
> 
> Thanks for looking.
> 
> It looked to me like sdhci-pxav3 devices created by mmp2_add_sdhost() might
> also be candidates.
> 

Oh, you are right, I missed that.

I remember looking at sdhci-pxav2 and not finding any machine defining one,
but I must have skipped over sdhci-pxav3 failing to realize that this is
a different one.

	Arnd

Potential issue with SDHCI DMA

[Adrian Hunter]

+ linux-mmc

On 29/04/16 11:58, Adrian Hunter wrote:
> On 18/04/16 17:33, Arnd Bergmann wrote:
>> On Monday 18 April 2016 09:58:04 Adrian Hunter wrote:
>>>
>>> Thanks for looking.
>>>
>>> It looked to me like sdhci-pxav3 devices created by mmp2_add_sdhost() might
>>> also be candidates.
>>>
>>
>> Oh, you are right, I missed that.
>>
>> I remember looking at sdhci-pxav2 and not finding any machine defining one,
>> but I must have skipped over sdhci-pxav3 failing to realize that this is
>> a different one.
>>
>> 	Arnd
>>
> 
> OK, so my plan to email sdhci driver maintainers ran into a snag.  It turned
> out to be too hard to dig up email addresses.  I have grabbed a few names
> and cc'ed them to this email anyway.
> 
> For those people, the issue is this:
> 
> An unexpected side-effect of commit 7b91369b4655 ("mmc: sdhci: Set DMA mask
> when adding host") is that SDHCI devices that do not define a DMA mask may
> find that DMA no longer works.  That was the case for a sdhci-esdhc-imx
> device, but that has been fixed - refer commit fc26fe9c3869 ("ARM: mach-imx:
> sdhci-esdhc-imx: initialize DMA mask").
> 
> DeviceTree, ACPI and PCI always set up a DMA mask for devices that they
> enumerate, so only hard-coded platform devices are expected to be affected.
> 
> We found only one other candidate: sdhci-pxav3 devices created by
> mmp2_add_sdhost().  Not sure if anyone has looked at that though.
> 
> Obviously, if you are unsure if your devices are affected, you can test and
> if there is a problem you will see the warning messages "mmcX: Failed to set
> 32-bit DMA mask" and "mmcX: No suitable DMA available - falling back to PIO".
> 
> Regards
> Adrian
> 
> 

Potential issue with SDHCI DMA

[Jisheng Zhang]

Dear Adrian,

On Fri, 29 Apr 2016 11:59:39 +0300 Adrian Hunter wrote:

> + linux-mmc
> 
> On 29/04/16 11:58, Adrian Hunter wrote:
> > On 18/04/16 17:33, Arnd Bergmann wrote:  
> >> On Monday 18 April 2016 09:58:04 Adrian Hunter wrote:  
> >>>
> >>> Thanks for looking.
> >>>
> >>> It looked to me like sdhci-pxav3 devices created by mmp2_add_sdhost() might
> >>> also be candidates.
> >>>  
> >>
> >> Oh, you are right, I missed that.
> >>
> >> I remember looking at sdhci-pxav2 and not finding any machine defining one,
> >> but I must have skipped over sdhci-pxav3 failing to realize that this is
> >> a different one.
> >>
> >> 	Arnd
> >>  
> > 
> > OK, so my plan to email sdhci driver maintainers ran into a snag.  It turned
> > out to be too hard to dig up email addresses.  I have grabbed a few names
> > and cc'ed them to this email anyway.
> > 
> > For those people, the issue is this:
> > 
> > An unexpected side-effect of commit 7b91369b4655 ("mmc: sdhci: Set DMA mask
> > when adding host") is that SDHCI devices that do not define a DMA mask may
> > find that DMA no longer works.  That was the case for a sdhci-esdhc-imx
> > device, but that has been fixed - refer commit fc26fe9c3869 ("ARM: mach-imx:
> > sdhci-esdhc-imx: initialize DMA mask").
> > 
> > DeviceTree, ACPI and PCI always set up a DMA mask for devices that they
> > enumerate, so only hard-coded platform devices are expected to be affected.
> > 
> > We found only one other candidate: sdhci-pxav3 devices created by
> > mmp2_add_sdhost().  Not sure if anyone has looked at that though.

Thanks for the kind remind. All Marvell berlin SoCs' linux kernel (no matter
upstreamed or internal) are DT based, so we (berlin SoCs) are not affected by
this issue.

I'm not sure who is maintaining mmp.

Thanks,
Jisheng

> > 
> > Obviously, if you are unsure if your devices are affected, you can test and
> > if there is a problem you will see the warning messages "mmcX: Failed to set
> > 32-bit DMA mask" and "mmcX: No suitable DMA available - falling back to PIO".
> > 
> > Regards
> > Adrian
> > 
> >   
> 

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Russell King - ARM Linux]

On Tue, Apr 12, 2016 at 12:59:14PM +0300, Adrian Hunter wrote:
> On 12/04/16 11:40, Russell King - ARM Linux wrote:
> > What do you mean "doesn't need a dma_mask" ?  DMA _always_ requires a
> > DMA mask.  The DMA mask defines how many address bits are capable of
> > being used on the bus.
> > 
> > If there's no DMA mask, then there's no usable address bits, and so
> > the device is not DMA capable.  Hence, dma_set_mask_and_coherent()
> > will fail because its not possible to negotiate a non-zero number of
> > address bits.
> > 
> 
> The point is, now we valid dma_set_mask_and_coherent(), DMA will stop
> working for any other SDHCI device that hasn't allocated dev.dma_mask.  Is
> that OK?

Two latent bugs.

1) Any device that doesn't have dma_mask allocated isn't capable of
   streaming DMA, but may be capable of coherent DMA - a device which
   is capable _should_ have a defaulted dma_mask setup.

2) Drivers must always use the dma_set_mask*() family of functions
   if they intend to use DMA.

Requirement (2) is nothing new, and predates MMC as a whole, so it's
a long-standing latent bug.


-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Sergei Shtylyov]

Hello.

On 4/11/2016 11:20 AM, Alexander Kurz wrote:

> With commit 7b91369b DMA access got disabled for device drivers with zero

    scripts/checkpatch.pl enforces certain commit citing style, your doesn't 
match it.

> DMA mask property. sdhci-esdhc-imx got blocked from DMA access by this.
> Hence: initialize the dma mask to enable access again.

    DMA -- please be consistent.

> Signed-off-by: Alexander Kurz <akurz@blala.de>
[...]

MBR, Sergei

[PATCH] ARM: mach-imx: sdhci-esdhc-imx: initialize DMA mask

[Shawn Guo]

On Mon, Apr 11, 2016 at 10:20:46AM +0200, Alexander Kurz wrote:
> With commit 7b91369b DMA access got disabled for device drivers with zero
> DMA mask property. sdhci-esdhc-imx got blocked from DMA access by this.
> Hence: initialize the dma mask to enable access again.
> 
> Signed-off-by: Alexander Kurz <akurz@blala.de>

I fixed up the commit log per comments from Uwe and Sergei, and then
applied the patch as a fix for v4.6-rc.

Shawn