On Wed, Mar 22, 2017 at 03:29:33PM +0200, Claudiu Beznea wrote:
> Changes in v3:
> - since v2 introduced per-IP register layout there is no need
> to keep update_cdty and set_cprd_cdty members in atmel_pwm_data
> data structure used in v2; doing in this way the atmel_pwm_data
> data structure will remain with only one member, regs. To avoid
> another level of indirection, the atmel_pwm_data has been removed
> and only atmel_pwm_registers data structure has been keept. In
> this way, "data" member of atmel_pwm_chip data structure was
> replaced by "regs" member; due to these changes prototype of
> atmel_pwm_get_driver_data() function was also changed;
> also, driver private data variables were renamed in the following
> format "atmel_pwm_regs_v*"
> - there is no need for the following checks at the begining
> of atmel_pwm_apply() which were present in v2:
>
> if (cstate.enabled &&
> (cstate.polarity != state->polarity ||
> cstate.period != state->period))
> pwm_reset = true;
>
> it is enough to add:
>
> if (state->enabled) {
> if (cstate.enabled &&
> cstate.polarity == state->polarity &&
> cstate.period == state->period) {
>
> inside "if (state->enabled)" block and also to check cstate.enabled
> instead of checking pwm_reset variable introduced in v2:
>
> if (cstate.enabled) {
> atmel_pwm_disable(chip, pwm, false);
> } else {
> ret = clk_enable(atmel_pwm->clk);
> if (ret) {
> dev_err(chip->dev, "failed to enable clock\n");
> return ret;
> }
> }
>
> Changes in v2:
> - update only duty factor without disabling the PWM channel
> - if PWM channel is enabled, period, as signal polarity, is
> updated by disabling + enabling the PWM channel
> - atmel_pwm_config_prepare() function has been removed and
> added instead two functions, one to compute the CPRD+Prescaler
> (atmel_pwm_calculate_cprd_and_pres()), one to compute CRDY
> (atmel_pwm_calculate_cdty())
> - atmel_pwm_config_set() body was directly moved to atmel_pwm_apply()
> - add 3 new members to atmel_pwm_data: update_cdty, set_cprd_cdty and
> regs:
> - update_cdty is called to configure duty factor without
> disabling PWM channel, when necessary
> - set_cprd_cdty is called to configure both period and
> duty factor parameters
> - regs keeps the period and duty registers and was added to
> have common functions for update_cdty and set_cprd_cdty
> members of atmel_pwm_data for all boards;
> - add a new parameter to atmel_pwm_disable(); this will be used in
> updating period + signal polarity by disabling + enabling the
> PWM channel. In this case, there is no need to disable PWM clock
> since new configuration will be imediately applied.
> - adapted the other reviewer comments excepts the one regarding
> "cdty = cprd - cycles;" from atmel_pwm_calculate_cdty() since
> in atmel_pwm_apply(), selecting polarity in the other way arround
> than is currently done in this commit will need the changing of DPOLI
> bit from Channel Mode Register, in order to keep the initial
> output level of PWM channel after disable operation; this works
> for sama5d2 but not for sam9rl which hasn't document the DPOLI
> bit in datasheet; sama5d3 also hasn't document the DPOLI bit in
> datasheet; one option was to have different aproach for different
> boards but the code becomes messy.
>
> Claudiu Beznea (2):
> drivers: pwm: pwm-atmel: switch to atomic PWM
> drivers: pwm: pwm-atmel: enable PWM on sama5d2
>
> .../devicetree/bindings/pwm/atmel-pwm.txt | 1 +
> drivers/pwm/pwm-atmel.c | 276 ++++++++++-----------
> 2 files changed, 133 insertions(+), 144 deletions(-)
Applied, thanks.
Thierry
Attachment:
signature.asc
Description: PGP signature
