Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Krzysztof Kozlowski]

On Tue, Oct 04, 2016 at 03:37:35PM +0200, Geert Uytterhoeven wrote:
> On Wed, Aug 24, 2016 at 3:49 PM, Chanwoo Choi  wrote:
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> 
> > +   cluster_a53_opp_table: opp_table0 {
> > +   compatible = "operating-points-v2";
> > +   opp-shared;
> > +
> > +   opp@4 {
> > +   opp-hz = /bits/ 64 <4>;
> > +   opp-microvolt = <90>;
> > +   };
> 
> With W=1:
> 
> Warning (unit_address_vs_reg): Node /opp_table0/opp@4 has a
> unit name, but no reg property

AFAIR, there should be an exception for this... [1] But maybe it was not
added after all?

Best regards,
Krzysztof


[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/419738.html

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Krzysztof Kozlowski]

On Tue, Oct 04, 2016 at 03:37:35PM +0200, Geert Uytterhoeven wrote:
> On Wed, Aug 24, 2016 at 3:49 PM, Chanwoo Choi  wrote:
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> 
> > +   cluster_a53_opp_table: opp_table0 {
> > +   compatible = "operating-points-v2";
> > +   opp-shared;
> > +
> > +   opp@4 {
> > +   opp-hz = /bits/ 64 <4>;
> > +   opp-microvolt = <90>;
> > +   };
> 
> With W=1:
> 
> Warning (unit_address_vs_reg): Node /opp_table0/opp@4 has a
> unit name, but no reg property

AFAIR, there should be an exception for this... [1] But maybe it was not
added after all?

Best regards,
Krzysztof


[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/419738.html

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Geert Uytterhoeven]

On Wed, Aug 24, 2016 at 3:49 PM, Chanwoo Choi  wrote:
> --- /dev/null
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi

> +   cluster_a53_opp_table: opp_table0 {
> +   compatible = "operating-points-v2";
> +   opp-shared;
> +
> +   opp@4 {
> +   opp-hz = /bits/ 64 <4>;
> +   opp-microvolt = <90>;
> +   };

With W=1:

Warning (unit_address_vs_reg): Node /opp_table0/opp@4 has a
unit name, but no reg property
...

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 v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Geert Uytterhoeven]

On Wed, Aug 24, 2016 at 3:49 PM, Chanwoo Choi  wrote:
> --- /dev/null
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi

> +   cluster_a53_opp_table: opp_table0 {
> +   compatible = "operating-points-v2";
> +   opp-shared;
> +
> +   opp@4 {
> +   opp-hz = /bits/ 64 <4>;
> +   opp-microvolt = <90>;
> +   };

With W=1:

Warning (unit_address_vs_reg): Node /opp_table0/opp@4 has a
unit name, but no reg property
...

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 v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Javier Martinez Canillas]

Hello Chanwoo,

On 09/02/2016 12:59 PM, Chanwoo Choi wrote:
> Hi Javier,
> 
> On 2016년 08월 27일 02:49, Javier Martinez Canillas wrote:
>> Hello Chanwoo,
>>

[snip]

>>
>>> +   mipi_phy: video-phy@105C0708 {
>>> +   compatible = "samsung,exynos5433-mipi-video-phy";
>>> +   #phy-cells = <1>;
>>> +   samsung,pmu-syscon = <_system_controller>;
>>> +   samsung,cam0-sysreg = <_cam0>;
>>> +   samsung,cam1-sysreg = <_cam1>;
>>> +   samsung,disp-sysreg = <_disp>;
>>> +   };
>>
>> This node has a unit name but it doesn't have a reg property so dtc will
>> warn about the mismatch when executed with W=1.
> 
> The mipi_phy address are included in the PMU (base 0x105c_).
> There is no memory map for only MIPI_PHY. Instead, the mipi_phy driver[1]
> uses the regmap (pmu, syscon_cam0/1/disp).
> [1] drivers/phy/phy-exynos-mipi-video.c don't use
>
> Also, the first MIPI_PHY's address is 0x105c0710. I'll fix it.
> 
> If you want to add the 'reg' property, I can add DT node as following:
> 
>   mipi_phy: video-phy@105c0710 {
>   reg = <0x105c0710 0x0>;
>   compatible = "samsung,exynos5433-mipi-video-phy";
>   ...
>   };
> 
> But it looks strange because the mipi_phy driver[1] never gets
> the base address with of_iomap().
>

The DT should describe the actual HW regardless of the driver implementation.

I'm not sure what's the correct way to handle this (i.e: adding an unused reg
property or not having a unit name) but the mismatch should be fixed to avoid
dtc to complain about it.

>>
>> [snip]
>>
>>> +   usbdrd30: usb@1540 {
>>> +   compatible = "samsung,exynos5250-dwusb3";
>>> +   clocks = <_fsys CLK_ACLK_USBDRD30>,
>>> +   <_fsys CLK_SCLK_USBDRD30>;
>>> +   clock-names = "usbdrd30", "usbdrd30_susp_clk";
>>> +   assigned-clocks =
>>> +   <_fsys CLK_MOUT_SCLK_USBDRD30_USER>,
>>> +   <_top CLK_MOUT_SCLK_USBDRD30>,
>>> +   <_top CLK_DIV_SCLK_USBDRD30>;
>>> +   assigned-clock-parents =
>>> +   <_top CLK_SCLK_USBDRD30_FSYS>,
>>> +   <_top CLK_MOUT_BUS_PLL_USER>;
>>> +   assigned-clock-rates = <0>, <0>, <6670>;
>>> +   #address-cells = <1>;
>>> +   #size-cells = <1>;
>>> +   ranges;
>>> +   status = "disabled";
>>> +
>>
>> Ditto, the node has unit address but no reg property.
> 
> The usbdrd[2] driver don't map the base address.
> So, this DT node don't include any code calling the of_iomap().
> [2] drivers/usb/dwc3/dwc3-exynos.c
>

Same comment than above.

> But, the usbdrd's base address is 0x1540.
> 
>>
>>> +   dwc3 {
>>> +   compatible = "snps,dwc3";
>>> +   reg = <0x1540 0x1>;
>>
>> Ditto, the node doesn't have a unit address but has reg property.
> 
> OK. I'll modify it as following:
>   dwc3@1540 {
>

Ok.

>>
>>> +   interrupts = <0 231 0>;
>>> +   phys = <_phy 0>, <_phy 1>;
>>> +   phy-names = "usb2-phy", "usb3-phy";
>>> +   };
>>> +   };
>>> +
>>> +
>>> +   usbhost30: usb@15a0 {
>>> +   compatible = "samsung,exynos5250-dwusb3";
>>> +   clocks = <_fsys CLK_ACLK_USBHOST30>,
>>> +   <_fsys CLK_SCLK_USBHOST30>;
>>> +   clock-names = "usbdrd30", "usbdrd30_susp_clk";
>>> +   assigned-clocks =
>>> +   <_fsys CLK_MOUT_SCLK_USBHOST30_USER>,
>>> +   <_top CLK_MOUT_SCLK_USBHOST30>,
>>> +   <_top CLK_DIV_SCLK_USBHOST30>;
>>> +   assigned-clock-parents =
>>> +   <_top CLK_SCLK_USBHOST30_FSYS>,
>>> +   <_top CLK_MOUT_BUS_PLL_USER>;
>>> +   assigned-clock-rates = <0>, <0>, <6670>;
>>> +   #address-cells = <1>;
>>> +   #size-cells = <1>;
>>> +   ranges;
>>> +   status = "disabled";
>>> +
>>
>> Ditto, the node has unit address but no reg property.
> 
> This case is same with above 'usbdrd30'.
>

Same comment than above. The DT shouldn't leak Linux implementation details.

>>
>>> +   usbdrd_dwc3_0: dwc3 {
>>> +   compatible = "snps,dwc3";
>>> +   reg = <0x15a0 0x1>;
>>
>> Ditto, the node doesn't have a unit address but has reg property.
> 
> I'll modify it as following:
>   usbdrd_dwc3_0: dwc3@15a0 {
> 
>>
>>> +   interrupts = <0 244 0>;
>>> +  

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Javier Martinez Canillas]

Hello Chanwoo,

On 09/02/2016 12:59 PM, Chanwoo Choi wrote:
> Hi Javier,
> 
> On 2016년 08월 27일 02:49, Javier Martinez Canillas wrote:
>> Hello Chanwoo,
>>

[snip]

>>
>>> +   mipi_phy: video-phy@105C0708 {
>>> +   compatible = "samsung,exynos5433-mipi-video-phy";
>>> +   #phy-cells = <1>;
>>> +   samsung,pmu-syscon = <_system_controller>;
>>> +   samsung,cam0-sysreg = <_cam0>;
>>> +   samsung,cam1-sysreg = <_cam1>;
>>> +   samsung,disp-sysreg = <_disp>;
>>> +   };
>>
>> This node has a unit name but it doesn't have a reg property so dtc will
>> warn about the mismatch when executed with W=1.
> 
> The mipi_phy address are included in the PMU (base 0x105c_).
> There is no memory map for only MIPI_PHY. Instead, the mipi_phy driver[1]
> uses the regmap (pmu, syscon_cam0/1/disp).
> [1] drivers/phy/phy-exynos-mipi-video.c don't use
>
> Also, the first MIPI_PHY's address is 0x105c0710. I'll fix it.
> 
> If you want to add the 'reg' property, I can add DT node as following:
> 
>   mipi_phy: video-phy@105c0710 {
>   reg = <0x105c0710 0x0>;
>   compatible = "samsung,exynos5433-mipi-video-phy";
>   ...
>   };
> 
> But it looks strange because the mipi_phy driver[1] never gets
> the base address with of_iomap().
>

The DT should describe the actual HW regardless of the driver implementation.

I'm not sure what's the correct way to handle this (i.e: adding an unused reg
property or not having a unit name) but the mismatch should be fixed to avoid
dtc to complain about it.

>>
>> [snip]
>>
>>> +   usbdrd30: usb@1540 {
>>> +   compatible = "samsung,exynos5250-dwusb3";
>>> +   clocks = <_fsys CLK_ACLK_USBDRD30>,
>>> +   <_fsys CLK_SCLK_USBDRD30>;
>>> +   clock-names = "usbdrd30", "usbdrd30_susp_clk";
>>> +   assigned-clocks =
>>> +   <_fsys CLK_MOUT_SCLK_USBDRD30_USER>,
>>> +   <_top CLK_MOUT_SCLK_USBDRD30>,
>>> +   <_top CLK_DIV_SCLK_USBDRD30>;
>>> +   assigned-clock-parents =
>>> +   <_top CLK_SCLK_USBDRD30_FSYS>,
>>> +   <_top CLK_MOUT_BUS_PLL_USER>;
>>> +   assigned-clock-rates = <0>, <0>, <6670>;
>>> +   #address-cells = <1>;
>>> +   #size-cells = <1>;
>>> +   ranges;
>>> +   status = "disabled";
>>> +
>>
>> Ditto, the node has unit address but no reg property.
> 
> The usbdrd[2] driver don't map the base address.
> So, this DT node don't include any code calling the of_iomap().
> [2] drivers/usb/dwc3/dwc3-exynos.c
>

Same comment than above.

> But, the usbdrd's base address is 0x1540.
> 
>>
>>> +   dwc3 {
>>> +   compatible = "snps,dwc3";
>>> +   reg = <0x1540 0x1>;
>>
>> Ditto, the node doesn't have a unit address but has reg property.
> 
> OK. I'll modify it as following:
>   dwc3@1540 {
>

Ok.

>>
>>> +   interrupts = <0 231 0>;
>>> +   phys = <_phy 0>, <_phy 1>;
>>> +   phy-names = "usb2-phy", "usb3-phy";
>>> +   };
>>> +   };
>>> +
>>> +
>>> +   usbhost30: usb@15a0 {
>>> +   compatible = "samsung,exynos5250-dwusb3";
>>> +   clocks = <_fsys CLK_ACLK_USBHOST30>,
>>> +   <_fsys CLK_SCLK_USBHOST30>;
>>> +   clock-names = "usbdrd30", "usbdrd30_susp_clk";
>>> +   assigned-clocks =
>>> +   <_fsys CLK_MOUT_SCLK_USBHOST30_USER>,
>>> +   <_top CLK_MOUT_SCLK_USBHOST30>,
>>> +   <_top CLK_DIV_SCLK_USBHOST30>;
>>> +   assigned-clock-parents =
>>> +   <_top CLK_SCLK_USBHOST30_FSYS>,
>>> +   <_top CLK_MOUT_BUS_PLL_USER>;
>>> +   assigned-clock-rates = <0>, <0>, <6670>;
>>> +   #address-cells = <1>;
>>> +   #size-cells = <1>;
>>> +   ranges;
>>> +   status = "disabled";
>>> +
>>
>> Ditto, the node has unit address but no reg property.
> 
> This case is same with above 'usbdrd30'.
>

Same comment than above. The DT shouldn't leak Linux implementation details.

>>
>>> +   usbdrd_dwc3_0: dwc3 {
>>> +   compatible = "snps,dwc3";
>>> +   reg = <0x15a0 0x1>;
>>
>> Ditto, the node doesn't have a unit address but has reg property.
> 
> I'll modify it as following:
>   usbdrd_dwc3_0: dwc3@15a0 {
> 
>>
>>> +   interrupts = <0 244 0>;
>>> +  

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Chanwoo Choi]

Hi Javier,

On 2016년 08월 27일 02:49, Javier Martinez Canillas wrote:
> Hello Chanwoo,
> 
> The patch looks good to me, I just have a few minor comments:
> 
> On 08/24/2016 09:49 AM, Chanwoo Choi wrote:
> 
> [snip]
> 
>> +
>> +reboot: syscon-reboot {
>> +compatible = "syscon-reboot";
>> +regmap = <_system_controller>;
>> +offset = <0x400>;
> 
> Maybe adding a comment that the register is SWRESET to be consistent with the
> arm32 Exynos node definition in arch/arm/boot/dts/exynos-syscon-restart.dtsi?

OK. I'll add a comment as following:
offset = <0x400>; /* SWRESET */

> 
> [snip]
> 
>> +
>> +tmu_atlas0: tmu@1006 {
>> +compatible = "samsung,exynos5433-tmu";
>> +reg = <0x1006 0x200>;
>> +interrupts = <0 95 0>;
> 
> For some interrupts definitions, you are using the GIC interrupt type macros
> while for others like this you are using the constant numbers. I think would
> be better to use the macros consistently (i.e: interrupts = ).

OK. I'll use the GIC_SPI at all of them.

> 
> [snip]
> 
>> +mipi_phy: video-phy@105C0708 {
>> +compatible = "samsung,exynos5433-mipi-video-phy";
>> +#phy-cells = <1>;
>> +samsung,pmu-syscon = <_system_controller>;
>> +samsung,cam0-sysreg = <_cam0>;
>> +samsung,cam1-sysreg = <_cam1>;
>> +samsung,disp-sysreg = <_disp>;
>> +};
> 
> This node has a unit name but it doesn't have a reg property so dtc will
> warn about the mismatch when executed with W=1.

The mipi_phy address are included in the PMU (base 0x105c_).
There is no memory map for only MIPI_PHY. Instead, the mipi_phy driver[1]
uses the regmap (pmu, syscon_cam0/1/disp).
[1] drivers/phy/phy-exynos-mipi-video.c don't use

Also, the first MIPI_PHY's address is 0x105c0710. I'll fix it.

If you want to add the 'reg' property, I can add DT node as following:

mipi_phy: video-phy@105c0710 {
reg = <0x105c0710 0x0>;
compatible = "samsung,exynos5433-mipi-video-phy";
...
};

But it looks strange because the mipi_phy driver[1] never gets
the base address with of_iomap().

> 
> [snip]
> 
>> +usbdrd30: usb@1540 {
>> +compatible = "samsung,exynos5250-dwusb3";
>> +clocks = <_fsys CLK_ACLK_USBDRD30>,
>> +<_fsys CLK_SCLK_USBDRD30>;
>> +clock-names = "usbdrd30", "usbdrd30_susp_clk";
>> +assigned-clocks =
>> +<_fsys CLK_MOUT_SCLK_USBDRD30_USER>,
>> +<_top CLK_MOUT_SCLK_USBDRD30>,
>> +<_top CLK_DIV_SCLK_USBDRD30>;
>> +assigned-clock-parents =
>> +<_top CLK_SCLK_USBDRD30_FSYS>,
>> +<_top CLK_MOUT_BUS_PLL_USER>;
>> +assigned-clock-rates = <0>, <0>, <6670>;
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges;
>> +status = "disabled";
>> +
> 
> Ditto, the node has unit address but no reg property.

The usbdrd[2] driver don't map the base address.
So, this DT node don't include any code calling the of_iomap().
[2] drivers/usb/dwc3/dwc3-exynos.c

But, the usbdrd's base address is 0x1540.

> 
>> +dwc3 {
>> +compatible = "snps,dwc3";
>> +reg = <0x1540 0x1>;
> 
> Ditto, the node doesn't have a unit address but has reg property.

OK. I'll modify it as following:
dwc3@1540 {

> 
>> +interrupts = <0 231 0>;
>> +phys = <_phy 0>, <_phy 1>;
>> +phy-names = "usb2-phy", "usb3-phy";
>> +};
>> +};
>> +
>> +
>> +usbhost30: usb@15a0 {
>> +compatible = "samsung,exynos5250-dwusb3";
>> +clocks = <_fsys CLK_ACLK_USBHOST30>,
>> +<_fsys CLK_SCLK_USBHOST30>;
>> +clock-names = "usbdrd30", "usbdrd30_susp_clk";
>> +assigned-clocks =
>> +<_fsys CLK_MOUT_SCLK_USBHOST30_USER>,
>> +<_top CLK_MOUT_SCLK_USBHOST30>,
>> +<_top CLK_DIV_SCLK_USBHOST30>;
>> +assigned-clock-parents =
>> +<_top CLK_SCLK_USBHOST30_FSYS>,
>> +<_top CLK_MOUT_BUS_PLL_USER>;
>> +assigned-clock-rates = <0>, <0>, <6670>;
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges;
>> + 

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Chanwoo Choi]

Hi Javier,

On 2016년 08월 27일 02:49, Javier Martinez Canillas wrote:
> Hello Chanwoo,
> 
> The patch looks good to me, I just have a few minor comments:
> 
> On 08/24/2016 09:49 AM, Chanwoo Choi wrote:
> 
> [snip]
> 
>> +
>> +reboot: syscon-reboot {
>> +compatible = "syscon-reboot";
>> +regmap = <_system_controller>;
>> +offset = <0x400>;
> 
> Maybe adding a comment that the register is SWRESET to be consistent with the
> arm32 Exynos node definition in arch/arm/boot/dts/exynos-syscon-restart.dtsi?

OK. I'll add a comment as following:
offset = <0x400>; /* SWRESET */

> 
> [snip]
> 
>> +
>> +tmu_atlas0: tmu@1006 {
>> +compatible = "samsung,exynos5433-tmu";
>> +reg = <0x1006 0x200>;
>> +interrupts = <0 95 0>;
> 
> For some interrupts definitions, you are using the GIC interrupt type macros
> while for others like this you are using the constant numbers. I think would
> be better to use the macros consistently (i.e: interrupts = ).

OK. I'll use the GIC_SPI at all of them.

> 
> [snip]
> 
>> +mipi_phy: video-phy@105C0708 {
>> +compatible = "samsung,exynos5433-mipi-video-phy";
>> +#phy-cells = <1>;
>> +samsung,pmu-syscon = <_system_controller>;
>> +samsung,cam0-sysreg = <_cam0>;
>> +samsung,cam1-sysreg = <_cam1>;
>> +samsung,disp-sysreg = <_disp>;
>> +};
> 
> This node has a unit name but it doesn't have a reg property so dtc will
> warn about the mismatch when executed with W=1.

The mipi_phy address are included in the PMU (base 0x105c_).
There is no memory map for only MIPI_PHY. Instead, the mipi_phy driver[1]
uses the regmap (pmu, syscon_cam0/1/disp).
[1] drivers/phy/phy-exynos-mipi-video.c don't use

Also, the first MIPI_PHY's address is 0x105c0710. I'll fix it.

If you want to add the 'reg' property, I can add DT node as following:

mipi_phy: video-phy@105c0710 {
reg = <0x105c0710 0x0>;
compatible = "samsung,exynos5433-mipi-video-phy";
...
};

But it looks strange because the mipi_phy driver[1] never gets
the base address with of_iomap().

> 
> [snip]
> 
>> +usbdrd30: usb@1540 {
>> +compatible = "samsung,exynos5250-dwusb3";
>> +clocks = <_fsys CLK_ACLK_USBDRD30>,
>> +<_fsys CLK_SCLK_USBDRD30>;
>> +clock-names = "usbdrd30", "usbdrd30_susp_clk";
>> +assigned-clocks =
>> +<_fsys CLK_MOUT_SCLK_USBDRD30_USER>,
>> +<_top CLK_MOUT_SCLK_USBDRD30>,
>> +<_top CLK_DIV_SCLK_USBDRD30>;
>> +assigned-clock-parents =
>> +<_top CLK_SCLK_USBDRD30_FSYS>,
>> +<_top CLK_MOUT_BUS_PLL_USER>;
>> +assigned-clock-rates = <0>, <0>, <6670>;
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges;
>> +status = "disabled";
>> +
> 
> Ditto, the node has unit address but no reg property.

The usbdrd[2] driver don't map the base address.
So, this DT node don't include any code calling the of_iomap().
[2] drivers/usb/dwc3/dwc3-exynos.c

But, the usbdrd's base address is 0x1540.

> 
>> +dwc3 {
>> +compatible = "snps,dwc3";
>> +reg = <0x1540 0x1>;
> 
> Ditto, the node doesn't have a unit address but has reg property.

OK. I'll modify it as following:
dwc3@1540 {

> 
>> +interrupts = <0 231 0>;
>> +phys = <_phy 0>, <_phy 1>;
>> +phy-names = "usb2-phy", "usb3-phy";
>> +};
>> +};
>> +
>> +
>> +usbhost30: usb@15a0 {
>> +compatible = "samsung,exynos5250-dwusb3";
>> +clocks = <_fsys CLK_ACLK_USBHOST30>,
>> +<_fsys CLK_SCLK_USBHOST30>;
>> +clock-names = "usbdrd30", "usbdrd30_susp_clk";
>> +assigned-clocks =
>> +<_fsys CLK_MOUT_SCLK_USBHOST30_USER>,
>> +<_top CLK_MOUT_SCLK_USBHOST30>,
>> +<_top CLK_DIV_SCLK_USBHOST30>;
>> +assigned-clock-parents =
>> +<_top CLK_SCLK_USBHOST30_FSYS>,
>> +<_top CLK_MOUT_BUS_PLL_USER>;
>> +assigned-clock-rates = <0>, <0>, <6670>;
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges;
>> + 

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Chanwoo Choi]

On 2016년 08월 27일 01:14, Krzysztof Kozlowski wrote:
> On Wed, Aug 24, 2016 at 10:49:09PM +0900, Chanwoo Choi wrote:
>> This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on
>> Octa-core CPUs (quad Cortex-A57 and quad Cortex-A53). And Exynos5433 supports
>> PSCI (Power State Coordination Interface) v0.1.
>>
>> This patch includes following Device Tree node to support Exynos5433 SoC:
>> 1. Octa cores for big.LITTLE architecture
>> - Cortex-A53 LITTLE Quad-core
>> - Cortex-A57 big Quad-core
>> - Support PSCI v0.1
>>
>> 2. Clock controller node
>> - CMU_TOP   : clocks for IMEM/FSYS/G3D/GSCL/HEVC/MSCL/G2D/MFC/PERIC/PERIS
>> - CMU_CPIF  : clocks for LLI (Low Latency Interface)
>> - CMU_MIF   : clocks for DRAM Memory Controller
>> - CMU_PERIC : clocks for UART/I2C/SPI/I2S/PCM/SPDIF/PWM/SLIMBUS
>> - CMU_PERIS : clocks for PMU/TMU/MCT/WDT/RTC/SECKEY/TZPC
>> - CMU_FSYS  : clocks for USB/UFS/SDMMC/TSI/PDMA
>> - CMU_G2D   : clocks for G2D/MDMA
>> - CMU_DISP  : clocks for DECON/HDMI/DSIM/MIXER
>> - CMU_AUD   : clocks for Cortex-A5/BUS/AUDIO
>> - CMU_BUS{0|1|2} : clocks for global data buses and global peripheral buses
>> - CMU_G3D   : clocks for 3D Graphics Engine
>> - CMU_GSCL  : clocks for GSCALER
>> - CMU_APOLLO: clocks for Cortex-A53 Quad-core processor.
>> - CMU_ATLAS : clocks for Cortex-A57 Quad-core processor,
>>   CoreSight and L2 cache controller.
>> - CMU_MSCL  : clocks for M2M (Memory to Memory) scaler and JPEG IPs.
>> - CMU_MFC   : clocks for MFC (Multi-Format Codec) IP.
>> - CMU_HEVC  : clocks for HEVC(High Efficiency Video Codec) decoder IP.
>> - CMU_ISP   : clocks for FIMC-ISP/DRC/SCLC/DIS/3DNR IPs.
>> - CMU_CAM0  : clocks for MIPI_CSIS{0|1}/FIMC_LITE_{A|B|D}/FIMC_3AA{0|1} IPs.
>> - CMU_CAM1  : clocks for COrtex-A5/MIPI_CSIS2/FIMC_LITE_C/FIMC-FD IPs.
>>
>> 3. pinctrl node for GPIO
>> - alive/aud/cpif/ese/finger/fsys/imem/nfc/peric/touch pad
>>
>> 4. Timer
>> - ARM architecture timer (armv8-timer)
>> - MCT (Multi Core Timer) timer
>>
>> 5. Interrupt controller (GIC-400)
>>
>> 6. BUS devices
>> - HS-I2C (High-Speed I2C) device
>> - SPI (Serial Peripheral Interface) device
>>
>> 7. Sound devices
>> - I2S bus
>> - LPASS (Low Power Audio Subsystem)
>>
>> 8. Power management devices
>> - CPUFREQ for for Cortex-A53/A57
>> - TMU (Thermal Management Unit) for Cortex-A53/A57, G3D, ISP
>>
>> 9. Display controller devices
>> - DECON (Display and enhancement controller) for panel output
>> - DSI (Display Serial Interface)
>> - MIC (Mobile Image Compressor)
>>
>> 10. USB
>> - USB 3.0 DRD (Dual Role Device) controller
>> - USB 3.0 Host controller
>>
>> 11. Storage devices
>> - MSHC (Mobile Stoarage Host Controller)
>>
>> 12. Misc devices
>> - UART device
>> - ADC (Analog Digital Converter)
>> - PWM (Pulse Width Modulation)
>> - ADMA (Advanced DMA) and PDMA (Peripheral DMA)
>>
>> Signed-off-by: Chanwoo Choi 
>> Signed-off-by: Jaehoon Chung 
>> Signed-off-by: Seung-Woo Kim 
>> Signed-off-by: Joonyoung Shim 
>> Signed-off-by: Inki Dae 
>> Signed-off-by: Jonghwa Lee 
>> Signed-off-by: Beomho Seo 
>> Signed-off-by: Jaewon Kim 
>> Signed-off-by: Hyungwon Hwang 
>> Signed-off-by: Inha Song 
>> Signed-off-by: Ingi kim 
>> Signed-off-by: Krzysztof Kozlowski 
>> Signed-off-by: Marek Szyprowski 
>> Signed-off-by: Andrzej Hajda 
>> Signed-off-by: Sylwester Nawrocki 
>> ---
>>  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  792 
>>  .../dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi |   23 +
>>  .../dts/exynos/exynos5433-tmu-sensor-conf.dtsi |   22 +
>>  arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi |  306 +
>>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 1327 
>> 
>>  5 files changed, 2470 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
>>  create mode 100644 
>> arch/arm64/boot/dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi
>>  create mode 100644 
>> arch/arm64/boot/dts/exynos/exynos5433-tmu-sensor-conf.dtsi
>>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
>>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi
> 
> I got only one question:
> 
>  +
>> +cooling-maps {
>> +map0 {
>> +/* Set maximum frequency as 1300MHz  */
>> +trip = <_alert_0>;
>> +cooling-device = < 0 0>;
> 
> Is cooling level=0 really a cooling level? I think it does nothing so it
> should be just removed.

You are right. I'll modify it for big cores as following: 
- cooling-device = < 1 2>;

Also, I'll use the only 5 step 

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Chanwoo Choi]

On 2016년 08월 27일 01:14, Krzysztof Kozlowski wrote:
> On Wed, Aug 24, 2016 at 10:49:09PM +0900, Chanwoo Choi wrote:
>> This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on
>> Octa-core CPUs (quad Cortex-A57 and quad Cortex-A53). And Exynos5433 supports
>> PSCI (Power State Coordination Interface) v0.1.
>>
>> This patch includes following Device Tree node to support Exynos5433 SoC:
>> 1. Octa cores for big.LITTLE architecture
>> - Cortex-A53 LITTLE Quad-core
>> - Cortex-A57 big Quad-core
>> - Support PSCI v0.1
>>
>> 2. Clock controller node
>> - CMU_TOP   : clocks for IMEM/FSYS/G3D/GSCL/HEVC/MSCL/G2D/MFC/PERIC/PERIS
>> - CMU_CPIF  : clocks for LLI (Low Latency Interface)
>> - CMU_MIF   : clocks for DRAM Memory Controller
>> - CMU_PERIC : clocks for UART/I2C/SPI/I2S/PCM/SPDIF/PWM/SLIMBUS
>> - CMU_PERIS : clocks for PMU/TMU/MCT/WDT/RTC/SECKEY/TZPC
>> - CMU_FSYS  : clocks for USB/UFS/SDMMC/TSI/PDMA
>> - CMU_G2D   : clocks for G2D/MDMA
>> - CMU_DISP  : clocks for DECON/HDMI/DSIM/MIXER
>> - CMU_AUD   : clocks for Cortex-A5/BUS/AUDIO
>> - CMU_BUS{0|1|2} : clocks for global data buses and global peripheral buses
>> - CMU_G3D   : clocks for 3D Graphics Engine
>> - CMU_GSCL  : clocks for GSCALER
>> - CMU_APOLLO: clocks for Cortex-A53 Quad-core processor.
>> - CMU_ATLAS : clocks for Cortex-A57 Quad-core processor,
>>   CoreSight and L2 cache controller.
>> - CMU_MSCL  : clocks for M2M (Memory to Memory) scaler and JPEG IPs.
>> - CMU_MFC   : clocks for MFC (Multi-Format Codec) IP.
>> - CMU_HEVC  : clocks for HEVC(High Efficiency Video Codec) decoder IP.
>> - CMU_ISP   : clocks for FIMC-ISP/DRC/SCLC/DIS/3DNR IPs.
>> - CMU_CAM0  : clocks for MIPI_CSIS{0|1}/FIMC_LITE_{A|B|D}/FIMC_3AA{0|1} IPs.
>> - CMU_CAM1  : clocks for COrtex-A5/MIPI_CSIS2/FIMC_LITE_C/FIMC-FD IPs.
>>
>> 3. pinctrl node for GPIO
>> - alive/aud/cpif/ese/finger/fsys/imem/nfc/peric/touch pad
>>
>> 4. Timer
>> - ARM architecture timer (armv8-timer)
>> - MCT (Multi Core Timer) timer
>>
>> 5. Interrupt controller (GIC-400)
>>
>> 6. BUS devices
>> - HS-I2C (High-Speed I2C) device
>> - SPI (Serial Peripheral Interface) device
>>
>> 7. Sound devices
>> - I2S bus
>> - LPASS (Low Power Audio Subsystem)
>>
>> 8. Power management devices
>> - CPUFREQ for for Cortex-A53/A57
>> - TMU (Thermal Management Unit) for Cortex-A53/A57, G3D, ISP
>>
>> 9. Display controller devices
>> - DECON (Display and enhancement controller) for panel output
>> - DSI (Display Serial Interface)
>> - MIC (Mobile Image Compressor)
>>
>> 10. USB
>> - USB 3.0 DRD (Dual Role Device) controller
>> - USB 3.0 Host controller
>>
>> 11. Storage devices
>> - MSHC (Mobile Stoarage Host Controller)
>>
>> 12. Misc devices
>> - UART device
>> - ADC (Analog Digital Converter)
>> - PWM (Pulse Width Modulation)
>> - ADMA (Advanced DMA) and PDMA (Peripheral DMA)
>>
>> Signed-off-by: Chanwoo Choi 
>> Signed-off-by: Jaehoon Chung 
>> Signed-off-by: Seung-Woo Kim 
>> Signed-off-by: Joonyoung Shim 
>> Signed-off-by: Inki Dae 
>> Signed-off-by: Jonghwa Lee 
>> Signed-off-by: Beomho Seo 
>> Signed-off-by: Jaewon Kim 
>> Signed-off-by: Hyungwon Hwang 
>> Signed-off-by: Inha Song 
>> Signed-off-by: Ingi kim 
>> Signed-off-by: Krzysztof Kozlowski 
>> Signed-off-by: Marek Szyprowski 
>> Signed-off-by: Andrzej Hajda 
>> Signed-off-by: Sylwester Nawrocki 
>> ---
>>  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  792 
>>  .../dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi |   23 +
>>  .../dts/exynos/exynos5433-tmu-sensor-conf.dtsi |   22 +
>>  arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi |  306 +
>>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 1327 
>> 
>>  5 files changed, 2470 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
>>  create mode 100644 
>> arch/arm64/boot/dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi
>>  create mode 100644 
>> arch/arm64/boot/dts/exynos/exynos5433-tmu-sensor-conf.dtsi
>>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
>>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi
> 
> I got only one question:
> 
>  +
>> +cooling-maps {
>> +map0 {
>> +/* Set maximum frequency as 1300MHz  */
>> +trip = <_alert_0>;
>> +cooling-device = < 0 0>;
> 
> Is cooling level=0 really a cooling level? I think it does nothing so it
> should be just removed.

You are right. I'll modify it for big cores as following: 
- cooling-device = < 1 2>;

Also, I'll use the only 5 step cooling level instead of 7 step.
for LITTLE cores.


For big cores:
cooling-maps {
map0 {
/* Set maximum frequency as 1800MHz  */
trip = <_alert_0>;
cooling-device = < 1 2>;
};
map1 {
/* Set maximum frequency as 

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Javier Martinez Canillas]

Hello Chanwoo,

The patch looks good to me, I just have a few minor comments:

On 08/24/2016 09:49 AM, Chanwoo Choi wrote:

[snip]

> +
> + reboot: syscon-reboot {
> + compatible = "syscon-reboot";
> + regmap = <_system_controller>;
> + offset = <0x400>;

Maybe adding a comment that the register is SWRESET to be consistent with the
arm32 Exynos node definition in arch/arm/boot/dts/exynos-syscon-restart.dtsi?

[snip]

> +
> + tmu_atlas0: tmu@1006 {
> + compatible = "samsung,exynos5433-tmu";
> + reg = <0x1006 0x200>;
> + interrupts = <0 95 0>;

For some interrupts definitions, you are using the GIC interrupt type macros
while for others like this you are using the constant numbers. I think would
be better to use the macros consistently (i.e: interrupts = ).

[snip]

> + mipi_phy: video-phy@105C0708 {
> + compatible = "samsung,exynos5433-mipi-video-phy";
> + #phy-cells = <1>;
> + samsung,pmu-syscon = <_system_controller>;
> + samsung,cam0-sysreg = <_cam0>;
> + samsung,cam1-sysreg = <_cam1>;
> + samsung,disp-sysreg = <_disp>;
> + };

This node has a unit name but it doesn't have a reg property so dtc will
warn about the mismatch when executed with W=1.

[snip]

> + usbdrd30: usb@1540 {
> + compatible = "samsung,exynos5250-dwusb3";
> + clocks = <_fsys CLK_ACLK_USBDRD30>,
> + <_fsys CLK_SCLK_USBDRD30>;
> + clock-names = "usbdrd30", "usbdrd30_susp_clk";
> + assigned-clocks =
> + <_fsys CLK_MOUT_SCLK_USBDRD30_USER>,
> + <_top CLK_MOUT_SCLK_USBDRD30>,
> + <_top CLK_DIV_SCLK_USBDRD30>;
> + assigned-clock-parents =
> + <_top CLK_SCLK_USBDRD30_FSYS>,
> + <_top CLK_MOUT_BUS_PLL_USER>;
> + assigned-clock-rates = <0>, <0>, <6670>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + status = "disabled";
> +

Ditto, the node has unit address but no reg property.

> + dwc3 {
> + compatible = "snps,dwc3";
> + reg = <0x1540 0x1>;

Ditto, the node doesn't have a unit address but has reg property.

> + interrupts = <0 231 0>;
> + phys = <_phy 0>, <_phy 1>;
> + phy-names = "usb2-phy", "usb3-phy";
> + };
> + };
> +
> +
> + usbhost30: usb@15a0 {
> + compatible = "samsung,exynos5250-dwusb3";
> + clocks = <_fsys CLK_ACLK_USBHOST30>,
> + <_fsys CLK_SCLK_USBHOST30>;
> + clock-names = "usbdrd30", "usbdrd30_susp_clk";
> + assigned-clocks =
> + <_fsys CLK_MOUT_SCLK_USBHOST30_USER>,
> + <_top CLK_MOUT_SCLK_USBHOST30>,
> + <_top CLK_DIV_SCLK_USBHOST30>;
> + assigned-clock-parents =
> + <_top CLK_SCLK_USBHOST30_FSYS>,
> + <_top CLK_MOUT_BUS_PLL_USER>;
> + assigned-clock-rates = <0>, <0>, <6670>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + status = "disabled";
> +

Ditto, the node has unit address but no reg property.

> + usbdrd_dwc3_0: dwc3 {
> + compatible = "snps,dwc3";
> + reg = <0x15a0 0x1>;

Ditto, the node doesn't have a unit address but has reg property.

> + interrupts = <0 244 0>;
> + phys = <_phy 0>, <_phy 1>;
> + phy-names = "usb2-phy", "usb3-phy";
> + };
> + };
> +

[snip]

> + audio-subsystem {
> + compatible = "samsung,exynos5433-lpass";
> + reg = <0x1140 0x100>, <0x1150 0x08>;

Ditto, the node doesn't have a unit address but has reg property.

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Javier Martinez Canillas]

Hello Chanwoo,

The patch looks good to me, I just have a few minor comments:

On 08/24/2016 09:49 AM, Chanwoo Choi wrote:

[snip]

> +
> + reboot: syscon-reboot {
> + compatible = "syscon-reboot";
> + regmap = <_system_controller>;
> + offset = <0x400>;

Maybe adding a comment that the register is SWRESET to be consistent with the
arm32 Exynos node definition in arch/arm/boot/dts/exynos-syscon-restart.dtsi?

[snip]

> +
> + tmu_atlas0: tmu@1006 {
> + compatible = "samsung,exynos5433-tmu";
> + reg = <0x1006 0x200>;
> + interrupts = <0 95 0>;

For some interrupts definitions, you are using the GIC interrupt type macros
while for others like this you are using the constant numbers. I think would
be better to use the macros consistently (i.e: interrupts = ).

[snip]

> + mipi_phy: video-phy@105C0708 {
> + compatible = "samsung,exynos5433-mipi-video-phy";
> + #phy-cells = <1>;
> + samsung,pmu-syscon = <_system_controller>;
> + samsung,cam0-sysreg = <_cam0>;
> + samsung,cam1-sysreg = <_cam1>;
> + samsung,disp-sysreg = <_disp>;
> + };

This node has a unit name but it doesn't have a reg property so dtc will
warn about the mismatch when executed with W=1.

[snip]

> + usbdrd30: usb@1540 {
> + compatible = "samsung,exynos5250-dwusb3";
> + clocks = <_fsys CLK_ACLK_USBDRD30>,
> + <_fsys CLK_SCLK_USBDRD30>;
> + clock-names = "usbdrd30", "usbdrd30_susp_clk";
> + assigned-clocks =
> + <_fsys CLK_MOUT_SCLK_USBDRD30_USER>,
> + <_top CLK_MOUT_SCLK_USBDRD30>,
> + <_top CLK_DIV_SCLK_USBDRD30>;
> + assigned-clock-parents =
> + <_top CLK_SCLK_USBDRD30_FSYS>,
> + <_top CLK_MOUT_BUS_PLL_USER>;
> + assigned-clock-rates = <0>, <0>, <6670>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + status = "disabled";
> +

Ditto, the node has unit address but no reg property.

> + dwc3 {
> + compatible = "snps,dwc3";
> + reg = <0x1540 0x1>;

Ditto, the node doesn't have a unit address but has reg property.

> + interrupts = <0 231 0>;
> + phys = <_phy 0>, <_phy 1>;
> + phy-names = "usb2-phy", "usb3-phy";
> + };
> + };
> +
> +
> + usbhost30: usb@15a0 {
> + compatible = "samsung,exynos5250-dwusb3";
> + clocks = <_fsys CLK_ACLK_USBHOST30>,
> + <_fsys CLK_SCLK_USBHOST30>;
> + clock-names = "usbdrd30", "usbdrd30_susp_clk";
> + assigned-clocks =
> + <_fsys CLK_MOUT_SCLK_USBHOST30_USER>,
> + <_top CLK_MOUT_SCLK_USBHOST30>,
> + <_top CLK_DIV_SCLK_USBHOST30>;
> + assigned-clock-parents =
> + <_top CLK_SCLK_USBHOST30_FSYS>,
> + <_top CLK_MOUT_BUS_PLL_USER>;
> + assigned-clock-rates = <0>, <0>, <6670>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + status = "disabled";
> +

Ditto, the node has unit address but no reg property.

> + usbdrd_dwc3_0: dwc3 {
> + compatible = "snps,dwc3";
> + reg = <0x15a0 0x1>;

Ditto, the node doesn't have a unit address but has reg property.

> + interrupts = <0 244 0>;
> + phys = <_phy 0>, <_phy 1>;
> + phy-names = "usb2-phy", "usb3-phy";
> + };
> + };
> +

[snip]

> + audio-subsystem {
> + compatible = "samsung,exynos5433-lpass";
> + reg = <0x1140 0x100>, <0x1150 0x08>;

Ditto, the node doesn't have a unit address but has reg property.

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Krzysztof Kozlowski]

On Wed, Aug 24, 2016 at 10:49:09PM +0900, Chanwoo Choi wrote:
> This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on
> Octa-core CPUs (quad Cortex-A57 and quad Cortex-A53). And Exynos5433 supports
> PSCI (Power State Coordination Interface) v0.1.
> 
> This patch includes following Device Tree node to support Exynos5433 SoC:
> 1. Octa cores for big.LITTLE architecture
> - Cortex-A53 LITTLE Quad-core
> - Cortex-A57 big Quad-core
> - Support PSCI v0.1
> 
> 2. Clock controller node
> - CMU_TOP   : clocks for IMEM/FSYS/G3D/GSCL/HEVC/MSCL/G2D/MFC/PERIC/PERIS
> - CMU_CPIF  : clocks for LLI (Low Latency Interface)
> - CMU_MIF   : clocks for DRAM Memory Controller
> - CMU_PERIC : clocks for UART/I2C/SPI/I2S/PCM/SPDIF/PWM/SLIMBUS
> - CMU_PERIS : clocks for PMU/TMU/MCT/WDT/RTC/SECKEY/TZPC
> - CMU_FSYS  : clocks for USB/UFS/SDMMC/TSI/PDMA
> - CMU_G2D   : clocks for G2D/MDMA
> - CMU_DISP  : clocks for DECON/HDMI/DSIM/MIXER
> - CMU_AUD   : clocks for Cortex-A5/BUS/AUDIO
> - CMU_BUS{0|1|2} : clocks for global data buses and global peripheral buses
> - CMU_G3D   : clocks for 3D Graphics Engine
> - CMU_GSCL  : clocks for GSCALER
> - CMU_APOLLO: clocks for Cortex-A53 Quad-core processor.
> - CMU_ATLAS : clocks for Cortex-A57 Quad-core processor,
>   CoreSight and L2 cache controller.
> - CMU_MSCL  : clocks for M2M (Memory to Memory) scaler and JPEG IPs.
> - CMU_MFC   : clocks for MFC (Multi-Format Codec) IP.
> - CMU_HEVC  : clocks for HEVC(High Efficiency Video Codec) decoder IP.
> - CMU_ISP   : clocks for FIMC-ISP/DRC/SCLC/DIS/3DNR IPs.
> - CMU_CAM0  : clocks for MIPI_CSIS{0|1}/FIMC_LITE_{A|B|D}/FIMC_3AA{0|1} IPs.
> - CMU_CAM1  : clocks for COrtex-A5/MIPI_CSIS2/FIMC_LITE_C/FIMC-FD IPs.
> 
> 3. pinctrl node for GPIO
> - alive/aud/cpif/ese/finger/fsys/imem/nfc/peric/touch pad
> 
> 4. Timer
> - ARM architecture timer (armv8-timer)
> - MCT (Multi Core Timer) timer
> 
> 5. Interrupt controller (GIC-400)
> 
> 6. BUS devices
> - HS-I2C (High-Speed I2C) device
> - SPI (Serial Peripheral Interface) device
> 
> 7. Sound devices
> - I2S bus
> - LPASS (Low Power Audio Subsystem)
> 
> 8. Power management devices
> - CPUFREQ for for Cortex-A53/A57
> - TMU (Thermal Management Unit) for Cortex-A53/A57, G3D, ISP
> 
> 9. Display controller devices
> - DECON (Display and enhancement controller) for panel output
> - DSI (Display Serial Interface)
> - MIC (Mobile Image Compressor)
> 
> 10. USB
> - USB 3.0 DRD (Dual Role Device) controller
> - USB 3.0 Host controller
> 
> 11. Storage devices
> - MSHC (Mobile Stoarage Host Controller)
> 
> 12. Misc devices
> - UART device
> - ADC (Analog Digital Converter)
> - PWM (Pulse Width Modulation)
> - ADMA (Advanced DMA) and PDMA (Peripheral DMA)
> 
> Signed-off-by: Chanwoo Choi 
> Signed-off-by: Jaehoon Chung 
> Signed-off-by: Seung-Woo Kim 
> Signed-off-by: Joonyoung Shim 
> Signed-off-by: Inki Dae 
> Signed-off-by: Jonghwa Lee 
> Signed-off-by: Beomho Seo 
> Signed-off-by: Jaewon Kim 
> Signed-off-by: Hyungwon Hwang 
> Signed-off-by: Inha Song 
> Signed-off-by: Ingi kim 
> Signed-off-by: Krzysztof Kozlowski 
> Signed-off-by: Marek Szyprowski 
> Signed-off-by: Andrzej Hajda 
> Signed-off-by: Sylwester Nawrocki 
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  792 
>  .../dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi |   23 +
>  .../dts/exynos/exynos5433-tmu-sensor-conf.dtsi |   22 +
>  arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi |  306 +
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 1327 
> 
>  5 files changed, 2470 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
>  create mode 100644 
> arch/arm64/boot/dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi
>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tmu-sensor-conf.dtsi
>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi

I got only one question:

 +
> + cooling-maps {
> + map0 {
> + /* Set maximum frequency as 1300MHz  */
> + trip = <_alert_0>;
> + cooling-device = < 0 0>;

Is cooling level=0 really a cooling level? I think it does nothing so it
should be just removed.

The same for big cluster.

Rest looks good.

BR,
Krzysztof

Re: [PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Krzysztof Kozlowski]

On Wed, Aug 24, 2016 at 10:49:09PM +0900, Chanwoo Choi wrote:
> This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on
> Octa-core CPUs (quad Cortex-A57 and quad Cortex-A53). And Exynos5433 supports
> PSCI (Power State Coordination Interface) v0.1.
> 
> This patch includes following Device Tree node to support Exynos5433 SoC:
> 1. Octa cores for big.LITTLE architecture
> - Cortex-A53 LITTLE Quad-core
> - Cortex-A57 big Quad-core
> - Support PSCI v0.1
> 
> 2. Clock controller node
> - CMU_TOP   : clocks for IMEM/FSYS/G3D/GSCL/HEVC/MSCL/G2D/MFC/PERIC/PERIS
> - CMU_CPIF  : clocks for LLI (Low Latency Interface)
> - CMU_MIF   : clocks for DRAM Memory Controller
> - CMU_PERIC : clocks for UART/I2C/SPI/I2S/PCM/SPDIF/PWM/SLIMBUS
> - CMU_PERIS : clocks for PMU/TMU/MCT/WDT/RTC/SECKEY/TZPC
> - CMU_FSYS  : clocks for USB/UFS/SDMMC/TSI/PDMA
> - CMU_G2D   : clocks for G2D/MDMA
> - CMU_DISP  : clocks for DECON/HDMI/DSIM/MIXER
> - CMU_AUD   : clocks for Cortex-A5/BUS/AUDIO
> - CMU_BUS{0|1|2} : clocks for global data buses and global peripheral buses
> - CMU_G3D   : clocks for 3D Graphics Engine
> - CMU_GSCL  : clocks for GSCALER
> - CMU_APOLLO: clocks for Cortex-A53 Quad-core processor.
> - CMU_ATLAS : clocks for Cortex-A57 Quad-core processor,
>   CoreSight and L2 cache controller.
> - CMU_MSCL  : clocks for M2M (Memory to Memory) scaler and JPEG IPs.
> - CMU_MFC   : clocks for MFC (Multi-Format Codec) IP.
> - CMU_HEVC  : clocks for HEVC(High Efficiency Video Codec) decoder IP.
> - CMU_ISP   : clocks for FIMC-ISP/DRC/SCLC/DIS/3DNR IPs.
> - CMU_CAM0  : clocks for MIPI_CSIS{0|1}/FIMC_LITE_{A|B|D}/FIMC_3AA{0|1} IPs.
> - CMU_CAM1  : clocks for COrtex-A5/MIPI_CSIS2/FIMC_LITE_C/FIMC-FD IPs.
> 
> 3. pinctrl node for GPIO
> - alive/aud/cpif/ese/finger/fsys/imem/nfc/peric/touch pad
> 
> 4. Timer
> - ARM architecture timer (armv8-timer)
> - MCT (Multi Core Timer) timer
> 
> 5. Interrupt controller (GIC-400)
> 
> 6. BUS devices
> - HS-I2C (High-Speed I2C) device
> - SPI (Serial Peripheral Interface) device
> 
> 7. Sound devices
> - I2S bus
> - LPASS (Low Power Audio Subsystem)
> 
> 8. Power management devices
> - CPUFREQ for for Cortex-A53/A57
> - TMU (Thermal Management Unit) for Cortex-A53/A57, G3D, ISP
> 
> 9. Display controller devices
> - DECON (Display and enhancement controller) for panel output
> - DSI (Display Serial Interface)
> - MIC (Mobile Image Compressor)
> 
> 10. USB
> - USB 3.0 DRD (Dual Role Device) controller
> - USB 3.0 Host controller
> 
> 11. Storage devices
> - MSHC (Mobile Stoarage Host Controller)
> 
> 12. Misc devices
> - UART device
> - ADC (Analog Digital Converter)
> - PWM (Pulse Width Modulation)
> - ADMA (Advanced DMA) and PDMA (Peripheral DMA)
> 
> Signed-off-by: Chanwoo Choi 
> Signed-off-by: Jaehoon Chung 
> Signed-off-by: Seung-Woo Kim 
> Signed-off-by: Joonyoung Shim 
> Signed-off-by: Inki Dae 
> Signed-off-by: Jonghwa Lee 
> Signed-off-by: Beomho Seo 
> Signed-off-by: Jaewon Kim 
> Signed-off-by: Hyungwon Hwang 
> Signed-off-by: Inha Song 
> Signed-off-by: Ingi kim 
> Signed-off-by: Krzysztof Kozlowski 
> Signed-off-by: Marek Szyprowski 
> Signed-off-by: Andrzej Hajda 
> Signed-off-by: Sylwester Nawrocki 
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  792 
>  .../dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi |   23 +
>  .../dts/exynos/exynos5433-tmu-sensor-conf.dtsi |   22 +
>  arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi |  306 +
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 1327 
> 
>  5 files changed, 2470 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
>  create mode 100644 
> arch/arm64/boot/dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi
>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tmu-sensor-conf.dtsi
>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi

I got only one question:

 +
> + cooling-maps {
> + map0 {
> + /* Set maximum frequency as 1300MHz  */
> + trip = <_alert_0>;
> + cooling-device = < 0 0>;

Is cooling level=0 really a cooling level? I think it does nothing so it
should be just removed.

The same for big cluster.

Rest looks good.

BR,
Krzysztof

[PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Chanwoo Choi]

This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on
Octa-core CPUs (quad Cortex-A57 and quad Cortex-A53). And Exynos5433 supports
PSCI (Power State Coordination Interface) v0.1.

This patch includes following Device Tree node to support Exynos5433 SoC:
1. Octa cores for big.LITTLE architecture
- Cortex-A53 LITTLE Quad-core
- Cortex-A57 big Quad-core
- Support PSCI v0.1

2. Clock controller node
- CMU_TOP   : clocks for IMEM/FSYS/G3D/GSCL/HEVC/MSCL/G2D/MFC/PERIC/PERIS
- CMU_CPIF  : clocks for LLI (Low Latency Interface)
- CMU_MIF   : clocks for DRAM Memory Controller
- CMU_PERIC : clocks for UART/I2C/SPI/I2S/PCM/SPDIF/PWM/SLIMBUS
- CMU_PERIS : clocks for PMU/TMU/MCT/WDT/RTC/SECKEY/TZPC
- CMU_FSYS  : clocks for USB/UFS/SDMMC/TSI/PDMA
- CMU_G2D   : clocks for G2D/MDMA
- CMU_DISP  : clocks for DECON/HDMI/DSIM/MIXER
- CMU_AUD   : clocks for Cortex-A5/BUS/AUDIO
- CMU_BUS{0|1|2} : clocks for global data buses and global peripheral buses
- CMU_G3D   : clocks for 3D Graphics Engine
- CMU_GSCL  : clocks for GSCALER
- CMU_APOLLO: clocks for Cortex-A53 Quad-core processor.
- CMU_ATLAS : clocks for Cortex-A57 Quad-core processor,
  CoreSight and L2 cache controller.
- CMU_MSCL  : clocks for M2M (Memory to Memory) scaler and JPEG IPs.
- CMU_MFC   : clocks for MFC (Multi-Format Codec) IP.
- CMU_HEVC  : clocks for HEVC(High Efficiency Video Codec) decoder IP.
- CMU_ISP   : clocks for FIMC-ISP/DRC/SCLC/DIS/3DNR IPs.
- CMU_CAM0  : clocks for MIPI_CSIS{0|1}/FIMC_LITE_{A|B|D}/FIMC_3AA{0|1} IPs.
- CMU_CAM1  : clocks for COrtex-A5/MIPI_CSIS2/FIMC_LITE_C/FIMC-FD IPs.

3. pinctrl node for GPIO
- alive/aud/cpif/ese/finger/fsys/imem/nfc/peric/touch pad

4. Timer
- ARM architecture timer (armv8-timer)
- MCT (Multi Core Timer) timer

5. Interrupt controller (GIC-400)

6. BUS devices
- HS-I2C (High-Speed I2C) device
- SPI (Serial Peripheral Interface) device

7. Sound devices
- I2S bus
- LPASS (Low Power Audio Subsystem)

8. Power management devices
- CPUFREQ for for Cortex-A53/A57
- TMU (Thermal Management Unit) for Cortex-A53/A57, G3D, ISP

9. Display controller devices
- DECON (Display and enhancement controller) for panel output
- DSI (Display Serial Interface)
- MIC (Mobile Image Compressor)

10. USB
- USB 3.0 DRD (Dual Role Device) controller
- USB 3.0 Host controller

11. Storage devices
- MSHC (Mobile Stoarage Host Controller)

12. Misc devices
- UART device
- ADC (Analog Digital Converter)
- PWM (Pulse Width Modulation)
- ADMA (Advanced DMA) and PDMA (Peripheral DMA)

Signed-off-by: Chanwoo Choi 
Signed-off-by: Jaehoon Chung 
Signed-off-by: Seung-Woo Kim 
Signed-off-by: Joonyoung Shim 
Signed-off-by: Inki Dae 
Signed-off-by: Jonghwa Lee 
Signed-off-by: Beomho Seo 
Signed-off-by: Jaewon Kim 
Signed-off-by: Hyungwon Hwang 
Signed-off-by: Inha Song 
Signed-off-by: Ingi kim 
Signed-off-by: Krzysztof Kozlowski 
Signed-off-by: Marek Szyprowski 
Signed-off-by: Andrzej Hajda 
Signed-off-by: Sylwester Nawrocki 
---
 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  792 
 .../dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi |   23 +
 .../dts/exynos/exynos5433-tmu-sensor-conf.dtsi |   22 +
 arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi |  306 +
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 1327 
 5 files changed, 2470 insertions(+)
 create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
 create mode 100644 
arch/arm64/boot/dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi
 create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tmu-sensor-conf.dtsi
 create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
 create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi 
b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
new file mode 100644
index ..409bfb488b0b
--- /dev/null
+++ b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
@@ -0,0 +1,792 @@
+/*
+ * Samsung's Exynos5433 SoC pin-mux and pin-config device tree source
+ *
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * Chanwoo Choi 
+ *
+ * Samsung's Exynos5433 SoC pin-mux and pin-config options are listed as device
+ * tree nodes are listed in this file.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define PIN_PULL_NONE  0
+#define PIN_PULL_DOWN  1
+#define PIN_PULL_UP3
+
+#define PIN_DRV_LV10
+#define 

[PATCH v2 5/7] arm64: dts: exynos: Add dts files for Samsung Exynos5433 64bit SoC

[Chanwoo Choi]

This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on
Octa-core CPUs (quad Cortex-A57 and quad Cortex-A53). And Exynos5433 supports
PSCI (Power State Coordination Interface) v0.1.

This patch includes following Device Tree node to support Exynos5433 SoC:
1. Octa cores for big.LITTLE architecture
- Cortex-A53 LITTLE Quad-core
- Cortex-A57 big Quad-core
- Support PSCI v0.1

2. Clock controller node
- CMU_TOP   : clocks for IMEM/FSYS/G3D/GSCL/HEVC/MSCL/G2D/MFC/PERIC/PERIS
- CMU_CPIF  : clocks for LLI (Low Latency Interface)
- CMU_MIF   : clocks for DRAM Memory Controller
- CMU_PERIC : clocks for UART/I2C/SPI/I2S/PCM/SPDIF/PWM/SLIMBUS
- CMU_PERIS : clocks for PMU/TMU/MCT/WDT/RTC/SECKEY/TZPC
- CMU_FSYS  : clocks for USB/UFS/SDMMC/TSI/PDMA
- CMU_G2D   : clocks for G2D/MDMA
- CMU_DISP  : clocks for DECON/HDMI/DSIM/MIXER
- CMU_AUD   : clocks for Cortex-A5/BUS/AUDIO
- CMU_BUS{0|1|2} : clocks for global data buses and global peripheral buses
- CMU_G3D   : clocks for 3D Graphics Engine
- CMU_GSCL  : clocks for GSCALER
- CMU_APOLLO: clocks for Cortex-A53 Quad-core processor.
- CMU_ATLAS : clocks for Cortex-A57 Quad-core processor,
  CoreSight and L2 cache controller.
- CMU_MSCL  : clocks for M2M (Memory to Memory) scaler and JPEG IPs.
- CMU_MFC   : clocks for MFC (Multi-Format Codec) IP.
- CMU_HEVC  : clocks for HEVC(High Efficiency Video Codec) decoder IP.
- CMU_ISP   : clocks for FIMC-ISP/DRC/SCLC/DIS/3DNR IPs.
- CMU_CAM0  : clocks for MIPI_CSIS{0|1}/FIMC_LITE_{A|B|D}/FIMC_3AA{0|1} IPs.
- CMU_CAM1  : clocks for COrtex-A5/MIPI_CSIS2/FIMC_LITE_C/FIMC-FD IPs.

3. pinctrl node for GPIO
- alive/aud/cpif/ese/finger/fsys/imem/nfc/peric/touch pad

4. Timer
- ARM architecture timer (armv8-timer)
- MCT (Multi Core Timer) timer

5. Interrupt controller (GIC-400)

6. BUS devices
- HS-I2C (High-Speed I2C) device
- SPI (Serial Peripheral Interface) device

7. Sound devices
- I2S bus
- LPASS (Low Power Audio Subsystem)

8. Power management devices
- CPUFREQ for for Cortex-A53/A57
- TMU (Thermal Management Unit) for Cortex-A53/A57, G3D, ISP

9. Display controller devices
- DECON (Display and enhancement controller) for panel output
- DSI (Display Serial Interface)
- MIC (Mobile Image Compressor)

10. USB
- USB 3.0 DRD (Dual Role Device) controller
- USB 3.0 Host controller

11. Storage devices
- MSHC (Mobile Stoarage Host Controller)

12. Misc devices
- UART device
- ADC (Analog Digital Converter)
- PWM (Pulse Width Modulation)
- ADMA (Advanced DMA) and PDMA (Peripheral DMA)

Signed-off-by: Chanwoo Choi 
Signed-off-by: Jaehoon Chung 
Signed-off-by: Seung-Woo Kim 
Signed-off-by: Joonyoung Shim 
Signed-off-by: Inki Dae 
Signed-off-by: Jonghwa Lee 
Signed-off-by: Beomho Seo 
Signed-off-by: Jaewon Kim 
Signed-off-by: Hyungwon Hwang 
Signed-off-by: Inha Song 
Signed-off-by: Ingi kim 
Signed-off-by: Krzysztof Kozlowski 
Signed-off-by: Marek Szyprowski 
Signed-off-by: Andrzej Hajda 
Signed-off-by: Sylwester Nawrocki 
---
 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  792 
 .../dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi |   23 +
 .../dts/exynos/exynos5433-tmu-sensor-conf.dtsi |   22 +
 arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi |  306 +
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 1327 
 5 files changed, 2470 insertions(+)
 create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
 create mode 100644 
arch/arm64/boot/dts/exynos/exynos5433-tmu-g3d-sensor-conf.dtsi
 create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tmu-sensor-conf.dtsi
 create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
 create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi 
b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
new file mode 100644
index ..409bfb488b0b
--- /dev/null
+++ b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
@@ -0,0 +1,792 @@
+/*
+ * Samsung's Exynos5433 SoC pin-mux and pin-config device tree source
+ *
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * Chanwoo Choi 
+ *
+ * Samsung's Exynos5433 SoC pin-mux and pin-config options are listed as device
+ * tree nodes are listed in this file.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define PIN_PULL_NONE  0
+#define PIN_PULL_DOWN  1
+#define PIN_PULL_UP3
+
+#define PIN_DRV_LV10
+#define PIN_DRV_LV22
+#define PIN_DRV_LV31
+#define PIN_DRV_LV43
+
+#define PIN_IN 0
+#define PIN_OUT1
+#define PIN_FUNC1  2
+
+#define PIN(_func, _pin, _pull, _drv)  \
+   _pin {  \
+   samsung,pins = #_pin;   \
+