mbox series

[0/4] regulator: qcom_spmi: fix Bad of_node_put() splat

Message ID 20180716113525.9335-1-niklas.cassel@linaro.org
Headers show
Series regulator: qcom_spmi: fix Bad of_node_put() splat | expand

Message

Niklas Cassel July 16, 2018, 11:35 a.m. UTC 
Fix Bad_of_node_put() splat, and some minor checkpatch issues.

Niklas Cassel (4):
  regulator: qcom_spmi: Fix warning Bad of_node_put()
  regulator: qcom_spmi: Use correct regmap when checking for error
  regulator: qcom_spmi: Do not initialise static to NULL
  regulator: qcom_spmi: Indent with tabs instead of spaces

 drivers/regulator/qcom_spmi-regulator.c | 48 +++++++++++++++++--------
 1 file changed, 33 insertions(+), 15 deletions(-)

-- 
2.17.1

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Johan Hovold July 16, 2018, 12:01 p.m. UTC | #1 
Hi Niklas,

On Mon, Jul 16, 2018 at 01:35:22PM +0200, Niklas Cassel wrote:
> For of_find_node_by_name(), you typically pass what the previous call

> returned. Therefore, of_find_node_by_name() increases the refcount of

> the returned node, and decreases the refcount of the node passed as the

> first argument.

> 

> However, in this case we don't pass what the previous call returned,

> so we have to increase the refcount of the first argument to compensate.


I don't think this is the right fix. of_find_node_by_name() should
generally not be used by drivers in the first place as it searches the
entire tree and can end up matching an entirely unrelated node.

I haven't looked at the device-tree binding in question, but you
probably want to use something like of_get_child_by_name() instead.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Niklas Cassel July 16, 2018, 1:29 p.m. UTC | #2 
On Mon, Jul 16, 2018 at 02:01:34PM +0200, Johan Hovold wrote:
> Hi Niklas,

> 

> On Mon, Jul 16, 2018 at 01:35:22PM +0200, Niklas Cassel wrote:

> > For of_find_node_by_name(), you typically pass what the previous call

> > returned. Therefore, of_find_node_by_name() increases the refcount of

> > the returned node, and decreases the refcount of the node passed as the

> > first argument.

> > 

> > However, in this case we don't pass what the previous call returned,

> > so we have to increase the refcount of the first argument to compensate.

> 

> I don't think this is the right fix. of_find_node_by_name() should

> generally not be used by drivers in the first place as it searches the

> entire tree and can end up matching an entirely unrelated node.

> 

> I haven't looked at the device-tree binding in question, but you

> probably want to use something like of_get_child_by_name() instead.

> 


Hello Johan,

of_find_node_by_name() will only search the whole tree if the
first argument is NULL, which isn't the case here.

However, of_get_child_by_name() is indeed better suited here.
Will send out a v2.

Thank you for your feedback, it is much appreciated :)

Kind regards,
Niklas
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html