| Message ID | 20210406164140.81423-4-clemens.gruber@pqgruber.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v7,1/8] pwm: pca9685: Switch to atomic API | expand |
On Tue, Apr 06, 2021 at 06:41:36PM +0200, Clemens Gruber wrote: > Add the flag and corresponding documentation for the new PWM staggering > mode feature. > > Cc: Rob Herring <robh+dt@kernel.org> > Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com> For the record, I don't like this and still prefer to make this staggering explicit for the consumer by expanding struct pwm_state with an .offset member to shift the active phase in the period. Best regards Uwe
On Wed, Apr 07, 2021 at 07:33:57AM +0200, Uwe Kleine-König wrote: > On Tue, Apr 06, 2021 at 06:41:36PM +0200, Clemens Gruber wrote: > > Add the flag and corresponding documentation for the new PWM staggering > > mode feature. > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com> > > For the record, I don't like this and still prefer to make this > staggering explicit for the consumer by expanding struct pwm_state with > an .offset member to shift the active phase in the period. How are consumers supposed to know which offset to choose? And worse: how should consumers even know that the driver supports phase shifts? Thierry
Hello Thierry, On Fri, Apr 09, 2021 at 02:27:34PM +0200, Thierry Reding wrote: > On Wed, Apr 07, 2021 at 07:33:57AM +0200, Uwe Kleine-König wrote: > > On Tue, Apr 06, 2021 at 06:41:36PM +0200, Clemens Gruber wrote: > > > Add the flag and corresponding documentation for the new PWM staggering > > > mode feature. > > > > > > Cc: Rob Herring <robh+dt@kernel.org> > > > Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com> > > > > For the record, I don't like this and still prefer to make this > > staggering explicit for the consumer by expanding struct pwm_state with > > an .offset member to shift the active phase in the period. > > How are consumers supposed to know which offset to choose? And worse: > how should consumers even know that the driver supports phase shifts? I'm aware that we're a long way from being able to use this. The clean approach would be to get the offset from the device tree in the same way as the period. And in the meantime I agree that introducing a flag that allows to shift the active part in the period is a sane idea. So I suggest we concentrate on getting the details in the corresponding discussion straight. Best regards Uwe
Hello Rob, On Tue, Apr 06, 2021 at 06:41:36PM +0200, Clemens Gruber wrote: > Add the flag and corresponding documentation for the new PWM staggering > mode feature. > > Cc: Rob Herring <robh+dt@kernel.org> For now reviewing this patch is not necessary, we're discussing a better name for this flag. Best regards Uwe
diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt index 084886bd721e..11d539302398 100644 --- a/Documentation/devicetree/bindings/pwm/pwm.txt +++ b/Documentation/devicetree/bindings/pwm/pwm.txt @@ -46,6 +46,7 @@ period in nanoseconds. Optionally, the pwm-specifier can encode a number of flags (defined in <dt-bindings/pwm/pwm.h>) in a third cell: - PWM_POLARITY_INVERTED: invert the PWM signal polarity +- PWM_STAGGERING_ALLOWED: allow delayed ON-time for improved EMI Example with optional PWM specifier for inverse polarity diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h index ab9a077e3c7d..51d67dec1aad 100644 --- a/include/dt-bindings/pwm/pwm.h +++ b/include/dt-bindings/pwm/pwm.h @@ -11,5 +11,6 @@ #define _DT_BINDINGS_PWM_PWM_H #define PWM_POLARITY_INVERTED (1 << 0) +#define PWM_STAGGERING_ALLOWED (1 << 1) #endif
Add the flag and corresponding documentation for the new PWM staggering mode feature. Cc: Rob Herring <robh+dt@kernel.org> Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com> --- Documentation/devicetree/bindings/pwm/pwm.txt | 1 + include/dt-bindings/pwm/pwm.h | 1 + 2 files changed, 2 insertions(+)