LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v5 00/45] CPU hotplug: stop_machine()-free CPU hotplug
From: Srivatsa S. Bhat @ 2013-02-15 19:40 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: linux-doc, peterz, fweisbec, linux-kernel, walken, mingo,
	linux-arch, Russell King - ARM Linux, xiaoguangrong, wangyun,
	paulmck, nikunj, linux-pm, Rusty Russell, rostedt, rjw, namhyung,
	tglx, linux-arm-kernel, netdev, oleg, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <CAKfTPtAo2hQTfBKTVUuLKgsGJ2ZLD0UTR3fH9UFuJwyFt4n__w@mail.gmail.com>

Hi Vincent,

On 02/15/2013 06:58 PM, Vincent Guittot wrote:
> Hi Srivatsa,
> 
> I have run some tests with you branch (thanks Paul for the git tree)
> and you will find results below.
>

Thank you very much for testing this patchset!
 
> The tests condition are:
> - 5 CPUs system in 2 clusters
> - The test plugs/unplugs CPU2 and it increases the system load each 20
> plug/unplug sequence with either more cyclictests threads
> - The test is done with all CPUs online and with only CPU0 and CPU2
> 
> The main conclusion is that there is no differences with and without
> your patches with my stress tests. I'm not sure that it was the
> expected results but the cpu_down is already quite low : 4-5ms in
> average
> 

Atleast my patchset doesn't perform _worse_ than mainline, with respect
to cpu_down duration :-)

So, here is the analysis:
Stop-machine() doesn't really slow down CPU-down operation, if the rest
of the CPUs are mostly running in userspace all the time. Because, the
CPUs running userspace workloads cooperate very eagerly with the stop-machine
dance - they receive the resched IPI, and allow the per-cpu cpu-stopper
thread to monopolize the CPU, almost immediately.

The scenario where stop-machine() takes longer to take effect is when
most of the online CPUs are running in kernelspace, because, then the
probability that they call preempt_disable() frequently (and hence inhibit
stop-machine) is higher. That's why, in my tests, I ran genload from LTP
which generated a lot of system-time (system-time in 'top' indicates activity
in kernelspace). Hence my patchset showed significant improvement over
mainline in my tests.

However, your test is very useful too, if we measure a different parameter:
the latency impact on the workloads running on the system (cyclic test).
One other important aim of this patchset is to make hotplug as less intrusive
as possible, for other workloads running on the system. So if you measure
the cyclictest numbers, I would expect my patchset to show better numbers
than mainline, when you do cpu-hotplug in parallel (same test that you did).
Mainline would run stop-machine and hence interrupt the cyclic test tasks
too often. My patchset wouldn't do that, and hence cyclic test should
ideally show better numbers.

I'd really appreciate if you could try that out and let me know how it
goes.. :-) Thank you very much!

Regards,
Srivatsa S. Bhat

> 
> 
> On 12 February 2013 04:58, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>> On 02/12/2013 12:38 AM, Paul E. McKenney wrote:
>>> On Mon, Feb 11, 2013 at 05:53:41PM +0530, Srivatsa S. Bhat wrote:
>>>> On 02/11/2013 05:28 PM, Vincent Guittot wrote:
>>>>> On 8 February 2013 19:09, Srivatsa S. Bhat
>>>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>
>>> [ . . . ]
>>>
>>>>>> Adding Vincent to CC, who had previously evaluated the performance and
>>>>>> latency implications of CPU hotplug on ARM platforms, IIRC.
>>>>>>
>>>>>
>>>>> Hi Srivatsa,
>>>>>
>>>>> I can try to run some of our stress tests on your patches.
>>>>
>>>> Great!
>>>>
>>>>> Have you
>>>>> got a git tree that i can pull ?
>>>>>
>>>>
>>>> Unfortunately, no, none at the moment..  :-(
>>>
>>> You do need to create an externally visible git tree.
>>
>> Ok, I'll do that soon.
>>
>>>  In the meantime,
>>> I have added your series at rcu/bhat.2013.01.21a on -rcu:
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git
>>>
>>> This should appear soon on a kernel.org mirror near you.  ;-)
>>>
>>
>> Thank you very much, Paul! :-)
>>
>> Regards,
>> Srivatsa S. Bhat
>>

^ permalink raw reply

* Re: [PATCH 8/15] arch/powerpc/platforms/85xx/p1022_ds.c: adjust duplicate test
From: Kumar Gala @ 2013-02-15 20:02 UTC (permalink / raw)
  To: Julia Lawall; +Cc: kernel-janitors, linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <1358773378-4700-9-git-send-email-Julia.Lawall@lip6.fr>


On Jan 21, 2013, at 7:02 AM, Julia Lawall wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  The code tested the result
> of a previous call, that itself was already tested.  It is changed to test
> the result of the most recent call.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
>   return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
>   return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> arch/powerpc/platforms/85xx/p1022_ds.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

applied to next

- k

^ permalink raw reply

* Re: [PATCH] [v2] powerpc/fsl: remove extraneous DIU platform functions
From: Gala Kumar-B11780 @ 2013-02-15 20:05 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <1358465214-18746-1-git-send-email-timur@tabi.org>


On Jan 17, 2013, at 5:26 PM, Timur Tabi wrote:

> From: Timur Tabi <timur@freescale.com>
>=20
> The Freescale DIU driver was recently updated to not require every DIU
> platform function, so now we can remove the unneeded functions from
> some boards.
>=20
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
> arch/powerpc/platforms/512x/mpc512x_shared.c |    5 ---
> arch/powerpc/platforms/85xx/p1022_ds.c       |   38 ---------------------=
-----
> arch/powerpc/platforms/85xx/p1022_rdk.c      |   12 --------
> 3 files changed, 0 insertions(+), 55 deletions(-)

applied to next

- k=

^ permalink raw reply

* Re: [PATCH v3 1/1] powerpc/85xx: Board support for ppa8548
From: Kumar Gala @ 2013-02-15 20:03 UTC (permalink / raw)
  To: Stef van Os; +Cc: scottwood, Paul Mackerras, linuxppc-dev, timur.tabi
In-Reply-To: <1360764540-4185-1-git-send-email-stef.van.os@prodrive.nl>


On Feb 13, 2013, at 8:09 AM, Stef van Os wrote:

> Initial board support for the Prodrive PPA8548 AMC module. Board
> is an MPC8548 AMC platform used in RapidIO systems. This module is
> also used to test/work on mainline linux RapidIO software.
>=20
> PPA8548 overview:
> - 1.3 GHz Freescale PowerQUICC III MPC8548 processor
> - 1 GB DDR2 @ 266 MHz
> - 8 MB NOR flash
> - Serial RapidIO 1.2
> - 1 x 10/100/1000 BASE-T front ethernet
> - 1 x 1000 BASE-BX ethernet on AMC connector
>=20
> Signed-off-by: Stef van Os <stef.van.os@prodrive.nl>
> ---
> arch/powerpc/boot/dts/ppa8548.dts           |  166 =
+++++++++++++++++++++++++++
> arch/powerpc/configs/85xx/ppa8548_defconfig |   65 +++++++++++
> arch/powerpc/platforms/85xx/Kconfig         |    7 +
> arch/powerpc/platforms/85xx/Makefile        |    1 +
> arch/powerpc/platforms/85xx/ppa8548.c       |   98 ++++++++++++++++
> 5 files changed, 337 insertions(+), 0 deletions(-)
> create mode 100644 arch/powerpc/boot/dts/ppa8548.dts
> create mode 100644 arch/powerpc/configs/85xx/ppa8548_defconfig
> create mode 100644 arch/powerpc/platforms/85xx/ppa8548.c

applied to next

- k=

^ permalink raw reply

* Re: [linuxppc-dev][PATCH] powerpc/fsl_pci: Store the pci controller device pointer in the pci controller structure.
From: Kumar Gala @ 2013-02-15 20:14 UTC (permalink / raw)
  To: Varun Sethi; +Cc: scottwood, linuxppc-dev
In-Reply-To: <1358162880-16012-1-git-send-email-Varun.Sethi@freescale.com>


On Jan 14, 2013, at 5:28 AM, Varun Sethi wrote:

> The pci controller structure has a provision to store the device =
strcuture
> pointer of the corresponding platform device. Currently this =
information is
> not stored during fsl pci controller initialization. This information =
is
> required while dealing with iommu groups for pci devices connected to =
the fsl
> pci controller. For the case where the pci devices can't be =
paritioned, they=20
> would fall under the same device group as the pci controller.
>=20
> This patch stores the platform device information in the pci =
controller
> structure during initialization.
>=20
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
> arch/powerpc/sysdev/fsl_pci.c |    9 +++++++--
> arch/powerpc/sysdev/fsl_pci.h |    2 +-
> 2 files changed, 8 insertions(+), 3 deletions(-)

