[PATCH] phram: Allow the user to set the erase page size.

2020-11-24 Thread Guohua Zhong
Permit the user to specify the erase page size as a parameter.
This solves two problems:

- phram can access images made by mkfs.jffs2.  mkfs.jffs2 won't
create images with erase sizes less than 8KiB; many architectures
define PAGE_SIZE as 4KiB.

- Allows more effective use of small capacity devices.  JFFS2
needs somewhere between 2 and 5 empty pages for garbage collection;
and for an NVRAM part with only 32KiB of space, a smaller erase page
allows much better utilization in applications where garbage collection
is important.

Signed-off-by: Patrick O'Grady 
Reviewed-by: Joern Engel 
Link: 
https://lore.kernel.org/lkml/CAJ7m5OqYv_=JB9NhHsqBsa8YU0DFRoP7C+W10PY22wonAGJK=a...@mail.gmail.com/
[Guohua Zhong: fix token array index out of bounds and update patch for kernel 
master branch]
Signed-off-by: Guohua Zhong 
---
 drivers/mtd/devices/phram.c | 52 +
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index 087b5e86d1bf..1729b94b2abf 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -6,14 +6,14 @@
  * Usage:
  *
  * one commend line parameter per device, each in the form:
- *   phram=,,
+ *   phram=,,[,]
  *  may be up to 63 characters.
- *  and  can be octal, decimal or hexadecimal.  If followed
+ * , , and  can be octal, decimal or hexadecimal.  If 
followed
  * by "ki", "Mi" or "Gi", the numbers will be interpreted as kilo, mega or
- * gigabytes.
+ * gigabytes.  is optional and defaults to PAGE_SIZE.
  *
  * Example:
- * phram=swap,64Mi,128Mi phram=test,900Mi,1Mi
+ * phram=swap,64Mi,128Mi phram=test,900Mi,1Mi,64Ki
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct phram_mtd_list {
struct mtd_info mtd;
@@ -88,7 +89,7 @@ static void unregister_devices(void)
}
 }
 
-static int register_device(char *name, phys_addr_t start, size_t len)
+static int register_device(char *name, phys_addr_t start, size_t len, uint32_t 
erasesize)
 {
struct phram_mtd_list *new;
int ret = -ENOMEM;
@@ -115,7 +116,7 @@ static int register_device(char *name, phys_addr_t start, 
size_t len)
new->mtd._write = phram_write;
new->mtd.owner = THIS_MODULE;
new->mtd.type = MTD_RAM;
-   new->mtd.erasesize = PAGE_SIZE;
+   new->mtd.erasesize = erasesize;
new->mtd.writesize = 1;
 
ret = -EAGAIN;
@@ -204,22 +205,23 @@ static inline void kill_final_newline(char *str)
 static int phram_init_called;
 /*
  * This shall contain the module parameter if any. It is of the form:
- * - phram=,, for module case
- * - phram.phram=,, for built-in case
- * We leave 64 bytes for the device name, 20 for the address and 20 for the
- * size.
- * Example: phram.phram=rootfs,0xa000,512Mi
+ * - phram=,,[,] for module case
+ * - phram.phram=,,[,] for built-in case
+ * We leave 64 bytes for the device name, 20 for the address , 20 for the
+ * size and 20 for the erasesize.
+ * Example: phram.phram=rootfs,0xa000,512Mi,65536
  */
-static char phram_paramline[64 + 20 + 20];
+static char phram_paramline[64 + 20 + 20 + 20];
 #endif
 
 static int phram_setup(const char *val)
 {
-   char buf[64 + 20 + 20], *str = buf;
-   char *token[3];
+   char buf[64 + 20 + 20 + 20], *str = buf;
+   char *token[4];
char *name;
uint64_t start;
uint64_t len;
+   uint64_t erasesize = PAGE_SIZE;
int i, ret;
 
if (strnlen(val, sizeof(buf)) >= sizeof(buf))
@@ -228,7 +230,7 @@ static int phram_setup(const char *val)
strcpy(str, val);
kill_final_newline(str);
 
-   for (i = 0; i < 3; i++)
+   for (i = 0; i < 4; i++)
token[i] = strsep(, ",");
 
if (str)
@@ -253,11 +255,25 @@ static int phram_setup(const char *val)
goto error;
}
 
-   ret = register_device(name, start, len);
+   if (token[3]) {
+   ret = parse_num64(, token[3]);
+   if (ret) {
+   parse_err("illegal erasesize\n");
+   goto error;
+   }
+   }
+
+   if (len == 0 || erasesize == 0 || erasesize > len
+   || erasesize > UINT_MAX || do_div(len, (uint32_t)erasesize) != 0) {
+   parse_err("illegal erasesize or len\n");
+   goto error;
+   }
+
+   ret = register_device(name, start, len, (uint32_t)erasesize);
if (ret)
goto error;
 
-   pr_info("%s device: %#llx at %#llx\n", name, len, start);
+   pr_info("%s device: %#llx at %#llx for erasesize %#llx\n", name, len, 
start, erasesize);
return 0;
 
 error:
@@ -298,7 +314,7 @@ static int phram_param_call(const char *val, const struct 
kernel_param *kp)
 }
 
 module_param_call(phram, phram_param_call, NULL, NULL, 0200);
