On Wed, 7 Nov 2018, Christoph Hellwig wrote:
> > +static int sb1250_bus_dma_mask(struct pci_dev *dev, void *data)
> > +{
> > + struct sb1250_bus_dma_mask_exclude *exclude = data;
> > +
> > + if (!exclude->set && (dev->vendor == PCI_VENDOR_ID_SIBYTE &&
> > + dev->device == PCI_DEVICE_ID_BCM1250_HT)) {
> > + exclude->start = dev->subordinate->number;
> > + exclude->end = pci_bus_max_busnr(dev->subordinate);
> > + exclude->set = true;
> > + dev_dbg(&dev->dev, "not disabling DAC for [bus %02x-%02x]",
> > + exclude->start, exclude->end);
> > + } else if (!exclude->set ||
> > + (exclude->set && (dev->bus->number < exclude->start ||
> > + dev->bus->number > exclude->end))) {
> > + dev_dbg(&dev->dev, "disabling DAC for device");
> > + dev->dev.bus_dma_mask = DMA_BIT_MASK(32);
> > + } else {
> > + dev_dbg(&dev->dev, "not disabling DAC for device");
> > + }
> > + return 0;
>
> Hmm, these conditions look very hard to read to me. Wouldn't this
> have the same effect?
>
> if (exclude->set)
> return;
Nope, `exclude->set' only means we already know what range to exclude
(and that gets set mid-way through scanning as the HT bridge is
encountered). Then if it's unset, we know we are (still) outside that
range.
Maybe I can flatten the conditions at the small cost of executing some
code unnecessarily. But that won't be a big deal as this stuff is only
executed once at boot and isn't performance critical.
It'll have to wait until next week though as I'll be travelling
throughout the rest of this and won't be able to test anything.
Maciej
