MUSB multiplatform work?

MUSB multiplatform work?

[Arnd Bergmann]

Hi Felipe,

We've gone through remaining work items for getting the ARM kernel
to full multiplatform support again, and MUSB came up. I'm sure you
have your own thoughts on this, but I'd like to know if there is
already a plan in place.

>From what I can see, the driver in PIO mode should almost work
on multiple platforms, but there are a couple of compile-time
dependencies in it that need to be turned into run-time conditionals.
In particular the TUSB version seem sufficiently different that
it needs some extra work to be a true run-time option.

The DMA support as far as I can tell has never been intended to
be usable in a multiplatform setup, but that also seems doable.
Looking just at the #ifdef statements in the driver, I found
that the following things need to be addressed:

* abstract musb_write_fifo and musb_read_fifo into callbacks
* move fifo_mode setting into glue driver for runtime selection
* turn TUSB compile-time switches into run-time conditionals
* turn musb_ep_select into run-time switch
* make is_dma_capable/is_cppi_enabled/tusb_dma_omap run-time conditionals
* abtract dma_controller_create/destroy interface

Aside from this, a recent discussion with Maxime has brought up
that the Allwinner A1x platform (mach-sunxi) contains an MUSB variant
that is currently used with an independently implemented device driver,
see https://github.com/linux-sunxi/linux-sunxi/tree/sunxi-3.0/drivers/usb/sun5i_usb
I wonder if you have any insight on how that can be integrated into
musb, or whether it is likely to be a compatible version to start with.

	Arnd

MUSB multiplatform work?

[Felipe Balbi]

Hi,

On Mon, May 27, 2013 at 05:02:09PM +0200, Arnd Bergmann wrote:
> Hi Felipe,
> 
> We've gone through remaining work items for getting the ARM kernel
> to full multiplatform support again, and MUSB came up. I'm sure you
> have your own thoughts on this, but I'd like to know if there is
> already a plan in place.
> 
> From what I can see, the driver in PIO mode should almost work
> on multiple platforms, but there are a couple of compile-time
> dependencies in it that need to be turned into run-time conditionals.
> In particular the TUSB version seem sufficiently different that
> it needs some extra work to be a true run-time option.

yeah, TUSB layer is quite messy, all the others should be doable though.

> The DMA support as far as I can tell has never been intended to
> be usable in a multiplatform setup, but that also seems doable.

we're looking into dmaengine for that but will take a lot of work to
have something usable.

> Looking just at the #ifdef statements in the driver, I found
> that the following things need to be addressed:
> 
> * abstract musb_write_fifo and musb_read_fifo into callbacks
> * move fifo_mode setting into glue driver for runtime selection

for the fifo mode, I'd rather detect the size of the internal fifo and
configure it dynamically based on that plus number of endpoints
configured in the IP.

> * turn TUSB compile-time switches into run-time conditionals
> * turn musb_ep_select into run-time switch
> * make is_dma_capable/is_cppi_enabled/tusb_dma_omap run-time conditionals

those can be remove, actually. Back at Nokia we did a huge cleanup on
the DMA programming part, it can be very simple with no ifdefs at all,
just needs someone to put the work and test on all platforms.

> * abtract dma_controller_create/destroy interface
> 
> Aside from this, a recent discussion with Maxime has brought up
> that the Allwinner A1x platform (mach-sunxi) contains an MUSB variant
> that is currently used with an independently implemented device driver,
> see https://github.com/linux-sunxi/linux-sunxi/tree/sunxi-3.0/drivers/usb/sun5i_usb
> I wonder if you have any insight on how that can be integrated into
> musb, or whether it is likely to be a compatible version to start with.

just write a glue layer, should be as easy as that :-)

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130528/7c6e6c26/attachment-0001.sig>

MUSB multiplatform work?

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [130528 09:42]:
> Hi,
> 
> On Mon, May 27, 2013 at 05:02:09PM +0200, Arnd Bergmann wrote:
> > Hi Felipe,
> > 
> > We've gone through remaining work items for getting the ARM kernel
> > to full multiplatform support again, and MUSB came up. I'm sure you
> > have your own thoughts on this, but I'd like to know if there is
> > already a plan in place.
> > 
> > From what I can see, the driver in PIO mode should almost work
> > on multiple platforms, but there are a couple of compile-time
> > dependencies in it that need to be turned into run-time conditionals.
> > In particular the TUSB version seem sufficiently different that
> > it needs some extra work to be a true run-time option.
> 
> yeah, TUSB layer is quite messy, all the others should be doable though.

