linux-kernel - Re: [PATCH v17] pwm: opencores: Add PWM driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <AM7P189MB10091DAF204BB660E619A01FE3112@AM7P189MB1009.EURP189.PROD.OUTLOOK.COM>
Date: Tue, 7 Jan 2025 07:37:13 +0100
From: Maud Spierings <maud_spierings@...mail.com>
To: Uwe Kleine-König <ukleinek@...nel.org>
Cc: william.qiu@...rfivetech.com, hal.feng@...rfivetech.com,
 linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org,
 p.zabel@...gutronix.de
Subject: Re: [PATCH v17] pwm: opencores: Add PWM driver support

Hello Uwe,

On 1/7/25 6:07 AM, Uwe Kleine-König wrote:
> Hello Maud,
>
> On Mon, Jan 06, 2025 at 07:59:23PM +0100, Maud Spierings wrote:
>> Hello William,
>>
>> I've once again put the patch to the test, and it seems the oops is
>> resolved.
>>
>> I did notice something odd though, when controlling the backlight  bl_power
>> 0 means the backlight is on and controllable, 1 seems like off, but instead
>> sets the screen to maximum brightness and then stops listening to any value
>> echoed into brightness.
> Note that for bl_power 0 is on and 4 is off. Still the behaviour you
> report sounds wrong. Quickly looking in the pwm_bl driver, I don't spot
> something obvious.

you are indeed correct thats alittle odd but ok.
>> The brightness is also reversed from what would be logical, so 255 is off
>> and 0 is maximum.
>>
>> Now the little text at the top specifies that the hardware only does
>> inverted polarity, which I guess explains this, but I don't understand it.
> The backlight's operation should still be fine, its usage be independent
> of the PWM's details.
>
>> I also encountered this when I got an error to start with so I had to add
>> PWM_POLARITY_INVERTED to my pwm-backlight definition.
> That makes me suspect the problem is on your end. If you add
> PWM_POLARITY_INVERTED the result is that the pwm_bl driver still
> configures duty_cycle=0 for backlight off, but you then get a constant
> high output.
>
> So with the hardware capabilities (i.e. not being able to emit a
> constant low output) I think you need to not use PWM_POLARITY_INVERTED
> and accept that completely off doesn't work (unless you have an
> additional GPIO or regulator to disable the backlight).

If I do not set PWM_POLARITY_INVERTED in the dts it will cause an EINVAL 
in ocores_pwm_apply

>> But I don't understand why it isn't supported. Wouldn't supporting non
>> inverted polarity be a very simple calculation? 40% negative duty cycle is
>> of course equal to 60% positive duty cycle, 20% N == 80% P etc. I don't see
>> why the hardware would specifically have to support this.
> If you only care about the mean voltage level (as is the case for a
> backlight), that's right. But only then. And if the hardware cannot emit
> a constant low signal, this doesn't help you.
>
> Best regards
> Uwe

I did some more digging, took a look at the pwm-backlight driver in the 
vendor kernel, and it seems there are some tweaks there [1] (all the way 
at the bottom). And it explains some things, first off why it didn't 
error before without the inverted polarity, because it sets this value 
in the pwm-backlight driver in that kernel.

but the interesting line is this one:
|brightness = (u8)~brightness;

Which is doing what I thought to do in the end of my previous email.

So maybe the hardware is indeed the odd one out here, sadly I have no 
access to the design of the board.
But there is definitely# something weird going on here, not quite sure 
where to fix it.
|

[1]: 
https://github.com/DC-DeepComputing/fml13v01-linux/commit/cc7131dd58e068c58e54f8c8fd23c5ac41c33f46

Kind regards,

Maud