Add a flag that indicate whether the clock is a crystal or not. Since
no clocks set this flag right now, include an additional flag that
indicates whether the type is set or not. If the CLK_IS_TYPE_DEFINED
flag is not set, the value of the CLK_IS_TYPE_XTAL flag is undefined.
This ensures backwards compatibility.
Additionally, parse a new device tree binding in clk-fixed-rate to set
this flag.
Signed-off-by: Luciano Coelho <[email protected]>
---
I'm not familiar with the common clock framework and I'm not
entirely sure the flags can be used in such a way, but to me it looks
reasonable, since some clock consumers may need to know what type of
clock is being provided.
Specifically, the wl12xx firmware needs to know if the clock is XTAL
or not to handle the stabilization and boosts properly.
My main idea is that I need to pass this information in the device
tree definition of the clocks, so that the driver can pass this
information on to the firmware.
Please let me know if this looks ok or not. If not, please let me
know if you have any other ideas on how to solve my problem (of
knowing whether the clock attached to the WiLink chip is XTAL or not).
drivers/clk/clk-fixed-rate.c | 6 +++++-
include/linux/clk-provider.h | 2 ++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index dc58fbd..4003a82 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -90,13 +90,17 @@ void of_fixed_clk_setup(struct device_node *node)
struct clk *clk;
const char *clk_name = node->name;
u32 rate;
+ unsigned long flags = CLK_IS_ROOT;
if (of_property_read_u32(node, "clock-frequency", &rate))
return;
+ if (of_property_read_bool(node, "clock-xtal"))
+ flags |= CLK_IS_TYPE_DEFINED | CLK_IS_TYPE_XTAL;
+
of_property_read_string(node, "clock-output-names", &clk_name);
- clk = clk_register_fixed_rate(NULL, clk_name, NULL, CLK_IS_ROOT, rate);
+ clk = clk_register_fixed_rate(NULL, clk_name, NULL, flags, rate);
if (!IS_ERR(clk))
of_clk_add_provider(node, of_clk_src_simple_get, clk);
}
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 1186098..034320b 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -27,6 +27,8 @@
#define CLK_IS_ROOT BIT(4) /* root clk, has no parent */
#define CLK_IS_BASIC BIT(5) /* Basic clk, can't do a to_clk_foo() */
#define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
+#define CLK_IS_TYPE_DEFINED BIT(7) /* the clock type is defined */
+#define CLK_IS_TYPE_XTAL BIT(8) /* this is a crystal clock */
struct clk_hw;
--
1.7.10.4
On Thu, 2013-07-04 at 15:25 -0700, Mike Turquette wrote:
> Quoting Luciano Coelho (2013-07-04 14:05:12)
> > Add a flag that indicate whether the clock is a crystal or not. Since
> > no clocks set this flag right now, include an additional flag that
> > indicates whether the type is set or not. If the CLK_IS_TYPE_DEFINED
> > flag is not set, the value of the CLK_IS_TYPE_XTAL flag is undefined.
> > This ensures backwards compatibility.
> >
> > Additionally, parse a new device tree binding in clk-fixed-rate to set
> > this flag.
> >
> > Signed-off-by: Luciano Coelho <[email protected]>
> > ---
> >
> > I'm not familiar with the common clock framework and I'm not
> > entirely sure the flags can be used in such a way, but to me it looks
> > reasonable, since some clock consumers may need to know what type of
> > clock is being provided.
> >
> > Specifically, the wl12xx firmware needs to know if the clock is XTAL
> > or not to handle the stabilization and boosts properly.
>
> Hi Luciano,
Hi Mike,
> I'd like clarification on one point. Is the same clock input signal used
> on the wifi chip? What I mean is, are there multiple clock inputs and
> XTAL is one, and not-XTAL is another?
This wifi chip can work with a few different clocks and some of them are
XTAL and others are not. What the chip's firmware can use is one of
these:
19.2MHz (not XTAL)
26.0MHz (not XTAL)
26.0MHz (XTAL)
38.4MHz (not XTAL)
38.4MHz (XTAL)
52.0MHz (not XTAL)
It treats the XTAL clocks differently (but I don't really understand
enough of the details), so the driver needs to configure the firmware
according to these values.
> Or is it the same clock input and basically the problem is that you need
> to know what kind of waveform to expect (e.g. square versus sine)?
It's the same clock input in the chip's perspective. One clock input
that can be any of the combinations I mentioned above. Again, I'm not
familiar with clocks, so I guess the square vs. sine explanation is
plausible. What I could see in the firmware is that it handles the
clocks differently if they're xtal or not.
--
Luca.
On Thu, 2013-07-04 at 16:19 -0700, Mike Turquette wrote:
> Quoting Luciano Coelho (2013-07-04 15:37:45)
> > On Thu, 2013-07-04 at 15:25 -0700, Mike Turquette wrote:
> > > Or is it the same clock input and basically the problem is that you need
> > > to know what kind of waveform to expect (e.g. square versus sine)?
> >
> > It's the same clock input in the chip's perspective. One clock input
> > that can be any of the combinations I mentioned above. Again, I'm not
> > familiar with clocks, so I guess the square vs. sine explanation is
> > plausible. What I could see in the firmware is that it handles the
> > clocks differently if they're xtal or not.
>
> OMAP has a similar thing where sys_clkin (the fast reference clock for
> the chip) can be 19.2, 26, 38.4, etc. This is easy to handle since only
> the rates matter.
Right, this part is easy and I already have the code for that. What I'm
missing is a way to pass this XTAL flag to the chip.
> In your case you need some extra metadata to know what to do. I'm really
> not sure if CLK_IS_TYPE_XTAL is the most useful form this metadata can
> take. It would be best to know if the waveform is what you really need
> to know, or perhaps something else. For instance you might be affected
> by some clock signal stabilization time. Can you talk to your hardware
> guys and figure it out? I'd rather model the actual needs instead of
> just tossing a flag in there.
I get your point. I have tried to investigate how this flag is used by
the firmware and I could see that it is used to set different "buffer
gains" and "delays" when waking up (I guess this means when the clock is
starting, so probably related to stabilization time). They specify two
"modes", "boost" and "normal" and use different delay values for each.
As I said, I don't know almost anything about clocks, so all this
doesn't make much sense to me. But maybe you can get an idea?
--
Cheers,
Luca.
On 4 July 2013 22:05, Luciano Coelho <[email protected]> wrote:
> Add a flag that indicate whether the clock is a crystal or not. Since
> no clocks set this flag right now, include an additional flag that
> indicates whether the type is set or not. If the CLK_IS_TYPE_DEFINED
> flag is not set, the value of the CLK_IS_TYPE_XTAL flag is undefined.
> This ensures backwards compatibility.
>
> Additionally, parse a new device tree binding in clk-fixed-rate to set
> this flag.
>
> Signed-off-by: Luciano Coelho <[email protected]>
> ---
>
> I'm not familiar with the common clock framework and I'm not
> entirely sure the flags can be used in such a way, but to me it looks
> reasonable, since some clock consumers may need to know what type of
> clock is being provided.
>
> Specifically, the wl12xx firmware needs to know if the clock is XTAL
> or not to handle the stabilization and boosts properly.
>
> My main idea is that I need to pass this information in the device
> tree definition of the clocks, so that the driver can pass this
> information on to the firmware.
>
> Please let me know if this looks ok or not. If not, please let me
> know if you have any other ideas on how to solve my problem (of
> knowing whether the clock attached to the WiLink chip is XTAL or not).
The TZ1090 SoC has something that sounds possibly similar, where some
of the XTAL pads have a bypass bit, which according to the hardware
engineer I asked should be enabled when you want to use the
corresponding XTAL pads as a clock input pad rather than an
oscillator. I was considering extending clk-fixed-rate (via a wrapper
driver) to parse a custom DT property and a register address / bit
number and set the bypass bit as appropriate itself.
So I was wondering, is there a particular reason you don't have a DT
property on the node for the device that needs to know what type of
clock it is, rather than the clock node itself? That way you're not
depending directly on the generic common clock framework to be able to
tell you such electrical details.
Cheers
James
On Fri, Jul 05, 2013 at 02:12:08PM +0100, James Hogan wrote:
> On 4 July 2013 22:05, Luciano Coelho <[email protected]> wrote:
> > Add a flag that indicate whether the clock is a crystal or not. Since
> > no clocks set this flag right now, include an additional flag that
> > indicates whether the type is set or not. If the CLK_IS_TYPE_DEFINED
> > flag is not set, the value of the CLK_IS_TYPE_XTAL flag is undefined.
> > This ensures backwards compatibility.
> >
> > Additionally, parse a new device tree binding in clk-fixed-rate to set
> > this flag.
> >
> > Signed-off-by: Luciano Coelho <[email protected]>
> > ---
> >
> > I'm not familiar with the common clock framework and I'm not
> > entirely sure the flags can be used in such a way, but to me it looks
> > reasonable, since some clock consumers may need to know what type of
> > clock is being provided.
> >
> > Specifically, the wl12xx firmware needs to know if the clock is XTAL
> > or not to handle the stabilization and boosts properly.
> >
> > My main idea is that I need to pass this information in the device
> > tree definition of the clocks, so that the driver can pass this
> > information on to the firmware.
> >
> > Please let me know if this looks ok or not. If not, please let me
> > know if you have any other ideas on how to solve my problem (of
> > knowing whether the clock attached to the WiLink chip is XTAL or not).
>
> The TZ1090 SoC has something that sounds possibly similar, where some
> of the XTAL pads have a bypass bit, which according to the hardware
> engineer I asked should be enabled when you want to use the
> corresponding XTAL pads as a clock input pad rather than an
> oscillator. I was considering extending clk-fixed-rate (via a wrapper
> driver) to parse a custom DT property and a register address / bit
> number and set the bypass bit as appropriate itself.
>
> So I was wondering, is there a particular reason you don't have a DT
> property on the node for the device that needs to know what type of
> clock it is, rather than the clock node itself? That way you're not
> depending directly on the generic common clock framework to be able to
> tell you such electrical details.
three things here:
1) you end up with several devices implementing the clock type
attribute.
2) the type is a property of the clock itself
3) At least WiLink, can work with both types of clocks.
considering those, I really think this should belong to the clock node.
Otherwise Imagine how your DT would look like:
clock {
compatible = "fixed-rate";
...
};
wilink {
clocks = &clock;
wilink,btw-this-time-we-are-using-xtal;
...
};
where you could:
clock {
compatible = "fixed-rate";
clock-type = "xtal";
...
};
wilink {
clocks = &clock;
...
};
second option looks a lot cleaner to me.
--
balbi
On Fri, 2013-07-05 at 14:12 +0100, James Hogan wrote:
> On 4 July 2013 22:05, Luciano Coelho <[email protected]> wrote:
> > Add a flag that indicate whether the clock is a crystal or not. Since
> > no clocks set this flag right now, include an additional flag that
> > indicates whether the type is set or not. If the CLK_IS_TYPE_DEFINED
> > flag is not set, the value of the CLK_IS_TYPE_XTAL flag is undefined.
> > This ensures backwards compatibility.
> >
> > Additionally, parse a new device tree binding in clk-fixed-rate to set
> > this flag.
> >
> > Signed-off-by: Luciano Coelho <[email protected]>
> > ---
> >
> > I'm not familiar with the common clock framework and I'm not
> > entirely sure the flags can be used in such a way, but to me it looks
> > reasonable, since some clock consumers may need to know what type of
> > clock is being provided.
> >
> > Specifically, the wl12xx firmware needs to know if the clock is XTAL
> > or not to handle the stabilization and boosts properly.
> >
> > My main idea is that I need to pass this information in the device
> > tree definition of the clocks, so that the driver can pass this
> > information on to the firmware.
> >
> > Please let me know if this looks ok or not. If not, please let me
> > know if you have any other ideas on how to solve my problem (of
> > knowing whether the clock attached to the WiLink chip is XTAL or not).
>
> The TZ1090 SoC has something that sounds possibly similar, where some
> of the XTAL pads have a bypass bit, which according to the hardware
> engineer I asked should be enabled when you want to use the
> corresponding XTAL pads as a clock input pad rather than an
> oscillator.
Cool, good to know that I'm not alone here. ;)
> I was considering extending clk-fixed-rate (via a wrapper
> driver) to parse a custom DT property and a register address / bit
> number and set the bypass bit as appropriate itself.
I thought about this too. I actually have a "driver" for my clocks,
because normally they're not part of the main board, but part of the
module that contains the WiLink chip. In those cases, I don't want my
clocks to be used as a generic clk-fixed-rate by the clock framework.
But the only difference in my "wrapper" is that it matches
"ti,wilink-clock" instead of "fixed-clock".
> So I was wondering, is there a particular reason you don't have a DT
> property on the node for the device that needs to know what type of
> clock it is, rather than the clock node itself? That way you're not
> depending directly on the generic common clock framework to be able to
> tell you such electrical details.
I think this is a detail of the clock itself, not of the user of the
clock. Of course, the user of the clock needs to know what to do if the
clock is XTAL. But the information of whether it is a XTAL or not
should be in the clock data.
I tried to make a generic solution that everyone could use. For
instance, you. :) You could use the same bit that I implemented and, in
your driver, convert that bit into the action you need to take.
--
Luca.
Hi Mike,
On Fri, 2013-07-05 at 10:54 +0300, Luciano Coelho wrote:
> On Thu, 2013-07-04 at 16:19 -0700, Mike Turquette wrote:
> > Quoting Luciano Coelho (2013-07-04 15:37:45)
> > > On Thu, 2013-07-04 at 15:25 -0700, Mike Turquette wrote:
> > > > Or is it the same clock input and basically the problem is that you need
> > > > to know what kind of waveform to expect (e.g. square versus sine)?
> > >
> > > It's the same clock input in the chip's perspective. One clock input
> > > that can be any of the combinations I mentioned above. Again, I'm not
> > > familiar with clocks, so I guess the square vs. sine explanation is
> > > plausible. What I could see in the firmware is that it handles the
> > > clocks differently if they're xtal or not.
> >
> > OMAP has a similar thing where sys_clkin (the fast reference clock for
> > the chip) can be 19.2, 26, 38.4, etc. This is easy to handle since only
> > the rates matter.
>
> Right, this part is easy and I already have the code for that. What I'm
> missing is a way to pass this XTAL flag to the chip.
>
>
> > In your case you need some extra metadata to know what to do. I'm really
> > not sure if CLK_IS_TYPE_XTAL is the most useful form this metadata can
> > take. It would be best to know if the waveform is what you really need
> > to know, or perhaps something else. For instance you might be affected
> > by some clock signal stabilization time. Can you talk to your hardware
> > guys and figure it out? I'd rather model the actual needs instead of
> > just tossing a flag in there.
>
> I get your point. I have tried to investigate how this flag is used by
> the firmware and I could see that it is used to set different "buffer
> gains" and "delays" when waking up (I guess this means when the clock is
> starting, so probably related to stabilization time). They specify two
> "modes", "boost" and "normal" and use different delay values for each.
I tried but I couldn't find any more information on how exactly this
works. But since this change is really simple and there seems to be
other people who need the same information, couldn't we add it as is and
try to figure out more specific information about the clocks later on?
Even if XTAL is not that useful if we know the other details, at least
it wouldn't hurt to have the flag there anyway.
--
Cheers,
Luca.
Hi,
Fixing Luca's address since he left TI
On Mon, Oct 07, 2013 at 12:44:24AM -0700, Mike Turquette wrote:
> Quoting Luciano Coelho (2013-07-29 06:50:42)
> > Hi Mike,
> >
> > On Fri, 2013-07-05 at 10:54 +0300, Luciano Coelho wrote:
> > > On Thu, 2013-07-04 at 16:19 -0700, Mike Turquette wrote:
> > > > Quoting Luciano Coelho (2013-07-04 15:37:45)
> > > > > On Thu, 2013-07-04 at 15:25 -0700, Mike Turquette wrote:
> > > > > > Or is it the same clock input and basically the problem is that you need
> > > > > > to know what kind of waveform to expect (e.g. square versus sine)?
> > > > >
> > > > > It's the same clock input in the chip's perspective. One clock input
> > > > > that can be any of the combinations I mentioned above. Again, I'm not
> > > > > familiar with clocks, so I guess the square vs. sine explanation is
> > > > > plausible. What I could see in the firmware is that it handles the
> > > > > clocks differently if they're xtal or not.
> > > >
> > > > OMAP has a similar thing where sys_clkin (the fast reference clock for
> > > > the chip) can be 19.2, 26, 38.4, etc. This is easy to handle since only
> > > > the rates matter.
> > >
> > > Right, this part is easy and I already have the code for that. What I'm
> > > missing is a way to pass this XTAL flag to the chip.
> > >
> > >
> > > > In your case you need some extra metadata to know what to do. I'm really
> > > > not sure if CLK_IS_TYPE_XTAL is the most useful form this metadata can
> > > > take. It would be best to know if the waveform is what you really need
> > > > to know, or perhaps something else. For instance you might be affected
> > > > by some clock signal stabilization time. Can you talk to your hardware
> > > > guys and figure it out? I'd rather model the actual needs instead of
> > > > just tossing a flag in there.
> > >
> > > I get your point. I have tried to investigate how this flag is used by
> > > the firmware and I could see that it is used to set different "buffer
> > > gains" and "delays" when waking up (I guess this means when the clock is
> > > starting, so probably related to stabilization time). They specify two
> > > "modes", "boost" and "normal" and use different delay values for each.
> >
> > I tried but I couldn't find any more information on how exactly this
> > works. But since this change is really simple and there seems to be
> > other people who need the same information, couldn't we add it as is and
> > try to figure out more specific information about the clocks later on?
> >
> > Even if XTAL is not that useful if we know the other details, at least
> > it wouldn't hurt to have the flag there anyway.
>
> Luca,
>
> By any chance did you come to a different solution for this problem? I
> can take the patch, but I do not feel like we're solving the right
> problem the right way.
>
> If not I will take it for 3.13.
>
> Regards,
> Mike
>
> >
> > --
> > Cheers,
> > Luca.
--
balbi
Hi,
Sorry for the delayed response.
On Tue, 2013-10-08 at 10:27 -0500, Felipe Balbi wrote:
> Fixing Luca's address since he left TI
Thanks, Felipe! I wouldn't have seen this otherwise.
> On Mon, Oct 07, 2013 at 12:44:24AM -0700, Mike Turquette wrote:
> > Quoting Luciano Coelho (2013-07-29 06:50:42)
> > > Hi Mike,
> > >
> > > On Fri, 2013-07-05 at 10:54 +0300, Luciano Coelho wrote:
> > > > On Thu, 2013-07-04 at 16:19 -0700, Mike Turquette wrote:
> > > > > Quoting Luciano Coelho (2013-07-04 15:37:45)
> > > > > > On Thu, 2013-07-04 at 15:25 -0700, Mike Turquette wrote:
> > > > > > > Or is it the same clock input and basically the problem is that you need
> > > > > > > to know what kind of waveform to expect (e.g. square versus sine)?
> > > > > >
> > > > > > It's the same clock input in the chip's perspective. One clock input
> > > > > > that can be any of the combinations I mentioned above. Again, I'm not
> > > > > > familiar with clocks, so I guess the square vs. sine explanation is
> > > > > > plausible. What I could see in the firmware is that it handles the
> > > > > > clocks differently if they're xtal or not.
> > > > >
> > > > > OMAP has a similar thing where sys_clkin (the fast reference clock for
> > > > > the chip) can be 19.2, 26, 38.4, etc. This is easy to handle since only
> > > > > the rates matter.
> > > >
> > > > Right, this part is easy and I already have the code for that. What I'm
> > > > missing is a way to pass this XTAL flag to the chip.
> > > >
> > > >
> > > > > In your case you need some extra metadata to know what to do. I'm really
> > > > > not sure if CLK_IS_TYPE_XTAL is the most useful form this metadata can
> > > > > take. It would be best to know if the waveform is what you really need
> > > > > to know, or perhaps something else. For instance you might be affected
> > > > > by some clock signal stabilization time. Can you talk to your hardware
> > > > > guys and figure it out? I'd rather model the actual needs instead of
> > > > > just tossing a flag in there.
> > > >
> > > > I get your point. I have tried to investigate how this flag is used by
> > > > the firmware and I could see that it is used to set different "buffer
> > > > gains" and "delays" when waking up (I guess this means when the clock is
> > > > starting, so probably related to stabilization time). They specify two
> > > > "modes", "boost" and "normal" and use different delay values for each.
> > >
> > > I tried but I couldn't find any more information on how exactly this
> > > works. But since this change is really simple and there seems to be
> > > other people who need the same information, couldn't we add it as is and
> > > try to figure out more specific information about the clocks later on?
> > >
> > > Even if XTAL is not that useful if we know the other details, at least
> > > it wouldn't hurt to have the flag there anyway.
> >
> > Luca,
> >
> > By any chance did you come to a different solution for this problem? I
> > can take the patch, but I do not feel like we're solving the right
> > problem the right way.
Unfortunately I didn't find any better solution for this. From the
WiLink firmware's point of view, there was not much information about
the details of stabilization time requirements and such.
> > If not I will take it for 3.13.
That would be great! As I mentioned above, the XTAL/non-XTAL flag
wouldn't hurt, even if in most cases it may not provide the necessary
details for the clock code. But at least for the WiLink hardware it is
very useful. ;)
--
Cheers,
Luca.
Quoting Luca Coelho (2013-10-16 03:24:27)
> Hi,
>
> Sorry for the delayed response.
>
> On Tue, 2013-10-08 at 10:27 -0500, Felipe Balbi wrote:
> > Fixing Luca's address since he left TI
>
> Thanks, Felipe! I wouldn't have seen this otherwise.
>
>
> > On Mon, Oct 07, 2013 at 12:44:24AM -0700, Mike Turquette wrote:
> > > Quoting Luciano Coelho (2013-07-29 06:50:42)
> > > > Hi Mike,
> > > >
> > > > On Fri, 2013-07-05 at 10:54 +0300, Luciano Coelho wrote:
> > > > > On Thu, 2013-07-04 at 16:19 -0700, Mike Turquette wrote:
> > > > > > Quoting Luciano Coelho (2013-07-04 15:37:45)
> > > > > > > On Thu, 2013-07-04 at 15:25 -0700, Mike Turquette wrote:
> > > > > > > > Or is it the same clock input and basically the problem is that you need
> > > > > > > > to know what kind of waveform to expect (e.g. square versus sine)?
> > > > > > >
> > > > > > > It's the same clock input in the chip's perspective. One clock input
> > > > > > > that can be any of the combinations I mentioned above. Again, I'm not
> > > > > > > familiar with clocks, so I guess the square vs. sine explanation is
> > > > > > > plausible. What I could see in the firmware is that it handles the
> > > > > > > clocks differently if they're xtal or not.
> > > > > >
> > > > > > OMAP has a similar thing where sys_clkin (the fast reference clock for
> > > > > > the chip) can be 19.2, 26, 38.4, etc. This is easy to handle since only
> > > > > > the rates matter.
> > > > >
> > > > > Right, this part is easy and I already have the code for that. What I'm
> > > > > missing is a way to pass this XTAL flag to the chip.
> > > > >
> > > > >
> > > > > > In your case you need some extra metadata to know what to do. I'm really
> > > > > > not sure if CLK_IS_TYPE_XTAL is the most useful form this metadata can
> > > > > > take. It would be best to know if the waveform is what you really need
> > > > > > to know, or perhaps something else. For instance you might be affected
> > > > > > by some clock signal stabilization time. Can you talk to your hardware
> > > > > > guys and figure it out? I'd rather model the actual needs instead of
> > > > > > just tossing a flag in there.
> > > > >
> > > > > I get your point. I have tried to investigate how this flag is used by
> > > > > the firmware and I could see that it is used to set different "buffer
> > > > > gains" and "delays" when waking up (I guess this means when the clock is
> > > > > starting, so probably related to stabilization time). They specify two
> > > > > "modes", "boost" and "normal" and use different delay values for each.
> > > >
> > > > I tried but I couldn't find any more information on how exactly this
> > > > works. But since this change is really simple and there seems to be
> > > > other people who need the same information, couldn't we add it as is and
> > > > try to figure out more specific information about the clocks later on?
> > > >
> > > > Even if XTAL is not that useful if we know the other details, at least
> > > > it wouldn't hurt to have the flag there anyway.
> > >
> > > Luca,
> > >
> > > By any chance did you come to a different solution for this problem? I
> > > can take the patch, but I do not feel like we're solving the right
> > > problem the right way.
>
> Unfortunately I didn't find any better solution for this. From the
> WiLink firmware's point of view, there was not much information about
> the details of stabilization time requirements and such.
>
>
> > > If not I will take it for 3.13.
>
> That would be great! As I mentioned above, the XTAL/non-XTAL flag
> wouldn't hurt, even if in most cases it may not provide the necessary
> details for the clock code. But at least for the WiLink hardware it is
> very useful. ;)
Why is the CLK_IS_TYPE_DEFINED necessary?
Also we're adding a property to the fixed-rate binding so
Documentation/devicetree/bindings/clock/fixed-clock.txt needs to be
updated.
Regards,
Mike
>
> --
> Cheers,
> Luca.
On Wed, 2013-10-23 at 02:24 -0700, Mike Turquette wrote:
> Quoting Luca Coelho (2013-10-16 03:24:27)
> > Hi,
> >
> > Sorry for the delayed response.
> >
> > On Tue, 2013-10-08 at 10:27 -0500, Felipe Balbi wrote:
> > > Fixing Luca's address since he left TI
> >
> > Thanks, Felipe! I wouldn't have seen this otherwise.
> >
> >
> > > On Mon, Oct 07, 2013 at 12:44:24AM -0700, Mike Turquette wrote:
> > > > Quoting Luciano Coelho (2013-07-29 06:50:42)
> > > > > Hi Mike,
> > > > >
> > > > > On Fri, 2013-07-05 at 10:54 +0300, Luciano Coelho wrote:
> > > > > > On Thu, 2013-07-04 at 16:19 -0700, Mike Turquette wrote:
> > > > > > > Quoting Luciano Coelho (2013-07-04 15:37:45)
> > > > > > > > On Thu, 2013-07-04 at 15:25 -0700, Mike Turquette wrote:
> > > > > > > > > Or is it the same clock input and basically the problem is that you need
> > > > > > > > > to know what kind of waveform to expect (e.g. square versus sine)?
> > > > > > > >
> > > > > > > > It's the same clock input in the chip's perspective. One clock input
> > > > > > > > that can be any of the combinations I mentioned above. Again, I'm not
> > > > > > > > familiar with clocks, so I guess the square vs. sine explanation is
> > > > > > > > plausible. What I could see in the firmware is that it handles the
> > > > > > > > clocks differently if they're xtal or not.
> > > > > > >
> > > > > > > OMAP has a similar thing where sys_clkin (the fast reference clock for
> > > > > > > the chip) can be 19.2, 26, 38.4, etc. This is easy to handle since only
> > > > > > > the rates matter.
> > > > > >
> > > > > > Right, this part is easy and I already have the code for that. What I'm
> > > > > > missing is a way to pass this XTAL flag to the chip.
> > > > > >
> > > > > >
> > > > > > > In your case you need some extra metadata to know what to do. I'm really
> > > > > > > not sure if CLK_IS_TYPE_XTAL is the most useful form this metadata can
> > > > > > > take. It would be best to know if the waveform is what you really need
> > > > > > > to know, or perhaps something else. For instance you might be affected
> > > > > > > by some clock signal stabilization time. Can you talk to your hardware
> > > > > > > guys and figure it out? I'd rather model the actual needs instead of
> > > > > > > just tossing a flag in there.
> > > > > >
> > > > > > I get your point. I have tried to investigate how this flag is used by
> > > > > > the firmware and I could see that it is used to set different "buffer
> > > > > > gains" and "delays" when waking up (I guess this means when the clock is
> > > > > > starting, so probably related to stabilization time). They specify two
> > > > > > "modes", "boost" and "normal" and use different delay values for each.
> > > > >
> > > > > I tried but I couldn't find any more information on how exactly this
> > > > > works. But since this change is really simple and there seems to be
> > > > > other people who need the same information, couldn't we add it as is and
> > > > > try to figure out more specific information about the clocks later on?
> > > > >
> > > > > Even if XTAL is not that useful if we know the other details, at least
> > > > > it wouldn't hurt to have the flag there anyway.
> > > >
> > > > Luca,
> > > >
> > > > By any chance did you come to a different solution for this problem? I
> > > > can take the patch, but I do not feel like we're solving the right
> > > > problem the right way.
> >
> > Unfortunately I didn't find any better solution for this. From the
> > WiLink firmware's point of view, there was not much information about
> > the details of stabilization time requirements and such.
> >
> >
> > > > If not I will take it for 3.13.
> >
> > That would be great! As I mentioned above, the XTAL/non-XTAL flag
> > wouldn't hurt, even if in most cases it may not provide the necessary
> > details for the clock code. But at least for the WiLink hardware it is
> > very useful. ;)
>
> Why is the CLK_IS_TYPE_DEFINED necessary?
This is just for backwards compatibility. Currently all the other
clocks don't set the XTAL/non-XTAL flag (even if they *are* XTAL), so if
the CLK_IS_TYPE_DEFINED is not set, we know we can't trust the
XTAL/no-XTAL flag.
> Also we're adding a property to the fixed-rate binding so
> Documentation/devicetree/bindings/clock/fixed-clock.txt needs to be
> updated.
Yeah, you're right. Unfortunately, I won't have the time to resubmit
this patch any time soon. :(
--
Luca.