Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20211101035635.26999-1-ricardo.martinez@linux.intel.com> <20211101035635.26999-3-ricardo.martinez@linux.intel.com>
In-Reply-To: <20211101035635.26999-3-ricardo.martinez@linux.intel.com>
From:   Sergey Ryazanov <ryazanov.s.a@gmail.com>
Date:   Sat, 6 Nov 2021 21:01:30 +0300
Message-ID: <CAHNKnsSp5RXkX8v=uE5ZX1evLfTSMGSf9iAUhgpLzdvKiahNzg@mail.gmail.com>
Subject: Re: [PATCH v2 02/14] net: wwan: t7xx: Add control DMA interface
To:     Ricardo Martinez <ricardo.martinez@linux.intel.com>
Cc:     netdev@vger.kernel.org, linux-wireless@vger.kernel.org,
        Jakub Kicinski <kuba@kernel.org>,
        David Miller <davem@davemloft.net>,
        Johannes Berg <johannes@sipsolutions.net>,
        Loic Poulain <loic.poulain@linaro.org>,
        M Chetan Kumar <m.chetan.kumar@intel.com>,
        chandrashekar.devegowda@intel.com,
        Intel Corporation <linuxwwan@intel.com>,
        chiranjeevi.rapolu@linux.intel.com, haijun.liu@mediatek.com,
        amir.hanania@intel.com,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        dinesh.sharma@intel.com, eliot.lee@intel.com,
        mika.westerberg@linux.intel.com, moises.veleta@intel.com,
        pierre-louis.bossart@intel.com, muralidharan.sethuraman@intel.com,
        Soumya.Prakash.Mishra@intel.com, sreehari.kancharla@intel.com,
        suresh.nagaraj@intel.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Nov 1, 2021 at 6:57 AM Ricardo Martinez
<ricardo.martinez@linux.intel.com> wrote:
> Cross Layer DMA (CLDMA) Hardware interface (HIF) enables the control
> path of Host-Modem data transfers. CLDMA HIF layer provides a common
> interface to the Port Layer.
>
> CLDMA manages 8 independent RX/TX physical channels with data flow
> control in HW queues. CLDMA uses ring buffers of General Packet
> Descriptors (GPD) for TX/RX. GPDs can represent multiple or single
> data buffers (DB).
>
> CLDMA HIF initializes GPD rings, registers ISR handlers for CLDMA
> interrupts, and initializes CLDMA HW registers.
>
> CLDMA TX flow:
> 1. Port Layer write
> 2. Get DB address
> 3. Configure GPD
> 4. Triggering processing via HW register write
>
> CLDMA RX flow:
> 1. CLDMA HW sends a RX "done" to host
> 2. Driver starts thread to safely read GPD
> 3. DB is sent to Port layer
> 4. Create a new buffer for GPD ring

[skipped]

> diff --git a/drivers/net/wwan/t7xx/t7xx_hif_cldma.c b/drivers/net/wwan/t7xx/t7xx_hif_cldma.c
> ...
> +static struct cldma_ctrl *cldma_md_ctrl[CLDMA_NUM];

Place this pointers array to a private _device_ state structure.
Otherwise, with these global pointers, the driver will break as soon
as a second modem is connected to the host.

Also all corresponding functions should be reworked to be able to
accept a modem state container pointer.

> +static DEFINE_MUTEX(ctl_cfg_mutex); /* protects CLDMA late init config */

Place this mutex to a private device state structure as well to avoid
useless inter-device locking and possible deadlocks.

> +static enum cldma_queue_type rxq_type[CLDMA_RXQ_NUM];
> +static enum cldma_queue_type txq_type[CLDMA_TXQ_NUM];
> +static int rxq_buff_size[CLDMA_RXQ_NUM];
> +static int rxq_buff_num[CLDMA_RXQ_NUM];
> +static int txq_buff_num[CLDMA_TXQ_NUM];

If these arrays could be shared between modem instances (i.e. if this
is a some kind of static configuration) then initialize them
statically or in the module_init(). Otherwise, if these arrays are
part of the runtime state, then place them in a device state
container.

--
Sergey