Hi Krzysztof,
> This device contains both master and slave controllers which can be
> enabled simultaneously. Both controllers share the same SDA/SCL lines
> and interrupt source but has separate control and status registers.
> Controllers also works in loopback mode - slave device can communicate
> with its own master controller internally. The controller can handle up
Cool, I never got this to work with my hardware. I always had to wire
two controllers together,
> to two addresses, both of which may be 10 bit. Most of the logic
> (sending (N)ACK, handling repeated start or switching between
> write/read) is handled automatically which makes working with this
> controller quite easy.
Yes, looks pretty straightforward. Nice!
> For simplicity, this patch adds basic support, limiting to only one
> slave address. Support for the 2nd device may be added in the future.
Fine with me. Incremental additions are easier to review.
> Note that checkpatch shows warnings about "line over 80 characters" for
> some of those register definitions added but I personally think
> splitting those comments would decrease readability, not increase it. I
> can do that, however, if you think otherwise.
I am fine with that, too.
> + if (fifo_status & SLV_FIFO_DV1) {
> + if (fifo_status & SLV_FIFO_STRC) {
> + dev_dbg(dev, "First data byte sent\n");
I think, however, these debug messages could go. They were surely
helpful during development but assuming things work now, they will not
help backend authors. Can you agree?
Rest looks good from what I can tell without knowing the hardware.
Thanks,
Wolfram
Attachment:
signature.asc
Description: PGP signature
