On Tue, 28 May 2019 at 11:27, Masahisa Kojima
<masahisa.kojima@xxxxxxxxxx> wrote:
>
> This patch adds support for controller found on synquacer platforms.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@xxxxxxxxxx>
> Signed-off-by: Jassi Brar <jaswinder.singh@xxxxxxxxxx>
> ---
> drivers/spi/Kconfig | 11 +
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-synquacer.c | 826 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 838 insertions(+)
> create mode 100644 drivers/spi/spi-synquacer.c
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 0fba8f400c59..24a3eac72d12 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -732,6 +732,17 @@ config SPI_SUN6I
> help
> This enables using the SPI controller on the Allwinner A31 SoCs.
>
> +config SPI_SYNQUACER
> + tristate "Socionext's Synquacer HighSpeed SPI controller"
> + depends on ARCH_SYNQUACER || COMPILE_TEST
> + select SPI_BITBANG
Do we really need this dependency?
> + help
> + SPI driver for Socionext's High speed SPI controller which provides
> + various operating modes for interfacing to serial peripheral devices
> + that use the de-facto standard SPI protocol.
> +
> + It also supports the new dual-bit and quad-bit SPI protocol.
> +
> config SPI_MXIC
> tristate "Macronix MX25F0A SPI controller"
> depends on SPI_MASTER
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index f2f78d03dc28..63dcab552bcb 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -106,6 +106,7 @@ obj-$(CONFIG_SPI_STM32_QSPI) += spi-stm32-qspi.o
> obj-$(CONFIG_SPI_ST_SSC4) += spi-st-ssc4.o
> obj-$(CONFIG_SPI_SUN4I) += spi-sun4i.o
> obj-$(CONFIG_SPI_SUN6I) += spi-sun6i.o
> +obj-$(CONFIG_SPI_SYNQUACER) += spi-synquacer.o
> obj-$(CONFIG_SPI_TEGRA114) += spi-tegra114.o
> obj-$(CONFIG_SPI_TEGRA20_SFLASH) += spi-tegra20-sflash.o
> obj-$(CONFIG_SPI_TEGRA20_SLINK) += spi-tegra20-slink.o
> diff --git a/drivers/spi/spi-synquacer.c b/drivers/spi/spi-synquacer.c
> new file mode 100644
> index 000000000000..27a9babaffc0
> --- /dev/null
> +++ b/drivers/spi/spi-synquacer.c
> @@ -0,0 +1,826 @@
...
> +static int synquacer_spi_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct spi_master *master;
> + struct synquacer_spi *sspi;
> + int ret;
> + int rx_irq, tx_irq;
> +
> + master = spi_alloc_master(&pdev->dev, sizeof(*sspi));
> + if (!master)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, master);
> +
> + sspi = spi_master_get_devdata(master);
> + sspi->dev = &pdev->dev;
> +
> + init_completion(&sspi->transfer_done);
> +
> + sspi->regs = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(sspi->regs)) {
> + ret = PTR_ERR(sspi->regs);
> + goto put_spi;
> + }
> +
> + sspi->clk_src_type = SYNQUACER_HSSPI_CLOCK_SRC_IHCLK; /* Default */
> + device_property_read_u32(&pdev->dev, "socionext,ihclk-rate",
> + &master->max_speed_hz); /* for ACPI */
> +
> + if (dev_of_node(&pdev->dev)) {
> + if (device_property_match_string(&pdev->dev,
> + "clock-names", "iHCLK") >= 0) {
> + sspi->clk_src_type = SYNQUACER_HSSPI_CLOCK_SRC_IHCLK;
> + sspi->clk = devm_clk_get(sspi->dev, "iHCLK");
> + } else if (device_property_match_string(&pdev->dev,
> + "clock-names", "iPCLK") >= 0) {
> + sspi->clk_src_type = SYNQUACER_HSSPI_CLOCK_SRC_IPCLK;
> + sspi->clk = devm_clk_get(sspi->dev, "iPCLK");
> + } else {
> + dev_err(&pdev->dev, "specified wrong clock source\n");
> + ret = -EINVAL;
> + goto put_spi;
> + }
> + if (IS_ERR(sspi->clk)) {
You could have received an -EPROBE_DEFER return value here, in which
case you should not print an error and just return.
> + dev_err(&pdev->dev, "clock not found\n");
> + ret = PTR_ERR(sspi->clk);
> + goto put_spi;
> + }
> +
> + ret = clk_prepare_enable(sspi->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to enable clock (%d)\n",
> + ret);
> + goto put_spi;
> + }
> +
> + master->max_speed_hz = clk_get_rate(sspi->clk);
> + }
> +
> + if (!master->max_speed_hz) {
> + dev_err(&pdev->dev, "missing clock source\n");
> + return -EINVAL;
> + }
> + master->min_speed_hz = master->max_speed_hz / 254;
> +
> + sspi->aces = device_property_read_bool(&pdev->dev,
> + "socionext,set-aces");
> + sspi->rtm = device_property_read_bool(&pdev->dev, "socionext,use-rtm");
> +
> + master->num_chipselect = SYNQUACER_HSSPI_NUM_CHIP_SELECT;
> +
> + rx_irq = platform_get_irq(pdev, 0);
> + if (rx_irq < 0) {
<= 0
> + dev_err(&pdev->dev, "get rx_irq failed (%d)\n", rx_irq);
> + ret = rx_irq;
> + goto put_spi;
> + }
> + snprintf(sspi->rx_irq_name, SYNQUACER_HSSPI_IRQ_NAME_MAX, "%s-%s",
Why not just use "%s-rx" as the format string?
> + dev_name(&pdev->dev), "rx");
> + ret = devm_request_irq(&pdev->dev, rx_irq, sq_spi_rx_handler,
> + 0, sspi->rx_irq_name, sspi);
> + if (ret) {
> + dev_err(&pdev->dev, "request rx_irq failed (%d)\n", ret);
> + goto put_spi;
> + }
> +
> + tx_irq = platform_get_irq(pdev, 1);
> + if (tx_irq < 0) {
<= 0
> + dev_err(&pdev->dev, "get tx_irq failed (%d)\n", tx_irq);
> + ret = tx_irq;
> + goto put_spi;
> + }
> + snprintf(sspi->tx_irq_name, SYNQUACER_HSSPI_IRQ_NAME_MAX, "%s-%s",
Likewise
> + dev_name(&pdev->dev), "tx");
> + ret = devm_request_irq(&pdev->dev, tx_irq, sq_spi_tx_handler,
> + 0, sspi->tx_irq_name, sspi);
> + if (ret) {
> + dev_err(&pdev->dev, "request tx_irq failed (%d)\n", ret);
> + goto put_spi;
> + }
> +
> + master->dev.of_node = np;
please add
master->dev.fwnode = pdev->dev.fwnode;
here as well
> + master->auto_runtime_pm = true;
> + master->bus_num = pdev->id;
> +
> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_TX_DUAL | SPI_RX_DUAL |
> + SPI_TX_QUAD | SPI_RX_QUAD;
> + master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(24) |
> + SPI_BPW_MASK(16) | SPI_BPW_MASK(8);
> +
> + master->set_cs = synquacer_spi_set_cs;
> + master->transfer_one = synquacer_spi_transfer_one;
> +
> + ret = synquacer_spi_enable(master);
> + if (ret)
> + goto fail_enable;
> +
> + pm_runtime_set_active(sspi->dev);
> + pm_runtime_enable(sspi->dev);
> +
> + ret = devm_spi_register_master(sspi->dev, master);
> + if (ret)
> + goto disable_pm;
> +
> + return 0;
> +
> +disable_pm:
> + pm_runtime_disable(sspi->dev);
> +fail_enable:
> + if (!IS_ERR(sspi->clk))
You can drop this check: clk_disable_unprepare() already ignores NULL
or ERR values.
> + clk_disable_unprepare(sspi->clk);
> +put_spi:
> + spi_master_put(master);
> +
> + return ret;
> +}
> +
> +static int synquacer_spi_remove(struct platform_device *pdev)
> +{
> + struct spi_master *master = platform_get_drvdata(pdev);
> + struct synquacer_spi *sspi = spi_master_get_devdata(master);
> +
> + pm_runtime_disable(sspi->dev);
> +
> + if (!IS_ERR(sspi->clk))
> + clk_disable_unprepare(sspi->clk);
> +
Same here
> + return 0;
> +}
> +
> +static int __maybe_unused synquacer_spi_suspend(struct device *dev)
> +{
> + struct spi_master *master = dev_get_drvdata(dev);
> + struct synquacer_spi *sspi = spi_master_get_devdata(master);
> + int ret;
> +
> + ret = spi_master_suspend(master);
> + if (ret)
> + return ret;
> +
> + if (!pm_runtime_suspended(dev))
> + if (!IS_ERR(sspi->clk))
> + clk_disable_unprepare(sspi->clk);
> +
... and here
> + return ret;
> +}
> +
> +static int __maybe_unused synquacer_spi_resume(struct device *dev)
> +{
> + struct spi_master *master = dev_get_drvdata(dev);
> + struct synquacer_spi *sspi = spi_master_get_devdata(master);
> + int ret;
> +
> + if (!pm_runtime_suspended(dev)) {
> + /* Ensure reconfigure during next xfer */
> + sspi->speed = 0;
> +
> + if (!IS_ERR(sspi->clk)) {
... and here
> + ret = clk_prepare_enable(sspi->clk);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable clk (%d)\n",
> + ret);
> + return ret;
> + }
> + }
> +
> + ret = synquacer_spi_enable(master);
> + if (ret) {
> + dev_err(dev, "failed to enable spi (%d)\n", ret);
> + return ret;
> + }
> + }
> +
> + ret = spi_master_resume(master);
> + if (ret < 0) {
> + if (!IS_ERR(sspi->clk))
> + clk_disable_unprepare(sspi->clk);
.. and here
> + }
> +
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(synquacer_spi_pm_ops, synquacer_spi_suspend,
> + synquacer_spi_resume);
> +
> +static const struct of_device_id synquacer_spi_of_match[] = {
> + {.compatible = "socionext,synquacer-spi"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, synquacer_spi_of_match);
> +
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id synquacer_hsspi_acpi_ids[] = {
> + { "SCX0004" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(acpi, synquacer_hsspi_acpi_ids);
> +#endif
> +
> +static struct platform_driver synquacer_spi_driver = {
> + .driver = {
> + .name = "synquacer-spi",
> + .pm = &synquacer_spi_pm_ops,
> + .of_match_table = synquacer_spi_of_match,
> + .acpi_match_table = ACPI_PTR(synquacer_hsspi_acpi_ids),
> + },
> + .probe = synquacer_spi_probe,
> + .remove = synquacer_spi_remove,
> +};
> +module_platform_driver(synquacer_spi_driver);
> +
> +MODULE_DESCRIPTION("Socionext Synquacer HS-SPI controller driver");
> +MODULE_AUTHOR("Masahisa Kojima <masahisa.kojima@xxxxxxxxxx>");
> +MODULE_AUTHOR("Jassi Brar <jaswinder.singh@xxxxxxxxxx>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.14.2
>
With the changes above,
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
but someone else has to review the SPI bits.
