Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1716587 > unrolled thread
| Started by | Rob Herring <robh@kernel.org> |
|---|---|
| First post | 2017-08-21 17:20 +0200 |
| Last post | 2017-08-21 17:20 +0200 |
| Articles | 9 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/5] Removing full paths from DT full_name Rob Herring <robh@kernel.org> - 2017-08-21 17:20 +0200
[PATCH v2 5/5] of/fdt: only store the device node basename in full_name Rob Herring <robh@kernel.org> - 2017-08-21 17:20 +0200
[PATCH v2 2/5] powerpc: pseries: vio: match parent nodes with of_find_node_by_path Rob Herring <robh@kernel.org> - 2017-08-21 17:20 +0200
Re: [PATCH v2 2/5] powerpc: pseries: vio: match parent nodes with of_find_node_by_path Michael Ellerman <mpe@ellerman.id.au> - 2017-08-22 07:20 +0200
Re: [PATCH v2 2/5] powerpc: pseries: vio: match parent nodes with of_find_node_by_path Rob Herring <robh@kernel.org> - 2017-08-24 20:10 +0200
Re: [PATCH v2 2/5] powerpc: pseries: vio: match parent nodes with of_find_node_by_path Michael Ellerman <mpe@ellerman.id.au> - 2017-08-25 02:20 +0200
[PATCH v2 3/5] powerpc: pseries: remove dlpar_attach_node dependency on full path Rob Herring <robh@kernel.org> - 2017-08-21 17:20 +0200
Re: [v2, 3/5] powerpc: pseries: remove dlpar_attach_node dependency on full path Michael Ellerman <patch-notifications@ellerman.id.au> - 2017-08-24 14:40 +0200
[PATCH v2 4/5] powerpc: pseries: only store the device node basename in full_name Rob Herring <robh@kernel.org> - 2017-08-21 17:20 +0200
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2017-08-21 17:20 +0200 |
| Subject | [PATCH v2 0/5] Removing full paths from DT full_name |
| Message-ID | <ugWEF-60L-3@gated-at.bofh.it> |
This series is the last steps to remove storing the full path for every DT node. Instead, we can create full path strings dynamically as needed with printf %pOF specifiers (commit ce4fecf1fe15). There are a number of remaining direct users of full_name after this series. I don't believe there should be any functional impact for those users with the change to only the node name (+unit-address). The majority are for struct resource.name. This should only affect /proc/iomem display. Michael, Please apply patches 1-3 for 4.14. Patches 4 and 5 are intended for 4.15. Testing of those in particular would be helpful. Rob Rob Herring (5): powerpc: Convert to using %pOF instead of full_name powerpc: pseries: vio: match parent nodes with of_find_node_by_path powerpc: pseries: remove dlpar_attach_node dependency on full path powerpc: pseries: only store the device node basename in full_name of/fdt: only store the device node basename in full_name arch/powerpc/kernel/btext.c | 2 +- arch/powerpc/kernel/cacheinfo.c | 34 ++++--- arch/powerpc/kernel/io-workarounds.c | 4 +- arch/powerpc/kernel/isa-bridge.c | 32 +++---- arch/powerpc/kernel/legacy_serial.c | 12 +-- arch/powerpc/kernel/of_platform.c | 2 +- arch/powerpc/kernel/pci-common.c | 15 ++- arch/powerpc/kernel/pci_32.c | 4 +- arch/powerpc/kernel/pci_64.c | 4 +- arch/powerpc/kernel/pci_of_scan.c | 24 ++--- arch/powerpc/kernel/setup-common.c | 2 +- arch/powerpc/platforms/4xx/cpm.c | 8 +- arch/powerpc/platforms/4xx/gpio.c | 3 +- arch/powerpc/platforms/4xx/msi.c | 3 +- arch/powerpc/platforms/4xx/pci.c | 116 ++++++++++------------- arch/powerpc/platforms/4xx/soc.c | 5 +- arch/powerpc/platforms/4xx/uic.c | 14 +-- arch/powerpc/platforms/512x/mpc512x_shared.c | 12 +-- arch/powerpc/platforms/52xx/efika.c | 8 +- arch/powerpc/platforms/52xx/media5200.c | 2 +- arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 4 +- arch/powerpc/platforms/52xx/mpc52xx_pci.c | 8 +- arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 3 +- arch/powerpc/platforms/83xx/mpc832x_rdb.c | 2 +- arch/powerpc/platforms/85xx/p1022_ds.c | 8 +- arch/powerpc/platforms/85xx/xes_mpc85xx.c | 4 +- arch/powerpc/platforms/amigaone/setup.c | 6 +- arch/powerpc/platforms/cell/axon_msi.c | 36 +++---- arch/powerpc/platforms/cell/interrupt.c | 4 +- arch/powerpc/platforms/cell/iommu.c | 24 ++--- arch/powerpc/platforms/cell/ras.c | 4 +- arch/powerpc/platforms/cell/spider-pci.c | 4 +- arch/powerpc/platforms/cell/spider-pic.c | 4 +- arch/powerpc/platforms/cell/spu_manage.c | 26 ++--- arch/powerpc/platforms/chrp/pci.c | 18 ++-- arch/powerpc/platforms/embedded6xx/linkstation.c | 6 +- arch/powerpc/platforms/embedded6xx/mvme5100.c | 2 +- arch/powerpc/platforms/embedded6xx/storcenter.c | 2 +- arch/powerpc/platforms/maple/pci.c | 10 +- arch/powerpc/platforms/pasemi/pci.c | 2 +- arch/powerpc/platforms/powermac/feature.c | 14 +-- arch/powerpc/platforms/powermac/low_i2c.c | 50 +++++----- arch/powerpc/platforms/powermac/pci.c | 6 +- arch/powerpc/platforms/powermac/pfunc_base.c | 24 ++--- arch/powerpc/platforms/powermac/pfunc_core.c | 6 +- arch/powerpc/platforms/powermac/pic.c | 8 +- arch/powerpc/platforms/powermac/setup.c | 2 +- arch/powerpc/platforms/powernv/opal-async.c | 4 +- arch/powerpc/platforms/powernv/opal-xscom.c | 8 +- arch/powerpc/platforms/powernv/pci-ioda.c | 15 ++- arch/powerpc/platforms/powernv/rng.c | 6 +- arch/powerpc/platforms/pseries/dlpar.c | 39 ++------ arch/powerpc/platforms/pseries/event_sources.c | 6 +- arch/powerpc/platforms/pseries/hotplug-cpu.c | 6 +- arch/powerpc/platforms/pseries/ibmebus.c | 5 +- arch/powerpc/platforms/pseries/iommu.c | 58 ++++++------ arch/powerpc/platforms/pseries/mobility.c | 2 +- arch/powerpc/platforms/pseries/msi.c | 12 +-- arch/powerpc/platforms/pseries/pci_dlpar.c | 2 +- arch/powerpc/platforms/pseries/pseries.h | 2 +- arch/powerpc/platforms/pseries/reconfig.c | 2 +- arch/powerpc/platforms/pseries/vio.c | 10 +- arch/powerpc/sysdev/axonram.c | 4 +- arch/powerpc/sysdev/dcr.c | 4 +- arch/powerpc/sysdev/fsl_85xx_cache_sram.c | 12 +-- arch/powerpc/sysdev/fsl_gtm.c | 14 +-- arch/powerpc/sysdev/fsl_msi.c | 16 ++-- arch/powerpc/sysdev/fsl_pci.c | 47 +++++---- arch/powerpc/sysdev/fsl_rio.c | 36 +++---- arch/powerpc/sysdev/fsl_rmu.c | 12 +-- arch/powerpc/sysdev/mpic.c | 4 +- arch/powerpc/sysdev/mpic_msgr.c | 2 +- arch/powerpc/sysdev/mpic_msi.c | 2 +- arch/powerpc/sysdev/mpic_timer.c | 19 ++-- arch/powerpc/sysdev/msi_bitmap.c | 4 +- arch/powerpc/sysdev/mv64x60_dev.c | 20 ++-- arch/powerpc/sysdev/mv64x60_pci.c | 4 +- arch/powerpc/sysdev/of_rtc.c | 12 +-- arch/powerpc/sysdev/scom.c | 5 +- arch/powerpc/sysdev/simple_gpio.c | 3 +- arch/powerpc/sysdev/tsi108_pci.c | 4 +- arch/powerpc/sysdev/xive/native.c | 2 +- drivers/of/fdt.c | 69 +++----------- 83 files changed, 487 insertions(+), 578 deletions(-) -- 2.11.0
[toc] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2017-08-21 17:20 +0200 |
| Subject | [PATCH v2 5/5] of/fdt: only store the device node basename in full_name |
| Message-ID | <ugWEG-60L-15@gated-at.bofh.it> |
| In reply to | #1716587 |
With dependencies on a statically allocated full path name converted to
use %pOF format specifier, we can store just the basename of node, and
the unflattening of the FDT can be simplified.
This commit will affect the remaining users of full_name. After
analyzing these users, the remaining cases should only change some print
messages. The main users of full_name are providing a name for struct
resource. The resource names shouldn't be important other than providing
/proc/iomem names.
We no longer distinguish between pre and post 0x10 dtb formats as either
a full path or basename will work. However, less than 0x10 formats have
been broken since the conversion to use libfdt (and no one has cared).
The conversion of the unflattening code to be non-recursive also broke
pre 0x10 formats as the populate_node function would return 0 in that
case.
Signed-off-by: Rob Herring <robh@kernel.org>
---
v2:
- rebase to linux-next
drivers/of/fdt.c | 69 +++++++++-----------------------------------------------
1 file changed, 11 insertions(+), 58 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index ce30c9a588a4..27c535af0be8 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -266,74 +266,32 @@ static void populate_properties(const void *blob,
*pprev = NULL;
}
-static unsigned int populate_node(const void *blob,
- int offset,
- void **mem,
- struct device_node *dad,
- unsigned int fpsize,
- struct device_node **pnp,
- bool dryrun)
+static bool populate_node(const void *blob,
+ int offset,
+ void **mem,
+ struct device_node *dad,
+ struct device_node **pnp,
+ bool dryrun)
{
struct device_node *np;
const char *pathp;
unsigned int l, allocl;
- int new_format = 0;
pathp = fdt_get_name(blob, offset, &l);
if (!pathp) {
*pnp = NULL;
- return 0;
+ return false;
}
allocl = ++l;
- /* version 0x10 has a more compact unit name here instead of the full
- * path. we accumulate the full path size using "fpsize", we'll rebuild
- * it later. We detect this because the first character of the name is
- * not '/'.
- */
- if ((*pathp) != '/') {
- new_format = 1;
- if (fpsize == 0) {
- /* root node: special case. fpsize accounts for path
- * plus terminating zero. root node only has '/', so
- * fpsize should be 2, but we want to avoid the first
- * level nodes to have two '/' so we use fpsize 1 here
- */
- fpsize = 1;
- allocl = 2;
- l = 1;
- pathp = "";
- } else {
- /* account for '/' and path size minus terminal 0
- * already in 'l'
- */
- fpsize += l;
- allocl = fpsize;
- }
- }
-
np = unflatten_dt_alloc(mem, sizeof(struct device_node) + allocl,
__alignof__(struct device_node));
if (!dryrun) {
char *fn;
of_node_init(np);
np->full_name = fn = ((char *)np) + sizeof(*np);
- if (new_format) {
- /* rebuild full path for new format */
- if (dad && dad->parent) {
- strcpy(fn, dad->full_name);
-#ifdef DEBUG
- if ((strlen(fn) + l + 1) != allocl) {
- pr_debug("%s: p: %d, l: %d, a: %d\n",
- pathp, (int)strlen(fn),
- l, allocl);
- }
-#endif
- fn += strlen(fn);
- }
- *(fn++) = '/';
- }
+
memcpy(fn, pathp, l);
if (dad != NULL) {
@@ -355,7 +313,7 @@ static unsigned int populate_node(const void *blob,
}
*pnp = np;
- return fpsize;
+ return true;
}
static void reverse_nodes(struct device_node *parent)
@@ -399,7 +357,6 @@ static int unflatten_dt_nodes(const void *blob,
struct device_node *root;
int offset = 0, depth = 0, initial_depth = 0;
#define FDT_MAX_DEPTH 64
- unsigned int fpsizes[FDT_MAX_DEPTH];
struct device_node *nps[FDT_MAX_DEPTH];
void *base = mem;
bool dryrun = !base;
@@ -418,7 +375,6 @@ static int unflatten_dt_nodes(const void *blob,
depth = initial_depth = 1;
root = dad;
- fpsizes[depth] = dad ? strlen(of_node_full_name(dad)) : 0;
nps[depth] = dad;
for (offset = 0;
@@ -427,11 +383,8 @@ static int unflatten_dt_nodes(const void *blob,
if (WARN_ON_ONCE(depth >= FDT_MAX_DEPTH))
continue;
- fpsizes[depth+1] = populate_node(blob, offset, &mem,
- nps[depth],
- fpsizes[depth],
- &nps[depth+1], dryrun);
- if (!fpsizes[depth+1])
+ if (!populate_node(blob, offset, &mem, nps[depth],
+ &nps[depth+1], dryrun))
return mem - base;
if (!dryrun && nodepp && !*nodepp)
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2017-08-21 17:20 +0200 |
| Subject | [PATCH v2 2/5] powerpc: pseries: vio: match parent nodes with of_find_node_by_path |
| Message-ID | <ugWEG-60L-19@gated-at.bofh.it> |
| In reply to | #1716587 |
In preparation to remove the full path from device_node.full_name, use
of_find_node_by_path instead of open coding with strcmp.
Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
v2:
- rebased to linux-next and removed spurious change fro patch 1.
arch/powerpc/platforms/pseries/vio.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
index aa5ca74316fa..5754572deb23 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -1357,9 +1357,9 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
*/
parent_node = of_get_parent(of_node);
if (parent_node) {
- if (!strcmp(parent_node->full_name, "/ibm,platform-facilities"))
+ if (parent_node == of_find_node_by_path("/ibm,platform-facilities"))
family = PFO;
- else if (!strcmp(parent_node->full_name, "/vdevice"))
+ else if (parent_node == of_find_node_by_path("/vdevice"))
family = VDEVICE;
else {
pr_warn("%s: parent(%pOF) of %s not recognized.\n",
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Michael Ellerman <mpe@ellerman.id.au> |
|---|---|
| Date | 2017-08-22 07:20 +0200 |
| Subject | Re: [PATCH v2 2/5] powerpc: pseries: vio: match parent nodes with of_find_node_by_path |
| Message-ID | <uh9LA-6cY-9@gated-at.bofh.it> |
| In reply to | #1716589 |
Rob Herring <robh@kernel.org> writes:
> In preparation to remove the full path from device_node.full_name, use
> of_find_node_by_path instead of open coding with strcmp.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
> v2:
> - rebased to linux-next and removed spurious change fro patch 1.
>
> arch/powerpc/platforms/pseries/vio.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
> index aa5ca74316fa..5754572deb23 100644
> --- a/arch/powerpc/platforms/pseries/vio.c
> +++ b/arch/powerpc/platforms/pseries/vio.c
> @@ -1357,9 +1357,9 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
> */
> parent_node = of_get_parent(of_node);
> if (parent_node) {
> - if (!strcmp(parent_node->full_name, "/ibm,platform-facilities"))
> + if (parent_node == of_find_node_by_path("/ibm,platform-facilities"))
> family = PFO;
> - else if (!strcmp(parent_node->full_name, "/vdevice"))
> + else if (parent_node == of_find_node_by_path("/vdevice"))
> family = VDEVICE;
This leaks references to the looked up nodes.
Both these nodes are defined in PAPR (our hypervisor spec), and both of
them must have a device_type, either "ibm,platform-facilities" or
"vdevice".
Looking at the commit that added the code I don't see any particular
reason it used the comparison against full_name, rather than using the
device_type.
So I'm inclined to do this instead:
diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
index 8a47f168476b..f26f906e6021 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -1357,9 +1357,9 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
*/
parent_node = of_get_parent(of_node);
if (parent_node) {
- if (!strcmp(parent_node->full_name, "/ibm,platform-facilities"))
+ if (!strcmp(parent_node->type, "ibm,platform-facilities"))
family = PFO;
- else if (!strcmp(parent_node->full_name, "/vdevice"))
+ else if (!strcmp(parent_node->type, "vdevice"))
family = VDEVICE;
else {
pr_warn("%s: parent(%s) of %s not recognized.\n",
I've checked both Qemu and kvmtool add the device_type, and I'm fairly
confident that PowerVM does too. Anyway I'll test it on all the machines
I can find.
cheers
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2017-08-24 20:10 +0200 |
| Subject | Re: [PATCH v2 2/5] powerpc: pseries: vio: match parent nodes with of_find_node_by_path |
| Message-ID | <ui4JQ-1xk-25@gated-at.bofh.it> |
| In reply to | #1717029 |
On Tue, Aug 22, 2017 at 12:12 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> In preparation to remove the full path from device_node.full_name, use
>> of_find_node_by_path instead of open coding with strcmp.
>>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> ---
>> v2:
>> - rebased to linux-next and removed spurious change fro patch 1.
>>
>> arch/powerpc/platforms/pseries/vio.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
>> index aa5ca74316fa..5754572deb23 100644
>> --- a/arch/powerpc/platforms/pseries/vio.c
>> +++ b/arch/powerpc/platforms/pseries/vio.c
>> @@ -1357,9 +1357,9 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
>> */
>> parent_node = of_get_parent(of_node);
>> if (parent_node) {
>> - if (!strcmp(parent_node->full_name, "/ibm,platform-facilities"))
>> + if (parent_node == of_find_node_by_path("/ibm,platform-facilities"))
>> family = PFO;
>> - else if (!strcmp(parent_node->full_name, "/vdevice"))
>> + else if (parent_node == of_find_node_by_path("/vdevice"))
>> family = VDEVICE;
>
> This leaks references to the looked up nodes.
>
> Both these nodes are defined in PAPR (our hypervisor spec), and both of
> them must have a device_type, either "ibm,platform-facilities" or
> "vdevice".
>
> Looking at the commit that added the code I don't see any particular
> reason it used the comparison against full_name, rather than using the
> device_type.
>
> So I'm inclined to do this instead:
>
> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
> index 8a47f168476b..f26f906e6021 100644
> --- a/arch/powerpc/platforms/pseries/vio.c
> +++ b/arch/powerpc/platforms/pseries/vio.c
> @@ -1357,9 +1357,9 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
> */
> parent_node = of_get_parent(of_node);
> if (parent_node) {
> - if (!strcmp(parent_node->full_name, "/ibm,platform-facilities"))
> + if (!strcmp(parent_node->type, "ibm,platform-facilities"))
> family = PFO;
> - else if (!strcmp(parent_node->full_name, "/vdevice"))
> + else if (!strcmp(parent_node->type, "vdevice"))
> family = VDEVICE;
> else {
> pr_warn("%s: parent(%s) of %s not recognized.\n",
>
>
> I've checked both Qemu and kvmtool add the device_type, and I'm fairly
> confident that PowerVM does too. Anyway I'll test it on all the machines
> I can find.
Okay, do you want me to respin the patch or will you update it with
the above change?
Rob
[toc] | [prev] | [next] | [standalone]
| From | Michael Ellerman <mpe@ellerman.id.au> |
|---|---|
| Date | 2017-08-25 02:20 +0200 |
| Subject | Re: [PATCH v2 2/5] powerpc: pseries: vio: match parent nodes with of_find_node_by_path |
| Message-ID | <uiavT-5eJ-7@gated-at.bofh.it> |
| In reply to | #1719488 |
Rob Herring <robh@kernel.org> writes:
> On Tue, Aug 22, 2017 at 12:12 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Rob Herring <robh@kernel.org> writes:
>>
>>> In preparation to remove the full path from device_node.full_name, use
>>> of_find_node_by_path instead of open coding with strcmp.
>>>
>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> ---
>>> v2:
>>> - rebased to linux-next and removed spurious change fro patch 1.
>>>
>>> arch/powerpc/platforms/pseries/vio.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
>>> index aa5ca74316fa..5754572deb23 100644
>>> --- a/arch/powerpc/platforms/pseries/vio.c
>>> +++ b/arch/powerpc/platforms/pseries/vio.c
>>> @@ -1357,9 +1357,9 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
>>> */
>>> parent_node = of_get_parent(of_node);
>>> if (parent_node) {
>>> - if (!strcmp(parent_node->full_name, "/ibm,platform-facilities"))
>>> + if (parent_node == of_find_node_by_path("/ibm,platform-facilities"))
>>> family = PFO;
>>> - else if (!strcmp(parent_node->full_name, "/vdevice"))
>>> + else if (parent_node == of_find_node_by_path("/vdevice"))
>>> family = VDEVICE;
>>
>> This leaks references to the looked up nodes.
>>
>> Both these nodes are defined in PAPR (our hypervisor spec), and both of
>> them must have a device_type, either "ibm,platform-facilities" or
>> "vdevice".
>>
>> Looking at the commit that added the code I don't see any particular
>> reason it used the comparison against full_name, rather than using the
>> device_type.
>>
>> So I'm inclined to do this instead:
>>
>> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
>> index 8a47f168476b..f26f906e6021 100644
>> --- a/arch/powerpc/platforms/pseries/vio.c
>> +++ b/arch/powerpc/platforms/pseries/vio.c
>> @@ -1357,9 +1357,9 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
>> */
>> parent_node = of_get_parent(of_node);
>> if (parent_node) {
>> - if (!strcmp(parent_node->full_name, "/ibm,platform-facilities"))
>> + if (!strcmp(parent_node->type, "ibm,platform-facilities"))
>> family = PFO;
>> - else if (!strcmp(parent_node->full_name, "/vdevice"))
>> + else if (!strcmp(parent_node->type, "vdevice"))
>> family = VDEVICE;
>> else {
>> pr_warn("%s: parent(%s) of %s not recognized.\n",
>>
>>
>> I've checked both Qemu and kvmtool add the device_type, and I'm fairly
>> confident that PowerVM does too. Anyway I'll test it on all the machines
>> I can find.
>
> Okay, do you want me to respin the patch or will you update it with
I merged it. Should be in next today.
cheers
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2017-08-21 17:20 +0200 |
| Subject | [PATCH v2 3/5] powerpc: pseries: remove dlpar_attach_node dependency on full path |
| Message-ID | <ugWEG-60L-23@gated-at.bofh.it> |
| In reply to | #1716587 |
In preparation to stop storing the full node path in full_name, remove the
dependency on full_name from dlpar_attach_node(). Callers of
dlpar_attach_node() already have the parent device_node, so just pass the
parent node into dlpar_attach_node instead of the path. This avoids doing
a lookup of the parent node by the path.
Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
v2:
- rebased to linux-next
arch/powerpc/platforms/pseries/dlpar.c | 6 ++----
arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +-
arch/powerpc/platforms/pseries/mobility.c | 2 +-
arch/powerpc/platforms/pseries/pseries.h | 2 +-
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 80b84c9c8509..783f36364690 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -254,13 +254,11 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
return first_dn;
}
-int dlpar_attach_node(struct device_node *dn)
+int dlpar_attach_node(struct device_node *dn, struct device_node *parent)
{
int rc;
- dn->parent = pseries_of_derive_parent(dn->full_name);
- if (IS_ERR(dn->parent))
- return PTR_ERR(dn->parent);
+ dn->parent = parent;
rc = of_attach_node(dn);
if (rc) {
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 0a93093fbcef..b357f1ae0b0a 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -463,7 +463,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
return -EINVAL;
}
- rc = dlpar_attach_node(dn);
+ rc = dlpar_attach_node(dn, parent);
if (rc) {
saved_rc = rc;
pr_warn("Failed to attach node %s, rc: %d, drc index: %x\n",
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 2da4851eff99..210ce632d63e 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -229,7 +229,7 @@ static int add_dt_node(__be32 parent_phandle, __be32 drc_index)
if (!dn)
return -ENOENT;
- rc = dlpar_attach_node(dn);
+ rc = dlpar_attach_node(dn, parent_dn);
if (rc)
dlpar_free_cc_nodes(dn);
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 1361a9db534b..4470a3194311 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -46,7 +46,7 @@ extern void dlpar_free_cc_nodes(struct device_node *);
extern void dlpar_free_cc_property(struct property *);
extern struct device_node *dlpar_configure_connector(__be32,
struct device_node *);
-extern int dlpar_attach_node(struct device_node *);
+extern int dlpar_attach_node(struct device_node *, struct device_node *);
extern int dlpar_detach_node(struct device_node *);
extern int dlpar_acquire_drc(u32 drc_index);
extern int dlpar_release_drc(u32 drc_index);
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Michael Ellerman <patch-notifications@ellerman.id.au> |
|---|---|
| Date | 2017-08-24 14:40 +0200 |
| Subject | Re: [v2, 3/5] powerpc: pseries: remove dlpar_attach_node dependency on full path |
| Message-ID | <uhZAu-6z1-7@gated-at.bofh.it> |
| In reply to | #1716590 |
On Mon, 2017-08-21 at 15:16:49 UTC, Rob Herring wrote: > In preparation to stop storing the full node path in full_name, remove the > dependency on full_name from dlpar_attach_node(). Callers of > dlpar_attach_node() already have the parent device_node, so just pass the > parent node into dlpar_attach_node instead of the path. This avoids doing > a lookup of the parent node by the path. > > Signed-off-by: Rob Herring <robh@kernel.org> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: linuxppc-dev@lists.ozlabs.org Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/215ee763f8cb9a2912d411f96f6f67 cheers
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2017-08-21 17:20 +0200 |
| Subject | [PATCH v2 4/5] powerpc: pseries: only store the device node basename in full_name |
| Message-ID | <ugWEH-60L-25@gated-at.bofh.it> |
| In reply to | #1716587 |
With dependencies on full_name containing the entire device node path
removed, stop storing the full_name in nodes created by
dlpar_configure_connector() and pSeries_reconfig_add_node().
Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
v2:
- Rework dlpar_parse_cc_node() to a single allocation and strcpy instead
of kasprintf
arch/powerpc/platforms/pseries/dlpar.c | 30 +++++++-----------------------
arch/powerpc/platforms/pseries/reconfig.c | 2 +-
2 files changed, 8 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 783f36364690..31a0615aeea1 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -75,28 +75,17 @@ static struct property *dlpar_parse_cc_property(struct cc_workarea *ccwa)
return prop;
}
-static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa,
- const char *path)
+static struct device_node *(dlpar_parse_cc_node)(struct cc_workarea *ccwa)
{
struct device_node *dn;
- char *name;
-
- /* If parent node path is "/" advance path to NULL terminator to
- * prevent double leading slashs in full_name.
- */
- if (!path[1])
- path++;
+ const char *name = (const char *)ccwa + be32_to_cpu(ccwa->name_offset);
- dn = kzalloc(sizeof(*dn), GFP_KERNEL);
+ dn = kzalloc(sizeof(*dn) + strlen(name) + 1, GFP_KERNEL);
if (!dn)
return NULL;
- name = (char *)ccwa + be32_to_cpu(ccwa->name_offset);
- dn->full_name = kasprintf(GFP_KERNEL, "%s/%s", path, name);
- if (!dn->full_name) {
- kfree(dn);
- return NULL;
- }
+ dn->full_name = (char *)(dn + 1);
+ strcpy((char *)dn->full_name, name);
of_node_set_flag(dn, OF_DYNAMIC);
of_node_init(dn);
@@ -148,7 +137,6 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
struct property *last_property = NULL;
struct cc_workarea *ccwa;
char *data_buf;
- const char *parent_path = parent->full_name;
int cc_token;
int rc = -1;
@@ -182,7 +170,7 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
break;
case NEXT_SIBLING:
- dn = dlpar_parse_cc_node(ccwa, parent_path);
+ dn = dlpar_parse_cc_node(ccwa);
if (!dn)
goto cc_error;
@@ -192,10 +180,7 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
break;
case NEXT_CHILD:
- if (first_dn)
- parent_path = last_dn->full_name;
-
- dn = dlpar_parse_cc_node(ccwa, parent_path);
+ dn = dlpar_parse_cc_node(ccwa);
if (!dn)
goto cc_error;
@@ -226,7 +211,6 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
case PREV_PARENT:
last_dn = last_dn->parent;
- parent_path = last_dn->parent->full_name;
break;
case CALL_AGAIN:
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 296c188fd5ca..f24d8159c9e1 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -33,7 +33,7 @@ static int pSeries_reconfig_add_node(const char *path, struct property *proplist
if (!np)
goto out_err;
- np->full_name = kstrdup(path, GFP_KERNEL);
+ np->full_name = kstrdup(kbasename(path), GFP_KERNEL);
if (!np->full_name)
goto out_err;
--
2.11.0
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web