From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752557AbcAAXxO (ORCPT ); Fri, 1 Jan 2016 18:53:14 -0500 Received: from mail-pf0-f173.google.com ([209.85.192.173]:36433 "EHLO mail-pf0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752244AbcAAXxK convert rfc822-to-8bit (ORCPT ); Fri, 1 Jan 2016 18:53:10 -0500 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT To: Simon Arlott , "Stephen Boyd" , "Kevin Cernekee" , "Florian Fainelli" , "devicetree@vger.kernel.org" From: Michael Turquette In-Reply-To: <5669F3C3.3090207@simon.arlott.org.uk> Cc: "Linux Kernel Mailing List" , linux-clk@vger.kernel.org, linux-mips@linux-mips.org, "Rob Herring" , "Pawel Moll" , "Mark Rutland" , "Ian Campbell" , "Kumar Gala" , "Jonas Gorski" References: <5669F361.60405@simon.arlott.org.uk> <5669F3C3.3090207@simon.arlott.org.uk> Message-ID: <20160101234031.7140.66812@quark.deferred.io> User-Agent: alot/0.3.6 Subject: Re: [PATCH linux-next (v2) 2/2] clk: bcm6345: Add BCM6345 gated clock support Date: Fri, 01 Jan 2016 15:40:31 -0800 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Simon, Quoting Simon Arlott (2015-12-10 13:50:59) > +#define to_clk_bcm6345(_hw) container_of(_hw, struct clk_bcm6345, hw) > + > +static int clk_bcm6345_enable(struct clk_hw *hw) > +{ > + struct clk_bcm6345 *gate = to_clk_bcm6345(hw); > + > + return regmap_write_bits(gate->map, gate->offset, > + gate->mask, gate->mask); Does your regmap hold a spinlock or mutex? If a mutex then this should be a .prepare callback instead of .enable. If it's a spinlock then nothing to see here, move along. > +} > + > +static void clk_bcm6345_disable(struct clk_hw *hw) > +{ > + struct clk_bcm6345 *gate = to_clk_bcm6345(hw); > + > + regmap_write_bits(gate->map, gate->offset, > + gate->mask, 0); > +} > + > +static int clk_bcm6345_is_enabled(struct clk_hw *hw) > +{ > + struct clk_bcm6345 *gate = to_clk_bcm6345(hw); > + unsigned int val; > + int ret; > + > + ret = regmap_read(gate->map, gate->offset, &val); > + if (ret) > + return ret; > + > + val &= gate->mask; > + > + return val ? 1 : 0; > +} > + > +const struct clk_ops clk_bcm6345_ops = { > + .enable = clk_bcm6345_enable, > + .disable = clk_bcm6345_disable, > + .is_enabled = clk_bcm6345_is_enabled, > +}; > + > +static struct clk * __init of_bcm6345_clk_register(const char *parent_name, > + const char *clk_name, struct regmap *map, u32 offset, u32 mask) > +{ > + struct clk_bcm6345 *gate; > + struct clk_init_data init; > + struct clk *ret; > + > + gate = kzalloc(sizeof(*gate), GFP_KERNEL); > + if (!gate) > + return ERR_PTR(-ENOMEM); > + > + init.name = clk_name; > + init.ops = &clk_bcm6345_ops; > + init.flags = CLK_IS_BASIC; Why is CLK_IS_BASIC set? > + init.parent_names = (parent_name ? &parent_name : NULL); > + init.num_parents = (parent_name ? 1 : 0); > + gate->hw.init = &init; > + gate->map = map; > + gate->offset = offset; > + gate->mask = mask; > + > + ret = clk_register(NULL, &gate->hw); > + if (IS_ERR(ret)) > + kfree(gate); > + > + return ret; > +} > + > +static void __init of_bcm6345_clk_setup(struct device_node *node) > +{ > + struct clk_onecell_data *clk_data; > + const char *parent_name = NULL; > + struct regmap *map; > + u32 offset; > + unsigned int clocks = 0; > + int i; > + > + clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL); > + if (!clk_data) > + return; > + > + clk_data->clk_num = 32; > + clk_data->clks = kmalloc_array(clk_data->clk_num, > + sizeof(*clk_data->clks), GFP_KERNEL); > + > + for (i = 0; i < clk_data->clk_num; i++) > + clk_data->clks[i] = ERR_PTR(-ENODEV); > + > + if (of_clk_get_parent_count(node) > 0) > + parent_name = of_clk_get_parent_name(node, 0); > + > + map = syscon_regmap_lookup_by_phandle(node, "regmap"); > + if (IS_ERR(map)) { > + pr_err("%s: regmap lookup error %ld\n", > + node->full_name, PTR_ERR(map)); > + goto out; > + } > + > + if (of_property_read_u32(node, "offset", &offset)) { > + pr_err("%s: offset not specified\n", node->full_name); > + goto out; > + } > + > + /* clks[] is sparse, indexed by bit, maximum clocks checked using i */ > + for (i = 0; i < clk_data->clk_num; i++) { > + u32 bit; > + const char *clk_name; > + > + if (of_property_read_u32_index(node, "clock-indices", > + i, &bit)) > + goto out; > + > + if (of_property_read_string_index(node, "clock-output-names", > + i, &clk_name)) > + goto out; > + > + if (bit >= clk_data->clk_num) { > + pr_err("%s: clock bit %u out of range\n", > + node->full_name, bit); > + continue; > + } > + > + if (!IS_ERR(clk_data->clks[bit])) { > + pr_err("%s: clock bit %u already exists\n", > + node->full_name, bit); > + continue; > + } > + > + clk_data->clks[bit] = of_bcm6345_clk_register(parent_name, > + clk_name, map, offset, BIT(bit)); > + if (!IS_ERR(clk_data->clks[bit])) > + clocks++; > + } > + > +out: > + if (clocks) { > + of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); > + pr_info("%s: registered %u clocks\n", node->name, clocks); > + } else { > + kfree(clk_data->clks); > + kfree(clk_data); > + } > +} > + > +CLK_OF_DECLARE(bcm6345_clk, "brcm,bcm6345-gate-clk", of_bcm6345_clk_setup); Please do not use CLK_OF_DECLARE unless there is a good reason. Ideally this should be a platform_driver. Regards, Mike > -- > 2.1.4 > > -- > Simon Arlott