diff mbox

[v10,10/10] mtd: nand: omap: remove selection of BCH ecc-scheme via KConfig

Message ID 1382172254-12448-11-git-send-email-pekon@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

pekon gupta Oct. 19, 2013, 8:44 a.m. UTC
With OMAP NAND driver updates, selection of ecc-scheme:
*DT enabled kernel*
 	depends on ti,nand-ecc-opt and ti,elm-id DT bindings.
*Non DT enabled kernel*
	depends on elm_dev and ecc-scheme passed along with platform-data
	from board file.

So, selection of ecc-scheme (BCH8 or BCH4) from KConfig can be removed

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/nand/Kconfig | 40 ++++++----------------------------------
 1 file changed, 6 insertions(+), 34 deletions(-)

Comments

Ezequiel Garcia Oct. 23, 2013, 1:44 p.m. UTC | #1
Gupta,

I already have a question :-)

On Sat, Oct 19, 2013 at 02:14:14PM +0530, Pekon Gupta wrote:
> With OMAP NAND driver updates, selection of ecc-scheme:
> *DT enabled kernel*
>  	depends on ti,nand-ecc-opt and ti,elm-id DT bindings.
> *Non DT enabled kernel*
> 	depends on elm_dev and ecc-scheme passed along with platform-data
> 	from board file.
> 
> So, selection of ecc-scheme (BCH8 or BCH4) from KConfig can be removed
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>  drivers/mtd/nand/Kconfig | 40 ++++++----------------------------------
>  1 file changed, 6 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index d885298..93ae6a6 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -96,43 +96,15 @@ config MTD_NAND_OMAP2
>  
>  config MTD_NAND_OMAP_BCH
>  	depends on MTD_NAND && MTD_NAND_OMAP2 && ARCH_OMAP3

I'm wondering how are you testing this in your SOC_AM33XX board (which
is not ARCH_OMAP3). You probably have ARCH_OMAP3 always selected?

IMO, no need to depend on any ARCH_xxx, just make it depend on the driver
option itself, which already depends on ARCH_OMAP2PLUS (which is the bigger
SoC family).

Care to send a one-patch fix for that, so this *independent* bug can be
taken by Brian? Please add my Reported-by when you do so.
pekon gupta Oct. 23, 2013, 1:55 p.m. UTC | #2
PiBGcm9tOiBFemVxdWllbCBHYXJjaWEgW21haWx0bzplemVxdWllbC5nYXJjaWFAZnJlZS1lbGVj
dHJvbnMuY29tXQ0KPiANCj4gR3VwdGEsDQo+IA0KUGxlYXNlIGNhbGwgbWUgJ1Bla29uJyA6LSkN
Cg0KWy4uLl0NCg0KPiANCj4gSSdtIHdvbmRlcmluZyBob3cgYXJlIHlvdSB0ZXN0aW5nIHRoaXMg
aW4geW91ciBTT0NfQU0zM1hYIGJvYXJkICh3aGljaA0KPiBpcyBub3QgQVJDSF9PTUFQMykuIFlv
dSBwcm9iYWJseSBoYXZlIEFSQ0hfT01BUDMgYWx3YXlzIHNlbGVjdGVkPw0KPiANClllcywgb21h
cDJwbHVzX2RlZmNvbmZpZyBpcyBhIHN1cGVyIHNldC4uDQphcmNoL2FybS9jb25maWdzL29tYXAy
cGx1c19kZWZjb25maWcgYXV0b21hdGljYWxseSBlbmFibGVzIEFSQ0hfT01BUDMuDQoNCj4gSU1P
LCBubyBuZWVkIHRvIGRlcGVuZCBvbiBhbnkgQVJDSF94eHgsIGp1c3QgbWFrZSBpdCBkZXBlbmQg
b24gdGhlDQo+IGRyaXZlciANCj4gb3B0aW9uIGl0c2VsZiwgd2hpY2ggYWxyZWFkeSBkZXBlbmRz
IG9uIEFSQ0hfT01BUDJQTFVTICh3aGljaCBpcyB0aGUNCj4gYmlnZ2VyIFNvQyBmYW1pbHkpLg0K
PiANCj4gQ2FyZSB0byBzZW5kIGEgb25lLXBhdGNoIGZpeCBmb3IgdGhhdCwgc28gdGhpcyAqaW5k
ZXBlbmRlbnQqIGJ1ZyBjYW4gYmUNCj4gdGFrZW4gYnkgQnJpYW4/IFBsZWFzZSBhZGQgbXkgUmVw
b3J0ZWQtYnkgd2hlbiB5b3UgZG8gc28uDQo+IA0KVGhhbmtzLiBJdCdzIGdvb2QgdG8gY2xlYW4g
dGhpcyBkZXBlbmRlbmN5Li4NClllcywgc3VyZWx5IEknbGwgc2VuZCBhIGluZGVwZW5kZW50IHBh
dGNoIHdpdGggeW91ciByZXBvcnRlZC1ieS4uDQoNCg0Kd2l0aCByZWdhcmRzLCBwZWtvbg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ezequiel Garcia Oct. 23, 2013, 2:13 p.m. UTC | #3
Pekon,

