RE: [PATCH 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hi, > >It would not scale very well - we already have multi-omap builds > >that select support for multiple boards. If the AM35x boards are > >part of such builds, then mutually exclusive config options won't > >work - already n8x0 MUSB is exclusive with 243x/omap3. > > > >If it is possible to detect the AM35x at runtime, then we could > >handle this well. Also, a similar set of changes will be needed for > >the DMA code as well (right now we can pick only one of the DMA > >engines at a time). > > it's time to split out platform code from musb core. I was thinking of > having omap2430.c blackfin.c tusb6010.c davinci.c be a platform_device > that instantiates musb_hdrc platform_device. It would also ioremap() the > area and pass the gotten/enabled clock to musb. Then we can have all of > the platforms enabled since the board files would pass down the > platform_device for the platform code. Something like: This approach would anyway not help the original issue discussed in this thread. We need to have some config option to differentiate musb ips between OMAP3 and AM35x. Another issue: Currently almost 90% of the code is same between drivers/usb/musb/da8xx.c and drivers/usb/musb/am3517.c. any idea on how to avoid duplication? Can we merge these two files and have compilation flags based on musb capability alone (like HAS_CPPI41) rather than CONFIG_ARCH_xx? This approach would also help the original issue of discussed in this thread. -Ajay > > arch/arm/mach-omap2/usb-musb.c: > [..] -- 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 3/3 v2] musb: AM35x: Workaround for fifo read issue
On Thu, May 20, 2010 at 12:06:30PM +0200, Balbi Felipe (Nokia-D/Helsinki) wrote: arm-based only) boards and be sure it would work. We would just need to put something similar down for dma engines. and of course, the same platform-code could instantiate a dma engine platform_device but that's trickier, I guess, since the base address would have to be used by two drivers, then platform code would have to handle synchronization. -- balbi DefectiveByDesign.org -- 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 3/3 v2] musb: AM35x: Workaround for fifo read issue
On Thu, May 20, 2010 at 11:41:14AM +0200, ext Gadiyar, Anand wrote:
It would not scale very well - we already have multi-omap builds
that select support for multiple boards. If the AM35x boards are
part of such builds, then mutually exclusive config options won't
work - already n8x0 MUSB is exclusive with 243x/omap3.
If it is possible to detect the AM35x at runtime, then we could
handle this well. Also, a similar set of changes will be needed for
the DMA code as well (right now we can pick only one of the DMA
engines at a time).
it's time to split out platform code from musb core. I was thinking of
having omap2430.c blackfin.c tusb6010.c davinci.c be a platform_device
that instantiates musb_hdrc platform_device. It would also ioremap() the
area and pass the gotten/enabled clock to musb. Then we can have all of
the platforms enabled since the board files would pass down the
platform_device for the platform code. Something like:
arch/arm/mach-omap2/usb-musb.c:
static struct platform_device omap_musb = {
.dev= {
.name = "omap-hsusbotg",/* using the IP block name */
},
[..]
};
[..]
platform_device_register(&omap_musb);
drivers/usb/musb/omap2430.c:
static struct musb_platform_data omap_musb_data;
static int __init omap_musb_probe(struct platform_device *pdev)
{
clk = clk_get(&pdev->dev, "ick");
base = ioremap(res->start, resource_size(res));
omap_musb_data.clk = clk;
omap_musb_data.base = base;
[... all other necessary fields, like mode, etc]
musb_pdev = platform_device_alloc("musb_hdrc", -1);
musb_pdev->dev.parent = &pdev->dev;
platform_device_add_data(musb_pdev, &omap_musb_data);
platform_device_add(musb_pdev);
return 0;
}
static int __exit omap_musb_remove(struct platform_device *pdev)
{
clk_put(clk);
iounmap(base);
platform_device_del(musb_pdev);
return 0;
}
[...]
io functions could also be passed through this platform_device so that
we remove the insane amounts of ifdefs. Also context save/restore could
be done a per-platform basis. Since the platform-code would be a parent
to musb_hdrc, platform-code's suspend would be called before musb_hdrc
suspend, then we would save context at that point already.
Any other ifdefferry on anything would be easier to remove with this
approach and it also means we can have one musb_hdrc.ko for all (well
arm-based only) boards and be sure it would work. We would just need to
put something similar down for dma engines.
--
balbi
DefectiveByDesign.org
--
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 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hi, > > Fixes/workaround based on CONFIG_MACH_OMAP3517EVM will be good only for > > OMAP3517EVM and would not scale well to other boards based on AM35x. > > (As commented earlier by Sergei) > > > > I just got to know of another board "LIZARD" based on AM35x so we really > > need to find a solution for this. > > > > This problem is due to the fact that AM35x is based on OMAP35x but musb > ip > > Is updated to RTL1.8 using CPPI4.1 DMA engine thus we need to have a > > config option to differentiate musb ips between actual OMAP3 and AM35x > > platforms. > > > > I am thinking of adding new config option OMAP_MUSB_RTL18 which should > be > > selected by all the boards based on AM35x in arch/arm/mach- > omap2/Kconfig. > > > > The same config option can be used for all the workaround/fixes specific > > to AM35x musb platform. > > > > -- > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig > > index b72ae06..3ab1156 100644 > > --- a/arch/arm/mach-omap2/Kconfig > > +++ b/arch/arm/mach-omap2/Kconfig > > @@ -95,6 +95,7 @@ config MACH_OMAP3517EVM > > bool "OMAP3517/ AM3517 EVM board" > > depends on ARCH_OMAP3 && ARCH_OMAP34XX > > select OMAP_PACKAGE_CBB > > + select OMAP_MUSB_RTL18 > > > > config PMIC_TPS65023 > > bool "TPS65023 Power Module" > > -- > > > > Does anyone has a better option to fix this issue or any > > comment on this approach? > > > > It would not scale very well - we already have multi-omap builds > that select support for multiple boards. If the AM35x boards are > part of such builds, then mutually exclusive config options won't > work - already n8x0 MUSB is exclusive with 243x/omap3. AM35x musb needs different initialization sequence which involves PHY configuration etc. being done in drivers/usb/musb/am3517.c. am3517.c also need to handle different ISR routine and all CPPi4.1 DMA related programmings. > > If it is possible to detect the AM35x at runtime, then we could > handle this well. > Also, a similar set of changes will be needed for > the DMA code as well (right now we can pick only one of the DMA > engines at a time). We do have such mechanism cpu_is_omap3517() but how about compilation flags for am3517.c? Do you intend to have different initialization sequence, ISR function and CPPI4.1 DMA programmings within drivers/usb/musb/omap2430.c ? -Ajay > > - Anand -- 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 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hi, > Gupta, Ajay Kumar wrote: > > >> AM35x supports only 32bit read operations so we need to have > >> workaround for 8bit and 16bit read operations. > > >> Signed-off-by: Ajay Kumar Gupta [...] > >> > >> +#if !defined(CONFIG_MACH_OMAP3517EVM) > > > Fixes/workaround based on CONFIG_MACH_OMAP3517EVM will be good only for > > OMAP3517EVM and would not scale well to other boards based on AM35x. > > (As commented earlier by Sergei) > > > I just got to know of another board "LIZARD" based on AM35x so we really > > need to find a solution for this. > > > This problem is due to the fact that AM35x is based on OMAP35x but musb > ip > > Is updated to RTL1.8 using CPPI4.1 DMA engine > > Is that really the only difference? Other difference: USB PHY is built inside the ip itself. Additionally, there is an errata which restricts byte/word (8/16 bit) data read which is due to the bus interface on AM35x. > > > thus we need to have a > > config option to differentiate musb ips between actual OMAP3 and AM35x > > platforms. > > > I am thinking of adding new config option OMAP_MUSB_RTL18 which should > be > > selected by all the boards based on AM35x in arch/arm/mach- > omap2/Kconfig. > > > The same config option can be used for all the workaround/fixes specific > > to AM35x musb platform. > > > -- > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig > > index b72ae06..3ab1156 100644 > > --- a/arch/arm/mach-omap2/Kconfig > > +++ b/arch/arm/mach-omap2/Kconfig > > @@ -95,6 +95,7 @@ config MACH_OMAP3517EVM > > bool "OMAP3517/ AM3517 EVM board" > > depends on ARCH_OMAP3 && ARCH_OMAP34XX > > select OMAP_PACKAGE_CBB > > + select OMAP_MUSB_RTL18 > > > > config PMIC_TPS65023 > > bool "TPS65023 Power Module" > > -- > > > > Does anyone has a better option to fix this issue or any comment on this > > approach? > > Why not introduce CONFIG_ARCH_AM35x instead? This is fine with me but don't know if Tony would agree with it. -Ajay > > 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 3/3 v2] musb: AM35x: Workaround for fifo read issue
Gupta, Ajay Kumar wrote: > Fixes/workaround based on CONFIG_MACH_OMAP3517EVM will be good only for > OMAP3517EVM and would not scale well to other boards based on AM35x. > (As commented earlier by Sergei) > > I just got to know of another board "LIZARD" based on AM35x so we really > need to find a solution for this. > > This problem is due to the fact that AM35x is based on OMAP35x but musb ip > Is updated to RTL1.8 using CPPI4.1 DMA engine thus we need to have a > config option to differentiate musb ips between actual OMAP3 and AM35x > platforms. > > I am thinking of adding new config option OMAP_MUSB_RTL18 which should be > selected by all the boards based on AM35x in arch/arm/mach-omap2/Kconfig. > > The same config option can be used for all the workaround/fixes specific > to AM35x musb platform. > > -- > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig > index b72ae06..3ab1156 100644 > --- a/arch/arm/mach-omap2/Kconfig > +++ b/arch/arm/mach-omap2/Kconfig > @@ -95,6 +95,7 @@ config MACH_OMAP3517EVM > bool "OMAP3517/ AM3517 EVM board" > depends on ARCH_OMAP3 && ARCH_OMAP34XX > select OMAP_PACKAGE_CBB > + select OMAP_MUSB_RTL18 > > config PMIC_TPS65023 > bool "TPS65023 Power Module" > -- > > Does anyone has a better option to fix this issue or any > comment on this approach? > It would not scale very well - we already have multi-omap builds that select support for multiple boards. If the AM35x boards are part of such builds, then mutually exclusive config options won't work - already n8x0 MUSB is exclusive with 243x/omap3. If it is possible to detect the AM35x at runtime, then we could handle this well. Also, a similar set of changes will be needed for the DMA code as well (right now we can pick only one of the DMA engines at a time). - Anand -- 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 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hello. Gupta, Ajay Kumar wrote: AM35x supports only 32bit read operations so we need to have workaround for 8bit and 16bit read operations. Signed-off-by: Ajay Kumar Gupta --- Patch created against linus'tree + all musb patches in Greg's queue Changes from v1: - removed unnecessary parens. - Removed 'memcpy' for 32 bit read loops. drivers/usb/musb/am3517.c| 30 ++ drivers/usb/musb/musb_core.c |2 ++ 2 files changed, 32 insertions(+), 0 deletions(-) diff --git a/drivers/usb/musb/am3517.c b/drivers/usb/musb/am3517.c index b74e664..3299c66 100644 --- a/drivers/usb/musb/am3517.c +++ b/drivers/usb/musb/am3517.c [...] + } +} diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 4093f6d..9c59a8e 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -262,6 +262,7 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len, const u8 *src) } } +#if !defined(CONFIG_MACH_OMAP3517EVM) Fixes/workaround based on CONFIG_MACH_OMAP3517EVM will be good only for OMAP3517EVM and would not scale well to other boards based on AM35x. (As commented earlier by Sergei) I just got to know of another board "LIZARD" based on AM35x so we really need to find a solution for this. This problem is due to the fact that AM35x is based on OMAP35x but musb ip Is updated to RTL1.8 using CPPI4.1 DMA engine Is that really the only difference? thus we need to have a config option to differentiate musb ips between actual OMAP3 and AM35x platforms. I am thinking of adding new config option OMAP_MUSB_RTL18 which should be selected by all the boards based on AM35x in arch/arm/mach-omap2/Kconfig. The same config option can be used for all the workaround/fixes specific to AM35x musb platform. -- diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index b72ae06..3ab1156 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -95,6 +95,7 @@ config MACH_OMAP3517EVM bool "OMAP3517/ AM3517 EVM board" depends on ARCH_OMAP3 && ARCH_OMAP34XX select OMAP_PACKAGE_CBB + select OMAP_MUSB_RTL18 config PMIC_TPS65023 bool "TPS65023 Power Module" -- Does anyone has a better option to fix this issue or any comment on this approach? Why not introduce CONFIG_ARCH_AM35x instead? 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 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hi, > AM35x supports only 32bit read operations so we need to have > workaround for 8bit and 16bit read operations. > > Signed-off-by: Ajay Kumar Gupta > --- > Patch created against linus'tree + all musb patches in Greg's queue > Changes from v1: > - removed unnecessary parens. > - Removed 'memcpy' for 32 bit read loops. > > drivers/usb/musb/am3517.c| 30 ++ > drivers/usb/musb/musb_core.c |2 ++ > 2 files changed, 32 insertions(+), 0 deletions(-) > > diff --git a/drivers/usb/musb/am3517.c b/drivers/usb/musb/am3517.c > index b74e664..3299c66 100644 > --- a/drivers/usb/musb/am3517.c > +++ b/drivers/usb/musb/am3517.c [...] > + } > +} > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > index 4093f6d..9c59a8e 100644 > --- a/drivers/usb/musb/musb_core.c > +++ b/drivers/usb/musb/musb_core.c > @@ -262,6 +262,7 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 > len, const u8 *src) > } > } > > +#if !defined(CONFIG_MACH_OMAP3517EVM) Fixes/workaround based on CONFIG_MACH_OMAP3517EVM will be good only for OMAP3517EVM and would not scale well to other boards based on AM35x. (As commented earlier by Sergei) I just got to know of another board "LIZARD" based on AM35x so we really need to find a solution for this. This problem is due to the fact that AM35x is based on OMAP35x but musb ip Is updated to RTL1.8 using CPPI4.1 DMA engine thus we need to have a config option to differentiate musb ips between actual OMAP3 and AM35x platforms. I am thinking of adding new config option OMAP_MUSB_RTL18 which should be selected by all the boards based on AM35x in arch/arm/mach-omap2/Kconfig. The same config option can be used for all the workaround/fixes specific to AM35x musb platform. -- diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index b72ae06..3ab1156 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -95,6 +95,7 @@ config MACH_OMAP3517EVM bool "OMAP3517/ AM3517 EVM board" depends on ARCH_OMAP3 && ARCH_OMAP34XX select OMAP_PACKAGE_CBB + select OMAP_MUSB_RTL18 config PMIC_TPS65023 bool "TPS65023 Power Module" -- Does anyone has a better option to fix this issue or any comment on this approach? Regards, Ajay > /* > * Unload an endpoint's FIFO > */ > @@ -299,6 +300,7 @@ void musb_read_fifo(struct musb_hw_ep *hw_ep, u16 len, > u8 *dst) > readsb(fifo, dst, len); > } > } > +#endif > > #endif /* normal PIO */ > > -- > 1.6.2.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
RE: [PATCH 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hi > AM35x supports only 32bit read operations so we need to have > workaround for 8bit and 16bit read operations. > But don't we need to override musb_readb() and musb_readw() then? Yes, Correct. I do have another patch on that which I will cleanup and submit later. Anyways with currebt three patch set, basic Host and device operation has been tested. -Ajay > Signed-off-by: Ajay Kumar Gupta >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 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hello. Ajay Kumar Gupta wrote: AM35x supports only 32bit read operations so we need to have workaround for 8bit and 16bit read operations. But don't we need to override musb_readb() and musb_readw() then? Signed-off-by: Ajay Kumar Gupta 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 3/3 v2] musb: AM35x: Workaround for fifo read issue
Ajay Kumar Gupta wrote:
> AM35x supports only 32bit read operations so we need to have
> workaround for 8bit and 16bit read operations.
>
> Signed-off-by: Ajay Kumar Gupta
> ---
> drivers/usb/musb/am3517.c | 30 ++
> drivers/usb/musb/musb_core.c | 2 ++
> 2 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/musb/am3517.c b/drivers/usb/musb/am3517.c
> index dd9e883..24bdc2e 100644
> --- a/drivers/usb/musb/am3517.c
> +++ b/drivers/usb/musb/am3517.c
> @@ -515,3 +515,33 @@ void musb_platform_restore_context(struct
> musb_context_registers
> phy_on();
> }
> #endif
> +
> +/* AM35x supports only 32bit read operation */
> +void musb_read_fifo(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 rest 1 to 3 bytes or complete length if
check multiline comments style
> + * unaligned address.
> + */
> + if (len > 4) {
> + for (i = 0; i < (len >> 2); i++) {
> + val = musb_readl(fifo, 0);
> + memcpy(dst, &val, 4);
> + dst += 4;
> + }
> + len %= 4;
> + }
> + if (len > 0) {
> + val = musb_readl(fifo, 0);
> + memcpy(dst, &val, len);
> + }
> +}
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 705cc4a..bc2cf14 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -191,6 +191,7 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len,
> const u8 *src)
> }
> }
>
> +#if !defined(CONFIG_MACH_OMAP3517EVM)
> /*
> * Unload an endpoint's FIFO
> */
> @@ -228,6 +229,7 @@ void musb_read_fifo(struct musb_hw_ep *hw_ep, u16 len, u8
> *dst)
> readsb(fifo, dst, len);
> }
> }
> +#endif
>
> #endif /* normal PIO */
>
> --
> 1.6.2.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
