Re: [PATCH 4/5] leds: leds-pwm: implement PWM inversion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/04/2014 at 09:52:45 +0100, Russell King - ARM Linux wrote :
> On Mon, Apr 07, 2014 at 10:46:51AM +0200, Alexandre Belloni wrote:
> > 
> > This will conflict with my patch (which is still lacking proper review)
> > there:
> > http://thread.gmane.org/gmane.linux.leds/482
> > 
> > I would say that it is better to hide the polarity inversion in the PWM
> > driver for your specific PWM. Else we will end up with all the drivers
> > using PWMs trying to detect whether the PWM supports inversion and if it
> > is not the case, calculating the inverted duty cycle.
> > 
> > So, I would go for my patch which is adding the missing polarity
> > inversion setting when using platform data and then implement software
> > polarity inversion in your underlying PWM driver. That also avoids patch
> > 5/5 and I believe not adding a DT property is always a good idea.
> > 
> > What is your PWM that is not supporting polarity inversion ?
> 
> Did you read the commit message properly, particularly the last sentence
> of the first paragraph which refers to the problem with your approach?
> 

If disabling the PWM is putting the output low, I would then enable the
channel on pwm_request() and disable it on pwm_free() because anyway,
you'll have to let it run continuously to get the correct result.
However, Thierry seems to believe that there is an other underlying
issue in the PWM driver when this happens.

Going with your approach will end up with all the drivers trying to use
PWMs having to use the same logic and I'm sure a lot of people will get
confused between polarity inversion and using active_low...

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux