Patchwork [U-Boot] Reg. CFI flash_init and hardware write protected devices

login
register
mail settings
Submitter Frank Svendsbøe
Date June 1, 2011, 2:33 p.m.
Message ID <BANLkTi=4+Jh47VRQ=MwZS607foxhrv5OJQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/98198/
State Not Applicable
Headers show

Comments

Frank Svendsbøe - June 1, 2011, 2:33 p.m.
Hi Stefan,

On Tue, May 31, 2011 at 4:37 PM, Stefan Roese <sr@denx.de> wrote:
> Hi Frank,
>
> On Tuesday 31 May 2011 15:55:56 Frank Svendsbøe wrote:
>> > Understood. But why don't you disable write-protection when you first
>> > call flash_init()? And enable the write-protection after the chip is
>> > correctly detected?
>>
>> Simply because disabling write-protection is not impossible after
>> installation. Our device will be located 3000m below sea level.
>
> I see.
>

Hmm.. then you read my mind :) I meant to say "is not possible" and not "is not
impossible" :)

>> As I explained Mike Frysinger, the write-protection settings is not
>> controlled by the PPC device running U-Boot. We can enable
>> write-protection in the lab (by setting a jumper), but not write software.
>>

.. and here I meant to say "explained to", and "but not via software".


>> Two ways/levels: 1) A hardware jumper on the factory default flash. 2)
>> On the non-factory default flash, write protection is enabled/disabled
>> by an FPGA and implicitly and AVR. To make it short, we cannot
>> change protection scheme from U-Boot (but we can via an SPI driver I
>> wrote for Linux).
>

.. and yet another self-correction: s/and implicitly and/and implicitly an/
.. guess I had too much coffee yesterday :)

> Theoretically also possible with U-Boot. But I understand that you don't want
> to do this.
>

True. We could write a driver for U-Boot that could access the AVR and
command the FPGA to
disable write protection. But in the case for the "default factory
flash" where the flash
write protection is enabled by a demounted jumper, and cannot be
modified after installation,
CFI probing wouldn't work anyway. So I think the use for such a driver
would be limited.

Note that for normal operation, our system is running from a
non-hardware protected flash. But
even in this mode, we must command the AVR to enable write access
before we're permitted
to write to it. This is one of the features supported by the SPI
driver running in GNU/Linux.

In addition to this, we have the usual CFI/MTD protection scheme where
the filesystems
are mounted as read-only, etc.

> <snip>
>
>> > Why don't you think that you can't access the original function if it's
>> > defined as a weak default? This should work just fine, see for example
>> > ft_board_setup() in arch/powerpc/cpu/ppc4xx/fdt.c:
>> >
>> > void __ft_board_setup(void *blob, bd_t *bd)
>> > {
>> >        ...
>> > }
>> > void ft_board_setup(void *blob, bd_t *bd) __attribute__((weak,
>> > alias("__ft_board_setup")));
>> >
>> >
>> > And then this weak default is overridden and still referenced in
>> > board/amcc/canyonlands/canyonlands.c:
>> >
>> > void ft_board_setup(void *blob, bd_t *bd)
>> > {
>> >        __ft_board_setup(blob, bd);
>> >        ...
>> >
>> >
>> > So no need for this ifdef in the common CFI driver. Or am I missing
>> > something here?
>>
>> Oh. I didn't knew I could access the function that was overridden by the
>> weak attribute. I guess that's the alias is for right?
>
> Yep.
>
>> If both can be
>> called, I'm happy to remove the ifdef.
>>
>> I'll test that tomorrow and provide a patch if it works.
>
> Good luck...
>

Thanks, this worked for me. Is it ok for you too?

 #if defined(CONFIG_ENV_IS_IN_FLASH) ||
defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >=
CONFIG_SYS_FLASH_BASE)
@@ -2151,7 +2152,8 @@ void flash_protect_default(void)
 #endif
 }

-unsigned long flash_init (void)
+unsigned long flash_init(void) __attribute__((weak, alias("__flash_init")));
+unsigned long __flash_init (void)
 {
 	unsigned long size = 0;
 	int i;
Stefan Roese - June 1, 2011, 3:34 p.m.
Hi Frank,

On Wednesday 01 June 2011 16:33:14 Frank Svendsbøe wrote:
> >> Simply because disabling write-protection is not impossible after
> >> installation. Our device will be located 3000m below sea level.
> > 
> > I see.
> 
> Hmm.. then you read my mind :) I meant to say "is not possible" and not "is
> not impossible" :)

