diff mbox series

[10/14] dt-bindings: pinctrl: milbeaut: Add Milbeaut M10V pinctrl description

Message ID 1542589336-13925-1-git-send-email-sugaya.taichi@socionext.com
State Changes Requested, archived
Headers show
Series Add basic support for Socionext Milbeaut M10V SoCs | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Sugaya Taichi Nov. 19, 2018, 1:02 a.m. UTC
Add DT bindings document for Milbeaut M10V pinctrl.

Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
 .../pinctrl/socionext,milbeaut-pinctrl.txt         | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt

Comments

Rob Herring Nov. 19, 2018, 3:46 p.m. UTC | #1
On Sun, Nov 18, 2018 at 7:01 PM Sugaya Taichi
<sugaya.taichi@socionext.com> wrote:
>
> Add devicetree for Milbeaut M10V SoC and M10V Evaluation board.
>
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
>  arch/arm/boot/dts/Makefile               |   1 +
>  arch/arm/boot/dts/milbeaut-m10v-evb.dts  |  35 +++
>  arch/arm/boot/dts/milbeaut-m10v-evb.dtsi |  17 ++
>  arch/arm/boot/dts/milbeaut-m10v.dtsi     | 510 +++++++++++++++++++++++++++++++

I'm not seeing why you need this split into 3 files instead of 2.

>  4 files changed, 563 insertions(+)
>  create mode 100644 arch/arm/boot/dts/milbeaut-m10v-evb.dts
>  create mode 100644 arch/arm/boot/dts/milbeaut-m10v-evb.dtsi
>  create mode 100644 arch/arm/boot/dts/milbeaut-m10v.dtsi

Build your dtb with 'W=12' and fix any warnings.

>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index b0e966d..ee6220b 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1207,6 +1207,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += \
>         mt7623n-bananapi-bpi-r2.dtb \
>         mt8127-moose.dtb \
>         mt8135-evbp1.dtb
> +dtb-$(CONFIG_MACH_M10V_EVB) += milbeaut-m10v-evb.dtb

ARM SoC maintainers,

Can we start at least start putting new SoCs in vendor subdirs? This
one doesn't appear to share anything.

>  dtb-$(CONFIG_ARCH_ZX) += zx296702-ad1.dtb
>  dtb-$(CONFIG_ARCH_ASPEED) += \
>         aspeed-ast2500-evb.dtb \
> diff --git a/arch/arm/boot/dts/milbeaut-m10v-evb.dts b/arch/arm/boot/dts/milbeaut-m10v-evb.dts
> new file mode 100644
> index 0000000..af8d6e4
> --- /dev/null
> +++ b/arch/arm/boot/dts/milbeaut-m10v-evb.dts
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Socionext Milbeaut M10V Evaluation Board */
> +/dts-v1/;
> +#include "milbeaut-m10v-evb.dtsi"
> +
> +/ {
> +       cpus {

cpus is not board specific.

> +               cpu@0 {

Unit-address is wrong. Should be '@f00' to match reg.

> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a7";
> +                       reg = <0xf00>;
> +               };
> +               cpu@1 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a7";
> +                       reg = <0xf01>;
> +               };
> +
> +               cpu@2 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a7";
> +                       reg = <0xf02>;
> +               };
> +               cpu@3 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a7";
> +                       reg = <0xf03>;
> +               };
> +
> +       };
> +       trampoline: trampoline@0x0000F100 {

Also, not board specific.

This should be under a simple-bus node. The unit-address should be '@f100'.

> +               compatible = "socionext,smp-trampoline";
> +               reg = <0x0000F100 0x100>;

Use lowercase hex.

> +       };
> +};
> diff --git a/arch/arm/boot/dts/milbeaut-m10v-evb.dtsi b/arch/arm/boot/dts/milbeaut-m10v-evb.dtsi
> new file mode 100644
> index 0000000..fc35c0b
> --- /dev/null
> +++ b/arch/arm/boot/dts/milbeaut-m10v-evb.dtsi
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "milbeaut-m10v.dtsi"
> +
> +/ {
> +       model = "Socionext M10V EVB";
> +       compatible = "socionext,sc2000a", "socionext,milbeaut-m10v-evb";

Wrong order. And these need to be documented.

> +       interrupt-parent = <&gic>;
> +       chosen {
> +               bootargs = "consoleblank=0 loglevel=8 init=/sbin/finit root=/dev/mmcblk0p2 rootwait ro console=ttyUSI0,115200n8 console=/dev/tty1 ";

Most of these options look user specific and should be dropped. Use
'stdout-path' to specify the default console.

> +               linux,initrd-start = <0x4A000000>;
> +               linux,initrd-end =   <0x4BF00000>;

initrd should be filled in by the bootloader.

> +       };
> +       memory {

Needs a unit-address.

> +               device_type = "memory";
> +               reg = <0x40000000  0x80000000>;
> +       };
> +};
> diff --git a/arch/arm/boot/dts/milbeaut-m10v.dtsi b/arch/arm/boot/dts/milbeaut-m10v.dtsi
> new file mode 100644
> index 0000000..4745dc6
> --- /dev/null
> +++ b/arch/arm/boot/dts/milbeaut-m10v.dtsi
> @@ -0,0 +1,510 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +#include "skeleton.dtsi"

Don't use skeleton.dtsi. It is deprecated.

> +
> +/ {
> +       compatible = "socionext,sc2000a";
> +       interrupt-parent = <&gic>;
> +       cpus {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +       };

Just move this to the other file with the cpu nodes.

> +
> +
> +       gic: interrupt-controller@1d000000 {
> +               compatible = "arm,cortex-a7-gic";
> +               interrupt-controller;
> +               #interrupt-cells = <3>;
> +               reg = <0x1d001000 0x1000>, /* Distributer base and size */
> +                     <0x1d002000 0x1000>; /* CPU I/f base and size */
> +       };
> +
> +       m10v-clk-tree@ {

unit-address

> +               compatible = "socionext,milbeaut-m10v-clk-regs";
> +               reg = <0x1d021000 0x4000>;
> +
> +               clocks {
> +                       #address-cells = <0>;
> +                       #size-cells = <0>;
> +
> +                       uclk40xi: uclk40xi {
> +                               compatible = "fixed-clock";
> +                               #clock-cells = <0>;
> +                               clock-frequency = <40000000>;
> +                       };

Any fixed clocks from oscillators on the board should be at the top
level and defined as inputs to the clock controller node with a
'clocks' property.

> +
> +                       aumclki: aumclki {
> +                               compatible = "fixed-clock";
> +                               #clock-cells = <0>;
> +                               clock-frequency = <20000000>;
> +                       };
> +
> +                       rtc32k: rtc32k {
> +                               compatible = "fixed-clock";
> +                               #clock-cells = <0>;
> +                               clock-frequency = <32768>;
> +                       };
> +
> +                       pxrefclk: pxrefclk {
> +                               compatible = "fixed-clock";
> +                               #clock-cells = <0>;
> +                               clock-frequency = <100000000>;
> +                       };
> +
> +                       pcisuppclk: pcisuppclk {
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&uclk40xi>;
> +                               clock-div = <20>;
> +                               clock-mult = <1>;
> +                       };

Please look at any recent example of clock bindings. We don't try to
describe every divider, mux, gate, etc. in DT, but just clock
controller blocks with all the output clocks enumerated in clock
cells.

> +
> +                       usb2_clk: usb2_clk {
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&uclk40xi>;
> +                               clock-div = <2>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       pll1: pll1 {
> +                               compatible =
> +                               "socionext,milbeaut-m10v-pll-fixed-factor";
> +                               #clock-cells = <0>;
> +                               clocks = <&uclk40xi>;
> +                               offset = <1>;
> +                               clock-div = <1>;
> +                               clock-mult = <40>;
> +                       };
> +
> +                       pll2: pll2 {
> +                               compatible =
> +                               "socionext,milbeaut-m10v-pll-fixed-factor";
> +                               #clock-cells = <0>;
> +                               clocks = <&uclk40xi>;
> +                               offset = <2>;
> +                               clock-div = <1>;
> +                               clock-mult = <30>;
> +                       };
> +
> +                       pll6: pll6 { /* CLK 6-1 */
> +                               compatible =
> +                               "socionext,milbeaut-m10v-pll-fixed-factor";
> +                               #clock-cells = <0>;
> +                               clocks = <&uclk40xi>;
> +                               offset = <7>;
> +                               clock-div = <1>;
> +                               clock-mult = <35>;
> +                       };
> +
> +                       pll7: pll7 { /* CLK 7-1 */
> +                               compatible =
> +                               "socionext,milbeaut-m10v-pll-fixed-factor";
> +                               #clock-cells = <0>;
> +                               clocks = <&uclk40xi>;
> +                               offset = <8>;
> +                               clock-div = <1>;
> +                               clock-mult = <40>;
> +                       };
> +
> +                       pll9: pll9 { /* CA7CLK, ATCLK */
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&uclk40xi>;
> +                               clock-div = <1>;
> +                               clock-mult = <33>;
> +                       };
> +
> +                       pll10: pll10 {
> +                               compatible =
> +                               "socionext,milbeaut-m10v-pll-fixed-factor";
> +                               #clock-cells = <0>;
> +                               clocks = <&uclk40xi>;
> +                               offset = <10>;
> +                               clock-div = <5>;
> +                               clock-mult = <108>;
> +                       };
> +
> +                       pll11: pll11 { /* CLK 11-1 */
> +                               compatible =
> +                               "socionext,milbeaut-m10v-pll-fixed-factor";
> +                               #clock-cells = <0>;
> +                               clocks = <&uclk40xi>;
> +                               offset = <12>;
> +                               clock-div = <2>;
> +                               clock-mult = <75>;
> +                       };
> +
> +                       emmcclk: emmcclk {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll11>;
> +                               offset = <28>; /* EMMCCLK */
> +                               mask = <0x3>;
> +                               ratios = <15 0x7 10 0x6 9 0x5 8 0x4>;
> +                       };
> +
> +                       pll1_div_1_2: pll1_div_1_2 { /* CLK 1-2 */
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll1>;
> +                               clock-div = <2>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       pll2_div_1_2: pll2_div_1_2 { /* CLK 2-2 */
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll2>;
> +                               clock-div = <2>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       pll6_div_1_2: pll6_div_1_2 { /* CLK 6-2 */
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll6>;
> +                               clock-div = <2>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       pll6_div_1_3: pll6_div_1_3 { /* CLK 6-3 */
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll6>;
> +                               clock-div = <3>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       pll7_div_1_2: pll7_div_1_2 { /* CLK 7-2 */
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll7>;
> +                               clock-div = <2>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       pll7_div_1_5: pll7_div_1_5 { /* CLK 7-5 */
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll7>;
> +                               clock-div = <5>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       pll10_div_1_2: pll10_div_1_2 { /* CLK 10-2 */
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll10>;
> +                               clock-div = <2>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       spiclk_mux_0: spiclk_mux_0 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll10_div_1_2>;
> +                               offset = <227>; /* SPICLK */
> +                               mask = <0x3>;
> +                               ratios = <4 0x5 2 0x4>;
> +                       };
> +
> +                       spiclk_mux_1: spiclk_mux_1 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll7_div_1_2>;
> +                               offset = <227>; /* SPICLK */
> +                               mask = <0x3>;
> +                               ratios = <8 0x6>;
> +                       };
> +
> +                       spiclk: spiclk {
> +                               compatible = "socionext,milbeaut-m10v-clk-mux";
> +                               #clock-cells = <0>;
> +                               clocks = <&spiclk_mux_0>, <&spiclk_mux_1>;
> +                       };
> +
> +                       ca7wdclk: ca7wdclk {
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll2_div_1_2>;
> +                               clock-div = <12>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       pll9_div_1_2: pll9_div_1_2 {
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll9>;
> +                               clock-div = <2>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       mclk400: mclk400 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll1_div_1_2>;
> +                               offset = <295>; /* MCLK400 */
> +                               mask = <0x3>;
> +                               ratios = <4 0x7 2 0x5>;
> +                       };
> +
> +                       mclk200: mclk200 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll1_div_1_2>;
> +                               offset = <291>; /* MCLK200 */
> +                               mask = <0x7>;
> +                               ratios = <8 0xf 4 0xb>;
> +                       };
> +
> +                       aclk400: aclk400 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll1_div_1_2>;
> +                               offset = <288>; /* ACLK400 */
> +                               mask = <0x3>;
> +                               ratios = <4 0x7 2 0x5>;
> +                       };
> +
> +                       aclk300: aclk300 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll2_div_1_2>;
> +                               offset = <352>; /* ACLK300 */
> +                               mask = <0x1>;
> +                               ratios = <6 0x3 4 0x2>;
> +                       };
> +
> +                       aclk: aclk {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll1_div_1_2>;
> +                               offset = <276>; /* ACLK */
> +                               mask = <0x7>;
> +                               ratios = <8 0xf 4 0xb>;
> +                       };
> +
> +                       aclkexs: aclkexs {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll1_div_1_2>;
> +                               offset = <272>; /* ACLKEXS */
> +                               mask = <0x7>;
> +                               ratios = <8 0xf 6 0xd 5 0xc 4 0xb>;
> +                       };
> +
> +                       hclk: hclk {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll1_div_1_2>;
> +                               offset = <263>; /* HCLK */
> +                               mask = <0xf>;
> +                               ratios = <16 0x1f 8 0x17>;
> +                       };
> +
> +                       hclkbmh: hclkbmh {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll1_div_1_2>;
> +                               offset = <268>; /* HCLKBMH */
> +                               mask = <0x7>;
> +                               ratios = <8 0xf 4 0xb>;
> +                       };
> +
> +                       pclk: pclk {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll1_div_1_2>;
> +                               offset = <256>; /* PCLK */
> +                               mask = <0x3f>;
> +                               ratios = <32 0x5f 16 0x4f>;
> +                       };
> +
> +                       pclkca7wd: pclkca7wd {
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll1_div_1_2>;
> +                               clock-div = <16>;
> +                               clock-mult = <1>;
> +                       };
> +
> +                       rclk: rclk {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll10_div_1_2>;
> +                               offset = <0>; /* RCLK */
> +                               mask = <0x3>;
> +                               ratios = <64 0x7 48 0x6 32 0x5 16 0x4>;
> +                       };
> +
> +                       uhs1clk0: uhs1clk0 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll7>;
> +                               offset = <3>; /* UHS1CLK0 */
> +                               mask = <0xf>;
> +                               ratios = <16 0x14 8 0x13 4 0x12 3 0x11 2 0x10>;
> +                       };
> +
> +                       uhs1clk1_div1: uhs1clk1_div1 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll7>;
> +                               offset = <8>; /* UHS1CLK1 */
> +                               mask = <0xf>;
> +                               ratios = <16 0x14 8 0x13>;
> +                       };
> +
> +                       uhs1clk1_div2: uhs1clk1_div2 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll6_div_1_2>;
> +                               offset = <8>; /* UHS1CLK1 */
> +                               mask = <0xf>;
> +                               ratios = <1 0x18>;
> +                       };
> +
> +                       uhs1clk1: uhs1clk1 {
> +                               compatible = "socionext,milbeaut-m10v-clk-mux";
> +                               #clock-cells = <0>;
> +                               clocks = <&uhs1clk1_div1>, <&uhs1clk1_div2>;
> +                       };
> +
> +                       uhs1clk2_div1: uhs1clk2_div1 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll7>;
> +                               offset = <13>; /* UHS1CLK2 */
> +                               mask = <0xf>;
> +                               ratios = <16 0x14 8 0x13 4 0x12>;
> +                       };
> +
> +                       uhs1clk2_div2: uhs1clk2_div2 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll6_div_1_2>;
> +                               offset = <13>; /* UHS1CLK2 */
> +                               mask = <0xf>;
> +                               ratios = <1 0x18>;
> +                       };
> +
> +                       uhs1clk2: uhs1clk2 {
> +                               compatible = "socionext,milbeaut-m10v-clk-mux";
> +                               #clock-cells = <0>;
> +                               clocks = <&uhs1clk2_div1>, <&uhs1clk2_div2>;
> +                       };
> +
> +                       uhs2clk: uhs2clk {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll6_div_1_3>;
> +                               offset = <18>; /* UHS2CLK */
> +                               mask = <0x7>;
> +                               ratios = <18 0xf 16 0xe 14 0xd 13 0xc
> +                                               12 0xb 11 0xa 10 0x9 9 0x8>;
> +                       };
> +
> +                       nfclk_div1: nfclk_div1 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll7_div_1_2>;
> +                               offset = <22>; /* NFCLK */
> +                               mask = <0x1f>;
> +                               ratios = <40 0x24 16 0x23 13 0x22 10 0x21
> +                                               8 0x20>;
> +                       };
> +
> +                       nfclk_div2: nfclk_div2 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll7_div_1_5>;
> +                               offset = <22>; /* NFCLK */
> +                               mask = <0x1f>;
> +                               ratios = <10 0x28>;
> +                       };
> +
> +                       nfclk: nfclk {
> +                               compatible = "socionext,milbeaut-m10v-clk-mux";
> +                               #clock-cells = <0>;
> +                               clocks = <&nfclk_div1>, <&nfclk_div2>;
> +                       };
> +
> +                       clk5: clk5 {
> +                               compatible = "socionext,milbeaut-m10v-clk-div";
> +                               #clock-cells = <0>;
> +                               clocks = <&pll10_div_1_2>;
> +                               offset = <239>; /* NETAUSEL */
> +                               mask = <0x3>;
> +                               ratios = <64 0x7 48 0x6 32 0x5 16 0x4>;
> +                       };
> +               };
> +       };
> +
> +       peri-timer@1e000000 { /* 32-bit Reload Timers */

timer@1e000050

This and all memory mapped peripherals go under simple-bus nodes(s).

> +               compatible = "socionext,milbeaut-m10v-timer";
> +               reg = <0x1e000050 0x10>, <0x1e000060 0x10>;
> +               interrupts = <0 91 4>;
> +               clocks = <&rclk>;
> +       };
> +
> +       timer { /* The Generic Timer */
> +               compatible = "arm,armv7-timer";
> +               interrupts = <GIC_PPI 13
> +                               (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +                       <GIC_PPI 14
> +                               (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +                       <GIC_PPI 11
> +                               (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +                       <GIC_PPI 10
> +                               (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> +               clock-frequency = <40000000>;//40M
> +               always-on;
> +               arm,cpu-registers-not-fw-configured;

This was a work-around for some existing platforms. New platforms
should fix the firmware/bootloader.

> +       };
> +
> +       dummy_clk: dummy_clk {
> +               compatible = "fixed-clock";
> +               #clock-cells = <0>;
> +               clock-frequency = <50000000>;
> +       };
> +
> +       pinctrl: pinctrl@1d022000 {
> +               compatible = "socionext,milbeaut-m10v-pinctrl";
> +               reg = <0x1d022000 0x1000>,
> +                     <0x1c26f000 0x1000>;
> +               reg-names = "pinctrl", "exiu";
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +               interrupt-controller;
> +               #interrupt-cells = <2>;
> +               clocks = <&dummy_clk>;

Please use a real clock or make clocks optional.

> +               interrupts = <0 54 4>, <0 55 4>, <0 56 4>, <0 57 4>,
> +                               <0 58 4>, <0 59 4>, <0 60 4>, <0 61 4>,
> +                               <0 62 4>, <0 63 4>, <0 64 4>, <0 65 4>,
> +                               <0 66 4>, <0 67 4>, <0 68 4>, <0 69 4>;
> +               interrupt-names = "pin-48", "pin-49", "pin-50", "pin-51",
> +                               "pin-52", "pin-53", "pin-54", "pin-55",
> +                               "pin-56", "pin-57", "pin-58", "pin-59",
> +                               "pin-60", "pin-61", "pin-62", "pin-63";
> +
> +               usio1_pins: usio1_pins {
> +                       pins = "PE4", "PE5", "P87";
> +                       function = "usio1";
> +               };
> +       };
> +
> +       usio1: usio_uart@1e700010 { /* PE4, PE5 */

serial@...

> +               /* Enable this as ttyUSI0 */
> +               index = <0>;

Drop this. aliases node is the right way to do this.

> +               compatible = "socionext,milbeaut-m10v-usio-uart";
> +               reg = <0x1e700010 0x10>;
> +               interrupts = <0 141 0x4>, <0 149 0x4>;
> +               interrupt-names = "rx", "tx";
> +               clocks = <&hclk>;
> +       };
> +};
> --
> 1.9.1
>
Sugaya Taichi Nov. 21, 2018, 2:36 a.m. UTC | #2
Hi Rob

Thank you for your comments.

> -----Original Message-----
> From: Rob Herring [mailto:robh+dt@kernel.org]
> Sent: Tuesday, November 20, 2018 12:46 AM
> To: Sugaya, Taichi; ARM-SoC Maintainers
> Cc: linux-clk; devicetree@vger.kernel.org; moderated list:ARM/FREESCALE
> IMX / MXC ARM ARCHITECTURE; linux-kernel@vger.kernel.org; open list:SERIAL
> DRIVERS; Michael Turquette; Stephen Boyd; Mark Rutland; Greg Kroah-Hartman;
> Daniel Lezcano; Thomas Gleixner; Russell King; Jiri Slaby; Masami Hiramatsu;
> Jassi Brar
> Subject: Re: [PATCH 12/14] ARM: dts: milbeaut: Add device tree set for the
> Milbeaut M10V board
> 
> On Sun, Nov 18, 2018 at 7:01 PM Sugaya Taichi
> <sugaya.taichi@socionext.com> wrote:
> >
> > Add devicetree for Milbeaut M10V SoC and M10V Evaluation board.
> >
> > Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> > ---
> >  arch/arm/boot/dts/Makefile               |   1 +
> >  arch/arm/boot/dts/milbeaut-m10v-evb.dts  |  35 +++
> >  arch/arm/boot/dts/milbeaut-m10v-evb.dtsi |  17 ++
> >  arch/arm/boot/dts/milbeaut-m10v.dtsi     | 510
> +++++++++++++++++++++++++++++++
> 
> I'm not seeing why you need this split into 3 files instead of 2.
Correct...
Modify using 2 files.

> 
> >  4 files changed, 563 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/milbeaut-m10v-evb.dts
> >  create mode 100644 arch/arm/boot/dts/milbeaut-m10v-evb.dtsi
> >  create mode 100644 arch/arm/boot/dts/milbeaut-m10v.dtsi
> 
> Build your dtb with 'W=12' and fix any warnings.
Ah, I may have missed warnings. will confirm and fix them.

> 
> >
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index b0e966d..ee6220b 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1207,6 +1207,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += \
> >         mt7623n-bananapi-bpi-r2.dtb \
> >         mt8127-moose.dtb \
> >         mt8135-evbp1.dtb
> > +dtb-$(CONFIG_MACH_M10V_EVB) += milbeaut-m10v-evb.dtb
> 
> ARM SoC maintainers,
> 
> Can we start at least start putting new SoCs in vendor subdirs? This
> one doesn't appear to share anything.
> 
> >  dtb-$(CONFIG_ARCH_ZX) += zx296702-ad1.dtb
> >  dtb-$(CONFIG_ARCH_ASPEED) += \
> >         aspeed-ast2500-evb.dtb \
> > diff --git a/arch/arm/boot/dts/milbeaut-m10v-evb.dts
> b/arch/arm/boot/dts/milbeaut-m10v-evb.dts
> > new file mode 100644
> > index 0000000..af8d6e4
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/milbeaut-m10v-evb.dts
> > @@ -0,0 +1,35 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Socionext Milbeaut M10V Evaluation Board */
> > +/dts-v1/;
> > +#include "milbeaut-m10v-evb.dtsi"
> > +
> > +/ {
> > +       cpus {
> 
> cpus is not board specific.
Yes..
Cpus Should be described in "milbeaut-m10v-evb.dtsi".

> 
> > +               cpu@0 {
> 
> Unit-address is wrong. Should be '@f00' to match reg.
Ok.

> 
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a7";
> > +                       reg = <0xf00>;
> > +               };
> > +               cpu@1 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a7";
> > +                       reg = <0xf01>;
> > +               };
> > +
> > +               cpu@2 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a7";
> > +                       reg = <0xf02>;
> > +               };
> > +               cpu@3 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a7";
> > +                       reg = <0xf03>;
> > +               };
> > +
> > +       };
> > +       trampoline: trampoline@0x0000F100 {
> 
> Also, not board specific.
> 
> This should be under a simple-bus node. The unit-address should be '@f100'.
> 
> > +               compatible = "socionext,smp-trampoline";
> > +               reg = <0x0000F100 0x100>;
> 
> Use lowercase hex.
OKay.

> 
> > +       };
> > +};
> > diff --git a/arch/arm/boot/dts/milbeaut-m10v-evb.dtsi
> b/arch/arm/boot/dts/milbeaut-m10v-evb.dtsi
> > new file mode 100644
> > index 0000000..fc35c0b
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/milbeaut-m10v-evb.dtsi
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include "milbeaut-m10v.dtsi"
> > +
> > +/ {
> > +       model = "Socionext M10V EVB";
> > +       compatible = "socionext,sc2000a",
> "socionext,milbeaut-m10v-evb";
> 
> Wrong order. And these need to be documented.
I got it.

> 
> > +       interrupt-parent = <&gic>;
> > +       chosen {
> > +               bootargs = "consoleblank=0 loglevel=8 init=/sbin/finit
> root=/dev/mmcblk0p2 rootwait ro console=ttyUSI0,115200n8
> console=/dev/tty1 ";
> 
> Most of these options look user specific and should be dropped. Use
> 'stdout-path' to specify the default console.
OK, try to use it.

> 
> > +               linux,initrd-start = <0x4A000000>;
> > +               linux,initrd-end =   <0x4BF00000>;
> 
> initrd should be filled in by the bootloader.
OK.

> 
> > +       };
> > +       memory {
> 
> Needs a unit-address.
OK.

> 
> > +               device_type = "memory";
> > +               reg = <0x40000000  0x80000000>;
> > +       };
> > +};
> > diff --git a/arch/arm/boot/dts/milbeaut-m10v.dtsi
> b/arch/arm/boot/dts/milbeaut-m10v.dtsi
> > new file mode 100644
> > index 0000000..4745dc6
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/milbeaut-m10v.dtsi
> > @@ -0,0 +1,510 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/input/input.h>
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +#include "skeleton.dtsi"
> 
> Don't use skeleton.dtsi. It is deprecated.
I got it. Get rid of it.

