Date:   Mon, 10 Jan 2022 09:28:36 -0800
From:   Jakub Kicinski <kuba@kernel.org>
To:     Corentin Labbe <clabbe.montjoie@gmail.com>
Cc:     Conley Lee <conleylee@foxmail.com>, davem@davemloft.net,
        mripard@kernel.org, wens@csie.org, netdev@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] net: ethernet: sun4i-emac: replace magic number with
 macro
Message-ID: <20220110092836.113dbae1@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
In-Reply-To: <Ydw7EzPvwArW/siQ@Red>
References: <tencent_58B12979F0BFDB1520949A6DB536ED15940A@qq.com>
        <tencent_AEEE0573A5455BBE4D5C05226C6C1E3AEF08@qq.com>
        <Ydw7EzPvwArW/siQ@Red>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 10 Jan 2022 14:56:35 +0100 Corentin Labbe wrote:
> > @@ -61,7 +62,21 @@
> >  #define EMAC_RX_IO_DATA_STATUS_OK	(1 << 7)
> >  #define EMAC_RX_FBC_REG		(0x50)
> >  #define EMAC_INT_CTL_REG	(0x54)
> > +#define EMAC_INT_CTL_RX_EN	(1 << 8)
> > +#define EMAC_INT_CTL_TX0_EN	(1)
> > +#define EMAC_INT_CTL_TX1_EN	(1 << 1)
> > +#define EMAC_INT_CTL_TX_EN	(EMAC_INT_CTL_TX0_EN | EMAC_INT_CTL_TX1_EN)
> > +#define EMAC_INT_CTL_TX0_ABRT_EN	(0x1 << 2)
> > +#define EMAC_INT_CTL_TX1_ABRT_EN	(0x1 << 3)
> > +#define EMAC_INT_CTL_TX_ABRT_EN	(EMAC_INT_CTL_TX0_ABRT_EN | EMAC_INT_CTL_TX1_ABRT_EN)
> >  #define EMAC_INT_STA_REG	(0x58)
> > +#define EMAC_INT_STA_TX0_COMPLETE	(0x1)
> > +#define EMAC_INT_STA_TX1_COMPLETE	(0x1 << 1)
> > +#define EMAC_INT_STA_TX_COMPLETE	(EMAC_INT_STA_TX0_COMPLETE | EMAC_INT_STA_TX1_COMPLETE)
> > +#define EMAC_INT_STA_TX0_ABRT	(0x1 << 2)
> > +#define EMAC_INT_STA_TX1_ABRT	(0x1 << 3)
> > +#define EMAC_INT_STA_TX_ABRT	(EMAC_INT_STA_TX0_ABRT | EMAC_INT_STA_TX1_ABRT)
> > +#define EMAC_INT_STA_RX_COMPLETE	(0x1 << 8)  
> 
> As proposed by checkpatch, I thing there are several place (like all
> EMAC_INT_STA) where you could use BIT(x) instead of (0xX << x)

That's not a hard requirement, if the driver already uses the shift you
can leave your code as is, some upstream developers actually prefer to
avoid the BIT() macro.