Re: [edk2-devel] [edk2-platforms PATCH v1 1/3] NetsecDxe: embed phy address into NETSEC SDK internal structure

2019-07-23 Thread Leif Lindholm
On Mon, Jul 22, 2019 at 08:56:34PM +0900, Masahisa Kojima wrote:
> This is a refactoring of phy address handling in Netsec driver.
> NETSEC SDK, low level driver for NetsecDxe, did not store phy address.
> User should specify the phy address as an argument to
> the SDK public functions.
> It prevented NETSEC SDK from internally controlling phy,
> and it also bothers user application with phy address management.
> 
> With that, we encapsulate the phy address into NETSEC SDK.
> 
> Signed-off-by: Masahisa Kojima 
> Signed-off-by: Satoru Okamoto 
> ---
>  .../Drivers/Net/NetsecDxe/NetsecDxe.c | 10 +--
>  .../Drivers/Net/NetsecDxe/NetsecDxe.h |  2 -
>  .../netsec_sdk/include/ogma_api.h |  6 +-
>  .../netsec_sdk/src/ogma_gmac_access.c | 61 +--
>  .../netsec_sdk/src/ogma_internal.h|  2 +
>  .../netsec_sdk/src/ogma_misc.c|  6 ++

Please use --stat=1000 when generating patches, to avoid truncating
filenames in the summary.

Other than that, looks like good cleanup:
Reviewed-by: Leif Lindholm 


