On Wed, Jan 31, 2024 at 03:53:23PM -0800, matthew.gerlach@xxxxxxxxxxxxxxx wrote: > > > On Wed, 31 Jan 2024, Xu Yilun wrote: > > > On Tue, Jan 30, 2024 at 09:13:56AM -0800, matthew.gerlach@xxxxxxxxxxxxxxx wrote: > > > > > > > > > On Tue, 30 Jan 2024, Xu Yilun wrote: > > > > > > > On Wed, Jan 24, 2024 at 11:40:05AM -0800, matthew.gerlach@xxxxxxxxxxxxxxx wrote: > > > > > > > > > > > > > > > On Tue, 23 Jan 2024, Xu Yilun wrote: > > > > > > > > > > > On Mon, Jan 22, 2024 at 09:24:33AM -0800, Matthew Gerlach wrote: > > > > > > > Revision 2 of the Device Feature List (DFL) Port feature has > > > > > > > slightly different requirements than revision 1. Revision 2 > > > > > > > does not need the port to reset at driver startup. In fact, > > > > > > > > > > > > Please help illustrate what's the difference between Revision 1 & 2, and > > > > > > why revision 2 needs not. > > > > > > > > > > I will update the commit message to clarify the differences between revision > > > > > 1 and 2. > > > > > > > > > > > > > > > > > > performing a port reset during driver initialization can cause > > > > > > > driver race conditions when the port is connected to a different > > > > > > > > > > > > Please reorganize this part, in this description there seems be a > > > > > > software racing bug and the patch is a workaround. But the fact is port > > > > > > reset shouldn't been done for a new HW. > > > > > > > > > > Reorganizing the commit message a bit will help to clarify why port reset > > > > > should not be performed during driver initialization with revision 2 of the > > > > > hardware. > > > > > > > > > > > > > > > > > BTW: Is there a way to tell whether the port is connected to a different > > > > > > PF? Any guarantee that revision 3, 4 ... would need a port reset or not? > > > > > > > > > > The use of revision 2 of the port_hdr IP block indicates that the port can > > > > > be connected multiple PFs, but there is nothing explicitly stating which PFs > > > > > > > > Sorry, I mean any specific indicator other than enumerate the revision > > > > number? As you said below, checking revision number may not make further > > > > things right, then you need to amend code each time. > > > > > > Using a revision number to indicate the level of functionality for a > > > particular IP block seems to be a widely used approach. What other indicator > > > > If you still want to make the existing driver work, some capability indication > > would have more compatibility. That's more reasonable approach. Or you > > need to change existing behavior for each new revision, that's not > > actually widely used. > > I understand some capability indication would be better for compatibility > implementation. A revision number change is not as explicit or precise as > capability lists. > > > > > > of functionality level did you have in mind? > > > > I'm not trying to make the design. You tell me. > > One could use parameter blocks introduced in version 1 of the Device Feature > Header (DFH), or capability registers could be added the IP block. > In this particular case it seems the least impact to upstreamed software is > to keep the DFH and the register map unchanged, except for an incremented > revision number field. > > > > > If finally no indicator could be used, we have to use revision number. That's > > OK but make SW work harder, so I'm asking if anything could be done to > > avoid that. > > In this case, I don't think anything else can be done without bigger impacts > to the SW. Changing the existing SW is not a problem, repeat the same change every time is a problem. So if we make sure port reset is no longer needed after version 1, then this patch is OK. Otherwise, please re-evaluate. Thanks, Yilun > > > > > > > > > The revision number of an IP block would change when new functionality is > > > added to an IP block or the behavior of the IP block changes. It would be > > > expected that SW might need to change in order to use the new functionality > > > or to handle the change in behavior of the IP block. Ideally the new > > > revision of an IP block would be compatible with existing SW, but that > > > cannot be guaranteed. > > > > People make the IP block, and be compatible should be the concern if it > > want upstream support. > > Agreed, and making sure some capability mechanism exists when an IP is > created would be a great start. > > Thanks, > Matthew > > > > > Thanks, > > Yilun > > > > > > > > Thanks, > > > Matthew > > > > > > > > > > > Thanks, > > > > Yilun > > > > > > > > > the port is connected to. > > > > > > > > > > It is hard to predict the requirements and implementation of a future > > > > > revision of an IP block. If a requirement of a future revision is to work > > > > > with existing software, then the future revision would not require a port > > > > > reset at driver initialization. > > > > > > > > > > > > >