linux-kernel - Re: [PATCH v5 03/21] drivers: crypto: meson: make CLK controller optional

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: <20240305134732.z6eyo4nppj7oc2su@cab-wsm-0029881.sigma.sbrf.ru>
Date: Tue, 5 Mar 2024 13:47:37 +0000
From: Alexey Romanov <avromanov@...utedevices.com>
To: Jerome Brunet <jbrunet@...libre.com>
CC: "neil.armstrong@...aro.org" <neil.armstrong@...aro.org>,
	"clabbe@...libre.com" <clabbe@...libre.com>, "herbert@...dor.apana.org.au"
	<herbert@...dor.apana.org.au>, "davem@...emloft.net" <davem@...emloft.net>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	"krzysztof.kozlowski+dt@...aro.org" <krzysztof.kozlowski+dt@...aro.org>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>, "khilman@...libre.com"
	<khilman@...libre.com>, "martin.blumenstingl@...glemail.com"
	<martin.blumenstingl@...glemail.com>, "vadim.fedorenko@...ux.dev"
	<vadim.fedorenko@...ux.dev>, "linux-crypto@...r.kernel.org"
	<linux-crypto@...r.kernel.org>, "linux-amlogic@...ts.infradead.org"
	<linux-amlogic@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, kernel <kernel@...rdevices.ru>
Subject: Re: [PATCH v5 03/21] drivers: crypto: meson: make CLK controller
 optional

On Mon, Mar 04, 2024 at 01:49:46PM +0000, Alexey Romanov wrote:
> Hello Jerome,
> 
> On Fri, Mar 01, 2024 at 04:21:20PM +0100, Jerome Brunet wrote:
> > 
> > On Fri 01 Mar 2024 at 16:29, Alexey Romanov <avromanov@...utedevices.com> wrote:
> > 
> > > Amlogic crypto IP doesn't take a clock input on some
> > > SoCs: AXG / A1 / S4 / G12. So make it optional.
> > >
> > 
> > I commented this patch on v2 and the comment keep on being un-addressed.
> > 
> > The SoC either:
> > * has a clock that is required for the IP to work
> > * Or does not
> > 
> > It is not something you are free to provide or not.
> > 
> > For the record, I find very hard believe that some SoC would have clock,
> > and other would not, for the same HW.
> > 
> > Isn't it more likely that the clock just happens to be left enabled by
> > the bootloader on some SoC and it conviently allows to ignore it ?
> 
> 
> S905X and newer SoC's uses DMA engine for crypto HW and they
> don't required clock input to work. Clock input is needed for
> blkmv engine.
> 
> Therefore, I'm not sure that it is needed for GXL too (and the second
> interrupt line). I tested it on vim1 board witouht them and everything
> works correctly.

Amlogic says that the crypto HW based on DMA engine doesn't require
a clock input. GXL uses DMA engine, so, I think we have to remove
whole clock controller calls in the next series from driver/dts/bindings.
And the second interrupt line from crypto node in meson-gxl.dtsi too.

> 
> > 
> > > Signed-off-by: Alexey Romanov <avromanov@...utedevices.com>
> > > ---
> > >  drivers/crypto/amlogic/amlogic-gxl-core.c | 14 +++-----------
> > >  1 file changed, 3 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/crypto/amlogic/amlogic-gxl-core.c b/drivers/crypto/amlogic/amlogic-gxl-core.c
> > > index e9e733ed98e0..a3a69a59f476 100644
> > > --- a/drivers/crypto/amlogic/amlogic-gxl-core.c
> > > +++ b/drivers/crypto/amlogic/amlogic-gxl-core.c
> > > @@ -269,16 +269,11 @@ static int meson_crypto_probe(struct platform_device *pdev)
> > >  		dev_err(&pdev->dev, "Cannot request MMIO err=%d\n", err);
> > >  		return err;
> > >  	}
> > > -	mc->busclk = devm_clk_get(&pdev->dev, "blkmv");
> > > +
> > > +	mc->busclk = devm_clk_get_optional_enabled(&pdev->dev, "blkmv");
> > >  	if (IS_ERR(mc->busclk)) {
> > >  		err = PTR_ERR(mc->busclk);
> > > -		dev_err(&pdev->dev, "Cannot get core clock err=%d\n", err);
> > > -		return err;
> > > -	}
> > > -
> > > -	err = clk_prepare_enable(mc->busclk);
> > > -	if (err != 0) {
> > > -		dev_err(&pdev->dev, "Cannot prepare_enable busclk\n");
> > > +		dev_err(&pdev->dev, "Cannot get and enable core clock err=%d\n", err);
> > >  		return err;
> > >  	}
> > >  
> > > @@ -306,7 +301,6 @@ static int meson_crypto_probe(struct platform_device *pdev)
> > >  	meson_unregister_algs(mc);
> > >  error_flow:
> > >  	meson_free_chanlist(mc, mc->flow_cnt - 1);
> > > -	clk_disable_unprepare(mc->busclk);
> > >  	return err;
> > >  }
> > >  
> > > @@ -321,8 +315,6 @@ static void meson_crypto_remove(struct platform_device *pdev)
> > >  	meson_unregister_algs(mc);
> > >  
> > >  	meson_free_chanlist(mc, mc->flow_cnt - 1);
> > > -
> > > -	clk_disable_unprepare(mc->busclk);
> > >  }
> > >  
> > >  static const struct meson_pdata meson_gxl_pdata = {
> > 
> > 
> > -- 
> > Jerome
> 
> -- 
> Thank you,
> Alexey

-- 
Thank you,
Alexey