Re: [PATCH 1/2] Initial support for Allwinner's Security ID fuses

[Oliver Schinagl]

On 05/18/13 19:19, Oliver Schinagl wrote:
<snip>
> > > +
> > > +
> > > +/* We read the entire key, using a look up table. Returned is only the
> > > + * requested byte. This is of course slower then it could be and uses 4 times
> > > + * more reads as needed but keeps code a little simpler.
> > > + */
> > > +u8 sunxi_sid_read_byte(const int key)
> > > +{
> > > +	u32 sid_key;
> > > +	u8 ret;
> > > +
> > > +	ret = 0;
> > > +	if (likely((key <= SUNXI_SID_SIZE))) {
> > > +		sid_key = ioread32(p->sid_base + keys_lut[key >> 2]);
> > > +		switch (key % 4) {
> > > +		case 0:
> > > +			ret = (sid_key >> 24) & 0xff;
> > > +			break;
> > > +		case 1:
> > > +			ret = (sid_key >> 16) & 0xff;
> > > +			break;
> > > +		case 2:
> > > +			ret = (sid_key >> 8) & 0xff;
> > > +			break;
> > > +		case 3:
> > > +			ret = sid_key & 0xff;
> > > +			break;
> > > +		}
> > > +	}
> >
> > Come on, you can do better. This lookup table is useless.
> I didn't want to depend on the fixed layout of memory, but consider it
> removed.
But i'm not smart enough :p

We can either use the look up table (which does have benefits as its 
potentially more future proof), or do some ((key >> 2) << 2) to 'drop' 
the LSB's that we want to ignore (unless there's some smarter way).

Personally, I think the LUT is a little cleaner and more readable, but I 
guess if you look at poor efficiency, the lut costs some memory, the 
left/right shift cost an additional >> 2 ... what you prefer.
> > Also, why the first key is the one with the MSBs?
> > I'd expect that the key 0 is the one holding the LSBs.
> Strangely enough, they have swapped the MSB and LSB bytes. I double
> checked it with u-boot and yep, swapped. Though in the end, if we write
> stuff there and we read stuff from there, order doesn't matter? So what
> do we prefer. Have it so that it makes sense and ignore how u-boot reads
> it, or correct it and be consistent?
You had me confused and I was looking at this for a little while. 
Bit-ordering does not change, Byte endianness is a different story of 
course. As it is now, I decided to use Big endianess. So now a 32bit key 
looks like:
0x162367c7 and if we read one byte at a time, we get 0x16, 0x23, 0x67 
and 0xc7. I made a comment that data is read as Big endian. If it is 
important, for eeprom data, to be stored little endian, I'll obviously 
change it per request.

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