SUPERH platform development
 help / color / mirror / Atom feed
* [PATCH 1/2] ARM: shmobile: sh73a0: Add CPU sleep suspend
From: Bastian Hecht @ 2012-12-05 12:13 UTC (permalink / raw)
  To: linux-arm-kernel

From: Bastian Hecht <hechtb@gmail.com>

Add the lighest possible sleep mode on Cortex-A9 cores: CPU sleep. It is
entered by a simple dsb and wfi instruction via cpu_do_idle(). As just
clocks are stopped there is no need to save or restore any state of the
system.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
---
 arch/arm/mach-shmobile/Makefile              |    1 +
 arch/arm/mach-shmobile/board-kzm9g.c         |    2 ++
 arch/arm/mach-shmobile/include/mach/common.h |    1 +
 arch/arm/mach-shmobile/pm-sh73a0.c           |   32 ++++++++++++++++++++++++++
 4 files changed, 36 insertions(+)
 create mode 100644 arch/arm/mach-shmobile/pm-sh73a0.c

diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index fe2c97c..d0d06f2 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_ARCH_SHMOBILE)	+= pm-rmobile.o
 obj-$(CONFIG_ARCH_SH7372)	+= pm-sh7372.o sleep-sh7372.o
 obj-$(CONFIG_ARCH_R8A7740)	+= pm-r8a7740.o
 obj-$(CONFIG_ARCH_R8A7779)	+= pm-r8a7779.o
+obj-$(CONFIG_ARCH_SH73A0)	+= pm-sh73a0.o
 
 # Board objects
 obj-$(CONFIG_MACH_G3EVM)	+= board-g3evm.o
diff --git a/arch/arm/mach-shmobile/board-kzm9g.c b/arch/arm/mach-shmobile/board-kzm9g.c
index 0a43f31..dc8c0f8 100644
--- a/arch/arm/mach-shmobile/board-kzm9g.c
+++ b/arch/arm/mach-shmobile/board-kzm9g.c
@@ -760,6 +760,8 @@ static void __init kzm_init(void)
 
 	sh73a0_add_standard_devices();
 	platform_add_devices(kzm_devices, ARRAY_SIZE(kzm_devices));
+
+	sh73a0_pm_init();
 }
 
 static void kzm9g_restart(char mode, const char *cmd)
diff --git a/arch/arm/mach-shmobile/include/mach/common.h b/arch/arm/mach-shmobile/include/mach/common.h
index d47e215..318551b3 100644
--- a/arch/arm/mach-shmobile/include/mach/common.h
+++ b/arch/arm/mach-shmobile/include/mach/common.h
@@ -54,6 +54,7 @@ extern void sh73a0_add_early_devices(void);
 extern void sh73a0_add_standard_devices(void);
 extern void sh73a0_clock_init(void);
 extern void sh73a0_pinmux_init(void);
+extern void sh73a0_pm_init(void);
 extern struct clk sh73a0_extal1_clk;
 extern struct clk sh73a0_extal2_clk;
 extern struct clk sh73a0_extcki_clk;
diff --git a/arch/arm/mach-shmobile/pm-sh73a0.c b/arch/arm/mach-shmobile/pm-sh73a0.c
new file mode 100644
index 0000000..9cf67bb
--- /dev/null
+++ b/arch/arm/mach-shmobile/pm-sh73a0.c
@@ -0,0 +1,32 @@
+/*
+ * sh73a0 Power management support
+ *
+ *  Copyright (C) 2012 Bastian Hecht <hechtb+renesas@gmail.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/suspend.h>
+#include <mach/common.h>
+
+#ifdef CONFIG_SUSPEND
+static int sh73a0_enter_suspend(suspend_state_t suspend_state)
+{
+	cpu_do_idle();
+	return 0;
+}
+
+static void sh73a0_suspend_init(void)
+{
+	shmobile_suspend_ops.enter = sh73a0_enter_suspend;
+}
+#else
+static void sh73a0_suspend_init(void) {}
+#endif
+
+void __init sh73a0_pm_init(void)
+{
+	sh73a0_suspend_init();
+}
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/2] ARM: shmobile: r8a7740: Add CPU sleep suspend
From: Bastian Hecht @ 2012-12-05 12:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1354709587-25288-1-git-send-email-hechtb+renesas@gmail.com>

From: Bastian Hecht <hechtb@gmail.com>

Add the lighest possible sleep mode on Cortex-A9 cores: CPU sleep.
It is entered by a simple dsb and wfi instruction via cpu_do_idle(). As
just clocks are stopped there is no need to save or restore any state of
the system.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
---
 arch/arm/mach-shmobile/board-armadillo800eva.c |    2 ++
 arch/arm/mach-shmobile/include/mach/common.h   |    1 +
 arch/arm/mach-shmobile/pm-r8a7740.c            |   20 ++++++++++++++++++++
 3 files changed, 23 insertions(+)

diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 3cc8b1c..48847f6 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -1213,6 +1213,8 @@ static void __init eva_init(void)
 	rmobile_add_device_to_domain("A4LC", &hdmi_lcdc_device);
 	if (usb)
 		rmobile_add_device_to_domain("A3SP", usb);
+
+	r8a7740_pm_init();
 }
 
 static void __init eva_earlytimer_init(void)
diff --git a/arch/arm/mach-shmobile/include/mach/common.h b/arch/arm/mach-shmobile/include/mach/common.h
index 318551b3..ebcd28d 100644
--- a/arch/arm/mach-shmobile/include/mach/common.h
+++ b/arch/arm/mach-shmobile/include/mach/common.h
@@ -66,6 +66,7 @@ extern void r8a7740_add_early_devices(void);
 extern void r8a7740_add_standard_devices(void);
 extern void r8a7740_clock_init(u8 md_ck);
 extern void r8a7740_pinmux_init(void);
+extern void r8a7740_pm_init(void);
 
 extern void r8a7779_init_irq(void);
 extern void r8a7779_map_io(void);
diff --git a/arch/arm/mach-shmobile/pm-r8a7740.c b/arch/arm/mach-shmobile/pm-r8a7740.c
index 21e5316d..8cfb5bc 100644
--- a/arch/arm/mach-shmobile/pm-r8a7740.c
+++ b/arch/arm/mach-shmobile/pm-r8a7740.c
@@ -58,3 +58,23 @@ void __init r8a7740_init_pm_domains(void)
 }
 
 #endif /* CONFIG_PM */
+
+#ifdef CONFIG_SUSPEND
+static int r8a7740_enter_suspend(suspend_state_t suspend_state)
+{
+	cpu_do_idle();
+	return 0;
+}
+
+static void r8a7740_suspend_init(void)
+{
+	shmobile_suspend_ops.enter = r8a7740_enter_suspend;
+}
+#else
+static void r8a7740_suspend_init(void) {}
+#endif
+
+void __init r8a7740_pm_init(void)
+{
+	r8a7740_suspend_init();
+}
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH v3] media: V4L2: add temporary clock helpers
From: Laurent Pinchart @ 2012-12-06  1:24 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux Media Mailing List, Sakari Ailus, Sylwester Nawrocki,
	Hans Verkuil, Sylwester Nawrocki, Magnus Damm, linux-sh
In-Reply-To: <Pine.LNX.4.64.1212041136250.26918@axis700.grange>

Hi Guennadi,

Thanks for the patch.

On Tuesday 04 December 2012 11:42:15 Guennadi Liakhovetski wrote:
> Typical video devices like camera sensors require an external clock source.
> Many such devices cannot even access their hardware registers without a
> running clock. These clock sources should be controlled by their consumers.
> This should be performed, using the generic clock framework. Unfortunately
> so far only very few systems have been ported to that framework. This patch
> adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
> Platforms, adopting the clock API, should switch to using it. Eventually
> this temporary API should be removed.

As discussed on Jabber, I think we should make the clock helpers use the 
common clock framework when available, to avoid pushing support for the two 
APIs to all sensor drivers. Do you plan to include that in v4 ? :-)

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH v2 00/77] SH pin control and GPIO rework with OF support
From: Laurent Pinchart @ 2012-12-06  1:34 UTC (permalink / raw)
  To: linux-sh
In-Reply-To: <1353974596-30033-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

Hi Linus,

Thank you for the review.

On Saturday 01 December 2012 23:55:35 Linus Walleij wrote:
> On Tue, Nov 27, 2012 at 1:01 AM, Laurent Pinchart wrote:
> > Here's the second version of the SH pin control and GPIO rework patches.
> > I've added OF support for PFC instantiation and GPIO mappings that was
> > missing from v1. PINCTRL bindings are still missing and will come soon.
> 
> So I've tried the only way I could to review this by cloning your tree
> and actually inspecting the end result ... overall it's looking very good!
> 
> Here are assorted comments:
> 
> - Some use of __devicexit/__devinit, this will be deleted in the v3.8
>   merge window so just remove this everywhere.

Will fix.

> - You're using the method to add ranges from the pinctrl side of
>   things. This is basically deprecated with the changes to gpiolib
>   I make in this merge window. If you study the way I changed
>   the pinctrl-u300.c and pinctrl-coh901.c to switch the registration
>   from being done in the pin controller to being done in the
>   gpiolib part, you will get the picture. The big upside is that
>   (A) makes the pin and GPIO references to the local GPIO
>   chip and pin controller and (B) that this supports adding ranges
>   from the device tree, which is probably what you want in the
>   end...

OK, I will have a look at the code.

> - The above probably means you can get rid of
>   sh_pfc_register_gpiochip() and decouple the pinctrl
>   and gpiolib code like I did in my patch set
>   (commit 8604ac34e in the pinctrl tree) and just
>   initialize the GPIO with some module_init().
>   But I might be wrong!

GPIO and pinmuxing are controlled by the same hardware block, with shared 
registers (there's one register by pin that controls the direction, pull-
ups/pull-downs and function selection). That's why I've implemented them in a 
single driver.

> - sh_pfc_register_pinctrl() in pinctrl.c is using kzalloc/kfree,
>   but since it has a struct sh_pfc, which contains a
>   struct device *, I suspect you can use devm_kzalloc()
>   and cut the kfree():s. This probably applies in more
>   places in the driver.

Agreed, I'll fix that.

> - core.c contains pfc_ioremap() and pfc_iounmap()
>   which actually seem to exist much due to the fact
>   that devm_kzalloc() and devm_ioremap_nocache() did not
>   exist at the time. By using devm_* helpers and
>   maybe also inlining the code I think it'll look way
>   smoother.

Will fix as well.

> - sh_pfc.h contains this:
>   typedef unsigned short pinmux_enum_t;
>   typedef unsigned short pinmux_flag_t;
>   But custom typedefs in the kernel is discouraged unless
>   there is a good reason for. What about you search/replace
>   this thing with u16 everywhere. I really like u16...
>   I can see macros building up some gigantic bit pattern in
>   these but in the end it's still just 16 unsigned bits.
>   The name "pinmux_enum_t" is very dangerously
>   close to the builting "enum" so it scares me a bit
>   for that reason too.

It's totally scary, and I want it to go away. It's old code that I still need 
to fix, you can expect a v3 with more than 77 patches :-)

> - sf_pfc.h contains a number of structs which I just
>   have no chance of understanding unless they are
>   supplied with something real nice like kerneldoc,
>   struct pinmux_gpio, pinmux_cfg_reg, pinmux_data_reg,
>   pinmux_irq, pinmux_range and sh_pfc_soc_info all
>   need some doc I think, I just don't understand these
>   structs.
>
> - Same goes for the helper macros.

Those structures and macros come from the existing driver (drivers/sh/pfc). I 
want to replace them with a cleaner pinctrl-aware implementation, that's still 
work in progress that should be in v3.

> - In core.h document struct pfc_window, i.e. what are
>   these windows? What do we mean by a window?
>   How many of them exist in a certain configuration
>   typically? etc, stuff you want to know when reading
>   the code.

It's basically an ioremapped region. The driver gets register physical 
addresses in the SoC info structures, and walks through the ioremapped regions 
when it needs to access the registers to find the ioremapped address.

> - struct sh_pfc is however quite self-explanatory.
> 
> - The pfc-* drivers include things like:
>   #include <cpu/sh7785.h>
>   #include <mach/r8a7740.h>
>   #include <mach/irqs.h>
>   #include <mach/r8a7779.h>
>   #include <mach/sh73a0.h>
>   #include <mach/irqs.h>
>   Why?

Because the code isn't new but comes from existing mach-level code.

>   Especially <mach/irqs.h> would be nice to
>   get rid of, as it sort of defeats the idea of passing
>   IRQs as resources or allocating them dynamically.
>   In some cases it seems these includes are just
>   surplus, so please look over this...

I'll have a look at it (maybe for v4 though ;-))

> - This stuff in setup_data_regs():
>   rp->reg_shadow = gpio_read_raw_reg(drp->mapped_reg,
>                                                     drp->reg_width);
>   You know, I think shadow registers is just another name
>   for regmap-mmio. Please consult
>   drivers/base/regmap/regmap-mmio.c and tell me if I'm
>   wrong. It's not like I'm going to require you to convert this
>   to regmap from day 1 if this is legacy stuff but it's probably
>   the same thing.

