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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux