On Fri, 21 Jan 2022 at 19:26, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
>
> On Fri, Jan 21, 2022 at 1:52 PM Sam Protsenko
> <semen.protsenko@xxxxxxxxxx> wrote:
> > On Wed, 19 Jan 2022 at 01:11, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> > >
> > > Convert the S3C64xx SPI host to use GPIO descriptors.
> > >
> > > Provide GPIO descriptor tables for the one user with CS
> > > 0 and 1.
>
> ...
>
> > > /* Configure feedback delay */
> > > - writel(cs->fb_delay & 0x3, sdd->regs + S3C64XX_SPI_FB_CLK);
> > > + if (!cs)
> > > + /* No delay if not defined */
> > > + writel(0, sdd->regs + S3C64XX_SPI_FB_CLK);
> > > + else
> > > + writel(cs->fb_delay & 0x3, sdd->regs + S3C64XX_SPI_FB_CLK);
> >
> > Looks good to me. I'd add {} braces and change "if (!cs)" to "if
> > (cs)", but that's hair splitting and not worth v4, it's fine as it is.
>
> If you are going to change code, then why not use positive conditions?
>
> if (cs)
> ...
> else {
> ...
> }
>
It was already pointed out earlier AFAIR. Personally I don't think
it's too major to send v4. But theoretically speaking, maybe it would
be even better this way:
u32 fb_delay = cs ? cs->fb_delay & 0x3 : 0;
/* Configure feedback delay */
writel(fb_delay, sdd->regs + S3C64XX_SPI_FB_CLK);
Again, I think it's not worth another submission, the same code will
be generated, and style is ok as it is.
> --
> With Best Regards,
> Andy Shevchenko
