Re: [PATCH 5/8] powerpc: Drop XILINX MAINTAINERS entry
On 24/02/2020 23:31, Michael Ellerman wrote: This has been orphaned for ~7 years, remove it. Cc: Grant Likely Signed-off-by: Michael Ellerman Acked-by: Grant Likely --- MAINTAINERS | 6 -- 1 file changed, 6 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 939da2ac08db..d5db5cac5a39 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9668,12 +9668,6 @@ L: linuxppc-dev@lists.ozlabs.org S:Maintained F:arch/powerpc/platforms/8xx/ -LINUX FOR POWERPC EMBEDDED XILINX VIRTEX -L: linuxppc-dev@lists.ozlabs.org -S: Orphan -F: arch/powerpc/*/*virtex* -F: arch/powerpc/*/*/*virtex* - LINUX KERNEL DUMP TEST MODULE (LKDTM) M:Kees Cook S:Maintained
Re: drivers binding to device node with multiple compatible strings
On 04/10/2018 10:32, Grant Likely wrote: On Fri, Sep 28, 2018 at 10:01 PM Li Yang wrote: On Fri, Sep 28, 2018 at 3:07 PM Rob Herring wrote: On Thu, Sep 27, 2018 at 5:25 PM Li Yang wrote: Hi Rob and Grant, Various device tree specs are recommending to include all the potential compatible strings in the device node, with the order from most specific to most general. But it looks like Linux kernel doesn't provide a way to bind the device to the most specific driver, however, the first registered compatible driver will be bound. As more and more generic drivers are added to the Linux kernel, they are competing with the more specific vendor drivers and causes problem when both are built into the kernel. I'm wondering if there is a generic solution (or in plan) to make the most specific driver bound to the device. Or we have to disable the more general driver or remove the more general compatible string from the device tree? It's been a known limitation for a long time. However, in practice it doesn't seem to be a common problem. Perhaps folks just remove the less specific compatible from their DT (though that's not ideal). For most modern bindings, there's so many other resources beyond compatible (clocks, resets, pinctrl, etc.) that there are few generic drivers that can work. I guess if we want to fix this, we'd need to have weighted matching in the driver core and unbind drivers when we get a better match. Though it could get messy if the better driver probe fails. Then we've got to rebind to the original driver. Probably we can populate the platform devices from device tree after the device_init phase? So that all built-in drivers are already registered when the devices are created and we can try find the best match in one go? For more specific loadable modules we probably need to unbind from the old driver and bind to the new one. But I agree with you that it could be messy. It's a tradeoff. Oops! Accidentally hit send too early. It's a tradeoff. If the platform device population is deferred until after all drivers are loaded, then there isn't any mechanism to ensure some devices get probed early in the init sequence. As Rob said, while it is a problem in theory, there haven't been a lot of actual cases where it is a problem. The solution has been to either remove the generic match from the device, or we can blacklist particular devices from the generic driver. g. Do you have a specific case where you hit this? Maybe not a new issue but "snps,dw-pcie" is competing with various "fsl,-pcie" compatibles. Also a specific PHY "ethernet-phy-id." with generic "ethernet-phy-ieee802.3-c45". Regards, Leo
Re: drivers binding to device node with multiple compatible strings
On Fri, Sep 28, 2018 at 10:01 PM Li Yang wrote: > > On Fri, Sep 28, 2018 at 3:07 PM Rob Herring wrote: > > > > On Thu, Sep 27, 2018 at 5:25 PM Li Yang wrote: > > > > > > Hi Rob and Grant, > > > > > > Various device tree specs are recommending to include all the > > > potential compatible strings in the device node, with the order from > > > most specific to most general. But it looks like Linux kernel doesn't > > > provide a way to bind the device to the most specific driver, however, > > > the first registered compatible driver will be bound. > > > > > > As more and more generic drivers are added to the Linux kernel, they > > > are competing with the more specific vendor drivers and causes problem > > > when both are built into the kernel. I'm wondering if there is a > > > generic solution (or in plan) to make the most specific driver bound > > > to the device. Or we have to disable the more general driver or > > > remove the more general compatible string from the device tree? > > > > It's been a known limitation for a long time. However, in practice it > > doesn't seem to be a common problem. Perhaps folks just remove the > > less specific compatible from their DT (though that's not ideal). For > > most modern bindings, there's so many other resources beyond > > compatible (clocks, resets, pinctrl, etc.) that there are few generic > > drivers that can work. > > > > I guess if we want to fix this, we'd need to have weighted matching in > > the driver core and unbind drivers when we get a better match. Though > > it could get messy if the better driver probe fails. Then we've got to > > rebind to the original driver. > > Probably we can populate the platform devices from device tree after > the device_init phase? So that all built-in drivers are already > registered when the devices are created and we can try find the best > match in one go? For more specific loadable modules we probably need > to unbind from the old driver and bind to the new one. But I agree > with you that it could be messy. It's a tradeoff. > > > > > Do you have a specific case where you hit this? > > Maybe not a new issue but "snps,dw-pcie" is competing with various > "fsl,-pcie" compatibles. Also a specific PHY > "ethernet-phy-id." with generic "ethernet-phy-ieee802.3-c45". > > Regards, > Leo
Re: DT case sensitivity
On 23/08/2018 13:08, Rob Herring wrote: On Thu, Aug 23, 2018 at 6:48 AM Benjamin Herrenschmidt wrote: On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote: On Thu, Aug 23, 2018 at 4:02 AM Grant Likely wrote: What problem are you trying to solve? I'm looking at removing device_node.name and using full_name instead (which now is only the local node name plus unit-address). This means replacing of_node_cmp() (and still some strcmp) calls in a lot of places. I need to use either strncmp or strncasecmp instead. I would think making everything case insensitive would be the direction to go if you do anything. Least possibility of breaking existing platforms in that scenario. Really? Even if all the "new" arches are effectively case sensitive? Anything using dtc and libfdt are (and json-schema certainly will be). But I frequently say the kernel's job is not DT validation, so you pass crap in, you get undefined results. I tend to agree with Grant. Let's put it this way: What is the drawback of being case insensitive ? It's a more expensive comparison. I don't think it's a hot path, but we do do a lot of string compares. I'd hazard to argue that the cost of the string compare will not be a source of performance problems when compared to doing a linear search everytime a DT lookup is performed. The search algorithm is far more problematic. Property names are case sensitive already. It would be nice if everything was treated the same way. If we're case sensitive then it is well defined which node we'll match and which one will be ignored. Otherwise, it is whichever one we happen to match first. If case-insensitive-always is chosen, then the kernel should probably complain at add node time if another matching node name already exists. That's a relatively easy thing to test. You are right however that in the FDT world we're case-sensitive now and if it is relaxed then we're never going back. You could try switching everyone over to case-sensitive and see if anything falls out in linux-next for a few cycles. I would not touch the PPC or SPARC behaviour. I don't think it is worth the risk to make the kernel more strict when we cannot test the result. Only if everything was changed to case-insensitive would it make sense to touch PPC and SPARC at the same time because it reduces the number of platform variations. If you do change compatible matches to be case sensitive, then you should be prepared to fix bugs where the driver uses a different case from the DTS. In which case drivers will need to be modified to accept the deviant property names. g. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Re: DT case sensitivity
On 23/08/2018 12:47, Benjamin Herrenschmidt wrote: On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote: On Thu, Aug 23, 2018 at 4:02 AM Grant Likely wrote: What problem are you trying to solve? I'm looking at removing device_node.name and using full_name instead (which now is only the local node name plus unit-address). This means replacing of_node_cmp() (and still some strcmp) calls in a lot of places. I need to use either strncmp or strncasecmp instead. Makes sense. Simplifies the code. I would think making everything case insensitive would be the direction to go if you do anything. Least possibility of breaking existing platforms in that scenario. Really? Even if all the "new" arches are effectively case sensitive? Anything using dtc and libfdt are (and json-schema certainly will be). But I frequently say the kernel's job is not DT validation, so you pass crap in, you get undefined results. I tend to agree with Grant. Let's put it this way: What is the drawback of being case insensitive ? Do we expect that there exist a case where we will want to distinguish between nodes that have the same name with a different case ? > If not, I don't see the point of being strict about it. I'd also add that any place where there are two nodes or props with the same name, but different case, then it is probably a bug (or a really bad design). Going the direction of case-insensitive eliminates the possibility. I do understand that that you'd like the kernel to be strict about what it accepts, but that strictness probably makes more sense back in DTC where it can also be checked against schema. g. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Re: DT case sensitivity
On 23/08/2018 02:29, Benjamin Herrenschmidt wrote: On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote: On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt wrote: On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote: The default DT string handling in the kernel is node names and compatibles are case insensitive and property names are case sensitive (Sparc is the the only variation and is opposite). It seems only PPC (and perhaps only Power Macs?) needs to support case insensitive comparisons. It was probably a mistake to follow PPC for new arches and we should have made everything case sensitive from the start. So I have a few questions for the DT historians. :) Open Firmware itself is insensitive. Doesn't it depend on the implementation? Otherwise, how is Sparc different? Not sure ... Forth itself is insensitive for words but maybe not for string comparisons. What problem are you trying to solve? I would think making everything case insensitive would be the direction to go if you do anything. Least possibility of breaking existing platforms in that scenario. g. What PPC systems are case insensitive? Can we limit that to certain systems? All PowerMacs at least, the problem is that I don't have DT images or access to all the historical systems (and yes some people occasionally still use them) to properly test a change in that area. I'm temped to break them so I can find folks to provide me with DT dumps. :) I have a collection of DT dumps but I'm not sure about the legality of publishing them... Cheers, Ben. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Re: [git pull] Please pull powerpc.git next branch
On Sun, Aug 8, 2010 at 10:11 PM, Benjamin Herrenschmidt wrote: > Hi Linus ! > > Here's a few misc things for .36. The big bit is that I trimmed all the > defconfigs using make savedefconfig. [...] > 108 files changed, 270 insertions(+), 134609 deletions(-) And how is anyone else to make it into the kernel statistics top contributors by lines changed list with stuff like this going in? :-) g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Devicetree specification process
On Sat, Apr 30, 2016 at 12:23 AM, Grant Likely <grant.lik...@linaro.org> wrote: > I'm pleased to announce the first *very early draft* of the new > devicetree specification that I and several others have been working > on since January. This document picks up where ePAPR left off in 2012. > I'm announcing it now along with the new devicetree.org organization > which will handle governance for the spec. > > We're still in the process of getting all the details sorted out (not > everything on the website is accurate at the moment), but in the mean > time you can look at the current state of the document. At the moment, > it is little more than repackaging the ePAPR text. The next step will > be pulling in the new core bindings that were never covered by ePAPR. > You can look at a draft copy of the document here: > > https://github.com/kvekaria/devicetree-specification-released/blob/master/prerelease/DevicetreeSpecification-20160429-pre1.pdf Oops, wrong link. This should be: https://github.com/devicetree-org/devicetree-specification-released/blob/master/prerelease/devicetree-specification-v0.1-pre1-20160429.pdf g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Fwd: Devicetree specification process
I'm pleased to announce the first *very early draft* of the new devicetree specification that I and several others have been working on since January. This document picks up where ePAPR left off in 2012. I'm announcing it now along with the new devicetree.org organization which will handle governance for the spec. We're still in the process of getting all the details sorted out (not everything on the website is accurate at the moment), but in the mean time you can look at the current state of the document. At the moment, it is little more than repackaging the ePAPR text. The next step will be pulling in the new core bindings that were never covered by ePAPR. You can look at a draft copy of the document here: https://github.com/kvekaria/devicetree-specification-released/blob/master/prerelease/DevicetreeSpecification-20160429-pre1.pdf If you want to help out, the document source markup can be cloned from GitHub. You can find the project page here: https://github.com/devicetree-org/devicetree-specification Feedback is greatly appreciate. Please send any comments and/or changes to the devicetree specification mailing list: devicetree-s...@vger.kernel.org For more information, you can refer to the slides presented at BKK16 and ELC[1]. [1] http://elinux.org/images/b/b7/Devicetree_specification_linaro_connect_bangkok_2016.pdf Thanks, g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Fix pSeries boot failure, by returning interrupt controller node when an interrupt-map property doesn't exist
On Tue, 30 Jun 2015 13:23:33 +1000 , Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Mon, 2015-06-29 at 18:50 -0500, Jeremy Linton wrote: This is a reformat of the patch attached to pSeries boot failure due to wrong interrupt controller. It allows of_irq_parse_raw() to return the node pointer of the interrupt controller, rather than the parent bus. This allows ics_rtas_host_match() to detect that the controller is a legacy 8259 and avoid using xics. This avoids an RTAS assertion/crash during early kernel bootstrapping Signed-off-by: Jeremy Linton lintonrjer...@gmail.com Reviewed-by: Benjamin Herrenschmidt b...@kernel.crashing.org --- Rob, do you want to take this or should we ? Merged, thanks. Jeremy, please check your mailer configuration. The patch was mangled and would not apply. I had to fix it up manually. Also, does this patch need to be backported into stable? What commit introduced this bug, and which kernel does it first appear in? Thanks, g. --- drivers/of/irq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 1a79806..cb4b9ae 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -252,8 +252,6 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq) * Successfully parsed an interrrupt-map translation; copy new * interrupt specifier into the out_irq structure */ - out_irq-np = newpar; - match_array = imap - newaddrsize - newintsize; for (i = 0; i newintsize; i++) out_irq-args[i] = be32_to_cpup(imap - newintsize + i); @@ -262,6 +260,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq) skiplevel: /* Iterate again with new parent */ + out_irq-np = newpar; pr_debug( - new parent: %s\n, of_node_full_name(newpar)); of_node_put(ipar); ipar = newpar; -- 1.8.1.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v5 39/42] drivers/of: Unflatten nodes equal or deeper than specified level
On Thu, 4 Jun 2015 16:42:08 +1000
, Gavin Shan gws...@linux.vnet.ibm.com
wrote:
unflatten_dt_node() is called recursively to unflatten FDT nodes
with the assumption that FDT blob has only one root node, which
isn't true when the FDT blob represents device sub-tree. The
patch improves the function to supporting device sub-tree that
have multiple root nodes:
* Rename original unflatten_dt_node() to __unflatten_dt_node().
* Wrapper unflatten_dt_node() calls __unflatten_dt_node() with
adjusted current node depth to 1 to avoid underflow.
Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
v5:
* Split from PATCH[v4 19/21]
* Fixed line over 80 characters from checkpatch.pl
---
drivers/of/fdt.c | 56
++--
1 file changed, 38 insertions(+), 18 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index cde35c5d01..b87c157 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -28,6 +28,8 @@
#include asm/setup.h /* for COMMAND_LINE_SIZE */
#include asm/page.h
+static int cur_node_depth;
+
k! We'll never be able to call this concurrently this way. That will
create theoretical race conditions in the overlay code. (actually, you
didn't introduce this problem, see below...)
/*
* of_fdt_limit_memory - limit the number of regions in the /memory node
* @limit: maximum entries
@@ -161,27 +163,26 @@ static void *unflatten_dt_alloc(void **mem, unsigned
long size,
}
/**
- * unflatten_dt_node - Alloc and populate a device_node from the flat tree
+ * __unflatten_dt_node - Alloc and populate a device_node from the flat tree
* @blob: The parent device tree blob
* @mem: Memory chunk to use for allocating device nodes and properties
* @p: pointer to node in flat tree
* @dad: Parent struct device_node
* @fpsize: Size of the node path up at the current depth.
*/
-static void * unflatten_dt_node(void *blob,
- void *mem,
- int *poffset,
- struct device_node *dad,
- struct device_node **nodepp,
- unsigned long fpsize,
- bool dryrun)
+static void *__unflatten_dt_node(void *blob,
+ void *mem,
+ int *poffset,
+ struct device_node *dad,
+ struct device_node **nodepp,
+ unsigned long fpsize,
+ bool dryrun)
nitpick: If you resist the temptation to reflow indentation, then the
diffstat is smaller.
{
const __be32 *p;
struct device_node *np;
struct property *pp, **prev_pp = NULL;
const char *pathp;
unsigned int l, allocl;
- static int depth = 0;
H.. looks like the race condition is already there. Well that's no
good. If you move *depth into the parameters to unflatten_dt_node(), then
you can solve both problems at once without having to create a __
version of the function. That will be a cleaner solution overall.
int old_depth;
int offset;
int has_name = 0;
@@ -334,13 +335,19 @@ static void * unflatten_dt_node(void *blob,
np-type = NULL;
}
- old_depth = depth;
- *poffset = fdt_next_node(blob, *poffset, depth);
- if (depth 0)
- depth = 0;
- while (*poffset 0 depth old_depth)
- mem = unflatten_dt_node(blob, mem, poffset, np, NULL,
- fpsize, dryrun);
+ old_depth = cur_node_depth;
+ *poffset = fdt_next_node(blob, *poffset, cur_node_depth);
+ while (*poffset 0) {
What is the reasoning here? Why change to looking for poffset 0?
+ if (cur_node_depth old_depth)
+ break;
+
+ if (cur_node_depth == old_depth)
+ mem = __unflatten_dt_node(blob, mem, poffset,
+ dad, NULL, fpsize, dryrun);
+ else if (cur_node_depth old_depth)
+ mem = __unflatten_dt_node(blob, mem, poffset,
+ np, NULL, fpsize, dryrun);
Ditto here, please describe the purpose of the new logic.
+ }
if (*poffset 0 *poffset != -FDT_ERR_NOTFOUND)
pr_err(unflatten: error %d processing FDT\n, *poffset);
@@ -366,6 +373,18 @@ static void * unflatten_dt_node(void *blob,
return mem;
}
+static void *unflatten_dt_node(void *blob,
+void *mem,
+int *poffset,
+struct device_node *dad,
+struct device_node **nodepp,
+bool dryrun)
+{
+ cur_node_depth = 1;
+ return __unflatten_dt_node(blob, mem,
Re: [PATCH v5 42/42] pci/hotplug: PowerPC PowerNV PCI hotplug driver
On Thu, 4 Jun 2015 16:42:11 +1000
, Gavin Shan gws...@linux.vnet.ibm.com
wrote:
The patch intends to add standalone driver to support PCI hotplug
for PowerPC PowerNV platform, which runs on top of skiboot firmware.
The firmware identified hotpluggable slots and marked their device
tree node with proper ibm,slot-pluggable and ibm,reset-by-firmware.
The driver simply scans device-tree to create/register PCI hotplug slot
accordingly.
If the skiboot firmware doesn't support slot status retrieval, the PCI
slot device node shouldn't have property ibm,reset-by-firmware. In
that case, none of valid PCI slots will be detected from device tree.
The skiboot firmware doesn't export the capability to access attention
LEDs yet and it's something for TBD.
Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
v5:
* Use OF OVERLAY to update the device-tree
* Removed unnecessary header files
* More meaningful return value from powernv_php_register_one()
* Use pnv_pci_hotplug_notifier_{register, unregister}()
* Decimal values for slot's states
* Removed struct powernv_php_slot::release()
* Merged two bool arguments to one for powernv_php_slot_enable()
* Rename release_device_nodes_info() to remove_device_nodes_info()
* Don't check on !len in slot_power_on_handler()
* Handle return value in get_adapter_status() as suggested by aik
* Drop invalid attention status in set_attention_status()
* Renaming functions
* Fixed coding style and added entry in MAINTAINERS reported by
checkpatch.pl
---
MAINTAINERS| 6 +
drivers/pci/hotplug/Kconfig| 12 +
drivers/pci/hotplug/Makefile | 4 +
drivers/pci/hotplug/powernv_php.c | 140 +++
drivers/pci/hotplug/powernv_php.h | 90
drivers/pci/hotplug/powernv_php_slot.c | 732
+
6 files changed, 984 insertions(+)
create mode 100644 drivers/pci/hotplug/powernv_php.c
create mode 100644 drivers/pci/hotplug/powernv_php.h
create mode 100644 drivers/pci/hotplug/powernv_php_slot.c
diff --git a/MAINTAINERS b/MAINTAINERS
index e308718..f5e1dce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7481,6 +7481,12 @@ L: linux-...@vger.kernel.org
S: Supported
F: Documentation/PCI/pci-error-recovery.txt
+PCI HOTPLUG DRIVER FOR POWERNV PLATFORM
+M: Gavin Shan gws...@linux.vnet.ibm.com
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/pci/hotplug/powernv_php*
+
PCI SUBSYSTEM
M: Bjorn Helgaas bhelg...@google.com
L: linux-...@vger.kernel.org
diff --git a/drivers/pci/hotplug/Kconfig b/drivers/pci/hotplug/Kconfig
index df8caec..ef55dae 100644
--- a/drivers/pci/hotplug/Kconfig
+++ b/drivers/pci/hotplug/Kconfig
@@ -113,6 +113,18 @@ config HOTPLUG_PCI_SHPC
When in doubt, say N.
+config HOTPLUG_PCI_POWERNV
+ tristate PowerPC PowerNV PCI Hotplug driver
+ depends on PPC_POWERNV EEH
+ help
+ Say Y here if you run PowerPC PowerNV platform that supports
+ PCI Hotplug
+
+ To compile this driver as a module, choose M here: the
+ module will be called powernv-php.
+
+ When in doubt, say N.
+
config HOTPLUG_PCI_RPA
tristate RPA PCI Hotplug driver
depends on PPC_PSERIES EEH
diff --git a/drivers/pci/hotplug/Makefile b/drivers/pci/hotplug/Makefile
index 4a9aa08..a69665e 100644
--- a/drivers/pci/hotplug/Makefile
+++ b/drivers/pci/hotplug/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_HOTPLUG_PCI_PCIE) += pciehp.o
obj-$(CONFIG_HOTPLUG_PCI_CPCI_ZT5550)+= cpcihp_zt5550.o
obj-$(CONFIG_HOTPLUG_PCI_CPCI_GENERIC) += cpcihp_generic.o
obj-$(CONFIG_HOTPLUG_PCI_SHPC) += shpchp.o
+obj-$(CONFIG_HOTPLUG_PCI_POWERNV)+= powernv-php.o
obj-$(CONFIG_HOTPLUG_PCI_RPA)+= rpaphp.o
obj-$(CONFIG_HOTPLUG_PCI_RPA_DLPAR) += rpadlpar_io.o
obj-$(CONFIG_HOTPLUG_PCI_SGI)+= sgi_hotplug.o
@@ -50,6 +51,9 @@ ibmphp-objs := ibmphp_core.o \
acpiphp-objs := acpiphp_core.o \
acpiphp_glue.o
+powernv-php-objs := powernv_php.o \
+ powernv_php_slot.o
+
rpaphp-objs := rpaphp_core.o \
rpaphp_pci.o\
rpaphp_slot.o
diff --git a/drivers/pci/hotplug/powernv_php.c
b/drivers/pci/hotplug/powernv_php.c
new file mode 100644
index 000..4cbff7a
--- /dev/null
+++ b/drivers/pci/hotplug/powernv_php.c
@@ -0,0 +1,140 @@
+/*
+ * PCI Hotplug Driver for PowerPC PowerNV platform.
+ *
+ * Copyright Gavin Shan, IBM Corporation 2015.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later
Re: [PATCH v5 40/42] drivers/of: Allow to specify root node in of_fdt_unflatten_tree()
On Thu, 4 Jun 2015 16:42:09 +1000
, Gavin Shan gws...@linux.vnet.ibm.com
wrote:
The patch introduces one more argument to of_fdt_unflatten_tree()
to specify the root node for the FDT blob, which is going to be
unflattened. In the result, the function can be used to unflatten
FDT blob, which represents device sub-tree in subsequent patches.
Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
In principle, looks okay. There are going to be lifecycle issues though
because nodes allocated from unflatten_dt_node cannot be cleanly freed
because they aren't allocated in the same way as OF_DYNAMIC nodes are
allocated.
It may be time to dump the special allocation of fdt.c entirely and
treat all nodes the same way, with name and properties all allocated
with normal kmallocs Investigation is needed to figure out if this
is feasible.
Comments below.
---
v5:
* Newly introduced
---
drivers/of/fdt.c | 26 ++
drivers/of/unittest.c | 2 +-
include/linux/of_fdt.h | 3 ++-
3 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index b87c157..b6a6c59 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -380,9 +380,16 @@ static void *unflatten_dt_node(void *blob,
struct device_node **nodepp,
bool dryrun)
{
+ unsigned long fpsize = 0;
+
+ if (dad)
+ fpsize = strlen(of_node_full_name(dad));
+ else
+ fpsize = 0;
The 'else' is redundant. Better yet:
unsigned long fpsize = dad ? strlen(of_node_full_name(dad)) : 0;
cur_node_depth = 1;
return __unflatten_dt_node(blob, mem, poffset,
-dad, nodepp, 0, dryrun);
+dad, nodepp, fpsize, dryrun);
}
/**
@@ -393,13 +400,15 @@ static void *unflatten_dt_node(void *blob,
* pointers of the nodes so the normal device-tree walking functions
* can be used.
* @blob: The blob to expand
+ * @dad: The root node of the created device_node tree
* @mynodes: The device_node tree created by the call
* @dt_alloc: An allocator that provides a virtual address to memory
* for the resulting tree
*/
static void __unflatten_device_tree(void *blob,
- struct device_node **mynodes,
- void * (*dt_alloc)(u64 size, u64 align))
+ struct device_node *dad,
+ struct device_node **mynodes,
+ void * (*dt_alloc)(u64 size, u64 align))
Same comment as before, don't reflow the indentation unless you really
need to.
{
unsigned long size;
int start;
@@ -425,7 +434,7 @@ static void __unflatten_device_tree(void *blob,
/* First pass, scan for size */
start = 0;
size = (unsigned long)unflatten_dt_node(blob, NULL, start,
- NULL, NULL, true);
+ dad, NULL, true);
size = ALIGN(size, 4);
pr_debug( size is %lx, allocating...\n, size);
@@ -440,7 +449,7 @@ static void __unflatten_device_tree(void *blob,
/* Second pass, do actual unflattening */
start = 0;
- unflatten_dt_node(blob, mem, start, NULL, mynodes, false);
+ unflatten_dt_node(blob, mem, start, dad, mynodes, false);
if (be32_to_cpup(mem + size) != 0xdeadbeef)
pr_warning(End of tree marker overwritten: %08x\n,
be32_to_cpup(mem + size));
@@ -462,9 +471,10 @@ static void *kernel_tree_alloc(u64 size, u64 align)
* can be used.
*/
void of_fdt_unflatten_tree(unsigned long *blob,
- struct device_node **mynodes)
+struct device_node *dad,
+struct device_node **mynodes)
{
- __unflatten_device_tree(blob, mynodes, kernel_tree_alloc);
+ __unflatten_device_tree(blob, dad, mynodes, kernel_tree_alloc);
}
EXPORT_SYMBOL_GPL(of_fdt_unflatten_tree);
@@ -1095,7 +1105,7 @@ bool __init early_init_dt_scan(void *params)
*/
void __init unflatten_device_tree(void)
{
- __unflatten_device_tree(initial_boot_params, of_root,
+ __unflatten_device_tree(initial_boot_params, NULL, of_root,
early_init_dt_alloc_memory_arch);
/* Get pointer to /chosen and /aliases nodes for use everywhere */
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 1801634..2270830 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -907,7 +907,7 @@ static int __init unittest_data_add(void)
not running tests\n, __func__);
return -ENOMEM;
}
- of_fdt_unflatten_tree(unittest_data, unittest_data_node);
+ of_fdt_unflatten_tree(unittest_data, NULL, unittest_data_node);
if (!unittest_data_node) {
Re: [PATCH v4 19/21] drivers/of: Support adding sub-tree
On Mon, Jun 8, 2015 at 9:57 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Sun, 2015-06-07 at 08:54 +0100, Grant Likely wrote: IE. conceptually, what overlays do today is quite rooted around the idea of having a fixed base DT and some pre-compiled DTB overlays that get added/removed. The design completely ignore the idea of a FW that maintains a live tree which we want to keep in sync, which is what we want to do here, or what we could do with a live open firmware implementation. Right, which is exactly the reason for the changeset/overlay split. Overlays assume a fixed base, and that overlays are kind of like plug-in modules. changeset makes no such assumption. So you suggest we create a function that takes an fdt and an anchor as input, and expands that FDT below that anchor, but does so by using the changeset API under the hood ? Even that looks somewhat tricky (turn that bit of FDT into a pile of changeset actions), however, I can see how we could create a new function inside changeset to attach a subtree. Ie. of_attach_subtree() (which could have it's own reconfig action but we don't care that much yet), which takes an expanded subtree and an anchor, and calls of_attach_node() in effect for all nodes in there. We could then have a two pass mechanism in our hotplug code: - Expand the bit of fdt into a separate tree - Use of_attach_subtree to add that subtree to the main tree What do you think ? I like that. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4 19/21] drivers/of: Support adding sub-tree
Sorry for not weighing in earlier, I've had other work keeping me away. My short answer: don't use overlays. They're not what you need. Generic CONFIG_OF_DYNAMIC should be all that is required to make changes in your use case. Overlays are a specific api for being able to apply a set of changes in a revertable way, but as you say, it is a lot more complicated. However, overlays are built on top of the of_changeset API which is a lot simpler. It doesn't do any phandle resolution, and it doesn't do any tracking. It takes a set of changes (attach node, detach node, add property, remove property), an applies them to the live tree. At that point the changes are permenant*. It is documented in Documentation/devicetree/changesets.txt Ideally, I want all DT changes to go through the changeset API so that the lifecycle issues are delt with in one place. It also defers firing notifiers until after the entire changeset is applied. With of_attach_node/of_detach_node the notifiers are sent immediately after each change when the tree may be in an inconsistent state. For example, a driver expecting child nodes, but the child nodes haven't been added yet. Comments below... * There is an API for reverting a changeset, which simply applies the changes backwards and in reverse. The overlay code uses it, but you won't need it. On Thu, 14 May 2015 10:54:31 +1000 , Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Wed, 2015-05-13 at 19:18 -0500, Rob Herring wrote: I haven't decided really. The main thing with the current patch is I don't really like the added complexity to unflatten_dt_node. It is already a fairly complex function. Perhaps removing of hybrid as discussed will help? I agree, we should be able to make that much simpler, I was planning on sorting that out with Gavin. Ditto here. I don't want to have any new kinds of nodes created either. They are either OF_DYNAMIC, and therefore freeable, or they are not and cannot be freed. If there are things we can do to make overlays easier to use in your use case, I'd like to hear ideas. I don't really buy that being more complex than needed is an obstacle. That is very often the case to have common, scale-able solutions. I want to see a simple case be simple to support. Well, it's a LOT more complex from the FW perspective for a bunch of features we don't really need, in a way because the DT update in our case is just purely informational to avoid keeping wrong/outdated DT bits, it has little functional impact (it might have a bit for interrupt routing through bridges though). However, I am also pursuing an approach on FW side using a generation count in our nodes and properties which we could use to generate arbitrary overlays if we know what generation linux has. There might actual be a usage scenario for a generic way for our firwmare to convey DT updates to Linux for other reasons. A few things that I don't find in the overlay code (but maybe I haven't looked at it hard enough): - Can it remove nodes/properties ? Overlays: No, because I asked Pantelis to drop them. Changeset: yes, absolutely - Can it commit a changeset so it's permanently part of the main DT ? We will never have a concept of revertable changesets, if we need a subsequent update, we will get a new overlay from FW that will remove what needs to be removed and add what needs to be added. Yes IE, our current mechanism without overlay is fairly simple: - On PCI unplug, we remove all nodes below the slot (from linux), the FW does the equivalent internally. - On PCI re-plug, the FW internally builds new nodes and sends a new subtree as an FDT that we can expand/attach. Now we could consider that subtree as a changeset that can be undone, but that wouldn't work for boot time. And subsequent updates wouldn't have that concept of undoing anyway. IE. conceptually, what overlays do today is quite rooted around the idea of having a fixed base DT and some pre-compiled DTB overlays that get added/removed. The design completely ignore the idea of a FW that maintains a live tree which we want to keep in sync, which is what we want to do here, or what we could do with a live open firmware implementation. Right, which is exactly the reason for the changeset/overlay split. Overlays assume a fixed base, and that overlays are kind of like plug-in modules. changeset makes no such assumption. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: of/dynamic: Fix test for PPC_PSERIES
On Thu, 4 Jun 2015 20:57:32 +1000 (AEST) , Michael Ellerman m...@ellerman.id.au wrote: On Thu, 2015-04-06 at 09:34:41 UTC, Geert Uytterhoeven wrote: IS_ENABLED(PPC_PSERIES) always evaluates to false, as IS_ENABLED() is supposed to be used with the full Kconfig symbol name, including the CONFIG_ prefix. Add the missing CONFIG_ prefix to fix this. Fixes: a25095d451ece23b (of: Move dynamic node fixups out of powerpc and into common code) Signed-off-by: Geert Uytterhoeven geert+rene...@glider.be --- Did this bug cause any breakage? If yes, the fix should go to stable (for v3.17 and later). Yikes. Not that I've heard of. But it's reasonably new so possibly it's not hit distros that folks tend to run on those machines. I'm also not clear how it would break, it could be subtle and we've not noticed. Nathan might have more of an idea (on CC). On my machine here everything that has an ibm,phandle also has a linux,phandle, so we wouldn't hit that code path. But I'm not sure how representative that box is. cheers Still, an obvious bug. I've picked it up and marked for stable. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of: return NUMA_NO_NODE from fallback of_node_to_nid()
On Mon, 13 Apr 2015 11:49:31 -0500
, Rob Herring robherri...@gmail.com
wrote:
On Mon, Apr 13, 2015 at 8:38 AM, Konstantin Khlebnikov
khlebni...@yandex-team.ru wrote:
On 13.04.2015 16:22, Rob Herring wrote:
On Wed, Apr 8, 2015 at 11:59 AM, Konstantin Khlebnikov
khlebni...@yandex-team.ru wrote:
Node 0 might be offline as well as any other numa node,
in this case kernel cannot handle memory allocation and crashes.
Signed-off-by: Konstantin Khlebnikov khlebni...@yandex-team.ru
Fixes: 0c3f061c195c (of: implement of_node_to_nid as a weak function)
---
drivers/of/base.c |2 +-
include/linux/of.h |5 -
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 8f165b112e03..51f4bd16e613 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -89,7 +89,7 @@ EXPORT_SYMBOL(of_n_size_cells);
#ifdef CONFIG_NUMA
int __weak of_node_to_nid(struct device_node *np)
{
- return numa_node_id();
+ return NUMA_NO_NODE;
This is going to break any NUMA machine that enables OF and expects
the weak function to work.
Why? NUMA_NO_NODE == -1 -- this's standard no-affinity signal.
As I see powerpc/sparc versions of of_node_to_nid returns -1 if they
cannot find out which node should be used.
Ah, I was thinking those platforms were relying on the default
implementation. I guess any real NUMA support is going to need to
override this function. The arm64 patch series does that as well. We
need to be sure this change is correct for metag which appears to be
the only other OF enabled platform with NUMA support.
In that case, then there is little reason to keep the inline and we
can just always enable the weak function (with your change). It is
slightly less optimal, but the few callers hardly appear to be hot
paths.
Sounds like you're in agreement with this patch then? Shall I apply it?
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of: clean-up unnecessary libfdt include paths
On Wed, 3 Jun 2015 10:26:38 +0200 , Ralf Baechle r...@linux-mips.org wrote: On Wed, Jun 03, 2015 at 12:10:25AM -0500, Rob Herring wrote: Date: Wed, 3 Jun 2015 00:10:25 -0500 From: Rob Herring r...@kernel.org To: devicet...@vger.kernel.org, linux-ker...@vger.kernel.org Cc: Grant Likely grant.lik...@linaro.org, Rob Herring r...@kernel.org, Ralf Baechle r...@linux-mips.org, Benjamin Herrenschmidt b...@kernel.crashing.org, Paul Mackerras pau...@samba.org, Michael Ellerman m...@ellerman.id.au, linux-m...@linux-mips.org, linuxppc-dev@lists.ozlabs.org Subject: [PATCH] of: clean-up unnecessary libfdt include paths With the latest dtc import include fixups, it is no longer necessary to add explicit include paths to use libfdt. Remove these across the kernel. Signed-off-by: Rob Herring r...@kernel.org Cc: Ralf Baechle r...@linux-mips.org Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: Michael Ellerman m...@ellerman.id.au Cc: Grant Likely grant.lik...@linaro.org Cc: linux-m...@linux-mips.org Cc: linuxppc-dev@lists.ozlabs.org For the MIPS bits; Acked-by: Ralf Baechle r...@linux-mips.org Ralf Acked-by: Grant Likely grant.lik...@lianro.org ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: drivers/of: Add empty ranges quirk for PA-Semi
On Mon, 23 Mar 2015 15:06:35 +1100 , Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Mon, 2015-03-23 at 14:50 +1100, Michael Ellerman wrote: On Mon, 2015-23-03 at 03:16:38 UTC, Benjamin Herrenschmidt wrote: The sdc node is missing the ranges property, it needs to be treated as having an empty one otherwise translation fails for its children. Tested-by: Steven Rostedt rost...@goodmis.org Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org Fixes: 746c9e9f92dd (of/base: Fix PowerPC address parsing hack) Which went into 3.18-rc6, and was CC'ed to stable. So this should probably also go to stable no? Sure, go for it. Applied, thanks. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 07/14] of/reconfig: Always use the same structure for notifiers
On Tue, 25 Nov 2014 21:11:58 -0600
, Nathan Fontenot nf...@linux.vnet.ibm.com
wrote:
On 11/25/2014 05:07 PM, Benjamin Herrenschmidt wrote:
On Mon, 2014-11-24 at 22:33 +, Grant Likely wrote:
The OF_RECONFIG notifier callback uses a different structure depending
on whether it is a node change or a property change. This is silly, and
not very safe. Rework the code to use the same data structure regardless
of the type of notifier.
I fell pretty good about this one except...
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index b9d1dfdbe5bb..9fe6002c1d5a 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1711,12 +1711,11 @@ static void stage_topology_update(int core_id)
static int dt_update_callback(struct notifier_block *nb,
unsigned long action, void *data)
{
- struct of_prop_reconfig *update;
+ struct of_reconfig_data *update = data;
int rc = NOTIFY_DONE;
switch (action) {
case OF_RECONFIG_UPDATE_PROPERTY:
- update = (struct of_prop_reconfig *)data;
Should we assert/bug on !update-dn / update-prop ?
(Same for the rest of the patch)
Or do you reckon it's pointless ?
I'm not sure it's worth it, if those are NULL pointers the drivers/of
code would have tried to use them before invoking the notifier chain.
We won't make it this far if they're NULL.
Agreed. I'm going to merge it as-is.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 07/14] of/reconfig: Always use the same structure for notifiers
The OF_RECONFIG notifier callback uses a different structure depending
on whether it is a node change or a property change. This is silly, and
not very safe. Rework the code to use the same data structure regardless
of the type of notifier.
Signed-off-by: Grant Likely grant.lik...@linaro.org
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Cc: Rob Herring robh...@kernel.org
Cc: Pantelis Antoniou pantelis.anton...@konsulko.com
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/mm/numa.c | 3 +-
arch/powerpc/platforms/pseries/hotplug-cpu.c| 7 +++--
arch/powerpc/platforms/pseries/hotplug-memory.c | 15 +
arch/powerpc/platforms/pseries/iommu.c | 5 +--
arch/powerpc/platforms/pseries/setup.c | 5 +--
drivers/crypto/nx/nx-842.c | 4 +--
drivers/of/dynamic.c| 41 +
include/linux/of.h | 23 --
8 files changed, 54 insertions(+), 49 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index b9d1dfdbe5bb..9fe6002c1d5a 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1711,12 +1711,11 @@ static void stage_topology_update(int core_id)
static int dt_update_callback(struct notifier_block *nb,
unsigned long action, void *data)
{
- struct of_prop_reconfig *update;
+ struct of_reconfig_data *update = data;
int rc = NOTIFY_DONE;
switch (action) {
case OF_RECONFIG_UPDATE_PROPERTY:
- update = (struct of_prop_reconfig *)data;
if (!of_prop_cmp(update-dn-type, cpu)
!of_prop_cmp(update-prop-name, ibm,associativity)) {
u32 core_id;
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 5c375f93c669..f30cf4d136a4 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -340,16 +340,17 @@ static void pseries_remove_processor(struct device_node
*np)
}
static int pseries_smp_notifier(struct notifier_block *nb,
- unsigned long action, void *node)
+ unsigned long action, void *data)
{
+ struct of_reconfig_data *rd = data;
int err = 0;
switch (action) {
case OF_RECONFIG_ATTACH_NODE:
- err = pseries_add_processor(node);
+ err = pseries_add_processor(rd-dn);
break;
case OF_RECONFIG_DETACH_NODE:
- pseries_remove_processor(node);
+ pseries_remove_processor(rd-dn);
break;
}
return notifier_from_errno(err);
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 3c4c0dcd90d3..1bbb78fab530 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -183,7 +183,7 @@ static int pseries_add_mem_node(struct device_node *np)
return (ret 0) ? -EINVAL : 0;
}
-static int pseries_update_drconf_memory(struct of_prop_reconfig *pr)
+static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
{
struct of_drconf_cell *new_drmem, *old_drmem;
unsigned long memblock_size;
@@ -232,22 +232,21 @@ static int pseries_update_drconf_memory(struct
of_prop_reconfig *pr)
}
static int pseries_memory_notifier(struct notifier_block *nb,
- unsigned long action, void *node)
+ unsigned long action, void *data)
{
- struct of_prop_reconfig *pr;
+ struct of_reconfig_data *rd = data;
int err = 0;
switch (action) {
case OF_RECONFIG_ATTACH_NODE:
- err = pseries_add_mem_node(node);
+ err = pseries_add_mem_node(rd-dn);
break;
case OF_RECONFIG_DETACH_NODE:
- err = pseries_remove_mem_node(node);
+ err = pseries_remove_mem_node(rd-dn);
break;
case OF_RECONFIG_UPDATE_PROPERTY:
- pr = (struct of_prop_reconfig *)node;
- if (!strcmp(pr-prop-name, ibm,dynamic-memory))
- err = pseries_update_drconf_memory(pr);
+ if (!strcmp(rd-prop-name, ibm,dynamic-memory))
+ err = pseries_update_drconf_memory(rd);
break;
}
return notifier_from_errno(err);
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index e32e00976a94..3e5bfdafee63 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1251,10 +1251,11 @@ static struct notifier_block iommu_mem_nb = {
.notifier_call = iommu_mem_notifier,
};
-static int iommu_reconfig_notifier(struct notifier_block
Re: [PATCH] of/platform: Move platform devices under /sys/devices/platform
On Tue, 18 Nov 2014 11:36:19 +0100 , Andrzej Hajda a.ha...@samsung.com wrote: On 11/04/2014 11:45 AM, Grant Likely wrote: Currently the devices created by drivers/of/platform.c get created at the root of /sys/devices. This goes against the typical pattern for sysfs where the top level /sys/devices structure contains categories of devices, and the structure of devices is placed below that. To fix this, make the code in drivers/of/platform.c follow the drivers/base/platform.c behaviour, and use platform_bus as the default parent for all new platform_devices and amba_devices. This change has been discussed for a long time, but nobody has actually acted on it. Userspace code that expects to find devices under a fixed /sys/devices/... path will be affected. It isn't /supposed/ to do that, but if anyone complains then I'll add a default-off workaround option to put them back into the root. One of side effects of this change is that platform drivers registering other platform drivers or devices in their probe callback can deadlock due to double device_lock on platform device. This is for example case of exynos_drm driver[1]. I guess it could/should be fixed in exynos_drm. Anyway it can affect other drivers as well. At least grep shows few possible candidates: What on earth is that driver doing registering additional drivers in the probe hook?!? That's madness, and should be treated as a bug. g. $ git grep -p platform_driver_register | grep -A1 -P '_probe\(struct platform_device' drivers/gpu/drm/exynos/exynos_drm_drv.c=static int exynos_drm_platform_probe(struct platform_device *pdev) drivers/gpu/drm/exynos/exynos_drm_drv.c: ret = platform_driver_register(fimd_driver); -- drivers/gpu/drm/sti/sti_drm_drv.c=static int sti_drm_platform_probe(struct platform_device *pdev) drivers/gpu/drm/sti/sti_drm_drv.c: platform_driver_register(sti_drm_master_driver); -- drivers/mtd/nand/atmel_nand.c=static int atmel_nand_probe(struct platform_device *pdev) drivers/mtd/nand/atmel_nand.c:res = platform_driver_register(atmel_nand_nfc_driver); [1]: http://permalink.gmane.org/gmane.comp.video.dri.devel/117727 Regards Andrzej Signed-off-by: Grant Likely grant.lik...@linaro.org Cc: Rob Herring robh...@kernel.org Cc: Arnd Bergmann a...@arndb.de Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: Benjamin Herrenschmidt b...@kernel.crashing.org --- drivers/of/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 3b64d0bf5bba..7c6771986c06 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -138,7 +138,7 @@ struct platform_device *of_device_alloc(struct device_node *np, } dev-dev.of_node = of_node_get(np); - dev-dev.parent = parent; + dev-dev.parent = parent ? : platform_bus; if (bus_id) dev_set_name(dev-dev, %s, bus_id); @@ -291,7 +291,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node, /* setup generic device info */ dev-dev.of_node = of_node_get(node); - dev-dev.parent = parent; + dev-dev.parent = parent ? : platform_bus; dev-dev.platform_data = platform_data; if (bus_id) dev_set_name(dev-dev, %s, bus_id); ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3] of/base: Fix PowerPC address parsing hack
On Fri, 14 Nov 2014 17:55:03 +1100
, Benjamin Herrenschmidt b...@kernel.crashing.org
wrote:
We have a historical hack that treats missing ranges properties as the
equivalent of an empty one. This is needed for ancient PowerMac bad
device-trees, and shouldn't be enabled for any other PowerPC platform,
otherwise we get some nasty layout of devices in sysfs or even
duplication when a set of otherwise identically named devices is
created multiple times under a different parent node with no ranges
property.
This fix is needed for the PowerNV i2c busses to be exposed properly
and will fix a number of other embedded cases.
Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
CC: sta...@vger.kernel.org
Acked-by: Grant Likely grant.lik...@linaro.org
Rob will pick up this patch and send it to Linus in his fixups tree for
v3.18
g.
---
V2: Make it less horrendously ugly
V3: use IS_ENABLED()
drivers/of/address.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index e371825..42e416a 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -403,6 +403,21 @@ static struct of_bus *of_match_bus(struct device_node
*np)
return NULL;
}
+static int of_empty_ranges_quirk(void)
+{
+ if (IS_ENABLED(CONFIG_PPC)) {
+ /* To save cycles, we cache the result */
+ static int quirk_state = -1;
+
+ if (quirk_state 0)
+ quirk_state =
+ of_machine_is_compatible(Power Macintosh) ||
+ of_machine_is_compatible(MacRISC);
+ return quirk_state;
+ }
+ return false;
+}
+
static int of_translate_one(struct device_node *parent, struct of_bus *bus,
struct of_bus *pbus, __be32 *addr,
int na, int ns, int pna, const char *rprop)
@@ -428,12 +443,10 @@ static int of_translate_one(struct device_node *parent,
struct of_bus *bus,
* This code is only enabled on powerpc. --gcl
*/
ranges = of_get_property(parent, rprop, rlen);
-#if !defined(CONFIG_PPC)
- if (ranges == NULL) {
+ if (ranges == NULL !of_empty_ranges_quirk()) {
pr_err(OF: no ranges; cannot translate\n);
return 1;
}
-#endif /* !defined(CONFIG_PPC) */
if (ranges == NULL || rlen == 0) {
offset = of_read_number(addr, na);
memset(addr, 0, pna * 4);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of/base: Fix PowerPC address parsing hack
On Thu, Nov 13, 2014 at 12:45 AM, Benjamin Herrenschmidt
b...@kernel.crashing.org wrote:
What about this one instead ? I want to cache it because that function
can be called quite a while and doing two additional property lookup
and string compares every time might hurt some platforms.
We have a historical hack that treats missing ranges properties as the
equivalent of an empty one. This is needed for ancient PowerMac bad
device-trees, and shouldn't be enabled for any other PowerPC platform,
otherwise we get some nasty layout of devices in sysfs or even
duplication when a set of otherwise identically named devices is
created multiple times under a different parent node with no ranges
property.
This fix is needed for the PowerNV i2c busses to be exposed properly
and will fix a number of other embedded cases.
Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
CC: sta...@vger.kernel.org
How far back does this need to go? I assume I need to get this in for v3.18.
diff --git a/drivers/of/address.c b/drivers/of/address.c
index e371825..5eae0cd 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -403,6 +403,17 @@ static struct of_bus *of_match_bus(struct device_node
*np)
return NULL;
}
+static int of_empty_ranges_quirk(void)
+{
+ /* To save cycles, we cache the result */
+ static int quirk_state = -1;
+
if (IS_ENABLED(CONFIG_POWERPC)) {
+ if (quirk_state 0)
+ quirk_state = of_machine_is_compatible(Power Macintosh) ||
+ of_machine_is_compatible(MacRISC);
+ return quirk_state;
}
return 0;
So it gets compiled out for non powerpc.
+}
+
static int of_translate_one(struct device_node *parent, struct of_bus *bus,
struct of_bus *pbus, __be32 *addr,
int na, int ns, int pna, const char *rprop)
@@ -428,12 +439,10 @@ static int of_translate_one(struct device_node *parent,
struct of_bus *bus,
* This code is only enabled on powerpc. --gcl
*/
ranges = of_get_property(parent, rprop, rlen);
-#if !defined(CONFIG_PPC)
- if (ranges == NULL) {
+ if (ranges == NULL !of_empty_ranges_quirk()) {
pr_err(OF: no ranges; cannot translate\n);
return 1;
}
-#endif /* !defined(CONFIG_PPC) */
if (ranges == NULL || rlen == 0) {
offset = of_read_number(addr, na);
memset(addr, 0, pna * 4);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] of/platform: Move platform devices under /sys/devices/platform
Currently the devices created by drivers/of/platform.c get created at the root of /sys/devices. This goes against the typical pattern for sysfs where the top level /sys/devices structure contains categories of devices, and the structure of devices is placed below that. To fix this, make the code in drivers/of/platform.c follow the drivers/base/platform.c behaviour, and use platform_bus as the default parent for all new platform_devices and amba_devices. This change has been discussed for a long time, but nobody has actually acted on it. Userspace code that expects to find devices under a fixed /sys/devices/... path will be affected. It isn't /supposed/ to do that, but if anyone complains then I'll add a default-off workaround option to put them back into the root. Signed-off-by: Grant Likely grant.lik...@linaro.org Cc: Rob Herring robh...@kernel.org Cc: Arnd Bergmann a...@arndb.de Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: Benjamin Herrenschmidt b...@kernel.crashing.org --- drivers/of/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 3b64d0bf5bba..7c6771986c06 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -138,7 +138,7 @@ struct platform_device *of_device_alloc(struct device_node *np, } dev-dev.of_node = of_node_get(np); - dev-dev.parent = parent; + dev-dev.parent = parent ? : platform_bus; if (bus_id) dev_set_name(dev-dev, %s, bus_id); @@ -291,7 +291,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node, /* setup generic device info */ dev-dev.of_node = of_node_get(node); - dev-dev.parent = parent; + dev-dev.parent = parent ? : platform_bus; dev-dev.platform_data = platform_data; if (bus_id) dev_set_name(dev-dev, %s, bus_id); -- 1.9.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH / RFC] PowerPC: boot: Parse chosen/cmdline-timeout parameter
On Tue, Sep 9, 2014 at 3:01 PM, Simon Kågström
simon.kagst...@netinsight.net wrote:
A 5 second timeout during boot might be too long, so make it
configurable.
The property is added to the chosen node, e.g.,
chosen {
bootargs = console=ttyUL0 root=/dev/ram0;
linux,stdout-path = /plb@0/serial@4600;
cmdline-timeout = 100;
} ;
Signed-off-by: Simon Kagstrom simon.kagst...@netinsight.net
---
We build a simpleImage for a Virtex 4 PPC405 target, and the
delay-for-command-line-edits is a significant part of the total boot
time.
Questions (apart from the patch in general):
- Should the property be in the chosen node?
yes
- Naming of the property?
Use a 'linux,' prefix. linux,cmdline-timeout
-static void serial_edit_cmdline(char *buf, int len)
+static void serial_edit_cmdline(char *buf, int len, unsigned int timeout)
{
int timer = 0, count;
char ch, *cp;
@@ -44,7 +44,7 @@ static void serial_edit_cmdline(char *buf, int len)
cp = buf[count];
count++;
- while (timer++ 5*1000) {
+ while (timer++ timeout) {
Perhaps allow the loop to go through at least once so that the editor
can be broken into by holding down a key.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of: make sure of_alias is initialized before accessing it
On Wed, 27 Aug 2014 17:09:39 +0300, Laurentiu Tudor b10...@freescale.com
wrote:
Simply swap of_alias and of_chosen initialization so
that of_alias ends up read first. This must be done
because it is accessed couple of lines below when
trying to initialize the of_stdout using the alias
based legacy method.
[Fixes a752ee5 - tty: Update hypervisor tty drivers to
use core stdout parsing code]
Signed-off-by: Laurentiu Tudor laurentiu.tu...@freescale.com
Cc: Grant Likely grant.lik...@linaro.org
---
drivers/of/base.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d8574ad..52f8506 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1847,6 +1847,10 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
{
struct property *pp;
+ of_aliases = of_find_node_by_path(/aliases);
+ if (!of_aliases)
+ return;
+
of_chosen = of_find_node_by_path(/chosen);
if (of_chosen == NULL)
of_chosen = of_find_node_by_path(/chosen@0);
@@ -1862,10 +1866,6 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
of_stdout = of_find_node_by_path(name);
}
- of_aliases = of_find_node_by_path(/aliases);
- if (!of_aliases)
- return;
-
Close, but not quite. The 'if (!of_aliases)' test should not be moved.
Only the search for of_find_node_by_path(). I've fixed it up and
applied.
g.
for_each_property_of_node(of_aliases, pp) {
const char *start = pp-name;
const char *end = start + strlen(start);
--
1.8.3.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Check flat device tree version at boot
On 29 Aug 2014 02:56, Michael Ellerman m...@ellerman.id.au wrote:
On Thu, 2014-08-28 at 09:27 -0500, Rob Herring wrote:
On Thu, Aug 28, 2014 at 3:40 AM, Michael Ellerman m...@ellerman.id.au
wrote:
In commit e6a6928c3ea1 of/fdt: Convert FDT functions to use libfdt,
the kernel stopped supporting old flat device tree formats. The minimum
supported version is now 0x10.
Ugg. Is that something which needs to be supported? Supporting it at
this point would mean adding support to libfdt.
Well it would have been nice to keep supporting v2, dropping it broke
kexec-tools for example. But it's done now, so we'll just deal with the
fallout.
Umm, so we broke userspace? That's not okay. At the very least we
should investigate how much work is needed to bring v2 support into
libfdt, or up-rev the flat tree at runtime before we parse it.
We should be able to update it in-line. IIRC, the main difference
between v2 and v0x10 is that only the leaf node name is encoded into
the node instead of the full name. We can loop over the tree and
truncate all the names while filling the unused bytes with FDT NOP
tags. Should be simple. Something like the following:
fixup_old_device_tree(blob)
{
for (offset = fdt_next_node(blob, -1, depth);
offset = 0 depth = 0 !rc;
offset = fdt_next_node(blob, offset, depth)) {
char *pathp = fdt_get_name(blob, offset, NULL);
if (*pathp == '/') {
char *leaf = kbasename(pathp);
int len = strlen(pathp);
int newlen = strlen(leaf);
strcpy(pathp, leaf); /* copying in place, need
to check make sure this code is safe */
node-size = newlen /* fixme - this is just
pseudocode */
newlen = FDT_TAGALIGN(newlen);
while (newlen len) {
*(pathp + newlen) = FDT_NOP;
newlen = FDT_TAGALIGN(newlen+1);
}
}
}
}
There's probable some more elegance that can be put into the above
block, but you get the idea. That could be run in-place without
copying the tree to another location, and it could possibly be done in
the boot wrapper. Then we'd also be able to get rid of the v2
compatibility code elsewhere in drivers/of/fdt.c because it would
already be taken care of.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: OF_DYNAMIC node lifecycle
On Wed, Jul 16, 2014 at 12:30 PM, Tyrel Datwyler turtle.in.the.ker...@gmail.com wrote: On 07/15/2014 10:33 PM, Grant Likely wrote: I've got another question about powerpc reconfiguration. I was looking at the dlpar_configure_connector() function in dlpar.c. I see that the function has the ability to process multiple nodes with additional sibling and child nodes. It appears to link them into a detached tree structure, and the function returns a pointer to the first node. All of the callers of that function then call dlpar_attach_node(), which calls of_attach_node(). However, of_attach_node() only handles a single node. It doesn't handle siblings or children. Is this a bug? Does the configure connector ever actually receive more than one node at once? Yes, it is sometimes the case we will get a tree structure back of more than one node. Under the proc interface implementation this just worked. With the move to sysfs it appears we have a regression here. What makes more sense here, for us to walk the tree calling of_attach_node, or to move such tree walking logic into of_attach_node? From what I can tell we are the only consumers of of_attach_node. That is very shortly going to change. The overlay code also uses of_attach_node(). I can make of_attach_node() recurse over descendants, but I'm also considering moving the powerpc code over to the of_changeset series that Panto and I are working on. Either way, the handling of multiple nodes should be common code. I think the easiest is to put the recursion into of_attach_node(), at least for fixing the bug. It can be reworked later. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: OF_DYNAMIC node lifecycle
On Wed, Jul 16, 2014 at 2:57 PM, Grant Likely grant.lik...@linaro.org wrote: On Wed, Jul 16, 2014 at 12:30 PM, Tyrel Datwyler turtle.in.the.ker...@gmail.com wrote: On 07/15/2014 10:33 PM, Grant Likely wrote: I've got another question about powerpc reconfiguration. I was looking at the dlpar_configure_connector() function in dlpar.c. I see that the function has the ability to process multiple nodes with additional sibling and child nodes. It appears to link them into a detached tree structure, and the function returns a pointer to the first node. All of the callers of that function then call dlpar_attach_node(), which calls of_attach_node(). However, of_attach_node() only handles a single node. It doesn't handle siblings or children. Is this a bug? Does the configure connector ever actually receive more than one node at once? Yes, it is sometimes the case we will get a tree structure back of more than one node. Under the proc interface implementation this just worked. With the move to sysfs it appears we have a regression here. What makes more sense here, for us to walk the tree calling of_attach_node, or to move such tree walking logic into of_attach_node? From what I can tell we are the only consumers of of_attach_node. That is very shortly going to change. The overlay code also uses of_attach_node(). I can make of_attach_node() recurse over descendants, but I'm also considering moving the powerpc code over to the of_changeset series that Panto and I are working on. Either way, the handling of multiple nodes should be common code. I think the easiest is to put the recursion into of_attach_node(), at least for fixing the bug. It can be reworked later. On pseries, do notifiers ever fail? ie. Does the reconfig code ever object to a DT change and prevent it from being applied? g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: OF_DYNAMIC node lifecycle
On Wed, Jul 16, 2014 at 5:12 PM, Nathan Fontenot nf...@austin.ibm.com wrote: On 07/16/2014 05:26 PM, Grant Likely wrote: On Wed, Jul 16, 2014 at 2:57 PM, Grant Likely grant.lik...@linaro.org wrote: On Wed, Jul 16, 2014 at 12:30 PM, Tyrel Datwyler turtle.in.the.ker...@gmail.com wrote: On 07/15/2014 10:33 PM, Grant Likely wrote: I've got another question about powerpc reconfiguration. I was looking at the dlpar_configure_connector() function in dlpar.c. I see that the function has the ability to process multiple nodes with additional sibling and child nodes. It appears to link them into a detached tree structure, and the function returns a pointer to the first node. All of the callers of that function then call dlpar_attach_node(), which calls of_attach_node(). However, of_attach_node() only handles a single node. It doesn't handle siblings or children. Is this a bug? Does the configure connector ever actually receive more than one node at once? Yes, it is sometimes the case we will get a tree structure back of more than one node. Under the proc interface implementation this just worked. With the move to sysfs it appears we have a regression here. What makes more sense here, for us to walk the tree calling of_attach_node, or to move such tree walking logic into of_attach_node? From what I can tell we are the only consumers of of_attach_node. That is very shortly going to change. The overlay code also uses of_attach_node(). I can make of_attach_node() recurse over descendants, but I'm also considering moving the powerpc code over to the of_changeset series that Panto and I are working on. Either way, the handling of multiple nodes should be common code. I think the easiest is to put the recursion into of_attach_node(), at least for fixing the bug. It can be reworked later. On pseries, do notifiers ever fail? ie. Does the reconfig code ever object to a DT change and prevent it from being applied? I cannot think of a time that I ever saw a notifier fail. Good to know. I was hoping it wasn't part of the design. I can't think of any situation where the kernel would want to inhibit a change to the device tree. Can you take a look at the following tree and give it a spin on PowerPC. I've reordered the notifiers and hopefully got the powerpc fixups right, but I don't have a way to test it... The following changes since commit cd3de83f147601356395b57a8673e9c5ff1e59d1: Linux 3.16-rc4 (2014-07-06 12:37:51 -0700) are available in the git repository at: git://git.secretlab.ca/git/linux devicetree/next-overlay for you to fetch changes up to 44ac93bb5583c5f1f85912309fe04045b61a6dd0: of: Transactional DT support. (2014-07-16 16:44:07 -0600) Grant Likely (6): of/platform: Fix of_platform_device_destroy iteration of devices of: Move CONFIG_OF_DYNAMIC code into a separate file of: Make sure attached nodes don't carry along extra children of: Move dynamic node fixups out of powerpc and into common code of: Make OF_DYNAMIC user selectable of: Reorder device tree changes and notifiers Pantelis Antoniou (5): of: rename of_aliases_mutex to just of_mutex OF: Utility helper functions for dynamic nodes of: Create unlocked versions of node and property add/remove functions of: Make devicetree sysfs update functions consistent. of: Transactional DT support. Documentation/devicetree/changesets.txt | 41 ++ arch/powerpc/kernel/prom.c | 70 --- arch/powerpc/platforms/pseries/hotplug-memory.c | 2 +- drivers/crypto/nx/nx-842.c | 30 +- drivers/of/Kconfig | 2 +- drivers/of/Makefile | 1 + drivers/of/base.c | 423 --- drivers/of/device.c | 4 +- drivers/of/dynamic.c| 671 drivers/of/of_private.h | 59 ++- drivers/of/platform.c | 32 +- drivers/of/selftest.c | 79 +++ drivers/of/testcase-data/testcases.dtsi | 10 + include/linux/of.h | 80 ++- include/linux/of_platform.h | 7 +- 15 files changed, 1067 insertions(+), 444 deletions(-) create mode 100644 Documentation/devicetree/changesets.txt create mode 100644 drivers/of/dynamic.c ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc/pseries: dynamically added OF nodes need to call of_node_init
On Thu, Jul 10, 2014 at 1:59 PM, Nathan Fontenot
nf...@linux.vnet.ibm.com wrote:
On 07/10/2014 01:50 PM, Tyrel Datwyler wrote:
Commit 75b57ecf9 refactored device tree nodes to use kobjects such that they
can be exposed via /sysfs. A secondary commit 0829f6d1f furthered this rework
by moving the kobect initialization logic out of of_node_add into its own
of_node_init function. The inital commit removed the existing kref_init calls
in the pseries dlpar code with the assumption kobject initialization would
occur in of_node_add. The second commit had the side effect of triggering a
BUG_ON during DLPAR, migration and suspend/resume operations as a result of
dynamically added nodes being uninitialized.
This patch fixes this by adding of_node_init calls in place of the previously
removed kref_init calls.
Fixes: 0829f6d1f69e (of: device_node kobject lifecycle fixes)
Cc: sta...@vger.kernel.org
Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com
Acked-by: Nathan Fontenot nf...@linux.vnet.ibm.com
Acked-by: Grant Likely grant.lik...@linaro.org
Ben, are you going to take this or should I take it via my tree?
g.
---
V2:
- included stable kernel list on Cc per comment by mpe
arch/powerpc/platforms/pseries/dlpar.c| 1 +
arch/powerpc/platforms/pseries/reconfig.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c
b/arch/powerpc/platforms/pseries/dlpar.c
index 022b38e..2d0b4d6 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -86,6 +86,7 @@ static struct device_node *dlpar_parse_cc_node(struct
cc_workarea *ccwa,
}
of_node_set_flag(dn, OF_DYNAMIC);
+ of_node_init(dn);
return dn;
}
diff --git a/arch/powerpc/platforms/pseries/reconfig.c
b/arch/powerpc/platforms/pseries/reconfig.c
index 0435bb6..1c0a60d 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -69,6 +69,7 @@ static int pSeries_reconfig_add_node(const char *path,
struct property *proplist
np-properties = proplist;
of_node_set_flag(np, OF_DYNAMIC);
+ of_node_init(np);
np-parent = derive_parent(path);
if (IS_ERR(np-parent)) {
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: OF_DYNAMIC node lifecycle
I've got another question about powerpc reconfiguration. I was looking at the dlpar_configure_connector() function in dlpar.c. I see that the function has the ability to process multiple nodes with additional sibling and child nodes. It appears to link them into a detached tree structure, and the function returns a pointer to the first node. All of the callers of that function then call dlpar_attach_node(), which calls of_attach_node(). However, of_attach_node() only handles a single node. It doesn't handle siblings or children. Is this a bug? Does the configure connector ever actually receive more than one node at once? g. On Fri, Jun 27, 2014 at 8:41 AM, Nathan Fontenot nf...@austin.ibm.com wrote: On 06/27/2014 07:41 AM, Grant Likely wrote: On Thu, 26 Jun 2014 15:01:49 -0500, Nathan Fontenot nf...@austin.ibm.com wrote: On 06/25/2014 03:24 PM, Grant Likely wrote: On Tue, 24 Jun 2014 15:10:55 -0500, Nathan Fontenot nf...@austin.ibm.com wrote: heh! I have often thought about adding reference counting to device tree properties. You horrible, horrible man. Yes. I are evil :) After looking again the work needed to add reference counts to properties would be huge. The few properties I am concerned with are specific to powerpc so perhaps just adding an arch specific lock around updating those properties would work. Which code/properties? I'd like to have a look myself. /ibm,dynamic-reconfiguration-memory/ibm,dynamic-memory The property is updated in arch/powerpc/platforms/pseries/hotplug-memory.c:pseries_update_drconf_memory() Specifically, what do you need for the locking? Are you wanting to hold off additional changes while that function is executing? Pantelis is adding a mutex for device tree writers. Holding that mutex would prevent any changes from happening in the tree without affecting readers. Would that be sufficient? That would work. -Nathan ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: OF_DYNAMIC node lifecycle
On Thu, 26 Jun 2014 14:59:31 -0500, Nathan Fontenot nf...@austin.ibm.com wrote: On 06/25/2014 03:22 PM, Grant Likely wrote: On Tue, 24 Jun 2014 15:07:05 -0500, Nathan Fontenot nf...@austin.ibm.com wrote: On 06/23/2014 09:58 AM, Grant Likely wrote: On Thu, 19 Jun 2014 11:33:20 +0300, Pantelis Antoniou pantelis.anton...@konsulko.com wrote: Hi Grant, CCing Thomas Gleixner Steven Rostedt, since they might have a few ideas... On Jun 18, 2014, at 11:07 PM, Grant Likely wrote: Hi Nathan and Tyrel, I'm looking into lifecycle issues on nodes modified by OF_DYNAMIC, and I'm hoping you can help me. Right now, pseries seems to be the only user of OF_DYNAMIC, but making OF_DYNAMIC work has a huge impact on the entire kernel because it requires all DT code to manage reference counting with iterating over nodes. Most users simply get it wrong. Pantelis did some investigation and found that the reference counts on a running kernel are all over the place. I have my doubts that any code really gets it right. The problem is that users need to know when it is appropriate to call of_node_get()/of_node_put(). All list traversals that exit early need an extra call to of_node_put(), and code that is searching for a node in the tree and holding a reference to it needs to call of_node_get(). In hindsight it appears that drivers just can't get the lifecycle right. So we need to simplify things. I've got a few pseries questions: - What are the changes being requested by pseries firmware? Is it only CPUs and memory nodes, or does it manipulate things all over the tree? - How frequent are the changes? How many changes would be likely over the runtime of the system? - Are you able to verify that removed nodes are actually able to be freed correctly? Do you have any testcases for node removal? I'm thinking very seriously about changing the locking semantics of DT code entirely so that most users never have to worry about of_node_get/put at all. If the DT code is switched to use rcu primitives for tree iteration (which also means making DT code use list_head, something I'm already investigating), then instead of trying to figure out of_node_get/put rules, callers could use rcu_read_lock()/rcu_read_unlock() to protect the region that is searching over nodes, and only call of_node_get() if the node pointer is needed outside the rcu read-side lock. I'd really like to be rid of the node reference counting entirely, but I can't figure out a way of doing that safely, so I'd settle for making it a lot easier to get correct. Since we're going about changing things, how about that devtree_lock? I believe rcu would pretty much eliminate the devtree_lock entirely. All modifiers would need to grab a mutex to ensure there is only one writer at any given time, but readers would have free reign to parse the tree however they like. DT writers would have to follow some strict rules about how to handle nodes that are removed (ie. don't modify or of_node_put() them until after rcu is syncronized), but the number of writers is very small and we have control of all of them. We're using a raw_spinlock and we're always taking the lock with interrupts disabled. If we're going to make DT changes frequently during normal runtime and not only during boot time, those are bad for any kind of real-time performance. So the question is, do we really have code that access the live tree during atomic sections? Is that something we want? Enforcing this will make our lives easier, and we'll get the change to replace that spinlock with a mutex. Yes, I believe the powerpc CPU hotplug code accesses the DT in atomic sections. I cannot put my finger on the exact code however. Nathan might know better. But, if I'm right, the whole problem goes away with RCU. I went back through the cpu hotplug code. we do update the DT during cpu hotplug but I don't see it happening during atomic sections. The code is in arch/powerpc/platforms/pseries/dlpar.c Great, thanks, By the way, notifiers currently get sent before any updates are applied to the tree. I want to change it so that the notifier gets sent afterwards. Does that work for you? I've looked through all the users and aside from a stupid block of code in arch/powerpc/kernel/prom.c which does things that should be done by of_attach_node(), it looks like everything should be fine. This would affect property updates. When doing a property update the notifier passes a pointer to a struct containing a device node pointer and a pointer to the new device node property. I know specifically in memory property updates we grab the current version of the device tree property and compare it to the 'new' version that was passed to us. If you want to do the DT update before calling the notifier that should be fine for the memory update
Re: OF_DYNAMIC node lifecycle
On Thu, 26 Jun 2014 15:01:49 -0500, Nathan Fontenot nf...@austin.ibm.com wrote: On 06/25/2014 03:24 PM, Grant Likely wrote: On Tue, 24 Jun 2014 15:10:55 -0500, Nathan Fontenot nf...@austin.ibm.com wrote: heh! I have often thought about adding reference counting to device tree properties. You horrible, horrible man. Yes. I are evil :) After looking again the work needed to add reference counts to properties would be huge. The few properties I am concerned with are specific to powerpc so perhaps just adding an arch specific lock around updating those properties would work. Which code/properties? I'd like to have a look myself. /ibm,dynamic-reconfiguration-memory/ibm,dynamic-memory The property is updated in arch/powerpc/platforms/pseries/hotplug-memory.c:pseries_update_drconf_memory() Specifically, what do you need for the locking? Are you wanting to hold off additional changes while that function is executing? Pantelis is adding a mutex for device tree writers. Holding that mutex would prevent any changes from happening in the tree without affecting readers. Would that be sufficient? g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: OF_DYNAMIC node lifecycle
On Tue, 24 Jun 2014 15:07:05 -0500, Nathan Fontenot nf...@austin.ibm.com wrote: On 06/23/2014 09:58 AM, Grant Likely wrote: On Thu, 19 Jun 2014 11:33:20 +0300, Pantelis Antoniou pantelis.anton...@konsulko.com wrote: Hi Grant, CCing Thomas Gleixner Steven Rostedt, since they might have a few ideas... On Jun 18, 2014, at 11:07 PM, Grant Likely wrote: Hi Nathan and Tyrel, I'm looking into lifecycle issues on nodes modified by OF_DYNAMIC, and I'm hoping you can help me. Right now, pseries seems to be the only user of OF_DYNAMIC, but making OF_DYNAMIC work has a huge impact on the entire kernel because it requires all DT code to manage reference counting with iterating over nodes. Most users simply get it wrong. Pantelis did some investigation and found that the reference counts on a running kernel are all over the place. I have my doubts that any code really gets it right. The problem is that users need to know when it is appropriate to call of_node_get()/of_node_put(). All list traversals that exit early need an extra call to of_node_put(), and code that is searching for a node in the tree and holding a reference to it needs to call of_node_get(). In hindsight it appears that drivers just can't get the lifecycle right. So we need to simplify things. I've got a few pseries questions: - What are the changes being requested by pseries firmware? Is it only CPUs and memory nodes, or does it manipulate things all over the tree? - How frequent are the changes? How many changes would be likely over the runtime of the system? - Are you able to verify that removed nodes are actually able to be freed correctly? Do you have any testcases for node removal? I'm thinking very seriously about changing the locking semantics of DT code entirely so that most users never have to worry about of_node_get/put at all. If the DT code is switched to use rcu primitives for tree iteration (which also means making DT code use list_head, something I'm already investigating), then instead of trying to figure out of_node_get/put rules, callers could use rcu_read_lock()/rcu_read_unlock() to protect the region that is searching over nodes, and only call of_node_get() if the node pointer is needed outside the rcu read-side lock. I'd really like to be rid of the node reference counting entirely, but I can't figure out a way of doing that safely, so I'd settle for making it a lot easier to get correct. Since we're going about changing things, how about that devtree_lock? I believe rcu would pretty much eliminate the devtree_lock entirely. All modifiers would need to grab a mutex to ensure there is only one writer at any given time, but readers would have free reign to parse the tree however they like. DT writers would have to follow some strict rules about how to handle nodes that are removed (ie. don't modify or of_node_put() them until after rcu is syncronized), but the number of writers is very small and we have control of all of them. We're using a raw_spinlock and we're always taking the lock with interrupts disabled. If we're going to make DT changes frequently during normal runtime and not only during boot time, those are bad for any kind of real-time performance. So the question is, do we really have code that access the live tree during atomic sections? Is that something we want? Enforcing this will make our lives easier, and we'll get the change to replace that spinlock with a mutex. Yes, I believe the powerpc CPU hotplug code accesses the DT in atomic sections. I cannot put my finger on the exact code however. Nathan might know better. But, if I'm right, the whole problem goes away with RCU. I went back through the cpu hotplug code. we do update the DT during cpu hotplug but I don't see it happening during atomic sections. The code is in arch/powerpc/platforms/pseries/dlpar.c Great, thanks, By the way, notifiers currently get sent before any updates are applied to the tree. I want to change it so that the notifier gets sent afterwards. Does that work for you? I've looked through all the users and aside from a stupid block of code in arch/powerpc/kernel/prom.c which does things that should be done by of_attach_node(), it looks like everything should be fine. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: OF_DYNAMIC node lifecycle
On Tue, 24 Jun 2014 15:10:55 -0500, Nathan Fontenot nf...@austin.ibm.com wrote: On 06/23/2014 09:48 AM, Grant Likely wrote: On Thu, 19 Jun 2014 10:26:15 -0500, Nathan Fontenot nf...@austin.ibm.com wrote: On 06/18/2014 03:07 PM, Grant Likely wrote: Hi Nathan and Tyrel, I'm looking into lifecycle issues on nodes modified by OF_DYNAMIC, and I'm hoping you can help me. Right now, pseries seems to be the only user of OF_DYNAMIC, but making OF_DYNAMIC work has a huge impact on the entire kernel because it requires all DT code to manage reference counting with iterating over nodes. Most users simply get it wrong. Pantelis did some investigation and found that the reference counts on a running kernel are all over the place. I have my doubts that any code really gets it right. The problem is that users need to know when it is appropriate to call of_node_get()/of_node_put(). All list traversals that exit early need an extra call to of_node_put(), and code that is searching for a node in the tree and holding a reference to it needs to call of_node_get(). I've got a few pseries questions: - What are the changes being requested by pseries firmware? Is it only CPUs and memory nodes, or does it manipulate things all over the tree? The short answer, everything. :-) For pseries the two big actions that can change the device tree are adding/removing resources and partition migration. The most frequent updates to the device tree happen during resource (cpu, memory, and pci/phb) add and remove. During this process we add and remove the node and its properties from the device tree. - For memory on newer systems this just involves updating the ibm,dynamic-reconfiguration-memory/ibm,dynamic-memory property. Older firmware levels add and remove the memroy@XXX nodes and their properties. - For cpus the cpus/PowerPC,POWER nodes and its properties are added or removed - For pci/phb the pci@X nodes and properties are added/removed. The less frequent operation of live partition migration (and suspend/resume) can update just about anything in the device tree. When this occurs and the systems starts after being migrated (or waking up after a suspend) we make a call to firmware to get updates to the device tree for the new hardware we are running on. - How frequent are the changes? How many changes would be likely over the runtime of the system? This can happen frequently. Thanks, that is exactly the information that I want. I'm not so much concerned with the addition or removal of nodes/properties, which is actually pretty easy to handle. It is the lifecycle of allocations on dynamic nodes that causes heartburn. - Are you able to verify that removed nodes are actually able to be freed correctly? Do you have any testcases for node removal? I have always tested this by doing resource add/remove, usually cpu and memory since it is the easiest. Is that just testing the functionality, or do you have tests that check if the memory gets freed? In general it's just functionality testing. I'm thinking very seriously about changing the locking semantics of DT code entirely so that most users never have to worry about of_node_get/put at all. If the DT code is switched to use rcu primitives for tree iteration (which also means making DT code use list_head, something I'm already investigating), then instead of trying to figure out of_node_get/put rules, callers could use rcu_read_lock()/rcu_read_unlock() to protect the region that is searching over nodes, and only call of_node_get() if the node pointer is needed outside the rcu read-side lock. This sounds good. I like just taking the rcu lock around accessing the DT. Do we have many places where DT node pointers are held that require keeping the of_node_get/put calls? If this did exist perhaps we could update those places to look up the DT node every time instead of holding on to the pointer. We could just get rid of the reference counting altogether then. There are a few, but I would be happy to restrict reference counting to only those locations. Most places will decode the DT data, and then throw away the reference. We /might/ even be able to do rcu_lock/unlock around the entire probe path which would make it transparent to all device drivers. I'd really like to be rid of the node reference counting entirely, but I can't figure out a way of doing that safely, so I'd settle for making it a lot easier to get correct. heh! I have often thought about adding reference counting to device tree properties. You horrible, horrible man. Yes. I are evil :) After looking again the work needed to add reference counts to properties would be huge. The few properties I am concerned with are specific to powerpc so perhaps just adding an arch specific lock around updating
Re: OF_DYNAMIC node lifecycle
On Thu, 19 Jun 2014 10:26:15 -0500, Nathan Fontenot nf...@austin.ibm.com wrote: On 06/18/2014 03:07 PM, Grant Likely wrote: Hi Nathan and Tyrel, I'm looking into lifecycle issues on nodes modified by OF_DYNAMIC, and I'm hoping you can help me. Right now, pseries seems to be the only user of OF_DYNAMIC, but making OF_DYNAMIC work has a huge impact on the entire kernel because it requires all DT code to manage reference counting with iterating over nodes. Most users simply get it wrong. Pantelis did some investigation and found that the reference counts on a running kernel are all over the place. I have my doubts that any code really gets it right. The problem is that users need to know when it is appropriate to call of_node_get()/of_node_put(). All list traversals that exit early need an extra call to of_node_put(), and code that is searching for a node in the tree and holding a reference to it needs to call of_node_get(). I've got a few pseries questions: - What are the changes being requested by pseries firmware? Is it only CPUs and memory nodes, or does it manipulate things all over the tree? The short answer, everything. :-) For pseries the two big actions that can change the device tree are adding/removing resources and partition migration. The most frequent updates to the device tree happen during resource (cpu, memory, and pci/phb) add and remove. During this process we add and remove the node and its properties from the device tree. - For memory on newer systems this just involves updating the ibm,dynamic-reconfiguration-memory/ibm,dynamic-memory property. Older firmware levels add and remove the memroy@XXX nodes and their properties. - For cpus the cpus/PowerPC,POWER nodes and its properties are added or removed - For pci/phb the pci@X nodes and properties are added/removed. The less frequent operation of live partition migration (and suspend/resume) can update just about anything in the device tree. When this occurs and the systems starts after being migrated (or waking up after a suspend) we make a call to firmware to get updates to the device tree for the new hardware we are running on. - How frequent are the changes? How many changes would be likely over the runtime of the system? This can happen frequently. Thanks, that is exactly the information that I want. I'm not so much concerned with the addition or removal of nodes/properties, which is actually pretty easy to handle. It is the lifecycle of allocations on dynamic nodes that causes heartburn. - Are you able to verify that removed nodes are actually able to be freed correctly? Do you have any testcases for node removal? I have always tested this by doing resource add/remove, usually cpu and memory since it is the easiest. Is that just testing the functionality, or do you have tests that check if the memory gets freed? I'm thinking very seriously about changing the locking semantics of DT code entirely so that most users never have to worry about of_node_get/put at all. If the DT code is switched to use rcu primitives for tree iteration (which also means making DT code use list_head, something I'm already investigating), then instead of trying to figure out of_node_get/put rules, callers could use rcu_read_lock()/rcu_read_unlock() to protect the region that is searching over nodes, and only call of_node_get() if the node pointer is needed outside the rcu read-side lock. This sounds good. I like just taking the rcu lock around accessing the DT. Do we have many places where DT node pointers are held that require keeping the of_node_get/put calls? If this did exist perhaps we could update those places to look up the DT node every time instead of holding on to the pointer. We could just get rid of the reference counting altogether then. There are a few, but I would be happy to restrict reference counting to only those locations. Most places will decode the DT data, and then throw away the reference. We /might/ even be able to do rcu_lock/unlock around the entire probe path which would make it transparent to all device drivers. I'd really like to be rid of the node reference counting entirely, but I can't figure out a way of doing that safely, so I'd settle for making it a lot easier to get correct. heh! I have often thought about adding reference counting to device tree properties. You horrible, horrible man. I don't really want to but there are some properties that can get updated frequently (namely the one mentioned above for memory) that can also get pretty big, especially on systems with a lot of memory. We never free the memory for old versions of a device tree property. This is a pretty minor issue though and probably best suited for a separate discussion after resolving this. We might be able to do some in-place modification of properties if the size of the property doesn't change
Re: OF_DYNAMIC node lifecycle
On Thu, 19 Jun 2014 11:33:20 +0300, Pantelis Antoniou pantelis.anton...@konsulko.com wrote: Hi Grant, CCing Thomas Gleixner Steven Rostedt, since they might have a few ideas... On Jun 18, 2014, at 11:07 PM, Grant Likely wrote: Hi Nathan and Tyrel, I'm looking into lifecycle issues on nodes modified by OF_DYNAMIC, and I'm hoping you can help me. Right now, pseries seems to be the only user of OF_DYNAMIC, but making OF_DYNAMIC work has a huge impact on the entire kernel because it requires all DT code to manage reference counting with iterating over nodes. Most users simply get it wrong. Pantelis did some investigation and found that the reference counts on a running kernel are all over the place. I have my doubts that any code really gets it right. The problem is that users need to know when it is appropriate to call of_node_get()/of_node_put(). All list traversals that exit early need an extra call to of_node_put(), and code that is searching for a node in the tree and holding a reference to it needs to call of_node_get(). In hindsight it appears that drivers just can't get the lifecycle right. So we need to simplify things. I've got a few pseries questions: - What are the changes being requested by pseries firmware? Is it only CPUs and memory nodes, or does it manipulate things all over the tree? - How frequent are the changes? How many changes would be likely over the runtime of the system? - Are you able to verify that removed nodes are actually able to be freed correctly? Do you have any testcases for node removal? I'm thinking very seriously about changing the locking semantics of DT code entirely so that most users never have to worry about of_node_get/put at all. If the DT code is switched to use rcu primitives for tree iteration (which also means making DT code use list_head, something I'm already investigating), then instead of trying to figure out of_node_get/put rules, callers could use rcu_read_lock()/rcu_read_unlock() to protect the region that is searching over nodes, and only call of_node_get() if the node pointer is needed outside the rcu read-side lock. I'd really like to be rid of the node reference counting entirely, but I can't figure out a way of doing that safely, so I'd settle for making it a lot easier to get correct. Since we're going about changing things, how about that devtree_lock? I believe rcu would pretty much eliminate the devtree_lock entirely. All modifiers would need to grab a mutex to ensure there is only one writer at any given time, but readers would have free reign to parse the tree however they like. DT writers would have to follow some strict rules about how to handle nodes that are removed (ie. don't modify or of_node_put() them until after rcu is syncronized), but the number of writers is very small and we have control of all of them. We're using a raw_spinlock and we're always taking the lock with interrupts disabled. If we're going to make DT changes frequently during normal runtime and not only during boot time, those are bad for any kind of real-time performance. So the question is, do we really have code that access the live tree during atomic sections? Is that something we want? Enforcing this will make our lives easier, and we'll get the change to replace that spinlock with a mutex. Yes, I believe the powerpc CPU hotplug code accesses the DT in atomic sections. I cannot put my finger on the exact code however. Nathan might know better. But, if I'm right, the whole problem goes away with RCU. The design with RCU is to switch struct device_node and struct property to use list_head and/or hlist_head with the _rcu accessors. They allow items to be removed from a list without syncronizing with readers. Right now we have two lists that need to be modified; the allnodes list and the sibling list. I *think* it will be fine for the two list removals to be non-atomic (there will be a brief period where the node can be found on one list, but not the other) because it is a transient state already accounted for in rcu read-side critical region. That said, I've also got a design to remove the allnodes list entirely and only work with the sibling list. I need to prototype this. We'll also need a transition plan to move to RCU. I think the existing iterators can be modified to do the rcu locking in-line, but still require the of_node_get/put stuff (basically, so existing code continue to works unchanged). Then we can add _rcu versions that drop the need for of_node_get/put(). When everything is converted, the old iterators can be dropped. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: OF_DYNAMIC node lifecycle
On Mon, 23 Jun 2014 18:26:04 +0300, Pantelis Antoniou pantelis.anton...@konsulko.com wrote: On Jun 23, 2014, at 5:58 PM, Grant Likely wrote: We'll also need a transition plan to move to RCU. I think the existing iterators can be modified to do the rcu locking in-line, but still require the of_node_get/put stuff (basically, so existing code continue to works unchanged). Then we can add _rcu versions that drop the need for of_node_get/put(). When everything is converted, the old iterators can be dropped. Eventually yes. We're not close to that yet. I'd be happy if we get the lifecycle issues fixed right now (with of_node_get/put) and plan ahead. I am sure we missed a few things, which we will come across. If we agree on the plan to keep of_node_get/put, but strongly reduce the usage by switching to RCU, then I'm generally okay with proceeding with the overlay feature because I can see how it would work in the new model. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V3 14/17] ppc/pci: create/release dev-tree node for VFs
On Thu, Jun 19, 2014 at 3:46 AM, Wei Yang weiy...@linux.vnet.ibm.com wrote: On Wed, Jun 18, 2014 at 07:26:27PM +0100, Grant Likely wrote: On Tue, Jun 10, 2014 at 2:56 AM, Wei Yang weiy...@linux.vnet.ibm.com wrote: Currently, powernv platform is not aware of VFs. This means no dev-node represents a VF. Also, VF PCI device is created when PF driver want to enable it. This leads to the pdn-pdev and pdn-pe_number an invalid value. This patch create/release dev-node for VF and fixs this when a VF's pci_dev is created. Signed-off-by: Wei Yang weiy...@linux.vnet.ibm.com I don't think this is the right way to handle this. Unless it is a fixup to a buggy devicetree provided by firmware, I don't want to see any code modifying the devicetree to describe stuff that is able to be directly enumerated. Really the pci code should handle the lack of a device_node gracefully. If it cannot then it should be fixed. Grant, Glad to see your comment. I will fix this in the firmware. That's not really what I meant. The kernel should be able to deal with virtual functions even if firmware doesn't know how, and the kernel should not require modifying the device tree to support them. I'm saying fix the kernel so that a device node is not necessary for virtual functions. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V3 14/17] ppc/pci: create/release dev-tree node for VFs
On Tue, Jun 10, 2014 at 2:56 AM, Wei Yang weiy...@linux.vnet.ibm.com wrote:
Currently, powernv platform is not aware of VFs. This means no dev-node
represents a VF. Also, VF PCI device is created when PF driver want to enable
it. This leads to the pdn-pdev and pdn-pe_number an invalid value.
This patch create/release dev-node for VF and fixs this when a VF's pci_dev
is created.
Signed-off-by: Wei Yang weiy...@linux.vnet.ibm.com
I don't think this is the right way to handle this. Unless it is a
fixup to a buggy devicetree provided by firmware, I don't want to see
any code modifying the devicetree to describe stuff that is able to be
directly enumerated. Really the pci code should handle the lack of a
device_node gracefully. If it cannot then it should be fixed.
g.
---
arch/powerpc/platforms/powernv/Kconfig|1 +
arch/powerpc/platforms/powernv/pci-ioda.c | 103
+
arch/powerpc/platforms/powernv/pci.c | 20 ++
3 files changed, 124 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/Kconfig
b/arch/powerpc/platforms/powernv/Kconfig
index 895e8a2..0dd331b 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -11,6 +11,7 @@ config PPC_POWERNV
select PPC_UDBG_16550
select PPC_SCOM
select ARCH_RANDOM
+ select OF_DYNAMIC
default y
config PPC_POWERNV_RTAS
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index e46c5bf..9ace027 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -23,6 +23,7 @@
#include linux/io.h
#include linux/msi.h
#include linux/memblock.h
+#include linux/of_pci.h
#include asm/sections.h
#include asm/io.h
@@ -771,6 +772,108 @@ static void pnv_pci_ioda_setup_PEs(void)
}
}
+#ifdef CONFIG_PCI_IOV
+static void pnv_pci_create_vf_node(struct pci_dev *dev, u16 vf_num)
+{
+ struct device_node *dn, *p_dn;
+ struct pci_dn *pdn;
+ struct pci_controller *hose;
+ struct property *pp;
+ void* value;
+ u16 id;
+
+ hose = pci_bus_to_host(dev-bus);
+
+ /* Create dev-tree node for VFs if this is a PF */
+ p_dn = pci_bus_to_OF_node(dev-bus);
+ if (p_dn == NULL) {
+ dev_err(dev-dev, SRIOV: VF bus NULL device node\n);
+ return;
+ }
+
+ for (id = 0; id vf_num; id++) {
+ dn = kzalloc(sizeof(*dn), GFP_KERNEL);
+ pdn = kzalloc(sizeof(*pdn), GFP_KERNEL);
+ pp = kzalloc(sizeof(*pp), GFP_KERNEL);
+ value = kzalloc(sizeof(u32), GFP_KERNEL);
+
+ if (!dn || !pdn || !pp || !value) {
+ kfree(dn);
+ kfree(pdn);
+ kfree(pp);
+ kfree(value);
+ dev_warn(dev-dev, %s: failed to create
+ dev-tree node for idx(%d)\n,
+ __func__, id);
+
+ break;
+ }
+
+ pp-value = value;
+ pdn-node = dn;
+ pdn-devfn = pci_iov_virtfn_devfn(dev, id);
+ pdn-busno = dev-bus-number;
+ pdn-pe_number = IODA_INVALID_PE;
+ pdn-phb = hose;
+
+ dn-data = pdn;
+ kref_init(dn-kref);
+ dn-full_name = dn-name =
+ kasprintf(GFP_KERNEL, %s/vf%d,
+ p_dn-full_name, pdn-devfn);
+ dn-parent = p_dn;
+
+ pp-name = kasprintf(GFP_KERNEL, reg);
+ pp-length = 5 * sizeof(__be32);
+ *(u32*)pp-value = cpu_to_be32(pdn-devfn) 8;
+ dn-properties = pp;
+
+ of_attach_node(dn);
+ }
+}
+
+static void pnv_pci_release_vf_node(struct pci_dev *dev, u16 vf_num)
+{
+ struct device_node *dn;
+ struct property *pp;
+ u16 id;
+
+ for (id = 0; id vf_num; id++) {
+ dn = of_pci_find_child_device(dev-bus-dev.of_node,
+ pci_iov_virtfn_devfn(dev, id));
+ if (!dn)
+ continue;
+
+ of_detach_node(dn);
+ pp = dn-properties;
+ kfree(pp-name);
+ kfree(pp-value);
+ kfree(pp);
+ kfree(dn-data);
+ kfree(dn);
+ }
+}
+
+int pcibios_sriov_disable(struct pci_dev *pdev)
+{
+ struct pci_sriov *iov;
+ u16 vf_num;
+
+ iov = pdev-sriov;
+ vf_num = iov-num_VFs;
+ pnv_pci_release_vf_node(pdev, vf_num);
+
+ return 0;
+}
+
+int pcibios_sriov_enable(struct pci_dev *pdev, u16 vf_num)
+{
+ pnv_pci_create_vf_node(pdev, vf_num);
+
+ return 0;
+}
OF_DYNAMIC node lifecycle
Hi Nathan and Tyrel, I'm looking into lifecycle issues on nodes modified by OF_DYNAMIC, and I'm hoping you can help me. Right now, pseries seems to be the only user of OF_DYNAMIC, but making OF_DYNAMIC work has a huge impact on the entire kernel because it requires all DT code to manage reference counting with iterating over nodes. Most users simply get it wrong. Pantelis did some investigation and found that the reference counts on a running kernel are all over the place. I have my doubts that any code really gets it right. The problem is that users need to know when it is appropriate to call of_node_get()/of_node_put(). All list traversals that exit early need an extra call to of_node_put(), and code that is searching for a node in the tree and holding a reference to it needs to call of_node_get(). I've got a few pseries questions: - What are the changes being requested by pseries firmware? Is it only CPUs and memory nodes, or does it manipulate things all over the tree? - How frequent are the changes? How many changes would be likely over the runtime of the system? - Are you able to verify that removed nodes are actually able to be freed correctly? Do you have any testcases for node removal? I'm thinking very seriously about changing the locking semantics of DT code entirely so that most users never have to worry about of_node_get/put at all. If the DT code is switched to use rcu primitives for tree iteration (which also means making DT code use list_head, something I'm already investigating), then instead of trying to figure out of_node_get/put rules, callers could use rcu_read_lock()/rcu_read_unlock() to protect the region that is searching over nodes, and only call of_node_get() if the node pointer is needed outside the rcu read-side lock. I'd really like to be rid of the node reference counting entirely, but I can't figure out a way of doing that safely, so I'd settle for making it a lot easier to get correct. Thoughts? g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] of: Handle memory@0 node on PPC32 only
On Thu, 24 Apr 2014 10:26:42 +0100, Leif Lindholm leif.lindh...@linaro.org wrote: On Wed, Apr 23, 2014 at 02:10:58PM +0100, Grant Likely wrote: Does anyone have a LongTrail DT to hand, and if so does the root have a compatible string? From grepping through the kernel I could only find a model string (IBM,LongTrail). Actually, on LongTrail this can be removed from the common code entirely. It has real open firmware and PowerPC already has the infrastructure for fixing up the device tree. Here's a draft patch that I've compile tested, but nothing else. I would certainly be happy with that. Consider my 3/3 withdrawn. And if the kernel proper will stop honoring nodes with no type, there is no need for the stub to treat those specially either. So, after thinking about it some more, I've changed my minde about the whole thing again. The impact is quite contained because there weren't a lot of systems that had ram based at 0. I'm going to commit this patch, but I'll include a note in the commit text that if it causes trouble for anyone that they should yell and I'll revert it. I don't think it will though. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 00/21] FDT clean-ups and libfdt support
On Tue, 22 Apr 2014 20:18:00 -0500, Rob Herring robherri...@gmail.com wrote: From: Rob Herring r...@kernel.org This is a series of clean-ups of architecture FDT code and converts the core FDT code over to using libfdt functions. This is in preparation to add FDT based address translation parsing functions for early console support. This series removes direct access to FDT data from all arches except powerpc. The current MIPS lantiq and xlp DT code is buggy as built-in DTBs need to be copied out of init section. Patches 2 and 3 should be applied to 3.15. Changes in v2 are relatively minor. There was a bug in the unflattening code where walking up the tree was not being handled correctly (thanks to Michal Simek). I re-worked things a bit to avoid globally adding libfdt include paths. A branch is available here[1], and I plan to put into linux-next in a few days. Please test! I've compiled on arm, arm64, mips, microblaze, xtensa, and powerpc and booted on arm and arm64. This is pretty great work. I'll read through them again and I may have a comment or two, but in general you can add my tested by tag: Tested-by: Grant Likely grant.lik...@linaro.org 40 files changed, 280 insertions(+), 598 deletions(-) I love the diffstat! g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] of: Handle memory@0 node on PPC32 only
On Wed, 23 Apr 2014 11:45:28 +0100, Mark Rutland mark.rutl...@arm.com wrote:
On Tue, Apr 22, 2014 at 02:35:15PM +0100, Grant Likely wrote:
On Fri, 18 Apr 2014 13:59:24 +0100, Leif Lindholm
leif.lindh...@linaro.org wrote:
Hi Geert,
On Fri, Apr 18, 2014 at 10:04:15AM +0200, Geert Uytterhoeven wrote:
On Thu, Apr 17, 2014 at 7:42 PM, Leif Lindholm
leif.lindh...@linaro.org wrote:
In order to deal with an firmware bug on a specific ppc32 platform
(longtrail), early_init_dt_scan_memory() looks for a node called
memory@0 on all platforms. Restrict this quirk to ppc32 kernels only.
This breaks backwards compatibilty with old DTSes (at least on ARM/MIPS,
where you added the missing property in patches 1 and 2 of the series)?
As Rob said in response to 0/3, the MIPSs would likely not be affected,
since they embed the DT.
For the Longtrail, I don't care much anymore, as mine died in 2004.
AFAIK, there have never been many users anyway.
There are still a few mentions of it under arch/powerpc/, so I wouldn't
want to be the one to kill it off...
How about the below v2 3/3 to address the ARM platform?
The problem with this approach is that selecting one board that needs it
automatically makes it active for all boards. It would need to be
something more like the following:
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 399e242e1a42..55d65b2b4c74 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -887,12 +887,10 @@ int __init early_init_dt_scan_memory(unsigned long
node, const char *uname,
/* We are scanning memory nodes only */
if (type == NULL) {
- /*
-* The longtrail doesn't have a device_type on the
-* /memory node, so look for the node called /memory@0.
-*/
if (depth != 1 || strcmp(uname, memory@0) != 0)
return 0;
+ if (!of_flat_dt_match(dt_root, memory_quirk_list))
+ return 0;
} else if (strcmp(type, memory) != 0)
return 0;
With a list of compatible properties for affected boards.
That looks sane to me.
Does anyone have a LongTrail DT to hand, and if so does the root have a
compatible string? From grepping through the kernel I could only find a
model string (IBM,LongTrail).
Actually, on LongTrail this can be removed from the common code
entirely. It has real open firmware and PowerPC already has the
infrastructure for fixing up the device tree.
Here's a draft patch that I've compile tested, but nothing else.
g.
---
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 078145acf7fb..18b2c3fee98f 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -2587,9 +2587,18 @@ static void __init fixup_device_tree_chrp(void)
phandle ph;
u32 prop[6];
u32 rloc = 0x01006000; /* IO space; PCI device = 12 */
- char *name;
+ char *name, strprop[16];
int rc;
+ /* Deal with missing device_type in LongTrail memory node */
+ name = /memory@0;
+ ph = call_prom(finddevice, 1, 1, ADDR(name));
+ rc = prom_getprop(ph, device_type, strprop, sizeof(strprop));
+ if (rc == PROM_ERROR || strcmp(strprop, memory) != 0) {
+ prom_printf(Fixing up missing device_type on /memory@0
node...\n);
+ prom_setprop(ph, name, device_type, memory,
sizeof(memory));
+ }
+
name = /pci@8000/isa@c;
ph = call_prom(finddevice, 1, 1, ADDR(name));
if (!PHANDLE_VALID(ph)) {
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 7a2ef7bb8022..7cda0d279cbe 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -886,14 +886,7 @@ int __init early_init_dt_scan_memory(unsigned long node,
const char *uname,
unsigned long l;
/* We are scanning memory nodes only */
- if (type == NULL) {
- /*
-* The longtrail doesn't have a device_type on the
-* /memory node, so look for the node called /memory@0.
-*/
- if (depth != 1 || strcmp(uname, memory@0) != 0)
- return 0;
- } else if (strcmp(type, memory) != 0)
+ if (!type || strcmp(type, memory) != 0)
return 0;
reg = of_get_flat_dt_prop(node, linux,usable-memory, l);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/3] of: dts: enable memory@0 quirk for PPC32 only
On Tue, 22 Apr 2014 15:05:26 +0100, Leif Lindholm leif.lindh...@linaro.org wrote: On Tue, Apr 22, 2014 at 02:08:29PM +0100, Grant Likely wrote: I would prefer to see a WARN_ON(!IS_ENABLED(CONFIG_PPC32)); added here. I completely agree. OK. So do we keep this around on unaffected architectures in perpetuity? Or can there be some cut-off date when the majority of DT-enabled platforms can stop including this workaround which permits new incorrect DTs to be implemented so long as they are incorrect in this particular way? Really, I would like to see quirks like this fixed up out of line from the normal flow somewhat like how PCI quirks are handled. So in this example, we would just add the missing property to the dtb for the broken platform before doing the memory scan. That does then require libfdt which is something I'm working on. In fact, for Leif's purposes, I would rather have a flag when booting via UEFI, and make the kernel skip the memory node parsing if the UEFI memory map is available. Then the stub doesn't need to do any munging of the DTB at all. The reason for me doing that is because we (including you) agreed at the discussion held during LCU13 that this was the safest way of preventing mischief like userland trying to read information from /proc/device-tree... I'm not the most consistent of people. I often change my mind and then have no recollection of ever thinking differently. Userland reading from /proc/device-tree isn't so much a problem, but kernel drivers doing it might be. But, regardless of whether or not the stub clears out the memory nodes, it is still I think good practice for the kernel to make the decision to ignore memory nodes, and not rely on them being cleared correctly. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/3] of: dts: enable memory@0 quirk for PPC32 only
On Tue, 22 Apr 2014 15:05:26 +0100, Leif Lindholm leif.lindh...@linaro.org wrote: On Tue, Apr 22, 2014 at 02:08:29PM +0100, Grant Likely wrote: I would prefer to see a WARN_ON(!IS_ENABLED(CONFIG_PPC32)); added here. I completely agree. OK. So do we keep this around on unaffected architectures in perpetuity? Or can there be some cut-off date when the majority of DT-enabled platforms can stop including this workaround which permits new incorrect DTs to be implemented so long as they are incorrect in this particular way? I'd be fine with a patch that can enable the workaround selectively for specific machines. Some people will still hit breakage, but we can fix that by adding their machine to the quirk list. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/3] of: dts: enable memory@0 quirk for PPC32 only
On Fri, 18 Apr 2014 10:37:58 -0500, Rob Herring robherri...@gmail.com wrote: On Fri, Apr 18, 2014 at 7:48 AM, Leif Lindholm leif.lindh...@linaro.org wrote: On Thu, Apr 17, 2014 at 07:43:13PM -0500, Rob Herring wrote: On Thu, Apr 17, 2014 at 12:41 PM, Leif Lindholm leif.lindh...@linaro.org wrote: drivers/of/fdt.c contains a workaround for a missing memory type entry on longtrail firmware. Make that quirk PPC32 only, and while at it - fix up the .dts files in the tree currently working only because of that quirk. But why do you need this? Apart from the current code permitting recreating a 15+ year old firmware bug into completely new platform ports? I would prefer to see a WARN_ON(!IS_ENABLED(CONFIG_PPC32)); added here. I completely agree. Really, I would like to see quirks like this fixed up out of line from the normal flow somewhat like how PCI quirks are handled. So in this example, we would just add the missing property to the dtb for the broken platform before doing the memory scan. That does then require libfdt which is something I'm working on. In fact, for Leif's purposes, I would rather have a flag when booting via UEFI, and make the kernel skip the memory node parsing if the UEFI memory map is available. Then the stub doesn't need to do any munging of the DTB at all. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] of: Handle memory@0 node on PPC32 only
On Fri, 18 Apr 2014 13:59:24 +0100, Leif Lindholm leif.lindh...@linaro.org
wrote:
Hi Geert,
On Fri, Apr 18, 2014 at 10:04:15AM +0200, Geert Uytterhoeven wrote:
On Thu, Apr 17, 2014 at 7:42 PM, Leif Lindholm leif.lindh...@linaro.org
wrote:
In order to deal with an firmware bug on a specific ppc32 platform
(longtrail), early_init_dt_scan_memory() looks for a node called
memory@0 on all platforms. Restrict this quirk to ppc32 kernels only.
This breaks backwards compatibilty with old DTSes (at least on ARM/MIPS,
where you added the missing property in patches 1 and 2 of the series)?
As Rob said in response to 0/3, the MIPSs would likely not be affected,
since they embed the DT.
For the Longtrail, I don't care much anymore, as mine died in 2004.
AFAIK, there have never been many users anyway.
There are still a few mentions of it under arch/powerpc/, so I wouldn't
want to be the one to kill it off...
How about the below v2 3/3 to address the ARM platform?
The problem with this approach is that selecting one board that needs it
automatically makes it active for all boards. It would need to be
something more like the following:
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 399e242e1a42..55d65b2b4c74 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -887,12 +887,10 @@ int __init early_init_dt_scan_memory(unsigned long node,
const char *uname,
/* We are scanning memory nodes only */
if (type == NULL) {
- /*
-* The longtrail doesn't have a device_type on the
-* /memory node, so look for the node called /memory@0.
-*/
if (depth != 1 || strcmp(uname, memory@0) != 0)
return 0;
+ if (!of_flat_dt_match(dt_root, memory_quirk_list))
+ return 0;
} else if (strcmp(type, memory) != 0)
return 0;
With a list of compatible properties for affected boards.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v6 00/11] reserved-memory regions/CMA in devicetree, again
On Tue, Mar 11, 2014 at 10:37 AM, Grant Likely grant.lik...@linaro.org wrote:
Hi Ben, Russell and Catalin,
I've got this series queued up, and I'd like to be ready to merge it
in the next merge window. I'm going to queue it up in linux-next. If
you have any concerns, please shout and it can be removed. I won't ask
Linus to pull it without you giving the okay.
Hi Ben and Russell.
Any feedback on this? The arch/powerpc and arch/arm patches are a
single line change each and are easy to revert if they cause problems.
I'm going to send Linus a pull request tomorrow morning if I don't
hear anything before then.
g.
On Fri, Feb 28, 2014 at 1:42 PM, Marek Szyprowski
m.szyprow...@samsung.com wrote:
Hello again!
Here is another update of the support for reserved memory regions in
device tree. I've fixes a few more minor issues pointed by Grant. See
changelog for more details.
The initial code for this feature were posted here [1], merged as commit
9d8eab7af79cb4ce2de5de39f82c455b1f796963 (drivers: of: add
initialization code for dma reserved memory) and later reverted by
commit 1931ee143b0ab72924944bc06e363d837ba05063. For more information,
see [2]. Finally a new bindings has been proposed [3] and Josh
Cartwright a few days ago prepared some code which implements those
bindings [4]. This finally pushed me again to find some time to finish
this task and review the code. Josh agreed to give me the ownership of
this series to continue preparing them for mainline inclusion.
For more information please refer to the changlelog and links below.
[1]:
http://lkml.kernel.org/g/1377527959-5080-1-git-send-email-m.szyprow...@samsung.com
[2]:
http://lkml.kernel.org/g/1381476448-14548-1-git-send-email-m.szyprow...@samsung.com
[3]: http://lkml.kernel.org/g/20131030134702.19b57c40...@trevor.secretlab.ca
[4]: http://thread.gmane.org/gmane.linux.documentation/19579
Changelog:
v6:
- removed the need for #memory-region-cells property
- fixed compilation issues on some systems
- some other minor code cleanups
v5: https://lkml.org/lkml/2014/2/21/147
- sliced main patch into several smaller patches on Grant's request
- fixed coding style issues pointed by Grant
- use node-phandle value directly instead of parsing properties manually
v4: https://lkml.org/lkml/2014/2/20/150
- dynamic allocations are processed after all static reservations has been
done
- moved code for handling static reservations to drivers/of/fdt.c
- removed node matching by string comparison, now phandle values are used
directly
- moved code for DMA and CMA handling directly to
drivers/base/dma-{coherent,contiguous}.c
- added checks for proper #size-cells, #address-cells, ranges properties
in /reserved-memory node
- even more code cleanup
- added init code for ARM64 and PowerPC
v3: http://article.gmane.org/gmane.linux.documentation/20169/
- refactored memory reservation code, created common code to parse reg, size,
align, alloc-ranges properties
- added support for multiple tuples in 'reg' property
- memory is reserved regardless of presence of the driver for its compatible
- prepared arch specific hooks for memory reservation (defaults use memblock
calls)
- removed node matching by string during device initialization
- CMA init code: added checks for required region alignment
- more code cleanup here and there
v2: http://thread.gmane.org/gmane.linux.documentation/19870/
- removed copying of the node name
- split shared-dma-pool handling into separate files (one for CMA and one
for dma_declare_coherent based implementations) for making the code easier
to understand
- added support for AMBA devices, changed prototypes to use struct decice
instead of struct platform_device
- renamed some functions to better match other names used in drivers/of/
- restructured the rest of the code a bit for better readability
- added 'reusable' property to exmaple linux,cma node in documentation
- exclusive dma (dma_coherent) is used for only handling 'shared-dma-pool'
regions without 'reusable' property and CMA is used only for handling
'shared-dma-pool' regions with 'reusable' property.
v1: http://thread.gmane.org/gmane.linux.documentation/19579
- initial version prepared by Josh Cartwright
Summary:
Grant Likely (1):
of: document bindings for reserved-memory nodes
Marek Szyprowski (10):
drivers: of: add initialization code for static reserved memory
drivers: of: add initialization code for dynamic reserved memory
drivers: of: add support for custom reserved memory drivers
drivers: of: add automated assignment of reserved regions to client
devices
drivers: of: initialize and assign reserved memory to newly created
devices
drivers: dma-coherent: add initialization from device tree
drivers: dma-contiguous: add initialization from device tree
arm: add support for reserved memory defined by device tree
arm64: add support for reserved
Re: [PATCH] of: give priority to the compatible match in __of_match_node()
On Wed, 19 Feb 2014 13:25:54 -0500, Paul Gortmaker paul.gortma...@windriver.com wrote: On Thu, Feb 13, 2014 at 2:01 PM, Rob Herring robherri...@gmail.com wrote: On Wed, Feb 12, 2014 at 5:38 AM, Kevin Hao haoke...@gmail.com wrote: When the device node do have a compatible property, we definitely prefer the compatible match besides the type and name. Only if there is no such a match, we then consider the candidate which doesn't have compatible entry but do match the type or name with the device node. This is based on a patch from Sebastian Hesselbarth. http://patchwork.ozlabs.org/patch/319434/ I did some code refactoring and also fixed a bug in the original patch. I'm inclined to just revert this once again and avoid possibly breaking yet another platform. Well, for what it is worth, today's (Feb19th) linux-next tree fails to boot on my sbc8548. It fails with: I think I've got it fixed now with the latest series. Please try the devicetree/merge branch on git://git.secretlab.ca/git/linux g. --- libphy: Freescale PowerQUICC MII Bus: probed mdio_bus ethernet@e002400: /soc8548@e000/ethernet@24000/mdio@520 PHY address 1312 is too large libphy: Freescale PowerQUICC MII Bus: probed libphy: Freescale PowerQUICC MII Bus: probed mdio_bus ethernet@e002500: /soc8548@e000/ethernet@25000/mdio@520 PHY address 1312 is too large libphy: Freescale PowerQUICC MII Bus: probed TCP: cubic registered Initializing XFRM netlink socket NET: Registered protocol family 17 fail nfs mount --- On a normal boot, we should see this: --- libphy: Freescale PowerQUICC MII Bus: probed libphy: Freescale PowerQUICC MII Bus: probed fsl-gianfar e0024000.ethernet: enabled errata workarounds, flags: 0x4 fsl-gianfar e0024000.ethernet eth0: mac: 02:e0:0c:00:05:fd fsl-gianfar e0024000.ethernet eth0: Running with NAPI enabled fsl-gianfar e0024000.ethernet eth0: RX BD ring size for Q[0]: 256 fsl-gianfar e0024000.ethernet eth0: TX BD ring size for Q[0]: 256 fsl-gianfar e0025000.ethernet: enabled errata workarounds, flags: 0x4 fsl-gianfar e0025000.ethernet eth1: mac: 02:e0:0c:00:06:fd fsl-gianfar e0025000.ethernet eth1: Running with NAPI enabled fsl-gianfar e0025000.ethernet eth1: RX BD ring size for Q[0]: 256 fsl-gianfar e0025000.ethernet eth1: TX BD ring size for Q[0]: 256 TCP: cubic registered Initializing XFRM netlink socket NET: Registered protocol family 17 --- Git bisect says: ee8b26ad943aa34acc03ae6cde2b81d8d3d483d4 is the first bad commit commit ee8b26ad943aa34acc03ae6cde2b81d8d3d483d4 Author: Kevin Hao haoke...@gmail.com Date: Tue Feb 18 15:57:30 2014 +0800 of: reimplement the matching method for __of_match_node() In the current implementation of __of_match_node(), it will compare each given match entry against all the node's compatible strings with of_device_is_compatible(). To achieve multiple compatible strings per node with ordering from specific to generic, this requires given matches to be ordered from specific to generic. For most of the drivers this is not true and also an alphabetical ordering is more sane there. Therefore, we define a following priority order for the match, and then scan all the entries to find the best match. 1. specific compatible type name 2. specific compatible type 3. specific compatible name 4. specific compatible 5. general compatible type name 6. general compatible type 7. general compatible name 8. general compatible 9. type name 10. type 11. name This is based on some pseudo-codes provided by Grant Likely. Signed-off-by: Kevin Hao haoke...@gmail.com [grant.likely: Changed multiplier to 4 which makes more sense] Signed-off-by: Grant Likely grant.lik...@linaro.org :04 04 8f5dd19174417aece63b308ff299a5dbe2efa5a0 8401b0e3903e23e973845ee75b26b04345d803d2 M drivers As a double check, I checked out the head of linux-next, and did a revert of the above commit, and my sbc8548 can then boot properly. Not doing anything fancy ; using the defconfig exactly as-is, and ensuring the dtb is fresh from linux-next HEAD of today. Thanks, Paul. -- However, I think I would like to see this structured differently. We basically have 2 ways of matching: the existing pre-3.14 way and the desired match on best compatible only. All new bindings should match with the new way and the old way needs to be kept for compatibility. So lets structure the code that way. Search the match table first for best compatible with name and type NULL, then search the table the old way. I realize it appears you are doing this, but it is not clear this is the intent
Re: [PATCH] of: give priority to the compatible match in __of_match_node()
On Wed, 19 Feb 2014 16:23:02 -0500, Paul Gortmaker paul.gortma...@windriver.com wrote: On 14-02-19 03:41 PM, Grant Likely wrote: On Wed, 19 Feb 2014 13:25:54 -0500, Paul Gortmaker paul.gortma...@windriver.com wrote: On Thu, Feb 13, 2014 at 2:01 PM, Rob Herring robherri...@gmail.com wrote: On Wed, Feb 12, 2014 at 5:38 AM, Kevin Hao haoke...@gmail.com wrote: When the device node do have a compatible property, we definitely prefer the compatible match besides the type and name. Only if there is no such a match, we then consider the candidate which doesn't have compatible entry but do match the type or name with the device node. This is based on a patch from Sebastian Hesselbarth. http://patchwork.ozlabs.org/patch/319434/ I did some code refactoring and also fixed a bug in the original patch. I'm inclined to just revert this once again and avoid possibly breaking yet another platform. Well, for what it is worth, today's (Feb19th) linux-next tree fails to boot on my sbc8548. It fails with: I think I've got it fixed now with the latest series. Please try the devicetree/merge branch on git://git.secretlab.ca/git/linux That seems to work. Great, thanks for the testing. Can I add a Tested-by line for you? g. libphy: Freescale PowerQUICC MII Bus: probed libphy: Freescale PowerQUICC MII Bus: probed fsl-gianfar e0024000.ethernet: enabled errata workarounds, flags: 0x4 fsl-gianfar e0024000.ethernet eth0: mac: 02:e0:0c:00:05:fd fsl-gianfar e0024000.ethernet eth0: Running with NAPI enabled fsl-gianfar e0024000.ethernet eth0: RX BD ring size for Q[0]: 256 fsl-gianfar e0024000.ethernet eth0: TX BD ring size for Q[0]: 256 fsl-gianfar e0025000.ethernet: enabled errata workarounds, flags: 0x4 fsl-gianfar e0025000.ethernet eth1: mac: 02:e0:0c:00:06:fd fsl-gianfar e0025000.ethernet eth1: Running with NAPI enabled fsl-gianfar e0025000.ethernet eth1: RX BD ring size for Q[0]: 256 fsl-gianfar e0025000.ethernet eth1: TX BD ring size for Q[0]: 256 TCP: cubic registered Initializing XFRM netlink socket NET: Registered protocol family 17 ... root@sbc8548:~# cat /proc/version Linux version 3.14.0-rc3-00024-g7119f42057c6 (paul@yow-lpgnfs-02) (gcc version 4.5.2 (GCC) ) #1 Wed Feb 19 16:08:17 EST 2014 root@sbc8548:~# Paul. -- g. --- libphy: Freescale PowerQUICC MII Bus: probed mdio_bus ethernet@e002400: /soc8548@e000/ethernet@24000/mdio@520 PHY address 1312 is too large libphy: Freescale PowerQUICC MII Bus: probed libphy: Freescale PowerQUICC MII Bus: probed mdio_bus ethernet@e002500: /soc8548@e000/ethernet@25000/mdio@520 PHY address 1312 is too large libphy: Freescale PowerQUICC MII Bus: probed TCP: cubic registered Initializing XFRM netlink socket NET: Registered protocol family 17 fail nfs mount --- On a normal boot, we should see this: --- libphy: Freescale PowerQUICC MII Bus: probed libphy: Freescale PowerQUICC MII Bus: probed fsl-gianfar e0024000.ethernet: enabled errata workarounds, flags: 0x4 fsl-gianfar e0024000.ethernet eth0: mac: 02:e0:0c:00:05:fd fsl-gianfar e0024000.ethernet eth0: Running with NAPI enabled fsl-gianfar e0024000.ethernet eth0: RX BD ring size for Q[0]: 256 fsl-gianfar e0024000.ethernet eth0: TX BD ring size for Q[0]: 256 fsl-gianfar e0025000.ethernet: enabled errata workarounds, flags: 0x4 fsl-gianfar e0025000.ethernet eth1: mac: 02:e0:0c:00:06:fd fsl-gianfar e0025000.ethernet eth1: Running with NAPI enabled fsl-gianfar e0025000.ethernet eth1: RX BD ring size for Q[0]: 256 fsl-gianfar e0025000.ethernet eth1: TX BD ring size for Q[0]: 256 TCP: cubic registered Initializing XFRM netlink socket NET: Registered protocol family 17 --- Git bisect says: ee8b26ad943aa34acc03ae6cde2b81d8d3d483d4 is the first bad commit commit ee8b26ad943aa34acc03ae6cde2b81d8d3d483d4 Author: Kevin Hao haoke...@gmail.com Date: Tue Feb 18 15:57:30 2014 +0800 of: reimplement the matching method for __of_match_node() In the current implementation of __of_match_node(), it will compare each given match entry against all the node's compatible strings with of_device_is_compatible(). To achieve multiple compatible strings per node with ordering from specific to generic, this requires given matches to be ordered from specific to generic. For most of the drivers this is not true and also an alphabetical ordering is more sane there. Therefore, we define a following priority order for the match, and then scan all the entries to find the best match. 1. specific compatible type name 2. specific compatible type 3. specific compatible name 4. specific compatible 5. general
Re: [PATCH 2/2] of: search the best compatible match first in __of_match_node()
On Fri, 14 Feb 2014 13:22:46 +0800, Kevin Hao haoke...@gmail.com wrote:
Currently, of_match_node compares each given match against all node's
compatible strings with of_device_is_compatible.
To achieve multiple compatible strings per node with ordering from
specific to generic, this requires given matches to be ordered from
specific to generic. For most of the drivers this is not true and also
an alphabetical ordering is more sane there.
Therefore, this patch introduces a function to match each of the node's
compatible strings against all given compatible matches without type and
name first, before checking the next compatible string. This implies
that node's compatibles are ordered from specific to generic while
given matches can be in any order. If we fail to find such a match
entry, then fall-back to the old method in order to keep compatibility.
Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
Signed-off-by: Kevin Hao haoke...@gmail.com
---
drivers/of/base.c | 43 ++-
1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ba195fbce4c6..10b51106c854 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -730,13 +730,49 @@ out:
}
EXPORT_SYMBOL(of_find_node_with_property);
+static const struct of_device_id *
+of_match_compatible(const struct of_device_id *matches,
+ const struct device_node *node)
+{
+ const char *cp;
+ int cplen, l;
+ const struct of_device_id *m;
+
+ cp = __of_get_property(node, compatible, cplen);
+ while (cp (cplen 0)) {
+ m = matches;
+ while (m-name[0] || m-type[0] || m-compatible[0]) {
+ /* Only match for the entries without type and name */
+ if (m-name[0] || m-type[0] ||
+ of_compat_cmp(m-compatible, cp,
+ strlen(m-compatible)))
+ m++;
This seems wrong also. The compatible order should be checked for even
when m-name or m-type are set. You actually need to score the entries
to do this properly. The pseudo-code should look like this:
uint best_score = ~0;
of_device_id *best_match = NULL;
for_each(matches) {
uint score = ~0;
for_each_compatible(index) {
if (match-compatible == compatible[index])
score = index * 10;
}
/* Matching name is a bit better than not */
if (match-name == node-name)
score--;
/* Matching type is better than matching name */
/* (but matching both is even better than that */
if (match-type == node-type)
score -= 2;
if (score best_score)
best_match = match;
}
return best_match;
This is actually very similar to the original code. It is an easy
modification. This is very similar to how the of_fdt_is_compatible()
function works.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] of: search the best compatible match first in __of_match_node()
On Fri, 14 Feb 2014 09:53:40 -0600, Rob Herring robherri...@gmail.com wrote: On Thu, Feb 13, 2014 at 11:22 PM, Kevin Hao haoke...@gmail.com wrote: Currently, of_match_node compares each given match against all node's compatible strings with of_device_is_compatible. To achieve multiple compatible strings per node with ordering from specific to generic, this requires given matches to be ordered from specific to generic. For most of the drivers this is not true and also an alphabetical ordering is more sane there. Therefore, this patch introduces a function to match each of the node's compatible strings against all given compatible matches without type and name first, before checking the next compatible string. This implies that node's compatibles are ordered from specific to generic while given matches can be in any order. If we fail to find such a match entry, then fall-back to the old method in order to keep compatibility. Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Signed-off-by: Kevin Hao haoke...@gmail.com Looks good to me. I'll put this in next for a few days. I'd really like to see some acks and tested-by's before sending to Linus. As I commented on the patch, I don't think the new solution is correct either. I've made a suggestion on how to fix it, but in the mean time the revert should be applied and sent to Linus. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of: give priority to the compatible match in __of_match_node()
On Thu, 13 Feb 2014 13:01:42 -0600, Rob Herring robherri...@gmail.com wrote: On Wed, Feb 12, 2014 at 5:38 AM, Kevin Hao haoke...@gmail.com wrote: When the device node do have a compatible property, we definitely prefer the compatible match besides the type and name. Only if there is no such a match, we then consider the candidate which doesn't have compatible entry but do match the type or name with the device node. This is based on a patch from Sebastian Hesselbarth. http://patchwork.ozlabs.org/patch/319434/ I did some code refactoring and also fixed a bug in the original patch. I'm inclined to just revert this once again and avoid possibly breaking yet another platform. However, I think I would like to see this structured differently. We basically have 2 ways of matching: the existing pre-3.14 way and the desired match on best compatible only. All new bindings should match with the new way and the old way needs to be kept for compatibility. So lets structure the code that way. Search the match table first for best compatible with name and type NULL, then search the table the old way. I realize it appears you are doing this, but it is not clear this is the intent of the code. I would like to see this written as a patch with commit 105353145eafb3ea919 reverted first and you add a new match function to call first and then fallback to the existing function. I disagree here, partially because it complicates the code and partially because it leaves an incorrect corner case. Compatible is always the preferred matching, even on old drivers, but there are bindings that take both compatible and type or name into account. Even those cases should rank the compatible order. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] move of_find_next_cache_node to DT core
On Fri, 04 Oct 2013 11:42:49 +0100, Sudeep KarkadaNagesha sudeep.karkadanage...@arm.com wrote: Hi Grant, On 18/09/13 17:18, Sudeep KarkadaNagesha wrote: On 18/09/13 15:51, Grant Likely wrote: On Wed, 18 Sep 2013 11:53:03 +0100, Sudeep KarkadaNagesha sudeep.karkadanage...@arm.com wrote: From: Sudeep KarkadaNagesha sudeep.karkadanage...@arm.com Hi, The cache bindings are generic and used by many other architectures apart from PPC. These patches fixes and move the existing definition of of_find_next_cache_node to DT common code. Regards, Sudeep Acked-by: Grant Likely grant.lik...@secretlab.ca However, do you have a user for this function on other architectures yet? I'd like to see a user for the function in the same patch series.. Yes I have posted an RFC[1] following this series implementing cacheinfo for ARM similar to x86 implementation. I was not sure if it's good idea to combine it as its still initial RFC version. Do you prefer to have this a independent change or to go with the cache topology support patches[1] on ARM ? Ben's already picked it up, so I'm fine with it. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device tree
On Wed, 9 Oct 2013 14:38:24 +0800, yuantian.t...@freescale.com wrote:
From: Tang Yuantian yuantian.t...@freescale.com
The following SoCs will be affected: p2041, p3041, p4080,
p5020, p5040, b4420, b4860, t4240
Signed-off-by: Tang Yuantian yuantian.t...@freescale.com
Signed-off-by: Li Yang le...@freescale.com
---
v5:
- refine the binding document
- update the compatible string
v4:
- add binding document
- update compatible string
- update the reg property
v3:
- fix typo
v2:
- add t4240, b4420, b4860 support
- remove pll/4 clock from p2041, p3041 and p5020 board
.../devicetree/bindings/clock/corenet-clock.txt| 111
arch/powerpc/boot/dts/fsl/b4420si-post.dtsi| 35 +++
arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 +
arch/powerpc/boot/dts/fsl/b4860si-post.dtsi| 35 +++
arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 4 +
arch/powerpc/boot/dts/fsl/p2041si-post.dtsi| 60 +++
arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 4 +
arch/powerpc/boot/dts/fsl/p3041si-post.dtsi| 60 +++
arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 4 +
arch/powerpc/boot/dts/fsl/p4080si-post.dtsi| 112
+
arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 8 ++
arch/powerpc/boot/dts/fsl/p5020si-post.dtsi| 42
arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 2 +
arch/powerpc/boot/dts/fsl/p5040si-post.dtsi| 60 +++
arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 4 +
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi| 85
arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 12 +++
17 files changed, 640 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/corenet-clock.txt
diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt
b/Documentation/devicetree/bindings/clock/corenet-clock.txt
new file mode 100644
index 000..8efc62d
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt
@@ -0,0 +1,111 @@
+* Clock Block on Freescale CoreNet Platforms
+
+Freescale CoreNet chips take primary clocking input from the external
+SYSCLK signal. The SYSCLK input (frequency) is multiplied using
+multiple phase locked loops (PLL) to create a variety of frequencies
+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.
+
+1. Clock Block Binding
+
+Required properties:
+- compatible: Should include one or more of the following:
+ - fsl,chip-clockgen: for chip specific clock block
+ - fsl,qoriq-clockgen-[1,2].x: for chassis 1.x and 2.x clock
+- reg: Offset and length of the clock register set
+- clock-frequency: Indicates input clock frequency of clock block.
+ Will be set by u-boot
+
+Recommended properties:
+- #ddress-cells: Specifies the number of cells used to represent
typo
+ physical base addresses. Must be present if the device has
+ sub-nodes and set to 1 if present
+- #size-cells: Specifies the number of cells used to represent
+ the size of an address. Must be present if the device has
+ sub-nodes and set to 1 if present
+
+2. Clock Provider/Consumer Binding
+
+Most of the binding are from the common clock binding[1].
+ [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : Should include one or more of the following:
+ - fsl,qoriq-core-pll-[1,2].x: Indicates a core PLL clock device
+ - fsl,qoriq-core-mux-[1,2].x: Indicates a core multiplexer clock
+ device; divided from the core PLL clock
+ - fixed-clock: From common clock binding; indicates output clock
+ of oscillator
+ - fsl,qoriq-sysclk-[1,2].x: Indicates input system clock
+- #clock-cells: From common clock binding; indicates the number of
+ output clock. 0 is for one output clock; 1 for more than one clock
+
+Recommended properties:
+- clocks: Should be the phandle of input parent clock
+- clock-names: From common clock binding, indicates the clock name
+- clock-output-names: From common clock binding, indicates the names of
+ output clocks
+- reg: Should be the offset and length of clock block base address.
+ The length should be 4.
Binding looks reasonable to me.
g.
+
+Example for clock block and clock provider:
+/ {
+ clockgen: global-utilities@e1000 {
+ compatible = fsl,p5020-clockgen, fsl,qoriq-clockgen-1.0;
+ reg = 0xe1000 0x1000;
+ clock-frequency = 0;
+ #address-cells = 1;
+ #size-cells = 1;
+
+ sysclk: sysclk {
+ #clock-cells = 0;
+ compatible = fsl,qoriq-sysclk-1.0, fixed-clock;
+
Re: [PATCH v2 08/10] of/platform: Resolve interrupt references at probe time
On Wed, 16 Oct 2013 00:24:36 +0100, Grant Likely grant.lik...@linaro.org wrote: On Wed, 18 Sep 2013 15:24:50 +0200, Thierry Reding thierry.red...@gmail.com wrote: Interrupt references are currently resolved very early (when a device is created). This has the disadvantage that it will fail in cases where the interrupt parent hasn't been probed and no IRQ domain for it has been registered yet. To work around that various drivers use explicit initcall ordering to force interrupt parents to be probed before devices that need them are created. That's error prone and doesn't always work. If a platform device uses an interrupt line connected to a different platform device (such as a GPIO controller), both will be created in the same batch, and the GPIO controller won't have been probed by its driver when the depending platform device is created. Interrupt resolution will fail in that case. What is the reason for all the rework on the irq parsing return values? A return value of '0' is always an error on irq parsing, regardless of architecture even if NO_IRQ is defined as -1. I may have missed it, but I don't see any checking for specific error values in the return paths of the functions. If the specific return value isn't required (and I don't think it is), then you can simplify the whole series by getting rid of the rework patches. I've not heard back about the above, but I've just had a conversation with Rob about what to do here. The problem that I have is that it makes a specific return code need to traverse several levels of function calls and have a meaning come out the other end. It becomes difficult to figure out where that code actually comes from when reading the code. That's more of a gut-feel reaction rather than pointing out specifics though. The other thing that makes me nervous how invasive the series is. However, even with saying all of the above, I'm not saying outright no. I want to get this feature in. It is obviously needed and I'll even merge the patches piecemeal as the look ready (I've already merged 2). Regardless, the current series needs to be reworked. It conflicts with the other IRQ rework that I've already put into my tree. The best thing to do would probably be respin it against my current tree and repost. I'll take a fresh look then In the mean time, anything you can do to /sanely/ reduce the impact will probably help. :-) g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 01/10] of/irq: Rework of_irq_count()
On Sun, 22 Sep 2013 16:19:27 -0500, Rob Herring robherri...@gmail.com wrote:
On Wed, Sep 18, 2013 at 8:24 AM, Thierry Reding
thierry.red...@gmail.com wrote:
The of_irq_to_resource() helper that is used to implement of_irq_count()
tries to resolve interrupts and in fact creates a mapping for resolved
interrupts. That's pretty heavy lifting for something that claims to
just return the number of interrupts requested by a given device node.
Instead, use the more lightweight of_irq_map_one(), which, despite the
name, doesn't create an actual mapping. Perhaps a better name would be
of_irq_translate_one().
Signed-off-by: Thierry Reding tred...@nvidia.com
Acked-by: Rob Herring rob.herr...@calxeda.com
Applied (and fixed to match the of_irq_map_one -- of_irq_parse_one
rename that I'm going to merge in v3.13).
g.
---
drivers/of/irq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 1752988..5f44388 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -368,9 +368,10 @@ EXPORT_SYMBOL_GPL(of_irq_to_resource);
*/
int of_irq_count(struct device_node *dev)
{
+ struct of_irq irq;
int nr = 0;
- while (of_irq_to_resource(dev, nr, NULL))
+ while (of_irq_map_one(dev, nr, irq) == 0)
nr++;
return nr;
--
1.8.4
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 01/10] of/irq: Rework of_irq_count()
On Sun, 22 Sep 2013 16:19:27 -0500, Rob Herring robherri...@gmail.com wrote:
On Wed, Sep 18, 2013 at 8:24 AM, Thierry Reding
thierry.red...@gmail.com wrote:
The of_irq_to_resource() helper that is used to implement of_irq_count()
tries to resolve interrupts and in fact creates a mapping for resolved
interrupts. That's pretty heavy lifting for something that claims to
just return the number of interrupts requested by a given device node.
Instead, use the more lightweight of_irq_map_one(), which, despite the
name, doesn't create an actual mapping. Perhaps a better name would be
of_irq_translate_one().
Signed-off-by: Thierry Reding tred...@nvidia.com
Acked-by: Rob Herring rob.herr...@calxeda.com
Applied, thanks.
g.
---
drivers/of/irq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 1752988..5f44388 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -368,9 +368,10 @@ EXPORT_SYMBOL_GPL(of_irq_to_resource);
*/
int of_irq_count(struct device_node *dev)
{
+ struct of_irq irq;
int nr = 0;
- while (of_irq_to_resource(dev, nr, NULL))
+ while (of_irq_map_one(dev, nr, irq) == 0)
nr++;
return nr;
--
1.8.4
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 04/10] irqdomain: Return errors from irq_create_of_mapping()
On Mon, 23 Sep 2013 10:13:38 +0200, Thierry Reding thierry.red...@gmail.com
wrote:
On Sun, Sep 22, 2013 at 04:14:43PM -0500, Rob Herring wrote:
On Wed, Sep 18, 2013 at 8:24 AM, Thierry Reding
thierry.red...@gmail.com wrote:
Instead of returning 0 for all errors, allow the precise error code to
be propagated. This will be used in subsequent patches to allow further
propagation of error codes.
The interrupt number corresponding to the new mapping is returned in an
output parameter so that the return value is reserved to signal success
(== 0) or failure ( 0).
Signed-off-by: Thierry Reding tred...@nvidia.com
One comment below, otherwise:
Acked-by: Rob Herring rob.herr...@calxeda.com
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 905a24b..ae71b14 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -230,6 +230,7 @@ static int pci_read_irq_line(struct pci_dev *pci_dev)
{
struct of_irq oirq;
unsigned int virq;
+ int ret;
pr_debug(PCI: Try to map irq for %s...\n, pci_name(pci_dev));
@@ -266,8 +267,10 @@ static int pci_read_irq_line(struct pci_dev *pci_dev)
oirq.size, oirq.specifier[0], oirq.specifier[1],
of_node_full_name(oirq.controller));
- virq = irq_create_of_mapping(oirq.controller,
oirq.specifier,
-oirq.size);
+ ret = irq_create_of_mapping(oirq.controller,
oirq.specifier,
+ oirq.size, virq);
+ if (ret)
+ virq = NO_IRQ;
}
if(virq == NO_IRQ) {
pr_debug( Failed to map !\n);
Can you get rid of NO_IRQ usage here instead of adding to it.
I was trying to stay consistent with the remainder of the code. PowerPC
is a pretty heavy user of NO_IRQ. Of all 348 references, more than half
(182) are in arch/powerpc, so I'd rather like to get a go-ahead from
Benjamin on this.
That said, perhaps we should just go all the way and get rid of NO_IRQ
for good. Things could get somewhat messy, though. There are a couple of
these spread through the code:
#ifndef NO_IRQ
#define NO_IRQ (-1)
#endif
And all of them are wrong and need to be removed. :-) We're /slowly/
getting rid of the -1 and the usage of NO_IRQ. A global search and
replace of s/NO_IRQ/0/g can be very safely done on arch/powerpc since
powerpc has had NO_IRQ set correctly to '0' for a very long time now.
So, yes, if you're keen to do it I'd love to see a series getting rid of
more NO_IRQ users.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 08/10] of/platform: Resolve interrupt references at probe time
On Wed, 18 Sep 2013 15:24:50 +0200, Thierry Reding thierry.red...@gmail.com
wrote:
Interrupt references are currently resolved very early (when a device is
created). This has the disadvantage that it will fail in cases where the
interrupt parent hasn't been probed and no IRQ domain for it has been
registered yet. To work around that various drivers use explicit
initcall ordering to force interrupt parents to be probed before devices
that need them are created. That's error prone and doesn't always work.
If a platform device uses an interrupt line connected to a different
platform device (such as a GPIO controller), both will be created in the
same batch, and the GPIO controller won't have been probed by its driver
when the depending platform device is created. Interrupt resolution will
fail in that case.
What is the reason for all the rework on the irq parsing return values?
A return value of '0' is always an error on irq parsing, regardless of
architecture even if NO_IRQ is defined as -1. I may have missed it, but
I don't see any checking for specific error values in the return paths
of the functions.
If the specific return value isn't required (and I don't think it is),
then you can simplify the whole series by getting rid of the rework
patches.
g.
Another common workaround is for drivers to explicitly resolve interrupt
references at probe time. This is suboptimal, however, because it will
require every driver to duplicate the code.
This patch adds support for late interrupt resolution to the platform
driver core, by resolving the references right before a device driver's
.probe() function will be called. This not only delays the resolution
until a much later time (giving interrupt parents a better chance of
being probed in the meantime), but it also allows the platform driver
core to queue the device for deferred probing if the interrupt parent
hasn't registered its IRQ domain yet.
Signed-off-by: Thierry Reding tred...@nvidia.com
---
Changes in v2:
- split off IRQ parsing into separate function to make code flow simpler
- add comments to point out some aspects of the implementation
- make code idempotent (as pointed out by Grygorii Strashko
drivers/base/platform.c | 4 ++
drivers/of/platform.c | 107
+---
include/linux/of_platform.h | 7 +++
3 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..8dcf835 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -481,6 +481,10 @@ static int platform_drv_probe(struct device *_dev)
struct platform_device *dev = to_platform_device(_dev);
int ret;
+ ret = of_platform_probe(dev);
+ if (ret)
+ return ret;
+
if (ACPI_HANDLE(_dev))
acpi_dev_pm_attach(_dev, true);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 9b439ac..df6d56e 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -142,7 +142,7 @@ struct platform_device *of_device_alloc(struct
device_node *np,
struct device *parent)
{
struct platform_device *dev;
- int rc, i, num_reg = 0, num_irq;
+ int rc, i, num_reg = 0;
struct resource *res, temp_res;
dev = platform_device_alloc(, -1);
@@ -153,23 +153,21 @@ struct platform_device *of_device_alloc(struct
device_node *np,
if (of_can_translate_address(np))
while (of_address_to_resource(np, num_reg, temp_res) == 0)
num_reg++;
- num_irq = of_irq_count(np);
/* Populate the resource table */
- if (num_irq || num_reg) {
- res = kzalloc(sizeof(*res) * (num_irq + num_reg), GFP_KERNEL);
+ if (num_reg) {
+ res = kzalloc(sizeof(*res) * num_reg, GFP_KERNEL);
if (!res) {
platform_device_put(dev);
return NULL;
}
- dev-num_resources = num_reg + num_irq;
+ dev-num_resources = num_reg;
dev-resource = res;
for (i = 0; i num_reg; i++, res++) {
rc = of_address_to_resource(np, i, res);
WARN_ON(rc);
}
- WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
}
dev-dev.of_node = of_node_get(np);
@@ -490,4 +488,101 @@ int of_platform_populate(struct device_node *root,
return rc;
}
EXPORT_SYMBOL_GPL(of_platform_populate);
+
+/**
+ * of_platform_parse_irq() - parse interrupt resource from device node
+ * @pdev: pointer to platform device
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+static int of_platform_parse_irq(struct platform_device *pdev)
+{
+ struct device_node *np = pdev-dev.of_node;
+ unsigned int num_res = pdev-num_resources;
+
Re: [PATCH 22/51] DMA-API: amba: get rid of separate dma_mask
On Thu, 19 Sep 2013 22:47:01 +0100, Russell King rmk+ker...@arm.linux.org.uk wrote: AMBA Primecell devices always treat streaming and coherent DMA exactly the same, so there's no point in having the masks separated. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk for the drivers/of/platform.c portion: Acked-by: Grant Likely grant.lik...@linaro.org g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] move of_find_next_cache_node to DT core
On Wed, 18 Sep 2013 11:53:03 +0100, Sudeep KarkadaNagesha sudeep.karkadanage...@arm.com wrote: From: Sudeep KarkadaNagesha sudeep.karkadanage...@arm.com Hi, The cache bindings are generic and used by many other architectures apart from PPC. These patches fixes and move the existing definition of of_find_next_cache_node to DT common code. Regards, Sudeep Acked-by: Grant Likely grant.lik...@secretlab.ca However, do you have a user for this function on other architectures yet? I'd like to see a user for the function in the same patch series.. g. Sudeep KarkadaNagesha (2): powerpc: remove big endianness assumption in of_find_next_cache_node of: move definition of of_find_next_cache_node into common code. arch/powerpc/include/asm/prom.h | 3 --- arch/powerpc/kernel/prom.c | 31 --- drivers/of/base.c | 31 +++ include/linux/of.h | 2 ++ 4 files changed, 33 insertions(+), 34 deletions(-) -- 1.8.1.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of: Feed entire flattened device tree into the random pool
On Mon, 29 Jul 2013 13:11:50 +1000, Anton Blanchard an...@samba.org wrote: Hi, be32_to_cpu(initial_boot_params-totalsize); Ouch, thanks Grant. Anton -- We feed the entire DMI table into the random pool to provide better random data during early boot, so do the same with the flattened device tree. Signed-off-by: Anton Blanchard an...@samba.org Applied, thanks g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V3] i2c: move of helpers into the core
On Thu, 22 Aug 2013 18:00:14 +0200, Wolfram Sang w...@the-dreams.de wrote: I2C of helpers used to live in of_i2c.c but experience (from SPI) shows that it is much cleaner to have this in the core. This also removes a circular dependency between the helpers and the core, and so we can finally register child nodes in the core instead of doing this manually in each driver. So, fix the drivers and documentation, too. Acked-by: Rob Herring rob.herr...@calxeda.com Reviewed-by: Felipe Balbi ba...@ti.com Acked-by: Rafael J. Wysocki rafael.j.wyso...@intel.com Tested-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Wolfram Sang w...@the-dreams.de Acked-by: Grant Likely grant.lik...@linaro.org --- V2-V3: Was trying to be too smart by only fixing includes needed. Took a more general approach this time, converting of_i2c.h to i2c.h in case i2c.h was not already there. Otherwise remove it. Improved my build scripts and no build failures, no complaints from buildbot as well. Documentation/acpi/enumeration.txt |1 - arch/powerpc/platforms/44x/warp.c |1 - drivers/gpu/drm/tilcdc/tilcdc_slave.c |1 - drivers/gpu/drm/tilcdc/tilcdc_tfp410.c |1 - drivers/gpu/host1x/drm/output.c |2 +- drivers/i2c/busses/i2c-at91.c |3 - drivers/i2c/busses/i2c-cpm.c|6 -- drivers/i2c/busses/i2c-davinci.c|2 - drivers/i2c/busses/i2c-designware-platdrv.c |2 - drivers/i2c/busses/i2c-gpio.c |3 - drivers/i2c/busses/i2c-i801.c |2 - drivers/i2c/busses/i2c-ibm_iic.c|4 - drivers/i2c/busses/i2c-imx.c|3 - drivers/i2c/busses/i2c-mpc.c|2 - drivers/i2c/busses/i2c-mv64xxx.c|3 - drivers/i2c/busses/i2c-mxs.c|3 - drivers/i2c/busses/i2c-nomadik.c|3 - drivers/i2c/busses/i2c-ocores.c |3 - drivers/i2c/busses/i2c-octeon.c |3 - drivers/i2c/busses/i2c-omap.c |3 - drivers/i2c/busses/i2c-pnx.c|3 - drivers/i2c/busses/i2c-powermac.c |9 +- drivers/i2c/busses/i2c-pxa.c|2 - drivers/i2c/busses/i2c-s3c2410.c|2 - drivers/i2c/busses/i2c-sh_mobile.c |2 - drivers/i2c/busses/i2c-sirf.c |3 - drivers/i2c/busses/i2c-stu300.c |2 - drivers/i2c/busses/i2c-tegra.c |3 - drivers/i2c/busses/i2c-versatile.c |2 - drivers/i2c/busses/i2c-wmt.c|3 - drivers/i2c/busses/i2c-xiic.c |3 - drivers/i2c/i2c-core.c | 109 +- drivers/i2c/i2c-mux.c |3 - drivers/i2c/muxes/i2c-arb-gpio-challenge.c |1 - drivers/i2c/muxes/i2c-mux-gpio.c|1 - drivers/i2c/muxes/i2c-mux-pinctrl.c |1 - drivers/media/platform/exynos4-is/fimc-is-i2c.c |4 +- drivers/media/platform/exynos4-is/fimc-is.c |2 +- drivers/media/platform/exynos4-is/media-dev.c |1 - drivers/of/Kconfig |6 -- drivers/of/Makefile |1 - drivers/of/of_i2c.c | 114 --- drivers/staging/imx-drm/imx-tve.c |2 +- include/linux/i2c.h | 20 include/linux/of_i2c.h | 46 - sound/soc/fsl/imx-sgtl5000.c|2 +- sound/soc/fsl/imx-wm8962.c |2 +- 47 files changed, 138 insertions(+), 262 deletions(-) delete mode 100644 drivers/of/of_i2c.c delete mode 100644 include/linux/of_i2c.h diff --git a/Documentation/acpi/enumeration.txt b/Documentation/acpi/enumeration.txt index d9be7a9..958266e 100644 --- a/Documentation/acpi/enumeration.txt +++ b/Documentation/acpi/enumeration.txt @@ -238,7 +238,6 @@ An I2C bus (controller) driver does: if (ret) /* handle error */ - of_i2c_register_devices(adapter); /* Enumerate the slave devices behind this bus via ACPI */ acpi_i2c_register_devices(adapter); diff --git a/arch/powerpc/platforms/44x/warp.c b/arch/powerpc/platforms/44x/warp.c index 4cfa499..534574a 100644 --- a/arch/powerpc/platforms/44x/warp.c +++ b/arch/powerpc/platforms/44x/warp.c @@ -16,7 +16,6 @@ #include linux/interrupt.h #include linux/delay.h #include linux/of_gpio.h -#include linux/of_i2c.h #include linux/slab.h #include linux/export.h diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c index dfffaf0..a19f657 100644
Re: [RFC PATCH v2 3/4] powerpc: refactor of_get_cpu_node to support other architectures
On Thu, 22 Aug 2013 14:59:30 +0100, Mark Rutland mark.rutl...@arm.com wrote:
On Mon, Aug 19, 2013 at 02:56:10PM +0100, Sudeep KarkadaNagesha wrote:
On 19/08/13 14:02, Rob Herring wrote:
On 08/19/2013 05:19 AM, Mark Rutland wrote:
On Sat, Aug 17, 2013 at 11:09:36PM +0100, Benjamin Herrenschmidt wrote:
On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
which
the updated bindings[1] define #address-cells = 0 and so no reg
property.
[1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
Why did you do that in the binding ? That sounds like looking to create
problems ...
Traditionally, UP setups just used 0 as the reg property on other
architectures, why do differently ?
The decision was taken because we defined our reg property to refer to
the MPIDR register's Aff{2,1,0} bitfields, and on UP cores before v7
there's no MPIDR register at all. Given there can only be a single CPU
in that case, describing a register that wasn't present didn't seem
necessary or helpful.
What exactly reg represents is up to the binding definition, but it
still should be present IMO. I don't see any issue with it being
different for pre-v7.
Yes it's better to have 'reg' with value 0 than not having it.
Otherwise this generic of_get_cpu_node implementation would need some
_hack_ to handle that case.
I'm not sure that having some code to handle a difference in standard
between two architectures is a hack. If anything, I'd argue encoding a
reg of 0 that corresponds to a nonexistent MPIDR value (given that's
what the reg property is defined to map to on ARM) is more of a hack ;)
I'm not averse to having a reg value of 0 for this case, but given that
there are existing devicetrees without it, requiring a reg property will
break compatibility with them.
Then special cases those device trees, but you changing existing
convention really needs to be avoided. The referenced documentation
change is brand new, so we're not stuck with it.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC 11/14] powerpc: Eliminate NO_IRQ usage
On Thu, Jul 25, 2013 at 3:58 PM, Geert Uytterhoeven
ge...@linux-m68k.org wrote:
On Wed, Jan 11, 2012 at 9:22 PM, Grant Likely grant.lik...@secretlab.ca
wrote:
NO_IRQ is evil. Stop using it in arch/powerpc and powerpc device drivers
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 3e06696..55c6ff9 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -666,7 +666,7 @@ static int __devinit fsl_ssi_probe(struct
platform_device *pdev)
ssi_private-ssi_phys = res.start;
ssi_private-irq = irq_of_parse_and_map(np, 0);
- if (ssi_private-irq == NO_IRQ) {
+ if (!ssi_private-irq) {
dev_err(pdev-dev, no irq for node %s\n, np-full_name);
ret = -ENXIO;
goto error_iomap;
What's the plan with this patch?
This is now failing on xtensa, as it's one of the architectures that doesn't
define NO_IRQ. Only arm, c6x, mn10300, openrisc, parisc, powerpc, and sparc
define it.
Wow. I'd pretty much dropped that patch because I didn't have time to
chase it down. It should be pursued though.
In that particular case it is safe I think to apply the change. PPC
defines NO_IRQ to be 0 anyway.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] of: Specify initrd location using 64-bit
On Mon, 01 Jul 2013 16:34:26 -0500, Rob Herring robherri...@gmail.com wrote: On 07/01/2013 01:20 PM, Santosh Shilimkar wrote: On some PAE architectures, the entire range of physical memory could reside outside the 32-bit limit. These systems need the ability to specify the initrd location using 64-bit numbers. This patch globally modifies the early_init_dt_setup_initrd_arch() function to use 64-bit numbers instead of the current unsigned long. There has been quite a bit of debate about whether to use u64 or phys_addr_t. It was concluded to stick to u64 to be consistent with rest of the device tree code. As summarized by Geert, The address to load the initrd is decided by the bootloader/user and set at that point later in time. The dtb should not be tied to the kernel you are booting That was quoting me. Otherwise: Acked-by: Rob Herring rob.herr...@calxeda.com Unless Grant feels compelled to pick this up for 3.11, I think it has to wait for 3.12. Nope, 3.12 is fine. Applied. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of: Fix address decoding on Bimini and js2x machines
On Wed, 3 Jul 2013 15:37:56 +0100, Grant Likely grant.lik...@linaro.org wrote: On Wed, Jul 3, 2013 at 3:10 PM, Rob Herring robherri...@gmail.com wrote: On 07/03/2013 01:01 AM, Benjamin Herrenschmidt wrote: Commit: e38c0a1fbc5803cbacdaac0557c70ac8ca5152e7 of/address: Handle #address-cells 2 specially broke real time clock access on Bimini, js2x, and similar powerpc machines using the maple platform. That code was indirectly relying on the old (broken) behaviour of the translation for the hypertransport to ISA bridge. This fixes it by treating hypertransport as a PCI bus Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org CC: sta...@vger.kernel.org [v3.6+] --- Rob, if you have no objection I will put that in powerpc -next NP. Acked-by: Rob Herring rob.herr...@calxeda.com I'll include this in my 3.11 pull request for Linus Oops. Ben, I misread what you wrote. It would have been just fine to include it in your powerpc -next branch. Sorry for the confusion. Anyway, I saw your powerpc pull req and that this patch wasn't in it, so I've picked it up and will send it to Linus as soon as the test build completes. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of: Fix address decoding on Bimini and js2x machines
On Wed, Jul 3, 2013 at 3:10 PM, Rob Herring robherri...@gmail.com wrote: On 07/03/2013 01:01 AM, Benjamin Herrenschmidt wrote: Commit: e38c0a1fbc5803cbacdaac0557c70ac8ca5152e7 of/address: Handle #address-cells 2 specially broke real time clock access on Bimini, js2x, and similar powerpc machines using the maple platform. That code was indirectly relying on the old (broken) behaviour of the translation for the hypertransport to ISA bridge. This fixes it by treating hypertransport as a PCI bus Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org CC: sta...@vger.kernel.org [v3.6+] --- Rob, if you have no objection I will put that in powerpc -next NP. Acked-by: Rob Herring rob.herr...@calxeda.com I'll include this in my 3.11 pull request for Linus g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of: Specify initrd location using 64-bit
On Thu, Jun 27, 2013 at 9:54 PM, Rob Herring robherri...@gmail.com wrote:
On 06/21/2013 12:20 PM, Santosh Shilimkar wrote:
On Friday 21 June 2013 05:04 AM, Sebastian Andrzej Siewior wrote:
On 06/21/2013 02:52 AM, Santosh Shilimkar wrote:
diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
index 0a2c68f..62e2e8f 100644
--- a/arch/microblaze/kernel/prom.c
+++ b/arch/microblaze/kernel/prom.c
@@ -136,8 +136,7 @@ void __init early_init_devtree(void *params)
}
#ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
- unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
{
initrd_start = (unsigned long)__va(start);
initrd_end = (unsigned long)__va(end);
I think it would better to go here for phys_addr_t instead of u64. This
would force you in of_flat_dt_match() to check if the value passed from
DT specifies a memory address outside of 32bit address space and the
kernel can't deal with this because its phys_addr_t is 32bit only due
to a Kconfig switch.
For x86, the initrd has to remain in the 32bit address space so passing
the initrd in the upper range would violate the ABI. Not sure if this
is true for other archs as well (ARM obviously not).
That pretty much means phys_addr_t. It will work for me as well but
in last thread from consistency with memory and reserved node, Rob
insisted to keep it as u64. So before I re-spin another version,
would like to here what Rob has to say considering the x86 requirement.
Rob,
Are you ok with phys_addr_t since your concern was about rest
of the memory specific bits of the device-tree code use u64 ?
No. I still think it should be u64 for same reasons I said originally.
+1
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC 08/10] irqdomain: Refactor irq_domain_associate_many()
On Tue, Jun 18, 2013 at 4:09 AM, Mike Qiu qiud...@linux.vnet.ibm.com wrote:
于 2013/6/10 8:49, Grant Likely 写道:
Originally, irq_domain_associate_many() was designed to unwind the
mapped irqs on a failure of any individual association. However, that
proved to be a problem with certain IRQ controllers. Some of them only
support a subset of irqs, and will fail when attempting to map a
reserved IRQ. In those cases we want to map as many IRQs as possible, so
instead it is better for irq_domain_associate_many() to make a
best-effort attempt to map irqs, but not fail if any or all of them
don't succeed. If a caller really cares about how many irqs got
associated, then it should instead go back and check that all of the
irqs is cares about were mapped.
The original design open-coded the individual association code into the
body of irq_domain_associate_many(), but with no longer needing to
unwind associations, the code becomes simpler to split out
irq_domain_associate() to contain the bulk of the logic, and
irq_domain_associate_many() to be a simple loop wrapper.
This patch also adds a new error check to the associate path to make
sure it isn't called for an irq larger than the controller can handle,
and adds locking so that the irq_domain_mutex is held while setting up a
new association.
Signed-off-by: Grant Likely grant.lik...@linaro.org
[...]
-int irq_domain_associate_many(struct irq_domain *domain, unsigned int
irq_base,
- irq_hw_number_t hwirq_base, int count)
+int irq_domain_associate(struct irq_domain *domain, unsigned int virq,
+irq_hw_number_t hwirq)
{
- unsigned int virq = irq_base;
- irq_hw_number_t hwirq = hwirq_base;
- int i, ret; + struct irq_data *irq_data =
irq_get_irq_data(virq);
+ int ret;
- pr_debug(%s(%s, irqbase=%i, hwbase=%i, count=%i)\n, __func__,
- of_node_full_name(domain-of_node), irq_base,
(int)hwirq_base, count);
+ if (WARN(hwirq = domain-hwirq_max,
+error: hwirq 0x%x is too large for %s\n, (int)hwirq,
domain-name))
+ return -EINVAL;
+ if (WARN(!irq_data, error: virq%i is not allocated, virq))
+ return -EINVAL;
+ if (WARN(irq_data-domain, error: virq%i is already associated,
virq))
+ return -EINVAL;
Hi Grant,
I have a qestion here, assume that we have three hwirqs and alloc three
virqs, for first irq, it check irq_data
and irq_data-domain pass, but the second is failed, then this code do
nothing with failed( in
irq_domain_associate_many()) and continue to associated the third one.
This should be very dangours, even though I have no idea of when this could
happen.
Define what you mean by dangerous. You are correct that it is a bad
situation, and the kernel will complain very loudly if it happens.
However, the users of irq_domain_associate_many() are usually irq
controllers using a legacy domain that needs to be set up during early
boot. If the IRQ controller setup fails and gets unwound, then it is
very likely that there will be no output at all from the kernel. This
has actually been a problem on a number of platforms and it is a big
part of why this irqdomain refactoring series wasn't merged 6 months
ago.
I think it is better to relax the behaviour of
irq_domain_associate_many() so that if something fails, then at least
it can limp along and try to get at least some of the irqs associated.
That way we can find and fix problematic platforms rather than failing
silently.
Also, I don't see anything particularly dangerous about this behaviour
other than having some of the hwirqs not being associated with a virq.
The code protects against this path and if an irq controller receives
an IRQ on a unassociated hwirq, then the kernel will also log that as
an error.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 08/11] irqdomain: Refactor irq_domain_associate_many()
On Tue, Jun 18, 2013 at 2:25 AM, Michael Neuling mi...@neuling.org wrote: Michael Neuling mi...@neuling.org wrote: Grant, In next-20130617 we are getting the below crash on POWER7. Bisecting, points to this patch (d39046ec72 in next) Also, reverting just d39046ec72 fixes the crash in next-20130617. Odd. Of all the changes in that series, I would not have expected that one to cause any problems. I'm digging into it now... g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 08/11] irqdomain: Refactor irq_domain_associate_many()
On Tue, Jun 18, 2013 at 12:04 PM, Michael Neuling mi...@neuling.org wrote: Grant Likely grant.lik...@linaro.org wrote: On Tue, 18 Jun 2013 10:05:31 +0100, Grant Likely grant.lik...@linaro.org wrote: On Tue, Jun 18, 2013 at 2:25 AM, Michael Neuling mi...@neuling.org wrote: Michael Neuling mi...@neuling.org wrote: Grant, In next-20130617 we are getting the below crash on POWER7. Bisecting, points to this patch (d39046ec72 in next) Also, reverting just d39046ec72 fixes the crash in next-20130617. Odd. Of all the changes in that series, I would not have expected that one to cause any problems. I'm digging into it now... Ugh. I flubbed the commit. Try this patch. It should solve the problem: Yep, it fixes it. Thanks. Excellent, thanks for testing. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC 10/10] irqchip: Make versatile fpga irq driver a generic chip
On Sat, Jun 15, 2013 at 10:22 PM, Linus Walleij linus.wall...@linaro.org wrote: On Sat, Jun 15, 2013 at 11:19 PM, Linus Walleij linus.wall...@linaro.org wrote: It still works like a charm on the Integrator/AP. Tested-by: Linus Walleij linus.wall...@linaro.org BTW here is the new hwirq output in /proc/interrupts and it's really nice: root@integrator:/proc cat interrupts CPU0 17: 1845 pic 1 uart-pl010 22: 13716 pic 6 timer 24: 0 pic 8 rtc-pl030 Err: 0 Glad you like it. :-) g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] dtc: ensure #line directives don't consume data from the next line
On Wed, 05 Jun 2013 10:50:02 -0600, Stephen Warren swar...@wwwdotorg.org
wrote:
On 06/03/2013 09:36 AM, Stephen Warren wrote:
From: Stephen Warren swar...@nvidia.com
Previously, the #line parsing regex ended with ({WS}+[0-9]+)?. The {WS}
could match line-break characters. If the #line directive did not contain
the optional flags field at the end, this could cause any integer data on
the next line to be consumed as part of the #line directive parsing. This
could cause syntax errors (i.e. #line parsing consuming the leading 0
from a hex literal 0x1234, leaving x1234 to be parsed as cell data,
which is a syntax error), or invalid compilation results (i.e. simply
consuming literal 1234 as part of the #line processing, thus removing it
from the cell data).
Fix this by replacing {WS} with [ \t] so that it can't match line-breaks.
Convert all instances of {WS}, even though the other instances should be
irrelevant for any well-formed #line directive. This is done for
consistency and ultimate safety.
This is a port of upstream dtc commit a1ee6f0 (with same subject) to the
kernel's copy of dtc.
Rob, Grant, does this look OK to apply for v3.10-rc*?
The fix is in mainline now. Please check to make sure it is working for you.
g.
--
email sent from notmuch.vim plugin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of: Fix locking vs. interrupts
On Wed, 12 Jun 2013 10:25:56 +0200 (CEST), Thomas Gleixner t...@linutronix.de wrote: On Wed, 12 Jun 2013, Benjamin Herrenschmidt wrote: The OF code uses irqsafe locks everywhere except in a handful of functions for no obvious reasons. Since the conversion from the old rwlocks, this now triggers lockdep warnings when used at interrupt time. At least one driver (ibmvscsi) seems to be doing that from softirq context. This converts the few non-irqsafe locks into irqsafe ones, making them consistent with the rest of the code. Fun. https://lkml.org/lkml/2013/2/4/416 seems to have got lost Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org CC: sta...@vger.kernel.org [v3.9+] Acked-by: Thomas Gleixner t...@linutronix.de --- Note: It's silly to access the device-tree at interrupt time in most cases, and we should probably fix ibmvscsi, but for the time being, let's fix the Right. obvious bug. Thomas, this can probably still go into 3.10... If not, I've CCed stable. Should go through Grant I think. Applied, thanks. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/5] ibmebus: convert of_platform_driver to platform_driver
On Wed, 12 Jun 2013 15:29:38 +1000, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Wed, 2013-05-22 at 07:26 -0500, Rob Herring wrote: git://sources.calxeda.com/kernel/linux.git of-platform-removal Ben, Did you have a chance to test this? I want to get this into -next. I tested the one in for-next (sorry for the high latency). Works fine. Ack. Added acks and applied to my for-next branch. Thanks! g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC 01/10] irqdomain: Relax failure path on setting up mappings
Commit 98aa468e, irqdomain: Support for static IRQ mapping and
association introduced an API for directly associating blocks of hwirqs
to linux irqs. However, if any irq in that block failed to map (say if
the mapping functions returns an error because the irq is already
mapped) then the whole thing will fail and roll back. This is probably
too aggressive since there are valid reasons why a mapping may fail.
ie. Firmware may have a particular IRQ marked as unusable.
This patch drops the error path out of irq_domain_associate(). If a
mapping fails, then it is simply skipped. There is no reason to fail the
entire allocation.
v2: Still output an information message on failed mappings and make sure
attempted mapping gets cleared out of the irq_data structure.
Signed-off-by: Grant Likely grant.lik...@linaro.org
Cc: Paul Mundt let...@linux-sh.org
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Cc: Thomas Gleixner t...@linutronix.de
---
kernel/irq/irqdomain.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 20b677d..61d6d3c 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -464,23 +464,15 @@ int irq_domain_associate_many(struct irq_domain *domain,
unsigned int irq_base,
/*
* If map() returns -EPERM, this interrupt is
protected
* by the firmware or some other service and
shall not
-* be mapped.
-*
-* Since on some platforms we blindly try to
map everything
-* we end up with a log full of backtraces.
-*
-* So instead, we silently fail on -EPERM, it
is the
-* responsibility of the PIC driver to display
a relevant
-* message if needed.
+* be mapped. Don't bother telling the user
about it.
*/
if (ret != -EPERM) {
- pr_err(irq-%i==hwirq-0x%lx mapping
failed: %d\n,
- virq, hwirq, ret);
- WARN_ON(1);
+ pr_info(%s didn't like hwirq-0x%lx to
VIRQ%i mapping (rc=%d)\n,
+
of_node_full_name(domain-of_node), hwirq, virq, ret);
}
irq_data-domain = NULL;
irq_data-hwirq = 0;
- goto err_unmap;
+ continue;
}
}
--
1.8.1.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC 02/10] irqdomain: Replace LEGACY mapping with LINEAR
The LEGACY mapping unnecessarily complicates the irqdomain code and
can easily be implemented with a linear mapping. By ripping it out
and replacing it with the LINEAR mapping the object size of
irqdomain.c shrinks by about 330 bytes (ARMv7) which offsets the
additional allocation required by the linear map. It also makes it
possible for current LEGACY map users to pre-allocate irq_descs for a
subset of the hwirqs and dynamically allocate the rest as needed.
Signed-off-by: Grant Likely grant.lik...@linaro.org
Cc: Paul Mundt let...@linux-sh.org
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Cc: Thomas Gleixner t...@linutronix.de
Cc: Rob Herring rob.herr...@calxeda.com
---
include/linux/irqdomain.h | 7
kernel/irq/irqdomain.c| 84 ---
2 files changed, 6 insertions(+), 85 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index ba2c708..6f06241 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -95,11 +95,6 @@ struct irq_domain {
union {
struct {
unsigned int size;
- unsigned int first_irq;
- irq_hw_number_t first_hwirq;
- } legacy;
- struct {
- unsigned int size;
unsigned int *revmap;
} linear;
struct {
@@ -117,8 +112,6 @@ struct irq_domain {
struct irq_domain_chip_generic *gc;
};
-#define IRQ_DOMAIN_MAP_LEGACY 0 /* driver allocated fixed range of irqs.
-* ie. legacy 8259, gets irqs 1..15 */
#define IRQ_DOMAIN_MAP_NOMAP 1 /* no fast reverse mapping */
#define IRQ_DOMAIN_MAP_LINEAR 2 /* linear map of interrupts */
#define IRQ_DOMAIN_MAP_TREE 3 /* radix tree */
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 61d6d3c..1ac8cf4 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -82,13 +82,6 @@ void irq_domain_remove(struct irq_domain *domain)
mutex_lock(irq_domain_mutex);
switch (domain-revmap_type) {
- case IRQ_DOMAIN_MAP_LEGACY:
- /*
-* Legacy domains don't manage their own irq_desc
-* allocations, we expect the caller to handle irq_desc
-* freeing on their own.
-*/
- break;
case IRQ_DOMAIN_MAP_TREE:
/*
* radix_tree_delete() takes care of destroying the root
@@ -122,17 +115,6 @@ void irq_domain_remove(struct irq_domain *domain)
}
EXPORT_SYMBOL_GPL(irq_domain_remove);
-static unsigned int irq_domain_legacy_revmap(struct irq_domain *domain,
-irq_hw_number_t hwirq)
-{
- irq_hw_number_t first_hwirq = domain-revmap_data.legacy.first_hwirq;
- int size = domain-revmap_data.legacy.size;
-
- if (WARN_ON(hwirq first_hwirq || hwirq = first_hwirq + size))
- return 0;
- return hwirq - first_hwirq + domain-revmap_data.legacy.first_irq;
-}
-
/**
* irq_domain_add_simple() - Allocate and register a simple irq_domain.
* @of_node: pointer to interrupt controller's device tree node.
@@ -213,57 +195,17 @@ struct irq_domain *irq_domain_add_legacy(struct
device_node *of_node,
void *host_data)
{
struct irq_domain *domain;
- unsigned int i;
- domain = irq_domain_alloc(of_node, IRQ_DOMAIN_MAP_LEGACY, ops,
host_data);
+ pr_debug(Setting up legacy domain virq[%i:%i] == hwirq[%i:%i]\n,
+first_irq, first_irq + size - 1,
+(int)first_hwirq, (int)first_hwirq + size -1);
+
+ domain = irq_domain_add_linear(of_node, first_hwirq + size, ops,
host_data);
if (!domain)
return NULL;
- domain-revmap_data.legacy.first_irq = first_irq;
- domain-revmap_data.legacy.first_hwirq = first_hwirq;
- domain-revmap_data.legacy.size = size;
-
- mutex_lock(irq_domain_mutex);
- /* Verify that all the irqs are available */
- for (i = 0; i size; i++) {
- int irq = first_irq + i;
- struct irq_data *irq_data = irq_get_irq_data(irq);
-
- if (WARN_ON(!irq_data || irq_data-domain)) {
- mutex_unlock(irq_domain_mutex);
- irq_domain_free(domain);
- return NULL;
- }
- }
+ WARN_ON(irq_domain_associate_many(domain, first_irq, first_hwirq,
size));
- /* Claim all of the irqs before registering a legacy domain */
- for (i = 0; i size; i++) {
- struct irq_data *irq_data = irq_get_irq_data(first_irq + i);
- irq_data-hwirq = first_hwirq + i;
- irq_data-domain = domain;
- }
- mutex_unlock(irq_domain_mutex);
-
- for (i = 0; i size; i++) {
- int irq = first_irq
[RFC 03/10] irqdomain: Add a name field
This patch adds a name field to the irq_domain structure to help mere
mortals understand the mappings between irq domains and virqs. It also
converts a number of places that have open-coded some kind of fudging
an irqdomain name to use the new field. This means a more consistent
display of names in irq domain log messages and debugfs output.
Signed-off-by: Grant Likely grant.lik...@linaro.org
---
include/linux/irqdomain.h | 1 +
kernel/irq/generic-chip.c | 1 +
kernel/irq/irqdomain.c| 19 ++-
3 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 6f06241..e5e513c 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -89,6 +89,7 @@ struct irq_domain_chip_generic;
*/
struct irq_domain {
struct list_head link;
+ const char *name;
/* type of reverse mapping_technique */
unsigned int revmap_type;
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 95575d8..ca98cc5 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -305,6 +305,7 @@ int irq_alloc_domain_generic_chips(struct irq_domain *d,
int irqs_per_chip,
/* Calc pointer to the next generic chip */
tmp += sizeof(*gc) + num_ct * sizeof(struct irq_chip_type);
}
+ d-name = name;
return 0;
}
EXPORT_SYMBOL_GPL(irq_alloc_domain_generic_chips);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 1ac8cf4..b1b5e67 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -410,12 +410,15 @@ int irq_domain_associate_many(struct irq_domain *domain,
unsigned int irq_base,
*/
if (ret != -EPERM) {
pr_info(%s didn't like hwirq-0x%lx to
VIRQ%i mapping (rc=%d)\n,
-
of_node_full_name(domain-of_node), hwirq, virq, ret);
+ domain-name, hwirq, virq, ret);
}
irq_data-domain = NULL;
irq_data-hwirq = 0;
continue;
}
+ /* If not already assigned, give the domain the chip's
name */
+ if (!domain-name irq_data-chip)
+ domain-name = irq_data-chip-name;
}
switch (domain-revmap_type) {
@@ -708,8 +711,6 @@ static int virq_debug_show(struct seq_file *m, void
*private)
{
unsigned long flags;
struct irq_desc *desc;
- const char *p;
- static const char none[] = none;
void *data;
int i;
@@ -731,20 +732,12 @@ static int virq_debug_show(struct seq_file *m, void
*private)
seq_printf(m, 0x%05lx , desc-irq_data.hwirq);
chip = irq_desc_get_chip(desc);
- if (chip chip-name)
- p = chip-name;
- else
- p = none;
- seq_printf(m, %-15s , p);
+ seq_printf(m, %-15s , (chip chip-name) ?
chip-name : none);
data = irq_desc_get_chip_data(desc);
seq_printf(m, data ? 0x%p : %p , data);
- if (desc-irq_data.domain)
- p =
of_node_full_name(desc-irq_data.domain-of_node);
- else
- p = none;
- seq_printf(m, %s\n, p);
+ seq_printf(m, %s\n, desc-irq_data.domain-name);
}
raw_spin_unlock_irqrestore(desc-lock, flags);
--
1.8.1.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC 05/10] irqdomain: Eliminate revmap type
The NOMAP irq_domain type is only used by a handful of interrupt
controllers and it unnecessarily complicates the code by adding special
cases on how to look up mappings and different revmap functions are used
for each type which need to validate the correct type is passed to it
before performing the reverse map. Eliminating the revmap_type and
making a single reverse mapping function simplifies the code. It also
shouldn't be any slower than having separate revmap functions because
the type of the revmap needed to be checked anyway.
The linear and tree revmap types were already merged in a previous
patch. This patch rolls the NOMAP or direct mapping behaviour into the
same domain code making is possible for an irq domain to do any mapping
type; linear, tree or direct; and that the mapping will be transparent
to the interrupt controller driver.
With this change, direct mappings will get stored in the linear or tree
mapping for consistency. Reverse mapping from the hwirq to virq will go
through the normal lookup process. However, any controller using a
direct mapping can take advantage of knowing that hwirq==virq for any
mapped interrupts skip doing a revmap lookup when handling IRQs.
Signed-off-by: Grant Likely grant.lik...@linaro.org
---
include/linux/irqdomain.h | 48 +
kernel/irq/generic-chip.c | 5 +
kernel/irq/irqdomain.c| 55 +++
3 files changed, 43 insertions(+), 65 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 1cbb741..51ef84a 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -73,50 +73,42 @@ struct irq_domain_chip_generic;
/**
* struct irq_domain - Hardware interrupt number translation object
* @link: Element in global irq_domain list.
- * @revmap_type: Method used for reverse mapping hwirq numbers to linux irq.
This
- * will be one of the IRQ_DOMAIN_MAP_* values.
+ * @name: Name of interrupt domain
* @ops: pointer to irq_domain methods
* @host_data: private data pointer for use by owner. Not touched by
irq_domain
* core code.
- * @irq_base: Start of irq_desc range assigned to the irq_domain. The creator
- *of the irq_domain is responsible for allocating the array of
- *irq_desc structures.
- * @nr_irq: Number of irqs managed by the irq domain
- * @hwirq_base: Starting number for hwirqs managed by the irq domain
- * @of_node: (optional) Pointer to device tree nodes associated with the
- * irq_domain. Used when decoding device tree interrupt specifiers.
+ *
+ * Optional elements
+ * @of_node: Pointer to device tree nodes associated with the irq_domain. Used
+ * when decoding device tree interrupt specifiers.
+ * @gc: Pointer to a list of generic chips. There is a helper function for
+ * setting up one or more generic chips for interrupt controllers
+ * drivers using the generic chip library which uses this pointer.
+ *
+ * Revmap data, used internally by irq_domain
+ * @revmap_direct_max_irq: The largest hwirq that can be set for controllers
that
+ * support direct mapping
+ * @revmap_size: Size of the linear map table @linear_revmap[]
+ * @revmap_tree: Radix map tree for hwirqs that don't fit in the linear map
+ * @linear_revmap: Linear table of hwirq-virq reverse mappings
*/
struct irq_domain {
struct list_head link;
const char *name;
-
- /* type of reverse mapping_technique */
- unsigned int revmap_type;
- struct {
- struct {
- unsigned int size;
- } linear;
- struct {
- unsigned int max_irq;
- } nomap;
- struct radix_tree_root tree;
- } revmap_data;
const struct irq_domain_ops *ops;
void *host_data;
- irq_hw_number_t inval_irq;
- /* Optional device node pointer */
+ /* Optional data */
struct device_node *of_node;
- /* Optional pointer to generic interrupt chips */
struct irq_domain_chip_generic *gc;
- /* Linear reverse map */
+ /* reverse map data. The linear map gets appended to the irq_domain */
+ unsigned int revmap_direct_max_irq;
+ unsigned int revmap_size;
+ struct radix_tree_root revmap_tree;
unsigned int linear_revmap[];
};
-#define IRQ_DOMAIN_MAP_NOMAP 1 /* no fast reverse mapping */
-#define IRQ_DOMAIN_MAP_LINEAR 2 /* linear map of interrupts */
-
#ifdef CONFIG_IRQ_DOMAIN
struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
unsigned int size,
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index ca98cc5..4b01106 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -270,10 +270,7 @@ int irq_alloc_domain_generic_chips(struct irq_domain *d,
int
[RFC 04/10] irqdomain: merge linear and tree reverse mappings.
From: Grant Likely grant.lik...@secretlab.ca
Keeping them separate makes irq_domain more complex and adds a lot of
code (as proven by the diffstat). Merging them simplifies the whole
scheme. This change makes it so both the tree and linear methods can be
used by the same irq_domain instance. If the hwirq is less than the
-linear_size, then the linear map is used to reverse map the hwirq.
Otherwise the radix tree is used. The test for which map to use is no
more expensive that the existing code, so the performance of fast path
is preserved.
It also means that complex interrupt controllers can use both the
linear map and a tree in the same domain. This may be useful for an
interrupt controller with a base set of core irqs and a large number
of GPIOs which might be used as irqs. The linear map could cover the
core irqs, and the tree used for thas irqs. The linear map could
cover the core irqs, and the tree used for the gpios.
v2: Drop reorganization of revmap data
Signed-off-by: Grant Likely grant.lik...@secretlab.ca
Cc: Paul Mundt let...@linux-sh.org
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Cc: Thomas Gleixner t...@linutronix.de
Cc: Rob Herring rob.herr...@calxeda.com
---
include/linux/irqdomain.h | 18
kernel/irq/irqdomain.c| 107 +-
2 files changed, 39 insertions(+), 86 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index e5e513c..1cbb741 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -75,7 +75,6 @@ struct irq_domain_chip_generic;
* @link: Element in global irq_domain list.
* @revmap_type: Method used for reverse mapping hwirq numbers to linux irq.
This
* will be one of the IRQ_DOMAIN_MAP_* values.
- * @revmap_data: Revmap method specific data.
* @ops: pointer to irq_domain methods
* @host_data: private data pointer for use by owner. Not touched by
irq_domain
* core code.
@@ -93,10 +92,9 @@ struct irq_domain {
/* type of reverse mapping_technique */
unsigned int revmap_type;
- union {
+ struct {
struct {
unsigned int size;
- unsigned int *revmap;
} linear;
struct {
unsigned int max_irq;
@@ -111,11 +109,13 @@ struct irq_domain {
struct device_node *of_node;
/* Optional pointer to generic interrupt chips */
struct irq_domain_chip_generic *gc;
+
+ /* Linear reverse map */
+ unsigned int linear_revmap[];
};
#define IRQ_DOMAIN_MAP_NOMAP 1 /* no fast reverse mapping */
#define IRQ_DOMAIN_MAP_LINEAR 2 /* linear map of interrupts */
-#define IRQ_DOMAIN_MAP_TREE 3 /* radix tree */
#ifdef CONFIG_IRQ_DOMAIN
struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
@@ -137,10 +137,6 @@ struct irq_domain *irq_domain_add_nomap(struct device_node
*of_node,
unsigned int max_irq,
const struct irq_domain_ops *ops,
void *host_data);
-struct irq_domain *irq_domain_add_tree(struct device_node *of_node,
-const struct irq_domain_ops *ops,
-void *host_data);
-
extern struct irq_domain *irq_find_host(struct device_node *node);
extern void irq_set_default_host(struct irq_domain *host);
@@ -152,6 +148,12 @@ static inline struct irq_domain *irq_domain_add_legacy_isa(
return irq_domain_add_legacy(of_node, NUM_ISA_INTERRUPTS, 0, 0, ops,
host_data);
}
+static inline struct irq_domain *irq_domain_add_tree(struct device_node
*of_node,
+const struct irq_domain_ops *ops,
+void *host_data)
+{
+ return irq_domain_add_linear(of_node, 0, ops, host_data);
+}
extern void irq_domain_remove(struct irq_domain *host);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index b1b5e67..5a1d8ec 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -34,22 +34,24 @@ static struct irq_domain *irq_default_domain;
* to IRQ domain, or NULL on failure.
*/
static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
- unsigned int revmap_type,
+ unsigned int revmap_type, int size,
const struct irq_domain_ops *ops,
void *host_data)
{
struct irq_domain *domain;
- domain = kzalloc_node(sizeof(*domain), GFP_KERNEL,
- of_node_to_nid(of_node));
+ domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
+ GFP_KERNEL, of_node_to_nid(of_node
[RFC 07/10] irqdomain: Beef up debugfs output
This patch increases the amount of output produced by the
irq_domain_mapping debugfs file by first listing all of the registered
irq domains at the beginning of the output, and then by including all
mapped IRQs in the output, not just the active ones. It is very useful
when debugging irqdomain issues to be able to see the entire list of
mapped irqs, not just the ones that happen to be connected to devices.
Signed-off-by: Grant Likely grant.lik...@linaro.org
---
kernel/irq/irqdomain.c | 35 ++-
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index e0db59e..280b804 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -596,12 +596,29 @@ static int virq_debug_show(struct seq_file *m, void
*private)
{
unsigned long flags;
struct irq_desc *desc;
- void *data;
+ struct irq_domain *domain;
+ struct radix_tree_iter iter;
+ void *data, **slot;
int i;
- seq_printf(m, %-5s %-7s %-15s %-*s %s\n, irq, hwirq,
+ seq_printf(m, %-16s %-6s %-10s %-10s %s\n,
+ name, mapped, linear-max, direct-max,
devtree-node);
+ mutex_lock(irq_domain_mutex);
+ list_for_each_entry(domain, irq_domain_list, link) {
+ int count = 0;
+ radix_tree_for_each_slot(slot, domain-revmap_tree, iter, 0)
+ count++;
+ seq_printf(m, %c%-16s %6u %10u %10u %s\n,
+ domain == irq_default_domain ? '*' : ' ',
domain-name,
+ domain-revmap_size + count, domain-revmap_size,
+ domain-revmap_direct_max_irq,
+ domain-of_node ? of_node_full_name(domain-of_node)
: );
+ }
+ mutex_unlock(irq_domain_mutex);
+
+ seq_printf(m, %-5s %-7s %-15s %-*s %6s %-14s %s\n, irq,
hwirq,
chip name, (int)(2 * sizeof(void *) + 2), chip data,
- domain name);
+ active, type, domain);
for (i = 1; i nr_irqs; i++) {
desc = irq_to_desc(i);
@@ -609,12 +626,15 @@ static int virq_debug_show(struct seq_file *m, void
*private)
continue;
raw_spin_lock_irqsave(desc-lock, flags);
+ domain = desc-irq_data.domain;
- if (desc-action desc-action-handler) {
+ if (domain) {
struct irq_chip *chip;
+ int hwirq = desc-irq_data.hwirq;
+ bool direct;
seq_printf(m, %5d , i);
- seq_printf(m, 0x%05lx , desc-irq_data.hwirq);
+ seq_printf(m, 0x%05x , hwirq);
chip = irq_desc_get_chip(desc);
seq_printf(m, %-15s , (chip chip-name) ?
chip-name : none);
@@ -622,6 +642,11 @@ static int virq_debug_show(struct seq_file *m, void
*private)
data = irq_desc_get_chip_data(desc);
seq_printf(m, data ? 0x%p : %p , data);
+ seq_printf(m,%c, (desc-action
desc-action-handler) ? '*' : ' ');
+ direct = (i == hwirq) (i
domain-revmap_direct_max_irq);
+ seq_printf(m, %6s%-8s ,
+ (hwirq domain-revmap_size) ? LINEAR :
RADIX,
+ direct ? (DIRECT) : );
seq_printf(m, %s\n, desc-irq_data.domain-name);
}
--
1.8.1.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC 06/10] irqdomain: Clean up aftermath of irq_domain refactoring
After refactoring the irqdomain code, there are a number of API
functions that are merely empty wrappers around core code. Drop those
wrappers out of the C file and replace them with static inlines in the
header.
Signed-off-by: Grant Likely grant.lik...@linaro.org
---
arch/powerpc/platforms/cell/beat_interrupt.c | 2 +-
arch/powerpc/platforms/powermac/smp.c| 2 +-
include/linux/irqdomain.h| 31 +--
kernel/irq/irqdomain.c | 127 ---
4 files changed, 62 insertions(+), 100 deletions(-)
diff --git a/arch/powerpc/platforms/cell/beat_interrupt.c
b/arch/powerpc/platforms/cell/beat_interrupt.c
index 8c6dc42..9e5dfbc 100644
--- a/arch/powerpc/platforms/cell/beat_interrupt.c
+++ b/arch/powerpc/platforms/cell/beat_interrupt.c
@@ -239,7 +239,7 @@ void __init beatic_init_IRQ(void)
ppc_md.get_irq = beatic_get_irq;
/* Allocate an irq host */
- beatic_host = irq_domain_add_nomap(NULL, 0, beatic_pic_host_ops, NULL);
+ beatic_host = irq_domain_add_nomap(NULL, ~0, beatic_pic_host_ops,
NULL);
BUG_ON(beatic_host == NULL);
irq_set_default_host(beatic_host);
}
diff --git a/arch/powerpc/platforms/powermac/smp.c
b/arch/powerpc/platforms/powermac/smp.c
index bdb738a..f921067 100644
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -192,7 +192,7 @@ static int psurge_secondary_ipi_init(void)
{
int rc = -ENOMEM;
- psurge_host = irq_domain_add_nomap(NULL, 0, psurge_host_ops, NULL);
+ psurge_host = irq_domain_add_nomap(NULL, ~0, psurge_host_ops, NULL);
if (psurge_host)
psurge_secondary_virq = irq_create_direct_mapping(psurge_host);
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 51ef84a..fd4b26f 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -110,6 +110,10 @@ struct irq_domain {
};
#ifdef CONFIG_IRQ_DOMAIN
+struct irq_domain *__irq_domain_add(struct device_node *of_node,
+ int size, int direct_max,
+ const struct irq_domain_ops *ops,
+ void *host_data);
struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
unsigned int size,
unsigned int first_irq,
@@ -121,17 +125,30 @@ struct irq_domain *irq_domain_add_legacy(struct
device_node *of_node,
irq_hw_number_t first_hwirq,
const struct irq_domain_ops *ops,
void *host_data);
-struct irq_domain *irq_domain_add_linear(struct device_node *of_node,
+extern struct irq_domain *irq_find_host(struct device_node *node);
+extern void irq_set_default_host(struct irq_domain *host);
+
+/**
+ * irq_domain_add_linear() - Allocate and register a linear revmap irq_domain.
+ * @of_node: pointer to interrupt controller's device tree node.
+ * @size: Number of interrupts in the domain.
+ * @ops: map/unmap domain callbacks
+ * @host_data: Controller private data pointer
+ */
+static inline struct irq_domain *irq_domain_add_linear(struct device_node
*of_node,
unsigned int size,
const struct irq_domain_ops *ops,
-void *host_data);
-struct irq_domain *irq_domain_add_nomap(struct device_node *of_node,
+void *host_data)
+{
+ return __irq_domain_add(of_node, size, 0, ops, host_data);
+}
+static inline struct irq_domain *irq_domain_add_nomap(struct device_node
*of_node,
unsigned int max_irq,
const struct irq_domain_ops *ops,
-void *host_data);
-extern struct irq_domain *irq_find_host(struct device_node *node);
-extern void irq_set_default_host(struct irq_domain *host);
-
+void *host_data)
+{
+ return __irq_domain_add(of_node, 0, max_irq, ops, host_data);
+}
static inline struct irq_domain *irq_domain_add_legacy_isa(
struct device_node *of_node,
const struct irq_domain_ops *ops,
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index c38be78..e0db59e 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -23,8 +23,11 @@ static DEFINE_MUTEX(revmap_trees_mutex);
static struct irq_domain *irq_default_domain;
/**
- * irq_domain_alloc() - Allocate a new irq_domain data structure
+ * __irq_domain_add() - Allocate a new irq_domain data structure
* @of_node: optional device-tree node of the interrupt controller
+ * @size: Size of linear map; 0 for radix mapping only
+ * @direct_max: Maximum value of direct
[RFC 08/10] irqdomain: Refactor irq_domain_associate_many()
Originally, irq_domain_associate_many() was designed to unwind the
mapped irqs on a failure of any individual association. However, that
proved to be a problem with certain IRQ controllers. Some of them only
support a subset of irqs, and will fail when attempting to map a
reserved IRQ. In those cases we want to map as many IRQs as possible, so
instead it is better for irq_domain_associate_many() to make a
best-effort attempt to map irqs, but not fail if any or all of them
don't succeed. If a caller really cares about how many irqs got
associated, then it should instead go back and check that all of the
irqs is cares about were mapped.
The original design open-coded the individual association code into the
body of irq_domain_associate_many(), but with no longer needing to
unwind associations, the code becomes simpler to split out
irq_domain_associate() to contain the bulk of the logic, and
irq_domain_associate_many() to be a simple loop wrapper.
This patch also adds a new error check to the associate path to make
sure it isn't called for an irq larger than the controller can handle,
and adds locking so that the irq_domain_mutex is held while setting up a
new association.
Signed-off-by: Grant Likely grant.lik...@linaro.org
---
include/linux/irqdomain.h | 22 +++---
kernel/irq/irqdomain.c| 185 +++---
2 files changed, 101 insertions(+), 106 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index fd4b26f..f9e8e06 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -103,6 +103,7 @@ struct irq_domain {
struct irq_domain_chip_generic *gc;
/* reverse map data. The linear map gets appended to the irq_domain */
+ irq_hw_number_t hwirq_max;
unsigned int revmap_direct_max_irq;
unsigned int revmap_size;
struct radix_tree_root revmap_tree;
@@ -110,8 +111,8 @@ struct irq_domain {
};
#ifdef CONFIG_IRQ_DOMAIN
-struct irq_domain *__irq_domain_add(struct device_node *of_node,
- int size, int direct_max,
+struct irq_domain *__irq_domain_add(struct device_node *of_node, int size,
+ irq_hw_number_t hwirq_max, int direct_max,
const struct irq_domain_ops *ops,
void *host_data);
struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
@@ -140,14 +141,14 @@ static inline struct irq_domain
*irq_domain_add_linear(struct device_node *of_no
const struct irq_domain_ops *ops,
void *host_data)
{
- return __irq_domain_add(of_node, size, 0, ops, host_data);
+ return __irq_domain_add(of_node, size, size, 0, ops, host_data);
}
static inline struct irq_domain *irq_domain_add_nomap(struct device_node
*of_node,
unsigned int max_irq,
const struct irq_domain_ops *ops,
void *host_data)
{
- return __irq_domain_add(of_node, 0, max_irq, ops, host_data);
+ return __irq_domain_add(of_node, 0, max_irq, max_irq, ops, host_data);
}
static inline struct irq_domain *irq_domain_add_legacy_isa(
struct device_node *of_node,
@@ -166,14 +167,11 @@ static inline struct irq_domain
*irq_domain_add_tree(struct device_node *of_node
extern void irq_domain_remove(struct irq_domain *host);
-extern int irq_domain_associate_many(struct irq_domain *domain,
-unsigned int irq_base,
-irq_hw_number_t hwirq_base, int count);
-static inline int irq_domain_associate(struct irq_domain *domain, unsigned int
irq,
- irq_hw_number_t hwirq)
-{
- return irq_domain_associate_many(domain, irq, hwirq, 1);
-}
+extern int irq_domain_associate(struct irq_domain *domain, unsigned int irq,
+ irq_hw_number_t hwirq);
+extern void irq_domain_associate_many(struct irq_domain *domain,
+ unsigned int irq_base,
+ irq_hw_number_t hwirq_base, int count);
extern unsigned int irq_create_mapping(struct irq_domain *host,
irq_hw_number_t hwirq);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 280b804..80e9249 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -35,8 +35,8 @@ static struct irq_domain *irq_default_domain;
* register allocated irq_domain with irq_domain_register(). Returns pointer
* to IRQ domain, or NULL on failure.
*/
-struct irq_domain *__irq_domain_add(struct device_node *of_node,
- int size, int direct_max,
+struct irq_domain *__irq_domain_add(struct device_node *of_node, int
[RFC 09/10] irqdomain: remove irq_domain_generate_simple()
Nobody calls it; remove the function
Signed-off-by: Grant Likely grant.lik...@linaro.org
---
include/linux/irqdomain.h | 8
kernel/irq/irqdomain.c| 15 ---
2 files changed, 23 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index f9e8e06..fe7c57d 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -205,14 +205,6 @@ int irq_domain_xlate_onetwocell(struct irq_domain *d,
struct device_node *ctrlr,
const u32 *intspec, unsigned int intsize,
irq_hw_number_t *out_hwirq, unsigned int *out_type);
-#if defined(CONFIG_OF_IRQ)
-extern void irq_domain_generate_simple(const struct of_device_id *match,
- u64 phys_base, unsigned int irq_start);
-#else /* CONFIG_OF_IRQ */
-static inline void irq_domain_generate_simple(const struct of_device_id *match,
- u64 phys_base, unsigned int irq_start)
{ }
-#endif /* !CONFIG_OF_IRQ */
-
#else /* CONFIG_IRQ_DOMAIN */
static inline void irq_dispose_mapping(unsigned int virq) { }
#endif /* !CONFIG_IRQ_DOMAIN */
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 80e9249..e47b356 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -741,18 +741,3 @@ const struct irq_domain_ops irq_domain_simple_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
EXPORT_SYMBOL_GPL(irq_domain_simple_ops);
-
-#ifdef CONFIG_OF_IRQ
-void irq_domain_generate_simple(const struct of_device_id *match,
- u64 phys_base, unsigned int irq_start)
-{
- struct device_node *node;
- pr_debug(looking for phys_base=%llx, irq_start=%i\n,
- (unsigned long long) phys_base, (int) irq_start);
- node = of_find_matching_node_by_address(NULL, match, phys_base);
- if (node)
- irq_domain_add_legacy(node, 32, irq_start, 0,
- irq_domain_simple_ops, NULL);
-}
-EXPORT_SYMBOL_GPL(irq_domain_generate_simple);
-#endif
--
1.8.1.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC 10/10] irqchip: Make versatile fpga irq driver a generic chip
This is an RFC patch to convert the versatile FPGA irq controller driver
to use generic irq chip. It builds on the series that extends the
generic chip code to allow a linear irq domain to contain one or more
generic irq chips so that each interrupt controller doesn't need to hand
code the generic chip setup.
I've written this as a proof of concept to see if the new generic irq
code does what it needs to. I had to extend it slightly to properly
handle the valid mask used by the versatile FPGA driver.
Tested on QEMU, but not on real hardware.
Signed-off-by: Grant Likely grant.lik...@linaro.org
Cc: Russell King li...@arm.linux.org.uk
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Thomas Gleixner t...@linutronix.de
---
drivers/irqchip/Kconfig | 1 +
drivers/irqchip/irq-versatile-fpga.c | 104 +--
2 files changed, 39 insertions(+), 66 deletions(-)
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 4a33351..8765502 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -14,6 +14,7 @@ config ARM_VIC
bool
select IRQ_DOMAIN
select MULTI_IRQ_HANDLER
+ select GENERIC_IRQ_CHIP
config ARM_VIC_NR
int
diff --git a/drivers/irqchip/irq-versatile-fpga.c
b/drivers/irqchip/irq-versatile-fpga.c
index 47a52ab..8c7097b 100644
--- a/drivers/irqchip/irq-versatile-fpga.c
+++ b/drivers/irqchip/irq-versatile-fpga.c
@@ -34,37 +34,18 @@
* @used_irqs: number of active IRQs on this controller
*/
struct fpga_irq_data {
- void __iomem *base;
- struct irq_chip chip;
- u32 valid;
struct irq_domain *domain;
- u8 used_irqs;
};
/* we cannot allocate memory when the controllers are initially registered */
static struct fpga_irq_data fpga_irq_devices[CONFIG_VERSATILE_FPGA_IRQ_NR];
static int fpga_irq_id;
-static void fpga_irq_mask(struct irq_data *d)
-{
- struct fpga_irq_data *f = irq_data_get_irq_chip_data(d);
- u32 mask = 1 d-hwirq;
-
- writel(mask, f-base + IRQ_ENABLE_CLEAR);
-}
-
-static void fpga_irq_unmask(struct irq_data *d)
-{
- struct fpga_irq_data *f = irq_data_get_irq_chip_data(d);
- u32 mask = 1 d-hwirq;
-
- writel(mask, f-base + IRQ_ENABLE_SET);
-}
-
static void fpga_irq_handle(unsigned int irq, struct irq_desc *desc)
{
- struct fpga_irq_data *f = irq_desc_get_handler_data(desc);
- u32 status = readl(f-base + IRQ_STATUS);
+ struct irq_domain *domain = irq_desc_get_handler_data(desc);
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
+ u32 status = readl(gc-reg_base + IRQ_STATUS);
if (status == 0) {
do_bad_IRQ(irq, desc);
@@ -74,7 +55,7 @@ static void fpga_irq_handle(unsigned int irq, struct irq_desc
*desc)
do {
irq = ffs(status) - 1;
status = ~(1 irq);
- generic_handle_irq(irq_find_mapping(f-domain, irq));
+ generic_handle_irq(irq_find_mapping(domain, irq));
} while (status);
}
@@ -85,11 +66,12 @@ static void fpga_irq_handle(unsigned int irq, struct
irq_desc *desc)
*/
static int handle_one_fpga(struct fpga_irq_data *f, struct pt_regs *regs)
{
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(f-domain, 0);
int handled = 0;
int irq;
u32 status;
- while ((status = readl(f-base + IRQ_STATUS))) {
+ while ((status = readl(gc-reg_base + IRQ_STATUS))) {
irq = ffs(status) - 1;
handle_IRQ(irq_find_mapping(f-domain, irq), regs);
handled = 1;
@@ -112,63 +94,53 @@ asmlinkage void __exception_irq_entry
fpga_handle_irq(struct pt_regs *regs)
} while (handled);
}
-static int fpga_irqdomain_map(struct irq_domain *d, unsigned int irq,
- irq_hw_number_t hwirq)
-{
- struct fpga_irq_data *f = d-host_data;
-
- /* Skip invalid IRQs, only register handlers for the real ones */
- if (!(f-valid BIT(hwirq)))
- return -EPERM;
- irq_set_chip_data(irq, f);
- irq_set_chip_and_handler(irq, f-chip,
- handle_level_irq);
- set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
- return 0;
-}
-
-static struct irq_domain_ops fpga_irqdomain_ops = {
- .map = fpga_irqdomain_map,
- .xlate = irq_domain_xlate_onetwocell,
-};
-
void __init fpga_irq_init(void __iomem *base, const char *name, int irq_start,
int parent_irq, u32 valid, struct device_node *node)
{
+ struct irq_chip_generic *gc;
struct fpga_irq_data *f;
- int i;
+ int ret;
if (fpga_irq_id = ARRAY_SIZE(fpga_irq_devices)) {
pr_err(%s: too few FPGA IRQ controllers, increase
CONFIG_VERSATILE_FPGA_IRQ_NR\n, __func__);
return;
}
f = fpga_irq_devices[fpga_irq_id];
- f-base = base;
- f-chip.name = name;
- f
Re: [RFC 10/10] irqchip: Make versatile fpga irq driver a generic chip
On Mon, Jun 10, 2013 at 10:03 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Mon, Jun 10, 2013 at 01:49:22AM +0100, Grant Likely wrote: This is an RFC patch to convert the versatile FPGA irq controller driver to use generic irq chip. It builds on the series that extends the generic chip code to allow a linear irq domain to contain one or more generic irq chips so that each interrupt controller doesn't need to hand code the generic chip setup. NAK, this makes functional changes. You assume that the validity mask is a set of zeros followed by a set of ones. This is not always the case. The PIC on Integrator/CP only has bits 29-22 and 11-0 set because 21-12 are not valid. Actually, I don't (at least I don't intend to). It chooses the size of the irq_domain based on fls(), but that is only because the size has to allocates for both the holes and the active irqs. You'll notice that gc-unused is set to ~valid, and the generic chip code will refuse to map in interrupt that isn't supported. If you see something different, please point out where because that would be a bug. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC 10/10] irqchip: Make versatile fpga irq driver a generic chip
On Mon, Jun 10, 2013 at 8:40 AM, Linus Walleij linus.wall...@linaro.org wrote: On Mon, Jun 10, 2013 at 2:49 AM, Grant Likely grant.lik...@linaro.org wrote: This is an RFC patch to convert the versatile FPGA irq controller driver to use generic irq chip. It builds on the series that extends the generic chip code to allow a linear irq domain to contain one or more generic irq chips so that each interrupt controller doesn't need to hand code the generic chip setup. I've written this as a proof of concept to see if the new generic irq code does what it needs to. I had to extend it slightly to properly handle the valid mask used by the versatile FPGA driver. Tested on QEMU, but not on real hardware. Is this the same as the one I tested previously? If it need re-testing please push a branch and I'll take it for a spin. Yes, it's the same, but if you can test the branch I would appreciate it. I've done some heavy rework on the irqdomain code that makes everything simpler, but also makes it likely that I've broken something. git://git.secretlab.ca/git/linux.git irqdomain/test g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC 00/10] Refactor irqdomain
I've done a bunch of refactoring work on the irq_domain infrastructure. Some of these patches I've posted before, and some our brand new. The goal of this is to greatly simplify how irq_domains work. With this series, instead of there being multiple different types of irq domains, each with different mapping rules, instead there is now only one time of irq_domain that contains both kinds of map; the linear map for irqs below a certain value, and the radix tree for large sparse irq controllers. As you can see from the following diffstat, the result is a fair bit less code. It should make it easier to understand irqdomains too. arch/powerpc/platforms/cell/beat_interrupt.c | 2 +- arch/powerpc/platforms/powermac/smp.c| 2 +- drivers/irqchip/Kconfig | 1 + drivers/irqchip/irq-versatile-fpga.c | 104 -- include/linux/irqdomain.h| 123 ++- kernel/irq/generic-chip.c| 6 +- kernel/irq/irqdomain.c | 555 -- 7 files changed, 282 insertions(+), 511 deletions(-) I've pushed this series out to my git server at the following branch: git://git.secretlab.ca/git/linux irqdomain/next It depends on the tip tree's irq/for-arm branch and also Linus' mainline (they need to be merged). The branch above includes both. I've tested this on ARM qemu models, but not much else. I'll test on real hardware before pushing out, but I would appreciate anybody doing additional testing, particularly on PowerPC and other non-ARM platforms. Cheers, g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3] irq: Set multiple MSI descriptor data for multiple IRQs
On Tue, 15 Jan 2013 15:38:54 +0800, Mike Qiu qiud...@linux.vnet.ibm.com wrote:
Multiple MSI only requires the IRQ in msi_desc entry to be set as
the value of irq_base.
This patch implements the above mentioned technique.
Signed-off-by: Mike Qiu qiud...@linux.vnet.ibm.com
Hi Mike,
question below...
---
+int irq_set_multiple_msi_desc(unsigned int irq_base, unsigned int nvec,
+ struct msi_desc *entry)
+{
+ unsigned long flags, i;
+ struct irq_desc *desc;
+
+ for (i = 0; i nvec; i++) {
+ desc = irq_get_desc_lock(irq_base + i, flags,
+ IRQ_GET_DESC_CHECK_GLOBAL);
+ if (!desc)
+ return -EINVAL;
+ desc-irq_data.msi_desc = entry;
+ if (i == 0 entry)
+ entry-irq = irq_base;
It's not clear to me why this code only sets the irq value for the first
desc. Why don't the other descs in the array want (irq_base + i) here? A
comment describing what is going on would be appropriate and helpful.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: DTB build failure due to preproccessing
On Fri, 31 May 2013 11:29:30 +0100, Ian Campbell ian.campb...@citrix.com wrote: This affects arch/powerpc/boot/dts/virtex440-ml510.dts but I think it is actually a more general issue: $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux- virtex440-ml510.dtb CC scripts/mod/devicetable-offsets.s GEN scripts/mod/devicetable-offsets.h HOSTCC scripts/mod/file2alias.o HOSTLD scripts/mod/modpost DTC arch/powerpc/boot/virtex440-ml510.dtb Error: arch/powerpc/boot/dts/virtex440-ml510.dts:374.6-7 syntax error FATAL ERROR: Unable to parse input tree make[1]: *** [arch/powerpc/boot/virtex440-ml510.dtb] Error 1 make: *** [virtex440-ml510.dtb] Error 2 Line 374 is the IDSEL 0x16... line here: interrupt-map = /* IRQ mapping for pci slots and ALI M1533 ... * management core also isn't used. */ /* IDSEL 0x16 / dev=6, bus=0 / PCI slot 3 */ 0x3000 0 0 1 xps_intc_0 3 2 0x3000 0 0 2 xps_intc_0 2 2 0x3000 0 0 3 xps_intc_0 5 2 0x3000 0 0 4 xps_intc_0 4 2 Which gets preprocessed into: interrupt-map = # 375 arch/powerpc/boot/dts/virtex440-ml510.dts 0x3000 0 0 1 xps_intc_0 3 2 0x3000 0 0 2 xps_intc_0 2 2 0x3000 0 0 3 xps_intc_0 5 2 0x3000 0 0 4 xps_intc_0 4 2 If I manually remove the # 375 line then that fixes the error (although there is then a subsequent one of the same type). I suppose this is a bug in dtc? It appears to have at least some awareness of these preprocessor line number comments since it manages to report the original source line number. dtc is only able to track line numbers when the native /include/ directive is used. The #include directive doesn't help it. It should be added, but until it is the following patch solves the problem: I've got this patch in my tree. Either Rob or I will push it to Linus in the next few days. g. --- commit d01dccdcb3ea8233b09efb9c24db9f057fbd3b37 Author: Grant Likely grant.lik...@linaro.org Date: Fri May 31 12:45:18 2013 +0100 dtc: Suppress cpp linemarker annotations DTC isn't able to parse cpp linemarker annotations, so suppress them in the cpp output by adding the -P flag to the cpp options. Signed-off-by: Grant Likely grant.lik...@linaro.org diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 51bb3de..fc08a2b 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -149,7 +149,7 @@ cpp_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ ld_flags = $(LDFLAGS) $(ldflags-y) -dtc_cpp_flags = -Wp,-MD,$(depfile).pre -nostdinc\ +dtc_cpp_flags = -Wp,-MD,$(depfile).pre -nostdinc -P \ -I$(srctree)/arch/$(SRCARCH)/boot/dts \ -I$(srctree)/arch/$(SRCARCH)/boot/dts/include \ -undef -D__DTS__ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: DTB build failure due to preproccessing
On Fri, May 31, 2013 at 5:04 PM, Stephen Warren swar...@wwwdotorg.org wrote: On 05/31/2013 05:48 AM, Grant Likely wrote: On Fri, 31 May 2013 11:29:30 +0100, Ian Campbell ian.campb...@citrix.com wrote: This affects arch/powerpc/boot/dts/virtex440-ml510.dts but I think it is actually a more general issue: $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux- virtex440-ml510.dtb CC scripts/mod/devicetable-offsets.s GEN scripts/mod/devicetable-offsets.h HOSTCC scripts/mod/file2alias.o HOSTLD scripts/mod/modpost DTC arch/powerpc/boot/virtex440-ml510.dtb Error: arch/powerpc/boot/dts/virtex440-ml510.dts:374.6-7 syntax error FATAL ERROR: Unable to parse input tree make[1]: *** [arch/powerpc/boot/virtex440-ml510.dtb] Error 1 make: *** [virtex440-ml510.dtb] Error 2 Line 374 is the IDSEL 0x16... line here: interrupt-map = /* IRQ mapping for pci slots and ALI M1533 ... * management core also isn't used. */ /* IDSEL 0x16 / dev=6, bus=0 / PCI slot 3 */ 0x3000 0 0 1 xps_intc_0 3 2 0x3000 0 0 2 xps_intc_0 2 2 0x3000 0 0 3 xps_intc_0 5 2 0x3000 0 0 4 xps_intc_0 4 2 Which gets preprocessed into: interrupt-map = # 375 arch/powerpc/boot/dts/virtex440-ml510.dts 0x3000 0 0 1 xps_intc_0 3 2 0x3000 0 0 2 xps_intc_0 2 2 0x3000 0 0 3 xps_intc_0 5 2 0x3000 0 0 4 xps_intc_0 4 2 If I manually remove the # 375 line then that fixes the error (although there is then a subsequent one of the same type). I suppose this is a bug in dtc? It appears to have at least some awareness of these preprocessor line number comments since it manages to report the original source line number. dtc is only able to track line numbers when the native /include/ directive is used. The #include directive doesn't help it. It should be added, but until it is the following patch solves the problem: I've got this patch in my tree. Either Rob or I will push it to Linus in the next few days. g. --- commit d01dccdcb3ea8233b09efb9c24db9f057fbd3b37 Author: Grant Likely grant.lik...@linaro.org Date: Fri May 31 12:45:18 2013 +0100 dtc: Suppress cpp linemarker annotations DTC isn't able to parse cpp linemarker annotations, so suppress them in the cpp output by adding the -P flag to the cpp options. That's not true; it explicitly does have code to parse the line markers. I'll have to investigate why it isn't working in this case. If you apply this patch, then anyone who has switched to #include rther than /include/ will get incorrect line numbers in dtc error messages. Admittedly that's a smaller population right now though. Perhaps we should just do a kernel-wide conversion though. My mistake. I tested the wrong thing. I've dropped the patch. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] irqdomain: Allow quiet failure mode
On Mon, May 6, 2013 at 7:41 AM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: Some interrupt controllers refuse to map interrupts marked as protected by firwmare. Since we try to map everyting in the device-tree on some platforms, we end up with a lot of nasty WARN's in the boot log for what is a normal situation on those machines. This defines a specific return code (-EPERM) from the host map() callback which cause irqdomain to fail silently. MPIC is updated to return this when hitting a protected source printing only a single line message for diagnostic purposes. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org Grant, I'm happy to merge that myself provided I have your ack, it fixes an annoying problem on Cell introduced by previous changes to irqdomain (not recent but users started to notice). Yes, please merge it since powerpc needs it. Acked-by: Grant Likely grant.lik...@linaro.org g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v7 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
On Wed, 17 Apr 2013 12:22:23 -0400, Jason Cooper ja...@lakedaemon.net wrote: On Wed, Apr 17, 2013 at 05:17:48PM +0100, Grant Likely wrote: On Wed, Apr 17, 2013 at 5:10 PM, Jason Cooper ja...@lakedaemon.net wrote: On Wed, Apr 17, 2013 at 05:00:15PM +0100, Grant Likely wrote: On Tue, 16 Apr 2013 11:30:06 +0100, Andrew Murray andrew.mur...@arm.com wrote: On Tue, Apr 16, 2013 at 11:18:26AM +0100, Andrew Murray wrote: The pci_process_bridge_OF_ranges function, used to parse the ranges property of a PCI host device, is found in both Microblaze and PowerPC architectures. These implementations are nearly identical. This patch moves this common code to a common place. Signed-off-by: Andrew Murray andrew.mur...@arm.com Signed-off-by: Liviu Dudau liviu.du...@arm.com Reviewed-by: Rob Herring rob.herr...@calxeda.com Tested-by: Thomas Petazzoni thomas.petazz...@free-electrons.com Tested-by: Linus Walleij linus.wall...@linaro.org Acked-by: Michal Simek mon...@monstr.eu --- arch/microblaze/include/asm/pci-bridge.h |5 +- arch/microblaze/pci/pci-common.c | 192 arch/powerpc/include/asm/pci-bridge.h|5 +- arch/powerpc/kernel/pci-common.c | 192 Is there anyone on linuxppc-dev/linux-mips that can help test this patchset? I've tested that it builds on powerpc with a variety of configs (some which include fsl_pci.c implementation). Though I don't have hardware to verify that it works. I haven't tested this builds or runs on MIPS. You shouldn't see any difference in behaviour or new warnings and PCI devices should continue to operate as before. I've got through a line-by-line comparison between powerpc, microblaze, and then new code. The differences are purely cosmetic, so I have absolutely no concerns about this patch. I've applied it to my tree. oops. Due to the number of dependencies the mvebu-pcie series has (this being one of them, we (arm-soc/mvebu) asked if we could take this through our tree. Rob Herring agreed to this several days ago. Is this a problem for you? It would truly (dogs and cats living together) upset the apple cart for us at this stage to pipe these through a different tree... Not a problem at all. I'll drop it. Great! Thanks. You can add my Acked-by: Grant Likely grant.lik...@linaro.org to the first patch. I've not reviewed out the second or third patches yet. None of this appears to be in linux-next yet. I've boot tested on PowerPC, but that isn't the same as an ack by the PowerPC maintainers. It is probably too late for the whole now since we're after -rc7. However, if you ask me to, I have absolutely no problem putting the first patch into my tree for v3.10 merging. I went over the patch line-by-line and am convinced that it is functionally identical. Let me know if you need me to do this. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v7 2/3] of/pci: Provide support for parsing PCI DT ranges property
On Tue, 16 Apr 2013 11:18:27 +0100, Andrew Murray andrew.mur...@arm.com wrote:
This patch factors out common implementation patterns to reduce overall kernel
code and provide a means for host bridge drivers to directly obtain struct
resources from the DT's ranges property without relying on architecture
specific
DT handling. This will make it easier to write archiecture independent host
bridge
drivers and mitigate against further duplication of DT parsing code.
This patch can be used in the following way:
struct of_pci_range_parser parser;
struct of_pci_range range;
if (of_pci_range_parser(parser, np))
; //no ranges property
for_each_of_pci_range(parser, range) {
/*
directly access properties of the address range, e.g.:
range.pci_space, range.pci_addr, range.cpu_addr,
range.size, range.flags
alternatively obtain a struct resource, e.g.:
struct resource res;
of_pci_range_to_resource(range, np, res);
*/
}
Additionally the implementation takes care of adjacent ranges and merges them
into a single range (as was the case with powerpc and microblaze).
Signed-off-by: Andrew Murray andrew.mur...@arm.com
Signed-off-by: Liviu Dudau liviu.du...@arm.com
Signed-off-by: Thomas Petazzoni thomas.petazz...@free-electrons.com
Reviewed-by: Rob Herring rob.herr...@calxeda.com
Tested-by: Thomas Petazzoni thomas.petazz...@free-electrons.com
Tested-by: Linus Walleij linus.wall...@linaro.org
Acked-by: Grant Likely grant.lik...@secretlab.ca
But comments below...
---
drivers/of/address.c | 67 ++
drivers/of/of_pci.c| 113
include/linux/of_address.h | 46 ++
3 files changed, 154 insertions(+), 72 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 04da786..6eec70c 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -227,6 +227,73 @@ int of_pci_address_to_resource(struct device_node *dev,
int bar,
return __of_address_to_resource(dev, addrp, size, flags, NULL, r);
}
EXPORT_SYMBOL_GPL(of_pci_address_to_resource);
+
+int of_pci_range_parser(struct of_pci_range_parser *parser,
+ struct device_node *node)
+{
+ const int na = 3, ns = 2;
+ int rlen;
+
+ parser-node = node;
+ parser-pna = of_n_addr_cells(node);
+ parser-np = parser-pna + na + ns;
+
+ parser-range = of_get_property(node, ranges, rlen);
+ if (parser-range == NULL)
+ return -ENOENT;
+
+ parser-end = parser-range + rlen / sizeof(__be32);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_pci_range_parser);
of_pci_range_parser_init would be a clearer name
+struct of_pci_range *of_pci_process_ranges(struct of_pci_range_parser
*parser,
+ struct of_pci_range *range)
Similarly, of_pci_range_parser_one would be more consistent.
+{
+ const int na = 3, ns = 2;
+
+ if (!range)
+ return NULL;
+
+ if (!parser-range || parser-range + parser-np parser-end)
+ return NULL;
+
+ range-pci_space = parser-range[0];
+ range-flags = of_bus_pci_get_flags(parser-range);
+ range-pci_addr = of_read_number(parser-range + 1, ns);
+ range-cpu_addr = of_translate_address(parser-node,
+ parser-range + na);
+ range-size = of_read_number(parser-range + parser-pna + na, ns);
+
+ parser-range += parser-np;
+
+ /* Now consume following elements while they are contiguous */
+ while (parser-range + parser-np = parser-end) {
+ u32 flags, pci_space;
+ u64 pci_addr, cpu_addr, size;
+
+ pci_space = be32_to_cpup(parser-range);
+ flags = of_bus_pci_get_flags(parser-range);
+ pci_addr = of_read_number(parser-range + 1, ns);
+ cpu_addr = of_translate_address(parser-node,
+ parser-range + na);
+ size = of_read_number(parser-range + parser-pna + na, ns);
+
+ if (flags != range-flags)
+ break;
+ if (pci_addr != range-pci_addr + range-size ||
+ cpu_addr != range-cpu_addr + range-size)
+ break;
+
+ range-size += size;
+ parser-range += parser-np;
+ }
+
+ return range;
+}
+EXPORT_SYMBOL_GPL(of_pci_process_ranges);
+
#endif /* CONFIG_PCI */
/*
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 1626172..e5ab604 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -2,6 +2,7 @@
#include linux/export.h
#include linux/of.h
#include linux/of_pci.h
+#include linux/of_address.h
#include asm/prom.h
Re: [PATCH v7 3/3] of/pci: mips: convert to common of_pci_range_parser
On Tue, 16 Apr 2013 11:18:28 +0100, Andrew Murray andrew.mur...@arm.com wrote: This patch converts the pci_load_of_ranges function to use the new common of_pci_range_parser. Signed-off-by: Andrew Murray andrew.mur...@arm.com Signed-off-by: Liviu Dudau liviu.du...@arm.com Reviewed-by: Rob Herring rob.herr...@calxeda.com Looks okay to me, and thank you very much for doing the legwork to merge the common code. Reviewed-by: Grant Likely grant.lik...@secretlab.ca ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
