| Message ID | 1493281194-5200-11-git-send-email-jacopo+renesas@jmondi.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Simon Horman |
| Headers | show |
Hi Jacopo, On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote: > Add pin configuration subnode for ETHER ethernet controller. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/arch/arm/boot/dts/r7s72100-genmai.dts > +++ b/arch/arm/boot/dts/r7s72100-genmai.dts > @@ -77,6 +105,19 @@ > status = "okay"; > }; > > +ðer { > + pinctrl-names = "default"; > + pinctrl-0 = <ðer_pins>; > + > + status = "okay"; > + > + renesas,no-ether-link; > + phy-handle = <&phy0>; > + phy0: ethernet-phy@0 { > + reg = <0>; Shouldn't the interrupt (connected to P1_15) be described? > + }; > +}; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, On Thursday, April 27, 2017, Geert Uytterhoeven wrote: > > +ðer { > > + pinctrl-names = "default"; > > + pinctrl-0 = <ðer_pins>; > > + > > + status = "okay"; > > + > > + renesas,no-ether-link; > > + phy-handle = <&phy0>; > > + phy0: ethernet-phy@0 { > > + reg = <0>; > > Shouldn't the interrupt (connected to P1_15) be described? That interrupt pin from the PHY is not used. It did not need to be connected. Chris
On Thu, Apr 27, 2017 at 10:48:45AM +0000, Chris Brandt wrote: > Hi Geert, > > On Thursday, April 27, 2017, Geert Uytterhoeven wrote: > > > +ðer { > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <ðer_pins>; > > > + > > > + status = "okay"; > > > + > > > + renesas,no-ether-link; > > > + phy-handle = <&phy0>; > > > + phy0: ethernet-phy@0 { > > > + reg = <0>; > > > > Shouldn't the interrupt (connected to P1_15) be described? > > > That interrupt pin from the PHY is not used. It did not need to be connected. So things are fine as above or should I expect to see v6?
Hi Chris, On Thu, Apr 27, 2017 at 12:48 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Thursday, April 27, 2017, Geert Uytterhoeven wrote: >> > +ðer { >> > + pinctrl-names = "default"; >> > + pinctrl-0 = <ðer_pins>; >> > + >> > + status = "okay"; >> > + >> > + renesas,no-ether-link; >> > + phy-handle = <&phy0>; >> > + phy0: ethernet-phy@0 { >> > + reg = <0>; >> >> Shouldn't the interrupt (connected to P1_15) be described? > > That interrupt pin from the PHY is not used. It did not need to be connected. But it is connected, according to the schematics. DT describes hardware, not software limitations. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote: > Add pin configuration subnode for ETHER ethernet controller. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> (...) > + pins_bidir { > + pinmux = <RZA1_PINMUX(3, 3, 2)>;/* P3_3 = ET_MDIO */ > + bi-directional; > + }; So I'm against merging this until someone explains what "bi-directional" actually means, electrically speaking. What happens physically on this pin? I think this just means open drain. It is dangerous to merge things we don't understand. Surely someone inside Renesas can answer this question. Yours, Linus Walleij
On Friday, April 28, 2017, Linus Walleij wrote: > > Add pin configuration subnode for ETHER ethernet controller. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > (...) > > + pins_bidir { > > + pinmux = <RZA1_PINMUX(3, 3, 2)>;/* P3_3 = > ET_MDIO */ > > + bi-directional; > > + }; > > So I'm against merging this until someone explains what "bi-directional" > actually means, electrically speaking. What happens physically on this > pin? > > I think this just means open drain. > > It is dangerous to merge things we don't understand. > > Surely someone inside Renesas can answer this question. I don't think this has anything to do with open drain because you need it for any pin that the peripheral IP block needs to transmit and receive over the same line, regardless of if it's a SDHI, I2C, Ethernet MDIO, etc... It's more about of allowing the internal IP block signals to get hooked up to the IO pad signals. Chris
Hi Geert, On Friday, April 28, 2017, Geert Uytterhoeven wrote: > >> Shouldn't the interrupt (connected to P1_15) be described? > > > > That interrupt pin from the PHY is not used. It did not need to be > connected. > > But it is connected, according to the schematics. P1_15 can be configured as: * GPIO_IN * AN7 * AVB_CAPTURE So...I guess you would 'describe' it as an GPIO-input. > DT describes hardware, not software limitations. Describing things is fine, but the kernel code takes the DT and then start configuring things based on it. For example, on the RSK board, that line is connected to P4_14. P4_14 can be configured as IRQ6...but IRQ6 comes out in 8 different pin choices, and I might want to use one of those other choices, so I don't want to describe P4_14 as an interrupt. If it was just describing that "pin 15 of the PHY chip is tied to pin Y19 of the RZ/A1H", that's fine because it's hardware connection that's not going to change. But...the DT also defines the pin muxing...which is a software decision (do I want to get interrupt or just manually poll or simple ignore it). This is the part of the whole "DT is for hardware description only" that doesn't really make sense to me. Chris
On Fri, Apr 28, 2017 at 4:48 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote: I think this was written by me, not Geert... >> DT describes hardware, not software limitations. > > Describing things is fine, but the kernel code takes the DT and then start configuring things based on it. > > For example, on the RSK board, that line is connected to P4_14. P4_14 can be configured as IRQ6...but IRQ6 comes out in 8 different pin choices, and I might want to use one of those other choices, so I don't want to describe P4_14 as an interrupt. > > If it was just describing that "pin 15 of the PHY chip is tied to pin Y19 of the RZ/A1H", that's fine because it's hardware connection that's not going to change. > But...the DT also defines the pin muxing...which is a software decision (do I want to get interrupt or just manually poll or simple ignore it). > This is the part of the whole "DT is for hardware description only" that doesn't really make sense to me. OK yeah we do hardware description AND configuration. And we never do interpreted languages. And then there is a bunch of grayzone things. For example we have a linux,input binding for connecting keypresses to certain Linux input codes. That is really grayzone, but very useful. Yours, Linus Walleij
Hi Linus, On Fri, May 5, 2017 at 2:06 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Apr 28, 2017 at 4:48 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote: > > I think this was written by me, not Geert... > >>> DT describes hardware, not software limitations. Na, I did write it. I can understand you sometimes mix up things, esp. when you're enjoying LLC ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Friday, May 05, 2017, Linus Walleij wrote: > > This is the part of the whole "DT is for hardware description only" that > doesn't really make sense to me. > > OK yeah we do hardware description AND configuration. > > And we never do interpreted languages. > > And then there is a bunch of grayzone things. For example we have a > linux,input binding for connecting keypresses to certain Linux input codes. > That is really grayzone, but very useful. Ah.... compatible = "linux,grayzone"; Thanks for the reply. I'll stop ranting now. Of course, I'll still probably screw it up again on a future patch somehow... Cheers Chris
On Fri, May 5, 2017 at 2:45 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Friday, May 05, 2017, Linus Walleij wrote: >> > This is the part of the whole "DT is for hardware description only" that >> doesn't really make sense to me. >> >> OK yeah we do hardware description AND configuration. >> >> And we never do interpreted languages. >> >> And then there is a bunch of grayzone things. For example we have a >> linux,input binding for connecting keypresses to certain Linux input codes. >> That is really grayzone, but very useful. > > Ah.... > > compatible = "linux,grayzone"; > > Thanks for the reply. I'll stop ranting now. > Of course, I'll still probably screw it up again on a future patch somehow... For this driver I remarked in some other thread that we may end up with some "renesas,bidirectional" or so. But yeah, it's kinda grayzone too. Yours, Linus Walleij
diff --git a/arch/arm/boot/dts/r7s72100-genmai.dts b/arch/arm/boot/dts/r7s72100-genmai.dts index f7c512e..328f4c9 100644 --- a/arch/arm/boot/dts/r7s72100-genmai.dts +++ b/arch/arm/boot/dts/r7s72100-genmai.dts @@ -63,6 +63,34 @@ pinmux = <RZA1_PINMUX(1, 4, 1)>, <RZA1_PINMUX(1, 5, 1)>; bi-directional; }; + + ether_pins: ether { + pins { + /* Ethernet on Ports 1,2,3,5 */ + pinmux = <RZA1_PINMUX(1, 14, 4)>,/* P1_14 = ET_COL */ + <RZA1_PINMUX(5, 9, 2)>, /* P5_9 = ET_MDC */ + <RZA1_PINMUX(3, 4, 2)>, /* P3_4 = ET_RXCLK */ + <RZA1_PINMUX(3, 5, 2)>, /* P3_5 = ET_RXER */ + <RZA1_PINMUX(3, 6, 2)>, /* P3_6 = ET_RXDV */ + <RZA1_PINMUX(2, 0, 2)>, /* P2_0 = ET_TXCLK */ + <RZA1_PINMUX(2, 1, 2)>, /* P2_1 = ET_TXER */ + <RZA1_PINMUX(2, 2, 2)>, /* P2_2 = ET_TXEN */ + <RZA1_PINMUX(2, 3, 2)>, /* P2_3 = ET_CRS */ + <RZA1_PINMUX(2, 4, 2)>, /* P2_4 = ET_TXD0 */ + <RZA1_PINMUX(2, 5, 2)>, /* P2_5 = ET_TXD1 */ + <RZA1_PINMUX(2, 6, 2)>, /* P2_6 = ET_TXD2 */ + <RZA1_PINMUX(2, 7, 2)>, /* P2_7 = ET_TXD3 */ + <RZA1_PINMUX(2, 8, 2)>, /* P2_8 = ET_RXD0 */ + <RZA1_PINMUX(2, 9, 2)>, /* P2_9 = ET_RXD1 */ + <RZA1_PINMUX(2, 10, 2)>,/* P2_10 = ET_RXD2 */ + <RZA1_PINMUX(2, 11, 2)>;/* P2_11 = ET_RXD3 */ + }; + + pins_bidir { + pinmux = <RZA1_PINMUX(3, 3, 2)>;/* P3_3 = ET_MDIO */ + bi-directional; + }; + }; }; &extal_clk { @@ -77,6 +105,19 @@ status = "okay"; }; +ðer { + pinctrl-names = "default"; + pinctrl-0 = <ðer_pins>; + + status = "okay"; + + renesas,no-ether-link; + phy-handle = <&phy0>; + phy0: ethernet-phy@0 { + reg = <0>; + }; +}; + &i2c2 { status = "okay"; clock-frequency = <400000>;
Add pin configuration subnode for ETHER ethernet controller. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- arch/arm/boot/dts/r7s72100-genmai.dts | 41 +++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)