Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 REPOST 22/24] gpio/omap: enable irq at the end of all configuration in restore
From: Tarun Kanti DebBarma @ 2011-09-27  0:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1317082460-31706-1-git-send-email-tarun.kanti@ti.com>

From: Nishanth Menon <nm@ti.com>

Setup the interrupt enable registers only after we have configured the
required edge and required configurations, not before, to prevent
spurious events as part of restore routine.

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 drivers/gpio/gpio-omap.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 981a1c1..75b6265 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1310,10 +1310,6 @@ void omap2_gpio_resume_after_idle(void)
 
 static void omap_gpio_restore_context(struct gpio_bank *bank)
 {
-	__raw_writel(bank->context.irqenable1,
-				bank->base + bank->regs->irqenable);
-	__raw_writel(bank->context.irqenable2,
-				bank->base + bank->regs->irqenable2);
 	__raw_writel(bank->context.wake_en,
 				bank->base + bank->regs->wkup_en);
 	__raw_writel(bank->context.ctrl, bank->base + bank->regs->ctrl);
@@ -1333,6 +1329,11 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
 		__raw_writel(bank->context.debounce_en,
 					bank->base + bank->regs->debounce_en);
 	}
+
+	__raw_writel(bank->context.irqenable1,
+				bank->base + bank->regs->irqenable);
+	__raw_writel(bank->context.irqenable2,
+				bank->base + bank->regs->irqenable2);
 }
 #else
 #define omap_gpio_suspend NULL
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH v7 REPOST 23/24] gpio/omap: restore OE only after setting the output level
From: Tarun Kanti DebBarma @ 2011-09-27  0:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1317082460-31706-1-git-send-email-tarun.kanti@ti.com>

From: Nishanth Menon <nm@ti.com>

Setup the dataout register before restoring OE. This is to make
sure that we have valid data in dataout register which would be
made available in output pins as soon as OE is enabled. Else,
there is risk of unknown data getting out into gpio pins.

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 drivers/gpio/gpio-omap.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 75b6265..05ab0e8 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1313,7 +1313,6 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
 	__raw_writel(bank->context.wake_en,
 				bank->base + bank->regs->wkup_en);
 	__raw_writel(bank->context.ctrl, bank->base + bank->regs->ctrl);
-	__raw_writel(bank->context.oe, bank->base + bank->regs->direction);
 	__raw_writel(bank->context.leveldetect0,
 				bank->base + bank->regs->leveldetect0);
 	__raw_writel(bank->context.leveldetect1,
@@ -1323,6 +1322,8 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
 	__raw_writel(bank->context.fallingdetect,
 				bank->base + bank->regs->fallingdetect);
 	__raw_writel(bank->context.dataout, bank->base + bank->regs->dataout);
+	__raw_writel(bank->context.oe, bank->base + bank->regs->direction);
+
 	if (bank->dbck_enable_mask) {
 		__raw_writel(bank->context.debounce, bank->base +
 					bank->regs->debounce);
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH v7 REPOST 24/24] gpio/omap: handle set_dataout reg capable IP on restore
From: Tarun Kanti DebBarma @ 2011-09-27  0:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1317082460-31706-1-git-send-email-tarun.kanti@ti.com>

From: Nishanth Menon <nm@ti.com>

GPIO IP revisions such as those used in OMAP4 have a set_dataout
while the previous revisions used a single dataout register.
Depending on what is available restore the dataout settings
to the right register.

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 drivers/gpio/gpio-omap.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 05ab0e8..349e774 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1048,7 +1048,7 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 	bank->get_context_loss_count = pdata->get_context_loss_count;
 	bank->regs = pdata->regs;
 
-	if (bank->regs->set_dataout && bank->regs->clr_dataout)
+	if (bank->regs->set_dataout)
 		bank->set_dataout = _set_gpio_dataout_reg;
 	else
 		bank->set_dataout = _set_gpio_dataout_mask;
@@ -1321,7 +1321,12 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
 				bank->base + bank->regs->risingdetect);
 	__raw_writel(bank->context.fallingdetect,
 				bank->base + bank->regs->fallingdetect);
-	__raw_writel(bank->context.dataout, bank->base + bank->regs->dataout);
+	if (bank->regs->set_dataout)
+		__raw_writel(bank->context.dataout,
+				bank->base + bank->regs->set_dataout);
+	else
+		__raw_writel(bank->context.dataout,
+				bank->base + bank->regs->dataout);
 	__raw_writel(bank->context.oe, bank->base + bank->regs->direction);
 
 	if (bank->dbck_enable_mask) {
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH v2 00/16] Switch GIC users (and omap2plus) to CONFIG_MULTI_IRQ_HANDLER
From: David Brown @ 2011-09-27  0:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110926202007.GB27355@huya.qualcomm.com>

On Mon, Sep 26, 2011 at 01:20:07PM -0700, David Brown wrote:
> On Mon, Sep 26, 2011 at 12:02:19PM +0100, Marc Zyngier wrote:
> 
> > This series has been tested on VE (A9, A5, A15), PB11MP, Panda, IGEPv2
> > and Harmony. Patches against next-20110926 plus my PPI series.
> 
> Tested on an msm8660.
> 
> Tested-by: David Brown <davidb@codeaurora.org>
> Acked-by: David Brown <davidb@codeaurora.org>

Tested as well on msm8250.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

^ permalink raw reply

* Pull request: removal of most instances of mach/memory.h
From: Nicolas Pitre @ 2011-09-27  0:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110926232943.GD23680@n2100.arm.linux.org.uk>

On Tue, 27 Sep 2011, Russell King - ARM Linux wrote:

> On Mon, Sep 26, 2011 at 07:03:12PM -0400, Nicolas Pitre wrote:
> > On Mon, 26 Sep 2011, Russell King - ARM Linux wrote:
> > 
> > > On Mon, Sep 26, 2011 at 04:10:03PM -0400, Nicolas Pitre wrote:
> > > > On Mon, 26 Sep 2011, Nicolas Pitre wrote:
> > > > 
> > > > > On Mon, 26 Sep 2011, Russell King - ARM Linux wrote:
> > > > > 
> > > > > > This also gives additional merge conflicts elsewhere, and while git
> > > > > > rerere makes some of them easy, the quantity is going to be a right
> > > > > > pain to deal with on a repeated basis.
> > > > > 
> > > > > Again, I'm perfectly willing to use a different base for this series and 
> > > > > fix the conflicts myself if you give me one.
> > > > 
> > > > Nevermind, I see that you merged it anyway.
> > > > 
> > > > You may consider the rerere.autoupdate config option if you want Git to 
> > > > automatically pick the conflict resolution without you manually 
> > > > confirming them all the time.
> > > 
> > > rerere doesn't deal with modified/deleted conflicts though.
> > 
> > True.  That can be considered a limitation worth fixing.
> 
> Well.  Now I'm not impressed, having investigated the reason behind
> some of these conflicts.
> 
> The reason I'm getting the modify/delete conflict is because you've
> made the same change to arch/arm/mach-prima2/include/mach/memory.h
> as the one which you already submitted to me.
> 
> I've no idea why you submitted this change to me if you'd already merged
> it into your own tree - this is total madness and is just making more
> work for me with no reason.

I submitted that change to you separately via the patch system because I 
was getting no feedback from you despite two subsequent pings.  Hence 
after more than two weeks of total silence I decided to send the obvious 
pieces separately.  

Today you replied saying that the series is unacceptable because it 
contains a patch you can't test right now, even if I clearly stated that 
I could remove it in my pull request message.

Then I notice you merged both the separate patch and the modified 
request, although I had given up hope about the later.

> A little more discipline and communication (such as asking me to drop
> the duplicate commit _first_) would be nice.

Certainly.  Improving communication both ways might help.


Nicolas

^ permalink raw reply

* Pull request: removal of most instances of mach/memory.h
From: Nicolas Pitre @ 2011-09-27  0:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1109262028310.2718@xanadu.home>

On Mon, 26 Sep 2011, Nicolas Pitre wrote:

> On Tue, 27 Sep 2011, Russell King - ARM Linux wrote:
> 
> > A little more discipline and communication (such as asking me to drop
> > the duplicate commit _first_) would be nice.
> 
> Certainly.  Improving communication both ways might help.

One way this could be improved would be for your patch system to send 
notification emails with the submitter address explicit in the To: 
field.  Right now it contains linux-arm-patches at lists.arm.linux.org.uk 
only, therefore those emails don't get sorted with the best priority 
they could get.


Nicolas

^ permalink raw reply

* [PATCH 3/4] iommu/exynos: Add iommu driver for Exynos4 Platforms
From: KyongHo Cho @ 2011-09-27  0:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAK=WgbYzZKTRRf6hk=L5ZvXmFAUpq66Ud3F7xh2-8j6zSvAq9g@mail.gmail.com>

