All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: "Duje Mihanović" <duje.mihanovic@skole.hr>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Kees Cook" <keescook@chromium.org>,
	"Tony Luck" <tony.luck@intel.com>,
	"Guilherme G. Piccoli" <gpiccoli@igalia.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-hardening@vger.kernel.org
Cc: ~postmarketos/upstreaming@lists.sr.ht,
	phone-devel@vger.kernel.org, afaerber@suse.com
Subject: Re: [PATCH 08/10] arm64: dts: Add DTS for Marvell PXA1908 and samsung,coreprimevelte
Date: Sat, 22 Jul 2023 11:25:41 +0200	[thread overview]
Message-ID: <e4024f81-2d05-f02c-020c-e0a83f9fc68c@linaro.org> (raw)
In-Reply-To: <20230721210042.21535-9-duje.mihanovic@skole.hr>

On 21/07/2023 22:37, Duje Mihanović wrote:
> Add DTS for Marvell PXA1908 SoC and Samsung Galaxy Core Prime Value
> Edition LTE, a smartphone based on said SoC.
> 
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
>  arch/arm64/boot/dts/marvell/Makefile          |   1 +
>  .../pxa1908-samsung-coreprimevelte.dts        | 321 ++++++++++++++++++
>  arch/arm64/boot/dts/marvell/pxa1908.dtsi      | 298 ++++++++++++++++
>  3 files changed, 620 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dts
>  create mode 100644 arch/arm64/boot/dts/marvell/pxa1908.dtsi
> 
> diff --git a/arch/arm64/boot/dts/marvell/Makefile b/arch/arm64/boot/dts/marvell/Makefile
> index 79ac09b58a89..0e277a0d368b 100644
> --- a/arch/arm64/boot/dts/marvell/Makefile
> +++ b/arch/arm64/boot/dts/marvell/Makefile
> @@ -27,3 +27,4 @@ dtb-$(CONFIG_ARCH_MVEBU) += cn9132-db-B.dtb
>  dtb-$(CONFIG_ARCH_MVEBU) += cn9130-crb-A.dtb
>  dtb-$(CONFIG_ARCH_MVEBU) += cn9130-crb-B.dtb
>  dtb-$(CONFIG_ARCH_MVEBU) += ac5-98dx35xx-rd.dtb
> +dtb-$(CONFIG_ARCH_MMP) += pxa1908-samsung-coreprimevelte.dtb
> diff --git a/arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dts b/arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dts
> new file mode 100644
> index 000000000000..3e10a77a106e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dts
> @@ -0,0 +1,321 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include "pxa1908.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/linux-event-codes.h>
> +
> +/ {
> +	pxa,rev-id = <3928 2>;

Drop. This is not documented.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).


> +	model = "Samsung Galaxy Core Prime VE LTE";
> +	compatible = "samsung,coreprimevelte", "marvell,pxa1908";

Missing bindings.

