Re: [PATCH] mtd/powernv_flash: Enable partition support

[Timothy Pearson <tpearson@raptorengineering.com>]

Will resend with Signed-off-by.

Before this patch, the driver was not picking up the OF-provided
partition list.  It seemed specifically designed to register only one
large partition covering the entire PNOR; mtd_device_register() does not
run parsing of any type AFAIK.

Example DT:

flash@0 {
	compatible = "ibm,opal-flash";
	ibm,flash-block-size = <0x10000>;
	ibm,opal-id = <0x0>;
	no-erase;
	#address-cells = <0x1>;
	#size-cells = <0x1>;
	phandle = <0x161>;
	reg = <0x0 0x4000000>;

	partitions {
		compatible = "fixed-partitions";
		#address-cells = <0x1>;
		#size-cells = <0x1>;
		phandle = <0x162>;

		partition@0 {
			label = "PNOR";
			phandle = <0x168>;
			reg = <0x0 0x4000000>;
		};

		partition@3389000 {
			read-only;
			label = "IMA_CATALOG";
			phandle = <0x165>;
			reg = <0x3389000 0x40000>;
		};

		partition@1a21000 {
			read-only;
			label = "BOOTKERNEL";
			phandle = <0x163>;
			reg = <0x1a21000 0x1800000>;
		};

		partition@3388000 {
			read-only;
			label = "VERSION";
			phandle = <0x166>;
			reg = <0x3388000 0x1000>;
		};

		partition@3710000 {
			label = "BOOTKERNFW";
			phandle = <0x167>;
			reg = <0x3710000 0x600000>;
		};

		partition@3344000 {
			read-only;
			label = "CAPP";
			phandle = <0x164>;
			reg = <0x3344000 0x24000>;
		};
	};
};

On 03/29/2018 12:29 AM, Rafał Miłecki wrote:
> Hi Tomthy,
> 
> On 25 March 2018 at 22:05, Timothy Pearson
> <tpearson@raptorengineering.com> wrote:
>> On certain systems, such as the Talos II, skiboot emits a partition
>> table for the main PNOR MTD device in the generated device tree.
>>
>> Allow this partition table to be parsed and the partitions to be
>> exposed via MTD device partition nodes.
> 
> Your commit is missing a Signed-off-by
> 
> 
>> ---
>>  drivers/mtd/devices/powernv_flash.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/devices/powernv_flash.c b/drivers/mtd/devices/powernv_flash.c
>> index 26f9feaa5d17..f76045f78221 100644
>> --- a/drivers/mtd/devices/powernv_flash.c
>> +++ b/drivers/mtd/devices/powernv_flash.c
>> @@ -2,6 +2,7 @@
>>   * OPAL PNOR flash MTD abstraction
>>   *
>>   * Copyright IBM 2015
>> + * Copyright Raptor Engineering, LLC 2018
>>   *
>>   * This program is free software; you can redistribute it and/or modify
>>   * it under the terms of the GNU General Public License as published by
>> @@ -47,6 +48,8 @@ enum flash_op {
>>         FLASH_OP_ERASE,
>>  };
>>
>> +static char const * const part_probes[] = { "ofpart", NULL };
>> +
>>  /*
>>   * Don't return -ERESTARTSYS if we can't get a token, the MTD core
>>   * might have split up the call from userspace and called into the
>> @@ -267,9 +270,14 @@ static int powernv_flash_probe(struct platform_device *pdev)
>>         /*
>>          * The current flash that skiboot exposes is one contiguous flash chip
>>          * with an ffs partition at the start, it should prove easier for users
>> -        * to deal with partitions or not as they see fit
>> +        * to deal with partitions or not as they see fit.  skitboot places this
>> +        * on the first MTD partition.
>> +        *
>> +        * Certain partitions may also be exposed to the host, such as the boot
>> +        * kernel firmware partition.
>>          */
>> -       return mtd_device_register(&data->mtd, NULL, 0);
>> +       mtd_set_of_node(&data->mtd, dev->of_node);
>> +       return mtd_device_parse_register(&data->mtd, part_probes, NULL, NULL, 0);
> 
> It seems the only change introduced by this patch is passing a list of
> parsers. That way you override a default list of parsers:
> "cmdlinepart", "ofpart", NULL
> with a custom one:
> "ofpart", NULL
> 
> I don't see how it really changes anything/much. Can you explain it
> please? It seems that the only purpose of your change is to don't
> probe "cmdlinepart" parser which reads "mtdparts=" from the cmd line.
> 
> Which parser exactly is supposed to detect partitions on your device?
> "fixed-partitions"? "ofoldpart"? Does your DTB contains "compatible"
> property for flash node? Please paste the relevant part of your DTB if
> applicable.
> 
> 
>>  }
>>
>>  /**
> 


-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com