On 28/07/2019 11:39, BALATON Zoltan wrote:
On Sun, 28 Jul 2019, Mark Cave-Ayland wrote:
Yeah, it's a bug - I missed that in the case of unknown devices we were still falling back to the old behaviour of force-creating devices at known paths which is the behaviour that this patchset is removing.
Attached is a fix that works for me when applied on top of the v2 series if you can test it?
Thanks it works but isn't it simpler to just write it as below? device-name is the same as encode-string " name" property so why open code it? device-name seems to be used more commonly elsewhere. Also your patch uses spaces for indent while lines around it still use tabs so maybe this should also indent with tabs for consistency. (There are also a few more whitespace errors elsewhere in this file from previous patches, git diff or git log -p shows them in red for me.)
diff --git a/drivers/pci.c b/drivers/pci.c index 4bc02c9..2c7d3ef 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -1261,21 +1285,15 @@ static void ob_pci_add_properties(phandle_t phandle, class_prog = pci_config_read8(addr, PCI_CLASS_PROG); class_code = pci_config_read16(addr, PCI_CLASS_DEVICE);
- if (pci_dev) {
/**/
if (pci_dev->name) {
push_str(pci_dev->name);
fword("device-name");
} else {
char path[256];
snprintf(path, sizeof(path),
"pci%x,%x", vendor_id, device_id);
push_str(path);
fword("device-name");
}
- } else {
PCI_DPRINTF("*** missing pci_dev\n");
- }
if (pci_dev && pci_dev->name) {
push_str(pci_dev->name);
} else {
char path[256];
snprintf(path, sizeof(path),
"pci%x,%x", vendor_id, device_id);
push_str(path);
}
fword("device-name");
/* create properties as described in 2.5 */
Certainly that is one way of writing it, however given that this is going to get squashed into the PCI changeover patch I'll probably leave it as-is for 2 reasons: 1) it's easier from the diff to see which logic has changed rather than throwing device-name into the mix, and 2) it was helpful for me to have separate breakpoints for all of the 3 outcomes rather than having to keep stepping through them all to find out which was the missing device, for example.
Regarding whitespace errors that is a long-running issue with OpenBIOS in that over time people have used either tabs and/or spaces, so I tend to use what's around me to keep the diffs readable. Sadly some files mix up tabs and spaces liberally so you'll never end up with a perfect diff :(
ATB,
Mark.