2019-08-20 01:42:15

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 0/10] Add mt7629 and fix mt7628 pwm

From: sam shih <[email protected]>

Changes since v4:
- Follow reviewer's comments (v3: pwm: mediatek: add a property "num-pwms")
Move the changes of droping the check for of_device_get_match_data
returning non-NULL to next patch
- Follow reviewers's comments
(v3: pwm: mediatek: allocate the clks array dynamically)
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.

Changes since v3:
- add a new property "clock-frequency" and fix mt7628 pwm
- add mt7629 pwm support

Changes since v2:
- use num-pwms instead of mediatek,num-pwms.
- rename the member from num_pwms to fallback_num_pwms to make it
more obvious that it doesn't represent the actually used value.
- add a dev_warn and a expressive comment to help other developers
to not start adding num_pwms in the compatible_data.

Changes since v1:
- add some checks for backwards compatibility.


Ryder Lee (6):
pwm: mediatek: add a property "num-pwms"
pwm: mediatek: allocate the clks array dynamically and fix mt7628 pwm
dt-bindings: pwm: add a property "num-pwms"
arm64: dts: mt7622: add a property "num-pwms" for PWM node
arm: dts: mt7623: add a property "num-pwms" for PWM node
dt-bindings: pwm: update bindings for MT7629 SoC

sam shih (4):
pwm: mediatek: droping the check for of_device_get_match_data
pwm: mediatek: use pwm_mediatek as common prefix
dt-bindings: pwm: update bindings for MT7628 SoC
arm: dts: mediatek: add mt7629 pwm support

.../devicetree/bindings/pwm/pwm-mediatek.txt | 11 +-
arch/arm/boot/dts/mt7623.dtsi | 1 +
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 1 +
drivers/pwm/pwm-mediatek.c | 242 ++++++++++--------
arch/arm/boot/dts/mt7629.dtsi | 16 ++++++++++++++++
5 files changed, 160 insertions(+), 111 deletions(-)
--
2.17.1


2019-08-20 01:42:45

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 1/10] pwm: mediatek: add a property "num-pwms"

From: Ryder Lee <[email protected]>

This adds a property "num-pwms" to avoid having an endless
list of compatibles with no differences for the same driver.

Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
---
Used:
https://patchwork.kernel.org/project/linux-mediatek/list/?series=68207

Changes since v4:
Follow reviewer's comments:
Move the changes of droping the check for of_device_get_match_data
returning non-NULL to next patch
---
drivers/pwm/pwm-mediatek.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index eb6674ce995f..287fda3674ce 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -55,7 +55,7 @@ static const char * const mtk_pwm_clk_name[MTK_CLK_MAX] = {
};

