Skip to content

drivers: syscon: bflb efuses #89108

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

Merged
merged 3 commits into from
May 19, 2025
Merged

Conversation

VynDragon
Copy link
Collaborator

@VynDragon VynDragon commented Apr 25, 2025

This introduces bflb efuse access via syscon api.

This is not used at the moment but will be in the next steps of integration (for clock_control and things like DAC and ADC).

Copy link

github-actions bot commented Apr 25, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
hal_bouffalolab zephyrproject-rtos/hal_bouffalolab@c6c44b8 zephyrproject-rtos/hal_bouffalolab@5811738 (main) zephyrproject-rtos/[email protected]

All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@github-actions github-actions bot added manifest manifest-hal_bouffalolab DNM (manifest) This PR should not be merged (controlled by action-manifest) labels Apr 25, 2025
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

Is the EEPROM API really the correct one to use here?

@VynDragon
Copy link
Collaborator Author

VynDragon commented Apr 26, 2025

Is the EEPROM API really the correct one to use here?

That is one of the main questions yea, what alternatives is there?

@henrikbrixandersen
Copy link
Member

Is the EEPROM API really the correct one to use here?

That is one of the main questions yea, what alternatives is there?

If you are just reading the eFUSE settings, perhaps the syscon API is better suited?

@VynDragon
Copy link
Collaborator Author

VynDragon commented Apr 26, 2025

It looks like it would fit yea... Is there any example driver for it that doesnt just read memory actually? Or would that be a first driver for a syscon interface?

@nandojve
Copy link
Member

needs rebase

@VynDragon VynDragon force-pushed the bl60x_efuses branch 2 times, most recently from 6515391 to f9d7b8c Compare May 2, 2025 14:52
@VynDragon
Copy link
Collaborator Author

Rebased and made to use syscon.

@VynDragon VynDragon changed the title drivers: eeprom: bflb efuses drivers: syscon: bflb efuses May 2, 2025
@VynDragon VynDragon force-pushed the bl60x_efuses branch 4 times, most recently from 4705786 to b92a322 Compare May 8, 2025 14:25
@VynDragon VynDragon marked this pull request as ready for review May 8, 2025 15:14
@github-actions github-actions bot added platform: bouffalolab area: RISCV RISCV Architecture (32-bit & 64-bit) labels May 8, 2025
@kartben
Copy link
Collaborator

kartben commented May 15, 2025

re-assigning to BFLB maintainers

This sets revision to the one with vendor headers from final hal version

Signed-off-by: Camille BAUD <[email protected]>
@VynDragon VynDragon dismissed stale reviews from nandojve and kartben via f75e066 May 18, 2025 04:22
@github-actions github-actions bot removed the DNM (manifest) This PR should not be merged (controlled by action-manifest) label May 18, 2025
@VynDragon VynDragon force-pushed the bl60x_efuses branch 2 times, most recently from ca83c05 to bd2febe Compare May 18, 2025 05:09
nandojve
nandojve previously approved these changes May 18, 2025
kartben
kartben previously approved these changes May 18, 2025
Comment on lines +35 to +39
/* 32 Mhz Oscillator: 0
* crystal: 1
* PLL and 32M: 2
* PLL and crystal: 3
*/
Copy link
Member

Choose a reason for hiding this comment

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

could have used enum for these

enum bflb_root_clock {
	BFLB_CLOCK_32M_OSC,
	BFLB_CLOCK_XTAL,
	BFLB_CLOCK_PLL_32M_OSC,
	BFLB_CLOCK_PLL_XTAL,
	BFLB_CLOCK_INVALID,
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Clock driver with this functionality and many others (and enums/defines) is next, but it is dependant on this driver.

Copy link
Member

@ycsin ycsin May 19, 2025

Choose a reason for hiding this comment

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

Clock driver with this functionality and many others (and enums/defines) is next

We shouldn't depend on upcoming PRs since they are not guaranteed, but this is reasonably commented so 🤷‍♂️

Comment on lines +44 to +47
/* invalid value, fallback to internal 32M */
if (clock > 3) {
clock = 0;
}
Copy link
Member

@ycsin ycsin May 19, 2025

Choose a reason for hiding this comment

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

then this would naturally be:

Suggested change
/* invalid value, fallback to internal 32M */
if (clock > 3) {
clock = 0;
}
if (clock >= BFLB_CLOCK_INVALID) {
clock = BFLB_CLOCK_32M_OSC;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See clock driver comment

tmp = sys_read32(HBN_BASE + HBN_GLB_OFFSET);
old_clock_root = (tmp & HBN_ROOT_CLK_SEL_MSK) >> HBN_ROOT_CLK_SEL_POS;

efuse_bflb_set_root_clock(0);
Copy link
Member

Choose a reason for hiding this comment

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

if the enum suggestion above is adopted:

Suggested change
efuse_bflb_set_root_clock(0);
efuse_bflb_set_root_clock(BFLB_CLOCK_32M_OSC);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See clock driver comment

Comment on lines +177 to +178
tmp = sys_read32(HBN_BASE + HBN_GLB_OFFSET);
old_clock_root = (tmp & HBN_ROOT_CLK_SEL_MSK) >> HBN_ROOT_CLK_SEL_POS;
Copy link
Member

Choose a reason for hiding this comment

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

nit: we can have a matching helper efuse_bflb_get_root_clock() function

Suggested change
tmp = sys_read32(HBN_BASE + HBN_GLB_OFFSET);
old_clock_root = (tmp & HBN_ROOT_CLK_SEL_MSK) >> HBN_ROOT_CLK_SEL_POS;
old_clock_root = efuse_bflb_get_root_clock();

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See clock driver comment

VynDragon added 2 commits May 19, 2025 06:12
This introduces a driver used to access bouffalolab efuses via syscon API

Signed-off-by: Camille BAUD <[email protected]>
This enables the driver by default, it will be needed at init in the future

Signed-off-by: Camille BAUD <[email protected]>
@VynDragon VynDragon dismissed stale reviews from kartben and nandojve via 47c3ee5 May 19, 2025 04:12
Copy link

@VynDragon VynDragon requested review from kartben and nandojve May 19, 2025 05:31
@kartben kartben merged commit 7521971 into zephyrproject-rtos:main May 19, 2025
28 checks passed
@VynDragon VynDragon deleted the bl60x_efuses branch May 19, 2025 13:55
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.

7 participants