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

usb:xHCI: add XHCI_RESET_ON_RESUME quirk for phytium #128

Merged
merged 3 commits into from
May 14, 2024
Merged

usb:xHCI: add XHCI_RESET_ON_RESUME quirk for phytium #128

merged 3 commits into from
May 14, 2024

Conversation

wangzhimin1179
Copy link
Contributor

@wangzhimin1179 wangzhimin1179 commented May 11, 2024

add XHCI_RESET_ON_RESUME qurik for phytium xhci controller .for example x100 pci switch and e2000 platform ...
this patch fix usb to work ,after resume from S3.

Chen Zhenhua and others added 3 commits May 11, 2024 15:03
The resume operation of Phytium Px210 xHCI host would failed
to restore state. Use the XHCI_RESET_ON_RESUME quirk to skip
it and reset the controller after resume.

Signed-off-by: Wang Zhimin <[email protected]>
Signed-off-by: Chen Zhenhua <[email protected]>
Reviewed-by: Chen Baozi <[email protected]>
Signed-off-by: Wang Yinfeng <[email protected]>
This patch documents the DT bindings for Phytium Pe220x XHCI controller.

Signed-off-by: Wang Zhimin <[email protected]>
Signed-off-by: Chen Baozi <[email protected]>
Phytium Pe220x xHCI host controller does not have suspend/resume
support. Therefore, use of the XHCI_RESET_ON_RESUME quirk is
mandatory in order to avoid failures after resume.

Signed-off-by: Wang Zhimin <[email protected]>
Signed-off-by: Chen Zhenhua <[email protected]>
@deepin-ci-robot deepin-ci-robot requested review from BLumia and myml May 11, 2024 08:49
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign goldendeng for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@deepin-ci-robot
Copy link

Hi @wangzhimin1179. Thanks for your PR.

I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Avenger-285714
Copy link
Collaborator

/ok-to-test

@Avenger-285714 Avenger-285714 merged commit d1f77da into deepin-community:linux-6.6.y May 14, 2024
2 of 3 checks passed
Avenger-285714 added a commit that referenced this pull request May 14, 2024
Add XHCI_RESET_ON_RESUME qurik for phytium xhci controller.
For example x100 pci switch and e2000 platform ...
this patch fix usb to work ,after resume from S3.

Include follow 3 commits:
* d1f77da usb: xhci: xhci-plat: Add XHCI_RESET_ON_RESUME quirk for Phytium Pe220x
  9b03336 dt-bindings: usb-xhci: Add Phytium Pe220x support
  80bd8b9 xHCI: add XHCI_RESET_ON_RESUME quirk for Phytium xHCI host

Signed-off-by: Wang Zhimin <[email protected]>
Signed-off-by: WangYuli <[email protected]>
@@ -113,6 +113,10 @@ static const struct xhci_plat_priv xhci_plat_brcm = {
.quirks = XHCI_RESET_ON_RESUME | XHCI_SUSPEND_RESUME_CLKS,
};

static const struct xhci_plat_priv xhci_plat_phytium_pe220x = {
Copy link
Member

Choose a reason for hiding this comment

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

此处代码会导致在没打开CONFIG_OF且打开CONFIG_ACPI时编译不通过。

Avenger-285714 pushed a commit to Avenger-285714/DeepinKernel that referenced this pull request Aug 30, 2024
commit 9a2fa1472083580b6c66bdaf291f591e1170123a upstream.

copy_fd_bitmaps(new, old, count) is expected to copy the first
count/BITS_PER_LONG bits from old->full_fds_bits[] and fill
the rest with zeroes.  What it does is copying enough words
(BITS_TO_LONGS(count/BITS_PER_LONG)), then memsets the rest.
That works fine, *if* all bits past the cutoff point are
clear.  Otherwise we are risking garbage from the last word
we'd copied.

For most of the callers that is true - expand_fdtable() has
count equal to old->max_fds, so there's no open descriptors
past count, let alone fully occupied words in ->open_fds[],
which is what bits in ->full_fds_bits[] correspond to.

The other caller (dup_fd()) passes sane_fdtable_size(old_fdt, max_fds),
which is the smallest multiple of BITS_PER_LONG that covers all
opened descriptors below max_fds.  In the common case (copying on
fork()) max_fds is ~0U, so all opened descriptors will be below
it and we are fine, by the same reasons why the call in expand_fdtable()
is safe.

Unfortunately, there is a case where max_fds is less than that
and where we might, indeed, end up with junk in ->full_fds_bits[] -
close_range(from, to, CLOSE_RANGE_UNSHARE) with
	* descriptor table being currently shared
	* 'to' being above the current capacity of descriptor table
	* 'from' being just under some chunk of opened descriptors.
In that case we end up with observably wrong behaviour - e.g. spawn
a child with CLONE_FILES, get all descriptors in range 0..127 open,
then close_range(64, ~0U, CLOSE_RANGE_UNSHARE) and watch dup(0) ending
up with descriptor deepin-community#128, despite deepin-community#64 being observably not open.

The minimally invasive fix would be to deal with that in dup_fd().
If this proves to add measurable overhead, we can go that way, but
let's try to fix copy_fd_bitmaps() first.

* new helper: bitmap_copy_and_expand(to, from, bits_to_copy, size).
* make copy_fd_bitmaps() take the bitmap size in words, rather than
bits; it's 'count' argument is always a multiple of BITS_PER_LONG,
so we are not losing any information, and that way we can use the
same helper for all three bitmaps - compiler will see that count
is a multiple of BITS_PER_LONG for the large ones, so it'll generate
plain memcpy()+memset().

Reproducer added to tools/testing/selftests/core/close_range_test.c

Cc: [email protected]
Signed-off-by: Al Viro <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Avenger-285714 pushed a commit that referenced this pull request Sep 2, 2024
commit 9a2fa1472083580b6c66bdaf291f591e1170123a upstream.

copy_fd_bitmaps(new, old, count) is expected to copy the first
count/BITS_PER_LONG bits from old->full_fds_bits[] and fill
the rest with zeroes.  What it does is copying enough words
(BITS_TO_LONGS(count/BITS_PER_LONG)), then memsets the rest.
That works fine, *if* all bits past the cutoff point are
clear.  Otherwise we are risking garbage from the last word
we'd copied.

For most of the callers that is true - expand_fdtable() has
count equal to old->max_fds, so there's no open descriptors
past count, let alone fully occupied words in ->open_fds[],
which is what bits in ->full_fds_bits[] correspond to.

The other caller (dup_fd()) passes sane_fdtable_size(old_fdt, max_fds),
which is the smallest multiple of BITS_PER_LONG that covers all
opened descriptors below max_fds.  In the common case (copying on
fork()) max_fds is ~0U, so all opened descriptors will be below
it and we are fine, by the same reasons why the call in expand_fdtable()
is safe.

Unfortunately, there is a case where max_fds is less than that
and where we might, indeed, end up with junk in ->full_fds_bits[] -
close_range(from, to, CLOSE_RANGE_UNSHARE) with
	* descriptor table being currently shared
	* 'to' being above the current capacity of descriptor table
	* 'from' being just under some chunk of opened descriptors.
In that case we end up with observably wrong behaviour - e.g. spawn
a child with CLONE_FILES, get all descriptors in range 0..127 open,
then close_range(64, ~0U, CLOSE_RANGE_UNSHARE) and watch dup(0) ending
up with descriptor #128, despite #64 being observably not open.

The minimally invasive fix would be to deal with that in dup_fd().
If this proves to add measurable overhead, we can go that way, but
let's try to fix copy_fd_bitmaps() first.

* new helper: bitmap_copy_and_expand(to, from, bits_to_copy, size).
* make copy_fd_bitmaps() take the bitmap size in words, rather than
bits; it's 'count' argument is always a multiple of BITS_PER_LONG,
so we are not losing any information, and that way we can use the
same helper for all three bitmaps - compiler will see that count
is a multiple of BITS_PER_LONG for the large ones, so it'll generate
plain memcpy()+memset().

Reproducer added to tools/testing/selftests/core/close_range_test.c

Cc: [email protected]
Signed-off-by: Al Viro <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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.

5 participants