Em 30-09-2010 18:03, Michael Krufky escreveu:
> On Thu, Sep 30, 2010 at 4:26 PM, Mauro Carvalho Chehab
> <mchehab@xxxxxxxxxx> wrote:
>> Em 30-09-2010 16:27, Michael Krufky escreveu:
>>> Mauro,
>>>
>>> I think that's a reasonable explanation. Would you be open to
>>> reworking the patch such that the register contents only show up if
>>> the device is not recognized? (when ret < 0) . In the case where the
>>> device is correctly identified (ret == 0), I'd rather preserve the
>>> original successful detection message, and not see the ID register
>>> contents.
>>
>> Patch enclosed.
>>
>> ---
>>
>> [PATCH] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned
>>
>> Instead of doing:
>>
>> [ 82.581639] tda18271 4-0060: creating new instance
>> [ 82.588411] Unknown device detected @ 4-0060, device not supported.
>> [ 82.594695] tda18271_attach: [4-0060|M] error -22 on line 1272
>> [ 82.600530] tda18271 4-0060: destroying instance
>>
>> Print:
>> [ 468.740392] Unknown device (0) detected @ 4-0060, device not supported.
>>
>> for the error message, to help detecting what's going wrong with the
>> device.
>>
>> This helps to detect when the driver is using the wrong I2C bus (or have
>> the i2g gate switch pointing to the wrong place), on devices like cx231xx
>> that just return 0 on reads to a non-existent i2c device.
>>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
>>
>> diff --git a/drivers/media/common/tuners/tda18271-fe.c b/drivers/media/common/tuners/tda18271-fe.c
>> index 7955e49..3db8727 100644
>> --- a/drivers/media/common/tuners/tda18271-fe.c
>> +++ b/drivers/media/common/tuners/tda18271-fe.c
>> @@ -1156,7 +1156,6 @@ static int tda18271_get_id(struct dvb_frontend *fe)
>> struct tda18271_priv *priv = fe->tuner_priv;
>> unsigned char *regs = priv->tda18271_regs;
>> char *name;
>> - int ret = 0;
>>
>> mutex_lock(&priv->lock);
>> tda18271_read_regs(fe);
>> @@ -1172,17 +1171,18 @@ static int tda18271_get_id(struct dvb_frontend *fe)
>> priv->id = TDA18271HDC2;
>> break;
>> default:
>> - name = "Unknown device";
>> - ret = -EINVAL;
>> - break;
>> + tda_info("Unknown device (%i) detected @ %d-%04x, device not supported.\n",
>> + regs[R_ID],
>> + i2c_adapter_id(priv->i2c_props.adap),
>> + priv->i2c_props.addr);
>> + return -EINVAL;
>> }
>>
>> - tda_info("%s detected @ %d-%04x%s\n", name,
>> + tda_info("%s detected @ %d-%04x\n", name,
>> i2c_adapter_id(priv->i2c_props.adap),
>> - priv->i2c_props.addr,
>> - (0 == ret) ? "" : ", device not supported.");
>> + priv->i2c_props.addr);
>>
>> - return ret;
>> + return 0;
>> }
>>
>> static int tda18271_setup_configuration(struct dvb_frontend *fe,
>>
>>
>
> This looks good to me, although you could concatenate these lines together:
>
>
>> + tda_info("Unknown device (%i) detected @ %d-%04x, device not supported.\n",
>> + regs[R_ID],
>> + i2c_adapter_id(priv->i2c_props.adap),
>> + priv->i2c_props.addr);
>> + return -EINVAL;
>
>
> to be more like this:
>
>
>> + tda_info("Unknown device (%i) detected @ %d-%04x, device not supported.\n",
>> + regs[R_ID], i2c_adapter_id(priv->i2c_props.adap), priv->i2c_props.addr);
>> + return -EINVAL;
>
>
> ...that is, if it fits within an 80-char line.
All parameters after the format string don't fit on 80 cols. I did a small optimization on that.
>
> Anyway,
>
> Reviewed-by: Michael Krufky <mkrufky@xxxxxxxxxxxxxx>
Thanks, committed.
Cheers,
Mauro.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
