Re: [PATCH RFC] net: qualcomm: rmnet: Move common struct definitions to include

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Tue 21 May 13:45 PDT 2019, Arnd Bergmann wrote:

> On Tue, May 21, 2019 at 9:35 PM Subash Abhinov Kasiviswanathan
> <subashab@codeaurora.org> wrote:
> >
> > Create if_rmnet.h and move the rmnet MAP packet structs to this
> > common include file. To account for portability, add little and
> > big endian bitfield definitions similar to the ip & tcp headers.
> >
> > The definitions in the headers can now be re-used by the
> > upcoming ipa driver series as well as qmi_wwan.
> >
> > Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
> > ---
> > This patch is an alternate implementation of the series posted by Elder.
> > This eliminates the changes needed in the rmnet packet parsing
> > while maintaining portability.
> > ---
> 
> I think I'd just duplicate the structure definitions then, to avoid having
> the bitfield definitions in a common header and using them in the new
> driver.
> 

Doing would allow each driver to represent the bits as suitable, at the
cost of some duplication and confusion. Confusion, because it doesn't
resolve the question of what the right bit order actually is.

Subash stated yesterday that bit 0 is "CD", which in the current struct
is represented as the 8th bit, while Alex's patch changes the definition
so that this bit is the lsb. I.e. I read Subash answer as confirming
that patch 1/8 from Alex is correct.


Subash, as we're not addressing individual bits in this machine, so
given a pointer map_hdr to a struct rmnet_map_header, which of the
following ways would give you the correct value of pad_len:

u8 p = *(char*)map_hdr;
pad_len = p & 0x3f;

or:

u8 p = *(char*)map_hdr;
pad_len = p >> 2;


PS. I do prefer the two drivers share the definition of these
structures...

Regards,
Bjorn