Re: [PATCH linux v3 3/6] hwmon: occ: Add I2C transport implementation for SCOM operations

[Benjamin Herrenschmidt]

On Sat, 2017-01-21 at 10:11 -0800, Guenter Roeck wrote:
> > +int occ_i2c_getscom(void *bus, u32 address, u64 *data)
> > +{
> > +ÂÂÂÂÂssize_t rc;
> > +ÂÂÂÂÂu64 buf;
> > +ÂÂÂÂÂstruct i2c_client *client = bus;
> > +
> > +ÂÂÂÂÂrc = i2c_master_send(client, (const char *)&address,
> > sizeof(u32));
> > +ÂÂÂÂÂif (rc < 0)
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂreturn rc;
> > +ÂÂÂÂÂelse if (rc != sizeof(u32))
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂreturn -EIO;
> > +
> > +ÂÂÂÂÂrc = i2c_master_recv(client, (char *)&buf, sizeof(u64));
> > +ÂÂÂÂÂif (rc < 0)
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂreturn rc;
> > +ÂÂÂÂÂelse if (rc != sizeof(u64))
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂreturn -EIO;
> > +
> > +ÂÂÂÂÂ*data = be64_to_cpu(buf);
> > +
> > +ÂÂÂÂÂreturn 0;
> > +}
> > +EXPORT_SYMBOL(occ_i2c_getscom);

Additionally, this assumes that is is the only other user of that i2c
path to P8. Something interleaving will break it. pdbg for example.

Talk to Alistair, the right thing here would probably be to have
a separate driver that provides the XSCOM interface via i2c to both
in-kernel and userspace, with proper arbitration.

An expedient might be to instead have the address and data cycles
of the above be 2 i2c messages in one i2c request, thus being
serialized somewhat atomically with other transactions to that i2c
bus.

You may need to make sure the underlying i2c controller driver supports
dual messages, and if it doesn't fix it. This is rather classic 4-bytes 
subaddress style i2c, it shouldn't be too hard.

Cheers,
Ben.