RE: [PATCH v2 1/4] usb: cdns2: Device side header file for CDNS2 driver

From: Pawel Laszczak
Date: Thu Apr 20 2023 - 05:51:45 EST


>
>On Thu, Apr 20, 2023, at 11:00, Pawel Laszczak wrote:
>> Patch defines macros, registers and structures used by Device side
>> driver.
>>
>> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx>
>
>> +struct cdns2_ep0_regs {
>> + __u8 rxbc;
>> + __u8 txbc;
>> + __u8 cs;
>> + __u8 reserved1[4];
>> + __u8 fifo;
>> + __le32 reserved2[94];
>> + __u8 setupdat[8];
>> + __u8 reserved4[88];
>> + __u8 maxpack;
>> +} __packed;
>
>> +struct cdns2_epx_base {
>> + __le16 rxbc;
>> + __u8 rxcon;
>> + __u8 rxcs;
>> + __le16 txbc;
>> + __u8 txcon;
>> + __u8 txcs;
>> +} __packed;
>
>> +struct cdns2_epx_regs {
>> + __le32 reserved[2];
>> + struct cdns2_epx_base ep[15];
>> + __u8 reserved2[290];
>> + __u8 endprst;
>> + __u8 reserved3[41];
>> + __le16 isoautoarm;
>> + __u8 reserved4[10];
>> + __le16 isodctrl;
>> + __le16 reserved5;
>> + __le16 isoautodump;
>> + __le32 reserved6;
>> + __le16 rxmaxpack[15];
>> + __le32 reserved7[65];
>> + __le32 rxstaddr[15];
>> + __u8 reserved8[4];
>> + __le32 txstaddr[15];
>> + __u8 reserved9[98];
>> + __le16 txmaxpack[15];
>> +} __packed;
>
>Register structures should generally not be __packed, otherwise any multi-
>byte registers will be accessed as individual bytes on architectures that have
>no unaligned load/store.
>
>If you are worried about struct packing on OABI arm, you can mark it as both
>__packed and __aligned(4). Most drivers just avoid the problem by not
>defining these as structures but instead use macros with register offsets.
>

Extra __alligned(4) is ok for me.
>From performance point of view the accessed as individual byte is not a problem.
All these registers are used mainly during initialization and enumeration.

While transfer, when the performance is important driver use DMA registers which are
32 bits and are aligned.

Thanks,
Pawel