On Fri, Jul 27, 2018 at 3:05 PM, Alexandre Belloni
<alexandre.belloni@xxxxxxxxxxx> wrote:
> Because the SPI controller deasserts the chip select when the TX fifo is
> empty (which may happen in the middle of a transfer), the CS should be
> handled by linux. Unfortunately, some or all of the first four chip
> selects are not muxable as GPIOs, depending on the SoC.
>
> There is a way to bitbang those pins by using the SPI boot controller so
> use it to set the chip selects.
>
> At init time, it is also necessary to give control of the SPI interface to
> the Designware IP.
Thanks for an update! My comments below (most of them just about style).
First of all, can we use 'controller' over the 'master' in the code?
> .../bindings/spi/snps,dw-apb-ssi.txt | 5 +-
> Required properties:
> -- compatible : "snps,dw-apb-ssi" or "mscc,<soc>-spi"
> -- reg : The register base for the controller. For "mscc,<soc>-spi", a second
> - register set is required (named ICPU_CFG:SPI_MST)
> +- compatible : "snps,dw-apb-ssi"
> +- reg : The register base for the controller.
This hunk is odd.
> struct dw_spi_mmio {
> struct dw_spi dws;
> struct clk *clk;
> + void *priv;
> };
> +#define MSCC_SPI_MST_SW_MODE 0x14
> +#define MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE BIT(13)
> +#define MSCC_SPI_MST_SW_MODE_SW_SPI_CS(x) (x << 5)
+ blank line ?
> +struct dw_spi_mscc {
> + struct regmap *syscon;
> + void __iomem *spi_mst;
A nit: do we need to repeat "spi" here?
> +};
> + if (!enable)
> + dw_writel(dws, DW_SPI_SER, BIT(cs));
This sounds like
dw_set_cs(spi, enable);
> + dwsmscc = devm_kzalloc(&pdev->dev, sizeof(struct dw_spi_mscc),
> + GFP_KERNEL);
sizeof(*dwsmcc) and one line in the result?
> + /* Deassert all CS */
> + writel(0, dwsmscc->spi_mst + MSCC_SPI_MST_SW_MODE);
Hmm... Don't we need to call dw_set_cs() for all of them as well?
If yes, perhaps better to call custom set_cs() in a loop?
--
With Best Regards,
Andy Shevchenko
