On 9/18/2023 10:14 PM, Alan Stern wrote:
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.
it is a lot of changes. i only expect common and minimal
change(macros/helpers) in gadget.h.
if you have better idea, please help share a patch.
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.
thanks everyone for reviewing for a long time, it is end from my side of
this topic.
Alan Stern
