Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [Aspeed, v2 2/2] net: ftgmac100: Change the order of getting MAC
 address
To:     Hongwei Zhang <hongweiz@ami.com>, linux-aspeed@lists.ozlabs.org,
        linux-kernel@vger.kernel.org, openbmc@lists.ozlabs.org,
        Jakub Kicinski <kuba@kernel.org>,
        David S Miller <davem@davemloft.net>
Cc:     netdev <netdev@vger.kernel.org>, Joel Stanley <joel@jms.id.au>,
        Andrew Jeffery <andrew@aj.id.au>
References: <20201221205157.31501-2-hongweiz@ami.com>
 <20201222201437.5588-3-hongweiz@ami.com>
From:   Heiner Kallweit <hkallweit1@gmail.com>
Message-ID: <96c355a2-ab7e-3cf0-57e7-16369da78035@gmail.com>
Date:   Tue, 22 Dec 2020 21:46:52 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.0
MIME-Version: 1.0
In-Reply-To: <20201222201437.5588-3-hongweiz@ami.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 22.12.2020 21:14, Hongwei Zhang wrote:
> Dear Reviewer,
> 
> Use native MAC address is preferred over other choices, thus change the order
> of reading MAC address, try to read it from MAC chip first, if it's not
>  availabe, then try to read it from device tree.
> 
> 
> Hi Heiner,
> 
>> From:	Heiner Kallweit <hkallweit1@gmail.com>
>> Sent:	Monday, December 21, 2020 4:37 PM
>>> Change the order of reading MAC address, try to read it from MAC chip 
>>> first, if it's not availabe, then try to read it from device tree.
>>>
>> This commit message leaves a number of questions. It seems the change isn't related at all to the 
>> change that it's supposed to fix.
>>
>> - What is the issue that you're trying to fix?
>> - And what is wrong with the original change?
> 
> There is no bug or something wrong with the original code. This patch is for
> improving the code. We thought if the native MAC address is available, then
> it's preferred over MAC address from dts (assuming both sources are available).
> 
> One possible scenario, a MAC address is set in dts and the BMC image is 
> compiled and loaded into more than one platform, then the platforms will
> have network issue due to the same MAC address they read.
> 

Typically the DTS MAC address is overwritten by the boot loader, e.g. uboot.
And the boot loader can read it from chip registers. There are more drivers
trying to read the MAC address from DTS first. Eventually, I think, the code
here will read the same MAC address from chip registers as uboot did before.

> Thanks for your review, I've update the patch to fix the comments.
>>
>>> Fixes: 35c54922dc97 ("ARM: dts: tacoma: Add reserved memory for 
>>> ramoops")
>>> Signed-off-by: Hongwei Zhang <hongweiz@ami.com>
>>> ---
>>>  drivers/net/ethernet/faraday/ftgmac100.c | 22 +++++++++++++---------
>>>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> --Hongwei
>