struct mtk_pwm_platform_data {
- unsigned int num_pwms;
+ unsigned int fallback_npwms;
bool pwm45_fixup;
bool has_clks;
};
@@ -226,10 +226,10 @@ static const struct pwm_ops mtk_pwm_ops = {

static int mtk_pwm_probe(struct platform_device *pdev)
{
- const struct mtk_pwm_platform_data *data;
+ struct device_node *np = pdev->dev.of_node;
struct mtk_pwm_chip *pc;
struct resource *res;
- unsigned int i;
+ unsigned int i, npwms;
int ret;

pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
@@ -246,7 +246,19 @@ static int mtk_pwm_probe(struct platform_device *pdev)
if (IS_ERR(pc->regs))
return PTR_ERR(pc->regs);

- for (i = 0; i < data->num_pwms + 2 && pc->soc->has_clks; i++) {
+ ret = of_property_read_u32(np, "num-pwms", &npwms);
+ if (ret < 0) {
+ /* It's deprecated, we should specify num_pwms via DT now. */
+ if (pc->soc->fallback_npwms) {
+ npwms = pc->soc->fallback_npwms;
+ dev_warn(&pdev->dev, "DT is outdated, please update it\n");
+ } else {
+ dev_err(&pdev->dev, "failed to get number of PWMs\n");
+ return ret;
+ }
+ }
+
+ 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",
@@ -260,7 +272,7 @@ static int mtk_pwm_probe(struct platform_device *pdev)
pc->chip.dev = &pdev->dev;
pc->chip.ops = &mtk_pwm_ops;
pc->chip.base = -1;
- pc->chip.npwm = data->num_pwms;
+ pc->chip.npwm = npwms;

ret = pwmchip_add(&pc->chip);
if (ret < 0) {
@@ -279,25 +291,25 @@ static int mtk_pwm_remove(struct platform_device *pdev)
}

static const struct mtk_pwm_platform_data mt2712_pwm_data = {
- .num_pwms = 8,
+ .fallback_npwms = 8,
.pwm45_fixup = false,
.has_clks = true,
};

static const struct mtk_pwm_platform_data mt7622_pwm_data = {
- .num_pwms = 6,
+ .fallback_npwms = 6,
.pwm45_fixup = false,
.has_clks = true,
};

static const struct mtk_pwm_platform_data mt7623_pwm_data = {
- .num_pwms = 5,
+ .fallback_npwms = 5,
.pwm45_fixup = true,
.has_clks = true,
};

static const struct mtk_pwm_platform_data mt7628_pwm_data = {
- .num_pwms = 4,
+ .fallback_npwms = 4,
.pwm45_fixup = true,
.has_clks = false,
};
--
2.17.1

2019-08-20 01:42:52

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 2/10] pwm: mediatek: droping the check for of_device_get_match_data

From: sam shih <[email protected]>

This patch drop the check for of_device_get_match_data.
Due to the only way call driver probe is compatible match,
In this case, the platform data should never be NULL.

Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
---
Used:
https://patchwork.kernel.org/patch/11096905/
Changes since v4:
Follow reviewer's comments:
Move the changes of droping the check for of_device_get_match_data
returning non-NULL to this patch
---
drivers/pwm/pwm-mediatek.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index 287fda3674ce..f9d67fb66adb 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -236,10 +236,7 @@ static int mtk_pwm_probe(struct platform_device *pdev)
if (!pc)
return -ENOMEM;

- data = of_device_get_match_data(&pdev->dev);
- if (data == NULL)
- return -EINVAL;
- pc->soc = data;
+ pc->soc = of_device_get_match_data(&pdev->dev);

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
pc->regs = devm_ioremap_resource(&pdev->dev, res);
--
2.17.1

2019-08-20 01:44:14

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 4/10] pwm: mediatek: use pwm_mediatek as common prefix

From: sam shih <[email protected]>

Use pwm_mediatek as common prefix to match the filename.
No functional change intended.

Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
---
drivers/pwm/pwm-mediatek.c | 117 ++++++++++++++++++-------------------
1 file changed, 58 insertions(+), 59 deletions(-)

diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index a70b69a975c1..d85761ad5283 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -1,12 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
/*
- * Mediatek Pulse Width Modulator driver
+ * MediaTek Pulse Width Modulator driver
*
* Copyright (C) 2015 John Crispin <[email protected]>
* Copyright (C) 2017 Zhi Mao <[email protected]>
*
- * This file is licensed under the terms of the GNU General Public
- * License version 2. This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
*/

#include <linux/err.h>
@@ -34,43 +32,44 @@
#define PWM45THRES_FIXUP 0x34

#define PWM_CLK_DIV_MAX 7
-
-struct mtk_pwm_platform_data {
+struct pwm_mediatek_of_data {
unsigned int fallback_npwms;
bool pwm45_fixup;
bool has_clks;
};

/**
- * struct mtk_pwm_chip - struct representing PWM chip
+ * struct pwm_mediatek_chip - struct representing PWM chip
* @chip: linux PWM chip representation
* @regs: base address of PWM chip
* @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_mediatek_chip {
struct pwm_chip chip;
void __iomem *regs;
struct clk *clk_top;
struct clk *clk_main;
struct clk **clk_pwms;
unsigned int clk_freq;
- const struct mtk_pwm_platform_data *soc;
+ const struct pwm_mediatek_of_data *soc;
};

-static const unsigned int mtk_pwm_reg_offset[] = {
+static const unsigned int pwm_mediatek_reg_offset[] = {
0x0010, 0x0050, 0x0090, 0x00d0, 0x0110, 0x0150, 0x0190, 0x0220
};

-static inline struct mtk_pwm_chip *to_mtk_pwm_chip(struct pwm_chip *chip)
+static inline struct pwm_mediatek_chip *
+to_pwm_mediatek_chip(struct pwm_chip *chip)
{
- return container_of(chip, struct mtk_pwm_chip, chip);
+ return container_of(chip, struct pwm_mediatek_chip, chip);
}

-static int mtk_pwm_clk_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+static int pwm_mediatek_clk_enable(struct pwm_chip *chip,
+ struct pwm_device *pwm)
{
- struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
+ struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
int ret;

if (!pc->soc->has_clks)
@@ -98,9 +97,10 @@ static int mtk_pwm_clk_enable(struct pwm_chip *chip, struct pwm_device *pwm)
return ret;
}

-static void mtk_pwm_clk_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+static void pwm_mediatek_clk_disable(struct pwm_chip *chip,
+ struct pwm_device *pwm)
{
- struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
+ struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);

if (!pc->soc->has_clks)
return;
@@ -110,23 +110,23 @@ static void mtk_pwm_clk_disable(struct pwm_chip *chip, struct pwm_device *pwm)
clk_disable_unprepare(pc->clk_top);
}

-static inline u32 mtk_pwm_readl(struct mtk_pwm_chip *chip, unsigned int num,
- unsigned int offset)
+static inline u32 pwm_mediatek_readl(struct pwm_mediatek_chip *chip,
+ unsigned int num, unsigned int offset)
{
- return readl(chip->regs + mtk_pwm_reg_offset[num] + offset);
+ return readl(chip->regs + pwm_mediatek_reg_offset[num] + offset);
}

-static inline void mtk_pwm_writel(struct mtk_pwm_chip *chip,
- unsigned int num, unsigned int offset,
- u32 value)
+static inline void pwm_mediatek_writel(struct pwm_mediatek_chip *chip,
+ unsigned int num, unsigned int offset,
+ u32 value)
{
- writel(value, chip->regs + mtk_pwm_reg_offset[num] + offset);
+ writel(value, chip->regs + pwm_mediatek_reg_offset[num] + offset);
}

-static int mtk_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+static int pwm_mediatek_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 pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
unsigned int clk_freq;
u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
reg_thres = PWMTHRES;
@@ -138,7 +138,7 @@ static int mtk_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
else
clk_freq = pc->clk_freq;

- ret = mtk_pwm_clk_enable(chip, pwm);
+ ret = pwm_mediatek_clk_enable(chip, pwm);
if (ret < 0)
return ret;

@@ -155,7 +155,7 @@ static int mtk_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
}

if (clkdiv > PWM_CLK_DIV_MAX) {
- mtk_pwm_clk_disable(chip, pwm);
+ pwm_mediatek_clk_disable(chip, pwm);
dev_err(chip->dev, "period %d not supported\n", period_ns);
return -EINVAL;
}
@@ -170,22 +170,22 @@ static int mtk_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
}

cnt_duty = DIV_ROUND_CLOSEST_ULL((u64)duty_ns * 1000, resolution);
- mtk_pwm_writel(pc, pwm->hwpwm, PWMCON, BIT(15) | clkdiv);
- mtk_pwm_writel(pc, pwm->hwpwm, reg_width, cnt_period);
- mtk_pwm_writel(pc, pwm->hwpwm, reg_thres, cnt_duty);
+ pwm_mediatek_writel(pc, pwm->hwpwm, PWMCON, BIT(15) | clkdiv);
+ pwm_mediatek_writel(pc, pwm->hwpwm, reg_width, cnt_period);
+ pwm_mediatek_writel(pc, pwm->hwpwm, reg_thres, cnt_duty);

- mtk_pwm_clk_disable(chip, pwm);
+ pwm_mediatek_clk_disable(chip, pwm);

return 0;
}

-static int mtk_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+static int pwm_mediatek_enable(struct pwm_chip *chip, struct pwm_device *pwm)
{
- struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
+ struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
u32 value;
int ret;

- ret = mtk_pwm_clk_enable(chip, pwm);
+ ret = pwm_mediatek_clk_enable(chip, pwm);
if (ret < 0)
return ret;

@@ -196,29 +196,29 @@ static int mtk_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
return 0;
}

-static void mtk_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+static void pwm_mediatek_disable(struct pwm_chip *chip, struct pwm_device *pwm)
{
- struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
+ struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
u32 value;

value = readl(pc->regs);
value &= ~BIT(pwm->hwpwm);
writel(value, pc->regs);

- mtk_pwm_clk_disable(chip, pwm);
+ pwm_mediatek_clk_disable(chip, pwm);
}

-static const struct pwm_ops mtk_pwm_ops = {
- .config = mtk_pwm_config,
- .enable = mtk_pwm_enable,
- .disable = mtk_pwm_disable,
+static const struct pwm_ops pwm_mediatek_ops = {
+ .config = pwm_mediatek_config,
+ .enable = pwm_mediatek_enable,
+ .disable = pwm_mediatek_disable,
.owner = THIS_MODULE,
};

-static int mtk_pwm_probe(struct platform_device *pdev)
+static int pwm_mediatek_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct mtk_pwm_chip *pc;
+ struct pwm_mediatek_chip *pc;
struct resource *res;
unsigned int npwms;
unsigned int clk_freq;
@@ -285,7 +285,7 @@ static int mtk_pwm_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pc);

pc->chip.dev = &pdev->dev;
- pc->chip.ops = &mtk_pwm_ops;
+ pc->chip.ops = &pwm_mediatek_ops;
pc->chip.base = -1;
pc->chip.npwm = npwms;

@@ -298,55 +298,55 @@ static int mtk_pwm_probe(struct platform_device *pdev)
return 0;
}

-static int mtk_pwm_remove(struct platform_device *pdev)
+static int pwm_mediatek_remove(struct platform_device *pdev)
{
- struct mtk_pwm_chip *pc = platform_get_drvdata(pdev);
+ struct pwm_mediatek_chip *pc = platform_get_drvdata(pdev);

return pwmchip_remove(&pc->chip);
}

-static const struct mtk_pwm_platform_data mt2712_pwm_data = {
+static const struct pwm_mediatek_of_data mt2712_pwm_data = {
.fallback_npwms = 8,
.pwm45_fixup = false,
.has_clks = true,
};

-static const struct mtk_pwm_platform_data mt7622_pwm_data = {
+static const struct pwm_mediatek_of_data mt7622_pwm_data = {
.fallback_npwms = 6,
.pwm45_fixup = false,
.has_clks = true,
};

-static const struct mtk_pwm_platform_data mt7623_pwm_data = {
+static const struct pwm_mediatek_of_data mt7623_pwm_data = {
.fallback_npwms = 5,
.pwm45_fixup = true,
.has_clks = true,
};

-static const struct mtk_pwm_platform_data mt7628_pwm_data = {
+static const struct pwm_mediatek_of_data mt7628_pwm_data = {
.fallback_npwms = 4,
.pwm45_fixup = true,
.has_clks = false,
};

-static const struct of_device_id mtk_pwm_of_match[] = {
+static const struct of_device_id pwm_mediatek_of_match[] = {
{ .compatible = "mediatek,mt2712-pwm", .data = &mt2712_pwm_data },
{ .compatible = "mediatek,mt7622-pwm", .data = &mt7622_pwm_data },
{ .compatible = "mediatek,mt7623-pwm", .data = &mt7623_pwm_data },
{ .compatible = "mediatek,mt7628-pwm", .data = &mt7628_pwm_data },
{ },
};
-MODULE_DEVICE_TABLE(of, mtk_pwm_of_match);
+MODULE_DEVICE_TABLE(of, pwm_mediatek_of_match);

-static struct platform_driver mtk_pwm_driver = {
+static struct platform_driver pwm_mediatek_driver = {
.driver = {
- .name = "mtk-pwm",
- .of_match_table = mtk_pwm_of_match,
+ .name = "pwm-mediatek",
+ .of_match_table = pwm_mediatek_of_match,
},
- .probe = mtk_pwm_probe,
- .remove = mtk_pwm_remove,
+ .probe = pwm_mediatek_probe,
+ .remove = pwm_mediatek_remove,
};
-module_platform_driver(mtk_pwm_driver);
+module_platform_driver(pwm_mediatek_driver);

MODULE_AUTHOR("John Crispin <[email protected]>");
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
--
2.17.1

2019-08-20 01:44:27

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 3/10] pwm: mediatek: allocate the clks array dynamically and fix mt7628 pwm

From: Ryder Lee <[email protected]>

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 <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
---
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) {
+ 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);
--
2.17.1

2019-08-20 01:45:32

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 8/10] arm: dts: mt7623: add a property "num-pwms" for PWM node

From: Ryder Lee <[email protected]>

This adds a property "num-pwms" for PWM controller.

Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
---
arch/arm/boot/dts/mt7623.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
index a79f0b6c3429..208e0d19a575 100644
--- a/arch/arm/boot/dts/mt7623.dtsi
+++ b/arch/arm/boot/dts/mt7623.dtsi
@@ -452,6 +452,7 @@
<&pericfg CLK_PERI_PWM5>;
clock-names = "top", "main", "pwm1", "pwm2",
"pwm3", "pwm4", "pwm5";
+ num-pwms = <5>;
status = "disabled";
};

--
2.17.1

2019-08-20 01:45:32

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 9/10] dt-bindings: pwm: update bindings for MT7629 SoC

From: Ryder Lee <[email protected]>

This updates bindings for MT7629 pwm controller.

Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
Reviewed-by: Matthias Brugger <[email protected]>
---
Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
index bd644ba85762..84d4b248db45 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
@@ -6,6 +6,7 @@ Required properties:
- "mediatek,mt7622-pwm": found on mt7622 SoC.
- "mediatek,mt7623-pwm": found on mt7623 SoC.
- "mediatek,mt7628-pwm": found on mt7628 SoC.
+ - "mediatek,mt7629-pwm", "mediatek,mt7622-pwm": found on mt7629 SoC.
- reg: physical base address and length of the controller's registers.
- #pwm-cells: must be 2. See pwm.txt in this directory for a description of
the cell format.
--
2.17.1

2019-08-20 01:45:33

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 7/10] arm64: dts: mt7622: add a property "num-pwms" for PWM node

From: Ryder Lee <[email protected]>

This adds a property "num-pwms" for PWM controller.

Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
index d1e13d340e26..9a043938881f 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
@@ -439,6 +439,7 @@
<&pericfg CLK_PERI_PWM6_PD>;
clock-names = "top", "main", "pwm1", "pwm2", "pwm3", "pwm4",
"pwm5", "pwm6";
+ num-pwms = <6>;
status = "disabled";
};

--
2.17.1

2019-08-20 01:45:46

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 5/10] dt-bindings: pwm: add a property "num-pwms"

From: Ryder Lee <[email protected]>

This adds a property "num-pwms" in example so that we could
specify the number of PWM channels via device tree.

Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
Reviewed-by: Matthias Brugger <[email protected]>
---
Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
index 991728cb46cb..ea95b490a913 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
@@ -14,12 +14,12 @@ Required properties:
has no clocks
- "top": the top clock generator
- "main": clock used by the PWM core
- - "pwm1-8": the eight per PWM clocks for mt2712
- - "pwm1-6": the six per PWM clocks for mt7622
- - "pwm1-5": the five per PWM clocks for mt7623
+ - "pwm1-N": the PWM clocks for each channel
+ where N starting from 1 to the maximum number of PWM channels
- pinctrl-names: Must contain a "default" entry.
- pinctrl-0: One property must exist for each entry in pinctrl-names.
See pinctrl/pinctrl-bindings.txt for details of the property values.
+ - num-pwms: the number of PWM channels.

Example:
pwm0: pwm@11006000 {
@@ -37,4 +37,5 @@ Example:
"pwm3", "pwm4", "pwm5";
pinctrl-names = "default";
pinctrl-0 = <&pwm0_pins>;
+ num-pwms = <5>;
};
--
2.17.1

2019-08-20 01:46:13

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 6/10] dt-bindings: pwm: update bindings for MT7628 SoC

From: sam shih <[email protected]>

This updates bindings for MT7628 pwm controller.

Signed-off-by: Sam Shih <[email protected]>
---
Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
index ea95b490a913..bd644ba85762 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
@@ -20,6 +20,10 @@ Required properties:
- pinctrl-0: One property must exist for each entry in pinctrl-names.
See pinctrl/pinctrl-bindings.txt for details of the property values.
- num-pwms: the number of PWM channels.
+
+ Optional properties:
+ - clock-frequency: fix clock frequency, this is only used in MT7628 SoC
+ for period calculation. This SoC has no complex clock tree.

Example:
pwm0: pwm@11006000 {
--
2.17.1

2019-08-20 01:46:49

by Sam Shih

[permalink] [raw]
Subject: [PATCH v4 10/10] arm: dts: mediatek: add mt7629 pwm support

From: sam shih <[email protected]>

This adds pwm support for MT7629.

Signed-off-by: Sam Shih <[email protected]>
---
arch/arm/boot/dts/mt7629.dtsi | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/mt7629.dtsi b/arch/arm/boot/dts/mt7629.dtsi
index 9608bc2ccb3f..493be9a9453b 100644
--- a/arch/arm/boot/dts/mt7629.dtsi
+++ b/arch/arm/boot/dts/mt7629.dtsi
@@ -241,6 +241,22 @@
status = "disabled";
};

+ pwm: pwm@11006000 {
+ compatible = "mediatek,mt7629-pwm",
+ "mediatek,mt7622-pwm";
+ reg = <0 0x11006000 0 0x1000>;
+ interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&topckgen CLK_TOP_PWM_SEL>,
+ <&pericfg CLK_PERI_PWM_PD>,
+ <&pericfg CLK_PERI_PWM1_PD>;
+ clock-names = "top", "main", "pwm1";
+ assigned-clocks = <&topckgen CLK_TOP_PWM_SEL>;
+ assigned-clock-parents =
+ <&topckgen CLK_TOP_UNIVPLL2_D4>;
+ num-pwms = <1>;
+ status = "disabled";
+ };
+
i2c: i2c@11007000 {
compatible = "mediatek,mt7629-i2c",
"mediatek,mt2712-i2c";
--
2.17.1

2019-08-20 05:33:47

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH v4 1/10] pwm: mediatek: add a property "num-pwms"

Hello,

On Tue, Aug 20, 2019 at 09:40:16AM +0800, Sam Shih wrote:
> From: Ryder Lee <[email protected]>
>
> This adds a property "num-pwms" to avoid having an endless
> list of compatibles with no differences for the same driver.
>
> Signed-off-by: Ryder Lee <[email protected]>
> Signed-off-by: Sam Shih <[email protected]>
> ---
> Used:
> https://patchwork.kernel.org/project/linux-mediatek/list/?series=68207
>
> Changes since v4:
> Follow reviewer's comments:
> Move the changes of droping the check for of_device_get_match_data
> returning non-NULL to next patch
> ---
> drivers/pwm/pwm-mediatek.c | 30 +++++++++++++++++++++---------
> 1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
> index eb6674ce995f..287fda3674ce 100644
> --- a/drivers/pwm/pwm-mediatek.c
> +++ b/drivers/pwm/pwm-mediatek.c
> @@ -55,7 +55,7 @@ static const char * const mtk_pwm_clk_name[MTK_CLK_MAX] = {
> };
>
> struct mtk_pwm_platform_data {
> - unsigned int num_pwms;
> + unsigned int fallback_npwms;
> bool pwm45_fixup;
> bool has_clks;
> };
> @@ -226,10 +226,10 @@ static const struct pwm_ops mtk_pwm_ops = {
>
> static int mtk_pwm_probe(struct platform_device *pdev)
> {
> - const struct mtk_pwm_platform_data *data;
> + struct device_node *np = pdev->dev.of_node;
> struct mtk_pwm_chip *pc;
> struct resource *res;
> - unsigned int i;
> + unsigned int i, npwms;
> int ret;
>
> pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
> @@ -246,7 +246,19 @@ static int mtk_pwm_probe(struct platform_device *pdev)
> if (IS_ERR(pc->regs))
> return PTR_ERR(pc->regs);
>
> - for (i = 0; i < data->num_pwms + 2 && pc->soc->has_clks; i++) {
> + ret = of_property_read_u32(np, "num-pwms", &npwms);
> + if (ret < 0) {
> + /* It's deprecated, we should specify num_pwms via DT now. */
> + if (pc->soc->fallback_npwms) {
> + npwms = pc->soc->fallback_npwms;
> + dev_warn(&pdev->dev, "DT is outdated, please update it\n");
> + } else {
> + dev_err(&pdev->dev, "failed to get number of PWMs\n");
> + return ret;
> + }
> + }

I'd suggest to add here:

if (npwms > ARRAY_SIZE(mtk_pwm_clk_name) + 2)
npwms = ARRAY_SIZE(mtk_pwm_clk_name) + 2;

to ensure to not use mtk_pwm_clk_name[10].

Best regards
Uwe

> +
> + 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",
> @@ -260,7 +272,7 @@ static int mtk_pwm_probe(struct platform_device *pdev)
> pc->chip.dev = &pdev->dev;
> pc->chip.ops = &mtk_pwm_ops;
> pc->chip.base = -1;
> - pc->chip.npwm = data->num_pwms;
> + pc->chip.npwm = npwms;
>
> ret = pwmchip_add(&pc->chip);
> if (ret < 0) {
> @@ -279,25 +291,25 @@ static int mtk_pwm_remove(struct platform_device *pdev)
> }
>
> static const struct mtk_pwm_platform_data mt2712_pwm_data = {
> - .num_pwms = 8,
> + .fallback_npwms = 8,
> .pwm45_fixup = false,
> .has_clks = true,
> };
>
> static const struct mtk_pwm_platform_data mt7622_pwm_data = {
> - .num_pwms = 6,
> + .fallback_npwms = 6,
> .pwm45_fixup = false,
> .has_clks = true,
> };
>
> static const struct mtk_pwm_platform_data mt7623_pwm_data = {
> - .num_pwms = 5,
> + .fallback_npwms = 5,
> .pwm45_fixup = true,
> .has_clks = true,
> };
>
> static const struct mtk_pwm_platform_data mt7628_pwm_data = {
> - .num_pwms = 4,
> + .fallback_npwms = 4,
> .pwm45_fixup = true,
> .has_clks = false,
> };
> --
> 2.17.1
>
>

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2019-08-20 05:43:27

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH v4 2/10] pwm: mediatek: droping the check for of_device_get_match_data