-MODULE_PARM_DESC(phram, "Memory region to map. 
\"phram=,,\"");

Re: [PATCH] phram: Allow the user to set the erase page size.

2020-11-24 Thread kernel test robot
Hi Guohua,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.10-rc5 next-20201123]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Guohua-Zhong/phram-Allow-the-user-to-set-the-erase-page-size/20201124-141421
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
d5beb3140f91b1c8a3d41b14d729aefa4dcc58bc
config: i386-randconfig-r034-20201124 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# 
https://github.com/0day-ci/linux/commit/6ec7a653a663b6a638f767aa844a685fdd1463d2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Guohua-Zhong/phram-Allow-the-user-to-set-the-erase-page-size/20201124-141421
git checkout 6ec7a653a663b6a638f767aa844a685fdd1463d2
# save the attached .config to linux build tree
make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   ld: drivers/mtd/devices/phram.o: in function `phram_setup':
>> drivers/mtd/devices/phram.c:266: undefined reference to `__umoddi3'

vim +266 drivers/mtd/devices/phram.c

   215  
   216  static int phram_setup(const char *val)
   217  {
   218  char buf[64 + 20 + 20 + 20], *str = buf;
   219  char *token[4];
   220  char *name;
   221  uint64_t start;
   222  uint64_t len;
   223  uint64_t erasesize = PAGE_SIZE;
   224  int i, ret;
   225  
   226  if (strnlen(val, sizeof(buf)) >= sizeof(buf))
   227  parse_err("parameter too long\n");
   228  
   229  strcpy(str, val);
   230  kill_final_newline(str);
   231  
   232  for (i = 0; i < 4; i++)
   233  token[i] = strsep(, ",");
   234  
   235  if (str)
   236  parse_err("too many arguments\n");
   237  
   238  if (!token[2])
   239  parse_err("not enough arguments\n");
   240  
   241  ret = parse_name(, token[0]);
   242  if (ret)
   243  return ret;
   244  
   245  ret = parse_num64(, token[1]);
   246  if (ret) {
   247  parse_err("illegal start address\n");
   248  goto error;
   249  }
   250  
   251  ret = parse_num64(, token[2]);
   252  if (ret) {
   253  parse_err("illegal device length\n");
   254  goto error;
   255  }
   256  
   257  if (token[3]) {
   258  ret = parse_num64(, token[3]);
   259  if (ret) {
   260  parse_err("illegal erasesize\n");
   261  goto error;
   262  }
   263  }
   264  
   265  if (len == 0 || erasesize == 0 || erasesize > len
 > 266  || erasesize > UINT_MAX || len % erasesize != 0) {
   267  parse_err("illegal erasesize or len\n");
   268  goto error;
   269  }
   270  
   271  ret = register_device(name, start, len, (uint32_t)erasesize);
   272  if (ret)
   273  goto error;
   274  
   275  pr_info("%s device: %#llx at %#llx for erasesize %#llx\n", 
name, len, start, erasesize);
   276  return 0;
   277  
   278  error:
   279  kfree(name);
   280  return ret;
   281  }
   282  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip


Re: [PATCH] phram: Allow the user to set the erase page size.

2020-11-24 Thread kernel test robot
Hi Guohua,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.10-rc5 next-20201123]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Guohua-Zhong/phram-Allow-the-user-to-set-the-erase-page-size/20201124-141421
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
d5beb3140f91b1c8a3d41b14d729aefa4dcc58bc
config: microblaze-randconfig-r012-20201124 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/0day-ci/linux/commit/6ec7a653a663b6a638f767aa844a685fdd1463d2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Guohua-Zhong/phram-Allow-the-user-to-set-the-erase-page-size/20201124-141421
git checkout 6ec7a653a663b6a638f767aa844a685fdd1463d2
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross 
ARCH=microblaze 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__umoddi3" [drivers/mtd/devices/phram.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip


[PATCH] phram: Allow the user to set the erase page size.

2020-11-23 Thread Guohua Zhong
Permit the user to specify the erase page size as a parameter.
This solves two problems:

- phram can access images made by mkfs.jffs2.  mkfs.jffs2 won't
create images with erase sizes less than 8KiB; many architectures
define PAGE_SIZE as 4KiB.

- Allows more effective use of small capacity devices.  JFFS2
needs somewhere between 2 and 5 empty pages for garbage collection;
and for an NVRAM part with only 32KiB of space, a smaller erase page
allows much better utilization in applications where garbage collection
is important.

Signed-off-by: Patrick O'Grady 
Reviewed-by: Joern Engel 
Link: 
https://lore.kernel.org/lkml/CAJ7m5OqYv_=JB9NhHsqBsa8YU0DFRoP7C+W10PY22wonAGJK=a...@mail.gmail.com/
[Guohua Zhong: fix token array index out of bounds and update patch for kernel 
master branch]
Signed-off-by: Guohua Zhong 
---
 drivers/mtd/devices/phram.c | 51 +
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index 087b5e86d1bf..3ac766b65bf2 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -6,14 +6,14 @@
  * Usage:
  *
  * one commend line parameter per device, each in the form:
- *   phram=,,
+ *   phram=,,[,]
  *  may be up to 63 characters.
- *  and  can be octal, decimal or hexadecimal.  If followed
+ * , , and  can be octal, decimal or hexadecimal.  If 
followed
  * by "ki", "Mi" or "Gi", the numbers will be interpreted as kilo, mega or
- * gigabytes.
+ * gigabytes.  is optional and defaults to PAGE_SIZE.
  *
  * Example:
- * phram=swap,64Mi,128Mi phram=test,900Mi,1Mi
+ * phram=swap,64Mi,128Mi phram=test,900Mi,1Mi,64Ki
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -88,7 +88,7 @@ static void unregister_devices(void)
}
 }
 
