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

Commit Message

Pali Rohár July 6, 2015, 8:26 p.m. UTC

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 <[e-mail]>
---
 arch/arm/boot/compressed/atags_to_fdt.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Russell King - ARM Linux July 7, 2015, 11:32 a.m. UTC | #1

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?

Pali Rohár July 7, 2015, 11:58 a.m. UTC | #2

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.

Tony Lindgren July 13, 2015, 1:19 p.m. UTC | #3

* Pali Rohár <[e-mail]> [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

Tony Lindgren Oct. 12, 2015, 8:16 p.m. UTC | #4

* Tony Lindgren <[e-mail]> [150713 06:21]:
> * Pali Rohár <[e-mail]> [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?

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.

Regards,

Tony

Pali Rohár Oct. 12, 2015, 8:25 p.m. UTC | #5

On Monday 12 October 2015 22:16:40 Tony Lindgren wrote:
> * Tony Lindgren <[e-mail]> [150713 06:21]:
> > * Pali Rohár <[e-mail]> [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?
> 
> 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.
> 
> Regards,
> 
> Tony

Tony, I'm not really sure what to do. Just wrap 4 and 5 patches into 
CONFIG_KEXEC? Or something more?

Tony Lindgren Oct. 12, 2015, 8:45 p.m. UTC | #6

* Pali Rohár <[e-mail]> [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

Pali Rohár Oct. 13, 2015, 2:37 p.m. UTC | #7

On Monday 12 October 2015 13:45:09 Tony Lindgren wrote:
> * Pali Rohár <[e-mail]> [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?

Pali Rohár Nov. 5, 2015, 11:40 a.m. UTC | #8

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 <[e-mail]> [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) ^^^

Tony Lindgren Nov. 5, 2015, 4:17 p.m. UTC | #9

* Pali Rohár <[e-mail]> [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 <[e-mail]> [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

Frank Rowand Nov. 12, 2015, 1:10 a.m. UTC | #10

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 <[e-mail]> [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 <[e-mail]> [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 [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Pavel Machek Nov. 22, 2015, 6:51 a.m. UTC | #11

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 <[e-mail]> [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 <[e-mail]> [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

Pali Rohár Nov. 23, 2015, 2:45 p.m. UTC | #12

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 <[e-mail]> [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 <[e-mail]> [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.

Tony Lindgren Nov. 25, 2015, 6:16 p.m. UTC | #13

* Pali Rohár <[e-mail]> [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 <[e-mail]> [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 <[e-mail]> [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

Arnd Bergmann Nov. 25, 2015, 7:48 p.m. UTC | #14

On Wednesday 25 November 2015 10:16:44 Tony Lindgren wrote:
> * Pali Rohár <[e-mail]> [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 <[e-mail]> [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 <[e-mail]> [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

Tony Lindgren Nov. 25, 2015, 9:03 p.m. UTC | #15

* Arnd Bergmann <[e-mail]> [151125 11:50]:
> On Wednesday 25 November 2015 10:16:44 Tony Lindgren wrote:
> > * Pali Rohár <[e-mail]> [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 <[e-mail]> [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 <[e-mail]> [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

Arnd Bergmann Nov. 25, 2015, 9:29 p.m. UTC | #16

On Wednesday 25 November 2015 13:03:10 Tony Lindgren wrote:
> * Arnd Bergmann <[e-mail]> [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

Pali Rohár Nov. 25, 2015, 9:44 p.m. UTC | #17

On Wednesday 25 November 2015 22:29:53 Arnd Bergmann wrote:
> On Wednesday 25 November 2015 13:03:10 Tony Lindgren wrote:
> > * Arnd Bergmann <[e-mail]> [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?

Arnd Bergmann Nov. 25, 2015, 9:51 p.m. UTC | #18

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

Pali Rohár Nov. 25, 2015, 10 p.m. UTC | #19

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.

Frank Rowand Nov. 26, 2015, 4:19 a.m. UTC | #20

On 11/25/2015 1:03 PM, Tony Lindgren wrote:
> * Arnd Bergmann <[e-mail]> [151125 11:50]:
>> On Wednesday 25 November 2015 10:16:44 Tony Lindgren wrote:
>>> * Pali Rohár <[e-mail]> [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 <[e-mail]> [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 <[e-mail]> [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

Pali Rohár Nov. 26, 2015, 9:07 a.m. UTC | #21

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...

Tony Lindgren Nov. 26, 2015, 8:39 p.m. UTC | #22

* Pali Rohár <[e-mail]> [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

Ivaylo Dimitrov Nov. 26, 2015, 9:12 p.m. UTC | #23

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

Pali Rohár Nov. 27, 2015, 8:38 a.m. UTC | #24

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).

Michael Nazzareno Trimarchi Nov. 27, 2015, 8:44 a.m. UTC | #25

Hi

On Fri, Nov 27, 2015 at 9:38 AM, Pali Rohár <[e-mail]> 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
> [e-mail]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Michael Nazzareno Trimarchi Nov. 27, 2015, 8:52 a.m. UTC | #26

Hi

On Fri, Nov 27, 2015 at 9:44 AM, Michael Trimarchi
<[e-mail]> wrote:
> Hi
>
> On Fri, Nov 27, 2015 at 9:38 AM, Pali Rohár <[e-mail]> 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
>> [e-mail]
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to [e-mail]
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Russell King - ARM Linux Nov. 27, 2015, 1:27 p.m. UTC | #27

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.

Tony Lindgren Nov. 27, 2015, 2:51 p.m. UTC | #28

* Pali Rohár <[e-mail]> [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

Russell King - ARM Linux Nov. 27, 2015, 7:51 p.m. UTC | #29

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.

Arnd Bergmann Nov. 27, 2015, 9:06 p.m. UTC | #30

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

Nicolas Pitre Nov. 27, 2015, 11:28 p.m. UTC | #31

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

Arnd Bergmann Nov. 28, 2015, 12:27 p.m. UTC | #32

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

Russell King - ARM Linux Nov. 28, 2015, 12:33 p.m. UTC | #33

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.

Russell King - ARM Linux Nov. 28, 2015, 12:54 p.m. UTC | #34

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.)

Nicolas Pitre Nov. 28, 2015, 5:34 p.m. UTC | #35

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

Frank Rowand Nov. 28, 2015, 9:02 p.m. UTC | #36

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

Russell King - ARM Linux Nov. 29, 2015, 6:09 p.m. UTC | #37

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.

Pali Rohár Nov. 29, 2015, 6:19 p.m. UTC | #38

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...

Russell King - ARM Linux Nov. 29, 2015, 11:13 p.m. UTC | #39

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.

Nicolas Pitre Nov. 30, 2015, 12:09 a.m. UTC | #40

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

Pali Rohár Nov. 30, 2015, 12:15 a.m. UTC | #41

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

Tony Lindgren Nov. 30, 2015, 3:23 p.m. UTC | #42

* Pali Rohár <[e-mail]> [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

Pali Rohár Nov. 30, 2015, 3:39 p.m. UTC | #43

On Monday 30 November 2015 07:23:53 Tony Lindgren wrote:
> * Pali Rohár <[e-mail]> [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?

Nicolas Pitre Nov. 30, 2015, 4:09 p.m. UTC | #44

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

> On Monday 30 November 2015 07:23:53 Tony Lindgren wrote:
> > * Pali Rohár <[e-mail]> [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

Pali Rohár Dec. 15, 2015, 9:33 a.m. UTC | #45

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 <[e-mail]> [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?

Arnd Bergmann Dec. 15, 2015, 11:04 a.m. UTC | #46

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 <[e-mail]> [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

Russell King - ARM Linux Dec. 15, 2015, 12:20 p.m. UTC | #47

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.

Nicolas Pitre Dec. 15, 2015, 5:24 p.m. UTC | #48

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

Patch

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);
 }