[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHNKnsTZhtmt6DNsEb+EvnFBo9mNPd4pPG=qjvkxyD3pH+KqrA@mail.gmail.com>
Date: Sat, 6 Nov 2021 21:07:03 +0300
From: Sergey Ryazanov <ryazanov.s.a@...il.com>
To: Ricardo Martinez <ricardo.martinez@...ux.intel.com>
Cc: netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
Jakub Kicinski <kuba@...nel.org>,
David Miller <davem@...emloft.net>,
Johannes Berg <johannes@...solutions.net>,
Loic Poulain <loic.poulain@...aro.org>,
M Chetan Kumar <m.chetan.kumar@...el.com>,
chandrashekar.devegowda@...el.com,
Intel Corporation <linuxwwan@...el.com>,
chiranjeevi.rapolu@...ux.intel.com, haijun.liu@...iatek.com,
amir.hanania@...el.com,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
dinesh.sharma@...el.com, eliot.lee@...el.com,
mika.westerberg@...ux.intel.com, moises.veleta@...el.com,
pierre-louis.bossart@...el.com, muralidharan.sethuraman@...el.com,
Soumya.Prakash.Mishra@...el.com, sreehari.kancharla@...el.com,
suresh.nagaraj@...el.com
Subject: Re: [PATCH v2 05/14] net: wwan: t7xx: Add control port
On Mon, Nov 1, 2021 at 6:57 AM Ricardo Martinez
<ricardo.martinez@...ux.intel.com> 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
Powered by blists - more mailing lists