* [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter
@ 2015-07-08 12:54 Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 01/10] i2c: add and export of_get_i2c_adapter_by_node() interface Vladimir Zapolskiy
` (9 more replies)
0 siblings, 10 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 12:54 UTC (permalink / raw
To: linux-arm-kernel
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 at mx6q:~# lsmod | grep i2c
i2c_imx 10213 0
root at 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 at mx6q:~# rmmod dw_hdmi_imx
root at mx6q:~# lsmod | grep i2c
i2c_imx 10213 -1
^^^^^
root at 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(-)
--
2.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 01/10] i2c: add and export of_get_i2c_adapter_by_node() interface
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
@ 2015-07-08 12:59 ` Vladimir Zapolskiy
2015-07-08 13:11 ` Thierry Reding
2015-07-08 12:59 ` [PATCH 02/10] drm: dw_hdmi: use of_get_i2c_adapter_by_node interface Vladimir Zapolskiy
` (8 subsequent siblings)
9 siblings, 1 reply; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 12:59 UTC (permalink / raw
To: linux-arm-kernel
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 at mx6q:~# lsmod | grep i2c
i2c_imx 10213 0
root at 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 at mx6q:~# rmmod dw_hdmi_imx
root at mx6q:~# lsmod | grep i2c
i2c_imx 10213 -1
^^^^^
root at 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 */
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 02/10] drm: dw_hdmi: use of_get_i2c_adapter_by_node interface
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 01/10] i2c: add and export of_get_i2c_adapter_by_node() interface Vladimir Zapolskiy
@ 2015-07-08 12:59 ` Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 03/10] drm: exynos_hdmi: " Vladimir Zapolskiy
` (7 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 12:59 UTC (permalink / raw
To: linux-arm-kernel
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 at mx6q:~# lsmod | grep i2c
i2c_imx 10213 0
root at 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 at mx6q:~# rmmod dw_hdmi_imx
root at mx6q:~# lsmod | grep i2c
i2c_imx 10213 -1
^^^^^
root at 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;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 03/10] drm: exynos_hdmi: use of_get_i2c_adapter_by_node interface
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 01/10] i2c: add and export of_get_i2c_adapter_by_node() interface Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 02/10] drm: dw_hdmi: use of_get_i2c_adapter_by_node interface Vladimir Zapolskiy
@ 2015-07-08 12:59 ` Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 04/10] drm: imx-tve: " Vladimir Zapolskiy
` (6 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 12:59 UTC (permalink / raw
To: linux-arm-kernel
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);
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 04/10] drm: imx-tve: use of_get_i2c_adapter_by_node interface
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
` (2 preceding siblings ...)
2015-07-08 12:59 ` [PATCH 03/10] drm: exynos_hdmi: " Vladimir Zapolskiy
@ 2015-07-08 12:59 ` Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 05/10] drm: panel-simple: " Vladimir Zapolskiy
` (5 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 12:59 UTC (permalink / raw
To: linux-arm-kernel
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 = {
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 05/10] drm: panel-simple: use of_get_i2c_adapter_by_node interface
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
` (3 preceding siblings ...)
2015-07-08 12:59 ` [PATCH 04/10] drm: imx-tve: " Vladimir Zapolskiy
@ 2015-07-08 12:59 ` Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 06/10] drm: sti_hdmi: " Vladimir Zapolskiy
` (4 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 12:59 UTC (permalink / raw
To: linux-arm-kernel
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);
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 06/10] drm: sti_hdmi: use of_get_i2c_adapter_by_node interface
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
` (4 preceding siblings ...)
2015-07-08 12:59 ` [PATCH 05/10] drm: panel-simple: " Vladimir Zapolskiy
@ 2015-07-08 12:59 ` Vladimir Zapolskiy
2015-07-08 13:00 ` [PATCH 07/10] drm: tegra: " Vladimir Zapolskiy
` (3 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 12:59 UTC (permalink / raw
To: linux-arm-kernel
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;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 07/10] drm: tegra: use of_get_i2c_adapter_by_node interface
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
` (5 preceding siblings ...)
2015-07-08 12:59 ` [PATCH 06/10] drm: sti_hdmi: " Vladimir Zapolskiy
@ 2015-07-08 13:00 ` Vladimir Zapolskiy
2015-07-08 13:00 ` [PATCH 08/10] drm: tilcdc: " Vladimir Zapolskiy
` (2 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 13:00 UTC (permalink / raw
To: linux-arm-kernel
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)
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 08/10] drm: tilcdc: use of_get_i2c_adapter_by_node interface
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
` (6 preceding siblings ...)
2015-07-08 13:00 ` [PATCH 07/10] drm: tegra: " Vladimir Zapolskiy
@ 2015-07-08 13:00 ` Vladimir Zapolskiy
2015-07-08 13:00 ` [PATCH 09/10] fbdev: omap2: connector-dvi: " Vladimir Zapolskiy
2015-07-08 13:00 ` [PATCH 10/10] i2c: i2c-arb-gpio-challenge: " Vladimir Zapolskiy
9 siblings, 0 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 13:00 UTC (permalink / raw
To: linux-arm-kernel
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)) {
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 09/10] fbdev: omap2: connector-dvi: use of_get_i2c_adapter_by_node interface
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
` (7 preceding siblings ...)
2015-07-08 13:00 ` [PATCH 08/10] drm: tilcdc: " Vladimir Zapolskiy
@ 2015-07-08 13:00 ` Vladimir Zapolskiy
2015-07-08 13:00 ` [PATCH 10/10] i2c: i2c-arb-gpio-challenge: " Vladimir Zapolskiy
9 siblings, 0 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 13:00 UTC (permalink / raw
To: linux-arm-kernel
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);
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 10/10] i2c: i2c-arb-gpio-challenge: use of_get_i2c_adapter_by_node interface
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
` (8 preceding siblings ...)
2015-07-08 13:00 ` [PATCH 09/10] fbdev: omap2: connector-dvi: " Vladimir Zapolskiy
@ 2015-07-08 13:00 ` Vladimir Zapolskiy
9 siblings, 0 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 13:00 UTC (permalink / raw
To: linux-arm-kernel
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.
By the way added of_node_put() to eliminate memory leak, if OF_DYNAMIC
is enabled.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
index 5cf1b60..402e3a6 100644
--- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
+++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
@@ -196,7 +196,8 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
dev_err(dev, "Cannot parse i2c-parent\n");
return -EINVAL;
}
- arb->parent = of_find_i2c_adapter_by_node(parent_np);
+ arb->parent = of_get_i2c_adapter_by_node(parent_np);
+ of_node_put(parent_np);
if (!arb->parent) {
dev_err(dev, "Cannot find parent bus\n");
return -EPROBE_DEFER;
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 01/10] i2c: add and export of_get_i2c_adapter_by_node() interface
2015-07-08 12:59 ` [PATCH 01/10] i2c: add and export of_get_i2c_adapter_by_node() interface Vladimir Zapolskiy
@ 2015-07-08 13:11 ` Thierry Reding
2015-07-08 13:31 ` Vladimir Zapolskiy
0 siblings, 1 reply; 14+ messages in thread
From: Thierry Reding @ 2015-07-08 13:11 UTC (permalink / raw
To: linux-arm-kernel
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 at mx6q:~# lsmod | grep i2c
> i2c_imx 10213 0
> root at 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 at mx6q:~# rmmod dw_hdmi_imx
> root at mx6q:~# lsmod | grep i2c
> i2c_imx 10213 -1
>
> ^^^^^
>
> root at 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);
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150708/212be892/attachment-0001.sig>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 01/10] i2c: add and export of_get_i2c_adapter_by_node() interface
2015-07-08 13:11 ` Thierry Reding
@ 2015-07-08 13:31 ` Vladimir Zapolskiy
2015-07-08 13:53 ` Thierry Reding
0 siblings, 1 reply; 14+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-08 13:31 UTC (permalink / raw
To: linux-arm-kernel
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 at mx6q:~# lsmod | grep i2c
>> i2c_imx 10213 0
>> root at 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 at mx6q:~# rmmod dw_hdmi_imx
>> root at mx6q:~# lsmod | grep i2c
>> i2c_imx 10213 -1
>>
>> ^^^^^
>>
>> root at 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);
>
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 01/10] i2c: add and export of_get_i2c_adapter_by_node() interface
2015-07-08 13:31 ` Vladimir Zapolskiy
@ 2015-07-08 13:53 ` Thierry Reding
0 siblings, 0 replies; 14+ messages in thread
From: Thierry Reding @ 2015-07-08 13:53 UTC (permalink / raw
To: linux-arm-kernel
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 at mx6q:~# lsmod | grep i2c
> >> i2c_imx 10213 0
> >> root at 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 at mx6q:~# rmmod dw_hdmi_imx
> >> root at mx6q:~# lsmod | grep i2c
> >> i2c_imx 10213 -1
> >>
> >> ^^^^^
> >>
> >> root at 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);
> >
> > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150708/06f627f2/attachment.sig>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-07-08 13:53 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-08 12:54 [PATCH 00/10] i2c/drm: fix i2c adapter device driver user counter Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 01/10] i2c: add and export of_get_i2c_adapter_by_node() interface Vladimir Zapolskiy
2015-07-08 13:11 ` Thierry Reding
2015-07-08 13:31 ` Vladimir Zapolskiy
2015-07-08 13:53 ` Thierry Reding
2015-07-08 12:59 ` [PATCH 02/10] drm: dw_hdmi: use of_get_i2c_adapter_by_node interface Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 03/10] drm: exynos_hdmi: " Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 04/10] drm: imx-tve: " Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 05/10] drm: panel-simple: " Vladimir Zapolskiy
2015-07-08 12:59 ` [PATCH 06/10] drm: sti_hdmi: " Vladimir Zapolskiy
2015-07-08 13:00 ` [PATCH 07/10] drm: tegra: " Vladimir Zapolskiy
2015-07-08 13:00 ` [PATCH 08/10] drm: tilcdc: " Vladimir Zapolskiy
2015-07-08 13:00 ` [PATCH 09/10] fbdev: omap2: connector-dvi: " Vladimir Zapolskiy
2015-07-08 13:00 ` [PATCH 10/10] i2c: i2c-arb-gpio-challenge: " Vladimir Zapolskiy
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 read-only IMAP folder(s) and NNTP newsgroup(s).