* [PATCH V4 1/7] PM / OPP: Use - instead of @ for DT entries
From: Viresh Kumar @ 2017-04-20 5:44 UTC (permalink / raw)
To: arm, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-arm-kernel, linux-pm, Rafael Wysocki,
Viresh Kumar, Krzysztof Kozlowski, Masahiro Yamada, Mark Rutland,
Rob Herring, devicetree, linux-kernel
In-Reply-To: <cover.1492666725.git.viresh.kumar@linaro.org>
Compiling the DT file with W=1, DTC warns like follows:
Warning (unit_address_vs_reg): Node /opp_table0/opp@1000000000 has a
unit name, but no reg property
Fix this by replacing '@' with '-' as the OPP nodes will never have a
"reg" property.
Reported-by: Krzysztof Kozlowski <krzk@kernel.org>
Reported-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/opp/opp.txt | 38 +++++++++++++--------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 63725498bd20..e36d261b9ba6 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -186,20 +186,20 @@ Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
compatible = "operating-points-v2";
opp-shared;
- opp@1000000000 {
+ opp-1000000000 {
opp-hz = /bits/ 64 <1000000000>;
opp-microvolt = <975000 970000 985000>;
opp-microamp = <70000>;
clock-latency-ns = <300000>;
opp-suspend;
};
- opp@1100000000 {
+ opp-1100000000 {
opp-hz = /bits/ 64 <1100000000>;
opp-microvolt = <1000000 980000 1010000>;
opp-microamp = <80000>;
clock-latency-ns = <310000>;
};
- opp@1200000000 {
+ opp-1200000000 {
opp-hz = /bits/ 64 <1200000000>;
opp-microvolt = <1025000>;
clock-latency-ns = <290000>;
@@ -265,20 +265,20 @@ independently.
* independently.
*/
- opp@1000000000 {
+ opp-1000000000 {
opp-hz = /bits/ 64 <1000000000>;
opp-microvolt = <975000 970000 985000>;
opp-microamp = <70000>;
clock-latency-ns = <300000>;
opp-suspend;
};
- opp@1100000000 {
+ opp-1100000000 {
opp-hz = /bits/ 64 <1100000000>;
opp-microvolt = <1000000 980000 1010000>;
opp-microamp = <80000>;
clock-latency-ns = <310000>;
};
- opp@1200000000 {
+ opp-1200000000 {
opp-hz = /bits/ 64 <1200000000>;
opp-microvolt = <1025000>;
opp-microamp = <90000;
@@ -341,20 +341,20 @@ DVFS state together.
compatible = "operating-points-v2";
opp-shared;
- opp@1000000000 {
+ opp-1000000000 {
opp-hz = /bits/ 64 <1000000000>;
opp-microvolt = <975000 970000 985000>;
opp-microamp = <70000>;
clock-latency-ns = <300000>;
opp-suspend;
};
- opp@1100000000 {
+ opp-1100000000 {
opp-hz = /bits/ 64 <1100000000>;
opp-microvolt = <1000000 980000 1010000>;
opp-microamp = <80000>;
clock-latency-ns = <310000>;
};
- opp@1200000000 {
+ opp-1200000000 {
opp-hz = /bits/ 64 <1200000000>;
opp-microvolt = <1025000>;
opp-microamp = <90000>;
@@ -367,20 +367,20 @@ DVFS state together.
compatible = "operating-points-v2";
opp-shared;
- opp@1300000000 {
+ opp-1300000000 {
opp-hz = /bits/ 64 <1300000000>;
opp-microvolt = <1050000 1045000 1055000>;
opp-microamp = <95000>;
clock-latency-ns = <400000>;
opp-suspend;
};
- opp@1400000000 {
+ opp-1400000000 {
opp-hz = /bits/ 64 <1400000000>;
opp-microvolt = <1075000>;
opp-microamp = <100000>;
clock-latency-ns = <400000>;
};
- opp@1500000000 {
+ opp-1500000000 {
opp-hz = /bits/ 64 <1500000000>;
opp-microvolt = <1100000 1010000 1110000>;
opp-microamp = <95000>;
@@ -409,7 +409,7 @@ Example 4: Handling multiple regulators
compatible = "operating-points-v2";
opp-shared;
- opp@1000000000 {
+ opp-1000000000 {
opp-hz = /bits/ 64 <1000000000>;
opp-microvolt = <970000>, /* Supply 0 */
<960000>, /* Supply 1 */
@@ -422,7 +422,7 @@ Example 4: Handling multiple regulators
/* OR */
- opp@1000000000 {
+ opp-1000000000 {
opp-hz = /bits/ 64 <1000000000>;
opp-microvolt = <975000 970000 985000>, /* Supply 0 */
<965000 960000 975000>, /* Supply 1 */
@@ -435,7 +435,7 @@ Example 4: Handling multiple regulators
/* OR */
- opp@1000000000 {
+ opp-1000000000 {
opp-hz = /bits/ 64 <1000000000>;
opp-microvolt = <975000 970000 985000>, /* Supply 0 */
<965000 960000 975000>, /* Supply 1 */
@@ -467,7 +467,7 @@ Example 5: opp-supported-hw
status = "okay";
opp-shared;
- opp@600000000 {
+ opp-600000000 {
/*
* Supports all substrate and process versions for 0xF
* cuts, i.e. only first four cuts.
@@ -478,7 +478,7 @@ Example 5: opp-supported-hw
...
};
- opp@800000000 {
+ opp-800000000 {
/*
* Supports:
* - cuts: only one, 6th cut (represented by 6th bit).
@@ -510,7 +510,7 @@ Example 5: opp-supported-hw
compatible = "operating-points-v2";
opp-shared;
- opp@1000000000 {
+ opp-1000000000 {
opp-hz = /bits/ 64 <1000000000>;
opp-microvolt-slow = <915000 900000 925000>;
opp-microvolt-fast = <975000 970000 985000>;
@@ -518,7 +518,7 @@ Example 5: opp-supported-hw
opp-microamp-fast = <71000>;
};
- opp@1200000000 {
+ opp-1200000000 {
opp-hz = /bits/ 64 <1200000000>;
opp-microvolt-slow = <915000 900000 925000>, /* Supply vcc0 */
<925000 910000 935000>; /* Supply vcc1 */
--
2.12.0.432.g71c3a4f4ba37
^ permalink raw reply related
* Re: [PATCH V4 1/9] PM / OPP: Allow OPP table to be used for power-domains
From: Viresh Kumar @ 2017-04-20 5:25 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rafael Wysocki, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman,
Viresh Kumar, Nishanth Menon, Stephen Boyd,
linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, lina.iyer-QSEj5FYQhm4dnm+yROfE0A,
rnayak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <9dee7c0d-e5f4-9fcd-3c92-bf7ec9d43a3b-5wv7dgnIgG8@public.gmane.org>
On 19-04-17, 14:58, Sudeep Holla wrote:
> On 19/04/17 12:47, Viresh Kumar wrote:
> > On 18-04-17, 17:01, Sudeep Holla wrote:
> >> Understood. I would incline towards reusing regulators we that's what is
> >
> > It can be just a regulator, but it can be anything else as well. That
> > entity may have its own clock/volt/current tunables, etc.
> >
> >> changed behind the scene. Calling this operating performance point
> >> is misleading and doesn't align well with existing specs/features.
> >
> > Yeah, but there are no voltage levels available here and that doesn't
> > fit as a regulator then.
> >
>
> We can't dismiss just based on that. We do have systems where
> performance index is mapped to clocks though it may not be 1:1 mapping.
> I am not disagreeing here, just trying to understand it better.
@Stephen: Can you answer here please ?
> >> Understood. We have exactly same thing with SCPI but it controls both
> >> frequency and voltage referred as operating points. In general, this OPP
> >> terminology is used in SCPI/ACPI/SCMI specifications as both frequency
> >> and voltage control. I am bit worried that this binding might introduce
> >> confusions on the definitions. But it can be reworded/renamed easily if
> >> required.
> >
> > Yeah, so far we have been looking at OPPs as freq-voltage pairs ONLY
> > and that is changing. I am not sure if it going in the wrong
> > direction really. Without frequency also it is an operating point for
> > the domain. Isn't it?
> >
>
> Yes, I completely agree. I am not saying the direction is wrong. I am
> saying it's confusing and binding needs to be more clear.
What exactly isn't clear? (Yeah, there had been lots of emails and I
want to know what improvements are you looking for).
> On the contrary(playing devil's advocate here), we can treat all
> existing regulators alone as OPP then if you strip the voltages and
> treat it as abstract number.
But then we are going to have lots of platform specific code which
will program the actual hardware, etc. Which is all handled by the
regulator framework. Also note that the regulator core selects the
common voltage selected by all the children, while we want to select
the highest performance point here.
Even if we have to configure both clock and voltage for the power
domain using standard clk/regulator frameworks, OPP will work just
fine as it will do that then. So, its not that we are bypassing the
regulator framework here. It will be used if we have the voltages
available for the power-domain's performance states.
> So if the firmware handles more than just
> regulators, I agree.
I don't know the internals of that really.
> At the same time, I would have preferred firmware
> to even abstract the frequency like ACPI CPPC.
Frequency isn't required to be configured for the cases I know, but it
can be in future implementations.
> It would be good to get
> more information on what exactly that firmware handles.
@Stephen ?
> I am just more cautious here since we are designing generic bindings and
> changing generic code, we need to understand what that firmware supports
> and how it may evolve(so that we can maintain DT compatibility)
Sure, I am fine with more discussions on it :)
> I did a brief check and wanted to check if this is SMD/RPM regulators ?
Yes, Qcom calls the external core as Resource and Power manager (RPM).
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] of: introduce event tracepoints for dynamic device_node lifecyle
From: Tyrel Datwyler @ 2017-04-20 5:24 UTC (permalink / raw)
To: Steven Rostedt
Cc: Michael Ellerman, Frank Rowand, Tyrel Datwyler,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
nfont-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
mingo-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <20170419223342.0bbe2593-2kNGR76GQU9OHLTnHDQRgA@public.gmane.org>
On 04/19/2017 07:33 PM, Steven Rostedt wrote:
> On Wed, 19 Apr 2017 16:27:10 -0700
> Tyrel Datwyler <turtle.in.the.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> # echo stacktrace > /sys/kernel/debug/tracing/trace_options
>> # cat trace | grep -A6 "/pci@800000020000018"
>
> Just to let you know that there is now stacktrace event triggers, where
> you don't need to stacktrace all events, you can pick and choose. And
> even filter the stack trace on specific fields of the event.
This is great, and I did figure that out this afternoon. One thing I was
still trying to determine though was whether its possible to set these
triggers at boot? As far as I could tell I'm still limited to
"trace_options=stacktrace" as a kernel boot parameter to get the stack
for event tracepoints.
-Tyrel
>
> # cd /sys/kernel/debug/tracing
> # echo "stacktrace if common_pid == $$ && reason == 3" \
> > events/tlb/tlb_flush/trigger
>
> # cat trace
> bash-1103 [003] ...1 1290.100133: tlb_flush: pages:-1 reason:local mm shootdown (3)
> bash-1103 [003] ...2 1290.100140: <stack trace>
> => copy_process.part.39
> => _do_fork
> => SyS_clone
> => do_syscall_64
> => return_from_SYSCALL_64
>
> -- Steve
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] of: introduce event tracepoints for dynamic device_node lifecyle
From: Frank Rowand @ 2017-04-20 5:13 UTC (permalink / raw)
To: Tyrel Datwyler, Michael Ellerman, Tyrel Datwyler,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
nfont-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <58F83C66.7030806-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 04/19/17 21:43, Frank Rowand wrote:
> On 04/19/17 16:27, Tyrel Datwyler wrote:
>> On 04/18/2017 06:31 PM, Michael Ellerman wrote:
< snip >
>>
>> To get that same info as far as I know is to add a dump_stack() after
>> each pr_debug.
>
> Here is a patch that I have used. It is not as user friendly in terms
> of human readable stack traces (though a very small user space program
> should be able to fix that). The patch is cut and pasted into this
> email, so probably white space damaged.
< snip >
> +
> + if (node) {
> + int k;
> + int refcount = refcount_read(&node->kobj.kref.refcount);
> + pr_err("XXX get 0x%p %3d [0x%08lx 0x%08lx 0x%08lx 0x%08lx 0x%08lx 0x%08lx] ",
> + node, refcount,
If this was a real patch, meant for people other than myself, the
pr_err() would instead be pr_debug().
-Frank
< snip >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] of: introduce event tracepoints for dynamic device_node lifecyle
From: Frank Rowand @ 2017-04-20 4:47 UTC (permalink / raw)
To: Steven Rostedt, Tyrel Datwyler
Cc: Michael Ellerman, Tyrel Datwyler, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
nfont-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
mingo-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <20170419223342.0bbe2593-2kNGR76GQU9OHLTnHDQRgA@public.gmane.org>
On 04/19/17 19:33, Steven Rostedt wrote:
> On Wed, 19 Apr 2017 16:27:10 -0700
> Tyrel Datwyler <turtle.in.the.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> # echo stacktrace > /sys/kernel/debug/tracing/trace_options
>> # cat trace | grep -A6 "/pci@800000020000018"
>
> Just to let you know that there is now stacktrace event triggers, where
> you don't need to stacktrace all events, you can pick and choose. And
> even filter the stack trace on specific fields of the event.
>
> # cd /sys/kernel/debug/tracing
> # echo "stacktrace if common_pid == $$ && reason == 3" \
> > events/tlb/tlb_flush/trigger
>
> # cat trace
> bash-1103 [003] ...1 1290.100133: tlb_flush: pages:-1 reason:local mm shootdown (3)
> bash-1103 [003] ...2 1290.100140: <stack trace>
> => copy_process.part.39
> => _do_fork
> => SyS_clone
> => do_syscall_64
> => return_from_SYSCALL_64
>
> -- Steve
> .
>
Thanks for chiming in.
The power and flexibility of the trace tools is quite amazing
I need to make room in my schedule to catch up on what has been
added in the last several years.
-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] of: introduce event tracepoints for dynamic device_node lifecyle
From: Frank Rowand @ 2017-04-20 4:43 UTC (permalink / raw)
To: Tyrel Datwyler, Michael Ellerman, Tyrel Datwyler, robh+dt
Cc: linuxppc-dev, linux-kernel, devicetree, nfont, rostedt, mingo
In-Reply-To: <1d51a229-612b-bf09-93d5-6e43b476e2cf@gmail.com>
On 04/19/17 16:27, Tyrel Datwyler wrote:
> On 04/18/2017 06:31 PM, Michael Ellerman wrote:
>> Frank Rowand <frowand.list@gmail.com> writes:
>>
>>> On 04/17/17 17:32, Tyrel Datwyler wrote:
>>>> This patch introduces event tracepoints for tracking a device_nodes
>>>> reference cycle as well as reconfig notifications generated in response
>>>> to node/property manipulations.
>>>>
>>>> With the recent upstreaming of the refcount API several device_node
>>>> underflows and leaks have come to my attention in the pseries (DLPAR) dynamic
>>>> logical partitioning code (ie. POWER speak for hotplugging virtual and physcial
>>>> resources at runtime such as cpus or IOAs). These tracepoints provide a
>>>> easy and quick mechanism for validating the reference counting of
>>>> device_nodes during their lifetime.
>>>>
>>>> Further, when pseries lpars are migrated to a different machine we
>>>> perform a live update of our device tree to bring it into alignment with the
>>>> configuration of the new machine. The of_reconfig_notify trace point
>>>> provides a mechanism that can be turned for debuging the device tree
>>>> modifications with out having to build a custom kernel to get at the
>>>> DEBUG code introduced by commit 00aa3720.
>>>
>>> I do not like changing individual (or small groups of) printk() style
>>> debugging information to tracepoint style.
>>
>> I'm not quite sure which printks() you're referring to.
>>
>> The only printks that are removed in this series are under #ifdef DEBUG,
>> and so are essentially not there unless you build a custom kernel.
>>
>> They also only cover the reconfig case, which is actually less
>> interesting than the much more common and bug-prone get/put logic.
>>
>>> As far as I know, there is no easy way to combine trace data and printk()
>>> style data to create a single chronology of events. If some of the
>>> information needed to debug an issue is trace data and some is printk()
>>> style data then it becomes more difficult to understand the overall
>>> situation.
>>
>> If you enable CONFIG_PRINTK_TIME then you should be able to just sort
>> the trace and the printk output by the timestamp. If you're really
>> trying to correlate the two then you should probably just be using
>> trace_printk().
>>
>> But IMO this level of detail, tracing every get/put, does not belong in
>> printk. Trace points are absolutely the right solution for this type of
>> debugging.
>
> Something else to keep in mind is that while pr_debugs could be used to
> provide feedback on the reference counts and of_reconfig events they
> don't in anyway tell us where they are happening in the kernel. The
Yes, that is critical information. When there are refcount issues, the
root cause is at varying levels back in the call stack.
> trace infrastructure provides the ability to stack trace those events.
> The following example provides me a lot more information about who is
> doing what and where after I hot-add an ethernet adapter:
>
> # echo stacktrace > /sys/kernel/debug/tracing/trace_options
> # cat trace | grep -A6 "/pci@800000020000018"
> ...
> drmgr-7349 [006] d... 7138.821875: of_node_get: refcount=8,
> dn->full_name=/pci@800000020000018
> drmgr-7349 [006] d... 7138.821876: <stack trace>
> => .msi_quota_for_device
> => .rtas_setup_msi_irqs
> => .arch_setup_msi_irqs
> => .__pci_enable_msix
> => .pci_enable_msix_range
Nice! It is great to have function names in the call stack.
> --
> drmgr-7349 [006] d... 7138.821876: of_node_put: refcount=2,
> dn->full_name=/pci@800000020000018/ethernet@0
> drmgr-7349 [006] d... 7138.821877: <stack trace>
> => .msi_quota_for_device
> => .rtas_setup_msi_irqs
> => .arch_setup_msi_irqs
> => .__pci_enable_msix
> => .pci_enable_msix_range
> --
> drmgr-7349 [006] .... 7138.821878: of_node_put: refcount=7,
> dn->full_name=/pci@800000020000018
> drmgr-7349 [006] .... 7138.821879: <stack trace>
> => .rtas_setup_msi_irqs
> => .arch_setup_msi_irqs
> => .__pci_enable_msix
> => .pci_enable_msix_range
> => .bnx2x_enable_msix
> --
>
> To get that same info as far as I know is to add a dump_stack() after
> each pr_debug.
Here is a patch that I have used. It is not as user friendly in terms
of human readable stack traces (though a very small user space program
should be able to fix that). The patch is cut and pasted into this
email, so probably white space damaged.
Instead of dumping the stack, each line in the "report" contains
the top six addresses in the call stack. If interesting, they
can be post-processed (as I will show in some examples below).
---
drivers/of/dynamic.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
Index: b/drivers/of/dynamic.c
===================================================================
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -13,6 +13,7 @@
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/proc_fs.h>
+#include <linux/ftrace.h>
#include "of_private.h"
@@ -27,6 +28,20 @@ struct device_node *of_node_get(struct d
{
if (node)
kobject_get(&node->kobj);
+
+ if (node) {
+ int k;
+ int refcount = refcount_read(&node->kobj.kref.refcount);
+ pr_err("XXX get 0x%p %3d [0x%08lx 0x%08lx 0x%08lx 0x%08lx 0x%08lx 0x%08lx] ",
+ node, refcount,
+ CALLER_ADDR5, CALLER_ADDR4, CALLER_ADDR3,
+ CALLER_ADDR2, CALLER_ADDR1, CALLER_ADDR0);
+ // refcount = (refcount > 20) ? 20 : refcount; /* clamp max */
+ for (k = 0; k < refcount; k++)
+ pr_cont("+");
+ pr_cont(" %s\n", of_node_full_name(node));
+ }
+
return node;
}
EXPORT_SYMBOL(of_node_get);
@@ -38,8 +53,22 @@ EXPORT_SYMBOL(of_node_get);
*/
void of_node_put(struct device_node *node)
{
+ if (node) {
+ int k;
+ int refcount = refcount_read(&node->kobj.kref.refcount);
+ pr_err("XXX put 0x%p %3d [0x%08lx 0x%08lx 0x%08lx 0x%08lx 0x%08lx 0x%08lx] ",
+ node, refcount,
+ CALLER_ADDR5, CALLER_ADDR4, CALLER_ADDR3,
+ CALLER_ADDR2, CALLER_ADDR1, CALLER_ADDR0);
+ // refcount = (refcount > 20) ? 20 : refcount; /* clamp max */
+ for (k = 0; k < refcount; k++)
+ pr_cont("-");
+ pr_cont(" %s\n", of_node_full_name(node));
+ }
+
if (node)
kobject_put(&node->kobj);
+
}
EXPORT_SYMBOL(of_node_put);
I have used this mostly in the context of boot time, so there is a lot
of output. My notes on configuration needed for my ARM boards are:
FIXME: Currently using pr_err() so I don't need to set loglevel on boot.
So obviously not a user friendly tool!!!
The process is:
- apply patch
- configure, build, boot kernel
- analyze data
- remove patch
LOG_BUF_SHIFT (was 17)
General Setup ->
[select 21] Kernel log buffer size (16 => 64KB, 17 => 128KB)
Device Drivers ->
Device Tree and Open Firmware support ->
Device Tree overlays
Want CONFIG_FRAME_POINTER so that CALLER_ADDR* will work.
To be able to enable CONFIG_FRAME_POINTER, need to disable CONFIG_ARM_UNWIND.
Kernel hacking ->
[unselect] Enable stack unwinding support (EXPERIMENTAL)
CONFIG_FRAME_POINTER will then be selected automatically
The output looks like:
[ 0.231430] OF: XXX get 0xeefeb5dc 2 [0xc0814c58 0xc08148b0 0xc080e970 0xc080e894 0xc080e678 0xc080de7c] ++ /smp2p-adsp/slave-kernel
[ 0.231457] OF: XXX get 0xeefeb5dc 3 [0xc08103c4 0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814188] +++ /smp2p-adsp/slave-kernel
[ 0.231495] OF: XXX get 0xeefeb5dc 4 [0xc08103c4 0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814258] ++++ /smp2p-adsp/slave-kernel
[ 0.231537] OF: XXX get 0xeefeb244 4 [0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814278 0xc080ccc8] ++++ /smp2p-adsp
[ 0.231568] OF: XXX put 0xeefeb5dc 4 [0xc08103c4 0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814284] ---- /smp2p-adsp/slave-kernel
[ 0.231610] OF: XXX get 0xeefe759c 23 [0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814278 0xc080ccc8] +++++++++++++++++++++++ /
[ 0.231702] OF: XXX put 0xeefeb244 4 [0xc08103c4 0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814284] ---- /smp2p-adsp
[ 0.231744] OF: XXX put 0xeefe759c 23 [0xc08103c4 0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814284] ----------------------- /
[ 0.231881] OF: XXX get 0xeefecb2c 22 [0xc0814c58 0xc08148b0 0xc080e970 0xc080e86c 0xc080e678 0xc080de7c] ++++++++++++++++++++++ /soc/interrupt-controller@f9000000
[ 0.231972] OF: XXX put 0xeefecb2c 22 [0xc080fd94 0xc0814c58 0xc08148b0 0xc080e970 0xc080e894 0xc080e61c] ---------------------- /soc/interrupt-controller@f9000000
[ 0.232101] OF: XXX get 0xeefeb5dc 4 [0xc0814c58 0xc08148b0 0xc080e970 0xc080e894 0xc080e678 0xc080de7c] ++++ /smp2p-adsp/slave-kernel
[ 0.232134] OF: XXX put 0xeefeb5dc 4 [0xc080fd94 0xc0814c58 0xc08148b0 0xc080e970 0xc080e894 0xc080e61c] ---- /smp2p-adsp/slave-kernel
[ 0.232178] OF: XXX get 0xeefeb5dc 4 [0xc0814c58 0xc08148b0 0xc080e970 0xc080e894 0xc080e678 0xc080de7c] ++++ /smp2p-adsp/slave-kernel
[ 0.232211] OF: XXX get 0xeefeb5dc 5 [0xc08103c4 0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814188] +++++ /smp2p-adsp/slave-kernel
[ 0.232257] OF: XXX get 0xeefeb5dc 6 [0xc08103c4 0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814258] ++++++ /smp2p-adsp/slave-kernel
[ 0.232308] OF: XXX get 0xeefeb244 4 [0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814278 0xc080ccc8] ++++ /smp2p-adsp
[ 0.232339] OF: XXX put 0xeefeb5dc 6 [0xc08103c4 0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814284] ------ /smp2p-adsp/slave-kernel
[ 0.232390] OF: XXX get 0xeefe759c 23 [0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814278 0xc080ccc8] +++++++++++++++++++++++ /
[ 0.232482] OF: XXX put 0xeefeb244 4 [0xc08103c4 0xc0810024 0xc080fd94 0xc0814c58 0xc08149a8 0xc0814284] ---- /smp2p-adsp
But I normally strip off the timestamp, and grep for the "OF: XXX ",
which gets me only the get and put info. It is also easy to grep
for a single node of interest.
The data fields are:
get or put
the struct device_node address
refcount
a 6 caller deep call stack
for get, one '+' per refcount or for put, one '-' per refcount
the full node name
The refcount for get is the post get value, for put is the pre put value,
so they are easy to match up for human scanning. The length of the "++++"
and "----" patterns on the end are also intended for easy human scanning.
Here are two actual refcount issues for the root node on my 4.11-rc1:
OF: XXX get 0xeefe759c 2 [0xc08138bc 0xc08137fc 0xc08136e4 0xc08136b4 0xc0813070 0xc080ccc8] ++ /
OF: XXX put 0xeefe759c 2 [0xc031aa28 0xc08138bc 0xc08137fc 0xc08136e4 0xc08136b4 0xc0813014] -- /
OF: XXX get 0xeefe759c 2 [0xc08138bc 0xc08137fc 0xc08136e4 0xc08136b4 0xc0813070 0xc080ccc8] ++ /
OF: XXX put 0xeefe759c 2 [0xc031aa40 0xc08138bc 0xc08137fc 0xc08136e4 0xc08136b4 0xc0813014] -- /
OF: XXX get 0xeefe759c 22 [0xc0308518 0xc09330e0 0xc0d00e3c 0xc03017d0 0xc0d3a1fc 0xc080d948] ++++++++++++++++++++++ /
OF: XXX put 0xeefe759c 22 [0xc0308518 0xc09330e0 0xc0d00e3c 0xc03017d0 0xc0d3a1fc 0xc080d8c8] ---------------------- /
OF: XXX get 0xeefe759c 22 [0xc0d00e3c 0xc03017d0 0xc0d3a234 0xc0810684 0xc081061c 0xc080d928] ++++++++++++++++++++++ /
OF: XXX get 0xeefe759c 23 [0xc08103c4 0xc0810024 0xc080fd84 0xc08137b4 0xc0812c88 0xc080ccc8] +++++++++++++++++++++++ /
OF: XXX put 0xeefe759c 23 [0xc08105d8 0xc08103c4 0xc0810024 0xc080fd84 0xc08137b4 0xc0812cb4] ----------------------- /
The call stack could easily be post-processed, for example using addr2line.
Here is the call stack for when the refcount incremented to 23 from 22 (or
more accurately, to 22 from 21):
0xc0d00e3c Line 857 of "init/main.c"
0xc03017d0 Line 792 of "init/main.c"
0xc0d3a234 Line 528 of "drivers/of/platform.c"
0xc0810684 Line 503 of "drivers/of/platform.c"
0xc081061c Line 267 of "include/linux/of.h"
0xc080d928 Line 815 of "drivers/of/base.c"
Which ends up being this code:
of_platform_default_populate_init()
of_platform_default_populate()
of_platform_populate()
[[ of_find_node_by_path("/") ]]
[[ of_find_node_opts_by_path(path, NULL) ]]
of_node_get(of_root)
Note that some functions can be left out of the ARM call stack, with
a return going back more than one level. The functions in the call
list above that are enclosed in '[[' and ']]' were found by source
inspection in those cases.
It looks like a put is missing, but about 250 get/put pairs later,
of_platform_populate() does the required put on node "/".
Then quite a bit later, after lots of balanced gets and puts, there is an
initcall that does a get on the root without a corresponding put.
The jump from refcount 2 to refcount 22 is an interesting case, insofar as it
is not the result of of_node_get(). It is instead inside a series of calls to
kobject_add():
kernel_init()
kernel_init_freeable()
do_basic_setup()
driver_init()
of_core_init()
for_each_of_allnodes(np)
__of_attach_node_sysfs(np)
kobject_add()
Filtering and reformatting is "easily done" with grep and other
normal unix tools.
For example, a simple stream of command line tools can show a
streamlined report of the refcounts of a single node (in this
case the root node), which can easily be scanned for interesting
events:
[ 0.199569] 2 ++ /
[ 0.199629] 2 -- /
[ 0.199826] 2 ++ /
[ 0.199886] 2 -- /
[ 0.212549] 22 ++++++++++++++++++++++ /
[ 0.212855] 22 ---------------------- /
[ 0.213087] 22 ++++++++++++++++++++++ /
[ 0.213700] 23 +++++++++++++++++++++++ /
[ 0.213797] 23 ----------------------- /
[ 0.213973] 23 +++++++++++++++++++++++ /
... hundreds of boring put/get pairs
[ 0.458737] 23 ----------------------- /
[ 0.458909] 23 +++++++++++++++++++++++ /
[ 0.459035] 23 ----------------------- /
[ 0.459305] 22 ---------------------- /
[ 0.470255] 22 ++++++++++++++++++++++ /
... hundreds of boring put/get pairs
[ 93.110548] 22 ++++++++++++++++++++++ /
[ 93.140046] 22 ---------------------- /
[ 93.264639] 22 ++++++++++++++++++++++ /
[ 93.389530] 23 +++++++++++++++++++++++ /
[ 93.414269] 23 ----------------------- /
You might have noticed that the call trace is not interesting for
most of the gets and puts. There are over 350 get/put pairs for
the root node in the boot that I collected the above examples on,
but only a few instances where the trace matters. Thus leaving
the call stack in a compact format until needed is a feature.
I will be the first to admit that the tool is not polished and not
easy to use, though it is easily extended with post-processing.
I wrote the patch as a proof of concept a while ago and have not
fleshed it out. In fact, calling it a "tool" is overstating what
it is.
> Further, filters can be set on the tracepoint event fields such that
> trace data could be restricted to a particular device_node or refcount
> threshold. For example:
>
> # cd /sys/kernel/debug/tracing# cat events/of/of_node_get/format
> # echo "dn_name == /pci@800000020000018" > events/of/filter
>
> # cat trace
> drmgr-10542 [003] .... 9630.677001: of_node_put: refcount=5,
> dn->full_name=/pci@800000020000018
> drmgr-10542 [003] d... 9631.677368: of_node_get: refcount=6,
> dn->full_name=/pci@800000020000018
> drmgr-10542 [003] .... 9631.677389: of_node_put: refcount=5,
> dn->full_name=/pci@800000020000018
> drmgr-10542 [003] .... 9631.677390: of_reconfig_notify:
> action=DETACH_NODE, dn->full_name=/pci@800000020000018, prop->name=null,
> old_prop->name=null
> drmgr-10542 [003] .n.. 9632.025656: of_node_put: refcount=4,
> dn->full_name=/pci@800000020000018
> drmgr-10542 [003] .n.. 9632.025657: of_node_put: refcount=3,
> dn->full_name=/pci@800000020000018
>
> After setting the filter and doing a hot-remove of the pci device in
> question the trace quickly tells me 3 references are being leaked. In
> combination with the stacktrace option I can quickly correlate call
> sites that take references without releasing them.
Thanks for sharing that. It is nice seeing your results.
> -Tyrel
>
>>
>> cheers
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
^ permalink raw reply
* [PATCH] arm64: dts: freescale: ls2081ardb: Add DTS support for NXP LS2081ARDB
From: Priyanka Jain @ 2017-04-20 4:37 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, shawnguo-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
leoyang.li-3arQi8VN3Tc, Priyanka Jain
This patch add support for NXP LS2081ARDB board which has
LS2081A SoC.
LS2081A SoC is 40-pin derivative of LS2088A SoC
So, from functional perspective both are same.
Hence,ls2088a SoC dtsi files are included from ls2081ARDB dts
Signed-off-by: Priyanka Jain <priyanka.jain-3arQi8VN3Tc@public.gmane.org>
---
arch/arm64/boot/dts/freescale/Makefile | 1 +
arch/arm64/boot/dts/freescale/fsl-ls2081a-rdb.dts | 139 +++++++++++++++++++++
2 files changed, 140 insertions(+), 0 deletions(-)
create mode 100755 arch/arm64/boot/dts/freescale/fsl-ls2081a-rdb.dts
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 72c4b52..58b80de 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -9,6 +9,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1088a-qds.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1088a-rdb.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2080a-qds.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2080a-rdb.dtb
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2081a-rdb.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2080a-simu.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2088a-qds.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2088a-rdb.dtb
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2081a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls2081a-rdb.dts
new file mode 100444
index 0000000..7ae408e
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2081a-rdb.dts
@@ -0,0 +1,139 @@
+/*
+ * Device Tree file for NXP LS2081A RDB Board.
+ *
+ * Copyright (C) 2017, NXP Semiconductors
+ *
+ * Priyanka Jain <priyanka.jain-3arQi8VN3Tc@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+
+#include "fsl-ls2088a.dtsi"
+
+/ {
+ model = "NXP Layerscape 2081A RDB Board";
+ compatible = "fsl,ls2081a-rdb", "fsl,ls2081a";
+
+ aliases {
+ serial0 = &serial0;
+ serial1 = &serial1;
+ };
+
+ chosen {
+ stdout-path = "serial1:115200n8";
+ };
+};
+
+&esdhc {
+ status = "okay";
+};
+
+&ifc {
+ status = "disabled";
+};
+
+&i2c0 {
+ status = "okay";
+ pca9547@75 {
+ compatible = "nxp,pca9547";
+ reg = <0x75>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ i2c@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x01>;
+ rtc@51 {
+ compatible = "nxp,pcf2129";
+ reg = <0x51>;
+ };
+ };
+
+ i2c@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x3>;
+
+ adt7481@4c {
+ compatible = "adi,adt7461";
+ reg = <0x4c>;
+ };
+ };
+ };
+};
+
+&dspi {
+ status = "okay";
+ dflash0: n25q512a {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "st,m25p80";
+ spi-max-frequency = <3000000>;
+ reg = <0>;
+ };
+};
+
+
+&qspi {
+ status = "okay";
+ flash0: n25q512a@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "st,m25p80";
+ spi-max-frequency = <20000000>;
+ reg = <0>;
+ };
+ flash1: n25q512a@1 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "st,m25p80";
+ spi-max-frequency = <20000000>;
+ reg = <0>;
+ };
+};
+
+&usb0 {
+ status = "okay";
+};
+
+&usb1 {
+ status = "okay";
+};
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] of: introduce event tracepoints for dynamic device_node lifecyle
From: Frank Rowand @ 2017-04-20 2:37 UTC (permalink / raw)
To: Tyrel Datwyler, Steven Rostedt
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
nfont-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
mpe-Gsx/Oe8HsFggBc27wqDAHg, mingo-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <641aa8ee-9b54-716a-77a1-076cafb95e3a-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
On 04/19/17 11:45, Tyrel Datwyler wrote:
> On 04/18/2017 07:49 PM, Steven Rostedt wrote:
>> On Tue, 18 Apr 2017 18:42:32 -0700
>> Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> And of course the other issue with using tracepoints is the extra space
>>> required to hold the tracepoint info. With the pr_debug() approach, the
>>> space usage can be easily removed for a production kernel via a config
>>> option.
>>
>> Now if you are saying you want to be able to enable debugging without
>> the tracing infrastructure I would agree. As the tracing infrastructure
>> is large. But I'm working on shrinking it more.
>
> The primary consumers of OF_DYNAMIC seem to be pseries and powernv where
> we are generally going to see the trace infrastructure enabled by
> default in production.
Another primary consumer will be overlays for ARM expansion boards. Still
a work in progress.
-Frank
>
> -Tyrel
>
>>
>>>
>>> Tracepoints are wonderful technology, but not always the proper tool to
>>> use for debug info.
>>
>> But if you are going to have tracing enabled regardless, adding a few
>> more tracepoints isn't going to make the difference.
>>
>> -- Steve
>>
>>>
>>>> If Rob wants to convert printk() style data to trace data (and I can't
>>>> convince him otherwise) then I will have further comments on this specific
>>>> patch.
>>>>
>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] of: introduce event tracepoints for dynamic device_node lifecyle
From: Steven Rostedt @ 2017-04-20 2:33 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: Michael Ellerman, Frank Rowand, Tyrel Datwyler,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
nfont-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
mingo-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1d51a229-612b-bf09-93d5-6e43b476e2cf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Wed, 19 Apr 2017 16:27:10 -0700
Tyrel Datwyler <turtle.in.the.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> # echo stacktrace > /sys/kernel/debug/tracing/trace_options
> # cat trace | grep -A6 "/pci@800000020000018"
Just to let you know that there is now stacktrace event triggers, where
you don't need to stacktrace all events, you can pick and choose. And
even filter the stack trace on specific fields of the event.
# cd /sys/kernel/debug/tracing
# echo "stacktrace if common_pid == $$ && reason == 3" \
> events/tlb/tlb_flush/trigger
# cat trace
bash-1103 [003] ...1 1290.100133: tlb_flush: pages:-1 reason:local mm shootdown (3)
bash-1103 [003] ...2 1290.100140: <stack trace>
=> copy_process.part.39
=> _do_fork
=> SyS_clone
=> do_syscall_64
=> return_from_SYSCALL_64
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v7 9/9] ASoC: add audio-graph-card support
From: Kuninori Morimoto @ 2017-04-20 1:36 UTC (permalink / raw)
To: Mark Brown, Rob Herring; +Cc: Linux-ALSA, Simon, Linux-DT
In-Reply-To: <87fuh3oon8.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
OF-graph base DT binding are used on V4L2, and ALSA SoC is using
different style of DT today. Now ALSA SoC supports simple-card driver
for generic/simple sound card.
In the future, V4L2 / ALSA will support HDMI, and then, DT bindings
between V4L2 / ALSA should be merged.
This patch adds new Audio Graph Card which is OF-graph base of
simple-card
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v6 -> v7
- no change
sound/soc/generic/Kconfig | 8 +
sound/soc/generic/Makefile | 2 +
sound/soc/generic/audio-graph-card.c | 308 +++++++++++++++++++++++++++++++++++
3 files changed, 318 insertions(+)
create mode 100644 sound/soc/generic/audio-graph-card.c
diff --git a/sound/soc/generic/Kconfig b/sound/soc/generic/Kconfig
index d023959..121a48e 100644
--- a/sound/soc/generic/Kconfig
+++ b/sound/soc/generic/Kconfig
@@ -14,3 +14,11 @@ config SND_SIMPLE_SCU_CARD
help
This option enables generic simple SCU sound card support.
It supports DPCM of multi CPU single Codec system.
+
+config SND_AUDIO_GRAPH_CARD
+ tristate "ASoC Audio Graph sound card support"
+ depends on OF
+ select SND_SIMPLE_CARD_UTILS
+ help
+ This option enables generic simple simple sound card support
+ with OF-graph DT bindings.
diff --git a/sound/soc/generic/Makefile b/sound/soc/generic/Makefile
index ee750f3..670068f 100644
--- a/sound/soc/generic/Makefile
+++ b/sound/soc/generic/Makefile
@@ -1,7 +1,9 @@
snd-soc-simple-card-utils-objs := simple-card-utils.o
snd-soc-simple-card-objs := simple-card.o
snd-soc-simple-scu-card-objs := simple-scu-card.o
+snd-soc-audio-graph-card-objs := audio-graph-card.o
obj-$(CONFIG_SND_SIMPLE_CARD_UTILS) += snd-soc-simple-card-utils.o
obj-$(CONFIG_SND_SIMPLE_CARD) += snd-soc-simple-card.o
obj-$(CONFIG_SND_SIMPLE_SCU_CARD) += snd-soc-simple-scu-card.o
+obj-$(CONFIG_SND_AUDIO_GRAPH_CARD) += snd-soc-audio-graph-card.o
diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
new file mode 100644
index 0000000..07e010d
--- /dev/null
+++ b/sound/soc/generic/audio-graph-card.c
@@ -0,0 +1,308 @@
+/*
+ * ASoC audio graph sound card support
+ *
+ * Copyright (C) 2016 Renesas Solutions Corp.
+ * Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
+ *
+ * based on ${LINUX}/sound/soc/generic/simple-card.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/string.h>
+#include <sound/jack.h>
+#include <sound/simple_card_utils.h>
+
+struct graph_card_data {
+ struct snd_soc_card snd_card;
+ struct graph_dai_props {
+ struct asoc_simple_dai cpu_dai;
+ struct asoc_simple_dai codec_dai;
+ } *dai_props;
+ struct snd_soc_dai_link *dai_link;
+};
+
+#define graph_priv_to_card(priv) (&(priv)->snd_card)
+#define graph_priv_to_props(priv, i) ((priv)->dai_props + (i))
+#define graph_priv_to_dev(priv) (graph_priv_to_card(priv)->dev)
+#define graph_priv_to_link(priv, i) (graph_priv_to_card(priv)->dai_link + (i))
+
+static int asoc_graph_card_startup(struct snd_pcm_substream *substream)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct graph_card_data *priv = snd_soc_card_get_drvdata(rtd->card);
+ struct graph_dai_props *dai_props = graph_priv_to_props(priv, rtd->num);
+ int ret;
+
+ ret = clk_prepare_enable(dai_props->cpu_dai.clk);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(dai_props->codec_dai.clk);
+ if (ret)
+ clk_disable_unprepare(dai_props->cpu_dai.clk);
+
+ return ret;
+}
+
+static void asoc_graph_card_shutdown(struct snd_pcm_substream *substream)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct graph_card_data *priv = snd_soc_card_get_drvdata(rtd->card);
+ struct graph_dai_props *dai_props = graph_priv_to_props(priv, rtd->num);
+
+ clk_disable_unprepare(dai_props->cpu_dai.clk);
+
+ clk_disable_unprepare(dai_props->codec_dai.clk);
+}
+
+static struct snd_soc_ops asoc_graph_card_ops = {
+ .startup = asoc_graph_card_startup,
+ .shutdown = asoc_graph_card_shutdown,
+};
+
+static int asoc_graph_card_dai_init(struct snd_soc_pcm_runtime *rtd)
+{
+ struct graph_card_data *priv = snd_soc_card_get_drvdata(rtd->card);
+ struct snd_soc_dai *codec = rtd->codec_dai;
+ struct snd_soc_dai *cpu = rtd->cpu_dai;
+ struct graph_dai_props *dai_props =
+ graph_priv_to_props(priv, rtd->num);
+ int ret;
+
+ ret = asoc_simple_card_init_dai(codec, &dai_props->codec_dai);
+ if (ret < 0)
+ return ret;
+
+ ret = asoc_simple_card_init_dai(cpu, &dai_props->cpu_dai);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int asoc_graph_card_dai_link_of(struct device_node *cpu_port,
+ struct graph_card_data *priv,
+ int idx)
+{
+ struct device *dev = graph_priv_to_dev(priv);
+ struct snd_soc_dai_link *dai_link = graph_priv_to_link(priv, idx);
+ struct graph_dai_props *dai_props = graph_priv_to_props(priv, idx);
+ struct asoc_simple_dai *cpu_dai = &dai_props->cpu_dai;
+ struct asoc_simple_dai *codec_dai = &dai_props->codec_dai;
+ struct snd_soc_card *card = graph_priv_to_card(priv);
+ struct device_node *cpu_ep = of_get_next_child(cpu_port, NULL);
+ struct device_node *codec_ep = of_graph_get_remote_endpoint(cpu_ep);
+ struct device_node *rcpu_ep = of_graph_get_remote_endpoint(codec_ep);
+ int ret;
+
+ if (rcpu_ep != cpu_ep) {
+ dev_err(dev, "remote-endpoint missmatch (%s/%s/%s)\n",
+ cpu_ep->name, codec_ep->name, rcpu_ep->name);
+ ret = -EINVAL;
+ goto dai_link_of_err;
+ }
+
+ ret = asoc_simple_card_parse_daifmt(dev, cpu_ep, codec_ep,
+ NULL, &dai_link->dai_fmt);
+ if (ret < 0)
+ goto dai_link_of_err;
+
+ /*
+ * we need to consider "mclk-fs" around here
+ * see simple-card
+ */
+
+ ret = asoc_simple_card_parse_graph_cpu(cpu_ep, dai_link);
+ if (ret < 0)
+ goto dai_link_of_err;
+
+ ret = asoc_simple_card_parse_graph_codec(codec_ep, dai_link);
+ if (ret < 0)
+ goto dai_link_of_err;
+
+ ret = snd_soc_of_parse_tdm_slot(cpu_ep,
+ &cpu_dai->tx_slot_mask,
+ &cpu_dai->rx_slot_mask,
+ &cpu_dai->slots,
+ &cpu_dai->slot_width);
+ if (ret < 0)
+ goto dai_link_of_err;
+
+ ret = snd_soc_of_parse_tdm_slot(codec_ep,
+ &codec_dai->tx_slot_mask,
+ &codec_dai->rx_slot_mask,
+ &codec_dai->slots,
+ &codec_dai->slot_width);
+ if (ret < 0)
+ goto dai_link_of_err;
+
+ ret = asoc_simple_card_parse_clk_cpu(dev, cpu_ep, dai_link, cpu_dai);
+ if (ret < 0)
+ goto dai_link_of_err;
+
+ ret = asoc_simple_card_parse_clk_codec(dev, codec_ep, dai_link, codec_dai);
+ if (ret < 0)
+ goto dai_link_of_err;
+
+ ret = asoc_simple_card_canonicalize_dailink(dai_link);
+ if (ret < 0)
+ goto dai_link_of_err;
+
+ ret = asoc_simple_card_set_dailink_name(dev, dai_link,
+ "%s-%s",
+ dai_link->cpu_dai_name,
+ dai_link->codec_dai_name);
+ if (ret < 0)
+ goto dai_link_of_err;
+
+ dai_link->ops = &asoc_graph_card_ops;
+ dai_link->init = asoc_graph_card_dai_init;
+
+ dev_dbg(dev, "\tname : %s\n", dai_link->stream_name);
+ dev_dbg(dev, "\tformat : %04x\n", dai_link->dai_fmt);
+ dev_dbg(dev, "\tcpu : %s / %d\n",
+ dai_link->cpu_dai_name,
+ cpu_dai->sysclk);
+ dev_dbg(dev, "\tcodec : %s / %d\n",
+ dai_link->codec_dai_name,
+ codec_dai->sysclk);
+
+ asoc_simple_card_canonicalize_cpu(dai_link,
+ card->num_links == 1);
+
+dai_link_of_err:
+ of_node_put(cpu_ep);
+ of_node_put(rcpu_ep);
+ of_node_put(codec_ep);
+
+ return ret;
+}
+
+static int asoc_graph_card_parse_of(struct graph_card_data *priv)
+{
+ struct of_phandle_iterator it;
+ struct device *dev = graph_priv_to_dev(priv);
+ struct snd_soc_card *card = graph_priv_to_card(priv);
+ struct device_node *node = dev->of_node;
+ int rc, idx = 0;
+ int ret;
+
+ /*
+ * we need to consider "widgets", "routing", "mclk-fs" around here
+ * see simple-card
+ */
+
+ of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
+ ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
+ of_node_put(it.node);
+ if (ret < 0)
+ return ret;
+ }
+
+ return asoc_simple_card_parse_card_name(card, NULL);
+}
+
+static int asoc_graph_get_dais_count(struct device *dev)
+{
+ struct of_phandle_iterator it;
+ struct device_node *node = dev->of_node;
+ int count = 0;
+ int rc;
+
+ of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
+ count++;
+ of_node_put(it.node);
+ }
+
+ return count;
+}
+
+static int asoc_graph_card_probe(struct platform_device *pdev)
+{
+ struct graph_card_data *priv;
+ struct snd_soc_dai_link *dai_link;
+ struct graph_dai_props *dai_props;
+ struct device *dev = &pdev->dev;
+ struct snd_soc_card *card;
+ int num, ret;
+
+ /* Allocate the private data and the DAI link array */
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ num = asoc_graph_get_dais_count(dev);
+ if (num == 0)
+ return -EINVAL;
+
+ dai_props = devm_kzalloc(dev, sizeof(*dai_props) * num, GFP_KERNEL);
+ dai_link = devm_kzalloc(dev, sizeof(*dai_link) * num, GFP_KERNEL);
+ if (!dai_props || !dai_link)
+ return -ENOMEM;
+
+ priv->dai_props = dai_props;
+ priv->dai_link = dai_link;
+
+ /* Init snd_soc_card */
+ card = graph_priv_to_card(priv);
+ card->owner = THIS_MODULE;
+ card->dev = dev;
+ card->dai_link = dai_link;
+ card->num_links = num;
+
+ ret = asoc_graph_card_parse_of(priv);
+ if (ret < 0) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "parse error %d\n", ret);
+ goto err;
+ }
+
+ snd_soc_card_set_drvdata(card, priv);
+
+ ret = devm_snd_soc_register_card(dev, card);
+ if (ret >= 0)
+ return ret;
+err:
+ asoc_simple_card_clean_reference(card);
+
+ return ret;
+}
+
+static int asoc_graph_card_remove(struct platform_device *pdev)
+{
+ struct snd_soc_card *card = platform_get_drvdata(pdev);
+
+ return asoc_simple_card_clean_reference(card);
+}
+
+static const struct of_device_id asoc_graph_of_match[] = {
+ { .compatible = "audio-graph-card", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, asoc_graph_of_match);
+
+static struct platform_driver asoc_graph_card = {
+ .driver = {
+ .name = "asoc-audio-graph-card",
+ .of_match_table = asoc_graph_of_match,
+ },
+ .probe = asoc_graph_card_probe,
+ .remove = asoc_graph_card_remove,
+};
+module_platform_driver(asoc_graph_card);
+
+MODULE_ALIAS("platform:asoc-audio-graph-card");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ASoC Audio Graph Sound Card");
+MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>");
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 8/9] ASoC: add audio-graph-card document
From: Kuninori Morimoto @ 2017-04-20 1:35 UTC (permalink / raw)
To: Mark Brown, Rob Herring; +Cc: Linux-ALSA, Simon, Linux-DT
In-Reply-To: <87fuh3oon8.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
"Audio Graph Card" = "Simple Card" + "OF-graph"
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v6 -> v7
- no change
.../devicetree/bindings/sound/audio-graph-card.txt | 124 +++++++++++++++++++++
1 file changed, 124 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/audio-graph-card.txt
diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card.txt b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
new file mode 100644
index 0000000..bac4b1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
@@ -0,0 +1,124 @@
+Audio Graph Card:
+
+Audio Graph Card specifies audio DAI connections of SoC <-> codec.
+It is based on common bindings for device graphs.
+see ${LINUX}/Documentation/devicetree/bindings/graph.txt
+
+Basically, Audio Graph Card property is same as Simple Card.
+see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.txt
+
+Below are same as Simple-Card.
+
+- label
+- dai-format
+- frame-master
+- bitclock-master
+- bitclock-inversion
+- frame-inversion
+- dai-tdm-slot-num
+- dai-tdm-slot-width
+- clocks / system-clock-frequency
+
+Required properties:
+
+- compatible : "audio-graph-card";
+- dais : list of CPU DAI port{s}
+
+Example: Single DAI case
+
+ sound_card {
+ compatible = "audio-graph-card";
+
+ dais = <&cpu_port>;
+ };
+
+ dai-controller {
+ ...
+ cpu_port: port {
+ cpu_endpoint: endpoint {
+ remote-endpoint = <&codec_endpoint>;
+
+ dai-format = "left_j";
+ ...
+ };
+ };
+ };
+
+ audio-codec {
+ ...
+ port {
+ codec_endpoint: endpoint {
+ remote-endpoint = <&cpu_endpoint>;
+ };
+ };
+ };
+
+Example: Multi DAI case
+
+ sound-card {
+ compatible = "audio-graph-card";
+
+ label = "sound-card";
+
+ dais = <&cpu_port0
+ &cpu_port1
+ &cpu_port2>;
+ };
+
+ audio-codec@0 {
+ ...
+ port {
+ codec0_endpoint: endpoint {
+ remote-endpoint = <&cpu_endpoint0>;
+ };
+ };
+ };
+
+ audio-codec@1 {
+ ...
+ port {
+ codec1_endpoint: endpoint {
+ remote-endpoint = <&cpu_endpoint1>;
+ };
+ };
+ };
+
+ audio-codec@2 {
+ ...
+ port {
+ codec2_endpoint: endpoint {
+ remote-endpoint = <&cpu_endpoint2>;
+ };
+ };
+ };
+
+ dai-controller {
+ ...
+ ports {
+ cpu_port0: port@0 {
+ cpu_endpoint0: endpoint {
+ remote-endpoint = <&codec0_endpoint>;
+
+ dai-format = "left_j";
+ ...
+ };
+ };
+ cpu_port1: port@1 {
+ cpu_endpoint1: endpoint {
+ remote-endpoint = <&codec1_endpoint>;
+
+ dai-format = "i2s";
+ ...
+ };
+ };
+ cpu_port2: port@2 {
+ cpu_endpoint2: endpoint {
+ remote-endpoint = <&codec2_endpoint>;
+
+ dai-format = "i2s";
+ ...
+ };
+ };
+ };
+ };
+
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 7/9] ASoC: simple-card-utils: add asoc_simple_card_parse_graph_dai()
From: Kuninori Morimoto @ 2017-04-20 1:35 UTC (permalink / raw)
To: Mark Brown, Rob Herring; +Cc: Linux-ALSA, Simon, Linux-DT
In-Reply-To: <87fuh3oon8.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
simple-card already has asoc_simple_card_parse_dai(),
but graph base parsing needs graph specific version of it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v6 -> v7
- no change
include/sound/simple_card_utils.h | 10 ++++++
sound/soc/generic/simple-card-utils.c | 57 +++++++++++++++++++++++++++++++++++
2 files changed, 67 insertions(+)
diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index af58d23..efab584 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -60,6 +60,16 @@ int asoc_simple_card_parse_dai(struct device_node *node,
const char *cells_name,
int *is_single_links);
+#define asoc_simple_card_parse_graph_cpu(ep, dai_link) \
+ asoc_simple_card_parse_graph_dai(ep, &dai_link->cpu_of_node, \
+ &dai_link->cpu_dai_name)
+#define asoc_simple_card_parse_graph_codec(ep, dai_link) \
+ asoc_simple_card_parse_graph_dai(ep, &dai_link->codec_of_node, \
+ &dai_link->codec_dai_name)
+int asoc_simple_card_parse_graph_dai(struct device_node *ep,
+ struct device_node **endpoint_np,
+ const char **dai_name);
+
int asoc_simple_card_init_dai(struct snd_soc_dai *dai,
struct asoc_simple_dai *simple_dai);
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index c5ab8ad..5a3d51e 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -10,6 +10,7 @@
#include <linux/clk.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <sound/simple_card_utils.h>
int asoc_simple_card_parse_daifmt(struct device *dev,
@@ -171,6 +172,62 @@ int asoc_simple_card_parse_dai(struct device_node *node,
}
EXPORT_SYMBOL_GPL(asoc_simple_card_parse_dai);
+static int asoc_simple_card_get_dai_id(struct device_node *ep)
+{
+ struct device_node *node;
+ struct device_node *endpoint;
+ int i, id;
+
+ node = of_graph_get_port_parent(ep);
+
+ i = 0;
+ id = -1;
+ for_each_endpoint_of_node(node, endpoint) {
+ if (endpoint == ep)
+ id = i;
+ i++;
+ }
+ if (id < 0)
+ return -ENODEV;
+
+ return id;
+}
+
+int asoc_simple_card_parse_graph_dai(struct device_node *ep,
+ struct device_node **dai_of_node,
+ const char **dai_name)
+{
+ struct device_node *node;
+ struct of_phandle_args args;
+ int ret;
+
+ if (!ep)
+ return 0;
+ if (!dai_name)
+ return 0;
+
+ /*
+ * of_graph_get_port_parent() will call
+ * of_node_put(). So, call of_node_get() here
+ */
+ of_node_get(ep);
+ node = of_graph_get_port_parent(ep);
+
+ /* Get dai->name */
+ args.np = node;
+ args.args[0] = asoc_simple_card_get_dai_id(ep);
+ args.args_count = (of_graph_get_endpoint_count(node) > 1);
+
+ ret = snd_soc_get_dai_name(&args, dai_name);
+ if (ret < 0)
+ return ret;
+
+ *dai_of_node = node;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_simple_card_parse_graph_dai);
+
int asoc_simple_card_init_dai(struct snd_soc_dai *dai,
struct asoc_simple_dai *simple_dai)
{
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 6/9] ASoC: simple-card-utils: enable "label" on asoc_simple_card_parse_card_name
From: Kuninori Morimoto @ 2017-04-20 1:34 UTC (permalink / raw)
To: Mark Brown, Rob Herring; +Cc: Linux-ALSA, Simon, Linux-DT
In-Reply-To: <87fuh3oon8.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Current asoc_simple_card_parse_card_name() detects [prefix]name,
but in generally, we uses "label" for user visible names.
This patch enables it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v6 -> v7
- don't allow "[prefix]label"
sound/soc/generic/simple-card-utils.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 343b291..c5ab8ad 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -81,15 +81,21 @@ int asoc_simple_card_set_dailink_name(struct device *dev,
int asoc_simple_card_parse_card_name(struct snd_soc_card *card,
char *prefix)
{
- char prop[128];
int ret;
- snprintf(prop, sizeof(prop), "%sname", prefix);
+ if (!prefix)
+ prefix = "";
/* Parse the card name from DT */
- ret = snd_soc_of_parse_card_name(card, prop);
- if (ret < 0)
- return ret;
+ ret = snd_soc_of_parse_card_name(card, "label");
+ if (ret < 0) {
+ char prop[128];
+
+ snprintf(prop, sizeof(prop), "%sname", prefix);
+ ret = snd_soc_of_parse_card_name(card, prop);
+ if (ret < 0)
+ return ret;
+ }
if (!card->name && card->dai_link)
card->name = card->dai_link->name;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 5/9] ASoC: soc-core: enable "dai-format" on snd_soc_of_parse_daifmt()
From: Kuninori Morimoto @ 2017-04-20 1:33 UTC (permalink / raw)
To: Mark Brown, Rob Herring; +Cc: Linux-ALSA, Simon, Linux-DT
In-Reply-To: <87fuh3oon8.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Current snd_soc_of_parse_daifmt() detects [prefix]format, but
"format" was unclear in some case. This patch checks "dai-format"
first, and try to check "[prefix]format" if "dai-format" was not
exist.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v6 -> v7
- no change
sound/soc/soc-core.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 8be2ce1..e84a820 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3955,11 +3955,15 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
prefix = "";
/*
- * check "[prefix]format = xxx"
+ * check "dai-format = xxx"
+ * or "[prefix]format = xxx"
* SND_SOC_DAIFMT_FORMAT_MASK area
*/
- snprintf(prop, sizeof(prop), "%sformat", prefix);
- ret = of_property_read_string(np, prop, &str);
+ ret = of_property_read_string(np, "dai-format", &str);
+ if (ret < 0) {
+ snprintf(prop, sizeof(prop), "%sformat", prefix);
+ ret = of_property_read_string(np, prop, &str);
+ }
if (ret == 0) {
for (i = 0; i < ARRAY_SIZE(of_fmt_table); i++) {
if (strcmp(str, of_fmt_table[i].name) == 0) {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 4/9] of_graph: add of_graph_get_endpoint_count()
From: Kuninori Morimoto @ 2017-04-20 1:32 UTC (permalink / raw)
To: Mark Brown, Rob Herring; +Cc: Linux-ALSA, Simon, Linux-DT
In-Reply-To: <87fuh3oon8.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
OF graph want to count its endpoint number, same as
of_get_child_count(). This patch adds of_graph_get_endpoint_count()
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v6 -> v7
- no change
drivers/of/base.c | 12 ++++++++++++
include/linux/of_graph.h | 6 ++++++
2 files changed, 18 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index acda15b..bc42f91 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2529,6 +2529,18 @@ struct device_node *of_graph_get_remote_port(const struct device_node *node)
}
EXPORT_SYMBOL(of_graph_get_remote_port);
+int of_graph_get_endpoint_count(const struct device_node *np)
+{
+ struct device_node *endpoint;
+ int num = 0;
+
+ for_each_endpoint_of_node(np, endpoint)
+ num++;
+
+ return num;
+}
+EXPORT_SYMBOL(of_graph_get_endpoint_count);
+
/**
* of_graph_get_remote_node() - get remote parent device_node for given port/endpoint
* @node: pointer to parent device_node containing graph port/endpoint
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index 9db632d..3e058f0 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -43,6 +43,7 @@ struct of_endpoint {
#ifdef CONFIG_OF
int of_graph_parse_endpoint(const struct device_node *node,
struct of_endpoint *endpoint);
+int of_graph_get_endpoint_count(const struct device_node *np);
struct device_node *of_graph_get_port_by_id(struct device_node *node, u32 id);
struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
struct device_node *previous);
@@ -64,6 +65,11 @@ static inline int of_graph_parse_endpoint(const struct device_node *node,
return -ENOSYS;
}
+static inline int of_graph_get_endpoint_count(const struct device_node *np)
+{
+ return 0;
+}
+
static inline struct device_node *of_graph_get_port_by_id(
struct device_node *node, u32 id)
{
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 3/9] of_graph: add of_graph_get_port_parent()
From: Kuninori Morimoto @ 2017-04-20 1:32 UTC (permalink / raw)
To: Mark Brown, Rob Herring; +Cc: Linux-ALSA, Simon, Linux-DT
In-Reply-To: <87fuh3oon8.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Linux kernel already has of_graph_get_remote_port_parent(),
but, sometimes we want to get own port parent.
This patch adds of_graph_get_port_parent()
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v6 -> v7
- no change
drivers/of/base.c | 30 ++++++++++++++++++++++--------
include/linux/of_graph.h | 7 +++++++
2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 1ae2510..acda15b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2471,6 +2471,27 @@ struct device_node *of_graph_get_remote_endpoint(const struct device_node *node)
EXPORT_SYMBOL(of_graph_get_remote_endpoint);
/**
+ * of_graph_get_port_parent() - get port's parent node
+ * @node: pointer to a local endpoint device_node
+ *
+ * Return: device node associated with endpoint node linked
+ * to @node. Use of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_port_parent(struct device_node *node)
+{
+ unsigned int depth;
+
+ /* Walk 3 levels up only if there is 'ports' node. */
+ for (depth = 3; depth && node; depth--) {
+ node = of_get_next_parent(node);
+ if (depth == 2 && of_node_cmp(node->name, "ports"))
+ break;
+ }
+ return node;
+}
+EXPORT_SYMBOL(of_graph_get_port_parent);
+
+/**
* of_graph_get_remote_port_parent() - get remote port's parent node
* @node: pointer to a local endpoint device_node
*
@@ -2481,18 +2502,11 @@ struct device_node *of_graph_get_remote_port_parent(
const struct device_node *node)
{
struct device_node *np;
- unsigned int depth;
/* Get remote endpoint node. */
np = of_graph_get_remote_endpoint(node);
- /* Walk 3 levels up only if there is 'ports' node. */
- for (depth = 3; depth && np; depth--) {
- np = of_get_next_parent(np);
- if (depth == 2 && of_node_cmp(np->name, "ports"))
- break;
- }
- return np;
+ return of_graph_get_port_parent(np);
}
EXPORT_SYMBOL(of_graph_get_remote_port_parent);
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index 0c9473a..9db632d 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -50,6 +50,7 @@ struct device_node *of_graph_get_endpoint_by_regs(
const struct device_node *parent, int port_reg, int reg);
struct device_node *of_graph_get_remote_endpoint(
const struct device_node *node);
+struct device_node *of_graph_get_port_parent(struct device_node *node);
struct device_node *of_graph_get_remote_port_parent(
const struct device_node *node);
struct device_node *of_graph_get_remote_port(const struct device_node *node);
@@ -88,6 +89,12 @@ static inline struct device_node *of_graph_get_remote_endpoint(
return NULL;
}
+static inline struct device_node *of_graph_get_port_parent(
+ struct device_node *node)
+{
+ return NULL;
+}
+
static inline struct device_node *of_graph_get_remote_port_parent(
const struct device_node *node)
{
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 2/9] of_graph: add of_graph_get_remote_endpoint()
From: Kuninori Morimoto @ 2017-04-20 1:31 UTC (permalink / raw)
To: Mark Brown, Rob Herring; +Cc: Linux-ALSA, Simon, Linux-DT
In-Reply-To: <87fuh3oon8.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
It should use same method to get same result.
To getting remote-endpoint node,
let's use of_graph_get_remote_endpoint()
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v6 -> v7
- no change
drivers/of/base.c | 18 ++++++++++++++++--
include/linux/of_graph.h | 8 ++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d9adaa9..1ae2510 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2457,6 +2457,20 @@ struct device_node *of_graph_get_endpoint_by_regs(
EXPORT_SYMBOL(of_graph_get_endpoint_by_regs);
/**
+ * of_graph_get_remote_endpoint() - get remote endpoint node
+ * @node: pointer to a local endpoint device_node
+ *
+ * Return: Remote endpoint node associated with remote endpoint node linked
+ * to @node. Use of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_remote_endpoint(const struct device_node *node)
+{
+ /* Get remote endpoint node. */
+ return of_parse_phandle(node, "remote-endpoint", 0);
+}
+EXPORT_SYMBOL(of_graph_get_remote_endpoint);
+
+/**
* of_graph_get_remote_port_parent() - get remote port's parent node
* @node: pointer to a local endpoint device_node
*
@@ -2470,7 +2484,7 @@ struct device_node *of_graph_get_remote_port_parent(
unsigned int depth;
/* Get remote endpoint node. */
- np = of_parse_phandle(node, "remote-endpoint", 0);
+ np = of_graph_get_remote_endpoint(node);
/* Walk 3 levels up only if there is 'ports' node. */
for (depth = 3; depth && np; depth--) {
@@ -2494,7 +2508,7 @@ struct device_node *of_graph_get_remote_port(const struct device_node *node)
struct device_node *np;
/* Get remote endpoint node. */
- np = of_parse_phandle(node, "remote-endpoint", 0);
+ np = of_graph_get_remote_endpoint(node);
if (!np)
return NULL;
return of_get_next_parent(np);
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index abdb02e..0c9473a 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -48,6 +48,8 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
struct device_node *previous);
struct device_node *of_graph_get_endpoint_by_regs(
const struct device_node *parent, int port_reg, int reg);
+struct device_node *of_graph_get_remote_endpoint(
+ const struct device_node *node);
struct device_node *of_graph_get_remote_port_parent(
const struct device_node *node);
struct device_node *of_graph_get_remote_port(const struct device_node *node);
@@ -80,6 +82,12 @@ static inline struct device_node *of_graph_get_endpoint_by_regs(
return NULL;
}
+static inline struct device_node *of_graph_get_remote_endpoint(
+ const struct device_node *node)
+{
+ return NULL;
+}
+
static inline struct device_node *of_graph_get_remote_port_parent(
const struct device_node *node)
{
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 1/9] of-graph: export symbol of_phandle_iterator_init/next
From: Kuninori Morimoto @ 2017-04-20 1:31 UTC (permalink / raw)
To: Mark Brown, Rob Herring; +Cc: Linux-ALSA, Simon, Linux-DT
In-Reply-To: <87fuh3oon8.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
of_for_each_phandle() uses of_phandle_iterator_init/next
but it isn't exported. So kernel module complile will say
ERROR: "of_phandle_iterator_init" [xxx.ko] undefined!
ERROR: "of_phandle_iterator_next" [xxx.ko] undefined!
This patch solved this issue
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v6 -> v7
- no change
drivers/of/base.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d7c4629..d9adaa9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1570,6 +1570,7 @@ int of_phandle_iterator_init(struct of_phandle_iterator *it,
return 0;
}
+EXPORT_SYMBOL_GPL(of_phandle_iterator_init);
int of_phandle_iterator_next(struct of_phandle_iterator *it)
{
@@ -1639,6 +1640,7 @@ int of_phandle_iterator_next(struct of_phandle_iterator *it)
return -EINVAL;
}
+EXPORT_SYMBOL_GPL(of_phandle_iterator_next);
int of_phandle_iterator_args(struct of_phandle_iterator *it,
uint32_t *args,
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 0/9] ASoC: add OF-graph base simple-card
From: Kuninori Morimoto @ 2017-04-20 1:30 UTC (permalink / raw)
To: Mark Brown, Rob Herring; +Cc: Linux-ALSA, Simon, Linux-DT
Hi Mark, Rob
These are v7 of OF-graph sound card support.
Main change from v6 "label" doesn't allow [prefix]label (in [6/9])
1) - 4) : OF-graph base functions
5) - 9) : OF-graph + simple-card (= audio_graph_card)
Kuninori Morimoto (9):
of-graph: export symbol of_phandle_iterator_init/next
of_graph: add of_graph_get_remote_endpoint()
of_graph: add of_graph_get_port_parent()
of_graph: add of_graph_get_endpoint_count()
ASoC: soc-core: enable "dai-format" on snd_soc_of_parse_daifmt()
ASoC: simple-card-utils: enable "label" on asoc_simple_card_parse_card_name
ASoC: simple-card-utils: add asoc_simple_card_parse_graph_dai()
ASoC: add audio-graph-card document
ASoC: add audio-graph-card support
.../devicetree/bindings/sound/audio-graph-card.txt | 124 +++++++++
drivers/of/base.c | 62 ++++-
include/linux/of_graph.h | 21 ++
include/sound/simple_card_utils.h | 10 +
sound/soc/generic/Kconfig | 8 +
sound/soc/generic/Makefile | 2 +
sound/soc/generic/audio-graph-card.c | 308 +++++++++++++++++++++
sound/soc/generic/simple-card-utils.c | 73 ++++-
sound/soc/soc-core.c | 10 +-
9 files changed, 600 insertions(+), 18 deletions(-)
create mode 100644 Documentation/devicetree/bindings/sound/audio-graph-card.txt
create mode 100644 sound/soc/generic/audio-graph-card.c
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] mmc: dw_mmc-rockchip: parse rockchip,default-num-phases from DT
From: Shawn Lin @ 2017-04-20 1:21 UTC (permalink / raw)
To: Doug Anderson
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ulf Hansson,
Ziyuan Xu, linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jaehoon Chung, open list:ARM/Rockchip SoC..., Rob Herring
In-Reply-To: <CAD=FV=X6PRe1u+s4Nnt=9gJD2T4=YyLdMWnKe5rZg2KgWGNAvg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Doug,
在 2017/4/20 4:19, Doug Anderson 写道:
> Hi,
>
> On Wed, Apr 19, 2017 at 2:00 AM, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>> Currently we unconditionally do tuning for each degree, which
>> costs 900ms for each boot and resume.
>>
>> May someone argue that this is a question of accuracy VS time. But I
>> would say it's a trick of how we need to do decision for our boards.
>> If we don't care the time we spend at all, we could definitely do tuning
>> for each degree. But when we need to improve the user experience, for
>> instance, speed up resuming from S3, we should also have the right to
>> do that. This patch add parsing "rockchip,default-num-phases", for folks
>> to specify the number of doing tuning. If not specified, 360 will be used
>> as before.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>> drivers/mmc/host/dw_mmc-rockchip.c | 48 ++++++++++++++++++++++++--------------
>> 1 file changed, 30 insertions(+), 18 deletions(-)
>
> No huge objection here, but I do remember we ended up at the 360
> phases due to some of the craziness with dw_mmc delay elements on
> rk3288. IIRC one of the big problems was that the delay elements
> changed _a lot_ with the "LOGIC" voltage and we tweaked the voltage at
> runtime for DDR DVFS. That imposed an extra need to be very accurate
> on that SoC, at least on any board that was designed to support DDR
> DVFS.
>
Not just with the vdd_logic but also with the process of Soc.
To better guaratee the accuracy, firstly we use delay element to do
tuning and then convert it to be combination of degree + delay element.
But as the dalay elements aren't accuracy themself, so all the math we
do here is trick.
> I also remember there being some weirdness on the Rockchip
> implementation where there was a certain set of phases that the MMC
> controller was essentially "blind". This blind spot was in the middle
> of an otherwise good range of points. Unfortunately this blind spot
> was somewhat hard to detect properly because it was not very big.
> ...the variability of the delay elements meant that there could be big
> ranges where we weren't getting any good test coverage, but also the
> fact that they changed with the LOGIC voltage might mean that we
> weren't in the "blind" spot and then suddenly we were.
I undertand all of these as I was suffering from it when bringing up
RK3288.
>
> One other note is that i remember that the vast majority of time spent
> tuning was dealing with "bad" phases, not dealing with "good" phases.
> If you're looking to speed things up, maybe finding a way to make
> "bad" phases fail faster would be wise? I think one of the reasons
> bad phases failed so slowly is because the dw_mmc timeouts are all so
> long.
Good point. I haven't thought of speeding up the handle of bad phases,
but I will take a look at this.
>
> Oh, and I guess one last note is that I have no idea if folks will
> like the device bindings here. Part of me thinks it should be
> something more "symbolic" like "rockchip,need-accurate-tuning" or
> something like that. I guess I'd let the DT experts chime in.
>
>
> So I guess to summarize:
> * On rk3288 boards w/ DDR DVFS (or any other similar boards), 360
> seems to provide real benefit.
> * On other boards, probably you can get by with fewer phases.
>
I would try to say it's a question of "900ms for a single time" VS.
"some of discrete tuning cost for more chance to do retune".
(1)We could try to do a more accurate tuning process and spends 900ms.
Then we have less chance to do retune later.
(2)We do a rough tuning and have more chance to do retune later.
I also would say that this is a game , and we can't say which
one is better. Obvious now the "900ms" alwyas happens in the spot
routine, for instance, booting and resuming from S3.
>
> -Doug
>
>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH v3 00/21] eeprom: at24: Add OF device ID table
From: Rob Herring @ 2017-04-20 0:12 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Mark Rutland, Andrew Lunn, Wolfram Sang, Tony Lindgren,
Catalin Marinas, Will Deacon, linux-kernel, Masahiro Yamada,
Alexandre Belloni, linux-i2c, Hongtao Jia, Russell King,
Mark Jackson, Herbert Xu, Horia Geantă, Michael Ellerman,
Magnus Damm, Michal Simek, Andy Shevchenko, linux-arm-kernel,
Benjamin Herrenschmidt, Jason Cooper
In-Reply-To: <20170414010445.21727-1-javier@osg.samsung.com>
On Thu, Apr 13, 2017 at 10:04:24PM -0300, Javier Martinez Canillas wrote:
> Hello Wolfram,
>
> This series is a follow-up to patch [0] that added an OF device ID table
> to the at24 EEPROM driver. As you suggested [1], this version instead of
> adding entries for every used <vendor,device> tuple, only adds a single
> entry for each chip type using the "atmel" vendor as a generic fallback.
>
> The first patch adds the OF device ID table for the at24 driver and the
> next patches adds a generic fallback compatible string to each DTS that
> defines a compatible I2C EEPROM device node.
>
> Patches can be applied independently since the DTS change without the
> driver change is a no-op and the OF device table won't be used without
> the DTS changes.
>
> [0]: https://lkml.org/lkml/2017/3/14/589
I don't have this patch in my inbox, but the i2c core will still match
on compatibles with no vendor prefix? If not, those need to be added to
the match table too.
Rob
^ permalink raw reply
* Re: [PATCH v6 6/9] ASoC: simple-card-utils: enable "label" on asoc_simple_card_parse_card_name
From: Kuninori Morimoto @ 2017-04-19 23:59 UTC (permalink / raw)
To: Rob Herring; +Cc: Mark Brown, Linux-ALSA, Simon, Linux-DT
In-Reply-To: <CAL_JsqKEuV38kGreCm4z4aEFVRjPaBhqRbELk9W2x1UL4KpMOQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Rob
> > int asoc_simple_card_parse_card_name(struct snd_soc_card *card,
> > char *prefix)
> > {
> > + char * const names[] = {
> > + "label", "name"
> > + };
> > char prop[128];
> > + int i;
> > int ret;
> >
> > - snprintf(prop, sizeof(prop), "%sname", prefix);
> > + if (!prefix)
> > + prefix = "";
> >
> > /* Parse the card name from DT */
> > - ret = snd_soc_of_parse_card_name(card, prop);
> > - if (ret < 0)
> > - return ret;
> > + for (i = 0; i < ARRAY_SIZE(names); i++) {
> > + snprintf(prop, sizeof(prop), "%s%s", prefix, names[i]);
> > + ret = snd_soc_of_parse_card_name(card, prop);
> > + if (ret < 0)
> > + return ret;
> > + if (card->name)
> > + break;
> > + }
>
> This is still wrong as you are allowing "<prefix>label" for property
> names. I think you want something like this:
>
> ret = snd_soc_of_parse_card_name(card, "label");
> if (ret < 0) {
> char prop[128];
> snprintf(prop, sizeof(prop), "%sname", prefix);
> /* Parse the card name from DT */
> ret = snd_soc_of_parse_card_name(card, prop);
> if (ret < 0)
> return ret;
> }
OK, will fix
Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 03/21] ARM: dts: omap: Add generic compatible string for I2C EEPROM
From: Rob Herring @ 2017-04-19 23:56 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Benoît Cousson, devicetree-u79uwXL29TY76Z2rM5mHXA,
Tony Lindgren, Mark Rutland, linux-omap-u79uwXL29TY76Z2rM5mHXA,
Russell King, Mark Jackson
In-Reply-To: <20170414010445.21727-4-javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On Thu, Apr 13, 2017 at 10:04:27PM -0300, Javier Martinez Canillas wrote:
> The at24 driver allows to register I2C EEPROM chips using different vendor
> and devices, but the I2C subsystem does not take the vendor into account
> when matching using the I2C table since it only has device entries.
>
> But when matching using an OF table, both the vendor and device has to be
> taken into account so the driver defines only a set of compatible strings
> using the "atmel" vendor as a generic fallback for compatible I2C devices.
>
> So add this generic fallback to the device node compatible string to make
> the device to match the driver using the OF device ID table.
>
> Signed-off-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> arch/arm/boot/dts/am335x-baltos.dtsi | 2 +-
> arch/arm/boot/dts/am335x-base0033.dts | 2 +-
> arch/arm/boot/dts/am335x-bone-common.dtsi | 10 +++++-----
> arch/arm/boot/dts/am335x-nano.dts | 2 +-
> arch/arm/boot/dts/am335x-pepper.dts | 2 +-
> arch/arm/boot/dts/am335x-shc.dts | 2 +-
> arch/arm/boot/dts/am335x-sl50.dts | 2 +-
> arch/arm/boot/dts/am437x-idk-evm.dts | 2 +-
> arch/arm/boot/dts/am437x-sk-evm.dts | 2 +-
> arch/arm/boot/dts/am43x-epos-evm.dts | 2 +-
> arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 2 +-
> arch/arm/boot/dts/omap3-cm-t3x.dtsi | 2 +-
> arch/arm/boot/dts/omap3-gta04.dtsi | 2 +-
> arch/arm/boot/dts/omap3-sb-t35.dtsi | 2 +-
> arch/arm/boot/dts/omap4-var-som-om44.dtsi | 2 +-
> arch/arm/boot/dts/omap5-cm-t54.dts | 2 +-
> arch/arm/boot/dts/omap5-sbc-t54.dts | 2 +-
> 17 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm/boot/dts/am335x-baltos.dtsi b/arch/arm/boot/dts/am335x-baltos.dtsi
> index d42b98f15e8b..6ca780d0623f 100644
> --- a/arch/arm/boot/dts/am335x-baltos.dtsi
> +++ b/arch/arm/boot/dts/am335x-baltos.dtsi
> @@ -255,7 +255,7 @@
> };
>
> at24@50 {
> - compatible = "at24,24c02";
> + compatible = "at24,24c02", "atmel,24c02";
I think you can just drop the at24 compatibles. A new kernel doesn't
need it. An old kernel ignores the manufacturer. I checked that u-boot
only matches on "atmel,*", so okay there. Don't know about the *BSDs. I
couldn't find anything.
Minimally, the deprecated compatible should come last.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 04/18] dt-bindings: syscon: Add DT bindings documentation for Allwinner syscon
From: André Przywara @ 2017-04-19 23:38 UTC (permalink / raw)
To: Corentin Labbe, robh+dt, mark.rutland, maxime.ripard, wens, linux,
catalin.marinas, will.deacon, peppe.cavallaro, alexandre.torgue
Cc: devicetree, netdev, linux-kernel, linux-sunxi, linux-arm-kernel
In-Reply-To: <20170412111400.2296-5-clabbe.montjoie@gmail.com>
On 12/04/17 12:13, Corentin Labbe wrote:
> This patch adds documentation for Device-Tree bindings for the
> syscon present in allwinner devices.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> .../devicetree/bindings/misc/allwinner,syscon.txt | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/misc/allwinner,syscon.txt
>
> diff --git a/Documentation/devicetree/bindings/misc/allwinner,syscon.txt b/Documentation/devicetree/bindings/misc/allwinner,syscon.txt
> new file mode 100644
> index 0000000..c056c5b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/allwinner,syscon.txt
> @@ -0,0 +1,19 @@
> +* Allwinner sun8i system controller
> +
> +This file describes the bindings for the system controller present in
> +Allwinner SoC H3, A83T and A64.
> +The principal function of this syscon is to control EMAC PHY choice and
> +config.
> +
> +Required properties for the system controller:
> +- reg: address and length of the register for the device.
> +- compatible: should be "syscon" and one of the following string:
> + "allwinner,sun8i-h3-system-controller"
> + "allwinner,sun8i-a64-system-controller"
While sun8i might make some sense technically, all 64-bit sunxi
compatible strings use the sun50i prefix to follow the Allwinner naming.
So this should read:
"allwinner,sun50i-a64-system-controller"
Also I am wondering if we should add a compatible string for the H5
(support for that SoC is in -next already):
"allwinner,sun50i-h5-system-controller"
Cheers,
Andre.
> + "allwinner,sun8i-a83t-system-controller"
> +
> +Example:
> +syscon: syscon@01c00000 {
> + compatible = "allwinner,sun8i-h3-system-controller", "syscon";
> + reg = <0x01c00000 0x1000>;
> +};
>
^ permalink raw reply
* Re: [PATCH v3 01/21] dt-bindings: i2c: eeprom: Document manufacturer used as generic fallback
From: Rob Herring @ 2017-04-19 23:35 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Wolfram Sang, Simon Horman,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sekhar Nori,
David Lechner, Alexandre Belloni, Mark Rutland
In-Reply-To: <20170419232759.mw6g6fwy4xvjaopv@rob-hp-laptop>
On Wed, Apr 19, 2017 at 6:27 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Apr 13, 2017 at 10:04:25PM -0300, Javier Martinez Canillas wrote:
>> The at24 driver allows to register I2C EEPROM chips using different vendor
>> and devices, but the I2C subsystem does not take the vendor into account
>> when matching using the I2C table since it only has device entries.
>>
>> But when matching using an OF table, both the vendor and device has to be
>> taken into account so the driver defines only a set of compatible strings
>> using the "atmel" vendor as a generic fallback for compatible I2C devices.
>>
>> Document in the Device Tree binding document that this manufacturer should
>> be used as the generic fallback.
>>
>> Suggested-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
>> Signed-off-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>>
>> ---
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>> Documentation/devicetree/bindings/eeprom/eeprom.txt | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt
>> index 5696eb508e95..d0395f14e2b3 100644
>> --- a/Documentation/devicetree/bindings/eeprom/eeprom.txt
>> +++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt
>> @@ -17,7 +17,8 @@ Required properties:
>> "renesas,r1ex24002"
>>
>> If there is no specific driver for <manufacturer>, a generic
>> - driver based on <type> is selected. Possible types are:
>> + driver based on <type> and manufacturer "atmel" is selected.
>> + Possible types are:
>
> This isn't quite right. What the driver does isn't really relevant to
> the binding.
>
> These types with no vendor are used as the compatible string, so we have
> to allow them. But it should be clear that no vendor is deprecated.
> Ironically, it is a lot of Atmel boards that do this.
>
> We should also explicitly list what are valid manufacturers. We also
> have "at" as a vendor prefix which perhaps we should explicitly say is
> deprecated.
I should perhaps look at the rest of the series before replying..
Based on that, the only comment that applies is listing the
manufacturers that are valid. From a DT perspective, I should not have
to know what the OS driver supports. If the device is compatible with
atmel, then that is required. If not, then the specific manufacturer's
compatible alone is enough and the OS has to match to that.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox