Re: [PATCH] spi: cadence-quadspi: Add upper limit safety check to baudrate divisor

[Mark Brown <broonie@kernel.org>]

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

On Thu, Nov 24, 2022 at 05:57:10PM +0530, Dhruva Gole wrote:
> On 24/11/22 17:05, Mark Brown wrote:

> > As far as I can tell the issue here is that the device is asking for a
> > rate which requires a larger divisor than the controller can support but
> > the driver doesn't do any bounds checking so it just writes the
> > calculated divisor out to the hardware, corrupting any adjacent fields.

> but, I am not sure it would anyway corrupt any adjacent bits,

> The code
> reg |= (div & CQSPI_REG_CONFIG_BAUD_MASK) << CQSPI_REG_CONFIG_BAUD_LSB

> does mask the div value to ensure bits ONLY in CQSPI_REG_CONFIG_BAUD_MASK
> region are set and nothing else right?

Yes, that'd avoid corrupting adjacent bits (though it'd still be making
things worse in that it makes the divider smaller).

> I believe a simple warning is enough, and better not touch the div variable
> because it seems unnecessary. We already have a mask to take care of masking
> the appropriate bits.

That'd still leave the clock driven too fast which could break things,
going for the maximum divider would mitigate this (though an error would
be even safer).

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