Luca Coelho <luca@xxxxxxxxx> writes:
> On Thu, 2021-08-26 at 14:30 +0300, Luca Coelho wrote:
>> On Sat, 2021-08-21 at 17:07 +0300, Kalle Valo wrote:
>> > Luca Coelho <luca@xxxxxxxxx> writes:
>> >
>> > > From: Matti Gottlieb <matti.gottlieb@xxxxxxxxx>
>> > >
>> > > 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 <matti.gottlieb@xxxxxxxxx>
>> > > Signed-off-by: Luca Coelho <luciano.coelho@xxxxxxxxx>
>> >
>> > [...]
>> >
>> > > +/*
>> > > + * 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. ;)
Good that endian is handled properly, thanks for checking.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
