Re: [PATCH net 2/5] bnxt_en: Fix devlink NVRAM related byte order related issues.

[Michael Chan <michael.chan@broadcom.com>]

On Mon, Oct 21, 2019 at 9:14 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Mon, 21 Oct 2019 01:34:26 -0400, Michael Chan wrote:
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> > index 68f74f5..bd4b9f3 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> > @@ -226,12 +226,55 @@ static const struct bnxt_dl_nvm_param nvm_params[] = {
> >        BNXT_NVM_SHARED_CFG, 1, 1},
> >  };
> >
> > +union bnxt_nvm_data {
> > +     u8      val8;
> > +     __le32  val32;
> > +};
> > +
> > +static void bnxt_copy_to_nvm_data(union bnxt_nvm_data *dst,
> > +                               union devlink_param_value *src,
> > +                               int nvm_num_bits, int dl_num_bytes)
> > +{
> > +     u32 val32 = 0;
> > +
> > +     if (nvm_num_bits == 1) {
> > +             dst->val8 = src->vbool;
> > +             return;
> > +     }
>
> Why do you special case the num_bits == 1? If val32 is __le32 the low
> byte would have landed on the first byte anyway, no?
>
> just curious

Just so that I don't have to do any casting.  Otherwise if I assign it
to the __le32, I believe I have to cast to avoid the warning.

>
> > +     if (dl_num_bytes == 4)
> > +             val32 = src->vu32;
> > +     else if (dl_num_bytes == 2)
> > +             val32 = (u32)src->vu16;
> > +     else if (dl_num_bytes == 1)
> > +             val32 = (u32)src->vu8;
> > +     dst->val32 = cpu_to_le32(val32);
> > +}