* [PATCH] mtd: rawnand: ams-delta: Drop board specific partition info @ 2019-03-19 22:37 Janusz Krzysztofik 2019-03-20 1:16 ` Aaro Koskinen 2019-03-24 22:33 ` [PATCH v2] " Janusz Krzysztofik 0 siblings, 2 replies; 16+ messages in thread From: Janusz Krzysztofik @ 2019-03-19 22:37 UTC (permalink / raw) To: Boris Brezillon, Miquel Raynal Cc: linux-omap, Aaro Koskinen, Tony Lindgren, Richard Weinberger, Janusz Krzysztofik, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse, linux-arm-kernel After recent modifications, only a hardcoded partition info makes the driver device specific. Other than that, the driver uses GPIO exclusively and can be used on any hardware. Drop the partition info and use MTD partition parser with default list of partition types instead. Amstrad Delta users should append the followig partition info to their kernel command line, possibly by embedding it in CONFIG_CMDLINE: mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). For their convenience, select CONFIG_MTD_CMDLINE_PARTS symbol from that board Kconfig automatically if this NAND driver is also selected. Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> Cc: Tony Lindgren <tony@atomide.com> --- arch/arm/mach-omap1/Kconfig | 1 + drivers/mtd/nand/raw/ams-delta.c | 28 +--------------------------- 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/arch/arm/mach-omap1/Kconfig b/arch/arm/mach-omap1/Kconfig index c4694f26b5c4..62cf20f22828 100644 --- a/arch/arm/mach-omap1/Kconfig +++ b/arch/arm/mach-omap1/Kconfig @@ -171,6 +171,7 @@ config MACH_AMS_DELTA select LEDS_GPIO_REGISTER select REGULATOR select REGULATOR_FIXED_VOLTAGE + select MTD_CMDLINE_PARTS if MTD_NAND_AMS_DELTA help Support for the Amstrad E3 (codename Delta) videophone. Say Y here if you have such a device. diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c index 8312182088c1..2e8e37ea549a 100644 --- a/drivers/mtd/nand/raw/ams-delta.c +++ b/drivers/mtd/nand/raw/ams-delta.c @@ -41,31 +41,6 @@ struct ams_delta_nand { bool data_in; }; -/* - * Define partitions for flash devices - */ - -static const struct mtd_partition partition_info[] = { - { .name = "Kernel", - .offset = 0, - .size = 3 * SZ_1M + SZ_512K }, - { .name = "u-boot", - .offset = 3 * SZ_1M + SZ_512K, - .size = SZ_256K }, - { .name = "u-boot params", - .offset = 3 * SZ_1M + SZ_512K + SZ_256K, - .size = SZ_256K }, - { .name = "Amstrad LDR", - .offset = 4 * SZ_1M, - .size = SZ_256K }, - { .name = "File system", - .offset = 4 * SZ_1M + 1 * SZ_256K, - .size = 27 * SZ_1M }, - { .name = "PBL reserved", - .offset = 32 * SZ_1M - 3 * SZ_256K, - .size = 3 * SZ_256K }, -}; - static void ams_delta_write_commit(struct ams_delta_nand *priv) { gpiod_set_value(priv->gpiod_nwe, 0); @@ -315,8 +290,7 @@ static int ams_delta_init(struct platform_device *pdev) return err; /* Register the partitions */ - err = mtd_device_register(mtd, partition_info, - ARRAY_SIZE(partition_info)); + err = mtd_device_parse_register(mtd, NULL, NULL, NULL, 0); if (err) goto err_nand_cleanup; -- 2.19.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: rawnand: ams-delta: Drop board specific partition info 2019-03-19 22:37 [PATCH] mtd: rawnand: ams-delta: Drop board specific partition info Janusz Krzysztofik @ 2019-03-20 1:16 ` Aaro Koskinen 2019-03-24 16:48 ` Janusz Krzysztofik 2019-03-24 22:33 ` [PATCH v2] " Janusz Krzysztofik 1 sibling, 1 reply; 16+ messages in thread From: Aaro Koskinen @ 2019-03-20 1:16 UTC (permalink / raw) To: Janusz Krzysztofik Cc: linux-omap, Boris Brezillon, Tony Lindgren, Richard Weinberger, linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse, linux-arm-kernel On Tue, Mar 19, 2019 at 11:37:18PM +0100, Janusz Krzysztofik wrote: > After recent modifications, only a hardcoded partition info makes > the driver device specific. Other than that, the driver uses GPIO > exclusively and can be used on any hardware. > > Drop the partition info and use MTD partition parser with default > list of partition types instead. > > Amstrad Delta users should append the followig partition info to their ^^^^^^^^ Should be "following". > kernel command line, possibly by embedding it in CONFIG_CMDLINE: > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). For their > convenience, select CONFIG_MTD_CMDLINE_PARTS symbol from that board > Kconfig automatically if this NAND driver is also selected. > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > Cc: Tony Lindgren <tony@atomide.com> Could we move the fixed partition setup to the board file instead? Otherwise this kind of change is not really nice for the users, as it will likely break existing setups. The default partition layout should remain the same. A. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: rawnand: ams-delta: Drop board specific partition info 2019-03-20 1:16 ` Aaro Koskinen @ 2019-03-24 16:48 ` Janusz Krzysztofik 2019-03-24 18:59 ` Aaro Koskinen 0 siblings, 1 reply; 16+ messages in thread From: Janusz Krzysztofik @ 2019-03-24 16:48 UTC (permalink / raw) To: Aaro Koskinen Cc: linux-omap, Boris Brezillon, Tony Lindgren, Richard Weinberger, Janusz Krzysztofik, linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse, linux-arm-kernel Hi Aaro, Thanks for your review. On Wednesday, March 20, 2019 2:16:30 AM CET Aaro Koskinen wrote: > On Tue, Mar 19, 2019 at 11:37:18PM +0100, Janusz Krzysztofik wrote: > > After recent modifications, only a hardcoded partition info makes > > the driver device specific. Other than that, the driver uses GPIO > > exclusively and can be used on any hardware. > > > > Drop the partition info and use MTD partition parser with default > > list of partition types instead. > > > > Amstrad Delta users should append the followig partition info to their > ^^^^^^^^ > Should be "following". > > > kernel command line, possibly by embedding it in CONFIG_CMDLINE: > > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ > > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). For their > > convenience, select CONFIG_MTD_CMDLINE_PARTS symbol from that board > > Kconfig automatically if this NAND driver is also selected. > > > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > > Cc: Tony Lindgren <tony@atomide.com> > > Could we move the fixed partition setup to the board file > instead? Otherwise this kind of change is not really nice for the users, > as it will likely break existing setups. The default partition layout > should remain the same. I'm wondering if it would be acceptable to pass partition info from a .dts file. I think that would be a better, more modern approach than adding a new header under include/linux/platform_data. The problem with a device tree based implementation is, I know of no u-boot version supporting both Amstrad Delta and FDT. However, I've already tested two solutions that work for me. One uses CONFIG_ARM_APPENDED_DTB and requires a user to manually append the blob to zImage and (re)generate uImage. I'm not sure how much more user- friendly it looks for you, compared to the command line version I proposed initially. If the above is not acceptable. I can propose still another approach. The blob is automagically built and embedded into the kernel with some assembler glue, then unflattened from the board init_machine(), somehow similar to the way drivers/of/unittest.c does it. Please advise which approach sounds best to you (platform_data, CONFIG_ARM_APPENDED_DTB or unittest like). Thanks, Janusz > > A. > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: rawnand: ams-delta: Drop board specific partition info 2019-03-24 16:48 ` Janusz Krzysztofik @ 2019-03-24 18:59 ` Aaro Koskinen 2019-03-24 19:24 ` H. Nikolaus Schaller 2019-03-24 20:30 ` Janusz Krzysztofik 0 siblings, 2 replies; 16+ messages in thread From: Aaro Koskinen @ 2019-03-24 18:59 UTC (permalink / raw) To: Janusz Krzysztofik Cc: linux-omap, Boris Brezillon, Tony Lindgren, Richard Weinberger, linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse, linux-arm-kernel Hi, On Sun, Mar 24, 2019 at 05:48:22PM +0100, Janusz Krzysztofik wrote: > Hi Aaro, > > Thanks for your review. > > On Wednesday, March 20, 2019 2:16:30 AM CET Aaro Koskinen wrote: > > On Tue, Mar 19, 2019 at 11:37:18PM +0100, Janusz Krzysztofik wrote: > > > After recent modifications, only a hardcoded partition info makes > > > the driver device specific. Other than that, the driver uses GPIO > > > exclusively and can be used on any hardware. > > > > > > Drop the partition info and use MTD partition parser with default > > > list of partition types instead. > > > > > > Amstrad Delta users should append the followig partition info to their > > ^^^^^^^^ > > Should be "following". > > > > > kernel command line, possibly by embedding it in CONFIG_CMDLINE: > > > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ > > > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). For their > > > convenience, select CONFIG_MTD_CMDLINE_PARTS symbol from that board > > > Kconfig automatically if this NAND driver is also selected. > > > > > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > > > Cc: Tony Lindgren <tony@atomide.com> > > > > Could we move the fixed partition setup to the board file > > instead? Otherwise this kind of change is not really nice for the users, > > as it will likely break existing setups. The default partition layout > > should remain the same. > > I'm wondering if it would be acceptable to pass partition info from a .dts > file. I think that would be a better, more modern approach than adding a new > header under include/linux/platform_data. Hmm, I thought there was some generic way to define partitions without adding any new headers. But if that is not possible, then I guess your CMDLINE proposal is the preferred one.. A. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: rawnand: ams-delta: Drop board specific partition info 2019-03-24 18:59 ` Aaro Koskinen @ 2019-03-24 19:24 ` H. Nikolaus Schaller 2019-03-24 20:40 ` Janusz Krzysztofik 2019-03-24 20:30 ` Janusz Krzysztofik 1 sibling, 1 reply; 16+ messages in thread From: H. Nikolaus Schaller @ 2019-03-24 19:24 UTC (permalink / raw) To: Aaro Koskinen, Janusz Krzysztofik Cc: linux-omap, Boris Brezillon, Tony Lindgren, Richard Weinberger, LKML, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse, linux-arm-kernel Hi, > Am 24.03.2019 um 19:59 schrieb Aaro Koskinen <aaro.koskinen@iki.fi>: > > Hi, > > On Sun, Mar 24, 2019 at 05:48:22PM +0100, Janusz Krzysztofik wrote: >> Hi Aaro, >> >> Thanks for your review. >> >> On Wednesday, March 20, 2019 2:16:30 AM CET Aaro Koskinen wrote: >>> On Tue, Mar 19, 2019 at 11:37:18PM +0100, Janusz Krzysztofik wrote: >>>> After recent modifications, only a hardcoded partition info makes >>>> the driver device specific. Other than that, the driver uses GPIO >>>> exclusively and can be used on any hardware. >>>> >>>> Drop the partition info and use MTD partition parser with default >>>> list of partition types instead. >>>> >>>> Amstrad Delta users should append the followig partition info to their >>> ^^^^^^^^ >>> Should be "following". >>> >>>> kernel command line, possibly by embedding it in CONFIG_CMDLINE: >>>> mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ >>>> 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). For their >>>> convenience, select CONFIG_MTD_CMDLINE_PARTS symbol from that board >>>> Kconfig automatically if this NAND driver is also selected. >>>> >>>> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> >>>> Cc: Tony Lindgren <tony@atomide.com> >>> >>> Could we move the fixed partition setup to the board file >>> instead? Otherwise this kind of change is not really nice for the users, >>> as it will likely break existing setups. The default partition layout >>> should remain the same. >> >> I'm wondering if it would be acceptable to pass partition info from a .dts >> file. I think that would be a better, more modern approach than adding a new >> header under include/linux/platform_data. > > Hmm, I thought there was some generic way to define partitions without > adding any new headers. But if that is not possible, then I guess your > CMDLINE proposal is the preferred one.. I am not sure what you exactly need, but partitions can be defined in the DTS as children of some NAND drivers. Example: arch/arm/boot/dts/omap3-beagle.dts So this design pattern could be copied instead of using CMDLINE. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: rawnand: ams-delta: Drop board specific partition info 2019-03-24 19:24 ` H. Nikolaus Schaller @ 2019-03-24 20:40 ` Janusz Krzysztofik 0 siblings, 0 replies; 16+ messages in thread From: Janusz Krzysztofik @ 2019-03-24 20:40 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: linux-omap, Boris Brezillon, Tony Lindgren, Richard Weinberger, Aaro Koskinen, Janusz Krzysztofik, LKML, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse, linux-arm-kernel Hi, On Sunday, March 24, 2019 8:24:48 PM CET H. Nikolaus Schaller wrote: > Hi, > > > Am 24.03.2019 um 19:59 schrieb Aaro Koskinen <aaro.koskinen@iki.fi>: > > > > Hi, > > > > On Sun, Mar 24, 2019 at 05:48:22PM +0100, Janusz Krzysztofik wrote: > >> Hi Aaro, > >> > >> Thanks for your review. > >> > >> On Wednesday, March 20, 2019 2:16:30 AM CET Aaro Koskinen wrote: > >>> On Tue, Mar 19, 2019 at 11:37:18PM +0100, Janusz Krzysztofik wrote: > >>>> After recent modifications, only a hardcoded partition info makes > >>>> the driver device specific. Other than that, the driver uses GPIO > >>>> exclusively and can be used on any hardware. > >>>> > >>>> Drop the partition info and use MTD partition parser with default > >>>> list of partition types instead. > >>>> > >>>> Amstrad Delta users should append the followig partition info to their > >>> ^^^^^^^^ > >>> Should be "following". > >>> > >>>> kernel command line, possibly by embedding it in CONFIG_CMDLINE: > >>>> mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params), \ > >>>> 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). For their > >>>> convenience, select CONFIG_MTD_CMDLINE_PARTS symbol from that board > >>>> Kconfig automatically if this NAND driver is also selected. > >>>> > >>>> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > >>>> Cc: Tony Lindgren <tony@atomide.com> > >>> > >>> Could we move the fixed partition setup to the board file > >>> instead? Otherwise this kind of change is not really nice for the users, > >>> as it will likely break existing setups. The default partition layout > >>> should remain the same. > >> > >> I'm wondering if it would be acceptable to pass partition info from a .dts > >> file. I think that would be a better, more modern approach than adding a new > >> header under include/linux/platform_data. > > > > Hmm, I thought there was some generic way to define partitions without > > adding any new headers. But if that is not possible, then I guess your > > CMDLINE proposal is the preferred one.. > > I am not sure what you exactly need, but partitions can be defined in > the DTS as children of some NAND drivers. Example: > arch/arm/boot/dts/omap3-beagle.dts > So this design pattern could be copied instead of using CMDLINE. The problem is, OMAP1 has no device tree support. Other than that, your proposed approach already works for me locally with some basic support for device tree added to the board file. Thanks, Janusz ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: rawnand: ams-delta: Drop board specific partition info 2019-03-24 18:59 ` Aaro Koskinen 2019-03-24 19:24 ` H. Nikolaus Schaller @ 2019-03-24 20:30 ` Janusz Krzysztofik 1 sibling, 0 replies; 16+ messages in thread From: Janusz Krzysztofik @ 2019-03-24 20:30 UTC (permalink / raw) To: Aaro Koskinen Cc: linux-omap, Boris Brezillon, Tony Lindgren, Richard Weinberger, linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse, linux-arm-kernel Hi, On Sunday, March 24, 2019 7:59:32 PM CET Aaro Koskinen wrote: > Hi, > > On Sun, Mar 24, 2019 at 05:48:22PM +0100, Janusz Krzysztofik wrote: > > Hi Aaro, > > > > Thanks for your review. > > > > On Wednesday, March 20, 2019 2:16:30 AM CET Aaro Koskinen wrote: > > > On Tue, Mar 19, 2019 at 11:37:18PM +0100, Janusz Krzysztofik wrote: > > > > After recent modifications, only a hardcoded partition info makes > > > > the driver device specific. Other than that, the driver uses GPIO > > > > exclusively and can be used on any hardware. > > > > > > > > Drop the partition info and use MTD partition parser with default > > > > list of partition types instead. > > > > > > > > Amstrad Delta users should append the followig partition info to their > > > ^^^^^^^^ > > > Should be "following". > > > > > > > kernel command line, possibly by embedding it in CONFIG_CMDLINE: > > > > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u- boot_params),\ > > > > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). For their > > > > convenience, select CONFIG_MTD_CMDLINE_PARTS symbol from that board > > > > Kconfig automatically if this NAND driver is also selected. > > > > > > > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > > > > Cc: Tony Lindgren <tony@atomide.com> > > > > > > Could we move the fixed partition setup to the board file > > > instead? Otherwise this kind of change is not really nice for the users, > > > as it will likely break existing setups. The default partition layout > > > should remain the same. > > > > I'm wondering if it would be acceptable to pass partition info from a .dts > > file. I think that would be a better, more modern approach than adding a new > > header under include/linux/platform_data. > > Hmm, I thought there was some generic way to define partitions without > adding any new headers. But if that is not possible, then I guess your > CMDLINE proposal is the preferred one.. I could for example reuse (or abuse) struct gpio_nand_platdata defined in include/linux/mtd/nand-gpio.h for use with drivers/mtd/nand/raw/gpio.c, but I'm not sure if hi-jacking a header that belongs to another driver would be an elegant solution. Thanks, Janusz > > A. > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] mtd: rawnand: ams-delta: Drop board specific partition info 2019-03-19 22:37 [PATCH] mtd: rawnand: ams-delta: Drop board specific partition info Janusz Krzysztofik 2019-03-20 1:16 ` Aaro Koskinen @ 2019-03-24 22:33 ` Janusz Krzysztofik 2019-04-17 9:40 ` Miquel Raynal 2019-04-24 18:02 ` [PATCH v3] " Janusz Krzysztofik 1 sibling, 2 replies; 16+ messages in thread From: Janusz Krzysztofik @ 2019-03-24 22:33 UTC (permalink / raw) To: Boris Brezillon, Miquel Raynal Cc: linux-omap, Aaro Koskinen, Tony Lindgren, Richard Weinberger, Janusz Krzysztofik, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse, linux-arm-kernel After recent modifications, only a hardcoded partition info makes the driver device specific. Other than that, the driver uses GPIO exclusively and can be used on any hardware. Drop the partition info and use MTD partition parser with default list of parser names instead. For the OF parser to work correctly, pass device of_node to mtd. Amstrad Delta users should append the following partition info to their kernel command line, possibly by embedding it in CONFIG_CMDLINE: mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). For their convenience, CONFIG_MTD_CMDLINE_PARTS symbol is selected automatically from that board Kconfig if this NAND driver is also selected. Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> Cc: Tony Lindgren <tony@atomide.com> --- CHangelog: v1->v2: - fix a typo poined out by Aaro - thanks! - fix device_node not passed to OF parser via mtd_info - commit message reworded and reformatted a bit for better readability arch/arm/mach-omap1/Kconfig | 1 + drivers/mtd/nand/raw/ams-delta.c | 29 ++--------------------------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/arch/arm/mach-omap1/Kconfig b/arch/arm/mach-omap1/Kconfig index c4694f26b5c4..62cf20f22828 100644 --- a/arch/arm/mach-omap1/Kconfig +++ b/arch/arm/mach-omap1/Kconfig @@ -171,6 +171,7 @@ config MACH_AMS_DELTA select LEDS_GPIO_REGISTER select REGULATOR select REGULATOR_FIXED_VOLTAGE + select MTD_CMDLINE_PARTS if MTD_NAND_AMS_DELTA help Support for the Amstrad E3 (codename Delta) videophone. Say Y here if you have such a device. diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c index 8312182088c1..e0f09179bbda 100644 --- a/drivers/mtd/nand/raw/ams-delta.c +++ b/drivers/mtd/nand/raw/ams-delta.c @@ -41,31 +41,6 @@ struct ams_delta_nand { bool data_in; }; -/* - * Define partitions for flash devices - */ - -static const struct mtd_partition partition_info[] = { - { .name = "Kernel", - .offset = 0, - .size = 3 * SZ_1M + SZ_512K }, - { .name = "u-boot", - .offset = 3 * SZ_1M + SZ_512K, - .size = SZ_256K }, - { .name = "u-boot params", - .offset = 3 * SZ_1M + SZ_512K + SZ_256K, - .size = SZ_256K }, - { .name = "Amstrad LDR", - .offset = 4 * SZ_1M, - .size = SZ_256K }, - { .name = "File system", - .offset = 4 * SZ_1M + 1 * SZ_256K, - .size = 27 * SZ_1M }, - { .name = "PBL reserved", - .offset = 32 * SZ_1M - 3 * SZ_256K, - .size = 3 * SZ_256K }, -}; - static void ams_delta_write_commit(struct ams_delta_nand *priv) { gpiod_set_value(priv->gpiod_nwe, 0); @@ -238,6 +213,7 @@ static int ams_delta_init(struct platform_device *pdev) mtd->dev.parent = &pdev->dev; nand_set_controller_data(this, priv); + nand_set_flash_node(this, pdev->dev.of_node); priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN); if (IS_ERR(priv->gpiod_rdy)) { @@ -315,8 +291,7 @@ static int ams_delta_init(struct platform_device *pdev) return err; /* Register the partitions */ - err = mtd_device_register(mtd, partition_info, - ARRAY_SIZE(partition_info)); + err = mtd_device_parse_register(mtd, NULL, NULL, NULL, 0); if (err) goto err_nand_cleanup; -- 2.19.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mtd: rawnand: ams-delta: Drop board specific partition info 2019-03-24 22:33 ` [PATCH v2] " Janusz Krzysztofik @ 2019-04-17 9:40 ` Miquel Raynal 2019-04-17 23:09 ` Janusz Krzysztofik 2019-04-24 18:02 ` [PATCH v3] " Janusz Krzysztofik 1 sibling, 1 reply; 16+ messages in thread From: Miquel Raynal @ 2019-04-17 9:40 UTC (permalink / raw) To: Janusz Krzysztofik Cc: linux-omap, Aaro Koskinen, Tony Lindgren, Richard Weinberger, Boris Brezillon, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse, linux-arm-kernel Hi Janusz, Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote on Sun, 24 Mar 2019 23:33:44 +0100: > After recent modifications, only a hardcoded partition info makes > the driver device specific. Other than that, the driver uses GPIO > exclusively and can be used on any hardware. > > Drop the partition info and use MTD partition parser with default list > of parser names instead. For the OF parser to work correctly, pass > device of_node to mtd. > > Amstrad Delta users should append the following partition info to their > kernel command line, possibly by embedding it in CONFIG_CMDLINE: > > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). > > For their convenience, CONFIG_MTD_CMDLINE_PARTS symbol is selected > automatically from that board Kconfig if this NAND driver is also > selected. > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > Cc: Tony Lindgren <tony@atomide.com> > --- FYI I am okay with the change but I am waiting for acks before applying it. Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mtd: rawnand: ams-delta: Drop board specific partition info 2019-04-17 9:40 ` Miquel Raynal @ 2019-04-17 23:09 ` Janusz Krzysztofik 2019-04-18 6:49 ` Miquel Raynal 0 siblings, 1 reply; 16+ messages in thread From: Janusz Krzysztofik @ 2019-04-17 23:09 UTC (permalink / raw) To: Aaro Koskinen, Tony Lindgren Cc: linux-omap, Boris Brezillon, Richard Weinberger, Janusz Krzysztofik, linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse, linux-arm-kernel Hi Aaro, Tony, On Wednesday, April 17, 2019 11:40:10 AM CEST Miquel Raynal wrote: > Hi Janusz, > > Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote on Sun, 24 Mar 2019 > 23:33:44 +0100: > > > After recent modifications, only a hardcoded partition info makes > > the driver device specific. Other than that, the driver uses GPIO > > exclusively and can be used on any hardware. > > > > Drop the partition info and use MTD partition parser with default list > > of parser names instead. For the OF parser to work correctly, pass > > device of_node to mtd. > > > > Amstrad Delta users should append the following partition info to their > > kernel command line, possibly by embedding it in CONFIG_CMDLINE: > > > > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ > > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). > > > > For their convenience, CONFIG_MTD_CMDLINE_PARTS symbol is selected > > automatically from that board Kconfig if this NAND driver is also > > selected. > > > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > > Cc: Tony Lindgren <tony@atomide.com> > > --- > > FYI I am okay with the change but I am waiting for acks before applying > it. May we have an ack from you? If still not convinced with my clarifications, I can add a comment to help text in Kconfig, either squashed or in a follow up patch, on the requirement of appending mtdparts parameter to command line. What do you think? Thanks, Janusz > > Thanks, > Miquèl > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mtd: rawnand: ams-delta: Drop board specific partition info 2019-04-17 23:09 ` Janusz Krzysztofik @ 2019-04-18 6:49 ` Miquel Raynal 2019-04-18 19:11 ` Janusz Krzysztofik 0 siblings, 1 reply; 16+ messages in thread From: Miquel Raynal @ 2019-04-18 6:49 UTC (permalink / raw) To: Janusz Krzysztofik Cc: linux-omap, Boris Brezillon, Tony Lindgren, Richard Weinberger, Aaro Koskinen, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse, linux-arm-kernel Hi Janusz, Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote on Thu, 18 Apr 2019 01:09:59 +0200: > Hi Aaro, Tony, > > On Wednesday, April 17, 2019 11:40:10 AM CEST Miquel Raynal wrote: > > Hi Janusz, > > > > Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote on Sun, 24 Mar 2019 > > 23:33:44 +0100: > > > > > After recent modifications, only a hardcoded partition info makes > > > the driver device specific. Other than that, the driver uses GPIO > > > exclusively and can be used on any hardware. > > > > > > Drop the partition info and use MTD partition parser with default list > > > of parser names instead. For the OF parser to work correctly, pass > > > device of_node to mtd. > > > > > > Amstrad Delta users should append the following partition info to their > > > kernel command line, possibly by embedding it in CONFIG_CMDLINE: > > > > > > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ > > > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). > > > > > > For their convenience, CONFIG_MTD_CMDLINE_PARTS symbol is selected > > > automatically from that board Kconfig if this NAND driver is also > > > selected. > > > > > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > > > Cc: Tony Lindgren <tony@atomide.com> > > > --- > > > > FYI I am okay with the change but I am waiting for acks before applying > > it. > > May we have an ack from you? > > If still not convinced with my clarifications, I can add a comment to help text > in Kconfig, either squashed or in a follow up patch, on the requirement of > appending mtdparts parameter to command line. What do you think? In the same patch I guess that would be fine. Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] mtd: rawnand: ams-delta: Drop board specific partition info 2019-04-18 6:49 ` Miquel Raynal @ 2019-04-18 19:11 ` Janusz Krzysztofik 0 siblings, 0 replies; 16+ messages in thread From: Janusz Krzysztofik @ 2019-04-18 19:11 UTC (permalink / raw) To: Miquel Raynal Cc: linux-omap, Boris Brezillon, Tony Lindgren, Richard Weinberger, Aaro Koskinen, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse, linux-arm-kernel Hi Miquèl, I'm wondering if we could register up a custom mtd partition parser from the board file. It could return a pointer to a statically defined table. Then we could add its name to the list of parsers the driver is going to try if the board is enabled in .config. Would that be acceptable from the MTD subsystem point of view? Thanks, Janusz On Thursday, April 18, 2019 8:49:29 AM CEST Miquel Raynal wrote: > Hi Janusz, > > Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote on Thu, 18 Apr 2019 > 01:09:59 +0200: > > > Hi Aaro, Tony, > > > > On Wednesday, April 17, 2019 11:40:10 AM CEST Miquel Raynal wrote: > > > Hi Janusz, > > > > > > Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote on Sun, 24 Mar 2019 > > > 23:33:44 +0100: > > > > > > > After recent modifications, only a hardcoded partition info makes > > > > the driver device specific. Other than that, the driver uses GPIO > > > > exclusively and can be used on any hardware. > > > > > > > > Drop the partition info and use MTD partition parser with default list > > > > of parser names instead. For the OF parser to work correctly, pass > > > > device of_node to mtd. > > > > > > > > Amstrad Delta users should append the following partition info to their > > > > kernel command line, possibly by embedding it in CONFIG_CMDLINE: > > > > > > > > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u- boot_params),\ > > > > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved). > > > > > > > > For their convenience, CONFIG_MTD_CMDLINE_PARTS symbol is selected > > > > automatically from that board Kconfig if this NAND driver is also > > > > selected. > > > > > > > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > > > > Cc: Tony Lindgren <tony@atomide.com> > > > > --- > > > > > > FYI I am okay with the change but I am waiting for acks before applying > > > it. > > > > May we have an ack from you? > > > > If still not convinced with my clarifications, I can add a comment to help text > > in Kconfig, either squashed or in a follow up patch, on the requirement of > > appending mtdparts parameter to command line. What do you think? > > In the same patch I guess that would be fine. > > Thanks, > Miquèl > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] mtd: rawnand: ams-delta: Drop board specific partition info 2019-03-24 22:33 ` [PATCH v2] " Janusz Krzysztofik 2019-04-17 9:40 ` Miquel Raynal @ 2019-04-24 18:02 ` Janusz Krzysztofik 2019-04-24 22:14 ` Ladislav Michl 1 sibling, 1 reply; 16+ messages in thread From: Janusz Krzysztofik @ 2019-04-24 18:02 UTC (permalink / raw) To: Miquel Raynal, Boris Brezillon Cc: linux-omap, Aaro Koskinen, Tony Lindgren, Richard Weinberger, Janusz Krzysztofik, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse, linux-arm-kernel After recent modifications, only a hardcoded partition info makes the driver device specific. Other than that, the driver uses GPIO exclusively and can be used on any hardware. Drop the partition info and use MTD partition parser with default list of parser names instead. For the OF parser to work correctly, pass device of_node to mtd. Amstrad Delta users should append the following partition info to their kernel command line, possibly embedding it in CONFIG_CMDLINE: mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved) For their convenience, that information is added to the board Kconfig entry help text, as well as CONFIG_MTD_CMDLINE_PARTS symbol is selected automatically from the board Kconfig entry if the NAND driver is also selected. Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> Cc: Tony Lindgren <tony@atomide.com> Cc: Aaro Koskinen <aaro.koskinen@iki.fi> --- Changelog: v2->v3: - add information on the requirement for passing partition info via kernel command line to the board Kconfig entry help text v1->v2: - fix a typo poitned out by Aaro - thanks! - fix device_node not passed to OF parser via mtd_info - commit message reworded and reformatted a bit for better readability arch/arm/mach-omap1/Kconfig | 7 ++++++- drivers/mtd/nand/raw/ams-delta.c | 29 ++--------------------------- 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/arch/arm/mach-omap1/Kconfig b/arch/arm/mach-omap1/Kconfig index c4694f26b5c4..41a47d251cac 100644 --- a/arch/arm/mach-omap1/Kconfig +++ b/arch/arm/mach-omap1/Kconfig @@ -164,17 +164,22 @@ config MACH_NOKIA770 have such a device. config MACH_AMS_DELTA - bool "Amstrad E3 (Delta)" + bool "Amstrad E3 (Delta) - see help for important information" depends on ARCH_OMAP1 && ARCH_OMAP15XX select FIQ select GPIO_GENERIC_PLATFORM select LEDS_GPIO_REGISTER select REGULATOR select REGULATOR_FIXED_VOLTAGE + select MTD_CMDLINE_PARTS if MTD_NAND_AMS_DELTA help Support for the Amstrad E3 (codename Delta) videophone. Say Y here if you have such a device. + If you are using built-in NAND, append the following partition + info to kernel command line: + mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved) + config MACH_OMAP_GENERIC bool "Generic OMAP board" depends on ARCH_OMAP1 && (ARCH_OMAP15XX || ARCH_OMAP16XX) diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c index 8312182088c1..e0f09179bbda 100644 --- a/drivers/mtd/nand/raw/ams-delta.c +++ b/drivers/mtd/nand/raw/ams-delta.c @@ -41,31 +41,6 @@ struct ams_delta_nand { bool data_in; }; -/* - * Define partitions for flash devices - */ - -static const struct mtd_partition partition_info[] = { - { .name = "Kernel", - .offset = 0, - .size = 3 * SZ_1M + SZ_512K }, - { .name = "u-boot", - .offset = 3 * SZ_1M + SZ_512K, - .size = SZ_256K }, - { .name = "u-boot params", - .offset = 3 * SZ_1M + SZ_512K + SZ_256K, - .size = SZ_256K }, - { .name = "Amstrad LDR", - .offset = 4 * SZ_1M, - .size = SZ_256K }, - { .name = "File system", - .offset = 4 * SZ_1M + 1 * SZ_256K, - .size = 27 * SZ_1M }, - { .name = "PBL reserved", - .offset = 32 * SZ_1M - 3 * SZ_256K, - .size = 3 * SZ_256K }, -}; - static void ams_delta_write_commit(struct ams_delta_nand *priv) { gpiod_set_value(priv->gpiod_nwe, 0); @@ -238,6 +213,7 @@ static int ams_delta_init(struct platform_device *pdev) mtd->dev.parent = &pdev->dev; nand_set_controller_data(this, priv); + nand_set_flash_node(this, pdev->dev.of_node); priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN); if (IS_ERR(priv->gpiod_rdy)) { @@ -315,8 +291,7 @@ static int ams_delta_init(struct platform_device *pdev) return err; /* Register the partitions */ - err = mtd_device_register(mtd, partition_info, - ARRAY_SIZE(partition_info)); + err = mtd_device_parse_register(mtd, NULL, NULL, NULL, 0); if (err) goto err_nand_cleanup; -- 2.21.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3] mtd: rawnand: ams-delta: Drop board specific partition info 2019-04-24 18:02 ` [PATCH v3] " Janusz Krzysztofik @ 2019-04-24 22:14 ` Ladislav Michl 2019-04-25 18:42 ` Janusz Krzysztofik 0 siblings, 1 reply; 16+ messages in thread From: Ladislav Michl @ 2019-04-24 22:14 UTC (permalink / raw) To: Janusz Krzysztofik Cc: linux-omap, Aaro Koskinen, Tony Lindgren, Richard Weinberger, Boris Brezillon, linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse, linux-arm-kernel Hi Janusz, On Wed, Apr 24, 2019 at 08:02:12PM +0200, Janusz Krzysztofik wrote: > After recent modifications, only a hardcoded partition info makes > the driver device specific. Other than that, the driver uses GPIO > exclusively and can be used on any hardware. > > Drop the partition info and use MTD partition parser with default list > of parser names instead. For the OF parser to work correctly, pass > device of_node to mtd. > > Amstrad Delta users should append the following partition info to their > kernel command line, possibly embedding it in CONFIG_CMDLINE: > > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved) now, when driver is no longer Amstrad Delta specific, why would you want to have ams-delta-nand hardcoded on kernel cmdline? I'm assuming at some point this driver will become gpio-nand [*] or something like that and asking users to change their kernel cmdline twice is just unwise :) [*] btw, it is really shame gpio-nand name is already taken by driver living in drivers/mtd/nand/raw/gpio.c which is more likely gpio-mem-nand used by at least CompuLab CM-X255 and Picochip picoXcell. Otherwise your work on this driver is so amazing that I just spent couple of hours finding that phone and compiling some decent userspace for it :) Thank you! > For their convenience, that information is added to the board Kconfig > entry help text, as well as CONFIG_MTD_CMDLINE_PARTS symbol is selected > automatically from the board Kconfig entry if the NAND driver is also > selected. > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > Cc: Tony Lindgren <tony@atomide.com> > Cc: Aaro Koskinen <aaro.koskinen@iki.fi> > --- > Changelog: > v2->v3: > - add information on the requirement for passing partition info via > kernel command line to the board Kconfig entry help text > v1->v2: > - fix a typo poitned out by Aaro - thanks! > - fix device_node not passed to OF parser via mtd_info > - commit message reworded and reformatted a bit for better readability > > arch/arm/mach-omap1/Kconfig | 7 ++++++- > drivers/mtd/nand/raw/ams-delta.c | 29 ++--------------------------- > 2 files changed, 8 insertions(+), 28 deletions(-) > > diff --git a/arch/arm/mach-omap1/Kconfig b/arch/arm/mach-omap1/Kconfig > index c4694f26b5c4..41a47d251cac 100644 > --- a/arch/arm/mach-omap1/Kconfig > +++ b/arch/arm/mach-omap1/Kconfig > @@ -164,17 +164,22 @@ config MACH_NOKIA770 > have such a device. > > config MACH_AMS_DELTA > - bool "Amstrad E3 (Delta)" > + bool "Amstrad E3 (Delta) - see help for important information" > depends on ARCH_OMAP1 && ARCH_OMAP15XX > select FIQ > select GPIO_GENERIC_PLATFORM > select LEDS_GPIO_REGISTER > select REGULATOR > select REGULATOR_FIXED_VOLTAGE > + select MTD_CMDLINE_PARTS if MTD_NAND_AMS_DELTA > help > Support for the Amstrad E3 (codename Delta) videophone. Say Y here > if you have such a device. > > + If you are using built-in NAND, append the following partition > + info to kernel command line: > + mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved) > + > config MACH_OMAP_GENERIC > bool "Generic OMAP board" > depends on ARCH_OMAP1 && (ARCH_OMAP15XX || ARCH_OMAP16XX) > diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c > index 8312182088c1..e0f09179bbda 100644 > --- a/drivers/mtd/nand/raw/ams-delta.c > +++ b/drivers/mtd/nand/raw/ams-delta.c > @@ -41,31 +41,6 @@ struct ams_delta_nand { > bool data_in; > }; > > -/* > - * Define partitions for flash devices > - */ > - > -static const struct mtd_partition partition_info[] = { > - { .name = "Kernel", > - .offset = 0, > - .size = 3 * SZ_1M + SZ_512K }, > - { .name = "u-boot", > - .offset = 3 * SZ_1M + SZ_512K, > - .size = SZ_256K }, > - { .name = "u-boot params", > - .offset = 3 * SZ_1M + SZ_512K + SZ_256K, > - .size = SZ_256K }, > - { .name = "Amstrad LDR", > - .offset = 4 * SZ_1M, > - .size = SZ_256K }, > - { .name = "File system", > - .offset = 4 * SZ_1M + 1 * SZ_256K, > - .size = 27 * SZ_1M }, > - { .name = "PBL reserved", > - .offset = 32 * SZ_1M - 3 * SZ_256K, > - .size = 3 * SZ_256K }, > -}; > - > static void ams_delta_write_commit(struct ams_delta_nand *priv) > { > gpiod_set_value(priv->gpiod_nwe, 0); > @@ -238,6 +213,7 @@ static int ams_delta_init(struct platform_device *pdev) > mtd->dev.parent = &pdev->dev; > > nand_set_controller_data(this, priv); > + nand_set_flash_node(this, pdev->dev.of_node); > > priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN); > if (IS_ERR(priv->gpiod_rdy)) { > @@ -315,8 +291,7 @@ static int ams_delta_init(struct platform_device *pdev) > return err; > > /* Register the partitions */ > - err = mtd_device_register(mtd, partition_info, > - ARRAY_SIZE(partition_info)); > + err = mtd_device_parse_register(mtd, NULL, NULL, NULL, 0); > if (err) > goto err_nand_cleanup; > > -- > 2.21.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] mtd: rawnand: ams-delta: Drop board specific partition info 2019-04-24 22:14 ` Ladislav Michl @ 2019-04-25 18:42 ` Janusz Krzysztofik 2019-04-27 9:18 ` Ladislav Michl 0 siblings, 1 reply; 16+ messages in thread From: Janusz Krzysztofik @ 2019-04-25 18:42 UTC (permalink / raw) To: Ladislav Michl Cc: linux-omap, Aaro Koskinen, Tony Lindgren, Richard Weinberger, Boris Brezillon, Janusz Krzysztofik, linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse, linux-arm-kernel Hi Ladislav, On Thursday, April 25, 2019 12:14:28 AM CEST Ladislav Michl wrote: > Hi Janusz, > > On Wed, Apr 24, 2019 at 08:02:12PM +0200, Janusz Krzysztofik wrote: > > After recent modifications, only a hardcoded partition info makes > > the driver device specific. Other than that, the driver uses GPIO > > exclusively and can be used on any hardware. > > > > Drop the partition info and use MTD partition parser with default list > > of parser names instead. For the OF parser to work correctly, pass > > device of_node to mtd. > > > > Amstrad Delta users should append the following partition info to their > > kernel command line, possibly embedding it in CONFIG_CMDLINE: > > > > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ > > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved) > > now, when driver is no longer Amstrad Delta specific, why would you want > to have ams-delta-nand hardcoded on kernel cmdline? I'm assuming at some > point this driver will become gpio-nand [*] or something like that and > asking users to change their kernel cmdline twice is just unwise :) Hmm, I have no idea of a good name for the driver if not "gpio-nand". Can you suggest one? As a workaround, I can add a platform device id table to the driver with "ams- delta-nand" as a supported device name in hope that survives possible future driver renaming. > [*] btw, it is really shame gpio-nand name is already taken by driver > living in drivers/mtd/nand/raw/gpio.c which is more likely gpio-mem-nand > used by at least CompuLab CM-X255 and Picochip picoXcell. I think the best approach would be to expose NAND data ports of those machines as GPIO ports, possibly reusing the "gpio-nand" driver code while creating a new GPIO driver for them if "basic-mmio-gpio" occurs inappropriate, and use the pure GPIO NAND driver on top. > Otherwise your work on this driver is so amazing that I just spent > couple of hours finding that phone and compiling some decent userspace > for it :) Thank you! I'm glad you like it :-) Janusz > > > For their convenience, that information is added to the board Kconfig > > entry help text, as well as CONFIG_MTD_CMDLINE_PARTS symbol is selected > > automatically from the board Kconfig entry if the NAND driver is also > > selected. > > > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > > Cc: Tony Lindgren <tony@atomide.com> > > Cc: Aaro Koskinen <aaro.koskinen@iki.fi> > > --- > > Changelog: > > v2->v3: > > - add information on the requirement for passing partition info via > > kernel command line to the board Kconfig entry help text > > v1->v2: > > - fix a typo poitned out by Aaro - thanks! > > - fix device_node not passed to OF parser via mtd_info > > - commit message reworded and reformatted a bit for better readability > > > > arch/arm/mach-omap1/Kconfig | 7 ++++++- > > drivers/mtd/nand/raw/ams-delta.c | 29 ++--------------------------- > > 2 files changed, 8 insertions(+), 28 deletions(-) > > > > diff --git a/arch/arm/mach-omap1/Kconfig b/arch/arm/mach-omap1/Kconfig > > index c4694f26b5c4..41a47d251cac 100644 > > --- a/arch/arm/mach-omap1/Kconfig > > +++ b/arch/arm/mach-omap1/Kconfig > > @@ -164,17 +164,22 @@ config MACH_NOKIA770 > > have such a device. > > > > config MACH_AMS_DELTA > > - bool "Amstrad E3 (Delta)" > > + bool "Amstrad E3 (Delta) - see help for important information" > > depends on ARCH_OMAP1 && ARCH_OMAP15XX > > select FIQ > > select GPIO_GENERIC_PLATFORM > > select LEDS_GPIO_REGISTER > > select REGULATOR > > select REGULATOR_FIXED_VOLTAGE > > + select MTD_CMDLINE_PARTS if MTD_NAND_AMS_DELTA > > help > > Support for the Amstrad E3 (codename Delta) videophone. Say Y here > > if you have such a device. > > > > + If you are using built-in NAND, append the following partition > > + info to kernel command line: > > + mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u- boot_params),256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved) > > + > > config MACH_OMAP_GENERIC > > bool "Generic OMAP board" > > depends on ARCH_OMAP1 && (ARCH_OMAP15XX || ARCH_OMAP16XX) > > diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams- delta.c > > index 8312182088c1..e0f09179bbda 100644 > > --- a/drivers/mtd/nand/raw/ams-delta.c > > +++ b/drivers/mtd/nand/raw/ams-delta.c > > @@ -41,31 +41,6 @@ struct ams_delta_nand { > > bool data_in; > > }; > > > > -/* > > - * Define partitions for flash devices > > - */ > > - > > -static const struct mtd_partition partition_info[] = { > > - { .name = "Kernel", > > - .offset = 0, > > - .size = 3 * SZ_1M + SZ_512K }, > > - { .name = "u-boot", > > - .offset = 3 * SZ_1M + SZ_512K, > > - .size = SZ_256K }, > > - { .name = "u-boot params", > > - .offset = 3 * SZ_1M + SZ_512K + SZ_256K, > > - .size = SZ_256K }, > > - { .name = "Amstrad LDR", > > - .offset = 4 * SZ_1M, > > - .size = SZ_256K }, > > - { .name = "File system", > > - .offset = 4 * SZ_1M + 1 * SZ_256K, > > - .size = 27 * SZ_1M }, > > - { .name = "PBL reserved", > > - .offset = 32 * SZ_1M - 3 * SZ_256K, > > - .size = 3 * SZ_256K }, > > -}; > > - > > static void ams_delta_write_commit(struct ams_delta_nand *priv) > > { > > gpiod_set_value(priv->gpiod_nwe, 0); > > @@ -238,6 +213,7 @@ static int ams_delta_init(struct platform_device *pdev) > > mtd->dev.parent = &pdev->dev; > > > > nand_set_controller_data(this, priv); > > + nand_set_flash_node(this, pdev->dev.of_node); > > > > priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN); > > if (IS_ERR(priv->gpiod_rdy)) { > > @@ -315,8 +291,7 @@ static int ams_delta_init(struct platform_device *pdev) > > return err; > > > > /* Register the partitions */ > > - err = mtd_device_register(mtd, partition_info, > > - ARRAY_SIZE(partition_info)); > > + err = mtd_device_parse_register(mtd, NULL, NULL, NULL, 0); > > if (err) > > goto err_nand_cleanup; > > > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] mtd: rawnand: ams-delta: Drop board specific partition info 2019-04-25 18:42 ` Janusz Krzysztofik @ 2019-04-27 9:18 ` Ladislav Michl 0 siblings, 0 replies; 16+ messages in thread From: Ladislav Michl @ 2019-04-27 9:18 UTC (permalink / raw) To: Janusz Krzysztofik Cc: linux-omap, Aaro Koskinen, Tony Lindgren, Richard Weinberger, Boris Brezillon, linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse, linux-arm-kernel On Thu, Apr 25, 2019 at 08:42:22PM +0200, Janusz Krzysztofik wrote: > Hi Ladislav, > > On Thursday, April 25, 2019 12:14:28 AM CEST Ladislav Michl wrote: > > Hi Janusz, > > > > On Wed, Apr 24, 2019 at 08:02:12PM +0200, Janusz Krzysztofik wrote: > > > After recent modifications, only a hardcoded partition info makes > > > the driver device specific. Other than that, the driver uses GPIO > > > exclusively and can be used on any hardware. > > > > > > Drop the partition info and use MTD partition parser with default list > > > of parser names instead. For the OF parser to work correctly, pass > > > device of_node to mtd. > > > > > > Amstrad Delta users should append the following partition info to their > > > kernel command line, possibly embedding it in CONFIG_CMDLINE: > > > > > > mtdparts=ams-delta-nand:3584k(Kernel),256k(u-boot),256k(u-boot_params),\ > > > 256k(Amstrad_LDR),27m(File_system),768k(PBL_reserved) > > > > now, when driver is no longer Amstrad Delta specific, why would you want > > to have ams-delta-nand hardcoded on kernel cmdline? I'm assuming at some > > point this driver will become gpio-nand [*] or something like that and > > asking users to change their kernel cmdline twice is just unwise :) > > Hmm, I have no idea of a good name for the driver if not "gpio-nand". Can you > suggest one? gpio-nand is so good name that it should be worth merging gpio.c and ams-delta.c into gpio_nand.c :) > As a workaround, I can add a platform device id table to the driver with "ams- > delta-nand" as a supported device name in hope that survives possible future > driver renaming. > > > [*] btw, it is really shame gpio-nand name is already taken by driver > > living in drivers/mtd/nand/raw/gpio.c which is more likely gpio-mem-nand > > used by at least CompuLab CM-X255 and Picochip picoXcell. > > I think the best approach would be to expose NAND data ports of those machines > as GPIO ports, possibly reusing the "gpio-nand" driver code while creating a > new GPIO driver for them if "basic-mmio-gpio" occurs inappropriate, and use > the pure GPIO NAND driver on top. What about adding two fields into struct ams_delta_nand holding pointers to either ams_delta_{read,write}_buf (renamed to gpio_nand_...) or mmio r/w functions depending on driver configuration? > > Otherwise your work on this driver is so amazing that I just spent > > couple of hours finding that phone and compiling some decent userspace > > for it :) Thank you! > > I'm glad you like it :-) > Janusz Best regards, ladis ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-04-27 9:18 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-19 22:37 [PATCH] mtd: rawnand: ams-delta: Drop board specific partition info Janusz Krzysztofik 2019-03-20 1:16 ` Aaro Koskinen 2019-03-24 16:48 ` Janusz Krzysztofik 2019-03-24 18:59 ` Aaro Koskinen 2019-03-24 19:24 ` H. Nikolaus Schaller 2019-03-24 20:40 ` Janusz Krzysztofik 2019-03-24 20:30 ` Janusz Krzysztofik 2019-03-24 22:33 ` [PATCH v2] " Janusz Krzysztofik 2019-04-17 9:40 ` Miquel Raynal 2019-04-17 23:09 ` Janusz Krzysztofik 2019-04-18 6:49 ` Miquel Raynal 2019-04-18 19:11 ` Janusz Krzysztofik 2019-04-24 18:02 ` [PATCH v3] " Janusz Krzysztofik 2019-04-24 22:14 ` Ladislav Michl 2019-04-25 18:42 ` Janusz Krzysztofik 2019-04-27 9:18 ` Ladislav Michl
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).