From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pa0-x22a.google.com ([2607:f8b0:400e:c03::22a]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Xhs4M-0000Jo-WE for linux-mtd@lists.infradead.org; Sat, 25 Oct 2014 03:31:12 +0000 Received: by mail-pa0-f42.google.com with SMTP id bj1so2231692pad.1 for ; Fri, 24 Oct 2014 20:30:49 -0700 (PDT) Message-ID: <544B194B.1090409@gmail.com> Date: Sat, 25 Oct 2014 11:30:19 +0800 From: Zhou Wang MIME-Version: 1.0 To: Haojian Zhuang Subject: Re: [PATCH v2 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc References: <1414073085-19296-1-git-send-email-wangzhou.bry@gmail.com> <1414073085-19296-2-git-send-email-wangzhou.bry@gmail.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Cc: Mark Rutland , devicetree@vger.kernel.org, pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, "linux-kernel@vger.kernel.org" , "xuwei \(O\)" , wangzhou1@hisilicon.com, Rob Herring , linux-mtd@lists.infradead.org, galak@codeaurora.org, caizhiyong@huawei.com, Brian Norris , David Woodhouse List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 2014年10月24日 19:46, Haojian Zhuang wrote: > On Thu, Oct 23, 2014 at 10:04 PM, Zhou Wang wrote: >> Signed-off-by: Zhou Wang >> --- >> drivers/mtd/nand/Kconfig | 5 + >> drivers/mtd/nand/Makefile | 1 + >> drivers/mtd/nand/hisi504_nand.c | 836 +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 842 insertions(+) >> create mode 100644 drivers/mtd/nand/hisi504_nand.c >> > > I think that you need to run scripts/checkpatch.pl. There're some > warnings reported on this patch. > Hi Haojian, I will run the checkpatch.pl again and fix the warnings. But for lines like this: + if (host->command == NAND_CMD_ERASE1) + host->addr_value[0] |= ((page_addr >>16) & 0xff) << 16; + else + host->addr_value[1] |= ((page_addr >> 16) & 0xff); Only over 80 for some characters in one line, it will report a warning. Can I just leave the warning there for the tidy of the code? >> + >> + case NAND_CMD_SEQIN: >> + host->offset = column; >> + > > It's better not using waterfall style. Maybe you can write it clearly. > case NAND_CMD_SEQIN: > host->offset = column; > set_addr(mtd, column, page_addr); > break; Thanks, I will modify the code like this above. > >> + chip->ecc.mode = of_get_nand_ecc_mode(np); >> + /* read ecc-bits from dts */ >> + of_property_read_u32(np, "hisi,nand-ecc-bits", &host->ecc_bits); > > Do you need to check the ecc_bits at here? Maybe user inputed the > wrong ecc_bits in DTS. Yes, it is better to check the ecc_bits here, thanks for your reminding. Best regards, Zhou Wang > > Best Regards > Haojian > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zhou Wang Subject: Re: [PATCH v2 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc Date: Sat, 25 Oct 2014 11:30:19 +0800 Message-ID: <544B194B.1090409@gmail.com> References: <1414073085-19296-1-git-send-email-wangzhou.bry@gmail.com> <1414073085-19296-2-git-send-email-wangzhou.bry@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Haojian Zhuang Cc: David Woodhouse , Brian Norris , linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Mark Rutland , pawel.moll-5wv7dgnIgG8@public.gmane.org, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org, Rob Herring , galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, caizhiyong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org, "xuwei (O)" , wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org On 2014=E5=B9=B410=E6=9C=8824=E6=97=A5 19:46, Haojian Zhuang wrote: > On Thu, Oct 23, 2014 at 10:04 PM, Zhou Wang = wrote: >> Signed-off-by: Zhou Wang >> --- >> drivers/mtd/nand/Kconfig | 5 + >> drivers/mtd/nand/Makefile | 1 + >> drivers/mtd/nand/hisi504_nand.c | 836 +++++++++++++++++++++++++++= ++++++++++++ >> 3 files changed, 842 insertions(+) >> create mode 100644 drivers/mtd/nand/hisi504_nand.c >> > > I think that you need to run scripts/checkpatch.pl. There're some > warnings reported on this patch. > Hi Haojian, I will run the checkpatch.pl again and fix the warnings. But for lines like this: + if (host->command =3D=3D NAND_CMD_ERASE1) + host->addr_value[0] |=3D ((page_addr >>16) & 0xff) << 16; + else + host->addr_value[1] |=3D ((page_addr >> 16) & 0xff); Only over 80 for some characters in one line, it will report a warning. Can I just leave the warning there for the tidy of the code? >> + >> + case NAND_CMD_SEQIN: >> + host->offset =3D column; >> + > > It's better not using waterfall style. Maybe you can write it clearly= =2E > case NAND_CMD_SEQIN: > host->offset =3D column; > set_addr(mtd, column, page_addr); > break; Thanks, I will modify the code like this above. > >> + chip->ecc.mode =3D of_get_nand_ecc_mode(np); >> + /* read ecc-bits from dts */ >> + of_property_read_u32(np, "hisi,nand-ecc-bits", &host->ecc_bi= ts); > > Do you need to check the ecc_bits at here? Maybe user inputed the > wrong ecc_bits in DTS. Yes, it is better to check the ecc_bits here, thanks for your reminding= =2E Best regards, Zhou Wang > > Best Regards > Haojian > -- To unsubscribe from this list: send the line "unsubscribe devicetree" i= n the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753347AbaJYDau (ORCPT ); Fri, 24 Oct 2014 23:30:50 -0400 Received: from mail-pd0-f172.google.com ([209.85.192.172]:59553 "EHLO mail-pd0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751827AbaJYDas (ORCPT ); Fri, 24 Oct 2014 23:30:48 -0400 Message-ID: <544B194B.1090409@gmail.com> Date: Sat, 25 Oct 2014 11:30:19 +0800 From: Zhou Wang User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130623 Thunderbird/17.0.7 MIME-Version: 1.0 To: Haojian Zhuang CC: David Woodhouse , Brian Norris , linux-mtd@lists.infradead.org, Mark Rutland , pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, Rob Herring , galak@codeaurora.org, caizhiyong@huawei.com, "xuwei (O)" , wangzhou1@hisilicon.com, "linux-kernel@vger.kernel.org" , devicetree@vger.kernel.org Subject: Re: [PATCH v2 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc References: <1414073085-19296-1-git-send-email-wangzhou.bry@gmail.com> <1414073085-19296-2-git-send-email-wangzhou.bry@gmail.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2014年10月24日 19:46, Haojian Zhuang wrote: > On Thu, Oct 23, 2014 at 10:04 PM, Zhou Wang wrote: >> Signed-off-by: Zhou Wang >> --- >> drivers/mtd/nand/Kconfig | 5 + >> drivers/mtd/nand/Makefile | 1 + >> drivers/mtd/nand/hisi504_nand.c | 836 +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 842 insertions(+) >> create mode 100644 drivers/mtd/nand/hisi504_nand.c >> > > I think that you need to run scripts/checkpatch.pl. There're some > warnings reported on this patch. > Hi Haojian, I will run the checkpatch.pl again and fix the warnings. But for lines like this: + if (host->command == NAND_CMD_ERASE1) + host->addr_value[0] |= ((page_addr >>16) & 0xff) << 16; + else + host->addr_value[1] |= ((page_addr >> 16) & 0xff); Only over 80 for some characters in one line, it will report a warning. Can I just leave the warning there for the tidy of the code? >> + >> + case NAND_CMD_SEQIN: >> + host->offset = column; >> + > > It's better not using waterfall style. Maybe you can write it clearly. > case NAND_CMD_SEQIN: > host->offset = column; > set_addr(mtd, column, page_addr); > break; Thanks, I will modify the code like this above. > >> + chip->ecc.mode = of_get_nand_ecc_mode(np); >> + /* read ecc-bits from dts */ >> + of_property_read_u32(np, "hisi,nand-ecc-bits", &host->ecc_bits); > > Do you need to check the ecc_bits at here? Maybe user inputed the > wrong ecc_bits in DTS. Yes, it is better to check the ecc_bits here, thanks for your reminding. Best regards, Zhou Wang > > Best Regards > Haojian >