| Message ID | 1428086445-10554-2-git-send-email-stefan@agner.ch |
|---|---|
| State | Superseded |
| Delegated to: | Scott Wood |
| Headers | show |
On Fri, 2015-04-03 at 20:40 +0200, Stefan Agner wrote: > This commit allows users to enable/disable the Freescale NFC > controller found in systems like Vybrid (VF610), MPC5125, MCF54418 > or Kinetis K70 via Kconfig with more detailed help docs. > > Signed-off-by: Stefan Agner <stefan@agner.ch> > --- > configs/vf610twr_defconfig | 2 ++ > drivers/mtd/nand/Kconfig | 15 +++++++++++++++ > include/configs/vf610twr.h | 3 --- > 3 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/configs/vf610twr_defconfig b/configs/vf610twr_defconfig > index 7de374a..5e0ac9f 100644 > --- a/configs/vf610twr_defconfig > +++ b/configs/vf610twr_defconfig > @@ -1,3 +1,5 @@ > CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC" > CONFIG_ARM=y > CONFIG_TARGET_VF610TWR=y > +CONFIG_NAND_VF610_NFC=y > +CONFIG_SYS_NAND_BUSWIDTH_16BIT=y > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > index 72825c3..8056c06 100644 > --- a/drivers/mtd/nand/Kconfig > +++ b/drivers/mtd/nand/Kconfig > @@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES > of OOB area before last ECC sector data starts. This is potentially > used to preserve the bad block marker in the OOB area. > > +config NAND_VF610_NFC > + bool "Support for Freescale NFC for VF610/MPC5125" > + select SYS_NAND_SELF_INIT > + help > + Enables support for NAND Flash Controller on some Freescale > + processors like the VF610, MPC5125, MCF54418 or Kinetis K70. > + The driver supports a maximum 2k page size. The driver > + currently does not support hardware ECC. > + > +config SYS_NAND_BUSWIDTH_16BIT > + bool "Use 16-bit NAND interface" > + depends on NAND_VF610_NFC > + help > + Use 16-bit wide NAND flash interface. Why does a generic-sounding config name depend on VF610? Especially when README already lists three other drivers as using this option... Also, the help text makes it sound like it's at the user's discretion, rather than a description of hardware. I'd phrase it as something like "NAND has 16-bit interface" -Scott
On 2015-04-03 22:30, Scott Wood wrote: > On Fri, 2015-04-03 at 20:40 +0200, Stefan Agner wrote: >> This commit allows users to enable/disable the Freescale NFC >> controller found in systems like Vybrid (VF610), MPC5125, MCF54418 >> or Kinetis K70 via Kconfig with more detailed help docs. >> >> Signed-off-by: Stefan Agner <stefan@agner.ch> >> --- >> configs/vf610twr_defconfig | 2 ++ >> drivers/mtd/nand/Kconfig | 15 +++++++++++++++ >> include/configs/vf610twr.h | 3 --- >> 3 files changed, 17 insertions(+), 3 deletions(-) >> >> diff --git a/configs/vf610twr_defconfig b/configs/vf610twr_defconfig >> index 7de374a..5e0ac9f 100644 >> --- a/configs/vf610twr_defconfig >> +++ b/configs/vf610twr_defconfig >> @@ -1,3 +1,5 @@ >> CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC" >> CONFIG_ARM=y >> CONFIG_TARGET_VF610TWR=y >> +CONFIG_NAND_VF610_NFC=y >> +CONFIG_SYS_NAND_BUSWIDTH_16BIT=y >> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig >> index 72825c3..8056c06 100644 >> --- a/drivers/mtd/nand/Kconfig >> +++ b/drivers/mtd/nand/Kconfig >> @@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES >> of OOB area before last ECC sector data starts. This is potentially >> used to preserve the bad block marker in the OOB area. >> >> +config NAND_VF610_NFC >> + bool "Support for Freescale NFC for VF610/MPC5125" >> + select SYS_NAND_SELF_INIT >> + help >> + Enables support for NAND Flash Controller on some Freescale >> + processors like the VF610, MPC5125, MCF54418 or Kinetis K70. >> + The driver supports a maximum 2k page size. The driver >> + currently does not support hardware ECC. >> + >> +config SYS_NAND_BUSWIDTH_16BIT >> + bool "Use 16-bit NAND interface" >> + depends on NAND_VF610_NFC >> + help >> + Use 16-bit wide NAND flash interface. > > Why does a generic-sounding config name depend on VF610? Especially > when README already lists three other drivers as using this option... That option is _not_ meant as being VF610 specific. Since we have the ability to specify dependencies with Kconfig, I think it is nice to have options only available if a driver supports it, hence the depends. So far the VF610 NAND driver is the only one which is in Kconfig and supports it... I would expect that when another driver which supports that option gets migrated, depends will be extended accordingly. However, I just realized that the option end up between Vybrid specific configs because of Patch 3. I will move the option at the very bottom in next revision. > Also, the help text makes it sound like it's at the user's discretion, > rather than a description of hardware. I'd phrase it as something like > "NAND has 16-bit interface" Agreed, will change that. -- Stefan
On Fri, 2015-04-03 at 22:42 +0200, Stefan Agner wrote: > On 2015-04-03 22:30, Scott Wood wrote: > > On Fri, 2015-04-03 at 20:40 +0200, Stefan Agner wrote: > >> This commit allows users to enable/disable the Freescale NFC > >> controller found in systems like Vybrid (VF610), MPC5125, MCF54418 > >> or Kinetis K70 via Kconfig with more detailed help docs. > >> > >> Signed-off-by: Stefan Agner <stefan@agner.ch> > >> --- > >> configs/vf610twr_defconfig | 2 ++ > >> drivers/mtd/nand/Kconfig | 15 +++++++++++++++ > >> include/configs/vf610twr.h | 3 --- > >> 3 files changed, 17 insertions(+), 3 deletions(-) > >> > >> diff --git a/configs/vf610twr_defconfig b/configs/vf610twr_defconfig > >> index 7de374a..5e0ac9f 100644 > >> --- a/configs/vf610twr_defconfig > >> +++ b/configs/vf610twr_defconfig > >> @@ -1,3 +1,5 @@ > >> CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC" > >> CONFIG_ARM=y > >> CONFIG_TARGET_VF610TWR=y > >> +CONFIG_NAND_VF610_NFC=y > >> +CONFIG_SYS_NAND_BUSWIDTH_16BIT=y > >> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > >> index 72825c3..8056c06 100644 > >> --- a/drivers/mtd/nand/Kconfig > >> +++ b/drivers/mtd/nand/Kconfig > >> @@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES > >> of OOB area before last ECC sector data starts. This is potentially > >> used to preserve the bad block marker in the OOB area. > >> > >> +config NAND_VF610_NFC > >> + bool "Support for Freescale NFC for VF610/MPC5125" > >> + select SYS_NAND_SELF_INIT > >> + help > >> + Enables support for NAND Flash Controller on some Freescale > >> + processors like the VF610, MPC5125, MCF54418 or Kinetis K70. > >> + The driver supports a maximum 2k page size. The driver > >> + currently does not support hardware ECC. > >> + > >> +config SYS_NAND_BUSWIDTH_16BIT > >> + bool "Use 16-bit NAND interface" > >> + depends on NAND_VF610_NFC > >> + help > >> + Use 16-bit wide NAND flash interface. > > > > Why does a generic-sounding config name depend on VF610? Especially > > when README already lists three other drivers as using this option... > > That option is _not_ meant as being VF610 specific. > > Since we have the ability to specify dependencies with Kconfig, I think > it is nice to have options only available if a driver supports it, hence > the depends. So far the VF610 NAND driver is the only one which is in > Kconfig and supports it... I would expect that when another driver which > supports that option gets migrated, depends will be extended > accordingly. > > However, I just realized that the option end up between Vybrid specific > configs because of Patch 3. I will move the option at the very bottom in > next revision. Could you also add a comment mentioning the other drivers that use it, which aren't yet kconfiged? And then remove the old text from the README. -Scott
On 2015-04-03 22:46, Scott Wood wrote: > On Fri, 2015-04-03 at 22:42 +0200, Stefan Agner wrote: >> On 2015-04-03 22:30, Scott Wood wrote: >> > On Fri, 2015-04-03 at 20:40 +0200, Stefan Agner wrote: >> >> This commit allows users to enable/disable the Freescale NFC >> >> controller found in systems like Vybrid (VF610), MPC5125, MCF54418 >> >> or Kinetis K70 via Kconfig with more detailed help docs. >> >> >> >> Signed-off-by: Stefan Agner <stefan@agner.ch> >> >> --- >> >> configs/vf610twr_defconfig | 2 ++ >> >> drivers/mtd/nand/Kconfig | 15 +++++++++++++++ >> >> include/configs/vf610twr.h | 3 --- >> >> 3 files changed, 17 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/configs/vf610twr_defconfig b/configs/vf610twr_defconfig >> >> index 7de374a..5e0ac9f 100644 >> >> --- a/configs/vf610twr_defconfig >> >> +++ b/configs/vf610twr_defconfig >> >> @@ -1,3 +1,5 @@ >> >> CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC" >> >> CONFIG_ARM=y >> >> CONFIG_TARGET_VF610TWR=y >> >> +CONFIG_NAND_VF610_NFC=y >> >> +CONFIG_SYS_NAND_BUSWIDTH_16BIT=y >> >> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig >> >> index 72825c3..8056c06 100644 >> >> --- a/drivers/mtd/nand/Kconfig >> >> +++ b/drivers/mtd/nand/Kconfig >> >> @@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES >> >> of OOB area before last ECC sector data starts. This is potentially >> >> used to preserve the bad block marker in the OOB area. >> >> >> >> +config NAND_VF610_NFC >> >> + bool "Support for Freescale NFC for VF610/MPC5125" >> >> + select SYS_NAND_SELF_INIT >> >> + help >> >> + Enables support for NAND Flash Controller on some Freescale >> >> + processors like the VF610, MPC5125, MCF54418 or Kinetis K70. >> >> + The driver supports a maximum 2k page size. The driver >> >> + currently does not support hardware ECC. >> >> + >> >> +config SYS_NAND_BUSWIDTH_16BIT >> >> + bool "Use 16-bit NAND interface" >> >> + depends on NAND_VF610_NFC >> >> + help >> >> + Use 16-bit wide NAND flash interface. >> > >> > Why does a generic-sounding config name depend on VF610? Especially >> > when README already lists three other drivers as using this option... >> >> That option is _not_ meant as being VF610 specific. >> >> Since we have the ability to specify dependencies with Kconfig, I think >> it is nice to have options only available if a driver supports it, hence >> the depends. So far the VF610 NAND driver is the only one which is in >> Kconfig and supports it... I would expect that when another driver which >> supports that option gets migrated, depends will be extended >> accordingly. >> >> However, I just realized that the option end up between Vybrid specific >> configs because of Patch 3. I will move the option at the very bottom in >> next revision. > > Could you also add a comment mentioning the other drivers that use it, > which aren't yet kconfiged? And then remove the old text from the > README. By comment, you mean a Kconfig comment at that option, so the next stumbles upon it? So I can keep that single depends NAND_VF610_NFC for now? Removing CONFIG_SYS_NAND_BUSWIDTH_16BIT from doc/README.nand right? But with that, the options for the other drivers would be "undocumented" for the time being... -- Stefan
On Sat, 2015-04-04 at 00:30 +0200, Stefan Agner wrote: > On 2015-04-03 22:46, Scott Wood wrote: > > On Fri, 2015-04-03 at 22:42 +0200, Stefan Agner wrote: > >> On 2015-04-03 22:30, Scott Wood wrote: > >> > On Fri, 2015-04-03 at 20:40 +0200, Stefan Agner wrote: > >> >> This commit allows users to enable/disable the Freescale NFC > >> >> controller found in systems like Vybrid (VF610), MPC5125, MCF54418 > >> >> or Kinetis K70 via Kconfig with more detailed help docs. > >> >> > >> >> Signed-off-by: Stefan Agner <stefan@agner.ch> > >> >> --- > >> >> configs/vf610twr_defconfig | 2 ++ > >> >> drivers/mtd/nand/Kconfig | 15 +++++++++++++++ > >> >> include/configs/vf610twr.h | 3 --- > >> >> 3 files changed, 17 insertions(+), 3 deletions(-) > >> >> > >> >> diff --git a/configs/vf610twr_defconfig b/configs/vf610twr_defconfig > >> >> index 7de374a..5e0ac9f 100644 > >> >> --- a/configs/vf610twr_defconfig > >> >> +++ b/configs/vf610twr_defconfig > >> >> @@ -1,3 +1,5 @@ > >> >> CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC" > >> >> CONFIG_ARM=y > >> >> CONFIG_TARGET_VF610TWR=y > >> >> +CONFIG_NAND_VF610_NFC=y > >> >> +CONFIG_SYS_NAND_BUSWIDTH_16BIT=y > >> >> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > >> >> index 72825c3..8056c06 100644 > >> >> --- a/drivers/mtd/nand/Kconfig > >> >> +++ b/drivers/mtd/nand/Kconfig > >> >> @@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES > >> >> of OOB area before last ECC sector data starts. This is potentially > >> >> used to preserve the bad block marker in the OOB area. > >> >> > >> >> +config NAND_VF610_NFC > >> >> + bool "Support for Freescale NFC for VF610/MPC5125" > >> >> + select SYS_NAND_SELF_INIT > >> >> + help > >> >> + Enables support for NAND Flash Controller on some Freescale > >> >> + processors like the VF610, MPC5125, MCF54418 or Kinetis K70. > >> >> + The driver supports a maximum 2k page size. The driver > >> >> + currently does not support hardware ECC. > >> >> + > >> >> +config SYS_NAND_BUSWIDTH_16BIT > >> >> + bool "Use 16-bit NAND interface" > >> >> + depends on NAND_VF610_NFC > >> >> + help > >> >> + Use 16-bit wide NAND flash interface. > >> > > >> > Why does a generic-sounding config name depend on VF610? Especially > >> > when README already lists three other drivers as using this option... > >> > >> That option is _not_ meant as being VF610 specific. > >> > >> Since we have the ability to specify dependencies with Kconfig, I think > >> it is nice to have options only available if a driver supports it, hence > >> the depends. So far the VF610 NAND driver is the only one which is in > >> Kconfig and supports it... I would expect that when another driver which > >> supports that option gets migrated, depends will be extended > >> accordingly. > >> > >> However, I just realized that the option end up between Vybrid specific > >> configs because of Patch 3. I will move the option at the very bottom in > >> next revision. > > > > Could you also add a comment mentioning the other drivers that use it, > > which aren't yet kconfiged? And then remove the old text from the > > README. > > By comment, you mean a Kconfig comment at that option, so the next > stumbles upon it? So I can keep that single depends NAND_VF610_NFC for > now? Yes. > Removing CONFIG_SYS_NAND_BUSWIDTH_16BIT from doc/README.nand right? But > with that, the options for the other drivers would be "undocumented" for > the time being... No, it'd be documented in the kconfig file, even if it's not presented via the kconfig tool. It seems bad to have the option description duplicated -- and then potentially getting changed one place and not the other. -Scott
diff --git a/configs/vf610twr_defconfig b/configs/vf610twr_defconfig index 7de374a..5e0ac9f 100644 --- a/configs/vf610twr_defconfig +++ b/configs/vf610twr_defconfig @@ -1,3 +1,5 @@ CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC" CONFIG_ARM=y CONFIG_TARGET_VF610TWR=y +CONFIG_NAND_VF610_NFC=y +CONFIG_SYS_NAND_BUSWIDTH_16BIT=y diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 72825c3..8056c06 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES of OOB area before last ECC sector data starts. This is potentially used to preserve the bad block marker in the OOB area. +config NAND_VF610_NFC + bool "Support for Freescale NFC for VF610/MPC5125" + select SYS_NAND_SELF_INIT + help + Enables support for NAND Flash Controller on some Freescale + processors like the VF610, MPC5125, MCF54418 or Kinetis K70. + The driver supports a maximum 2k page size. The driver + currently does not support hardware ECC. + +config SYS_NAND_BUSWIDTH_16BIT + bool "Use 16-bit NAND interface" + depends on NAND_VF610_NFC + help + Use 16-bit wide NAND flash interface. + if SPL config SPL_NAND_DENALI diff --git a/include/configs/vf610twr.h b/include/configs/vf610twr.h index 05bc7d0..621aa13 100644 --- a/include/configs/vf610twr.h +++ b/include/configs/vf610twr.h @@ -50,10 +50,7 @@ #define CONFIG_CMD_NAND_TRIMFFS #ifdef CONFIG_CMD_NAND -#define CONFIG_NAND_VF610_NFC -#define CONFIG_SYS_NAND_SELF_INIT #define CONFIG_USE_ARCH_MEMCPY -#define CONFIG_SYS_NAND_BUSWIDTH_16BIT #define CONFIG_SYS_MAX_NAND_DEVICE 1 #define CONFIG_SYS_NAND_BASE NFC_BASE_ADDR
This commit allows users to enable/disable the Freescale NFC controller found in systems like Vybrid (VF610), MPC5125, MCF54418 or Kinetis K70 via Kconfig with more detailed help docs. Signed-off-by: Stefan Agner <stefan@agner.ch> --- configs/vf610twr_defconfig | 2 ++ drivers/mtd/nand/Kconfig | 15 +++++++++++++++ include/configs/vf610twr.h | 3 --- 3 files changed, 17 insertions(+), 3 deletions(-)