I'll have a look at it.

> - In core.c, gpio_read_raw_reg() and gpio_write_raw_reg()
>   are looking like marshalling functions to me. This is also
>   what regmap is about. Marshalling and caching/shadow.
>   If you keep the functions as they are, atleast rename them
>   to sh_pfc_* because the gpio_* namespace is for gpiolib.
>   (There are a few other functions that need to be prefixed
>   in that file by the way.)

OK.

> - While keeping Magnus' and Paul's names as authors in the
>   files it would be proper if you atleast add your own name since
>   you've probably written most of the code in these files now.
>   Also for MODULE_AUTHOR().

OK :-)

> Let's have this patchset finished for v3.9 say? :-)

I'll try to.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH v3] media: V4L2: add temporary clock helpers
From: Guennadi Liakhovetski @ 2012-12-06  7:41 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Linux Media Mailing List, Sakari Ailus, Sylwester Nawrocki,
	Hans Verkuil, Sylwester Nawrocki, Magnus Damm, linux-sh
In-Reply-To: <1885008.m7crYYR2Uy@avalon>

Hi Laurent

On Thu, 6 Dec 2012, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> Thanks for the patch.
> 
> On Tuesday 04 December 2012 11:42:15 Guennadi Liakhovetski wrote:
> > Typical video devices like camera sensors require an external clock source.
> > Many such devices cannot even access their hardware registers without a
> > running clock. These clock sources should be controlled by their consumers.
> > This should be performed, using the generic clock framework. Unfortunately
> > so far only very few systems have been ported to that framework. This patch
> > adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
> > Platforms, adopting the clock API, should switch to using it. Eventually
> > this temporary API should be removed.
> 
> As discussed on Jabber, I think we should make the clock helpers use the 
> common clock framework when available, to avoid pushing support for the two 
> APIs to all sensor drivers. Do you plan to include that in v4 ? :-)

AAMOF, no, I don't. Originally I planned to add this only when the first 
user appears. We can also add it earlier - a test case could be hacked up 
pretty quickly. But in either case I'd prefer to have it as a separate 
patch.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* [PATCH] gpio: pcf857x: use client->irq for gpio_to_irq()
From: Kuninori Morimoto @ 2012-12-06  9:10 UTC (permalink / raw)
  To: linus.walleij@linaro.org
  Cc: Simon, Linux-SH, ENGELMAYER Christian, linux-kernel
In-Reply-To: <EA116A090B102B4EBF532A9EECFA1EC579525758@vie196nt>

6e20a0a429bd4dc07d6de16d9c247270e04e4aa0
(gpio: pcf857x: enable gpio_to_irq() support)
added gpio_to_irq() support on pcf857x driver,
but it used pdata->irq.
This patch modifies driver to use client->irq instead of it.
It modifies kzm9g board platform settings,
and device probe information too.
This patch is tested on kzm9g board

Reported-by: ENGELMAYER Christian Christian.Engelmayer@frequentis.com
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
>> Simon

This patch modify kzm9g board settings.
Please give your acked-by on this patch

 arch/arm/mach-shmobile/board-kzm9g.c |    2 +-
 drivers/gpio/gpio-pcf857x.c          |   29 +++++++++++------------------
 include/linux/i2c/pcf857x.h          |    3 ---
 3 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-kzm9g.c b/arch/arm/mach-shmobile/board-kzm9g.c
