* [platforms: PATCH v2 1/4] Marvell/Armada: Switch to dynamic tftp command
2017-12-01 15:35 [platforms: PATCH v2 0/4] Armada 7k/8k - misc improvements pt.3 Marcin Wojtas
@ 2017-12-01 15:35 ` Marcin Wojtas
2017-12-04 13:32 ` Leif Lindholm
2017-12-01 15:35 ` [platforms: PATCH v2 2/4] Marvell/Armada: Fix watchdog control base Marcin Wojtas
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Marcin Wojtas @ 2017-12-01 15:35 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
jsd
After removal of the tftp library in EDK2, the tftp was
disabled on Armada platform. Re-enable this functionality
as a dynamic command on Armada 70x0 DB board. For this
purpose add it as a build-time selectable option, depending
on a new INCLUDE_TFTP_COMMAND parameter.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
Platform/Marvell/Armada/Armada.dsc.inc | 4 ++++
Platform/Marvell/Armada/Armada70x0.dsc | 1 +
Platform/Marvell/Armada/Armada70x0.fdf | 3 +++
3 files changed, 8 insertions(+)
diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
index 2a8a226..6040493 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -525,6 +525,10 @@
gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
}
+!ifdef $(INCLUDE_TFTP_COMMAND)
+ ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
+!endif #$(INCLUDE_TFTP_COMMAND)
+
[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
index 8e4cdb2..4e7d43c 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -44,6 +44,7 @@
BUILD_TARGETS = DEBUG|RELEASE
SKUID_IDENTIFIER = DEFAULT
FLASH_DEFINITION = Platform/Marvell/Armada/Armada70x0.fdf
+ DEFINE INCLUDE_TFTP_COMMAND = 1
!include Armada.dsc.inc
diff --git a/Platform/Marvell/Armada/Armada70x0.fdf b/Platform/Marvell/Armada/Armada70x0.fdf
index ca92c60..c03bc42 100644
--- a/Platform/Marvell/Armada/Armada70x0.fdf
+++ b/Platform/Marvell/Armada/Armada70x0.fdf
@@ -176,6 +176,9 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
# UEFI application (Shell Embedded Boot Loader)
INF ShellPkg/Application/Shell/Shell.inf
+!ifdef $(INCLUDE_TFTP_COMMAND)
+ INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
+!endif #$(INCLUDE_TFTP_COMMAND)
# Bds
INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [platforms: PATCH v2 1/4] Marvell/Armada: Switch to dynamic tftp command
2017-12-01 15:35 ` [platforms: PATCH v2 1/4] Marvell/Armada: Switch to dynamic tftp command Marcin Wojtas
@ 2017-12-04 13:32 ` Leif Lindholm
2017-12-04 15:04 ` Marcin Wojtas
0 siblings, 1 reply; 13+ messages in thread
From: Leif Lindholm @ 2017-12-04 13:32 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel, ard.biesheuvel, nadavh, neta, kostap, jinghua, jsd
On Fri, Dec 01, 2017 at 04:35:04PM +0100, Marcin Wojtas wrote:
> After removal of the tftp library in EDK2, the tftp was
> disabled on Armada platform. Re-enable this functionality
> as a dynamic command on Armada 70x0 DB board. For this
> purpose add it as a build-time selectable option, depending
> on a new INCLUDE_TFTP_COMMAND parameter.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
> Platform/Marvell/Armada/Armada.dsc.inc | 4 ++++
> Platform/Marvell/Armada/Armada70x0.dsc | 1 +
> Platform/Marvell/Armada/Armada70x0.fdf | 3 +++
> 3 files changed, 8 insertions(+)
>
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
> index 2a8a226..6040493 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -525,6 +525,10 @@
> gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
> }
>
> +!ifdef $(INCLUDE_TFTP_COMMAND)
> + ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> +!endif #$(INCLUDE_TFTP_COMMAND)
> +
> [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
> GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>
> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
> index 8e4cdb2..4e7d43c 100644
> --- a/Platform/Marvell/Armada/Armada70x0.dsc
> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
> @@ -44,6 +44,7 @@
> BUILD_TARGETS = DEBUG|RELEASE
> SKUID_IDENTIFIER = DEFAULT
> FLASH_DEFINITION = Platform/Marvell/Armada/Armada70x0.fdf
> + DEFINE INCLUDE_TFTP_COMMAND = 1
This looks very much to me like it will always include the tftp
command, which is not what I wanted.
Then again, you have done _exactly_ what I requested in emulating
what the Hisilicon platforms do (and will shortly stop doing).
My thoughts were that, since this is a debug/development feature, it
would be enabled at build time by adding -D INCLUDE_TFTP_COMMAND to
the build command line.
As such, do you have any issues with me deleting this hunk before
pushing? If not:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> !include Armada.dsc.inc
>
> diff --git a/Platform/Marvell/Armada/Armada70x0.fdf b/Platform/Marvell/Armada/Armada70x0.fdf
> index ca92c60..c03bc42 100644
> --- a/Platform/Marvell/Armada/Armada70x0.fdf
> +++ b/Platform/Marvell/Armada/Armada70x0.fdf
> @@ -176,6 +176,9 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
>
> # UEFI application (Shell Embedded Boot Loader)
> INF ShellPkg/Application/Shell/Shell.inf
> +!ifdef $(INCLUDE_TFTP_COMMAND)
> + INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> +!endif #$(INCLUDE_TFTP_COMMAND)
>
> # Bds
> INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [platforms: PATCH v2 1/4] Marvell/Armada: Switch to dynamic tftp command
2017-12-04 13:32 ` Leif Lindholm
@ 2017-12-04 15:04 ` Marcin Wojtas
0 siblings, 0 replies; 13+ messages in thread
From: Marcin Wojtas @ 2017-12-04 15:04 UTC (permalink / raw)
To: Leif Lindholm
Cc: edk2-devel-01, Ard Biesheuvel, nadavh, Neta Zur Hershkovits,
Kostya Porotchkin, Hua Jing, semihalf-dabros-jan
Hi Leif,
2017-12-04 14:32 GMT+01:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Fri, Dec 01, 2017 at 04:35:04PM +0100, Marcin Wojtas wrote:
>> After removal of the tftp library in EDK2, the tftp was
>> disabled on Armada platform. Re-enable this functionality
>> as a dynamic command on Armada 70x0 DB board. For this
>> purpose add it as a build-time selectable option, depending
>> on a new INCLUDE_TFTP_COMMAND parameter.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>> Platform/Marvell/Armada/Armada.dsc.inc | 4 ++++
>> Platform/Marvell/Armada/Armada70x0.dsc | 1 +
>> Platform/Marvell/Armada/Armada70x0.fdf | 3 +++
>> 3 files changed, 8 insertions(+)
>>
>> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
>> index 2a8a226..6040493 100644
>> --- a/Platform/Marvell/Armada/Armada.dsc.inc
>> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
>> @@ -525,6 +525,10 @@
>> gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
>> }
>>
>> +!ifdef $(INCLUDE_TFTP_COMMAND)
>> + ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>> +!endif #$(INCLUDE_TFTP_COMMAND)
>> +
>> [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
>> GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>>
>> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
>> index 8e4cdb2..4e7d43c 100644
>> --- a/Platform/Marvell/Armada/Armada70x0.dsc
>> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
>> @@ -44,6 +44,7 @@
>> BUILD_TARGETS = DEBUG|RELEASE
>> SKUID_IDENTIFIER = DEFAULT
>> FLASH_DEFINITION = Platform/Marvell/Armada/Armada70x0.fdf
>> + DEFINE INCLUDE_TFTP_COMMAND = 1
>
> This looks very much to me like it will always include the tftp
> command, which is not what I wanted.
>
> Then again, you have done _exactly_ what I requested in emulating
> what the Hisilicon platforms do (and will shortly stop doing).
I took a look at Platform/Hisilicon/D02/Pv660D02.dsc and took the
DEFINE method from there.
>
> My thoughts were that, since this is a debug/development feature, it
> would be enabled at build time by adding -D INCLUDE_TFTP_COMMAND to
> the build command line.
>
It wasn't clear to me in such way :)
> As such, do you have any issues with me deleting this hunk before
> pushing? If not:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
Sure, please remove this line.
Best regards,
Marcin
>>
>> !include Armada.dsc.inc
>>
>> diff --git a/Platform/Marvell/Armada/Armada70x0.fdf b/Platform/Marvell/Armada/Armada70x0.fdf
>> index ca92c60..c03bc42 100644
>> --- a/Platform/Marvell/Armada/Armada70x0.fdf
>> +++ b/Platform/Marvell/Armada/Armada70x0.fdf
>> @@ -176,6 +176,9 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
>>
>> # UEFI application (Shell Embedded Boot Loader)
>> INF ShellPkg/Application/Shell/Shell.inf
>> +!ifdef $(INCLUDE_TFTP_COMMAND)
>> + INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>> +!endif #$(INCLUDE_TFTP_COMMAND)
>>
>> # Bds
>> INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [platforms: PATCH v2 2/4] Marvell/Armada: Fix watchdog control base
2017-12-01 15:35 [platforms: PATCH v2 0/4] Armada 7k/8k - misc improvements pt.3 Marcin Wojtas
2017-12-01 15:35 ` [platforms: PATCH v2 1/4] Marvell/Armada: Switch to dynamic tftp command Marcin Wojtas
@ 2017-12-01 15:35 ` Marcin Wojtas
2017-12-01 15:35 ` [platforms: PATCH v2 3/4] Marvell/Applications: FirmwareUpdate: Fix usage information Marcin Wojtas
2017-12-01 15:35 ` [platforms: PATCH v2 4/4] Marvell/Drivers: MvPhyDxe: Cleanup the header Marcin Wojtas
3 siblings, 0 replies; 13+ messages in thread
From: Marcin Wojtas @ 2017-12-01 15:35 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
jsd
Proper watchdog control base is 0xf0610000, so fix the
incorrect value.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
Platform/Marvell/Armada/Armada.dsc.inc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
index 6040493..4b29691 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -269,7 +269,7 @@
gEmbeddedTokenSpaceGuid.PcdMetronomeTickPeriod|1000
# ARM SBSA Watchdog
- gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0xF0620000
+ gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0xF0610000
gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0xF0600000
gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum|34
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [platforms: PATCH v2 3/4] Marvell/Applications: FirmwareUpdate: Fix usage information
2017-12-01 15:35 [platforms: PATCH v2 0/4] Armada 7k/8k - misc improvements pt.3 Marcin Wojtas
2017-12-01 15:35 ` [platforms: PATCH v2 1/4] Marvell/Armada: Switch to dynamic tftp command Marcin Wojtas
2017-12-01 15:35 ` [platforms: PATCH v2 2/4] Marvell/Armada: Fix watchdog control base Marcin Wojtas
@ 2017-12-01 15:35 ` Marcin Wojtas
2017-12-04 13:52 ` Leif Lindholm
2017-12-01 15:35 ` [platforms: PATCH v2 4/4] Marvell/Drivers: MvPhyDxe: Cleanup the header Marcin Wojtas
3 siblings, 1 reply; 13+ messages in thread
From: Marcin Wojtas @ 2017-12-01 15:35 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
jsd
fupdate command's usage information referred to a deprecated
'-f' flag in 'examples' section. It was a residue from the
initial version of the application, removed during review
before merging to upstream branch. Correct the help information.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
Platform/Marvell/Applications/FirmwareUpdate/FUpdate.uni | Bin 5146 -> 5190 bytes
1 file changed, 0 insertions(+), 0 deletions(-)
diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.uni b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.uni
index 9d52e590c6d239621d226a240a3f9755210f52fe..146f624737d96e2cd354857dbb63390ebe2f347e 100644
GIT binary patch
delta 56
zcmbQGaZF=_h>&<DLmq<yLoh=CgC~$qW5{7hWGH6Hm@FuixVb_of^o76yTD`vb`~(c
HAnXMIdh-p2
delta 28
kcmX@6F-v2Eh|pv`p*Ut;hP27{LYA8k2`Mp7{v+%K0D@Hs0RR91
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [platforms: PATCH v2 3/4] Marvell/Applications: FirmwareUpdate: Fix usage information
2017-12-01 15:35 ` [platforms: PATCH v2 3/4] Marvell/Applications: FirmwareUpdate: Fix usage information Marcin Wojtas
@ 2017-12-04 13:52 ` Leif Lindholm
2017-12-04 15:15 ` Marcin Wojtas
0 siblings, 1 reply; 13+ messages in thread
From: Leif Lindholm @ 2017-12-04 13:52 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel, ard.biesheuvel, nadavh, neta, kostap, jinghua, jsd
On Fri, Dec 01, 2017 at 04:35:06PM +0100, Marcin Wojtas wrote:
> fupdate command's usage information referred to a deprecated
> '-f' flag in 'examples' section. It was a residue from the
> initial version of the application, removed during review
> before merging to upstream branch. Correct the help information.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
I boggled slightly at how the file with deleted content is larger than
the original, so went to have a look, and what I see is:
--- pre.txt 2017-12-04 13:43:32.985446108 +0000
+++ post.txt 2017-12-04 13:44:30.433120594 +0000
@@ -50,10 +50,10 @@
".SH EXAMPLES\r\n"
" \r\n"
"EXAMPLES:\r\n"
-"Update firmware from file fs2:flash-image.bin\r\n"
-" fupdate -f fs2:flash-image.bin\r\n"
+"Update firmware in SPI flash from file fs2:flash-image.bin\r\n"
+" fupdate fs2:flash-image.bin\r\n"
".SH RETURNVALUES\r\n"
" \r\n"
"RETURN VALUES:\r\n"
" SHELL_SUCCESS The action was completed as requested.\r\n"
-" SHELL_ABORTED Error while processing command\r\n"
+" SHELL_ABORTED Error while processing command\r\n"
Now, I don't mind random format cleanup as much in .uni files as
elsewhere, since we don't get sane diffs anyway. But that also makes
detailed and accurate commit messages much more important.
So please, add something about the rewording and the whitespace fixes.
/
Leif
> ---
> Platform/Marvell/Applications/FirmwareUpdate/FUpdate.uni | Bin 5146 -> 5190 bytes
> 1 file changed, 0 insertions(+), 0 deletions(-)
>
> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.uni b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.uni
> index 9d52e590c6d239621d226a240a3f9755210f52fe..146f624737d96e2cd354857dbb63390ebe2f347e 100644
> GIT binary patch
> delta 56
> zcmbQGaZF=_h>&<DLmq<yLoh=CgC~$qW5{7hWGH6Hm@FuixVb_of^o76yTD`vb`~(c
> HAnXMIdh-p2
>
> delta 28
> kcmX@6F-v2Eh|pv`p*Ut;hP27{LYA8k2`Mp7{v+%K0D@Hs0RR91
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [platforms: PATCH v2 3/4] Marvell/Applications: FirmwareUpdate: Fix usage information
2017-12-04 13:52 ` Leif Lindholm
@ 2017-12-04 15:15 ` Marcin Wojtas
0 siblings, 0 replies; 13+ messages in thread
From: Marcin Wojtas @ 2017-12-04 15:15 UTC (permalink / raw)
To: Leif Lindholm
Cc: edk2-devel-01, Ard Biesheuvel, nadavh, Neta Zur Hershkovits,
Kostya Porotchkin, Hua Jing, semihalf-dabros-jan
2017-12-04 14:52 GMT+01:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Fri, Dec 01, 2017 at 04:35:06PM +0100, Marcin Wojtas wrote:
>> fupdate command's usage information referred to a deprecated
>> '-f' flag in 'examples' section. It was a residue from the
>> initial version of the application, removed during review
>> before merging to upstream branch. Correct the help information.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>
> I boggled slightly at how the file with deleted content is larger than
> the original, so went to have a look, and what I see is:
>
> --- pre.txt 2017-12-04 13:43:32.985446108 +0000
> +++ post.txt 2017-12-04 13:44:30.433120594 +0000
> @@ -50,10 +50,10 @@
> ".SH EXAMPLES\r\n"
> " \r\n"
> "EXAMPLES:\r\n"
> -"Update firmware from file fs2:flash-image.bin\r\n"
> -" fupdate -f fs2:flash-image.bin\r\n"
> +"Update firmware in SPI flash from file fs2:flash-image.bin\r\n"
> +" fupdate fs2:flash-image.bin\r\n"
> ".SH RETURNVALUES\r\n"
> " \r\n"
> "RETURN VALUES:\r\n"
> " SHELL_SUCCESS The action was completed as requested.\r\n"
> -" SHELL_ABORTED Error while processing command\r\n"
> +" SHELL_ABORTED Error while processing command\r\n"
>
>
> Now, I don't mind random format cleanup as much in .uni files as
> elsewhere, since we don't get sane diffs anyway. But that also makes
> detailed and accurate commit messages much more important.
>
> So please, add something about the rewording and the whitespace fixes.
>
Sure, will do.
Thanks,
Marcin
^ permalink raw reply [flat|nested] 13+ messages in thread
* [platforms: PATCH v2 4/4] Marvell/Drivers: MvPhyDxe: Cleanup the header
2017-12-01 15:35 [platforms: PATCH v2 0/4] Armada 7k/8k - misc improvements pt.3 Marcin Wojtas
` (2 preceding siblings ...)
2017-12-01 15:35 ` [platforms: PATCH v2 3/4] Marvell/Applications: FirmwareUpdate: Fix usage information Marcin Wojtas
@ 2017-12-01 15:35 ` Marcin Wojtas
2017-12-04 14:00 ` Leif Lindholm
3 siblings, 1 reply; 13+ messages in thread
From: Marcin Wojtas @ 2017-12-01 15:35 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
jsd
This patch removes unused macros defined in MvPhyDxe.h, as well
as improves the style and comments. Pick single definition
of the autonegotiation timeout - two different macros were used
for the same purpose.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c | 2 +-
Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h | 152 ++++----------------
2 files changed, 31 insertions(+), 123 deletions(-)
diff --git a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
index e776a91..dd2edae 100644
--- a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
+++ b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
@@ -328,7 +328,7 @@ MvPhyInit1512 (
DEBUG((DEBUG_ERROR, "MvPhyDxe: Waiting for PHY auto negotiation... "));
for (i = 0; !(Data & BMSR_ANEGCOMPLETE); i++) {
- if (i > PHY_ANEG_TIMEOUT) {
+ if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
DEBUG((DEBUG_ERROR, "timeout\n"));
PhyDev->LinkUp = FALSE;
return EFI_TIMEOUT;
diff --git a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h
index 0c3d935..66974bb 100644
--- a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h
+++ b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h
@@ -34,137 +34,45 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#ifndef __MV_PHY_DXE_H__
#define __MV_PHY_DXE_H__
-#define MII_BMCR 0x00 /* Basic mode control Register */
-#define MII_BMSR 0x01 /* Basic mode status Register */
-#define MII_PHYSID1 0x02 /* PHYS ID 1 */
-#define MII_PHYSID2 0x03 /* PHYS ID 2 */
-#define MII_ADVERTISE 0x04 /* Advertisement control Reg */
-#define MII_LPA 0x05 /* Link partner ability Reg */
-#define MII_EXPANSION 0x06 /* Expansion Register */
-#define MII_CTRL1000 0x09 /* 1000BASE-T control */
-#define MII_STAT1000 0x0a /* 1000BASE-T status */
-#define MII_ESTATUS 0x0f /* Extended Status */
-#define MII_DCOUNTER 0x12 /* Disconnect counter */
-#define MII_FCSCOUNTER 0x13 /* False carrier counter */
-#define MII_NWAYTEST 0x14 /* N-way auto-neg test Reg */
-#define MII_RERRCOUNTER 0x15 /* Receive error counter */
-#define MII_SREVISION 0x16 /* Silicon revision */
-#define MII_RESV1 0x17 /* Reserved... */
-#define MII_LBRERROR 0x18 /* Lpback, rx, bypass error */
-#define MII_PHYADDR 0x19 /* PHY address */
-#define MII_RESV2 0x1a /* Reserved... */
-#define MII_TPISTATUS 0x1b /* TPI status for 10mbps */
-#define MII_NCONFIG 0x1c /* Network interface config */
-
-/* Basic mode control Register. */
-#define BMCR_RESV 0x003f /* Unused... */
-#define BMCR_SPEED1000 0x0040 /* MSB of Speed (1000) */
-#define BMCR_CTST 0x0080 /* Collision test */
-#define BMCR_FULLDPLX 0x0100 /* Full duplex */
-#define BMCR_ANRESTART 0x0200 /* Auto negotiation restart */
-#define BMCR_ISOLATE 0x0400 /* Disconnect DP83840 from MII */
-#define BMCR_PDOWN 0x0800 /* Powerdown the DP83840 */
-#define BMCR_ANENABLE 0x1000 /* Enable auto negotiation */
-#define BMCR_SPEED100 0x2000 /* Select 100Mbps */
-#define BMCR_LOOPBACK 0x4000 /* TXD loopback bits */
-#define BMCR_RESET 0x8000 /* Reset the DP83840 */
-
-/* Basic mode status Register. */
-#define BMSR_ERCAP 0x0001 /* Ext-Reg capability */
-#define BMSR_JCD 0x0002 /* Jabber detected */
-#define BMSR_LSTATUS 0x0004 /* Link status */
-#define BMSR_ANEGCAPABLE 0x0008 /* Able to do auto-negotiation */
-#define BMSR_RFAULT 0x0010 /* Remote fault detected */
-#define BMSR_ANEGCOMPLETE 0x0020 /* Auto-negotiation complete */
-#define BMSR_RESV 0x00c0 /* Unused... */
-#define BMSR_ESTATEN 0x0100 /* Extended Status in R15 */
-#define BMSR_100HALF2 0x0200 /* Can do 100BASE-T2 HDX */
-#define BMSR_100FULL2 0x0400 /* Can do 100BASE-T2 FDX */
-#define BMSR_10HALF 0x0800 /* Can do 10mbps, half-duplex */
-#define BMSR_10FULL 0x1000 /* Can do 10mbps, full-duplex */
-#define BMSR_100HALF 0x2000 /* Can do 100mbps, half-duplex */
-#define BMSR_100FULL 0x4000 /* Can do 100mbps, full-duplex */
-#define BMSR_100BASE4 0x8000 /* Can do 100mbps, 4k packets */
-
-#define PHY_ANEG_TIMEOUT 4000
-
-#define PHY_INTERFACE_MODE_RGMII 0
-#define PHY_INTERFACE_MODE_RGMII_ID 1
-#define PHY_INTERFACE_MODE_RGMII_RXID 2
-#define PHY_INTERFACE_MODE_RGMII_TXID 3
-#define PHY_INTERFACE_MODE_SGMII 4
-#define PHY_INTERFACE_MODE_RTBI 5
-
-#define PHY_AUTONEGOTIATE_TIMEOUT 5000
+#define MII_BMCR 0x00 /* Basic mode control Register */
+#define MII_BMSR 0x01 /* Basic mode status Register */
+
+/* BMCR */
+#define BMCR_ANRESTART 0x0200 /* 1 = Restart autonegotiation */
+#define BMCR_ISOLATE 0x0400 /* 0 = Isolate PHY */
+#define BMCR_ANENABLE 0x1000 /* 1 = Enable autonegotiation */
+#define BMCR_RESET 0x8000 /* 1 = Reset the PHY */
+
+/* BSMR */
+#define BMSR_LSTATUS 0x0004 /* 1 = Link up */
+#define BMSR_ANEGCAPABLE 0x0008 /* 1 = Able to perform auto-neg */
+#define BMSR_ANEGCOMPLETE 0x0020 /* 1 = Auto-neg complete */
+
+#define PHY_AUTONEGOTIATE_TIMEOUT 5000
/* 88E1011 PHY Status Register */
-#define MIIM_88E1xxx_PHY_STATUS 0x11
-#define MIIM_88E1xxx_PHYSTAT_SPEED 0xc000
-#define MIIM_88E1xxx_PHYSTAT_GBIT 0x8000
-#define MIIM_88E1xxx_PHYSTAT_100 0x4000
-#define MIIM_88E1xxx_PHYSTAT_DUPLEX 0x2000
-#define MIIM_88E1xxx_PHYSTAT_SPDDONE 0x0800
-#define MIIM_88E1xxx_PHYSTAT_LINK 0x0400
-
-#define MIIM_88E1xxx_PHY_SCR 0x10
-#define MIIM_88E1xxx_PHY_MDI_X_AUTO 0x0060
-
-/* 88E1111 PHY LED Control Register */
-#define MIIM_88E1111_PHY_LED_CONTROL 24
-#define MIIM_88E1111_PHY_LED_DIRECT 0x4100
-#define MIIM_88E1111_PHY_LED_COMBINE 0x411C
+#define MIIM_88E1xxx_PHY_STATUS 0x11
+#define MIIM_88E1xxx_PHYSTAT_SPEED 0xc000
+#define MIIM_88E1xxx_PHYSTAT_GBIT 0x8000
+#define MIIM_88E1xxx_PHYSTAT_100 0x4000
+#define MIIM_88E1xxx_PHYSTAT_DUPLEX 0x2000
+#define MIIM_88E1xxx_PHYSTAT_SPDDONE 0x0800
+#define MIIM_88E1xxx_PHYSTAT_LINK 0x0400
/* 88E1111 Extended PHY Specific Control Register */
-#define MIIM_88E1111_PHY_EXT_CR 0x14
-#define MIIM_88E1111_RX_DELAY 0x80
-#define MIIM_88E1111_TX_DELAY 0x2
+#define MIIM_88E1111_PHY_EXT_CR 0x14
+#define MIIM_88E1111_RX_DELAY 0x80
+#define MIIM_88E1111_TX_DELAY 0x02
/* 88E1111 Extended PHY Specific Status Register */
-#define MIIM_88E1111_PHY_EXT_SR 0x1b
-#define MIIM_88E1111_HWCFG_MODE_MASK 0xf
+#define MIIM_88E1111_PHY_EXT_SR 0x1b
+#define MIIM_88E1111_HWCFG_MODE_MASK 0xf
#define MIIM_88E1111_HWCFG_MODE_COPPER_RGMII 0xb
-#define MIIM_88E1111_HWCFG_MODE_FIBER_RGMII 0x3
+#define MIIM_88E1111_HWCFG_MODE_FIBER_RGMII 0x3
#define MIIM_88E1111_HWCFG_MODE_SGMII_NO_CLK 0x4
-#define MIIM_88E1111_HWCFG_MODE_COPPER_RTBI 0x9
+#define MIIM_88E1111_HWCFG_MODE_COPPER_RTBI 0x9
#define MIIM_88E1111_HWCFG_FIBER_COPPER_AUTO 0x8000
-#define MIIM_88E1111_HWCFG_FIBER_COPPER_RES 0x2000
-
-#define MIIM_88E1111_COPPER 0
-#define MIIM_88E1111_FIBER 1
-
-/* 88E1118 PHY defines */
-#define MIIM_88E1118_PHY_PAGE 22
-#define MIIM_88E1118_PHY_LED_PAGE 3
-
-/* 88E1121 PHY LED Control Register */
-#define MIIM_88E1121_PHY_LED_CTRL 16
-#define MIIM_88E1121_PHY_LED_PAGE 3
-#define MIIM_88E1121_PHY_LED_DEF 0x0030
-
-/* 88E1121 PHY IRQ Enable/Status Register */
-#define MIIM_88E1121_PHY_IRQ_EN 18
-#define MIIM_88E1121_PHY_IRQ_STATUS 19
-
-#define MIIM_88E1121_PHY_PAGE 22
-
-/* 88E1145 Extended PHY Specific Control Register */
-#define MIIM_88E1145_PHY_EXT_CR 20
-#define MIIM_M88E1145_RGMII_RX_DELAY 0x0080
-#define MIIM_M88E1145_RGMII_TX_DELAY 0x0002
-
-#define MIIM_88E1145_PHY_LED_CONTROL 24
-#define MIIM_88E1145_PHY_LED_DIRECT 0x4100
-
-#define MIIM_88E1145_PHY_PAGE 29
-#define MIIM_88E1145_PHY_CAL_OV 30
-
-#define MIIM_88E1149_PHY_PAGE 29
-
-/* 88E1310 PHY defines */
-#define MIIM_88E1310_PHY_LED_CTRL 16
-#define MIIM_88E1310_PHY_IRQ_EN 18
-#define MIIM_88E1310_PHY_RGMII_CTRL 21
-#define MIIM_88E1310_PHY_PAGE 22
+#define MIIM_88E1111_HWCFG_FIBER_COPPER_RES 0x2000
typedef enum {
MV_PHY_DEVICE_1512
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [platforms: PATCH v2 4/4] Marvell/Drivers: MvPhyDxe: Cleanup the header
2017-12-01 15:35 ` [platforms: PATCH v2 4/4] Marvell/Drivers: MvPhyDxe: Cleanup the header Marcin Wojtas
@ 2017-12-04 14:00 ` Leif Lindholm
2017-12-04 15:22 ` Marcin Wojtas
0 siblings, 1 reply; 13+ messages in thread
From: Leif Lindholm @ 2017-12-04 14:00 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel, ard.biesheuvel, nadavh, neta, kostap, jinghua, jsd
On Fri, Dec 01, 2017 at 04:35:07PM +0100, Marcin Wojtas wrote:
> This patch removes unused macros defined in MvPhyDxe.h, as well
> as improves the style and comments. Pick single definition
> of the autonegotiation timeout - two different macros were used
> for the same purpose.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
> Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c | 2 +-
> Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h | 152 ++++----------------
> 2 files changed, 31 insertions(+), 123 deletions(-)
>
> diff --git a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
> index e776a91..dd2edae 100644
> --- a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
> +++ b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
> @@ -328,7 +328,7 @@ MvPhyInit1512 (
>
> DEBUG((DEBUG_ERROR, "MvPhyDxe: Waiting for PHY auto negotiation... "));
> for (i = 0; !(Data & BMSR_ANEGCOMPLETE); i++) {
> - if (i > PHY_ANEG_TIMEOUT) {
> + if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
I think you've had a git accident somewhere:
The existing line upstream is still
if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
This hunk should have just dropped out.
/
Leif
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [platforms: PATCH v2 4/4] Marvell/Drivers: MvPhyDxe: Cleanup the header
2017-12-04 14:00 ` Leif Lindholm
@ 2017-12-04 15:22 ` Marcin Wojtas
2017-12-04 16:10 ` Leif Lindholm
0 siblings, 1 reply; 13+ messages in thread
From: Marcin Wojtas @ 2017-12-04 15:22 UTC (permalink / raw)
To: Leif Lindholm
Cc: edk2-devel-01, Ard Biesheuvel, nadavh, Neta Zur Hershkovits,
Kostya Porotchkin, Hua Jing, semihalf-dabros-jan
Hi Leif,
2017-12-04 15:00 GMT+01:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Fri, Dec 01, 2017 at 04:35:07PM +0100, Marcin Wojtas wrote:
>> This patch removes unused macros defined in MvPhyDxe.h, as well
>> as improves the style and comments. Pick single definition
>> of the autonegotiation timeout - two different macros were used
>> for the same purpose.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>> Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c | 2 +-
>> Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h | 152 ++++----------------
>> 2 files changed, 31 insertions(+), 123 deletions(-)
>>
>> diff --git a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
>> index e776a91..dd2edae 100644
>> --- a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
>> +++ b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
>> @@ -328,7 +328,7 @@ MvPhyInit1512 (
>>
>> DEBUG((DEBUG_ERROR, "MvPhyDxe: Waiting for PHY auto negotiation... "));
>> for (i = 0; !(Data & BMSR_ANEGCOMPLETE); i++) {
>> - if (i > PHY_ANEG_TIMEOUT) {
>> + if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
>
> I think you've had a git accident somewhere:
> The existing line upstream is still
> if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
> This hunk should have just dropped out.
>
No, it's ok. It just turned out, there were two concurrent definitions
of the same thing. Please take a look at what's available on top of
the master branch:
Header:
[...]
#define PHY_ANEG_TIMEOUT 4000
#define PHY_INTERFACE_MODE_RGMII 0
#define PHY_INTERFACE_MODE_RGMII_ID 1
#define PHY_INTERFACE_MODE_RGMII_RXID 2
#define PHY_INTERFACE_MODE_RGMII_TXID 3
#define PHY_INTERFACE_MODE_SGMII 4
#define PHY_INTERFACE_MODE_RTBI 5
#define PHY_AUTONEGOTIATE_TIMEOUT 5000
[...]
Driver:
~Line 204:
DEBUG((DEBUG_ERROR,"MvPhyDxe: Waiting for PHY realtime link"));
while (!(Data & MIIM_88E1xxx_PHYSTAT_SPDDONE)) {
if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
~Line 327:
DEBUG((DEBUG_ERROR, "MvPhyDxe: Waiting for PHY auto negotiation... "));
for (i = 0; !(Data & BMSR_ANEGCOMPLETE); i++) {
if (i > PHY_ANEG_TIMEOUT) {
DEBUG((DEBUG_ERROR, "timeout\n"));
Best regards,
Marcin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [platforms: PATCH v2 4/4] Marvell/Drivers: MvPhyDxe: Cleanup the header
2017-12-04 15:22 ` Marcin Wojtas
@ 2017-12-04 16:10 ` Leif Lindholm
2017-12-04 16:35 ` Marcin Wojtas
0 siblings, 1 reply; 13+ messages in thread
From: Leif Lindholm @ 2017-12-04 16:10 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel-01, Ard Biesheuvel, nadavh, Neta Zur Hershkovits,
Kostya Porotchkin, Hua Jing, semihalf-dabros-jan
On Mon, Dec 04, 2017 at 04:22:33PM +0100, Marcin Wojtas wrote:
> Hi Leif,
>
>
>
> 2017-12-04 15:00 GMT+01:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Fri, Dec 01, 2017 at 04:35:07PM +0100, Marcin Wojtas wrote:
> >> This patch removes unused macros defined in MvPhyDxe.h, as well
> >> as improves the style and comments. Pick single definition
> >> of the autonegotiation timeout - two different macros were used
> >> for the same purpose.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> ---
> >> Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c | 2 +-
> >> Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h | 152 ++++----------------
> >> 2 files changed, 31 insertions(+), 123 deletions(-)
> >>
> >> diff --git a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
> >> index e776a91..dd2edae 100644
> >> --- a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
> >> +++ b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
> >> @@ -328,7 +328,7 @@ MvPhyInit1512 (
> >>
> >> DEBUG((DEBUG_ERROR, "MvPhyDxe: Waiting for PHY auto negotiation... "));
> >> for (i = 0; !(Data & BMSR_ANEGCOMPLETE); i++) {
> >> - if (i > PHY_ANEG_TIMEOUT) {
> >> + if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
> >
> > I think you've had a git accident somewhere:
> > The existing line upstream is still
> > if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
> > This hunk should have just dropped out.
> >
>
> No, it's ok. It just turned out, there were two concurrent definitions
> of the same thing. Please take a look at what's available on top of
> the master branch:
Aah, that's not the same location as in your v1, is it? :)
*grumble*
Sure:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> Header:
> [...]
> #define PHY_ANEG_TIMEOUT 4000
>
> #define PHY_INTERFACE_MODE_RGMII 0
> #define PHY_INTERFACE_MODE_RGMII_ID 1
> #define PHY_INTERFACE_MODE_RGMII_RXID 2
> #define PHY_INTERFACE_MODE_RGMII_TXID 3
> #define PHY_INTERFACE_MODE_SGMII 4
> #define PHY_INTERFACE_MODE_RTBI 5
>
> #define PHY_AUTONEGOTIATE_TIMEOUT 5000
> [...]
>
> Driver:
> ~Line 204:
> DEBUG((DEBUG_ERROR,"MvPhyDxe: Waiting for PHY realtime link"));
> while (!(Data & MIIM_88E1xxx_PHYSTAT_SPDDONE)) {
> if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
>
> ~Line 327:
> DEBUG((DEBUG_ERROR, "MvPhyDxe: Waiting for PHY auto negotiation... "));
> for (i = 0; !(Data & BMSR_ANEGCOMPLETE); i++) {
> if (i > PHY_ANEG_TIMEOUT) {
> DEBUG((DEBUG_ERROR, "timeout\n"));
>
> Best regards,
> Marcin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [platforms: PATCH v2 4/4] Marvell/Drivers: MvPhyDxe: Cleanup the header
2017-12-04 16:10 ` Leif Lindholm
@ 2017-12-04 16:35 ` Marcin Wojtas
0 siblings, 0 replies; 13+ messages in thread
From: Marcin Wojtas @ 2017-12-04 16:35 UTC (permalink / raw)
To: Leif Lindholm
Cc: edk2-devel-01, Ard Biesheuvel, nadavh, Neta Zur Hershkovits,
Kostya Porotchkin, Hua Jing, semihalf-dabros-jan
2017-12-04 17:10 GMT+01:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Mon, Dec 04, 2017 at 04:22:33PM +0100, Marcin Wojtas wrote:
>> Hi Leif,
>>
>>
>>
>> 2017-12-04 15:00 GMT+01:00 Leif Lindholm <leif.lindholm@linaro.org>:
>> > On Fri, Dec 01, 2017 at 04:35:07PM +0100, Marcin Wojtas wrote:
>> >> This patch removes unused macros defined in MvPhyDxe.h, as well
>> >> as improves the style and comments. Pick single definition
>> >> of the autonegotiation timeout - two different macros were used
>> >> for the same purpose.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> >> ---
>> >> Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c | 2 +-
>> >> Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h | 152 ++++----------------
>> >> 2 files changed, 31 insertions(+), 123 deletions(-)
>> >>
>> >> diff --git a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
>> >> index e776a91..dd2edae 100644
>> >> --- a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
>> >> +++ b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
>> >> @@ -328,7 +328,7 @@ MvPhyInit1512 (
>> >>
>> >> DEBUG((DEBUG_ERROR, "MvPhyDxe: Waiting for PHY auto negotiation... "));
>> >> for (i = 0; !(Data & BMSR_ANEGCOMPLETE); i++) {
>> >> - if (i > PHY_ANEG_TIMEOUT) {
>> >> + if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
>> >
>> > I think you've had a git accident somewhere:
>> > The existing line upstream is still
>> > if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
>> > This hunk should have just dropped out.
>> >
>>
>> No, it's ok. It just turned out, there were two concurrent definitions
>> of the same thing. Please take a look at what's available on top of
>> the master branch:
>
> Aah, that's not the same location as in your v1, is it? :)
>
Indeed :)
Thanks,
Marcin
> *grumble*
>
> Sure:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
>> Header:
>> [...]
>> #define PHY_ANEG_TIMEOUT 4000
>>
>> #define PHY_INTERFACE_MODE_RGMII 0
>> #define PHY_INTERFACE_MODE_RGMII_ID 1
>> #define PHY_INTERFACE_MODE_RGMII_RXID 2
>> #define PHY_INTERFACE_MODE_RGMII_TXID 3
>> #define PHY_INTERFACE_MODE_SGMII 4
>> #define PHY_INTERFACE_MODE_RTBI 5
>>
>> #define PHY_AUTONEGOTIATE_TIMEOUT 5000
>> [...]
>>
>> Driver:
>> ~Line 204:
>> DEBUG((DEBUG_ERROR,"MvPhyDxe: Waiting for PHY realtime link"));
>> while (!(Data & MIIM_88E1xxx_PHYSTAT_SPDDONE)) {
>> if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
>>
>> ~Line 327:
>> DEBUG((DEBUG_ERROR, "MvPhyDxe: Waiting for PHY auto negotiation... "));
>> for (i = 0; !(Data & BMSR_ANEGCOMPLETE); i++) {
>> if (i > PHY_ANEG_TIMEOUT) {
>> DEBUG((DEBUG_ERROR, "timeout\n"));
>>
>> Best regards,
>> Marcin
^ permalink raw reply [flat|nested] 13+ messages in thread