>  6 files changed, 28 insertions(+), 59 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c 
> b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> index 160bb08a4632..0b91d4af44a3 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> @@ -59,6 +59,8 @@ Probe (
>// phy-interface
>Param.gmac_config.phy_interface = OGMA_PHY_INTERFACE_RGMII;
>  
> +  Param.phy_addr = LanDriver->Dev->Resources[2].AddrRangeMin;
> +
>// Read and save the Permanent MAC Address
>EepromBase = LanDriver->Dev->Resources[1].AddrRangeMin;
>GetCurrentMacAddress (EepromBase, 
> LanDriver->SnpMode.PermanentAddress.Addr);
> @@ -107,8 +109,6 @@ Probe (
>  return EFI_DEVICE_ERROR;
>}
>  
> -  LanDriver->PhyAddress = LanDriver->Dev->Resources[2].AddrRangeMin;
> -
>ogma_enable_top_irq (LanDriver->Handle,
> OGMA_TOP_IRQ_REG_NRM_RX | OGMA_TOP_IRQ_REG_NRM_TX);
>  
> @@ -280,7 +280,7 @@ SnpInitialize (
>  ReturnUnlock (EFI_DEVICE_ERROR);
>}
>  
> -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, 
> LanDriver->PhyAddress,
> +  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> _link_status);
>if (ogma_err != OGMA_ERR_OK) {
>  DEBUG ((DEBUG_ERROR,
> @@ -438,7 +438,7 @@ NetsecPollPhyStatus (
>LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
>  
>// Update the media status
> -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, 
> LanDriver->PhyAddress,
> +  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> _link_status);
>if (ogma_err != OGMA_ERR_OK) {
>  DEBUG ((DEBUG_ERROR,
> @@ -662,7 +662,7 @@ SnpGetStatus (
>LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
>  
>// Update the media status
> -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, 
> LanDriver->PhyAddress,
> +  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> _link_status);
>if (ogma_err != OGMA_ERR_OK) {
>  DEBUG ((DEBUG_ERROR,
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h 
> b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
> index 870833c8d31c..c95ff215199d 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
> @@ -70,8 +70,6 @@ typedef struct {
>NON_DISCOVERABLE_DEVICE   *Dev;
>  
>NETSEC_DEVICE_PATHDevicePath;
> -
> -  UINTN PhyAddress;
>  } NETSEC_DRIVER;
>  
>  #define NETSEC_SIGNATURESIGNATURE_32('n', 't', 's', 'c')
> diff --git 
> a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
>  
> b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
> index 66f39150430b..be80dd9ae1fd 100644
> --- 
> a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
> +++ 
> b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
> @@ -318,6 +318,7 @@ struct ogma_param_s{
>  ogma_desc_ring_param_t desc_ring_param[OGMA_DESC_RING_ID_MAX+1];
>  ogma_gmac_config_t gmac_config;
>  ogma_uint8 mac_addr[6];
> +ogma_uint8 phy_addr;
>  };
>  
>  struct ogma_tx_pkt_ctrl_s{
> @@ -412,14 +413,12 @@ ogma_err_t ogma_set_gmac_mode (
>  
>  void ogma_set_phy_reg (
>  ogma_handle_t ogma_handle,
> -ogma_uint8 phy_addr,
>  ogma_uint8 reg_addr,
>  ogma_uint16 value
>  );
>  
>  ogma_uint16 ogma_get_phy_reg (
>  ogma_handle_t ogma_handle,
> -ogma_uint8 phy_addr,
>  ogma_uint8 reg_addr
>  );
>  
> @@ -660,7 +659,6 @@ ogma_err_t ogma_get_gmac_lpitimer_reg (
>  
>  void ogma_set_phy_mmd_reg (
>  

[edk2-devel] [edk2-platforms PATCH v1 1/3] NetsecDxe: embed phy address into NETSEC SDK internal structure

2019-07-22 Thread Masahisa Kojima
This is a refactoring of phy address handling in Netsec driver.
NETSEC SDK, low level driver for NetsecDxe, did not store phy address.
User should specify the phy address as an argument to
the SDK public functions.
It prevented NETSEC SDK from internally controlling phy,
and it also bothers user application with phy address management.

With that, we encapsulate the phy address into NETSEC SDK.

Signed-off-by: Masahisa Kojima 
Signed-off-by: Satoru Okamoto 
---
 .../Drivers/Net/NetsecDxe/NetsecDxe.c | 10 +--
 .../Drivers/Net/NetsecDxe/NetsecDxe.h |  2 -
 .../netsec_sdk/include/ogma_api.h |  6 +-
 .../netsec_sdk/src/ogma_gmac_access.c | 61 +--
 .../netsec_sdk/src/ogma_internal.h|  2 +
 .../netsec_sdk/src/ogma_misc.c|  6 ++
 6 files changed, 28 insertions(+), 59 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c 
b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
index 160bb08a4632..0b91d4af44a3 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
@@ -59,6 +59,8 @@ Probe (
   // phy-interface
   Param.gmac_config.phy_interface = OGMA_PHY_INTERFACE_RGMII;
 
+  Param.phy_addr = LanDriver->Dev->Resources[2].AddrRangeMin;
+
   // Read and save the Permanent MAC Address
   EepromBase = LanDriver->Dev->Resources[1].AddrRangeMin;
   GetCurrentMacAddress (EepromBase, LanDriver->SnpMode.PermanentAddress.Addr);
@@ -107,8 +109,6 @@ Probe (
 return EFI_DEVICE_ERROR;
   }
 
-  LanDriver->PhyAddress = LanDriver->Dev->Resources[2].AddrRangeMin;
-
   ogma_enable_top_irq (LanDriver->Handle,
OGMA_TOP_IRQ_REG_NRM_RX | OGMA_TOP_IRQ_REG_NRM_TX);
 
@@ -280,7 +280,7 @@ SnpInitialize (
 ReturnUnlock (EFI_DEVICE_ERROR);
   }
 
-  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, 
LanDriver->PhyAddress,
+  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
_link_status);
   if (ogma_err != OGMA_ERR_OK) {
 DEBUG ((DEBUG_ERROR,
@@ -438,7 +438,7 @@ NetsecPollPhyStatus (
   LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
 
   // Update the media status
-  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, 
LanDriver->PhyAddress,
+  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
_link_status);
   if (ogma_err != OGMA_ERR_OK) {
 DEBUG ((DEBUG_ERROR,
@@ -662,7 +662,7 @@ SnpGetStatus (
   LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
 
   // Update the media status
-  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, 
LanDriver->PhyAddress,
+  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
_link_status);
   if (ogma_err != OGMA_ERR_OK) {
 DEBUG ((DEBUG_ERROR,
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h 
b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
index 870833c8d31c..c95ff215199d 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
@@ -70,8 +70,6 @@ typedef struct {
   NON_DISCOVERABLE_DEVICE   *Dev;
 
   NETSEC_DEVICE_PATHDevicePath;
-
-  UINTN PhyAddress;
 } NETSEC_DRIVER;
 
 #define NETSEC_SIGNATURESIGNATURE_32('n', 't', 's', 'c')
diff --git 
a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
 
b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
index 66f39150430b..be80dd9ae1fd 100644
--- 
a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
+++ 
b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
@@ -318,6 +318,7 @@ struct ogma_param_s{
 ogma_desc_ring_param_t desc_ring_param[OGMA_DESC_RING_ID_MAX+1];
 ogma_gmac_config_t gmac_config;
 ogma_uint8 mac_addr[6];
+ogma_uint8 phy_addr;
 };
 
 struct ogma_tx_pkt_ctrl_s{
@@ -412,14 +413,12 @@ ogma_err_t ogma_set_gmac_mode (
 
 void ogma_set_phy_reg (
 ogma_handle_t ogma_handle,
-ogma_uint8 phy_addr,
 ogma_uint8 reg_addr,
 ogma_uint16 value
 );
 
 ogma_uint16 ogma_get_phy_reg (
 ogma_handle_t ogma_handle,
-ogma_uint8 phy_addr,
 ogma_uint8 reg_addr
 );
 
@@ -660,7 +659,6 @@ ogma_err_t ogma_get_gmac_lpitimer_reg (
 
 void ogma_set_phy_mmd_reg (
 ogma_handle_t ogma_handle,
-ogma_uint8 phy_addr,
 ogma_uint8 dev_addr,
 ogma_uint16 reg_addr,
 ogma_uint16 value
@@ -668,14 +666,12 @@ void ogma_set_phy_mmd_reg (
 
 ogma_uint16 ogma_get_phy_mmd_reg (
 ogma_handle_t ogma_handle,
-ogma_uint8 phy_addr,
 ogma_uint8 dev_addr,
 ogma_uint16 reg_addr
 );
 
 ogma_err_t ogma_get_phy_link_status (
 ogma_handle_t ogma_handle,
-ogma_uint8 phy_addr,
 ogma_phy_link_status_t *phy_link_status_p
 );