From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4001:c06::22a; helo=mail-io0-x22a.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-io0-x22a.google.com (mail-io0-x22a.google.com [IPv6:2607:f8b0:4001:c06::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0B2A52034A890 for ; Fri, 27 Oct 2017 07:46:12 -0700 (PDT) Received: by mail-io0-x22a.google.com with SMTP id e89so13202593ioi.11 for ; Fri, 27 Oct 2017 07:50:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=E4Tphowz9a8vQZUeQ4tn3Oy6lQHjNErZejNxyJx9FvU=; b=eJTahdRk4nsYZyR+EtwtLto1S6Em/OgkKmelhzDkz7AHVcRKPeoJmy9J/z5/rqxc3x TwN9h1e75ZLjtCH/iCmevtOgvwUNwU3EODkoH8oWjg4H0wu8lOCMqLFz1MbnXSrNQx/w nobZYMzKHtMKIIc1sDoCESlJpa81LHJK8DiagYIoysOB+PxIn/hwZ19/9g4iCoTYI/N6 lBxjGkjFIRqtEqn1LoUZtY6qReAqrgRCcRWziYWF4Sxh3YcSLihoCgHRORe53QXvzLRm HQieIjYXlkknPcQ/6WxEoh4+fH5C7ddtKFY9U1zGALQlGt/8gVJpyxv7GJEQfZPVXRc8 /y4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=E4Tphowz9a8vQZUeQ4tn3Oy6lQHjNErZejNxyJx9FvU=; b=HwKSg5DKTwxAGpmKwb2455qjA8lHsqpTpRw2zlndGGOuVz/bGGR1z0lWVYKUPGGHyS 6gLGLmL3uAlIrDQao6haPLf1PDEEX+J9p7TmDyawGh1RIVEAkPtMTn5A/9e3AwTX5TBC cppv56g1fH6nJle/kg74yQbx9fWqfzuS/PxHOgK2slE5ocgG+geBJ8FybfMhMn/n8KLZ rcpyNtLDOIwB4z6kCHNoQuAKpcbCKdSDMAhyAeNrQbYrIMMEal1XwlSsFXo9ZU4zVUKf pbju5j+ls2udi4mDlr6NaOFS+lMp/xfTPFiDRSsqUf0LexbTNAEq7001HMffFlcnvPF+ OPcg== X-Gm-Message-State: AMCzsaUMm8T07MOLBo7DcPlm5AGLVJq5Jmu72w49AV9cx03YTOCMBmCy oIW8u8zMIXeU+Ugt/dINTdh6bEjVWaFtM0dj2YvnCg== X-Google-Smtp-Source: ABhQp+St8UwxXhtsZqQE59a891fkvmAWDMpb3b06eXyOfIX6ZcjN2SsO+dppILxQRwtN2FOiyInlndsByKQSJYnnwbY= X-Received: by 10.107.130.162 with SMTP id m34mr938136ioi.276.1509115799609; Fri, 27 Oct 2017 07:49:59 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.167.208 with HTTP; Fri, 27 Oct 2017 07:49:59 -0700 (PDT) In-Reply-To: <20171027143640.msneayaflrcc6u54@bivouac.eciton.net> References: <1509066832-5285-1-git-send-email-mw@semihalf.com> <1509066832-5285-8-git-send-email-mw@semihalf.com> <20171027143640.msneayaflrcc6u54@bivouac.eciton.net> From: Marcin Wojtas Date: Fri, 27 Oct 2017 16:49:59 +0200 Message-ID: To: Leif Lindholm Cc: edk2-devel-01 , Ard Biesheuvel , nadavh@marvell.com, Neta Zur Hershkovits , Kostya Porotchkin , Hua Jing , semihalf-dabros-jan Subject: Re: [platforms: PATCH v2 07/10] Marvell/Drivers: Pp2Dxe: Change settings for the always-up link X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Oct 2017 14:46:13 -0000 Content-Type: text/plain; charset="UTF-8" 2017-10-27 16:36 GMT+02:00 Leif Lindholm : > > On Fri, Oct 27, 2017 at 03:13:49AM +0200, Marcin Wojtas wrote: > > Currently initial forcing link status happened for all ports, not only > > marked as 'always-up'. Although this didn't actually matter for the MAC > > settings, because MAC is automatically updated with PHY HW polling > > feature of the controller, perform mv_gop110_fl_cfg only when > > the appropriate flag is true. Also in such case, force the link as up, > > using a new library routine. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas > > --- > > Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c | 25 ++++++++++++++++++++ > > Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h | 6 +++++ > > Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 6 ++++- > > 3 files changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c > > index 53154db..c2d0199 100644 > > --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c > > +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c > > @@ -4804,6 +4804,31 @@ MvGop110PortEventsMask ( > > return 0; > > } > > > > +/* > > + * Sets "Force Link Pass" and "Do Not Force Link Fail" bits. > > + * This function should only be called when the port is disabled. > > + */ > > +VOID > > +MvGop110GmacForceLinkModeSet( > > + IN PP2DXE_PORT *Port, > > + IN BOOLEAN LinkUp > > + ) > > +{ > > + UINT32 RegVal; > > + > > + RegVal = MvGop110GmacRead (Port, MVPP2_PORT_AUTO_NEG_CFG_REG); > > + > > + if (LinkUp) { > > + RegVal |= MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_UP_MASK; > > + RegVal &= ~MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_DOWN_MASK; > > So, I know UP and DOWN are separate bits (what's that about!?), but > unless you know that, the above two lines look like they're in the > wrong order. > There is a 3rd case that if both are set to '1', autonegotiation is forced, but we don't use it. I can even more simplify according to the usecase (only 1 call in Pp2Dxe driver): VOID MvGop110GmacForceLinkUp( IN PP2DXE_PORT *Port ) { UINT32 RegVal; RegVal = MvGop110GmacRead (Port, MVPP2_PORT_AUTO_NEG_CFG_REG); RegVal |= MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_UP_MASK; RegVal &= ~MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_DOWN_MASK; MvGop110GmacWrite (Port, MVPP2_PORT_AUTO_NEG_CFG_REG, RegVal); } What do you think? > If you flip those around (and make it like the 'else' clause): Ok. > Reviewed-by: Leif Lindholm > > > + } else { > > + RegVal &= ~MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_UP_MASK; > > + RegVal |= MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_DOWN_MASK; > > + } > > + > > + MvGop110GmacWrite (Port, MVPP2_PORT_AUTO_NEG_CFG_REG, RegVal); > > +} > > + > > INT32 > > MvGop110FlCfg ( > > IN PP2DXE_PORT *Port > > diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h > > index a7011f7..3ebe294 100644 > > --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h > > +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h > > @@ -504,6 +504,12 @@ MvGop110XlgPortLinkEventMask ( > > IN PP2DXE_PORT *Port > > ); > > > > +VOID > > +MvGop110GmacForceLinkModeSet ( > > + IN PP2DXE_PORT *Port, > > + IN BOOLEAN LinkUp > > + ); > > + > > INT32 > > MvGop110FlCfg ( > > IN PP2DXE_PORT *Port > > diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c > > index 2827976..4a1b9d5 100644 > > --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c > > +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c > > @@ -1310,7 +1310,11 @@ Pp2DxeInitialiseController ( > > NetCompConfig |= MvpPp2xGop110NetcCfgCreate(&Pp2Context->Port); > > > > MvGop110PortInit(&Pp2Context->Port); > > - MvGop110FlCfg(&Pp2Context->Port); > > + > > + if (Pp2Context->Port.AlwaysUp == TRUE) { > > + MvGop110GmacForceLinkModeSet (&Pp2Context->Port, TRUE); > > + MvGop110FlCfg (&Pp2Context->Port); > > + } > > > > Status = gBS->CreateEvent ( > > EVT_SIGNAL_EXIT_BOOT_SERVICES, > > -- > > 2.7.4 > >