Re: [PATCH 1/1] ARM: dma: fix dma_max_pfn()
On Wed, Sep 28, 2016 at 10:53:49AM +0300, Roger Quadros wrote: > Hi, > > On 12/09/16 14:38, Roger Quadros wrote: > > Hi Santosh & Russell, > > > > On 19/08/16 19:38, Santosh Shilimkar wrote: > >> > >> On 8/19/2016 12:30 AM, Roger Quadros wrote: > >>> Hi Santosh, > >>> > >> > > So I'm 99.9% convinced that the proposed change is correct. > > > I will got with that then :-) and take my objection back. Just > saying that if there other breakages which I can't recollect now, > those drivers needs to be patched as well. > > >>> I was able to boot the Keystone2 Edison EVM over NFS with the $subject > >>> patch. > >>> Boot log is below. Do you see anything suspicious? > >>> > >> Logs looks ok to me. Probably do some tests where DMA and bounce buffers > >> etc gets tested. Running it through your internal regression > >> suit will be good idea as well if thats possible. > >> > > > > This has been running in our internal test suite for a week on various TI > > platforms. There haven't been any surprises. > > > > Is it a good idea to at least put this in -next for a wider test audience? > > Gentle reminder. Please put it in the patch system, thanks. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: dma: fix dma_max_pfn()
Hi, On 12/09/16 14:38, Roger Quadros wrote: > Hi Santosh & Russell, > > On 19/08/16 19:38, Santosh Shilimkar wrote: >> >> On 8/19/2016 12:30 AM, Roger Quadros wrote: >>> Hi Santosh, >>> >> > So I'm 99.9% convinced that the proposed change is correct. > I will got with that then :-) and take my objection back. Just saying that if there other breakages which I can't recollect now, those drivers needs to be patched as well. >>> I was able to boot the Keystone2 Edison EVM over NFS with the $subject >>> patch. >>> Boot log is below. Do you see anything suspicious? >>> >> Logs looks ok to me. Probably do some tests where DMA and bounce buffers etc >> gets tested. Running it through your internal regression >> suit will be good idea as well if thats possible. >> > > This has been running in our internal test suite for a week on various TI > platforms. There haven't been any surprises. > > Is it a good idea to at least put this in -next for a wider test audience? Gentle reminder. regards, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: dma: fix dma_max_pfn()
Hi Santosh & Russell, On 19/08/16 19:38, Santosh Shilimkar wrote: > > On 8/19/2016 12:30 AM, Roger Quadros wrote: >> Hi Santosh, >> > So I'm 99.9% convinced that the proposed change is correct. >>> I will got with that then :-) and take my objection back. Just >>> saying that if there other breakages which I can't recollect now, >>> those drivers needs to be patched as well. >>> >> I was able to boot the Keystone2 Edison EVM over NFS with the $subject patch. >> Boot log is below. Do you see anything suspicious? >> > Logs looks ok to me. Probably do some tests where DMA and bounce buffers etc > gets tested. Running it through your internal regression > suit will be good idea as well if thats possible. > This has been running in our internal test suite for a week on various TI platforms. There haven't been any surprises. Is it a good idea to at least put this in -next for a wider test audience? cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: dma: fix dma_max_pfn()
On 8/19/2016 12:30 AM, Roger Quadros wrote: Hi Santosh, So I'm 99.9% convinced that the proposed change is correct. I will got with that then :-) and take my objection back. Just saying that if there other breakages which I can't recollect now, those drivers needs to be patched as well. I was able to boot the Keystone2 Edison EVM over NFS with the $subject patch. Boot log is below. Do you see anything suspicious? Logs looks ok to me. Probably do some tests where DMA and bounce buffers etc gets tested. Running it through your internal regression suit will be good idea as well if thats possible. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: dma: fix dma_max_pfn()
Hi Santosh,
On 19/08/16 05:01, Santosh Shilimkar wrote:
> On 8/18/2016 4:07 PM, Russell King - ARM Linux wrote:
>> On Thu, Aug 18, 2016 at 09:55:55AM -0700, Santosh Shilimkar wrote:
>>> Hi Russell,
>>>
>>> On 8/18/2016 7:24 AM, Russell King - ARM Linux wrote:
On Wed, Aug 17, 2016 at 03:05:17PM +0300, Roger Quadros wrote:
> Since commit 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address
> translation"),
> dma_to_pfn() already returns the PFN with the physical memory start offset
> so we don't need to add it again.
>
> This fixes USB mass storage lock-up problem on systems that can't do DMA
> over the entire physical memory range (e.g.) Keystone 2 systems with 4GB
> RAM
> can only do DMA over the first 2GB. [K2E-EVM].
>
> What happens there is that without this patch SCSI layer sets a wrong
> bounce buffer limit in scsi_calculate_bounce_limit() for the USB mass
> storage device. dma_max_pfn() evaluates to 0x8f and bounce_limit
> is set to 0x8f000 whereas maximum DMA'ble physical memory on Keystone
> 2
> is 0x87fff. This results in non DMA'ble pages being given to the
> USB controller and hence the lock-up.
>
> NOTE: in the above case, USB-SCSI-device's dma_pfn_offset was showing as
> 0.
> This should have really been 0x78 as on K2e, LOWMEM_START is
> 0x8000
> and HIGHMEM_START is 0x8. DMA zone is 2GB so dma_max_pfn should be
> 0x87. The incorrect dma_pfn_offset for the USB storage device is
> because
> USB devices are not correctly inheriting the dma_pfn_offset from the
> USB host controller. This will be fixed by a separate patch.
I'd like to hear from Santosh, as the author of the original change.
The original commit doesn't mention which platform it was intended for
or what the problem was, which would've been helpful.
>>> From what I recollect, we did these changes to make the max pfn behave
>>> same on ARM arch as other archs. This patch was evolved as part of
>>> fixing the max*pfn assumption.
>>
>> To me, the proposed patch _looks_ correct, because...
>>
> diff --git a/arch/arm/include/asm/dma-mapping.h
> b/arch/arm/include/asm/dma-mapping.h
> index d009f79..bf02dbd 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -111,7 +111,7 @@ static inline dma_addr_t virt_to_dma(struct device
> *dev, void *addr)
> /* The ARM override for dma_max_pfn() */
> static inline unsigned long dma_max_pfn(struct device *dev)
> {
> -return PHYS_PFN_OFFSET + dma_to_pfn(dev, *dev->dma_mask);
> +return dma_to_pfn(dev, *dev->dma_mask);
> }
> #define dma_max_pfn(dev) dma_max_pfn(dev)
>>> By doing this change I hope we don't break other drivers on Keystone so
>>> am not sure about the change.
>>
>> dma_to_pfn() returns the page frame number referenced from physical
>> address zero - the default implementation of dma_to_pfn() is
>> bus_to_pfn(), which is __phys_to_pfn(x), which is just x >> PAGE_SHIFT.
>> The other thing about dma_to_pfn() is that it should return a
>> zero-referenced PFN number, where PFN 0 = physical address 0.
>>
>> If there is some offset for keystone2, that should be taken care of
>> via "dev->dma_pfn_offset", and not offsetting the return value from
>> dma_to_pfn().
>>
>> So I'm 99.9% convinced that the proposed change is correct.
>>
> I will got with that then :-) and take my objection back. Just
> saying that if there other breakages which I can't recollect now,
> those drivers needs to be patched as well.
>
I was able to boot the Keystone2 Edison EVM over NFS with the $subject patch.
Boot log is below. Do you see anything suspicious?
---
U-Boot SPL 2016.05-00100-g052f3bf-dirty (Aug 10 2016 - 12:28:40)
Trying to boot from SPI
U-Boot 2016.05-00100-g052f3bf-dirty (Aug 10 2016 - 12:28:40 +0300)
CPU: 66AK2Ex SR1.0
I2C: ready
DRAM: DDR3A Speed will be configured for 1600 Operation.
Detected SO-DIMM [18KSF51272HZ-1G6K2]
DDR3 speed 1600
DRAM: 4 GiB
Clear entire DDR3 memory to enable ECC
2 GiB
NAND: 512 MiB
Net: eth0: netcp@2400
Hit any key to stop autoboot: 0
netcp@2400 Waiting for SGMII auto negotiation to complete. done
Using netcp@2400 device
TFTP from server 192.168.1.36; our IP address is 192.168.1.90
Filename '/tftpboot/k2-fw-initrd.cpio.gz'.
Load address: 0x8808
Loading: ##
958 KiB/s
done
Bytes transferred = 48117 (bbf5 hex)
netcp@2400 Waiting for SGMII auto negotiation to complete. done
Using netcp@2400 device
TFTP from server 192.168.1.36; our IP address is 192.168.1.90
Filename 'keystone-k2e-evm.dtb'.
Load address: 0x8800
Loading: #
884.8 KiB/s
done
Bytes transferred = 22660 (5884 hex)
netcp@2400 Waiting for SGMII auto negotiation to complete. done
Using netcp@2400 device
TFTP from server 192.168.1.36; our IP address is
Re: [PATCH 1/1] ARM: dma: fix dma_max_pfn()
Hi Russell,
On 8/18/2016 7:24 AM, Russell King - ARM Linux wrote:
On Wed, Aug 17, 2016 at 03:05:17PM +0300, Roger Quadros wrote:
Since commit 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address
translation"),
dma_to_pfn() already returns the PFN with the physical memory start offset
so we don't need to add it again.
This fixes USB mass storage lock-up problem on systems that can't do DMA
over the entire physical memory range (e.g.) Keystone 2 systems with 4GB RAM
can only do DMA over the first 2GB. [K2E-EVM].
What happens there is that without this patch SCSI layer sets a wrong
bounce buffer limit in scsi_calculate_bounce_limit() for the USB mass
storage device. dma_max_pfn() evaluates to 0x8f and bounce_limit
is set to 0x8f000 whereas maximum DMA'ble physical memory on Keystone 2
is 0x87fff. This results in non DMA'ble pages being given to the
USB controller and hence the lock-up.
NOTE: in the above case, USB-SCSI-device's dma_pfn_offset was showing as 0.
This should have really been 0x78 as on K2e, LOWMEM_START is 0x8000
and HIGHMEM_START is 0x8. DMA zone is 2GB so dma_max_pfn should be
0x87. The incorrect dma_pfn_offset for the USB storage device is because
USB devices are not correctly inheriting the dma_pfn_offset from the
USB host controller. This will be fixed by a separate patch.
I'd like to hear from Santosh, as the author of the original change.
The original commit doesn't mention which platform it was intended for
or what the problem was, which would've been helpful.
From what I recollect, we did these changes to make the max pfn behave
same on ARM arch as other archs. This patch was evolved as part of
fixing the max*pfn assumption.
commit 26ba47b18318abe7dadbe9294a611c0e932651d8
Author: Santosh Shilimkar
Date: Thu Aug 1 03:12:01 2013 +0100
ARM: 7805/1: mm: change max*pfn to include the physical offset of
memory
Most of the kernel code assumes that max*pfn is maximum pfns because
the physical start of memory is expected to be PFN0. Since this
assumption is not true on ARM architectures, the meaning of max*pfn
is number of memory pages. This is done to keep drivers happy which
are making use of of these variable to calculate the dma bounce limit
using dma_mask.
Now since we have a architecture override possibility for DMAable
maximum pfns, lets make meaning of max*pfns as maximum pnfs on ARM
as well.
Fixes: 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address translation")
Cc: sta...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: Russell King
Cc: Arnd Bergmann
Cc: Olof Johansson
Cc: Catalin Marinas
Cc: Linus Walleij
Reported-by: Grygorii Strashko
Signed-off-by: Roger Quadros
---
arch/arm/include/asm/dma-mapping.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index d009f79..bf02dbd 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -111,7 +111,7 @@ static inline dma_addr_t virt_to_dma(struct device *dev,
void *addr)
/* The ARM override for dma_max_pfn() */
static inline unsigned long dma_max_pfn(struct device *dev)
{
- return PHYS_PFN_OFFSET + dma_to_pfn(dev, *dev->dma_mask);
+ return dma_to_pfn(dev, *dev->dma_mask);
}
#define dma_max_pfn(dev) dma_max_pfn(dev)
By doing this change I hope we don't break other drivers on Keystone so
am not sure about the change.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: dma: fix dma_max_pfn()
On Thu, Aug 18, 2016 at 09:55:55AM -0700, Santosh Shilimkar wrote:
> Hi Russell,
>
> On 8/18/2016 7:24 AM, Russell King - ARM Linux wrote:
> >On Wed, Aug 17, 2016 at 03:05:17PM +0300, Roger Quadros wrote:
> >>Since commit 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address
> >>translation"),
> >>dma_to_pfn() already returns the PFN with the physical memory start offset
> >>so we don't need to add it again.
> >>
> >>This fixes USB mass storage lock-up problem on systems that can't do DMA
> >>over the entire physical memory range (e.g.) Keystone 2 systems with 4GB RAM
> >>can only do DMA over the first 2GB. [K2E-EVM].
> >>
> >>What happens there is that without this patch SCSI layer sets a wrong
> >>bounce buffer limit in scsi_calculate_bounce_limit() for the USB mass
> >>storage device. dma_max_pfn() evaluates to 0x8f and bounce_limit
> >>is set to 0x8f000 whereas maximum DMA'ble physical memory on Keystone 2
> >>is 0x87fff. This results in non DMA'ble pages being given to the
> >>USB controller and hence the lock-up.
> >>
> >>NOTE: in the above case, USB-SCSI-device's dma_pfn_offset was showing as 0.
> >>This should have really been 0x78 as on K2e, LOWMEM_START is 0x8000
> >>and HIGHMEM_START is 0x8. DMA zone is 2GB so dma_max_pfn should be
> >>0x87. The incorrect dma_pfn_offset for the USB storage device is because
> >>USB devices are not correctly inheriting the dma_pfn_offset from the
> >>USB host controller. This will be fixed by a separate patch.
> >
> >I'd like to hear from Santosh, as the author of the original change.
> >The original commit doesn't mention which platform it was intended for
> >or what the problem was, which would've been helpful.
> >
> From what I recollect, we did these changes to make the max pfn behave
> same on ARM arch as other archs. This patch was evolved as part of
> fixing the max*pfn assumption.
To me, the proposed patch _looks_ correct, because...
> >>diff --git a/arch/arm/include/asm/dma-mapping.h
> >>b/arch/arm/include/asm/dma-mapping.h
> >>index d009f79..bf02dbd 100644
> >>--- a/arch/arm/include/asm/dma-mapping.h
> >>+++ b/arch/arm/include/asm/dma-mapping.h
> >>@@ -111,7 +111,7 @@ static inline dma_addr_t virt_to_dma(struct device
> >>*dev, void *addr)
> >> /* The ARM override for dma_max_pfn() */
> >> static inline unsigned long dma_max_pfn(struct device *dev)
> >> {
> >>- return PHYS_PFN_OFFSET + dma_to_pfn(dev, *dev->dma_mask);
> >>+ return dma_to_pfn(dev, *dev->dma_mask);
> >> }
> >> #define dma_max_pfn(dev) dma_max_pfn(dev)
> By doing this change I hope we don't break other drivers on Keystone so
> am not sure about the change.
dma_to_pfn() returns the page frame number referenced from physical
address zero - the default implementation of dma_to_pfn() is
bus_to_pfn(), which is __phys_to_pfn(x), which is just x >> PAGE_SHIFT.
The other thing about dma_to_pfn() is that it should return a
zero-referenced PFN number, where PFN 0 = physical address 0.
If there is some offset for keystone2, that should be taken care of
via "dev->dma_pfn_offset", and not offsetting the return value from
dma_to_pfn().
So I'm 99.9% convinced that the proposed change is correct.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: dma: fix dma_max_pfn()
On Wed, Aug 17, 2016 at 03:05:17PM +0300, Roger Quadros wrote:
> Since commit 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address
> translation"),
> dma_to_pfn() already returns the PFN with the physical memory start offset
> so we don't need to add it again.
>
> This fixes USB mass storage lock-up problem on systems that can't do DMA
> over the entire physical memory range (e.g.) Keystone 2 systems with 4GB RAM
> can only do DMA over the first 2GB. [K2E-EVM].
>
> What happens there is that without this patch SCSI layer sets a wrong
> bounce buffer limit in scsi_calculate_bounce_limit() for the USB mass
> storage device. dma_max_pfn() evaluates to 0x8f and bounce_limit
> is set to 0x8f000 whereas maximum DMA'ble physical memory on Keystone 2
> is 0x87fff. This results in non DMA'ble pages being given to the
> USB controller and hence the lock-up.
>
> NOTE: in the above case, USB-SCSI-device's dma_pfn_offset was showing as 0.
> This should have really been 0x78 as on K2e, LOWMEM_START is 0x8000
> and HIGHMEM_START is 0x8. DMA zone is 2GB so dma_max_pfn should be
> 0x87. The incorrect dma_pfn_offset for the USB storage device is because
> USB devices are not correctly inheriting the dma_pfn_offset from the
> USB host controller. This will be fixed by a separate patch.
I'd like to hear from Santosh, as the author of the original change.
The original commit doesn't mention which platform it was intended for
or what the problem was, which would've been helpful.
>
> Fixes: 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address
> translation")
> Cc: sta...@vger.kernel.org
> Cc: Greg Kroah-Hartman
> Cc: Russell King
> Cc: Arnd Bergmann
> Cc: Olof Johansson
> Cc: Catalin Marinas
> Cc: Linus Walleij
> Reported-by: Grygorii Strashko
> Signed-off-by: Roger Quadros
> ---
> arch/arm/include/asm/dma-mapping.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/dma-mapping.h
> b/arch/arm/include/asm/dma-mapping.h
> index d009f79..bf02dbd 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -111,7 +111,7 @@ static inline dma_addr_t virt_to_dma(struct device *dev,
> void *addr)
> /* The ARM override for dma_max_pfn() */
> static inline unsigned long dma_max_pfn(struct device *dev)
> {
> - return PHYS_PFN_OFFSET + dma_to_pfn(dev, *dev->dma_mask);
> + return dma_to_pfn(dev, *dev->dma_mask);
> }
> #define dma_max_pfn(dev) dma_max_pfn(dev)
>
> --
> 2.7.4
>
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] ARM: dma: fix dma_max_pfn()
Since commit 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address
translation"),
dma_to_pfn() already returns the PFN with the physical memory start offset
so we don't need to add it again.
This fixes USB mass storage lock-up problem on systems that can't do DMA
over the entire physical memory range (e.g.) Keystone 2 systems with 4GB RAM
can only do DMA over the first 2GB. [K2E-EVM].
What happens there is that without this patch SCSI layer sets a wrong
bounce buffer limit in scsi_calculate_bounce_limit() for the USB mass
storage device. dma_max_pfn() evaluates to 0x8f and bounce_limit
is set to 0x8f000 whereas maximum DMA'ble physical memory on Keystone 2
is 0x87fff. This results in non DMA'ble pages being given to the
USB controller and hence the lock-up.
NOTE: in the above case, USB-SCSI-device's dma_pfn_offset was showing as 0.
This should have really been 0x78 as on K2e, LOWMEM_START is 0x8000
and HIGHMEM_START is 0x8. DMA zone is 2GB so dma_max_pfn should be
0x87. The incorrect dma_pfn_offset for the USB storage device is because
USB devices are not correctly inheriting the dma_pfn_offset from the
USB host controller. This will be fixed by a separate patch.
Fixes: 6ce0d2001692 ("ARM: dma: Use dma_pfn_offset for dma address translation")
Cc: sta...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: Russell King
Cc: Arnd Bergmann
Cc: Olof Johansson
Cc: Catalin Marinas
Cc: Linus Walleij
Reported-by: Grygorii Strashko
Signed-off-by: Roger Quadros
---
arch/arm/include/asm/dma-mapping.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index d009f79..bf02dbd 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -111,7 +111,7 @@ static inline dma_addr_t virt_to_dma(struct device *dev,
void *addr)
/* The ARM override for dma_max_pfn() */
static inline unsigned long dma_max_pfn(struct device *dev)
{
- return PHYS_PFN_OFFSET + dma_to_pfn(dev, *dev->dma_mask);
+ return dma_to_pfn(dev, *dev->dma_mask);
}
#define dma_max_pfn(dev) dma_max_pfn(dev)
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
