Re: [PATCH 2/8] usb: gadget: add anonymous definition in some struct for trace purpose

[Linyu Yuan <quic_linyyuan@quicinc.com>]

On 9/15/2023 9:51 AM, Alan Stern wrote:
> On Fri, Sep 15, 2023 at 09:02:48AM +0800, Linyu Yuan wrote:
>> On 9/14/2023 10:54 PM, Alan Stern wrote:
>>> You didn't include the version number in the Subject: line.  Undoubtedly
>>> Greg's automatic error checker will warn you about this.  Unless the
>>> version number is clearly marked for each patch, it's difficult for his
>>> programs to tell which email message contains the most recent version.
>>>
>>> On Thu, Sep 14, 2023 at 06:02:56PM +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 is wast trace
>>>> buffer.
>>>>
>>>> Add anonymous union which have one u32 member can be used by trace event
>>>> during fast assign stage to save more entries with same trace ring buffer
>>>> size.
>>>>
>>>> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
>>>> ---
>>> And you didn't include the version change information here, below the
>>> "---" line.
>>>
>>> Apart from that, this is a _lot_ better than before!  I don't know if
>>> Greg will think this change is worth merging, but at least now it's
>>> possible to read the code and understand what's going on.
>>
>> according Steven's comment, maybe will always save data in little endian at
>> trace event
>>
>> fast assign stage.
>>
>> it will add definition of bit field back.
> Yes, that would be even better because you wouldn't have to change the
> definition of struct usb_gadget or struct usb_endpoint at all.  The fast
> assign stage can simply do:
>
> 	__entry->dw1 = (g->sg_supported << 0) |
> 			(g->is_otg << 1) |
> 			...
>
> and then you can easily access the individual bits in __entry.  It
> wouldn't be as fast but it would still save a lot of space.


how about __entry->dw1 = cpu_to_le32(g->dw1) ?




>
> Alan Stern