Yep. I read so fast, that I didn't catch the misspelled words. ;)
 
> Thanks, this worked for me. Is it ok for you too?
> 
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 6039e1f..99360af 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -176,6 +176,7 @@ u64 flash_read64(void *addr)__attribute__((weak,
> alias("__flash_read64")));
>  #define flash_read64	__flash_read64
>  #endif
> 
> +

Don't add this empty line.

>  /*-----------------------------------------------------------------------
>   */
>  #if defined(CONFIG_ENV_IS_IN_FLASH) ||
> defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >=
> CONFIG_SYS_FLASH_BASE)
> @@ -2151,7 +2152,8 @@ void flash_protect_default(void)
>  #endif
>  }
> 
> -unsigned long flash_init (void)
> +unsigned long flash_init(void) __attribute__((weak,
> alias("__flash_init"))); +unsigned long __flash_init (void)
>  {
>  	unsigned long size = 0;
>  	int i;

Looks good. I have no objections. Please go ahead and send this as a "proper" 
patch.

Best regards,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
Frank Svendsbøe - June 1, 2011, 4:59 p.m.
On Wed, Jun 1, 2011 at 5:34 PM, Stefan Roese <sr@denx.de> wrote:
> Hi Frank,
>
> On Wednesday 01 June 2011 16:33:14 Frank Svendsbøe wrote:
>> >> Simply because disabling write-protection is not impossible after
>> >> installation. Our device will be located 3000m below sea level.
>> >
>> > I see.
>>
>> Hmm.. then you read my mind :) I meant to say "is not possible" and not "is
>> not impossible" :)
>
> Yep. I read so fast, that I didn't catch the misspelled words. ;)
>
>> Thanks, this worked for me. Is it ok for you too?
>>
>> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
>> index 6039e1f..99360af 100644
>> --- a/drivers/mtd/cfi_flash.c
>> +++ b/drivers/mtd/cfi_flash.c
>> @@ -176,6 +176,7 @@ u64 flash_read64(void *addr)__attribute__((weak,
>> alias("__flash_read64")));
>>  #define flash_read64 __flash_read64
>>  #endif
>>
>> +
>
> Don't add this empty line.
>

Thanks for spotting this. I'll remove this of course.

>>  /*-----------------------------------------------------------------------
>>   */
>>  #if defined(CONFIG_ENV_IS_IN_FLASH) ||
>> defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >=
>> CONFIG_SYS_FLASH_BASE)
>> @@ -2151,7 +2152,8 @@ void flash_protect_default(void)
>>  #endif
>>  }
>>
>> -unsigned long flash_init (void)
>> +unsigned long flash_init(void) __attribute__((weak,
>> alias("__flash_init"))); +unsigned long __flash_init (void)
>>  {
>>       unsigned long size = 0;
>>       int i;
>
> Looks good. I have no objections. Please go ahead and send this as a "proper"
> patch.
>

Will do (as soon as I get back to work).

Best regards,
Frank.
Frank Svendsbøe - June 23, 2011, 1:50 p.m.
On Wed, Jun 1, 2011 at 6:59 PM, Frank Svendsbøe
<frank.svendsboe@gmail.com> wrote:
> On Wed, Jun 1, 2011 at 5:34 PM, Stefan Roese <sr@denx.de> wrote:
>> Hi Frank,
>>
>> On Wednesday 01 June 2011 16:33:14 Frank Svendsbøe wrote:
>>> >> Simply because disabling write-protection is not impossible after
>>> >> installation. Our device will be located 3000m below sea level.
>>> >
>>> > I see.
>>>
>>> Hmm.. then you read my mind :) I meant to say "is not possible" and not "is
>>> not impossible" :)
>>
>> Yep. I read so fast, that I didn't catch the misspelled words. ;)
>>
>>> Thanks, this worked for me. Is it ok for you too?
>>>
>>> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
>>> index 6039e1f..99360af 100644
>>> --- a/drivers/mtd/cfi_flash.c
>>> +++ b/drivers/mtd/cfi_flash.c
>>> @@ -176,6 +176,7 @@ u64 flash_read64(void *addr)__attribute__((weak,
>>> alias("__flash_read64")));
>>>  #define flash_read64 __flash_read64
>>>  #endif
>>>
>>> +
>>
>> Don't add this empty line.
>>
>
> Thanks for spotting this. I'll remove this of course.
>
>>>  /*-----------------------------------------------------------------------
>>>   */
>>>  #if defined(CONFIG_ENV_IS_IN_FLASH) ||
>>> defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >=
>>> CONFIG_SYS_FLASH_BASE)
>>> @@ -2151,7 +2152,8 @@ void flash_protect_default(void)
>>>  #endif
>>>  }
>>>
>>> -unsigned long flash_init (void)
>>> +unsigned long flash_init(void) __attribute__((weak,
>>> alias("__flash_init"))); +unsigned long __flash_init (void)
>>>  {
>>>       unsigned long size = 0;
>>>       int i;
>>
>> Looks good. I have no objections. Please go ahead and send this as a "proper"
>> patch.
>>
>
> Will do (as soon as I get back to work).
>
> Best regards,
> Frank.
>

Hi Stefan,
A few weeks ago, I applied this change and everything worked fine in
U-Boot. However, I didn't succeed doing the same hack in Linux. I've
done some hardcoding experiments in drivers/mtd/chips/cfi_probe.c, but
so far no success. After reading some CFI manuals, it seems CFI
will/should use bus write operations even during CFI read operations.
Can you confirm this? Then my next question is how come U-Boot read
operations works by just hardcoding the flash geometry. It seems
U-Boot utilize just plain localbus accesses to read flash data, ie.
not the "CFI protocol". What did I miss here.. ?

Now I'm considering two options: 1) Either write a custom flash map
driver, and access the flash the same way U-Boot does, or 2) try to
convince the FPGA designers that our way of forcing write protection
is not according the the CFI specifications, and should be done
otherwise. What do you think is the best solution?

