[lkml]   [2020]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 07/12] pcie: qcom: add tx term offset support
On Fri, Mar 20, 2020 at 07:34:49PM +0100, Ansuel Smith wrote:
> From: Sham Muthayyan <>
> Add tx term offset support to pcie qcom driver
> need in some revision of the ipq806x soc

The usual (s/pcie/PCIe/, s/soc/SoC/, wrap to fill 75 columns, add
period at end). Apply to all patches.

> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK (0x1f << 16)
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x) (x << 16)

Since the rest of the file uses BIT(), I think it would make sense to
use GENMASK() here. And below, of course.

> +static inline void
> +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)

Follow coding style of rest of file, i.e.,

static inline void writel_masked(...)

The name "writel_masked" suggests that we're writing "val & mask" to a
register, but that's not what's happening. This is really a "clear
some bits and set others" operation.

The names are wordy, but we do have pci_clear_and_set_dword(),
pcie_capability_clear_and_set_word(), etc., functions that work

> +{
> + u32 val = readl(addr);
> +
> + val &= ~clear_mask;
> + val |= set_mask;
> + writel(val, addr);
> +}

> + /* Set Tx termination offset */

Capitalize consistently with other comments in the file. Below also.
Hmm, I see the file is already a bit of a mess in that regard. So
just do what you can.

 \ /
  Last update: 2020-03-20 20:23    [W:0.082 / U:1.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site