Re: [PATCH v2 3/4] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support

[Markus Elfring]

>>> +++ b/drivers/gpu/drm/solomon/ssd130x.c
>>> @@ -146,6 +146,33 @@
>>>  #define SSD133X_COLOR_DEPTH_256			0x0
>>>  #define SSD133X_COLOR_DEPTH_65K			0x1
>>>  
>>> +/* ssd135x commands */
>>> +#define SSD135X_SET_COL_RANGE			0x15
>>> +#define SSD135X_WRITE_RAM			0x5c
>>> +#define SSD135X_SET_ROW_RANGE			0x75
>> [...]
>>
>> How do you think about to use an enumeration for such data?
>> https://en.wikipedia.org/wiki/Enumerated_type#C_and_syntactically_similar_languages
> 
> Thank you for the suggestion.
> 
> I used #define to stay consistent with the rest of ssd130x.c, where the
> command constants for the other families are all defined the same way.

Will any further adjustments become more interesting?


> In my opinion an enum could be a readable solution for these values,

This is nice.


> but I don't think the switch should be included in this series.
What does hinder you to take another design option better into account?

Regards,
Markus