2015-07-06 20:26:52

by Pali Rohár

[permalink] [raw]
Subject: [PATCH 0/5] ATAGs to DT patches

This patch series converts some legacy ATAGs into DT and provide them to
userspace. It is needed for userspace applications which needs some
informations from legacy bootloaders which are not DT compatible.

Patch series is for now without DT documentation files. It was tested in
QEMU for Nokia N900. File /proc/cpuinfo contains correct "Revision" line
from ATAG_REVISION and /proc/atags contains full ATAGs structure.

Pali Rohár (5):
arm: devtree: Set system_rev from DT "/revision"
arm: boot: convert ATAG_REVISION to DT "/revision" entry
arm: atags: Fix declaration of function save_atags
arm: devtree: Read ATAGs structure from DT "/chosen/linux,atags"
entry
arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

arch/arm/boot/compressed/atags_to_fdt.c | 8 ++++++-
arch/arm/kernel/atags.h | 4 ++--
arch/arm/kernel/devtree.c | 36 ++++++++++++++++++++++++++++---
3 files changed, 42 insertions(+), 6 deletions(-)

--
1.7.9.5


2015-07-06 20:27:49

by Pali Rohár

[permalink] [raw]
Subject: [PATCH 1/5] arm: devtree: Set system_rev from DT "/revision"

With this patch "/revision" DT entry is used to set global system_rev
variable. DT "/revision" is expected to be u32 numeric value.

TODO: add documentation

Signed-off-by: Pali Rohár <[email protected]>
---
arch/arm/kernel/devtree.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index 11c54de..7d82749 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -19,6 +19,7 @@
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/smp.h>
+#include <linux/libfdt_env.h>

#include <asm/cputype.h>
#include <asm/setup.h>
@@ -26,6 +27,7 @@
#include <asm/smp_plat.h>
#include <asm/mach/arch.h>
#include <asm/mach-types.h>
+#include <asm/system_info.h>


#ifdef CONFIG_SMP
@@ -204,6 +206,8 @@ static const void * __init arch_get_next_mach(const char *const **match)
const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
{
const struct machine_desc *mdesc, *mdesc_best = NULL;
+ unsigned long dt_root;
+ const u32 *rev;

#ifdef CONFIG_ARCH_MULTIPLATFORM
DT_MACHINE_START(GENERIC_DT, "Generic DT based system")
@@ -215,17 +219,16 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
if (!dt_phys || !early_init_dt_verify(phys_to_virt(dt_phys)))
return NULL;

+ dt_root = of_get_flat_dt_root();
mdesc = of_flat_dt_match_machine(mdesc_best, arch_get_next_mach);

if (!mdesc) {
const char *prop;
int size;
- unsigned long dt_root;

early_print("\nError: unrecognized/unsupported "
"device tree compatible list:\n[ ");

- dt_root = of_get_flat_dt_root();
prop = of_get_flat_dt_prop(dt_root, "compatible", &size);
while (size > 0) {
early_print("'%s' ", prop);
@@ -246,5 +249,10 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
/* Change machine number to match the mdesc we're using */
__machine_arch_type = mdesc->nr;

+ /* Set system revision from DT */
+ rev = of_get_flat_dt_prop(dt_root, "revision", NULL);
+ if (rev)
+ system_rev = fdt32_to_cpu(*rev);
+
return mdesc;
}
--
1.7.9.5

2015-07-06 20:27:56

by Pali Rohár

[permalink] [raw]
Subject: [PATCH 2/5] arm: boot: convert ATAG_REVISION to DT "/revision" entry

When uncompressing kernel image this patch convert ATAG_REVISION structure
into DT "/revision" entry. For legacy bootloaders which do not support DT
booting there is no other option how to pass ATAG_REVISION into kernel.

Signed-off-by: Pali Rohár <[email protected]>
---
arch/arm/boot/compressed/atags_to_fdt.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/compressed/atags_to_fdt.c
index 9448aa0..e7e1cc9 100644
--- a/arch/arm/boot/compressed/atags_to_fdt.c
+++ b/arch/arm/boot/compressed/atags_to_fdt.c
@@ -171,6 +171,8 @@ int atags_to_fdt(void *atag_list, void *fdt, int total_space)
cpu_to_fdt32(atag->u.mem.size);
}

+ } else if (atag->hdr.tag == ATAG_REVISION) {
+ setprop_cell(fdt, "/", "revision", atag->u.revision.rev);
} else if (atag->hdr.tag == ATAG_INITRD2) {
uint32_t initrd_start, initrd_size;
initrd_start = atag->u.initrd.start;
--
1.7.9.5

2015-07-06 20:27:00

by Pali Rohár

[permalink] [raw]
Subject: [PATCH 3/5] arm: atags: Fix declaration of function save_atags

In file atags_proc.c function save_atags() expect const argument, but in
atags.h file is declarated as non const. Fix declaration in atags.h file to
match what is expected.

Signed-off-by: Pali Rohár <[email protected]>
---
arch/arm/kernel/atags.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/atags.h b/arch/arm/kernel/atags.h
index ec4164d..2dfc30f 100644
--- a/arch/arm/kernel/atags.h
+++ b/arch/arm/kernel/atags.h
@@ -1,7 +1,7 @@
#ifdef CONFIG_ATAGS_PROC
-extern void save_atags(struct tag *tags);
+extern void save_atags(const struct tag *tags);
#else
-static inline void save_atags(struct tag *tags) { }
+static inline void save_atags(const struct tag *tags) { }
#endif

void convert_to_tag_list(struct tag *tags);
--
1.7.9.5

2015-07-06 20:27:11

by Pali Rohár

[permalink] [raw]
Subject: [PATCH 4/5] arm: devtree: Read ATAGs structure from DT "/chosen/linux,atags" entry

With this patch when linux kernel is compiled with ARM_ATAG_DTB_COMPAT it
reads ATAGs structure from "/chosen/linux,atags" entry and store it into
/proc/atags file. ATAGs structure is not parsed or evaluated, just kernel
exports it to userspace via procfs. It is needed for legacy userspace
application which expect some data from bootloader in /proc/atags file.
This patch keeps them running also on DT kernel if DT "/chosen/linux,atags"
entry exists.

TODO: add documentation

Signed-off-by: Pali Rohár <[email protected]>
---
arch/arm/kernel/devtree.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index 7d82749..69ff048 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -20,6 +20,7 @@
#include <linux/of_platform.h>
#include <linux/smp.h>
#include <linux/libfdt_env.h>
+#include <linux/libfdt.h>

#include <asm/cputype.h>
#include <asm/setup.h>
@@ -29,6 +30,7 @@
#include <asm/mach-types.h>
#include <asm/system_info.h>

+#include "atags.h"

#ifdef CONFIG_SMP
extern struct of_cpu_method __cpu_method_of_table[];
@@ -208,6 +210,11 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
const struct machine_desc *mdesc, *mdesc_best = NULL;
unsigned long dt_root;
const u32 *rev;
+ void *dt_virt;
+#ifdef CONFIG_ARM_ATAG_DTB_COMPAT
+ const void *atags;
+ unsigned long dt_chosen;
+#endif

#ifdef CONFIG_ARCH_MULTIPLATFORM
DT_MACHINE_START(GENERIC_DT, "Generic DT based system")
@@ -216,7 +223,12 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
mdesc_best = &__mach_desc_GENERIC_DT;
#endif

- if (!dt_phys || !early_init_dt_verify(phys_to_virt(dt_phys)))
+ if (!dt_phys)
+ return NULL;
+
+ dt_virt = phys_to_virt(dt_phys);
+
+ if (!early_init_dt_verify(dt_virt))
return NULL;

dt_root = of_get_flat_dt_root();
@@ -254,5 +266,15 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
if (rev)
system_rev = fdt32_to_cpu(*rev);

+#ifdef CONFIG_ARM_ATAG_DTB_COMPAT
+ /* Store DT /chosen/linux,atags into /proc/atags */
+ dt_chosen = fdt_path_offset(dt_virt, "/chosen");
+ if (dt_chosen >= 0) {
+ atags = of_get_flat_dt_prop(dt_chosen, "linux,atags", NULL);
+ if (atags)
+ save_atags(atags);
+ }
+#endif
+
return mdesc;
}
--
1.7.9.5

2015-07-06 20:27:07

by Pali Rohár

[permalink] [raw]
Subject: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

Legacy bootloaders can pass additional information for kernel or legacy
userspace applications. When booting DT kernel then ATAGs structure is not
more visible after running kernel uncompress code. This patch stores full
ATAGs structure into DT "/chosen/linux,atags" entry, so kernel can later
reuse it and export via /proc/atags to userspace.

Signed-off-by: Pali Rohár <[email protected]>
---
arch/arm/boot/compressed/atags_to_fdt.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/compressed/atags_to_fdt.c
index e7e1cc9..ae93973 100644
--- a/arch/arm/boot/compressed/atags_to_fdt.c
+++ b/arch/arm/boot/compressed/atags_to_fdt.c
@@ -112,7 +112,7 @@ int atags_to_fdt(void *atag_list, void *fdt, int total_space)
* address and size for each bank */
uint32_t mem_reg_property[2 * 2 * NR_BANKS];
int memcount = 0;
- int ret, memsize;
+ int ret, memsize, atag_size;

/* make sure we've got an aligned pointer */
if ((u32)atag_list & 0x3)
@@ -189,5 +189,9 @@ int atags_to_fdt(void *atag_list, void *fdt, int total_space)
4 * memcount * memsize);
}

+ /* include the terminating ATAG_NONE */
+ atag_size = (char *)atag - (char *)atag_list + sizeof(struct tag_header);
+ setprop(fdt, "/chosen", "linux,atags", atag_list, atag_size);
+
return fdt_pack(fdt);
}
--
1.7.9.5

2015-07-07 11:32:47

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Mon, Jul 06, 2015 at 10:26:13PM +0200, Pali Roh?r wrote:
> Legacy bootloaders can pass additional information for kernel or legacy
> userspace applications. When booting DT kernel then ATAGs structure is not
> more visible after running kernel uncompress code. This patch stores full
> ATAGs structure into DT "/chosen/linux,atags" entry, so kernel can later
> reuse it and export via /proc/atags to userspace.

I think you need to go through your commit messages and improve them,
especially the ones with "TODO" in them. As long as there's still things
to be done, they're obviously not ready for merging.

Moreover, exporting the ATAGS is questionable, even _if_ there are non-
kexec programs making use of this. The ATAGs have _never_ been exported
to userspace when kexec disabled is the kernel - it was introduced for
kexec, and has always had this:

config ATAGS_PROC
bool "Export atags in procfs"
depends on ATAGS && KEXEC
default y

Now, the fact that someone decided to start using it is pretty sad,
because it means that if you disable KEXEC, userspace breaks. That's
not a kernel regression in any shape or form, because /proc/atags has
never been there without KEXEC enabled. That's a userspace bug, plain
and simple.

Given that, I'm in two minds about whether to accept the last two
patches which make this more than just "for KEXEC use to enable a KEXEC
kernel to be booted."

Had it been provided without the KEXEC conditional, then I don't have
a problem with these two patches.

It also sets a precedent: by adding this into DT, it is creating a new
DT ABI as well, and we'll end up seeing dts files with an ATAG block
patched into them.

