Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[Benjamin Herrenschmidt]

> Basically, we only care about host devices, since anything else is going 
> to be talked to through a driver and is thus not even platform-dependent 
> any more. See what I'm saying? 

I see. Well... as long as the actual dma mapping calls are always
done at the host controller level, that's fine with me.

> .../...

> (And btw, yes, it all booted, so PPC64 is happy again. I pushed out the 
> one-liner fix).

BTW. Did you ever reproduce that lockup ?

Ben.

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[Benjamin Herrenschmidt]

> If you mean the USB target device itself, can't you walk the
> tree until you find a device that is no longer on bus_type
> usb to determine your root?

I don't feel like walking the tree on each pci_dma access

> You could stuff that into platform_data on PCI devices on your platforms.

I want automatic inheritance to child devices, shouldb't be _that_
difficult to do ;)

> I think we're not quite there yet, but once you have the device
> struct, in theory, you can walk up the tree to grab the platform_data
> for say the device's parent and do any tweaks based on platform-specific
> bus parameters.  With PCI, you could even stuff this into pci_bus->sysdata.
> 
> I think having a function pointer table for things like dma mapping
> and ioremap on all devices would be a very good thing, but not sure
> if the powers that be would allow that in 2.6.
> 
> ~Deepak
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[David Brownell]

Greg KH wrote:

> As for how ARM deals with their devices on non-pci busses, I really do
> not know, I never looked into that.

One way is that initialization specific to a given board or machine
is called from its INIT_MACHINE callback.  That can define platform
devices (SOC, ASICs, discrete parts, etc) and initialize platform_data
to feed drivers the right hardware info (addressing, which GPIOs etc).

Then normal platform_bus binding magic can apply, so that a driver
named "fred" gets probed for devices named "fred".

- Dave

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[Deepak Saxena]

On Feb 20 2004, at 18:47, Benjamin Herrenschmidt was caught saying:
> 
> > If you mean the USB target device itself, can't you walk the
> > tree until you find a device that is no longer on bus_type
> > usb to determine your root?
> 
> I don't feel like walking the tree on each pci_dma access

I never said it would be pretty. :)

> 
> > You could stuff that into platform_data on PCI devices on your platforms.
> 
> I want automatic inheritance to child devices, shouldb't be _that_
> difficult to do ;)

Hmm, I wonder if the easiet way to do this at the moment would be
to add a platform specific hook that gets called during device_add().
On arches that don't need to do this, it would just be a nop, but on 
PPC64 and others it could do whatever is required.  By the time that 
device_add() is called, it is already attached to a bus, so this 
function could walk the tree to inherit parameters at discovery time 
instead of the above suggestion. 

Seems doable without impacting other arches.

~Deepak

-- 
Deepak Saxena - dsaxena at plexity dot net - http://www.plexity.net/

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[David Brownell]

Benjamin Herrenschmidt wrote:

> Yes. I also remember a time where the dma mask for the DMA API was all
> broken too (would not be possible to map the PCI one on top of it),
> but I think that got fixed. 

I thought it was still broken.  Last I compared different asm-* arch
implementations of dma_supported(), the semantics were inconsistent.
The "mask" was sometimes ignored, sometimes treated as an upper
bound on the address, once I recall it even being used as a mask!
Some of that inconsistency seemed to come from PCI though.

- Dave

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[David Brownell]

Deepak Saxena wrote:
> On Feb 20 2004, at 18:47, Benjamin Herrenschmidt was caught saying:
> 
>>>If you mean the USB target device itself, can't you walk the
>>>tree until you find a device that is no longer on bus_type
>>>usb to determine your root?
>>
>>I don't feel like walking the tree on each pci_dma access

Why should that be needed though?


> I never said it would be pretty. :)

Well, usb_device->bus->controller is the only access that
should be needed ... much prettier than a tree walk!  It's
set up as part of device enumeration.

Some of the usb_buffer_*() mapping calls could probably
start to get inlined now, using the generic DMA calls.

- Dave

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[Benjamin Herrenschmidt]

> Well, usb_device->bus->controller is the only access that
> should be needed ... much prettier than a tree walk!  It's
> set up as part of device enumeration.
> 
> Some of the usb_buffer_*() mapping calls could probably
> start to get inlined now, using the generic DMA calls.

It all depends what the USB device driver does. If it does
pass the struct device of it's controller, it's fine. If we
want it to be able to pass its own struct device, we need
this walk... it's a matter of how we want this API to behave.

Same goes for firewire, and possibly others

Anyway, a platform hook in device_add() seem like it could be
useful for other things as well...

Ben.

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[Russell King]

On Fri, Feb 20, 2004 at 12:08:08AM -0800, David Brownell wrote:
> Benjamin Herrenschmidt wrote:
> > Yes. I also remember a time where the dma mask for the DMA API was all
> > broken too (would not be possible to map the PCI one on top of it),
> > but I think that got fixed. 
> 
> I thought it was still broken.  Last I compared different asm-* arch
> implementations of dma_supported(), the semantics were inconsistent.
> The "mask" was sometimes ignored, sometimes treated as an upper
> bound on the address, once I recall it even being used as a mask!
> Some of that inconsistency seemed to come from PCI though.

My understanding is that it's a mask, which just happens to be treated
as an upper address limit on sane platforms.