-static int register_device(char *name, phys_addr_t start, size_t len)
+static int register_device(char *name, phys_addr_t start, size_t len, uint32_t 
erasesize)
 {
struct phram_mtd_list *new;
int ret = -ENOMEM;
@@ -115,7 +115,7 @@ static int register_device(char *name, phys_addr_t start, 
size_t len)
new->mtd._write = phram_write;
new->mtd.owner = THIS_MODULE;
new->mtd.type = MTD_RAM;
-   new->mtd.erasesize = PAGE_SIZE;
+   new->mtd.erasesize = erasesize;
new->mtd.writesize = 1;
 
ret = -EAGAIN;
@@ -204,22 +204,23 @@ static inline void kill_final_newline(char *str)
 static int phram_init_called;
 /*
  * This shall contain the module parameter if any. It is of the form:
- * - phram=,, for module case
- * - phram.phram=,, for built-in case
- * We leave 64 bytes for the device name, 20 for the address and 20 for the
- * size.
- * Example: phram.phram=rootfs,0xa000,512Mi
+ * - phram=,,[,] for module case
+ * - phram.phram=,,[,] for built-in case
+ * We leave 64 bytes for the device name, 20 for the address , 20 for the
+ * size and 20 for the erasesize.
+ * Example: phram.phram=rootfs,0xa000,512Mi,65536
  */
-static char phram_paramline[64 + 20 + 20];
+static char phram_paramline[64 + 20 + 20 + 20];
 #endif
 
 static int phram_setup(const char *val)
 {
-   char buf[64 + 20 + 20], *str = buf;
-   char *token[3];
+   char buf[64 + 20 + 20 + 20], *str = buf;
+   char *token[4];
char *name;
uint64_t start;
uint64_t len;
+   uint64_t erasesize = PAGE_SIZE;
int i, ret;
 
if (strnlen(val, sizeof(buf)) >= sizeof(buf))
@@ -228,7 +229,7 @@ static int phram_setup(const char *val)
strcpy(str, val);
kill_final_newline(str);
 
-   for (i = 0; i < 3; i++)
+   for (i = 0; i < 4; i++)
token[i] = strsep(, ",");
 
if (str)
@@ -253,11 +254,25 @@ static int phram_setup(const char *val)
goto error;
}
 
-   ret = register_device(name, start, len);
+   if (token[3]) {
+   ret = parse_num64(, token[3]);
+   if (ret) {
+   parse_err("illegal erasesize\n");
+   goto error;
+   }
+   }
+
+   if (len == 0 || erasesize == 0 || erasesize > len
+   || erasesize > UINT_MAX || len % erasesize != 0) {
+   parse_err("illegal erasesize or len\n");
+   goto error;
+   }
+
+   ret = register_device(name, start, len, (uint32_t)erasesize);
if (ret)
goto error;
 
-   pr_info("%s device: %#llx at %#llx\n", name, len, start);
+   pr_info("%s device: %#llx at %#llx for erasesize %#llx\n", name, len, 
start, erasesize);
return 0;
 
 error:
@@ -298,7 +313,7 @@ static int phram_param_call(const char *val, const struct 
kernel_param *kp)
 }
 
 module_param_call(phram, phram_param_call, NULL, NULL, 0200);
-MODULE_PARM_DESC(phram, "Memory region to map. 
\"phram=,,\"");
+MODULE_PARM_DESC(phram, "Memory region to map. 
\"phram=,,[,]\"");
 
 
 static int __init init_phram(void)
-- 
2.12.3



Re: [PATCH] phram: Allow the user to set the erase page size.

2013-03-02 Thread Artem Bityutskiy
On Fri, 2013-02-15 at 12:35 -0800, Patrick O'Grady wrote:
> From: Patrick O'Grady 
> 
> Permit the user to specify the erase page size as a parameter.
> This solves two problems:
> 
> - phram can access images made by mkfs.jffs2.  mkfs.jffs2 won't
> create images with erase sizes less than 8KiB; many architectures
> define PAGE_SIZE as 4KiB.
> 
> - Allows more effective use of small capacity devices.  JFFS2
> needs somewhere between 2 and 5 empty pages for garbage collection;
> and for an NVRAM part with only 32KiB of space, a smaller erase page
> allows much better utilization in applications where garbage collection
> is important.

Unfortunately, the patch is line-wrapped and not applicable to the
latest kernel tree. Would you please refresh and re-send properly?

-- 
Best Regards,
Artem Bityutskiy

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


Re: [PATCH] phram: Allow the user to set the erase page size.

