Re: [PATCH net-next V2 2/5] net: lan743x: Add support to software-nodes for sfp

[Raju Lakkaraju]

Hi Andrew,

The 09/11/2024 19:17, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> > diff --git a/drivers/net/ethernet/microchip/Kconfig b/drivers/net/ethernet/microchip/Kconfig
> > index 2e3eb37a45cd..9c08a4af257a 100644
> > --- a/drivers/net/ethernet/microchip/Kconfig
> > +++ b/drivers/net/ethernet/microchip/Kconfig
> > @@ -50,6 +50,8 @@ config LAN743X
> >       select CRC16
> >       select CRC32
> >       select PHYLINK
> > +     select I2C_PCI1XXXX
> > +     select GP_PCI1XXXX
> 
> GP_ is odd. GPIO drivers usually use GPIO_. Saying that, GPIO_PCI1XXXX
> is not in 6.11-rc7. Is it in gpio-next?
> 

Yes. But GPIO driver developer use this.
I have to use the same here.

It's exist in 6.11-rc6
drivers/misc/mchp_pci1xxxx/Kconfig
This was defined along back.

>Is it in gpio-next? 
No. available in net-next

> > +static void *pci1xxxx_perif_drvdata_get(struct lan743x_adapter *adapter,
> > +                                     u16 perif_id)
> > +{
> > +     struct pci_dev *pdev = adapter->pdev;
> > +     struct pci_bus *perif_bus;
> > +     struct pci_dev *perif_dev;
> > +     struct pci_dev *br_dev;
> > +     struct pci_bus *br_bus;
> > +     struct pci_dev *dev;
> > +
> > +     /* PCI11x1x devices' PCIe topology consists of a top level pcie
> > +      * switch with up to four downstream ports, some of which have
> > +      * integrated endpoints connected to them. One of the downstream ports
> > +      * has an embedded single function pcie ethernet controller which is
> > +      * handled by this driver. Another downstream port has an
> > +      * embedded multifunction pcie endpoint, with four pcie functions
> > +      * (the "peripheral controllers": I2C controller, GPIO controller,
> > +      * UART controllers, SPIcontrollers)
> > +      * The code below navigates the PCI11x1x topology
> > +      * to find (by matching its PCI device ID) the peripheral controller
> > +      * that should be paired to the embedded ethernet controller.
> > +      */
> > +     br_dev = pci_upstream_bridge(pdev);
> > +     if (!br_dev) {
> > +             netif_err(adapter, drv, adapter->netdev,
> > +                       "upstream bridge not found\n");
> > +             return br_dev;
> > +     }
> > +
> > +     br_bus = br_dev->bus;
> > +     list_for_each_entry(dev, &br_bus->devices, bus_list) {
> > +             if (dev->vendor == PCI1XXXX_VENDOR_ID &&
> > +                 (dev->device & ~PCI1XXXX_DEV_MASK) ==
> > +                  PCI1XXXX_BR_PERIF_ID) {
> > +                     perif_bus = dev->subordinate;
> > +                     list_for_each_entry(perif_dev, &perif_bus->devices,
> > +                                         bus_list) {
> > +                             if (perif_dev->vendor == PCI1XXXX_VENDOR_ID &&
> > +                                 (perif_dev->device & ~PCI1XXXX_DEV_MASK) ==
> > +                                  perif_id)
> > +                                     return pci_get_drvdata(perif_dev);
> > +                     }
> > +             }
> > +     }
> 
> It would be good to have the PCI Maintainers review of this. Maybe
> pull this out into a patch of its own and Cc: Bjorn Helgaas
> <bhelgaas@xxxxxxxxxx>

Sure. I will add Cc: Bjorn Helgaas.

> 
>         Andrew

-- 
Thanks,                                                                         
Raju