Hello,

On Tue, Aug 20, 2019 at 09:40:17AM +0800, Sam Shih wrote:
> From: sam shih <[email protected]>

Your Signed-off-by and the mail's From uses capital letters. Can you
please make them all match?

> This patch drop the check for of_device_get_match_data.
> Due to the only way call driver probe is compatible match,
> In this case, the platform data should never be NULL.

Yeah, all entries in the of_match_table have a .data pointer.
Theoretically the driver could be bound by driver.name and then
of_device_get_match_data returns NULL.

It's still ok to drop this check if you want to, I just want to make
sure this possibility is known.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2019-08-20 05:53:15

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH v4 3/10] pwm: mediatek: allocate the clks array dynamically and fix mt7628 pwm

Hello,

On Tue, Aug 20, 2019 at 09:40:18AM +0800, Sam Shih wrote:
> From: Ryder Lee <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Sam Shih <[email protected]>
> ---
> 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

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2019-08-20 05:57:08

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH v4 4/10] pwm: mediatek: use pwm_mediatek as common prefix

On Tue, Aug 20, 2019 at 09:40:19AM +0800, Sam Shih wrote:
> From: sam shih <[email protected]>
>
> Use pwm_mediatek as common prefix to match the filename.
> No functional change intended.
>
> Signed-off-by: Ryder Lee <[email protected]>
> Signed-off-by: Sam Shih <[email protected]>
> ---
> drivers/pwm/pwm-mediatek.c | 117 ++++++++++++++++++-------------------
> 1 file changed, 58 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
> index a70b69a975c1..d85761ad5283 100644
> --- a/drivers/pwm/pwm-mediatek.c
> +++ b/drivers/pwm/pwm-mediatek.c
> @@ -1,12 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> - * Mediatek Pulse Width Modulator driver
> + * MediaTek Pulse Width Modulator driver
> *
> * Copyright (C) 2015 John Crispin <[email protected]>
> * Copyright (C) 2017 Zhi Mao <[email protected]>
> *
> - * This file is licensed under the terms of the GNU General Public
> - * License version 2. This program is licensed "as is" without any
> - * warranty of any kind, whether express or implied.

