lkml.org 
[lkml]   [2015]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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


    \
     
     \ /
      Last update: 2015-06-16 17:41    [W:4.134 / U:0.656 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site