On 20.04.2009 14:28, Luc Verhaegen wrote: > On Mon, Apr 20, 2009 at 01:17:43PM +0200, Carl-Daniel Hailfinger wrote: > >> On 20.04.2009 12:12, Luc Verhaegen wrote: >> >>> Flashrom: board_enables: reconstruct table. >>> >>> This patch restores the pciid based board matching table. It makes this >>> table readable and hackable again, and the only disadvantage is that the >>> right margin is way beyond the rather dogmatic 80. All 0x0000 pci ids have >>> been string replaced by 0 to more easily spot missing ids, and extra >>> comments have been added to explain how the various entries are used. >>> >>> Signed-Off-By: Luc Verhaegen <[email protected]> >>> >>> >> The extra comments are good, but "coreboot bios" in such a text really >> hurts. Please change the word BIOS to all-caps and don't refer to >> coreboot as a BIOS. Thanks. >> > > These are the lines in question: > > * The coreboot ids are used two fold. When a coreboot bios is installed, it > * uniquely matches the coreboot board identification string. When a legacy > * bios is installed and when autodetection is not possible, these ids can be > * used to identify the board through a command line argument. > > It is clear and concise. > > Let's take the other option: "when coreboot is installed" What does that > mean? Did the user check out the tree, did he install some of the utils, > or did he install a coreboot bios image into his rom? >
Coreboot is NOT a BIOS. If you really insist calling coreboot a BIOS, why not call it LinuxBIOS? > Your statement is more about politics than anything else. > There was almost universal agreement about the name change from LinuxBIOS to coreboot (well, I was unhappy initially, but I soon found out that the absence of BIOS in the new name is really beneficial). One of the reasons for the name change was that coreboot is NOT a BIOS nor should it ever be called one. Feel free to propose calling coreboot a BIOS in a separate RFC on this list. >> The documentation part can be committed after one more iteration. >> > > I would never have altered these comments if it wasn't for the previous > flamewar. For those just tuning in, this is the thread in the > mailarchives: > http://www.coreboot.org/pipermail/coreboot/2009-January/044086.html > Ah yes, I forgot. In http://www.coreboot.org/pipermail/coreboot/2009-January/044129.html you promised to "create all board enable functions and table entries for the foreseeable future for everyone who asks once we switch back to the old layout [...] next to keeping X from degrading even further." Was that irony or a real promise? >> The rest of the patch makes board entries unreadable. >> > > We had this one before. It makes it unreadable for you. But not for > everyone. Well, more flashrom developers on this list favour the multiline solution than those opposed to it. That means I'm part of the majority. And I do help people on IRC add board enables. >> In the past few weeks, we had quite a few people in #coreboot who wanted >> to add support for their boards. The presence of struct member names >> helped immensely to explain how to add board enables. Some even figured >> it out from similar entries. Lowering the barrier for possible future >> developers is very important. >> > > What makes you so certain that it is this change that made people add 4 > entries in 3 months? Couldn't that be because flashrom just is getting > used more? And besides, this growth is not significantly faster than > before. > That's a straw man. I didn't claim any of the points you try to question above. >> Since IIRC you said some time ago that the multiline layout hinders your >> workflow, feel free to submit any new board enables in the single-line >> layout. I'll fix them up. >> > > Why is my workflow relevant? If your workflow is irrelevant, why would we want to accommodate it? > Do you really think that i have all my > editors open at 160 chars all the time? My you are referring to my > statements about being able to oversee everything more quickly with a > condensed table. This is just common sense, everybody will have to > acknowledge that one. > Unless you get to define everbody's common sense (which is inherently subjective), it is unlikely that everybody (in the strict sense) will have to acknowledge your view. After all, I'm a person (and thus part of the "everybody" group of people) and I disagree with you. You could claim that I don't have common sense, but I hope you won't do that. > [...] > What will you do when it reaches a hundred or so entries? Will you be > happy with a 1400line mess? Is any table with 1400 lines a mess? > Do you think that this table will stop > growing at some point? > Why should it? Hardware will probably have quirks for the foreseeable future. > How many actual board enable functions will there be with multiple > entries in the table? How much space will the actual enables take > compared to the table? > I expect at least half a dozen entries with flash translation chips because these boards exist out there. > How will people tell whether something similar has already been added? > Like they do now. > Will you impose some entry ordering scheme (which, in light of likely > future table growth, is actually a useful change)? > Your patch doesn't touch that issue, so the point is orthogonal to our discussion. > It rapidly becomes non-obvious when you don't have a tight table. And i > know what you are going to say here: just make sure that they are added > in whatever order already. And my answer to that is: you can't. Left or > right, entries will be added that aren't ordered, and afterwards no-one > will spot them being unordered anymore. In a tight table, you spot them > right away and can fix it easily on the next iteration. > See above. There are lots of possible ordering schemes, but deciding which makes sense is a different matter. > It really is creating a situation where you are unable to see the forest > through the trees. > Grep works nicely. > Also, you originally suggested to remove lines like = 0x0000; or = NULL; > This is a very dangerous path. People will only see the last 2-3 > entries, and never look beyond. Remove a line, and it will not be > thought of for the next entry. This is simply human behaviour. > So you suggest to keep these lines. Fine. > What you then end up with is a whole range of boards where just a single > pair of main ids, the coreboot ids, the string and the enable will be > created. No autodetection will be provided any more, as the trivial > addition of yet another argument-specified-only board is of course > obvious for the person who added the board enable to begin with. > > The end result then is that all users will just randomly go through the > board names for ages, all the time, some of which will work, as the > amount of pci devices really isn't that great. This is quite a dangerous > situation and a really bad user experience. > Adding unique subsystem IDs to that struct is hard. First, I know for a fact that my laptop and a related model have identical subsystem IDs. That alone is proof that subsystem IDs are not always unique. Unless you know all subsystem IDs of all boards from a given manufacturer, you can't say for sure whether the board enable you're adding subsystem IDs for is actually _not_ going to trigger on another board. To make this even more fun, which subsystem ID will you use > With a condensed table, people are forced to pad, and will put more > thought into their entries and will feel obliged to add subsystem ids. > You must be joking. If you can _prove_ that, you'll probably get a PhD in psychology right away. If pressing tab once really forces people to think for minutes about something that is neither easy nor obvious, I want a tab key in every conversation. > This in turn leads to more autodetection and less people blindly forcing > boardnames, and in general a better user experience. > > So maybe making it "easier" for people to "understand" what the > different entries are for (which actually boils down to being able to > ignore how the entries are used), is not a goal that should be pursued > above other goals. For existing developers, at least those willing to > not religiously stick to dogmas when it makes sense, the condensed table > is the better option. > Let me modify one word of your statement to express my opinion: For existing developers, at least those willing to not religiously stick to dogmas when it makes sense, the multiline table is the better option. Regards, Carl-Daniel -- http://www.hailfinger.org/ -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
