On 06/08/2025 14:48, Iker Pedrosa wrote: > This adds a functional DRM driver for ST7920 that communicates with the > display via the SPI bus.
Please do not use "This commit/patch/change", but imperative mood. See longer explanation here: https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > > Signed-off-by: Iker Pedrosa <ikerpedro...@gmail.com> > --- ... > +#define BOTTOM_HORIZONTAL_ADDRESS 0x80 > + > +#define CMD_SIZE 35 > + > +const struct st7920_deviceinfo st7920_variants[] = { > + [ST7920_ID] = { > + .default_width = 128, > + .default_height = 64, > + .family_id = ST7920_FAMILY, Don't add dead code. This cannot be anything else than ST7920_FAMILY. Several places here can be simplified (and "possible" future code is not an argument here - this patch must be correct, simple and stand on its own because we do not write code just in case). ... > +static int st7920_probe(struct spi_device *spi) > +{ > + struct st7920_device *st7920; > + struct regmap *regmap; > + struct device *dev = &spi->dev; > + struct drm_device *drm; > + int ret; > + > + regmap = devm_regmap_init_spi(spi, &st7920_spi_regmap_config); > + if (IS_ERR(regmap)) > + return PTR_ERR(regmap); > + > + st7920 = devm_drm_dev_alloc(dev, &st7920_drm_driver, > + struct st7920_device, drm); > + if (IS_ERR(st7920)) > + return dev_err_probe(dev, PTR_ERR(st7920), > + "Failed to allocate > DRM device\n"); Misaligned but also this looks like ENOMEM error and such should never have dev_err. Best regards, Krzysztof