However, we have a case where it's a real mask (no surprises there!)
Intel decided not to fix a bug in a chip which means that an address
line (normally A20) must never be asserted during DMA cycles, although
it can address more than 1MB of memory.  The effect of this is that
even MB are dma-able and odd MB aren't.  Not nice.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[Russell King]

On Fri, Feb 20, 2004 at 07:48:13PM +1100, Benjamin Herrenschmidt wrote:
> Anyway, a platform hook in device_add() seem like it could be
> useful for other things as well...

What about platform_notify() which already seems to be provided for you ?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[Paulo Marques]

Andy Lutomirski wrote:

> Greg KH wrote:
> 
>>
>> Paulo Marques:
>>   o USB: fix usblp.c
>>
> 
> Unless I'm missing something, this won't fix the usblp_write spinning 
> bug I hit.  If it helps, I can try to reproduce it with some debugging 
> code attached.
> 


That is why on our latest thread about this I requested that you tested the 
patch to check your bug went way.

My patch *does* correct *a* bug. I tested it myself because I could trigger the 
bug easily, and after the patch the bug was gone.

I just wanted you to test if our bugs were different, or on the contrary, they 
are in fact the same.

They could be the same, because in my case, the driver would hang sending 
garbage to the printer. Because the printer was a dot-matrix "print what it 
receives" kind of printer, it would output the garbage continously. If it were a 
more "inteligent" printer it might refuse the garbage and not print anything at 
all, giving the same result as if nothing was being printed.

Anyway, if the bugs are different, then yes, another patch is needed to fix 
"your" bug :)

You will be the best person to do it, since you can trigger the bug, and test a 
before / after scenario.

-- 
Paulo Marques - www.grupopie.com

"In a world without walls and fences who needs windows and gates?"

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[Alan Stern]

Just to clear up a small disagreement...

On Fri, 20 Feb 2004, Linus Torvalds wrote:

> On Thu, 19 Feb 2004, David S. Miller wrote:
> > On Fri, 20 Feb 2004 18:10:41 +1100
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> > 
> > > Hrm... so if the USB device drivers are actually doing the dma mapping
> > > themselves, it make sense for them to pass their own struct device, no ?
> > 
> > That's right, at least that was the idea.
> 
> No. That would be _fundamentally_ wrong.
> 
> There's no way a USB device can do DMA in the first place. It has no DMA 
> controller, and no way to read/write memory except through the USB host.
> 
> So it is the host - and only the host - that matters. Anything else is a 
> bug.

Both of you are right.  Linus is right that USB devices don't do DMA.  
Ben is right that sometimes a USB device _driver_ will set up a DMA 
mapping.  And of course, it is set up on behalf of the corresponding USB 
controller, so the controller's struct device would be passed.

Alan Stern

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[Benjamin Herrenschmidt]

On Sat, 2004-02-21 at 06:32, Linus Torvalds wrote:
> On Fri, 20 Feb 2004, Hollis Blanchard wrote:
> > 
> > Well, I was picturing all those *_dma_supported() functions as being 
> > plugged into (new) fields in struct bus_type:
> > 	struct bus_type {
> > 		...
> > 		int (*dma_supported)(struct device *dev, u64 mask);
> > 	}
> 
> Ok, that would work. It might even be a good idea (not just DMA-related) 
> to make sure that everything you can portably "do" with a device would 
> show up as device operations. Right now it's not very well specified, and 
> there's obviously a lot of confusion.

Yes, which is why I don't agree with Hollis about having that in
bus_type, but rather in the device itself and inherit from the
parents by default ;) (Hrm... did I repeat myself ?)

The bus_type was is pretty useless for things like USB, and we
may have several controllers of the same bus with different
caracteristics (an on-chip OHCI and a PCI EHCI comes to mind)

Hrm... reading your last sentence, it seems you don't want people
to pass the struct device of the USB device but the host one instead,
fair enough...

> > If your *_dma_supported functions only take usb_dev, pci_dev, etc, then 
> > you end up with code like asm-generic/dma-mapping.h:
> 
> I agree, that is horrible. On the other hand, some architectures don't 
> need any indirection or any conditionals at all, since they know that they 
> only have one type of DMA. 
> 
> Making the device operations explicit would be good, though, and would 
> match how we do things in general. It's a fairly big change at this point, 
> but if somebody is willing to put the effort into the cleanup, then I'm 
> all for it.
> 
> I'd still ask that people don't do DMA on non-host devices. I'd rather 
> have a USB "struct device" just return "DMA not supported", to make sure 
> that everybody asks the right chip.
> 

Re: [linux-usb-devel] Re: [BK PATCH] USB update for 2.6.3

[Andy Lutomirski]

Paulo Marques wrote:

> Andy Lutomirski wrote:
> 
>> Greg KH wrote:
>>
>>>
>>> Paulo Marques:
>>>   o USB: fix usblp.c
>>>
>>
>> Unless I'm missing something, this won't fix the usblp_write spinning 
>> bug I hit.  If it helps, I can try to reproduce it with some debugging 
>> code attached.
>>
> 
> 

I added some printks to usblp_write on 2.6.1 vanilla and I couldn't reproduce 
the bug (even without your patch).  So, unless I can trigger it again, I won't 
worry about it.

Thanks.

--Andy