applied to next

- k=

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/mpic: allow coreint to be determined by MPIC version
From: Kumar Gala @ 2013-02-15 20:14 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <1358819804-28665-1-git-send-email-scottwood@freescale.com>


On Jan 21, 2013, at 7:56 PM, Scott Wood wrote:

> This will be used by the qemu-e500 platform, as the MPIC version (and
> thus whether we have coreint) depends on how QEMU is configured.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> arch/powerpc/sysdev/mpic.c |   26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)

applied to next

- k

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/e500/qemu-e500: enable coreint
From: Kumar Gala @ 2013-02-15 20:14 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <1358819804-28665-3-git-send-email-scottwood@freescale.com>


On Jan 21, 2013, at 7:56 PM, Scott Wood wrote:

> The MPIC code will disable coreint if it detects an insufficient
> MPIC version.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> arch/powerpc/platforms/85xx/qemu_e500.c |    7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)

applied to next

- k

^ permalink raw reply

* [PATCH] i2c: Remove unneeded xxx_set_drvdata(..., NULL) calls
From: Doug Anderson @ 2013-02-15 23:18 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Tony Lindgren, Linus Walleij, Thierry Reding, Sekhar Nori,
	linux-kernel, linux-i2c, Guan Xuetao, Kevin Hilman, Sonic Zhang,
	linux-arm-kernel, Deepak Sikri, Havard Skinnemoen, Marek Vasut,
	Pawel Moll, Stephen Warren, Sascha Hauer, Uwe Kleine-König,
	Rob Herring, uclinux-dist-devel, Jean Delvare, Lars-Peter Clausen,
	Ben Dooks (embedded platforms), Barry Song, linux-omap,
	Mika Westerberg, Oskar Schirmer, Fabio Estevam,
	davinci-linux-open-source, Shawn Guo, Jim Cromie,
	Greg Kroah-Hartman, Tomoya MORINAGA, Doug Anderson, Kyungmin Park,
	Viresh Kumar, Karol Lewandowski, Jiri Kosina, STEricsson,
	Joe Perches, Andrew Morton, Alessandro Rubini, linuxppc-dev,
	Alexander Stein
In-Reply-To: <1360953682-25066-1-git-send-email-dianders@chromium.org>

There is simply no reason to be manually setting the private driver
data to NULL in the remove/fail to probe cases.  This is just extra
cruft code that can be removed.

A few notes:
* Nothing relies on drvdata being set to NULL.
* The __device_release_driver() function eventually calls
  dev_set_drvdata(dev, NULL) anyway, so there's no need to do it
  twice.
* I verified that there were no cases where xxx_get_drvdata() was
  being called in these drivers and checking for / relying on the NULL
  return value.

This could be cleaned up kernel-wide but for now just take the baby
step and remove from the i2c subsystem.

Reported-by: Wolfram Sang <wsa@the-dreams.de>
Reported-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/i2c/busses/i2c-au1550.c             | 1 -
 drivers/i2c/busses/i2c-bfin-twi.c           | 2 --
 drivers/i2c/busses/i2c-cpm.c                | 2 --
 drivers/i2c/busses/i2c-davinci.c            | 2 --
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 2 --
 drivers/i2c/busses/i2c-designware-platdrv.c | 2 --
 drivers/i2c/busses/i2c-eg20t.c              | 2 --
 drivers/i2c/busses/i2c-highlander.c         | 4 ----
 drivers/i2c/busses/i2c-i801.c               | 1 -
 drivers/i2c/busses/i2c-ibm_iic.c            | 3 ---
 drivers/i2c/busses/i2c-imx.c                | 1 -
 drivers/i2c/busses/i2c-intel-mid.c          | 2 --
 drivers/i2c/busses/i2c-iop3xx.c             | 2 --
 drivers/i2c/busses/i2c-mpc.c                | 2 --
 drivers/i2c/busses/i2c-mxs.c                | 2 --
 drivers/i2c/busses/i2c-nomadik.c            | 2 --
 drivers/i2c/busses/i2c-ocores.c             | 1 -
 drivers/i2c/busses/i2c-octeon.c             | 5 +----
 drivers/i2c/busses/i2c-omap.c               | 3 ---
 drivers/i2c/busses/i2c-pca-platform.c       | 1 -
 drivers/i2c/busses/i2c-pmcmsp.c             | 2 --
 drivers/i2c/busses/i2c-pnx.c                | 2 --
 drivers/i2c/busses/i2c-powermac.c           | 1 -
 drivers/i2c/busses/i2c-puv3.c               | 2 --
 drivers/i2c/busses/i2c-pxa-pci.c            | 2 --
 drivers/i2c/busses/i2c-pxa.c                | 2 --
 drivers/i2c/busses/i2c-s6000.c              | 1 -
 drivers/i2c/busses/i2c-sh7760.c             | 1 -
 drivers/i2c/busses/i2c-stu300.c             | 1 -
 drivers/i2c/busses/i2c-taos-evm.c           | 2 --
 drivers/i2c/busses/i2c-versatile.c          | 2 --
 drivers/i2c/busses/i2c-xiic.c               | 2 --
 drivers/i2c/busses/i2c-xlr.c                | 1 -
 drivers/i2c/busses/scx200_acb.c             | 1 -
 drivers/i2c/muxes/i2c-mux-gpio.c            | 1 -
 35 files changed, 1 insertion(+), 64 deletions(-)

diff --git a/drivers/i2c/busses/i2c-au1550.c b/drivers/i2c/busses/i2c-au1550.c
index b278298..b5b8923 100644
--- a/drivers/i2c/busses/i2c-au1550.c
+++ b/drivers/i2c/busses/i2c-au1550.c
@@ -376,7 +376,6 @@ static int i2c_au1550_remove(struct platform_device *pdev)
 {
 	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
 
-	platform_set_drvdata(pdev, NULL);
 	i2c_del_adapter(&priv->adap);
 	i2c_au1550_disable(priv);
 	iounmap(priv->psc_base);
diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index 0cf780f..05080c4 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -724,8 +724,6 @@ static int i2c_bfin_twi_remove(struct platform_device *pdev)
 {
 	struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
 
-	platform_set_drvdata(pdev, NULL);
-
 	i2c_del_adapter(&(iface->adap));
 	free_irq(iface->irq, iface);
 	peripheral_free_list((unsigned short *)pdev->dev.platform_data);
diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index 2e79c10..3823623 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -682,7 +682,6 @@ static int cpm_i2c_probe(struct platform_device *ofdev)
 out_shut:
 	cpm_i2c_shutdown(cpm);
 out_free:
-	dev_set_drvdata(&ofdev->dev, NULL);
 	kfree(cpm);
 
 	return result;
@@ -696,7 +695,6 @@ static int cpm_i2c_remove(struct platform_device *ofdev)
 
 	cpm_i2c_shutdown(cpm);
 
-	dev_set_drvdata(&ofdev->dev, NULL);
 	kfree(cpm);
 
 	return 0;
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 6a0a553..7d1e590 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -755,7 +755,6 @@ err_mem_ioremap:
 	clk_put(dev->clk);
 	dev->clk = NULL;
 err_free_mem:
-	platform_set_drvdata(pdev, NULL);
 	put_device(&pdev->dev);
 	kfree(dev);
 err_release_region:
@@ -771,7 +770,6 @@ static int davinci_i2c_remove(struct platform_device *pdev)
 
 	i2c_davinci_cpufreq_deregister(dev);
 
-	platform_set_drvdata(pdev, NULL);
 	i2c_del_adapter(&dev->adapter);
 	put_device(&pdev->dev);
 
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 6add851..7c5e383 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -319,7 +319,6 @@ err_free_irq:
 	free_irq(pdev->irq, dev);
 err_iounmap:
 	iounmap(dev->base);
-	pci_set_drvdata(pdev, NULL);
 	put_device(&pdev->dev);
 	kfree(dev);
 err_release_region:
@@ -336,7 +335,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
 	pm_runtime_forbid(&pdev->dev);
 	pm_runtime_get_noresume(&pdev->dev);
 
-	pci_set_drvdata(pdev, NULL);
 	i2c_del_adapter(&dev->adapter);
 	put_device(&pdev->dev);
 
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d2a33e9..0ceb6e1 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -207,7 +207,6 @@ err_unuse_clocks:
 	clk_put(dev->clk);
 	dev->clk = NULL;
 err_free_mem:
-	platform_set_drvdata(pdev, NULL);
 	put_device(&pdev->dev);
 	kfree(dev);
 err_release_region:
@@ -221,7 +220,6 @@ static int dw_i2c_remove(struct platform_device *pdev)
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
 	struct resource *mem;
 
-	platform_set_drvdata(pdev, NULL);
 	pm_runtime_get_sync(&pdev->dev);
 
 	i2c_del_adapter(&dev->adapter);
diff --git a/drivers/i2c/busses/i2c-eg20t.c b/drivers/i2c/busses/i2c-eg20t.c
index 5e7886e..0f37529 100644
--- a/drivers/i2c/busses/i2c-eg20t.c
+++ b/drivers/i2c/busses/i2c-eg20t.c
@@ -869,8 +869,6 @@ static void pch_i2c_remove(struct pci_dev *pdev)
 	for (i = 0; i < adap_info->ch_num; i++)
 		adap_info->pch_data[i].pch_base_address = NULL;
 
-	pci_set_drvdata(pdev, NULL);
-
 	pci_release_regions(pdev);
 
 	pci_disable_device(pdev);
diff --git a/drivers/i2c/busses/i2c-highlander.c b/drivers/i2c/busses/i2c-highlander.c
index 3351cc7..436b0f2 100644
--- a/drivers/i2c/busses/i2c-highlander.c
+++ b/drivers/i2c/busses/i2c-highlander.c
@@ -436,8 +436,6 @@ err_unmap:
 err:
 	kfree(dev);
 
-	platform_set_drvdata(pdev, NULL);
-
 	return ret;
 }
 
@@ -453,8 +451,6 @@ static int highlander_i2c_remove(struct platform_device *pdev)
 	iounmap(dev->base);
 	kfree(dev);
 
-	platform_set_drvdata(pdev, NULL);
-
 	return 0;
 }
 
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index b00c29d..38e13cd 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1239,7 +1239,6 @@ static void i801_remove(struct pci_dev *dev)
 		free_irq(dev->irq, priv);
 	pci_release_region(dev, SMBBAR);
 
