On Tue, Jun 28, 2022 at 03:23:56PM +0300, Serge Semin wrote: > Bjorn, > Do you have anything to say based on the notes below? I'm focused on the first series for now. > On Wed, Jun 22, 2022 at 08:04:37PM +0300, Serge Semin wrote: > > On Tue, Jun 21, 2022 at 01:29:41PM -0500, Bjorn Helgaas wrote: > > > On Mon, Jun 20, 2022 at 08:13:47PM +0300, Serge Semin wrote: > > > > On Wed, Jun 15, 2022 at 11:48:48AM -0500, Bjorn Helgaas wrote: > > > > > On Fri, Jun 10, 2022 at 11:57:05AM +0300, Serge Semin wrote: > > > > > > +struct dw_pcie_ops bt1_pcie_dw_ops = { > > > > > > + .read_dbi = bt1_pcie_read_dbi, > > > > > > + .write_dbi = bt1_pcie_write_dbi, > > > > > > + .write_dbi2 = bt1_pcie_write_dbi2, > > > > > > + .start_link = bt1_pcie_start_ltssm, > > > > > > + .stop_link = bt1_pcie_stop_ltssm, > > > > > > +}; > > > > > > > > Please rename to "dw_pcie_ops" as most drivers use. > > > > > > > > IMO matching the structure and its instance names is not a good idea. > > > > Other than confusing objects nature, at the very least it forces you to > > > > violate the local namespace convention. Thus in the line of the > > > > dw_pcie->ops initialization it looks like you use some generic > > > > operations while in fact you just refer to the locally defined > > > > DW PCIe ops instance with the generic variable name. Moreover AFAICS > > > > the latest platform drivers mainly use the vendor-specific prefix in > > > > the dw_pcie_ops structure instance including the ones acked by you, > > > > Lorenzo and Gustavo. What makes my code any different from them? > > > > > That's fair. I suggest "bt1_pcie_ops" or "bt1_dw_pcie_ops" to match > > > the other drivers that include the driver name: > > > > > > intel_pcie_ops > > > keembay_pcie_ops > > > kirin_dw_pcie_ops > > > tegra_dw_pcie_ops > > > > + ks_pcie_dw_pcie_ops > > > > which is even further from the suggested names.) As I said, they're not 100% consistent, but they almost all end in "pcie_ops". Yours ends in "pcie_dw_ops", which is why I suggested renaming to be similar to the others. I don't want to make a huge deal about this, but I do want as much similarity across drivers as possible. I get that it doesn't benefit *you*, but it's a huge benefit to everybody else. > > > They're not 100% consistent, but hopefully we can at least not make > > > things *less* consistent. > > > > I don't think we can make something less consistent if there is no real > > consistency.) There are at most five ops descriptors can be defined in > > the DW PCIe platform drivers: