linux-kernel - Re: [PATCH 2/2] drivers: misc: adi-axi-tdd: Add new TDD engine 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <30b7dd4107285c33339e63478a100890de72d346.camel@gmail.com>
Date:   Thu, 27 Jul 2023 08:40:29 +0200
From:   Nuno Sá <noname.nuno@...il.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Eliza Balas <eliza.balas@...log.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Derek Kiernan <derek.kiernan@....com>,
        Dragan Cvetic <dragan.cvetic@....com>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/2] drivers: misc: adi-axi-tdd: Add new TDD engine
 driver

On Wed, 2023-07-26 at 20:39 +0200, Krzysztof Kozlowski wrote:
> On 26/07/2023 09:11, Eliza Balas wrote:
> > This patch introduces the driver for the new ADI TDD engine HDL.
> > The generic TDD controller is in essence a waveform generator
> > capable of addressing RF applications which require Time Division
> > Duplexing, as well as controlling other modules of general
> > applications through its dedicated 32 channel outputs.
> > 
> > The reason of creating the generic TDD controller was to reduce
> > the naming confusion around the existing repurposed TDD core
> > built for AD9361, as well as expanding its number of output
> > channels for systems which require more than six controlling signals.
> > 
> > Signed-off-by: Eliza Balas <eliza.balas@...log.com>
> > ---
> >  .../sysfs-bus-platform-drivers-adi-axi-tdd    | 158 ++++
> >  MAINTAINERS                                   |   2 +
> >  drivers/misc/Kconfig                          |  10 +
> >  drivers/misc/Makefile                         |   1 +
> >  drivers/misc/adi-axi-tdd.c                    | 753 ++++++++++++++++++
> >  5 files changed, 924 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-
> > tdd
> >  create mode 100644 drivers/misc/adi-axi-tdd.c
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-tdd
> > b/Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-tdd
> > new file mode 100644
> > index 000000000000..eb5f3db7d0cb
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-tdd
> > @@ -0,0 +1,158 @@
> > +What:           /sys/bus/platform/drivers/adi-axi-tdd/*/burst_count
> > +Date:           July 2023
> > +KernelVersion:  6.5
> 
> We are in 6.5 now, so there is no way your driver will be in 6.5. Target
> 6.6 and use phb crystall ball for next release date (September).
> 
> ...
> 
> > +
> > +enum adi_axi_tdd_attribute_id {
> > +       ADI_TDD_ATTR_VERSION,
> > +       ADI_TDD_ATTR_CORE_ID,
> > +       ADI_TDD_ATTR_SCRATCH,
> > +       ADI_TDD_ATTR_MAGIC,
> > +
> 
> ...
> 
> > +
> > +static int adi_axi_tdd_probe(struct platform_device *pdev)
> > +{
> > +       unsigned int expected_version, version, data;
> > +       struct adi_axi_tdd_state *st;
> > +       struct clk *aclk;
> > +       int ret;
> > +
> > +       st = devm_kzalloc(&pdev->dev, sizeof(*st), GFP_KERNEL);
> > +       if (!st)
> > +               return -ENOMEM;
> > +
> > +       st->base = devm_platform_ioremap_resource(pdev, 0);
> > +       if (IS_ERR(st->base))
> > +               return PTR_ERR(st->base);
> > +
> > +       platform_set_drvdata(pdev, st);
> > +
> > +       aclk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
> > +       if (IS_ERR(aclk))
> > +               return PTR_ERR(aclk);
> > +
> > +       ret = devm_add_action_or_reset(&pdev->dev, adi_axi_tdd_clk_disable,
> > aclk);
> 
> Looks you have here double disable.
> 
> > +       if (ret)
> > +               return ret;
> > +
> > +       st->clk.clk = devm_clk_get(&pdev->dev, "intf_clk");
> > +       if (IS_ERR(st->clk.clk))
> > +               return PTR_ERR(st->clk.clk);
> > +
> > +       ret = clk_prepare_enable(st->clk.clk);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = devm_add_action_or_reset(&pdev->dev, adi_axi_tdd_clk_disable, st-
> > >clk.clk);
> 
> Looks you have here double disable.
> 

Not in here but it should actually drop the action and use devm_clk_get_enabled()

- Nuno Sá