Hi Uwe, Thanks for your review comments. On Sun, May 23 2021, Uwe Kleine-König wrote: > On Wed, May 19, 2021 at 10:48:44AM +0300, Baruch Siach wrote: >> Driver for the PWM block in Qualcomm IPQ6018 line of SoCs. Based on >> driver from downstream Codeaurora kernel tree. Removed support for older >> (V1) variants because I have no access to that hardware. >> >> Tested on IPQ6010 based hardware. >> >> Signed-off-by: Baruch Siach <baruch@xxxxxxxxxx> [...] >> +static void ipq_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) >> +{ >> + struct ipq_pwm_chip *ipq_chip = to_ipq_pwm_chip(pwm->chip); >> + unsigned offset = ipq_pwm_reg_offset(pwm, PWM_CFG_REG1); >> + unsigned long val; >> + >> + val = readl(ipq_chip->mem + offset); >> + val |= PWM_UPDATE; > > What is the effect of this register bit? > > Does the output become inactive or does it freeze at state that happens > to be emitted when the ENABLE bit is removed? I don't know. PWM does not work when this bit is not set here. The original downstream driver[1] does not set this bit on disable. But it also enables PWM unconditionally on .config. I added the 'enabled' check in .config, and then PWM stopped working even when enabled later. It was only by accident (excess copy/paste) that I found this workaround. A comment on the original code says that PWM_UPDATE is "auto cleared". This is evidently not true on my hardware (IPQ6010). This might be true for older variants of this PWM block. Unfortunately, I have no access to hardware documentation. [1] https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/drivers/pwm/pwm-ipq.c?h=NHSS.QSDK.11.4.1.r1&id=9e4627b7088b0c06ddd910c8770274d26613de9e baruch -- ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch@xxxxxxxxxx - tel: +972.52.368.4656, http://www.tkos.co.il -