Re: [PATCH RFC] clk: add gpio controlled clock
Hi,
Jyri Sarha wrote:
> The added clk-gpio is a basic clock that can be enabled and disabled
> trough a gpio output. The DT binding document for the clock is also
> added.
>
> Signed-off-by: Jyri Sarha
> ---
> .../devicetree/bindings/clock/gpio-clock.txt | 21 +++
> drivers/clk/Makefile |1 +
> drivers/clk/clk-gpio.c | 154
>
> include/linux/clk-provider.h | 25
> 4 files changed, 201 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/gpio-clock.txt
> create mode 100644 drivers/clk/clk-gpio.c
>
> diff --git a/Documentation/devicetree/bindings/clock/gpio-clock.txt
> b/Documentation/devicetree/bindings/clock/gpio-clock.txt
> new file mode 100644
> index 000..54fea39
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/gpio-clock.txt
[...]
> +/**
> + * clk_register_gpio - register a gpip clock with the clock framework
> + * @dev: device that is registering this clock
> + * @name: name of this clock
> + * @parent_name: name of this clock's parent
> + * @flags: framework-specific flags for this clock
> + * @gpio: gpio to control this clock
> + * @active_low: gpio polarity
> + */
> +struct clk *clk_register_gpio(struct device *dev, const char *name,
> + const char *parent_name, unsigned long flags,
> + unsigned int gpio, bool active_low)
> +{
> + struct clk_gpio *clk_gpio;
> + struct clk *clk;
> + struct clk_init_data init = { NULL };
> + unsigned long gpio_flags;
> + int err;
> +
> + if (active_low)
> + gpio_flags = GPIOF_OUT_INIT_LOW;
> + else
> + gpio_flags = GPIOF_OUT_INIT_HIGH;
> +
> + err = gpio_request_one(gpio, gpio_flags, name);
> + if (err) {
> + pr_err("%s: Error requesting clock control gpio %u\n",
> +__func__, gpio);
> + return ERR_PTR(-EINVAL);
>
You already have an error code from the gpio_request_one() call.
Why return a different one?
> +
> + clk_gpio = kzalloc(sizeof(struct clk_gpio), GFP_KERNEL);
>
devm_kzalloc()?
Lothar Waßmann
--
___
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | i...@karo-electronics.de
___
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv9 06/43] CLK: ti: add init support for clock IP blocks
On 11/01/2013 09:13 PM, Nishanth Menon wrote:
On 11/01/2013 04:12 AM, Tero Kristo wrote:
On 10/31/2013 05:42 PM, Nishanth Menon wrote:
On 10/25/2013 10:57 AM, Tero Kristo wrote:
ti_dt_clk_init_provider() can now be used to initialize the contents of
a single clock IP block. This parses all the clocks under the IP block
and calls the corresponding init function for them.
Signed-off-by: Tero Kristo
---
drivers/clk/ti/clk.c | 59
include/linux/clk/ti.h |1 +
2 files changed, 60 insertions(+)
diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
index ad58b01..7f030d7 100644
--- a/drivers/clk/ti/clk.c
+++ b/drivers/clk/ti/clk.c
@@ -19,6 +19,9 @@
#include
#include
#include
+#include
+
+extern struct of_device_id __clk_of_table[];
/**
* ti_dt_clocks_register - register DT duplicate clocks during boot
@@ -50,3 +53,59 @@ void __init ti_dt_clocks_register(struct ti_dt_clk oclks[])
}
}
}
+
+typedef int (*ti_of_clk_init_cb_t)(struct device_node *, struct regmap *);
+
+struct clk_init_item {
+ struct regmap *regmap;
+ struct device_node *np;
+ ti_of_clk_init_cb_t init_cb;
+ struct list_head node;
+};
+
+static LIST_HEAD(retry_list);
+
+void __init ti_dt_clk_init_provider(struct device_node *parent,
+ struct regmap *regmap)
+{
+ const struct of_device_id *match;
+ struct device_node *np;
+ ti_of_clk_init_cb_t clk_init_cb;
+ struct clk_init_item *retry;
+ struct clk_init_item *tmp;
+ int ret;
+
+ for_each_child_of_node(parent, np) {
+ match = of_match_node(__clk_of_table, np);
+ if (!match)
+ continue;
+ clk_init_cb = match->data;
I must admit I am confused here.
Yea this patch is something I am not quite comfortable myself yet and
would like improvement ideas
a) of_clk_init (in the generic clk.c) uses of_clk_init_cb_t as match
data. The prototype of the generic of_clk_init_cb_t is typedef void
(*of_clk_init_cb_t)(struct device_node *);
b) both of_clk_init and ti_dt_clk_init_provider looks up clock nodes
from __clk_of_table
__clk_of_table contains the function pointers for the clock init
functions, not clock nodes.
yes, apologies, should have stated the prototypes of functions in a
single __clk_of_table should not be two different argument list. The
reason is as follows: CLK_OF_DECLARE fills up that list. there can be
generic drivers such as [1] which might be registered, OR in the case
of MULTI_ARCH - multiple other SoC drivers would have registered
there. There is a bunch of stuff we could mess up here.
If we do not have a choice, if we would like to maintain a different
prototype based init list, we should probably do it in a different
structure - example __clk_of_table_regmap or something similar..
I assume we need ti_dt_clk_init_provider to be always called with
clock list, as a result of_clk_init(NULL); will never be invoked. This
is counter productive as you have have generic non SoC clock providers
as well who would have been invoked with of_clk_init(NULL);
Can't call of_clk_init(NULL) anymore, as Paul wants to map the clock
nodes under respective IP blocks. Two reasons here:
a) This requires me to pass some info from the IP block (CM1/CM2/PRM) to
the clock init functions, basically the pointer to the memory map region
(regmap.)
b) of_clk_init(NULL) will initialize all the clock nodes in the system,
irrespective of the hierarchy considerations.
Only thing that can be done, is to make the API introduced in this patch
a generic API and call it something like of_clk_init_children().
you still can do that:
of_prcm_init->ti_dt_clk_init_provider-> instead of matching nodes from
__clk_of_table, you can make it __clk_of_regmap_table -> the
CLK_OF_DECLARE will not be good enough here ofcourse, but this will
probably belong to a generic regmap enabled clock support framework -
i dont see much that is TI specific in drivers/clk/ti/clk.c and it
makes sense to be part of generic clock framework handling as generic
clock framework will receive regmap support based on previous patches.
that allows for other platforms to use regmap based clk drivers as well.
Yea different init tables can be introduced, however I still need to
split the support between hierarchical version and the blunt version
that registers everything. Looking at the work from Jyri, its possible I
need to add calls to both.
-Tero
[1] http://marc.info/?l=linux-omap&m=138331451210184&w=2
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv9 00/43] ARM: TI SoC clock DT conversion
On 11/01/2013 11:25 PM, Nishanth Menon wrote:
On 10/31/2013 08:55 AM, Nishanth Menon wrote:
On 10/31/2013 04:10 AM, Tero Kristo wrote:
On 10/30/2013 10:10 PM, Nishanth Menon wrote:
On 10/30/2013 10:00 AM, Nishanth Menon wrote:
On 10/30/2013 03:23 AM, Tero Kristo wrote:
On 10/29/2013 06:19 PM, Nishanth Menon wrote:
On 10/25/2013 10:56 AM, Tero Kristo wrote:
Testing done:
- omap3-beagle: boot + suspend/resume (ret + off)
- omap4-panda-es: boot + suspend/resume
- omap5-uevm: boot
- dra7-evm: boot
- am335x-bone: boot
Test branches available:
tree: https://github.com/t-kristo/linux-pm.git
Fully functioning test branch: 3.12-rc6-dt-clks-v9
^^ I tested this branch (boot testing):
Beagle-XM: http://pastebin.com/50A1qtFq (crashes + clkdm issues, dpll5
failed to transition)
I just sent you a private email with a patch to try out, should fix the
boot crash at least hopefully. Basically I forgot to convert one part of
the kernel to the new regmap stuff for omap36xx.
it does bootup yes.
clkdm issues are caused by wrong data in omap_hwmod_3xxx_data.c, USB
nodes are listing l3_init_clkdm for them, but this only exists on
omap4+. Seems like some copy paste bug introduced by someone.
dpll5 part I am not too sure, can you check if the same happens with
non-dt boot?
no-dt: http://pastebin.com/bYP9fTzH
dt: http://pastebin.com/xHup4L9Y
dpll5 warning seems to be only in dt-boot?
Tracked this down: you were missing the following - looks like the
conversion script might be missing converting the flags clock data
over to dts?
diff --git
a/arch/arm/boot/dts/omap36xx-am35xx-omap3430es2plus-clocks.dtsi
b/arch/arm/boot/dts/omap36xx-am35xx-omap3430es2plus-clocks.dtsi
index 7e37e3e..c9b77c8 100644
--- a/arch/arm/boot/dts/omap36xx-am35xx-omap3430es2plus-clocks.dtsi
+++ b/arch/arm/boot/dts/omap36xx-am35xx-omap3430es2plus-clocks.dtsi
@@ -30,6 +30,7 @@
compatible = "ti,omap3-dpll-clock";
clocks = <&sys_ck>, <&sys_ck>;
reg = <0x0d04>, <0x0d24>, <0x0d34>, <0x0d4c>;
+ ti,low-power-stop;
};
dpll5_m2_ck: dpll5_m2_ck {
Yea, seems I introduced the problem with the conversion script changes.
The valid fix for this is actually at the end of this mail (this fixes
both of the problems introduced, and also completes the fix you did), I
will add the fixes to the next rev.
One debug feedback:
reg = <0x0d04>, <0x0d24>, <0x0d34>, <0x0d4c>;
clocks = <&sys_ck>, <&sys_ck>;
we used indexing for varied register and clocks. however the register
meaning per index changes based on type of DPLL. This was one painful
thing to track down when debugging. the only robust option to debug
was to use prints as part of register write/reads to ensure that right
sequence was being followed.
I understand based on review comments for dtb size, we have removed
the names, but we lost sane debug capability as well with that.
I dont have any further feedback at this point beyond what is already
shared and so far my minimal tests have been good..
I have the following warnings:
sparse build warnings: http://pastebin.com/HZ1TWzyh
kernel-doc warnings: http://pastebin.com/JQwFEuaC
Hopefully, the next rev will not introducing nothing newer that what
we have in mainline.
I will have a look at those for the next rev, will most likely post next
against 3.13-rc1, there's no much point posting before.
-Tero
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: smsc911x: fix return value check in gpmc_smsc911x_init()
On 10/25/13 11:31, Wei Yongjun wrote:
> From: Wei Yongjun
>
> In case of error, the function platform_device_register_resndata()
> returns ERR_PTR() and never returns NULL. The NULL test in the return
> value check should be replaced with IS_ERR().
>
> Signed-off-by: Wei Yongjun
Acked-by: Igor Grinberg
> ---
> arch/arm/mach-omap2/gpmc-smsc911x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c
> b/arch/arm/mach-omap2/gpmc-smsc911x.c
> index ef99011..2757504 100644
> --- a/arch/arm/mach-omap2/gpmc-smsc911x.c
> +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
> @@ -83,7 +83,7 @@ void __init gpmc_smsc911x_init(struct
> omap_smsc911x_platform_data *gpmc_cfg)
> pdev = platform_device_register_resndata(NULL, "smsc911x", gpmc_cfg->id,
>gpmc_smsc911x_resources, ARRAY_SIZE(gpmc_smsc911x_resources),
>&gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config));
> - if (!pdev) {
> + if (IS_ERR(pdev)) {
> pr_err("Unable to register platform device\n");
> gpio_free(gpmc_cfg->gpio_reset);
> goto free2;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards,
Igor.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
