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@xxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxx>
> > > ---
> > > 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
