| Message ID | 20160620182636.21075.97627.stgit@obelix.dresden.micronet24.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
+Stephen Hi Benjamin, On 20 June 2016 at 12:26, Benjamin Tietz <uboot@dresden.micronet24.de> wrote: > From: Benjamin Tietz <benjamin@micronet24.de> > > Currently, clocks can be enabled, only. To be feature-complete - and allow > a bit of power-saving in applications - disabling a clock should be > possible, too. > > This extend the API of the DM clock driver to allow a clock to be disabled. > > The corresponding operation is optional for not breaking existing drivers. What does this mean? I can't see how it is optional. Also please can you rebase to mainline and resend, as the API has changed (sorry). > --- > drivers/clk/clk-uclass.c | 10 ++++++++++ > include/clk.h | 18 ++++++++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c > index b483c1e..462f5f8 100644 > --- a/drivers/clk/clk-uclass.c > +++ b/drivers/clk/clk-uclass.c > @@ -44,6 +44,16 @@ int clk_enable(struct udevice *dev, int periph) > return ops->enable(dev, periph); > } > > +int clk_disable(struct udevice *dev, int periph) > +{ > + struct clk_ops *ops = clk_get_ops(dev); > + > + if (!ops->disable) > + return -ENOSYS; > + > + return ops->disable(dev, periph); > +} > + > ulong clk_get_periph_rate(struct udevice *dev, int periph) > { > struct clk_ops *ops = clk_get_ops(dev); > diff --git a/include/clk.h b/include/clk.h > index ca20c3d..395f813 100644 > --- a/include/clk.h > +++ b/include/clk.h > @@ -43,6 +43,15 @@ struct clk_ops { > int (*enable)(struct udevice *dev, int periph); > > /** > + * disable() - Disable the clock for a peripheral > + * > + * @dev: clock provider > + * @periph: Peripheral ID to enable disable > + * @return zero on success, or -ve error code > + */ > + int (*disable)(struct udevice *dev, int periph); > + > + /** > * get_periph_rate() - Get clock rate for a peripheral > * > * @dev: Device to check (UCLASS_CLK) > @@ -90,6 +99,15 @@ ulong clk_set_rate(struct udevice *dev, ulong rate); > int clk_enable(struct udevice *dev, int periph); > > /** > + * clk_disable() - Disable the clock for a peripheral > + * > + * @dev: clock provider > + * @periph: Peripheral ID to enable > + * @return zero on success, or -ve error code > + */ > +int clk_disable(struct udevice *dev, int periph); > + > +/** > * clk_get_periph_rate() - Get current clock rate for a peripheral > * > * @dev: Device to check (UCLASS_CLK) > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
On 07/12/2016 10:02 AM, Simon Glass wrote: > +Stephen > > Hi Benjamin, > > On 20 June 2016 at 12:26, Benjamin Tietz <uboot@dresden.micronet24.de> wrote: >> From: Benjamin Tietz <benjamin@micronet24.de> >> >> Currently, clocks can be enabled, only. To be feature-complete - and allow >> a bit of power-saving in applications - disabling a clock should be >> possible, too. >> >> This extend the API of the DM clock driver to allow a clock to be disabled. >> >> The corresponding operation is optional for not breaking existing drivers. > > What does this mean? I can't see how it is optional. > > Also please can you rebase to mainline and resend, as the API has > changed (sorry). include/clk.h already contains a clk_disable() function. I suspect you can just go ahead and implement it and ignore this patch?
Hi Simon, On Tue, Jul 12, 2016 at 10:02:43AM -0600, Simon Glass wrote: > +Stephen > > Hi Benjamin, > > On 20 June 2016 at 12:26, Benjamin Tietz <uboot@dresden.micronet24.de> wrote: > > From: Benjamin Tietz <benjamin@micronet24.de> > > > > Currently, clocks can be enabled, only. To be feature-complete - and allow > > a bit of power-saving in applications - disabling a clock should be > > possible, too. > > > > This extend the API of the DM clock driver to allow a clock to be disabled. > > > > The corresponding operation is optional for not breaking existing drivers. > > What does this mean? I can't see how it is optional. What was meant was: If a driver can't handling disabling of peripherals, the request will be ignored silently - I think, the expected behaviour. > > Also please can you rebase to mainline and resend, as the API has > changed (sorry). ... including a clk_disable. So at least this patch can be dropped. regards Benjamin
Hello Vikas, hello Simon, the new clk-API leaves me with a problem. Previously there was a seperate way to access the clock-device itself (using clk_[gs]et_rate) and the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't available no more. But the system clock in STM32 doesn't have a separate ID. According to the device-tree the kernel doesn't care about that - or uses special logic. Possible solutions: a) Using a magic ID. Unfortunately, the peripheral used in the current device-tree are 0-based (and 0 is already in use), so this number isn't available. Moving the IDs around would break compatibility to the linux kernel. Negative numbers look like errors. Using a special high number looks unintuitive. And often result in additional work-arounds in the future. b) moving the sysclock and PLL-stuff in a seperate driver. That driver should be found in the device-tree, too. The device-tree should represent the hardware. Because of that, the source-clock of the STM32 RCC device (the clocking subsystem), should be either the external quartz or one of the internal sources, not a pll-device. Apart from that, the kernel in its current configuration probably is using this information to compute is current clock-speed. c) extending the struct clk, which looks clumsy, too. Any ideas? regards Benjamin
Hello Vikas, hello Simon, sleeping a night over this issue, I've came along with the following concept: The peripheral clocks are grouped by peripheral bus, that they are connected to. Each bus is driven by one or more set of registers, allowing up to 32 clocks to be selected. Having more than 8 registers would require massive modifications anyway. So it can be expected to have at most 256 clocks. As these domains partly have special clock-domains, partly rely on their bus-clock, I would like to interprete the id as a bitfield. At least the least significant byte represents a simple counter over the clocks - with all other bits set to zero representing real clocks. Setting the LSB of the third byte (using the mask 1<<16) might represent the "master clocks" - starting with the individual port clocks continued by the specialized clock domains. Using 1<<17 than could represent the individual PLLs directly. regards Benjamin On Thu, Jul 28, 2016 at 09:28:09PM +0200, Benjamin Tietz wrote: > Hello Vikas, hello Simon, > > the new clk-API leaves me with a problem. Previously there was a > seperate way to access the clock-device itself (using clk_[gs]et_rate) and > the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't > available no more. But the system clock in STM32 doesn't have a separate ID. > According to the device-tree the kernel doesn't care about that - or > uses special logic. > > Possible solutions: > > a) Using a magic ID. Unfortunately, the peripheral used in the current > device-tree are 0-based (and 0 is already in use), so this number isn't > available. Moving the IDs around would break compatibility to the linux > kernel. > > Negative numbers look like errors. > > Using a special high number looks unintuitive. And often result in > additional work-arounds in the future. > > b) moving the sysclock and PLL-stuff in a seperate driver. That driver > should be found in the device-tree, too. > The device-tree should represent the hardware. Because of that, the > source-clock of the STM32 RCC device (the clocking subsystem), should > be either the external quartz or one of the internal sources, not a > pll-device. Apart from that, the kernel in its current configuration > probably is using this information to compute is current clock-speed. > > c) extending the struct clk, which looks clumsy, too. > > Any ideas? > > regards > Benjamin
On 07/28/2016 01:28 PM, Benjamin Tietz wrote: > Hello Vikas, hello Simon, > > the new clk-API leaves me with a problem. Previously there was a > seperate way to access the clock-device itself (using clk_[gs]et_rate) and > the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't > available no more. But the system clock in STM32 doesn't have a separate ID. > According to the device-tree the kernel doesn't care about that - or > uses special logic. I don't understand the issue here. The device tree model is that every clock has some provider (a node/phandle) and some ID (a sequence of integer values). There's no such thing as "the clock-device itself". The kernel is consistent with that model; each client executes clk_get(), which for a DT-based system parses the phandle+clock_specifier and returns a clock handle, and then the client just uses that handle. That's exactly how U-Boot works too. Perhaps you can show the portion of DT that's causing the issue? Is the issue that there are clocks that your code needs to use that haven't yet been assigned a clock ID (clock specifier value) for use in DT (i.e. you have SoC-specific code that's calling clk_get() and the mapping isn't via DT)? If so, you simply need to assign one and everything will work fine. High numbers work fine for this. > Possible solutions: > > a) Using a magic ID. Unfortunately, the peripheral used in the current > device-tree are 0-based (and 0 is already in use), so this number isn't > available. Moving the IDs around would break compatibility to the linux > kernel. > > Negative numbers look like errors. > > Using a special high number looks unintuitive. And often result in > additional work-arounds in the future. What specific issues are you thinking of? I haven't experienced any when assigning IDs on Tegra, and I haven't observed anyone having issues doing this on any platform within the Linux kernel, where the exact same thing would be required.
Hello Stephen, On Fri, Jul 29, 2016 at 10:04:56AM -0600, Stephen Warren wrote: > On 07/28/2016 01:28 PM, Benjamin Tietz wrote: > >Hello Vikas, hello Simon, > > > >the new clk-API leaves me with a problem. Previously there was a > >seperate way to access the clock-device itself (using clk_[gs]et_rate) and > >the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't > >available no more. But the system clock in STM32 doesn't have a separate ID. > >According to the device-tree the kernel doesn't care about that - or > >uses special logic. > > I don't understand the issue here. The device tree model is that every clock > has some provider (a node/phandle) and some ID (a sequence of integer > values). There's no such thing as "the clock-device itself". For the current STM32 DT exactly that is the problem. The phandle is available and the set of IDs is defined. There is - at least at the moment - no ID defined for the system clock itself, but only for the derived clocks for the individual peripherals. The existing peripherals IDs even start to count at 0, so the "intuitive" solution to use that ID, doesn't work either. > > The kernel is consistent with that model; each client executes clk_get(), > which for a DT-based system parses the phandle+clock_specifier and returns a > clock handle, and then the client just uses that handle. That's exactly how > U-Boot works too. I must admit, that I haven't had an in-depth look on the STM32 clocking kernel sources yet. From other architectures I've seen, the system clock is either accessed by a certain ID, based on the underlying hardware - which isn't available on STM32 - or assumed to be "just there". On a first glance, the kernel STM32 driver seems to fall into the second category. > > Perhaps you can show the portion of DT that's causing the issue? It is the not existing potion of the DT ;) > > Is the issue that there are clocks that your code needs to use that haven't > yet been assigned a clock ID (clock specifier value) for use in DT (i.e. you > have SoC-specific code that's calling clk_get() and the mapping isn't via > DT)? If so, you simply need to assign one and everything will work fine. > High numbers work fine for this. > > >Possible solutions: > > > >a) Using a magic ID. Unfortunately, the peripheral used in the current > >device-tree are 0-based (and 0 is already in use), so this number isn't > >available. Moving the IDs around would break compatibility to the linux > >kernel. > > > >Negative numbers look like errors. > > > >Using a special high number looks unintuitive. And often result in > >additional work-arounds in the future. > > What specific issues are you thinking of? I haven't experienced any when > assigning IDs on Tegra, and I haven't observed anyone having issues doing > this on any platform within the Linux kernel, where the exact same thing > would be required. I'm no friend of magic numbers. Experience had shown, that - especially those introduced as a work-around - generate complicate problems at a later point of time. The first thing I can think of is to run into a hardware, having more peripheral clocks than expected. Nevertheless, in a follow-up mail I made a suggestion interpreting part of the ID as a bit-field - with enough room for bigger hardware. regards Benjamin
On 07/29/2016 11:26 AM, Benjamin Tietz wrote: > Hello Stephen, > > On Fri, Jul 29, 2016 at 10:04:56AM -0600, Stephen Warren wrote: >> On 07/28/2016 01:28 PM, Benjamin Tietz wrote: >>> Hello Vikas, hello Simon, >>> >>> the new clk-API leaves me with a problem. Previously there was a >>> seperate way to access the clock-device itself (using clk_[gs]et_rate) and >>> the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't >>> available no more. But the system clock in STM32 doesn't have a separate ID. >>> According to the device-tree the kernel doesn't care about that - or >>> uses special logic. >> >> I don't understand the issue here. The device tree model is that every clock >> has some provider (a node/phandle) and some ID (a sequence of integer >> values). There's no such thing as "the clock-device itself". > > For the current STM32 DT exactly that is the problem. The phandle is > available and the set of IDs is defined. There is - at least at the > moment - no ID defined for the system clock itself, but only for the > derived clocks for the individual peripherals. > > The existing peripherals IDs even start to count at 0, so the "intuitive" > solution to use that ID, doesn't work either. > >> >> The kernel is consistent with that model; each client executes clk_get(), >> which for a DT-based system parses the phandle+clock_specifier and returns a >> clock handle, and then the client just uses that handle. That's exactly how >> U-Boot works too. > > I must admit, that I haven't had an in-depth look on the STM32 clocking > kernel sources yet. From other architectures I've seen, the system clock > is either accessed by a certain ID, based on the underlying hardware - > which isn't available on STM32 - or assumed to be "just there". On a > first glance, the kernel STM32 driver seems to fall into the second category. > >> >> Perhaps you can show the portion of DT that's causing the issue? > > It is the not existing potion of the DT ;) > >> >> Is the issue that there are clocks that your code needs to use that haven't >> yet been assigned a clock ID (clock specifier value) for use in DT (i.e. you >> have SoC-specific code that's calling clk_get() and the mapping isn't via >> DT)? If so, you simply need to assign one and everything will work fine. >> High numbers work fine for this. >> >>> Possible solutions: >>> >>> a) Using a magic ID. Unfortunately, the peripheral used in the current >>> device-tree are 0-based (and 0 is already in use), so this number isn't >>> available. Moving the IDs around would break compatibility to the linux >>> kernel. >>> >>> Negative numbers look like errors. >>> >>> Using a special high number looks unintuitive. And often result in >>> additional work-arounds in the future. >> >> What specific issues are you thinking of? I haven't experienced any when >> assigning IDs on Tegra, and I haven't observed anyone having issues doing >> this on any platform within the Linux kernel, where the exact same thing >> would be required. > > I'm no friend of magic numbers. Experience had shown, that - especially > those introduced as a work-around - generate complicate problems at a > later point of time. Well, the numbers aren't magic. For DT to work in any way at all, the numbering used by DT properties must match the numbering implemented by the clock driver code. The only way to do that is to use a shared header files that defines what those IDs are. That header is included by both the DT compiler and the C compiler, so there's no chance of them getting out of sync. In the U-Boot source code, look at include/dt-bindings/clock/tegra20-car.h for example. > The first thing I can think of is to run into a hardware, having more > peripheral clocks than expected. Nevertheless, in a follow-up mail I > made a suggestion interpreting part of the ID as a bit-field - with > enough room for bigger hardware. Every piece of different HW requires a different DT binding (or at least a different compatible value; compatible values that represent similar HW can share a binding definition file). Each compatible value defines a separate set of clock IDs, and hence there's potentially a separate header file for each HW (although of course where different HW just happens to use the same set of IDs, they can use the same header file to define them). As such, it's easy to assign different "virtual" IDs for each different HW. For example, there's a separate clock ID header for Tegra20, Tegra30, etc. That's because all of (a) there are more clocks on Tegra30 thus requiring some additional IDs to be defined for it (b) Tegra30 probably moved some clock IDs around in HW thus requiring some clock IDs to be changed (c) virtual IDs had to be changed to leave space for the new clocks. Note that the clock IDs defined by the DT binding should match HW where it makes sense. For example on Tegra20, there are 96 clocks whose IDs correspond exactly to bit positions in a nice regular set of HW registers. However there are many clocks (mainly SoC clock inputs, PLLs, etc.) that aren't covered by that main register set, and hence have IDs assigned that are entirely arbitrary. With DT, if you have any clocks that don't have an obvious HW-defined ID, you have no choice but to assign them some arbitrary/chosen ID, and have a mapping table that converts DT IDs into information about those clocks; their HW identity, their type (PLL, peripheral, ...), register address, ...
Hello Stephen, On Fri, Jul 29, 2016 at 12:02:02PM -0600, Stephen Warren wrote: > On 07/29/2016 11:26 AM, Benjamin Tietz wrote: > >Hello Stephen, [snip] > >>>Using a special high number looks unintuitive. And often result in > >>>additional work-arounds in the future. > >> > >>What specific issues are you thinking of? I haven't experienced any when > >>assigning IDs on Tegra, and I haven't observed anyone having issues doing > >>this on any platform within the Linux kernel, where the exact same thing > >>would be required. > > > >I'm no friend of magic numbers. Experience had shown, that - especially > >those introduced as a work-around - generate complicate problems at a > >later point of time. > > Well, the numbers aren't magic. For DT to work in any way at all, the > numbering used by DT properties must match the numbering implemented by the > clock driver code. The only way to do that is to use a shared header files > that defines what those IDs are. That header is included by both the DT > compiler and the C compiler, so there's no chance of them getting out of > sync. In the U-Boot source code, look at > include/dt-bindings/clock/tegra20-car.h for example. Just because they are defined in a single place doesn't make that numbers less magic. They are choosen just for a specific software component. And U-Boot isn't the only user of device-trees. See: Linux Kernel. Syncing files of magic values between different projects is a pain and should be reduced to a bare minimum. As part of that, choosing those values should be avoided. If they are choosen once, modification isn't really possible anymore. If one must be choosen, this should be done carefully. > > >The first thing I can think of is to run into a hardware, having more > >peripheral clocks than expected. Nevertheless, in a follow-up mail I > >made a suggestion interpreting part of the ID as a bit-field - with > >enough room for bigger hardware. > > Every piece of different HW requires a different DT binding (or at least a > different compatible value; compatible values that represent similar HW can > share a binding definition file). Each compatible value defines a separate > set of clock IDs, and hence there's potentially a separate header file for > each HW (although of course where different HW just happens to use the same > set of IDs, they can use the same header file to define them). As such, it's > easy to assign different "virtual" IDs for each different HW. For example, > there's a separate clock ID header for Tegra20, Tegra30, etc. That's because > all of (a) there are more clocks on Tegra30 thus requiring some additional > IDs to be defined for it (b) Tegra30 probably moved some clock IDs around in > HW thus requiring some clock IDs to be changed (c) virtual IDs had to be > changed to leave space for the new clocks. > > Note that the clock IDs defined by the DT binding should match HW where it > makes sense. For example on Tegra20, there are 96 clocks whose IDs > correspond exactly to bit positions in a nice regular set of HW registers. > However there are many clocks (mainly SoC clock inputs, PLLs, etc.) that > aren't covered by that main register set, and hence have IDs assigned that > are entirely arbitrary. > > With DT, if you have any clocks that don't have an obvious HW-defined ID, > you have no choice but to assign them some arbitrary/chosen ID, and have a > mapping table that converts DT IDs into information about those clocks; > their HW identity, their type (PLL, peripheral, ...), register address, ... The device-tree was introduced as an additional abstraction layer between the hardware in use and the software running on this hardware, esp. the bootloader and the OS kernel. One goal was to have a single firmware running on different boards, based on the device-tree flashed on that board. This should improve the ways to reuse code written once. If the hardware changes there must be some way to represent this change in software - either by having a new driver ( involving dubled code ), or by a compatibility layer ( adding additional complexity ) or sometimes by a reasonable decision on the design or some simple IDs in the beginning. For the stm32-clk driver the currently implemented logic does need some kind of decision. Please don't interprete this as criticism on your work, but as I am, as everyone else, not perfect and there is no obvious solution I wrote down my complaints, asking for some comment or hint. To discuss a solution and not a problem, I would prefer to take my mail from this morning at 07:22 as base. In any case, have a nice weekend. regards Benjamin
Hi Benjamin, On 29 July 2016 at 12:34, Benjamin Tietz <uboot@dresden.micronet24.de> wrote: > Hello Stephen, > > On Fri, Jul 29, 2016 at 12:02:02PM -0600, Stephen Warren wrote: >> On 07/29/2016 11:26 AM, Benjamin Tietz wrote: >> >Hello Stephen, > > [snip] > >> >>>Using a special high number looks unintuitive. And often result in >> >>>additional work-arounds in the future. >> >> >> >>What specific issues are you thinking of? I haven't experienced any when >> >>assigning IDs on Tegra, and I haven't observed anyone having issues doing >> >>this on any platform within the Linux kernel, where the exact same thing >> >>would be required. >> > >> >I'm no friend of magic numbers. Experience had shown, that - especially >> >those introduced as a work-around - generate complicate problems at a >> >later point of time. >> >> Well, the numbers aren't magic. For DT to work in any way at all, the >> numbering used by DT properties must match the numbering implemented by the >> clock driver code. The only way to do that is to use a shared header files >> that defines what those IDs are. That header is included by both the DT >> compiler and the C compiler, so there's no chance of them getting out of >> sync. In the U-Boot source code, look at >> include/dt-bindings/clock/tegra20-car.h for example. > > Just because they are defined in a single place doesn't make that > numbers less magic. They are choosen just for a specific software > component. And U-Boot isn't the only user of device-trees. See: Linux > Kernel. > > Syncing files of magic values between different projects is a pain and > should be reduced to a bare minimum. As part of that, choosing those > values should be avoided. If they are choosen once, modification isn't > really possible anymore. If one must be choosen, this should be done > carefully. Note though that the numbers are part of the device-tree binding for an SoC. They are actually fixed in value. So the shared header file (between C code and DT) is pretty-much essential. This is the way it works at present. > >> >> >The first thing I can think of is to run into a hardware, having more >> >peripheral clocks than expected. Nevertheless, in a follow-up mail I >> >made a suggestion interpreting part of the ID as a bit-field - with >> >enough room for bigger hardware. >> >> Every piece of different HW requires a different DT binding (or at least a >> different compatible value; compatible values that represent similar HW can >> share a binding definition file). Each compatible value defines a separate >> set of clock IDs, and hence there's potentially a separate header file for >> each HW (although of course where different HW just happens to use the same >> set of IDs, they can use the same header file to define them). As such, it's >> easy to assign different "virtual" IDs for each different HW. For example, >> there's a separate clock ID header for Tegra20, Tegra30, etc. That's because >> all of (a) there are more clocks on Tegra30 thus requiring some additional >> IDs to be defined for it (b) Tegra30 probably moved some clock IDs around in >> HW thus requiring some clock IDs to be changed (c) virtual IDs had to be >> changed to leave space for the new clocks. >> >> Note that the clock IDs defined by the DT binding should match HW where it >> makes sense. For example on Tegra20, there are 96 clocks whose IDs >> correspond exactly to bit positions in a nice regular set of HW registers. >> However there are many clocks (mainly SoC clock inputs, PLLs, etc.) that >> aren't covered by that main register set, and hence have IDs assigned that >> are entirely arbitrary. >> >> With DT, if you have any clocks that don't have an obvious HW-defined ID, >> you have no choice but to assign them some arbitrary/chosen ID, and have a >> mapping table that converts DT IDs into information about those clocks; >> their HW identity, their type (PLL, peripheral, ...), register address, ... > > The device-tree was introduced as an additional abstraction layer > between the hardware in use and the software running on this hardware, > esp. the bootloader and the OS kernel. One goal was to have a single > firmware running on different boards, based on the device-tree flashed > on that board. This should improve the ways to reuse code written once. > > If the hardware changes there must be some way to represent this change > in software - either by having a new driver ( involving dubled code ), > or by a compatibility layer ( adding additional complexity ) or > sometimes by a reasonable decision on the design or some simple IDs in > the beginning. For the stm32-clk driver the currently implemented logic > does need some kind of decision. Please don't interprete this as > criticism on your work, but as I am, as everyone else, not perfect > and there is no obvious solution I wrote down my complaints, asking for > some comment or hint. > > To discuss a solution and not a problem, I would prefer to take my mail > from this morning at 07:22 as base. > In any case, have a nice weekend. As to your problem, can you not have separate clock drivers? For the ones that don't have multiple clocks, they can ignore the id. For the others, they can use the ID from the device tree. Regards, Simon
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index b483c1e..462f5f8 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -44,6 +44,16 @@ int clk_enable(struct udevice *dev, int periph) return ops->enable(dev, periph); } +int clk_disable(struct udevice *dev, int periph) +{ + struct clk_ops *ops = clk_get_ops(dev); + + if (!ops->disable) + return -ENOSYS; + + return ops->disable(dev, periph); +} + ulong clk_get_periph_rate(struct udevice *dev, int periph) { struct clk_ops *ops = clk_get_ops(dev); diff --git a/include/clk.h b/include/clk.h index ca20c3d..395f813 100644 --- a/include/clk.h +++ b/include/clk.h @@ -43,6 +43,15 @@ struct clk_ops { int (*enable)(struct udevice *dev, int periph); /** + * disable() - Disable the clock for a peripheral + * + * @dev: clock provider + * @periph: Peripheral ID to enable + * @return zero on success, or -ve error code + */ + int (*disable)(struct udevice *dev, int periph); + + /** * get_periph_rate() - Get clock rate for a peripheral * * @dev: Device to check (UCLASS_CLK) @@ -90,6 +99,15 @@ ulong clk_set_rate(struct udevice *dev, ulong rate); int clk_enable(struct udevice *dev, int periph); /** + * clk_disable() - Disable the clock for a peripheral + * + * @dev: clock provider + * @periph: Peripheral ID to enable + * @return zero on success, or -ve error code + */ +int clk_disable(struct udevice *dev, int periph); + +/** * clk_get_periph_rate() - Get current clock rate for a peripheral * * @dev: Device to check (UCLASS_CLK)
From: Benjamin Tietz <benjamin@micronet24.de> Currently, clocks can be enabled, only. To be feature-complete - and allow a bit of power-saving in applications - disabling a clock should be possible, too. This extend the API of the DM clock driver to allow a clock to be disabled. The corresponding operation is optional for not breaking existing drivers. --- drivers/clk/clk-uclass.c | 10 ++++++++++ include/clk.h | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+)