Re: [PATCH 3/3] lpd: marvell: do not exit early on empty /sys/class/leds dir (OPAL)

On 11/08/2016 09:09 AM, Vasant Hegde wrote:
>> @@ -293,6 +293,7 @@ opal_indicator_probe(void)
<...>
>>       led_dir = open_sysfs_led_dir();
>>       if (!led_dir)
>> @@ -314,6 +315,18 @@ opal_indicator_probe(void)
>>           return 0;
>>       }
>
> In theory, open_sysfs_led_dir() may fail. You are returning if
> open_sysfs_led_dir() fails.. I think we have to fix that.

Nice catch.

> Also I'm thinking of splitting this function. How about splitting this
> to two function
>   - One detects platform LEDS and second one for Marvell LED ?

Okay, sure.  Implemented in v2.

>> +    /*
>> +     * Marvell HDD LEDs are not presented/controlled via kernel LEDs
>> +     * (i.e., /sys/class/leds), and some OPAL systems might not have
>> +     * any kernel LEDs (e.g., modules not loaded) but still have the
>> +     * Marvell SATA controller with LEDs available, and able to work.
>> +     */
>> +    get_mv_indices(LED_TYPE_IDENT, &list);
>> +    if (list) {
>> +        free_indicator_list(list);
>
> +close_sysfs_led_dir(led_dir);

Nice catch.

>
> -Vasant
>

Again, thanks for the review.  I'll test the v2 and submit once OK.

Kind regards,

-- 
Mauricio Faria de Oliveira
IBM Linux Technology Center


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi