Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)

2012-11-07 Thread Ryan Mallon
On 06/11/12 08:40, Tabi Timur-B04825 wrote:
 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.

This is probably okay for some hardware, but doesn't work in the general
case. Not all hardware is detectable, for example a cape which just adds
a set of LEDs for GPIO pins. Also, some hardware might not easily be
detectable without adding additional complexity to the boot loader.

~Ryan

--
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 3/6] clk: introduce the common clock framework

2011-12-13 Thread Ryan Mallon
On 14/12/11 14:53, Mike Turquette wrote:

 The common clk framework is an attempt to define a common struct clk
 which can be used by most platforms, and an API that drivers can always
 use safely for managing clks.
 
 The net result is consolidation of many different struct clk definitions
 and platform-specific clk framework implementations.
 
 This patch introduces the common struct clk, struct clk_ops and
 definitions for the long-standing clk API in include/clk/clk.h.
 Platforms may define their own hardware-specific clk structure, so long
 as it wraps an instance of the common struct clk which is passed to
 clk_init at initialization time.  From this point on all of the usual
 clk APIs should be supported, so long as the platform supports them
 through hardware-specific callbacks in struct clk_ops.
 
 See Documentation/clk.txt for more details.
 
 This patch is based on the work of Jeremy Kerr, which in turn was based
 on the work of Ben Herrenschmidt.  Big thanks to both of them for
 kickstarting this effort.
 
 Signed-off-by: Mike Turquette mturque...@linaro.org
 Cc: Jeremy Kerr jeremy.k...@canonical.com


Hi Mike,

Some comments below,

~Ryan

 ---
  drivers/clk/Kconfig  |4 +
  drivers/clk/Makefile |1 +
  drivers/clk/clk.c|  564 
 ++
  include/linux/clk.h  |  130 +++-
  4 files changed, 695 insertions(+), 4 deletions(-)
  create mode 100644 drivers/clk/clk.c
 
 diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
 index 7a9899bd..adc0586 100644
 --- a/drivers/clk/Kconfig
 +++ b/drivers/clk/Kconfig
 @@ -8,3 +8,7 @@ config HAVE_MACH_CLKDEV
  
  config HAVE_CLK_PREPARE
   bool
 +
 +config GENERIC_CLK
 + bool
 + select HAVE_CLK_PREPARE
 diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
 index 07613fa..570d5b9 100644
 --- a/drivers/clk/Makefile
 +++ b/drivers/clk/Makefile
 @@ -1,2 +1,3 @@
  
  obj-$(CONFIG_CLKDEV_LOOKUP)  += clkdev.o
 +obj-$(CONFIG_GENERIC_CLK)+= clk.o
 diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
 new file mode 100644
 index 000..8cadadd
 --- /dev/null
 +++ b/drivers/clk/clk.c
 @@ -0,0 +1,564 @@
 +/*
 + * Copyright (C) 2010-2011 Canonical Ltd jeremy.k...@canonical.com
 + * Copyright (C) 2011 Linaro Ltd 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.
 + *
 + * Standard functionality for the common clock API.  See 
 Documentation/clk.txt
 + */
 +
 +#include linux/clk.h
 +#include linux/module.h
 +#include linux/mutex.h
 +#include linux/spinlock.h
 +#include linux/err.h
 +#include linux/list.h
 +
 +static DEFINE_SPINLOCK(enable_lock);
 +static DEFINE_MUTEX(prepare_lock);
 +
 +static HLIST_HEAD(clk_root_list);
 +
 +/***clk API***/
 +
 +void __clk_unprepare(struct clk *clk)
 +{
 + if (!clk)
 + return;
 +
 + if (WARN_ON(clk-prepare_count == 0))
 + return;
 +
 + if (--clk-prepare_count  0)
 + return;
 +
 + WARN_ON(clk-enable_count  0);
 +
 + if (clk-ops-unprepare)
 + clk-ops-unprepare(clk);
 +
 + __clk_unprepare(clk-parent);
 +}


I think you can rewrite this to get rid of the recursion as below:

while (clk) {
if (WARN_ON(clk-prepare_count == 0))
return;

if (--clk-prepare_count  0)
return;

WARN_ON(clk-enable_count  0)

if (clk-ops-unprepare)
clk-ops-unprepare(clk);

clk = clk-parent;
}

