Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: espi: ite: Receive vwire level without checking the valid bit #76813

Closed
wants to merge 1 commit into from

Conversation

yimjiajun
Copy link
Contributor

If the valid bit is set to '0', the corresponding virtual wire will retain its previous value and will not be updated by the current virtual wire packet.

The virtual wire value will be reset to its default by an eSPI reset. Therefore, this commit skips the check for the valid bit when receiving the level.

When a similar index with a single event valid is sent in a separate packet, the previous valid bit will be cleared. However, since it was previously valid, at the moment it will appear as non-valid and return '0' as a non-valid value.

Thus, we return the level directly without checking the valid bit, as the virtual wire will reset to its default value upon an eSPI reset.

Valid bit on ‘0’, the corresponding virtual wire will retain its
previous value and it not be updated for this virtual wire packet.

Virtual wire value will be reset as default by eSPI reset.

Signed-off-by: JiaJun Yim <[email protected]>
@MaureenHelm
Copy link
Member

@Dino-Li @GTLin08 @fabiobaltieri please take a look

@Dino-Li
Copy link
Contributor

Dino-Li commented Sep 27, 2024

If the valid bit is set to '0', the corresponding virtual wire will retain its previous value and will not be updated by the current virtual wire packet.

The virtual wire value will be reset to its default by an eSPI reset. Therefore, this commit skips the check for the valid bit when receiving the level.

When a similar index with a single event valid is sent in a separate packet, the previous valid bit will be cleared. However, since it was previously valid, at the moment it will appear as non-valid and return '0' as a non-valid value.

Thus, we return the level directly without checking the valid bit, as the virtual wire will reset to its default value upon an eSPI reset.

Is this symptom only seen on a specific platform? I'm curious why we need to think the level of VW signal is valid when its valid bit is set to invalid?

Copy link
Collaborator

@albertofloyd albertofloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yimjiajun This is probably not the best approach to solve the issue you are facing.
Ignoring the valid bit status (valid_mask) feels wrong (per eSPI spec), valid bit should be always checked, otherwise how can we distinguish between the bit that is updated and the ones that are unchanged.

image

Perhaps the problem is that the rest of bits in same index should be untouched.

@Dino-Li does the HW keeps track of valid bit once is set and reset only during eSPI reset.
If not maybe need to account this for the driver

@yimjiajun
Copy link
Contributor Author

For the diagram shown, the value needs to retain the previous state when the valid bit is not set to 1.

For example, if the same index 2 is used, and three separate virtual packets (SLP_Sx) are sent:

•	Initially, the value is 0x70 with bits 2-0 set to zero.
•	The first packet received is 0x11.
•	Then, the second packet received is 0x23 (with bit 1 set).
•	Finally, the third packet is 0x47 (with bit 2 set).

All three virtual packets conform to the specification, with the valid bit set individually for each packet while preserving the previous state.

However, during the last instance, only one valid bit is set. If the function API is used to read this, it may return a zero for the other valid bits that were not raised, even though those bits were previously determined to be valid based on prior packets.

@albertofloyd albertofloyd self-requested a review October 10, 2024 15:59
@Dino-Li
Copy link
Contributor

Dino-Li commented Nov 19, 2024

@yimjiajun there is PR merged: #81096

@Dino-Li
Copy link
Contributor

Dino-Li commented Nov 27, 2024

@yimjiajun can we close this PR?

@yimjiajun
Copy link
Contributor Author

@yimjiajun can we close this PR?

sure, no problem.

@Dino-Li
Copy link
Contributor

Dino-Li commented Nov 27, 2024

Fixed by this PR:
#81096

@Dino-Li Dino-Li closed this Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants