| Message ID | 1395735604-26706-14-git-send-email-lee.jones@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Lee, >From: Lee Jones [mailto:lee.jones@linaro.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@linaro.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> >From: Lee Jones [mailto:lee.jones@linaro.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@linaro.org> [...] > >+#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} Sure, I can make use of the generic bindings already offered here. > >+ 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. I think the bit-flip threshold is/can be chip specific, and as we know which chip we're likely to be booting on, we can specify this via the hardware description. Thus, I think it's perfectly viable for an option to pass through DT to exist. > >+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 ? We have 3 options here, you _can_ use parse_ofpart_partitions() if your partition information conforms to its schema, but said schema does not support banks and/or other information that we choose to place within the bank node. The second option is to register a parser. My personal view is that registering a parser is using the framework for 'using the framework's' sake i.e. doesn't actually achieve anything special. We've chosen the third option, which is to parse and extract the information ourselves - which is actually fairly trivial, and pass the required partition data in through the mtd_device_parse_register() call - which is where the information would be parsed in the case of the first two options.
Hi Lee, >From: Lee Jones [mailto:lee.jones@linaro.org] >> >From: Lee Jones [mailto:lee.jones@linaro.org] [...] >> >+ 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. > >I think the bit-flip threshold is/can be chip specific, and as we know >which chip we're likely to be booting on, we can specify this via the >hardware description. Thus, I think it's perfectly viable for an >option to pass through DT to exist. > I don't think that’s the correct interpretation of bitflip_threshold. (1) bitflip_threshold is dependent on ecc.strength (ECC scheme) of your driver. MTD layers uses bitflip_threshold to warn above layers that number of correctable bit-flips have reached a dangerous level beyond which driver's ECC scheme may not be able to correct them. So above layers should start taking additional corrective action like scrubbing. @@drivers/mtd/mtdcore.c: mtd_read() return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0; (2) Also, user-space may control it based on how your device ages on field. A fresh silicon may not show too many bitflips. But as device ages the probability of simultaneous bitflips will increase. So user-space may lower the bitflip_threshold to avoid accumulation of bitflips in a single page. Thus, bitflip_threshold should not be passed via DT. It's neither a hardware parameter, nor it’s a static constant. > >> >+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 ? > >We have 3 options here, you _can_ use parse_ofpart_partitions() if >your partition information conforms to its schema, but said schema >does not support banks and/or other information that we choose to >place within the bank node. The second option is to register a >parser. My personal view is that registering a parser is using the >framework for 'using the framework's' sake i.e. doesn't actually >achieve anything special. We've chosen the third option, which is to >parse and extract the information ourselves - which is actually fairly >trivial, and pass the required partition data in through the >mtd_device_parse_register() call - which is where the information >would be parsed in the case of the first two options. > Do you really want to have *custom* partition format ? If your previous code was non-DT (platform file based), then I think It's good point to move to unified generic partition format which others are following, as you make your driver DT compliant, and in mainline. I understand you primary objective would be to get ST driver work out of mainline asap, but if you upstream too many custom stuff you are only adding maintenance burden for your code. This is where most of my comments originate. However, I leave it to Brian to decide, if he is okay with these. with regards, pekon
> >> >+ 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. > > > >I think the bit-flip threshold is/can be chip specific, and as we know > >which chip we're likely to be booting on, we can specify this via the > >hardware description. Thus, I think it's perfectly viable for an > >option to pass through DT to exist. > > > I don't think that’s the correct interpretation of bitflip_threshold. > > (1) bitflip_threshold is dependent on ecc.strength (ECC scheme) of your driver. > MTD layers uses bitflip_threshold to warn above layers that number of > correctable bit-flips have reached a dangerous level beyond which driver's > ECC scheme may not be able to correct them. So above layers should start > taking additional corrective action like scrubbing. > @@drivers/mtd/mtdcore.c: mtd_read() > return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0; > > (2) Also, user-space may control it based on how your device ages on field. > A fresh silicon may not show too many bitflips. But as device ages the > probability of simultaneous bitflips will increase. So user-space may lower > the bitflip_threshold to avoid accumulation of bitflips in a single page. > > Thus, bitflip_threshold should not be passed via DT. > It's neither a hardware parameter, nor it’s a static constant. Ah, I see. I will fixup, thanks for the explanation. > >> >+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 ? > > > >We have 3 options here, you _can_ use parse_ofpart_partitions() if > >your partition information conforms to its schema, but said schema > >does not support banks and/or other information that we choose to > >place within the bank node. The second option is to register a > >parser. My personal view is that registering a parser is using the > >framework for 'using the framework's' sake i.e. doesn't actually > >achieve anything special. We've chosen the third option, which is to > >parse and extract the information ourselves - which is actually fairly > >trivial, and pass the required partition data in through the > >mtd_device_parse_register() call - which is where the information > >would be parsed in the case of the first two options. > > > Do you really want to have *custom* partition format ? > If your previous code was non-DT (platform file based), then I think > It's good point to move to unified generic partition format which others > are following, as you make your driver DT compliant, and in mainline. > > I understand you primary objective would be to get ST driver work > out of mainline asap, but if you upstream too many custom stuff you > are only adding maintenance burden for your code. This is where > most of my comments originate. > However, I leave it to Brian to decide, if he is okay with these. > > > with regards, pekon
>From: Lee Jones [mailto:lee.jones@linaro.org] > >> >> >+ 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. >> > >> >I think the bit-flip threshold is/can be chip specific, and as we know >> >which chip we're likely to be booting on, we can specify this via the >> >hardware description. Thus, I think it's perfectly viable for an >> >option to pass through DT to exist. >> > >> I don't think that’s the correct interpretation of bitflip_threshold. >> >> (1) bitflip_threshold is dependent on ecc.strength (ECC scheme) of your driver. >> MTD layers uses bitflip_threshold to warn above layers that number of >> correctable bit-flips have reached a dangerous level beyond which driver's >> ECC scheme may not be able to correct them. So above layers should start >> taking additional corrective action like scrubbing. >> @@drivers/mtd/mtdcore.c: mtd_read() >> return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0; >> >> (2) Also, user-space may control it based on how your device ages on field. >> A fresh silicon may not show too many bitflips. But as device ages the >> probability of simultaneous bitflips will increase. So user-space may lower >> the bitflip_threshold to avoid accumulation of bitflips in a single page. >> >> Thus, bitflip_threshold should not be passed via DT. >> It's neither a hardware parameter, nor it’s a static constant. > >Ah, I see. I will fixup, thanks for the explanation. > Please wait, I'll review your [v2] series also, then you can further send all fixes together. I'm bit caught in other commitments for 3.16, so hopefully I'll be able to review your patches by next week. with regards, pekon
> >Ah, I see. I will fixup, thanks for the explanation. > > > > Please wait, I'll review your [v2] series also, then you can further > send all fixes together. I'm bit caught in other commitments for 3.16, > so hopefully I'll be able to review your patches by next week. I have already fixed up, but I won't resubmit until you have reviewed. FYI: I have made some changes since v2 including moving over to the recently submitted core timing structure submitted by Boris Brezillon and removal bitflip_threshold as an internal variable.
> >> >> >+ 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. > >> > > >> >I think the bit-flip threshold is/can be chip specific, and as we know > >> >which chip we're likely to be booting on, we can specify this via the > >> >hardware description. Thus, I think it's perfectly viable for an > >> >option to pass through DT to exist. > >> > > >> I don't think that’s the correct interpretation of bitflip_threshold. > >> > >> (1) bitflip_threshold is dependent on ecc.strength (ECC scheme) of your driver. > >> MTD layers uses bitflip_threshold to warn above layers that number of > >> correctable bit-flips have reached a dangerous level beyond which driver's > >> ECC scheme may not be able to correct them. So above layers should start > >> taking additional corrective action like scrubbing. > >> @@drivers/mtd/mtdcore.c: mtd_read() > >> return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0; > >> > >> (2) Also, user-space may control it based on how your device ages on field. > >> A fresh silicon may not show too many bitflips. But as device ages the > >> probability of simultaneous bitflips will increase. So user-space may lower > >> the bitflip_threshold to avoid accumulation of bitflips in a single page. > >> > >> Thus, bitflip_threshold should not be passed via DT. > >> It's neither a hardware parameter, nor it’s a static constant. > > > >Ah, I see. I will fixup, thanks for the explanation. > > Please wait, I'll review your [v2] series also, then you can further > send all fixes together. I'm bit caught in other commitments for 3.16, > so hopefully I'll be able to review your patches by next week. Did you manage to find some time to review the driver at all last week?
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 2bf972c..28155c0 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -516,4 +516,11 @@ config MTD_NAND_STM_BCH help Adds support for the STMicroelectronics NANDi BCH Controller. +config MTD_NAND_STM_BCH_DT + tristate "STMicroelectronics: NANDi BCH Controller Device Tree support" + default MTD_NAND_STM_BCH if OF + help + Adds support for the STMicroelectronics NANDi BCH Controller's + Device Tree component. + endif # MTD_NAND diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 94e7737..890f47f 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_MTD_NAND_MPC5121_NFC) += mpc5121_nfc.o obj-$(CONFIG_MTD_NAND_RICOH) += r852.o obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o obj-$(CONFIG_MTD_NAND_STM_BCH) += stm_nand_bch.o +obj-$(CONFIG_MTD_NAND_STM_BCH_DT) += stm_nand_dt.o obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/ obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/ diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c index 1de13bd..3115fb4 100644 --- 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); + 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); + + return pdata; +} +#else +static void *stm_bch_dt_get_pdata(struct platform_device *pdev) +{ + return NULL; +} +#endif + static int stm_nand_bch_probe(struct platform_device *pdev) { struct stm_plat_nand_bch_data *pdata = pdev->dev.platform_data; + struct device_node *np = pdev->dev.of_node; + struct mtd_part_parser_data ppdata; struct stm_nand_bank_data *bank; struct nandi_bbt_info *bbt_info; struct nandi_controller *nandi; @@ -377,8 +422,18 @@ static int stm_nand_bch_probe(struct platform_device *pdev) int err; if (!pdata) { - dev_err(&pdev->dev, "no platform data found\n"); - return -EINVAL; + if (!np) { + dev_err(&pdev->dev, "no pdata or DT found\n"); + return -EINVAL; + } + + pdata = stm_bch_dt_get_pdata(pdev); + if (IS_ERR(pdata)) + return PTR_ERR(pdata); + + ppdata.of_node = stm_of_get_partitions_node(np, 0); + + pdev->dev.platform_data = pdata; } nandi = nandi_init_resources(pdev); @@ -460,12 +515,21 @@ static const struct dev_pm_ops stm_nand_bch_pm_ops = { static const struct dev_pm_ops stm_nand_bch_pm_ops; #endif +#ifdef CONFIG_OF +static struct of_device_id nand_bch_match[] = { + { .compatible = "st,nand-bch", }, + {}, +}; +MODULE_DEVICE_TABLE(of, nand_bch_match); +#endif + static struct platform_driver stm_nand_bch_driver = { .probe = stm_nand_bch_probe , .remove = stm_nand_bch_remove, .driver = { .name = "stm-nand-bch", .owner = THIS_MODULE, + .of_match_table = of_match_ptr(nand_bch_match), .pm = &stm_nand_bch_pm_ops, }, }; diff --git a/drivers/mtd/nand/stm_nand_dt.c b/drivers/mtd/nand/stm_nand_dt.c new file mode 100644 index 0000000..0e91bba --- /dev/null +++ b/drivers/mtd/nand/stm_nand_dt.c @@ -0,0 +1,146 @@ +/* + * drivers/mtd/nand/stm_nand_dt.c + * + * Support for NANDi BCH Controller Device Tree component + * + * Copyright (c) 2014 STMicroelectronics Limited + * Author: Author: Srinivas Kandagatla <srinivas.kandagatla@st.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/err.h> +#include <linux/byteorder/generic.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_mtd.h> +#include <linux/mtd/nand.h> +#include <linux/mtd/stm_nand.h> + +/** +* stm_of_get_partitions_node - get partitions node from stm-nand type devices. +* +* @dev device pointer to use for devm allocations. +* @np device node of the driver. +* @bank_nr which bank number to use to get partitions. +* +* Returns a node pointer if found, with refcount incremented, use +* of_node_put() on it when done. +* +*/ +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); + + return partsnp; +} +EXPORT_SYMBOL(stm_of_get_partitions_node); + +/** + * stm_of_get_nand_banks - Get nand banks info from a given device node. + * + * @dev device pointer to use for devm allocations. + * @np device node of the driver. + * @banksptr double pointer to banks which is allocated + * and filled with bank data. + * + * Returns a count of banks found in the given device node. + * + */ +int stm_of_get_nand_banks(struct device *dev, struct device_node *np, + struct stm_nand_bank_data **banksptr) +{ + struct stm_nand_bank_data *banks; + struct device_node *banknp, *banksnp; + int nr_banks = 0; + + if (!np) + return -ENODEV; + + banksnp = of_parse_phandle(np, "st,nand-banks", 0); + if (!banksnp) { + dev_warn(dev, "No NAND banks specified in DT: %s\n", + np->full_name); + return -ENODEV; + } + + for_each_child_of_node(banksnp, banknp) + nr_banks++; + + *banksptr = devm_kzalloc(dev, sizeof(*banks) * nr_banks, GFP_KERNEL); + banks = *banksptr; + banknp = NULL; + + for_each_child_of_node(banksnp, banknp) { + struct device_node *timingnp; + int bank = 0; + + of_property_read_u32(banknp, "st,nand-csn", &banks[bank].csn); + + if (of_get_nand_bus_width(banknp) == 16) + banks[bank].options |= NAND_BUSWIDTH_16; + if (of_get_nand_on_flash_bbt(banknp)) + banks[bank].bbt_options |= NAND_BBT_USE_FLASH; + + banks[bank].nr_partitions = 0; + banks[bank].partitions = NULL; + + timingnp = of_parse_phandle(banknp, "st,nand-timing-spec", 0); + if (timingnp) { + struct nand_timing_spec *ts; + + ts = devm_kzalloc(dev, sizeof(struct nand_timing_spec), + GFP_KERNEL); + + of_property_read_u32(timingnp, "tR", &ts->tR); + of_property_read_u32(timingnp, "tCLS", &ts->tCLS); + of_property_read_u32(timingnp, "tCS", &ts->tCS); + of_property_read_u32(timingnp, "tALS", &ts->tALS); + of_property_read_u32(timingnp, "tDS", &ts->tDS); + of_property_read_u32(timingnp, "tWP", &ts->tWP); + of_property_read_u32(timingnp, "tCLH", &ts->tCLH); + of_property_read_u32(timingnp, "tCH", &ts->tCH); + of_property_read_u32(timingnp, "tALH", &ts->tALH); + of_property_read_u32(timingnp, "tDH", &ts->tDH); + of_property_read_u32(timingnp, "tWB", &ts->tWB); + of_property_read_u32(timingnp, "tWH", &ts->tWH); + of_property_read_u32(timingnp, "tWC", &ts->tWC); + of_property_read_u32(timingnp, "tRP", &ts->tRP); + of_property_read_u32(timingnp, "tREH", &ts->tREH); + of_property_read_u32(timingnp, "tRC", &ts->tRC); + of_property_read_u32(timingnp, "tREA", &ts->tREA); + of_property_read_u32(timingnp, "tRHOH", &ts->tRHOH); + of_property_read_u32(timingnp, "tCEA", &ts->tCEA); + of_property_read_u32(timingnp, "tCOH", &ts->tCOH); + of_property_read_u32(timingnp, "tCHZ", &ts->tCHZ); + of_property_read_u32(timingnp, "tCSD", &ts->tCSD); + banks[bank].timing_spec = ts; + } + of_property_read_u32(banknp, "st,nand-timing-relax", + &banks[bank].timing_relax); + bank++; + } + + return nr_banks; +} +EXPORT_SYMBOL(stm_of_get_nand_banks); diff --git a/drivers/mtd/nand/stm_nand_dt.h b/drivers/mtd/nand/stm_nand_dt.h new file mode 100644 index 0000000..de4507c --- /dev/null +++ b/drivers/mtd/nand/stm_nand_dt.h @@ -0,0 +1,39 @@ +/* + * drivers/mtd/nand/stm_nand_dt.h + * + * Support for NANDi BCH Controller Device Tree component + * + * Copyright (c) 2014 STMicroelectronics Limited + * Author: Author: Srinivas Kandagatla <srinivas.kandagatla@st.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#ifndef STM_NAND_DT_H +#define STM_NAND_DT_H + +#if defined(CONFIG_MTD_NAND_STM_BCH_DT) +struct device_node *stm_of_get_partitions_node(struct device_node *np, + int bank_nr); + +int stm_of_get_nand_banks(struct device *dev, struct device_node *np, + struct stm_nand_bank_data **banksp); +#else +static inline +struct device_node *stm_of_get_partitions_node(struct device_node *np, + int bank_nr) +{ + return NULL; +} + +static inline int stm_of_get_nand_banks(struct device *dev, + struct device_node *np, + struct stm_nand_bank_data **banksp) +{ + return 0; +} +#endif /* CONFIG_MTD_NAND_STM_BCH_DT */ +#endif /* STM_NAND_DT_H */ diff --git a/include/linux/mtd/stm_nand.h b/include/linux/mtd/stm_nand.h index 99a69ca..f82f9f7 100644 --- a/include/linux/mtd/stm_nand.h +++ b/include/linux/mtd/stm_nand.h @@ -37,6 +37,15 @@ struct stm_nand_bank_data { int timing_relax; }; +/* ECC Modes */ +enum stm_nand_bch_ecc_config { + BCH_18BIT_ECC = 0, + BCH_30BIT_ECC, + BCH_NO_ECC, + BCH_ECC_RSRV, + BCH_ECC_AUTO, +}; + struct stm_plat_nand_bch_data { struct stm_nand_bank_data *bank; enum stm_nand_bch_ecc_config bch_ecc_cfg;
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@linaro.org> --- drivers/mtd/nand/Kconfig | 7 ++ drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/stm_nand_bch.c | 68 ++++++++++++++++++- drivers/mtd/nand/stm_nand_dt.c | 146 ++++++++++++++++++++++++++++++++++++++++ drivers/mtd/nand/stm_nand_dt.h | 39 +++++++++++ include/linux/mtd/stm_nand.h | 9 +++ 6 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 drivers/mtd/nand/stm_nand_dt.c create mode 100644 drivers/mtd/nand/stm_nand_dt.h