Also, should this function be static?

 +
 +/**
 + * clk_unprepare - perform the slow part of a clk gate
 + * @clk: the clk being gated
 + *
 + * clk_unprepare may sleep, which differentiates it from clk_disable.  In a
 + * simple case, clk_unprepare can be used instead of clk_disable to gate a 
 clk
 + * if the operation may sleep.  One example is a clk which is accessed over
 + * I2c.  In the complex case a clk gate operation may require a fast and a 
 slow
 + * part.  It is this reason that clk_unprepare and clk_disable are not 
 mutually
 + * exclusive.  In fact clk_disable must be called before clk_unprepare.
 + */
 +void clk_unprepare(struct clk *clk)
 +{
 + mutex_lock(prepare_lock);
 + __clk_unprepare(clk);
 + mutex_unlock(prepare_lock);
 +}
 +EXPORT_SYMBOL_GPL(clk_unprepare);
 +
 +int __clk_prepare(struct clk *clk)
 +{
 + int ret = 0;
 +
 + if (!clk)
 + return 0;
 +
 + if (clk-prepare_count == 0) {
 + ret = __clk_prepare(clk-parent);
 + if (ret)
 + return ret;
 +
 + if (clk-ops-prepare) {
 + ret = clk-ops-prepare(clk);
 + if (ret) {
 + __clk_unprepare(clk-parent);
 + return ret;
 +

Re: [PATCH v4 5/6] clk: basic gateable and fixed-rate clks

2011-12-13 Thread Ryan Mallon
On 14/12/11 14:53, Mike Turquette wrote:

 Many platforms support simple gateable clks and fixed-rate clks that
 should not be re-implemented by every platform.
 
 This patch introduces a gateable clk with a common programming model of
 gate control via a write of 1 bit to a register.  Both set-to-enable and
 clear-to-enable are supported.
 
 Also introduced is a fixed-rate clk which has no reprogrammable aspects.
 
 The purpose of both types of clks is documented in drivers/clk/basic.c.
 
 TODO: add support for a simple divider, simple mux and a dummy clk for
 stubbing out platform support.
 
 Based on original patch by Jeremy Kerr and contribution by Jamie Iles.
 
 Signed-off-by: Mike Turquette mturque...@linaro.org
 Cc: Jeremy Kerr jeremy.k...@canonical.com
 Cc: Jamie Iles ja...@jamieiles.com

snip

 +int clk_register_gate(struct device *dev, const char *name, unsigned long 
 flags,
 + struct clk *fixed_parent, void __iomem *reg, u8 bit_idx,
 + int set_to_enable)
 +{
 + struct clk_hw_gate *gclk;
 + struct clk *clk;
 +
 + gclk = kmalloc(sizeof(struct clk_hw_gate), GFP_KERNEL);
 +
 + if (!gclk) {
 + pr_err(%s: could not allocate gated clk\n, __func__);
 + return -ENOMEM;
 + }
 +
 + clk = gclk-clk;
 +
 + /* struct clk_hw_gate assignments */
 + gclk-fixed_parent = fixed_parent;
 + gclk-reg = reg;
 + gclk-bit_idx = bit_idx;
 +
 + /* struct clk assignments */
 + clk-name = name;
 + clk-flags = flags;
 +
 + if (set_to_enable)
 + clk-ops = clk_hw_gate_set_enable_ops;
 + else
 + clk-ops = clk_hw_gate_set_disable_ops;


You could avoid having two sets of operations if you stored the
set_to_enable value in struct clk_hw_gate. It might be useful to store
additional information in struct clk_hw_gate if you also want to extend
to supporting non-32bit registers (readb, etc), clocks with write only
registers, or support clocks which require more than one bit to be set
or cleared to enable them, etc. See the basic mmio gpio driver for a
similar case.

~Ryan

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv5 0/3] Introduce the /proc/socinfo and use it to export OMAP data

2011-03-01 Thread Ryan Mallon
On 03/02/2011 03:55 PM, Saravana Kannan wrote:
 On 03/01/2011 06:41 PM, Ryan Mallon wrote:
 On 03/02/2011 03:23 PM, Saravana Kannan wrote:
 I don't have any attachment to the arch file suggestion. If there is a
 better solution to identify the different implementations of socinfo
 without having to maintain some unique id list in the kernel, then I'm
 all for it. But cpuinfo is not it.

 Sorry I am confusing the 'arch' and 'mach' bits here. I definitely have
 an objection to having an 'arch' file (i.e. ARM). A 'mach' (i.e. omap)
 file makes a bit more sense, but should probably be called 'mach' rather
 than 'arch' to avoid this confusion :-).
 
 Sorry for the confusion. Sure, I don't care much for the filename as
 long as we can all agree on it. I care more about the content of the
 file (using names very close to  in mach-). I like soc-family
 better since it's generic enough to not force, say omap3 and omap4, to
 report different values.
 
 Linus Walleij, Eduardo, Maxime, Andrei,
 
 Would like to hear your opinion on the file name (soc-family vs. mach vs
 somethingelse) and the path /sys/devices/system/soc/.

'family' sounds good. I don't think we need the 'soc-' prefix on
filenames if they are already in /sys/devices/system/soc/.

 
 If we settle on this, may be it would be easier to get this through.
 
 I still think it is a solution in search of a problem though. What
 userspace programs need to know what specific SoC they are on? My
 feeling is that if userspace needs to know this information, then it is
 probably dicking around with things that should be managed by the
 kernel. Differences in available peripherals, etc can be determined by
 looking at existing sysfs files.
 
 I certainly have seen several use cases. Couple of easy examples:
 
 * A lot of test scripts would find this very useful. For example, some
 clock (present is all/most MSMs) shouldn't be tested on some SOCs as it
 would lock up the system if you try to turn it off while the CPU is
 running.

I don't follow here. Do you mean a struct clk clock or something else?
Why is userspace allowed to disable a clock which will effectively hang
the system? :-).

 * Some of the user space tools might want to report different product
 id/type (nothing to do with USB, etc) depending on what SOC it is
 running on.

This makes more sense. It would actually be useful for custom USB
devices (gadget) which can be done from user space.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon 5 Amuri Park, 404 Barbadoes St
r...@bluewatersys.com   PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127Freecall: Australia 1800 148 751
Fax:   +64 3 3779135  USA 1800 261 2934
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv5 0/3] Introduce the /proc/socinfo and use it to export OMAP data

2011-03-01 Thread Ryan Mallon
On 03/02/2011 04:21 PM, Saravana Kannan wrote:
 On 03/01/2011 07:11 PM, Ryan Mallon wrote:
 On 03/02/2011 03:55 PM, Saravana Kannan wrote:
 On 03/01/2011 06:41 PM, Ryan Mallon wrote:
 On 03/02/2011 03:23 PM, Saravana Kannan wrote:
 I don't have any attachment to the arch file suggestion. If there
 is a
 better solution to identify the different implementations of socinfo
 without having to maintain some unique id list in the kernel,
 then I'm
 all for it. But cpuinfo is not it.

 Sorry I am confusing the 'arch' and 'mach' bits here. I definitely have
 an objection to having an 'arch' file (i.e. ARM). A 'mach' (i.e. omap)
 file makes a bit more sense, but should probably be called 'mach'
 rather
 than 'arch' to avoid this confusion :-).

 Sorry for the confusion. Sure, I don't care much for the filename as
 long as we can all agree on it. I care more about the content of the
 file (using names very close to  in mach-). I like soc-family
 better since it's generic enough to not force, say omap3 and omap4, to
 report different values.

 Linus Walleij, Eduardo, Maxime, Andrei,

 Would like to hear your opinion on the file name (soc-family vs. mach vs
 somethingelse) and the path /sys/devices/system/soc/.

 'family' sounds good. I don't think we need the 'soc-' prefix on
 filenames if they are already in /sys/devices/system/soc/.
 
 Makes sense. We can drop the soc- prefix. So the contenders left: family
 vs somethingelse. Would still be nice if the other folks chime in.
 
 If we settle on this, may be it would be easier to get this through.

 I still think it is a solution in search of a problem though. What
 userspace programs need to know what specific SoC they are on? My
 feeling is that if userspace needs to know this information, then it is
 probably dicking around with things that should be managed by the
 kernel. Differences in available peripherals, etc can be determined by
 looking at existing sysfs files.

 I certainly have seen several use cases. Couple of easy examples:

 * A lot of test scripts would find this very useful. For example, some
 clock (present is all/most MSMs) shouldn't be tested on some SOCs as it
 would lock up the system if you try to turn it off while the CPU is
 running.

 I don't follow here. Do you mean a struct clk clock or something else?
 Why is userspace allowed to disable a clock which will effectively hang
 the system? :-).
 
 Ah, sorry. Didn't give enough details. To give some context, I manage
 the clock stuff for MSM. The MSM clock driver exports clock control thru
 debugfs. We have test scripts that bang the clocks to test them. Each
 SoC has a different set of touch me and you die clocks that the test
 script shouldn't mess with. This socinfo would be useful for those test
 cases.

