Re: [PATCH] i2c-axxia: support slave mode

From: Adamski, Krzysztof (Nokia - PL/Wroclaw)
Date: Wed Aug 07 2019 - 03:09:39 EST


Hi Wolfram,

>Hi Krzysztof,
>
>> + 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?

Good point. I'll remove those verbose messages and maybe leave one or
two debug messages with just a summary of the status which will
hopefully be a good compromise. Will that be ok?

>
>Rest looks good from what I can tell without knowing the hardware.

It also seems to work correctly and I tried to do everything in a way
that nothing is changed if slave mode is not used to eliminate the risk
of regressions.

BTW, I have added this synchronize_irq() in unreg_slave callback just to
make sure it is save to set idev->slave to NULL already. Most of the
controllers do not have such a guard and I'm wondering why that wouldn't
be a problem for them. Like the i2c-rcar.c - isn't there a small race
condition if some slave interrupt triggers just before ICSIER is cleared
and somehow does not finish before priv->slave is set to NULL? This is
the situation I was afraid of and tried to solve by using this
synchronize_irq().

Krzysztof