[PATCH 0/3] pwm: Add support for Amlogic Meson G12A

[PATCH 0/3] pwm: Add support for Amlogic Meson G12A

[Neil Armstrong]

This adds support for the PWM modules on the Amlogic Meson G12A SoC
family, including bindings changes and DT nodes.

Neil Armstrong (3):
  dt-bindings: pwm: Update bindings for the Meson G12A Family
  pwm: meson: Add clock source configuration for Meson G12A
  arm64: dts: meson-g12a: Add PWM nodes

 .../devicetree/bindings/pwm/pwm-meson.txt     |   2 +
 arch/arm64/boot/dts/amlogic/meson-g12a.dtsi   | 179 ++++++++++++++++++
 drivers/pwm/pwm-meson.c                       |  17 ++
 3 files changed, 198 insertions(+)

-- 
2.21.0

[PATCH 1/3] dt-bindings: pwm: Update bindings for the Meson G12A Family

[Neil Armstrong]

Update the doc to explicitly support Meson G12A Family

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 Documentation/devicetree/bindings/pwm/pwm-meson.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-meson.txt b/Documentation/devicetree/bindings/pwm/pwm-meson.txt
index 1fa3f7182133..e2bcd8e92ad0 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-meson.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-meson.txt
@@ -7,6 +7,8 @@ Required properties:
                          or "amlogic,meson-gxbb-ao-pwm"
                          or "amlogic,meson-axg-ee-pwm"
                          or "amlogic,meson-axg-ao-pwm"
+                         or "amlogic,meson-g12a-ee-pwm"
+                         or "amlogic,meson-g12a-ao-pwm"
 - #pwm-cells: Should be 3. See pwm.txt in this directory for a description of
   the cells format.
 
-- 
2.21.0

[PATCH 2/3] pwm: meson: Add clock source configuration for Meson G12A

[Neil Armstrong]

For PWM controller in the Meson G12A SoC, the EE domain and AO domain
have different clock sources. This patch tries to describe them in the
DT compatible data.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/pwm/pwm-meson.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 2b03938039b6..46287cc8a0eb 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -434,6 +434,15 @@ static const struct meson_pwm_data pwm_axg_ao_data = {
 	.num_parents = ARRAY_SIZE(pwm_axg_ao_parent_names),
 };
 
+static const char * const pwm_g12a_ee_parent_names[] = {
+	"xtal", "hdmi_pll", "fclk_div4", "fclk_div3"
+};
+
+static const struct meson_pwm_data pwm_g12a_ee_data = {
+	.parent_names = pwm_g12a_ee_parent_names,
+	.num_parents = ARRAY_SIZE(pwm_g12a_ee_parent_names),
+};
+
 static const struct of_device_id meson_pwm_matches[] = {
 	{
 		.compatible = "amlogic,meson8b-pwm",
@@ -455,6 +464,14 @@ static const struct of_device_id meson_pwm_matches[] = {
 		.compatible = "amlogic,meson-axg-ao-pwm",
 		.data = &pwm_axg_ao_data
 	},
+	{
+		.compatible = "amlogic,meson-g12a-ee-pwm",
+		.data = &pwm_g12a_ee_data
+	},
+	{
+		.compatible = "amlogic,meson-g12a-ao-pwm",
+		.data = &pwm_axg_ao_data
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, meson_pwm_matches);
-- 
2.21.0

[PATCH 3/3] arm64: dts: meson-g12a: Add PWM nodes

[Neil Armstrong]

This adds the EE and AO PWM nodes and the possible pinctrl settings.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 179 ++++++++++++++++++++
 1 file changed, 179 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
index 501496eb34c6..5b7c9076e2d1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
@@ -355,6 +355,94 @@
 						};
 					};
 
+					pwm_a_pins: pwm-a {
+						mux {
+							groups = "pwm_a";
+							function = "pwm_a";
+							bias-disable;
+						};
+					};
+
+					pwm_b_x7_pins: pwm-b-x7 {
+						mux {
+							groups = "pwm_b_x7";
+							function = "pwm_b";
+							bias-disable;
+						};
+					};
+
+					pwm_b_x19_pins: pwm-b-x19 {
+						mux {
+							groups = "pwm_b_x19";
+							function = "pwm_b";
+							bias-disable;
+						};
+					};
+
+					pwm_c_c_pins: pwm-c-c {
+						mux {
+							groups = "pwm_c_c";
+							function = "pwm_c";
+							bias-disable;
+						};
+					};
+
+					pwm_c_x5_pins: pwm-c-x5 {
+						mux {
+							groups = "pwm_c_x5";
+							function = "pwm_c";
+							bias-disable;
+						};
+					};
+
+					pwm_c_x8_pins: pwm-c-x8 {
+						mux {
+							groups = "pwm_c_x8";
+							function = "pwm_c";
+							bias-disable;
+						};
+					};
+
+					pwm_d_x3_pins: pwm-d-x3 {
+						mux {
+							groups = "pwm_d_x3";
+							function = "pwm_d";
+							bias-disable;
+						};
+					};
+
+					pwm_d_x6_pins: pwm-d-x6 {
+						mux {
+							groups = "pwm_d_x6";
+							function = "pwm_d";
+							bias-disable;
+						};
+					};
+
+					pwm_e_pins: pwm-e {
+						mux {
+							groups = "pwm_e";
+							function = "pwm_e";
+							bias-disable;
+						};
+					};
+
+					pwm_f_x_pins: pwm-f-x {
+						mux {
+							groups = "pwm_f_x";
+							function = "pwm_f";
+							bias-disable;
+						};
+					};
+
+					pwm_f_h_pins: pwm-f-h {
+						mux {
+							groups = "pwm_f_h";
+							function = "pwm_f";
+							bias-disable;
+						};
+					};
+
 					uart_a_pins: uart-a {
 						mux {
 							groups = "uart_a_tx",
@@ -632,6 +720,62 @@
 							bias-disable;
 						};
 					};
+
+					pwm_ao_a_pins: pwm-ao-a {
+						mux {
+							groups = "pwm_ao_a";
+							function = "pwm_ao_a";
+							bias-disable;
+						};
+					};
+
+					pwm_ao_b_pins: pwm-ao-b {
+						mux {
+							groups = "pwm_ao_b";
+							function = "pwm_ao_b";
+							bias-disable;
+						};
+					};
+
+					pwm_ao_c_4_pins: pwm-ao-c-4 {
+						mux {
+							groups = "pwm_ao_c_4";
+							function = "pwm_ao_c";
+							bias-disable;
+						};
+					};
+
+					pwm_ao_c_6_pins: pwm-ao-c-6 {
+						mux {
+							groups = "pwm_ao_c_6";
+							function = "pwm_ao_c";
+							bias-disable;
+						};
+					};
+
+					pwm_ao_d_5_pins: pwm-ao-d-5 {
+						mux {
+							groups = "pwm_ao_d_5";
+							function = "pwm_ao_d";
+							bias-disable;
+						};
+					};
+
+					pwm_ao_d_10_pins: pwm-ao-d-10 {
+						mux {
+							groups = "pwm_ao_d_10";
+							function = "pwm_ao_d";
+							bias-disable;
+						};
+					};
+
+					pwm_ao_d_e_pins: pwm-ao-d-e {
+						mux {
+							groups = "pwm_ao_d_e";
+							function = "pwm_ao_d";
+							bias-disable;
+						};
+					};
 				};
 			};
 
@@ -659,6 +803,13 @@
 				status = "disabled";
 			};
 
+			pwm_AO_cd: pwm@2000 {
+				compatible = "amlogic,meson-g12a-ao-pwm";
+				reg = <0x0 0x2000 0x0 0x20>;
+				#pwm-cells = <3>;
+				status = "disabled";
+			};
+
 			uart_AO: serial@3000 {
 				compatible = "amlogic,meson-gx-uart",
 					     "amlogic,meson-ao-uart";
@@ -681,6 +832,13 @@
 				status = "disabled";
 			};
 