Ah, okay. This is still within a single SoC family though since we don't
yet (AFAIK) support mutliple SoCs in a single kernel.

 * Some of the user space tools might want to report different product
 id/type (nothing to do with USB, etc) depending on what SOC it is
 running on.

 This makes more sense. It would actually be useful for custom USB
 devices (gadget) which can be done from user space.
 
 Hmm... didn't know USB devices/gadgets could be handled from userspace.

The gadgetfs driver allows for writing custom usb device
implementations. The SoC info could be used to set the USB
vendor/product id. Again, I see this more useful within the SoC family
(ie at91sam9260 vs at91sam9263) rather than between families. From an
embedded perspective at least, I think it is unlikely for an application
to need to work on multiple SoC families.

The only real objection I have to adding the SoC family information is
basically to discourage it being abused by userspace. I can see it being
useful in debug situations, but I can also see stupid userspace
applications explicitly testing for some particular SoC, rather than
more correctly (IMHO) checking for presence of certain drivers etc.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon 5 Amuri Park, 404 Barbadoes St
r...@bluewatersys.com   PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127Freecall: Australia 1800 148 751
Fax:   +64 3 3779135  USA 1800 261 2934
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv5 0/3] Introduce the /proc/socinfo and use it to export OMAP data

2011-03-01 Thread Ryan Mallon
On 03/02/2011 04:46 PM, Saravana Kannan wrote:
 On 03/01/2011 07:35 PM, Ryan Mallon wrote:
 On 03/02/2011 04:21 PM, Saravana Kannan wrote:
 On 03/01/2011 07:11 PM, Ryan Mallon wrote:
 On 03/02/2011 03:55 PM, Saravana Kannan wrote:
 On 03/01/2011 06:41 PM, Ryan Mallon wrote:
 On 03/02/2011 03:23 PM, Saravana Kannan wrote:

