Re: [PATCH 10/12] iwlwifi: Add support for getting rf id with blank otp

[Kalle Valo <kvalo@codeaurora.org>]

Luca Coelho <luca@coelho.fi> 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@coelho.fi> writes:
>> > 
>> > > From: Matti Gottlieb <matti.gottlieb@intel.com>
>> > > 
>> > > 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@intel.com>
>> > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>> > 
>> > [...]
>> > 
>> > > +/*
>> > > + * 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