Are the ATAGs at a fixed address on the N900? Can that be handled in
some kind of legacy file for the N900 which calls save_atags() on it, so
we don't end up introducing yet more stuff that we have to maintain into
the distant future? If not, what about copying a known working atag
structure into a legacy file for the N900?

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-07-07 11:59:06

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Tuesday 07 July 2015 12:32:13 Russell King - ARM Linux wrote:
> On Mon, Jul 06, 2015 at 10:26:13PM +0200, Pali Rohár wrote:
> > Legacy bootloaders can pass additional information for kernel or legacy
> > userspace applications. When booting DT kernel then ATAGs structure is not
> > more visible after running kernel uncompress code. This patch stores full
> > ATAGs structure into DT "/chosen/linux,atags" entry, so kernel can later
> > reuse it and export via /proc/atags to userspace.
>
> I think you need to go through your commit messages and improve them,
> especially the ones with "TODO" in them. As long as there's still things
> to be done, they're obviously not ready for merging.
>

I know, in cover letter email I wrote that documentation is not ready...
I send patches for review and comments (like yours). I think it is still
better to send something and mark it as incomplete. It could prevent to
work on something which will be again rewritten...

> Moreover, exporting the ATAGS is questionable, even _if_ there are non-
> kexec programs making use of this. The ATAGs have _never_ been exported
> to userspace when kexec disabled is the kernel - it was introduced for
> kexec, and has always had this:
>
> config ATAGS_PROC
> bool "Export atags in procfs"
> depends on ATAGS && KEXEC
> default y
>
> Now, the fact that someone decided to start using it is pretty sad,
> because it means that if you disable KEXEC, userspace breaks. That's
> not a kernel regression in any shape or form, because /proc/atags has
> never been there without KEXEC enabled. That's a userspace bug, plain
> and simple.
>
> Given that, I'm in two minds about whether to accept the last two
> patches which make this more than just "for KEXEC use to enable a KEXEC
> kernel to be booted."
>
> Had it been provided without the KEXEC conditional, then I don't have
> a problem with these two patches.
>

I understand it. Nokia originally invented their own entries in /proc/
which export needed ATAGs from kernel in human-readable form, but all
those entries were non-standard and specific for Nokia's kernels.

Do you have some other idea how to provide ATAGs information created
dynamically by legacy closed proprietary bootloader to userspace from DT
booted kernel?

Anyway, for supporting kexec (with passing ATAGs) it is needed to have
working /proc/atags file, right?

> It also sets a precedent: by adding this into DT, it is creating a new
> DT ABI as well, and we'll end up seeing dts files with an ATAG block
> patched into them.
>
> Are the ATAGs at a fixed address on the N900?

Yes, in board-rx51.c is:

.atag_offset = 0x100

and Nokia Bootloader (proprietary) store them to that address.

> Can that be handled in
> some kind of legacy file for the N900 which calls save_atags() on it, so
> we don't end up introducing yet more stuff that we have to maintain into
> the distant future? If not, what about copying a known working atag
> structure into a legacy file for the N900?
>

I already asked question if it is possible to read ATAGs from DT booted
kernel. And somebody (do not remember who) wrote to ML, that it is not
possible and it can be done in that uncompress code.

--
Pali Rohár
[email protected]

2015-07-13 13:19:10

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

* Pali Rohár <[email protected]> [150707 05:00]:
> On Tuesday 07 July 2015 12:32:13 Russell King - ARM Linux wrote:
> > On Mon, Jul 06, 2015 at 10:26:13PM +0200, Pali Rohár wrote:
> > > Legacy bootloaders can pass additional information for kernel or legacy
> > > userspace applications. When booting DT kernel then ATAGs structure is not
> > > more visible after running kernel uncompress code. This patch stores full
> > > ATAGs structure into DT "/chosen/linux,atags" entry, so kernel can later
> > > reuse it and export via /proc/atags to userspace.
> >
> > I think you need to go through your commit messages and improve them,
> > especially the ones with "TODO" in them. As long as there's still things
> > to be done, they're obviously not ready for merging.
> >
>
> I know, in cover letter email I wrote that documentation is not ready...
> I send patches for review and comments (like yours). I think it is still
> better to send something and mark it as incomplete. It could prevent to
> work on something which will be again rewritten...
>
> > Moreover, exporting the ATAGS is questionable, even _if_ there are non-
> > kexec programs making use of this. The ATAGs have _never_ been exported
> > to userspace when kexec disabled is the kernel - it was introduced for
> > kexec, and has always had this:
> >
> > config ATAGS_PROC
> > bool "Export atags in procfs"
> > depends on ATAGS && KEXEC
> > default y
> >
> > Now, the fact that someone decided to start using it is pretty sad,
> > because it means that if you disable KEXEC, userspace breaks. That's
> > not a kernel regression in any shape or form, because /proc/atags has
> > never been there without KEXEC enabled. That's a userspace bug, plain
> > and simple.
> >
> > Given that, I'm in two minds about whether to accept the last two
> > patches which make this more than just "for KEXEC use to enable a KEXEC
> > kernel to be booted."
> >
> > Had it been provided without the KEXEC conditional, then I don't have
> > a problem with these two patches.
> >
>
> I understand it. Nokia originally invented their own entries in /proc/
> which export needed ATAGs from kernel in human-readable form, but all
> those entries were non-standard and specific for Nokia's kernels.
>
> Do you have some other idea how to provide ATAGs information created
> dynamically by legacy closed proprietary bootloader to userspace from DT
> booted kernel?
>
> Anyway, for supporting kexec (with passing ATAGs) it is needed to have
> working /proc/atags file, right?

Yeah I think that since we already have it in /proc, we should just
support it. And keep it behind CONFIG_KEXEC and CONFIG_ARM_APPENDED_DTB
and hope we don't find other users for it.. Then reconsider the Kconfig
dependencies if we do find other users.

> > It also sets a precedent: by adding this into DT, it is creating a new
> > DT ABI as well, and we'll end up seeing dts files with an ATAG block
> > patched into them.
> >
> > Are the ATAGs at a fixed address on the N900?
>
> Yes, in board-rx51.c is:
>
> .atag_offset = 0x100
>
> and Nokia Bootloader (proprietary) store them to that address.
>
> > Can that be handled in
> > some kind of legacy file for the N900 which calls save_atags() on it, so
> > we don't end up introducing yet more stuff that we have to maintain into
> > the distant future? If not, what about copying a known working atag
> > structure into a legacy file for the N900?
>
> I already asked question if it is possible to read ATAGs from DT booted
> kernel. And somebody (do not remember who) wrote to ML, that it is not
> possible and it can be done in that uncompress code.

I guess the other option would be to keep the raw ATAG area reserved,
and only initialize /proc/atags from a board specific initcall.
But I think that would complicate the already fragile uncompress
relocation code even further?

Regards,

Tony

2015-11-05 11:40:51

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Tuesday 13 October 2015 16:37:46 Pali Rohár wrote:
> On Monday 12 October 2015 13:45:09 Tony Lindgren wrote:
> > * Pali Rohár <[email protected]> [151012 13:29]:
> > > On Monday 12 October 2015 22:16:40 Tony Lindgren wrote:
> > > >
> > > > Pali, any news on posting an updated series with the comments
> > > > addressed in this thread? It seems that we all pretty much agree
> > > > what needs to be done.
> > >
> > > Tony, I'm not really sure what to do. Just wrap 4 and 5 patches into
> > > CONFIG_KEXEC? Or something more?
> >
> > Well for most part your patches are fine, I think there were some
> > minor comments on the series.
> >
> > For the CONFIG_KEXEC dependency, we should just keep the existing
> > behavior and keep /proc/atags behind CONFIG_KEXEC. That's all
> > I believe :)
> >
> > Regards,
> >
> > Tony
> >
> >
>
> Ok. I will add CONFIG_KEXEC into atag patches.
>
> And there is missing documentation for these two new DT properties
> (marked as TODO in commit messages). Where to put them?
>

Tony (or somebody else) ^^^

--
Pali Rohár
[email protected]

2015-11-05 16:18:07

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

* Pali Rohár <[email protected]> [151105 03:41]:
> On Tuesday 13 October 2015 16:37:46 Pali Rohár wrote:
> > On Monday 12 October 2015 13:45:09 Tony Lindgren wrote:
> > > * Pali Rohár <[email protected]> [151012 13:29]:
> > > > On Monday 12 October 2015 22:16:40 Tony Lindgren wrote:
> > > > >
> > > > > Pali, any news on posting an updated series with the comments
> > > > > addressed in this thread? It seems that we all pretty much agree
> > > > > what needs to be done.
> > > >
> > > > Tony, I'm not really sure what to do. Just wrap 4 and 5 patches into
> > > > CONFIG_KEXEC? Or something more?
> > >
> > > Well for most part your patches are fine, I think there were some
> > > minor comments on the series.
> > >
> > > For the CONFIG_KEXEC dependency, we should just keep the existing
> > > behavior and keep /proc/atags behind CONFIG_KEXEC. That's all
> > > I believe :)
> > >
> > > Regards,
> > >
> > > Tony
> > >
> > >
> >
> > Ok. I will add CONFIG_KEXEC into atag patches.
> >
> > And there is missing documentation for these two new DT properties
> > (marked as TODO in commit messages). Where to put them?
> >
>
> Tony (or somebody else) ^^^

How about Documentation/devicetree/bindings/arm/atags.txt?

Regards,

Tony

2015-11-12 01:10:52

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

Adding devicetree list.

Thread starts at
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/354459.html

On 11/5/2015 8:17 AM, Tony Lindgren wrote:
> * Pali Rohár <[email protected]> [151105 03:41]:
>> On Tuesday 13 October 2015 16:37:46 Pali Rohár wrote:
>>> On Monday 12 October 2015 13:45:09 Tony Lindgren wrote:
>>>> * Pali Rohár <[email protected]> [151012 13:29]:
>>>>> On Monday 12 October 2015 22:16:40 Tony Lindgren wrote:
>>>>>>
>>>>>> Pali, any news on posting an updated series with the comments
>>>>>> addressed in this thread? It seems that we all pretty much agree
>>>>>> what needs to be done.

I'm not real happy with the concept of patches 4 and 5 in this series.
My concern is that those two patches are using the FDT as a transport
mechanism for a binary blob (the atags object).

Patches 1 and 2 do follow the spirit of atags_to_fdt() since an
atags kernel already may set system_rev from an atag.

-Frank


>>>>>
>>>>> Tony, I'm not really sure what to do. Just wrap 4 and 5 patches into
>>>>> CONFIG_KEXEC? Or something more?
>>>>
>>>> Well for most part your patches are fine, I think there were some
>>>> minor comments on the series.
>>>>
>>>> For the CONFIG_KEXEC dependency, we should just keep the existing
>>>> behavior and keep /proc/atags behind CONFIG_KEXEC. That's all
>>>> I believe :)
>>>>
>>>> Regards,
>>>>
>>>> Tony
>>>>
>>>>
>>>
>>> Ok. I will add CONFIG_KEXEC into atag patches.
>>>
>>> And there is missing documentation for these two new DT properties
>>> (marked as TODO in commit messages). Where to put them?
>>>
>>
>> Tony (or somebody else) ^^^
>
> How about Documentation/devicetree/bindings/arm/atags.txt?
>
> Regards,
>
> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2015-11-22 06:52:35

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Wed 2015-11-11 17:10:46, Frank Rowand wrote:
> Adding devicetree list.
>
> Thread starts at
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/354459.html
>
> On 11/5/2015 8:17 AM, Tony Lindgren wrote:
> > * Pali Roh?r <[email protected]> [151105 03:41]:
> >> On Tuesday 13 October 2015 16:37:46 Pali Roh?r wrote:
> >>> On Monday 12 October 2015 13:45:09 Tony Lindgren wrote:
> >>>> * Pali Roh?r <[email protected]> [151012 13:29]:
> >>>>> On Monday 12 October 2015 22:16:40 Tony Lindgren wrote:
> >>>>>>
> >>>>>> Pali, any news on posting an updated series with the comments
> >>>>>> addressed in this thread? It seems that we all pretty much agree
> >>>>>> what needs to be done.
>
> I'm not real happy with the concept of patches 4 and 5 in this series.
> My concern is that those two patches are using the FDT as a transport
> mechanism for a binary blob (the atags object).

