Re: [PATCH] USB/ISP1760: Fix for unaligned exceptions

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

Hennerich, Michael wrote:
> I know the issue is originated in either RTL8150 set_registers or
> get_registers. We get some unaligned address from the stack to the
> ISP1760 priv_read/wite_copy.
> 
> The RTL8150 driver does something like this:
> 
> 	u8 data[3], tmp;
> 
> 	data[0] = phy;
> 	data[1] = data[2] = 0;
> 	tmp = indx | PHY_READ | PHY_GO;
> 	i = 0;
> 
> 	set_registers(dev, PHYADD, sizeof(data), data);

ach. So that's wrong anyway. There are arches which can't DMA stack 
memory. So fixing this properly does not fix just your arch.

>>> I wonder if it's only us (NOMMU) seeing these odd aligned buffers?
>> Not sure. The only problem I have with this patch is that you might
>> cover bugs in drivers and you don't notice it anymore since you choose
>> "voluntary" the slow path.
> 
> Well here I disagree, but I agree with the fact that there are buggy
> drivers.
> 
> Since most processors running Linux do have unaligned access handling,
> this issue goes unnoticed for all of them. Believe me the penalty taken
> by any Processor doing this automatically and unnoticed is typically
> much higher than using get/put_unaligned.
Okay. A packed struct with a u8 followed by u16 which is required by the 
spec can't be fixed. unaligned helper is the only solution. I agree here. 
Allocating memory on the stack for a dma transfer is wrong.
On PowerPC and X86 get_unaligned() does not behave any different than a 
normal dereference. So I doubt that there is a performance improvement.

> I'm tiered fixing all unaligned issues in drivers. It's a hassle getting
> them merged, since most people don't care. Having a workaround in a
> single place, the hcd driver is much easier.

Having a fixup in the exception handler like sparc does is probably little 
slower than the fixup here. On the other hand you would not have to fix 
unaligned access anymore.

>>> -Michael

Sebastian