RE: [PATCH 3/3] ARM: dts: r7s72100: add USB device to device tree

2018-01-05 Thread Chris Brandt 
Hi Geert,

On Friday, January 05, 2018, Geert Uytterhoeven wrote:
> > But then...the number goes back to '73' when you look at it in
> > /proc/interrupts.
> 
> Having an identical number in /proc/interrupts is a coincidence.
> These numbers are virtual, and may change even across reboots.

I'm not talking about the id number (the 1st column). I'm talking 
about the 4th column.

$ uname -rs
Linux 4.15.0-rc5-8-g366d22ea091b
$ cat /proc/interrupts
   CPU0
 17:   2365 GIC-0 135 Edge  ostm
 18:  0 GIC-0 230 Level e8008000.serial:rx err
 19: 45 GIC-0 231 Level e8008000.serial:rx full
 20:194 GIC-0 232 Level e8008000.serial:tx empty
 21:  0 GIC-0 229 Level e8008000.serial:break
 24:  0 GIC-0 189 Level riic-tend
 25:  0 GIC-0 190 Edge  riic-rdrf
 26:  0 GIC-0 191 Edge  riic-tdre
 27:  0 GIC-0 192 Level riic-stop
 29:  0 GIC-0 194 Level riic-nack
 32:160 GIC-0 213 Level riic-tend
 33:160 GIC-0 214 Edge  riic-rdrf
 34:480 GIC-0 215 Edge  riic-tdre
 35:240 GIC-0 216 Level riic-stop
 37:  0 GIC-0 218 Level riic-nack
 40:  0 GIC-0 139 Level fcff.timer
 42:  0 GIC-0 305 Level e804e800.sd
 43: 52 GIC-0 306 Level e804e800.sd
 44:  0 GIC-0 307 Level e804e800.sd
 45:  0 GIC-0 308 Edge  sh-rtc period
 46:  0 GIC-0 309 Edge  sh-rtc carry
 47:  0 GIC-0 310 Edge  sh-rtc alarm
 48:  0 GIC-0  73 Level e801.usbhs
Err:  0


> > Any opinions
> 
> Not really, except that no single .dts(i) file seems to have "- 32".

Then I can be a pioneer of new software!!  ;)


I'll just change it back to a single number when I upstream code just to
be consistent.


Thanks,
Chris

Re: [PATCH 3/3] ARM: dts: r7s72100: add USB device to device tree

2018-01-05 Thread Geert Uytterhoeven 
Hi Chris,

On Fri, Jan 5, 2018 at 1:54 PM, Chris Brandt  wrote:
> On Friday, January 05, 2018, Geert Uytterhoeven wrote:
>> > +   interrupts = ;
>>
>> "41", all other interrupt properties already have the SPI offset
>> subtracted?
>
> The actual HW vector number in the hardware manual is '73'.
> As you know, you need to subtract 32 for the number you use in the
> device tree.
> But then...the number goes back to '73' when you look at it in
> /proc/interrupts.

Having an identical number in /proc/interrupts is a coincidence.
These numbers are virtual, and may change even across reboots.

> So it was confusing to people on what number you needed to use.
>
> Therefore in the RZ/A1 BSP, I was doing (xx-32) so at least people could
> see the relationship between what's in the hardware manual and what
> gets put into the device tree.
>
> So, if doing (73-32) looks wrong, I can change it back to '41' for the
> upstream version.
>
> Any opinions

Not really, except that no single .dts(i) file seems to have "- 32".

Note that e.g. the R-Car Gen3 manuals do list both "interrupt ID"
and "SGI, PPI, or SPI No" in the documentation for INTC-AP.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 3/3] ARM: dts: r7s72100: add USB device to device tree

2018-01-05 Thread Chris Brandt 
Hi Geert,

Thanks for the review.

On Friday, January 05, 2018, Geert Uytterhoeven wrote:
> > +   interrupts = ;
> 
> "41", all other interrupt properties already have the SPI offset
> subtracted?

The actual HW vector number in the hardware manual is '73'.
As you know, you need to subtract 32 for the number you use in the 
device tree.
But then...the number goes back to '73' when you look at it in 
/proc/interrupts.

So it was confusing to people on what number you needed to use.

Therefore in the RZ/A1 BSP, I was doing (xx-32) so at least people could
see the relationship between what's in the hardware manual and what 
gets put into the device tree.

So, if doing (73-32) looks wrong, I can change it back to '41' for the 
upstream version.


Any opinions


Chris

RE: [PATCH 3/3] ARM: dts: r7s72100: add USB device to device tree

2018-01-05 Thread Chris Brandt 
On Friday, January 05, 2018, Sergei Shtylyov wrote:
> > +   usbhs0: usbhs@e801 {
> 
> The node names should be generic, i.e "usb@e801".
> 
> > +   compatible = "renesas,usbhs-r7s72100";
> > +   reg = <0xe801 0x1A0>;
> 
> Lowercase in the hex values, please be consistent...
> 
> > +   interrupts = ;
> > +   clocks = <_clks R7S72100_CLK_USB0>;
> > +   renesas,buswait = <4>;
> > +   power-domains = <_clocks>;
> > +   status = "disabled";
> > +   };
> > +
> > +   usbhs1: usbhs@e8207000 {
> > +   compatible = "renesas,usbhs-r7s72100";
> > +   reg = <0xe8207000 0x1A0>;
> > +   interrupts = ;
> > +   clocks = <_clks R7S72100_CLK_USB1>;
> > +   renesas,buswait = <4>;
> > +   power-domains = <_clocks>;
> > +   status = "disabled";
> > +   };
> >   };
> 
> The same comments for the 2nd device.

Thank you for the review.

I'll make the changes and resubmit.

Chris

N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: [PATCH 3/3] ARM: dts: r7s72100: add USB device to device tree

2018-01-05 Thread Simon Horman 
On Thu, Jan 04, 2018 at 03:01:50PM -0500, Chris Brandt wrote:
> Add USB device support.
> 
> Signed-off-by: Chris Brandt 

Hi Chris,

it looks like there have been some requests for (minor) changes to this
patch. Please consider addressing those and reposting.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: dts: r7s72100: add USB device to device tree

2018-01-05 Thread Sergei Shtylyov 

Hello!

On 1/4/2018 11:01 PM, Chris Brandt wrote:


Add USB device support.

Signed-off-by: Chris Brandt 
---
  arch/arm/boot/dts/r7s72100.dtsi | 20 
  1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index ab9645a42eca..eb414e735185 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -667,4 +667,24 @@
power-domains = <_clocks>;
status = "disabled";
};
+
+   usbhs0: usbhs@e801 {


   The node names should be generic, i.e "usb@e801".


+   compatible = "renesas,usbhs-r7s72100";
+   reg = <0xe801 0x1A0>;


   Lowercase in the hex values, please be consistent...


+   interrupts = ;
+   clocks = <_clks R7S72100_CLK_USB0>;
+   renesas,buswait = <4>;
+   power-domains = <_clocks>;
+   status = "disabled";
+   };
+
+   usbhs1: usbhs@e8207000 {
+   compatible = "renesas,usbhs-r7s72100";
+   reg = <0xe8207000 0x1A0>;
+   interrupts = ;
+   clocks = <_clks R7S72100_CLK_USB1>;
+   renesas,buswait = <4>;
+   power-domains = <_clocks>;
+   status = "disabled";
+   };
  };


   The same comments for the 2nd device.

MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: dts: r7s72100: add USB device to device tree

2018-01-05 Thread Geert Uytterhoeven 
On Thu, Jan 4, 2018 at 9:01 PM, Chris Brandt  wrote:
> Add USB device support.
>
> Signed-off-by: Chris Brandt 

Reviewed-by: Geert Uytterhoeven 

Minor nits below.

> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -667,4 +667,24 @@
> power-domains = <_clocks>;
> status = "disabled";
> };
> +
> +   usbhs0: usbhs@e801 {
> +   compatible = "renesas,usbhs-r7s72100";
> +   reg = <0xe801 0x1A0>;

0x1a0

> +   interrupts = ;

"41", all other interrupt properties already have the SPI offset subtracted?

> +   clocks = <_clks R7S72100_CLK_USB0>;
> +   renesas,buswait = <4>;
> +   power-domains = <_clocks>;
> +   status = "disabled";
> +   };
> +
> +   usbhs1: usbhs@e8207000 {
> +   compatible = "renesas,usbhs-r7s72100";
> +   reg = <0xe8207000 0x1A0>;

0x1a0

> +   interrupts = ;

"42", all other interrupt properties already have the SPI offset subtracted?

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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html