Re: [PATCH 2/8] spi: cadence-quadspi: Add QSPI support for Pensando Elba SoC

From: Arnd Bergmann <arnd@arndb.de>
To: Brad Larson <brad@pensando.io>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Mark Brown <broonie@kernel.org>,
	Serge Semin <fancer.lancer@gmail.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Olof Johansson <olof@lixom.net>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	linux-spi <linux-spi@vger.kernel.org>,
	linux-mmc <linux-mmc@vger.kernel.org>,
	DTML <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/8] spi: cadence-quadspi: Add QSPI support for Pensando Elba SoC
Date: Thu, 4 Mar 2021 10:29:58 +0100	[thread overview]
Message-ID: <CAK8P3a1aVfA=kp-dW+YKZ9cG=sD6+efSBYtB6GXu0X4MBjb0xg@mail.gmail.com> (raw)
In-Reply-To: <20210304034141.7062-3-brad@pensando.io>

On Thu, Mar 4, 2021 at 4:41 AM Brad Larson <brad@pensando.io> wrote:
>
> Add QSPI controller support fo Pensando Elba SoC.
>
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  drivers/spi/spi-cadence-quadspi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 442cc7c53a47..fb0d9b0bd596 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -1353,6 +1353,7 @@ static int cqspi_request_mmap_dma(struct cqspi_st *cqspi)
>         cqspi->rx_chan = dma_request_chan_by_mask(&mask);
>         if (IS_ERR(cqspi->rx_chan)) {
>                 int ret = PTR_ERR(cqspi->rx_chan);
> +
>                 cqspi->rx_chan = NULL;
>                 return dev_err_probe(&cqspi->pdev->dev, ret, "No Rx DMA available\n");
>         }

Please don't mix whitespace changes with code changes.

> @@ -1632,6 +1633,10 @@ static const struct cqspi_driver_platdata intel_lgm_qspi = {
>         .quirks = CQSPI_DISABLE_DAC_MODE,
>  };
>
> +static const struct cqspi_driver_platdata pen_cdns_qspi = {
> +       .quirks = CQSPI_NEEDS_WR_DELAY | CQSPI_DISABLE_DAC_MODE,
> +};
> +
>  static const struct of_device_id cqspi_dt_ids[] = {
>         {
>                 .compatible = "cdns,qspi-nor",
> @@ -1649,6 +1654,10 @@ static const struct of_device_id cqspi_dt_ids[] = {
>                 .compatible = "intel,lgm-qspi",
>                 .data = &intel_lgm_qspi,
>         },
> +       {
> +               .compatible = "pensando,cdns-qspi",
> +               .data = &pen_cdns_qspi,
> +       },
>         { /* end of table */ }

As mentioned in my reply to the dts file, the compatible string needs to be
somewhat more specific.

I also wonder if it would be better to define separate DT properties for the
quirks at this point, so not every new SoC using this device needs to have
its own quirks definition.

       Arnd