Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 01/21/2017
02:53:17 PM:
> 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.

Agreed, no one has experienced any issues, but it is a hole that should be
addressed. I just tested with i2c_transfer instead of send and receive and
it seems to work fine. I'll include that in the next patch set.

Thanks,
Eddie

>
> Cheers,
> Ben.
>