Re: [PATCH v4 2/2] spi: microchip-core: use XOR instead of ANDNOT to simplify the logic

[Andy Shevchenko]

On Fri, Nov 28, 2025 at 08:30:43PM +0100, Jonas Gorski wrote:
> On Fri, Nov 28, 2025 at 7:56 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

...

> > -       if (spi->mode & SPI_MODE_X_MASK & ~spi->controller->mode_bits) {
> > +       if ((spi->mode ^ spi->controller->mode_bits) & SPI_MODE_X_MASK) {
> 
> This changes the behavior: if a bit isn't set in spi->mode that is set
> in mode_bits, it would have been previously accepted, now it's
> refused. E.g. controller has (SPI_CPOL | SPI_CPHA), device only
> SPI_CPOL. 0x1 & 0x3 & ~0x3 => 0, vs (0x1 ^ 0x3) & 0x3 => 0x2
> 
> If this is the actually intended behavior here, it is a fix and should
> carry a Fixes tag (the message below implies that).

Yeah, yesterday I was thinking about the same and I was confused by the logic
behind. As far as I understood the comments regarding mode provided by DT is
that the mode is configured in IP and may not be changed. And you are right
about the fix, but let's wait for Microchip to elaborate on the expected
behaviour.

> >                 dev_err(&spi->dev, "incompatible CPOL/CPHA, must match controller's Motorola mode\n");
> >                 return -EINVAL;
> >         }

Thanks for the review!

-- 
With Best Regards,
Andy Shevchenko