Umm. Ok. Do you have alternative proposal that works for everyone?

I mean. This discussion was going for quite a long time, and it would
be nice to have some solution... patch proposal... something.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-11-23 14:45:51

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Sunday 22 November 2015 07:51:46 Pavel Machek wrote:
> On Wed 2015-11-11 17:10:46, Frank Rowand wrote:
> > Adding devicetree list.
> >
> > Thread starts at
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/354459.html
> >
> > On 11/5/2015 8:17 AM, Tony Lindgren wrote:
> > > * Pali Rohár <[email protected]> [151105 03:41]:
> > >> On Tuesday 13 October 2015 16:37:46 Pali Rohár wrote:
> > >>> On Monday 12 October 2015 13:45:09 Tony Lindgren wrote:
> > >>>> * Pali Rohár <[email protected]> [151012 13:29]:
> > >>>>> On Monday 12 October 2015 22:16:40 Tony Lindgren wrote:
> > >>>>>>
> > >>>>>> Pali, any news on posting an updated series with the comments
> > >>>>>> addressed in this thread? It seems that we all pretty much agree
> > >>>>>> what needs to be done.
> >
> > I'm not real happy with the concept of patches 4 and 5 in this series.
> > My concern is that those two patches are using the FDT as a transport
> > mechanism for a binary blob (the atags object).
>
> Umm. Ok. Do you have alternative proposal that works for everyone?
>
> I mean. This discussion was going for quite a long time, and it would
> be nice to have some solution... patch proposal... something.
> Pavel

Yes, discussion is going for a long time! So should I spend time for
adding documentation to my solution (this is last one thing which is
missing)? Or my solution is wrong and somebody else will propose new?
I do not want to spend time on something which will be rejected and
discarded.

--
Pali Rohár
[email protected]

2015-11-25 18:16:59

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

* Pali Rohár <[email protected]> [151123 06:46]:
> On Sunday 22 November 2015 07:51:46 Pavel Machek wrote:
> > On Wed 2015-11-11 17:10:46, Frank Rowand wrote:
> > > Adding devicetree list.
> > >
> > > Thread starts at
> > > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/354459.html
> > >
> > > On 11/5/2015 8:17 AM, Tony Lindgren wrote:
> > > > * Pali Rohár <[email protected]> [151105 03:41]:
> > > >> On Tuesday 13 October 2015 16:37:46 Pali Rohár wrote:
> > > >>> On Monday 12 October 2015 13:45:09 Tony Lindgren wrote:
> > > >>>> * Pali Rohár <[email protected]> [151012 13:29]:
> > > >>>>> On Monday 12 October 2015 22:16:40 Tony Lindgren wrote:
> > > >>>>>>
> > > >>>>>> Pali, any news on posting an updated series with the comments
> > > >>>>>> addressed in this thread? It seems that we all pretty much agree
> > > >>>>>> what needs to be done.
> > >
> > > I'm not real happy with the concept of patches 4 and 5 in this series.
> > > My concern is that those two patches are using the FDT as a transport
> > > mechanism for a binary blob (the atags object).
> >
> > Umm. Ok. Do you have alternative proposal that works for everyone?
> >
> > I mean. This discussion was going for quite a long time, and it would
> > be nice to have some solution... patch proposal... something.
> > Pavel
>
> Yes, discussion is going for a long time! So should I spend time for
> adding documentation to my solution (this is last one thing which is
> missing)? Or my solution is wrong and somebody else will propose new?
> I do not want to spend time on something which will be rejected and
> discarded.

At least I don't have better solutions in mind.

Regards,

Tony

2015-11-25 19:49:50

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Wednesday 25 November 2015 10:16:44 Tony Lindgren wrote:
> * Pali Roh?r <[email protected]> [151123 06:46]:
> > On Sunday 22 November 2015 07:51:46 Pavel Machek wrote:
> > > On Wed 2015-11-11 17:10:46, Frank Rowand wrote:
> > > > Adding devicetree list.
> > > >
> > > > Thread starts at
> > > > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/354459.html
> > > >
> > > > On 11/5/2015 8:17 AM, Tony Lindgren wrote:
> > > > > * Pali Roh?r <[email protected]> [151105 03:41]:
> > > > >> On Tuesday 13 October 2015 16:37:46 Pali Roh?r wrote:
> > > > >>> On Monday 12 October 2015 13:45:09 Tony Lindgren wrote:
> > > > >>>> * Pali Roh?r <[email protected]> [151012 13:29]:
> > > > >>>>> On Monday 12 October 2015 22:16:40 Tony Lindgren wrote:
> > > > >>>>>>
> > > > >>>>>> Pali, any news on posting an updated series with the comments
> > > > >>>>>> addressed in this thread? It seems that we all pretty much agree
> > > > >>>>>> what needs to be done.
> > > >
> > > > I'm not real happy with the concept of patches 4 and 5 in this series.
> > > > My concern is that those two patches are using the FDT as a transport
> > > > mechanism for a binary blob (the atags object).
> > >
> > > Umm. Ok. Do you have alternative proposal that works for everyone?
> > >
> > > I mean. This discussion was going for quite a long time, and it would
> > > be nice to have some solution... patch proposal... something.
> > > Pavel
> >
> > Yes, discussion is going for a long time! So should I spend time for
> > adding documentation to my solution (this is last one thing which is
> > missing)? Or my solution is wrong and somebody else will propose new?
> > I do not want to spend time on something which will be rejected and
> > discarded.
>
> At least I don't have better solutions in mind.

I would be happier if we could restrict this as much as possible to the
boards that need it, as an opt-in. That way it doesn't become an ABI
for people that don't already rely in this information. How about
adding a check the code adds the linux,atags property to do it
only for a whitelist of board numbers?

Arnd

2015-11-25 21:03:22

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

* Arnd Bergmann <[email protected]> [151125 11:50]:
> On Wednesday 25 November 2015 10:16:44 Tony Lindgren wrote:
> > * Pali Rohár <[email protected]> [151123 06:46]:
> > > On Sunday 22 November 2015 07:51:46 Pavel Machek wrote:
> > > > On Wed 2015-11-11 17:10:46, Frank Rowand wrote:
> > > > > Adding devicetree list.
> > > > >
> > > > > Thread starts at
> > > > > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/354459.html
> > > > >
> > > > > On 11/5/2015 8:17 AM, Tony Lindgren wrote:
> > > > > > * Pali Rohár <[email protected]> [151105 03:41]:
> > > > > >> On Tuesday 13 October 2015 16:37:46 Pali Rohár wrote:
> > > > > >>> On Monday 12 October 2015 13:45:09 Tony Lindgren wrote:
> > > > > >>>> * Pali Rohár <[email protected]> [151012 13:29]:
> > > > > >>>>> On Monday 12 October 2015 22:16:40 Tony Lindgren wrote:
> > > > > >>>>>>
> > > > > >>>>>> Pali, any news on posting an updated series with the comments
> > > > > >>>>>> addressed in this thread? It seems that we all pretty much agree
> > > > > >>>>>> what needs to be done.
> > > > >
> > > > > I'm not real happy with the concept of patches 4 and 5 in this series.
> > > > > My concern is that those two patches are using the FDT as a transport
> > > > > mechanism for a binary blob (the atags object).
> > > >
> > > > Umm. Ok. Do you have alternative proposal that works for everyone?
> > > >
> > > > I mean. This discussion was going for quite a long time, and it would
> > > > be nice to have some solution... patch proposal... something.
> > > > Pavel
> > >
> > > Yes, discussion is going for a long time! So should I spend time for
> > > adding documentation to my solution (this is last one thing which is
> > > missing)? Or my solution is wrong and somebody else will propose new?
> > > I do not want to spend time on something which will be rejected and
> > > discarded.
> >
> > At least I don't have better solutions in mind.
>
> I would be happier if we could restrict this as much as possible to the
> boards that need it, as an opt-in. That way it doesn't become an ABI
> for people that don't already rely in this information. How about
> adding a check the code adds the linux,atags property to do it
> only for a whitelist of board numbers?

Or populate /proc/atags only for the ones that need it from machine
specific init_early?

Regards,

Tony

2015-11-25 21:30:44

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Wednesday 25 November 2015 13:03:10 Tony Lindgren wrote:
> * Arnd Bergmann <[email protected]> [151125 11:50]:
> > On Wednesday 25 November 2015 10:16:44 Tony Lindgren wrote:
> > > At least I don't have better solutions in mind.
> >
> > I would be happier if we could restrict this as much as possible to the
> > boards that need it, as an opt-in. That way it doesn't become an ABI
> > for people that don't already rely in this information. How about
> > adding a check the code adds the linux,atags property to do it
> > only for a whitelist of board numbers?
>
> Or populate /proc/atags only for the ones that need it from machine
> specific init_early?

That would also address my main concern about /proc/atags, but still
leave the atags in /proc/device-tree/chosen/linux,atags, and it would
be bad if someone who currently uses /proc/atags changes their code
to use the other file instead of finding a proper solution.

Arnd

2015-11-25 21:44:35

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Wednesday 25 November 2015 22:29:53 Arnd Bergmann wrote:
> On Wednesday 25 November 2015 13:03:10 Tony Lindgren wrote:
> > * Arnd Bergmann <[email protected]> [151125 11:50]:
> > > On Wednesday 25 November 2015 10:16:44 Tony Lindgren wrote:
> > > > At least I don't have better solutions in mind.
> > >
> > > I would be happier if we could restrict this as much as possible
> > > to the boards that need it, as an opt-in. That way it doesn't
> > > become an ABI for people that don't already rely in this
> > > information. How about adding a check the code adds the
> > > linux,atags property to do it only for a whitelist of board
> > > numbers?
> >
> > Or populate /proc/atags only for the ones that need it from machine
> > specific init_early?
>
> That would also address my main concern about /proc/atags, but still
> leave the atags in /proc/device-tree/chosen/linux,atags, and it would
> be bad if someone who currently uses /proc/atags changes their code
> to use the other file instead of finding a proper solution.
>
> Arnd

Arnd, my question about proper solution reminds... Proprietary
bootloader which cannot be replaced (e.g. it is signed or do unknown
magic) provides information to booted kernel via custom specific ATAGs
fields. How userspace could properly read those custom information from
bootloader?

--
Pali Rohár
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2015-11-25 21:51:47

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Wednesday 25 November 2015 22:44:28 Pali Roh?r wrote:
>
> Arnd, my question about proper solution reminds... Proprietary
> bootloader which cannot be replaced (e.g. it is signed or do unknown
> magic) provides information to booted kernel via custom specific ATAGs
> fields. How userspace could properly read those custom information from
> bootloader?

The typical solution for nonstandard bootloaders is to have a boot wrapper
like the one from https://github.com/zonque/pxa-impedance-matcher that
translates whatever information we have at the bootloader level into
DT properties.

As I understand, the reason we are not doing that here is that we also
have proprietary user space that we can't fix to look in a different
place, i.e. the interface is between the bootloader and some user
binary, not bootloader to kernel.

Arnd

2015-11-25 22:01:06

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Wednesday 25 November 2015 22:51:00 Arnd Bergmann wrote:
> On Wednesday 25 November 2015 22:44:28 Pali Rohár wrote:
> > Arnd, my question about proper solution reminds... Proprietary
> > bootloader which cannot be replaced (e.g. it is signed or do
> > unknown magic) provides information to booted kernel via custom
> > specific ATAGs fields. How userspace could properly read those
> > custom information from bootloader?
>
> The typical solution for nonstandard bootloaders is to have a boot
> wrapper like the one from
> https://github.com/zonque/pxa-impedance-matcher that translates
> whatever information we have at the bootloader level into DT
> properties.
>

