[PATCH v2 5/6] mtd: partitions: pass around 'mtd_partitions' wrapper struct
Boris Brezillon
boris.brezillon at free-electrons.com
Sat Dec 5 00:27:44 PST 2015
Hi Brian,
On Fri, 4 Dec 2015 17:45:11 -0800
Brian Norris <computersforpeace at gmail.com> wrote:
> Hi Boris,
>
> On Sat, Dec 05, 2015 at 01:30:49AM +0100, Boris Brezillon wrote:
> > On Fri, 4 Dec 2015 15:25:17 -0800
> > Brian Norris <computersforpeace at gmail.com> wrote:
>
>
> > How about defining a new function to encourage mtd drivers to pass an
> > mtd_partitions structure instead of the parts + nr_parts arguments.
>
> Hmm, I'm not sure about having drivers use 'mtd_partitions' yet. It's a
> little awkward for them to have access to a 'parser' field there, since
> they might think of it as an input argument. But I do like the wrapper
> function, as it simplifies the core function a bit. I'll try that, and
> if we decide it's good to have drivers also start using something more
> like the non-wrapped version, then maybe we can export it later?
>
> I'm also slightly hesitant, because I think there may be more things
> we'd want to refactor on how drivers pass partition info to the MTD core
> in the future. Ideally, they'd have to pass less, but for the bits we
> can't kill, it might be good to stick all into a single struct. (For
> one, why should the mtd_part_parser_data be separate too?) So in the
> end, we might want a different struct for MTD drivers' use, and I'd like
> not to touch all that right now, if that's OK with you.
Fair enough.
>
> (BTW, I have some RFC of_match_table stuff that I'm preparing to send. I
> might just tack that onto the end of this series' v3.)
>
> > Note that I don't ask to update all call sites, but only to add a new
> > function and transform mtd_device_parse_register() into a wrapper.
> >
> > int mtd_device_parse_and_register_parts(struct mtd_info *mtd,
> > const char *const *types,
> > const struct mtd_partitions *parts)
>
> You've missed parser_data in the prototype?
>
> > {
> > struct mtd_partitions parsed = { };
> > int ret;
> >
> > ret = parse_mtd_partitions(mtd, types, &parsed, parser_data);
> > if (!ret)
> > parts = &parsed;
> >
> > if (!parts || !parts->nr_parts) {
> > /* Didn't come up with parsed OR fallback partitions */
> > pr_info("mtd: failed to find partitions; one or more parsers reports errors (%d)\n",
> > ret);
> > /* Don't abort on errors; we can still use unpartitioned MTD */
> > }
> >
> > ret = mtd_add_device_partitions(mtd, &parsed);
>
> s/&parsed/parts/
>
> right?
Right, but you know how much I like to propose code that does not even
compile ;-).
Best Regards,
Bori
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
More information about the linux-mtd
mailing list