Message ID | 20140905174122.GD8080@google.com |
---|---|
State | Changes Requested |
Headers | show |
Dear Bjorn Helgaas, On Fri, 5 Sep 2014 11:41:22 -0600, Bjorn Helgaas wrote: > This fix looks right to me. I added a stable tag as follows. Thomas > and/or Jason, and you ack this? Hum, I think I would actually prefer something like: if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_IO) rtype = IORESOURCE_IO; else if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_MEM32) rtype = IORESOURCE_MEM; + else + continue; So that we're explicit with the fact that we only care about I/O and MEM32 resource types. Thanks, Thomas
On Friday 05 September 2014 20:20:44 Thomas Petazzoni wrote: > Hum, I think I would actually prefer something like: > > if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_IO) > rtype = IORESOURCE_IO; > else if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_MEM32) > rtype = IORESOURCE_MEM; > + else > + continue; > > So that we're explicit with the fact that we only care about I/O and > MEM32 resource types. Agreed, that looks better than my patch as well. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 5, 2014 at 12:34 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 05 September 2014 20:20:44 Thomas Petazzoni wrote: >> Hum, I think I would actually prefer something like: >> >> if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_IO) >> rtype = IORESOURCE_IO; >> else if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_MEM32) >> rtype = IORESOURCE_MEM; >> + else >> + continue; >> >> So that we're explicit with the fact that we only care about I/O and >> MEM32 resource types. > > Agreed, that looks better than my patch as well. I like it better, too, but we still need the "range += rangesz" part, so I don't think it will work. I suppose that could be moved to the update expression of the "for" loop. Or, since we don't use "i" in the loop at all, maybe we could do something like this: for (; range < rend; range += rangesz) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 05, 2014 at 01:00:29PM -0600, Bjorn Helgaas wrote: > On Fri, Sep 5, 2014 at 12:34 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Friday 05 September 2014 20:20:44 Thomas Petazzoni wrote: > >> Hum, I think I would actually prefer something like: > >> > >> if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_IO) > >> rtype = IORESOURCE_IO; > >> else if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_MEM32) > >> rtype = IORESOURCE_MEM; > >> + else > >> + continue; > >> > >> So that we're explicit with the fact that we only care about I/O and > >> MEM32 resource types. > > > > Agreed, that looks better than my patch as well. > > I like it better, too, but we still need the "range += rangesz" part, > so I don't think it will work. I suppose that could be moved to the > update expression of the "for" loop. Or, since we don't use "i" in > the loop at all, maybe we could do something like this: > > for (; range < rend; range += rangesz) Any more input on this? I don't think I've seen anything actually acked by Thomas or Jason. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c index a8c6f1a92e0f..081579c0971e 100644 --- a/drivers/pci/host/pci-mvebu.c +++ b/drivers/pci/host/pci-mvebu.c @@ -877,7 +877,7 @@ static int mvebu_get_tgt_attr(struct device_node *np, int devfn, u32 flags = of_read_number(range, 1); u32 slot = of_read_number(range + 1, 1); u64 cpuaddr = of_read_number(range + na, pna); - unsigned long rtype; + unsigned long rtype = 0; if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_IO) rtype = IORESOURCE_IO;