> 
> > +
> > +/ {
> > +       compatible = "socionext,sc2000a";
> > +       interrupt-parent = <&gic>;
> > +       cpus {
> > +               #address-cells = <1>;
> > +               #size-cells = <0>;
> > +       };
> 
> Just move this to the other file with the cpu nodes.
I see.
I'd like to select the way to move the cpu nodes to this file.

> 
> > +
> > +
> > +       gic: interrupt-controller@1d000000 {
> > +               compatible = "arm,cortex-a7-gic";
> > +               interrupt-controller;
> > +               #interrupt-cells = <3>;
> > +               reg = <0x1d001000 0x1000>, /* Distributer base and size
> */
> > +                     <0x1d002000 0x1000>; /* CPU I/f base and size */
> > +       };
> > +
> > +       m10v-clk-tree@ {
> 
> unit-address
OK, add it.

> 
> > +               compatible = "socionext,milbeaut-m10v-clk-regs";
> > +               reg = <0x1d021000 0x4000>;
> > +
> > +               clocks {
> > +                       #address-cells = <0>;
> > +                       #size-cells = <0>;
> > +
> > +                       uclk40xi: uclk40xi {
> > +                               compatible = "fixed-clock";
> > +                               #clock-cells = <0>;
> > +                               clock-frequency = <40000000>;
> > +                       };
> 
> Any fixed clocks from oscillators on the board should be at the top
> level and defined as inputs to the clock controller node with a
> 'clocks' property.
OK.

> 
> > +
> > +                       aumclki: aumclki {
> > +                               compatible = "fixed-clock";
> > +                               #clock-cells = <0>;
> > +                               clock-frequency = <20000000>;
> > +                       };
> > +
> > +                       rtc32k: rtc32k {
> > +                               compatible = "fixed-clock";
> > +                               #clock-cells = <0>;
> > +                               clock-frequency = <32768>;
> > +                       };
> > +
> > +                       pxrefclk: pxrefclk {
> > +                               compatible = "fixed-clock";
> > +                               #clock-cells = <0>;
> > +                               clock-frequency = <100000000>;
> > +                       };
> > +
> > +                       pcisuppclk: pcisuppclk {
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uclk40xi>;
> > +                               clock-div = <20>;
> > +                               clock-mult = <1>;
> > +                       };
> 
> Please look at any recent example of clock bindings. We don't try to
> describe every divider, mux, gate, etc. in DT, but just clock
> controller blocks with all the output clocks enumerated in clock
> cells.
OK, study them at first.

> 
> > +
> > +                       usb2_clk: usb2_clk {
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uclk40xi>;
> > +                               clock-div = <2>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       pll1: pll1 {
> > +                               compatible =
> > +
> "socionext,milbeaut-m10v-pll-fixed-factor";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uclk40xi>;
> > +                               offset = <1>;
> > +                               clock-div = <1>;
> > +                               clock-mult = <40>;
> > +                       };
> > +
> > +                       pll2: pll2 {
> > +                               compatible =
> > +
> "socionext,milbeaut-m10v-pll-fixed-factor";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uclk40xi>;
> > +                               offset = <2>;
> > +                               clock-div = <1>;
> > +                               clock-mult = <30>;
> > +                       };
> > +
> > +                       pll6: pll6 { /* CLK 6-1 */
> > +                               compatible =
> > +
> "socionext,milbeaut-m10v-pll-fixed-factor";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uclk40xi>;
> > +                               offset = <7>;
> > +                               clock-div = <1>;
> > +                               clock-mult = <35>;
> > +                       };
> > +
> > +                       pll7: pll7 { /* CLK 7-1 */
> > +                               compatible =
> > +
> "socionext,milbeaut-m10v-pll-fixed-factor";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uclk40xi>;
> > +                               offset = <8>;
> > +                               clock-div = <1>;
> > +                               clock-mult = <40>;
> > +                       };
> > +
> > +                       pll9: pll9 { /* CA7CLK, ATCLK */
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uclk40xi>;
> > +                               clock-div = <1>;
> > +                               clock-mult = <33>;
> > +                       };
> > +
> > +                       pll10: pll10 {
> > +                               compatible =
> > +
> "socionext,milbeaut-m10v-pll-fixed-factor";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uclk40xi>;
> > +                               offset = <10>;
> > +                               clock-div = <5>;
> > +                               clock-mult = <108>;
> > +                       };
> > +
> > +                       pll11: pll11 { /* CLK 11-1 */
> > +                               compatible =
> > +
> "socionext,milbeaut-m10v-pll-fixed-factor";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uclk40xi>;
> > +                               offset = <12>;
> > +                               clock-div = <2>;
> > +                               clock-mult = <75>;
> > +                       };
> > +
> > +                       emmcclk: emmcclk {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll11>;
> > +                               offset = <28>; /* EMMCCLK */
> > +                               mask = <0x3>;
> > +                               ratios = <15 0x7 10 0x6 9 0x5 8 0x4>;
> > +                       };
> > +
> > +                       pll1_div_1_2: pll1_div_1_2 { /* CLK 1-2 */
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll1>;
> > +                               clock-div = <2>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       pll2_div_1_2: pll2_div_1_2 { /* CLK 2-2 */
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll2>;
> > +                               clock-div = <2>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       pll6_div_1_2: pll6_div_1_2 { /* CLK 6-2 */
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll6>;
> > +                               clock-div = <2>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       pll6_div_1_3: pll6_div_1_3 { /* CLK 6-3 */
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll6>;
> > +                               clock-div = <3>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       pll7_div_1_2: pll7_div_1_2 { /* CLK 7-2 */
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll7>;
> > +                               clock-div = <2>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       pll7_div_1_5: pll7_div_1_5 { /* CLK 7-5 */
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll7>;
> > +                               clock-div = <5>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       pll10_div_1_2: pll10_div_1_2 { /* CLK 10-2 */
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll10>;
> > +                               clock-div = <2>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       spiclk_mux_0: spiclk_mux_0 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll10_div_1_2>;
> > +                               offset = <227>; /* SPICLK */
> > +                               mask = <0x3>;
> > +                               ratios = <4 0x5 2 0x4>;
> > +                       };
> > +
> > +                       spiclk_mux_1: spiclk_mux_1 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll7_div_1_2>;
> > +                               offset = <227>; /* SPICLK */
> > +                               mask = <0x3>;
> > +                               ratios = <8 0x6>;
> > +                       };
> > +
> > +                       spiclk: spiclk {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-mux";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&spiclk_mux_0>,
> <&spiclk_mux_1>;
> > +                       };
> > +
> > +                       ca7wdclk: ca7wdclk {
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll2_div_1_2>;
> > +                               clock-div = <12>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       pll9_div_1_2: pll9_div_1_2 {
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll9>;
> > +                               clock-div = <2>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       mclk400: mclk400 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll1_div_1_2>;
> > +                               offset = <295>; /* MCLK400 */
> > +                               mask = <0x3>;
> > +                               ratios = <4 0x7 2 0x5>;
> > +                       };
> > +
> > +                       mclk200: mclk200 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll1_div_1_2>;
> > +                               offset = <291>; /* MCLK200 */
> > +                               mask = <0x7>;
> > +                               ratios = <8 0xf 4 0xb>;
> > +                       };
> > +
> > +                       aclk400: aclk400 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll1_div_1_2>;
> > +                               offset = <288>; /* ACLK400 */
> > +                               mask = <0x3>;
> > +                               ratios = <4 0x7 2 0x5>;
> > +                       };
> > +
> > +                       aclk300: aclk300 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll2_div_1_2>;
> > +                               offset = <352>; /* ACLK300 */
> > +                               mask = <0x1>;
> > +                               ratios = <6 0x3 4 0x2>;
> > +                       };
> > +
> > +                       aclk: aclk {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll1_div_1_2>;
> > +                               offset = <276>; /* ACLK */
> > +                               mask = <0x7>;
> > +                               ratios = <8 0xf 4 0xb>;
> > +                       };
> > +
> > +                       aclkexs: aclkexs {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll1_div_1_2>;
> > +                               offset = <272>; /* ACLKEXS */
> > +                               mask = <0x7>;
> > +                               ratios = <8 0xf 6 0xd 5 0xc 4 0xb>;
> > +                       };
> > +
> > +                       hclk: hclk {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll1_div_1_2>;
> > +                               offset = <263>; /* HCLK */
> > +                               mask = <0xf>;
> > +                               ratios = <16 0x1f 8 0x17>;
> > +                       };
> > +
> > +                       hclkbmh: hclkbmh {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll1_div_1_2>;
> > +                               offset = <268>; /* HCLKBMH */
> > +                               mask = <0x7>;
> > +                               ratios = <8 0xf 4 0xb>;
> > +                       };
> > +
> > +                       pclk: pclk {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll1_div_1_2>;
> > +                               offset = <256>; /* PCLK */
> > +                               mask = <0x3f>;
> > +                               ratios = <32 0x5f 16 0x4f>;
> > +                       };
> > +
> > +                       pclkca7wd: pclkca7wd {
> > +                               compatible = "fixed-factor-clock";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll1_div_1_2>;
> > +                               clock-div = <16>;
> > +                               clock-mult = <1>;
> > +                       };
> > +
> > +                       rclk: rclk {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll10_div_1_2>;
> > +                               offset = <0>; /* RCLK */
> > +                               mask = <0x3>;
> > +                               ratios = <64 0x7 48 0x6 32 0x5 16 0x4>;
> > +                       };
> > +
> > +                       uhs1clk0: uhs1clk0 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll7>;
> > +                               offset = <3>; /* UHS1CLK0 */
> > +                               mask = <0xf>;
> > +                               ratios = <16 0x14 8 0x13 4 0x12 3 0x11
> 2 0x10>;
> > +                       };
> > +
> > +                       uhs1clk1_div1: uhs1clk1_div1 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll7>;
> > +                               offset = <8>; /* UHS1CLK1 */
> > +                               mask = <0xf>;
> > +                               ratios = <16 0x14 8 0x13>;
> > +                       };
> > +
> > +                       uhs1clk1_div2: uhs1clk1_div2 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll6_div_1_2>;
> > +                               offset = <8>; /* UHS1CLK1 */
> > +                               mask = <0xf>;
> > +                               ratios = <1 0x18>;
> > +                       };
> > +
> > +                       uhs1clk1: uhs1clk1 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-mux";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uhs1clk1_div1>,
> <&uhs1clk1_div2>;
> > +                       };
> > +
> > +                       uhs1clk2_div1: uhs1clk2_div1 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll7>;
> > +                               offset = <13>; /* UHS1CLK2 */
> > +                               mask = <0xf>;
> > +                               ratios = <16 0x14 8 0x13 4 0x12>;
> > +                       };
> > +
> > +                       uhs1clk2_div2: uhs1clk2_div2 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll6_div_1_2>;
> > +                               offset = <13>; /* UHS1CLK2 */
> > +                               mask = <0xf>;
> > +                               ratios = <1 0x18>;
> > +                       };
> > +
> > +                       uhs1clk2: uhs1clk2 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-mux";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&uhs1clk2_div1>,
> <&uhs1clk2_div2>;
> > +                       };
> > +
> > +                       uhs2clk: uhs2clk {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll6_div_1_3>;
> > +                               offset = <18>; /* UHS2CLK */
> > +                               mask = <0x7>;
> > +                               ratios = <18 0xf 16 0xe 14 0xd 13 0xc
> > +                                               12 0xb 11 0xa 10 0x9 9
> 0x8>;
> > +                       };
> > +
> > +                       nfclk_div1: nfclk_div1 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll7_div_1_2>;
> > +                               offset = <22>; /* NFCLK */
> > +                               mask = <0x1f>;
> > +                               ratios = <40 0x24 16 0x23 13 0x22 10 0x21
> > +                                               8 0x20>;
> > +                       };
> > +
> > +                       nfclk_div2: nfclk_div2 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll7_div_1_5>;
> > +                               offset = <22>; /* NFCLK */
> > +                               mask = <0x1f>;
> > +                               ratios = <10 0x28>;
> > +                       };
> > +
> > +                       nfclk: nfclk {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-mux";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&nfclk_div1>, <&nfclk_div2>;
> > +                       };
> > +
> > +                       clk5: clk5 {
> > +                               compatible =
> "socionext,milbeaut-m10v-clk-div";
> > +                               #clock-cells = <0>;
> > +                               clocks = <&pll10_div_1_2>;
> > +                               offset = <239>; /* NETAUSEL */
> > +                               mask = <0x3>;
> > +                               ratios = <64 0x7 48 0x6 32 0x5 16 0x4>;
> > +                       };
> > +               };
> > +       };
> > +
> > +       peri-timer@1e000000 { /* 32-bit Reload Timers */
> 
> timer@1e000050
> 
> This and all memory mapped peripherals go under simple-bus nodes(s).
Okay.

> 
> > +               compatible = "socionext,milbeaut-m10v-timer";
> > +               reg = <0x1e000050 0x10>, <0x1e000060 0x10>;
> > +               interrupts = <0 91 4>;
> > +               clocks = <&rclk>;
> > +       };
> > +
> > +       timer { /* The Generic Timer */
> > +               compatible = "arm,armv7-timer";
> > +               interrupts = <GIC_PPI 13
> > +                               (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > +                       <GIC_PPI 14
> > +                               (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > +                       <GIC_PPI 11
> > +                               (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > +                       <GIC_PPI 10
> > +                               (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>;
> > +               clock-frequency = <40000000>;//40M
> > +               always-on;
> > +               arm,cpu-registers-not-fw-configured;
> 
> This was a work-around for some existing platforms. New platforms
> should fix the firmware/bootloader.
OK. try to fix without the property.

> 
> > +       };
> > +
> > +       dummy_clk: dummy_clk {
> > +               compatible = "fixed-clock";
> > +               #clock-cells = <0>;
> > +               clock-frequency = <50000000>;
> > +       };
> > +
> > +       pinctrl: pinctrl@1d022000 {
> > +               compatible = "socionext,milbeaut-m10v-pinctrl";
> > +               reg = <0x1d022000 0x1000>,
> > +                     <0x1c26f000 0x1000>;
> > +               reg-names = "pinctrl", "exiu";
> > +               gpio-controller;
> > +               #gpio-cells = <2>;
> > +               interrupt-controller;
> > +               #interrupt-cells = <2>;
> > +               clocks = <&dummy_clk>;
> 
> Please use a real clock or make clocks optional.
OK.

> 
> > +               interrupts = <0 54 4>, <0 55 4>, <0 56 4>, <0 57 4>,
> > +                               <0 58 4>, <0 59 4>, <0 60 4>, <0 61 4>,
> > +                               <0 62 4>, <0 63 4>, <0 64 4>, <0 65 4>,
> > +                               <0 66 4>, <0 67 4>, <0 68 4>, <0 69 4>;
> > +               interrupt-names = "pin-48", "pin-49", "pin-50",
> "pin-51",
> > +                               "pin-52", "pin-53", "pin-54", "pin-55",
> > +                               "pin-56", "pin-57", "pin-58", "pin-59",
> > +                               "pin-60", "pin-61", "pin-62", "pin-63";
> > +
> > +               usio1_pins: usio1_pins {
> > +                       pins = "PE4", "PE5", "P87";
> > +                       function = "usio1";
> > +               };
> > +       };
> > +
> > +       usio1: usio_uart@1e700010 { /* PE4, PE5 */
> 
> serial@...
OK.

> 
> > +               /* Enable this as ttyUSI0 */
> > +               index = <0>;
> 
> Drop this. aliases node is the right way to do this.
I see. try to use the way.

Thanks
Sugaya Taichi

> 
> > +               compatible = "socionext,milbeaut-m10v-usio-uart";
> > +               reg = <0x1e700010 0x10>;
> > +               interrupts = <0 141 0x4>, <0 149 0x4>;
> > +               interrupt-names = "rx", "tx";
> > +               clocks = <&hclk>;
> > +       };
> > +};
> > --
> > 1.9.1
> >
Masahiro Yamada Dec. 4, 2018, 11:23 a.m. UTC | #3
Hi Sugaya-san

On Mon, Nov 19, 2018 at 10:01 AM Sugaya Taichi
<sugaya.taichi@socionext.com> wrote:
>
> Add Milbeaut M10V pinctrl.
> The M10V has the pins that can be used GPIOs or take multiple other
> functions.
>
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>


This patch was sent to:

linux-clk@vger.kernel.org,
devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org




Unfortunately, the most important ML

  linux-gpio@vger.kernel.org

was not addressed.


The pinctrl maintainer may not notice this patch.







> diff --git a/drivers/pinctrl/pinctrl-m10v.c b/drivers/pinctrl/pinctrl-m10v.c
> new file mode 100644
> index 0000000..d4ca713
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-m10v.c
> @@ -0,0 +1,765 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Socionext Ltd.
> + * Copyright (C) 2015 Linaro Ltd.
> + * Author: Jassi Brar <jaswinder.singh@linaro.org>


My company name is "Socionext Inc." instead of "Socionext Ltd."







> +static struct platform_driver m10v_pinctrl_driver = {
> +       .probe  = m10v_pinctrl_probe,
> +       .driver = {
> +               .name           = "m10v-pinctrl",
> +               .of_match_table = m10v_pmatch,
> +       },
> +};
> +
> +static int __init m10v_pinctrl_init(void)
> +{
> +       return platform_driver_register(&m10v_pinctrl_driver);
> +}
> +arch_initcall(m10v_pinctrl_init);


Can't it be builtin_platform_driver()?

Which device requires this to be arch_initcall()?





--
Best Regards
Masahiro Yamada
Rob Herring (Arm) Dec. 4, 2018, 11:11 p.m. UTC | #4
On Mon, Nov 19, 2018 at 10:02:12AM +0900, Sugaya Taichi wrote:
> Add DT bindings document for Milbeaut M10V pinctrl.
> 
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
>  .../pinctrl/socionext,milbeaut-pinctrl.txt         | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt
> new file mode 100644
> index 0000000..7469189
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt
> @@ -0,0 +1,33 @@
> +Milbeaut SoCs pin controller
> +
> +Required properties:
> +- compatible: should be one of the following:
> +    "socionext,milbeaut-m10v-pinctrl"  - for m10v SoC
> +- reg: offset and length of the register set.
> +- reg-names: should be "pinctrl", "exiu".
> +- gpio-cells; should be 2.

#gpio-cells

gpio-controller?

> +- interrupt-cells: should be 2.

#interrupt-cells

interrupt-controller?

> +- clocks: phandle to the input clock.
> +- interrupts: three interrupts specifer.
> +- interrupt-names: corresponds "interrupts" factor.
> +
> +Example:
> +	pinctrl: pinctrl@1d022000 {
> +		compatible = "socionext,milbeaut-m10v-pinctrl";
> +		reg = <0x1d022000 0x1000>,
> +			<0x1c26f000 0x1000>;
> +		reg-names = "pinctrl", "exiu";
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +		clocks = <&dummy_clk>;
> +		interrupts = <0 54 4>, <0 55 4>, <0 56 4>, <0 57 4>,
> +				<0 58 4>, <0 59 4>, <0 60 4>, <0 61 4>,
> +				<0 62 4>, <0 63 4>, <0 64 4>, <0 65 4>,
> +				<0 66 4>, <0 67 4>, <0 68 4>, <0 69 4>;
> +		interrupt-names = "pin-48", "pin-49", "pin-50", "pin-51",
> +				"pin-52", "pin-53", "pin-54", "pin-55",
> +				"pin-56", "pin-57", "pin-58", "pin-59",
> +				"pin-60", "pin-61", "pin-62", "pin-63";
> +	}
> -- 
> 1.9.1
>
Sugaya Taichi Dec. 5, 2018, 5:03 a.m. UTC | #5
Hi

Thank you for your comments.

On 2018/12/04 20:23, Masahiro Yamada wrote:
> Hi Sugaya-san
> 
> On Mon, Nov 19, 2018 at 10:01 AM Sugaya Taichi
> <sugaya.taichi@socionext.com> wrote:
>>
>> Add Milbeaut M10V pinctrl.
>> The M10V has the pins that can be used GPIOs or take multiple other
>> functions.
>>
>> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> 
> 
> This patch was sent to:
> 
> linux-clk@vger.kernel.org,
> devicetree@vger.kernel.org,
> linux-arm-kernel@lists.infradead.org,
> linux-kernel@vger.kernel.org,
> linux-serial@vger.kernel.org
> 
> 
> 
> 
> Unfortunately, the most important ML
> 
>    linux-gpio@vger.kernel.org
> 
> was not addressed.
> 
> 
> The pinctrl maintainer may not notice this patch.

Ah I took a critical mistake...

> 
> 
> 
> 
> 
> 
> 
>> diff --git a/drivers/pinctrl/pinctrl-m10v.c b/drivers/pinctrl/pinctrl-m10v.c
>> new file mode 100644
>> index 0000000..d4ca713
>> --- /dev/null
>> +++ b/drivers/pinctrl/pinctrl-m10v.c
>> @@ -0,0 +1,765 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Socionext Ltd.
>> + * Copyright (C) 2015 Linaro Ltd.
>> + * Author: Jassi Brar <jaswinder.singh@linaro.org>
> 
> 
> My company name is "Socionext Inc." instead of "Socionext Ltd."

Yes, modify it.

> 
> 
> 
> 
> 
> 
> 
>> +static struct platform_driver m10v_pinctrl_driver = {
>> +       .probe  = m10v_pinctrl_probe,
>> +       .driver = {
>> +               .name           = "m10v-pinctrl",
>> +               .of_match_table = m10v_pmatch,
>> +       },
>> +};
>> +
>> +static int __init m10v_pinctrl_init(void)
>> +{
>> +       return platform_driver_register(&m10v_pinctrl_driver);
>> +}
>> +arch_initcall(m10v_pinctrl_init);
> 
> 
> Can't it be builtin_platform_driver()?

I think using builtin_platform_driver() is no problem.

> 
> Which device requires this to be arch_initcall()?

This driver was originally a module, so no need to be arch_initcall().
I will use arch_initcall() instead.

Thanks
Sugaya Taichi

> 
> 
> 
> 
> 
> --
> Best Regards
> Masahiro Yamada
>
Sugaya Taichi Dec. 6, 2018, 8:29 a.m. UTC | #6
Hi,

Thank you for your comments.

On 2018/12/05 8:11, Rob Herring wrote:
> On Mon, Nov 19, 2018 at 10:02:12AM +0900, Sugaya Taichi wrote:
>> Add DT bindings document for Milbeaut M10V pinctrl.
>>
>> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
>> ---
>>   .../pinctrl/socionext,milbeaut-pinctrl.txt         | 33 ++++++++++++++++++++++
>>   1 file changed, 33 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt
>> new file mode 100644
>> index 0000000..7469189
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt
>> @@ -0,0 +1,33 @@
>> +Milbeaut SoCs pin controller
>> +
>> +Required properties:
>> +- compatible: should be one of the following:
>> +    "socionext,milbeaut-m10v-pinctrl"  - for m10v SoC
>> +- reg: offset and length of the register set.
>> +- reg-names: should be "pinctrl", "exiu".
>> +- gpio-cells; should be 2.
> 
> #gpio-cells
> 
> gpio-controller?

Ah yes. I forgot to add it.
Add it.

> 
>> +- interrupt-cells: should be 2.
> 
> #interrupt-cells
> 
> interrupt-controller?

Add it also.

Thanks,
Sugaya Taichi

> 
>> +- clocks: phandle to the input clock.
>> +- interrupts: three interrupts specifer.
>> +- interrupt-names: corresponds "interrupts" factor.
>> +
>> +Example:
>> +	pinctrl: pinctrl@1d022000 {
>> +		compatible = "socionext,milbeaut-m10v-pinctrl";
>> +		reg = <0x1d022000 0x1000>,
>> +			<0x1c26f000 0x1000>;
>> +		reg-names = "pinctrl", "exiu";
>> +		gpio-controller;
>> +		#gpio-cells = <2>;
>> +		interrupt-controller;
>> +		#interrupt-cells = <2>;
>> +		clocks = <&dummy_clk>;
>> +		interrupts = <0 54 4>, <0 55 4>, <0 56 4>, <0 57 4>,
>> +				<0 58 4>, <0 59 4>, <0 60 4>, <0 61 4>,
>> +				<0 62 4>, <0 63 4>, <0 64 4>, <0 65 4>,
>> +				<0 66 4>, <0 67 4>, <0 68 4>, <0 69 4>;
>> +		interrupt-names = "pin-48", "pin-49", "pin-50", "pin-51",
>> +				"pin-52", "pin-53", "pin-54", "pin-55",
>> +				"pin-56", "pin-57", "pin-58", "pin-59",
>> +				"pin-60", "pin-61", "pin-62", "pin-63";
>> +	}
>> -- 
>> 1.9.1
>>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt
new file mode 100644
index 0000000..7469189
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/socionext,milbeaut-pinctrl.txt
@@ -0,0 +1,33 @@ 
+Milbeaut SoCs pin controller
+
+Required properties:
+- compatible: should be one of the following:
+    "socionext,milbeaut-m10v-pinctrl"  - for m10v SoC
+- reg: offset and length of the register set.
+- reg-names: should be "pinctrl", "exiu".
+- gpio-cells; should be 2.
+- interrupt-cells: should be 2.
+- clocks: phandle to the input clock.
+- interrupts: three interrupts specifer.
+- interrupt-names: corresponds "interrupts" factor.
+
+Example:
+	pinctrl: pinctrl@1d022000 {
+		compatible = "socionext,milbeaut-m10v-pinctrl";
+		reg = <0x1d022000 0x1000>,
+			<0x1c26f000 0x1000>;
+		reg-names = "pinctrl", "exiu";
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		clocks = <&dummy_clk>;
+		interrupts = <0 54 4>, <0 55 4>, <0 56 4>, <0 57 4>,
+				<0 58 4>, <0 59 4>, <0 60 4>, <0 61 4>,
+				<0 62 4>, <0 63 4>, <0 64 4>, <0 65 4>,
+				<0 66 4>, <0 67 4>, <0 68 4>, <0 69 4>;
+		interrupt-names = "pin-48", "pin-49", "pin-50", "pin-51",
+				"pin-52", "pin-53", "pin-54", "pin-55",
+				"pin-56", "pin-57", "pin-58", "pin-59",
+				"pin-60", "pin-61", "pin-62", "pin-63";
+	}