"Jared D. McNeill" wrote:

> Hey folks --
> 
> On the road to building a flash filesystem for NetBSD, I decided that  
> a generic flash API was needed first. Feedback is welcome.
> 
> Externally, flash(4) exposes the following ioctl operations:
> 	FLASH_ERASEBLOCK: erase a single block
> 	FLASH_GBLOCKSTATUS: get the status of a single block
> 	FLASH_SBLOCKSTATUS: set the status of a single block
> 	FLASH_GETINFO: query flash device capabilities
> 
> Flash device capabilities is defined by flash_info_t, which provides:
> 	enum flash_type fi_type;		/* NOR or NAND */
> 	block_id_t fi_nblocks;
> 	size_t fi_bytesperblock;
> 	size_t fi_sectorsperblock;
> 	size_t fi_sectorsize;

I'd go with fixed-sized types here.  size_t can be a 32- or 64-bit
quantity depending on the arch, and this will make compat32 more
difficult.  I'm not sure if we want to add a fi_flags member or not?
Now would be the time to do it.

What's the difference between a "block" and a "sector"?  As far as I
know, a sector is the highest allocation level you deal with on a flash
device.

I know that SD/USB/whatever flash cards are 4GB plus now, but don't
know what we're expected to see as embedded flash in systems.  Should
fi_nblocks be a uint32_t or uint64_t?  If we want to be cautious, then
perhaps go for something like (assuming we keep both a "sector" and a
"block"):

	uint32_t fi_type;
	uint32_t fi_bytesperblock;
	uint32_t fi_sectorsperblock;
	uint32_t fi_sectorsize;
	uint64_t fi_nblocks;

which will also pack nicely, 64-bit wise.  Assuming too of course we
didn't add fi_flags :)

> Internally, a hardware flash device driver must implement the  
> following callbacks:
> 	init(void *)
> 	deinit(void *)
> 	get_info(void *, flash_info_t *)
> 	erase_block(void *, block_id_t)
> 	get_block_status(void *, block_id_t)
> 	set_block_status(void *, block_id_t, int)
> 	read_sector(void *, int, uint8_t *, sector_info_t *, int)
> 	write_sector(void *, int, uint8_t *, sector_info_t *, int)
> 	ioctl(void *, u_long, caddr_t, int, struct lwp *)	<-- optional
> 
> After filling in the structure of function callbacks, the hardware  
> driver must call flash_attach_mi.

"deinit"?  It's only a function name, but seems a bit wierd...  Is that
called after initialisation, or at final close?

> Additionally, a userland utility is provided as flashctl(8) that  
> allows you to query device info, erase blocks, and get or set block  
> status.
> 
> Source code is available here for review:
>    http://www.invisible.ca/~jmcneill/netbsd/flash-20060904.tar

I think /sbin may be better for flashctl - you may need it in
single-user mode.

No reason for <sys/flash.h>, just use <dev/flash/flash.h>.

Cheers,
Simon.