On Mon, Nov 1, 2021 at 6:57 AM Ricardo Martinez
<ricardo.martinez@xxxxxxxxxxxxxxx> wrote:
> Control Port implements driver control messages such as modem-host
> handshaking, controls port enumeration, and handles exception messages.
>
> The handshaking process between the driver and the modem happens during
> the init sequence. The process involves the exchange of a list of
> supported runtime features to make sure that modem and host are ready
> to provide proper feature lists including port enumeration. Further
> features can be enabled and controlled in this handshaking process.
[skipped]
> diff --git a/drivers/net/wwan/t7xx/t7xx_modem_ops.c b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
> ...
> +struct feature_query {
> + u32 head_pattern;
Looks like this field should be __le32 since it is sent to modem as is.
> + u8 feature_set[FEATURE_COUNT];
> + u32 tail_pattern;
Ditto.
> +};
> +
> +static void prepare_host_rt_data_query(struct core_sys_info *core)
> +{
> ...
> + ft_query->head_pattern = MD_FEATURE_QUERY_ID;
This should be a
ft_query->head_pattern = cpu_to_le32(MD_FEATURE_QUERY_ID);
to run on the big-endians CPU. strace will notify you about each
endians mismatch as soon as you change head_pattern field type to
__le32.
> + memcpy(ft_query->feature_set, core->feature_set, FEATURE_COUNT);
> + ft_query->tail_pattern = MD_FEATURE_QUERY_ID;
Ditto.
> + /* send HS1 message to device */
> + port_proxy_send_skb(core->ctl_port, skb, 0);
> +}
[skipped]
> diff --git a/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c b/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c
> ...
> +static void fsm_ee_message_handler(struct sk_buff *skb)
> +{
> ...
> + ctrl_msg_h = (struct ctrl_msg_header *)skb->data;
> ...
> + switch (ctrl_msg_h->ctrl_msg_id) {
This should be:
switch (le32_to_cpu(ctrl_msg_h->ctrl_msg_id)) {
> + case CTL_ID_MD_EX:
> + if (ctrl_msg_h->reserved != MD_EX_CHK_ID) {
Why is this field called 'reserved', but used to perform message validation?
> ...
> +static void control_msg_handler(struct t7xx_port *port, struct sk_buff *skb)
> +{
> + struct ctrl_msg_header *ctrl_msg_h;
> ...
> + ctrl_msg_h = (struct ctrl_msg_header *)skb->data;
> ..
> + switch (ctrl_msg_h->ctrl_msg_id) {
This should be something like this:
switch (le32_to_cpu(ctrl_msg_h->ctrl_msg_id)) {
--
Sergey
