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

Add ist415 semaphore and recording mode. #6626

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

IMJuBBang
Copy link

@IMJuBBang IMJuBBang commented Jan 10, 2025

  1. Apply semaphore related to the IC reset of ist415.
  2. Apply recording mode to the ist415_cmd.

Copy link
Contributor

@ewoodev ewoodev left a comment

Choose a reason for hiding this comment

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

Thank you for your changes

we are following the commit rule as below
Could you please modify commit title and description?

driver/input: <commit title>

<commit changes details>
<commit changes details>
<commit changes details>

https://github.com/Samsung/TizenRT/wiki/Documentations#commit-rules

if (ret) {
ist415dbg("Fail to write rec start scan\n");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please enter last line?

Copy link
Author

Choose a reason for hiding this comment

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

Your request has been completed.

@@ -581,14 +581,18 @@ int ist415_check_fw(struct ist415_dev_s *dev)

ist415vdbg("Main : %08X Core: %x, Config %x, Release: %x, Test: %x\n", fw->bin.main_ver, fw->bin.core_ver, fw->bin.config_ver, fw->bin.release_ver, fw->bin.test_ver);

sem_wait(&dev->sem);
Copy link
Contributor

@ewoodev ewoodev Jan 10, 2025

Choose a reason for hiding this comment

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

It is possible that the semaphore may be released by another signals.
Please apply this code

	while (sem_wait(&dev->sem) != OK) {
		ASSERT(get_errno() == EINTR);
	}

Copy link
Author

Choose a reason for hiding this comment

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

Your request has been completed.

Comment on lines 70 to 75
"sstream": "cpp",
"hash_map": "cpp",
"hash_set": "cpp",
"unordered_set": "cpp"
"unordered_set": "cpp",
"complex": "c",
"valarray": "c",
"list": "c",
"regex": "c",
"stack": "c",
"xhash": "c",
"xstring": "c",
"xtree": "c",
"xutility": "c",
"__hash_table": "c",
"xlocmon": "c",
"xtr1common": "c",
"__memory": "c",
"__refstring": "c",
"filesystem": "c",
"config.h": "c",
"poll.h": "c"
}
}
Copy link
Contributor

@ewoodev ewoodev Jan 10, 2025

Choose a reason for hiding this comment

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

Could you please exclude this change?

git reset HEAD^ .vscode/settings.json
git restore .vscode/settings.json
git commit --amend
git push -f <your fork remote> WORK-250108_ist415_semaphore_and_recording

Copy link
Author

Choose a reason for hiding this comment

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

Your request has been completed.

@IMJuBBang IMJuBBang force-pushed the WORK-250108_ist415_semaphore_and_recording branch 2 times, most recently from bcbf385 to 5172a66 Compare January 10, 2025 04:27
- Apply IC reset semaphore.
- Apply recording mode.

Signed-off-by: sanghoon <[email protected]>
@IMJuBBang IMJuBBang force-pushed the WORK-250108_ist415_semaphore_and_recording branch from 5172a66 to 5f1ad88 Compare January 10, 2025 04:47
The sem_wait() code modified the called part.

- sem_wait(&dev->sem)
+ while (sem_wait(&dev->sem) != OK) {
+     ASSERT(get_errno() == EINTR);
+ }

Signed-off-by: sanghoon <[email protected]>
@@ -1076,7 +1103,7 @@ int ist415_initialize(const char *path, struct i2c_dev_s *i2c, struct ist415_con
dev->enable = false;
dev->pre_enable = false;
dev->suspend = false;
dev->log = IST415_LOG_LEVEL_ERRO;
dev->log = IST415_LOG_LEVEL_INFO;
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't set the log level to default info because this may cause performance degradation.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I didn't check.
The log level has been reverted to IST415_LOG_LEVEL_ERRO.

Log level : IST415_LOG_LEVEL_INFO -> IST415_LOG_LEVEL_ERRO
@@ -490,6 +496,10 @@ static void ist415_stop_device(struct ist415_dev_s *dev)
{
ist415vdbg("%s\n", __func__);

while (sem_wait(&dev->sem) != OK) {
ASSERT(get_errno() == EINTR);
Copy link
Contributor

Choose a reason for hiding this comment

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

It call ASSERT, not retry, is it right?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes it's checking errno == EINTR. sem_wait fail case must be EINTR only. If not system is corrupted

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