On Tue, Sep 27, 2011 at 12:59 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> On Mon, Sep 26, 2011 at 5:48 PM, KyongHo Cho <pullip.cho@samsung.com> wrote:
>> I am sorry but I still think that
>> installing default fault handler is quite simple and straightforward.
>
> ... and abusing the IOMMU API.
>
> Please don't do that. Interfaces are written for specific goals, and
> this one was added for IOMMU users, not drivers. Just like all the
> other map/unmap/attach/... APIs.
>
> By using this from the IOMMU driver itself, you are adding burden on
> future development and evolution of this API. As use cases
> materialize, we will have to change it to support them, while
> considering existing use cases. Using this API inappropriately will
> make our life harder later on.
I didn't think about this case.
It can be a problem if IOMMU driver is dependent upon the change of
IOMMU API as you told above.

>
> Moreover, I'm really not sure how exactly are you going to use this.
>
> If you're having an IOMMU user which installs its own fault handler,
> then this is all moot since the default behavior you provide here will
> be overridden.
Yes, this is what I really want.
I wanted the IOMMU driver to provide default fault handler to its users
that does not provide fault handler for debugging purpose.
If a user want to handle MMU fault, IOMMU driver welcomes to overwrite
the existing 'default' fault handler.

> If you're not having an IOMMU user which installs its own fault
> handler, then just provide the default behavior in your irq handler,
> regardless whether you report the fault to the IOMMU core or not. Or
> only when it fails. and yes, we can provide different error values for
> different scenarios. it's not rocket science :).
>
> Really, there's a myriad of ways to do this right. Please explain your
> exact settings/use-case and I'll gladly help you find a clean
> solution.
>
Thank you.

As I explained before, MMU fault in our system is abnormal and
the situation cannot be recovered because mapping information is set up
completely before System MMU starts address translation with the mapping
information.
However we have to capture the MMU fault for debugging purpose.
That's why our fault handler just prints debugging messages and
generates kernel OOPS

A user of IOMMU may handle MMU faults due to the following reason:
 - Its IOMMU sees the virtual address space of the user process (IOMMU
refers the page table of CPU)
   and virtual memory region allocated by userspace is not filled with
physical memory.
   Device driver which is user of IOMMU does not call get_user_pages()
for the region but
   handle page fault generated by IOMMU.
 - A device driver working in the Monitor mode in ARM processors need
to protect a memory region
   from access in the Normal mode. If it happens, the device driver
must catch the situation,
   stop the current operation and inform the problem to the userspace.

For the above reasons, some user of IOMMU may override the default
fault handler.

> Thanks,
> Ohad.

Thanks and regards,
Cho KyongHo.

^ permalink raw reply

* [RFC 1/5] ARM: dev_archdata: add private iommu extension
From: Grant Likely @ 2011-09-27  1:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316948337-7924-2-git-send-email-ohad@wizery.com>

