Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
Hi Paul,
I've been looking at this stuff today, and this will cause pretty huge
changes to the set. This probably means a few weeks delay in
implementation at least.
Some comments on the technical perspectives below.
On 10/14/2013 02:45 AM, Paul Walmsley wrote:
Here you go:
1. Create DT nodes for the IP blocks that contain the clock control
registers, underneath the appropriate bus node. So for example OMAP4
would have:
/ocp {
prm@4a306000 {
compatible = xxx;
regs = 0x4a306000 ;
};
cm1@4a004000 {
compatible = xxx;
regs = 0x4a004000 ;
};
cm2@4a008000 {
compatible = xxx;
regs = 0x4a008000 ;
};
};
This is pure OS-independent hardware description (with the unfortunate
exception of the ocp part).
2. Create the clock data underneath the corresponding PRCM/CM/PRM device
nodes that their control registers exist in. So for example, dpll_per_ck
would belong to the cm1 device, since that's where its control registers
are located. Something like this:
cm1@4a004000 {
...
clocks {
dpll_per_ck@ {
clock data goes here
};
};
};
The DT data is intended to portray addressable devices from the
perspective of the CPUs that are booted with that data, organized by bus
structure. From this point of view, clocks that are controlled via a
particular IP block should have their data associated with that block.
3. Place clock register accesses in the low-level PRCM/CM/PRM device
driver. If other kernel code needs some resource that's provided by the
IP block, it should either come from common Linux framework routines (like
the clock code), or use functions exported from the low-level driver that
don't expose register addresses.
Otherwise, if MMIO accesses to that device are allowed from all over the
codebase, it creates an undebuggable mess:
A. It could result in IP blocks being partially programmed before they are
probed by their drivers. If their drivers (or the integration code) reset
the IP blocks, the non-driver code has no way of knowing that it needs to
reprogram the underlying IP block.
B. If any preparation is needed before the clock registers can be
accessed, like runtime PM calls or device_enable()-type calls, this should
be coordinated by the low-level IP block itself. We don't expect this to
be the case for PRCM/CM/PRM, but we do expect it to be the case for UART,
DSS, some audio clocks, etc.
C. The low-level IP block drivers may want to implement some caching layer
like regmap. If some register writes bypass the low-level driver, then
someone is likely to get a big unpleasant surprise.
D. It impairs the common debugging technique of adding code to the IP
block MMIO read/write functions to log register accesses.
4. Use relative register offsets from the top of the containing IP block's
base address, rather than absolute addresses.
cm1@4a004000 {
...
clocks {
dpll_per_ck@4140 {
clock data goes here
};
};
};
This makes it possible to reuse the same DT clock data in cases where the
same IP block is used, but at a different base address. This is probably
not a big issue with the system integration IP blocks, but quite possible
with the non-SoC-integration IP blocks. It also makes it obvious if
someone tries to sneak clocks into the IP block data that aren't
controlled by that IP block.
Relative addressing basically means I need to copy paste the code for
clk_divider + clk_mux from drivers/clk to drivers/clk/ti. Or
alternatively add a way to provide register read/write ops to the basic
clock.
5. Register the clocks from the low-level IP block drivers, rather than
from external code. That way there's no need to export low-level register
manipulation functions off to other kernel code. This registration can be
done when the PRCM/CM/PRM driver probes.
6. Move the OMAP clockdomain data underneath the DT node for the low-level
IP block that contains them:
cm1@4a004000 {
...
clocks {
...
};
clockdomains {
l3_init_clkdm: l3_init_clkdm@... {
...
};
};
};
I think clocks + clockdomains grouping nodes are unnecessary in this
case. Same can be accomplished with simply using the compatible string
for mapping to corresponding types. (This is probably valid for current
code also.)
For non-OMAP folks reading this thread, OMAP clockdomains have control
registers associated with them, located in the PRCM/CM/PRM IP block
address space, so that's where they belong.
7. drivers/clk/ti is probably the wrong place for most of the low-level
drivers for IP blocks like the PRM/PRCM/CM. Most of these IP blocks do
more than just control clocks: they also control other system entities
like reset lines, OMAP powerdomains,
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On Mon, 14 Oct 2013, Tero Kristo wrote:
On 10/14/2013 02:45 AM, Paul Walmsley wrote:
8. A few random comments about the individual clock binding data formats
themselves, based on a quick look:
A. It seems pretty odd and unnecessarily obscure to do something like
this:
dpll_usb_ck: dpll_usb_ck@... {
...
reg = 0x4a008180 0x4, 0x4a008184 0x4, 0x4a008188 0x4,
0x4a00818c 0x4;
...
};
It's at least self-documenting to do something like this:
dpll_usb_ck: dpll_usb_ck@... {
...
control-reg = ... ;
idlest-reg = ... ;
.. etc. ..
};
Some earlier version had something along these lines, but it was turned down.
I had also reg-names as documentation purposes along, but this was unnecessary
and was dropped also.
Which itself might not even be needed, depending on how the DPLL control
code is implemented. For example, if the relative offsets are always the
same for all OMAP4-family devices, maybe there's not even a need to
explicitly encode that into the DT data.
If I want to get rid of these, I need to add extra compatible strings for the
dpll types. There are several weird register offsets for omap3/am3 devices.
omap4/5/dra7/am4 behave more sanely.
B. Seems like you can remove the ti, prefix on the properties, since
they have no pretentions at genericity: they are specific to the
PRCM/CM/PRM IP block data, and registered by those drivers.
Can or should? It seems existing bindings use ti, prefix even on non-generic
bindings, meaning if I look at any other data in DT.
My comments in #8 are just regarding minor issues that don't seem
right. I don't have a significant objection to staying with the
existing property names here if you think that they are important.
C. Looks like the patches use the property autoidle-low to indicate that
the autoidle bit should be inverted. Low seems like the wrong
expression here - it invokes the actual voltage logic level of a hardware
signal, and we have no idea whether the hardware signal is using a low
voltage or a high voltage to express this condition. Would suggest
something like 'invert-autoidle-bit' instead.
D. Regarding ti,index-starts-at-one, it seems best to explicitly state
which index starts at one. The code mentions a mux index so please
consider renaming this something like mux-index-starts-at-one or
one-based-mux-index
The index is explicitly defined by the clock node where this is present. If it
is a mux-clock, then it is for mux-index. If for divider clock, it is index
for the divider.
- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On 10/14/2013 09:27 PM, Paul Walmsley wrote:
On Mon, 14 Oct 2013, Tero Kristo wrote:
On 10/14/2013 02:45 AM, Paul Walmsley wrote:
8. A few random comments about the individual clock binding data formats
themselves, based on a quick look:
A. It seems pretty odd and unnecessarily obscure to do something like
this:
dpll_usb_ck: dpll_usb_ck@... {
...
reg = 0x4a008180 0x4, 0x4a008184 0x4, 0x4a008188 0x4,
0x4a00818c 0x4;
...
};
It's at least self-documenting to do something like this:
dpll_usb_ck: dpll_usb_ck@... {
...
control-reg = ... ;
idlest-reg = ... ;
.. etc. ..
};
Some earlier version had something along these lines, but it was turned down.
I had also reg-names as documentation purposes along, but this was unnecessary
and was dropped also.
Which itself might not even be needed, depending on how the DPLL control
code is implemented. For example, if the relative offsets are always the
same for all OMAP4-family devices, maybe there's not even a need to
explicitly encode that into the DT data.
If I want to get rid of these, I need to add extra compatible strings for the
dpll types. There are several weird register offsets for omap3/am3 devices.
omap4/5/dra7/am4 behave more sanely.
B. Seems like you can remove the ti, prefix on the properties, since
they have no pretentions at genericity: they are specific to the
PRCM/CM/PRM IP block data, and registered by those drivers.
Can or should? It seems existing bindings use ti, prefix even on non-generic
bindings, meaning if I look at any other data in DT.
My comments in #8 are just regarding minor issues that don't seem
right. I don't have a significant objection to staying with the
existing property names here if you think that they are important.
Ok thanks, I'll be reworking most of the other items and will hopefully
have something ready soonish. Taking care of the register mappings is
rather nasty thing to do.
-Tero
C. Looks like the patches use the property autoidle-low to indicate that
the autoidle bit should be inverted. Low seems like the wrong
expression here - it invokes the actual voltage logic level of a hardware
signal, and we have no idea whether the hardware signal is using a low
voltage or a high voltage to express this condition. Would suggest
something like 'invert-autoidle-bit' instead.
D. Regarding ti,index-starts-at-one, it seems best to explicitly state
which index starts at one. The code mentions a mux index so please
consider renaming this something like mux-index-starts-at-one or
one-based-mux-index
The index is explicitly defined by the clock node where this is present. If it
is a mux-clock, then it is for mux-index. If for divider clock, it is index
for the divider.
- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
Here you go:
1. Create DT nodes for the IP blocks that contain the clock control
registers, underneath the appropriate bus node. So for example OMAP4
would have:
/ocp {
prm@4a306000 {
compatible = xxx;
regs = 0x4a306000 ;
};
cm1@4a004000 {
compatible = xxx;
regs = 0x4a004000 ;
};
cm2@4a008000 {
compatible = xxx;
regs = 0x4a008000 ;
};
};
This is pure OS-independent hardware description (with the unfortunate
exception of the ocp part).
2. Create the clock data underneath the corresponding PRCM/CM/PRM device
nodes that their control registers exist in. So for example, dpll_per_ck
would belong to the cm1 device, since that's where its control registers
are located. Something like this:
cm1@4a004000 {
...
clocks {
dpll_per_ck@ {
clock data goes here
};
};
};
The DT data is intended to portray addressable devices from the
perspective of the CPUs that are booted with that data, organized by bus
structure. From this point of view, clocks that are controlled via a
particular IP block should have their data associated with that block.
3. Place clock register accesses in the low-level PRCM/CM/PRM device
driver. If other kernel code needs some resource that's provided by the
IP block, it should either come from common Linux framework routines (like
the clock code), or use functions exported from the low-level driver that
don't expose register addresses.
Otherwise, if MMIO accesses to that device are allowed from all over the
codebase, it creates an undebuggable mess:
A. It could result in IP blocks being partially programmed before they are
probed by their drivers. If their drivers (or the integration code) reset
the IP blocks, the non-driver code has no way of knowing that it needs to
reprogram the underlying IP block.
B. If any preparation is needed before the clock registers can be
accessed, like runtime PM calls or device_enable()-type calls, this should
be coordinated by the low-level IP block itself. We don't expect this to
be the case for PRCM/CM/PRM, but we do expect it to be the case for UART,
DSS, some audio clocks, etc.
C. The low-level IP block drivers may want to implement some caching layer
like regmap. If some register writes bypass the low-level driver, then
someone is likely to get a big unpleasant surprise.
D. It impairs the common debugging technique of adding code to the IP
block MMIO read/write functions to log register accesses.
4. Use relative register offsets from the top of the containing IP block's
base address, rather than absolute addresses.
cm1@4a004000 {
...
clocks {
dpll_per_ck@4140 {
clock data goes here
};
};
};
This makes it possible to reuse the same DT clock data in cases where the
same IP block is used, but at a different base address. This is probably
not a big issue with the system integration IP blocks, but quite possible
with the non-SoC-integration IP blocks. It also makes it obvious if
someone tries to sneak clocks into the IP block data that aren't
controlled by that IP block.
5. Register the clocks from the low-level IP block drivers, rather than
from external code. That way there's no need to export low-level register
manipulation functions off to other kernel code. This registration can be
done when the PRCM/CM/PRM driver probes.
6. Move the OMAP clockdomain data underneath the DT node for the low-level
IP block that contains them:
cm1@4a004000 {
...
clocks {
...
};
clockdomains {
l3_init_clkdm: l3_init_clkdm@... {
...
};
};
};
For non-OMAP folks reading this thread, OMAP clockdomains have control
registers associated with them, located in the PRCM/CM/PRM IP block
address space, so that's where they belong.
7. drivers/clk/ti is probably the wrong place for most of the low-level
drivers for IP blocks like the PRM/PRCM/CM. Most of these IP blocks do
more than just control clocks: they also control other system entities
like reset lines, OMAP powerdomains, AVS, or OMAP clockdomains (which in
some cases may have nothing to do with clocks).
drivers/clk/ti is almost defensible for CM, if it weren't for the OMAP
clockdomain registers that are there. However, for the other OMAP
integration IP blocks, drivers/clk/ti definitely seems out of place. And
since some OMAP chips like OMAP2420 or AM43XX don't have separate CM and
PRM blocks -- they combine them both into a single PRCM module -- it seems
best to place CM, PRM, and other related code into a single common
directory, so they can easily share code across chips.
I'd suggest putting them in drivers/sysctrl/omap/. This would be intended
for low-level SoC IP block drivers that control
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On 10/12/2013 01:37 AM, Paul Walmsley wrote: On Fri, 11 Oct 2013, Tero Kristo wrote: Oh yea, one additional note you probably have missed. Mike asked us to fall back to vendor specific bindings at around v6 or so of this set. Take a look at v8, we have dropped the use of generic bindings, we are not trying to declare those with this set. No, I didn't miss it. This set introduces vendor specific bindings only, and even these are claimed 'unstable', which should be enough to discourage people from burning those to OTP type memory. Better to just avoid merging unstable bindings in the first place. Please keep this in mind: any change that anyone makes to the DT data needs to be supportable by the kernel into the indefinite future, once it makes it into arch/arm/boot/dts or the DT binding documentation. Also, any changes need to work for other OSes, i.e., the changes should not be Linux-specific. As pointed out earlier, the unstable claim just points to the fact that we might have some generic clock bindings in distant future at which point we _might_ evolve that way. Personally I have no problems carrying these binding to the far future, as they are vendor specific, and pretty much also device specific (am3-dpll-* omap4-dpll-* etc.) Later on, we can add also support for clock firmware or whatever if we want to optimize stuff. If we do not merge the bindings now, what is your proposal then? We have a couple of new SoC:s in queue and getting them boot mainline currently we only have this option to get them to work. Hardware is not going to wait for two years so we can have generic clock bindings in place. I would rather have this resolved as soon as possible, and it would have been much better if you came forward with your claims at version 1 or 2 of this set, rather than at v8 and saying all should be forfeit. -Tero Those have been stated goals for the Linux DT project since day one. Some folks haven't been monitoring those goals very closely and that's unfortunate. We could have avoided some pretty big messes. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On Thu, 10 Oct 2013, Tero Kristo wrote: On 10/09/2013 09:59 PM, Paul Walmsley wrote: Eh, one correction: On Wed, 9 Oct 2013, Paul Walmsley wrote: We could easily wind up with kernels that won't boot at all when used with newer DT data. This is a misstatement of the issue: the concern here is that newer kernels may not boot at all with older DT data - which could easily be in locked areas of the flash or firmware. I wonder who would be crazy enough to put DT data into a locked area, and to what purpose. If you can update the kernel, there is no point locking down DT data, this will just cause you unnecessary misery. The DT data will be used by bootloaders also :-( In situations where the bootloaders are signed and locked, the security people are also insisting that the DT data be signed and locked. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On 10/11/2013 08:54 PM, Paul Walmsley wrote: On Thu, 10 Oct 2013, Tero Kristo wrote: On 10/09/2013 09:59 PM, Paul Walmsley wrote: Eh, one correction: On Wed, 9 Oct 2013, Paul Walmsley wrote: We could easily wind up with kernels that won't boot at all when used with newer DT data. This is a misstatement of the issue: the concern here is that newer kernels may not boot at all with older DT data - which could easily be in locked areas of the flash or firmware. I wonder who would be crazy enough to put DT data into a locked area, and to what purpose. If you can update the kernel, there is no point locking down DT data, this will just cause you unnecessary misery. The DT data will be used by bootloaders also :-( In situations where the bootloaders are signed and locked, the security people are also insisting that the DT data be signed and locked. Well, even if you sign something, you can still update it. Writing any software to true OTP memory is one way to commit suicide IMO. How many nasty bugs have you seen with ROM code? Also, if people want to make their custom security solutions which are not supported by the kernel, why should the kernel care about it? We don't know the details, and can't influence the design, so we can't prepare for it anyway. -Tero -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On 10/11/2013 08:54 PM, Paul Walmsley wrote: On Thu, 10 Oct 2013, Tero Kristo wrote: On 10/09/2013 09:59 PM, Paul Walmsley wrote: Eh, one correction: On Wed, 9 Oct 2013, Paul Walmsley wrote: We could easily wind up with kernels that won't boot at all when used with newer DT data. This is a misstatement of the issue: the concern here is that newer kernels may not boot at all with older DT data - which could easily be in locked areas of the flash or firmware. I wonder who would be crazy enough to put DT data into a locked area, and to what purpose. If you can update the kernel, there is no point locking down DT data, this will just cause you unnecessary misery. The DT data will be used by bootloaders also :-( In situations where the bootloaders are signed and locked, the security people are also insisting that the DT data be signed and locked. Oh yea, one additional note you probably have missed. Mike asked us to fall back to vendor specific bindings at around v6 or so of this set. Take a look at v8, we have dropped the use of generic bindings, we are not trying to declare those with this set. This set introduces vendor specific bindings only, and even these are claimed 'unstable', which should be enough to discourage people from burning those to OTP type memory. -Tero -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On Fri, 11 Oct 2013, Tero Kristo wrote: Well, even if you sign something, you can still update it. Writing any software to true OTP memory is one way to commit suicide IMO. How many nasty bugs have you seen with ROM code? Also, if people want to make their custom security solutions which are not supported by the kernel, why should the kernel care about it? We don't know the details, and can't influence the design, so we can't prepare for it anyway. The point is that newer kernels need to be compatible with older DT data: that's the stable ABI criterion that's been discussed quite a bit. The worst-case scenario if the bindings are at a top-level node and not fully cooked is that it can make it very difficult, if not impossible, to have this forward compatibility for the DT data without having extremely nasty hacks. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On Fri, 11 Oct 2013, Tero Kristo wrote: Oh yea, one additional note you probably have missed. Mike asked us to fall back to vendor specific bindings at around v6 or so of this set. Take a look at v8, we have dropped the use of generic bindings, we are not trying to declare those with this set. No, I didn't miss it. This set introduces vendor specific bindings only, and even these are claimed 'unstable', which should be enough to discourage people from burning those to OTP type memory. Better to just avoid merging unstable bindings in the first place. Please keep this in mind: any change that anyone makes to the DT data needs to be supportable by the kernel into the indefinite future, once it makes it into arch/arm/boot/dts or the DT binding documentation. Also, any changes need to work for other OSes, i.e., the changes should not be Linux-specific. Those have been stated goals for the Linux DT project since day one. Some folks haven't been monitoring those goals very closely and that's unfortunate. We could have avoided some pretty big messes. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
* Tero Kristo t-kri...@ti.com [131010 01:25]: On 10/09/2013 09:55 PM, Paul Walmsley wrote: So in my view, the right things to do here are to: 1. associate SoC DT clock data with the device node that contains the clock control registers So, either cm, prcm, and maybe control_module instead of current clocks. How much do we benefit from this? This would get rid of need to call of_iomap() for each register basically. It seems that all you need to do for that is to set up two or three clock driver instances and have the clocks registered under them. Then later on those can be children of cm, prcm and scm core drivers as needed without much of an issue. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On 10/09/2013 09:59 PM, Paul Walmsley wrote: Eh, one correction: On Wed, 9 Oct 2013, Paul Walmsley wrote: We could easily wind up with kernels that won't boot at all when used with newer DT data. This is a misstatement of the issue: the concern here is that newer kernels may not boot at all with older DT data - which could easily be in locked areas of the flash or firmware. I wonder who would be crazy enough to put DT data into a locked area, and to what purpose. If you can update the kernel, there is no point locking down DT data, this will just cause you unnecessary misery. -Tero -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
Hey Paul, My dibs on this below. On 10/09/2013 09:55 PM, Paul Walmsley wrote: On Mon, 7 Oct 2013, Tony Lindgren wrote: And assuming Paul is OK with these patches in general. Actually, I have several concerns with this series, as you and I discussed. Some of us have been talking them over with other folks for the past few months to try to figure out what to do. Most of the concerns have fairly easy technical solutions, but we shouldn't merge these patches until they're resolved. The issues are: 1. whether the clock data should go into DT 2. what the right place for DT clock data is in the DT 3. whether it makes sense to merge experimental DT bindings in this case 4. where clockdomain data belongs The first issue - and the one I'm least concerned about - is that, in my view, it still does not make technical sense to move this data into DT. This is chip-level hardware data that never needs to be changed by board designers or end users. Adding it to DT is going to result in a boot-time cost (due to DT parse overhead) and additional memory consumption for very little benefit, compared to an implementation that places this data into a dynamically loadable module. For some users, the boot-time bloat is a big deal: the example that's been mentioned to me recently is an automotive back-up camera that needs to cold-boot to complete functionality in a few hundred microseconds. Personally I share the concern, I don't see much point in using the DT for any kind of purpose... it is just another binary compatibility breaker in the picture in addition to boot-loader, and it basically does not solve any _real_ problems either. Boot time issues can be solved by adding alternative clock data sources like Tony pointed out, granted, we don't have support for those yet, but we need to start somewhere. However, according to some other upstream maintainers, Linus's goal is to move most of the device-specific static data out of the kernel tree, into DT files (in the ARM case). If that non-technical constraint is indeed the dominant consideration, then I agree that moving this data to DT is the only viable course of action. Yeah... Personally I can't see any other way forward right now either as I was basically given the option to use DT for this work or not do it at all... ... The second issue concerns where the SoC clock nodes should go in the DT. In these patches, the clock data has been encoded in a large clocks node off the top level. This is almost certainly not the right thing to do from a device hardware point of view. These clocks don't exist as standalone devices with their own address spaces decoded on the interconnect. In all of the SoC cases that I'm aware of, clock control registers are part of larger IP blocks. For example, in the OMAP case, most of the system integration clock control registers are part of the OMAP-specific PRCM block, PRM block, or CM block. Then there are other device-specific clocks, like DSS PLLs or UART dividers. The control registers for these are generally located in the subsystem or device IP block itself, and are inaccessible when the subsystem or IP block is disabled. These device-specific clocks belong to the IP block, not the SoC integration. So, for example, if two SoCs use the same IP block, then the clock registers, and their offsets from the IP block address space base, are likely to be identical. None of the clock registers defined in this set reside outside PRCM / control module, so they are always accessible. IP block internal dividers for UART and like, are defined and used only internally by the device drivers. So in my view, the right things to do here are to: 1. associate SoC DT clock data with the device node that contains the clock control registers So, either cm, prcm, and maybe control_module instead of current clocks. How much do we benefit from this? This would get rid of need to call of_iomap() for each register basically. 2. specify relative offsets for clock registers from the start of the IP block address range, rather than absolute addresses for clock registers 3. place the responsibility for registering clocks into the IP block drivers themselves This naturally separates clocks into per-IP block DT files. It also provides the CCF with an easy way to ensure that the device that encloses the clock is enabled and accessible by the CPU core, before trying to access registers inside. Similarly, non-SoC off-chip clock data (such as for dedicated I2C PLLs) should also be associated with their I2C device node. Making these changes to Tero's existing patches should be relatively straightforward, based on what I've seen. For the set, it doesn't matter where the clock nodes reside, if someone can point me out where to put them, they can be easily moved around. Separating the individual clocks based on their IP mapping can probably just be done by checking their register address.
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
Eh, one correction: On Wed, 9 Oct 2013, Paul Walmsley wrote: We could easily wind up with kernels that won't boot at all when used with newer DT data. This is a misstatement of the issue: the concern here is that newer kernels may not boot at all with older DT data - which could easily be in locked areas of the flash or firmware. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On Mon, 7 Oct 2013, Tony Lindgren wrote: And assuming Paul is OK with these patches in general. Actually, I have several concerns with this series, as you and I discussed. Some of us have been talking them over with other folks for the past few months to try to figure out what to do. Most of the concerns have fairly easy technical solutions, but we shouldn't merge these patches until they're resolved. The issues are: 1. whether the clock data should go into DT 2. what the right place for DT clock data is in the DT 3. whether it makes sense to merge experimental DT bindings in this case 4. where clockdomain data belongs The first issue - and the one I'm least concerned about - is that, in my view, it still does not make technical sense to move this data into DT. This is chip-level hardware data that never needs to be changed by board designers or end users. Adding it to DT is going to result in a boot-time cost (due to DT parse overhead) and additional memory consumption for very little benefit, compared to an implementation that places this data into a dynamically loadable module. For some users, the boot-time bloat is a big deal: the example that's been mentioned to me recently is an automotive back-up camera that needs to cold-boot to complete functionality in a few hundred microseconds. However, according to some other upstream maintainers, Linus's goal is to move most of the device-specific static data out of the kernel tree, into DT files (in the ARM case). If that non-technical constraint is indeed the dominant consideration, then I agree that moving this data to DT is the only viable course of action. ... The second issue concerns where the SoC clock nodes should go in the DT. In these patches, the clock data has been encoded in a large clocks node off the top level. This is almost certainly not the right thing to do from a device hardware point of view. These clocks don't exist as standalone devices with their own address spaces decoded on the interconnect. In all of the SoC cases that I'm aware of, clock control registers are part of larger IP blocks. For example, in the OMAP case, most of the system integration clock control registers are part of the OMAP-specific PRCM block, PRM block, or CM block. Then there are other device-specific clocks, like DSS PLLs or UART dividers. The control registers for these are generally located in the subsystem or device IP block itself, and are inaccessible when the subsystem or IP block is disabled. These device-specific clocks belong to the IP block, not the SoC integration. So, for example, if two SoCs use the same IP block, then the clock registers, and their offsets from the IP block address space base, are likely to be identical. So in my view, the right things to do here are to: 1. associate SoC DT clock data with the device node that contains the clock control registers 2. specify relative offsets for clock registers from the start of the IP block address range, rather than absolute addresses for clock registers 3. place the responsibility for registering clocks into the IP block drivers themselves This naturally separates clocks into per-IP block DT files. It also provides the CCF with an easy way to ensure that the device that encloses the clock is enabled and accessible by the CPU core, before trying to access registers inside. Similarly, non-SoC off-chip clock data (such as for dedicated I2C PLLs) should also be associated with their I2C device node. Making these changes to Tero's existing patches should be relatively straightforward, based on what I've seen. ... Regarding the third issue: let's postulate for the moment that the clock binding issues that I mention in #2 above are ignored (ha!), and that the existing DT clock data is merged. These bindings are currently marked as Unstable - ABI compatibility may be broken in the future. What happens if, when we meet to discuss clock bindings at the kernel summit, we decide that significant changes are needed? We could easily wind up with kernels that won't boot at all when used with newer DT data. Not to mention, merging such a large amount of code and data before the bindings are stable will increase the risk of massive churn as the bindings evolve towards stability. So IMHO, the way to fix this is to consider the clock data to be IP-block specific, as mentioned in #2. Then there's no need for global clock bindings for the SoC clock cases. Otherwise, it seems prudent to at least wait until the global clock bindings are no longer marked as unstable. The whole DT migration was predicated on the ideas of reducing churn in the Linux codebase, and preserving forward compatibility for DT data. We shouldn't discard these goals just to merge something a little sooner. ... The fourth issue is where the clockdomain data should go. The basic issue here is that the notion of clockdomains here is
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
* Paul Walmsley p...@pwsan.com [131009 12:04]: On Mon, 7 Oct 2013, Tony Lindgren wrote: And assuming Paul is OK with these patches in general. Actually, I have several concerns with this series, as you and I discussed. Some of us have been talking them over with other folks for the past few months to try to figure out what to do. Most of the concerns have fairly easy technical solutions, but we shouldn't merge these patches until they're resolved. The issues are: 1. whether the clock data should go into DT 2. what the right place for DT clock data is in the DT 3. whether it makes sense to merge experimental DT bindings in this case 4. where clockdomain data belongs The first issue - and the one I'm least concerned about - is that, in my view, it still does not make technical sense to move this data into DT. This is chip-level hardware data that never needs to be changed by board designers or end users. Adding it to DT is going to result in a boot-time cost (due to DT parse overhead) and additional memory consumption for very little benefit, compared to an implementation that places this data into a dynamically loadable module. For some users, the boot-time bloat is a big deal: the example that's been mentioned to me recently is an automotive back-up camera that needs to cold-boot to complete functionality in a few hundred microseconds. However, according to some other upstream maintainers, Linus's goal is to move most of the device-specific static data out of the kernel tree, into DT files (in the ARM case). If that non-technical constraint is indeed the dominant consideration, then I agree that moving this data to DT is the only viable course of action. I've been advocating that any combination of dt defined clocks, loadable module clocks and clocks loaded via /lib/firmware should be allowed. That allows mapping just the bootime clocks in DT, or all the clocks in DT depending what suits the use case. And yes, based on the flames I've received over the years, Linus clearly wants the SoC specific data out of the kernel. And there's already a lot of the SoC specific data that's now building up in various linux generic frameworks.. The second issue concerns where the SoC clock nodes should go in the DT. In these patches, the clock data has been encoded in a large clocks node off the top level. This is almost certainly not the right thing to do from a device hardware point of view. These clocks don't exist as standalone devices with their own address spaces decoded on the interconnect. In all of the SoC cases that I'm aware of, clock control registers are part of larger IP blocks. For example, in the OMAP case, most of the system integration clock control registers are part of the OMAP-specific PRCM block, PRM block, or CM block. Then there are other device-specific clocks, like DSS PLLs or UART dividers. The control registers for these are generally located in the subsystem or device IP block itself, and are inaccessible when the subsystem or IP block is disabled. These device-specific clocks belong to the IP block, not the SoC integration. So, for example, if two SoCs use the same IP block, then the clock registers, and their offsets from the IP block address space base, are likely to be identical. So in my view, the right things to do here are to: 1. associate SoC DT clock data with the device node that contains the clock control registers 2. specify relative offsets for clock registers from the start of the IP block address range, rather than absolute addresses for clock registers 3. place the responsibility for registering clocks into the IP block drivers themselves This naturally separates clocks into per-IP block DT files. It also provides the CCF with an easy way to ensure that the device that encloses the clock is enabled and accessible by the CPU core, before trying to access registers inside. Similarly, non-SoC off-chip clock data (such as for dedicated I2C PLLs) should also be associated with their I2C device node. Making these changes to Tero's existing patches should be relatively straightforward, based on what I've seen. Yes that's a valid concern and makes sense to me. It also it seems that moving clocks around with this approach does not break the binding ABI. Regarding the third issue: let's postulate for the moment that the clock binding issues that I mention in #2 above are ignored (ha!), and that the existing DT clock data is merged. These bindings are currently marked as Unstable - ABI compatibility may be broken in the future. What happens if, when we meet to discuss clock bindings at the kernel summit, we decide that significant changes are needed? We could easily wind up with kernels that won't boot at all when used with newer DT data. Not to mention, merging such a large amount of code and data before the
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
* Paul Walmsley p...@pwsan.com [131009 12:07]: Eh, one correction: On Wed, 9 Oct 2013, Paul Walmsley wrote: We could easily wind up with kernels that won't boot at all when used with newer DT data. This is a misstatement of the issue: the concern here is that newer kernels may not boot at all with older DT data - which could easily be in locked areas of the flash or firmware. That's easy to avoid by allowing any combination of DT defined clocks, loadable module clocks and /lib/firmware clocks depending on the use case. The older kernels have the clocks defined in DT, the newer kernels just need to parse them. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On 10/08/2013 08:40 AM, Mike Turquette wrote: Quoting Tero Kristo (2013-09-25 01:48:06) Hi all, Version 7 contains following high level changes: - Dropped support for basic bindings from Mike Turquette, instead using vendor specific bindings for all clocks - Mux clock + divider clock vendor specific bindings get rid of use of the bit-masks, instead these are generated runtime based on parent clock / divider min/max info, see individual patches for details - Added support for dummy_ck nodes, was missing from previous version - Fixed support for omap3630 - Added support for new clock node type: ti,mux-gate-clock (using composite clock type) - OMAP4 / OMAP5 only build fixes (compiler flag checks added to some files) - most of the of_omap_* calls renamed to of_ti_* - Rebased on top of v3.12-rc2 Hi Tero, I had one really minor request for one of the patches, otherwise I'm pretty happy with this series and I can take in the clock parts for 3.13. A few general comments: Yea, I'll add the is_enabled check to the APLL and repost. 1) I think I mentioned some time back that it would be wise to put a disclaimer at the top of each DT binding description stating something to the effect: Binding status: Unstable - ABI compatibility may be broken in the future. Are you OK merging these bindings as-is without that kind of a disclaimer? Well, personally I don't mind, but if you think this would be good, I'll add this to next rev. 2) I hope to see the clockdomain stuff go away in the future. I'm OK to take it for now to aid in this DT transition but I would like some commitment that it will not stay in drivers/clk/omap forever. I guess for clockdomain you'll need to figure out how to handle those in a generic driver way. I think the clockdomain stuff should be possible to move to wherever CM driver is going to move. drivers/power/omap/... or something maybe. I have been starting some initial work on the CM driver cleanup for this purpose. 3) Same concern as above but for the DT clkdev alias stuff. I guess you'll need to make all of the dependent drivers play nice with DT. Do you plan to remove it within a reasonable timeframe? It would be a nice reduction in LoC and more importantly it would mean that drivers were using clkdev in a more-correct fashion. This is probably harder, I think Tony is better to comment here, but my idea is that in the worst case we can just break non-conforming drivers by dropping the clkdev tables if nothing else helps. :) Most of the tables are currently needed by hwmod, and there are already patches around for adding DT clock support for hwmods, so once these go in, we can at least reduce the table sizes considerably already. -Tero Regards, Mike Some detailed comments about patch level changes (if no mention, no major changes): - Patch #1: * removed use of reg-names property, instead registers mapped using compatible string * removed ti,modes property, instead uses DPLL mode flags now * separated AM33XX DPLLs under their own compatible strings - Patch #4 #5: new patches - Patch #6: merged basic gate support from Mike to this patch as vendor specific gate clock support - Patch #9 #10: new patches - Patch #11: dummy clocks added, USB / ABE DPLL setup ordering changed - Patch #14: dummy clocks added - Patch #20: dropped usage of reg-names property - Patch #21: dummy clocks added - Patch #22 #23: new patches - Patch #30: AM35XX clock data added to this patch - Patch #31: * dummy clocks added * added missing 3630 clocks Test branch available here: https://github.com/t-kristo/linux-pm.git branch: mainline-3.12-rc2-ti-dt-clks-v7 Testing done: - am335x-bone : boot only - omap5-sevm : boot only - dra7-evm : boot only - omap3-beagle : boot + suspend/resume (ret + off) - omap4-panda-es : boot + suspend/resume Nishanth has also been doing some tests with omap3-beagle-xm with some WIP versions of this branch, but not with this latest one. Nishanth, maybe you can provide test results to this one also? -Tero -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
* Tero Kristo t-kri...@ti.com [131008 01:26]: On 10/08/2013 08:40 AM, Mike Turquette wrote: 3) Same concern as above but for the DT clkdev alias stuff. I guess you'll need to make all of the dependent drivers play nice with DT. Do you plan to remove it within a reasonable timeframe? It would be a nice reduction in LoC and more importantly it would mean that drivers were using clkdev in a more-correct fashion. This is probably harder, I think Tony is better to comment here, but my idea is that in the worst case we can just break non-conforming drivers by dropping the clkdev tables if nothing else helps. :) Most of the tables are currently needed by hwmod, and there are already patches around for adding DT clock support for hwmods, so once these go in, we can at least reduce the table sizes considerably already. It should be possible to remove these as we go as things become DT only. Probably the biggest blocker right now is making omap3 DT only. But let's not start breaking things intentionally! Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On 10/07/2013 05:15 AM, Tony Lindgren wrote: * Tero Kristo t-kri...@ti.com [131004 08:42]: Hi, Just a gentle reminder, anybody have any comments on this series or should we start queuing stuff? Well omap4 seems to work for me just fine, and omap3 in legacy mode. But looks like omap3 device tree based booting is breaking after I pulled in your test branch. I get the following on 3630 based dm3730. Uart4 breaks because of the issue with basic DT blobs. You need at least this patch: http://linux-kernel.2935.n7.nabble.com/RFC-PATCH-ARM-OMAP3630-Add-generic-machine-descriptor-td722791.html ... from Nishanth to get it to work right. Otherwise the clock init tries to use omap3430 clock data for omap3630 which leaves uart4 nodes out. -Tero Regards, Tony [0.187652] omap_hwmod: uart4: cannot clk_get main_clk uart4_fck [0.187713] omap_hwmod: uart4: cannot _init_clocks [0.187713] [ cut here ] [0.187774] WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2434 _init+0x6c/0x80() [0.187774] omap_hwmod: uart4: couldn't init clocks [0.187774] Modules linked in: [0.187805] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.12.0-rc2-00020-gdea1778-dirty #791 [0.187866] [c001cd54] (unwind_backtrace+0x0/0xf4) from [c0018e80] (show_stack+0x14/0x1c) [0.187896] [c0018e80] (show_stack+0x14/0x1c) from [c0554400] (dump_stack+0x6c/0x9c) [0.187927] [c0554400] (dump_stack+0x6c/0x9c) from [c00488a8] (warn_slowpath_common+0x64/0x84) [0.187957] [c00488a8] (warn_slowpath_common+0x64/0x84) from [c004895c] (warn_slowpath_fmt+0x30/0x40) [0.187988] [c004895c] (warn_slowpath_fmt+0x30/0x40) from [c07a4bec] (_init+0x6c/0x80) [0.188018] [c07a4bec] (_init+0x6c/0x80) from [c002e068] (omap_hwmod_for_each+0x50/0x64) [0.188049] [c002e068] (omap_hwmod_for_each+0x50/0x64) from [c07a5440] (__omap_hwmod_setup_all+0x34/0x4c) [0.188079] [c07a5440] (__omap_hwmod_setup_all+0x34/0x4c) from [c00087ac] (do_one_initcall+0x2c/0x150) [0.188110] [c00087ac] (do_one_initcall+0x2c/0x150) from [c0797508] (do_basic_setup+0x94/0xd0) [0.188140] [c0797508] (do_basic_setup+0x94/0xd0) from [c07975c0] (kernel_init_freeable+0x7c/0x120) [0.188171] [c07975c0] (kernel_init_freeable+0x7c/0x120) from [c05526b4] (kernel_init+0xc/0x164) [0.188201] [c05526b4] (kernel_init+0xc/0x164) from [c00149c8] (ret_from_fork+0x14/0x2c) [0.188415] ---[ end trace 1b75b31a2719ed1c ]--- [0.810241] [ cut here ] [0.810302] WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126 _enable+0x254/0x280() [0.810302] omap_hwmod: timer12: enabled state can only be entered from initialized, idle, or disabled state [0.810333] Modules linked in: [0.810363] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW 3.12.0-rc2-00020-gdea1778-dirty #791 [0.810394] [c001cd54] (unwind_backtrace+0x0/0xf4) from [c0018e80] (show_stack+0x14/0x1c) [0.810424] [c0018e80] (show_stack+0x14/0x1c) from [c0554400] (dump_stack+0x6c/0x9c) [0.810455] [c0554400] (dump_stack+0x6c/0x9c) from [c00488a8] (warn_slowpath_common+0x64/0x84) [0.810485] [c00488a8] (warn_slowpath_common+0x64/0x84) from [c004895c] (warn_slowpath_fmt+0x30/0x40) [0.810516] [c004895c] (warn_slowpath_fmt+0x30/0x40) from [c003085c] (_enable+0x254/0x280) [0.810546] [c003085c] (_enable+0x254/0x280) from [c00308b0] (omap_hwmod_enable+0x28/0x40) [0.810577] [c00308b0] (omap_hwmod_enable+0x28/0x40) from [c0030f8c] (omap_device_enable+0x3c/0x80) [0.810607] [c0030f8c] (omap_device_enable+0x3c/0x80) from [c0030fe0] (_od_runtime_resume+0x10/0x1c) [0.810638] [c0030fe0] (_od_runtime_resume+0x10/0x1c) from [c036e120] (__rpm_callback+0x2c/0x80) [0.810668] [c036e120] (__rpm_callback+0x2c/0x80) from [c036e198] (rpm_callback+0x24/0x78) [0.810699] [c036e198] (rpm_callback+0x24/0x78) from [c036f444] (rpm_resume+0x3c0/0x60c) [0.810729] [c036f444] (rpm_resume+0x3c0/0x60c) from [c036f970] (__pm_runtime_resume+0x4c/0x68) [0.810760] [c036f970] (__pm_runtime_resume+0x4c/0x68) from [c0044954] (omap_dm_timer_probe+0x234/0x334) [0.810791] [c0044954] (omap_dm_timer_probe+0x234/0x334) from [c03659ac] (platform_drv_probe+0x1c/0x24) [0.810821] [c03659ac] (platform_drv_probe+0x1c/0x24) from [c03645e4] (really_probe+0x70/0x200) [0.810852] [c03645e4] (really_probe+0x70/0x200) from [c03647a8] (driver_probe_device+0x34/0x50) [0.810852] [c03647a8] (driver_probe_device+0x34/0x50) from [c0364858] (__driver_attach+0x94/0x98) [0.810882] [c0364858] (__driver_attach+0x94/0x98) from [c0362cd4] (bus_for_each_dev+0x74/0x98) [0.810913] [c0362cd4] (bus_for_each_dev+0x74/0x98) from [c0363510] (bus_add_driver+0xe8/0x278) [0.810943] [c0363510] (bus_add_driver+0xe8/0x278) from [c0364e30] (driver_register+0x5c/0xf8) [0.810974] [c0364e30] (driver_register+0x5c/0xf8) from [c00087ac] (do_one_initcall+0x2c/0x150) [0.811004]
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
* Tero Kristo t-kri...@ti.com [131006 23:43]: On 10/07/2013 05:15 AM, Tony Lindgren wrote: * Tero Kristo t-kri...@ti.com [131004 08:42]: Hi, Just a gentle reminder, anybody have any comments on this series or should we start queuing stuff? Well omap4 seems to work for me just fine, and omap3 in legacy mode. But looks like omap3 device tree based booting is breaking after I pulled in your test branch. I get the following on 3630 based dm3730. Uart4 breaks because of the issue with basic DT blobs. You need at least this patch: http://linux-kernel.2935.n7.nabble.com/RFC-PATCH-ARM-OMAP3630-Add-generic-machine-descriptor-td722791.html ... from Nishanth to get it to work right. Otherwise the clock init tries to use omap3430 clock data for omap3630 which leaves uart4 nodes out. OK thanks yes that solves it, I was missing the compatible flag for my not quite ready test boards (3730-evm and zoom3). I'll apply Nishant's fix with some changes. Then for this series, once you have the necessary acks, can you please split into following two separate immutable branches against v3.12-rc3: 1. clock code changes 2. dts changes Then those can be pulled in as needed by Benoit, Mike and I. For the whole series, I think this quite nicely removes some nasty kernel data dependencies for omaps, so feel free to add: Acked-by: Tony Lindgren t...@atomide.com Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On 10/07/2013 10:41 AM, Tony Lindgren wrote: * Tero Kristo t-kri...@ti.com [131006 23:43]: On 10/07/2013 05:15 AM, Tony Lindgren wrote: * Tero Kristo t-kri...@ti.com [131004 08:42]: Hi, Just a gentle reminder, anybody have any comments on this series or should we start queuing stuff? Well omap4 seems to work for me just fine, and omap3 in legacy mode. But looks like omap3 device tree based booting is breaking after I pulled in your test branch. I get the following on 3630 based dm3730. Uart4 breaks because of the issue with basic DT blobs. You need at least this patch: http://linux-kernel.2935.n7.nabble.com/RFC-PATCH-ARM-OMAP3630-Add-generic-machine-descriptor-td722791.html ... from Nishanth to get it to work right. Otherwise the clock init tries to use omap3430 clock data for omap3630 which leaves uart4 nodes out. OK thanks yes that solves it, I was missing the compatible flag for my not quite ready test boards (3730-evm and zoom3). I'll apply Nishant's fix with some changes. I will post out an RFC based on Olof's suggestion in a few mins. will be nice to see any deltas needed. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
* Tony Lindgren t...@atomide.com [131007 08:49]: * Tero Kristo t-kri...@ti.com [131006 23:43]: On 10/07/2013 05:15 AM, Tony Lindgren wrote: * Tero Kristo t-kri...@ti.com [131004 08:42]: Hi, Just a gentle reminder, anybody have any comments on this series or should we start queuing stuff? Well omap4 seems to work for me just fine, and omap3 in legacy mode. But looks like omap3 device tree based booting is breaking after I pulled in your test branch. I get the following on 3630 based dm3730. Uart4 breaks because of the issue with basic DT blobs. You need at least this patch: http://linux-kernel.2935.n7.nabble.com/RFC-PATCH-ARM-OMAP3630-Add-generic-machine-descriptor-td722791.html ... from Nishanth to get it to work right. Otherwise the clock init tries to use omap3430 clock data for omap3630 which leaves uart4 nodes out. OK thanks yes that solves it, I was missing the compatible flag for my not quite ready test boards (3730-evm and zoom3). I'll apply Nishant's fix with some changes. Then for this series, once you have the necessary acks, can you please split into following two separate immutable branches against v3.12-rc3: 1. clock code changes 2. dts changes Then those can be pulled in as needed by Benoit, Mike and I. And Paul too natuarlly if these patches conflict with stuff that's he's merging. And assuming Paul is OK with these patches in general. For the whole series, I think this quite nicely removes some nasty kernel data dependencies for omaps, so feel free to add: Acked-by: Tony Lindgren t...@atomide.com Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
Quoting Tero Kristo (2013-09-25 01:48:06) Hi all, Version 7 contains following high level changes: - Dropped support for basic bindings from Mike Turquette, instead using vendor specific bindings for all clocks - Mux clock + divider clock vendor specific bindings get rid of use of the bit-masks, instead these are generated runtime based on parent clock / divider min/max info, see individual patches for details - Added support for dummy_ck nodes, was missing from previous version - Fixed support for omap3630 - Added support for new clock node type: ti,mux-gate-clock (using composite clock type) - OMAP4 / OMAP5 only build fixes (compiler flag checks added to some files) - most of the of_omap_* calls renamed to of_ti_* - Rebased on top of v3.12-rc2 Hi Tero, I had one really minor request for one of the patches, otherwise I'm pretty happy with this series and I can take in the clock parts for 3.13. A few general comments: 1) I think I mentioned some time back that it would be wise to put a disclaimer at the top of each DT binding description stating something to the effect: Binding status: Unstable - ABI compatibility may be broken in the future. Are you OK merging these bindings as-is without that kind of a disclaimer? 2) I hope to see the clockdomain stuff go away in the future. I'm OK to take it for now to aid in this DT transition but I would like some commitment that it will not stay in drivers/clk/omap forever. I guess for clockdomain you'll need to figure out how to handle those in a generic driver way. 3) Same concern as above but for the DT clkdev alias stuff. I guess you'll need to make all of the dependent drivers play nice with DT. Do you plan to remove it within a reasonable timeframe? It would be a nice reduction in LoC and more importantly it would mean that drivers were using clkdev in a more-correct fashion. Regards, Mike Some detailed comments about patch level changes (if no mention, no major changes): - Patch #1: * removed use of reg-names property, instead registers mapped using compatible string * removed ti,modes property, instead uses DPLL mode flags now * separated AM33XX DPLLs under their own compatible strings - Patch #4 #5: new patches - Patch #6: merged basic gate support from Mike to this patch as vendor specific gate clock support - Patch #9 #10: new patches - Patch #11: dummy clocks added, USB / ABE DPLL setup ordering changed - Patch #14: dummy clocks added - Patch #20: dropped usage of reg-names property - Patch #21: dummy clocks added - Patch #22 #23: new patches - Patch #30: AM35XX clock data added to this patch - Patch #31: * dummy clocks added * added missing 3630 clocks Test branch available here: https://github.com/t-kristo/linux-pm.git branch: mainline-3.12-rc2-ti-dt-clks-v7 Testing done: - am335x-bone : boot only - omap5-sevm : boot only - dra7-evm : boot only - omap3-beagle : boot + suspend/resume (ret + off) - omap4-panda-es : boot + suspend/resume Nishanth has also been doing some tests with omap3-beagle-xm with some WIP versions of this branch, but not with this latest one. Nishanth, maybe you can provide test results to this one also? -Tero -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
* Tero Kristo t-kri...@ti.com [131004 08:42]: Hi, Just a gentle reminder, anybody have any comments on this series or should we start queuing stuff? Well omap4 seems to work for me just fine, and omap3 in legacy mode. But looks like omap3 device tree based booting is breaking after I pulled in your test branch. I get the following on 3630 based dm3730. Regards, Tony [0.187652] omap_hwmod: uart4: cannot clk_get main_clk uart4_fck [0.187713] omap_hwmod: uart4: cannot _init_clocks [0.187713] [ cut here ] [0.187774] WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2434 _init+0x6c/0x80() [0.187774] omap_hwmod: uart4: couldn't init clocks [0.187774] Modules linked in: [0.187805] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.12.0-rc2-00020-gdea1778-dirty #791 [0.187866] [c001cd54] (unwind_backtrace+0x0/0xf4) from [c0018e80] (show_stack+0x14/0x1c) [0.187896] [c0018e80] (show_stack+0x14/0x1c) from [c0554400] (dump_stack+0x6c/0x9c) [0.187927] [c0554400] (dump_stack+0x6c/0x9c) from [c00488a8] (warn_slowpath_common+0x64/0x84) [0.187957] [c00488a8] (warn_slowpath_common+0x64/0x84) from [c004895c] (warn_slowpath_fmt+0x30/0x40) [0.187988] [c004895c] (warn_slowpath_fmt+0x30/0x40) from [c07a4bec] (_init+0x6c/0x80) [0.188018] [c07a4bec] (_init+0x6c/0x80) from [c002e068] (omap_hwmod_for_each+0x50/0x64) [0.188049] [c002e068] (omap_hwmod_for_each+0x50/0x64) from [c07a5440] (__omap_hwmod_setup_all+0x34/0x4c) [0.188079] [c07a5440] (__omap_hwmod_setup_all+0x34/0x4c) from [c00087ac] (do_one_initcall+0x2c/0x150) [0.188110] [c00087ac] (do_one_initcall+0x2c/0x150) from [c0797508] (do_basic_setup+0x94/0xd0) [0.188140] [c0797508] (do_basic_setup+0x94/0xd0) from [c07975c0] (kernel_init_freeable+0x7c/0x120) [0.188171] [c07975c0] (kernel_init_freeable+0x7c/0x120) from [c05526b4] (kernel_init+0xc/0x164) [0.188201] [c05526b4] (kernel_init+0xc/0x164) from [c00149c8] (ret_from_fork+0x14/0x2c) [0.188415] ---[ end trace 1b75b31a2719ed1c ]--- [0.810241] [ cut here ] [0.810302] WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2126 _enable+0x254/0x280() [0.810302] omap_hwmod: timer12: enabled state can only be entered from initialized, idle, or disabled state [0.810333] Modules linked in: [0.810363] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW 3.12.0-rc2-00020-gdea1778-dirty #791 [0.810394] [c001cd54] (unwind_backtrace+0x0/0xf4) from [c0018e80] (show_stack+0x14/0x1c) [0.810424] [c0018e80] (show_stack+0x14/0x1c) from [c0554400] (dump_stack+0x6c/0x9c) [0.810455] [c0554400] (dump_stack+0x6c/0x9c) from [c00488a8] (warn_slowpath_common+0x64/0x84) [0.810485] [c00488a8] (warn_slowpath_common+0x64/0x84) from [c004895c] (warn_slowpath_fmt+0x30/0x40) [0.810516] [c004895c] (warn_slowpath_fmt+0x30/0x40) from [c003085c] (_enable+0x254/0x280) [0.810546] [c003085c] (_enable+0x254/0x280) from [c00308b0] (omap_hwmod_enable+0x28/0x40) [0.810577] [c00308b0] (omap_hwmod_enable+0x28/0x40) from [c0030f8c] (omap_device_enable+0x3c/0x80) [0.810607] [c0030f8c] (omap_device_enable+0x3c/0x80) from [c0030fe0] (_od_runtime_resume+0x10/0x1c) [0.810638] [c0030fe0] (_od_runtime_resume+0x10/0x1c) from [c036e120] (__rpm_callback+0x2c/0x80) [0.810668] [c036e120] (__rpm_callback+0x2c/0x80) from [c036e198] (rpm_callback+0x24/0x78) [0.810699] [c036e198] (rpm_callback+0x24/0x78) from [c036f444] (rpm_resume+0x3c0/0x60c) [0.810729] [c036f444] (rpm_resume+0x3c0/0x60c) from [c036f970] (__pm_runtime_resume+0x4c/0x68) [0.810760] [c036f970] (__pm_runtime_resume+0x4c/0x68) from [c0044954] (omap_dm_timer_probe+0x234/0x334) [0.810791] [c0044954] (omap_dm_timer_probe+0x234/0x334) from [c03659ac] (platform_drv_probe+0x1c/0x24) [0.810821] [c03659ac] (platform_drv_probe+0x1c/0x24) from [c03645e4] (really_probe+0x70/0x200) [0.810852] [c03645e4] (really_probe+0x70/0x200) from [c03647a8] (driver_probe_device+0x34/0x50) [0.810852] [c03647a8] (driver_probe_device+0x34/0x50) from [c0364858] (__driver_attach+0x94/0x98) [0.810882] [c0364858] (__driver_attach+0x94/0x98) from [c0362cd4] (bus_for_each_dev+0x74/0x98) [0.810913] [c0362cd4] (bus_for_each_dev+0x74/0x98) from [c0363510] (bus_add_driver+0xe8/0x278) [0.810943] [c0363510] (bus_add_driver+0xe8/0x278) from [c0364e30] (driver_register+0x5c/0xf8) [0.810974] [c0364e30] (driver_register+0x5c/0xf8) from [c00087ac] (do_one_initcall+0x2c/0x150) [0.811004] [c00087ac] (do_one_initcall+0x2c/0x150) from [c0797508] (do_basic_setup+0x94/0xd0) [0.811035] [c0797508] (do_basic_setup+0x94/0xd0) from [c07975c0] (kernel_init_freeable+0x7c/0x120) [0.811065] [c07975c0] (kernel_init_freeable+0x7c/0x120) from [c05526b4] (kernel_init+0xc/0x164) [0.811096] [c05526b4] (kernel_init+0xc/0x164) from [c00149c8] (ret_from_fork+0x14/0x2c) [0.811126] ---[ end
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
Hi, Just a gentle reminder, anybody have any comments on this series or should we start queuing stuff? -Tero On 09/25/2013 11:48 AM, Tero Kristo wrote: Hi all, Version 7 contains following high level changes: - Dropped support for basic bindings from Mike Turquette, instead using vendor specific bindings for all clocks - Mux clock + divider clock vendor specific bindings get rid of use of the bit-masks, instead these are generated runtime based on parent clock / divider min/max info, see individual patches for details - Added support for dummy_ck nodes, was missing from previous version - Fixed support for omap3630 - Added support for new clock node type: ti,mux-gate-clock (using composite clock type) - OMAP4 / OMAP5 only build fixes (compiler flag checks added to some files) - most of the of_omap_* calls renamed to of_ti_* - Rebased on top of v3.12-rc2 Some detailed comments about patch level changes (if no mention, no major changes): - Patch #1: * removed use of reg-names property, instead registers mapped using compatible string * removed ti,modes property, instead uses DPLL mode flags now * separated AM33XX DPLLs under their own compatible strings - Patch #4 #5: new patches - Patch #6: merged basic gate support from Mike to this patch as vendor specific gate clock support - Patch #9 #10: new patches - Patch #11: dummy clocks added, USB / ABE DPLL setup ordering changed - Patch #14: dummy clocks added - Patch #20: dropped usage of reg-names property - Patch #21: dummy clocks added - Patch #22 #23: new patches - Patch #30: AM35XX clock data added to this patch - Patch #31: * dummy clocks added * added missing 3630 clocks Test branch available here: https://github.com/t-kristo/linux-pm.git branch: mainline-3.12-rc2-ti-dt-clks-v7 Testing done: - am335x-bone : boot only - omap5-sevm : boot only - dra7-evm : boot only - omap3-beagle : boot + suspend/resume (ret + off) - omap4-panda-es : boot + suspend/resume Nishanth has also been doing some tests with omap3-beagle-xm with some WIP versions of this branch, but not with this latest one. Nishanth, maybe you can provide test results to this one also? -Tero -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On 09/25/2013 03:48 AM, Tero Kristo wrote: [...] Test branch available here: https://github.com/t-kristo/linux-pm.git branch: mainline-3.12-rc2-ti-dt-clks-v7 Testing done: - am335x-bone : boot only - omap5-sevm : boot only - dra7-evm : boot only - omap3-beagle : boot + suspend/resume (ret + off) - omap4-panda-es : boot + suspend/resume Nishanth has also been doing some tests with omap3-beagle-xm with some WIP versions of this branch, but not with this latest one. Nishanth, maybe you can provide test results to this one also? I rebased the branch on top of benoit's for_3.13/dts merged with v3.12-rc2 tag 45646cd ARM: dts: AM33XX: don't redefine OCP bus and device nodes All platforms were to MMC filesystem boot (SD card). u-boot used v2013.10-rc3 on all platforms (mainline u-boot). OMAP3630: Beagle-XM: http://pastebin.pandaboard.org/index.php/view/96577127 (needs additional patch - https://patchwork.kernel.org/patch/2919661/ ) OMAP4460: Panda-ES: http://pastebin.pandaboard.org/index.php/view/85143661 OMAP5432: OMAP5-UEVM: http://pastebin.pandaboard.org/index.php/view/91361843 (nice to have patch - https://patchwork.kernel.org/patch/2907761/ ) AM335x: BeagleBone Black: http://pastebin.pandaboard.org/index.php/view/42000597 (needs additional patches: https://patchwork.kernel.org/patch/2902711/ https://patchwork.kernel.org/patch/2902711/ ) DRA7: DRA7-EVM: http://pastebin.pandaboard.org/index.php/view/38362805 (needs additional patches: - https://patchwork.kernel.org/patch/2849391/ - https://patchwork.kernel.org/patch/2849601/ - https://patchwork.kernel.org/patch/2849602/ - https://patchwork.kernel.org/patch/2907761/ ) So, for the entire series: Tested-by: Nishanth Menon n...@ti.com -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 00/36] ARM: OMAP: clock data conversion to DT
On 09/26/2013 10:21 AM, Nishanth Menon wrote: On 09/25/2013 03:48 AM, Tero Kristo wrote: [...] Test branch available here: https://github.com/t-kristo/linux-pm.git branch: mainline-3.12-rc2-ti-dt-clks-v7 Testing done: - am335x-bone : boot only - omap5-sevm : boot only - dra7-evm : boot only - omap3-beagle : boot + suspend/resume (ret + off) - omap4-panda-es : boot + suspend/resume Nishanth has also been doing some tests with omap3-beagle-xm with some WIP versions of this branch, but not with this latest one. Nishanth, maybe you can provide test results to this one also? I rebased the branch on top of benoit's for_3.13/dts merged with v3.12-rc2 tag 45646cd ARM: dts: AM33XX: don't redefine OCP bus and device nodes All platforms were to MMC filesystem boot (SD card). u-boot used v2013.10-rc3 on all platforms (mainline u-boot). OMAP3630: Beagle-XM: http://pastebin.pandaboard.org/index.php/view/96577127 (needs additional patch - https://patchwork.kernel.org/patch/2919661/ ) dependency patch needs rework OMAP4460: Panda-ES: http://pastebin.pandaboard.org/index.php/view/85143661 OMAP5432: OMAP5-UEVM: http://pastebin.pandaboard.org/index.php/view/91361843 (nice to have patch - https://patchwork.kernel.org/patch/2907761/ ) Acked,tested, pending merge from maintainers AM335x: BeagleBone Black: http://pastebin.pandaboard.org/index.php/view/42000597 (needs additional patches: https://patchwork.kernel.org/patch/2902711/ - merged to Shekar's davinci branch - pending upstream. https://patchwork.kernel.org/patch/2902711/ Correction on ^^: should have been: https://patchwork.kernel.org/patch/2919661/ tested, pending ack, merge. ) DRA7: DRA7-EVM: http://pastebin.pandaboard.org/index.php/view/38362805 (needs additional patches: - https://patchwork.kernel.org/patch/2849391/ - https://patchwork.kernel.org/patch/2849601/ - https://patchwork.kernel.org/patch/2849602/ - https://patchwork.kernel.org/patch/2907761/ All of the above: Acked,tested, pending merge from maintainers ) So, for the entire series: Tested-by: Nishanth Menon n...@ti.com Maybe Benoit and Tony could help reduce few of these acked patches pending merge from having to be cherry-picked by folks. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
