Re: [PATCH 3/3] drm: bridge: anx78xx: Add anx78xx bridge driver support.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 24, 2016 at 11:41:46AM +0100, Enric Balletbo i Serra wrote:
> +	/* Map slave addresses of ANX7814 */
> +	for (i = 0; i < I2C_NUM_ADDRESSES; i++) {
> +		anx78xx->i2c_dummy[i] = i2c_new_dummy(client->adapter,
> +						anx78xx_i2c_addresses[i] >> 1);
> +		if (!anx78xx->i2c_dummy[i]) {
> +			DRM_ERROR("Failed to reserve i2c bus %02x.\n",
> +				  anx78xx_i2c_addresses[i]);

Missing error code here.

> +			goto err_i2c;
> +		}

I'm, of course, not a fan of the naming.  The name should be based on
what the goto location does...  In this case it turns it off.  Which is
slightly weird because we have not turned it on yet...  I always say
that you should have multiple error labels and you only undo things
which have been done.

Having a common exit path for the other functions where it was "goto out"
makes sense.  But again in those cases I would prefer a meaningful label
name like "goto unlock;".  In the kernel "goto out;" is meaningless, it
could do anything or everything or nothing.  A lot of people like it
of course, but out: label code tends to be buggier than using a
meaningful name.

regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux