Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753966AbdHRBhl (ORCPT ); Thu, 17 Aug 2017 21:37:41 -0400 Received: from szxga04-in.huawei.com ([45.249.212.190]:4489 "EHLO szxga04-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753482AbdHRBhk (ORCPT ); Thu, 17 Aug 2017 21:37:40 -0400 Subject: Re: [PATCH] mtd: nand: convert to unified device property interface To: Boris Brezillon CC: Richard Weinberger , , , References: <1502868545-124616-1-git-send-email-wangkefeng.wang@huawei.com> <20170817235502.6c0e8804@bbrezillon> From: Kefeng Wang Message-ID: Date: Fri, 18 Aug 2017 09:35:14 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: <20170817235502.6c0e8804@bbrezillon> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [10.177.19.180] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A0B0204.599644D6.004D,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 576b0155b2c434b18380a37a5b42f93c Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6580 Lines: 195 On 2017/8/18 5:55, Boris Brezillon wrote: > Le Wed, 16 Aug 2017 15:29:05 +0800, > Kefeng Wang a écrit : > >> Changing from of_* to device_* interface, then we can also extract >> the properties from ACPI tables as well as from DT. >> >> Signed-off-by: Kefeng Wang >> --- >> >> - APCI will be supported in hisi504_nand.c, and it will use nand_scan_ident(). >> >> drivers/mtd/nand/nand_base.c | 54 ++++++++++++++++++++++---------------------- >> 1 file changed, 27 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c >> index c6c18b8..27a0947 100644 >> --- a/drivers/mtd/nand/nand_base.c >> +++ b/drivers/mtd/nand/nand_base.c >> @@ -46,7 +46,7 @@ >> #include >> #include >> #include >> -#include >> +#include >> >> static int nand_get_device(struct mtd_info *mtd, int new_state); >> >> @@ -4209,12 +4209,12 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type) >> [NAND_ECC_ON_DIE] = "on-die", >> }; >> >> -static int of_get_nand_ecc_mode(struct device_node *np) >> +static int device_get_nand_ecc_mode(struct device *dev) >> { >> const char *pm; >> int err, i; >> >> - err = of_property_read_string(np, "nand-ecc-mode", &pm); >> + err = device_property_read_string(dev, "nand-ecc-mode", &pm); >> if (err < 0) >> return err; >> >> @@ -4238,12 +4238,12 @@ static int of_get_nand_ecc_mode(struct device_node *np) >> [NAND_ECC_BCH] = "bch", >> }; >> >> -static int of_get_nand_ecc_algo(struct device_node *np) >> +static int device_get_nand_ecc_algo(struct device *dev) >> { >> const char *pm; >> int err, i; >> >> - err = of_property_read_string(np, "nand-ecc-algo", &pm); >> + err = device_property_read_string(dev, "nand-ecc-algo", &pm); >> if (!err) { >> for (i = NAND_ECC_HAMMING; i < ARRAY_SIZE(nand_ecc_algos); i++) >> if (!strcasecmp(pm, nand_ecc_algos[i])) >> @@ -4255,7 +4255,7 @@ static int of_get_nand_ecc_algo(struct device_node *np) >> * For backward compatibility we also read "nand-ecc-mode" checking >> * for some obsoleted values that were specifying ECC algorithm. >> */ >> - err = of_property_read_string(np, "nand-ecc-mode", &pm); >> + err = device_property_read_string(dev, "nand-ecc-mode", &pm); >> if (err < 0) >> return err; >> >> @@ -4267,29 +4267,29 @@ static int of_get_nand_ecc_algo(struct device_node *np) >> return -ENODEV; >> } >> >> -static int of_get_nand_ecc_step_size(struct device_node *np) >> +static int device_get_nand_ecc_step_size(struct device *dev) >> { >> int ret; >> u32 val; >> >> - ret = of_property_read_u32(np, "nand-ecc-step-size", &val); >> + ret = device_property_read_u32(dev, "nand-ecc-step-size", &val); >> return ret ? ret : val; >> } >> >> -static int of_get_nand_ecc_strength(struct device_node *np) >> +static int device_get_nand_ecc_strength(struct device *dev) >> { >> int ret; >> u32 val; >> >> - ret = of_property_read_u32(np, "nand-ecc-strength", &val); >> + ret = device_property_read_u32(dev, "nand-ecc-strength", &val); >> return ret ? ret : val; >> } >> >> -static int of_get_nand_bus_width(struct device_node *np) >> +static int device_get_nand_bus_width(struct device *dev) >> { >> u32 val; >> >> - if (of_property_read_u32(np, "nand-bus-width", &val)) >> + if (device_property_read_u32(dev, "nand-bus-width", &val)) >> return 8; >> >> switch (val) { >> @@ -4301,29 +4301,28 @@ static int of_get_nand_bus_width(struct device_node *np) >> } >> } >> >> -static bool of_get_nand_on_flash_bbt(struct device_node *np) >> +static bool device_get_nand_on_flash_bbt(struct device *dev) > Not sure I like the device prefix, for the same reason I don't like > nand_chip_init(). How about fw_ or fwnode_? OK. > >> { >> - return of_property_read_bool(np, "nand-on-flash-bbt"); >> + return device_property_read_bool(dev, "nand-on-flash-bbt"); >> } >> >> -static int nand_dt_init(struct nand_chip *chip) >> +static int nand_chip_init(struct nand_chip *chip, struct device *dev) > Hm, nand_chip_init() is confusing. How about nand_fwnode_init() or > nand_get_fw_props()? OK. >> { >> - struct device_node *dn = nand_get_flash_node(chip); >> int ecc_mode, ecc_algo, ecc_strength, ecc_step; >> >> - if (!dn) >> + if (!dev) >> return 0; >> >> - if (of_get_nand_bus_width(dn) == 16) >> + if (device_get_nand_bus_width(dev) == 16) >> chip->options |= NAND_BUSWIDTH_16; >> >> - if (of_get_nand_on_flash_bbt(dn)) >> + if (device_get_nand_on_flash_bbt(dev)) >> chip->bbt_options |= NAND_BBT_USE_FLASH; >> >> - ecc_mode = of_get_nand_ecc_mode(dn); >> - ecc_algo = of_get_nand_ecc_algo(dn); >> - ecc_strength = of_get_nand_ecc_strength(dn); >> - ecc_step = of_get_nand_ecc_step_size(dn); >> + ecc_mode = device_get_nand_ecc_mode(dev); >> + ecc_algo = device_get_nand_ecc_algo(dev); >> + ecc_strength = device_get_nand_ecc_strength(dev); >> + ecc_step = device_get_nand_ecc_step_size(dev); >> >> if (ecc_mode >= 0) >> chip->ecc.mode = ecc_mode; >> @@ -4337,7 +4336,7 @@ static int nand_dt_init(struct nand_chip *chip) >> if (ecc_step > 0) >> chip->ecc.size = ecc_step; >> >> - if (of_property_read_bool(dn, "nand-ecc-maximize")) >> + if (device_property_read_bool(dev, "nand-ecc-maximize")) >> chip->ecc.options |= NAND_ECC_MAXIMIZE; >> >> return 0; >> @@ -4358,14 +4357,15 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips, >> { >> int i, nand_maf_id, nand_dev_id; >> struct nand_chip *chip = mtd_to_nand(mtd); >> + struct device *dev = mtd->dev.parent; > Sorry but this is wrong. If you do that you break all drivers that have > NAND devices represented as children nodes of the NAND controller. > > If you want to support ACPI, you'll have to use &mtd->dev as the > reference dev, and make sure its ->fwnode is correctly set (just patch > nand_get_flash_node() to do the right thing). Oops, sorry, this patch should be a RFC, will update it based on your suggestion, thanks. > >> int ret; >> >> - ret = nand_dt_init(chip); >> + ret = nand_chip_init(chip, dev); > No need to pass dev as a second argument here (can be extracted inside > the function). OK. >> if (ret) >> return ret; >> >> - if (!mtd->name && mtd->dev.parent) >> - mtd->name = dev_name(mtd->dev.parent); >> + if (!mtd->name && dev) >> + mtd->name = dev_name(dev); >> >> if ((!chip->cmdfunc || !chip->select_chip) && !chip->cmd_ctrl) { >> /* > > . >