index f63f2ee..c02448d 100644
--- a/arch/arm/mach-shmobile/board-kzm9g.c
+++ b/arch/arm/mach-shmobile/board-kzm9g.c
@@ -552,7 +552,6 @@ static struct platform_device fsi_ak4648_device = {
 /* I2C */
 static struct pcf857x_platform_data pcf8575_pdata = {
 	.gpio_base	= GPIO_PCF8575_BASE,
-	.irq		= intcs_evt2irq(0x3260), /* IRQ19 */
 };
 
 static struct i2c_board_info i2c0_devices[] = {
@@ -582,6 +581,7 @@ static struct i2c_board_info i2c1_devices[] = {
 static struct i2c_board_info i2c3_devices[] = {
 	{
 		I2C_BOARD_INFO("pcf8575", 0x20),
+		.irq		= intcs_evt2irq(0x3260), /* IRQ19 */
 		.platform_data = &pcf8575_pdata,
 	},
 };
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 16af35c..a19b745 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -223,11 +223,11 @@ static void pcf857x_irq_domain_cleanup(struct pcf857x *gpio)
 
 static int pcf857x_irq_domain_init(struct pcf857x *gpio,
 				   struct pcf857x_platform_data *pdata,
-				   struct device *dev)
+				   struct i2c_client *client)
 {
 	int status;
 
-	gpio->irq_domain = irq_domain_add_linear(dev->of_node,
+	gpio->irq_domain = irq_domain_add_linear(client->dev.of_node,
 						 gpio->chip.ngpio,
 						 &pcf857x_irq_domain_ops,
 						 NULL);
@@ -235,15 +235,15 @@ static int pcf857x_irq_domain_init(struct pcf857x *gpio,
 		goto fail;
 
 	/* enable real irq */
-	status = request_irq(pdata->irq, pcf857x_irq_demux, 0,
-			     dev_name(dev), gpio);
+	status = request_irq(client->irq, pcf857x_irq_demux, 0,
+			     dev_name(&client->dev), gpio);
 	if (status)
 		goto fail;
 
 	/* enable gpio_to_irq() */
 	INIT_WORK(&gpio->work, pcf857x_irq_demux_work);
 	gpio->chip.to_irq	= pcf857x_to_irq;
-	gpio->irq		= pdata->irq;
+	gpio->irq		= client->irq;
 
 	return 0;
 
@@ -285,8 +285,8 @@ static int pcf857x_probe(struct i2c_client *client,
 	gpio->chip.ngpio		= id->driver_data;
 
 	/* enable gpio_to_irq() if platform has settings */
-	if (pdata && pdata->irq) {
-		status = pcf857x_irq_domain_init(gpio, pdata, &client->dev);
+	if (pdata && client->irq) {
+		status = pcf857x_irq_domain_init(gpio, pdata, client);
 		if (status < 0) {
 			dev_err(&client->dev, "irq_domain init failed\n");
 			goto fail;
@@ -368,15 +368,6 @@ static int pcf857x_probe(struct i2c_client *client,
 	if (status < 0)
 		goto fail;
 
-	/* NOTE: these chips can issue "some pin-changed" IRQs, which we
-	 * don't yet even try to use.  Among other issues, the relevant
-	 * genirq state isn't available to modular drivers; and most irq
-	 * methods can't be called from sleeping contexts.
-	 */
-
-	dev_info(&client->dev, "%s\n",
-			client->irq ? " (irq ignored)" : "");
-
 	/* Let platform code set up the GPIOs and their users.
 	 * Now is the first time anyone could use them.
 	 */
@@ -388,13 +379,15 @@ static int pcf857x_probe(struct i2c_client *client,
 			dev_warn(&client->dev, "setup --> %d\n", status);
 	}
 
+	dev_info(&client->dev, "probed\n");
+
 	return 0;
 
 fail:
 	dev_dbg(&client->dev, "probe error %d for '%s'\n",
 			status, client->name);
 
-	if (pdata && pdata->irq)
+	if (pdata && client->irq)
 		pcf857x_irq_domain_cleanup(gpio);
 
 	kfree(gpio);
@@ -418,7 +411,7 @@ static int pcf857x_remove(struct i2c_client *client)
 		}
 	}
 
-	if (pdata && pdata->irq)
+	if (pdata && client->irq)
 		pcf857x_irq_domain_cleanup(gpio);
 
 	status = gpiochip_remove(&gpio->chip);
diff --git a/include/linux/i2c/pcf857x.h b/include/linux/i2c/pcf857x.h
index 781e6bd..0767a2a 100644
--- a/include/linux/i2c/pcf857x.h
+++ b/include/linux/i2c/pcf857x.h
@@ -10,7 +10,6 @@
  * @setup: optional callback issued once the GPIOs are valid
  * @teardown: optional callback issued before the GPIOs are invalidated
  * @context: optional parameter passed to setup() and teardown()
- * @irq: optional interrupt number
  *
  * In addition to the I2C_BOARD_INFO() state appropriate to each chip,
  * the i2c_board_info used with the pcf875x driver must provide its
@@ -40,8 +39,6 @@ struct pcf857x_platform_data {
 					int gpio, unsigned ngpio,
 					void *context);
 	void		*context;
-
-	int		irq;
 };
 
 #endif /* __LINUX_PCF857X_H */
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH v3] media: V4L2: add temporary clock helpers
From: Laurent Pinchart @ 2012-12-06 10:13 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux Media Mailing List, Sakari Ailus, Sylwester Nawrocki,
	Hans Verkuil, Sylwester Nawrocki, Magnus Damm, linux-sh
In-Reply-To: <Pine.LNX.4.64.1212060839540.15211@axis700.grange>

Hi Guennadi,

On Thursday 06 December 2012 08:41:41 Guennadi Liakhovetski wrote:
> On Thu, 6 Dec 2012, Laurent Pinchart wrote:
> > On Tuesday 04 December 2012 11:42:15 Guennadi Liakhovetski wrote:
> > > Typical video devices like camera sensors require an external clock
> > > source. Many such devices cannot even access their hardware registers
> > > without a running clock. These clock sources should be controlled by
> > > their consumers. This should be performed, using the generic clock
> > > framework. Unfortunately so far only very few systems have been ported
> > > to that framework. This patch adds a set of temporary helpers, mimicking
> > > the generic clock API, to V4L2. Platforms, adopting the clock API,
> > > should switch to using it. Eventually this temporary API should be
> > > removed.
> > 
> > As discussed on Jabber, I think we should make the clock helpers use the
> > common clock framework when available, to avoid pushing support for the
> > two APIs to all sensor drivers. Do you plan to include that in v4 ? :-)
> 
> AAMOF, no, I don't. Originally I planned to add this only when the first
> user appears. We can also add it earlier - a test case could be hacked up
> pretty quickly. But in either case I'd prefer to have it as a separate
> patch.

OK, I'm fine with that.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [PATCH 1/2] ARM: SH-Mobile: sh73a0: Secondary CPUs handle own SCU flags
From: Bastian Hecht @ 2012-12-06 12:08 UTC (permalink / raw)
  To: linux-arm-kernel

From: Bastian Hecht <hechtb@gmail.com>

When booting secondary CPUs we have used the main CPU to set up the
Snoop Control Unit flags of these CPUs. It is a cleaner approach
if every CPU takes care of its own flags. We avoid the need for
locking and the program logic is more concise. With this patch the file
headsmp-sh73a0.S is added that contains a startup vector for secondary CPUs
that sets up its own SCU flags.
Further in sh73a0_smp_prepare_cpus() we can rely on the generic ARM helper
scu_power_mode(). This is possible as we don't cross borders anymore (every
CPU handles its own flags) and need no locking. So we can throw out the
needless function modify_scu_cpu_psr().

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
---
 arch/arm/mach-shmobile/Makefile              |    2 +-
 arch/arm/mach-shmobile/headsmp-sh73a0.S      |   50 ++++++++++++++++++++++++++
 arch/arm/mach-shmobile/include/mach/common.h |    1 +
 arch/arm/mach-shmobile/smp-sh73a0.c          |   30 +++-------------
 4 files changed, 56 insertions(+), 27 deletions(-)
 create mode 100644 arch/arm/mach-shmobile/headsmp-sh73a0.S

diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index fe2c97c..a7beb61 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -17,7 +17,7 @@ obj-$(CONFIG_ARCH_EMEV2)	+= setup-emev2.o clock-emev2.o
 # SMP objects
 smp-y				:= platsmp.o headsmp.o
 smp-$(CONFIG_HOTPLUG_CPU)	+= hotplug.o
-smp-$(CONFIG_ARCH_SH73A0)	+= smp-sh73a0.o
+smp-$(CONFIG_ARCH_SH73A0)	+= smp-sh73a0.o headsmp-sh73a0.o
 smp-$(CONFIG_ARCH_R8A7779)	+= smp-r8a7779.o
 smp-$(CONFIG_ARCH_EMEV2)	+= smp-emev2.o
 
diff --git a/arch/arm/mach-shmobile/headsmp-sh73a0.S b/arch/arm/mach-shmobile/headsmp-sh73a0.S
new file mode 100644
index 0000000..bec4c0d
--- /dev/null
+++ b/arch/arm/mach-shmobile/headsmp-sh73a0.S
@@ -0,0 +1,50 @@
+/*
+ * SMP support for SoC sh73a0
+ *
+ * Copyright (C) 2012 Bastian Hecht
+ *
+ * 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 the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR /PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <linux/linkage.h>
+#include <linux/init.h>
+#include <asm/memory.h>
+
+	__CPUINIT
+/*
+ * Reset vector for secondary CPUs.
+ *
+ * First we turn on L1 cache coherency for our CPU. Then we jump to
+ * shmobile_invalidate_start that invalidates the cache and hands over control
+ * to the common ARM startup code.
+ * This function will be mapped to address 0 by the SBAR register.
+ * A normal branch is out of range here so we need a long jump. We jump to
+ * the physical address as the MMU is still turned off.
+ */
+	.align	12
+ENTRY(sh73a0_secondary_vector)
+	mrc     p15, 0, r0, c0, c0, 5	@ read MIPDR
+	and	r0, r0, #3		@ mask out cpu ID
+	lsl	r0, r0, #3		@ we will shift by cpu_id * 8 bits
+	mov	r1, #0xf0000000		@ SCU base address
+	ldr	r2, [r1, #8]		@ SCU Power Status Register
+	mov	r3, #3
+	bic	r2, r2, r3, lsl r0	@ Clear bits of our CPU (Run Mode)
+	str	r2, [r1, #8]		@ write back
+
+	ldr	pc, 1f
+1:	.long shmobile_invalidate_start - PAGE_OFFSET + PLAT_PHYS_OFFSET
+ENDPROC(sh73a0_secondary_vector)
diff --git a/arch/arm/mach-shmobile/include/mach/common.h b/arch/arm/mach-shmobile/include/mach/common.h
index d47e215..f2e2c29 100644
--- a/arch/arm/mach-shmobile/include/mach/common.h
+++ b/arch/arm/mach-shmobile/include/mach/common.h
@@ -54,6 +54,7 @@ extern void sh73a0_add_early_devices(void);
 extern void sh73a0_add_standard_devices(void);
 extern void sh73a0_clock_init(void);
 extern void sh73a0_pinmux_init(void);
+extern void sh73a0_secondary_vector(void);
 extern struct clk sh73a0_extal1_clk;
 extern struct clk sh73a0_extal2_clk;
 extern struct clk sh73a0_extcki_clk;
diff --git a/arch/arm/mach-shmobile/smp-sh73a0.c b/arch/arm/mach-shmobile/smp-sh73a0.c
index 624f00f..5e36f5d 100644
--- a/arch/arm/mach-shmobile/smp-sh73a0.c
+++ b/arch/arm/mach-shmobile/smp-sh73a0.c
@@ -41,9 +41,6 @@ static void __iomem *scu_base_addr(void)
 	return (void __iomem *)0xf0000000;
 }
 
-static DEFINE_SPINLOCK(scu_lock);
-static unsigned long tmp;
-
 #ifdef CONFIG_HAVE_ARM_TWD
 static DEFINE_TWD_LOCAL_TIMER(twd_local_timer, 0xf0000600, 29);
 void __init sh73a0_register_twd(void)
@@ -52,20 +49,6 @@ void __init sh73a0_register_twd(void)
 }
 #endif
 
-static void modify_scu_cpu_psr(unsigned long set, unsigned long clr)
-{
-	void __iomem *scu_base = scu_base_addr();
-
-	spin_lock(&scu_lock);
-	tmp = __raw_readl(scu_base + 8);
-	tmp &= ~clr;
-	tmp |= set;
-	spin_unlock(&scu_lock);
-
-	/* disable cache coherency after releasing the lock */
-	__raw_writel(tmp, scu_base + 8);
-}
-
 static unsigned int __init sh73a0_get_core_count(void)
 {
 	void __iomem *scu_base = scu_base_addr();
@@ -82,9 +65,6 @@ static int __cpuinit sh73a0_boot_secondary(unsigned int cpu, struct task_struct
 {
 	cpu = cpu_logical_map(cpu);
 
-	/* enable cache coherency */
-	modify_scu_cpu_psr(0, 3 << (cpu * 8));
-
 	if (((__raw_readl(PSTR) >> (4 * cpu)) & 3) = 3)
 		__raw_writel(1 << cpu, WUPCR);	/* wake up */
 	else
@@ -95,16 +75,14 @@ static int __cpuinit sh73a0_boot_secondary(unsigned int cpu, struct task_struct
 
 static void __init sh73a0_smp_prepare_cpus(unsigned int max_cpus)
 {
-	int cpu = cpu_logical_map(0);
-
 	scu_enable(scu_base_addr());
 
-	/* Map the reset vector (in headsmp.S) */
+	/* Map the reset vector (in headsmp-sh73a0.S) */
 	__raw_writel(0, APARMBAREA);      /* 4k */
-	__raw_writel(__pa(shmobile_secondary_vector), SBAR);
+	__raw_writel(__pa(sh73a0_secondary_vector), SBAR);
 
-	/* enable cache coherency on CPU0 */
-	modify_scu_cpu_psr(0, 3 << (cpu * 8));
+	/* enable cache coherency on booting CPU */
+	scu_power_mode(scu_base_addr(), SCU_PM_NORMAL);
 }
 
 static void __init sh73a0_smp_init_cpus(void)
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/2] ARM: SH-Mobile: sh73a0: Add CPU Hotplug
From: Bastian Hecht @ 2012-12-06 12:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1354795719-5578-1-git-send-email-hechtb+renesas@gmail.com>

From: Bastian Hecht <hechtb@gmail.com>

Add the capability to add and remove CPUs on the fly.
The Cortex-A9 offers the possibility to take single cores out of the
MP Core. We add this capabilty taking care that caches are kept
coherent. For the actual shutdown via a WFI instruction, a code snippet
from the omap2 code tree is copied. Thanks for that! For verifying the
shutdown we rely on the internal SH73A0 Power Status Register
PSTR.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
---
 arch/arm/mach-shmobile/headsmp-sh73a0.S      |   46 ++++++++++++++++++++++++++
 arch/arm/mach-shmobile/include/mach/common.h |    1 +
 arch/arm/mach-shmobile/smp-sh73a0.c          |   41 +++++++++++++++++++----
 3 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-shmobile/headsmp-sh73a0.S b/arch/arm/mach-shmobile/headsmp-sh73a0.S
index bec4c0d..be463a3 100644
--- a/arch/arm/mach-shmobile/headsmp-sh73a0.S
+++ b/arch/arm/mach-shmobile/headsmp-sh73a0.S
@@ -23,6 +23,52 @@
 #include <linux/init.h>
 #include <asm/memory.h>
 
+/* Taken from arch/arm/mach-omap2/sleep44xx.S. Thanks! */
+ENTRY(sh73a0_do_wfi)
+        stmfd   sp!, {lr}
+
+        /*
+         * Execute an ISB instruction to ensure that all of the
+         * CP15 register changes have been committed.
+         */
+        isb
+
+        /*
+         * Execute a barrier instruction to ensure that all cache,
+         * TLB and branch predictor maintenance operations issued
+         * by any CPU in the cluster have completed.
+         */
+        dsb
+        dmb
+
+        /*
+         * Execute a WFI instruction and wait until the
+         * STANDBYWFI output is asserted to indicate that the
+         * CPU is in idle and low power state. CPU can specualatively
+         * prefetch the instructions so add NOPs after WFI. Sixteen
+         * NOPs as per Cortex-A9 pipeline.
+         */
+        wfi                                     @ Wait For Interrupt
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+        nop
+
+        ldmfd   sp!, {pc}
+ENDPROC(sh73a0_do_wfi)
+
 	__CPUINIT
 /*
  * Reset vector for secondary CPUs.
diff --git a/arch/arm/mach-shmobile/include/mach/common.h b/arch/arm/mach-shmobile/include/mach/common.h
index f2e2c29..40f767e 100644
--- a/arch/arm/mach-shmobile/include/mach/common.h
+++ b/arch/arm/mach-shmobile/include/mach/common.h
@@ -55,6 +55,7 @@ extern void sh73a0_add_standard_devices(void);
 extern void sh73a0_clock_init(void);
 extern void sh73a0_pinmux_init(void);
 extern void sh73a0_secondary_vector(void);
+extern void sh73a0_do_wfi(void);
 extern struct clk sh73a0_extal1_clk;
 extern struct clk sh73a0_extal2_clk;
 extern struct clk sh73a0_extcki_clk;
diff --git a/arch/arm/mach-shmobile/smp-sh73a0.c b/arch/arm/mach-shmobile/smp-sh73a0.c
index 5e36f5d..9237e13 100644
--- a/arch/arm/mach-shmobile/smp-sh73a0.c
+++ b/arch/arm/mach-shmobile/smp-sh73a0.c
@@ -24,6 +24,7 @@
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <mach/common.h>
+#include <asm/cacheflush.h>
 #include <asm/smp_plat.h>
 #include <mach/sh73a0.h>
 #include <asm/smp_scu.h>
@@ -36,6 +37,8 @@
 #define SBAR		IOMEM(0xe6180020)
 #define APARMBAREA	IOMEM(0xe6f10020)
 
+#define PSTR_SHUTDOWN_MODE	3
+
 static void __iomem *scu_base_addr(void)
 {
 	return (void __iomem *)0xf0000000;
@@ -92,16 +95,20 @@ static void __init sh73a0_smp_init_cpus(void)
 	shmobile_smp_init_cpus(ncores);
 }
 
-static int __maybe_unused sh73a0_cpu_kill(unsigned int cpu)
+#ifdef CONFIG_HOTPLUG_CPU
+static int sh73a0_cpu_kill(unsigned int cpu)
 {
+
 	int k;
+	u32 pstr;
 
-	/* this function is running on another CPU than the offline target,
-	 * here we need wait for shutdown code in platform_cpu_die() to
-	 * finish before asking SoC-specific code to power off the CPU core.
+	/*
+	 * wait until the power status register confirms the shutdown of the
+	 * offline target
 	 */
 	for (k = 0; k < 1000; k++) {
-		if (shmobile_cpu_is_dead(cpu))
+		pstr = (__raw_readl(PSTR) >> (4 * cpu)) & 3;
+		if (pstr = PSTR_SHUTDOWN_MODE)
 			return 1;
 
 		mdelay(1);
@@ -110,6 +117,28 @@ static int __maybe_unused sh73a0_cpu_kill(unsigned int cpu)
 	return 0;
 }
 
+static void sh73a0_cpu_die(unsigned int cpu)
+{
+	/*
+	 * The ARM MPcore does not issue a cache coherency request for the L1
+	 * cache when powering off single CPUs. We must take care of this and
+	 * further caches.
+	 */
+	dsb();
+	flush_cache_all();
+
+	/* Set power off mode. This takes the CPU out of the MP cluster */
+	scu_power_mode(scu_base_addr(), SCU_PM_POWEROFF);
+
+	/* Enter shutdown mode */
+	sh73a0_do_wfi();
+
+	/* We assume success always. We never reach this */
+	pr_err("Shutting down CPU failed. This should never happen!\n");
+	for (;;)
+		;
+}
+#endif /* CONFIG_HOTPLUG_CPU */
 
 struct smp_operations sh73a0_smp_ops __initdata = {
 	.smp_init_cpus		= sh73a0_smp_init_cpus,
@@ -118,7 +147,7 @@ struct smp_operations sh73a0_smp_ops __initdata = {
 	.smp_boot_secondary	= sh73a0_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
 	.cpu_kill		= sh73a0_cpu_kill,
-	.cpu_die		= shmobile_cpu_die,
+	.cpu_die		= sh73a0_cpu_die,
 	.cpu_disable		= shmobile_cpu_disable,
 #endif
 };
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 1/4] spi: sh-msiof: Remove unneeded clock name
From: Grant Likely @ 2012-12-06 14:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1352288407-20594-2-git-send-email-hechtb@gmail.com>

On Wed,  7 Nov 2012 12:40:04 +0100, Bastian Hecht <hechtb@gmail.com> wrote:
> clk_get() no longer needs a character string for associating the right
> clock as this is done via the device struct now.
> 
> Signed-off-by: Bastian Hecht <hechtb@gmail.com>

Applied, thanks.

g.

> ---
>  drivers/spi/spi-sh-msiof.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 1f466bc..96358d0 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -597,7 +597,6 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
>  	struct resource	*r;
>  	struct spi_master *master;
>  	struct sh_msiof_spi_priv *p;
> -	char clk_name[16];
>  	int i;
>  	int ret;
>  
> @@ -614,10 +613,9 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
>  	p->info = pdev->dev.platform_data;
>  	init_completion(&p->done);
>  
> -	snprintf(clk_name, sizeof(clk_name), "msiof%d", pdev->id);
> -	p->clk = clk_get(&pdev->dev, clk_name);
> +	p->clk = clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(p->clk)) {
> -		dev_err(&pdev->dev, "cannot get clock \"%s\"\n", clk_name);
> +		dev_err(&pdev->dev, "cannot get clock\n");
>  		ret = PTR_ERR(p->clk);
>  		goto err1;
>  	}
> -- 
> 1.7.5.4
> 
> 
> ------------------------------------------------------------------------------
> LogMeIn Central: Instant, anywhere, Remote PC access and management.
> Stay in control, update software, and manage PCs from one command center
> Diagnose problems and improve visibility into emerging IT issues
> Automate, monitor and manage. Do more in less time with Central
> http://p.sf.net/sfu/logmein12331_d2d
> _______________________________________________
> spi-devel-general mailing list
> spi-devel-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

^ permalink raw reply

* Re: [PATCH 4/4] devicetree: Add Renesas SH Mobile MSIOF spi controller binding doc
From: Grant Likely @ 2012-12-06 14:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1352288407-20594-5-git-send-email-hechtb@gmail.com>

On Wed,  7 Nov 2012 12:40:07 +0100, Bastian Hecht <hechtb@gmail.com> wrote:
> Add binding documentation for Renesas' MSIOF SPI controller.
> 
> Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> ---
>  Documentation/devicetree/bindings/spi/sh-msiof.txt |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/sh-msiof.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
> new file mode 100644
> index 0000000..b62312e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
> @@ -0,0 +1,12 @@
> +Renesas MSIOF spi controller
> +
> +Required properties:
> +- compatible : 	"renesas,sh-msiof" for SuperH or
> +		"renesas,sh-mobile-msiof" for SH Mobile series
> +- reg : Offset and length of the register set for the device
> +- interrupts : interrupt line used by MSIOF
> +
> +Optional properties:
> +- chip_select  : Chip select, defaults to 0

This doesn't make a lot of sense to me. What is this property for? Is
there more than one CS? And if there is, shouldn't the SPI driver be
able to manipulate more than one?

> +- tx_fifo_size : Overrides the default tx fifo size given in words
> +- rx_fifo_size : Overrides the default rx fifo size given in words

Nit: property names should use '-' instead of '_' (by convention) and
custom properties should be prefixed with the manufacturer prefix to
avoid namespace collisions. ie. "renesas,tx-fifo-size.

g.

^ permalink raw reply

* Re: [PATCH 4/4] devicetree: Add Renesas SH Mobile MSIOF spi controller binding doc
From: Grant Likely @ 2012-12-06 14:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121206140849.280243E089D@localhost>

On Thu, Dec 6, 2012 at 2:08 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Wed,  7 Nov 2012 12:40:07 +0100, Bastian Hecht <hechtb@gmail.com> wrote:
>> Add binding documentation for Renesas' MSIOF SPI controller.
>>
>> Signed-off-by: Bastian Hecht <hechtb@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/spi/sh-msiof.txt |   12 ++++++++++++
>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/spi/sh-msiof.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
>> new file mode 100644
>> index 0000000..b62312e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
>> @@ -0,0 +1,12 @@
>> +Renesas MSIOF spi controller
>> +
>> +Required properties:
>> +- compatible :       "renesas,sh-msiof" for SuperH or
>> +             "renesas,sh-mobile-msiof" for SH Mobile series
>> +- reg : Offset and length of the register set for the device
>> +- interrupts : interrupt line used by MSIOF
>> +
>> +Optional properties:
>> +- chip_select  : Chip select, defaults to 0
>
> This doesn't make a lot of sense to me. What is this property for? Is
> there more than one CS? And if there is, shouldn't the SPI driver be
> able to manipulate more than one?

Oh, wait, from reading the code, is this the number of chip select
lines? If so, then there is a common property for this. This property
should be named "num-cs".

>
>> +- tx_fifo_size : Overrides the default tx fifo size given in words
>> +- rx_fifo_size : Overrides the default rx fifo size given in words
>
> Nit: property names should use '-' instead of '_' (by convention) and
> custom properties should be prefixed with the manufacturer prefix to
> avoid namespace collisions. ie. "renesas,tx-fifo-size.
>
> g.



--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 3/4] spi: sh-msiof: Add device tree parsing to driver
From: Grant Likely @ 2012-12-06 14:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1352288407-20594-4-git-send-email-hechtb@gmail.com>

On Wed,  7 Nov 2012 12:40:06 +0100, Bastian Hecht <hechtb@gmail.com> wrote:
> This adds the capability to retrieve setup data from the device tree
> node. The usage of platform data is still available.
> 
> Signed-off-by: Bastian Hecht <hechtb@gmail.com>

Aside from the binding issues I've pointed out on patch 4, this change
looks fine. I'll wait for an updated patch before committing.

g.


^ permalink raw reply

* Re: [PATCH v2 00/77] SH pin control and GPIO rework with OF support
From: Linus Walleij @ 2012-12-06 18:52 UTC (permalink / raw)
  To: linux-sh
In-Reply-To: <1353974596-30033-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

On Thu, Dec 6, 2012 at 2:34 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

>> Let's have this patchset finished for v3.9 say? :-)
>
> I'll try to.

Any working condition along the way will basically be fine to merge
from my point of view. Doing so leaves the kernel in a way better
shape than before, and I fully trust you to go in and finalize it, so
I think we can move ahead quite swiftly.

Yours,
Linus Walleij

^ permalink raw reply

* Unsere Referenz: G20/CT/GA12
From: THE G20 @ 2012-12-06 21:02 UTC (permalink / raw)


Unsere Referenz: G20/CT/GA12

Wir freuen uns, Ihnen mitzuteilen, dass Sie von der G-20 wurden
ausgewahlt, um ein Preisgeld von $ 861,937.00 USD Preis zu erhalten.
Weitere Informationen zu den Bedingungen der Zahlung wird in seiner
Antwort veroffentlicht werden. Wir erwarten empfangt eine schnelle Antwort
von Ihnen.

Grube
Nate Wiley. (Claims Department)
Es konnte dir sein, ist ein eingetragenes Warenzeichen der G-20.





^ permalink raw reply

* Re: [PATCH] gpio: pcf857x: use client->irq for gpio_to_irq()
From: Simon Horman @ 2012-12-07  0:26 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: linus.walleij@linaro.org, Linux-SH, ENGELMAYER Christian,
	linux-kernel
In-Reply-To: <87lidb7dhb.wl%kuninori.morimoto.gx@renesas.com>

On Thu, Dec 06, 2012 at 01:10:28AM -0800, Kuninori Morimoto wrote:
> 6e20a0a429bd4dc07d6de16d9c247270e04e4aa0
> (gpio: pcf857x: enable gpio_to_irq() support)
> added gpio_to_irq() support on pcf857x driver,
> but it used pdata->irq.
> This patch modifies driver to use client->irq instead of it.
> It modifies kzm9g board platform settings,
> and device probe information too.
> This patch is tested on kzm9g board
> 
> Reported-by: ENGELMAYER Christian Christian.Engelmayer@frequentis.com
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> >> Simon
> 
> This patch modify kzm9g board settings.
> Please give your acked-by on this patch

Thanks Morimoto-san,

This seems reasonable to me and a quick boot check doesn't show up
any breakage.

Acked-by: Simon Horman <horms+renesas@verge.net.au>

> 
>  arch/arm/mach-shmobile/board-kzm9g.c |    2 +-
>  drivers/gpio/gpio-pcf857x.c          |   29 +++++++++++------------------
>  include/linux/i2c/pcf857x.h          |    3 ---
>  3 files changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-kzm9g.c b/arch/arm/mach-shmobile/board-kzm9g.c
> index f63f2ee..c02448d 100644
> --- a/arch/arm/mach-shmobile/board-kzm9g.c
> +++ b/arch/arm/mach-shmobile/board-kzm9g.c
> @@ -552,7 +552,6 @@ static struct platform_device fsi_ak4648_device = {
>  /* I2C */
>  static struct pcf857x_platform_data pcf8575_pdata = {
>  	.gpio_base	= GPIO_PCF8575_BASE,
> -	.irq		= intcs_evt2irq(0x3260), /* IRQ19 */
>  };
>  
>  static struct i2c_board_info i2c0_devices[] = {
> @@ -582,6 +581,7 @@ static struct i2c_board_info i2c1_devices[] = {
>  static struct i2c_board_info i2c3_devices[] = {
>  	{
>  		I2C_BOARD_INFO("pcf8575", 0x20),
> +		.irq		= intcs_evt2irq(0x3260), /* IRQ19 */
>  		.platform_data = &pcf8575_pdata,
>  	},
>  };
> diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
> index 16af35c..a19b745 100644
> --- a/drivers/gpio/gpio-pcf857x.c
> +++ b/drivers/gpio/gpio-pcf857x.c
> @@ -223,11 +223,11 @@ static void pcf857x_irq_domain_cleanup(struct pcf857x *gpio)
>  
>  static int pcf857x_irq_domain_init(struct pcf857x *gpio,
>  				   struct pcf857x_platform_data *pdata,
> -				   struct device *dev)
> +				   struct i2c_client *client)
>  {
>  	int status;
>  
> -	gpio->irq_domain = irq_domain_add_linear(dev->of_node,
> +	gpio->irq_domain = irq_domain_add_linear(client->dev.of_node,
>  						 gpio->chip.ngpio,
>  						 &pcf857x_irq_domain_ops,
>  						 NULL);
> @@ -235,15 +235,15 @@ static int pcf857x_irq_domain_init(struct pcf857x *gpio,
>  		goto fail;
>  
>  	/* enable real irq */
> -	status = request_irq(pdata->irq, pcf857x_irq_demux, 0,
> -			     dev_name(dev), gpio);
> +	status = request_irq(client->irq, pcf857x_irq_demux, 0,
> +			     dev_name(&client->dev), gpio);
>  	if (status)
>  		goto fail;
>  
>  	/* enable gpio_to_irq() */
>  	INIT_WORK(&gpio->work, pcf857x_irq_demux_work);
>  	gpio->chip.to_irq	= pcf857x_to_irq;
> -	gpio->irq		= pdata->irq;
> +	gpio->irq		= client->irq;
>  
>  	return 0;
>  
> @@ -285,8 +285,8 @@ static int pcf857x_probe(struct i2c_client *client,
>  	gpio->chip.ngpio		= id->driver_data;
>  
>  	/* enable gpio_to_irq() if platform has settings */
> -	if (pdata && pdata->irq) {
> -		status = pcf857x_irq_domain_init(gpio, pdata, &client->dev);
> +	if (pdata && client->irq) {
> +		status = pcf857x_irq_domain_init(gpio, pdata, client);
>  		if (status < 0) {
>  			dev_err(&client->dev, "irq_domain init failed\n");
>  			goto fail;
> @@ -368,15 +368,6 @@ static int pcf857x_probe(struct i2c_client *client,
>  	if (status < 0)
>  		goto fail;
>  
> -	/* NOTE: these chips can issue "some pin-changed" IRQs, which we
> -	 * don't yet even try to use.  Among other issues, the relevant
> -	 * genirq state isn't available to modular drivers; and most irq
> -	 * methods can't be called from sleeping contexts.
> -	 */
> -
> -	dev_info(&client->dev, "%s\n",
> -			client->irq ? " (irq ignored)" : "");
> -
>  	/* Let platform code set up the GPIOs and their users.
>  	 * Now is the first time anyone could use them.
>  	 */
> @@ -388,13 +379,15 @@ static int pcf857x_probe(struct i2c_client *client,
>  			dev_warn(&client->dev, "setup --> %d\n", status);
>  	}
>  
> +	dev_info(&client->dev, "probed\n");
> +
>  	return 0;
>  
>  fail:
>  	dev_dbg(&client->dev, "probe error %d for '%s'\n",
>  			status, client->name);
>  
> -	if (pdata && pdata->irq)
> +	if (pdata && client->irq)
>  		pcf857x_irq_domain_cleanup(gpio);
>  
>  	kfree(gpio);
> @@ -418,7 +411,7 @@ static int pcf857x_remove(struct i2c_client *client)
>  		}
>  	}
>  
> -	if (pdata && pdata->irq)
> +	if (pdata && client->irq)
>  		pcf857x_irq_domain_cleanup(gpio);
>  
>  	status = gpiochip_remove(&gpio->chip);
> diff --git a/include/linux/i2c/pcf857x.h b/include/linux/i2c/pcf857x.h
> index 781e6bd..0767a2a 100644
> --- a/include/linux/i2c/pcf857x.h
> +++ b/include/linux/i2c/pcf857x.h
> @@ -10,7 +10,6 @@
>   * @setup: optional callback issued once the GPIOs are valid
>   * @teardown: optional callback issued before the GPIOs are invalidated
>   * @context: optional parameter passed to setup() and teardown()
> - * @irq: optional interrupt number
>   *
>   * In addition to the I2C_BOARD_INFO() state appropriate to each chip,
>   * the i2c_board_info used with the pcf875x driver must provide its
> @@ -40,8 +39,6 @@ struct pcf857x_platform_data {
>  					int gpio, unsigned ngpio,
>  					void *context);
>  	void		*context;
> -
> -	int		irq;
>  };
>  
>  #endif /* __LINUX_PCF857X_H */
> -- 
> 1.7.9.5
> 

^ permalink raw reply

* Re: [Patch v4 08/12] memory-hotplug: remove memmap of sparse-vmemmap
From: Tang Chen @ 2012-12-07  1:42 UTC (permalink / raw)
  To: Jianguo Wu
  Cc: Wen Congyang, x86, linux-mm, linux-kernel, linuxppc-dev,
	linux-acpi, linux-s390, linux-sh, linux-ia64, cmetcalf,
	sparclinux, David Rientjes, Jiang Liu, Len Brown, benh, paulus,
	Christoph Lameter, Minchan Kim, Andrew Morton, KOSAKI Motohiro,
	Yasuaki Ishimatsu
In-Reply-To: <50BC0D2D.8040008@huawei.com>

Hi Wu,

I met some problems when I was digging into the code. It's very
kind of you if you could help me with that. :)

If I misunderstood your code, please tell me.
Please see below. :)

On 12/03/2012 10:23 AM, Jianguo Wu wrote:
> Signed-off-by: Jianguo Wu<wujianguo@huawei.com>
> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
> ---
>   include/linux/mm.h  |    1 +
>   mm/sparse-vmemmap.c |  231 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   mm/sparse.c         |    3 +-
>   3 files changed, 234 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5657670..1f26af5 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1642,6 +1642,7 @@ int vmemmap_populate(struct page *start_page, unsigned long pages, int node);
>   void vmemmap_populate_print_last(void);
>   void register_page_bootmem_memmap(unsigned long section_nr, struct page *map,
>   				  unsigned long size);
> +void vmemmap_free(struct page *memmap, unsigned long nr_pages);
>
>   enum mf_flags {
>   	MF_COUNT_INCREASED = 1<<  0,
> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> index 1b7e22a..748732d 100644
> --- a/mm/sparse-vmemmap.c
> +++ b/mm/sparse-vmemmap.c
> @@ -29,6 +29,10 @@
>   #include<asm/pgalloc.h>
>   #include<asm/pgtable.h>
>
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +#include<asm/tlbflush.h>
> +#endif
> +
>   /*
>    * Allocate a block of memory to be used to back the virtual memory map
>    * or to back the page tables that are used to create the mapping.
> @@ -224,3 +228,230 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>   		vmemmap_buf_end = NULL;
>   	}
>   }
> +
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +
> +#define PAGE_INUSE 0xFD
> +
> +static void vmemmap_free_pages(struct page *page, int order)
> +{
> +	struct zone *zone;
> +	unsigned long magic;
> +
> +	magic = (unsigned long) page->lru.next;
> +	if (magic = SECTION_INFO || magic = MIX_SECTION_INFO) {
> +		put_page_bootmem(page);
> +
> +		zone = page_zone(page);
> +		zone_span_writelock(zone);
> +		zone->present_pages++;
> +		zone_span_writeunlock(zone);
> +		totalram_pages++;
> +	} else
> +		free_pages((unsigned long)page_address(page), order);

Here, I think SECTION_INFO and MIX_SECTION_INFO pages are all allocated
by bootmem, so I put this function this way.

I'm not sure if parameter order is necessary here. It will always be 0
in your code. Is this OK to you ?

static void free_pagetable(struct page *page)
{
         struct zone *zone;
         bool bootmem = false;
         unsigned long magic;

         /* bootmem page has reserved flag */
         if (PageReserved(page)) {
                 __ClearPageReserved(page);
                 bootmem = true;
         }

         magic = (unsigned long) page->lru.next;
         if (magic = SECTION_INFO || magic = MIX_SECTION_INFO)
                 put_page_bootmem(page);
         else
                 __free_page(page);

         /*
          * SECTION_INFO pages and MIX_SECTION_INFO pages
          * are all allocated by bootmem.
          */
         if (bootmem) {
                 zone = page_zone(page);
                 zone_span_writelock(zone);
                 zone->present_pages++;
                 zone_span_writeunlock(zone);
                 totalram_pages++;
         }
}

(snip)

> +
> +static void vmemmap_pte_remove(pmd_t *pmd, unsigned long addr, unsigned long end)
> +{
> +	pte_t *pte;
> +	unsigned long next;
> +	void *page_addr;
> +
> +	pte = pte_offset_kernel(pmd, addr);
> +	for (; addr<  end; pte++, addr += PAGE_SIZE) {
> +		next = (addr + PAGE_SIZE)&  PAGE_MASK;
> +		if (next>  end)
> +			next = end;
> +
> +		if (pte_none(*pte))

Here, you checked xxx_none() in your vmemmap_xxx_remove(), but you used
!xxx_present() in your x86_64 patches. Is it OK if I only check
!xxx_present() ?

> +			continue;
> +		if (IS_ALIGNED(addr, PAGE_SIZE)&&
> +		    IS_ALIGNED(next, PAGE_SIZE)) {
> +			vmemmap_free_pages(pte_page(*pte), 0);
> +			spin_lock(&init_mm.page_table_lock);
> +			pte_clear(&init_mm, addr, pte);
> +			spin_unlock(&init_mm.page_table_lock);
> +		} else {
> +			/*
> +			 * Removed page structs are filled with 0xFD.
> +			 */
> +			memset((void *)addr, PAGE_INUSE, next - addr);
> +			page_addr = page_address(pte_page(*pte));
> +
> +			if (!memchr_inv(page_addr, PAGE_INUSE, PAGE_SIZE)) {
> +				spin_lock(&init_mm.page_table_lock);
> +				pte_clear(&init_mm, addr, pte);
> +				spin_unlock(&init_mm.page_table_lock);

Here, since we clear pte, we should also free the page, right ?

> +			}
> +		}
> +	}
> +
> +	free_pte_table(pmd);
> +	__flush_tlb_all();
> +}
> +
> +static void vmemmap_pmd_remove(pud_t *pud, unsigned long addr, unsigned long end)
> +{
> +	unsigned long next;
> +	pmd_t *pmd;
> +
> +	pmd = pmd_offset(pud, addr);
> +	for (; addr<  end; addr = next, pmd++) {
> +		next = (addr, end);

And by the way, there isn't pte_addr_end() in kernel, why ?
I saw you calculated it like this:

                 next = (addr + PAGE_SIZE) & PAGE_MASK;
                 if (next > end)
                         next = end;

This logic is very similar to {pmd|pud|pgd}_addr_end(). Shall we add a
pte_addr_end() or something ? :)
Since there is no such code in kernel for a long time, I think there
must be some reasons.

I merged free_xxx_table() and remove_xxx_table() as common interfaces.

And again, thanks for your patient and nice explanation. :)

(snip)

^ permalink raw reply

* Re: [Patch v4 08/12] memory-hotplug: remove memmap of sparse-vmemmap
From: Jianguo Wu @ 2012-12-07  2:20 UTC (permalink / raw)
  To: Tang Chen
  Cc: Wen Congyang, x86, linux-mm, linux-kernel, linuxppc-dev,
	linux-acpi, linux-s390, linux-sh, linux-ia64, cmetcalf,
	sparclinux, David Rientjes, Jiang Liu, Len Brown, benh, paulus,
	Christoph Lameter, Minchan Kim, Andrew Morton, KOSAKI Motohiro,
	Yasuaki Ishimatsu
In-Reply-To: <50C14976.2050606@cn.fujitsu.com>

Hi Tang,

On 2012/12/7 9:42, Tang Chen wrote:

> Hi Wu,
> 
> I met some problems when I was digging into the code. It's very
> kind of you if you could help me with that. :)
> 
> If I misunderstood your code, please tell me.
> Please see below. :)
> 
> On 12/03/2012 10:23 AM, Jianguo Wu wrote:
>> Signed-off-by: Jianguo Wu<wujianguo@huawei.com>
>> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
>> ---
>>   include/linux/mm.h  |    1 +
>>   mm/sparse-vmemmap.c |  231 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   mm/sparse.c         |    3 +-
>>   3 files changed, 234 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 5657670..1f26af5 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -1642,6 +1642,7 @@ int vmemmap_populate(struct page *start_page, unsigned long pages, int node);
>>   void vmemmap_populate_print_last(void);
>>   void register_page_bootmem_memmap(unsigned long section_nr, struct page *map,
>>                     unsigned long size);
>> +void vmemmap_free(struct page *memmap, unsigned long nr_pages);
>>
>>   enum mf_flags {
>>       MF_COUNT_INCREASED = 1<<  0,
>> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
>> index 1b7e22a..748732d 100644
>> --- a/mm/sparse-vmemmap.c
>> +++ b/mm/sparse-vmemmap.c
>> @@ -29,6 +29,10 @@
>>   #include<asm/pgalloc.h>
>>   #include<asm/pgtable.h>
>>
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +#include<asm/tlbflush.h>
>> +#endif
>> +
>>   /*
>>    * Allocate a block of memory to be used to back the virtual memory map
>>    * or to back the page tables that are used to create the mapping.
>> @@ -224,3 +228,230 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>>           vmemmap_buf_end = NULL;
>>       }
>>   }
>> +
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +
>> +#define PAGE_INUSE 0xFD
>> +
>> +static void vmemmap_free_pages(struct page *page, int order)
>> +{
>> +    struct zone *zone;
>> +    unsigned long magic;
>> +
>> +    magic = (unsigned long) page->lru.next;
>> +    if (magic = SECTION_INFO || magic = MIX_SECTION_INFO) {
>> +        put_page_bootmem(page);
>> +
>> +        zone = page_zone(page);
>> +        zone_span_writelock(zone);
>> +        zone->present_pages++;
>> +        zone_span_writeunlock(zone);
>> +        totalram_pages++;
>> +    } else
>> +        free_pages((unsigned long)page_address(page), order);
> 
> Here, I think SECTION_INFO and MIX_SECTION_INFO pages are all allocated
> by bootmem, so I put this function this way.
> 
> I'm not sure if parameter order is necessary here. It will always be 0
> in your code. Is this OK to you ?
> 

parameter order is necessary in cpu_has_pse case:
	vmemmap_pmd_remove
		free_pagetable(pmd_page(*pmd), get_order(PMD_SIZE))

> static void free_pagetable(struct page *page)
> {
>         struct zone *zone;
>         bool bootmem = false;
>         unsigned long magic;
> 
>         /* bootmem page has reserved flag */
>         if (PageReserved(page)) {
>                 __ClearPageReserved(page);
>                 bootmem = true;
>         }
> 
>         magic = (unsigned long) page->lru.next;
>         if (magic = SECTION_INFO || magic = MIX_SECTION_INFO)
>                 put_page_bootmem(page);
>         else
>                 __free_page(page);
> 
>         /*
>          * SECTION_INFO pages and MIX_SECTION_INFO pages
>          * are all allocated by bootmem.
>          */
>         if (bootmem) {
>                 zone = page_zone(page);
>                 zone_span_writelock(zone);
>                 zone->present_pages++;
>                 zone_span_writeunlock(zone);
>                 totalram_pages++;
>         }
> }
> 
> (snip)
> 
>> +
>> +static void vmemmap_pte_remove(pmd_t *pmd, unsigned long addr, unsigned long end)
>> +{
>> +    pte_t *pte;
>> +    unsigned long next;
>> +    void *page_addr;
>> +
>> +    pte = pte_offset_kernel(pmd, addr);
>> +    for (; addr<  end; pte++, addr += PAGE_SIZE) {
>> +        next = (addr + PAGE_SIZE)&  PAGE_MASK;
>> +        if (next>  end)
>> +            next = end;
>> +
>> +        if (pte_none(*pte))
> 
> Here, you checked xxx_none() in your vmemmap_xxx_remove(), but you used
> !xxx_present() in your x86_64 patches. Is it OK if I only check
> !xxx_present() ?

It is Ok.

> 
>> +            continue;
>> +        if (IS_ALIGNED(addr, PAGE_SIZE)&&
>> +            IS_ALIGNED(next, PAGE_SIZE)) {
>> +            vmemmap_free_pages(pte_page(*pte), 0);
>> +            spin_lock(&init_mm.page_table_lock);
>> +            pte_clear(&init_mm, addr, pte);
>> +            spin_unlock(&init_mm.page_table_lock);
>> +        } else {
>> +            /*
>> +             * Removed page structs are filled with 0xFD.
>> +             */
>> +            memset((void *)addr, PAGE_INUSE, next - addr);
>> +            page_addr = page_address(pte_page(*pte));
>> +
>> +            if (!memchr_inv(page_addr, PAGE_INUSE, PAGE_SIZE)) {
>> +                spin_lock(&init_mm.page_table_lock);
>> +                pte_clear(&init_mm, addr, pte);
>> +                spin_unlock(&init_mm.page_table_lock);
> 
> Here, since we clear pte, we should also free the page, right ?
> 

Right, I forgot here, sorry.

>> +            }
>> +        }
>> +    }
>> +
>> +    free_pte_table(pmd);
>> +    __flush_tlb_all();
>> +}
>> +
>> +static void vmemmap_pmd_remove(pud_t *pud, unsigned long addr, unsigned long end)
>> +{
>> +    unsigned long next;
>> +    pmd_t *pmd;
>> +
>> +    pmd = pmd_offset(pud, addr);
>> +    for (; addr<  end; addr = next, pmd++) {
>> +        next = (addr, end);
> 
> And by the way, there isn't pte_addr_end() in kernel, why ?
> I saw you calculated it like this:
> 
>                 next = (addr + PAGE_SIZE) & PAGE_MASK;
>                 if (next > end)
>                         next = end;
> 
> This logic is very similar to {pmd|pud|pgd}_addr_end(). Shall we add a
> pte_addr_end() or something ? :)

Maybe just keep this for now if no other place need pte_addr_end()?

> Since there is no such code in kernel for a long time, I think there
> must be some reasons.

Maybe in current kernel, doesn't deal not PTE_SIZE alignment address?
 

> 
> I merged free_xxx_table() and remove_xxx_table() as common interfaces.

Greate!

Thanks for your work:).

> 
> And again, thanks for your patient and nice explanation. :)
> 
> (snip)
> 
> .
> 




^ permalink raw reply

* Re: [Patch v4 09/12] memory-hotplug: remove page table of x86_64 architecture
From: Tang Chen @ 2012-12-07  6:43 UTC (permalink / raw)
  To: Wen Congyang
  Cc: x86, linux-mm, linux-kernel, linuxppc-dev, linux-acpi, linux-s390,
	linux-sh, linux-ia64, cmetcalf, sparclinux, David Rientjes,
	Jiang Liu, Len Brown, benh, paulus, Christoph Lameter,
	Minchan Kim, Andrew Morton, KOSAKI Motohiro, Yasuaki Ishimatsu,
	Jianguo Wu, Jiang Liu
In-Reply-To: <1354010422-19648-10-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
> For hot removing memory, we sholud remove page table about the memory.
> So the patch searches a page table about the removed memory, and clear
> page table.

(snip)

> +void __meminit
> +kernel_physical_mapping_remove(unsigned long start, unsigned long end)
> +{
> +	unsigned long next;
> +	bool pgd_changed = false;
> +
> +	start = (unsigned long)__va(start);
> +	end = (unsigned long)__va(end);

Hi Wu,

Here, you expect start and end are physical addresses. But in
phys_xxx_remove() function, I think using virtual addresses is just
fine. Functions like pmd_addr_end() and pud_index() only calculate
an offset.

So, would you please tell me if we have to use physical addresses here ?

Thanks. :)

> +
> +	for (; start<  end; start = next) {
> +		pgd_t *pgd = pgd_offset_k(start);
> +		pud_t *pud;
> +
> +		next = pgd_addr_end(start, end);
> +
> +		if (!pgd_present(*pgd))
> +			continue;
> +
> +		pud = map_low_page((pud_t *)pgd_page_vaddr(*pgd));
> +		phys_pud_remove(pud, __pa(start), __pa(next));
> +		if (free_pud_table(pud, pgd))
> +			pgd_changed = true;
> +		unmap_low_page(pud);
> +	}
> +
> +	if (pgd_changed)
> +		sync_global_pgds(start, end - 1);
> +
> +	flush_tlb_all();
> +}
> +
>   #ifdef CONFIG_MEMORY_HOTREMOVE
>   int __ref arch_remove_memory(u64 start, u64 size)
>   {
> @@ -692,6 +921,8 @@ int __ref arch_remove_memory(u64 start, u64 size)
>   	ret = __remove_pages(zone, start_pfn, nr_pages);
>   	WARN_ON_ONCE(ret);
>
> +	kernel_physical_mapping_remove(start, start + size);
> +
>   	return ret;
>   }
>   #endif



^ permalink raw reply

* Re: [Patch v4 09/12] memory-hotplug: remove page table of x86_64 architecture
From: Jianguo Wu @ 2012-12-07  7:06 UTC (permalink / raw)
  To: Tang Chen
  Cc: Wen Congyang, x86, linux-mm, linux-kernel, linuxppc-dev,
	linux-acpi, linux-s390, linux-sh, linux-ia64, cmetcalf,
	sparclinux, David Rientjes, Jiang Liu, Len Brown, benh, paulus,
	Christoph Lameter, Minchan Kim, Andrew Morton, KOSAKI Motohiro,
	Yasuaki Ishimatsu, Jiang Liu
In-Reply-To: <50C19022.9000501@cn.fujitsu.com>

On 2012/12/7 14:43, Tang Chen wrote:

> On 11/27/2012 06:00 PM, Wen Congyang wrote:
>> For hot removing memory, we sholud remove page table about the memory.
>> So the patch searches a page table about the removed memory, and clear
>> page table.
> 
> (snip)
> 
>> +void __meminit
>> +kernel_physical_mapping_remove(unsigned long start, unsigned long end)
>> +{
>> +    unsigned long next;
>> +    bool pgd_changed = false;
>> +
>> +    start = (unsigned long)__va(start);
>> +    end = (unsigned long)__va(end);
> 
> Hi Wu,
> 
> Here, you expect start and end are physical addresses. But in
> phys_xxx_remove() function, I think using virtual addresses is just
> fine. Functions like pmd_addr_end() and pud_index() only calculate
> an offset.
>

Hi Tang,

 

Virtual addresses will work fine, I used physical addresses in order to
keep consistent with phys_pud[pmd/pte]_init(), So I think we should keep this.

Thanks,
Jianguo Wu

> So, would you please tell me if we have to use physical addresses here ?
> 
> Thanks. :)
> 
>> +
>> +    for (; start<  end; start = next) {
>> +        pgd_t *pgd = pgd_offset_k(start);
>> +        pud_t *pud;
>> +
>> +        next = pgd_addr_end(start, end);
>> +
>> +        if (!pgd_present(*pgd))
>> +            continue;
>> +
>> +        pud = map_low_page((pud_t *)pgd_page_vaddr(*pgd));
>> +        phys_pud_remove(pud, __pa(start), __pa(next));
>> +        if (free_pud_table(pud, pgd))
>> +            pgd_changed = true;
>> +        unmap_low_page(pud);
>> +    }
>> +
>> +    if (pgd_changed)
>> +        sync_global_pgds(start, end - 1);
>> +
>> +    flush_tlb_all();
>> +}
>> +
>>   #ifdef CONFIG_MEMORY_HOTREMOVE
>>   int __ref arch_remove_memory(u64 start, u64 size)
>>   {
>> @@ -692,6 +921,8 @@ int __ref arch_remove_memory(u64 start, u64 size)
>>       ret = __remove_pages(zone, start_pfn, nr_pages);
>>       WARN_ON_ONCE(ret);
>>
>> +    kernel_physical_mapping_remove(start, start + size);
>> +
>>       return ret;
>>   }
>>   #endif
> 
> 
> 
> .
> 




^ permalink raw reply

* Re: [PATCH] gpio: pcf857x: use client->irq for gpio_to_irq()
From: Linus Walleij @ 2012-12-07  8:16 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Simon, Linux-SH, ENGELMAYER Christian, linux-kernel
In-Reply-To: <87lidb7dhb.wl%kuninori.morimoto.gx@renesas.com>

On Thu, Dec 6, 2012 at 10:10 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:

> 6e20a0a429bd4dc07d6de16d9c247270e04e4aa0
> (gpio: pcf857x: enable gpio_to_irq() support)
> added gpio_to_irq() support on pcf857x driver,
> but it used pdata->irq.
> This patch modifies driver to use client->irq instead of it.
> It modifies kzm9g board platform settings,
> and device probe information too.
> This patch is tested on kzm9g board
>
> Reported-by: ENGELMAYER Christian Christian.Engelmayer@frequentis.com
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Patch applied with Simon's ACK, thanks!

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v2 00/77] SH pin control and GPIO rework with OF support
From: Laurent Pinchart @ 2012-12-07 18:35 UTC (permalink / raw)
  To: linux-sh
In-Reply-To: <1353974596-30033-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

Hi Linus,

On Thursday 06 December 2012 02:34:39 Laurent Pinchart wrote:
> On Saturday 01 December 2012 23:55:35 Linus Walleij wrote:
> > On Tue, Nov 27, 2012 at 1:01 AM, Laurent Pinchart wrote:
> > > Here's the second version of the SH pin control and GPIO rework patches.
> > > I've added OF support for PFC instantiation and GPIO mappings that was
> > > missing from v1. PINCTRL bindings are still missing and will come soon.
> > 
> > So I've tried the only way I could to review this by cloning your tree
> > and actually inspecting the end result ... overall it's looking very good!
> > 
> > Here are assorted comments:

[snip]

> > - You're using the method to add ranges from the pinctrl side of
> >   things. This is basically deprecated with the changes to gpiolib
> >   I make in this merge window. If you study the way I changed
> >   the pinctrl-u300.c and pinctrl-coh901.c to switch the registration
> >   from being done in the pin controller to being done in the
> >   gpiolib part, you will get the picture. The big upside is that
> >   (A) makes the pin and GPIO references to the local GPIO
> >   chip and pin controller and (B) that this supports adding ranges
> >   from the device tree, which is probably what you want in the
> >   end...
> 
> OK, I will have a look at the code.

Do you have a tree with those patches ?

[snip]

> > - This stuff in setup_data_regs():
> >   rp->reg_shadow = gpio_read_raw_reg(drp->mapped_reg, drp->reg_width);
> >   
> >   You know, I think shadow registers is just another name
> >   for regmap-mmio. Please consult
> >   drivers/base/regmap/regmap-mmio.c and tell me if I'm
> >   wrong. It's not like I'm going to require you to convert this
> >   to regmap from day 1 if this is legacy stuff but it's probably
> >   the same thing.
> 
> I'll have a look at it.

I've considered regmap but I think it's a bit overkill. True, the reg_shadow 
is a different name for regmap-mmio (or rather for a small subset of it), but 
I already have a data structure instance for each register due to other 
requirements of the driver, so storing the cached value there is pretty much 
free.

I might end up reworking the data registers related code in which case I will 
try to use regmap.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules
From: Laurent Pinchart @ 2012-12-10 15:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1350290093-998-2-git-send-email-hdk@igel.co.jp>

Dear Eiraku-san,

On Monday 15 October 2012 17:34:52 Hideki EIRAKU wrote:
> This is the Renesas IPMMU driver and IOMMU API implementation.
> 
> The IPMMU module supports the MMU function and the PMB function.

That sentence make me believe that both MMU and PMB were supported by the 
driver, as "module" often refers to Linux kernel modules in this context. 
Maybe you could replace "module" by "hardware module".

> The MMU function provides address translation by pagetable compatible with
> ARMv6. The PMB function provides address translation including tile-linear
> translation. This patch implements the MMU function.
>
> The iommu driver does not register a platform driver directly because:
> - the register space of the MMU function and the PMB function
>   have a common register (used for settings flush), so they should ideally
>   have a way to appropriately share this register.
> - the MMU function uses the IOMMU API while the PMB function does not.
> - the two functions may be used independently.
> 
> Signed-off-by: Hideki EIRAKU <hdk@igel.co.jp>
> ---
>  arch/arm/mach-shmobile/Kconfig              |    6 +
>  arch/arm/mach-shmobile/Makefile             |    3 +
>  arch/arm/mach-shmobile/include/mach/ipmmu.h |   16 ++
>  arch/arm/mach-shmobile/ipmmu.c              |  150 ++++++++++++
>  drivers/iommu/Kconfig                       |   56 +++++
>  drivers/iommu/Makefile                      |    1 +
>  drivers/iommu/shmobile-iommu.c              |  352 ++++++++++++++++++++++++
>  7 files changed, 584 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-shmobile/include/mach/ipmmu.h
>  create mode 100644 arch/arm/mach-shmobile/ipmmu.c
>  create mode 100644 drivers/iommu/shmobile-iommu.c

What is the reason for splitting the driver in two files ? Can't you put all 
the code in drivers/iommu/shmobile-iommu.c ? Storing driver code in arch/* is 
discouraged.

> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 8ae100c..de69ab3 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -210,6 +210,12 @@ endmenu
>  config SH_CLK_CPG
>  	bool
> 
> +config SHMOBILE_IPMMU
> +	bool
> +
> +config SHMOBILE_IPMMU_TLB
> +	bool
> +
>  source "drivers/sh/Kconfig"
> 
>  endif
> diff --git a/arch/arm/mach-shmobile/Makefile
> b/arch/arm/mach-shmobile/Makefile index fe2c97c..4ffba9d 100644
> --- a/arch/arm/mach-shmobile/Makefile
> +++ b/arch/arm/mach-shmobile/Makefile
> @@ -60,3 +60,6 @@ obj-$(CONFIG_MACH_KZM9G)	+= board-kzm9g.o
>  # Framework support
>  obj-$(CONFIG_SMP)		+= $(smp-y)
>  obj-$(CONFIG_GENERIC_GPIO)	+= $(pfc-y)
> +
> +# IPMMU/IPMMUI
> +obj-$(CONFIG_SHMOBILE_IPMMU)	+= ipmmu.o
> diff --git a/arch/arm/mach-shmobile/include/mach/ipmmu.h
> b/arch/arm/mach-shmobile/include/mach/ipmmu.h new file mode 100644
> index 0000000..ede2f0b
> --- /dev/null
> +++ b/arch/arm/mach-shmobile/include/mach/ipmmu.h
> @@ -0,0 +1,16 @@
> +#ifdef CONFIG_SHMOBILE_IPMMU_TLB
> +void ipmmu_tlb_flush(struct device *ipmmu_dev);
> +void ipmmu_tlb_set(struct device *ipmmu_dev, unsigned long phys, int size,
> +		   int asid);
> +void ipmmu_add_device(struct device *dev);
> +int ipmmu_iommu_init(struct device *dev);
> +#else
> +static inline void ipmmu_add_device(struct device *dev)
> +{
> +}
> +
> +static int ipmmu_iommu_init(struct device *dev)
> +{
> +	return -EINVAL;
> +}
> +#endif
> diff --git a/arch/arm/mach-shmobile/ipmmu.c b/arch/arm/mach-shmobile/ipmmu.c
> new file mode 100644
> index 0000000..72cacb9
> --- /dev/null
> +++ b/arch/arm/mach-shmobile/ipmmu.c
> @@ -0,0 +1,150 @@
> +/*
> + * IPMMU/IPMMUI
> + * Copyright (C) 2012  Hideki EIRAKU
> + *
> + * 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; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
> USA

You can remove this last paragraph, we don't want to patch every file in the 
kernel if the FSF moves to a new building :-)

> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/slab.h>
> +#include <mach/ipmmu.h>
> +
> +#define IMCTR1 0x000
> +#define IMCTR2 0x004
> +#define IMASID 0x010
> +#define IMTTBR 0x014
> +#define IMTTBCR 0x018
> +
> +#define IMCTR1_TLBEN (1 << 0)
> +#define IMCTR1_FLUSH (1 << 1)
> +
> +struct ipmmu_priv {
> +	void __iomem *ipmmu_base;
> +	int tlb_enabled;
> +	struct mutex flush_lock;
> +};
> +
> +static void ipmmu_reg_write(struct ipmmu_priv *priv, unsigned long reg_off,
> +			    unsigned long data)
> +{
> +	iowrite32(data, priv->ipmmu_base + reg_off);
> +}
> +
> +void ipmmu_tlb_flush(struct device *dev)
> +{
> +	struct ipmmu_priv *priv;
> +
> +	if (!dev)
> +		return;
> +	priv = dev_get_drvdata(dev);
> +	mutex_lock(&priv->flush_lock);
> +	if (priv->tlb_enabled)
> +		ipmmu_reg_write(priv, IMCTR1, IMCTR1_FLUSH | IMCTR1_TLBEN);
> +	else
> +		ipmmu_reg_write(priv, IMCTR1, IMCTR1_FLUSH);
> +	mutex_unlock(&priv->flush_lock);
> +}
> +
> +void ipmmu_tlb_set(struct device *dev, unsigned long phys, int size, int
> asid)
> +{
> +	struct ipmmu_priv *priv;
> +
> +	if (!dev)
> +		return;
> +	priv = dev_get_drvdata(dev);
> +	mutex_lock(&priv->flush_lock);
> +	switch (size) {
> +	default:
> +		priv->tlb_enabled = 0;
> +		break;
> +	case 0x2000:
> +		ipmmu_reg_write(priv, IMTTBCR, 1);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x1000:
> +		ipmmu_reg_write(priv, IMTTBCR, 2);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x800:
> +		ipmmu_reg_write(priv, IMTTBCR, 3);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x400:
> +		ipmmu_reg_write(priv, IMTTBCR, 4);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x200:
> +		ipmmu_reg_write(priv, IMTTBCR, 5);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x100:
> +		ipmmu_reg_write(priv, IMTTBCR, 6);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x80:
> +		ipmmu_reg_write(priv, IMTTBCR, 7);
> +		priv->tlb_enabled = 1;
> +		break;
> +	}
> +	ipmmu_reg_write(priv, IMTTBR, phys);
> +	ipmmu_reg_write(priv, IMASID, asid);
> +	mutex_unlock(&priv->flush_lock);
> +}
> +
> +static int __devinit ipmmu_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct ipmmu_priv *priv;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "cannot get platform resources\n");
> +		return -ENOENT;
> +	}
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		dev_err(&pdev->dev, "cannot allocate device data\n");
> +		return -ENOMEM;
> +	}
> +	mutex_init(&priv->flush_lock);
> +	priv->ipmmu_base = ioremap_nocache(res->start, resource_size(res));
> +	if (!priv->ipmmu_base) {
> +		dev_err(&pdev->dev, "ioremap_nocache failed\n");
> +		kfree(priv);
> +		return -ENOMEM;
> +	}
> +	platform_set_drvdata(pdev, priv);
> +	ipmmu_reg_write(priv, IMCTR1, 0x0); /* disable TLB */
> +	ipmmu_reg_write(priv, IMCTR2, 0x0); /* disable PMB */
> +	ipmmu_iommu_init(&pdev->dev);
> +	return 0;
> +}
> +
> +static struct platform_driver ipmmu_driver = {
> +	.probe = ipmmu_probe,
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name = "ipmmu",
> +	},
> +};
> +
> +static int __init ipmmu_init(void)
> +{
> +	return platform_driver_register(&ipmmu_driver);
> +}
> +subsys_initcall(ipmmu_init);
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index e39f9db..265829f 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -187,4 +187,60 @@ config EXYNOS_IOMMU_DEBUG
> 
>  	  Say N unless you need kernel log message for IOMMU debugging
> 
> +config SHMOBILE_IOMMU
> +	bool "IOMMU for Renesas IPMMU/IPMMUI"
> +	default n
> +	select IOMMU_API
> +	select ARM_DMA_USE_IOMMU
> +	select SHMOBILE_IPMMU
> +	select SHMOBILE_IPMMU_TLB
> +
> +choice
> +	prompt "IPMMU/IPMMUI address space size"
> +	default SHMOBILE_IOMMU_ADDRSIZE_2048MB
> +	depends on SHMOBILE_IOMMU
> +	help
> +	  This option sets IPMMU/IPMMUI address space size by
> +	  adjusting the 1st level page table size. The page table size
> +	  is calculated as follows:
> +
> +	      page table size = number of page table entries * 4 bytes
> +	      number of page table entries = address space size / 1 MiB
> +
> +	  For example, when the address space size is 2048 MiB, the
> +	  1st level page table size is 8192 bytes.
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_2048MB
> +		bool "2 GiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_1024MB
> +		bool "1 GiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_512MB
> +		bool "512 MiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_256MB
> +		bool "256 MiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_128MB
> +		bool "128 MiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_64MB
> +		bool "64 MiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_32MB
> +		bool "32 MiB"
> +
> +endchoice
> +
> +config SHMOBILE_IOMMU_L1SIZE
> +	int
> +	default 8192 if SHMOBILE_IOMMU_ADDRSIZE_2048MB
> +	default 4096 if SHMOBILE_IOMMU_ADDRSIZE_1024MB
> +	default 2048 if SHMOBILE_IOMMU_ADDRSIZE_512MB
> +	default 1024 if SHMOBILE_IOMMU_ADDRSIZE_256MB
> +	default 512 if SHMOBILE_IOMMU_ADDRSIZE_128MB
> +	default 256 if SHMOBILE_IOMMU_ADDRSIZE_64MB
> +	default 128 if SHMOBILE_IOMMU_ADDRSIZE_32MB
> +
>  endif # IOMMU_SUPPORT
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 14a4d5f..62cf917 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o
>  obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o
>  obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o
>  obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
> +obj-$(CONFIG_SHMOBILE_IOMMU) += shmobile-iommu.o
> diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
> new file mode 100644
> index 0000000..bbbf1bc
> --- /dev/null
> +++ b/drivers/iommu/shmobile-iommu.c
> @@ -0,0 +1,352 @@
> +/*
> + * IOMMU for IPMMU/IPMMUI
> + * Copyright (C) 2012  Hideki EIRAKU
> + *
> + * 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; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
> USA + *
> + */
> +
> +#include <linux/io.h>
> +#include <linux/dmapool.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/iommu.h>
> +#include <linux/dma-mapping.h>
> +#include <mach/ipmmu.h>
> +#include <asm/dma-iommu.h>
> +
> +#define L1_SIZE CONFIG_SHMOBILE_IOMMU_L1SIZE
> +#define L1_LEN (L1_SIZE / 4)
> +#define L1_ALIGN L1_SIZE
> +#define L2_SIZE 0x400
> +#define L2_LEN (L2_SIZE / 4)
> +#define L2_ALIGN L2_SIZE
> +
> +struct shmobile_iommu_priv_pgtable {
> +	uint32_t *pgtable;
> +	dma_addr_t handle;
> +};
> +
> +struct shmobile_iommu_priv {
> +	struct shmobile_iommu_priv_pgtable l1, l2[L1_LEN];
> +	spinlock_t map_lock;
> +	atomic_t active;
> +};
> +
> +static struct dma_iommu_mapping *iommu_mapping;
> +static struct device *ipmmu_devices;
> +static struct dma_pool *l1pool, *l2pool;
> +static spinlock_t lock;
> +static DEFINE_SPINLOCK(lock_add);
> +static struct shmobile_iommu_priv *attached;
> +static int num_attached_devices;
> +static struct device *ipmmu_access_device;

This many global variables is usually a sign that something is wrong, please 
see below.

> +
> +static int shmobile_iommu_domain_init(struct iommu_domain *domain)
> +{
> +	struct shmobile_iommu_priv *priv;
> +	int i;
> +
> +	priv = kmalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +	priv->l1.pgtable = dma_pool_alloc(l1pool, GFP_KERNEL,
> +					  &priv->l1.handle);
> +	if (!priv->l1.pgtable) {
> +		kfree(priv);
> +		return -ENOMEM;
> +	}
> +	for (i = 0; i < L1_LEN; i++)
> +		priv->l2[i].pgtable = NULL;
> +	memset(priv->l1.pgtable, 0, L1_SIZE);
> +	spin_lock_init(&priv->map_lock);
> +	atomic_set(&priv->active, 0);
> +	domain->priv = priv;
> +	return 0;
> +}
> +
> +static void shmobile_iommu_domain_destroy(struct iommu_domain *domain)
> +{
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +	int i;
> +
> +	for (i = 0; i < L1_LEN; i++) {
> +		if (priv->l2[i].pgtable)
> +			dma_pool_free(l2pool, priv->l2[i].pgtable,
> +				      priv->l2[i].handle);
> +	}
> +	dma_pool_free(l1pool, priv->l1.pgtable, priv->l1.handle);
> +	kfree(priv);
> +	domain->priv = NULL;
> +}
> +
> +static int shmobile_iommu_attach_device(struct iommu_domain *domain,
> +					struct device *dev)
> +{
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +	int ret = -EBUSY;
> +
> +	spin_lock(&lock);
> +	if (attached != priv) {
> +		if (attached)
> +			goto err;
> +		atomic_set(&priv->active, 1);
> +		ipmmu_tlb_set(ipmmu_access_device, priv->l1.handle, L1_SIZE,
> +			      0);
> +		wmb();
> +		ipmmu_tlb_flush(ipmmu_access_device);
> +		attached = priv;
> +		num_attached_devices = 0;
> +	}
> +	num_attached_devices++;
> +	ret = 0;
> +err:
> +	spin_unlock(&lock);
> +	return ret;
> +}
> +
> +static void shmobile_iommu_detach_device(struct iommu_domain *domain,
> +					 struct device *dev)
> +{
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +
> +	spin_lock(&lock);
> +	atomic_set(&priv->active, 0);
> +	num_attached_devices--;
> +	if (!num_attached_devices) {
> +		ipmmu_tlb_set(ipmmu_access_device, 0, 0, 0);
> +		ipmmu_tlb_flush(ipmmu_access_device);
> +		attached = NULL;
> +	}
> +	spin_unlock(&lock);
> +}
> +
> +static int
> +l2alloc(struct shmobile_iommu_priv *priv, unsigned int l1index)
> +{
> +	if (!priv->l2[l1index].pgtable) {
> +		priv->l2[l1index].pgtable = dma_pool_alloc(l2pool, GFP_KERNEL,
> +						&priv->l2[l1index].handle);
> +		if (!priv->l2[l1index].pgtable)
> +			return -ENOMEM;
> +		memset(priv->l2[l1index].pgtable, 0, L2_SIZE);
> +	}
> +	priv->l1.pgtable[l1index] = priv->l2[l1index].handle | 0x1;
> +	return 0;
> +}
> +
> +static void
> +l2realfree(struct shmobile_iommu_priv_pgtable *l2)
> +{
> +	if (l2->pgtable)
> +		dma_pool_free(l2pool, l2->pgtable, l2->handle);
> +}
> +
> +static int
> +l2free(struct shmobile_iommu_priv *priv, unsigned int l1index,
> +	struct shmobile_iommu_priv_pgtable *l2)
> +{
> +	priv->l1.pgtable[l1index] = 0;
> +	if (priv->l2[l1index].pgtable) {
> +		*l2 = priv->l2[l1index];
> +		priv->l2[l1index].pgtable = NULL;
> +	}
> +	return 0;
> +}
> +
> +static int shmobile_iommu_map(struct iommu_domain *domain, unsigned long
> iova, +			      phys_addr_t paddr, size_t size, int prot)
> +{
> +	struct shmobile_iommu_priv_pgtable l2 = { .pgtable = NULL };
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +	unsigned int l1index, l2index, i;
> +	int ret;
> +
> +	l1index = iova >> 20;
> +	switch (size) {
> +	case 0x1000:
> +		l2index = (iova >> 12) & 0xff;
> +		spin_lock(&priv->map_lock);
> +		ret = l2alloc(priv, l1index);
> +		if (!ret)
> +			priv->l2[l1index].pgtable[l2index] = paddr | 0xff2;
> +		spin_unlock(&priv->map_lock);
> +		break;
> +	case 0x10000:
> +		l2index = (iova >> 12) & 0xf0;
> +		spin_lock(&priv->map_lock);
> +		ret = l2alloc(priv, l1index);
> +		if (!ret) {
> +			for (i = 0; i < 0x10; i++)
> +				priv->l2[l1index].pgtable[l2index + i] > +					paddr | 0xff1;
> +		}
> +		spin_unlock(&priv->map_lock);
> +		break;
> +	case 0x100000:
> +		spin_lock(&priv->map_lock);
> +		l2free(priv, l1index, &l2);
> +		priv->l1.pgtable[l1index] = paddr | 0xc02;
> +		spin_unlock(&priv->map_lock);
> +		ret = 0;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +	if (!ret && atomic_read(&priv->active)) {
> +		wmb();
> +		ipmmu_tlb_flush(ipmmu_access_device);
> +		l2realfree(&l2);
> +	}
> +	return ret;
> +}
> +
> +static size_t shmobile_iommu_unmap(struct iommu_domain *domain,
> +				   unsigned long iova, size_t size)
> +{
> +	struct shmobile_iommu_priv_pgtable l2 = { .pgtable = NULL };
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +	unsigned int l1index, l2index, i;
> +	uint32_t l2entry = 0;
> +	size_t ret = 0;
> +
> +	l1index = iova >> 20;
> +	if (!(iova & 0xFFFFF) && size >= 0x100000) {
> +		spin_lock(&priv->map_lock);
> +		l2free(priv, l1index, &l2);
> +		spin_unlock(&priv->map_lock);
> +		ret = 0x100000;
> +		goto done;
> +	}
> +	l2index = (iova >> 12) & 0xff;
> +	spin_lock(&priv->map_lock);
> +	if (priv->l2[l1index].pgtable)
> +		l2entry = priv->l2[l1index].pgtable[l2index];
> +	switch (l2entry & 3) {
> +	case 1:
> +		if (l2index & 0xf)
> +			break;
> +		for (i = 0; i < 0x10; i++)
> +			priv->l2[l1index].pgtable[l2index + i] = 0;
> +		ret = 0x10000;
> +		break;
> +	case 2:
> +		priv->l2[l1index].pgtable[l2index] = 0;
> +		ret = 0x1000;
> +		break;
> +	}
> +	spin_unlock(&priv->map_lock);
> +done:
> +	if (ret && atomic_read(&priv->active)) {
> +		wmb();
> +		ipmmu_tlb_flush(ipmmu_access_device);
> +		l2realfree(&l2);
> +	}
> +	return ret;
> +}
> +
> +static phys_addr_t shmobile_iommu_iova_to_phys(struct iommu_domain *domain,
> +					       unsigned long iova)
> +{
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +	uint32_t l1entry = 0, l2entry = 0;
> +	unsigned int l1index, l2index;
> +
> +	l1index = iova >> 20;
> +	l2index = (iova >> 12) & 0xff;
> +	spin_lock(&priv->map_lock);
> +	if (priv->l2[l1index].pgtable)
> +		l2entry = priv->l2[l1index].pgtable[l2index];
> +	else
> +		l1entry = priv->l1.pgtable[l1index];
> +	spin_unlock(&priv->map_lock);
> +	switch (l2entry & 3) {
> +	case 1:
> +		return (l2entry & ~0xffff) | (iova & 0xffff);
> +	case 2:
> +		return (l2entry & ~0xfff) | (iova & 0xfff);
> +	default:
> +		if ((l1entry & 3) = 2)
> +			return (l1entry & ~0xfffff) | (iova & 0xfffff);
> +		return 0;
> +	}
> +}
> +
> +static struct iommu_ops shmobile_iommu_ops = {
> +	.domain_init = shmobile_iommu_domain_init,
> +	.domain_destroy = shmobile_iommu_domain_destroy,
> +	.attach_dev = shmobile_iommu_attach_device,
> +	.detach_dev = shmobile_iommu_detach_device,
> +	.map = shmobile_iommu_map,
> +	.unmap = shmobile_iommu_unmap,
> +	.iova_to_phys = shmobile_iommu_iova_to_phys,
> +	.pgsize_bitmap = 0x111000,
> +};
> +
> +static int shmobile_iommu_attach_all_devices(void)
> +{
> +	struct device *dev;
> +	int ret = 0;
> +
> +	spin_lock(&lock_add);
> +	iommu_mapping = arm_iommu_create_mapping(&platform_bus_type, 0x0,
> +						 L1_LEN << 20, 0);
> +	if (IS_ERR_OR_NULL(iommu_mapping)) {
> +		ret = PTR_ERR(iommu_mapping);
> +		goto err;
> +	}
> +	for (dev = ipmmu_devices; dev; dev = dev->archdata.iommu) {
> +		if (arm_iommu_attach_device(dev, iommu_mapping))
> +			pr_err("arm_iommu_attach_device failed\n");
> +	}
> +err:
> +	spin_unlock(&lock_add);
> +	return 0;
> +}
> +
> +void ipmmu_add_device(struct device *dev)
> +{
> +	spin_lock(&lock_add);
> +	dev->archdata.iommu = ipmmu_devices;
> +	ipmmu_devices = dev;

That looks a bit hackish to me. I'd like to suggest a different approach, that 
would be compatible with supporting multiple IPMMU instances.

dev->archdata.iommu should point to a new sh_ipmmu_arch_data structure that 
would contain an IPMMU name (const char *) and a pointer to a struct 
shmobile_iommu_priv.

ipmmu_add_device() would take a new IPMMU name argument, allocate an 
sh_ipmmu_arch_data instance dynamically and initialize its name field to the 
name passed to the function. The shmobile_iommu_priv pointer would be set to 
NULL. No other operation would be performed (you will likely get rid of the 
global ipmmu_devices and iommu_mapping variables).

Then, the attach_dev operation handler would retrieve the dev->archdata.iommu 
pointer, cast that to an sh_ipmmu_arch_data, and retrieve the IPMMU associated 
with the name (either by walking a driver-global list of IPMMUs, or by using 
driver_find_device()).

This mechanism would get rid of several global variables in the driver 
(several of them would move to the shmobile_ipmmu_priv structure - which I 
would have named shmobile_ipmmu or even sh_ipmmu, but that's up to you) and 
add support for several IPMMU instances (there's 3 of them in the sh7372, even 
if we only need to support one right now it's still a good practice to design 
the driver in a way that multiple instances can be supported).

Could you try to rework the driiver in that direction ? You can have a look at 
the OMAP IOMMU driver if you need sample code, and obviously feel free to 
contact me if you have any question.

> +	if (!IS_ERR_OR_NULL(iommu_mapping)) {
> +		if (arm_iommu_attach_device(dev, iommu_mapping))
> +			pr_err("arm_iommu_attach_device failed\n");
> +	}
> +	spin_unlock(&lock_add);
> +}
> +
> +int ipmmu_iommu_init(struct device *dev)
> +{
> +	dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> +	l1pool = dma_pool_create("shmobile-iommu-pgtable1", dev,
> +				 L1_SIZE, L1_ALIGN, 0);
> +	if (!l1pool)
> +		goto nomem_pool1;
> +	l2pool = dma_pool_create("shmobile-iommu-pgtable2", dev,
> +				 L2_SIZE, L2_ALIGN, 0);
> +	if (!l2pool)
> +		goto nomem_pool2;
> +	spin_lock_init(&lock);
> +	attached = NULL;
> +	ipmmu_access_device = dev;
> +	bus_set_iommu(&platform_bus_type, &shmobile_iommu_ops);
> +	if (shmobile_iommu_attach_all_devices())
> +		pr_err("shmobile_iommu_attach_all_devices failed\n");
> +	return 0;
> +nomem_pool2:
> +	dma_pool_destroy(l1pool);
> +nomem_pool1:
> +	return -ENOMEM;
> +}

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [PATCH 0/6 v3] ARM: mach-shmobile: kzm9g: Reference DT implementation
From: Simon Horman @ 2012-12-11  3:07 UTC (permalink / raw)
  To: linux-arm-kernel

this is a third pass at at providing a refernce DT implementation
for the kzm9g baord which is intended to act as a guide for mach-shmobile
developers.

It provides a minimal setup of the board using as little C code as
possible.

It may be used by selecting CONFIG_MACH_KZM9G_REFERENCE
and using sh73a0-kzm9g-reference.dtb

*** Please note that the clock initialisation shcheme used in
    this series does not currently work with SMP as there
    is a yet to be resolved lock-up in workqueue initialisation.

    Resolving this problem is currently regarded as future-work.

    CONFIG_SMP must be disabled when using this code. ***

Major difference between v2 and v3:
* Allow UP boot to complete by initialiing CMT earlier.
  This is achieved by changing the position of clocsources/
  in drivers/Makefile

Major difference between v1 and v2:
* Drop touchscreen initialisation, it requires INTC
* Drop SDHI initialisation, the driver seems to need some work
* Add extra entries to clock-sh73a0.c:lookups[] and
  remove auxdata. Ultimtely the clocks will be described
  in DT, but the code isn't there yet.
* Drop MMCIF patch, it is not needed if regulators are enabled
* Drop PCF hacks, instead, the series is based on Laruent Pinchart's *
* series: [PATCH v2 00/77] SH pin control and GPIO rework with OF support
* Use shmobile_setup_delay()

Git and diffstat information provided to aid review.

----------------------------------------------------------------
The following changes since commit 940956fe1d8de5cbcc963214fe592f76d8354bef:

  ARM: shmobile: kzm9g: Add LED1-LED4 to the device tree (2012-11-27 00:59:10 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git devel/of-kzm9g

for you to fetch changes up to e64ad872028f921a7a30be325fbdecc725958b40:

  ARM: mach-shmobile: kzm9g: Reference DT implementation (2012-12-11 11:51:56 +0900)

----------------------------------------------------------------
Simon Horman (6):
      ARM: clocksource: Initialise early
      ARM: mach-shmobile: sh73a0: Allow initialisation of GIC by DT
      ARM: mach-shmobile: sh73a0: Minimal setup using DT
      ARM: mach-shmobile: sh73a0: Initialise MMCIF using DT
      ARM: mach-shmobile: kzm9g: use voltage regulators by default
      ARM: mach-shmobile: kzm9g: Reference DT implementation

 arch/arm/boot/dts/Makefile                     |    3 +-
 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts   |   37 +++++++++
 arch/arm/boot/dts/sh73a0-reference.dtsi        |   24 ++++++
 arch/arm/boot/dts/sh73a0.dtsi                  |   69 +++++++++++++++++
 arch/arm/configs/kzm9g_defconfig               |    2 +
 arch/arm/mach-shmobile/Kconfig                 |   10 +++
 arch/arm/mach-shmobile/Makefile                |    1 +
 arch/arm/mach-shmobile/board-kzm9g-reference.c |   99 ++++++++++++++++++++++++
 arch/arm/mach-shmobile/clock-sh73a0.c          |   13 ++++
 arch/arm/mach-shmobile/include/mach/common.h   |    3 +
 arch/arm/mach-shmobile/intc-sh73a0.c           |   14 ++++
 arch/arm/mach-shmobile/setup-sh73a0.c          |   68 +++++++++++++++-
 drivers/Makefile                               |    8 +-
 13 files changed, 346 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
 create mode 100644 arch/arm/boot/dts/sh73a0-reference.dtsi
 create mode 100644 arch/arm/mach-shmobile/board-kzm9g-reference.c

^ permalink raw reply

* [PATCH 1/6] ARM: clocksource: Initialise early
From: Simon Horman @ 2012-12-11  3:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1355195254-4563-1-git-send-email-horms+renesas@verge.net.au>

Initialise clock sources early, since some subsystems rely on them to initialize

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

---

v3
* Initial post
---
 drivers/Makefile |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/Makefile b/drivers/Makefile
index a16a8d0..eba9b58 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -37,6 +37,11 @@ obj-$(CONFIG_XEN)		+= xen/
 # regulators early, since some subsystems rely on them to initialize
 obj-$(CONFIG_REGULATOR)		+= regulator/
 
+# clocksource early, since some subsystems rely on them to initialize
+ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
+obj-y				+= clocksource/
+endif
+
 # tty/ comes before char/ so that the VT console is the boot-time
 # default.
 obj-y				+= tty/
@@ -114,9 +119,6 @@ obj-y				+= firmware/
 obj-$(CONFIG_CRYPTO)		+= crypto/
 obj-$(CONFIG_SUPERH)		+= sh/
 obj-$(CONFIG_ARCH_SHMOBILE)	+= sh/
-ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
-obj-y				+= clocksource/
-endif
 obj-$(CONFIG_DCA)		+= dca/
 obj-$(CONFIG_HID)		+= hid/
 obj-$(CONFIG_PPC_PS3)		+= ps3/
-- 
1.7.10.4


^ permalink raw reply related


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