Changes since v9:
1. PATCH 03/11: Add an Acked-by tag
Changes since v8:
1. Fix warning and build-error for patch 04/11
Changes since v7:
1. PATCH v7 10/11: Add a missed Reviewed-by tag
Changes since v6:
1. Due to we can use fixed-clock in DT
We removed has_clks and fixed-clock properties
Changes since v5:
- Follow reviewer's comments:
1. the license stuff is a separate change
2. split fix mt7628 pwm into a single patch
3. to ensure to not use mtk_pwm_clk_name[10]
(After dynamic allocate clock array patch,
this is no need to check)
4. Use clock-frequency property to replace
the use of has_clks
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
calculation; otherwise, use clk_get_rate to get clock frequency and
apply period calculation.
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 (5):
pwm: mediatek: add a property "num-pwms"
dt-bindings: pwm: add a property "num-pwms"
arm64: dts: mt7622: add a property "num-pwms" for PWM
arm: dts: mt7623: add a property "num-pwms" for PWM
dt-bindings: pwm: update bindings for MT7629 SoC
Sam Shih (6):
pwm: mediatek: droping the check for of_device_get_match_data
pwm: mediatek: remove a property "has-clks"
pwm: mediatek: allocate the clks array dynamically
pwm: mediatek: use pwm_mediatek as common prefix
pwm: mediatek: update license and switch to SPDX tag
arm: dts: mediatek: add mt7629 pwm support
.../devicetree/bindings/pwm/pwm-mediatek.txt | 8 +-
arch/arm/boot/dts/mt7623.dtsi | 1 +
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 1 +
drivers/pwm/pwm-mediatek.c | 245 +++++++++---------
arch/arm/boot/dts/mt7629.dtsi | 16 ++++++++++++++++
5 files changed, 149 insertions(+), 122 deletions(-)
--
2.17.1
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]>
Reviewed-by: Uwe Kleine-König <[email protected]>
---
Changes since v6:
Add a Reviewed-by tag
Changes since v5:
Check num-pwms value is no more than MTK_CLK_MAX - 2 (MAIN + TOP)
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 | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index eb6674ce995f..e214f4f57107 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;
};
@@ -227,9 +227,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 +247,26 @@ 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;
+ }
+ }
+
+ /* MAIN + TOP + NPWM < MTK_CLK_MAX */
+ if ((npwms + 2) > MTK_CLK_MAX) {
+ dev_warn(&pdev->dev, "number of PWMs is larger than %d\n",
+ MTK_CLK_MAX - 2);
+ npwms = MTK_CLK_MAX - 2;
+ }
+
+ 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 +280,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 +299,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
Instead of using fixed size of arrays, allocate the memory for them
based on the information we get from the DT.
Also remove the check for num_pwms, due to dynamically allocate pwm
should not cause array index out of bound.
Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
Reviewed-by: Uwe Kleine-König <[email protected]>
---
Changes since v8:
- Fix warning and build-error
Changes since v6:
- Add a Reviewed-by tag
Changes since v5:
- Follow reviewers's comments
Make the changes of allocate the clks array dynamically as a single patch
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 | 84 +++++++++++++++++++-------------------
1 file changed, 42 insertions(+), 42 deletions(-)
diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index 07e843aeddb1..c234b8fa65e7 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;
@@ -63,12 +44,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
+ * @clk_freq: the fix clock frequency of legacy MIPS SoC
*/
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;
const struct mtk_pwm_platform_data *soc;
};
@@ -86,24 +72,24 @@ static int mtk_pwm_clk_enable(struct pwm_chip *chip, struct pwm_device *pwm)
struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
int ret;
- 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;
}
@@ -112,9 +98,9 @@ static void mtk_pwm_clk_disable(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
- 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,
@@ -134,7 +120,7 @@ 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];
+ struct clk *clk = pc->clk_pwms[pwm->hwpwm];
u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
reg_thres = PWMTHRES;
u64 resolution;
@@ -222,8 +208,9 @@ 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;
int ret;
+ int i;
pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
if (!pc)
@@ -248,21 +235,33 @@ static int mtk_pwm_probe(struct platform_device *pdev)
}
}
- /* MAIN + TOP + NPWM < MTK_CLK_MAX */
- if ((npwms + 2) > MTK_CLK_MAX) {
- dev_warn(&pdev->dev, "number of PWMs is larger than %d\n",
- MTK_CLK_MAX - 2);
- npwms = MTK_CLK_MAX - 2;
+ 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)) {
+ dev_err(&pdev->dev, "clock: top fail: %ld\n",
+ PTR_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)) {
+ dev_err(&pdev->dev, "clock: main fail: %ld\n",
+ PTR_ERR(pc->clk_main));
+ return PTR_ERR(pc->clk_main);
}
+ for (i = 0; i < npwms; i++) {
+ char name[8];
- for (i = 0; i < npwms + 2 ; i++) {
- pc->clks[i] = devm_clk_get(&pdev->dev,
- mtk_pwm_clk_name[i]);
- if (IS_ERR(pc->clks[i])) {
+ 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, "clock: %s fail: %ld\n",
- mtk_pwm_clk_name[i],
- PTR_ERR(pc->clks[i]));
- return PTR_ERR(pc->clks[i]);
+ name, PTR_ERR(pc->clk_pwms[i]));
+ return PTR_ERR(pc->clk_pwms[i]);
}
}
--
2.17.1
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
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]>
---
Changes since v6:
Follow reviewers's comments:
- The subject should indicate this is for Mediatek
Changes since v5:
- Add an Acked-by tag
- This file is original v4 patch 5/10
(https://patchwork.kernel.org/patch/11102577/)
---
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
We can use fixed-clock to repair mt7628 pwm during configure from
userspace. The SoC is legacy MIPS and has no complex clock tree.
Due to we can get clock frequency for period calculation from DT
fixed-clock, so we can remove has-clock property, and directly
use devm_clk_get and clk_get_rate.
Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
Acked-by: Uwe Kleine-Kö <[email protected]>
---
Changes since v9:
Added an Acked-by tag
Changes since v6:
Based on fixed-clock in DT, we can remove has-clks property
Changes since v5:
1. Follow reviewer's comments
Make the changes of fix mt7628 pwm as a single patch
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 | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index ebd62629e3fe..07e843aeddb1 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -57,7 +57,6 @@ static const char * const mtk_pwm_clk_name[MTK_CLK_MAX] = {
struct mtk_pwm_platform_data {
unsigned int fallback_npwms;
bool pwm45_fixup;
- bool has_clks;
};
/**
@@ -87,9 +86,6 @@ static int mtk_pwm_clk_enable(struct pwm_chip *chip, struct pwm_device *pwm)
struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
int ret;
- if (!pc->soc->has_clks)
- return 0;
-
ret = clk_prepare_enable(pc->clks[MTK_CLK_TOP]);
if (ret < 0)
return ret;
@@ -116,9 +112,6 @@ static void mtk_pwm_clk_disable(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
- 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]);
@@ -262,11 +255,13 @@ static int mtk_pwm_probe(struct platform_device *pdev)
npwms = MTK_CLK_MAX - 2;
}
- for (i = 0; i < npwms + 2 && pc->soc->has_clks; i++) {
- pc->clks[i] = devm_clk_get(&pdev->dev, mtk_pwm_clk_name[i]);
+ for (i = 0; i < npwms + 2 ; 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]));
+ mtk_pwm_clk_name[i],
+ PTR_ERR(pc->clks[i]));
return PTR_ERR(pc->clks[i]);
}
}
@@ -297,25 +292,21 @@ static int mtk_pwm_remove(struct platform_device *pdev)
static const struct mtk_pwm_platform_data mt2712_pwm_data = {
.fallback_npwms = 8,
.pwm45_fixup = false,
- .has_clks = true,
};
static const struct mtk_pwm_platform_data mt7622_pwm_data = {
.fallback_npwms = 6,
.pwm45_fixup = false,
- .has_clks = true,
};
static const struct mtk_pwm_platform_data mt7623_pwm_data = {
.fallback_npwms = 5,
.pwm45_fixup = true,
- .has_clks = true,
};
static const struct mtk_pwm_platform_data mt7628_pwm_data = {
.fallback_npwms = 4,
.pwm45_fixup = true,
- .has_clks = false,
};
static const struct of_device_id mtk_pwm_of_match[] = {
--
2.17.1
This patch drop the check for of_device_get_match_data.
Due to the only way call driver probe is compatible match.
The .data pointer which point to the SoC specify data is
directly set by driver, and it should not be NULL in our case.
We can safety remove the check for of_device_get_match_data.
Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
Acked-by: Uwe Kleine-König <[email protected]>
---
Used:
https://patchwork.kernel.org/patch/11096905/
Changes since v6:
Add an Acked-by tag
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 | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index e214f4f57107..ebd62629e3fe 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -226,7 +226,6 @@ 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;
@@ -237,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
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]>
Acked-by: Uwe Kleine-König <[email protected]>
---
Changes since v6:
Add an Acked-by tag
Changes since v5:
- Follow reviewers's comments
The license stuff is a separate change
---
drivers/pwm/pwm-mediatek.c | 116 +++++++++++++++++++------------------
1 file changed, 59 insertions(+), 57 deletions(-)
diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index c234b8fa65e7..d2e0f4692de1 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -34,14 +34,13 @@
#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;
};
/**
- * 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
@@ -49,27 +48,29 @@ struct mtk_pwm_platform_data {
* @clk_pwms: the clock used by each PWM channel
* @clk_freq: the fix clock frequency of legacy MIPS SoC
*/
-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;
- 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;
ret = clk_prepare_enable(pc->clk_top);
@@ -94,45 +95,46 @@ 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);
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,
- 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,
- int duty_ns, int period_ns)
+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 clk *clk = pc->clk_pwms[pwm->hwpwm];
+ struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
reg_thres = PWMTHRES;
u64 resolution;
int ret;
- ret = mtk_pwm_clk_enable(chip, pwm);
+ ret = pwm_mediatek_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_get_rate(pc->clk_pwms[pwm->hwpwm]));
cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution);
while (cnt_period > 8191) {
@@ -143,7 +145,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;
}
@@ -158,22 +160,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;
@@ -184,29 +186,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;
int ret;
@@ -268,7 +270,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;
@@ -281,51 +283,51 @@ 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,
};
-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,
};
-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,
};
-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,
};
-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");
--
2.17.1
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
Add SPDX identifiers to pwm-mediatek.c
Update license to GNU General Public License v2.0
Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
Reviewed-by: Uwe Kleine-König <[email protected]>
---
Changes since v6:
Add a Reviewed-by tag
Changes since v5:
- Follow reviewers's comments
The license stuff is a separate change
---
drivers/pwm/pwm-mediatek.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index 11f9cc446f14..9a61829766fc 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>
@@ -331,4 +329,4 @@ static struct platform_driver pwm_mediatek_driver = {
module_platform_driver(pwm_mediatek_driver);
MODULE_AUTHOR("John Crispin <[email protected]>");
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
--
2.17.1
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: Rob Herring <[email protected]>
Reviewed-by: Matthias Brugger <[email protected]>
---
Changes since v7:
- add a missed Reviewed-by tag back from v1:
https://patchwork.kernel.org/patch/10769381/
Changes since v1:
- add a Reviewed-by tag
---
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 ea95b490a913..c7bd5633d1eb 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
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
On Fri, Sep 20, 2019 at 06:49:07AM +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]>
> ---
> Changes since v6:
> Follow reviewers's comments:
> - The subject should indicate this is for Mediatek
>
> Changes since v5:
> - Add an Acked-by tag
> - This file is original v4 patch 5/10
> (https://patchwork.kernel.org/patch/11102577/)
>
> ---
> Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
You failed to address Rob's questions repeatedly and I agree with him
that you can just as easily derive the number of PWMs from the specific
compatible string. I won't be applying this and none of the patches that
depend on it.
Thierry
On Sat, 2019-09-21 at 02:21 +0200, Thierry Reding wrote:
> On Fri, Sep 20, 2019 at 06:49:07AM +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]>
> > ---
> > Changes since v6:
> > Follow reviewers's comments:
> > - The subject should indicate this is for Mediatek
> >
> > Changes since v5:
> > - Add an Acked-by tag
> > - This file is original v4 patch 5/10
> > (https://patchwork.kernel.org/patch/11102577/)
> >
> > ---
> > Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
>
> You failed to address Rob's questions repeatedly and I agree with him
> that you can just as easily derive the number of PWMs from the specific
> compatible string. I won't be applying this and none of the patches that
> depend on it.
>
Hi,
Thanks for getting back to me.
New pwm driver (patch 04/11 : "pwm: mediatek: allocate the clks array
dynamically") can support different variants with different number of
PWMs by the new property <num-pwms>
For example:
1. Use "num-pwms" = <2> and assign clocks pwm1, pwm2 for mt7622
2. Use "num-pwms" = <6> and assign clocks pwm1, pwm2, pwm3, pwm4, pwm5,
pwm6 for mt7622.
If we just as easily derive the number of PWMs from the specific
compatible string in this document:
- "pwm1-6": the six per PWM clocks for mt7622
This looks like all "pwm1", "pwm2", "pwm3", "pwm4", "pwm5", "pwm6" is
required property in DT, It doesn't make sense.
So we removed those descriptions and added
- "pwm1-N": the PWM clocks for each channel
But the max number of clocks from the compatible string are still
important information that should be provide in this document.
What do you think of this?
- "pwm1-N": per PWM clocks for mt2712, the max number of PWM channels
is 8
- "pwm1-N": per PWM clocks for mt7622, the max number of PWM channels
is 6
- "pwm1-N": per PWM clocks for mt7623, the max number of PWM channels
is 5
where N starting from 1 to the maximum number of PWM channels
- num-pwms: the number of PWM channels.
Thanks
Best Regards
Sam
On Mon, Sep 23, 2019 at 11:20:57AM +0800, Sam Shih wrote:
> On Sat, 2019-09-21 at 02:21 +0200, Thierry Reding wrote:
> > On Fri, Sep 20, 2019 at 06:49:07AM +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]>
> > > ---
> > > Changes since v6:
> > > Follow reviewers's comments:
> > > - The subject should indicate this is for Mediatek
> > >
> > > Changes since v5:
> > > - Add an Acked-by tag
> > > - This file is original v4 patch 5/10
> > > (https://patchwork.kernel.org/patch/11102577/)
> > >
> > > ---
> > > Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 7 ++++---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > You failed to address Rob's questions repeatedly and I agree with him
> > that you can just as easily derive the number of PWMs from the specific
> > compatible string. I won't be applying this and none of the patches that
> > depend on it.
> >
>
> Hi,
>
> Thanks for getting back to me.
>
> New pwm driver (patch 04/11 : "pwm: mediatek: allocate the clks array
> dynamically") can support different variants with different number of
> PWMs by the new property <num-pwms>
>
> For example:
> 1. Use "num-pwms" = <2> and assign clocks pwm1, pwm2 for mt7622
> 2. Use "num-pwms" = <6> and assign clocks pwm1, pwm2, pwm3, pwm4, pwm5,
> pwm6 for mt7622.
>
> If we just as easily derive the number of PWMs from the specific
> compatible string in this document:
>
> - "pwm1-6": the six per PWM clocks for mt7622
>
> This looks like all "pwm1", "pwm2", "pwm3", "pwm4", "pwm5", "pwm6" is
> required property in DT, It doesn't make sense.
I don't understand. Why doesn't that make sense? If your hardware block
has 6 PWMs and each can be driven by its own clock, then you need to
provide references for each of those clocks, otherwise you won't be able
to use them.
>
> So we removed those descriptions and added
>
> - "pwm1-N": the PWM clocks for each channel
>
>
> But the max number of clocks from the compatible string are still
> important information that should be provide in this document.
>
>
> What do you think of this?
>
> - "pwm1-N": per PWM clocks for mt2712, the max number of PWM channels
> is 8
>
> - "pwm1-N": per PWM clocks for mt7622, the max number of PWM channels
> is 6
>
> - "pwm1-N": per PWM clocks for mt7623, the max number of PWM channels
> is 5
That's what's in the bindings already, isn't it?
- clocks: phandle and clock specifier of the PWM reference clock.
- clock-names: must contain the following, except for MT7628 which
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
Note that the description of the "clocks" property isn't quite accurate.
It should be something like:
- clocks: One phandle and clock specifier for each entry in the
"clock-names" property.
In the above you clearly describe which PWMs you have to specify for
each generation of the hardware block.
>
> where N starting from 1 to the maximum number of PWM channels
> - num-pwms: the number of PWM channels.
That's redundant information. The specific number of PWMs in already
implied by the compatible string, so you don't need to duplicate that
information here.
Thierry
On Mon, Sep 23, 2019 at 11:11:55PM +0800, Sam Shih wrote:
> On Mon, 2019-09-23 at 15:36 +0200, Thierry Reding wrote:
> > On Mon, Sep 23, 2019 at 11:20:57AM +0800, Sam Shih wrote:
> > > On Sat, 2019-09-21 at 02:21 +0200, Thierry Reding wrote:
> > > > On Fri, Sep 20, 2019 at 06:49:07AM +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]>
> > > > > ---
> > > > > Changes since v6:
> > > > > Follow reviewers's comments:
> > > > > - The subject should indicate this is for Mediatek
> > > > >
> > > > > Changes since v5:
> > > > > - Add an Acked-by tag
> > > > > - This file is original v4 patch 5/10
> > > > > (https://patchwork.kernel.org/patch/11102577/)
> > > > >
> > > > > ---
> > > > > Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 7 ++++---
> > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > >
> > > > You failed to address Rob's questions repeatedly and I agree with him
> > > > that you can just as easily derive the number of PWMs from the specific
> > > > compatible string. I won't be applying this and none of the patches that
> > > > depend on it.
> > > >
> > >
> > > Hi,
> > >
> > > Thanks for getting back to me.
> > >
> > > New pwm driver (patch 04/11 : "pwm: mediatek: allocate the clks array
> > > dynamically") can support different variants with different number of
> > > PWMs by the new property <num-pwms>
> > >
> > > For example:
> > > 1. Use "num-pwms" = <2> and assign clocks pwm1, pwm2 for mt7622
> > > 2. Use "num-pwms" = <6> and assign clocks pwm1, pwm2, pwm3, pwm4, pwm5,
> > > pwm6 for mt7622.
> > >
> > > If we just as easily derive the number of PWMs from the specific
> > > compatible string in this document:
> > >
> > > - "pwm1-6": the six per PWM clocks for mt7622
> > >
> > > This looks like all "pwm1", "pwm2", "pwm3", "pwm4", "pwm5", "pwm6" is
> > > required property in DT, It doesn't make sense.
> >
> > I don't understand. Why doesn't that make sense? If your hardware block
> > has 6 PWMs and each can be driven by its own clock, then you need to
> > provide references for each of those clocks, otherwise you won't be able
> > to use them.
>
> Thank you for your instruction,
> I will add all clock-names and clocks according to
> hardware blocks instead of value of <num-pwms> in DT.
>
> > >
> > > So we removed those descriptions and added
> > >
> > > - "pwm1-N": the PWM clocks for each channel
> > >
> > >
> > > But the max number of clocks from the compatible string are still
> > > important information that should be provide in this document.
> > >
> > >
> > > What do you think of this?
> > >
> > > - "pwm1-N": per PWM clocks for mt2712, the max number of PWM channels
> > > is 8
> > >
> > > - "pwm1-N": per PWM clocks for mt7622, the max number of PWM channels
> > > is 6
> > >
> > > - "pwm1-N": per PWM clocks for mt7623, the max number of PWM channels
> > > is 5
> >
> > That's what's in the bindings already, isn't it?
> >
> > - clocks: phandle and clock specifier of the PWM reference clock.
> > - clock-names: must contain the following, except for MT7628 which
> > 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
>
> Yes, You are right,
> I will keep original description and remove "pwm1-N" from this patch.
> - "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
>
> Actually, MT7629 also use "mediatek,mt7622-pwm" as compatible string,
> but it's hardware only support one pwm, so I was wrongly stick by
> expecting "pwm1-N" in clock-names based on "num-pwms" in DT.
> (that we can assign num-pwms to 1 and only provide pwm1 as clock-names)
>
> Maybe added mt7629 description to this document can solve this simply.
> - "pwm1": the PWM1 clock for mt7629
Yeah, if mt7629 support only one PWM channel, you should list it's
compatible string separately and also update the driver to reflect the
number of channels associated with the hardware.
Thierry
On Mon, 2019-09-23 at 15:36 +0200, Thierry Reding wrote:
> On Mon, Sep 23, 2019 at 11:20:57AM +0800, Sam Shih wrote:
> > On Sat, 2019-09-21 at 02:21 +0200, Thierry Reding wrote:
> > > On Fri, Sep 20, 2019 at 06:49:07AM +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]>
> > > > ---
> > > > Changes since v6:
> > > > Follow reviewers's comments:
> > > > - The subject should indicate this is for Mediatek
> > > >
> > > > Changes since v5:
> > > > - Add an Acked-by tag
> > > > - This file is original v4 patch 5/10
> > > > (https://patchwork.kernel.org/patch/11102577/)
> > > >
> > > > ---
> > > > Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 7 ++++---
> > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > You failed to address Rob's questions repeatedly and I agree with him
> > > that you can just as easily derive the number of PWMs from the specific
> > > compatible string. I won't be applying this and none of the patches that
> > > depend on it.
> > >
> >
> > Hi,
> >
> > Thanks for getting back to me.
> >
> > New pwm driver (patch 04/11 : "pwm: mediatek: allocate the clks array
> > dynamically") can support different variants with different number of
> > PWMs by the new property <num-pwms>
> >
> > For example:
> > 1. Use "num-pwms" = <2> and assign clocks pwm1, pwm2 for mt7622
> > 2. Use "num-pwms" = <6> and assign clocks pwm1, pwm2, pwm3, pwm4, pwm5,
> > pwm6 for mt7622.
> >
> > If we just as easily derive the number of PWMs from the specific
> > compatible string in this document:
> >
> > - "pwm1-6": the six per PWM clocks for mt7622
> >
> > This looks like all "pwm1", "pwm2", "pwm3", "pwm4", "pwm5", "pwm6" is
> > required property in DT, It doesn't make sense.
>
> I don't understand. Why doesn't that make sense? If your hardware block
> has 6 PWMs and each can be driven by its own clock, then you need to
> provide references for each of those clocks, otherwise you won't be able
> to use them.
Thank you for your instruction,
I will add all clock-names and clocks according to
hardware blocks instead of value of <num-pwms> in DT.
> >
> > So we removed those descriptions and added
> >
> > - "pwm1-N": the PWM clocks for each channel
> >
> >
> > But the max number of clocks from the compatible string are still
> > important information that should be provide in this document.
> >
> >
> > What do you think of this?
> >
> > - "pwm1-N": per PWM clocks for mt2712, the max number of PWM channels
> > is 8
> >
> > - "pwm1-N": per PWM clocks for mt7622, the max number of PWM channels
> > is 6
> >
> > - "pwm1-N": per PWM clocks for mt7623, the max number of PWM channels
> > is 5
>
> That's what's in the bindings already, isn't it?
>
> - clocks: phandle and clock specifier of the PWM reference clock.
> - clock-names: must contain the following, except for MT7628 which
> 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
Yes, You are right,
I will keep original description and remove "pwm1-N" from this patch.
- "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
Actually, MT7629 also use "mediatek,mt7622-pwm" as compatible string,
but it's hardware only support one pwm, so I was wrongly stick by
expecting "pwm1-N" in clock-names based on "num-pwms" in DT.
(that we can assign num-pwms to 1 and only provide pwm1 as clock-names)
Maybe added mt7629 description to this document can solve this simply.
- "pwm1": the PWM1 clock for mt7629
> Note that the description of the "clocks" property isn't quite accurate.
> It should be something like:
>
> - clocks: One phandle and clock specifier for each entry in the
> "clock-names" property.
>
> In the above you clearly describe which PWMs you have to specify for
> each generation of the hardware block.
Thanks for your advise, I will update this description
- clocks: One phandle and clock specifier for each entry in the
"clock-names" property.
> >
> > where N starting from 1 to the maximum number of PWM channels
> > - num-pwms: the number of PWM channels.
>
> That's redundant information. The specific number of PWMs in already
> implied by the compatible string, so you don't need to duplicate that
> information here.
Okay, I will remove it.
Thanks,
Best Regards,
Sam
On Fri, Sep 20, 2019 at 06:49:03AM +0800, Sam Shih wrote:
> We can use fixed-clock to repair mt7628 pwm during configure from
> userspace. The SoC is legacy MIPS and has no complex clock tree.
> Due to we can get clock frequency for period calculation from DT
> fixed-clock, so we can remove has-clock property, and directly
> use devm_clk_get and clk_get_rate.
>
> Signed-off-by: Ryder Lee <[email protected]>
> Signed-off-by: Sam Shih <[email protected]>
> Acked-by: Uwe Kleine-K? <[email protected]>
> ---
> Changes since v9:
> Added an Acked-by tag
Argh, my name was croped and ended up in this state in
5c50982af47ffe36df3e31bc9e11be5a067ddd18. Thierry, any chance to repair
that? Something
like
git filter-branch --msg-filter 'sed "s/Kleine-K? /Kleine-K?nig /"' linus/master..
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Wed, Sep 25, 2019 at 08:30:03AM +0200, Uwe Kleine-König wrote:
> On Fri, Sep 20, 2019 at 06:49:03AM +0800, Sam Shih wrote:
> > We can use fixed-clock to repair mt7628 pwm during configure from
> > userspace. The SoC is legacy MIPS and has no complex clock tree.
> > Due to we can get clock frequency for period calculation from DT
> > fixed-clock, so we can remove has-clock property, and directly
> > use devm_clk_get and clk_get_rate.
> >
> > Signed-off-by: Ryder Lee <[email protected]>
> > Signed-off-by: Sam Shih <[email protected]>
> > Acked-by: Uwe Kleine-Kö <[email protected]>
> > ---
> > Changes since v9:
> > Added an Acked-by tag
>
> Argh, my name was croped and ended up in this state in
> 5c50982af47ffe36df3e31bc9e11be5a067ddd18. Thierry, any chance to repair
> that? Something
> like
>
> git filter-branch --msg-filter 'sed "s/Kleine-Kö /Kleine-König /"' linus/master..
Done, though I ended up doing it manually. I don't trust my git
filter-branch skills. =)
Thierry