* [PATCH] Check return code from pdc20621_i2c_read() @ 2015-08-02 10:12 Tomer Barletz 2015-08-02 11:09 ` Sergei Shtylyov 0 siblings, 1 reply; 16+ messages in thread From: Tomer Barletz @ 2015-08-02 10:12 UTC (permalink / raw) To: tj; +Cc: linux-ide, linux-kernel, Tomer Barletz The variable spd0 might be used uninitialized when pdc20621_i2c_read() fails. This also generates a compilation warning with gcc 5.1. Signed-off-by: Tomer Barletz <barletz@gmail.com> --- drivers/ata/sata_sx4.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c index 3a18a8a..bed311b 100644 --- a/drivers/ata/sata_sx4.c +++ b/drivers/ata/sata_sx4.c @@ -1238,8 +1238,11 @@ static unsigned int pdc20621_prog_dimm_global(struct ata_host *host) readl(mmio + PDC_SDRAM_CONTROL); /* Turn on for ECC */ - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, - PDC_DIMM_SPD_TYPE, &spd0); + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, + PDC_DIMM_SPD_TYPE, &spd0)) { + printk(KERN_ERR "Failed in i2c read: device=0x%d, subaddr=0x%d\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); + return 1; + } if (spd0 == 0x02) { data |= (0x01 << 16); writel(data, mmio + PDC_SDRAM_CONTROL); @@ -1380,8 +1383,11 @@ static unsigned int pdc20621_dimm_init(struct ata_host *host) /* ECC initiliazation. */ - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, - PDC_DIMM_SPD_TYPE, &spd0); + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, + PDC_DIMM_SPD_TYPE, &spd0)) { + printk(KERN_ERR "Failed in i2c read: device=0x%d, subaddr=0x%d\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); + return 1; + } if (spd0 == 0x02) { void *buf; VPRINTK("Start ECC initialization\n"); -- 2.4.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] Check return code from pdc20621_i2c_read() 2015-08-02 10:12 [PATCH] Check return code from pdc20621_i2c_read() Tomer Barletz @ 2015-08-02 11:09 ` Sergei Shtylyov [not found] ` <CAO-S75CRyh=FPbeTJMZWiYTvpd_wn57f4eGQgOHW2p16+9ANhQ@mail.gmail.com> 0 siblings, 1 reply; 16+ messages in thread From: Sergei Shtylyov @ 2015-08-02 11:09 UTC (permalink / raw) To: Tomer Barletz, tj; +Cc: linux-ide, linux-kernel Hello. On 8/2/2015 1:12 PM, Tomer Barletz wrote: > The variable spd0 might be used uninitialized when pdc20621_i2c_read() > fails. > This also generates a compilation warning with gcc 5.1. > Signed-off-by: Tomer Barletz <barletz@gmail.com> > --- > drivers/ata/sata_sx4.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c > index 3a18a8a..bed311b 100644 > --- a/drivers/ata/sata_sx4.c > +++ b/drivers/ata/sata_sx4.c > @@ -1238,8 +1238,11 @@ static unsigned int pdc20621_prog_dimm_global(struct ata_host *host) > readl(mmio + PDC_SDRAM_CONTROL); > > /* Turn on for ECC */ > - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > - PDC_DIMM_SPD_TYPE, &spd0); > + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > + PDC_DIMM_SPD_TYPE, &spd0)) { > + printk(KERN_ERR "Failed in i2c read: device=0x%d, subaddr=0x%d\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); Please use pr_err() instead. And "0x%d" makes no sense at all, please use "%#x" instead. [...] > @@ -1380,8 +1383,11 @@ static unsigned int pdc20621_dimm_init(struct ata_host *host) > > /* ECC initiliazation. */ > > - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > - PDC_DIMM_SPD_TYPE, &spd0); > + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > + PDC_DIMM_SPD_TYPE, &spd0)) { > + printk(KERN_ERR "Failed in i2c read: device=0x%d, subaddr=0x%d\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); Likewise. MNR, Sergei ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <CAO-S75CRyh=FPbeTJMZWiYTvpd_wn57f4eGQgOHW2p16+9ANhQ@mail.gmail.com>]
* Re: [PATCH] Check return code from pdc20621_i2c_read() [not found] ` <CAO-S75CRyh=FPbeTJMZWiYTvpd_wn57f4eGQgOHW2p16+9ANhQ@mail.gmail.com> @ 2015-08-02 17:55 ` Tomer Barletz 2015-08-02 18:03 ` Joe Perches ` (2 more replies) 2015-08-03 6:06 ` [PATCH] " Tomer Barletz 1 sibling, 3 replies; 16+ messages in thread From: Tomer Barletz @ 2015-08-02 17:55 UTC (permalink / raw) To: Sergei Shtylyov; +Cc: tj, linux-ide, linux-kernel Re-sending in plain-text. On Sun, Aug 2, 2015 at 4:09 AM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Please use pr_err() instead. And "0x%d" makes no sense at all, please use "%#x" instead. > Yeah, not sure what I was drinking before writing this 0x%d thing... Regarding the pr_err() - it is not used at all in this file, and printk() is used instead. Wouldn't it be better to leave it with printk for this change, then have another change that replaces printk()s with pr_err()s? --Tomer On Sun, Aug 2, 2015 at 10:53 AM, Tomer Barletz <barletz@gmail.com> wrote: > > On Sun, Aug 2, 2015 at 4:09 AM, Sergei Shtylyov > <sergei.shtylyov@cogentembedded.com> wrote: > >> Please use pr_err() instead. And "0x%d" makes no sense at all, please >> use "%#x" instead. >> > > Yeah, not sure what I was drinking before writing this 0x%d thing... > > Regarding the pr_err() - it is not used at all in this file, and printk() is > used instead. Wouldn't it be better to leave it with printk for this change, > then have another change that replaces printk()s with pr_err()s? > > --Tomer > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Check return code from pdc20621_i2c_read() 2015-08-02 17:55 ` Tomer Barletz @ 2015-08-02 18:03 ` Joe Perches 2015-08-02 20:22 ` Joe Perches 2015-08-03 15:42 ` Sergei Shtylyov 2 siblings, 0 replies; 16+ messages in thread From: Joe Perches @ 2015-08-02 18:03 UTC (permalink / raw) To: Tomer Barletz; +Cc: Sergei Shtylyov, tj, linux-ide, linux-kernel On Sun, 2015-08-02 at 10:55 -0700, Tomer Barletz wrote: > Re-sending in plain-text. > > On Sun, Aug 2, 2015 at 4:09 AM, Sergei Shtylyov > <sergei.shtylyov@cogentembedded.com> wrote: > > > Please use pr_err() instead. And "0x%d" makes no sense at all, > > please use "%#x" instead. > > > > Yeah, not sure what I was drinking before writing this 0x%d thing... > > Regarding the pr_err() - it is not used at all in this file, and > printk() is used instead. Wouldn't it be better to leave it with > printk for this change, then have another change that replaces > printk()s with pr_err()s? Where possible, it'd be better to use ata_dev_<level> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Check return code from pdc20621_i2c_read() 2015-08-02 17:55 ` Tomer Barletz 2015-08-02 18:03 ` Joe Perches @ 2015-08-02 20:22 ` Joe Perches 2015-08-03 15:42 ` Sergei Shtylyov 2 siblings, 0 replies; 16+ messages in thread From: Joe Perches @ 2015-08-02 20:22 UTC (permalink / raw) To: Tomer Barletz; +Cc: Sergei Shtylyov, tj, linux-ide, linux-kernel On Sun, 2015-08-02 at 10:55 -0700, Tomer Barletz wrote: > On Sun, Aug 2, 2015 at 4:09 AM, Sergei Shtylyov > <sergei.shtylyov@cogentembedded.com> wrote: > > > Please use pr_err() instead. And "0x%d" makes no sense at all, > > please use "%#x" instead. > > > > Yeah, not sure what I was drinking before writing this 0x%d thing... btw: you and a few others ([dui]: debugging under the influence?): $ git grep -n -E "0x[\*\d\.]*%[dui]" drivers/block/DAC960.c:2957: DAC960_Error("IO port 0x%d busy for Controller at\n", drivers/block/DAC960.c:2993: DAC960_Error("IO port 0x%d busy for Controller at\n", drivers/block/cciss.c:3857: dev_dbg(&h->pdev->dev, " Max outstanding commands = 0x%d\n", drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c:523: mfc_debug(2, "stream buf addr: 0x%08lx, size: 0x%d\n", drivers/misc/sgi-gru/grumain.c:286: gru_dbg(grudev, "gid %d, gts %p, gms %p, ctxnum 0x%d, asidmap 0x%lx\n", sound/soc/atmel/atmel_ssc_dai.c:293: pr_debug("atmel_ssc_startup: SSC_SR=0x%u\n", I'll send a few patches... ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Check return code from pdc20621_i2c_read() 2015-08-02 17:55 ` Tomer Barletz 2015-08-02 18:03 ` Joe Perches 2015-08-02 20:22 ` Joe Perches @ 2015-08-03 15:42 ` Sergei Shtylyov 2015-08-03 15:52 ` Sergei Shtylyov 2 siblings, 1 reply; 16+ messages in thread From: Sergei Shtylyov @ 2015-08-03 15:42 UTC (permalink / raw) To: Tomer Barletz; +Cc: tj, linux-ide, linux-kernel On 08/02/2015 08:55 PM, Tomer Barletz wrote: >> Please use pr_err() instead. And "0x%d" makes no sense at all, please use "%#x" instead. > Yeah, not sure what I was drinking before writing this 0x%d thing... > Regarding the pr_err() - it is not used at all in this file, and > printk() is used instead. The problem is these printk() calls cause complaints from scripts/checkpatch.pl. > Wouldn't it be better to leave it with > printk for this change, then have another change that replaces > printk()s with pr_err()s? Probably yes... > --Tomer MBR, Sergei ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Check return code from pdc20621_i2c_read() 2015-08-03 15:42 ` Sergei Shtylyov @ 2015-08-03 15:52 ` Sergei Shtylyov 2015-08-03 18:46 ` [PATCH] sata_sx4: " Tomer Barletz 0 siblings, 1 reply; 16+ messages in thread From: Sergei Shtylyov @ 2015-08-03 15:52 UTC (permalink / raw) To: Tomer Barletz; +Cc: tj, linux-ide, linux-kernel On 08/03/2015 06:42 PM, Sergei Shtylyov wrote: >>> Please use pr_err() instead. And "0x%d" makes no sense at all, please >>> use "%#x" instead. >> Yeah, not sure what I was drinking before writing this 0x%d thing... >> Regarding the pr_err() - it is not used at all in this file, and >> printk() is used instead. Didn't notice before: the patch subject should start with the driver name and a colon, "sats_sx4: ". >> --Tomer MBR, Sergei ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() 2015-08-03 15:52 ` Sergei Shtylyov @ 2015-08-03 18:46 ` Tomer Barletz 2015-08-03 18:52 ` Sergei Shtylyov 0 siblings, 1 reply; 16+ messages in thread From: Tomer Barletz @ 2015-08-03 18:46 UTC (permalink / raw) To: tj, sergei.shtylyov; +Cc: linux-ide, linux-kernel, Tomer Barletz The variable spd0 might be used uninitialized when pdc20621_i2c_read() fails. This also generates a compilation warning with gcc 5.1. Signed-off-by: Tomer Barletz <barletz@gmail.com> --- drivers/ata/sata_sx4.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c index 3a18a8a..e1c1423 100644 --- a/drivers/ata/sata_sx4.c +++ b/drivers/ata/sata_sx4.c @@ -1238,8 +1238,12 @@ static unsigned int pdc20621_prog_dimm_global(struct ata_host *host) readl(mmio + PDC_SDRAM_CONTROL); /* Turn on for ECC */ - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, - PDC_DIMM_SPD_TYPE, &spd0); + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, + PDC_DIMM_SPD_TYPE, &spd0)) { + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", + PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); + return 1; + } if (spd0 == 0x02) { data |= (0x01 << 16); writel(data, mmio + PDC_SDRAM_CONTROL); @@ -1380,8 +1384,12 @@ static unsigned int pdc20621_dimm_init(struct ata_host *host) /* ECC initiliazation. */ - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, - PDC_DIMM_SPD_TYPE, &spd0); + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, + PDC_DIMM_SPD_TYPE, &spd0)) { + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", + PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); + return 1; + } if (spd0 == 0x02) { void *buf; VPRINTK("Start ECC initialization\n"); -- 2.4.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() 2015-08-03 18:46 ` [PATCH] sata_sx4: " Tomer Barletz @ 2015-08-03 18:52 ` Sergei Shtylyov 2015-08-03 19:04 ` Tomer Barletz 2015-08-03 19:18 ` Tomer Barletz 0 siblings, 2 replies; 16+ messages in thread From: Sergei Shtylyov @ 2015-08-03 18:52 UTC (permalink / raw) To: Tomer Barletz, tj; +Cc: linux-ide, linux-kernel On 08/03/2015 09:46 PM, Tomer Barletz wrote: > The variable spd0 might be used uninitialized when pdc20621_i2c_read() > fails. > This also generates a compilation warning with gcc 5.1. > Signed-off-by: Tomer Barletz <barletz@gmail.com> > --- > drivers/ata/sata_sx4.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c > index 3a18a8a..e1c1423 100644 > --- a/drivers/ata/sata_sx4.c > +++ b/drivers/ata/sata_sx4.c > @@ -1238,8 +1238,12 @@ static unsigned int pdc20621_prog_dimm_global(struct ata_host *host) > readl(mmio + PDC_SDRAM_CONTROL); > > /* Turn on for ECC */ > - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > - PDC_DIMM_SPD_TYPE, &spd0); > + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > + PDC_DIMM_SPD_TYPE, &spd0)) { That won't do, you didn't fix the indentation here. > + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", > + PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); > + return 1; > + } > if (spd0 == 0x02) { > data |= (0x01 << 16); > writel(data, mmio + PDC_SDRAM_CONTROL); > @@ -1380,8 +1384,12 @@ static unsigned int pdc20621_dimm_init(struct ata_host *host) > > /* ECC initiliazation. */ > > - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > - PDC_DIMM_SPD_TYPE, &spd0); > + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > + PDC_DIMM_SPD_TYPE, &spd0)) { And here. > + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", > + PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); > + return 1; > + } > if (spd0 == 0x02) { > void *buf; > VPRINTK("Start ECC initialization\n"); MBR, Sergei ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() 2015-08-03 18:52 ` Sergei Shtylyov @ 2015-08-03 19:04 ` Tomer Barletz 2015-08-03 19:09 ` Joe Perches 2015-08-03 19:19 ` Sergei Shtylyov 2015-08-03 19:18 ` Tomer Barletz 1 sibling, 2 replies; 16+ messages in thread From: Tomer Barletz @ 2015-08-03 19:04 UTC (permalink / raw) To: Sergei Shtylyov; +Cc: tj, linux-ide, linux-kernel I see how it makes sense to add a tab to align with the previous line of code, as it will always look similar in all editors, no matter how their tab character is set up to be. However, adding more tabs will just mess up editors that are not set up with 8-space width tabs. Is this a bug in checkpatch.pl, or are we saying everyone should have their editor set to 8-spaces width tabs? --Tomer On Mon, Aug 3, 2015 at 11:52 AM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > On 08/03/2015 09:46 PM, Tomer Barletz wrote: > >> The variable spd0 might be used uninitialized when pdc20621_i2c_read() >> fails. >> This also generates a compilation warning with gcc 5.1. > > >> Signed-off-by: Tomer Barletz <barletz@gmail.com> >> --- >> drivers/ata/sata_sx4.c | 16 ++++++++++++---- >> 1 file changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c >> index 3a18a8a..e1c1423 100644 >> --- a/drivers/ata/sata_sx4.c >> +++ b/drivers/ata/sata_sx4.c >> @@ -1238,8 +1238,12 @@ static unsigned int >> pdc20621_prog_dimm_global(struct ata_host *host) >> readl(mmio + PDC_SDRAM_CONTROL); >> >> /* Turn on for ECC */ >> - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, >> - PDC_DIMM_SPD_TYPE, &spd0); >> + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, >> + PDC_DIMM_SPD_TYPE, &spd0)) { > > > That won't do, you didn't fix the indentation here. > >> + printk(KERN_ERR "Failed in i2c read: device=%#x, >> subaddr=%#x\n", >> + PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); >> + return 1; >> + } >> if (spd0 == 0x02) { >> data |= (0x01 << 16); >> writel(data, mmio + PDC_SDRAM_CONTROL); >> @@ -1380,8 +1384,12 @@ static unsigned int pdc20621_dimm_init(struct >> ata_host *host) >> >> /* ECC initiliazation. */ >> >> - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, >> - PDC_DIMM_SPD_TYPE, &spd0); >> + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, >> + PDC_DIMM_SPD_TYPE, &spd0)) { > > > And here. > >> + printk(KERN_ERR "Failed in i2c read: device=%#x, >> subaddr=%#x\n", >> + PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); >> + return 1; >> + } >> if (spd0 == 0x02) { >> void *buf; >> VPRINTK("Start ECC initialization\n"); > > > MBR, Sergei > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() 2015-08-03 19:04 ` Tomer Barletz @ 2015-08-03 19:09 ` Joe Perches 2015-08-03 19:19 ` Sergei Shtylyov 1 sibling, 0 replies; 16+ messages in thread From: Joe Perches @ 2015-08-03 19:09 UTC (permalink / raw) To: Tomer Barletz, Sergei Shtylyov; +Cc: tj, linux-ide, linux-kernel On Mon, 2015-08-03 at 12:04 -0700, Tomer Barletz wrote: > I see how it makes sense to add a tab to align with the previous line > of code, as it will always look similar in all editors, no matter how > their tab character is set up to be. > However, adding more tabs will just mess up editors that are not set > up with 8-space width tabs. > > Is this a bug in checkpatch.pl, or are we saying everyone should have > their editor set to 8-spaces width tabs? from Documentation/CodingStyle: Chapter 1: Indentation Tabs are 8 characters, and thus indentations are also 8 characters. There are heretic movements that try to make indentations 4 (or even 2!) characters deep, and that is akin to trying to define the value of PI to be 3. Rationale: The whole idea behind indentation is to clearly define where a block of control starts and ends. Especially when you've been looking at your screen for 20 straight hours, you'll find it a lot easier to see how the indentation works if you have large indentations. Now, some people will claim that having 8-character indentations makes the code move too far to the right, and makes it hard to read on a 80-character terminal screen. The answer to that is that if you need more than 3 levels of indentation, you're screwed anyway, and should fix your program. In short, 8-char indents make things easier to read, and have the added benefit of warning you when you're nesting your functions too deep. Heed that warning. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() 2015-08-03 19:04 ` Tomer Barletz 2015-08-03 19:09 ` Joe Perches @ 2015-08-03 19:19 ` Sergei Shtylyov 1 sibling, 0 replies; 16+ messages in thread From: Sergei Shtylyov @ 2015-08-03 19:19 UTC (permalink / raw) To: Tomer Barletz; +Cc: tj, linux-ide, linux-kernel On 08/03/2015 10:04 PM, Tomer Barletz wrote: Please don't top-post. > I see how it makes sense to add a tab to align with the previous line > of code, as it will always look similar in all editors, no matter how > their tab character is set up to be. > However, adding more tabs will just mess up editors that are not set > up with 8-space width tabs. > Is this a bug in checkpatch.pl, or are we saying everyone should have > their editor set to 8-spaces width tabs? The latter. :-) MBR, Sergei ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() 2015-08-03 18:52 ` Sergei Shtylyov 2015-08-03 19:04 ` Tomer Barletz @ 2015-08-03 19:18 ` Tomer Barletz 2015-08-06 16:40 ` Tejun Heo 1 sibling, 1 reply; 16+ messages in thread From: Tomer Barletz @ 2015-08-03 19:18 UTC (permalink / raw) To: tj, sergei.shtylyov; +Cc: linux-ide, linux-kernel, Tomer Barletz The variable spd0 might be used uninitialized when pdc20621_i2c_read() fails. This also generates a compilation warning with gcc 5.1. Signed-off-by: Tomer Barletz <barletz@gmail.com> --- drivers/ata/sata_sx4.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c index 3a18a8a..b482c25 100644 --- a/drivers/ata/sata_sx4.c +++ b/drivers/ata/sata_sx4.c @@ -1238,8 +1238,12 @@ static unsigned int pdc20621_prog_dimm_global(struct ata_host *host) readl(mmio + PDC_SDRAM_CONTROL); /* Turn on for ECC */ - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, - PDC_DIMM_SPD_TYPE, &spd0); + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, + PDC_DIMM_SPD_TYPE, &spd0)) { + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", + PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); + return 1; + } if (spd0 == 0x02) { data |= (0x01 << 16); writel(data, mmio + PDC_SDRAM_CONTROL); @@ -1380,8 +1384,12 @@ static unsigned int pdc20621_dimm_init(struct ata_host *host) /* ECC initiliazation. */ - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, - PDC_DIMM_SPD_TYPE, &spd0); + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, + PDC_DIMM_SPD_TYPE, &spd0)) { + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", + PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); + return 1; + } if (spd0 == 0x02) { void *buf; VPRINTK("Start ECC initialization\n"); -- 2.4.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() 2015-08-03 19:18 ` Tomer Barletz @ 2015-08-06 16:40 ` Tejun Heo 0 siblings, 0 replies; 16+ messages in thread From: Tejun Heo @ 2015-08-06 16:40 UTC (permalink / raw) To: Tomer Barletz; +Cc: sergei.shtylyov, linux-ide, linux-kernel On Mon, Aug 03, 2015 at 12:18:13PM -0700, Tomer Barletz wrote: > The variable spd0 might be used uninitialized when pdc20621_i2c_read() > fails. > This also generates a compilation warning with gcc 5.1. > > Signed-off-by: Tomer Barletz <barletz@gmail.com> Applied to libata/for-4.2-fixes with minor edits. Thanks. -- tejun ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] Check return code from pdc20621_i2c_read() [not found] ` <CAO-S75CRyh=FPbeTJMZWiYTvpd_wn57f4eGQgOHW2p16+9ANhQ@mail.gmail.com> 2015-08-02 17:55 ` Tomer Barletz @ 2015-08-03 6:06 ` Tomer Barletz 2015-08-03 15:40 ` Sergei Shtylyov 1 sibling, 1 reply; 16+ messages in thread From: Tomer Barletz @ 2015-08-03 6:06 UTC (permalink / raw) To: tj, sergei.shtylyov; +Cc: linux-ide, linux-kernel, Tomer Barletz The variable spd0 might be used uninitialized when pdc20621_i2c_read() fails. This also generates a compilation warning with gcc 5.1. Signed-off-by: Tomer Barletz <barletz@gmail.com> --- drivers/ata/sata_sx4.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c index 3a18a8a..070b272 100644 --- a/drivers/ata/sata_sx4.c +++ b/drivers/ata/sata_sx4.c @@ -1238,8 +1238,11 @@ static unsigned int pdc20621_prog_dimm_global(struct ata_host *host) readl(mmio + PDC_SDRAM_CONTROL); /* Turn on for ECC */ - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, - PDC_DIMM_SPD_TYPE, &spd0); + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, + PDC_DIMM_SPD_TYPE, &spd0)) { + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); + return 1; + } if (spd0 == 0x02) { data |= (0x01 << 16); writel(data, mmio + PDC_SDRAM_CONTROL); @@ -1380,8 +1383,11 @@ static unsigned int pdc20621_dimm_init(struct ata_host *host) /* ECC initiliazation. */ - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, - PDC_DIMM_SPD_TYPE, &spd0); + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, + PDC_DIMM_SPD_TYPE, &spd0)) { + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); + return 1; + } if (spd0 == 0x02) { void *buf; VPRINTK("Start ECC initialization\n"); -- 2.4.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] Check return code from pdc20621_i2c_read() 2015-08-03 6:06 ` [PATCH] " Tomer Barletz @ 2015-08-03 15:40 ` Sergei Shtylyov 0 siblings, 0 replies; 16+ messages in thread From: Sergei Shtylyov @ 2015-08-03 15:40 UTC (permalink / raw) To: Tomer Barletz, tj; +Cc: linux-ide, linux-kernel Hello. On 08/03/2015 09:06 AM, Tomer Barletz wrote: > The variable spd0 might be used uninitialized when pdc20621_i2c_read() > fails. > This also generates a compilation warning with gcc 5.1. > Signed-off-by: Tomer Barletz <barletz@gmail.com> > --- > drivers/ata/sata_sx4.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c > index 3a18a8a..070b272 100644 > --- a/drivers/ata/sata_sx4.c > +++ b/drivers/ata/sata_sx4.c > @@ -1238,8 +1238,11 @@ static unsigned int pdc20621_prog_dimm_global(struct ata_host *host) > readl(mmio + PDC_SDRAM_CONTROL); > > /* Turn on for ECC */ > - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > - PDC_DIMM_SPD_TYPE, &spd0); > + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > + PDC_DIMM_SPD_TYPE, &spd0)) { > + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); > + return 1; > + } > if (spd0 == 0x02) { > data |= (0x01 << 16); > writel(data, mmio + PDC_SDRAM_CONTROL); > @@ -1380,8 +1383,11 @@ static unsigned int pdc20621_dimm_init(struct ata_host *host) > > /* ECC initiliazation. */ > > - pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > - PDC_DIMM_SPD_TYPE, &spd0); > + if (!pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, > + PDC_DIMM_SPD_TYPE, &spd0)) { > + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); > + return 1; > + } > if (spd0 == 0x02) { > void *buf; > VPRINTK("Start ECC initialization\n"); $ scripts/checkpatch.pl ~/patches/Check-return-code-from-pdc20621_i2c_read.patch ERROR: code indent should use tabs where possible #23: FILE: drivers/ata/sata_sx4.c:1242: +^I PDC_DIMM_SPD_TYPE, &spd0)) {$ WARNING: line over 80 characters #24: FILE: drivers/ata/sata_sx4.c:1243: + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... #24: FILE: drivers/ata/sata_sx4.c:1243: + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); ERROR: code indent should use tabs where possible #37: FILE: drivers/ata/sata_sx4.c:1387: +^I PDC_DIMM_SPD_TYPE, &spd0)) {$ WARNING: line over 80 characters #38: FILE: drivers/ata/sata_sx4.c:1388: + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... #38: FILE: drivers/ata/sata_sx4.c:1388: + printk(KERN_ERR "Failed in i2c read: device=%#x, subaddr=%#x\n", PDC_DIMM0_SPD_DEV_ADDRESS, PDC_DIMM_SPD_TYPE); total: 2 errors, 4 warnings, 26 lines checked NOTE: Whitespace errors detected. You may wish to use scripts/cleanpatch or scripts/cleanfile /home/headless/patches/Check-return-code-from-pdc20621_i2c_read.patch has style problems, please review. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. MBR, Sergei ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-08-06 16:40 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-02 10:12 [PATCH] Check return code from pdc20621_i2c_read() Tomer Barletz
2015-08-02 11:09 ` Sergei Shtylyov
[not found] ` <CAO-S75CRyh=FPbeTJMZWiYTvpd_wn57f4eGQgOHW2p16+9ANhQ@mail.gmail.com>
2015-08-02 17:55 ` Tomer Barletz
2015-08-02 18:03 ` Joe Perches
2015-08-02 20:22 ` Joe Perches
2015-08-03 15:42 ` Sergei Shtylyov
2015-08-03 15:52 ` Sergei Shtylyov
2015-08-03 18:46 ` [PATCH] sata_sx4: " Tomer Barletz
2015-08-03 18:52 ` Sergei Shtylyov
2015-08-03 19:04 ` Tomer Barletz
2015-08-03 19:09 ` Joe Perches
2015-08-03 19:19 ` Sergei Shtylyov
2015-08-03 19:18 ` Tomer Barletz
2015-08-06 16:40 ` Tejun Heo
2015-08-03 6:06 ` [PATCH] " Tomer Barletz
2015-08-03 15:40 ` Sergei Shtylyov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).