Re: [PATCH 15/44 take 2] [UBI] scanning unit header

2007-02-17 Thread Josh Boyer
On Sat, Feb 17, 2007 at 06:07:46PM -0500, Theodore Tso wrote:
> 
> This is a general comment that applies across your entire patchset.
> It would be a lot easier to review the patchset if you put the Docbook
> description of the function with the .c file instead of the .h file.
> This will also make it much more likely that when you or other people
> update the code function, that the documentation will get updated as
> well.
> 
> I'd recommend doing this along with combining all of your *.h files
> into a ubi_private.h and ubi.h file.

I agree.  Thanks for the suggestions Ted.

josh
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 15/44 take 2] [UBI] scanning unit header

2007-02-17 Thread Theodore Tso
On Sat, Feb 17, 2007 at 06:55:40PM +0200, Artem Bityutskiy wrote:
> +/**
> + * ubi_scan_erase_peb - erase a physical eraseblock.
> + *
> + * @ubi: the UBI device description object
> + * @si: a pointer to the scanning information
> + * @pnum: physical eraseblock number to erase;
> + * @ec: erase counter value to write (%NAND_SCAN_UNKNOWN_EC if it is unknown)
> + *
> + * This function erases physical eraseblock 'pnum', and writes the erase
> + * counter header to it. This function should only be used on UBI device
> + * initialization stages, when the EBA unit had not been yet initialized. 
> This
> + * function returns zero in case of success and a negative error code in case
> + * of failure.
> + */

This is a general comment that applies across your entire patchset.
It would be a lot easier to review the patchset if you put the Docbook
description of the function with the .c file instead of the .h file.
This will also make it much more likely that when you or other people
update the code function, that the documentation will get updated as
well.

I'd recommend doing this along with combining all of your *.h files
into a ubi_private.h and ubi.h file.

Regards,

- Ted
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 15/44 take 2] [UBI] scanning unit header

