linux-next: manual merge of the devicetree tree with the powerpc tree

linux-next: manual merge of the devicetree tree with the powerpc tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

Hi Grant,

Today's linux-next merge of the devicetree tree got a conflict in
include/linux/of.h between commit eb740b5f3e65 ("powerpc/eeh: Introduce
EEH device") from the powerpc tree and commit 0f22dd395fc4 ("of: Only
compile OF_DYNAMIC on PowerPC pseries and iseries") from the devicetree
tree.

Just context changes.  I fixed it up (see below) and can carry the fix as
necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc include/linux/of.h
index bdb1c07,533603e..0000000
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@@ -75,14 -72,10 +75,17 @@@ struct of_phandle_args 
  	uint32_t args[MAX_PHANDLE_ARGS];
  };
  
 +#if defined(CONFIG_EEH)
 +static inline struct eeh_dev *of_node_to_eeh_dev(struct device_node *dn)
 +{
 +	return dn->edev;
 +}
 +#endif
 +
- #if defined(CONFIG_SPARC) || !defined(CONFIG_OF)
+ #ifdef CONFIG_OF_DYNAMIC
+ extern struct device_node *of_node_get(struct device_node *node);
+ extern void of_node_put(struct device_node *node);
+ #else /* CONFIG_OF_DYNAMIC */
  /* Dummy ref counting routines - to be implemented later */
  static inline struct device_node *of_node_get(struct device_node *node)
  {

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: manual merge of the devicetree tree with the powerpc tree

[Grant Likely]

On Tue, 13 Mar 2012 16:01:00 +1100, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Grant,
> 
> Today's linux-next merge of the devicetree tree got a conflict in
> include/linux/of.h between commit eb740b5f3e65 ("powerpc/eeh: Introduce
> EEH device") from the powerpc tree and commit 0f22dd395fc4 ("of: Only
> compile OF_DYNAMIC on PowerPC pseries and iseries") from the devicetree
> tree.
> 
> Just context changes.  I fixed it up (see below) and can carry the fix as
> necessary.
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --cc include/linux/of.h
> index bdb1c07,533603e..0000000
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@@ -75,14 -72,10 +75,17 @@@ struct of_phandle_args 
>   	uint32_t args[MAX_PHANDLE_ARGS];
>   };
>   
>  +#if defined(CONFIG_EEH)
>  +static inline struct eeh_dev *of_node_to_eeh_dev(struct device_node *dn)
>  +{
>  +	return dn->edev;
>  +}
>  +#endif

Ben, What is this?  I don't want the eeh_dev pointer in struct device_node.  Up to
now we've avoided putting any reverse references into device_nodes.  For everything
else we use a reverse lookup, particularly for devices, to avoid growing the
device_node for each new type of lookup.

g.

Re: linux-next: manual merge of the devicetree tree with the powerpc tree

[Gavin Shan]

Hi Grant,

> > 
> > Today's linux-next merge of the devicetree tree got a conflict in
> > include/linux/of.h between commit eb740b5f3e65 ("powerpc/eeh: Introduce
> > EEH device") from the powerpc tree and commit 0f22dd395fc4 ("of: Only
> > compile OF_DYNAMIC on PowerPC pseries and iseries") from the devicetree
> > tree.
> > 
> > Just context changes.  I fixed it up (see below) and can carry the fix as
> > necessary.
> > -- 
> > Cheers,
> > Stephen Rothwell                    sfr@canb.auug.org.au
> > 
> > diff --cc include/linux/of.h
> > index bdb1c07,533603e..0000000
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > @@@ -75,14 -72,10 +75,17 @@@ struct of_phandle_args 
> >   	uint32_t args[MAX_PHANDLE_ARGS];
> >   };
> >   
> >  +#if defined(CONFIG_EEH)
> >  +static inline struct eeh_dev *of_node_to_eeh_dev(struct device_node *dn)
> >  +{
> >  +	return dn->edev;
> >  +}
> >  +#endif
> 
> Ben, What is this?  I don't want the eeh_dev pointer in struct device_node.  Up to
> now we've avoided putting any reverse references into device_nodes.  For everything
> else we use a reverse lookup, particularly for devices, to avoid growing the
> device_node for each new type of lookup.
> 

It's used to trace the EEH device. When EEH (Enhanced Error Hanlding) is enabled,
EEH device will be created against PCI sensitive OF node to trace the EEH state
accordingly. Since you don't want see this in struct device_node, we have to change
struct eeh_dev for a little bit to so that all struct eeh_dev instances will form
a global list and we can search eeh_dev according to the given device_node through
the global list. 

I don't know the policy or rule here for much. I think we can have 2 options.

1. Keep the code as being, and fix it later.
2. Fix it now.

Thanks,
Gavin

Re: linux-next: manual merge of the devicetree tree with the powerpc tree

[Benjamin Herrenschmidt]

On Wed, 2012-03-14 at 09:53 +0800, Gavin Shan wrote:
> >  +#if defined(CONFIG_EEH)
> > >  +static inline struct eeh_dev *of_node_to_eeh_dev(struct device_node *dn)
> > >  +{
> > >  +  return dn->edev;
> > >  +}
> > >  +#endif
> > 
> > Ben, What is this?  I don't want the eeh_dev pointer in struct device_node.  Up to
> > now we've avoided putting any reverse references into device_nodes.  For everything
> > else we use a reverse lookup, particularly for devices, to avoid growing the
> > device_node for each new type of lookup.
> > 
> 
> It's used to trace the EEH device. When EEH (Enhanced Error Hanlding) is enabled,
> EEH device will be created against PCI sensitive OF node to trace the EEH state
> accordingly. Since you don't want see this in struct device_node, we have to change
> struct eeh_dev for a little bit to so that all struct eeh_dev instances will form
> a global list and we can search eeh_dev according to the given device_node through
> the global list. 
> 
> I don't know the policy or rule here for much. I think we can have 2 options.
> 
> 1. Keep the code as being, and fix it later.
> 2. Fix it now. 

My bad, it's a mis-review, I thought it was still in pci_dn, I din't
catch Gavin moving it to device-node.

Yes, Gavin, we need to do something else, a chained list we walk or
something like that. For the "fast path" which is when we have a pci_dev
around, we can either add it to dev_archdata or hijack the pci-dev
platform_data (I don't think anything uses it, Grant, do you know of
anything ?)

The patches are already in -next and I won't rebase, so we need to fix
it on top of the existing patches. Gavin, can you make a patch that puts
it back into pci_dn to begin with, then we can contemplate what better
long term solution we have ?

Cheers,
Ben.

Re: linux-next: manual merge of the devicetree tree with the powerpc tree

[Gavin Shan]

>> >  +#if defined(CONFIG_EEH)
>> > >  +static inline struct eeh_dev *of_node_to_eeh_dev(struct device_node *dn)
>> > >  +{
>> > >  +  return dn->edev;
>> > >  +}
>> > >  +#endif
>> > 
>> > Ben, What is this?  I don't want the eeh_dev pointer in struct device_node.  Up to
>> > now we've avoided putting any reverse references into device_nodes.  For everything
>> > else we use a reverse lookup, particularly for devices, to avoid growing the
>> > device_node for each new type of lookup.
>> > 
>> 
>> It's used to trace the EEH device. When EEH (Enhanced Error Hanlding) is enabled,
>> EEH device will be created against PCI sensitive OF node to trace the EEH state
>> accordingly. Since you don't want see this in struct device_node, we have to change
>> struct eeh_dev for a little bit to so that all struct eeh_dev instances will form
>> a global list and we can search eeh_dev according to the given device_node through
>> the global list. 
>> 
>> I don't know the policy or rule here for much. I think we can have 2 options.
>> 
>> 1. Keep the code as being, and fix it later.
>> 2. Fix it now. 
>
>My bad, it's a mis-review, I thought it was still in pci_dn, I din't
>catch Gavin moving it to device-node.
>
>Yes, Gavin, we need to do something else, a chained list we walk or
>something like that. For the "fast path" which is when we have a pci_dev
>around, we can either add it to dev_archdata or hijack the pci-dev
>platform_data (I don't think anything uses it, Grant, do you know of
>anything ?)
>

Yes, Ben. I'll come up another patch on top of -next. It's supposed
to introduce global list for newly created eeh_dev and retrieve
the corresponding eeh_dev according to the given device_node through
it.

>The patches are already in -next and I won't rebase, so we need to fix
>it on top of the existing patches. Gavin, can you make a patch that puts
>it back into pci_dn to begin with, then we can contemplate what better
>long term solution we have ?
>

I've had something more in my private git tree regarding this. I'll
talk with you for your comments ;-)