-	pci_set_drvdata(dev, NULL);
 	kfree(priv);
 	/*
 	 * do not call pci_disable_device(dev) since it can cause hard hangs on
diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 33a2abb..405a2e2 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -773,7 +773,6 @@ error_cleanup:
 	if (dev->vaddr)
 		iounmap(dev->vaddr);
 
-	dev_set_drvdata(&ofdev->dev, NULL);
 	kfree(dev);
 	return ret;
 }
@@ -785,8 +784,6 @@ static int iic_remove(struct platform_device *ofdev)
 {
 	struct ibm_iic_private *dev = dev_get_drvdata(&ofdev->dev);
 
-	dev_set_drvdata(&ofdev->dev, NULL);
-
 	i2c_del_adapter(&dev->adap);
 
 	if (dev->irq) {
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index a71ece6..82f20c6 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -605,7 +605,6 @@ static int __exit i2c_imx_remove(struct platform_device *pdev)
 	/* remove adapter */
 	dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n");
 	i2c_del_adapter(&i2c_imx->adapter);
-	platform_set_drvdata(pdev, NULL);
 
 	/* setup chip registers to defaults */
 	writeb(0, i2c_imx->base + IMX_I2C_IADR);
diff --git a/drivers/i2c/busses/i2c-intel-mid.c b/drivers/i2c/busses/i2c-intel-mid.c
index de3736b..323fa01 100644
--- a/drivers/i2c/busses/i2c-intel-mid.c
+++ b/drivers/i2c/busses/i2c-intel-mid.c
@@ -1069,7 +1069,6 @@ static int intel_mid_i2c_probe(struct pci_dev *dev,
 fail3:
 	free_irq(dev->irq, mrst);
 fail2:
-	pci_set_drvdata(dev, NULL);
 	kfree(mrst);
 fail1:
 	iounmap(base);
@@ -1087,7 +1086,6 @@ static void intel_mid_i2c_remove(struct pci_dev *dev)
 		dev_err(&dev->dev, "Failed to delete i2c adapter");
 
 	free_irq(dev->irq, mrst);
-	pci_set_drvdata(dev, NULL);
 	iounmap(mrst->base);
 	kfree(mrst);
 	pci_release_region(dev, 0);
diff --git a/drivers/i2c/busses/i2c-iop3xx.c b/drivers/i2c/busses/i2c-iop3xx.c
index 2f99613..bc99333 100644
--- a/drivers/i2c/busses/i2c-iop3xx.c
+++ b/drivers/i2c/busses/i2c-iop3xx.c
@@ -415,8 +415,6 @@ iop3xx_i2c_remove(struct platform_device *pdev)
 	kfree(adapter_data);
 	kfree(padapter);
 
-	platform_set_drvdata(pdev, NULL);
-
 	return 0;
 }
 
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index a69459e..5e705ee 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -696,7 +696,6 @@ static int fsl_i2c_probe(struct platform_device *op)
 	return result;
 
  fail_add:
-	dev_set_drvdata(&op->dev, NULL);
 	free_irq(i2c->irq, i2c);
  fail_request:
 	irq_dispose_mapping(i2c->irq);
@@ -711,7 +710,6 @@ static int fsl_i2c_remove(struct platform_device *op)
 	struct mpc_i2c *i2c = dev_get_drvdata(&op->dev);
 
 	i2c_del_adapter(&i2c->adap);
-	dev_set_drvdata(&op->dev, NULL);
 
 	if (i2c->irq)
 		free_irq(i2c->irq, i2c);
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 22d8ad3..120f246 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -697,8 +697,6 @@ static int mxs_i2c_remove(struct platform_device *pdev)
 
 	writel(MXS_I2C_CTRL0_SFTRST, i2c->regs + MXS_I2C_CTRL0_SET);
 
-	platform_set_drvdata(pdev, NULL);
-
 	return 0;
 }
 
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 5b1b194..650293f 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -1105,7 +1105,6 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
  err_irq:
 	iounmap(dev->virtbase);
  err_no_ioremap:
-	amba_set_drvdata(adev, NULL);
 	kfree(dev);
  err_pinctrl:
  err_no_mem:
@@ -1130,7 +1129,6 @@ static int nmk_i2c_remove(struct amba_device *adev)
 		release_mem_region(res->start, resource_size(res));
 	clk_put(dev->clk);
 	pm_runtime_disable(&adev->dev);
-	amba_set_drvdata(adev, NULL);
 	kfree(dev);
 
 	return 0;
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index a337d08..45150e3 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -452,7 +452,6 @@ static int ocores_i2c_remove(struct platform_device *pdev)
 
 	/* remove adapter & data */
 	i2c_del_adapter(&i2c->adap);
-	platform_set_drvdata(pdev, NULL);
 
 	return 0;
 }
diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c
index 484ca77..935585e 100644
--- a/drivers/i2c/busses/i2c-octeon.c
+++ b/drivers/i2c/busses/i2c-octeon.c
@@ -595,7 +595,7 @@ static int octeon_i2c_probe(struct platform_device *pdev)
 	result = i2c_add_adapter(&i2c->adap);
 	if (result < 0) {
 		dev_err(i2c->dev, "failed to add adapter\n");
-		goto fail_add;
+		goto out;
 	}
 	dev_info(i2c->dev, "version %s\n", DRV_VERSION);
 
@@ -603,8 +603,6 @@ static int octeon_i2c_probe(struct platform_device *pdev)
 
 	return 0;
 
-fail_add:
-	platform_set_drvdata(pdev, NULL);
 out:
 	return result;
 };
@@ -614,7 +612,6 @@ static int octeon_i2c_remove(struct platform_device *pdev)
 	struct octeon_i2c *i2c = platform_get_drvdata(pdev);
 
 	i2c_del_adapter(&i2c->adap);
-	platform_set_drvdata(pdev, NULL);
 	return 0;
 };
 
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 3ee1886..e02f9e3 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1260,7 +1260,6 @@ err_unuse_clocks:
 	pm_runtime_put(dev->dev);
 	pm_runtime_disable(&pdev->dev);
 err_free_mem:
-	platform_set_drvdata(pdev, NULL);
 
 	return r;
 }
@@ -1270,8 +1269,6 @@ static int omap_i2c_remove(struct platform_device *pdev)
 	struct omap_i2c_dev	*dev = platform_get_drvdata(pdev);
 	int ret;
 
-	platform_set_drvdata(pdev, NULL);
-
 	i2c_del_adapter(&dev->adapter);
 	ret = pm_runtime_get_sync(&pdev->dev);
 	if (IS_ERR_VALUE(ret))
diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c
index a30d2f6..aa00df1 100644
--- a/drivers/i2c/busses/i2c-pca-platform.c
+++ b/drivers/i2c/busses/i2c-pca-platform.c
@@ -260,7 +260,6 @@ e_print:
 static int i2c_pca_pf_remove(struct platform_device *pdev)
 {
 	struct i2c_pca_pf_data *i2c = platform_get_drvdata(pdev);
-	platform_set_drvdata(pdev, NULL);
 
 	i2c_del_adapter(&i2c->adap);
 
diff --git a/drivers/i2c/busses/i2c-pmcmsp.c b/drivers/i2c/busses/i2c-pmcmsp.c
index 083d68c..f6389e2 100644
--- a/drivers/i2c/busses/i2c-pmcmsp.c
+++ b/drivers/i2c/busses/i2c-pmcmsp.c
@@ -349,7 +349,6 @@ static int pmcmsptwi_probe(struct platform_device *pldev)
 	return 0;
 
 ret_unmap:
-	platform_set_drvdata(pldev, NULL);
 	if (pmcmsptwi_data.irq) {
 		pmcmsptwi_writel(0,
 			pmcmsptwi_data.iobase + MSP_TWI_INT_MSK_REG_OFFSET);
@@ -374,7 +373,6 @@ static int pmcmsptwi_remove(struct platform_device *pldev)
 
 	i2c_del_adapter(&pmcmsptwi_adapter);
 
-	platform_set_drvdata(pldev, NULL);
 	if (pmcmsptwi_data.irq) {
 		pmcmsptwi_writel(0,
 			pmcmsptwi_data.iobase + MSP_TWI_INT_MSK_REG_OFFSET);
diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index ce40970..5f39c6d 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -761,7 +761,6 @@ out_clkget:
 out_drvdata:
 	kfree(alg_data);
 err_kzalloc:
-	platform_set_drvdata(pdev, NULL);
 	return ret;
 }
 
@@ -776,7 +775,6 @@ static int i2c_pnx_remove(struct platform_device *pdev)
 	release_mem_region(alg_data->base, I2C_PNX_REGION_SIZE);
 	clk_put(alg_data->clk);
 	kfree(alg_data);
-	platform_set_drvdata(pdev, NULL);
 
 	return 0;
 }
diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c
index 0dd5b33..da54e67 100644
--- a/drivers/i2c/busses/i2c-powermac.c
+++ b/drivers/i2c/busses/i2c-powermac.c
@@ -221,7 +221,6 @@ static int i2c_powermac_remove(struct platform_device *dev)
 		printk(KERN_WARNING
 		       "i2c-powermac.c: Failed to remove bus %s !\n",
 		       adapter->name);
-	platform_set_drvdata(dev, NULL);
 	memset(adapter, 0, sizeof(*adapter));
 
 	return 0;
diff --git a/drivers/i2c/busses/i2c-puv3.c b/drivers/i2c/busses/i2c-puv3.c
index d7c512d..261d7db 100644
--- a/drivers/i2c/busses/i2c-puv3.c
+++ b/drivers/i2c/busses/i2c-puv3.c
@@ -223,7 +223,6 @@ static int puv3_i2c_probe(struct platform_device *pdev)
 	return 0;
 
 fail_add_adapter:
-	platform_set_drvdata(pdev, NULL);
 	kfree(adapter);
 fail_nomem:
 	release_mem_region(mem->start, resource_size(mem));
@@ -245,7 +244,6 @@ static int puv3_i2c_remove(struct platform_device *pdev)
 	}
 
 	put_device(&pdev->dev);
-	platform_set_drvdata(pdev, NULL);
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
diff --git a/drivers/i2c/busses/i2c-pxa-pci.c b/drivers/i2c/busses/i2c-pxa-pci.c
index 3d49856..9639be8 100644
--- a/drivers/i2c/busses/i2c-pxa-pci.c
+++ b/drivers/i2c/busses/i2c-pxa-pci.c
@@ -128,7 +128,6 @@ static int ce4100_i2c_probe(struct pci_dev *dev,
 	return 0;
 
 err_dev_add:
-	pci_set_drvdata(dev, NULL);
 	kfree(sds);
 err_mem:
 	pci_disable_device(dev);
@@ -141,7 +140,6 @@ static void ce4100_i2c_remove(struct pci_dev *dev)
 	unsigned int i;
 
 	sds = pci_get_drvdata(dev);
-	pci_set_drvdata(dev, NULL);
 
 	for (i = 0; i < ARRAY_SIZE(sds->pdev); i++)
 		platform_device_unregister(sds->pdev[i]);
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..fec18a4 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1219,8 +1219,6 @@ static int __exit i2c_pxa_remove(struct platform_device *dev)
 {
 	struct pxa_i2c *i2c = platform_get_drvdata(dev);
 
-	platform_set_drvdata(dev, NULL);
-
 	i2c_del_adapter(&i2c->adap);
 	if (!i2c->use_pio)
 		free_irq(i2c->irq, i2c);
diff --git a/drivers/i2c/busses/i2c-s6000.c b/drivers/i2c/busses/i2c-s6000.c
index 0088364..7c1ca5a 100644
--- a/drivers/i2c/busses/i2c-s6000.c
+++ b/drivers/i2c/busses/i2c-s6000.c
@@ -365,7 +365,6 @@ static int s6i2c_remove(struct platform_device *pdev)
 {
 	struct s6i2c_if *iface = platform_get_drvdata(pdev);
 	i2c_wr16(iface, S6_I2C_ENABLE, 0);
-	platform_set_drvdata(pdev, NULL);
 	i2c_del_adapter(&iface->adap);
 	free_irq(iface->irq, iface);
 	clk_disable(iface->clk);
diff --git a/drivers/i2c/busses/i2c-sh7760.c b/drivers/i2c/busses/i2c-sh7760.c
index 3a2253e..5351a2f 100644
--- a/drivers/i2c/busses/i2c-sh7760.c
+++ b/drivers/i2c/busses/i2c-sh7760.c
@@ -546,7 +546,6 @@ static int sh7760_i2c_remove(struct platform_device *pdev)
 	release_resource(id->ioarea);
 	kfree(id->ioarea);
 	kfree(id);
-	platform_set_drvdata(pdev, NULL);
 
 	return 0;
 }
diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
index 60195b5..0a6f941 100644
--- a/drivers/i2c/busses/i2c-stu300.c
+++ b/drivers/i2c/busses/i2c-stu300.c
@@ -975,7 +975,6 @@ stu300_remove(struct platform_device *pdev)
 	i2c_del_adapter(&dev->adapter);
 	/* Turn off everything */
 	stu300_wr8(0x00, dev->virtbase + I2C_CR);
-	platform_set_drvdata(pdev, NULL);
 	return 0;
 }
 
diff --git a/drivers/i2c/busses/i2c-taos-evm.c b/drivers/i2c/busses/i2c-taos-evm.c
index 26c352a..6ffa56e0 100644
--- a/drivers/i2c/busses/i2c-taos-evm.c
+++ b/drivers/i2c/busses/i2c-taos-evm.c
@@ -271,7 +271,6 @@ static int taos_connect(struct serio *serio, struct serio_driver *drv)
  exit_close:
 	serio_close(serio);
  exit_kfree:
-	serio_set_drvdata(serio, NULL);
 	kfree(taos);
  exit:
 	return err;
@@ -285,7 +284,6 @@ static void taos_disconnect(struct serio *serio)
 		i2c_unregister_device(taos->client);
 	i2c_del_adapter(&taos->adapter);
 	serio_close(serio);
-	serio_set_drvdata(serio, NULL);
 	kfree(taos);
 
 	dev_info(&serio->dev, "Disconnected from TAOS EVM\n");
diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
index eec20db..f3a8790 100644
--- a/drivers/i2c/busses/i2c-versatile.c
+++ b/drivers/i2c/busses/i2c-versatile.c
@@ -125,8 +125,6 @@ static int i2c_versatile_remove(struct platform_device *dev)
 {
 	struct i2c_versatile *i2c = platform_get_drvdata(dev);
 
-	platform_set_drvdata(dev, NULL);
-
 	i2c_del_adapter(&i2c->adap);
 	return 0;
 }
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index f042f6d..332c720 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -784,8 +784,6 @@ static int xiic_i2c_remove(struct platform_device *pdev)
 
 	xiic_deinit(i2c);
 
-	platform_set_drvdata(pdev, NULL);
-
 	free_irq(platform_get_irq(pdev, 0), i2c);
 
 	iounmap(i2c->base);
diff --git a/drivers/i2c/busses/i2c-xlr.c b/drivers/i2c/busses/i2c-xlr.c
index 93f029e..7945b05 100644
--- a/drivers/i2c/busses/i2c-xlr.c
+++ b/drivers/i2c/busses/i2c-xlr.c
@@ -256,7 +256,6 @@ static int xlr_i2c_remove(struct platform_device *pdev)
 
 	priv = platform_get_drvdata(pdev);
 	i2c_del_adapter(&priv->adap);
-	platform_set_drvdata(pdev, NULL);
 	return 0;
 }
 