TUSB we can make depend on ARMv7, the only implementation we have
is for omap2420, which is ARMv6. This should solve the issue for
ARMv7 multiplatform builds for now.
 
> > The DMA support as far as I can tell has never been intended to
> > be usable in a multiplatform setup, but that also seems doable.
> 
> we're looking into dmaengine for that but will take a lot of work to
> have something usable.

TUSB would work with dmaengine, but AFAIK we're still missing the
dmaengine configuration options to support increasing device end
FIFO address.
 
> > Looking just at the #ifdef statements in the driver, I found
> > that the following things need to be addressed:
> > 
> > * abstract musb_write_fifo and musb_read_fifo into callbacks
> > * move fifo_mode setting into glue driver for runtime selection
> 
> for the fifo mode, I'd rather detect the size of the internal fifo and
> configure it dynamically based on that plus number of endpoints
> configured in the IP.
> 
> > * turn TUSB compile-time switches into run-time conditionals
> > * turn musb_ep_select into run-time switch
> > * make is_dma_capable/is_cppi_enabled/tusb_dma_omap run-time conditionals
> 
> those can be remove, actually. Back at Nokia we did a huge cleanup on
> the DMA programming part, it can be very simple with no ifdefs at all,
> just needs someone to put the work and test on all platforms.
> 
> > * abtract dma_controller_create/destroy interface
> > 
> > Aside from this, a recent discussion with Maxime has brought up
> > that the Allwinner A1x platform (mach-sunxi) contains an MUSB variant
> > that is currently used with an independently implemented device driver,
> > see https://github.com/linux-sunxi/linux-sunxi/tree/sunxi-3.0/drivers/usb/sun5i_usb
> > I wonder if you have any insight on how that can be integrated into
> > musb, or whether it is likely to be a compatible version to start with.
> 
> just write a glue layer, should be as easy as that :-)

Yes the MUSB code should be able to support it considering the number
of implementations we already have there.

Regards,

Tony

MUSB multiplatform work?

[Linus Walleij]

On Thu, May 30, 2013 at 10:18 PM, Tony Lindgren <tony@atomide.com> wrote:

> TUSB would work with dmaengine, but AFAIK we're still missing the
> dmaengine configuration options to support increasing device end
> FIFO address.

Can you elaborate on this? What is the usecase here?

Yours,
Linus Walleij

MUSB multiplatform work?

[Tony Lindgren]

* Linus Walleij <linus.walleij@linaro.org> [130530 13:27]:
> On Thu, May 30, 2013 at 10:18 PM, Tony Lindgren <tony@atomide.com> wrote:
> 
> > TUSB would work with dmaengine, but AFAIK we're still missing the
> > dmaengine configuration options to support increasing device end
> > FIFO address.
> 
> Can you elaborate on this? What is the usecase here?

There are many devices where the device FIFO is memory mapped to the
GPMC bus on omaps, like TUSB, OneNAND, smc911x etc. I believe the
only reason why these have not been converted to the dmaengine is
the fact that dmaengine cannot configure the DMA hardware to do
autoincrement and loop over the device FIFO.

You can see an example of this in tusb_omap_dma_program() if you
look at the various dma_params entries and omap_set_dma_* functions.

Regards,

Tony

MUSB multiplatform work?

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [130530 13:25]:
> 
> TUSB we can make depend on ARMv7, the only implementation we have
> is for omap2420, which is ARMv6. This should solve the issue for
> ARMv7 multiplatform builds for now.

Sorry mean depend on !ARMv7. But thinking about it more, that does
not make much sense as it would break things for omap2plus_defconfig
at least which is multiplatform v6 and v7. For now TUSB can be made
to depend on MACH_NOKIA_N8X0. I doubt that anybody is using the add
on cards for H4 boards any longer..

Regards,

Tony

MUSB multiplatform work?

[Linus Walleij]

On Thu, May 30, 2013 at 10:31 PM, Tony Lindgren <tony@atomide.com> wrote:

> There are many devices where the device FIFO is memory mapped to the
> GPMC bus on omaps, like TUSB, OneNAND, smc911x etc. I believe the
> only reason why these have not been converted to the dmaengine is
> the fact that dmaengine cannot configure the DMA hardware to do
> autoincrement and loop over the device FIFO.

OK that seems like something pretty generic that we could just add
to the struct dma_slave_config actually, something like:

u32 src_fifoloop;
u32 dst_fifoloop;

