| Message ID | 20230620104722.16465-1-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [01/11] mmc: core: Use BIT() macro | expand |
On Tue, 20 Jun 2023 at 12:47, Marek Vasut <marex@denx.de> wrote: > > Use the BIT(n) macro instead of (1<<n), no functional change. > Regex 's@(1 \?<< \?\([0-9A-Z_]\+\))@BIT(\1)' . > > Signed-off-by: Marek Vasut <marex@denx.de> I don't think the benefit of this change is worth it. For example, it's quite useful to run a git blame to see the history of what has happened. So, sorry, but I am not going to pick this up - or any other similar changes, at least for the core layer. Kind regards Uffe > --- > Cc: Adrian Hunter <adrian.hunter@intel.com> > Cc: Avri Altman <avri.altman@wdc.com> > Cc: Bo Liu <liubo03@inspur.com> > Cc: Deren Wu <deren.wu@mediatek.com> > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Cc: Pierre Ossman <pierre@ossman.eu> > Cc: Russell King <linux@armlinux.org.uk> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: Yang Yingliang <yangyingliang@huawei.com> > Cc: linux-mmc@vger.kernel.org > --- > include/linux/mmc/core.h | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h > index 6efec0b9820c1..23db84630ae8a 100644 > --- a/include/linux/mmc/core.h > +++ b/include/linux/mmc/core.h > @@ -26,16 +26,16 @@ enum mmc_blk_status { > struct mmc_command { > u32 opcode; > u32 arg; > -#define MMC_CMD23_ARG_REL_WR (1 << 31) > +#define MMC_CMD23_ARG_REL_WR BIT(31) > #define MMC_CMD23_ARG_PACKED ((0 << 31) | (1 << 30)) > -#define MMC_CMD23_ARG_TAG_REQ (1 << 29) > +#define MMC_CMD23_ARG_TAG_REQ BIT(29) > u32 resp[4]; > unsigned int flags; /* expected response type */ > -#define MMC_RSP_PRESENT (1 << 0) > -#define MMC_RSP_136 (1 << 1) /* 136 bit response */ > -#define MMC_RSP_CRC (1 << 2) /* expect valid crc */ > -#define MMC_RSP_BUSY (1 << 3) /* card may send busy */ > -#define MMC_RSP_OPCODE (1 << 4) /* response contains opcode */ > +#define MMC_RSP_PRESENT BIT(0) > +#define MMC_RSP_136 BIT(1) /* 136 bit response */ > +#define MMC_RSP_CRC BIT(2) /* expect valid crc */ > +#define MMC_RSP_BUSY BIT(3) /* card may send busy */ > +#define MMC_RSP_OPCODE BIT(4) /* response contains opcode */ > > #define MMC_CMD_MASK (3 << 5) /* non-SPI command type */ > #define MMC_CMD_AC (0 << 5) > @@ -43,10 +43,10 @@ struct mmc_command { > #define MMC_CMD_BC (2 << 5) > #define MMC_CMD_BCR (3 << 5) > > -#define MMC_RSP_SPI_S1 (1 << 7) /* one status byte */ > -#define MMC_RSP_SPI_S2 (1 << 8) /* second byte */ > -#define MMC_RSP_SPI_B4 (1 << 9) /* four data bytes */ > -#define MMC_RSP_SPI_BUSY (1 << 10) /* card may send busy */ > +#define MMC_RSP_SPI_S1 BIT(7) /* one status byte */ > +#define MMC_RSP_SPI_S2 BIT(8) /* second byte */ > +#define MMC_RSP_SPI_B4 BIT(9) /* four data bytes */ > +#define MMC_RSP_SPI_BUSY BIT(10) /* card may send busy */ > > /* > * These are the native response types, and correspond to valid bit > -- > 2.39.2 >
On 6/20/23 13:15, Ulf Hansson wrote: > On Tue, 20 Jun 2023 at 12:47, Marek Vasut <marex@denx.de> wrote: >> >> Use the BIT(n) macro instead of (1<<n), no functional change. >> Regex 's@(1 \?<< \?\([0-9A-Z_]\+\))@BIT(\1)' . >> >> Signed-off-by: Marek Vasut <marex@denx.de> > > I don't think the benefit of this change is worth it. For example, > it's quite useful to run a git blame to see the history of what has > happened. Understood. git blame does allow you to specify either --since or revision range though. > So, sorry, but I am not going to pick this up - or any other similar > changes, at least for the core layer. Is this a policy of the mmc subsystem to reject all code clean ups then ?
On Wed, 21 Jun 2023 at 04:36, Marek Vasut <marex@denx.de> wrote: > > On 6/20/23 13:15, Ulf Hansson wrote: > > On Tue, 20 Jun 2023 at 12:47, Marek Vasut <marex@denx.de> wrote: > >> > >> Use the BIT(n) macro instead of (1<<n), no functional change. > >> Regex 's@(1 \?<< \?\([0-9A-Z_]\+\))@BIT(\1)' . > >> > >> Signed-off-by: Marek Vasut <marex@denx.de> > > > > I don't think the benefit of this change is worth it. For example, > > it's quite useful to run a git blame to see the history of what has > > happened. > > Understood. > > git blame does allow you to specify either --since or revision range though. Yes, but I think you get my point. > > > So, sorry, but I am not going to pick this up - or any other similar > > changes, at least for the core layer. > > Is this a policy of the mmc subsystem to reject all code clean ups then ? Of course it isn't, I regularly pick up clean ups. My point here is that the clean-up should make the code better, in some way. I don't think converting to the BIT macro helps in this regard. It may be preferred to use the BIT macro by some and by others not. Kind regards Uffe
On 6/21/23 11:18, Ulf Hansson wrote: > On Wed, 21 Jun 2023 at 04:36, Marek Vasut <marex@denx.de> wrote: >> >> On 6/20/23 13:15, Ulf Hansson wrote: >>> On Tue, 20 Jun 2023 at 12:47, Marek Vasut <marex@denx.de> wrote: >>>> >>>> Use the BIT(n) macro instead of (1<<n), no functional change. >>>> Regex 's@(1 \?<< \?\([0-9A-Z_]\+\))@BIT(\1)' . >>>> >>>> Signed-off-by: Marek Vasut <marex@denx.de> >>> >>> I don't think the benefit of this change is worth it. For example, >>> it's quite useful to run a git blame to see the history of what has >>> happened. >> >> Understood. >> >> git blame does allow you to specify either --since or revision range though. > > Yes, but I think you get my point. > >> >>> So, sorry, but I am not going to pick this up - or any other similar >>> changes, at least for the core layer. >> >> Is this a policy of the mmc subsystem to reject all code clean ups then ? > > Of course it isn't, I regularly pick up clean ups. > > My point here is that the clean-up should make the code better, in > some way. I don't think converting to the BIT macro helps in this > regard. It may be preferred to use the BIT macro by some and by others > not. ACK
> -----Original Message----- > From: Ulf Hansson <ulf.hansson@linaro.org> > Sent: Mittwoch, 21. Juni 2023 11:19 > To: Marek Vasut <marex@denx.de> > Cc: linux-mmc@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; > Avri Altman <avri.altman@wdc.com>; Bo Liu <liubo03@inspur.com>; Deren > Wu <deren.wu@mediatek.com>; Philipp Zabel <p.zabel@pengutronix.de>; > Pierre Ossman <pierre@ossman.eu>; Russell King <linux@armlinux.org.uk>; > Yang Yingliang <yangyingliang@huawei.com> > Subject: Re: [PATCH 01/11] mmc: core: Use BIT() macro > > CAUTION: this mail comes from external!/ACHTUNG: Diese Mail kommt von > extern! > > On Wed, 21 Jun 2023 at 04:36, Marek Vasut <marex@denx.de> wrote: > > > > On 6/20/23 13:15, Ulf Hansson wrote: > > > On Tue, 20 Jun 2023 at 12:47, Marek Vasut <marex@denx.de> wrote: > > >> > > >> Use the BIT(n) macro instead of (1<<n), no functional change. > > >> Regex 's@(1 \?<< \?\([0-9A-Z_]\+\))@BIT(\1)' . > > >> > > >> Signed-off-by: Marek Vasut <marex@denx.de> > > > > > > I don't think the benefit of this change is worth it. For example, > > > it's quite useful to run a git blame to see the history of what has > > > happened. > > > > Understood. > > > > git blame does allow you to specify either --since or revision range though. > > Yes, but I think you get my point. > > > > > > So, sorry, but I am not going to pick this up - or any other similar > > > changes, at least for the core layer. > > > > Is this a policy of the mmc subsystem to reject all code clean ups then ? > > Of course it isn't, I regularly pick up clean ups. > > My point here is that the clean-up should make the code better, in some > way. I don't think converting to the BIT macro helps in this regard. It may be > preferred to use the BIT macro by some and by others not. FWIW I agree with Uffe here. For host/ files, which are mostly written by a handful each, it's still a nuisance. (One could argue that they are often git blamed by people not familiar with mmc subsystem, thus giving off the wrong picture). For much of the core code you already have to go many revisions back, I'm grateful for each I don't have to. Something like the mq rework would have been a good moment to do these minor nitpicks, if something like that ever happens again, but even then I would prefer just one commit including everything. Regards, Christian
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index 6efec0b9820c1..23db84630ae8a 100644 --- a/include/linux/mmc/core.h +++ b/include/linux/mmc/core.h @@ -26,16 +26,16 @@ enum mmc_blk_status { struct mmc_command { u32 opcode; u32 arg; -#define MMC_CMD23_ARG_REL_WR (1 << 31) +#define MMC_CMD23_ARG_REL_WR BIT(31) #define MMC_CMD23_ARG_PACKED ((0 << 31) | (1 << 30)) -#define MMC_CMD23_ARG_TAG_REQ (1 << 29) +#define MMC_CMD23_ARG_TAG_REQ BIT(29) u32 resp[4]; unsigned int flags; /* expected response type */ -#define MMC_RSP_PRESENT (1 << 0) -#define MMC_RSP_136 (1 << 1) /* 136 bit response */ -#define MMC_RSP_CRC (1 << 2) /* expect valid crc */ -#define MMC_RSP_BUSY (1 << 3) /* card may send busy */ -#define MMC_RSP_OPCODE (1 << 4) /* response contains opcode */ +#define MMC_RSP_PRESENT BIT(0) +#define MMC_RSP_136 BIT(1) /* 136 bit response */ +#define MMC_RSP_CRC BIT(2) /* expect valid crc */ +#define MMC_RSP_BUSY BIT(3) /* card may send busy */ +#define MMC_RSP_OPCODE BIT(4) /* response contains opcode */ #define MMC_CMD_MASK (3 << 5) /* non-SPI command type */ #define MMC_CMD_AC (0 << 5) @@ -43,10 +43,10 @@ struct mmc_command { #define MMC_CMD_BC (2 << 5) #define MMC_CMD_BCR (3 << 5) -#define MMC_RSP_SPI_S1 (1 << 7) /* one status byte */ -#define MMC_RSP_SPI_S2 (1 << 8) /* second byte */ -#define MMC_RSP_SPI_B4 (1 << 9) /* four data bytes */ -#define MMC_RSP_SPI_BUSY (1 << 10) /* card may send busy */ +#define MMC_RSP_SPI_S1 BIT(7) /* one status byte */ +#define MMC_RSP_SPI_S2 BIT(8) /* second byte */ +#define MMC_RSP_SPI_B4 BIT(9) /* four data bytes */ +#define MMC_RSP_SPI_BUSY BIT(10) /* card may send busy */ /* * These are the native response types, and correspond to valid bit
Use the BIT(n) macro instead of (1<<n), no functional change. Regex 's@(1 \?<< \?\([0-9A-Z_]\+\))@BIT(\1)' . Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Avri Altman <avri.altman@wdc.com> Cc: Bo Liu <liubo03@inspur.com> Cc: Deren Wu <deren.wu@mediatek.com> Cc: Philipp Zabel <p.zabel@pengutronix.de> Cc: Pierre Ossman <pierre@ossman.eu> Cc: Russell King <linux@armlinux.org.uk> Cc: Ulf Hansson <ulf.hansson@linaro.org> Cc: Yang Yingliang <yangyingliang@huawei.com> Cc: linux-mmc@vger.kernel.org --- include/linux/mmc/core.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)