Thanks,
Gavin

linux-next: manual merge of the devicetree tree with the powerpc tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

Hi all,

Today's linux-next merge of the devicetree tree got a conflict in:

  arch/powerpc/kexec/elf_64.c

between commit:

  2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump kernel")

from the powerpc tree and commit:

  130b2d59cec0 ("powerpc: Use common of_kexec_alloc_and_setup_fdt()")

from the devicetree tree.

I can't easily see how to resolve these, so for now I have just used
the latter' changes to this file.

I fixed it up and can carry the fix as necessary. This is now fixed as
far as linux-next is concerned, but any non trivial conflicts should be
mentioned to your upstream maintainer when your tree is submitted for
merging.  You may also want to consider cooperating with the maintainer
of the conflicting tree to minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the devicetree tree with the powerpc tree

[Michael Ellerman]

Stephen Rothwell <sfr@canb.auug.org.au> writes:
> Hi all,
>
> Today's linux-next merge of the devicetree tree got a conflict in:
>
>   arch/powerpc/kexec/elf_64.c
>
> between commit:
>
>   2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump kernel")
>
> from the powerpc tree and commit:
>
>   130b2d59cec0 ("powerpc: Use common of_kexec_alloc_and_setup_fdt()")
>
> from the devicetree tree.
>
> I can't easily see how to resolve these, so for now I have just used
> the latter' changes to this file.

