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:
