linux-kernel - Re: [PATCH 2/2] spi: atcspi200: Add ATCSPI200 SPI driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aSAyoP3ohz73sd1U@swlinux02>
Date: Fri, 21 Nov 2025 17:36:32 +0800
From: CL Wang <cl634@...estech.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
CC: <broonie@...nel.org>, <linux-spi@...r.kernel.org>, <robh@...nel.org>,
        <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <tim609@...estech.com>, <cl634@...estech.com>
Subject: Re: [PATCH 2/2] spi: atcspi200: Add ATCSPI200 SPI driver

Hi Krzysztof,

Thanks for your review, and please see my responses inline.
> > +     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.
You are right — the error paths miss the corresponding
clk_disable_unprepare() cleanup. I will update the probe logic to ensure
the clock is properly disabled along all failure paths

> > +
> > +free_controller:
> > +     spi_controller_put(host);
> 
> Where is DMA channel release? Same for unbind path.
Thanks for pointing this out. To ensure proper cleanup in both the probe
error path and a potential future remove() implementation, I will switch to
devm_dma_request_chan(), so that DMA channels are automatically released
by the device core.

> > +static const struct of_device_id atcspi_of_match[] = {
> > +     { .compatible = "andestech,qilai-spi", },
> > +     { .compatible = "andestech,atcspi200", },
Understood. I will fix the compatible strings and ensure all of them are
properly documented in the binding.

Thanks again for your review and suggestions.

Best regards,
CL