Re: Importing Flash and NAND subsystem for NetBSD

[Adam Hoka <adam.hoka%gmail.com@localhost>]

On Sun, 20 Feb 2011 19:00:38 +0000
Mindaugas Rasiukevicius <rmind%netbsd.org@localhost> wrote:

> Adam Hoka <adam.hoka%gmail.com@localhost> wrote:
> > Latest update: http://www.netbsd.org/~ahoka/patches/flash_9.diff
> > 
> > I will commit this in the following days if there are no other
> > suggestions.
> 
> - Why hamming code is sys/dev rather than libkern?  Perhaps also move
>   nand_crc16() there as well?

hamming could be moved, crc 16 a naive implementation, which is not useful
general use.

> - nand_sync_thread_start: pass KTHREAD_MPSAFE to kthread_create()?

done, thanks.

> - Currently, sc_io_cv is unused i.e. you can use kpause(9) for that.
>   Is nand_sync_thread() LWP supposed to be explicitly awaken later?

it may be used for implementing suspend, so i wont remove it now.

> - flashioctl() leaks 'ei' in error path (re-check other error paths?).

ei is on the stack, but i have reworked the error handling a bit

> - s/struct lwp *process/struct lwp *l/, to not confuse it with struct proc.

done

> - KNF: no variable names in prototype declarations.  There are random spare
>   tabs in the code (just re-read with editor making tabs/spaces visible).

emacs :p

> General note: there are not many comments in the source code, e.g. there
> are no descriptions of Bad Block Tables (BBTs), or how syncer is used, or
> what is the locking protocol (and from quick glance - I would bet there
> are issues with it), etc.  If you want to get more developers familiar with
> the code - sprinkling more comments would make it less difficult for us.

i will try to add some more in the future

-- 
NetBSD - Simplicity is prerequisite for reliability