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

From: Javier Martinez Canillas
Date: Tue Oct 10 2023 - 04:39:55 EST


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