On Wed, Sep 10, 2014 at 05:37:47PM +0100, Lorenzo Pieralisi wrote: > On Wed, Sep 10, 2014 at 04:32:33PM +0100, Liviu Dudau wrote: > > [...] > > > > > > > + /* > > > > > > + * If we failed translation or got a zero-sized region > > > > > > + * then skip this range > > > > > > + */ > > > > > > + if (range.cpu_addr == OF_BAD_ADDR || range.size == 0) > > > > > > + continue; > > > > > > + > > > > > > + res = kzalloc(sizeof(struct resource), GFP_KERNEL); > > > > > > + if (!res) { > > > > > > + err = -ENOMEM; > > > > > > + goto parse_failed; > > > > > > + } > > > > > > + > > > > > > + err = of_pci_range_to_resource(&range, dev, res); > > > > > > + if (err) { > > > > > > + kfree(res); > > > > > > > > > > You might want to add a label to free res to make things more uniform. > > > > Sorry, not following you. How would a label help here? > > It was just a suggestion so ignore it if you do not think it is cleaner. > It is to make code more uniform and undo operations in one place instead of > doing it piecemeal (you kfree the res here and jump to complete the clean-up, > whereas you might want to add a different label and a different goto > destination and carry out the kfree there). But that kfree is the only done once, while the pci_free_resource_list() is done twice. > > I do not mind either, it is just what I noticed. > > > > > > > + goto parse_failed; > > > > > > + } > > > > > > + > > > > > > + if (resource_type(res) == IORESOURCE_IO) { > > > > > > + if (*io_base) > > > > > > > > > > You do not zero io_base in the first place so you should ask the API > > > > > user to do that. Is 0 a valid value BTW ? If it is you've got to resort > > > > > to something else to detect multiple IO resources. > > > > No, zero is not a valid value. It is the cpu_addr value from the IO range, I'm > > hopying that no one is crazy enough to map PCI address space at CPU address zero. > > Thanks for spotting the lack of initialisation though, I need to fix it. > > Mmm...wasn't a trick question sorry :D > > PCI host bridge /pci ranges: > IO 0x00000000..0x0000ffff -> 0x00000000 > More than one I/O resource converted. CPU offset for old range lost! > MEM 0x41000000..0x7fffffff -> 0x41000000 > pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 > pci_bus 0000:00: root bus resource [bu-01] > pci_bus 0000:00: root bus resource [io 0x0000-0xffff] > pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] > Your DT puts the IO space at CPU address zero? Could you copy the relevant host bridge node from DT here? Best regards, Liviu -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html