Ok. So there is no better solution. With some hacks we can use U-Boot as
3rd stage bootloader. But this is not useful for debugging or
developing...

Ideal "wrapper" solution would be to compile wrapper and linux zImage
and then glue them together to one binary. Something like internal linux
uncompress code which translate atags to dt.

> As I understand, the reason we are not doing that here is that we
> also have proprietary user space that we can't fix to look in a
> different place, i.e. the interface is between the bootloader and
> some user binary, not bootloader to kernel.
>

Yes, proprietary/closed applications are problems which we cannot fix
(without rewriting them).

New applications could use new "proper" interface. But without that
interface we cannot do that.

--
Pali Rohár
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2015-11-26 04:19:38

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On 11/25/2015 1:03 PM, Tony Lindgren wrote:
> * Arnd Bergmann <[email protected]> [151125 11:50]:
>> On Wednesday 25 November 2015 10:16:44 Tony Lindgren wrote:
>>> * Pali Rohár <[email protected]> [151123 06:46]:
>>>> On Sunday 22 November 2015 07:51:46 Pavel Machek wrote:
>>>>> On Wed 2015-11-11 17:10:46, Frank Rowand wrote:
>>>>>> Adding devicetree list.
>>>>>>
>>>>>> Thread starts at
>>>>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/354459.html
>>>>>>
>>>>>> On 11/5/2015 8:17 AM, Tony Lindgren wrote:
>>>>>>> * Pali Rohár <[email protected]> [151105 03:41]:
>>>>>>>> On Tuesday 13 October 2015 16:37:46 Pali Rohár wrote:
>>>>>>>>> On Monday 12 October 2015 13:45:09 Tony Lindgren wrote:
>>>>>>>>>> * Pali Rohár <[email protected]> [151012 13:29]:
>>>>>>>>>>> On Monday 12 October 2015 22:16:40 Tony Lindgren wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Pali, any news on posting an updated series with the comments
>>>>>>>>>>>> addressed in this thread? It seems that we all pretty much agree
>>>>>>>>>>>> what needs to be done.
>>>>>>
>>>>>> I'm not real happy with the concept of patches 4 and 5 in this series.
>>>>>> My concern is that those two patches are using the FDT as a transport
>>>>>> mechanism for a binary blob (the atags object).
>>>>>
>>>>> Umm. Ok. Do you have alternative proposal that works for everyone?
>>>>>
>>>>> I mean. This discussion was going for quite a long time, and it would
>>>>> be nice to have some solution... patch proposal... something.
>>>>> Pavel
>>>>
>>>> Yes, discussion is going for a long time! So should I spend time for
>>>> adding documentation to my solution (this is last one thing which is
>>>> missing)? Or my solution is wrong and somebody else will propose new?
>>>> I do not want to spend time on something which will be rejected and
>>>> discarded.
>>>
>>> At least I don't have better solutions in mind.
>>
>> I would be happier if we could restrict this as much as possible to the
>> boards that need it, as an opt-in. That way it doesn't become an ABI

The feature (in whatever form it takes) should be definitely be highly
restricted and marked as deprecated.

>> for people that don't already rely in this information. How about
>> adding a check the code adds the linux,atags property to do it
>> only for a whitelist of board numbers?
>
> Or populate /proc/atags only for the ones that need it from machine
> specific init_early?

This is circling back to the first comment from Russell King where
he suggested a legacy file for the N900 which calls save_atags():

Are the ATAGs at a fixed address on the N900? Can that be handled in
some kind of legacy file for the N900 which calls save_atags() on it, so
we don't end up introducing yet more stuff that we have to maintain into
the distant future? If not, what about copying a known working atag
structure into a legacy file for the N900?

It seems to me that patches 1, 2, 4, and 5 could be replaced by this
approach.

Regards,

Frank

2015-11-26 09:08:13

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Wednesday 25 November 2015 20:19:21 Frank Rowand wrote:
> > Or populate /proc/atags only for the ones that need it from machine
> > specific init_early?
>
> This is circling back to the first comment from Russell King where
> he suggested a legacy file for the N900 which calls save_atags():
>
> Are the ATAGs at a fixed address on the N900? Can that be handled in
> some kind of legacy file for the N900 which calls save_atags() on it, so
> we don't end up introducing yet more stuff that we have to maintain into
> the distant future? If not, what about copying a known working atag
> structure into a legacy file for the N900?
>
> It seems to me that patches 1, 2, 4, and 5 could be replaced by this
> approach.

Hi Frank, in this case I will ask my question again: It is possible to
read atags from that legacy file. And if yes how? I was not thinking
about this approach because somebody in past wrote that this is not
possible...

--
Pali Rohár
[email protected]

2015-11-26 20:39:44

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

* Pali Rohár <[email protected]> [151126 01:08]:
> On Wednesday 25 November 2015 20:19:21 Frank Rowand wrote:
> > > Or populate /proc/atags only for the ones that need it from machine
> > > specific init_early?
> >
> > This is circling back to the first comment from Russell King where
> > he suggested a legacy file for the N900 which calls save_atags():
> >
> > Are the ATAGs at a fixed address on the N900? Can that be handled in
> > some kind of legacy file for the N900 which calls save_atags() on it, so
> > we don't end up introducing yet more stuff that we have to maintain into
> > the distant future? If not, what about copying a known working atag
> > structure into a legacy file for the N900?
> >
> > It seems to me that patches 1, 2, 4, and 5 could be replaced by this
> > approach.
>
> Hi Frank, in this case I will ask my question again: It is possible to
> read atags from that legacy file. And if yes how? I was not thinking
> about this approach because somebody in past wrote that this is not
> possible...

Just to explore options.. How about make a minimal device driver that
just loads the atags blob from /lib/firmware and then shows it in
/proc/atags? Of course some checking on the atags should be done by
the driver..

That would work as long as the kernel no longer needs it.

Regards,

Tony

2015-11-26 21:13:00

by Ivaylo Dimitrov

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry



On 26.11.2015 22:39, Tony Lindgren wrote:

> Just to explore options.. How about make a minimal device driver that
> just loads the atags blob from /lib/firmware and then shows it in
> /proc/atags? Of course some checking on the atags should be done by
> the driver..
>

What is the chance for such a driver to be accepted upstream? As IIRC
the current situation is because similar driver was rejected. Might be
wrong as well, it was about 2-3 years ago.

Regards,
Ivo

2015-11-27 08:38:44

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Thursday 26 November 2015 12:39:30 Tony Lindgren wrote:
> Just to explore options.. How about make a minimal device driver that
> just loads the atags blob from /lib/firmware and then shows it in
> /proc/atags? Of course some checking on the atags should be done by
> the driver..

And who can dynamically create that blob file in /lib/firmware? If
kernel does not export those atags (somehow) from bootloader, then
userspace is not able to create that blob... cyclic problem.

So no, problem is that bootloader provides via custom atags dynamic
information like: boot reason (how was device started, by rtc alarm? by
reboot? by usb charger? by power button?), boot mode (should be enter
into firmware update mode?, is this normal boot mode?), ... and those
information are needed for some proprietary software (e.g. firmware
upgrade/flash) but also by any other open source applications (based on
usb charger we enter different runlevel -- for just only charging
device).

--
Pali Rohár
[email protected]

Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

Hi

On Fri, Nov 27, 2015 at 9:38 AM, Pali Rohár <[email protected]> wrote:
> On Thursday 26 November 2015 12:39:30 Tony Lindgren wrote:
>> Just to explore options.. How about make a minimal device driver that
>> just loads the atags blob from /lib/firmware and then shows it in
>> /proc/atags? Of course some checking on the atags should be done by
>> the driver..
>
> And who can dynamically create that blob file in /lib/firmware? If
> kernel does not export those atags (somehow) from bootloader, then
> userspace is not able to create that blob... cyclic problem.
>

Are those atags from bootloader fix or they change device by device. If they
are fixed they can be included in some firmware and get from the disk.

Michael

> So no, problem is that bootloader provides via custom atags dynamic
> information like: boot reason (how was device started, by rtc alarm? by
> reboot? by usb charger? by power button?), boot mode (should be enter
> into firmware update mode?, is this normal boot mode?), ... and those
> information are needed for some proprietary software (e.g. firmware
> upgrade/flash) but also by any other open source applications (based on
> usb charger we enter different runlevel -- for just only charging
> device).
>
> --
> Pali Rohár
> [email protected]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

Hi

On Fri, Nov 27, 2015 at 9:44 AM, Michael Trimarchi
<[email protected]> wrote:
> Hi
>
> On Fri, Nov 27, 2015 at 9:38 AM, Pali Rohár <[email protected]> wrote:
>> On Thursday 26 November 2015 12:39:30 Tony Lindgren wrote:
>>> Just to explore options.. How about make a minimal device driver that
>>> just loads the atags blob from /lib/firmware and then shows it in
>>> /proc/atags? Of course some checking on the atags should be done by
>>> the driver..
>>
>> And who can dynamically create that blob file in /lib/firmware? If
>> kernel does not export those atags (somehow) from bootloader, then
>> userspace is not able to create that blob... cyclic problem.
>>
>
> Are those atags from bootloader fix or they change device by device. If they
> are fixed they can be included in some firmware and get from the disk.
>

Sorry, miss the second part of email ;)

Michael