diff --git a/drivers/i2c/busses/scx200_acb.c b/drivers/i2c/busses/scx200_acb.c
index 3862a95..2d1d2c5 100644
--- a/drivers/i2c/busses/scx200_acb.c
+++ b/drivers/i2c/busses/scx200_acb.c
@@ -542,7 +542,6 @@ static int scx200_remove(struct platform_device *pdev)
 	struct scx200_acb_iface *iface;
 
 	iface = platform_get_drvdata(pdev);
-	platform_set_drvdata(pdev, NULL);
 	scx200_cleanup_iface(iface);
 
 	return 0;
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 9f50ef0..abc2e55a 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -250,7 +250,6 @@ static int i2c_mux_gpio_remove(struct platform_device *pdev)
 	for (i = 0; i < mux->data.n_gpios; i++)
 		gpio_free(mux->gpio_base + mux->data.gpios[i]);
 
-	platform_set_drvdata(pdev, NULL);
 	i2c_put_adapter(mux->parent);
 
 	return 0;
-- 
1.8.1.3

^ permalink raw reply related

* Re: [RFC PATCH 2/5] powerpc: Exception hooks for context tracking subsystem
From: Li Zhong @ 2013-02-16  9:41 UTC (permalink / raw)
  To: Frederic Weisbecker; +Cc: paulmck, linuxppc-dev, linux-kernel, paulus
In-Reply-To: <CAFTL4hzfXfCGDjwjW=Frcs5+jS_i3U_JHytNOEhEF5X7_Fw6Sg@mail.gmail.com>

On Sun, 2013-02-10 at 15:10 +0100, Frederic Weisbecker wrote:
> 2013/2/1 Li Zhong <zhong@linux.vnet.ibm.com>:
> > This is the exception hooks for context tracking subsystem, including
> > data access, program check, single step, instruction breakpoint, machine check,
> > alignment, fp unavailable, altivec assist, unknown exception, whose handlers
> > might use RCU.
> >
> > This patch corresponds to
> > [PATCH] x86: Exception hooks for userspace RCU extended QS
> >   commit 6ba3c97a38803883c2eee489505796cb0a727122
> >
> > Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> 
> Looks good!
> 
> I guess we should move exception_enter/exit definition to the generic
> code. They should be the same for all archs after all. 

Indeed.

> Also we are
> relying on user_mode(regs) but this may be buggy with some corner
> cases. For example if an exception happen after a call to user_exit()

I guess you mean user_enter() here, or am I confused?

> (on syscall exit) but before we actually resume in userspace, the
> exception will exit in kernel mode from the context tracking POV.
> 
> So instead on relying on the regs, which are not sync with the context
> tracking state, we should use something like:
> 
> prev_state = exception_enter();
> ...
> exception_exit(prev_state);
> 
> Also preempt_schedule_irq() is concerned as well by this problem. So I
> should convert it to that scheme as well. I'm going to prepare some
> patches.
> 
> Feel free to merge this patch in the powerpc tree, I'll do the
> conversion along the way.

Or if your patches gets merged earlier than these, I can update my code
according to yours.

Thanks, Zhong

> 
> Thanks.
> 

^ permalink raw reply

* Re: PS3: Strange issue with kexec and FreeBSD loader
From: Phileas Fogg @ 2013-02-16 10:53 UTC (permalink / raw)
  To: Phileas Fogg; +Cc: linuxppc-dev
In-Reply-To: <1360365046.495584377@f356.mail.ru>

I was able to capture the debug output from the purgatory code and it's very odd.

This the SHA256 digest calculated by kexec-tools:

root@ps3-linux:~# kexec -l loader.ps3
Warning: append= option is not passed. Using the first kernel root partition
Modified cmdline:
Unable to find /proc/device-tree//chosen/linux,stdout-path, printing from 
purgatory is diabled
segment[0].mem:0x131d000 memsz:262144
segment[1].mem:0x135d000 memsz:36864
segment[2].mem:0x7fff000 memsz:4096
sha256_digest: 77 d5 30 a7 67 5f 67 93 f1 e0 ce 84 bd 4e 1b ec 3c 4a 9e 86 5c a1 
33 87 9e b1 5f c8 91 ce e8 61


And this is the debug output i'm always getting from the purgatory code:

I'm in purgatory
sha256 digests do not match :(
        digest: fd 4f df a8 af 5b e1 6b bc 51 5d b8 ab be 75 fb 76 fd 64 64 26 
3e a8 9f 46 ec 91 de 05 4e 72 78
sha256_digest: 00 39 e3 b2 45 0d 20 68 74 c2 4e ee e4 4a cf ec c3 78 4f 1c 65 ff 
a8 76 73 68 5d 01 70 0b b6 50

regards

^ permalink raw reply

* Re: PS3: Strange issue with kexec and FreeBSD loader
From: Phileas Fogg @ 2013-02-16 18:51 UTC (permalink / raw)
  To: Phileas Fogg; +Cc: linuxppc-dev
In-Reply-To: <1360365046.495584377@f356.mail.ru>

Phileas Fogg wrote:
>
> Hi,
>
> i'm using OpenWRT petitboot bootloader on my PS3 to boot FreeBSD loader which is a simple PPC32 ELF file.
> I haven't had any issues with it and OpenWRT based on Linux 3.3.8.
> Recently i built an OpenWRT image with Linux 3.7, i have no issues at all with kexec and any Linux kernels starting with 2.6 but
> FreeBSD loader won't boot and just hangs. The same issue with OpenWRT based on Linux 3.6 kernel.
> So, i started to analyze this problem and found out where it hangs.
>
> It seems that the purgatory code from kexec-tools loops endlessly if SHA256 verification of the loaded segments
> fails.
>
> See
>    http://git.kernel.org/?p=utils/kernel/kexec/kexec-tools.git;a=blob_plain;f=purgatory/purgatory.c;hb=566ca8a12145196b00ad37939cfd58a97f96ba89
>
> Because the function _verify_sha256_digest fails, the function _purgatory_ loops endlessly.
> This problem occurs only with Linux 3.6 or Linux 3.7 and FreeBSD loader.
> I killed the endless loop and could boot the FreeBSD loader on Linux 3.7 too.
>
> Any idea what could cause this problem ?
>
> Thanks.
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

Found another strange problem. I'm not able to boot FreeBSD LiveCD with
OpenWRT + Linux 3.8 (or Linux 3.7), the same CD which boots on
OpenWRT + Linux 3.3.8.

The LiveCD just panics and the PS3 console shuts down. Very odd.
The problem is probably connected with the kexec issue i'm having
and happens only with the recent Linux kernels.

regards

^ permalink raw reply

* Re: [PATCH] i2c: Remove unneeded xxx_set_drvdata(..., NULL) calls
From: Jean Delvare @ 2013-02-16 19:52 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Wolfram Sang, Tony Lindgren, Linus Walleij, Thierry Reding,
	Sekhar Nori, linux-i2c, Guan Xuetao, Kevin Hilman, Sonic Zhang,
	linux-arm-kernel, Deepak Sikri, Havard Skinnemoen, Marek Vasut,
	Pawel Moll, Stephen Warren, Sascha Hauer, Uwe Kleine-König,
	Rob Herring, Joe Perches, Lars-Peter Clausen,
	Ben Dooks (embedded platforms), Barry Song, linux-omap,
	Mika Westerberg, Oskar Schirmer, Fabio Estevam,
	davinci-linux-open-source, Shawn Guo, Jim Cromie,
	Greg Kroah-Hartman, Tomoya MORINAGA, linux-kernel, Kyungmin Park,
	Viresh Kumar, Karol Lewandowski, Jiri Kosina, STEricsson,
	uclinux-dist-devel, Andrew Morton, Alessandro Rubini,
	linuxppc-dev, Alexander Stein
In-Reply-To: <1360970315-32116-1-git-send-email-dianders@chromium.org>

On Fri, 15 Feb 2013 15:18:35 -0800, Doug Anderson wrote:
> There is simply no reason to be manually setting the private driver
> data to NULL in the remove/fail to probe cases.  This is just extra
> cruft code that can be removed.
> 
> A few notes:
> * Nothing relies on drvdata being set to NULL.
> * The __device_release_driver() function eventually calls
>   dev_set_drvdata(dev, NULL) anyway, so there's no need to do it
>   twice.

I had not noticed this change. Very good news!

> * I verified that there were no cases where xxx_get_drvdata() was
>   being called in these drivers and checking for / relying on the NULL
>   return value.
> 
> This could be cleaned up kernel-wide but for now just take the baby
> step and remove from the i2c subsystem.
> 
> Reported-by: Wolfram Sang <wsa@the-dreams.de>
> Reported-by: Stephen Warren <swarren@wwwdotorg.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> (...)

For i2c-taos-evm:

Reviewed-by: Jean Delvare <khali@linux-fr.org>

Also a note:

> --- a/drivers/i2c/busses/i2c-octeon.c
> +++ b/drivers/i2c/busses/i2c-octeon.c
> @@ -595,7 +595,7 @@ static int octeon_i2c_probe(struct platform_device *pdev)
>  	result = i2c_add_adapter(&i2c->adap);
>  	if (result < 0) {
>  		dev_err(i2c->dev, "failed to add adapter\n");
> -		goto fail_add;
> +		goto out;
>  	}
>  	dev_info(i2c->dev, "version %s\n", DRV_VERSION);
>  
> @@ -603,8 +603,6 @@ static int octeon_i2c_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -fail_add:
> -	platform_set_drvdata(pdev, NULL);
>  out:
>  	return result;
>  };

