<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 23, 2017 at 2:11 PM, Ben Chan <span dir="ltr"><<a href="mailto:benchan@chromium.org" target="_blank">benchan@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Thu, Feb 23, 2017 at 2:07 PM, Dan Williams <span dir="ltr"><<a href="mailto:dcbw@redhat.com" target="_blank">dcbw@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On Thu, 2017-02-23 at 11:45 -0800, Ben Chan wrote:<br>
> This patch simplifies the handling of platform/pci/pnp/sdio device in<br>
> find_physical_gudevdevice(). When the code finds the first parent<br>
> device<br>
> under the subsystem 'platform', 'pci', 'pnp', or 'sdio', it stops<br>
> traversing up the device tree, so there is no need to keep track of<br>
> whether a platform / pci / pnp / sdio device has previously been<br>
> visited<br>
> via those is_platform/is_pci/is_pnp/is_s<wbr>dio Boolean variables.<br>
<br>
</span>I'm not sure this will work in all cases, though I should verify this.<br>
Depending on the actual bus type, the common parent for all ports<br>
exposed by that device might be a couple parents up the chain.<br>
<br></blockquote><div><br></div></span><div>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.</div></div></div></div></blockquote><div><br></div><div>The third patch aside, what do you think about the first two?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I do still have PCMCIA (Sierra 860) and PCI (Option Nozomi) WWAN cards<br>
that I can check to see if this is the case, though I'll have to<br>
install an older OS that still supports PCMCIA.<br>
<br>
The "platform" case IIRC is when a serial device is attached to the<br>
onboard serial port, so perhaps that one is OK since there will only<br>
ever be one device on those ports.<br>
<br>
Dan<br>
<br>
> ---<br>
>  src/kerneldevice/mm-kernel-de<wbr>vice-udev.c | 23 ++++++--------------<br>
<div class="m_-3605171761685517405HOEnZb"><div class="m_-3605171761685517405h5">> ---<br>
>  1 file changed, 6 insertions(+), 17 deletions(-)<br>
><br>
> diff --git a/src/kerneldevice/mm-kernel-d<wbr>evice-udev.c<br>
> b/src/kerneldevice/mm-kernel-d<wbr>evice-udev.c<br>
> index 763ccf86..ab75aa34 100644<br>
> --- a/src/kerneldevice/mm-kernel-d<wbr>evice-udev.c<br>
> +++ b/src/kerneldevice/mm-kernel-d<wbr>evice-udev.c<br>
> @@ -177,8 +177,7 @@ find_physical_gudevdevice (GUdevDevice *child)<br>
>      GUdevDevice *physdev = NULL;<br>
>      const char *subsys, *type, *name;<br>
>      guint32 i = 0;<br>
> -    gboolean is_usb = FALSE, is_pci = FALSE, is_pcmcia = FALSE,<br>
> is_platform = FALSE;<br>
> -    gboolean is_pnp = FALSE, is_sdio = FALSE;<br>
> +    gboolean is_usb = FALSE, is_pcmcia = FALSE;<br>
>  <br>
>      g_return_val_if_fail (child != NULL, NULL);<br>
>  <br>
> @@ -220,21 +219,11 @@ find_physical_gudevdevice (GUdevDevice *child)<br>
>                      if (physdev)<br>
>                          break<wbr>;<br>
>                  }<br>
> -            } else if (is_platform || !strcmp (subsys, "platform"))<br>
> {<br>
> -                /* Take the first platform parent as the physical<br>
> device */<br>
> -                is_platform = TRUE;<br>
> -                physdev = iter;<br>
> -                break;<br>
> -            } else if (is_pci || !strcmp (subsys, "pci")) {<br>
> -                is_pci = TRUE;<br>
> -                physdev = iter;<br>
> -                break;<br>
> -            } else if (is_pnp || !strcmp (subsys, "pnp")) {<br>
> -                is_pnp = TRUE;<br>
> -                physdev = iter;<br>
> -                break;<br>
> -            } else if (is_sdio || !strcmp (subsys, "sdio")) {<br>
> -                is_sdio = TRUE;<br>
> +            } else if (!strcmp (subsys, "platform") ||<br>
> +                       !strcm<wbr>p (subsys, "pci") ||<br>
> +                       !strcm<wbr>p (subsys, "pnp") ||<br>
> +                       !strcm<wbr>p (subsys, "sdio")) {<br>
> +                /* Take the first parent as the physical device */<br>
>                  physdev = iter;<br>
>                  break;<br>
>              }<br>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>