Hi Heiner,
> /* read ack: SDA should be pulled down by slave, or it may
> * NAK (usually to report problems with the data we wrote).
> + * Report ACK if SDA is write-only.
Minor nit: On first read, I didn't understand. "Always report ACK..." is
maybe a tad clearer.
> */
> @@ -203,6 +204,9 @@ static int i2c_inb(struct i2c_adapter *i2c_adap)
> unsigned char indata = 0;
> struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
>
> + if (!adap->getsda)
> + return -EOPNOTSUPP;
Wouldn't it be better in 'readbytes' returning an errno there?
> - /* Complain if SCL can't be read */
> - if (bit_adap->getscl == NULL) {
> + if (bit_adap->getscl == NULL && bit_adap->getsda == NULL)
> + dev_info(&adap->dev, "I2C-like interface, SDA and SCL are write-only\n");
> + else if (bit_adap->getscl == NULL) {
> + /* Complain if SCL can't be read */
> dev_warn(&adap->dev, "Not I2C compliant: can't read SCL\n");
> dev_warn(&adap->dev, "Bus may be unreliable\n");
Hmm, this is a bit inconsistent with dev_warn and dev_info. How about
this?
if (bit_adap->getscl == NULL)
dev_warn(&adap->dev, "Not I2C compliant: can't read SCL\n");
if (bit_adap->getsda == NULL)
dev_warn(&adap->dev, "Not I2C compliant: can't read SDA\n");
if (bit_adap->getscl == NULL || bit_adap->getsda == NULL)
dev_warn(&adap->dev, "Bus may be unreliable\n");
The above code can surely be simplified. I just wanted to show this
simple approach so we can discuss my suggestion.
All the best,
Wolfram
Attachment:
signature.asc
Description: PGP signature
