Re: [PATCH] clk: add userspace clock consumer

[Michael Turquette <mturquette@baylibre.com>]

Quoting Akinobu Mita (2016-02-17 05:18:41)
> 2016-02-16 8:02 GMT+09:00 Michael Turquette <mturquette@baylibre.com>:
> > Hello Akinobu Mita,
> >
> > Quoting Akinobu Mita (2016-02-15 06:40:51)
> >> This adds userspace consumer for common clock.
> >>
> >> This driver is inspired from Userspace regulator consumer
> >> (REGULATOR_USERSPACE_CONSUMER) and it is useful for test purposes and
> >> some classes of devices that are controlled entirely from user space.
> >
> > Thanks for submitting the patch. Your implementation looks OK, but I
> > generally do not like to expose clock hardware controls to userspace
> > (and have a NAK'd a lot of patches trying to do this in the past). It
> > can be quite dangerous for the system to allow userspace to control
> > clocks.
> =

> I understand your concern.  I'll happily add the features and
> documentation for this module to avoid misuses and abuses.
> =

> > Can you explain your use case more? If your main concern is testing,
> =

> I can use this for my own electrical circuit.  But mainly it's
> useful for testing.  When I tried to add clock provider for DS3231
> clkout, I wrote very ad hoc kernel code for testing it.  I think
> other people also have been trying something similar.
> =

> > should COMMON_CLK_USERSPACE_CONSUMER be hidden behind CONFIG_DEBUG_FS?
> =

> But this driver does not use debugfs.  Instead of that, I have no
> problem moving this driver to lib/ and add config option to
> lib/Kconfig.debug.  (under "Kernel hacking" in menuconfig)

Hiding it behind some sort of debug option sounds good to me. Taking a
quick look through lib/Kconfig.debug, it seems that there are no other
examples of debug options from drivers/* in there.

I'm wondering what is the best way to do it? Just create a
CONFIG_COMMON_CLK_DEBUG symbol and source drivers/clk/Kconfig.debug
conditionally?

Regards,
Mike

> =

> > Have you looked at the per-provider DEBUGFS hooks? See commit
> > fb2b3c9f6857, "clk: define and export clk_debugs_add_file".
> =

> I think it is useful for providing device specific debug features.
> But this userspace consumer only provides generic feature for most
> clock provider and allows controlling the devices specified in
> the device tree.