linux-kernel - Re: [PATCH 2/2] spi: microchip-core-qspi: Add support for microchip fpga qspi controllers

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <3ec68df2-7843-abc9-c490-29cbbac80edc@microchip.com>
Date:   Mon, 1 Aug 2022 12:56:26 +0000
From:   <Conor.Dooley@...rochip.com>
To:     <broonie@...nel.org>
CC:     <Nagasuresh.Relli@...rochip.com>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <linux-spi@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] spi: microchip-core-qspi: Add support for microchip
 fpga qspi controllers

On 01/08/2022 13:33, Mark Brown wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> On Mon, Aug 01, 2022 at 10:40:33AM +0000, Conor.Dooley@...rochip.com wrote:
>> On 01/08/2022 10:42, Naga Sureshkumar Relli wrote:
>
>>> +	qspi = spi_controller_get_devdata(ctlr);
>
>> Is there a reason to use spi_controller_get_devdata() rather than
>> spi_master_get_devdata() ?
>
> We are trying to move everything away from the old terminonlogy to using
> controller, this also applies to the the other API functions.
>

Right, makes sense.

>>> +clk_dis_all:
>>> +	clk_disable_unprepare(qspi->clk);
>
>> If we move the clk prepare & enable later in probe, ie after
>> getting the irq, this goto could be removed too because the
>> only place requiring teardown of the clock would be the error
>> path of devm_spi_register_controller().
>
> Is the clock needed for handling of the interrupt?

I guess I was thinking in terms of the "hard" controller. In that case,
without a clock you're not going to get an interrupt in the first place
since it is not a shared line (nor a shared clock). But that is not the
case for the "soft" controller, so keeping the clk_prepare_enable()
prior to registering the interrupt is a good idea. Sorry for the noise
Suresh!

That did make me check the flags for devm_request_irq(), which needs to
be changed to support shared IRQs. The handler already checks to make
sure that it's its interrupt so we just need that small change in probe.

Thanks,
Conor