[PATCH 1/5] dt-bindings: add binding for Allwinner A31 SoC PWM controller

[PATCH 1/5] dt-bindings: add binding for Allwinner A31 SoC PWM controller

[Icenowy Zheng]

Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
 Documentation/devicetree/bindings/pwm/pwm-sun4i.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
index cf6068b..903fc30 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
@@ -5,6 +5,7 @@ Required properties:
     - "allwinner,sun4i-a10-pwm"
     - "allwinner,sun5i-a10s-pwm"
     - "allwinner,sun5i-a13-pwm"
+    - "allwinner,sun6i-a31-pwm"
     - "allwinner,sun7i-a20-pwm"
   - reg: physical base address and length of the controller's registers
   - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
-- 
2.10.1

[PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Icenowy Zheng]

The PWM controller in A31 is different with other Allwinner SoCs, with a
control register per channel (in other SoCs the control register is
shared), and each channel are allocated 16 bytes of address (but only 8
bytes are used.). The register map in one channel is just like a
single-channel A10 PWM controller, however, A31 have a different
prescaler table than other SoCs.

In order to use the driver for all 4 channels, device nodes should be
created per channel.

Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
 drivers/pwm/pwm-sun4i.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 03a99a5..3e93bdf 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -46,7 +46,7 @@
 
 #define BIT_CH(bit, chan)	((bit) << ((chan) * PWMCH_OFFSET))
 
-static const u32 prescaler_table[] = {
+static const u32 prescaler_table_a10[] = {
 	120,
 	180,
 	240,
@@ -65,10 +65,30 @@ static const u32 prescaler_table[] = {
 	0, /* Actually 1 but tested separately */
 };
 
+static const u32 prescaler_table_a31[] = {
+	1,
+	2,
+	4,
+	8,
+	16,
+	32,
+	64,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+};
+
 struct sun4i_pwm_data {
 	bool has_prescaler_bypass;
 	bool has_rdy;
 	unsigned int npwm;
+	const u32 *prescaler_table;
 };
 
 struct sun4i_pwm_chip {
@@ -100,6 +120,7 @@ static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 			    int duty_ns, int period_ns)
 {
 	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
+	const u32 *prescaler_table = sun4i_pwm->data->prescaler_table;
 	u32 prd, dty, val, clk_gate;
 	u64 clk_rate, div = 0;
 	unsigned int prescaler = 0;
@@ -264,24 +285,35 @@ static const struct sun4i_pwm_data sun4i_pwm_data_a10 = {
 	.has_prescaler_bypass = false,
 	.has_rdy = false,
 	.npwm = 2,
+	.prescaler_table = prescaler_table_a10,
 };
 
 static const struct sun4i_pwm_data sun4i_pwm_data_a10s = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 2,
+	.prescaler_table = prescaler_table_a10,
 };
 
 static const struct sun4i_pwm_data sun4i_pwm_data_a13 = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 1,
+	.prescaler_table = prescaler_table_a10,
 };
 
 static const struct sun4i_pwm_data sun4i_pwm_data_a20 = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 2,
+	.prescaler_table = prescaler_table_a10,
+};
+
+static const struct sun4i_pwm_data sun4i_pwm_data_a31 = {
+	.has_prescaler_bypass = false,
+	.has_rdy = true,
+	.npwm = 1,
+	.prescaler_table = prescaler_table_a31,
 };
 
 static const struct of_device_id sun4i_pwm_dt_ids[] = {
@@ -298,6 +330,9 @@ static const struct of_device_id sun4i_pwm_dt_ids[] = {
 		.compatible = "allwinner,sun7i-a20-pwm",
 		.data = &sun4i_pwm_data_a20,
 	}, {
+		.compatible = "allwinner,sun6i-a31-pwm",
+		.data = &sun4i_pwm_data_a31
+	}, {
 		/* sentinel */
 	},
 };
-- 
2.10.1

[PATCH 3/5] ARM: dts: sun6i: add PWM controller

[Icenowy Zheng]

The A31 and A31s SoCs feature a different PWM controller with 4
channels, all channels have dedicated control register.

Add device nodes for them.

Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index ce19604..97626ce 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -322,6 +322,38 @@
 			#size-cells = <0>;
 		};
 
