The 01/10 change adds and exports new of_get_i2c_adapter_by_node() interface of i2c core, the rest of patches fix current users of of_find_i2c_adapter_by_node() interface.
of_find_i2c_adapter_by_node() call requires quite often missing put_device(), and i2c_put_adapter() releases a device locked by i2c_get_adapter() only. In general module_put(adapter->owner) and put_device(dev) are not interchangeable.
This is a common error reproduction scenario as a result of the misusage described above (this is run on iMX6 platform with HDMI and I2C bus drivers compiled as kernel modules for clearness):
root@mx6q:~# lsmod | grep i2c i2c_imx 10213 0 root@mx6q:~# lsmod | grep dw_hdmi_imx dw_hdmi_imx 3631 0 dw_hdmi 11846 1 dw_hdmi_imx imxdrm 8674 3 dw_hdmi_imx,imx_ipuv3_crtc,imx_ldb drm_kms_helper 113765 5 dw_hdmi,imxdrm,imx_ipuv3_crtc,imx_ldb root@mx6q:~# rmmod dw_hdmi_imx root@mx6q:~# lsmod | grep i2c i2c_imx 10213 -1
^^^^^
root@mx6q:~# rmmod i2c_imx rmmod: ERROR: Module i2c_imx is in use
To fix existing users of these interfaces and to avoid any further confusion and misusage in future, add one more interface of_get_i2c_adapter_by_node(), it is similar to i2c_get_adapter() in sense that an I2C bus device driver found and locked by user can be correctly unlocked by i2c_put_adapter().
Mainly the change concerns DRM users of I2C bus device.
The change is based on torvalds/master branch, d6ac4ffc61a
RFC of the 01/10 change is http://www.spinics.net/lists/linux-i2c/msg20257.html
Vladimir Zapolskiy (10): i2c: add and export of_get_i2c_adapter_by_node() interface drm: dw_hdmi: use of_get_i2c_adapter_by_node interface drm: exynos_hdmi: use of_get_i2c_adapter_by_node interface drm: imx-tve: use of_get_i2c_adapter_by_node interface drm: panel-simple: use of_get_i2c_adapter_by_node interface drm: sti_hdmi: use of_get_i2c_adapter_by_node interface drm: tegra: use of_get_i2c_adapter_by_node interface drm: tilcdc: use of_get_i2c_adapter_by_node interface fbdev: omap2: connector-dvi: use of_get_i2c_adapter_by_node interface i2c: i2c-arb-gpio-challenge: use of_get_i2c_adapter_by_node interface
drivers/gpu/drm/bridge/dw_hdmi.c | 14 ++++-- drivers/gpu/drm/exynos/exynos_hdmi.c | 7 +-- drivers/gpu/drm/imx/imx-tve.c | 56 +++++++++++++++------- drivers/gpu/drm/panel/panel-simple.c | 9 ++-- drivers/gpu/drm/sti/sti_hdmi.c | 19 +++----- drivers/gpu/drm/tegra/output.c | 19 ++++---- drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 6 +-- drivers/i2c/i2c-core.c | 20 ++++++++ drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 3 +- .../video/fbdev/omap2/displays-new/connector-dvi.c | 2 +- include/linux/i2c.h | 6 +++ 11 files changed, 104 insertions(+), 57 deletions(-)
of_find_i2c_adapter_by_node() call requires quite often missing put_device(), and i2c_put_adapter() releases a device locked by i2c_get_adapter() only. In general module_put(adapter->owner) and put_device(dev) are not interchangeable.
This is a common error reproduction scenario as a result of the misusage described above (for clearness this is run on iMX6 platform with HDMI and I2C bus drivers compiled as kernel modules):
root@mx6q:~# lsmod | grep i2c i2c_imx 10213 0 root@mx6q:~# lsmod | grep dw_hdmi_imx dw_hdmi_imx 3631 0 dw_hdmi 11846 1 dw_hdmi_imx imxdrm 8674 3 dw_hdmi_imx,imx_ipuv3_crtc,imx_ldb drm_kms_helper 113765 5 dw_hdmi,imxdrm,imx_ipuv3_crtc,imx_ldb root@mx6q:~# rmmod dw_hdmi_imx root@mx6q:~# lsmod | grep i2c i2c_imx 10213 -1
^^^^^
root@mx6q:~# rmmod i2c_imx rmmod: ERROR: Module i2c_imx is in use
To fix existing users of these interfaces and to avoid any further confusion and misusage in future, add one more interface of_get_i2c_adapter_by_node(), it is similar to i2c_get_adapter() in sense that an I2C bus device driver found and locked by user can be correctly unlocked by i2c_put_adapter().
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com --- The change is based on RFC http://www.spinics.net/lists/linux-i2c/msg20257.html
* added new exported function declaration in include/linux/i2c.h * added put_device(dev) call right inside of_get_i2c_adapter_by_node() * corrected authorship of the change
drivers/i2c/i2c-core.c | 20 ++++++++++++++++++++ include/linux/i2c.h | 6 ++++++ 2 files changed, 26 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 069a41f..0d902ab 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -1356,6 +1356,26 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node) return i2c_verify_adapter(dev); } EXPORT_SYMBOL(of_find_i2c_adapter_by_node); + +struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node) +{ + struct device *dev; + struct i2c_adapter *adapter; + + dev = bus_find_device(&i2c_bus_type, NULL, node, + of_dev_node_match); + if (!dev) + return NULL; + + adapter = i2c_verify_adapter(dev); + if (adapter && !try_module_get(adapter->owner)) + adapter = NULL; + + put_device(dev); + + return adapter; +} +EXPORT_SYMBOL(of_get_i2c_adapter_by_node); #else static void of_i2c_register_devices(struct i2c_adapter *adap) { } #endif /* CONFIG_OF */ diff --git a/include/linux/i2c.h b/include/linux/i2c.h index e83a738..87bb217 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -638,6 +638,7 @@ extern struct i2c_client *of_find_i2c_device_by_node(struct device_node *node); /* must call put_device() when done with returned i2c_adapter device */ extern struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node);
+struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node); #else
static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node *node) @@ -649,6 +650,11 @@ static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node { return NULL; } + +static inline struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node) +{ + return NULL; +} #endif /* CONFIG_OF */
#endif /* _LINUX_I2C_H */
On Wed, Jul 08, 2015 at 03:59:12PM +0300, Vladimir Zapolskiy wrote:
of_find_i2c_adapter_by_node() call requires quite often missing put_device(), and i2c_put_adapter() releases a device locked by i2c_get_adapter() only. In general module_put(adapter->owner) and put_device(dev) are not interchangeable.
This is a common error reproduction scenario as a result of the misusage described above (for clearness this is run on iMX6 platform with HDMI and I2C bus drivers compiled as kernel modules):
root@mx6q:~# lsmod | grep i2c i2c_imx 10213 0 root@mx6q:~# lsmod | grep dw_hdmi_imx dw_hdmi_imx 3631 0 dw_hdmi 11846 1 dw_hdmi_imx imxdrm 8674 3 dw_hdmi_imx,imx_ipuv3_crtc,imx_ldb drm_kms_helper 113765 5 dw_hdmi,imxdrm,imx_ipuv3_crtc,imx_ldb root@mx6q:~# rmmod dw_hdmi_imx root@mx6q:~# lsmod | grep i2c i2c_imx 10213 -1 ^^^^^ root@mx6q:~# rmmod i2c_imx rmmod: ERROR: Module i2c_imx is in useTo fix existing users of these interfaces and to avoid any further confusion and misusage in future, add one more interface of_get_i2c_adapter_by_node(), it is similar to i2c_get_adapter() in sense that an I2C bus device driver found and locked by user can be correctly unlocked by i2c_put_adapter().
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com
The change is based on RFC http://www.spinics.net/lists/linux-i2c/msg20257.html
- added new exported function declaration in include/linux/i2c.h
- added put_device(dev) call right inside of_get_i2c_adapter_by_node()
- corrected authorship of the change
drivers/i2c/i2c-core.c | 20 ++++++++++++++++++++ include/linux/i2c.h | 6 ++++++ 2 files changed, 26 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 069a41f..0d902ab 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -1356,6 +1356,26 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node) return i2c_verify_adapter(dev); } EXPORT_SYMBOL(of_find_i2c_adapter_by_node);
+struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node) +{
- struct device *dev;
- struct i2c_adapter *adapter;
- dev = bus_find_device(&i2c_bus_type, NULL, node,
of_dev_node_match);- if (!dev)
return NULL;- adapter = i2c_verify_adapter(dev);
- if (adapter && !try_module_get(adapter->owner))
adapter = NULL;- put_device(dev);
I don't think this is correct. Users still need to keep a reference to the device, otherwise it can simply disappear even if the module stays around (think sysfs bind/unbind attributes).
Looking at i2c_put_adapter() it seems like it would need to do more than just drop the module reference. Then again, that probably means that we need to add a get_device() somewhere in i2c_get_adapter() to balance the put_device() in i2c_put_adapter().
Thierry
Hi Thierry,
On 08.07.2015 16:11, Thierry Reding wrote:
On Wed, Jul 08, 2015 at 03:59:12PM +0300, Vladimir Zapolskiy wrote:
of_find_i2c_adapter_by_node() call requires quite often missing put_device(), and i2c_put_adapter() releases a device locked by i2c_get_adapter() only. In general module_put(adapter->owner) and put_device(dev) are not interchangeable.
This is a common error reproduction scenario as a result of the misusage described above (for clearness this is run on iMX6 platform with HDMI and I2C bus drivers compiled as kernel modules):
root@mx6q:~# lsmod | grep i2c i2c_imx 10213 0 root@mx6q:~# lsmod | grep dw_hdmi_imx dw_hdmi_imx 3631 0 dw_hdmi 11846 1 dw_hdmi_imx imxdrm 8674 3 dw_hdmi_imx,imx_ipuv3_crtc,imx_ldb drm_kms_helper 113765 5 dw_hdmi,imxdrm,imx_ipuv3_crtc,imx_ldb root@mx6q:~# rmmod dw_hdmi_imx root@mx6q:~# lsmod | grep i2c i2c_imx 10213 -1 ^^^^^ root@mx6q:~# rmmod i2c_imx rmmod: ERROR: Module i2c_imx is in useTo fix existing users of these interfaces and to avoid any further confusion and misusage in future, add one more interface of_get_i2c_adapter_by_node(), it is similar to i2c_get_adapter() in sense that an I2C bus device driver found and locked by user can be correctly unlocked by i2c_put_adapter().
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com
The change is based on RFC http://www.spinics.net/lists/linux-i2c/msg20257.html
- added new exported function declaration in include/linux/i2c.h
- added put_device(dev) call right inside of_get_i2c_adapter_by_node()
- corrected authorship of the change
drivers/i2c/i2c-core.c | 20 ++++++++++++++++++++ include/linux/i2c.h | 6 ++++++ 2 files changed, 26 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 069a41f..0d902ab 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -1356,6 +1356,26 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node) return i2c_verify_adapter(dev); } EXPORT_SYMBOL(of_find_i2c_adapter_by_node);
+struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node) +{
- struct device *dev;
- struct i2c_adapter *adapter;
- dev = bus_find_device(&i2c_bus_type, NULL, node,
of_dev_node_match);- if (!dev)
return NULL;- adapter = i2c_verify_adapter(dev);
- if (adapter && !try_module_get(adapter->owner))
adapter = NULL;- put_device(dev);
I don't think this is correct. Users still need to keep a reference to the device, otherwise it can simply disappear even if the module stays around (think sysfs bind/unbind attributes).
Looking at i2c_put_adapter() it seems like it would need to do more than just drop the module reference. Then again, that probably means that we need to add a get_device() somewhere in i2c_get_adapter() to balance the put_device() in i2c_put_adapter().
it makes sense for me, thanks for momentary review.
I'm hesitating to add put_device(dev) to i2c_put_adapter() etc. in this series though. After development and testing I would like to send another preceding independent change updating i2c_get_adapter(), i2c_put_adapter() and clients (or if you wish you can do it), then I'll rebase 01/10 on top of it, the rest most probably is unchanged.
-- With best wishes, Vladimir
On Wed, Jul 08, 2015 at 04:31:37PM +0300, Vladimir Zapolskiy wrote:
Hi Thierry,
On 08.07.2015 16:11, Thierry Reding wrote:
On Wed, Jul 08, 2015 at 03:59:12PM +0300, Vladimir Zapolskiy wrote:
of_find_i2c_adapter_by_node() call requires quite often missing put_device(), and i2c_put_adapter() releases a device locked by i2c_get_adapter() only. In general module_put(adapter->owner) and put_device(dev) are not interchangeable.
This is a common error reproduction scenario as a result of the misusage described above (for clearness this is run on iMX6 platform with HDMI and I2C bus drivers compiled as kernel modules):
root@mx6q:~# lsmod | grep i2c i2c_imx 10213 0 root@mx6q:~# lsmod | grep dw_hdmi_imx dw_hdmi_imx 3631 0 dw_hdmi 11846 1 dw_hdmi_imx imxdrm 8674 3 dw_hdmi_imx,imx_ipuv3_crtc,imx_ldb drm_kms_helper 113765 5 dw_hdmi,imxdrm,imx_ipuv3_crtc,imx_ldb root@mx6q:~# rmmod dw_hdmi_imx root@mx6q:~# lsmod | grep i2c i2c_imx 10213 -1 ^^^^^ root@mx6q:~# rmmod i2c_imx rmmod: ERROR: Module i2c_imx is in useTo fix existing users of these interfaces and to avoid any further confusion and misusage in future, add one more interface of_get_i2c_adapter_by_node(), it is similar to i2c_get_adapter() in sense that an I2C bus device driver found and locked by user can be correctly unlocked by i2c_put_adapter().
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com
The change is based on RFC http://www.spinics.net/lists/linux-i2c/msg20257.html
- added new exported function declaration in include/linux/i2c.h
- added put_device(dev) call right inside of_get_i2c_adapter_by_node()
- corrected authorship of the change
drivers/i2c/i2c-core.c | 20 ++++++++++++++++++++ include/linux/i2c.h | 6 ++++++ 2 files changed, 26 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 069a41f..0d902ab 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -1356,6 +1356,26 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node) return i2c_verify_adapter(dev); } EXPORT_SYMBOL(of_find_i2c_adapter_by_node);
+struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node) +{
- struct device *dev;
- struct i2c_adapter *adapter;
- dev = bus_find_device(&i2c_bus_type, NULL, node,
of_dev_node_match);- if (!dev)
return NULL;- adapter = i2c_verify_adapter(dev);
- if (adapter && !try_module_get(adapter->owner))
adapter = NULL;- put_device(dev);
I don't think this is correct. Users still need to keep a reference to the device, otherwise it can simply disappear even if the module stays around (think sysfs bind/unbind attributes).
Looking at i2c_put_adapter() it seems like it would need to do more than just drop the module reference. Then again, that probably means that we need to add a get_device() somewhere in i2c_get_adapter() to balance the put_device() in i2c_put_adapter().
it makes sense for me, thanks for momentary review.
I'm hesitating to add put_device(dev) to i2c_put_adapter() etc. in this series though. After development and testing I would like to send another preceding independent change updating i2c_get_adapter(), i2c_put_adapter() and clients (or if you wish you can do it), then I'll rebase 01/10 on top of it, the rest most probably is unchanged.
I think that would make sense, yes.
Thierry
This change is needed to properly lock I2C bus driver, which serves DDC.
The change fixes an overflow over zero of I2C bus driver user counter:
root@mx6q:~# lsmod | grep i2c i2c_imx 10213 0 root@mx6q:~# lsmod | grep dw_hdmi_imx dw_hdmi_imx 3631 0 dw_hdmi 11846 1 dw_hdmi_imx imxdrm 8674 3 dw_hdmi_imx,imx_ipuv3_crtc,imx_ldb drm_kms_helper 113765 5 dw_hdmi,imxdrm,imx_ipuv3_crtc,imx_ldb root@mx6q:~# rmmod dw_hdmi_imx root@mx6q:~# lsmod | grep i2c i2c_imx 10213 -1
^^^^^
root@mx6q:~# rmmod i2c_imx rmmod: ERROR: Module i2c_imx is in use
Note that prior to this change put_device() coupled with of_find_i2c_adapter_by_node() was missing on error path of dw_hdmi_bind(), added i2c_put_adapter() there along with the change.
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com --- drivers/gpu/drm/bridge/dw_hdmi.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c index 816d104..e86776c 100644 --- a/drivers/gpu/drm/bridge/dw_hdmi.c +++ b/drivers/gpu/drm/bridge/dw_hdmi.c @@ -1591,7 +1591,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); if (ddc_node) { - hdmi->ddc = of_find_i2c_adapter_by_node(ddc_node); + hdmi->ddc = of_get_i2c_adapter_by_node(ddc_node); of_node_put(ddc_node); if (!hdmi->ddc) { dev_dbg(hdmi->dev, "failed to read ddc node\n"); @@ -1603,20 +1603,22 @@ int dw_hdmi_bind(struct device *dev, struct device *master, }
hdmi->regs = devm_ioremap_resource(dev, iores); - if (IS_ERR(hdmi->regs)) - return PTR_ERR(hdmi->regs); + if (IS_ERR(hdmi->regs)) { + ret = PTR_ERR(hdmi->regs); + goto err_res; + }
hdmi->isfr_clk = devm_clk_get(hdmi->dev, "isfr"); if (IS_ERR(hdmi->isfr_clk)) { ret = PTR_ERR(hdmi->isfr_clk); dev_err(hdmi->dev, "Unable to get HDMI isfr clk: %d\n", ret); - return ret; + goto err_res; }
ret = clk_prepare_enable(hdmi->isfr_clk); if (ret) { dev_err(hdmi->dev, "Cannot enable HDMI isfr clock: %d\n", ret); - return ret; + goto err_res; }
hdmi->iahb_clk = devm_clk_get(hdmi->dev, "iahb"); @@ -1682,6 +1684,8 @@ err_iahb: clk_disable_unprepare(hdmi->iahb_clk); err_isfr: clk_disable_unprepare(hdmi->isfr_clk); +err_res: + i2c_put_adapter(hdmi->ddc);
return ret; }
This change is needed to properly lock I2C bus driver, which serves DDC.
On release of_get_i2c_adapter_by_node() requires i2c_put_adapter() call, which replaces put_device(). By the way added of_node_put(ddc_node) to eliminate memory leak, if OF_DYNAMIC is enabled.
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com --- drivers/gpu/drm/exynos/exynos_hdmi.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 99e2864..399eff9 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2407,7 +2407,8 @@ static int hdmi_probe(struct platform_device *pdev) }
out_get_ddc_adpt: - hdata->ddc_adpt = of_find_i2c_adapter_by_node(ddc_node); + hdata->ddc_adpt = of_get_i2c_adapter_by_node(ddc_node); + of_node_put(ddc_node); if (!hdata->ddc_adpt) { DRM_ERROR("Failed to get ddc i2c adapter by node\n"); return -EPROBE_DEFER; @@ -2485,7 +2486,7 @@ err_hdmiphy: if (hdata->hdmiphy_port) put_device(&hdata->hdmiphy_port->dev); err_ddc: - put_device(&hdata->ddc_adpt->dev); + i2c_put_adapter(hdata->ddc_adpt);
return ret; } @@ -2501,7 +2502,7 @@ static int hdmi_remove(struct platform_device *pdev)
if (hdata->hdmiphy_port) put_device(&hdata->hdmiphy_port->dev); - put_device(&hdata->ddc_adpt->dev); + i2c_put_adapter(hdata->ddc_adpt);
pm_runtime_disable(&pdev->dev); component_del(&pdev->dev, &hdmi_component_ops);
This change is needed to properly lock I2C bus driver, which serves DDC.
Note that prior to this change put_device() coupled with of_find_i2c_adapter_by_node() is missing on imx_tve_bind() error path and imx_tve_unbind(), also the change fixes possibly left enabled voltage regulator on imx_tve_bind() error path.
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com --- drivers/gpu/drm/imx/imx-tve.c | 56 +++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index 214ecee..f1ac927 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -581,14 +581,15 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data)
ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); if (ddc_node) { - tve->ddc = of_find_i2c_adapter_by_node(ddc_node); + tve->ddc = of_get_i2c_adapter_by_node(ddc_node); of_node_put(ddc_node); }
tve->mode = of_get_tve_mode(np); if (tve->mode != TVE_MODE_VGA) { dev_err(dev, "only VGA mode supported, currently\n"); - return -EINVAL; + ret = -EINVAL; + goto i2c_release; }
if (tve->mode == TVE_MODE_VGA) { @@ -597,7 +598,7 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data)
if (ret < 0) { dev_err(dev, "failed to get vsync pin\n"); - return ret; + goto i2c_release; }
ret |= of_property_read_u32(np, "fsl,vsync-pin", @@ -605,14 +606,16 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data)
if (ret < 0) { dev_err(dev, "failed to get vsync pin\n"); - return ret; + goto i2c_release; } }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0); base = devm_ioremap_resource(dev, res); - if (IS_ERR(base)) - return PTR_ERR(base); + if (IS_ERR(base)) { + ret = PTR_ERR(base); + goto i2c_release; + }
tve_regmap_config.lock_arg = tve; tve->regmap = devm_regmap_init_mmio_clk(dev, "tve", base, @@ -620,13 +623,15 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) if (IS_ERR(tve->regmap)) { dev_err(dev, "failed to init regmap: %ld\n", PTR_ERR(tve->regmap)); - return PTR_ERR(tve->regmap); + ret = PTR_ERR(tve->regmap); + goto i2c_release; }
irq = platform_get_irq(pdev, 0); if (irq < 0) { dev_err(dev, "failed to get irq\n"); - return irq; + ret = irq; + goto i2c_release; }
ret = devm_request_threaded_irq(dev, irq, NULL, @@ -634,7 +639,7 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) "imx-tve", tve); if (ret < 0) { dev_err(dev, "failed to request irq: %d\n", ret); - return ret; + goto i2c_release; }
tve->dac_reg = devm_regulator_get(dev, "dac"); @@ -642,14 +647,15 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) regulator_set_voltage(tve->dac_reg, 2750000, 2750000); ret = regulator_enable(tve->dac_reg); if (ret) - return ret; + goto i2c_release; }
tve->clk = devm_clk_get(dev, "tve"); if (IS_ERR(tve->clk)) { dev_err(dev, "failed to get high speed tve clock: %ld\n", PTR_ERR(tve->clk)); - return PTR_ERR(tve->clk); + ret = PTR_ERR(tve->clk); + goto regulator_release; }
/* this is the IPU DI clock input selector, can be parented to tve_di */ @@ -657,36 +663,48 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) if (IS_ERR(tve->di_sel_clk)) { dev_err(dev, "failed to get ipu di mux clock: %ld\n", PTR_ERR(tve->di_sel_clk)); - return PTR_ERR(tve->di_sel_clk); + ret = PTR_ERR(tve->di_sel_clk); + goto regulator_release; }
ret = tve_clk_init(tve, base); if (ret < 0) - return ret; + goto regulator_release;
ret = regmap_read(tve->regmap, TVE_COM_CONF_REG, &val); if (ret < 0) { dev_err(dev, "failed to read configuration register: %d\n", ret); - return ret; + goto regulator_release; } if (val != 0x00100000) { - dev_err(dev, "configuration register default value indicates this is not a TVEv2\n"); - return -ENODEV; + dev_err(dev, + "configuration register default value indicates this is not a TVEv2\n"); + ret = -ENODEV; + goto regulator_release; }
/* disable cable detection for VGA mode */ ret = regmap_write(tve->regmap, TVE_CD_CONT_REG, 0); if (ret) - return ret; + goto regulator_release;
ret = imx_tve_register(drm, tve); if (ret) - return ret; + goto regulator_release;
dev_set_drvdata(dev, tve);
return 0; + + regulator_release: + if (!IS_ERR(tve->dac_reg)) + regulator_disable(tve->dac_reg); + + i2c_release: + i2c_put_adapter(tve->ddc); + + return ret; }
static void imx_tve_unbind(struct device *dev, struct device *master, @@ -699,6 +717,8 @@ static void imx_tve_unbind(struct device *dev, struct device *master,
if (!IS_ERR(tve->dac_reg)) regulator_disable(tve->dac_reg); + + i2c_put_adapter(tve->ddc); }
static const struct component_ops imx_tve_ops = {
This change is needed to properly lock I2C bus driver, which serves DDC.
On release of_get_i2c_adapter_by_node() requires i2c_put_adapter() call, which replaces put_device().
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com --- drivers/gpu/drm/panel/panel-simple.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index f94201b..b8a8b23 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -313,7 +313,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
ddc = of_parse_phandle(dev->of_node, "ddc-i2c-bus", 0); if (ddc) { - panel->ddc = of_find_i2c_adapter_by_node(ddc); + panel->ddc = of_get_i2c_adapter_by_node(ddc); of_node_put(ddc);
if (!panel->ddc) { @@ -335,8 +335,8 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) return 0;
free_ddc: - if (panel->ddc) - put_device(&panel->ddc->dev); + i2c_put_adapter(panel->ddc); + free_backlight: if (panel->backlight) put_device(&panel->backlight->dev); @@ -353,8 +353,7 @@ static int panel_simple_remove(struct device *dev)
panel_simple_disable(&panel->base);
- if (panel->ddc) - put_device(&panel->ddc->dev); + i2c_put_adapter(panel->ddc);
if (panel->backlight) put_device(&panel->backlight->dev);
This change is needed to properly lock I2C bus driver, which serves DDC.
On release of_get_i2c_adapter_by_node() requires i2c_put_adapter() call.
Note, that prior to the change put_device() coupled with of_find_i2c_adapter_by_node() was incorrectly placed to sti_hdmi_remove() instead of sti_hdmi_unbind().
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com --- drivers/gpu/drm/sti/sti_hdmi.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index f28a4d5..580a413 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -698,14 +698,10 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
ddc = of_parse_phandle(dev->of_node, "ddc", 0); if (ddc) { - hdmi->ddc_adapt = of_find_i2c_adapter_by_node(ddc); - if (!hdmi->ddc_adapt) { - err = -EPROBE_DEFER; - of_node_put(ddc); - return err; - } - + hdmi->ddc_adapt = of_get_i2c_adapter_by_node(ddc); of_node_put(ddc); + if (!hdmi->ddc_adapt) + return -EPROBE_DEFER; }
/* Set the drm device handle */ @@ -762,14 +758,15 @@ err_sysfs: err_connector: drm_connector_cleanup(drm_connector); err_adapt: - put_device(&hdmi->ddc_adapt->dev); + i2c_put_adapter(hdmi->ddc_adapt); + return -EINVAL; }
static void sti_hdmi_unbind(struct device *dev, struct device *master, void *data) { - /* do nothing */ + i2c_put_adapter(hdmi->ddc_adapt); }
static const struct component_ops sti_hdmi_ops = { @@ -885,10 +882,8 @@ static int sti_hdmi_remove(struct platform_device *pdev) { struct sti_hdmi *hdmi = dev_get_drvdata(&pdev->dev);
- if (hdmi->ddc_adapt) - put_device(&hdmi->ddc_adapt->dev); - component_del(&pdev->dev, &sti_hdmi_ops); + return 0; }
This change is needed to properly lock I2C bus driver, which serves DDC.
On release of_get_i2c_adapter_by_node() requires i2c_put_adapter() call.
Note, that prior to the change put_device() coupled with of_find_i2c_adapter_by_node() was missing on error path of tegra_output_probe().
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com --- drivers/gpu/drm/tegra/output.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c index 37db479..d7731cd 100644 --- a/drivers/gpu/drm/tegra/output.c +++ b/drivers/gpu/drm/tegra/output.c @@ -116,7 +116,7 @@ int tegra_output_probe(struct tegra_output *output)
ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0); if (ddc) { - output->ddc = of_find_i2c_adapter_by_node(ddc); + output->ddc = of_get_i2c_adapter_by_node(ddc); if (!output->ddc) { err = -EPROBE_DEFER; of_node_put(ddc); @@ -136,14 +136,13 @@ int tegra_output_probe(struct tegra_output *output) "HDMI hotplug detect"); if (err < 0) { dev_err(output->dev, "gpio_request_one(): %d\n", err); - return err; + goto i2c_release; }
err = gpio_to_irq(output->hpd_gpio); if (err < 0) { dev_err(output->dev, "gpio_to_irq(): %d\n", err); - gpio_free(output->hpd_gpio); - return err; + goto gpio_release; }
output->hpd_irq = err; @@ -156,8 +155,7 @@ int tegra_output_probe(struct tegra_output *output) if (err < 0) { dev_err(output->dev, "failed to request IRQ#%u: %d\n", output->hpd_irq, err); - gpio_free(output->hpd_gpio); - return err; + goto gpio_release; }
output->connector.polled = DRM_CONNECTOR_POLL_HPD; @@ -171,6 +169,12 @@ int tegra_output_probe(struct tegra_output *output) }
return 0; + + gpio_release: + gpio_free(output->hpd_gpio); + i2c_release: + i2c_put_adapter(output->ddc); + return err; }
void tegra_output_remove(struct tegra_output *output) @@ -180,8 +184,7 @@ void tegra_output_remove(struct tegra_output *output) gpio_free(output->hpd_gpio); }
- if (output->ddc) - put_device(&output->ddc->dev); + i2c_put_adapter(output->ddc); }
int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
This change is needed to properly lock I2C bus driver, which serves DDC.
Prior to this change i2c_put_adapter() is misused, which may lead to an overflow over zero of I2C bus driver user counter.
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com --- drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c index 354c47c..4dc78c7 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c @@ -348,15 +348,13 @@ static int tfp410_probe(struct platform_device *pdev) goto fail; }
- tfp410_mod->i2c = of_find_i2c_adapter_by_node(i2c_node); + tfp410_mod->i2c = of_get_i2c_adapter_by_node(i2c_node); + of_node_put(i2c_node); if (!tfp410_mod->i2c) { dev_err(&pdev->dev, "could not get i2c\n"); - of_node_put(i2c_node); goto fail; }
- of_node_put(i2c_node); - tfp410_mod->gpio = of_get_named_gpio_flags(node, "powerdn-gpio", 0, NULL); if (IS_ERR_VALUE(tfp410_mod->gpio)) {
This change is needed to properly lock I2C bus driver, which serves DDC.
Prior to this change i2c_put_adapter() is misused, which may lead to an overflow over zero of I2C bus driver user counter.
Signed-off-by: Vladimir Zapolskiy vladimir_zapolskiy@mentor.com --- drivers/video/fbdev/omap2/displays-new/connector-dvi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap2/displays-new/connector-dvi.c b/drivers/video/fbdev/omap2/displays-new/connector-dvi.c index a8ce920..d811e6d 100644 --- a/drivers/video/fbdev/omap2/displays-new/connector-dvi.c +++ b/drivers/video/fbdev/omap2/displays-new/connector-dvi.c @@ -294,7 +294,7 @@ static int dvic_probe_of(struct platform_device *pdev)
adapter_node = of_parse_phandle(node, "ddc-i2c-bus", 0); if (adapter_node) { - adapter = of_find_i2c_adapter_by_node(adapter_node); + adapter = of_get_i2c_adapter_by_node(adapter_node); if (adapter == NULL) { dev_err(&pdev->dev, "failed to parse ddc-i2c-bus\n"); omap_dss_put_device(ddata->in);
dri-devel@lists.freedesktop.org
-
Thierry Reding -
Vladimir Zapolskiy