> +
> +	/* Bootloader fills this in */
> +	memory {
> +		device_type = "memory";
> +		reg = <0 0 0 0>;
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		framebuffer@17000000 {
> +			reg = <0 0x17000000 0 0x1800000>;
> +			no-map;
> +		};
> +
> +		gpu@9000000 {
> +			reg = <0 0x9000000 0 0x1000000>;
> +		};
> +
> +		/* Communications processor, aka modem */
> +		cp@3000000 {
> +			reg = <0 0x3000000 0 0x5000000>;
> +		};
> +
> +		cm3@a000000 {
> +			reg = <0 0xa000000 0 0x80000>;
> +		};
> +
> +		seclog@8000000 {
> +			reg = <0 0x8000000 0 0x100000>;
> +		};
> +
> +		ramoops@8100000 {
> +			compatible = "ramoops";
> +			reg = <0 0x8100000 0 0x40000>;
> +			record-size = <0x8000>;
> +			console-size = <0x20000>;
> +			max-reason = <5>;
> +		};
> +	};
> +
> +	fb0: framebuffer@17177000 {
> +		compatible = "simple-framebuffer";
> +		reg = <0 0x17177000 0 (480 * 800 * 4)>;
> +		width = <480>;
> +		height = <800>;
> +		stride = <(480 * 4)>;
> +		format = "a8r8g8b8";
> +	};
> +
> +	muic-i2c {
> +		compatible = "i2c-gpio";
> +		sda-gpios = <&gpio 30 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +		scl-gpios = <&gpio 29 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +		i2c-gpio,delay-us = <3>;
> +		i2c-gpio,timeout-ms = <100>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&muic_i2c_pins>;
> +
> +		muic: extcon@14 {
> +			compatible = "siliconmitus,sm5504-muic";
> +			reg = <0x14>;
> +			interrupt-parent = <&gpio>;
> +			interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
> +		};
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&gpio_keys_pins>;
> +		autorepeat;
> +
> +		key-home {
> +			label = "Home";
> +			linux,code = <KEY_HOME>;
> +			gpios = <&gpio 50 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		key-volup {
> +			label = "Volume Up";
> +			linux,code = <KEY_VOLUMEUP>;
> +			gpios = <&gpio 16 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		key-voldown {
> +			label = "Volume Down";
> +			linux,code = <KEY_VOLUMEDOWN>;
> +			gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +	chosen {};

chosen is usually at the top (as alphabetical order suggests)

> +};
> +
> +&smmu {
> +	status = "okay";
> +};
> +
> +&pmx {
> +	pinctrl-single,gpio-range = <&range 55 55 0>,
> +				    <&range 110 32 0>,
> +				    <&range 52 1 0>;
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&board_pins_1 &board_pins_2 &board_pins_3>;
> +
> +	board_pins_1: pinmux_board_1 {

No underscores in node names.

...

> diff --git a/arch/arm64/boot/dts/marvell/pxa1908.dtsi b/arch/arm64/boot/dts/marvell/pxa1908.dtsi
> new file mode 100644
> index 000000000000..7131b2070b72
> --- /dev/null
> +++ b/arch/arm64/boot/dts/marvell/pxa1908.dtsi
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/dts-v1/;
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/marvell,pxa1908.h>
> +
> +/ {
> +	model = "Marvell Armada PXA1908";
> +	compatible = "marvell,pxa1908";

Undocumented compatible.

Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.


Best regards,
Krzysztof


WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: "Duje Mihanović" <duje.mihanovic@skole.hr>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Kees Cook" <keescook@chromium.org>,
	"Tony Luck" <tony.luck@intel.com>,
	"Guilherme G. Piccoli" <gpiccoli@igalia.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-hardening@vger.kernel.org
Cc: ~postmarketos/upstreaming@lists.sr.ht,
	phone-devel@vger.kernel.org, afaerber@suse.com
Subject: Re: [PATCH 08/10] arm64: dts: Add DTS for Marvell PXA1908 and samsung,coreprimevelte
Date: Sat, 22 Jul 2023 11:25:41 +0200	[thread overview]
Message-ID: <e4024f81-2d05-f02c-020c-e0a83f9fc68c@linaro.org> (raw)
In-Reply-To: <20230721210042.21535-9-duje.mihanovic@skole.hr>

On 21/07/2023 22:37, Duje Mihanović wrote:
> Add DTS for Marvell PXA1908 SoC and Samsung Galaxy Core Prime Value
> Edition LTE, a smartphone based on said SoC.
> 
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
>  arch/arm64/boot/dts/marvell/Makefile          |   1 +
>  .../pxa1908-samsung-coreprimevelte.dts        | 321 ++++++++++++++++++
>  arch/arm64/boot/dts/marvell/pxa1908.dtsi      | 298 ++++++++++++++++
>  3 files changed, 620 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dts
>  create mode 100644 arch/arm64/boot/dts/marvell/pxa1908.dtsi
> 
> diff --git a/arch/arm64/boot/dts/marvell/Makefile b/arch/arm64/boot/dts/marvell/Makefile
> index 79ac09b58a89..0e277a0d368b 100644
> --- a/arch/arm64/boot/dts/marvell/Makefile
> +++ b/arch/arm64/boot/dts/marvell/Makefile
> @@ -27,3 +27,4 @@ dtb-$(CONFIG_ARCH_MVEBU) += cn9132-db-B.dtb
>  dtb-$(CONFIG_ARCH_MVEBU) += cn9130-crb-A.dtb
>  dtb-$(CONFIG_ARCH_MVEBU) += cn9130-crb-B.dtb
>  dtb-$(CONFIG_ARCH_MVEBU) += ac5-98dx35xx-rd.dtb
> +dtb-$(CONFIG_ARCH_MMP) += pxa1908-samsung-coreprimevelte.dtb
> diff --git a/arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dts b/arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dts
> new file mode 100644
> index 000000000000..3e10a77a106e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/marvell/pxa1908-samsung-coreprimevelte.dts
> @@ -0,0 +1,321 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include "pxa1908.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/linux-event-codes.h>
> +
> +/ {
> +	pxa,rev-id = <3928 2>;

Drop. This is not documented.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).


> +	model = "Samsung Galaxy Core Prime VE LTE";
> +	compatible = "samsung,coreprimevelte", "marvell,pxa1908";

Missing bindings.

> +
> +	/* Bootloader fills this in */
> +	memory {
> +		device_type = "memory";
> +		reg = <0 0 0 0>;
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		framebuffer@17000000 {
> +			reg = <0 0x17000000 0 0x1800000>;
> +			no-map;
> +		};
> +
> +		gpu@9000000 {
> +			reg = <0 0x9000000 0 0x1000000>;
> +		};
> +
> +		/* Communications processor, aka modem */
> +		cp@3000000 {
> +			reg = <0 0x3000000 0 0x5000000>;
> +		};
> +
> +		cm3@a000000 {
> +			reg = <0 0xa000000 0 0x80000>;
> +		};
> +
> +		seclog@8000000 {
> +			reg = <0 0x8000000 0 0x100000>;
> +		};
> +
> +		ramoops@8100000 {
> +			compatible = "ramoops";
> +			reg = <0 0x8100000 0 0x40000>;
> +			record-size = <0x8000>;
> +			console-size = <0x20000>;
> +			max-reason = <5>;
> +		};
> +	};
> +
> +	fb0: framebuffer@17177000 {
> +		compatible = "simple-framebuffer";
> +		reg = <0 0x17177000 0 (480 * 800 * 4)>;
> +		width = <480>;
> +		height = <800>;
> +		stride = <(480 * 4)>;
> +		format = "a8r8g8b8";
> +	};
> +
> +	muic-i2c {
> +		compatible = "i2c-gpio";
> +		sda-gpios = <&gpio 30 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +		scl-gpios = <&gpio 29 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +		i2c-gpio,delay-us = <3>;
> +		i2c-gpio,timeout-ms = <100>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&muic_i2c_pins>;
> +
> +		muic: extcon@14 {
> +			compatible = "siliconmitus,sm5504-muic";
> +			reg = <0x14>;
> +			interrupt-parent = <&gpio>;
> +			interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
> +		};
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&gpio_keys_pins>;
> +		autorepeat;
> +
> +		key-home {
> +			label = "Home";
> +			linux,code = <KEY_HOME>;
> +			gpios = <&gpio 50 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		key-volup {
> +			label = "Volume Up";
> +			linux,code = <KEY_VOLUMEUP>;
> +			gpios = <&gpio 16 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		key-voldown {
> +			label = "Volume Down";
> +			linux,code = <KEY_VOLUMEDOWN>;
> +			gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +	chosen {};

chosen is usually at the top (as alphabetical order suggests)

> +};
> +
> +&smmu {
> +	status = "okay";
> +};
> +
> +&pmx {
> +	pinctrl-single,gpio-range = <&range 55 55 0>,
> +				    <&range 110 32 0>,
> +				    <&range 52 1 0>;
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&board_pins_1 &board_pins_2 &board_pins_3>;
> +
> +	board_pins_1: pinmux_board_1 {

No underscores in node names.

...

> diff --git a/arch/arm64/boot/dts/marvell/pxa1908.dtsi b/arch/arm64/boot/dts/marvell/pxa1908.dtsi
> new file mode 100644
> index 000000000000..7131b2070b72
> --- /dev/null
> +++ b/arch/arm64/boot/dts/marvell/pxa1908.dtsi
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/dts-v1/;
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/marvell,pxa1908.h>
> +
> +/ {
> +	model = "Marvell Armada PXA1908";
> +	compatible = "marvell,pxa1908";

Undocumented compatible.

Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.


Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-07-22  9:25 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-21 20:37 [PATCH 00/10] Initial Marvell PXA1908 support Duje Mihanović
2023-07-21 20:37 ` Duje Mihanović
2023-07-21 20:37 ` [PATCH 01/10] tty: serial: 8250: Define earlycon for mrvl,mmp-uart Duje Mihanović
2023-07-21 20:37 ` [PATCH 02/10] gpio: pxa: use dynamic allocation of base Duje Mihanović
2023-07-24  9:00   ` Andy Shevchenko
2023-07-21 20:37 ` [PATCH 03/10] gpio: pxa: make pxa_gpio_has_pinctrl return false for MMP_GPIO Duje Mihanović
2023-07-24  9:01   ` Andy Shevchenko
2023-07-21 20:37 ` [PATCH 04/10] clk: mmp: Add Marvell PXA1908 clock driver Duje Mihanović
2023-07-21 20:37   ` Duje Mihanović
2023-07-24  9:05   ` Andy Shevchenko
2023-07-21 20:37 ` [PATCH 05/10] dt-bindings: clock: Add Marvell PXA1908 clock bindings Duje Mihanović
2023-07-21 20:37   ` Duje Mihanović
2023-07-22  9:21   ` Krzysztof Kozlowski
2023-07-22  9:21     ` Krzysztof Kozlowski
2023-07-21 20:37 ` [PATCH 06/10] dt-bindings: clock: Add documentation for Marvell PXA1908 Duje Mihanović
2023-07-22  9:23   ` Krzysztof Kozlowski
2023-07-21 20:37 ` [PATCH 07/10] arm64: Kconfig.platforms: Add config for Marvell PXA1908 platform Duje Mihanović
2023-07-21 20:37   ` Duje Mihanović
2023-07-21 20:37 ` [PATCH 08/10] arm64: dts: Add DTS for Marvell PXA1908 and samsung,coreprimevelte Duje Mihanović
2023-07-21 20:37   ` Duje Mihanović
2023-07-22  9:25   ` Krzysztof Kozlowski [this message]
2023-07-22  9:25     ` Krzysztof Kozlowski
2023-07-21 20:37 ` [PATCH 09/10] dt-bindings: marvell: Document PXA1908 SoC Duje Mihanović
2023-07-21 20:37   ` Duje Mihanović
2023-07-21 22:28   ` Rob Herring
2023-07-21 22:28     ` Rob Herring
2023-07-22  9:27   ` Krzysztof Kozlowski
2023-07-22  9:27     ` Krzysztof Kozlowski
2023-07-22 21:52     ` Duje Mihanović
2023-07-24 14:14     ` Rob Herring
2023-07-24 21:37       ` Duje Mihanović
2023-07-21 20:37 ` [PATCH 10/10] MAINTAINERS: add myself as Marvell PXA1908 maintainer Duje Mihanović

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e4024f81-2d05-f02c-020c-e0a83f9fc68c@linaro.org \
    --to=krzysztof.kozlowski@linaro.org \
    --cc=afaerber@suse.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=duje.mihanovic@skole.hr \
    --cc=gpiccoli@igalia.com \
    --cc=keescook@chromium.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=phone-devel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=tony.luck@intel.com \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.