snip

 The only real objection I have to adding the SoC family information is
 basically to discourage it being abused by userspace. I can see it being
 useful in debug situations, but I can also see stupid userspace
 applications explicitly testing for some particular SoC, rather than
 more correctly (IMHO) checking for presence of certain drivers etc.
 
 True, but so many other things could be misused by stupid userspace
 programs. When there are legitimate usecases, I think we shouldn't
 prevent them just because we think a stupid userspace program could
 misuse it.
 
 Again, although you might not be gung-ho about this, I think I have at
 least made you indifferent/mildly supportive to adding socinfo. If you
 don't mind, I would like to wait for others to chime in before
 continuing this discussion.

Agreed.

In general I am in support of having the SoC information exposed
somewhere. I think we just want to be careful that it doesn't become a
dumping ground for anything and everything SoC related whether the
information is useful or not. I think each piece of exposed information
should have a genuine use case, not just because we can.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon 5 Amuri Park, 404 Barbadoes St
r...@bluewatersys.com   PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127Freecall: Australia 1800 148 751
Fax:   +64 3 3779135  USA 1800 261 2934
--
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] omap: add hwspinlock device

2010-10-19 Thread Ryan Mallon
On 10/20/2010 12:53 PM, Kevin Hilman wrote:
 Ohad Ben-Cohen o...@wizery.com writes:
 
 On Tue, Oct 19, 2010 at 7:03 PM, Kevin Hilman
 khil...@deeprootsystems.com wrote:
 +postcore_initcall(hwspinlocks_init);

 Any reason this needs to be a postcore_initcall?  Are there users of
 hwspinlocks this early in boot?

 i2c-omap, which is subsys_initcall (the I2C bus is shared between the
 A9 and the M3 on some OMAP4 boards).
 
 Rather than moving towards having more drivers have to be built in (and
 depend on their probe order) we need to be moving towards building all
 these drivers as modules, including omap-i2c.

The issue of probe order still needs to be resolved for those of us who
do want all the drivers built into the kernel. Everything comes out in
the wash already if everything is built as modules by installing the
modules in the correct order right?

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon 5 Amuri Park, 404 Barbadoes St
r...@bluewatersys.com   PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127Freecall: Australia 1800 148 751
Fax:   +64 3 3779135  USA 1800 261 2934
--
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