LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 5/6] powerpc/corenet: Add DPAA FMan support to the SoC device tree(s)
From: Scott Wood @ 2014-05-07 23:14 UTC (permalink / raw)
  To: Emil Medve
  Cc: devicetree, Kanetkar Shruti-B44454, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <5368811A.3060609@Freescale.com>

On Tue, 2014-05-06 at 01:28 -0500, Emil Medve wrote:
> Hello Scott,
> 
> 
> On 05/05/2014 06:34 PM, Scott Wood wrote:
> > On Sun, 2014-05-04 at 05:59 -0500, Emil Medve wrote:
> >>  Anyway, most days PHYs can be discovered so they don't use/need
> >> compatible properties. That's I guess part of the reason we don't have
> >> bindings for them PHY nodes
> > 
> > I don't see why there couldn't be a compatible that describes the
> > standard programming interface.
> 
> Because it can be detected at runtime and I guess stuff like that should
> stay out of the device tree. I'm using PCI as an analogy here

But in this case aren't you using a standardized component of the
programming model itself to probe the specific PHY type?  I think a
better analogy is the "cfi-flash" compatible.

-Scott

^ permalink raw reply

* Re: [PATCH 1/1] booke/watchdog: refine and clean up the codes
From: Scott Wood @ 2014-05-07 23:20 UTC (permalink / raw)
  To: Tang Yuantian; +Cc: linuxppc-dev
In-Reply-To: <1399433437-19520-1-git-send-email-Yuantian.Tang@freescale.com>

On Wed, 2014-05-07 at 11:30 +0800, Tang Yuantian wrote:
> From: Tang Yuantian <yuantian.tang@freescale.com>
> 
> Basically, this patch does the following:
> 1. Move the codes of parsing boot parameters from setup-common.c
>    to driver. In this way, code reader can know directly that
>    there are boot parameters that can change the timeout.
> 2. Make boot parameter 'booke_wdt_period' effective.
>    currently, when driver is loaded, default timeout is always
>    being used in stead of booke_wdt_period.
> 3. Wrap up the watchdog timeout in device struct and clean up
>    unnecessary codes.
> 
> Signed-off-by: Tang Yuantian <yuantian.tang@freescale.com>
> ---
>  arch/powerpc/kernel/setup-common.c | 27 --------------------
>  drivers/watchdog/booke_wdt.c       | 51 ++++++++++++++++++++++++--------------
>  2 files changed, 33 insertions(+), 45 deletions(-)

Acked-by: Scott Wood <scottwood@freescale.com>

...but it looks like you didn't send this to the WDT maintainer and
list.

-Scott

^ permalink raw reply

* [PATCH 1/1] booke/watchdog: refine and clean up the codes
From: Yuantian.Tang @ 2014-05-08  2:04 UTC (permalink / raw)
  To: wim; +Cc: scottwood, Tang Yuantian, linuxppc-dev, linux-watchdog

From: Tang Yuantian <yuantian.tang@freescale.com>

Basically, this patch does the following:
1. Move the codes of parsing boot parameters from setup-common.c
   to driver. In this way, code reader can know directly that
   there are boot parameters that can change the timeout.
2. Make boot parameter 'booke_wdt_period' effective.
   currently, when driver is loaded, default timeout is always
   being used in stead of booke_wdt_period.
3. Wrap up the watchdog timeout in device struct and clean up
   unnecessary codes.

Signed-off-by: Tang Yuantian <yuantian.tang@freescale.com>
Acked-by: Scott Wood <scottwood@freescale.com>
---
resend to watchdog maintainer

 arch/powerpc/kernel/setup-common.c | 27 --------------------
 drivers/watchdog/booke_wdt.c       | 51 ++++++++++++++++++++++++--------------
 2 files changed, 33 insertions(+), 45 deletions(-)

diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index bc76cc6..5874aef 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -715,33 +715,6 @@ static int powerpc_debugfs_init(void)
 arch_initcall(powerpc_debugfs_init);
 #endif
 
-#ifdef CONFIG_BOOKE_WDT
-extern u32 booke_wdt_enabled;
-extern u32 booke_wdt_period;
-
-/* Checks wdt=x and wdt_period=xx command-line option */
-notrace int __init early_parse_wdt(char *p)
-{
-	if (p && strncmp(p, "0", 1) != 0)
-		booke_wdt_enabled = 1;
-
-	return 0;
-}
-early_param("wdt", early_parse_wdt);
-
-int __init early_parse_wdt_period(char *p)
-{
-	unsigned long ret;
-	if (p) {
-		if (!kstrtol(p, 0, &ret))
-			booke_wdt_period = ret;
-	}
-
-	return 0;
-}
-early_param("wdt_period", early_parse_wdt_period);
-#endif	/* CONFIG_BOOKE_WDT */
-
 void ppc_printk_progress(char *s, unsigned short hex)
 {
 	pr_info("%s\n", s);
diff --git a/drivers/watchdog/booke_wdt.c b/drivers/watchdog/booke_wdt.c
index a8dbceb3..08a7853 100644
--- a/drivers/watchdog/booke_wdt.c
+++ b/drivers/watchdog/booke_wdt.c
@@ -41,6 +41,28 @@ u32 booke_wdt_period = CONFIG_BOOKE_WDT_DEFAULT_TIMEOUT;
 #define WDTP_MASK	(TCR_WP_MASK)
 #endif
 
+/* Checks wdt=x and wdt_period=xx command-line option */
+notrace int __init early_parse_wdt(char *p)
+{
+	if (p && strncmp(p, "0", 1) != 0)
+		booke_wdt_enabled = 1;
+
+	return 0;
+}
+early_param("wdt", early_parse_wdt);
+
+int __init early_parse_wdt_period(char *p)
+{
+	unsigned long ret;
+	if (p) {
+		if (!kstrtol(p, 0, &ret))
+			booke_wdt_period = ret;
+	}
+
+	return 0;
+}
+early_param("wdt_period", early_parse_wdt_period);
+
 #ifdef CONFIG_PPC_FSL_BOOK3E
 
 /* For the specified period, determine the number of seconds
@@ -103,17 +125,18 @@ static unsigned int sec_to_period(unsigned int secs)
 static void __booke_wdt_set(void *data)
 {
 	u32 val;
+	struct watchdog_device *wdog = data;
 
 	val = mfspr(SPRN_TCR);
 	val &= ~WDTP_MASK;
-	val |= WDTP(booke_wdt_period);
+	val |= WDTP(sec_to_period(wdog->timeout));
 
 	mtspr(SPRN_TCR, val);
 }
 
-static void booke_wdt_set(void)
+static void booke_wdt_set(void *data)
 {
-	on_each_cpu(__booke_wdt_set, NULL, 0);
+	on_each_cpu(__booke_wdt_set, data, 0);
 }
 
 static void __booke_wdt_ping(void *data)
@@ -131,12 +154,13 @@ static int booke_wdt_ping(struct watchdog_device *wdog)
 static void __booke_wdt_enable(void *data)
 {
 	u32 val;
+	struct watchdog_device *wdog = data;
 
 	/* clear status before enabling watchdog */
 	__booke_wdt_ping(NULL);
 	val = mfspr(SPRN_TCR);
 	val &= ~WDTP_MASK;
-	val |= (TCR_WIE|TCR_WRC(WRC_CHIP)|WDTP(booke_wdt_period));
+	val |= (TCR_WIE|TCR_WRC(WRC_CHIP)|WDTP(sec_to_period(wdog->timeout)));
 
 	mtspr(SPRN_TCR, val);
 }
@@ -162,25 +186,17 @@ static void __booke_wdt_disable(void *data)
 
 }
 
-static void __booke_wdt_start(struct watchdog_device *wdog)
+static int booke_wdt_start(struct watchdog_device *wdog)
 {
-	on_each_cpu(__booke_wdt_enable, NULL, 0);
+	on_each_cpu(__booke_wdt_enable, wdog, 0);
 	pr_debug("watchdog enabled (timeout = %u sec)\n", wdog->timeout);
-}
 
