Re: [PATCH] pwm: meson: add support for S4 chip family

[Jerome Brunet <jbrunet@baylibre.com>]

On Fri 24 Mar 2023 at 23:23, Heiner Kallweit <hkallweit1@gmail.com> wrote:

> This adds pwm support for (at least) the s4 chip family. The extension
> is based on the vendor driver that can be found at [0]. There the
> version with the new clock handling is called meson-v2-pwm.
> Central change is that the clock is now fully provided by the SoC clock
> core. The multiplexer isn't any longer part of the pwm block.

As far as the documentation is concerned this is not true.
There is a input multiplexer with the xtal, vid_pll, fdiv3 and fdiv4

I'm not sure the differences mentionned here actually exists.

>
> This was tested on a sc2-based system that uses the same pwm block.
>
> [0] https://github.com/khadas/linux/blob/khadas-vims-5.4.y/drivers/pwm/pwm-meson.c

AFAICT, this looks more like a choice in vendor SDK to not use the input
mux. IOW, just SW decision.

I don't think such change makes sense in mainline if the HW has not
actually changed.

>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> Adding the amlogic,meson-s4-pwm compatible to the documentation was part
> of the yaml conversion already.
> ---
>  drivers/pwm/pwm-meson.c | 38 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index 16d79ca5d..7a93fdada 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -98,6 +98,7 @@ struct meson_pwm_channel {
>  struct meson_pwm_data {
>  	const char * const *parent_names;
>  	unsigned int num_parents;
> +	unsigned int ext_clk:1;
>  };
>  
>  struct meson_pwm {
> @@ -158,6 +159,7 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
>  	struct meson_pwm_channel *channel = &meson->channels[pwm->hwpwm];
>  	unsigned int duty, period, pre_div, cnt, duty_cnt;
>  	unsigned long fin_freq;
> +	int err;
>  
>  	duty = state->duty_cycle;
>  	period = state->period;
> @@ -165,6 +167,14 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
>  	if (state->polarity == PWM_POLARITY_INVERSED)
>  		duty = period - duty;
>  
> +	if (meson->data->ext_clk) {
> +		err = clk_set_rate(channel->clk, 0xffffUL * NSEC_PER_SEC / period);
> +		if (err) {
> +			dev_err(meson->chip.dev, "failed to set pwm clock rate\n");
> +			return err;
> +		}
> +	}
> +
>  	fin_freq = clk_get_rate(channel->clk);
>  	if (fin_freq == 0) {
>  		dev_err(meson->chip.dev, "invalid source clock frequency\n");
> @@ -173,10 +183,14 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
>  
>  	dev_dbg(meson->chip.dev, "fin_freq: %lu Hz\n", fin_freq);
>  
> -	pre_div = div64_u64(fin_freq * (u64)period, NSEC_PER_SEC * 0xffffLL);
> -	if (pre_div > MISC_CLK_DIV_MASK) {
> -		dev_err(meson->chip.dev, "unable to get period pre_div\n");
> -		return -EINVAL;
> +	if (meson->data->ext_clk) {
> +		pre_div = 0;
> +	} else {
> +		pre_div = div64_u64(fin_freq * (u64)period, NSEC_PER_SEC * 0xffffLL);
> +		if (pre_div > MISC_CLK_DIV_MASK) {
> +			dev_err(meson->chip.dev, "unable to get period pre_div\n");
> +			return -EINVAL;
> +		}
>  	}
>  
>  	cnt = div64_u64(fin_freq * (u64)period, NSEC_PER_SEC * (pre_div + 1));
> @@ -445,6 +459,10 @@ static const struct meson_pwm_data pwm_g12a_ee_data = {
>  	.num_parents = ARRAY_SIZE(pwm_g12a_ee_parent_names),
>  };
>  
> +static const struct meson_pwm_data pwm_s4_data = {
> +	.ext_clk = 1,
> +};
> +
>  static const struct of_device_id meson_pwm_matches[] = {
>  	{
>  		.compatible = "amlogic,meson8b-pwm",
> @@ -478,6 +496,10 @@ static const struct of_device_id meson_pwm_matches[] = {
>  		.compatible = "amlogic,meson-g12a-ao-pwm-cd",
>  		.data = &pwm_g12a_ao_cd_data
>  	},
> +	{
> +		.compatible = "amlogic,meson-s4-pwm",
> +		.data = &pwm_s4_data
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, meson_pwm_matches);
> @@ -493,6 +515,14 @@ static int meson_pwm_init_channels(struct meson_pwm *meson)
>  	for (i = 0; i < meson->chip.npwm; i++) {
>  		struct meson_pwm_channel *channel = &meson->channels[i];
>  
> +		if (meson->data->ext_clk) {
> +			snprintf(name, sizeof(name), "clkin%u", i);
> +			channel->clk = devm_clk_get(dev, name);
> +			if (IS_ERR(channel->clk))
> +				return PTR_ERR(channel->clk);
> +			continue;
> +		}
> +
>  		snprintf(name, sizeof(name), "%s#mux%u", dev_name(dev), i);
>  
>  		init.name = name;


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