回覆: [PATCH 7/7] pci: aspeed: Add ASPEED PCIe host controller driver

[Jacky Chou]

> > > > +	u32 reg;
> > > > +	bool link;
> > > > +
> > > > +	regmap_read(pcie->pciephy, PEHR_MISC_300, &reg);
> > > > +	if (reg & CAPABILITY_GEN2) {
> > > > +		regmap_read(pcie->pciephy, PEHR_MISC_344, &reg);
> > > > +		link = !!(reg & LINK_STATUS_GEN2);
> > > > +	} else {
> > > > +		regmap_read(pcie->pciephy, PEHR_MISC_358, &reg);
> > > > +		link = !!(reg & LINK_STATUS_GEN4);
> > >
> > > While I don't entirely know the meaning of these bits, what if the
> > > link is not using maximum speed it is capable of, does this check
> misbehave?
> > >
> >
> > In our AST2700, there are gen4 RC and gen2 RC.
> > Therefore, here will get capability to confirm it is gen2 or gen4.
> > And the link status is in different register.
> 
> Okay, but then I'm a bit worried the naming of the defines as "Link Status" has
> a well-known meaning in PCIe Spec so it is confusing to call something else
> LINK_STATUS_*. Does that name come from some AST spec? In not, change the
> define naming. If yes, you should prefix it with so it is clear this is not referring
> to a generic PCIe thing and I suggest also adding a comment above those
> defines too so anyone looking them later won't wonder if you're just
> duplicating something from the PCIe spec.

Thanks for your reminder.
This code is used to get information whether PCIe have completed physical training.
I should change them to call LINK_UP_GEN2 and LINK_UP_GEN4.
I will modify this part to avoid confusion and add more comment, as you said.

Thanks,
Jacky