Messages in this thread | | | Subject | Re: [PATCH v2 2/5] iommu/mediatek: Add error path for loop of mm_dts_parse | From | Yong Wu <> | Date | Thu, 23 Jun 2022 09:54:44 +0800 |
| |
On Thu, 2022-06-16 at 11:31 +0100, Robin Murphy wrote: > On 2022-06-16 11:08, Yong Wu wrote: > > On Thu, 2022-06-16 at 09:59 +0100, Robin Murphy wrote: > > > On 2022-06-16 06:42, Yong Wu wrote: > > > > The mtk_iommu_mm_dts_parse will parse the smi larbs nodes. if > > > > the > > > > i+1 > > > > larb is parsed fail(return -EINVAL), we should of_node_put for > > > > the > > > > 0..i > > > > larbs. In the fail path, one of_node_put matches with > > > > of_parse_phandle in > > > > it. > > > > > > > > Fixes: d2e9a1102cfc ("iommu/mediatek: Contain MM IOMMU flow > > > > with > > > > the MM TYPE") > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > > > --- > > > > drivers/iommu/mtk_iommu.c | 21 ++++++++++++++++----- > > > > 1 file changed, 16 insertions(+), 5 deletions(-)
[snip..]
> > > > +err_larbnode_put: > > > > + while (i--) { > > > > + larbnode = of_parse_phandle(dev->of_node, > > > > "mediatek,larbs", i); > > > > + if (larbnode && > > > > of_device_is_available(larbnode)) { > > > > + of_node_put(larbnode); > > > > + of_node_put(larbnode); > > > > + } > > > > > > This looks a bit awkward - could we not just iterate through > > > data->larb_imu and put dev->of_node for each valid dev? > > > > It should work. Thanks very much. > > > > > > > > Also, of_find_device_by_node() takes a reference on the struct > > > device > > > itself, so strictly we should be doing put_device() on those as > > > well > > > if we're bailing out. > > > > Thanks for this hint. A new reference for me. I will add it. > > In fact, thinking about it some more we may as well do the > of_node_put() > unconditionally immediately after the of_find_device_by_node() call,
of_node_put is called in component_release_of in the normal case, thus we shouldn't call of_node_put unconditionally. Right?
(Sorry for reply late)
> so > then it's *only* the device references we'd need to worry about > cleaning > up in the failure path. > > Robin.
| |