Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Matteo Croce <technoboy85@gmail.com>]

Il Thursday 20 September 2007 22:00:59 Jörn Engel ha scritto:
> On Thu, 20 September 2007 21:35:49 +0200, Christoph Hellwig wrote:
> > On Thu, Sep 20, 2007 at 09:29:11PM +0200, Matteo Croce wrote:
> > > +#ifdef CONFIG_CPU_LITTLE_ENDIAN
> > > +#define LOADER_MAGIC1	0xfeedfa42
> > > +#define LOADER_MAGIC2	0xfeed1281
> > > +#else
> > > +#define LOADER_MAGIC1	0x42faedfe
> > > +#define LOADER_MAGIC2	0x8112edfe
> > > +#endif
> > 
> > Please keep only one defintion and use le/be32_to_cpu on it.
> > 
> > > +struct ar7_bin_rec {
> > > +	unsigned int checksum;
> > > +	unsigned int length;
> > > +	unsigned int address;
> > > +};
> > 
> > Wich means you'd need an endianess annotation here.  What about the
> > length and address fields, are they always native-endian unlike
> > the checksum field or will the need to be byte-swapped aswell?
> 
> <slightly off-topic, feel free to skip>
> If this is indeed the squashfs magic, le/be32_to_cpu won't help.
> Squashfs can have either endianness, tries to detect the one actually
> used by checking either magic and sets a flag in the superblock.
> Afterwards every single access checks the flag and conditionally swaps
> fields around or not.
> 
> If squashfs had a fixed endianness, quite a lot of this logic could get
> removed and both source and object size would shrink.  Some two years
> after requesting this for the first time, I'm thinking about just doing
> it myself.  If I find a sponsor who pays me for it, I might even do it
> soon.
> </offtopic>
> 
> 
> I don't really understand why the squashfs magic number should be used
> in this code at all.  It may have set a bad example, though.  In general
> you should decide on a fixed endianness (1) and use the beXX_to_cpu
> macros when accessing data unless you have a very good reason to do
> otherwise.
> 
> 1) Big endian is my preferred choice because it is easy to read in a
> hexdump and the opposite of my notebook.  Being forced to do endian
> conversions during development/testing helps to find problems early.

I use little endian since 99% of AR7s are little endian. Dunno if
le/be32_to_cpu does some runtime calculations. Do they?