2007-02-17 Thread Artem Bityutskiy
diff -auNrp tmp-from/drivers/mtd/ubi/scan.h tmp-to/drivers/mtd/ubi/scan.h
--- tmp-from/drivers/mtd/ubi/scan.h 1970-01-01 02:00:00.0 +0200
+++ tmp-to/drivers/mtd/ubi/scan.h   2007-02-17 18:07:26.0 +0200
@@ -0,0 +1,279 @@
+/*
+ * Copyright (c) International Business Machines Corp., 2006
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: Artem B. Bityutskiy
+ */
+
+/*
+ * UBI scanning unit.
+ *
+ * This unit is responsible for scanning the flash media, checking UBI
+ * headers and providing complete information about the UBI flash image.
+ */
+
+#ifndef __UBI_SCAN_H__
+#define __UBI_SCAN_H__
+
+#include 
+#include 
+
+/* The erase counter of this eraseblock is unknown */
+#define NAND_SCAN_UNKNOWN_EC (-1)
+
+struct ubi_info;
+struct ubi_scan_info;
+struct ubi_scan_volume;
+struct ubi_scan_leb;
+struct ubi_vid_hdr;
+
+/**
+ * ubi_scan_add_peb - add information about a physical eraseblock to the
+ * scanning information.
+ *
+ * @ubi: the UBI device description object
+ * @si: a pointer to the scanning information
+ * @pnum: the physical eraseblock number
+ * @ec: erase counter
+ * @vid_hdr: the volume identifier header
+ * @bitflips: if a bit-flips were detected while reading this physical
+ * eraseblock
+ *
+ * This function returns zero in case of success and a negative error code in
+ * case of failure.
+ */
+int ubi_scan_add_peb(const struct ubi_info *ubi, struct ubi_scan_info *si,
+int pnum, int ec, const struct ubi_vid_hdr *vid_hdr,
+int bitflips);
+
+/**
+ * ubi_scan_add_corr_peb - add a physical eraseblock to the list of corrupted
+ * physical eraseblocks.
+ *
+ * @si: a pointer to the scanning information
+ * @pnum: the physical eraseblock number
+ * @ec: erase counter of this physical eraseblock
+ *
+ * If @ec is not known, %NAND_SCAN_UNKNOWN_EC has to be passed and mean erase
+ * counter will be used. This function returns zero in case of success and a
+ * negative error code in case of failure.
+ */
+int ubi_scan_add_corr_peb(struct ubi_scan_info *si, int pnum, int ec);
+
+/**
+ * ubi_scan_find_sv - find information about a particular volume in the
+ * scanning information.
+ *
+ * @si: a pointer to the scanning information
+ * @vol_id: the requested volume ID
+ *
+ * This function returns a pointer to the volume description or %NULL if there
+ * are no data about this volume in the scanning information.
+ */
+struct ubi_scan_volume *ubi_scan_find_sv(const struct ubi_scan_info *si,
+int vol_id);
+
+/**
+ * ubi_scan_find_seb - find information about a particular logical
+ * eraseblock in the volume scanning information.
+ *
+ * @sv: a pointer to the volume scanning information
+ * @lnum: the requested logical eraseblock
+ *
+ * This function returns a pointer to the scanning logical eraseblock or %NULL
+ * if there are no data about it in the scanning volume information.
+ */
+struct ubi_scan_leb *ubi_scan_find_seb(const struct ubi_scan_volume *sv,
+  int lnum);
+
+/**
+ * ubi_scan_erase_peb - erase a physical eraseblock.
+ *
+ * @ubi: the UBI device description object
+ * @si: a pointer to the scanning information
+ * @pnum: physical eraseblock number to erase;
+ * @ec: erase counter value to write (%NAND_SCAN_UNKNOWN_EC if it is unknown)
+ *
+ * This function erases physical eraseblock 'pnum', and writes the erase
+ * counter header to it. This function should only be used on UBI device
+ * initialization stages, when the EBA unit had not been yet initialized. This
+ * function returns zero in case of success and a negative error code in case
+ * of failure.
+ */
+int ubi_scan_erase_peb(const struct ubi_info *ubi,
+  const struct ubi_scan_info *si, int pnum, int ec);
+
+/**
+ * ubi_scan_get_free_peb - get a free physical eraseblock.
+ *
+ * @ubi: the UBI device description object
+ * @si: a pointer to the scanning information
+ *
+ * This function returns a free physical eraseblock. It is supposed to be
+ * called on the UBI initialization stages when the wear-leveling unit is not
+ * initialized yet. This function picks a physical eraseblocks from one of the
+ * lists, writes the EC header if it is needed, and removes it from the list.
+ *
+ * This function 

[PATCH 15/44 take 2] [UBI] scanning unit header

2007-02-17 Thread Artem Bityutskiy
diff -auNrp tmp-from/drivers/mtd/ubi/scan.h tmp-to/drivers/mtd/ubi/scan.h
--- tmp-from/drivers/mtd/ubi/scan.h 1970-01-01 02:00:00.0 +0200
+++ tmp-to/drivers/mtd/ubi/scan.h   2007-02-17 18:07:26.0 +0200
@@ -0,0 +1,279 @@
+/*
+ * Copyright (c) International Business Machines Corp., 2006
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: Artem B. Bityutskiy
+ */
+
+/*
+ * UBI scanning unit.
+ *
+ * This unit is responsible for scanning the flash media, checking UBI
+ * headers and providing complete information about the UBI flash image.
+ */
+
+#ifndef __UBI_SCAN_H__
+#define __UBI_SCAN_H__
+
+#include linux/list.h
+#include linux/rbtree.h
+
+/* The erase counter of this eraseblock is unknown */
+#define NAND_SCAN_UNKNOWN_EC (-1)
+
+struct ubi_info;
+struct ubi_scan_info;
+struct ubi_scan_volume;
+struct ubi_scan_leb;
+struct ubi_vid_hdr;
+
+/**
+ * ubi_scan_add_peb - add information about a physical eraseblock to the
+ * scanning information.
+ *
+ * @ubi: the UBI device description object
+ * @si: a pointer to the scanning information
+ * @pnum: the physical eraseblock number
+ * @ec: erase counter
+ * @vid_hdr: the volume identifier header
+ * @bitflips: if a bit-flips were detected while reading this physical
+ * eraseblock
+ *
+ * This function returns zero in case of success and a negative error code in
+ * case of failure.
+ */
+int ubi_scan_add_peb(const struct ubi_info *ubi, struct ubi_scan_info *si,
+int pnum, int ec, const struct ubi_vid_hdr *vid_hdr,
+int bitflips);
+
+/**
+ * ubi_scan_add_corr_peb - add a physical eraseblock to the list of corrupted
+ * physical eraseblocks.
+ *
+ * @si: a pointer to the scanning information
+ * @pnum: the physical eraseblock number
+ * @ec: erase counter of this physical eraseblock
+ *
+ * If @ec is not known, %NAND_SCAN_UNKNOWN_EC has to be passed and mean erase
+ * counter will be used. This function returns zero in case of success and a
+ * negative error code in case of failure.
+ */
+int ubi_scan_add_corr_peb(struct ubi_scan_info *si, int pnum, int ec);
+
+/**
+ * ubi_scan_find_sv - find information about a particular volume in the
+ * scanning information.
+ *
+ * @si: a pointer to the scanning information
+ * @vol_id: the requested volume ID
+ *
+ * This function returns a pointer to the volume description or %NULL if there
+ * are no data about this volume in the scanning information.
+ */
+struct ubi_scan_volume *ubi_scan_find_sv(const struct ubi_scan_info *si,
+int vol_id);
+
+/**
+ * ubi_scan_find_seb - find information about a particular logical
+ * eraseblock in the volume scanning information.
+ *
+ * @sv: a pointer to the volume scanning information
+ * @lnum: the requested logical eraseblock
+ *
+ * This function returns a pointer to the scanning logical eraseblock or %NULL
+ * if there are no data about it in the scanning volume information.
+ */
+struct ubi_scan_leb *ubi_scan_find_seb(const struct ubi_scan_volume *sv,
+  int lnum);
+
+/**
+ * ubi_scan_erase_peb - erase a physical eraseblock.
+ *
+ * @ubi: the UBI device description object
+ * @si: a pointer to the scanning information
+ * @pnum: physical eraseblock number to erase;
+ * @ec: erase counter value to write (%NAND_SCAN_UNKNOWN_EC if it is unknown)
+ *
+ * This function erases physical eraseblock 'pnum', and writes the erase
+ * counter header to it. This function should only be used on UBI device
+ * initialization stages, when the EBA unit had not been yet initialized. This
+ * function returns zero in case of success and a negative error code in case
+ * of failure.
+ */
+int ubi_scan_erase_peb(const struct ubi_info *ubi,
+  const struct ubi_scan_info *si, int pnum, int ec);
+
+/**
+ * ubi_scan_get_free_peb - get a free physical eraseblock.
+ *
+ * @ubi: the UBI device description object
+ * @si: a pointer to the scanning information
+ *
+ * This function returns a free physical eraseblock. It is supposed to be
+ * called on the UBI initialization stages when the wear-leveling unit is not
+ * initialized yet. This function picks a physical eraseblocks from one of the
+ * lists, writes the EC header if it is needed, and removes it from the 

Re: [PATCH 15/44 take 2] [UBI] scanning unit header

2007-02-17 Thread Theodore Tso
On Sat, Feb 17, 2007 at 06:55:40PM +0200, Artem Bityutskiy wrote:
 +/**
 + * ubi_scan_erase_peb - erase a physical eraseblock.
 + *
 + * @ubi: the UBI device description object
 + * @si: a pointer to the scanning information
 + * @pnum: physical eraseblock number to erase;
 + * @ec: erase counter value to write (%NAND_SCAN_UNKNOWN_EC if it is unknown)
 + *
 + * This function erases physical eraseblock 'pnum', and writes the erase
 + * counter header to it. This function should only be used on UBI device
 + * initialization stages, when the EBA unit had not been yet initialized. 
 This
 + * function returns zero in case of success and a negative error code in case
 + * of failure.
 + */

This is a general comment that applies across your entire patchset.
It would be a lot easier to review the patchset if you put the Docbook
description of the function with the .c file instead of the .h file.
This will also make it much more likely that when you or other people
update the code function, that the documentation will get updated as
well.

I'd recommend doing this along with combining all of your *.h files
into a ubi_private.h and ubi.h file.

Regards,

- Ted
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 15/44 take 2] [UBI] scanning unit header

2007-02-17 Thread Josh Boyer
On Sat, Feb 17, 2007 at 06:07:46PM -0500, Theodore Tso wrote:
 
 This is a general comment that applies across your entire patchset.
 It would be a lot easier to review the patchset if you put the Docbook
 description of the function with the .c file instead of the .h file.
 This will also make it much more likely that when you or other people
 update the code function, that the documentation will get updated as
 well.
 
 I'd recommend doing this along with combining all of your *.h files
 into a ubi_private.h and ubi.h file.

I agree.  Thanks for the suggestions Ted.

josh
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/