diff mbox series

[v4,3/10] pwm: mediatek: allocate the clks array dynamically and fix mt7628 pwm

Message ID 1566265225-27452-4-git-send-email-sam.shih@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Add mt7629 and fix mt7628 pwm | expand

Commit Message

Sam Shih (史碩三) Aug. 20, 2019, 1:40 a.m. UTC
From: Ryder Lee <ryder.lee@mediatek.com>

Instead of using fixed size of arrays, allocate the memory for them
based on the information we get from the chips.

Also fix mt7628 pwm during configure from userspace. The SoC
is legacy MIPS and has no complex clock tree. This patch add property
clock-frequency to the SoC specific data and legacy MIPS SoC need to
configure it in DT. This property is use for period calculation.

Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
Signed-off-by: Sam Shih <sam.shih@mediatek.com>
---
Changes since v4:
- Follow reviewers's comments
1. use pc->soc->has_clks to check clocks exist or not.
2. Add error message when probe() unable to get clks
- Fixes bug when SoC is old mips which has no complex clock tree.
if clocks not exist, use the new property from DT to apply period caculation;
otherwise, use clk_get_rate to get clock frequency and apply period caculation.
---
 drivers/pwm/pwm-mediatek.c | 94 +++++++++++++++++++++++---------------
 1 file changed, 56 insertions(+), 38 deletions(-)

Comments

Uwe Kleine-König Aug. 20, 2019, 5:51 a.m. UTC | #1
Hello,

On Tue, Aug 20, 2019 at 09:40:18AM +0800, Sam Shih wrote:
> From: Ryder Lee <ryder.lee@mediatek.com>
> 
> Instead of using fixed size of arrays, allocate the memory for them
> based on the information we get from the chips.
> 
> Also fix mt7628 pwm during configure from userspace. The SoC
> is legacy MIPS and has no complex clock tree. This patch add property
> clock-frequency to the SoC specific data and legacy MIPS SoC need to
> configure it in DT. This property is use for period calculation.

This fix is worth a separate patch.

> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> Signed-off-by: Sam Shih <sam.shih@mediatek.com>
> ---
> Changes since v4:
> - Follow reviewers's comments
> 1. use pc->soc->has_clks to check clocks exist or not.
> 2. Add error message when probe() unable to get clks
> - Fixes bug when SoC is old mips which has no complex clock tree.
> if clocks not exist, use the new property from DT to apply period caculation;
> otherwise, use clk_get_rate to get clock frequency and apply period caculation.
> ---
>  drivers/pwm/pwm-mediatek.c | 94 +++++++++++++++++++++++---------------
>  1 file changed, 56 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
> index f9d67fb66adb..a70b69a975c1 100644
> --- a/drivers/pwm/pwm-mediatek.c
> +++ b/drivers/pwm/pwm-mediatek.c
> @@ -35,25 +35,6 @@
>  
>  #define PWM_CLK_DIV_MAX		7
>  
> -enum {
> -	MTK_CLK_MAIN = 0,
> -	MTK_CLK_TOP,
> -	MTK_CLK_PWM1,
> -	MTK_CLK_PWM2,
> -	MTK_CLK_PWM3,
> -	MTK_CLK_PWM4,
> -	MTK_CLK_PWM5,
> -	MTK_CLK_PWM6,
> -	MTK_CLK_PWM7,
> -	MTK_CLK_PWM8,
> -	MTK_CLK_MAX,
> -};
> -
> -static const char * const mtk_pwm_clk_name[MTK_CLK_MAX] = {
> -	"main", "top", "pwm1", "pwm2", "pwm3", "pwm4", "pwm5", "pwm6", "pwm7",
> -	"pwm8"
> -};
> -
>  struct mtk_pwm_platform_data {
>  	unsigned int fallback_npwms;
>  	bool pwm45_fixup;
> @@ -64,12 +45,17 @@ struct mtk_pwm_platform_data {
>   * struct mtk_pwm_chip - struct representing PWM chip
>   * @chip: linux PWM chip representation
>   * @regs: base address of PWM chip
> - * @clks: list of clocks
> + * @clk_top: the top clock generator
> + * @clk_main: the clock used by PWM core
> + * @clk_pwms: the clock used by each PWM channel
>   */
>  struct mtk_pwm_chip {
>  	struct pwm_chip chip;
>  	void __iomem *regs;
> -	struct clk *clks[MTK_CLK_MAX];
> +	struct clk *clk_top;
> +	struct clk *clk_main;
> +	struct clk **clk_pwms;
> +	unsigned int clk_freq;
>  	const struct mtk_pwm_platform_data *soc;
>  };
>  
> @@ -90,24 +76,24 @@ static int mtk_pwm_clk_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>  	if (!pc->soc->has_clks)
>  		return 0;
>  
> -	ret = clk_prepare_enable(pc->clks[MTK_CLK_TOP]);
> +	ret = clk_prepare_enable(pc->clk_top);
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = clk_prepare_enable(pc->clks[MTK_CLK_MAIN]);
> +	ret = clk_prepare_enable(pc->clk_main);
>  	if (ret < 0)
>  		goto disable_clk_top;
>  
> -	ret = clk_prepare_enable(pc->clks[MTK_CLK_PWM1 + pwm->hwpwm]);
> +	ret = clk_prepare_enable(pc->clk_pwms[pwm->hwpwm]);
>  	if (ret < 0)
>  		goto disable_clk_main;
>  
>  	return 0;
>  
>  disable_clk_main:
> -	clk_disable_unprepare(pc->clks[MTK_CLK_MAIN]);
> +	clk_disable_unprepare(pc->clk_main);
>  disable_clk_top:
> -	clk_disable_unprepare(pc->clks[MTK_CLK_TOP]);
> +	clk_disable_unprepare(pc->clk_top);
>  
>  	return ret;
>  }
> @@ -119,9 +105,9 @@ static void mtk_pwm_clk_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>  	if (!pc->soc->has_clks)
>  		return;
>  
> -	clk_disable_unprepare(pc->clks[MTK_CLK_PWM1 + pwm->hwpwm]);
> -	clk_disable_unprepare(pc->clks[MTK_CLK_MAIN]);
> -	clk_disable_unprepare(pc->clks[MTK_CLK_TOP]);
> +	clk_disable_unprepare(pc->clk_pwms[pwm->hwpwm]);
> +	clk_disable_unprepare(pc->clk_main);
> +	clk_disable_unprepare(pc->clk_top);
>  }
>  
>  static inline u32 mtk_pwm_readl(struct mtk_pwm_chip *chip, unsigned int num,
> @@ -141,19 +127,24 @@ static int mtk_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  			  int duty_ns, int period_ns)
>  {
>  	struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
> -	struct clk *clk = pc->clks[MTK_CLK_PWM1 + pwm->hwpwm];
> +	unsigned int clk_freq;
>  	u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
>  	    reg_thres = PWMTHRES;
>  	u64 resolution;
>  	int ret;
>  
> +	if (pc->soc->has_clks)
> +		clk_freq = clk_get_rate(pc->clk_pwms[pwm->hwpwm]);
> +	else
> +		clk_freq = pc->clk_freq;
> +
>  	ret = mtk_pwm_clk_enable(chip, pwm);
>  	if (ret < 0)
>  		return ret;
>  
>  	/* Using resolution in picosecond gets accuracy higher */
>  	resolution = (u64)NSEC_PER_SEC * 1000;
> -	do_div(resolution, clk_get_rate(clk));
> +	do_div(resolution, clk_freq);
>  
>  	cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution);
>  	while (cnt_period > 8191) {
> @@ -229,7 +220,8 @@ static int mtk_pwm_probe(struct platform_device *pdev)
>  	struct device_node *np = pdev->dev.of_node;
>  	struct mtk_pwm_chip *pc;
>  	struct resource *res;
> -	unsigned int i, npwms;
> +	unsigned int npwms;
> +	unsigned int clk_freq;
>  	int ret;
>  
>  	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
> @@ -255,13 +247,40 @@ static int mtk_pwm_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	for (i = 0; i < npwms + 2 && pc->soc->has_clks; i++) {
> -		pc->clks[i] = devm_clk_get(&pdev->dev, mtk_pwm_clk_name[i]);
> -		if (IS_ERR(pc->clks[i])) {
> -			dev_err(&pdev->dev, "clock: %s fail: %ld\n",
> -				mtk_pwm_clk_name[i], PTR_ERR(pc->clks[i]));
> -			return PTR_ERR(pc->clks[i]);
> +	if (pc->soc->has_clks) {

Instead of using pc->soc->has_clks you could try

	ret = of_property_read_u32(np, "clock-frequency", &clk_freq);

and depend on that in the above if. This might allow to drop the
.has_clks member.

> +		int i;
> +
> +		pc->clk_pwms = devm_kcalloc(&pdev->dev, npwms,
> +					    sizeof(*pc->clk_pwms), GFP_KERNEL);
> +		if (!pc->clk_pwms)
> +			return -ENOMEM;
> +
> +		pc->clk_top = devm_clk_get(&pdev->dev, "top");
> +		if (IS_ERR(pc->clk_top))
> +			return PTR_ERR(pc->clk_top);
> +
> +		pc->clk_main = devm_clk_get(&pdev->dev, "main");
> +		if (IS_ERR(pc->clk_main))
> +			return PTR_ERR(pc->clk_main);

You missed to add an error message for "top" and "main".

> +		for (i = 0; i < npwms; i++) {
> +			char name[8];
> +
> +			snprintf(name, sizeof(name), "pwm%d", i + 1);
> +			pc->clk_pwms[i] = devm_clk_get(&pdev->dev, name);
> +			if (IS_ERR(pc->clk_pwms[i])) {
> +				dev_err(&pdev->dev, "failed to get %s\n", name);

I'd mention "clock" in the error string and the return code.

> +				return PTR_ERR(pc->clk_pwms[i]);
> +			}
> +		}
> +	} else {
> +		ret = of_property_read_u32(np, "clock-frequency",
> +						&clk_freq);

Please align follow up lines to the opening (.

> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "failed to get clk_freq\n");
> +			return ret;
>  		}
> +		pc->clk_freq = clk_freq;
>  	}
>  
>  	platform_set_drvdata(pdev, pc);

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index f9d67fb66adb..a70b69a975c1 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -35,25 +35,6 @@ 
 
 #define PWM_CLK_DIV_MAX		7
 
-enum {
-	MTK_CLK_MAIN = 0,
-	MTK_CLK_TOP,
-	MTK_CLK_PWM1,
-	MTK_CLK_PWM2,
-	MTK_CLK_PWM3,
-	MTK_CLK_PWM4,
-	MTK_CLK_PWM5,
-	MTK_CLK_PWM6,
-	MTK_CLK_PWM7,
-	MTK_CLK_PWM8,
-	MTK_CLK_MAX,
-};
-
-static const char * const mtk_pwm_clk_name[MTK_CLK_MAX] = {
-	"main", "top", "pwm1", "pwm2", "pwm3", "pwm4", "pwm5", "pwm6", "pwm7",
-	"pwm8"
-};
-
 struct mtk_pwm_platform_data {
 	unsigned int fallback_npwms;
 	bool pwm45_fixup;
@@ -64,12 +45,17 @@  struct mtk_pwm_platform_data {
  * struct mtk_pwm_chip - struct representing PWM chip
  * @chip: linux PWM chip representation
  * @regs: base address of PWM chip
- * @clks: list of clocks
+ * @clk_top: the top clock generator
+ * @clk_main: the clock used by PWM core
+ * @clk_pwms: the clock used by each PWM channel
  */
 struct mtk_pwm_chip {
 	struct pwm_chip chip;
 	void __iomem *regs;
-	struct clk *clks[MTK_CLK_MAX];
+	struct clk *clk_top;
+	struct clk *clk_main;
+	struct clk **clk_pwms;
+	unsigned int clk_freq;
 	const struct mtk_pwm_platform_data *soc;
 };
 
@@ -90,24 +76,24 @@  static int mtk_pwm_clk_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (!pc->soc->has_clks)
 		return 0;
 
-	ret = clk_prepare_enable(pc->clks[MTK_CLK_TOP]);
+	ret = clk_prepare_enable(pc->clk_top);
 	if (ret < 0)
 		return ret;
 
-	ret = clk_prepare_enable(pc->clks[MTK_CLK_MAIN]);
+	ret = clk_prepare_enable(pc->clk_main);
 	if (ret < 0)
 		goto disable_clk_top;
 
-	ret = clk_prepare_enable(pc->clks[MTK_CLK_PWM1 + pwm->hwpwm]);
+	ret = clk_prepare_enable(pc->clk_pwms[pwm->hwpwm]);
 	if (ret < 0)
 		goto disable_clk_main;
 
 	return 0;
 
 disable_clk_main:
-	clk_disable_unprepare(pc->clks[MTK_CLK_MAIN]);
+	clk_disable_unprepare(pc->clk_main);
 disable_clk_top:
-	clk_disable_unprepare(pc->clks[MTK_CLK_TOP]);
+	clk_disable_unprepare(pc->clk_top);
 
 	return ret;
 }
@@ -119,9 +105,9 @@  static void mtk_pwm_clk_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (!pc->soc->has_clks)
 		return;
 
-	clk_disable_unprepare(pc->clks[MTK_CLK_PWM1 + pwm->hwpwm]);
-	clk_disable_unprepare(pc->clks[MTK_CLK_MAIN]);
-	clk_disable_unprepare(pc->clks[MTK_CLK_TOP]);
+	clk_disable_unprepare(pc->clk_pwms[pwm->hwpwm]);
+	clk_disable_unprepare(pc->clk_main);
+	clk_disable_unprepare(pc->clk_top);
 }
 
 static inline u32 mtk_pwm_readl(struct mtk_pwm_chip *chip, unsigned int num,
@@ -141,19 +127,24 @@  static int mtk_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 			  int duty_ns, int period_ns)
 {
 	struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
-	struct clk *clk = pc->clks[MTK_CLK_PWM1 + pwm->hwpwm];
+	unsigned int clk_freq;
 	u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
 	    reg_thres = PWMTHRES;
 	u64 resolution;
 	int ret;
 
+	if (pc->soc->has_clks)
+		clk_freq = clk_get_rate(pc->clk_pwms[pwm->hwpwm]);
+	else
+		clk_freq = pc->clk_freq;
+
 	ret = mtk_pwm_clk_enable(chip, pwm);
 	if (ret < 0)
 		return ret;
 
 	/* Using resolution in picosecond gets accuracy higher */
 	resolution = (u64)NSEC_PER_SEC * 1000;
-	do_div(resolution, clk_get_rate(clk));
+	do_div(resolution, clk_freq);
 
 	cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution);
 	while (cnt_period > 8191) {
@@ -229,7 +220,8 @@  static int mtk_pwm_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct mtk_pwm_chip *pc;
 	struct resource *res;
-	unsigned int i, npwms;
+	unsigned int npwms;
+	unsigned int clk_freq;
 	int ret;
 
 	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
@@ -255,13 +247,40 @@  static int mtk_pwm_probe(struct platform_device *pdev)
 		}
 	}
 
-	for (i = 0; i < npwms + 2 && pc->soc->has_clks; i++) {
-		pc->clks[i] = devm_clk_get(&pdev->dev, mtk_pwm_clk_name[i]);
-		if (IS_ERR(pc->clks[i])) {
-			dev_err(&pdev->dev, "clock: %s fail: %ld\n",
-				mtk_pwm_clk_name[i], PTR_ERR(pc->clks[i]));
-			return PTR_ERR(pc->clks[i]);
+	if (pc->soc->has_clks) {
+		int i;
+
+		pc->clk_pwms = devm_kcalloc(&pdev->dev, npwms,
+					    sizeof(*pc->clk_pwms), GFP_KERNEL);
+		if (!pc->clk_pwms)
+			return -ENOMEM;
+
+		pc->clk_top = devm_clk_get(&pdev->dev, "top");
+		if (IS_ERR(pc->clk_top))
+			return PTR_ERR(pc->clk_top);
+
+		pc->clk_main = devm_clk_get(&pdev->dev, "main");
+		if (IS_ERR(pc->clk_main))
+			return PTR_ERR(pc->clk_main);
+
+		for (i = 0; i < npwms; i++) {
+			char name[8];
+
+			snprintf(name, sizeof(name), "pwm%d", i + 1);
+			pc->clk_pwms[i] = devm_clk_get(&pdev->dev, name);
+			if (IS_ERR(pc->clk_pwms[i])) {
+				dev_err(&pdev->dev, "failed to get %s\n", name);
+				return PTR_ERR(pc->clk_pwms[i]);
+			}
+		}
+	} else {
+		ret = of_property_read_u32(np, "clock-frequency",
+						&clk_freq);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to get clk_freq\n");
+			return ret;
 		}
+		pc->clk_freq = clk_freq;
 	}
 
 	platform_set_drvdata(pdev, pc);