| Message ID | 20241013154056.12532-4-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: atomisp: mt9m114: Make it work on Asus T100TA | expand |
On Sun, Oct 13, 2024 at 05:40:55PM +0200, Hans de Goede wrote: > The input_lock was not being initialized, fix this. > > Also switch to devm_kzalloc() for the main driver data struct, so that > devm_mutex_init() can be used for this. ... > ret = mt9m114_s_config(&dev->sd, client->irq, pdata); > if (!pdata || ret) { Hmm... What is the ret value when no pdata is provided? > v4l2_device_unregister_subdev(&dev->sd); > - kfree(dev); > return ret; > } ... > ret = atomisp_register_i2c_module(&dev->sd, pdata); > if (ret) { > v4l2_device_unregister_subdev(&dev->sd); > - kfree(dev); > /* Coverity CID 298095 - return on error */ This comment is useless. But it seems we tend to drop this code completely in the future. > return ret; > }
Hi, On 14-Oct-24 1:31 PM, Andy Shevchenko wrote: > On Sun, Oct 13, 2024 at 05:40:55PM +0200, Hans de Goede wrote: >> The input_lock was not being initialized, fix this. >> >> Also switch to devm_kzalloc() for the main driver data struct, so that >> devm_mutex_init() can be used for this. > > ... > >> ret = mt9m114_s_config(&dev->sd, client->irq, pdata); >> if (!pdata || ret) { > > Hmm... What is the ret value when no pdata is provided? 0, so this error-exit leaves the driver bound (pre-existing problem) The error handling in the probe function is quite foobar in general, but I just wanted to get the driver working as a POC / to have a known working setup before focusing on using the other driver since that is the way forward. Regards, Hans > >> v4l2_device_unregister_subdev(&dev->sd); >> - kfree(dev); >> return ret; >> } > > ... > >> ret = atomisp_register_i2c_module(&dev->sd, pdata); >> if (ret) { >> v4l2_device_unregister_subdev(&dev->sd); >> - kfree(dev); >> /* Coverity CID 298095 - return on error */ > > This comment is useless. But it seems we tend to drop this code completely in > the future. > >> return ret; >> } >
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c index b0b740dd3ca3..1a67f93a53d7 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c @@ -30,6 +30,7 @@ #include <linux/delay.h> #include <linux/i2c.h> #include <linux/acpi.h> +#include <linux/mutex.h> #include "../include/linux/atomisp_gmin_platform.h" #include <media/v4l2-device.h> @@ -1527,7 +1528,6 @@ static void mt9m114_remove(struct i2c_client *client) v4l2_device_unregister_subdev(sd); media_entity_cleanup(&dev->sd.entity); v4l2_ctrl_handler_free(&dev->ctrl_handler); - kfree(dev); } static int mt9m114_probe(struct i2c_client *client) @@ -1538,10 +1538,14 @@ static int mt9m114_probe(struct i2c_client *client) void *pdata; /* Setup sensor configuration structure */ - dev = kzalloc(sizeof(*dev), GFP_KERNEL); + dev = devm_kzalloc(&client->dev, sizeof(*dev), GFP_KERNEL); if (!dev) return -ENOMEM; + ret = devm_mutex_init(&client->dev, &dev->input_lock); + if (ret) + return ret; + v4l2_i2c_subdev_init(&dev->sd, client, &mt9m114_ops); pdata = gmin_camera_platform_data(&dev->sd, ATOMISP_INPUT_FORMAT_RAW_10, @@ -1550,14 +1554,12 @@ static int mt9m114_probe(struct i2c_client *client) ret = mt9m114_s_config(&dev->sd, client->irq, pdata); if (!pdata || ret) { v4l2_device_unregister_subdev(&dev->sd); - kfree(dev); return ret; } ret = atomisp_register_i2c_module(&dev->sd, pdata); if (ret) { v4l2_device_unregister_subdev(&dev->sd); - kfree(dev); /* Coverity CID 298095 - return on error */ return ret; }
The input_lock was not being initialized, fix this. Also switch to devm_kzalloc() for the main driver data struct, so that devm_mutex_init() can be used for this. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)