Message ID | 20190318073354.12151-5-wens@kernel.org (mailing list archive) |
---|---|
State | Mainlined, archived |
Headers | show |
Series | nvmem: sunxi_sid: native format and A83T/H5 support | expand |
Hi, On Mon, Mar 18, 2019 at 03:33:52PM +0800, Chen-Yu Tsai wrote: > From: Chen-Yu Tsai <wens@csie.org> > > Originally the SID e-fuses were thought to be in big-endian format. > Later sources show that they are in fact native or little-endian. > The most compelling evidence is the thermal sensor calibration data, > which is a set of one to three 16-bit values. In native-endian they > are in 16-bit cells with increasing offsets, whereas with big-endian > they are in the wrong order, and a gap with no data will show if there > are one or three cells. > > Switch to a native endian representation for the nvmem device. For the > H3, the register read-out method was already returning data in native > endian. This only affects the other SoCs. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> I thought only the newer SoCs were impacted by this issue? -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Mon, Mar 18, 2019 at 4:42 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi, > > On Mon, Mar 18, 2019 at 03:33:52PM +0800, Chen-Yu Tsai wrote: > > From: Chen-Yu Tsai <wens@csie.org> > > > > Originally the SID e-fuses were thought to be in big-endian format. > > Later sources show that they are in fact native or little-endian. > > The most compelling evidence is the thermal sensor calibration data, > > which is a set of one to three 16-bit values. In native-endian they > > are in 16-bit cells with increasing offsets, whereas with big-endian > > they are in the wrong order, and a gap with no data will show if there > > are one or three cells. > > > > Switch to a native endian representation for the nvmem device. For the > > H3, the register read-out method was already returning data in native > > endian. This only affects the other SoCs. > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > I thought only the newer SoCs were impacted by this issue? It is noticable on the newer SoCs. The old ones only have the 128-bit SID, which could be read either way, as AFAIK it's just a serial number. If you think we should leave the old ones alone I can factor that in. ChenYu
On Mon, Mar 18, 2019 at 04:45:19PM +0800, Chen-Yu Tsai wrote: > On Mon, Mar 18, 2019 at 4:42 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > Hi, > > > > On Mon, Mar 18, 2019 at 03:33:52PM +0800, Chen-Yu Tsai wrote: > > > From: Chen-Yu Tsai <wens@csie.org> > > > > > > Originally the SID e-fuses were thought to be in big-endian format. > > > Later sources show that they are in fact native or little-endian. > > > The most compelling evidence is the thermal sensor calibration data, > > > which is a set of one to three 16-bit values. In native-endian they > > > are in 16-bit cells with increasing offsets, whereas with big-endian > > > they are in the wrong order, and a gap with no data will show if there > > > are one or three cells. > > > > > > Switch to a native endian representation for the nvmem device. For the > > > H3, the register read-out method was already returning data in native > > > endian. This only affects the other SoCs. > > > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > > > I thought only the newer SoCs were impacted by this issue? > > It is noticable on the newer SoCs. The old ones only have the 128-bit SID, > which could be read either way, as AFAIK it's just a serial number. > > If you think we should leave the old ones alone I can factor that in. IIRC, there was also the SoC ID in the SID on those SoCs as well, which we might have to use in the future so we'll want to make sure it is correct. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Mon, Mar 18, 2019 at 4:57 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Mon, Mar 18, 2019 at 04:45:19PM +0800, Chen-Yu Tsai wrote: > > On Mon, Mar 18, 2019 at 4:42 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > Hi, > > > > > > On Mon, Mar 18, 2019 at 03:33:52PM +0800, Chen-Yu Tsai wrote: > > > > From: Chen-Yu Tsai <wens@csie.org> > > > > > > > > Originally the SID e-fuses were thought to be in big-endian format. > > > > Later sources show that they are in fact native or little-endian. > > > > The most compelling evidence is the thermal sensor calibration data, > > > > which is a set of one to three 16-bit values. In native-endian they > > > > are in 16-bit cells with increasing offsets, whereas with big-endian > > > > they are in the wrong order, and a gap with no data will show if there > > > > are one or three cells. > > > > > > > > Switch to a native endian representation for the nvmem device. For the > > > > H3, the register read-out method was already returning data in native > > > > endian. This only affects the other SoCs. > > > > > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > > > > > I thought only the newer SoCs were impacted by this issue? > > > > It is noticable on the newer SoCs. The old ones only have the 128-bit SID, > > which could be read either way, as AFAIK it's just a serial number. > > > > If you think we should leave the old ones alone I can factor that in. > > IIRC, there was also the SoC ID in the SID on those SoCs as well, > which we might have to use in the future so we'll want to make sure it > is correct. We'll need to ask Allwinner about this then. FWIW, the fel command in sunxi-tools reads them out in little endian. I believe this and the SID page on the linux-sunxi wiki predate the sunxi_sid driver. ChenYu
On Mon, Mar 18, 2019 at 05:09:44PM +0800, Chen-Yu Tsai wrote: > On Mon, Mar 18, 2019 at 4:57 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > On Mon, Mar 18, 2019 at 04:45:19PM +0800, Chen-Yu Tsai wrote: > > > On Mon, Mar 18, 2019 at 4:42 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > Hi, > > > > > > > > On Mon, Mar 18, 2019 at 03:33:52PM +0800, Chen-Yu Tsai wrote: > > > > > From: Chen-Yu Tsai <wens@csie.org> > > > > > > > > > > Originally the SID e-fuses were thought to be in big-endian format. > > > > > Later sources show that they are in fact native or little-endian. > > > > > The most compelling evidence is the thermal sensor calibration data, > > > > > which is a set of one to three 16-bit values. In native-endian they > > > > > are in 16-bit cells with increasing offsets, whereas with big-endian > > > > > they are in the wrong order, and a gap with no data will show if there > > > > > are one or three cells. > > > > > > > > > > Switch to a native endian representation for the nvmem device. For the > > > > > H3, the register read-out method was already returning data in native > > > > > endian. This only affects the other SoCs. > > > > > > > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > > > > > > > I thought only the newer SoCs were impacted by this issue? > > > > > > It is noticable on the newer SoCs. The old ones only have the 128-bit SID, > > > which could be read either way, as AFAIK it's just a serial number. > > > > > > If you think we should leave the old ones alone I can factor that in. > > > > IIRC, there was also the SoC ID in the SID on those SoCs as well, > > which we might have to use in the future so we'll want to make sure it > > is correct. > > We'll need to ask Allwinner about this then. > > FWIW, the fel command in sunxi-tools reads them out in little endian. I > believe this and the SID page on the linux-sunxi wiki predate the sunxi_sid > driver. Yeah, and the driver has a readl as well: https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/arch/arm/plat-sunxi/soc-detect.c#L92 For the whole series, Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c index 75c1f48cb3d0..14c114620ed6 100644 --- a/drivers/nvmem/sunxi_sid.c +++ b/drivers/nvmem/sunxi_sid.c @@ -46,33 +46,12 @@ struct sunxi_sid { u32 value_offset; }; -/* We read the entire key, due to a 32 bit read alignment requirement. Since we - * want to return the requested byte, this results in somewhat slower code and - * uses 4 times more reads as needed but keeps code simpler. Since the SID is - * only very rarely probed, this is not really an issue. - */ -static u8 sunxi_sid_read_byte(const struct sunxi_sid *sid, - const unsigned int offset) -{ - u32 sid_key; - - sid_key = ioread32be(sid->base + round_down(offset, 4)); - sid_key >>= (offset % 4) * 8; - - return sid_key; /* Only return the last byte */ -} - static int sunxi_sid_read(void *context, unsigned int offset, void *val, size_t bytes) { struct sunxi_sid *sid = context; - u8 *buf = val; - - /* Offset the read operation to the real position of SID */ - offset += sid->value_offset; - while (bytes--) - *buf++ = sunxi_sid_read_byte(sid, offset++); + memcpy_fromio(val, sid->base + sid->value_offset + offset, bytes); return 0; }