I think it just needs this?

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 87e34611f93d..0492ca6003f3 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 
 	fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
 					   initrd_len, cmdline,
-					   fdt_totalsize(initial_boot_params));
+					   kexec_fdt_totalsize_ppc64(image));
 	if (!fdt) {
 		pr_err("Error setting up the new device tree.\n");
 		ret = -EINVAL;


cheers

Re: linux-next: manual merge of the devicetree tree with the powerpc tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

Hi Michael,

On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> I think it just needs this?
> 
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 87e34611f93d..0492ca6003f3 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>  
>  	fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
>  					   initrd_len, cmdline,
> -					   fdt_totalsize(initial_boot_params));
> +					   kexec_fdt_totalsize_ppc64(image));
>  	if (!fdt) {
>  		pr_err("Error setting up the new device tree.\n");
>  		ret = -EINVAL;
> 

I thought about that, but the last argument to
of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
done is for this:

fdt_size = fdt_totalsize(initial_boot_params) +
                   (cmdline ? strlen(cmdline) : 0) +
                   FDT_EXTRA_SPACE +
                   extra_fdt_size;

and kexec_fdt_totalsize_ppc64() also includes
fdt_totalsize(initial_boot_params) so I was not sure.  Maybe
kexec_fdt_totalsize_ppc64() needs modification as well?

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the devicetree tree with the powerpc tree

[Rob Herring]

On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Michael,
>
> On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > I think it just needs this?
> >
> > diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> > index 87e34611f93d..0492ca6003f3 100644
> > --- a/arch/powerpc/kexec/elf_64.c
> > +++ b/arch/powerpc/kexec/elf_64.c
> > @@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> >
> >       fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
> >                                          initrd_len, cmdline,
> > -                                        fdt_totalsize(initial_boot_params));
> > +                                        kexec_fdt_totalsize_ppc64(image));
> >       if (!fdt) {
> >               pr_err("Error setting up the new device tree.\n");
> >               ret = -EINVAL;
> >
>
> I thought about that, but the last argument to
> of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
> done is for this:
>
> fdt_size = fdt_totalsize(initial_boot_params) +
>                    (cmdline ? strlen(cmdline) : 0) +
>                    FDT_EXTRA_SPACE +
>                    extra_fdt_size;
>
> and kexec_fdt_totalsize_ppc64() also includes
> fdt_totalsize(initial_boot_params) so I was not sure.  Maybe
> kexec_fdt_totalsize_ppc64() needs modification as well?

You're both right. Michael's fix is sufficient for the merge. The only
risk with a larger size is failing to allocate it, but we're talking
only 10s of KB. Historically until the commit causing the conflict,
PPC was just used 2x fdt_totalsize(initial_boot_params). You could
drop 'fdt_size = fdt_totalsize(initial_boot_params) + (2 *
COMMAND_LINE_SIZE);' from kexec_fdt_totalsize_ppc64() as well, but
then the function name is misleading.

Lakshmi can send a follow-up patch to fine tune the size and rename
kexec_fdt_totalsize_ppc64.

Rob

Re: linux-next: manual merge of the devicetree tree with the powerpc tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]

Hi all,

