Hi David,
On Mon, 2017-07-03 at 19:06 +1000, David Gibson wrote:
On Wed, Jun 14, 2017 at 05:52:25PM +0300, Pantelis Antoniou wrote:
quoted
This patch enables an overlay to refer to a previous overlay's
labels by performing a merge of symbol information at application
time.
This seems to be doing things the hard way.
It is the minimal implementation to get things to work, with the current
overlay implementation. I do have plans for a version 2 with fixes to
a number of areas.
You're essentially extending the semantics of overlay application to
add the symbol merging. You've implemented these extended semantics
in libfdt, which is all very well, but that's not the only overlay
application implementation.
This is a port of the same patch that's against the linux kernel.
As far as I know there's no other implementations, or at least none
that are open source.
It seems to me a better approach would be to change dtc's -@
implementation, so that in /plugin/ mode instead of making a global
__symbols__ node, it puts it into the individual fragments. That way
the existing overlay application semantics will update the __symbols__
node.
A lot of things can be made better, on the next version. These are
minimally intrusive patches to address user requests for the current
implementation.
Why don't we start by making a list, and work towards that goal?
Care to start about what you want addressed and how?
Regards
-- Pantelis
On Mon, Jul 03, 2017 at 03:41:14PM +0300, Pantelis Antoniou wrote:
Hi David,
On Mon, 2017-07-03 at 19:06 +1000, David Gibson wrote:
quoted
On Wed, Jun 14, 2017 at 05:52:25PM +0300, Pantelis Antoniou wrote:
quoted
This patch enables an overlay to refer to a previous overlay's
labels by performing a merge of symbol information at application
time.
This seems to be doing things the hard way.
It is the minimal implementation to get things to work, with the current
overlay implementation.
Is it, though? I'd expect reworking the symbol creation during
compile to be of similar complexity to the symbol merging here. And
it only needs to be done in one place, not two. And it doesn't
implicitly extend the overlay spec.
I do have plans for a version 2 with fixes to
a number of areas.
Saying you'll fix it in v2 is missing the point. If v1 is out there,
we have to keep supporting it. The number of half-arsed overlay
variants out in the wild just seems to keep growing.
quoted
You're essentially extending the semantics of overlay application to
add the symbol merging. You've implemented these extended semantics
in libfdt, which is all very well, but that's not the only overlay
application implementation.
This is a port of the same patch that's against the linux kernel.
As far as I know there's no other implementations, or at least none
that are open source.
So, it's already in the wild and we have to deal with it. Yay.
The whole history of DT overlays has been this - hacking something up
to grab some desired feature with a complete lack of forethought about
what the long term, or even medium term, consequences will be. It's
kind of pissing me off.
That's exactly why it took so long to get the overlay patches merged
in the first place. I was hoping to encourage a bit more thinking
*before* putting an approach in the wild that would predictably cause
us trouble later on. Didn't work, alas.
quoted
It seems to me a better approach would be to change dtc's -@
implementation, so that in /plugin/ mode instead of making a global
__symbols__ node, it puts it into the individual fragments. That way
the existing overlay application semantics will update the __symbols__
node.
A lot of things can be made better, on the next version. These are
minimally intrusive patches to address user requests for the current
implementation.
Except that a) I'm not really convinced of that and b) I don't see any
signs of really trying to approach this methodically, rather than just
moving from one hack to the next.
Why don't we start by making a list, and work towards that goal?
Care to start about what you want addressed and how?
The biggest thing is a question of design culture, not any specific
properties. Think in terms of specification, rather than just
implementation, and make at least a minimal effort to ensure that that
specification is both sufficient and minimal for the requirements at
hand. Overlays as they stand are a long way from either.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On 07/03/17 05:41, Pantelis Antoniou wrote:
Hi David,
On Mon, 2017-07-03 at 19:06 +1000, David Gibson wrote:
quoted
On Wed, Jun 14, 2017 at 05:52:25PM +0300, Pantelis Antoniou wrote:
quoted
This patch enables an overlay to refer to a previous overlay's
labels by performing a merge of symbol information at application
time.
This seems to be doing things the hard way.
It is the minimal implementation to get things to work, with the current
overlay implementation. I do have plans for a version 2 with fixes to
a number of areas.
quoted
You're essentially extending the semantics of overlay application to
add the symbol merging. You've implemented these extended semantics
in libfdt, which is all very well, but that's not the only overlay
application implementation.
This is a port of the same patch that's against the linux kernel.
As far as I know there's no other implementations, or at least none
that are open source.
That patch never made it into the kernel. I submitted a different
patch last week (now at v2, probably soon to be v3, then probably
v4 when 4.13-rc1 comes out), so hopefully we will get the overlay
symbol loading support into the kernel soon.
quoted
It seems to me a better approach would be to change dtc's -@
implementation, so that in /plugin/ mode instead of making a global
__symbols__ node, it puts it into the individual fragments. That way
the existing overlay application semantics will update the __symbols__
node.
A lot of things can be made better, on the next version. These are
minimally intrusive patches to address user requests for the current
implementation.
Why don't we start by making a list, and work towards that goal?
Care to start about what you want addressed and how?
Regards
-- Pantelis