linux-kernel - Re: [PATCH] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 05 Feb 2013 13:29:27 +1100
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	Michal Simek <monstr@...str.eu>,
	Vineet Gupta <Vineet.Gupta1@...opsys.com>,
	Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
	linux-kernel@...r.kernel.org, grant.likely@...retlab.ca,
	alan@...rguk.ukuu.org.uk, geert@...ux-m68k.org,
	dahinds@...rs.sourceforge.net
Subject: Re: [PATCH] drivers/block/xsysace - replace
 in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)

On Mon, 2013-02-04 at 17:24 +0000, Arnd Bergmann wrote:
> On Monday 04 February 2013, Michal Simek wrote:
> > >
> > > and select the CONFIG_FOO_BIG_ENDIAN and CONFIG_FOO_LITTLE_ENDIAN
> > > symbols in Kconfig based on the system you are building for.
> > 
> > Using CONFIG_FOO_BIG/LITTLE is not good because it is just another
> > Kconfig option.
> > You can easily detect it at runtime and for dedicated hw with fixed
> > endians you can just
> > handle it in the driver and don't care about global setting.
> 
> The configuration option should not be visible, so it's
> not something a user would have to worry about. It's just
> sometimes nicer to express the configuration of the platform
> in terms of Kconfig syntax than it is in C code if you have
> complex platform dependencies.

As long as you never have a case where both are needed at runtime...

> > > This of course gets further complicated if you require different
> > > accessors per architecture, like ARM wanting readl or ioread32
> > > and PowerPC wanting in_le32 for a little-endian SoC component.

powerpc should never "want" in_le32. ioread32 should work just as well.
in_le32 is historical stuff and is never required.
 
> > FYI: I have got two responses on linux-arch from Alan
> > "Set the pointers up and pass them as data with your platform device, that
> > way the function definitions are buried in your platform code where they
> > depend."
> > 
> > and Geert:
> > "Or embed a struct io_ops * in struct device, to be set up by the bus driver?
> > 
> > Wasn't David Hinds working on something like this in the context of PCMCIA
> > a few decades ago?"
> > 
> > Based on their suggestions one way can be to pass it through void *platform_data
> > which is probably not the best and then which make more sense to me is to extend
> > struct dev_archdata archdata to add there native read/write functions.
> > 
> > What do you think?
> 
> I worry a little about code size if we have a lot of drivers that go
> from one instruction to an indirect function call for each readl/writel.
> Using platform_data ss also something that does not work too well with
> device tree based platform configuration, which tries hard to leave
> all run-time configuration inside of the driver.

readl/writel and ioread32/iowrite32 should always be equivalent.

So it all boils down to whether your device has different endianness (it
should not, doing so is stupid ... but if it really does then make it a
runtime wrapper that chooses between ioread32 and ioread32be

Cheers,
Ben.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/