[PATCH] pwm: tiehrpwm: Fix disabling of output of PWMs

[PATCH] pwm: tiehrpwm: Fix disabling of output of PWMs

[Christoph Vogtländer]

[-- Attachment #1: Type: text/plain, Size: 1603 bytes --]

pwm-tiehrpwm driver disables PWM output by putting it in low output
state via active AQCSFRC register in ehrpwm_pwm_disable(). But, the
AQCSFRC shadow register is not updated. Therefore, when shadow AQCSFRC
register is re-enabled in ehrpwm_pwm_enable() (say to enable second PWM
output), previous settings are lost as shadow register value is loaded
into active register. This results in things like PWMA getting enabled
automatically, when PWMB is enabled and vice versa. Fix this by
updating AQCSFRC shadow register as well during ehrpwm_pwm_disable().


This is a follow up to commit 38dabd91ff0bde33352ca3cc65ef515599b77a05.
It must be made sure that immediate mode is not already set when the shadow
register value is set in ehrpwm_pwm_disable(). Otherwise modifications to the
action-qualifier continuous S/W force register will be done in the active register.
This may happen when both channels get disabled. Only the first channel state
will be recorded as disabled in the shadow register. When enabling the first
channel again, the second channel would be enabled as well.
Setting RLDCSF to zero, first. This ensures that the shadow register is updated
as desired.

Please CC me in replies as I'm not a member of the list.
--
Best Regards
Christoph Vogtländer

SIGMA Surface Science GmbH
Software Development
Configuration Manager

Tel: +49 (0)6128/85905-190
Fax: +49 (0)6128/85905-200

Idsteiner Str. 78
D-65232 Taunusstein


Sigma Surface Science GmbH, Idsteiner Str. 78, 65232 Taunusstein. Amtsgericht Wiesbaden, HRB 27422. Geschäftsführer: Norbert Nold

[-- Attachment #2: 0002-pwm-tiehrpwm-Fix-disabling-of-output-of-PWMs.patch --]
[-- Type: application/octet-stream, Size: 1460 bytes --]

From 4e20fc85d20ce1f42b637e1a88ffc22d91457518 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20Vogtl=C3=A4nder?=
 <c.vogtlaender@sigma-surface-science.com>
Date: Tue, 4 Sep 2018 17:00:18 +0200
Subject: [PATCH] pwm: tiehrpwm: Fix disabling of output of PWMs

pwm-tiehrpwm driver disables PWM output by putting it in low output
state via active AQCSFRC register in ehrpwm_pwm_disable(). But, the
AQCSFRC shadow register is not updated. Therefore, when shadow AQCSFRC
register is re-enabled in ehrpwm_pwm_enable() (say to enable second PWM
output), previous settings are lost as shadow register value is loaded
into active register. This results in things like PWMA getting enabled
automatically, when PWMB is enabled and vice versa. Fix this by
updating AQCSFRC shadow register as well during ehrpwm_pwm_disable().
---
 drivers/pwm/pwm-tiehrpwm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 467ccd1..7688b8a 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -383,6 +383,8 @@ static void ehrpwm_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	}
 
 	/* Update shadow register first before modifying active register */
+	ehrpwm_modify(pc->mmio_base, AQSFRC, AQSFRC_RLDCSF_MASK,
+			AQSFRC_RLDCSF_ZRO);
 	ehrpwm_modify(pc->mmio_base, AQCSFRC, aqcsfrc_mask, aqcsfrc_val);
 	/*
 	 * Changes to immediate action on Action Qualifier. This puts
-- 
2.7.4

Re: [PATCH] pwm: tiehrpwm: Fix disabling of output of PWMs

[Vignesh R]

Hi Christoph,

On Tuesday 04 September 2018 09:34 PM, Christoph Vogtl�nder wrote:
> pwm-tiehrpwm driver disables PWM output by putting it in low output
> state via active AQCSFRC register in ehrpwm_pwm_disable(). But, the
> AQCSFRC shadow register is not updated. Therefore, when shadow AQCSFRC
> register is re-enabled in ehrpwm_pwm_enable() (say to enable second PWM
> output), previous settings are lost as shadow register value is loaded
> into active register. This results in things like PWMA getting enabled
> automatically, when PWMB is enabled and vice versa. Fix this by
> updating AQCSFRC shadow register as well during ehrpwm_pwm_disable().
> 
> 
> This is a follow up to commit 38dabd91ff0bde33352ca3cc65ef515599b77a05.
> It must be made sure that immediate mode is not already set when the shadow
> register value is set in ehrpwm_pwm_disable(). Otherwise modifications to the
> action-qualifier continuous S/W force register will be done in the active register.
> This may happen when both channels get disabled. Only the first channel state
> will be recorded as disabled in the shadow register. When enabling the first
> channel again, the second channel would be enabled as well.
> Setting RLDCSF to zero, first. This ensures that the shadow register is updated
> as desired.
> 

Thanks for your work! Your change makes sense.
But, could you submit a formal patch generated by git format-patch and
also use git send-email to send to the mailing list?
Please run ./scripts/checkpatch.pl --strict on the patch and fix
warnings reported.
Documentation/process/submitting-patches.rst in kernel source talks
about how to submit patches to kernel mailing list.


-- 
Regards
Vignesh