On Fri, Jul 07, 2017 at 05:09:15PM +1000, David Gibson wrote: > 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: > > > On Wed, Jun 14, 2017 at 05:52:25PM +0300, Pantelis Antoniou wrote: > > > > 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. Erm, v1 is out there, because the patches are posted here in public, where reviews happen. If some group picks them up and runs with them, perhaps it's worth asking why some group would be willing to pick up and run with v1 of a series? Which, AFAIK, hasn't actually happened just yet.. > > > 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. So there's literally no one that thinks the way the first part over overlay support happened was a good example? Good. Shall we try and fix that moving forward? Isn't the way that open source works is that people develop, in the open and code evolves based on feedback from other developers and real world uses? > > > 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. So to be clear for (a), you're saying the change to dtc's -@ implementation would be less invasive? > > 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. So you wish that someone had written some "overlay doc" before posting patches? Documentation should come with code, not before code, as we already (right?) have general understanding that things like overlays need to exist and need to be useful because there's real use cases out there. -- Tom
Attachment:
signature.asc
Description: Digital signature