Is there anybody else out there supporting hardware write protection
on CFI devices?

Best regards,
Frank
Wolfgang Denk - June 23, 2011, 3:21 p.m.
Dear Frank,

In message <BANLkTinFMA7FN-BD7z2-ZYAT+-ubWn7S9Q@mail.gmail.com> you wrote:
>
> A few weeks ago, I applied this change and everything worked fine in
> U-Boot. However, I didn't succeed doing the same hack in Linux. I've
> done some hardcoding experiments in drivers/mtd/chips/cfi_probe.c, but
> so far no success. After reading some CFI manuals, it seems CFI
> will/should use bus write operations even during CFI read operations.

What exactly do you mean by "CFI read operations"?

> Can you confirm this? Then my next question is how come U-Boot read
> operations works by just hardcoding the flash geometry. It seems
> U-Boot utilize just plain localbus accesses to read flash data, ie.
> not the "CFI protocol". What did I miss here.. ?

It seems you misunderstand the different modes.  In command mode, you
will use the specific command sequences defined in the CFI protocol to
talk to the controller poart of the flash chip. In this mode, you can
read for example query information, flash geometry data, status
information, etc.  Yes, any such command sequence includes write
operations to the flash device, even when you actually want to read
some specific data.  In data mode, flash is working just as normal
real-only memory. No specific protocol is used, you just use plain
read accesses to read the user data programmed into the flash chip.

Any kind of probing, auto-identification etc. of the flash chip will
need to use CFI command sequences, and thus write access to the
device.  As long as you just want to read the data or execute code
from the flash no write accesses are needed.

> Now I'm considering two options: 1) Either write a custom flash map
> driver, and access the flash the same way U-Boot does, or 2) try to
> convince the FPGA designers that our way of forcing write protection
> is not according the the CFI specifications, and should be done
> otherwise. What do you think is the best solution?

Linux can also read from ROM memory. You just have to make sure that
you do not try to run the normal CFI flash drivers on your flash
devices - these will not work, because they will try to probe the
flash chips.

> Is there anybody else out there supporting hardware write protection
> on CFI devices?

CFI flash chips with the level of write protection you implemented
have to be dealt with in the same way as other ROM memory.  Normal CFI
driver are not appropriate here.

Best regards,

