Skip to contentSkip to conversation

char driver for Blackfin on-chip OTP memory

PATCH In review Bryan Wu 4 participants 9 messages Mar 26, 2008 Char and misc drivers

lore
Share & tools
lore

Bryan Wu

author
#0 lore raw

PATCH 1/1 char driver for Blackfin on-chip OTP memory

From: Mike Frysinger <[email protected]>

initial char driver for otp memory
(only read supported atm ... needs real examples/docs for write support)

Signed-off-by Mike Frysinger <[email protected]>
Signed-off-by Bryan Wu <[email protected]>
---
 drivers/char/Kconfig    |   28 +++++++
 drivers/char/Makefile   |    1 +
 drivers/char/bfin-otp.c |  189 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 218 insertions(+), 0 deletions(-)
 create mode 100644 drivers/char/bfin-otp.c
drivers/char/Kconfig +28 -0
index dd17d06..989dade 100644
@@ -495,6 +495,34 @@ config BRIQ_PANEL
 
 	  It's safe to say N here.
 
+config BFIN_OTP
+	tristate "Blackfin On-Chip OTP Memory Support"
+	depends on BLACKFIN && (BF52x || BF54x)
+	default y
+	help
+	  If you say Y here, you will get support for a character device
+	  interface into the One Time Programmable memory pages that are
+	  stored on the Blackfin processor.  This will not get you access
+	  to the secure memory pages however.  You will need to write your
+	  own secure code and reader for that.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called bfin-otp.
+
+	  If unsure, it is safe to say Y.
+
+config BFIN_OTP_WRITE_ENABLE
+	bool "Enable writing support of OTP pages"
+	depends on BFIN_OTP
+	default n
+	help
+	  If you say Y here, you will enable support for writing of the
+	  OTP pages.  This is dangerous by nature as you can only program
+	  the pages once, so only enable this option when you actually
+	  need it so as to not inadvertently clobber data.
+
+	  If unsure, say N.
+
 config PRINTER
 	tristate "Parallel printer support"
 	depends on PARPORT
drivers/char/Makefile +1 -0
index e087ec1..06e6754 100644
@@ -59,6 +59,7 @@ obj-$(CONFIG_VIOTAPE)		+= viotape.o
 obj-$(CONFIG_HVCS)		+= hvcs.o
 obj-$(CONFIG_SGI_MBCS)		+= mbcs.o
 obj-$(CONFIG_BRIQ_PANEL)	+= briq_panel.o
+obj-$(CONFIG_BFIN_OTP)		+= bfin-otp.o
 
 obj-$(CONFIG_PRINTER)		+= lp.o
 obj-$(CONFIG_TIPAR)		+= tipar.o
drivers/char/bfin-otp.c new file +189 -0

189 lines not on this page · raw message

Signature · 1 line
1.5.4.3
Acked-by on v1 1/1
#1 lore raw
Quoted 7 lines · Bryan Wu

From: Mike Frysinger <[email protected]>

initial char driver for otp memory
(only read supported atm ... needs real examples/docs for write support)

Signed-off-by Mike Frysinger <[email protected]>
Signed-off-by Bryan Wu <[email protected]>

Nice,

Acked-by Jiri Slaby <[email protected]>
Quoted 5 lines
drivers/char/bfin-otp.c new file +0 -0
new file mode 100644
index 0000000..2edc284

[...]

drivers/char/bfin-otp.c line 166