Given in # of words on the src/dst address simply, left as zero
for hitting a constant address over and over again.

We'd need both to make space for device->device transfers.

If this is all that is needed to convert them do DMAengine
I'd surely ACK it (FWIW).

Yours,
Linus Walleij

MUSB multiplatform work?

[Felipe Balbi]

On Thu, May 30, 2013 at 01:54:49PM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [130530 13:25]:
> > 
> > TUSB we can make depend on ARMv7, the only implementation we have
> > is for omap2420, which is ARMv6. This should solve the issue for
> > ARMv7 multiplatform builds for now.
> 
> Sorry mean depend on !ARMv7. But thinking about it more, that does
> not make much sense as it would break things for omap2plus_defconfig
> at least which is multiplatform v6 and v7. For now TUSB can be made
> to depend on MACH_NOKIA_N8X0. I doubt that anybody is using the add
> on cards for H4 boards any longer..

could also be made to depend on BROKEN. I don't think anyone *really*
cares about that glue layer other than the very few in the world who
still like the nostalgic feeling of booting current linux releases on
n8x0 devices.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130531/90cafc85/attachment.sig>

MUSB multiplatform work?

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [130530 21:06]:
> On Thu, May 30, 2013 at 01:54:49PM -0700, Tony Lindgren wrote:
> > * Tony Lindgren <tony@atomide.com> [130530 13:25]:
> > > 
> > > TUSB we can make depend on ARMv7, the only implementation we have
> > > is for omap2420, which is ARMv6. This should solve the issue for
> > > ARMv7 multiplatform builds for now.
> > 
> > Sorry mean depend on !ARMv7. But thinking about it more, that does
> > not make much sense as it would break things for omap2plus_defconfig
> > at least which is multiplatform v6 and v7. For now TUSB can be made
> > to depend on MACH_NOKIA_N8X0. I doubt that anybody is using the add
> > on cards for H4 boards any longer..
> 
> could also be made to depend on BROKEN. I don't think anyone *really*
> cares about that glue layer other than the very few in the world who
> still like the nostalgic feeling of booting current linux releases on
> n8x0 devices.

Unfortunately that's the best test case we have for GPMC and DMA stress
testing so it's good to keep around.

And n8x0 is still quite usable general purpose pocket computer with
keyboard and WLAN and tons of storage :)

Regards,

Tony

MUSB multiplatform work?

[Jassi Brar]

On 31 May 2013 02:01, Tony Lindgren <tony@atomide.com> wrote:
> * Linus Walleij <linus.walleij@linaro.org> [130530 13:27]:
>> On Thu, May 30, 2013 at 10:18 PM, Tony Lindgren <tony@atomide.com> wrote:
>>
>> > TUSB would work with dmaengine, but AFAIK we're still missing the
>> > dmaengine configuration options to support increasing device end
>> > FIFO address.
>>
>> Can you elaborate on this? What is the usecase here?
>
> There are many devices where the device FIFO is memory mapped to the
> GPMC bus on omaps, like TUSB, OneNAND, smc911x etc. I believe the
> only reason why these have not been converted to the dmaengine is
> the fact that dmaengine cannot configure the DMA hardware to do
> autoincrement and loop over the device FIFO.
>
Doesn't device_prep_interleaved_dma() help?

-jassi

MUSB multiplatform work?

[Vinod Koul]

On Thu, May 30, 2013 at 11:19:33PM +0200, Linus Walleij wrote:
> On Thu, May 30, 2013 at 10:31 PM, Tony Lindgren <tony@atomide.com> wrote:
> 
> > There are many devices where the device FIFO is memory mapped to the
> > GPMC bus on omaps, like TUSB, OneNAND, smc911x etc. I believe the
> > only reason why these have not been converted to the dmaengine is
> > the fact that dmaengine cannot configure the DMA hardware to do
> > autoincrement and loop over the device FIFO.
> 
> OK that seems like something pretty generic that we could just add
> to the struct dma_slave_config actually, something like:
> 
> u32 src_fifoloop;
> u32 dst_fifoloop;
Yes for genric but not for loop. For most of our cases we are considering the
FIFO address as a constant. Typically DMA controllers have this ability to
perform incremental/decremental/constant FIFO address.

What would make sense to have is:

enum dmaengine_slave_addr_mode {
	DMAENGINE_SLAVE_ADDR_CONSTANT = 0,
	DMAENGINE_SLAVE_ADDR_INCREMANT,
	DMAENGINE_SLAVE_ADDR_DECREMENT,
};

