Re: [PATCH v2 1/2] backlight: Add Congatec Board Controller (CGBC) backlight support

[Petri Karhula <petri.karhula@novatron.fi>]

Hello Thomas and Krzysztof, 

Thanks for the comments. I have fixed most of them, but I disagree
about removing the error messages. I think that those would be needed.
Without those the kernel does not seem to print any errors to the logs
if the operation fails. It would be then the responsible of the calling
user space program to print the error to the log. But that also could
be missing due to the bug or bad design. This is targeted to be running
in the embedded device that is possibly used in places without internet
coverage. If the user is in such case calling to the support and
complains that backlight control is not working, the only way to get
further information are the logs that the device will send to the cloud
when it happens to be in the coverage of the network. When the log is
the only source of information, it would be really nice to be able to
read clear error messages if the oration has been failed in the driver.
> 

> +	bl_data->current_brightness = reply_buf[0] &
BLT_PWM_DUTY_MASK;
> +
> +	/* Verify the setting was applied correctly */
> +	if (bl_data->current_brightness != brightness) {
> +		dev_err(bl_data->dev,
> +			"Brightness setting verification failed\n");
> +		return -EIO;
> +	}
> 
> Do we really need to check the brightness returned by the board
> controller? Have you ever run into a situation where cbgc_command
> completed without errors, but the brightness level didn’t match what
> you
> expected? Maybe we could assume that if the cbgc_command returned
> successfully the brightness value is correct?
> 
> I'm not against checking the backlight value. I looked at Congatec's
> implementation and they also check it.
> 

You are absolutely right. It is extremely unlikely that the brightness
level wouldn't match to what is expected. A bug in the Congatec's
firmware could be one possibility. But then that extremely unlikely
situation will happen, I definitely would like to see it in the log.


Best Regards, 
Petri