On Mon, Sep 18, 2023 at 02:06:34PM +0200, Greg Kroah-Hartman wrote: > On Mon, Sep 18, 2023 at 07:25:32PM +0800, Linyu Yuan wrote: > > Some UDC trace event will save usb udc information, but it use one int > > size buffer to save one bit information of usb udc, it waste trace buffer. > > > > Add anonymous union which have u32 members can be used by trace event > > during fast assign stage to save more entries with same trace ring buffer > > size. > > > > In order to access each bit with BIT() macro, add different definition for > > each bit fields according host little/big endian to make sure it has same > > eacho bit field have same bit position in memory. > > typo? > > > Add some macros or helper for later trace event usage which follow the > > udc structs, As when possible future changes to udc related structs, > > developers will easy notice them. > > This isn't going to work at all, there's nothing to keep the two in > sync. > > As you are using bitmasks now, wonderful, just use those only and ignore > the bitfield definitions, that's not going to work mixing the two at > all. Greg is right. If you really want to access these fields using bitmask operations, you should do it by changing all of the fields into bitmasks; mixing bitmasks and bitfields is not a good idea. For example, in order to do this you will have to change the definition of struct usb_gadget. Replace unsigned sg_supported:1; unsigned is_otg:1; unsigned is_a_peripheral:1; unsigned b_hnp_enable:1; unsigned a_hnp_support:1; unsigned a_alt_hnp_support:1; unsigned hnp_polling_support:1; unsigned host_request_flag:1; unsigned quirk_ep_out_aligned_size:1; unsigned quirk_altset_not_supp:1; unsigned quirk_stall_not_supp:1; unsigned quirk_zlp_not_supp:1; unsigned quirk_avoids_skb_reserve:1; unsigned is_selfpowered:1; unsigned deactivated:1; unsigned connected:1; unsigned lpm_capable:1; unsigned wakeup_capable:1; unsigned wakeup_armed:1; with unsigned int dw1; #define USB_GADGET_SG_SUPPORTED BIT(0) #define USB_GADGET_IS_OTG BIT(1) ... #define USB_GADGET_WAKEUP_ARMED BIT(18) Then change all the drivers that use these fields. For example, change g->connected = 1; to g->dw1 |= USB_GADGET_CONNECTED; and change if (g->is_selfpowered) to if (g->dw1 & USB_GADGET_IS_SELFPOWERED) Or if you prefer, invent some inline routines or macros that will handle this for you. Make these changes in every gadget and UDC driver. Then it will be safe to say __entry->dw1 = g->dw1; in the fast assign stage. But it's not safe to use bitfields in the gadget and UDC drivers while using bitmasks in the tracing code. Alan Stern
