* [PATCH 0/3] ARM: da8xx: fix section mismatch in new drivers @ 2016-11-22 10:41 Bartosz Golaszewski 2016-11-22 10:41 ` [PATCH 1/3] of: base: add support to get machine compatible string Bartosz Golaszewski ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Bartosz Golaszewski @ 2016-11-22 10:41 UTC (permalink / raw) To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy, Sudeep Holla, Bartosz Golaszewski Sekhar noticed there's a section mismatch in the da8xx-mstpri and da8xx-ddrctl drivers. This is caused by calling of_flat_dt_get_machine_name() which has an __init annotation. This series addresses this issue by introducing a new function that allows to retrieve the compatible property of the root node and using it instead of of_flat_dt_get_machine_name() in the new drivers. Bartosz Golaszewski (3): of: base: add support to get machine compatible string bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name() memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name() drivers/bus/da8xx-mstpri.c | 2 +- drivers/memory/da8xx-ddrctl.c | 2 +- drivers/of/base.c | 22 ++++++++++++++++++++++ include/linux/of.h | 6 ++++++ 4 files changed, 30 insertions(+), 2 deletions(-) -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] of: base: add support to get machine compatible string 2016-11-22 10:41 [PATCH 0/3] ARM: da8xx: fix section mismatch in new drivers Bartosz Golaszewski @ 2016-11-22 10:41 ` Bartosz Golaszewski [not found] ` <1479811311-3080-2-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2016-11-22 10:41 ` [PATCH 2/3] bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name() Bartosz Golaszewski 2016-11-22 10:41 ` [PATCH 3/3] memory: da8xx-ddrctl: " Bartosz Golaszewski 2 siblings, 1 reply; 17+ messages in thread From: Bartosz Golaszewski @ 2016-11-22 10:41 UTC (permalink / raw) To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King Cc: linux-devicetree, LKML, linux-drm, Bartosz Golaszewski, Tomi Valkeinen, Jyri Sarha, Sudeep Holla, Robin Murphy, arm-soc, Laurent Pinchart Add a function allowing to retrieve the compatible string of the root node of the device tree. Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> --- drivers/of/base.c | 22 ++++++++++++++++++++++ include/linux/of.h | 6 ++++++ 2 files changed, 28 insertions(+) diff --git a/drivers/of/base.c b/drivers/of/base.c index a0bccb5..bbfe5e9 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -546,6 +546,28 @@ int of_machine_is_compatible(const char *compat) EXPORT_SYMBOL(of_machine_is_compatible); /** + * of_machine_get_compatible - Get the compatible property of the root node + * + * Returns a NULL-terminated string containing the compatible if it could + * be found, NULL otherwise. + */ +const char *of_machine_get_compatible(void) +{ + struct device_node *root; + const char *compatible; + int ret = -1; + + root = of_find_node_by_path("/"); + if (root) { + ret = of_property_read_string(root, "compatible", &compatible); + of_node_put(root); + } + + return ret ? NULL : compatible; +} +EXPORT_SYMBOL(of_machine_get_compatible); + +/** * __of_device_is_available - check if a device is available for use * * @device: Node to check for availability, with locks already held diff --git a/include/linux/of.h b/include/linux/of.h index 299aeb1..664b734 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -367,6 +367,7 @@ extern int of_alias_get_id(struct device_node *np, const char *stem); extern int of_alias_get_highest_id(const char *stem); extern int of_machine_is_compatible(const char *compat); +extern const char *of_machine_get_compatible(void); extern int of_add_property(struct device_node *np, struct property *prop); extern int of_remove_property(struct device_node *np, struct property *prop); @@ -788,6 +789,11 @@ static inline int of_machine_is_compatible(const char *compat) return 0; } +static inline const char *of_machine_get_compatible(void) +{ + return NULL; +} + static inline bool of_console_check(const struct device_node *dn, const char *name, int index) { return false; -- 2.9.3 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 17+ messages in thread
[parent not found: <1479811311-3080-2-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH 1/3] of: base: add support to get machine compatible string [not found] ` <1479811311-3080-2-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> @ 2016-11-22 10:53 ` Sudeep Holla [not found] ` <5ce9fb9f-459a-562b-2e9f-85d35f9ec035-5wv7dgnIgG8@public.gmane.org> 2016-11-22 15:06 ` Sekhar Nori 0 siblings, 2 replies; 17+ messages in thread From: Sudeep Holla @ 2016-11-22 10:53 UTC (permalink / raw) To: Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King Cc: Sudeep Holla, LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy 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. -- Regards, Sudeep [1] http://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg1274549.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <5ce9fb9f-459a-562b-2e9f-85d35f9ec035-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH 1/3] of: base: add support to get machine compatible string [not found] ` <5ce9fb9f-459a-562b-2e9f-85d35f9ec035-5wv7dgnIgG8@public.gmane.org> @ 2016-11-22 10:57 ` Bartosz Golaszewski 2016-11-22 10:58 ` Bartosz Golaszewski 2016-11-22 11:06 ` Sudeep Holla 0 siblings, 2 replies; 17+ messages in thread From: Bartosz Golaszewski @ 2016-11-22 10:57 UTC (permalink / raw) To: Sudeep Holla Cc: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King, LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy 2016-11-22 11:53 GMT+01:00 Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>: > > > 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. > > -- > Regards, > Sudeep > > [1] http://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg1274549.html Rob's patch checks the model first - I'm not sure this is the behavior we want here as Sekhar suggested we print the machine compatible. Thanks, Bartosz Golaszewski -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] of: base: add support to get machine compatible string 2016-11-22 10:57 ` Bartosz Golaszewski @ 2016-11-22 10:58 ` Bartosz Golaszewski 2016-11-22 11:06 ` Sudeep Holla 1 sibling, 0 replies; 17+ messages in thread From: Bartosz Golaszewski @ 2016-11-22 10:58 UTC (permalink / raw) To: Sudeep Holla Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, Kevin Hilman, Michael Turquette, Sekhar Nori, Russell King, linux-drm, LKML, Peter Ujfalusi, Rob Herring, Jyri Sarha, Robin Murphy, Frank Rowand, arm-soc, Laurent Pinchart 2016-11-22 11:57 GMT+01:00 Bartosz Golaszewski <bgolaszewski@baylibre.com>: > 2016-11-22 11:53 GMT+01:00 Sudeep Holla <sudeep.holla@arm.com>: >> >> >> 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. >> >> -- >> Regards, >> Sudeep >> >> [1] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1274549.html > > Rob's patch checks the model first - I'm not sure this is the behavior > we want here as Sekhar suggested we print the machine compatible. > I meant your patch of course. Thanks, Bartosz _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] of: base: add support to get machine compatible string 2016-11-22 10:57 ` Bartosz Golaszewski 2016-11-22 10:58 ` Bartosz Golaszewski @ 2016-11-22 11:06 ` Sudeep Holla [not found] ` <dd85c5f9-0899-7da3-ab58-730107bfb02c-5wv7dgnIgG8@public.gmane.org> 1 sibling, 1 reply; 17+ messages in thread From: Sudeep Holla @ 2016-11-22 11:06 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Sudeep Holla, Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King, LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy On 22/11/16 10:57, Bartosz Golaszewski wrote: > 2016-11-22 11:53 GMT+01:00 Sudeep Holla <sudeep.holla@arm.com>: >> >> >> 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. > > Rob's patch checks the model first - I'm not sure this is the behavior > we want here as Sekhar suggested we print the machine compatible. IIUC, you are replacing of_flat_dt_get_machine_name and of_machine_get_model_name does exactly same. So I don't see any point in adding this new function and it's just used for logging purpose. Also Sekhar just gave example by using just compatible adding that function in the driver itself. As Arnd suggested me[1], you should for v4.10 fix it in the driver itself to avoid the cross tree dependencies at this point similar to [2] -- Regards, Sudeep [1] https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg111428.html [2] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1274502.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <dd85c5f9-0899-7da3-ab58-730107bfb02c-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH 1/3] of: base: add support to get machine compatible string [not found] ` <dd85c5f9-0899-7da3-ab58-730107bfb02c-5wv7dgnIgG8@public.gmane.org> @ 2016-11-22 12:17 ` Sekhar Nori 0 siblings, 0 replies; 17+ messages in thread From: Sekhar Nori @ 2016-11-22 12:17 UTC (permalink / raw) To: Sudeep Holla, Bartosz Golaszewski Cc: Kevin Hilman, Michael Turquette, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King, LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy On Tuesday 22 November 2016 04:36 PM, Sudeep Holla wrote: > > > On 22/11/16 10:57, Bartosz Golaszewski wrote: >> 2016-11-22 11:53 GMT+01:00 Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>: >>> >>> >>> 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. >> >> Rob's patch checks the model first - I'm not sure this is the behavior >> we want here as Sekhar suggested we print the machine compatible. > > IIUC, you are replacing of_flat_dt_get_machine_name and > of_machine_get_model_name does exactly same. So I don't see any point in > adding this new function and it's just used for logging purpose. > Also Sekhar just gave example by using just compatible adding that > function in the driver itself. > > As Arnd suggested me[1], you should for v4.10 fix it in the driver > itself to avoid the cross tree dependencies at this point similar to [2] +1. Bartosz, can you please fix it in the driver for v4.10. If there is an API available, we can move to it for v4.11 Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] of: base: add support to get machine compatible string 2016-11-22 10:53 ` Sudeep Holla [not found] ` <5ce9fb9f-459a-562b-2e9f-85d35f9ec035-5wv7dgnIgG8@public.gmane.org> @ 2016-11-22 15:06 ` Sekhar Nori 2016-11-22 15:46 ` Sudeep Holla 1 sibling, 1 reply; 17+ messages in thread From: Sekhar Nori @ 2016-11-22 15:06 UTC (permalink / raw) To: Sudeep Holla, Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy 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. This is the code for reference: +int of_machine_get_model_name(const char **model) +{ + int error; + + if (!of_node_get(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); + of_node_put(of_root); + + return error; +} +EXPORT_SYMBOL(of_machine_get_model_name); Thanks, Sekhar ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] of: base: add support to get machine compatible string 2016-11-22 15:06 ` Sekhar Nori @ 2016-11-22 15:46 ` Sudeep Holla [not found] ` <fdb93150-9089-c7bb-2b0a-21ded241a647-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Sudeep Holla @ 2016-11-22 15:46 UTC (permalink / raw) To: Sekhar Nori, Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King Cc: Sudeep Holla, LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy 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 ? Also I have seen drivers today just using this pointer directly, but it's better to copy the string(I just saw this done in one case) > This is the code for reference: > > +int of_machine_get_model_name(const char **model) > +{ > + int error; > + > + if (!of_node_get(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); > + of_node_put(of_root); > + > + return error; > +} > +EXPORT_SYMBOL(of_machine_get_model_name); > > Thanks, > Sekhar > -- Regards, Sudeep ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <fdb93150-9089-c7bb-2b0a-21ded241a647-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH 1/3] of: base: add support to get machine compatible string [not found] ` <fdb93150-9089-c7bb-2b0a-21ded241a647-5wv7dgnIgG8@public.gmane.org> @ 2016-11-23 7:49 ` Sekhar Nori 2016-11-23 10:05 ` Sudeep Holla 0 siblings, 1 reply; 17+ messages in thread From: Sekhar Nori @ 2016-11-23 7:49 UTC (permalink / raw) To: Sudeep Holla, Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy 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. > Also I have seen drivers today just using this pointer directly, but > it's better to copy the string(I just saw this done in one case) Hmm, the reference is given up before the API returns, so I doubt copying it later is any additional benefit. I suspect this is a theoretical issue though since root device node is probably never freed. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] of: base: add support to get machine compatible string 2016-11-23 7:49 ` Sekhar Nori @ 2016-11-23 10:05 ` Sudeep Holla [not found] ` <2a644b8c-d91e-5ab1-200b-00f749a36307-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Sudeep Holla @ 2016-11-23 10:05 UTC (permalink / raw) To: Sekhar Nori Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, David Airlie, Kevin Hilman, Michael Turquette, Russell King, linux-drm, LKML, Bartosz Golaszewski, Rob Herring, Jyri Sarha, Sudeep Holla, Robin Murphy, Frank Rowand, arm-soc, Laurent Pinchart 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. >> Also I have seen drivers today just using this pointer directly, but >> it's better to copy the string(I just saw this done in one case) > > Hmm, the reference is given up before the API returns, so I doubt > copying it later is any additional benefit. > True. > I suspect this is a theoretical issue though since root device node is > probably never freed. > Indeed, not sure if it's worth adding additional code to release the nod at all call sites. -- Regards, Sudeep ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <2a644b8c-d91e-5ab1-200b-00f749a36307-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH 1/3] of: base: add support to get machine compatible string [not found] ` <2a644b8c-d91e-5ab1-200b-00f749a36307-5wv7dgnIgG8@public.gmane.org> @ 2016-11-23 11:47 ` Sekhar Nori 2016-11-23 12:07 ` Sudeep Holla 0 siblings, 1 reply; 17+ messages in thread From: Sekhar Nori @ 2016-11-23 11:47 UTC (permalink / raw) To: Sudeep Holla Cc: Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King, LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] of: base: add support to get machine compatible string 2016-11-23 11:47 ` Sekhar Nori @ 2016-11-23 12:07 ` Sudeep Holla [not found] ` <f0ba1a7a-8948-21c3-8a96-d46bf105bb7e-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Sudeep Holla @ 2016-11-23 12:07 UTC (permalink / raw) To: Sekhar Nori Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, David Airlie, Kevin Hilman, Michael Turquette, Russell King, linux-drm, LKML, Bartosz Golaszewski, Rob Herring, Jyri Sarha, Sudeep Holla, Robin Murphy, Frank Rowand, arm-soc, Laurent Pinchart On 23/11/16 11:47, Sekhar Nori wrote: > 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: > Are we planning to use this in any time sensitive paths? Anyways I am fine removing them. > +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. Frank expressed his concerns and it may be reverted. -- Regards, Sudeep ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <f0ba1a7a-8948-21c3-8a96-d46bf105bb7e-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH 1/3] of: base: add support to get machine compatible string [not found] ` <f0ba1a7a-8948-21c3-8a96-d46bf105bb7e-5wv7dgnIgG8@public.gmane.org> @ 2016-11-23 12:13 ` Sekhar Nori [not found] ` <11467504-c700-cbfa-a945-be9ec8776144-l0cyMroinI0@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Sekhar Nori @ 2016-11-23 12:13 UTC (permalink / raw) To: Sudeep Holla Cc: Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King, LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy On Wednesday 23 November 2016 05:37 PM, Sudeep Holla wrote: >> 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: >> > > Are we planning to use this in any time sensitive paths? Anyways I am > fine removing them. Not worried about the time taken as much as it serving as a bad example and getting carried over to other places where the impact might actually be real. > >> +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. > > Frank expressed his concerns and it may be reverted. Didn't notice that. I will check that thread. Thanks! Regards, Sekhar -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <11467504-c700-cbfa-a945-be9ec8776144-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH 1/3] of: base: add support to get machine compatible string [not found] ` <11467504-c700-cbfa-a945-be9ec8776144-l0cyMroinI0@public.gmane.org> @ 2016-11-23 12:15 ` Sudeep Holla 0 siblings, 0 replies; 17+ messages in thread From: Sudeep Holla @ 2016-11-23 12:15 UTC (permalink / raw) To: Sekhar Nori Cc: Sudeep Holla, Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King, LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy On 23/11/16 12:13, Sekhar Nori wrote: > On Wednesday 23 November 2016 05:37 PM, Sudeep Holla wrote: >>> 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: >>> >> >> Are we planning to use this in any time sensitive paths? Anyways I am >> fine removing them. > > Not worried about the time taken as much as it serving as a bad example > and getting carried over to other places where the impact might actually > be real. > Ah OK, sure. -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name() 2016-11-22 10:41 [PATCH 0/3] ARM: da8xx: fix section mismatch in new drivers Bartosz Golaszewski 2016-11-22 10:41 ` [PATCH 1/3] of: base: add support to get machine compatible string Bartosz Golaszewski @ 2016-11-22 10:41 ` Bartosz Golaszewski 2016-11-22 10:41 ` [PATCH 3/3] memory: da8xx-ddrctl: " Bartosz Golaszewski 2 siblings, 0 replies; 17+ messages in thread From: Bartosz Golaszewski @ 2016-11-22 10:41 UTC (permalink / raw) To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King Cc: linux-devicetree, LKML, linux-drm, Bartosz Golaszewski, Tomi Valkeinen, Jyri Sarha, Sudeep Holla, Robin Murphy, arm-soc, Laurent Pinchart In order to avoid a section mismatch use of_machine_get_compatible() instead of of_flat_dt_get_machine_name() when printing the error message. Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> --- drivers/bus/da8xx-mstpri.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/bus/da8xx-mstpri.c b/drivers/bus/da8xx-mstpri.c index 85f0b53..41cbbe6 100644 --- a/drivers/bus/da8xx-mstpri.c +++ b/drivers/bus/da8xx-mstpri.c @@ -227,7 +227,7 @@ static int da8xx_mstpri_probe(struct platform_device *pdev) prio_list = da8xx_mstpri_get_board_prio(); if (!prio_list) { dev_err(dev, "no master priotities defined for board '%s'\n", - of_flat_dt_get_machine_name()); + of_machine_get_compatible()); return -EINVAL; } -- 2.9.3 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name() 2016-11-22 10:41 [PATCH 0/3] ARM: da8xx: fix section mismatch in new drivers Bartosz Golaszewski 2016-11-22 10:41 ` [PATCH 1/3] of: base: add support to get machine compatible string Bartosz Golaszewski 2016-11-22 10:41 ` [PATCH 2/3] bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name() Bartosz Golaszewski @ 2016-11-22 10:41 ` Bartosz Golaszewski 2 siblings, 0 replies; 17+ messages in thread From: Bartosz Golaszewski @ 2016-11-22 10:41 UTC (permalink / raw) To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King Cc: linux-devicetree, LKML, linux-drm, Bartosz Golaszewski, Tomi Valkeinen, Jyri Sarha, Sudeep Holla, Robin Murphy, arm-soc, Laurent Pinchart In order to avoid a section mismatch use of_machine_get_compatible() instead of of_flat_dt_get_machine_name() when printing the error message. Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> --- drivers/memory/da8xx-ddrctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c index a20e7bb..179f505 100644 --- a/drivers/memory/da8xx-ddrctl.c +++ b/drivers/memory/da8xx-ddrctl.c @@ -118,7 +118,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev) setting = da8xx_ddrctl_get_board_settings(); if (!setting) { dev_err(dev, "no settings for board '%s'\n", - of_flat_dt_get_machine_name()); + of_machine_get_compatible()); return -EINVAL; } -- 2.9.3 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-11-23 12:15 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-22 10:41 [PATCH 0/3] ARM: da8xx: fix section mismatch in new drivers Bartosz Golaszewski 2016-11-22 10:41 ` [PATCH 1/3] of: base: add support to get machine compatible string Bartosz Golaszewski [not found] ` <1479811311-3080-2-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2016-11-22 10:53 ` Sudeep Holla [not found] ` <5ce9fb9f-459a-562b-2e9f-85d35f9ec035-5wv7dgnIgG8@public.gmane.org> 2016-11-22 10:57 ` Bartosz Golaszewski 2016-11-22 10:58 ` Bartosz Golaszewski 2016-11-22 11:06 ` Sudeep Holla [not found] ` <dd85c5f9-0899-7da3-ab58-730107bfb02c-5wv7dgnIgG8@public.gmane.org> 2016-11-22 12:17 ` Sekhar Nori 2016-11-22 15:06 ` Sekhar Nori 2016-11-22 15:46 ` Sudeep Holla [not found] ` <fdb93150-9089-c7bb-2b0a-21ded241a647-5wv7dgnIgG8@public.gmane.org> 2016-11-23 7:49 ` Sekhar Nori 2016-11-23 10:05 ` Sudeep Holla [not found] ` <2a644b8c-d91e-5ab1-200b-00f749a36307-5wv7dgnIgG8@public.gmane.org> 2016-11-23 11:47 ` Sekhar Nori 2016-11-23 12:07 ` Sudeep Holla [not found] ` <f0ba1a7a-8948-21c3-8a96-d46bf105bb7e-5wv7dgnIgG8@public.gmane.org> 2016-11-23 12:13 ` Sekhar Nori [not found] ` <11467504-c700-cbfa-a945-be9ec8776144-l0cyMroinI0@public.gmane.org> 2016-11-23 12:15 ` Sudeep Holla 2016-11-22 10:41 ` [PATCH 2/3] bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name() Bartosz Golaszewski 2016-11-22 10:41 ` [PATCH 3/3] memory: da8xx-ddrctl: " Bartosz Golaszewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).