This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
> > I wonder if my changes are significant enough, or the risk > of breaking > > existing platforms is high enough, that I should change this to an > > entirely new device driver, perhaps intel/cfiflash, or > something like > > that; perhaps even move it up one layer in the tree, since it is > > nolonger specific to Intel -- in theory any flash that implents CFI > > should work with this. > > I think it would be better to not make yet another driver. I keep going back and forth on this issue. On the one hand, I really don't want to add yet another driver to the tree that provides the same support for the same devices as the existing drivers do (although, if we put comments/#warnings in the existing drivers, perhaps folks would migrate to the new driver). On the other hand, this driver is no longer Intel and/or strata specific (although it supports those parts), so how would a newcomer know that this driver might be more appropriate than, for example the 28Fxxx driver? It's a judgement call. > Here's my changes against your patch to make it compile > with my two configs I tested with. In the UNLOCK_SINGLE_BLOCKS turned > off case there were some vars missing. Oops, sorry 'bout that Chief. I actually did compile the nano redboot (which uses the B3 device), but I didn't realize that it didn't support block locking/unlocking. I just tried assabet -- it failed until I applied your patches. > I also moved the new block_op in strata.c and made it not > inline. It is > relatively big so calling it saves some space but more importanatly > since it is not guarranteed that it is inlined even if you tell gcc to > try it can end up not in .2ram section so relocations truncated errors > are given at linkage.That's what happened here anyway. I put it in > .2ram as well. That makes sense. I was originally maintaining three sets of code that did essentially the same thing, so I created 'flash_block_op()' because I got tired of that. Your comments about inline functions and GCC makes sense. Thanks for the rework. >I changed the bootblock detection to work with > SERIES and > with the B3 device. Now for some good, old fashioned, controversy. :-) I deliberately did not fold in your changes for CYGNUM_FLASH_BASE_MASK. My understanding of that parameter is that it exists to cope with oversized devices fitted with some high address lines ignored, into an existing platform. Given that is the case, it doesn't make sense to me populate additional devices in series. On my platform, I (essentially) have two devices attached to two separate Chip-select pins, and I use the MMU to make them show up sequentially in the virtual address space. The 'CYGNUM_FLASH_BASE_MASK' parameter specifies the size of each device. Unfortunately, while the comment in "strata.c" implies that 'CYGNUM_FLASH_BASE_MASK' is optional, code elsewhere in the driver assumes it is defined. I'm not actually happy with this solution as it stands. Since, downstream, we may be forced to buy larger parts (where I use the term "forced" to mean "we can get them cheaper because there is more demand for them). I would like for RedBoot, and my application(s) to adapt dynamically to the size of the FLASH without requiring a recompile. In my case, I could adjust the MMU mappings at bootup based upon information returned from the FLASH device(s) via CFI. But I'm not ready to bite off that project right now. > So far with these changes it seems to work on the B3 and W3 > parts I have > here.I'll keep testing though. Great, and thanks. I have (perhaps) attached a new version of the complete patch, folding in your changes (except for the CYGNUM_FLASH_BASE_MASK change as discussed above). Enjoy! > > Jani >
Attachment:
st3.diff
Description: Binary data
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |