| Message ID | 20190324223344.24590-1-jmkrzyszt@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] mtd: rawnand: ams-delta: Drop board specific partition info | expand |
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
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 >
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
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 >
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;
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(-)