Re: [4.4-rc][PATCH] ARM: dts: am4372: disable arm twd and global timer's nodes
On Wed, Nov 18, 2015 at 04:01:55PM +0200, Grygorii Strashko wrote: > Keep ARM TWD and Global timer's nodes disabled by default - if someone > would like to use them then those nodes have to be enabled explicitly > in board file. > > The reason for this change is: > - ARM TWD is not always-on timer on am437x and it will stop in low > CPUIdle states and, therefore, broadcast timer has to configured > properly if CPU_IDLE=y. > - ARM Global timer is not always-on timer on am437x and it can't be > used as clocksource device if CPU_IDLE=y. I don't understand. What timer do you use in the absence of a TWD, and if it is better why is it not used even if TWD is present? > - ARM Global timer driver doesn't support CPUfreq now. Surely that should be fixed in the driver (e.g. make it fail to probe if CPUfreq is present)? It's broken for any other users too... Mark. > Cc: Felipe Balbi> Cc: Santosh Shilimkar > Signed-off-by: Grygorii Strashko > --- > arch/arm/boot/dts/am4372.dtsi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi > index d83ff9c..11376e3 100644 > --- a/arch/arm/boot/dts/am4372.dtsi > +++ b/arch/arm/boot/dts/am4372.dtsi > @@ -75,6 +75,7 @@ > interrupts = ; > interrupt-parent = <>; > clocks = <_mpu_m2_ck>; > + status = "disabled"; > }; > > local_timer: timer@48240600 { > @@ -83,6 +84,7 @@ > interrupts = ; > interrupt-parent = <>; > clocks = <_mpu_m2_ck>; > + status = "disabled"; > }; > > l2-cache-controller@48242000 { > -- > 2.6.3 > > -- > 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/RESEND] ARM: Remove __ref on hotplug cpu die path
On Mon, Oct 19, 2015 at 01:05:33PM -0700, Stephen Boyd wrote: > Now that __cpuinit has been removed, the __ref markings on these > functions are useless. Remove them. This also reduces the size of > the multi_v7_defconfig image: > > $ size before after >textdata bss dec hex filename >126835781470996 348904 14503478 dd4e36 before >126832741470996 348904 14503174 dd4d06 after > > presumably because now we don't have to jump to code in the > .ref.text section and/or the noinline marking is removed. FWIW: Acked-by: Mark Rutland <mark.rutl...@arm.com> Mark. > Cc: Tony Lindgren <t...@atomide.com> > Acked-by: Barry Song <bao...@kernel.org> > Acked-by: Andy Gross <agr...@codeaurora.org> > Acked-by: Viresh Kumar <vire...@kernel.org> > Cc: Shiraz Hashim <shiraz.linux.ker...@gmail.com> > Cc: Stephen Warren <swar...@wwwdotorg.org> > Acked-by: Thierry Reding <thierry.red...@gmail.com> > Cc: Alexandre Courbot <gnu...@gmail.com> > Acked-by: Linus Walleij <linus.wall...@linaro.org> > Acked-by: Sudeep Holla <sudeep.ho...@arm.com> > Cc: Lorenzo Pieralisi <lorenzo.pieral...@arm.com> > Cc: Will Deacon <will.dea...@arm.com> > Cc: Mark Rutland <mark.rutl...@arm.com> > Cc: <linux-omap@vger.kernel.org> > Cc: <linux-arm-...@vger.kernel.org> > Cc: <spear-de...@list.st.com> > Cc: <linux-te...@vger.kernel.org> > Signed-off-by: Stephen Boyd <sb...@codeaurora.org> > --- > > Collected the acks and resent. > > arch/arm/kernel/psci_smp.c | 4 ++-- > arch/arm/mach-omap2/omap-hotplug.c | 2 +- > arch/arm/mach-omap2/omap-wakeupgen.c | 2 +- > arch/arm/mach-prima2/hotplug.c | 2 +- > arch/arm/mach-qcom/platsmp.c | 2 +- > arch/arm/mach-realview/hotplug.c | 2 +- > arch/arm/mach-spear/hotplug.c| 2 +- > arch/arm/mach-tegra/hotplug.c| 2 +- > arch/arm/mach-ux500/hotplug.c| 2 +- > arch/arm/mach-vexpress/hotplug.c | 2 +- > 10 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c > index 61c04b02faeb..9d479b2ea40d 100644 > --- a/arch/arm/kernel/psci_smp.c > +++ b/arch/arm/kernel/psci_smp.c > @@ -71,7 +71,7 @@ int psci_cpu_disable(unsigned int cpu) > return 0; > } > > -void __ref psci_cpu_die(unsigned int cpu) > +void psci_cpu_die(unsigned int cpu) > { > u32 state = PSCI_POWER_STATE_TYPE_POWER_DOWN << > PSCI_0_2_POWER_STATE_TYPE_SHIFT; > @@ -83,7 +83,7 @@ void __ref psci_cpu_die(unsigned int cpu) > panic("psci: cpu %d failed to shutdown\n", cpu); > } > > -int __ref psci_cpu_kill(unsigned int cpu) > +int psci_cpu_kill(unsigned int cpu) > { > int err, i; > > diff --git a/arch/arm/mach-omap2/omap-hotplug.c > b/arch/arm/mach-omap2/omap-hotplug.c > index 971791fe9a3f..593fec753b28 100644 > --- a/arch/arm/mach-omap2/omap-hotplug.c > +++ b/arch/arm/mach-omap2/omap-hotplug.c > @@ -27,7 +27,7 @@ > * platform-specific code to shutdown a CPU > * Called with IRQs disabled > */ > -void __ref omap4_cpu_die(unsigned int cpu) > +void omap4_cpu_die(unsigned int cpu) > { > unsigned int boot_cpu = 0; > void __iomem *base = omap_get_wakeupgen_base(); > diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c > b/arch/arm/mach-omap2/omap-wakeupgen.c > index db7e0bab3587..2da0d974f2f7 100644 > --- a/arch/arm/mach-omap2/omap-wakeupgen.c > +++ b/arch/arm/mach-omap2/omap-wakeupgen.c > @@ -330,7 +330,7 @@ static int irq_cpu_hotplug_notify(struct notifier_block > *self, > return NOTIFY_OK; > } > > -static struct notifier_block __refdata irq_hotplug_notifier = { > +static struct notifier_block irq_hotplug_notifier = { > .notifier_call = irq_cpu_hotplug_notify, > }; > > diff --git a/arch/arm/mach-prima2/hotplug.c b/arch/arm/mach-prima2/hotplug.c > index 0ab2f8bae28e..a728c78b996f 100644 > --- a/arch/arm/mach-prima2/hotplug.c > +++ b/arch/arm/mach-prima2/hotplug.c > @@ -32,7 +32,7 @@ static inline void platform_do_lowpower(unsigned int cpu) > * > * Called with IRQs disabled > */ > -void __ref sirfsoc_cpu_die(unsigned int cpu) > +void sirfsoc_cpu_die(unsigned int cpu) > { > platform_do_lowpower(cpu); > } > diff --git a/arch/arm/mach-qcom/platsmp.c b/arch/arm/mach-qcom/platsmp.c > index 5cde63a64b34..9b00123a315d 100644 > --- a/arch/arm/mach-qcom/platsmp.c > +++ b/arch/arm/mach-qcom/platsmp.c > @@ -49,7 +49,7 @@ extern void secondary_startup_arm(void); > static DEFINE_SPINLOCK(boot_lock); > > #ifdef CONF
Re: [PATCH v2 2/3] iio:adc:palmas: add DT support
On Sun, Oct 04, 2015 at 06:05:59PM +0200, H. Nikolaus Schaller wrote: > From: Marek Belisko> > Code was found at: > https://android.googlesource.com/kernel/tegra/+/a90856a6626d502d42c6e7abccbdf9d730b36270%5E%21/#F1 > > Signed-off-by: Laxman Dewangan > [Fixed minor typos + add channels list to documentation] > Signed-off-by: Marek Belisko > --- > .../devicetree/bindings/iio/adc/palmas-gpadc.txt | 46 +++ > drivers/iio/adc/palmas_gpadc.c | 52 +++--- > 2 files changed, 93 insertions(+), 5 deletions(-) > create mode 100644 Documentation/devicetree/bindings/iio/adc/palmas-gpadc.txt > > diff --git a/Documentation/devicetree/bindings/iio/adc/palmas-gpadc.txt > b/Documentation/devicetree/bindings/iio/adc/palmas-gpadc.txt > new file mode 100644 > index 000..2149afe > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/palmas-gpadc.txt > @@ -0,0 +1,46 @@ > +* Palmas general purpose ADC IP block devicetree bindings > + > +Channels list: > + 0 battery type > + 1 battery temp NTC (optional current source) > + 2 GP > + 3 temp (with ext. diode, optional current source) > + 4 GP > + 5 GP > + 6 VBAT_SENSE > + 7 VCC_SENSE > + 8 Backup Battery voltage > + 9 external charger (VCHG) > + 10 VBUS > + 11 DC-DC current probe (how does this work?) > + 12 internal die temp > + 13 internal die temp > + 14 USB ID pin voltage > + 15 test network > + > +Required properties: > +- compatible : Must be "ti,palmas-gpadc". > + > +Optional sub-nodes: > +ti,channel0-current-microamp: Channel 0 current in uA. > + Values are rounded to derive 0uA, 5uA, 15uA, 20uA. > +ti,channel3-current-microamp: Channel 3 current in uA. > + Valid are rounded to derive 0uA, 10uA, 400uA, 800uA. It's only possible to configure channels 0 and 3 in this manner? > +ti,enable-extended-delay: Enable extended delay. What is this? When would I select it? Why does it belong in the DT rather than being a runtime option? > +Example: > + > +pmic { > + compatible = "ti,twl6035-pmic", "ti,palmas-pmic"; > + ... > + gpadc { > + compatible = "ti,palmas-gpadc"; > + interrupts = <18 0 > + 16 0 > + 17 0>; > + ti,channel0-current-microamp = <5>; > + ti,channel3-current-microamp = <10>; > + }; > + }; > + ... > +}; I thought you needed #iio-cells for encoding the channel? Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dts: dra7: arch timer sits in always-on power domain
On Mon, Jul 13, 2015 at 09:41:41PM +0100, Felipe Balbi wrote: According to DRA7x TRM section 4.3.5 Realtime Counter (Master Counter), the realtime counter sits in the Wakeup Always-On Power domain. Furthermore, the counter will automatically switch the 32K clock source when MPU goes into standby and automatically switch back to SYS_CLK or ABE_LP when MPU goes out of standby. While the counter is in an always-on domain (the architecture mandates this) I don't think that applies to the timers (i.e. the comparators within a CPU), which are almost certainly not in an always-on domain. I suspect that this is incorrect, and it will be very painful to debug if it is... Thanks, Mark. Signed-off-by: Felipe Balbi ba...@ti.com --- arch/arm/boot/dts/dra7.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi index 8f1e25bcecbd..c1e6fd82485f 100644 --- a/arch/arm/boot/dts/dra7.dtsi +++ b/arch/arm/boot/dts/dra7.dtsi @@ -50,6 +50,7 @@ GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW), GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW); interrupt-parent = gic; + always-on; }; gic: interrupt-controller@48211000 { -- 2.4.4 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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 v8 0/4] hwspinlock core omap dt support
Hi, Apologies for the delay on this. Gentle reminder. Can you please provide your ack on the bindings in this series (Patches 1 3) for Ohad to queue up the series for 4.1. Ping again, if you can provide your ack on these bindings and the qcom hwmutex bindings, then Ohad can pick up both the series for 4.1. Both series are blocked on getting the binding acks. Both the bindings look sane to me, so for patches 1 and 3: Acked-by: Mark Rutland mark.rutl...@arm.com Mark. -- 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, Sorry I've been away from this thread for a while. On Wed, Feb 11, 2015 at 10:29:37AM +, Ohad Ben-Cohen wrote: On Fri, Feb 6, 2015 at 2:34 AM, Bjorn Andersson bj...@kryo.se wrote: 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 :/ I suspect we want to wait with framework changes until there's a real hardware use case justifying them. With regard to DT, however, we obviously do want to be prepared for multiple hwlock blocks even if they do not exist today. So how about adopting an approach where: - DT fully supports multi hwlock blocks (i.e. no global id field) - Framework left mostly unchanged at the moment. This means issuing an explicit error in case a secondary hwlock block shows up, and then hwlock id could trivially be the lock index. If multi hwlock hardware use case, or some new hwlock id translation requirements, do show up in the future, it's always easy to add framework support for it. At that point we'll know better what the requirements are, and framework changes would be justifiable. As mentioned in my other reply I think we need to be explicit now when defining the set of hwlocks (and their namming/numbering) shared between a given set of processors, as we do with other resources (GPIOs/regulators/whatever). We also need to be explicit in describing the set of actors which use those locks. I think my previous proposal covered both of those. Thanks, Mark. -- 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, Jan 29, 2015 at 03:58:42AM +, Suman Anna wrote: On 01/22/2015 12:56 PM, Mark Rutland wrote: On Wed, Jan 21, 2015 at 05:56:37PM +, Suman Anna wrote: On 01/21/2015 06:41 AM, Ohad Ben-Cohen wrote: On Tue, Jan 20, 2015 at 8:05 PM, Tony Lindgren t...@atomide.com wrote: How about default to Linux id space and allow overriding that with a module param option if needed? I'm not sure I'm following. If the main point of contention is the base_id field, I'm also fine with removing it entirely, as I'm not aware of any actual user for it (Suman please confirm?). Yeah, well the current implementations that I am aware of only have a single bank, so all of them would be using a value of 0. I am yet to see a platform with multiple instances where the property really makes a difference. v7 has the property mandatory, so all the implementations would need to define this value even if it is 0. regards Suman Mark? Rob? Will you accept Suman's patches if the base_id field is removed? My concern is that the mapping of hwspinlock IDs doesn't seem to be explicit in the DT on a per-context basis, which is what I'd expect. e.g. lck: hwspinlock-device@f00 { ... #hwlock-cells = 1; }; some-other-os-interface { ... hwlocks = lck 0, lck 1, lck 2, lck 3; hwlock-names = glbl, pool0, pool1, pool2; }; a-different-os-interface { ... hwlocks = lck 18, lck 21, lck 4, lck 5; hwlock-names = init, teardown, pool0, pool1; }; That's the only way I would expect this to possibly remain a stable over time, and it's the entire reason for #hwlock-cells, no? How do you expect the other components sharing the hwspinlocks to be described? Yes indeed, this is what any of the clients will use on Linux. But this is not necessarily the semantics for exchanging hwlocks with the other processor(s) which is where the global id space comes into picture. I did try to consider that above. Rather than thinking about the numbering as global, think of it as unique within the a given pool shared between processors. That's what the poolN names are about above. That way you can dynamically allocate within the pool and know that Linux and the SW on the other processors will use the same ID. You can have pools that span multiple hwlock hardware blocks, and you can have multiple separate pools in operation at once. Surely that covers the cases you care about? If using names is clunky, we could instead have a pool-hwlocks property for that purpose. Thanks, Mark. -- 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: bluetooth on n900 -- working patch
On Wed, Feb 11, 2015 at 10:41:36AM +, Pavel Machek wrote: Hi! Here's current version of the bluetooth patch, I hope I did not miss anything. This time including dts changes, so that driver is active. I have firmware in /lib/firmware/nokia/bcmfw.bin Best regards, Pavel Signed-off-by: Pavel Machek pa...@ucw.cz diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt index 342eedd..e9fde42 100644 --- a/Documentation/devicetree/bindings/serial/omap_serial.txt +++ b/Documentation/devicetree/bindings/serial/omap_serial.txt @@ -4,7 +4,14 @@ Required properties: - compatible : should be ti,omap2-uart for OMAP2 controllers - compatible : should be ti,omap3-uart for OMAP3 controllers - compatible : should be ti,omap4-uart for OMAP4 controllers +- compatible : should be ti+brcm,omap3-uart,bcm2048 for bcm2048 + bluetooth connected to OMAP3 serial As I mentioned previously, these should be separate devices, using the UART as a bus. NAK to this pseudo-composite device. Mark. - ti,hwmods : Must be uartn, n being the instance number (1-based) Optional properties: - clock-frequency : frequency of the clock input to the UART + +Required properties for ti+brcm,omap3-uart,bcm2048: +- reset-gpios : gpio for reset pin +- host-wakeup-gpios : gpio for host wakeup +- bluetooth-wakeup-gpios : gpio for bluetooth wakeup diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts index b550c41..e19bbe8 100644 --- a/arch/arm/boot/dts/omap3-n900.dts +++ b/arch/arm/boot/dts/omap3-n900.dts @@ -821,9 +867,19 @@ }; uart2 { +compatible = ti+brcm,omap3-uart,bcm2048, ti,omap3-uart; interrupts-extended = intc 73 omap3_pmx_core OMAP3_UART2_RX; pinctrl-names = default; pinctrl-0 = uart2_pins; + + clocks = uart2_fck, uart2_ick; + clock-names = fck, ick; + + device { + reset-gpios = gpio3 27 GPIO_ACTIVE_HIGH; /* 91 */ + host-wakeup-gpios = gpio4 5 GPIO_ACTIVE_HIGH; /* 101 */ + bluetooth-wakeup-gpios = gpio2 5 GPIO_ACTIVE_HIGH; /* 37 */ + }; }; uart3 { diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi index 01b7111..60e6d6b 100644 --- a/arch/arm/boot/dts/omap3.dtsi +++ b/arch/arm/boot/dts/omap3.dtsi @@ -275,7 +275,8 @@ }; uart2: serial@4806c000 { - compatible = ti,omap3-uart; + // This is bluetooth on n900, ttyO1 + compatible = not-a-ti,omap3-not-a-uart; reg = 0x4806c000 0x400; interrupts-extended = intc 73; dmas = sdma 51 sdma 52; diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig index 364f080..e1e1935 100644 --- a/drivers/bluetooth/Kconfig +++ b/drivers/bluetooth/Kconfig @@ -244,4 +244,14 @@ config BT_WILINK Say Y here to compile support for Texas Instrument's WiLink7 driver into the kernel or say M to compile it as module (btwilink). +config BT_NOKIA_H4P + tristate Nokia H4+ Extensions (H4P) driver + help + Bluetooth HCI driver with H4 extensions. This driver provides + support for H4+ Bluetooth chip with vendor-specific H4 extensions. + It works on Broadcom based chip found in Nokia N900. + + Say Y here to compile support for h4 extended devices into the kernel + or say M to compile it as module (nokia_h4p). + endmenu diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile index 9fe8a87..f1cc580 100644 --- a/drivers/bluetooth/Makefile +++ b/drivers/bluetooth/Makefile @@ -31,4 +31,8 @@ hci_uart-$(CONFIG_BT_HCIUART_ATH3K) += hci_ath.o hci_uart-$(CONFIG_BT_HCIUART_3WIRE)+= hci_h5.o hci_uart-objs := $(hci_uart-y) +obj-$(CONFIG_BT_NOKIA_H4P) += nokia_h4p.o + +nokia_h4p-y:= nokia_core.o nokia_fw.o nokia_uart.o + ccflags-y += -D__CHECK_ENDIAN__ diff --git a/drivers/bluetooth/nokia_core.c b/drivers/bluetooth/nokia_core.c new file mode 100644 index 000..ad461a9 --- /dev/null +++ b/drivers/bluetooth/nokia_core.c @@ -0,0 +1,1140 @@ +/* + * This file is part of Nokia H4P bluetooth driver + * + * Copyright (C) 2005-2008 Nokia Corporation. + * Copyright (C) 2014 Pavel Machek pa...@ucw.cz + * + * 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. + * + * This program 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 + *
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 v7 1/4] Documentation: dt: add common bindings for hwspinlock
On Wed, Jan 21, 2015 at 05:56:37PM +, Suman Anna wrote: On 01/21/2015 06:41 AM, Ohad Ben-Cohen wrote: On Tue, Jan 20, 2015 at 8:05 PM, Tony Lindgren t...@atomide.com wrote: How about default to Linux id space and allow overriding that with a module param option if needed? I'm not sure I'm following. If the main point of contention is the base_id field, I'm also fine with removing it entirely, as I'm not aware of any actual user for it (Suman please confirm?). Yeah, well the current implementations that I am aware of only have a single bank, so all of them would be using a value of 0. I am yet to see a platform with multiple instances where the property really makes a difference. v7 has the property mandatory, so all the implementations would need to define this value even if it is 0. regards Suman Mark? Rob? Will you accept Suman's patches if the base_id field is removed? My concern is that the mapping of hwspinlock IDs doesn't seem to be explicit in the DT on a per-context basis, which is what I'd expect. e.g. lck: hwspinlock-device@f00 { ... #hwlock-cells = 1; }; some-other-os-interface { ... hwlocks = lck 0, lck 1, lck 2, lck 3; hwlock-names = glbl, pool0, pool1, pool2; }; a-different-os-interface { ... hwlocks = lck 18, lck 21, lck 4, lck 5; hwlock-names = init, teardown, pool0, pool1; }; That's the only way I would expect this to possibly remain a stable over time, and it's the entire reason for #hwlock-cells, no? How do you expect the other components sharing the hwspinlocks to be described? Thanks, Mark. -- 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 Fri, Jan 16, 2015 at 06:09:00AM +, Ohad Ben-Cohen wrote: On Thu, Jan 15, 2015 at 4:42 PM, Rob Herring robherri...@gmail.com wrote: On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland mark.rutl...@arm.com wrote: On Thu, Jan 15, 2015 at 01:52:01PM +, Mark Rutland wrote: On Wed, Jan 14, 2015 at 08:58:18PM +, Suman Anna wrote: +- hwlock-base-id: An unique base Id for the locks for a particular hwlock + device. This property is mandatory for all platform + implementations. This property makes no sense. The ID encoded in the hwlock cells is relative to the instance (identified by phandle), not global. So the DT has no global ID space. Why do you think you need this? Having looked at the way this proeprty is used, NAK. If you need to carve up a Linux-internal ID space, do that dynamically. There is no need for this property. Better yet, don't create a Linux ID space for this. Everywhere we have one, we want to get rid of it. Rob, Mark, The hwlock is a basic hardware primitive that allow synchronization between different processors in the system, which may be running Linux as well as other operating systems, and may have no other means of communication. The hwlock id numbers are predefined, global and static across the entire system: Linux may boot well after other operating systems are already running and using these hwlocks to communicate, and therefore, in order to use these hardware devices, it must not enumerate them differently than the rest of the system. That's not true. In order to communicate it must agree with the other users as to the meaning of each instance, and the protocol for use. That doesn't necessarily mean that Linux needs to know the numerical ID from a datasheet, and regardless that ID is separate from the logical ID Linux uses internally. Given that these id numbers are global, system-wide, static and predefined, where Linux may just be one user of them, please reconsider the approach as implemented by Suman, or suggest an alternative one you may prefer. To communicate with the other processors, Linux will need to understand the protocol. So there will need to be nodes in the DT describing the protocol. Those nodes can refer to the relevant locks using phandle + args (with a hwlock-names list if indexing is not appropriate). The entire point of the hwlock-cells is to allow individual locks to be referred to in this way. Thanks, Mark. -- 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, Jan 15, 2015 at 02:42:23PM +, Rob Herring wrote: On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland mark.rutl...@arm.com wrote: On Thu, Jan 15, 2015 at 01:52:01PM +, Mark Rutland wrote: On Wed, Jan 14, 2015 at 08:58:18PM +, Suman Anna wrote: This patch adds the generic common bindings used to represent a hwlock device and use/request locks in a device-tree build. All the platform-specific hwlock driver implementations need the number of locks and associated base id for registering the locks present within the device with the driver core. This base id needs to be unique across multiple IP instances of a hwspinlock device, so that each hwlock can be represented uniquely in a system. The number of locks is represented by 'hwlock-num-locks' property, and the base id is represented by the 'hwlock-base-id' property. The args specifier length is dependent on each vendor-specific implementation and is represented through the '#hwlock-cells' property. Client users need to use the property 'hwlocks' for requesting specific lock(s). Note that the document is named hwlock.txt deliberately to keep it a bit more generic. Cc: Rob Herring robh...@kernel.org Signed-off-by: Suman Anna s-a...@ti.com --- v7: Revised binding info for hwlock-base-id, it is mandatory now .../devicetree/bindings/hwlock/hwlock.txt | 55 ++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt new file mode 100644 index ..8de7aaf878f9 --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt @@ -0,0 +1,55 @@ +Generic hwlock bindings +=== + +Generic bindings that are common to all the hwlock platform specific driver +implementations. + +The validity and need of these common properties may vary from one platform +implementation to another. The platform specific bindings should explicitly +state if an optional property is used. Please also look through the individual +platform specific hwlock binding documentations for identifying the applicable +properties. + +Common properties: +- #hwlock-cells: Specifies the number of cells needed to represent a + specific lock. This property is mandatory for all + platform implementations. +- hwlock-num-locks:Number of locks present in a hwlock device. This + property is needed on hwlock devices, where the number + of supported locks within a hwlock device cannot be + read from a register. +- hwlock-base-id: An unique base Id for the locks for a particular hwlock + device. This property is mandatory for all platform + implementations. This property makes no sense. The ID encoded in the hwlock cells is relative to the instance (identified by phandle), not global. So the DT has no global ID space. Why do you think you need this? Having looked at the way this proeprty is used, NAK. If you need to carve up a Linux-internal ID space, do that dynamically. There is no need for this property. Better yet, don't create a Linux ID space for this. Everywhere we have one, we want to get rid of it. Agreed. A completely opaque token / desc structure would prevent a lot of potential abuse and save us from painful breakage. Mark -- 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 Wed, Jan 14, 2015 at 08:58:18PM +, Suman Anna wrote: This patch adds the generic common bindings used to represent a hwlock device and use/request locks in a device-tree build. All the platform-specific hwlock driver implementations need the number of locks and associated base id for registering the locks present within the device with the driver core. This base id needs to be unique across multiple IP instances of a hwspinlock device, so that each hwlock can be represented uniquely in a system. The number of locks is represented by 'hwlock-num-locks' property, and the base id is represented by the 'hwlock-base-id' property. The args specifier length is dependent on each vendor-specific implementation and is represented through the '#hwlock-cells' property. Client users need to use the property 'hwlocks' for requesting specific lock(s). Note that the document is named hwlock.txt deliberately to keep it a bit more generic. Cc: Rob Herring robh...@kernel.org Signed-off-by: Suman Anna s-a...@ti.com --- v7: Revised binding info for hwlock-base-id, it is mandatory now .../devicetree/bindings/hwlock/hwlock.txt | 55 ++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt new file mode 100644 index ..8de7aaf878f9 --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt @@ -0,0 +1,55 @@ +Generic hwlock bindings +=== + +Generic bindings that are common to all the hwlock platform specific driver +implementations. + +The validity and need of these common properties may vary from one platform +implementation to another. The platform specific bindings should explicitly +state if an optional property is used. Please also look through the individual +platform specific hwlock binding documentations for identifying the applicable +properties. + +Common properties: +- #hwlock-cells: Specifies the number of cells needed to represent a + specific lock. This property is mandatory for all + platform implementations. +- hwlock-num-locks: Number of locks present in a hwlock device. This + property is needed on hwlock devices, where the number + of supported locks within a hwlock device cannot be + read from a register. +- hwlock-base-id:An unique base Id for the locks for a particular hwlock + device. This property is mandatory for all platform + implementations. This property makes no sense. The ID encoded in the hwlock cells is relative to the instance (identified by phandle), not global. So the DT has no global ID space. Why do you think you need this? The definition here isn't sufficient, and the example is incomplete (lacking provider nodes and hence this property), which is unhelpful. Thanks, Mark. + +Hwlock Users: += + +Nodes that require specific hwlock(s) should specify them using the property +hwlocks, each containing a phandle to the hwlock node and an args specifier +value as indicated by #hwlock-cells. Multiple hwlocks can be requested using +an array of the phandle and hwlock number specifier tuple. + +1. Example of a node using a single specific hwlock: + +The following example has a node requesting a hwlock in the bank defined by +the node hwlock1. hwlock1 is a hwlock provider with an argument specifier +of length 1. + + node { + ... + hwlocks = hwlock1 2; + ... + }; + +2. Example of a node using multiple specific hwlocks: + +The following example has a node requesting two hwlocks, a hwlock within +the hwlock device node 'hwlock1' with #hwlock-cells value of 1, and another +hwlock within the hwlock device node 'hwlock2' with #hwlock-cells value of 2. + + node { + ... + hwlocks = hwlock1 2, hwlock2 0 3; + ... + }; -- 2.2.1 -- 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, Jan 15, 2015 at 01:52:01PM +, Mark Rutland wrote: On Wed, Jan 14, 2015 at 08:58:18PM +, Suman Anna wrote: This patch adds the generic common bindings used to represent a hwlock device and use/request locks in a device-tree build. All the platform-specific hwlock driver implementations need the number of locks and associated base id for registering the locks present within the device with the driver core. This base id needs to be unique across multiple IP instances of a hwspinlock device, so that each hwlock can be represented uniquely in a system. The number of locks is represented by 'hwlock-num-locks' property, and the base id is represented by the 'hwlock-base-id' property. The args specifier length is dependent on each vendor-specific implementation and is represented through the '#hwlock-cells' property. Client users need to use the property 'hwlocks' for requesting specific lock(s). Note that the document is named hwlock.txt deliberately to keep it a bit more generic. Cc: Rob Herring robh...@kernel.org Signed-off-by: Suman Anna s-a...@ti.com --- v7: Revised binding info for hwlock-base-id, it is mandatory now .../devicetree/bindings/hwlock/hwlock.txt | 55 ++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt new file mode 100644 index ..8de7aaf878f9 --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt @@ -0,0 +1,55 @@ +Generic hwlock bindings +=== + +Generic bindings that are common to all the hwlock platform specific driver +implementations. + +The validity and need of these common properties may vary from one platform +implementation to another. The platform specific bindings should explicitly +state if an optional property is used. Please also look through the individual +platform specific hwlock binding documentations for identifying the applicable +properties. + +Common properties: +- #hwlock-cells: Specifies the number of cells needed to represent a + specific lock. This property is mandatory for all + platform implementations. +- hwlock-num-locks:Number of locks present in a hwlock device. This + property is needed on hwlock devices, where the number + of supported locks within a hwlock device cannot be + read from a register. +- hwlock-base-id: An unique base Id for the locks for a particular hwlock + device. This property is mandatory for all platform + implementations. This property makes no sense. The ID encoded in the hwlock cells is relative to the instance (identified by phandle), not global. So the DT has no global ID space. Why do you think you need this? Having looked at the way this proeprty is used, NAK. If you need to carve up a Linux-internal ID space, do that dynamically. There is no need for this property. Mark. -- 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: BCM2048 bluetooth connected over OMAP serial
On Wed, Dec 10, 2014 at 05:02:42PM +, Arnd Bergmann wrote: On Wednesday 10 December 2014 17:43:33 Pavel Machek wrote: So, there's bluetooth chip that's connected to the SoC by UART and some GPIOs. What would be right representation in the device tree? Something like this? bluetooth { compatible = broadcom,bcm2048; uart = uart2; reset-gpios = gpio3 27 GPIO_ACTIVE_HIGH; /* want 91 */ host-wakeup-gpios = gpio4 5 GPIO_ACTIVE_HIGH; /* want 101 */ bluetooth-wakeup-gpios = gpio2 5 GPIO_ACTIVE_HIGH; /* want 37 */ chip-type = ; bt-sysclk = 2; reset-gpio-shared = 0; }; Is there some way to prevent OMAP tty driver from binding to the device and exporting the device to userspace? I think from the driver perspective, you want this to be a tty line discipline rather than a driver that attaches to the physical uart. For the DT representation, I fear we haven't got a precedent. A uart phandle sounds reasonable, but there might be other ways to do it and we should consider if there are better alternatives. It could possibly be a child node of the uart, but that would require other infrastructure in the kernel because we don't currently create devices for those. I think the child node is the way to go; that would match what we do for I2C and SPI. We might need new infrastructure, but I don't think we should treat this differently simlpy because we don't have that yet. Mark. -- 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 v5 4/7] ARM: l2c: Add support for overriding prefetch settings
On Wed, Sep 24, 2014 at 12:05:38PM +0100, Marek Szyprowski wrote: From: Tomasz Figa t.f...@samsung.com Firmware on certain boards (e.g. ODROID-U3) can leave incorrect L2C prefetch settings configured in registers leading to crashes if L2C is enabled without overriding them. This patch introduces bindings to enable prefetch settings to be specified from DT and necessary support in the driver. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- Documentation/devicetree/bindings/arm/l2cc.txt | 10 +++ arch/arm/mm/cache-l2x0.c | 39 ++ 2 files changed, 49 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt index af527ee111c2..3443d2d76788 100644 --- a/Documentation/devicetree/bindings/arm/l2cc.txt +++ b/Documentation/devicetree/bindings/arm/l2cc.txt @@ -47,6 +47,16 @@ Optional properties: - cache-id-part: cache id part number to be used if it is not present on hardware - wt-override: If present then L2 is forced to Write through mode +- arm,double-linefill : Override double linefill enable setting. Enable if + non-zero, disable if zero. +- arm,double-linefill-incr : Override double linefill on INCR read. Enable + if non-zero, disable if zero. +- arm,double-linefill-wrap : Override double linefill on WRAP read. Enable + if non-zero, disable if zero. +- arm,prefetch-drop : Override prefetch drop enable setting. Enable if non-zero, + disable if zero. I'm not too keen on tristate properties. Is this level of flexibility really required? What exact overrides do you need for boards you know of? Why do these cause crashes if not overridden? Mark. +- arm,prefetch-offset : Override prefetch offset value. Valid values are + 0-7, 15, 23, and 31. Example: diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c index 84c6c55ab896..af90a6ff6b49 100644 --- a/arch/arm/mm/cache-l2x0.c +++ b/arch/arm/mm/cache-l2x0.c @@ -1059,6 +1059,8 @@ static void __init l2c310_of_parse(const struct device_node *np, u32 data[3] = { 0, 0, 0 }; u32 tag[3] = { 0, 0, 0 }; u32 filter[2] = { 0, 0 }; + u32 prefetch; + u32 val; of_property_read_u32_array(np, arm,tag-latency, tag, ARRAY_SIZE(tag)); if (tag[0] tag[1] tag[2]) @@ -1083,6 +1085,43 @@ static void __init l2c310_of_parse(const struct device_node *np, l2x0_saved_regs.filter_start = (filter[0] ~(SZ_1M - 1)) | L310_ADDR_FILTER_EN; } + + prefetch = l2x0_saved_regs.prefetch_ctrl; + + if (!of_property_read_u32(np, arm,double-linefill, val)) { + if (val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL; + else + prefetch = ~L310_PREFETCH_CTRL_DBL_LINEFILL; + } + + if (!of_property_read_u32(np, arm,double-linefill-incr, val)) { + if (val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL_INCR; + else + prefetch = ~L310_PREFETCH_CTRL_DBL_LINEFILL_INCR; + } + + if (!of_property_read_u32(np, arm,double-linefill-wrap, val)) { + if (!val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL_WRAP; + else + prefetch = ~L310_PREFETCH_CTRL_DBL_LINEFILL_WRAP; + } + + if (!of_property_read_u32(np, arm,prefetch-drop, val)) { + if (val) + prefetch |= L310_PREFETCH_CTRL_PREFETCH_DROP; + else + prefetch = ~L310_PREFETCH_CTRL_PREFETCH_DROP; + } + + if (!of_property_read_u32(np, arm,prefetch-offset, val)) { + prefetch = ~L310_PREFETCH_CTRL_OFFSET_MASK; + prefetch |= val L310_PREFETCH_CTRL_OFFSET_MASK; + } + + l2x0_saved_regs.prefetch_ctrl = prefetch; } static const struct l2c_init_data of_l2c310_data __initconst = { -- 1.9.2 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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 v5 4/7] ARM: l2c: Add support for overriding prefetch settings
On Wed, Sep 24, 2014 at 12:19:45PM +0100, Tomasz Figa wrote: On 24.09.2014 13:14, Mark Rutland wrote: On Wed, Sep 24, 2014 at 12:05:38PM +0100, Marek Szyprowski wrote: From: Tomasz Figa t.f...@samsung.com Firmware on certain boards (e.g. ODROID-U3) can leave incorrect L2C prefetch settings configured in registers leading to crashes if L2C is enabled without overriding them. This patch introduces bindings to enable prefetch settings to be specified from DT and necessary support in the driver. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- Documentation/devicetree/bindings/arm/l2cc.txt | 10 +++ arch/arm/mm/cache-l2x0.c | 39 ++ 2 files changed, 49 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt index af527ee111c2..3443d2d76788 100644 --- a/Documentation/devicetree/bindings/arm/l2cc.txt +++ b/Documentation/devicetree/bindings/arm/l2cc.txt @@ -47,6 +47,16 @@ Optional properties: - cache-id-part: cache id part number to be used if it is not present on hardware - wt-override: If present then L2 is forced to Write through mode +- arm,double-linefill : Override double linefill enable setting. Enable if + non-zero, disable if zero. +- arm,double-linefill-incr : Override double linefill on INCR read. Enable + if non-zero, disable if zero. +- arm,double-linefill-wrap : Override double linefill on WRAP read. Enable + if non-zero, disable if zero. +- arm,prefetch-drop : Override prefetch drop enable setting. Enable if non-zero, + disable if zero. I'm not too keen on tristate properties. Is this level of flexibility really required? What exact overrides do you need for boards you know of? Why do these cause crashes if not overridden? Well, this is all thanks to broken firmware, which doesn't initialize those values properly on certain systems and they must be overridden. On the other side, there are systems with firmware that does it correctly and for those the boot defaults should be preferred. I don't see any other reasonable option of handling this. With the lack of warnings for present but empty properties, I can forsee people placing empty properties (as the names make them sound like booleans which enable features). Surely for enabling/disabling options we should only need to override one-way, disabling a feature that causes breakage for some reason? Otherwise we can keep the reset value (which might be sub-optimal). Perhaps a simple warning is sufficient if the property exists but is empty. As for why they cause crashes, I'm not an expert if it is about L2C internals, so I can't really tell, but apparently the cache can work correctly only on certain settings. Likewise, I'm no expert on the l2x0 family. It would be nice to know if this justs masks an issue elsewhere in Linux, is required for some reason by the interconnect, etc. I guess we don't have enough information to figure that out. Mark. -- 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/5] usb: dwc3: exynos: Add support for SCLK present on Exynos7
On Tue, Sep 02, 2014 at 11:39:08AM +0100, Vivek Gautam wrote: Hi, On Fri, Aug 29, 2014 at 12:18 AM, Mark Rutland mark.rutl...@arm.com wrote: On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote: Exynos7 also has a separate special gate clock going to the IP apart from the usual AHB clock. So add support for the same. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/dwc3/dwc3-exynos.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index f9fb8ad..bab6395 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -35,6 +35,7 @@ struct dwc3_exynos { struct device *dev; struct clk *clk; + struct clk *sclk; struct regulator*vdd33; struct regulator*vdd10; }; @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev) return -EINVAL; } + /* Exynos7 has a special gate clock going to this IP */ + exynos-sclk = devm_clk_get(dev, usbdrd30_sclk); + if (IS_ERR(exynos-sclk)) + dev_warn(dev, couldn't get sclk\n); Doesn't this introduce a pointless warning for Exynos SoCs other than Exynos7? True, it will introduce an unnecessary warning for non-Exynos7 systems. I initially thought of introducing a compatible check for Exynos7-dwc3, but that way we may end up adding such checks for future SoCs which have similar controller but have some clock difference or some other small change, no ? Perhaps. I don't know what your future hardware will look like. Is the usbdrd30_sclk input unique to Exynos7, or was it previously there but just without an input? If the latter you could just reduce this to: dev_info(dev, no sclk specified); Mark. -- 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 2/5] phy: exynos5-usbdrd: Add pipe-clk and utmi-clk support
On Thu, Aug 28, 2014 at 09:01:57AM +0100, Vivek Gautam wrote: Exynos7 SoC has now separate gate control for 125MHz pipe3 phy clock, as well as 60MHz utmi phy clock. So get the same and control in the phy-exynos5-usbdrd driver. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- .../devicetree/bindings/phy/samsung-phy.txt|4 drivers/phy/phy-exynos5-usbdrd.c | 24 2 files changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt index 7a6feea..b64d616 100644 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt @@ -135,6 +135,10 @@ Required properties: PHY operations, associated by phy name. It is used to determine bit values for clock settings register. For Exynos5420 this is given as 'sclk_usbphy30' in CMU. + - optional clocks: Next gen Exynos SoCs have following additional It's not going to be 'Next gen' for long... +gate clocks available: +- phy_pipe: for PIPE3 phy +- phy_utmi: for UTMI+ phy - samsung,pmu-syscon: phandle for PMU system controller interface, used to control pmu registers for power isolation. - #phy-cells : from the generic PHY bindings, must be 1; diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c index b05302b..685c108 100644 --- a/drivers/phy/phy-exynos5-usbdrd.c +++ b/drivers/phy/phy-exynos5-usbdrd.c @@ -148,6 +148,8 @@ struct exynos5_usbdrd_phy_drvdata { * @dev: pointer to device instance of this platform device * @reg_phy: usb phy controller register memory base * @clk: phy clock for register access + * @pipeclk: clock for pipe3 phy + * @utmiclk: clock for utmi+ phy * @drv_data: pointer to SoC level driver data structure * @phys[]: array for 'EXYNOS5_DRDPHYS_NUM' number of PHY * instances each with its 'phy' and 'phy_cfg'. @@ -161,6 +163,8 @@ struct exynos5_usbdrd_phy { struct device *dev; void __iomem *reg_phy; struct clk *clk; + struct clk *pipeclk; + struct clk *utmiclk; const struct exynos5_usbdrd_phy_drvdata *drv_data; struct phy_usb_instance { struct phy *phy; @@ -446,6 +450,10 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy) dev_dbg(phy_drd-dev, Request to power_on usbdrd_phy phy\n); + if (!IS_ERR(phy_drd-utmiclk)) + clk_prepare_enable(phy_drd-utmiclk); + if (!IS_ERR(phy_drd-pipeclk)) + clk_prepare_enable(phy_drd-pipeclk); clk_prepare_enable(phy_drd-ref_clk); /* Enable VBUS supply */ @@ -464,6 +472,10 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy) fail_vbus: clk_disable_unprepare(phy_drd-ref_clk); + if (!IS_ERR(phy_drd-pipeclk)) + clk_disable_unprepare(phy_drd-pipeclk); + if (!IS_ERR(phy_drd-utmiclk)) + clk_disable_unprepare(phy_drd-utmiclk); return ret; } @@ -483,6 +495,10 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy) regulator_disable(phy_drd-vbus); clk_disable_unprepare(phy_drd-ref_clk); + if (!IS_ERR(phy_drd-pipeclk)) + clk_disable_unprepare(phy_drd-pipeclk); + if (!IS_ERR(phy_drd-utmiclk)) + clk_disable_unprepare(phy_drd-utmiclk); return 0; } @@ -581,6 +597,14 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) return PTR_ERR(phy_drd-clk); } + phy_drd-pipeclk = devm_clk_get(dev, phy_pipe); + if (IS_ERR(phy_drd-pipeclk)) + dev_warn(dev, Failed to get pipe3 phy operational clock\n); + + phy_drd-utmiclk = devm_clk_get(dev, phy_utmi); + if (IS_ERR(phy_drd-utmiclk)) + dev_warn(dev, Failed to get utmi phy operational clock\n); + Pointless warnings for !Exynos7? Would it not be better to set these to NULL and not litter the code with IS_ERR checks? Mark. -- 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/5] usb: dwc3: exynos: Add support for SCLK present on Exynos7
On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote: Exynos7 also has a separate special gate clock going to the IP apart from the usual AHB clock. So add support for the same. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/dwc3/dwc3-exynos.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index f9fb8ad..bab6395 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -35,6 +35,7 @@ struct dwc3_exynos { struct device *dev; struct clk *clk; + struct clk *sclk; struct regulator*vdd33; struct regulator*vdd10; }; @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev) return -EINVAL; } + /* Exynos7 has a special gate clock going to this IP */ + exynos-sclk = devm_clk_get(dev, usbdrd30_sclk); + if (IS_ERR(exynos-sclk)) + dev_warn(dev, couldn't get sclk\n); Doesn't this introduce a pointless warning for Exynos SoCs other than Exynos7? + exynos-dev = dev; exynos-clk = clk; clk_prepare_enable(exynos-clk); + if (!IS_ERR(exynos-sclk)) + clk_prepare_enable(exynos-sclk); If you replaced the returned err value with NULL you could avoid these IS_ERR cases. Mark. exynos-vdd33 = devm_regulator_get(dev, vdd33); if (IS_ERR(exynos-vdd33)) { @@ -187,6 +195,8 @@ err4: err3: regulator_disable(exynos-vdd33); err2: + if (!IS_ERR(exynos-sclk)) + clk_disable_unprepare(exynos-sclk); clk_disable_unprepare(clk); return ret; } @@ -199,6 +209,8 @@ static int dwc3_exynos_remove(struct platform_device *pdev) platform_device_unregister(exynos-usb2_phy); platform_device_unregister(exynos-usb3_phy); + if (!IS_ERR(exynos-sclk)) + clk_disable_unprepare(exynos-sclk); clk_disable_unprepare(exynos-clk); regulator_disable(exynos-vdd33); @@ -220,6 +232,8 @@ static int dwc3_exynos_suspend(struct device *dev) { struct dwc3_exynos *exynos = dev_get_drvdata(dev); + if (!IS_ERR(exynos-sclk)) + clk_disable(exynos-sclk); clk_disable(exynos-clk); regulator_disable(exynos-vdd33); @@ -245,6 +259,8 @@ static int dwc3_exynos_resume(struct device *dev) } clk_enable(exynos-clk); + if (!IS_ERR(exynos-sclk)) + clk_enable(exynos-sclk); /* runtime set active to reflect active state. */ pm_runtime_disable(dev); -- 1.7.10.4 -- 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 v2 3/8] ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus
On Tue, Aug 19, 2014 at 08:40:51PM +0100, Jyri Sarha wrote: On 08/19/2014 04:16 PM, Mark Rutland wrote: On Mon, Aug 18, 2014 at 10:46:41PM +0100, Jyri Sarha wrote: Add machine driver support for BeagleBone-Black HDMI audio. BBB has NXP TDA998X HDMI transmitter connected to McASP port in I2S mode. The 44100 Hz sample-rate and it's multiples can not be accurately produced on BBB. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE. The 8 least significant bits are ignored. Signed-off-by: Jyri Sarha jsa...@ti.com --- .../bindings/sound/davinci-evm-audio.txt |4 +- sound/soc/davinci/davinci-evm.c| 82 +++- 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt index 963e100..c137436 100644 --- a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt +++ b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt @@ -1,7 +1,9 @@ * Texas Instruments SoC audio setups with TLV320AIC3X Codec Required properties: -- compatible : ti,da830-evm-audio : forDM365/DA8xx/OMAPL1x/AM33xx +- compatible : ti,da830-evm-audio : for DM365/DA8xx/OMAPL1x/AM33xx + ti,am335x-beaglebone-black-audio : for Beaglebone-black HDMI +audio To keep this legible I'd recommend reorganising this like: - compatible: should contain one of: * ti,da830-evm-audio for DM365/DA8xx/OMAPL1x/AM33xx * ti,am335x-beaglebone-black-audio for Beaglebone-black HDMI audio The above looks indeed better. I'll make the change. Oh, and I probably should change the first line to something like: * TI SoC audio using McASP to connect to TLV320AIC3X or HDMI endcoder For the BBB case, do you expect both strings or just the BBB-specific string? Normally just the one. In theory a BBB with an audio-cape on top could utilize another sound node with ti,da830-evm-audio compatible property. Sorry, I meant in the same node. I take it we never expect: compatible = ti,am335x-beaglebone-black-audio, ti,da830-evm-audio; Is the 'x' in the BBB string a wildcard? If we know the particular number for BBB we should use that. Yes, its a wild card. BBBs have been made at least with AM3358 and AM3359 SoCs. In general we've pushed back against wildcard strings, and used the first implementation for the naming. We should be able to use ti,am3358-beaglebone-black-audio for AM335{8,9} assuming the block is the same? Otherwise we might need separate strings anyway. Thanks, Mark. -- 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 v2 1/8] clk: ti: add gpio controlled clock
On Mon, Aug 18, 2014 at 10:46:39PM +0100, Jyri Sarha wrote: The added ti,gpio-clock is a basic clock that can be enabled and disabled trough a gpio output. The DT binding document for the clock is also added. For EPROBE_DEFER handling the registering of the clock has to be delayed until of_clk_get() call time. I guess this is basically an AND gate with the GPIO and parent clock as inputs? Or is this something more complex that we might want to do more things with later? If it's the former it sounds like this could be a completely generic gpio-gate-clock. Thanks, Mark. Signed-off-by: Jyri Sarha jsa...@ti.com --- .../devicetree/bindings/clock/ti/gpio-clock.txt| 21 ++ drivers/clk/ti/Makefile|2 +- drivers/clk/ti/gpio.c | 202 3 files changed, 224 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/gpio-clock.txt create mode 100644 drivers/clk/ti/gpio.c diff --git a/Documentation/devicetree/bindings/clock/ti/gpio-clock.txt b/Documentation/devicetree/bindings/clock/ti/gpio-clock.txt new file mode 100644 index 000..2eb854d --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/gpio-clock.txt @@ -0,0 +1,21 @@ +Binding for simple gpio controlled clock. + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be ti,gpio-clock. +- #clock-cells : from common clock binding; shall be set to 0. +- enable-gpios : GPIO reference for enabling and disabling the clock. + +Optional properties: +- clocks: Maximum of one parent clock is supported. + +Example: + clock { + compatible = ti,gpio-clock; + clocks = parentclk; + #clock-cells = 0; + enable-gpios = gpio 1 GPIO_ACTIVE_HIGH; + }; diff --git a/drivers/clk/ti/Makefile b/drivers/clk/ti/Makefile index ed4d0aa..88074d3 100644 --- a/drivers/clk/ti/Makefile +++ b/drivers/clk/ti/Makefile @@ -1,7 +1,7 @@ ifneq ($(CONFIG_OF),) obj-y+= clk.o autoidle.o clockdomain.o clk-common = dpll.o composite.o divider.o gate.o \ - fixed-factor.o mux.o apll.o + fixed-factor.o mux.o apll.o gpio.o obj-$(CONFIG_SOC_AM33XX) += $(clk-common) clk-33xx.o obj-$(CONFIG_ARCH_OMAP2) += $(clk-common) interface.o clk-2xxx.o obj-$(CONFIG_ARCH_OMAP3) += $(clk-common) interface.o clk-3xxx.o diff --git a/drivers/clk/ti/gpio.c b/drivers/clk/ti/gpio.c new file mode 100644 index 000..f4c668e --- /dev/null +++ b/drivers/clk/ti/gpio.c @@ -0,0 +1,202 @@ +/* + * Copyright (C) 2012 - 2014 Texas Instruments Incorporated - http://www.ti.com + * Author: Jyri Sarha jsa...@ti.com + * + * 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. + * + * Gpio controlled clock implementation + */ + +#include linux/clk-provider.h +#include linux/module.h +#include linux/slab.h +#include linux/gpio.h +#include linux/of_gpio.h +#include linux/err.h +#include linux/device.h + +struct clk_gpio { + struct clk_hw hw; + struct gpio_desc *gpiod; +}; + +/** + * DOC: basic gpio controlled clock which can be enabled and disabled + * with gpio output + * Traits of this clock: + * prepare - clk_(un)prepare only ensures parent is (un)prepared + * enable - clk_enable and clk_disable are functional control gpio + * rate - inherits rate from parent. No clk_set_rate support + * parent - fixed parent. No clk_set_parent support + */ + +#define to_clk_gpio(_hw) container_of(_hw, struct clk_gpio, hw) + +static int clk_gpio_enable(struct clk_hw *hw) +{ + struct clk_gpio *clk = to_clk_gpio(hw); + + gpiod_set_value(clk-gpiod, 1); + + return 0; +} + +static void clk_gpio_disable(struct clk_hw *hw) +{ + struct clk_gpio *clk = to_clk_gpio(hw); + + gpiod_set_value(clk-gpiod, 0); +} + +static int clk_gpio_is_enabled(struct clk_hw *hw) +{ + struct clk_gpio *clk = to_clk_gpio(hw); + + return gpiod_get_value(clk-gpiod); +} + +static const struct clk_ops clk_gpio_ops = { + .enable = clk_gpio_enable, + .disable = clk_gpio_disable, + .is_enabled = clk_gpio_is_enabled, +}; + +/** + * clk_register_gpio - register a gpip clock with the clock framework + * @dev: device that is registering this clock + * @name: name of this clock + * @parent_name: name of this clock's parent + * @gpiod: gpio descriptor to control this clock + */ +static struct clk *clk_register_gpio(struct device *dev, const char *name, + const char *parent_name, struct gpio_desc
Re: [PATCH v2 1/8] clk: ti: add gpio controlled clock
On Tue, Aug 19, 2014 at 01:23:12PM +0100, Jyri Sarha wrote: On 08/19/2014 02:32 PM, Mark Rutland wrote: On Mon, Aug 18, 2014 at 10:46:39PM +0100, Jyri Sarha wrote: The added ti,gpio-clock is a basic clock that can be enabled and disabled trough a gpio output. The DT binding document for the clock is also added. For EPROBE_DEFER handling the registering of the clock has to be delayed until of_clk_get() call time. I guess this is basically an AND gate with the GPIO and parent clock as inputs? Or is this something more complex that we might want to do more things with later? If it's the former it sounds like this could be a completely generic gpio-gate-clock. Yes, its completely generic. However, since there has been no response to my several earlier attempts to get this patch in I finally gave up and moved it under clk/ti. Ah. Apolgoies for that having happened. gpio-gate-clock sound like more accurate name for ti. I'll change that. Do you think I should give this one more try to get it in as generic basic clock? I'd certainly be happy with a generic gpio-gate-clock. Mike, thoughts? Thanks, Mark. -- 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 v2 3/8] ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus
On Mon, Aug 18, 2014 at 10:46:41PM +0100, Jyri Sarha wrote: Add machine driver support for BeagleBone-Black HDMI audio. BBB has NXP TDA998X HDMI transmitter connected to McASP port in I2S mode. The 44100 Hz sample-rate and it's multiples can not be accurately produced on BBB. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE. The 8 least significant bits are ignored. Signed-off-by: Jyri Sarha jsa...@ti.com --- .../bindings/sound/davinci-evm-audio.txt |4 +- sound/soc/davinci/davinci-evm.c| 82 +++- 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt index 963e100..c137436 100644 --- a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt +++ b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt @@ -1,7 +1,9 @@ * Texas Instruments SoC audio setups with TLV320AIC3X Codec Required properties: -- compatible : ti,da830-evm-audio : forDM365/DA8xx/OMAPL1x/AM33xx +- compatible : ti,da830-evm-audio : for DM365/DA8xx/OMAPL1x/AM33xx + ti,am335x-beaglebone-black-audio : for Beaglebone-black HDMI +audio To keep this legible I'd recommend reorganising this like: - compatible: should contain one of: * ti,da830-evm-audio for DM365/DA8xx/OMAPL1x/AM33xx * ti,am335x-beaglebone-black-audio for Beaglebone-black HDMI audio For the BBB case, do you expect both strings or just the BBB-specific string? Is the 'x' in the BBB string a wildcard? If we know the particular number for BBB we should use that. Cheers, Mark. -- 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/1] clk: ti: add support for external clock provider
Hi Tero, On Fri, Aug 01, 2014 at 02:15:48PM +0100, Tero Kristo wrote: External clock provider can now be used to register external clocks under it. This is needed as the TI clock driver only registers clocks hierarchically under clock providers, and external clocks do not belong under any of the current ones. I must admit that I don't understand the justification for this binding. Why can't these clocks be descrbied elsewhere and wired up as usual? Why must they be under the TI clock provide node? From the limited description above it sounds like the only reason this exists is to work around a deficiency in the TI clock driver. That does not sound like a very good reason for having this. Thanks, Mark. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/arm/omap/ext-clocks.txt| 32 arch/arm/mach-omap2/io.c | 12 ++-- drivers/clk/ti/clk.c | 23 ++ include/linux/clk/ti.h |2 ++ 4 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/omap/ext-clocks.txt diff --git a/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt b/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt new file mode 100644 index 000..8e784bb --- /dev/null +++ b/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt @@ -0,0 +1,32 @@ +TI external clock provider properties + +External clock provider is used to group SoC external board specific +clock nodes. A separate provider node is required as the TI clock +driver registers clocks hierarchically. + +Required properties: +- compatible:Shall be: ti,external-clocks +- clocks:Contains the external clocks for the board +- clockdomains: Contains the external clockdomains for the board + +Example: + +ext_clocks { + compatible = ti,external-clocks; + + ext_clocks: clocks { + }; + + ext_clockdomains: clockdomains { + }; +}; + +... + +ext_clocks { + gpio_test_clock { + compatible = gpio-clock; + #clock-cells = 0; + enable-gpios = gpio5 25 GPIO_ACTIVE_HIGH; + }; +}; diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c index 8f55945..77be18b 100644 --- a/arch/arm/mach-omap2/io.c +++ b/arch/arm/mach-omap2/io.c @@ -21,6 +21,8 @@ #include linux/init.h #include linux/io.h #include linux/clk.h +#include linux/clk-provider.h +#include linux/clk/ti.h #include asm/tlb.h #include asm/mach/map.h @@ -729,8 +731,14 @@ int __init omap_clk_init(void) return 0; ret = of_prcm_init(); - if (!ret) - ret = omap_clk_soc_init(); + if (ret) + return ret; + + ret = ti_dt_clk_ext_init(); + if (ret) + return ret; + + ret = omap_clk_soc_init(); return ret; } diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c index b1a6f71..c84ce4d 100644 --- a/drivers/clk/ti/clk.c +++ b/drivers/clk/ti/clk.c @@ -165,3 +165,26 @@ void ti_dt_clk_init_provider(struct device_node *parent, int index) kfree(retry); } } + +static struct of_device_id ti_ext_clk_match_table[] = { + { .compatible = ti,external-clocks }, + { } +}; + +/** + * ti_dt_clk_ext_init - initialize external clocks from DT + * + * Some clocks are provided from external chips outside the main SoC. + * The details for these are given under a special DT node, which will + * be parsed by this function. + */ +int ti_dt_clk_ext_init(void) +{ + struct device_node *np; + + for_each_matching_node(np, ti_ext_clk_match_table) { + ti_dt_clk_init_provider(np, -1); + } + + return 0; +} diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h index e8d8a35..188270c 100644 --- a/include/linux/clk/ti.h +++ b/include/linux/clk/ti.h @@ -310,6 +310,8 @@ int am43xx_dt_clk_init(void); int omap2420_dt_clk_init(void); int omap2430_dt_clk_init(void); +int ti_dt_clk_ext_init(void); + #ifdef CONFIG_OF void of_ti_clk_allow_autoidle_all(void); void of_ti_clk_deny_autoidle_all(void); -- 1.7.9.5 -- 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 07/13] Documentation: devicetree: add bindings for TI PRUSS
On Sun, Jun 29, 2014 at 05:21:41PM +0100, Andre Heider wrote: Signed-off-by: Andre Heider a.hei...@gmail.com --- Documentation/devicetree/bindings/misc/ti,pruss.txt | 19 +++ 1 file changed, 19 insertions(+) create mode 100644 Documentation/devicetree/bindings/misc/ti,pruss.txt diff --git a/Documentation/devicetree/bindings/misc/ti,pruss.txt b/Documentation/devicetree/bindings/misc/ti,pruss.txt new file mode 100644 index 000..4eacc41 --- /dev/null +++ b/Documentation/devicetree/bindings/misc/ti,pruss.txt @@ -0,0 +1,19 @@ +TI Programmable Real-Time Unit Sub System (PRUSS) + +Required properties: +- compatible : + - ti,pruss-v1 - for PRUv1 as found on the OMAPL138/DA850/AM18xx SoC families + - ti,pruss-v2 - for PRUv2 as found on the AM33xx SoC family +- ti,hwmods: Name of the hwmod associated to the PRUSS +- reg: Address range of rtc register set +- interrupts: host event interrupts in order How many of these do we expect? +- interrupt-parent: phandle for the interrupt controller + +Example: +pruss: pruss@4a30 { + compatible = ti,pruss-v2; + ti,hwmods = pruss; + reg = 0x4a30 0x08; + interrupts = 20 21 22 23 24 25 26 27; Assuming these represent more than one interrupt, could you please bracket them individually? e.g. interrupts = 20 21, 22 23, 24 25, 26 27; It makes it far clearer that it's a list of multi-cell elements rather than a giant binary blob, and usually makes it easier to read a dts. Thanks, Mark. -- 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 09/13] uio: uio_pruss: add devicetree support
On Sun, Jun 29, 2014 at 05:21:43PM +0100, Andre Heider wrote: Add support to probe via devicetree. Signed-off-by: Andre Heider a.hei...@gmail.com --- drivers/uio/uio_pruss.c | 46 +++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c index afaf726..2df54ab 100644 --- a/drivers/uio/uio_pruss.c +++ b/drivers/uio/uio_pruss.c @@ -26,6 +26,7 @@ #include linux/dma-mapping.h #include linux/slab.h #include linux/genalloc.h +#include linux/of_device.h #define DRV_NAME pruss_uio #define DRV_VERSION 1.0 @@ -70,6 +71,27 @@ struct uio_pruss_dev { struct gen_pool *sram_pool; }; +#ifdef CONFIG_OF +struct uio_pruss_params { + u32 pintc_offset; +}; + +static const struct uio_pruss_params uio_pruss_v1_params = { + .pintc_offset = 0x4000, +}; + +static const struct uio_pruss_params uio_pruss_v2_params = { + .pintc_offset = 0x2, +}; + +static const struct of_device_id pruss_of_match_table[] = { + { .compatible = ti,pruss-v1, .data = uio_pruss_v1_params, }, + { .compatible = ti,pruss-v2, .data = uio_pruss_v2_params, }, + {}, +}; +MODULE_DEVICE_TABLE(of, pruss_of_match_table); +#endif + static irqreturn_t pruss_handler(int irq, struct uio_info *info) { struct uio_pruss_dev *gdev = info-priv; @@ -111,6 +133,8 @@ static int pruss_probe(struct platform_device *pdev) struct uio_pruss_dev *gdev; struct resource *regs_prussio; struct device *dev = pdev-dev; + const struct of_device_id *match; + const struct uio_pruss_params *params; int ret = -ENODEV, cnt = 0, i; struct uio_pruss_pdata *pdata = dev_get_platdata(dev); dma_addr_t ddr_paddr; @@ -123,13 +147,21 @@ static int pruss_probe(struct platform_device *pdev) if (!gdev-info) return -ENOMEM; - /* Power on PRU in case its not done as part of boot-loader */ - gdev-pruss_clk = clk_get(dev, pruss); - if (IS_ERR(gdev-pruss_clk)) { - dev_err(dev, Failed to get clock\n); - return PTR_ERR(gdev-pruss_clk); + if (dev-of_node) { + match = of_match_device(pruss_of_match_table, dev); + params = match-data; + gdev-pintc_base = params-pintc_offset; } else { + /* Power on PRU in case its not done as part of boot-loader */ + gdev-pruss_clk = clk_get(dev, pruss); + if (IS_ERR(gdev-pruss_clk)) { + dev_err(dev, Failed to get clock\n); + return PTR_ERR(gdev-pruss_clk); + } The pruss clock was not documented in the binding. Is the clock really called pruss, or is it given a specific name in the manual? Cheers, Mark. -- 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 12/13] ARM: dts: am33xx: add the PRUSSv2 device
On Sun, Jun 29, 2014 at 05:21:46PM +0100, Andre Heider wrote: Signed-off-by: Andre Heider a.hei...@gmail.com --- arch/arm/boot/dts/am33xx.dtsi | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index 4a4e02d..28a7e5d 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -409,6 +409,15 @@ ti,hwmods = rtc; }; + pruss: pruss@4a30 { + compatible = ti,pruss-v2; + ti,hwmods = pruss; + reg = 0x4a30 0x08; + interrupts = 20 21 22 23 24 25 26 27; Could this please have entries individually bracketed? Thanks, Mark. + interrupt-parent = intc; + status = disabled; + }; + spi0: spi@4803 { compatible = ti,omap4-mcspi; #address-cells = 1; -- 2.0.0 -- 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 02/13] uio: uio_pruss: use devm_kzalloc()
On Sun, Jun 29, 2014 at 05:21:36PM +0100, Andre Heider wrote: Replace kzalloc() by devm_kzalloc() and remove the kfree() calls. Signed-off-by: Andre Heider a.hei...@gmail.com --- drivers/uio/uio_pruss.c | 15 --- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c index c28d6e2..f07545b 100644 --- a/drivers/uio/uio_pruss.c +++ b/drivers/uio/uio_pruss.c @@ -109,9 +109,7 @@ static void pruss_cleanup(struct device *dev, struct uio_pruss_dev *gdev) gen_pool_free(gdev-sram_pool, gdev-sram_vaddr, sram_pool_sz); - kfree(gdev-info); clk_put(gdev-pruss_clk); - kfree(gdev); } static int pruss_probe(struct platform_device *pdev) @@ -123,24 +121,19 @@ static int pruss_probe(struct platform_device *pdev) int ret = -ENODEV, cnt = 0, len; struct uio_pruss_pdata *pdata = dev_get_platdata(dev); - gdev = kzalloc(sizeof(struct uio_pruss_dev), GFP_KERNEL); + gdev = devm_kzalloc(dev, sizeof(struct uio_pruss_dev), GFP_KERNEL); If this is changing anyway, how about: gdev = devm_kzalloc(dev, sizeof(*gdev), GFP_KERNEL); Cheers, Mark. -- 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 1/2] arm: dts: am4372: let boards access all nodes through phandles
On Wed, Jun 25, 2014 at 02:40:16AM +0100, Felipe Balbi wrote: Hi, On Tue, Jun 24, 2014 at 04:11:48PM -0500, Rob Herring wrote: On Mon, Jun 23, 2014 at 1:20 PM, Felipe Balbi ba...@ti.com wrote: by providing phandles to rtc, wdt, cpu and dispc nodes, boards can access them to add board-specific data. Strictly speaking, you are adding labels, not phandles. You can do heh, fair point. Easily editable when applying, though (?) phandles without using labels, but the syntax is not so obvious. I'd tell you what it is but offhand I don't remember. :) something along the lines of using the full path ? Yup. The full path in braces: phandle-property = {/full/path/to/node@0xDEAD}; Mark. -- 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 0/2] Sil9022 DPI to HDMI Encoder
On Tue, Mar 18, 2014 at 10:07:55AM +, Sathya Prakash M R wrote: This patch series adds the Silicon Image Sil9022 driver. Sil9022 is HDMI Transmitter compliant to HDMI 1.2a and DVI 1.0. It supports 1080p and UXGA. It has single slave I2C from Host, passing through to master I2C interface for DDC Connection. It provides Transmitter Programming Interface (TPI) for simplified API programming. Product brief: http://www.semiconductorstore.com/pdf/newsite/siliconimage/SiI9022a_pb.pdf Current driver is tested on AM43x SOC. This series is on top of series posted to add DSS support on AM43x [2] [2] https://patchwork.kernel.org/patch/3822691/ This is initial driver supporting limited features. TODO - Audio Support Detailed Hot Plug event handling and also routing the hot plug event to host processor These features will be added in future. Sathya Prakash M R (2): OMAPDSS: Add Sil9022 DPI-HDMI Encoder Driver ARM: DTS: AM43x: Add sii9022 dt information arch/arm/boot/dts/am437x-gp-evm.dts| 58 +- arch/arm/boot/dts/am43x-epos-evm.dts | 59 +- drivers/video/omap2/displays-new/Kconfig |8 + drivers/video/omap2/displays-new/Makefile |1 + drivers/video/omap2/displays-new/encoder-sil9022.c | 748 drivers/video/omap2/displays-new/encoder-sil9022.h | 105 +++ 6 files changed, 973 insertions(+), 6 deletions(-) create mode 100644 drivers/video/omap2/displays-new/encoder-sil9022.c create mode 100644 drivers/video/omap2/displays-new/encoder-sil9022.h Is there already a binding document? If not, please add one. Cheers, Mark. -- 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 v2 4/4] ARM: DTS: AM43x: Add DSS node
On Thu, Mar 13, 2014 at 06:22:54PM +, Tomi Valkeinen wrote: On 13/03/14 19:46, Mark Rutland wrote: On Thu, Mar 13, 2014 at 08:58:29AM +, Sathya Prakash M R wrote: Add device node for DSS module for AM4372. Both the AM437x-Gp evm and Am43x-Epos evm use the same LCD panel. The lcd timings are added in respective dts files. Adds display pinctrl and enables required gpio. Also set the right parent clock to the DSS clock. Signed-off-by: Sathya Prakash M R sath...@ti.com --- arch/arm/boot/dts/am4372.dtsi| 28 + arch/arm/boot/dts/am437x-gp-evm.dts | 77 ++ arch/arm/boot/dts/am43x-epos-evm.dts | 73 arch/arm/boot/dts/am43xx-clocks.dtsi |2 + 4 files changed, 180 insertions(+) diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index ea55a4e..b72a7df 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -684,6 +684,34 @@ num-cs = 4; status = disabled; }; + + dss: dss@4832A000 { + compatible = ti,omap3-dss, simple-bus; This doesn't look right to me. I'm not sure it makes sense for simple-bus to be in the compatible list. Are the child nodes usable in isolation, or are they dependent on the ti,omap3-dss node? What exactly does the ti,omap3-dss node represent? The child nodes are dependent on the dss node. Ok. Then simple-bus is not appropriate, as the dss node cannot be ignored for the children to function. The ti,omap3-dss represents the dss_core block of the OMAP display subsystem. The dss_core is a small IP, a wrapper for the submodules, handling things like clock and video path routing between the submodules and the OMAP's other components (like the PRCM where we get clocks). It also handles reset, so when dss_core is reset, all the submodules are reset. The HW design is a bit odd, in my opinion, as the submodules are proper IP blocks, and as far as I see, they could be designed to be independent of each others. But they have not been designed so. Having dss_core as the parent node for the submodules gives us automatic runtime-pm handling, so when one submodule is enabled, it forces dss_core to be enabled first. This makes the reset work right (i.e. we don't accidentally reset dss_core when one of the submodules is in use), and, as the dss_core is always needed to setup the clock and video path routing, it gets properly initialized before any of the submodules will use it. What simple-bus mostly gives us here is automatic creation of the platform devices for the submodules. We could also create the devices for submodules in the driver of the dss_core. I did have that at some point, but the simple-bus does identical job, and it seemed to make sense to me. The simple-bus compatible string is intended for busses which are transparent (bar some address remapping expressed via ranges), and is not intended as an annotation to get Linux to probe child nodes. Any node with a simple-bus entry in the compatible list should either be handled as a transparent bus, or optionally as the more specific bus it claims to be (where some hardware configuration may be required before children can be probed). Unfortunately Linux probes chidlren regardless, which is arguable a Linux bug. There's no reason to leak this issue into dts files. Please remove the simple-bus string, and get the dss driver to probe children as required -- as described above the dss node never makes sense as a simple-bus. Note that the same method is used for omap2/3/4 also, in the patches that have been going around for some time in the lists. And those should be fixed. Cheers, Mark. -- 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 v2 4/4] ARM: DTS: AM43x: Add DSS node
On Fri, Mar 14, 2014 at 09:42:26AM +, Tomi Valkeinen wrote: On 14/03/14 11:10, Mark Rutland wrote: The simple-bus compatible string is intended for busses which are transparent (bar some address remapping expressed via ranges), and is not intended as an annotation to get Linux to probe child nodes. Any node with a simple-bus entry in the compatible list should either be handled as a transparent bus, or optionally as the more specific bus it claims to be (where some hardware configuration may be required before children can be probed). Unfortunately Linux probes chidlren regardless, which is arguable a Linux bug. There's no reason to leak this issue into dts files. Please remove the simple-bus string, and get the dss driver to probe children as required -- as described above the dss node never makes sense as a simple-bus. Ok. I'll remove the simple-bus, and make the dss_core register the devices. I presume of_platform_populate() is fine for this? Seems to work fine for registration, but I haven't figured out yet how to unregister the devices (I get a crash in platform_device_del() if I just call platform_device_unregister for the submodules). I think of_platform_populate should be ok. It's not fantastic -- all child nodes will be probed, regardless of whether you expect them to exist, but it's not as broken as using simple-bus. I'm unfortunately not familiar with how unregistration works. I can't see anything obviously wrong in platform_device_del. Do you have a backtrace? Thanks, Mark. -- 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 v2 4/4] ARM: DTS: AM43x: Add DSS node
On Fri, Mar 14, 2014 at 11:07:05AM +, Tomi Valkeinen wrote: On 14/03/14 12:19, Tomi Valkeinen wrote: On 14/03/14 12:14, Mark Rutland wrote: I can't see anything obviously wrong in platform_device_del. Do you have a backtrace? Yes, below. I can see at least drivers/usb/dwc3/dwc3-exynos.c doing the exact same thing I do, so maybe I've got something wrong with the omapdss driver. Looks to me that the devices created by of_platform_populate() are not unregisterable in all cases. The address resource created via of_platform_populate() had NULL res-parent, which causes release_resource to crash. Hmm. I can't see that unregistering such devices ever works as you say, given that __release_resource expects a non-NULL parent pointer. Either we should be setting the parent pointer when initialising devices from dt or we should teach __release_resource to not care. I'll have a go at fixing that. It looks like drivers/usb/dwc3/dwc3-exynos.c only unregisters the top-level device, not children. This top-level device has no IORESOURCE_{IO,MEM} resources judging by arch/arm/boot/dts/exynos5250.dtsi, which would explain why that driver isn't exploding: __release_resource will never get called. Anton, Felipe: Does unregistering the parent ensure the children get cleaned up, or does it leave them dangling in the dwc3-exynos driver? Cheers, Mark. -- 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 v2 4/4] ARM: DTS: AM43x: Add DSS node
On Fri, Mar 14, 2014 at 04:34:19PM +, Tomi Valkeinen wrote: On 14/03/14 18:04, Felipe Balbi wrote: On Fri, Mar 14, 2014 at 02:07:45PM +, Mark Rutland wrote: On Fri, Mar 14, 2014 at 11:07:05AM +, Tomi Valkeinen wrote: On 14/03/14 12:19, Tomi Valkeinen wrote: On 14/03/14 12:14, Mark Rutland wrote: I can't see anything obviously wrong in platform_device_del. Do you have a backtrace? Yes, below. I can see at least drivers/usb/dwc3/dwc3-exynos.c doing the exact same thing I do, so maybe I've got something wrong with the omapdss driver. Looks to me that the devices created by of_platform_populate() are not unregisterable in all cases. The address resource created via of_platform_populate() had NULL res-parent, which causes release_resource to crash. Hmm. I can't see that unregistering such devices ever works as you say, given that __release_resource expects a non-NULL parent pointer. Either we should be setting the parent pointer when initialising devices from dt or we should teach __release_resource to not care. I'll have a go at fixing that. It looks like drivers/usb/dwc3/dwc3-exynos.c only unregisters the top-level device, not children. This top-level device has no IORESOURCE_{IO,MEM} resources judging by arch/arm/boot/dts/exynos5250.dtsi, which would explain why that driver isn't exploding: __release_resource will never get called. Anton, Felipe: Does unregistering the parent ensure the children get cleaned up, or does it leave them dangling in the dwc3-exynos driver? you should platform_device_unregister() for each children and dwc3-exynos does that just fine: Yes, that's what I do also, and it crashes. What Mark said above about unregistering never working for such devices is correct, but I don't know why he said dwc3-exynos only unregisters the top level device. Because I'd failed to read the code correctly. It does indeed unregister each of the children via platform_device_unregister. Apologies for the confusion there. such devices above meaning devices with a 'reg' defined in the DT data, if I'm not mistaken. Yes. As far as I can see, IORESOURCE_MEM resources instantiated from reg entries in dt do not have their parent pointer initialised, and will cause __release_resource to blow up. A quick inspection of resources on my TC2 shows this to be the case -- the parent, sibling, and child pointers are all NULL. I'm at a loss to explain how the dwc3-exynos driver can call platform_device_unregister on devices with such resource and not blow up. Is anyone able to test dwc3_exynos_remove? So at the moment, I think of_platform_populate() and platform_device_unregister() combination in a driver is a bit risky. Work fine for certain cases, not for some other. It certainly looks to be broken at the moment, but it should be fixable. Thanks, Mark. -- 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 v2 4/4] ARM: DTS: AM43x: Add DSS node
On Thu, Mar 13, 2014 at 08:58:29AM +, Sathya Prakash M R wrote: Add device node for DSS module for AM4372. Both the AM437x-Gp evm and Am43x-Epos evm use the same LCD panel. The lcd timings are added in respective dts files. Adds display pinctrl and enables required gpio. Also set the right parent clock to the DSS clock. Signed-off-by: Sathya Prakash M R sath...@ti.com --- arch/arm/boot/dts/am4372.dtsi| 28 + arch/arm/boot/dts/am437x-gp-evm.dts | 77 ++ arch/arm/boot/dts/am43x-epos-evm.dts | 73 arch/arm/boot/dts/am43xx-clocks.dtsi |2 + 4 files changed, 180 insertions(+) diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index ea55a4e..b72a7df 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -684,6 +684,34 @@ num-cs = 4; status = disabled; }; + + dss: dss@4832A000 { + compatible = ti,omap3-dss, simple-bus; This doesn't look right to me. I'm not sure it makes sense for simple-bus to be in the compatible list. Are the child nodes usable in isolation, or are they dependent on the ti,omap3-dss node? What exactly does the ti,omap3-dss node represent? Thanks, Mark. + reg = 0x4832A000 0x200; + ti,hwmods = dss_core; + #address-cells = 1; + #size-cells = 1; + ranges; + + dispc@4832A400 { + compatible = ti,omap3-dispc; + reg = 0x4832A400 0x400; + interrupts = GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH; + ti,hwmods = dss_dispc; + }; + + dpi: encoder@0 { + compatible = ti,omap3-dpi; + }; + + rfbi: rfbi@4832A800 { + compatible = ti,omap3-rfbi; + reg = 0x4832A800 0x100; + ti,hwmods = dss_rfbi; + }; + + }; + }; -- 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 v4] ASoC: tlv320aic31xx: Add basic codec driver implementation
On Mon, Mar 10, 2014 at 08:52:21AM +, Jyri Sarha wrote: This commit adds a bare bones driver support for TLV320AIC31XX family audio codecs. The driver adds basic stereo playback trough headphone and speaker outputs and mono capture trough microphone inputs. The driver is currently missing support at least for mini DSP features and jack detection. I have tested the driver only on TLV320AIC3111, but based on the data sheets TLV320AIC3100, TLV320AIC3110, and TLV320AIC3120 should work Ok too. The base for the implementation was taken from: g...@gitorious.org:ti-codecs/ti-codecs.git ajitk/topics/k3.10.1-aic31xx -branch at commit 77504eba0294764e9e63b4a0c696b44db187cd13. Signed-off-by: Jyri Sarha jsa...@ti.com --- .../devicetree/bindings/sound/tlv320aic31xx.txt| 61 + include/dt-bindings/sound/tlv320aic31xx-micbias.h |9 + sound/soc/codecs/Kconfig |4 + sound/soc/codecs/Makefile |2 + sound/soc/codecs/tlv320aic31xx.c | 1343 sound/soc/codecs/tlv320aic31xx.h | 258 6 files changed, 1677 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/tlv320aic31xx.txt create mode 100644 include/dt-bindings/sound/tlv320aic31xx-micbias.h create mode 100644 sound/soc/codecs/tlv320aic31xx.c create mode 100644 sound/soc/codecs/tlv320aic31xx.h diff --git a/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt b/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt new file mode 100644 index 000..0109df1 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt @@ -0,0 +1,61 @@ +Texas Instruments - tlv320aic31xx Codec module + +The tlv320aic31xx serial control bus communicates through I2C protocols + +Required properties: + +- compatible - string - One of: +ti,tlv320aic310x - Generic TLV320AIC31xx with mono speaker amp +ti,tlv320aic311x - Generic TLV320AIC31xx with stereo speaker amp +ti,tlv320aic3100 - TLV320AIC3100 (mono speaker amp, no MiniDSP) +ti,tlv320aic3110 - TLV320AIC3110 (stereo speaker amp, no MiniDSP) +ti,tlv320aic3120 - TLV320AIC3120 (mono speaker amp, MiniDSP) +ti,tlv320aic3111 - TLV320AIC3111 (stereo speaker amp, MiniDSP) + +- reg - int - I2C slave address + + +Optional properties: + +- gpio-reset - gpio pin number used for codec reset I believe calling this reset-gpio would be more in keeping with the gpio bindings. I'd get rid of pin number from the description. GPIOs are referred to with phandle + gpio-specifier pairs, and this makes it sound like a single integer value. +- ai31xx-micbias-vg - MicBias Voltage setting This name is a bit terse. Perhaps s/vg/voltage/ ? +0 or MICBIAS_OFF - MICBIAS output it not powered s/it/is/ +1 or MICBIAS_2_0V - MICBIAS output is powered to 2.0V +2 or MICBIAS_2_5V - MICBIAS output is powered to 2.5V +3 or MICBIAS_AVDD - MICBIAS output is connected to AVDD + If this node is not mentioned or if the value is unknown, then + micbias is set to 2.0V. Why does the dts write need to select these -- what prevents this getting selected dynamically? Cheers, Mark. -- 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 v4] ASoC: tlv320aic31xx: Add basic codec driver implementation
On Mon, Mar 10, 2014 at 01:15:56PM +, Jyri Sarha wrote: On 03/10/2014 02:09 PM, Mark Rutland wrote: On Mon, Mar 10, 2014 at 08:52:21AM +, Jyri Sarha wrote: This commit adds a bare bones driver support for TLV320AIC31XX family audio codecs. The driver adds basic stereo playback trough headphone and speaker outputs and mono capture trough microphone inputs. The driver is currently missing support at least for mini DSP features and jack detection. I have tested the driver only on TLV320AIC3111, but based on the data sheets TLV320AIC3100, TLV320AIC3110, and TLV320AIC3120 should work Ok too. The base for the implementation was taken from: g...@gitorious.org:ti-codecs/ti-codecs.git ajitk/topics/k3.10.1-aic31xx -branch at commit 77504eba0294764e9e63b4a0c696b44db187cd13. Signed-off-by: Jyri Sarha jsa...@ti.com --- .../devicetree/bindings/sound/tlv320aic31xx.txt| 61 + include/dt-bindings/sound/tlv320aic31xx-micbias.h |9 + sound/soc/codecs/Kconfig |4 + sound/soc/codecs/Makefile |2 + sound/soc/codecs/tlv320aic31xx.c | 1343 sound/soc/codecs/tlv320aic31xx.h | 258 6 files changed, 1677 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/tlv320aic31xx.txt create mode 100644 include/dt-bindings/sound/tlv320aic31xx-micbias.h create mode 100644 sound/soc/codecs/tlv320aic31xx.c create mode 100644 sound/soc/codecs/tlv320aic31xx.h diff --git a/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt b/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt new file mode 100644 index 000..0109df1 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt @@ -0,0 +1,61 @@ +Texas Instruments - tlv320aic31xx Codec module + +The tlv320aic31xx serial control bus communicates through I2C protocols + +Required properties: + +- compatible - string - One of: +ti,tlv320aic310x - Generic TLV320AIC31xx with mono speaker amp +ti,tlv320aic311x - Generic TLV320AIC31xx with stereo speaker amp +ti,tlv320aic3100 - TLV320AIC3100 (mono speaker amp, no MiniDSP) +ti,tlv320aic3110 - TLV320AIC3110 (stereo speaker amp, no MiniDSP) +ti,tlv320aic3120 - TLV320AIC3120 (mono speaker amp, MiniDSP) +ti,tlv320aic3111 - TLV320AIC3111 (stereo speaker amp, MiniDSP) + +- reg - int - I2C slave address + + +Optional properties: + +- gpio-reset - gpio pin number used for codec reset I believe calling this reset-gpio would be more in keeping with the gpio bindings. I'd get rid of pin number from the description. GPIOs are referred to with phandle + gpio-specifier pairs, and this makes it sound like a single integer value. +- ai31xx-micbias-vg - MicBias Voltage setting This name is a bit terse. Perhaps s/vg/voltage/ ? This parameter name is symmetric to already existing ai3x-micbias-vg in tlv320aic3x driver. Well... that is not a very good reason for not to change it. I'll change that to the next version of the patch. If we already have ai3x-micbias-vg then there's no reason to do differently here -- that'll just make things more confusing. +0 or MICBIAS_OFF - MICBIAS output it not powered s/it/is/ To be fixed. Cheers. Mark. -- 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: [PATCHv1 1/6] HSI: add Device Tree support for HSI clients
On Sun, Feb 23, 2014 at 11:49:56PM +, Sebastian Reichel wrote: Add new method hsi_add_clients_from_dt, which can be used to initialize HSI clients from a device tree node. The patch also documents the DT binding for trivial HSI clients. Signed-off-by: Sebastian Reichel s...@debian.org --- .../devicetree/bindings/hsi/trivial-devices.txt| 36 +++ drivers/hsi/hsi.c | 70 +- include/dt-bindings/hsi/hsi.h | 17 ++ include/linux/hsi/hsi.h| 2 + 4 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/hsi/trivial-devices.txt create mode 100644 include/dt-bindings/hsi/hsi.h It would be nice if we had a general HSI binding document that explains what HSI is and how HSI bus devices and clients are expected too look. Does HSI have an addressing scheme, or does each port have a single device? diff --git a/Documentation/devicetree/bindings/hsi/trivial-devices.txt b/Documentation/devicetree/bindings/hsi/trivial-devices.txt new file mode 100644 index 000..1ace14a --- /dev/null +++ b/Documentation/devicetree/bindings/hsi/trivial-devices.txt @@ -0,0 +1,36 @@ +This is a list of trivial hsi client devices that have simple +device tree bindings, consisting only of a compatible field +and the optional hsi configuration. + +If a device needs more specific bindings, such as properties to +describe some aspect of it, there needs to be a specific binding +document for it just like any other devices. Might this make more sense as a hsi-clients binding doc? I assume these properties could be applied to non-trivial devices? + +Optional HSI configuration properties: + +- hsi,mode Bit transmission mode (STREAM or FRAME) + The first value is used for RX and the second one for + TX configuration. If only one value is provided it will + be used for RX and TX. + The assignments may be found in header file + dt-bindings/hsi/hsi.h. Why not have hsi,rx-mode and hsi,tx-mode? I'll certainly get confused as to which cell is tx and which is rx with the current binding. Having separate tx-* and rx-* versions of the remaining properties would be good too. I note that the defines are called HSI_MODE_STREAM and HSI_MODE_FRAME, not STREAM and FRAME as the binding document implies. Please refer to exact names. It may make more sense to use a string here and for the other properties. They're easier for humans to read, and they survive decompilation (so you get _much_ better error messages). +- hsi,channels Number of channels to use [1..16] + The first value is used for RX and the second one for + TX configuration. If only one value is provided it will + be used for RX and TX. +- hsi,speed Max bit transmission speed (Kbit/s) + The first value is used for RX and the second one for + TX configuration. If only one value is provided it will + be used for RX and TX. +- hsi,flow RX flow type (SYNCHRONIZED or PIPELINE) + The assignments may be found in header file + dt-bindings/hsi/hsi.h. +- hsi,arb_mode Arbitration mode for TX frame (Round robin, priority) + The assignments may be found in header file + dt-bindings/hsi/hsi.h. s/_/-/ in property names please. + +This is the list of trivial client devices: + +Compatible Description +== = +hsi-char HSI character device What exactly is a HSI character device? This seems more like a Linux abstraction than a real class of device. Cheers, Mark. -- 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: [PATCHv1 3/6] HSI: hsi-char: add Device Tree support
On Sun, Feb 23, 2014 at 11:49:58PM +, Sebastian Reichel wrote: Add of_match_table to hsi_char driver, so that it can be referenced from Device Tree. Signed-off-by: Sebastian Reichel s...@debian.org --- drivers/hsi/clients/hsi_char.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/hsi/clients/hsi_char.c b/drivers/hsi/clients/hsi_char.c index e61e5f9..7f64bed 100644 --- a/drivers/hsi/clients/hsi_char.c +++ b/drivers/hsi/clients/hsi_char.c @@ -42,6 +42,7 @@ #include linux/stat.h #include linux/hsi/hsi.h #include linux/hsi/hsi_char.h +#include linux/of_device.h #define HSC_DEVS 16 /* Num of channels */ #define HSC_MSGS 4 @@ -758,12 +759,22 @@ static int hsc_remove(struct device *dev) return 0; } +#ifdef CONFIG_OF +static const struct of_device_id hsi_char_of_match[] = { + { .compatible = ssi-char, }, This string is undocumented. + { .compatible = hsi-char, }, I'm not sure either string makes sense though; this feels like a binding for the sake of the driver rather than describing the device and allowing the driver to pick it up if it makes sense to do so. What exactly is a ssi-char device or a hsi-char device? Cheers, Mark. -- 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: [PATCHv1 5/6] HSI: Introduce OMAP SSI driver
On Sun, Feb 23, 2014 at 11:50:00PM +, Sebastian Reichel wrote: Add OMAP SSI driver to the HSI subsystem. The Synchronous Serial Interface (SSI) is a legacy version of HSI. As in the case of HSI, it is mainly used to connect Application engines (APE) with cellular modem engines (CMT) in cellular handsets. It provides a multichannel, full-duplex, multi-core communication with no reference clock. The OMAP SSI block is capable of reaching speeds of 110 Mbit/s. Signed-off-by: Sebastian Reichel s...@debian.org --- drivers/hsi/Kconfig |1 + drivers/hsi/Makefile|1 + drivers/hsi/controllers/Kconfig | 19 + drivers/hsi/controllers/Makefile|6 + drivers/hsi/controllers/omap_ssi.c | 618 ++ drivers/hsi/controllers/omap_ssi.h | 166 drivers/hsi/controllers/omap_ssi_port.c | 1401 +++ drivers/hsi/controllers/omap_ssi_regs.h | 171 8 files changed, 2383 insertions(+) create mode 100644 drivers/hsi/controllers/Kconfig create mode 100644 drivers/hsi/controllers/Makefile create mode 100644 drivers/hsi/controllers/omap_ssi.c create mode 100644 drivers/hsi/controllers/omap_ssi.h create mode 100644 drivers/hsi/controllers/omap_ssi_port.c create mode 100644 drivers/hsi/controllers/omap_ssi_regs.h [...] + irq = platform_get_resource_byname(pd, IORESOURCE_IRQ, gdd_mpu); + if (!irq) { + dev_err(pd-dev, GDD IRQ resource missing\n); + err = -ENXIO; + goto out_err; + } + omap_ssi-gdd_irq = irq-start; You can use platform_get_irq_byname here. [...] +static inline int ssi_of_get_available_child_count(const struct device_node *np) +{ + struct device_node *child; + int num = 0; + + for_each_child_of_node(np, child) + if (of_device_is_available(child)) + num++; + + return num; +} You can find of_get_available_child_count in linux/of.h. That said, this seems to be trying to count the numbero f ports, which should all be compatible with ti,omap3-ssi-port, no? So maybe you should count all available child nodes compatible with that. + +static int __init ssi_probe(struct platform_device *pd) +{ + struct device_node *np = pd-dev.of_node; + struct hsi_controller *ssi; + int err; + int num_ports; + + if (!np) { + dev_err(pd-dev, missing device tree data\n); + return -EINVAL; + } + + num_ports = ssi_of_get_available_child_count(np); + + ssi = hsi_alloc_controller(num_ports, GFP_KERNEL); + if (!ssi) { + dev_err(pd-dev, No memory for controller\n); + return -ENOMEM; + } + + platform_set_drvdata(pd, ssi); + + err = ssi_add_controller(ssi, pd); + if (err 0) + goto out1; + + pm_runtime_irq_safe(pd-dev); + pm_runtime_enable(pd-dev); + + err = ssi_hw_init(ssi); + if (err 0) + goto out2; +#ifdef CONFIG_DEBUG_FS + err = ssi_debug_add_ctrl(ssi); + if (err 0) + goto out2; +#endif + + err = of_platform_populate(pd-dev.of_node, NULL, NULL, pd-dev); I'm not keen on doing this because it allows arbitrary devices which are not ssi ports to be placed in the ssi host controller node that will be probed, which is nonsensical and something I'd like to avoid by construction. Is there any reason the ports have to be platform devices at all? If so, is there no way we can register them directly and skip any other devices? [...] +static int __exit ssi_remove(struct platform_device *pd) +{ + struct hsi_controller *ssi = platform_get_drvdata(pd); + +#ifdef CONFIG_DEBUG_FS + ssi_debug_remove_ctrl(ssi); +#endif + ssi_remove_controller(ssi); + platform_set_drvdata(pd, NULL); + + pm_runtime_disable(pd-dev); + + /* cleanup of of_platform_populate() call */ + device_for_each_child(pd-dev, NULL, ssi_remove_ports); This would certainly be broken for a non ssi port device. [...] +static int omap_ssi_port_runtime_suspend(struct device *dev) +{ + struct hsi_port *port = dev_get_drvdata(dev); + struct omap_ssi_port *omap_port = hsi_port_drvdata(port); + struct hsi_controller *ssi = to_hsi_controller(port-device.parent); + struct omap_ssi_controller *omap_ssi = hsi_controller_drvdata(ssi); + + dev_dbg(dev, port runtime suspend!\n); + + ssi_set_port_mode(omap_port, SSI_MODE_SLEEP); + if (omap_ssi-get_loss) + omap_port-loss_count = + (*omap_ssi-get_loss)(ssi-device.parent); You don't need to do (*struct-func)(args) when invoking a function pointer. You can jsut have struct-func(args) as we do elsewhere. This can be:
Re: [PATCH] ARM: EDMA: Use platform_get_resource functions for DT
On Fri, Feb 21, 2014 at 04:24:21AM +, Joel Fernandes wrote: Currently, EDMA driver uses of_address_to_resource for getting Channel controller and x-bar register resources. Use platform_get_resource_by_name instead regardless of whether its DT-boot or not, document the new reg-names properties. Doesn't this break existing DTBs? Also, while at it get rid of the assumption in the code that CC is at reg index 0 in the DT and xbar is at offset 1. Instead use reg-names to get the memory resource in concern keeping things much cleaner and simpler. This also makes it possible to have multiple channel controllers. While this is nice I think we have to have a fallback to the existing behaviour if there's no reg-names property present, unless we know for certain no-one is possibly using an existing DTB. Cheers, Mark. Signed-off-by: Joel Fernandes jo...@ti.com --- Documentation/devicetree/bindings/dma/ti-edma.txt |7 ++ arch/arm/boot/dts/am33xx.dtsi |1 + arch/arm/boot/dts/am4372.dtsi |1 + arch/arm/common/edma.c| 28 ++--- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/Documentation/devicetree/bindings/dma/ti-edma.txt b/Documentation/devicetree/bindings/dma/ti-edma.txt index 9fbbdb7..176e42b 100644 --- a/Documentation/devicetree/bindings/dma/ti-edma.txt +++ b/Documentation/devicetree/bindings/dma/ti-edma.txt @@ -8,6 +8,12 @@ Required properties: Clients should use a single channel number per DMA request. - dma-channels: Specify total DMA channels per CC - reg: Memory map for accessing module +- reg-names: Since there can be different memory regions, each reg + entry should correspond to one of the following reg-names (X being 0 to N): + edma_ccX: memory map for Xth Channel Controller + edma_tcX: memory map for Xth Transfer Controller + Additionally there can be a memory map for xbar (in control module): + edma_xbar: memory map for xbar access. - interrupt-parent: Interrupt controller the interrupt is routed through - interrupts: Exactly 3 interrupts need to be specified in the order: 1. Transfer completion interrupt. @@ -21,6 +27,7 @@ Example: edma: edma@4900 { reg = 0x4900 0x1; + reg-names = edma_cc0; interrupt-parent = intc; interrupts = 12 13 14; compatible = ti,edma3; diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index 2b66e67..55f5723 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -123,6 +123,7 @@ ti,hwmods = tpcc, tptc0, tptc1, tptc2; reg = 0x4900 0x1, 0x44e10f90 0x10; + reg-names = edma_cc0, edma_xbar; interrupts = 12 13 14; #dma-cells = 1; dma-channels = 64; diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index babdc84..1323a97 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -100,6 +100,7 @@ ti,hwmods = tpcc, tptc0, tptc1, tptc2; reg = 0x4900 0x1, 0x44e10f90 0x10; + reg-names = edma_cc0, edma_xbar; interrupts = GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH, GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH, GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH; diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c index dc95efc..ae0ccae 100644 --- a/arch/arm/common/edma.c +++ b/arch/arm/common/edma.c @@ -1458,10 +1458,11 @@ static int edma_xbar_event_map(struct device *dev, struct edma_soc_info *pdata, int len) { int ret, i; - struct resource res; + struct resource *res; void __iomem *xbar; const s16 (*xbar_chans)[2]; u32 shift, offset, mux; + struct platform_device *pdev; xbar_chans = devm_kzalloc(dev, len/sizeof(s16) + 2*sizeof(s16), @@ -1469,11 +1470,14 @@ static int edma_xbar_event_map(struct device *dev, if (!xbar_chans) return -ENOMEM; - ret = of_address_to_resource(node, 1, res); - if (ret) + pdev = to_platform_device(dev); + res = platform_get_resource_byname(pdev, +IORESOURCE_MEM, +edma_xbar); + if (!res) return -EIO; - xbar = devm_ioremap(dev, res.start, resource_size(res)); + xbar = devm_ioremap(dev, res-start, resource_size(res)); if (!xbar) return -ENOMEM; @@ -1614,7 +1618,6 @@ static int edma_probe(struct platform_device *pdev) int
Re: [PATCH v7 5/7] ARM: dts: add pbias dt node
On Wed, Jan 08, 2014 at 02:51:46PM +, Balaji T K wrote: On Tuesday 07 January 2014 05:53 PM, Balaji T K wrote: On Tuesday 07 January 2014 04:27 PM, Mark Rutland wrote: On Tue, Jan 07, 2014 at 10:18:15AM +, Balaji T K wrote: On Monday 06 January 2014 11:49 PM, Mark Rutland wrote: On Fri, Dec 20, 2013 at 05:35:53PM +, Balaji T K wrote: Add pbias regulator node as a child of system control module - syscon. Signed-off-by: Balaji T K balaj...@ti.com --- arch/arm/boot/dts/dra7.dtsi | 18 ++ arch/arm/boot/dts/omap2430.dtsi | 18 ++ arch/arm/boot/dts/omap3.dtsi| 18 ++ arch/arm/boot/dts/omap4.dtsi| 18 ++ arch/arm/boot/dts/omap5.dtsi| 18 ++ 5 files changed, 90 insertions(+), 0 deletions(-) diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi index d0df4c4..4e68df1 100644 --- a/arch/arm/boot/dts/dra7.dtsi +++ b/arch/arm/boot/dts/dra7.dtsi @@ -110,6 +110,23 @@ ti,hwmods = counter_32k; }; +dra7_ctrl_general: tisyscon@4a002e00 { +compatible = ti,control-syscon, syscon, simple-bus; Please, don't use simple-bus like that. The components below this node depend on it. It is _NOT_ a simple bus. Make the ti,control-syscon driver probe it's children. Hi Mark, Actually ti,control-syscon driver does not exist, so I can remove it, and simple-bus is needed for child creation. This still shows up as a syscon node, with a reg property, and syscon is not an extension of simple-bus. There are properties in the parent node that children depend on, and that makes me wary of describing it as a simple bus. I'd expect to be able to move child nodes out of a simple-bus if ranges provided an idmap, and I can't do that here. Hi Mark, Not sure if I am understanding here, can you please add more info. A node's compatible string list describes the set of devices (or rather descriptions of devices) with which it is compatible. The list goes from most specific to least specific. The idea is that an OS reads through the list until it finds a string it knows how to handle, then treats the node as that. Ignoring ti,control-syscon, the node has both syscon and simple-bus. Given you expect the OS to recognise syscon, you should _not_ expect it to also treat the node as simple-bus. Doing so is an abuse of the property and current Linux implementation details. The syscon binding is not a more specific version of simple-bus. They imply completely different things. Either make a ti,control-syscon driver that probes the child nodes, or move the child nodes out and give them a phandle to the dra7_ctrl_general node that they can parse. I'd prefer the latter as relies on fewer topology details, is as easy to implement as your current parent node parsing, and is easier to extend in future. Hi Mark, From Documentation/devicetree/bindings.. , I could get below info about simple-bus - simple-bus compatible value (to ensure creation of the children) compatible = simple-bus; That's from the vexpress binding description of the smb node? In that case, the smb node _is_ a simple-bus. It has no nonstandard properties, and the child nodes aren't reading arbitrary properties out of the smb node. There the simple-bus is used to remap addresses of child nodes via ranges. It does represent a simple bus, and that's all the OS is expected to know. Thanks, Mark. -- 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 v4 1/5] mfd: omap-usb-host: Update DT clock binding information
On Wed, Jan 08, 2014 at 06:15:38AM +, Roger Quadros wrote: The omap-usb-host driver expects the 60MHz functional clock of the USB host module to be named as init_60m_fclk. Add this information in the DT binding document. CC: Lee Jones lee.jo...@linaro.org CC: Samuel Ortiz sa...@linux.intel.com Signed-off-by: Roger Quadros rog...@ti.com --- Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt index b381fa6..5635202 100644 --- a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt @@ -32,6 +32,10 @@ Optional properties: - single-ulpi-bypass: Must be present if the controller contains a single ULPI bypass control bit. e.g. OMAP3 silicon = ES2.1 +- clocks: phandle to 60MHz functional clock to the USB Host module. + +- clock-names: must be init_60m_fclk + Nit: clocks aren't just phandles, there's a clock-specifier too. Also, it would be nicer if clocks was defined in terms of clock-names, as it makes the relationship between clocks and clock-names clear, and makes it easier to extend the list in future. Something like: - clocks: a list of phandles + clock-specifiers, one for each entry in clock-names - clock-names: should include: * init_60m_fclk - the 60MHz functional clock to the USB host module. Thanks, Mark. -- 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 5/7] ARM: dts: add pbias dt node
On Tue, Jan 07, 2014 at 10:18:15AM +, Balaji T K wrote: On Monday 06 January 2014 11:49 PM, Mark Rutland wrote: On Fri, Dec 20, 2013 at 05:35:53PM +, Balaji T K wrote: Add pbias regulator node as a child of system control module - syscon. Signed-off-by: Balaji T K balaj...@ti.com --- arch/arm/boot/dts/dra7.dtsi | 18 ++ arch/arm/boot/dts/omap2430.dtsi | 18 ++ arch/arm/boot/dts/omap3.dtsi| 18 ++ arch/arm/boot/dts/omap4.dtsi| 18 ++ arch/arm/boot/dts/omap5.dtsi| 18 ++ 5 files changed, 90 insertions(+), 0 deletions(-) diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi index d0df4c4..4e68df1 100644 --- a/arch/arm/boot/dts/dra7.dtsi +++ b/arch/arm/boot/dts/dra7.dtsi @@ -110,6 +110,23 @@ ti,hwmods = counter_32k; }; + dra7_ctrl_general: tisyscon@4a002e00 { + compatible = ti,control-syscon, syscon, simple-bus; Please, don't use simple-bus like that. The components below this node depend on it. It is _NOT_ a simple bus. Make the ti,control-syscon driver probe it's children. Hi Mark, Actually ti,control-syscon driver does not exist, so I can remove it, and simple-bus is needed for child creation. This still shows up as a syscon node, with a reg property, and syscon is not an extension of simple-bus. There are properties in the parent node that children depend on, and that makes me wary of describing it as a simple bus. I'd expect to be able to move child nodes out of a simple-bus if ranges provided an idmap, and I can't do that here. Thanks, Mark. + reg = 0x4a002e00 0x7c; + #address-cells = 1; + #size-cells = 1; + ranges; + pbias_regulator: pbias_regulator { + compatible = ti,pbias-omap; + reg = 0 0x4; + pbias_mmc_reg: pbias_mmc_omap5 { + regulator-name = pbias_mmc_omap5; + regulator-min-microvolt = 180; + regulator-max-microvolt = 300; + }; + }; + }; + Thanks, Mark. -- 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 v1 2/5] misc: tda8026: Add NXP TDA8026 PHY driver
On Mon, Jan 06, 2014 at 12:07:39PM +, Satish Patel wrote: TDA8026 is a SmartCard PHY from NXP. The PHY interfaces with the main processor over the I2C interface and acts as a slave device. The driver also exposes the phy interface (defined@include/linux/sc_phy.h) for SmartCard controller. Controller uses this interface to communicate with smart card inserted to the phy's slot. Note: gpio irq is not validated as I do not have device with that. I have validated interrupt with dedicated interrupt line on my device. Signed-off-by: Maulik Mankad mau...@ti.com Signed-off-by: Satish Patel satish.pa...@ti.com --- Documentation/devicetree/bindings/misc/tda8026.txt | 14 + drivers/misc/Kconfig |7 + drivers/misc/Makefile |1 + drivers/misc/tda8026.c | 1271 4 files changed, 1293 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/misc/tda8026.txt create mode 100644 drivers/misc/tda8026.c diff --git a/Documentation/devicetree/bindings/misc/tda8026.txt b/Documentation/devicetree/bindings/misc/tda8026.txt new file mode 100644 index 000..d3083bf --- /dev/null +++ b/Documentation/devicetree/bindings/misc/tda8026.txt @@ -0,0 +1,14 @@ +TDA8026 smart card slot interface + +Required properties: +- compatible: nxp,tda8026 Please quote strings: - compatible: should contain nxp,tda8026 +- shutdown-gpio = GPIO pin mapping for SDWNN pin +- reg = i2c interface address It's probably worth mentioning at the start that this is an i2c device. [...] +static int tda8026_parse_dt(struct device *dev, struct tda8026 *pdata) +{ + struct device_node *np = dev-of_node; + const struct of_device_id *match; + int ret = 0; + + match = of_match_device(of_match_ptr(tda8026_id_table), dev); + if (!match) + return -EINVAL; Why do this? The of_device_id table contains one entry with no additional data. If you just want to see if this was probed via DT, dev-of_node not being NULL would tell you that. Is this going to be used without DT anywhere? [...] + if (pdata-irq == 0) { + /* look for the field irq-gpio in DT */ + irq_gpio = of_get_named_gpio(np, irq-gpio, 0); + if (!gpio_is_valid(irq_gpio)) { + dev_err(dev, Failed to get irq gpio,\n); + return -EIO; + } This is horrible. If the gpio controller can act as an irq controller then it should be annotated as such, with #interrupt-cells, and you should just describe the interrupt. The smart card controller cares about having an interrupt line, not a GPIO. Additionally, this was not described in the binding. Thanks, Mark. -- 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 5/7] ARM: dts: add pbias dt node
On Fri, Dec 20, 2013 at 05:35:53PM +, Balaji T K wrote: Add pbias regulator node as a child of system control module - syscon. Signed-off-by: Balaji T K balaj...@ti.com --- arch/arm/boot/dts/dra7.dtsi | 18 ++ arch/arm/boot/dts/omap2430.dtsi | 18 ++ arch/arm/boot/dts/omap3.dtsi| 18 ++ arch/arm/boot/dts/omap4.dtsi| 18 ++ arch/arm/boot/dts/omap5.dtsi| 18 ++ 5 files changed, 90 insertions(+), 0 deletions(-) diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi index d0df4c4..4e68df1 100644 --- a/arch/arm/boot/dts/dra7.dtsi +++ b/arch/arm/boot/dts/dra7.dtsi @@ -110,6 +110,23 @@ ti,hwmods = counter_32k; }; + dra7_ctrl_general: tisyscon@4a002e00 { + compatible = ti,control-syscon, syscon, simple-bus; Please, don't use simple-bus like that. The components below this node depend on it. It is _NOT_ a simple bus. Make the ti,control-syscon driver probe it's children. + reg = 0x4a002e00 0x7c; + #address-cells = 1; + #size-cells = 1; + ranges; + pbias_regulator: pbias_regulator { + compatible = ti,pbias-omap; + reg = 0 0x4; + pbias_mmc_reg: pbias_mmc_omap5 { + regulator-name = pbias_mmc_omap5; + regulator-min-microvolt = 180; + regulator-max-microvolt = 300; + }; + }; + }; + Thanks, Mark. -- 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 3/7] regulator: add pbias regulator support
On Fri, Dec 20, 2013 at 05:35:51PM +, Balaji T K wrote: pbias register controls internal power supply to sd card i/o pads in most OMAPs (OMAP2-5, DRA7). Control bits for selecting voltage level and enabling/disabling are in the same PBIAS register. Signed-off-by: Balaji T K balaj...@ti.com --- .../bindings/regulator/pbias-regulator.txt | 16 ++ drivers/regulator/Kconfig |9 + drivers/regulator/Makefile |1 + drivers/regulator/pbias-regulator.c| 261 4 files changed, 287 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/regulator/pbias-regulator.txt create mode 100644 drivers/regulator/pbias-regulator.c diff --git a/Documentation/devicetree/bindings/regulator/pbias-regulator.txt b/Documentation/devicetree/bindings/regulator/pbias-regulator.txt new file mode 100644 index 000..359d3f9 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/pbias-regulator.txt @@ -0,0 +1,16 @@ +PBIAS internal regulator for SD card dual voltage i/o pads on OMAP SoCs. + +Required properties: +- compatible: + - ti,pbias-omap for OMAP2, OMAP3, OMAP4, OMAP5, DRA7 + +Optional properties: +- Any optional property defined in bindings/regulator/regulator.txt + +Example: + + pbias_regulator: pbias_regulator { + regulator-name = pbias_mmc_omap4; + regulator-min-microvolt = 180; + regulator-max-microvolt = 300; + }; This doesn't look like a full example. I don't see the required compatible string. [...] +static struct of_regulator_match pbias_matches[] = { + { .name = pbias_mmc_omap2430, .driver_data = (void *)pbias_mmc_omap2430}, + { .name = pbias_sim_omap3, .driver_data = (void *)pbias_sim_omap3}, + { .name = pbias_mmc_omap4, .driver_data = (void *)pbias_mmc_omap4}, + { .name = pbias_mmc_omap5, .driver_data = (void *)pbias_mmc_omap5}, +}; These weren't documented. [...] +static int pbias_regulator_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev-dev.of_node; + struct device_node *dev_node; + struct pbias_regulator_data *drvdata; + struct resource *res; + struct regulator_config cfg = { }; + struct regmap *syscon; + const struct pbias_reg_info *info; + int ret = 0; + int count, idx, data_idx = 0; + + count = of_regulator_match(pdev-dev, np, pbias_matches, + PBIAS_NUM_REGS); + if (count 0) + return count; + + drvdata = devm_kzalloc(pdev-dev, sizeof(struct pbias_regulator_data) +* count, GFP_KERNEL); + if (drvdata == NULL) { + dev_err(pdev-dev, Failed to allocate device data\n); + return -ENOMEM; + } + + dev_node = of_get_parent(np); + if (!dev_node) + return -ENODEV; + + syscon = syscon_node_to_regmap(dev_node); If we're relying on a particular parent node, that _must_ be described in the binding. + of_node_put(dev_node); + if (IS_ERR(syscon)) + return PTR_ERR(syscon); + + cfg.dev = pdev-dev; + + for (idx = 0; idx PBIAS_NUM_REGS, data_idx count; idx++) { + if (!pbias_matches[idx].init_data || + !pbias_matches[idx].of_node) + continue; + + info = pbias_matches[idx].driver_data; + if (!info) + return -ENODEV; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); A reg entry was not mentioned in the binding. Why is this inside the loop? It's grabbing the reg from the parent, not each of the children it's walking over. Thanks, Mark. -- 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] input: touchscreen: fix spelling mistake in TSC/ADC DT binding
On Fri, Nov 15, 2013 at 05:53:56PM +, Felipe Balbi wrote: Hi, On Fri, Nov 15, 2013 at 03:55:40PM +, Mark Rutland wrote: diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c index e1c5300..b61df9d 100644 --- a/drivers/input/touchscreen/ti_am335x_tsc.c +++ b/drivers/input/touchscreen/ti_am335x_tsc.c @@ -348,9 +348,16 @@ static int titsc_parse_dt(struct platform_device *pdev, if (err 0) return err; - err = of_property_read_u32(node, ti,coordiante-readouts, + /* + * try with new binding first. If it fails, still try with + * bogus, miss-spelled version. + */ + err = of_property_read_u32(node, ti,coordinate-readouts, ts_dev-coordinate_readouts); if (err 0) + err = of_property_read_u32(node, ti,coordiante-readouts, + ts_dev-coordinate_readouts); + if (err 0) return err; Thanks, very good. Do we keep this fallback for ever or just for a year or two? That's for DT maintainers to decide but considering DT is an ABI, I guess we need to keep for 30 years or so :-p We keep it as long as we have to. If no-one's relying on the typo by the next merge window, I see no reason we'd have to keep support for the and how could you know that ? considering it's an ABI, how could you ever know that ? If you know that the only user of a binding is a dts for a particular product that you're in charge of, then you'd know the set of kernel + dtb combinations out there, and can judge. once the binding has made into mainline, it's next to impossible to figure out who has downloaded a tarball containing that driver and made a product out of it. Besides keeping that check in the driver won't hurt at all in the long run. I would give it at least until 4.0 before thinking about removing, and that might still not be enough time. That sounds sensible to me. As mentioned before I'd recommend adding a warning for the typo now in the (possibly naïve) hope that it will encourage people to fix up their dts early. Thanks, Mark. -- 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] input: touchscreen: ti_am335x_tsc: warn about incorrect spelling
On Mon, Nov 18, 2013 at 03:29:01PM +, Felipe Balbi wrote: In the hopes that people run new kernels on their devices, let's add a warning message asking users to have their DTS file fixed. The goal is that by Linux 4.0 we will be able to remove support for the bogus version of our touchscreen's DTS. Suggested-by: Mark Rutland mark.rutl...@arm.com Signed-off-by: Felipe Balbi ba...@ti.com --- Here you go, I've added your Suggested-by Mark, if you wish I can remove or change to something else. cheers Looks fine to me, feel free to add my Ack. Thanks, Mark. drivers/input/touchscreen/ti_am335x_tsc.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c index b61df9d..91302cd 100644 --- a/drivers/input/touchscreen/ti_am335x_tsc.c +++ b/drivers/input/touchscreen/ti_am335x_tsc.c @@ -354,9 +354,12 @@ static int titsc_parse_dt(struct platform_device *pdev, */ err = of_property_read_u32(node, ti,coordinate-readouts, ts_dev-coordinate_readouts); - if (err 0) + if (err 0) { + dev_warn(pdev-dev, please use 'ti,coordinate-readouts' instead\n); err = of_property_read_u32(node, ti,coordiante-readouts, ts_dev-coordinate_readouts); + } + if (err 0) return err; -- 1.8.4.GIT -- 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 V4 2/4] DRIVERS: IRQCHIP: CROSSBAR: Add support for Crossbar IP
On Thu, Nov 14, 2013 at 04:41:41PM +, Sricharan R wrote: Hi Mark, On Thursday 14 November 2013 07:42 PM, Mark Rutland wrote: On Thu, Nov 14, 2013 at 12:18:48PM +, Sricharan R wrote: Some socs have a large number of interrupts requests to service the needs of its many peripherals and subsystems. All of the interrupt lines from the subsystems are not needed at the same time, so they have to be muxed to the irq-controller appropriately. In such places a interrupt controllers are preceded by an CROSSBAR that provides flexibility in muxing the device requests to the controller inputs. This driver takes care a allocating a free irq and then configuring the crossbar IP as a part of the mpu's irqchip callbacks. crossbar_init should be called right before the irqchip_init, so that it is setup to handle the irqchip callbacks. Cc: Thomas Gleixner t...@linutronix.de Cc: Linus Walleij linus.wall...@linaro.org Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Russell King li...@arm.linux.org.uk Cc: Tony Lindgren t...@atomide.com Cc: Rajendra Nayak rna...@ti.com Cc: Marc Zyngier marc.zyng...@arm.com Cc: Grant Likely grant.lik...@linaro.org Cc: Rob Herring rob.herr...@calxeda.com Signed-off-by: Sricharan R r.sricha...@ti.com Acked-by: Kumar Gala ga...@codeaurora.org (for DT binding portion) Acked-by: Santosh Shilimkar santosh.shilim...@ti.com --- [V2] Addressed Thomas Gleixner t...@linutronix.de comments and renamed the bindings as per Kumar Gala ga...@codeaurora.org comments. [V3] Changed static inline const to static inline int and removed unnecessary variable initialization as per Thomas Gleixner t...@linutronix.de. Updated commit tags [V4] Renamed crossbar_init as irqcrossbar_init as per Rajendra Nayak rna...@ti.com suggestion. .../devicetree/bindings/arm/omap/crossbar.txt | 27 +++ drivers/irqchip/Kconfig|8 + drivers/irqchip/Makefile |1 + drivers/irqchip/irq-crossbar.c | 206 include/linux/irqchip/irq-crossbar.h | 11 ++ 5 files changed, 253 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/omap/crossbar.txt create mode 100644 drivers/irqchip/irq-crossbar.c create mode 100644 include/linux/irqchip/irq-crossbar.h diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt new file mode 100644 index 000..fb88585 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt @@ -0,0 +1,27 @@ +Some socs have a large number of interrupts requests to service +the needs of its many peripherals and subsystems. All of the +interrupt lines from the subsystems are not needed at the same +time, so they have to be muxed to the irq-controller appropriately. +In such places a interrupt controllers are preceded by an CROSSBAR +that provides flexibility in muxing the device requests to the controller +inputs. + +Required properties: +- compatible : Should be ti,irq-crossbar +- reg: Base address and the size of the crossbar registers. +- ti,max-irqs: Total number of irqs available at the interrupt controller. +- ti,reg-size: Size of a individual register in bytes. Every individual + register is assumed to be of same size. Valid sizes are 1, 2, 4. +- ti,irqs-reserved: List of the reserved irq lines that are not muxed using + crossbar. These interrupt lines are reserved in the soc, + so crossbar bar driver should not consider them as free + lines. The combination of the ti,max-irqs and ti,irqs-reserved properties seems backwards to me. Why can we not describe the set of IRQs that _can_ be used? Total set of irqs that are usable is max - reserved. Since reserved irqs are not continuous, we have to give the list. During the init we count the total number of reserved and get the usable one. So why not describe the set of usable IRQs, rather than a set of IRQs for which only some are usable then subtracting the set of unusable IRQs? It seems backwards to me to have a binding for a device describe resources it doesn't have. + +Examples: + crossbar_mpu: @4a02 { + compatible = ti,irq-crossbar; + reg = 0x4a002a48 0x130; + ti,max-irqs = 160; + ti,reg-size = 2; + ti,irqs-reserved = 0 1 2 3 5 6 131 132 139 140; + }; [...] + /* Get and mark reserved irqs */ + irqsr = of_get_property(node, ti,irqs-reserved, size); + if (irqsr) { + size /= sizeof(__be32); + + for (i = 0; i size; i++) { + entry = be32_to_cpup(irqsr + i); + if (entry max) { + pr_err
Re: [PATCH V4 1/4] DRIVERS: IRQCHIP: IRQ-GIC: Add support for routable irqs
On Thu, Nov 14, 2013 at 04:46:36PM +, Sricharan R wrote: Hi Mark, On Thursday 14 November 2013 07:31 PM, Mark Rutland wrote: On Thu, Nov 14, 2013 at 12:18:47PM +, Sricharan R wrote: In some socs the gic can be preceded by a crossbar IP which routes the peripheral interrupts to the gic inputs. The peripheral interrupts are associated with a fixed crossbar input line and the crossbar routes that to one of the free gic input line. The DT entries for peripherals provides the fixed crossbar input line as its interrupt number and the mapping code should associate this with a free gic input line. This patch adds the support inside the gic irqchip to handle such routable irqs. The routable irqs are registered in a linear domain. The registered routable domain's callback should be implemented to get a free irq and to configure the IP to route it. Cc: Thomas Gleixner t...@linutronix.de Cc: Linus Walleij linus.wall...@linaro.org Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Russell King li...@arm.linux.org.uk Cc: Tony Lindgren t...@atomide.com Cc: Rajendra Nayak rna...@ti.com Cc: Marc Zyngier marc.zyng...@arm.com Cc: Grant Likely grant.lik...@linaro.org Cc: Rob Herring rob.herr...@calxeda.com Signed-off-by: Sricharan R r.sricha...@ti.com Acked-by: Santosh Shilimkar santosh.shilim...@ti.com --- [V2] Added default routable-irqs functions to avoid unnecessary if checks as per Thomas Gleixner comments and renamed routable-irq binding as per Kumar Gala ga...@codeaurora.org comments. [V3] Addressed unnecessary warn-on and updated default xlate function as per Thomas Gleixner comments Documentation/devicetree/bindings/arm/gic.txt |6 ++ drivers/irqchip/irq-gic.c | 81 ++--- include/linux/irqchip/arm-gic.h |7 ++- 3 files changed, 83 insertions(+), 11 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt index 3dfb0c0..5357745 100644 --- a/Documentation/devicetree/bindings/arm/gic.txt +++ b/Documentation/devicetree/bindings/arm/gic.txt @@ -49,6 +49,11 @@ Optional regions, used when the GIC doesn't have banked registers. The offset is cpu-offset * cpu-nr. +- arm,routable-irqs : Total number of gic irq inputs which are not directly +connected from the peripherals, but are routed dynamically +by a crossbar/multiplexer preceding the GIC. The GIC irq +input line is assigned dynamically when the corresponding +peripheral's crossbar line is mapped. I'm not keen on the design of the arm,routable-irqs property. The set of IRQs which the crossbar IP can use is a property of which IRQ lines it has routed to the GIC. I don't see why that should be considered a property of the GIC; it's a property of the crossbar IP's attachment to the GIC. Given we already have a mechanism for describing the attachment (i.e. the interrupts property) where the property appears on the node for the device generating/propagating the interrupt, I don't see why we should do differently here. We did try using interrupts= property for all peripherals and mapping them as crossbar's parent. But that approach of representing crossbar as a interrupt parent was not accepted, since the crossbar was just routing the interrupts from peripherals to GIC and nothing more. Also mapping all the interrupts using interrupt-map like property by a fixed way in DTS itself was considered hacky I'm not suggesting you should interrupt-map. I agree that that interrupt-map is not suitable for a dynamically configurable device like the crossbar. When you say that the crossbar is just routing the interrupts, at what level is it doing so? Does it accept a logical interrupt and output another logical interrupt, or does it just connect the two lines electrically? We don't necessarily have to use the interrupts property, but I still think that the set of GIC input IRQ lines that the crossbar is wired to should be described on the crossbar node. Listing 160 interrupts in the crossbar node is clearly something we don't want to have to do. If we had a property that we could use to define a range (or multiple ranges) of interrupts, then the crossbar driver could go and request those ranges from its interrupt-parent (the GIC) and the GIC driver could reserve/allocate the irqdomain at that time. Again, this kind of approach of crossbar requesting irqs from GIC was tried earlier and it did not go anywhere. Subsequently after lot of discussions this design was considered the best one. http://www.spinics.net/lists/linux-omap/msg97085.html As far as I can see, the comment there was to use irqdomains, which I am not arguing against. I am arguing that the linkage of the GIC and the crossbar is being
Re: [PATCH 2/2] dt: binding documentation for isp1704 charger
On Thu, Nov 14, 2013 at 12:38:54PM +, Sebastian Reichel wrote: Add devicetree binding documentation for isp1704-charger. Signed-off-by: Sebastian Reichel s...@debian.org --- Documentation/devicetree/bindings/power/isp1704.txt | 17 + 1 file changed, 17 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/isp1704.txt diff --git a/Documentation/devicetree/bindings/power/isp1704.txt b/Documentation/devicetree/bindings/power/isp1704.txt new file mode 100644 index 000..bbeec11 --- /dev/null +++ b/Documentation/devicetree/bindings/power/isp1704.txt @@ -0,0 +1,17 @@ +Binding for NXP ISP1704 USB Charger Detection + +Required properties: +- compatible: Should contain one of the following: + * nxp,isp1704 +- nxp,enable-gpio: Should contain a phandle to the Nit: phandle + gpio-specifier Otherwise this looks fine. Mark. -- 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] input: touchscreen: fix spelling mistake in TSC/ADC DT binding
On Thu, Nov 14, 2013 at 03:54:04PM +, Felipe Balbi wrote: HI, On Thu, Nov 14, 2013 at 11:19:59AM +, Mark Rutland wrote: On Tue, Oct 22, 2013 at 10:42:00AM +0200, Sebastian Andrzej Siewior wrote: On 10/21/2013 10:13 PM, Felipe Balbi wrote: diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c index e1c5300..b61df9d 100644 --- a/drivers/input/touchscreen/ti_am335x_tsc.c +++ b/drivers/input/touchscreen/ti_am335x_tsc.c @@ -348,9 +348,16 @@ static int titsc_parse_dt(struct platform_device *pdev, if (err 0) return err; - err = of_property_read_u32(node, ti,coordiante-readouts, + /* + * try with new binding first. If it fails, still try with + * bogus, miss-spelled version. + */ + err = of_property_read_u32(node, ti,coordinate-readouts, ts_dev-coordinate_readouts); if (err 0) + err = of_property_read_u32(node, ti,coordiante-readouts, + ts_dev-coordinate_readouts); + if (err 0) return err; Thanks, very good. Do we keep this fallback for ever or just for a year or two? That's for DT maintainers to decide but considering DT is an ABI, I guess we need to keep for 30 years or so :-p We keep it as long as we have to. If no-one's relying on the typo by the next merge window, I see no reason we'd have to keep support for the and how could you know that ? considering it's an ABI, how could you ever know that ? If you know that the only user of a binding is a dts for a particular product that you're in charge of, then you'd know the set of kernel + dtb combinations out there, and can judge. If a bug is found in a driver such that it hasn't worked for a number of releases, and no-one's complained, the binding is clearly not in use and thus support for it can be removed. If maintaining compatibility becomes too hard, and all users are happy to migrate to a newer dtb, then it's not necessary to maintain compatiblity for the old binding. While we can't always remove existing bindings, there are cases where it's possible and appropriate. However, we should strive for compatibility for as long a term as possible. Thanks, Mark. -- 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/8] net: smc91x: Fix device tree based configuration so it's usable
On Thu, Nov 14, 2013 at 02:35:30AM +, Tony Lindgren wrote: Commit 89ce376c6bdc (drivers/net: Use of_match_ptr() macro in smc91x.c) added minimal device tree support to smc91x, but it's not working on many platforms because of the lack of some key configuration bits. Fix the issue by parsing the necessary configuration like the smc911x driver is doing. Cc: Nicolas Pitre n...@fluxnic.net Cc: David S. Miller da...@davemloft.net Cc: net...@vger.kernel.org Cc: devicet...@vger.kernel.org Signed-off-by: Tony Lindgren t...@atomide.com --- If this looks OK, I'd like to merge this as a fix via arm-soc tree along with the other patches in this series as my later patches depend on patches in this series. --- .../devicetree/bindings/net/smsc-lan91c111.txt | 4 ++ drivers/net/ethernet/smsc/smc91x.c | 52 +- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/Documentation/devicetree/bindings/net/smsc-lan91c111.txt b/Documentation/devicetree/bindings/net/smsc-lan91c111.txt index 953049b..53d69e3 100644 --- a/Documentation/devicetree/bindings/net/smsc-lan91c111.txt +++ b/Documentation/devicetree/bindings/net/smsc-lan91c111.txt @@ -8,3 +8,7 @@ Required properties: Optional properties: - phy-device : phandle to Ethernet phy - local-mac-address : Ethernet mac address to use +- reg-io-width : Specify the size (in bytes) of the IO accesses that + should be performed on the device. Valid value for SMSC LAN is + 1, 2 or 4. If it's omitted or invalid, the size would be 2. In the driver the supported access sizes are not mutually exclusive. It would be nice for the binding to have the same property. +- smsc,nowait : Setup for fast register access with no waits I'm confused by what this means. When would this be selected, and when wouldn't it be? Thanks, Mark. -- 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 3/8] mmc: omap: Fix I2C dependency and make driver usable with device tree
On Thu, Nov 14, 2013 at 02:35:32AM +, Tony Lindgren wrote: Some features can be configured by the companion I2C chips, which may not be available at the probe time. Fix the issue by returning -EPROBE_DEFER when the MMC controller slots are not configured. While at it, let's also add minimal device tree support so omap24xx platforms can use this driver without legacy mode since we claim to support device tree for mach-omap2 based systems. Although adding the minimal device tree support is not strictly a fix, it does remove one of the last blockers for dropping a bunch of legacy platform data for mach-omap2. Cc: Chris Ball c...@laptop.org Cc: linux-...@vger.kernel.org Signed-off-by: Tony Lindgren t...@atomide.com --- If this looks OK, I'd like to merge this as a fix via arm-soc tree along with the other patches in this series as my later patches depend on patches in this series. --- drivers/mmc/host/omap.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c index ed56868..43c66ad 100644 --- a/drivers/mmc/host/omap.c +++ b/drivers/mmc/host/omap.c @@ -22,6 +22,7 @@ #include linux/delay.h #include linux/spinlock.h #include linux/timer.h +#include linux/of.h #include linux/omap-dma.h #include linux/mmc/host.h #include linux/mmc/card.h @@ -1330,7 +1331,7 @@ static int mmc_omap_probe(struct platform_device *pdev) } if (pdata-nr_slots == 0) { dev_err(pdev-dev, no slots\n); - return -ENXIO; + return -EPROBE_DEFER; } res = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -1553,6 +1554,12 @@ static int mmc_omap_resume(struct platform_device *pdev) #define mmc_omap_resume NULL #endif +#if IS_BUILTIN(CONFIG_OF) +static const struct of_device_id mmc_omap_match[] = { + { .compatible = ti,omap2420-mmc, }, Missing binding document. Thanks, Mark. -- 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 4/8] i2c: omap: Fix missing device tree flags for omap2
On Thu, Nov 14, 2013 at 02:35:33AM +, Tony Lindgren wrote: As we claim to support device tree for mach-omap2, we should have the necessary flags in the driver to make it usable. Cc: Wolfram Sang w...@the-dreams.de Cc: linux-...@vger.kernel.org Signed-off-by: Tony Lindgren t...@atomide.com --- If this looks OK, I'd like to merge this as a fix via arm-soc tree along with the other patches in this series as my later patches depend on patches in this series. --- drivers/i2c/busses/i2c-omap.c | 22 ++ 1 file changed, 22 insertions(+) [...] + { + .compatible = ti,omap2430-i2c, + .data = omap2430_pdata, + }, + { + .compatible = ti,omap2420-i2c, + .data = omap2420_pdata, Please update Documentation/devicetree/bindings/i2c/i2c-omap.txt Otherwise, this is fine. Thanks, Mark. -- 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] input: touchscreen: fix spelling mistake in TSC/ADC DT binding
On Tue, Oct 22, 2013 at 01:02:53PM +0100, Felipe Balbi wrote: Hi, On Tue, Oct 22, 2013 at 10:42:00AM +0200, Sebastian Andrzej Siewior wrote: On 10/21/2013 10:13 PM, Felipe Balbi wrote: diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c index e1c5300..b61df9d 100644 --- a/drivers/input/touchscreen/ti_am335x_tsc.c +++ b/drivers/input/touchscreen/ti_am335x_tsc.c @@ -348,9 +348,16 @@ static int titsc_parse_dt(struct platform_device *pdev, if (err 0) return err; - err = of_property_read_u32(node, ti,coordiante-readouts, + /* + * try with new binding first. If it fails, still try with + * bogus, miss-spelled version. + */ + err = of_property_read_u32(node, ti,coordinate-readouts, ts_dev-coordinate_readouts); if (err 0) + err = of_property_read_u32(node, ti,coordiante-readouts, + ts_dev-coordinate_readouts); + if (err 0) return err; Thanks, very good. Do we keep this fallback for ever or just for a year or two? That's for DT maintainers to decide but considering DT is an ABI, I guess we need to keep for 30 years or so :-p We keep it as long as we have to. If no-one's relying on the typo by the next merge window, I see no reason we'd have to keep support for the typo beyond that. If someone's shipped a device with a dtb with the typo hard-coded into some ROM, that's another matter... It might be worth printing a warning in the case of the typo'd version, suggesting correcting the DT. That will encourage anyone with a broken dt to get a fixed one. Mark. -- 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 V4 1/4] DRIVERS: IRQCHIP: IRQ-GIC: Add support for routable irqs
On Thu, Nov 14, 2013 at 12:18:47PM +, Sricharan R wrote: In some socs the gic can be preceded by a crossbar IP which routes the peripheral interrupts to the gic inputs. The peripheral interrupts are associated with a fixed crossbar input line and the crossbar routes that to one of the free gic input line. The DT entries for peripherals provides the fixed crossbar input line as its interrupt number and the mapping code should associate this with a free gic input line. This patch adds the support inside the gic irqchip to handle such routable irqs. The routable irqs are registered in a linear domain. The registered routable domain's callback should be implemented to get a free irq and to configure the IP to route it. Cc: Thomas Gleixner t...@linutronix.de Cc: Linus Walleij linus.wall...@linaro.org Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Russell King li...@arm.linux.org.uk Cc: Tony Lindgren t...@atomide.com Cc: Rajendra Nayak rna...@ti.com Cc: Marc Zyngier marc.zyng...@arm.com Cc: Grant Likely grant.lik...@linaro.org Cc: Rob Herring rob.herr...@calxeda.com Signed-off-by: Sricharan R r.sricha...@ti.com Acked-by: Santosh Shilimkar santosh.shilim...@ti.com --- [V2] Added default routable-irqs functions to avoid unnecessary if checks as per Thomas Gleixner comments and renamed routable-irq binding as per Kumar Gala ga...@codeaurora.org comments. [V3] Addressed unnecessary warn-on and updated default xlate function as per Thomas Gleixner comments Documentation/devicetree/bindings/arm/gic.txt |6 ++ drivers/irqchip/irq-gic.c | 81 ++--- include/linux/irqchip/arm-gic.h |7 ++- 3 files changed, 83 insertions(+), 11 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt index 3dfb0c0..5357745 100644 --- a/Documentation/devicetree/bindings/arm/gic.txt +++ b/Documentation/devicetree/bindings/arm/gic.txt @@ -49,6 +49,11 @@ Optional regions, used when the GIC doesn't have banked registers. The offset is cpu-offset * cpu-nr. +- arm,routable-irqs : Total number of gic irq inputs which are not directly + connected from the peripherals, but are routed dynamically + by a crossbar/multiplexer preceding the GIC. The GIC irq + input line is assigned dynamically when the corresponding + peripheral's crossbar line is mapped. I'm not keen on the design of the arm,routable-irqs property. The set of IRQs which the crossbar IP can use is a property of which IRQ lines it has routed to the GIC. I don't see why that should be considered a property of the GIC; it's a property of the crossbar IP's attachment to the GIC. Given we already have a mechanism for describing the attachment (i.e. the interrupts property) where the property appears on the node for the device generating/propagating the interrupt, I don't see why we should do differently here. Listing 160 interrupts in the crossbar node is clearly something we don't want to have to do. If we had a property that we could use to define a range (or multiple ranges) of interrupts, then the crossbar driver could go and request those ranges from its interrupt-parent (the GIC) and the GIC driver could reserve/allocate the irqdomain at that time. This feels like a point-hack, counter in style to the vast majority of provider/consumer bindings. It only allows for one multiplexer before the GIC. What if we had multiple multiplexers feeding into the GIC? Describing the attachment on the multiplexer allows that to be handled, describing that on the GIC does not. Describing the attachement on the multiplexer would also prevent the duplication of information (i.e. the max-irqs property in the crossbar binding). Thanks, Mark. -- 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 V4 2/4] DRIVERS: IRQCHIP: CROSSBAR: Add support for Crossbar IP
On Thu, Nov 14, 2013 at 12:18:48PM +, Sricharan R wrote: Some socs have a large number of interrupts requests to service the needs of its many peripherals and subsystems. All of the interrupt lines from the subsystems are not needed at the same time, so they have to be muxed to the irq-controller appropriately. In such places a interrupt controllers are preceded by an CROSSBAR that provides flexibility in muxing the device requests to the controller inputs. This driver takes care a allocating a free irq and then configuring the crossbar IP as a part of the mpu's irqchip callbacks. crossbar_init should be called right before the irqchip_init, so that it is setup to handle the irqchip callbacks. Cc: Thomas Gleixner t...@linutronix.de Cc: Linus Walleij linus.wall...@linaro.org Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Russell King li...@arm.linux.org.uk Cc: Tony Lindgren t...@atomide.com Cc: Rajendra Nayak rna...@ti.com Cc: Marc Zyngier marc.zyng...@arm.com Cc: Grant Likely grant.lik...@linaro.org Cc: Rob Herring rob.herr...@calxeda.com Signed-off-by: Sricharan R r.sricha...@ti.com Acked-by: Kumar Gala ga...@codeaurora.org (for DT binding portion) Acked-by: Santosh Shilimkar santosh.shilim...@ti.com --- [V2] Addressed Thomas Gleixner t...@linutronix.de comments and renamed the bindings as per Kumar Gala ga...@codeaurora.org comments. [V3] Changed static inline const to static inline int and removed unnecessary variable initialization as per Thomas Gleixner t...@linutronix.de. Updated commit tags [V4] Renamed crossbar_init as irqcrossbar_init as per Rajendra Nayak rna...@ti.com suggestion. .../devicetree/bindings/arm/omap/crossbar.txt | 27 +++ drivers/irqchip/Kconfig|8 + drivers/irqchip/Makefile |1 + drivers/irqchip/irq-crossbar.c | 206 include/linux/irqchip/irq-crossbar.h | 11 ++ 5 files changed, 253 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/omap/crossbar.txt create mode 100644 drivers/irqchip/irq-crossbar.c create mode 100644 include/linux/irqchip/irq-crossbar.h diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt new file mode 100644 index 000..fb88585 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt @@ -0,0 +1,27 @@ +Some socs have a large number of interrupts requests to service +the needs of its many peripherals and subsystems. All of the +interrupt lines from the subsystems are not needed at the same +time, so they have to be muxed to the irq-controller appropriately. +In such places a interrupt controllers are preceded by an CROSSBAR +that provides flexibility in muxing the device requests to the controller +inputs. + +Required properties: +- compatible : Should be ti,irq-crossbar +- reg: Base address and the size of the crossbar registers. +- ti,max-irqs: Total number of irqs available at the interrupt controller. +- ti,reg-size: Size of a individual register in bytes. Every individual + register is assumed to be of same size. Valid sizes are 1, 2, 4. +- ti,irqs-reserved: List of the reserved irq lines that are not muxed using + crossbar. These interrupt lines are reserved in the soc, + so crossbar bar driver should not consider them as free + lines. The combination of the ti,max-irqs and ti,irqs-reserved properties seems backwards to me. Why can we not describe the set of IRQs that _can_ be used? + +Examples: + crossbar_mpu: @4a02 { + compatible = ti,irq-crossbar; + reg = 0x4a002a48 0x130; + ti,max-irqs = 160; + ti,reg-size = 2; + ti,irqs-reserved = 0 1 2 3 5 6 131 132 139 140; + }; [...] + /* Get and mark reserved irqs */ + irqsr = of_get_property(node, ti,irqs-reserved, size); + if (irqsr) { + size /= sizeof(__be32); + + for (i = 0; i size; i++) { + entry = be32_to_cpup(irqsr + i); + if (entry max) { + pr_err(Invalid reserved entry\n); + goto err3; + } + cb-irq_map[entry] = 0; + } + } Don't deal with the raw DTB. Use of_property_read_u32_index. + + cb-register_offsets = kzalloc(max * sizeof(int), GFP_KERNEL); + if (!cb-register_offsets) + goto err3; + + of_property_read_u32(node, ti,reg-size, size); If ti,reg-size isn't present, size is uninitialized. Please check the return value of of_property_read_u32. Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-omap in
Re: [PATCHv3 0/8] omap hwspinlock dt support
On Tue, Nov 12, 2013 at 06:16:42PM +, Anna, Suman wrote: Hi, This is an updated series addressing the review comments from the v2 series. The hwmod patches have been dropped from the repost as per Paul's request, they have already been queued. Mark, Hi Suman, Any comments on this series? Tony has picked up the OMAP DTS patches for 3.13, and so the ti,omap4-hwspinlock compatible string is showing up as undocumented in linux-next. How do you want me to proceed here? I will be separating out the bindings into separate patches in the future. The only thing I note that I'm not so keen on is that the hwlock-specifier is always one cell, rather than using a #hwlock-cells property on the provider (even if we required it to be 1 for the moment and just failed if it wasn't). If possible, I would like an amendment to always use #hwlock-cells, but otherwise this looks fine to me. Feel free to add my Ack: Acked-by: Mark Rutland mark.rutl...@arm.com Thanks, Mark. -- 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] doc: devicetree: Add bindings documentation for omap-des driver
Hi Joel, I realise I'm a little late in replying to this, but there are a few things that would be nice to fix up. On Fri, Nov 08, 2013 at 12:37:09AM +, Joel Fernandes wrote: Add documentation for the generic OMAP DES crypto module describing the device tree bindings. Signed-off-by: Joel Fernandes jo...@ti.com --- .../devicetree/bindings/crypto/omap-des.txt| 28 ++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/omap-des.txt diff --git a/Documentation/devicetree/bindings/crypto/omap-des.txt b/Documentation/devicetree/bindings/crypto/omap-des.txt new file mode 100644 index 000..0637647 --- /dev/null +++ b/Documentation/devicetree/bindings/crypto/omap-des.txt @@ -0,0 +1,28 @@ +OMAP SoC DES crypto Module + +Required properties: + +- compatible : Should be ti,omap4-des Nit: s/Should be/Should contain/ +- ti,hwmods: Name of the hwmod associated with the DES module +- reg : Offset and length of the register set for the module +- interrupts : the interrupt-specifier for the DES module Just to check: there's only one interrupt? +- clocks : phandle to the functional clock node of the DES module +- clock-names : Name of the functional clock What name is expected here? The intent of clock-names is to describe which clock input lines the clocks are wired to, so this should be well-defined. From the looks of the example, the clock input is called fck. Is that correct? When you have clock-names, the nicest thing to do is to define clocks in terms of clock-names. Something like: - clocks: A phandle + clock-specifier pair for each entry in clock-names. - clock-names: Should contain: * fck for the functional clock Which implies that the correct way to find clocks is to look in clock-names first to find the clock's index, rather than grabbing the clock by index. This makes it far easier to add/remove/change clocks in future. + +Optional properties: +- dmas: DMA specifiers for tx and rx dma. See the DMA client binding, + Documentation/devicetree/bindings/dma/dma.txt Similarly here it would be nice to have dmas refer to dma-names for the description of which dmas exist. +- dma-names: DMA request names should include tx and rx if present + +Example: + /* DRA7xx SoC */ + des: des@480a5000 { + compatible = ti,omap4-des; + ti,hwmods = des; + reg = 0x480a5000 0xa0; + interrupts = GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH; + dmas = sdma 117, sdma 116; + dma-names = tx, rx; + clocks = l3_iclk_div; + clock-names = fck; + }; Cheers, Mark. -- 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] doc: devicetree: Add bindings documentation for omap-des driver
On Mon, Nov 11, 2013 at 05:13:35PM +, Joel Fernandes wrote: On 11/11/2013 05:01 AM, Mark Rutland wrote: Hi Joel, I realise I'm a little late in replying to this, but there are a few things that would be nice to fix up. On Fri, Nov 08, 2013 at 12:37:09AM +, Joel Fernandes wrote: Add documentation for the generic OMAP DES crypto module describing the device tree bindings. Signed-off-by: Joel Fernandes jo...@ti.com --- .../devicetree/bindings/crypto/omap-des.txt| 28 ++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/omap-des.txt diff --git a/Documentation/devicetree/bindings/crypto/omap-des.txt b/Documentation/devicetree/bindings/crypto/omap-des.txt new file mode 100644 index 000..0637647 --- /dev/null +++ b/Documentation/devicetree/bindings/crypto/omap-des.txt @@ -0,0 +1,28 @@ +OMAP SoC DES crypto Module + +Required properties: + +- compatible : Should be ti,omap4-des Nit: s/Should be/Should contain/ ok +- ti,hwmods: Name of the hwmod associated with the DES module +- reg : Offset and length of the register set for the module +- interrupts : the interrupt-specifier for the DES module Just to check: there's only one interrupt? Yes. +- clocks : phandle to the functional clock node of the DES module +- clock-names : Name of the functional clock What name is expected here? The intent of clock-names is to describe which clock input lines the clocks are wired to, so this should be well-defined. From the looks of the example, the clock input is called fck. Is that correct? Yes. The clock name is used by PM code to get the clock for the particular device. Yes the code assumes it to be fck, I can change doc to be so. When you have clock-names, the nicest thing to do is to define clocks in terms of clock-names. Something like: - clocks: A phandle + clock-specifier pair for each entry in clock-names. - clock-names: Should contain: * fck for the functional clock Which implies that the correct way to find clocks is to look in clock-names first to find the clock's index, rather than grabbing the clock by index. This makes it far easier to add/remove/change clocks in future. Ok. I'll reword documentation of clocks as above. + +Optional properties: +- dmas: DMA specifiers for tx and rx dma. See the DMA client binding, + Documentation/devicetree/bindings/dma/dma.txt Similarly here it would be nice to have dmas refer to dma-names for the description of which dmas exist. Ok, will make the above 3 changes and resubmit, if its ok will add your Reviewed-by. That would be fine by me. With the changes: Reviewed-by: Mark Rutland mark.rutl...@arm.com Mark. -- 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 v4 2/2] drm: omap: Enable DT support for DMM
On Tue, Oct 15, 2013 at 08:04:20AM +0100, Archit Taneja wrote: Enable use of DT for DMM/Tiler. Originally worked on by Andy Gross andy...@gmail.com Cc: Andy Gross andy...@gmail.com Cc: DRI Development dri-de...@lists.freedesktop.org Signed-off-by: Archit Taneja arc...@ti.com --- drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c index acf6678..59f17de 100644 --- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c +++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c @@ -968,12 +968,23 @@ static const struct dev_pm_ops omap_dmm_pm_ops = { }; #endif +#if defined(CONFIG_OF) +static const struct of_device_id dmm_of_match[] = { + { .compatible = ti,omap4-dmm, }, + { .compatible = ti,omap5-dmm, }, + {}, +}; +#else +#define dmm_of_match NULL +#endif + struct platform_driver omap_dmm_driver = { .probe = omap_dmm_probe, .remove = omap_dmm_remove, .driver = { .owner = THIS_MODULE, .name = DMM_DRIVER_NAME, + .of_match_table = dmm_of_match, If you use of_match_ptr(dmm_of_match) here you don't need the #else case above. Thanks, Mark. -- 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 v6 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DW PHY's
[...] + phy-sleep_a_clk = devm_clk_get(phy-dev, sleep_a); What means the _a in clock name? They are 2 PHY's on 8074 chip. This drivers is supposed to operate on PHY 0, thus sleep_a. PHY 1 is using sleep_b. Take a look here http://www.spinics.net/lists/arm-kernel/msg276945.html When you say two PHYs, do you mean the HS PHY and the SS PHY? Or are there two HS PHYs? If so, would the other HS PHY have a sleep_b clock? The clock-names property should describe the clocks from the view of the device itself. As we're describing the PHY in isolation, rather than a big block that contains the PHY, the presesnce or absence of other PHYs should not affect the name. If the _a suffix is only to differentiate the instance of PHY, it should be dropped. Thanks, Mark. -- 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 v9 1/9] mtd: nand: omap: combine different flavours of 1-bit hamming ecc schemes
Hi Pekon, On Tue, Oct 15, 2013 at 06:49:49AM +0100, Pekon Gupta wrote: OMAP NAND driver currently supports multiple flavours of 1-bit Hamming ecc-scheme, like: - OMAP_ECC_HAMMING_CODE_DEFAULT 1-bit hamming ecc code using software library - OMAP_ECC_HAMMING_CODE_HW 1-bit hamming ecc-code using GPMC h/w engine - OMAP_ECC_HAMMING_CODE_HW_ROMCODE 1-bit hamming ecc-code using GPMC h/w engin with ecc-layout compatible to ROM code. This patch combines above multiple ecc-schemes into single implementation: - OMAP_ECC_HAM1_CODE_HW 1-bit hamming ecc-code using GPMC h/w engine with ROM-code compatible ecc-layout. Signed-off-by: Pekon Gupta pe...@ti.com Reviewed-by: Felipe Balbi ba...@ti.com Acked-by: Tony Lindgren t...@atomide.com --- Documentation/devicetree/bindings/mtd/gpmc-nand.txt | 8 arch/arm/mach-omap2/board-flash.c | 2 +- arch/arm/mach-omap2/gpmc.c | 4 +--- drivers/mtd/nand/omap2.c| 9 +++-- include/linux/platform_data/mtd-nand-omap2.h| 6 +- 5 files changed, 10 insertions(+), 19 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt index df338cb..25ee232 100644 --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt @@ -22,10 +22,10 @@ Optional properties: width of 8 is assumed. - ti,nand-ecc-opt: A string setting the ECC layout to use. One of: - - swSoftware method (default) - hwHardware method - hw-romcodegpmc hamming mode method romcode layout + swdeprecated use ham1 instead + hwdeprecated use ham1 instead + hw-romcodedeprecated use ham1 instead + ham1 1-bit Hamming ecc code bch4 4-bit BCH ecc code bch8 8-bit BCH ecc code [...] diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c index 579697a..c9fb353 100644 --- a/arch/arm/mach-omap2/gpmc.c +++ b/arch/arm/mach-omap2/gpmc.c @@ -1342,9 +1342,7 @@ static void __maybe_unused gpmc_read_timings_dt(struct device_node *np, #ifdef CONFIG_MTD_NAND static const char * const nand_ecc_opts[] = { - [OMAP_ECC_HAMMING_CODE_DEFAULT] = sw, - [OMAP_ECC_HAMMING_CODE_HW] = hw, - [OMAP_ECC_HAMMING_CODE_HW_ROMCODE] = hw-romcode, + [OMAP_ECC_HAM1_CODE_HW] = ham1, [OMAP_ECC_BCH4_CODE_HW] = bch4, [OMAP_ECC_BCH8_CODE_HW] = bch8, }; As the parsing isn't updated until the next patch, doesn't this temporarily break DTBs with the deprecated ti,nand-ecc-opt values? Thanks, Mark. -- 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/6] mmc: omap_hsmmc: start using generic non-removable DT binding
On Wed, Oct 16, 2013 at 04:18:22PM +0100, Balaji T K wrote: From: Sekhar Nori nsek...@ti.com add generic non-removable binding support for omap_hsmmc Signed-off-by: Sekhar Nori nsek...@ti.com Signed-off-by: Balaji T K balaj...@ti.com --- .../devicetree/bindings/mmc/ti-omap-hsmmc.txt |2 +- drivers/mmc/host/omap_hsmmc.c |3 +++ 2 files changed, 4 insertions(+), 1 deletions(-) diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt index 8c8908a..3b95719 100644 --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt @@ -17,7 +17,7 @@ Optional properties: ti,dual-volt: boolean, supports dual voltage cards supply-name-supply: phandle to the regulator device tree node supply-name examples are vmmc, vmmc_aux etc -ti,non-removable: non-removable slot (like eMMC) +ti,non-removable: non-removable eMMC with always on vccq and configurable vcc Why this change? What do vccq and vcc correspond to? The regulators are called vmmc and vmmc_aux... Why is no mention of non-removable added, given that it's added to the code? Is one preferred over the other? That should be noted. ti,needs-special-reset: Requires a special softreset sequence ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed dmas: List of DMA specifiers with the controller specific format diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 6ac63df..5992048 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1738,6 +1738,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) pdata-slots[0].switch_pin = cd_gpio; pdata-slots[0].gpio_wp = wp_gpio; + if (of_find_property(np, non-removable, NULL)) { + pdata-slots[0].nonremovable = true; + } This wasn't mentioned in the binding, and it seems to have different semantics to ti,non-removable. Why is it different? if (of_find_property(np, ti,non-removable, NULL)) { pdata-slots[0].nonremovable = true; pdata-slots[0].no_regulator_off_init = true; Cheers, Mark. -- 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/6] mmc: omap_hsmmc: start using generic non-removable DT binding
On Thu, Oct 17, 2013 at 11:53:48AM +0100, Balaji T K wrote: On Thursday 17 October 2013 02:08 PM, Mark Rutland wrote: On Wed, Oct 16, 2013 at 04:18:22PM +0100, Balaji T K wrote: From: Sekhar Nori nsek...@ti.com add generic non-removable binding support for omap_hsmmc Signed-off-by: Sekhar Nori nsek...@ti.com Signed-off-by: Balaji T K balaj...@ti.com --- .../devicetree/bindings/mmc/ti-omap-hsmmc.txt |2 +- drivers/mmc/host/omap_hsmmc.c |3 +++ 2 files changed, 4 insertions(+), 1 deletions(-) diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt index 8c8908a..3b95719 100644 --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt @@ -17,7 +17,7 @@ Optional properties: ti,dual-volt: boolean, supports dual voltage cards supply-name-supply: phandle to the regulator device tree node supply-name examples are vmmc, vmmc_aux etc -ti,non-removable: non-removable slot (like eMMC) +ti,non-removable: non-removable eMMC with always on vccq and configurable vcc Why this change? Hi, earlier ti,non-removable was used for all eMMC and SDIO card, now it will be used only for eMMC with always on vccq and configurable vcc. Please expand the commit message to mention this. It wasn't clear why adding support for a property meant modifying the description of another. What do vccq and vcc correspond to? The regulators are called vmmc and vmmc_aux... vccq and vcc are supply names of eMMC part The binding has vmmc-supply and vmmc_aux-supply. How do {vmmc,vmmc_aux} and {vcc,vccq} relate? That should be clarified in the binding document, something like: - vmmc-supply: phandle of the regulator for the VCC input - vmmc_aux-supply: phandle of the regulator for the VCCQ input Why is no mention of non-removable added, given that it's added to the code? Because this file makes a reference to mmc.txt and the core properties described by mmc.txt are not added in ti-omap-hsmmc.txt There is room for confusion here. While non-removable is a generic property, it would be good to contrast non-removable and ti,non-removable in the binding as they imply different things. Is one preferred over the other? That should be noted. ti,needs-special-reset: Requires a special softreset sequence ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed dmas: List of DMA specifiers with the controller specific format diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 6ac63df..5992048 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1738,6 +1738,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev) pdata-slots[0].switch_pin = cd_gpio; pdata-slots[0].gpio_wp = wp_gpio; + if (of_find_property(np, non-removable, NULL)) { + pdata-slots[0].nonremovable = true; + } This wasn't mentioned in the binding, and it seems to have different semantics to ti,non-removable. Why is it different? When ti,non-removable was added, Only OMAP platform that had eMMC was that on OMAP4 where power to eMMC cannot be switched off without sending CMD5 sleep command, so no_regulator_off_init was needed to get it detected during boot. Now start using generic non-removable for all removable cards which do not have such limitation. OK. I think this would be much clearer with something in the binding contrasting the two properties. Thanks, Mark. -- 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: [RESEND PATCH v3 03/11] ASoC: davinci-mcasp: Add DMA register locations to DT
On Thu, Oct 10, 2013 at 06:29:59PM +0100, Peter Ujfalusi wrote: On 10/10/2013 07:59 PM, Mark Rutland wrote: No, they're not actually of much practical use to us at the minute but it was generally felt better to include the information and not use it so that if someone does come up with a use for them then the trees for deployed systems already have the information. Sure, but if this device can generate multiple interrupts, we should make it possible to describe all of them, unambiguously. This is why Jyri added them to the DT. They are not used by the Linux driver, but the HW have interrupt lines (two of them: TX and RX). The binding only describes a single interrupt, and even if multiple interrupts were listed, there's no way to disambiguate them (e.g. interrupt-names). It would be nice to remedy this. Cheers, Mark. -- 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 1/2] arm: dts: omap4+: Add DMM bindings
On Thu, Oct 10, 2013 at 07:36:33AM +0100, Archit Taneja wrote: Add Dynamic Memory Manager (DMM) bindings for OMAP4 and OMAP5 devices. DMM only requires address and irq information. Add documentation for the DMM bindings. Originally worked on by Andy Gross andy...@gmail.com Cc: Andy Gross andy...@gmail.com Signed-off-by: Archit Taneja arc...@ti.com --- Documentation/devicetree/bindings/arm/omap/dmm.txt | 16 arch/arm/boot/dts/omap4.dtsi | 7 +++ arch/arm/boot/dts/omap5.dtsi | 7 +++ 3 files changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/omap/dmm.txt diff --git a/Documentation/devicetree/bindings/arm/omap/dmm.txt b/Documentation/devicetree/bindings/arm/omap/dmm.txt new file mode 100644 index 000..6fc3d79 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/omap/dmm.txt @@ -0,0 +1,16 @@ +OMAP Dynamic Memory Manager (DMM) bindings Is there any documentation? A brief description of what this is would be nice. + +Required properties: +- compatible:Must be ti,omap4-dmm for OMAP4 family + Must be ti,omap5-dmm for OMAP5 and DRA7x family s/must be/should contain/ +- reg: Contains timer register address range (base address and length) Huh? What's a timer got to do with the DMM? +- interrupts:Contains interrupt information (source, etc) for the DMM IRQ Is there a single interrupt? If so: - interrupts: Should contain an interrupt-specifier for the DMM IRQ. Assuming the DMM IRQ is well defined. If there's a name for it in documentation, using that's preferable. If a future revision may have multiple interrupts, please use interrupt-names now to save us endless pain in future. Cheers, Mark. -- 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: [RESEND PATCH v3 03/11] ASoC: davinci-mcasp: Add DMA register locations to DT
On Tue, Oct 08, 2013 at 01:46:41AM +0100, Mark Brown wrote: On Mon, Oct 07, 2013 at 10:47:18PM +0100, Mark Rutland wrote: On Thu, Sep 26, 2013 at 08:18:28PM +0100, Jyri Sarha wrote: - interrupts : Interrupt number for McASP The device also seems to be able to generate multiple interrupts -- which interrupt does this actually cover? The driver doesn't seem to use it (by a grep of irq|interrupt). Have I missed something? No, they're not actually of much practical use to us at the minute but it was generally felt better to include the information and not use it so that if someone does come up with a use for them then the trees for deployed systems already have the information. Sure, but if this device can generate multiple interrupts, we should make it possible to describe all of them, unambiguously. Thanks, Mark. -- 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: [PATCHv2 1/9] hwspinlock/core: add common dt bindings and OF helpers
On Thu, Oct 03, 2013 at 05:04:15AM +0100, Suman Anna wrote: Hi Mark, On 10/01/2013 03:36 AM, Mark Rutland wrote: Hi Suman, Apologies for replying to a subthread, due to an earlier mistake on my part I don't have the original to hand. No issues, thanks for your review. On Fri, Sep 27, 2013 at 05:04:22PM +0100, Kumar Gala wrote: On Sep 17, 2013, at 2:30 PM, Suman Anna wrote: All the platform-specific hwlock driver implementations need the number of locks and the associated base id for registering the locks present within a hwspinlock device with the driver core. These two variables are represented by 'hwlock-num-locks' and 'hwlock-base-id' properties. The documentation and OF helpers to retrieve these common properties have been added to the driver core. Signed-off-by: Suman Anna s-a...@ti.com --- .../devicetree/bindings/hwlock/hwlock.txt | 26 + drivers/hwspinlock/hwspinlock_core.c | 61 +- include/linux/hwspinlock.h | 11 ++-- 3 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt new file mode 100644 index 000..789930e --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt @@ -0,0 +1,26 @@ +Generic hwlock bindings +=== + +Generic bindings that are common to all the hwlock platform specific driver +implementations, the retrieved values are used for registering the device +specific parameters with the hwspinlock core. + +The validity and need of these common properties may vary from one driver +implementation to another. Look through the individual hwlock driver +binding documentations for identifying which are mandatory and which are +optional for that specific driver. + +Common properties: +- hwlock-base-id:Base Id for the locks for a particular hwlock device. + This property is used for representing a set of locks + present in a hwlock device with a unique base id in + the driver core. This property is mandatory ONLY if a + SoC has several hwlock devices. + + See documentation on struct hwspinlock_pdata in + linux/hwspinlock.h for more details. I don't like this, as it seems to be encoding a Linux implementation detail (the ID of the lock, which means that we have to have a numeric representation of each hwlock) in the device tree. I don't see why the ID within Linux should be a concern of the device tree binding. I think that whatever internal identifier we have in Linux (be it an integer or struct) should be allocated by Linux. If a driver needs to request specific hwlocks, then we should have a phandle + args representation for referring to a specific hwlock that bidnings can use, and some code for parsing that and returning a Linux-internal identifier/struct as we do with interrupts and clocks. This is based on gathering the information required by the platform implementation drivers for registering with the driver core. The driver core currently maintains all the locks from different instances as a radix tree, as it is simpler to manage when granting locks to users. The current version is based on allowing the platform implementation drivers to retrieve the required data for registering with the hwspinlock driver core. Ok. I don't see why this implementation detail needs to leak into the dt. The users would either get a lock dynamically by requesting any free one (and extract the id thereafter to share with others), or a specific one which is currently by id. I agree that the phandle + args approach is best suited for requesting a specific one through DT, but I would think that the args would still have to be a relative lock number from 0 w.r.t a phandle. I will look into the feasibility of such an approach retaining the radix tree implementation, as this requires new OF friendly registration and request functions. The value in the args would be a unique identifier within the unit pointed to be the phandle, but the mechanism by which it would refer to a particular lock would be binding-specific. It's perfectly fine for this to be an zero-based index in most bindings, but it should not be a global namespace as with the hwlock-base-id property approach. + +- hwlock-num-locks: Number of locks present in a hwlock device. This + property is needed on hwlock devices, where the number + of supported locks within a hwlock device cannot be + read from a register. Hmm... this seems generic, but it doesn't allow for sparse ID spaces on the hwlock module. It should
Re: [PATCHv2 2/9] hwspinlock/omap: add support for dt nodes
On Thu, Oct 03, 2013 at 05:12:15AM +0100, Suman Anna wrote: Hi Mark, On Fri, Sep 27, 2013 at 05:06:38PM +0100, Kumar Gala wrote: On Sep 17, 2013, at 2:30 PM, Suman Anna wrote: HwSpinlock IP is present only on OMAP4 and other newer SoCs, which are all device-tree boot only. This patch adds the base support for parsing the DT nodes, and removes the code dealing with the traditional platform device instantiation. Signed-off-by: Suman Anna s-a...@ti.com --- .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 31 +++ arch/arm/mach-omap2/Makefile | 3 -- arch/arm/mach-omap2/hwspinlock.c | 60 -- drivers/hwspinlock/omap_hwspinlock.c | 23 +++-- 4 files changed, 50 insertions(+), 67 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt delete mode 100644 arch/arm/mach-omap2/hwspinlock.c diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt new file mode 100644 index 000..235b7c5 --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt @@ -0,0 +1,31 @@ +OMAP4+ HwSpinlock Driver + + +Required properties: +- compatible:Currently supports only ti,omap4-hwspinlock for + OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs Currently supports is not something I expect to see in a binding document. That sounds like a description of the driver rather than the binding. How similar are these hardware modules? What are the differences? The IP is almost the same, they all have the same revision id. The number of locks (each represented by a register) though vary from one SoC to another (OMAP4, OMAP5, DRA7 have same number of locks, and AM33xx/AM43xx have a different number). The number of locks is directly read by the driver from a module register. There is no separate .data associated with the of_device_id table, so I used a single compatible property for all the SoCs. Ok. Probeability is good, it keeps these simpler :) I think This can be reworded to say should contain rather than currently supports only: - compatible: Should contain ti,omap4-hwspinlock for OMAP44xx, OMAP54xx, AM33xx, AM43xx, or DRA7xx SoCs That way the binding allows for a future backwards-compatible variant, and doesn't mention the current level of support in Linux. +- reg: Contains the hwspinlock register address range (base + address and length) Is there only one register bank for the hwlock module? The lock registers start at a certain offset (0x800) within the module register space, and the offsets for various registers are identical between all SoCs. What are the other registers within the module? Are they shared with other devices, or are they simply unused by the hwspinlock driver? +- ti,hwmods: Name of the hwmod associated with the hwspinlock device + +Common hwlock properties: +The following describes the usage of the common hwlock properties (defined in +Documentation/devicetree/bindings/hwlock/hwlock.txt) on OMAP. + +- hwlock-base-id:There are currently no OMAP SoCs with multiple + hwspinlock devices. The OMAP driver uses a default + base id value of 0 for the locks present within the + single hwspinlock device on all SoCs. Driver details should not leak into bindngs... OK, will remove the info on driver details. As mentioned in the other patch, I don't think this is the way to handle this. I think we need a phandle + args representation. This is an optional parameter for now and I was going to revise the description based on comments from Kumar Gala on this thread, but I will wait and adjust this based on the outcome on the first patch. Ok. +- hwlock-num-locks: This property is not required on OMAP SoCs, since the + number of locks present within a device can be deduced + from the SPINLOCK_SYSSTATUS device register. Huh? Why define this property at all here if we don't need it and don't use it? The common document should state that specific bindings may use it and should explicitly state if they do, rather than stating they don't... Yeah, I wasn't sure how to go about the split between the common file and the platform-specific bindings. I will clean this up and revise the common bindings. Ok. Cheers, Mark. -- 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/6] mtd: nand: omap: combine different flavours of 1-bit hamming ecc schemes
On Fri, Oct 04, 2013 at 08:49:43PM +0100, Pekon Gupta wrote: OMAP NAND driver currently supports multiple flavours of 1-bit Hamming ecc-scheme, like: - OMAP_ECC_HAMMING_CODE_DEFAULT 1-bit hamming ecc code using software library - OMAP_ECC_HAMMING_CODE_HW 1-bit hamming ecc-code using GPMC h/w engine - OMAP_ECC_HAMMING_CODE_HW_ROMCODE 1-bit hamming ecc-code using GPMC h/w engin with ecc-layout compatible to ROM code. This patch combines above multiple ecc-schemes into single implementation: - OMAP_ECC_HAM1_CODE_HW 1-bit hamming ecc-code using GPMC h/w engine with ROM-code compatible ecc-layout. If I have my NAND formatted with one of the existing ECC schemes (e.g. OMAP_ECC_HAMMING_CODE_DEFAULT) will it work with the new OMAP_ECC_HAM1_CODE_HW scheme? Are they all compatible? Signed-off-by: Pekon Gupta pe...@ti.com --- Documentation/devicetree/bindings/mtd/gpmc-nand.txt | 8 arch/arm/mach-omap2/board-flash.c | 2 +- arch/arm/mach-omap2/gpmc.c | 4 +--- drivers/mtd/nand/omap2.c| 9 +++-- include/linux/platform_data/mtd-nand-omap2.h| 6 +- 5 files changed, 10 insertions(+), 19 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt index df338cb..25ee232 100644 --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt @@ -22,10 +22,10 @@ Optional properties: width of 8 is assumed. - ti,nand-ecc-opt: A string setting the ECC layout to use. One of: - - swSoftware method (default) - hwHardware method - hw-romcodegpmc hamming mode method romcode layout + swdeprecated use ham1 instead + hwdeprecated use ham1 instead + hw-romcodedeprecated use ham1 instead + ham1 1-bit Hamming ecc code bch4 4-bit BCH ecc code bch8 8-bit BCH ecc code diff --git a/arch/arm/mach-omap2/board-flash.c b/arch/arm/mach-omap2/board-flash.c index fc20a61..ac82512 100644 --- a/arch/arm/mach-omap2/board-flash.c +++ b/arch/arm/mach-omap2/board-flash.c @@ -142,7 +142,7 @@ __init board_nand_init(struct mtd_partition *nand_parts, u8 nr_parts, u8 cs, board_nand_data.nr_parts= nr_parts; board_nand_data.devsize = nand_type; - board_nand_data.ecc_opt = OMAP_ECC_HAMMING_CODE_DEFAULT; + board_nand_data.ecc_opt = OMAP_ECC_BCH8_CODE_HW; gpmc_nand_init(board_nand_data, gpmc_t); } #endif /* CONFIG_MTD_NAND_OMAP2 || CONFIG_MTD_NAND_OMAP2_MODULE */ diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c index 9f4795a..1c45b72 100644 --- a/arch/arm/mach-omap2/gpmc.c +++ b/arch/arm/mach-omap2/gpmc.c @@ -1342,9 +1342,7 @@ static void __maybe_unused gpmc_read_timings_dt(struct device_node *np, #ifdef CONFIG_MTD_NAND static const char * const nand_ecc_opts[] = { - [OMAP_ECC_HAMMING_CODE_DEFAULT] = sw, - [OMAP_ECC_HAMMING_CODE_HW] = hw, - [OMAP_ECC_HAMMING_CODE_HW_ROMCODE] = hw-romcode, + [OMAP_ECC_HAM1_CODE_HW] = ham1, [OMAP_ECC_BCH4_CODE_HW] = bch4, [OMAP_ECC_BCH8_CODE_HW] = bch8, Won't this break existing dts which have sw, hw, or hw-romcode? Someone may try to use a new kernel with an old dt, and we marked them as deprecated, not removed. Thanks, Mark. -- 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 2/6] ARM: OMAP2+: cleaned-up DT support of various ECC schemes
On Fri, Oct 04, 2013 at 08:49:44PM +0100, Pekon Gupta wrote: OMAP NAND driver support multiple ECC scheme, which can used in following different flavours, depending on in-build Hardware engines supported by SoC. +---+---+---+ | ECC scheme|ECC calculation|Error detection| +---+---+---+ |OMAP_ECC_HAM1_CODE_HW |H/W (GPMC) |S/W| +---+---+---+ |OMAP_ECC_BCH8_CODE_HW_DETECTION_SW |H/W (GPMC) |S/W| |(requires CONFIG_MTD_NAND_ECC_BCH) | | | +---+---+---+ |OMAP_ECC_BCH8_CODE_HW |H/W (GPMC) |H/W (ELM) | |(requires CONFIG_MTD_NAND_OMAP_BCH | | | | ti,elm-id in DT) | | | +---+---+---+ To optimize footprint of omap2-nand driver, selection of some ECC schemes also require enabling following Kconfigs, in addition to setting appropriate DT bindings - Kconfig:CONFIG_MTD_NAND_ECC_BCHerror detection done in software - Kconfig:CONFIG_MTD_NAND_OMAP_BCH error detection done by h/w engine DT binding updates in this patch are: - ti,elm-id: replaces elm_id - ti,nand-ecc-opts: supported values ham1, bch4, and bch8 selection of h/w or s/w implementation depends on ti,elm-id Signed-off-by: Pekon Gupta pe...@ti.com --- .../devicetree/bindings/mtd/gpmc-nand.txt | 8 +++- arch/arm/mach-omap2/gpmc.c | 47 -- include/linux/platform_data/mtd-nand-omap2.h | 14 +-- 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt index 25ee232..7785666 100644 --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt @@ -36,8 +36,12 @@ Optional properties: prefetch-dma Prefetch enabled sDMA mode prefetch-irq Prefetch enabled irq mode - - elm_id: Specifies elm device node. This is required to support BCH - error correction using ELM module. + - elm_id: deprecated use ti,elm-id instead + - ti,elm-id:Specifies pHandle of the ELM devicetree node. s/pHandle/phandle/ + ELM is an on-chip hardware engine on TI SoC which is used for + locating ECC errors for BCHx algorithms. SoC devices which have + ELM hardware engines should specify this device node in .dtsi + Using ELM for ECC error correction frees some CPU cycles. For inline partiton table parsing (optional): diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c index 1c45b72..5a607fa 100644 --- a/arch/arm/mach-omap2/gpmc.c +++ b/arch/arm/mach-omap2/gpmc.c @@ -1341,12 +1341,6 @@ static void __maybe_unused gpmc_read_timings_dt(struct device_node *np, #ifdef CONFIG_MTD_NAND -static const char * const nand_ecc_opts[] = { - [OMAP_ECC_HAM1_CODE_HW] = ham1, - [OMAP_ECC_BCH4_CODE_HW] = bch4, - [OMAP_ECC_BCH8_CODE_HW] = bch8, -}; - static const char * const nand_xfer_types[] = { [NAND_OMAP_PREFETCH_POLLED] = prefetch-polled, [NAND_OMAP_POLLED] = polled, @@ -1361,6 +1355,8 @@ static int gpmc_probe_nand_child(struct platform_device *pdev, const char *s; struct gpmc_timings gpmc_t; struct omap_nand_platform_data *gpmc_nand_data; + const __be32 *phandle; I don't think you need this. With of_parse_phandle you should never need to see the raw phandle value. + int lenp; if (of_property_read_u32(child, reg, val) 0) { dev_err(pdev-dev, %s has no 'reg' property\n, @@ -1376,12 +1372,39 @@ static int gpmc_probe_nand_child(struct platform_device *pdev, gpmc_nand_data-cs = val; gpmc_nand_data-of_node = child; - if (!of_property_read_string(child, ti,nand-ecc-opt, s)) - for (val = 0; val ARRAY_SIZE(nand_ecc_opts); val++) - if (!strcasecmp(s, nand_ecc_opts[val])) { - gpmc_nand_data-ecc_opt = val; - break; - } + /* Detect availability of ELM module */ + phandle = of_get_property(child, ti,elm-id, lenp); + if ((phandle == NULL) || (lenp != sizeof(void *))) { + pr_warn(%s: ti,elm-id property not found\n, __func__); + gpmc_nand_data-elm_of_node = NULL; + } else { + gpmc_nand_data-elm_of_node
Re: [PATCH v7 5/6] ARM: dts: AM33xx: updated default ECC scheme in nand-ecc-opt
On Fri, Oct 04, 2013 at 08:49:47PM +0100, Pekon Gupta wrote: DT property values for OMAP based gpmc-nand have been updated to match changes in commit: 6faf096 ARM: OMAP2+: cleaned-up DT support of various ECC schemes Refer: Documentation/devicetree/bindings/mtd/gpmc-nand.txt Doesn't this mean these dts were broken between the changes to the driver and the changes to the dts? I think the existing properties need to continue to be supoprted for backwards compatibility. The dts can be fixed up to have the preferred binding style, but they shouldn't _have_ to be modified to continue to function... Thanks, Mark. Signed-off-by: Pekon Gupta pe...@ti.com --- arch/arm/boot/dts/am335x-evm.dts | 3 +-- arch/arm/boot/dts/omap3430-sdp.dts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts index e8ec875..1aee6ac 100644 --- a/arch/arm/boot/dts/am335x-evm.dts +++ b/arch/arm/boot/dts/am335x-evm.dts @@ -269,7 +269,6 @@ reg = 0 0 0; /* CS0, offset 0 */ nand-bus-width = 8; ti,nand-ecc-opt = bch8; - gpmc,device-nand = true; gpmc,device-width = 1; gpmc,sync-clk-ps = 0; gpmc,cs-on-ns = 0; @@ -296,7 +295,7 @@ #address-cells = 1; #size-cells = 1; - elm_id = elm; + ti,elm-id = elm; /* MTD partition table */ partition@0 { diff --git a/arch/arm/boot/dts/omap3430-sdp.dts b/arch/arm/boot/dts/omap3430-sdp.dts index e2249bc..501f863 100644 --- a/arch/arm/boot/dts/omap3430-sdp.dts +++ b/arch/arm/boot/dts/omap3430-sdp.dts @@ -105,7 +105,7 @@ reg = 1 0 0x0800; nand-bus-width = 8; - ti,nand-ecc-opt = sw; + ti,nand-ecc-opt = ham1; gpmc,cs-on-ns = 0; gpmc,cs-rd-off-ns = 36; gpmc,cs-wr-off-ns = 36; -- 1.8.1 -- 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: [RESEND PATCH v3 03/11] ASoC: davinci-mcasp: Add DMA register locations to DT
Hello, On Thu, Sep 26, 2013 at 08:18:28PM +0100, Jyri Sarha wrote: This patch adds DMA register location to mcasp DT bindings. On am33xx SoCs the McASP registers are mapped trough L4 interconnect, which is not accessible by the DMA controller, so McASP data port is mapped trough L3 to a different location. Signed-off-by: Hebbar, Gururaja gururaja.heb...@ti.com Signed-off-by: Darren Etheridge detheri...@ti.com Signed-off-by: Jyri Sarha jsa...@ti.com --- .../bindings/sound/davinci-mcasp-audio.txt |8 ++- sound/soc/davinci/davinci-mcasp.c | 59 +--- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt index 374e145..63b67ae 100644 --- a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt +++ b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt @@ -6,7 +6,11 @@ Required properties: ti,da830-mcasp-audio : for both DA830 DA850 platforms ti,omap2-mcasp-audio : for OMAP2 platforms (TI81xx, AM33xx) -- reg : Should contain McASP registers offset and length +- reg : Should contain McASP registers address and length for mpu and + optionally for dma controller access. +- reg-names : The mandatory reg-range must be named mpu and the optional DMA + reg-range must be named dma. For backward compatibility it is + good to keep mpu first in the list. I've never heard the term reg-range before. The should probably be something like reg entry. How about something like the following instead: - reg: Should contain reg specifiers for the entries in the reg-names property. - reg-names: Should contain: * mpu for the main registers (required). For compatibility with existing software, it is recommended this is the first entry. * dma for the DMA registers (optional). That way we don't end up describing each reg entry twice. I have some questions however. I took a look at the McASP (TMS320C6000) reference guide, and the registers appeared to all be in one contiguous bank, and mpu and dma don't appear to be names of particular registers or names of banks of particular registers. Am I looking in the wrong place? Is there up-to-date documentation I can look at? Why are these split across two reg entries, and which particular registers do they actually cover? I have some concern about the description of other properties too. If we're going to amend the binding, they should be fixed up too. - interrupts : Interrupt number for McASP The device also seems to be able to generate multiple interrupts -- which interrupt does this actually cover? The driver doesn't seem to use it (by a grep of irq|interrupt). Have I missed something? - op-mode : I2S/DIT ops mode. What type is this? What are valid values? - tdm-slots : Slots for TDM operation. Here too. This description is completely opaque. Taking a look at the version in kernel sources this appears to be a list of values, in groups of four? What is the format of this property? @@ -15,7 +19,6 @@ Required properties: to num-serializer parameter. Each entry is a number indication serializer pin direction. (0 - INACTIVE, 1 - TX, 2 - RX) - Optional properties: - ti,hwmods : Must be mcaspn, n is controller instance starting 0 @@ -31,6 +34,7 @@ mcasp0: mcasp0@1d0 { #address-cells = 1; #size-cells = 0; Why does this have #address-cells and #size-cells? There are no children in the example. reg = 0x10 0x3000; + reg-names mpu; interrupts = 82 83; op-mode = 0; /* MCASP_IIS_MODE */ tdm-slots = 2; A few questions from a brief skim of the documentation: There seem to be external clocks (AHCLKR and ACLKR), but they aren't described. Are we always able to use an internal clock instead? Is no external reference clock needed? The err_release_clk label in the error path confuses me -- which clock(s) does the preceding pm_runtime_get ensure remains active? One internal to the McASP? It looks like some pins can be used as GPIO -- is there not a need for something like pinctrl for configuring this? Cheers, Mark. diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index 32ddb7f..a056fc5 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -1001,18 +1001,40 @@ static const struct snd_soc_component_driver davinci_mcasp_component = { .name = davinci-mcasp, }; +/* Some HW specific values and defaults. The rest is filled in from DT. */ +static struct snd_platform_data dm646x_mcasp_pdata = { + .tx_dma_offset = 0x400, + .rx_dma_offset = 0x400, + .asp_chan_q = EVENTQ_0, + .version = MCASP_VERSION_1, +}; + +static struct snd_platform_data
Re: [RESEND PATCH v3 04/11] ASoC: davinci-mcasp: Extract DMA channels directly from DT
On Thu, Sep 26, 2013 at 08:18:29PM +0100, Jyri Sarha wrote: Extract DMA channels directly from DT as they can not be found from platform resources anymore. This is a work-around until davinci audio driver is updated to use dmaengine. How long will this conversion take? Signed-off-by: Jyri Sarha jsa...@ti.com --- .../bindings/sound/davinci-mcasp-audio.txt |5 +++ include/linux/platform_data/davinci_asp.h |2 + sound/soc/davinci/davinci-mcasp.c | 47 +--- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt index 63b67ae..68e0f47 100644 --- a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt +++ b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt @@ -18,6 +18,11 @@ Required properties: - serial-dir : A list of serializer pin mode. The list number should be equal to num-serializer parameter. Each entry is a number indication serializer pin direction. (0 - INACTIVE, 1 - TX, 2 - RX) +- dmas: two element list of DMA controller phandles and DMA request line +ordered pairs. Please describe this in terms of dma-names. That makes it clear that elements cannot be retrieved consistently by index, and prevents duplicate descriptions. I'd prefer for the sake of consistent terminology that these were referred to as phandles + dma-specifiers rather than phandles and DMA request lines -- #dma-cells may be an arbitrary number of cells and encode arbitrary information for some abstract DMA engine. +- dma-names: identifier string for each DMA request line in the dmas property. + These strings correspond 1:1 with the ordered pairs in dmas. The dma + identifiers must be rx and tx. For consistency and future expandability: s/must be/should contain/ Cheers, Mark. Optional properties: diff --git a/include/linux/platform_data/davinci_asp.h b/include/linux/platform_data/davinci_asp.h index 8db5ae0..689a856 100644 --- a/include/linux/platform_data/davinci_asp.h +++ b/include/linux/platform_data/davinci_asp.h @@ -84,6 +84,8 @@ struct snd_platform_data { u8 version; u8 txnumevt; u8 rxnumevt; + int tx_dma_channel; + int rx_dma_channel; }; enum { diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index a056fc5..acbf5f8 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -1047,6 +1047,7 @@ static struct snd_platform_data *davinci_mcasp_set_pdata_from_of( struct snd_platform_data *pdata = NULL; const struct of_device_id *match = of_match_device(mcasp_dt_ids, pdev-dev); + struct of_phandle_args dma_spec; const u32 *of_serial_dir32; u8 *of_serial_dir; @@ -1109,6 +1110,28 @@ static struct snd_platform_data *davinci_mcasp_set_pdata_from_of( pdata-serial_dir = of_serial_dir; } + ret = of_property_match_string(np, dma-names, tx); + if (ret 0) + goto nodata; + + ret = of_parse_phandle_with_args(np, dmas, #dma-cells, ret, + dma_spec); + if (ret 0) + goto nodata; + + pdata-tx_dma_channel = dma_spec.args[0]; + + ret = of_property_match_string(np, dma-names, rx); + if (ret 0) + goto nodata; + + ret = of_parse_phandle_with_args(np, dmas, #dma-cells, ret, + dma_spec); + if (ret 0) + goto nodata; + + pdata-rx_dma_channel = dma_spec.args[0]; + ret = of_property_read_u32(np, tx-num-evt, val); if (ret = 0) pdata-txnumevt = val; @@ -1139,7 +1162,7 @@ nodata: static int davinci_mcasp_probe(struct platform_device *pdev) { struct davinci_pcm_dma_params *dma_data; - struct resource *mem, *ioarea, *res; + struct resource *mem, *ioarea, *res, *dma; struct snd_platform_data *pdata; struct davinci_audio_dev *dev; int ret; @@ -1213,15 +1236,11 @@ static int davinci_mcasp_probe(struct platform_device *pdev) dma_data-sram_size = pdata-sram_size_playback; dma_data-dma_addr = dma-start + pdata-tx_dma_offset; - /* first TX, then RX */ res = platform_get_resource(pdev, IORESOURCE_DMA, 0); - if (!res) { - dev_err(pdev-dev, no DMA resource\n); - ret = -ENODEV; - goto err_release_clk; - } - - dma_data-channel = res-start; + if (res) + dma_data-channel = res-start; + else + dma_data-channel = pdata-tx_dma_channel; dma_data = dev-dma_params[SNDRV_PCM_STREAM_CAPTURE]; dma_data-asp_chan_q = pdata-asp_chan_q; @@ -1231,13 +1250,11 @@ static int
Re: [RESEND PATCH v3 05/11] ASoC: davinci-mcasp: Interrupts property to optional and add interrupt-names
On Thu, Sep 26, 2013 at 08:18:30PM +0100, Jyri Sarha wrote: Makes interrupts property optional as the interrupts are not currently used by the driver and adds interrupt-names property to name listed interrupts. Currently know interrupt names are tx and rx. Signed-off-by: Jyri Sarha jsa...@ti.com --- .../bindings/sound/davinci-mcasp-audio.txt |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt index 68e0f47..2fd0bf2 100644 --- a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt +++ b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt @@ -11,7 +11,6 @@ Required properties: - reg-names : The mandatory reg-range must be named mpu and the optional DMA reg-range must be named dma. For backward compatibility it is good to keep mpu first in the list. -- interrupts : Interrupt number for McASP - op-mode : I2S/DIT ops mode. - tdm-slots : Slots for TDM operation. - num-serializer : Serializers used by McASP. @@ -31,6 +30,8 @@ Optional properties: - rx-num-evt : FIFO levels. - sram-size-playback : size of sram to be allocated during playback - sram-size-capture : size of sram to be allocated during capture +- interrupts : Interrupt numbers for McASP, currently not used by the driver +- interrupt-names : Known interrupt names are tx and rx Are these _all_ the interrupts the McASP may generate? I was under the impression there were also separate interrupts for errors. Cheers, Mark. Example: @@ -41,6 +42,7 @@ mcasp0: mcasp0@1d0 { reg = 0x10 0x3000; reg-names mpu; interrupts = 82 83; + interrupts-names = tx, rx; op-mode = 0; /* MCASP_IIS_MODE */ tdm-slots = 2; num-serializer = 16; -- 1.7.9.5 -- 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: [RESEND PATCH v3 10/11] ARM/dts: am33xx: mcasp: Add new dma register location to reg-property
On Thu, Sep 26, 2013 at 08:18:35PM +0100, Jyri Sarha wrote: This patch adds an optional address range to reg property. The range describes the register location for DMA controller on am33xx. The both address ranges are named accordingly in the reg-names property. Signed-off-by: Hebbar, Gururaja gururaja.heb...@ti.com Signed-off-by: Darren Etheridge detheri...@ti.com Signed-off-by: Jyri Sarha jsa...@ti.com --- arch/arm/boot/dts/am33xx.dtsi | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index fe53ce0..4dc388a 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -556,19 +556,29 @@ mcasp0: mcasp@48038000 { compatible = ti,omap2-mcasp-audio; ti,hwmods = mcasp0; - reg = 0x48038000 0x2000; + reg = 0x48038000 0x2000, + 0x4640 0x40; + reg-names = mpu, dma; interrupts = 80 81; interrupts-names = tx, rx; status = disabled; + dmas = edma 8 + edma 9; For consistency with reg and other composite value properties, I'd prefer that each entry in the list were individually bracketed: dmas = edma 8, edma 9; It would also be nice if interrupts were written this way. + dma-names = tx, rx; }; mcasp1: mcasp@4803C000 { compatible = ti,omap2-mcasp-audio; ti,hwmods = mcasp1; - reg = 0x4803C000 0x2000; + reg = 0x4803C000 0x2000, + 0x4640 0x40; + reg-names = mpu, dma; interrupts = 82 83; interrupts-names = tx, rx; status = disabled; + dmas = edma 10 + edma 11; Similarly here. Cheers, Mark. -- 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: [RESEND PATCH v3 11/11] ARM/dts: am335x-evm: Add audio support for am335x-evm.dts
On Thu, Sep 26, 2013 at 08:18:36PM +0100, Jyri Sarha wrote: From: Darren Etheridge detheri...@ti.com Adds sound, tlv320aic3x, mcasp1, and am335x_evm_audio_pin nodes. Signed-off-by: Darren Etheridge detheri...@ti.com Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com Signed-off-by: Jyri Sarha jsa...@ti.com --- arch/arm/boot/dts/am335x-evm.dts | 56 ++ 1 file changed, 56 insertions(+) diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts index 3aee1a4..4a49229 100644 --- a/arch/arm/boot/dts/am335x-evm.dts +++ b/arch/arm/boot/dts/am335x-evm.dts @@ -149,6 +149,16 @@ 0x14c (PIN_INPUT_PULLDOWN | MUX_MODE7) ; }; + + am335x_evm_audio_pins: am335x_evm_audio_pins { + pinctrl-single,pins = + 0x10c (PIN_INPUT_PULLDOWN | MUX_MODE4) /* mii1_rx_dv.mcasp1_aclkx */ + 0x110 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* mii1_txd3.mcasp1_fsx */ + 0x108 (PIN_OUTPUT_PULLDOWN | MUX_MODE4) /* mii1_col.mcasp1_axr2 */ + 0x144 (PIN_INPUT_PULLDOWN | MUX_MODE4) /* rmii1_ref_clk.mcasp1_axr3 */ + ; + }; + }; ocp { @@ -215,6 +225,19 @@ compatible = ti,tmp275; reg = 0x48; }; + + tlv320aic3x: tlv320aic3x@1b { + compatible = ti,tlv320aic3x; + reg = 0x1b; + status = okay; + + /* Regulators */ + AVDD-supply = vaux2_reg; + IOVDD-supply = vaux2_reg; + DRVDD-supply = vaux2_reg; + DVDD-supply = vbat; + }; + }; elm: elm@4808 { @@ -311,6 +334,20 @@ }; }; }; + + sound { + compatible = ti,da830-evm-audio; + ti,model = DA830 EVM; + ti,audio-codec = tlv320aic3x; + ti,mcasp-controller = mcasp1; + ti,codec-clock-rate = 1200; + ti,audio-routing = + Headphone Jack, HPLOUT, + Headphone Jack, HPROUT, + LINE1L, Line In, + LINE1R, Line In; + }; + }; vbat: fixedregulator@0 { @@ -378,6 +415,25 @@ #include tps65910.dtsi +mcasp1 { + pinctrl-names = default; + pinctrl-0 = am335x_evm_audio_pins; I didn't see mention of pinctrl added to the binding. It should be. Thanks, Mark. + + status = okay; + + op-mode = 0; /* MCASP_IIS_MODE */ + tdm-slots = 2; + num-serializer = 16; + serial-dir = /* 0: INACTIVE, 1: TX, 2: RX */ + 0 0 1 2 + 0 0 0 0 + 0 0 0 0 + 0 0 0 0 + ; + tx-num-evt = 1; + rx-num-evt = 1; +}; + tps { vcc1-supply = vbat; vcc2-supply = vbat; -- 1.7.9.5 -- 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 v2 3/9] ARM: dts: Add SHAM data and documentation for AM33XX
On Mon, Sep 30, 2013 at 04:13:00PM +0100, Joel Fernandes wrote: From: Mark A. Greer mgr...@animalcreek.com Add the generic AM33XX SHAM module's device tree data and enable it for the am335x-evm, am335x-evmsk, and am335x-bone platforms. Also add Documentation file describing the data for the SHAM module. [jo...@ti.com: Dropped interrupt-parrent property] CC: Paul Walmsley p...@pwsan.com Signed-off-by: Mark A. Greer mgr...@animalcreek.com --- .../devicetree/bindings/crypto/omap-sham.txt | 31 ++ arch/arm/boot/dts/am335x-bone.dts | 4 +++ arch/arm/boot/dts/am335x-evm.dts | 4 +++ arch/arm/boot/dts/am335x-evmsk.dts | 4 +++ arch/arm/boot/dts/am33xx.dtsi | 9 +++ 5 files changed, 52 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/omap-sham.txt diff --git a/Documentation/devicetree/bindings/crypto/omap-sham.txt b/Documentation/devicetree/bindings/crypto/omap-sham.txt new file mode 100644 index 000..b97710f --- /dev/null +++ b/Documentation/devicetree/bindings/crypto/omap-sham.txt @@ -0,0 +1,31 @@ +OMAP SoC SHA crypto Module + +Required properties: + +- compatible : Should contain entries for this and backward compatible + SHAM versions: + - ti,omap2-sham for OMAP2 OMAP3. + - ti,omap4-sham for OMAP4 and AM33XX. + Note that these two versions are incompatible. +- ti,hwmods: Name of the hwmod associated with the SHAM module +- reg : Offset and length of the register set for the module +- interrupt-parent : the phandle for the interrupt controller that + services interrupts for this module. I don't think this is strictly speaking necessary -- it's mostly going to be implicit (it is in the dtsi below). As this is a standard property, you don't need to document it here. +- interrupts : the interrupt number for the SHAM module. Sorry, I missed this last time, but this should be interrupt-specifier rather than interrupt number. Otherwise, this looks good to me. With the fixups above: Acked-by: Mark Rutland mark.rutl...@arm.com + +Optional properties: +- dmas: DMA specifier for the rx dma. See the DMA client binding, + Documentation/devicetree/bindings/dma/dma.txt +- dma-names: DMA request name. Should be rx if a dma is present. + +Example: + /* AM335x */ + sham: sham@5310 { + compatible = ti,omap4-sham; + ti,hwmods = sham; + reg = 0x5310 0x200; + interrupt-parent = intc; + interrupts = 109; + dmas = edma 36; + dma-names = rx; + }; diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts index 0d63348..8a9802e 100644 --- a/arch/arm/boot/dts/am335x-bone.dts +++ b/arch/arm/boot/dts/am335x-bone.dts @@ -19,3 +19,7 @@ mmc1 { vmmc-supply = ldo3_reg; }; + +sham { + status = okay; +}; diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts index 23b0a3e..d59e51c 100644 --- a/arch/arm/boot/dts/am335x-evm.dts +++ b/arch/arm/boot/dts/am335x-evm.dts @@ -522,3 +522,7 @@ status = okay; vmmc-supply = vmmc_reg; }; + +sham { + status = okay; +}; diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts index bc93895..d45a330 100644 --- a/arch/arm/boot/dts/am335x-evmsk.dts +++ b/arch/arm/boot/dts/am335x-evmsk.dts @@ -424,3 +424,7 @@ status = okay; vmmc-supply = vmmc_reg; }; + +sham { + status = okay; +}; diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index 553adc6..299710b 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -710,5 +710,14 @@ #size-cells = 1; status = disabled; }; + + sham: sham@5310 { + compatible = ti,omap4-sham; + ti,hwmods = sham; + reg = 0x5310 0x200; + interrupts = 109; + dmas = edma 36; + dma-names = rx; + }; }; }; -- 1.8.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/9] ARM: dts: Add AES data and documentation for AM33XX
On Mon, Sep 30, 2013 at 04:13:01PM +0100, Joel Fernandes wrote: From: Mark A. Greer mgr...@animalcreek.com Add the generic AM33XX AES module's device tree data and enable it for the am335x-evm, am335x-evmsk, and am335x-bone platforms. Also add Documentation file describing the data for the AES module. [jo...@ti.com: Dropped interrupt-parent propert] CC: Paul Walmsley p...@pwsan.com Signed-off-by: Mark A. Greer mgr...@animalcreek.com --- .../devicetree/bindings/crypto/omap-aes.txt| 34 ++ arch/arm/boot/dts/am335x-bone.dts | 4 +++ arch/arm/boot/dts/am335x-evm.dts | 4 +++ arch/arm/boot/dts/am335x-evmsk.dts | 4 +++ arch/arm/boot/dts/am33xx.dtsi | 10 +++ 5 files changed, 56 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/omap-aes.txt diff --git a/Documentation/devicetree/bindings/crypto/omap-aes.txt b/Documentation/devicetree/bindings/crypto/omap-aes.txt new file mode 100644 index 000..4bb1e27 --- /dev/null +++ b/Documentation/devicetree/bindings/crypto/omap-aes.txt @@ -0,0 +1,34 @@ +OMAP SoC AES crypto Module + +Required properties: + +- compatible : Should contain entries for this and backward compatible + AES versions: + - ti,omap2-aes for OMAP2. + - ti,omap3-aes for OMAP3. + - ti,omap4-aes for OMAP4 and AM33XX. + Note that the OMAP2 and 3 versions are compatible (OMAP3 supports + more algorithms) but they are incompatible with OMAP4. +- ti,hwmods: Name of the hwmod associated with the AES odule +- reg : Offset and length of the register set for the module +- interrupt-parent : the phandle for the interrupt controller that + services interrupts for this module. +- interrupts : the interrupt number for the AES odule. Similar comments to the SHAM module here: * s/interrupt number/interrupt-specifier/ * Drop interrupt-parent. * s/AES odule/AES module/ + +Optional properties: +- dmas: DMA specifier for tx and rx dma. See the DMA client binding, + Documentation/devicetree/bindings/dma/dma.txt s/DMA specifier/DMA specifiers/ +- dma-names: DMA request names. Should be 'tx, rx' if dma is present. Nit: I'd prefer 'Should include tx and rx if present' -- I hope the driver's requesting these by name rather than relying on a specific ordering (it makes future expansion and optional components far easier to handle sanely). + +Example: + /* AM335x */ + aes: aes@5350 { + compatible = ti,omap4-aes; + ti,hwmods = aes; + reg = 0x5350 0xa0; + interrupt-parent = intc; + interrupts = 102; + dmas = edma 6 + edma 5; + dma-names = tx, rx; + }; Minor nit, but for consistency could you bracket the DMAs individually: dmas = edma 6, edma 5; diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts index 8a9802e..94ee427 100644 --- a/arch/arm/boot/dts/am335x-bone.dts +++ b/arch/arm/boot/dts/am335x-bone.dts @@ -23,3 +23,7 @@ sham { status = okay; }; + +aes { + status = okay; +}; diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts index d59e51c..86463fa 100644 --- a/arch/arm/boot/dts/am335x-evm.dts +++ b/arch/arm/boot/dts/am335x-evm.dts @@ -526,3 +526,7 @@ sham { status = okay; }; + +aes { + status = okay; +}; diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts index d45a330..f577e65 100644 --- a/arch/arm/boot/dts/am335x-evmsk.dts +++ b/arch/arm/boot/dts/am335x-evmsk.dts @@ -428,3 +428,7 @@ sham { status = okay; }; + +aes { + status = okay; +}; diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index 299710b..0daa1b2 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -719,5 +719,15 @@ dmas = edma 36; dma-names = rx; }; + + aes: aes@5350 { + compatible = ti,omap4-aes; + ti,hwmods = aes; + reg = 0x5350 0xa0; + interrupts = 102; + dmas = edma 6 + edma 5; Bracketing here too, please. Cheers, Mark. + dma-names = tx, rx; + }; }; }; -- 1.8.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/9] hwspinlock/core: add common dt bindings and OF helpers
Hi Suman, Apologies for replying to a subthread, due to an earlier mistake on my part I don't have the original to hand. On Fri, Sep 27, 2013 at 05:04:22PM +0100, Kumar Gala wrote: On Sep 17, 2013, at 2:30 PM, Suman Anna wrote: All the platform-specific hwlock driver implementations need the number of locks and the associated base id for registering the locks present within a hwspinlock device with the driver core. These two variables are represented by 'hwlock-num-locks' and 'hwlock-base-id' properties. The documentation and OF helpers to retrieve these common properties have been added to the driver core. Signed-off-by: Suman Anna s-a...@ti.com --- .../devicetree/bindings/hwlock/hwlock.txt | 26 + drivers/hwspinlock/hwspinlock_core.c | 61 +- include/linux/hwspinlock.h | 11 ++-- 3 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt new file mode 100644 index 000..789930e --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt @@ -0,0 +1,26 @@ +Generic hwlock bindings +=== + +Generic bindings that are common to all the hwlock platform specific driver +implementations, the retrieved values are used for registering the device +specific parameters with the hwspinlock core. + +The validity and need of these common properties may vary from one driver +implementation to another. Look through the individual hwlock driver +binding documentations for identifying which are mandatory and which are +optional for that specific driver. + +Common properties: +- hwlock-base-id: Base Id for the locks for a particular hwlock device. + This property is used for representing a set of locks + present in a hwlock device with a unique base id in + the driver core. This property is mandatory ONLY if a + SoC has several hwlock devices. + + See documentation on struct hwspinlock_pdata in + linux/hwspinlock.h for more details. I don't like this, as it seems to be encoding a Linux implementation detail (the ID of the lock, which means that we have to have a numeric representation of each hwlock) in the device tree. I don't see why the ID within Linux should be a concern of the device tree binding. I think that whatever internal identifier we have in Linux (be it an integer or struct) should be allocated by Linux. If a driver needs to request specific hwlocks, then we should have a phandle + args representation for referring to a specific hwlock that bidnings can use, and some code for parsing that and returning a Linux-internal identifier/struct as we do with interrupts and clocks. + +- hwlock-num-locks:Number of locks present in a hwlock device. This + property is needed on hwlock devices, where the number + of supported locks within a hwlock device cannot be + read from a register. Hmm... this seems generic, but it doesn't allow for sparse ID spaces on the hwlock module. It should probably be common for the moment, but if we encounter a hwlock module with a sparse ID space, we'll need to come up with a way of handling sparse IDs (that might be device-specific). diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c index 461a0d7..aec32e7 100644 --- a/drivers/hwspinlock/hwspinlock_core.c +++ b/drivers/hwspinlock/hwspinlock_core.c @@ -1,7 +1,7 @@ /* * Hardware spinlock framework * - * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com + * Copyright (C) 2010-2013 Texas Instruments Incorporated - http://www.ti.com * * Contact: Ohad Ben-Cohen o...@wizery.com * @@ -27,6 +27,7 @@ #include linux/hwspinlock.h #include linux/pm_runtime.h #include linux/mutex.h +#include linux/of.h #include hwspinlock_internal.h @@ -308,6 +309,64 @@ out: } /** + * of_hwspin_lock_get_base_id() - OF helper to retrieve base id + * @dn: device node pointer + * + * This is an OF helper function that can be called by the underlying + * platform-specific implementations, to retrieve the base id for the + * set of locks present within a hwspinlock device instance. + * + * Returns the base id value on success, -ENODEV on generic failure or + * an appropriate error code as returned by the OF layer + */ +int of_hwspin_lock_get_base_id(struct device_node *dn) +{ + unsigned int val; + int ret = -ENODEV; + +#ifdef CONFIG_OF + if (!dn) + return -ENODEV; Checking !dn is done in of_property_read_u32, so you don't need to duplicate
Re: [PATCHv2 2/9] hwspinlock/omap: add support for dt nodes
Hi Suman, On Fri, Sep 27, 2013 at 05:06:38PM +0100, Kumar Gala wrote: On Sep 17, 2013, at 2:30 PM, Suman Anna wrote: HwSpinlock IP is present only on OMAP4 and other newer SoCs, which are all device-tree boot only. This patch adds the base support for parsing the DT nodes, and removes the code dealing with the traditional platform device instantiation. Signed-off-by: Suman Anna s-a...@ti.com --- .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 31 +++ arch/arm/mach-omap2/Makefile | 3 -- arch/arm/mach-omap2/hwspinlock.c | 60 -- drivers/hwspinlock/omap_hwspinlock.c | 23 +++-- 4 files changed, 50 insertions(+), 67 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt delete mode 100644 arch/arm/mach-omap2/hwspinlock.c diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt new file mode 100644 index 000..235b7c5 --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt @@ -0,0 +1,31 @@ +OMAP4+ HwSpinlock Driver + + +Required properties: +- compatible: Currently supports only ti,omap4-hwspinlock for + OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs Currently supports is not something I expect to see in a binding document. That sounds like a description of the driver rather than the binding. How similar are these hardware modules? What are the differences? +- reg: Contains the hwspinlock register address range (base + address and length) Is there only one register bank for the hwlock module? +- ti,hwmods: Name of the hwmod associated with the hwspinlock device + +Common hwlock properties: +The following describes the usage of the common hwlock properties (defined in +Documentation/devicetree/bindings/hwlock/hwlock.txt) on OMAP. + +- hwlock-base-id: There are currently no OMAP SoCs with multiple + hwspinlock devices. The OMAP driver uses a default + base id value of 0 for the locks present within the + single hwspinlock device on all SoCs. Driver details should not leak into bindngs... As mentioned in the other patch, I don't think this is the way to handle this. I think we need a phandle + args representation. +- hwlock-num-locks:This property is not required on OMAP SoCs, since the + number of locks present within a device can be deduced + from the SPINLOCK_SYSSTATUS device register. Huh? Why define this property at all here if we don't need it and don't use it? The common document should state that specific bindings may use it and should explicitly state if they do, rather than stating they don't... Cheers, Mark. -- 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 v6 1/4] ARM: OMAP2+: cleaned-up DT support of various ECC schemes
On Wed, Sep 25, 2013 at 10:29:03PM +0100, Brian Norris wrote: On Wed, Sep 25, 2013 at 01:33:27PM -0700, Olof Johansson wrote: On Wed, Sep 25, 2013 at 1:05 PM, Brian Norris computersforpe...@gmail.com wrote: Olof has given good advice on your DT binding and has (slowly) been responding to other requests for DT review that make it to his list. I see that he hasn't followed up on your changes (this v6), so pinging him (as you did) is probably the correct approach. But please do recognize that the DT list is very high volume, so it's hard to get good timely responses there. I am not a DT mainainer, but sometimes when I see a binding that appears to be wrong I speak up. In this case, the binding was one of those. Whoops, my bad. I was deceived by the responses I've seen from you on other issues (thanks, BTW). In that case, I haven't seen any response from a proper DT binding maintainer :( Hmm... this is the first email in this thread I've received, and I don't have older postings either. It looks like I must have cocked up subscribing to the devicetree list, but as I was CC'd directly on many patches I hadn't noticed. As far as I can see I wasn't CC'd directly on any version of this series, which would have helped to highlight the series as needing review. Apologies for that. I've attempted to correct the problem. Hopefully I've got this right and mails are not being silently dropped somewhere. Pekon, could you please re-send this version of the patches? Cheers, Mark. So, I have no more objections to it, and I hope you can get a quick review from a DT maintainer on the rest of the binding. At this point, I'm comfortable going ahead without their ack, since they obviously don't care/don't have the manpower to review. Brian -- 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 v4 1/3] usb: dwc3: msm: Add device tree binding information
On Mon, Sep 23, 2013 at 08:31:48PM +0100, Felipe Balbi wrote: Hi, On Tue, Aug 20, 2013 at 12:56:03PM +0300, Ivan T. Ivanov wrote: From: Ivan T. Ivanov iiva...@mm-sol.com MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys (SNPS) and HS, SS PHY's control and configuration registers. It could operate in device mode (SS, HS, FS) and host mode (SS, HS, FS, LS). Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com Any acks for the DT part ? This patch has been pending forever. Apologies for the delay. I have a couple of comments that would be nice to fix up now. --- .../devicetree/bindings/usb/msm-ssusb.txt | 104 1 file changed, 104 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/msm-ssusb.txt diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt b/Documentation/devicetree/bindings/usb/msm-ssusb.txt new file mode 100644 index 000..cacbd3b --- /dev/null +++ b/Documentation/devicetree/bindings/usb/msm-ssusb.txt @@ -0,0 +1,104 @@ +MSM SuperSpeed DWC3 USB SoC controller + + +DWC3 Highspeed USB PHY +== +Required properities : +- compatible : sould be qcom,dwc3-hsphy; s/sould/should/ In general, compatible properties are should contain rather than should be, as we might have backwards compatible hardware in future. +- reg : offset and length of the register set in the memory map +- clocks : phandles to clock instances of the device tree nodes Clocks are referred to be phandle + clock-specifier pairs rather than phandles, it would be nice to fix the terminology here +- clock-names : + xo : External reference clock 19 MHz + sleep_a : Sleep clock, used when USB3 core goes into low + power mode (U3). I think it would be nicer to say: - clocks : A list of phandle + clock-specifier pairs for the clocks listed in clock-names - clock-names: Should contain the following: xo - External reference clock (19MHz) sleep_a - Sleep clock used when USB3 core goes into low power mode (U3). I'm not sure we need to describe the frequency of the xo clock -- either it's a requriement of the IP and thus doesn't need to be a part of the binding, or it's configurable by the driver and thus doesn't need to be documented. +supply-name-supply : phandle to the regulator device tree node +Required supply-name are: + v1p8 : 1.8v supply for HSPHY + v3p3 : 3.3v supply for HSPHY + vbus : vbus supply for host mode + vddcx : vdd supply for HS-PHY digital circuit operation Any reason for the HSPHY/HS-PHY difference? I'd list these separately with their full names: - v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY. - v3p3-supply: phandle to the regulator for the 3.3v supply to HSPHY. - vbus-supply: phandle to the regulator for the vbus supply for host mode. - vddcx-supply: phandle to the regulator for the vdd supply for HSPHY digital circuit operation. + +DWC3 Superspeed USB PHY +=== +Required properities : +- compatible : sould be qcom,dwc3-ssphy; +- reg : offset and length of the register set in the memory map +- clocks : phandles to clock instances of the device tree nodes +- clock-names : + xo : External reference clock 19 MHz + ref : Reference clock - used in host mode. +supply-name-supply : phandle to the regulator device tree node +Required supply-name are: + v1p8 : 1.8v supply for SS-PHY + vddcx : vdd supply for SS-PHY digital circuit operation The commments on compatible, clocks, clock-names and the regulators apply here. + +DWC3 controller wrapper +=== +Required properties : +- compatible : should be qcom,dwc3 +- reg : offset and length of the register set in the memory map + offset and length of the TCSR register for routing USB + signals to either picoPHY0 or picoPHY1. +- clocks : phandles to clock instances of the device tree nodes +- clock-names : + core : Master/Core clock, have to be = 125 MHz for SS + operation and = 60MHz for HS operation + iface : System bus AXI clock + sleep : Sleep clock, used when USB3 core goes into low + power mode (U3). + utmi : Generated by HS-PHY. Used to clock the low power + parts of thr HS Link layer. +Optional properties : +- gdsc-supply : phandle to the globally distributed switch controller + regulator node to the USB controller. The commments on compatible, clocks, and clock-names apply here too. I see the regulator is defined individually :) I'm fine with the binding itself, I'd just like the documentation fixed up. Cheers, Mark. +Required child node: +A child node must exist to represent the core DWC3 IP block. The name of +the node is not important. The content of the node is defined in dwc3.txt. + +Example device nodes: + +
Re: [PATCH v6 1/4] ARM: OMAP2+: cleaned-up DT support of various ECC schemes
only matter of which path to take. Request you to please help me finalize this before I resend next patch series addressing other comments from Brian. + Mark Rutland mark.rutl...@arm.com + Pawel Moll pawel.m...@arm.com + Ian Campbell ijc+devicet...@hellion.org.uk + Stephen Warren swar...@wwwdotorg.org CC other DT maintainers, who were missed in this branch of mail-chain. with regards, pekon -- 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 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
Hi, I have a few comments, mostly on the DT binding and parsing. diff --git a/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt b/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt new file mode 100644 index 000..5d465cf --- /dev/null +++ b/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt @@ -0,0 +1,39 @@ +* IRQ CROSSBAR + +Some socs have a large number of interrupts requests to service +the needs of its many peripherals and subsystems. All of the +interrupt lines from the subsystems are not needed at the same +time, so they have to be muxed to the irq-controller appropriately. +In such places a interrupt controllers are preceded by an CROSSBAR +that provides flexibility in muxing the device requests to the controller +inputs. + +Required properties: +- compatible : Should be irq-crossbar Missing vendor prefix, this should be something like ti,irq-crossbar. Does this have a more specific name than CROSSBAR that can be used to qualify it? +- interrupt-parent: phandle to crossbar's interrupt parent. +- interrupt-controller: Identifies the node as an interrupt controller. +- interrupt-cells: Should be the same value as the interrupt parent. That doesn't make sense. The crossbar driver is necessarily interpreting these cells in a way the parent won't (as it supports more interrupts). What are the meaning of these cells? +- reg: Base address and the size of the crossbar registers. +- max-crossbar-lines: Total number of input lines of the crossbar. +- max-irqs: Total number of irqs available at the interrupt controller. Is this the maximum number of interrupts targeting the parent interrupt controller? Starting at what number, ending at what number? Can this have gaps? Is this a shortcut so in the GIC case you don't have to describe up to 160 interrupts? I can see why you don't want to, but there's a big loss of generality here... +- reg-size: size of the crossbar registers. As in the size of all the registers (the size component of reg)? Or is this the size of each individual register? Does that apply to all registers or only a subset of them? What units are these in, bytes? What are valid sizes? Is this really that configurable? +- irqs-reserved: List of the reserved irq lines that are not muxed using +crossbar. These interrupt lines are reserved in the soc, +so crossbar bar driver should not consider them as free +lines. Are these reserved inputs lines, or outputs to the parent interrupt controller? What is the format of each entry in this list? The example seems to be a different format to the parent interrupt controller (which per your binding also defined the crossbar's interrupt format). While 0 1 2 is a valid interrupt per the GIC binding (SPI 0 edge-triggered both ways), 3 5 6, 131 132 139, and 140 . . are not. + +Examples: + crossbar_mpu: @4a02 { + compatible = irq-crossbar; + interrupt-parent = gic; + interrupt-controller; + #interrupt-cells = 3; + reg = 0x4a002a48 0x130; + max-crossbar-lines = 512; + max-irqs = 160; + reg-size = 2; + irqs-reserved = 0 1 2 3 5 6 131 132 139 140; + #address-cells = 1; + #size-cells = 1; Why are there #address-cells and #size cells? This has no children, and this affects any interrupt-map property (as the parent unit address now must be a single cell, that isn't going to be used). [...] +static int crossbar_set_affinity(struct irq_data *d, +const struct cpumask *mask_val, +bool force) +{ + struct irq_chip *chip; + struct irq_data *data; + int ret = 0; + + crossbar_to_irq_chip_data(d-hwirq, chip, data); + + if (chip-irq_set_affinity) + ret = chip-irq_set_affinity(data, mask_val, force); + + return ret; So if our parent chip can't set affinity, we pretend it can? irq_set_affinity in kernel/irq/manage.c returns -EINVAL if an irqchip doesn't have irq_set_affinity. +/* + * Request and free are already called in atomic contexts + */ +unsigned int crossbar_request_irq(struct irq_data *d) +{ + int cb_no = d-hwirq; + int virq = allocate_free_irq(cb_no); + void *irq = cb-crossbar_map[cb_no].hwirq; + int err; + + err = request_threaded_irq(virq, crossbar_irq, NULL, + 0, CROSSBAR, irq); + if (err) + pr_err(\n request_irq failed for crossbar %d, cb_no); Why does the print begin with a newline rather than ending with one? + + return 0; +} [...] +static int crossbar_domain_xlate(struct irq_domain *d, +struct
Re: [PATCH v2 1/2] ARM: dts: Add SHAM data and documentation for AM33XX
On Wed, Jul 17, 2013 at 05:23:41PM +0100, Mark A. Greer wrote: From: Mark A. Greer mgr...@animalcreek.com Add the generic AM33XX SHAM module's device tree data and enable it for the am335x-evm, am335x-evmsk, and am335x-bone platforms. Also add Documentation file describing the data for the SHAM module. CC: Paul Walmsley p...@pwsan.com Signed-off-by: Mark A. Greer mgr...@animalcreek.com --- .../devicetree/bindings/crypto/omap-sham.txt | 33 ++ arch/arm/boot/dts/am335x-bone.dts | 4 +++ arch/arm/boot/dts/am335x-evm.dts | 4 +++ arch/arm/boot/dts/am335x-evmsk.dts | 4 +++ arch/arm/boot/dts/am33xx.dtsi | 10 +++ 5 files changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/omap-sham.txt diff --git a/Documentation/devicetree/bindings/crypto/omap-sham.txt b/Documentation/devicetree/bindings/crypto/omap-sham.txt new file mode 100644 index 000..c6d1202 --- /dev/null +++ b/Documentation/devicetree/bindings/crypto/omap-sham.txt @@ -0,0 +1,33 @@ +OMAP SoC SHA crypto Module + +Required properties: + +- compatible : Should contain entries for this and backward compatible + SHAM versions: + - ti,omap2-sham for OMAP2 OMAP3. + - ti,omap4-sham for OMAP4 and AM33XX. + Note that these two versions are incompatible. +- ti,hwmods: Name of the hwmod associated with the SHAM module +- reg : Offset and length of the register set for the module +- interrupt-parent : the phandle for the interrupt controller that + services interrupts for this module. +- interrupts : the interrupt number for the SHAM module. + +Optional properties: +- dmas: DMA controller phandle and DMA request ordered pair. + Only one rx pair is valid per SHAM module. This may be a little late, but... Nit: A dma specifier may have many cells, so calling it pair is not necessarily correct: dmas = dma0 432 7 5, dma1 3, dma0 212 1 13; You could instead say: - dmas: DMA specifier for the rx dma. See the DMA client binding, Documentation/devicetree/bindings/dma/dma.txt +- dma-names: DMA request name. This string corresponds 1:1 with + the ordered pair in dmas. The string naming is to be + rx for RX request. Similarly: - dma-names: DMA request name. Should be rx if a dma is present. It would be nice to get the bindings using consistent terminology so we don't confuse everyone further. Thanks, Mark. -- 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: [PATCHv5 02/31] CLK: TI: Add DPLL clock support
On Mon, Aug 19, 2013 at 02:34:45PM +0100, Tero Kristo wrote: On 08/13/2013 01:50 PM, Mark Rutland wrote: Hi, On Fri, Aug 02, 2013 at 05:25:21PM +0100, Tero Kristo wrote: The OMAP clock driver now supports DPLL clock type. This patch also adds support for DT DPLL nodes. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/clock/ti/dpll.txt | 70 +++ arch/arm/mach-omap2/clock.h| 144 +- arch/arm/mach-omap2/clock3xxx.h|2 - drivers/clk/Makefile |1 + drivers/clk/ti/Makefile|3 + drivers/clk/ti/dpll.c | 488 include/linux/clk/ti.h | 164 +++ 7 files changed, 727 insertions(+), 145 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt create mode 100644 drivers/clk/ti/Makefile create mode 100644 drivers/clk/ti/dpll.c create mode 100644 include/linux/clk/ti.h diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt b/Documentation/devicetree/bindings/clock/ti/dpll.txt new file mode 100644 index 000..dcd6e63 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt @@ -0,0 +1,70 @@ +Binding for Texas Instruments DPLL clock. + +This binding uses the common clock binding[1]. It assumes a +register-mapped DPLL with usually two selectable input clocks +(reference clock and bypass clock), with digital phase locked +loop logic for multiplying the input clock to a desired output +clock. This clock also typically supports different operation +modes (locked, low power stop etc.) This binding has several +sub-types, which effectively result in slightly different setup +for the actual DPLL clock. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of: + ti,omap4-dpll-x2-clock, + ti,omap3-dpll-clock, + ti,omap3-dpll-core-clock, + ti,omap3-dpll-per-clock, + ti,omap3-dpll-per-j-type-clock, + ti,omap4-dpll-clock, + ti,omap4-dpll-core-clock, + ti,omap4-dpll-m4xen-clock, + ti,omap4-dpll-j-type-clock, + ti,omap4-dpll-no-gate-clock, + ti,omap4-dpll-no-gate-j-type-clock, + +- #clock-cells : from common clock binding; shall be set to 0. +- clocks : link phandles of parent clocks (clk-ref and clk-bypass) It might be a good idea to use clock-names to clarify which clocks are present (I notice your examples contain only one clock input). All DPLLs have both bypass and reference clocks, but these can be the same. Is it better to just list both always (and hence drop clock-names), or provide clock-names always? If they're separate inputs, it's worth listing both (even if they're fed by the same provider). If it's possible one input might not be wired up, use clock-names. +- reg : array of register base addresses for controlling the DPLL (some + of these can also be left as NULL): + reg[0] = control register + reg[1] = idle status register + reg[2] = autoidle register + reg[3] = multiplier / divider set register Are all of these always present? Using reg-names seems sensible here. Not always, I'll change the code. I am quite new to DT and didn't actually know of the existence of reg-names prop. Ok. :) +- ti,recal-en-bit : recalibration enable bit +- ti,recal-st-bit : recalibration status bit +- ti,auto-recal-bit : auto recalibration enable bit These seem rather low-level, but I see other clock bindings are doing similar things. When are these needed, and why? What type are they? Bit shift values for the auto recalibration feature. HOWEVER, it seems these are actually legacy, kernel does not have support for these anymore if it ever had I think I can just drop these for now as they are unused by the support code even. Ok. +- ti,clkdm-name : clockdomain name for the DPLL Could you elaborate on what this is for? What constitutes a valid name? I'm not sure a string is the best way to define the linkage of several elements to a domain... Well, I am not sure if we can do this any better at this point, we don't have DT nodes for clockdomain at the moment (I am not sure if we will have those either as there are only a handful of those per SoC) but I'll add some more documentation for this. I'll have to see the documentation, but I'd be very wary of putting that in as-is. Does having the clock domain string link this up to domains in platform data? I'm not sure how well we'll be able to maintain support for that in future if it requires other platform code to use now, and we're not sure how
Re: [PATCHv5 05/31] CLK: TI: add support for OMAP gate clock
On Mon, Aug 19, 2013 at 02:42:05PM +0100, Tero Kristo wrote: On 08/13/2013 02:04 PM, Mark Rutland wrote: On Fri, Aug 02, 2013 at 05:25:24PM +0100, Tero Kristo wrote: This node adds support for a clock node which allows control to the clockdomain enable / disable. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/clock/ti/gate.txt | 41 arch/arm/mach-omap2/clock.h|9 -- drivers/clk/ti/Makefile|2 +- drivers/clk/ti/gate.c | 106 include/linux/clk/ti.h |8 ++ 5 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/gate.txt create mode 100644 drivers/clk/ti/gate.c diff --git a/Documentation/devicetree/bindings/clock/ti/gate.txt b/Documentation/devicetree/bindings/clock/ti/gate.txt new file mode 100644 index 000..620a73d --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/gate.txt @@ -0,0 +1,41 @@ +Binding for Texas Instruments gate clock. + +This binding uses the common clock binding[1]. This clock is +quite much similar to the basic gate-clock [2], however, +it supports a number of additional features. If no register +is provided for this clock, the code assumes that a clockdomain +will be controlled instead and the corresponding hw-ops for +that is used. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt +[2] Documentation/devicetree/bindings/clock/gate-clock.txt + +Required properties: +- compatible : shall be ti,gate-clock +- #clock-cells : from common clock binding; shall be set to 0 +- clocks : link to phandle of parent clock + +Optional properties: +- reg : base address for register controlling adjustable gate Optional? That's odd. If I have a clock with registers, but don't specify the register, will it still work? i.e. are registerless clocks really compatible with clocks with registers?. I think I implemented this in somewhat confusing manner. This could be split to: ti,gate-clock: requires reg and ti,enable-bit info ti,clkdm-clock: requires ti,clkdm-name clkdm clock is kind of a master clock for clockdomain, the clock is provided always if the clockdomain is active. Ok. +- ti,enable-bit : bit shift for programming the clock gate Why is this needed? Does the hardware vary wildly, or are several clocks sharing the same register(s)? Yea, same register is shared. Ok. Are those gate clocks are part of a larger gate clocks block, with the register that controls them? or are they really independent? Does the register control other items too? If they were part of some bigger unit, it might be possible to have a binding like the following (though maybe not): gate_clks: gate_clks { compatible = ti,gate-clocks-block; reg = 0x4003a000 0x1000; #clock-cells = 1; /* * has 4 gate clocks at bit shifts 0,1,2,3, * corresponding to input clocks 0,1,2,3 */ clocks = clk1 0 23, clk1 0 4, clk3 2, clk3 5; }; device { compatible = vendor,some-device; clocks = gate_clks 1; /* gate clock with bitshift 1*/ }; +- ti,dss-clk : use DSS hardware OPS for the clock +- ti,am35xx-clk : use AM35xx hardware OPS for the clock Those last two sounds like the kind of thing that should be derived from the compatible string (e.g. ti,am35xx-gate-clock). Hmm yea, I think I can change this and add some sub-types. +- ti,clkdm-name : clockdomain to control this gate As previously mentioned, I'm not a fan of this property. It would make more sense to describe the domain and have an explicit link to it (either nodes being children of the domain or having a phandle). Same comments as with patch #2. Same reply as to patch #2 :) [...] +void __init of_omap_gate_clk_setup(struct device_node *node) +{ + struct clk *clk; + struct clk_init_data init = { 0 }; + struct clk_hw_omap *clk_hw; + const char *clk_name = node-name; + int num_parents; + const char **parent_names = NULL; + int i; + u32 val; + + clk_hw = kzalloc(sizeof(*clk_hw), GFP_KERNEL); + if (!clk_hw) { + pr_err(%s: could not allocate clk_hw_omap\n, __func__); + return; + } + + clk_hw-hw.init = init; + + of_property_read_string(node, clock-output-names, clk_name); + of_property_read_string(node, ti,clkdm-name, clk_hw-clkdm_name); + + init.name = clk_name; + init.flags = 0; + + if (of_property_read_u32_index(node, reg, 0, val)) { + /* No register, clkdm control only */ + init.ops
Re: [PATCH v3 1/2] rtc: omap: update of_device_id to reflect latest ip revisions
On Fri, Aug 16, 2013 at 07:12:46PM +0100, Benoit Cousson wrote: Hi Mark, On 16/08/2013 19:20, Mark Rutland wrote: Hi Benoit, On Fri, Aug 16, 2013 at 03:15:57PM +0100, Benoit Cousson wrote: Hi Gururaja, On 16/08/2013 13:36, Hebbar, Gururaja wrote: The syntax of compatible property in DT is to mention the Most specific match to most generic match. Since AM335x is the platform with latest IP revision, add it 1st in the device id table. I don't understand why? The order should not matter at all. I've tried to follow the thread you had with Mark on the v2, but AFAIK, you've never answered to his latest question. Moreover, checking the differences between the Davinci and the am3352 RTC IP, I would not claim that both are compatible. Sure you can use the am3352 with the Davinci driver, but you will lose the wakeup functionality without even being notify about that. Could you describe the wakeup functionality, and how it differs between the am3352-rtc and the da830-rtc? AFAIK, da830-rtc does not have that functionality at all. This is something that was added to the am3352-rtc. Ok. So the am3352-rtc can be driven with the full functionality of the da830-rtc (ie. it's compatible with the da830-rtc programming model), or it can be driven as an am3352-rtc, for the OS to gain wakeup functionality in addition to the da830-rtc features. :) As I understand it, the am3352 functionality is a superset of the da830 functionality. You can use the old driver, and get some functionality, or use the new driver and get it all. Mmm, what your are saying now seems to make sense to me as well. So I'm even more confused :-) I'll convince you yet :) That means that am3352-rtc is compatible with da830. As long as the kernel first checks for am3352-rtc, there will be *no* loss of functionality. All this does is enable older kernels to use the hardware in some fashion, and given the older kernel didn't have support for the am3352-rtc features, this is a *gain* in functionality, not a loss. For my point of view, compatible mean that the HW will still be fully functional with both versions of the driver, which is not the case here. What? A driver for any entry in the compatible list should be able to drive the hardware to *some* level of functionality. We list from most-specific to most-general to allow a graceful degradation from fully supported to bare minimum functionality. OK, but where is it written in the DT spec that this is what the compatible is supposed to mean? I'm quoting it again: The compatible property value consists of one or more strings that define the specific programming model for the device. This list of strings should be used by a client program for device driver selection. The property value consists of a concatenated list of null terminated strings, from most specific to most general. They allow a device to express its compatibility with a family of similar devices, potentially allowing a single device driver to match against several devices. The graceful degradation or the loss of functionality is not something that I really understand in that text. I think it's implicit in the example that follows, where a failure to match against a specific device results in the OS falling back to a more general device. The more general device may not have all the features of a more specific device (conversely, the more general device may have more optional features that a more specific device is known not to implement). Anyway, I'm probably too tired... I'll go back home, and think about that after the week-end. Ok, let me know what you think. :) Thanks, Mark. -- 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: [PATCHv5 05/31] CLK: TI: add support for OMAP gate clock
On Mon, Aug 19, 2013 at 03:43:15PM +0100, Tero Kristo wrote: On 08/19/2013 05:29 PM, Mark Rutland wrote: On Mon, Aug 19, 2013 at 02:42:05PM +0100, Tero Kristo wrote: On 08/13/2013 02:04 PM, Mark Rutland wrote: On Fri, Aug 02, 2013 at 05:25:24PM +0100, Tero Kristo wrote: This node adds support for a clock node which allows control to the clockdomain enable / disable. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/clock/ti/gate.txt | 41 arch/arm/mach-omap2/clock.h|9 -- drivers/clk/ti/Makefile|2 +- drivers/clk/ti/gate.c | 106 include/linux/clk/ti.h |8 ++ 5 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/gate.txt create mode 100644 drivers/clk/ti/gate.c diff --git a/Documentation/devicetree/bindings/clock/ti/gate.txt b/Documentation/devicetree/bindings/clock/ti/gate.txt new file mode 100644 index 000..620a73d --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/gate.txt @@ -0,0 +1,41 @@ +Binding for Texas Instruments gate clock. + +This binding uses the common clock binding[1]. This clock is +quite much similar to the basic gate-clock [2], however, +it supports a number of additional features. If no register +is provided for this clock, the code assumes that a clockdomain +will be controlled instead and the corresponding hw-ops for +that is used. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt +[2] Documentation/devicetree/bindings/clock/gate-clock.txt + +Required properties: +- compatible : shall be ti,gate-clock +- #clock-cells : from common clock binding; shall be set to 0 +- clocks : link to phandle of parent clock + +Optional properties: +- reg : base address for register controlling adjustable gate Optional? That's odd. If I have a clock with registers, but don't specify the register, will it still work? i.e. are registerless clocks really compatible with clocks with registers?. I think I implemented this in somewhat confusing manner. This could be split to: ti,gate-clock: requires reg and ti,enable-bit info ti,clkdm-clock: requires ti,clkdm-name clkdm clock is kind of a master clock for clockdomain, the clock is provided always if the clockdomain is active. Ok. +- ti,enable-bit : bit shift for programming the clock gate Why is this needed? Does the hardware vary wildly, or are several clocks sharing the same register(s)? Yea, same register is shared. Ok. Are those gate clocks are part of a larger gate clocks block, with the register that controls them? or are they really independent? Does the register control other items too? Not really. Typically they only have the clockdomain in common, and the individual clocks are mostly controlled independently from each other. For example on omap3 we have following register: You say they typically only have the clockdomain in common. Do you mean that they always have the same clockdomain, but not necessarily other properties, or may they not even have the clockdomain in common? CM_FCLKEN_PER Physical Address 0x4800 5000 BIT31:19 RESERVED Write 0s for future compatibility. Read returns 0. BIT18 EN_UART4 UART4 functional clock control. BIT17 EN_GPIO6 GPIO6 functional clock control. BIT16 EN_GPIO5 GPIO5 functional clock control. BIT15 EN_GPIO4 GPIO4 functional clock control. BIT14 EN_GPIO3 GPIO3 functional clock control. BIT13 EN_GPIO2 GPIO2 functional clock control. BIT12 EN_WDT3 WDT3 functional clock control. BIT11 EN_UART3 Type UART3 functional clock control. BIT10 EN_GPT9 GPTIMER 9 functional clock control. BIT9 EN_GPT8 GPTIMER 8 functional clock control. BIT8 EN_GPT7 GPTIMER 7 functional clock control. BIT7 EN_GPT6 GPTIMER 6 functional clock control. BIT6 EN_GPT5 GPTIMER 5 functional clock control. BIT5 EN_GPT4 GPTIMER 4 functional clock control. BIT4 EN_GPT3GPTIMER 3 functional clock control. BIT3 EN_GPT2 GPTIMER 2 functional clock control. BIT2 EN_MCBSP4 McBSP 4 functional clock control. BIT1 EN_MCBSP3 McBSP3 functional clock control. BIT0 EN_MCBSP2 McBSP 2 functional clock control. So multiple drivers will be using this register for example. The point I was trying to get across is that this looks like a single logical block which controls the (independent) gating of several clocks, along the same lines as multiple swtiches bound together in a DIP switch. It's equally valid to view that as several clock gates which happen to have their control bits in close proximity in the memory map, as you suggest. Thanks, Mark. -- 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
Re: [PATCHv5 02/31] CLK: TI: Add DPLL clock support
On Mon, Aug 19, 2013 at 04:09:37PM +0100, Tero Kristo wrote: On 08/19/2013 05:18 PM, Mark Rutland wrote: On Mon, Aug 19, 2013 at 02:34:45PM +0100, Tero Kristo wrote: On 08/13/2013 01:50 PM, Mark Rutland wrote: Hi, On Fri, Aug 02, 2013 at 05:25:21PM +0100, Tero Kristo wrote: The OMAP clock driver now supports DPLL clock type. This patch also adds support for DT DPLL nodes. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/clock/ti/dpll.txt | 70 +++ arch/arm/mach-omap2/clock.h| 144 +- arch/arm/mach-omap2/clock3xxx.h|2 - drivers/clk/Makefile |1 + drivers/clk/ti/Makefile|3 + drivers/clk/ti/dpll.c | 488 include/linux/clk/ti.h | 164 +++ 7 files changed, 727 insertions(+), 145 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt create mode 100644 drivers/clk/ti/Makefile create mode 100644 drivers/clk/ti/dpll.c create mode 100644 include/linux/clk/ti.h diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt b/Documentation/devicetree/bindings/clock/ti/dpll.txt new file mode 100644 index 000..dcd6e63 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt @@ -0,0 +1,70 @@ +Binding for Texas Instruments DPLL clock. + +This binding uses the common clock binding[1]. It assumes a +register-mapped DPLL with usually two selectable input clocks +(reference clock and bypass clock), with digital phase locked +loop logic for multiplying the input clock to a desired output +clock. This clock also typically supports different operation +modes (locked, low power stop etc.) This binding has several +sub-types, which effectively result in slightly different setup +for the actual DPLL clock. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of: + ti,omap4-dpll-x2-clock, + ti,omap3-dpll-clock, + ti,omap3-dpll-core-clock, + ti,omap3-dpll-per-clock, + ti,omap3-dpll-per-j-type-clock, + ti,omap4-dpll-clock, + ti,omap4-dpll-core-clock, + ti,omap4-dpll-m4xen-clock, + ti,omap4-dpll-j-type-clock, + ti,omap4-dpll-no-gate-clock, + ti,omap4-dpll-no-gate-j-type-clock, + +- #clock-cells : from common clock binding; shall be set to 0. +- clocks : link phandles of parent clocks (clk-ref and clk-bypass) It might be a good idea to use clock-names to clarify which clocks are present (I notice your examples contain only one clock input). All DPLLs have both bypass and reference clocks, but these can be the same. Is it better to just list both always (and hence drop clock-names), or provide clock-names always? If they're separate inputs, it's worth listing both (even if they're fed by the same provider). If it's possible one input might not be wired up, use clock-names. Ref and bypass clocks are used currently by all DPLLs (also the APLL) so I guess I just enforce both to be specified. Ok. It's always possible to add clock-names later if a platform doesn't wire an input. We lose the possibility of future compatibility, but backwards compatibility is easy enough to maintain. +- ti,clkdm-name : clockdomain name for the DPLL Could you elaborate on what this is for? What constitutes a valid name? I'm not sure a string is the best way to define the linkage of several elements to a domain... Well, I am not sure if we can do this any better at this point, we don't have DT nodes for clockdomain at the moment (I am not sure if we will have those either as there are only a handful of those per SoC) but I'll add some more documentation for this. I'll have to see the documentation, but I'd be very wary of putting that in as-is. Does having the clock domain string link this up to domains in platform data? I'm not sure how well we'll be able to maintain support for that in future if it requires other platform code to use now, and we're not sure how the domains themselves will be represented in dt. Hmm so, should I add a stub representation for the clockdomains to the DT then for now or how should this be handled? The stubs could then be mapped to the actual clock domains based on their node names. I unfortunately don't have a good answer here, because I'm not that familiar with how we handle clockdomains for power management purposes. As I understand it, each clock domain is essentially a clock gate controlling multiple clock signals, so it's possible to describe that with the common clock bindings, with a domain's clocks
Re: [PATCH v3 1/2] rtc: omap: update of_device_id to reflect latest ip revisions
Hi Benoit, On Fri, Aug 16, 2013 at 03:15:57PM +0100, Benoit Cousson wrote: Hi Gururaja, On 16/08/2013 13:36, Hebbar, Gururaja wrote: The syntax of compatible property in DT is to mention the Most specific match to most generic match. Since AM335x is the platform with latest IP revision, add it 1st in the device id table. I don't understand why? The order should not matter at all. I've tried to follow the thread you had with Mark on the v2, but AFAIK, you've never answered to his latest question. Moreover, checking the differences between the Davinci and the am3352 RTC IP, I would not claim that both are compatible. Sure you can use the am3352 with the Davinci driver, but you will lose the wakeup functionality without even being notify about that. Could you describe the wakeup functionality, and how it differs between the am3352-rtc and the da830-rtc? As I understand it, the am3352 functionality is a superset of the da830 functionality. You can use the old driver, and get some functionality, or use the new driver and get it all. That means that am3352-rtc is compatible with da830. As long as the kernel first checks for am3352-rtc, there will be *no* loss of functionality. All this does is enable older kernels to use the hardware in some fashion, and given the older kernel didn't have support for the am3352-rtc features, this is a *gain* in functionality, not a loss. For my point of view, compatible mean that the HW will still be fully functional with both versions of the driver, which is not the case here. What? A driver for any entry in the compatible list should be able to drive the hardware to *some* level of functionality. We list from most-specific to most-general to allow a graceful degradation from fully supported to bare minimum functionality. am3352 DTS must use the ti,am3352-rtc to have the expected behavior. Using the ti,da830-rtc version will not make the board working as expected. So we cannot claim the compatibility. Please can you explain what you mean by expected behaviour? This way, we can add new matching compatible as 1st and maintain old compatible string for backwards compatibility. ex: compatible = ti,am3352-rtc, ti,da830-rtc; Signed-off-by: Hebbar, Gururaja gururaja.heb...@ti.com CC: mark.rutl...@arm.com --- Changes in v3: - new patch :100644 100644 dc62cc3... 2f0968c... M drivers/rtc/rtc-omap.c drivers/rtc/rtc-omap.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c index dc62cc3..2f0968c 100644 --- a/drivers/rtc/rtc-omap.c +++ b/drivers/rtc/rtc-omap.c @@ -330,12 +330,12 @@ static struct platform_device_id omap_rtc_devtype[] = { MODULE_DEVICE_TABLE(platform, omap_rtc_devtype); static const struct of_device_id omap_rtc_of_match[] = { - { .compatible = ti,da830-rtc, - .data = omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX], - }, { .compatible = ti,am3352-rtc, .data = omap_rtc_devtype[OMAP_RTC_DATA_AM335X_IDX], }, + { .compatible = ti,da830-rtc, + .data = omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX], + }, {}, }; MODULE_DEVICE_TABLE(of, omap_rtc_of_match); Bottom-line, if you get rid of the old compatible entry, you will not have to play some trick with the entries order. I don't think it's playing tricks. It's ensuring compatibility, and it's a simple change in ordering. Thanks, Mark. -- 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 v8 0/2] TWL6030, TWL6032 GPADC driver
Hi, apologies for the late reply. On Thu, Jul 25, 2013 at 02:26:51PM +0100, Oleksandr Kozaruk wrote: Hello, v8 - removed unused test channels completely, removed die temperature channels, as it is not known how to convert ADC code to temperature. There if formula for twl6030, but no formula for twl6032. v7 - addressed clean up comments, removed test channels v6 - addressed comments about trim bits, checkpatch clean up v5 - gpadc DT node renamed from gpadc to generic adc, added temperature channels; raw code is corracted with calibration data. v4 - addressed comments: fixed style violation, bug in freeing memory, added comments explaining calibration method, removed test network channels from exposing to userspace, error handling for wait_for_complition v3 - fixed compiler warning v2 - the driver put in drivers/iio, and converted using iio facilities as suggested by Graeme. TWL603[02] GPADC is used to measure battery voltage, battery temperature, battery presence ID, and could be used to measure twl603[02] die temperature. This is used on TI blaze, blaze tablet platforms. The TWL6030/TWL6032 is a PMIC that has a GPADC with 17/19 channels respectively. Some channels have current source and are used for measuring voltage drop on resistive load for detecting battery ID resistance, or measuring voltage drop on NTC resistors for external temperature measurements, other channels measure voltage, (i.e. battery voltage), and have inbuilt voltage dividers, thus, capable to scale voltage. Some channels are dedicated for measuring die temperature. Some channels could be calibrated in 2 points, having offsets from ideal values in trim registers. The difference between GPADC in TWL6030 and TWL6032: - 10 bit vs 12 bit ADC; - 17 vs 19 channels; - channels have different purpose(i. e. battery voltage channel 8 vs channel 18); - trim values are interpreted differently. The driver is derived from git://git.omapzoom.org/kernel/omap.git The original driver's authors and contributors are Balaji T K, Graeme Gregory, Ambresh K, Girish S Ghongdemath. The changes to the original driver: - device tree adaptation; I couldn't see a binding document in this series or in mainline. Have I looked in the wrong places? Mark. -- 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 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT
[Adding Mike Turquette and dt maintainers to Cc] On Tue, Jul 23, 2013 at 07:24:38AM +0100, Rajendra Nayak wrote: With clocks for OMAP moving to DT, its now possible to pass all optional clock data for each device from DT instead of having it in hwmod. Signed-off-by: Rajendra Nayak rna...@ti.com --- arch/arm/mach-omap2/omap_hwmod.c | 66 -- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 12fa589..e5c804b 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh) return ret; } +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh, +struct device_node *np, +int *opt_clks_cnt) +{ + int i, clks_cnt; + const char *clk_name; + const char **opt_clk_names; + + clks_cnt = of_property_count_strings(np, clock-names); + if (!clks_cnt) + return NULL; + + opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL); + if (!opt_clk_names) + return NULL; + + for (i = 0; i clks_cnt; i++) { + of_property_read_string_index(np, clock-names, i, clk_name); + if (!strcmp(clk_name, fck)) + continue; + opt_clks_cnt++; + opt_clk_names[i] = clk_name; + } + return opt_clk_names; +} + +static int _init_opt_clks_dt(struct omap_hwmod *oh, struct device_node *np) +{ + struct clk *c; + int i, opt_clks_cnt = 0; + int ret = 0; + const char **opt_clk_names; + + opt_clk_names = _parse_opt_clks_dt(oh, np, opt_clks_cnt); + if (!opt_clk_names) + return -EINVAL; + + oh-opt_clks = kzalloc(sizeof(struct omap_hwmod_opt_clk *) +* opt_clks_cnt, GFP_KERNEL); + if (!oh-opt_clks) + return -ENOMEM; + + oh-opt_clks_cnt = opt_clks_cnt; + + for (i = 0; i oh-opt_clks_cnt; i++) { + c = of_clk_get_by_name(np, opt_clk_names[i]); + if (IS_ERR(c)) { + pr_warn(omap_hwmod: %s: cannot clk_get opt_clk %s\n, + oh-name, opt_clk_names[i]); + ret = -EINVAL; + } + oh-opt_clks[i]._clk = c; + oh-opt_clks[i].role = opt_clk_names[i]; + oh-opt_clks_cnt++; + clk_prepare(oh-opt_clks[i]._clk); + } + return ret; +} I don't like this. clock-names is used to represent the names of clocks as inputs to the device. The driver must know the names of each and every one of the clock inputs it intends to use -- there's a finite number, and if it doesn't know about it it clearly has no idea how that clock's meant to be used. Consider a future revision of the hardware that has an additional clock input. Some new feature may require that clock, but your driver won't support that new feature, so you don't need it. Preparing that clock is a waste of power, and could cause issues if for some reason the clock was mutually exlcusive with another clock (so preparing it would make the hardware unusable). If the new revision *requires* that clock to provide the same interface otherwise, it's not backwards compatible and needs a new binding, and the driver needs to be extended to support it. Given that, preparing all the clocks you've been handed is a hack. Simply request by name the ones you know you need, and attempt to request the optional ones as necessary. Don't blindly go and prepare everything. Thanks, Mark. -- 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 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT
[Adding Mike Turquette and dt maintainers] On Wed, Aug 14, 2013 at 02:39:44PM +0100, Nishanth Menon wrote: On 08/14/2013 08:20 AM, Rajendra Nayak wrote: On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote: Hi Rajendra, On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak rna...@ti.com wrote: [..] diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 12fa589..e5c804b 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh) return ret; } +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh, + struct device_node *np, + int *opt_clks_cnt) +{ + int i, clks_cnt; + const char *clk_name; + const char **opt_clk_names; + + clks_cnt = of_property_count_strings(np, clock-names); + if (!clks_cnt) + return NULL; + + opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL); + if (!opt_clk_names) + return NULL; + + for (i = 0; i clks_cnt; i++) { + of_property_read_string_index(np, clock-names, i, clk_name); + if (!strcmp(clk_name, fck)) Could we instead parse for names that are optional,role_name instead of assuming anything other than fck is optional clocks? you mean look for anything with optional,*? because the role names would change. yes. the idea being, we now have a meaning to the clock name - there are two types of clocks here.. functional and optional, we *might* have facility to add interface clock(we dont know interface clock handling yet, but something in the future).. we might increase the support for number of functional clocks.. it might help to keep the format such that it is a bit extendable. I completely disagree. The only things that should appear in clock-names are the names of the clock inputs that appear in the manual for the device. The driver should know which ones are optional, as that's a fixed property of the IP and the way the driver uses it. You should not be embedding additional semantics in name properties. Thanks, Mark. -- 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 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT
On Wed, Aug 14, 2013 at 02:54:57PM +0100, Rajendra Nayak wrote: On Wednesday 14 August 2013 07:15 PM, Mark Rutland wrote: [Adding Mike Turquette and dt maintainers to Cc] On Tue, Jul 23, 2013 at 07:24:38AM +0100, Rajendra Nayak wrote: With clocks for OMAP moving to DT, its now possible to pass all optional clock data for each device from DT instead of having it in hwmod. Signed-off-by: Rajendra Nayak rna...@ti.com --- arch/arm/mach-omap2/omap_hwmod.c | 66 -- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 12fa589..e5c804b 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh) return ret; } +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh, + struct device_node *np, + int *opt_clks_cnt) +{ + int i, clks_cnt; + const char *clk_name; + const char **opt_clk_names; + + clks_cnt = of_property_count_strings(np, clock-names); + if (!clks_cnt) + return NULL; + + opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL); + if (!opt_clk_names) + return NULL; + + for (i = 0; i clks_cnt; i++) { + of_property_read_string_index(np, clock-names, i, clk_name); + if (!strcmp(clk_name, fck)) + continue; + opt_clks_cnt++; + opt_clk_names[i] = clk_name; + } + return opt_clk_names; +} + +static int _init_opt_clks_dt(struct omap_hwmod *oh, struct device_node *np) +{ + struct clk *c; + int i, opt_clks_cnt = 0; + int ret = 0; + const char **opt_clk_names; + + opt_clk_names = _parse_opt_clks_dt(oh, np, opt_clks_cnt); + if (!opt_clk_names) + return -EINVAL; + + oh-opt_clks = kzalloc(sizeof(struct omap_hwmod_opt_clk *) + * opt_clks_cnt, GFP_KERNEL); + if (!oh-opt_clks) + return -ENOMEM; + + oh-opt_clks_cnt = opt_clks_cnt; + + for (i = 0; i oh-opt_clks_cnt; i++) { + c = of_clk_get_by_name(np, opt_clk_names[i]); + if (IS_ERR(c)) { + pr_warn(omap_hwmod: %s: cannot clk_get opt_clk %s\n, + oh-name, opt_clk_names[i]); + ret = -EINVAL; + } + oh-opt_clks[i]._clk = c; + oh-opt_clks[i].role = opt_clk_names[i]; + oh-opt_clks_cnt++; + clk_prepare(oh-opt_clks[i]._clk); + } + return ret; +} I don't like this. clock-names is used to represent the names of clocks as inputs to the device. The driver must know the names of each and every one of the clock inputs it intends to use -- there's a finite number, and if it doesn't know about it it clearly has no idea how that clock's meant to be used. Consider a future revision of the hardware that has an additional clock input. Some new feature may require that clock, but your driver won't support that new feature, so you don't need it. Preparing that clock is a waste of power, and could cause issues if for some reason the clock was mutually exlcusive with another clock (so preparing it would make the hardware unusable). If the new revision *requires* that clock to provide the same interface otherwise, it's not backwards compatible and needs a new binding, and the driver needs to be extended to support it. Given that, preparing all the clocks you've been handed is a hack. Mark, this is a piece of platform code (hwmod framework for omap) which does a enable/reset/idle of all devices on the SoC early at boot to get rid of bootloader dependencies. This isn;t something used by the drivers when they enable the devices. I don't see any issue with 'waste of power'. The framework (unlike the driver) has no knowledge of what clocks are needed and hence does a enable all momentarily to reset and put the device in a known state. Ok, I'd misunderstood there. Apologies for the noise. Thanks, Mark. -- 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