On Sun, Sep 25, 2011 at 01:58:53PM +0300, Ohad Ben-Cohen wrote:
> Add a private iommu pointer to the ARM-specific arch data in the
> device struct, which will be used to attach iommu-specific data
> to devices which require iommu support.
> 
> Different iommu implementations (on different platforms) will attach
> different types of data to this pointer, so 'void *' is currently used
> (the downside is reduced typesafety).
> 
> Note: ia64, x86 and sparc have this exact iommu extension as well, and
> if others are likely to adopt it too, we might want to consider
> adding this to the device struct itself directly.
> 
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> ---
>  arch/arm/include/asm/device.h |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
> index 9f390ce..6615f03 100644
> --- a/arch/arm/include/asm/device.h
> +++ b/arch/arm/include/asm/device.h
> @@ -10,6 +10,9 @@ struct dev_archdata {
>  #ifdef CONFIG_DMABOUNCE
>  	struct dmabounce_device_info *dmabounce;
>  #endif
> +#ifdef CONFIG_IOMMU_API
> +	void *iommu; /* private IOMMU data */
> +#endif

Blech.  Oh well.  Not much point in doing something different if x86
uses a void*.

g.

^ permalink raw reply

* [PATCH v3 2/2] OMAP: omap_device: Add a method to build an omap_device from a DT node
From: Grant Likely @ 2011-09-27  1:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316724745-24896-3-git-send-email-b-cousson@ti.com>

On Thu, Sep 22, 2011 at 10:52:25PM +0200, Benoit Cousson wrote:
> Add a notifier called during device_add phase. If an of_node is present,
> retrieve the hwmod entry in order to populate properly the omap_device
> structure.
> For the moment the resource from the device-tree are overloaded.
> DT does not support named resource yet, and thus, most driver
> will not work without that information.
> 
> Add two helpers function to parse a property that contains multiple
> strings. Please note that these helpers will be replaced by a more generic
> iterator currently proposed on devicetree mailing list as soon as it
> will be merged.
> 
> Add a documentation to capture the specifics OMAP bindings
> needed for device-tree support.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>
> ---
>  .../devicetree/bindings/arm/omap/omap.txt          |   42 ++++++
>  arch/arm/plat-omap/omap_device.c                   |  145 ++++++++++++++++++++
>  2 files changed, 187 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/omap/omap.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
> new file mode 100644
> index 0000000..6513d05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> @@ -0,0 +1,42 @@
> +* Texas Instruments OMAP
> +
> +OMAP is currently using a static file per SoC family to describe the
> +IPs present in the SoC.
> +On top of that an omap_device is created to extend the platform_device
> +capabilities and to allow binding with one or several hwmods.
> +The hwmods will contain all the information to build the device:
> +adresse range, irq lines, dma lines, interconnect, PRCM register,
> +clock domain, input clocks.
> +For the moment just point to the existing hwmod, the next step will be
> +to move data from hwmod to device-tree representation.
> +
> +
> +Required properties:
> +- compatible: Every devices present in OMAP SoC should be in the
> +  form: "ti,XXX"
> +- ti,hwmods: list of hwmods attached to a device. Must contain at least
> +  one hwmod.

Nit: This should specify that ti,hwmods is a list of hwmod names
(ascii strings), and that the hwmod names come from the OMAP
documentation.  Don't respin the patch over this though, just do a
follow-up patch.

> +
> +Optional properties:
> +- ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
> +  during suspend.
> +
> +
> +Example:
> +
> +spinlock at 1 {
> +    compatible = "ti,omap-spinlock";
> +    ti,hwmods = "spinlock";
> +};
> +
> +
> +Boards:
> +
> +- OMAP3 BeagleBoard : Low cost community board
> +  compatible = "ti,omap3-beagle", "ti,omap3"
> +
> +- OMAP4 SDP : Software Developement Board
> +  compatible = "ti,omap4-sdp", "ti,omap4430"
> +
> +- OMAP4 PandaBoard : Low cost community board
> +  compatible = "ti,omap4-panda", "ti,omap4430"
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index cac7b9a..5b63989 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -85,6 +85,8 @@
>  #include <linux/clk.h>
>  #include <linux/clkdev.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/of.h>
> +#include <linux/notifier.h>
>  
>  #include <plat/omap_device.h>
>  #include <plat/omap_hwmod.h>
> @@ -94,12 +96,15 @@
>  #define USE_WAKEUP_LAT			0
>  #define IGNORE_WAKEUP_LAT		1
>  
> +#define MAX_HWMOD_NAME_SIZE		32
> +
>  static int omap_device_register(struct platform_device *pdev);
>  static int omap_early_device_register(struct platform_device *pdev);
>  static struct omap_device *omap_device_alloc(struct platform_device *pdev,
>  				      struct omap_hwmod **ohs, int oh_cnt,
>  				      struct omap_device_pm_latency *pm_lats,
>  				      int pm_lats_cnt);
> +static void omap_device_delete(struct omap_device *od);
>  
>  
>  static struct omap_device_pm_latency omap_default_latency[] = {
> @@ -315,6 +320,138 @@ static void _add_hwmod_clocks_clkdev(struct omap_device *od,
>  		_add_clkdev(od, oh->opt_clks[i].role, oh->opt_clks[i].clk);
>  }
>  
> +/*
> + * XXX: DT helper functions that should be replaced by more generic
> + * API introduced by Stephen Warren once they'll be in mainline.
> + */
> +static int _dt_count_property_string(const char *prop, int len)
> +{
> +	int i = 0;
> +	size_t l = 0, total = 0;
> +
> +	if (!prop || !len)
> +		return -EINVAL;
> +
> +	for (i = 0; len >= total; total += l, prop += l) {
> +		l = strlen(prop) + 1;
> +		if (*prop != 0)
> +			i++;
> +	}
> +	return i;
> +}
> +
> +static int _dt_get_property(const char *prop, int len, int index, char *output,
> +			    int size)
> +{
> +	int i = 0;
> +	size_t l = 0, total = 0;
> +
> +	if (!prop || !len)
> +		return -EINVAL;
> +
> +	for (i = 0; len >= total; total += l, prop += l) {
> +		l = strlcpy(output, prop, size) + 1;
> +		if (*prop != 0) {
> +			if (i++ == index)
> +				return 0;
> +		}
> +	}
> +	return -ENODEV;
> +}

Don't merge this.  Kevin or I could put Stephen's patch into a separate
branch that both Kevin and I pull.  There's no need to merge temporary
code.

That said, I just looked at Stephen's iterator, and even without it
this particular hunk shouldn't be merged here.  A
of_property_count_strings() function is useful in and of itself, and
of_property_read_string() could be extended with an
of_property_read_string_index() version.  Both changes should be in
the common drivers/of code, and you can easily do them.

Otherwise, both patches look good to me.

g.

> +
> +static struct dev_pm_domain omap_device_pm_domain;
> +
> +/**
> + * omap_device_build_from_dt - build an omap_device with multiple hwmods
> + * @pdev_name: name of the platform_device driver to use
> + * @pdev_id: this platform_device's connection ID
> + * @oh: ptr to the single omap_hwmod that backs this omap_device
> + * @pdata: platform_data ptr to associate with the platform_device
> + * @pdata_len: amount of memory pointed to by @pdata
> + * @pm_lats: pointer to a omap_device_pm_latency array for this device
> + * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
> + * @is_early_device: should the device be registered as an early device or not
> + *
> + * Function for building an omap_device already registered from device-tree
> + *
> + * Returns 0 or PTR_ERR() on error.
> + */
> +static int omap_device_build_from_dt(struct platform_device *pdev)
> +{
> +	struct omap_hwmod **hwmods;
> +	struct omap_device *od;
> +	struct omap_hwmod *oh;
> +	char oh_name[MAX_HWMOD_NAME_SIZE];
> +	const char *prop;
> +	int oh_cnt, i, prop_len;
> +	int ret = 0;
> +
> +	prop = of_get_property(pdev->dev.of_node, "ti,hwmods", &prop_len);
> +	oh_cnt = _dt_count_property_string(prop, prop_len);
> +	if (!oh_cnt || IS_ERR_VALUE(oh_cnt)) {
> +		dev_warn(&pdev->dev, "No 'hwmods' to build omap_device\n");
> +		return -ENODEV;
> +	}
> +
> +	hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt, GFP_KERNEL);
> +	if (!hwmods) {
> +		ret = -ENOMEM;
> +		goto odbfd_exit;
> +	}
> +
> +	for (i = 0; i < oh_cnt; i++) {
> +		_dt_get_property(prop, prop_len, i, oh_name,
> +				 MAX_HWMOD_NAME_SIZE);
> +
> +		oh = omap_hwmod_lookup(oh_name);
> +		if (!oh) {
> +			dev_err(&pdev->dev, "Cannot lookup hwmod '%s'\n",
> +				oh_name);
> +			ret = -EINVAL;
> +			goto odbfd_exit1;
> +		}
> +		hwmods[i] = oh;
> +	}
> +
> +	od = omap_device_alloc(pdev, hwmods, oh_cnt, NULL, 0);
> +	if (!od) {
> +		dev_err(&pdev->dev, "Cannot allocate omap_device for :%s\n",
> +			oh_name);
> +		ret = PTR_ERR(od);
> +		goto odbfd_exit1;
> +	}
> +
> +	if (of_get_property(pdev->dev.of_node, "ti,no_idle_on_suspend", NULL))
> +		omap_device_disable_idle_on_suspend(pdev);
> +
> +	pdev->dev.pm_domain = &omap_device_pm_domain;
> +
> +odbfd_exit1:
> +	kfree(hwmods);
> +odbfd_exit:
> +	return ret;
> +}
> +
> +static int _omap_device_notifier_call(struct notifier_block *nb,
> +				      unsigned long event, void *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	switch (event) {
> +	case BUS_NOTIFY_ADD_DEVICE:
> +		if (pdev->dev.of_node)
> +			omap_device_build_from_dt(pdev);
> +		break;
> +
> +	case BUS_NOTIFY_DEL_DEVICE:
> +		if (pdev->archdata.od)
> +			omap_device_delete(pdev->archdata.od);
> +		break;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
>  
>  /* Public functions for use by core code */
>  
> @@ -499,6 +636,9 @@ oda_exit1:
>  
>  static void omap_device_delete(struct omap_device *od)
>  {
> +	if (!od)
> +		return;
> +
>  	od->pdev->archdata.od = NULL;
>  	kfree(od->pm_lats);
>  	kfree(od->hwmods);
> @@ -1034,8 +1174,13 @@ struct device omap_device_parent = {
>  	.parent         = &platform_bus,
>  };
>  
> +static struct notifier_block platform_nb = {
> +	.notifier_call = _omap_device_notifier_call,
> +};
> +
>  static int __init omap_device_init(void)
>  {
> +	bus_register_notifier(&platform_bus_type, &platform_nb);
>  	return device_register(&omap_device_parent);
>  }
>  core_initcall(omap_device_init);
> -- 
> 1.7.0.4
> 

^ permalink raw reply

* [PATCH v4] of/irq: introduce of_irq_init
From: Grant Likely @ 2011-09-27  1:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1317065083-23573-1-git-send-email-robherring2@gmail.com>

On Mon, Sep 26, 2011 at 02:24:43PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> of_irq_init will scan the devicetree for matching interrupt controller
> nodes. Then it calls an initialization function for each found controller
> in the proper order with parent nodes initialized before child nodes.
> 
> Based on initial pseudo code from Grant Likely.
> 
> Changes in v4:
> - Drop unnecessary empty list check
> - Be more verbose on errors
> - Simplify "if (!desc) WARN_ON(1)" to "if (WARN_ON(!desc))"
> 
> Changes in v3:
> - add missing kfree's found by Jamie
> - Implement Grant's comments to simplify the init loop
> - fix function comments
> 
> Changes in v2:
> - Complete re-write of list searching code from Grant Likely
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>

Looks good to me.  Merged.

g.

> ---
>  drivers/of/irq.c       |  107 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_irq.h |    3 +
>  2 files changed, 110 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 6a5b5e7..6d3dd39 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -19,10 +19,12 @@
>   */
>  
>  #include <linux/errno.h>
> +#include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/string.h>
> +#include <linux/slab.h>
>  
>  /* For archs that don't support NO_IRQ (such as x86), provide a dummy value */
>  #ifndef NO_IRQ
> @@ -386,3 +388,108 @@ int of_irq_to_resource_table(struct device_node *dev, struct resource *res,
>  
>  	return i;
>  }
> +
> +struct intc_desc {
> +	struct list_head	list;
> +	struct device_node	*dev;
> +	struct device_node	*interrupt_parent;
> +};
> +
> +/**
> + * of_irq_init - Scan and init matching interrupt controllers in DT
> + * @matches: 0 terminated array of nodes to match and init function to call
> + *
> + * This function scans the device tree for matching interrupt controller nodes,
> + * and calls their initialization functions in order with parents first.
> + */
> +void __init of_irq_init(const struct of_device_id *matches)
> +{
> +	struct device_node *np, *parent = NULL;
> +	struct intc_desc *desc, *temp_desc;
> +	struct list_head intc_desc_list, intc_parent_list;
> +
> +	INIT_LIST_HEAD(&intc_desc_list);
> +	INIT_LIST_HEAD(&intc_parent_list);
> +
> +	for_each_matching_node(np, matches) {
> +		if (!of_find_property(np, "interrupt-controller", NULL))
> +			continue;
> +		/*
> +		 * Here, we allocate and populate an intc_desc with the node
> +		 * pointer, interrupt-parent device_node etc.
> +		 */
> +		desc = kzalloc(sizeof(*desc), GFP_KERNEL);
> +		if (WARN_ON(!desc))
> +			goto err;
> +
> +		desc->dev = np;
> +		desc->interrupt_parent = of_irq_find_parent(np);
> +		list_add_tail(&desc->list, &intc_desc_list);
> +	}
> +
> +	/*
> +	 * The root irq controller is the one without an interrupt-parent.
> +	 * That one goes first, followed by the controllers that reference it,
> +	 * followed by the ones that reference the 2nd level controllers, etc.
> +	 */
> +	while (!list_empty(&intc_desc_list)) {
> +		/*
> +		 * Process all controllers with the current 'parent'.
> +		 * First pass will be looking for NULL as the parent.
> +		 * The assumption is that NULL parent means a root controller.
> +		 */
> +		list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) {
> +			const struct of_device_id *match;
> +			int ret;
> +			of_irq_init_cb_t irq_init_cb;
> +
> +			if (desc->interrupt_parent != parent)
> +				continue;
> +
> +			list_del(&desc->list);
> +			match = of_match_node(matches, desc->dev);
> +			if (WARN(!match->data,
> +			    "of_irq_init: no init function for %s\n",
> +			    match->compatible)) {
> +				kfree(desc);
> +				continue;
> +			}
> +
> +			pr_debug("of_irq_init: init %s @ %p, parent %p\n",
> +				 match->compatible,
> +				 desc->dev, desc->interrupt_parent);
> +			irq_init_cb = match->data;
> +			ret = irq_init_cb(desc->dev, desc->interrupt_parent);
> +			if (ret) {
> +				kfree(desc);
> +				continue;
> +			}
> +
> +			/*
> +			 * This one is now set up; add it to the parent list so
> +			 * its children can get processed in a subsequent pass.
> +			 */
> +			list_add_tail(&desc->list, &intc_parent_list);
> +		}
> +
> +		/* Get the next pending parent that might have children */
> +		desc = list_first_entry(&intc_parent_list, typeof(*desc), list);
> +		if (list_empty(&intc_parent_list) || !desc) {
> +			pr_err("of_irq_init: children remain, but no parents\n");
> +			break;
> +		}
> +		list_del(&desc->list);
> +		parent = desc->dev;
> +		kfree(desc);
> +	}
> +
> +	list_for_each_entry_safe(desc, temp_desc, &intc_parent_list, list) {
> +		list_del(&desc->list);
> +		kfree(desc);
> +	}
> +err:
> +	list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) {
> +		list_del(&desc->list);
> +		kfree(desc);
> +	}
> +}
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index cd2e61c..d0307ee 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -33,6 +33,8 @@ struct of_irq {
>  	u32 specifier[OF_MAX_IRQ_SPEC]; /* Specifier copy */
>  };
>  
> +typedef int (*of_irq_init_cb_t)(struct device_node *, struct device_node *);
> +
>  /*
>   * Workarounds only applied to 32bit powermac machines
>   */
> @@ -73,6 +75,7 @@ extern int of_irq_to_resource_table(struct device_node *dev,
>  		struct resource *res, int nr_irqs);
>  extern struct device_node *of_irq_find_parent(struct device_node *child);
>  
> +extern void of_irq_init(const struct of_device_id *matches);
>  
>  #endif /* CONFIG_OF_IRQ */
>  #endif /* CONFIG_OF */
> -- 
> 1.7.5.4
> 

^ permalink raw reply

* [PATCH v2 16/16] ARM: GIC: Make MULTI_IRQ_HANDLER mandatory
From: Shawn Guo @ 2011-09-27  3:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1317034955-1029-17-git-send-email-marc.zyngier@arm.com>

On Mon, Sep 26, 2011 at 12:02:35PM +0100, Marc Zyngier wrote:
> Now that MULTI_IRQ_HANDLER is selected by all the in-tree
> GIC users, make it mandatory and remove the unused macros.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/Kconfig                                |    3 -
>  arch/arm/common/Kconfig                         |    1 +
>  arch/arm/common/gic.c                           |    2 +-
>  arch/arm/include/asm/hardware/entry-macro-gic.S |   60 -----------------------
>  arch/arm/include/asm/hardware/gic.h             |    1 -
>  arch/arm/mach-msm/Kconfig                       |    2 -
>  arch/arm/mach-omap2/Kconfig                     |    1 +
>  arch/arm/mach-tegra/Kconfig                     |    1 -
>  arch/arm/mach-ux500/Kconfig                     |    1 -
>  arch/arm/plat-omap/Kconfig                      |    1 -
>  10 files changed, 3 insertions(+), 70 deletions(-)
>  delete mode 100644 arch/arm/include/asm/hardware/entry-macro-gic.S
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 0514264..d3e246c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -334,7 +334,6 @@ config ARCH_CNS3XXX
>  	select ARM_GIC
>  	select MIGHT_HAVE_PCI
>  	select PCI_DOMAINS if PCI
> -	select MULTI_IRQ_HANDLER
>  	help
>  	  Support for Cavium Networks CNS3XXX platform.
>  
> @@ -788,7 +787,6 @@ config ARCH_EXYNOS4
>  	select HAVE_S3C_RTC if RTC_CLASS
>  	select HAVE_S3C2410_I2C if I2C
>  	select HAVE_S3C2410_WATCHDOG if WATCHDOG
> -	select MULTI_IRQ_HANDLER
>  	help
>  	  Samsung EXYNOS4 series based systems
>  
> @@ -911,7 +909,6 @@ config ARCH_ZYNQ
>  	select ARM_AMBA
>  	select ICST
>  	select USE_OF
> -	select MULTI_IRQ_HANDLER
>  	help
>  	  Support for Xilinx Zynq ARM Cortex A9 Platform
>  endchoice

Hi Marc,

I'm unsure about your base.  But I have the following platforms in
arch/arm/Kconfig selecting MULTI_IRQ_HANDLER at the end of your series,
which should also be removed from?

  ARCH_REALVIEW
  ARCH_VEXPRESS
  ARCH_PXA
  ARCH_SHMOBILE

-- 
Regards,
Shawn

^ permalink raw reply

* [RFC PATCH 3/3] ARM: mm: add l2x0 suspend/resume support
From: Santosh Shilimkar @ 2011-09-27  5:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110926171452.GC11647@e102568-lin.cambridge.arm.com>

Lorenzo,

On Monday 26 September 2011 10:44 PM, Lorenzo Pieralisi wrote:
> On Mon, Sep 26, 2011 at 04:06:45PM +0100, Russell King - ARM Linux wrote:
>> On Mon, Sep 26, 2011 at 03:32:41PM +0100, Lorenzo Pieralisi wrote:
>>> Context is saved once for all at boot time, along with the L2 physical address
>>> and cache type.
>>
>> Why is this assembly code?  As Barry's previous patch shows, it's
>> entirely possible to save all the state required at init time from
>> the existing C code without needing any additional functions.
>>
> 
I too didn't like this approach.

The platforms which needs to take care of security needs to take
care of the L2 on their own and this code doesn't help them.

Other platform which wants to use the L2 code, looks like are
fine to use the C-version which saves registers in init and restores
it in resume path whenever needed. The patch which was posted
on that was good enough. Regarding turning OFF MMU is really
not necessary unless and until some hardware's are buggy. In that
case, instead of adding that un-necessary code in generic code,
it's better to patch only that buggy SOC code.

Regards
Santosh

^ permalink raw reply

* [PATCH 02/13] gpio/omap: Adapt GPIO driver to DT
From: Rajendra Nayak @ 2011-09-27  5:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1317055821-20652-3-git-send-email-b-cousson@ti.com>

On Monday 26 September 2011 10:20 PM, Benoit Cousson wrote:
> Adapt the GPIO driver to retrieve information from a DT file.
> Note that since the driver is currently under cleanup, some hacks
> will have to be removed after.
>
> Add documentation for GPIO properties specific to OMAP.
>
> Remove an un-needed whitespace.
>
> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
> Cc: Grant Likely<grant.likely@secretlab.ca>
> Cc: Charulatha V<charu@ti.com>
> Cc: Tarun Kanti DebBarma<tarun.kanti@ti.com>
> ---
>   .../devicetree/bindings/gpio/gpio-omap.txt         |   33 ++++++
>   drivers/gpio/gpio-omap.c                           |  108 ++++++++++++++++++--
>   2 files changed, 132 insertions(+), 9 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-omap.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-omap.txt b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
> new file mode 100644
> index 0000000..bdd63de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
> @@ -0,0 +1,33 @@
> +OMAP GPIO controller
> +
> +Required properties:
> +- compatible:
> +  - "ti,omap2-gpio" for OMAP2 and OMAP3 controllers

Would it be more readable to have
"ti,omap2-gpio" for OMAP2 controllers and
"ti,omap3-gpio" for OMAP3 controllers?

> +  - "ti,omap4-gpio" for OMAP4 controller
> +- #gpio-cells : Should be two.
> +  - first cell is the pin number
> +  - second cell is used to specify optional parameters (unused)
> +- gpio-controller : Marks the device node as a GPIO controller.
> +
> +OMAP specific properties:
> +- ti,hwmods: Name of the hwmod associated to the GPIO
> +- id: 32 bits to identify the id (1 based index)
> +- bank-width: number of pin supported by the controller (16 or 32)
> +- debounce: set if the controller support the debouce funtionnality
> +- bank-count: number of controller support by the SoC. This is a temporary
> +  hack until the bank_count is removed from the driver.

Is there a general rule to be followed as to when to use
"ti,<prop-name>" and when to use just "<prop-name>".
Since all these are OMAP specific properties, shouldn't all
of them be "ti,<prop-name>"?

> +
> +
> +Example:
> +
> +gpio4: gpio4 {
> +    compatible = "ti,omap4-gpio", "ti,omap-gpio";
> +    ti,hwmods = "gpio4";
> +    id =<4>;
> +    bank-width =<32>;
> +    debounce;
> +    no_idle_on_suspend;
> +    #gpio-cells =<2>;
> +    gpio-controller;
> +};
> +
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 0599854..f878fa4 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -21,6 +21,8 @@
>   #include<linux/io.h>
>   #include<linux/slab.h>
>   #include<linux/pm_runtime.h>
> +#include<linux/of.h>
> +#include<linux/of_device.h>
>
>   #include<mach/hardware.h>
>   #include<asm/irq.h>
> @@ -521,7 +523,7 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
>   	unsigned long flags;
>
>   	if (bank->non_wakeup_gpios&  gpio_bit) {
> -		dev_err(bank->dev,
> +		dev_err(bank->dev,

Stray change?

>   			"Unable to modify wakeup on non-wakeup GPIO%d\n", gpio);
>   		return -EINVAL;
>   	}
> @@ -1150,6 +1152,8 @@ static void __devinit omap_gpio_chip_init(struct gpio_bank *bank)
>   	irq_set_handler_data(bank->irq, bank);
>   }
>
> +static const struct of_device_id omap_gpio_match[];
> +
>   static int __devinit omap_gpio_probe(struct platform_device *pdev)
>   {
>   	static int gpio_init_done;
> @@ -1157,11 +1161,31 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
>   	struct resource *res;
>   	int id;
>   	struct gpio_bank *bank;
> +	struct device_node *node = pdev->dev.of_node;
> +	const struct of_device_id *match;
> +
> +	match = of_match_device(omap_gpio_match,&pdev->dev);
> +	if (match) {
> +		pdata = match->data;
> +		/* XXX: big hack until the bank_count is removed */
> +		of_property_read_u32(node, "bank-count",&gpio_bank_count);
> +		if (of_property_read_u32(node, "id",&id))

id should be u32.

> +			return -EINVAL;
> +		/*
> +		 * In an ideal world, the id should not be needed, but since
> +		 * the OMAP TRM consider the multiple GPIO controllers as
> +		 * multiple banks, the GPIO number is based on the whole set
> +		 * of banks. Hence the need to provide an id in order to
> +		 * respect the order and the correct GPIO number.
> +		 */
> +		id -= 1;
> +	} else {
> +		if (!pdev->dev.platform_data)
> +			return -EINVAL;
>
> -	if (!pdev->dev.platform_data)
> -		return -EINVAL;
> -
> -	pdata = pdev->dev.platform_data;
> +		pdata = pdev->dev.platform_data;
> +		id = pdev->id;
> +	}
>
>   	if (!gpio_init_done) {
>   		int ret;
> @@ -1171,7 +1195,6 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
>   			return ret;
>   	}
>
> -	id = pdev->id;
>   	bank =&gpio_bank[id];
>
>   	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> @@ -1181,12 +1204,19 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
>   	}
>
>   	bank->irq = res->start;
> -	bank->virtual_irq_start = pdata->virtual_irq_start;
>   	bank->method = pdata->bank_type;
>   	bank->dev =&pdev->dev;
> -	bank->dbck_flag = pdata->dbck_flag;
>   	bank->stride = pdata->bank_stride;
> -	bank->width = pdata->bank_width;
> +	if (match) {
> +		of_property_read_u32(node, "bank-width",&bank->width);

Bank width should be u32.

> +		if (of_get_property(node, "debounce", NULL))

of_find_property() should suffice.

regards,
Rajendra

> +			bank->dbck_flag = true;
> +		bank->virtual_irq_start = IH_GPIO_BASE + 32 * id;
> +	} else {
> +		bank->width = pdata->bank_width;
> +		bank->dbck_flag = pdata->dbck_flag;
> +		bank->virtual_irq_start = pdata->virtual_irq_start;
> +	}
>
>   	bank->regs = pdata->regs;
>
> @@ -1559,10 +1589,70 @@ void omap_gpio_restore_context(void)
>   }
>   #endif
>
> +#if defined(CONFIG_OF)
> +static struct omap_gpio_reg_offs omap2_gpio_regs = {
> +	.revision =		OMAP24XX_GPIO_REVISION,
> +	.direction =		OMAP24XX_GPIO_OE,
> +	.datain =		OMAP24XX_GPIO_DATAIN,
> +	.dataout =		OMAP24XX_GPIO_DATAOUT,
> +	.set_dataout =		OMAP24XX_GPIO_SETDATAOUT,
> +	.clr_dataout =		OMAP24XX_GPIO_CLEARDATAOUT,
> +	.irqstatus =		OMAP24XX_GPIO_IRQSTATUS1,
> +	.irqstatus2 =		OMAP24XX_GPIO_IRQSTATUS2,
> +	.irqenable =		OMAP24XX_GPIO_IRQENABLE1,
> +	.set_irqenable =	OMAP24XX_GPIO_SETIRQENABLE1,
> +	.clr_irqenable =	OMAP24XX_GPIO_CLEARIRQENABLE1,
> +	.debounce =		OMAP24XX_GPIO_DEBOUNCE_VAL,
> +	.debounce_en =		OMAP24XX_GPIO_DEBOUNCE_EN,
> +};
> +
> +static struct omap_gpio_platform_data omap2_pdata = {
> +	.bank_type = METHOD_GPIO_24XX,
> +	.regs =&omap2_gpio_regs,
> +};
> +
> +static struct omap_gpio_reg_offs omap4_gpio_regs = {
> +	.revision =		OMAP4_GPIO_REVISION,
> +	.direction =		OMAP4_GPIO_OE,
> +	.datain =		OMAP4_GPIO_DATAIN,
> +	.dataout =		OMAP4_GPIO_DATAOUT,
> +	.set_dataout =		OMAP4_GPIO_SETDATAOUT,
> +	.clr_dataout =		OMAP4_GPIO_CLEARDATAOUT,
> +	.irqstatus =		OMAP4_GPIO_IRQSTATUS1,
> +	.irqstatus2 =		OMAP4_GPIO_IRQSTATUS2,
> +	.irqenable =		OMAP4_GPIO_IRQENABLE1,
> +	.set_irqenable =	OMAP4_GPIO_SETIRQENABLE1,
> +	.clr_irqenable =	OMAP4_GPIO_CLEARIRQENABLE1,
> +	.debounce =		OMAP4_GPIO_DEBOUNCINGTIME,
> +	.debounce_en =		OMAP4_GPIO_DEBOUNCENABLE,
> +};
> +
> +static struct omap_gpio_platform_data omap4_pdata = {
> +	.bank_type = METHOD_GPIO_44XX,
> +	.regs =&omap4_gpio_regs,
> +};
> +
> +static const struct of_device_id omap_gpio_match[] = {
> +	{
> +		.compatible = "ti,omap4-gpio",
> +		.data =&omap4_pdata,
> +	},
> +	{
> +		.compatible = "ti,omap2-gpio",
> +		.data =&omap2_pdata,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, omap_gpio_match);
> +#else
> +#define omap_gpio_match NULL
> +#endif
> +
>   static struct platform_driver omap_gpio_driver = {
>   	.probe		= omap_gpio_probe,
>   	.driver		= {
>   		.name	= "omap_gpio",
> +		.of_match_table = omap_gpio_match,
>   	},
>   };
>

^ permalink raw reply

* [PATCH 06/13] mfd: twl-core: Add initial DT support for twl4030/twl6030
From: Rajendra Nayak @ 2011-09-27  5:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1317055821-20652-7-git-send-email-b-cousson@ti.com>

On Monday 26 September 2011 10:20 PM, Benoit Cousson wrote:
> Add initial device-tree support for twl familly chips.

s/familly/family

> The current version is missing the regulator entries due
> to the lack of DT regulator bindings for the moment.
> Only the simple sub-modules that do not depend on
> platform_data information can be initialized properly.
>
> Add documentation for the Texas Instruments TWL Integrated Chip.
>
> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
> Cc: Balaji T K<balajitk@ti.com>
> Cc: Graeme Gregory<gg@slimlogic.co.uk>
> Cc: Samuel Ortiz<sameo@linux.intel.com>
> ---
>   .../devicetree/bindings/mfd/twl-familly.txt        |   47 +++++++++++++++++
>   drivers/mfd/twl-core.c                             |   53 ++++++++++++++++++--
>   2 files changed, 96 insertions(+), 4 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/mfd/twl-familly.txt

s/familly.txt/family.txt

>
> diff --git a/Documentation/devicetree/bindings/mfd/twl-familly.txt b/Documentation/devicetree/bindings/mfd/twl-familly.txt
> new file mode 100644
> index 0000000..ff4cacd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/twl-familly.txt
> @@ -0,0 +1,47 @@
> +Texas Instruments TWL family
> +
> +The TWLs are Integrated Power Management Chips.
> +Some version might contain much more analog function like
> +USB transceiver or Audio amplifier.
> +These chips are connected to an i2c bus.
> +
> +
> +Required properties:
> +- compatible : Must be "ti,twl4030";
> +  For Integrated power-management/audio CODEC device used in OMAP3
> +  based boards
> +- compatible : Must be "ti,twl6030";
> +  For Integrated power-management used in OMAP4 based boards
> +- interrupts : This i2c device has an IRQ line connected to the main SoC
> +- interrupt-controller : Since the twl support several interrupts internally,
> +  it is considered as an interrupt controller cascaded to the SoC one.
> +- #interrupt-cells =<1>;
> +- interrupt-parent : The parent interrupt controller.
> +
> +Optional node:
> +- Child nodes contain in the twl. The twl family is made of severals variants
> +  that support a different number of features.
> +  The children nodes will thus depend of the capabilty of the variant.
> +
> +
> +Example:
> +/*
> + * Integrated Power Management Chip
> + * http://www.ti.com/lit/ds/symlink/twl6030.pdf
> + */
> +twl at 48 {
> +    compatible = "ti,twl6030";
> +    reg =<0x48>;

What does the 'reg' property signify here for twl?

> +    interrupts =<39>; /* IRQ_SYS_1N cascaded to gic */
> +    interrupt-controller;
> +    #interrupt-cells =<1>;
> +    interrupt-parent =<&gic>;
> +    #address-cells =<1>;
> +    #size-cells =<0>;
> +
> +    twl_rtc {
> +        compatible = "ti,twl_rtc";
> +        interrupts =<11>;
> +        reg =<0>;

Does the 'reg' property need to be faked for
every twl child node, even if it does not have
any?

> +    };
> +};
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 01ecfee..3ef0b43 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -33,6 +33,10 @@
>   #include<linux/platform_device.h>
>   #include<linux/clk.h>
>   #include<linux/err.h>
> +#include<linux/slab.h>
> +#include<linux/of_irq.h>
> +#include<linux/of_platform.h>
> +#include<linux/irqdomain.h>
>
>   #include<linux/regulator/machine.h>
>
> @@ -1182,22 +1186,53 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>   	int				status;
>   	unsigned			i;
>   	struct twl4030_platform_data	*pdata = client->dev.platform_data;
> +	struct device_node		*node = client->dev.of_node;
>   	u8 temp;
>   	int ret = 0;
>
> +	if (node&&  !pdata) {
> +		/*
> +		 * XXX: Temporary fake pdata until the information
> +		 * is correctly retrieved by every TWL modules from DT.
> +		 */
> +		pdata = kzalloc(sizeof(struct twl4030_platform_data),
> +				GFP_KERNEL);

devm_kzalloc instead?

> +		if (!pdata) {
> +			status = -ENOMEM;
> +			goto exit;
> +		}
> +
> +		/*
> +		 * XXX: For the moment the IRQs for TWL seems to be encoded in
> +		 * the global OMAP space. That should be cleaned to allow
> +		 * dynamically adding a new IRQ controller.
> +		 */
> +		if ((id->driver_data)&  TWL6030_CLASS) {
> +			pdata->irq_base = TWL6030_IRQ_BASE;
> +			pdata->irq_end = pdata->irq_base + TWL6030_BASE_NR_IRQS;
> +		} else {
> +			pdata->irq_base = TWL4030_IRQ_BASE;
> +			pdata->irq_end = pdata->irq_base + TWL4030_BASE_NR_IRQS;
> +		}
> +		irq_domain_add_simple(node, pdata->irq_base);
> +	}
> +
>   	if (!pdata) {
>   		dev_dbg(&client->dev, "no platform data?\n");
> -		return -EINVAL;
> +		status = -EINVAL;
> +		goto fail_free;
>   	}
>
>   	if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C) == 0) {
>   		dev_dbg(&client->dev, "can't talk I2C?\n");
> -		return -EIO;
> +		status = -EIO;
> +		goto fail_free;
>   	}
>
>   	if (inuse) {
>   		dev_dbg(&client->dev, "driver is already in use\n");
> -		return -EBUSY;
> +		status = -EBUSY;
> +		goto fail_free;
>   	}
>
>   	for (i = 0; i<  TWL_NUM_SLAVES; i++) {
> @@ -1269,10 +1304,20 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>   		twl_i2c_write_u8(TWL4030_MODULE_INTBR, temp, REG_GPPUPDCTR1);
>   	}
>
> -	status = add_children(pdata, id->driver_data);
> +#ifdef CONFIG_OF_DEVICE

