Hi,
On 5/13/24 2:46 PM, Andy Shevchenko wrote:
> On Mon, May 13, 2024 at 01:15:51PM +0200, Hans de Goede wrote:
>> Dell All In One (AIO) models released after 2017 use a backlight controller
>> board connected to an UART.
>>
>> Add a small emulator to allow development and testing of
>> the drivers/platform/x86/dell/dell-uart-backlight.c driver for
>> this board, without requiring access to an actual Dell All In One.
>
> ...
>
>> + if (argc != 2) {
>> + fprintf(stderr, "Invalid or missing arguments\n");
>> + fprintf(stderr, "Usage: %s <serial-port>\n", argv[0]);
>> + return 1;
>> + }
>> +
>> + serial_fd = open(argv[1], O_RDWR | O_NOCTTY);
>> + if (serial_fd == -1) {
>> + fprintf(stderr, "Error opening %s: %s\n", argv[1], strerror(errno));
>> + return 1;
>
> So, looking at the `man error` it works like your custom approach with an
> additional things.
But that is a GNU / glibc extension. I would prefer to stick with plain libc
here, so that this can be build with other libc-s too.
> Also, don't you want to use either different error codes (above is +1 and all
> below seems using -1), or be consistent and return -1 always?
the +1 code is for when the cmdline argument is likely wrong , -1 / 255 is
for IO errors.
>
>> + }
>
> ...
>
>> + /* Respond with <total-len> <cmd> <data...> <csum> */
>> + response[0] = len + 3; /* response length in bytes */
>> + response[1] = buf[1]; /* ack cmd */
>> + csum = dell_uart_checksum(response, len + 2);
>> + response[len + 2] = csum;
>
>> + ret = write(serial_fd, response, len + 3);
>
> response[0] can be reused here.
>
>> + if (ret != (len + 3))
>
> And here.
Ack will fix.
Regards,
Hans
>
>> + fprintf(stderr, "Error writing %d bytes: %d\n",
>> + len + 3, ret);
>> + }
>
