Re: [PATCH 1/8] drm/solomon: Rename ssd130x driver to ssd13xx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Maxime Ripard <mripard@xxxxxxxxxx> writes:

Hello Maxime,

Thanks for the feedback.

> Hi,
>
> On Mon, Oct 09, 2023 at 08:34:15PM +0200, Javier Martinez Canillas wrote:
>> The driver only supports the SSD130x family of Solomon OLED controllers
>> now, but will be extended to also support the SSD132x (4-bit grayscale)
>> and SSD133x (16-bit RGB) controller families. Rename driver to ssd13xx.
>> 
>> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
>
> I'm not sure it's worth it. I understand what you want to achieve, but
> this will create conflicts, affect the configuration of everyone
> enabling that driver, etc.
>
> And we have plenty of drivers that don't match all the devices they
> support anyway.
>

Yeah, I was on the fence and even discussed this with others. I'm OK with
dropping this patch if the agreegment is that isn't worth it to make the
name more accurate.

> Plus ....
>
>> @@ -11,10 +11,10 @@
>>  #include <linux/i2c.h>
>>  #include <linux/module.h>
>>  
>> -#include "ssd130x.h"
>> +#include "ssd13xx.h"
>>  
>> -#define DRIVER_NAME	"ssd130x-i2c"
>> -#define DRIVER_DESC	"DRM driver for Solomon SSD130x OLED displays (I2C)"
>> +#define DRIVER_NAME	"ssd13xx-i2c"
>> +#define DRIVER_DESC	"DRM driver for Solomon SSD13xx OLED displays (I2C)"
>>  
>>  static const struct regmap_config ssd130x_i2c_regmap_config = {
>>  	.reg_bits = 8,
>
> ... We now end up with a lot of inconsistencies where some things are
> called ssd130x and others ssd13xx.
>

Yes, but I fix that in patch #2.

> Maxime

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux