Re: [PATCH] gpio:asm28xx-18xx: new driver

[Bartosz Golaszewski <brgl@xxxxxxxx>]

czw., 4 cze 2020 o 19:55 Bjorn Helgaas <helgaas@xxxxxxxxxx> napisał(a):
>
> > > +       /* We look for our device - Asmedia 28XX and 18XX Bridge
> > > +        * I don't know about a system with two such bridges,
> > > +        * so we can assume that there is max. one device.
> > > +        *
> > > +        * We can't use plain pci_driver mechanism,
> > > +        * as the device is really a multiple function device,
> > > +        * main driver that binds to the pci_device is an bus
> > > +        * driver and have to find & bind to the device this way.
> > > +        */
> > > +
> > > +       for_each_pci_dev(pdev) {
> > > +               ent = pci_match_id(pci_tbl, pdev);
> > > +               if (ent) {
> > > +                       /* Because GPIO Registers only work on Upstream port. */
> > > +                       type = pci_pcie_type(pdev);
> > > +                       if (type == PCI_EXP_TYPE_UPSTREAM) {
> > > +                               dev_info(&pdev->dev, "ASMEDIA-28xx/18xx Init Upstream detected\n");
> > > +                               goto found;
> > > +                       }
> > > +               }
> > > +       }
> > > +       goto out;
> > > +
> >
> > Bjorn: is this approach really correct? It looks very strange to me
> > and even if we were to do this kind of lookup I'd expect there to be a
> > real pci device registered as child of pdev here so that we can have a
> > proper driver in place with probe() et al.
>
> No, this is pretty broken.  The model is that one PCI device goes with
> one driver.  If there are two bits of functionality associated with a
> single PCI device, it's up to the single PCI driver to sort that out.
>
> The comment above mentions "multiple function device," which may lead
> to some confusion about the terminology.  In the PCI specs, the
> smallest addressable unit of PCI hardware is the "Function."  A
> "Device" may consist of one or more Functions.  A Device with more
> than one Function is referred to in the spec as a "Multi-Function
> Device".
>
> These PCI Functions are addressed by a (domain, bus, device, function)
> tuple.  For example, my system has these:
>
>   0000:00:14.0 Intel USB 3.0 xHCI Controller
>   0000:00:14.2 Intel Thermal Subsystem
>
> These two Functions are parts of the 0000:00:14 Multi-Function Device.
>
> In Linux, a "struct pci_dev" refers to a single Function, so there's
> a pci_dev for 0000:00:14.0 and another for 0000:00:14.2.  These are
> pretty much independent, and can be claimed by two separate drivers.
>
> But I think the "multiple function device" comment in *this* patch
> probably doesn't refer to a "Multi-Function Device" as used in the PCI
> specs.  It probably means a single PCI Function that has two kinds of
> functionality.
>
> In the Linux model, that means the Function should be claimed by a
> single driver, and that driver is responsible for coordinating the two
> pieces of functionality.
>

Thanks for the detailed explanation!

Richard: in this case I think it's pretty clear now that whatever
driver supports the "bridge" mentioned in the comment - needs to be
extended with GPIO functionality.

Bart