Re: [PATCH 1/2] leds: bcm6328: improve write and read functions

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

Hi!

> >> Yeah, but ideally you should not be copying comments; there should be
> >> one central place which does it and does it right.
> > 
> > I’m open to suggestions :).
> > Which central place would be a good place for you?
> 
> I did consider creating an include/linux/brcm/brcm_io.h header or
> something like that but I am really not sure what the benefit would
> be.

Less code duplication? It is immediately clear that driver including
this is specific for brcm SoCs and would not try to work somewhere else?

> As far as using _relaxed() this is absolutely correct because the bus
> logic that connects the CPU to its on-chip registers is non re-ordering
> non posted. That is true on the MIPS BE/LE and ARM when configured in LE
> or BE.

If that's right on particular SoC, then _relaxed and normal versions
should be same; drivers still need to use normal versions, because
they may be running on different SoC...?

> We need the swapping for ARM because when running in ARM BE32, the data
> is going to be in the host CPU endian, but the register bus is hard
> wired to little endian.

Yeah I see you need to do some byteswapping. But I'm pretty sure not
all MIPS BE boxes do the magic swapping, right? And drivers/leds is
not a place where you encode knowledge about SoC byte swapping.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]