Re: [PATCH v3 7/9] uio: add Xilinx user clock monitor support

[Harry Austen <hpausten@protonmail.com>]

On Mon Aug 26, 2024 at 2:11 PM BST, Greg Kroah-Hartman wrote:
> On Mon, Aug 26, 2024 at 12:38:36PM +0000, Harry Austen wrote:
> > 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.
> >
> > Implemented as an auxiliary_driver to avoid introducing UIO dependency
> > to the main clock driver.
> >
> > Signed-off-by: Harry Austen <hpausten@protonmail.com>
> > ---
> >  drivers/uio/Kconfig            |  8 ++++
> >  drivers/uio/Makefile           |  1 +
> >  drivers/uio/uio_xlnx_clk_mon.c | 71 ++++++++++++++++++++++++++++++++++
> >  3 files changed, 80 insertions(+)
> >  create mode 100644 drivers/uio/uio_xlnx_clk_mon.c
> >
> > diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> > index b060dcd7c6350..ca8a53de26a67 100644
> > --- a/drivers/uio/Kconfig
> > +++ b/drivers/uio/Kconfig
> > @@ -164,4 +164,12 @@ config UIO_DFL
> >  	    opae-sdk/tools/libopaeuio/
> >
> >  	  If you compile this as a module, it will be called uio_dfl.
> > +
> > +config UIO_XLNX_CLK_MON
> > +	tristate "Xilinx user clock monitor support"
> > +	depends on COMMON_CLK_XLNX_CLKWZRD
> > +	help
> > +	  Userspace I/O interface to the user clock monitor logic within the
> > +	  Xilinx Clocking Wizard IP core.
>
> Why do you want a UIO api for a clock device?  What userspace code is
> going to access the hardware this way?  Why not use the normal
> kernel/user apis instead?

I was just trying to provide userspace access to these _unexpected_ clock
status event indications (clock stopped, underrun, overrun or glitched) and UIO
seemed like an easy way to do it and leave interrupt enablement and monitoring
up to userspace. I'm not aware of any existing clock event notification
framework. Are you suggesting that such a generic event notification mechanism
should be added to the clk subsystem? e.g. additional clk_ops callbacks etc.?

>
> thanks,
>
> greg k-h

Thanks for the review,
Harry