Re: [PATCH v3 07/12] PCI: rockchip-ep: Refactor rockchip_pcie_ep_probe() MSI-X hiding

Manivannan Sadhasivam
2024-10-12T12:14:10Z

On Fri, Oct 11, 2024 at 05:30:00PM +0900, Damien Le Moal wrote:
> On 10/10/24 17:09, Manivannan Sadhasivam wrote:
> > On Thu, Oct 10, 2024 at 12:55:12PM +0530, Manivannan Sadhasivam wrote:
> >> On Mon, Oct 07, 2024 at 01:12:13PM +0900, Damien Le Moal wrote:
> >>> Move the code in rockchip_pcie_ep_probe() to hide the MSI-X capability
> >>> to its own function, rockchip_pcie_ep_hide_msix_cap(). No functional
> >>> changes.
> >>>
> >>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> >>
> >> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >>
> >> Btw, can someone from Rockchip confirm if this hiding is necessary for all the
> >> SoCs? It looks to me like an SoC quirk.
> >>
> >> - Mani
> >>
> >>> ---
> >>> drivers/pci/controller/pcie-rockchip-ep.c | 54 +++++++++++++----------
> >>> 1 file changed, 30 insertions(+), 24 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
> >>> index 523e9cdfd241..7a1798fcc2ad 100644
> >>> --- a/drivers/pci/controller/pcie-rockchip-ep.c
> >>> +++ b/drivers/pci/controller/pcie-rockchip-ep.c
> >>> @@ -581,6 +581,34 @@ static void rockchip_pcie_ep_release_resources(struct rockchip_pcie_ep *ep)
> >>> pci_epc_mem_exit(ep->epc);
> >>> }
> >>>
> >>> +static void rockchip_pcie_ep_hide_msix_cap(struct rockchip_pcie *rockchip)
> >
> > Perhaps a better name would be rockchip_pcie_disable_broken_msix()? As the
> > function essentially disables MSIx which is broken. Again, it'd be good to know
> > if this applies to all SoCs or just a few.
>
> The function does not disable anything but *really* simply hides the capability
> in the capability list so that the host does not see it. So I think the better
> name is:
>
> rockchip_pcie_ep_hide_broken_msix_cap()

Sounds good to me.

- Mani

--
மணிவண்ணன் சதாசிவம்