2013-03-02 Thread Artem Bityutskiy
On Fri, 2013-02-15 at 12:35 -0800, Patrick O'Grady wrote:
 From: Patrick O'Grady patr...@baymotion.com
 
 Permit the user to specify the erase page size as a parameter.
 This solves two problems:
 
 - phram can access images made by mkfs.jffs2.  mkfs.jffs2 won't
 create images with erase sizes less than 8KiB; many architectures
 define PAGE_SIZE as 4KiB.
 
 - Allows more effective use of small capacity devices.  JFFS2
 needs somewhere between 2 and 5 empty pages for garbage collection;
 and for an NVRAM part with only 32KiB of space, a smaller erase page
 allows much better utilization in applications where garbage collection
 is important.

Unfortunately, the patch is line-wrapped and not applicable to the
latest kernel tree. Would you please refresh and re-send properly?

-- 
Best Regards,
Artem Bityutskiy

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


Re: [PATCH] phram: Allow the user to set the erase page size.

2013-02-15 Thread Jörn Engel
On Fri, 15 February 2013 12:35:46 -0800, Patrick O'Grady wrote:
> 
> Permit the user to specify the erase page size as a parameter.
> This solves two problems:
> 
> - phram can access images made by mkfs.jffs2.  mkfs.jffs2 won't
> create images with erase sizes less than 8KiB; many architectures
> define PAGE_SIZE as 4KiB.
> 
> - Allows more effective use of small capacity devices.  JFFS2
> needs somewhere between 2 and 5 empty pages for garbage collection;
> and for an NVRAM part with only 32KiB of space, a smaller erase page
> allows much better utilization in applications where garbage collection
> is important.
> 
> Signed-off-by: Patrick O'Grady 

I can't see any argument against this.

Reviewed-by: Joern Engel 

Jörn

--
"[One] doesn't need to know [...] how to cause a headache in order
to take an aspirin."
-- Scott Culp, Manager of the Microsoft Security Response Center, 2001
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] phram: Allow the user to set the erase page size.

2013-02-15 Thread Patrick O'Grady
From: Patrick O'Grady 

Permit the user to specify the erase page size as a parameter.
This solves two problems:

- phram can access images made by mkfs.jffs2.  mkfs.jffs2 won't
create images with erase sizes less than 8KiB; many architectures
define PAGE_SIZE as 4KiB.

- Allows more effective use of small capacity devices.  JFFS2
needs somewhere between 2 and 5 empty pages for garbage collection;
and for an NVRAM part with only 32KiB of space, a smaller erase page
allows much better utilization in applications where garbage collection
is important.

Signed-off-by: Patrick O'Grady 
---
diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index 67823de..5eee2a6 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -5,14 +5,15 @@
  * Usage:
  *
  * one commend line parameter per device, each in the form:
- *   phram=,,
+ *   phram=,,[,]
  *  may be up to 63 characters.
- *  and  can be octal, decimal or hexadecimal.  If followed
- * by "ki", "Mi" or "Gi", the numbers will be interpreted as kilo, mega or
- * gigabytes.
+ * , , and  can be octal, decimal or
+ * hexadecimal.  If followed by "ki", "Mi" or "Gi", the numbers
+ * will be interpreted as kilo, mega or gigabytes.  
+ * is optional and defaults to PAGE_SIZE.
  *
  * Example:
- * phram=swap,64Mi,128Mi phram=test,900Mi,1Mi
+ * phram=swap,64Mi,128Mi phram=test,900Mi,1Mi,64Ki
  */

 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -94,7 +95,8 @@ static void unregister_devices(void)
}
 }

