linux-kernel - RE: [RFC 13/47] mtd: nand: stm_nand

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 26 Mar 2014 09:18:59 +0000
From:	"Gupta, Pekon" <pekon@...com>
To:	Lee Jones <lee.jones@...aro.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	"kernel@...inux.com" <kernel@...inux.com>,
	"computersforpeace@...il.com" <computersforpeace@...il.com>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"dwmw2@...radead.org" <dwmw2@...radead.org>,
	"angus.clark@...com" <angus.clark@...com>
Subject: RE: [RFC 13/47] mtd: nand: stm_nand_bch: provide Device Tree support

Hi Lee,

>From: Lee Jones [mailto:lee.jones@...aro.org]
>
>Fetch platform specific data from Device Tree. Any functions which
>are useful to other STM NAND Controllers have been separated into a
>separate file so they can be easily referenced by them as they
>appear.
>
>Signed-off-by: Lee Jones <lee.jones@...aro.org>
>---
[...]

>--- a/drivers/mtd/nand/stm_nand_bch.c
>+++ b/drivers/mtd/nand/stm_nand_bch.c
>@@ -16,6 +16,7 @@
> #include <linux/module.h>
> #include <linux/delay.h>
> #include <linux/io.h>
>+#include <linux/of.h>
> #include <linux/clk.h>
> #include <linux/interrupt.h>
> #include <linux/device.h>
>@@ -23,8 +24,10 @@
> #include <linux/completion.h>
> #include <linux/mtd/nand.h>
> #include <linux/mtd/stm_nand.h>
>+#include <linux/mtd/partitions.h>
>
> #include "stm_nand_regs.h"
>+#include "stm_nand_dt.h"
>
> /* Bad Block Table (BBT) */
> struct nandi_bbt_info {
>@@ -365,9 +368,51 @@ nandi_init_resources(struct platform_device *pdev)
> 	return nandi;
> }
>
>+#ifdef CONFIG_OF
>+static void *stm_bch_dt_get_pdata(struct platform_device *pdev)
>+{
>+	struct device_node *np = pdev->dev.of_node;
>+	struct stm_plat_nand_bch_data *pdata;
>+	const char *ecc_config;
>+	int ret;
>+
>+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>+	if (!pdata)
>+		return ERR_PTR(-ENOMEM);
>+
>+	of_property_read_string(np, "st,bch-ecc-mode", &ecc_config);

Can you use any of the existing generic NAND bindings like combination of
"nand-ecc-mode" and "nand-ecc-strength" ?
Example: nand-ecc-mode = "bch" & nand-ecc-strength=18
Some of generic bindings are recently added
	commit 8dd49165ef5f46b5ad9ba296c559ccff315f9421  (currently in l2-mtd tree)
	mtd: nand: Add a devicetree binding for ECC strength and ECC step size

It's good to use generic bindings as much as you can, and you don't need
any approvals from DT Maintainers too. Though you may need to add
new values for "nand-ecc-mode" like {auto, hw-ham, hw-bch}
 

>+	if (!strcmp("noecc", ecc_config))
>+		pdata->bch_ecc_cfg = BCH_NO_ECC;
>+	else if (!strcmp("18bit", ecc_config))
>+		pdata->bch_ecc_cfg = BCH_18BIT_ECC;
>+	else if (!strcmp("30bit", ecc_config))
>+		pdata->bch_ecc_cfg = BCH_30BIT_ECC;
>+	else
>+		pdata->bch_ecc_cfg = BCH_ECC_AUTO;
>+
>+	ret = stm_of_get_nand_banks(&pdev->dev, np, &pdata->bank);
>+	if (ret < 0)
>+		return ERR_PTR(ret);
>+
>+	pdata->flashss = of_property_read_bool(np, "st,nand-flashss");
>+
>+	of_property_read_u32(np, "st,bch-bitflip-threshold",
>+			     &pdata->bch_bitflip_threshold);
>+
mtd->bitflip_threshold is by default set to ecc.strength (unless a driver initializes it).
And then can be re-configured for each MTD partition separately 
	/sys/class/mtd/mtdX/bitflip_threshold
	Refer: $kernel/Documentation/ABI/testing/sysfs-class-mtd
So, I don't think this is a HW parameter, and so should not be passed from DT.


>+	return pdata;
>+}
>+#else

[...]

>+struct device_node *stm_of_get_partitions_node(struct device_node *np,
>+					       int bank_nr)
>+{
>+	struct device_node *banksnp, *banknp, *partsnp = NULL;
>+	char name[10];
>+
>+	banksnp = of_parse_phandle(np, "st,nand-banks", 0);
>+	if (!banksnp)
>+		return NULL;
>+
>+	sprintf(name, "bank%d", bank_nr);
>+	banknp = of_get_child_by_name(banksnp, name);
>+	if (banknp)
>+		return NULL;
>+
>+	partsnp = of_get_child_by_name(banknp, "partitions");
>+	of_node_put(banknp);
>+
Sorry, I'm bit confused here .. I think you don't need to find children of
Your bank node. This should already taken care in default parser
drivers/mtd/ofpart.c : parse_ofpart_partitions()
And all you need to pass is 'of_node' of bank (device).
Is my understanding correct ?

>+	return partsnp;
>+}
>+EXPORT_SYMBOL(stm_of_get_partitions_node);
>+


with regards, pekon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/