is the #ifdef really needed?

> +	if (node)
> +		status = of_platform_populate(node, NULL, NULL,&client->dev);
> +	else
> +#endif
> +		status = add_children(pdata, id->driver_data);
> +
>   fail:
>   	if (status<  0)
>   		twl_remove(client);
> +fail_free:
> +	if (node)
> +		kfree(pdata);
> +exit:
>   	return status;
>   }
>

^ permalink raw reply

* [PATCH 08/13] arm/dts: OMAP4: Add i2c controller nodes
From: Rajendra Nayak @ 2011-09-27  5:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1317055821-20652-9-git-send-email-b-cousson@ti.com>

On Monday 26 September 2011 10:20 PM, Benoit Cousson wrote:
> Add i2c controllers nodes into the main ocp bus.
>
> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
> Cc: G, Manjunath Kondaiah<manjugk@ti.com>
> ---
>   arch/arm/boot/dts/omap4.dtsi |   32 ++++++++++++++++++++++++++++++++
>   1 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index bb19bca..447f482 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -21,6 +21,10 @@
>   	interrupt-parent =<&gic>;
>
>   	aliases {
> +		i2c1 =&i2c1;
> +		i2c2 =&i2c2;
> +		i2c3 =&i2c3;
> +		i2c4 =&i2c4;
>   	};
>
>   	cpus {
> @@ -173,5 +177,33 @@
>   			#gpio-cells =<2>;
>   			gpio-controller;
>   		};
> +
> +		i2c1: i2c at 1 {
> +			compatible = "ti,omap3-i2c";

This is a little confusing as omap4.dtsi has
compatible "ti,omap3-i2c" nodes in it.

> +			#address-cells =<1>;
> +			#size-cells =<0>;
> +			ti,hwmods = "i2c1";
> +		};
> +
> +		i2c2: i2c at 2 {
> +			compatible = "ti,omap3-i2c";
> +			#address-cells =<1>;
> +			#size-cells =<0>;
> +			ti,hwmods = "i2c2";
> +		};
> +
> +		i2c3: i2c at 3 {
> +			compatible = "ti,omap3-i2c";
> +			#address-cells =<1>;
> +			#size-cells =<0>;
> +			ti,hwmods = "i2c3";
> +		};
> +
> +		i2c4: i2c at 4 {
> +			compatible = "ti,omap3-i2c";
> +			#address-cells =<1>;
> +			#size-cells =<0>;
> +			ti,hwmods = "i2c4";
> +		};
>   	};
>   };