On Thu, 18 Feb 2021 07:52:52 -0600 Rob Herring <robherring2@gmail.com> wrote:
>
> On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote:  
> > >
> > > I think it just needs this?
> > >
> > > diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> > > index 87e34611f93d..0492ca6003f3 100644
> > > --- a/arch/powerpc/kexec/elf_64.c
> > > +++ b/arch/powerpc/kexec/elf_64.c
> > > @@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> > >
> > >       fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
> > >                                          initrd_len, cmdline,
> > > -                                        fdt_totalsize(initial_boot_params));
> > > +                                        kexec_fdt_totalsize_ppc64(image));
> > >       if (!fdt) {
> > >               pr_err("Error setting up the new device tree.\n");
> > >               ret = -EINVAL;
> > >  
> >
> > I thought about that, but the last argument to
> > of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
> > done is for this:
> >
> > fdt_size = fdt_totalsize(initial_boot_params) +
> >                    (cmdline ? strlen(cmdline) : 0) +
> >                    FDT_EXTRA_SPACE +
> >                    extra_fdt_size;
> >
> > and kexec_fdt_totalsize_ppc64() also includes
> > fdt_totalsize(initial_boot_params) so I was not sure.  Maybe
> > kexec_fdt_totalsize_ppc64() needs modification as well?  
> 
> You're both right. Michael's fix is sufficient for the merge. The only
> risk with a larger size is failing to allocate it, but we're talking
> only 10s of KB. Historically until the commit causing the conflict,
> PPC was just used 2x fdt_totalsize(initial_boot_params). You could
> drop 'fdt_size = fdt_totalsize(initial_boot_params) + (2 *
> COMMAND_LINE_SIZE);' from kexec_fdt_totalsize_ppc64() as well, but
> then the function name is misleading.
> 
> Lakshmi can send a follow-up patch to fine tune the size and rename
> kexec_fdt_totalsize_ppc64.

OK, I have mode Michael's suggested change to my resolution from today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: linux-next: manual merge of the devicetree tree with the powerpc tree

[Michael Ellerman]

Rob Herring <robherring2@gmail.com> writes:
> On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote:
>> >
>> > I think it just needs this?
>> >
>> > diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>> > index 87e34611f93d..0492ca6003f3 100644
>> > --- a/arch/powerpc/kexec/elf_64.c
>> > +++ b/arch/powerpc/kexec/elf_64.c
>> > @@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>> >
>> >       fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
>> >                                          initrd_len, cmdline,
>> > -                                        fdt_totalsize(initial_boot_params));
>> > +                                        kexec_fdt_totalsize_ppc64(image));
>> >       if (!fdt) {
>> >               pr_err("Error setting up the new device tree.\n");
>> >               ret = -EINVAL;
>> >
>>
>> I thought about that, but the last argument to
>> of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
>> done is for this:
>>
>> fdt_size = fdt_totalsize(initial_boot_params) +
>>                    (cmdline ? strlen(cmdline) : 0) +
>>                    FDT_EXTRA_SPACE +
>>                    extra_fdt_size;
>>
>> and kexec_fdt_totalsize_ppc64() also includes
>> fdt_totalsize(initial_boot_params) so I was not sure.  Maybe
>> kexec_fdt_totalsize_ppc64() needs modification as well?
>
> You're both right. Michael's fix is sufficient for the merge. The only
> risk with a larger size is failing to allocate it, but we're talking
> only 10s of KB. Historically until the commit causing the conflict,
> PPC was just used 2x fdt_totalsize(initial_boot_params). You could
> drop 'fdt_size = fdt_totalsize(initial_boot_params) + (2 *
> COMMAND_LINE_SIZE);' from kexec_fdt_totalsize_ppc64() as well, but
> then the function name is misleading.
>
> Lakshmi can send a follow-up patch to fine tune the size and rename
> kexec_fdt_totalsize_ppc64.

Sounds good.

cheers

linux-next: manual merge of the devicetree tree with the powerpc tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

Hi Grant,

Today's linux-next merge of the devicetree tree got a conflict in
arch/powerpc/platforms/iseries/Kconfig between commit 3d066d77cf46
("powerpc: remove CONFIG_PPC_ISERIES from the architecture Kconfig
files") from the powerpc tree and commit 0f22dd395fc4 ("of: Only compile
OF_DYNAMIC on PowerPC pseries and iseries") from the devicetree tree.

The former commit removes the file (as the legacy iSeries platform is
being removed), so I did that.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: manual merge of the devicetree tree with the powerpc tree

[Grant Likely]

On Mon, Feb 27, 2012 at 7:06 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Grant,
>
> Today's linux-next merge of the devicetree tree got a conflict in
> arch/powerpc/platforms/iseries/Kconfig between commit 3d066d77cf46
> ("powerpc: remove CONFIG_PPC_ISERIES from the architecture Kconfig
> files") from the powerpc tree and commit 0f22dd395fc4 ("of: Only compile
> OF_DYNAMIC on PowerPC pseries and iseries") from the devicetree tree.
>
> The former commit removes the file (as the legacy iSeries platform is
> being removed), so I did that.

Okay, thanks.

g.

> --
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au



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