Hi Andrew Lunn, Thanks for the feedback. > Subject: Re: [PATCH/RFC 1/2] ravb: Preparation for supporting Gigabit > Ethernet driver > > On Wed, Jul 14, 2021 at 03:54:07PM +0100, Biju Das wrote: > > The DMAC and EMAC blocks of Gigabit Ethernet IP is almost similar to > > Ethernet AVB. With few canges in driver we can > > changes Ok. Will fix it. > > > support both the IP. This patch is in preparation for supporting the > > same. > > Please break this up a bit, it is hard to review. You can put all the > refactoring into helpers into one patch. But changes like > > - if (priv->chip_id == RCAR_GEN2) { > > + if (priv->chip_id != RCAR_GEN3) { > > should be in a seperate patch with an explanation. Ok. > > You are aiming for lots of very simple patches which are obviously > correct. OK will make simple patches 1) Code common to R-Car Gen2 and RZ/G2L (priv->chip_id != RCAR_GEN3) 2) Code specific to R-Car Gen3. 3) Spelling mistake 4) White space 5) Refactorization patches > > > diff --git a/drivers/net/ethernet/renesas/ravb.h > > b/drivers/net/ethernet/renesas/ravb.h > > index 86a1eb0634e8..80e62ca2e3d3 100644 > > --- a/drivers/net/ethernet/renesas/ravb.h > > +++ b/drivers/net/ethernet/renesas/ravb.h > > @@ -864,7 +864,7 @@ enum GECMR_BIT { > > > > /* The Ethernet AVB descriptor definitions. */ struct ravb_desc { > > - __le16 ds; /* Descriptor size */ > > + __le16 ds; /* Descriptor size */ > > u8 cc; /* Content control MSBs (reserved) */ > > u8 die_dt; /* Descriptor interrupt enable and type */ > > __le32 dptr; /* Descriptor pointer */ > > Please put white spaces changes in a patch of its own. OK. Thanks, Biju