On Thu, 2021-08-26 at 14:30 +0300, Luca Coelho wrote:

On Sat, 2021-08-21 at 17:07 +0300, Kalle Valo wrote:

quoted

Luca Coelho [off-list ref] writes:

quoted

From: Matti Gottlieb <redacted>

When having a blank OTP the only way to get the rf id
and the cdb info is from prph registers.

Currently there is some implementation for this, but it
is located in the wrong place in the code (should be before
trying to understand what HW is connected and not after),
and it has a partial implementation.

Signed-off-by: Matti Gottlieb <redacted>
Signed-off-by: Luca Coelho <redacted>

[...]

quoted

+/*
+ * struct iwl_crf_chip_id_reg
+ *
+ * type: bits 0-11
+ * reserved: bits 12-18
+ * slave_exist: bit 19
+ * dash: bits 20-23
+ * step: bits 24-26
+ * flavor: bits 27-31
+ */
+struct iwl_crf_chip_id_reg {
+	u32 type : 12;
+	u32 reserved : 7;
+	u32 slave_exist : 1;
+	u32 dash : 4;
+	u32 step : 4;
+	u32 flavor : 4;
+};

This doesn't look endian safe.

It's not exactly that this is not endian safe, but we had two issues:

1. AFAIK these bitfields are not guaranteed to be kept in order, so we
shouldn't use them.  I'll change it to decode this in some other way.

2. We are actually reading the register without caring for endianess. 
I will fix it.

Oops, as Johannes pointed out offline, this is not an issue, actually.
I got confused with some places where we do cpu_to_le32() after
reading, which is the opposite and essentially proves that the read is
in cpu-endianess.  So I won't "fix" it. ;)

--
Cheers,
Luca.