public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Leif Lindholm <leif.lindholm@linaro.org>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Pipat/メタワニットポン ピパット" <methavanitpong.pipat@socionext.com>,
	"Masahisa Kojima" <masahisa.kojima@linaro.org>,
	"Masami Hiramatsu" <masami.hiramatsu@linaro.org>
Subject: Re: [PATCH edk2-platforms 08/14] Silicon/Socionext: add driver for NETSEC network controller
Date: Sat, 28 Oct 2017 14:06:42 +0100	[thread overview]
Message-ID: <CAKv+Gu-4aZHKyzeMFD8h-PppmuAtVaK1EtoPZfgE9wEdvdFw-w@mail.gmail.com> (raw)
In-Reply-To: <20170911161201.y3lhr3rq5vz433nz@bivouac.eciton.net>

On 11 September 2017 at 17:12, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Fri, Sep 08, 2017 at 07:23:09PM +0100, Ard Biesheuvel wrote:
>> This adds the NetSecDxe driver provided by Socionext, but reworked
>> extensively to improve compliance with the SimpleNetworkProtocol API,
>> and to avoid uncached allocations for streaming DMA.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/NetsecDxe.c                                                     | 1000 ++++++++++++++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/NetsecDxe.dec                                                   |   47 +
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/NetsecDxe.h                                                     |   88 ++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/NetsecDxe.inf                                                   |   69 +
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h                   |  736 ++++++++++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_basic_type.h            |   45 +
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_version.h               |   24 +
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_basic_access.c              |   88 ++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_basic_access.h              |   52 +
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_desc_ring_access.c          | 1391 +++++++++++++++++++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_desc_ring_access_internal.h |  111 ++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_gmac_access.c               | 1454 ++++++++++++++++++++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_internal.h                  |  210 +++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c                      | 1385 +++++++++++++++++++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc_internal.h             |   38 +
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h                       |  219 +++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg_f_gmac_4mt.h            |  222 +++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg_netsec.h                |  368 +++++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h                                   |   25 +
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/pfdep.h                                         |  265 ++++
>>  Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/netsec_for_uefi/pfdep_uefi.c                                    |  176 +++
>>  21 files changed, 8013 insertions(+)
>>
>> diff --git a/Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/NetsecDxe.c b/Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/NetsecDxe.c
>> new file mode 100644
>> index 000000000000..7c3f12362f14
>> --- /dev/null
>> +++ b/Silicon/Socionext/Synquacer/Drivers/Net/NetsecDxe/NetsecDxe.c
>> @@ -0,0 +1,1000 @@
>> +/** @file
>> +
>> +  Copyright (c) 2016 Socionext Inc. All rights reserved.<BR>
>> +  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
>> +
>> +  This program and the accompanying materials
>> +  are licensed and made available under the terms and conditions of the BSD License
>> +  which accompanies this distribution.  The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include <Library/DebugLib.h>
>> +#include <Library/DmaLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/IoLib.h>
>> +#include <Library/NetLib.h>
>
> Sorted alphabetically, please?
>
>> +
>> +#include "NetsecDxe.h"
>> +#include "netsec_for_uefi/pfdep.h"
>
> Hmm, could that be folded into NetsecDxe.h?
>

Yes, but I would have to modify all the 'platform independent' source
files, and not having to modify them is kind of the point.
...

>> +
>> +  ogma_disable_desc_ring_irq (LanDriver->Handle, OGMA_DESC_RING_ID_NRM_TX,
>> +                              OGMA_CH_IRQ_REG_EMPTY);
>> +
>> +  // ##### configure_mac
>
> In general, it feels like each of these comment headers indicate a
> good place to break a block out into a helper function.
>

Meh. This function is not complex at all, it just does a bunch of
steps in sequence. Don't see the point really.
...

Apologies for snipping the context - my edit window became intolerably
slow due to the size of the email. I /think/ I incorporated all other
feedback you gave to this patch.


  reply	other threads:[~2017-10-28 13:02 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-08 18:23 [PATCH edk2-platforms 00/14] add support for Socionext Synquacer EVB Ard Biesheuvel
2017-09-08 18:23 ` [PATCH edk2-platforms 01/14] Silicon/Synquacer: add package with platform headers Ard Biesheuvel
2017-09-11 13:31   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 02/14] Silicon/Synquacer: add MemoryInitPeiLib implementation Ard Biesheuvel
2017-09-11 13:36   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 03/14] Platform: add support for Socionext Synquacer eval board Ard Biesheuvel
2017-09-11 13:54   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 04/14] Silicon/Synquacer: implement PciSegmentLib to support dual RCs Ard Biesheuvel
2017-09-11 14:03   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 05/14] Silicon/Synquacer: implement PciHostBridgeLib support Ard Biesheuvel
2017-09-11 14:22   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 06/14] Silicon/Synquacer: implement EFI_CPU_IO2_PROTOCOL Ard Biesheuvel
2017-09-11 14:45   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 07/14] Platform/SynquacerEvalBoard: add PCI support Ard Biesheuvel
2017-09-11 14:48   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 08/14] Silicon/Socionext: add driver for NETSEC network controller Ard Biesheuvel
2017-09-11 16:12   ` Leif Lindholm
2017-10-28 13:06     ` Ard Biesheuvel [this message]
2017-10-28 21:25       ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 09/14] Platform/SynquacerEvalBoard: add NETSEC driver Ard Biesheuvel
2017-09-11 16:23   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 10/14] Silicon/Synquacer: add ACPI support Ard Biesheuvel
2017-09-11 16:33   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 11/14] Silicon/Synquacer: add device tree support for eval board Ard Biesheuvel
2017-09-11 16:37   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 12/14] Silicon/Synquacer: add NorFlashPlatformLib implementation Ard Biesheuvel
2017-09-11 16:38   ` Leif Lindholm
2017-09-08 18:23 ` [PATCH edk2-platforms 13/14] Silicon/Socionext: add driver for SPI NOR flash Ard Biesheuvel
2017-09-11 19:13   ` Leif Lindholm
     [not found]     ` <e55ff6c595f74189bd53787f3b6b2283@SOC-EX03V.e01.socionext.com>
2017-09-12  8:38       ` Leif Lindholm
2017-09-12 10:48         ` methavanitpong.pipat
2017-09-08 18:23 ` [PATCH edk2-platforms 14/14] Platform/Synquacer: incorporate NOR flash and variable drivers Ard Biesheuvel
2017-09-11 19:13   ` Leif Lindholm

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-4aZHKyzeMFD8h-PppmuAtVaK1EtoPZfgE9wEdvdFw-w@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