+			pwm_AO_ab: pwm@7000 {
+				compatible = "amlogic,meson-g12a-ao-pwm";
+				reg = <0x0 0x7000 0x0 0x20>;
+				#pwm-cells = <3>;
+				status = "disabled";
+			};
+
 			ir: ir@8000 {
 				compatible = "amlogic,meson-gxbb-ir";
 				reg = <0x0 0x8000 0x0 0x20>;
@@ -774,6 +932,27 @@
 				#reset-cells = <1>;
 			};
 
+			pwm_ef: pwm@19000 {
+				compatible = "amlogic,meson-g12a-ee-pwm";
+				reg = <0x0 0x19000 0x0 0x20>;
+				#pwm-cells = <3>;
+				status = "disabled";
+			};
+
+			pwm_cd: pwm@1a000 {
+				compatible = "amlogic,meson-g12a-ee-pwm";
+				reg = <0x0 0x1a000 0x0 0x20>;
+				#pwm-cells = <3>;
+				status = "disabled";
+			};
+
+			pwm_ab: pwm@1b000 {
+				compatible = "amlogic,meson-g12a-ee-pwm";
+				reg = <0x0 0x1b000 0x0 0x20>;
+				#pwm-cells = <3>;
+				status = "disabled";
+			};
+
 			clk_msr: clock-measure@18000 {
 				compatible = "amlogic,meson-g12a-clk-measure";
 				reg = <0x0 0x18000 0x0 0x10>;
-- 
2.21.0

Re: [PATCH 2/3] pwm: meson: Add clock source configuration for Meson G12A

[Martin Blumenstingl]

Hi Neil,

On Fri, Apr 12, 2019 at 11:24 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> For PWM controller in the Meson G12A SoC, the EE domain and AO domain
> have different clock sources. This patch tries to describe them in the
> DT compatible data.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/pwm/pwm-meson.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index 2b03938039b6..46287cc8a0eb 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -434,6 +434,15 @@ static const struct meson_pwm_data pwm_axg_ao_data = {
>         .num_parents = ARRAY_SIZE(pwm_axg_ao_parent_names),
>  };
>
> +static const char * const pwm_g12a_ee_parent_names[] = {
> +       "xtal", "hdmi_pll", "fclk_div4", "fclk_div3"
> +};
> +
> +static const struct meson_pwm_data pwm_g12a_ee_data = {
> +       .parent_names = pwm_g12a_ee_parent_names,
> +       .num_parents = ARRAY_SIZE(pwm_g12a_ee_parent_names),
> +};
> +
>  static const struct of_device_id meson_pwm_matches[] = {
>         {
>                 .compatible = "amlogic,meson8b-pwm",
> @@ -455,6 +464,14 @@ static const struct of_device_id meson_pwm_matches[] = {
>                 .compatible = "amlogic,meson-axg-ao-pwm",
>                 .data = &pwm_axg_ao_data
>         },
> +       {
> +               .compatible = "amlogic,meson-g12a-ee-pwm",
> +               .data = &pwm_g12a_ee_data
> +       },
the PWM part is fine for me

> +       {
> +               .compatible = "amlogic,meson-g12a-ao-pwm",
> +               .data = &pwm_axg_ao_data
> +       },
>         {},
but I'm not sure about "amlogic,meson-g12a-ao-pwm":
the public S922X datasheet from Hardkernel [0] section 6.6.1.2 "AO
Clock Tree" (page 107) mentions two different clock sources for the AO
PWMs:
- AO PWM A and B has parents xtal, aoclk81, fclk_div4 and fclk_div5
(pwm_axg_ao_data has the first two parents swapped)
- AO PWM C and D only have xtal and aoclk81 as parents

regarding the clock parents:
I'm not sure whether pwm_axg_ao_data is wrong, G12A is different from
G12B or the G12B datasheet is "correct". can you please list what you
have tested so far and confirm that the parents you are using are
"correct"

regarding the compatible string "amlogic,meson-g12a-ao-pwm":
if there are two different AO PWM modules, should we name it
differently, for example by splitting this compatible string into:
- "amlogic,meson-g12a-ao-pwm-ab" (with parents: xtal, aoclk81,
fclk_div4 and fclk_div5)
- "amlogic,meson-g12a-ao-pwm-cd" (with parents: xtal and aoclk81)


Regards
Martin


[0] https://dn.odroid.com/S922X/ODROID-N2/Datasheet/S922X_Public_Datasheet_V0.2.pdf

Re: [PATCH 3/3] arm64: dts: meson-g12a: Add PWM nodes

[Martin Blumenstingl]

On Fri, Apr 12, 2019 at 11:24 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This adds the EE and AO PWM nodes and the possible pinctrl settings.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
I reviewed the controller definitions and picked three pinctrl nodes -
all of them are fine:
Reviewed-by: Martin Blumenstingl<martin.blumenstingl@googlemail.com>

that said, I had two questions regarding the AO PWM controllers in
PATCH 2/3 of this series which should be considered before applying
this patch

Re: [PATCH 2/3] pwm: meson: Add clock source configuration for Meson G12A

[Neil Armstrong]

On 13/04/2019 13:33, Martin Blumenstingl wrote:
> Hi Neil,
> 
> On Fri, Apr 12, 2019 at 11:24 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> For PWM controller in the Meson G12A SoC, the EE domain and AO domain
>> have different clock sources. This patch tries to describe them in the
>> DT compatible data.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/pwm/pwm-meson.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
>> index 2b03938039b6..46287cc8a0eb 100644
>> --- a/drivers/pwm/pwm-meson.c
>> +++ b/drivers/pwm/pwm-meson.c
>> @@ -434,6 +434,15 @@ static const struct meson_pwm_data pwm_axg_ao_data = {
>>         .num_parents = ARRAY_SIZE(pwm_axg_ao_parent_names),
>>  };
>>
>> +static const char * const pwm_g12a_ee_parent_names[] = {
>> +       "xtal", "hdmi_pll", "fclk_div4", "fclk_div3"
>> +};
>> +
>> +static const struct meson_pwm_data pwm_g12a_ee_data = {
>> +       .parent_names = pwm_g12a_ee_parent_names,
>> +       .num_parents = ARRAY_SIZE(pwm_g12a_ee_parent_names),
>> +};
>> +
>>  static const struct of_device_id meson_pwm_matches[] = {
>>         {
>>                 .compatible = "amlogic,meson8b-pwm",
>> @@ -455,6 +464,14 @@ static const struct of_device_id meson_pwm_matches[] = {
>>                 .compatible = "amlogic,meson-axg-ao-pwm",
>>                 .data = &pwm_axg_ao_data
>>         },
>> +       {
>> +               .compatible = "amlogic,meson-g12a-ee-pwm",
>> +               .data = &pwm_g12a_ee_data
>> +       },
> the PWM part is fine for me
> 
>> +       {
>> +               .compatible = "amlogic,meson-g12a-ao-pwm",
>> +               .data = &pwm_axg_ao_data
>> +       },
>>         {},
> but I'm not sure about "amlogic,meson-g12a-ao-pwm":
> the public S922X datasheet from Hardkernel [0] section 6.6.1.2 "AO
> Clock Tree" (page 107) mentions two different clock sources for the AO
> PWMs:
> - AO PWM A and B has parents xtal, aoclk81, fclk_div4 and fclk_div5
> (pwm_axg_ao_data has the first two parents swapped)
> - AO PWM C and D only have xtal and aoclk81 as parents

You are right, and it's the same on G12A.

> 
> regarding the clock parents:
> I'm not sure whether pwm_axg_ao_data is wrong, G12A is different from
> G12B or the G12B datasheet is "correct". can you please list what you
> have tested so far and confirm that the parents you are using are
> "correct"

You were right, we need 2 different compatibles here...

> 
> regarding the compatible string "amlogic,meson-g12a-ao-pwm":
> if there are two different AO PWM modules, should we name it
> differently, for example by splitting this compatible string into:
> - "amlogic,meson-g12a-ao-pwm-ab" (with parents: xtal, aoclk81,
> fclk_div4 and fclk_div5)
> - "amlogic,meson-g12a-ao-pwm-cd" (with parents: xtal and aoclk81)
> 

Exact

> 
> Regards
> Martin
> 
> 
> [0] https://dn.odroid.com/S922X/ODROID-N2/Datasheet/S922X_Public_Datasheet_V0.2.pdf
> 


Thanks,
Neil