Comments

On 02/11/2013 04:04 AM, Peter De Schrijver wrote:
> Add references to tegra_car clocks for the basic device nodes.
In this patch, you also need to remove the "clock-frequency" properties
from tegra114-*.dts board files.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

On Tue, Feb 12, 2013 at 06:24:05PM +0100, Stephen Warren wrote:
> On 02/11/2013 04:04 AM, Peter De Schrijver wrote:> > Add references to tegra_car clocks for the basic device nodes.> > In this patch, you also need to remove the "clock-frequency" properties> from tegra114-*.dts board files.
That would probably break bisectability no? I think this needs to be done
in a separate patch. Also, the UARTD enabling in init_table[] can then be
removed.
Cheers,
Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

On 02/13/2013 04:38 AM, Peter De Schrijver wrote:
> On Tue, Feb 12, 2013 at 06:24:05PM +0100, Stephen Warren wrote:>> On 02/11/2013 04:04 AM, Peter De Schrijver wrote:>>> Add references to tegra_car clocks for the basic device nodes.>>>> In this patch, you also need to remove the "clock-frequency" properties>> from tegra114-*.dts board files.> > That would probably break bisectability no?
Oh right, I read the patch order backwards. You can fix this by moving
this patch to the end of the series.
> I think this needs to be done in a separate patch.
To avoid bisect issues, the clock-frequency property should be removed
in the same patch that adds the clocks property, or after it.
> Also, the UARTD enabling in init_table[] can then be removed.
You still need to initialize all the UART clocks in init_table[]. This
is required to make sure the clocks have the correct parent. Without
this explicit initialization, on Tegra30 at least, it was found that
UART C didn't work correctly for Bluetooth since the default parent was
something that wasn't generating the expected frequency.
Before removing the clock-frequency property, you also need to make sure
that init_table[] forces the UART clock on, otherwise since nothing will
clk_get()/enable() the UART clock, clk_disable_unused() will disable it.
After removing clock-frequency property, of_serial.c will
clk_get()/enable() the UART clock based on the clocks property, and
hence init_table[] doesn't need to force the UART clock on.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

On Wed, Feb 13, 2013 at 05:48:03PM +0100, Stephen Warren wrote:
> On 02/13/2013 04:38 AM, Peter De Schrijver wrote:> > On Tue, Feb 12, 2013 at 06:24:05PM +0100, Stephen Warren wrote:> >> On 02/11/2013 04:04 AM, Peter De Schrijver wrote:> >>> Add references to tegra_car clocks for the basic device nodes.> >>> >> In this patch, you also need to remove the "clock-frequency" properties> >> from tegra114-*.dts board files.> > > > That would probably break bisectability no?> > Oh right, I read the patch order backwards. You can fix this by moving> this patch to the end of the series.>
Probably. Need to check if this doesn't cause other problems.
> > I think this needs to be done in a separate patch.> > To avoid bisect issues, the clock-frequency property should be removed> in the same patch that adds the clocks property, or after it.> > > Also, the UARTD enabling in init_table[] can then be removed.> > You still need to initialize all the UART clocks in init_table[]. This> is required to make sure the clocks have the correct parent. Without> this explicit initialization, on Tegra30 at least, it was found that> UART C didn't work correctly for Bluetooth since the default parent was> something that wasn't generating the expected frequency.>
Yes. The parent relationships still need to be defined. But I think that's
the only thing we actually need to define still? Also the parent relationships
can be board specific I think in some cases, so maybe we want to move those to
DT as well at some point?
Cheers,
Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

On 02/14/2013 03:01 AM, Peter De Schrijver wrote:
> On Wed, Feb 13, 2013 at 05:48:03PM +0100, Stephen Warren wrote:
...
>> You still need to initialize all the UART clocks in init_table[]. This
...
> Yes. The parent relationships still need to be defined. But I think that's> the only thing we actually need to define still?
You might want to explicitly set the rate too, if there is a divider in
the clk module that affects it. If not, then parenting is indeed all you
need.
> Also the parent relationships> can be board specific I think in some cases, so maybe we want to move those to> DT as well at some point?
Yes, that's the plan. I think/thought Prashant was planning to work on that.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

On Thu, Feb 14, 2013 at 06:30:39PM +0100, Stephen Warren wrote:
> On 02/14/2013 03:01 AM, Peter De Schrijver wrote:> > On Wed, Feb 13, 2013 at 05:48:03PM +0100, Stephen Warren wrote:> ...> >> You still need to initialize all the UART clocks in init_table[]. This> ...> > Yes. The parent relationships still need to be defined. But I think that's> > the only thing we actually need to define still?> > You might want to explicitly set the rate too, if there is a divider in> the clk module that affects it. If not, then parenting is indeed all you> need.
Yes, for PLLs that might be useful. Device clock rates can also be set by the
driver and probably should be set by the driver.
Cheers,
Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html