RE: [PATCH v3 1/6] mfd: TPS65910: Handle non-existent devices

[Mohammed, Afzal]

Hi Kyla,

On Thu, Nov 03, 2011 at 22:38:01, Kyle Manna wrote:
> The JTAGREVNUM register contains a silicon revision number in the lower
> four bits and the upper four bits are to always read 0.
> 
> To detect the presence of the device, attempt to read JTAGREVNUM
> register and check that it returns a valid value.  If the I2C device
> fails to respond or returns an invalid value, return -ENODEV.
> 
:
:
> +	/* Check that the device is there */
> +	ret = tps65910_i2c_read(tps65910, TPS65910_JTAGVERNUM, 1, &reg);
> +	if (ret < 0 || (reg & ~JTAGVERNUM_VERNUM_MASK)) {
> +		dev_err(tps65910->dev, "unknown version: JTAGREVNUM = 0x%x\n",
> +				       reg);
> +		ret = -ENODEV;
>  		goto err;
> +	}

If i2c read fails, "0" would get printed as version.

Perhaps it would be better to print version irrespective of whether it
is unknown or not, and return error for unknown value, something like,

        unsigned char buff;

        /* Check that the device is actually there */
        ret = tps65910_i2c_read(tps65910, 0x0, 1, &buff);
        if (ret < 0) {
                dev_err(tps65910->dev, "could not be detected\n");
                ret = -ENODEV;
                goto err;
        }

        dev_info(tps65910->dev, "JTAGREVNUM 0x%x\n", buff);

        if (buff & ~JTAGVERNUM_VERNUM_MASK) {
                dev_err(tps65910->dev, "unknown version\n");
                ret = -ENODEV;
                goto err;
        }



Regards
Afzal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/