From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751702AbdFGHVo (ORCPT ); Wed, 7 Jun 2017 03:21:44 -0400 Received: from conssluserg-02.nifty.com ([210.131.2.81]:36688 "EHLO conssluserg-02.nifty.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751591AbdFGHVm (ORCPT ); Wed, 7 Jun 2017 03:21:42 -0400 DKIM-Filter: OpenDKIM Filter v2.10.3 conssluserg-02.nifty.com v577LG3D000562 X-Nifty-SrcIP: [209.85.161.177] MIME-Version: 1.0 In-Reply-To: <20170607090250.729625c9@bbrezillon> References: <1496704922-12261-1-git-send-email-yamada.masahiro@socionext.com> <1496704922-12261-5-git-send-email-yamada.masahiro@socionext.com> <20170607000133.69fbf82b@bbrezillon> <20170607090250.729625c9@bbrezillon> From: Masahiro Yamada Date: Wed, 7 Jun 2017 16:21:15 +0900 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4 04/23] mtd: nand: denali: avoid hard-coding ECC step, strength, bytes To: Boris Brezillon Cc: Mark Rutland , devicetree@vger.kernel.org, Richard Weinberger , Marek Vasut , Artem Bityutskiy , Cyrille Pitchen , Linux Kernel Mailing List , Dinh Nguyen , Rob Herring , linux-mtd@lists.infradead.org, Masami Hiramatsu , Chuanxiao Dong , Jassi Brar , Brian Norris , Enrico Jorns , David Woodhouse , Graham Moore Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Boris, 2017-06-07 16:02 GMT+09:00 Boris Brezillon : > On Wed, 7 Jun 2017 12:09:31 +0900 > Masahiro Yamada wrote: > >> >> + >> >> +static int denali_ecc_setup(struct mtd_info *mtd, struct nand_chip *chip, >> >> + struct denali_nand_info *denali) >> >> +{ >> >> + struct nand_ecc_caps caps; >> >> + int ret; >> >> + >> >> + caps.stepinfos = denali->stepinfo; >> >> + caps.nstepinfos = 1; >> >> + caps.calc_ecc_bytes = denali_calc_ecc_bytes; >> >> + caps.oob_reserve_bytes = denali->bbtskipbytes; >> > >> > If you get rid of this oob_reserve_bytes field, you can define caps as >> > a static const and even directly store ecc_caps in denali_nand_info. >> >> To make caps static const, denali_calc_ecc_bytes must be exported >> to be referenced from denali_dt/denali_pci. >> I am reluctant to do it. > > You already duplicate other information in denali_dt.c and > denali_pci.c, The ECC step-size and strength are tightly associated to each IP variant. I see duplication between denali_dt and denali_pci, but it is just because Intel and Altera happened to have the same parameters. On the other hand, denali_calc_ecc_bytes() is common to all variants because ECC algorithm is not customizable. > so what prevents you from duplicating this one-line > function? > > Also, denali core already exports 2 functions, They are entries for probe/remove. > I don't see the problem > in exporting the common nand_ecc_caps object. Why are you reluctant to > that? denali_calc_ecc_bytes() is independent of DT, PCI, or whatever. I see less reason to expose it. caps is only used on probing, so I used a local variable. I do not think it is a big problem. -- Best Regards Masahiro Yamada From mboxrd@z Thu Jan 1 00:00:00 1970 From: Masahiro Yamada Subject: Re: [PATCH v4 04/23] mtd: nand: denali: avoid hard-coding ECC step, strength, bytes Date: Wed, 7 Jun 2017 16:21:15 +0900 Message-ID: References: <1496704922-12261-1-git-send-email-yamada.masahiro@socionext.com> <1496704922-12261-5-git-send-email-yamada.masahiro@socionext.com> <20170607000133.69fbf82b@bbrezillon> <20170607090250.729625c9@bbrezillon> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <20170607090250.729625c9@bbrezillon> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Boris Brezillon Cc: Mark Rutland , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Richard Weinberger , Marek Vasut , Artem Bityutskiy , Cyrille Pitchen , Linux Kernel Mailing List , Dinh Nguyen , Rob Herring , linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Masami Hiramatsu , Chuanxiao Dong , Jassi Brar , Brian Norris , Enrico Jorns , David Woodhouse , Graham Moore List-Id: devicetree@vger.kernel.org Hi Boris, 2017-06-07 16:02 GMT+09:00 Boris Brezillon : > On Wed, 7 Jun 2017 12:09:31 +0900 > Masahiro Yamada wrote: > >> >> + >> >> +static int denali_ecc_setup(struct mtd_info *mtd, struct nand_chip *chip, >> >> + struct denali_nand_info *denali) >> >> +{ >> >> + struct nand_ecc_caps caps; >> >> + int ret; >> >> + >> >> + caps.stepinfos = denali->stepinfo; >> >> + caps.nstepinfos = 1; >> >> + caps.calc_ecc_bytes = denali_calc_ecc_bytes; >> >> + caps.oob_reserve_bytes = denali->bbtskipbytes; >> > >> > If you get rid of this oob_reserve_bytes field, you can define caps as >> > a static const and even directly store ecc_caps in denali_nand_info. >> >> To make caps static const, denali_calc_ecc_bytes must be exported >> to be referenced from denali_dt/denali_pci. >> I am reluctant to do it. > > You already duplicate other information in denali_dt.c and > denali_pci.c, The ECC step-size and strength are tightly associated to each IP variant. I see duplication between denali_dt and denali_pci, but it is just because Intel and Altera happened to have the same parameters. On the other hand, denali_calc_ecc_bytes() is common to all variants because ECC algorithm is not customizable. > so what prevents you from duplicating this one-line > function? > > Also, denali core already exports 2 functions, They are entries for probe/remove. > I don't see the problem > in exporting the common nand_ecc_caps object. Why are you reluctant to > that? denali_calc_ecc_bytes() is independent of DT, PCI, or whatever. I see less reason to expose it. caps is only used on probing, so I used a local variable. I do not think it is a big problem. -- Best Regards Masahiro Yamada -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html