Re: [PATCH 1/3] i2c: rcar: sanity check for minimal DMA length

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]

On Mon, Mar 11, 2019 at 11:08:13AM +0100, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Tue, Mar 5, 2019 at 7:52 PM Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
> > Use a macro for the hardcoded value and apply a build check. If it is
> > not met, the driver logic will not work anymore.
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >  drivers/i2c/busses/i2c-rcar.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> > index 3ce74edcd70c..925858915569 100644
> > --- a/drivers/i2c/busses/i2c-rcar.c
> > +++ b/drivers/i2c/busses/i2c-rcar.c
> 
> > @@ -921,6 +922,9 @@ static int rcar_i2c_probe(struct platform_device *pdev)
> >         struct i2c_timings i2c_t;
> >         int irq, ret;
> >
> > +       /* Otherwise logic will break because some bytes must always use PIO */
> > +       BUILD_BUG_ON_MSG(RCAR_MIN_DMA_LEN < 3, "Invalid min DMA length");
> 
> Given patch 3/3, it should still work with RCAR_MIN_DMA_LEN == 2, right?

Nope. It is not that we transfer one byte more with PIO now. The change
in patch 3 is that we explicitly wait for an interrupt when the (already
existing) PIO transfer ended. Before that patch, we assumed DMA would
take over on its own once the data register is empty again. Should I
update the commit message to make this more clear?

Also, it is the _read_ case which needs the minimum lenght of 3. This is
fixing the _write_ code path :)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]