| Message ID | 1589034315-19722-6-git-send-email-amittomer25@gmail.com |
|---|---|
| State | Accepted |
| Commit | 75523d54ac62fb433bb0a105093e996570b61e87 |
| Headers | show |
| Series | add Ethernet support for S700 | expand |
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 at 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 at 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"; > + }; > + > }; > }; > >
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.
Hi, On Tue, May 12, 2020 at 7:49 PM Andr? Przywara <andre.przywara at 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 at 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 at 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.
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
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!
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
On 12/05/2020 15:37, Amit Tomer wrote: > Hi, > > On Tue, May 12, 2020 at 7:49 PM Andr? Przywara <andre.przywara at 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 at 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 at 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
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.
> 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
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.
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!
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 at 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"; + }; + }; };
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 at gmail.com> --- arch/arm/dts/s700-u-boot.dtsi | 13 +++++++++++++ 1 file changed, 13 insertions(+)