Prameela Rani Garnepudi <prameela.j04cs@xxxxxxxxx> writes:
> * Switch clock info is divided in to different clock information fields for
> readability and synchronization with firmware code.
> * Other parameters are added for future use and to make the frame size in sync
> with latest firmware. Otherwise firmware will discard the frame considering
> corrupted frame.
>
> Signed-off-by: Prameela Rani Garnepudi <prameela.j04cs@xxxxxxxxx>
Thanks, this is much easier to review now that the style changes are gone.
> /* structure to store configs related to UMAC clk programming */
> struct switch_clk {
> - __le16 switch_clk_info;
> + __le16 switch_umac_clk : 1; /* If set rest is valid */
> + __le16 switch_qspi_clk : 1; /* If set qspi clk will be changed */
> + __le16 switch_slp_clk_2_32 : 1;
> + __le16 switch_bbp_lmac_clk_reg : 1;
> + __le16 switch_mem_ctrl_cfg : 1;
> + __le16 reserved : 11;
This immediately caugh my eye. Are you sure this works on big endian
machines? I have never seen __le16 mixed with bitfields so I'm skeptical
that this will even work, but I don't have time to really check. Anyone
else know?
Anyway, the original code used the preferred format:
- .switch_clk_info = cpu_to_le16(BIT(3)),
That is use BIT() or GENMASK()/FIELD_PREP() to create the bitmask in cpu
native format and then convert it with cpu_to_le*() family of functions.
--
Kalle Valo
