Re: [PATCH 1/3] usb: misc: Add Intel USBIO bridge driver

Hans de Goede
2025-08-09T15:05:41Z

Hi Greg,

On 9-Aug-25 4:28 PM, Greg Kroah-Hartman wrote:
> On Sat, Aug 09, 2025 at 12:23:24PM +0200, Hans de Goede wrote:
>> +struct usbio_protver {
>> + uint8_t ver;
>
> Nit, but you do this everywhere. Kernel types are "u8", not "uint8_t",
> that's a userspace C type. Please use the correct ones when writing
> kernel code.

Ack and also ack for the mission endianness on
the bigger word sizes.

I'll fix this all for the next version.

>> +} __packed;
>> +
>> +struct usbio_fwver {
>> + uint8_t major;
>> + uint8_t minor;
>> + uint16_t patch;
>> + uint16_t build;
>
> What is the endian of these u16 variables?
>
>> +/* USBIO Packet Header */
>> +struct usbio_packet_header {
>> + uint8_t type;
>> + uint8_t cmd;
>> + uint8_t flags;
>
> Are these crossing the user/kernel boundry? I think so (same with
> above), and so shouldn't they use the proper types (__u8)?

No these only cross the kernel <-> hw boundary. This is not
uapi. I'll switch all these (everywhere in this driver) to plain
u8 / __le16 / __le32 for the next version and use
le16_to_cpu, etc. to access the bigger word sizes.

Regards,

Hans