Quoted 13 lines
drivers/char/bfin-otp.c new file +13 -0
+static int __init bfin_otp_init(void)
+{
+	int ret;
+
+	stampit();
+
+	ret = misc_register(&bfin_otp_misc_device);
+	if (ret) {
+		pr_init(KERN_ERR PFX "unable to register a misc device\n");
+		return ret;
+	}
+
+	pr_init(KERN_INFO PFX "initialized\n");

except the fact, that pr_init definition seems to be broken. Its defined __fmt is const, so it should reside in .init.rodata (__initconst), not .init.data (__initdata).

#2 lore raw
Quoted 11 lines · Jiri Slaby
Quoted 10 lines · Bryan Wu

From: Mike Frysinger <[email protected]>

drivers/char/bfin-otp.c new file +9 -0
+static int __init bfin_otp_init(void)
+{
+     int ret;
+
+     stampit();
+
+     ret = misc_register(&bfin_otp_misc_device);
+     if (ret) {
+             pr_init(KERN_ERR PFX "unable to register a misc device\n");
Quoted 8 lines

drivers/char/bfin-otp.c line 166

Quoted 4 lines
drivers/char/bfin-otp.c new file +4 -0
+             return ret;
+     }
+
+     pr_init(KERN_INFO PFX "initialized\n");

except the fact, that pr_init definition seems to be broken. Its defined __fmt is const, so it should reside in .init.rodata (__initconst), not .init.data (__initdata).

i dont see an __initconst macro anywhere ?

also, it isnt "broken" in the sense that it "doesnt work" ;) -mike

#3 lore raw
Quoted 17 lines · Mike Frysinger
Quoted 16 lines · Jiri Slaby
Quoted 14 lines · Bryan Wu

From: Mike Frysinger <[email protected]>

drivers/char/bfin-otp.c new file +13 -0
+static int __init bfin_otp_init(void)
+{
+     int ret;
+
+     stampit();
+
+     ret = misc_register(&bfin_otp_misc_device);
+     if (ret) {
+             pr_init(KERN_ERR PFX "unable to register a misc device\n");
+             return ret;
+     }
+
+     pr_init(KERN_INFO PFX "initialized\n");
Quoted 5 lines
Quoted 3 lines

except the fact, that pr_init definition seems to be broken. Its defined __fmt is const, so it should reside in .init.rodata (__initconst), not .init.data (__initdata).

i dont see an __initconst macro anywhere ?

It's in include/linux/init.h as of 2.6.25-rc2.

Quoted 1 line

also, it isnt "broken" in the sense that it "doesnt work" ;)

Gcc simply doesn't allow that, you likely got 2 sections with init.data, with and without readonly bit set, which is wrong, and it seems, that on some systems (debian) you won't even compile it.

#4 lore raw
Quoted 18 lines · Jiri Slaby
Quoted 17 lines · Mike Frysinger
Quoted 16 lines · Jiri Slaby
Quoted 14 lines · Bryan Wu

From: Mike Frysinger <[email protected]>

drivers/char/bfin-otp.c new file +13 -0
+static int __init bfin_otp_init(void)
+{
+     int ret;
+
+     stampit();
+
+     ret = misc_register(&bfin_otp_misc_device);
+     if (ret) {
+             pr_init(KERN_ERR PFX "unable to register a misc device\n");
+             return ret;
+     }
+
+     pr_init(KERN_INFO PFX "initialized\n");
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

except the fact, that pr_init definition seems to be broken. Its defined __fmt is const, so it should reside in .init.rodata (__initconst), not .init.data (__initdata).

i dont see an __initconst macro anywhere ?

It's in include/linux/init.h as of 2.6.25-rc2.

driver was developed against ~2.6.21 and that isnt even in 2.6.24, that's why the drivers dont currently use it

Quoted 5 lines
Quoted 1 line

also, it isnt "broken" in the sense that it "doesnt work" ;)

Gcc simply doesn't allow that, you likely got 2 sections with init.data, with and without readonly bit set, which is wrong, and it seems, that on some systems (debian) you won't even compile it.

gcc-4.1.2/binutils-2.17 doesnt have a problem compiling/linking the code in question. and i imagine i would have noticed a build/link failure long before testing it on actual hardware.

Bryan can make the changes before pushing for 2.6.25 inclusion ... otherwise they'll just stay that way until 2.6.25 gets released, and then i'll do it.
-mike

#5 lore raw
Quoted 10 lines
Quoted 6 lines
Quoted 2 lines

also, it isnt "broken" in the sense that it "doesnt work" ;)

