ok, thank you, so mainline dts is also wrong and seans patch useless…or does pwm7 basicly exists on other mt7622 boards? if not also pinctrl is wrong. or is only another gpio used here (and so other pinctrl-definition)?
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/pinctrl/mediatek/pinctrl-mt7622.c?h=linux-5.4.y#n442
says pwm7 is available on mt7622 on gpio 70,82 or 101
any idea about pwm3 (inversion and not working in my tests)?
I found inversion code for mt8173…maybe this is similar to mt7622
https://android.googlesource.com/kernel/mediatek/+/android-mtk-3.18/drivers/misc/mediatek/pwm/mt8173/mt_pwm_hal.c#514
Register is defined here as offset to pwm-base:
https://android.googlesource.com/kernel/mediatek/+/android-mtk-3.18/drivers/misc/mediatek/pwm/mt8173/include/mach/mt_pwm_prv.h#35
In mt7622document there is a clk_pwm_inv as bit 4 in clk_cfg_1 (page 17),but this seems to be for all pwm
@moore Btw. From mt7622 datasheet
I think i’ve found it…on page 528
based on enable (11006000 on mt7622) (where pwm_num is a bit too - not like the pwmX-registers PWMx_*)
value = readl(pc->regs);
value |= BIT(pwm->hwpwm);
writel(value, pc->regs);
imho we need to do something like this:
value = readl(pc->regs + 0x1d0);
value |= BIT(pwm_no +16); // set aux bit for pwmX
writel(value, pc->regs); //write aux-bit
value |= BIT(pwm_no); // set inv bit for pwmX
writel(value, pc->regs); //write inv bit
for disabling we can use “value &= ~BIT(…);”
but i do not know if the offset (0x1d0) is same on other mtk-socs, so we need to define soc-depended const and choose which to use based on current soc
maybe we can set both bits at same time
diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index d2efc47655a0..ba0db66bc5e9 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -197,10 +197,30 @@ static void pwm_mediatek_disable(struct pwm_chip *chip, struct pwm_device *pwm)
pwm_mediatek_clk_disable(chip, pwm);
}
+static int pwm_mediatek_set_polarity(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ enum pwm_polarity polarity)
+{
+ struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
+ u32 val;
+
+ val = readl(pc->regs + 0x1d0);//3DLCM register
+
+ if (polarity == PWM_POLARITY_INVERSED)
+ val |= (BIT(pwm->hwpwm + 16) | BIT(pwm->hwpwm));
+ else
+ val &= ~(BIT(pwm->hwpwm + 16) | BIT(pwm->hwpwm));
+
+ writel(val, pc->regs + 0x1d0);
+
+ return 0;
+}
+
static const struct pwm_ops pwm_mediatek_ops = {
.config = pwm_mediatek_config,
.enable = pwm_mediatek_enable,
.disable = pwm_mediatek_disable,
+ .set_polarity = pwm_mediatek_set_polarity,
.owner = THIS_MODULE,
};
android code sets first the aux-bit/enable so maybe it’s better to do this separately…in the docs there are base bits (offset +8) that should not be set if aux-bits (+16) set.
i created new function to set/clear bits in the 3dclm register and call it from the set_inversion function 3 times to set all 3 bit positions (hope i made no mistake).
https://github.com/frank-w/BPI-R2-4.14/commits/5.7-pwm
now we have to define inversion in dts…
i have found this: https://www.kernel.org/doc/Documentation/devicetree/bindings/pwm/pwm.txt
so we need to do something like this, right?
pwm: pwm {
#pwm-cells = <2>;
};
fan: fan {
pwms = <&pwm 2 5000000 PWM_POLARITY_INVERTED>;
pwm-names = "fan";
};
but imho the driver needs to pick the right parameter as inversion…is this same across drivers? maybe we can skip the default period and only pass inversion mode for the pwm (not changing pwm-cells).
i can compile dtb without the 500000, how to tell driver which value is the inversion one?
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts b/arch/arm
64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
index ef92ec7921dc..2ea1ddb3716b 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
@@ -8,6 +8,7 @@
/dts-v1/;
#include <dt-bindings/input/input.h>
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pwm/pwm.h>
#include "mt7622.dtsi"
#include "mt6380.dtsi"
@@ -427,6 +428,11 @@ mux {
};
};
+ fan: fan-pwm {
+ pwms = <&pwm 2 PWM_POLARITY_INVERTED>;
+ pwm-names = "fan";
+ };
+
wled_pins: wled-pins {
mux {
function = "led";
diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
index 15b46c742f93..5b59f9b98caa 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
@@ -440,6 +440,7 @@ pwm: pwm@11006000 {
<&pericfg CLK_PERI_PWM7_PD>;
clock-names = "top", "main", "pwm1", "pwm2", "pwm3", "pwm4",
"pwm5", "pwm6", "pwm7";
+ #pwm-cells=<2>;
status = "disabled";
};
mhm, it looks like the inversion is parsed by pwm/core.c and first param after pwm_no is always the period
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/pwm/core.c?h=linux-5.4.y#n146
but this inverts not the enable-state of course…so still confusing