-static int register_device(char *name, unsigned long start, unsigned long len)
+static int register_device(char *name, unsigned long start,
+   unsigned long len, unsigned long erasesize)
 {
struct phram_mtd_list *new;
int ret = -ENOMEM;
@@ -121,7 +123,7 @@ static int register_device(char *name, unsigned
long start, unsigned long len)
new->mtd._write = phram_write;
new->mtd.owner = THIS_MODULE;
new->mtd.type = MTD_RAM;
-   new->mtd.erasesize = PAGE_SIZE;
+   new->mtd.erasesize = erasesize;
new->mtd.writesize = 1;

ret = -EAGAIN;
@@ -207,21 +209,22 @@ static inline void kill_final_newline(char *str)

 /*
  * This shall contain the module parameter if any. It is of the form:
- * - phram=,, for module case
- * - phram.phram=,, for built-in case
- * We leave 64 bytes for the device name, 12 for the address and 12 for the
- * size.
- * Example: phram.phram=rootfs,0xa000,512Mi
+ * - phram=,,[,] for module case
+ * - phram.phram=,,[,] for built-in case
+ * We leave 64 bytes for the device name, 12 for the address, 12 for the
+ * size, and 12 for the erasesize.
+ * Example: phram.phram=rootfs,0xa000,512Mi,65536
  */
 static __initdata char phram_paramline[64+12+12];

 static int __init phram_setup(const char *val)
 {
-   char buf[64+12+12], *str = buf;
+   char buf[64+12+12+12], *str = buf;
char *token[3];
char *name;
uint32_t start;
uint32_t len;
+   uint32_t erasesize = PAGE_SIZE ;
int i, ret;

if (strnlen(val, sizeof(buf)) >= sizeof(buf))
@@ -230,7 +233,7 @@ static int __init phram_setup(const char *val)
strcpy(str, val);
kill_final_newline(str);

-   for (i=0; i<3; i++)
+   for (i = 0; i < 4; i++)
token[i] = strsep(, ",");

if (str)
@@ -255,7 +258,15 @@ static int __init phram_setup(const char *val)
parse_err("illegal device length\n");
}

-   ret = register_device(name, start, len);
+   if (token[3]) {
+   ret = parse_num32(, token[3]);
+   if (ret) {
+   kfree(name);
+   parse_err("illegal erase size\n");
+   }
+   }
+
+   ret = register_device(name, start, len, erasesize);
if (!ret)
pr_info("%s device: %#x at %#x\n", name, len, start);
else
@@ -278,7 +289,7 @@ static int __init phram_param_call(const char
*val, struct kernel_param *kp)
 }

 module_param_call(phram, phram_param_call, NULL, NULL, 000);
-MODULE_PARM_DESC(phram, "Memory region to map.
\"phram=,,\"");
+MODULE_PARM_DESC(phram, "Memory region to map.
\"phram=,,[,]\"");


 static int __init init_phram(void)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] phram: Allow the user to set the erase page size.

2013-02-15 Thread Patrick O'Grady
From: Patrick O'Grady patr...@baymotion.com

Permit the user to specify the erase page size as a parameter.
This solves two problems:

- phram can access images made by mkfs.jffs2.  mkfs.jffs2 won't
create images with erase sizes less than 8KiB; many architectures
define PAGE_SIZE as 4KiB.

- Allows more effective use of small capacity devices.  JFFS2
needs somewhere between 2 and 5 empty pages for garbage collection;
and for an NVRAM part with only 32KiB of space, a smaller erase page
allows much better utilization in applications where garbage collection
is important.

Signed-off-by: Patrick O'Grady patr...@baymotion.com
---
diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index 67823de..5eee2a6 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -5,14 +5,15 @@
  * Usage:
  *
  * one commend line parameter per device, each in the form:
- *   phram=name,start,len
+ *   phram=name,start,len[,erasesize]
  * name may be up to 63 characters.
- * start and len can be octal, decimal or hexadecimal.  If followed
- * by ki, Mi or Gi, the numbers will be interpreted as kilo, mega or
- * gigabytes.
+ * start, len, and erasesize can be octal, decimal or
+ * hexadecimal.  If followed by ki, Mi or Gi, the numbers
+ * will be interpreted as kilo, mega or gigabytes.  erasesize
+ * is optional and defaults to PAGE_SIZE.
  *
  * Example:
- * phram=swap,64Mi,128Mi phram=test,900Mi,1Mi
+ * phram=swap,64Mi,128Mi phram=test,900Mi,1Mi,64Ki
  */

 #define pr_fmt(fmt) KBUILD_MODNAME :  fmt
@@ -94,7 +95,8 @@ static void unregister_devices(void)
}
 }