Wolfgang Denk
Frank Svendsbøe - June 23, 2011, 4:15 p.m.
On Thu, Jun 23, 2011 at 5:21 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Frank,
>
> In message <BANLkTinFMA7FN-BD7z2-ZYAT+-ubWn7S9Q@mail.gmail.com> you wrote:
>>
>> A few weeks ago, I applied this change and everything worked fine in
>> U-Boot. However, I didn't succeed doing the same hack in Linux. I've
>> done some hardcoding experiments in drivers/mtd/chips/cfi_probe.c, but
>> so far no success. After reading some CFI manuals, it seems CFI
>> will/should use bus write operations even during CFI read operations.
>
> What exactly do you mean by "CFI read operations"?
>

Good question. When reading Intels CFI manual I was referring to
something called "read array" which used bus write commands. I guess
normal byte/word read access don't utilize this then.

>> Can you confirm this? Then my next question is how come U-Boot read
>> operations works by just hardcoding the flash geometry. It seems
>> U-Boot utilize just plain localbus accesses to read flash data, ie.
>> not the "CFI protocol". What did I miss here.. ?
>
> It seems you misunderstand the different modes.  In command mode, you
> will use the specific command sequences defined in the CFI protocol to
> talk to the controller poart of the flash chip. In this mode, you can
> read for example query information, flash geometry data, status
> information, etc.  Yes, any such command sequence includes write
> operations to the flash device, even when you actually want to read
> some specific data.  In data mode, flash is working just as normal
> real-only memory. No specific protocol is used, you just use plain
> read accesses to read the user data programmed into the flash chip.
>

Right, and thanks for the explanation. What I've done to U-Boot is
bypass the probing which uses the command mode and thus avoid getting
problems with bus write accesses when flash protection is on.

> Any kind of probing, auto-identification etc. of the flash chip will
> need to use CFI command sequences, and thus write access to the
> device.  As long as you just want to read the data or execute code
> from the flash no write accesses are needed.
>
>> Now I'm considering two options: 1) Either write a custom flash map
>> driver, and access the flash the same way U-Boot does, or 2) try to
>> convince the FPGA designers that our way of forcing write protection
>> is not according the the CFI specifications, and should be done
>> otherwise. What do you think is the best solution?
>
> Linux can also read from ROM memory. You just have to make sure that
> you do not try to run the normal CFI flash drivers on your flash
> devices - these will not work, because they will try to probe the
> flash chips.
>

Yes, well I tried to fool Linux's CFI driver too, but no luck so far.
It was after this I started reading about CFI and came to the (wrong)
conclusion that CFI uses bus write operations during read accesses
too.

>> Is there anybody else out there supporting hardware write protection
>> on CFI devices?
>
> CFI flash chips with the level of write protection you implemented
> have to be dealt with in the same way as other ROM memory.  Normal CFI
> driver are not appropriate here.
>

Ok. So you recommend I stop "hacking" the CFI probing and instead
write a custom flash map driver?

Best regards,
Frank
Wolfgang Denk - June 23, 2011, 5:55 p.m.
Dear Frank,

In message <BANLkTi=p9gdOGOETWojRqkmrxXM7Do_tBg@mail.gmail.com> you wrote:
>
> > CFI flash chips with the level of write protection you implemented
> > have to be dealt with in the same way as other ROM memory. =A0Normal CFI
> > driver are not appropriate here.
> 
> Ok. So you recommend I stop "hacking" the CFI probing and instead
> write a custom flash map driver?

No.  Never write new drivers when you can use existing ones :-)

Describe the flash memory as "mtd-ram" (instead of "cfi-flash") in
the device tree and select the physmap driver in your kernel config.

Best regards,

Wolfgang Denk
Frank Svendsbøe - June 23, 2011, 7:05 p.m.
On Thu, Jun 23, 2011 at 7:55 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Frank,
>
> In message <BANLkTi=p9gdOGOETWojRqkmrxXM7Do_tBg@mail.gmail.com> you wrote:
>>
>> > CFI flash chips with the level of write protection you implemented
>> > have to be dealt with in the same way as other ROM memory. =A0Normal CFI
>> > driver are not appropriate here.
>>
>> Ok. So you recommend I stop "hacking" the CFI probing and instead
>> write a custom flash map driver?
>
> No.  Never write new drivers when you can use existing ones :-)
>
> Describe the flash memory as "mtd-ram" (instead of "cfi-flash") in
> the device tree and select the physmap driver in your kernel config.
>

Oh. I will try that. Thanks a lot for this tip :)

.. and sorry for asking about Linux related topics on the U-Boot ML.

