[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <73a39371-5bf0-4a3d-a48b-9e91668b779c@kernel.org>
Date: Fri, 14 Nov 2025 12:58:41 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: CL Wang <cl634@...estech.com>, broonie@...nel.org,
linux-spi@...r.kernel.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
tim609@...estech.com
Subject: Re: [PATCH 2/2] spi: atcspi200: Add ATCSPI200 SPI driver
On 12/11/2025 04:47, CL Wang wrote:
> +
> +static int atcspi_enable_clk(struct atcspi_dev *spi)
> +{
> + int ret;
> +
> + ret = clk_prepare_enable(spi->clk);
> + if (ret)
> + return dev_err_probe(spi->dev, ret,
> + "Failed to enable clock\n");
> +
> + spi->clk_rate = clk_get_rate(spi->clk);
> + if (!spi->clk_rate)
> + return dev_err_probe(spi->dev, -EINVAL,
> + "Failed to get SPI clock rate\n");
You miss clock enable/prepare cleanup. In other places as well.
> +
> + return 0;
> +}
> +
> +static void atcspi_init_controller(struct platform_device *pdev,
> + struct atcspi_dev *spi,
> + struct spi_controller *host,
> + struct resource *mem_res)
> +{
> + /* Get the physical address of the data register for DMA transfers. */
> + spi->dma_addr = (dma_addr_t)(mem_res->start + ATCSPI_DATA);
> +
> + /* Initialize controller properties */
> + host->bus_num = pdev->id;
> + host->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_QUAD | SPI_TX_QUAD;
> + host->dev.of_node = pdev->dev.of_node;
> + host->num_chipselect = ATCSPI_MAX_CS_NUM;
> + host->mem_ops = &atcspi_mem_ops;
> + host->max_speed_hz = spi->sclk_rate;
> +}
> +
> +static int atcspi_probe(struct platform_device *pdev)
> +{
> + struct spi_controller *host;
> + struct atcspi_dev *spi;
> + struct resource *mem_res;
> + int ret;
> +
> + host = spi_alloc_host(&pdev->dev, sizeof(*spi));
> + if (!host)
> + return -ENOMEM;
> +
> + spi = spi_controller_get_devdata(host);
> + spi->host = host;
> + spi->dev = &pdev->dev;
> + platform_set_drvdata(pdev, host);
> +
> + ret = atcspi_init_resources(pdev, spi, &mem_res);
> + if (ret)
> + goto free_controller;
> +
> + ret = atcspi_enable_clk(spi);
> + if (ret)
> + goto free_controller;
> +
> + atcspi_init_controller(pdev, spi, host, mem_res);
> +
> + ret = atcspi_setup(spi);
> + if (ret)
> + goto free_controller;
> +
> + ret = devm_spi_register_controller(&pdev->dev, host);
> + if (ret) {
> + dev_err_probe(spi->dev, ret,
> + "Failed to register SPI controller\n");
> + goto free_controller;
> + }
> +
> + spi->use_dma = false;
> + if (ATCSPI_DMA_SUPPORT) {
> + ret = atcspi_configure_dma(spi);
> + if (ret)
> + dev_info(spi->dev,
> + "Failed to init DMA, fallback to PIO mode\n");
> + else
> + spi->use_dma = true;
> + }
> + mutex_init(&spi->mutex_lock);
> +
> + return 0;
> +
> +free_controller:
> + spi_controller_put(host);
Where is DMA channel release? Same for unbind path.
> + return ret;
> +}
> +
> +static const struct of_device_id atcspi_of_match[] = {
> + { .compatible = "andestech,qilai-spi", },
> + { .compatible = "andestech,atcspi200", },
Where did you document this compatible/ABI?
Best regards,
Krzysztof
Powered by blists - more mailing lists