On Fri, Feb 11, 2022 at 03:33:55PM +0100, Javier Martinez Canillas wrote:
This adds a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon OLED display controllers.
It's only the core part of the driver and a bus specific driver is needed for each transport interface supported by the display controllers.
...
+#define SSD130X_SET_CLOCK_DIV_MASK GENMASK(3, 0) +#define SSD130X_SET_CLOCK_DIV_SET(val) FIELD_PREP(SSD130X_SET_CLOCK_DIV_MASK, (val)) +#define SSD130X_SET_CLOCK_FREQ_MASK GENMASK(7, 4) +#define SSD130X_SET_CLOCK_FREQ_SET(val) FIELD_PREP(SSD130X_SET_CLOCK_FREQ_MASK, (val)) +#define SSD130X_SET_PRECHARGE_PERIOD1_MASK GENMASK(3, 0) +#define SSD130X_SET_PRECHARGE_PERIOD1_SET(val) FIELD_PREP(SSD130X_SET_PRECHARGE_PERIOD1_MASK, (val)) +#define SSD130X_SET_PRECHARGE_PERIOD2_MASK GENMASK(7, 4) +#define SSD130X_SET_PRECHARGE_PERIOD2_SET(val) FIELD_PREP(SSD130X_SET_PRECHARGE_PERIOD2_MASK, (val))
+#define SSD130X_SET_COM_PINS_CONFIG1_MASK GENMASK(4, 4)
BIT(4)
+#define SSD130X_SET_COM_PINS_CONFIG1_SET(val) FIELD_PREP(SSD130X_SET_COM_PINS_CONFIG1_MASK, (!val)) +#define SSD130X_SET_COM_PINS_CONFIG2_MASK GENMASK(5, 5)
BIT(5)
+#define SSD130X_SET_COM_PINS_CONFIG2_SET(val) FIELD_PREP(SSD130X_SET_COM_PINS_CONFIG2_MASK, (val))
I would put GENMASK() directly into FIELD(), but it's up to you (and I haven't checked the use of *_MASK anyway).
...
+static int ssd130x_write_data(struct ssd130x_device *ssd130x, u8 *values, int count) +{
- int ret;
- ret = regmap_bulk_write(ssd130x->regmap, SSD130X_DATA, values, count);
- if (ret)
return ret;- return 0;
return regmap_bulk_write(...);
+}
...
+/*
- Helper to write command (SSD130X_COMMAND). The fist variadic argument
- is the command to write and the following are the command options.
- Note that the ssd130x protocol requires each command and option to be
- written as a SSD130X_COMMAND device register value. That is why a call
- to regmap_write(..., SSD130X_COMMAND, ...) is done for each argument.
- */
Thanks!
+static int ssd130x_write_cmd(struct ssd130x_device *ssd130x, int count,
/* u8 cmd, u8 option, ... */...)+{
- va_list ap;
- u8 value;
- int ret;
- va_start(ap, count);
- do {
value = va_arg(ap, int);ret = regmap_write(ssd130x->regmap, SSD130X_COMMAND, (u8)value);
Wondering if you really need this casting. value is u8 by definition.
if (ret)goto out_end;- } while (--count);
+out_end:
- va_end(ap);
- return ret;
+}
...
- ssd130x = devm_drm_dev_alloc(dev, &ssd130x_drm_driver,
struct ssd130x_device, drm);- if (IS_ERR(ssd130x)) {
dev_err_probe(dev, PTR_ERR(ssd130x),"Failed to allocate DRM device\n");return ssd130x;
This...
- }
...
- bl = devm_backlight_device_register(dev, dev_name(dev), dev, ssd130x,
&ssd130xfb_bl_ops, NULL);- if (IS_ERR(bl))
return ERR_PTR(dev_err_probe(dev, PTR_ERR(bl),"Unable to register backlight device\n"));
Can be consistent with this then.