Best regards,
Frank
Frank Svendsbøe - June 24, 2011, 1:59 p.m.
>> Ok. So you recommend I stop "hacking" the CFI probing and instead
>> write a custom flash map driver?
>
> No.  Never write new drivers when you can use existing ones :-)
>
> Describe the flash memory as "mtd-ram" (instead of "cfi-flash") in
> the device tree and select the physmap driver in your kernel config.
>

Hi Wolfgang, I did as you recommended and replaced cfi-flash with
mtd-ram in the device tree. I also defined CONFIG_MTD_PHYSMAP_OF,
CONFIG_MTD_MTDRAM, CONFIG_MTDRAM_TOTAL_SIZE according to our
specifications. The default erase-size was 128k, which is what we have
too, so I didn't touch that. Now when I boot the kernel recognizes all
the partitions I've defined in the dts. But, when mounting a
jffs2-filesystem, it ends with a jffs2_scan_eraseblock(): Magic
bitmask 0x1985 not found...

Do you have any other tips?

When working with this, I realised that if I could get it to work we'd
still might have a problem. You see, we need write access for one of
the flashes when upgrading software. We can't treat this as a simple
ROM. So do we need CFI working in order to set the device into
write-mode, erase flash sectors, etc.? Or do mtd-ram handle flash
write operations too?

In theory, I guess I could unmount the root fs, unload the mtd module,
insert the cfi-flash module, mount the filesystem, then write, etc..
But isn't that harder than write a custom map driver?

Best regards,
Frank
Wolfgang Denk - June 24, 2011, 2:26 p.m.
Dear =?ISO-8859-1?Q?Frank_Svendsb=F8e?=,

In message <BANLkTimt0LYXr-B5PAr1ONEaEoWKCyEHbw@mail.gmail.com> you wrote:
>
> Hi Wolfgang, I did as you recommended and replaced cfi-flash with
> mtd-ram in the device tree. I also defined CONFIG_MTD_PHYSMAP_OF,
> CONFIG_MTD_MTDRAM, CONFIG_MTDRAM_TOTAL_SIZE according to our
> specifications. The default erase-size was 128k, which is what we have
> too, so I didn't touch that. Now when I boot the kernel recognizes all
> the partitions I've defined in the dts. But, when mounting a
> jffs2-filesystem, it ends with a jffs2_scan_eraseblock(): Magic
> bitmask 0x1985 not found...
> 
> Do you have any other tips?

Difficult to speculate - I don't know your hardware (eventually you
have two 16 bit flash chips in parallel to build a 32 bit bus, and
have to double the chip's erase block size?), and I don;t know how you
created the JFFS2 file system.

Are you sure you want to use JFFS2?  UBIFS is considered to be a
better choice these days...

> When working with this, I realised that if I could get it to work we'd
> still might have a problem. You see, we need write access for one of
> the flashes when upgrading software. We can't treat this as a simple
> ROM. So do we need CFI working in order to set the device into
> write-mode, erase flash sectors, etc.? Or do mtd-ram handle flash
> write operations too?

mtd-ram provides a pure memory interface, i. e. you cannot use this to
erase or program any blocks in a CFI flash device.  To do so, you need
the CFI driver.

> In theory, I guess I could unmount the root fs, unload the mtd module,
> insert the cfi-flash module, mount the filesystem, then write, etc..

Yes, or you could start with the CFI driver in the first place.

> But isn't that harder than write a custom map driver?

I consider your chances to get such a customdriver into mainline to be
epsilon.  And you don't really want to use any out of tree drivers.


Best regards,

Wolfgang Denk
Frank Svendsbøe - June 24, 2011, 7:58 p.m.
On Fri, Jun 24, 2011 at 4:26 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear =?ISO-8859-1?Q?Frank_Svendsb=F8e?=,
>
> In message <BANLkTimt0LYXr-B5PAr1ONEaEoWKCyEHbw@mail.gmail.com> you wrote:
>>
>> Hi Wolfgang, I did as you recommended and replaced cfi-flash with
>> mtd-ram in the device tree. I also defined CONFIG_MTD_PHYSMAP_OF,
>> CONFIG_MTD_MTDRAM, CONFIG_MTDRAM_TOTAL_SIZE according to our
>> specifications. The default erase-size was 128k, which is what we have
>> too, so I didn't touch that. Now when I boot the kernel recognizes all
>> the partitions I've defined in the dts. But, when mounting a
>> jffs2-filesystem, it ends with a jffs2_scan_eraseblock(): Magic
>> bitmask 0x1985 not found...
>>
>> Do you have any other tips?
>
> Difficult to speculate - I don't know your hardware (eventually you
> have two 16 bit flash chips in parallel to build a 32 bit bus, and
> have to double the chip's erase block size?), and I don;t know how you
> created the JFFS2 file system.
>

