Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1198324 > unrolled thread

[PATCH] Check return code from pdc20621_i2c_read()

Started byTomer Barletz <barletz@gmail.com>
First post2015-08-02 12:20 +0200
Last post2015-08-03 21:20 +0200
Articles 13 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] Check return code from pdc20621_i2c_read() Tomer Barletz <barletz@gmail.com> - 2015-08-02 12:20 +0200
    Re: [PATCH] Check return code from pdc20621_i2c_read() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-08-02 13:10 +0200
      Re: [PATCH] Check return code from pdc20621_i2c_read() Joe Perches <joe@perches.com> - 2015-08-02 20:10 +0200
      Re: [PATCH] Check return code from pdc20621_i2c_read() Tomer Barletz <barletz@gmail.com> - 2015-08-02 20:00 +0200
        Re: [PATCH] Check return code from pdc20621_i2c_read() Joe Perches <joe@perches.com> - 2015-08-02 22:30 +0200
        Re: [PATCH] Check return code from pdc20621_i2c_read() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-08-03 17:50 +0200
          Re: [PATCH] Check return code from pdc20621_i2c_read() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-08-03 18:00 +0200
            [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() Tomer Barletz <barletz@gmail.com> - 2015-08-03 20:50 +0200
              Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-08-03 21:00 +0200
                Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() Joe Perches <joe@perches.com> - 2015-08-03 21:10 +0200
                Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() Tomer Barletz <barletz@gmail.com> - 2015-08-03 21:10 +0200
                  Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-08-03 21:20 +0200
                [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() Tomer Barletz <barletz@gmail.com> - 2015-08-03 21:20 +0200

#1198324 — [PATCH] Check return code from pdc20621_i2c_read()

FromTomer Barletz <barletz@gmail.com>
Date2015-08-02 12:20 +0200
Subject[PATCH] Check return code from pdc20621_i2c_read()
Message-ID<pSYx4-Um-1@gated-at.bofh.it>
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1198330

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2015-08-02 13:10 +0200
Message-ID<pSZjr-25y-17@gated-at.bofh.it>
In reply to#1198324
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1198416

FromJoe Perches <joe@perches.com>
Date2015-08-02 20:10 +0200
Message-ID<pT5RU-3eo-5@gated-at.bofh.it>
In reply to#1198330
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>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1198444

FromTomer Barletz <barletz@gmail.com>
Date2015-08-02 20:00 +0200
Message-ID<pT5If-2NJ-29@gated-at.bofh.it>
In reply to#1198330
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1198447

FromJoe Perches <joe@perches.com>
Date2015-08-02 22:30 +0200
Message-ID<pT83o-6i0-7@gated-at.bofh.it>
In reply to#1198444
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...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1198974

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2015-08-03 17:50 +0200
Message-ID<pTq9X-7oo-1@gated-at.bofh.it>
In reply to#1198444
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1198994

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2015-08-03 18:00 +0200
Message-ID<pTqjG-7A6-55@gated-at.bofh.it>
In reply to#1198974
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1199181 — [PATCH] sata_sx4: Check return code from pdc20621_i2c_read()

FromTomer Barletz <barletz@gmail.com>
Date2015-08-03 20:50 +0200
Subject[PATCH] sata_sx4: Check return code from pdc20621_i2c_read()
Message-ID<pTsY9-31v-17@gated-at.bofh.it>
In reply to#1198994
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1199191 — Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read()

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2015-08-03 21:00 +0200
SubjectRe: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read()
Message-ID<pTt7R-3cQ-21@gated-at.bofh.it>
In reply to#1199181
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1199196 — Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read()

FromJoe Perches <joe@perches.com>
Date2015-08-03 21:10 +0200
SubjectRe: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read()
Message-ID<pTthx-3DC-21@gated-at.bofh.it>
In reply to#1199191
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1199198 — Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read()

FromTomer Barletz <barletz@gmail.com>
Date2015-08-03 21:10 +0200
SubjectRe: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read()
Message-ID<pTthx-3DC-23@gated-at.bofh.it>
In reply to#1199191
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1199203 — Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read()

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2015-08-03 21:20 +0200
SubjectRe: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read()
Message-ID<pTtrc-3P6-5@gated-at.bofh.it>
In reply to#1199198
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1199202 — [PATCH] sata_sx4: Check return code from pdc20621_i2c_read()

FromTomer Barletz <barletz@gmail.com>
Date2015-08-03 21:20 +0200
Subject[PATCH] sata_sx4: Check return code from pdc20621_i2c_read()
Message-ID<pTtrb-3P6-3@gated-at.bofh.it>
In reply to#1199191
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web