On Wed, Oct 23, 2013 at 01:55:58PM +0000, Gupta, Pekon wrote:
> > 
> > I'm wondering how are you testing this in your SOC_AM33XX board (which
> > is not ARCH_OMAP3). You probably have ARCH_OMAP3 always selected?
> > 
> Yes, omap2plus_defconfig is a super set..
> arch/arm/configs/omap2plus_defconfig automatically enables ARCH_OMAP3.
> 

Yes, but I always remove what I won't use to reduce build time.

And  now that you bring this issue. IMHO, the AM33xx family is going
to be more and more widely used, so maybe introducing an am33xx_defconfig
makes sense.

Or is the trend to have the least possible amount of defconfigs?
Javier Martinez Canillas Oct. 24, 2013, 7:49 p.m. UTC | #4
Hi Ezequiel

On Wed, Oct 23, 2013 at 4:13 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Pekon,
>
> On Wed, Oct 23, 2013 at 01:55:58PM +0000, Gupta, Pekon wrote:
>> >
>> > I'm wondering how are you testing this in your SOC_AM33XX board (which
>> > is not ARCH_OMAP3). You probably have ARCH_OMAP3 always selected?
>> >
>> Yes, omap2plus_defconfig is a super set..
>> arch/arm/configs/omap2plus_defconfig automatically enables ARCH_OMAP3.
>>
>
> Yes, but I always remove what I won't use to reduce build time.
>
> And  now that you bring this issue. IMHO, the AM33xx family is going
> to be more and more widely used, so maybe introducing an am33xx_defconfig
> makes sense.
>
> Or is the trend to have the least possible amount of defconfigs?

I'm not familiar with AM33xx so I don't know how similar is to OMAP3
but we used to have different defconfigs for each OMAP board before
and consolidated everything in only two defconfigs: omap1_defconfig to
be used for all OMAP1 devices and omap2plus_defconfig to be used for
all OMAP2+ (i.e: OMAP{2,3,4,5}) devices.

Those have all the common kconfig options and board vendors can
customize to fit their needs and have a delta with something like:

$ ./scripts/kconfig/merge_config.sh
arch/arm/configs/omap2plus_defconfig /foo/bar/igep_defconfig

Again I'm not familiar with AM33xx but what I do know is that we
should keep defconfigs to a minimum.

> --
> Ezequiel GarcĂ­a, Free Electrons
> Embedded Linux, Kernel and Android Engineering
> http://free-electrons.com
> --

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ezequiel Garcia Oct. 24, 2013, 8:05 p.m. UTC | #5
Javier,

