From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dongpo Li <lidongpo@hisilicon.com>
Date: Wed, 16 Nov 2016 18:08:25 +0800
Subject: [U-Boot] [RFC PATCH] common: miiphyutil: Work and report phy
 address in hex in mdio cmd
In-Reply-To: <8a0ef5cc-4007-5d37-565a-2c1d8d8f38ce@xilinx.com>
References: <52d0c195a77f77b0e35501f022f9d65fce83bc76.1479284686.git.michal.simek@xilinx.com>
 <582C2567.1090104@hisilicon.com>
 <8a0ef5cc-4007-5d37-565a-2c1d8d8f38ce@xilinx.com>
Message-ID: <582C3019.5000403@hisilicon.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de



On 2016/11/16 17:31, Michal Simek wrote:
> On 16.11.2016 10:22, Dongpo Li wrote:
>>
>>
>> On 2016/11/16 16:24, Michal Simek wrote:
>>> It is confusing that mdio commands work and report phy id as
>>> decimal value when mii is working with hex values.
>>>
>>> For example:
>>> ZynqMP> mdio list
>>> gem:
>>> 21 - TI DP83867 <--> ethernet at ff0e0000
>>> ZynqMP> mdio read ethernet at ff0e0000 0
>>> Reading from bus gem
>>> PHY at address 21:
>>> 0 - 0x1140
>>> ZynqMP> mii dump 21 0
>>> Incorrect PHY address. Range should be 0-31
>>> ...
>>> ZynqMP> mii dump 15
>>> 0.     (1140)                 -- PHY control register --
>>>   (8000:0000) 0.15    =     0    reset
>>>
>>> U-Boot normally takes hex values that's why this patch is changing mdio
>>> command to handle hex instead of changing mii command to handle decimal
>>> values.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>>  cmd/mdio.c             | 6 +++---
>>>  common/miiphyutil.c    | 2 +-
>>>  drivers/net/zynq_gem.c | 2 +-
>>>  3 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/cmd/mdio.c b/cmd/mdio.c
>>> index fb13d050752a..21dc103736e7 100644
>>> --- a/cmd/mdio.c
>>> +++ b/cmd/mdio.c
>>> @@ -27,12 +27,12 @@ static uint last_reg_hi;
>>>  static int extract_range(char *input, int *plo, int *phi)
>>>  {
>>>  	char *end;
>>> -	*plo = simple_strtol(input, &end, 0);
>>> +	*plo = simple_strtol(input, &end, 16);
>> Hi, the last parameter 0 of function simple_strtol should mean
>> it can recognize the integer automatic as hex value if the data is beginning with 0x,
>> as octet value if beginning with 0, else decimal value.
>> So this code has no problem.
>> I checked the implementation of function simple_strtol and found the following code:
>>         if (!base)
>>                 base = 10;
>> So I think we should fix the implementation of simple_strtol.
> 
> yes. I have checked that implementation too. But the question is if this
> is aligned with others U-Boot parameters. It is confusing that with one
> command uses 21 as dec, another 15 or 0x15.
> 
> That's why will be worth to synchronize this.
> 
> md is one example where all is taken as hex.
> 
> ZynqMP> md 15 10
> 00000015: 00000000 00000000 01000000 00000000    ................
> 00000025: 00000000 00000000 00000000 00000000    ................
> 00000035: 00000000 00000000 00000000 00000000    ................
> 00000045: 00000000 00000000 00000000 00000000    ................
> ZynqMP> md f 10
> 0000000f: fe5000ff 000000ff 00000000 00000000    ..P.............
> 0000001f: 00000100 00000000 00000000 00000000    ................
> 0000002f: 00000000 00000000 00000000 00000000    ................
> 0000003f: 00000000 00000000 00000000 00000000    ................
> ZynqMP> md 0xf 10
> 0000000f: fe5000ff 000000ff 00000000 00000000    ..P.............
> 0000001f: 00000100 00000000 00000000 00000000    ................
> 0000002f: 00000000 00000000 00000000 00000000    ................
> 0000003f: 00000000 00000000 00000000 00000000    ................
I checked the md command, the code is:
addr = simple_strtoul(argv[1], NULL, 16);
length = simple_strtoul(argv[2], NULL, 16);
That's to say, you must consider the input addr and length as hex value
if you want to use the md command.