Re: [PATCH 3/4] ath10k: Enable SRRI/DRRI support on ddr for WCN3990

[Kalle Valo <kvalo@codeaurora.org>]

pillair@codeaurora.org writes:

> From: Govind Singh <govinds@codeaurora.org>
>
> SRRI/DRRI are not mapped in the HW Shadow block and can lead
> to un-clocked access if common subsystem in the target is
> powered down due to idle mode.
>
> To mitigate this problem SRRI/DRRI can be read from
> DDR instead of doing an actual hardware read.
> Host allocates non cached memory on ddr and configures
> the physical address of this memory to the CE hardware.
> The hardware updates the RRI on this particular location.
> Read SRRI/DRRI from DDR location instead of
> direct target read.
>
> Enable retention restore on ddr using hw params to enable
> in specific targets.
>
> Signed-off-by: Govind Singh <govinds@codeaurora.org>
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>

[...]

> +	for (i = 0; i < CE_COUNT; i++) {
> +		ctrl1_regs = ar->hw_ce_regs->ctrl1_regs->addr;
> +		ce_base_addr = ath10k_ce_base_address(ar, i);
> +		ath10k_ce_write32(ar, ce_base_addr + ctrl1_regs,
> +				  ath10k_ce_read32(ar,
> +				  ce_base_addr + ctrl1_regs) |
> +				  ar->hw_ce_regs->upd->mask);
> +	}

This gives a checkpatch warning:

drivers/net/wireless/ath/ath10k/ce.c:1917: Alignment should match open parenthesis

You could fix that, and make the code a lot more readable, with
something like this:

tmp = ath10k_ce_read32(ar, ce_base_addr + ctrl1_regs);
tmp |= ar->hw_ce_regs->upd->mask;
ath10k_ce_write32(ar, ce_base_addr + ctrl1_regs, tmp);

Usually it's a good practise avoid making clever tricks, simple code is
a lot easier to read.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k