On Fri, Feb 11, 2022 at 01:05:57PM +0100, Javier Martinez Canillas wrote:

On 2/11/22 12:33, Andy Shevchenko wrote:

quoted

On Fri, Feb 11, 2022 at 10:19:24AM +0100, Javier Martinez Canillas wrote:

...

quoted

quoted

+ * Helper to write command (SSD130X_COMMAND). The fist variadic argument
+ * is the command to write and the following are the command options.

This is not correct explanation. Please, rephrase to show that _each_ of the
options is sent with a preceding command.

It's a correct explanation IMO from the caller point of view. The first argument
is the command sent (i.e: SSD130X_SET_ADDRESS_MODE) and the next ones are the
the command options (i.e: SSD130X_SET_ADDRESS_MODE_HORIZONTAL).

The fact that each command and options are preceding with a SSD130X_COMMAND
value is part of the protocol of the device and a detail that's abstracted
away by this helper function to the callers.

My previous suggestion about bulk transaction was purely based on this
(misinterpreted) description. Can we make sure somehow that another reader
don't trap into the same?

...

quoted

quoted

+	bl = devm_backlight_device_register(dev, dev_name(dev), dev, ssd130x,
+					    &ssd130xfb_bl_ops, NULL);
+	if (IS_ERR(bl)) {

quoted

+		ret = PTR_ERR(bl);
+		dev_err_probe(dev, ret, "Unable to register backlight device\n");
+		return ERR_PTR(ret);

		dev_err_probe(dev, PTR_ERR(bl), "Unable to register backlight device\n");
		return bl;

?

No, because this function's return value is a struct ssd130x_device pointer,
not a struct backlight_device pointer.

	return ERR_CAST(bl);

-- 
With Best Regards,
Andy Shevchenko