Re: [PATCH] usb: musb: Fix fifo reads for dm816x with musb_dsps
* Andrew Morton a...@linux-foundation.org [150401 14:36]: On Wed, 18 Mar 2015 15:48:02 -0700 Tony Lindgren t...@atomide.com wrote: Looks like dm81xx can only do 32-bit fifo reads like am35x. Let's set up musb-dsps with a custom read_fifo function based on the compatible flag. Otherwise we can get the following errors when starting dhclient on a asix USB Ethernet adapter: asix 2-1:1.0 eth2: asix_rx_fixup() Bad Header Length 0x003c, offset 4 While at it, let's also remove pointless cast of the driver data. This breaks my i386 allmodconfig build. +/* Similar to am35x, dm81xx support only 32-bit read operation */ +static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) +{ + void __iomem *fifo = hw_ep-fifo; + u32 val; + int i; + + /* Read for 32bit-aligned destination address */ + if (likely((0x03 (unsigned long)dst) == 0) len = 4) { + readsl(fifo, dst, len 2); akpm3:/usr/src/linux-4.0-rc6 grep -r readsl arch/x86 akpm3:/usr/src/linux-4.0-rc6 Yes sorry, the fix is to use ioread32_rep() instead. I already sent a fix for that on the 25th as: [PATCH 1/1] usb: musb: dsps: fix build on i386 when COMPILE_TEST is set https://www.marc.info/?l=linux-usbm=142731859732737w=1 Felipe, can you please get that into Linux next? Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Fix fifo reads for dm816x with musb_dsps
On Wed, 18 Mar 2015 15:48:02 -0700 Tony Lindgren t...@atomide.com wrote: Looks like dm81xx can only do 32-bit fifo reads like am35x. Let's set up musb-dsps with a custom read_fifo function based on the compatible flag. Otherwise we can get the following errors when starting dhclient on a asix USB Ethernet adapter: asix 2-1:1.0 eth2: asix_rx_fixup() Bad Header Length 0x003c, offset 4 While at it, let's also remove pointless cast of the driver data. This breaks my i386 allmodconfig build. +/* Similar to am35x, dm81xx support only 32-bit read operation */ +static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) +{ + void __iomem *fifo = hw_ep-fifo; + u32 val; + int i; + + /* Read for 32bit-aligned destination address */ + if (likely((0x03 (unsigned long)dst) == 0) len = 4) { + readsl(fifo, dst, len 2); akpm3:/usr/src/linux-4.0-rc6 grep -r readsl arch/x86 akpm3:/usr/src/linux-4.0-rc6 + dst += len ~0x03; + len = 0x03; + } + /* + * Now read the remaining 1 to 3 byte or complete length if + * unaligned address. + */ + if (len 4) { + for (i = 0; i (len 2); i++) { + *(u32 *)dst = musb_readl(fifo, 0); + dst += 4; + } + len = 0x03; + } + if (len 0) { + val = musb_readl(fifo, 0); + memcpy(dst, val, len); + } +} -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Fix fifo reads for dm816x with musb_dsps
Hello. On 3/19/2015 1:48 AM, Tony Lindgren wrote: Looks like dm81xx can only do 32-bit fifo reads like am35x. Let's set up musb-dsps with a custom read_fifo function based on the compatible flag. Otherwise we can get the following errors when starting dhclient on a asix USB Ethernet adapter: asix 2-1:1.0 eth2: asix_rx_fixup() Bad Header Length 0x003c, offset 4 While at it, let's also remove pointless cast of the driver data. Cc: Bin Liu binml...@gmail.com Cc: Brian Hutchinson b.hutch...@gmail.com Cc: George Cherian george.cher...@ti.com Signed-off-by: Tony Lindgren t...@atomide.com --- drivers/usb/musb/musb_dsps.c | 37 - 1 file changed, 36 insertions(+), 1 deletion(-) --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -655,6 +655,36 @@ static int dsps_musb_reset(struct musb *musb) return !session_restart; } +/* Similar to am35x, dm81xx support only 32-bit read operation */ +static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) +{ + void __iomem *fifo = hw_ep-fifo; + u32 val; + int i; + + /* Read for 32bit-aligned destination address */ + if (likely((0x03 (unsigned long)dst) == 0) len = 4) { + readsl(fifo, dst, len 2); + dst += len ~0x03; + len = 0x03; + } + /* +* Now read the remaining 1 to 3 byte or complete length if +* unaligned address. +*/ This comment seems misplaced, it belongs before the next *if*. + if (len 4) { + for (i = 0; i (len 2); i++) { + *(u32 *)dst = musb_readl(fifo, 0); + dst += 4; + } Not sure how this is different to using readsl(). + len = 0x03; + } + if (len 0) { + val = musb_readl(fifo, 0); + memcpy(dst, val, len); + } +} + static struct musb_platform_ops dsps_ops = { .quirks = MUSB_INDEXED_EP, .init = dsps_musb_init, [...] WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Fix fifo reads for dm816x with musb_dsps
* Sergei Shtylyov sergei.shtyl...@cogentembedded.com [150319 10:50]: On 03/19/2015 04:30 PM, Sergei Shtylyov wrote: --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -655,6 +655,36 @@ static int dsps_musb_reset(struct musb *musb) return !session_restart; } +/* Similar to am35x, dm81xx support only 32-bit read operation */ +static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) +{ +void __iomem *fifo = hw_ep-fifo; +u32val; +inti; + +/* Read for 32bit-aligned destination address */ +if (likely((0x03 (unsigned long)dst) == 0) len = 4) { +readsl(fifo, dst, len 2); +dst += len ~0x03; +len = 0x03; +} +/* + * Now read the remaining 1 to 3 byte or complete length if + * unaligned address. + */ This comment seems misplaced, it belongs before the next *if*. +if (len 4) { +for (i = 0; i (len 2); i++) { +*(u32 *)dst = musb_readl(fifo, 0); +dst += 4; +} Not sure how this is different to using readsl(). Ah, the default implementation of musb_readl() uses __raw_readl(). So you'd probably want to keep this loop, not readsl() call. Not sure I follow you here.. Also include/asm-generic/io.h readsl() uses __raw_readl()? It seems things work with what I posted, so a readsl() loop, then just read the remaining 1 to 3 bytes. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Fix fifo reads for dm816x with musb_dsps
On 03/19/2015 04:30 PM, Sergei Shtylyov wrote: Looks like dm81xx can only do 32-bit fifo reads like am35x. Let's set up musb-dsps with a custom read_fifo function based on the compatible flag. Otherwise we can get the following errors when starting dhclient on a asix USB Ethernet adapter: asix 2-1:1.0 eth2: asix_rx_fixup() Bad Header Length 0x003c, offset 4 While at it, let's also remove pointless cast of the driver data. Cc: Bin Liu binml...@gmail.com Cc: Brian Hutchinson b.hutch...@gmail.com Cc: George Cherian george.cher...@ti.com Signed-off-by: Tony Lindgren t...@atomide.com --- drivers/usb/musb/musb_dsps.c | 37 - 1 file changed, 36 insertions(+), 1 deletion(-) --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -655,6 +655,36 @@ static int dsps_musb_reset(struct musb *musb) return !session_restart; } +/* Similar to am35x, dm81xx support only 32-bit read operation */ +static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) +{ +void __iomem *fifo = hw_ep-fifo; +u32val; +inti; + +/* Read for 32bit-aligned destination address */ +if (likely((0x03 (unsigned long)dst) == 0) len = 4) { +readsl(fifo, dst, len 2); +dst += len ~0x03; +len = 0x03; +} +/* + * Now read the remaining 1 to 3 byte or complete length if + * unaligned address. + */ This comment seems misplaced, it belongs before the next *if*. +if (len 4) { +for (i = 0; i (len 2); i++) { +*(u32 *)dst = musb_readl(fifo, 0); +dst += 4; +} Not sure how this is different to using readsl(). Ah, the default implementation of musb_readl() uses __raw_readl(). So you'd probably want to keep this loop, not readsl() call. +len = 0x03; +} +if (len 0) { +val = musb_readl(fifo, 0); +memcpy(dst, val, len); +} +} + static struct musb_platform_ops dsps_ops = { .quirks= MUSB_INDEXED_EP, .init= dsps_musb_init, [...] WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Fix fifo reads for dm816x with musb_dsps
On 03/19/2015 08:55 PM, Tony Lindgren wrote: --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -655,6 +655,36 @@ static int dsps_musb_reset(struct musb *musb) return !session_restart; } +/* Similar to am35x, dm81xx support only 32-bit read operation */ +static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) +{ +void __iomem *fifo = hw_ep-fifo; +u32val; +inti; + +/* Read for 32bit-aligned destination address */ +if (likely((0x03 (unsigned long)dst) == 0) len = 4) { +readsl(fifo, dst, len 2); +dst += len ~0x03; +len = 0x03; +} +/* + * Now read the remaining 1 to 3 byte or complete length if + * unaligned address. + */ This comment seems misplaced, it belongs before the next *if*. +if (len 4) { +for (i = 0; i (len 2); i++) { +*(u32 *)dst = musb_readl(fifo, 0); +dst += 4; +} Not sure how this is different to using readsl(). Ah, the default implementation of musb_readl() uses __raw_readl(). So you'd probably want to keep this loop, not readsl() call. Not sure I follow you here.. I just wrongly thought readsl() uses readl() internally. readl() is supposed to swap bytes when needed (BE case), while __raw_readl() is not. Also include/asm-generic/io.h readsl() uses __raw_readl()? Looking at the arch/arm/include/asm/io.h, readsl() is equivalent to __raw_readsl() too. Forgot about this asymmetry. It seems things work with what I posted, so a readsl() loop, then just read the remaining 1 to 3 bytes. In LE mode, there would have been no difference anyway. Regards, Tony WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Fix fifo reads for dm816x with musb_dsps
* Sergei Shtylyov sergei.shtyl...@cogentembedded.com [150319 06:30]: On 3/19/2015 1:48 AM, Tony Lindgren wrote: Looks like dm81xx can only do 32-bit fifo reads like am35x. Let's set up musb-dsps with a custom read_fifo function based on the compatible flag. ... --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c +/* Similar to am35x, dm81xx support only 32-bit read operation */ +static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) +{ +void __iomem *fifo = hw_ep-fifo; +u32 val; +int i; + +/* Read for 32bit-aligned destination address */ +if (likely((0x03 (unsigned long)dst) == 0) len = 4) { +readsl(fifo, dst, len 2); +dst += len ~0x03; +len = 0x03; +} +/* + * Now read the remaining 1 to 3 byte or complete length if + * unaligned address. + */ This comment seems misplaced, it belongs before the next *if*. +if (len 4) { +for (i = 0; i (len 2); i++) { +*(u32 *)dst = musb_readl(fifo, 0); +dst += 4; +} Not sure how this is different to using readsl(). +len = 0x03; +} +if (len 0) { +val = musb_readl(fifo, 0); +memcpy(dst, val, len); +} +} Indeed, thanks for looking at it. That function in the TI kernel probably had something else for the 32-bit aligned case that got swapped to use readsl(). Looks like the following works just fine for me with a USB Ethernet and variable size ping test: static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) { void __iomem *fifo = hw_ep-fifo; if (len = 4) { readsl(fifo, dst, len 2); dst += len ~0x03; len = 0x03; } /* Read any remaining 1 to 3 bytes */ if (len 0) { u32 val = musb_readl(fifo, 0); memcpy(dst, val, len); } } Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: musb: Fix fifo reads for dm816x with musb_dsps
Looks like dm81xx can only do 32-bit fifo reads like am35x. Let's set up musb-dsps with a custom read_fifo function based on the compatible flag. Otherwise we can get the following errors when starting dhclient on a asix USB Ethernet adapter: asix 2-1:1.0 eth2: asix_rx_fixup() Bad Header Length 0x003c, offset 4 While at it, let's also remove pointless cast of the driver data. Cc: Bin Liu binml...@gmail.com Cc: Brian Hutchinson b.hutch...@gmail.com Cc: George Cherian george.cher...@ti.com Signed-off-by: Tony Lindgren t...@atomide.com --- drivers/usb/musb/musb_dsps.c | 37 - 1 file changed, 36 insertions(+), 1 deletion(-) --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -655,6 +655,36 @@ static int dsps_musb_reset(struct musb *musb) return !session_restart; } +/* Similar to am35x, dm81xx support only 32-bit read operation */ +static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) +{ + void __iomem *fifo = hw_ep-fifo; + u32 val; + int i; + + /* Read for 32bit-aligned destination address */ + if (likely((0x03 (unsigned long)dst) == 0) len = 4) { + readsl(fifo, dst, len 2); + dst += len ~0x03; + len = 0x03; + } + /* +* Now read the remaining 1 to 3 byte or complete length if +* unaligned address. +*/ + if (len 4) { + for (i = 0; i (len 2); i++) { + *(u32 *)dst = musb_readl(fifo, 0); + dst += 4; + } + len = 0x03; + } + if (len 0) { + val = musb_readl(fifo, 0); + memcpy(dst, val, len); + } +} + static struct musb_platform_ops dsps_ops = { .quirks = MUSB_INDEXED_EP, .init = dsps_musb_init, @@ -802,6 +832,9 @@ static int dsps_probe(struct platform_device *pdev) } wrp = match-data; + if (of_device_is_compatible(pdev-dev.of_node, ti,musb-dm816)) + dsps_ops.read_fifo = dsps_read_fifo32; + /* allocate glue */ glue = devm_kzalloc(pdev-dev, sizeof(*glue), GFP_KERNEL); if (!glue) @@ -878,7 +911,9 @@ static const struct dsps_musb_wrapper am33xx_driver_data = { static const struct of_device_id musb_dsps_of_match[] = { { .compatible = ti,musb-am33xx, - .data = (void *) am33xx_driver_data, }, + .data = am33xx_driver_data, }, + { .compatible = ti,musb-dm816, + .data = am33xx_driver_data, }, { }, }; MODULE_DEVICE_TABLE(of, musb_dsps_of_match); -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html