Messages in this thread | | | Date | Tue, 6 Aug 2019 22:51:24 +0200 | From | Wolfram Sang <> | Subject | Re: [PATCH] i2c-axxia: support slave mode |
| |
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
[unhandled content-type:application/pgp-signature] | |