The license stuff is a separate change.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2019-08-20 05:59:28

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH v4 5/10] dt-bindings: pwm: add a property "num-pwms"

On Tue, Aug 20, 2019 at 09:40:20AM +0800, Sam Shih wrote:
> From: Ryder Lee <[email protected]>
>
> This adds a property "num-pwms" in example so that we could
> specify the number of PWM channels via device tree.
>
> Signed-off-by: Ryder Lee <[email protected]>
> Signed-off-by: Sam Shih <[email protected]>
> Reviewed-by: Matthias Brugger <[email protected]>

Acked-by: Uwe Kleine-K?nig <[email protected]>

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2019-08-20 05:59:46

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH v4 6/10] dt-bindings: pwm: update bindings for MT7628 SoC

On Tue, Aug 20, 2019 at 09:40:21AM +0800, Sam Shih wrote:
> From: sam shih <[email protected]>
>
> This updates bindings for MT7628 pwm controller.
>
> Signed-off-by: Sam Shih <[email protected]>

Apart from different capitalisation in Author name and S-o-B, this patch
looks fine.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2019-08-20 18:02:53

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 9/10] dt-bindings: pwm: update bindings for MT7629 SoC

On Tue, 20 Aug 2019 09:40:24 +0800, Sam Shih wrote:
> From: Ryder Lee <[email protected]>
>
> This updates bindings for MT7629 pwm controller.
>
> Signed-off-by: Ryder Lee <[email protected]>
> Signed-off-by: Sam Shih <[email protected]>
> Reviewed-by: Matthias Brugger <[email protected]>
> ---
> Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 1 +
> 1 file changed, 1 insertion(+)
>

Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.