Message-ID: <558042E6.2090100@freescale.com>
Date: Tue, 16 Jun 2015 08:38:14 -0700
From: York Sun <yorksun@freescale.com>
User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.7.0
MIME-Version: 1.0
To: Paul Bolle <pebolle@tiscali.nl>
CC: <mturquette@baylibre.com>, <linux-kernel@vger.kernel.org>,
        <linux-i2c@vger.kernel.org>,
        Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
        Guenter Roeck <linux@roeck-us.net>,
        Andrey Filippov <andrey@elphel.com>
Subject: Re: [Patch v2] driver/clk/clk-si5338: Add common clock framework
 driver for si5338
References: <1434388051-23814-1-git-send-email-yorksun@freescale.com> <1434442897.2069.66.camel@x220> <55803E29.2040800@freescale.com>
In-Reply-To: <55803E29.2040800@freescale.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jun 2015 15:38:17.6685 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL2PR03MB148
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2137
Lines: 63


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/