[PATCH 3/3] kernel-device: simplify handling of platform/pci/pnp/sdio parent

[Ben Chan]

On Thu, Feb 23, 2017 at 2:11 PM, Ben Chan <benchan at chromium.org> wrote:

>
>
> On Thu, Feb 23, 2017 at 2:07 PM, Dan Williams <dcbw at redhat.com> wrote:
>
>> On Thu, 2017-02-23 at 11:45 -0800, Ben Chan wrote:
>> > This patch simplifies the handling of platform/pci/pnp/sdio device in
>> > find_physical_gudevdevice(). When the code finds the first parent
>> > device
>> > under the subsystem 'platform', 'pci', 'pnp', or 'sdio', it stops
>> > traversing up the device tree, so there is no need to keep track of
>> > whether a platform / pci / pnp / sdio device has previously been
>> > visited
>> > via those is_platform/is_pci/is_pnp/is_sdio Boolean variables.
>>
>> I'm not sure this will work in all cases, though I should verify this.
>> Depending on the actual bus type, the common parent for all ports
>> exposed by that device might be a couple parents up the chain.
>>
>>
> That's also what I was wondering when reading through the original code.
> If that's the case, the original code would have stopped too soon when
> traversing up the chain.
>

The third patch aside, what do you think about the first two?


>
>
>> I do still have PCMCIA (Sierra 860) and PCI (Option Nozomi) WWAN cards
>> that I can check to see if this is the case, though I'll have to
>> install an older OS that still supports PCMCIA.
>>
>> The "platform" case IIRC is when a serial device is attached to the
>> onboard serial port, so perhaps that one is OK since there will only
>> ever be one device on those ports.
>>
>> Dan
>>
>> > ---
>> >  src/kerneldevice/mm-kernel-device-udev.c | 23 ++++++--------------
>> > ---
>> >  1 file changed, 6 insertions(+), 17 deletions(-)
>> >
>> > diff --git a/src/kerneldevice/mm-kernel-device-udev.c
>> > b/src/kerneldevice/mm-kernel-device-udev.c
>> > index 763ccf86..ab75aa34 100644
>> > --- a/src/kerneldevice/mm-kernel-device-udev.c
>> > +++ b/src/kerneldevice/mm-kernel-device-udev.c
>> > @@ -177,8 +177,7 @@ find_physical_gudevdevice (GUdevDevice *child)
>> >      GUdevDevice *physdev = NULL;
>> >      const char *subsys, *type, *name;
>> >      guint32 i = 0;
>> > -    gboolean is_usb = FALSE, is_pci = FALSE, is_pcmcia = FALSE,
>> > is_platform = FALSE;
>> > -    gboolean is_pnp = FALSE, is_sdio = FALSE;
>> > +    gboolean is_usb = FALSE, is_pcmcia = FALSE;
>> >
>> >      g_return_val_if_fail (child != NULL, NULL);
>> >
>> > @@ -220,21 +219,11 @@ find_physical_gudevdevice (GUdevDevice *child)
>> >                      if (physdev)
>> >                          break;
>> >                  }
>> > -            } else if (is_platform || !strcmp (subsys, "platform"))
>> > {
>> > -                /* Take the first platform parent as the physical
>> > device */
>> > -                is_platform = TRUE;
>> > -                physdev = iter;
>> > -                break;
>> > -            } else if (is_pci || !strcmp (subsys, "pci")) {
>> > -                is_pci = TRUE;
>> > -                physdev = iter;
>> > -                break;
>> > -            } else if (is_pnp || !strcmp (subsys, "pnp")) {
>> > -                is_pnp = TRUE;
>> > -                physdev = iter;
>> > -                break;
>> > -            } else if (is_sdio || !strcmp (subsys, "sdio")) {
>> > -                is_sdio = TRUE;
>> > +            } else if (!strcmp (subsys, "platform") ||
>> > +                       !strcmp (subsys, "pci") ||
>> > +                       !strcmp (subsys, "pnp") ||
>> > +                       !strcmp (subsys, "sdio")) {
>> > +                /* Take the first parent as the physical device */
>> >                  physdev = iter;
>> >                  break;
>> >              }
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170224/ebd1e483/attachment.html>