[{"id":1577877,"web_url":"http://patchwork.ozlabs.org/comment/1577877/","msgid":"","list_archive_url":null,"date":"2017-02-08T05:09:18","subject":"Re: [U-Boot] [PATCH] nand: atmel: add drvice tree support and dm\n\tgpio APIs","submitter":{"id":6170,"url":"http://patchwork.ozlabs.org/api/people/6170/","name":"Simon Glass","email":"sjg@chromium.org"},"content":"Hi Wenyou,\n\nOn 3 February 2017 at 18:32, Wenyou Yang wrote:\n>\n> In order to use the driver model gpio APIs, add the device tree\n> support.\n>\n> Signed-off-by: Wenyou Yang \n> ---\n>\n> drivers/mtd/nand/atmel_nand.c | 143 ++++++++++++++++++++++++++++++++++--------\n> include/fdtdec.h | 1 +\n> lib/fdtdec.c | 1 +\n> 3 files changed, 119 insertions(+), 26 deletions(-)\n>\n> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c\n> index 8669432deb..87ff5c2eb4 100644\n> --- a/drivers/mtd/nand/atmel_nand.c\n> +++ b/drivers/mtd/nand/atmel_nand.c\n> @@ -14,29 +14,34 @@\n> #include \n> #include \n> #include \n> +#ifdef CONFIG_DM_GPIO\n> +#include \n> +#endif\n>\n> +#include \n> #include \n> #include \n> #include \n> #include \n>\n> -#ifdef CONFIG_ATMEL_NAND_HWECC\n> -\n> -/* Register access macros */\n> -#define ecc_readl(add, reg) \\\n> - readl(add + ATMEL_ECC_##reg)\n> -#define ecc_writel(add, reg, value) \\\n> - writel((value), add + ATMEL_ECC_##reg)\n\nWhat is happening here? Seems like it should be a separate patch.\n\n> +DECLARE_GLOBAL_DATA_PTR;\n> +\n> +struct atmel_nand_data {\n> + struct gpio_desc enable_pin; /* chip enable */\n> + struct gpio_desc det_pin; /* card detect */\n> + struct gpio_desc rdy_pin; /* ready/busy */\n> + u8 ale; /* address line number connected to ALE */\n> + u8 cle; /* address line number connected to CLE */\n> + u8 bus_width_16; /* buswidth is 16 bit */\n> + u8 ecc_mode; /* ecc mode */\n> + u8 on_flash_bbt; /* bbt on flash */\n> +};\n>\n> -#include \"atmel_nand_ecc.h\" /* Hardware ECC registers */\n> +struct atmel_nand_host {\n> + void __iomem *io_base;\n> + struct atmel_nand_data board;\n>\n> #ifdef CONFIG_ATMEL_NAND_HW_PMECC\n> -\n> -#ifdef CONFIG_SPL_BUILD\n> -#undef CONFIG_SYS_NAND_ONFI_DETECTION\n> -#endif\n> -\n> -struct atmel_nand_host {\n> struct pmecc_regs __iomem *pmecc;\n> struct pmecc_errloc_regs __iomem *pmerrloc;\n> void __iomem *pmecc_rom_base;\n> @@ -63,9 +68,26 @@ struct atmel_nand_host {\n> int *pmecc_mu;\n> int *pmecc_dmu;\n> int *pmecc_delta;\n> +#endif\n> };\n>\n> -static struct atmel_nand_host pmecc_host;\n> +static struct atmel_nand_host nand_host;\n> +#ifdef CONFIG_ATMEL_NAND_HWECC\n> +\n> +/* Register access macros */\n> +#define ecc_readl(add, reg) \\\n> + readl(add + ATMEL_ECC_##reg)\n> +#define ecc_writel(add, reg, value) \\\n> + writel((value), add + ATMEL_ECC_##reg)\n> +\n> +#include \"atmel_nand_ecc.h\" /* Hardware ECC registers */\n> +\n> +#ifdef CONFIG_ATMEL_NAND_HW_PMECC\n> +\n> +#ifdef CONFIG_SPL_BUILD\n> +#undef CONFIG_SYS_NAND_ONFI_DETECTION\n> +#endif\n> +\n> static struct nand_ecclayout atmel_pmecc_oobinfo;\n>\n> /*\n> @@ -805,12 +827,9 @@ static uint16_t *create_lookup_table(int sector_size)\n> static int atmel_pmecc_nand_init_params(struct nand_chip *nand,\n> struct mtd_info *mtd)\n> {\n> - struct atmel_nand_host *host;\n> + struct atmel_nand_host *host = nand_get_controller_data(nand);\n> int cap, sector_size;\n>\n> - host = &pmecc_host;\n> - nand_set_controller_data(nand, host);\n> -\n> nand->ecc.mode = NAND_ECC_HW;\n> nand->ecc.calculate = NULL;\n> nand->ecc.correct = NULL;\n> @@ -1207,12 +1226,24 @@ int atmel_hwecc_nand_init_param(struct nand_chip *nand, struct mtd_info *mtd)\n> #endif /* CONFIG_ATMEL_NAND_HWECC */\n>\n> static void at91_nand_hwcontrol(struct mtd_info *mtd,\n> - int cmd, unsigned int ctrl)\n> + int cmd, unsigned int ctrl)\n> {\n> struct nand_chip *this = mtd_to_nand(mtd);\n> +#if CONFIG_IS_ENABLED(OF_CONTROL)\n> + struct atmel_nand_host *host = nand_get_controller_data(this);\n> +#endif\n>\n> if (ctrl & NAND_CTRL_CHANGE) {\n> - ulong IO_ADDR_W = (ulong) this->IO_ADDR_W;\n> + ulong IO_ADDR_W = (ulong)this->IO_ADDR_W;\n> +#if CONFIG_IS_ENABLED(OF_CONTROL)\n> + IO_ADDR_W &= ~((1 << host->board.ale)\n> + | (1 << host->board.cle));\n> +\n> + if (ctrl & NAND_CLE)\n> + IO_ADDR_W |= (1 << host->board.cle);\n> + if (ctrl & NAND_ALE)\n> + IO_ADDR_W |= (1 << host->board.ale);\n> +#else\n> IO_ADDR_W &= ~(CONFIG_SYS_NAND_MASK_ALE\n> | CONFIG_SYS_NAND_MASK_CLE);\n\nYou could adjust it so that CONFIG_SYS_NAND_MASK_CLE is assigned to\nhost->board.cle in the init function, and then avoid this #ifdef here.\n\n>\n> @@ -1220,10 +1251,18 @@ static void at91_nand_hwcontrol(struct mtd_info *mtd,\n> IO_ADDR_W |= CONFIG_SYS_NAND_MASK_CLE;\n> if (ctrl & NAND_ALE)\n> IO_ADDR_W |= CONFIG_SYS_NAND_MASK_ALE;\n> +#endif\n>\n> +#ifdef CONFIG_DM_GPIO\n> + if (dm_gpio_is_valid(&host->board.enable_pin))\n> + dm_gpio_set_value(&host->board.enable_pin,\n> + !(ctrl & NAND_NCE));\n> +#else\n> #ifdef CONFIG_SYS_NAND_ENABLE_PIN\n> gpio_set_value(CONFIG_SYS_NAND_ENABLE_PIN, !(ctrl & NAND_NCE));\n> #endif\n> +#endif\n> +\n> this->IO_ADDR_W = (void *) IO_ADDR_W;\n> }\n>\n> @@ -1231,12 +1270,24 @@ static void at91_nand_hwcontrol(struct mtd_info *mtd,\n> writeb(cmd, this->IO_ADDR_W);\n> }\n>\n> -#ifdef CONFIG_SYS_NAND_READY_PIN\n> static int at91_nand_ready(struct mtd_info *mtd)\n> {\n> +#ifdef CONFIG_DM_GPIO\n> + struct nand_chip *nand_chip = mtd_to_nand(mtd);\n> + struct atmel_nand_host *host = nand_get_controller_data(nand_chip);\n> +\n> + if (dm_gpio_is_valid(&host->board.rdy_pin))\n> + return dm_gpio_get_value(&host->board.rdy_pin);\n> +\n> + return 0;\n> +#else\n> +#ifdef CONFIG_SYS_NAND_READY_PIN\n> return gpio_get_value(CONFIG_SYS_NAND_READY_PIN);\n> -}\n> +#else\n> + return 0;\n> #endif\n> +#endif\n> +}\n>\n> #ifdef CONFIG_SPL_BUILD\n> /* The following code is for SPL */\n> @@ -1419,6 +1470,10 @@ int at91_nand_wait_ready(struct mtd_info *mtd)\n> int board_nand_init(struct nand_chip *nand)\n> {\n> int ret = 0;\n> + struct atmel_nand_host *host;\n> +\n> + host = &nand_host;\n> + nand_set_controller_data(nand, host);\n>\n> nand->ecc.mode = NAND_ECC_SOFT;\n> #ifdef CONFIG_SYS_NAND_DBW_16\n> @@ -1486,9 +1541,45 @@ int atmel_nand_chip_init(int devnum, ulong base_addr)\n> int ret;\n> struct nand_chip *nand = &nand_chip[devnum];\n> struct mtd_info *mtd = nand_to_mtd(nand);\n> + struct atmel_nand_host *host;\n> +\n> + host = &nand_host;\n> + nand_set_controller_data(nand, host);\n> +\n> +#if CONFIG_IS_ENABLED(OF_CONTROL)\n> + struct atmel_nand_data *board;\n> + int node;\n> +\n> + board = &host->board;\n> + node = fdtdec_next_compatible(gd->fdt_blob, 0,\n> + COMPAT_ATMEL_NAND);\n\nCan you instead use a proper driver? Then you can avoid adding a\nCOMPAT string. I think you have a nand uclass.\n\n> + if (node < 0)\n> + return -1;\n\nThis is -EPERM. Perhaps use -EINVAL ?\n\n> +\n> + nand->IO_ADDR_R = (void __iomem *)\n> + fdtdec_get_addr_size_auto_noparent(gd->fdt_blob,\n> + node, \"reg\",\n> + 0, NULL, true);\n> + nand->IO_ADDR_W = nand->IO_ADDR_R;\n>\n> - nand->IO_ADDR_R = nand->IO_ADDR_W = (void __iomem *)base_addr;\n> + board->ale = fdtdec_get_uint(gd->fdt_blob, node,\n> + \"atmel,nand-addr-offset\", 21);\n>\n> + board->cle = fdtdec_get_uint(gd->fdt_blob, node,\n> + \"atmel,nand-cmd-offset\", 22);\n> +\n> + gpio_request_by_name_nodev(gd->fdt_blob, node, \"gpios\", 0,\n> + &board->rdy_pin, GPIOD_IS_IN);\n> +\n> + gpio_request_by_name_nodev(gd->fdt_blob, node, \"gpios\", 1,\n> + &board->enable_pin, GPIOD_IS_OUT);\n> +\n> + gpio_request_by_name_nodev(gd->fdt_blob, node, \"gpios\", 2,\n> + &board->det_pin, GPIOD_IS_IN);\n> +#else\n> + nand->IO_ADDR_R = (void __iomem *)base_addr;\n> + nand->IO_ADDR_W = (void __iomem *)base_addr;\n> +#endif\n> #ifdef CONFIG_NAND_ECC_BCH\n> nand->ecc.mode = NAND_ECC_SOFT_BCH;\n> #else\n> @@ -1498,9 +1589,9 @@ int atmel_nand_chip_init(int devnum, ulong base_addr)\n> nand->options = NAND_BUSWIDTH_16;\n> #endif\n> nand->cmd_ctrl = at91_nand_hwcontrol;\n> -#ifdef CONFIG_SYS_NAND_READY_PIN\n> +\n> nand->dev_ready = at91_nand_ready;\n> -#endif\n> +\n> nand->chip_delay = 75;\n> #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT\n> nand->bbt_options |= NAND_BBT_USE_FLASH;\n> diff --git a/include/fdtdec.h b/include/fdtdec.h\n> index d074478f14..eacf251d63 100644\n> --- a/include/fdtdec.h\n> +++ b/include/fdtdec.h\n> @@ -155,6 +155,7 @@ enum fdt_compat_id {\n> COMPAT_INTEL_BAYTRAIL_FSP_MDP, /* Intel FSP memory-down params */\n> COMPAT_INTEL_IVYBRIDGE_FSP, /* Intel Ivy Bridge FSP */\n> COMPAT_SUNXI_NAND, /* SUNXI NAND controller */\n> + COMPAT_ATMEL_NAND, /* Atmel NAND controller */\n>\n> COMPAT_COUNT,\n> };\n> diff --git a/lib/fdtdec.c b/lib/fdtdec.c\n> index 81f47ef2c7..a89e8272e1 100644\n> --- a/lib/fdtdec.c\n> +++ b/lib/fdtdec.c\n> @@ -66,6 +66,7 @@ static const char * const compat_names[COMPAT_COUNT] = {\n> COMPAT(INTEL_BAYTRAIL_FSP_MDP, \"intel,baytrail-fsp-mdp\"),\n> COMPAT(INTEL_IVYBRIDGE_FSP, \"intel,ivybridge-fsp\"),\n> COMPAT(COMPAT_SUNXI_NAND, \"allwinner,sun4i-a10-nand\"),\n> + COMPAT(COMPAT_ATMEL_NAND, \"atmel,at91rm9200-nand\"),\n> };\n>\n> const char *fdtdec_get_compatible(enum fdt_compat_id id)\n> --\n> 2.11.0\n>\n\nRegards,\nSimon","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Received":["from theia.denx.de (theia.denx.de [85.214.87.163])\n\tby ozlabs.org (Postfix) with ESMTP id 3vJ8Qn4P78z9s7c\n\tfor ;\n\tWed, 8 Feb 2017 16:09:53 +1100 (AEDT)","from localhost (localhost [127.0.0.1])\n\tby theia.denx.de (Postfix) with ESMTP id 56F4B4B5B7;\n\tWed, 8 Feb 2017 06:09:51 +0100 (CET)","from theia.denx.de ([127.0.0.1])\n\tby localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id MuWj-3DcqHtG; Wed, 8 Feb 2017 06:09:51 +0100 (CET)","from theia.denx.de (localhost [127.0.0.1])\n\tby theia.denx.de (Postfix) with ESMTP id CF5724B309;\n\tWed, 8 Feb 2017 06:09:50 +0100 (CET)","from localhost (localhost [127.0.0.1])\n\tby theia.denx.de (Postfix) with ESMTP id 430554B309\n\tfor ; Wed, 8 Feb 2017 06:09:45 +0100 (CET)","from theia.denx.de ([127.0.0.1])\n\tby localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id IiojrQoDBaiW for ;\n\tWed, 8 Feb 2017 06:09:45 +0100 (CET)","from mail-vk0-f42.google.com (mail-vk0-f42.google.com\n\t[209.85.213.42]) by theia.denx.de (Postfix) with ESMTPS id AA01A4B270\n\tfor ; Wed, 8 Feb 2017 06:09:41 +0100 (CET)","by mail-vk0-f42.google.com with SMTP id k127so93312663vke.0\n\tfor ; Tue, 07 Feb 2017 21:09:40 -0800 (PST)","by 10.176.16.4 with HTTP; Tue, 7 Feb 2017 21:09:18 -0800 (PST)"],"Authentication-Results":"ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"HjD3oDM9\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"bJPiGy4u\"; dkim-atps=neutral","X-policyd-weight":"NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5\n\tNOT_IN_BL_NJABL=-1.5 (only DNSBL check requested)","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=20161025; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=PFpBIAJ2a4dyVFy7s2O9wxV+vYNb3iB/BhkSDOyLyRY=;\n\tb=HjD3oDM9IOx58Z7YAawyqF1g3BQ5yG5TixYPbt91fFGDcz4dEWnLP3/SnFVriHhmFi\n\tNLUqa6kwdaL0jkMEc6cSM0c1FEtr4K0NF0Hl/u9IKkGAiGwqMm3CooKxjFmC4eHvtFAX\n\tfhHi4nPd4Kg2hW+GIi7L2VD0p4Zd5aDPNF9r/ymg9ek+zcXQySzrMzSFbSzdo36cll6d\n\tEoRKEY1cKL6+lH5lH8Y/iTuAlvAq2ZreQQfNw8h2EY7VVJviBftvyw3iuE0rFtKOU4fb\n\tLVzldo3ql+T4cK4yccYtViqYVTbsLoWaE2RZ1K95QAOku9AWKTp+2pwds0bT6bsDtYVB\n\tq9CA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=PFpBIAJ2a4dyVFy7s2O9wxV+vYNb3iB/BhkSDOyLyRY=;\n\tb=bJPiGy4uYFt81nRcWVfz4/n/ieIOujxde2XiUwHq6wGsPBYCXG6gW6pWNOQNfmNQqd\n\t8IbWE2Uqp9p5VYJBgTNSny1br+jPdFhZ1PJpyGeZWjsPoUcuVFlkD9SparUH3ePCygOl\n\tklAEUz6/wWaz3r3MepZinAuvzxNUPy05szG2I="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=PFpBIAJ2a4dyVFy7s2O9wxV+vYNb3iB/BhkSDOyLyRY=;\n\tb=V4F/rRGaSGH1bmA0fH29kfvVnqTVPkPK5x1eEc7hLRwrR/IHpuVEGlPmujVQlpAaIY\n\tqymHHIKYmBw/YeTDPVfZF8KJDZpG+EloP5/gFrQ3ecjLBg3Te7jfru4VIgofCIjybVNa\n\t15rH0ljyOzX+zq252luUiqTg2/cSWGMc344iu2LCXf1xKyK4b0NzJeKWzteLB/kSH2fD\n\tiXjusYMAjLnrevJ8VlwNsjpxC+pTPbgbarB88865v9454UlA7z49nVu1rWWD/Y0lOypn\n\tDN8Hh65Dtfddsa8v48lQNT5EQ8KBnXUJyAQKTZgNjgC0qLQIDiNC1LnVNww38v3gtBG1\n\tmF/g==","X-Gm-Message-State":"AMke39lkZyivydy14n1N3BYTedJFBMrNAuH5MNaLHFW8gLaM4opugLkAFRj6BIilN1FEVm2DuRLQKhhI7iMbvt9T","X-Received":"by 10.31.135.137 with SMTP id j131mr7656072vkd.77.1486530579268; \n\tTue, 07 Feb 2017 21:09:39 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<20170204023258.10080-1-wenyou.yang@atmel.com>","References":"<20170204023258.10080-1-wenyou.yang@atmel.com>","From":"Simon Glass ","Date":"Tue, 7 Feb 2017 22:09:18 -0700","X-Google-Sender-Auth":"OXWMkcmpj6Hw5guGGbQLx1c4irc","Message-ID":"","To":"Wenyou Yang ","Cc":"Scott Wood ,\n\tU-Boot Mailing List ","Subject":"Re: [U-Boot] [PATCH] nand: atmel: add drvice tree support and dm\n\tgpio APIs","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.15","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1579036,"web_url":"http://patchwork.ozlabs.org/comment/1579036/","msgid":"","list_archive_url":null,"date":"2017-02-09T05:06:56","subject":"Re: [U-Boot] [PATCH] nand: atmel: add drvice tree support and dm\n\tgpio APIs","submitter":{"id":69532,"url":"http://patchwork.ozlabs.org/api/people/69532/","name":"Wenyou Yang","email":"Wenyou.Yang@microchip.com"},"content":"Hi Simon,\r\n\r\n> -----Original Message-----\r\n> From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass\r\n> Sent: 2017年2月8日 13:09\r\n> To: Wenyou Yang - A41535 \r\n> Cc: U-Boot Mailing List ; Andreas Bießmann\r\n> ; Scott Wood ; Wenyou\r\n> Yang - A41535 \r\n> Subject: Re: [PATCH] nand: atmel: add drvice tree support and dm gpio APIs\r\n> \r\n> Hi Wenyou,\r\n> \r\n> On 3 February 2017 at 18:32, Wenyou Yang wrote:\r\n> >\r\n> > In order to use the driver model gpio APIs, add the device tree\r\n> > support.\r\n> >\r\n> > Signed-off-by: Wenyou Yang \r\n> > ---\r\n> >\r\n> > drivers/mtd/nand/atmel_nand.c | 143\r\n> ++++++++++++++++++++++++++++++++++--------\r\n> > include/fdtdec.h | 1 +\r\n> > lib/fdtdec.c | 1 +\r\n> > 3 files changed, 119 insertions(+), 26 deletions(-)\r\n> >\r\n> > diff --git a/drivers/mtd/nand/atmel_nand.c\r\n> > b/drivers/mtd/nand/atmel_nand.c index 8669432deb..87ff5c2eb4 100644\r\n> > --- a/drivers/mtd/nand/atmel_nand.c\r\n> > +++ b/drivers/mtd/nand/atmel_nand.c\r\n> > @@ -14,29 +14,34 @@\r\n> > #include \r\n> > #include \r\n> > #include \r\n> > +#ifdef CONFIG_DM_GPIO\r\n> > +#include \r\n> > +#endif\r\n> >\r\n> > +#include \r\n> > #include \r\n> > #include \r\n> > #include \r\n> > #include \r\n> >\r\n> > -#ifdef CONFIG_ATMEL_NAND_HWECC\r\n> > -\r\n> > -/* Register access macros */\r\n> > -#define ecc_readl(add, reg) \\\r\n> > - readl(add + ATMEL_ECC_##reg)\r\n> > -#define ecc_writel(add, reg, value) \\\r\n> > - writel((value), add + ATMEL_ECC_##reg)\r\n> \r\n> What is happening here? Seems like it should be a separate patch.\r\n> \r\n> > +DECLARE_GLOBAL_DATA_PTR;\r\n> > +\r\n> > +struct atmel_nand_data {\r\n> > + struct gpio_desc enable_pin; /* chip enable */\r\n> > + struct gpio_desc det_pin; /* card detect */\r\n> > + struct gpio_desc rdy_pin; /* ready/busy */\r\n> > + u8 ale; /* address line number connected to ALE */\r\n> > + u8 cle; /* address line number connected to CLE */\r\n> > + u8 bus_width_16; /* buswidth is 16 bit */\r\n> > + u8 ecc_mode; /* ecc mode */\r\n> > + u8 on_flash_bbt; /* bbt on flash */\r\n> > +};\r\n> >\r\n> > -#include \"atmel_nand_ecc.h\" /* Hardware ECC registers */\r\n> > +struct atmel_nand_host {\r\n> > + void __iomem *io_base;\r\n> > + struct atmel_nand_data board;\r\n> >\r\n> > #ifdef CONFIG_ATMEL_NAND_HW_PMECC\r\n> > -\r\n> > -#ifdef CONFIG_SPL_BUILD\r\n> > -#undef CONFIG_SYS_NAND_ONFI_DETECTION -#endif\r\n> > -\r\n> > -struct atmel_nand_host {\r\n> > struct pmecc_regs __iomem *pmecc;\r\n> > struct pmecc_errloc_regs __iomem *pmerrloc;\r\n> > void __iomem *pmecc_rom_base;\r\n> > @@ -63,9 +68,26 @@ struct atmel_nand_host {\r\n> > int *pmecc_mu;\r\n> > int *pmecc_dmu;\r\n> > int *pmecc_delta;\r\n> > +#endif\r\n> > };\r\n> >\r\n> > -static struct atmel_nand_host pmecc_host;\r\n> > +static struct atmel_nand_host nand_host; #ifdef\r\n> > +CONFIG_ATMEL_NAND_HWECC\r\n> > +\r\n> > +/* Register access macros */\r\n> > +#define ecc_readl(add, reg) \\\r\n> > + readl(add + ATMEL_ECC_##reg)\r\n> > +#define ecc_writel(add, reg, value) \\\r\n> > + writel((value), add + ATMEL_ECC_##reg)\r\n> > +\r\n> > +#include \"atmel_nand_ecc.h\" /* Hardware ECC registers */\r\n> > +\r\n> > +#ifdef CONFIG_ATMEL_NAND_HW_PMECC\r\n> > +\r\n> > +#ifdef CONFIG_SPL_BUILD\r\n> > +#undef CONFIG_SYS_NAND_ONFI_DETECTION #endif\r\n> > +\r\n> > static struct nand_ecclayout atmel_pmecc_oobinfo;\r\n> >\r\n> > /*\r\n> > @@ -805,12 +827,9 @@ static uint16_t *create_lookup_table(int\r\n> > sector_size) static int atmel_pmecc_nand_init_params(struct nand_chip *nand,\r\n> > struct mtd_info *mtd)\r\n> > {\r\n> > - struct atmel_nand_host *host;\r\n> > + struct atmel_nand_host *host = nand_get_controller_data(nand);\r\n> > int cap, sector_size;\r\n> >\r\n> > - host = &pmecc_host;\r\n> > - nand_set_controller_data(nand, host);\r\n> > -\r\n> > nand->ecc.mode = NAND_ECC_HW;\r\n> > nand->ecc.calculate = NULL;\r\n> > nand->ecc.correct = NULL;\r\n> > @@ -1207,12 +1226,24 @@ int atmel_hwecc_nand_init_param(struct\r\n> > nand_chip *nand, struct mtd_info *mtd) #endif /*\r\n> > CONFIG_ATMEL_NAND_HWECC */\r\n> >\r\n> > static void at91_nand_hwcontrol(struct mtd_info *mtd,\r\n> > - int cmd, unsigned int ctrl)\r\n> > + int cmd, unsigned int ctrl)\r\n> > {\r\n> > struct nand_chip *this = mtd_to_nand(mtd);\r\n> > +#if CONFIG_IS_ENABLED(OF_CONTROL)\r\n> > + struct atmel_nand_host *host = nand_get_controller_data(this);\r\n> > +#endif\r\n> >\r\n> > if (ctrl & NAND_CTRL_CHANGE) {\r\n> > - ulong IO_ADDR_W = (ulong) this->IO_ADDR_W;\r\n> > + ulong IO_ADDR_W = (ulong)this->IO_ADDR_W; #if\r\n> > +CONFIG_IS_ENABLED(OF_CONTROL)\r\n> > + IO_ADDR_W &= ~((1 << host->board.ale)\r\n> > + | (1 << host->board.cle));\r\n> > +\r\n> > + if (ctrl & NAND_CLE)\r\n> > + IO_ADDR_W |= (1 << host->board.cle);\r\n> > + if (ctrl & NAND_ALE)\r\n> > + IO_ADDR_W |= (1 << host->board.ale); #else\r\n> > IO_ADDR_W &= ~(CONFIG_SYS_NAND_MASK_ALE\r\n> > | CONFIG_SYS_NAND_MASK_CLE);\r\n> \r\n> You could adjust it so that CONFIG_SYS_NAND_MASK_CLE is assigned to\r\n> host->board.cle in the init function, and then avoid this #ifdef here.\r\n\r\nAccepted\r\n\r\n> \r\n> >\r\n> > @@ -1220,10 +1251,18 @@ static void at91_nand_hwcontrol(struct mtd_info\r\n> *mtd,\r\n> > IO_ADDR_W |= CONFIG_SYS_NAND_MASK_CLE;\r\n> > if (ctrl & NAND_ALE)\r\n> > IO_ADDR_W |= CONFIG_SYS_NAND_MASK_ALE;\r\n> > +#endif\r\n> >\r\n> > +#ifdef CONFIG_DM_GPIO\r\n> > + if (dm_gpio_is_valid(&host->board.enable_pin))\r\n> > + dm_gpio_set_value(&host->board.enable_pin,\r\n> > + !(ctrl & NAND_NCE)); #else\r\n> > #ifdef CONFIG_SYS_NAND_ENABLE_PIN\r\n> > gpio_set_value(CONFIG_SYS_NAND_ENABLE_PIN, !(ctrl &\r\n> > NAND_NCE)); #endif\r\n> > +#endif\r\n> > +\r\n> > this->IO_ADDR_W = (void *) IO_ADDR_W;\r\n> > }\r\n> >\r\n> > @@ -1231,12 +1270,24 @@ static void at91_nand_hwcontrol(struct mtd_info\r\n> *mtd,\r\n> > writeb(cmd, this->IO_ADDR_W); }\r\n> >\r\n> > -#ifdef CONFIG_SYS_NAND_READY_PIN\r\n> > static int at91_nand_ready(struct mtd_info *mtd) {\r\n> > +#ifdef CONFIG_DM_GPIO\r\n> > + struct nand_chip *nand_chip = mtd_to_nand(mtd);\r\n> > + struct atmel_nand_host *host =\r\n> > +nand_get_controller_data(nand_chip);\r\n> > +\r\n> > + if (dm_gpio_is_valid(&host->board.rdy_pin))\r\n> > + return dm_gpio_get_value(&host->board.rdy_pin);\r\n> > +\r\n> > + return 0;\r\n> > +#else\r\n> > +#ifdef CONFIG_SYS_NAND_READY_PIN\r\n> > return gpio_get_value(CONFIG_SYS_NAND_READY_PIN);\r\n> > -}\r\n> > +#else\r\n> > + return 0;\r\n> > #endif\r\n> > +#endif\r\n> > +}\r\n> >\r\n> > #ifdef CONFIG_SPL_BUILD\r\n> > /* The following code is for SPL */\r\n> > @@ -1419,6 +1470,10 @@ int at91_nand_wait_ready(struct mtd_info *mtd)\r\n> > int board_nand_init(struct nand_chip *nand) {\r\n> > int ret = 0;\r\n> > + struct atmel_nand_host *host;\r\n> > +\r\n> > + host = &nand_host;\r\n> > + nand_set_controller_data(nand, host);\r\n> >\r\n> > nand->ecc.mode = NAND_ECC_SOFT; #ifdef\r\n> CONFIG_SYS_NAND_DBW_16\r\n> > @@ -1486,9 +1541,45 @@ int atmel_nand_chip_init(int devnum, ulong\r\n> base_addr)\r\n> > int ret;\r\n> > struct nand_chip *nand = &nand_chip[devnum];\r\n> > struct mtd_info *mtd = nand_to_mtd(nand);\r\n> > + struct atmel_nand_host *host;\r\n> > +\r\n> > + host = &nand_host;\r\n> > + nand_set_controller_data(nand, host);\r\n> > +\r\n> > +#if CONFIG_IS_ENABLED(OF_CONTROL)\r\n> > + struct atmel_nand_data *board;\r\n> > + int node;\r\n> > +\r\n> > + board = &host->board;\r\n> > + node = fdtdec_next_compatible(gd->fdt_blob, 0,\r\n> > + COMPAT_ATMEL_NAND);\r\n> \r\n> Can you instead use a proper driver? Then you can avoid adding a COMPAT\r\n> string. I think you have a nand uclass.\r\n\r\nYou mean the patch:\r\nhttp://lists.denx.de/pipermail/u-boot/2017-January/279904.html\r\n\r\nIf so, that would be very nice. I will convert the nand driver to support driver model, and device tree as well.\r\n\r\n> \r\n> > + if (node < 0)\r\n> > + return -1;\r\n> \r\n> This is -EPERM. Perhaps use -EINVAL ?\r\n> \r\n> > +\r\n> > + nand->IO_ADDR_R = (void __iomem *)\r\n> > + fdtdec_get_addr_size_auto_noparent(gd->fdt_blob,\r\n> > + node, \"reg\",\r\n> > + 0, NULL, true);\r\n> > + nand->IO_ADDR_W = nand->IO_ADDR_R;\r\n> >\r\n> > - nand->IO_ADDR_R = nand->IO_ADDR_W = (void __iomem *)base_addr;\r\n> > + board->ale = fdtdec_get_uint(gd->fdt_blob, node,\r\n> > + \"atmel,nand-addr-offset\", 21);\r\n> >\r\n> > + board->cle = fdtdec_get_uint(gd->fdt_blob, node,\r\n> > + \"atmel,nand-cmd-offset\", 22);\r\n> > +\r\n> > + gpio_request_by_name_nodev(gd->fdt_blob, node, \"gpios\", 0,\r\n> > + &board->rdy_pin, GPIOD_IS_IN);\r\n> > +\r\n> > + gpio_request_by_name_nodev(gd->fdt_blob, node, \"gpios\", 1,\r\n> > + &board->enable_pin, GPIOD_IS_OUT);\r\n> > +\r\n> > + gpio_request_by_name_nodev(gd->fdt_blob, node, \"gpios\", 2,\r\n> > + &board->det_pin, GPIOD_IS_IN);\r\n> > +#else\r\n> > + nand->IO_ADDR_R = (void __iomem *)base_addr;\r\n> > + nand->IO_ADDR_W = (void __iomem *)base_addr; #endif\r\n> > #ifdef CONFIG_NAND_ECC_BCH\r\n> > nand->ecc.mode = NAND_ECC_SOFT_BCH; #else @@ -1498,9 +1589,9\r\n> > @@ int atmel_nand_chip_init(int devnum, ulong base_addr)\r\n> > nand->options = NAND_BUSWIDTH_16; #endif\r\n> > nand->cmd_ctrl = at91_nand_hwcontrol; -#ifdef\r\n> > CONFIG_SYS_NAND_READY_PIN\r\n> > +\r\n> > nand->dev_ready = at91_nand_ready; -#endif\r\n> > +\r\n> > nand->chip_delay = 75;\r\n> > #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT\r\n> > nand->bbt_options |= NAND_BBT_USE_FLASH; diff --git\r\n> > a/include/fdtdec.h b/include/fdtdec.h index d074478f14..eacf251d63\r\n> > 100644\r\n> > --- a/include/fdtdec.h\r\n> > +++ b/include/fdtdec.h\r\n> > @@ -155,6 +155,7 @@ enum fdt_compat_id {\r\n> > COMPAT_INTEL_BAYTRAIL_FSP_MDP, /* Intel FSP memory-down\r\n> params */\r\n> > COMPAT_INTEL_IVYBRIDGE_FSP, /* Intel Ivy Bridge FSP */\r\n> > COMPAT_SUNXI_NAND, /* SUNXI NAND controller */\r\n> > + COMPAT_ATMEL_NAND, /* Atmel NAND controller */\r\n> >\r\n> > COMPAT_COUNT,\r\n> > };\r\n> > diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 81f47ef2c7..a89e8272e1\r\n> > 100644\r\n> > --- a/lib/fdtdec.c\r\n> > +++ b/lib/fdtdec.c\r\n> > @@ -66,6 +66,7 @@ static const char * const\r\n> compat_names[COMPAT_COUNT] = {\r\n> > COMPAT(INTEL_BAYTRAIL_FSP_MDP, \"intel,baytrail-fsp-mdp\"),\r\n> > COMPAT(INTEL_IVYBRIDGE_FSP, \"intel,ivybridge-fsp\"),\r\n> > COMPAT(COMPAT_SUNXI_NAND, \"allwinner,sun4i-a10-nand\"),\r\n> > + COMPAT(COMPAT_ATMEL_NAND, \"atmel,at91rm9200-nand\"),\r\n> > };\r\n> >\r\n> > const char *fdtdec_get_compatible(enum fdt_compat_id id)\r\n> > --\r\n> > 2.11.0\r\n> >\r\n> \r\n> Regards,\r\n> Simon\r\n\r\nThank you\r\n\r\n\r\nBest Regards,\r\nWenyou Yang","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Received":["from theia.denx.de (theia.denx.de [85.214.87.163])\n\tby ozlabs.org (Postfix) with ESMTP id 3vJmKG51lnz9s73\n\tfor ;\n\tThu, 9 Feb 2017 16:07:13 +1100 (AEDT)","from localhost (localhost [127.0.0.1])\n\tby theia.denx.de (Postfix) with ESMTP id BB1854BDBD;\n\tThu, 9 Feb 2017 06:07:09 +0100 (CET)","from theia.denx.de ([127.0.0.1])\n\tby localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id yZnje_RTTmoE; Thu, 9 Feb 2017 06:07:09 +0100 (CET)","from theia.denx.de (localhost [127.0.0.1])\n\tby theia.denx.de (Postfix) with ESMTP id 2C7D74B8AA;\n\tThu, 9 Feb 2017 06:07:09 +0100 (CET)","from localhost (localhost [127.0.0.1])\n\tby theia.denx.de (Postfix) with ESMTP id 6D3E64B8AA\n\tfor ; Thu, 9 Feb 2017 06:07:04 +0100 (CET)","from theia.denx.de ([127.0.0.1])\n\tby localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id T-_EM2sSJB_Z for ;\n\tThu, 9 Feb 2017 06:07:04 +0100 (CET)","from email.microchip.com (exsmtp01.microchip.com [198.175.253.37])\n\tby theia.denx.de (Postfix) with ESMTPS id 795514B891\n\tfor ; Thu, 9 Feb 2017 06:06:59 +0100 (CET)","from CHN-SV-EXMX07.mchp-main.com ([fe80::3807:afd9:985d:6e4b]) by\n\tCHN-SV-EXCH01.mchp-main.com ([fe80::9840:ffdf:ec5:1335%29]) with\n\tmapi id 14.03.0181.006; Wed, 8 Feb 2017 22:06:57 -0700"],"X-policyd-weight":"NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5\n\tNOT_IN_BL_NJABL=-1.5 (only DNSBL check requested)","From":"","To":"","Thread-Topic":"[PATCH] nand: atmel: add drvice tree support and dm gpio APIs","Thread-Index":"AQHSfo+lkXr+yYEjx0y243kbEQkaraFfCzkAgAEa6DA=","Date":"Thu, 9 Feb 2017 05:06:56 +0000","Message-ID":"","References":"<20170204023258.10080-1-wenyou.yang@atmel.com>\n\t","In-Reply-To":"","Accept-Language":"zh-CN, en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","x-originating-ip":"[10.10.76.4]","MIME-Version":"1.0","Cc":"scottwood@freescale.com, u-boot@lists.denx.de","Subject":"Re: [U-Boot] [PATCH] nand: atmel: add drvice tree support and dm\n\tgpio APIs","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.15","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}}]