On Tue, Oct 12, 2021 at 2:37 PM Alvin Šipraga <alvin@xxxxxxx> wrote: > From: Alvin Šipraga <alsi@xxxxxxxxxxxxxxx> > > This commit implements a basic version of the 8 byte tag protocol used > in the Realtek RTL8365MB-VC unmanaged switch, which carries with it a > protocol version of 0x04. > > The implementation itself only handles the parsing of the EtherType > value and Realtek protocol version, together with the source or > destination port fields. The rest is left unimplemented for now. > > The tag format is described in a confidential document provided to my > company by Realtek Semiconductor Corp. Permission has been granted by > the vendor to publish this driver based on that material, together with > an extract from the document describing the tag format and its fields. > It is hoped that this will help future implementors who do not have > access to the material but who wish to extend the functionality of > drivers for chips which use this protocol. > > In addition, two possible values of the REASON field are specified, > based on experiments on my end. Realtek does not specify what value this > field can take. > > Signed-off-by: Alvin Šipraga <alsi@xxxxxxxxxxxxxxx> The code definately looks good: Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx> Some nitpicky personal preferences below: > +#define RTL8_4_TAG_LEN 8 > +/* 0x04 = RTL8365MB DSA protocol > + */ > +#define RTL8_4_PROTOCOL_RTL8365MB 0x04 This is #defined > + /* Zero FID_EN, FID, PRI_EN, PRI, KEEP; set LEARN_DIS */ > + tag[2] = htons(1 << 5); I would create defines for the flags like this: #define RTL8365MB_LEARN_DIS BIT(5) > + /* Parse Protocol */ > + proto = ntohs(tag[1]) >> 8; In the 4byte header code we have something like this: #define RTL8_4_PROTOCOL_SHIFT 8 > + /* Parse TX (switch->CPU) */ > + port = ntohs(tag[3]) & 0xf; /* Port number is the LSB 4 bits */ This I think is fair enough. No need to define that mask. Yours, Linus Walleij