^ permalink raw reply

* Pull request: removal of most instances of mach/memory.h
From: Russell King - ARM Linux @ 2011-09-27  7:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1109262047290.2718@xanadu.home>

On Mon, Sep 26, 2011 at 08:52:48PM -0400, Nicolas Pitre wrote:
> On Mon, 26 Sep 2011, Nicolas Pitre wrote:
> 
> > On Tue, 27 Sep 2011, Russell King - ARM Linux wrote:
> > 
> > > A little more discipline and communication (such as asking me to drop
> > > the duplicate commit _first_) would be nice.
> > 
> > Certainly.  Improving communication both ways might help.
> 
> One way this could be improved would be for your patch system to send 
> notification emails with the submitter address explicit in the To: 
> field.  Right now it contains linux-arm-patches at lists.arm.linux.org.uk 
> only, therefore those emails don't get sorted with the best priority 
> they could get.

Stop splitting hairs.

The fact of the matter is that you should have said in the final pull
request that you'd also sent one of the patches to the patch system
_whether_ _or_ _not_ it was applied.  Whether it was applied or not
is completely irrelevant - because either way it requires some action
(either to remove it from my git tree if applied _OR_ discard it from
the patch system if not.)

The fact you didn't mention it in the final pull request at all is the
problem.

^ permalink raw reply

