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

Do not check for ID in the device name #23

Open
wants to merge 2 commits into
base: staging.kernelci.org
Choose a base branch
from

Conversation

crazoes
Copy link

@crazoes crazoes commented Dec 23, 2022

The ID used in the device name for cros-ec drivers keeps changing and has no guarantee to remain stable across different kernel versions. Hence, do not check for ID in the device name.

Signed-off-by: Shreeya Patel [email protected]

The ID used in the device name for cros-ec drivers keeps changing
and has no gaurantee to remain stable accross different kernel versions.
Hence, do not check for ID in the device name.

Signed-off-by: Shreeya Patel <[email protected]>
Remove rockchip-i2s1-probed test and add a new rockchip-spdif1-probed test
as per the kernel changes from v5.18.

Signed-off-by: Shreeya Patel <[email protected]>
@crazoes
Copy link
Author

crazoes commented Jan 13, 2023

@hardboprobot can you please review these patches. It's just a simple fix which should work for now. I think more future-proof solution would be to use the firmware ID + driver name of the devices but that will require a lot of changes to the file and can be applied on other board files too. It is really not required to make big changes to the file currently imo.

@staging-kernelci-org staging-kernelci-org force-pushed the staging.kernelci.org branch 3 times, most recently from cb7bba1 to 278ce61 Compare February 8, 2023 08:03
@staging-kernelci-org staging-kernelci-org force-pushed the staging.kernelci.org branch 2 times, most recently from 49b1738 to 3d870e9 Compare February 22, 2023 00:01
fi

assert_device_present cros-ec-sensors-accel0-probed cros-ec-sensors cros-ec-accel.*

Choose a reason for hiding this comment

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

The issue with this is that there are multiple (2) files that will match the cros-ec-accel.* glob. So assert_device_present will see each of them as a different parameter. The first will be used as it's supposed to, but the second will be passed to the timeout command, which I think will cause an error. So the * globbing can only be used in the bootrr commands when it matches a single file (which is why I couldn't have #40 be a single check).

Copy link

Choose a reason for hiding this comment

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

@nfraprado @crazoes

How about fixing that in assert_device_present? Example: r-c-n@50a07b2

I guess this can be extended to other helpers.

Choose a reason for hiding this comment

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

@hardboprobot sounds sensible to me, the only thing I'm not sure about is requiring bash instead of sh. But if you could create a PR we can discuss/test it further there.

Copy link

Choose a reason for hiding this comment

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

Ugh I didn't get a notification for this, sorry. PR: #41

Copy link

@r-c-n r-c-n left a comment

Choose a reason for hiding this comment

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

See #41 to allow glob patterns in device names

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants