Messages in this thread | | | From | Ricardo Ribalda Delgado <> | Date | Fri, 6 May 2016 20:21:26 +0200 | Subject | Re: [PATCH RESEND] mtd: spi-nor: Add support for S3AN spi-nor devices |
| |
Hi Brian
Thanks for your comments, and sorry for sending v2 off list and also not commenting directly to this mail. See my comments bellow
On Fri, May 6, 2016 at 2:56 AM, Brian Norris <computersforpeace@gmail.com> wrote: > On Wed, Apr 27, 2016 at 03:14:22PM +0200, Ricardo Ribalda Delgado wrote:
> > Whoa this is fugly. Why does this mode exist? Can we ignore it and just > use the poewr-of-two mode? If not, I think we need a bit more verbose of > comments in the code to explain what/why we are doing.
Indeed it is :). This driver is for a device that is a bit special.
FPGAs need to read the configuration from somewhere. Traditionally this is done from a parallel flash, serial flash or another MCU that bitbangs the configuration. The Spartan 3AN is a family of FPGA with a serial flash embeeded in the chip: http://www.xilinx.com/support/documentation/user_guides/ug333.pdf This family allows you to remove one chip from your BOM.
This flash is mainly written by the FPGA tools, but it can also be accessed by the FPGA fabric via an internal spi port, and this is why I have developed this driver.
The flash is almost a standard spi nor, but with some differences. The major one being the addressing mode.
By default the flash is accessed using the Default Addressing Mode. This mode provides an extra 3% space and the FPGA tools expect the device to be in these mode.
The big disadvantage of this mode is that the 3 byte address is calculated a bit differently. And therefore we need this nasty convert function.
The FPGA can also be configured to use Power-of-two mode (standard mode), but you lose 3% of space (a lot, considering the reduced resources), it is an irreversible operation and this mode will corrupt the original data. By default I believe that it is safer to keep the initial mode. (Check from page 19 of the provided UG).
>
> > Maybe instead of this flag, you can just use the mfr ID? So: > > if (JEDEC_MFR(info) == SNOR_MFR_...) { // what would you call the "manfacturer" here anyway? >
The MFR corresponds to Atmel, therefore we cannot use it :(. We need to probe th whole ID, and I think this is more clean than:
if (id== ID1 || id==ID2...... || id=ID1000){ }
But if you think otherwise I do not have a strong objection to change it.
> > Brian
Thanks for your review, if you think that there is something else missing I will fwd v2 to the list, otherwise I will prepare a v2 with your comments.
-- Ricardo Ribalda
| |