Yes I know, and I don't expect you to find the answer based on so
little data. FYI: We have two flash chips yes, but not to build a
32-bit bus, we have two for redundancy issues (one is storing a
factory default version). 16 bits databus and each sector is 64k
words, ie. 128k bytes. I created the jffs2 images using ELDKs
mkfs.jffs2. Thanks for providing me this tool :)

> Are you sure you want to use JFFS2?  UBIFS is considered to be a
> better choice these days...
>

No, not sure. I've got two reasons for staying with JFFS2. 1) We
normally don't write to flash except when upgrading, so flash
performance/duration isn't very critical 2) I discovered UBIFS after I
had JFFS2 working and haven't seen the real need for it yet.

>> When working with this, I realised that if I could get it to work we'd
>> still might have a problem. You see, we need write access for one of
>> the flashes when upgrading software. We can't treat this as a simple
>> ROM. So do we need CFI working in order to set the device into
>> write-mode, erase flash sectors, etc.? Or do mtd-ram handle flash
>> write operations too?
>
> mtd-ram provides a pure memory interface, i. e. you cannot use this to
> erase or program any blocks in a CFI flash device.  To do so, you need
> the CFI driver.
>

Right. mtd-ram is thus a dead-end solution for our purpose. We need to
be able to write in some specific cases.

>> In theory, I guess I could unmount the root fs, unload the mtd module,
>> insert the cfi-flash module, mount the filesystem, then write, etc..
>
> Yes, or you could start with the CFI driver in the first place.
>

That's what I did before you started interrupting me about mtd-ram...
No, just kidding :-)

Did you mean I should adapt the general CFI driver to support our
configuration, or... Based on what you say below I'm not sure what you
actually mean.

>> But isn't that harder than write a custom map driver?
>
> I consider your chances to get such a customdriver into mainline to be
> epsilon.  And you don't really want to use any out of tree drivers.
>

Me too. Small chances. So we have two options I guess: 1) Ditch
hardware write protection and use CFI as it is supposed to be used, or
2) write a custom out-of-tree driver. What would you go for?

I'll consider if and how to get things into mainline once we have
things working as we want. If my port isn't accepted I will at least
learn why. Maybe the next will :)

Best regards,
Frank
Wolfgang Denk - June 24, 2011, 8:26 p.m.
Dear Frank,

In message <BANLkTindk4f2An2VqhSftPcGuvsEqLY7wQ@mail.gmail.com> you wrote:
>
> > Difficult to speculate - I don't know your hardware (eventually you
> > have two 16 bit flash chips in parallel to build a 32 bit bus, and
> > have to double the chip's erase block size?), and I don;t know how you
> > created the JFFS2 file system.
...
> factory default version). 16 bits databus and each sector is 64k
> words, ie. 128k bytes. I created the jffs2 images using ELDKs
> mkfs.jffs2. Thanks for providing me this tool :)

You need to provide the correct parameters to mkfs.jffs2...

> No, not sure. I've got two reasons for staying with JFFS2. 1) We
> normally don't write to flash except when upgrading, so flash
> performance/duration isn't very critical 2) I discovered UBIFS after I

Hm... JFFS2 is probaly not a good choice anyway.  On NOR, where you
don't have to deal with bad blocks, you might use ext2 or cramfs or
... - all of them are more efficient than JFFS2 then.

> had JFFS2 working and haven't seen the real need for it yet.

Mount time?

> Did you mean I should adapt the general CFI driver to support our
> configuration, or... Based on what you say below I'm not sure what you
> actually mean.

You have two different use cases that require (if we don't want to use
a proprietary driver) two different, mutually exclusive drivers.  I
would load the driver I need as module, and unload it if I need the
other one. This is pretty simple in principle, but a bit ore of a
challange in your case as you use this as a root file system
(including to load the modules from).

> Me too. Small chances. So we have two options I guess: 1) Ditch
> hardware write protection and use CFI as it is supposed to be used, or
> 2) write a custom out-of-tree driver. What would you go for?

If this is an option: 1)

Best regards,

Wolfgang Denk
Frank Svendsbøe - June 24, 2011, 9:12 p.m.
On Fri, Jun 24, 2011 at 10:26 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Frank,
>
> In message <BANLkTindk4f2An2VqhSftPcGuvsEqLY7wQ@mail.gmail.com> you wrote:
>>
>> > Difficult to speculate - I don't know your hardware (eventually you
>> > have two 16 bit flash chips in parallel to build a 32 bit bus, and
>> > have to double the chip's erase block size?), and I don;t know how you
>> > created the JFFS2 file system.
> ...
>> factory default version). 16 bits databus and each sector is 64k
>> words, ie. 128k bytes. I created the jffs2 images using ELDKs
>> mkfs.jffs2. Thanks for providing me this tool :)
>
> You need to provide the correct parameters to mkfs.jffs2...
>

Of course, but I don't see how changing from CFI to raw memory
accesses on target change the way the jffs2 image should be created.
What didn't I understand?

I'm currently using:
mkfs.jss2 -U -b -d /${input} -e 0x20000 -o ${output}

>> No, not sure. I've got two reasons for staying with JFFS2. 1) We
>> normally don't write to flash except when upgrading, so flash
>> performance/duration isn't very critical 2) I discovered UBIFS after I
>
> Hm... JFFS2 is probaly not a good choice anyway.  On NOR, where you
> don't have to deal with bad blocks, you might use ext2 or cramfs or
> ... - all of them are more efficient than JFFS2 then.
>

Oh. Thanks for letting me know. I knew cramfs would be more efficient,
but surprised that ext2 would be too. Thought ext2 was "heavier".

>> had JFFS2 working and haven't seen the real need for it yet.
>
> Mount time?
>
Maybe 1-2 seconds. Haven't measured it yet. Maybe boot-time could've
been improved yes.

>> Did you mean I should adapt the general CFI driver to support our
>> configuration, or... Based on what you say below I'm not sure what you
>> actually mean.
>
> You have two different use cases that require (if we don't want to use
> a proprietary driver) two different, mutually exclusive drivers.  I
> would load the driver I need as module, and unload it if I need the
> other one. This is pretty simple in principle, but a bit ore of a
> challange in your case as you use this as a root file system
> (including to load the modules from).
>

When thinking a bit more about this, I realize we don't need to
support writing to the current filesystem in use. I've partitioned our
device into three region, where each "region" consists of four
partitions (dtb, kernel, rootfs and "home"). When upgrading we write a
complete "region image" into another flash region, which is not part
of the region we're running. So maybe we could have both mtd-ram and
cfi-flash running at the same time, since we don't actually need CFI
for the flash we're currently booting from.

I currently use mtd-utils' flash_erase to erase, and 'dd if=/dev/mtdX'
to write. The mtds is created based on info in the dts.. Regarding
DTS: Is it possible to have different partitions within the same
physical flash using different drivers? Like this:

flash@0,0 {
   compatible = "amd,s29gl512n", "cfi-flash",
   reg = <0x0 0x0 0x4000000>;
   partition@...
}

... into something like this:

flash@0,0 {
   compatible = "amd,s29gl512n", "cfi-flash",
   reg = <0x0, 0x0 0x2000000>;
   partition@..
}

flash@0,2000000 {
   compatible = "amd,s29gl512n", "mtd-ram",
   reg = <0x2000000, 0x0, 0x2000000>;
   partition@
}

In other words, partition a flash to use different drivers when
accessing different partitions? I think that could solve the problems.

>> Me too. Small chances. So we have two options I guess: 1) Ditch
>> hardware write protection and use CFI as it is supposed to be used, or
>> 2) write a custom out-of-tree driver. What would you go for?
>
> If this is an option: 1)
>

Good to know your opinion on this.

> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> Do not simplify the design of a program if a way can be found to make
> it complex and wonderful.
>

Hehe. Who wrote that?

Best regards,
Frank

Patch

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 6039e1f..99360af 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -176,6 +176,7 @@  u64 flash_read64(void *addr)__attribute__((weak,
alias("__flash_read64")));
 #define flash_read64	__flash_read64
 #endif

+
 /*-----------------------------------------------------------------------
  */