-static int register_device(char *name, unsigned long start, unsigned long len)
+static int register_device(char *name, unsigned long start,
+   unsigned long len, unsigned long erasesize)
 {
struct phram_mtd_list *new;
int ret = -ENOMEM;
@@ -121,7 +123,7 @@ static int register_device(char *name, unsigned
long start, unsigned long len)
new-mtd._write = phram_write;
new-mtd.owner = THIS_MODULE;
new-mtd.type = MTD_RAM;
-   new-mtd.erasesize = PAGE_SIZE;
+   new-mtd.erasesize = erasesize;
new-mtd.writesize = 1;

ret = -EAGAIN;
@@ -207,21 +209,22 @@ static inline void kill_final_newline(char *str)

 /*
  * This shall contain the module parameter if any. It is of the form:
- * - phram=device,address,size for module case
- * - phram.phram=device,address,size for built-in case
- * We leave 64 bytes for the device name, 12 for the address and 12 for the
- * size.
- * Example: phram.phram=rootfs,0xa000,512Mi
+ * - phram=device,address,size[,erasesize] for module case
+ * - phram.phram=device,address,size[,erasesize] for built-in case
+ * We leave 64 bytes for the device name, 12 for the address, 12 for the
+ * size, and 12 for the erasesize.
+ * Example: phram.phram=rootfs,0xa000,512Mi,65536
  */
 static __initdata char phram_paramline[64+12+12];

 static int __init phram_setup(const char *val)
 {
-   char buf[64+12+12], *str = buf;
+   char buf[64+12+12+12], *str = buf;
char *token[3];
char *name;
uint32_t start;
uint32_t len;
+   uint32_t erasesize = PAGE_SIZE ;
int i, ret;

if (strnlen(val, sizeof(buf)) = sizeof(buf))
@@ -230,7 +233,7 @@ static int __init phram_setup(const char *val)
strcpy(str, val);
kill_final_newline(str);

-   for (i=0; i3; i++)
+   for (i = 0; i  4; i++)
token[i] = strsep(str, ,);

if (str)
@@ -255,7 +258,15 @@ static int __init phram_setup(const char *val)
parse_err(illegal device length\n);
}

-   ret = register_device(name, start, len);
+   if (token[3]) {
+   ret = parse_num32(erasesize, token[3]);
+   if (ret) {
+   kfree(name);
+   parse_err(illegal erase size\n);
+   }
+   }
+
+   ret = register_device(name, start, len, erasesize);
if (!ret)
pr_info(%s device: %#x at %#x\n, name, len, start);
else
@@ -278,7 +289,7 @@ static int __init phram_param_call(const char
*val, struct kernel_param *kp)
 }

 module_param_call(phram, phram_param_call, NULL, NULL, 000);
-MODULE_PARM_DESC(phram, Memory region to map.
\phram=name,start,length\);
+MODULE_PARM_DESC(phram, Memory region to map.
\phram=name,start,length[,erasesize]\);


 static int __init init_phram(void)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] phram: Allow the user to set the erase page size.

2013-02-15 Thread Jörn Engel
On Fri, 15 February 2013 12:35:46 -0800, Patrick O'Grady wrote:
 
 Permit the user to specify the erase page size as a parameter.
 This solves two problems:
 
 - phram can access images made by mkfs.jffs2.  mkfs.jffs2 won't
 create images with erase sizes less than 8KiB; many architectures
 define PAGE_SIZE as 4KiB.
 
 - Allows more effective use of small capacity devices.  JFFS2
 needs somewhere between 2 and 5 empty pages for garbage collection;
 and for an NVRAM part with only 32KiB of space, a smaller erase page
 allows much better utilization in applications where garbage collection
 is important.
 
 Signed-off-by: Patrick O'Grady patr...@baymotion.com

I can't see any argument against this.

Reviewed-by: Joern Engel jo...@logfs.org

Jörn

--
[One] doesn't need to know [...] how to cause a headache in order
to take an aspirin.
-- Scott Culp, Manager of the Microsoft Security Response Center, 2001
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/