Re: [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group

[Linus Walleij]

On Fri, May 5, 2017 at 2:45 PM, Chris Brandt  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

RE: [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group

[Chris Brandt]

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

Re: [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group

[Geert Uytterhoeven]

Hi Linus,

On Fri, May 5, 2017 at 2:06 PM, Linus Walleij  wrote:
> On Fri, Apr 28, 2017 at 4:48 PM, Chris Brandt  
> 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 -- ge...@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

RE: [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group

[Chris Brandt]

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

Re: [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group

[Linus Walleij]

On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi
 wrote:

> Add pin configuration subnode for ETHER ethernet controller.
>
> Signed-off-by: Jacopo Mondi 
(...)
> +   pins_bidir {
> +   pinmux = ;/* 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

Re: [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group

[Geert Uytterhoeven]

Hi Chris,

On Thu, Apr 27, 2017 at 12:48 PM, Chris Brandt  wrote:
> On Thursday, April 27, 2017, Geert Uytterhoeven wrote:
>> > + {
>> > +   pinctrl-names = "default";
>> > +   pinctrl-0 = <_pins>;
>> > +
>> > +   status = "okay";
>> > +
>> > +   renesas,no-ether-link;
>> > +   phy-handle = <>;
>> > +   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 -- ge...@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

Re: [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group

[Simon Horman]

On Thu, Apr 27, 2017 at 10:48:45AM +, Chris Brandt wrote:
> Hi Geert,
> 
> On Thursday, April 27, 2017, Geert Uytterhoeven wrote:
> > > + {
> > > +   pinctrl-names = "default";
> > > +   pinctrl-0 = <_pins>;
> > > +
> > > +   status = "okay";
> > > +
> > > +   renesas,no-ether-link;
> > > +   phy-handle = <>;
> > > +   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?

RE: [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group

[Chris Brandt]

Hi Geert,

On Thursday, April 27, 2017, Geert Uytterhoeven wrote:
> > + {
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_pins>;
> > +
> > +   status = "okay";
> > +
> > +   renesas,no-ether-link;
> > +   phy-handle = <>;
> > +   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

Re: [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group

[Geert Uytterhoeven]

Hi Jacopo,

On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi
 wrote:
> Add pin configuration subnode for ETHER ethernet controller.
>
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Geert Uytterhoeven 

> --- a/arch/arm/boot/dts/r7s72100-genmai.dts
> +++ b/arch/arm/boot/dts/r7s72100-genmai.dts

> @@ -77,6 +105,19 @@
> status = "okay";
>  };
>
> + {
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_pins>;
> +
> +   status = "okay";
> +
> +   renesas,no-ether-link;
> +   phy-handle = <>;
> +   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 -- ge...@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