Am 03.12.2014 um 13:58 schrieb Harald Geyer:
> Richard Weinberger writes:
>> Currently the driver uses pr_* and dev_* functions.
>> Change all logging functions to dev_* style to be consistent
>> and have the correct device prefix in all messages.
>
> Yes, actually this was on purpose:
> The dev_ messages are really about something wrong with the device.
> Ie if something goes wrong with one device but could perfectly work
> with some other device.
> The pr_ messages OTOH are about something wrong with clock resolution,
> etc that would affect any DHT11 sensor on the system. Ideally we would
> notice these things during probe() and just return with an error there.
> Right now we aren't as clever as that, so we just log an error message
> about the driver, when actually we are reading the device.
>
> That said, I don't have strong feelings about this. If you want to
> change this, I won't object. However if you really want to fix this,
> then the proper thing would be to check for this conditions in
> probe().
Currently we get log messages of style:
"iio iio:deviceX: foo bar"
and:
"dht11 <name in DT>: foo bar"
I really favorite "dht11 <name in DT".
In my device tree every senor has a sane name and log messages look like:
"dht11 toiletten sensor: invalid checksum"
>> This change set also adds new messages to diagnose issues.
>
> Comment below.
>
>> Signed-off-by: Richard Weinberger <rich...@nod.at>
>> ---
>> drivers/iio/humidity/dht11.c | 16 ++++++++++------
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/humidity/dht11.c b/drivers/iio/humidity/dht11.c
>> index 0023699..fbcd7cb 100644
>> --- a/drivers/iio/humidity/dht11.c
>> +++ b/drivers/iio/humidity/dht11.c
>> @@ -96,20 +96,22 @@ static int dht11_decode(struct dht11 *dht11, int offset)
>> timeres = t;
>> }
>> if (2*timeres > DHT11_DATA_BIT_HIGH) {
>> - pr_err("dht11: timeresolution %d too bad for decoding\n",
>> + dev_err(dht11->dev, "timeresolution %d too bad for decoding\n",
>> timeres);
>> return -EIO;
>> }
>> threshold = DHT11_DATA_BIT_HIGH / timeres;
>> if (DHT11_DATA_BIT_LOW/timeres + 1 >= threshold)
>> - pr_err("dht11: WARNING: decoding ambiguous\n");
>> + dev_err(dht11->dev, "decoding ambiguous\n");
>>
>> /* scale down with timeres and check validity */
>> for (i = 0; i < DHT11_BITS_PER_READ; ++i) {
>> t = dht11->edges[offset + 2*i + 2].ts -
>> dht11->edges[offset + 2*i + 1].ts;
>> - if (!dht11->edges[offset + 2*i + 1].value)
>> - return -EIO; /* lost synchronisation */
>> + if (!dht11->edges[offset + 2*i + 1].value) {
>> + dev_err(dht11->dev, "lost synchronisation\n");
>> + return -EIO;
>> + }
>
> Are you sure this warrants a log message? I don't think this provides
> much information. The userspace application should just try reading
> the sensor again.
>
> We could do someting smart and try to recover from such errors, but
> ultimately userspace will need to deal with failed sensor communication
> anyway, so I don't see the point.
The sensors are rather flaky and if they go nuts the user/developer maybe wants
to know why.
>From a plain EIO she has no chance to find out. He'll have to add printk()s by
>hand
to find out.
With a log message one can start digging into the issue.
>> timing[i] = t / timeres;
>> }
>>
>> @@ -119,8 +121,10 @@ static int dht11_decode(struct dht11 *dht11, int offset)
>> temp_dec = dht11_decode_byte(&timing[24], threshold);
>> checksum = dht11_decode_byte(&timing[32], threshold);
>>
>> - if (((hum_int + hum_dec + temp_int + temp_dec) & 0xff) != checksum)
>> + if (((hum_int + hum_dec + temp_int + temp_dec) & 0xff) != checksum) {
>> + dev_err(dht11->dev, "invalid checksum\n");
>> return -EIO;
>> + }
>
> Same thing here.
Same here.
I had some wiring issues with my sensors and wanted to know from where the EIO
comes.
Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
