Messages in this thread | | | Date | Tue, 16 Jun 2015 08:38:14 -0700 | From | York Sun <> | Subject | Re: [Patch v2] driver/clk/clk-si5338: Add common clock framework driver for si5338 |
| |
On 06/16/2015 08:18 AM, York Sun wrote: > Paul, > > Thanks for reviewing. > > On 06/16/2015 01:21 AM, Paul Bolle wrote: >> One question and a few nits follow. >> >> On Mon, 2015-06-15 at 10:07 -0700, York Sun wrote: >>> SI5338 is a programmable clock generator. It has 4 sets of inputs, >>> PLL, multisynth and dividers to make 4 outputs. This driver splits >>> them into multiple clocks to comply with common clock framework. >>> >>> See Documentation/devicetree/bindings/clock/silabs,si5338.txt for >>> details. >>> >>> Signed-off-by: York Sun <yorksun@freescale.com> >>> CC: Mike Turquette <mturquette@linaro.org> >> >> Apparently that's now mturquette@baylibre.com . > > Thanks. Will change. > >> >>> CC: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> >>> CC: Guenter Roeck <linux@roeck-us.net> >>> CC: Andrey Filippov <andrey@elphel.com> >> >>> --- a/drivers/clk/Kconfig >>> +++ b/drivers/clk/Kconfig >> >>> config COMMON_CLK >>> - bool >>> + tristate "Common Clock" >>> select HAVE_CLK_PREPARE >>> select CLKDEV_LOOKUP >>> select SRCU >> >> Why? The commit explanation doesn't mention this. Did you use an unclean >> tree? If not, you just created over a dozen of new modules: > > Thanks for catching this. I was testing building the driver within and outside > of kernel tree for another kernel version. If this driver is built-in, I don't > need to make it tristate. Will revert in next version. >
Now I remember why I did this. COMMON_CLK wasn't an option users can select, because it is a bool and only selected by some platforms. I think it should be a tristate so one can build a driver with it. When it is selected, some drivers are built, either into kernel or as modules, up to user's choice. They are needed by common clock framework.
I should add explanation in commit message. Or separate it into an individual patch. Which one is preferred?
York
| |