[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20210625050649.GB4974@workstation>
Date: Fri, 25 Jun 2021 10:36:49 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Bhaumik Bhatt <bbhatt@...eaurora.org>
Cc: linux-arm-msm@...r.kernel.org, hemantk@...eaurora.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] bus: mhi: core: Add support for processing priority
of event ring
On Thu, Jun 24, 2021 at 08:06:19PM -0700, Bhaumik Bhatt wrote:
> From: Hemant Kumar <hemantk@...eaurora.org>
>
> Event ring priorities are currently set to 1 and are unused.
> Default processing priority for event rings is set to regular
> tasklet. Controllers can choose to use high priority tasklet
> scheduling for certain event rings critical for processing such
> as ones transporting control information if they wish to avoid
> system scheduling delays for those packets. In order to support
> these use cases, allow controllers to set event ring priority to
> high. This patch only adds support and does not enable usage of
> these priorities.
>
> Signed-off-by: Hemant Kumar <hemantk@...eaurora.org>
> Signed-off-by: Bhaumik Bhatt <bbhatt@...eaurora.org>
Please add the controller driver changes in this patch itself.
Couple of nits below...
Thanks,
Mani
> ---
> v4:
> -Update fixed priority for all events to default to fix bug in v3
> -Supply changelog
>
> v3:
> -Revert to enum approach
> -Use 0 as default and 1 as high in enum
> -Do not use config values for event rings
>
> v2:
> -Use boolean approach for easy maintenance as controllers do not need updates
>
> drivers/bus/mhi/core/init.c | 4 ++--
> drivers/bus/mhi/core/internal.h | 2 +-
> drivers/bus/mhi/core/main.c | 19 ++++++++++++++++---
> include/linux/mhi.h | 14 ++++++++++++--
> 4 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index c81b377..23386b8 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -673,8 +673,8 @@ static int parse_ev_cfg(struct mhi_controller *mhi_cntrl,
> &mhi_cntrl->mhi_chan[mhi_event->chan];
> }
>
> - /* Priority is fixed to 1 for now */
> - mhi_event->priority = 1;
> + /* Priority is fixed to deafult for now */
default
> + mhi_event->priority = MHI_ER_PRIORITY_DEFAULT;
>
> mhi_event->db_cfg.brstmode = event_cfg->mode;
> if (MHI_INVALID_BRSTMODE(mhi_event->db_cfg.brstmode))
> diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
> index 672052f..666e102 100644
> --- a/drivers/bus/mhi/core/internal.h
> +++ b/drivers/bus/mhi/core/internal.h
> @@ -535,7 +535,7 @@ struct mhi_event {
> u32 intmod;
> u32 irq;
> int chan; /* this event ring is dedicated to a channel (optional) */
> - u32 priority;
> + enum mhi_er_priority priority;
> enum mhi_er_data_type data_type;
> struct mhi_ring ring;
> struct db_cfg db_cfg;
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 8ac73f9..bfc9776 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -425,10 +425,11 @@ void mhi_create_devices(struct mhi_controller *mhi_cntrl)
> }
> }
>
> -irqreturn_t mhi_irq_handler(int irq_number, void *dev)
> +irqreturn_t mhi_irq_handler(int irq_number, void *priv)
> {
> - struct mhi_event *mhi_event = dev;
> + struct mhi_event *mhi_event = priv;
> struct mhi_controller *mhi_cntrl = mhi_event->mhi_cntrl;
> + struct device *dev = &mhi_cntrl->mhi_dev->dev;
> struct mhi_event_ctxt *er_ctxt =
> &mhi_cntrl->mhi_ctxt->er_ctxt[mhi_event->er_index];
> struct mhi_ring *ev_ring = &mhi_event->ring;
> @@ -454,8 +455,20 @@ irqreturn_t mhi_irq_handler(int irq_number, void *dev)
>
> if (mhi_dev)
> mhi_notify(mhi_dev, MHI_CB_PENDING_DATA);
> - } else {
> +
> + return IRQ_HANDLED;
> + }
> +
> + switch (mhi_event->priority) {
> + case MHI_ER_PRIORITY_HI:
> + tasklet_hi_schedule(&mhi_event->task);
> + break;
> + case MHI_ER_PRIORITY_DEFAULT:
> tasklet_schedule(&mhi_event->task);
> + break;
> + default:
> + dev_err(dev, "Skip event of unknown priority\n");
> + break;
> }
>
> return IRQ_HANDLED;
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index 86cea52..62ddead 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -198,6 +198,16 @@ enum mhi_er_data_type {
> };
>
> /**
> + * enum mhi_er_priority - Event ring processing priority
> + * @MHI_ER_PRIORITY_DEFAULT: processed by regular tasklet
> + * @MHI_ER_PRIORITY_HI: processed by hi-priority tasklet
s/hi/high
> + */
> +enum mhi_er_priority {
> + MHI_ER_PRIORITY_DEFAULT,
> + MHI_ER_PRIORITY_HI,
> +};
> +
> +/**
> * enum mhi_db_brst_mode - Doorbell mode
> * @MHI_DB_BRST_DISABLE: Burst mode disable
> * @MHI_DB_BRST_ENABLE: Burst mode enable
> @@ -250,7 +260,7 @@ struct mhi_channel_config {
> * @irq_moderation_ms: Delay irq for additional events to be aggregated
> * @irq: IRQ associated with this ring
> * @channel: Dedicated channel number. U32_MAX indicates a non-dedicated ring
> - * @priority: Priority of this ring. Use 1 for now
> + * @priority: Processing priority of this ring.
> * @mode: Doorbell mode
> * @data_type: Type of data this ring will process
> * @hardware_event: This ring is associated with hardware channels
> @@ -262,7 +272,7 @@ struct mhi_event_config {
> u32 irq_moderation_ms;
> u32 irq;
> u32 channel;
> - u32 priority;
> + enum mhi_er_priority priority;
> enum mhi_db_brst_mode mode;
> enum mhi_er_data_type data_type;
> bool hardware_event;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Powered by blists - more mailing lists