On Tue, Oct 29, 2019 at 04:59:17PM +0800, Dilip Kota wrote: > > On 10/25/2019 5:09 PM, Andrew Murray wrote: > > On Tue, Oct 22, 2019 at 05:04:21PM +0800, Dilip Kota wrote: > > > Hi Andrew Murray, > > > > > > On 10/21/2019 9:03 PM, Andrew Murray wrote: > > > > On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote: > > > > > + > > > > > +void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts) > > > > > +{ > > > > > + u32 val; > > > > > + > > > > > + val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL); > > > > > + val &= ~PORT_LOGIC_N_FTS; > > > > > + val |= n_fts; > > > > > + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val); > > > > > +} > > > > I notice that pcie-artpec6.c (artpec6_pcie_set_nfts) also writes the FTS > > > > and defines a bunch of macros to support this. It doesn't make sense to > > > > duplicate this there. Therefore I think we need to update pcie-artpec6.c > > > > to use this new function. > > > I think we can do in a separate patch after these changes get merged and > > > keep this patch series for intel PCIe driver and required changes in PCIe > > > DesignWare framework. > > The pcie-artpec6.c is a DWC driver as well. So I think we can do all this > > together. This helps reduce the technical debt that will otherwise build up > > in duplicated code. > I agree with you to remove duplicated code, but at this point not sure what > all drivers has defined FTS configuration. > Reviewing all other DWC drivers and removing them can be done in one single > separate patch. I'm not asking to set up an FTS configuration for all DWC drivers, but instead to move this helper function you've created to somewhere like pcie-designware.c and call it from this driver and pcie-artpec6.c. Thanks, Andrew Murray > > Regards, > Dilip