Re: [PATCH 3/3 v2] spi: s3c64xx: Convert to use GPIO descriptors

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Tue, Jan 18, 2022 at 4:27 PM Sam Protsenko
<semen.protsenko@xxxxxxxxxx> wrote:

> > +       if (!cs)
> > +               return 0;
>
> Shouldn't some error code be returned in that case? Or is it normal case?

No, because it just means the platform didn't define any fb_delay.

> >         /* Configure feedback delay */
> >         writel(cs->fb_delay & 0x3, sdd->regs + S3C64XX_SPI_FB_CLK);

But maybe I should always write 0 into this then.

> > - * This is per SPI-Slave Chipselect information.
> > + * This is per SPI-Slave FB delay information.
>
> Not sure if this change is needed: this comments will be incorrect if
> some new fields are added in future. For example, downstream trees
> like [1] have more fields there, not only feedback delay one.

OK I fix

> Other than minor comments above:
>
> Reviewed-by: Sam Protsenko <semen.protsenko@xxxxxxxxxx>
>
> Btw, this SPI driver is used in Exynos850 SoC, which I'm working on
> right now. It's the shame I didn't enable it yet on my board, so I
> can't test the code. But I'm happy to see it's being modernized, kudos
> for doing that!

Yeah I saw it being used in the Tesla SoC and also Exynos850 so as
I now know this is not going away any time soon we might as well just
fix it up.

Yours,
Linus Walleij