RE: [PATCH v5 5/8] thunderbolt: Networking state machine

From: Levy, Amir (Jer)
Date: Sun Jul 31 2016 - 07:59:04 EST


On Thu, Jul 28 2016, 02:35 PM, Lukas Wunner wrote:
> On Thu, Jul 28, 2016 at 11:15:18AM +0300, Amir Levy wrote:
> > + nhi_ctxt->net_devices[
> > + port_num].medium_sts =
>
> Looks like a carriage return slipped in here.

Will be fixed.

>
> In patch [4/8], I've found it a bit puzzling that FW->SW responses and
> FW->SW notifications are defined in icm_nhi.c, whereas SW->FW commands
> are defined in net.h. It would perhaps be more logical to have them all in the
> header file. The FW->SW responses and SW->FW commands are almost
> identical, there are odd spelling differences (CONNEXION vs.
> CONNECTION).

Will move them to the header and will change to CONNECTION.

>
> It would probably be good to explain the PDF acronym somewhere.

Will explain in the enum.

>
> I've skimmed over all patches in the series, too superficial to provide a
> Reviewed-by, it's just too much code to review thoroughly and I also lack the
> hardware to test it, but broadly this LGTM.

Thank you for the review.