* [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons @ 2020-04-29 16:53 Ronald G. Minnich 2020-11-22 0:14 ` Sven Eckelmann 0 siblings, 1 reply; 13+ messages in thread From: Ronald G. Minnich @ 2020-04-29 16:53 UTC (permalink / raw) Cc: Boris Brezillon, Ron Minnich, David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut, Richard Weinberger, Cyrille Pitchen, linux-mtd, linux-kernel From: Boris Brezillon <boris.brezillon@collabora.com> Looks like some drivers define MTD names with a colon in it, thus making mtdpart= parsing impossible. Let's fix the parser to gracefully handle that case: the last ':' in a partition definition sequence is considered instead of the first one. Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> Signed-off-by: Ron Minnich <rminnich@google.com> Tested-by: Ron Minnich <rminnich@google.com> --- drivers/mtd/parsers/cmdlinepart.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/parsers/cmdlinepart.c b/drivers/mtd/parsers/cmdlinepart.c index c86f2db8c882..0625b25620ca 100644 --- a/drivers/mtd/parsers/cmdlinepart.c +++ b/drivers/mtd/parsers/cmdlinepart.c @@ -218,12 +218,29 @@ static int mtdpart_setup_real(char *s) struct cmdline_mtd_partition *this_mtd; struct mtd_partition *parts; int mtd_id_len, num_parts; - char *p, *mtd_id; + char *p, *mtd_id, *semicol; + + /* + * Replace the first ';' by a NULL char so strrchr can work + * properly. + */ + semicol = strchr(s, ';'); + if (semicol) + *semicol = '\0'; mtd_id = s; - /* fetch <mtd-id> */ - p = strchr(s, ':'); + /* + * fetch <mtd-id>. We use strrchr to ignore all ':' that could + * be present in the MTD name, only the last one is interpreted + * as an <mtd-id>/<part-definition> separator. + */ + p = strrchr(s, ':'); + + /* Restore the ';' now. */ + if (semicol) + *semicol = ';'; + if (!p) { pr_err("no mtd-id\n"); return -EINVAL; ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-04-29 16:53 [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons Ronald G. Minnich @ 2020-11-22 0:14 ` Sven Eckelmann 2020-11-27 16:32 ` ron minnich 0 siblings, 1 reply; 13+ messages in thread From: Sven Eckelmann @ 2020-11-22 0:14 UTC (permalink / raw) To: linux-mtd, John Audia, Adrian Schmutzler, jstefek Cc: Boris Brezillon, Richard Weinberger, Cyrille Pitchen, linux-kernel, Marek Vasut, Boris Brezillon, linux-mtd, Ron Minnich, Brian Norris, David Woodhouse, Ronald G. Minnich, Sasha Levin [-- Attachment #1.1: Type: text/plain, Size: 2146 bytes --] On Wednesday, 29 April 2020 18:53:47 CET Ronald G. Minnich wrote: > From: Boris Brezillon <boris.brezillon@collabora.com> > > Looks like some drivers define MTD names with a colon in it, thus > making mtdpart= parsing impossible. Let's fix the parser to gracefully > handle that case: the last ':' in a partition definition sequence is > considered instead of the first one. > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > Signed-off-by: Ron Minnich <rminnich@google.com> > Tested-by: Ron Minnich <rminnich@google.com> > --- > drivers/mtd/parsers/cmdlinepart.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) This change broke OpenWrt booting on some IPQ40xx devices. Here for example the parts of the cmdline which the u-boot on an OpenMesh A62 sets automatically: root=31:11 mtdparts=spi0.0:256k(0:SBL1),128k(0:MIBIB),384k(0:QSEE),64k(0:CDT),64k(0:DDRPARAMS),64k(0:APPSBLENV),512k(0:APPSBL),64k(0:ART),64k(custom),64k(0:KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) rootfsname=rootfs rootwait This is then parsed by newpart as: KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) And of course, this results in: mtd: partition has size 0 [...] /dev/root: Can't open blockdev VFS: Cannot open root device "31:11" or unknown-block(31,11): error -6 Please append a correct "root=" boot option; here are the available partitions: 1f00 32768 mtdblock0 (driver?) Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,11) CPU1: stopping CPU: 1 PID: 0 Comm: This affects OpenWrt since the commit d6a9a92e3217 ("kernel: bump 5.4 to 5.4.69") because this change was backported to Linux v5.4.69. Reverting this change fixes the problem for me. And if I see it correctly, this is also affecting the kernel (4.14) for the OpenWrt 18.06.x + 19.07.x branch because it can also be found in there as part of the v4.14.200 release. Another workaround is to replace the first "(" with a NULL too. See the attached patch for the one which I used to fix the OpenWrt bootup. Kind regards, Sven [-- Attachment #1.2: 499-mtd-parser-cmdline-Fix-parsing-of-part-names-with-co.patch --] [-- Type: text/x-patch, Size: 2308 bytes --] From: Sven Eckelmann <sven@narfation.org> Date: Sun, 22 Nov 2020 00:48:33 +0100 Subject: [PATCH RFC] mtd: parser: cmdline: Fix parsing of part-names with colons Some devices (especially QCA ones) are already using hardcoded partition names with colons in it. The OpenMesh A62 for example provides following mtd relevant information via cmdline: root=31:11 mtdparts=spi0.0:256k(0:SBL1),128k(0:MIBIB),384k(0:QSEE),64k(0:CDT),64k(0:DDRPARAMS),64k(0:APPSBLENV),512k(0:APPSBL),64k(0:ART),64k(custom),64k(0:KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) rootfsname=rootfs rootwait The change to split only on the last colon between mtd-id and partitions will cause newpart to see following string for the first partition: KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) Such a partition list cannot be parsed and thus the device fails to boot. Avoid this behavior by making sure that the start of the first part-name ("(") will also be the last byte the colon search algorithm is using for its mtd-id search. Fixes: eb13fa022741 ("mtd: parser: cmdline: Support MTD names containing one or more colons") Signed-off-by: Sven Eckelmann <sven@narfation.org> diff --git a/drivers/mtd/parsers/cmdlinepart.c b/drivers/mtd/parsers/cmdlinepart.c index 0625b25620ca766318ea4646a6e3761ff4d3a4cc..22881ea4c132ea5a5ba7aebd025d91bf1cd023af 100644 --- a/drivers/mtd/parsers/cmdlinepart.c +++ b/drivers/mtd/parsers/cmdlinepart.c @@ -218,7 +218,7 @@ static int mtdpart_setup_real(char *s) struct cmdline_mtd_partition *this_mtd; struct mtd_partition *parts; int mtd_id_len, num_parts; - char *p, *mtd_id, *semicol; + char *p, *mtd_id, *semicol, *open_parenth; /* * Replace the first ';' by a NULL char so strrchr can work @@ -228,6 +228,13 @@ static int mtdpart_setup_real(char *s) if (semicol) *semicol = '\0'; + /* make sure that part-names with ":" will not be handled as + * part of the mtd-id with an ":" + */ + open_parenth = strchr(s, '('); + if (open_parenth) + *open_parenth = '\0'; + mtd_id = s; /* @@ -237,6 +244,10 @@ static int mtdpart_setup_real(char *s) */ p = strrchr(s, ':'); + /* Restore the '(' now. */ + if (open_parenth) + *open_parenth = '('; + /* Restore the ';' now. */ if (semicol) *semicol = ';'; [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-11-22 0:14 ` Sven Eckelmann @ 2020-11-27 16:32 ` ron minnich 2020-11-27 17:05 ` Sven Eckelmann 0 siblings, 1 reply; 13+ messages in thread From: ron minnich @ 2020-11-27 16:32 UTC (permalink / raw) To: Sven Eckelmann Cc: linux-mtd, John Audia, Adrian Schmutzler, jstefek, Boris Brezillon, Richard Weinberger, Cyrille Pitchen, lkml - Kernel Mailing List, Marek Vasut, Boris Brezillon, Ron Minnich, Brian Norris, David Woodhouse, Sasha Levin I'm a bit worried about how tricky this starts to get. I'm inclined to go back to an earlier implementation which used a character that had not yet been used (iirc I used [] around the PCI ID in a very early version). What if we used, e.g, a single ! and searched for that? It need not be !; pick a character. Just something not already in use, as the ambiguity around which ':' delimits the device has become an issue, as you show. Almost nothing in the original patch would change, save the character being searched for. By using a character we'd never used, we'd avoid breaking existing usage. Comments? On Sat, Nov 21, 2020 at 4:14 PM Sven Eckelmann <sven@narfation.org> wrote: > > On Wednesday, 29 April 2020 18:53:47 CET Ronald G. Minnich wrote: > > From: Boris Brezillon <boris.brezillon@collabora.com> > > > > Looks like some drivers define MTD names with a colon in it, thus > > making mtdpart= parsing impossible. Let's fix the parser to gracefully > > handle that case: the last ':' in a partition definition sequence is > > considered instead of the first one. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > Signed-off-by: Ron Minnich <rminnich@google.com> > > Tested-by: Ron Minnich <rminnich@google.com> > > --- > > drivers/mtd/parsers/cmdlinepart.c | 23 ++++++++++++++++++++--- > > 1 file changed, 20 insertions(+), 3 deletions(-) > > This change broke OpenWrt booting on some IPQ40xx devices. Here for example > the parts of the cmdline which the u-boot on an OpenMesh A62 sets > automatically: > > root=31:11 mtdparts=spi0.0:256k(0:SBL1),128k(0:MIBIB),384k(0:QSEE),64k(0:CDT),64k(0:DDRPARAMS),64k(0:APPSBLENV),512k(0:APPSBL),64k(0:ART),64k(custom),64k(0:KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) rootfsname=rootfs rootwait > > This is then parsed by newpart as: KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) > > And of course, this results in: > > mtd: partition has size 0 > [...] > /dev/root: Can't open blockdev > VFS: Cannot open root device "31:11" or unknown-block(31,11): error -6 > Please append a correct "root=" boot option; here are the available partitions: > 1f00 32768 mtdblock0 > (driver?) > Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,11) > CPU1: stopping > CPU: 1 PID: 0 Comm: > > This affects OpenWrt since the commit d6a9a92e3217 ("kernel: bump 5.4 to > 5.4.69") because this change was backported to Linux v5.4.69. Reverting > this change fixes the problem for me. > > And if I see it correctly, this is also affecting the kernel (4.14) for > the OpenWrt 18.06.x + 19.07.x branch because it can also be found in > there as part of the v4.14.200 release. > > Another workaround is to replace the first "(" with a NULL too. See the > attached patch for the one which I used to fix the OpenWrt bootup. > > Kind regards, > Sven ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-11-27 16:32 ` ron minnich @ 2020-11-27 17:05 ` Sven Eckelmann 2020-11-27 17:16 ` ron minnich 0 siblings, 1 reply; 13+ messages in thread From: Sven Eckelmann @ 2020-11-27 17:05 UTC (permalink / raw) To: ron minnich Cc: linux-mtd, John Audia, Adrian Schmutzler, jstefek, Boris Brezillon, Richard Weinberger, Cyrille Pitchen, lkml - Kernel Mailing List, Marek Vasut, Boris Brezillon, Ron Minnich, Brian Norris, David Woodhouse, Sasha Levin [-- Attachment #1: Type: text/plain, Size: 983 bytes --] On Friday, 27 November 2020 17:32:02 CET ron minnich wrote: > I'm a bit worried about how tricky this starts to get. I'm inclined to > go back to an earlier implementation which used a character that had > not yet been used (iirc I used [] around the PCI ID in a very early > version). What if we used, e.g, a single ! and searched for that? It > need not be !; pick a character. Just something not already in use, as > the ambiguity around which ':' delimits the device has become an > issue, as you show. > > Almost nothing in the original patch would change, save the character > being searched for. By using a character we'd never used, we'd avoid > breaking existing usage. What? Doesn't make any sense to me. The mtdparts shown in the the commit message is as it is. I cannot simply change it because it is in the control of the bootloader - not the linux kernel or me. So I can also not introduce a different character like ! for separating things. KInd regards, Sven [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-11-27 17:05 ` Sven Eckelmann @ 2020-11-27 17:16 ` ron minnich 2020-11-27 17:35 ` Sven Eckelmann 0 siblings, 1 reply; 13+ messages in thread From: ron minnich @ 2020-11-27 17:16 UTC (permalink / raw) To: Sven Eckelmann Cc: linux-mtd, John Audia, Adrian Schmutzler, jstefek, Richard Weinberger, Cyrille Pitchen, lkml - Kernel Mailing List, Marek Vasut, Boris Brezillon, Ron Minnich, Brian Norris, David Woodhouse, Sasha Levin Ah ha, I see how different my world is from yours :-) My world is linuxboot, which means the command line is always under control of linux, as linux is my bootloader. So this kind of thing is very easy for me to change. Let me go back to the initial problem and hope I can make better sense. The original problem was that ':' was used to separate the device from the partitions. A problem came along in that PCI device specifiers have ':', and the original parsing code broke. The patch that caused you trouble fixed that by using the right-most ':' as the separator for device and partitions. What none of the people involved in the original patch knew was that there would be other ':' in use. Sorry! But you are right, my idea is a complete non-starter, don't know what I was thinking. So it seems your patch, if it works, is the way to go? I can't think of anything better that lets us preserve current behavior and support PCI device specifiers? Ron On Fri, Nov 27, 2020 at 9:06 AM Sven Eckelmann <sven@narfation.org> wrote: > > On Friday, 27 November 2020 17:32:02 CET ron minnich wrote: > > I'm a bit worried about how tricky this starts to get. I'm inclined to > > go back to an earlier implementation which used a character that had > > not yet been used (iirc I used [] around the PCI ID in a very early > > version). What if we used, e.g, a single ! and searched for that? It > > need not be !; pick a character. Just something not already in use, as > > the ambiguity around which ':' delimits the device has become an > > issue, as you show. > > > > Almost nothing in the original patch would change, save the character > > being searched for. By using a character we'd never used, we'd avoid > > breaking existing usage. > > What? Doesn't make any sense to me. The mtdparts shown in the the commit > message is as it is. I cannot simply change it because it is in the control of > the bootloader - not the linux kernel or me. So I can also not introduce a > different character like ! for separating things. > > KInd regards, > Sven > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-11-27 17:16 ` ron minnich @ 2020-11-27 17:35 ` Sven Eckelmann 2020-11-27 18:54 ` ron minnich 0 siblings, 1 reply; 13+ messages in thread From: Sven Eckelmann @ 2020-11-27 17:35 UTC (permalink / raw) To: ron minnich Cc: linux-mtd, John Audia, Adrian Schmutzler, jstefek, Richard Weinberger, Cyrille Pitchen, lkml - Kernel Mailing List, Marek Vasut, Boris Brezillon, Ron Minnich, Brian Norris, David Woodhouse, Sasha Levin [-- Attachment #1: Type: text/plain, Size: 1804 bytes --] On Friday, 27 November 2020 18:16:54 CET ron minnich wrote: > What none of the people involved in the original patch knew was that > there would be other ':' in use. Sorry! > > But you are right, my idea is a complete non-starter, don't know what > I was thinking. I am still not sure because I still didn't get what you actually wanted to change. I first thought that you wanted to change mtdparts=spi0.0:256k(0:SBL1) to mtdparts=spi0.0!256k(0:SBL1) which wouldn't work for me when ":" is not supported anymore. And it would break a lot of already working installations. But maybe I completely misread it. Maybe you wanted to introduce an optional(!!!) stop marker like ! mtdparts=spi0.0!:256k(0:SBL1) to inform the parser that it doesn't have to search for : before the !. While this could work for me, I am not qualified enough to say which character is not yet used and can be utilized. But the note about [ and ] at least makes sense to me (if it is optional): mtdparts=[spi0.0]:256k(0:SBL1) But I am not sure if this will be a problem for people which already adopted PCI IDs inside the mtdparts without [ and ]. > So it seems your patch, if it works, is the way to go? At least this is a workaround [1] which can be pushed to all the stable kernels which broke with the "Support MTD names containing one or more colons" patch. And the one which OpenWrt adopted now to get the devices booting again. It is only waiting for a Tested-by from you. > I can't think > of anything better that lets us preserve current behavior and support > PCI device specifiers? I am not that deep in this topic. So I am not sure what else could be done. Kind regards, Sven [1] https://patchwork.ozlabs.org/project/linux-mtd/patch/20201124062506.185392-1-sven@narfation.org/ [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-11-27 17:35 ` Sven Eckelmann @ 2020-11-27 18:54 ` ron minnich 2020-12-07 7:52 ` Sven Eckelmann 0 siblings, 1 reply; 13+ messages in thread From: ron minnich @ 2020-11-27 18:54 UTC (permalink / raw) To: Sven Eckelmann Cc: linux-mtd, John Audia, Adrian Schmutzler, jstefek, Richard Weinberger, Cyrille Pitchen, lkml - Kernel Mailing List, Marek Vasut, Boris Brezillon, Ron Minnich, Brian Norris, David Woodhouse, Sasha Levin Thanks, Sven, for your patience, I will indeed try to test this next week. On Fri, Nov 27, 2020 at 9:35 AM Sven Eckelmann <sven@narfation.org> wrote: > > On Friday, 27 November 2020 18:16:54 CET ron minnich wrote: > > What none of the people involved in the original patch knew was that > > there would be other ':' in use. Sorry! > > > > But you are right, my idea is a complete non-starter, don't know what > > I was thinking. > > I am still not sure because I still didn't get what you actually wanted to > change. I first thought that you wanted to change > > mtdparts=spi0.0:256k(0:SBL1) > > to > > mtdparts=spi0.0!256k(0:SBL1) > > which wouldn't work for me when ":" is not supported anymore. And it would > break a lot of already working installations. > > But maybe I completely misread it. Maybe you wanted to introduce an > optional(!!!) stop marker like ! > > mtdparts=spi0.0!:256k(0:SBL1) > > to inform the parser that it doesn't have to search for : before the !. While > this could work for me, I am not qualified enough to say which character is > not yet used and can be utilized. > > But the note about [ and ] at least makes sense to me (if it is optional): > > mtdparts=[spi0.0]:256k(0:SBL1) > > But I am not sure if this will be a problem for people which already adopted > PCI IDs inside the mtdparts without [ and ]. > > > So it seems your patch, if it works, is the way to go? > > At least this is a workaround [1] which can be pushed to all the stable > kernels which broke with the "Support MTD names containing one or more colons" > patch. And the one which OpenWrt adopted now to get the devices booting again. > It is only waiting for a Tested-by from you. > > > I can't think > > of anything better that lets us preserve current behavior and support > > PCI device specifiers? > > I am not that deep in this topic. So I am not sure what else could be done. > > Kind regards, > Sven > > [1] https://patchwork.ozlabs.org/project/linux-mtd/patch/20201124062506.185392-1-sven@narfation.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-11-27 18:54 ` ron minnich @ 2020-12-07 7:52 ` Sven Eckelmann 2020-12-07 15:24 ` ron minnich 0 siblings, 1 reply; 13+ messages in thread From: Sven Eckelmann @ 2020-12-07 7:52 UTC (permalink / raw) To: ron minnich Cc: linux-mtd, John Audia, Adrian Schmutzler, jstefek, Richard Weinberger, Cyrille Pitchen, lkml - Kernel Mailing List, Marek Vasut, Boris Brezillon, Ron Minnich, Brian Norris, David Woodhouse, Sasha Levin [-- Attachment #1: Type: text/plain, Size: 176 bytes --] On Friday, 27 November 2020 19:54:30 CET ron minnich wrote: > Thanks, Sven, for your patience, I will indeed try to test this next week. Any test results? Kind regards, Sven [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-12-07 7:52 ` Sven Eckelmann @ 2020-12-07 15:24 ` ron minnich 2020-12-09 21:34 ` Ron Minnich 0 siblings, 1 reply; 13+ messages in thread From: ron minnich @ 2020-12-07 15:24 UTC (permalink / raw) To: Sven Eckelmann Cc: linux-mtd, John Audia, Adrian Schmutzler, jstefek, Richard Weinberger, Cyrille Pitchen, lkml - Kernel Mailing List, Marek Vasut, Boris Brezillon, Ron Minnich, Brian Norris, David Woodhouse, Sasha Levin I pinged the person again. Hope to hear today. Sorry for delay. On Sun, Dec 6, 2020 at 11:52 PM Sven Eckelmann <sven@narfation.org> wrote: > > On Friday, 27 November 2020 19:54:30 CET ron minnich wrote: > > Thanks, Sven, for your patience, I will indeed try to test this next week. > > Any test results? > > Kind regards, > Sven ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-12-07 15:24 ` ron minnich @ 2020-12-09 21:34 ` Ron Minnich 0 siblings, 0 replies; 13+ messages in thread From: Ron Minnich @ 2020-12-09 21:34 UTC (permalink / raw) To: ron minnich Cc: Sven Eckelmann, linux-mtd, John Audia, Adrian Schmutzler, jstefek, Richard Weinberger, Cyrille Pitchen, lkml - Kernel Mailing List, Marek Vasut, Boris Brezillon, Brian Norris, David Woodhouse, Sasha Levin Tested-by: Ian Goegebuer <goegebuer@google.com> On Mon, Dec 7, 2020 at 7:24 AM ron minnich <rminnich@gmail.com> wrote: > > I pinged the person again. Hope to hear today. Sorry for delay. > > On Sun, Dec 6, 2020 at 11:52 PM Sven Eckelmann <sven@narfation.org> wrote: > > > > On Friday, 27 November 2020 19:54:30 CET ron minnich wrote: > > > Thanks, Sven, for your patience, I will indeed try to test this next week. > > > > Any test results? > > > > Kind regards, > > Sven ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: spi-nor: controllers: intel-spi: Add support for command line partitions
@ 2020-09-25 4:21 Vignesh Raghavendra
2020-12-23 21:56 ` [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons Ian Goegebuer
0 siblings, 1 reply; 13+ messages in thread
From: Vignesh Raghavendra @ 2020-09-25 4:21 UTC (permalink / raw)
To: Mika Westerberg, Ronald G. Minnich
Cc: Ronald G. Minnich, Tudor Ambarus, Miquel Raynal,
Richard Weinberger, Boris Brezillon, Jethro Beekman,
Greg Kroah-Hartman, Alexander Sverdlin, Thomas Gleixner,
linux-mtd, linux-kernel
On 4/17/20 9:37 PM, Mika Westerberg wrote:
> On Fri, Apr 17, 2020 at 08:26:11AM -0700, Ronald G. Minnich wrote:
>> On Intel platforms, the usable SPI area is located several
>> MiB in from the start, to leave room for descriptors and
>> the Management Engine binary. Further, not all the remaining
>> space can be used, as the last 16 MiB contains firmware.
>>
>> To make the SPI usable for mtdblock and other devices,
>> it is necessary to enable command line partitions so the
>> middle usable region can be specified.
>>
>> Add a part_probes array which includes only "cmdelineparts",
>> and change to mtd_device_parse_register to use this part_probes.
>>
>> Signed-off-by: Ronald G. Minnich <rminnich@google.com>
>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>
scripts/checkpatch.pl --strict complains:
CHECK: Alignment should match open parenthesis
#46: FILE: drivers/mtd/spi-nor/controllers/intel-spi.c:956:
+ ret = mtd_device_parse_register(&ispi->nor.mtd, part_probes,
+ NULL, &part, 1);
WARNING: Missing Signed-off-by: line by nominal patch author '"Ronald G. Minnich" <rminnich@gmail.com>'
Regards
Vignesh
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-09-25 4:21 [PATCH] mtd: spi-nor: controllers: intel-spi: Add support for command line partitions Vignesh Raghavendra @ 2020-12-23 21:56 ` Ian Goegebuer 2021-01-04 9:08 ` Miquel Raynal 0 siblings, 1 reply; 13+ messages in thread From: Ian Goegebuer @ 2020-12-23 21:56 UTC (permalink / raw) To: Vignesh Raghavendra Cc: Ronald G . Minnich, Tudor Ambarus, Miquel Raynal, Richard Weinberger, Mika Westerberg, Boris Brezillon, Jethro Beekman, Greg Kroah-Hartman, Alexander Sverdlin, Thomas Gleixner, linux-mtd, linux-kernel, Ian Goegebuer On Intel platforms, the usable SPI area is located several MiB in from the start, to leave room for descriptors and the Management Engine binary. Further, not all the remaining space can be used, as the last 16 MiB contains firmware. To make the SPI usable for mtdblock and other devices, it is necessary to enable command line partitions so the middle usable region can be specified. Add a part_probes array which includes only "cmdelineparts", and change to mtd_device_parse_register to use this part_probes. Signed-off-by: "Ronald G. Minnich" <rminnich@google.com> Signed-off-by: Ian Goegebuer <goegebuer@google.com> --- drivers/mtd/spi-nor/controllers/intel-spi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c index b54a56a68100..9de38851c411 100644 --- a/drivers/mtd/spi-nor/controllers/intel-spi.c +++ b/drivers/mtd/spi-nor/controllers/intel-spi.c @@ -903,6 +903,8 @@ static const struct spi_nor_controller_ops intel_spi_controller_ops = { .erase = intel_spi_erase, }; +static const char * const part_probes[] = { "cmdlinepart", NULL }; + struct intel_spi *intel_spi_probe(struct device *dev, struct resource *mem, const struct intel_spi_boardinfo *info) { @@ -950,7 +952,8 @@ struct intel_spi *intel_spi_probe(struct device *dev, if (!ispi->writeable || !writeable) ispi->nor.mtd.flags &= ~MTD_WRITEABLE; - ret = mtd_device_register(&ispi->nor.mtd, &part, 1); + ret = mtd_device_parse_register(&ispi->nor.mtd, part_probes, + NULL, &part, 1); if (ret) return ERR_PTR(ret); -- 2.29.2.729.g45daf8777d-goog ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2020-12-23 21:56 ` [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons Ian Goegebuer @ 2021-01-04 9:08 ` Miquel Raynal 2021-01-04 16:24 ` Ron Minnich 0 siblings, 1 reply; 13+ messages in thread From: Miquel Raynal @ 2021-01-04 9:08 UTC (permalink / raw) To: Ian Goegebuer Cc: Vignesh Raghavendra, Ronald G . Minnich, Tudor Ambarus, Richard Weinberger, Mika Westerberg, Boris Brezillon, Jethro Beekman, Greg Kroah-Hartman, Alexander Sverdlin, Thomas Gleixner, linux-mtd, linux-kernel Hello Ian, Ian Goegebuer <goegebuer@google.com> wrote on Wed, 23 Dec 2020 13:56:30 -0800: > On Intel platforms, the usable SPI area is located several > MiB in from the start, to leave room for descriptors and > the Management Engine binary. Further, not all the remaining > space can be used, as the last 16 MiB contains firmware. > > To make the SPI usable for mtdblock and other devices, > it is necessary to enable command line partitions so the > middle usable region can be specified. > > Add a part_probes array which includes only "cmdelineparts", > and change to mtd_device_parse_register to use this part_probes. The commit title seem to be taken from another patch and does not match the below change. Or am I missing something? > Signed-off-by: "Ronald G. Minnich" <rminnich@google.com> > Signed-off-by: Ian Goegebuer <goegebuer@google.com> > --- > drivers/mtd/spi-nor/controllers/intel-spi.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c > index b54a56a68100..9de38851c411 100644 > --- a/drivers/mtd/spi-nor/controllers/intel-spi.c > +++ b/drivers/mtd/spi-nor/controllers/intel-spi.c > @@ -903,6 +903,8 @@ static const struct spi_nor_controller_ops intel_spi_controller_ops = { > .erase = intel_spi_erase, > }; > > +static const char * const part_probes[] = { "cmdlinepart", NULL }; > + > struct intel_spi *intel_spi_probe(struct device *dev, > struct resource *mem, const struct intel_spi_boardinfo *info) > { > @@ -950,7 +952,8 @@ struct intel_spi *intel_spi_probe(struct device *dev, > if (!ispi->writeable || !writeable) > ispi->nor.mtd.flags &= ~MTD_WRITEABLE; > > - ret = mtd_device_register(&ispi->nor.mtd, &part, 1); > + ret = mtd_device_parse_register(&ispi->nor.mtd, part_probes, > + NULL, &part, 1); > if (ret) > return ERR_PTR(ret); > Thanks, Miquèl ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons 2021-01-04 9:08 ` Miquel Raynal @ 2021-01-04 16:24 ` Ron Minnich 0 siblings, 0 replies; 13+ messages in thread From: Ron Minnich @ 2021-01-04 16:24 UTC (permalink / raw) To: Miquel Raynal Cc: Ian Goegebuer, Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger, Mika Westerberg, Boris Brezillon, Jethro Beekman, Greg Kroah-Hartman, Alexander Sverdlin, Thomas Gleixner, linux-mtd, lkml - Kernel Mailing List It is likely we're missing something -- I am not fully checked out on the Linux patch process and it's Ian's first patch. Guidance appreciated. On Mon, Jan 4, 2021 at 1:08 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hello Ian, > > Ian Goegebuer <goegebuer@google.com> wrote on Wed, 23 Dec 2020 13:56:30 > -0800: > > > On Intel platforms, the usable SPI area is located several > > MiB in from the start, to leave room for descriptors and > > the Management Engine binary. Further, not all the remaining > > space can be used, as the last 16 MiB contains firmware. > > > > To make the SPI usable for mtdblock and other devices, > > it is necessary to enable command line partitions so the > > middle usable region can be specified. > > > > Add a part_probes array which includes only "cmdelineparts", > > and change to mtd_device_parse_register to use this part_probes. > > The commit title seem to be taken from another patch and does not > match the below change. Or am I missing something? > > > Signed-off-by: "Ronald G. Minnich" <rminnich@google.com> > > Signed-off-by: Ian Goegebuer <goegebuer@google.com> > > --- > > drivers/mtd/spi-nor/controllers/intel-spi.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c > > index b54a56a68100..9de38851c411 100644 > > --- a/drivers/mtd/spi-nor/controllers/intel-spi.c > > +++ b/drivers/mtd/spi-nor/controllers/intel-spi.c > > @@ -903,6 +903,8 @@ static const struct spi_nor_controller_ops intel_spi_controller_ops = { > > .erase = intel_spi_erase, > > }; > > > > +static const char * const part_probes[] = { "cmdlinepart", NULL }; > > + > > struct intel_spi *intel_spi_probe(struct device *dev, > > struct resource *mem, const struct intel_spi_boardinfo *info) > > { > > @@ -950,7 +952,8 @@ struct intel_spi *intel_spi_probe(struct device *dev, > > if (!ispi->writeable || !writeable) > > ispi->nor.mtd.flags &= ~MTD_WRITEABLE; > > > > - ret = mtd_device_register(&ispi->nor.mtd, &part, 1); > > + ret = mtd_device_parse_register(&ispi->nor.mtd, part_probes, > > + NULL, &part, 1); > > if (ret) > > return ERR_PTR(ret); > > > > Thanks, > Miquèl ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-01-04 16:25 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-29 16:53 [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons Ronald G. Minnich 2020-11-22 0:14 ` Sven Eckelmann 2020-11-27 16:32 ` ron minnich 2020-11-27 17:05 ` Sven Eckelmann 2020-11-27 17:16 ` ron minnich 2020-11-27 17:35 ` Sven Eckelmann 2020-11-27 18:54 ` ron minnich 2020-12-07 7:52 ` Sven Eckelmann 2020-12-07 15:24 ` ron minnich 2020-12-09 21:34 ` Ron Minnich 2020-09-25 4:21 [PATCH] mtd: spi-nor: controllers: intel-spi: Add support for command line partitions Vignesh Raghavendra 2020-12-23 21:56 ` [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons Ian Goegebuer 2021-01-04 9:08 ` Miquel Raynal 2021-01-04 16:24 ` Ron Minnich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).