Skip to contentSkip to conversation

mtd: phram: Make phram 64-bit compatible

PATCH Applied Alexander Sverdlin 4 participants 6 messages Oct 2, 2013 – Oct 8, 2013

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mtd: phram: Make phram 64-bit compatible

mtd: phram: Make phram 64-bit compatible

phram was 32-bit limited by design. Machines are growing up, but phram module is still useful. Update it. The patch is bigger than minimum, because simple_strtoul() is obsolete.

Tested on MIPS64 and compile-tested for PPC (32 bit).

---

drivers/mtd/devices/phram.c +32 -32
@@ -94,7 +94,7 @@ static void unregister_devices(void)
 	}
 }
 
-static int register_device(char *name, unsigned long start, unsigned long len)
+static int register_device(char *name, phys_addr_t start, size_t len)
 {
 	struct phram_mtd_list *new;
 	int ret = -ENOMEM;
@@ -141,35 +141,35 @@ out0:
 	return ret;
 }
 
-static int ustrtoul(const char *cp, char **endp, unsigned int base)
+static int parse_num64(uint64_t *num64, char *token)
 {
-	unsigned long result = simple_strtoul(cp, endp, base);
+	size_t len;
+	int shift = 0;
+	int ret;
 
-	switch (**endp) {
-	case 'G':
-		result *= 1024;
-	case 'M':
-		result *= 1024;
-	case 'k':
-		result *= 1024;
+	len = strlen(token);
 	/* By dwmw2 editorial decree, "ki", "Mi" or "Gi" are to be used. */
-		if ((*endp)[1] == 'i')
-			(*endp) += 2;
+	if (len > 2) {
+		if (token[len - 1] == 'i') {
+			switch (token[len - 2]) {
+			case 'G':
+				shift += 10;
+			case 'M':
+				shift += 10;
+			case 'k':
+				shift += 10;
+				token[len - 2] = 0;
+				break;
+			default:
+				return -EINVAL;
+			}
+		}
 	}
-	return result;
-}
-
-static int parse_num32(uint32_t *num32, const char *token)
-{
-	char *endp;
-	unsigned long n;
 
-	n = ustrtoul(token, &endp, 0);
-	if (*endp)
-		return -EINVAL;
+	ret = kstrtou64(token, 0, num64);
+	*num64 <<= shift;
 
-	*num32 = n;
-	return 0;
+	return ret;
 }
 
 static int parse_name(char **pname, const char *token)
@@ -209,19 +209,19 @@ static inline void kill_final_newline(ch
  * 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
+ * We leave 64 bytes for the device name, 20 for the address and 20 for the
  * size.
  * Example: phram.phram=rootfs,0xa0000000,512Mi
  */
-static __initdata char phram_paramline[64+12+12];
+static __initdata char phram_paramline[64 + 20 + 20];
 
 static int __init phram_setup(const char *val)
 {
-	char buf[64+12+12], *str = buf;
+	char buf[64 + 20 + 20], *str = buf;
 	char *token[3];
 	char *name;
-	uint32_t start;
-	uint32_t len;
+	uint64_t start;
+	uint64_t len;
 	int i, ret;
 
 	if (strnlen(val, sizeof(buf)) >= sizeof(buf))
@@ -243,13 +243,13 @@ static int __init phram_setup(const char
 	if (ret)
 		return ret;
 
-	ret = parse_num32(&start, token[1]);
+	ret = parse_num64(&start, token[1]);
 	if (ret) {
 		kfree(name);
 		parse_err("illegal start address\n");
 	}
 
-	ret = parse_num32(&len, token[2]);
+	ret = parse_num64(&len, token[2]);
 	if (ret) {
 		kfree(name);
 		parse_err("illegal device length\n");
@@ -257,7 +257,7 @@ static int __init phram_setup(const char
 
 	ret = register_device(name, start, len);
 	if (!ret)
-		pr_info("%s device: %#x at %#x\n", name, len, start);
+		pr_info("%s device: %#llx at %#llx\n", name, len, start);
 	else
 		kfree(name);
Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · Alexander Sverdlin

mtd: phram: Make phram 64-bit compatible

phram was 32-bit limited by design. Machines are growing up, but phram module is still useful. Update it. The patch is bigger than minimum, because simple_strtoul() is obsolete.

Tested on MIPS64 and compile-tested for PPC (32 bit).

I quite like it.

Jörn

--
Antivirus is homeopathy for computers: This software was in contact with malware in the past, so it’ll recognize other malware in the future. $79
-- thegrugq

Applied
#2 lore raw
In reply to Alexander Sverdlin

+ LKML

Quoted 9 lines · Alexander Sverdlin

mtd: phram: Make phram 64-bit compatible

phram was 32-bit limited by design. Machines are growing up, but phram module is still useful. Update it. The patch is bigger than minimum, because simple_strtoul() is obsolete.

Tested on MIPS64 and compile-tested for PPC (32 bit).

Pushed to l2-mtd.git. Thanks!

Can this driver use the library memparse() function instead of open-coding it?

[To answer myself:] I noticed this in drivers/mtd/devices/phram.c, which prevents us from using memparse():

/* By dwmw2 editorial decree, "ki", "Mi" or "Gi" are to be used. */

Are we (MTD) holding a revolutionary position against the standard kernel libraries, which recognize [KkMmGg] prefixes, but not [kMG]i prefixes? Should we extend memparse() to accept either form? Or would doing so simply pollute the library and not really satisfy anyone?

[After more research:] It looks like this topic may be the subject of some long-past flame wars. If I am digging up past demons, then I'd prefer to let sleeping Balrogs lie.

Brian

#3 lore raw
Quoted 4 lines · Brian Norris

[After more research:] It looks like this topic may be the subject of some long-past flame wars. If I am digging up past demons, then I'd prefer to let sleeping Balrogs lie.

Most of the time it is obvious from context whether you want base-1000 or base-1024 numbers. So in the common case the extra letter is plain annoying. In less common cases it matters a lot and lack of the extra letter is rather irritating.

One possible solution would be to have three suffixes. Ki for base-1024, Kd for base-1000 and K for "I don't care, you decide for me". But I am sure that would simply cause another round of flamewars.

It is not a hard technical problem. As a result, noone on this list has the required expertise to solve it.

Jörn

--
Money can buy bandwidth, but latency is forever.
-- John R. Mashey

#4 lore raw
Quoted 4 lines · Jörn Engel

One possible solution would be to have three suffixes. Ki for base-1024, Kd for base-1000 and K for "I don't care, you decide for me". But I am sure that would simply cause another round of flamewars.

Indeed. Why deviate from SI with "Kd" (a new invention)?? That should just be "k".

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#5 lore raw
In reply to Brian Norris

Hi!

Quoted 4 lines · ext Brian Norris

Are we (MTD) holding a revolutionary position against the standard kernel libraries, which recognize [KkMmGg] prefixes, but not [kMG]i prefixes? Should we extend memparse() to accept either form? Or would doing so simply pollute the library and not really satisfy anyone?

I was also irritated re-inventing the wheel here, but the problem is -- millions of people out there have their startup scripts and uboot environments for phram, who expect this just to work with the next kernel upgrade...

Signature · 2 lines
Best regards,
Alexander Sverdlin.