Re: [RFC PATCH 1/3] of/device: manage resources similar to platform_device_add
On Thu, 22 Jan 2015 15:49:59 -0600 , Suman Anna s-a...@ti.com wrote: Hi Grant, On 01/13/2015 05:04 PM, Suman Anna wrote: On 01/13/2015 04:00 PM, Rob Herring wrote: On Tue, Jan 13, 2015 at 3:25 PM, Suman Anna s-a...@ti.com wrote: Hi Rob, On 01/13/2015 02:38 PM, Rob Herring wrote: On Wed, Jan 7, 2015 at 11:30 AM, Suman Anna s-a...@ti.com wrote: Drivers can use of_platform_populate() to create platform devices for children of the device main node, and a complementary API of_platform_depopulate() is provided to delete these child platform devices. The of_platform_depopulate() leverages the platform API for performing the cleanup of these devices. The platform device resources are managed differently between of_device_add and platform_device_add, and this asymmetry causes a kernel oops in platform_device_del during removal of the resources. Manage the platform device resources similar to platform_device_add to fix this kernel oops. This is a known issue and has been attempted to be fixed before (I believe there is a revert in mainline). The problem is there are known devicetrees which have overlapping resources and they will break with your change. Are you referring to 02bbde7849e6 (Revert of: use platform_device_add)? I believe that's the one. That one seems to be in registration path, and this crash is in the unregistration path. If so, to fix the crash, should we be skipping the release_resource() for now in platform_device_del for DT nodes, or replace platform_device_unregister with of_device_unregister in of_platform_device_destroy()? IIRC, the problem is inserting a resource twice on add from 2 different nodes, not the removal path. Perhaps we could make a collision non-fatal for in the DT case. We may be talking two different things here, I understand that this patch would create an issue with inserting a resource twice in the devicetrees with overlapping resources (just like the commit that was reverted above), but the crash is on devices with resources whose parent, child, sibling pointers have never been initialized (the of_device_add path does not touch these at all), and get dereferenced in platform_device_del()-release_resource(). See the following that has a better explanation [1]. regards Suman [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/274412.html Grant may have some ideas on what's needed here. Ping, any suggestions here? Do we ought to replace platform_device_unregister() with of_device_unregister() similar to the approach taken in 02bbde7849e6 (Revert of: use platform_device_add)? Hi Suman, Yes, I think the solution to both problems is to create an of_device_unregister() function. It's not the prettiest thing, but I think it is for the best. g. -- 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 v2 1/4] regulator: Add helper function to get poweroff-source property
On Fri, Oct 10, 2014 at 1:29 PM, PERIER Romain romain.per...@gmail.com wrote: What I'm more concerned about is the semantics of the property. What do the generic code paths gain by standardizing this property? Is it expected that We really need to come up with a standard property for this and document it rather than continuing to add individual device specific properties each time a driver adds poweroff capability, all doing the same thing (a lot of regulators driver, mfd drivers, soc specific drivers, power drivers already do that, that's very redudant) . This is a simple unification logic. Yes, it's fine. I accidentally left that paragraph in when I sent my reply. I started writing that concern, and then thought better of it. The property is fine. g. -- 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 v2 1/4] regulator: Add helper function to get poweroff-source property
On Tue, Oct 7, 2014 at 8:43 PM, PERIER Romain romain.per...@gmail.com wrote:
This is not the final location, I have no idea exactly where I might
put this helper function. This is why I ask you your opinion (and
also, this is why that's a proposal)
2014-10-07 21:45 GMT+02:00 Romain Perier romain.per...@gmail.com:
Several drivers create their own devicetree property when they register
poweroff capabilities. This is for example the case for mfd, regulator
or power drivers which define vendor,system-power-controller property.
This patch adds support for a standard property poweroff-source
which marks the device as able to shutdown the system.
Signed-off-by: Romain Perier romain.per...@gmail.com
---
drivers/regulator/of_regulator.c | 12
include/linux/regulator/of_regulator.h | 6 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/regulator/of_regulator.c
b/drivers/regulator/of_regulator.c
index 7a51814..8b898e6 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -240,3 +240,15 @@ struct regulator_init_data
*regulator_of_get_init_data(struct device *dev,
return init_data;
}
+
+/**
+ * is_system_poweroff_source - Tells if poweroff-source is found for
device_node
+ * @np: Pointer to the given device_node
+ *
+ * return true if present false otherwise
+ */
+bool is_system_poweroff_source(const struct device_node *np)
+{
+ return of_property_read_bool(np, poweroff-source);
+}
+EXPORT_SYMBOL_GPL(is_system_poweroff_source);
Hi Romain,
This is an extremely simple wrapper. It may be best to simply make it
a static inline. It is also generic enough that I don't have a problem
with it living in include/linux/of.h; but of_regulator.h would be fine
too.
Since this is a device tree specific function (it doesn't work with
ACPI or platform_data), you should prefix the function name with of_
g.
What I'm more concerned about is the semantics of the property. What
do the generic code paths gain by standardizing this property? Is it
expected that
diff --git a/include/linux/regulator/of_regulator.h
b/include/linux/regulator/of_regulator.h
index f921796..9d8fbb2 100644
--- a/include/linux/regulator/of_regulator.h
+++ b/include/linux/regulator/of_regulator.h
@@ -20,6 +20,7 @@ extern struct regulator_init_data
extern int of_regulator_match(struct device *dev, struct device_node *node,
struct of_regulator_match *matches,
unsigned int num_matches);
+extern bool is_system_poweroff_source(const struct device_node *np);
#else
static inline struct regulator_init_data
*of_get_regulator_init_data(struct device *dev,
@@ -35,6 +36,11 @@ static inline int of_regulator_match(struct device *dev,
{
return 0;
}
+
+static inline bool is_system_poweroff_source(const struct device_node *np)
+{
+ return false;
+}
#endif /* CONFIG_OF */
#endif /* __LINUX_OF_REG_H */
--
1.9.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: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Use clkops-clocks property to specify clocks handled by
clock_ops domain PM domain. Only clocks defined in clkops-clocks
set of clocks will be handled by Runtime PM through clock_ops
Pm domain.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/of/of_clk.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c
index 35f5e9f..5f9b90e 100644
--- a/drivers/of/of_clk.c
+++ b/drivers/of/of_clk.c
@@ -86,11 +86,8 @@ int of_clk_register_runtime_pm_clocks(struct device_node
*np,
struct clk *clk;
int error;
- for (i = 0; (clk = of_clk_get(np, i)) !IS_ERR(clk); i++) {
- if (!clk_may_runtime_pm(clk)) {
- clk_put(clk);
- continue;
- }
+ for (i = 0; (clk = of_clk_get_from_set(np, clkops, i))
+ !IS_ERR(clk); i++) {
This really looks like an ABI break to me. What happens to all the
existing platforms who don't have this new clkops-clocks in their device
tree?
g.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
On Mon, Jul 28, 2014 at 11:47 AM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Hi Grant.
On 07/28/2014 05:05 PM, Grant Likely wrote:
On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Use clkops-clocks property to specify clocks handled by
clock_ops domain PM domain. Only clocks defined in clkops-clocks
set of clocks will be handled by Runtime PM through clock_ops
Pm domain.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/of/of_clk.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c
index 35f5e9f..5f9b90e 100644
--- a/drivers/of/of_clk.c
+++ b/drivers/of/of_clk.c
@@ -86,11 +86,8 @@ int of_clk_register_runtime_pm_clocks(struct device_node
*np,
struct clk *clk;
int error;
-for (i = 0; (clk = of_clk_get(np, i)) !IS_ERR(clk); i++) {
-if (!clk_may_runtime_pm(clk)) {
-clk_put(clk);
-continue;
-}
+for (i = 0; (clk = of_clk_get_from_set(np, clkops, i))
+ !IS_ERR(clk); i++) {
This really looks like an ABI break to me. What happens to all the
existing platforms who don't have this new clkops-clocks in their device
tree?
Agree. This patch as is will break such platforms.
As possible solution for above problem - the NULL can be used as clock's
prefix
by default and platform code can configure new value of clock's prefix during
initialization.
In addition, to make this solution full the of_clk_get_by_name() will
need to be modified too.
But note pls, this is pure RFC patches which I did to find out the answer on
questions:
- What is better: maintain Runtime PM clocks configuration in DT or in code?
In code. I don't think it is workable to embed runtime PM behaviour
into the DT bindings. I think there will be too much variance in what
hardware requires. We can create helpers to make this simpler, but I
don't think it is a good idea to set it up automatically without any
control from the driver itself.
- Where and when to call of_clk_register_runtime_pm_clocks()?
Bus notifier/ platform core/ device drivers
I would say in device drivers.
Also, May be platform dependent solution [1] can be acceptable for now?
[1] https://lkml.org/lkml/2014/7/25/630
I need to look at the series before I comment. I've flagged it and
will hopefully look at it tomorrow.
g.
--
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: Unifying cape overlays into boot .dtb for BeagleBoard.org boards
On Tue, 17 Jun 2014 10:09:31 +0100, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Mon, Jun 16, 2014 at 09:22:50AM -0400, Jason Kridner wrote: Adding devicetree and linux-arm-kernel lists based on feedback on IRC... On Tue, Jun 10, 2014 at 12:46 PM, Jason Kridner jkrid...@gmail.com wrote: I'd like to discuss moving our current library of cape devicetree overlay sources into a single tree, including the boot .dtb files for BeagleBoard.org boards and moving towards enabling as much of the cape support into a single boot-time .dtb file with an approach similar to the cape-universal overlay (https://github.com/cdsteinkuehler/beaglebone-universal-io), but not in an overlay. First of all, I want to note this doesn't change my view on the importance of mainline support for devicetree overlays. They are still absolutely critical and highly useful, solving problems that cannot be solved through boot-time devicetrees. I'm simply looking for an approach that will complement the availability of overlays and provide the best user experience. Here's the most obvious question in the world on this topic. Are capes hot-pluggable? Looking at the posts on google+ from David Anders, they're using pin headers for connectivity, with no additional protection against hot- plugging, and no sequencing of pin connection. In other words, they are not hot-pluggable. So, why do we need to add a load of infrastructure to the kernel to allow the device tree to be modified at run time? At present, the way the entire DT infrastructure works is that it assumes the DT remains static and never changes - this applies not only to the core DT code, but also to all the drivers which have been converted. As others have pointed out, capes aren't the only use case. pseries already modifies the tree at runtime, and the FPGA users want the ability to add/remove additional DT blocks. I've also heard from hobbiest/maker developers that by deferring the load of additional data to userspace means they don't need to mess with the boot path once it is working. The feature is coming. g. -- 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/RFC 3/4] of/clk: Register clocks suitable for Runtime PM with the PM core
On Wed, 30 Apr 2014 23:54:37 +0200, Geert Uytterhoeven ge...@linux-m68k.org
wrote:
Hi Grant,
On Tue, Apr 29, 2014 at 3:16 PM, Grant Likely grant.lik...@secretlab.ca
wrote:
On Fri, 25 Apr 2014 16:44:58 -0700, Kevin Hilman khil...@linaro.org wrote:
Geert Uytterhoeven geert+rene...@glider.be writes:
When adding a device from DT, check if its clocks are suitable for
Runtime
PM, and register them with the PM core.
If Runtime PM is disabled, just enable the clock.
This allows the PM core to automatically manage gate clocks of devices
for
Runtime PM.
...unless the device is already in an existing pm_domain, right?
I like this approach, and it extends nicely what we already do on
platforms using drivers/base/power/clock_ops.c into DT land.
My only concern is how this will interact if it's used along with
devices that have existing pm_domains. I don't have any specific
concerns (yet, because it's Friday, and my brain is turing off), but it
just made me wonder if this will be potentially confusing.
I have big concerns about this approach. First, it will only work if
a clock is available at deivce creation time. The conversion of irq
controllers to normal device drivers has already shown that is a bad
idea.
That's indeed a valid concern that needs to be addressed.
I also don't like that it tries to set up every clock, but there is no
guarantee that the driver will even use it. I would rather see this
behaviour linked into the function that obtains the clock at driver
.probe() time. That way it can handle deferred probe correctly and it
only sets up clocks that are actually used by the driver.
Not every clock. Only the clocks that are advertised by the clock driver as
being suitable for runtime_pm management. These are typically module
clocks, that must be enabled for the module to work. The driver doesn't
always want to handle these explicitly.
Help me out here becasue I don't understand how that works with this
patch set. From my, admittedly naive, reading it looks like the setup is
being done at device creation time, but if the driver (or module) gets
to declare which clocks need to be enabled in order to work, then that
information is not available at device creation time.
In fact we have one case on shmobile where the module clock for an IP
core (rcar-gpio) is enabled unconditionally in one SoC, while it became
controllable through a gate clock in a later SoC.
With my patch, just adding the clock to the DT node is sufficient to make
it work.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like
that.
-- Linus Torvalds
--
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/RFC 3/4] of/clk: Register clocks suitable for Runtime PM with the PM core
On Thu, May 1, 2014 at 2:41 PM, Geert Uytterhoeven ge...@linux-m68k.org wrote: Hi Grant, On Thu, May 1, 2014 at 10:03 AM, Grant Likely grant.lik...@secretlab.ca wrote: On Wed, 30 Apr 2014 23:54:37 +0200, Geert Uytterhoeven ge...@linux-m68k.org wrote: On Tue, Apr 29, 2014 at 3:16 PM, Grant Likely grant.lik...@secretlab.ca wrote: I also don't like that it tries to set up every clock, but there is no guarantee that the driver will even use it. I would rather see this behaviour linked into the function that obtains the clock at driver .probe() time. That way it can handle deferred probe correctly and it only sets up clocks that are actually used by the driver. Not every clock. Only the clocks that are advertised by the clock driver as being suitable for runtime_pm management. These are typically module clocks, that must be enabled for the module to work. The driver doesn't always want to handle these explicitly. Help me out here becasue I don't understand how that works with this patch set. From my, admittedly naive, reading it looks like the setup is being done at device creation time, but if the driver (or module) gets to declare which clocks need to be enabled in order to work, then that information is not available at device creation time. Setup is indeed done at registration time. Note the check calling clk_may_runtime_pm(), which is introduced in [PATCH/RFC 1/4] clk: Add CLK_RUNTIME_PM and clk_may_runtime_pm(). Clock drivers are initialized much earlier, so they can set the CLK_RUNTIME_PM flag for suitable clocks before platform devices are created from DT, cfr. the example for shmobile MSTP clocks in [PATCH/RFC 4/4] clk: shmobile: mstp: Set CLK_RUNTIME_PM flag. This is where I have issue. You're *assuming* clock drivers are initialized much earlier. That is not guaranteed. It is perfectly valid for clocks to be set up by a normal device driver, just like for interrupt controllers or gpio controllers. g. -- 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 51/97] ARM: l2c: remove platforms/SoCs setting early BRESP
On Tue, 29 Apr 2014 01:21:41 +0100, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Apr 29, 2014 at 09:02:27AM +0900, Simon Horman wrote: On Mon, Apr 28, 2014 at 08:30:32PM +0100, Russell King wrote: Since we now automatically enable early BRESP in core L2C-310 code when we detect a Cortex-A9, we don't need platforms/SoCs to set this bit explicitly. Instead, they should seek to preserve the value of bit 30 in the auxiliary control register. Acked-by: Tony Lindgren t...@atomide.com Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk I would prefer if this patch was broken out into individual patches for each board or SoC file and that they were then picked up by their respective platform maintainers. Likewise for patch 66/97. Although it is only for shmobile I would prefer it broken out. Oh fuck that. Okay, I'm dropping the whole patch set right now and forgetting the whole damned thing. The L2 cache code can damned well stay as it is and remain an unmaintainable mess. FWIW, there are an awful lot of people, myself included, who do care that you've done this work. It is 100% okay for you to say no to requests to split things up because of the complexity of the series. I really hope you're reconsider and not give up on this series. g. -- 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/RFC 3/4] of/clk: Register clocks suitable for Runtime PM with the PM core
On Fri, 25 Apr 2014 16:44:58 -0700, Kevin Hilman khil...@linaro.org wrote: Geert Uytterhoeven geert+rene...@glider.be writes: When adding a device from DT, check if its clocks are suitable for Runtime PM, and register them with the PM core. If Runtime PM is disabled, just enable the clock. This allows the PM core to automatically manage gate clocks of devices for Runtime PM. ...unless the device is already in an existing pm_domain, right? I like this approach, and it extends nicely what we already do on platforms using drivers/base/power/clock_ops.c into DT land. My only concern is how this will interact if it's used along with devices that have existing pm_domains. I don't have any specific concerns (yet, because it's Friday, and my brain is turing off), but it just made me wonder if this will be potentially confusing. I have big concerns about this approach. First, it will only work if a clock is available at deivce creation time. The conversion of irq controllers to normal device drivers has already shown that is a bad idea. I also don't like that it tries to set up every clock, but there is no guarantee that the driver will even use it. I would rather see this behaviour linked into the function that obtains the clock at driver .probe() time. That way it can handle deferred probe correctly and it only sets up clocks that are actually used by the driver. g. -- 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: pandaboard boot crash with linux-next
[cc'ing linux-next ml] On Wed, Mar 19, 2014 at 2:35 PM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Hi Tomi, On Mar 19, 2014, at 4:33 PM, Tomi Valkeinen wrote: On 19/03/14 16:29, Pantelis Antoniou wrote: Hi Tomi, On Mar 19, 2014, at 4:25 PM, Tomi Valkeinen wrote: On 17/03/14 16:09, Tomi Valkeinen wrote: Hi, I noticed that my omap4 panda does not boot with today's linux-next (8808b950581f71e3ee4cf8e6cae479f4c7106405). I didn't have much time to study it, but I didn't find any posts about the issue with a quick look. Below is the crash. I bisected this to the commit: commit ad2c12e9bc250b3387bcb4ab9ab114f43ff6122f Author: Pantelis Antoniou pa...@antoniou-consulting.com Date: Fri Dec 13 20:08:59 2013 +0200 of: device_node kobject lifecycle fixes After the move to having device nodes be proper kobjects the lifecycle of the node needs to be controlled better. At first convert of_add_node() in the unflattened functions to of_init_node() which initializes the kobject so that of_node_get/put work correctly even before of_init is called. Afterwards introduce of_node_is_initialized of_node_is_attached that query the underlying kobject about the state (attached means kobj is visible in sysfs) Using that make sure the lifecycle of the tree is correct at all times. Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com [grant.likely: moved of_node_init() calls, fixed up locking, and dropped __of_populate() hunks] Signed-off-by: Grant Likely grant.lik...@linaro.org Can you try this? It should fix it (plus it should be in -next soon) Thanks, that fixes the issue (tested on omap4 panda). Tomi Yeah I know; my beaglebone hangs as well without it :) Hi Tomi, Pantelis sent the fix to me yesterday, but I hadn't tested and pushed it out until now. Tomorrow's linux-next it should be okay. g. -- 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/3] Input: twl4030-keypad - add device tree support
On Sun, 27 Oct 2013 12:47:53 +0100, Pavel Machek pa...@ucw.cz wrote: +#if IS_ENABLED(CONFIG_OF) I'm probably missing something here, but why not #ifdef CONFIG_OF? I have been told for other drivers, that IS_ENABLED() is the prefered way to check for configuration these days. CONFIG_OF can not be module, using IS_ENABLED() on it is just wrong. That makes no sense. There is absolutely nothing wrong with using IS_ENABLED() for CONFIG_OF. g. -- 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] twl4030_charger: add devicetree support.
On Thu, 24 Oct 2013 04:44:03 -0500, Kumar Gala ga...@codeaurora.org wrote:
On Oct 24, 2013, at 12:50 AM, NeilBrown wrote:
[my first device-tree related patch. Please let me know what I got wrong so
I wont repeat the mistake in all the others I have queued]
This allows the charger to be enabled with devicetree, and
allows the parameters for charging the backup battery to be set.
Signed-off-by: NeilBrown ne...@suse.de
diff --git a/Documentation/devicetree/bindings/power/twl-charger.txt
b/Documentation/devicetree/bindings/power/twl-charger.txt
new file mode 100644
index 000..8afaa9a
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/twl-charger.txt
@@ -0,0 +1,20 @@
+TWL BCI (Battery Charger Interface)
+
+Required properties:
+- compatible:
+ - ti,twl4030-bci
+- interrupts: two interrupt lines from the TWL SIH (secondary
+ interrupt handler) - interrupts 9 and 2.
+
+Optional properties:
+- bb-uvolt: microvolts for charging the backup battery.
+- bb-uamp: microamps for charging the backup battery.
prop should have vendor prefix.
ti,bb-uvolt, ti,bb-uamp
Agreed. Otherwise:
Acked-by: Grant Likely grant.lik...@linaro.org
- k
+
+Examples:
+
+bci {
+ compatible = ti,twl4030-bci;
+ interrupts = 9, 2;
+ bb-uvolt = 320;
+ bb-uamp = 150;
+};
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
--
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] serial: omap: Add support for optional wake-up
On Tue, 22 Oct 2013 06:49:48 -0700, Tony Lindgren t...@atomide.com wrote:
With the recent pinctrl-single changes, omaps can treat
wake-up events from deeper idle states as interrupts.
There's a separate io chain controller on most omaps
that stays enabled when the device hits off-idle and the
regular interrupt controller is powered off.
Let's add support for the optional second interrupt for
wake-up events. And then serial-omap can manage the
wake-up interrupt from it's runtime PM calls to avoid
spurious interrupts during runtime.
Note that the wake interrupt is board specific as it
uses the UART RX pin, and for omap3, there are six pin
options for UART3 RX pin.
Also Note that the legacy platform based booting handles
the wake-ups in the legacy mux driver and does not need to
pass the wake-up interrupt to the driver.
And finally, to pass the wake-up interrupt in the dts file,
either interrupt-map or the pending interrupts-extended
property needs to be passed. It's probably best to use
interrupts-extended when it's available.
Cc: Felipe Balbi ba...@ti.com
Cc: Kevin Hilman khil...@linaro.org
Cc: Linus Walleij linus.wall...@linaro.org
Reviewed-by: Felipe Balbi ba...@ti.com
Reviewed-by: Roger Quadros rog...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
Updated to simplify based on the review comments.
Greg, care to queue this for v3.13 if not too late?
This removes a dependency for omaps for having runtime
PM working with serial port using device tree, so we
can start moving to device tree only booting for omap3
for v3.14.
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -39,6 +39,7 @@
#include linux/irq.h
#include linux/pm_runtime.h
#include linux/of.h
+#include linux/of_irq.h
#include linux/gpio.h
#include linux/of_gpio.h
#include linux/platform_data/serial-omap.h
@@ -134,6 +135,7 @@ struct uart_omap_port {
struct uart_portport;
struct uart_omap_dmauart_dma;
struct device *dev;
+ int wakeirq;
unsigned char ier;
unsigned char lcr;
@@ -214,10 +216,23 @@ static int serial_omap_get_context_loss_count(struct
uart_omap_port *up)
return pdata-get_context_loss_count(up-dev);
}
+static inline void serial_omap_enable_wakeirq(struct uart_omap_port *up,
+bool enable)
+{
+ if (!up-wakeirq)
+ return;
+
+ if (enable)
+ enable_irq(up-wakeirq);
+ else
+ disable_irq(up-wakeirq);
+}
+
static void serial_omap_enable_wakeup(struct uart_omap_port *up, bool enable)
{
struct omap_uart_port_info *pdata = dev_get_platdata(up-dev);
+ serial_omap_enable_wakeirq(up, enable);
if (!pdata || !pdata-enable_wakeup)
return;
@@ -699,6 +714,20 @@ static int serial_omap_startup(struct uart_port *port)
if (retval)
return retval;
+ /* Optional wake-up IRQ */
+ if (up-wakeirq) {
+ retval = request_irq(up-wakeirq, serial_omap_irq,
+ up-port.irqflags, up-name, up);
+ if (retval) {
+ free_irq(up-port.irq, up);
+ return retval;
+ }
+ disable_irq(up-wakeirq);
+ } else {
+ dev_info(up-port.dev, no wakeirq for uart%d\n,
+ up-port.line);
+ }
+
dev_dbg(up-port.dev, serial_omap_startup+%d\n, up-port.line);
pm_runtime_get_sync(up-dev);
@@ -787,6 +816,8 @@ static void serial_omap_shutdown(struct uart_port *port)
pm_runtime_mark_last_busy(up-dev);
pm_runtime_put_autosuspend(up-dev);
free_irq(up-port.irq, up);
+ if (up-wakeirq)
+ free_irq(up-wakeirq, up);
}
static void serial_omap_uart_qos_work(struct work_struct *work)
@@ -1572,11 +1603,23 @@ static int serial_omap_probe(struct platform_device
*pdev)
struct uart_omap_port *up;
struct resource *mem, *irq;
struct omap_uart_port_info *omap_up_info = dev_get_platdata(pdev-dev);
- int ret;
+ int ret, uartirq = 0, wakeirq = 0;
+ /* The optional wakeirq may be specified in the board dts file */
if (pdev-dev.of_node) {
+ uartirq = irq_of_parse_and_map(pdev-dev.of_node, 0);
+ if (!uartirq)
+ return -EPROBE_DEFER;
+ wakeirq = irq_of_parse_and_map(pdev-dev.of_node, 1);
omap_up_info = of_get_uart_port_info(pdev-dev);
pdev-dev.platform_data = omap_up_info;
+ } else {
+ irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (!irq) {
+ dev_err(pdev-dev, no irq resource?\n);
+ return -ENODEV;
+ }
+ uartirq = irq-start;
Ugh. This is such a
Re: [PATCH 22/51] DMA-API: amba: get rid of separate dma_mask
On Thu, 19 Sep 2013 22:47:01 +0100, Russell King rmk+ker...@arm.linux.org.uk wrote: AMBA Primecell devices always treat streaming and coherent DMA exactly the same, so there's no point in having the masks separated. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk for the drivers/of/platform.c portion: Acked-by: Grant Likely grant.lik...@linaro.org g. -- 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] RFC: interrupt consistency check for OF GPIO IRQs
On Thu, 12 Sep 2013 17:57:00 +0200, Alexander Holler hol...@ahsoftware.de
wrote:
Am 12.09.2013 17:19, schrieb Stephen Warren:
IRQs, DMA channels, and GPIOs are all different things. Their bindings
are defined independently. While it's good to define new types of
bindings consistently with other bindings, this hasn't always happened,
so you can make zero assumptions about the IRQ bindings by reading the
documentation for any other kind of binding.
Multiple interrupts are defined as follows:
// Optional; otherwise inherited from parent/grand-parent/...
interrupt-parent = gpio6;
// Must be in a fixed order, unless binding defines that the
// optional interrupt-names property is to be used.
interrupts = 1 IRQF_TRIGGER_HIGH 2 IRQF_TRIGGER_LOW;
// Optional; binding for device defines whether it must
// be present
interrupt-names = foo, bar;
If you need multiple interrupts, each with a different parent, you need
to use an interrupt-map property (Google it for a more complete
explanation I guess). Unlike interrupts, interrupt-map has a phandle
in each entry, and hence each entry can refer to a different IRQ
controller. You end up defining a dummy interrupt controller node (which
may be the leaf node with multiple IRQ outputs, which then points at
itself as the interrupt parent), pointing the leaf node's
interrupt-parent at that node, and then having interrupt-map demux the
N interrupt outputs to the various interrupt controllers.
What a mess. I assume that is the price that bindings don't have to change.
Thanks for clarifying that,
Alexander Holler
Actually, I think it is solveable but doing so requires a new binding
for interrupts. I took a shot at implementing it earlier this week and
I've got working patches that I'll be posting soon. I created a new
interrupts-extended property that uses a phandle+args type of
binding like this:
intc1: intc@1000 {
interrupt-controller;
#interrupt-cells = 1;
};
intc2: intc@2000 {
interrupt-controller;
#interrupt-cells = 2;
};
device@3000 {
interrupts-extended = intc1 5 intc2 3 4 intc1 6;
};
'interrupts-extended' will be proposed as a directly replacement of the
'interrupts' property and it will eliminate the need for an
interrupt-map property. A node will be allowed to have one or the other,
but not both.
I'll write up a proper binding document and post for review.
g.
--
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] i2c: move of helpers into the core
On Thu, 22 Aug 2013 18:00:14 +0200, Wolfram Sang w...@the-dreams.de wrote: I2C of helpers used to live in of_i2c.c but experience (from SPI) shows that it is much cleaner to have this in the core. This also removes a circular dependency between the helpers and the core, and so we can finally register child nodes in the core instead of doing this manually in each driver. So, fix the drivers and documentation, too. Acked-by: Rob Herring rob.herr...@calxeda.com Reviewed-by: Felipe Balbi ba...@ti.com Acked-by: Rafael J. Wysocki rafael.j.wyso...@intel.com Tested-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Wolfram Sang w...@the-dreams.de Acked-by: Grant Likely grant.lik...@linaro.org --- V2-V3: Was trying to be too smart by only fixing includes needed. Took a more general approach this time, converting of_i2c.h to i2c.h in case i2c.h was not already there. Otherwise remove it. Improved my build scripts and no build failures, no complaints from buildbot as well. Documentation/acpi/enumeration.txt |1 - arch/powerpc/platforms/44x/warp.c |1 - drivers/gpu/drm/tilcdc/tilcdc_slave.c |1 - drivers/gpu/drm/tilcdc/tilcdc_tfp410.c |1 - drivers/gpu/host1x/drm/output.c |2 +- drivers/i2c/busses/i2c-at91.c |3 - drivers/i2c/busses/i2c-cpm.c|6 -- drivers/i2c/busses/i2c-davinci.c|2 - drivers/i2c/busses/i2c-designware-platdrv.c |2 - drivers/i2c/busses/i2c-gpio.c |3 - drivers/i2c/busses/i2c-i801.c |2 - drivers/i2c/busses/i2c-ibm_iic.c|4 - drivers/i2c/busses/i2c-imx.c|3 - drivers/i2c/busses/i2c-mpc.c|2 - drivers/i2c/busses/i2c-mv64xxx.c|3 - drivers/i2c/busses/i2c-mxs.c|3 - drivers/i2c/busses/i2c-nomadik.c|3 - drivers/i2c/busses/i2c-ocores.c |3 - drivers/i2c/busses/i2c-octeon.c |3 - drivers/i2c/busses/i2c-omap.c |3 - drivers/i2c/busses/i2c-pnx.c|3 - drivers/i2c/busses/i2c-powermac.c |9 +- drivers/i2c/busses/i2c-pxa.c|2 - drivers/i2c/busses/i2c-s3c2410.c|2 - drivers/i2c/busses/i2c-sh_mobile.c |2 - drivers/i2c/busses/i2c-sirf.c |3 - drivers/i2c/busses/i2c-stu300.c |2 - drivers/i2c/busses/i2c-tegra.c |3 - drivers/i2c/busses/i2c-versatile.c |2 - drivers/i2c/busses/i2c-wmt.c|3 - drivers/i2c/busses/i2c-xiic.c |3 - drivers/i2c/i2c-core.c | 109 +- drivers/i2c/i2c-mux.c |3 - drivers/i2c/muxes/i2c-arb-gpio-challenge.c |1 - drivers/i2c/muxes/i2c-mux-gpio.c|1 - drivers/i2c/muxes/i2c-mux-pinctrl.c |1 - drivers/media/platform/exynos4-is/fimc-is-i2c.c |4 +- drivers/media/platform/exynos4-is/fimc-is.c |2 +- drivers/media/platform/exynos4-is/media-dev.c |1 - drivers/of/Kconfig |6 -- drivers/of/Makefile |1 - drivers/of/of_i2c.c | 114 --- drivers/staging/imx-drm/imx-tve.c |2 +- include/linux/i2c.h | 20 include/linux/of_i2c.h | 46 - sound/soc/fsl/imx-sgtl5000.c|2 +- sound/soc/fsl/imx-wm8962.c |2 +- 47 files changed, 138 insertions(+), 262 deletions(-) delete mode 100644 drivers/of/of_i2c.c delete mode 100644 include/linux/of_i2c.h diff --git a/Documentation/acpi/enumeration.txt b/Documentation/acpi/enumeration.txt index d9be7a9..958266e 100644 --- a/Documentation/acpi/enumeration.txt +++ b/Documentation/acpi/enumeration.txt @@ -238,7 +238,6 @@ An I2C bus (controller) driver does: if (ret) /* handle error */ - of_i2c_register_devices(adapter); /* Enumerate the slave devices behind this bus via ACPI */ acpi_i2c_register_devices(adapter); diff --git a/arch/powerpc/platforms/44x/warp.c b/arch/powerpc/platforms/44x/warp.c index 4cfa499..534574a 100644 --- a/arch/powerpc/platforms/44x/warp.c +++ b/arch/powerpc/platforms/44x/warp.c @@ -16,7 +16,6 @@ #include linux/interrupt.h #include linux/delay.h #include linux/of_gpio.h -#include linux/of_i2c.h #include linux/slab.h #include linux/export.h diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c index dfffaf0..a19f657 100644
Re: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs
On Mon, Jul 29, 2013 at 6:36 AM, Linus Walleij linus.wall...@linaro.org wrote: To solve this dilemma, perform an interrupt consistency check when adding a GPIO chip: if the chip is both gpio-controller and interrupt-controller, walk all children of the device tree, check if these in turn reference the interrupt-controller, and if they do, loop over the interrupts used by that child and perform gpio_reques() and gpio_direction_input() on these, making them unreachable from the GPIO side. Ugh, that's pretty awful, and it doesn't actually solve the root problem of the GPIO and IRQ subsystems not cooperating. It's also a very DT-centric solution even though we're going to see the exact same issue on ACPI machines. We have to solve the problem in a better way than that. Rearranging your patch description, here are some of the points you brought up so I can comment on them... This has the following undesired effects: - The GPIOlib subsystem is not aware that the line is in use and willingly lets some other consumer perform gpio_request() on it, leading to a complex resource conflict if it occurs. If a gpio line is being both requested as a gpio and used as an interrupt line, then either a) it's a bug, or b) the gpio line needs to be used as input only so it is compatible with irq usage. b) should be supportable. - The GPIO debugfs file claims this GPIO line is free. Surely we can fix this. I still don't see a problem of having the controller request the gpio when it is claimed as an irq if we can get around the problem of another user performing a /valid/ request on the same GPIO line. The solution may be to have a special form of request or flag that allows it to be shared. - The line direction of the interrupt GPIO line is not explicitly set as input, even though it is obvious that such a line need to be set up in this way, often making the system depend on boot-on defaults for this kind of settings. Should also be solvable if the gpio request problem is solved. g. -- 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] of: provide of_platform_unpopulate()
On Mon, 22 Jul 2013 16:16:07 -0500, Rob Herring robherri...@gmail.com wrote: On 07/21/2013 06:44 PM, Grant Likely wrote: On Sun, Jul 21, 2013 at 9:48 PM, Rob Herring robherri...@gmail.com wrote: On 07/21/2013 09:42 AM, Rob Herring wrote: On 07/19/2013 01:14 PM, Sebastian Andrzej Siewior wrote: So I called of_platform_populate() on a device to get each child device probed and on rmmod and I need to reverse its doing. After a quick grep I did what others did as well and rmmod ended in: | Unable to handle kernel NULL pointer dereference at virtual address 0018 | PC is at release_resource+0x18/0x80 | Process rmmod (pid: 2005, stack limit = 0xedc30238) | [c003add0] (release_resource+0x18/0x80) from [c0300e08] (platform_device_del+0x78/0xac) | [c0300e08] (platform_device_del+0x78/0xac) from [c0301358] (platform_device_unregister+0xc/0x18) The problem is that platform_device_del() releases each ressource in its tree. This does not work on platform_devices created by OF becuase they were never added via insert_resource(). As a consequence old-parent in __release_resource() is NULL and we explode while accessing -child. So I either I do something completly wrong _or_ nobody here tested the rmmod path of their driver. Wouldn't the correct fix be to call insert_resource somehow? The problem I have is that while of_platform_populate is all about parsing the DT and creating devices, the removal side has nothing to do with DT. So this should not be in the DT code. I think the core device code should be able to handle removal if the device creation side is done correctly. It looks to me like of_device_add either needs to call platform_device_add rather than device_add. I think the device name setting in platform_device_add should be a nop. If not, a check that the name is already set could be added. BTW, it looks like Grant has attempted this already: Yup, things broke badly. Unfortunately the of_platform_device and platform_device history doesn't treat resources in the same way. I would like to merge the code, but I haven't been able to figure out a clean way to do it. Looks like we do need the unpopulate function. Was there more breakage than imx6 and amba devices? Your first version had a fallback case for powerpc. Couldn't we do just allow that for more than just powerpc? I'd much rather see some work-around within the core DT code with a warning to prevent more proliferation than putting this into drivers. It's tricky stuff. I've not figured out a solution I'm happy with. Trying to figure out when to apply a work around is hard because the resource reservation makes assumptions about the memory range layout that doesn't match the assumptions made by device tree code. One /possible/ option is to not add the resources to the devices at all when the device is registered and instead resolve them right at bind time. Jean Christophe proposed doing this already to solve a different problem; obtaining resources that require other drivers to be probed first. If the resources are resolved at .probe() time, then the resource registration problem should also go away. The downside to that approach is that it makes each deferred probe more expensive; potentially a *lot* more expensive depending on how much work the xlate functions have to do. It would be worth prototyping though to see how well it works. g. -- 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] of: provide of_platform_unpopulate()
On Sun, Jul 21, 2013 at 9:48 PM, Rob Herring robherri...@gmail.com wrote: On 07/21/2013 09:42 AM, Rob Herring wrote: On 07/19/2013 01:14 PM, Sebastian Andrzej Siewior wrote: So I called of_platform_populate() on a device to get each child device probed and on rmmod and I need to reverse its doing. After a quick grep I did what others did as well and rmmod ended in: | Unable to handle kernel NULL pointer dereference at virtual address 0018 | PC is at release_resource+0x18/0x80 | Process rmmod (pid: 2005, stack limit = 0xedc30238) | [c003add0] (release_resource+0x18/0x80) from [c0300e08] (platform_device_del+0x78/0xac) | [c0300e08] (platform_device_del+0x78/0xac) from [c0301358] (platform_device_unregister+0xc/0x18) The problem is that platform_device_del() releases each ressource in its tree. This does not work on platform_devices created by OF becuase they were never added via insert_resource(). As a consequence old-parent in __release_resource() is NULL and we explode while accessing -child. So I either I do something completly wrong _or_ nobody here tested the rmmod path of their driver. Wouldn't the correct fix be to call insert_resource somehow? The problem I have is that while of_platform_populate is all about parsing the DT and creating devices, the removal side has nothing to do with DT. So this should not be in the DT code. I think the core device code should be able to handle removal if the device creation side is done correctly. It looks to me like of_device_add either needs to call platform_device_add rather than device_add. I think the device name setting in platform_device_add should be a nop. If not, a check that the name is already set could be added. BTW, it looks like Grant has attempted this already: Yup, things broke badly. Unfortunately the of_platform_device and platform_device history doesn't treat resources in the same way. I would like to merge the code, but I haven't been able to figure out a clean way to do it. Looks like we do need the unpopulate function. g. -- 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/2] gpio/omap: don't create an IRQ mapping for every GPIO on DT
On Mon, Jul 1, 2013 at 9:04 AM, Linus Walleij linus.wall...@linaro.org wrote: On Sun, Jun 30, 2013 at 2:25 AM, Javier Martinez Canillas martinez.jav...@gmail.com wrote: Yes, It doesn't apply cleanly to your next branch cleanly because this patch-set depends on the following bugfix patch merged late on the -rc cycle (3.10-rc7): 397eada9 (gpio/omap: don't use linear domain mapping for OMAP1) Aha, well this fix was only CC:ed to Grant so I never saw it happen. Obviously it cannot be merged through my GPIO tree then. So, I could change the patches so they can be applied cleanly on your branch but then it will not apply cleanly when you send your pull request to Torvalds. That will probably not work as it would cause even more conflicts upstream, and now the merge window is open so no way can I rebase the tree either. Let's see if we can cram it in as part of a late v3.11 merge or if we'll have to defer to v3.12. Linus, you can actually merge in the final v3.10 tag into your gpio-next branch and then apply on top of that. I would send it as two separate pull requests though. One pull with all the stuff that was in linux-next, and a second that includes the v3.10 merge and the OMAP patches. Include an explaination in that pull request that it is an important bug fix, but it took a bit of time to get it worked out correctly; hence the reason for the merge. That will let Linus T make a decision about whether or not to merge it without affecting the bulk of the gpio changes. g. -- 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] gpio/omap: don't create an IRQ mapping for every GPIO on DT
On Wed, Jun 26, 2013 at 8:50 PM, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
When a GPIO is defined as an interrupt line using Device
Tree, a call to irq_create_of_mapping() is made that calls
irq_create_mapping(). So, is not necessary to do the mapping
for all OMAP GPIO lines and explicitly call irq_create_mapping()
on the driver probe() when booting with Device Tree.
Add a custom IRQ domain .map function handler that will be
called by irq_create_mapping() to map the GPIO lines used as IRQ.
This also allows to execute needed setup code such as configuring
a GPIO as input and enabling the GPIO bank.
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
Acked-by: Grant Likely grant.lik...@linaro.org
---
Changes since v2:
- Unconditionally do the IRQ setup in the .map() function and
only call irq_create_mapping() in the gpio chip init to avoid
code duplication as suggested by Grant Likely.
Changes since v1:
- Split the addition of the .map function handler and the
automatic gpio request in two different patches.
- Add GPIO IRQ setup logic to the irq domain mapping function.
- Only call irq_create_mapping for every GPIO on legacy boot.
- Only setup a GPIO IRQ on the .map function for DeviceTree boot.
drivers/gpio/gpio-omap.c | 54
++
1 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 4a43036..42f04ff 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1068,24 +1068,50 @@ static void omap_gpio_chip_init(struct gpio_bank
*bank)
gpiochip_add(bank-chip);
- for (j = 0; j bank-width; j++) {
- int irq = irq_create_mapping(bank-domain, j);
- irq_set_lockdep_class(irq, gpio_lock_class);
- irq_set_chip_data(irq, bank);
- if (bank-is_mpuio) {
- omap_mpuio_alloc_gc(bank, irq, bank-width);
- } else {
- irq_set_chip_and_handler(irq, gpio_irq_chip,
-handle_simple_irq);
- set_irq_flags(irq, IRQF_VALID);
- }
- }
+ /*
+* REVISIT these explicit calls to irq_create_mapping()
+* to do the GPIO to IRQ domain mapping for each GPIO in
+* the bank can be removed once all OMAP platforms have
+* been migrated to Device Tree boot only.
+* Since in DT boot irq_create_mapping() is called from
+* irq_create_of_mapping() only for the GPIO lines that
+* are used as interrupts.
+*/
+ if (!of_have_populated_dt())
+ for (j = 0; j bank-width; j++)
+ irq_create_mapping(bank-domain, j);
irq_set_chained_handler(bank-irq, gpio_irq_handler);
irq_set_handler_data(bank-irq, bank);
}
static const struct of_device_id omap_gpio_match[];
+static int omap_gpio_irq_map(struct irq_domain *d, unsigned int virq,
+irq_hw_number_t hwirq)
+{
+ struct gpio_bank *bank = d-host_data;
+
+ if (!bank)
+ return -EINVAL;
+
+ irq_set_lockdep_class(virq, gpio_lock_class);
+ irq_set_chip_data(virq, bank);
+ if (bank-is_mpuio) {
+ omap_mpuio_alloc_gc(bank, virq, bank-width);
+ } else {
+ irq_set_chip_and_handler(virq, gpio_irq_chip,
+handle_simple_irq);
+ set_irq_flags(virq, IRQF_VALID);
+ }
+
+ return 0;
+}
+
+static struct irq_domain_ops omap_gpio_irq_ops = {
+ .xlate = irq_domain_xlate_onetwocell,
+ .map= omap_gpio_irq_map,
+};
+
static int omap_gpio_probe(struct platform_device *pdev)
{
struct device *dev = pdev-dev;
@@ -1151,10 +1177,10 @@ static int omap_gpio_probe(struct platform_device
*pdev)
}
bank-domain = irq_domain_add_legacy(node, bank-width, irq_base,
-0, irq_domain_simple_ops, NULL);
+0, omap_gpio_irq_ops, bank);
#else
bank-domain = irq_domain_add_linear(node, bank-width,
-irq_domain_simple_ops, NULL);
+omap_gpio_irq_ops, bank);
#endif
if (!bank-domain) {
dev_err(dev, Couldn't register an IRQ domain\n);
--
1.7.7.6
--
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 2/2] gpio/omap: auto request GPIO as input if used as IRQ via DT
On Wed, Jun 26, 2013 at 8:50 PM, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
When an OMAP GPIO is used as an IRQ line, a call to gpio_request()
has to be made to initialize the OMAP GPIO bank before a driver
request the IRQ. Otherwise the call to request_irq() fails.
Drives should not be aware of this neither care wether an IRQ line
is a GPIO or not. They should just request the IRQ and this has to
be handled by the irq_chip driver.
With the current OMAP GPIO DT binding, if we define:
gpio6: gpio@49058000 {
compatible = ti,omap3-gpio;
reg = 0x49058000 0x200;
interrupts = 34;
ti,hwmods = gpio6;
gpio-controller;
#gpio-cells = 2;
interrupt-controller;
#interrupt-cells = 2;
};
interrupt-parent = gpio6;
interrupts = 16 8;
The GPIO is correctly mapped as an IRQ but a call to gpio_request()
is never made. Since a call to the custom IRQ domain .map function
handler is made for each GPIO used as an IRQ, the GPIO can be setup
and configured as input there automatically.
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
Acked-by: Grant Likely grant.lik...@linaro.org
---
Changes since v2:
- Only make the call to gpio_request_one() conditional in the DT
case as suggested by Grant Likely.
Changes since v1:
- Split the irq domain mapping function handler and the GPIO
request in two different patches.
drivers/gpio/gpio-omap.c | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 42f04ff..98848c9 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1090,6 +1090,8 @@ static int omap_gpio_irq_map(struct irq_domain *d,
unsigned int virq,
irq_hw_number_t hwirq)
{
struct gpio_bank *bank = d-host_data;
+ int gpio;
+ int ret;
if (!bank)
return -EINVAL;
@@ -1104,6 +1106,15 @@ static int omap_gpio_irq_map(struct irq_domain *d,
unsigned int virq,
set_irq_flags(virq, IRQF_VALID);
}
+ if (of_have_populated_dt()) {
+ gpio = irq_to_gpio(bank, hwirq);
+ ret = gpio_request_one(gpio, GPIOF_IN, NULL);
+ if (ret) {
+ dev_err(bank-dev, Could not request GPIO%d\n,
gpio);
+ return ret;
+ }
+ }
+
return 0;
}
--
1.7.7.6
--
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
[GIT PULL] GPIO regression fix for omap1 for v3.10-rc
Hi Linus, It took a while to work out the correct solution to this regression. It is sorted now. This branch was constructed and tested by Tony. I've verified that it builds and signed the tag. Please pull into v3.10. g. The following changes since commit 9e895ace5d82df8929b16f58e9f515f6d54ab82d: Linux 3.10-rc7 (2013-06-22 09:47:31 -1000) are available in the git repository at: git://git.secretlab.ca/git/linux tags/gpio-for-linus for you to fetch changes up to 397eada946712b90e0620c378b366bcc6c98c9f6: gpio/omap: don't use linear domain mapping for OMAP1 (2013-06-25 23:13:40 -0700) Fix for omap1 GPIO breaking regression Javier Martinez Canillas (1): gpio/omap: don't use linear domain mapping for OMAP1 drivers/gpio/gpio-omap.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) On Wed, Jun 26, 2013 at 8:05 AM, Tony Lindgren t...@atomide.com wrote: The following changes since commit 9e895ace5d82df8929b16f58e9f515f6d54ab82d: Linux 3.10-rc7 (2013-06-22 09:47:31 -1000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap tags/omap-for-v3.10/gpio-signed for you to fetch changes up to 397eada946712b90e0620c378b366bcc6c98c9f6: gpio/omap: don't use linear domain mapping for OMAP1 (2013-06-25 23:13:40 -0700) Fix for omap1 GPIO breaking regression. Javier Martinez Canillas (1): gpio/omap: don't use linear domain mapping for OMAP1 drivers/gpio/gpio-omap.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) -- 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: [BISECTED] 3.10-rc1 OMAP1 GPIO IRQ regression
On Tue, Jun 25, 2013 at 8:04 AM, Tony Lindgren t...@atomide.com wrote: * Grant Likely grant.lik...@secretlab.ca [130624 09:00]: On Mon, Jun 24, 2013 at 8:21 AM, Tony Lindgren t...@atomide.com wrote: * Javier Martinez Canillas martinez.jav...@gmail.com [130623 18:08]: On Mon, Jun 24, 2013 at 1:43 AM, Aaro Koskinen aaro.koski...@iki.fi wrote: Hi, On Mon, Jun 24, 2013 at 01:06:37AM +0200, Javier Martinez Canillas wrote: On Mon, Jun 24, 2013 at 12:16 AM, Aaro Koskinen aaro.koski...@iki.fi wrote: What is the status of this patch? We're already at 3.10-rc7 and GPIO IRQs are still broken on OMAP1. [...] There is a problem with this patch. [...] So I think that the correct solution is to add SPARSE_IRQ support to omap1 and not reverting Jon's patch. Of course this may not be possible since we are so close to 3.10 and most OMAP patches already merged for 3.11 but we should definitely try to have this at least for 3.12. Otherwise we won't be able to move to DT-only booting for OMAP2+. OMAP1 does not use DT. So we could put this code under #ifdef CONFIG_ARCH_OMAP1 or similar. It's just a few lines of code. OMAP2+ work should not regress OMAP1. Demanding SPARSE_IRQ support for OMAP1 should have been discussed before these changes were made. It's not reasonable to assume such things can be made during rc-cycle. Also, now, I don't think it's reasonable to wait for that to be done, as it would take until 3.12 or even later to get OMAP1 functional again. A. Hi, Yes, since we are so late in the -rc cycle and OMAP1 is currently broken I agree that the only sensible solution is to revert the patch for now. Agreed. I just wanted to point out the issue that keeping the OMAP GPIO driver using legacy mapping domain represents a blocker to have GPIO-IRQ working with Device Tree for OMAP2+ Yes. We can do the ifdef Aaro suggested, and let's also plan on converting omap1 to use SPARSE_IRQ. But with the ifdef we can cut away the dependency between these two. Alright. Sorry I dropped the ball on this one. I've lost track of which patch needs to get applied, but given that it is so late in the cycle, it would be better for someone else to apply the change, test and send a pull request to Linus. I'm okay with it going through the OMAP tree if that is the most expedient. Alternately, send me the pull request and I'll pass it on to Linus. OK, I'll wait for Aaro's ack on Javier's patch and then put it into a branch for you. Thanks Tony. g. -- 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/2] gpio/omap: don't create an IRQ mapping for every GPIO on DT
On Sat, 22 Jun 2013 00:50:53 +0200, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
When a GPIO is defined as an interrupt line using Device
Tree, a call to irq_create_of_mapping() is made that calls
irq_create_mapping(). So, is not necessary to do the mapping
on the OMAP GPIO platform_driver and in fact is wrong to
assume that all GPIO lines will be used as an IRQ.
Add a custom IRQ domain .map function handler that will be
called by the IRQ core to map only the GPIO lines used as
IRQ. This also allows to execute needed setup code such as
configuring a GPIO as input and enabling the GPIO bank.
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
Changes since v1:
- Split the addition of the .map function handler and the
automatic gpio request in two different patches.
- Add GPIO IRQ setup logic to the irq domain mapping function.
- Only call irq_create_mapping for every GPIO on legacy boot.
- Only setup a GPIO IRQ on the .map function for DeviceTree boot.
drivers/gpio/gpio-omap.c | 52 -
1 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index d3f7d2d..31cbe65 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1068,16 +1068,18 @@ static void omap_gpio_chip_init(struct gpio_bank
*bank)
gpiochip_add(bank-chip);
- for (j = 0; j bank-width; j++) {
- int irq = irq_create_mapping(bank-domain, j);
- irq_set_lockdep_class(irq, gpio_lock_class);
- irq_set_chip_data(irq, bank);
- if (bank-is_mpuio) {
- omap_mpuio_alloc_gc(bank, irq, bank-width);
- } else {
- irq_set_chip_and_handler(irq, gpio_irq_chip,
- handle_simple_irq);
- set_irq_flags(irq, IRQF_VALID);
+ if (!of_have_populated_dt()) {
+ for (j = 0; j bank-width; j++) {
+ int irq = irq_create_mapping(bank-domain, j);
+ irq_set_lockdep_class(irq, gpio_lock_class);
+ irq_set_chip_data(irq, bank);
+ if (bank-is_mpuio) {
+ omap_mpuio_alloc_gc(bank, irq, bank-width);
+ } else {
+ irq_set_chip_and_handler(irq, gpio_irq_chip,
+ handle_simple_irq);
+ set_irq_flags(irq, IRQF_VALID);
+ }
}
}
irq_set_chained_handler(bank-irq, gpio_irq_handler);
@@ -1086,6 +1088,34 @@ static void omap_gpio_chip_init(struct gpio_bank *bank)
static const struct of_device_id omap_gpio_match[];
+static int omap_gpio_irq_map(struct irq_domain *d, unsigned int virq,
+ irq_hw_number_t hwirq)
+{
+ struct gpio_bank *bank = d-host_data;
+
+ if (!bank)
+ return -EINVAL;
+
+ if (of_have_populated_dt()) {
+ irq_set_lockdep_class(virq, gpio_lock_class);
+ irq_set_chip_data(virq, bank);
+ if (bank-is_mpuio) {
+ omap_mpuio_alloc_gc(bank, virq, bank-width);
+ } else {
+ irq_set_chip_and_handler(virq, gpio_irq_chip,
+ handle_simple_irq);
+ set_irq_flags(virq, IRQF_VALID);
+ }
+ }
Actually, this looks wrong. You'll notice that the same block of code is
now duplicated in the map function and outside the map. The problem is
that the original code is manually walking through all the irqs and
doing the mapping work, but all of that stuff is what the .map() hook is
for!
Instead, the entire of the above block should be executed
unconditionally inside the map hook. In the DT case, it will get called
automatically only on referenced irqs. In the non-DT case, the for loop
in omap_gpio_chip_init() will only need to call irq_create_mapping() for
each irq, which in turn will call -map().
g.
+
+ return 0;
+}
+
+static struct irq_domain_ops omap_gpio_irq_ops = {
+ .xlate = irq_domain_xlate_onetwocell,
+ .map= omap_gpio_irq_map,
+};
+
static int omap_gpio_probe(struct platform_device *pdev)
{
struct device *dev = pdev-dev;
@@ -1137,7 +1167,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
bank-domain = irq_domain_add_linear(node, bank-width,
- irq_domain_simple_ops, NULL);
+ omap_gpio_irq_ops, bank);
if (!bank-domain)
return -ENODEV;
--
1.7.7.6
--
email sent from notmuch.vim plugin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to
Re: [PATCH v2 2/2] gpio/omap: auto request GPIO as input if used as IRQ via DT
On Sat, 22 Jun 2013 00:50:54 +0200, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
When an OMAP GPIO is used as an IRQ line, a call to gpio_request()
has to be made to initialize the OMAP GPIO bank before a driver
request the IRQ. Otherwise the call to request_irq() fails.
Drives should not be aware of this neither care wether an IRQ line
is a GPIO or not. They should just request the IRQ and this has to
be handled by the irq_chip driver.
With the current OMAP GPIO DT binding, if we define:
gpio6: gpio@49058000 {
compatible = ti,omap3-gpio;
reg = 0x49058000 0x200;
interrupts = 34;
ti,hwmods = gpio6;
gpio-controller;
#gpio-cells = 2;
interrupt-controller;
#interrupt-cells = 2;
};
interrupt-parent = gpio6;
interrupts = 16 8;
The GPIO is correctly mapped as an IRQ but a call to gpio_request()
is never made. Since a call to the custom IRQ domain .map function
handler is made for each GPIO used as an IRQ, the GPIO can be setup
and configured as input there automatically.
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
Changes since v1:
- Split the irq domain mapping function handler and the GPIO
request in two different patches.
drivers/gpio/gpio-omap.c |9 +
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 31cbe65..5ec6a00 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1092,6 +1092,8 @@ static int omap_gpio_irq_map(struct irq_domain *d,
unsigned int virq,
irq_hw_number_t hwirq)
{
struct gpio_bank *bank = d-host_data;
+ int gpio;
+ int ret;
if (!bank)
return -EINVAL;
@@ -1106,6 +1108,13 @@ static int omap_gpio_irq_map(struct irq_domain *d,
unsigned int virq,
handle_simple_irq);
set_irq_flags(virq, IRQF_VALID);
}
+
+ gpio = irq_to_gpio(bank, hwirq);
+ ret = gpio_request_one(gpio, GPIOF_IN, NULL);
+ if (ret) {
+ dev_err(bank-dev, Could not request GPIO%d\n, gpio);
+ return ret;
+ }
Following from my comment on patch 1, this is the only bit that you'd
want to be conditional on the presence of a DT, not the whole block.
g.
--
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: [BISECTED] 3.10-rc1 OMAP1 GPIO IRQ regression
On Mon, Jun 24, 2013 at 8:21 AM, Tony Lindgren t...@atomide.com wrote: * Javier Martinez Canillas martinez.jav...@gmail.com [130623 18:08]: On Mon, Jun 24, 2013 at 1:43 AM, Aaro Koskinen aaro.koski...@iki.fi wrote: Hi, On Mon, Jun 24, 2013 at 01:06:37AM +0200, Javier Martinez Canillas wrote: On Mon, Jun 24, 2013 at 12:16 AM, Aaro Koskinen aaro.koski...@iki.fi wrote: What is the status of this patch? We're already at 3.10-rc7 and GPIO IRQs are still broken on OMAP1. [...] There is a problem with this patch. [...] So I think that the correct solution is to add SPARSE_IRQ support to omap1 and not reverting Jon's patch. Of course this may not be possible since we are so close to 3.10 and most OMAP patches already merged for 3.11 but we should definitely try to have this at least for 3.12. Otherwise we won't be able to move to DT-only booting for OMAP2+. OMAP1 does not use DT. So we could put this code under #ifdef CONFIG_ARCH_OMAP1 or similar. It's just a few lines of code. OMAP2+ work should not regress OMAP1. Demanding SPARSE_IRQ support for OMAP1 should have been discussed before these changes were made. It's not reasonable to assume such things can be made during rc-cycle. Also, now, I don't think it's reasonable to wait for that to be done, as it would take until 3.12 or even later to get OMAP1 functional again. A. Hi, Yes, since we are so late in the -rc cycle and OMAP1 is currently broken I agree that the only sensible solution is to revert the patch for now. Agreed. I just wanted to point out the issue that keeping the OMAP GPIO driver using legacy mapping domain represents a blocker to have GPIO-IRQ working with Device Tree for OMAP2+ Yes. We can do the ifdef Aaro suggested, and let's also plan on converting omap1 to use SPARSE_IRQ. But with the ifdef we can cut away the dependency between these two. Alright. Sorry I dropped the ball on this one. I've lost track of which patch needs to get applied, but given that it is so late in the cycle, it would be better for someone else to apply the change, test and send a pull request to Linus. I'm okay with it going through the OMAP tree if that is the most expedient. Alternately, send me the pull request and I'll pass it on to Linus. g. -- 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 RFC 1/3] clk: omap: introduce clock driver
On Mon, 3 Jun 2013 23:39:16 -0700, Mike Turquette mturque...@linaro.org
wrote:
Parses OMAP clock data from DT and registers those clocks with the clock
framework. dt_omap_clk_init must be called early during boot for timer
initialization so it is exported and called from the existing clock code
instead of probing like a real driver.
Cc: Benoit Cousson b-cous...@ti.com
Cc: Rajendra Nayak rna...@ti.com
Cc: Joel A Fernandes joelag...@ti.com
Cc: Nishanth Menon n...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Tony Lindgren t...@atomide.com
Signed-off-by: Mike Turquette mturque...@linaro.org
Hi Mike,
Comments below...
---
This driver simply matches the basic bindings (so far). Eventually it
would match omap-specific bindings for DPLLs, CLKOUTX2 and strange leaf
clocks as well. This doesn't scale well since non-OMAP related clock
data (e.g. a pmic or discrete audio codec) will get grouped into this
driver if it matches on a basic clock type. Suggestions?
Take a look at the definition of irqchip_init(). It would be possible to
do the same think for clk chips so that merely configuring in the driver
would add the support to a global list of clk chip initializers.
drivers/irqchip/irqchip.c
drivers/clk/Makefile | 1 +
drivers/clk/omap/Makefile | 1 +
drivers/clk/omap/clk.c| 55
+++
include/linux/clk/omap.h | 24 +
4 files changed, 81 insertions(+)
create mode 100644 drivers/clk/omap/Makefile
create mode 100644 drivers/clk/omap/clk.c
create mode 100644 include/linux/clk/omap.h
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index f51b52b..efd4f2a 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
obj-$(CONFIG_ARCH_ZYNQ) += clk-zynq.o
obj-$(CONFIG_ARCH_TEGRA) += tegra/
obj-$(CONFIG_PLAT_SAMSUNG) += samsung/
+obj-$(CONFIG_ARCH_OMAP) += omap/
obj-$(CONFIG_X86)+= x86/
diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
new file mode 100644
index 000..8195931
--- /dev/null
+++ b/drivers/clk/omap/Makefile
@@ -0,0 +1 @@
+obj-y+= clk.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
new file mode 100644
index 000..e9e5c95
--- /dev/null
+++ b/drivers/clk/omap/clk.c
@@ -0,0 +1,55 @@
+/*
+ * OMAP PRCM clock driver
+ *
+ * Copyright (C) 2013 Linaro.org - http://www.linaro.org
+ * Mike Turquette mturque...@linaro.org
+ *
+ * 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 as is WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk-provider.h
+#include linux/clk/omap.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of_device.h
+#include linux/platform_device.h
+
+/* FIXME - should the OMAP PRCM clock driver match generic types? */
+static const struct of_device_id clk_match[] = {
+ {.compatible = fixed-clock, .data = of_fixed_clk_setup, },
+ {.compatible = mux-clock, .data = of_mux_clk_setup, },
+ {.compatible = fixed-factor-clock,
+ .data = of_fixed_factor_clk_setup, },
+ {},
+};
+
+static int omap_clk_probe(struct platform_device *pdev)
+{
+ of_clk_init(clk_match);
+ return 0;
+}
+
+static struct platform_driver omap_clk_driver = {
+ .probe = omap_clk_probe,
+ .driver = {
+.name = omap_clk,
+.of_match_table = of_match_ptr(clk_match),
+},
+};
+
+/* FIXME - need to initialize early; skip real driver registration probe */
+int __init dt_omap_clk_init(void)
+{
+ return omap_clk_probe(NULL);
+}
Since this isn't remotely a platform_driver, I would drop the pretense
and cut out all the platform_drivers references. omap_clk_driver isn't
even referenced anywhere!
+
+MODULE_DESCRIPTION(OMAP Clock driver);
+MODULE_AUTHOR(Texas Instruments Inc.);
+MODULE_LICENSE(GPL v2);
diff --git a/include/linux/clk/omap.h b/include/linux/clk/omap.h
new file mode 100644
index 000..504e838
--- /dev/null
+++ b/include/linux/clk/omap.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2010 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even
Re: [PATCH 1/3] ARM: dts: Add headers with constants for MTD partitions
On Tue, 11 Jun 2013 16:48:56 +0200, Florian Vaussard florian.vauss...@epfl.ch wrote: These constants can be used to easily declare MTD partitions inside DTS. The constants MTDPART_OFS_* are purposely not included. Indeed, parse_ofpart_partitions() is expecting u64, but a DT cell is u32. Negative constants, as defined by MTDPART_OFS_*, would be wrongly The DT binding uses the number of cells defined by #address-cells. It is not fixed to a u32 or a u64 interpreted by parse_ofpart_partitions(). Two cells should be used to correctly encode the negative constants, but this breaks current usage. The binding doesn't even allow for shortcuts like MTDPART_SIZ_FULL. If a partition fills the whole device, then the reg property should include the actual size. If the code is allowing '0' to be used to mean MTDPART_SIZ_FULL, then that is a bug that needs to be fixed. Please drop the mtd/partitions.h hunk from this patch. g. Signed-off-by: Florian Vaussard florian.vauss...@epfl.ch --- include/dt-bindings/mtd/partitions.h | 12 include/dt-bindings/sizes.h | 52 ++ 2 files changed, 64 insertions(+), 0 deletions(-) create mode 100644 include/dt-bindings/mtd/partitions.h create mode 100644 include/dt-bindings/sizes.h diff --git a/include/dt-bindings/mtd/partitions.h b/include/dt-bindings/mtd/partitions.h new file mode 100644 index 000..7dfa676 --- /dev/null +++ b/include/dt-bindings/mtd/partitions.h @@ -0,0 +1,12 @@ +/* + * This header provides constants used with MTD partitions. + */ + +#ifndef _DT_BINDINGS_MTD_PARTITIONS_H +#define _DT_BINDINGS_MTD_PARTITIONS_H + +/* Partition size */ +#define MTDPART_SIZ_FULL 0 + +#endif + diff --git a/include/dt-bindings/sizes.h b/include/dt-bindings/sizes.h new file mode 100644 index 000..995f2de --- /dev/null +++ b/include/dt-bindings/sizes.h @@ -0,0 +1,52 @@ +/* + * This header provides size constants. + * + * Original version: + * include/linux/sizes.h + * + * 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. + */ + +#ifndef _DT_BINDINGS_SIZES_H +#define _DT_BINDINGS_SIZES_H + +#define SZ_1 0x0001 +#define SZ_2 0x0002 +#define SZ_4 0x0004 +#define SZ_8 0x0008 +#define SZ_160x0010 +#define SZ_320x0020 +#define SZ_640x0040 +#define SZ_128 0x0080 +#define SZ_256 0x0100 +#define SZ_512 0x0200 + +#define SZ_1K0x0400 +#define SZ_2K0x0800 +#define SZ_4K0x1000 +#define SZ_8K0x2000 +#define SZ_16K 0x4000 +#define SZ_32K 0x8000 +#define SZ_64K 0x0001 +#define SZ_128K 0x0002 +#define SZ_256K 0x0004 +#define SZ_512K 0x0008 + +#define SZ_1M0x0010 +#define SZ_2M0x0020 +#define SZ_4M0x0040 +#define SZ_8M0x0080 +#define SZ_16M 0x0100 +#define SZ_32M 0x0200 +#define SZ_64M 0x0400 +#define SZ_128M 0x0800 +#define SZ_256M 0x1000 +#define SZ_512M 0x2000 + +#define SZ_1G0x4000 +#define SZ_2G0x8000 + +#endif + -- 1.7.5.4 ___ devicetree-discuss mailing list devicetree-disc...@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- 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/3] ARM: dts: OMAP3: Use MTD constants for OMAP3 boards
On Tue, 11 Jun 2013 17:29:43 +0200, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
On 06/11/2013 04:48 PM, Florian Vaussard wrote:
Use the MTD constants for NAND and OneNAND nodes used in OMAP3
DTS.
Signed-off-by: Florian Vaussard florian.vauss...@epfl.ch
---
arch/arm/boot/dts/omap3-devkit8000.dts | 10 +-
arch/arm/boot/dts/omap3-igep0020.dts | 10 +-
arch/arm/boot/dts/omap3-igep0030.dts | 10 +-
arch/arm/boot/dts/omap3430-sdp.dts | 28 ++--
4 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-devkit8000.dts
b/arch/arm/boot/dts/omap3-devkit8000.dts
index 5be71b1..08699cb 100644
--- a/arch/arm/boot/dts/omap3-devkit8000.dts
+++ b/arch/arm/boot/dts/omap3-devkit8000.dts
@@ -143,27 +143,27 @@
x-loader@0 {
label = X-Loader;
- reg = 0 0x8;
+ reg = (0 * SZ_128K) (4 * SZ_128K);
};
bootloaders@8 {
label = U-Boot;
- reg = 0x8 0x1e;
+ reg = (4 * SZ_128K) (15 * SZ_128K);
};
bootloaders_env@26 {
label = U-Boot Env;
- reg = 0x26 0x2;
+ reg = (19 * SZ_128K) (1 * SZ_128K);
};
kernel@28 {
label = Kernel;
- reg = 0x28 0x40;
+ reg = (20 * SZ_128K) (32 * SZ_128K);
};
filesystem@68 {
label = File System;
- reg = 0x68 0xf98;
+ reg = (52 * SZ_128K) MTDPART_SIZ_FULL;
};
};
};
diff --git a/arch/arm/boot/dts/omap3-igep0020.dts
b/arch/arm/boot/dts/omap3-igep0020.dts
index e8c4828..3476b3c 100644
--- a/arch/arm/boot/dts/omap3-igep0020.dts
+++ b/arch/arm/boot/dts/omap3-igep0020.dts
@@ -97,23 +97,23 @@
partition@0 {
label = SPL;
- reg = 0 0x10;
+ reg = (0 * SZ_256K) (4 * SZ_256K);
};
partition@0x8 {
label = U-Boot;
- reg = 0x10 0x18;
+ reg = (4 * SZ_256K) (6 * SZ_256K);
};
partition@0x1c {
label = Environment;
- reg = 0x28 0x10;
+ reg = (10 * SZ_256K) (4 * SZ_256K);
};
partition@0x28 {
label = Kernel;
- reg = 0x38 0x30;
+ reg = (14 * SZ_256K) (12 * SZ_256K);
};
partition@0x78 {
label = Filesystem;
- reg = 0x68 0x1f98;
+ reg = (26 * SZ_256K) MTDPART_SIZ_FULL;
};
};
diff --git a/arch/arm/boot/dts/omap3-igep0030.dts
b/arch/arm/boot/dts/omap3-igep0030.dts
index 644d053..e4f078c 100644
--- a/arch/arm/boot/dts/omap3-igep0030.dts
+++ b/arch/arm/boot/dts/omap3-igep0030.dts
@@ -72,23 +72,23 @@
partition@0 {
label = SPL;
- reg = 0 0x10;
+ reg = (0 * SZ_256K) (4 * SZ_256K);
};
partition@0x8 {
label = U-Boot;
- reg = 0x10 0x18;
+ reg = (4 * SZ_256K) (6 * SZ_256K);
};
partition@0x1c {
label = Environment;
- reg = 0x28 0x10;
+ reg = (10 * SZ_256K) (4 * SZ_256K);
};
partition@0x28 {
label = Kernel;
- reg = 0x38 0x30;
+ reg = (14 * SZ_256K) (12 * SZ_256K);
};
partition@0x78 {
label = Filesystem;
- reg = 0x68 0x1f98;
+ reg = (26 * SZ_256K) MTDPART_SIZ_FULL;
};
};
};
Hi Florian,
I don't have access to my IGEP board so I can test it right now but the patch
looks good to me.
In fact I wanted to use MTDPART_SIZ_FULL when added the NAND nodes since not
all
IGEP boards have 512MB flash but I didn't know that a value of 0 meant that.
So thanks a lot for doing this!
Acked-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
However, the binding doesn't allow for that and so it is a bug in the
parser. Relying on '0' is not safe. Nor does it match device tree
convention for the reg property, so don't count on getting an extension
added to allow it.
NAK
--
To unsubscribe from this list: send the line unsubscribe linux-omap
Re: [PATCH 3/5] gpio/omap: Add DT support to GPIO driver
On Fri, 26 Apr 2013 16:31:24 -0500, Jon Hunter jon-hun...@ti.com wrote:
On 04/26/2013 02:31 AM, Linus Walleij wrote:
On Wed, Apr 17, 2013 at 2:41 AM, Javier Martinez Canillas
martinez.jav...@gmail.com wrote:
So:
+static int omap_gpio_irq_domain_xlate(struct irq_domain *d,
+ struct device_node *ctrlr,
+ const u32 *intspec, unsigned int
intsize,
+ irq_hw_number_t *out_hwirq,
+ unsigned int *out_type)
+{
+ int ret;
+ struct gpio_bank *bank = d-host_data;
+ int gpio = bank-chip.base + intspec[0];
+
+ if (WARN_ON(intsize 2))
+ return -EINVAL;
+
+ ret = gpio_request_one(gpio, GPIOF_IN, ctrlr-full_name);
+ if (ret)
+ return ret;
So how to figure out if a device is already requesting this GPIO
on some orthogonal axis?
I really don't think that is necessary. Hopefully, my other email [1]
elaborates on why. Let me know if this makes sense.
I would agree here. If the irq specified happens to be a GPIO; then the
onus is on the GPIO/IRQ controller driver to make sure that GPIO is
actually set up to work as an IRQ.
g.
--
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: omap3-devkit8000: fix NAND memory binding
On Tue, 11 Jun 2013 16:53:19 +0200, Florian Vaussard florian.vauss...@epfl.ch wrote: Hello, On 05/31/2013 03:49 PM, Florian Vaussard wrote: Hello, Gentle ping. Does someone has any comments on this fix? Can someone tests on the real hardware? Nobody has this hardware somewhere in a drawer? :-) I've just gone ahead and applied it. As you say it's broken anyway, so if this is also broken, then at least it is /less/ broken. :) g. Regards, Florian Regards, Florian On 05/23/2013 10:11 AM, Florian Vaussard wrote: Commit d36b4cd 'ARM: OMAP2+: Add additional GPMC timing parameters' updated GPMC binding, but omap3-devkit8000 was not updated accordingly, resulting in a broken configuration. Signed-off-by: Florian Vaussard florian.vauss...@epfl.ch --- arch/arm/boot/dts/omap3-devkit8000.dts | 29 +++-- 1 files changed, 15 insertions(+), 14 deletions(-) diff --git a/arch/arm/boot/dts/omap3-devkit8000.dts b/arch/arm/boot/dts/omap3-devkit8000.dts index 8a5cdcc..e5b35f5 100644 --- a/arch/arm/boot/dts/omap3-devkit8000.dts +++ b/arch/arm/boot/dts/omap3-devkit8000.dts @@ -123,20 +123,21 @@ reg = 0 0 0; /* CS0, offset 0 */ nand-bus-width = 16; -gpmc,sync-clk = 0; -gpmc,cs-on = 0; -gpmc,cs-rd-off = 44; -gpmc,cs-wr-off = 44; -gpmc,adv-on = 6; -gpmc,adv-rd-off = 34; -gpmc,adv-wr-off = 44; -gpmc,we-off = 40; -gpmc,oe-off = 54; -gpmc,access = 64; -gpmc,rd-cycle = 82; -gpmc,wr-cycle = 82; -gpmc,wr-access = 40; -gpmc,wr-data-mux-bus = 0; +gpmc,device-nand; +gpmc,sync-clki-ps = 0; +gpmc,cs-on-ns = 0; +gpmc,cs-rd-off-ns = 44; +gpmc,cs-wr-off-ns = 44; +gpmc,adv-on-ns = 6; +gpmc,adv-rd-off-ns = 34; +gpmc,adv-wr-off-ns = 44; +gpmc,we-off-ns = 40; +gpmc,oe-off-ns = 54; +gpmc,access-ns = 64; +gpmc,rd-cycle-ns = 82; +gpmc,wr-cycle-ns = 82; +gpmc,wr-access-ns = 40; +gpmc,wr-data-mux-bus-ns = 0; #address-cells = 1; #size-cells = 1; -- Florian Vaussard EPFL - STI - IMT - LSRO1 MEB330 - Station 9 1015 Lausanne / Switzerland tel: +41 21 693 78 39 fax: +41 21 693 78 07 http://lsro.epfl.ch ___ devicetree-discuss mailing list devicetree-disc...@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- 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: Protect pinctrl headers against multiple inclusions
On Tue, 11 Jun 2013 16:50:50 +0200, Florian Vaussard florian.vauss...@epfl.ch wrote: Pinctrl headers were not protected with #ifndef. Signed-off-by: Florian Vaussard florian.vauss...@epfl.ch Obviously this needs to go in via whatever tree added the modified header files. Acked-by: Grant Likely grant.lik...@secretlab.ca --- include/dt-bindings/pinctrl/am33xx.h |5 + include/dt-bindings/pinctrl/omap.h |5 + 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/include/dt-bindings/pinctrl/am33xx.h b/include/dt-bindings/pinctrl/am33xx.h index a3fddd4..469e032 100644 --- a/include/dt-bindings/pinctrl/am33xx.h +++ b/include/dt-bindings/pinctrl/am33xx.h @@ -2,6 +2,9 @@ * This header provides constants specific to AM33XX pinctrl bindings. */ +#ifndef _DT_BINDINGS_PINCTRL_AM33XX_H +#define _DT_BINDINGS_PINCTRL_AM33XX_H + #include include/dt-bindings/pinctrl/omap.h /* am33xx specific mux bit defines */ @@ -35,3 +38,5 @@ #undef PIN_OFF_INPUT_PULLDOWN #undef PIN_OFF_WAKEUPENABLE +#endif + diff --git a/include/dt-bindings/pinctrl/omap.h b/include/dt-bindings/pinctrl/omap.h index 370df3f..edbd250 100644 --- a/include/dt-bindings/pinctrl/omap.h +++ b/include/dt-bindings/pinctrl/omap.h @@ -5,6 +5,9 @@ * Copyright (C) 2009-2010 Texas Instruments */ +#ifndef _DT_BINDINGS_PINCTRL_OMAP_H +#define _DT_BINDINGS_PINCTRL_OMAP_H + /* 34xx mux mode options for each pin. See TRM for options */ #define MUX_MODE00 #define MUX_MODE11 @@ -48,3 +51,5 @@ #define PIN_OFF_INPUT_PULLDOWN (OFF_EN | OFF_PULL_EN) #define PIN_OFF_WAKEUPENABLE WAKEUP_EN +#endif + -- 1.7.5.4 ___ devicetree-discuss mailing list devicetree-disc...@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- 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: [BISECTED] 3.10-rc1 OMAP1 GPIO IRQ regression
On Mon, 20 May 2013 10:46:21 -0700, Tony Lindgren t...@atomide.com wrote:
* Tony Lindgren t...@atomide.com [130516 14:50]:
* Aaro Koskinen aaro.koski...@iki.fi [130516 14:05]:
On Thu, May 16, 2013 at 11:09:34AM -0700, Tony Lindgren wrote:
* Aaro Koskinen aaro.koski...@iki.fi [130513 13:58]:
I tested 3.10-rc1 on OMAP1 / Nokia 770, and Retu MFD probe is broken:
[2.264221] retu-mfd 2-0001: Retu v3.2 found
[2.281951] retu-mfd 2-0001: Failed to allocate IRQs: -12
[2.300140] retu-mfd: probe of 2-0001 failed with error -12
The error is coming from regmap code. According to git bisect, it is
caused by:
commit ede4d7a5b9835510fd1f724367f68d2fa4128453
Author: Jon Hunter jon-hun...@ti.com
Date: Fri Mar 1 11:22:47 2013 -0600
gpio/omap: convert gpio irq domain to linear mapping
The commit does not anymore revert cleanly, and I haven't yet tried
crafting a manual revert, so any fix proposals/ideas are welcome...
Hmm this might be a bit trickier to fix. Obviously the real solution
is to convert omap1 to SPARSE_IRQ like we did for omap2+.
For the -rc cycle, it might be possible to fix this by adding a
different irq_to_gpio() and gpio_to_irq() functions for omap1.
The commit reverts cleanly if we also revert
3513cdeccc647d41c4a9ff923af17deaaac04a66 (gpio/omap: optimise interrupt
service routine), which seems to be just some minor optimization. The
result is below, and with it 770 works again.
Hmm in this case it seems that we should just fix it rather than go back
to the old code, so let's take a look at that first.
Does the following fix it for you or do we need to fix something else
there too?
Hi Tony,
Do you want me to apply this fix? It sounds like it solves the symptoms,
but I'd like to know more about what the root cause is.
Send me your s-o-b line and I'll apply the patch
g.
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1094,6 +1094,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
const struct omap_gpio_platform_data *pdata;
struct resource *res;
struct gpio_bank *bank;
+ int irq_base;
match = of_match_device(of_match_ptr(omap_gpio_match), dev);
@@ -1135,11 +1136,23 @@ static int omap_gpio_probe(struct platform_device
*pdev)
pdata-get_context_loss_count;
}
-
- bank-domain = irq_domain_add_linear(node, bank-width,
- irq_domain_simple_ops, NULL);
- if (!bank-domain)
+ /*
+ * REVISIT: Once we have omap1 supporting SPARSE_IRQ, we can drop
+ * irq_alloc_descs() and irq_domain_add_legacy() and just do:
+ *
+ * bank-domain = irq_domain_add_linear(node, bank-width,
+ * irq_domain_simple_ops, NULL);
+ * if (!bank-domain)
+ * return -ENODEV;
+ */
+ irq_base = irq_alloc_descs(-1, 0, bank-width, 0);
+ if (irq_base 0) {
+ dev_err(dev, Couldn't allocate IRQ numbers\n);
return -ENODEV;
+ }
+
+ bank-domain = irq_domain_add_legacy(node, bank-width, irq_base,
+ 0, irq_domain_simple_ops, NULL);
if (bank-regs-set_dataout bank-regs-clr_dataout)
bank-set_dataout = _set_gpio_dataout_reg;
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
--
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] of/irq: store IRQ trigger/level in struct resource flags
On Tue, 09 Apr 2013 00:44:05 +0200, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: On 04/09/2013 12:05 AM, Rob Herring wrote: On 04/05/2013 02:48 AM, Javier Martinez Canillas wrote: This means that drivers that need the IRQ type/level flags defined in the DT won't be able to get it. But the interrupt controllers that need the information should be able to get to it via irqd_get_trigger_type. What problem exactly are you trying to fix? What driver would use this? Yes but this is not about the interrupt controller wanting this information but a device driver that is using the IORESOURCE_IRQ struct resource that has the information about the virtual IRQ associated with a GPIO-IRQ. The driver doesn't know neither care if its IRQ line is connected to a line of an real IRQ controller or to a GPIO controller that allows a GPIO line to be used as an IRQ. My understanding of the IORESOURCE_IRQ_xxx (and DMA) bits are they are ISA specific and therefore should not be used on non-ISA buses. Many TI OMAP2+ SoC based boards have an SMSC LAN911x/912x controller (drivers/net/ethernet/smsc/smsc911x.c) that is connected to the OMAP processor through its General-Purpose Memory Controller (GPMC) and this LAN driver obtain its IRQ and I/O address space from a struct resource IORESOURCE_IRQ and IORESOURCE_MEM respectively, that is filled by the DeviceTree core. It does this: irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); irq_flags = irq_res-flags IRQF_TRIGGER_MASK; Since of_irq_to_resource() doesn't fill the trigger/level flags on the IORESOURCE_IRQ struct resource, irq_flags will always be 0 regarding the value specified on the second cell of the interrupts DT property. A previous discussion about this can be found here [1]. I can't remember if there was ever a reason for not returning the IRQ flags, but I don't have any major objection to doing so if drivers find them useful. The one concern I do have however is if it will cause any problems with drivers that expect flags == IORESOURCE_IRQ without any additional flags. Any users doing that are buggy anyway, but I do want to be careful about breakage. I'll go over your patch and reply with comments. g. -- 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] of/irq: store IRQ trigger/level in struct resource flags
On Fri, 5 Apr 2013 09:48:08 +0200, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: [...] irq_of_parse_and_map() calls to irq_create_of_mapping() which calls to the correct xlate function handler according to #interrupt-cells (irq_domain_xlate_onecell or irq_domain_xlate_twocell) and to irq_set_irq_type() to set the IRQ type. But the type is never returned so it can't be saved on the IRQ struct resource flags member. This means that drivers that need the IRQ type/level flags defined in the DT won't be able to get it. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk [...] diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h index 535cecf..98aec57 100644 --- a/include/linux/of_irq.h +++ b/include/linux/of_irq.h @@ -66,6 +66,10 @@ extern int of_irq_map_one(struct device_node *device, int index, extern unsigned int irq_create_of_mapping(struct device_node *controller, const u32 *intspec, unsigned int intsize); +extern unsigned int irq_create_of_mapping_type(struct device_node *controller, +const u32 *intspec, +unsigned int intsize, +unsigned int *otype); I count 11 users of irq_create_of_mapping(). That's a managable number to update. Instead of creating a new function, please modify the existing one and split it off into a separate patch. Otherwise the patch looks fine to me. g. -- 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/6] drivers: phy: add generic PHY framework
On Tue, 16 Apr 2013 15:48:07 +0530, Kishon Vijay Abraham I kis...@ti.com
wrote:
On Tuesday 16 April 2013 01:20 AM, Grant Likely wrote:
On Mon, 15 Apr 2013 17:56:10 +0530, Kishon Vijay Abraham I kis...@ti.com
wrote:
On Monday 15 April 2013 05:04 PM, Grant Likely wrote:
On Wed, 20 Mar 2013 14:42:00 +0530, Kishon Vijay Abraham I
kis...@ti.com wrote:
We have decided not to implement the PHY layer as a separate bus layer.
The PHY provider can be part of any other bus. Making the PHY layer as a
bus will make the PHY provider to be part of multiple buses which will
lead to bad design. All we are trying to do here is keep the pool of PHY
devices under PHY class in this layer and help any controller that wants
to use the PHY to get it.
If you're using a class, then you already have your list of registered
phy devices! :-) No need to create another global list that you need to
manage.
right. We already use _class_dev_iter_ for finding the phy device.
.
.
+static struct phy *of_phy_lookup(struct device *dev, struct device_node
*node)
+{
+ struct phy *phy;
+ struct class_dev_iter iter;
+
+ class_dev_iter_init(iter, phy_class, NULL, NULL);
+ while ((dev = class_dev_iter_next(iter))) {
+ phy = container_of(dev, struct phy, dev);
+ if (node != phy-of_node)
+ continue;
+
+ class_dev_iter_exit(iter);
+ return phy;
+ }
+
+ class_dev_iter_exit(iter);
+ return ERR_PTR(-EPROBE_DEFER);
+}
.
.
however we can't get rid of the other list (phy_bind_list) where we
maintain the phy binding information. It's used for the non-dt boot case.
Why? If you're using a class, then it is always there. Why would non-DT
and DT be different in this regard? (more below)
Since there is at most a 1:N relationship between host controllers and
PHYs, there shouldn't be any need for a separate structure to describe
binding. Put the inding data into the struct phy itself. Each host
controller can have a list of phys that it is bound to.
No. Having the host controller to have a list of phys wont be a good
idea IMHO. The host controller is just an IP and the PHY to which it
will be connected can vary from board to board, platform to platform. So
ideally this binding should come from platform initialization code/dt data.
That is not what I mean. I mean the host controller instance should
contain a list of all the PHYs that are attached to it. There should not
Doesn't sound correct IMO. The host controller instance need not know
anything about the PHY instances that is connected to it. Think of it
similar to regulator, the controller wouldn't know which regulator it is
connected to, all it has to know is it just has a regulator connected to
it. It's up-to the regulator framework to give the controller the
correct regulator. It's similar here. It makes sense for me to keep a
list in the PHY framework in order for it to return the correct PHY (but
note that this list is not needed for dt boot).
With regulators and clocks it makes sense to have a global
registration place becase both implement an interconnected network
independent of the device that use them. (clocks depend on other clocks;
regulators depend on other regulators).
Phys are different. There is a 1:N relationship between host controllers
and phys, and you don't get a interconnected network of PHYs. Its a bad
idea to keep the binding data separate from the actual host controller
when there is nothing else that actually needs to use the data. It
creates a new set of data structures that need housekeeping to keep them
in sync with the actual device structures. It really is just a bad idea
and it becomes more difficult (in the non-DT case) to determine what
data is associated with a given host controller. You can't tell by
looking at the struct device.
Instead, for the non-DT case, do what we've always done for describing
connections. Put the phy reference into the host controllers
platform_data structure. That is what it is there for. That completely
eliminates the need to housekeep a new set of data structures.
g.
--
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] Documentation: dt: update properties in TI GPMC NAND example
On Tue, 9 Apr 2013 12:23:35 -0500, Jon Hunter jon-hun...@ti.com wrote: The GPMC timing properties for device-tree have been updated by adding a -ns or -ps suffix to indicate the units of time the property represents. Therefore, update the timing property names for TI GPMC NAND example. Signed-off-by: Jon Hunter jon-hun...@ti.com Acked-by: Grant Likely grant.lik...@secretlab.ca --- .../devicetree/bindings/mtd/gpmc-nand.txt | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt index e7f8d7e..6a983c1 100644 --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt @@ -56,20 +56,20 @@ Example for an AM33xx board: nand-bus-width = 16; ti,nand-ecc-opt = bch8; - gpmc,sync-clk = 0; - gpmc,cs-on = 0; - gpmc,cs-rd-off = 44; - gpmc,cs-wr-off = 44; - gpmc,adv-on = 6; - gpmc,adv-rd-off = 34; - gpmc,adv-wr-off = 44; - gpmc,we-off = 40; - gpmc,oe-off = 54; - gpmc,access = 64; - gpmc,rd-cycle = 82; - gpmc,wr-cycle = 82; - gpmc,wr-access = 40; - gpmc,wr-data-mux-bus = 0; + gpmc,sync-clk-ps = 0; + gpmc,cs-on-ns = 0; + gpmc,cs-rd-off-ns = 44; + gpmc,cs-wr-off-ns = 44; + gpmc,adv-on-ns = 6; + gpmc,adv-rd-off-ns = 34; + gpmc,adv-wr-off-ns = 44; + gpmc,we-off-ns = 40; + gpmc,oe-off-ns = 54; + gpmc,access-ns = 64; + gpmc,rd-cycle-ns = 82; + gpmc,wr-cycle-ns = 82; + gpmc,wr-access-ns = 40; + gpmc,wr-data-mux-bus-ns = 0; #address-cells = 1; #size-cells = 1; -- 1.7.10.4 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/6] Generic PHY Framework
On Wed, 20 Mar 2013 14:41:59 +0530, Kishon Vijay Abraham I kis...@ti.com wrote: Added a generic PHY framework that provides a set of APIs for the PHY drivers to create/destroy a PHY and APIs for the PHY users to obtain a reference to the PHY with or without using phandle. To obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. This framework will be of use only to devices that uses external PHY (PHY functionality is not embedded within the controller). The intention of creating this framework is to bring the phy drivers spread all over the Linux kernel to drivers/phy to increase code re-use and to increase code maintainability. Comments to make PHY as bus wasn't done because PHY devices can be part of other bus and making a same device attached to multiple bus leads to bad design. Making omap-usb2 and twl4030 to use this framework is provided as a sample. This patch series is developed on 3.9-rc3. Once the patch series gets finalised I'll resend omap-usb2 and twl4030 part based on Felipe's tree. [...] drivers/Kconfig|2 + drivers/Makefile |2 + drivers/phy/Kconfig| 13 + drivers/phy/Makefile |5 + drivers/phy/phy-core.c | 574 This looks to be very specific for USB PHYs. Are you intending it to be used for other types of PHYs, like Ethernet PHYs? If not, then this infrastruction should be named something like usb-phy so that it isn't confused with other layers, and it really should live under drivers/usb. g. -- 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/6] drivers: phy: add generic PHY framework
On Wed, 20 Mar 2013 14:42:00 +0530, Kishon Vijay Abraham I kis...@ti.com
wrote:
The PHY framework provides a set of APIs for the PHY drivers to
create/destroy a PHY and APIs for the PHY users to obtain a reference to the
PHY with or without using phandle. To obtain a reference to the PHY without
using phandle, the platform specfic intialization code (say from board file)
should have already called phy_bind with the binding information. The binding
information consists of phy's device name, phy user device name and an index.
The index is used when the same phy user binds to mulitple phys.
PHY drivers should create the PHY by passing phy_descriptor that has
describes the PHY (label, type etc..) and ops like init, exit, suspend,
resume,
poweron, shutdown.
The documentation for the generic PHY framework is added in
Documentation/phy.txt and the documentation for the sysfs entry is added
in Documentation/ABI/testing/sysfs-class-phy.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Hi Kishon,
For review purposes, I'll skip most of the implementation and focus on
the data structures. I think you need to take another look at the
approach your using. The kernel already provides a lot of support for
implementing devices and binding them to drivers that you should be
able to use...
[...]
+/**
+ * struct phy - represent the phy device
+ * @dev: phy device
+ * @label: label given to phy
+ * @type: specifies the phy type
+ * @of_node: device node of the phy
+ * @ops: function pointers for performing phy operations
+ */
+struct phy {
+ struct device dev;
+ const char *label;
+ int type;
+ struct bus_type *bus;
+ struct device_node *of_node;
+ struct phy_ops *ops;
+};
Alright, so the core of the functionality is this 'struct phy' which
tracks all the instances of PHY devices. As I understand it each
physical phy will have a 'struct phy' instance tracking it. That makes
sense.
struct phy embeds a struct device. Also good. The linux driver model
knows all about devices and how to handle them. However, most of the
rest of this structure looks to be reinventing stuff the driver model
already does.
'label' seems unnecessary. struct device embeds a struct kobject, which
means it has a name and shows up in sysfs. Is there a reason that the
device name cannot be used directly?
'type' I just don't understand. I don't see any users of it in this
patch. I only see where it is set.
'bus' absolutely should not be here. The bus type should be set in the
struct device by this subsystem when the device gets registered. There
is no reason to have a pointer to it here.
'of_node' is already in struct device
Finally, it really doesn't look right for a device object to have an
'ops' structure. The whole point of the driver model is that a struct
device doesn't encapsulate any behaviour. A device gets registers to a
bus type, and then the driver core will associate a struct device_driver
to a device that it is able to drive, and the struct device_driver is
supposed to contain any ops structure used to control the device.
+
+/**
+ * struct phy_bind - represent the binding for the phy
+ * @dev_name: the device name of the device that will bind to the phy
+ * @phy_dev_name: the device name of the phy
+ * @index: used if a single controller uses multiple phys
+ * @auto_bind: tells if the binding is done explicitly from board file or not
+ * @phy: reference to the phy
+ * @list: to maintain a linked list of the binding information
+ */
+struct phy_bind {
+ const char *dev_name;
+ const char *phy_dev_name;
+ int index;
+ int auto_bind:1;
+ struct phy *phy;
+ struct list_head list;
+};
How are PHYs attached to the system. Would the PHY be a direct child of
the host controller device, or would a PHY hang off another bus (like
i2c) for control? (this is how Ethernet phys work; they hang off the
MDIO bus, but may be attached to any Ethernet controller).
Since there is at most a 1:N relationship between host controllers and
PHYs, there shouldn't be any need for a separate structure to describe
binding. Put the inding data into the struct phy itself. Each host
controller can have a list of phys that it is bound to.
Tring to maintain a separate global list of PHY bindings isn't a good
idea. Keep it local to the host controller and the specific phys
instead.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/6] Generic PHY Framework
On Mon, 15 Apr 2013 16:06:37 +0530, Kishon Vijay Abraham I kis...@ti.com wrote: Hi, On Monday 15 April 2013 03:50 PM, Grant Likely wrote: On Wed, 20 Mar 2013 14:41:59 +0530, Kishon Vijay Abraham I kis...@ti.com wrote: Added a generic PHY framework that provides a set of APIs for the PHY drivers to create/destroy a PHY and APIs for the PHY users to obtain a reference to the PHY with or without using phandle. To obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. This framework will be of use only to devices that uses external PHY (PHY functionality is not embedded within the controller). The intention of creating this framework is to bring the phy drivers spread all over the Linux kernel to drivers/phy to increase code re-use and to increase code maintainability. Comments to make PHY as bus wasn't done because PHY devices can be part of other bus and making a same device attached to multiple bus leads to bad design. Making omap-usb2 and twl4030 to use this framework is provided as a sample. This patch series is developed on 3.9-rc3. Once the patch series gets finalised I'll resend omap-usb2 and twl4030 part based on Felipe's tree. [...] drivers/Kconfig|2 + drivers/Makefile |2 + drivers/phy/Kconfig| 13 + drivers/phy/Makefile |5 + drivers/phy/phy-core.c | 574 This looks to be very specific for USB PHYs. Are you intending it to be used for other types of PHYs, like Ethernet PHYs? If not, then this Not really. This can be used by USB, SATA and Sylwester was planning to use it for video PHY's. So what are the common bits that are shared between those phys? Merely matching phys to controllers? Besides that, each of those devices have very different behaviour. You wouldn't be able to attach any interface logic to the generic struct phy. I don't think it makes a whole lot of sense to lump them all into the same type of registration. g. -- 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 10/18] ARM: OMAP2+: Add function to read GPMC settings from device-tree
On Tue, 19 Mar 2013 11:35:48 -0500, Jon Hunter jon-hun...@ti.com wrote: Adds a function to read the various GPMC chip-select settings from device-tree and store them in the gpmc_settings structure. Update the GPMC device-tree binding documentation to describe these options. Signed-off-by: Jon Hunter jon-hun...@ti.com Tested-by: Ezequiel Garcia ezequiel.gar...@free-electrons.com --- Documentation/devicetree/bindings/bus/ti-gpmc.txt | 23 arch/arm/mach-omap2/gpmc.c| 40 + arch/arm/mach-omap2/gpmc.h|2 ++ 3 files changed, 65 insertions(+) diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt index 5ddb2e9..6fde1cf 100644 --- a/Documentation/devicetree/bindings/bus/ti-gpmc.txt +++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt @@ -65,6 +65,29 @@ The following are only applicable to OMAP3+ and AM335x: - gpmc,wr-access - gpmc,wr-data-mux-bus +GPMC chip-select settings properties for child nodes. All are optional. + +- gpmc,burst-length Page/burst length. Must be 4, 8 or 16. +- gpmc,burst-wrapEnables wrap bursting +- gpmc,burst-readEnables read page/burst mode +- gpmc,burst-write Enables write page/burst mode +- gpmc,device-nand Device is NAND +- gpmc,device-width Total width of device(s) connected to a GPMC + chip-select in bytes. The GPMC supports 8-bit + and 16-bit devices and so this property must be + 1 or 2. I would suggest specifying the actual number of bits. ie. 8 or 16. There is some precidence for that already in DT bindings. +- gpmc,mux-add-data Address and data multiplexing configuration. + Valid values are 1 for address-address-data + multiplexing mode and 2 for address-data + multiplexing mode. +- gpmc,sync-read Enables synchronous read. Defaults to asynchronous + is this is not set. 'if'? +- gpmc,sync-writeEnables synchronous writes. Defaults to asynchronous + is this is not set. +- gpmc,wait-pin Wait-pin used by client. Must be less than + gpmc,num-waitpins. +- gpmc,wait-on-read Enables wait monitoring on reads. +- gpmc,wait-on-write Enables wait monitoring on writes. Otherwise looks okay to me. g. -- 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/6] drivers: phy: add generic PHY framework
On Mon, 15 Apr 2013 17:56:10 +0530, Kishon Vijay Abraham I kis...@ti.com
wrote:
Hi,
On Monday 15 April 2013 05:04 PM, Grant Likely wrote:
On Wed, 20 Mar 2013 14:42:00 +0530, Kishon Vijay Abraham I kis...@ti.com
wrote:
The PHY framework provides a set of APIs for the PHY drivers to
create/destroy a PHY and APIs for the PHY users to obtain a reference to
the
PHY with or without using phandle. To obtain a reference to the PHY without
using phandle, the platform specfic intialization code (say from board
file)
should have already called phy_bind with the binding information. The
binding
information consists of phy's device name, phy user device name and an
index.
The index is used when the same phy user binds to mulitple phys.
PHY drivers should create the PHY by passing phy_descriptor that has
describes the PHY (label, type etc..) and ops like init, exit, suspend,
resume,
poweron, shutdown.
The documentation for the generic PHY framework is added in
Documentation/phy.txt and the documentation for the sysfs entry is added
in Documentation/ABI/testing/sysfs-class-phy.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Hi Kishon,
For review purposes, I'll skip most of the implementation and focus on
the data structures. I think you need to take another look at the
approach your using. The kernel already provides a lot of support for
implementing devices and binding them to drivers that you should be
able to use...
[...]
+/**
+ * struct phy - represent the phy device
+ * @dev: phy device
+ * @label: label given to phy
+ * @type: specifies the phy type
+ * @of_node: device node of the phy
+ * @ops: function pointers for performing phy operations
+ */
+struct phy {
+ struct device dev;
+ const char *label;
+ int type;
+ struct bus_type *bus;
+ struct device_node *of_node;
+ struct phy_ops *ops;
+};
Alright, so the core of the functionality is this 'struct phy' which
tracks all the instances of PHY devices. As I understand it each
physical phy will have a 'struct phy' instance tracking it. That makes
sense.
struct phy embeds a struct device. Also good. The linux driver model
knows all about devices and how to handle them. However, most of the
rest of this structure looks to be reinventing stuff the driver model
already does.
'label' seems unnecessary. struct device embeds a struct kobject, which
means it has a name and shows up in sysfs. Is there a reason that the
device name cannot be used directly?
hmm.. the label name is supposed to be a simpler name than device name.
Say for instance omap-usb2.1.auto device name can simply be
omap-usb2-phy. Further the device name while using dt will have
register address in it. However it's used only for displaying the label
in sysfs entry (/sys/class/phy/phy/label).
I wouldn't go mucking with names in that way. Stick with one name and
drop the separate label. Otherwise you introduce addtional sources of
confusion.
'type' I just don't understand. I don't see any users of it in this
patch. I only see where it is set.
yeah. Was planning to remove this in my next version (btw the latest is
version 5).
'bus' absolutely should not be here. The bus type should be set in the
struct device by this subsystem when the device gets registered. There
is no reason to have a pointer to it here.
right. I had removed it in version 5 of this patch series.
'of_node' is already in struct device
I wasn't sure if we can manually assign the of_node of one device to
of_node of an another device. Here the of_node comes from _phy provider_.
There is no problem with multiple devices referencing the same node. The
only time it may cause problems is when two devices of the same bus type
are referencing the same of_node. In that situation the device will get
probed more than once. You're not in that situation.
Finally, it really doesn't look right for a device object to have an
'ops' structure. The whole point of the driver model is that a struct
device doesn't encapsulate any behaviour. A device gets registers to a
bus type, and then the driver core will associate a struct device_driver
We have decided not to implement the PHY layer as a separate bus layer.
The PHY provider can be part of any other bus. Making the PHY layer as a
bus will make the PHY provider to be part of multiple buses which will
lead to bad design. All we are trying to do here is keep the pool of PHY
devices under PHY class in this layer and help any controller that wants
to use the PHY to get it.
If you're using a class, then you already have your list of registered
phy devices! :-) No need to create another global list that you need to
manage.
You really need to be careful here though. By lumping all the phy types
into a single pot your glossing over
Re: [PATCH 2/5] capemgr: Add beaglebone's cape driver bindings
;
+ };
+ };
+
+ slots {
+ slot@0 {
+ eeprom = cape_eeprom0;
+ };
+
+ slot@1 {
+ eeprom = cape_eeprom1;
+ };
+
+ slot@2 {
+ eeprom = cape_eeprom2;
+ };
+
+ slot@3 {
+ eeprom = cape_eeprom3;
+ };
+ };
+
+ /* mapping between board names and dtb objects */
+ capemaps {
+ /* Weather cape */
+ cape@0 {
+ part-number = BB-BONE-WTHR-01;
+ version@00A0 {
+ version = 00A0;
+ dtbo = cape-bone-weather-00A0.dtbo;
+ };
+ };
+ };
+};
+
+Example of the override syntax when used on a bone compatible foo board.
+
+{
+ ...
+
+ baseboardmaps {
+ ...
+ baseboard_beaglebone: board@0 {
+ board-name = A335FOO;
+ compatible-name = ti,foo;
+ };
+
+ slot@6 {
+ ti,cape-override;
+ compatible = ti,foo;
+ board-name = FOO-hardcoded;
+ version = 00A0;
+ manufacturer = Texas Instruments;
+ part-number = BB-BONE-FOO-01;
+ };
+ };
+
+};
+
--
1.7.12
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
--
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] capemgr: Beaglebone DT overlay based cape manager
On Tue, 8 Jan 2013 12:10:20 +0200, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Hi Lee, On Jan 8, 2013, at 12:00 PM, Lee Jones wrote: At the end of the line, some kind of hardware glue is going to be needed. I just feel that drawing from a sample size of 1 (maybe 2 if I get to throw in the beagleboard), it is a bit premature to think about making it overly general, besides the part that are obviously part of the infrastructure (like the DT overlay stuff). What I'm getting at, is that we need some user experience about this, before going away and creating structure out of possible misconception about the uses. IMHO stuff like this will be needed by many SoCs. Some examples of similar things for omaps that have eventually become generic frameworks have been the clock framework, USB OTG support, runtime PM, pinmux framework and so on. So I suggest a minimal generic API from the start as that will make things a lot easier in the long run. I agree. The ux500 platform already has the concept of user interface boards, which currently is not well integrated into devicetree. I believe Sascha mentioned that Pengutronix had been shipping some other systems with add-on boards and generating device tree binaries from source for each combination. Ideally, both of the above should be able to use the same DT overlay logic as BeagleBone, and I'm sure there are more of those. Hmm, I see. I will need some more information about the interface of the 'user interface boards'. I.e. how is the board identified, what is typically present on those boards, etc. User Interface Boards are mearly removable PCBs which are interchangeable amongst various hardware platforms. They are connected via numerous connectors which carry all sorts of different data links; i2c, spi, rs232, etc. The UIB I'm looking at right now has a touchscreen, speakers, a key pad, leds, jumpers, switches and a bunch of sensors. You can find a small example of how we interface to these by viewing 'arch/arm/boot/dts/stuib.dtsi'. To add a UIB to a particular build, we currently include it as a *.dtsi from a platform's dts file. I see. What I'm asking about is whether there's a method where you can read an EEPROM, or some GPIO code combination where I can find out what kind of board is plugged each time. If there is not, there is no way to automatically load the overlays; you can always use the kernel command line, or have the a user space application to request the loading of a specific board's overlay. In this case the best thing to do is announce the availability of the expansion via a request_firmware() call and let udev handle supplying the correct overlay file. g. -- 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/6] OF: Export all DT proc update functions
On Tue, 19 Mar 2013 13:42:32 +0200, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Hi Grant, The 3rd patch is in preparation for some patches I have in WIP that would allow drivers to set notifications for properties that are changed in runtime. Okay, submit it with that series then please. g. -- 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 5/6] OF: Introduce Device Tree resolve support.
On Tue, 19 Mar 2013 13:51:01 +0200, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Hi Grant, On Mar 16, 2013, at 11:24 AM, Grant Likely wrote: On Wed, 23 Jan 2013 12:58:02 +0200, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Hi David, On Jan 23, 2013, at 6:40 AM, David Gibson wrote: Ok. Nonetheless it's not hard to avoid a recursive approach here. How can I find the maximum phandle value of a subtree without using recursion. Note that the whole function is just 6 lines long. It's a failure in the existing kernel DT data structures. We need a hash lookup for the phandles to eliminate the search entirely. Then you'd be able to allocated new phandles on the fly easily and resolve phandles without searching the whole tree (which has always been horrible). Yes, it is pretty obvious that the in-kernel data structures are sub-optimal. But I was not after modifying them, since that's a different kind of problem. Think about it this way; fixing up that aspect of the data structure makes the job you're trying to do a lot easier. I don't feel bad about asking you to add a radix tree for phandle lookups when it makes your patches a whole lot better. :-) Since we're having a 'sub-optimal' data structures, I'd like to point out that the usage of of_find_by_name(), mostly by drivers trying to find a child of their own node, works by a lucky accident of how the device nodes are instantiated by the flat tree loader. Most of the use cases should be replaced by a call to of_get_child_by_name() which does the right thing. It is true. In fact, calling of_find_node_by_name() when using .dtb is most likely a bug since using node name to determine behaviour is strongly discouraged. Fair enough, but be warned that phandle resolution the overlay feature is mostly useless. In actual practice the amount of driver nodes that can be overlaid without a single case of referencing phandles outside (or within) their own blob is close to zero. That's not what I'm saying. I'm saying that (at least for now) we should require the overlay to already know the phandles from the parent and to refuse to load an overlay that defines phandles already in use in the base. Overlays do become usable at that point. A mechanism for phandle resolution so that conflicts can be found and resolved can be added as a feature enhancement. By splitting it out you'll be able to get the overlay feature merged even if we don't have agreement on the resolution mechanism yet. g. -- 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 5/6] OF: Introduce Device Tree resolve support.
On Wed, 23 Jan 2013 12:58:02 +0200, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Hi David, On Jan 23, 2013, at 6:40 AM, David Gibson wrote: Ok. Nonetheless it's not hard to avoid a recursive approach here. How can I find the maximum phandle value of a subtree without using recursion. Note that the whole function is just 6 lines long. It's a failure in the existing kernel DT data structures. We need a hash lookup for the phandles to eliminate the search entirely. Then you'd be able to allocated new phandles on the fly easily and resolve phandles without searching the whole tree (which has always been horrible). That said, I'd like to punt on the whole phandle resolution thing. The DT overlay support can be merged without the phandle resolution support if the core rejects any overlays with phandle collisions. g. -- 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/6] OF: Export all DT proc update functions
On Fri, 4 Jan 2013 21:31:07 +0200, Pantelis Antoniou pa...@antoniou-consulting.com wrote: There are other users for the proc DT functions. Export them. Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com Actually, I cannot find the user of this patch. Why is it needed? g. -- 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/6] OF: Export all DT proc update functions
On Fri, 4 Jan 2013 21:31:07 +0200, Pantelis Antoniou pa...@antoniou-consulting.com wrote: There are other users for the proc DT functions. Export them. Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com Hi Pantelis. Patches 1 2 look good. No comments there. This patch bothers me. The manipulation of the proc entries is part and parcel of adding and removing nodes. The real problem seems to be that the node addition/removal APIs need to be made usable by the overlay code. g. -- 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/5] gpio/omap: Add DT support to GPIO driver
On Tue, 26 Feb 2013 17:01:22 -0600, Jon Hunter jon-hun...@ti.com wrote: On 02/26/2013 04:44 PM, Stephen Warren wrote: On 02/26/2013 03:40 PM, Jon Hunter wrote: On 02/26/2013 04:01 AM, Javier Martinez Canillas wrote: Are you requesting the gpio anywhere? If not then this is not going to work as-is. This was discussed fairly recently [1] and the conclusion was that the gpio needs to be requested before we can use as an interrupt. That seems wrong; the GPIO/IRQ driver should handle this internally. The Ethernet driver shouldn't know/care whether the interrupt it's given is some form of dedicated interrupt or a GPIO-based interrupt, and even if it somehow did, there's no irq_to_gpio() any more, so the driver can't tell which GPIO ID it should request, unless it's given yet another property to represent this. I agree that ideally this should be handled internally. Did you read the discussion on the thread that I referenced [1]? If you have any thoughts we are open to ideas :-) I'm on an airplane right now, but I agree 100% with Stephen. I'll try to remember to go read that thread and respond, but this falls firmly in the its-a-bug category for me. :-) g. -- 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] gpio/omap: convert gpio irq domain to linear mapping
On Fri, 1 Mar 2013 11:22:47 -0600, Jon Hunter jon-hun...@ti.com wrote: Currently the OMAP GPIO driver uses a legacy mapping for the GPIO IRQ domain. This is not necessary because we do not need to assign a specific interrupt number to the GPIO IRQ domain. Therefore, convert the OMAP GPIO driver to use a linear mapping instead. Please note that this also allows to simplify the logic in the OMAP gpio_irq_handler() routine, by using irq_find_mapping() to obtain the virtual irq number from the GPIO bank and bank index. Reported-by: Linus Walleij linus.wall...@linaro.org Signed-off-by: Jon Hunter jon-hun...@ti.com Applied, thanks. g. -- 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/2] gpio/omap: warn if bank is not enabled on setting irq type
On Fri, 1 Mar 2013 11:22:48 -0600, Jon Hunter jon-hun...@ti.com wrote: For OMAP devices, if a gpio is being used as an interrupt source but has not been requested by calling gpio_request(), a call to request_irq() may cause the kernel hang because the gpio bank may be disabled and hence the register access will fail. To prevent such hangs, test for this case and warn if this is detected. Signed-off-by: Jon Hunter jon-hun...@ti.com Applied, thanks. g. -- 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 RFC 1/7] platform: add a device node
On Sun, Feb 10, 2013 at 9:37 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote: I knew this would be controversial and that's why I didn't mean it to be a patch but a RFC :) The problem basically is that you have to associate the platform device with its corresponding DT device node because it can be used in the driver probe function. When DT is being used, doesn't DT create the platform devices for you, with the device node already set correctly? Manually creating platform devices and adding DT nodes to it sounds like the wrong thing to be doing. Right; I'd expect your platform device creation to go through the of_platform_populate() route. What is your use-case? g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- 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 RFC 1/7] platform: add a device node
On Sun, Feb 10, 2013 at 11:35 AM, Javier Martinez Canillas
martinez.jav...@gmail.com wrote:
On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
I knew this would be controversial and that's why I didn't mean it to be a
patch
but a RFC :)
The problem basically is that you have to associate the platform device
with its
corresponding DT device node because it can be used in the driver probe
function.
When DT is being used, doesn't DT create the platform devices for you,
with the device node already set correctly?
Well they usually do but not always just like usually you have a
platform_device in your board code and call platform_device_register().
But in some configurations you can't just define the platform_device
required resources (I/O memory, IRQ, etc) as static platform data.
In some cases you need a level of indirection.
In this particular case a SMSC ethernet chip is connected to an
OMAP3 processor through its General-Purpose Memory Controller.
You can't just define the I/O memory used by the device since you first
need to request that address to the GPMC. The same happens with the
IRQ line since a OMAP GPIO pin is used so you have to first configure
the GPIO line as input.
So in platform code you call to gpmc_smsc911x_init() passing all the
GPMC specific configurations (GPIO used for IRQ, GPMC chip select, etc)
Then gpmc_smsc911x_init() does all the needed setup, fills a platform_data
for the real platform_device and calls platform_device_register_resndata().
From the smsc911x platform_driver probe function point of view it just have
resources and doesn't know if it's I/O memory is directly mapped or is
through a memory controller as the GPMC. It also doesn't know if the IRQ is
a GPIO or not.
It's still the same difference as far as the device is concerned.
External bus chip-select lines are well understood. The key here is to
encode the CS line number into the reg property of the child node so
that the GPMC driver has the information it needs to configure the
chip selects. You do this by setting #address-cells to '2' in the GPMC
node, and use the ranges property to map from the gpmc address space
to the cpu address space. Like so (if you had 2 Ethernet controllers)
gpmc {
#address-cells = 2; // First cell is CS#, second
cell is offset from CS base
#size-cells = 1;
compatible = ti,gpmc;
ranges = 0 0 0xf100 0x1000, //CS0 mapped to
0xf100..0xf1000fff
1 0 0xf1001000 0x1000; //CS1 mapped
to 0xf1001000..0xf1001fff
ethernet@0,0 {
compatible = smsc,91c111;
reg = 0 0 0x1000;
}
ethernet@1,0 {
compatible = smsc,91c111;
reg = 1 0 0x1000;
}
}
The GPMC driver can use the information in the ranges property for
setting up the chip select mappings. For the smsc,91c111 driver the
mapping becomes transparent.
You can see another example of this in
arch/powerpc/boot/dts/media5200.dts in the localbus node.
g.
--
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] gpio: twl4030: Cache the direction and output states in private data
On Thu, 10 Jan 2013 14:09:35 +0100, Peter Ujfalusi peter.ujfal...@ti.com wrote: Hi Linus, On 01/10/2013 11:41 AM, Linus Walleij wrote: On Thu, Dec 20, 2012 at 10:44 AM, Peter Ujfalusi peter.ujfal...@ti.com wrote: Use more coherent locking in the driver. Use bitfield to store the GPIO direction and if the pin is configured as output store the status also in a bitfiled. In this way we can just look at these bitfields when we need information about the pin status and only reach out to the chip when it is needed. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com Acked-by: Linus Walleij linus.wall...@linaro.org --- Hi Grant, Changes sicne v2: - Fixed the mutex_unlock found by Michael. - Removed the debug prints addedd by v2 patch (remains from debugging) - Removed one blank line between includes and the first comment section. Sorry Peter this must have been missed somehow. This does not apply to the current v3.8-rc3, could you respin this on top of Torvalds' tree? Grant applied the patch which this one depends on: [1] https://patchwork.kernel.org/patch/1844511/ I had applied it, never pushed the tree out because I wasn't able to test my kernel tree for a couple of weeks due to travel. I saw the patch in Linus' tree and pulled it out of mine before I pushed it out. g. -- 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] drivers/gpio: using common order: let 'static const' instead of 'const static'
On Wed, 6 Feb 2013 16:17:24 +0530, Santosh Shilimkar santosh.shilim...@ti.com wrote: On Wednesday 06 February 2013 04:14 PM, Chen Gang wrote: 'const static ' is not a common order, better to use 'static const' instead. building: make EXTRA_CFLAGS=-W ARCH=arm drivers/gpio/gpio-omap.c:1479: warning: 'static' is not at beginning of declaration drivers/gpio/gpio-omap.c:1485: warning: 'static' is not at beginning of declaration drivers/gpio/gpio-omap.c:1491: warning: 'static' is not at beginning of declaration Signed-off-by: Chen Gang gang.c...@asianux.com Cc: Santosh Shilimkar santosh.shilim...@ti.com --- Thanks for update. Acked-by: Santosh Shilimkar santosh.shilim...@ti.com Applied, thanks. g. -- 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] mmc: omap_hsmmc: Enable SDIO IRQ using a GPIO in idle mode.
On Thu, 20 Dec 2012 23:12:12 +0100, Andreas Fenkart
andreas.fenk...@streamunlimited.com wrote:
Without functional clock the omap_hsmmc module can't forward
SDIO IRQs to the system. This patch reconfigures dat1 line
as a gpio while the fclk is off. And uses SDIO IRQ detection of
the module, while fclk is present.
Signed-off-by: Andreas Fenkart andreas.fenk...@streamunlimited.com
---
.../devicetree/bindings/mmc/ti-omap-hsmmc.txt | 42
arch/arm/plat-omap/include/plat/mmc.h |4 +
drivers/mmc/host/omap_hsmmc.c | 219
++--
3 files changed, 247 insertions(+), 18 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
index d1b8932..4d57637 100644
--- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
+++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
@@ -24,6 +24,29 @@ One tx and one rx pair is required.
dma-names: DMA request names. These strings correspond 1:1 with
the ordered pairs in dmas. The RX request must be rx and the
TX request must be tx.
+ti,cirq-gpio: When omap_hsmmc module is suspended, its functional
+clock is turned off. Without fclk it can't forward SDIO IRQs to the
+system. For that to happen, it needs to tell the PRCM to restore
+its fclk, which is done through the swakeup line.
+
+ --
+ | PRCM |
+ --
+| ^
+ fclk | | swakeup
+v |
+ --- --
+ -- IRQ -- | hsmmc | -- CIRQ -- | card |
+ --- --
+
+The problem is, that on the AM335x family the swakeup line is
+missing, it has not been routed from the module to the PRCM.
+The way to work around this, is to reconfigure the dat1 line as a
+GPIO upon suspend. Beyond this option you also need to set named
+states default and idle in the .dts file for the pins, using
+pinctrl-single.c. The MMC driver will then then toggle between
+default and idle during the runtime.
+
Examples:
@@ -53,3 +76,22 @@ Examples:
edma 25;
dma-names = tx, rx;
};
+
+[am335x with with gpio for sdio irq]
+
+ mmc1_cirq_pin: pinmux_cirq_pin {
+ pinctrl-single,pins =
+ 0x0f8 0x3f /* MMC0_DAT1 as GPIO2_28 */
+ ;
+ };
+
+ mmc1: mmc@4806 {
+ pinctrl-names = default, idle;
+ pinctrl-0 = mmc1_pins;
+ pinctrl-1 = mmc1_cirq_pin;
+ ti,cirq-gpio = gpio3 28 0;
+ ti,non-removable;
+ bus-width = 4;
+ vmmc-supply = ldo2_reg;
+ vmmc_aux-supply = vmmc;
+ };
Binding looks reasonable.
Reviewed-by: Grant Likely grant.lik...@secretlab.ca
--
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/3] gpio: twl4030: TODO comment to remove the PWMA/B (LEDA/B) handling
On Thu, Dec 20, 2012 at 9:23 AM, Peter Ujfalusi peter.ujfal...@ti.com wrote: On 12/19/2012 06:07 PM, Grant Likely wrote: On Thu, 6 Dec 2012 11:52:07 +0100, Peter Ujfalusi peter.ujfal...@ti.com wrote: This GPIO driver should not configure anything else then GPIOs. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com I'm not sure if this is the right direction. I actually have no problem with a single driver that registers itself with multiple interfaces (ie. GPIO and PWM) if it makes sense for it to do so. I suspec that a lot of the multifunction device drivers break things up more than is strictly necessary. We have PWM drivers for these IPs. As you remember this is the reason I started to work on the gpio-pwm driver so we can have cleaner, more generic way to map a PWM as a gpio. I really don't like the idea of having the same PWM code sitting in various places in the kernel just because it was easier to hack it like that rather then to make an effort for a clean implementation. The PWM handling in the gpio-twl4030 driver is a prime example of this IMHO. It is just a shortcut, nothing else. Ah, right. (there's nothing wrong with my memory, it's just short) :-p g. -- 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 2/3] gpio: twl4030: Cache the direction and output states in private data
;
+ if (priv-direction BIT(offset))
+ status = priv-out_state BIT(offset);
else
- status = cached_leden LEDEN_LEDBON;
+ status = twl4030_get_gpio_datain(offset);
- return (status 0) ? 0 : status;
+ ret = (status = 0) ? 0 : 1;
+out:
+ mutex_unlock(priv-mutex);
+ dev_err(chip-dev, %s: offset %d value %d\n, __func__, offset, ret);
+ return ret;
}
-static int twl_direction_out(struct gpio_chip *chip, unsigned offset, int
value)
+static void twl_set(struct gpio_chip *chip, unsigned offset, int value)
{
- if (offset TWL4030_GPIO_MAX) {
+ struct gpio_twl4030_priv *priv = to_gpio_twl4030(chip);
+
+ dev_err(chip-dev, %s: offset %d value %d\n, __func__, offset, value);
+ mutex_lock(priv-mutex);
+ if (offset TWL4030_GPIO_MAX)
twl4030_set_gpio_dataout(offset, value);
- return twl4030_set_gpio_direction(offset, 0);
- } else {
+ else
twl4030_led_set_value(offset - TWL4030_GPIO_MAX, value);
- return 0;
- }
+
+ if (value)
+ priv-out_state |= BIT(offset);
+ else
+ priv-out_state = ~BIT(offset);
+
+ mutex_unlock(priv-mutex);
}
-static void twl_set(struct gpio_chip *chip, unsigned offset, int value)
+static int twl_direction_out(struct gpio_chip *chip, unsigned offset, int
value)
{
+ struct gpio_twl4030_priv *priv = to_gpio_twl4030(chip);
+
+ dev_err(chip-dev, %s: offset %d value %d\n, __func__, offset, value);
+ mutex_lock(priv-mutex);
if (offset TWL4030_GPIO_MAX)
twl4030_set_gpio_dataout(offset, value);
- else
- twl4030_led_set_value(offset - TWL4030_GPIO_MAX, value);
+
+ priv-direction |= BIT(offset);
+ mutex_unlock(priv-mutex);
+
+ twl_set(chip, offset, value);
+
+ return 0;
}
static int twl_to_irq(struct gpio_chip *chip, unsigned offset)
@@ -469,6 +500,8 @@ no_irqs:
priv-gpio_chip.ngpio = TWL4030_GPIO_MAX;
priv-gpio_chip.dev = pdev-dev;
+ mutex_init(priv-mutex);
+
if (node)
pdata = of_gpio_twl4030(pdev-dev);
--
1.8.0
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
--
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/3] gpio: twl4030: TODO comment to remove the PWMA/B (LEDA/B) handling
On Thu, 6 Dec 2012 11:52:07 +0100, Peter Ujfalusi peter.ujfal...@ti.com wrote: This GPIO driver should not configure anything else then GPIOs. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com I'm not sure if this is the right direction. I actually have no problem with a single driver that registers itself with multiple interfaces (ie. GPIO and PWM) if it makes sense for it to do so. I suspec that a lot of the multifunction device drivers break things up more than is strictly necessary. I'll still apply this if you think it is the right direction, but I wanted to throw that though out there for consideration. g. --- drivers/gpio/gpio-twl4030.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c index a38e6e9c..1e9f08c4 100644 --- a/drivers/gpio/gpio-twl4030.c +++ b/drivers/gpio/gpio-twl4030.c @@ -47,6 +47,7 @@ * intended to support multiple hosts. * * There are also two LED pins used sometimes as output-only GPIOs. + * TODO: Handling of PWMA/B (LEDA/B) should be removed from this GPIO driver! */ /* genirq interfaces are not available to modules */ @@ -131,6 +132,7 @@ static inline int gpio_twl4030_read(u8 address) /*--*/ +/* TODO: Handling of PWMA/B (LEDA/B) should be removed from this GPIO driver! */ static u8 cached_leden; /* The LED lines are open drain outputs ... a FET pulls to GND, so an -- 1.8.0 -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- 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/3] gpio: twl4030: Introduce private structure to store variables needed runtime
;
struct device_node *node = pdev-dev.of_node;
+ struct gpio_twl4030_priv *priv;
int ret, irq_base;
+ priv = devm_kzalloc(pdev-dev, sizeof(struct gpio_twl4030_priv),
+ GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
/* maybe setup IRQs */
if (is_module()) {
dev_err(pdev-dev, can't dispatch IRQs from modules\n);
@@ -445,12 +461,13 @@ static int __devinit gpio_twl4030_probe(struct
platform_device *pdev)
if (ret 0)
return ret;
- twl4030_gpio_irq_base = irq_base;
+ priv-irq_base = irq_base;
no_irqs:
- twl_gpiochip.base = -1;
- twl_gpiochip.ngpio = TWL4030_GPIO_MAX;
- twl_gpiochip.dev = pdev-dev;
+ priv-gpio_chip = template_chip;
+ priv-gpio_chip.base = -1;
+ priv-gpio_chip.ngpio = TWL4030_GPIO_MAX;
+ priv-gpio_chip.dev = pdev-dev;
if (node)
pdata = of_gpio_twl4030(pdev-dev);
@@ -481,23 +498,23 @@ no_irqs:
* is (still) clear if use_leds is set.
*/
if (pdata-use_leds)
- twl_gpiochip.ngpio += 2;
+ priv-gpio_chip.ngpio += 2;
- ret = gpiochip_add(twl_gpiochip);
+ ret = gpiochip_add(priv-gpio_chip);
if (ret 0) {
dev_err(pdev-dev, could not register gpiochip, %d\n, ret);
- twl_gpiochip.ngpio = 0;
+ priv-gpio_chip.ngpio = 0;
gpio_twl4030_remove(pdev);
goto out;
}
- twl4030_gpio_base = twl_gpiochip.base;
+ platform_set_drvdata(pdev, priv);
if (pdata pdata-setup) {
int status;
- status = pdata-setup(pdev-dev,
- twl4030_gpio_base, TWL4030_GPIO_MAX);
+ status = pdata-setup(pdev-dev, priv-gpio_chip.base,
+ TWL4030_GPIO_MAX);
if (status)
dev_dbg(pdev-dev, setup -- %d\n, status);
}
@@ -510,18 +527,19 @@ out:
static int gpio_twl4030_remove(struct platform_device *pdev)
{
struct twl4030_gpio_platform_data *pdata = pdev-dev.platform_data;
+ struct gpio_twl4030_priv *priv = platform_get_drvdata(pdev);
int status;
if (pdata pdata-teardown) {
- status = pdata-teardown(pdev-dev,
- twl4030_gpio_base, TWL4030_GPIO_MAX);
+ status = pdata-teardown(pdev-dev, priv-gpio_chip.base,
+ TWL4030_GPIO_MAX);
if (status) {
dev_dbg(pdev-dev, teardown -- %d\n, status);
return status;
}
}
- status = gpiochip_remove(twl_gpiochip);
+ status = gpiochip_remove(priv-gpio_chip);
if (status 0)
return status;
--
1.8.0
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
--
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 2/3] gpio: twl4030: Cache the direction and output states in private data
On Wed, 19 Dec 2012 21:53:23 +0100, Michael Trimarchi
mich...@amarulasolutions.com wrote:
Hi
Grant Likely grant.lik...@secretlab.ca wrote:
On Thu, 6 Dec 2012 11:52:06 +0100, Peter Ujfalusi
peter.ujfal...@ti.com wrote:
Use more coherent locking in the driver. Use bitfield to store the
GPIO
direction and if the pin is configured as output store the status
also in a
bitfiled.
In this way we can just look at these bitfields when we need
information
about the pin status and only reach out to the chip when it is
needed.
Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com
Applied, thanks
g.
@@ -279,64 +276,98 @@ static void twl_free(struct gpio_chip *chip,
unsigned offset)
{
struct gpio_twl4030_priv *priv = to_gpio_twl4030(chip);
+ mutex_lock(priv-mutex);
if (offset = TWL4030_GPIO_MAX) {
twl4030_led_set_value(offset - TWL4030_GPIO_MAX, 1);
I have the mobile but where is the unlock here?
Good catch. I've dropped the patch. Peter, please resend a fixed-up version.
g.
--
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] OMAP GPIO - don't wake from suspend unless requested.
On Fri, 14 Dec 2012 18:05:53 +1100, NeilBrown ne...@suse.de wrote: On Mon, 10 Sep 2012 10:57:07 -0700 Kevin Hilman khil...@deeprootsystems.com wrote: OK thanks, I'll queue this up for v3.6-rc as this should qualify as a regression. I don't think this did actually get queued. At least I'm still seeing the bug in 3.7 and I cannot see a patch in the git history that looks right. But then I don't remember what we ended up with - it was 3 months ago!!! This is the last thing I can find in my email history - it still seems to apply and still seems to work. NeilBrown Kevin, let me know if I need to do anything here. Since you might have it in one of you're trees, I'm not going to do anything unless I hear from you. g. -- 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 5/5] ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
On Fri, 14 Dec 2012 11:36:44 +0100, Daniel Mack zon...@gmail.com wrote: This patch adds basic DT bindings for OMAP GPMC. The actual peripherals are instantiated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND. Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them. Successfully tested on an AM33xx board. Signed-off-by: Daniel Mack zon...@gmail.com For all patches in this series: Acked-by: Grant Likely grant.lik...@secretlab.ca -- 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/2] spi: devicetree: add support for loopback mode
On Sat, 15 Dec 2012 16:55:46 +0200, Felipe Balbi ba...@ti.com wrote: On Sat, Dec 15, 2012 at 12:32:24AM +, Grant Likely wrote: On Wed, 12 Dec 2012 10:46:00 +0200, Felipe Balbi ba...@ti.com wrote: there are a few spi master drivers which make use of that flag but there is no way to pass it through devicetree. This patch just creates a way to pass SPI_LOOP via devicetree. I don't understand how this would be useful since loopback mode is really just a test feature. Is there any reason to do loopback for something other than test? I think it would be better to add a sysfs or debugfs property to manipulate the SPI_LOOP flag from userspace. What do you think? might be nicer in the long run, indeed. Want me to look into it, or do you wanna do it yourself ? Yes, please look into it. After all, you're the one who needs the feature/ :-) g. -- 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] spi: omap2: disable DMA requests before complete()
On Wed, 12 Dec 2012 10:45:59 +0200, Felipe Balbi ba...@ti.com wrote: No actual errors have been found for completing before disabling DMA request lines, but it just looks more semantically correct that on our DMA callback we quiesce the whole thing before stating transfer is finished. Signed-off-by: Felipe Balbi ba...@ti.com Applied, thanks. g. --- drivers/spi/spi-omap2-mcspi.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c index b610f52..68446db 100644 --- a/drivers/spi/spi-omap2-mcspi.c +++ b/drivers/spi/spi-omap2-mcspi.c @@ -298,10 +298,10 @@ static void omap2_mcspi_rx_callback(void *data) struct omap2_mcspi *mcspi = spi_master_get_devdata(spi-master); struct omap2_mcspi_dma *mcspi_dma = mcspi-dma_channels[spi-chip_select]; - complete(mcspi_dma-dma_rx_completion); - /* We must disable the DMA RX request */ omap2_mcspi_set_dma_req(spi, 1, 0); + + complete(mcspi_dma-dma_rx_completion); } static void omap2_mcspi_tx_callback(void *data) @@ -310,10 +310,10 @@ static void omap2_mcspi_tx_callback(void *data) struct omap2_mcspi *mcspi = spi_master_get_devdata(spi-master); struct omap2_mcspi_dma *mcspi_dma = mcspi-dma_channels[spi-chip_select]; - complete(mcspi_dma-dma_tx_completion); - /* We must disable the DMA TX request */ omap2_mcspi_set_dma_req(spi, 0, 0); + + complete(mcspi_dma-dma_tx_completion); } static void omap2_mcspi_tx_dma(struct spi_device *spi, -- 1.8.1.rc1.5.g7e0651a -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- 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/5] ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
On Wed, 12 Dec 2012 17:02:10 -0600, Jon Hunter jon-hun...@ti.com wrote: So looking at this today, here is what I see when comparing the registers ... omap2430 != omap2420 omap3430 != omap2430 omap3630 == omap3430 omap4430 != omap3430 omap4460 == omap4430 omap543x == omap4430 am335x != omap4430 Therefore, I believe that we need to have the following compatible strings ... ti,omap2420-gpmc ti,omap2430-gpmc ti,omap3430-gpmc (omap3430 omap3630) ti,omap4430-gpmc (omap4430 omap4460 omap543x) ti,am3352-gpmc (am335x devices) Probably worth adding a comment to the Documentation what should be used for which device. Yes, it is completely appropriate to add a comment to the documentation here so this doesn't get lost. g. -- 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/2] spi: devicetree: add support for loopback mode
On Wed, 12 Dec 2012 10:46:00 +0200, Felipe Balbi ba...@ti.com wrote: there are a few spi master drivers which make use of that flag but there is no way to pass it through devicetree. This patch just creates a way to pass SPI_LOOP via devicetree. I don't understand how this would be useful since loopback mode is really just a test feature. Is there any reason to do loopback for something other than test? I think it would be better to add a sysfs or debugfs property to manipulate the SPI_LOOP flag from userspace. What do you think? g. Signed-off-by: Felipe Balbi ba...@ti.com --- Documentation/devicetree/bindings/spi/spi-bus.txt | 2 ++ drivers/spi/spi.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt index 296015e..1949586 100644 --- a/Documentation/devicetree/bindings/spi/spi-bus.txt +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt @@ -55,6 +55,8 @@ contain the following properties. chip select active high - spi-3wire - (optional) Empty property indicating device requires 3-wire mode. +- spi-loopback- (optional) Empty property indicating device requires + loopback mode. If a gpio chipselect is used for the SPI slave the gpio number will be passed via the cs_gpio diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 3f1b9ee..6bcdc03 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -868,6 +868,8 @@ static void of_register_spi_devices(struct spi_master *master) spi-mode |= SPI_CS_HIGH; if (of_find_property(nc, spi-3wire, NULL)) spi-mode |= SPI_3WIRE; + if (of_find_property(nc, spi-loopback, NULL)) + spi-mode |= SPI_LOOP; /* Device speed */ prop = of_get_property(nc, spi-max-frequency, len); -- 1.8.1.rc1.5.g7e0651a -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- 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 0/3] gpio: twl4030: Correct status reporting for outputs
On Wed, Dec 12, 2012 at 11:12 AM, Peter Ujfalusi peter.ujfal...@ti.com wrote: Hi Grant, On 12/07/2012 09:09 AM, Linus Walleij wrote: On Thu, Dec 6, 2012 at 11:52 AM, Peter Ujfalusi peter.ujfal...@ti.com wrote: As Grant commneted on the first version: https://lkml.org/lkml/2012/12/5/53 Introduce bitfields to cache the directionand output status of the pins so we can report them correctly. To do this I did some cleanup within the driver to get rid of the global variables and moved them under a private structure, changed the locking as well to protect the bitfield operation. As a last patch I added a note that the PWMA/B handling should not be in this driver at all. This looks good to me: Acked-by: Linus Walleij linus.wall...@linaro.org Since Grant was requesting the changes I'll let him decide to merge. Would you have time to take a look at this set? I will take a look at it this week, but I cannot pick it up for v3.8 unless it is a regression bug fix from v3.6. It will have to wait for v3.9 and it can be merged into linux-next after the v3.8 merge window closes. g. -- 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/5] rtc: OMAP: Add system pm_power_off to rtc driver
On Tue, 20 Nov 2012 15:18:43 +0530, AnilKumar Ch anilku...@ti.com wrote:
From: Colin Foe-Parker colin.foepar...@logicpd.com
Add system power off control to rtc driver which is the in-charge
of controlling the BeagleBone system power. The power_off routine
can be hooked up to pm_power_off system call.
System power off sequence:-
* Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
* Enable PMIC_POWER_EN in rtc module
* Set rtc ALARM2 time
* Enable ALARM2 interrupt
Added while (1); after the above steps to make sure that no other
process acquire cpu. Otherwise we might see an unexpected behaviour
because we are shutting down all the power rails of SoC except RTC.
Signed-off-by: Colin Foe-Parker colin.foepar...@logicpd.com
[anilku...@ti.com: move poweroff additions to rtc driver]
Signed-off-by: AnilKumar Ch anilku...@ti.com
---
Documentation/devicetree/bindings/rtc/rtc-omap.txt |5 ++
drivers/rtc/rtc-omap.c | 81
+++-
2 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
index b47aa41..8d9f4f9 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
@@ -6,6 +6,10 @@ Required properties:
- interrupts: rtc timer, alarm interrupts in order
- interrupt-parent: phandle for the interrupt controller
+Optional properties:
+- ti,system-power-controller: Telling whether or not rtc is controlling
+ the system power.
+
Acked-by: Grant Likely grant.lik...@secretlab.ca
Example:
rtc@1c23000 {
@@ -14,4 +18,5 @@ rtc@1c23000 {
interrupts = 19
19;
interrupt-parent = intc;
+ ti,system-power-controller;
};
diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index 6009714..c31f93a 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -72,6 +72,14 @@
#define OMAP_RTC_KICK0_REG 0x6c
#define OMAP_RTC_KICK1_REG 0x70
+#define OMAP_RTC_ALARM2_SECONDS_REG 0x80
+#define OMAP_RTC_ALARM2_MINUTES_REG 0x84
+#define OMAP_RTC_ALARM2_HOURS_REG0x88
+#define OMAP_RTC_ALARM2_DAYS_REG 0x8c
+#define OMAP_RTC_ALARM2_MONTHS_REG 0x90
+#define OMAP_RTC_ALARM2_YEARS_REG0x94
+#define OMAP_RTC_PMIC_REG0x98
+
/* OMAP_RTC_CTRL_REG bit fields: */
#define OMAP_RTC_CTRL_SPLIT (17)
#define OMAP_RTC_CTRL_DISABLE(16)
@@ -93,15 +101,21 @@
#define OMAP_RTC_STATUS_BUSY(10)
/* OMAP_RTC_INTERRUPTS_REG bit fields: */
+#define OMAP_RTC_INTERRUPTS_IT_ALARM2 (14)
#define OMAP_RTC_INTERRUPTS_IT_ALARM(13)
#define OMAP_RTC_INTERRUPTS_IT_TIMER(12)
+/* OMAP_RTC_PMIC_REG bit fields: */
+#define OMAP_RTC_PMIC_POWER_EN_EN (116)
+
/* OMAP_RTC_KICKER values */
#define KICK0_VALUE 0x83e70b13
#define KICK1_VALUE 0x95a4f1e0
#define OMAP_RTC_HAS_KICKER 0x1
+#define SHUTDOWN_TIME_SEC2
+
static void __iomem *rtc_base;
#define rtc_read(addr) readb(rtc_base + (addr))
@@ -290,6 +304,63 @@ static int omap_rtc_set_alarm(struct device *dev, struct
rtc_wkalrm *alm)
return 0;
}
+/*
+ * rtc_power_off: Set the pmic power off sequence. The RTC generates
+ * pmic_pwr_enable control, which can be used to control an external
+ * PMIC.
+ */
+static void rtc_power_off(void)
+{
+ u32 val;
+ struct rtc_time tm;
+ spinlock_t lock;
+ unsigned long flags, time;
+
+ spin_lock_init(lock);
+
+ /* Set PMIC power enable */
+ val = readl(rtc_base + OMAP_RTC_PMIC_REG);
+ writel(val | OMAP_RTC_PMIC_POWER_EN_EN, rtc_base + OMAP_RTC_PMIC_REG);
+
+ /* Read rtc time */
+ omap_rtc_read_time(NULL, tm);
+
+ /* Convert Gregorian date to seconds since 01-01-1970 00:00:00 */
+ rtc_tm_to_time(tm, time);
+
+ /* Add shutdown time to the current value */
+ time += SHUTDOWN_TIME_SEC;
+
+ /* Convert seconds since 01-01-1970 00:00:00 to Gregorian date */
+ rtc_time_to_tm(time, tm);
+
+ if (tm2bcd(tm) 0)
+ return;
+
+ pr_info(System will go to power_off state in approx. %d secs\n,
+ SHUTDOWN_TIME_SEC);
+
+ /*
+ * pmic_pwr_enable is controlled by means of ALARM2 event. So here
+ * programming alarm2 expiry time and enabling alarm2 interrupt
+ */
+ rtc_write(tm.tm_sec, OMAP_RTC_ALARM2_SECONDS_REG);
+ rtc_write(tm.tm_min, OMAP_RTC_ALARM2_MINUTES_REG);
+ rtc_write(tm.tm_hour, OMAP_RTC_ALARM2_HOURS_REG);
+ rtc_write(tm.tm_mday, OMAP_RTC_ALARM2_DAYS_REG);
+ rtc_write(tm.tm_mon, OMAP_RTC_ALARM2_MONTHS_REG);
+ rtc_write(tm.tm_year, OMAP_RTC_ALARM2_YEARS_REG);
+
+ /* Enable alarm2 interrupt */
+ val = readl(rtc_base
Re: [PATCH v3 2/3] mtd: devices: elm: Add support for ELM error correction
On Thu, 29 Nov 2012 17:16:33 +0530, Philip, Avinash avinashphi...@ti.com wrote: The ELM hardware module can be used to speedup BCH 4/8/16 ECC scheme error correction. For now only 4 8 bit support is added Signed-off-by: Philip, Avinash avinashphi...@ti.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net --- Changes since v2: - Remove __devinit __devexit annotations Changes since v1: - Change build attribute to CONFIG_MTD_NAND_OMAP_BCH - Reduced indentation using by passing elm_info , offset to elm_read elm_write - Removed syndrome manipulation functions. :00 100644 000... b88ee83... A Documentation/devicetree/bindings/mtd/elm.txt :100644 100644 395733a... 369a194... Mdrivers/mtd/devices/Makefile :00 100644 000... d2667f3... Adrivers/mtd/devices/elm.c :00 100644 000... d4fce31... A include/linux/platform_data/elm.h Documentation/devicetree/bindings/mtd/elm.txt | 17 + drivers/mtd/devices/Makefile |4 +- drivers/mtd/devices/elm.c | 418 + include/linux/platform_data/elm.h | 54 4 files changed, 493 insertions(+), 1 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/elm.txt b/Documentation/devicetree/bindings/mtd/elm.txt new file mode 100644 index 000..b88ee83 --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/elm.txt @@ -0,0 +1,17 @@ +Error location module + +Required properties: +- compatible: Must be ti,elm Compatible string is too generic. Need to specify a specific SoC here. ie: ti,omap3430-elm Otherwise the binding looks fine. I haven't reviewed the code though. g. -- 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/5] ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
),
+ GFP_KERNEL);
+ if (!gpmc_nand_data)
+ return -ENOMEM;
+
+ 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;
+ }
+
+ val = of_get_nand_bus_width(child);
+ if (val == 16)
+ gpmc_nand_data-devsize = NAND_BUSWIDTH_16;
+
+ gpmc_read_timings_dt(child, gpmc_t);
+ gpmc_nand_init(gpmc_nand_data, gpmc_t);
+
+ return 0;
+}
+#else
+static int gpmc_probe_nand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ return 0;
+}
+#endif
+
+static int gpmc_probe_dt(struct platform_device *pdev)
+{
+ int ret;
+ struct device_node *child;
+ const struct of_device_id *of_id =
+ of_match_device(gpmc_dt_ids, pdev-dev);
+
+ if (!of_id)
+ return 0;
+
+ for_each_node_by_name(child, nand) {
+ ret = gpmc_probe_nand_child(pdev, child);
+ of_node_put(child);
+ if (ret 0)
+ return ret;
+ }
+
+ return 0;
+}
+#else
+static int gpmc_probe_dt(struct platform_device *pdev)
+{
+ return 0;
+}
+#endif
+
+static int __devinit gpmc_probe(struct platform_device *pdev)
{
int rc;
u32 l;
@@ -1174,6 +1334,14 @@ static __devinit int gpmc_probe(struct platform_device
*pdev)
if (IS_ERR_VALUE(gpmc_setup_irq()))
dev_warn(gpmc_dev, gpmc_setup_irq failed\n);
+ rc = gpmc_probe_dt(pdev);
+ if (rc 0) {
+ clk_disable_unprepare(gpmc_l3_clk);
+ clk_put(gpmc_l3_clk);
+ dev_err(gpmc_dev, failed to probe DT parameters\n);
+ return rc;
+ }
+
return 0;
}
@@ -1191,6 +1359,7 @@ static struct platform_driver gpmc_driver = {
.driver = {
.name = DEVICE_NAME,
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(gpmc_dt_ids),
},
};
--
1.7.11.7
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
--
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] gpio: twl4030: Correct status reporting when the GPIO is used as output
On Wed, 5 Dec 2012 10:49:45 +0100, Peter Ujfalusi peter.ujfal...@ti.com wrote: When the GPIO is configured as output we need to read the GPIODATAOUT* register to get correct information. When the GPIO is output the GPIODATAIN* registers report 0 all the time (no feedback from output path). Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com --- drivers/gpio/gpio-twl4030.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c index 55b4fe4..e7aa620 100644 --- a/drivers/gpio/gpio-twl4030.c +++ b/drivers/gpio/gpio-twl4030.c @@ -191,13 +191,19 @@ static int twl4030_get_gpio_datain(int gpio) u8 d_bnk = gpio 3; u8 d_off = gpio 0x7; u8 base = 0; + int direction; int ret = 0; if (unlikely((gpio = TWL4030_GPIO_MAX) || !(gpio_usage_count BIT(gpio return -EPERM; - base = REG_GPIODATAIN1 + d_bnk; + direction = gpio_twl4030_read(REG_GPIODATADIR1 + d_bnk); + if (direction 0 (direction d_off) 0x1) + base = REG_SETGPIODATAOUT1 + d_bnk; + else + base = REG_GPIODATAIN1 + d_bnk; + This is probably quite expensive considering that reads need to go out the i2c bus. Things like the output state and the pin direction should be cached by the driver in its private data structure so that you don't add an additional i2c round trip. g. -- 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/5] ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
On Wed, 5 Dec 2012 16:33:48 -0600, Jon Hunter jon-hun...@ti.com wrote: Hi Grant, On 12/05/2012 04:22 PM, Grant Likely wrote: On Wed, 5 Dec 2012 20:09:31 +0100, Daniel Mack zon...@gmail.com wrote: This patch adds basic DT bindings for OMAP GPMC. The actual peripherals are instantiated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND. Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them. Successfully tested on an AM33xx board. Signed-off-by: Daniel Mack zon...@gmail.com --- Documentation/devicetree/bindings/bus/ti-gpmc.txt | 77 ++ .../devicetree/bindings/mtd/gpmc-nand.txt | 76 + arch/arm/mach-omap2/gpmc.c | 171 - 3 files changed, 323 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt new file mode 100644 index 000..7d2a645 --- /dev/null +++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt @@ -0,0 +1,77 @@ +Device tree bindings for OMAP general purpose memory controllers (GPMC) + +The actual devices are instantiated from the child nodes of a GPMC node. + +Required properties: + + - compatible:Should be set to ti,gpmc Please, be specific. Use something like ti,am3340-gpmc or ti,omap3430-gpmc. The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine. We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have ti,omap2-gpmc implying all omap2+ based devices or should we have a compatible string for each device supported? Are they each register-level compatible with one another? The general recommended approach here is to make subsequent silicon claim compatibility with the first compatible implementation. So, for an am3358 board: compatible = ti,am3358-gpmc, ti,omap2420-gpmc; Essentially, what this means is that ti,omap2420-gpmc is the generic value instead of omap2-gpmc. The reason for this is so that the value is anchored against a specific implementation, and not against something completely imaginary or idealized. If a newer version isn't quite compatible with the omap2420-gpmc, then it can drop the compatible claim and the driver really should be told about the new device. g. Thanks Jon ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- 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] gpio: New driver for GPO emulation using PWM generators
On Fri, 30 Nov 2012 07:47:52 +0100, Thierry Reding
thierry.red...@avionic-design.de wrote:
On Thu, Nov 29, 2012 at 04:10:24PM +, Grant Likely wrote:
On Wed, 28 Nov 2012 09:54:57 +0100, Peter Ujfalusi peter.ujfal...@ti.com
wrote:
Hi Grant, Lars, Thierry,
On 11/26/2012 04:46 PM, Grant Likely wrote:
You're effectively asking the pwm layer to behave like a gpio (which
is completely reasonable). Having a completely separate translation node
really doesn't make sense because it is entirely a software construct.
In fact, the way your using it is *entirely* to make the Linux driver
model instantiate the translation code. It has *nothing* to do with the
structure of the hardware. It makes complete sense that if a PWM is
going to be used as a GPIO, then the PWM node should conform to the GPIO
binding.
I understand your point around this. I might say I agree with it as
well...
I spent yesterday with prototyping and I'm not really convinced that it
is a
good approach from C code point of view. I got it working, yes.
In essence this is what I have on top of the slightly modified gpio-pwm.c
driver I have submitted:
DTS files:
twl_pwm: pwm {
/* provides two PWMs (id 0, 1 for PWM1 and PWM2) */
compatible = ti,twl6030-pwm;
#pwm-cells = 2;
/* Enable GPIO us of the PWMs */
gpio-controller = 1;
This line should be simply (the property shouldn't have any data):
gpio-controller;
#gpio-cells = 2;
pwm,period_ns = 7812500;
Nit: property names should use '-' instead of '_'.
};
leds {
compatible = gpio-leds;
backlight {
label = omap4::backlight;
gpios = twl_pwm 1 0; /* PWM1 of twl6030 */
};
keypad {
label = omap4::keypad;
gpios = twl_pwm 0 0; /* PWM0 of twl6030 */
};
};
The bulk of the code in drivers/pwm/core.c to create the pwm-gpo device
when
it is requested going to look something like this. I have removed the
error
checks for now and I still don't have the code to clean up the allocated
memory for the created device on error, or in case the module is
unloaded. We
should also prevent the pwm core from removal when the pwm-gpo driver is
loaded.
We need to create the platform device for gpo-pwm, create the pdata
structure
for it and fill it in. We also need to hand craft the pwm_lookup table so
we
can use pwm_get() to request the PWM. I have other minor changes around
this
to get things working when we booted with DT.
So the function to do the heavy lifting is something like this:
static void of_pwmchip_as_gpio(struct pwm_chip *chip)
{
struct platform_device *pdev;
struct gpio_pwm *gpos;
struct gpio_pwm_pdata *pdata;
struct pwm_lookup *lookup;
char gpodev_name[15];
int i;
u32 gpio_mode = 0;
u32 period_ns = 0;
of_property_read_u32(chip-dev-of_node, gpio-controller,
gpio_mode);
if (!gpio_mode)
return;
of_property_read_u32(chip-dev-of_node, pwm,period_ns, period_ns);
if (!period_ns) {
dev_err(chip-dev,
period_ns is not specified for GPIO use\n);
return;
}
This property name seems ambiguous. What do you need to encode here? It
looks like there is a specific PWM period used for the 'on' state. What
about the 'off' state? Would different pwm outputs have different
frequencies required for GPIO usage.
Actually, I'm a bit surprised here that a period value is needed at all.
I would expect if a PWM is used as a GPIO then the driver would already
know how to set it up that way.
Just to make sure we're talking about the same thing here: if a PWM is
used as GPIO the assumption is that it would be set to 0% duty-cycle
when the GPIO value is set to 0 and 100% duty-cycle when set to the 1.
The period will still need to be set here, otherwise how would the PWM
core know what the hardware even supports?
Umm, I agree with you on duty cycle, but that's got nothing to do with
period. 100% duty cycle looks exactly the same whether the period is
10ns or 100s.
Unless you're proposing to not include that in the PWM core but rather
in individual drivers. Then I suppose the driver could choose some
sensible default.
Mostly I'm asking questions because I'm not familiar with the subsystem.
If the property is needed then I have no objections, but at the moment
it doesn't make any sense to me.
One other problem is that some PWM devices cannot be setup to achieve a
0% or 100% duty-cycle but instead will toggle for at least one period.
This would be another argument in favour of moving the functionality to
the individual drivers, perhaps with some functionality provided by the
core to do the gpio_chip registration (a period could be passed to that
function
Re: [PATCH] gpio: New driver for GPO emulation using PWM generators
On Fri, Nov 30, 2012 at 11:04 AM, Thierry Reding thierry.red...@avionic-design.de wrote: One other problem is that some PWM devices cannot be setup to achieve a 0% or 100% duty-cycle but instead will toggle for at least one period. This would be another argument in favour of moving the functionality to the individual drivers, perhaps with some functionality provided by the core to do the gpio_chip registration (a period could be passed to that function at registration time), which will likely be the same for all hardware that can and wants to support this feature. It is a bit of an oddball case where if the hardware engineer wires up a PWM to use as a GPIO, then he better be sure that it is actually fit for the purpose. Yes, I agree. So what we really want is to make this configurable in some way. For DT this could just be controlled by the gpio-controller property. The PWM core could easily setup the gpio_chip in the presence of that property. For non-DT it could probably be done via a flag that is passed to the driver via platform data, in which case the driver would have to call the helper explicitly based on the setting of this flag. That doesn't prevent the PWM core having some gpio-emulation helper functionality, but do need to be careful about it. On the other hand, if we turn that support into a helper maybe it is easier to leave it up to the driver whether to call it or not. A big advantage of this would be that the driver could pass a period along that it can choose sensibly according to the chip's capabilities. Something as simple as: int pwmchip_emulate_gpio(struct pwm_chip *chip, unsigned long period); could do. Cleanup could be done automatically in pwmchip_remove(). Looks reasonable. g. -- 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] gpio: New driver for GPO emulation using PWM generators
On Wed, 28 Nov 2012 09:54:57 +0100, Peter Ujfalusi peter.ujfal...@ti.com
wrote:
Hi Grant, Lars, Thierry,
On 11/26/2012 04:46 PM, Grant Likely wrote:
You're effectively asking the pwm layer to behave like a gpio (which
is completely reasonable). Having a completely separate translation node
really doesn't make sense because it is entirely a software construct.
In fact, the way your using it is *entirely* to make the Linux driver
model instantiate the translation code. It has *nothing* to do with the
structure of the hardware. It makes complete sense that if a PWM is
going to be used as a GPIO, then the PWM node should conform to the GPIO
binding.
I understand your point around this. I might say I agree with it as well...
I spent yesterday with prototyping and I'm not really convinced that it is a
good approach from C code point of view. I got it working, yes.
In essence this is what I have on top of the slightly modified gpio-pwm.c
driver I have submitted:
DTS files:
twl_pwm: pwm {
/* provides two PWMs (id 0, 1 for PWM1 and PWM2) */
compatible = ti,twl6030-pwm;
#pwm-cells = 2;
/* Enable GPIO us of the PWMs */
gpio-controller = 1;
This line should be simply (the property shouldn't have any data):
gpio-controller;
#gpio-cells = 2;
pwm,period_ns = 7812500;
Nit: property names should use '-' instead of '_'.
};
leds {
compatible = gpio-leds;
backlight {
label = omap4::backlight;
gpios = twl_pwm 1 0; /* PWM1 of twl6030 */
};
keypad {
label = omap4::keypad;
gpios = twl_pwm 0 0; /* PWM0 of twl6030 */
};
};
The bulk of the code in drivers/pwm/core.c to create the pwm-gpo device when
it is requested going to look something like this. I have removed the error
checks for now and I still don't have the code to clean up the allocated
memory for the created device on error, or in case the module is unloaded. We
should also prevent the pwm core from removal when the pwm-gpo driver is
loaded.
We need to create the platform device for gpo-pwm, create the pdata structure
for it and fill it in. We also need to hand craft the pwm_lookup table so we
can use pwm_get() to request the PWM. I have other minor changes around this
to get things working when we booted with DT.
So the function to do the heavy lifting is something like this:
static void of_pwmchip_as_gpio(struct pwm_chip *chip)
{
struct platform_device *pdev;
struct gpio_pwm *gpos;
struct gpio_pwm_pdata *pdata;
struct pwm_lookup *lookup;
char gpodev_name[15];
int i;
u32 gpio_mode = 0;
u32 period_ns = 0;
of_property_read_u32(chip-dev-of_node, gpio-controller,
gpio_mode);
if (!gpio_mode)
return;
of_property_read_u32(chip-dev-of_node, pwm,period_ns, period_ns);
if (!period_ns) {
dev_err(chip-dev,
period_ns is not specified for GPIO use\n);
return;
}
This property name seems ambiguous. What do you need to encode here? It
looks like there is a specific PWM period used for the 'on' state. What
about the 'off' state? Would different pwm outputs have different
frequencies required for GPIO usage.
Actually, I'm a bit surprised here that a period value is needed at all.
I would expect if a PWM is used as a GPIO then the driver would already
know how to set it up that way.
lookup = devm_kzalloc(chip-dev, sizeof(*lookup) * chip-npwm,
GFP_KERNEL);
pdata = devm_kzalloc(chip-dev, sizeof(*pdata), GFP_KERNEL);
gpos = devm_kzalloc(chip-dev, sizeof(*gpos) * chip-npwm,
GFP_KERNEL);
pdata-gpos = gpos;
pdata-num_gpos = chip-npwm;
pdata-gpio_base = -1;
pdev = platform_device_alloc(pwm-gpo, chip-base);
pdev-dev.parent = chip-dev;
sprintf(gpodev_name, pwm-gpo.%d, chip-base);
for (i = 0; i chip-npwm; i++) {
struct gpio_pwm *gpo = gpos[i];
struct pwm_lookup *pl = lookup[i];
char con_id[15];
sprintf(con_id, pwm-gpo.%d, chip-base + i);
/* prepare GPO information */
gpo-pwm_period_ns = period_ns;
gpo-name = kmemdup(con_id, sizeof(con_id), GFP_KERNEL);;
/* prepare pwm lookup table */
pl-provider = dev_name(chip-dev);
pl-index = i;
pl-dev_id = kmemdup(gpodev_name, sizeof(gpodev_name),
GFP_KERNEL);
pl-con_id = kmemdup(con_id, sizeof(con_id), GFP_KERNEL);
}
platform_device_add_data(pdev, pdata, sizeof(*pdata));
pwm_add_table(lookup, chip-npwm);
platform_device_add(pdev);
Ugh, yeah this isn't the way to go. Don't register
Re: [PATCH] gpio: New driver for GPO emulation using PWM generators
On Fri, 23 Nov 2012 10:44:36 +0100, Peter Ujfalusi peter.ujfal...@ti.com
wrote:
Hi Grant,
On 11/23/2012 10:13 AM, Peter Ujfalusi wrote:
Hi Grant,
On 11/23/2012 08:55 AM, Grant Likely wrote:
Ugh. and this is why I wanted the PWM and GPIO subsystems to use the
same namespace and binding. grumble, mutter But that's not your fault.
It's pretty horrible to have a separate translator node to convert a PWM
into a GPIO (with output only of course). The gpio properties should
appear directly in the PWM node itself and the translation code should
be in either the pwm or the gpio core. I don't think it should look like
a separate device.
Let me see if I understand your suggestion correctly. In the DT you suggest
something like this:
twl_pwmled: pwmled {
compatible = ti,twl4030-pwmled;
#pwm-cells = 2;
#gpio-cells = 2;
gpio-controller;
};
After I thought about this.. Is this what we really want?
After all what we have here is a PWM generator used to emulate a GPIO signal.
The PWM itself can be used for driving a LED (standard LED or backlight and we
have DT bindings for these already), vibra motor, or other things.
If we combine the PWM with GPIO we should go and 'bloat' the DT node to also
include all sort of other uses of PWM at once?
IMHO it is better to keep them as separate things.
pwm node to describe the PWM generator,
separate nodes to describe it's uses like led, backlight, motor and gpio.
You're effectively asking the pwm layer to behave like a gpio (which
is completely reasonable). Having a completely separate translation node
really doesn't make sense because it is entirely a software construct.
In fact, the way your using it is *entirely* to make the Linux driver
model instantiate the translation code. It has *nothing* to do with the
structure of the hardware. It makes complete sense that if a PWM is
going to be used as a GPIO, then the PWM node should conform to the GPIO
binding.
g.
--
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] gpio: New driver for GPO emulation using PWM generators
On Thu, 22 Nov 2012 14:42:03 +0100, Peter Ujfalusi peter.ujfal...@ti.com wrote: There seams to be board designs using PWM generators as enable/disable signals. For these boards we used to have custom code as hacks to deal with such a situations. With the gpio-pwm driver we can emulate the GPIO functionality using PWM generators via standard interfaces. The PWM will be configured to be off when the GPIO is off and to full duty cycle when the GPIO is requested to be on. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com --- Hi Linus, So this is the driver I came up regarding to the issue that some boards (BeagleBoard for example) are using the PWM generators as enable/disable signal. On BeagleBoard the situation is like this: TWL4030 PWMA - TWL4030 LEDA - nUSBHOST_PWR_EN - external power switch - USB host hub power. So I have tested this driver on BeagleBoard: - Custom code to toggle the GPIO just to see if it switches correctly - Real life usecase: fixed voltage regulator with GPIO control (the gpio is the gpio-pwm provided). ehci-omap has been modified to allow deferred probe. the regulator is used by ehci-omap. All works fine. Ugh. and this is why I wanted the PWM and GPIO subsystems to use the same namespace and binding. grumble, mutter But that's not your fault. It's pretty horrible to have a separate translator node to convert a PWM into a GPIO (with output only of course). The gpio properties should appear directly in the PWM node itself and the translation code should be in either the pwm or the gpio core. I don't think it should look like a separate device. g. -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Sat, 17 Nov 2012 23:27:18 +0100, Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com wrote: On 16:23 Fri 09 Nov , Stephen Warren wrote: On 11/09/2012 09:28 AM, Grant Likely wrote: However, I think the process for an end-user needs to be as simple as drop this .dts/.dtb file into some standard directory, and I imagine it'll be much easier for distros/... to make that process work if they're dealing with a .dtb that they can just blast into the kernel's firmware loader interface, rather than having to also locate the base-board .dts/.dtb file, and run dtc and/or other tools on both .dts files together. I've exactly the same issue on Calao or the new atmel boards We have lego boards with different cpu-modues running on differetn mother boards with diferrentdaugther boards on atmel we are lucky enough we can identity via 1-wire all of them but on Calao no On Somfy platform we can detect hardware version and need different pinctrl So personally I'll prefer to be able to request dtb from the kernel or push them from the userspace as it will depends where you will detect the hardware present The main concern will which part of the kenel will now handle hw detection? Along the lines of what you said above, it will depend on the board. If the board /can/ be detected, then the kernel should do so and request the appropriate configuration. If it cannot, then it simply must be left up to either userspace or something explicit in the boot devicetree. g. -- 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-dt: Enable DT proc updates.
On Wed, 31 Oct 2012 21:04:24 -0500, Rob Herring robherri...@gmail.com wrote: On 10/31/2012 10:57 AM, Pantelis Antoniou wrote: This simple patch enables dynamic changes of the DT tree on runtime to be visible to the device-tree proc interface. Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com Acked-by: Rob Herring rob.herr...@calxeda.com Applied, thanks. g. -- 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] spi: Export OF interfaces.
On Wed, 31 Oct 2012 17:57:33 +0200, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Export an interface that other in-kernel users can utilize. Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com I'm not going to apply this before an in-kernel user exists for this. I know it's part of the whole runtime population of the device tree stuff and I don't think it will survive in it's current form by the time we're done. g. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Tue, Nov 13, 2012 at 8:09 AM, Pantelis Antoniou
pa...@antoniou-consulting.com wrote:
On Nov 13, 2012, at 9:25 AM, David Gibson wrote:
Not good to rely on userspace kicking off dtc and compiling from source.
Some capes/expansion boards might have your root fs device, for example
there is an eMMC cape coming up, while networking capes are common too.
However I have a compromise.
I agree that compiling from source can be an option for a runtime definable
cape, but for built-in capes we could do a bit better.
In particular, I don't see any particular need to have a DT fragment
reference anything that dependent of the runtime device tree. It should
be possible to compile the DT fragment in kernel, against the generated
flattened device tree, producing a flattened DT fragment with the phandles
already resolved.
Do you mean linking dtc into the kernel?
So the sequence could be something like this:
$ dtc -O dtb -o am335x-bone.dtb -b 0 am335x-bone.dts -@ ${LAST_PHANDLE_FILE}
$ dtc -O dtbf -R am335x-bone.dtb -o weather-cape.dtb -b 0 weather-cape.dts -@
${LAST_PHANDLE_FILE}
$ dtc -O dtbf -R am335x-bone.dtb -o geiger-cape.dtb -b 0 geiger-cape.dts -@
${LAST_PHANDLE_FILE}
The ${LAST_PHANDLE_FILE} can be updated with the last phandle value generated.
Alternatively we could have a way to statically assign a phandle range
for well known capes. All the others will have to use the runtime compile
mechanism.
$ dtc -O dtb -o am335x-bone.dtb -b 0 am335x-bone.dts
$ dtc -O dtbf -R am335x-bone.dtb -o weather-cape.dtb -b 0 weather-cape.dts
$ dtc -O dtbf -R am335x-bone.dtb -o geiger-cape.dtb -b 0 geiger-cape.dts
With the cape dtses having a /phandle-range/ statement at the top.
I really think the whole phandle allocation thing is a non problem.
Generating phandles is the easy part, and using a good hash function
pretty much solves it entirely. I know people are worried about
collisions, but there are only two scenarios where collisions may
happen:
1) base and overlay try to define same phandle
- Easy to detect - kernel can complain and refuse to load if it happens
- Will never happen when overlay is compiled against the base used
to boot (dtc can resolve the conflict)
- Hash phandle generation makes this exceptionally rare
2) two overlays try to define same phandle
- Also easy to detect on loading the second overlay - kernel will
- Hash phandle generation still makes this exceptionally rare
In both cases a collision is extremely rare and if it does happen it
will not fail silently. Besides, in the odd scenario where it does
happen, a node can be manually assigned a phandle. It is far better to
do the manual override maybe once or twice (actually, I'd be surprised
if it is ever needed) than to get into any kind of numberspace
management for phandles. That's just a maintenance nightmare.
The hard bit to solve has always been when the overlay expects
different phandles than the base is using, and that problem only
occurs if the overlay is compiled against a different base dtb than
was used to boot the system. Hashed phandle generation even makes
phandles very stable when the base dtb is recompiled, and if they do
change, then the kernel can detect it and report an error. Again, no
silent failures. Phandle fixup does make the problem disappears
entirely but I'm concerned that it is still incomplete (see below) and
would be conceptually expensive.
Once of the requests is to support one overlay .dtb with many
different baseboards, but now that I've thought about it more I
realize that it just won't work for anything beyond the most trivial
case. Phandle fixup isn's sufficient. The format of GPIO, Interrupt,
clock, etc specifiers may change if the base board nodes change. The
specifiers are not portable.
So, I no longer think that plain dtb overlays alone will work against
any base board. Something more is needed. It may be the mechanism for
loading new data into the kernel, but something really generic does
require either being compiled at runtime (as David suggests) or each
expansion interface (ie. the BeagleBone expansion or the RPi
expansion) to really specify what kinds of references are allowed and
run all specifiers through translation nodes. Similar to how
interrupt-map works.
i2c2: i2c@4819c000 {
compatible = ti,omap4-i2c;
#address-cells = 1;
#size-cells = 0;
ti,hwmods = i2c3;
reg = 0x4819c000 0x1000;
interrupt-parent = intc;
interrupts = 30;
status = disabled;
};
And in the cape definition (when compiled with the special mode I describe
below)
/ {
plugin-bundle;
compatible = cco,weather-cape;
version = 00A0;
i2c2-graft = {
compatible = dt,graft;
graft-point = i2c2;
Since overlays are different, we can interpret them slightly
differently. The node name itself could be the graft point, and the
name could be either a full path
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Mon, Nov 12, 2012 at 11:34 AM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Hi Grant, On Nov 9, 2012, at 10:33 PM, Grant Likely wrote: On Wed, Nov 7, 2012 at 11:02 AM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: On Nov 7, 2012, at 11:19 AM, Benoit Cousson wrote: Maybe some extra version match table can just be passed during the board machine_init of_platform_populate(NULL, omap_dt_match_table, NULL, NULL, panda_version_match_table); Would we need explicit of_platform_populate calls if we have node modification notifiers? In that case the notifier would pick it up automatically, and can do the per version matching internally. There still needs to be something to register everything below this node is interesting which is exactly what of_platform_populate() does now. I see the notifiers being used by the of_platform_populate backend to know when nodes have been created (or destroyed). g. I see. So of_platform_populate could just register the notifier and not do the tree walk itself. Perhaps the name is a bit misleading then? Kind of, yes. of_platform_populate() would still have the same effect that it does now except that it would also pay attention to additions and removals from the DT nodes it is interested in. This would work cleanly enough for node additions/removals, but it wouldn't process changes to properties on existing nodes. g. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Mon, Nov 5, 2012 at 11:22 PM, Tony Lindgren t...@atomide.com wrote: Hi, * Tabi Timur-B04825 b04...@freescale.com [121105 13:42]: On Mon, Nov 5, 2012 at 2:40 PM, Grant Likely grant.lik...@secretlab.ca wrote: Jane is building custom BeagleBone expansion boards called 'capes'. She can boot the system with a stock BeagleBoard device tree, but additional data is needed before a cape can be used. She could replace the FDT file used by U-Boot with one that contains the extra data, but she uses the same Linux system image regardless of the cape, and it is inconvenient to have to select a different device tree at boot time depending on the cape. What's wrong with having the boot loader detect the presence of the Cape and update the device tree accordingly? We do this all the time in U-Boot. Doing stuff like reading EEPROMs and testing for the presence of hardware is easier in U-Boot than in Linux. For configurations that can be determined by the boot loader, I'm not sure overlays are practical. I guess the beaglebone capes could be stackable and hotpluggable if handled carefully enough. And even if it can't on the beaglebone, it will happen somewhere else. I don't want to exclude that use-case just because nobody thought about it. g. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Tue, Nov 6, 2012 at 10:37 PM, Stephen Warren swar...@wwwdotorg.org wrote:
On 11/05/2012 01:40 PM, Grant Likely wrote:
Hey folks,
As promised, here is my early draft to try and capture what device
tree overlays need to do and how to get there. Comments and
suggestions greatly appreciated.
Interesting. This just came up internally at NVIDIA within the last
couple weeks, and was discussed on the U-Boot mailing list very recently
too:
http://lists.denx.de/pipermail/u-boot/2012-October/thread.html#138227
(it spills into the November archive too)
For these cases it is proposed to implement an overlay feature for the
so that the initial device tree data can be modified by userspace at
I don't know if you're maintaining this as a document and taking patches
to it, but if so:
Sure, why not...
http://git.secretlab.ca/?p=devicetree-overlays.git;a=summary
for the so split across those two lines.
fixed
- SHOULD reliably handle changes between different underlying overlays
(ie. what happens to existing .dtb overly files if the structure of
the dtb it is layered over changes. If not possible, then SHALL
detect when the base tree doesn't match and refuse to apply the
overlay.
Perhaps use (versioned) DT bindings to represent the interface between
the two .dts files? See the links to the U-Boot mailing list discussions
below?
Implementing versioning is conceptually a lot more complex than plain
overlays since it means either the kernel or u-boot needs to start
filtering the data that it's given. This can get really complex in a
hurry. Mitch makes a valid point later in this thread that when it
comes to manipulating the data depending on the board then the data
overlay model alone doesn't handle it well.
I'm not actually opposed to it, but it needs to be done in an elegant
way. The DT data model already imposes more of a conceptual learning
curve than I wish it did and I don't want to make that worse with a
versioning model that is difficult to get ones head around.
Suggestions and proposals are definitely welcome here.
- What is the model for overlays?
- Can an overlay modify existing properties?
- Can an overlay add new properties to existing nodes?
- Can an overlay delete existing nodes/properties?
This proposal is very oriented at an overlay-based approach. I'm not
totally convinced that a pure overlay approach (as in how dtc does
overlayed DT nodes) will be flexible enough, but would love to be
persuaded. Again see below.
The way I'm currently thinking about the overlay approach is as a
discrete set of changes that all can be applied at once.* That
certainly doesn't preclude the change being generated with a script or
other tool in either firmware or userspace. For most changes it works
really well. Of the scenarios that don't work well, I can think of
two. The first is it moving or renaming existing nodes, and the second
is if the structure of the base tree changes (say due to a firmware
update). Although the second limitation is specifically with binary
.dtb overlays. Recompiling the dts overlay against the new tree would
work fine.**
*with the caveat that not all types of changes are a good idea and we
may disallow. For example, is changing properties in existing nodes a
good idea?
** all this is based on my current ideas about the .dtb overlay format
which would be trivial to implement, but I'm not committed to that
approach just yet. The above problems could be solved.
It may be sufficient to solve it by making the phandle values less
volatile. Right now dtc generates phandles linearly. Generated phandles
could be overridden with explicit phandle properties, but it isn't a
fantastic solution. Perhaps generating the phandle from a hash of the
node name would be sufficient.
Node names don't have to be unique though right; perhaps hash the
path-name instead of the node-name? But then, why not just reference by
path name; similar to {/path/to/node} rather than label?
I was thinking about using the full_name for generating phandles. On
the odd chance there is a collision, one of the nodes will get
something like the hash value +1. Or in that condition we can require
one of the nodes to have the phandle value explicitly set in the
source file.
Note; this doesn't actually affect anything outside of dtc. Right now
dtc starts at 1 and assigns phandles incrementally. I'm suggesting
using a hash of the full_name to assign the phandle for a node so that
it will not change unless the full_path changes.
This handles many of the use cases, but it assumes that an overlay is
board specific. If it ever is required to support multiple base boards
with a single overlay file then there is a problem. The .dtb overlays
generated in this manor cannot handle different phandles or nodes that
are in a different place. On the other hand, the overlay source files
should have no problem being compiled for multiple targets.
s/manor/manner/
Fixed
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Wed, Nov 7, 2012 at 12:54 AM, Mitch Bradley w...@firmworks.com wrote: On 11/6/2012 12:37 PM, Stephen Warren wrote: This proposal is very oriented at an overlay-based approach. I'm not totally convinced that a pure overlay approach (as in how dtc does overlayed DT nodes) will be flexible enough, but would love to be persuaded. Again see below. An overlay approach will not be powerful enough to solve the sorts of problems that occur when a product goes into full production, becomes a family, and starts to evolve. Issues like second-source parts that aren't quite compatible and need to be detected and reported, board-stuff options for different customer profiles, speed grades of parts that aren't properly probeable but instead need to be identified by some subterfuge, the list of tedious issues goes on and on. It's nice to pretend that the world fits into a few coherent simple use cases, but 30 years of experience shipping computer product families proves otherwise. You need a programming language to solve the full set of problems - which I why the device tree is designed so it can be generated and modified by a program. I'm not going to argue with that. There is nothing saying that the overlay wouldn't be generated or applied by a script. However, I do strongly think that the data model needs to be independent of any tool or execution environment used to manipulate it. I certainly am not interested in encoding scripts or bytecode into the tree - the opposite of the approach used by ACPI which must execute AML to even retrieve the device tree. I like the overlay approach because it is conceptually straightforward and provides a working model of how changes could be made from userspace while still being usable by firmware. An alternate approach from userspace would be to use a live virtual filesystem to manipulate the device tree, though that approach only applies to Linux userspace. Firmware would still need its own approach. g. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Wed, Nov 7, 2012 at 8:06 AM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Hi Grant, On Nov 6, 2012, at 9:45 PM, Grant Likely wrote: Yes, the locking does need to be sorted out. Perhaps come up with a dt-stress test tool/boot time validator? I would like that. I've started adding DT test cases to the kernel source tree. g. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Wed, Nov 7, 2012 at 11:02 AM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: On Nov 7, 2012, at 11:19 AM, Benoit Cousson wrote: Maybe some extra version match table can just be passed during the board machine_init of_platform_populate(NULL, omap_dt_match_table, NULL, NULL, panda_version_match_table); Would we need explicit of_platform_populate calls if we have node modification notifiers? In that case the notifier would pick it up automatically, and can do the per version matching internally. There still needs to be something to register everything below this node is interesting which is exactly what of_platform_populate() does now. I see the notifiers being used by the of_platform_populate backend to know when nodes have been created (or destroyed). g. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 9, 2012 at 2:26 AM, David Gibson
da...@gibson.dropbear.id.au wrote:
Summary points:
- Create an FDT overlay data format and usage model
- SHALL reliable resolve or validate of phandles between base and
overlay trees
So, I'm not at all clear on what this proposed phandle validation
would involve. I'm also not convinced it's as necessary as you
think, more on that below.
Simply this: I'm taking this example from the omap3-beagle-xm.dts. It
has the following stanza which is currently rolled into the resulting
.dtb when compiled.
i2c1 {
clock-frequency = 260;
twl: twl@48 {
reg = 0x48;
interrupts = 7; /* SYS_NIRQ cascaded to intc */
interrupt-parent = intc;
vsim: regulator-vsim {
compatible = ti,twl4030-vsim;
regulator-min-microvolt = 180;
regulator-max-microvolt = 300;
};
twl_audio: audio {
compatible = ti,twl4030-audio;
codec {
};
};
};
};
However, if it were compiled into a separate dtb overlay it might look
like this:
/ {
.readonly;
ocp {
.readonly;
interrupt-controller@4820 {
phandle = 0x1234; /* EXPECTED PHANDLE */
.readonly;
};
i2c@4807 {
.must-exist;
clock-frequency = 260;
twl@48 {
reg = 0x48;
interrupts = 7;
interrupt-parent = 0x1234; /* RESOLVED PHANDLE */
vsim: regulator-vsim {
compatible = ti,twl4030-vsim;
regulator-min-microvolt = 180;
regulator-max-microvolt = 300;
};
twl_audio: audio {
compatible = ti,twl4030-audio;
codec {
};
};
};
};
};
};
Notice I've included the intc node and it's phandle. By phandle
validation I merely mean that when applying an overly the firmware or
kernel must verify that the phandles in the overlay match the phandle
in the base tree. If they don't match, then refuse to apply the
overhead. This approach avoids the need to find and fixup phandles in
the overlay. And if the phandle is generated from a hash of the
full_name, then the resulting phandle will only change if the node
moves.
Similarly, at application time it should be verified that the nodes
with a .readonly or .must-exist property could be verified to actually
exist before attempting to apply the overlay. I used two different
properties with the idea that only certain nodes would need to be
modified... exactly what the policies should be is yet to be
determined.
g.
--
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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 9, 2012 at 5:32 AM, Joel A Fernandes agnel.j...@gmail.com wrote: Hi Pantelis, I hope I'm not too late to reply as I'm traveling. On Nov 6, 2012, at 5:30 AM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Joanne has purchased one of Jane's capes and packaged it into a rugged case for data logging. As far as Joanne is concerned, the BeagleBone and cape together are a single unit and she'd prefer a single monolithic FDT instead of using an FDT overlay. Option A: Using dtc, she uses the BeagleBone and cape .dts source files to generate a single .dtb for the entire system which is loaded by U-Boot. -or- Unlikely. Option B: Joanne uses a tool to merge the BeagleBone and cape .dtb files (instead of .dts files), -or- Possible but low probability. Option C: U-Boot loads both the base and overlay FDT files, merges them, and passes the resolved tree to the kernel. Could be made to work. Only really required if Joanne wants the cape interface to work for u-boot too. For example if the cape has some kind of network interface that u-boot will use to boot from. I love Grant's hashing idea a lot keeping the phandle problem for compile time and not requiring fixups. IMO it is still a cleaner approach if u-boot does the simple tree merging for all cases, and not the kernel reasons mentioned below. I'm more than sufficiently convinced that making changes at runtime from userspace is pretty much required. Also consider: it is order of magnitudes easier to modify the kernel than it is to change the firmware for end users. (1) From a development standpoint, very little or nothing will have to be changed in kernel (except for scripts/dtc) considering we are moving forward with hashing. (2) Also this discussed a while back but at some point is going to brought up again- loading of dt fragment directly from EEPROM and merging at run time. If we were to implement this in kernel, we would have to add cape specific EEPROM reading code, merge the tree before it is unflattened and parse. Unless it is required for boot to userspace I'm not considering merging before userspace starts. That's well after the tree is unflattened into the live form. If it is require to boot then I agree that is should be done in firmware. I see zero problem with having a beaglebone specific cape driver that knows to read the eeprom and request a specific configuration file. Heck, the kernel doesn't even need to parse the eeprom data. It can be read from userspace and userspace decides which overlay to provide. There's nothing stopping userspace from reading the eeprom, looking up the correct dts for the board, downloading the file from the Internet, compiling it with dtc and installing it and yes that is getting a little extreme. I think doing tree merging in kernel is messy and we should do it in uboot considering we might have to read EEPROM for this use case. Ideally reading the fragment from the EEPROM for all capes and merging without worrying about version detection, Doing the merge and passing the merged blob to the kernel which (kernel) works the same way it does today. It may be sufficient to solve it by making the phandle values less volatile. Right now dtc generates phandles linearly. Generated phandles could be overridden with explicit phandle properties, but it isn't a fantastic solution. Perhaps generating the phandle from a hash of the node name would be sufficient. I doubt the hash method will work reliably. We only have 32 bits to work with, nothing like the SHA hashes of git. I was wondering I have worked with kernel's crypto code in the past to generate 32 bit md5sums of 1000s of dataitems, from what I've seen, collisions are rare and since we are talking about just a few nodes that are being referenced in the base dt. I think the probability is even less (ofcourse such an analysis strongly depends on dataset). this method also takes away a lot of complexity with having it to do runtime fixups and will help us get off the ground quickly. It wouldn't be hard to put together a test and run it on all the .dts files in the kernel; generating md5 sums for all the full_name paths and seeing if we've got any collisions yet. We can also put in a collision handling mechanism if needed. I think it is worthy doing a sample hash of all nodes in all dts we have in a script and see for once if we have collisions and what it looks like. Collision handling is a must, but again this is all internal to dtc. I don't foresee any problems with it. Alternatively to hashing, reading David Gibson's paper I followed, phandle is supposed to 'uniquely' identity node. I wonder why the node name itself is not sufficient to uniquely identify. Simply because the FDT draws upon the existing OFW bindings which use a 32bit value to reference other nodes. The code that does the tree walking can then just strcmp the node name
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 9, 2012 at 2:26 AM, David Gibson
da...@gibson.dropbear.id.au wrote:
(3) Resolving phandle references from the subtree to the main tree.
So, I think this can actually be avoided, at least in cases where what
physical connections are available to the expansion module is well
defined. The main causes to have external references are interrupts
and gpios. Interrupts we handle by defining an interrupt specifier
space for the interrupts available on the expansion
socket/connector/bus/whatever. In the master tree we then have
something like:
...
expansion-socket@ {
expansion-id = SlotA;
interrupt-map = /* map expansion irq specs to
board interrupt controllers */ ;
interrupt-map-mask = ... ;
ranges = /* map expansion local addresses to global
mmio */ ;
};
The subtree for the expansion module gets attached as a subnode of
this one. It doesn't use explicit interrupt-parents but instead just
uses the expansion local irq specifiers, letting the parent be the
default which will bubble up to this socket node where the
interrupt-map will send them to the right places.
I don't recall the gpio bindings off hand, but as I recall we based
them off the irq tree bindings so we ought to be able to do the same
thing for them.
Likewise, if there are several interchangeable expansion sockets that
have some address bits hard wired to distinguish them, we can just use
socket local mmio addresses within the subtree and the ranges property
here will sort that out.
If I'm reading correctly, the suggestion is that everything be grafted
below a single node and all connections route through that node using
mapping. Correct?
For interrupts that works today
For gpios, it isn't currently supported, but we could do it.
For SPI it would mean that the new spi devices would not appear below
the actual spi bus they are attached to
For I2C, MDIO, and one wire, same problem.
For memory mapped devices, the expansion node would need to a ranges
for all the windows that map through it, and it assumes only one
memory mapped bus (or at least it prefers only one memory mapped bus.
If there were more than one then the expansion node placement wouldn't
have a natural place to sit)
The problem is that this is not like a PCI bus where there is only one
kind of interface. It is a whole bunch of interfaces that happen to be
grouped together loosely (as an expansion connector in the beaglebone
case, but expansion isn't the only problem that I'm hearing about).
So, with a group of i2c, spi, memory mapped and other devices than are
all plugged in together, how does that look? They really should not
sit on the same level. An spi device cannot be a peer of an i2c device
for instance, the address mapping is entirely different. The kernel
really wants i2c devices to be a child of the actual i2c bus which may
already have an i2c device or too on the main board. Does the
expansion node need to have some kind of redirect node for each of the
busses where the children of it need to create devices as children of
the master bus?
To me that seems to get really complex in a hurry. More complex than
the overlay approach.
g.
--
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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 9, 2012 at 10:57 PM, Stephen Warren swar...@wwwdotorg.org wrote: On 11/08/2012 07:26 PM, David Gibson wrote: ... I also think graft will handle most of your use cases, although as I said I don't fully understand the implications of some of them, so I could be wrong. So, the actual insertion of the subtree is pretty trivial to implement. phandles are the obvious other matter to be dealt with. I haven't found the right thread where what you've envisaged so far is discussed, so here are things I can see: 1) Avoiding phandle collisions between main tree and subtree (or between subtrees). I'm hopeful that this can be resolved just by establishing some conventions about the ranges of phandles to be used for various components. I'd certainly be happy to add a directive to dtc which enforces allocation of phandles within a specified range. One case I wonder about is: Base board A that's split into two .dts files e.g. due to there being two versions of the base board which are 90% identical, yet there being a few differences between them. Base board B that's just a single .dts file. We might allocate phandle range 0..999 for the base board. Child board X plugs into either, so the two base boards need to export the same set of phandle IDs to the child board to allow it to reference them. It's more than just that. The boards really need to be equivalent since the irq specifiers and gpio specifiers need to match the gpio and irq controllers provided by the board. So a simple overlay design won't cover the case of a single file that will work generically on any board. If base board A version 2 comes along after the phandle IDs that the child board uses are defined, then how do we handle assigning a specific phandle ID range to each of base board A's two version-specific overlays, when the version-specific changes might affect arbitrary phandle IDs within the range, and require some to be moved into the base board version-specific overlays and some to stay in the common base board .dts? With the design I'm currently considering, at the dts level the overlay could be compiled against any base boards if the specifiers are equivalent and the labels match up as indicated above. The compiled dtb could also be somewhat portable, but only if care is taken to make sure the phandles match too; possibly by explicitly specifying them instead of letting them be generated by a hash. g. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 9, 2012 at 11:06 PM, Stephen Warren swar...@wwwdotorg.org wrote: On 11/05/2012 01:40 PM, Grant Likely wrote: As promised, here is my early draft to try and capture what device tree overlays need to do and how to get there. Comments and suggestions greatly appreciated. Here's one other requirement I'd like that I don't think I saw explicitly mentioned in your document: Assuming a base file board.dts and a child board file child.dts, the compiled file child.dtb should be usable with a modified board.dtb assuming it exports the same set of attachment-points (hashed phandles, socket objects, whatever). This allows bug-fixes etc. to board.dts without forcing every child.dts to be recompiled. No, I hadn't explicitly captured that one, but yes it is the intent. If the attachment points is hashed node names or node content from board.dts, I'm not sure how this is possible? Ummm, I think there is misunderstanding about the hashed phandles. My thought is merely that using a hash to generate a phandle is 'better' than starting at 1 and counting upwards when dtc compiles the tree. That way, if the path to the node has not changed, then the phandle will not have changed. However, phandles can still be explicitly specified if slightly different trees need to have the same target point. That said, if portability of dtb files is a strong requirement, then it will be difficult to do with simple overlays. Even if the phandles line up, the irq/gpio specifiers probably need to be different. That makes things harder. g. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 9, 2012 at 11:23 PM, Stephen Warren swar...@wwwdotorg.org wrote: On 11/09/2012 09:28 AM, Grant Likely wrote: On Tue, Nov 6, 2012 at 10:37 PM, Stephen Warren swar...@wwwdotorg.org wrote: ... I do rather suspect this use-case is quite common. NVIDIA certainly has a bunch of development boards with pluggable PMIC/audio/WiFi/display/..., and I believe there's some ability to re-use the pluggable components with a variety of base-boards. Given people within NVIDIA started talking about this recently, I asked them to enumerate all the boards we have that support pluggable components, and how common it is that some boards support being plugged into different main boards. I don't know when that enumeration will complete (or even start) but hopefully I can provide some feedback on how common the use-case is for us once it's done. From your perspective, is it important to use the exact same .dtb overlays for those add-on boards, or is it okay to have a separate build of the overlay for each base tree? I certainly think it'd be extremely beneficial to use the exact same child board .dtb with arbitrary base boards. Consider something like the Arduino shield connector format, which I /believe/ has been re-used across a wide variety of Arduino boards and other compatible or imitation boards. Now consider a vendor of an Arduino shield. The shield vendor probably wants to publish a single .dtb file that works for users irrespective of which board they're using it with. (Well, I'm not sure that Arduino can run Linux; perhaps that's why you picked BeagleBone capes for your document!) Correct, the Arduino is only an AVR with a tiny amount of ram. No Linux there. However, Arduino shields are a good example of a use case. I think there are even some Arduino shield compatible Linux boards out there. I suppose it would be acceptable for the shield vendor to ship the .dts file rather than the .dtb, and hence need to build the shield .dtb for a specific base board. That would be better I think than relying on a binary. However, some though needs to go into how to handle base boards that /aren't/ mostly equivalent. Such as if they have a different GPIO controller. It may be that for gpios and irqs, the solution really is to use interrupt-map and create a gpio-map. i2c, spi and others still would need to become children of the correct bus. However, I think the process for an end-user needs to be as simple as drop this .dts/.dtb file into some standard directory, and I imagine it'll be much easier for distros/... to make that process work if they're dealing with a .dtb that they can just blast into the kernel's firmware loader interface, rather than having to also locate the base-board .dts/.dtb file, and run dtc and/or other tools on both .dts files together. The base-board .dts is unnecessary. dtc is fully capable of using /proc/device-tree as the source material. :-) g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Tue, Nov 6, 2012 at 10:30 AM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Hi Grant, On Nov 5, 2012, at 9:40 PM, Grant Likely wrote: Hey folks, As promised, here is my early draft to try and capture what device tree overlays need to do and how to get there. Comments and suggestions greatly appreciated. Device Tree Overlay Feature Purpose === Sometimes it is not convenient to describe an entire system with a single FDT. For example, processor modules that are plugged into one or more modules (a la the BeagleBone), or systems with an FPGA peripheral that is programmed after the system is booted. For these cases it is proposed to implement an overlay feature for the so that the initial device tree data can be modified by userspace at runtime by loading additional overlay FDTs that amend the original data. User Stories Note - These are potential use cases, but just because it is listed here doesn't mean it is important. I just want to thoroughly think through the implications before making design decisions. Jane is building custom BeagleBone expansion boards called 'capes'. She can boot the system with a stock BeagleBoard device tree, but additional data is needed before a cape can be used. She could replace the FDT file used by U-Boot with one that contains the extra data, but she uses the same Linux system image regardless of the cape, and it is inconvenient to have to select a different device tree at boot time depending on the cape. Jane solves this problem by storing an FDT overlay for each cape in the root filesystem. When the kernel detects that a cape is installed it reads the cape's eeprom to identify it and uses request_firmware() to obtain the appropriate overlay. Userspace passes the overlay to the kernel in the normal way. If the cape doesn't have an eeprom, then the kernel will still use firmware_request(), but userspace needs to already know which cape is installed. Jane is a really productive hardware engineer - she manages to fix a number of problems with her cape design by spinning different revisions of the cape. Using the flexibility that the DT provides, documents and defines the hardware changes of the cape revisions in the FDT overlay. The loader matches the revision of the cape with the proper FDT overlay so that the drivers are relieved of having to do revision management. Okay By installing dtc on the Pi, Mandy compiles the overlay for her prototype hardware. However, she doesn't have a copy of the Pi's original FDT source, so instead she uses the dtc 'fs' input format to compile the overlay file against the live DT data in /proc. Jane (the cape designer) can use this too. Developing the cape, she really appreciates that she doesn't have to reboot every time she makes a change in the cape hardware. By removing the FDT overlay, compiling with the dtc on the board, and re-inserting the overlay, she can be more productive by waiting less. Yes, but I'll leave this paragraph out of the spec. It isn't significantly different from what is already there. Johnny, Jane's little son, doesn't know anything about device trees, linux kernel trees, or hard-core s/w engineering. He is a bright kid, and due to the board having a node.js based educational electronic design kit, he can use the web-based simplified development environment, that allows him graphically to connect the parts in his kit. He can save the design and the IDE creates on the fly the DT overlay for later use. Yes. Joanne has purchased one of Jane's capes and packaged it into a rugged case for data logging. As far as Joanne is concerned, the BeagleBone and cape together are a single unit and she'd prefer a single monolithic FDT instead of using an FDT overlay. Option A: Using dtc, she uses the BeagleBone and cape .dts source files to generate a single .dtb for the entire system which is loaded by U-Boot. -or- Unlikely. Option B: Joanne uses a tool to merge the BeagleBone and cape .dtb files (instead of .dts files), -or- Possible but low probability. Option C: U-Boot loads both the base and overlay FDT files, merges them, and passes the resolved tree to the kernel. Could be made to work. Only really required if Joanne wants the cape interface to work for u-boot too. For example if the cape has some kind of network interface that u-boot will use to boot from. Unlikely for your focus perhaps, but I'm trying to capture all the relevant permutations, and I can guarantee that some people really will want this. If not on the bone, then on some other platform. Summary points: - Create an FDT overlay data format and usage model - SHALL reliable resolve or validate of phandles between base and overlay trees - SHOULD reliably handle changes between different underlying overlays (ie. what happens to existing .dtb overly files if the structure of the dtb it is layered
Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2
On Tue, Nov 6, 2012 at 8:14 AM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: On Nov 6, 2012, at 4:06 AM, Joel A Fernandes wrote: Sure, so if we add data type supplementary properties to the tree to indicate the data type as indirect phandle, then kernel could refer to the index in the got-like table to fetch the actual phandle address (1-level of indirection), instead of directly using the address in the data field. I'm fine with this. But if the data about phandles is already in the tree, then the need for a GOT simply goes away. The phandles can be fixed up directly and it is so much better because it works with existing parsing code after the merge is applied. g. -- 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] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Tue, Nov 6, 2012 at 7:34 PM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: On Nov 6, 2012, at 12:14 PM, Grant Likely wrote: On Tue, Nov 6, 2012 at 10:30 AM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: For hot-plugging, you need it. Whether kernel code can deal with large parts of the DT going away... How about we use the dead properties method and move/tag the removed modes as such, and not really remove them. Nodes already use krefs, and I'm thinking about making them kobjects so that they appear in sysfs and we'll have some tools to figure out when reference counts don't get decremented properly. From the little I've looked in the of code, and the drivers, it's going to be pretty bad. I don't think all users take references properly, and we have a big global lock for accessing the DT. I'm a lot more optimistic on this front... I wrote a patch today to make the change and took some measurements: On the versatile express qemu model I measured the free memory with /proc/device-tree, with /sys/device-tree, and with both. Here's what I found: /proc/device-tree only: 114776kB free /sys/device-tree only: 114792kB free both enabled: 114716kB free The back of a napkin calculation indicates that on this platform /proc/devicetree costs 76kB and /sys/device-tree costs 60kb. I'm happy to see that using /sys instead of /proc appears to be slightly cheaper which makes it easier to justify the change. The diffstat makes me even happier: arch/arm/plat-omap/Kconfig|1 - arch/powerpc/platforms/pseries/dlpar.c| 23 --- arch/powerpc/platforms/pseries/reconfig.c | 40 -- drivers/of/Kconfig|8 drivers/of/base.c | 116 drivers/of/fdt.c |5 ++- fs/proc/Makefile |1 - fs/proc/proc_devtree.c| 13 +- fs/proc/root.c|4 +- include/linux/of.h| 35 include/linux/proc_fs.h | 16 include/linux/string.h| 11 + 12 files changed, 107 insertions(+), 166 deletions(-) There are still a few odds and ends that need to be tidied up, but I'll get it out for review shortly. I've not touched the sparc code yet, and I need to take another look over the existing OF_DYNAMIC code. I think that CONFIG_OF_DYNAMIC will probably go away and the add node/property functions will get used by fdt.c and pdt.c for initial construction of the device tree. Adding and removing nodes at runtime as part of the normal operation of the system (and not as something that happens once in a blue moon under controlled conditions) will uncover lots of bugs. I'm hoping so! Its time to clean that mess up. :-) Fortunately adding nodes is not where we're going to have problems. The problems will be on node removal. Addition-only at least means we can have something useful before hunting down and squashing all the bugs. So let's think about locking too Yes, the locking does need to be sorted out. g. -- 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
