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> <20160217212415.2278.61232@quark.deferred.io> Message-ID: <20160218193424.2278.3229@quark.deferred.io> Subject: Re: [PATCH] clk: add userspace clock consumer Date: Thu, 18 Feb 2016 11:34:24 -0800 List-ID: Quoting Akinobu Mita (2016-02-18 06:09:46) > 2016-02-18 6:24 GMT+09:00 Michael Turquette : > > 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 a= nd > >> >> some classes of devices that are controlled entirely from user spac= e. > >> > > >> > 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_F= S? > >> > >> 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? > = > How about just add that to drivers/clk/Kconfig like below? > = > config COMMON_CLK_DEBUG > bool "Clock driver debugging support" > depends on DEBUG_KERNEL > ... Looks good. If this symbol is selected then we will source drivers/clk/Kconfig.debug. > = > config COMMON_CLK_USERSPACE_CONSUMER > tristate "Userspace clock consumer support" > depends on COMMON_CLK_DEBUG > ... This should go into drivers/clk/Kconfig.debug. There are some clk tests that I've been slowly working on and they will end up in the file as well at some point in the future. Regards, Mike > = > = > We can add COMMON_CLK_DEBUG_FS for /sys/kernel/debug/clk/ if preferred.