* [PATCH 1/2] drivers: create a pin control subsystem v7
From: Linus Walleij @ 2011-09-27  7:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF1738180167@HQMAIL01.nvidia.com>

On Wed, Sep 21, 2011 at 9:45 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Linus Walleij wrote at Wednesday, September 21, 2011 3:17 AM:
>>
>> Great, I'm hunting your Acked-by/Reviewed-by ...
>
> Very close; I was just holding off until we resolved the discussion on
> hog'd non-system mapping table entries, and I figured I'd wait until v8
> which presumably would delete the pinmux_config function from the header?

Yep, I will submit that soon. After this I plan to request its inclusion
into linux-next.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH 0/4] mmc: mmci: Improvements and bugfixes for SDIO
From: Ulf Hansson @ 2011-09-27  7:45 UTC (permalink / raw)
  To: linux-arm-kernel

This patchserie fixes a couple of SDIO related issues for the mmci host
driver. The intention is to keep each patch small and to have one patch
for one problem. It might makes sence to squash some of the patches, but
I leave this for discussion.

Stefan Nilsson XK (3):
  mmc: mmci: Bugfix in pio read for small packets
  mmc: mmci: Support non-power-of-two block sizes for ux500v2 variant
  mmc: mmci: Fix incorrect handling of HW flow control for SDIO

