Received-SPF: pass (google.com: domain of linux-kernel+bounces-195318-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <4cc234db-b795-4cfc-8e47-e89642f932f5@salutedevices.com>
Date: Thu, 30 May 2024 14:50:03 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [DMARC error][DKIM error] [PATCH v6 2/2] arm64: dts: amlogic: Add
 Amlogic S4 PWM
To: <kelvin.zhang@amlogic.com>
CC: Jerome Brunet <jbrunet@baylibre.com>, Krzysztof Kozlowski
	<krzk+dt@kernel.org>, Kevin Hilman <khilman@baylibre.com>, Neil Armstrong
	<neil.armstrong@linaro.org>, =?UTF-8?Q?Uwe_Kleine-K=C3=B6nig?=
	<u.kleine-koenig@pengutronix.de>, Conor Dooley <conor+dt@kernel.org>,
	<linux-pwm@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>,
	<linux-amlogic@lists.infradead.org>, <linux-kernel@vger.kernel.org>, Martin
 Blumenstingl <martin.blumenstingl@googlemail.com>,
	<devicetree@vger.kernel.org>, Junyi Zhao <junyi.zhao@amlogic.com>, Rob
 Herring <robh@kernel.org>, "kernel@salutedevices.com"
	<kernel@salutedevices.com>
References: <20240529-s4-pwm-v6-0-270f63049f20@amlogic.com>
 <20240529-s4-pwm-v6-2-270f63049f20@amlogic.com>
Content-Language: en-US
From: George Stark <gnstark@salutedevices.com>
In-Reply-To: <20240529-s4-pwm-v6-2-270f63049f20@amlogic.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 5/29/24 13:00, Kelvin Zhang via B4 Relay wrote:
> From: Junyi Zhao <junyi.zhao@amlogic.com>
> 
> Add device nodes for PWM_AB, PWM_CD, PWM_EF, PWM_GH and PWM_IJ
> along with GPIO PIN configs of each channel.
> 
> Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
> Signed-off-by: Kelvin Zhang <kelvin.zhang@amlogic.com>
> ---
>   arch/arm64/boot/dts/amlogic/meson-s4.dtsi | 207 ++++++++++++++++++++++++++++++
>   1 file changed, 207 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> index 10896f9df682..98f554577bae 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> @@ -312,6 +312,168 @@ mux {
>   					};
>   				};
>   
> +				pwm_a_pins1: pwm-a-pins1 {
> +					mux {
> +						groups = "pwm_a_d";
> +						function = "pwm_a";
> +					};
> +				};
> +
> +				pwm_a_pins2: pwm-a-pins2 {
> +					mux {
> +						groups = "pwm_a_x";
> +						function = "pwm_a";
> +					};
> +				};
> +
> +				pwm_a_pins: pwm-a-pins {
> +					mux {
> +						groups = "pwm_a_d";
> +						function = "pwm_a";
> +					};
> +				};
pwm_a_pins is just a copy of pwm_a_pins1 node
> +
> +				pwm_b_pins1: pwm-b-pins1 {
> +					mux {
> +						groups = "pwm_b_d";
> +						function = "pwm_b";
> +					};
> +				};
> +
> +				pwm_b_pins2: pwm-b-pins2 {
> +					mux {
> +						groups = "pwm_b_x";
> +						function = "pwm_b";
> +					};
> +				};
> +
> +				pwm_c_pins1: pwm-c-pins1 {
> +					mux {
> +						groups = "pwm_c_d";
> +						function = "pwm_c";
> +					};
> +				};
> +
> +				pwm_c_pins2: pwm-c-pins2 {
> +					mux {
> +						groups = "pwm_c_x";
> +						function = "pwm_c";
> +					};
> +				};
> +
> +				pwm_d_pins1: pwm-d-pins1 {
> +					mux {
> +						groups = "pwm_d_d";
> +						function = "pwm_d";
> +					};
> +				};
> +
> +				pwm_d_pins2: pwm-d-pins2 {
> +					mux {
> +						groups = "pwm_d_h";
> +						function = "pwm_d";
> +					};
> +				};
> +
> +				pwm_e_pins1: pwm-e-pins1 {
> +					mux {
> +						groups = "pwm_e_x";
> +						function = "pwm_e";
> +						drive-strength-microamp = <500>;
AFAIU GPIOX_16 (groups = "pwm_e_x") is frequently used to generate
clock for wifi module and drive-strength-microamp property here is 
needed only for that special case. If so then should that property be 
put in board dts file instead?
> +					};
> +				};
> +
> +				pwm_e_pins2: pwm-e-pins2 {
> +					mux {
> +						groups = "pwm_e_z";
> +						function = "pwm_e";
> +					};
> +				};
> +
> +				pwm_f_pins1: pwm-f-pins1 {
> +					mux {
> +						groups = "pwm_f_x";
> +						function = "pwm_f";
> +					};
> +				};
> +
> +				pwm_f_pins2: pwm-f-pins2 {
> +					mux {
> +						groups = "pwm_f_z";
> +						function = "pwm_f";
> +					};
> +				};
> +
> +				pwm_g_pins1: pwm-g-pins1 {
> +					mux {
> +						groups = "pwm_g_d";
> +						function = "pwm_g";
> +					};
> +				};
> +
> +				pwm_g_pins2: pwm-g-pins2 {
> +					mux {
> +						groups = "pwm_g_z";
> +						function = "pwm_g";
> +					};
> +				};
> +
> +				pwm_h_pins: pwm-h-pins {
> +					mux {
> +						groups = "pwm_h";
> +						function = "pwm_h";
> +					};
> +				};
> +
> +				pwm_i_pins1: pwm-i-pins1 {
> +					mux {
> +						groups = "pwm_i_d";
> +						function = "pwm_i";
> +					};
> +				};
> +
> +				pwm_i_pins2: pwm-i-pins2 {
> +					mux {
> +						groups = "pwm_i_h";
> +						function = "pwm_i";
> +					};
> +				};
> +
> +				pwm_j_pins: pwm-j-pins {
> +					mux {
> +						groups = "pwm_j";
> +						function = "pwm_j";
> +					};
> +				};
> +
> +				pwm_a_hiz_pins: pwm-a-hiz-pins {
> +					mux {
> +						groups = "pwm_a_hiz";
> +						function = "pwm_a_hiz";
> +					};
> +				};
> +
> +				pwm_b_hiz_pins: pwm-b-hiz-pins {
> +					mux {
> +						groups = "pwm_b_hiz";
> +						function = "pwm_b_hiz";
> +					};
> +				};
> +
> +				pwm_c_hiz_pins: pwm-c-hiz-pins {
> +					mux {
> +						groups = "pwm_c_hiz";
> +						function = "pwm_b_hiz";
Should it be function = "pwm_c_hiz"?
> +					};
> +				};
> +
> +				pwm_g_hiz_pins: pwm-g-hiz-pins {
> +					mux {
> +						groups = "pwm_g_hiz";
> +						function = "pwm_g_hiz";
> +					};
> +				};
> +
>   				spicc0_pins_x: spicc0-pins_x {
>   					mux {
>   						groups = "spi_a_mosi_x",
> @@ -399,6 +561,51 @@ spicc0: spi@50000 {
>   				status = "disabled";
>   			};
>   
> +			pwm_ab: pwm@58000 {
> +				compatible = "amlogic,meson-s4-pwm";
> +				reg = <0x0 0x58000 0x0 0x24>;
> +				clocks = <&clkc_periphs CLKID_PWM_A>,
> +						<&clkc_periphs CLKID_PWM_B>;
> +				#pwm-cells = <3>;
> +				status = "disabled";
> +			};
> +
> +			pwm_cd: pwm@5a000 {
> +				compatible = "amlogic,meson-s4-pwm";
> +				reg = <0x0 0x5a000 0x0 0x24>;
> +				clocks = <&clkc_periphs CLKID_PWM_C>,
> +						<&clkc_periphs CLKID_PWM_D>;
> +				#pwm-cells = <3>;
> +				status = "disabled";
> +			};
> +
> +			pwm_ef: pwm@5c000 {
> +				compatible = "amlogic,meson-s4-pwm";
> +				reg = <0x0 0x5c000 0x0 0x24>;
> +				clocks = <&clkc_periphs CLKID_PWM_E>,
> +						<&clkc_periphs CLKID_PWM_F>;
> +				#pwm-cells = <3>;
> +				status = "disabled";
> +			};
> +
> +			pwm_gh: pwm@5e000 {
> +				compatible = "amlogic,meson-s4-pwm";
> +				reg = <0x0 0x5e000 0x0 0x24>;
> +				clocks = <&clkc_periphs CLKID_PWM_G>,
> +						<&clkc_periphs CLKID_PWM_H>;
> +				#pwm-cells = <3>;
> +				status = "disabled";
> +			};
> +
> +			pwm_ij: pwm@60000 {
> +				compatible = "amlogic,meson-s4-pwm";
> +				reg = <0x0 0x60000 0x0 0x24>;
> +				clocks = <&clkc_periphs CLKID_PWM_I>,
> +						<&clkc_periphs CLKID_PWM_J>;
> +				#pwm-cells = <3>;
> +				status = "disabled";
> +			};
> +
>   			i2c0: i2c@66000 {
>   				compatible = "amlogic,meson-axg-i2c";
>   				reg = <0x0 0x66000 0x0 0x20>;
> 

-- 
Best regards
George