From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Marcin Wojtas <mw@semihalf.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
"Leif Lindholm" <leif.lindholm@linaro.org>,
"Nadav Haklai" <nadavh@marvell.com>,
"Neta Zur Hershkovits" <neta@marvell.com>,
"Kostya Porotchkin" <kostap@marvell.com>,
jinghua@marvell.com, "Alexander Graf" <agraf@suse.de>,
"Jan Dąbroś" <jsd@semihalf.com>
Subject: Re: [platforms: PATCH 7/7] Drivers/Net/Pp2Dxe: Enable using ports from different controllers
Date: Fri, 1 Sep 2017 10:34:25 +0100 [thread overview]
Message-ID: <CAKv+Gu-s_n_0ZT2Gmn2Z1CQdCBqd8MHOXZh8q1NwN===UUhTOQ@mail.gmail.com> (raw)
In-Reply-To: <1504233451-6455-8-git-send-email-mw@semihalf.com>
On 1 September 2017 at 03:37, Marcin Wojtas <mw@semihalf.com> wrote:
> After Pp2Dxe data migrated to MvHwDescLib, both controllers
> could be used, but not at the same time. It was caused by
> ports' insufficient description. This patch fixes this problem by
> introducing new PCD responsible for the mapping between port and
> its controller. Also it was possible to remove redundant
> PcdPp2NumPorts. Update documentation accordingly.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Platform/Marvell/Armada/Armada70x0.dsc | 2 +-
> .../Marvell/Documentation/PortingGuide/Pp2.txt | 4 +-
> Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c | 63 ++++++++++++++--------
> Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h | 1 +
> Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf | 2 +-
> Platform/Marvell/Marvell.dec | 2 +-
> 6 files changed, 47 insertions(+), 27 deletions(-)
>
> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
> index 334bfaa..f519196 100644
> --- a/Platform/Marvell/Armada/Armada70x0.dsc
> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
> @@ -124,7 +124,7 @@
> gMarvellTokenSpaceGuid.PcdPp2GopIndexes|{ 0x0, 0x2, 0x3 }
> gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp|{ 0x0, 0x0, 0x0 }
> gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed|{ 0x5, 0x3, 0x3 }
> - gMarvellTokenSpaceGuid.PcdPp2NumPorts|3
> + gMarvellTokenSpaceGuid.PcdPp2Port2Controller|{ 0x0, 0x0, 0x0 }
> gMarvellTokenSpaceGuid.PcdPp2PortIds|{ 0x0, 0x1, 0x2 }
> gMarvellTokenSpaceGuid.PcdPp2Controllers|{ 0x1 }
>
> diff --git a/Platform/Marvell/Documentation/PortingGuide/Pp2.txt b/Platform/Marvell/Documentation/PortingGuide/Pp2.txt
> index 9b829c9..f05ba27 100644
> --- a/Platform/Marvell/Documentation/PortingGuide/Pp2.txt
> +++ b/Platform/Marvell/Documentation/PortingGuide/Pp2.txt
> @@ -6,8 +6,8 @@ are required to operate:
> Array with used controllers - Set to 0x1 for enabled, 0x0 for disabled:
> gMarvellTokenSpaceGuid.PcdPp2Controllers
>
> -Number of ports/network interfaces:
> - gMarvellTokenSpaceGuid.PcdPp2NumPorts
> +Array specifying, to which controller the port belongs to:
> + gMarvellTokenSpaceGuid.PcdPp2Port2Controller
>
> Addresses of PHY devices:
> gMarvellTokenSpaceGuid.PcdPhySmiAddresses
> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> index 8e6bfbc..620bd5c 100644
> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> @@ -508,9 +508,7 @@ Pp2DxePhyInitialize (
> )
> {
> EFI_STATUS Status;
> - UINT8 *PhyAddresses;
>
> - PhyAddresses = PcdGetPtr (PcdPhySmiAddresses);
> Status = gBS->LocateProtocol (
> &gMarvellPhyProtocolGuid,
> NULL,
> @@ -521,14 +519,14 @@ Pp2DxePhyInitialize (
> return Status;
> }
>
> - if (PhyAddresses[Pp2Context->Instance] == 0xff) {
> + if (Pp2Context->Port.PhyAddr == 0xff) {
> /* PHY iniitalization not required */
> return EFI_SUCCESS;
> }
>
> Status = Pp2Context->Phy->Init(
> Pp2Context->Phy,
> - PhyAddresses[Pp2Context->Instance],
> + Pp2Context->Port.PhyAddr,
> Pp2Context->Port.PhyInterface,
> &Pp2Context->PhyDev
> );
> @@ -1145,14 +1143,16 @@ Pp2DxeSnpInstall (
> STATIC
> VOID
> Pp2DxeParsePortPcd (
> - IN PP2DXE_CONTEXT *Pp2Context
> + IN PP2DXE_CONTEXT *Pp2Context,
> + IN INTN Index
> )
> {
> - UINT8 *PortIds, *GopIndexes, *PhyConnectionTypes, *AlwaysUp, *Speed;
> + UINT8 *PortIds, *GopIndexes, *PhyConnectionTypes, *AlwaysUp, *Speed, *PhyAddresses;
>
> PortIds = PcdGetPtr (PcdPp2PortIds);
> GopIndexes = PcdGetPtr (PcdPp2GopIndexes);
> PhyConnectionTypes = PcdGetPtr (PcdPhyConnectionTypes);
> + PhyAddresses = PcdGetPtr (PcdPhySmiAddresses);
> AlwaysUp = PcdGetPtr (PcdPp2InterfaceAlwaysUp);
> Speed = PcdGetPtr (PcdPp2InterfaceSpeed);
>
> @@ -1160,17 +1160,20 @@ Pp2DxeParsePortPcd (
> ASSERT (PcdGetSize (PcdPhyConnectionTypes) == PcdGetSize (PcdPp2PortIds));
> ASSERT (PcdGetSize (PcdPp2InterfaceAlwaysUp) == PcdGetSize (PcdPp2PortIds));
> ASSERT (PcdGetSize (PcdPp2InterfaceSpeed) == PcdGetSize (PcdPp2PortIds));
> -
> - Pp2Context->Port.Id = PortIds[Pp2Context->Instance];
> - Pp2Context->Port.GopIndex = GopIndexes[Pp2Context->Instance];
> - Pp2Context->Port.PhyInterface = PhyConnectionTypes[Pp2Context->Instance];
> - Pp2Context->Port.AlwaysUp = AlwaysUp[Pp2Context->Instance];
> - Pp2Context->Port.Speed = Speed[Pp2Context->Instance];
> + ASSERT (PcdGetSize (PcdPhySmiAddresses) == PcdGetSize (PcdPp2PortIds));
> +
> + Pp2Context->Port.Id = PortIds[Index];
> + Pp2Context->Port.GopIndex = GopIndexes[Index];
> + Pp2Context->Port.PhyInterface = PhyConnectionTypes[Index];
> + Pp2Context->Port.PhyAddr = PhyAddresses[Index];
> + Pp2Context->Port.AlwaysUp = AlwaysUp[Index];
> + Pp2Context->Port.Speed = Speed[Index];
> }
>
> STATIC
> EFI_STATUS
> Pp2DxeInitialiseController (
> + IN UINT8 ControllerIndex,
> IN MVPP2_SHARED *Mvpp2Shared,
> IN UINTN BaseAddress,
> IN UINTN ClockFrequency
> @@ -1179,14 +1182,11 @@ Pp2DxeInitialiseController (
> PP2DXE_CONTEXT *Pp2Context = NULL;
> EFI_STATUS Status;
> INTN Index;
> + INTN PortIndex = 0;
> VOID *BufferSpace;
> UINT32 NetCompConfig = 0;
> - UINT8 NumPorts = PcdGet32 (PcdPp2NumPorts);
> -
> - if (NumPorts == 0) {
> - DEBUG((DEBUG_ERROR, "Pp2Dxe: port number set to 0\n"));
> - return EFI_INVALID_PARAMETER;
> - }
> + STATIC UINT8 DeviceInstance;
> + UINT8 *Pp2PortMappingTable;
>
> Mvpp2Shared->Base = BaseAddress;
> Mvpp2Shared->Rfu1Base = Mvpp2Shared->Base + MVPP22_RFU1_OFFSET;
> @@ -1265,7 +1265,18 @@ Pp2DxeInitialiseController (
> Mvpp2Shared->AggrTxqs->LogId = 0;
> Mvpp2Shared->AggrTxqs->Size = MVPP2_AGGR_TXQ_SIZE;
>
> - for (Index = 0; Index < NumPorts; Index++) {
> + Pp2PortMappingTable = (UINT8 *)PcdGetPtr (PcdPp2Port2Controller);
> +
> + for (Index = 0; Index < PcdGetSize (PcdPp2Port2Controller); Index++) {
> + if (Pp2PortMappingTable[Index] != ControllerIndex) {
> + continue;
> + }
> +
> + if (PortIndex++ > MVPP2_MAX_PORT) {
> + DEBUG ((DEBUG_ERROR, "Pp2Dxe: Wrong too many ports for single controller\n"));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> Pp2Context = AllocateZeroPool (sizeof (PP2DXE_CONTEXT));
> if (Pp2Context == NULL) {
> /*
> @@ -1277,7 +1288,8 @@ Pp2DxeInitialiseController (
> }
>
> /* Instances are enumerated from 0 */
> - Pp2Context->Instance = Index;
> + Pp2Context->Instance = DeviceInstance;
> + DeviceInstance++;
>
> /* Install SNP protocol */
> Status = Pp2DxeSnpInstall(Pp2Context);
> @@ -1285,10 +1297,10 @@ Pp2DxeInitialiseController (
> return Status;
> }
>
> - Pp2DxeParsePortPcd(Pp2Context);
> + Pp2DxeParsePortPcd(Pp2Context, Index);
> Pp2Context->Port.TxpNum = 1;
> Pp2Context->Port.Priv = Mvpp2Shared;
> - Pp2Context->Port.FirstRxq = 4 * Pp2Context->Instance;
> + Pp2Context->Port.FirstRxq = 4 * (PortIndex - 1);
> Pp2Context->Port.GmacBase = Mvpp2Shared->Base + MVPP22_GMAC_OFFSET +
> MVPP22_GMAC_REG_SIZE * Pp2Context->Port.GopIndex;
> Pp2Context->Port.XlgBase = Mvpp2Shared->Base + MVPP22_XLG_OFFSET +
> @@ -1343,6 +1355,12 @@ Pp2DxeInitialise (
> return EFI_INVALID_PARAMETER;
> }
>
> + /* Check amount of declared ports */
> + if (PcdGetSize (PcdPp2Port2Controller) > Desc->Pp2DevCount * MVPP2_MAX_PORT) {
> + DEBUG ((DEBUG_ERROR, "Pp2Dxe: Wrong too many ports declared\n"));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> /* Initialize enabled chips */
> for (Index = 0; Index < PcdGetSize (PcdPp2Controllers); Index++) {
> if (!MVHW_DEV_ENABLED (Pp2, Index)) {
> @@ -1358,6 +1376,7 @@ Pp2DxeInitialise (
> }
>
> Status = Pp2DxeInitialiseController (
> + Index,
> Mvpp2Shared,
> Desc->Pp2BaseAddresses[Index],
> Desc->Pp2ClockFrequency[Index]
> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
> index 7071cef..cde2995 100644
> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
> @@ -327,6 +327,7 @@ struct Pp2DxePort {
> UINT16 RxRingSize;
>
> INT32 PhyInterface;
> + UINTN PhyAddr;
> BOOLEAN Link;
> BOOLEAN Duplex;
> BOOLEAN AlwaysUp;
> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> index b67162d..752fcc0 100644
> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> @@ -77,7 +77,7 @@
> gMarvellTokenSpaceGuid.PcdPp2GopIndexes
> gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp
> gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed
> - gMarvellTokenSpaceGuid.PcdPp2NumPorts
> + gMarvellTokenSpaceGuid.PcdPp2Port2Controller
> gMarvellTokenSpaceGuid.PcdPp2PortIds
>
> [Depex]
> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
> index e6a3621..4e2dd6d 100644
> --- a/Platform/Marvell/Marvell.dec
> +++ b/Platform/Marvell/Marvell.dec
> @@ -173,7 +173,7 @@
> gMarvellTokenSpaceGuid.PcdPp2GopIndexes|{ 0x0 }|VOID*|0x3000029
> gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp|{ 0x0 }|VOID*|0x300002A
> gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed|{ 0x0 }|VOID*|0x300002B
> - gMarvellTokenSpaceGuid.PcdPp2NumPorts|0|UINT32|0x300002D
> + gMarvellTokenSpaceGuid.PcdPp2Port2Controller|{ 0x0 }|VOID*|0x300002D
> gMarvellTokenSpaceGuid.PcdPp2PortIds|{ 0x0 }|VOID*|0x300002C
>
> #PciEmulation
> --
> 1.8.3.1
>
next prev parent reply other threads:[~2017-09-01 9:31 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-01 2:37 [platforms: PATCH 0/7] Armada 70x0/80x0 network improvements Marcin Wojtas
2017-09-01 2:37 ` [platforms: PATCH 1/7] Drivers/Net/Pp2Dxe: Move registers' description to macros Marcin Wojtas
2017-09-01 9:22 ` Ard Biesheuvel
2017-09-01 2:37 ` [platforms: PATCH 2/7] Drivers/Net/Pp2Dxe: Add SFI support Marcin Wojtas
2017-09-01 9:26 ` Ard Biesheuvel
2017-09-01 9:39 ` Marcin Wojtas
2017-09-01 9:40 ` Ard Biesheuvel
2017-09-01 9:49 ` Marcin Wojtas
2017-09-01 9:51 ` Ard Biesheuvel
2017-09-01 2:37 ` [platforms: PATCH 3/7] Drivers/Net/Pp2Dxe: Support multiple ethernet ports simultaneously Marcin Wojtas
2017-09-01 9:30 ` Ard Biesheuvel
2017-09-01 2:37 ` [platforms: PATCH 4/7] Drivers/Net/Pp2Dxe: Increase amount of ingress resources Marcin Wojtas
2017-09-01 9:30 ` Ard Biesheuvel
2017-09-01 2:37 ` [platforms: PATCH 5/7] Platforms/Marvell: Update ethernet ports types on A70x0 DB Marcin Wojtas
2017-09-01 9:31 ` Ard Biesheuvel
2017-09-01 2:37 ` [platforms: PATCH 6/7] Drivers/Net/Pp2Dxe: Move devices description to MvHwDescLib Marcin Wojtas
2017-09-01 9:33 ` Ard Biesheuvel
2017-09-01 2:37 ` [platforms: PATCH 7/7] Drivers/Net/Pp2Dxe: Enable using ports from different controllers Marcin Wojtas
2017-09-01 9:34 ` Ard Biesheuvel [this message]
2017-09-01 9:27 ` [platforms: PATCH 0/7] Armada 70x0/80x0 network improvements Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKv+Gu-s_n_0ZT2Gmn2Z1CQdCBqd8MHOXZh8q1NwN===UUhTOQ@mail.gmail.com' \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox