[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANOLnOM6dp2_WthKUo31X_0os4cwax9Y42qSLqi09XiR-HF-Uw@mail.gmail.com>
Date: Thu, 8 Nov 2012 13:53:43 +0200
From: Grazvydas Ignotas <notasas@...il.com>
To: Péter Ujfalusi <peter.ujfalusi@...com>
Cc: Thierry Reding <thierry.reding@...onic-design.de>,
Tero Kristo <t-kristo@...com>, linux-kernel@...r.kernel.org,
linux-omap@...r.kernel.org
Subject: Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030
series of PMICs
On Thu, Nov 8, 2012 at 9:14 AM, Péter Ujfalusi <peter.ujfalusi@...com> wrote:
> On 11/07/2012 06:50 PM, Grazvydas Ignotas wrote:
>>> + if (pwm->hwpwm) {
>>> + /* PWM 1 */
>>> + mask = TWL4030_GPIO7_VIBRASYNC_PWM1_MASK;
>>> + bits = TWL4030_GPIO7_VIBRASYNC_PWM1_PWM1;
>>> + } else {
>>> + /* PWM 0 */
>>> + mask = TWL4030_GPIO6_PWM0_MUTE_MASK;
>>> + bits = TWL4030_GPIO6_PWM0_MUTE_PWM0;
>>> + }
>>> +
>>> + /* Save the current MUX configuration for the PWM */
>>> + twl->twl4030_pwm_mux &= ~mask;
>>> + twl->twl4030_pwm_mux |= (val & mask);
>>
>> Do we really need this mask clearing here? After probe twl4030_pwm_mux
>> should be zero, and if twl4030_pwm_request is called twice you don't
>> clear the important bits before |=, I think 'twl4030_pwm_mux = val &
>> mask' would be better here.
>
> I'm storing both PWM's state in the same variable, but in different offsets:
> PWM0: bits 2-3
> PWM1: bits 4-5
> Probably it is over engineering to clear the relevant bits in the backup
> storage, but better to be safe IMHO.
> I would leave this part as it is.
Oh, it should be good then.
--
Gražvydas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists