lkml.org 
[lkml]   [2019]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] mtd: rawnand: ams-delta: Drop board specific partition info
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

\
 
 \ /
  Last update: 2019-04-25 00:15    [W:0.095 / U:1.512 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site