Hi Andrew, thanks for your patience, I can finally work on this issue. > > You could wire up two R-Car I2C instances, set up one as an I2C slave > > handled by the I2C testunit and then use the other instance with > > SMBUS_BLOCK_PROC_CALL which also needs RECV_LEN. Check > > Documentation/i2c/slave-testunit-backend.rst for details. > > You mean physical connection of two R-Car boards via I2C bus, > or physical connection of I2C bus wires on the single board, right? I have two instances on the same board wired. > It looks like all the boards, that I have access to, do not have > I2C bus wires exposed to some connectors, so both variants would > require hardware re-wiring modification of the boards, which is > not an option for me. Or do I understand you incorrectly and you > mean something different? Probably you understood correctly. Which boards do you have access to? I use the EXIO connectors on a Salvator-X(S). > Most of complexity in my patch is related to DMA transfers support, > that I'm trying to retain for SMBus block data transfers too (for the rest > of bytes after the first "length" byte). Your simple patch makes > the driver perform all M_RECV_LEN transfers in PIO mode only (with no DMA at all), > which is probably not quite good (it's a pity to loose existing HW capability, > already supported by the driver). I will have a look into RECV_LEN and DMA. I already started looking into it but will need to dive in some more. Stay tuned, I hope to have the next response ready this week. > > /* If next received data is the _LAST_, go to new phase. */ > > - if (priv->pos + 1 == msg->len) { > > + if (priv->pos + 1 == msg->len && !recv_len_init) { > > If a message contains a single byte after the length byte, > when we come here after processing the length (in the same function call), > "pos" is 1, "len" is 2, and we indeed are going to process the last byte. > However, "recv_len_init" is still "true", and we skip these corresponding > register writes, which is probably incorrect. > The flag in this case should be re-set back to "false" after length > processing and "pos" moving, but I think the variant in my patch > (leaving this "if" unchanged, but skipping it on the first pass with "goto") > may be even simpler. I also need to look into this but thank you already for the detailed explanation! > > u32 func = I2C_FUNC_I2C | I2C_FUNC_SLAVE | > > - (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK); > > + (I2C_FUNC_SMBUS_EMUL_ALL & ~I2C_FUNC_SMBUS_QUICK); > > This flags setting adds also I2C_FUNC_SMBUS_BLOCK_PROC_CALL flag, > which is missed in my patch. My patch should probably be updated > to include it too (if you'll agree to take my variant ;-) ). Yes, the final version, whatever it will be, should use this new macro. Until soon, Wolfram
Attachment:
signature.asc
Description: PGP signature