On 11/1/2021 7:03 AM, Andy Shevchenko wrote:

On Sun, Oct 31, 2021 at 08:56:23PM -0700, Ricardo Martinez wrote:

quoted

From: Haijun Lio <haijun.liu@mediatek.com>

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

...

quoted

+void cldma_hw_reset(void __iomem *ao_base)
+{
+	iowrite32(ioread32(ao_base + REG_INFRA_RST4_SET) | RST4_CLDMA1_SW_RST_SET,
+		  ao_base + REG_INFRA_RST4_SET);
+	iowrite32(ioread32(ao_base + REG_INFRA_RST2_SET) | RST2_CLDMA1_AO_SW_RST_SET,
+		  ao_base + REG_INFRA_RST2_SET);
+	udelay(1);
+	iowrite32(ioread32(ao_base + REG_INFRA_RST4_CLR) | RST4_CLDMA1_SW_RST_CLR,
+		  ao_base + REG_INFRA_RST4_CLR);
+	iowrite32(ioread32(ao_base + REG_INFRA_RST2_CLR) | RST2_CLDMA1_AO_SW_RST_CLR,
+		  ao_base + REG_INFRA_RST2_CLR);

Setting and clearing are in the same order, is it okay?
Can we do it rather symmetrical?

In this case, order does not matter.

This will be symmetrical in the next iteration.

quoted

+}

...

quoted

+	mb(); /* prevents outstanding GPD updates */

Is there any counterpart of this barrier?

This is not needed, removing it.

...

quoted

+		ret = cldma_gpd_rx_from_queue(queue, budget, &over_budget);
+		if (ret == -ENODATA)
+			return 0;
+
+		if (ret)
+			return ret;

Drop redundant blank line

The style followed is to keep a blank line after 'if' blocks.

Is that acceptable as long as it is consistent across the driver?

quoted

+			/* greedy mode */
+			l2_rx_int = cldma_hw_int_status(hw_info, BIT(queue->index), true);

...

quoted

+exit:

Seems useless.

This tag is used when the PM patch is introduced later in the same series.

+	return ret;

...