linux-kernel - Re: [PATCH 5/7] clk: clocking-wizard: add user clock monitor support

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: <1cacce63c7263a3532cca148ad2c567f.sboyd@kernel.org>
Date: Tue, 23 Jul 2024 16:29:51 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Conor Dooley <conor+dt@...nel.org>, Harry Austen <hpausten@...tonmail.com>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Michael Turquette <mturquette@...libre.com>, Michal Simek <michal.simek@....com>, Rob Herring <robh@...nel.org>
Cc: Shubhrajyoti Datta <shubhrajyoti.datta@....com>, linux-clk@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, Harry Austen <hpausten@...tonmail.com>
Subject: Re: [PATCH 5/7] clk: clocking-wizard: add user clock monitor support

Quoting Harry Austen (2024-07-20 05:01:53)
> Xilinx clocking wizard IP core supports monitoring of up to four
> optional user clock inputs, with a corresponding interrupt for
> notification in change of clock state (stop, underrun, overrun or
> glitch). Give userspace access to this monitor logic through use of the
> UIO framework.
> 
> Use presence of the user monitor interrupt description in devicetree to
> indicate whether or not the UIO device should be registered. Also, this
> functionality is only supported from v6.0 onwards, so add indication of
> support to the device match data, in order to be tied to the utilised
> compatible string.
> 
> Signed-off-by: Harry Austen <hpausten@...tonmail.com>
> ---
> diff --git a/drivers/clk/xilinx/Kconfig b/drivers/clk/xilinx/Kconfig
> index 051756953558b..907a435694687 100644
> --- a/drivers/clk/xilinx/Kconfig
> +++ b/drivers/clk/xilinx/Kconfig
> @@ -21,6 +21,7 @@ config COMMON_CLK_XLNX_CLKWZRD
>         tristate "Xilinx Clocking Wizard"
>         depends on OF
>         depends on HAS_IOMEM
> +       depends on UIO

If I have a pre-v6.0 device I probably don't want UIO though. Perhaps
you should use the auxiliary bus framework to register a device that is
otherwise unused and then have the uio driver live in drivers/uio and
match that device made here. I think you can have 'imply UIO' if you
like to put a weak Kconfig dependency.

>         help
>           Support for the Xilinx Clocking Wizard IP core clock generator.
>           Adds support for clocking wizard and compatible.
> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index 7b262d73310fe..2d419e8ad4419 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> @@ -1165,6 +1209,17 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>  
> +       data = device_get_match_data(&pdev->dev);
> +       if (data && data->supports_monitor) {
> +               irq = platform_get_irq(pdev, 0);
> +               if (irq > 0) {
> +                       ret = clk_wzrd_setup_monitor(&pdev->dev, irq,
> +                                                    platform_get_resource(pdev, IORESOURCE_IO, 0));

Any reason this can't be

		ret = clk_wzrd_setup_monitor(pdev);
		if (ret)
			return ret;

and then all the surrounding code be moved into the function, including
the dev_err_probe()?

> +                       if (ret)
> +                               return dev_err_probe(&pdev->dev, ret, "failed to setup monitor\n");
> +               }
> +       }
> +
>         ret = of_property_read_u32(np, "xlnx,nr-outputs", &nr_outputs);