On Thu, Oct 24, 2013 at 09:49:09PM +0200, Javier Martinez Canillas wrote:
> Hi Ezequiel
> 
> On Wed, Oct 23, 2013 at 4:13 PM, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
> > Pekon,
> >
> > On Wed, Oct 23, 2013 at 01:55:58PM +0000, Gupta, Pekon wrote:
> >> >
> >> > I'm wondering how are you testing this in your SOC_AM33XX board (which
> >> > is not ARCH_OMAP3). You probably have ARCH_OMAP3 always selected?
> >> >
> >> Yes, omap2plus_defconfig is a super set..
> >> arch/arm/configs/omap2plus_defconfig automatically enables ARCH_OMAP3.
> >>
> >
> > Yes, but I always remove what I won't use to reduce build time.
> >
> > And  now that you bring this issue. IMHO, the AM33xx family is going
> > to be more and more widely used, so maybe introducing an am33xx_defconfig
> > makes sense.
> >
> > Or is the trend to have the least possible amount of defconfigs?
> 
> I'm not familiar with AM33xx so I don't know how similar is to OMAP3
> but we used to have different defconfigs for each OMAP board before
> and consolidated everything in only two defconfigs: omap1_defconfig to
> be used for all OMAP1 devices and omap2plus_defconfig to be used for
> all OMAP2+ (i.e: OMAP{2,3,4,5}) devices.
> 
> Those have all the common kconfig options and board vendors can
> customize to fit their needs and have a delta with something like:
> 
> $ ./scripts/kconfig/merge_config.sh
> arch/arm/configs/omap2plus_defconfig /foo/bar/igep_defconfig
> 
> Again I'm not familiar with AM33xx but what I do know is that we
> should keep defconfigs to a minimum.
> 

Fair enough.

It's just a bit annoying to have a bigger-than-minimal kernel,
and I get tired of stripping the options.

But I can't ask the kernel to hold my favorite picks either:
we have enough churn already ;-)
diff mbox

Patch

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index d885298..93ae6a6 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -96,43 +96,15 @@  config MTD_NAND_OMAP2
 
 config MTD_NAND_OMAP_BCH
 	depends on MTD_NAND && MTD_NAND_OMAP2 && ARCH_OMAP3
-	tristate "Enable support for hardware BCH error correction"
+	tristate "Support hardware based BCH error correction"
 	default n
 	select BCH
-	select BCH_CONST_PARAMS
 	help
-	 Support for hardware BCH error correction.
-
-choice
-	prompt "BCH error correction capability"
-	depends on MTD_NAND_OMAP_BCH
-
-config MTD_NAND_OMAP_BCH8
-	bool "8 bits / 512 bytes (recommended)"
-	help
-	 Support correcting up to 8 bitflips per 512-byte block.
-	 This will use 13 bytes of spare area per 512 bytes of page data.
-	 This is the recommended mode, as 4-bit mode does not work
-	 on some OMAP3 revisions, due to a hardware bug.
-
-config MTD_NAND_OMAP_BCH4
-	bool "4 bits / 512 bytes"
-	help
-	 Support correcting up to 4 bitflips per 512-byte block.
-	 This will use 7 bytes of spare area per 512 bytes of page data.
-	 Note that this mode does not work on some OMAP3 revisions, due to a
-	 hardware bug. Please check your OMAP datasheet before selecting this
-	 mode.
-
-endchoice
-
-if MTD_NAND_OMAP_BCH
-config BCH_CONST_M
-	default 13
-config BCH_CONST_T
-	default 4 if MTD_NAND_OMAP_BCH4
-	default 8 if MTD_NAND_OMAP_BCH8
-endif
+	  This config enables the ELM hardware engine, which can be used to
+	  locate and correct errors when using BCH ECC scheme. This offloads
+	  the cpu from doing ECC error searching and correction. However some
+	  legacy OMAP families like OMAP2xxx, OMAP3xxx do not have ELM engine
+	  so they should not enable this config symbol.
 
 config MTD_NAND_IDS
 	tristate