Re: [RESEND PATCH v7 2/4] Documentation, dt, arm64/arm: dt bindings for numa.
On 2015/12/21 22:27, Will Deacon wrote: Mark, On Fri, Dec 18, 2015 at 06:03:47PM +, Mark Rutland wrote: On Fri, Dec 18, 2015 at 09:00:18PM +0530, Ganapatrao Kulkarni wrote: Hi Mark, On Fri, Dec 18, 2015 at 7:48 PM, Mark Rutland wrote: +- distance-matrix + This property defines a matrix to describe the relative distances + between all numa nodes. + It is represented as a list of node pairs and their relative distance. + + Note: + 1. Each entry represents distance from first node to second node. + The distance are equal in either direction. + 2. The distance from a node to self(local distance) is represented + with value 10 and all inter node distance should be represented with + value greater than 10. + 3. distance-matrix shold have entries in lexicographical ascending + order of nodes. + 4. There must be only one Device node distance-map and must reside in the root node. I am still concerned that the local distance of 10 is completely arbitrary. IMHO, i do not see any issue in having defined local distance to arbitrary number(10). inter node numa distance is relative number with respect to local distance we have to fix local distance to some value, having it in dt to make generic will not add any additional value as compared to having the fixed local distance to 10. That's not quite true. The figure chosen for the local distance affects the granularity with which you can describe all distances. By using a local distance of 10 we can only encode distances in 10% chunks of that. Using a local distance of 100 we could encode in 1% chunks of that. Whilst I see what you're saying, the local distance of 10 seems to be part of the ACPI spec, and is the reason why the core code defines it that way. Now, we can of course do our own thing for device-tree, but I really don't think it's worth our while to change this without a compelling use-case. I agree, that would simplify the kernel code as well. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] EDAC: Add ARM64 EDAC
On 2015/10/24 1:58, Brijesh Singh wrote: >> So I checked the x86 code: the driver is always loaded as soon as the >> hardware is there (looking at PCI device IDs from the on-chip >> northbridge, for instance). The trick here is to have the Kconfig option >> defaulting to "=n", so a kernel builder would have to explicitly enable >> this. Android or embedded kernels wouldn't do this, for instance, while >> a server distribution would do. >> If a user doesn't want to be bothered with the driver, there is always >> the possibility of blacklisting the module. >> Setting a system policy is IMHO out of scope for a DT binding. >> > Will update Kconfig to make it n by default. > * Its possible that other SoC's might handle single-bit and double-bit errors differently compare to Seattle platform. In Seattle platform both errors are handled by firmware but if other SoC wants OS to handle these errors then they might need DT binding to provide the irq numbers etc. >> What do you mean exactly with "firmware handles these errors"? >> What would the firmware do? I guess just logging the error and then >> possibly reset the register? How would this change the driver? >> > On Seattle platform SoC generates a interrupt on both single bit and double > bit error > and that interrupt is handled by firmware, so we don't need to do anything in > the driver. > Driver just need to poll registers to log correctable errors (because they do > not generate interrupt). > This very driver is doing exactly what we want. DT binding is not required. > > But Hanjun's comment on very first patch hinted me that there is possibility > that > SoC generate a interrupt on single bit and double bit but firmware does not > handle it. > In those cases driver will need be extended to handle interrupt. yes, exactly. > > I will submit v3 for review with DT binding removed. We can revisit DT > binding need in future. > >>> I totally agree with you here, thanks for putting them together. >>> Different SoCs may handle the error in different ways, we need >>> bindings to specialize them, irq number is a good example :) >> But how does this affect this very driver, polling just those two registers? >> Where would the interrupt come into the game here? Where is the proposed >> DT binding for that interrupt? >> >> AFAICT EL3 firmware handling errors would just hide this information >> from the driver, so if the f/w decides to "handle" uncorrectable ECC >> errors in a fatal way, there is nothing the driver could do anyway, right? Yes, if EL3 firmware is involved, the driver don't need to handle such interrupt. >> >> Can you sketch a concrete example where we would actually need the >> driver to know about the firmware capabilities? So if firmware don't handle it, just like the APM xgene did in xgene_edac.c, we need handle it in the driver, then DT bindings with irq number are needed. You know, I'm working on ACPI and will enthusiastically encourage people using APEI with firmware handle error first :) , but I think we can't rule out such cases (driver handle the errors). Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] EDAC: Add ARM64 EDAC
Hi Brijesh,
On 2015/10/22 22:46, Brijesh Singh wrote:
> Hi Andre,
>
> On 10/21/2015 06:52 PM, Andre Przywara wrote:
>> On 21/10/15 21:41, Brijesh Singh wrote:
>>> Add support for Cortex A57 and A53 EDAC driver.
>> Hi Brijesh,
>>
>> thanks for the quick update! Some comments below.
>>
>>> Signed-off-by: Brijesh Singh
>>> CC: robh...@kernel.org
>>> CC: pawel.m...@arm.com
>>> CC: mark.rutl...@arm.com
>>> CC: ijc+devicet...@hellion.org.uk
>>> CC: ga...@codeaurora.org
>>> CC: dougthomp...@xmission.com
>>> CC: b...@alien8.de
>>> CC: mche...@osg.samsung.com
>>> CC: devicetree@vger.kernel.org
>>> CC: guohan...@huawei.com
>>> CC: andre.przyw...@arm.com
>>> CC: a...@arndb.de
>>> CC: linux-ker...@vger.kernel.org
>>> CC: linux-e...@vger.kernel.org
>>> ---
>>>
>>> v2:
>>> * convert into generic arm64 edac driver
>>> * remove AMD specific references from dt binding
>>> * remove poll_msec property from dt binding
>>> * add poll_msec as a module param, default is 100ms
>>> * update copyright text
>>> * define macro mnemonics for L1 and L2 RAMID
>>> * check L2 error per-cluster instead of per core
>>> * update function names
>>> * use get_online_cpus() and put_online_cpus() to make L1 and L2 register
>>> read hotplug-safe
>>> * add error check in probe routine
>>>
>>> .../devicetree/bindings/edac/armv8-edac.txt| 15 +
>>> drivers/edac/Kconfig | 6 +
>>> drivers/edac/Makefile | 1 +
>>> drivers/edac/cortex_arm64_edac.c | 457
>>> +
>>> 4 files changed, 479 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/edac/armv8-edac.txt
>>> create mode 100644 drivers/edac/cortex_arm64_edac.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/edac/armv8-edac.txt
>>> b/Documentation/devicetree/bindings/edac/armv8-edac.txt
>>> new file mode 100644
>>> index 000..dfd128f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/edac/armv8-edac.txt
>>> @@ -0,0 +1,15 @@
>>> +* ARMv8 L1/L2 cache error reporting
>>> +
>>> +On ARMv8, CPU Memory Error Syndrome Register and L2 Memory Error Syndrome
>>> +Register can be used for checking L1 and L2 memory errors.
>>> +
>>> +The following section describes the ARMv8 EDAC DT node binding.
>>> +
>>> +Required properties:
>>> +- compatible: Should be "arm,armv8-edac"
>>> +
>>> +Example:
>>> + edac {
>>> + compatible = "arm,armv8-edac";
>>> + };
>>> +
>> So if there is nothing in here, why do we need the DT binding at all (I
>> think Mark hinted at that already)?
>> Can't we just use the MIDR as already suggested by others?
>> Secondly, armv8-edac is wrong here, as this feature is ARM-Cortex
>> specific and not architectural.
>>
> Yes, I was going with Mark suggestion to remove DT binding but then came
> across these cases which kind of hinted to keep DT binding:
>
> * Without DT binding, the driver will always be loaded on arm64 unless its
> blacklisted.
> * Its possible that other SoC's might handle single-bit and double-bit errors
> differently compare to
> Seattle platform. In Seattle platform both errors are handled by firmware
> but if other SoC
> wants OS to handle these errors then they might need DT binding to provide
> the irq numbers etc.
I totally agree with you here, thanks for putting them together.
Different SoCs may handle the error in different ways, we need
bindings to specialize them, irq number is a good example :)
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] EDAC: Add AMD Seattle SoC EDAC
On 2015/10/21 17:35, Borislav Petkov wrote: > On Wed, Oct 21, 2015 at 09:55:43AM +0800, Hanjun Guo wrote: >> So I think the meaning of those error register is the same, but the way >> of handle it may different from SoCs, for single bit error: >> >> - SoC may trigger a interrupt; >> - SoC may just keep silent so we need to scan the registers using poll >>mechanism. >> >> For Double bit error: >> - SoC may also keep silent >> - Trigger a interrupt >> - Trigger a SEI (system error) >> >> Any suggestion to cover those cases? > Well, I guess we can implement all those and have them configurable > in the sense that a single driver loads, it has all functionality and > dependent on the vendor detection, it does only what the vendor wants > like trigger an interrupt or remain silent or ... Hmm, so we need to keep the DT bindings for different SoCs which have different ways of handling the errors. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] EDAC: Add AMD Seattle SoC EDAC
Hi Boris, Mark, On 2015/10/21 1:36, Borislav Petkov wrote: > On Tue, Oct 20, 2015 at 06:26:55PM +0100, Mark Rutland wrote: >>> Btw, how much of this is implementing generic A57 functionality? >> The driver is entirely A57 generic. >> >>> If a lot, can we make this a generic a57_edac driver so that multiple >>> vendors can use it? >> Yes. > Ok, cool. > >>> How fast and how ugly can something like that become? >> Not sure I follow. > In the sense that some vendor might require just a little bit different > handling or maybe wants to read some vendor-specific registers in > addition to the architectural ones. Yes, you are right and foresight :) > > Then we'll start adding vendor-specific hacks to that generic driver. > And therefore the question how fast and how ugly such hacks would > become. > > I guess we'll worry about that when we get there... So I think the meaning of those error register is the same, but the way of handle it may different from SoCs, for single bit error: - SoC may trigger a interrupt; - SoC may just keep silent so we need to scan the registers using poll mechanism. For Double bit error: - SoC may also keep silent - Trigger a interrupt - Trigger a SEI (system error) Any suggestion to cover those cases? Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] EDAC: Add AMD Seattle SoC EDAC
On 2015/10/21 1:25, Mark Rutland wrote: > On Tue, Oct 20, 2015 at 11:44:46AM -0500, Brijesh Singh wrote: >> Hi Mark, >> >> Thanks for review. >> >> -Brijesh >> >> On 10/19/2015 03:52 PM, Mark Rutland wrote: >>> Hi, >>> >>> Please Cc the devicetree list (devicetree@vger.kernel.org) when sending >>> binding patches. I see you've added the people from the MAINTAINERS >>> entry; the list should also be Cc'd. >>> >> Noted. >>> On Mon, Oct 19, 2015 at 02:23:17PM -0500, Brijesh Singh wrote: Add support for the AMD Seattle SoC EDAC driver. Signed-off-by: Brijesh Singh --- .../devicetree/bindings/edac/amd-seattle-edac.txt | 15 + drivers/edac/Kconfig | 6 + drivers/edac/Makefile | 1 + drivers/edac/seattle_edac.c| 306 + 4 files changed, 328 insertions(+) create mode 100644 Documentation/devicetree/bindings/edac/amd-seattle-edac.txt create mode 100644 drivers/edac/seattle_edac.c diff --git a/Documentation/devicetree/bindings/edac/amd-seattle-edac.txt b/Documentation/devicetree/bindings/edac/amd-seattle-edac.txt new file mode 100644 index 000..a0354e8 --- /dev/null +++ b/Documentation/devicetree/bindings/edac/amd-seattle-edac.txt @@ -0,0 +1,15 @@ +* AMD Seattle SoC EDAC node + +EDAC node is defined to describe on-chip error detection and reporting. + +Required properties: +- compatible: Should be "amd,arm-seattle-edac" +- poll-delay-msec: Indicates how often the edac check callback should be called. + Time in msec. >>> This second property doesn't describe the hardware in any way. It should >>> be runtime-configurable and dpesn't belong in the DT. >>> >>> Regardless, the binding is wrong. This is in no way specific to AMD >>> Seattle, and per the code is actually used to imply the presence of a >>> Cortex-A57 feature. No reference to AMD Seattle belongs in the DT >>> binding (with the exception of the example, perhaps), nor in the driver. >>> >>> NAK while this pretends to be something that it isn't. At minimum, you >>> need to correctly describe the feature you are trying to add support >>> for. >>> >> I will remove AMD specific string in compatibility field and make the >> poll-delay-msec optional. Will also expose this as module parameter as >> you suggested below. > I don't think it should be optional; I don't think it should be there at > all. > > I'm not sure if we even need a DT binding if we can derive the presence > of the feature from the MIDR. > + * The driver polls CPUMERRSR_EL1 and L2MERRSR_EL1 registers to logs the >>> These are IMPLEMENTATION DEFINED registers which are specific to >>> Cortex-A57, and I note that L2MERRSR_EL1 changed in r1p0. >>> >>> Which revisions of Cortex-A57 does this work with? >>> >> I have tested my code on r1p2. >> >>> Generally we avoid touching IMPLEMENTATION DEFINED registers as they may >>> not exist in some configurations or revisions, and could trap or undef. >>> Is it always safe to access these registers (in current revisions of >>> Cortex-A57)? >>> >> So far I have not ran into any trap error, was able to read/write >> registers from EL1 all the times. I looked at TRM but could not find >> reference that it would fail. As per doc the register should be >> available at all EL's except EL0. > Ok. This also works on Hisilicon D02 board. but the mechanism to handle the error is a little bit different, on D02, it use poll mechanism to scan the single bit ECC error just as Seattle, but D02 will trigger SEI when double bit ECC error happened. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/4] Documentation: arm64/arm: dt bindings for numa.
On 10/14/2015 12:47 AM, Mark Rutland wrote:
Hi Mark,
i am thinking, if we could not address(or becomes complex) these topologies
using associativity,
we should think of an alternate binding which suits existing and upcoming
arm64 platforms.
can we think of below numa binding which is inline with ACPI and will
address all sort of topologies!
i am proposing as below,
1. introduce "proximity" node property. this property will be
present in dt nodes like memory, cpu, bus and devices(like associativity
property) and
will tell which numa node(proximity domain) this dt node belongs to.
examples:
cpu@000 {
device_type = "cpu";
compatible = "cavium,thunder", "arm,armv8";
reg = <0x0 0x000>;
enable-method = "psci";
proximity = <0>;
};
cpu@001 {
device_type = "cpu";
compatible = "cavium,thunder", "arm,armv8";
reg = <0x0 0x001>;
enable-method = "psci";
proximity = <1>;
};
memory@ {
device_type = "memory";
reg = <0x0 0x0140 0x3 0xFEC0>;
proximity =<0>;
};
memory@100 {
device_type = "memory";
reg = <0x100 0x0040 0x3 0xFFC0>;
proximity =<1>;
};
pcie0@0x8480, {
compatible = "cavium,thunder-pcie";
device_type = "pci";
msi-parent = <&its>;
bus-range = <0 255>;
#size-cells = <2>;
#address-cells = <3>;
#stream-id-cells = <1>;
reg = <0x8480 0x 0 0x1000>; /*Configuration
space */
ranges = <0x0300 0x8010 0x 0x8010 0x
0x70 0x>, /* mem ranges */
<0x0300 0x8300 0x 0x8300 0x
0x500 0x>;
proximity =<0>;
};
2. Introduce new dt node "proximity-map" which will capture the NxN numa
node distance matrix.
for example, 4 nodes connected in mesh/ring structure as,
A(0) B(1) C(2) D(3) A(1)
relative distance would be,
A -> B = 20
B -> C = 20
C -> D = 20
D -> A = 20
A -> C = 40
B -> D = 40
and dt presentation for this distance matrix is :
proximity-map {
node-count = <4>;
distance-matrix = <0 0 10>,
<0 1 20>,
<0 2 40>,
<0 3 20>,
<1 0 20>,
<1 1 10>,
<1 2 20>,
<1 3 40>,
<2 0 40>,
<2 1 20>,
<2 2 10>,
<2 3 20>,
<3 0 20>,
<3 1 40>,
<3 2 20>,
<3 3 10>;
}
the entries like < 0 0 > < 1 1> < 2 2> < 3 3> can be optional and code can
put default value(local distance).
the entries like <1 0> can be optional if <0 1> and <1 0> are of same
distance.
is this binding looks ok?
This looks roughly requivalent to the ACPI SLIT, which means it's as
powerful, which allays my previous concerns.
Cool, I think those bindings are quite extensible and easy understood.
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/4] Documentation: arm64/arm: dt bindings for numa.
Hi Ganapatrao,
On 08/29/2015 10:56 PM, Ganapatrao Kulkarni wrote:
Hi Thunder,
On Sat, Aug 29, 2015 at 3:16 PM, Leizhen (ThunderTown)
wrote:
On 2015/8/28 22:02, Rob Herring wrote:
+benh
On Fri, Aug 28, 2015 at 7:32 AM, Mark Rutland wrote:
Hi,
On Fri, Aug 14, 2015 at 05:39:32PM +0100, Ganapatrao Kulkarni wrote:
DT bindings for numa map for memory, cores and IOs using
arm,associativity device node property.
Given this is just a copy of ibm,associativity, I'm not sure I see much
point in renaming the properties.
So just keep the ibm? I'm okay with that. That would help move to
common code. Alternatively, we could drop the vendor prefix and have
common code just check for both.
Hi all,
Why not copy the method of ACPI numa? There only three elements should be
configured:
1) a cpu belong to which node
2) a memory block belong to which node
3) the distance of each two nodes
The devicetree nodes of numa can be like below:
/ {
...
numa-nodes-info {
node-name: node-description {
mem-ranges = <...>;
cpus-list = <...>;
};
nodes-distance {
distance-list = <...>;
};
};
...
};
some what similar to what your are proposing is already implemented in
my v2 patchset.
https://lwn.net/Articles/623920/
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305164.html
we have went to associativity property based implementation to keep it
more generic.
i do have both acpi(using linaro/hanjun's patches) and associativity
based implementations on our internal tree
and tested on thunderx platform.
Great thanks!
i do see issue in creating numa mapping using ACPI for IOs(for
example, i am not able to create numa mapping for ITS which is on each
node, using ACPI tables), since ACPI spec (tables SRAT and SLIT)
talks only about processor and memory.
I'm not sure why the ITS needs to know the NUMA domain, for my
understanding, the interrupt will route to the correct NUMA domain
using setting the affinity, ITS will configured to route it to
the right GICR(cpu), so I think the ITS don't need to know which
NUMA node belonging to, correct me if I missed something.
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 8/8] clocksource: simplify ACPI code in arm_arch_timer.c
On 08/27/2015 08:28 PM, Hanjun Guo wrote:
On 08/27/2015 08:08 PM, Thomas Gleixner wrote:
On Thu, 27 Aug 2015, Hanjun Guo wrote:
On 08/26/2015 03:17 AM, Thomas Gleixner wrote:
On Wed, 26 Aug 2015, Fu Wei wrote:
/* Initialize per-processor generic timer */
-static int __init arch_timer_acpi_init(struct acpi_table_header
*table)
+void __init arch_timer_acpi_init(void)
{
And how is that supposed to work when we have next generation CPUs
which implement a different timer? You break multisystem kernels that
way.
Sorry, I think I missed some context here that I don't understand
why the code here will break multisystem kernels? I'm trying to
understand the problem here and update the code :)
yes, you are right, If there is a next generation CPUs which
implement a different timer, (maybe) this driver can not work.
we may need a new timer driver.
But,
(1) for now, aarch64 core always has the arch timer(this timer is
part of aarch64 architecture).
and the existing code make ARM64 kernel "select ARM_ARCH_TIMER "
(2) GTDT is designed for generic timer, so in this call "
arch_timer_acpi_init" we parse the gtdt info.
(3) once we have a ARM64 CPUs which implement a different timer, we
may need to select a right timer in the config stage.
and this timer may not be described in GTDT. So we can implement
another arch_timer_acpi_init by that time in new timer driver..
if the new time still uses GTDT(or new version GTDT), we may need to
update gtdt.c for new timer by that time.
That's simply wrong. You want to build kernels which run on both cpus
and the selection of the timer happens at runtime depending on the
ACPI info. We do the same thing with device tree.
I think the code can do that if I understand correctly. The code for
now is that we only support arch timer on ARM64, and this patch set
is adding SBSA watchdog timer support which need same function in
arch timer, so we move that function to common place.
We will load the driver (arch timer, memory mapped timer) when the
ACPI table defines them, which when new timer is coming, that will
defined in the ACPI table and load the driver as needed.
Please correct me if I misse something, thanks.
arch_timer_acpi_init() is called from the architecture boot code. So
how is that supposed to work with different timers?
Are you going to have bla_timer_acpi_init() and foo_timer_acpi_init()
calls as well?
Why not having a something like DT has: DECLARE_
and the arch_timer_acpi_init() using that to figure out which of the
timers to initialize.
Ah, ok, I can fully understand you now, thanks for your patience.
Yes, I agree with you, so this is not a problem for this patch, but
for the code implementation of previous code. Actually we are on the
road to do as you suggested, we introduced something like
#define ACPI_DECLARE(table, name, table_id, subtable, data, fn) [1]
in the GICv3 and GIC self probe patch set, and I said that
infrastructure can be used as clock declare too, we just trying
to not add such dependence on that patch set (it's still on discussion),
[1]: https://lkml.org/lkml/2015/7/29/236
If that is ok with you, we will introduce similar DECLARE_ thing
for clock declare.
Or we can drop this patch from this patch set, and clean up this
patch when the ACPI_DECLARE() infrastructure is ready for upstream.
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 8/8] clocksource: simplify ACPI code in arm_arch_timer.c
On 08/27/2015 08:08 PM, Thomas Gleixner wrote:
On Thu, 27 Aug 2015, Hanjun Guo wrote:
On 08/26/2015 03:17 AM, Thomas Gleixner wrote:
On Wed, 26 Aug 2015, Fu Wei wrote:
/* Initialize per-processor generic timer */
-static int __init arch_timer_acpi_init(struct acpi_table_header
*table)
+void __init arch_timer_acpi_init(void)
{
And how is that supposed to work when we have next generation CPUs
which implement a different timer? You break multisystem kernels that
way.
Sorry, I think I missed some context here that I don't understand
why the code here will break multisystem kernels? I'm trying to
understand the problem here and update the code :)
yes, you are right, If there is a next generation CPUs which
implement a different timer, (maybe) this driver can not work.
we may need a new timer driver.
But,
(1) for now, aarch64 core always has the arch timer(this timer is
part of aarch64 architecture).
and the existing code make ARM64 kernel "select ARM_ARCH_TIMER "
(2) GTDT is designed for generic timer, so in this call "
arch_timer_acpi_init" we parse the gtdt info.
(3) once we have a ARM64 CPUs which implement a different timer, we
may need to select a right timer in the config stage.
and this timer may not be described in GTDT. So we can implement
another arch_timer_acpi_init by that time in new timer driver..
if the new time still uses GTDT(or new version GTDT), we may need to
update gtdt.c for new timer by that time.
That's simply wrong. You want to build kernels which run on both cpus
and the selection of the timer happens at runtime depending on the
ACPI info. We do the same thing with device tree.
I think the code can do that if I understand correctly. The code for
now is that we only support arch timer on ARM64, and this patch set
is adding SBSA watchdog timer support which need same function in
arch timer, so we move that function to common place.
We will load the driver (arch timer, memory mapped timer) when the
ACPI table defines them, which when new timer is coming, that will
defined in the ACPI table and load the driver as needed.
Please correct me if I misse something, thanks.
arch_timer_acpi_init() is called from the architecture boot code. So
how is that supposed to work with different timers?
Are you going to have bla_timer_acpi_init() and foo_timer_acpi_init()
calls as well?
Why not having a something like DT has: DECLARE_
and the arch_timer_acpi_init() using that to figure out which of the
timers to initialize.
Ah, ok, I can fully understand you now, thanks for your patience.
Yes, I agree with you, so this is not a problem for this patch, but
for the code implementation of previous code. Actually we are on the
road to do as you suggested, we introduced something like
#define ACPI_DECLARE(table, name, table_id, subtable, data, fn) [1]
in the GICv3 and GIC self probe patch set, and I said that
infrastructure can be used as clock declare too, we just trying
to not add such dependence on that patch set (it's still on discussion),
[1]: https://lkml.org/lkml/2015/7/29/236
If that is ok with you, we will introduce similar DECLARE_ thing
for clock declare.
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 8/8] clocksource: simplify ACPI code in arm_arch_timer.c
Hi Thomas,
Thanks for the comments, I got some questions and
reply below.
On 08/26/2015 03:17 AM, Thomas Gleixner wrote:
On Wed, 26 Aug 2015, Fu Wei wrote:
/* Initialize per-processor generic timer */
-static int __init arch_timer_acpi_init(struct acpi_table_header *table)
+void __init arch_timer_acpi_init(void)
{
And how is that supposed to work when we have next generation CPUs
which implement a different timer? You break multisystem kernels that
way.
Sorry, I think I missed some context here that I don't understand
why the code here will break multisystem kernels? I'm trying to
understand the problem here and update the code :)
yes, you are right, If there is a next generation CPUs which
implement a different timer, (maybe) this driver can not work.
we may need a new timer driver.
But,
(1) for now, aarch64 core always has the arch timer(this timer is
part of aarch64 architecture).
and the existing code make ARM64 kernel "select ARM_ARCH_TIMER "
(2) GTDT is designed for generic timer, so in this call "
arch_timer_acpi_init" we parse the gtdt info.
(3) once we have a ARM64 CPUs which implement a different timer, we
may need to select a right timer in the config stage.
and this timer may not be described in GTDT. So we can implement
another arch_timer_acpi_init by that time in new timer driver..
if the new time still uses GTDT(or new version GTDT), we may need to
update gtdt.c for new timer by that time.
That's simply wrong. You want to build kernels which run on both cpus
and the selection of the timer happens at runtime depending on the
ACPI info. We do the same thing with device tree.
I think the code can do that if I understand correctly. The code for
now is that we only support arch timer on ARM64, and this patch set
is adding SBSA watchdog timer support which need same function in
arch timer, so we move that function to common place.
We will load the driver (arch timer, memory mapped timer) when the
ACPI table defines them, which when new timer is coming, that will
defined in the ACPI table and load the driver as needed.
Please correct me if I misse something, thanks.
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH non-pretimeout 5/7] ACPI: add GTDT table parse driver into ACPI driver
Hi Rafael,
Any comments on patch 5,6,7/7 of this patch set?
Thanks
Hanjun
On 06/11/2015 01:47 AM, fu@linaro.org wrote:
From: Fu Wei
This driver adds support for parsing SBSA Generic Watchdog
Structure in GTDT, and creating a platform device with that
information. This allows the operating system to obtain device
data from the resource of platform device.
The platform device named "sbsa-gwdt" can be used by the
ARM SBSA Generic Watchdog driver.
Signed-off-by: Fu Wei
Signed-off-by: Hanjun Guo
---
drivers/acpi/Kconfig | 9
drivers/acpi/Makefile | 1 +
drivers/acpi/gtdt.c | 137 ++
3 files changed, 147 insertions(+)
create mode 100644 drivers/acpi/gtdt.c
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 1bbaa3d..e125698 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -433,4 +433,13 @@ config XPOWER_PMIC_OPREGION
endif
+config ACPI_GTDT
+ bool "ACPI GTDT Support"
+ depends on ARM64
+ help
+ GTDT (Generic Timer Description Table) provides information
+ for per-processor timers and Platform (memory-mapped) timers
+ for ARM platforms. Select this option to provide information
+ needed for the timers init.
+
endif # ACPI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 431e587..2c5a194 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -96,3 +96,4 @@ obj-$(CONFIG_ACPI_EXTLOG) += acpi_extlog.o
obj-$(CONFIG_PMIC_OPREGION) += pmic/intel_pmic.o
obj-$(CONFIG_CRC_PMIC_OPREGION) += pmic/intel_pmic_crc.o
obj-$(CONFIG_XPOWER_PMIC_OPREGION) += pmic/intel_pmic_xpower.o
+obj-$(CONFIG_ACPI_GTDT)+= gtdt.o
diff --git a/drivers/acpi/gtdt.c b/drivers/acpi/gtdt.c
new file mode 100644
index 000..a92abf2
--- /dev/null
+++ b/drivers/acpi/gtdt.c
@@ -0,0 +1,137 @@
+/*
+ * ARM Specific GTDT table Support
+ *
+ * Copyright (C) 2015, Linaro Ltd.
+ * Author: Fu Wei
+ * Hanjun Guo
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
+{
+ int trigger, polarity;
+
+ if (!interrupt)
+ return 0;
+
+ trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
+ : ACPI_LEVEL_SENSITIVE;
+
+ polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
+ : ACPI_ACTIVE_HIGH;
+
+ return acpi_register_gsi(NULL, interrupt, trigger, polarity);
+}
+
+/*
+ * Initialize a SBSA generic Watchdog platform device info from GTDT
+ * According to SBSA specification the size of refresh and control
+ * frames of SBSA Generic Watchdog is SZ_4K(Offset 0x000 – 0xFFF).
+ */
+static int __init gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
+ int index)
+{
+ struct platform_device *pdev;
+ int irq = map_generic_timer_interrupt(wd->timer_interrupt,
+ wd->timer_flags);
+ struct resource res[] = {
+ DEFINE_RES_IRQ_NAMED(irq, "ws0"),
+ DEFINE_RES_MEM_NAMED(wd->refresh_frame_address, SZ_4K,
+"refresh"),
+ DEFINE_RES_MEM_NAMED(wd->control_frame_address, SZ_4K,
+"control"),
+ };
+
+ pr_debug("GTDT: a Watchdog GT(0x%llx/0x%llx gsi:%u flags:0x%x)\n",
+wd->refresh_frame_address, wd->control_frame_address,
+wd->timer_interrupt, wd->timer_flags);
+
+ if (!(wd->refresh_frame_address &&
+ wd->control_frame_address &&
+ wd->timer_interrupt)) {
+ pr_err("GTDT: failed geting the device info.\n");
+ return -EINVAL;
+ }
+
+ if (irq < 0) {
+ pr_err("GTDT: failed to register GSI of the Watchdog GT.\n");
+ return -EINVAL;
+ }
+
+ /*
+* Add a platform device named "sbsa-gwdt" to match the platform driver.
+* "sbsa-gwdt": SBSA(Server Base System Architecture) Generic Watchdog
+* The platform driver (like drivers/watchdog/sbsa_gwdt.c)can get device
+* info below by matching this name.
+*/
+ pdev = platform_device_register_simple("sbsa-gwdt", index, res,
+ ARRAY_SIZE(res));
+ if (IS_ERR(pdev)) {
+ acpi_unregister_gsi(wd->timer_interrupt);
+ return PTR_ERR(pdev);
+ }
+
+ return 0;
+}
+
+static int __
Re: [PATCH v3 6/6] ACPI: import watchdog info of GTDT into platform device
On 2015年05月27日 00:35, Timur Tabi wrote: On 05/26/2015 03:28 AM, Hanjun Guo wrote: early_acpi_os_unmap_memory((char *)table, tbl_size); } please add #ifdef CONFIG_ARM_SBSA_WATCHDOG (acpi gtdt code) #endif I don't agree with this. The GTDT should be parsed even if there's no watchdog driver compiled for this kernel. There are no other #ifdefs in this file. So what's the point of parse GTDT and alloc memories for it if there is no watchdog driver compiled for the kernel? will the module insmod later even if the CONFIG_ARM_SBSA_WATCHDOG=n? + * Add a platform device named "sbsa-gwdt" to match the platform driver. + * "sbsa-gwdt": SBSA(Server Base System Architecture) Generic Watchdog + * The platform driver can get device info below by matching this name. * The platform driver (drivers/watchdog/sbsa_gwdt.c) can get device info below by matching this name. Adding the file name which will help for review and maintain in my opinion. Except it will cause problems if the driver is renamed or moved. I don't think this is a good idea, either (sorry!) OK, that's good point. but what I proposed is some hint to which driver will use the data prepared in this file, we can easily understand it in this patchset, but if just review the code in this fiel, I think people will be confused without detail comments. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/6] ACPI: import watchdog info of GTDT into platform device
Hi Fu Wei,
Some minor comments inline.
On 2015年05月25日 18:03, fu@linaro.org wrote:
From: Fu Wei
Parse SBSA Generic Watchdog Structure in GTDT table of ACPI,
and create a platform device with that information.
This platform device can be used by the ARM SBSA Generic
Watchdog driver.
Tested-by: Suravee Suthikulpanit
Tested-by: Timur Tabi
Signed-off-by: Fu Wei
---
arch/arm64/kernel/acpi.c | 145 +++
1 file changed, 145 insertions(+)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 8b83955..c95deec 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
@@ -343,3 +344,147 @@ void __init acpi_gic_init(void)
early_acpi_os_unmap_memory((char *)table, tbl_size);
}
please add
#ifdef CONFIG_ARM_SBSA_WATCHDOG
(acpi gtdt code)
#endif
+
+static int __init acpi_gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
+int index)
+{
+ struct platform_device *pdev;
+ struct resource *res;
+ u32 gsi, flags;
+ int irq, trigger, polarity;
+ resource_size_t rf_base_phy, cf_base_phy;
+ int err = -ENOMEM;
+
+ /*
+* Get SBSA Generic Watchdog info
+* from a Watchdog GT type structure in GTDT
+*/
+ rf_base_phy = (resource_size_t)wd->refresh_frame_address;
+ cf_base_phy = (resource_size_t)wd->control_frame_address;
+ gsi = wd->timer_interrupt;
+ flags = wd->timer_flags;
+
+ pr_debug("GTDT: a Watchdog GT structure(0x%llx/0x%llx gsi:%u
flags:0x%x)\n",
+rf_base_phy, cf_base_phy, gsi, flags);
+
+ if (!(rf_base_phy && cf_base_phy && gsi)) {
+ pr_err("GTDT: failed geting the device info.\n");
+ return -EINVAL;
+ }
+
+ trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
+ : ACPI_LEVEL_SENSITIVE;
+ polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
+ : ACPI_ACTIVE_HIGH;
+ irq = acpi_register_gsi(NULL, gsi, trigger, polarity);
+ if (irq < 0) {
+ pr_err("GTDT: failed to register GSI of the Watchdog GT.\n");
+ return -EINVAL;
+ }
+
+ /*
+* Add a platform device named "sbsa-gwdt" to match the platform driver.
+* "sbsa-gwdt": SBSA(Server Base System Architecture) Generic Watchdog
+* The platform driver can get device info below by matching this name.
* The platform driver (drivers/watchdog/sbsa_gwdt.c) can get device info
below by matching this name.
Adding the file name which will help for review and maintain in my
opinion.
+*/
+ pdev = platform_device_alloc("sbsa-gwdt", index);
+ if (!pdev)
+ goto err_unregister_gsi;
+
+ res = kcalloc(3, sizeof(*res), GFP_KERNEL);
+ if (!res)
+ goto err_device_put;
+
+ /*
+* According to SBSA specification the size of refresh and control
+* frames of SBSA Generic Watchdog is SZ_4K(Offset 0x000 – 0xFFF).
+*/
+ res[0].start = rf_base_phy;
+ res[0].end = rf_base_phy + SZ_4K - 1;
+ res[0].name = "refresh";
+ res[0].flags = IORESOURCE_MEM;
+
+ res[1].start = cf_base_phy;
+ res[1].end = cf_base_phy + SZ_4K - 1;
+ res[1].name = "control";
+ res[1].flags = IORESOURCE_MEM;
+
+ res[2].start = irq;
+ res[2].end = res[2].start;
+ res[2].name = "ws0";
+ res[2].flags = IORESOURCE_IRQ;
+
+ err = platform_device_add_resources(pdev, res, 3);
+ if (err)
+ goto err_free_res;
+
+ err = platform_device_add(pdev);
...
+ if (err)
+ goto err_free_res;
+
+ return 0;
How about
if (!err)
return 0;
then no need for goto err_free_res and save two lines of codes.
Other than that,
Acked-by: Hanjun Guo
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/7] ACPI: import watchdog info of GTDT into platform device
+CC Catalin and Will
On 2015年05月21日 16:32, fu@linaro.org wrote:
From: Fu Wei
Parse SBSA Generic Watchdog Structure in GTDT table of ACPI,
and create a platform device with that information.
This platform device can be used by the ARM SBSA Generic
Watchdog driver.
Signed-off-by: Fu Wei
---
arch/arm64/kernel/acpi.c | 136 +++
1 file changed, 136 insertions(+)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 8b83955..1ed11fd 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
@@ -343,3 +344,138 @@ void __init acpi_gic_init(void)
early_acpi_os_unmap_memory((char *)table, tbl_size);
}
+
+static int __init acpi_gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
+int index)
+{
+ struct platform_device *pdev;
+ struct resource *res;
+ u32 gsi, flags;
+ int irq, trigger, polarity;
+ resource_size_t rf_base_phy, cf_base_phy;
+ int err = -ENOMEM;
+
+ /*
+* Get SBSA Generic Watchdog info
+* from a Watchdog GT type structure in GTDT
+*/
+ rf_base_phy = (resource_size_t)wd->refresh_frame_address;
+ cf_base_phy = (resource_size_t)wd->control_frame_address;
+ gsi = wd->timer_interrupt;
+ flags = wd->timer_flags;
+
+ pr_info("GTDT: a Watchdog GT structure(0x%llx/0x%llx gsi:%u
flags:0x%x)\n",
+ rf_base_phy, cf_base_phy, gsi, flags);
Can we use pr_debug here? I don't think those information
worthy a pr_info.
+
+ if (!(rf_base_phy && cf_base_phy && gsi)) {
+ pr_err("GTDT: failed geting the device info.\n");
+ return -EINVAL;
+ }
+
+ trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
+ : ACPI_LEVEL_SENSITIVE;
+ polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
+ : ACPI_ACTIVE_HIGH;
I see places to duplicate this, I will look into this to see we can form
a function to handle it.
+ irq = acpi_register_gsi(NULL, gsi, trigger, polarity);
+ if (irq < 0) {
+ pr_err("GTDT: failed to register GSI of the Watchdog GT.\n");
+ return -EINVAL;
+ }
+
+ pdev = platform_device_alloc("sbsa-gwdt", index);
Please put a comment before this function to explain
why we need a "sbsa-gwdt" name here.
+ if (!pdev)
+ goto err_unregister_gsi;
+
+ res = kcalloc(3, sizeof(*res), GFP_KERNEL);
+ if (!res)
+ goto err_device_put;
+
+ res[0].start = rf_base_phy;
+ res[0].end = rf_base_phy + SZ_4K - 1;
+ res[0].name = "refresh";
+ res[0].flags = IORESOURCE_MEM;
+
+ res[1].start = cf_base_phy;
+ res[1].end = cf_base_phy + SZ_4K - 1;
So why SZ_4K? is it defined in SBSA spec? if so, please
comment on it too.
+ res[1].name = "control";
+ res[1].flags = IORESOURCE_MEM;
+
+ res[2].start = irq;
+ res[2].end = res[2].start;
+ res[2].name = "ws0";
+ res[2].flags = IORESOURCE_IRQ;
+
+ err = platform_device_add_resources(pdev, res, 3);
+ if (err)
+ goto err_free_res;
+
+ err = platform_device_add(pdev);
+ if (err)
+ goto err_free_res;
+
+ return 0;
+
+err_free_res:
+ kfree(res);
+err_device_put:
+ platform_device_put(pdev);
+err_unregister_gsi:
+ acpi_unregister_gsi(gsi);
+
+ return err;
+}
+
+/* Initialize SBSA generic Watchdog platform device info from GTDT */
+static int __init acpi_gtdt_sbsa_gwdt_init(struct acpi_table_header *table)
+{
+ struct acpi_table_gtdt *gtdt;
+ struct acpi_gtdt_header *header;
+ void *gtdt_subtable;
+ int i, gwdt_index;
+ int ret = 0;
+
+ if (table->revision < 2) {
+ pr_info("GTDT: Revision:%d doesn't support Platform Timers.\n",
+ table->revision);
pr_warn() would be good.
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/7] Watchdog: introduce ARM SBSA watchdog driver
On 2015年05月22日 23:01, Guenter Roeck wrote: On Fri, May 22, 2015 at 04:55:04PM +0200, Arnd Bergmann wrote: On Friday 22 May 2015 22:50:30 Hanjun Guo wrote: diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index e5e7c55..25a0df1 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -152,6 +152,18 @@ config ARM_SP805_WATCHDOG ARM Primecell SP805 Watchdog timer. This will reboot your system when the timeout is reached. +config ARM_SBSA_WATCHDOG + tristate "ARM SBSA Generic Watchdog" + depends on ARM || ARM64 || COMPILE_TEST SBSA is for ARMv8-A based (64-bit) servers, no need to depends on ARM, and why we depends on COMPILE_TEST? I think it's a reasonable assumption that someone will sooner or later put that hardware into an ARM32 machine, or run a 32-bit kernel on a chip that has it. While SBSA requires this watchdog device, nothing prevents SoC manufacturers from using the same design in something that is not a server. From this point of view, I agree that SBSA watchdog design may used in other ARM SoCs in the future, but how about add it back when this kind of hardware showing up? Tricky, though. Since teh driver uses arm specific clock functions, I don't think this can compile on a non-arm machine. Since it depends on ARM64/ARM, we can temporary release from that now :) Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/7] Watchdog: introduce ARM SBSA watchdog driver
On 2015年05月21日 16:32, fu@linaro.org wrote: From: Fu Wei This driver bases on linux kernel watchdog framework, and use "pretimeout" in the framework. It supports getting timeout and pretimeout from parameter and FDT at the driver init stage. In first timeout(WS0), the interrupt routine run panic to save system context. Signed-off-by: Fu Wei --- drivers/watchdog/Kconfig | 12 ++ drivers/watchdog/Makefile| 1 + drivers/watchdog/sbsa_gwdt.c | 476 +++ 3 files changed, 489 insertions(+) create mode 100644 drivers/watchdog/sbsa_gwdt.c diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index e5e7c55..25a0df1 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -152,6 +152,18 @@ config ARM_SP805_WATCHDOG ARM Primecell SP805 Watchdog timer. This will reboot your system when the timeout is reached. +config ARM_SBSA_WATCHDOG + tristate "ARM SBSA Generic Watchdog" + depends on ARM || ARM64 || COMPILE_TEST SBSA is for ARMv8-A based (64-bit) servers, no need to depends on ARM, and why we depends on COMPILE_TEST? + depends on ARM_ARCH_TIMER + select WATCHDOG_CORE + help + ARM SBSA Generic Watchdog timer. This has two Watchdog Signals + (WS0/WS1), will trigger a warning interrupt(do panic) at the first + timeout(WS0); will reboot your system when the second timeout(WS1) + is reached. + More details: DEN0029B - Server Base System Architecture (SBSA) + config AT91RM9200_WATCHDOG tristate "AT91RM9200 watchdog" depends on SOC_AT91RM9200 && MFD_SYSCON diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 5c19294..471f1b7c 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -30,6 +30,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o # ARM Architecture obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o +obj-$(CONFIG_ARM_SBSA_WATCHDOG) += sbsa_gwdt.o obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o obj-$(CONFIG_CADENCE_WATCHDOG) += cadence_wdt.o diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c new file mode 100644 index 000..4ebe7c3 --- /dev/null +++ b/drivers/watchdog/sbsa_gwdt.c @@ -0,0 +1,476 @@ +/* + * SBSA(Server Base System Architecture) Generic Watchdog driver + * + * Copyright (c) 2015, Linaro Ltd. + * Author: Fu Wei + * Suravee Suthikulpanit + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License 2 as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Note: This SBSA Generic watchdog driver is compatible with + * the pretimeout concept of Linux kernel. + * The timeout and pretimeout are set by the different REGs. + * The first watch period is set by writing WCV directly, + * that can support more than 10s timeout at the maximum + * system counter frequency. + * The second watch period is set by WOR(32bit) which will be loaded + * automatically by hardware, when WS0 is triggered. + * This gives a maximum watch period of around 10s at the maximum + * system counter frequency. + * The System Counter shall run at maximum of 400MHz. + * More details: DEN0029B - Server Base System Architecture (SBSA) + * + * Kernel/API: P-| pretimeout + * |---T timeout + * SBSA GWDT: P--WOR---WS1 pretimeout + * |---WCV--WS0T timeout + */ + +#include please put it under #include , this is the convention of order head files. + +#include Will we add any acpi realted code in this patch? if not, please remove this head file. +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* SBSA Generic Watchdog register definitions */ +/* refresh frame */ +#define SBSA_GWDT_WRR 0x000 + +/* control frame */ +#define SBSA_GWDT_WCS 0x000 +#define SBSA_GWDT_WOR 0x008 +#define SBSA_GWDT_WCV_LO 0x010 +#define SBSA_GWDT_WCV_HI 0x014 + +/* refresh/control frame */ +#define SBSA_GWDT_W_IIDR 0xfcc +#define SBSA_GWDT_IDR 0xfd0 + +/* Watchdog Control and Status Register */ +#define SBSA_GWDT_WCS_EN BIT(0) +#define SBSA_GWDT_WCS_WS0 BIT(1) +#define SBSA_GWDT_WCS_WS1 BIT(2) + +/** + * struct sbsa_gwdt - Internal rep
Re: [PATCH v2 1/7] clocksource: export "arch_timer_get_rate" for the other drivers
On 2015年05月21日 16:32, fu@linaro.org wrote:
From: Fu Wei
Some devices get clock from system counter, like SBSA watchdog
driver. They may need to get system counter rate.
and please add a comment that SBSA watchdog is a kernel module,
then it would explicit that why EXPORT_SYMBOL_GPL is needed.
Signed-off-by: Fu Wei
Other than that,
Acked-by: Hanjun Guo
---
drivers/clocksource/arm_arch_timer.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clocksource/arm_arch_timer.c
b/drivers/clocksource/arm_arch_timer.c
index 0aa135d..4327bf9 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -410,6 +410,7 @@ u32 arch_timer_get_rate(void)
{
return arch_timer_rate;
}
+EXPORT_SYMBOL_GPL(arch_timer_get_rate);
static u64 arch_counter_get_cntvct_mem(void)
{
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 4/4] arm64:numa: adding numa support for arm64 platforms.
Hi Ganapatrao,
On 2015年01月22日 19:05, Ganapatrao Kulkarni wrote:
Adding numa support for arm64 based platforms.
Adding dt node pasring for numa topology using property arm,associativity.
Signed-off-by: Ganapatrao Kulkarni
---
arch/arm64/Kconfig | 32 +++
arch/arm64/include/asm/mmzone.h | 32 +++
arch/arm64/include/asm/numa.h | 43
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/dt_numa.c | 302 +++
arch/arm64/kernel/setup.c | 8 +
arch/arm64/kernel/smp.c | 2 +
arch/arm64/mm/Makefile | 1 +
arch/arm64/mm/init.c| 34 ++-
arch/arm64/mm/numa.c| 522
10 files changed, 971 insertions(+), 6 deletions(-)
create mode 100644 arch/arm64/include/asm/mmzone.h
create mode 100644 arch/arm64/include/asm/numa.h
create mode 100644 arch/arm64/kernel/dt_numa.c
create mode 100644 arch/arm64/mm/numa.c
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 242419d..6d262b1 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -59,6 +59,7 @@ config ARM64
select HAVE_PERF_USER_STACK_DUMP
select HAVE_RCU_TABLE_FREE
select HAVE_SYSCALL_TRACEPOINTS
+ select HAVE_MEMBLOCK_NODE_MAP if NUMA
select IRQ_DOMAIN
select MODULES_USE_ELF_RELA
select NO_BOOTMEM
@@ -315,6 +316,37 @@ config HOTPLUG_CPU
Say Y here to experiment with turning CPUs off and on. CPUs
can be controlled through /sys/devices/system/cpu.
+# Common NUMA Features
+config NUMA
+ bool "Numa Memory Allocation and Scheduler Support"
+ depends on SMP
+ ---help---
+ Enable NUMA (Non Uniform Memory Access) support.
+
+ The kernel will try to allocate memory used by a CPU on the
+ local memory controller of the CPU and add some more
+ NUMA awareness to the kernel.
+
+config ARM64_DT_NUMA
+ def_bool n
+ prompt "DT NUMA detection"
+ ---help---
+ Enable DT based numa.
+
+config NODES_SHIFT
+ int "Maximum NUMA Nodes (as a power of 2)"
+ range 1 10
+ default "2"
+ depends on NEED_MULTIPLE_NODES
+ ---help---
+ Specify the maximum number of NUMA Nodes available on the target
+ system. Increases memory reserved to accommodate various tables.
+
+config USE_PERCPU_NUMA_NODE_ID
+ def_bool y
+ depends on NUMA
+
+
source kernel/Kconfig.preempt
config HZ
diff --git a/arch/arm64/include/asm/mmzone.h b/arch/arm64/include/asm/mmzone.h
new file mode 100644
index 000..d27ee66
--- /dev/null
+++ b/arch/arm64/include/asm/mmzone.h
@@ -0,0 +1,32 @@
+#ifndef __ASM_ARM64_MMZONE_H_
+#define __ASM_ARM64_MMZONE_H_
+
+#ifdef CONFIG_NUMA
+
+#include
+#include
+#include
+#include
+
+extern struct pglist_data *node_data[];
+
+#define NODE_DATA(nid) (node_data[nid])
+
+
+struct numa_memblk {
+ u64 start;
+ u64 end;
+ int nid;
+};
+
+struct numa_meminfo {
+ int nr_blks;
+ struct numa_memblk blk[NR_NODE_MEMBLKS];
+};
+
+void __init numa_remove_memblk_from(int idx, struct numa_meminfo *mi);
+int __init numa_cleanup_meminfo(struct numa_meminfo *mi);
+void __init numa_reset_distance(void);
+
+#endif /* CONFIG_NUMA */
+#endif /* __ASM_ARM64_MMZONE_H_ */
diff --git a/arch/arm64/include/asm/numa.h b/arch/arm64/include/asm/numa.h
new file mode 100644
index 000..8be5cb1
--- /dev/null
+++ b/arch/arm64/include/asm/numa.h
@@ -0,0 +1,43 @@
+#ifndef _ASM_ARM64_NUMA_H
+#define _ASM_ARM64_NUMA_H
Use _ASM_NUMA_H please.
+
+#include
+#include
+
+#ifdef CONFIG_NUMA
+
+#define NR_NODE_MEMBLKS(MAX_NUMNODES * 2)
+#define ZONE_ALIGN (1UL << (MAX_ORDER + PAGE_SHIFT))
+
+/* currently, arm64 implements flat NUMA topology */
+#define parent_node(node) (node)
+
+/* dummy definitions for pci functions */
+#define pcibus_to_node(node) 0
+#define cpumask_of_pcibus(bus) 0
+
+struct __node_cpu_hwid {
+ u32 node_id;/* logical node containing this CPU */
+ u64 cpu_hwid; /* MPIDR for this CPU */
+};
+
+const struct cpumask *cpumask_of_node(int node);
+/* Mappings between node number and cpus on that node. */
+extern cpumask_var_t node_to_cpumask_map[MAX_NUMNODES];
+
+void __init arm64_numa_init(void);
+int __init numa_add_memblk(u32 nodeid, u64 start, u64 end);
+void numa_store_cpu_info(int cpu);
+void numa_set_node(int cpu, int node);
+void numa_clear_node(int cpu);
+void numa_add_cpu(int cpu);
+void numa_remove_cpu(int cpu);
+void __init numa_set_distance(int from, int to, int distance);
+int dt_get_cpu_node_id(int cpu);
+int __init arm64_dt_numa_init(void);
Do we really need all those function declared here?
+#else /* CONFIG_NUMA */
+static inline void arm64_numa_init(void);
Compile error when !CONFIG_NUMA, should be
static inline void arm64_numa_init(void) { }
+s
Re: [RFC PATCH v2 2/4] Documentation: arm64/arm: dt bindings for numa.
On 2014年12月12日 22:20, Arnd Bergmann wrote: On Thursday 11 December 2014 17:16:35 Hanjun Guo wrote: On 2014年12月10日 18:57, Arnd Bergmann wrote: On Wednesday 26 November 2014 17:12:49 Hanjun Guo wrote: The above topology is not easy to represent, but I think it would work like this (ignoring the threads/cores/clusters on the socket, which would also need to be described in a full DT), using multiple logical paths between the nodes: socket 0 ibm,associativity = <0 0 0 0>, <1 1 1 0>, <2 2 0 0>, 0 0 0>; socket 1 ibm,associativity = <1 1 1 1>, <0 0 0 1>, <2 2 2 1>, 3 1 1>; socket 2 ibm,associativity = <2 2 2 2>, <0 0 2 2>, <1 1 1 2>, 3 3 2>; socket 3 ibm,associativity = 3 3 3>, <0 3 3 3>, <1 1 3 3>, <2 2 2 3>; This describes four levels or hierarchy, with the lowest level being a single CPU core on one socket, and four paths between the sockets. To compute the associativity between two sockets, you need to look at each combination of paths to find the best match. Comparing sockets 0 and 1, the best matches are <1 1 1 0> with <1 1 1 1>, and <0 0 0 0> with <0 0 0 1>. In each case, the associativity is "3", meaning the first three entries match. Comparing sockets 0 and 3, we have four equally bad matches that each only match in the highest-level domain, e.g. <0 0 0 0> with <0 3 3 3>, so the associativity is only "1", and that means the two nodes are less closely associated than two neighboring ones. With the algorithm that powerpc uses to turn associativity into distance, 2**(numlevels - associativity), this would put the distance of neighboring nodes at "2", and the longest distance at "8". Thanks for the explain, I can understand how it works now, a bit complicated for me and I think the distance property "node-matrix" in Ganapatrao's patch is straight forward, what do you think? I still think we should go the whole way of having something compatible with the existing bindings, possibly using different property names if there are objections to using the "ibm," prefix. I agree that we should keep using existing bindings and not introducing a new one. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 2/4] Documentation: arm64/arm: dt bindings for numa.
Hi Arnd, On 2014年12月10日 18:57, Arnd Bergmann wrote: On Wednesday 26 November 2014 17:12:49 Hanjun Guo wrote: Thanks for the detail information. I have the concerns about the distance for NUMA nodes, does the "ibm,associativity-reference-points" property can represent the distance between NUMA nodes? For example, a system with 4 sockets connected like below: Socket 0 <> Socket 1 <> Socket 2 <> Socket 3 So from socket 0 to socket 1 (maybe on the same board), it just need 1 jump to access the memory, but from socket 0 to socket 2/3, it needs 2/3 jumps and the *distance* relative longer. Can "ibm,associativity-reference-points" property cover this? Hi Hanjun, I only today found your replies in my spam folder, I need to put you on a whitelist so that doesn't happen again. Thanks. I hope my ACPI patches will not scare your email filter :) The above topology is not easy to represent, but I think it would work like this (ignoring the threads/cores/clusters on the socket, which would also need to be described in a full DT), using multiple logical paths between the nodes: socket 0 ibm,associativity = <0 0 0 0>, <1 1 1 0>, <2 2 0 0>, <3 0 0 0>; socket 1 ibm,associativity = <1 1 1 1>, <0 0 0 1>, <2 2 2 1>, <3 3 1 1>; socket 2 ibm,associativity = <2 2 2 2>, <0 0 2 2>, <1 1 1 2>, <3 3 3 2>; socket 3 ibm,associativity = <3 3 3 3>, <0 3 3 3>, <1 1 3 3>, <2 2 2 3>; This describes four levels or hierarchy, with the lowest level being a single CPU core on one socket, and four paths between the sockets. To compute the associativity between two sockets, you need to look at each combination of paths to find the best match. Comparing sockets 0 and 1, the best matches are <1 1 1 0> with <1 1 1 1>, and <0 0 0 0> with <0 0 0 1>. In each case, the associativity is "3", meaning the first three entries match. Comparing sockets 0 and 3, we have four equally bad matches that each only match in the highest-level domain, e.g. <0 0 0 0> with <0 3 3 3>, so the associativity is only "1", and that means the two nodes are less closely associated than two neighboring ones. With the algorithm that powerpc uses to turn associativity into distance, 2**(numlevels - associativity), this would put the distance of neighboring nodes at "2", and the longest distance at "8". Thanks for the explain, I can understand how it works now, a bit complicated for me and I think the distance property "node-matrix" in Ganapatrao's patch is straight forward, what do you think? Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 2/4] Documentation: arm64/arm: dt bindings for numa.
On 2014-11-26 3:00, Arnd Bergmann wrote:
> On Tuesday 25 November 2014 08:15:47 Ganapatrao Kulkarni wrote:
>>> No, don't hardcode ARM specifics into a common binding either. I've looked
>>> at the ibm,associativity properties again, and I think we should just use
>>> those, they can cover all cases and are completely independent of the
>>> architecture. We should probably discuss about the property name though,
>>> as using the "ibm," prefix might not be the best idea.
>>
>> We have started with new proposal, since not got enough details how
>> ibm/ppc is managing the numa using dt.
>> there is no documentation and there is no power/PAPR spec for numa in
>> public domain and there are no single dt file in arch/powerpc which
>> describes the numa. if we get any one of these details, we can align
>> to powerpc implementation.
>
> Basically the idea is to have an "ibm,associativity" property in each
> bus or device that is node specific, and this includes all CPUs and
> memory nodes. The property contains an array of 32-bit integers that
> count the resources. Take an example of a NUMA cluster of two machines
> with four sockets and four cores each (32 cores total), a memory
> channel on each socket and one PCI host per board that is connected
> at equal speed to each socket on the board.
>
> The ibm,associativity property in each PCI host, CPU or memory device
> node consequently has an array of three (board, socket, core) integers:
>
> memory@0,0 {
> device_type = "memory";
> reg = <0x0 0x0 0x4 0x0;
> /* board 0, socket 0, no specific core */
> ibm,asssociativity = <0 0 0x>;
> };
>
> memory@4,0 {
> device_type = "memory";
> reg = <0x4 0x0 0x4 0x0>;
> /* board 0, socket 1, no specific core */
> ibm,asssociativity = <0 1 0x>;
> };
>
> ...
>
> memory@1c,0 {
> device_type = "memory";
> reg = <0x1c 0x0 0x4 0x0>;
> /* board 0, socket 7, no specific core */
> ibm,asssociativity = <1 7 0x>;
> };
>
> cpus {
> #address-cells = <2>;
> #size-cells = <0>;
> cpu@0 {
> device_type = "cpu";
> reg = <0 0>;
> /* board 0, socket 0, core 0*/
> ibm,asssociativity = <0 0 0>;
> };
>
> cpu@1 {
> device_type = "cpu";
> reg = <0 0>;
> /* board 0, socket 0, core 0*/
> ibm,asssociativity = <0 0 0>;
> };
>
> ...
>
> cpu@31 {
> device_type = "cpu";
> reg = <0 32>;
> /* board 1, socket 7, core 31*/
> ibm,asssociativity = <1 7 31>;
> };
> };
>
> pci@100,0 {
> device_type = "pci";
> /* board 0 */
> ibm,associativity = <0 0x 0x>;
> ...
> };
>
> pci@200,0 {
> device_type = "pci";
> /* board 1 */
> ibm,associativity = <1 0x 0x>;
> ...
> };
>
> ibm,associativity-reference-points = <0 1>;
>
> The "ibm,associativity-reference-points" property here indicates that index 2
> of each array is the most important NUMA boundary for the particular system,
> because the performance impact of allocating memory on the remote board
> is more significant than the impact of using memory on a remote socket of the
> same board. Linux will consequently use the first field in the array as
> the NUMA node ID. If the link between the boards however is relatively fast,
> so you care mostly about allocating memory on the same socket, but going to
> another board isn't much worse than going to another socket on the same
> board, this would be
>
> ibm,associativity-reference-points = <1 0>;
>
> so Linux would ignore the board ID and use the socket ID as the NUMA node
> number. The same would apply if you have only one (otherwise identical
> board, then you would get
>
> ibm,associativity-reference-points = <1>;
>
> which means that index 0 is completely irrelevant for NUMA considerations
> and you just care about the socket ID. In this case, devices on the PCI
> bus would also not care about NUMA policy and just allocate buffers from
> anywhere, while in original example Linux would allocate DMA buffers only
> from the local board.
Thanks for the detail information. I have the concerns about the distance
for NUMA nodes, does the "ibm,associativity-reference-points" property can
represent the distance between NUMA nodes?
For example, a system with 4 sockets connected like below:
Socket 0 <> Socket 1 <> Socket 2 <> Socket 3
So from socket 0 to
Re: [RFC PATCH v2 2/4] Documentation: arm64/arm: dt bindings for numa.
Hi Arnd,
On 2014年11月25日 19:02, Arnd Bergmann wrote:
On Tuesday 25 November 2014 17:42:44 Hanjun Guo wrote:
On 2014-11-25 11:55, Shannon Zhao wrote:
Hi,
On 2014/11/22 5:23, Ganapatrao Kulkarni wrote:
[...]
+==
+4 - Example dts
+==
+
+Example 1: 2 Node system each having 8 CPUs and a Memory.
+
+numa-map {
+#address-cells = <2>;
+#size-cells = <1>;
+#node-count = <2>;
+mem-map = <0x0 0x 0>,
+ <0x100 0x 1>;
+
+cpu-map = <0 7 0>,
+ <8 15 1>;
The cpu range is continuous here. But if there is a situation like below:
0 2 4 6 belong to node 0
1 3 5 7 belong to node 1
This case is very common on X86. I don't know the real situation of arm as
I don't have a hardware with 2 nodes.
How can we generate a DTS about this situation? like below? Can be parsed?
cpu-map = <0 2 4 6 0>,
<1 3 5 7 1>;
I think the binding proposed here can not cover your needs, and I think this
binding is not suitable, there are some reasons.
- CPU logical ID is allocated by OS, and it depends on the order of CPU node
in the device tree, so it may be in a clean order like this patch proposed,
or it will like the order Shannon pointed out.
- Since CPU logical ID is allocated by OS, DTS file will not know these
numbers.
Also:
- you cannot support hierarchical NUMA topology
- you cannot have CPU-less or memory-less nodes
- you cannot associate I/O devices with NUMA nodes, only memory and CPU
Yes, I agree.
So the problem behind this is the mappings between CPUs and NUMA nodes,
there is already mapping for CPU hardware ID (MPIDR) and CPU logical ID,
and MPIDR will be not changed, why not using MPIDR for the mapping of
NUMA node and CPU? then the mappings will be:
CPU logical ID <--> CPU MPIDR <-> NUMA node ID <-> proximity domain
(allocated by OS) (constant) (allocated by OS)
No, don't hardcode ARM specifics into a common binding either. I've looked
at the ibm,associativity properties again, and I think we should just use
those, they can cover all cases and are completely independent of the
architecture. We should probably discuss about the property name though,
as using the "ibm," prefix might not be the best idea.
Is there any doc/code related to this? please give me some hints and I
will read that.
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 2/4] Documentation: arm64/arm: dt bindings for numa.
On 2014-11-25 11:55, Shannon Zhao wrote:
> Hi,
>
> On 2014/11/22 5:23, Ganapatrao Kulkarni wrote:
[...]
>> +==
>> +4 - Example dts
>> +==
>> +
>> +Example 1: 2 Node system each having 8 CPUs and a Memory.
>> +
>> +numa-map {
>> +#address-cells = <2>;
>> +#size-cells = <1>;
>> +#node-count = <2>;
>> +mem-map = <0x0 0x 0>,
>> + <0x100 0x 1>;
>> +
>> +cpu-map = <0 7 0>,
>> + <8 15 1>;
>
> The cpu range is continuous here. But if there is a situation like below:
>
> 0 2 4 6 belong to node 0
> 1 3 5 7 belong to node 1
>
> This case is very common on X86. I don't know the real situation of arm as
> I don't have a hardware with 2 nodes.
>
> How can we generate a DTS about this situation? like below? Can be parsed?
>
> cpu-map = <0 2 4 6 0>,
> <1 3 5 7 1>;
I think the binding proposed here can not cover your needs, and I think this
binding is not suitable, there are some reasons.
- CPU logical ID is allocated by OS, and it depends on the order of CPU node
in the device tree, so it may be in a clean order like this patch proposed,
or it will like the order Shannon pointed out.
- Since CPU logical ID is allocated by OS, DTS file will not know these
numbers.
So the problem behind this is the mappings between CPUs and NUMA nodes,
there is already mapping for CPU hardware ID (MPIDR) and CPU logical ID,
and MPIDR will be not changed, why not using MPIDR for the mapping of
NUMA node and CPU? then the mappings will be:
CPU logical ID <--> CPU MPIDR <-> NUMA node ID <-> proximity domain
(allocated by OS) (constant) (allocated by OS)
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] dt:numa: adding numa node mapping for memory nodes.
On 2014-9-18 5:48, Nathan Lynch wrote: > On 09/17/2014 03:56 AM, Ganapatrao Kulkarni wrote: >> From: Ganapatrao Kulkarni >> >> This patch adds property "nid" to memory node to provide the memory range to >> numa node id mapping. >> >> Signed-off-by: Ganapatrao Kulkarni >> >> --- >> Documentation/devicetree/bindings/numa.txt | 58 >> ++ >> 1 file changed, 58 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/numa.txt >> >> diff --git a/Documentation/devicetree/bindings/numa.txt >> b/Documentation/devicetree/bindings/numa.txt >> new file mode 100644 >> index 000..c4a94f2 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/numa.txt >> @@ -0,0 +1,58 @@ >> +== >> +numa id binding description >> +== >> + >> +== >> +1 - Introduction >> +== >> +The device node property "nid(numa node id)" can be added to memory >> +device node to map the range of memory addresses as defined in property >> "reg". >> +The property "nid" maps the memory range to the numa node id, which is used >> to >> +find the local and remory pages on numa aware systems. > > "Local" and "remote" memory are notions that relate to some other > resource -- typically a CPU, but also I/O resources on some systems. It > seems to me that a useful NUMA binding would at least specify a "nid" > property, or something like it, for both cpu and memory nodes. But this > document speaks only of memory nodes. Agreed. and more, I think I/O resources also need such property, it will have performance influence for the proximity domain of I/O devices too. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] dt:numa: adding numa node mapping for memory nodes.
On 2014-9-18 3:34, Mark Rutland wrote: > On Wed, Sep 17, 2014 at 04:37:30PM +0100, Kumar Gala wrote: >> >> On Sep 17, 2014, at 1:56 AM, Ganapatrao Kulkarni >> wrote: >> >>> From: Ganapatrao Kulkarni >>> >>> This patch adds property "nid" to memory node to provide the memory range to >>> numa node id mapping. >>> >>> Signed-off-by: Ganapatrao Kulkarni >>> >>> — >> >> Adding the PPC guys as they’ve been doing NUMA on IBM Power Servers >> for years with OF/DT. So we should really try and follow what they’ve >> done. > > Agreed. > >>> Documentation/devicetree/bindings/numa.txt | 58 >>> ++ >>> 1 file changed, 58 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/numa.txt >>> >>> diff --git a/Documentation/devicetree/bindings/numa.txt >>> b/Documentation/devicetree/bindings/numa.txt >>> new file mode 100644 >>> index 000..c4a94f2 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/numa.txt >>> @@ -0,0 +1,58 @@ >>> +== >>> +numa id binding description >>> +== >>> + >>> +== >>> +1 - Introduction >>> +== >>> +The device node property "nid(numa node id)" can be added to memory > > Why the quotes? > >>> +device node to map the range of memory addresses as defined in property >>> "reg". >>> +The property "nid" maps the memory range to the numa node id, which is >>> used to >>> +find the local and remory pages on numa aware systems. > > What is a "numa node id", exactly, and how is the OS intended to use it? I think "Proximity Domain" would be more suitably, processors and memory or IOs in the same domain will have better performance than crossing other domains. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/9] ACPI: Add support for device specific properties
On 2014-8-18 16:27, Mika Westerberg wrote:
> On Mon, Aug 18, 2014 at 04:13:29PM +0800, Hanjun Guo wrote:
>> Hi,
>>
>> Some minor comments below.
>>
>> On 2014-8-17 14:04, Mika Westerberg wrote:
>>> Device Tree is used in many embedded systems to describe the system
>>> configuration to the OS. It supports attaching properties or name-value
>>> pairs to the devices it describe. With these properties one can pass
>>> additional information to the drivers that would not be available
>>> otherwise.
>>>
>>> ACPI is another configuration mechanism (among other things) typically
>>> seen, but not limited to, x86 machines. ACPI allows passing arbitrary
>>> data from methods but there has not been mechanism equivalent to Device
>>> Tree until the introduction of _DSD in the recent publication of the
>>> ACPI 5.1 specification.
>>>
>>> In order to facilitate ACPI usage in systems where Device Tree is
>>> typically used, it would be beneficial to standardize a way to retrieve
>>> Device Tree style properties from ACPI devices, which is what we do in
>>> this patch.
>>>
>>> If a given device described in ACPI namespace wants to export properties it
>>> must implement _DSD method (Device Specific Data, introduced with ACPI 5.1)
>>> that returns the properties in a package of packages. For example:
>>>
>>> Name (_DSD, Package () {
>>> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>> Package () {
>>> Package () {"name1", },
>>> Package () {"name2", },
>>> ...
>>> }
>>> })
>>>
>>> The UUID reserved for properties is daffd814-6eba-4d8c-8a91-bc9bbf4aa301
>>> and is documented in the ACPI 5.1 companion document called "_DSD
>>> Implementation Guide" [1], [2].
>>>
>>> We add several helper functions that can be used to extract these
>>> properties and convert them to different Linux data types.
>>>
>>> The ultimate goal is that we only have one device property API that
>>> retrieves the requested properties from Device Tree or from ACPI
>>> transparent to the caller.
>>>
>>> [1]
>>> http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm
>>> [2]
>>> http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
>>>
>>> Signed-off-by: Darren Hart
>>> Signed-off-by: Rafael J. Wysocki
>>> Signed-off-by: Mika Westerberg
>>> ---
>>> drivers/acpi/Makefile | 1 +
>>> drivers/acpi/internal.h | 6 +
>>> drivers/acpi/property.c | 365
>>>
>>> drivers/acpi/scan.c | 2 +
>>> include/acpi/acpi_bus.h | 7 +
>>> include/linux/acpi.h| 40 ++
>>> 6 files changed, 421 insertions(+)
>>> create mode 100644 drivers/acpi/property.c
>>>
>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>> index ea55e0179f81..6e8269a111db 100644
>>> --- a/drivers/acpi/Makefile
>>> +++ b/drivers/acpi/Makefile
>>> @@ -45,6 +45,7 @@ acpi-y+= acpi_pnp.o
>>> acpi-y += power.o
>>> acpi-y += event.o
>>> acpi-y += sysfs.o
>>> +acpi-y += property.o
>>> acpi-$(CONFIG_X86) += acpi_cmos_rtc.o
>>> acpi-$(CONFIG_DEBUG_FS)+= debugfs.o
>>> acpi-$(CONFIG_ACPI_NUMA) += numa.o
>>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>>> index 7de5b603f272..35001e2a8769 100644
>>> --- a/drivers/acpi/internal.h
>>> +++ b/drivers/acpi/internal.h
>>> @@ -178,4 +178,10 @@ struct platform_device
>>> *acpi_create_platform_device(struct acpi_device *adev);
>>> bool acpi_osi_is_win8(void);
>>> #endif
>>>
>>> +/*--
>>> + Device properties
>>> +
>>> --
>>> */
>>> +void acpi_init_properties(struct acpi_device *adev);
>>> +void acpi_free_properties(struct acpi_device *adev);
>>> +
>>> #endif /* _ACPI_INTERNAL_H_ */
>>&
Re: [RFC PATCH 1/9] ACPI: Add support for device specific properties
affd814-6eba-4d8c-8a91-bc9bbf4aa301 */
> +static const u8 prp_uuid[16] = {
s/prp_uuid/dsd_uuid ?
> + 0x14, 0xd8, 0xff, 0xda, 0xba, 0x6e, 0x8c, 0x4d,
> + 0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01
> +};
> +
> +static bool acpi_property_value_ok(const union acpi_object *value)
> +{
> + int j;
> +
> + /*
> + * The value must be an integer, a string, a reference, or a package
> + * whose every element must be an integer, a string, or a reference.
> + */
> + switch (value->type) {
> + case ACPI_TYPE_INTEGER:
> + case ACPI_TYPE_STRING:
> + case ACPI_TYPE_LOCAL_REFERENCE:
> + return true;
> +
> + case ACPI_TYPE_PACKAGE:
> + for (j = 0; j < value->package.count; j++)
> + switch (value->package.elements[j].type) {
> + case ACPI_TYPE_INTEGER:
> + case ACPI_TYPE_STRING:
> + case ACPI_TYPE_LOCAL_REFERENCE:
> + continue;
> +
> + default:
> + return false;
> + }
> +
> + return true;
> + }
> + return false;
> +}
> +
> +static bool acpi_properties_format_valid(const union acpi_object *properties)
> +{
> + int i;
> +
> + for (i = 0; i < properties->package.count; i++) {
> + const union acpi_object *property;
> +
> + property = &properties->package.elements[i];
> + /*
> + * Only two elements allowed, the first one must be a string and
> + * the second one has to satisfy certain conditions.
> + */
> + if (property->package.count != 2
> + || property->package.elements[0].type != ACPI_TYPE_STRING
> + || !acpi_property_value_ok(&property->package.elements[1]))
> + return false;
> + }
> + return true;
> +}
> +
> +void acpi_init_properties(struct acpi_device *adev)
> +{
> +
I think the above line is not needed.
With that fixed,
Reviewed-by: Hanjun Guo
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/8] arm64: kernel: refactor the CPU suspend API for retention states
On 2014-8-13 23:52, Lorenzo Pieralisi wrote:
> CPU suspend is the standard kernel interface to be used to enter
> low-power states on ARM64 systems. Current cpu_suspend implementation
> by default assumes that all low power states are losing the CPU context,
> so the CPU registers must be saved and cleaned to DRAM upon state
> entry. Furthermore, the current cpu_suspend() implementation assumes
> that if the CPU suspend back-end method returns when called, this has
> to be considered an error regardless of the return code (which can be
> successful) since the CPU was not expected to return from a code path that
> is different from cpu_resume code path - eg returning from the reset vector.
>
> All in all this means that the current API does not cope well with low-power
> states that preserve the CPU context when entered (ie retention states),
> since first of all the context is saved for nothing on state entry for
> those states and a successful state entry can return as a normal function
> return, which is considered an error by the current CPU suspend
> implementation.
>
> This patch refactors the cpu_suspend() API so that it can be split in
> two separate functionalities. The arm64 cpu_suspend API just provides
> a wrapper around CPU suspend operation hook. A new function is
> introduced (for architecture code use only) for states that require
> context saving upon entry:
>
> __cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
>
> __cpu_suspend() saves the context on function entry and calls the
> so called suspend finisher (ie fn) to complete the suspend operation.
> The finisher is not expected to return, unless it fails in which case
> the error is propagated back to the __cpu_suspend caller.
>
> The API refactoring results in the following pseudo code call sequence for a
> suspending CPU, when triggered from a kernel subsystem:
>
> /*
> * int cpu_suspend(unsigned long idx)
> * @idx: idle state index
> */
> {
> -> cpu_suspend(idx)
> |---> CPU operations suspend hook called, if present
> |--> if (retention_state)
> |--> direct suspend back-end call (eg PSCI suspend)
>else
> |--> __cpu_suspend(idx, &back_end_finisher);
> }
>
> By refactoring the cpu_suspend API this way, the CPU operations back-end
> has a chance to detect whether idle states require state saving or not
> and can call the required suspend operations accordingly either through
> simple function call or indirectly through __cpu_suspend() which carries out
> state saving and suspend finisher dispatching to complete idle state entry.
>
> Signed-off-by: Lorenzo Pieralisi
This patch is pretty fine to me,
Reviewed-by: Hanjun Guo
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] ACPICA: Only include ACPI asm files if ACPI is enabled
On 2014-6-5 9:14, Zheng, Lv wrote: > Hi, Lee > >> From: Lee Jones [mailto:lee.jo...@linaro.org] >> Sent: Wednesday, June 04, 2014 8:52 PM >> To: Rafael J. Wysocki >> >> On Wed, 04 Jun 2014, Rafael J. Wysocki wrote: >> >>> On Wednesday, June 04, 2014 01:09:50 PM Lee Jones wrote: Any drivers which support ACPI and Device Tree probing need to include both respective header files. Without this patch, if a driver is being used on a platform which does not support ACPI and subsequently does not have the config option enabled, but includes linux/acpi.h the build breaks with: In file included from ../include/acpi/platform/acenv.h:150:0, from ../include/acpi/acpi.h:56, from ../include/linux/match.h:2, from ../drivers/i2c/i2c-core.c:43: ../include/acpi/platform/aclinux.h:73:23: fatal error: asm/acenv.h: No such file or directory #include ^ > > Note that: > In our tree: > is only included by . > And is only included by > 1. when CONFIG_ACPI enabled > 2. - this is x86 specific, we'll clean it up by > implementing stubs for all ACPI external interfaces. > So there is no case we need to exclude when CONFIG_ACPI is not > enabled. > > I cannot find linux/match.h here. > If want to include ACPI features, it shouldn't include > , but should include . > Please refer to: > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8b48463f > And stop including directly in any cases. Ah, I agree, please ignore my previous email, sorry for the noise. Since it is very important to include but not , can we document it somewhere as the guidance? Then people will not make such mistake :) Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] ACPICA: Only include ACPI asm files if ACPI is enabled
Hi Lv, On 2014-6-5 8:56, Zheng, Lv wrote: > Hi, Lee > >> From: Lee Jones [mailto:lee.jo...@linaro.org] >> Sent: Wednesday, June 04, 2014 8:10 PM >> >> Any drivers which support ACPI and Device Tree probing need to include >> both respective header files. Without this patch, if a driver is being >> used on a platform which does not support ACPI and subsequently does not >> have the config option enabled, but includes linux/acpi.h the build >> breaks with: >> >> In file included from ../include/acpi/platform/acenv.h:150:0, >>from ../include/acpi/acpi.h:56, >>from ../include/linux/match.h:2, >>from ../drivers/i2c/i2c-core.c:43: >> ../include/acpi/platform/aclinux.h:73:23: >>fatal error: asm/acenv.h: No such file or directory >>#include >>^ >> Cc: Lv Zheng >> Cc: Rafael J. Wysocki >> Cc: linux-a...@vger.kernel.org >> Cc: de...@acpica.org >> Signed-off-by: Lee Jones >> --- >> include/acpi/platform/aclinux.h | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/include/acpi/platform/aclinux.h >> b/include/acpi/platform/aclinux.h >> index cd1f052..fdf7663 100644 >> --- a/include/acpi/platform/aclinux.h >> +++ b/include/acpi/platform/aclinux.h >> @@ -70,9 +70,10 @@ >> #ifdef EXPORT_ACPI_INTERFACES >> #include >> #endif >> -#include >> >> -#ifndef CONFIG_ACPI >> +#ifdef CONFIG_ACPI >> +#include >> +#else > > This is exactly what I want to do in the next step. > But you are a bit faster here. > I believe: > The miss-ordered inclusions of is the culprit of all of the > miss-ordered inclusions in arch/x86/include/asm. > You should have noted that was originally unexpected included by > some x86 specific headers. > Simply doing exlusion in this way might be able to fix your > issue for your architecture, but it could be very likely breaking x86 builds. > You might be able to find another way to solve your build issue - for > example, creating an empty for arch/arm. Yes, we solve this issue as you suggested for arch/arm64. since ARM32 will not support ACPI in the near future, we may find another way to fix it. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 6/6] pci: Add support for creating a generic host_bridge from device tree
Hi Liviu,
On 2014-3-14 23:34, Liviu Dudau wrote:
> Several platforms use a rather generic version of parsing
> the device tree to find the host bridge ranges. Move the common code
> into the generic PCI code and use it to create a pci_host_bridge
> structure that can be used by arch code.
>
> Based on early attempts by Andrew Murray to unify the code.
> Used powerpc and microblaze PCI code as starting point.
>
> Signed-off-by: Liviu Dudau
> Tested-by: Tanmay Inamdar
> ---
> drivers/pci/host-bridge.c | 158
> include/linux/pci.h | 13 +++
> 2 files changed, 171 insertions(+)
>
> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> index 8708b652..7cda90b 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c
> @@ -6,9 +6,14 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
>
> #include "pci.h"
>
> +static atomic_t domain_nr = ATOMIC_INIT(-1);
domain_nr will only be used inside the #ifdef CONFIG_OF,
and this will lead to compile warning which complains that
'domain_nr' defined but not used when CONFIG_OF=n (for example
on x86).
How about moving the definition to --->
> +
> static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
> {
> while (bus->parent)
> @@ -92,3 +97,156 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct
> resource *res,
> res->end = region->end + offset;
> }
> EXPORT_SYMBOL(pcibios_bus_to_resource);
> +
> +#ifdef CONFIG_OF
here?
Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
