On Wed, Sep 23, 2015 at 08:39:27PM +0100, Arnd Bergmann wrote: > On Wednesday 23 September 2015 20:35:45 Will Deacon wrote: > > On Wed, Sep 23, 2015 at 08:27:41PM +0100, Arnd Bergmann wrote: > > > On Wednesday 23 September 2015 11:21:56 David Daney wrote: > > > > >> > > > > >> /* Limit the bus-range to fit within reg */ > > > > >> - bus_max = pci->cfg.bus_range->start + > > > > >> - (resource_size(&pci->cfg.res) >> pci->cfg.ops.bus_shift) - 1; > > > > >> + bus_max = (resource_size(&pci->cfg.res) >> pci->cfg.ops.bus_shift) - 1; > > > > >> + if (bus_max > 255) > > > > >> + bus_max = 255; > > > > > > > > > > I still don't understand the need for this part. If the cfg space is bigger > > > > > than bus_max, isn't that simply an invalid resource? Given that the resource > > > > > could be broken in other ways too, this check feels more like a specific > > > > > workaround rather than generally useful code. > > > > > > > > Imagine... > > > > > > > > bus-range [0x80 .. 0xff], this requires a cfg.res that will cover the > > > > entire range of 0..0xff. > > > > > > > > according to the calculations above, (resource_size(&pci->cfg.res) >> > > > > pci->cfg.ops.bus_shift) - 1 will have a value of 0xff, so... > > > > > > Extending the computation to 32 bit seems fine, but I'd rather warn loudly > > > if the bus range does not fit within the registers. > > > > > > Also note that the computation is already correct with my interpretation > > > of the reg property. > > > > From what Lorenzo was saying, ACPI shares the interpretation that David is > > implementing here and, given that the DT version seems to be subjective, > > aligning this reg property with MMCFG seems to make sense. > > We should then make it very clear in the binding that the driver > is not allowed to actually map the registers for the buses outside > of the bus-range, as that is highly unusual. > > We would also need a special exception for this if we ever get to > implement the DT source checker that we have been talking about for > years, as the reg property might then overlap with a property from > another device. Completely agreed. Having a base that isn't actually safe to map is horrible and should be called out. Will -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
