From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 To: Akinobu Mita , From: Michael Turquette In-Reply-To: Cc: linux-clk@vger.kernel.org, "Stephen Boyd" References: <1455547251-4944-1-git-send-email-akinobu.mita@gmail.com> <20160215230252.2278.43532@quark.deferred.io> Message-ID: <20160217212415.2278.61232@quark.deferred.io> Subject: Re: [PATCH] clk: add userspace clock consumer Date: Wed, 17 Feb 2016 13:24:15 -0800 List-ID: Quoting Akinobu Mita (2016-02-17 05:18:41) > 2016-02-16 8:02 GMT+09:00 Michael Turquette : > > 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.