Ulf Hansson (1):
  mmc: mmci: Prepare for SDIO before setting up DMA job

 drivers/mmc/host/mmci.c |   78 ++++++++++++++++++++++++++++++++---------------
 1 files changed, 53 insertions(+), 25 deletions(-)

-- 
1.7.5.4

^ permalink raw reply

* [PATCH 1/4] mmc: mmci: Bugfix in pio read for small packets
From: Ulf Hansson @ 2011-09-27  7:46 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stefan Nilsson XK <stefan.xk.nilsson@stericsson.com>

Corrects a bug in pio read when reading packets < 4 bytes. These
small packets are only relevant for SDIO transfers.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Stefan Nilsson XK <stefan.xk.nilsson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 40e4c05..8831b2f 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -776,7 +776,24 @@ static int mmci_pio_read(struct mmci_host *host, char *buffer, unsigned int rema
 		if (count <= 0)
 			break;
 
-		readsl(base + MMCIFIFO, ptr, count >> 2);
+		/*
+		 * SDIO especially may want to receive something that is
+		 * not divisible by 4 (as opposed to card sectors
+		 * etc). Therefore make sure we always read the last bytes
+		 * out of the FIFO.
+		 */
+		switch (count) {
+		case 1:
+		case 3:
+			readsb(base + MMCIFIFO, ptr, count);
+			break;
+		case 2:
+			readsw(base + MMCIFIFO, ptr, 1);
+			break;
+		default:
+			readsl(base + MMCIFIFO, ptr, count >> 2);
+			break;
+		}
 
 		ptr += count;
 		remain -= count;
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 2/4] mmc: mmci: Support non-power-of-two block sizes for ux500v2 variant
From: Ulf Hansson @ 2011-09-27  7:46 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stefan Nilsson XK <stefan.xk.nilsson@stericsson.com>

For the ux500v2 variant of the PL18x block, non power of two block
sizes are supported. This will make it possible to decrease data
overhead for SDIO transfers.

Signed-off-by: Stefan Nilsson XK <stefan.xk.nilsson@stericsson.com>
Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/mmc/host/mmci.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 8831b2f..f1d27d9 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -52,6 +52,7 @@ static unsigned int fmax = 515633;
  * @sdio: variant supports SDIO
  * @st_clkdiv: true if using a ST-specific clock divider algorithm
  * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
+ * @non_power_of_2_blksize: true if block sizes can be other than power of two
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -62,6 +63,7 @@ struct variant_data {
 	bool			sdio;
 	bool			st_clkdiv;
 	bool			blksz_datactrl16;
+	bool			non_power_of_2_blksize;
 };
 
 static struct variant_data variant_arm = {
@@ -103,6 +105,7 @@ static struct variant_data variant_ux500v2 = {
 	.sdio			= true,
 	.st_clkdiv		= true,
 	.blksz_datactrl16	= true,
+	.non_power_of_2_blksize	= true,
 };
 
 /*
@@ -592,7 +595,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	writel(host->size, base + MMCIDATALENGTH);
 
 	blksz_bits = ffs(data->blksz) - 1;
-	BUG_ON(1 << blksz_bits != data->blksz);
 
 	if (variant->blksz_datactrl16)
 		datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
@@ -989,11 +991,14 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 static void mmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct mmci_host *host = mmc_priv(mmc);
+	struct variant_data *variant = host->variant;
 	unsigned long flags;
 
 	WARN_ON(host->mrq != NULL);
 
-	if (mrq->data && !is_power_of_2(mrq->data->blksz)) {
+	if (mrq->data &&
+	    !variant->non_power_of_2_blksize &&
+	    !is_power_of_2(mrq->data->blksz)) {
 		dev_err(mmc_dev(mmc), "unsupported block size (%d bytes)\n",
 			mrq->data->blksz);
 		mrq->cmd->error = -EINVAL;
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 3/4] mmc: mmci: Prepare for SDIO before setting up DMA job
From: Ulf Hansson @ 2011-09-27  7:46 UTC (permalink / raw)
  To: linux-arm-kernel

Move the SDIO preparation to be done before the DMA job is setup.
This makes it possible to do DMA for SDIO transfers as well as the
earlier supported pio mode.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Stefan Nilsson XK <stefan.xk.nilsson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index f1d27d9..cd3bc25 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -604,6 +604,11 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	if (data->flags & MMC_DATA_READ)
 		datactrl |= MCI_DPSM_DIRECTION;
 
+	/* The ST Micro variants has a special bit to enable SDIO */
+	if (variant->sdio && host->mmc->card)
+		if (mmc_card_sdio(host->mmc->card))
+			datactrl |= MCI_ST_DPSM_SDIOEN;
+
 	/*
 	 * Attempt to use DMA operation mode, if this
 	 * should fail, fall back to PIO mode
@@ -632,11 +637,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 		irqmask = MCI_TXFIFOHALFEMPTYMASK;
 	}
 
-	/* The ST Micro variants has a special bit to enable SDIO */
-	if (variant->sdio && host->mmc->card)
-		if (mmc_card_sdio(host->mmc->card))
-			datactrl |= MCI_ST_DPSM_SDIOEN;
-
 	writel(datactrl, base + MMCIDATACTRL);
 	writel(readl(base + MMCIMASK0) & ~MCI_DATAENDMASK, base + MMCIMASK0);
 	mmci_set_mask1(host, irqmask);
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 4/4] mmc: mmci: Fix incorrect handling of HW flow control for SDIO
From: Ulf Hansson @ 2011-09-27  7:46 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stefan Nilsson XK <stefan.xk.nilsson@stericsson.com>

For data writes smaller <= 8 bytes (only SDIO case), HW flow control was
disabled but never re-enabled again. This meant that a following large read
request would randomly give buffer overrun errors.

Moreover HW flow control is not needed for transfers that fits in the
FIFO of PL18x. Thus it is disabled for write operations <= the FIFO size.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Stefan Nilsson XK <stefan.xk.nilsson@stericsson.com>
---
 drivers/mmc/host/mmci.c |   42 ++++++++++++++++++++++++------------------
 1 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index cd3bc25..4da20ec 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -606,9 +606,32 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 
 	/* The ST Micro variants has a special bit to enable SDIO */
 	if (variant->sdio && host->mmc->card)