-static int booke_wdt_start(struct watchdog_device *wdog)
-{
-	if (booke_wdt_enabled == 0) {
-		booke_wdt_enabled = 1;
-		__booke_wdt_start(wdog);
-	}
 	return 0;
 }
 
 static int booke_wdt_stop(struct watchdog_device *wdog)
 {
 	on_each_cpu(__booke_wdt_disable, NULL, 0);
-	booke_wdt_enabled = 0;
 	pr_debug("watchdog disabled\n");
 
 	return 0;
@@ -191,9 +207,8 @@ static int booke_wdt_set_timeout(struct watchdog_device *wdt_dev,
 {
 	if (timeout > MAX_WDT_TIMEOUT)
 		return -EINVAL;
-	booke_wdt_period = sec_to_period(timeout);
 	wdt_dev->timeout = timeout;
-	booke_wdt_set();
+	booke_wdt_set(wdt_dev);
 
 	return 0;
 }
@@ -231,10 +246,10 @@ static int __init booke_wdt_init(void)
 	pr_info("powerpc book-e watchdog driver loaded\n");
 	booke_wdt_info.firmware_version = cur_cpu_spec->pvr_value;
 	booke_wdt_set_timeout(&booke_wdt_dev,
-			      period_to_sec(CONFIG_BOOKE_WDT_DEFAULT_TIMEOUT));
+			      period_to_sec(booke_wdt_period));
 	watchdog_set_nowayout(&booke_wdt_dev, nowayout);
 	if (booke_wdt_enabled)
-		__booke_wdt_start(&booke_wdt_dev);
+		booke_wdt_start(&booke_wdt_dev);
 
 	ret = watchdog_register_device(&booke_wdt_dev);
 
-- 
1.8.5

^ permalink raw reply related

* [PATCH v2] clk: qoriq: Update the clock bindings
From: Yuantian.Tang @ 2014-05-08  3:12 UTC (permalink / raw)
  To: scottwood; +Cc: devicetree, linuxppc-dev, Tang Yuantian

From: Tang Yuantian <yuantian.tang@freescale.com>

Main changs include:
	- Clarified the clock nodes' version number
	- Fixed a issue in example

Singed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
---
v2:
	- rename this binding
	- rewrite the description

 .../bindings/clock/{corenet-clock.txt => qoriq-clock.txt}      | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
 rename Documentation/devicetree/bindings/clock/{corenet-clock.txt => qoriq-clock.txt} (95%)

diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
similarity index 95%
rename from Documentation/devicetree/bindings/clock/corenet-clock.txt
rename to Documentation/devicetree/bindings/clock/qoriq-clock.txt
index 24711af..5666812 100644
--- a/Documentation/devicetree/bindings/clock/corenet-clock.txt
+++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
@@ -7,6 +7,14 @@ which can then be passed to a variety of internal logic, including
 cores and peripheral IP blocks.
 Please refer to the Reference Manual for details.
 
+All references to "1.0" and "2.0" refer to the QorIQ chassis version to
+which the chip complies.
+
+Chassis Version		Example Chips
+---------------		-------------
+1.0			p4080, p5020, p5040
+2.0			t4240, b4860, t1040
+
 1. Clock Block Binding
 
 Required properties:
@@ -85,7 +93,7 @@ Example for clock block and clock provider:
 			#clock-cells = <0>;
 			compatible = "fsl,qoriq-sysclk-1.0";
 			clock-output-names = "sysclk";
-		}
+		};
 
 		pll0: pll0@800 {
 			#clock-cells = <1>;
-- 
1.8.5

^ permalink raw reply related

* Re: [PATCH 4/6] powerpc/corenet: Create the dts components for the DPAA FMan
From: Emil Medve @ 2014-05-08  3:23 UTC (permalink / raw)
  To: Scott Wood; +Cc: devicetree, Shruti Kanetkar, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1399431248.15726.255.camel@snotra.buserror.net>

Hello Scott,


At this point, I'm getting a bit lost in this thread and I feel we're
getting into more generic comments. I'll continue to answer to some of
your comments to make sure I understand them. However, one major comment
I feel you made was about seeing a full FMan binding. If that's a deal
breaker, please let me know and we'll just drop the patches


Cheers,


On 05/06/2014 09:54 PM, Scott Wood wrote:
> On Tue, 2014-05-06 at 00:54 -0500, Emil Medve wrote:
>> Hello Scott,
>>
>>
>> On 05/05/2014 06:25 PM, Scott Wood wrote:
>>> On Sat, 2014-05-03 at 05:02 -0500, Emil Medve wrote:
>>>> Hello Scott,
>>>>
>>>>
>>>> On 04/21/2014 05:11 PM, Scott Wood wrote:
>>>>> On Fri, 2014-04-18 at 07:21 -0500, Shruti Kanetkar wrote:
>>>>>> +fman@400000 {
>>>>>> +	mdio@f1000 {
>>>>>> +		#address-cells = <1>;
>>>>>> +		#size-cells = <0>;
>>>>>> +		compatible = "fsl,fman-xmdio";
>>>>>> +		reg = <0xf1000 0x1000>;
>>>>>> +	};
>>>>>> +};
>>>>>
>>>>> I'd like to see a complete fman binding before we start adding pieces.
>>>>
>>>> The driver for the FMan 10 Gb/s MDIO has upstreamed a couple of years
>>>> ago: '9f35a73 net/fsl: introduce Freescale 10G MDIO driver', granted
>>>> without a binding writeup.
>>>
>>> Pushing driver code through the netdev tree does not establish device
>>> tree ABI.  Binding documents and dts files do.
>>
>> Sure, ideally and formally. But upstreaming a driver represents, if
>> nothing else, a statement of intent to observe a device tree ABI.
> 
> Ideally, yes (or rather, ideally the driver patch should have been
> rejected due to lack of a binding document).  But in practice it's way
> too easy for bad stuff to slip in via driver code, especially when it
> goes via a subsystem maintainer that is different from the one who would
> be reviewing the binding.
> 
>>  Via the SDK, FSL customers are using the device tree ABI the driver de
>> facto establishes.
> 
> ABI of any sort established by the SDK or other non-upstream trees is
> not binding on upstream.  Yes, it's a pain for customers, which is why
> ABI should go upstream ASAP.
> 
>>>> This patch series should probably include a
>>>> binding blurb. However, let's not gate this patchset on a complete
>>>> binding for the FMan
>>>
>>> I at least want to see enough of the FMan binding to have confidence
>>> that what we're adding now is correct.
>>
>> I'm not sure what you're looking for. The nodes we're adding are
>> describing a very common CCSR space interface for quite common device blocks
> 
> ...embedded in a variety of different blocks.
> 
> If the mdio can truly stand alone, then maybe just submit the mdio node
> without being enclosed in an fman node.
> 
>>>> As you know we don't own the FMan work and the FMan work is... not ready
>>>> for upstreaming.
>>>
>>> I'm not asking for a driver, just a binding that describes hardware.  Is
>>> there any reason why the fman node needs to be anywhere near as
>>> complicated as it is in the SDK, if we're limiting it to actual hardware
>>> description?
>>
>> Is this a trick question? :-) Of course it doesn't need to be more
>> complicated than actual hardware. But, to repeat myself, said
>> description is not... ready and I don't know when it will be. Somebody
>> else owns pushing the bulk of FMan upstream and I'd rather not step on
>> their turf quite like this
> 
> If they want to defend their "turf" they can submit a patch.  Now.  This
> has gone on long enough.
> 
> I'm tempted to submit a binding myself.  I don't know much about
> datapath, so I'll probably screw it up.  Please beat me to it. :-)
> 
>>> Do we really need to have nodes for all the sub-blocks?
>>
>> Definitely no, and internally I'm pushing to clean that up. However, you
>> surely remember we've been pushing from the early days of P4080 and it's
>> been, to put it optimistically, slow
> 
> Stop pushing them and start pushing patches.
> 
> Just do it in the right order. :-)
> 
>>>> In an attempt to make some sort of progress we've
>>>> decided to upstream the pieces that are less controversial and MDIO is
>>>> an obvious candidate
>>>>
>>>>>> +fman@400000 {
>>>>>> +	mdio0: mdio@e1120 {
>>>>>> +		#address-cells = <1>;
>>>>>> +		#size-cells = <0>;
>>>>>> +		compatible = "fsl,fman-mdio";
>>>>>> +		reg = <0xe1120 0xee0>;
>>>>>> +	};
>>>>>> +};
>>>>>
>>>>> What is the difference between "fsl,fman-mdio" and "fsl,fman-xmdio"?  I
>>>>> don't see the latter on the list of compatibles in patch 3/6.
>>>>
>>>> 'fsl,fman-mdio' is the 1 Gb/s MDIO (Clause 22 only). 'fsl,fman-xmdio' is
>>>> the 10 Gb/s MDIO (Clause 45 only). We can respin this patch wi
>>>>
>>>
>>> "respin this patch wi..."?
>>
>> Not sure where the end of that sentence went. I meant we'll re-spin with
>> a binding for the 10 Gb/s MDIO block
>>
>>>> I believe 'fsl,fman-mdio' (and others on that list) was added
>>>> gratuitously as the FMan MDIO is completely compatible with the
>>>> eTSEC/gianfar MDIO driver, but we can deal with that later
>>>
>>> It's still good to identify the specific device, even if it's believed
>>> to be 100% compatible.
>>
>> You suggesting we create new compatibles for every instance/integration
>> of a hardware block even though is identical with an earlier hardware
>> integration?
> 
> "100% compatible" is a different statement from actually being identical
> logic.  How do you know that absolutely nothing changed?
> 
>>  Well, I guess that's been done that and now we have about 8
>> different compatibles that convey no real difference at all
> 
> So?  Remember that a node can have multiple compatible strings.  After
> you identify the exact logic being used, you can list older instances
> that are believed to be compatible.
> 
>>>>> Within each category, is the exact fman version discoverable from the
>>>>> mdio registers?
>>>>
>>>> No, but that's irrelevant as that's not the difference between the two
>>>> compatibles
>>>
>>> It's relevant because it means the compatible string should have a block
>>> version number in it, or at least some other way in the MDIO node to
>>> indicate the block version.
>>
>> The 1 Gb/s MDIO block doesn't track a version of its own and from a
>> programming interface perspective it has no visible difference since
>> eTSEC. The 10 Gb/s MDIO doesn't track a version of its own either and
>> across the existing FMan versions is identical from a programming
>> interface perspective
>>
>> I guess we can append a 'v1.0' to the MDIO compatible(s). However, given
>> the SDK we'll have to support the compatibles the (already upstream)
>> drivers support. Dealing with all that legacy is going to be so tedious
> 
> I'm not going to insist that the drivers stop supporting what they
> currently support, but I don't agree with the statement that "given the
> SDK we'll have to...".
> 
>>>>>> +fman@500000 {
>>>>>> +	#address-cells = <1>;
>>>>>> +	#size-cells = <1>;
>>>>>> +	compatible = "simple-bus";
>>>>>
>>>>> Why is this simple-bus?
>>>>
>>>> Because that's the translation type for the FMan sub-nodes.
>>>
>>> What do you mean by "translation type"?
>>
>> I mean address translation across buses
> 
> What translation across buses?
> 
>>>> We need it now to get the MDIO nodes probed
>>>
>>> No.  "simple-bus" is stating an attribute of the hardware, that the
>>> child nodes represent simple memory-mapped devices that can be used
>>> without special bus knowledge.  I don't think that applies here.
>>
>> Yes it does. The FMan sub-nodes are "simple memory-mapped devices that
>> can be used without special bus knowledge". Perhaps you're thinking
>> about the PHY devices on the MDIO bus
> 
> No, I'm not thinking about that.
> 
> What I find particularly disturbing about putting simple-bus on the fman
> node is that it applies to whatever other subnodes get added in the
> future, and we don't yet have any idea what those nodes will be (or at
> least I don't).
> 
>>> You can get the MDIO node probed without misusing simple-bus by adding
>>> the fman node's compatible to the probe list in the kernel code.
>>
>> I think that's gratuitous and it's been done gratuitously in the past
>> for CCSR space (sub-)nodes
> 
> CCSR is a simple-bus, so I'm not sure what you're referring to.
> 
>>> This sort of thing is why I want to see what the rest of the fman
>>> binding will look like.
>>>
>>>>  and we'll needed later to probe other nodes/devices that will have
>>>> standalone drivers: MAC, MURAM. etc. 
>>>
>>> How are they truly standalone?
>>
>> I meant that they have individual drivers and they are not handled by
>> the high-level FMan driver
> 
> That's software description, not hardware description.  Surely those
> drivers cooperate in some manner.
> 
>>> The exist in service to the greater
>>> entity that is fman.  They presumably work together in some fashion.
>>
>> Some blocks can work independently. 
> 
> If any cannot, then simple-bus is wrong.
> 
>> The MURAM is an example and it seems the existing CPM/QE MURAM code
>> allows it to be used as regular memory.
> 
> But it's supposed to be used for CPM/QE/Fman.  If an OS chooses to
> ignore that and bind a generic driver to it, that's the OS's choice, but
> the device tree shouldn't pretend that this is an unrelated bag of
> devices.
> 
>>  The MDIO block could handle
>> PHY(s) for other MACs in the system.
> 
> That doesn't necessarily mean the MDIO is totally independent.  E.g.
> some versions of eTSEC have a TBIPA register that affects the operation
> of the MDIO controller, but is not in the MDIO register area.
> 
>>>>> Where's the compatible?  Why is this file different from all the others?
>>>>
>>>> The FMan v3 MDIO block (supports both Clause 22/45) is compatible with
>>>> the FMan v2 10 Gb/s MDIO (the xgmac-mdio driver). However, the driver
>>>> needs a small clean-up patch (still in internal review) that will get it
>>>> working for FMan v3 MDIO.
>>>
>>> This suggests that it is not 100% backwards compatible.
>>
>> It is. The code is just not everything it should be
> 
> The code works with one v2, and breaks with v3.  Thus, something is
> different.  Whether the difference is enough to prevent claiming
> compatibility depends on the detials (e.g. if the difference is
> parameterized through another DT property, or if the difference is
> within what is allowed by the specs of the original device), but it
> suggests they are not literally the same logic.  Unles the difference is
> not due to v2 versus v3 but some difference elsewhere in the system?
> 
> -Scott

^ permalink raw reply

* Re: [PATCH 4/6] powerpc/corenet: Create the dts components for the DPAA FMan
From: Scott Wood @ 2014-05-08  3:36 UTC (permalink / raw)
  To: Emil Medve; +Cc: devicetree, Shruti Kanetkar, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <536AF8C7.8080304@Freescale.com>

On Wed, 2014-05-07 at 22:23 -0500, Emil Medve wrote:
> Hello Scott,
> 
> 
> At this point, I'm getting a bit lost in this thread and I feel we're
> getting into more generic comments. I'll continue to answer to some of
> your comments to make sure I understand them. However, one major comment
> I feel you made was about seeing a full FMan binding. If that's a deal
> breaker, please let me know and we'll just drop the patches

It's not an absolute deal breaker -- I did suggest the possibility of
providing the mdio node as an ordinary ccsr node, not underneath an fman
container.

I'm just frustrated that getting a proper fman node is such a big deal.
That should have been the first step of the upstreaming process -- not
something that waits until everything is finished.

-Scott

^ permalink raw reply

* Re: [PATCH 4/6] powerpc/corenet: Create the dts components for the DPAA FMan
From: Emil Medve @ 2014-05-08  4:31 UTC (permalink / raw)
  To: Scott Wood; +Cc: devicetree, Shruti Kanetkar, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1399520191.15726.371.camel@snotra.buserror.net>

Hello Scott,


On 05/07/2014 10:36 PM, Scott Wood wrote:
> On Wed, 2014-05-07 at 22:23 -0500, Emil Medve wrote:
>> Hello Scott,
>>
>>
>> At this point, I'm getting a bit lost in this thread and I feel we're
>> getting into more generic comments. I'll continue to answer to some of
>> your comments to make sure I understand them. However, one major comment
>> I feel you made was about seeing a full FMan binding. If that's a deal
>> breaker, please let me know and we'll just drop the patches
> 
> It's not an absolute deal breaker -- I did suggest the possibility of
> providing the mdio node as an ordinary ccsr node, not underneath an fman
> container.

I know you've done something similar in Topaz, but you had other reasons
for it

> I'm just frustrated that getting a proper fman node is such a big deal.
> That should have been the first step of the upstreaming process -- not
> something that waits until everything is finished.

We started upstreaming MDIO (Timur before us) because of this
frustration. We thought this might help in terms of motivation with
making progress on the FMan upstreaming


Cheers,

^ permalink raw reply

* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64
From: Ananth N Mavinakayanahalli @ 2014-05-08  4:47 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Jeremy Fitzhardinge, linux-ia64, sparse,
	Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin,
	Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy,
	Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell,
	Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao,
	Linus Torvalds, rdunlap, Tony Luck, dl9pf, Andrew Morton,
	linuxppc-dev, David S. Miller
In-Reply-To: <20140507115551.22259.70581.stgit@ltc230.yrl.intra.hitachi.co.jp>

On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote:

...

> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
> +/*
> + * On PPC64 ABIv1 the function pointer actually points to the
> + * function's descriptor. The first entry in the descriptor is the
> + * address of the function text.
> + */
> +#define constant_function_entry(fn)	(((func_descr_t *)(fn))->entry)
> +#else
> +#define constant_function_entry(fn)	((unsigned long)(fn))
> +#endif
> +
>  #endif /* __ASSEMBLY__ */

Hi Masami,

You could just use ppc_function_entry() instead.

Ananth

^ permalink raw reply

* Re: [PATCH 5/6] powerpc/corenet: Add DPAA FMan support to the SoC device tree(s)
From: Emil Medve @ 2014-05-08  5:18 UTC (permalink / raw)
  To: Scott Wood
  Cc: devicetree, Kanetkar Shruti-B44454, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1399504442.15726.353.camel@snotra.buserror.net>

Hello Scott,


On 05/07/2014 06:14 PM, Scott Wood wrote:
> On Tue, 2014-05-06 at 01:28 -0500, Emil Medve wrote:
>> Hello Scott,
>>
>>
>> On 05/05/2014 06:34 PM, Scott Wood wrote:
>>> On Sun, 2014-05-04 at 05:59 -0500, Emil Medve wrote:
>>>>  Anyway, most days PHYs can be discovered so they don't use/need
>>>> compatible properties. That's I guess part of the reason we don't have
>>>> bindings for them PHY nodes
>>>
>>> I don't see why there couldn't be a compatible that describes the
>>> standard programming interface.
>>
>> Because it can be detected at runtime and I guess stuff like that should
>> stay out of the device tree. I'm using PCI as an analogy here
> 
> But in this case aren't you using a standardized component of the
> programming model itself to probe the specific PHY type?  I think a
> better analogy is the "cfi-flash" compatible.

Well, to speak your language,
Documentation/devicetree/bindings/net/phy.txt claims the 'compatible' to
be optional, case in which at least 'ethernet-phy-ieee802.3-c22' is
implied. 'ethernet-phy-ieee802.3-c22' (1 Gb/s MDIO/PHY) conveys the
standardized programming model that allows probing


Cheers,

^ permalink raw reply

* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64
From: Masami Hiramatsu @ 2014-05-08  5:40 UTC (permalink / raw)
  To: ananth
  Cc: Jeremy Fitzhardinge, linux-ia64, sparse,
	Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin,
	Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy,
	Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell,
	Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao,
	Linus Torvalds, rdunlap, Tony Luck, dl9pf, Andrew Morton,
	linuxppc-dev, David S. Miller
In-Reply-To: <20140508044753.GA13413@in.ibm.com>

(2014/05/08 13:47), Ananth N Mavinakayanahalli wrote:
> On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote:
> 
> ...
> 
>> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
>> +/*
>> + * On PPC64 ABIv1 the function pointer actually points to the
>> + * function's descriptor. The first entry in the descriptor is the
>> + * address of the function text.
>> + */
>> +#define constant_function_entry(fn)	(((func_descr_t *)(fn))->entry)
>> +#else
>> +#define constant_function_entry(fn)	((unsigned long)(fn))
>> +#endif
>> +
>>  #endif /* __ASSEMBLY__ */
> 
> Hi Masami,
> 
> You could just use ppc_function_entry() instead.

No, I think ppc_function_entry() has two problems (on the latest -next kernel)

At first, that is an inlined functions which is not applied in build time.
Since the NOKPROBE_SYMBOL() is used outside of any functions as like as
EXPORT_SYMBOL(), we can only use preprocessed macros.
Next, on PPC64 ABI*v2*, ppc_function_entry() returns local function entry,
which seems global function entry + 2 insns. I'm not sure about implementation
of the kallsyms on PPC64 ABIv2, but I guess we need global function entry
for kallsyms.

BTW, could you test this patch on the latest -next tree on PPC64 if possible?

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

^ permalink raw reply

* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64
From: Ananth N Mavinakayanahalli @ 2014-05-08  6:16 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Jeremy Fitzhardinge, linux-ia64, sparse,
	Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin,
	Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy,
	Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell,
	Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao,
	Linus Torvalds, rdunlap, Tony Luck, dl9pf, Andrew Morton,
	linuxppc-dev, David S. Miller
In-Reply-To: <536B18B0.9060209@hitachi.com>

On Thu, May 08, 2014 at 02:40:00PM +0900, Masami Hiramatsu wrote:
> (2014/05/08 13:47), Ananth N Mavinakayanahalli wrote:
> > On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote:
> > 
> > ...
> > 
> >> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
> >> +/*
> >> + * On PPC64 ABIv1 the function pointer actually points to the
> >> + * function's descriptor. The first entry in the descriptor is the
> >> + * address of the function text.
> >> + */
> >> +#define constant_function_entry(fn)	(((func_descr_t *)(fn))->entry)
> >> +#else
> >> +#define constant_function_entry(fn)	((unsigned long)(fn))
> >> +#endif
> >> +
> >>  #endif /* __ASSEMBLY__ */
> > 
> > Hi Masami,
> > 
> > You could just use ppc_function_entry() instead.
> 
> No, I think ppc_function_entry() has two problems (on the latest -next kernel)
> 
> At first, that is an inlined functions which is not applied in build time.
> Since the NOKPROBE_SYMBOL() is used outside of any functions as like as
> EXPORT_SYMBOL(), we can only use preprocessed macros.
> Next, on PPC64 ABI*v2*, ppc_function_entry() returns local function entry,
> which seems global function entry + 2 insns. I'm not sure about implementation
> of the kallsyms on PPC64 ABIv2, but I guess we need global function entry
> for kallsyms.

ABIv2 does away with function descriptors and Anton fixed up that
routine to handle the change (the +2 is an artefact of that).

> BTW, could you test this patch on the latest -next tree on PPC64 if possible?

I'll test it, but it may take a bit.

Ananth

^ permalink raw reply

* [PATCH] powerpc: reduce multi-hit of pcibios_setup_device() in hotplug
From: Wei Yang @ 2014-05-08  6:30 UTC (permalink / raw)
  To: gwshan, aik; +Cc: Wei Yang, linuxppc-dev

During the EEH hotplug event, pcibios_setup_device() will be invoked two
times. And the last time will trigger a warning of re-attachment of iommu
group.

The two times are:

    pci_device_add
        ...
        pcibios_add_device
        pcibios_setup_device   <- 1st time
    pcibios_add_pci_devices
        ...
        pcibios_setup_bus_devices
        pcibios_setup_device   <- 2rd time

As we see, in pcibios_add_pci_devices() the pci_bus passed in as a parameter
is initialized and already added in the system. Which means the
pcibios_setup_device() in pcibios_add_device() will be called. Then the
pcibios_setup_device() in pcibios_setup_bus_devices() is the 2nd time to be
called on the same pci_dev.

After applying the patch, the warning of re-attach the iommu group will be
fixed.

[  161.133979] ------------[ cut here ]------------
[  161.134013] WARNING: at arch/powerpc/kernel/iommu.c:1125
[  161.134046] Modules linked in: xt_CHECKSUM bnep bluetooth 6lowpan_iphc rfkill nf_conntrack_netbios_ns nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT xt_conntrack ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw mlx4_ib ib_sa ib_mad ib_core ib_addr bnx2x ses enclosure mlx4_core(O) tg3 be2net ptp pps_core mdio libcrc32c shpchp kvm nfsd nfs_acl lockd sunrpc binfmt_misc uinput lpfc scsi_transport_fc ipr scsi_tgt
[  161.134678] CPU: 23 PID: 650 Comm: eehd Tainted: G           O 3.14.0-rc5yw+ #173
[  161.134727] task: c0000027ed485670 ti: c0000027ed50c000 task.ti: c0000027ed50c000
[  161.134777] NIP: c00000000003ca40 LR: c00000000006c738 CTR: c00000000006c6b0
[  161.134827] REGS: c0000027ed50f470 TRAP: 0700   Tainted: G           O  (3.14.0-rc5yw+)
[  161.134876] MSR: 9000000000029032 <SF,HV,EE,ME,IR,DR,RI>  CR: 88008084  XER: 20000000
[  161.134992] CFAR: c00000000006c734 SOFTE: 1
GPR00: c00000000006c738 c0000027ed50f6f0 c0000000013982f0 c0000027ed464000
GPR04: c0000027e8a87000 c00000000006a980 c0000000016e84a0 0000000000004400
GPR08: c0000000012cd490 0000000000000001 c0000027ed463fff 000000003003b868
GPR12: 0000000028008084 c00000000fdccf00 c0000000000d1970 c000002d74cf0840
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 0000000000000001 000000005360a3b9 00000000000f2d17 c000000fe7659800
GPR28: 0000000000000000 c0000000016e3a80 c0000027e8a87090 c0000027e8a87000
[  161.135624] NIP [c00000000003ca40] .iommu_add_device+0x30/0x1f0
[  161.135668] LR [c00000000006c738] .pnv_pci_ioda_dma_dev_setup+0x88/0xb0
[  161.135709] Call Trace:
[  161.135728] [c0000027ed50f6f0] [c0000027ed50f780] 0xc0000027ed50f780 (unreliable)
[  161.135787] [c0000027ed50f780] [c00000000006c738] .pnv_pci_ioda_dma_dev_setup+0x88/0xb0
[  161.135845] [c0000027ed50f800] [c0000000000699c8] .pnv_pci_dma_dev_setup+0x78/0x310
[  161.135903] [c0000027ed50f8a0] [c000000000044008] .pcibios_setup_device+0x88/0x2f0
[  161.135961] [c0000027ed50f970] [c000000000045d40] .pcibios_setup_bus_devices+0x60/0xd0
[  161.136019] [c0000027ed50f9f0] [c0000000000436dc] .pcibios_add_pci_devices+0xdc/0x1c0
[  161.136078] [c0000027ed50fa80] [c00000000086f4ac] .eeh_reset_device+0x21c/0x2d8
[  161.136136] [c0000027ed50fb40] [c000000000039db8] .eeh_handle_normal_event+0x3b8/0x3f0
[  161.136193] [c0000027ed50fbd0] [c000000000039e38] .eeh_handle_event+0x48/0x320
[  161.136251] [c0000027ed50fc80] [c00000000003a20c] .eeh_event_handler+0xfc/0x1b0
[  161.136309] [c0000027ed50fd30] [c0000000000d1a80] .kthread+0x110/0x130
[  161.136360] [c0000027ed50fe30] [c00000000000a460] .ret_from_kernel_thread+0x5c/0x7c
[  161.136417] Instruction dump:
[  161.136442] 7c0802a6 fba1ffe8 fbc1fff0 fbe1fff8 f8010010 f821ff71 7c7e1b78 60000000
[  161.136524] 60000000 e87e0298 3143ffff 7d2a1910 <0b090000> 2fa90000 40de00c8 ebfe0218
[  161.136608] ---[ end trace b959c4ca6f08c270 ]---
[  161.136641] iommu_tce: device 0003:05:00.0 is already in iommu group 7, skipping

This patch removes the pcibios_setup_bus_devices() in
pcibios_add_pci_devices().

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/pci-hotplug.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index c1e17ae..39a1bac 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -94,7 +94,6 @@ void pcibios_add_pci_devices(struct pci_bus * bus)
 		 */
 		slotno = PCI_SLOT(PCI_DN(dn->child)->devfn);
 		pci_scan_slot(bus, PCI_DEVFN(slotno, 0));
-		pcibios_setup_bus_devices(bus);
 		max = bus->busn_res.start;
 		for (pass = 0; pass < 2; pass++) {
 			list_for_each_entry(dev, &bus->devices, bus_list) {
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 1/2] bootmem/powerpc: Unify bootmem initialization
From: Emil Medve @ 2014-05-08  7:05 UTC (permalink / raw)
  To: benh, linuxppc-dev; +Cc: Emil Medve

Unify the low/highmem code path from do_init_bootmem() by using (the)
lowmem related variables/parameters even when the low/highmem split
is not needed (64-bit) or configured. In such cases the "lowmem"
variables/parameters continue to observe the definition by referring
to memory directly mapped by the kernel

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---

v2: Rebased, no changes

v3: No changes

 arch/powerpc/mm/mem.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 32202c9..eaf5d1d8 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -188,27 +188,31 @@ EXPORT_SYMBOL_GPL(walk_system_ram_range);
 void __init do_init_bootmem(void)
 {
 	unsigned long start, bootmap_pages;
-	unsigned long total_pages;
 	struct memblock_region *reg;
 	int boot_mapsize;
+	phys_addr_t _total_lowmem;
+	phys_addr_t _lowmem_end_addr;
 
-	max_low_pfn = max_pfn = memblock_end_of_DRAM() >> PAGE_SHIFT;
-	total_pages = (memblock_end_of_DRAM() - memstart_addr) >> PAGE_SHIFT;
-#ifdef CONFIG_HIGHMEM
-	total_pages = total_lowmem >> PAGE_SHIFT;
-	max_low_pfn = lowmem_end_addr >> PAGE_SHIFT;
+#ifndef CONFIG_HIGHMEM
+	_lowmem_end_addr = memblock_end_of_DRAM();
+#else
+	_lowmem_end_addr = lowmem_end_addr;
 #endif
 
+	max_pfn = memblock_end_of_DRAM() >> PAGE_SHIFT;
+	max_low_pfn = _lowmem_end_addr >> PAGE_SHIFT;
+	min_low_pfn = MEMORY_START >> PAGE_SHIFT;
+
 	/*
 	 * Find an area to use for the bootmem bitmap.  Calculate the size of
 	 * bitmap required as (Total Memory) / PAGE_SIZE / BITS_PER_BYTE.
 	 * Add 1 additional page in case the address isn't page-aligned.
 	 */
-	bootmap_pages = bootmem_bootmap_pages(total_pages);
+	_total_lowmem = _lowmem_end_addr - memstart_addr;
+	bootmap_pages = bootmem_bootmap_pages(_total_lowmem >> PAGE_SHIFT);
 
 	start = memblock_alloc(bootmap_pages << PAGE_SHIFT, PAGE_SIZE);
 
-	min_low_pfn = MEMORY_START >> PAGE_SHIFT;
 	boot_mapsize = init_bootmem_node(NODE_DATA(0), start >> PAGE_SHIFT, min_low_pfn, max_low_pfn);
 
 	/* Place all memblock_regions in the same node and merge contiguous
@@ -219,26 +223,18 @@ void __init do_init_bootmem(void)
 	/* Add all physical memory to the bootmem map, mark each area
 	 * present.
 	 */
-#ifdef CONFIG_HIGHMEM
-	free_bootmem_with_active_regions(0, lowmem_end_addr >> PAGE_SHIFT);
+	free_bootmem_with_active_regions(0, max_low_pfn);
 
 	/* reserve the sections we're already using */
 	for_each_memblock(reserved, reg) {
-		unsigned long top = reg->base + reg->size - 1;
-		if (top < lowmem_end_addr)
+		if (reg->base + reg->size - 1 < _lowmem_end_addr)
 			reserve_bootmem(reg->base, reg->size, BOOTMEM_DEFAULT);
-		else if (reg->base < lowmem_end_addr) {
-			unsigned long trunc_size = lowmem_end_addr - reg->base;
+		else if (reg->base < _lowmem_end_addr) {
+			unsigned long trunc_size = _lowmem_end_addr - reg->base;
 			reserve_bootmem(reg->base, trunc_size, BOOTMEM_DEFAULT);
 		}
 	}
-#else
-	free_bootmem_with_active_regions(0, max_pfn);
 
-	/* reserve the sections we're already using */
-	for_each_memblock(reserved, reg)
-		reserve_bootmem(reg->base, reg->size, BOOTMEM_DEFAULT);
-#endif
 	/* XXX need to clip this if using highmem? */
 	sparse_memory_present_with_active_regions(0);
 
-- 
1.9.2

^ permalink raw reply related

* [PATCH 2/2] powerpc: Enable NO_BOOTMEM
From: Emil Medve @ 2014-05-08  7:05 UTC (permalink / raw)
  To: benh, linuxppc-dev; +Cc: Emil Medve
In-Reply-To: <1399532702-1514-1-git-send-email-Emilian.Medve@Freescale.com>

Currently bootmem is just a wrapper around/on top of memblock. This
eliminates from the build/kernel image the bootmem code and the
initialization wrapper code just as other ARHC(es) did: x86, arm,
etc

For now only cover !NUMA systems

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---

v2: Acknowledge that NUMA systems/builds are not covered by this patch

v3: Don't re-define NO_BOOTMEM
    Update the commit message

 arch/powerpc/Kconfig  | 1 +
 arch/powerpc/mm/mem.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e099899..3499303 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -105,6 +105,7 @@ config PPC
 	select HAVE_ARCH_KGDB
 	select HAVE_KRETPROBES
 	select HAVE_ARCH_TRACEHOOK
+	select NO_BOOTMEM if !NUMA
 	select HAVE_MEMBLOCK
 	select HAVE_MEMBLOCK_NODE_MAP
 	select HAVE_DMA_ATTRS
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index eaf5d1d8..d3e1d5f 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -187,10 +187,12 @@ EXPORT_SYMBOL_GPL(walk_system_ram_range);
 #ifndef CONFIG_NEED_MULTIPLE_NODES
 void __init do_init_bootmem(void)
 {
+#ifndef CONFIG_NO_BOOTMEM
 	unsigned long start, bootmap_pages;
 	struct memblock_region *reg;
 	int boot_mapsize;
 	phys_addr_t _total_lowmem;
+#endif
 	phys_addr_t _lowmem_end_addr;
 
 #ifndef CONFIG_HIGHMEM
@@ -203,6 +205,7 @@ void __init do_init_bootmem(void)
 	max_low_pfn = _lowmem_end_addr >> PAGE_SHIFT;
 	min_low_pfn = MEMORY_START >> PAGE_SHIFT;
 
+#ifndef CONFIG_NO_BOOTMEM
 	/*
 	 * Find an area to use for the bootmem bitmap.  Calculate the size of
 	 * bitmap required as (Total Memory) / PAGE_SIZE / BITS_PER_BYTE.
@@ -214,12 +217,14 @@ void __init do_init_bootmem(void)
 	start = memblock_alloc(bootmap_pages << PAGE_SHIFT, PAGE_SIZE);
 
 	boot_mapsize = init_bootmem_node(NODE_DATA(0), start >> PAGE_SHIFT, min_low_pfn, max_low_pfn);
+#endif
 
 	/* Place all memblock_regions in the same node and merge contiguous
 	 * memblock_regions
 	 */
 	memblock_set_node(0, (phys_addr_t)ULLONG_MAX, &memblock.memory, 0);
 
+#ifndef CONFIG_NO_BOOTMEM
 	/* Add all physical memory to the bootmem map, mark each area
 	 * present.
 	 */
@@ -234,11 +239,14 @@ void __init do_init_bootmem(void)
 			reserve_bootmem(reg->base, trunc_size, BOOTMEM_DEFAULT);
 		}
 	}
+#endif
 
 	/* XXX need to clip this if using highmem? */
 	sparse_memory_present_with_active_regions(0);
 
+#ifndef CONFIG_NO_BOOTMEM
 	init_bootmem_done = 1;
+#endif
 }
 
 /* mark pages that don't exist as nosave */
-- 
1.9.2

^ permalink raw reply related

* [PATCH 2/2 v3] powerpc: Enable NO_BOOTMEM
From: Emil Medve @ 2014-05-08  7:06 UTC (permalink / raw)
  To: benh, linuxppc-dev; +Cc: Emil Medve
In-Reply-To: <1399532799-1557-1-git-send-email-Emilian.Medve@Freescale.com>

Currently bootmem is just a wrapper around/on top of memblock. This
eliminates from the build/kernel image the bootmem code and the
initialization wrapper code just as other ARHC(es) did: x86, arm,
etc

For now only cover !NUMA systems

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---

v2: Acknowledge that NUMA systems/builds are not covered by this patch

v3: Don't re-define NO_BOOTMEM
    Update the commit message

 arch/powerpc/Kconfig  | 1 +
 arch/powerpc/mm/mem.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e099899..3499303 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -105,6 +105,7 @@ config PPC
 	select HAVE_ARCH_KGDB
 	select HAVE_KRETPROBES
 	select HAVE_ARCH_TRACEHOOK
+	select NO_BOOTMEM if !NUMA
 	select HAVE_MEMBLOCK
 	select HAVE_MEMBLOCK_NODE_MAP
 	select HAVE_DMA_ATTRS
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index eaf5d1d8..d3e1d5f 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -187,10 +187,12 @@ EXPORT_SYMBOL_GPL(walk_system_ram_range);
 #ifndef CONFIG_NEED_MULTIPLE_NODES
 void __init do_init_bootmem(void)
 {
+#ifndef CONFIG_NO_BOOTMEM
 	unsigned long start, bootmap_pages;
 	struct memblock_region *reg;
 	int boot_mapsize;
 	phys_addr_t _total_lowmem;
+#endif
 	phys_addr_t _lowmem_end_addr;
 
 #ifndef CONFIG_HIGHMEM
@@ -203,6 +205,7 @@ void __init do_init_bootmem(void)
 	max_low_pfn = _lowmem_end_addr >> PAGE_SHIFT;
 	min_low_pfn = MEMORY_START >> PAGE_SHIFT;
 
+#ifndef CONFIG_NO_BOOTMEM
 	/*
 	 * Find an area to use for the bootmem bitmap.  Calculate the size of
 	 * bitmap required as (Total Memory) / PAGE_SIZE / BITS_PER_BYTE.
@@ -214,12 +217,14 @@ void __init do_init_bootmem(void)
 	start = memblock_alloc(bootmap_pages << PAGE_SHIFT, PAGE_SIZE);
 
 	boot_mapsize = init_bootmem_node(NODE_DATA(0), start >> PAGE_SHIFT, min_low_pfn, max_low_pfn);
+#endif
 
 	/* Place all memblock_regions in the same node and merge contiguous
 	 * memblock_regions
 	 */
 	memblock_set_node(0, (phys_addr_t)ULLONG_MAX, &memblock.memory, 0);
 
+#ifndef CONFIG_NO_BOOTMEM
 	/* Add all physical memory to the bootmem map, mark each area
 	 * present.
 	 */
@@ -234,11 +239,14 @@ void __init do_init_bootmem(void)
 			reserve_bootmem(reg->base, trunc_size, BOOTMEM_DEFAULT);
 		}
 	}
+#endif
 
 	/* XXX need to clip this if using highmem? */
 	sparse_memory_present_with_active_regions(0);
 
+#ifndef CONFIG_NO_BOOTMEM
 	init_bootmem_done = 1;
+#endif
 }
 
 /* mark pages that don't exist as nosave */
