[PATCH v3 2/3]: mtdoops: Make page size configurable

Artem Bityutskiy Artem.Bityutskiy at nokia.com
Sun Oct 11 06:38:52 EDT 2009


On Thu, 2009-10-08 at 17:27 +0200, Simon Kagstrom wrote:
> The main justification for this is to allow catching long messages
> during a panic, where the top part might otherwise be lost since moving
> to the next block can require a flash erase.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom at netinsight.net>
> ---
>  drivers/mtd/mtdoops.c |   83 +++++++++++++++++++++++++++++++-----------------
>  1 files changed, 53 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
> index 435961e..7045578 100644
> --- a/drivers/mtd/mtdoops.c
> +++ b/drivers/mtd/mtdoops.c
> @@ -32,9 +32,14 @@
>  #include <linux/spinlock.h>
>  #include <linux/interrupt.h>
>  #include <linux/mtd/mtd.h>
> +#include <linux/log2.h>
>  
>  #define MTDOOPS_KERNMSG_MAGIC 0x5d005d00
> -#define OOPS_PAGE_SIZE 4096
> +
> +static int mtdoops_page_size = 4096;
> +module_param(mtdoops_page_size, int, 0);
> +MODULE_PARM_DESC(mtdoops_page_size,
> +		"page size for MTD OOPS pages in bytes (default 4096)");

Term "page" is so overloaded. I'd avoid exporting parameters with "page"
in the name. Could we please call them "records"? At least for the names
we export to users. Of course, ideally you would re-name all symbols in
mtdoops which use "page" term to use "record" term.

Also, the module is called "mtdoops", so it makes little sense to prefix
parameter names with "mtdoops". E.g., if you pass the parameter via the
kernel command line, with current naming you will have:

mtdoops.mtdoops_page_size

Too long, unreadable, confusing :-)

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)




More information about the linux-mtd mailing list