| Message ID | 20221114081956.37853-1-yuancan@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panel/raspberrypi-touchscreen: Fix error handling in rpi_touchscreen_init() | expand |
Hi Yuan On Mon, 14 Nov 2022 at 08:21, Yuan Can <yuancan@huawei.com> wrote: > > A problem about modprobe panel-raspberrypi-touchscreen is triggered with > the following log given: > > [ 542.980748] Error: Driver 'rpi-ts-dsi' is already registered, aborting... > > And with this log printed, the panel_raspberrypi_touchscreen is listed by > lsmod, rmmod on it can trigger the WARN of "Unexpected driver unregister". > > The reason is that the return value of mipi_dsi_driver_register() and > i2c_add_driver() is not checked in rpi_touchscreen_init(), if > i2c_add_driver() failed, the rpi_touchscreen_dsi_driver is never > unregistered, and next time when install this module, the > mipi_dsi_driver_register() is failed but rpi_touchscreen_init() returns 0, > leading to the panel_raspberrypi_touchscreen listed by lsmod. > > Call graph of modprobe panel-raspberrypi-touchscreen at the first time: > rpi_touchscreen_init() > mipi_dsi_driver_register() # register rpi_touchscreen_dsi_driver > i2c_add_driver() > driver_register() > bus_add_driver() > priv = kzalloc(...) # OOM happened > # return without unregister rpi_touchscreen_dsi_driver > > Call graph of retrying modprobe panel-raspberrypi-touchscreen: > rpi_touchscreen_init() > mipi_dsi_driver_register() # Error message printed, register failed! > i2c_add_driver() # succeed and return > > Fix by checking the return value of both functions and unregister > rpi_touchscreen_dsi_driver if i2c_add_driver() failed. > > Fixes: 2f733d6194bd ("drm/panel: Add support for the Raspberry Pi 7" Touchscreen.") > Signed-off-by: Yuan Can <yuancan@huawei.com> Acked-by: Dave Stevenson <dave.stevenson@raspberrypi.com> I would strongly recommend that you look at switching to the tc358762 bridge, panel-simple, and the rpi-panel-attiny-regulator regulator driver, rather than this driver. The raspberrypi-touchscreen driver has no suitable hooks for the edt-ft5x06 touch driver to register for regulator control. If the display sleeps then the power is killed to the touch controller but the touch driver has no knowledge of this. This issue should have been solved with rpi-panel-attiny / tc358762 / panel-simple. Dave > --- > .../gpu/drm/panel/panel-raspberrypi-touchscreen.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c > index 79f852465a84..9f3d0fedc3f2 100644 > --- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c > +++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c > @@ -497,8 +497,17 @@ static struct i2c_driver rpi_touchscreen_driver = { > > static int __init rpi_touchscreen_init(void) > { > - mipi_dsi_driver_register(&rpi_touchscreen_dsi_driver); > - return i2c_add_driver(&rpi_touchscreen_driver); > + int ret; > + > + ret = mipi_dsi_driver_register(&rpi_touchscreen_dsi_driver); > + if (ret) > + return ret; > + > + ret = i2c_add_driver(&rpi_touchscreen_driver); > + if (ret) > + mipi_dsi_driver_unregister(&rpi_touchscreen_dsi_driver); > + > + return ret; > } > module_init(rpi_touchscreen_init); > > -- > 2.17.1 >
在 2022/11/14 19:55, Dave Stevenson 写道: > Hi Yuan > > On Mon, 14 Nov 2022 at 08:21, Yuan Can <yuancan@huawei.com> wrote: >> A problem about modprobe panel-raspberrypi-touchscreen is triggered with >> the following log given: >> >> [ 542.980748] Error: Driver 'rpi-ts-dsi' is already registered, aborting... >> >> And with this log printed, the panel_raspberrypi_touchscreen is listed by >> lsmod, rmmod on it can trigger the WARN of "Unexpected driver unregister". >> >> The reason is that the return value of mipi_dsi_driver_register() and >> i2c_add_driver() is not checked in rpi_touchscreen_init(), if >> i2c_add_driver() failed, the rpi_touchscreen_dsi_driver is never >> unregistered, and next time when install this module, the >> mipi_dsi_driver_register() is failed but rpi_touchscreen_init() returns 0, >> leading to the panel_raspberrypi_touchscreen listed by lsmod. >> >> Call graph of modprobe panel-raspberrypi-touchscreen at the first time: >> rpi_touchscreen_init() >> mipi_dsi_driver_register() # register rpi_touchscreen_dsi_driver >> i2c_add_driver() >> driver_register() >> bus_add_driver() >> priv = kzalloc(...) # OOM happened >> # return without unregister rpi_touchscreen_dsi_driver >> >> Call graph of retrying modprobe panel-raspberrypi-touchscreen: >> rpi_touchscreen_init() >> mipi_dsi_driver_register() # Error message printed, register failed! >> i2c_add_driver() # succeed and return >> >> Fix by checking the return value of both functions and unregister >> rpi_touchscreen_dsi_driver if i2c_add_driver() failed. >> >> Fixes: 2f733d6194bd ("drm/panel: Add support for the Raspberry Pi 7" Touchscreen.") >> Signed-off-by: Yuan Can <yuancan@huawei.com> > Acked-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > I would strongly recommend that you look at switching to the tc358762 > bridge, panel-simple, and the rpi-panel-attiny-regulator regulator > driver, rather than this driver. > > The raspberrypi-touchscreen driver has no suitable hooks for the > edt-ft5x06 touch driver to register for regulator control. If the > display sleeps then the power is killed to the touch controller but > the touch driver has no knowledge of this. This issue should have been > solved with rpi-panel-attiny / tc358762 / panel-simple. Thanks for the suggestion!
diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c index 79f852465a84..9f3d0fedc3f2 100644 --- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c +++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c @@ -497,8 +497,17 @@ static struct i2c_driver rpi_touchscreen_driver = { static int __init rpi_touchscreen_init(void) { - mipi_dsi_driver_register(&rpi_touchscreen_dsi_driver); - return i2c_add_driver(&rpi_touchscreen_driver); + int ret; + + ret = mipi_dsi_driver_register(&rpi_touchscreen_dsi_driver); + if (ret) + return ret; + + ret = i2c_add_driver(&rpi_touchscreen_driver); + if (ret) + mipi_dsi_driver_unregister(&rpi_touchscreen_dsi_driver); + + return ret; } module_init(rpi_touchscreen_init);
A problem about modprobe panel-raspberrypi-touchscreen is triggered with the following log given: [ 542.980748] Error: Driver 'rpi-ts-dsi' is already registered, aborting... And with this log printed, the panel_raspberrypi_touchscreen is listed by lsmod, rmmod on it can trigger the WARN of "Unexpected driver unregister". The reason is that the return value of mipi_dsi_driver_register() and i2c_add_driver() is not checked in rpi_touchscreen_init(), if i2c_add_driver() failed, the rpi_touchscreen_dsi_driver is never unregistered, and next time when install this module, the mipi_dsi_driver_register() is failed but rpi_touchscreen_init() returns 0, leading to the panel_raspberrypi_touchscreen listed by lsmod. Call graph of modprobe panel-raspberrypi-touchscreen at the first time: rpi_touchscreen_init() mipi_dsi_driver_register() # register rpi_touchscreen_dsi_driver i2c_add_driver() driver_register() bus_add_driver() priv = kzalloc(...) # OOM happened # return without unregister rpi_touchscreen_dsi_driver Call graph of retrying modprobe panel-raspberrypi-touchscreen: rpi_touchscreen_init() mipi_dsi_driver_register() # Error message printed, register failed! i2c_add_driver() # succeed and return Fix by checking the return value of both functions and unregister rpi_touchscreen_dsi_driver if i2c_add_driver() failed. Fixes: 2f733d6194bd ("drm/panel: Add support for the Raspberry Pi 7" Touchscreen.") Signed-off-by: Yuan Can <yuancan@huawei.com> --- .../gpu/drm/panel/panel-raspberrypi-touchscreen.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)