| Message ID | 1448611534-21142-1-git-send-email-anuplimbu14@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
On Fri, Nov 27, 2015 at 01:35:34PM +0530, Anup Limbu wrote: > remove unnecessary if cases > > Signed-off-by: Anup Limbu <anuplimbu14@gmail.com> > --- Did you test this? > drivers/mtd/nand/davinci_nand.c | 26 ++++++++++---------------- > 1 file changed, 10 insertions(+), 16 deletions(-) > > diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c > index c72313d..1f86ee1 100644 > --- a/drivers/mtd/nand/davinci_nand.c > +++ b/drivers/mtd/nand/davinci_nand.c > @@ -568,21 +568,16 @@ static struct davinci_nand_pdata > pdev->dev.platform_data = pdata; > if (!pdata) > return ERR_PTR(-ENOMEM); > - if (!of_property_read_u32(pdev->dev.of_node, > - "ti,davinci-chipselect", &prop)) > - pdev->id = prop; > - else > + if (of_property_read_u32(pdev->dev.of_node, > + "ti,davinci-chipselect", &pdev->id)) > return ERR_PTR(-EINVAL); > > - if (!of_property_read_u32(pdev->dev.of_node, > - "ti,davinci-mask-ale", &prop)) > - pdata->mask_ale = prop; > - if (!of_property_read_u32(pdev->dev.of_node, > - "ti,davinci-mask-cle", &prop)) > - pdata->mask_cle = prop; > - if (!of_property_read_u32(pdev->dev.of_node, > - "ti,davinci-mask-chipsel", &prop)) > - pdata->mask_chipsel = prop; > + of_property_read_u32(pdev->dev.of_node, > + "ti,davinci-mask-ale", &pdata->mask_ale); > + of_property_read_u32(pdev->dev.of_node, > + "ti,davinci-mask-cle", &pdata->mask_cle); > + of_property_read_u32(pdev->dev.of_node, > + "ti,davinci-mask-chipsel", &pdata->mask_chipsel); > if (!of_property_read_string(pdev->dev.of_node, > "nand-ecc-mode", &mode) || > !of_property_read_string(pdev->dev.of_node, > @@ -594,9 +589,8 @@ static struct davinci_nand_pdata > if (!strncmp("hw", mode, 2)) > pdata->ecc_mode = NAND_ECC_HW; > } > - if (!of_property_read_u32(pdev->dev.of_node, > - "ti,davinci-ecc-bits", &prop)) > - pdata->ecc_bits = prop; > + of_property_read_u32(pdev->dev.of_node, > + "ti,davinci-ecc-bits", &pdata->ecc_bits); NAK! You're treating a u8 as u32. This should give you some subset of the following warnings, from gcc and sparse: In file included from include/linux/of_device.h:8:0, from drivers/mtd/nand/davinci_nand.c:35: include/linux/of.h:857:19: note: expected ‘u32 *’ but argument is of type ‘u8 *’ drivers/mtd/nand/davinci_nand.c: In function ‘nand_davinci_get_pdata’: drivers/mtd/nand/davinci_nand.c:593:4: warning: passing argument 3 of ‘of_property_read_u32’ from incompatible pointer type [enabled by default] drivers/mtd/nand/davinci_nand.c:572:51: warning: incorrect type in argument 3 (different signedness) [sparse] drivers/mtd/nand/davinci_nand.c:572:51: expected unsigned int [usertype] *out_value [sparse] drivers/mtd/nand/davinci_nand.c:572:51: got int *<noident> [sparse] drivers/mtd/nand/davinci_nand.c:593:49: warning: incorrect type in argument 3 (different type sizes) [sparse] drivers/mtd/nand/davinci_nand.c:593:49: expected unsigned int [usertype] *out_value [sparse] drivers/mtd/nand/davinci_nand.c:593:49: got unsigned char *<noident> [sparse] and it leaves you open to memory corruption, as of_property_read_u32() might try clobbering 3 more bytes of data after the ecc_bits field. All in all, there's a reason the code is written the way it is, and I don't see any good reason to do what your patch does. Brian > > prop = of_get_nand_bus_width(pdev->dev.of_node); > if (0 < prop || !of_property_read_u32(pdev->dev.of_node, > -- > 1.9.1 >
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c index c72313d..1f86ee1 100644 --- a/drivers/mtd/nand/davinci_nand.c +++ b/drivers/mtd/nand/davinci_nand.c @@ -568,21 +568,16 @@ static struct davinci_nand_pdata pdev->dev.platform_data = pdata; if (!pdata) return ERR_PTR(-ENOMEM); - if (!of_property_read_u32(pdev->dev.of_node, - "ti,davinci-chipselect", &prop)) - pdev->id = prop; - else + if (of_property_read_u32(pdev->dev.of_node, + "ti,davinci-chipselect", &pdev->id)) return ERR_PTR(-EINVAL); - if (!of_property_read_u32(pdev->dev.of_node, - "ti,davinci-mask-ale", &prop)) - pdata->mask_ale = prop; - if (!of_property_read_u32(pdev->dev.of_node, - "ti,davinci-mask-cle", &prop)) - pdata->mask_cle = prop; - if (!of_property_read_u32(pdev->dev.of_node, - "ti,davinci-mask-chipsel", &prop)) - pdata->mask_chipsel = prop; + of_property_read_u32(pdev->dev.of_node, + "ti,davinci-mask-ale", &pdata->mask_ale); + of_property_read_u32(pdev->dev.of_node, + "ti,davinci-mask-cle", &pdata->mask_cle); + of_property_read_u32(pdev->dev.of_node, + "ti,davinci-mask-chipsel", &pdata->mask_chipsel); if (!of_property_read_string(pdev->dev.of_node, "nand-ecc-mode", &mode) || !of_property_read_string(pdev->dev.of_node, @@ -594,9 +589,8 @@ static struct davinci_nand_pdata if (!strncmp("hw", mode, 2)) pdata->ecc_mode = NAND_ECC_HW; } - if (!of_property_read_u32(pdev->dev.of_node, - "ti,davinci-ecc-bits", &prop)) - pdata->ecc_bits = prop; + of_property_read_u32(pdev->dev.of_node, + "ti,davinci-ecc-bits", &pdata->ecc_bits); prop = of_get_nand_bus_width(pdev->dev.of_node); if (0 < prop || !of_property_read_u32(pdev->dev.of_node,
remove unnecessary if cases Signed-off-by: Anup Limbu <anuplimbu14@gmail.com> --- drivers/mtd/nand/davinci_nand.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-)