Re: [PATCH v2 2/4] spi: dw-mmio: add MSCC Ocelot support

From: Andy Shevchenko
Date: Fri Jul 27 2018 - 09:07:59 EST


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