and add these to struct dma_slave_config:
	enum dmaengine_slave_addr_mode src_mode;
	enum dmaengine_slave_addr_mode dstn_mode;

For loopover we can have:
	u32 loop_counter;

on 0 means no loop, and valid value tells when to loopover (offset).

will this help for all of the above controllers and their conversion?
 
> Given in # of words on the src/dst address simply, left as zero
> for hitting a constant address over and over again.
> 
> We'd need both to make space for device->device transfers.
> 
> If this is all that is needed to convert them do DMAengine
> I'd surely ACK it (FWIW).

--
~Vinod

MUSB multiplatform work?

[Jassi Brar]

On 12 June 2013 15:35, Vinod Koul <vinod.koul@intel.com> wrote:
> On Thu, May 30, 2013 at 11:19:33PM +0200, Linus Walleij wrote:
>> On Thu, May 30, 2013 at 10:31 PM, Tony Lindgren <tony@atomide.com> wrote:
>>
>> > There are many devices where the device FIFO is memory mapped to the
>> > GPMC bus on omaps, like TUSB, OneNAND, smc911x etc. I believe the
>> > only reason why these have not been converted to the dmaengine is
>> > the fact that dmaengine cannot configure the DMA hardware to do
>> > autoincrement and loop over the device FIFO.
>>
>> OK that seems like something pretty generic that we could just add
>> to the struct dma_slave_config actually, something like:
>>
>> u32 src_fifoloop;
>> u32 dst_fifoloop;
> Yes for genric but not for loop. For most of our cases we are considering the
> FIFO address as a constant. Typically DMA controllers have this ability to
> perform incremental/decremental/constant FIFO address.
>
> What would make sense to have is:
>
> enum dmaengine_slave_addr_mode {
>         DMAENGINE_SLAVE_ADDR_CONSTANT = 0,
>         DMAENGINE_SLAVE_ADDR_INCREMANT,
>         DMAENGINE_SLAVE_ADDR_DECREMENT,
> };
>
> and add these to struct dma_slave_config:
>         enum dmaengine_slave_addr_mode src_mode;
>         enum dmaengine_slave_addr_mode dstn_mode;
>
> For loopover we can have:
>         u32 loop_counter;
>
> on 0 means no loop, and valid value tells when to loopover (offset).
>
> will this help for all of the above controllers and their conversion?
>
IIRC,  TI's Sundaram Raju proposed a TI specific api to DMAEngine for
this very purpose, which was generalized into
device_prep_interleaved_dma().  Which I think should already be enough
to serve the purpose. Is it not?

-jassi

MUSB multiplatform work?

[Vinod Koul]

On Wed, Jun 12, 2013 at 05:27:53PM +0530, Jassi Brar wrote:
> IIRC,  TI's Sundaram Raju proposed a TI specific api to DMAEngine for
> this very purpose, which was generalized into
> device_prep_interleaved_dma().  Which I think should already be enough
> to serve the purpose. Is it not?
The interleaved for having to get/set data from interleaved or an 2d array.
Think of a raw image from camera where you need to get some region only and skip
rest. In those case interleaved API helps

Here this is just normal slave DMA with changing FIFO address and which just
loops over the FIFO value

--
~Vinod

MUSB multiplatform work?

[Jassi Brar]

On Mon, Jun 17, 2013 at 7:13 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Wed, Jun 12, 2013 at 05:27:53PM +0530, Jassi Brar wrote:
>> IIRC,  TI's Sundaram Raju proposed a TI specific api to DMAEngine for
>> this very purpose, which was generalized into
>> device_prep_interleaved_dma().  Which I think should already be enough
>> to serve the purpose. Is it not?
> The interleaved for having to get/set data from interleaved or an 2d array.
> Think of a raw image from camera where you need to get some region only and skip
> rest. In those case interleaved API helps
>
IIRC I designed the interleaved api ;)  and I am sure it was not
designed solely for async, rather the first users of the api are
slave.

> Here this is just normal slave DMA with changing FIFO address and which just
> loops over the FIFO value
>
It is possible that I didn't understand the OMAP usecase and the
interleave api won't work, but if it does work please let us not
introduce yet another api for a 'normal' usecase.

In fact my bigger plan was to call the api 'generic' and have it
eventually consume most, if not all, dma_transaction_type's but then
we couldn't see the same and I got busy with non-dma stuff ...
anyways.

cheers.