Re: [PATCH v2 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver

On Wed, Oct 09, 2019 at 02:30:32PM +0300, Dan Carpenter wrote:

...

• platform = kzalloc(sizeof(*platform), GFP_KERNEL);
• if (!platform) {

• DRM_DEV_ERROR(dev, "failed to allocate driver data\n");
  

• ret = -ENOMEM;
  

• goto exit;
  

return -ENOMEM;

...

• }
• pdata = &platform->pdata;
• /* device tree parsing function call */
• ret = anx7625_parse_dt(&client->dev, pdata);
• if (ret != 0) /* if occurs error */

• goto err0;
  

!= 0 is double negative. Choose better label names. Remove the obvious comment.

if (ret) goto free_platform;

...

• anx7625_init_gpio(platform);
• /* to access global platform data */
• platform->client = client;
• i2c_set_clientdata(client, platform);
• if (platform->pdata.extcon_supported) {

• /* get extcon device from DTS */
  

• platform->extcon = extcon_get_edev_by_phandle(&client->dev, 0);
  

• if (PTR_ERR(platform->extcon) == -EPROBE_DEFER)
  

• 	goto err0;
  

Preserve the error code.

...

• if (IS_ERR(platform->extcon)) {
  

• 	DRM_DEV_ERROR(&client->dev,
  

• 		      "can not get extcon device!");
  

• 	goto err0;
  

Prerve the error code.

...

• }
  

• ret = anx7625_extcon_notifier_init(platform);
  

• if (ret < 0)
  

• 	goto err0;
  

• }
• atomic_set(&platform->power_status, 0);
• mutex_init(&platform->lock);
• if (platform->pdata.gpio_intr_hpd) {

• INIT_WORK(&platform->work, anx7625_work_func);
  

• platform->workqueue = create_workqueue("anx7625_work");
  

• if (!platform->workqueue) {
  

• 	DRM_DEV_ERROR(dev, "failed to create work queue\n");
  

• 	ret = -ENOMEM;
  

• 	goto err1;
  

This goto will crash. Because you have forgotten what the most recently allocated resource was. It should be "goto free_platform;" still.

...

• }
  

• platform->pdata.hpd_irq =
  

• 	gpiod_to_irq(platform->pdata.gpio_intr_hpd);
  

• if (platform->pdata.hpd_irq < 0) {
  

• 	DRM_DEV_ERROR(dev, "failed to get gpio irq\n");
  

• 	goto err1;
  

goto free_wq;

...

• }
  

• ret = request_threaded_irq(platform->pdata.hpd_irq,
  

• 			   NULL, anx7625_intr_hpd_isr,
  

• 			   IRQF_TRIGGER_FALLING |
  

• 			   IRQF_TRIGGER_RISING |
  

• 			   IRQF_ONESHOT,
  

• 			   "anx7625-hpd", platform);
  

• if (ret < 0) {
  

• 	DRM_DEV_ERROR(dev, "failed to request irq\n");
  

• 	goto err1;
  

• }
  

• ret = irq_set_irq_wake(platform->pdata.hpd_irq, 1);
  

• if (ret < 0) {
  

• 	DRM_DEV_ERROR(dev, "Request irq for hpd");
  

• 	DRM_DEV_ERROR(dev, "interrupt wake set fail\n");
  

• 	goto err1;
  

• }
  

• ret = enable_irq_wake(platform->pdata.hpd_irq);
  

• if (ret < 0) {
  

• 	DRM_DEV_ERROR(dev, "Enable irq for HPD");
  

• 	DRM_DEV_ERROR(dev, "interrupt wake enable fail\n");
  

• 	goto err1;
  

• }
  

• }
• if (anx7625_register_i2c_dummy_clients(platform, client) != 0) {

Preserve the error code.

ret = anx7625_register_i2c_dummy_clients(); if (ret) goto free_platform;

I meant goto free_wq here. That's the most recent allocation.

regards, dan carpenter