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

device: add api to check if device initialized or not #25053

Closed
wants to merge 1 commit into from

Conversation

wentongwu
Copy link
Contributor

Add api to check if device initialized success or not and
power management will create device list based on this API.

Fixes: #24750
Signed-off-by: Wentong Wu wentong.wu@intel.com

@wentongwu wentongwu requested a review from nashif May 7, 2020 03:36
@zephyrbot zephyrbot added the area: API Changes to public APIs label May 7, 2020
@wentongwu
Copy link
Contributor Author

The only failed case is not related to this PR, re-tun CI

2/2 qemu_x86_64               tests/drivers/ipm/peripheral.mailbox               FAILED: timeout
-------------out-3nd-pass/qemu_x86_64/tests/drivers/ipm/peripheral.mailbox/handler.log--------------
�cSeaBIOS (version rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org)
Booting from ROM..***** Booting Zephyr OS v1.14.2-1-gad387464460f *****
starting test - Test IPM
ipm_recv0: 'everything is awesome'
ipm_recv0: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor '
ipm_recv0: 'incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nosASSERTION FAIL [z_spin_lock_valid(l)] @ ../../../../../../include/spinlock.h:95
	Recursive spinlock
*** FATAL ERROR vector 6661 code 1016
***  RIP 8:0x100dc7 RSP 16:0x105908 RFLAGS 0x2
***  RAX 0x105017 RCX 0xa RDX 0x3f8 RSI 0x5f RDI 0x104fc9
***  R8 0xffffffff R9 0x0 R10 0x0 R11 0x0
-------------out-3nd-pass/qemu_x86_64/tests/drivers/ipm/peripheral.mailbox/handler.log--------------
1 of 2 tests passed with 0 warnings in 70 seconds 

@wentongwu wentongwu requested review from pabigot and vanti May 8, 2020 01:38
Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

Change to use the prefix that all other operations on devices use.
Don't put everything into the loop twice.

include/device.h Outdated Show resolved Hide resolved
break;
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this loop repeated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is this loop repeated?

@pabigot we should put the core devices at the first, but we don't know how many core devices are initialized like before. The first loop is to find the core devices and then other devices.

Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

Also this is targeting v1.14 but doesn't say that anywhere. It needs to be added to master and backported.

@andrewboie
Copy link
Contributor

why is this against the 1.4 branch and not master?

@wentongwu
Copy link
Contributor Author

why is this against the 1.4 branch and not master?

@andrewboie @pabigot because master is doing the refactor of device module and also PM module. Thanks

Add api to check if device initialized success or not and
power management will create device list based on this API.

Signed-off-by: Wentong Wu <wentong.wu@intel.com>
Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

I'm going to say "no" to this until we have guidance on the conditions for changing a stable release to add capability that is not currently supported in master.

In particular while device_is_initialized() is locally adequate, it is inconsistent with device_check_status() which is being proposed in #23589 (+ #24873) in the context of #22941. At this time that API hasn't gotten enough attention to determine whether it's the right approach.

If something needs to be done in 1.14 there should be an issue describing exactly what needs to be done, and why it's important enough to do even when it's not compatible with master. #24750 is generic, and is currently neither classified as a bug nor prioritized.

@nashif @carlescufi please provide programmatic guidance.

@tbursztyka
Copy link
Collaborator

I am not commenting the bug and the fix itself, but there is no need to add an API for a bug fix. Just check device->driver_api pointer where relevant directly (as it is being done in various places). It's not "nice" indeed, but adding an API won't fit with 1.14 maintenance I think.

@wentongwu wentongwu closed this May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants