Skip to content

Conversation

@erwango
Copy link
Member

@erwango erwango commented Mar 15, 2022

Add configure() function to clock_control API.
This function should enable caller to configure its source clock.

Extracted from #42097 where its usage is illustrated.

Note:
On maintainer's request, initial select() has been replaced by more generic configure()
#42097 is using the initial select() proposal.

Signed-off-by: Erwan Gouriou [email protected]

@erwango erwango force-pushed the dev_clock_control_api branch from d47d678 to d76cd9a Compare March 15, 2022 10:36
@erwango erwango changed the title include/drivers: clock_control.h: Add select() to api include/drivers: clock_control.h: Add configure() to api Mar 15, 2022
@erwango erwango force-pushed the dev_clock_control_api branch from d76cd9a to 2d7119f Compare March 15, 2022 10:41
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should rename this slightly to be specific to what is getting configure (ie is this just for clock source selection)

Copy link
Member Author

Choose a reason for hiding this comment

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

This was initially _select() and was then updated to be more generic on this initial comment: #43790 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

@galak Also, given the discussion we had yesterday, maybe it's not bad to 'open' the API given:

  • we'll likely have a clock_control overhaul in medium term. Letting experiments happen might be a good thing before and then build on these experiments
  • since this api implementation is vendor specific anyway, these isn't much risk anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

My concern with the name is its not clear at all what is being configured an adding an API that vague doesn't make sense to me.

@erwango erwango force-pushed the dev_clock_control_api branch 2 times, most recently from 754f34e to 7f9a126 Compare March 16, 2022 08:09
Add configure() function to clock_control API.
This function allows caller to configure a given clock.

Signed-off-by: Erwan Gouriou <[email protected]>
@erwango erwango force-pushed the dev_clock_control_api branch from 7f9a126 to ee352d6 Compare March 16, 2022 08:10
@erwango
Copy link
Member Author

erwango commented Mar 16, 2022

@nordic-krch, @gmarull PR updated according to comments. Thanks for the review

@carlescufi
Copy link
Member

API meeting:

  • The future of clock control in Zephyr likely needs an overhaul (similar to the one that took place with pinctrl)
  • In the meantime, this API is as close as we get to common infrastructure for clock control
  • No objections to extending the API as proposed

@carlescufi carlescufi requested review from galak and nordic-krch March 16, 2022 11:37
Copy link
Member

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

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

No objection to adding this function to the API, but it needs at least one implementation before we merge the PR.

@erwango
Copy link
Member Author

erwango commented Mar 16, 2022

No objection to adding this function to the API, but it needs at least one implementation before we merge the PR.

@MaureenHelm On going in following PR: #42097

@carlescufi carlescufi requested a review from MaureenHelm March 22, 2022 11:43
@MaureenHelm
Copy link
Member

No objection to adding this function to the API, but it needs at least one implementation before we merge the PR.

@MaureenHelm On going in following PR: #42097

Can we close this PR and merge the API change together with the implementation?

@erwango
Copy link
Member Author

erwango commented Mar 23, 2022

Can we close this PR and merge the API change together with the implementation?

Up to you. On my side, no problem to keep it open, as it's easier to access than deep down buried into #42097, if people would like to check it.

@carlescufi
Copy link
Member

No objection to adding this function to the API, but it needs at least one implementation before we merge the PR.

@MaureenHelm On going in following PR: #42097

Can we close this PR and merge the API change together with the implementation?

I would personally prefer merging this first @MaureenHelm, for future visibility. But I won't argue if you'd rather not.

@MaureenHelm
Copy link
Member

No objection to adding this function to the API, but it needs at least one implementation before we merge the PR.

@MaureenHelm On going in following PR: #42097

Can we close this PR and merge the API change together with the implementation?

I would personally prefer merging this first @MaureenHelm, for future visibility. But I won't argue if you'd rather not.

I'd rather not. Do we have any non-STM32 implementations in the works?

@erwango
Copy link
Member Author

erwango commented May 23, 2022

Merged as part of #45053. Closed

@erwango erwango closed this May 23, 2022
@erwango erwango deleted the dev_clock_control_api branch January 19, 2024 14:32
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 area: Clock Control

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants