Re: [PATCH 1/2] mtd: add support for partition parsers

[RafaÅ MiÅecki]

On 17 September 2016 at 12:00, Richard Weinberger
<richard.weinberger@xxxxxxxxx> wrote:
> On Tue, Jul 19, 2016 at 10:51 PM, RafaÅ MiÅecki <zajec5@xxxxxxxxx> wrote:
>> This extends MTD subsystem by adding a support for partition parsers
>> that should be used for creating subpartitions. There are some types of
>> partitions that require splitting, like firmware containers.
>> It's common some home routers that a single firmware image gets flashed
>> to predefined partition and then we need to parse it dynamically.
>>
>> The reason for having such parsers is to share code. Right now we have
>> e.g. TRX parsing implemented in bcm47xxpart but this could be used in
>> more cases (e.g. on ramips which doesn't use bcm47xxpart or with DT).
>>
>> This implementation requires marking partition as requiring parsing with
>> a specific parser. This can be used right away with some parsers like
>> bcm47xxpart, in future we will hopefully also find a solution for doing
>> it with ofpart ("fixed-partitions").
>>
>> Signed-off-by: RafaÅ MiÅecki <zajec5@xxxxxxxxx>
>> ---
>> One thing I'm not proud of in this patch is struct mtd_partition casting
>> in order to be able to modify partition offset. It's marked as const so
>> it was the only way I could think of. Any better ideas? Currently parser
>> gets a single MTD and it doesn't have to deal with its offset, which I
>> kind of like as it simplifies things.
>
> I have to admit, I don't fully understand what you are trying to solve.
> Why are you introducing a new concept of partition parsing in patch 2/2?
> We have already one.
>
> Are you unhappy with the way parsers are _requested_?
> Currently MTD core tries cmdlinepart and ofpart, drivers can override that.
> Do you need a way to request a parser via DT or cmdline?

Actually I'm introducing a new concept in 1/2, the later 2/2 just adds
a first case using it.

Yes, I need parser(s) that can be requested in a slightly different
way. I didn't need to change parsers design (you can see my PATCH 2/2
uses standard struct mtd_part_parser). I just needed a way to call a
parser for a specified partition.

I'm working on a standalone TRX parser. Right now I need to request it
from bcm47xxpart. I need this as ramips arch will need TRX support as
well and I don't want to duplicate TRX support code.

In the future I'd like to use TRX from ofpart as well. I was hoping to
work on support for something like this:
partitions {
        compatible = "fixed-partitions";
        #address-cells = <1>;
        #size-cells = <1>;

        partition@0 {
                label = "bootloader";
                reg = <0x00000 0x40000>;
        };

        partition@40000 {
                label = "firmware";
                format = "trx";
                reg = <0x40000 0x1fb0000>;
        };

        partition@1ff0000 {
                label = "nvram";
                reg = <0x1ff0000 0x10000>;
        };
};

-- 
RafaÅ