[RFC PATCH v2 2/2] drm/bridge: parade-ps8640: Add support for AUX channel

[Doug Anderson]

Hi,

On Tue, Sep 14, 2021 at 5:28 PM Philip Chen <philipchen at chromium.org> wrote:
>
> > > Changes in v2:
> > > - Handle the case where an AUX transaction has no payload
> > > - Add a reg polling for p0.0x83 to confirm AUX cmd is issued and
> > >   read data is returned
> > > - Replace regmap_noinc_read/write with looped regmap_read/write,
> > >   as regmap_noinc_read/write doesn't read one byte at a time unless
> > >   max_raw_read/write is set to 1.
> >
> > What about if you set val_bytes? I think you just need to set that to
> > "1" and it'll work?
> I think val_bytes is already set to 1 as we set val_bits to 8. See:
> map->format.val_bytes = DIV_ROUND_UP(config->val_bits, 8);

To me that feels like a bug in the regmap API, then. I can't see how
it would make any sense for this function not to take val_bytes into
account...

I wonder if other users are somehow getting lucky today. Maybe users
that are using this for MMIO get lucky because max_raw_read is set
properly. ...and maybe other i2c users get lucky because some
peripherals are OK w/ this bug? AKA, maybe this actually works in most
cases for FIFOs:

write address of bridge chip on i2c bus
write R/W bit on i2c bus
write FIFO register address on i2c bus
read byte
read byte
read byte
...
read byte
read byte
end transaction

Normally for i2c you assume that the other side will read from
subsequent register addresses for each "read byte", but I suppose it's
possible that some i2c devices are setup to realize that if the
register address was the address of a FIFO that it shouldn't read from
the next register address but should just read the next byte in the
FIFO?


In any case, it's fine to do it with a loop like you're doing but it
still seems weird that you'd need to.


-Doug