On Wed, 1 Dec 2010, Charles Manning wrote:
> Signed-off-by: Charles Manning <cdhmanning@xxxxxxxxx>
> ---
[...]
> +#include "yportenv.h"
> +
> +#include "yaffs_mtdif.h"
> +
> +#include "linux/mtd/mtd.h"
> +#include "linux/types.h"
> +#include "linux/time.h"
> +#include "linux/mtd/nand.h"
> +
> +#include "yaffs_linux.h"
Small thing, but why these blank lines between includes?
> +
> +int nandmtd_erase_block(struct yaffs_dev *dev, int block_no)
> +{
> + struct mtd_info *mtd = yaffs_dev_to_mtd(dev);
> + u32 addr =
> + ((loff_t) block_no) * dev->param.total_bytes_per_chunk
> + * dev->param.chunks_per_block;
> + struct erase_info ei;
> +
> + int retval = 0;
Why not kill the blank line before "int retval = 0;" ?
> +
> + ei.mtd = mtd;
> + ei.addr = addr;
> + ei.len = dev->param.total_bytes_per_chunk * dev->param.chunks_per_block;
> + ei.time = 1000;
> + ei.retries = 2;
> + ei.callback = NULL;
> + ei.priv = (u_long) dev;
> +
> + retval = mtd->erase(mtd, &ei);
> +
> + if (retval == 0)
> + return YAFFS_OK;
> + else
> + return YAFFS_FAIL;
Personal preference thing I guess, but I couldn't help thinking
return mtd->erase(mtd, &ei) ? YAFFS_FAIL : YAFFS_OK;
[...]
> + retval = mtd->block_markbad(mtd, (loff_t) blocksize * block_no);
> + return (retval) ? YAFFS_FAIL : YAFFS_OK;
Pointless parenthesis.
[...]
> +
> +/* mtd interface for YAFFS2 */
> +
> +#include "yportenv.h"
> +#include "yaffs_trace.h"
> +
> +#include "yaffs_mtdif2.h"
> +
> +#include "linux/mtd/mtd.h"
> +#include "linux/types.h"
> +#include "linux/time.h"
> +
> +#include "yaffs_packedtags2.h"
> +
> +#include "yaffs_linux.h"
> +
Again the blank lines between includes - why?
I can see grouping the "linux/..." includes and the rest and then put a
blank line between the two, but the above just looks like a waste of
vertical screen space to me.
[...]
> + yaffs_pack_tags2_tags_only(pt2tp, tags);
> + } else {
> + yaffs_pack_tags2(&pt, tags, !dev->param.no_tags_ecc);
> + }
spaces used for indentation where tabs should have been.
[...]
> +int yaffs_erase_block(struct yaffs_dev *dev, int flash_block)
> +{
> + int result;
> +
> + flash_block -= dev->block_offset;
> +
> + dev->n_erasures++;
> +
> + result = dev->param.erase_fn(dev, flash_block);
> +
> + return result;
> +}
How about
int yaffs_erase_block(struct yaffs_dev *dev, int flash_block)
{
flash_block -= dev->block_offset;
dev->n_erasures++;
return dev->param.erase_fn(dev, flash_block);
}
--
Jesper Juhl <jj@xxxxxxxxxxxxx> http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