There no longer is any point in this error path, all gotos in this
function could be changed to returns (in a separate patch, obviously.)

-- 
Jean Delvare

^ permalink raw reply

* Re: PS3: Strange issue with kexec and FreeBSD loader
From: Phileas Fogg @ 2013-02-16 22:14 UTC (permalink / raw)
  To: Phileas Fogg; +Cc: linuxppc-dev
In-Reply-To: <511F652F.4090508@mail.ru>

Phileas Fogg wrote:
> I was able to capture the debug output from the purgatory code and it's very odd.
>
> This the SHA256 digest calculated by kexec-tools:
>
> root@ps3-linux:~# kexec -l loader.ps3
> Warning: append= option is not passed. Using the first kernel root partition
> Modified cmdline:
> Unable to find /proc/device-tree//chosen/linux,stdout-path, printing from
> purgatory is diabled
> segment[0].mem:0x131d000 memsz:262144
> segment[1].mem:0x135d000 memsz:36864
> segment[2].mem:0x7fff000 memsz:4096
> sha256_digest: 77 d5 30 a7 67 5f 67 93 f1 e0 ce 84 bd 4e 1b ec 3c 4a 9e 86 5c a1
> 33 87 9e b1 5f c8 91 ce e8 61
>
>
> And this is the debug output i'm always getting from the purgatory code:
>
> I'm in purgatory
> sha256 digests do not match :(
>         digest: fd 4f df a8 af 5b e1 6b bc 51 5d b8 ab be 75 fb 76 fd 64 64 26
> 3e a8 9f 46 ec 91 de 05 4e 72 78
> sha256_digest: 00 39 e3 b2 45 0d 20 68 74 c2 4e ee e4 4a cf ec c3 78 4f 1c 65 ff
> a8 76 73 68 5d 01 70 0b b6 50
>
> regards
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

I was able to analyze the problem more and found out that the device tree memory 
region gets corrupted. I slightly modified kexec-tools and made it first compute 
a checksum of the first segment only where the new kernel is located.
And the checksum was always verified as correct in the purgatoroy code.
Then i made kexec-tools compute the checksum of the 3rd segment only where a 
device tree is stored. And this time the verify function in the purgatory failed 
always.

Output form the purgatory code:
--------------------------------

I'm in purgatory
sha256 digests do not match :(
        digest: e3 b0 c4 42 98 fc 1c 14 9a fb f4 c8 99 6f b9 24 27 ae 41 e4 64 
9b 93 4c a4 95 99 1b 78 52 b8 55
sha256_digest: 57 08 81 e7 62 c3 22 2f d9 1d 94 a5 d0 f7 53 8f fe 69 64 84 4d 71 
2d aa e2 07 45 b3 78 79 6e 26
sha256_regions:
start=0x0000000007fff000 len=0x0000000000001000

The sha256_digest is actually the correct SHA256 checksum precomputed by 
kexec-tools when the new kernel was given to the old kernel.

I will try to analyze the problem more later.

regards

^ permalink raw reply

* Re: PS3: Strange issue with kexec and FreeBSD loader
From: Phileas Fogg @ 2013-02-16 23:12 UTC (permalink / raw)
  To: Phileas Fogg; +Cc: linuxppc-dev
In-Reply-To: <511F652F.4090508@mail.ru>


I found new clues about the problem.

Normally the device tree memory segment is allocated at the top of the boot 
memory region. The boot memory size on the PS3 console is 128MB.

root@ps3-linux:~# kexec -l loader.ps3
segment[0].mem:0x131d000 memsz:262144
segment[1].mem:0x135d000 memsz:36864
segment[2].mem:0x7fff000 memsz:4096

And the device tree is located at address 0x7fff000, it's the last page of the 
boot memory.

I changed the kexec-tools and made it store the device tree just after the 
purgatory code which is located at address 0x135d000. Like here:

root@ps3-linux:~# kexec -l loader.ps3
segment[0].mem:0x131d000 memsz:262144
segment[1].mem:0x135d000 memsz:36864
segment[2].mem:0x1366000 memsz:4096   <---- new address of device tree segment

And now the sha256 verification is always successful for the FreeBSD loader too.
But still no idea what actually corrupts the device tree segment when it's 
located at the top of the boot memory region. And why it happens on Linux 3.7 
and Linux 3.8 but not on Linux 3.3.8.

regards

^ permalink raw reply

* RE: [PATCH] powerpc/85xx: dts - add ranges property for SEC
From: Liu Po-B43644 @ 2013-02-17  2:40 UTC (permalink / raw)
  To: Kumar Gala, Phillips Kim-R1AAHA; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <3D277CC6-2C2C-46E7-B0F8-FAECED8BC868@kernel.crashing.org>

Hi Kim,

Thank you for the fixing.=20


Best regards,
Liu Po
- 8038

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]=20
Sent: Wednesday, February 13, 2013 1:27 AM
To: Phillips Kim-R1AAHA
Cc: Liu Po-B43644; linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] powerpc/85xx: dts - add ranges property for SEC


On Jan 18, 2013, at 2:40 PM, Kim Phillips wrote:

> On Fri, 18 Jan 2013 17:16:13 +0800
> Po Liu <po.liu@freescale.com> wrote:
>=20
>> This facilitates getting the physical address of the SEC node.
>>=20
>> Signed-off-by: Liu po <po.liu@freescale.com>
>> ---
> Reviewed-by: Kim Phillips <kim.phillips@freescale.com>
>=20
> Kim

This was missing a trailing ';', so wondering if it was ever tested?

I fixed when I applied.

applied.

- k

^ permalink raw reply

* Re: [PATCH 2/2] of: use platform_device_add
From: Shawn Guo @ 2013-02-17  3:03 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, Rob Herring, Jason Gunthorpe, Greg Kroah-Hartman,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358473200-17886-2-git-send-email-grant.likely@secretlab.ca>

On Fri, Jan 18, 2013 at 01:40:00AM +0000, Grant Likely wrote:
> This allows platform_device_add a chance to call insert_resource on all
> of the resources from OF. At a minimum this fills in proc/iomem and
> presumably makes resource tracking and conflict detection work better.
> However, it has the side effect of moving all OF generated platform
> devices from /sys/devices to /sys/devices/platform/. It /shouldn't/
> break userspace because userspace is not supposed to depend on the full
> path (because userspace always does what it is supposed to, right?).
> 
> This may cause breakage if either:
> 1) any two nodes in a given device tree have overlapping & staggered
>    regions (ie. 0x80..0xbf and 0xa0..0xdf; where one is not contained
>    within the other). In this case one of the devices will fail to
>    register and an exception will be needed in platform_device_add() to
>    complain but not fail.

Grant,

The patch introduce a regression on imx6q boot.  The IOMUXC block on
imx6q is special.  It acts not only a pin controller but also a system
controller with a bunch of system level registers in there.  That's why
we currently have the following two nodes in imx6q device tree with the
same start "reg" address, which work with drivers/mfd/syscon.c and
drivers/pinctrl/pinctrl-imx6q.c respectively.

	gpr: iomuxc-gpr@020e0000 {
		compatible = "fsl,imx6q-iomuxc-gpr", "syscon";
		reg = <0x020e0000 0x38>;
	};

	iomuxc: iomuxc@020e0000 {
		compatible = "fsl,imx6q-iomuxc";
		reg = <0x020e0000 0x4000>;
	};

With the patch in place, pinctrl-imx6q fails to register like below.

syscon 20e0000.iomuxc: syscon regmap start 0x20e0000 end 0x20e3fff registered
imx6q-pinctrl 20e0000.iomuxc: can't request region for resource [mem 0x020e0000-0x020e3fff]
imx6q-pinctrl: probe of 20e0000.iomuxc failed with error -16

Shawn

> 2) any device calls request_mem_region() on a region larger than
>    specified in the device tree. In this case the device node may be
>    wrong, or the driver is overreaching. In either case I'd like to know
>    about any problems and fix them.

