[review] aleksander/huawei-ndisdup-cdc-wdm-rebased-1.2

[Aleksander Morgado]

On Thu, Jun 12, 2014 at 9:47 PM, Dan Williams <dcbw at redhat.com> wrote:
>> huawei: use the cdc-wdm port dialing if available
>
> g_clear_object() instead of if (foo) g_object_unref (foo)?
>

I usually prefer to use g_clear_object() when I explicitly want the
foo pointer to be re-seted to NULL after the unref(); otherwise I see
"if(foo) unref(foo)" just cleaner to read... Anyway, I think it may be
just ok to start switching to using g_clear_object() always, even if
we're not going to use the pointer again...

> I'd extract the "find dial port" stuff from connect_3gpp() and
> disconnect_3gpp() into its own function since both code does the same
> thing.
>

Done.

> Also, a rationale in the commit message would be nice; since I forget
> why this was necessary.  Does the modem just always want the commands on
> the cdc-wdm interface if it exists?
>

The modem requires the NDISDUP call in that cdc-wdm interface, or it
won't connect the net port. Added the comment in the commit msg.

> peek_port_at_for_data() seems kind ugly to me; can't we figure this out
> when constructing the modem, instead of hitting up udev every time we
> want to dial?  Plus during modem probe we're guaranteed to only get the
> ports of the modem, so we wouldn't need to do any relationship walking
> right?

The relationship walking is coming from e.g. the QMI and MBIM
implementations. They both do the same stuff when connecting, looking
for which is the correct control port to use with a given interface.
In QMI modems, for example, you may get several WWAN interfaces and
several QMI ports; and the relationship is 1-to-1 there; you cannot
use any QMI port for any WWAN.

In the case of the Huawei driver in the kernel, the logic is likely
the same, there is a direct relationship between the /dev/cdc-wdm port
and the network interface being managed, so I wouldn't remove those
checks.

What we can do, though, is to improve the logic (not only here, but
also for QMI and MBIM) so that the check of which control port
corresponds to which net interface is done once during bootup. I'll do
that, but out of this branch for now.

-- 
Aleksander
https://aleksander.es