linux-kernel - Re: [PATCH] leds: rgb: leds-qcom-lpg: Add support for 6-bit PWM resolution

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: <e97d17ca-be8c-42a2-9767-77d73eaa916c@oss.qualcomm.com>
Date: Thu, 20 Feb 2025 13:30:53 -0800
From: Anjelique Melendez <anjelique.melendez@....qualcomm.com>
To: Lee Jones <lee@...nel.org>
Cc: pavel@...nel.org, andersson@...nel.org, krzysztof.kozlowski@...aro.org,
        morf3089@...il.com, u.kleine-koenig@...gutronix.de,
        marijn.suijten@...ainline.org, linux-leds@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        Zejiong Huang <zejiongh@....qualcomm.com>
Subject: Re: [PATCH] leds: rgb: leds-qcom-lpg: Add support for 6-bit PWM
 resolution



On 2/20/2025 6:55 AM, Lee Jones wrote:
> On Wed, 12 Feb 2025, Anjelique Melendez wrote:
> 
>> Currently, driver only allows for PWM modules to use 9-bit resolution.
>> However, PWM modules can support 6-bit and 9-bit resolution. Add support
>> for 6-bit resolution.
>>
>> Suggested-by: Zejiong Huang <zejiongh@....qualcomm.com>
>> Signed-off-by: Anjelique Melendez <anjelique.melendez@....qualcomm.com>
>> ---
>>   drivers/leds/rgb/leds-qcom-lpg.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
>> index f3c9ef2bfa57..4e5c56ded1f0 100644
>> --- a/drivers/leds/rgb/leds-qcom-lpg.c
>> +++ b/drivers/leds/rgb/leds-qcom-lpg.c
>> @@ -24,6 +24,7 @@
>>   #define LPG_PATTERN_CONFIG_REG	0x40
>>   #define LPG_SIZE_CLK_REG	0x41
>>   #define  PWM_CLK_SELECT_MASK	GENMASK(1, 0)
>> +#define  PWM_SIZE_SELECT_MASK	BIT(2)
> 
> Are you sure you want to shove this between 2 seemingly related defines?
> 
We placed the PWM_SIZE_SELECT_MASK here so we could group all the masks 
used for PWM together then the masks used for Hi resolution PWM together
i.e
   1. pwm clk mask
   2. pwm size mask
   3. hi resolution pwm clk mask
   4. hi resolution pwm size mask

Would you rather have definitions grouped based on mask type?
i.e
   1. pwm clk mask
   2. hi resolution pwm clk mask
   3. pwm size mask
   4. hi resolution pwm size mask


>>   #define  PWM_CLK_SELECT_HI_RES_MASK	GENMASK(2, 0)
>>   #define  PWM_SIZE_HI_RES_MASK	GENMASK(6, 4)
>>   #define LPG_PREDIV_CLK_REG	0x42

Thanks,
Anjelique