^ permalink raw reply

* Re: [PATCH] arch/powerpc/kernel: using %12.12s instead of %12s for avoiding memory overflow.
From: Chen Gang @ 2013-02-17  4:00 UTC (permalink / raw)
  To: benh, paulus; +Cc: linuxppc-dev
In-Reply-To: <5100B53C.3030109@asianux.com>

Hello relative members:

  please give a glance to this patch, when you have time.

  thanks.

  :-)

gchen.


于 2013年01月24日 12:14, Chen Gang 写道:
> 
>   for tmp_part->header.name:
>     it is "Terminating null required only for names < 12 chars".
>     so need to limit the %.12s for it in printk
> 
>   additional info:
> 
>     %12s  limit the width, not for the original string output length
>           if name length is more than 12, it still can be fully displayed.
>           if name length is less than 12, the ' ' will be filled before name.
> 
>     %.12s truly limit the original string output length (precision)
> 
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  arch/powerpc/kernel/nvram_64.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index bec1e93..57bf6d2 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -202,7 +202,7 @@ static void __init nvram_print_partitions(char * label)
>  	printk(KERN_WARNING "--------%s---------\n", label);
>  	printk(KERN_WARNING "indx\t\tsig\tchks\tlen\tname\n");
>  	list_for_each_entry(tmp_part, &nvram_partitions, partition) {
> -		printk(KERN_WARNING "%4d    \t%02x\t%02x\t%d\t%12s\n",
> +		printk(KERN_WARNING "%4d    \t%02x\t%02x\t%d\t%12.12s\n",
>  		       tmp_part->index, tmp_part->header.signature,
>  		       tmp_part->header.checksum, tmp_part->header.length,
>  		       tmp_part->header.name);
> 


-- 
Chen Gang

Asianux Corporation

^ permalink raw reply

* Re: [PATCH 2/2] of: use platform_device_add
From: Shawn Guo @ 2013-02-17  7:43 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, Rob Herring, Jason Gunthorpe, Greg Kroah-Hartman,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <20130217030331.GA15048@S2101-09.ap.freescale.net>

On Sun, Feb 17, 2013 at 11:03:35AM +0800, Shawn Guo wrote:
> On Fri, Jan 18, 2013 at 01:40:00AM +0000, Grant Likely wrote:
> > This allows platform_device_add a chance to call insert_resource on all
> > of the resources from OF. At a minimum this fills in proc/iomem and
> > presumably makes resource tracking and conflict detection work better.
> > However, it has the side effect of moving all OF generated platform
> > devices from /sys/devices to /sys/devices/platform/. It /shouldn't/
> > break userspace because userspace is not supposed to depend on the full
> > path (because userspace always does what it is supposed to, right?).
> > 
> > This may cause breakage if either:
> > 1) any two nodes in a given device tree have overlapping & staggered
> >    regions (ie. 0x80..0xbf and 0xa0..0xdf; where one is not contained
> >    within the other). In this case one of the devices will fail to
> >    register and an exception will be needed in platform_device_add() to
> >    complain but not fail.
> 
> Grant,
> 
> The patch introduce a regression on imx6q boot.

It also breaks all of_amba_device users.

of_amba_device_create() --> amba_device_add() --> request_resource()
and fails.

Shawn

^ permalink raw reply

* Re: [PATCH 2/2] of: use platform_device_add
From: Russell King - ARM Linux @ 2013-02-17 10:19 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Jason Gunthorpe, linux-kernel, Rob Herring, Greg Kroah-Hartman,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <20130217074317.GB16632@S2101-09.ap.freescale.net>

On Sun, Feb 17, 2013 at 03:43:20PM +0800, Shawn Guo wrote:
> It also breaks all of_amba_device users.
> 
> of_amba_device_create() --> amba_device_add() --> request_resource()
> and fails.

Presumably that's because we no longer know what the parent resource
is supposed to be?

^ permalink raw reply

* Re: PS3: Strange issue with kexec and FreeBSD loader
From: Geert Uytterhoeven @ 2013-02-17  8:53 UTC (permalink / raw)
  To: Phileas Fogg; +Cc: linuxppc-dev
In-Reply-To: <51201276.8020104@mail.ru>

Hi Phileas,

On Sun, Feb 17, 2013 at 12:12 AM, Phileas Fogg <phileas-fogg@mail.ru> wrote:
> I found new clues about the problem.
>
> Normally the device tree memory segment is allocated at the top of the boot
> memory region. The boot memory size on the PS3 console is 128MB.
>
>
> root@ps3-linux:~# kexec -l loader.ps3
> segment[0].mem:0x131d000 memsz:262144
> segment[1].mem:0x135d000 memsz:36864
> segment[2].mem:0x7fff000 memsz:4096
>
> And the device tree is located at address 0x7fff000, it's the last page of
> the boot memory.
>
> I changed the kexec-tools and made it store the device tree just after the
> purgatory code which is located at address 0x135d000. Like here:
>
>
> root@ps3-linux:~# kexec -l loader.ps3
> segment[0].mem:0x131d000 memsz:262144
> segment[1].mem:0x135d000 memsz:36864
> segment[2].mem:0x1366000 memsz:4096   <---- new address of device tree
> segment
>
> And now the sha256 verification is always successful for the FreeBSD loader
> too.
> But still no idea what actually corrupts the device tree segment when it's
> located at the top of the boot memory region. And why it happens on Linux
> 3.7 and Linux 3.8 but not on Linux 3.3.8.

Have you looked at the actual data that ends up being written there?
It may give a clue...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 2/2] of: use platform_device_add
From: Grant Likely @ 2013-02-17 10:49 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Linux Kernel Mailing List, Rob Herring, Jason Gunthorpe,
	Greg Kroah-Hartman, Shawn Guo, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20130217101958.GR17833@n2100.arm.linux.org.uk>

On Sun, Feb 17, 2013 at 10:19 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Feb 17, 2013 at 03:43:20PM +0800, Shawn Guo wrote:
> > The patch introduce a regression on imx6q boot.  The IOMUXC block on
> > imx6q is special.  It acts not only a pin controller but also a system
> > controller with a bunch of system level registers in there.  That's why
> > we currently have the following two nodes in imx6q device tree with the
> > same start "reg" address, which work with drivers/mfd/syscon.c and
> > drivers/pinctrl/pinctrl-imx6q.c respectively.
> >
> >         gpr: iomuxc-gpr@020e0000 {
> >                 compatible = "fsl,imx6q-iomuxc-gpr", "syscon";
> >                 reg = <0x020e0000 0x38>;
> >         };
> >
> >         iomuxc: iomuxc@020e0000 {
> >                 compatible = "fsl,imx6q-iomuxc";
> >                 reg = <0x020e0000 0x4000>;
> >         };
> >
> > With the patch in place, pinctrl-imx6q fails to register like below.
> >
> > syscon 20e0000.iomuxc: syscon regmap start 0x20e0000 end 0x20e3fff registered
> > imx6q-pinctrl 20e0000.iomuxc: can't request region for resource [mem 0x020e0000-0x020e3fff]
> > imx6q-pinctrl: probe of 20e0000.iomuxc failed with error -16

Strictly you're not supposed to do that with the device tree. There
shouldn't be two nodes using the same overlapping memory region unless
they are parent/child. That rule has been around for a long time, but
the core never checked for it. What /should/ happen is the two drivers
should be cooperating for the register region and only one device
driver probe sets up both behaviours.

However, neither is it okay to just break the existing device trees.
Best thing to do I think is to deprecate one of the nodes.

>> It also breaks all of_amba_device users.
>>
>> of_amba_device_create() --> amba_device_add() --> request_resource()
>> and fails.
>
> Presumably that's because we no longer know what the parent resource
> is supposed to be?

Hmmm, it looks that way, yes. Currently the OF code is using
iomem_resource as the parent for all amba_device_add() calls
(driver/of/platform.c), but if the parent node gets registered as a
platform device and it has the resources then the parenthood chain
doesn't match up. It isn't immediately obvious to me how to fix this.
I'm going to drop the patch from my tree. I could use some help
figuring out what the correct behaviour really should be here.

g.

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

^ permalink raw reply

* Re: PS3: Strange issue with kexec and FreeBSD loader
From: Phileas Fogg @ 2013-02-17 12:40 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linuxppc-dev
In-Reply-To: <CAMuHMdUTQWpQH=UaHc=iZPiJrry7_VqOjtm7bX+jW6r3vOge8A@mail.gmail.com>

