[U-Boot-Users] [PATCH] mtd: SPI Flash: Support the ST Microelectronics M25P80 and M25P40

[Haavard Skinnemoen]

Jason McMullan <mcmullan at netapp.com> wrote:
> This commit adds MTD support for the M25P80 (1Mx8) and the M25P40 (512kx8)
> SPI Flash components from ST Microelectronics.
> 
> Tested with the M25P40, but should work for the M25P80 according
> to the spec sheet.
> ---

Nice. I have a few minor comments below.

Could you add some information on how to obtain data sheets for these
things? I had some difficulty finding it.

FWIW, I think it's here:

http://www.numonyx.com/en-US/MemoryProducts/NOR/Pages/M25PTechnicalDocuments.aspx

> +struct stmicro_spi_flash_params {
> +	uint8_t		esig;
> +	/* Log2 of page size in power-of-two mode */

Do these chips support anything but power-of-two page sizes? I always
thought that was an AT45 DataFlash peculiarity...

I can't find anything in the M25P80 indicating that the page size can
be different from 256...

> +	uint8_t		l2_page_size;

> +/*
> + * Assemble the address part of a command for STMicro devices in
> + * non-power-of-two page size mode.
> + */

This comment is misleading. The below code only works for power-of-two
page sizes.

> +static void stmicro_build_address(struct stmicro_spi_flash *stm, u8 *cmd, u32 offset)
> +{
> +	unsigned long page_addr;
> +	unsigned long byte_addr;
> +	unsigned long page_size;
> +	unsigned int page_shift;
> +
> +	/*
> +	 * The "extra" space per page is the power-of-two page size
> +	 * divided by 32.
> +	 */

This comment is misleading too. There's no "extra" space in the
calculations below...

> +	page_shift = stm->params->l2_page_size;
> +	page_size = (1 << page_shift);
> +	page_addr = offset / page_size;
> +	byte_addr = offset % page_size;

...which means that you can make these operations cheaper by using
shift and mask instead of divide and modulo. Though I suppose a clever
compiler might figure that out on its own...

> +static int stmicro_write(struct spi_flash *flash,
> +		u32 offset, size_t len, const void *buf)
> +{
> +	struct stmicro_spi_flash *stm = to_stmicro_spi_flash(flash);
> +	unsigned long page_addr;
> +	unsigned long byte_addr;
> +	unsigned long page_size;
> +	unsigned int page_shift;
> +	size_t chunk_len;
> +	size_t actual;
> +	int ret;
> +	u8 cmd[4];
> +
> +	page_shift = stm->params->l2_page_size;
> +	page_size = (1 << page_shift);
> +	page_addr = offset / page_size;
> +	byte_addr = offset % page_size;

Same thing here. Could you have a look at the disassembly to see if the
compiler _is_ clever enough to optimize away the divide/modulo?

Haavard