Amitkumar Karwar <amitkarwar@xxxxxxxxx> writes:
> From: Prameela Rani Garnepudi <prameela.j04cs@xxxxxxxxx>
>
> After successful loading of firmware, a CARD READY indication is
> received by host. Common device configuration parameters are sent
> to the device after this. It includes information like device
> operating mode (Wi-Fi alone or BT coex), power save related
> parameters, GPIO information etc. As device supports BT coex,
> this frame is send in COEX queue initially. Based on the operating
> mode, CARD READY indication is received from each protocol module
> in firmware i.e. WLAN, BT.
>
> Signed-off-by: Prameela Rani Garnepudi <prameela.j04cs@xxxxxxxxx>
> Signed-off-by: Amitkumar Karwar <amit.karwar@xxxxxxxxxxxxxxxxxx>
[...]
> +struct rsi_ulp_gpio_vals {
> +#ifdef __LITTLE_ENDIAN
> + u8 motion_sensor_gpio_ulp_wakeup:1;
> + u8 sleep_ind_from_device:1;
> + u8 ulp_gpio_2:1;
> + u8 push_button_ulp_wakeup:1;
> + u8 reserved:4;
> +#else
> + u8 reserved:4;
> + u8 push_button_ulp_wakeup:1;
> + u8 ulp_gpio_2:1;
> + u8 sleep_ind_from_device:1;
> + u8 motion_sensor_gpio_ulp_wakeup:1;
> +#endif
> +} __packed;
This is something I'm not exactly thrilled to see (bitfields are not
really liked in upstream community) but I'm not going to reject that
either as we have other offenders, rtl8xxxx being one. But it's much
better that to __le_to_cpu(), FIELD_GET() & co. That you don't need to
duplicate the fields and is less error prone.
> +struct rsi_config_vals {
> +#ifdef __LITTLE_ENDIAN
> + u16 len:12;
> + u16 q_no:4;
> +#else
> + u16 q_no:4;
> + u16 len:12;
> +#endif
> + u8 pkt_type;
> + u8 misc_flags;
> + __le16 reserved1[6];
> + u8 lp_ps_handshake;
> + u8 ulp_ps_handshake;
> + u8 sleep_config_params; /* 0 for no handshake,
> + * 1 for GPIO based handshake,
> + * 2 packet handshake
> + */
> + u8 unused_ulp_gpio;
> + u32 unused_soc_gpio_bitmap;
> + u8 ext_pa_or_bt_coex_en;
> + u8 opermode;
> + u8 wlan_rf_pwr_mode;
> + u8 bt_rf_pwr_mode;
> + u8 zigbee_rf_pwr_mode;
> + u8 driver_mode;
> + u8 region_code;
> + u8 antenna_sel_val;
> + u8 reserved2[16];
> +} __packed;
But here you are even mixing use of __LITTLE_ENDIAN and __le16, that's
just weird. And shouldn't unused_soc_gpio_bitmap be __le32?
--
Kalle Valo
