Re: [PATCH 2.6.28] mfd: Correct WM8350 I2C return code usage

[Mark Brown <broonie@sirena.org.uk>]

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.