Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1528340

Re: [PATCH 1/3] of: base: add support to get machine compatible string

From Sekhar Nori <nsekhar@ti.com>
Newsgroups linux.kernel
Subject Re: [PATCH 1/3] of: base: add support to get machine compatible string
Date 2016-11-23 12:50 +0100
Message-ID <sGEdP-4jn-1@gated-at.bofh.it> (permalink)
References (2 earlier) <sGgXT-65c-11@gated-at.bofh.it> <sGkRR-lv-67@gated-at.bofh.it> <sGluy-yw-33@gated-at.bofh.it> <sGADg-1Z9-19@gated-at.bofh.it> <sGCF3-3u2-25@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wednesday 23 November 2016 03:35 PM, Sudeep Holla wrote:
> 
> 
> On 23/11/16 07:49, Sekhar Nori wrote:
>> On Tuesday 22 November 2016 09:16 PM, Sudeep Holla wrote:
>>> Hi Sekhar,
>>>
>>> On 22/11/16 15:06, Sekhar Nori wrote:
>>>> Hi Sudeep,
>>>>
>>>> On Tuesday 22 November 2016 04:23 PM, Sudeep Holla wrote:
>>>>>
>>>>>
>>>>> On 22/11/16 10:41, Bartosz Golaszewski wrote:
>>>>>> Add a function allowing to retrieve the compatible string of the root
>>>>>> node of the device tree.
>>>>>>
>>>>>
>>>>> Rob has queued [1] and it's in -next today. You can reuse that if you
>>>>> are planning to target this for v4.11 or just use open coding in your
>>>>> driver for v4.10 and target this move for v4.11 to avoid cross tree
>>>>> dependencies as I already mentioned in your previous thread.
>>>>
>>>> I dont have your original patch in my mailbox, but I wonder if
>>>> returning a pointer to property string for a node whose reference has
>>>> already been released is safe to do? Probably not an issue for the root
>>>> node, but still feels counter-intuitive.
>>>>
>>>
>>> I am not sure if I understand the issue here. Are you referring a case
>>> where of_root is freed ?
>>
>> Yes, right, thats what I was hinting at. Since you are giving up the
>> reference to the device node before the function returns, the user can
>> be left with a dangling reference.
>>
> 
> Yes I agree.

So, the if(!of_node_get()) is just an expensive NULL pointer check. I think 
it is better to be explicit about it by not using of_node_get/put() at all. 
How about:

+int of_machine_get_model_name(const char **model)
+{
+       int error;
+
+       if (!of_root)
+               return -EINVAL;
+
+       error = of_property_read_string(of_root, "model", model);
+       if (error)
+               error = of_property_read_string_index(of_root, "compatible",
+                                                     0, model);
+       return error;
+}
+EXPORT_SYMBOL(of_machine_get_model_name);

I know the patch is already in -next so I guess it depends on how strongly 
Rob feels about this.

Thanks,
Sekhar

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/3] ARM: da8xx: fix section mismatch in new drivers Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-11-22 11:50 +0100
  [PATCH 2/3] bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name() Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-11-22 11:50 +0100
  [PATCH 3/3] memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name() Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-11-22 11:50 +0100
  [PATCH 1/3] of: base: add support to get machine compatible string Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-11-22 11:50 +0100
    Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sudeep Holla <sudeep.holla@arm.com> - 2016-11-22 12:00 +0100
      Re: [PATCH 1/3] of: base: add support to get machine compatible string Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-11-22 12:00 +0100
        Re: [PATCH 1/3] of: base: add support to get machine compatible string Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-11-22 12:00 +0100
        Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sudeep Holla <sudeep.holla@arm.com> - 2016-11-22 12:10 +0100
          Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sekhar Nori <nsekhar@ti.com> - 2016-11-22 13:20 +0100
      Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sekhar Nori <nsekhar@ti.com> - 2016-11-22 16:10 +0100
        Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sudeep Holla <sudeep.holla@arm.com> - 2016-11-22 16:50 +0100
          Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sekhar Nori <nsekhar@ti.com> - 2016-11-23 09:00 +0100
            Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sudeep Holla <sudeep.holla@arm.com> - 2016-11-23 11:10 +0100
              Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sekhar Nori <nsekhar@ti.com> - 2016-11-23 12:50 +0100
                Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sekhar Nori <nsekhar@ti.com> - 2016-11-23 13:20 +0100
                Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sudeep Holla <sudeep.holla@arm.com> - 2016-11-23 13:20 +0100
                Re: [PATCH 1/3] of: base: add support to get machine compatible  string Sudeep Holla <sudeep.holla@arm.com> - 2016-11-23 13:20 +0100

csiph-web