+		pwm0: pwm@01c21400 {
+			compatible = "allwinner,sun6i-a31-pwm";
+			reg = <0x01c21400 0x4>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm1: pwm@01c21410 {
+			compatible = "allwinner,sun6i-a31-pwm";
+			reg = <0x01c21400 0x4>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm2: pwm@01c21420 {
+			compatible = "allwinner,sun6i-a31-pwm";
+			reg = <0x01c21400 0x4>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm3: pwm@01c21430 {
+			compatible = "allwinner,sun6i-a31-pwm";
+			reg = <0x01c21430 0x4>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		usb_otg: usb@01c19000 {
 			compatible = "allwinner,sun6i-a31-musb";
 			reg = <0x01c19000 0x0400>;
-- 
2.10.1

[PATCH 4/5] ARM: dts: sun6i: add pinmux for PWM0

[Icenowy Zheng]

PWM0 is used by sun6i tablets as the backlight PWM.

Add pinmux for it.

Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 97626ce..76f5a06 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -494,6 +494,13 @@
 				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
 			};
 
+			pwm0_pins: pwm0@0 {
+				allwinner,pins = "PH13";
+				allwinner,function = "pwm0";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
 			mmc0_pins_a: mmc0@0 {
 				allwinner,pins = "PF0", "PF1", "PF2",
 						 "PF3", "PF4", "PF5";
-- 
2.10.1

[PATCH 5/5] ARM: dts: sun6i: add pwm backlight for reference design tablet

[Icenowy Zheng]

The reference tablet design of sun6i uses pwm0 to control the backlight.

As we have PWM support for sun6i now, enable the backlight control.

Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
 .../boot/dts/sun6i-reference-design-tablet.dtsi    | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-reference-design-tablet.dtsi b/arch/arm/boot/dts/sun6i-reference-design-tablet.dtsi
index c3fcf16..aa96acf 100644
--- a/arch/arm/boot/dts/sun6i-reference-design-tablet.dtsi
+++ b/arch/arm/boot/dts/sun6i-reference-design-tablet.dtsi
@@ -45,6 +45,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	aliases {
@@ -54,6 +55,16 @@
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
+
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pinctrl-names = "default";
+		pinctrl-0 = <&bl_en_pin>;
+		pwms = <&pwm0 0 50000 PWM_POLARITY_INVERTED>;
+		brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
+		default-brightness-level = <8>;
+		enable-gpios = <&pio 0 25 GPIO_ACTIVE_HIGH>; /* PA25 */
+	};
 };
 
 &cpu0 {
@@ -76,6 +87,13 @@
 };
 
 &pio {
+	bl_en_pin: bl_en_pin@0 {
+		allwinner,pins = "PA25";
+		allwinner,function = "gpio_in";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+	};
+
 	mmc0_cd_pin_e708_q1: mmc0_cd_pin@0 {
 		allwinner,pins = "PA8";
 		allwinner,function = "gpio_in";
@@ -91,6 +109,12 @@
 	};
 };
 
+&pwm0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm0_pins>;
+	status = "okay";
+};
+
 &p2wi {
 	status = "okay";
 
-- 
2.10.1

Re: [linux-sunxi] [PATCH 4/5] ARM: dts: sun6i: add pinmux for PWM0

[Chen-Yu Tsai]

On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy@aosc.xyz> wrote:
> PWM0 is used by sun6i tablets as the backlight PWM.
>
> Add pinmux for it.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
>  arch/arm/boot/dts/sun6i-a31.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
> index 97626ce..76f5a06 100644
> --- a/arch/arm/boot/dts/sun6i-a31.dtsi
> +++ b/arch/arm/boot/dts/sun6i-a31.dtsi
> @@ -494,6 +494,13 @@
>                                 allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>                         };
>
> +                       pwm0_pins: pwm0@0 {
> +                               allwinner,pins = "PH13";
> +                               allwinner,function = "pwm0";
> +                               allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +                               allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;

Maxime is updating the pinctrl bindings to use generic pinconf,
but otherwise this patch looks good.

ChenYu

> +                       };
> +
>                         mmc0_pins_a: mmc0@0 {
>                                 allwinner,pins = "PF0", "PF1", "PF2",
>                                                  "PF3", "PF4", "PF5";
> --
> 2.10.1
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 4/5] ARM: dts: sun6i: add pinmux for PWM0

[Chen-Yu Tsai]

On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> wrote:
> PWM0 is used by sun6i tablets as the backlight PWM.
>
> Add pinmux for it.
>
> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
> ---
>  arch/arm/boot/dts/sun6i-a31.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
> index 97626ce..76f5a06 100644
> --- a/arch/arm/boot/dts/sun6i-a31.dtsi
> +++ b/arch/arm/boot/dts/sun6i-a31.dtsi
> @@ -494,6 +494,13 @@
>                                 allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>                         };
>
> +                       pwm0_pins: pwm0@0 {
> +                               allwinner,pins = "PH13";
> +                               allwinner,function = "pwm0";
> +                               allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +                               allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;

Maxime is updating the pinctrl bindings to use generic pinconf,
but otherwise this patch looks good.

ChenYu

> +                       };
> +
>                         mmc0_pins_a: mmc0@0 {
>                                 allwinner,pins = "PF0", "PF1", "PF2",
>                                                  "PF3", "PF4", "PF5";
> --
> 2.10.1
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] [PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Chen-Yu Tsai]

Hi,

On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy@aosc.xyz> wrote:
> The PWM controller in A31 is different with other Allwinner SoCs, with a
> control register per channel (in other SoCs the control register is
> shared), and each channel are allocated 16 bytes of address (but only 8
> bytes are used.). The register map in one channel is just like a
> single-channel A10 PWM controller, however, A31 have a different
> prescaler table than other SoCs.
>
> In order to use the driver for all 4 channels, device nodes should be
> created per channel.

I think Maxime wants you to support the different register offsets
in this driver, and have all 4 channels in the same device (node).

ChenYu


> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
>  drivers/pwm/pwm-sun4i.c | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 03a99a5..3e93bdf 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -46,7 +46,7 @@
>
>  #define BIT_CH(bit, chan)      ((bit) << ((chan) * PWMCH_OFFSET))
>
> -static const u32 prescaler_table[] = {
> +static const u32 prescaler_table_a10[] = {
>         120,
>         180,
>         240,
> @@ -65,10 +65,30 @@ static const u32 prescaler_table[] = {
>         0, /* Actually 1 but tested separately */
>  };
>
> +static const u32 prescaler_table_a31[] = {
> +       1,
> +       2,
> +       4,
> +       8,
> +       16,
> +       32,
> +       64,
> +       0,
> +       0,
> +       0,
> +       0,
> +       0,
> +       0,
> +       0,
> +       0,
> +       0,
> +};
> +
>  struct sun4i_pwm_data {
>         bool has_prescaler_bypass;
>         bool has_rdy;
>         unsigned int npwm;
> +       const u32 *prescaler_table;
>  };
>
>  struct sun4i_pwm_chip {
> @@ -100,6 +120,7 @@ static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>                             int duty_ns, int period_ns)
>  {
>         struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> +       const u32 *prescaler_table = sun4i_pwm->data->prescaler_table;
>         u32 prd, dty, val, clk_gate;
>         u64 clk_rate, div = 0;
>         unsigned int prescaler = 0;
> @@ -264,24 +285,35 @@ static const struct sun4i_pwm_data sun4i_pwm_data_a10 = {
>         .has_prescaler_bypass = false,
>         .has_rdy = false,
>         .npwm = 2,
> +       .prescaler_table = prescaler_table_a10,
>  };
>
>  static const struct sun4i_pwm_data sun4i_pwm_data_a10s = {
>         .has_prescaler_bypass = true,
>         .has_rdy = true,
>         .npwm = 2,
> +       .prescaler_table = prescaler_table_a10,
>  };
>
>  static const struct sun4i_pwm_data sun4i_pwm_data_a13 = {
>         .has_prescaler_bypass = true,
>         .has_rdy = true,
>         .npwm = 1,
> +       .prescaler_table = prescaler_table_a10,
>  };
>
>  static const struct sun4i_pwm_data sun4i_pwm_data_a20 = {
>         .has_prescaler_bypass = true,
>         .has_rdy = true,
>         .npwm = 2,
> +       .prescaler_table = prescaler_table_a10,
> +};
> +
> +static const struct sun4i_pwm_data sun4i_pwm_data_a31 = {
> +       .has_prescaler_bypass = false,
> +       .has_rdy = true,
> +       .npwm = 1,
> +       .prescaler_table = prescaler_table_a31,
>  };
>
>  static const struct of_device_id sun4i_pwm_dt_ids[] = {
> @@ -298,6 +330,9 @@ static const struct of_device_id sun4i_pwm_dt_ids[] = {
>                 .compatible = "allwinner,sun7i-a20-pwm",
>                 .data = &sun4i_pwm_data_a20,
>         }, {
> +               .compatible = "allwinner,sun6i-a31-pwm",
> +               .data = &sun4i_pwm_data_a31
> +       }, {
>                 /* sentinel */
>         },
>  };
> --
> 2.10.1
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Chen-Yu Tsai]

Hi,

On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> wrote:
> The PWM controller in A31 is different with other Allwinner SoCs, with a
> control register per channel (in other SoCs the control register is
> shared), and each channel are allocated 16 bytes of address (but only 8
> bytes are used.). The register map in one channel is just like a
> single-channel A10 PWM controller, however, A31 have a different
> prescaler table than other SoCs.
>
> In order to use the driver for all 4 channels, device nodes should be
> created per channel.

I think Maxime wants you to support the different register offsets
in this driver, and have all 4 channels in the same device (node).

ChenYu


> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
> ---
>  drivers/pwm/pwm-sun4i.c | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 03a99a5..3e93bdf 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -46,7 +46,7 @@
>
>  #define BIT_CH(bit, chan)      ((bit) << ((chan) * PWMCH_OFFSET))
>
> -static const u32 prescaler_table[] = {
> +static const u32 prescaler_table_a10[] = {
>         120,
>         180,
>         240,
> @@ -65,10 +65,30 @@ static const u32 prescaler_table[] = {
>         0, /* Actually 1 but tested separately */
>  };
>
> +static const u32 prescaler_table_a31[] = {
> +       1,
> +       2,
> +       4,
> +       8,
> +       16,
> +       32,
> +       64,
> +       0,
> +       0,
> +       0,
> +       0,
> +       0,
> +       0,
> +       0,
> +       0,
> +       0,
> +};
> +
>  struct sun4i_pwm_data {
>         bool has_prescaler_bypass;
>         bool has_rdy;
>         unsigned int npwm;
> +       const u32 *prescaler_table;
>  };
>
>  struct sun4i_pwm_chip {
> @@ -100,6 +120,7 @@ static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>                             int duty_ns, int period_ns)
>  {
>         struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> +       const u32 *prescaler_table = sun4i_pwm->data->prescaler_table;
>         u32 prd, dty, val, clk_gate;
>         u64 clk_rate, div = 0;
>         unsigned int prescaler = 0;
> @@ -264,24 +285,35 @@ static const struct sun4i_pwm_data sun4i_pwm_data_a10 = {
>         .has_prescaler_bypass = false,
>         .has_rdy = false,
>         .npwm = 2,
> +       .prescaler_table = prescaler_table_a10,
>  };
>
>  static const struct sun4i_pwm_data sun4i_pwm_data_a10s = {
>         .has_prescaler_bypass = true,
>         .has_rdy = true,
>         .npwm = 2,
> +       .prescaler_table = prescaler_table_a10,
>  };
>
>  static const struct sun4i_pwm_data sun4i_pwm_data_a13 = {
>         .has_prescaler_bypass = true,
>         .has_rdy = true,
>         .npwm = 1,
> +       .prescaler_table = prescaler_table_a10,
>  };
>
>  static const struct sun4i_pwm_data sun4i_pwm_data_a20 = {
>         .has_prescaler_bypass = true,
>         .has_rdy = true,
>         .npwm = 2,
> +       .prescaler_table = prescaler_table_a10,
> +};
> +
> +static const struct sun4i_pwm_data sun4i_pwm_data_a31 = {
> +       .has_prescaler_bypass = false,
> +       .has_rdy = true,
> +       .npwm = 1,
> +       .prescaler_table = prescaler_table_a31,
>  };
>
>  static const struct of_device_id sun4i_pwm_dt_ids[] = {
> @@ -298,6 +330,9 @@ static const struct of_device_id sun4i_pwm_dt_ids[] = {
>                 .compatible = "allwinner,sun7i-a20-pwm",
>                 .data = &sun4i_pwm_data_a20,
>         }, {
> +               .compatible = "allwinner,sun6i-a31-pwm",
> +               .data = &sun4i_pwm_data_a31
> +       }, {
>                 /* sentinel */
>         },
>  };
> --
> 2.10.1
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Icenowy Zheng]

Hi,

----- 原件 -----
> Hi,
> 
> On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> wrote:
> > The PWM controller in A31 is different with other Allwinner SoCs, with
> > a control register per channel (in other SoCs the control register is
> > shared), and each channel are allocated 16 bytes of address (but only 8
> > bytes are used.). The register map in one channel is just like a
> > single-channel A10 PWM controller, however, A31 have a different
> > prescaler table than other SoCs.
> > 
> > In order to use the driver for all 4 channels, device nodes should be
> > created per channel.
> 
> I think Maxime wants you to support the different register offsets
> in this driver, and have all 4 channels in the same device (node).

I think that will make the code much more complex...
And in hardware there may also be 4 controllers... as the register is aligned at 0x10.

The error I made in my previous patch set is making
a dedicatded pwm-sun6i driver. So I reworked this.
(This also why I do not call the current patchset PATCH v2 -- it's a new patchset)

Icenowy Zheng

> 
> ChenYu
> 
> 
> > Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
> > ---
> > drivers/pwm/pwm-sun4i.c | 37 ++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 36 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 03a99a5..3e93bdf 100644
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -46,7 +46,7 @@
> > 
> > #define BIT_CH(bit, chan)           ((bit) << ((chan) * PWMCH_OFFSET))
> > 
> > -static const u32 prescaler_table[] = {
> > +static const u32 prescaler_table_a10[] = {
> > 120,
> > 180,
> > 240,
> > @@ -65,10 +65,30 @@ static const u32 prescaler_table[] = {
> > 0, /* Actually 1 but tested separately */
> > };
> > 
> > +static const u32 prescaler_table_a31[] = {
> > +             1,
> > +             2,
> > +             4,
> > +             8,
> > +             16,
> > +             32,
> > +             64,
> > +             0,
> > +             0,
> > +             0,
> > +             0,
> > +             0,
> > +             0,
> > +             0,
> > +             0,
> > +             0,
> > +};
> > +
> > struct sun4i_pwm_data {
> > bool has_prescaler_bypass;
> > bool has_rdy;
> > unsigned int npwm;
> > +             const u32 *prescaler_table;
> > };
> > 
> > struct sun4i_pwm_chip {
> > @@ -100,6 +120,7 @@ static int sun4i_pwm_config(struct pwm_chip *chip,
> > struct pwm_device *pwm, int duty_ns, int period_ns)
> > {
> > struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> > +             const u32 *prescaler_table = sun4i_pwm->data->prescaler_table;
> > u32 prd, dty, val, clk_gate;
> > u64 clk_rate, div = 0;
> > unsigned int prescaler = 0;
> > @@ -264,24 +285,35 @@ static const struct sun4i_pwm_data
> > sun4i_pwm_data_a10 = { .has_prescaler_bypass = false,
> > .has_rdy = false,
> > .npwm = 2,
> > +             .prescaler_table = prescaler_table_a10,
> > };
> > 
> > static const struct sun4i_pwm_data sun4i_pwm_data_a10s = {
> > .has_prescaler_bypass = true,
> > .has_rdy = true,
> > .npwm = 2,
> > +             .prescaler_table = prescaler_table_a10,
> > };
> > 
> > static const struct sun4i_pwm_data sun4i_pwm_data_a13 = {
> > .has_prescaler_bypass = true,
> > .has_rdy = true,
> > .npwm = 1,
> > +             .prescaler_table = prescaler_table_a10,
> > };
> > 
> > static const struct sun4i_pwm_data sun4i_pwm_data_a20 = {
> > .has_prescaler_bypass = true,
> > .has_rdy = true,
> > .npwm = 2,
> > +             .prescaler_table = prescaler_table_a10,
> > +};
> > +
> > +static const struct sun4i_pwm_data sun4i_pwm_data_a31 = {
> > +             .has_prescaler_bypass = false,
> > +             .has_rdy = true,
> > +             .npwm = 1,
> > +             .prescaler_table = prescaler_table_a31,
> > };
> > 
> > static const struct of_device_id sun4i_pwm_dt_ids[] = {
> > @@ -298,6 +330,9 @@ static const struct of_device_id
> > sun4i_pwm_dt_ids[] = { .compatible = "allwinner,sun7i-a20-pwm",
> > .data = &sun4i_pwm_data_a20,
> > }, {
> > +                             .compatible = "allwinner,sun6i-a31-pwm",
> > +                             .data = &sun4i_pwm_data_a31
> > +             }, {
> > /* sentinel */
> > },
> > };
> > --
> > 2.10.1
> > 
> > --
> > You received this message because you are subscribed to the Google
> > Groups "linux-sunxi" group. To unsubscribe from this group and stop
> > receiving emails from it, send an email to
> > linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit
> > https://groups.google.com/d/optout.
> 
> -- 
> You received this message because you are subscribed to the Google
> Groups "linux-sunxi" group. To unsubscribe from this group and stop
> receiving emails from it, send an email to
> linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit
> https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] [PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

On Wed, Oct 12, 2016 at 03:30:07PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy@aosc.xyz> wrote:
> > The PWM controller in A31 is different with other Allwinner SoCs, with a
> > control register per channel (in other SoCs the control register is
> > shared), and each channel are allocated 16 bytes of address (but only 8
> > bytes are used.). The register map in one channel is just like a
> > single-channel A10 PWM controller, however, A31 have a different
> > prescaler table than other SoCs.
> >
> > In order to use the driver for all 4 channels, device nodes should be
> > created per channel.
> 
> I think Maxime wants you to support the different register offsets
> in this driver, and have all 4 channels in the same device (node).

Indeed.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

On Wed, Oct 12, 2016 at 03:30:07PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> wrote:
> > The PWM controller in A31 is different with other Allwinner SoCs, with a
> > control register per channel (in other SoCs the control register is
> > shared), and each channel are allocated 16 bytes of address (but only 8
> > bytes are used.). The register map in one channel is just like a
> > single-channel A10 PWM controller, however, A31 have a different
> > prescaler table than other SoCs.
> >
> > In order to use the driver for all 4 channels, device nodes should be
> > created per channel.
> 
> I think Maxime wants you to support the different register offsets
> in this driver, and have all 4 channels in the same device (node).

Indeed.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [linux-sunxi] [PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]

Hi,

On Wed, Oct 12, 2016 at 04:10:03PM +0800, Icenowy Zheng wrote:
> > On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy@aosc.xyz> wrote:
> > > The PWM controller in A31 is different with other Allwinner SoCs, with
> > > a control register per channel (in other SoCs the control register is
> > > shared), and each channel are allocated 16 bytes of address (but only 8
> > > bytes are used.). The register map in one channel is just like a
> > > single-channel A10 PWM controller, however, A31 have a different
> > > prescaler table than other SoCs.
> > > 
> > > In order to use the driver for all 4 channels, device nodes should be
> > > created per channel.
> > 
> > I think Maxime wants you to support the different register offsets
> > in this driver, and have all 4 channels in the same device (node).
> 
> I think that will make the code much more complex...  And in
> hardware there may also be 4 controllers... as the register is
> aligned at 0x10.

You also have to think about it the other way around. This is exposed
everywhere as a single device. There may be some undocumented
registers hidden somewhere in the memory space of that device. How
would you deal with that without touching the device tree?

Exposing this as a single device is the best solution both from the
philosophical point of view, but also from a maintainance aspect.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]

Hi,

On Wed, Oct 12, 2016 at 04:10:03PM +0800, Icenowy Zheng wrote:
> > On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> wrote:
> > > The PWM controller in A31 is different with other Allwinner SoCs, with
> > > a control register per channel (in other SoCs the control register is
> > > shared), and each channel are allocated 16 bytes of address (but only 8
> > > bytes are used.). The register map in one channel is just like a
> > > single-channel A10 PWM controller, however, A31 have a different
> > > prescaler table than other SoCs.
> > > 
> > > In order to use the driver for all 4 channels, device nodes should be
> > > created per channel.
> > 
> > I think Maxime wants you to support the different register offsets
> > in this driver, and have all 4 channels in the same device (node).
> 
> I think that will make the code much more complex...  And in
> hardware there may also be 4 controllers... as the register is
> aligned at 0x10.

You also have to think about it the other way around. This is exposed
everywhere as a single device. There may be some undocumented
registers hidden somewhere in the memory space of that device. How
would you deal with that without touching the device tree?

Exposing this as a single device is the best solution both from the
philosophical point of view, but also from a maintainance aspect.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Icenowy Zheng]

Hi,

14.10.2016, 20:58, "Maxime Ripard" <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> Hi,
>
> On Wed, Oct 12, 2016 at 04:10:03PM +0800, Icenowy Zheng wrote:
>>  > On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy-87fMjt6guo8@public.gmane.orgz> wrote:
>>  > > The PWM controller in A31 is different with other Allwinner SoCs, with
>>  > > a control register per channel (in other SoCs the control register is
>>  > > shared), and each channel are allocated 16 bytes of address (but only 8
>>  > > bytes are used.). The register map in one channel is just like a
>>  > > single-channel A10 PWM controller, however, A31 have a different
>>  > > prescaler table than other SoCs.
>>  > >
>>  > > In order to use the driver for all 4 channels, device nodes should be
>>  > > created per channel.
>>  >
>>  > I think Maxime wants you to support the different register offsets
>>  > in this driver, and have all 4 channels in the same device (node).
>>
>>  I think that will make the code much more complex... And in
>>  hardware there may also be 4 controllers... as the register is
>>  aligned at 0x10.
>
> You also have to think about it the other way around. This is exposed
> everywhere as a single device. There may be some undocumented
> registers hidden somewhere in the memory space of that device. How
> would you deal with that without touching the device tree?

Is the reason only they're listed in the one chapter of user manual?

>
> Exposing this as a single device is the best solution both from the
> philosophical point of view, but also from a maintainance aspect.

If we really do so, I will go back to the original patch (pwm-sun6i)
and merge 4 channels.

No other PWM block of Allwinner devices uses 4 control registers, and it's
a significant difference to make it a dedicated driver.

However, I still think we should have 4 nodes, since the 4 channels can work
very dedicatedly, with different control register... This can be a reason to see
them as 4 dedicated controllers.

(And as PWM uses only oscXX, we cannot judge it according to the clock tree,
and Occam's Razor will apply to think it's 4 A10-like PWM controller...)

If we just think it's because it's a whole part, why don't we combine ehci and ohci
to one driver? Just because we can reuse {e,o}hci-platform...

It's the same reason to see it as 4 controllers.

>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Regards,
Icenowy

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] [PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3129 bytes --]

Hi Icenowy,

On Sun, Oct 16, 2016 at 10:29:17PM +0800, Icenowy Zheng wrote:
> Hi,
> 
> 14.10.2016, 20:58, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> > Hi,
> >
> > On Wed, Oct 12, 2016 at 04:10:03PM +0800, Icenowy Zheng wrote:
> >>  > On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy@aosc.xyz> wrote:
> >>  > > The PWM controller in A31 is different with other Allwinner SoCs, with
> >>  > > a control register per channel (in other SoCs the control register is
> >>  > > shared), and each channel are allocated 16 bytes of address (but only 8
> >>  > > bytes are used.). The register map in one channel is just like a
> >>  > > single-channel A10 PWM controller, however, A31 have a different
> >>  > > prescaler table than other SoCs.
> >>  > >
> >>  > > In order to use the driver for all 4 channels, device nodes should be
> >>  > > created per channel.
> >>  >
> >>  > I think Maxime wants you to support the different register offsets
> >>  > in this driver, and have all 4 channels in the same device (node).
> >>
> >>  I think that will make the code much more complex... And in
> >>  hardware there may also be 4 controllers... as the register is
> >>  aligned at 0x10.
> >
> > You also have to think about it the other way around. This is exposed
> > everywhere as a single device. There may be some undocumented
> > registers hidden somewhere in the memory space of that device. How
> > would you deal with that without touching the device tree?
> 
> Is the reason only they're listed in the one chapter of user manual?

Well, yes, because it is one single device.

> >
> > Exposing this as a single device is the best solution both from the
> > philosophical point of view, but also from a maintainance aspect.
> 
> If we really do so, I will go back to the original patch (pwm-sun6i)
> and merge 4 channels.
> 
> No other PWM block of Allwinner devices uses 4 control registers, and it's
> a significant difference to make it a dedicated driver.
> 
> However, I still think we should have 4 nodes, since the 4 channels can work
> very dedicatedly, with different control register... This can be a reason to see
> them as 4 dedicated controllers.
> 
> (And as PWM uses only oscXX, we cannot judge it according to the clock tree,
> and Occam's Razor will apply to think it's 4 A10-like PWM controller...)
> 
> If we just think it's because it's a whole part, why don't we combine ehci and ohci
> to one driver? Just because we can reuse {e,o}hci-platform...
> 
> It's the same reason to see it as 4 controllers.

Duplicating code is usually not a good idea. In this case, this will
be easier for you to deal with the A31 PWM, but all the rest of the
code (how to enable, disable it, setup the rates, when to enable the
clocks, when to disable them) will be duplicated, even though part of
them are really non-trivial.

Have you looked at reg_field? It really allows you to deal quite
cleanly with those kinds of quirks.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/5] pwm: sun4i: Add support for PWM controller on sun6i SoCs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3498 bytes --]

Hi Icenowy,

On Sun, Oct 16, 2016 at 10:29:17PM +0800, Icenowy Zheng wrote:
> Hi,
> 
> 14.10.2016, 20:58, "Maxime Ripard" <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> > Hi,
> >
> > On Wed, Oct 12, 2016 at 04:10:03PM +0800, Icenowy Zheng wrote:
> >>  > On Wed, Oct 12, 2016 at 12:20 PM, Icenowy Zheng <icenowy@aosc.xyz> wrote:
> >>  > > The PWM controller in A31 is different with other Allwinner SoCs, with
> >>  > > a control register per channel (in other SoCs the control register is
> >>  > > shared), and each channel are allocated 16 bytes of address (but only 8
> >>  > > bytes are used.). The register map in one channel is just like a
> >>  > > single-channel A10 PWM controller, however, A31 have a different
> >>  > > prescaler table than other SoCs.
> >>  > >
> >>  > > In order to use the driver for all 4 channels, device nodes should be
> >>  > > created per channel.
> >>  >
> >>  > I think Maxime wants you to support the different register offsets
> >>  > in this driver, and have all 4 channels in the same device (node).
> >>
> >>  I think that will make the code much more complex... And in
> >>  hardware there may also be 4 controllers... as the register is
> >>  aligned at 0x10.
> >
> > You also have to think about it the other way around. This is exposed
> > everywhere as a single device. There may be some undocumented
> > registers hidden somewhere in the memory space of that device. How
> > would you deal with that without touching the device tree?
> 
> Is the reason only they're listed in the one chapter of user manual?

Well, yes, because it is one single device.

> >
> > Exposing this as a single device is the best solution both from the
> > philosophical point of view, but also from a maintainance aspect.
> 
> If we really do so, I will go back to the original patch (pwm-sun6i)
> and merge 4 channels.
> 
> No other PWM block of Allwinner devices uses 4 control registers, and it's
> a significant difference to make it a dedicated driver.
> 
> However, I still think we should have 4 nodes, since the 4 channels can work
> very dedicatedly, with different control register... This can be a reason to see
> them as 4 dedicated controllers.
> 
> (And as PWM uses only oscXX, we cannot judge it according to the clock tree,
> and Occam's Razor will apply to think it's 4 A10-like PWM controller...)
> 
> If we just think it's because it's a whole part, why don't we combine ehci and ohci
> to one driver? Just because we can reuse {e,o}hci-platform...
> 
> It's the same reason to see it as 4 controllers.

Duplicating code is usually not a good idea. In this case, this will
be easier for you to deal with the A31 PWM, but all the rest of the
code (how to enable, disable it, setup the rates, when to enable the
clocks, when to disable them) will be duplicated, even though part of
them are really non-trivial.

Have you looked at reg_field? It really allows you to deal quite
cleanly with those kinds of quirks.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]