-- 
1.9.2

^ permalink raw reply related

* [PATCH 1/2 v3] bootmem/powerpc: Unify bootmem initialization
From: Emil Medve @ 2014-05-08  7:06 UTC (permalink / raw)
  To: benh, linuxppc-dev; +Cc: Emil Medve

Unify the low/highmem code path from do_init_bootmem() by using (the)
lowmem related variables/parameters even when the low/highmem split
is not needed (64-bit) or configured. In such cases the "lowmem"
variables/parameters continue to observe the definition by referring
to memory directly mapped by the kernel

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---

v2: Rebased, no changes

v3: No changes

 arch/powerpc/mm/mem.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 32202c9..eaf5d1d8 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -188,27 +188,31 @@ EXPORT_SYMBOL_GPL(walk_system_ram_range);
 void __init do_init_bootmem(void)
 {
 	unsigned long start, bootmap_pages;
-	unsigned long total_pages;
 	struct memblock_region *reg;
 	int boot_mapsize;
+	phys_addr_t _total_lowmem;
+	phys_addr_t _lowmem_end_addr;
 
-	max_low_pfn = max_pfn = memblock_end_of_DRAM() >> PAGE_SHIFT;
-	total_pages = (memblock_end_of_DRAM() - memstart_addr) >> PAGE_SHIFT;
-#ifdef CONFIG_HIGHMEM
-	total_pages = total_lowmem >> PAGE_SHIFT;
-	max_low_pfn = lowmem_end_addr >> PAGE_SHIFT;
+#ifndef CONFIG_HIGHMEM
+	_lowmem_end_addr = memblock_end_of_DRAM();
+#else
+	_lowmem_end_addr = lowmem_end_addr;
 #endif
 
+	max_pfn = memblock_end_of_DRAM() >> PAGE_SHIFT;
+	max_low_pfn = _lowmem_end_addr >> PAGE_SHIFT;
+	min_low_pfn = MEMORY_START >> PAGE_SHIFT;
+
 	/*
 	 * Find an area to use for the bootmem bitmap.  Calculate the size of
 	 * bitmap required as (Total Memory) / PAGE_SIZE / BITS_PER_BYTE.
 	 * Add 1 additional page in case the address isn't page-aligned.
 	 */
-	bootmap_pages = bootmem_bootmap_pages(total_pages);
+	_total_lowmem = _lowmem_end_addr - memstart_addr;
+	bootmap_pages = bootmem_bootmap_pages(_total_lowmem >> PAGE_SHIFT);
 
 	start = memblock_alloc(bootmap_pages << PAGE_SHIFT, PAGE_SIZE);
 
-	min_low_pfn = MEMORY_START >> PAGE_SHIFT;
 	boot_mapsize = init_bootmem_node(NODE_DATA(0), start >> PAGE_SHIFT, min_low_pfn, max_low_pfn);
 
 	/* Place all memblock_regions in the same node and merge contiguous
@@ -219,26 +223,18 @@ void __init do_init_bootmem(void)
 	/* Add all physical memory to the bootmem map, mark each area
 	 * present.
 	 */
-#ifdef CONFIG_HIGHMEM
-	free_bootmem_with_active_regions(0, lowmem_end_addr >> PAGE_SHIFT);
+	free_bootmem_with_active_regions(0, max_low_pfn);
 
 	/* reserve the sections we're already using */
 	for_each_memblock(reserved, reg) {
-		unsigned long top = reg->base + reg->size - 1;
-		if (top < lowmem_end_addr)
+		if (reg->base + reg->size - 1 < _lowmem_end_addr)
 			reserve_bootmem(reg->base, reg->size, BOOTMEM_DEFAULT);
-		else if (reg->base < lowmem_end_addr) {
-			unsigned long trunc_size = lowmem_end_addr - reg->base;
+		else if (reg->base < _lowmem_end_addr) {
+			unsigned long trunc_size = _lowmem_end_addr - reg->base;
 			reserve_bootmem(reg->base, trunc_size, BOOTMEM_DEFAULT);
 		}
 	}
-#else
-	free_bootmem_with_active_regions(0, max_pfn);
 
-	/* reserve the sections we're already using */
-	for_each_memblock(reserved, reg)
-		reserve_bootmem(reg->base, reg->size, BOOTMEM_DEFAULT);
-#endif
 	/* XXX need to clip this if using highmem? */
 	sparse_memory_present_with_active_regions(0);
 
-- 
1.9.2

^ permalink raw reply related

* [RFC PATCH] powerpc: fix wrong sp saved in save_stack_trace()
From: Li Zhong @ 2014-05-08  9:01 UTC (permalink / raw)
  To: PowerPC email list; +Cc: Paul Mackerras

I found stack trace couldn't be saved sometimes. After some
investigation, it seems that when function trace is enabled, 

void save_stack_trace(struct stack_trace *trace)
{
        unsigned long sp;

        asm("mr %0,1" : "=r" (sp));

        save_context_stack(trace, sp, current, 1);
}

is compiled into: 

c0000000000432c0 <.save_stack_trace>:
c0000000000432c0:       7c 08 02 a6     mflr    r0
c0000000000432c4:       f8 01 00 10     std     r0,16(r1)
c0000000000432c8:       f8 21 ff 81     stdu    r1,-128(r1)
c0000000000432cc:       f8 61 00 70     std     r3,112(r1)
c0000000000432d0:       4b fc 77 bd     bl      c00000000000aa8c
<._mcount>
c0000000000432d4:       60 00 00 00     nop
c0000000000432d8:       7c 24 0b 78     mr      r4,r1

c0000000000432dc:       e8 ad 02 78     ld      r5,632(r13)
c0000000000432e0:       e8 61 00 70     ld      r3,112(r1)
c0000000000432e4:       38 c0 00 01     li      r6,1
c0000000000432e8:       38 21 00 80     addi    r1,r1,128
c0000000000432ec:       e8 01 00 10     ld      r0,16(r1)
c0000000000432f0:       7c 08 03 a6     mtlr    r0
c0000000000432f4:       4b ff fe 5c     b       c000000000043150
<.save_context_stack>
c0000000000432f8:       60 00 00 00     nop
c0000000000432fc:       60 42 00 00     ori     r2,r2,0

new stack frame -128(r1) is created to call ._mcount, and this new r1 is
copied into sp as the stack pointer, which then could be overwritten by
save_context_stack's prolog. 

I don't know how to specify in C that the embedded asm be compiled after
r1 being added back to the original value. But as a workaround, maybe we
could move this embedded asm into save_context_stack(). 

Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/stacktrace.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index 3d30ef1..5c0b461 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -22,6 +22,9 @@
 static void save_context_stack(struct stack_trace *trace, unsigned long sp,
 			struct task_struct *tsk, int savesched)
 {
+	if (tsk == current)
+		asm("mr %0,1" : "=r" (sp));
+
 	for (;;) {
 		unsigned long *stack = (unsigned long *) sp;
 		unsigned long newsp, ip;
@@ -48,11 +51,7 @@ static void save_context_stack(struct stack_trace *trace, unsigned long sp,
 
 void save_stack_trace(struct stack_trace *trace)
 {
-	unsigned long sp;
-
-	asm("mr %0,1" : "=r" (sp));
-
-	save_context_stack(trace, sp, current, 1);
+	save_context_stack(trace, 0, current, 1);
 }
 EXPORT_SYMBOL_GPL(save_stack_trace);
 

^ permalink raw reply related

* [PATCH V4 0/2] mm: FAULT_AROUND_ORDER patchset performance data for powerpc
From: Madhavan Srinivasan @ 2014-05-08  9:28 UTC (permalink / raw)
  To: linux-kernel, linuxppc-dev, linux-mm, linux-arch, x86
  Cc: riel, ak, peterz, rusty, dave.hansen, Madhavan Srinivasan, paulus,
	mgorman, akpm, mingo, kirill.shutemov

Kirill A. Shutemov with 8c6e50b029 commit introduced
vm_ops->map_pages() for mapping easy accessible pages around
fault address in hope to reduce number of minor page faults.

This patch creates infrastructure to modify the FAULT_AROUND_ORDER
value using mm/Kconfig. This will enable architecture maintainers
to decide on suitable FAULT_AROUND_ORDER value based on
performance data for that architecture. First patch also defaults
FAULT_AROUND_ORDER Kconfig element to 4. Second patch list
out the performance numbers for powerpc (platform pseries) and
initialize the fault around order variable for pseries platform of
powerpc.

V4 Changes:
  Replaced the BUILD_BUG_ON with VM_BUG_ON.
  Moved fault_around_pages() and fault_around_mask() functions outside of
   #ifdef CONFIG_DEBUG_FS.

V3 Changes:
  Replaced FAULT_AROUND_ORDER macro to a variable to support arch's that
   supports sub platforms.
  Made changes in commit messages.

V2 Changes:
  Created Kconfig parameter for FAULT_AROUND_ORDER
  Added check in do_read_fault to handle FAULT_AROUND_ORDER value of 0
  Made changes in commit messages.

Madhavan Srinivasan (2):
  mm: move FAULT_AROUND_ORDER to arch/
  powerpc/pseries: init fault_around_order for pseries

 arch/powerpc/platforms/pseries/pseries.h |    2 ++
 arch/powerpc/platforms/pseries/setup.c   |    5 +++++
 mm/Kconfig                               |    8 ++++++++
 mm/memory.c                              |   25 ++++++-------------------
 4 files changed, 21 insertions(+), 19 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH V4 2/2] powerpc/pseries: init fault_around_order for pseries
From: Madhavan Srinivasan @ 2014-05-08  9:28 UTC (permalink / raw)
  To: linux-kernel, linuxppc-dev, linux-mm, linux-arch, x86
  Cc: riel, ak, peterz, rusty, dave.hansen, Madhavan Srinivasan, paulus,
	mgorman, akpm, mingo, kirill.shutemov
In-Reply-To: <1399541296-18810-1-git-send-email-maddy@linux.vnet.ibm.com>

Performance data for different FAULT_AROUND_ORDER values from 4 socket
Power7 system (128 Threads and 128GB memory). perf stat with repeat of 5
is used to get the stddev values. Test ran in v3.14 kernel (Baseline) and
v3.15-rc1 for different fault around order values. %change here is calculated
in this method ((new value - baseline)/baseline). And negative %change says
its a drop in time.

FAULT_AROUND_ORDER      Baseline        1               3               4               5               8

Linux build (make -j64)
minor-faults            47,437,359      35,279,286      25,425,347      23,461,275      22,002,189      21,435,836
times in seconds        347.302528420   344.061588460   340.974022391   348.193508116   348.673900158   350.986543618
 stddev for time        ( +-  1.50% )   ( +-  0.73% )   ( +-  1.13% )   ( +-  1.01% )   ( +-  1.89% )   ( +-  1.55% )
 %chg time to baseline                  -0.9%           -1.8%           0.2%            0.39%           1.06%

Linux rebuild (make -j64)
minor-faults            941,552         718,319         486,625         440,124         410,510         397,416
times in seconds        30.569834718    31.219637539    31.319370649    31.434285472    31.972367174    31.443043580
 stddev for time        ( +-  1.07% )   ( +-  0.13% )   ( +-  0.43% )   ( +-  0.18% )   ( +-  0.95% )   ( +-  0.58% )
 %chg time to baseline                  2.1%            2.4%            2.8%            4.58%           2.85%

Binutils build (make all -j64 )
minor-faults            474,821         371,380         269,463         247,715         235,255         228,337
times in seconds        53.882492432    53.584289348    53.882773216    53.755816431    53.607824348    53.423759642
 stddev for time        ( +-  0.08% )   ( +-  0.56% )   ( +-  0.17% )   ( +-  0.11% )   ( +-  0.60% )   ( +-  0.69% )
 %chg time to baseline                  -0.55%          0.0%            -0.23%          -0.51%          -0.85%

Two synthetic tests: access every word in file in sequential/random order.

Sequential access 16GiB file
FAULT_AROUND_ORDER      Baseline        1               3               4               5               8
1 thread
       minor-faults     263,148         131,166         32,908          16,514          8,260           1,093
       times in seconds 53.091138345    53.113191672    53.188776177    53.233017218    53.206841347    53.429979442
       stddev for time  ( +-  0.06% )   ( +-  0.07% )   ( +-  0.08% )   ( +-  0.09% )   ( +-  0.03% )   ( +-  0.03% )
       %chg time to baseline            0.04%           0.18%           0.26%           0.21%           0.63%
8 threads
       minor-faults     2,097,267       1,048,753       262,237         131,397         65,621          8,274
       times in seconds 55.173790028    54.591880790    54.824623287    54.802162211    54.969680503    54.790387715
       stddev for time  ( +-  0.78% )   ( +-  0.09% )   ( +-  0.08% )   ( +-  0.07% )   ( +-  0.28% )   ( +-  0.05% )
       %chg time to baseline            -1.05%          -0.63%          -0.67%          -0.36%          -0.69%
32 threads
       minor-faults     8,388,751       4,195,621       1,049,664       525,461         262,535         32,924
       times in seconds 60.431573046    60.669110744    60.485336388    60.697789706    60.077959564    60.588855032
       stddev for time  ( +-  0.44% )   ( +-  0.27% )   ( +-  0.46% )   ( +-  0.67% )   ( +-  0.31% )   ( +-  0.49% )
       %chg time to baseline            0.39%           0.08%           0.44%           -0.58%          0.25%
64 threads
       minor-faults     16,777,409      8,607,527       2,289,766       1,202,264       598,405         67,587
       times in seconds 96.932617720    100.675418760   102.109880836   103.881733383   102.580199555   105.751194041
       stddev for time  ( +-  1.39% )   ( +-  1.06% )   ( +-  0.99% )   ( +-  0.76% )   ( +-  1.65% )   ( +-  1.60% )
       %chg time to baseline            3.86%           5.34%           7.16%           5.82%           9.09%
128 threads
       minor-faults     33,554,705      17,375,375      4,682,462       2,337,245       1,179,007       134,819
       times in seconds 128.766704495   115.659225437   120.353046307   115.291871270   115.450886036   113.991902150
       stddev for time  ( +-  2.93% )   ( +-  0.30% )   ( +-  2.93% )   ( +-  1.24% )   ( +-  1.03% )   ( +-  0.70% )
       %chg time to baseline            -10.17%         -6.53%          -10.46%         -10.34%         -11.47%

Random access 1GiB file
FAULT_AROUND_ORDER      Baseline        1               3               4               5               8
1 thread
       minor-faults     17,155          8,678           2,126           1,097           581             134
       times in seconds 51.904430523    51.658017987    51.919270792    51.560531738    52.354431597    51.976469502
       stddev for time  ( +-  3.19% )   ( +-  1.35% )   ( +-  1.56% )   ( +-  0.91% )   ( +-  1.70% )   ( +-  2.02% )
       %chg time to baseline            -0.47%          0.02%           -0.66%          0.86%           0.13%
8 threads
       minor-faults     131,844         70,705          17,457          8,505           4,251           598
       times in seconds 58.162813956    54.991706305    54.952675791    55.323057492    54.755587379    53.376722828
       stddev for time  ( +-  1.44% )   ( +-  0.69% )   ( +-  1.23% )   ( +-  2.78% )   ( +-  1.90% )   ( +-  2.91% )
       %chg time to baseline            -5.45%          -5.52%          -4.88%          -5.86%          -8.22%
32 threads
       minor-faults     524,437         270,760         67,069          33,414          16,641          2,204
       times in seconds 69.981777072    76.539570015    79.753578505    76.245943618    77.254258344    79.072596831
       stddev for time  ( +-  2.81% )   ( +-  1.95% )   ( +-  2.66% )   ( +-  0.99% )   ( +-  2.35% )   ( +-  3.22% )
       %chg time to baseline            9.37%           13.96%          8.95%           10.39%          12.98%
64 threads
       minor-faults     1,049,117       527,451         134,016         66,638          33,391          4,559
       times in seconds 108.024517536   117.575067996   115.322659914   111.943998437   115.049450815   119.218450840
       stddev for time  ( +-  2.40% )   ( +-  1.77% )   ( +-  1.19% )   ( +-  3.29% )   ( +-  2.32% )   ( +-  1.42% )
       %chg time to baseline            8.84%           6.75%           3.62%           6.5%            10.3%
128 threads
       minor-faults     2,097,440       1,054,360       267,042         133,328         66,532          8,652
       times in seconds 155.055861167   153.059625968   152.449492156   151.024005282   150.844647770   155.954366718
       stddev for time  ( +-  1.32% )   ( +-  1.14% )   ( +-  1.32% )   ( +-  0.81% )   ( +-  0.75% )   ( +-  0.72% )
       %chg time to baseline            -1.28%          -1.68%          -2.59%          -2.71%          0.57%

In case of kernel build, fault around order (fao) value of 1 and 3 wins when compared to 4 (but bit noisy).
Incase of kernel rebuild, slowdown for fao > 0 is seen. Incase of synthetic test, there are sporadic agains, but mostly
slowdown. No clear sweet spot fao value that can be suggested for the ppc64/pseries with the current
performance data. Hence, patch suggest value of zero to the fao.

Worst case scenario: we touch one page every 16M to demonstrate overhead.

Touch only one page in page table in 16GiB file
FAULT_AROUND_ORDER      Baseline        1               3               4               5               8
1 thread
       minor-faults     1,104           1,090           1,071           1,068           1,065           1,063
       times in seconds 0.006583298     0.008531502     0.019733795     0.036033763     0.062300553     0.406857086
       stddev for time  ( +-  2.79% )   ( +-  2.42% )   ( +-  3.47% )   ( +-  2.81% )   ( +-  2.01% )   ( +-  1.33% )
8 threads
       minor-faults     8,279           8,264           8,245           8,243           8,239           8,240
       times in seconds 0.044572398     0.057211811     0.107606306     0.205626815     0.381679120     2.647979955
       stddev for time  ( +-  1.95% )   ( +-  2.98% )   ( +-  1.74% )   ( +-  2.80% )   ( +-  2.01% )   ( +-  1.86% )
32 threads
       minor-faults     32,879          32,864          32,849          32,845          32,839          32,843
       times in seconds 0.197659343     0.218486087     0.445116407     0.694235883     1.296894038     9.127517045
       stddev for time  ( +-  3.05% )   ( +-  3.05% )   ( +-  4.33% )   ( +-  3.08% )   ( +-  3.75% )   ( +-  0.56% )
64 threads
       minor-faults     65,680          65,664          65,646          65,645          65,640          65,647
       times in seconds 0.455537304     0.489688780     0.866490093     1.427393118     2.379628982     17.059295051
       stddev for time  ( +-  4.01% )   ( +-  4.13% )   ( +-  2.92% )   ( +-  1.68% )   ( +-  1.79% )   ( +-  0.48% )
128 threads
       minor-faults     131,279         131,265         131,250         131,245         131,241         131,254
       times in seconds 1.026880651     1.095327536     1.721728274     2.808233068     4.662729948     31.732848290
       stddev for time  ( +-  6.85% )   ( +-  4.09% )   ( +-  1.71% )   ( +-  3.45% )   ( +-  2.40% )   ( +-  0.68% )

Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/pseries.h |    2 ++
 arch/powerpc/platforms/pseries/setup.c   |    5 +++++
 2 files changed, 7 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 9921953..6e6c993 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -17,6 +17,8 @@ struct device_node;
 extern void request_event_sources_irqs(struct device_node *np,
 				       irq_handler_t handler, const char *name);
 
+extern unsigned int fault_around_order;
+
 #include <linux/of.h>
 
 extern void __init fw_hypertas_feature_init(const char *hypertas,
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 2db8cc6..4391c3c 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -465,6 +465,11 @@ static void __init pSeries_setup_arch(void)
 {
 	set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);
 
+	/*
+	 * Defaulting to zero since no sweet spot value found in the performance test.
+	 */
+	fault_around_order = 0;
+
 	/* Discover PIC type and setup ppc_md accordingly */
 	pseries_discover_pic();
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH V4 1/2] mm: move FAULT_AROUND_ORDER to arch/
From: Madhavan Srinivasan @ 2014-05-08  9:28 UTC (permalink / raw)
  To: linux-kernel, linuxppc-dev, linux-mm, linux-arch, x86
  Cc: riel, ak, peterz, rusty, dave.hansen, Madhavan Srinivasan, paulus,
	mgorman, akpm, mingo, kirill.shutemov
In-Reply-To: <1399541296-18810-1-git-send-email-maddy@linux.vnet.ibm.com>

Kirill A. Shutemov with 8c6e50b029 commit introduced
vm_ops->map_pages() for mapping easy accessible pages around
fault address in hope to reduce number of minor page faults.

This patch creates infrastructure to modify the FAULT_AROUND_ORDER
value using mm/Kconfig. This will enable architecture maintainers
to decide on suitable FAULT_AROUND_ORDER value based on
performance data for that architecture. Patch also defaults
FAULT_AROUND_ORDER Kconfig element to 4.

Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
 mm/Kconfig  |    8 ++++++++
 mm/memory.c |   25 ++++++-------------------
 2 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index ebe5880..c7fc4f1 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -176,6 +176,14 @@ config MOVABLE_NODE
 config HAVE_BOOTMEM_INFO_NODE
 	def_bool n
 
+#
+# Fault around order is a control knob to decide the fault around pages.
+# Default value is set to 4 , but the arch can override it as desired.
+#
+config FAULT_AROUND_ORDER
+	int
+	default	4
+
 # eventually, we can have this option just 'select SPARSEMEM'
 config MEMORY_HOTPLUG
 	bool "Allow for memory hot-add"
diff --git a/mm/memory.c b/mm/memory.c
index 037b812..e3931ef 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3402,11 +3402,9 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address,
 	update_mmu_cache(vma, address, pte);
 }
 
-#define FAULT_AROUND_ORDER 4
+unsigned int fault_around_order __read_mostly = CONFIG_FAULT_AROUND_ORDER;
 
 #ifdef CONFIG_DEBUG_FS
-static unsigned int fault_around_order = FAULT_AROUND_ORDER;
-
 static int fault_around_order_get(void *data, u64 *val)
 {
 	*val = fault_around_order;
@@ -3415,7 +3413,6 @@ static int fault_around_order_get(void *data, u64 *val)
 
 static int fault_around_order_set(void *data, u64 val)
 {
-	BUILD_BUG_ON((1UL << FAULT_AROUND_ORDER) > PTRS_PER_PTE);
 	if (1UL << val > PTRS_PER_PTE)
 		return -EINVAL;
 	fault_around_order = val;
@@ -3435,31 +3432,21 @@ static int __init fault_around_debugfs(void)
 	return 0;
 }
 late_initcall(fault_around_debugfs);
+#endif
 
 static inline unsigned long fault_around_pages(void)
 {
-	return 1UL << fault_around_order;
-}
-
-static inline unsigned long fault_around_mask(void)
-{
-	return ~((1UL << (PAGE_SHIFT + fault_around_order)) - 1);
-}
-#else
-static inline unsigned long fault_around_pages(void)
-{
 	unsigned long nr_pages;
 
-	nr_pages = 1UL << FAULT_AROUND_ORDER;
-	BUILD_BUG_ON(nr_pages > PTRS_PER_PTE);
+	nr_pages = 1UL << fault_around_order;
+	VM_BUG_ON(nr_pages > PTRS_PER_PTE);
 	return nr_pages;
 }
 
 static inline unsigned long fault_around_mask(void)
 {
-	return ~((1UL << (PAGE_SHIFT + FAULT_AROUND_ORDER)) - 1);
+	return ~((1UL << (PAGE_SHIFT + fault_around_order)) - 1);
 }
-#endif
 
 static void do_fault_around(struct vm_area_struct *vma, unsigned long address,
 		pte_t *pte, pgoff_t pgoff, unsigned int flags)
@@ -3515,7 +3502,7 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 	 * if page by the offset is not ready to be mapped (cold cache or
 	 * something).
 	 */
-	if (vma->vm_ops->map_pages) {
+	if ((vma->vm_ops->map_pages) && fault_around_order) {
 		pte = pte_offset_map_lock(mm, pmd, address, &ptl);
 		do_fault_around(vma, address, pte, pgoff, flags);
 		if (!pte_same(*pte, orig_pte))
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH RFC v12 3/7] dma: mpc512x: add support for peripheral transfers
From: Alexander Popov @ 2014-05-08  9:49 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Lars-Peter Clausen, Arnd Bergmann, Gerhard Sittig,
	Andy Shevchenko, Alexander Popov, dmaengine, Dan Williams,
	Anatolij Gustschin, linuxppc-dev
In-Reply-To: <20140502170333.GJ32284@intel.com>

Hello, Vinod.
Thanks for your feedback.

2014-05-02 21:03 GMT+04:00 Vinod Koul <vinod.koul@intel.com>:
> On Wed, Apr 23, 2014 at 05:53:25PM +0400, Alexander Popov wrote:
>> +static struct dma_async_tx_descriptor *
>> +mpc_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>> +             unsigned int sg_len, enum dma_transfer_direction direction,
>> +             unsigned long flags, void *context)
>> +{
>> +     struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan);
>> +     struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
>> +     struct mpc_dma_desc *mdesc = NULL;
>> +     dma_addr_t per_paddr;
>> +     u32 tcd_nunits;
>> +     struct mpc_dma_tcd *tcd;
>> +     unsigned long iflags;
>> +     struct scatterlist *sg;
>> +     size_t len;
>> +     int iter, i;
>> +
>> +     /* Currently there is no proper support for scatter/gather */
> Why? Is this HW or SW limitation?
This is the software limitation.

As Gerhard noticed, unfortunately the Linux DMA API combines
peripheral access with scatter/gather function.

But the original MPC512x DMA driver already uses scatter/gather feature
of the hardware for chaining together individual mpc_dma_desc's
in mpc_dma_execute() while mpc_dma_desc itself cannot use
scatter/gather feature, because each mpc_dma_desc is associated
with exactly one mpc_dma_tcd.

>> +             if (direction == DMA_DEV_TO_MEM) {
>> +                     tcd->saddr = per_paddr;
>> +                     tcd->daddr = sg_dma_address(sg);
>> +                     tcd->soff = 0;
>> +                     tcd->doff = 4;
> what are these?
SOFF is source address signed offset. It is applied to the current
source address to form the next-state value as each source read is completed.
DOFF is destination address signed offset.

>> +     case DMA_TERMINATE_ALL:
>> +             /* Disable channel requests */
>> +             mdma = dma_chan_to_mpc_dma(chan);
>> +
>> +             spin_lock_irqsave(&mchan->lock, flags);
>> +
>> +             out_8(&mdma->regs->dmacerq, chan->chan_id);
>> +             list_splice_tail_init(&mchan->prepared, &mchan->free);
>> +             list_splice_tail_init(&mchan->queued, &mchan->free);
>> +             list_splice_tail_init(&mchan->active, &mchan->free);
>> +
>> +             spin_unlock_irqrestore(&mchan->lock, flags);
>> +
>> +             return 0;
> empty line after this pls
ok

>> +     case DMA_SLAVE_CONFIG:
>> +             /*
>> +              * Constraints:
>> +              *  - only transfers between a peripheral device and
>> +              *     memory are supported;
>> +              *  - minimal transfer chunk is 4 bytes and consequently
>> +              *     source and destination addresses must be 4-byte aligned
>> +              *     and transfer size must be aligned on (4 * maxburst)
>> +              *     boundary;
>> +              *  - during the transfer RAM address is being incremented by
>> +              *     the size of minimal transfer chunk;
>> +              *  - peripheral port's address is constant during the transfer.
>> +              */
>> +
>> +             cfg = (void *)arg;
>> +
>> +             if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES ||
>> +                 cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES ||
> and why this limtation, doesnt seem covered above?
I created this limitation because FIFO registers of LPC and SDHC
support _only_ 4-byte access.

I tried to cover this limitation in the statement "minimal transfer chunk
is 4 bytes". Should I make it more explicit?

>> +                 !IS_ALIGNED(cfg->src_addr, 4) ||
>> +                 !IS_ALIGNED(cfg->dst_addr, 4)) {
>> +                     return -EINVAL;
>> +             }
>> +
>> +             spin_lock_irqsave(&mchan->lock, flags);
>> +
>> +             mchan->src_per_paddr = cfg->src_addr;
>> +             mchan->src_tcd_nunits = cfg->src_maxburst;
>> +             mchan->dst_per_paddr = cfg->dst_addr;
>> +             mchan->dst_tcd_nunits = cfg->dst_maxburst;
>> +
>> +             /* Apply defaults */
>> +             if (mchan->src_tcd_nunits == 0)
>> +                     mchan->src_tcd_nunits = 1;
>> +             if (mchan->dst_tcd_nunits == 0)
>> +                     mchan->dst_tcd_nunits = 1;
>> +
>> +             spin_unlock_irqrestore(&mchan->lock, flags);
>> +
>> +             return 0;
> empty line here too
ok

Best regards,
Alexander

^ permalink raw reply

* Re: [PATCH v4 8/8] DMA: Freescale: add suspend resume functions for DMA driver
From: Hongbo Zhang @ 2014-05-08  9:52 UTC (permalink / raw)
  To: Shevchenko, Andriy
  Cc: dmaengine@vger.kernel.org, leo.li@freescale.com, Koul, Vinod,
	linux-kernel@vger.kernel.org, vkoul@infradead.org,
	scottwood@freescale.com, Williams, Dan J,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1399451493.2677.11.camel@smile.fi.intel.com>


On 05/07/2014 04:31 PM, Shevchenko, Andriy wrote:
> On Sun, 2014-05-04 at 18:22 +0800, Hongbo Zhang wrote:
>> On 05/03/2014 12:46 AM, Vinod Koul wrote:
>>> On Fri, Apr 18, 2014 at 04:17:51PM +0800, hongbo.zhang@freescale.com wrote:
>>>> From: Hongbo Zhang <hongbo.zhang@freescale.com>
>>>>
>>>> This patch adds suspend resume functions for Freescale DMA driver.
>>>> .prepare callback is used to stop further descriptors from being added into the
>>>> pending queue, and also issue pending queues into execution if there is any.
>>>> .suspend callback makes sure all the pending jobs are cleaned up and all the
>>>> channels are idle, and save the mode registers.
>>>> .resume callback re-initializes the channels by restore the mode registers.
>>>>
>>>> +
>>>> +static const struct dev_pm_ops fsldma_pm_ops = {
>>>> +	.prepare	= fsldma_prepare,
>>>> +	.suspend	= fsldma_suspend,
>>>> +	.resume		= fsldma_resume,
>>>> +};
>>> I think this is not correct. We discussed this sometime back on list. The
>>> DMAengine drivers should use late resume and early suspend to ensure they get
>>> suspended after clients (who should use normal ones) and resume before them
>>>
>> OK, will update it like this:
>> use .suspend to take place of current .prepare
> Could you remove this at all?
>
> Answering to your previous statements I could say that.
> Device drivers (DMAc users) that don't implement .suspend callback are
> on their own with troubles, you have not to care about them in the DMA
> driver.

Thanks for pointing out this issue.
Then how to handle the descriptors in the pending list if there is any?
a. let them finished.
     but if the DMA user has already suspended prior DMA controller, it 
is meaningless somehow and may even ask for trouble.
b. don't touch them.
     after resume these pending descriptors could be executed, it is 
also meaningless because the resumed DMA user may in different state 
from before being suspended.
c. delete them.
     should we do this? is is a bit crude?
d. return a non-success value
     then the whole suspend process is reversed, e.g. suspend fails.
I've looked through some dma drivers, most of them is in case b.

>> use .suspend_late to take place of current .suspend
>> use .resume_early to take place of current .resume
>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH v4 6/8] DMA: Freescale: change descriptor release process for supporting async_tx
From: Hongbo Zhang @ 2014-05-08 10:03 UTC (permalink / raw)
  To: Vinod Koul, dan.j.williams
  Cc: leo.li, vkoul, linux-kernel, scottwood, dmaengine, linuxppc-dev
In-Reply-To: <20140502165053.GH32284@intel.com>


On 05/03/2014 12:50 AM, Vinod Koul wrote:
> On Fri, Apr 18, 2014 at 04:17:49PM +0800, hongbo.zhang@freescale.com wrote:
>
> This need review from Dan ...
>
Dan, could you please have a look at this? thanks.

^ permalink raw reply

* Re: [PATCH V5] KVM: PPC: BOOK3S: Use the saved dar value and generic make_dsisr
From: Alexander Graf @ 2014-05-08 10:06 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: paulus, linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1399386622-21407-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>

On 05/06/2014 04:30 PM, Aneesh Kumar K.V wrote:
> Although it's optional IBM POWER cpus always had DAR value set on
> alignment interrupt. So don't try to compute these values.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

The patch body is fine now. The commit message however is not. The patch 
does two things:

   * Merge DSISR calculation with Linux's implementation
   * Use the real DAR on book3s_64 alignment interrupts

In fact, you probably should make this 2 patches.


Alex

^ permalink raw reply

* Re: [PATCH V2] KVM: PPC: BOOK3S: HV: Prefer CMA region for hash page table allocation
From: Alexander Graf @ 2014-05-08 10:11 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: paulus, linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1399391658-1308-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>

On 05/06/2014 05:54 PM, Aneesh Kumar K.V wrote:
> Today when KVM tries to reserve memory for the hash page table it
> allocates from the normal page allocator first. If that fails it
> falls back to CMA's reserved region. One of the side effects of
> this is that we could end up exhausting the page allocator and
> get linux into OOM conditions while we still have plenty of space
> available in CMA.
>
> This patch addresses this issue by first trying hash page table
> allocation from CMA's reserved region before falling back to the normal
> page allocator. So if we run out of memory, we really are out of memory.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Thanks, applied to kvm-ppc-queue.


Alex

^ 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