[En-Nut-Discussion] AT25DF_INFO

[Ole Reinhardt]

Hi Uwe,

Am 21.03.2016 15:19, schrieb Uwe Bonnes:
> dev/spi_at25df.h defines in struct _AT25DF_INFO {
> 
>     /*! \brief Total number of pages. */
>     uint32_t at25df_erase_blocks;
> 
> Description and name contradict.  Shouldn't it read something like
> 
>     /*! \brief Total number of blocks. */
>     uint32_t at25df_blocks;

Indeed the description is a C&P and shoult be

/*! \brief Total number of erase blocks. */

The name of the variable I'd like to keep as "at25df_erase_blocks". I
personally prefer talking variable names.

> Otherwise, I think at25df_ebshft is not needed and instead of e.g.
> ./dev/spi_flash_at25df.c:        pga = pgn << at->dxb_dfinfo->at25df_ebshft;
> we could use
> ./dev/spi_flash_at25df.c:        pga = pgn << 12;
> Having redundant parameters increases the chance for wrong
> parameters. Smallest erase size is always 4096 bytes for all XX25 serial
> flash I had a look at,

Yes and no.

I'd like to keep in configurable, just because I do now know, if there
is any XX25 device with different configuration that we do not know yet.
On the other hand you could configure larger erase blocks, if you like
to make use of the larger block erase commands.

You are right, that at25df_ebshft and at25df_ebsize are somewhat
redundant. If I remember correctly I took these configs from older xx25
implementations.
Unfortunately the code needs to know the shift width. You could
calculate it dynamically from the ebsize or you could calculate the
ebsize from the shift value...

I don't have a good solution and would just keep it :)

Btw: at25df_ebshft is not only used in spi_flash_at25df.c but also in
spi_blkio_at25df.c.

> Shouldn't
> ./dev/spi_at25df_info.c:AT25DF_INFO at25df_info[] = {
> read
> ./dev/spi_at25df_info.c:const AT25DF_INFO at25df_info[] = {
>                         ^^^^
> to not waste RAM?

Yes.

Best regards

Ole

-- 
Embedded-IT
Alter Weg 3
57223 Kreuztal
http://www.embedded-it.de

Tel.: +49-177-7420433