From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: Richard Weinberger <richard@nod.at>,
linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
guohanjun@huawei.com
Subject: Re: [PATCH] mtd: nand: convert to unified device property interface
Date: Thu, 17 Aug 2017 23:55:02 +0200 [thread overview]
Message-ID: <20170817235502.6c0e8804@bbrezillon> (raw)
In-Reply-To: <1502868545-124616-1-git-send-email-wangkefeng.wang@huawei.com>
Le Wed, 16 Aug 2017 15:29:05 +0800,
Kefeng Wang <wangkefeng.wang@huawei.com> 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 <wangkefeng.wang@huawei.com>
> ---
>
> - 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 <linux/bitops.h>
> #include <linux/io.h>
> #include <linux/mtd/partitions.h>
> -#include <linux/of.h>
> +#include <linux/property.h>
>
> 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_?
> {
> - 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()?
> {
> - 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).
> 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).
> 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) {
> /*
next prev parent reply other threads:[~2017-08-17 21:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-16 7:29 [PATCH] mtd: nand: convert to unified device property interface Kefeng Wang
2017-08-17 21:55 ` Boris Brezillon [this message]
2017-08-18 1:35 ` Kefeng Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170817235502.6c0e8804@bbrezillon \
--to=boris.brezillon@free-electrons.com \
--cc=guohanjun@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=richard@nod.at \
--cc=wangkefeng.wang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox