diff mbox series

[v2,6/7] ARM: dts: r9a06g032: Add the USBF controller node

Message ID 20221114111513.1436165-7-herve.codina@bootlin.com
State New
Headers
Series Add the Renesas USBF controller support |

Commit Message

Herve Codina Nov. 14, 2022, 11:15 a.m. UTC 
  Add the USBF controller available in the r9a06g032 SoC.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 arch/arm/boot/dts/r9a06g032.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Krzysztof Kozlowski Nov. 15, 2022, 1:16 p.m. UTC | #1 
On 14/11/2022 12:15, Herve Codina wrote:
> Add the USBF controller available in the r9a06g032 SoC.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  arch/arm/boot/dts/r9a06g032.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
> index 563024c9a4ae..a4bb069457a3 100644
> --- a/arch/arm/boot/dts/r9a06g032.dtsi
> +++ b/arch/arm/boot/dts/r9a06g032.dtsi
> @@ -117,6 +117,18 @@ dmamux: dma-router@a0 {
>  			};
>  		};
>  
> +		udc: usb@4001e000 {
> +			compatible = "renesas,r9a06g032-usbf", "renesas,rzn1-usbf";
> +			reg = <0x4001e000 0x2000>;
> +			interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&sysctrl R9A06G032_HCLK_USBF>,
> +				 <&sysctrl R9A06G032_HCLK_USBPM>;
> +			clock-names = "hclkf", "hclkpm";
> +			power-domains = <&sysctrl>;
> +			status = "disabled";

If you provided all resources (clocks, power domains etc), why disabling it?

Best regards,
Krzysztof
Herve Codina Nov. 15, 2022, 1:27 p.m. UTC | #2 
Hi Krzysztof,

On Tue, 15 Nov 2022 14:16:27 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 14/11/2022 12:15, Herve Codina wrote:
> > Add the USBF controller available in the r9a06g032 SoC.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  arch/arm/boot/dts/r9a06g032.dtsi | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
> > index 563024c9a4ae..a4bb069457a3 100644
> > --- a/arch/arm/boot/dts/r9a06g032.dtsi
> > +++ b/arch/arm/boot/dts/r9a06g032.dtsi
> > @@ -117,6 +117,18 @@ dmamux: dma-router@a0 {
> >  			};
> >  		};
> >  
> > +		udc: usb@4001e000 {
> > +			compatible = "renesas,r9a06g032-usbf", "renesas,rzn1-usbf";
> > +			reg = <0x4001e000 0x2000>;
> > +			interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&sysctrl R9A06G032_HCLK_USBF>,
> > +				 <&sysctrl R9A06G032_HCLK_USBPM>;
> > +			clock-names = "hclkf", "hclkpm";
> > +			power-domains = <&sysctrl>;
> > +			status = "disabled";  
> 
> If you provided all resources (clocks, power domains etc), why disabling it?

Because I forgot to remove the 'status' property ...
'status' will be simply removed in v3.
Sorry for this mistake.

Thanks for the review,
Hervé
Geert Uytterhoeven Nov. 15, 2022, 2:11 p.m. UTC | #3 
Hi Krzysztof,

On Tue, Nov 15, 2022 at 2:16 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 14/11/2022 12:15, Herve Codina wrote:
> > Add the USBF controller available in the r9a06g032 SoC.
> >
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  arch/arm/boot/dts/r9a06g032.dtsi | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
> > index 563024c9a4ae..a4bb069457a3 100644
> > --- a/arch/arm/boot/dts/r9a06g032.dtsi
> > +++ b/arch/arm/boot/dts/r9a06g032.dtsi
> > @@ -117,6 +117,18 @@ dmamux: dma-router@a0 {
> >                       };
> >               };
> >
> > +             udc: usb@4001e000 {
> > +                     compatible = "renesas,r9a06g032-usbf", "renesas,rzn1-usbf";
> > +                     reg = <0x4001e000 0x2000>;
> > +                     interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
> > +                                  <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
> > +                     clocks = <&sysctrl R9A06G032_HCLK_USBF>,
> > +                              <&sysctrl R9A06G032_HCLK_USBPM>;
> > +                     clock-names = "hclkf", "hclkpm";
> > +                     power-domains = <&sysctrl>;
> > +                     status = "disabled";
>
> If you provided all resources (clocks, power domains etc), why disabling it?

Doesn't this depend on wiring on the board, and providing pin control
in the board DTS?

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
Krzysztof Kozlowski Nov. 15, 2022, 2:58 p.m. UTC | #4 
On 15/11/2022 15:11, Geert Uytterhoeven wrote:
>>> +             udc: usb@4001e000 {
>>> +                     compatible = "renesas,r9a06g032-usbf", "renesas,rzn1-usbf";
>>> +                     reg = <0x4001e000 0x2000>;
>>> +                     interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
>>> +                                  <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
>>> +                     clocks = <&sysctrl R9A06G032_HCLK_USBF>,
>>> +                              <&sysctrl R9A06G032_HCLK_USBPM>;
>>> +                     clock-names = "hclkf", "hclkpm";
>>> +                     power-domains = <&sysctrl>;
>>> +                     status = "disabled";
>>
>> If you provided all resources (clocks, power domains etc), why disabling it?
> 
> Doesn't this depend on wiring on the board, and providing pin control
> in the board DTS?
> 

Yes, that could be the reason, so if this was the intention, it's fine.

Best regards,
Krzysztof
Herve Codina Nov. 15, 2022, 3:09 p.m. UTC | #5 
Hi Krzysztof

On Tue, 15 Nov 2022 14:27:54 +0100
Herve Codina <herve.codina@bootlin.com> wrote:

> Hi Krzysztof,
> 
> On Tue, 15 Nov 2022 14:16:27 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> > On 14/11/2022 12:15, Herve Codina wrote:  
> > > Add the USBF controller available in the r9a06g032 SoC.
> > > 
> > > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > > ---
> > >  arch/arm/boot/dts/r9a06g032.dtsi | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
> > > index 563024c9a4ae..a4bb069457a3 100644
> > > --- a/arch/arm/boot/dts/r9a06g032.dtsi
> > > +++ b/arch/arm/boot/dts/r9a06g032.dtsi
> > > @@ -117,6 +117,18 @@ dmamux: dma-router@a0 {
> > >  			};
> > >  		};
> > >  
> > > +		udc: usb@4001e000 {
> > > +			compatible = "renesas,r9a06g032-usbf", "renesas,rzn1-usbf";
> > > +			reg = <0x4001e000 0x2000>;
> > > +			interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
> > > +			clocks = <&sysctrl R9A06G032_HCLK_USBF>,
> > > +				 <&sysctrl R9A06G032_HCLK_USBPM>;
> > > +			clock-names = "hclkf", "hclkpm";
> > > +			power-domains = <&sysctrl>;
> > > +			status = "disabled";    
> > 
> > If you provided all resources (clocks, power domains etc), why disabling it?  
> 
> Because I forgot to remove the 'status' property ...
> 'status' will be simply removed in v3.
> Sorry for this mistake.
> 
> Thanks for the review,
> Hervé
> 

I said something completely wrong for this point.

status is set disabled because it is a .dtsi and can be
included by several dts to represent a board.
This node (USB device) can be wired on some board and not on
some others.
So, the node will be enabled in each dts board that has the USBF
device wired and used.

Hervé
Krzysztof Kozlowski Nov. 15, 2022, 4:30 p.m. UTC | #6 
On 15/11/2022 16:09, Herve Codina wrote:
> Hi Krzysztof
> 
> On Tue, 15 Nov 2022 14:27:54 +0100
> Herve Codina <herve.codina@bootlin.com> wrote:
> 
>> Hi Krzysztof,
>>
>> On Tue, 15 Nov 2022 14:16:27 +0100
>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>
>>> On 14/11/2022 12:15, Herve Codina wrote:  
>>>> Add the USBF controller available in the r9a06g032 SoC.
>>>>
>>>> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
>>>> ---
>>>>  arch/arm/boot/dts/r9a06g032.dtsi | 12 ++++++++++++
>>>>  1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
>>>> index 563024c9a4ae..a4bb069457a3 100644
>>>> --- a/arch/arm/boot/dts/r9a06g032.dtsi
>>>> +++ b/arch/arm/boot/dts/r9a06g032.dtsi
>>>> @@ -117,6 +117,18 @@ dmamux: dma-router@a0 {
>>>>  			};
>>>>  		};
>>>>  
>>>> +		udc: usb@4001e000 {
>>>> +			compatible = "renesas,r9a06g032-usbf", "renesas,rzn1-usbf";
>>>> +			reg = <0x4001e000 0x2000>;
>>>> +			interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
>>>> +			clocks = <&sysctrl R9A06G032_HCLK_USBF>,
>>>> +				 <&sysctrl R9A06G032_HCLK_USBPM>;
>>>> +			clock-names = "hclkf", "hclkpm";
>>>> +			power-domains = <&sysctrl>;
>>>> +			status = "disabled";    
>>>
>>> If you provided all resources (clocks, power domains etc), why disabling it?  
>>
>> Because I forgot to remove the 'status' property ...
>> 'status' will be simply removed in v3.
>> Sorry for this mistake.
>>
>> Thanks for the review,
>> Hervé
>>
> 
> I said something completely wrong for this point.
> 
> status is set disabled because it is a .dtsi and can be
> included by several dts to represent a board.
> This node (USB device) can be wired on some board and not on
> some others.
> So, the node will be enabled in each dts board that has the USBF
> device wired and used.

So it depends on having the connector? Yes, makes sense as well.
Actually my recommendation was about internal parts of OS, which usually
do not require anything from board. I missed the part that it is an USB...

Best regards,
Krzysztof
Geert Uytterhoeven Nov. 16, 2022, 8:51 a.m. UTC | #7 
On Mon, Nov 14, 2022 at 12:15 PM Herve Codina <herve.codina@bootlin.com> wrote:
> Add the USBF controller available in the r9a06g032 SoC.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
index 563024c9a4ae..a4bb069457a3 100644
--- a/arch/arm/boot/dts/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/r9a06g032.dtsi
@@ -117,6 +117,18 @@  dmamux: dma-router@a0 {
 			};
 		};
 
+		udc: usb@4001e000 {
+			compatible = "renesas,r9a06g032-usbf", "renesas,rzn1-usbf";
+			reg = <0x4001e000 0x2000>;
+			interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sysctrl R9A06G032_HCLK_USBF>,
+				 <&sysctrl R9A06G032_HCLK_USBPM>;
+			clock-names = "hclkf", "hclkpm";
+			power-domains = <&sysctrl>;
+			status = "disabled";
+		};
+
 		pci_usb: pci@40030000 {
 			compatible = "renesas,pci-r9a06g032", "renesas,pci-rzn1";
 			device_type = "pci";