Hi Jacek, I have testet with a i.MX6 custom board. It is possible to use qemu to test it. Using the smdkc210 machine with modified device tree: ```patch diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts index 7c39dd1c4d3a..9ca10f69b3a2 100644 --- a/arch/arm/boot/dts/exynos4210-trats.dts +++ b/arch/arm/boot/dts/exynos4210-trats.dts @@ -26,8 +26,8 @@ }; chosen { - bootargs = "root=/dev/mmcblk0p5 rootwait earlyprintk panic=5"; - stdout-path = "serial2:115200n8"; + bootargs = "console=ttySAC0 rdinit=/sbin/init root=/dev/mmcblk0p5 rootwait earlyprintk panic=5"; + stdout-path = "serial0:115200n8"; }; regulators { @@ -124,7 +124,7 @@ fixed-rate-clocks { xxti { compatible = "samsung,clock-xxti"; - clock-frequency = <0>; + clock-frequency = <24000000>; }; xusbxti { @@ -148,12 +148,27 @@ }; }; + pwm-leds { + compatible = "pwm-leds"; + led { + label = "led"; + pwms = <&pwm 0 1000000000 0>; + max-brightness = <255>; + default-state = "keep"; + }; + }; + +}; + +&pwm { + status = "okay"; + samsung,pwm-outputs = <0>; }; &camera { pinctrl-names = "default"; pinctrl-0 = <>; - status = "okay"; + status = "disabled"; }; &cpu0 { @@ -166,7 +181,7 @@ samsung,burst-clock-frequency = <500000000>; samsung,esc-clock-frequency = <20000000>; samsung,pll-clock-frequency = <24000000>; - status = "okay"; + status = "disabled"; panel@0 { reg = <0>; @@ -196,15 +211,16 @@ }; }; }; + }; &exynos_usbphy { - status = "okay"; + status = "disabled"; vbus-supply = <&safe1_sreg>; }; &fimc_0 { - status = "okay"; + status = "disabled"; assigned-clocks = <&clock CLK_MOUT_FIMC0>, <&clock CLK_SCLK_FIMC0>; assigned-clock-parents = <&clock CLK_SCLK_MPLL>; @@ -212,7 +228,7 @@ }; &fimc_1 { - status = "okay"; + status = "disabled"; assigned-clocks = <&clock CLK_MOUT_FIMC1>, <&clock CLK_SCLK_FIMC1>; assigned-clock-parents = <&clock CLK_SCLK_MPLL>; @@ -220,7 +236,7 @@ }; &fimc_2 { - status = "okay"; + status = "disabled"; assigned-clocks = <&clock CLK_MOUT_FIMC2>, <&clock CLK_SCLK_FIMC2>; assigned-clock-parents = <&clock CLK_SCLK_MPLL>; @@ -228,7 +244,7 @@ }; &fimc_3 { - status = "okay"; + status = "disabled"; assigned-clocks = <&clock CLK_MOUT_FIMC3>, <&clock CLK_SCLK_FIMC3>; assigned-clock-parents = <&clock CLK_SCLK_MPLL>; @@ -236,18 +252,18 @@ }; &fimd { - status = "okay"; + status = "disabled"; }; &gpu { - status = "okay"; + status = "disabled"; }; &hsotg { vusb_d-supply = <&vusb_reg>; vusb_a-supply = <&vusbdac_reg>; dr_mode = "peripheral"; - status = "okay"; + status = "disabled"; }; &i2c_3 { @@ -256,7 +272,7 @@ samsung,i2c-max-bus-freq = <400000>; pinctrl-0 = <&i2c3_bus>; pinctrl-names = "default"; - status = "okay"; + status = "disabled"; mms114-touchscreen@48 { compatible = "melfas,mms114"; @@ -276,7 +292,7 @@ samsung,i2c-max-bus-freq = <100000>; pinctrl-0 = <&i2c5_bus>; pinctrl-names = "default"; - status = "okay"; + status = "disabled"; max8997_pmic@66 { compatible = "maxim,max8997-pmic"; ``` To actually see what happens on HW you need to patch qemu as well: ```patch diff --git a/hw/timer/exynos4210_pwm.c b/hw/timer/exynos4210_pwm.c index 9bc03277852d..a1c2bc05b935 100644 --- a/hw/timer/exynos4210_pwm.c +++ b/hw/timer/exynos4210_pwm.c @@ -30,7 +30,7 @@ #include "hw/arm/exynos4210.h" -//#define DEBUG_PWM +#define DEBUG_PWM #ifdef DEBUG_PWM #define DPRINTF(fmt, ...) \ @@ -199,8 +199,8 @@ static void exynos4210_pwm_tick(void *opaque) } if (cmp) { - DPRINTF("auto reload timer %d count to %x\n", id, - p->timer[id].reg_tcntb); + DPRINTF("auto reload timer %d count to 0x%08x and compare to 0x%08x\n", id, + p->timer[id].reg_tcntb, p->timer[id].reg_tcmpb); ptimer_set_count(p->timer[id].ptimer, p->timer[id].reg_tcntb); ptimer_run(p->timer[id].ptimer, 1); } else { ``` With this changes you can test on and off default-state. The on state will print: PWM: [ exynos4210_pwm_tick: 202] auto reload timer 0 count to 0x00b7d73f and compare to 0xffffffff The off state only checks that it is disabled. To test the default-state keep, more changes are needed: ```patch diff --git a/hw/timer/exynos4210_pwm.c b/hw/timer/exynos4210_pwm.c index 9bc03277852d..a1c2bc05b935 100644 --- a/hw/timer/exynos4210_pwm.c +++ b/hw/timer/exynos4210_pwm.c @@ -370,6 +370,12 @@ static void exynos4210_pwm_reset(DeviceState *d) exynos4210_pwm_update_freq(s, s->timer[i].id); ptimer_stop(s->timer[i].ptimer); } + + exynos4210_pwm_write(s, TCON, 4, 4); + exynos4210_pwm_write(s, TCNTB0, 0x00b7d73f, 4); + exynos4210_pwm_write(s, TCMPB0, 0x005b8f57, 4); + exynos4210_pwm_write(s, TCON, 6, 4); + exynos4210_pwm_write(s, TCON, 0xd, 4); } static const MemoryRegionOps exynos4210_pwm_ops = { ``` ```patch diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c index 87a886f7dc2f..cf578a235208 100644 --- a/drivers/pwm/pwm-samsung.c +++ b/drivers/pwm/pwm-samsung.c @@ -387,6 +387,23 @@ static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm, return __pwm_samsung_config(chip, pwm, duty_ns, period_ns, false); } +static void pwm_samsung_get_state(struct pwm_chip *chip, + struct pwm_device *pwm, struct pwm_state *state) +{ + struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip); + u32 tin_rate = pwm_samsung_get_tin_rate(our_chip, pwm->hwpwm); + u32 tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm)); + u32 tcmp = readl(our_chip->base + REG_TCMPB(pwm->hwpwm)); + u32 tcon = readl(our_chip->base + REG_TCON); + u64 tmp; + + state->enabled = !!(tcon & TCON_AUTORELOAD(pwm->hwpwm)); + tmp = NSEC_PER_SEC * (u64)tcmp; + state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, tin_rate); + tmp = NSEC_PER_SEC * (u64)tcnt; + state->period = DIV_ROUND_CLOSEST_ULL(tmp, tin_rate); +} + static void pwm_samsung_set_invert(struct samsung_pwm_chip *chip, unsigned int channel, bool invert) { @@ -430,6 +447,7 @@ static const struct pwm_ops pwm_samsung_ops = { .enable = pwm_samsung_enable, .disable = pwm_samsung_disable, .config = pwm_samsung_config, + .get_state = pwm_samsung_get_state, .set_polarity = pwm_samsung_set_polarity, .owner = THIS_MODULE, }; ``` Without the get_state() a divide by zero is triggered and device is not there. I think it is valid to use default-state keep with a disabled PWM and this may cause a division by zero, I will add a `if (pwmstate->enabled)` to avoid this. Regards Denis Am Dienstag, den 17.03.2020, 21:43 +0100 schrieb Jacek Anaszewski: > Hi Denis, > > On 3/16/20 9:24 PM, Denis Osterland-Heim wrote: > > Hi Jacek, > > > > Am Montag, den 16.03.2020, 19:36 +0100 schrieb Jacek Anaszewski: > > > Hi Denis, > > > > > > On 3/16/20 1:53 PM, Denis Osterland-Heim wrote: > > > > ... > > > > > > > > @@ -92,13 +96,27 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, > > > > > > > > pwm_init_state(led_data->pwm, &led_data->pwmstate); > > > > > > > > + if (led->default_state == LEDS_PWM_DEFSTATE_ON) > > > > + led_data->cdev.brightness = led->max_brightness; > > > > + else if (led->default_state == LEDS_PWM_DEFSTATE_KEEP) { > > > > + uint64_t brightness; > > > > + > > > > + pwm_get_state(led_data->pwm, &led_data->pwmstate); > > > > > > This seems to not be reading the device state, i.e. what you tried > > > to address by direct call to pwm->chip->ops->get_state() before. > > > > > > Am I missing something? > > > > > > > well, not you, but I missed cfc4c189bc70b1acc17e6f1abf1dc1c0ae890bd8. > > Since this commit pwm_get_state() is sufficient. > > I assume you tested it? > > With that, for the whole set: > > Acked-by: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx> > Diehl Connectivity Solutions GmbH Geschäftsführung: Horst Leonberger Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht Nürnberg: HRB 32315 ___________________________________________________________________________________________________ Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen. Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt. - Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter https://www.diehl.com/group/de/transparenz-und-informationspflichten/ The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited. - For general information on data protection and your respective rights please visit https://www.diehl.com/group/en/transparency-and-information-obligations/