Skip to content

drivers: hwinfo_smartbond: Implement hwinfo_get_device_id() function #89943

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.

Sign up for GitHub

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

Jump to bottom
Merged
kartben merged 1 commit into from
May 21, 2025
Merged

drivers: hwinfo_smartbond: Implement hwinfo_get_device_id() function #89943

kartben merged 1 commit into from
May 21, 2025

Conversation

@ydamigos
Copy link
Contributor

@ydamigos ydamigos commented May 14, 2025

Implement hwinfo_get_device_id() function

@github-actions github-actions bot added platform: Renesas SmartBond Renesas Electronics Corporation, SmartBond area: HWINFO Hardware Information Driver labels May 14, 2025
alexanderwachter
alexanderwachter reviewed May 14, 2025
View reviewed changes
Copy link
Member

@alexanderwachter alexanderwachter left a comment

Choose a reason for hiding this comment

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

This is a unique id for esch chip right?

@ydamigos
Copy link
Contributor Author

ydamigos commented May 14, 2025

This is a unique id for esch chip right?

It's the same for all DA1469x chips of the same silicon version.
Please check: da1469x-datasheet section 42.18 Silicon Version Registers.

The API for hwinfo_get_device_id() mentions that "the ID depends on the hardware and is not guaranteed unique".

alexanderwachter
alexanderwachter requested changes May 14, 2025
View reviewed changes
Copy link
Member

@alexanderwachter alexanderwachter left a comment

Choose a reason for hiding this comment

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

Just checked the datasheet. This values are constant.
This API is not to obtain a device type, but to get a somehow unique id for things like usb.

@ydamigos ydamigos force-pushed the hwinfo-smartbond-get-device-id branch from 6bebf0e to 8ec5e24 Compare May 15, 2025 09:42
@ydamigos
Copy link
Contributor Author

ydamigos commented May 15, 2025

Just checked the datasheet. This values are constant. This API is not to obtain a device type, but to get a somehow unique id for things like usb.

Clear.
Changed it to read PRODUCT_INFO and CHIP_ID from Configuration Script in OTP.
image

@ydamigos ydamigos force-pushed the hwinfo-smartbond-get-device-id branch from 8ec5e24 to a2c443d Compare May 15, 2025 10:38
@ydamigos ydamigos force-pushed the hwinfo-smartbond-get-device-id branch from a2c443d to 7337785 Compare May 15, 2025 13:42
@alexanderwachter alexanderwachter self-assigned this May 17, 2025
blauret
blauret previously approved these changes May 20, 2025
View reviewed changes
@blauret
Copy link
Contributor

blauret commented May 20, 2025

Just checked the datasheet. This values are constant. This API is not to obtain a device type, but to get a somehow unique id for things like usb.

With the current implementation it is guaranteed to be unique.

This routine copies "length" number of bytes of the device ID to the buffer. If the device ID is smaller than length, the rest of the buffer is left unchanged. The ID depends on the hardware and is not guaranteed unique.

@alexanderwachter, where does the knowledge of the function intent come from? To me the documentation is not too clear on the goal. Is it worth adding a bit more details in the description?

@alexanderwachter
Copy link
Member

alexanderwachter commented May 20, 2025

Just checked the datasheet. This values are constant. This API is not to obtain a device type, but to get a somehow unique id for things like usb.

With the current implementation it is guaranteed to be unique.

This routine copies "length" number of bytes of the device ID to the buffer. If the device ID is smaller than length, the rest of the buffer is left unchanged. The ID depends on the hardware and is not guaranteed unique.

@alexanderwachter, where does the knowledge of the function intent come from? To me the documentation is not too clear on the goal. Is it worth adding a bit more details in the description?

My knowledge comes because it was my intent when I introduced this API. I'll think about adding some clarification. Or you can come up with a proposal.
At the time I wrote the description for me it is clear that the device id is there to identify a device. Now that i see that a device ID can identify a device type, it is maybe not so clear as I thought.

alexanderwachter
alexanderwachter requested changes May 20, 2025
View reviewed changes
Copy link
Member

@alexanderwachter alexanderwachter left a comment

Choose a reason for hiding this comment

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

I think the hal already checks the length.

drivers/hwinfo/hwinfo_smartbond.c Outdated
return -ENODATA;
}

da1469x_trimv_group_read(PRODUCT_INFO_GPOUP, &unique_id[0], product_info_len);
Copy link
Member

@alexanderwachter alexanderwachter May 20, 2025

Choose a reason for hiding this comment

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

Suggested change
da1469x_trimv_group_read(PRODUCT_INFO_GPOUP, &unique_id[0], product_info_len);
product_info_len = da1469x_trimv_group_read(PRODUCT_INFO_GPOUP, &unique_id[0], sizeof(unique_id[0]) * 3U);

