Re: [Cocci] [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances
Sorry let me do that properly. On 05/02/2015 16:45, Quentin Lambert wrote: On 05/02/2015 00:26, Stephen Boyd wrote: If you want me to I can enlarge the search to other directories. Yes please do. And if you could share the coccinelle patch that would be great. Thanks. structclk.cocci is the coccinelle patch structclk-arm.patch is the result I got when applying it to the arch/arm directory Is there anything else I can do to help? These are the new instances I found by applying the patch to arch/arm directory: ./arch/arm/mach-imx/mach-imx6q.c @@ -211,7 +211,6 @@ static void __init imx6q_1588_init(void) * set bit IOMUXC_GPR1[21]. Or the PTP clock must be from pad * (external OSC), and we need to clear the bit. */ clksel = ptp_clk == enet_ref ? IMX6Q_GPR1_ENET_CLK_SEL_ANATOP : IMX6Q_GPR1_ENET_CLK_SEL_PAD; gpr = syscon_regmap_lookup_by_compatible(fsl,imx6q-iomuxc-gpr); if (!IS_ERR(gpr)) ./arch/arm/mach-shmobile/clock-r8a73a4.c @@ -139,7 +139,6 @@ static int pll_set_parent(struct clk *cl /* Search the parent */ for (i = 0; i clk-parent_num; i++) if (clk-parent_table[i] == parent) break; if (i == clk-parent_num) ./arch/arm/mach-shmobile/clock-sh7372.c @@ -223,7 +223,6 @@ static int pllc2_set_parent(struct clk * /* Search the parent */ for (i = 0; i clk-parent_num; i++) if (clk-parent_table[i] == parent) break; if (i == clk-parent_num) ./arch/arm/mach-shmobile/clock-r8a7740.c @@ -195,7 +195,6 @@ static int usb24s_set_parent(struct clk /* Search the parent */ for (i = 0; i clk-parent_num; i++) if (clk-parent_table[i] == parent) break; if (i == clk-parent_num) -- 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
am335x: USB DMA IRQ issue
I have a problem with my am335x based board and USB. Kernel: Linux version 3.18.1 (YegorYefremov@development1) (gcc version 4.9.2 (Buildroot 2015.02-git-00797-gf1b07c0) ) #1 SMP Thu Jan 15 15:31:27 CET 2015 I took two devices and equipped them with USB WLAN cards: Bus 001 Device 003: ID 148f:5370 Ralink Technology, Corp. RT5370 Wireless Adapter. One device as AP and another as client. Client makes permanent ping to AP. And from time to time I start nuttcp session. After 2-3 days I get following errors: CPU: 0 PID: 6123 Comm: kworker/u2:2 Tainted: GW 3.18.1 #1 Workqueue: phy0 rt2x00link_watchdog [rt2x00lib] [c001596c] (unwind_backtrace) from [c0012218] (show_stack+0x10/0x14) [c0012218] (show_stack) from [c05c7fe8] (dump_stack+0x84/0x9c) [c05c7fe8] (dump_stack) from [c004031c] (warn_slowpath_common+0x6c/0x90) [c004031c] (warn_slowpath_common) from [c00403dc] (warn_slowpath_null+0x1c/0x24) [c00403dc] (warn_slowpath_null) from [c02d0d44] (cppi41_dma_control+0x298/0x2e0) [c02d0d44] (cppi41_dma_control) from [c03eb100] (cppi41_dma_channel_abort+0xc0/0x134) [c03eb100] (cppi41_dma_channel_abort) from [c03e69f8] (musb_cleanup_urb.isra.12+0x40/0xfc) [c03e69f8] (musb_cleanup_urb.isra.12) from [c03e6d04] (musb_urb_dequeue+0xe8/0x128) [c03e6d04] (musb_urb_dequeue) from [c03b74bc] (usb_hcd_unlink_urb+0x4c/0x84) [c03b74bc] (usb_hcd_unlink_urb) from [c03b863c] (usb_kill_urb+0x4c/0xc4) [c03b863c] (usb_kill_urb) from [bf01291c] (rt2x00usb_flush_entry+0x30/0x58 [rt2x00usb]) [bf01291c] (rt2x00usb_flush_entry [rt2x00usb]) from [bf007128] (rt2x00queue_for_each_entry+0x84/0x250 [rt2x00lib]) [bf007128] (rt2x00queue_for_each_entry [rt2x00lib]) from [bf0128e4] (rt2x00usb_flush_queue+0x98/0xa0 [rt2x00usb]) [bf0128e4] (rt2x00usb_flush_queue [rt2x00usb]) from [bf007bcc] (rt2x00queue_flush_queue+0x44/0xa8 [rt2x00lib]) [bf007bcc] (rt2x00queue_flush_queue [rt2x00lib]) from [bf012d50] (rt2x00usb_watchdog+0xc4/0xec [rt2x00usb]) [bf012d50] (rt2x00usb_watchdog [rt2x00usb]) from [bf02f950] (rt2800usb_watchdog+0x74/0x1c0 [rt2800usb]) [bf02f950] (rt2800usb_watchdog [rt2800usb]) from [bf008b30] (rt2x00link_watchdog+0x2c/0x58 [rt2x00lib]) [bf008b30] (rt2x00link_watchdog [rt2x00lib]) from [c005586c] (process_one_work+0x1c4/0x494) [c005586c] (process_one_work) from [c00562dc] (worker_thread+0x38/0x4bc) [c00562dc] (worker_thread) from [c005a6e8] (kthread+0xc8/0xe4) [c005a6e8] (kthread) from [c000e888] (ret_from_fork+0x14/0x2c) ---[ end trace 981fe2c9ed6055b8 ]--- [ cut here ] WARNING: CPU: 0 PID: 6123 at drivers/dma/cppi41.c:320 cppi41_irq+0x144/0x198() Modules linked in: rt2800usb rt2800lib rt2x00usb rt2x00lib led_class CPU: 0 PID: 6123 Comm: kworker/u2:2 Tainted: GW 3.18.1 #1 Workqueue: phy0 rt2x00link_watchdog [rt2x00lib] [c001596c] (unwind_backtrace) from [c0012218] (show_stack+0x10/0x14) [c0012218] (show_stack) from [c05c7fe8] (dump_stack+0x84/0x9c) [c05c7fe8] (dump_stack) from [c004031c] (warn_slowpath_common+0x6c/0x90) [c004031c] (warn_slowpath_common) from [c00403dc] (warn_slowpath_null+0x1c/0x24) [c00403dc] (warn_slowpath_null) from [c02d12dc] (cppi41_irq+0x144/0x198) [c02d12dc] (cppi41_irq) from [c0089544] (handle_irq_event_percpu+0x3c/0x1d8) [c0089544] (handle_irq_event_percpu) from [c008971c] (handle_irq_event+0x3c/0x5c) [c008971c] (handle_irq_event) from [c008c2ec] (handle_level_irq+0xb4/0x144) [c008c2ec] (handle_level_irq) from [c0088c70] (generic_handle_irq+0x28/0x3c) [c0088c70] (generic_handle_irq) from [c0088f38] (__handle_domain_irq+0x64/0xc8) [c0088f38] (__handle_domain_irq) from [c0008780] (omap_intc_handle_irq+0xb4/0xc4) [c0008780] (omap_intc_handle_irq) from [c05cf9a4] (__irq_svc+0x44/0x5c) Exception stack(0xcf7a5d30 to 0xcf7a5d78) 5d20: 0001 ccb64af8 ccb64580 5d40: a013 cf5a2010 cf5a2010 a013 c0008300 0001 cf365934 5d60: 0002 cf7a5d78 c007cd34 c05cefe8 2013 [c05cf9a4] (__irq_svc) from [c05cefe8] (_raw_spin_unlock_irqrestore+0x34/0x44) [c05cefe8] (_raw_spin_unlock_irqrestore) from [c03e6cc0] (musb_urb_dequeue+0xa4/0x128) [c03e6cc0] (musb_urb_dequeue) from [c03b74bc] (usb_hcd_unlink_urb+0x4c/0x84) [c03b74bc] (usb_hcd_unlink_urb) from [c03b863c] (usb_kill_urb+0x4c/0xc4) [c03b863c] (usb_kill_urb) from [bf01291c] (rt2x00usb_flush_entry+0x30/0x58 [rt2x00usb]) [bf01291c] (rt2x00usb_flush_entry [rt2x00usb]) from [bf007128] (rt2x00queue_for_each_entry+0x84/0x250 [rt2x00lib]) [bf007128] (rt2x00queue_for_each_entry [rt2x00lib]) from [bf0128e4] (rt2x00usb_flush_queue+0x98/0xa0 [rt2x00usb]) [bf0128e4] (rt2x00usb_flush_queue [rt2x00usb]) from [bf007bcc] (rt2x00queue_flush_queue+0x44/0xa8 [rt2x00lib]) [bf007bcc] (rt2x00queue_flush_queue [rt2x00lib]) from [bf012d50] (rt2x00usb_watchdog+0xc4/0xec [rt2x00usb]) [bf012d50] (rt2x00usb_watchdog [rt2x00usb]) from [bf02f950] (rt2800usb_watchdog+0x74/0x1c0 [rt2800usb]) [bf02f950] (rt2800usb_watchdog [rt2800usb]) from [bf008b30] (rt2x00link_watchdog+0x2c/0x58
Re: [Cocci] [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances
On 05/02/2015 00:26, Stephen Boyd wrote:
If you want me to I can enlarge the search to other directories.
Yes please do. And if you could share the coccinelle patch that would be
great. Thanks.
structclk.cocci is the coccinelle patch
structclk-arm.patch is the result I got when applying it to the arch/arm
directory
Is there anything else I can do to help?
diff -u -p ./arch/arm/mach-imx/mach-imx6q.c /tmp/nothing/mach-imx/mach-imx6q.c
--- ./arch/arm/mach-imx/mach-imx6q.c
+++ /tmp/nothing/mach-imx/mach-imx6q.c
@@ -211,7 +211,6 @@ static void __init imx6q_1588_init(void)
* set bit IOMUXC_GPR1[21]. Or the PTP clock must be from pad
* (external OSC), and we need to clear the bit.
*/
- clksel = ptp_clk == enet_ref ? IMX6Q_GPR1_ENET_CLK_SEL_ANATOP :
IMX6Q_GPR1_ENET_CLK_SEL_PAD;
gpr = syscon_regmap_lookup_by_compatible(fsl,imx6q-iomuxc-gpr);
if (!IS_ERR(gpr))
diff -u -p ./arch/arm/mach-shmobile/clock-r8a73a4.c /tmp/nothing/mach-shmobile/clock-r8a73a4.c
--- ./arch/arm/mach-shmobile/clock-r8a73a4.c
+++ /tmp/nothing/mach-shmobile/clock-r8a73a4.c
@@ -139,7 +139,6 @@ static int pll_set_parent(struct clk *cl
/* Search the parent */
for (i = 0; i clk-parent_num; i++)
- if (clk-parent_table[i] == parent)
break;
if (i == clk-parent_num)
diff -u -p ./arch/arm/mach-shmobile/clock-sh7372.c /tmp/nothing/mach-shmobile/clock-sh7372.c
--- ./arch/arm/mach-shmobile/clock-sh7372.c
+++ /tmp/nothing/mach-shmobile/clock-sh7372.c
@@ -223,7 +223,6 @@ static int pllc2_set_parent(struct clk *
/* Search the parent */
for (i = 0; i clk-parent_num; i++)
- if (clk-parent_table[i] == parent)
break;
if (i == clk-parent_num)
diff -u -p ./arch/arm/mach-shmobile/clock-r8a7740.c /tmp/nothing/mach-shmobile/clock-r8a7740.c
--- ./arch/arm/mach-shmobile/clock-r8a7740.c
+++ /tmp/nothing/mach-shmobile/clock-r8a7740.c
@@ -195,7 +195,6 @@ static int usb24s_set_parent(struct clk
/* Search the parent */
for (i = 0; i clk-parent_num; i++)
- if (clk-parent_table[i] == parent)
break;
if (i == clk-parent_num)
diff -u -p ./arch/arm/mach-omap2/clkt_clksel.c /tmp/nothing/mach-omap2/clkt_clksel.c
--- ./arch/arm/mach-omap2/clkt_clksel.c
+++ /tmp/nothing/mach-omap2/clkt_clksel.c
@@ -67,7 +67,6 @@ static const struct clksel *_get_clksel_
return NULL;
for (clks = clk-clksel; clks-parent; clks++)
- if (clks-parent == src_clk)
break; /* Found the requested parent */
if (!clks-parent) {
diff -u -p ./arch/arm/mach-omap2/timer.c /tmp/nothing/mach-omap2/timer.c
--- ./arch/arm/mach-omap2/timer.c
+++ /tmp/nothing/mach-omap2/timer.c
@@ -298,7 +298,6 @@ static int __init omap_dm_timer_init_one
if (IS_ERR(src))
return PTR_ERR(src);
- if (clk_get_parent(timer-fclk) != src) {
r = clk_set_parent(timer-fclk, src);
if (r 0) {
pr_warn(%s: %s cannot set source\n, __func__,
/// Find any attempt to compare or dereference struct clk pointers.
///
// Confidence: High
// Copyright: (C) 2015 Quentin Lambert, INRIA/LiP6. GPLv2
// URL: http://coccinelle.lip6.fr/
// Options: --recursive-includes --relax-include-path
// Options: --include-headers-for-types
virtual context
virtual org
virtual report
//
@comparison_dereference depends on context || org || report@
struct clk *x1, x2;
position j0;
@@
(
* x1@j0 == x2
|
* x1@j0 != x2
|
* *x1@j0
)
//
@script:python comparison_dereference_org depends on org@
j0 comparison_dereference.j0;
@@
msg = WARNING trying to compare or dereference struct clk pointers.
coccilib.org.print_todo(j0[0], msg)
//
@script:python comparison_dereference_report depends on report@
j0 comparison_dereference.j0;
@@
msg = WARNING trying to compare or dereference struct clk pointers.
coccilib.report.print_report(j0[0], msg)
Re: [PATCH 1/2] usb: musb: Fix use for of_property_read_bool for disabled multipoint
* Felipe Balbi ba...@ti.com [150205 10:12]:
On Thu, Feb 05, 2015 at 08:35:12AM -0800, Tony Lindgren wrote:
The value for the multipoint dts property is ignored when parsing with
of_property_read_bool, so we currently have multipoint always set as 1
even if value 0 is specified in the dts file.
Let's fix this to read the value too instead of just the property like
the binding documentation says as otherwise MUSB will fail to work
on devices with Mentor configuration that does not support multipoint.
Cc: Brian Hutchinson b.hutch...@gmail.com
Signed-off-by: Tony Lindgren t...@atomide.com
do you mind waiting a little bit to see if my boolean properties with
value patch is accepted ?
This can wait for v3.20-rc1 for sure but cc stable would be nice to
avoid more pointless debugging by somebody else.
http://marc.info/?l=linux-omapm=142315930232743w=2
At least let's see where the discussion moves.
Cool yeah at least a warning should be printed, sounds like it may
not be usable for fixing $subject though.
I also noticed that a last minute change I did from read_u32 to
read_u8 in $subject patch broke things and introduced new build
warnings. Here's a fixed version back to using read_u32 instead
of read_u8 so we don't need to specify the storage size with
/bits/ 8 values in the dts files.
Regards,
Tony
8
From: Tony Lindgren t...@atomide.com
Date: Wed, 4 Feb 2015 06:28:49 -0800
Subject: [PATCH] usb: musb: Fix use for of_property_read_bool for disabled
multipoint
The value for the multipoint dts property is ignored when parsing with
of_property_read_bool, so we currently have multipoint always set as 1
even if value 0 is specified in the dts file.
Let's fix this to read the value too instead of just the property like
the binding documentation says as otherwise MUSB will fail to work
on devices with Mentor configuration that does not support multipoint.
Cc: Brian Hutchinson b.hutch...@gmail.com
Signed-off-by: Tony Lindgren t...@atomide.com
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -687,7 +687,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
struct musb_hdrc_config *config;
struct platform_device *musb;
struct device_node *dn = parent-dev.of_node;
- int ret;
+ int ret, val;
memset(resources, 0, sizeof(resources));
res = platform_get_resource_byname(parent, IORESOURCE_MEM, mc);
@@ -739,7 +739,10 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
pdata.mode = get_musb_port_mode(dev);
/* DT keeps this entry in mA, musb expects it as per USB spec */
pdata.power = get_int_prop(dn, mentor,power) / 2;
- config-multipoint = of_property_read_bool(dn, mentor,multipoint);
+
+ ret = of_property_read_u32(dn, mentor,multipoint, val);
+ if (!ret val)
+ config-multipoint = true;
ret = platform_device_add_data(musb, pdata, sizeof(pdata));
if (ret) {
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -516,7 +516,7 @@ static int omap2430_probe(struct platform_device *pdev)
struct omap2430_glue*glue;
struct device_node *np = pdev-dev.of_node;
struct musb_hdrc_config *config;
- int ret = -ENOMEM;
+ int ret = -ENOMEM, val;
glue = devm_kzalloc(pdev-dev, sizeof(*glue), GFP_KERNEL);
if (!glue)
@@ -559,7 +559,10 @@ static int omap2430_probe(struct platform_device *pdev)
of_property_read_u32(np, num-eps, (u32 *)config-num_eps);
of_property_read_u32(np, ram-bits, (u32 *)config-ram_bits);
of_property_read_u32(np, power, (u32 *)pdata-power);
- config-multipoint = of_property_read_bool(np, multipoint);
+
+ ret = of_property_read_u32(np, multipoint, val);
+ if (!ret val)
+ config-multipoint = true;
pdata-board_data = data;
pdata-config = config;
--
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 v3 0/2] couple of generic remoteproc enhancements
Hi Suman, On Tue, Feb 3, 2015 at 10:55 PM, Suman Anna s-a...@ti.com wrote: On 01/09/2015 03:21 PM, Suman Anna wrote: Hi Ohad, The following is an updated patchset addressing the previous pending comments from v1 v2, and are rebased onto the latest 3.19-rc3 (are rc independent actually). The patches are mainly developed to support the WkupM3 remote processor driver on TI AM335x/AM437x SoCs, and I have verified the loading using the latest version of Dave's WkupM3 remoteproc work [1] The only change in v3 is on the second patch, it mainly leverages the memcpy_toio and memset_io functions for copying/loading code into the internal memory sections. An additional argument has to be added to the rproc_da_to_va function to make this distinction. Any comments on this series, or can I assume that this will make it to v3.20? ping, want to make sure this has not fallen off your radar.. It has not. I'm a bit swamped so apologies for not taking care of this faster. I believe I'll get to it next week. Thanks, Ohad. -- 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 v13 3/6] clk: Make clk API return per-user struct clk instances
On 02/05/15 12:07, Stephen Boyd wrote:
On 02/05/15 11:44, Sylwester Nawrocki wrote:
Hi Tomeu,
On 23/01/15 12:03, Tomeu Vizoso wrote:
int __clk_get(struct clk *clk)
{
- if (clk) {
- if (!try_module_get(clk-owner))
+ struct clk_core *core = !clk ? NULL : clk-core;
+
+ if (core) {
+ if (!try_module_get(core-owner))
return 0;
- kref_get(clk-ref);
+ kref_get(core-ref);
}
return 1;
}
-void __clk_put(struct clk *clk)
+static void clk_core_put(struct clk_core *core)
{
struct module *owner;
- if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
- return;
+ owner = core-owner;
clk_prepare_lock();
- owner = clk-owner;
- kref_put(clk-ref, __clk_release);
+ kref_put(core-ref, __clk_release);
clk_prepare_unlock();
module_put(owner);
}
+void __clk_put(struct clk *clk)
+{
+ if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
+ return;
+
+ clk_core_put(clk-core);
+ kfree(clk);
Why do we have kfree() here? clk_get() doesn't allocate the data structure
being freed here. What happens if we do clk_get(), clk_put(), clk_get()
on same clock?
I suspect __clk_free_clk() should be called in __clk_release() callback
instead, but then there is an issue of safely getting reference to
struct clk from struct clk_core pointer.
I tested linux-next on Odroid U3 and booting fails with oopses as below.
There is no problems when the above kfree() is commented out.
Ah now I get it. You meant to say that of_clk_get_by_clkspec() doesn't
return an allocated clk pointer. Let's fix that.
8
From: Stephen Boyd sb...@codeaurora.org
Subject: [PATCH] clkdev: Always allocate a struct clk in OF functions
of_clk_get_by_clkspec() returns a struct clk pointer but it
doesn't create a new handle for the consumers. Instead it just
returns whatever the OF clk provider hands out. Let's create a
per-user handle here so that clk_put() can properly unlink it and
free it when the consumer is done.
Fixes: 035a61c314eb clk: Make clk API return per-user struct clk instances
Reported-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
drivers/clk/clkdev.c | 44 +++-
1 file changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 29a1ab7af4b8..00d747d09b2a 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -29,15 +29,8 @@ static DEFINE_MUTEX(clocks_mutex);
#if defined(CONFIG_OF) defined(CONFIG_COMMON_CLK)
-/**
- * of_clk_get_by_clkspec() - Lookup a clock form a clock provider
- * @clkspec: pointer to a clock specifier data structure
- *
- * This function looks up a struct clk from the registered list of clock
- * providers, an input is a clock specifier data structure as returned
- * from the of_parse_phandle_with_args() function call.
- */
-struct clk *of_clk_get_by_clkspec(struct of_phandle_args *clkspec)
+static struct clk *__of_clk_get_by_clkspec(struct of_phandle_args *clkspec,
+ const char *dev_id, const char *con_id)
{
struct clk *clk;
@@ -47,6 +40,8 @@ struct clk *of_clk_get_by_clkspec(struct of_phandle_args
*clkspec)
of_clk_lock();
clk = __of_clk_get_from_provider(clkspec);
+ if (!IS_ERR(clk))
+ clk = __clk_create_clk(__clk_get_hw(clk), dev_id, con_id);
if (!IS_ERR(clk) !__clk_get(clk))
clk = ERR_PTR(-ENOENT);
Actually we can bury the __clk_create_clk() inside
__of_clk_get_from_provider(). We should also move __clk_get() into there
because right now we have a hole where whoever calls
of_clk_get_from_provider() never calls __clk_get() on the clk, leading
to possible badness. v2 coming soon.
--
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-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 v7 1/4] Documentation: dt: add common bindings for hwspinlock
On Mon, Feb 2, 2015 at 1:07 PM, Suman Anna s-a...@ti.com wrote: On 02/01/2015 11:55 AM, Bjorn Andersson wrote: On Fri, Jan 30, 2015 at 9:41 PM, Ohad Ben-Cohen o...@wizery.com wrote: On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson bj...@kryo.se wrote: In a system where you have two hwlock blocks lckA and lckB, each consisting of 8 locks and you have dspB that can only access lckB This is a good example - thanks. To be able to cope with such cases we will have to pass a hwlock block reference and its relative lock id. Correct, so the #hwlock-cells and hwlock part from the proposal are the important one. Having an optional hwlock-names will make things easier to read as well, but is not necessary. Right, if anything, it would be useful only for the clients, but the hwspinlock core itself would not need it. So, I would forgo adding the hwlock-names for now. The DT binding should definitely be prepared for such cases (just kill the base-id field?), but let's see what it means about the Linux implementation. From the dt binding PoV, we should be able to skip num-locks as well. It seems most hwlock blocks have a fixed amount of locks provided and the drivers are reporting this to the core when registering. I added this originally based on the initial MSM HW Mutex block bindings. It's not entirely correct to have this in DT for the MSM HW, as the hardware has a fixed number of mutexes. As soon as we have the binding sorted out I will follow up with a new revision of the tcsr/sfpb-mutex driver. So I think we can reduce the binding to: Providers: #hwlock-cells Consumers: hwlocks hwlock-names For the hardware where number of locks is actually variable (e.g. different variants of same block) there can be driver specific entries for this. Right, we should be able to drop this and use the driver match data. As it is, the field is used during registration of the block with the hwspinlock core. If we have certain systems where it actually is a property to be configured then they can have individual properties, extending the standard set. Either way, it's not a dynamic property shared by all hwlock drivers, so it should not be in the common binding. Will you send out a new revision of the binding? I would love to get this integrated so I can move on with the dependents. Regards, Bjorn -- 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 v7 1/4] Documentation: dt: add common bindings for hwspinlock
Hi Bjorn, On 02/05/2015 05:01 PM, Bjorn Andersson wrote: On Mon, Feb 2, 2015 at 1:07 PM, Suman Anna s-a...@ti.com wrote: On 02/01/2015 11:55 AM, Bjorn Andersson wrote: On Fri, Jan 30, 2015 at 9:41 PM, Ohad Ben-Cohen o...@wizery.com wrote: On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson bj...@kryo.se wrote: In a system where you have two hwlock blocks lckA and lckB, each consisting of 8 locks and you have dspB that can only access lckB This is a good example - thanks. To be able to cope with such cases we will have to pass a hwlock block reference and its relative lock id. Correct, so the #hwlock-cells and hwlock part from the proposal are the important one. Having an optional hwlock-names will make things easier to read as well, but is not necessary. Right, if anything, it would be useful only for the clients, but the hwspinlock core itself would not need it. So, I would forgo adding the hwlock-names for now. The DT binding should definitely be prepared for such cases (just kill the base-id field?), but let's see what it means about the Linux implementation. From the dt binding PoV, we should be able to skip num-locks as well. It seems most hwlock blocks have a fixed amount of locks provided and the drivers are reporting this to the core when registering. I added this originally based on the initial MSM HW Mutex block bindings. It's not entirely correct to have this in DT for the MSM HW, as the hardware has a fixed number of mutexes. As soon as we have the binding sorted out I will follow up with a new revision of the tcsr/sfpb-mutex driver. So I think we can reduce the binding to: Providers: #hwlock-cells Consumers: hwlocks hwlock-names For the hardware where number of locks is actually variable (e.g. different variants of same block) there can be driver specific entries for this. Right, we should be able to drop this and use the driver match data. As it is, the field is used during registration of the block with the hwspinlock core. If we have certain systems where it actually is a property to be configured then they can have individual properties, extending the standard set. Either way, it's not a dynamic property shared by all hwlock drivers, so it should not be in the common binding. Will you send out a new revision of the binding? I would love to get this integrated so I can move on with the dependents. Yep, will do as soon as I hear from Ohad on what to do with the patch hwspinlock/core: maintain a list of registered hwspinlock banks that I dropped from v7. Without that and dropping hwlock-base-id, I can't get the translations done. regards Suman -- 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 v7 1/4] Documentation: dt: add common bindings for hwspinlock
On Thu, Feb 5, 2015 at 4:11 PM, Suman Anna s-a...@ti.com wrote: Hi Bjorn, On 02/05/2015 05:01 PM, Bjorn Andersson wrote: [..] Will you send out a new revision of the binding? I would love to get this integrated so I can move on with the dependents. Yep, will do as soon as I hear from Ohad on what to do with the patch hwspinlock/core: maintain a list of registered hwspinlock banks that I dropped from v7. Without that and dropping hwlock-base-id, I can't get the translations done. My suggestion is to replace the global id-tree with a list of hwlocks and iterate over these if we ever get more than one driver registered. As long as #hwlock-drivers log(#total-hwlocks) this should be faster. I would however argue that clients that would notice any kind of difference are using the API incorrectly. Unfortunately this is a somewhat larger change than just slapping DT support on the framework :/ Regards, Bjorn -- 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 v13 3/6] clk: Make clk API return per-user struct clk instances
On Thu, Feb 05, 2015 at 02:14:01PM -0800, Stephen Boyd wrote:
Actually we can bury the __clk_create_clk() inside
__of_clk_get_from_provider(). We should also move __clk_get() into there
because right now we have a hole where whoever calls
of_clk_get_from_provider() never calls __clk_get() on the clk, leading
to possible badness. v2 coming soon.
There's some other issues here too...
sound/soc/kirkwood/kirkwood-i2s.c:
priv-clk = devm_clk_get(pdev-dev, np ? internal : NULL);
...
priv-extclk = devm_clk_get(pdev-dev, extclk);
if (IS_ERR(priv-extclk)) {
...
} else {
if (priv-extclk == priv-clk) {
devm_clk_put(pdev-dev, priv-extclk);
priv-extclk = ERR_PTR(-EINVAL);
} else {
dev_info(pdev-dev, found external clock\n);
clk_prepare_enable(priv-extclk);
soc_dai = kirkwood_i2s_dai_extclk;
}
It should be fine provided your trick is only done for DT clocks,
but not for legacy - with legacy, a NULL in the clkdev tables will
match both these requests, hence the need to compare the clk_get()
return value to tell whether we get the same clock.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
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
[PATCH] ARM: dts: Fix USB dts configuration for dm816x
Commit 7800064ba507 (ARM: dts: Add basic dm816x device tree
configuration) added basic devices for dm816x, but I was not able
to test the USB completely because of a unconfigure USB phy, and
I only tested it to make sure the Mentor chips are detected and
clocked without a phy.
After testing the USB with actual devices I noticed a few issues
that should be tested to avoid confusion:
- The USB id pin on dm8168-evm is hardwired and can be changed
only by software. As there are two USB-A type connectors, let's
start both in host mode instead of otg.
- The Mentor core is configured in such a way on dm8168-evm that
it's not capable for multipoint at least on revision c board
that I have.
- We need ranges for the syscon to properly set up the phy as
children of the SCM syscon area.
- Let's not disable the second interface, the board specific
dts files can do that if really needed. Most boards should
just keep it enabled to ensure the device is idled properly.
Note that also a phy and several musb fixes are still needed to
make the USB to work properly in addition to this fix.
Cc: Brian Hutchinson b.hutch...@gmail.com
Signed-off-by: Tony Lindgren t...@atomide.com
--- a/arch/arm/boot/dts/dm8168-evm.dts
+++ b/arch/arm/boot/dts/dm8168-evm.dts
@@ -35,6 +35,18 @@
DM816X_IOPAD(0x0aac, PIN_INPUT | MUX_MODE0) /*
SPI_D1 */
;
};
+
+ usb0_pins: pinmux_usb0_pins {
+ pinctrl-single,pins =
+ DM816X_IOPAD(0x0d00, MUX_MODE0) /*
USB0_DRVVBUS */
+ ;
+ };
+
+ usb1_pins: pinmux_usb0_pins {
+ pinctrl-single,pins =
+ DM816X_IOPAD(0x0d04, MUX_MODE0) /*
USB1_DRVVBUS */
+ ;
+ };
};
i2c1 {
@@ -127,3 +139,16 @@
mmc1 {
vmmc-supply = vmmcsd_fixed;
};
+
+/* At least dm8168-evm rev c won't support multipoint, later may */
+usb0 {
+ pinctrl-names = default;
+ pinctrl-0 = usb0_pins;
+ mentor,multipoint = 0;
+};
+
+usb1 {
+ pinctrl-names = default;
+ pinctrl-0 = usb1_pins;
+ mentor,multipoint = 0;
+};
--- a/arch/arm/boot/dts/dm816x.dtsi
+++ b/arch/arm/boot/dts/dm816x.dtsi
@@ -97,10 +97,31 @@
/* Device Configuration Registers */
scm_conf: syscon@600 {
- compatible = syscon;
+ compatible = syscon, simple-bus;
reg = 0x600 0x110;
#address-cells = 1;
#size-cells = 1;
+ ranges = 0 0x600 0x110;
+
+ usb_phy0: usb-phy@20 {
+ compatible = ti,dm8168-usb-phy;
+ reg = 0x20 0x8;
+ reg-names = phy;
+ clocks = main_fapll 6;
+ clock-names = refclk;
+ #phy-cells = 0;
+ syscon = scm_conf;
+ };
+
+ usb_phy1: usb-phy@28 {
+ compatible = ti,dm8168-usb-phy;
+ reg = 0x28 0x8;
+ reg-names = phy;
+ clocks = main_fapll 6;
+ clock-names = refclk;
+ #phy-cells = 0;
+ syscon = scm_conf;
+ };
};
scrm_clocks: clocks {
@@ -357,7 +378,10 @@
reg-names = mc, control;
interrupts = 18;
interrupt-names = mc;
- dr_mode = otg;
+ dr_mode = host;
+ interface-type = 0;
+ phys = usb_phy0;
+ phy-names = usb2-phy;
mentor,multipoint = 1;
mentor,num-eps = 16;
mentor,ram-bits = 12;
@@ -366,13 +390,15 @@
usb1: usb@47401800 {
compatible = ti,musb-am33xx;
- status = disabled;
reg = 0x47401c00 0x400
0x47401800 0x200;
reg-names = mc, control;
interrupts = 19;
interrupt-names = mc;
- dr_mode = otg;
+ dr_mode = host;
+
[PATCH 2/2] usb: musb: Fix getting a generic phy for musb_dsps
We still have a combination of legacy phys and generic phys in
use so we need to support both types of phy for musb_dsps.c.
Cc: Brian Hutchinson b.hutch...@gmail.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
drivers/usb/musb/musb_dsps.c | 17 +
1 file changed, 17 insertions(+)
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -457,12 +457,25 @@ static int dsps_musb_init(struct musb *musb)
if (IS_ERR(musb-xceiv))
return PTR_ERR(musb-xceiv);
+ musb-phy = devm_phy_get(dev-parent, usb2-phy);
+
/* Returns zero if e.g. not clocked */
rev = dsps_readl(reg_base, wrp-revision);
if (!rev)
return -ENODEV;
usb_phy_init(musb-xceiv);
+ if (IS_ERR(musb-phy)) {
+ musb-phy = NULL;
+ } else {
+ ret = phy_init(musb-phy);
+ if (ret 0)
+ return ret;
+ ret = phy_power_on(musb-phy);
+ if (ret)
+ return ret;
+ }
+
setup_timer(glue-timer, otg_timer, (unsigned long) musb);
/* Reset the musb */
@@ -502,6 +515,10 @@ static int dsps_musb_exit(struct musb *musb)
del_timer_sync(glue-timer);
usb_phy_shutdown(musb-xceiv);
+ if (musb-phy) {
+ phy_power_off(musb-phy);
+ phy_exit(musb-phy);
+ }
debugfs_remove_recursive(glue-dbgfs_root);
return 0;
--
2.1.4
--
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
[PATCH 0/2] MUSB fix for disabled multipoint and generic phy
Hi, Here are two fixes, the first one probably should be Cc stable. Regards, Tony Tony Lindgren (2): usb: musb: Fix use for of_property_read_bool for disabled multipoint usb: musb: Fix getting a generic phy for musb_dsps drivers/usb/musb/musb_dsps.c | 24 ++-- drivers/usb/musb/omap2430.c | 7 +-- 2 files changed, 27 insertions(+), 4 deletions(-) -- 2.1.4 -- 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
[PATCH 1/2] usb: musb: Fix use for of_property_read_bool for disabled multipoint
The value for the multipoint dts property is ignored when parsing with
of_property_read_bool, so we currently have multipoint always set as 1
even if value 0 is specified in the dts file.
Let's fix this to read the value too instead of just the property like
the binding documentation says as otherwise MUSB will fail to work
on devices with Mentor configuration that does not support multipoint.
Cc: Brian Hutchinson b.hutch...@gmail.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
drivers/usb/musb/musb_dsps.c | 7 +--
drivers/usb/musb/omap2430.c | 7 +--
2 files changed, 10 insertions(+), 4 deletions(-)
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -687,7 +687,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
struct musb_hdrc_config *config;
struct platform_device *musb;
struct device_node *dn = parent-dev.of_node;
- int ret;
+ int ret, val;
memset(resources, 0, sizeof(resources));
res = platform_get_resource_byname(parent, IORESOURCE_MEM, mc);
@@ -739,7 +739,10 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
pdata.mode = get_musb_port_mode(dev);
/* DT keeps this entry in mA, musb expects it as per USB spec */
pdata.power = get_int_prop(dn, mentor,power) / 2;
- config-multipoint = of_property_read_bool(dn, mentor,multipoint);
+
+ ret = of_property_read_u8(dn, mentor,multipoint, val);
+ if (!ret val)
+ config-multipoint = true;
ret = platform_device_add_data(musb, pdata, sizeof(pdata));
if (ret) {
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -516,7 +516,7 @@ static int omap2430_probe(struct platform_device *pdev)
struct omap2430_glue*glue;
struct device_node *np = pdev-dev.of_node;
struct musb_hdrc_config *config;
- int ret = -ENOMEM;
+ int ret = -ENOMEM, val;
glue = devm_kzalloc(pdev-dev, sizeof(*glue), GFP_KERNEL);
if (!glue)
@@ -559,7 +559,10 @@ static int omap2430_probe(struct platform_device *pdev)
of_property_read_u32(np, num-eps, (u32 *)config-num_eps);
of_property_read_u32(np, ram-bits, (u32 *)config-ram_bits);
of_property_read_u32(np, power, (u32 *)pdata-power);
- config-multipoint = of_property_read_bool(np, multipoint);
+
+ ret = of_property_read_u8(np, multipoint, val);
+ if (!ret val)
+ config-multipoint = true;
pdata-board_data = data;
pdata-config = config;
--
2.1.4
--
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 v13 3/6] clk: Make clk API return per-user struct clk instances
On 02/05/15 16:42, Russell King - ARM Linux wrote:
On Thu, Feb 05, 2015 at 02:14:01PM -0800, Stephen Boyd wrote:
Actually we can bury the __clk_create_clk() inside
__of_clk_get_from_provider(). We should also move __clk_get() into there
because right now we have a hole where whoever calls
of_clk_get_from_provider() never calls __clk_get() on the clk, leading
to possible badness. v2 coming soon.
There's some other issues here too...
sound/soc/kirkwood/kirkwood-i2s.c:
priv-clk = devm_clk_get(pdev-dev, np ? internal : NULL);
...
priv-extclk = devm_clk_get(pdev-dev, extclk);
if (IS_ERR(priv-extclk)) {
...
} else {
if (priv-extclk == priv-clk) {
devm_clk_put(pdev-dev, priv-extclk);
priv-extclk = ERR_PTR(-EINVAL);
} else {
dev_info(pdev-dev, found external clock\n);
clk_prepare_enable(priv-extclk);
soc_dai = kirkwood_i2s_dai_extclk;
}
It should be fine provided your trick is only done for DT clocks,
but not for legacy - with legacy, a NULL in the clkdev tables will
match both these requests, hence the need to compare the clk_get()
return value to tell whether we get the same clock.
Are we still talking about of_clk_get_from_provider()? Or are we talking
about comparing struct clk pointers? From what I can tell this code is
now broken because we made all clk getting functions (there's quite a
few...) return unique pointers every time they're called. It seems that
the driver wants to know if extclk and clk are the same so it can do
something differently in kirkwood_set_rate(). Do we need some sort of
clk_equal(struct clk *a, struct clk *b) function for drivers like this?
Also, even on DT this could fail if the DT author made internal and
extclk map to the same clock provider and cell.
--
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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Cocci] [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances
On 02/05/15 08:02, Quentin Lambert wrote: Sorry let me do that properly. On 05/02/2015 16:45, Quentin Lambert wrote: On 05/02/2015 00:26, Stephen Boyd wrote: If you want me to I can enlarge the search to other directories. Yes please do. And if you could share the coccinelle patch that would be great. Thanks. structclk.cocci is the coccinelle patch structclk-arm.patch is the result I got when applying it to the arch/arm directory Is there anything else I can do to help? These are the new instances I found by applying the patch to arch/arm directory: ./arch/arm/mach-imx/mach-imx6q.c @@ -211,7 +211,6 @@ static void __init imx6q_1588_init(void) * set bit IOMUXC_GPR1[21]. Or the PTP clock must be from pad * (external OSC), and we need to clear the bit. */ clksel = ptp_clk == enet_ref ? IMX6Q_GPR1_ENET_CLK_SEL_ANATOP : IMX6Q_GPR1_ENET_CLK_SEL_PAD; gpr = syscon_regmap_lookup_by_compatible(fsl,imx6q-iomuxc-gpr); if (!IS_ERR(gpr)) This one looks like a real problem and it could probably use a clk_equal(struct clk *a, struct clk *b) function. ./arch/arm/mach-shmobile/clock-r8a73a4.c @@ -139,7 +139,6 @@ static int pll_set_parent(struct clk *cl /* Search the parent */ for (i = 0; i clk-parent_num; i++) if (clk-parent_table[i] == parent) break; if (i == clk-parent_num) ./arch/arm/mach-shmobile/clock-sh7372.c @@ -223,7 +223,6 @@ static int pllc2_set_parent(struct clk * /* Search the parent */ for (i = 0; i clk-parent_num; i++) if (clk-parent_table[i] == parent) break; if (i == clk-parent_num) ./arch/arm/mach-shmobile/clock-r8a7740.c @@ -195,7 +195,6 @@ static int usb24s_set_parent(struct clk /* Search the parent */ for (i = 0; i clk-parent_num; i++) if (clk-parent_table[i] == parent) break; if (i == clk-parent_num) I don't think shmobile uses the CCF so this should be safe, but we should probably fix them up to also use a clk_equal() function that just does pointer comparisons. -- 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-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Cocci] [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances
On 02/05/15 07:45, Quentin Lambert wrote: On 05/02/2015 00:26, Stephen Boyd wrote: If you want me to I can enlarge the search to other directories. Yes please do. And if you could share the coccinelle patch that would be great. Thanks. structclk.cocci is the coccinelle patch structclk-arm.patch is the result I got when applying it to the arch/arm directory Is there anything else I can do to help? Thanks for the coccinelle patch. Thinking more about it, I don't think we care if the pointer is dereferenced because that would require a definition of struct clk and that is most likely not the case outside of the clock framework. Did you scan the entire kernel? I'm running it now but it seems to be taking a while. -- 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-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 2/2] of: allow for boolean flags to have value
allowing values to boolean flags lets us setup
defaults on DTSI which can get disabled later
at board DTS if, for whatever reason, board can't
use that default.
One such example is DM81xx EVM where we can't use
MUSB's multipoint feature even though SoC supports
it. Something at the board level prevents us from
using the feature.
Instead of removing multipoint; from DTSI and
adding it to all board DTS just so we can remove
it from our quirky board seems like overkill when
we could just add:
multipoint = 0;
to that quirky board's DTS.
Note that the description here is but one example
and it's likely many others have faced something
similar.
Signed-off-by: Felipe Balbi ba...@ti.com
---
include/linux/of.h | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/include/linux/of.h b/include/linux/of.h
index 76c055b20fef..c5ee9320f237 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -792,14 +792,32 @@ static inline int of_property_read_u32(const struct
device_node *np,
* @propname: name of the property to be searched.
*
* Search for a property in a device node.
- * Returns true if the property exist false otherwise.
+ * Returns true if the property exist and has a value greater than zero,
+ * fals otherwise.
*/
static inline bool of_property_read_bool(const struct device_node *np,
const char *propname)
{
struct property *prop = of_find_property(np, propname, NULL);
+ int rc;
+ u32 val;
- return prop ? true : false;
+ if (!prop)
+ return false;
+
+ rc = of_property_read_u32(np, propname, val);
+
+ /*
+* if property doesn't have a value, or prop-length == 0 and
+* we overflow, treat it as simple value-less flag.
+*/
+ if (rc == -ENODATA || rc == -EOVERFLOW)
+ return true;
+ if (WARN(rc 0, failed to read '%s' value - %d\n,
+ propname, rc))
+ return false;
+
+ return !!val;
}
static inline int of_property_read_s32(const struct device_node *np,
--
2.3.0-rc1
--
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
[RFC/PATCH 1/2] of: move of_property_read_bool further down
A follow-up patch will make use of of_property_read_u32()
to allow for boolean properties to have a value, this
is just in preparation for that patch in order to make
review easier.
Signed-off-by: Felipe Balbi ba...@ti.com
---
include/linux/of.h | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/include/linux/of.h b/include/linux/of.h
index dfde07e77a63..76c055b20fef 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -765,22 +765,6 @@ static inline int of_property_read_string_index(struct
device_node *np,
return rc 0 ? rc : 0;
}
-/**
- * of_property_read_bool - Findfrom a property
- * @np:device node from which the property value is to be read.
- * @propname: name of the property to be searched.
- *
- * Search for a property in a device node.
- * Returns true if the property exist false otherwise.
- */
-static inline bool of_property_read_bool(const struct device_node *np,
-const char *propname)
-{
- struct property *prop = of_find_property(np, propname, NULL);
-
- return prop ? true : false;
-}
-
static inline int of_property_read_u8(const struct device_node *np,
const char *propname,
u8 *out_value)
@@ -802,6 +786,22 @@ static inline int of_property_read_u32(const struct
device_node *np,
return of_property_read_u32_array(np, propname, out_value, 1);
}
+/**
+ * of_property_read_bool - Findfrom a property
+ * @np:device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ *
+ * Search for a property in a device node.
+ * Returns true if the property exist false otherwise.
+ */
+static inline bool of_property_read_bool(const struct device_node *np,
+const char *propname)
+{
+ struct property *prop = of_find_property(np, propname, NULL);
+
+ return prop ? true : false;
+}
+
static inline int of_property_read_s32(const struct device_node *np,
const char *propname,
s32 *out_value)
--
2.3.0-rc1
--
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 1/2] usb: musb: Fix use for of_property_read_bool for disabled multipoint
On Thu, Feb 05, 2015 at 08:35:12AM -0800, Tony Lindgren wrote: The value for the multipoint dts property is ignored when parsing with of_property_read_bool, so we currently have multipoint always set as 1 even if value 0 is specified in the dts file. Let's fix this to read the value too instead of just the property like the binding documentation says as otherwise MUSB will fail to work on devices with Mentor configuration that does not support multipoint. Cc: Brian Hutchinson b.hutch...@gmail.com Signed-off-by: Tony Lindgren t...@atomide.com do you mind waiting a little bit to see if my boolean properties with value patch is accepted ? http://marc.info/?l=linux-omapm=142315930232743w=2 At least let's see where the discussion moves. -- balbi signature.asc Description: Digital signature
Re: [RFC/PATCH 2/2] of: allow for boolean flags to have value
* Mark Rutland mark.rutl...@arm.com [150205 10:26]: On Thu, Feb 05, 2015 at 06:01:06PM +, Felipe Balbi wrote: allowing values to boolean flags lets us setup defaults on DTSI which can get disabled later at board DTS if, for whatever reason, board can't use that default. One such example is DM81xx EVM where we can't use MUSB's multipoint feature even though SoC supports it. Something at the board level prevents us from using the feature. Instead of removing multipoint; from DTSI and adding it to all board DTS just so we can remove it from our quirky board seems like overkill when we could just add: multipoint = 0; to that quirky board's DTS. Note that the description here is but one example and it's likely many others have faced something similar. While I appreciate that adding and removing properties in this way is painful, I think that this must be dealt with at DTB compile-time rather than kernel run-time. There are codebases other than Linux which parse DTs, and not all drivers call of_property_read_bool to parse boolean properties, an awful lot still just check of_find_property. Additionally, some bindings _explicitly_ state boolean properties are empty and have no value, which this extension would break. I think that this patch only adds to the inconsistency we currently have, and given that, I would rather not have this extension to of_property_read_bool. Arguably of_proeprty_read_bool should warn if it encounters a non-empty property. How about a WARN_ON there as in that case the value will always be set to 1 in kernel even if specified as 0 in the .dts file? Regards, Tony -- 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: [RFC/PATCH 2/2] of: allow for boolean flags to have value
On Thu, Feb 05, 2015 at 12:01:06PM -0600, Felipe Balbi wrote: allowing values to boolean flags lets us setup defaults on DTSI which can get disabled later at board DTS if, for whatever reason, board can't use that default. One such example is DM81xx EVM where we can't use MUSB's multipoint feature even though SoC supports it. Something at the board level prevents us from using the feature. Instead of removing multipoint; from DTSI and adding it to all board DTS just so we can remove it from our quirky board seems like overkill when we could just add: multipoint = 0; to that quirky board's DTS. Note that the description here is but one example and it's likely many others have faced something similar. And others even came up with solutions, too: The right thing to do in this case is /delete-property/ multipoint; This works since cd296721a964 (dtc: import latest upstream dtc) which is in 3.7-rc1. See http://permalink.gmane.org/gmane.linux.drivers.devicetree/19170 for the related discussion. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.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: [RFC/PATCH 2/2] of: allow for boolean flags to have value
On Thursday 05 February 2015 12:01:06 Felipe Balbi wrote: + /* +* if property doesn't have a value, or prop-length == 0 and +* we overflow, treat it as simple value-less flag. +*/ + if (rc == -ENODATA || rc == -EOVERFLOW) + return true; + if (WARN(rc 0, failed to read '%s' value - %d\n, + propname, rc)) + return false; I think there are drivers today that use of_property_read_bool() to check for the presence of a property that is not already empty. If the property starts with a zero cell, that would break here. Arnd -- 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: [RFC/PATCH 2/2] of: allow for boolean flags to have value
On Thu, Feb 05, 2015 at 06:01:06PM +, Felipe Balbi wrote:
allowing values to boolean flags lets us setup
defaults on DTSI which can get disabled later
at board DTS if, for whatever reason, board can't
use that default.
One such example is DM81xx EVM where we can't use
MUSB's multipoint feature even though SoC supports
it. Something at the board level prevents us from
using the feature.
Instead of removing multipoint; from DTSI and
adding it to all board DTS just so we can remove
it from our quirky board seems like overkill when
we could just add:
multipoint = 0;
to that quirky board's DTS.
Note that the description here is but one example
and it's likely many others have faced something
similar.
While I appreciate that adding and removing properties in this way is
painful, I think that this must be dealt with at DTB compile-time rather
than kernel run-time.
There are codebases other than Linux which parse DTs, and not all
drivers call of_property_read_bool to parse boolean properties, an awful
lot still just check of_find_property. Additionally, some bindings
_explicitly_ state boolean properties are empty and have no value, which
this extension would break.
I think that this patch only adds to the inconsistency we currently
have, and given that, I would rather not have this extension to
of_property_read_bool.
Arguably of_proeprty_read_bool should warn if it encounters a non-empty
property.
Thanks,
Mark.
Signed-off-by: Felipe Balbi ba...@ti.com
---
include/linux/of.h | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/include/linux/of.h b/include/linux/of.h
index 76c055b20fef..c5ee9320f237 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -792,14 +792,32 @@ static inline int of_property_read_u32(const struct
device_node *np,
* @propname:name of the property to be searched.
*
* Search for a property in a device node.
- * Returns true if the property exist false otherwise.
+ * Returns true if the property exist and has a value greater than zero,
+ * fals otherwise.
*/
static inline bool of_property_read_bool(const struct device_node *np,
const char *propname)
{
struct property *prop = of_find_property(np, propname, NULL);
+ int rc;
+ u32 val;
- return prop ? true : false;
+ if (!prop)
+ return false;
+
+ rc = of_property_read_u32(np, propname, val);
+
+ /*
+ * if property doesn't have a value, or prop-length == 0 and
+ * we overflow, treat it as simple value-less flag.
+ */
+ if (rc == -ENODATA || rc == -EOVERFLOW)
+ return true;
+ if (WARN(rc 0, failed to read '%s' value - %d\n,
+ propname, rc))
+ return false;
+
+ return !!val;
}
static inline int of_property_read_s32(const struct device_node *np,
--
2.3.0-rc1
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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 v13 3/6] clk: Make clk API return per-user struct clk instances
Hi Tomeu,
On 23/01/15 12:03, Tomeu Vizoso wrote:
int __clk_get(struct clk *clk)
{
- if (clk) {
- if (!try_module_get(clk-owner))
+ struct clk_core *core = !clk ? NULL : clk-core;
+
+ if (core) {
+ if (!try_module_get(core-owner))
return 0;
- kref_get(clk-ref);
+ kref_get(core-ref);
}
return 1;
}
-void __clk_put(struct clk *clk)
+static void clk_core_put(struct clk_core *core)
{
struct module *owner;
- if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
- return;
+ owner = core-owner;
clk_prepare_lock();
- owner = clk-owner;
- kref_put(clk-ref, __clk_release);
+ kref_put(core-ref, __clk_release);
clk_prepare_unlock();
module_put(owner);
}
+void __clk_put(struct clk *clk)
+{
+ if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
+ return;
+
+ clk_core_put(clk-core);
+ kfree(clk);
Why do we have kfree() here? clk_get() doesn't allocate the data structure
being freed here. What happens if we do clk_get(), clk_put(), clk_get()
on same clock?
I suspect __clk_free_clk() should be called in __clk_release() callback
instead, but then there is an issue of safely getting reference to
struct clk from struct clk_core pointer.
I tested linux-next on Odroid U3 and booting fails with oopses as below.
There is no problems when the above kfree() is commented out.
+}
[1.345850] Unable to handle kernel paging request at virtual address
00200200
[1.349319] pgd = c0004000
[1.352072] [00200200] *pgd=
[1.355574] Internal error: Oops: 805 [#1] PREEMPT SMP ARM
[1.361035] Modules linked in:
[1.364078] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
3.19.0-rc1-00104-ga251361a-dirty #992
[1.372405] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[1.378483] task: ee00b000 ti: ee088000 task.ti: ee088000
[1.383879] PC is at __clk_put+0x24/0xd0
[1.387773] LR is at clk_prepare_lock+0xc/0xec
[1.392198] pc : [c03eef38]lr : [c03ec1f4]psr: 2153
[1.392198] sp : ee089de8 ip : fp :
[1.403653] r10: ee02f480 r9 : 0001 r8 :
[1.408862] r7 : ee031cc0 r6 : ee089e08 r5 : r4 : ee02f480
[1.415373] r3 : 00100100 r2 : 00200200 r1 : 091e r0 : 0001
[1.421884] Flags: nzCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment
kernel
[1.429261] Control: 10c5387d Table: 4000404a DAC: 0015
[1.434989] Process swapper/0 (pid: 1, stack limit = 0xee088238)
[1.440978] Stack: (0xee089de8 to 0xee08a000)
[1.445321] 9de0: ee7c8f14 c03f0ec8 ee089e08
c0718dc8 0001
[1.453480] 9e00: c04ee0f0 ee7e0844 0001 0181 c04edb58
ee2bd320
[1.461639] 9e20: c011dc5c ee16a1e0 c0718dc8
ee16a1e0 ee2bd1e0
[1.469798] 9e40: c0641740 ee16a1e0 ee2bd320 c0718dc8 ee1d3e10
ee1d3e10
[1.477957] 9e60: c0769a88 c0718dc8 c02c3124
c02c310c ee1d3e10
[1.486117] 9e80: c07b4eec c0769a88 c02c1d0c ee1d3e10 c0769a88
ee1d3e44
[1.494276] 9ea0: c07091dc c02c1eb8 c0769a88 c02c1e2c c02c0544
ee005478 ee1676c0
[1.502435] 9ec0: c0769a88 ee3a4e80 c0760ce8 c02c150c c0669b90 c0769a88
c0746cd8 c0769a88
[1.510594] 9ee0: c0746cd8 ee2bc4c0 c0778c00 c02c24e0 c0746cd8
c0746cd8 c07091f0
[1.518753] 9f00: c0008944 c04f405c 0025 ee00b000 6153
c074ab00
[1.526913] 9f20: c074ab90 6153 ef7fca5d c050860c
00b6 c0036b88
[1.535071] 9f40: c065ecc4 c06bc728 0006 0006 c074ab30 ef7fca40
c0739bdc 0006
[1.543231] 9f60: c0718dbc c0778c00 00b6 c0718dc8 c06ed598 c06edd64
0006 0006
[1.551390] 9f80: c06ed598 c003b438 c04e64f4
[1.559549] 9fa0: c04e64fc c000e838
[1.567708] 9fc0:
[1.575867] 9fe0: 0013
c0c0c0c0 c0c0c0c0
[1.584045] [c03eef38] (__clk_put) from [c03f0ec8]
(of_clk_set_defaults+0xe0/0x2c0)
[1.592024] [c03f0ec8] (of_clk_set_defaults) from [c02c3124]
(platform_drv_probe+0x18/0xa4)
[1.600698] [c02c3124] (platform_drv_probe) from [c02c1d0c]
(driver_probe_device+0x10c/0x22c)
[1.609549] [c02c1d0c] (driver_probe_device) from [c02c1eb8]
(__driver_attach+0x8c/0x90)
[1.617968] [c02c1eb8] (__driver_attach) from [c02c0544]
(bus_for_each_dev+0x54/0x88)
[1.626128] [c02c0544] (bus_for_each_dev) from [c02c150c]
(bus_add_driver+0xd4/0x1d0)
[1.634286] [c02c150c] (bus_add_driver) from [c02c24e0]
(driver_register+0x78/0xf4)
[1.642275] [c02c24e0] (driver_register) from [c07091f0]
(fimc_md_init+0x14/0x30)
[1.650089]
Re: [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances
On 02/05/15 11:44, Sylwester Nawrocki wrote:
Hi Tomeu,
On 23/01/15 12:03, Tomeu Vizoso wrote:
int __clk_get(struct clk *clk)
{
-if (clk) {
-if (!try_module_get(clk-owner))
+struct clk_core *core = !clk ? NULL : clk-core;
+
+if (core) {
+if (!try_module_get(core-owner))
return 0;
-kref_get(clk-ref);
+kref_get(core-ref);
}
return 1;
}
-void __clk_put(struct clk *clk)
+static void clk_core_put(struct clk_core *core)
{
struct module *owner;
-if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
-return;
+owner = core-owner;
clk_prepare_lock();
-owner = clk-owner;
-kref_put(clk-ref, __clk_release);
+kref_put(core-ref, __clk_release);
clk_prepare_unlock();
module_put(owner);
}
+void __clk_put(struct clk *clk)
+{
+if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
+return;
+
+clk_core_put(clk-core);
+kfree(clk);
Why do we have kfree() here? clk_get() doesn't allocate the data structure
being freed here. What happens if we do clk_get(), clk_put(), clk_get()
on same clock?
I suspect __clk_free_clk() should be called in __clk_release() callback
instead, but then there is an issue of safely getting reference to
struct clk from struct clk_core pointer.
I tested linux-next on Odroid U3 and booting fails with oopses as below.
There is no problems when the above kfree() is commented out.
Ah now I get it. You meant to say that of_clk_get_by_clkspec() doesn't
return an allocated clk pointer. Let's fix that.
8
From: Stephen Boyd sb...@codeaurora.org
Subject: [PATCH] clkdev: Always allocate a struct clk in OF functions
of_clk_get_by_clkspec() returns a struct clk pointer but it
doesn't create a new handle for the consumers. Instead it just
returns whatever the OF clk provider hands out. Let's create a
per-user handle here so that clk_put() can properly unlink it and
free it when the consumer is done.
Fixes: 035a61c314eb clk: Make clk API return per-user struct clk instances
Reported-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
drivers/clk/clkdev.c | 44 +++-
1 file changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 29a1ab7af4b8..00d747d09b2a 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -29,15 +29,8 @@ static DEFINE_MUTEX(clocks_mutex);
#if defined(CONFIG_OF) defined(CONFIG_COMMON_CLK)
-/**
- * of_clk_get_by_clkspec() - Lookup a clock form a clock provider
- * @clkspec: pointer to a clock specifier data structure
- *
- * This function looks up a struct clk from the registered list of clock
- * providers, an input is a clock specifier data structure as returned
- * from the of_parse_phandle_with_args() function call.
- */
-struct clk *of_clk_get_by_clkspec(struct of_phandle_args *clkspec)
+static struct clk *__of_clk_get_by_clkspec(struct of_phandle_args *clkspec,
+const char *dev_id, const char *con_id)
{
struct clk *clk;
@@ -47,6 +40,8 @@ struct clk *of_clk_get_by_clkspec(struct of_phandle_args
*clkspec)
of_clk_lock();
clk = __of_clk_get_from_provider(clkspec);
+ if (!IS_ERR(clk))
+ clk = __clk_create_clk(__clk_get_hw(clk), dev_id, con_id);
if (!IS_ERR(clk) !__clk_get(clk))
clk = ERR_PTR(-ENOENT);
@@ -54,7 +49,21 @@ struct clk *of_clk_get_by_clkspec(struct of_phandle_args
*clkspec)
return clk;
}
-static struct clk *__of_clk_get(struct device_node *np, int index)
+/**
+ * of_clk_get_by_clkspec() - Lookup a clock form a clock provider
+ * @clkspec: pointer to a clock specifier data structure
+ *
+ * This function looks up a struct clk from the registered list of clock
+ * providers, an input is a clock specifier data structure as returned
+ * from the of_parse_phandle_with_args() function call.
+ */
+struct clk *of_clk_get_by_clkspec(struct of_phandle_args *clkspec)
+{
+ return __of_clk_get_by_clkspec(clkspec, NULL, __func__);
+}
+
+static struct clk *__of_clk_get(struct device_node *np, int index,
+ const char *dev_id, const char *con_id)
{
struct of_phandle_args clkspec;
struct clk *clk;
@@ -68,7 +77,7 @@ static struct clk *__of_clk_get(struct device_node *np, int
index)
if (rc)
return ERR_PTR(rc);
- clk = of_clk_get_by_clkspec(clkspec);
+ clk = __of_clk_get_by_clkspec(clkspec, dev_id, con_id);
of_node_put(clkspec.np);
return clk;
@@ -76,12 +85,7 @@ static struct clk *__of_clk_get(struct device_node *np, int
index)
struct clk *of_clk_get(struct device_node *np, int index)
{
- struct clk *clk = __of_clk_get(np, index);
-
- if (!IS_ERR(clk))
- clk =
Re: [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances
On 05/02/15 20:44, Sylwester Nawrocki wrote:
+void __clk_put(struct clk *clk)
+{
+ if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
+ return;
+
+ clk_core_put(clk-core);
+ kfree(clk);
Why do we have kfree() here? clk_get() doesn't allocate the data structure
being freed here. What happens if we do clk_get(), clk_put(), clk_get()
on same clock?
I suspect __clk_free_clk() should be called in __clk_release() callback
instead, but then there is an issue of safely getting reference to
struct clk from struct clk_core pointer.
Please ignore this comment, I missed __clk_create_clk() calls in clkdev.c
Anyway, in current -next I'm seeing random pointer dereferences while
booting Odroid U3, I'll get back to debugging this tomorrow morning.
--
Regards,
Sylwester
--
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
