Re: [PATCH 3/3] mtd: nand: Force omap_elm to be built as a module if omap2_nand is a module
On Fri, Sep 12, 2014 at 05:56:36PM +0100, Ezequiel Garcia wrote: Ultimately, I don't care much as I don't think anyone will build it as a module, except maybe for testing the driver under probe/remove cycles. I see that you sort of answered one of my questions here. On what do you base this claim? I see maintstream distros are gaining support for some common ARM platforms, and I bet they ship loadable modules. Or are you seeing otherwise? Brian -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] mtd: nand: Force omap_elm to be built as a module if omap2_nand is a module
On 18 September 2014 04:00, Brian Norris computersforpe...@gmail.com wrote:
On Mon, Sep 15, 2014 at 11:27:43AM +0300, Roger Quadros wrote:
On 09/12/2014 07:56 PM, Ezequiel Garcia wrote:
On 12 Sep 12:01 PM, Roger Quadros wrote:
On 09/11/2014 04:47 PM, Ezequiel Garcia wrote:
This commit adds a hidden option to build the omap_elm as a module, if
omap2_nand is a module (and similarly in the built-in case).
This fixes the following build error when omap2_nand is chosen built-in,
and omap_elm is chosen as a module:
drivers/built-in.o: In function `omap_nand_probe':
/work/linux-2.6/drivers/mtd/nand/omap2.c:2010: undefined reference to
`elm_config'
/work/linux-2.6/drivers/mtd/nand/omap2.c:1980: undefined reference to
`elm_config'
/work/linux-2.6/drivers/mtd/nand/omap2.c:1927: undefined reference to
`elm_config'
drivers/built-in.o: In function `omap_elm_correct_data':
/work/linux-2.6/drivers/mtd/nand/omap2.c:1444: undefined reference to
`elm_decode_bch_error_page'
Signed-off-by: Ezequiel Garcia ezequ...@vanguardiasur.com.ar
---
drivers/mtd/nand/Kconfig | 8 +++-
drivers/mtd/nand/Makefile | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index f1cf503..12e8ee8 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -96,7 +96,7 @@ config MTD_NAND_OMAP2
config MTD_NAND_OMAP_BCH
depends on MTD_NAND_OMAP2
- tristate Support hardware based BCH error correction
+ bool Support hardware based BCH error correction
default n
select BCH
help
@@ -106,6 +106,12 @@ config MTD_NAND_OMAP_BCH
legacy OMAP families like OMAP2xxx, OMAP3xxx do not have ELM engine
so they should not enable this config symbol.
+config MTD_NAND_OMAP_BCH_BUILD
+ tristate
+ depends on MTD_NAND_OMAP2
+ default m if MTD_NAND_OMAP2=m MTD_NAND_OMAP_BCH
+ default y if MTD_NAND_OMAP2=y MTD_NAND_OMAP_BCH
I think the last 2 lines could be combined:
default MTD_NAND_OMAP2 if MTD_NAND_OMAP_BCH
+
config MTD_NAND_IDS
tristate
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 4bcdeb0..3580188 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -27,7 +27,7 @@ obj-$(CONFIG_MTD_NAND_NDFC) += ndfc.o
obj-$(CONFIG_MTD_NAND_ATMEL) += atmel_nand.o
obj-$(CONFIG_MTD_NAND_GPIO) += gpio.o
obj-$(CONFIG_MTD_NAND_OMAP2) += omap2_nand.o
-obj-$(CONFIG_MTD_NAND_OMAP_BCH) += omap_elm.o
+obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)+= omap_elm.o
obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270_nand.o
obj-$(CONFIG_MTD_NAND_PXA3xx)+= pxa3xx_nand.o
obj-$(CONFIG_MTD_NAND_TMIO) += tmio_nand.o
Apparently I came up with a nearly identical solution, so it must be
good! ;)
The overall logic seems to work but I still see the following issue.
In menuconfig, the OMAP_BCH option is still visible as a boolean even
though
the ELM module finally gets built as a module.
This can be confusing to the user and I'd avoid that behaviour.
Yes, I know. But it's either this solution or no solution at all, I think.
Let me give some further context about this patch, so we can have more
information to decide.
First of all (and IMO) the user doesn't have to know about the ELM being
a module or not, because modprobe will load it (since omap2_nand depends
on omap_elm's symbols).
So, the new way seems a bit more intuitive for me. The user choses if he
wants to have hardware BCH support, and such support gets built the right
way.
If we still consider this highly confusing, and we want to avoid it,
then it seems we can only make omap_elm a boolean, which means it'll always
be built-in. I crafted this patch in an effort to avoid making it boolean.
Finally, the solution is taken from media/usb/stk1160. For good or for bad,
we have a precedent in doing things this way.
Ultimately, I don't care much as I don't think anyone will build it as a
module,
except maybe for testing the driver under probe/remove cycles.
OK. I personally prefer boolean than the Kconfig magic as it makes my life a
bit
easier and less confusing to the user i.e. wysiwyg ;).
Let's see what the MTD maintainers prefer.
Brian and David, any insights on this problem?
It seems like 'elm' serves more as an accessory piece of omap2.{o,ko},
not really a separate module, so it's possible it'd make your life even
easier to just link elm.o into omap2.o. There's no requirement that two
source files create two separate kernel modules. I think this would
present the simplest possible interface to the user.
Thoughts?
Yeah, but that means a larger refactoring, because now you'd have to
call the ELM driver probe() from the NAND driver probe().
It'd be a nice cleanup, but maybe this is the second
Re: [PATCH 3/3] mtd: nand: Force omap_elm to be built as a module if omap2_nand is a module
On 09/18/2014 06:00 AM, Brian Norris wrote:
On Mon, Sep 15, 2014 at 11:27:43AM +0300, Roger Quadros wrote:
On 09/12/2014 07:56 PM, Ezequiel Garcia wrote:
On 12 Sep 12:01 PM, Roger Quadros wrote:
On 09/11/2014 04:47 PM, Ezequiel Garcia wrote:
This commit adds a hidden option to build the omap_elm as a module, if
omap2_nand is a module (and similarly in the built-in case).
This fixes the following build error when omap2_nand is chosen built-in,
and omap_elm is chosen as a module:
drivers/built-in.o: In function `omap_nand_probe':
/work/linux-2.6/drivers/mtd/nand/omap2.c:2010: undefined reference to
`elm_config'
/work/linux-2.6/drivers/mtd/nand/omap2.c:1980: undefined reference to
`elm_config'
/work/linux-2.6/drivers/mtd/nand/omap2.c:1927: undefined reference to
`elm_config'
drivers/built-in.o: In function `omap_elm_correct_data':
/work/linux-2.6/drivers/mtd/nand/omap2.c:1444: undefined reference to
`elm_decode_bch_error_page'
Signed-off-by: Ezequiel Garcia ezequ...@vanguardiasur.com.ar
---
drivers/mtd/nand/Kconfig | 8 +++-
drivers/mtd/nand/Makefile | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index f1cf503..12e8ee8 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -96,7 +96,7 @@ config MTD_NAND_OMAP2
config MTD_NAND_OMAP_BCH
depends on MTD_NAND_OMAP2
- tristate Support hardware based BCH error correction
+ bool Support hardware based BCH error correction
default n
select BCH
help
@@ -106,6 +106,12 @@ config MTD_NAND_OMAP_BCH
legacy OMAP families like OMAP2xxx, OMAP3xxx do not have ELM engine
so they should not enable this config symbol.
+config MTD_NAND_OMAP_BCH_BUILD
+ tristate
+ depends on MTD_NAND_OMAP2
+ default m if MTD_NAND_OMAP2=m MTD_NAND_OMAP_BCH
+ default y if MTD_NAND_OMAP2=y MTD_NAND_OMAP_BCH
I think the last 2 lines could be combined:
default MTD_NAND_OMAP2 if MTD_NAND_OMAP_BCH
+
config MTD_NAND_IDS
tristate
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 4bcdeb0..3580188 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -27,7 +27,7 @@ obj-$(CONFIG_MTD_NAND_NDFC) += ndfc.o
obj-$(CONFIG_MTD_NAND_ATMEL) += atmel_nand.o
obj-$(CONFIG_MTD_NAND_GPIO) += gpio.o
obj-$(CONFIG_MTD_NAND_OMAP2) += omap2_nand.o
-obj-$(CONFIG_MTD_NAND_OMAP_BCH) += omap_elm.o
+obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)+= omap_elm.o
obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270_nand.o
obj-$(CONFIG_MTD_NAND_PXA3xx)+= pxa3xx_nand.o
obj-$(CONFIG_MTD_NAND_TMIO) += tmio_nand.o
Apparently I came up with a nearly identical solution, so it must be
good! ;)
The overall logic seems to work but I still see the following issue.
In menuconfig, the OMAP_BCH option is still visible as a boolean even
though
the ELM module finally gets built as a module.
This can be confusing to the user and I'd avoid that behaviour.
Yes, I know. But it's either this solution or no solution at all, I think.
Let me give some further context about this patch, so we can have more
information to decide.
First of all (and IMO) the user doesn't have to know about the ELM being
a module or not, because modprobe will load it (since omap2_nand depends
on omap_elm's symbols).
So, the new way seems a bit more intuitive for me. The user choses if he
wants to have hardware BCH support, and such support gets built the right
way.
If we still consider this highly confusing, and we want to avoid it,
then it seems we can only make omap_elm a boolean, which means it'll always
be built-in. I crafted this patch in an effort to avoid making it boolean.
Finally, the solution is taken from media/usb/stk1160. For good or for bad,
we have a precedent in doing things this way.
Ultimately, I don't care much as I don't think anyone will build it as a
module,
except maybe for testing the driver under probe/remove cycles.
OK. I personally prefer boolean than the Kconfig magic as it makes my life a
bit
easier and less confusing to the user i.e. wysiwyg ;).
Let's see what the MTD maintainers prefer.
Brian and David, any insights on this problem?
It seems like 'elm' serves more as an accessory piece of omap2.{o,ko},
not really a separate module, so it's possible it'd make your life even
easier to just link elm.o into omap2.o. There's no requirement that two
source files create two separate kernel modules. I think this would
present the simplest possible interface to the user.
Sounds fine to me.
Point to note is that ELM is not present on all OMAPs and has its own device
tree node
and compatible id which is different from the NAND controller node. So in
theory it
needs to have it's own separate driver, but since the only user is OMAP NAND
driver
I don't see why they can't be combined.
Re: [PATCH 3/3] mtd: nand: Force omap_elm to be built as a module if omap2_nand is a module
On 09/18/2014 11:40 AM, Ezequiel Garcia wrote:
On 18 September 2014 04:00, Brian Norris computersforpe...@gmail.com wrote:
On Mon, Sep 15, 2014 at 11:27:43AM +0300, Roger Quadros wrote:
On 09/12/2014 07:56 PM, Ezequiel Garcia wrote:
On 12 Sep 12:01 PM, Roger Quadros wrote:
On 09/11/2014 04:47 PM, Ezequiel Garcia wrote:
This commit adds a hidden option to build the omap_elm as a module, if
omap2_nand is a module (and similarly in the built-in case).
This fixes the following build error when omap2_nand is chosen built-in,
and omap_elm is chosen as a module:
drivers/built-in.o: In function `omap_nand_probe':
/work/linux-2.6/drivers/mtd/nand/omap2.c:2010: undefined reference to
`elm_config'
/work/linux-2.6/drivers/mtd/nand/omap2.c:1980: undefined reference to
`elm_config'
/work/linux-2.6/drivers/mtd/nand/omap2.c:1927: undefined reference to
`elm_config'
drivers/built-in.o: In function `omap_elm_correct_data':
/work/linux-2.6/drivers/mtd/nand/omap2.c:1444: undefined reference to
`elm_decode_bch_error_page'
Signed-off-by: Ezequiel Garcia ezequ...@vanguardiasur.com.ar
---
drivers/mtd/nand/Kconfig | 8 +++-
drivers/mtd/nand/Makefile | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index f1cf503..12e8ee8 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -96,7 +96,7 @@ config MTD_NAND_OMAP2
config MTD_NAND_OMAP_BCH
depends on MTD_NAND_OMAP2
- tristate Support hardware based BCH error correction
+ bool Support hardware based BCH error correction
default n
select BCH
help
@@ -106,6 +106,12 @@ config MTD_NAND_OMAP_BCH
legacy OMAP families like OMAP2xxx, OMAP3xxx do not have ELM engine
so they should not enable this config symbol.
+config MTD_NAND_OMAP_BCH_BUILD
+ tristate
+ depends on MTD_NAND_OMAP2
+ default m if MTD_NAND_OMAP2=m MTD_NAND_OMAP_BCH
+ default y if MTD_NAND_OMAP2=y MTD_NAND_OMAP_BCH
I think the last 2 lines could be combined:
default MTD_NAND_OMAP2 if MTD_NAND_OMAP_BCH
+
config MTD_NAND_IDS
tristate
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 4bcdeb0..3580188 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -27,7 +27,7 @@ obj-$(CONFIG_MTD_NAND_NDFC) += ndfc.o
obj-$(CONFIG_MTD_NAND_ATMEL) += atmel_nand.o
obj-$(CONFIG_MTD_NAND_GPIO) += gpio.o
obj-$(CONFIG_MTD_NAND_OMAP2) += omap2_nand.o
-obj-$(CONFIG_MTD_NAND_OMAP_BCH) += omap_elm.o
+obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)+= omap_elm.o
obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270_nand.o
obj-$(CONFIG_MTD_NAND_PXA3xx)+= pxa3xx_nand.o
obj-$(CONFIG_MTD_NAND_TMIO) += tmio_nand.o
Apparently I came up with a nearly identical solution, so it must be
good! ;)
The overall logic seems to work but I still see the following issue.
In menuconfig, the OMAP_BCH option is still visible as a boolean even
though
the ELM module finally gets built as a module.
This can be confusing to the user and I'd avoid that behaviour.
Yes, I know. But it's either this solution or no solution at all, I think.
Let me give some further context about this patch, so we can have more
information to decide.
First of all (and IMO) the user doesn't have to know about the ELM being
a module or not, because modprobe will load it (since omap2_nand depends
on omap_elm's symbols).
So, the new way seems a bit more intuitive for me. The user choses if he
wants to have hardware BCH support, and such support gets built the right
way.
If we still consider this highly confusing, and we want to avoid it,
then it seems we can only make omap_elm a boolean, which means it'll always
be built-in. I crafted this patch in an effort to avoid making it boolean.
Finally, the solution is taken from media/usb/stk1160. For good or for bad,
we have a precedent in doing things this way.
Ultimately, I don't care much as I don't think anyone will build it as a
module,
except maybe for testing the driver under probe/remove cycles.
OK. I personally prefer boolean than the Kconfig magic as it makes my life
a bit
easier and less confusing to the user i.e. wysiwyg ;).
Let's see what the MTD maintainers prefer.
Brian and David, any insights on this problem?
It seems like 'elm' serves more as an accessory piece of omap2.{o,ko},
not really a separate module, so it's possible it'd make your life even
easier to just link elm.o into omap2.o. There's no requirement that two
source files create two separate kernel modules. I think this would
present the simplest possible interface to the user.
Thoughts?
Yeah, but that means a larger refactoring, because now you'd have to
call the ELM driver probe() from the NAND driver probe().
It'd be a nice cleanup, but maybe this is the second step?
Yes, let's do this as a next step. For
Re: [PATCH 3/3] mtd: nand: Force omap_elm to be built as a module if omap2_nand is a module
On Mon, Sep 15, 2014 at 11:27:43AM +0300, Roger Quadros wrote:
On 09/12/2014 07:56 PM, Ezequiel Garcia wrote:
On 12 Sep 12:01 PM, Roger Quadros wrote:
On 09/11/2014 04:47 PM, Ezequiel Garcia wrote:
This commit adds a hidden option to build the omap_elm as a module, if
omap2_nand is a module (and similarly in the built-in case).
This fixes the following build error when omap2_nand is chosen built-in,
and omap_elm is chosen as a module:
drivers/built-in.o: In function `omap_nand_probe':
/work/linux-2.6/drivers/mtd/nand/omap2.c:2010: undefined reference to
`elm_config'
/work/linux-2.6/drivers/mtd/nand/omap2.c:1980: undefined reference to
`elm_config'
/work/linux-2.6/drivers/mtd/nand/omap2.c:1927: undefined reference to
`elm_config'
drivers/built-in.o: In function `omap_elm_correct_data':
/work/linux-2.6/drivers/mtd/nand/omap2.c:1444: undefined reference to
`elm_decode_bch_error_page'
Signed-off-by: Ezequiel Garcia ezequ...@vanguardiasur.com.ar
---
drivers/mtd/nand/Kconfig | 8 +++-
drivers/mtd/nand/Makefile | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index f1cf503..12e8ee8 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -96,7 +96,7 @@ config MTD_NAND_OMAP2
config MTD_NAND_OMAP_BCH
depends on MTD_NAND_OMAP2
- tristate Support hardware based BCH error correction
+ bool Support hardware based BCH error correction
default n
select BCH
help
@@ -106,6 +106,12 @@ config MTD_NAND_OMAP_BCH
legacy OMAP families like OMAP2xxx, OMAP3xxx do not have ELM engine
so they should not enable this config symbol.
+config MTD_NAND_OMAP_BCH_BUILD
+ tristate
+ depends on MTD_NAND_OMAP2
+ default m if MTD_NAND_OMAP2=m MTD_NAND_OMAP_BCH
+ default y if MTD_NAND_OMAP2=y MTD_NAND_OMAP_BCH
I think the last 2 lines could be combined:
default MTD_NAND_OMAP2 if MTD_NAND_OMAP_BCH
+
config MTD_NAND_IDS
tristate
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 4bcdeb0..3580188 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -27,7 +27,7 @@ obj-$(CONFIG_MTD_NAND_NDFC) += ndfc.o
obj-$(CONFIG_MTD_NAND_ATMEL) += atmel_nand.o
obj-$(CONFIG_MTD_NAND_GPIO) += gpio.o
obj-$(CONFIG_MTD_NAND_OMAP2) += omap2_nand.o
-obj-$(CONFIG_MTD_NAND_OMAP_BCH) += omap_elm.o
+obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)+= omap_elm.o
obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270_nand.o
obj-$(CONFIG_MTD_NAND_PXA3xx)+= pxa3xx_nand.o
obj-$(CONFIG_MTD_NAND_TMIO) += tmio_nand.o
Apparently I came up with a nearly identical solution, so it must be
good! ;)
The overall logic seems to work but I still see the following issue.
In menuconfig, the OMAP_BCH option is still visible as a boolean even
though
the ELM module finally gets built as a module.
This can be confusing to the user and I'd avoid that behaviour.
Yes, I know. But it's either this solution or no solution at all, I think.
Let me give some further context about this patch, so we can have more
information to decide.
First of all (and IMO) the user doesn't have to know about the ELM being
a module or not, because modprobe will load it (since omap2_nand depends
on omap_elm's symbols).
So, the new way seems a bit more intuitive for me. The user choses if he
wants to have hardware BCH support, and such support gets built the right
way.
If we still consider this highly confusing, and we want to avoid it,
then it seems we can only make omap_elm a boolean, which means it'll always
be built-in. I crafted this patch in an effort to avoid making it boolean.
Finally, the solution is taken from media/usb/stk1160. For good or for bad,
we have a precedent in doing things this way.
Ultimately, I don't care much as I don't think anyone will build it as a
module,
except maybe for testing the driver under probe/remove cycles.
OK. I personally prefer boolean than the Kconfig magic as it makes my life a
bit
easier and less confusing to the user i.e. wysiwyg ;).
Let's see what the MTD maintainers prefer.
Brian and David, any insights on this problem?
It seems like 'elm' serves more as an accessory piece of omap2.{o,ko},
not really a separate module, so it's possible it'd make your life even
easier to just link elm.o into omap2.o. There's no requirement that two
source files create two separate kernel modules. I think this would
present the simplest possible interface to the user.
Thoughts?
Brian
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] mtd: nand: Force omap_elm to be built as a module if omap2_nand is a module
On 09/12/2014 07:56 PM, Ezequiel Garcia wrote: On 12 Sep 12:01 PM, Roger Quadros wrote: On 09/11/2014 04:47 PM, Ezequiel Garcia wrote: This commit adds a hidden option to build the omap_elm as a module, if omap2_nand is a module (and similarly in the built-in case). This fixes the following build error when omap2_nand is chosen built-in, and omap_elm is chosen as a module: drivers/built-in.o: In function `omap_nand_probe': /work/linux-2.6/drivers/mtd/nand/omap2.c:2010: undefined reference to `elm_config' /work/linux-2.6/drivers/mtd/nand/omap2.c:1980: undefined reference to `elm_config' /work/linux-2.6/drivers/mtd/nand/omap2.c:1927: undefined reference to `elm_config' drivers/built-in.o: In function `omap_elm_correct_data': /work/linux-2.6/drivers/mtd/nand/omap2.c:1444: undefined reference to `elm_decode_bch_error_page' Signed-off-by: Ezequiel Garcia ezequ...@vanguardiasur.com.ar --- drivers/mtd/nand/Kconfig | 8 +++- drivers/mtd/nand/Makefile | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index f1cf503..12e8ee8 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -96,7 +96,7 @@ config MTD_NAND_OMAP2 config MTD_NAND_OMAP_BCH depends on MTD_NAND_OMAP2 - tristate Support hardware based BCH error correction + bool Support hardware based BCH error correction default n select BCH help @@ -106,6 +106,12 @@ config MTD_NAND_OMAP_BCH legacy OMAP families like OMAP2xxx, OMAP3xxx do not have ELM engine so they should not enable this config symbol. +config MTD_NAND_OMAP_BCH_BUILD + tristate + depends on MTD_NAND_OMAP2 + default m if MTD_NAND_OMAP2=m MTD_NAND_OMAP_BCH + default y if MTD_NAND_OMAP2=y MTD_NAND_OMAP_BCH + config MTD_NAND_IDS tristate diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 4bcdeb0..3580188 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -27,7 +27,7 @@ obj-$(CONFIG_MTD_NAND_NDFC) += ndfc.o obj-$(CONFIG_MTD_NAND_ATMEL) += atmel_nand.o obj-$(CONFIG_MTD_NAND_GPIO)+= gpio.o obj-$(CONFIG_MTD_NAND_OMAP2) += omap2_nand.o -obj-$(CONFIG_MTD_NAND_OMAP_BCH)+= omap_elm.o +obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD) += omap_elm.o obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270_nand.o obj-$(CONFIG_MTD_NAND_PXA3xx) += pxa3xx_nand.o obj-$(CONFIG_MTD_NAND_TMIO)+= tmio_nand.o The overall logic seems to work but I still see the following issue. In menuconfig, the OMAP_BCH option is still visible as a boolean even though the ELM module finally gets built as a module. This can be confusing to the user and I'd avoid that behaviour. Yes, I know. But it's either this solution or no solution at all, I think. Let me give some further context about this patch, so we can have more information to decide. First of all (and IMO) the user doesn't have to know about the ELM being a module or not, because modprobe will load it (since omap2_nand depends on omap_elm's symbols). So, the new way seems a bit more intuitive for me. The user choses if he wants to have hardware BCH support, and such support gets built the right way. If we still consider this highly confusing, and we want to avoid it, then it seems we can only make omap_elm a boolean, which means it'll always be built-in. I crafted this patch in an effort to avoid making it boolean. Finally, the solution is taken from media/usb/stk1160. For good or for bad, we have a precedent in doing things this way. Ultimately, I don't care much as I don't think anyone will build it as a module, except maybe for testing the driver under probe/remove cycles. OK. I personally prefer boolean than the Kconfig magic as it makes my life a bit easier and less confusing to the user i.e. wysiwyg ;). Let's see what the MTD maintainers prefer. Brian and David, any insights on this problem? cheers, -roger -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] mtd: nand: Force omap_elm to be built as a module if omap2_nand is a module
On 09/11/2014 04:47 PM, Ezequiel Garcia wrote: This commit adds a hidden option to build the omap_elm as a module, if omap2_nand is a module (and similarly in the built-in case). This fixes the following build error when omap2_nand is chosen built-in, and omap_elm is chosen as a module: drivers/built-in.o: In function `omap_nand_probe': /work/linux-2.6/drivers/mtd/nand/omap2.c:2010: undefined reference to `elm_config' /work/linux-2.6/drivers/mtd/nand/omap2.c:1980: undefined reference to `elm_config' /work/linux-2.6/drivers/mtd/nand/omap2.c:1927: undefined reference to `elm_config' drivers/built-in.o: In function `omap_elm_correct_data': /work/linux-2.6/drivers/mtd/nand/omap2.c:1444: undefined reference to `elm_decode_bch_error_page' Signed-off-by: Ezequiel Garcia ezequ...@vanguardiasur.com.ar --- drivers/mtd/nand/Kconfig | 8 +++- drivers/mtd/nand/Makefile | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index f1cf503..12e8ee8 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -96,7 +96,7 @@ config MTD_NAND_OMAP2 config MTD_NAND_OMAP_BCH depends on MTD_NAND_OMAP2 - tristate Support hardware based BCH error correction + bool Support hardware based BCH error correction default n select BCH help @@ -106,6 +106,12 @@ config MTD_NAND_OMAP_BCH legacy OMAP families like OMAP2xxx, OMAP3xxx do not have ELM engine so they should not enable this config symbol. +config MTD_NAND_OMAP_BCH_BUILD + tristate + depends on MTD_NAND_OMAP2 + default m if MTD_NAND_OMAP2=m MTD_NAND_OMAP_BCH + default y if MTD_NAND_OMAP2=y MTD_NAND_OMAP_BCH + config MTD_NAND_IDS tristate diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 4bcdeb0..3580188 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -27,7 +27,7 @@ obj-$(CONFIG_MTD_NAND_NDFC) += ndfc.o obj-$(CONFIG_MTD_NAND_ATMEL) += atmel_nand.o obj-$(CONFIG_MTD_NAND_GPIO) += gpio.o obj-$(CONFIG_MTD_NAND_OMAP2) += omap2_nand.o -obj-$(CONFIG_MTD_NAND_OMAP_BCH) += omap_elm.o +obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)+= omap_elm.o obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270_nand.o obj-$(CONFIG_MTD_NAND_PXA3xx)+= pxa3xx_nand.o obj-$(CONFIG_MTD_NAND_TMIO) += tmio_nand.o The overall logic seems to work but I still see the following issue. In menuconfig, the OMAP_BCH option is still visible as a boolean even though the ELM module finally gets built as a module. This can be confusing to the user and I'd avoid that behaviour. cheers, -roger -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] mtd: nand: Force omap_elm to be built as a module if omap2_nand is a module
On 12 Sep 12:01 PM, Roger Quadros wrote: On 09/11/2014 04:47 PM, Ezequiel Garcia wrote: This commit adds a hidden option to build the omap_elm as a module, if omap2_nand is a module (and similarly in the built-in case). This fixes the following build error when omap2_nand is chosen built-in, and omap_elm is chosen as a module: drivers/built-in.o: In function `omap_nand_probe': /work/linux-2.6/drivers/mtd/nand/omap2.c:2010: undefined reference to `elm_config' /work/linux-2.6/drivers/mtd/nand/omap2.c:1980: undefined reference to `elm_config' /work/linux-2.6/drivers/mtd/nand/omap2.c:1927: undefined reference to `elm_config' drivers/built-in.o: In function `omap_elm_correct_data': /work/linux-2.6/drivers/mtd/nand/omap2.c:1444: undefined reference to `elm_decode_bch_error_page' Signed-off-by: Ezequiel Garcia ezequ...@vanguardiasur.com.ar --- drivers/mtd/nand/Kconfig | 8 +++- drivers/mtd/nand/Makefile | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index f1cf503..12e8ee8 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -96,7 +96,7 @@ config MTD_NAND_OMAP2 config MTD_NAND_OMAP_BCH depends on MTD_NAND_OMAP2 - tristate Support hardware based BCH error correction + bool Support hardware based BCH error correction default n select BCH help @@ -106,6 +106,12 @@ config MTD_NAND_OMAP_BCH legacy OMAP families like OMAP2xxx, OMAP3xxx do not have ELM engine so they should not enable this config symbol. +config MTD_NAND_OMAP_BCH_BUILD + tristate + depends on MTD_NAND_OMAP2 + default m if MTD_NAND_OMAP2=m MTD_NAND_OMAP_BCH + default y if MTD_NAND_OMAP2=y MTD_NAND_OMAP_BCH + config MTD_NAND_IDS tristate diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 4bcdeb0..3580188 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -27,7 +27,7 @@ obj-$(CONFIG_MTD_NAND_NDFC) += ndfc.o obj-$(CONFIG_MTD_NAND_ATMEL) += atmel_nand.o obj-$(CONFIG_MTD_NAND_GPIO)+= gpio.o obj-$(CONFIG_MTD_NAND_OMAP2) += omap2_nand.o -obj-$(CONFIG_MTD_NAND_OMAP_BCH)+= omap_elm.o +obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD) += omap_elm.o obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270_nand.o obj-$(CONFIG_MTD_NAND_PXA3xx) += pxa3xx_nand.o obj-$(CONFIG_MTD_NAND_TMIO)+= tmio_nand.o The overall logic seems to work but I still see the following issue. In menuconfig, the OMAP_BCH option is still visible as a boolean even though the ELM module finally gets built as a module. This can be confusing to the user and I'd avoid that behaviour. Yes, I know. But it's either this solution or no solution at all, I think. Let me give some further context about this patch, so we can have more information to decide. First of all (and IMO) the user doesn't have to know about the ELM being a module or not, because modprobe will load it (since omap2_nand depends on omap_elm's symbols). So, the new way seems a bit more intuitive for me. The user choses if he wants to have hardware BCH support, and such support gets built the right way. If we still consider this highly confusing, and we want to avoid it, then it seems we can only make omap_elm a boolean, which means it'll always be built-in. I crafted this patch in an effort to avoid making it boolean. Finally, the solution is taken from media/usb/stk1160. For good or for bad, we have a precedent in doing things this way. Ultimately, I don't care much as I don't think anyone will build it as a module, except maybe for testing the driver under probe/remove cycles. -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] mtd: nand: Force omap_elm to be built as a module if omap2_nand is a module
This commit adds a hidden option to build the omap_elm as a module, if omap2_nand is a module (and similarly in the built-in case). This fixes the following build error when omap2_nand is chosen built-in, and omap_elm is chosen as a module: drivers/built-in.o: In function `omap_nand_probe': /work/linux-2.6/drivers/mtd/nand/omap2.c:2010: undefined reference to `elm_config' /work/linux-2.6/drivers/mtd/nand/omap2.c:1980: undefined reference to `elm_config' /work/linux-2.6/drivers/mtd/nand/omap2.c:1927: undefined reference to `elm_config' drivers/built-in.o: In function `omap_elm_correct_data': /work/linux-2.6/drivers/mtd/nand/omap2.c:1444: undefined reference to `elm_decode_bch_error_page' Signed-off-by: Ezequiel Garcia ezequ...@vanguardiasur.com.ar --- drivers/mtd/nand/Kconfig | 8 +++- drivers/mtd/nand/Makefile | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index f1cf503..12e8ee8 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -96,7 +96,7 @@ config MTD_NAND_OMAP2 config MTD_NAND_OMAP_BCH depends on MTD_NAND_OMAP2 - tristate Support hardware based BCH error correction + bool Support hardware based BCH error correction default n select BCH help @@ -106,6 +106,12 @@ config MTD_NAND_OMAP_BCH legacy OMAP families like OMAP2xxx, OMAP3xxx do not have ELM engine so they should not enable this config symbol. +config MTD_NAND_OMAP_BCH_BUILD + tristate + depends on MTD_NAND_OMAP2 + default m if MTD_NAND_OMAP2=m MTD_NAND_OMAP_BCH + default y if MTD_NAND_OMAP2=y MTD_NAND_OMAP_BCH + config MTD_NAND_IDS tristate diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 4bcdeb0..3580188 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -27,7 +27,7 @@ obj-$(CONFIG_MTD_NAND_NDFC) += ndfc.o obj-$(CONFIG_MTD_NAND_ATMEL) += atmel_nand.o obj-$(CONFIG_MTD_NAND_GPIO)+= gpio.o obj-$(CONFIG_MTD_NAND_OMAP2) += omap2_nand.o -obj-$(CONFIG_MTD_NAND_OMAP_BCH)+= omap_elm.o +obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD) += omap_elm.o obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270_nand.o obj-$(CONFIG_MTD_NAND_PXA3xx) += pxa3xx_nand.o obj-$(CONFIG_MTD_NAND_TMIO)+= tmio_nand.o -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
