On Wed, Dec 09, 2020 at 12:13:50PM -0600, Rob Herring wrote:
> On Wed, Dec 02, 2020 at 03:31:56PM +0800, Wan Ahmad Zainie wrote:
...
> > +static void keembay_pcie_ltssm_enable(struct keembay_pcie *pcie, bool enable)
> > +{
> > + u32 val;
> > +
> > + val = keembay_pcie_readl(pcie, PCIE_REGS_PCIE_APP_CNTRL);
> > + if (enable)
> > + val |= APP_LTSSM_ENABLE;
> > + else
> > + val &= ~APP_LTSSM_ENABLE;
> > + keembay_pcie_writel(pcie, PCIE_REGS_PCIE_APP_CNTRL, val);
>
> If this is the only bit in this register, do you really need RMW?
I think it's safer than do direct write and have something wrong on next
generations of hardware.
...
> > +static int keembay_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > + enum pci_epc_irq_type type,
> > + u16 interrupt_num)
> > +{
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +
> > + switch (type) {
> > + case PCI_EPC_IRQ_LEGACY:
> > + /* Legacy interrupts are not supported in Keem Bay */
> > + dev_err(pci->dev, "Legacy IRQ is not supported\n");
> > + return -EINVAL;
> > + case PCI_EPC_IRQ_MSI:
> > + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> > + case PCI_EPC_IRQ_MSIX:
> > + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> > + default:
> > + dev_err(pci->dev, "Unknown IRQ type %d\n", type);
> > + return -EINVAL;
> > + }
>
> Doesn't the lack of a 'return' give a warning?
Where? I don't see any lack of return.
> > +}
--
With Best Regards,
Andy Shevchenko