Gcc simply doesn't allow that, you likely got 2 sections with init.data, with and without readonly bit set, which is wrong, and it seems, that on some systems (debian) you won't even compile it.

gcc-4.1.2/binutils-2.17 doesnt have a problem compiling/linking the code in question. and i imagine i would have noticed a build/link failure long before testing it on actual hardware.

target arch dependent. Try with 64 bit powerpc.

Sam

#6 lore raw
Quoted 11 lines · Sam Ravnborg
Quoted 9 lines
Quoted 5 lines
Quoted 1 line

also, it isnt "broken" in the sense that it "doesnt work" ;)

Gcc simply doesn't allow that, you likely got 2 sections with init.data, with and without readonly bit set, which is wrong, and it seems, that on some systems (debian) you won't even compile it.

gcc-4.1.2/binutils-2.17 doesnt have a problem compiling/linking the code in question. and i imagine i would have noticed a build/link failure long before testing it on actual hardware.

target arch dependent. Try with 64 bit powerpc.

this code appears in drivers that are for Blackfin-specific pieces of hardware. it may fail to build on some-random-arch-that-isnt-Blackfin, but if it works for Blackfin, what do the other arches realistically matter ?

yes, the code will get changed as suggested as it is the right thing to do, it just isnt a real problem today.
-mike

#7 lore raw
Quoted 13 lines · Mike Frysinger
Quoted 11 lines · Sam Ravnborg
Quoted 9 lines
Quoted 5 lines
Quoted 1 line

also, it isnt "broken" in the sense that it "doesnt work" ;)

Gcc simply doesn't allow that, you likely got 2 sections with init.data, with and without readonly bit set, which is wrong, and it seems, that on some systems (debian) you won't even compile it.

gcc-4.1.2/binutils-2.17 doesnt have a problem compiling/linking the code in question. and i imagine i would have noticed a build/link failure long before testing it on actual hardware.

target arch dependent. Try with 64 bit powerpc.

Quoted 7 lines

this code appears in drivers that are for Blackfin-specific pieces of hardware. it may fail to build on some-random-arch-that-isnt-Blackfin, but if it works for Blackfin, what do the other arches realistically matter ?

yes, the code will get changed as suggested as it is the right thing to do, it just isnt a real problem today.

It is a real problem for build monkeys (like me sometimes) if an allyesconfig suddenly does not build.

If this driver can be build for other than blackfin I do not know.

Sam

#8 lore raw
Quoted 19 lines · Sam Ravnborg
Quoted 18 lines · Mike Frysinger
Quoted 11 lines · Sam Ravnborg
Quoted 9 lines
Quoted 5 lines
Quoted 1 line

also, it isnt "broken" in the sense that it "doesnt work" ;)

Gcc simply doesn't allow that, you likely got 2 sections with init.data, with and without readonly bit set, which is wrong, and it seems, that on some systems (debian) you won't even compile it.

gcc-4.1.2/binutils-2.17 doesnt have a problem compiling/linking the code in question. and i imagine i would have noticed a build/link failure long before testing it on actual hardware.

target arch dependent. Try with 64 bit powerpc.

this code appears in drivers that are for Blackfin-specific pieces of hardware. it may fail to build on some-random-arch-that-isnt-Blackfin, but if it works for Blackfin, what do the other arches realistically matter ?

Quoted 5 lines
Quoted 2 lines

yes, the code will get changed as suggested as it is the right thing to do, it just isnt a real problem today.

It is a real problem for build monkeys (like me sometimes) if an allyesconfig suddenly does not build.

it isnt a problem at all considering the next part:

Quoted 2 lines

If this driver can be build for other than blackfin I do not know.

it's a device driver for hardware on the Blackfin processor itself. the Kconfig has "depends on BLACKFIN". it just aint gonna be an issue :).
-mike