Messages in this thread | | | Date | Fri, 20 Mar 2020 14:22:45 -0500 | From | Bjorn Helgaas <> | Subject | Re: [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 <smuthayy@codeaurora.org> > > 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 similarly.
> +{ > + 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.
| |