Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver

[Dan Murphy <dmurphy@ti.com>]

On 4/15/19 9:03 AM, Pavel Machek wrote:
> 
>>>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>>>> index 735009e73414..688bb9a6f275 100644
>>>> --- a/drivers/leds/Kconfig
>>>> +++ b/drivers/leds/Kconfig
>>>> @@ -776,9 +776,15 @@ config LEDS_NIC78BX
>>>>  	  To compile this driver as a module, choose M here: the module
>>>>  	  will be called leds-nic78bx.
>>>>  
>>>> +config LEDS_LM3697
>>>> +	tristate "LED driver for LM3697"
>>>> +	depends on LEDS_TI_LMU_COMMON
>>>> +	help
>>>> +	  Say Y to enable the LM3697 LED driver for TI LMU devices.
>>>> +	  This supports the LED device LM3697.
>>>> +
>>>>  config LEDS_TI_LMU_COMMON
>>>>  	tristate "LED driver for TI LMU"
>>>> -	depends on REGMAP
>>>>  	help
>>>>            Say Y to enable the LED driver for TI LMU devices.
>>>>            This supports common features between the TI LM3532, LM3631, LM3632,
> ...
>>> Plus we'd normally expect "COMMON" first and then specific driver. Not
>>> sure if Kconfig can handle it out-of-order...
>>
>> OK.  Should I rename the ti_lmu file to leds-common-ti-lmu?
> 
> Oh, no, that is not what I meant.
> 
> You have
> 
> config B
>        depends on A
> 
> config A
> 
> above. We really want
> 
> config A
> 
> config B
>        depends on A
> 

Got it I will rearrange that in v2.

>> This keeps the naming convention the same in the leds directory as well.
>>
>> FYI I will not add your acked-by on the LMU patch that introduced the code unless you approve.
>> Since you found issues with the kconfig
> 
> I did not review it in great detail, so no acked-by's, yet :-).

Thanks
Dan

> 									Pavel
>