[PATCH 0/3] pwm: meson: Support constant and polarity bits

[PATCH 0/3] pwm: meson: Support constant and polarity bits

[George Stark]

This patch series add suppot for amlogic's newer PWM IPs hardware features:
constant and polarity bits.

Using polarity bit for inverting output signal allows to identify inversion
in .get_state() callback which can only rely on data read from registers.

Using constant bit allows to have steady output level when duty sycle is zero or
equal to period. Without this bit there will always be single-clock spikes on output.

Those bits are supported in axg, g12 and newer SoC familes like s4, a1 etc.
Tested on g12, a1.

George Stark (3):
  pwm: meson: Support constant and polarity bits
  pwm: meson: Use separate chip data struct for g12a-ee-pwm
  pwm: meson: Enable constant and polarity features for g12, axg, s4

 drivers/pwm/pwm-meson.c | 94 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 87 insertions(+), 7 deletions(-)

--
2.25.1

[PATCH 1/3] pwm: meson: Support constant and polarity bits

[George Stark]

Newer meson PWM IPs support constant and polarity bits. Support them to
correctly implement constant and inverted output levels.

Using constant bit allows to have truly stable low or high output level.
Since hi and low regs internally increment its values by 1 just writing
zero to any of them gives 1 clock count impulse. If constant bit is set
zero value in hi and low regs is not incremented.

Using polarity bit instead of swapping hi and low reg values allows to
correctly identify inversion in .get_state().

Signed-off-by: George Stark <gnstark@salutedevices.com>
---
 drivers/pwm/pwm-meson.c | 75 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 98e6c1533312..5d51404bdce3 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -6,7 +6,7 @@
  * PWM output is achieved by calculating a clock that permits calculating
  * two periods (low and high). The counter then has to be set to switch after
  * N cycles for the first half period.
- * The hardware has no "polarity" setting. This driver reverses the period
+ * Partly the hardware has no "polarity" setting. This driver reverses the period
  * cycles (the low length is inverted with the high length) for
  * PWM_POLARITY_INVERSED. This means that .get_state cannot read the polarity
  * from the hardware.
@@ -56,6 +56,10 @@
 #define MISC_B_CLK_SEL_SHIFT	6
 #define MISC_A_CLK_SEL_SHIFT	4
 #define MISC_CLK_SEL_MASK	0x3
+#define MISC_B_CONSTANT_EN	BIT(29)
+#define MISC_A_CONSTANT_EN	BIT(28)
+#define MISC_B_INVERT_EN	BIT(27)
+#define MISC_A_INVERT_EN	BIT(26)
 #define MISC_B_EN		BIT(1)
 #define MISC_A_EN		BIT(0)

