[Pdbg] [PATCH 2/2] i2c: Add FORCE option

Alistair Popple alistair at popple.id.au
Fri Jun 15 14:45:22 AEST 2018


I've already taken the first patch in this series but I'm not sure about this
one, comments below.

On Wednesday, 13 June 2018 4:10:54 PM AEST Joel Stanley wrote:
> On P8 machines with OpenBMC, the i2c address is claimed by the OCC hwmon
> driver. We can ignore it and talk to the device anyway by passing
> I2C_SLAVE_FORCE instead of I2C_SLAVE to the open ioctl.
> 
> This might not be the best idea, but it works fine for eg. getscom.

Probably not :-)

At least I'd like to see some kind of warning to the user that we might be
breaking their I2C/OCC. Perhaps something along the lines of attempting a normal
I2C_SLAVE ioctl and if that fails print a warning that you should unbind the
hwmon driver before continuing with I2C_SLAVE_FORCE anyway. Or a --force flag but I'd
be happy enough with just a warning prior to breaking things.

- Alistair

> Signed-off-by: Joel Stanley <joel at jms.id.au>
> --
> We could put this behind a --force option.
> 
> Also, when the user does not pass --force, we could advise them to
> unbind the hwmon driver. I don't know what OpenBMC userspace does when
> you unbind it - it may decide to shut down the machine.
> 
> Signed-off-by: Joel Stanley <joel at jms.id.au>
> ---
>  libpdbg/i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libpdbg/i2c.c b/libpdbg/i2c.c
> index b1580e176023..aa0a73ef8c19 100644
> --- a/libpdbg/i2c.c
> +++ b/libpdbg/i2c.c
> @@ -34,7 +34,7 @@ struct i2c_data {
>  
>  static int i2c_set_addr(int fd, int addr)
>  {
> -	if (ioctl(fd, I2C_SLAVE, addr) < 0) {
> +	if (ioctl(fd, I2C_SLAVE_FORCE, addr) < 0) {
>  		PR_ERROR("Unable to set i2c slave address\n");
>  		return -1;
>  	}
> 




More information about the Pdbg mailing list