Re: [PATCHv3 1/2] spi: imx: fix use of native chip-selects with devicetree

[Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>]

On Thu, Aug 10, 2017 at 09:35:15PM +1000, Greg Ungerer wrote:
> On 10/08/17 19:35, Vladimir Zapolskiy wrote:
> > Errors in DTB (or platform data) may confuse a driver and lead to runtime
> > misbehaviour. You describe an error in a board DTB, which is definitely
> > better to handle in the SPI driver, but I don't think it is strictly
> > mandatory to do it, because DTB errors are supposed to be fixed in DTB.
> > 
> > May be one day a formal check of DTBs against Documentation/devicetree
> > descriptions will be added and such DTB errors could be captured on DTB
> > compilation stage.
> 
> I completely agree with Vladmir here. Since "cs-gpios" defines the
> number of chip selects, as per the code you point out, it is the range
> limit. So if a DTB defines it wrongly then you can expect some things
> not to work right. The spi code quite rightly relies on the DTB
> definitions to be correct for proper operation.
> 
> 
> > > And in this case:
> > > cs-gpios = <&gpio1 0 0>, <&gpio1 1 0>, <&gpio1 2 0>, <&gpio1 3 0>, <0>;
> > > 
> > > we should produce a 3 bit value b100 which will be shifted left and
> > > "or"-ed with other ctrl bits.
> 
> So the register settings will be wrong and the device will not work.
> You can't really expect any other behavior from an incorrect DTB.

I think nobody expects that a wrong dtb is good enough to make
everything work. Maybe you can argue what should happen in a driver if
it gets a wrong dtb. It can make the kernel oops, just silently not work
or issue an error message; probably there are more options.

The current state is that a broken dtb makes the spi-imx driver write to
a unrelated register bit when asked to set a chip select signal. Oleksij
tries to improve that and make the driver error out instead. It makes it
easier for users to report problems or find the culprit themselves. I
like that.

I don't care much if you call it a driver bug or a dtb bug if the above
happens. For sure the error checking that Oleksij introduces isn't
mandatory, but still it improves the driver.

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html