On Tue, Mar 07, 2017 at 01:09:55PM -0600, Bjorn Helgaas wrote:
> On Wed, Feb 22, 2017 at 03:08:07PM -0800, Joao Pinto wrote:
> > Hi Dan,
> >
> > Às 3:26 PM de 2/17/2017, Dan Carpenter escreveu:
> > > The bug is that "val" is unsigned long but we only initialize 32 bits
> > > of it. Then we test "if (val)" and that might be true not because we
> > > set the bits but because some were never initialized.
> > >
> > > Fixes: f342d940ee0e ("PCI: exynos: Add support for MSI")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > > ---
> > > Static analysis. Not tested.
> >
> > What you are statiting makes perfect sense, since the register is indeed 32 bits
> > and can have undesirable behavior in 64-bit systems for example.
> > We have more examples like this for MSI related operations in pcie-designware.
> > Could you please change them as well just?
> >
> > For example, the irq variable declaration is also not consistent as you can see
> > in these examples:
> >
> > static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
> >
> > static int dw_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
> > irq_hw_number_t hwirq)
> >
> > static void dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
> >
> > static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
>
> Where are we with this? It sounds like there's a real problem here,
> and Dan's original patch fixes one case of it. But if there are other
> similar cases, we should fix them all at once.
>
> Since this doesn't sound like an urgent bug fix (I don't see user
> problem reports), I guess I'll wait for an updated patch?
Oh... Hm. I misread. I thought that Joao was going to send a patch.
Looking at it more closely now, I think my patch is sufficient. Perhaps
I have misunderstood something but I don't see any other bugs here
beyond the one I fixed.
regards,
dan carpenter
