Re: [PATCH v5 3/6] drm: Add driver for Solomon SSD130x OLED displays
flat view
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2022-02-11 16:14:20
Also in:
dri-devel, linux-pwm, lkml
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.
...
quoted hunk ↗ jump to hunk
+#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). ...
quoted hunk ↗ jump to hunk
+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(...);
+}
...
quoted hunk ↗ jump to hunk
+/* + * 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!
quoted hunk ↗ jump to hunk
+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.
quoted hunk ↗ jump to hunk
+ if (ret) + goto out_end; + } while (--count); + +out_end: + va_end(ap); + + return ret; +}
...
quoted hunk ↗ jump to hunk
+ ssd130x = devm_drm_dev_alloc(dev, &ssd130x_drm_driver, + struct ssd130x_device, drm); + if (IS_ERR(ssd130x)) {
quoted hunk ↗ jump to hunk
+ dev_err_probe(dev, PTR_ERR(ssd130x), + "Failed to allocate DRM device\n"); + return ssd130x;
This...
+ }
...
quoted hunk ↗ jump to hunk
+ 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. -- With Best Regards, Andy Shevchenko