> Another question I have is why there's a separate device for the > coreboot-table? Couldn't the kernel just create the coreboot bus and > then populate it with the table entries? DT does that. If the > coreboot-table device is really just the parent for the other deivce, > that is incorrect, as I describe above.
To be honest I didn't write this code and probably don't understand it well enough to answer that question (we should probably CC +Samuel, who did). I assume that it was done to get the automatic matching of the ACPI/DT entry that tells the kernel that a coreboot table exists. If there's a better way to achieve that, feel free to change it. > * the check on whether the CB_TAG_FRAMEBUFFER should be used or not by the > kernel should also be there (what Thomas did in patch 1/8) but instead > of omiting registering the coreboot device in the bus, the device could > be marked as "unused" or "disabled" (by adding a field to coreboot_device). This sounds like a reasonable compromise. It still feels a bit odd to me that the check whether other framebuffers exist happens during parsing of the table rather than at the point where the table entry is actually used by something, but if that's the easiest way to make it work, I guess there's no harm in doing it there. (I'm still curious, though, why can't you just do the check in corebootdrm_probe(), and return -ENODEV if the other framebuffer is used? Wouldn't that be equivalent to what the old driver did?) > * the coreboot_bus_match() function then can take this new field into > account and only match if (device->entry.tag == id->tag && !device->unused) > or something like that. > > That way no device will be removed from the bus and the corebootdrm driver > will only be probed when the device has to actually be used by the kernel. > > -- > Best regards, > > Javier Martinez Canillas > Core Platforms > Red Hat >
