Re: [PATCH 19/31] net: realtek: changing LED_* from enum led_brightness to actual value

[Larry Finger]

On 1/21/22 11:12, Hin-Tak Leung wrote:
> > -        if (brightness == LED_FULL) {
> > +        if (brightness == 255) {
>
> > -        if (brightness == LED_OFF) {
> > +        if (brightness == 0) {
>
> NAKed. I haven't received the other 30 patches in this series so I don't know the full context, but I don't think replacing meaningful enum names with numerical values is an improvement. If the ENUMs are gone from a common include (and why is the ENUM removed, if drivers use them??), and the realtek driver still have such a functionality, it probably should be defined in one of the rtl818*.h as RTL818X_LED_FULL and RTL818X_LED_OFF .
>
> My $0.02, based on this one only of the 31.

I agree. Substituting magic numbers for enum names should not be done. NACK.

Larry