diff mbox

[U-Boot] arm: dts: Pine64: add Ethernet alias

Message ID 1477008706-13387-1-git-send-email-andre.przywara@arm.com
State Accepted
Commit 6301e80ccfdf8b8daf921c67ffb5c5a17960f895
Delegated to: Hans de Goede
Headers show

Commit Message

Andre Przywara Oct. 21, 2016, 12:11 a.m. UTC
The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
missing an alias entry to trigger the driver instantiation by U-Boot.
Add the line to point U-Boot to the Ethernet DT node.
This enables TFTP boot on the Pine64.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

Comments

Jagan Teki Oct. 21, 2016, 9:31 a.m. UTC | #1
On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara <andre.przywara@arm.com> wrote:
> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
> missing an alias entry to trigger the driver instantiation by U-Boot.
> Add the line to point U-Boot to the Ethernet DT node.
> This enables TFTP boot on the Pine64.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
> index d5a7249..c0fde44 100644
> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
> @@ -46,6 +46,7 @@
>
>         aliases {
>                 serial0 = &uart0;
> +               ethernet0 = &emac;

I think alias doesn’t require for probing emac, it will straight away
probed like

> dm tree
....
eth         [ + ]     ethernet@01c30000

Did you find any issue while detecting eth?

thanks!
Andre Przywara Oct. 21, 2016, 10:06 a.m. UTC | #2
Hi,

On 21/10/16 10:31, Jagan Teki wrote:
> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>> missing an alias entry to trigger the driver instantiation by U-Boot.
>> Add the line to point U-Boot to the Ethernet DT node.
>> This enables TFTP boot on the Pine64.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>> index d5a7249..c0fde44 100644
>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>> @@ -46,6 +46,7 @@
>>
>>         aliases {
>>                 serial0 = &uart0;
>> +               ethernet0 = &emac;
> 
> I think alias doesn’t require for probing emac, it will straight away
> probed like
> 
>> dm tree
> ....
> eth         [ + ]     ethernet@01c30000
> 
> Did you find any issue while detecting eth?

Yes, it just didn't work ;-) I don't have a board here, but can post the
error message later tonight.
In fact I was wondering about that already, maybe it's worth
investigating this further.

But aside from that I think the MAC address calculation based on the SID
serial number does not get triggered without an alias, so we need this
line anyway.

Cheers,
Andre.
Hans de Goede Oct. 21, 2016, 10:28 a.m. UTC | #3
Hi,

On 21-10-16 12:06, Andre Przywara wrote:
> Hi,
>
> On 21/10/16 10:31, Jagan Teki wrote:
>> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>>> missing an alias entry to trigger the driver instantiation by U-Boot.
>>> Add the line to point U-Boot to the Ethernet DT node.
>>> This enables TFTP boot on the Pine64.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>> index d5a7249..c0fde44 100644
>>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>> @@ -46,6 +46,7 @@
>>>
>>>         aliases {
>>>                 serial0 = &uart0;
>>> +               ethernet0 = &emac;
>>
>> I think alias doesn’t require for probing emac, it will straight away
>> probed like
>>
>>> dm tree
>> ....
>> eth         [ + ]     ethernet@01c30000
>>
>> Did you find any issue while detecting eth?
>
> Yes, it just didn't work ;-) I don't have a board here, but can post the
> error message later tonight.
> In fact I was wondering about that already, maybe it's worth
> investigating this further.
>
> But aside from that I think the MAC address calculation based on the SID
> serial number does not get triggered without an alias, so we need this
> line anyway.

Correct, the MAC address code relies on the alias.

Regards,

Hans
Andre Przywara Oct. 24, 2016, 10:29 p.m. UTC | #4
On 21/10/16 11:28, Hans de Goede wrote:
> Hi,
> 
> On 21-10-16 12:06, Andre Przywara wrote:
>> Hi,
>>
>> On 21/10/16 10:31, Jagan Teki wrote:
>>> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara
>>> <andre.przywara@arm.com> wrote:
>>>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>>>> missing an alias entry to trigger the driver instantiation by U-Boot.
>>>> Add the line to point U-Boot to the Ethernet DT node.
>>>> This enables TFTP boot on the Pine64.
>>>>
>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>> ---
>>>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>> b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>> index d5a7249..c0fde44 100644
>>>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>> @@ -46,6 +46,7 @@
>>>>
>>>>         aliases {
>>>>                 serial0 = &uart0;
>>>> +               ethernet0 = &emac;
>>>
>>> I think alias doesn’t require for probing emac, it will straight away
>>> probed like
>>>
>>>> dm tree
>>> ....
>>> eth         [ + ]     ethernet@01c30000
>>>
>>> Did you find any issue while detecting eth?
>>
>> Yes, it just didn't work ;-) I don't have a board here, but can post the
>> error message later tonight.

....
Net:   phy interface7

Error: ethernet@01c30000 address not set.
No ethernet found.
....

>> In fact I was wondering about that already, maybe it's worth
>> investigating this further.

So the reason is that CONFIG_NET_RANDOM_ETHADDR isn't defined, so
without the DT alias triggering the SID MAC generation there will be
_no_ MAC address at all, which makes the driver give up.
AFAIK on the A64 the MAC generation from the SID serial number works
just fine, so we should in any case add the alias.

I can resend the patch with an amended commit message.

Cheers,
Andre.

>>
>> But aside from that I think the MAC address calculation based on the SID
>> serial number does not get triggered without an alias, so we need this
>> line anyway.
> 
> Correct, the MAC address code relies on the alias.
> 
> Regards,
> 
> Hans
>
Jagan Teki Oct. 26, 2016, 7 a.m. UTC | #5
On Tue, Oct 25, 2016 at 3:59 AM, André Przywara <andre.przywara@arm.com> wrote:
> On 21/10/16 11:28, Hans de Goede wrote:
>> Hi,
>>
>> On 21-10-16 12:06, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 21/10/16 10:31, Jagan Teki wrote:
>>>> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara
>>>> <andre.przywara@arm.com> wrote:
>>>>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>>>>> missing an alias entry to trigger the driver instantiation by U-Boot.
>>>>> Add the line to point U-Boot to the Ethernet DT node.
>>>>> This enables TFTP boot on the Pine64.
>>>>>
>>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>>> ---
>>>>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>> b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>> index d5a7249..c0fde44 100644
>>>>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>> @@ -46,6 +46,7 @@
>>>>>
>>>>>         aliases {
>>>>>                 serial0 = &uart0;
>>>>> +               ethernet0 = &emac;
>>>>
>>>> I think alias doesn’t require for probing emac, it will straight away
>>>> probed like
>>>>
>>>>> dm tree
>>>> ....
>>>> eth         [ + ]     ethernet@01c30000
>>>>
>>>> Did you find any issue while detecting eth?
>>>
>>> Yes, it just didn't work ;-) I don't have a board here, but can post the
>>> error message later tonight.
>
> ....
> Net:   phy interface7
>
> Error: ethernet@01c30000 address not set.
> No ethernet found.
> ....
>
>>> In fact I was wondering about that already, maybe it's worth
>>> investigating this further.
>
> So the reason is that CONFIG_NET_RANDOM_ETHADDR isn't defined, so
> without the DT alias triggering the SID MAC generation there will be
> _no_ MAC address at all, which makes the driver give up.
> AFAIK on the A64 the MAC generation from the SID serial number works
> just fine, so we should in any case add the alias.

I wondered how the alias related to MAC generation, tried fec_mxc
without ethernet0 alias but couldn't find the change/generation on MAC
at-all.

thanks!
Hans de Goede Oct. 26, 2016, 7:44 a.m. UTC | #6
Hi,

On 26-10-16 09:00, Jagan Teki wrote:
> On Tue, Oct 25, 2016 at 3:59 AM, André Przywara <andre.przywara@arm.com> wrote:
>> On 21/10/16 11:28, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 21-10-16 12:06, Andre Przywara wrote:
>>>> Hi,
>>>>
>>>> On 21/10/16 10:31, Jagan Teki wrote:
>>>>> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara
>>>>> <andre.przywara@arm.com> wrote:
>>>>>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>>>>>> missing an alias entry to trigger the driver instantiation by U-Boot.
>>>>>> Add the line to point U-Boot to the Ethernet DT node.
>>>>>> This enables TFTP boot on the Pine64.
>>>>>>
>>>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>>>> ---
>>>>>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>>>>>  1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>> b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>> index d5a7249..c0fde44 100644
>>>>>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>> @@ -46,6 +46,7 @@
>>>>>>
>>>>>>         aliases {
>>>>>>                 serial0 = &uart0;
>>>>>> +               ethernet0 = &emac;
>>>>>
>>>>> I think alias doesn’t require for probing emac, it will straight away
>>>>> probed like
>>>>>
>>>>>> dm tree
>>>>> ....
>>>>> eth         [ + ]     ethernet@01c30000
>>>>>
>>>>> Did you find any issue while detecting eth?
>>>>
>>>> Yes, it just didn't work ;-) I don't have a board here, but can post the
>>>> error message later tonight.
>>
>> ....
>> Net:   phy interface7
>>
>> Error: ethernet@01c30000 address not set.
>> No ethernet found.
>> ....
>>
>>>> In fact I was wondering about that already, maybe it's worth
>>>> investigating this further.
>>
>> So the reason is that CONFIG_NET_RANDOM_ETHADDR isn't defined, so
>> without the DT alias triggering the SID MAC generation there will be
>> _no_ MAC address at all, which makes the driver give up.
>> AFAIK on the A64 the MAC generation from the SID serial number works
>> just fine, so we should in any case add the alias.
>
> I wondered how the alias related to MAC generation,

See the setup_environment() function in board/sunxi/board.c, we use
the alias-es to determine for which interface we need to generate
a random MAC and set ethaddr / eth1addr in the environment.

Regards,

Hans
Jagan Teki Oct. 26, 2016, 6:34 p.m. UTC | #7
On Wed, Oct 26, 2016 at 1:14 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 26-10-16 09:00, Jagan Teki wrote:
>>
>> On Tue, Oct 25, 2016 at 3:59 AM, André Przywara <andre.przywara@arm.com>
>> wrote:
>>>
>>> On 21/10/16 11:28, Hans de Goede wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 21-10-16 12:06, Andre Przywara wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 21/10/16 10:31, Jagan Teki wrote:
>>>>>>
>>>>>> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara
>>>>>> <andre.przywara@arm.com> wrote:
>>>>>>>
>>>>>>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>>>>>>> missing an alias entry to trigger the driver instantiation by U-Boot.
>>>>>>> Add the line to point U-Boot to the Ethernet DT node.
>>>>>>> This enables TFTP boot on the Pine64.
>>>>>>>
>>>>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>>>>> ---
>>>>>>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>>>>>>  1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>>> b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>>> index d5a7249..c0fde44 100644
>>>>>>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>>> @@ -46,6 +46,7 @@
>>>>>>>
>>>>>>>         aliases {
>>>>>>>                 serial0 = &uart0;
>>>>>>> +               ethernet0 = &emac;
>>>>>>
>>>>>>
>>>>>> I think alias doesn’t require for probing emac, it will straight away
>>>>>> probed like
>>>>>>
>>>>>>> dm tree
>>>>>>
>>>>>> ....
>>>>>> eth         [ + ]     ethernet@01c30000
>>>>>>
>>>>>> Did you find any issue while detecting eth?
>>>>>
>>>>>
>>>>> Yes, it just didn't work ;-) I don't have a board here, but can post
>>>>> the
>>>>> error message later tonight.
>>>
>>>
>>> ....
>>> Net:   phy interface7
>>>
>>> Error: ethernet@01c30000 address not set.
>>> No ethernet found.
>>> ....
>>>
>>>>> In fact I was wondering about that already, maybe it's worth
>>>>> investigating this further.
>>>
>>>
>>> So the reason is that CONFIG_NET_RANDOM_ETHADDR isn't defined, so
>>> without the DT alias triggering the SID MAC generation there will be
>>> _no_ MAC address at all, which makes the driver give up.
>>> AFAIK on the A64 the MAC generation from the SID serial number works
>>> just fine, so we should in any case add the alias.
>>
>>
>> I wondered how the alias related to MAC generation,
>
>
> See the setup_environment() function in board/sunxi/board.c, we use
> the alias-es to determine for which interface we need to generate
> a random MAC and set ethaddr / eth1addr in the environment.

Ohh,OK.
But not a good way of setting MAC on the board files. I think if we
add .read_rom_hwaddr on the driver by placing SIM code on arch.

thanks!
Jagan Teki Oct. 26, 2016, 6:51 p.m. UTC | #8
On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara <andre.przywara@arm.com> wrote:
> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
> missing an alias entry to trigger the driver instantiation by U-Boot.
> Add the line to point U-Boot to the Ethernet DT node.
> This enables TFTP boot on the Pine64.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
> index d5a7249..c0fde44 100644
> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
> @@ -46,6 +46,7 @@
>
>         aliases {
>                 serial0 = &uart0;
> +               ethernet0 = &emac;

Better to have this alias on sun50i-a64.dtsi since the node is defined there?

thanks!
Andre Przywara Oct. 26, 2016, 8:50 p.m. UTC | #9
On 26/10/16 19:51, Jagan Teki wrote:
Hi,

> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>> missing an alias entry to trigger the driver instantiation by U-Boot.
>> Add the line to point U-Boot to the Ethernet DT node.
>> This enables TFTP boot on the Pine64.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>> index d5a7249..c0fde44 100644
>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>> @@ -46,6 +46,7 @@
>>
>>         aliases {
>>                 serial0 = &uart0;
>> +               ethernet0 = &emac;
> 
> Better to have this alias on sun50i-a64.dtsi since the node is defined there?

Mmh, I find examples for both ways (.dtsi vs. .dts) in the kernel. I
need to learn what's the best practice here.

But the U-Boot DTs need to get replaced anyway soon-ish, since the Linux
DTs (which are WIP) have already diverged.
So what about we keep this alias here next to the existing one for the
2016.11 release and possibly fix this later once we replace the DTs
anyway with what gets merged into the kernel eventually?

In the end of the day it's an easy and obvious fix which makes many
people happy (since it allows TFTP boot of the Pine64).

Cheers,
Andre.
Andreas Färber Oct. 27, 2016, 4:18 p.m. UTC | #10
Hi André,

Am 26.10.2016 um 22:50 schrieb André Przywara:
> On 26/10/16 19:51, Jagan Teki wrote:
>> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>>> missing an alias entry to trigger the driver instantiation by U-Boot.
>>> Add the line to point U-Boot to the Ethernet DT node.
>>> This enables TFTP boot on the Pine64.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>> index d5a7249..c0fde44 100644
>>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>> @@ -46,6 +46,7 @@
>>>
>>>         aliases {
>>>                 serial0 = &uart0;
>>> +               ethernet0 = &emac;
>>
>> Better to have this alias on sun50i-a64.dtsi since the node is defined there?
> 
> Mmh, I find examples for both ways (.dtsi vs. .dts) in the kernel. I
> need to learn what's the best practice here.

Last I was told by kernel maintainers (in an Amlogic context) aliases
should be in the board's .dts[i] so that only the enabled nodes get
numbered. If they're enabled at .dtsi level then I guess nothing is
wrong with having the alias there. So I'm guessing the patch is good.

Reviewed-by: Andreas Färber <afaerber@suse.de>

Cheers,
Andreas
Jagan Teki Oct. 28, 2016, 5:51 p.m. UTC | #11
On Thu, Oct 27, 2016 at 2:20 AM, André Przywara <andre.przywara@arm.com> wrote:
> On 26/10/16 19:51, Jagan Teki wrote:
> Hi,
>
>> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>>> missing an alias entry to trigger the driver instantiation by U-Boot.
>>> Add the line to point U-Boot to the Ethernet DT node.
>>> This enables TFTP boot on the Pine64.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>> index d5a7249..c0fde44 100644
>>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>> @@ -46,6 +46,7 @@
>>>
>>>         aliases {
>>>                 serial0 = &uart0;
>>> +               ethernet0 = &emac;
>>
>> Better to have this alias on sun50i-a64.dtsi since the node is defined there?
>
> Mmh, I find examples for both ways (.dtsi vs. .dts) in the kernel. I
> need to learn what's the best practice here.
>
> But the U-Boot DTs need to get replaced anyway soon-ish, since the Linux
> DTs (which are WIP) have already diverged.
> So what about we keep this alias here next to the existing one for the
> 2016.11 release and possibly fix this later once we replace the DTs
> anyway with what gets merged into the kernel eventually?

OK, but please stick with the current discussion and still I see a
good point to see alias on dtsi because the dts files which are using
this dtsi shouldn't separately define the alias.

And also status on dtsi is showing "disabled" are you enabled it some
other patches? or missed?

thanks!
Hans de Goede Oct. 29, 2016, 12:29 p.m. UTC | #12
Hi,

On 21-10-16 02:11, Andre Przywara wrote:
> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
> missing an alias entry to trigger the driver instantiation by U-Boot.
> Add the line to point U-Boot to the Ethernet DT node.
> This enables TFTP boot on the Pine64.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>


Thank you I've applied this to my tree and will
include it in my next pull-req.

Regards,

Hans




> ---
>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
> index d5a7249..c0fde44 100644
> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
> @@ -46,6 +46,7 @@
>
>  	aliases {
>  		serial0 = &uart0;
> +		ethernet0 = &emac;
>  	};
>
>  	soc {
>
Jagan Teki Oct. 30, 2016, 8:48 a.m. UTC | #13
On Fri, Oct 28, 2016 at 11:21 PM, Jagan Teki <jagan@openedev.com> wrote:
> On Thu, Oct 27, 2016 at 2:20 AM, André Przywara <andre.przywara@arm.com> wrote:
>> On 26/10/16 19:51, Jagan Teki wrote:
>> Hi,
>>
>>> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>>>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>>>> missing an alias entry to trigger the driver instantiation by U-Boot.
>>>> Add the line to point U-Boot to the Ethernet DT node.
>>>> This enables TFTP boot on the Pine64.
>>>>
>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>> ---
>>>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>> index d5a7249..c0fde44 100644
>>>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>> @@ -46,6 +46,7 @@
>>>>
>>>>         aliases {
>>>>                 serial0 = &uart0;
>>>> +               ethernet0 = &emac;
>>>
>>> Better to have this alias on sun50i-a64.dtsi since the node is defined there?
>>
>> Mmh, I find examples for both ways (.dtsi vs. .dts) in the kernel. I
>> need to learn what's the best practice here.
>>
>> But the U-Boot DTs need to get replaced anyway soon-ish, since the Linux
>> DTs (which are WIP) have already diverged.
>> So what about we keep this alias here next to the existing one for the
>> 2016.11 release and possibly fix this later once we replace the DTs
>> anyway with what gets merged into the kernel eventually?
>
> OK, but please stick with the current discussion and still I see a
> good point to see alias on dtsi because the dts files which are using
> this dtsi shouldn't separately define the alias.
>
> And also status on dtsi is showing "disabled" are you enabled it some
> other patches? or missed?

Any response for this?

thanks!
Andre Przywara Oct. 30, 2016, 9:46 a.m. UTC | #14
On 30/10/16 08:48, Jagan Teki wrote:
> On Fri, Oct 28, 2016 at 11:21 PM, Jagan Teki <jagan@openedev.com> wrote:
>> On Thu, Oct 27, 2016 at 2:20 AM, André Przywara <andre.przywara@arm.com> wrote:
>>> On 26/10/16 19:51, Jagan Teki wrote:
>>> Hi,
>>>
>>>> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>>>>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>>>>> missing an alias entry to trigger the driver instantiation by U-Boot.
>>>>> Add the line to point U-Boot to the Ethernet DT node.
>>>>> This enables TFTP boot on the Pine64.
>>>>>
>>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>>> ---
>>>>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>> index d5a7249..c0fde44 100644
>>>>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>> @@ -46,6 +46,7 @@
>>>>>
>>>>>         aliases {
>>>>>                 serial0 = &uart0;
>>>>> +               ethernet0 = &emac;
>>>>
>>>> Better to have this alias on sun50i-a64.dtsi since the node is defined there?
>>>
>>> Mmh, I find examples for both ways (.dtsi vs. .dts) in the kernel. I
>>> need to learn what's the best practice here.
>>>
>>> But the U-Boot DTs need to get replaced anyway soon-ish, since the Linux
>>> DTs (which are WIP) have already diverged.
>>> So what about we keep this alias here next to the existing one for the
>>> 2016.11 release and possibly fix this later once we replace the DTs
>>> anyway with what gets merged into the kernel eventually?
>>
>> OK, but please stick with the current discussion and still I see a
>> good point to see alias on dtsi because the dts files which are using
>> this dtsi shouldn't separately define the alias.
>>
>> And also status on dtsi is showing "disabled" are you enabled it some
>> other patches? or missed?

We have the status = "okay" in sun50i-a64-pine64-plus.dts, which is the
only dts that is used these days.
The non-plus board version has a different PHY (100 MBit only) and a
quick test a while ago showed that this didn't work out of the box, so
we just don't enable it there. It seems like that not many people have
this version of the board and they would need to hack U-Boot to use it
anyway (because defconfig select -plus.dts).
This will be revisited when the SPL FIT support introduces board
detection and we actually choose the proper DT for each board.

Cheers,
Andre.
Jagan Teki Oct. 30, 2016, 12:16 p.m. UTC | #15
On Sun, Oct 30, 2016 at 3:16 PM, André Przywara <andre.przywara@arm.com> wrote:
> On 30/10/16 08:48, Jagan Teki wrote:
>> On Fri, Oct 28, 2016 at 11:21 PM, Jagan Teki <jagan@openedev.com> wrote:
>>> On Thu, Oct 27, 2016 at 2:20 AM, André Przywara <andre.przywara@arm.com> wrote:
>>>> On 26/10/16 19:51, Jagan Teki wrote:
>>>> Hi,
>>>>
>>>>> On Fri, Oct 21, 2016 at 5:41 AM, Andre Przywara <andre.przywara@arm.com> wrote:
>>>>>> The sun8i-emac driver works fine with the A64 Ethernet IP, but we are
>>>>>> missing an alias entry to trigger the driver instantiation by U-Boot.
>>>>>> Add the line to point U-Boot to the Ethernet DT node.
>>>>>> This enables TFTP boot on the Pine64.
>>>>>>
>>>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>>>> ---
>>>>>>  arch/arm/dts/sun50i-a64-pine64-common.dtsi | 1 +
>>>>>>  1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>> index d5a7249..c0fde44 100644
>>>>>> --- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>> +++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
>>>>>> @@ -46,6 +46,7 @@
>>>>>>
>>>>>>         aliases {
>>>>>>                 serial0 = &uart0;
>>>>>> +               ethernet0 = &emac;
>>>>>
>>>>> Better to have this alias on sun50i-a64.dtsi since the node is defined there?
>>>>
>>>> Mmh, I find examples for both ways (.dtsi vs. .dts) in the kernel. I
>>>> need to learn what's the best practice here.
>>>>
>>>> But the U-Boot DTs need to get replaced anyway soon-ish, since the Linux
>>>> DTs (which are WIP) have already diverged.
>>>> So what about we keep this alias here next to the existing one for the
>>>> 2016.11 release and possibly fix this later once we replace the DTs
>>>> anyway with what gets merged into the kernel eventually?
>>>
>>> OK, but please stick with the current discussion and still I see a
>>> good point to see alias on dtsi because the dts files which are using
>>> this dtsi shouldn't separately define the alias.
>>>
>>> And also status on dtsi is showing "disabled" are you enabled it some
>>> other patches? or missed?
>
> We have the status = "okay" in sun50i-a64-pine64-plus.dts, which is the
> only dts that is used these days.
> The non-plus board version has a different PHY (100 MBit only) and a
> quick test a while ago showed that this didn't work out of the box, so
> we just don't enable it there. It seems like that not many people have
> this version of the board and they would need to hack U-Boot to use it
> anyway (because defconfig select -plus.dts).
> This will be revisited when the SPL FIT support introduces board
> detection and we actually choose the proper DT for each board.

OK.

Reviewed-by: Jagan Teki <jagan@openedev.com>

thanks!
diff mbox

Patch

diff --git a/arch/arm/dts/sun50i-a64-pine64-common.dtsi b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
index d5a7249..c0fde44 100644
--- a/arch/arm/dts/sun50i-a64-pine64-common.dtsi
+++ b/arch/arm/dts/sun50i-a64-pine64-common.dtsi
@@ -46,6 +46,7 @@ 
 
 	aliases {
 		serial0 = &uart0;
+		ethernet0 = &emac;
 	};
 
 	soc {