Re: [PATCH v2 1/4] PCI: pciehp: Add pciehp_set_indicators() to jointly set LED indicators

From: sathyanarayanan kuppuswamy
Date: Mon Aug 12 2019 - 17:07:38 EST

On 8/12/19 1:40 PM, Lukas Wunner wrote:
On Mon, Aug 12, 2019 at 11:49:23AM -0700, sathyanarayanan kuppuswamy wrote:
On 8/11/19 12:59 PM, Denis Efremov wrote:
+ if ((!PWR_LED(ctrl) || pwr == PWR_NONE) &&
+ (!ATTN_LED(ctrl) || attn == ATTN_NONE))
+ return;
Also I think this condition needs to expand to handle the case whether pwr
!= PWR_NONE and !PWR_LED(ctrl) is true.

you need to return for case, pwr = PWR_ON, !PWR_LED(ctrl)=true ,
!ATTN_LED(ctrl)=false, attn=on
Why should we return in that case? We need to update the Attention
Indicator Control to On.

Attempting to PWR_ON when !PWR_LED(ctrl) is true is incorrect right ? Even if you don't want to return (to handle ATTN part of the function), may be you should skip updating PWR mask and cmd for this case.



Sathyanarayanan Kuppuswamy
Linux kernel developer