On 10/09, 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 <york...@freescale.com>
> CC: Mike Turquette <mturque...@baylibre.com>
> CC: Sebastian Hesselbarth <sebastian.hesselba...@gmail.com>
> CC: Guenter Roeck <li...@roeck-us.net>
> CC: Andrey Filippov <and...@elphel.com>
> CC: Paul Bolle <pebo...@tiscali.nl>
>
> ---
> This is a resend. I didn't see the v6 patch in mailing list archive.
Can you please refresh this on clk-next? It doesn't compile
there. I've added some more nitpicks. I'll look in more detail
next week.
> +
> +#include <dt-bindings/clock/clk-si5338.h>
> +#include <linux/bsearch.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/freezer.h>
This is used?
> +#include <linux/kernel.h>
> +#include <linux/i2c.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
Is this used?
> +#include <linux/platform_data/si5338.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +
[...]
> +
> +static u32 awe_fcal[] = {
const?
> + AWE_FCAL_07_00,
> + AWE_FCAL_15_08,
> + AWE_FCAL_17_16,
> + 0
> +};
> +
> +static u32 awe_fcal_ovrd[] = {
const?
> + AWE_FCAL_OVRD_07_00,
> + AWE_FCAL_OVRD_15_08,
> + AWE_FCAL_OVRD_17_15,
> + 0
> +};
> +
[..]
> +
> +static const struct clk_ops si5338_clkout_ops = {
> + .prepare = si5338_clkout_prepare,
> + .unprepare = si5338_clkout_unprepare,
> + .enable = si5338_clkout_enable,
> + .disable = si5338_clkout_disable,
> + .set_parent = si5338_clkout_set_parent,
> + .get_parent = si5338_clkout_get_parent,
> + .recalc_rate = si5338_clkout_recalc_rate,
> + .round_rate = si5338_clkout_round_rate,
> + .set_rate = si5338_clkout_set_rate,
> +};
> +
> +#ifdef CONFIG_DEBUG_FS
> +#include <linux/debugfs.h>
Just include this at the top unconditionally please.
> +
> +/*
> + * Create debugfs files for status for each si5338 clkout
> + */
> +
> +static int clkout_status_show(struct seq_file *s, void *data)
> +{
> + struct si5338_hw_data *clkout = (struct si5338_hw_data *)s->private;
Useless cast, please remove.
> + struct si5338_driver_data *drvdata = clkout->drvdata;
> + int i, j, match = 0;
> + int drv_type, drv_vdd, drv_trim, drv_invert;
> + int out_src, src_group = 0, src = 0;
> + const int in_numbers[] = {
> + 12, 3, 4, 56
[...]
> +/*
> + * To support multiple si5338 chips, we cannot use devm_clk_get because
> + * each chip has its own clock sources. If device tree is not used,
> + * platform driver should provide these clocks. Let the clocks be freed
> + * automatically when device is unbound. We implement our own
> devm_of_clk_get.
> + */
> +static void devm_of_clk_release(struct device *dev, void *res)
> +{
> + clk_put(*(struct clk **)res);
> +}
> +
> +static struct clk *devm_of_clk_get(struct device *dev, struct device_node
> *np,
What is this? I don't get it at all.
> + int index)
> +{
> + struct clk **ptr, *clk;
> +
> + ptr = devres_alloc(devm_of_clk_release, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + clk = of_clk_get(np, index);
> +
[...]
> + /*
> + * Important: Go through the procedure to check PLL locking
> + * and other steps required by si5338 reference manual.
> + */
> + ret = post_init(drvdata);
> + if (ret)
> + return ret;
> +
> + dev_dbg(&client->dev, "%s clocks are registered\n", id->name);
> +
> +#ifdef CONFIG_OF
Remove this ifdef
> + ret = of_clk_add_provider(client->dev.of_node,
> + of_clk_src_onecell_get,
> + &drvdata->onecell);
> + if (ret) {
> + dev_err(&client->dev, "unable to add clk provider\n");
> + return ret;
[..]
> +
> +static int si5338_i2c_remove(struct i2c_client *client)
> +{
> + struct si5338_driver_data *drvdata = i2c_get_clientdata(client);
> + int n;
> +
> +#ifdef CONFIG_OF
And this one.
> + of_clk_del_provider(client->dev.of_node);
> +#endif
> +
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
