Re: [PATCH V2 1/2] mtd: bcm47xxpart: move TRX parsing code to separated function

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Thu, 9 Feb 2017 19:21:03 +0100
Marek Vasut <marek.vasut@gmail.com> wrote:

> On 02/09/2017 07:05 PM, Rafał Miłecki wrote:
> > On 2017-02-09 18:28, Marek Vasut wrote:  
> >> On 01/10/2017 11:15 PM, Rafał Miłecki wrote:  
> >>> From: Rafał Miłecki <rafal@milecki.pl>
> >>>
> >>> This change simplifies main parsing loop logic a bit. In future it may
> >>> be useful for moving TRX support to separated module / parser (if we
> >>> implement support for them at some point).
> >>> Finally parsing TRX at the end puts us in a better position as we have
> >>> better flash layout knowledge. It may be useful e.g. if it appears there
> >>> is more than 1 TRX partition.
> >>>
> >>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> >>> ---
> >>> V2: Keep using bcm47xxpart_add_part in new parsing function
> >>> ---
> >>>  drivers/mtd/bcm47xxpart.c | 121
> >>> ++++++++++++++++++++++++++++------------------
> >>>  1 file changed, 74 insertions(+), 47 deletions(-)
> >>>
> >>> diff --git a/drivers/mtd/bcm47xxpart.c b/drivers/mtd/bcm47xxpart.c
> >>> index 283ff7e..1093025 100644
> >>> --- a/drivers/mtd/bcm47xxpart.c
> >>> +++ b/drivers/mtd/bcm47xxpart.c
> >>> @@ -83,6 +83,67 @@ static const char
> >>> *bcm47xxpart_trx_data_part_name(struct mtd_info *master,
> >>>      return "rootfs";
> >>>  }
> >>>
> >>> +static int bcm47xxpart_parse_trx(struct mtd_info *master,
> >>> +                 struct mtd_partition *trx,
> >>> +                 struct mtd_partition *parts,
> >>> +                 size_t parts_len)
> >>> +{
> >>> +    struct trx_header header;
> >>> +    size_t bytes_read;
> >>> +    int curr_part = 0;
> >>> +    int i, err;
> >>> +
> >>> +    if (parts_len < 3) {
> >>> +        pr_warn("No enough space to add TRX partitions!\n");
> >>> +        return -ENOMEM;
> >>> +    }
> >>> +
> >>> +    err = mtd_read(master, trx->offset, sizeof(header), &bytes_read,
> >>> +               (uint8_t *)&header);
> >>> +    if (err && !mtd_is_bitflip(err)) {
> >>> +        pr_err("mtd_read error while reading TRX header: %d\n", err);  
> >>
> >> Minor nit -- Can we make this dev_err() ?  
> > 
> > The problem is parsers receive struct mtd_info (as an argument) with has
> > empty
> > dev. I just tried
> > dev_info(&master->dev, "TEST TEST TEST :)\n");
> > and got this:
> > [    0.491576]  (null): TEST TEST TEST :)
> > 
> > I guess that's the reason why parse_mtd_partitions and
> > mtd_device_parse_register
> > also use pr_* functions.
> > 
> > So this should be improved at mtd core subsystem first, then we can use
> > it in
> > drivers as well.  
> 
> Isn't this patch fixing just that?
>  [PATCH] mtd: Add partition device node to mtd partition devices
> 


No, this patch is only adding a pointer to a device_node object (DT
node).
To solve the problem, we need to have the master dev name set before we
register its partitions, and this is currently only the case if you
enable CONFIG_MTD_PARTITIONED_MASTER.