Re: [PATCH 1/2] mtd: parsers: trx: Allow to specify trx-magic in DT

[Hauke Mehrtens <hauke@hauke-m.de>]

On 3/18/21 7:43 AM, Rafał Miłecki wrote:
> On 15.03.2021 18:07, Hauke Mehrtens wrote:
>> Buffalo uses a different TRX magic for every device, to be able to use
>> this trx parser, make it possible to specify the TRX magic in device
>> tree. If no TRX magic is specified in device tree, the standard value
>> will be used. This value should only be specified if a vendor chooses to
>> use a non standard TRX magic.
>>
>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>> ---
>>   .../bindings/mtd/partitions/brcm,trx.txt      |  5 +++++
>>   drivers/mtd/parsers/parser_trx.c              | 21 ++++++++++++++++++-
>>   2 files changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/mtd/partitions/brcm,trx.txt 
>> b/Documentation/devicetree/bindings/mtd/partitions/brcm,trx.txt
>> index b677147ca4e1..715a18ca36bd 100644
>> --- a/Documentation/devicetree/bindings/mtd/partitions/brcm,trx.txt
>> +++ b/Documentation/devicetree/bindings/mtd/partitions/brcm,trx.txt
>> @@ -28,6 +28,11 @@ detected by a software parsing TRX header.
>>   Required properties:
>>   - compatible : (required) must be "brcm,trx"
>> +Optional properties:
>> +
>> +- trx-magic: TRX magic, if it is different from the default magic
>> +         0x30524448 as a u32.
> 
> It may need to be brcm,trx-magic and ack for sending a separated 
> bt-bindings patch.

Ok, I will rename it to brcm,trx-magic.

>> diff --git a/drivers/mtd/parsers/parser_trx.c 
>> b/drivers/mtd/parsers/parser_trx.c
>> index 8541182134d4..fd424587caa4 100644
>> --- a/drivers/mtd/parsers/parser_trx.c
>> +++ b/drivers/mtd/parsers/parser_trx.c
>> @@ -47,6 +47,24 @@ static const char *parser_trx_data_part_name(struct 
>> mtd_info *master,
>>       return "rootfs";
>>   }
>> +static uint32_t parser_trx_get_magic(struct mtd_info *mtd)
>> +{
>> +    uint32_t trx_magic = TRX_MAGIC;
>> +    struct device_node *np;
>> +    int err;
>> +
>> +    np = mtd_get_of_node(mtd);
>> +    if (!np)
>> +        return trx_magic;
> 
> This check seems redundant, of_ returns -EINVAL also for NULL node.

Thanks for the information, I will remove this check. Then it could also 
be easier to just inline this code.

> 
> 
>> +    /* Get different magic from device tree if specified */
>> +    err = of_property_read_u32(np, "trx-magic", &trx_magic);
>> +    if (err != 0 && err != -EINVAL)
>> +        pr_err("failed to parse \"trx-magic\" DT attribute, use 
>> default: %d\n", err);
> 
> I'm not native, but shouldn't that be s/use/using/ ?

I am also not a native speaker, but I agree with you and will change this.

Hauke

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/