Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932302AbbHCTEX (ORCPT ); Mon, 3 Aug 2015 15:04:23 -0400 Received: from mail-io0-f169.google.com ([209.85.223.169]:34844 "EHLO mail-io0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932085AbbHCTEU (ORCPT ); Mon, 3 Aug 2015 15:04:20 -0400 MIME-Version: 1.0 In-Reply-To: <55BFB87C.1010405@cogentembedded.com> References: <55BF8E29.6000200@cogentembedded.com> <1438627595-17126-1-git-send-email-barletz@gmail.com> <55BFB87C.1010405@cogentembedded.com> Date: Mon, 3 Aug 2015 12:04:20 -0700 Message-ID: Subject: Re: [PATCH] sata_sx4: Check return code from pdc20621_i2c_read() From: Tomer Barletz To: Sergei Shtylyov Cc: tj@kernel.org, linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2824 Lines: 81 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 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 >> --- >> 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/