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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Bjorn Helgass,
    Thanks for your detailed explanation.
Hi Bartosz Golaszewski,
    I am grateful for your suggestion. The driver of bridge is a pci bus driver. It isn't written by us and is more complex. I have zero knowledge of
 the PCI sub-system, and perhaps it(pci bus driver) don't use gpio subsystem with /sys/class/gpio/(sysfs interface) that I need. I just follow the other driver(gpio-amd8111.c)'s
framework and maybe it can be used again. Thank you 

BR,
  Richard


       
    

-----Original Message-----
From: Bartosz Golaszewski <brgl@xxxxxxxx> 
Sent: Friday, June 5, 2020 3:56 PM
To: Bjorn Helgaas <helgaas@xxxxxxxxxx>
Cc: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>; Richard Hsu <saraon640529@xxxxxxxxx>; Richard Hsu(許育彰) <Richard_Hsu@xxxxxxxxxxxxxx>; Yd Tseng(曾裕達) <Yd_Tseng@xxxxxxxxxxxxxx>; Jesse1 Chang(張國器) <Jesse1_Chang@xxxxxxxxxxxxxx>; Linus Walleij <linus.walleij@xxxxxxxxxx>; Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; kbuild test robot <lkp@xxxxxxxxx>; kbuild-all@xxxxxxxxxxxx; linux-gpio <linux-gpio@xxxxxxxxxxxxxxx>; linux-pci@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] gpio:asm28xx-18xx: new driver

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
==================================================================================================================
This email and any attachments to it contain confidential information and are intended solely for the use of the individual to whom it 
is addressed.If you are not the intended recipient or receive it accidentally, please immediately notify the sender by e-mail and delete 
the message and any attachments from your computer system, and destroy all hard copies. If any, please be advised that any unauthorized 
disclosure, copying, distribution or any action taken or omitted in reliance on this, is illegal and prohibited. Furthermore, any views 
or opinions expressed are solely those of the author and do not represent those of ASMedia Technology Inc. Thank you for your cooperation.
==================================================================================================================




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux