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. ;)
--
Cheers,
Luca.
