| Message ID | 1429639796-2169-10-git-send-email-eric@anholt.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/21/2015 12:09 PM, Eric Anholt wrote: > We were previously relying on the fixed clock registration in > clk-bcm2835, but there doesn't seem to be any real reason to not just > define it in the DT (and for the 2836 port, I would have needed to > change the clock's physical address in clk-bcm2835.c). Also, because > we weren't registering the apb_pclk in clk-bcm2835 as a clock device, > we were picking up the uart clock node as apb_pclk by accident. Doesn't the following do just that? clk = clk_register_fixed_rate(NULL, "apb_pclk", NULL, CLK_IS_ROOT, 126000000); Anyway, with this patch, shouldn't we fix drivers/clk/clk-bcm2835.c not to register clocks that match those that are added to DT in this patch? Actually, to maintain DT ABI (new kernels working with old or new DT), we probably need to keep the code in clk-bcm2835.c, but make it conditional upon whether there's a clock node in the DT file.
Stephen Warren <swarren@wwwdotorg.org> writes: > On 04/21/2015 12:09 PM, Eric Anholt wrote: >> We were previously relying on the fixed clock registration in >> clk-bcm2835, but there doesn't seem to be any real reason to not just >> define it in the DT (and for the 2836 port, I would have needed to >> change the clock's physical address in clk-bcm2835.c). Also, because >> we weren't registering the apb_pclk in clk-bcm2835 as a clock device, >> we were picking up the uart clock node as apb_pclk by accident. > > Doesn't the following do just that? > > clk = clk_register_fixed_rate(NULL, "apb_pclk", NULL, CLK_IS_ROOT, > 126000000); Nope! You also need the clk_register_clkdev for the type of lookup being done by APB to find it. > Anyway, with this patch, shouldn't we fix drivers/clk/clk-bcm2835.c not > to register clocks that match those that are added to DT in this patch? > Actually, to maintain DT ABI (new kernels working with old or new DT), > we probably need to keep the code in clk-bcm2835.c, but make it > conditional upon whether there's a clock node in the DT file. Yeah, I hadn't modified the .c code because of DT ABI
On 04/24/2015 11:06 AM, Eric Anholt wrote: > Stephen Warren <swarren@wwwdotorg.org> writes: > >> On 04/21/2015 12:09 PM, Eric Anholt wrote: >>> We were previously relying on the fixed clock registration in >>> clk-bcm2835, but there doesn't seem to be any real reason to >>> not just define it in the DT (and for the 2836 port, I would >>> have needed to change the clock's physical address in >>> clk-bcm2835.c). Also, because we weren't registering the >>> apb_pclk in clk-bcm2835 as a clock device, we were picking up >>> the uart clock node as apb_pclk by accident. >> >> Doesn't the following do just that? >> >> clk = clk_register_fixed_rate(NULL, "apb_pclk", NULL, >> CLK_IS_ROOT, 126000000); > > Nope! You also need the clk_register_clkdev for the type of > lookup being done by APB to find it. Oh right, that just creates a clock object without registering it in the lookup table? >> Anyway, with this patch, shouldn't we fix >> drivers/clk/clk-bcm2835.c not to register clocks that match those >> that are added to DT in this patch? Actually, to maintain DT ABI >> (new kernels working with old or new DT), we probably need to >> keep the code in clk-bcm2835.c, but make it conditional upon >> whether there's a clock node in the DT file. > > Yeah, I hadn't modified the .c code because of DT ABI Should the C file be modified not the create those clock objects if the DT contains them?
diff --git a/arch/arm/boot/dts/bcm283x-common.dtsi b/arch/arm/boot/dts/bcm283x-common.dtsi index c15e309..2ce6661 100644 --- a/arch/arm/boot/dts/bcm283x-common.dtsi +++ b/arch/arm/boot/dts/bcm283x-common.dtsi @@ -67,8 +67,9 @@ compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell"; reg = <0x7e201000 0x1000>; interrupts = <2 25>; - clock-frequency = <3000000>; arm,primecell-periphid = <0x00241011>; + clocks = <&uart0_clk>, <&apb_pclk>; + clock-names = "uartclk", "apb_pclk"; }; i2s: i2s@7e203000 { @@ -155,5 +156,19 @@ clock-output-names = "spi"; clock-frequency = <250000000>; }; + + apb_pclk: apb_pclk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-output-names = "apb_pclk"; + clock-frequency = <126000000>; + }; + + uart0_clk: uart0_clk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-output-names = "uart0_clk"; + clock-frequency = <3000000>; + }; }; };
We were previously relying on the fixed clock registration in clk-bcm2835, but there doesn't seem to be any real reason to not just define it in the DT (and for the 2836 port, I would have needed to change the clock's physical address in clk-bcm2835.c). Also, because we weren't registering the apb_pclk in clk-bcm2835 as a clock device, we were picking up the uart clock node as apb_pclk by accident. This gets serial working on 2836. Signed-off-by: Eric Anholt <eric@anholt.net> --- arch/arm/boot/dts/bcm283x-common.dtsi | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)