diff mbox series

[v1,5/7] arm: dts: s700: add node for ethernet controller

Message ID 1589034315-19722-6-git-send-email-amittomer25@gmail.com
State Accepted
Commit 75523d54ac62fb433bb0a105093e996570b61e87
Delegated to: Tom Rini
Headers show
Series add Ethernet support for S700 | expand

Commit Message

Amit Tomer May 9, 2020, 2:25 p.m. UTC
This patch adds node for ethernet controller found on Action Semi OWL
S700 SoC.

Since, there is no upstream Linux binding exist for S700 ethernet
controller, Changes are put in u-boot specific dtsi file.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
 arch/arm/dts/s700-u-boot.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Andre Przywara May 12, 2020, 2:18 p.m. UTC | #1
On 09/05/2020 15:25, Amit Singh Tomar wrote:
> This patch adds node for ethernet controller found on Action Semi OWL
> S700 SoC.
> 
> Since, there is no upstream Linux binding exist for S700 ethernet
> controller, Changes are put in u-boot specific dtsi file.

But that should not be the S700 SoC .dtsi, instead the cubieboard .dts
file, since you specify the PHY mode in here (which is board specific).

> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
>  arch/arm/dts/s700-u-boot.dtsi | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/dts/s700-u-boot.dtsi b/arch/arm/dts/s700-u-boot.dtsi
> index a527cccc75f2..1b2768272c62 100644
> --- a/arch/arm/dts/s700-u-boot.dtsi
> +++ b/arch/arm/dts/s700-u-boot.dtsi
> @@ -6,6 +6,19 @@
>  /{
>  	soc {
>  		u-boot,dm-pre-reloc;
> +
> +		gmac:  ethernet@e0220000 {
> +			compatible = "actions,s700-ethernet";
> +			reg = <0 0xe0220000 0 0x2000>;
> +			interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "macirq";
> +			local-mac-address = [ 00 18 fe 66 66 66 ];

Is there another solution to that? Maybe put that in the environment
instead? Or generate this randomly or ideally by hashing some serial number?

Cheers,
Andre.

> +			clocks = <&cmu CLK_ETHERNET>, <&cmu CLK_RMII_REF>;
> +			clock-names = "ethernet", "rmii_ref";
> +			phy-mode = "rmii";
> +			status = "okay";
> +                };
> +
>  	};
>  };
>  
>
Tom Rini May 12, 2020, 2:25 p.m. UTC | #2
On Tue, May 12, 2020 at 03:18:33PM +0100, André Przywara wrote:
> On 09/05/2020 15:25, Amit Singh Tomar wrote:
> > This patch adds node for ethernet controller found on Action Semi OWL
> > S700 SoC.
> > 
> > Since, there is no upstream Linux binding exist for S700 ethernet
> > controller, Changes are put in u-boot specific dtsi file.
> 
> But that should not be the S700 SoC .dtsi, instead the cubieboard .dts
> file, since you specify the PHY mode in here (which is board specific).

The general way to move forward here is that bindings should be getting
proposed to Linux and accepted there, and U-Boot can take a WIP of them,
that gets updated later on to match, but we shouldn't get it here first.
Amit Tomer May 12, 2020, 2:37 p.m. UTC | #3
Hi,

On Tue, May 12, 2020 at 7:49 PM André Przywara <andre.przywara@arm.com> wrote:
>
> On 09/05/2020 15:25, Amit Singh Tomar wrote:
> > This patch adds node for ethernet controller found on Action Semi OWL
> > S700 SoC.
> >
> > Since, there is no upstream Linux binding exist for S700 ethernet
> > controller, Changes are put in u-boot specific dtsi file.
>
> But that should not be the S700 SoC .dtsi, instead the cubieboard .dts
> file, since you specify the PHY mode in here (which is board specific).

But MAC is present on SoC , so I thought of adding it s700 specific
u-boot.dtsi as
it is already hosting few other things .

> > Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> > ---
> >  arch/arm/dts/s700-u-boot.dtsi | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/arm/dts/s700-u-boot.dtsi b/arch/arm/dts/s700-u-boot.dtsi
> > index a527cccc75f2..1b2768272c62 100644
> > --- a/arch/arm/dts/s700-u-boot.dtsi
> > +++ b/arch/arm/dts/s700-u-boot.dtsi
> > @@ -6,6 +6,19 @@
> >  /{
> >       soc {
> >               u-boot,dm-pre-reloc;
> > +
> > +             gmac:  ethernet@e0220000 {
> > +                     compatible = "actions,s700-ethernet";
> > +                     reg = <0 0xe0220000 0 0x2000>;
> > +                     interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
> > +                     interrupt-names = "macirq";
> > +                     local-mac-address = [ 00 18 fe 66 66 66 ];
>
> Is there another solution to that? Maybe put that in the environment
> instead? Or generate this randomly or ideally by hashing some serial number?

Yeah, I tried having "CONFIG_NET_RANDOM_ETHADDR" other day which seems
to work(would double check it).

But is there a reason not to use "local-mac-address", since driver
does not support parsing this
property ?

Thanks
-Amit.
Amit Tomer May 12, 2020, 2:42 p.m. UTC | #4
Hi,

> The general way to move forward here is that bindings should be getting
> proposed to Linux and accepted there, and U-Boot can take a WIP of them,
> that gets updated later on to match, but we shouldn't get it here first.

I do have a plan to propose this binding to Linux but this is kind of
ready(I mean, the other bits
like PHY and driver) for U-boot. So thought of posting it to the list here.

Thanks
-Amit
Tom Rini May 12, 2020, 2:53 p.m. UTC | #5
On Tue, May 12, 2020 at 08:12:37PM +0530, Amit Tomer wrote:
> Hi,
> 
> > The general way to move forward here is that bindings should be getting
> > proposed to Linux and accepted there, and U-Boot can take a WIP of them,
> > that gets updated later on to match, but we shouldn't get it here first.
> 
> I do have a plan to propose this binding to Linux but this is kind of
> ready(I mean, the other bits
> like PHY and driver) for U-boot. So thought of posting it to the list here.

For some initial review, fine, but we need to have the Linux bindings
being reviewed at the same time, and before things come in to U-Boot.
Thanks!
Andre Przywara May 12, 2020, 2:53 p.m. UTC | #6
On 12/05/2020 15:25, Tom Rini wrote:
> On Tue, May 12, 2020 at 03:18:33PM +0100, André Przywara wrote:
>> On 09/05/2020 15:25, Amit Singh Tomar wrote:
>>> This patch adds node for ethernet controller found on Action Semi OWL
>>> S700 SoC.
>>>
>>> Since, there is no upstream Linux binding exist for S700 ethernet
>>> controller, Changes are put in u-boot specific dtsi file.
>>
>> But that should not be the S700 SoC .dtsi, instead the cubieboard .dts
>> file, since you specify the PHY mode in here (which is board specific).
> 
> The general way to move forward here is that bindings should be getting
> proposed to Linux and accepted there, and U-Boot can take a WIP of them,
> that gets updated later on to match, but we shouldn't get it here first.

Yes, this is of course a stop-gap measure, but a useful one: Being able
to TFTP a kernel helps with developing the kernel port, especially since
U-Boot doesn't support MMC (yet).
And there are easier things than pushing a DT binding into the kernel
tree without having a Linux driver ready ...

Cheers,
Andre
Andre Przywara May 12, 2020, 2:56 p.m. UTC | #7
On 12/05/2020 15:37, Amit Tomer wrote:
> Hi,
> 
> On Tue, May 12, 2020 at 7:49 PM André Przywara <andre.przywara@arm.com> wrote:
>>
>> On 09/05/2020 15:25, Amit Singh Tomar wrote:
>>> This patch adds node for ethernet controller found on Action Semi OWL
>>> S700 SoC.
>>>
>>> Since, there is no upstream Linux binding exist for S700 ethernet
>>> controller, Changes are put in u-boot specific dtsi file.
>>
>> But that should not be the S700 SoC .dtsi, instead the cubieboard .dts
>> file, since you specify the PHY mode in here (which is board specific).
> 
> But MAC is present on SoC , so I thought of adding it s700 specific
> u-boot.dtsi as
> it is already hosting few other things .

The MAC is indeed, but not the PHY and the MAC address.
And since this -u-boot.dtsi trick only works once in the chain, you have
to move this to the board version of the file.
Doesn't really make any difference looking at the vast number of boards
using the S700 ;-)

> 
>>> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
>>> ---
>>>  arch/arm/dts/s700-u-boot.dtsi | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/arch/arm/dts/s700-u-boot.dtsi b/arch/arm/dts/s700-u-boot.dtsi
>>> index a527cccc75f2..1b2768272c62 100644
>>> --- a/arch/arm/dts/s700-u-boot.dtsi
>>> +++ b/arch/arm/dts/s700-u-boot.dtsi
>>> @@ -6,6 +6,19 @@
>>>  /{
>>>       soc {
>>>               u-boot,dm-pre-reloc;
>>> +
>>> +             gmac:  ethernet@e0220000 {
>>> +                     compatible = "actions,s700-ethernet";
>>> +                     reg = <0 0xe0220000 0 0x2000>;
>>> +                     interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
>>> +                     interrupt-names = "macirq";
>>> +                     local-mac-address = [ 00 18 fe 66 66 66 ];
>>
>> Is there another solution to that? Maybe put that in the environment
>> instead? Or generate this randomly or ideally by hashing some serial number?
> 
> Yeah, I tried having "CONFIG_NET_RANDOM_ETHADDR" other day which seems
> to work(would double check it).
> 
> But is there a reason not to use "local-mac-address", since driver
> does not support parsing this
> property ?

I don't think local-mac-address is meant to be in a .dts file, actually.
It's more useful to communicate some MAC address between firmware and
bootloaders/kernels. You can use this in your development setup, but
this should not be submitted. Otherwise every board on the planet would
use the same MAC address.

Cheers,
Andre
Tom Rini May 12, 2020, 3:09 p.m. UTC | #8
On Tue, May 12, 2020 at 03:53:55PM +0100, André Przywara wrote:
> On 12/05/2020 15:25, Tom Rini wrote:
> > On Tue, May 12, 2020 at 03:18:33PM +0100, André Przywara wrote:
> >> On 09/05/2020 15:25, Amit Singh Tomar wrote:
> >>> This patch adds node for ethernet controller found on Action Semi OWL
> >>> S700 SoC.
> >>>
> >>> Since, there is no upstream Linux binding exist for S700 ethernet
> >>> controller, Changes are put in u-boot specific dtsi file.
> >>
> >> But that should not be the S700 SoC .dtsi, instead the cubieboard .dts
> >> file, since you specify the PHY mode in here (which is board specific).
> > 
> > The general way to move forward here is that bindings should be getting
> > proposed to Linux and accepted there, and U-Boot can take a WIP of them,
> > that gets updated later on to match, but we shouldn't get it here first.
> 
> Yes, this is of course a stop-gap measure, but a useful one: Being able
> to TFTP a kernel helps with developing the kernel port, especially since
> U-Boot doesn't support MMC (yet).
> And there are easier things than pushing a DT binding into the kernel
> tree without having a Linux driver ready ...

So, we're at -rc2 for v2020.07.  The DDR calculation stuff I can see
getting pulled in.  Is the ethernet driver for this SoC so far from done
that it's not ready for linux-next?  Things don't have to be in mainline
proper, but the expectation is that it's making reasonable progress
there and been reviewed so that binding changes between what we take in
U-Boot at first and a final re-sync once it is in mainline are minimal.
Amit Tomer May 12, 2020, 4:14 p.m. UTC | #9
> So, we're at -rc2 for v2020.07.  The DDR calculation stuff I can see
> getting pulled in.  Is the ethernet driver for this SoC so far from done
> that it's not ready for linux-next?  Things don't have to be in mainline
> proper, but the expectation is that it's making reasonable progress
> there and been reviewed so that binding changes between what we take in
> U-Boot at first and a final re-sync once it is in mainline are minimal.

To be honest, we haven't put any of the Ethernet bits for Linux yet.
We have put few patches for review to support MMC for S700 in Linux,
and once those gets merged we would start working on Ethernet.

Thanks
-Amit
Andre Przywara May 12, 2020, 4:39 p.m. UTC | #10
On 12/05/2020 16:09, Tom Rini wrote:

Hi,

> On Tue, May 12, 2020 at 03:53:55PM +0100, André Przywara wrote:
>> On 12/05/2020 15:25, Tom Rini wrote:
>>> On Tue, May 12, 2020 at 03:18:33PM +0100, André Przywara wrote:
>>>> On 09/05/2020 15:25, Amit Singh Tomar wrote:
>>>>> This patch adds node for ethernet controller found on Action Semi OWL
>>>>> S700 SoC.
>>>>>
>>>>> Since, there is no upstream Linux binding exist for S700 ethernet
>>>>> controller, Changes are put in u-boot specific dtsi file.
>>>>
>>>> But that should not be the S700 SoC .dtsi, instead the cubieboard .dts
>>>> file, since you specify the PHY mode in here (which is board specific).
>>>
>>> The general way to move forward here is that bindings should be getting
>>> proposed to Linux and accepted there, and U-Boot can take a WIP of them,
>>> that gets updated later on to match, but we shouldn't get it here first.
>>
>> Yes, this is of course a stop-gap measure, but a useful one: Being able
>> to TFTP a kernel helps with developing the kernel port, especially since
>> U-Boot doesn't support MMC (yet).
>> And there are easier things than pushing a DT binding into the kernel
>> tree without having a Linux driver ready ...
> 
> So, we're at -rc2 for v2020.07.  The DDR calculation stuff I can see
> getting pulled in.  Is the ethernet driver for this SoC so far from done
> that it's not ready for linux-next?  Things don't have to be in mainline
> proper, but the expectation is that it's making reasonable progress
> there and been reviewed so that binding changes between what we take in
> U-Boot at first and a final re-sync once it is in mainline are minimal.

I don't think there is any particular rush in getting this actually
merged. After all it seems like the users of this board can be counted
on one hand.
I think it's nice to have this on the list, so interested parties can
pull it in if there is a need. But we can surely wait how the binding
evolves in the kernel tree, though this might take some time.

Cheers,
Andre.
Tom Rini May 12, 2020, 7:59 p.m. UTC | #11
On Tue, May 12, 2020 at 05:39:46PM +0100, André Przywara wrote:
> On 12/05/2020 16:09, Tom Rini wrote:
> 
> Hi,
> 
> > On Tue, May 12, 2020 at 03:53:55PM +0100, André Przywara wrote:
> >> On 12/05/2020 15:25, Tom Rini wrote:
> >>> On Tue, May 12, 2020 at 03:18:33PM +0100, André Przywara wrote:
> >>>> On 09/05/2020 15:25, Amit Singh Tomar wrote:
> >>>>> This patch adds node for ethernet controller found on Action Semi OWL
> >>>>> S700 SoC.
> >>>>>
> >>>>> Since, there is no upstream Linux binding exist for S700 ethernet
> >>>>> controller, Changes are put in u-boot specific dtsi file.
> >>>>
> >>>> But that should not be the S700 SoC .dtsi, instead the cubieboard .dts
> >>>> file, since you specify the PHY mode in here (which is board specific).
> >>>
> >>> The general way to move forward here is that bindings should be getting
> >>> proposed to Linux and accepted there, and U-Boot can take a WIP of them,
> >>> that gets updated later on to match, but we shouldn't get it here first.
> >>
> >> Yes, this is of course a stop-gap measure, but a useful one: Being able
> >> to TFTP a kernel helps with developing the kernel port, especially since
> >> U-Boot doesn't support MMC (yet).
> >> And there are easier things than pushing a DT binding into the kernel
> >> tree without having a Linux driver ready ...
> > 
> > So, we're at -rc2 for v2020.07.  The DDR calculation stuff I can see
> > getting pulled in.  Is the ethernet driver for this SoC so far from done
> > that it's not ready for linux-next?  Things don't have to be in mainline
> > proper, but the expectation is that it's making reasonable progress
> > there and been reviewed so that binding changes between what we take in
> > U-Boot at first and a final re-sync once it is in mainline are minimal.
> 
> I don't think there is any particular rush in getting this actually
> merged. After all it seems like the users of this board can be counted
> on one hand.
> I think it's nice to have this on the list, so interested parties can
> pull it in if there is a need. But we can surely wait how the binding
> evolves in the kernel tree, though this might take some time.

OK, I think that'll be good enough for a while then.  I'll mark this as
RFC in patchwork so I don't forget it's not quite ready for merging
anyhow.  Thanks!
Tom Rini July 8, 2020, 3:03 a.m. UTC | #12
On Sat, May 09, 2020 at 07:55:13PM +0530, Amit Singh Tomar wrote:

> This patch adds node for ethernet controller found on Action Semi OWL
> S700 SoC.
> 
> Since, there is no upstream Linux binding exist for S700 ethernet
> controller, Changes are put in u-boot specific dtsi file.
> 
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/arch/arm/dts/s700-u-boot.dtsi b/arch/arm/dts/s700-u-boot.dtsi
index a527cccc75f2..1b2768272c62 100644
--- a/arch/arm/dts/s700-u-boot.dtsi
+++ b/arch/arm/dts/s700-u-boot.dtsi
@@ -6,6 +6,19 @@ 
 /{
 	soc {
 		u-boot,dm-pre-reloc;
+
+		gmac:  ethernet@e0220000 {
+			compatible = "actions,s700-ethernet";
+			reg = <0 0xe0220000 0 0x2000>;
+			interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "macirq";
+			local-mac-address = [ 00 18 fe 66 66 66 ];
+			clocks = <&cmu CLK_ETHERNET>, <&cmu CLK_RMII_REF>;
+			clock-names = "ethernet", "rmii_ref";
+			phy-mode = "rmii";
+			status = "okay";
+                };
+
 	};
 };