linux-kernel - Re: [PATCH v2 6/7] clk: Add initial WM831x clock driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 24 Sep 2011 22:29:59 -0700
From:	"Turquette, Mike" <mturquette@...com>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	jeremy.kerr@...onical.com, broonie@...nsource.wolfsonmicro.com,
	tglx@...utronix.de, linus.walleij@...ricsson.com,
	amit.kucheria@...aro.org, dsaxena@...aro.org, patches@...aro.org,
	linaro-dev@...ts.linaro.org, paul@...an.com, sboyd@...inc.com,
	shawn.guo@...escale.com, skannan@...cinc.com,
	magnus.damm@...il.com, arnd.bergmann@...aro.org,
	linux@....linux.org.uk, eric.miao@...aro.org,
	richard.zhao@...aro.org
Subject: Re: [PATCH v2 6/7] clk: Add initial WM831x clock driver

On Sat, Sep 24, 2011 at 9:08 PM, Grant Likely <grant.likely@...retlab.ca> wrote:
> On Thu, Sep 22, 2011 at 03:27:01PM -0700, Mike Turquette wrote:
>> From: Mark Brown <broonie@...nsource.wolfsonmicro.com>
>>
>> The WM831x and WM832x series of PMICs contain a flexible clocking
>> subsystem intended to provide always on and system core clocks.  It
>> features:
>>
>> - A 32.768kHz crystal oscillator which can optionally be used to pass
>>   through an externally generated clock.
>> - A FLL which can be clocked from either the 32.768kHz oscillator or
>>   the CLKIN pin.
>> - A CLKOUT pin which can bring out either the oscillator or the FLL
>>   output.
>> - The 32.768kHz clock can also optionally be brought out on the GPIO
>>   pins of the device.
>>
>> This driver fully supports the 32.768kHz oscillator and CLKOUT.  The FLL
>> is supported only in AUTO mode, the full flexibility of the FLL cannot
>> currently be used.  The use of clock references other than the internal
>> oscillator is not currently supported, and since clk_set_parent() is not
>> implemented in the generic clock API the clock tree configuration cannot
>> be changed at runtime.
>>
>> Due to a lack of access to systems where the core SoC has been converted
>> to use the generic clock API this driver has been compile tested only.
>>
>> Signed-off-by: Mark Brown <broonie@...nsource.wolfsonmicro.com>
>> Signed-off-by: Mike Turquette <mturquette@...com>
>
> A few minor comments below.  Otherwise looks fine to me.
>
>> +static __devinit int wm831x_clk_probe(struct platform_device *pdev)
>> +{
>> +     struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent);
>> +     struct wm831x_clk *clkdata;
>> +     int ret;
>> +
>> +     clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL);
>
> If devm_kzalloc() is used, then all the kfree unwinding can be
> dropped.
>
>> +static int __init wm831x_clk_init(void)
>> +{
>> +     int ret;
>> +
>> +     ret = platform_driver_register(&wm831x_clk_driver);
>> +     if (ret != 0)
>> +             pr_err("Failed to register WM831x clock driver: %d\n", ret);
>> +
>> +     return ret;
>
> No need for this song-and-dance.  The driver core is pretty well
> debugged.  Just use "return platform_driver_register(...);"

Grant,

Thanks for the review.

Mark,

I know you're not carrying this whole set of patches but do you want
to rework this and resend or do you just want me to fix it up?
Changes are trivial if you don't want to touch it.

Regards,
Mike

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