@@ -68,6 +72,8 @@ static struct meson_pwm_channel_data {
 	u8		clk_div_shift;
 	u8		clk_en_shift;
 	u32		pwm_en_mask;
+	u32		const_en_mask;
+	u32		inv_en_mask;
 } meson_pwm_per_channel_data[MESON_NUM_PWMS] = {
 	{
 		.reg_offset	= REG_PWM_A,
@@ -75,6 +81,8 @@ static struct meson_pwm_channel_data {
 		.clk_div_shift	= MISC_A_CLK_DIV_SHIFT,
 		.clk_en_shift	= MISC_A_CLK_EN_SHIFT,
 		.pwm_en_mask	= MISC_A_EN,
+		.const_en_mask	= MISC_A_CONSTANT_EN,
+		.inv_en_mask	= MISC_A_INVERT_EN,
 	},
 	{
 		.reg_offset	= REG_PWM_B,
@@ -82,6 +90,8 @@ static struct meson_pwm_channel_data {
 		.clk_div_shift	= MISC_B_CLK_DIV_SHIFT,
 		.clk_en_shift	= MISC_B_CLK_EN_SHIFT,
 		.pwm_en_mask	= MISC_B_EN,
+		.const_en_mask	= MISC_B_CONSTANT_EN,
+		.inv_en_mask	= MISC_B_INVERT_EN,
 	}
 };

@@ -99,6 +109,8 @@ struct meson_pwm_channel {
 struct meson_pwm_data {
 	const char *const parent_names[MESON_NUM_MUX_PARENTS];
 	int (*channels_init)(struct pwm_chip *chip);
+	bool has_constant;
+	bool has_polarity;
 };

 struct meson_pwm {
@@ -160,7 +172,7 @@ static int meson_pwm_calc(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * Fixing this needs some care however as some machines might rely on
 	 * this.
 	 */
-	if (state->polarity == PWM_POLARITY_INVERSED)
+	if (state->polarity == PWM_POLARITY_INVERSED && !meson->data->has_polarity)
 		duty = period - duty;

 	freq = div64_u64(NSEC_PER_SEC * 0xffffULL, period);
@@ -204,6 +216,46 @@ static int meson_pwm_calc(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }

+static void meson_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
+				   bool inverted)
+{
+	struct meson_pwm *meson = to_meson_pwm(chip);
+	const struct meson_pwm_channel_data *channel_data;
+	unsigned long flags;
+	u32 value;
+
+	channel_data = &meson_pwm_per_channel_data[pwm->hwpwm];
+
+	spin_lock_irqsave(&meson->lock, flags);
+	value = readl(meson->base + REG_MISC_AB);
+	if (inverted)
+		value |= channel_data->inv_en_mask;
+	else
+		value &= ~channel_data->inv_en_mask;
+	writel(value, meson->base + REG_MISC_AB);
+	spin_unlock_irqrestore(&meson->lock, flags);
+}
+
+static void meson_pwm_set_constant(struct pwm_chip *chip, struct pwm_device *pwm,
+				   bool enable)
+{
+	struct meson_pwm *meson = to_meson_pwm(chip);
+	const struct meson_pwm_channel_data *channel_data;
+	unsigned long flags;
+	u32 value;
+
+	channel_data = &meson_pwm_per_channel_data[pwm->hwpwm];
+
+	spin_lock_irqsave(&meson->lock, flags);
+	value = readl(meson->base + REG_MISC_AB);
+	if (enable)
+		value |= channel_data->const_en_mask;
+	else
+		value &= ~channel_data->const_en_mask;
+	writel(value, meson->base + REG_MISC_AB);
+	spin_unlock_irqrestore(&meson->lock, flags);
+}
+
 static void meson_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct meson_pwm *meson = to_meson_pwm(chip);
@@ -255,9 +307,9 @@ static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	int err = 0;

 	if (!state->enabled) {
-		if (state->polarity == PWM_POLARITY_INVERSED) {
+		if (state->polarity == PWM_POLARITY_INVERSED && !meson->data->has_polarity) {
 			/*
-			 * This IP block revision doesn't have an "always high"
+			 * Some of IP block revisions don't have an "always high"
 			 * setting which we can use for "inverted disabled".
 			 * Instead we achieve this by setting mux parent with
 			 * highest rate and minimum divider value, resulting
@@ -284,6 +336,14 @@ static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		meson_pwm_enable(chip, pwm);
 	}

+	if (meson->data->has_constant)
+		meson_pwm_set_constant(chip, pwm,
+				       state->duty_cycle == state->period ||
+				       !state->duty_cycle);
+	if (meson->data->has_polarity)
+		meson_pwm_set_polarity(chip, pwm,
+				       !(state->polarity == PWM_POLARITY_NORMAL));
+
 	return 0;
 }

@@ -318,6 +378,11 @@ static int meson_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	value = readl(meson->base + REG_MISC_AB);
 	state->enabled = value & channel_data->pwm_en_mask;

+	if (meson->data->has_polarity && (value & channel_data->inv_en_mask))
+		state->polarity = PWM_POLARITY_INVERSED;
+	else
+		state->polarity = PWM_POLARITY_NORMAL;
+
 	value = readl(meson->base + channel_data->reg_offset);
 	channel->lo = FIELD_GET(PWM_LOW_MASK, value);
 	channel->hi = FIELD_GET(PWM_HIGH_MASK, value);
@@ -325,8 +390,6 @@ static int meson_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	state->period = meson_pwm_cnt_to_ns(chip, pwm, channel->lo + channel->hi);
 	state->duty_cycle = meson_pwm_cnt_to_ns(chip, pwm, channel->hi);

-	state->polarity = PWM_POLARITY_NORMAL;
-
 	return 0;
 }

--
2.25.1

Re: [PATCH 1/3] pwm: meson: Support constant and polarity bits

[neil.armstrong]

Hi,

On 07/10/2024 21:32, George Stark wrote:
> Newer meson PWM IPs support constant and polarity bits. Support them to
> correctly implement constant and inverted output levels.
> 
> Using constant bit allows to have truly stable low or high output level.
> Since hi and low regs internally increment its values by 1 just writing
> zero to any of them gives 1 clock count impulse. If constant bit is set
> zero value in hi and low regs is not incremented.
> 
> Using polarity bit instead of swapping hi and low reg values allows to
> correctly identify inversion in .get_state().
> 
> Signed-off-by: George Stark <gnstark@salutedevices.com>
> ---
>   drivers/pwm/pwm-meson.c | 75 +++++++++++++++++++++++++++++++++++++----
>   1 file changed, 69 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index 98e6c1533312..5d51404bdce3 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -6,7 +6,7 @@
>    * PWM output is achieved by calculating a clock that permits calculating
>    * two periods (low and high). The counter then has to be set to switch after
>    * N cycles for the first half period.
> - * The hardware has no "polarity" setting. This driver reverses the period
> + * Partly the hardware has no "polarity" setting. This driver reverses the period
>    * cycles (the low length is inverted with the high length) for
>    * PWM_POLARITY_INVERSED. This means that .get_state cannot read the polarity
>    * from the hardware.
> @@ -56,6 +56,10 @@
>   #define MISC_B_CLK_SEL_SHIFT	6
>   #define MISC_A_CLK_SEL_SHIFT	4
>   #define MISC_CLK_SEL_MASK	0x3
> +#define MISC_B_CONSTANT_EN	BIT(29)
> +#define MISC_A_CONSTANT_EN	BIT(28)
> +#define MISC_B_INVERT_EN	BIT(27)
> +#define MISC_A_INVERT_EN	BIT(26)
>   #define MISC_B_EN		BIT(1)
>   #define MISC_A_EN		BIT(0)

Nice, seems I completely missed those 2 features!

> 
> @@ -68,6 +72,8 @@ static struct meson_pwm_channel_data {
>   	u8		clk_div_shift;
>   	u8		clk_en_shift;
>   	u32		pwm_en_mask;
> +	u32		const_en_mask;
> +	u32		inv_en_mask;
>   } meson_pwm_per_channel_data[MESON_NUM_PWMS] = {
>   	{
>   		.reg_offset	= REG_PWM_A,
> @@ -75,6 +81,8 @@ static struct meson_pwm_channel_data {
>   		.clk_div_shift	= MISC_A_CLK_DIV_SHIFT,
>   		.clk_en_shift	= MISC_A_CLK_EN_SHIFT,
>   		.pwm_en_mask	= MISC_A_EN,
> +		.const_en_mask	= MISC_A_CONSTANT_EN,
> +		.inv_en_mask	= MISC_A_INVERT_EN,
>   	},
>   	{
>   		.reg_offset	= REG_PWM_B,
> @@ -82,6 +90,8 @@ static struct meson_pwm_channel_data {
>   		.clk_div_shift	= MISC_B_CLK_DIV_SHIFT,
>   		.clk_en_shift	= MISC_B_CLK_EN_SHIFT,
>   		.pwm_en_mask	= MISC_B_EN,
> +		.const_en_mask	= MISC_B_CONSTANT_EN,
> +		.inv_en_mask	= MISC_B_INVERT_EN,
>   	}
>   };
> 
> @@ -99,6 +109,8 @@ struct meson_pwm_channel {
>   struct meson_pwm_data {
>   	const char *const parent_names[MESON_NUM_MUX_PARENTS];
>   	int (*channels_init)(struct pwm_chip *chip);
> +	bool has_constant;
> +	bool has_polarity;
>   };
> 
>   struct meson_pwm {
> @@ -160,7 +172,7 @@ static int meson_pwm_calc(struct pwm_chip *chip, struct pwm_device *pwm,
>   	 * Fixing this needs some care however as some machines might rely on
>   	 * this.
>   	 */
> -	if (state->polarity == PWM_POLARITY_INVERSED)
> +	if (state->polarity == PWM_POLARITY_INVERSED && !meson->data->has_polarity)
>   		duty = period - duty;
> 
>   	freq = div64_u64(NSEC_PER_SEC * 0xffffULL, period);
> @@ -204,6 +216,46 @@ static int meson_pwm_calc(struct pwm_chip *chip, struct pwm_device *pwm,
>   	return 0;
>   }
> 
> +static void meson_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
> +				   bool inverted)
> +{
> +	struct meson_pwm *meson = to_meson_pwm(chip);
> +	const struct meson_pwm_channel_data *channel_data;
> +	unsigned long flags;
> +	u32 value;
> +
> +	channel_data = &meson_pwm_per_channel_data[pwm->hwpwm];
> +
> +	spin_lock_irqsave(&meson->lock, flags);
> +	value = readl(meson->base + REG_MISC_AB);
> +	if (inverted)
> +		value |= channel_data->inv_en_mask;
> +	else
> +		value &= ~channel_data->inv_en_mask;
> +	writel(value, meson->base + REG_MISC_AB);
> +	spin_unlock_irqrestore(&meson->lock, flags);
> +}
> +
> +static void meson_pwm_set_constant(struct pwm_chip *chip, struct pwm_device *pwm,
> +				   bool enable)
> +{
> +	struct meson_pwm *meson = to_meson_pwm(chip);
> +	const struct meson_pwm_channel_data *channel_data;
> +	unsigned long flags;
> +	u32 value;
> +
> +	channel_data = &meson_pwm_per_channel_data[pwm->hwpwm];
> +
> +	spin_lock_irqsave(&meson->lock, flags);
> +	value = readl(meson->base + REG_MISC_AB);
> +	if (enable)
> +		value |= channel_data->const_en_mask;
> +	else
> +		value &= ~channel_data->const_en_mask;
> +	writel(value, meson->base + REG_MISC_AB);
> +	spin_unlock_irqrestore(&meson->lock, flags);
> +}

Those functions looks quite complicated, why can't they be part of
meson_pwm_enable/disable where we already write REG_MISC_AB ?

> +
>   static void meson_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>   {
>   	struct meson_pwm *meson = to_meson_pwm(chip);
> @@ -255,9 +307,9 @@ static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>   	int err = 0;
> 
>   	if (!state->enabled) {
> -		if (state->polarity == PWM_POLARITY_INVERSED) {
> +		if (state->polarity == PWM_POLARITY_INVERSED && !meson->data->has_polarity) {
>   			/*
> -			 * This IP block revision doesn't have an "always high"
> +			 * Some of IP block revisions don't have an "always high"
>   			 * setting which we can use for "inverted disabled".
>   			 * Instead we achieve this by setting mux parent with
>   			 * highest rate and minimum divider value, resulting
> @@ -284,6 +336,14 @@ static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>   		meson_pwm_enable(chip, pwm);
>   	}
> 
> +	if (meson->data->has_constant)
> +		meson_pwm_set_constant(chip, pwm,
> +				       state->duty_cycle == state->period ||
> +				       !state->duty_cycle);
> +	if (meson->data->has_polarity)
> +		meson_pwm_set_polarity(chip, pwm,
> +				       !(state->polarity == PWM_POLARITY_NORMAL));
> +
>   	return 0;
>   }
> 
> @@ -318,6 +378,11 @@ static int meson_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
>   	value = readl(meson->base + REG_MISC_AB);
>   	state->enabled = value & channel_data->pwm_en_mask;
> 
> +	if (meson->data->has_polarity && (value & channel_data->inv_en_mask))
> +		state->polarity = PWM_POLARITY_INVERSED;
> +	else
> +		state->polarity = PWM_POLARITY_NORMAL;
> +
>   	value = readl(meson->base + channel_data->reg_offset);
>   	channel->lo = FIELD_GET(PWM_LOW_MASK, value);
>   	channel->hi = FIELD_GET(PWM_HIGH_MASK, value);
> @@ -325,8 +390,6 @@ static int meson_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
>   	state->period = meson_pwm_cnt_to_ns(chip, pwm, channel->lo + channel->hi);
>   	state->duty_cycle = meson_pwm_cnt_to_ns(chip, pwm, channel->hi);
> 
> -	state->polarity = PWM_POLARITY_NORMAL;
> -
>   	return 0;
>   }
> 
> --
> 2.25.1
> 

[PATCH 2/3] pwm: meson: Use separate chip data struct for g12a-ee-pwm

[George Stark]

PWM module of g12a SoC family has different set of features than meson8
so use separate chip data struct for it.

Signed-off-by: George Stark <gnstark@salutedevices.com>
---
 drivers/pwm/pwm-meson.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 5d51404bdce3..6701738c55e3 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -578,6 +578,11 @@ static const struct meson_pwm_data pwm_axg_ao_data = {
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
 };
 
+static const struct meson_pwm_data pwm_g12a_ee_data = {
+	.parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" },
+	.channels_init = meson_pwm_init_channels_meson8b_legacy,
+};
+
 static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
 	.parent_names = { "xtal", "g12a_ao_clk81", "fclk_div4", "fclk_div5" },
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
@@ -624,7 +629,7 @@ static const struct of_device_id meson_pwm_matches[] = {
 	},
 	{
 		.compatible = "amlogic,meson-g12a-ee-pwm",
-		.data = &pwm_meson8b_data
+		.data = &pwm_g12a_ee_data
 	},
 	{
 		.compatible = "amlogic,meson-g12a-ao-pwm-ab",
-- 
2.25.1

Re: [PATCH 2/3] pwm: meson: Use separate chip data struct for g12a-ee-pwm

[neil.armstrong]

On 07/10/2024 21:32, George Stark wrote:
> PWM module of g12a SoC family has different set of features than meson8
> so use separate chip data struct for it.
> 
> Signed-off-by: George Stark <gnstark@salutedevices.com>
> ---
>   drivers/pwm/pwm-meson.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index 5d51404bdce3..6701738c55e3 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -578,6 +578,11 @@ static const struct meson_pwm_data pwm_axg_ao_data = {
>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
>   };
>   
> +static const struct meson_pwm_data pwm_g12a_ee_data = {
> +	.parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" },
> +	.channels_init = meson_pwm_init_channels_meson8b_legacy,
> +};
> +
>   static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
>   	.parent_names = { "xtal", "g12a_ao_clk81", "fclk_div4", "fclk_div5" },
>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
> @@ -624,7 +629,7 @@ static const struct of_device_id meson_pwm_matches[] = {
>   	},
>   	{
>   		.compatible = "amlogic,meson-g12a-ee-pwm",
> -		.data = &pwm_meson8b_data
> +		.data = &pwm_g12a_ee_data
>   	},
>   	{
>   		.compatible = "amlogic,meson-g12a-ao-pwm-ab",

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

[PATCH 3/3] pwm: meson: Enable constant and polarity features for g12, axg, s4

[George Stark]

g12, axg and s4 SoC families support constant and polarity bits so enable
those features in corresponding chip data structs.

Signed-off-by: George Stark <gnstark@salutedevices.com>
---
 drivers/pwm/pwm-meson.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 6701738c55e3..c6f032bdfe78 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -571,26 +571,36 @@ static const struct meson_pwm_data pwm_gxbb_ao_data = {
 static const struct meson_pwm_data pwm_axg_ee_data = {
 	.parent_names = { "xtal", "fclk_div5", "fclk_div4", "fclk_div3" },
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
+	.has_constant = true,
+	.has_polarity = true,
 };
 
 static const struct meson_pwm_data pwm_axg_ao_data = {
 	.parent_names = { "xtal", "axg_ao_clk81", "fclk_div4", "fclk_div5" },
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
+	.has_constant = true,
+	.has_polarity = true,
 };
 
 static const struct meson_pwm_data pwm_g12a_ee_data = {
 	.parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" },
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
+	.has_constant = true,
+	.has_polarity = true,
 };
 
 static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
 	.parent_names = { "xtal", "g12a_ao_clk81", "fclk_div4", "fclk_div5" },
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
+	.has_constant = true,
+	.has_polarity = true,
 };
 
 static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
 	.parent_names = { "xtal", "g12a_ao_clk81", NULL, NULL },
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
+	.has_constant = true,
+	.has_polarity = true,
 };
 
 static const struct meson_pwm_data pwm_meson8_v2_data = {
@@ -599,6 +609,8 @@ static const struct meson_pwm_data pwm_meson8_v2_data = {
 
 static const struct meson_pwm_data pwm_s4_data = {
 	.channels_init = meson_pwm_init_channels_s4,
+	.has_constant = true,
+	.has_polarity = true,
 };
 
 static const struct of_device_id meson_pwm_matches[] = {
-- 
2.25.1

Re: [PATCH 3/3] pwm: meson: Enable constant and polarity features for g12, axg, s4

[neil.armstrong]

On 07/10/2024 21:32, George Stark wrote:
> g12, axg and s4 SoC families support constant and polarity bits so enable
> those features in corresponding chip data structs.
> 
> Signed-off-by: George Stark <gnstark@salutedevices.com>
> ---
>   drivers/pwm/pwm-meson.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index 6701738c55e3..c6f032bdfe78 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -571,26 +571,36 @@ static const struct meson_pwm_data pwm_gxbb_ao_data = {
>   static const struct meson_pwm_data pwm_axg_ee_data = {
>   	.parent_names = { "xtal", "fclk_div5", "fclk_div4", "fclk_div3" },
>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
> +	.has_constant = true,
> +	.has_polarity = true,
>   };
>   
>   static const struct meson_pwm_data pwm_axg_ao_data = {
>   	.parent_names = { "xtal", "axg_ao_clk81", "fclk_div4", "fclk_div5" },
>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
> +	.has_constant = true,
> +	.has_polarity = true,
>   };
>   
>   static const struct meson_pwm_data pwm_g12a_ee_data = {
>   	.parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" },
>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
> +	.has_constant = true,
> +	.has_polarity = true,
>   };
>   
>   static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
>   	.parent_names = { "xtal", "g12a_ao_clk81", "fclk_div4", "fclk_div5" },
>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
> +	.has_constant = true,
> +	.has_polarity = true,
>   };
>   
>   static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
>   	.parent_names = { "xtal", "g12a_ao_clk81", NULL, NULL },
>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
> +	.has_constant = true,
> +	.has_polarity = true,
>   };
>   
>   static const struct meson_pwm_data pwm_meson8_v2_data = {
> @@ -599,6 +609,8 @@ static const struct meson_pwm_data pwm_meson8_v2_data = {
>   
>   static const struct meson_pwm_data pwm_s4_data = {
>   	.channels_init = meson_pwm_init_channels_s4,
> +	.has_constant = true,
> +	.has_polarity = true,
>   };
>   
>   static const struct of_device_id meson_pwm_matches[] = {

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH 3/3] pwm: meson: Enable constant and polarity features for g12, axg, s4

[Jerome Brunet]

On Tue 08 Oct 2024 at 09:31, neil.armstrong@linaro.org wrote:

> On 07/10/2024 21:32, George Stark wrote:
>> g12, axg and s4 SoC families support constant and polarity bits so enable
>> those features in corresponding chip data structs.
>> Signed-off-by: George Stark <gnstark@salutedevices.com>
>> ---
>>   drivers/pwm/pwm-meson.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
>> index 6701738c55e3..c6f032bdfe78 100644
>> --- a/drivers/pwm/pwm-meson.c
>> +++ b/drivers/pwm/pwm-meson.c
>> @@ -571,26 +571,36 @@ static const struct meson_pwm_data pwm_gxbb_ao_data = {
>>   static const struct meson_pwm_data pwm_axg_ee_data = {
>>   	.parent_names = { "xtal", "fclk_div5", "fclk_div4", "fclk_div3" },
>>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
>> +	.has_constant = true,
>> +	.has_polarity = true,
>>   };
>>     static const struct meson_pwm_data pwm_axg_ao_data = {
>>   	.parent_names = { "xtal", "axg_ao_clk81", "fclk_div4", "fclk_div5" },
>>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
>> +	.has_constant = true,
>> +	.has_polarity = true,
>>   };
>>     static const struct meson_pwm_data pwm_g12a_ee_data = {
>>   	.parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" },
>>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
>> +	.has_constant = true,
>> +	.has_polarity = true,
>>   };
>>     static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
>>   	.parent_names = { "xtal", "g12a_ao_clk81", "fclk_div4", "fclk_div5" },
>>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
>> +	.has_constant = true,
>> +	.has_polarity = true,
>>   };
>>     static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
>>   	.parent_names = { "xtal", "g12a_ao_clk81", NULL, NULL },
>>   	.channels_init = meson_pwm_init_channels_meson8b_legacy,
>> +	.has_constant = true,
>> +	.has_polarity = true,
>>   };
>>     static const struct meson_pwm_data pwm_meson8_v2_data = {

This needs to be splitted and adjusted as well then.
g12 and axg are covered by pwm_meson8_v2_data.

>> @@ -599,6 +609,8 @@ static const struct meson_pwm_data pwm_meson8_v2_data = {
>>     static const struct meson_pwm_data pwm_s4_data = {
>>   	.channels_init = meson_pwm_init_channels_s4,
>> +	.has_constant = true,
>> +	.has_polarity = true,
>>   };
>>     static const struct of_device_id meson_pwm_matches[] = {
>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

-- 
Jerome