On 01/25/2017 05:51 PM, Boris Brezillon wrote:
> 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).
>
ok. I will cook a v2 with your proposal then.
Thanks,
C.
--
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