Geert Uytterhoeven wrote:
> Hi Phileas,
>
> On Sun, Feb 17, 2013 at 12:12 AM, Phileas Fogg <phileas-fogg@mail.ru> wrote:
>> I found new clues about the problem.
>>
>> Normally the device tree memory segment is allocated at the top of the boot
>> memory region. The boot memory size on the PS3 console is 128MB.
>>
>>
>> root@ps3-linux:~# kexec -l loader.ps3
>> segment[0].mem:0x131d000 memsz:262144
>> segment[1].mem:0x135d000 memsz:36864
>> segment[2].mem:0x7fff000 memsz:4096
>>
>> And the device tree is located at address 0x7fff000, it's the last page of
>> the boot memory.
>>
>> I changed the kexec-tools and made it store the device tree just after the
>> purgatory code which is located at address 0x135d000. Like here:
>>
>>
>> root@ps3-linux:~# kexec -l loader.ps3
>> segment[0].mem:0x131d000 memsz:262144
>> segment[1].mem:0x135d000 memsz:36864
>> segment[2].mem:0x1366000 memsz:4096   <---- new address of device tree
>> segment
>>
>> And now the sha256 verification is always successful for the FreeBSD loader
>> too.
>> But still no idea what actually corrupts the device tree segment when it's
>> located at the top of the boot memory region. And why it happens on Linux
>> 3.7 and Linux 3.8 but not on Linux 3.3.8.
>
> Have you looked at the actual data that ends up being written there?
> It may give a clue...
>
> Gr{oetje,eeting}s,
>
>                          Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

i was able to dump the device tree data from the purgatory code and compared the 
original DT which i dumped from kexec-tools and the one from purgatory.
About 20 bytes at the end of the string table of the device tree were corrupted. 
Large part of the new data are 0s.

regards

^ permalink raw reply

* Re: [PATCH 2/2] of: use platform_device_add
From: Fabio Estevam @ 2013-02-17 13:18 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Jason Gunthorpe, linux-kernel, Rob Herring, Greg Kroah-Hartman,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <20130217074317.GB16632@S2101-09.ap.freescale.net>

On Sun, Feb 17, 2013 at 4:43 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Sun, Feb 17, 2013 at 11:03:35AM +0800, Shawn Guo wrote:
>> On Fri, Jan 18, 2013 at 01:40:00AM +0000, Grant Likely wrote:
>> > This allows platform_device_add a chance to call insert_resource on all
>> > of the resources from OF. At a minimum this fills in proc/iomem and
>> > presumably makes resource tracking and conflict detection work better.
>> > However, it has the side effect of moving all OF generated platform
>> > devices from /sys/devices to /sys/devices/platform/. It /shouldn't/
>> > break userspace because userspace is not supposed to depend on the full
>> > path (because userspace always does what it is supposed to, right?).
>> >
>> > This may cause breakage if either:
>> > 1) any two nodes in a given device tree have overlapping & staggered
>> >    regions (ie. 0x80..0xbf and 0xa0..0xdf; where one is not contained
>> >    within the other). In this case one of the devices will fail to
>> >    register and an exception will be needed in platform_device_add() to
>> >    complain but not fail.
>>
>> Grant,
>>
>> The patch introduce a regression on imx6q boot.
>
> It also breaks all of_amba_device users.
>
> of_amba_device_create() --> amba_device_add() --> request_resource()
> and fails.

Yes, correct: amba-pl011 does not register anymore after this patch,
which causes the serial console to be not functional.

^ permalink raw reply

* Re: [PATCH] i2c: Remove unneeded xxx_set_drvdata(..., NULL) calls
From: Peter Korsgaard @ 2013-02-17 15:12 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Wolfram Sang, Tony Lindgren, Linus Walleij, Thierry Reding,
	Sekhar Nori, linux-i2c, Guan Xuetao, Kevin Hilman, Sonic Zhang,
	linux-arm-kernel, Deepak Sikri, Havard Skinnemoen, Marek Vasut,
	Pawel Moll, Stephen Warren, Sascha Hauer, Uwe Kleine-König,
	Rob Herring, uclinux-dist-devel, Jean Delvare, Lars-Peter Clausen,
	Ben Dooks (embedded platforms), Barry Song, linux-omap,
	Mika Westerberg, Oskar Schirmer, Fabio Estevam,
	davinci-linux-open-source, Shawn Guo, Jim Cromie,
	Greg Kroah-Hartman, Tomoya MORINAGA, linux-kernel, Kyungmin Park,
	Viresh Kumar, Karol Lewandowski, Jiri Kosina, STEricsson,
	Joe Perches, Andrew Morton, Alessandro Rubini, linuxppc-dev,
	Alexander Stein
In-Reply-To: <1360970315-32116-1-git-send-email-dianders@chromium.org>

>>>>> "Doug" == Doug Anderson <dianders@chromium.org> writes:

 Doug> There is simply no reason to be manually setting the private driver
 Doug> data to NULL in the remove/fail to probe cases.  This is just extra
 Doug> cruft code that can be removed.

 Doug> A few notes:
 Doug> * Nothing relies on drvdata being set to NULL.
 Doug> * The __device_release_driver() function eventually calls
 Doug>   dev_set_drvdata(dev, NULL) anyway, so there's no need to do it
 Doug>   twice.
 Doug> * I verified that there were no cases where xxx_get_drvdata() was
 Doug>   being called in these drivers and checking for / relying on the NULL
 Doug>   return value.

 Doug> This could be cleaned up kernel-wide but for now just take the baby
 Doug> step and remove from the i2c subsystem.

 Doug> Reported-by: Wolfram Sang <wsa@the-dreams.de>
 Doug> Reported-by: Stephen Warren <swarren@wwwdotorg.org>
 Doug> Signed-off-by: Doug Anderson <dianders@chromium.org>

For i2c-ocores.c + i2c-mux-gpio.c:

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

-- 
Bye, Peter Korsgaard

^ permalink raw reply

* RE: [PATCH][UPSTEAM] powerpc/mpic: add irq_set_wake support
From: Wang Dongsheng-B40534 @ 2013-02-18  2:52 UTC (permalink / raw)
  To: Kumar Gala, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1359601823-9861-1-git-send-email-dongsheng.wang@freescale.com>

Hi Kumar,
Could you ack this patch?
Thanks.

> -----Original Message-----
> From: Wang Dongsheng-B40534
> Sent: Thursday, January 31, 2013 11:10 AM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Wang Dongsheng-B40534
> Subject: [PATCH][UPSTEAM] powerpc/mpic: add irq_set_wake support
>=20
> Add irq_set_wake support. Just add IRQF_NO_SUSPEND to desc->action->flag.
> So the wake up interrupt will not be disable in suspend_device_irqs.
>=20
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> ---
>  arch/powerpc/sysdev/mpic.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
>=20
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 9c6e535..2ed0220 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -920,6 +920,18 @@ int mpic_set_irq_type(struct irq_data *d, unsigned
> int flow_type)
>  	return IRQ_SET_MASK_OK_NOCOPY;
>  }
>=20
> +static int mpic_irq_set_wake(struct irq_data *d, unsigned int on)
> +{
> +	struct irq_desc *desc =3D container_of(d, struct irq_desc, irq_data);
> +
> +	if (on)
> +		desc->action->flags |=3D IRQF_NO_SUSPEND;
> +	else
> +		desc->action->flags &=3D ~IRQF_NO_SUSPEND;
> +
> +	return 0;
> +}
> +
>  void mpic_set_vector(unsigned int virq, unsigned int vector)
>  {
>  	struct mpic *mpic =3D mpic_from_irq(virq);
> @@ -957,6 +969,7 @@ static struct irq_chip mpic_irq_chip =3D {
>  	.irq_unmask	=3D mpic_unmask_irq,
>  	.irq_eoi	=3D mpic_end_irq,
>  	.irq_set_type	=3D mpic_set_irq_type,
> +	.irq_set_wake	=3D mpic_irq_set_wake,
>  };
>=20
>  #ifdef CONFIG_SMP
> @@ -971,6 +984,7 @@ static struct irq_chip mpic_tm_chip =3D {
>  	.irq_mask	=3D mpic_mask_tm,
>  	.irq_unmask	=3D mpic_unmask_tm,
>  	.irq_eoi	=3D mpic_end_irq,
> +	.irq_set_wake	=3D mpic_irq_set_wake,
>  };
>=20
>  #ifdef CONFIG_MPIC_U3_HT_IRQS
> @@ -981,6 +995,7 @@ static struct irq_chip mpic_irq_ht_chip =3D {
>  	.irq_unmask	=3D mpic_unmask_ht_irq,
>  	.irq_eoi	=3D mpic_end_ht_irq,
>  	.irq_set_type	=3D mpic_set_irq_type,
> +	.irq_set_wake	=3D mpic_irq_set_wake,
>  };
>  #endif /* CONFIG_MPIC_U3_HT_IRQS */
>=20
> --
> 1.7.5.1

^ permalink raw reply


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