> Michael
>
>> So no, problem is that bootloader provides via custom atags dynamic
>> information like: boot reason (how was device started, by rtc alarm? by
>> reboot? by usb charger? by power button?), boot mode (should be enter
>> into firmware update mode?, is this normal boot mode?), ... and those
>> information are needed for some proprietary software (e.g. firmware
>> upgrade/flash) but also by any other open source applications (based on
>> usb charger we enter different runlevel -- for just only charging
>> device).
>>
>> --
>> Pali Rohár
>> [email protected]
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
| Michael Nazzareno Trimarchi Amarula Solutions BV |
| COO - Founder Cruquiuskade 47 |
| +31(0)851119172 Amsterdam 1018 AM NL |
| [`as] http://www.amarulasolutions.com |

2015-11-27 13:27:49

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Thu, Nov 26, 2015 at 10:07:39AM +0100, Pali Roh?r wrote:
> On Wednesday 25 November 2015 20:19:21 Frank Rowand wrote:
> > > Or populate /proc/atags only for the ones that need it from machine
> > > specific init_early?
> >
> > This is circling back to the first comment from Russell King where
> > he suggested a legacy file for the N900 which calls save_atags():
> >
> > Are the ATAGs at a fixed address on the N900? Can that be handled in
> > some kind of legacy file for the N900 which calls save_atags() on it, so
> > we don't end up introducing yet more stuff that we have to maintain into
> > the distant future? If not, what about copying a known working atag
> > structure into a legacy file for the N900?
> >
> > It seems to me that patches 1, 2, 4, and 5 could be replaced by this
> > approach.
>
> Hi Frank, in this case I will ask my question again: It is possible to
> read atags from that legacy file. And if yes how? I was not thinking
> about this approach because somebody in past wrote that this is not
> possible...

It is possible to redirect any program to open any other file. You can
do it via a LD preload, and intercepting the open(), and possibly the
read() calls if you want to do something more fancy. The down-side is
that you have to arrange for the preloaded object to be used by the
linker, and the additional overhead it places on the intercepted
functions.

Eg,

openatags.c:

#define open libc_open
#include <fcntl.h>
#undef open
#include <string.h>

int open(const char *pathname, int flags, mode_t mode)
{
static int (*old_open)(const char *pathname, int flags, mode_t mode);

if (strcmp(pathname, "/proc/atags") == 0)
pathname = "/tmp/my-atags";

if (!old_open)
old_open = dlsym(RTLD_NEXT, "open");

return old_open(pathname, flags, mode);
}

Build the above (untested) with:
gcc -O2 -o openatags.o -c openatags.c
gcc -shared -o openatags.so openatags.o -ldl

Now, when running one of these programs, you can test it with:
LD_PRELOAD=openatags.so /name/of/program

You could also list the full pathname to openatags.so in /etc/ld.so.preload,
but test it first, because it will always be used by the linker in that
case, and you wouldn't want normal commands to misbehave.

Note that putting it in /etc/ld.so.preload will also have the effect that
cat /proc/atags will also get redirected to /tmp/my-atags too.

--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

2015-11-27 14:51:42

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

* Pali Rohár <[email protected]> [151127 00:39]:
> On Thursday 26 November 2015 12:39:30 Tony Lindgren wrote:
> > Just to explore options.. How about make a minimal device driver that
> > just loads the atags blob from /lib/firmware and then shows it in
> > /proc/atags? Of course some checking on the atags should be done by
> > the driver..
>
> And who can dynamically create that blob file in /lib/firmware? If
> kernel does not export those atags (somehow) from bootloader, then
> userspace is not able to create that blob... cyclic problem.
>
> So no, problem is that bootloader provides via custom atags dynamic
> information like: boot reason (how was device started, by rtc alarm? by
> reboot? by usb charger? by power button?), boot mode (should be enter
> into firmware update mode?, is this normal boot mode?), ... and those
> information are needed for some proprietary software (e.g. firmware
> upgrade/flash) but also by any other open source applications (based on
> usb charger we enter different runlevel -- for just only charging
> device).

OK if the ATAGs are not static then naturally we can't create the blobs
then.

Regards,

Tony

2015-11-27 19:52:04

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Fri, Nov 27, 2015 at 01:27:23PM +0000, Russell King - ARM Linux wrote:
> It is possible to redirect any program to open any other file. You can
> do it via a LD preload, and intercepting the open(), and possibly the
> read() calls if you want to do something more fancy. The down-side is
> that you have to arrange for the preloaded object to be used by the
> linker, and the additional overhead it places on the intercepted
> functions.

Another idea if people don't like the preload idea.

We could create a zero-sized /proc/atags, and then use a bind mount in
userspace to bind some other file containing the required information
on top. That could even be the atag blob from /sys/firmware/whatever.
The N700 (or whatever platform needs it) could be responsible for
creating the zero-sized /proc/atags so that we don't have it everywhere.

--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

2015-11-27 21:07:16

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Friday 27 November 2015 19:51:48 Russell King - ARM Linux wrote:
> On Fri, Nov 27, 2015 at 01:27:23PM +0000, Russell King - ARM Linux wrote:
> > It is possible to redirect any program to open any other file. You can
> > do it via a LD preload, and intercepting the open(), and possibly the
> > read() calls if you want to do something more fancy. The down-side is
> > that you have to arrange for the preloaded object to be used by the
> > linker, and the additional overhead it places on the intercepted
> > functions.
>
> Another idea if people don't like the preload idea.
>
> We could create a zero-sized /proc/atags, and then use a bind mount in
> userspace to bind some other file containing the required information
> on top. That could even be the atag blob from /sys/firmware/whatever.
> The N700 (or whatever platform needs it) could be responsible for
> creating the zero-sized /proc/atags so that we don't have it everywhere.

I don't mind creating the /proc/atags compatibility hack from the kernel
for a DT based N700 kernel, as long as we limit it as much as we can
to the machines that need it. Leaving a board file for the N700 in place
that contains the procfs code (and not much more) seems reasonable
here, as we are talking about a board specific hack and the whole point
appears to be running unmodified user space.

Regarding how to get the data into the kernel in the first place, my
preferred choice would still be to have an intermediate bootloader
such as pxa-impedance-matcher, but I won't complain if others are
happy enough about putting it into the ATAGS compat code we already
have, as long as it's limited to the boards we know need it.

Arnd

2015-11-27 23:28:58

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Fri, 27 Nov 2015, Arnd Bergmann wrote:

> I don't mind creating the /proc/atags compatibility hack from the kernel
> for a DT based N700 kernel, as long as we limit it as much as we can
> to the machines that need it. Leaving a board file for the N700 in place
> that contains the procfs code (and not much more) seems reasonable
> here, as we are talking about a board specific hack and the whole point
> appears to be running unmodified user space.
>
> Regarding how to get the data into the kernel in the first place, my
> preferred choice would still be to have an intermediate bootloader
> such as pxa-impedance-matcher, but I won't complain if others are
> happy enough about putting it into the ATAGS compat code we already
> have, as long as it's limited to the boards we know need it.

Assuming you have a N700 board file for special procfs code, then why
not getting at the atags in memory where the bootloader has put them
directly from that same board file? This way it'll really be limited to
the board we know needs it and the special exception will be contained
to that one file. Amongst the machine specific hooks, there is one that
gets invoked early during boot before those atags are overwritten.


Nicolas

2015-11-28 04:06:12

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH 0/5] ATAGs to DT patches

On 7/6/2015 1:26 PM, Pali Rohár wrote:
> This patch series converts some legacy ATAGs into DT and provide them to
> userspace. It is needed for userspace applications which needs some
> informations from legacy bootloaders which are not DT compatible.
>
> Patch series is for now without DT documentation files. It was tested in
> QEMU for Nokia N900. File /proc/cpuinfo contains correct "Revision" line
> from ATAG_REVISION and /proc/atags contains full ATAGs structure.

Pali,

Is this needed just for the N900, or are there other devices that need it?
If so, which devices?

Thanks,

Frank

2015-11-28 05:55:36

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH 0/5] ATAGs to DT patches

On 11/27/2015 8:06 PM, Frank Rowand wrote:
> On 7/6/2015 1:26 PM, Pali Rohár wrote:
>> This patch series converts some legacy ATAGs into DT and provide them to
>> userspace. It is needed for userspace applications which needs some
>> informations from legacy bootloaders which are not DT compatible.
>>
>> Patch series is for now without DT documentation files. It was tested in
>> QEMU for Nokia N900. File /proc/cpuinfo contains correct "Revision" line
>> from ATAG_REVISION and /proc/atags contains full ATAGs structure.
>
> Pali,
>
> Is this needed just for the N900, or are there other devices that need it?
> If so, which devices?

Nevermind... I've re-read the earlier threads.

-Frank

2015-11-28 12:27:59

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Friday 27 November 2015 18:28:50 Nicolas Pitre wrote:
> On Fri, 27 Nov 2015, Arnd Bergmann wrote:
>
> > I don't mind creating the /proc/atags compatibility hack from the kernel
> > for a DT based N700 kernel, as long as we limit it as much as we can
> > to the machines that need it. Leaving a board file for the N700 in place
> > that contains the procfs code (and not much more) seems reasonable
> > here, as we are talking about a board specific hack and the whole point
> > appears to be running unmodified user space.
> >
> > Regarding how to get the data into the kernel in the first place, my
> > preferred choice would still be to have an intermediate bootloader
> > such as pxa-impedance-matcher, but I won't complain if others are
> > happy enough about putting it into the ATAGS compat code we already
> > have, as long as it's limited to the boards we know need it.
>
> Assuming you have a N700 board file for special procfs code, then why
> not getting at the atags in memory where the bootloader has put them
> directly from that same board file? This way it'll really be limited to
> the board we know needs it and the special exception will be contained
> to that one file. Amongst the machine specific hooks, there is one that
> gets invoked early during boot before those atags are overwritten.

I didn't realize this was possible, as we don't know the atags pointer
when we instead get a DTB pointer. However you are right: the board
file knows exactly that the atag_offset is 0x100, so we can grab it
from there, and that will make the implementation really easy and
contained to a single file that has access to the atags and that
can create the /proc/atags file for it.

Arnd

2015-11-28 12:33:50

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Fri, Nov 27, 2015 at 06:28:50PM -0500, Nicolas Pitre wrote:
> On Fri, 27 Nov 2015, Arnd Bergmann wrote:
>
> > I don't mind creating the /proc/atags compatibility hack from the kernel
> > for a DT based N700 kernel, as long as we limit it as much as we can
> > to the machines that need it. Leaving a board file for the N700 in place
> > that contains the procfs code (and not much more) seems reasonable
> > here, as we are talking about a board specific hack and the whole point
> > appears to be running unmodified user space.
> >
> > Regarding how to get the data into the kernel in the first place, my
> > preferred choice would still be to have an intermediate bootloader
> > such as pxa-impedance-matcher, but I won't complain if others are
> > happy enough about putting it into the ATAGS compat code we already
> > have, as long as it's limited to the boards we know need it.
>
> Assuming you have a N700 board file for special procfs code, then why
> not getting at the atags in memory where the bootloader has put them
> directly from that same board file? This way it'll really be limited to
> the board we know needs it and the special exception will be contained
> to that one file. Amongst the machine specific hooks, there is one that
> gets invoked early during boot before those atags are overwritten.

I've already suggested that.

--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

2015-11-28 12:55:16

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Sat, Nov 28, 2015 at 01:27:07PM +0100, Arnd Bergmann wrote:
> On Friday 27 November 2015 18:28:50 Nicolas Pitre wrote:
> > On Fri, 27 Nov 2015, Arnd Bergmann wrote:
> >
> > > I don't mind creating the /proc/atags compatibility hack from the kernel
> > > for a DT based N700 kernel, as long as we limit it as much as we can
> > > to the machines that need it. Leaving a board file for the N700 in place
> > > that contains the procfs code (and not much more) seems reasonable
> > > here, as we are talking about a board specific hack and the whole point
> > > appears to be running unmodified user space.
> > >
> > > Regarding how to get the data into the kernel in the first place, my
> > > preferred choice would still be to have an intermediate bootloader
> > > such as pxa-impedance-matcher, but I won't complain if others are
> > > happy enough about putting it into the ATAGS compat code we already
> > > have, as long as it's limited to the boards we know need it.
> >
> > Assuming you have a N700 board file for special procfs code, then why
> > not getting at the atags in memory where the bootloader has put them
> > directly from that same board file? This way it'll really be limited to
> > the board we know needs it and the special exception will be contained
> > to that one file. Amongst the machine specific hooks, there is one that
> > gets invoked early during boot before those atags are overwritten.
>
> I didn't realize this was possible, as we don't know the atags pointer
> when we instead get a DTB pointer. However you are right: the board
> file knows exactly that the atag_offset is 0x100, so we can grab it
> from there, and that will make the implementation really easy and
> contained to a single file that has access to the atags and that
> can create the /proc/atags file for it.

I've made several suggestions over the year or so that this problem has
been around, and solving this problem appears to be getting nowhere...
(because we _still_ have the problem today.) When the same suggestions
start to be made by other people, I think there's not much more that can
be done to help resolve the situation. It's probably time to walk away
from the problem, and let those who are supposedly motivated to use
these troublesome platforms just get on with it.

I'm not sure what Tony does at this point: if he rips out the non-DT
OMAP code, it'll cause a regression, but at the same time, it provides
additional motivation to get the problem resolved. I can quite well
see Pavel going off and whinging at Linus, Linus getting stressed at
us for intentionally breaking something that used to work, and telling
everyone that they shouldn't be working on the kernel, in his usual
friendly way.

So, I think if the non-DT OMAP stuff is getting in the way of further
OMAP development, then the only solution is to put pressure on those
who are holding it up: in other words, put pressure on those to get
this damned problem solved.

The only thing I can think of doing is to give the N900 people notice
that they're causing a problem here, explaining exactly why - maybe
explaining that it's been causing a problem however long it has and
that the only option is going to be to fork mainline and effectively
leave the code in mainline unmaintained because of this.

Then, of course, those who have caused this situation then get the fun
job of maintaining _all_ the OMAP code in mainline on their own, which
I think would bury them under such a huge mountain that the code would
end up being terminally broken, and ripe for deletion. At which point,
it'd make sense to merge the maintained fork back into mainline, which
of course wouldn't have the troublesome code platforms by that time. :)

Yes, it's not particularly nice, but I don't see this problem getting
resolved.

(Maybe this email will be enough to motivate the N900 users to sort this
out, but I suspect they'll prefer to spend time whinging and moaning at
me in email rather than doing what needs to be done and fixing the
problem.)

--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

2015-11-28 17:34:31

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Sat, 28 Nov 2015, Russell King - ARM Linux wrote:

> On Fri, Nov 27, 2015 at 06:28:50PM -0500, Nicolas Pitre wrote:
> > On Fri, 27 Nov 2015, Arnd Bergmann wrote:
> >
> > > I don't mind creating the /proc/atags compatibility hack from the kernel
> > > for a DT based N700 kernel, as long as we limit it as much as we can
> > > to the machines that need it. Leaving a board file for the N700 in place
> > > that contains the procfs code (and not much more) seems reasonable
> > > here, as we are talking about a board specific hack and the whole point
> > > appears to be running unmodified user space.
> > >
> > > Regarding how to get the data into the kernel in the first place, my
> > > preferred choice would still be to have an intermediate bootloader
> > > such as pxa-impedance-matcher, but I won't complain if others are
> > > happy enough about putting it into the ATAGS compat code we already
> > > have, as long as it's limited to the boards we know need it.
> >
> > Assuming you have a N700 board file for special procfs code, then why
> > not getting at the atags in memory where the bootloader has put them
> > directly from that same board file? This way it'll really be limited to
> > the board we know needs it and the special exception will be contained
> > to that one file. Amongst the machine specific hooks, there is one that
> > gets invoked early during boot before those atags are overwritten.
>
> I've already suggested that.

Good. And Arnd likes the idea too. So we might be converging at last
which is a good thing.


Nicolas

2015-11-28 21:02:35

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On 11/28/2015 9:34 AM, Nicolas Pitre wrote:
> On Sat, 28 Nov 2015, Russell King - ARM Linux wrote:
>
>> On Fri, Nov 27, 2015 at 06:28:50PM -0500, Nicolas Pitre wrote:
>>> On Fri, 27 Nov 2015, Arnd Bergmann wrote:
>>>
>>>> I don't mind creating the /proc/atags compatibility hack from the kernel
>>>> for a DT based N700 kernel, as long as we limit it as much as we can
>>>> to the machines that need it. Leaving a board file for the N700 in place
>>>> that contains the procfs code (and not much more) seems reasonable
>>>> here, as we are talking about a board specific hack and the whole point
>>>> appears to be running unmodified user space.
>>>>
>>>> Regarding how to get the data into the kernel in the first place, my
>>>> preferred choice would still be to have an intermediate bootloader
>>>> such as pxa-impedance-matcher, but I won't complain if others are
>>>> happy enough about putting it into the ATAGS compat code we already
>>>> have, as long as it's limited to the boards we know need it.
>>>
>>> Assuming you have a N700 board file for special procfs code, then why
>>> not getting at the atags in memory where the bootloader has put them
>>> directly from that same board file? This way it'll really be limited to
>>> the board we know needs it and the special exception will be contained
>>> to that one file. Amongst the machine specific hooks, there is one that
>>> gets invoked early during boot before those atags are overwritten.
>>
>> I've already suggested that.
>
> Good. And Arnd likes the idea too. So we might be converging at last
> which is a good thing.

It makes me happy too.

-Frank

2015-11-29 18:10:08

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Sat, Nov 28, 2015 at 12:34:23PM -0500, Nicolas Pitre wrote:
> Good. And Arnd likes the idea too. So we might be converging at last
> which is a good thing.

I disagree with the idea that there is convergence. There might be
convergence towards an idea, but... Here's a mail extract, from July
7th, from earlier in this very thread:

Pali:
> Me:
> > Are the ATAGs at a fixed address on the N900?
>
> Yes, in board-rx51.c is:
>
> .atag_offset = 0x100
>
> and Nokia Bootloader (proprietary) store them to that address.
>
> > Can that be handled in
> > some kind of legacy file for the N900 which calls save_atags() on it, so
> > we don't end up introducing yet more stuff that we have to maintain into
> > the distant future? If not, what about copying a known working atag
> > structure into a legacy file for the N900?
> >
>
> I already asked question if it is possible to read ATAGs from DT booted
> kernel. And somebody (do not remember who) wrote to ML, that it is not
> possible and it can be done in that uncompress code.

So you're converging on an idea that has already been rejected. That's
not a good thing, IMHO.

--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

2015-11-29 18:19:24

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Sunday 29 November 2015 19:09:39 Russell King - ARM Linux wrote:
> On Sat, Nov 28, 2015 at 12:34:23PM -0500, Nicolas Pitre wrote:
> > Good. And Arnd likes the idea too. So we might be converging at
> > last which is a good thing.
>
> I disagree with the idea that there is convergence. There might be
> convergence towards an idea, but... Here's a mail extract, from July
> 7th, from earlier in this very thread:
>
> Pali:
> > Me:
> > > Are the ATAGs at a fixed address on the N900?
> >
> > Yes, in board-rx51.c is:
> >
> > .atag_offset = 0x100
> >
> > and Nokia Bootloader (proprietary) store them to that address.
> >
> > > Can that be handled in
> > > some kind of legacy file for the N900 which calls save_atags() on
> > > it, so we don't end up introducing yet more stuff that we have
> > > to maintain into the distant future? If not, what about copying
> > > a known working atag structure into a legacy file for the N900?
> >
> > I already asked question if it is possible to read ATAGs from DT
> > booted kernel. And somebody (do not remember who) wrote to ML,
> > that it is not possible and it can be done in that uncompress
> > code.
>
> So you're converging on an idea that has already been rejected.
> That's not a good thing, IMHO.

Or in other case show that such implementation is possible...

--
Pali Rohár
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2015-11-29 23:13:28

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Sun, Nov 29, 2015 at 07:19:18PM +0100, Pali Roh?r wrote:
> On Sunday 29 November 2015 19:09:39 Russell King - ARM Linux wrote:
> > On Sat, Nov 28, 2015 at 12:34:23PM -0500, Nicolas Pitre wrote:
> > > Good. And Arnd likes the idea too. So we might be converging at
> > > last which is a good thing.
> >
> > I disagree with the idea that there is convergence. There might be
> > convergence towards an idea, but... Here's a mail extract, from July
> > 7th, from earlier in this very thread:
> >
> > Pali:
> > > Me:
> > > > Are the ATAGs at a fixed address on the N900?
> > >
> > > Yes, in board-rx51.c is:
> > >
> > > .atag_offset = 0x100
> > >
> > > and Nokia Bootloader (proprietary) store them to that address.
> > >
> > > > Can that be handled in
> > > > some kind of legacy file for the N900 which calls save_atags() on
> > > > it, so we don't end up introducing yet more stuff that we have
> > > > to maintain into the distant future? If not, what about copying
> > > > a known working atag structure into a legacy file for the N900?
> > >
> > > I already asked question if it is possible to read ATAGs from DT
> > > booted kernel. And somebody (do not remember who) wrote to ML,
> > > that it is not possible and it can be done in that uncompress
> > > code.
> >
> > So you're converging on an idea that has already been rejected.
> > That's not a good thing, IMHO.
>
> Or in other case show that such implementation is possible...

Only those with the problem can do that.

--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

2015-11-30 00:09:24

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Sun, 29 Nov 2015, Russell King - ARM Linux wrote:

> On Sat, Nov 28, 2015 at 12:34:23PM -0500, Nicolas Pitre wrote:
> > Good. And Arnd likes the idea too. So we might be converging at last
> > which is a good thing.
>
> I disagree with the idea that there is convergence. There might be
> convergence towards an idea, but... Here's a mail extract, from July
> 7th, from earlier in this very thread:
>
> Pali:
> > Me:
> > > Are the ATAGs at a fixed address on the N900?
> >
> > Yes, in board-rx51.c is:
> >
> > .atag_offset = 0x100
> >
> > and Nokia Bootloader (proprietary) store them to that address.
> >
> > > Can that be handled in
> > > some kind of legacy file for the N900 which calls save_atags() on it, so
> > > we don't end up introducing yet more stuff that we have to maintain into
> > > the distant future? If not, what about copying a known working atag
> > > structure into a legacy file for the N900?
> > >
> >
> > I already asked question if it is possible to read ATAGs from DT booted
> > kernel. And somebody (do not remember who) wrote to ML, that it is not
> > possible and it can be done in that uncompress code.

Who is that somebody? If ever it happened to be me then objection is
withdrawn. Otherwise that somebody should come forth and speak up again.

> So you're converging on an idea that has already been rejected. That's
> not a good thing, IMHO.

All the alternatives are worse and being rejected as well.

In that case we should settle on the idea that satisfies the most
people.


Nicolas

2015-11-30 00:15:19

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Monday 30 November 2015 01:09:17 Nicolas Pitre wrote:
> On Sun, 29 Nov 2015, Russell King - ARM Linux wrote:
> > On Sat, Nov 28, 2015 at 12:34:23PM -0500, Nicolas Pitre wrote:
> > > Good. And Arnd likes the idea too. So we might be converging at
> > > last which is a good thing.
> >
> > I disagree with the idea that there is convergence. There might be
> > convergence towards an idea, but... Here's a mail extract, from
> > July 7th, from earlier in this very thread:
> >
> > Pali:
> > > Me:
> > > > Are the ATAGs at a fixed address on the N900?
> > >
> > > Yes, in board-rx51.c is:
> > >
> > > .atag_offset = 0x100
> > >
> > > and Nokia Bootloader (proprietary) store them to that address.
> > >
> > > > Can that be handled in
> > > > some kind of legacy file for the N900 which calls save_atags()
> > > > on it, so we don't end up introducing yet more stuff that we
> > > > have to maintain into the distant future? If not, what about
> > > > copying a known working atag structure into a legacy file for
> > > > the N900?
> > >
> > > I already asked question if it is possible to read ATAGs from DT
> > > booted kernel. And somebody (do not remember who) wrote to ML,
> > > that it is not possible and it can be done in that uncompress
> > > code.
>
> Who is that somebody? If ever it happened to be me then objection is
> withdrawn. Otherwise that somebody should come forth and speak up
> again.
>

... do not remember ... this discussion were in more email threads and
takes more then one year... sorry but my memory is not excellent

--
Pali Rohár
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2015-11-30 15:24:05

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

* Pali Rohár <[email protected]> [151129 16:16]:
> On Monday 30 November 2015 01:09:17 Nicolas Pitre wrote:
> > On Sun, 29 Nov 2015, Russell King - ARM Linux wrote:
> > > On Sat, Nov 28, 2015 at 12:34:23PM -0500, Nicolas Pitre wrote:
> > > > Good. And Arnd likes the idea too. So we might be converging at
> > > > last which is a good thing.
> > >
> > > I disagree with the idea that there is convergence. There might be
> > > convergence towards an idea, but... Here's a mail extract, from
> > > July 7th, from earlier in this very thread:
> > >
> > > Pali:
> > > > Me:
> > > > > Are the ATAGs at a fixed address on the N900?
> > > >
> > > > Yes, in board-rx51.c is:
> > > >
> > > > .atag_offset = 0x100
> > > >
> > > > and Nokia Bootloader (proprietary) store them to that address.
> > > >
> > > > > Can that be handled in
> > > > > some kind of legacy file for the N900 which calls save_atags()
> > > > > on it, so we don't end up introducing yet more stuff that we
> > > > > have to maintain into the distant future? If not, what about
> > > > > copying a known working atag structure into a legacy file for
> > > > > the N900?
> > > >
> > > > I already asked question if it is possible to read ATAGs from DT
> > > > booted kernel. And somebody (do not remember who) wrote to ML,
> > > > that it is not possible and it can be done in that uncompress
> > > > code.
> >
> > Who is that somebody? If ever it happened to be me then objection is
> > withdrawn. Otherwise that somebody should come forth and speak up
> > again.
> >
>
> ... do not remember ... this discussion were in more email threads and
> takes more then one year... sorry but my memory is not excellent

Yes this certainly seems like the best solution. I think we got into
the atags-to-dt track as some of the atags are already being translated.

In this case there's no need to translate them AFAIK. You can just
parse them and have them available for the user space. So as long as
nothing trashes the atags at the atag_offset, you should be able to
call a function to parse them in the n900 specific init_machine.

Regards,

Tony

2015-11-30 15:39:39

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Monday 30 November 2015 07:23:53 Tony Lindgren wrote:
> * Pali Rohár <[email protected]> [151129 16:16]:
> > On Monday 30 November 2015 01:09:17 Nicolas Pitre wrote:
> > > On Sun, 29 Nov 2015, Russell King - ARM Linux wrote:
> > > > On Sat, Nov 28, 2015 at 12:34:23PM -0500, Nicolas Pitre wrote:
> > > > > Good. And Arnd likes the idea too. So we might be converging at
> > > > > last which is a good thing.
> > > >
> > > > I disagree with the idea that there is convergence. There might be
> > > > convergence towards an idea, but... Here's a mail extract, from
> > > > July 7th, from earlier in this very thread:
> > > >
> > > > Pali:
> > > > > Me:
> > > > > > Are the ATAGs at a fixed address on the N900?
> > > > >
> > > > > Yes, in board-rx51.c is:
> > > > >
> > > > > .atag_offset = 0x100
> > > > >
> > > > > and Nokia Bootloader (proprietary) store them to that address.
> > > > >
> > > > > > Can that be handled in
> > > > > > some kind of legacy file for the N900 which calls save_atags()
> > > > > > on it, so we don't end up introducing yet more stuff that we
> > > > > > have to maintain into the distant future? If not, what about
> > > > > > copying a known working atag structure into a legacy file for
> > > > > > the N900?
> > > > >
> > > > > I already asked question if it is possible to read ATAGs from DT
> > > > > booted kernel. And somebody (do not remember who) wrote to ML,
> > > > > that it is not possible and it can be done in that uncompress
> > > > > code.
> > >
> > > Who is that somebody? If ever it happened to be me then objection is
> > > withdrawn. Otherwise that somebody should come forth and speak up
> > > again.
> > >
> >
> > ... do not remember ... this discussion were in more email threads and
> > takes more then one year... sorry but my memory is not excellent
>
> Yes this certainly seems like the best solution. I think we got into
> the atags-to-dt track as some of the atags are already being translated.
>
> In this case there's no need to translate them AFAIK. You can just
> parse them and have them available for the user space. So as long as
> nothing trashes the atags at the atag_offset, you should be able to
> call a function to parse them in the n900 specific init_machine.
>
> Regards,
>
> Tony

In arch/arm/kernel/setup.c is function setup_arch() and it calls:

mdesc = setup_machine_fdt(__atags_pointer);
if (!mdesc)
mdesc = setup_machine_tags(__atags_pointer, __machine_arch_type);

So it looks like that on atags address is stored either atags structure
or DT structure... so it is truth kernel uncompress code put DT blob to
same offset where is expected atags structure? If yes, then this is
probably reason why atags cannot be read from booted DT kernel. Can
somebody with deep knowledge of DT/atags and uncompress code verify this?

--
Pali Rohár
[email protected]

2015-11-30 16:09:48

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Mon, 30 Nov 2015, Pali Rohár wrote:

> On Monday 30 November 2015 07:23:53 Tony Lindgren wrote:
> > * Pali Rohár <[email protected]> [151129 16:16]:
> > > On Monday 30 November 2015 01:09:17 Nicolas Pitre wrote:
> > > > On Sun, 29 Nov 2015, Russell King - ARM Linux wrote:
> > > > > On Sat, Nov 28, 2015 at 12:34:23PM -0500, Nicolas Pitre wrote:
> > > > > > Good. And Arnd likes the idea too. So we might be converging at
> > > > > > last which is a good thing.
> > > > >
> > > > > I disagree with the idea that there is convergence. There might be
> > > > > convergence towards an idea, but... Here's a mail extract, from
> > > > > July 7th, from earlier in this very thread:
> > > > >
> > > > > Pali:
> > > > > > Me:
> > > > > > > Are the ATAGs at a fixed address on the N900?
> > > > > >
> > > > > > Yes, in board-rx51.c is:
> > > > > >
> > > > > > .atag_offset = 0x100
> > > > > >
> > > > > > and Nokia Bootloader (proprietary) store them to that address.
> > > > > >
> > > > > > > Can that be handled in
> > > > > > > some kind of legacy file for the N900 which calls save_atags()
> > > > > > > on it, so we don't end up introducing yet more stuff that we
> > > > > > > have to maintain into the distant future? If not, what about
> > > > > > > copying a known working atag structure into a legacy file for
> > > > > > > the N900?
> > > > > >
> > > > > > I already asked question if it is possible to read ATAGs from DT
> > > > > > booted kernel. And somebody (do not remember who) wrote to ML,
> > > > > > that it is not possible and it can be done in that uncompress
> > > > > > code.
> > > >
> > > > Who is that somebody? If ever it happened to be me then objection is
> > > > withdrawn. Otherwise that somebody should come forth and speak up
> > > > again.
> > > >
> > >
> > > ... do not remember ... this discussion were in more email threads and
> > > takes more then one year... sorry but my memory is not excellent
> >
> > Yes this certainly seems like the best solution. I think we got into
> > the atags-to-dt track as some of the atags are already being translated.
> >
> > In this case there's no need to translate them AFAIK. You can just
> > parse them and have them available for the user space. So as long as
> > nothing trashes the atags at the atag_offset, you should be able to
> > call a function to parse them in the n900 specific init_machine.
> >
> > Regards,
> >
> > Tony
>
> In arch/arm/kernel/setup.c is function setup_arch() and it calls:
>
> mdesc = setup_machine_fdt(__atags_pointer);
> if (!mdesc)
> mdesc = setup_machine_tags(__atags_pointer, __machine_arch_type);
>
> So it looks like that on atags address is stored either atags structure
> or DT structure... so it is truth kernel uncompress code put DT blob to
> same offset where is expected atags structure?

No. It doesn't put it anywhere. Those functions read DT/ATAGs from the
passed address. But you know this address won't be the one you want for
the legacy ATAGs.

What you should do is to add a init_early hook to your mdesc structure
and retrieve your ATAGs from there directly at PAGE_OFFSET + 0x100.

Now I suspect paging_init() marks the point where the ATAGs will be
overwritten. To prevent this, you might have to add an additional tweak
in arm_mm_memblock_reserve() similar to the one already present for
CONFIG_SA1111. Something like:

memblock_reserve(PHYS_OFFSET, PAGE_SIZE);

And later on you can return that page back to the system.


Nicolas

2015-12-15 09:33:34

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Monday 30 November 2015 11:09:42 Nicolas Pitre wrote:
> On Mon, 30 Nov 2015, Pali Rohár wrote:
>
> > On Monday 30 November 2015 07:23:53 Tony Lindgren wrote:
> > > * Pali Rohár <[email protected]> [151129 16:16]:
> > > > On Monday 30 November 2015 01:09:17 Nicolas Pitre wrote:
> > > > > On Sun, 29 Nov 2015, Russell King - ARM Linux wrote:
> > > > > > On Sat, Nov 28, 2015 at 12:34:23PM -0500, Nicolas Pitre wrote:
> > > > > > > Good. And Arnd likes the idea too. So we might be converging at
> > > > > > > last which is a good thing.
> > > > > >
> > > > > > I disagree with the idea that there is convergence. There might be
> > > > > > convergence towards an idea, but... Here's a mail extract, from
> > > > > > July 7th, from earlier in this very thread:
> > > > > >
> > > > > > Pali:
> > > > > > > Me:
> > > > > > > > Are the ATAGs at a fixed address on the N900?
> > > > > > >
> > > > > > > Yes, in board-rx51.c is:
> > > > > > >
> > > > > > > .atag_offset = 0x100
> > > > > > >
> > > > > > > and Nokia Bootloader (proprietary) store them to that address.
> > > > > > >
> > > > > > > > Can that be handled in
> > > > > > > > some kind of legacy file for the N900 which calls save_atags()
> > > > > > > > on it, so we don't end up introducing yet more stuff that we
> > > > > > > > have to maintain into the distant future? If not, what about
> > > > > > > > copying a known working atag structure into a legacy file for
> > > > > > > > the N900?
> > > > > > >
> > > > > > > I already asked question if it is possible to read ATAGs from DT
> > > > > > > booted kernel. And somebody (do not remember who) wrote to ML,
> > > > > > > that it is not possible and it can be done in that uncompress
> > > > > > > code.
> > > > >
> > > > > Who is that somebody? If ever it happened to be me then objection is
> > > > > withdrawn. Otherwise that somebody should come forth and speak up
> > > > > again.
> > > > >
> > > >
> > > > ... do not remember ... this discussion were in more email threads and
> > > > takes more then one year... sorry but my memory is not excellent
> > >
> > > Yes this certainly seems like the best solution. I think we got into
> > > the atags-to-dt track as some of the atags are already being translated.
> > >
> > > In this case there's no need to translate them AFAIK. You can just
> > > parse them and have them available for the user space. So as long as
> > > nothing trashes the atags at the atag_offset, you should be able to
> > > call a function to parse them in the n900 specific init_machine.
> > >
> > > Regards,
> > >
> > > Tony
> >
> > In arch/arm/kernel/setup.c is function setup_arch() and it calls:
> >
> > mdesc = setup_machine_fdt(__atags_pointer);
> > if (!mdesc)
> > mdesc = setup_machine_tags(__atags_pointer, __machine_arch_type);
> >
> > So it looks like that on atags address is stored either atags structure
> > or DT structure... so it is truth kernel uncompress code put DT blob to
> > same offset where is expected atags structure?
>
> No. It doesn't put it anywhere. Those functions read DT/ATAGs from the
> passed address. But you know this address won't be the one you want for
> the legacy ATAGs.
>
> What you should do is to add a init_early hook to your mdesc structure
> and retrieve your ATAGs from there directly at PAGE_OFFSET + 0x100.
>
> Now I suspect paging_init() marks the point where the ATAGs will be
> overwritten. To prevent this, you might have to add an additional tweak
> in arm_mm_memblock_reserve() similar to the one already present for
> CONFIG_SA1111. Something like:
>
> memblock_reserve(PHYS_OFFSET, PAGE_SIZE);
>
> And later on you can return that page back to the system.
>
>
> Nicolas

So am I understand correctly that solution would be to hack
arch/arm/mm/mmu.c to not overwrite page at PHYS_OFFSET?

And should be this just when when we detect Nokia N900 in DT? Or for all
OMAP2 boards? Or all ARM boards?

--
Pali Rohár
[email protected]

2015-12-15 11:05:30

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Tuesday 15 December 2015 10:33:25 Pali Roh?r wrote:
> On Monday 30 November 2015 11:09:42 Nicolas Pitre wrote:
> > On Mon, 30 Nov 2015, Pali Roh?r wrote:
> > > On Monday 30 November 2015 07:23:53 Tony Lindgren wrote:
> > > > * Pali Roh?r <[email protected]> [151129 16:16]:
> > > > > On Monday 30 November 2015 01:09:17 Nicolas Pitre wrote:
> > > > > > On Sun, 29 Nov 2015, Russell King - ARM Linux wrote:
> > > In arch/arm/kernel/setup.c is function setup_arch() and it calls:
> > >
> > > mdesc = setup_machine_fdt(__atags_pointer);
> > > if (!mdesc)
> > > mdesc = setup_machine_tags(__atags_pointer, __machine_arch_type);
> > >
> > > So it looks like that on atags address is stored either atags structure
> > > or DT structure... so it is truth kernel uncompress code put DT blob to
> > > same offset where is expected atags structure?
> >
> > No. It doesn't put it anywhere. Those functions read DT/ATAGs from the
> > passed address. But you know this address won't be the one you want for
> > the legacy ATAGs.
> >
> > What you should do is to add a init_early hook to your mdesc structure
> > and retrieve your ATAGs from there directly at PAGE_OFFSET + 0x100.
> >
> > Now I suspect paging_init() marks the point where the ATAGs will be
> > overwritten. To prevent this, you might have to add an additional tweak
> > in arm_mm_memblock_reserve() similar to the one already present for
> > CONFIG_SA1111. Something like:
> >
> > memblock_reserve(PHYS_OFFSET, PAGE_SIZE);
> >
> > And later on you can return that page back to the system.
> >
>
> So am I understand correctly that solution would be to hack
> arch/arm/mm/mmu.c to not overwrite page at PHYS_OFFSET?

I would think we can just copy the data from PAGE_OFFSET + 0x100
to a some other page from your init_early hook. IIRC you can't use
kmalloc there, but memblock_alloc() should work.

Arnd

2015-12-15 12:21:06

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Tue, Dec 15, 2015 at 10:33:25AM +0100, Pali Roh?r wrote:
> So am I understand correctly that solution would be to hack
> arch/arm/mm/mmu.c to not overwrite page at PHYS_OFFSET?

That's completely unnecessary: there are enough platform hooks to cope
with whatever the platform requires.

If you want to reserve the memory, then you have the ->reserve callback,
where you can call:

memblock_reserve(PHYS_OFFSET, PAGE_SIZE);

if you wish to prevent the first page being overwritten. You're then
responsible for freeing this page later in the boot sequence, or you
could just keep it around and refer to the atags in that page directly.

You could also just save_atags() in there, with a comment saying that
this is a work-around for N900 which needs the ATAGs saved, and this
is allowed in ->reserve as a special exception.

--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

2015-12-15 17:24:59

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

On Tue, 15 Dec 2015, Russell King - ARM Linux wrote:

> On Tue, Dec 15, 2015 at 10:33:25AM +0100, Pali Roh?r wrote:
> > So am I understand correctly that solution would be to hack
> > arch/arm/mm/mmu.c to not overwrite page at PHYS_OFFSET?
>
> That's completely unnecessary: there are enough platform hooks to cope
> with whatever the platform requires.

Indeed. I didn't notice that mdesc->reserve() exists these days and is
perfect for this purpose as you say.


Nicolas

2015-12-23 14:54:39

by Ivaylo Dimitrov

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry

Hi,

On 15.12.2015 14:20, Russell King - ARM Linux wrote:
>
> You could also just save_atags() in there, with a comment saying that
> this is a work-around for N900 which needs the ATAGs saved, and this
> is allowed in ->reserve as a special exception.
>

What about this (just to confirm I got the idea correctly, proper patch
will follow if that's the case):

diff --git a/arch/arm/mach-omap2/board-generic.c
b/arch/arm/mach-omap2/board-generic.c
index 34ff14b..8916856 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -83,8 +83,25 @@ static const char *const n900_boards_compat[]
__initconst = {
NULL,
};

+#ifdef CONFIG_ATAGS_PROC
+extern void save_atags(const struct tag *tags);
+
+/* Legacy userspace on Nokia N900 needs ATAGS exported in /proc/atags,
+ * save them while the data is still not overwritten
+ */
+static void __init rx51_reserve(void)
+{
+ const phys_addr_t __atags_pointer = 0x100;
+
+ save_atags(phys_to_virt(__atags_pointer));
+ omap_reserve();
+}
+#else
+#define rx51_reserve omap_reserve
+#endif
+
DT_MACHINE_START(OMAP3_N900_DT, "Nokia RX-51 board")
- .reserve = omap_reserve,
+ .reserve = rx51_reserve,
.map_io = omap3_map_io,
.init_early = omap3430_init_early,
.init_machine = omap_generic_init,

2015-12-24 19:02:49

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 1/5] arm: devtree: Set system_rev from DT "/revision"

On Monday 06 July 2015 22:26:09 Pali Rohár wrote:
> With this patch "/revision" DT entry is used to set global system_rev
> variable. DT "/revision" is expected to be u32 numeric value.
>
> TODO: add documentation
>
> Signed-off-by: Pali Rohár <[email protected]>
> ---
> arch/arm/kernel/devtree.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> index 11c54de..7d82749 100644
> --- a/arch/arm/kernel/devtree.c
> +++ b/arch/arm/kernel/devtree.c
> @@ -19,6 +19,7 @@
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/smp.h>
> +#include <linux/libfdt_env.h>
>
> #include <asm/cputype.h>
> #include <asm/setup.h>
> @@ -26,6 +27,7 @@
> #include <asm/smp_plat.h>
> #include <asm/mach/arch.h>
> #include <asm/mach-types.h>
> +#include <asm/system_info.h>
>
>
> #ifdef CONFIG_SMP
> @@ -204,6 +206,8 @@ static const void * __init arch_get_next_mach(const char *const **match)
> const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
> {
> const struct machine_desc *mdesc, *mdesc_best = NULL;
> + unsigned long dt_root;
> + const u32 *rev;
>
> #ifdef CONFIG_ARCH_MULTIPLATFORM
> DT_MACHINE_START(GENERIC_DT, "Generic DT based system")
> @@ -215,17 +219,16 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
> if (!dt_phys || !early_init_dt_verify(phys_to_virt(dt_phys)))
> return NULL;
>
> + dt_root = of_get_flat_dt_root();
> mdesc = of_flat_dt_match_machine(mdesc_best, arch_get_next_mach);
>
> if (!mdesc) {
> const char *prop;
> int size;
> - unsigned long dt_root;
>
> early_print("\nError: unrecognized/unsupported "
> "device tree compatible list:\n[ ");
>
> - dt_root = of_get_flat_dt_root();
> prop = of_get_flat_dt_prop(dt_root, "compatible", &size);
> while (size > 0) {
> early_print("'%s' ", prop);
> @@ -246,5 +249,10 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
> /* Change machine number to match the mdesc we're using */
> __machine_arch_type = mdesc->nr;
>
> + /* Set system revision from DT */
> + rev = of_get_flat_dt_prop(dt_root, "revision", NULL);
> + if (rev)
> + system_rev = fdt32_to_cpu(*rev);
> +
> return mdesc;
> }

This patch and second one (with subject "[PATCH 2/5] arm: boot: convert
ATAG_REVISION to DT "/revision" entry") are still needed.

Are there any objections for them? If not, I will add missing DT
documentation and will resend them.

--
Pali Rohár
[email protected]

2015-12-28 21:01:29

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH 1/5] arm: devtree: Set system_rev from DT "/revision"

Adding devicetree-spec, and commenting below.

On 12/24/2015 11:02 AM, Pali Rohár wrote:
> On Monday 06 July 2015 22:26:09 Pali Rohár wrote:
>> With this patch "/revision" DT entry is used to set global system_rev
>> variable. DT "/revision" is expected to be u32 numeric value.
>>
>> TODO: add documentation
>>
>> Signed-off-by: Pali Rohár <[email protected]>
>> ---
>> arch/arm/kernel/devtree.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
>> index 11c54de..7d82749 100644
>> --- a/arch/arm/kernel/devtree.c
>> +++ b/arch/arm/kernel/devtree.c
>> @@ -19,6 +19,7 @@
>> #include <linux/of_irq.h>
>> #include <linux/of_platform.h>
>> #include <linux/smp.h>
>> +#include <linux/libfdt_env.h>
>>
>> #include <asm/cputype.h>
>> #include <asm/setup.h>
>> @@ -26,6 +27,7 @@
>> #include <asm/smp_plat.h>
>> #include <asm/mach/arch.h>
>> #include <asm/mach-types.h>
>> +#include <asm/system_info.h>
>>
>>
>> #ifdef CONFIG_SMP
>> @@ -204,6 +206,8 @@ static const void * __init arch_get_next_mach(const char *const **match)
>> const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
>> {
>> const struct machine_desc *mdesc, *mdesc_best = NULL;
>> + unsigned long dt_root;
>> + const u32 *rev;
>>
>> #ifdef CONFIG_ARCH_MULTIPLATFORM
>> DT_MACHINE_START(GENERIC_DT, "Generic DT based system")
>> @@ -215,17 +219,16 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
>> if (!dt_phys || !early_init_dt_verify(phys_to_virt(dt_phys)))
>> return NULL;
>>
>> + dt_root = of_get_flat_dt_root();
>> mdesc = of_flat_dt_match_machine(mdesc_best, arch_get_next_mach);
>>
>> if (!mdesc) {
>> const char *prop;
>> int size;
>> - unsigned long dt_root;
>>
>> early_print("\nError: unrecognized/unsupported "
>> "device tree compatible list:\n[ ");
>>
>> - dt_root = of_get_flat_dt_root();
>> prop = of_get_flat_dt_prop(dt_root, "compatible", &size);
>> while (size > 0) {
>> early_print("'%s' ", prop);
>> @@ -246,5 +249,10 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
>> /* Change machine number to match the mdesc we're using */
>> __machine_arch_type = mdesc->nr;
>>
>> + /* Set system revision from DT */
>> + rev = of_get_flat_dt_prop(dt_root, "revision", NULL);
>> + if (rev)
>> + system_rev = fdt32_to_cpu(*rev);
>> +
>> return mdesc;
>> }
>
> This patch and second one (with subject "[PATCH 2/5] arm: boot: convert
> ATAG_REVISION to DT "/revision" entry") are still needed.
>
> Are there any objections for them? If not, I will add missing DT
> documentation and will resend them.

Patch 2/5 copies the value from ATAG_REVISION into the fdt "/revision"
property.

If the use of /revision is limited to being a location to hold an ATAG
value to pass to the global variable system_rev, then it would make
sense to just copy directly from the ATAG value into system_rev in the
same board file where you are copying the ATAGs.

-Frank

2015-12-28 22:28:05

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/5] arm: devtree: Set system_rev from DT "/revision"

On Monday 28 December 2015 13:01:22 Frank Rowand wrote:
>
> Patch 2/5 copies the value from ATAG_REVISION into the fdt "/revision"
> property.
>
> If the use of /revision is limited to being a location to hold an ATAG
> value to pass to the global variable system_rev, then it would make
> sense to just copy directly from the ATAG value into system_rev in the
> same board file where you are copying the ATAGs.

Agreed. That would be simpler, and avoid a situation where someone relies
on the /revision property in DT to be set from the atags compat code
(preventing an upgrade to a newer bootloader), or on the system_rev variable
to be the same across multiple boot loaders, in the absence of other
kernel code setting it.

Arnd