Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions include/drivers/clock_control.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,18 @@ typedef int (*clock_control_set)(const struct device *dev,
clock_control_subsys_t sys,
clock_control_subsys_rate_t rate);

typedef int (*clock_control_configure_fn)(const struct device *dev,
clock_control_subsys_t sys,
void *data);

struct clock_control_driver_api {
clock_control on;
clock_control off;
clock_control_async_on_fn async_on;
clock_control_get get_rate;
clock_control_get_status_fn get_status;
clock_control_set set_rate;
clock_control_configure_fn configure;
};

/**
Expand Down Expand Up @@ -270,6 +275,45 @@ static inline int clock_control_set_rate(const struct device *dev,
return api->set_rate(dev, sys, rate);
}

/**
* @brief Configure a source clock
*
* This function is non-blocking and can be called from any context.
* On success, the selected clock is configured as per caller's request.
*
* It is caller's responsibility to ensure that subsequent calls to the API
* provide the right information to allows clock_control driver to perform
* the right action (such as using the right clock source on clock_control_get_rate
* call).
*
* @p data is implementation specific and could be used to convey
* supplementary information required for expected clock configuration.
*
* @param dev Device structure whose driver controls the clock
* @param sys Opaque data representing the clock
* @param data Opaque data providing additional input for clock configuration
*
* @retval 0 On success
* @retval -ENOSYS If the device driver does not implement this call
* @retval -errno Other negative errno on failure.
*/
static inline int clock_control_configure(const struct device *dev,
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.

clock_control_subsys_t sys,
void *data)
{
if (!device_is_ready(dev)) {
return -ENODEV;
}

const struct clock_control_driver_api *api =
(const struct clock_control_driver_api *)dev->api;

if (api->configure == NULL) {
return -ENOSYS;
}

return api->configure(dev, sys, data);
}

#ifdef __cplusplus
}
Expand Down