Messages in this thread | | | Date | Wed, 12 Nov 2008 23:43:31 +0000 | From | Mark Brown <> | Subject | Re: [PATCH 2.6.28] mfd: Correct WM8350 I2C return code usage |
| |
On Thu, Nov 13, 2008 at 12:06:36AM +0100, Samuel Ortiz wrote:
> I understand that. I'm just saying that I would prefer wm8350->read_dev() to > return the actual bytes read, be it for SPI or I2C. Same for write_dev(), of > course.
Hrm. That's never been the case, even with the buggy code since the register address was included in the physical access for at least the writes.
> With this patch you're breaking that expectation because the read|write_dev() > I'd prefer to fix the callers code, so that we keep the expected semantics
Just to clarify, when you say "expectation" and "fix" are you talking about your preference that the hardware access functions return the number of bytes read or written or is there something else going on here?
> for your read|write_dev() routines. For example with wm8350_clear_bits():
> - if (err) { > + ret = wm8350_read(wm8350, reg, 1, &data); > + if (ret != 1) { > dev_err(wm8350->dev, "read from reg R%d failed\n", reg); > + err = -EIO;
Note that wm8350_read() works a level up, only calling the physical read function if the register cache can't satisfy the access. Changing that would be another layer of alteration.
To be honest I'm really not sure it's worth changing this - I'm having a hard time thinking of a user that would be able to do anything useful with a short access. It also seems like it'd be a rather invasive change for an -rc.
| |