On Wed, 25 Jan 2017 17:47:13 +0100
Cédric Le Goater <clg@xxxxxxxx> wrote:
> On 01/24/2017 03:13 PM, Boris Brezillon wrote:
> > Hi Cédric,
> >
> > On Mon, 16 Jan 2017 14:27:03 +0100
> > Cédric Le Goater <clg@xxxxxxxx> wrote:
> >
> >> This can be used to easily identify a specific chip on a system with
> >> multiple chips.
> >>
> >> Suggested-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>
> >> Signed-off-by: Cédric Le Goater <clg@xxxxxxxx>
> >> ---
> >> drivers/mtd/mtdcore.c | 7 +++++++
> >> 1 file changed, 7 insertions(+)
> >>
> >> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> >> index 052772f7caef..bf61557b599d 100644
> >> --- a/drivers/mtd/mtdcore.c
> >> +++ b/drivers/mtd/mtdcore.c
> >> @@ -654,6 +654,13 @@ static int mtd_add_device_partitions(struct mtd_info *mtd,
> >> */
> >> static void mtd_set_dev_defaults(struct mtd_info *mtd)
> >> {
> >> + /*
> >> + * If DT support is enabled and we have a valid of_node pointer, try to
> >> + * extract the MTD name from the label property.
> >> + */
> >> + if (IS_ENABLED(CONFIG_OF) && mtd->dev.of_node)
> >> + of_property_read_string(mtd->dev.of_node, "label", &mtd->name);
> >> +
> >
> > I realize this kind of thing would be better placed in mtd_set_of_node()
> > (see the patch below). Modifying the mtd->name pointer in the back of
> > MTD drivers is not such a good idea (suppose the driver allocated the
> > memory with a regular kmalloc() and tries to free mtd->name in the remove
> > path).
> >
> > If we move that to mtd_set_of_node(), drivers that wants to support this
> > label property just have to check if mtd->name is NULL (after calling
> > mtd_set_of_node() or nand_set_flash_node()) before assigning a default
> > name.
> > For unmodified drivers we keep the existing behavior: they'll just
> > unconditionally override mtd->name with their own value (which might or
> > might not be dynamically allocated).
>
> ok. So the expected behavior looks correct to me, but adding a call to
> of_property_read_string() in the inline below feels a little hacky.
> Doesn't it ?
>
> May be we need an extra check on IS_ENABLED(CONFIG_OF) also ?
This is all safe, because the of.h header defines stubs if CONFIG_OF is
not set. That just means the name will be unchanged, but you shouldn't
have any problem (neither as compilation time nor at runtime).
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
