| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8b1bd24c-f14c-0244-d2fb-69d4f02b46d5@intel.com>
Date: Fri, 6 Jan 2023 10:31:24 -0800
From: Jesse Brandeburg <jesse.brandeburg@...el.com>
To: <m.chetan.kumar@...ux.intel.com>, <netdev@...r.kernel.org>
CC: <kuba@...nel.org>, <davem@...emloft.net>,
<johannes@...solutions.net>, <ryazanov.s.a@...il.com>,
<loic.poulain@...aro.org>, <ilpo.jarvinen@...ux.intel.com>,
<ricardo.martinez@...ux.intel.com>,
<chiranjeevi.rapolu@...ux.intel.com>, <haijun.liu@...iatek.com>,
<edumazet@...gle.com>, <pabeni@...hat.com>, <linuxwwan@...el.com>,
<linuxwwan_5g@...el.com>, <chandrashekar.devegowda@...el.com>,
<matthias.bgg@...il.com>, <linux-arm-kernel@...ts.infradead.org>,
<linux-mediatek@...ts.infradead.org>,
Madhusmita Sahu <madhusmita.sahu@...el.com>
Subject: Re: [PATCH v3 net-next 2/5] net: wwan: t7xx: Infrastructure for early
port configuration
On 1/6/2023 8:26 AM, m.chetan.kumar@...ux.intel.com wrote:
> From: M Chetan Kumar <m.chetan.kumar@...ux.intel.com>
>
> To support cases such as FW update or Core dump, the t7xx
> device is capable of signaling the host that a special port
> needs to be created before the handshake phase.
>
> This patch adds the infrastructure required to create the
> early ports which also requires a different configuration of
> CLDMA queues.
nit: use imperative voice in your commit messages: no "this patch".
instead:
"Add the infrastructure..."
>
> Signed-off-by: Haijun Liu <haijun.liu@...iatek.com>
> Co-developed-by: Madhusmita Sahu <madhusmita.sahu@...el.com>
> Signed-off-by: Madhusmita Sahu <madhusmita.sahu@...el.com>
> Signed-off-by: Ricardo Martinez <ricardo.martinez@...ux.intel.com>
> Signed-off-by: Devegowda Chandrashekar <chandrashekar.devegowda@...el.com>
> Signed-off-by: M Chetan Kumar <m.chetan.kumar@...ux.intel.com>
> --
> v3:
> * No Change.
> v2:
> * Move recv_skb handler to cldma_queue.
> * Drop cldma_queue_type.
> * Restore prototype of t7xx_port_send_raw_skb().
> * Remove PORT_CFG_ID_INVALID check in t7xx_port_proxy_set_cfg().
> * Add space before */.
> * Drop unnecessary logs.
> * Use WARN_ON on early port.
> * Use new MISC_DEV_STATUS_INVALID instead of MISC_DEV_STATUS.
> * Use macros instead of const identifiers.
> * Change ports member type from pointer to array type.
> * Prefix LK_EVENT_XX with MISC prefix.
> * Use t7xx prefix for device_stage enums.
> * Correct log messages.
> * Don’t override pkt_size for non-download port under dedicated Queue.
> * Drop cldma_txq_rxq_ids.
> * Use macro for txq/rxq index.
> * Use warn_on for rxq_idx comparison.
> * Drop t7xx_port_proxy_get_port_by_name().
> * Replace fsm poll with read_poll_timeout().
> * Use "\n" consistently across log message.
> * Remove local var _dev prefixes in fsm_routine_start().
> * Use max_t.
...
> diff --git a/drivers/net/wwan/t7xx/t7xx_reg.h b/drivers/net/wwan/t7xx/t7xx_reg.h
> index c41d7d094c08..44352cd02460 100644
> --- a/drivers/net/wwan/t7xx/t7xx_reg.h
> +++ b/drivers/net/wwan/t7xx/t7xx_reg.h
> @@ -102,10 +102,28 @@ enum t7xx_pm_resume_state {
> };
>
> #define T7XX_PCIE_MISC_DEV_STATUS 0x0d1c
> -#define MISC_STAGE_MASK GENMASK(2, 0)
> -#define MISC_RESET_TYPE_PLDR BIT(26)
> #define MISC_RESET_TYPE_FLDR BIT(27)
> -#define LINUX_STAGE 4
> +#define MISC_RESET_TYPE_PLDR BIT(26)
> +#define MISC_DEV_STATUS_MASK GENMASK(15, 0)
> +#define MISC_DEV_STATUS_INVALID GENMASK(15, 0)
I don't see any uses of this, even though it's mentioned in the commit
message. The only reason I looked was because it was weird to have
DEV_STATUS_MASK and STATUS_INVALID be the same values, is that correct?
> +#define MISC_LK_EVENT_MASK GENMASK(11, 8)
> +
> +enum lk_event_id {
> + LK_EVENT_NORMAL = 0,
> + LK_EVENT_CREATE_PD_PORT = 1,
> + LK_EVENT_CREATE_POST_DL_PORT = 2,
> + LK_EVENT_RESET = 7,
> +};
> +
> +#define MISC_STAGE_MASK GENMASK(2, 0)
> +
> +enum t7xx_device_stage {
> + T7XX_DEV_STAGE_INIT = 0,
> + T7XX_DEV_STAGE_BROM_PRE = 1,
> + T7XX_DEV_STAGE_BROM_POST = 2,
> + T7XX_DEV_STAGE_LK = 3,
> + T7XX_DEV_STAGE_LINUX = 4,
> +};
>
> #define T7XX_PCIE_RESOURCE_STATUS 0x0d28
> #define T7XX_PCIE_RESOURCE_STS_MSK GENMASK(4, 0)
> diff --git a/drivers/net/wwan/t7xx/t7xx_state_monitor.c b/drivers/net/wwan/t7xx/t7xx_state_monitor.c
> index 80edb8e75a6a..76fb5d57d4d7 100644
> --- a/drivers/net/wwan/t7xx/t7xx_state_monitor.c
> +++ b/drivers/net/wwan/t7xx/t7xx_state_monitor.c
> @@ -206,6 +206,34 @@ static void fsm_routine_exception(struct t7xx_fsm_ctl *ctl, struct t7xx_fsm_comm
> fsm_finish_command(ctl, cmd, 0);
> }
>
> +static void t7xx_lk_stage_event_handling(struct t7xx_fsm_ctl *ctl, unsigned int status)
> +{
> + struct t7xx_modem *md = ctl->md;
> + struct cldma_ctrl *md_ctrl;
> + enum lk_event_id lk_event;
> + struct device *dev;
> +
> + dev = &md->t7xx_dev->pdev->dev;
> + lk_event = FIELD_GET(MISC_LK_EVENT_MASK, status);
> + switch (lk_event) {
> + case LK_EVENT_NORMAL:
> + case LK_EVENT_RESET:
> + break;
> +
> + case LK_EVENT_CREATE_PD_PORT:
> + md_ctrl = md->md_ctrl[CLDMA_ID_AP];
> + t7xx_cldma_hif_hw_init(md_ctrl);
> + t7xx_cldma_stop(md_ctrl);
> + t7xx_cldma_switch_cfg(md_ctrl, CLDMA_DEDICATED_Q_CFG);
> + t7xx_cldma_start(md_ctrl);
> + break;
> +
> + default:
> + dev_err(dev, "Invalid LK event %d\n", lk_event);
> + break;
> + }
> +}
> +
> static int fsm_stopped_handler(struct t7xx_fsm_ctl *ctl)
> {
> ctl->curr_state = FSM_STATE_STOPPED;
> @@ -317,8 +345,9 @@ static int fsm_routine_starting(struct t7xx_fsm_ctl *ctl)
> static void fsm_routine_start(struct t7xx_fsm_ctl *ctl, struct t7xx_fsm_command *cmd)
> {
> struct t7xx_modem *md = ctl->md;
> - u32 dev_status;
> - int ret;
> + struct device *dev;
> + u32 status, stage;
> + int ret = 0;
>
> if (!md)
> return;
> @@ -329,23 +358,55 @@ static void fsm_routine_start(struct t7xx_fsm_ctl *ctl, struct t7xx_fsm_command
> return;
> }
>
> + dev = &md->t7xx_dev->pdev->dev;
> ctl->curr_state = FSM_STATE_PRE_START;
> t7xx_md_event_notify(md, FSM_PRE_START);
>
> - ret = read_poll_timeout(ioread32, dev_status,
> - (dev_status & MISC_STAGE_MASK) == LINUX_STAGE, 20000, 2000000,
> - false, IREG_BASE(md->t7xx_dev) + T7XX_PCIE_MISC_DEV_STATUS);
> + ret = read_poll_timeout(ioread32, status,
> + ((status & MISC_STAGE_MASK) == T7XX_DEV_STAGE_LINUX) ||
> + ((status & MISC_STAGE_MASK) == T7XX_DEV_STAGE_LK), 100000,
> + 20000000, false, IREG_BASE(md->t7xx_dev) +
> + T7XX_PCIE_MISC_DEV_STATUS);
> +
> if (ret) {
> - struct device *dev = &md->t7xx_dev->pdev->dev;
> + ret = -ETIMEDOUT;
> + dev_err(dev, "read poll %d\n", ret);
> + goto finish_command;
> + }
>
> - fsm_finish_command(ctl, cmd, -ETIMEDOUT);
> - dev_err(dev, "Invalid device status 0x%lx\n", dev_status & MISC_STAGE_MASK);
> - return;
> + if (status != ctl->prev_status) {
> + stage = FIELD_GET(MISC_STAGE_MASK, status);
if stage is only used down here you can declare it locally. cppcheck has
a check that will find these for you.
> + switch (stage) {
> + case T7XX_DEV_STAGE_INIT:
> + case T7XX_DEV_STAGE_BROM_PRE:
> + case T7XX_DEV_STAGE_BROM_POST:
> + dev_info(dev, "BROM_STAGE Entered\n");
> + ret = t7xx_fsm_append_cmd(ctl, FSM_CMD_START, 0);
> + break;
> +
> + case T7XX_DEV_STAGE_LK:
> + dev_info(dev, "LK_STAGE Entered\n");
> + t7xx_lk_stage_event_handling(ctl, status);
> + break;
> +
> + case T7XX_DEV_STAGE_LINUX:
> + dev_info(dev, "LINUX_STAGE Entered\n");
> + t7xx_cldma_hif_hw_init(md->md_ctrl[CLDMA_ID_AP]);
> + t7xx_cldma_hif_hw_init(md->md_ctrl[CLDMA_ID_MD]);
> + t7xx_mhccif_mask_clr(md->t7xx_dev, D2H_INT_PORT_ENUM |
> + D2H_INT_ASYNC_MD_HK | D2H_INT_ASYNC_AP_HK);
> + t7xx_port_proxy_set_cfg(md, PORT_CFG_ID_NORMAL);
> + ret = fsm_routine_starting(ctl);
> + break;
> +
> + default:
> + break;
> + }
> + ctl->prev_status = status;
> }
>
> - t7xx_cldma_hif_hw_init(md->md_ctrl[CLDMA_ID_AP]);
> - t7xx_cldma_hif_hw_init(md->md_ctrl[CLDMA_ID_MD]);
> - fsm_finish_command(ctl, cmd, fsm_routine_starting(ctl));
> +finish_command:
> + fsm_finish_command(ctl, cmd, ret);
> }
>
> static int fsm_main_thread(void *data)
> @@ -516,6 +577,7 @@ void t7xx_fsm_reset(struct t7xx_modem *md)
> fsm_flush_event_cmd_qs(ctl);
> ctl->curr_state = FSM_STATE_STOPPED;
> ctl->exp_flg = false;
> + ctl->prev_status = 0;
> }
>
> int t7xx_fsm_init(struct t7xx_modem *md)
> diff --git a/drivers/net/wwan/t7xx/t7xx_state_monitor.h b/drivers/net/wwan/t7xx/t7xx_state_monitor.h
> index b6e76f3903c8..5e8012567ba1 100644
> --- a/drivers/net/wwan/t7xx/t7xx_state_monitor.h
> +++ b/drivers/net/wwan/t7xx/t7xx_state_monitor.h
> @@ -96,6 +96,7 @@ struct t7xx_fsm_ctl {
> bool exp_flg;
> spinlock_t notifier_lock; /* Protects notifier list */
> struct list_head notifier_list;
> + u32 prev_status;
> };
>
> struct t7xx_fsm_event {
Powered by blists - more mailing lists