[U-Boot] [PATCH 2/4] mtd: vf610_nfc: add Freescale NFC controller configs to Kconfig

[Scott Wood]

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 at 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