Copy link
Contributor Author

@ydamigos ydamigos May 21, 2025

Choose a reason for hiding this comment

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

We need length in words. I introduced macro PRODUCT_INFO_LENGTH.

drivers/hwinfo/hwinfo_smartbond.c Outdated
}

da1469x_trimv_group_read(PRODUCT_INFO_GPOUP, &unique_id[0], product_info_len);
da1469x_trimv_group_read(CHIP_ID_GPOUP, &unique_id[3], chip_id_len);
Copy link
Member

@alexanderwachter alexanderwachter May 20, 2025

Choose a reason for hiding this comment

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

Suggested change
da1469x_trimv_group_read(CHIP_ID_GPOUP, &unique_id[3], chip_id_len);
chip_id_len = da1469x_trimv_group_read(CHIP_ID_GPOUP, &unique_id[3], sizeof(unique_id[3]));

Copy link
Contributor Author

@ydamigos ydamigos May 21, 2025

Choose a reason for hiding this comment

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

We need length in words. I introduced macro CHIP_ID_LENGTH.

drivers/hwinfo/hwinfo_smartbond.c Outdated
size_t len;
uint32_t unique_id[4];
uint8_t product_info_len = da1469x_trimv_group_num_words_get(PRODUCT_INFO_GPOUP);
uint8_t chip_id_len = da1469x_trimv_group_num_words_get(CHIP_ID_GPOUP);
Copy link
Member

@alexanderwachter alexanderwachter May 20, 2025

Choose a reason for hiding this comment

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

Suggested change
uint8_t chip_id_len = da1469x_trimv_group_num_words_get(CHIP_ID_GPOUP);
uint8_t chip_id_len;

Copy link
Contributor Author

@ydamigos ydamigos May 21, 2025

Choose a reason for hiding this comment

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

Done.

drivers/hwinfo/hwinfo_smartbond.c Outdated
{
size_t len;
uint32_t unique_id[4];
uint8_t product_info_len = da1469x_trimv_group_num_words_get(PRODUCT_INFO_GPOUP);
Copy link
Member

@alexanderwachter alexanderwachter May 20, 2025

Choose a reason for hiding this comment

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

I think

Suggested change
uint8_t product_info_len = da1469x_trimv_group_num_words_get(PRODUCT_INFO_GPOUP);
uint8_t product_info_len;

Copy link
Contributor Author

@ydamigos ydamigos May 21, 2025

Choose a reason for hiding this comment

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

Done

drivers/hwinfo/hwinfo_smartbond.c Outdated
uint8_t product_info_len = da1469x_trimv_group_num_words_get(PRODUCT_INFO_GPOUP);
uint8_t chip_id_len = da1469x_trimv_group_num_words_get(CHIP_ID_GPOUP);

if ((product_info_len != 3) || (chip_id_len != 1)) {
Copy link
Member

@alexanderwachter alexanderwachter May 20, 2025

Choose a reason for hiding this comment

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

Move this check after the read.

Copy link
Contributor Author

@ydamigos ydamigos May 21, 2025

Choose a reason for hiding this comment

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

Done

@ydamigos
drivers: hwinfo_smartbond: Implement hwinfo_get_device_id() function
b35e4f7 
Implement hwinfo_get_device_id() function

Signed-off-by: Ioannis Damigos <ioannis.damigos.uj@renesas.com>
@ydamigos ydamigos dismissed stale reviews from blauret and ioannis-karachalios via b35e4f7 May 21, 2025 08:52
@ydamigos ydamigos force-pushed the hwinfo-smartbond-get-device-id branch from 7337785 to b35e4f7 Compare May 21, 2025 08:52
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 21, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@kartben kartben merged commit 3956dff into zephyrproject-rtos:main May 21, 2025
26 checks passed
@ydamigos ydamigos deleted the hwinfo-smartbond-get-device-id branch May 21, 2025 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexanderwachter alexanderwachter alexanderwachter approved these changes

@ioannis-karachalios ioannis-karachalios ioannis-karachalios approved these changes

@andrzej-kaczmarek andrzej-kaczmarek Awaiting requested review from andrzej-kaczmarek

@blauret blauret Awaiting requested review from blauret

Assignees

@blauret blauret

@alexanderwachter alexanderwachter

@ioannis-karachalios ioannis-karachalios

Labels

area: HWINFO Hardware Information Driver platform: Renesas SmartBond Renesas Electronics Corporation, SmartBond

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ydamigos @blauret @alexanderwachter @ioannis-karachalios @kartben @andrzej-kaczmarek