Re: [PATCH v2 3/6] spi: dw: Add Microchip Sparx5 support

Mark Brown writes:

On Fri, Jun 19, 2020 at 01:31:18PM +0200, Lars Povlsen wrote:

quoted

+/*
+ * The Designware SPI controller (referred to as master in the
+ * documentation) automatically deasserts chip select when the tx fifo
+ * is empty. The chip selects then needs to be driven by a CS override
+ * register. enable is an active low signal.
+ */
+static void dw_spi_sparx5_set_cs(struct spi_device *spi, bool nEnable)

The value that is passed in here is the value that should be driven on
the output pin, the driver should not be interpreting the value in any
way here.  Documenting it as nEnable adds a layer of confusion, and it
may not be an active high signal depending on the system.

Ok, I will make the CS function more like the others.

quoted

+	if (!nEnable) {
+		/* Ensure CS toggles, so start off all disabled */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_VAL, ~0);
+		/* CS override drive enable */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_ENA, 1);

This should just be setting the value to whatever the core asked for it
to be set to, the driver adding extra toggles is likely to disrupt
things.

I will have a look at this again. But it was added for a reason. The
issue is that we have two different busses in front of the controller,
so we might need more settle time when switching interface.

Thank you for you comments,

Cheers

-- 
Lars Povlsen,
Microchip