-		if (mmc_card_sdio(host->mmc->card))
+		if (mmc_card_sdio(host->mmc->card)) {
+			/*
+			 * The ST Micro variants has a special bit
+			 * to enable SDIO.
+			 */
 			datactrl |= MCI_ST_DPSM_SDIOEN;
 
+			/*
+			 * The ST Micro variant for SDIO transfer sizes
+			 * less then or equal to 8 bytes needs to have clock
+			 * H/W flow control disabled. Since flow control is
+			 * not really needed for anything that fits in the
+			 * FIFO, we can disable it for any write smaller
+			 * than the FIFO size.
+			 */
+			if ((host->size <= variant->fifosize) &&
+			    (data->flags & MMC_DATA_WRITE))
+				writel(readl(host->base + MMCICLOCK) &
+				       ~variant->clkreg_enable,
+				       host->base + MMCICLOCK);
+			else
+				writel(readl(host->base + MMCICLOCK) |
+				       variant->clkreg_enable,
+				       host->base + MMCICLOCK);
+		}
+
 	/*
 	 * Attempt to use DMA operation mode, if this
 	 * should fail, fall back to PIO mode
@@ -824,23 +847,6 @@ static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int rem
 		count = min(remain, maxcnt);
 
 		/*
-		 * The ST Micro variant for SDIO transfer sizes
-		 * less then 8 bytes should have clock H/W flow
-		 * control disabled.
-		 */
-		if (variant->sdio &&
-		    mmc_card_sdio(host->mmc->card)) {
-			if (count < 8)
-				writel(readl(host->base + MMCICLOCK) &
-					~variant->clkreg_enable,
-					host->base + MMCICLOCK);
-			else
-				writel(readl(host->base + MMCICLOCK) |
-					variant->clkreg_enable,
-					host->base + MMCICLOCK);
-		}
-
-		/*
 		 * SDIO especially may want to send something that is
 		 * not divisible by 4 (as opposed to card sectors
 		 * etc), and the FIFO only accept full 32-bit writes.
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 1/2] drivers: create a pin control subsystem v7
From: Linus Walleij @ 2011-09-27  7:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF1738180167@HQMAIL01.nvidia.com>

On Wed, Sep 21, 2011 at 9:45 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Linus Walleij wrote at Wednesday, September 21, 2011 3:17 AM:
>> To abstract things the stuff we can do with the group should be
>> something enumerated too. So:
>>
>> pinctrl_config_group(const char *pinctrl_device, const char *group,
>> const char *mode);
>> pinctrl_config_pin(const char *pinctrl_device, int pin, const char *mode);
>>
>> So the driver need an API to enumerate pin and group modes.
>>
>> I might want to save this thing for post-merge of the basic API and
>> pinmux stuff though so we don't try to push too much upfront
>> design at once.
>
> Yes, adding in a replacement _config API can certainly be a later patch.

If I'm efficient enough we might get that add-on before the v3.2
merge window, I'll iterate that patch separately though.

> One comment on the API above: I think you want "mode" (or "setting"?)
> /and/ a value parameter; Tegra's pull strength definition for example
> is:
>
> enum tegra_pull_strength {
> ? ? ? ?TEGRA_PULL_0 = 0,
> ? ? ? ?TEGRA_PULL_1,
> // ... (every value in between)
> ? ? ? ?TEGRA_PULL_30,
> ? ? ? ?TEGRA_PULL_31,
> ? ? ? ?TEGRA_MAX_PULL,
> };
>
> And it seems better to represent that as ("pull", 0) ... ("pull", 31) than
> "pull0" .. "pull31" such that the value needs to be parsed out of the "mode"
> string somehow by the driver.

OK so we might want some public defintion of "things you can do"
with pins, then an opaque parameter.

like:

#define PINCTRL_PULL "pull"
#define PINCTRL_BIAS "bias"
#define PINCTRL_LOADCAP "load-capacitance"
#define PINCTRL_DRIVE "drive"

...but then it's just simple enumerators, and we might be better off
with a simple enum after all:

enum pinctrl_pin_ops {
    PINCTRL_PULL,
    PINCTRL_BIAS,
    PINCTRL_LOADCAP,
    PINCTRL_DRIVE,
}

Surely the device tree will have to translate that enum into strings
(I guess? Sorry for my low DT competence) but that is more of a
DT pecularity and can be kept in the DT pin control parser?

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH 2/2 v7] pinmux: add a driver for the U300 pinmux
From: Linus Walleij @ 2011-09-27  8:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF1738180164@HQMAIL01.nvidia.com>

On Wed, Sep 21, 2011 at 9:39 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Linus Walleij wrote at Wednesday, September 21, 2011 2:25 AM:
>> On Wed, Sep 21, 2011 at 12:15 AM, Stephen Warren <swarren@nvidia.com> wrote:
>> > Linus Walleij wrote at Friday, September 16, 2011 6:14 AM:
>> >> + ? ? for (i = 0; i < ARRAY_SIZE(u300_mux_hogs); i++) {
>> >> + ? ? ? ? ? ? struct pinmux *pmx;
>> >> + ? ? ? ? ? ? int ret;
>> >> +
>> >> + ? ? ? ? ? ? pmx = pinmux_get(u300_mux_hogs[i].dev, NULL);
>> >> + ? ? ? ? ? ? if (IS_ERR(pmx)) {
>> >> + ? ? ? ? ? ? ? ? ? ? pr_err("u300: could not get pinmux hog %s\n",
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?u300_mux_hogs[i].name);
>> >> + ? ? ? ? ? ? ? ? ? ? continue;
>> >> + ? ? ? ? ? ? }
>> >> + ? ? ? ? ? ? ret = pinmux_enable(pmx);
>> >> + ? ? ? ? ? ? if (ret) {
>> >> + ? ? ? ? ? ? ? ? ? ? pr_err("u300: could enable pinmux hog %s\n",
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?u300_mux_hogs[i].name);
>> >> + ? ? ? ? ? ? ? ? ? ? continue;
>> >> + ? ? ? ? ? ? }
>> >> + ? ? ? ? ? ? u300_mux_hogs[i].pmx = pmx;
>> >> + ? ? }
>> >> + ? ? return 0;
>> >> +}
>> >> +subsys_initcall(u300_pinmux_fetch);
>> >
>> > Why not just have the pinmux core support hogging on non-"system" mapping
>> > entries; then everything I quoted above except u300_pinmux_map[] could
>> > be deleted, and the "hog" flag set on the last 3 u300_pinmux_map[] entries.
>>
>> Very good question, luckily I have a good answer.
>>
>> There is no way for the pinmux core to traverse the system and look
>> up the apropriate struct device * pointers.
>
> It's unclear to me why that would be needed.
>
> For non-system mappings, either the device-driver in question will be
> get'ing and enabling them (this case isn't relevant to this discussion),
> or they'll be hogged. In the hog case, I doubt anything would ever want
> to disable them and switch the device to a different mapping entry; if
> that were the case, the entries shouldn't be hogged, but get/enabled by
> the device anyway. Hence, I don't see the need for an activation of a
> hog'd non-system mapping entry to care about the device fields that are
> in the mapping table at all; they could just be ignored couldn't they?

I don't think so, atleast not in this case. I think it is helpful for the system
to know which device is related to the specific hog, so it turns up in
debugfs etc "aha, it is hogged for that device" etc. Not doing that
makes the subsystem loose relevant information.

So for that reason I prefer to pass in the relevant struct device *
pointers, atleast when they're readily available as in this boardfile.

If they are not available, or hard to get at, we could just redefine
them as system mappings and remove the device fields, but
in this case I'd say let's keep it around.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v3 2/2] input: samsung-keypad: Add device tree support
From: Thomas Abraham @ 2011-09-27  8:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110922174054.GO17168@ponder.secretlab.ca>

Hi Grant,

On 22 September 2011 23:10, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Mon, Sep 19, 2011 at 03:49:13PM +0530, Thomas Abraham wrote:
>> Add device tree based discovery support for Samsung's keypad controller.
>>
>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>> Cc: Donghwa Lee <dh09.lee@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>
> A few things to fix below, but you can add my acked-by when you've
> addressed them.

Ok. Thanks for your review. I will fix as per your comments.

[...]

>
>> diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
>> index f689f49..2a477bc 100644
>> --- a/drivers/input/keyboard/samsung-keypad.c
>> +++ b/drivers/input/keyboard/samsung-keypad.c

[...]

>> +
>> + ? ? of_property_read_u32(np, "samsung,keypad-num-rows", &num_rows);
>> + ? ? of_property_read_u32(np, "samsung,keypad-num-columns", &num_cols);
>
> property_read doesn't touch the values of num_rows or num_cols on
> failure. ?The values need to be initialized if you're going to test
> the result value.

Ok. num_rows and num_cols variables will be set to zero before the
property read call.

>
>> + ? ? if (!num_rows || !num_cols) {
>> + ? ? ? ? ? ? dev_err(dev, "number of keypad rows/columns not specified\n");
>> + ? ? ? ? ? ? return NULL;
>> + ? ? }

[...]

>>
>> + ? ? if (pdev->dev.of_node) {
>> + ? ? ? ? ? ? samsung_keypad_parse_dt_gpio(&pdev->dev, keypad);
>> +#ifdef CONFIG_OF
>> + ? ? ? ? ? ? keypad->type = of_device_is_compatible(pdev->dev.of_node,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "samsung,s5pv210-keypad");
>> +#endif
>
> This still looks odd. ?If you move the #ifdef up one line, then you
> can remove the empty version of samsung_keypad_parse_dt_gpio(), and
> this block won't look so weird.
>

That would be a better approach. Thanks for the suggestion.

Regards,
Thomas.

[...]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox