Re: [PATCH] arm64: dts: sdm630 SoC and Sony Pioneer (Xperia XA2) support

2018-09-24 Thread Bjorn Andersson
On Mon 24 Sep 13:42 PDT 2018, Craig wrote:
> On 24 September 2018 20:19:29 BST, Bjorn Andersson 
>  wrote:
> >On Sat 11 Aug 09:25 PDT 2018, Craig Tatlor wrote:
> >
> >> Initial device tree support for Qualcomm SDM630 SoC and
> >> Sony Pioneer (Xperia XA2).
> >> 
> >> SDM630 is based off of the SDM660 soc and all SDM660 specific drivers
> >are
> >> compatible with it. SDM660 is also based off of MSM8998 so it uses
> >some
> >> of its drivers aswell.
> >
> >Consider adding both sdm630 and sdm660 compatibles to the bindings and
> >drivers and use the right one in the dts, in case we find details that
> >differs in the future.
> 
> This includes pinctrl and GCC?

Yes

> >
> >> 
> >> The device tree is based on the CAF 4.4 kernel tree.
> >> 
> >> The device can be booted into the initrd with a shell over UART.
> >> 
> >> Signed-off-by: Craig Tatlor 
> >[..]
> >> diff --git a/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
> >b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
> >> new file mode 100644
> >> index ..78b79c1076f1
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
> >> @@ -0,0 +1,17 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/* Copyright (c) 2018, Craig Tatlor. */
> >> +
> >> + {
> >> +  blsp1_uart1_default: blsp1_uart1_default {
> >> +  pinmux {
> >> +  pins = "gpio0", "gpio1", "gpio2", "gpio3";
> >> +  function = "gpio";
> >
> >Please put these in the sdm630.dtsi directly, rather than spreading the
> >pins out in a separate file.
> >
> Okay, just followed what 8996 did

I have started to rework that based on the last year's discussions, but
haven't posted any patches yet.

> >> +  };
> >> +
> >> +  pinconf {
> >> +  pins = "gpio0", "gpio1", "gpio2", "gpio3";
> >> +  drive-strength = <2>;
> >> +  bias-disable;
> >
> >Please extend _uart1_default in the pioneer dtsi with these
> >"electrical properties".
> Are you meaning to put this in the pioneer DTS or just drive strength and 
> bias?

The drive-strength and bias are board-specific properties, so we want to
keep those in the board file, so I mean push the pinconf part into the
board dtsi.

[..]
> >> +  gcc: clock-controller@10 {
> >> +  compatible = "qcom,gcc-sdm660";
> >> +  #clock-cells = <1>;
> >> +  #reset-cells = <1>;
> >> +  #power-domain-cells = <1>;
> >> +  reg = <0x10 0x94000>;
> >
> >Please 0-pad addresses in "reg", makes it easier to sort them as well
> >(but keep the @address after the node name unpadded)
> Sure, how much should I pad up to?

reg = <0x0010 0xb>;

Regards,
Bjorn


Re: [PATCH] arm64: dts: sdm630 SoC and Sony Pioneer (Xperia XA2) support

2018-09-24 Thread Bjorn Andersson
On Mon 24 Sep 13:42 PDT 2018, Craig wrote:
> On 24 September 2018 20:19:29 BST, Bjorn Andersson 
>  wrote:
> >On Sat 11 Aug 09:25 PDT 2018, Craig Tatlor wrote:
> >
> >> Initial device tree support for Qualcomm SDM630 SoC and
> >> Sony Pioneer (Xperia XA2).
> >> 
> >> SDM630 is based off of the SDM660 soc and all SDM660 specific drivers
> >are
> >> compatible with it. SDM660 is also based off of MSM8998 so it uses
> >some
> >> of its drivers aswell.
> >
> >Consider adding both sdm630 and sdm660 compatibles to the bindings and
> >drivers and use the right one in the dts, in case we find details that
> >differs in the future.
> 
> This includes pinctrl and GCC?

Yes

> >
> >> 
> >> The device tree is based on the CAF 4.4 kernel tree.
> >> 
> >> The device can be booted into the initrd with a shell over UART.
> >> 
> >> Signed-off-by: Craig Tatlor 
> >[..]
> >> diff --git a/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
> >b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
> >> new file mode 100644
> >> index ..78b79c1076f1
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
> >> @@ -0,0 +1,17 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/* Copyright (c) 2018, Craig Tatlor. */
> >> +
> >> + {
> >> +  blsp1_uart1_default: blsp1_uart1_default {
> >> +  pinmux {
> >> +  pins = "gpio0", "gpio1", "gpio2", "gpio3";
> >> +  function = "gpio";
> >
> >Please put these in the sdm630.dtsi directly, rather than spreading the
> >pins out in a separate file.
> >
> Okay, just followed what 8996 did

I have started to rework that based on the last year's discussions, but
haven't posted any patches yet.

> >> +  };
> >> +
> >> +  pinconf {
> >> +  pins = "gpio0", "gpio1", "gpio2", "gpio3";
> >> +  drive-strength = <2>;
> >> +  bias-disable;
> >
> >Please extend _uart1_default in the pioneer dtsi with these
> >"electrical properties".
> Are you meaning to put this in the pioneer DTS or just drive strength and 
> bias?

The drive-strength and bias are board-specific properties, so we want to
keep those in the board file, so I mean push the pinconf part into the
board dtsi.

[..]
> >> +  gcc: clock-controller@10 {
> >> +  compatible = "qcom,gcc-sdm660";
> >> +  #clock-cells = <1>;
> >> +  #reset-cells = <1>;
> >> +  #power-domain-cells = <1>;
> >> +  reg = <0x10 0x94000>;
> >
> >Please 0-pad addresses in "reg", makes it easier to sort them as well
> >(but keep the @address after the node name unpadded)
> Sure, how much should I pad up to?

reg = <0x0010 0xb>;

Regards,
Bjorn


Re: [PATCH] arm64: dts: sdm630 SoC and Sony Pioneer (Xperia XA2) support

2018-09-24 Thread Craig



On 24 September 2018 20:19:29 BST, Bjorn Andersson  
wrote:
>On Sat 11 Aug 09:25 PDT 2018, Craig Tatlor wrote:
>
>> Initial device tree support for Qualcomm SDM630 SoC and
>> Sony Pioneer (Xperia XA2).
>> 
>> SDM630 is based off of the SDM660 soc and all SDM660 specific drivers
>are
>> compatible with it. SDM660 is also based off of MSM8998 so it uses
>some
>> of its drivers aswell.
>
>Consider adding both sdm630 and sdm660 compatibles to the bindings and
>drivers and use the right one in the dts, in case we find details that
>differs in the future.

This includes pinctrl and GCC?
>
>> 
>> The device tree is based on the CAF 4.4 kernel tree.
>> 
>> The device can be booted into the initrd with a shell over UART.
>> 
>> Signed-off-by: Craig Tatlor 
>[..]
>> diff --git a/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
>b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
>> new file mode 100644
>> index ..78b79c1076f1
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
>> @@ -0,0 +1,17 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2018, Craig Tatlor. */
>> +
>> + {
>> +blsp1_uart1_default: blsp1_uart1_default {
>> +pinmux {
>> +pins = "gpio0", "gpio1", "gpio2", "gpio3";
>> +function = "gpio";
>
>Please put these in the sdm630.dtsi directly, rather than spreading the
>pins out in a separate file.
>
Okay, just followed what 8996 did
>> +};
>> +
>> +pinconf {
>> +pins = "gpio0", "gpio1", "gpio2", "gpio3";
>> +drive-strength = <2>;
>> +bias-disable;
>
>Please extend _uart1_default in the pioneer dtsi with these
>"electrical properties".
Are you meaning to put this in the pioneer DTS or just drive strength and bias?
>
>> +};
>> +};
>> +};
>[..]
>> diff --git a/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi
>b/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi
>> new file mode 100644
>> index ..512792c23369
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi
>> @@ -0,0 +1,22 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2018, Craig Tatlor. */
>> +
>> +#include "sdm630.dtsi"
>> +
>> +/ {
>> +aliases {
>> +serial0 = _uart1;
>> +};
>> +
>> +chosen {
>> +stdout-path = "serial0:115200n8";
>> +};
>> +};
>> +
>> + {
>> +serial@c17 {
>
>Please reference this by _uart1, rather than duplicating the
>hierarchy.
Okay, should the same still apply once more stuff is in soc? Or should I use 
references for them?
>
>> +status = "okay";
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_uart1_default>;
>> +};
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi
>b/arch/arm64/boot/dts/qcom/sdm630.dtsi
>> new file mode 100644
>> index ..8a544979b7c0
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
>> @@ -0,0 +1,383 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2018, Craig Tatlor. */
>> +
>> +#include 
>> +#include 
>> +
>> +/ {
>> +model = "Qualcomm Technologies, Inc. SDM630";
>
>We expect the board to always override this, so no need to specify it
>here.
Yup
>> +
>> +interrupt-parent = <>;
>> +
>> +qcom,msm-id = <318 0x0>;
>> +
>> +#address-cells = <2>;
>> +#size-cells = <2>;
>> +
>> +chosen { };
>> +
>> +memory {
>> +device_type = "memory";
>> +/* We expect the bootloader to fill in the reg */
>> +reg = <0 0 0 0>;
>> +};
>> +
>> +
>
>Extra empty line.
Okay
>> +cpus {
>[..]
>> +
>> +timer {
>
>Please sort these nodes by name, except for "soc" which is convenient
>to have last.
Okay
>
>> +compatible = "arm,armv8-timer";
>> +interrupts = ,
>> + ,
>> + ,
>> + ;
>> +};
>[..]
>> +firmware {
>> +scm {
>> +compatible = "qcom,scm-sdm660";
>> +};
>> +};
>> +
>> +
>
>Extra empty line.
Okay
>
>> +rpm-glink {
>> +compatible = "qcom,glink-rpm";
>> +
>> +interrupts = ;
>> +
>> +qcom,rpm-msg-ram = <_msg_ram>;
>> +
>> +mboxes = <_glb 0>;
>
>Remove a few of these extra empty lines and add the rpm_requests
>channel
>here while you're at it:
>
Sure
>   rpm_requests: glink-channel {
>compatible = "qcom,rpm-sdm660";
>qcom,glink-channels = "rpm_requests";
>};
>
>> +};
>> +
>> +soc: soc {
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges = <0 0 0 0x>;
>> +compatible = "simple-bus";
>> +
>> +intc: interrupt-controller@17a0 {
>
>Please sort these nodes by base address.
>
Will do
>> +compatible = "arm,gic-v3";
>> +reg 

Re: [PATCH] arm64: dts: sdm630 SoC and Sony Pioneer (Xperia XA2) support

2018-09-24 Thread Craig



On 24 September 2018 20:19:29 BST, Bjorn Andersson  
wrote:
>On Sat 11 Aug 09:25 PDT 2018, Craig Tatlor wrote:
>
>> Initial device tree support for Qualcomm SDM630 SoC and
>> Sony Pioneer (Xperia XA2).
>> 
>> SDM630 is based off of the SDM660 soc and all SDM660 specific drivers
>are
>> compatible with it. SDM660 is also based off of MSM8998 so it uses
>some
>> of its drivers aswell.
>
>Consider adding both sdm630 and sdm660 compatibles to the bindings and
>drivers and use the right one in the dts, in case we find details that
>differs in the future.

This includes pinctrl and GCC?
>
>> 
>> The device tree is based on the CAF 4.4 kernel tree.
>> 
>> The device can be booted into the initrd with a shell over UART.
>> 
>> Signed-off-by: Craig Tatlor 
>[..]
>> diff --git a/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
>b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
>> new file mode 100644
>> index ..78b79c1076f1
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
>> @@ -0,0 +1,17 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2018, Craig Tatlor. */
>> +
>> + {
>> +blsp1_uart1_default: blsp1_uart1_default {
>> +pinmux {
>> +pins = "gpio0", "gpio1", "gpio2", "gpio3";
>> +function = "gpio";
>
>Please put these in the sdm630.dtsi directly, rather than spreading the
>pins out in a separate file.
>
Okay, just followed what 8996 did
>> +};
>> +
>> +pinconf {
>> +pins = "gpio0", "gpio1", "gpio2", "gpio3";
>> +drive-strength = <2>;
>> +bias-disable;
>
>Please extend _uart1_default in the pioneer dtsi with these
>"electrical properties".
Are you meaning to put this in the pioneer DTS or just drive strength and bias?
>
>> +};
>> +};
>> +};
>[..]
>> diff --git a/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi
>b/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi
>> new file mode 100644
>> index ..512792c23369
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi
>> @@ -0,0 +1,22 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2018, Craig Tatlor. */
>> +
>> +#include "sdm630.dtsi"
>> +
>> +/ {
>> +aliases {
>> +serial0 = _uart1;
>> +};
>> +
>> +chosen {
>> +stdout-path = "serial0:115200n8";
>> +};
>> +};
>> +
>> + {
>> +serial@c17 {
>
>Please reference this by _uart1, rather than duplicating the
>hierarchy.
Okay, should the same still apply once more stuff is in soc? Or should I use 
references for them?
>
>> +status = "okay";
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_uart1_default>;
>> +};
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi
>b/arch/arm64/boot/dts/qcom/sdm630.dtsi
>> new file mode 100644
>> index ..8a544979b7c0
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
>> @@ -0,0 +1,383 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2018, Craig Tatlor. */
>> +
>> +#include 
>> +#include 
>> +
>> +/ {
>> +model = "Qualcomm Technologies, Inc. SDM630";
>
>We expect the board to always override this, so no need to specify it
>here.
Yup
>> +
>> +interrupt-parent = <>;
>> +
>> +qcom,msm-id = <318 0x0>;
>> +
>> +#address-cells = <2>;
>> +#size-cells = <2>;
>> +
>> +chosen { };
>> +
>> +memory {
>> +device_type = "memory";
>> +/* We expect the bootloader to fill in the reg */
>> +reg = <0 0 0 0>;
>> +};
>> +
>> +
>
>Extra empty line.
Okay
>> +cpus {
>[..]
>> +
>> +timer {
>
>Please sort these nodes by name, except for "soc" which is convenient
>to have last.
Okay
>
>> +compatible = "arm,armv8-timer";
>> +interrupts = ,
>> + ,
>> + ,
>> + ;
>> +};
>[..]
>> +firmware {
>> +scm {
>> +compatible = "qcom,scm-sdm660";
>> +};
>> +};
>> +
>> +
>
>Extra empty line.
Okay
>
>> +rpm-glink {
>> +compatible = "qcom,glink-rpm";
>> +
>> +interrupts = ;
>> +
>> +qcom,rpm-msg-ram = <_msg_ram>;
>> +
>> +mboxes = <_glb 0>;
>
>Remove a few of these extra empty lines and add the rpm_requests
>channel
>here while you're at it:
>
Sure
>   rpm_requests: glink-channel {
>compatible = "qcom,rpm-sdm660";
>qcom,glink-channels = "rpm_requests";
>};
>
>> +};
>> +
>> +soc: soc {
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges = <0 0 0 0x>;
>> +compatible = "simple-bus";
>> +
>> +intc: interrupt-controller@17a0 {
>
>Please sort these nodes by base address.
>
Will do
>> +compatible = "arm,gic-v3";
>> +reg 

Re: [PATCH] arm64: dts: sdm630 SoC and Sony Pioneer (Xperia XA2) support

2018-09-24 Thread Bjorn Andersson
On Sat 11 Aug 09:25 PDT 2018, Craig Tatlor wrote:

> Initial device tree support for Qualcomm SDM630 SoC and
> Sony Pioneer (Xperia XA2).
> 
> SDM630 is based off of the SDM660 soc and all SDM660 specific drivers are
> compatible with it. SDM660 is also based off of MSM8998 so it uses some
> of its drivers aswell.

Consider adding both sdm630 and sdm660 compatibles to the bindings and
drivers and use the right one in the dts, in case we find details that
differs in the future.

> 
> The device tree is based on the CAF 4.4 kernel tree.
> 
> The device can be booted into the initrd with a shell over UART.
> 
> Signed-off-by: Craig Tatlor 
[..]
> diff --git a/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi 
> b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
> new file mode 100644
> index ..78b79c1076f1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018, Craig Tatlor. */
> +
> + {
> + blsp1_uart1_default: blsp1_uart1_default {
> + pinmux {
> + pins = "gpio0", "gpio1", "gpio2", "gpio3";
> + function = "gpio";

Please put these in the sdm630.dtsi directly, rather than spreading the
pins out in a separate file.

> + };
> +
> + pinconf {
> + pins = "gpio0", "gpio1", "gpio2", "gpio3";
> + drive-strength = <2>;
> + bias-disable;

Please extend _uart1_default in the pioneer dtsi with these
"electrical properties".

> + };
> + };
> +};
[..]
> diff --git a/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi 
> b/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi
> new file mode 100644
> index ..512792c23369
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018, Craig Tatlor. */
> +
> +#include "sdm630.dtsi"
> +
> +/ {
> + aliases {
> + serial0 = _uart1;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +};
> +
> + {
> + serial@c17 {

Please reference this by _uart1, rather than duplicating the
hierarchy.

> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <_uart1_default>;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi 
> b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> new file mode 100644
> index ..8a544979b7c0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> @@ -0,0 +1,383 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018, Craig Tatlor. */
> +
> +#include 
> +#include 
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. SDM630";

We expect the board to always override this, so no need to specify it
here.

> +
> + interrupt-parent = <>;
> +
> + qcom,msm-id = <318 0x0>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen { };
> +
> + memory {
> + device_type = "memory";
> + /* We expect the bootloader to fill in the reg */
> + reg = <0 0 0 0>;
> + };
> +
> +

Extra empty line.

> + cpus {
[..]
> +
> + timer {

Please sort these nodes by name, except for "soc" which is convenient
to have last.

> + compatible = "arm,armv8-timer";
> + interrupts = ,
> +  ,
> +  ,
> +  ;
> + };
[..]
> + firmware {
> + scm {
> + compatible = "qcom,scm-sdm660";
> + };
> + };
> +
> +

Extra empty line.

> + rpm-glink {
> + compatible = "qcom,glink-rpm";
> +
> + interrupts = ;
> +
> + qcom,rpm-msg-ram = <_msg_ram>;
> +
> + mboxes = <_glb 0>;

Remove a few of these extra empty lines and add the rpm_requests channel
here while you're at it:

rpm_requests: glink-channel {
compatible = "qcom,rpm-sdm660";
qcom,glink-channels = "rpm_requests";
};

> + };
> +
> + soc: soc {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0 0 0x>;
> + compatible = "simple-bus";
> +
> + intc: interrupt-controller@17a0 {

Please sort these nodes by base address.

> + compatible = "arm,gic-v3";
> + reg = <0x17a0 0x1>,
> +   <0x17b0 0x10>;
> + #interrupt-cells = <3>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + interrupt-controller;
> + #redistributor-regions = <1>;
> + redistributor-stride = <0x0 0x2>;
> + interrupts = ;
> + 

Re: [PATCH] arm64: dts: sdm630 SoC and Sony Pioneer (Xperia XA2) support

2018-09-24 Thread Bjorn Andersson
On Sat 11 Aug 09:25 PDT 2018, Craig Tatlor wrote:

> Initial device tree support for Qualcomm SDM630 SoC and
> Sony Pioneer (Xperia XA2).
> 
> SDM630 is based off of the SDM660 soc and all SDM660 specific drivers are
> compatible with it. SDM660 is also based off of MSM8998 so it uses some
> of its drivers aswell.

Consider adding both sdm630 and sdm660 compatibles to the bindings and
drivers and use the right one in the dts, in case we find details that
differs in the future.

> 
> The device tree is based on the CAF 4.4 kernel tree.
> 
> The device can be booted into the initrd with a shell over UART.
> 
> Signed-off-by: Craig Tatlor 
[..]
> diff --git a/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi 
> b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
> new file mode 100644
> index ..78b79c1076f1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm630-pins.dtsi
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018, Craig Tatlor. */
> +
> + {
> + blsp1_uart1_default: blsp1_uart1_default {
> + pinmux {
> + pins = "gpio0", "gpio1", "gpio2", "gpio3";
> + function = "gpio";

Please put these in the sdm630.dtsi directly, rather than spreading the
pins out in a separate file.

> + };
> +
> + pinconf {
> + pins = "gpio0", "gpio1", "gpio2", "gpio3";
> + drive-strength = <2>;
> + bias-disable;

Please extend _uart1_default in the pioneer dtsi with these
"electrical properties".

> + };
> + };
> +};
[..]
> diff --git a/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi 
> b/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi
> new file mode 100644
> index ..512792c23369
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018, Craig Tatlor. */
> +
> +#include "sdm630.dtsi"
> +
> +/ {
> + aliases {
> + serial0 = _uart1;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +};
> +
> + {
> + serial@c17 {

Please reference this by _uart1, rather than duplicating the
hierarchy.

> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <_uart1_default>;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi 
> b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> new file mode 100644
> index ..8a544979b7c0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> @@ -0,0 +1,383 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018, Craig Tatlor. */
> +
> +#include 
> +#include 
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. SDM630";

We expect the board to always override this, so no need to specify it
here.

> +
> + interrupt-parent = <>;
> +
> + qcom,msm-id = <318 0x0>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen { };
> +
> + memory {
> + device_type = "memory";
> + /* We expect the bootloader to fill in the reg */
> + reg = <0 0 0 0>;
> + };
> +
> +

Extra empty line.

> + cpus {
[..]
> +
> + timer {

Please sort these nodes by name, except for "soc" which is convenient
to have last.

> + compatible = "arm,armv8-timer";
> + interrupts = ,
> +  ,
> +  ,
> +  ;
> + };
[..]
> + firmware {
> + scm {
> + compatible = "qcom,scm-sdm660";
> + };
> + };
> +
> +

Extra empty line.

> + rpm-glink {
> + compatible = "qcom,glink-rpm";
> +
> + interrupts = ;
> +
> + qcom,rpm-msg-ram = <_msg_ram>;
> +
> + mboxes = <_glb 0>;

Remove a few of these extra empty lines and add the rpm_requests channel
here while you're at it:

rpm_requests: glink-channel {
compatible = "qcom,rpm-sdm660";
qcom,glink-channels = "rpm_requests";
};

> + };
> +
> + soc: soc {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0 0 0x>;
> + compatible = "simple-bus";
> +
> + intc: interrupt-controller@17a0 {

Please sort these nodes by base address.

> + compatible = "arm,gic-v3";
> + reg = <0x17a0 0x1>,
> +   <0x17b0 0x10>;
> + #interrupt-cells = <3>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + interrupt-controller;
> + #redistributor-regions = <1>;
> + redistributor-stride = <0x0 0x2>;
> + interrupts = ;
> + 

Re: [PATCH] arm64: dts: sdm630 SoC and Sony Pioneer (Xperia XA2) support

2018-08-11 Thread kbuild test robot
Hi Craig,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on agross/for-next]
[also build test ERROR on v4.18-rc8 next-20180810]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Craig-Tatlor/arm64-dts-sdm630-SoC-and-Sony-Pioneer-Xperia-XA2-support/20180812-061913
base:   https://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git 
for-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   In file included from arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi:4:0,
from arch/arm64/boot/dts/qcom/sdm630-pioneer.dts:6:
>> arch/arm64/boot/dts/qcom/sdm630.dtsi:5:10: fatal error: 
>> dt-bindings/clock/qcom,gcc-sdm660.h: No such file or directory
#include 
 ^
   compilation terminated.

vim +5 arch/arm64/boot/dts/qcom/sdm630.dtsi

 3  
 4  #include 
   > 5  #include 
 6  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip


Re: [PATCH] arm64: dts: sdm630 SoC and Sony Pioneer (Xperia XA2) support

2018-08-11 Thread kbuild test robot
Hi Craig,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on agross/for-next]
[also build test ERROR on v4.18-rc8 next-20180810]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Craig-Tatlor/arm64-dts-sdm630-SoC-and-Sony-Pioneer-Xperia-XA2-support/20180812-061913
base:   https://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git 
for-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   In file included from arch/arm64/boot/dts/qcom/sdm630-pioneer.dtsi:4:0,
from arch/arm64/boot/dts/qcom/sdm630-pioneer.dts:6:
>> arch/arm64/boot/dts/qcom/sdm630.dtsi:5:10: fatal error: 
>> dt-bindings/clock/qcom,gcc-sdm660.h: No such file or directory
#include 
 ^
   compilation terminated.

vim +5 arch/arm64/boot/dts/qcom/sdm630.dtsi

 3  
 4  #include 
   > 5  #include 
 6  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip