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

[Prajna Rajendra Kumar]

On 01/12/2025 17:57, Andy Shevchenko wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Mon, Dec 01, 2025 at 04:08:57PM +0000, Conor Dooley wrote:
> > On Sat, Nov 29, 2025 at 10:19:00AM +0200, Andy Shevchenko wrote:
> > > 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.
> > Prajna is on holiday and I don't have a setup to actually test this on,
> > but I'm 99% sure that you're both right and the original behaviour was
> > wrong. There's a verilog parameter to the IP block that determines which
> > motorola mode it is and a device that's not an exact match won't work.
> Okay, let's not hurry up with this and wait for testing results.
>
> > FWIW:
> > Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > > > >                  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
Hi,

I've tested this on my setup and XOR check matches how the controller
behaves. The SPI mode is fixed in hardware, so the previous logic was
wrong.

Tested-by: Prajna Rajendra Kumar <prajna.rajendrakumar@xxxxxxxxxxxxx>