| Message ID | 1549613335-30319-8-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | New |
| Headers | show |
| Series | mtd: rawnand: denali: exec_op(), controller/chip separation, and cleanups | expand |
On Fri, 2019-02-08 at 17:08 +0900, Masahiro Yamada wrote: > Use 'bool' type for some function arguments. > > - write (write or read?) > - raw (the raw access mode or not?) > > It is true that denali_nand_info::dma_avail is also boolean, but > I am keeping it as 'int' because 'scripts/checkpatch --strict' would > report the following: > > CHECK: Avoid using bool structure members because of possible alignment issues > - see: https://lkml.org/lkml/2017/11/21/384 > > I do not think it is a matter here, but I am sticking to the suggestion. just fyi: that suggestion has been removed by: commit 7967656ffbfa493f5546c0f18bf8a28f702c4baa Author: Jason Gunthorpe <jgg@ziepe.ca> Date: Fri Jan 18 15:50:47 2019 -0700 coding-style: Clarify the expectations around bool There has been some confusion since checkpatch started warning about bool use in structures, and people have been avoiding using it. Many people feel there is still a legitimate place for bool in structures, so provide some guidance on bool usage derived from the entire thread that spawned the checkpatch warning. Link: https://lkml.kernel.org/r/CA+55aFwVZk1OfB9T2v014PTAKFhtVan_Zj2dOjnCy3x
On Fri, Feb 8, 2019 at 6:24 PM Joe Perches <joe@perches.com> wrote: > > On Fri, 2019-02-08 at 17:08 +0900, Masahiro Yamada wrote: > > Use 'bool' type for some function arguments. > > > > - write (write or read?) > > - raw (the raw access mode or not?) > > > > It is true that denali_nand_info::dma_avail is also boolean, but > > I am keeping it as 'int' because 'scripts/checkpatch --strict' would > > report the following: > > > > CHECK: Avoid using bool structure members because of possible alignment issues > > - see: https://lkml.org/lkml/2017/11/21/384 > > > > I do not think it is a matter here, but I am sticking to the suggestion. > > just fyi: that suggestion has been removed by: > > commit 7967656ffbfa493f5546c0f18bf8a28f702c4baa > Author: Jason Gunthorpe <jgg@ziepe.ca> > Date: Fri Jan 18 15:50:47 2019 -0700 > > coding-style: Clarify the expectations around bool > > There has been some confusion since checkpatch started warning about bool > use in structures, and people have been avoiding using it. > > Many people feel there is still a legitimate place for bool in structures, > so provide some guidance on bool usage derived from the entire thread that > spawned the checkpatch warning. > > Link: https://lkml.kernel.org/r/CA+55aFwVZk1OfB9T2v014PTAKFhtVan_Zj2dOjnCy3x Thanks, this is good to know. I will use 'bool' for denali_nand_info::dma_avail in v2. I will wait for other comments until then. -- Best Regards Masahiro Yamada
Hi Joe, Joe Perches <joe@perches.com> wrote on Fri, 08 Feb 2019 01:23:37 -0800: > On Fri, 2019-02-08 at 17:08 +0900, Masahiro Yamada wrote: > > Use 'bool' type for some function arguments. > > > > - write (write or read?) > > - raw (the raw access mode or not?) > > > > It is true that denali_nand_info::dma_avail is also boolean, but > > I am keeping it as 'int' because 'scripts/checkpatch --strict' would > > report the following: > > > > CHECK: Avoid using bool structure members because of possible alignment issues > > - see: https://lkml.org/lkml/2017/11/21/384 > > > > I do not think it is a matter here, but I am sticking to the suggestion. > > just fyi: that suggestion has been removed by: > > commit 7967656ffbfa493f5546c0f18bf8a28f702c4baa > Author: Jason Gunthorpe <jgg@ziepe.ca> > Date: Fri Jan 18 15:50:47 2019 -0700 > > coding-style: Clarify the expectations around bool > > There has been some confusion since checkpatch started warning about bool > use in structures, and people have been avoiding using it. > > Many people feel there is still a legitimate place for bool in structures, > so provide some guidance on bool usage derived from the entire thread that > spawned the checkpatch warning. > > Link: https://lkml.kernel.org/r/CA+55aFwVZk1OfB9T2v014PTAKFhtVan_Zj2dOjnCy3x > > > Interesting, thanks for the link! I will consider this for my ongoing developments. Regards, Miquèl
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c index 514d189..748711e 100644 --- a/drivers/mtd/nand/raw/denali.c +++ b/drivers/mtd/nand/raw/denali.c @@ -357,7 +357,7 @@ static int denali_sw_ecc_fixup(struct nand_chip *chip, } static void denali_setup_dma64(struct denali_nand_info *denali, - dma_addr_t dma_addr, int page, int write) + dma_addr_t dma_addr, int page, bool write) { uint32_t mode; const int page_count = 1; @@ -371,7 +371,8 @@ static void denali_setup_dma64(struct denali_nand_info *denali, * burst len = 64 bytes, the number of pages */ denali->host_write(denali, mode, - 0x01002000 | (64 << 16) | (write << 8) | page_count); + 0x01002000 | (64 << 16) | + (write ? BIT(8) : 0) | page_count); /* 2. set memory low address */ denali->host_write(denali, mode, lower_32_bits(dma_addr)); @@ -381,7 +382,7 @@ static void denali_setup_dma64(struct denali_nand_info *denali, } static void denali_setup_dma32(struct denali_nand_info *denali, - dma_addr_t dma_addr, int page, int write) + dma_addr_t dma_addr, int page, bool write) { uint32_t mode; const int page_count = 1; @@ -392,7 +393,7 @@ static void denali_setup_dma32(struct denali_nand_info *denali, /* 1. setup transfer type and # of pages */ denali->host_write(denali, mode | page, - 0x2000 | (write << 8) | page_count); + 0x2000 | (write ? BIT(8) : 0) | page_count); /* 2. set memory high address bits 23:8 */ denali->host_write(denali, mode | ((dma_addr >> 16) << 8), 0x2200); @@ -453,7 +454,7 @@ static int denali_pio_write(struct denali_nand_info *denali, const u32 *buf, } static int denali_pio_xfer(struct denali_nand_info *denali, void *buf, - size_t size, int page, int write) + size_t size, int page, bool write) { if (write) return denali_pio_write(denali, buf, size, page); @@ -462,7 +463,7 @@ static int denali_pio_xfer(struct denali_nand_info *denali, void *buf, } static int denali_dma_xfer(struct denali_nand_info *denali, void *buf, - size_t size, int page, int write) + size_t size, int page, bool write) { dma_addr_t dma_addr; u32 irq_mask, irq_stat, ecc_err_mask; @@ -519,7 +520,7 @@ static int denali_dma_xfer(struct denali_nand_info *denali, void *buf, } static int denali_data_xfer(struct nand_chip *chip, void *buf, size_t size, - int page, int raw, int write) + int page, bool raw, bool write) { struct denali_nand_info *denali = to_denali(chip); @@ -644,7 +645,7 @@ static int denali_read_page_raw(struct nand_chip *chip, uint8_t *buf, if (!buf) return -EINVAL; - ret = denali_data_xfer(chip, tmp_buf, size, page, 1, 0); + ret = denali_data_xfer(chip, tmp_buf, size, page, true, false); if (ret) return ret; @@ -700,7 +701,7 @@ static int denali_write_page_raw(struct nand_chip *chip, const uint8_t *buf, return ret; } - return denali_data_xfer(chip, tmp_buf, size, page, 1, 1); + return denali_data_xfer(chip, tmp_buf, size, page, true, true); } static int denali_change_read_column_op(void *buf, unsigned int offset, @@ -752,7 +753,7 @@ static int denali_read_page(struct nand_chip *chip, uint8_t *buf, int stat = 0; int ret; - ret = denali_data_xfer(chip, buf, mtd->writesize, page, 0, 0); + ret = denali_data_xfer(chip, buf, mtd->writesize, page, false, false); if (ret && ret != -EBADMSG) return ret; @@ -782,7 +783,7 @@ static int denali_write_page(struct nand_chip *chip, const uint8_t *buf, struct mtd_info *mtd = nand_to_mtd(chip); return denali_data_xfer(chip, (void *)buf, mtd->writesize, page, - 0, 1); + false, true); } static int denali_setup_data_interface(struct nand_chip *chip, int chipnr, diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h index 46296f2..8e91438 100644 --- a/drivers/mtd/nand/raw/denali.h +++ b/drivers/mtd/nand/raw/denali.h @@ -314,7 +314,7 @@ struct denali_nand_info { u32 (*host_read)(struct denali_nand_info *denali, u32 addr); void (*host_write)(struct denali_nand_info *denali, u32 addr, u32 data); void (*setup_dma)(struct denali_nand_info *denali, dma_addr_t dma_addr, - int page, int write); + int page, bool write); }; #define DENALI_CAP_HW_ECC_FIXUP BIT(0)
Use 'bool' type for some function arguments. - write (write or read?) - raw (the raw access mode or not?) It is true that denali_nand_info::dma_avail is also boolean, but I am keeping it as 'int' because 'scripts/checkpatch --strict' would report the following: CHECK: Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384 I do not think it is a matter here, but I am sticking to the suggestion. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- drivers/mtd/nand/raw/denali.c | 23 ++++++++++++----------- drivers/mtd/nand/raw/denali.h | 2 +- 2 files changed, 13 insertions(+), 12 deletions(-) -- 2.7.4