Message ID | 1421113055-17867-6-git-send-email-wens@csie.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, Jan 13, 2015 at 09:37:27AM +0800, Chen-Yu Tsai wrote: > of_clk_get_parent_name() uses the clock-indices property to resolve > clock phandle arguments in case that the argument index does not > match the clock-output-names sequence. > > This is the case on sunxi, where we use the actual bit index as the > argument to the phandle. Add the clock-indices property so that > of_clk_get_parent_name() resolves the names correctly. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Applied. Are the mask in the clock driver still of any use now? I don't think they are, and if we're going that way, I'd rather have them removed from the driver. Maxime
On Thu, Jan 15, 2015 at 12:33 AM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Tue, Jan 13, 2015 at 09:37:27AM +0800, Chen-Yu Tsai wrote: >> of_clk_get_parent_name() uses the clock-indices property to resolve >> clock phandle arguments in case that the argument index does not >> match the clock-output-names sequence. >> >> This is the case on sunxi, where we use the actual bit index as the >> argument to the phandle. Add the clock-indices property so that >> of_clk_get_parent_name() resolves the names correctly. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > Applied. Are the mask in the clock driver still of any use now? I > don't think they are, and if we're going that way, I'd rather have > them removed from the driver. Yes they are still passed through factors_data, for mux_clk_ops to know about the width of the mux, which is 3 bits on older SoCs vs 4 bits on sun9i. ChenYu
On Thu, Jan 15, 2015 at 10:24:04AM +0800, Chen-Yu Tsai wrote: > On Thu, Jan 15, 2015 at 12:33 AM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > On Tue, Jan 13, 2015 at 09:37:27AM +0800, Chen-Yu Tsai wrote: > >> of_clk_get_parent_name() uses the clock-indices property to resolve > >> clock phandle arguments in case that the argument index does not > >> match the clock-output-names sequence. > >> > >> This is the case on sunxi, where we use the actual bit index as the > >> argument to the phandle. Add the clock-indices property so that > >> of_clk_get_parent_name() resolves the names correctly. > >> > >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > > > Applied. Are the mask in the clock driver still of any use now? I > > don't think they are, and if we're going that way, I'd rather have > > them removed from the driver. > > Yes they are still passed through factors_data, for mux_clk_ops to > know about the width of the mux, which is 3 bits on older SoCs vs > 4 bits on sun9i. Erm.... These are gates. They are not muxable and are not handled through clk-factors, so I'm not sure how it is relevant :) Maxime
On Thu, Jan 15, 2015 at 11:20 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Thu, Jan 15, 2015 at 10:24:04AM +0800, Chen-Yu Tsai wrote: >> On Thu, Jan 15, 2015 at 12:33 AM, Maxime Ripard >> <maxime.ripard@free-electrons.com> wrote: >> > On Tue, Jan 13, 2015 at 09:37:27AM +0800, Chen-Yu Tsai wrote: >> >> of_clk_get_parent_name() uses the clock-indices property to resolve >> >> clock phandle arguments in case that the argument index does not >> >> match the clock-output-names sequence. >> >> >> >> This is the case on sunxi, where we use the actual bit index as the >> >> argument to the phandle. Add the clock-indices property so that >> >> of_clk_get_parent_name() resolves the names correctly. >> >> >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> > >> > Applied. Are the mask in the clock driver still of any use now? I >> > don't think they are, and if we're going that way, I'd rather have >> > them removed from the driver. >> >> Yes they are still passed through factors_data, for mux_clk_ops to >> know about the width of the mux, which is 3 bits on older SoCs vs >> 4 bits on sun9i. > > Erm.... These are gates. They are not muxable and are not handled > through clk-factors, so I'm not sure how it is relevant :) Sorry. I jumped to the mux mask stuff. Yes the gate masks are still used, and the gates are still referenced by the bit offset. As described in the commit message, clock-indices is used by of_clk_get_parent_name() to match the index used in the phandle to the correct name in clock-names. Take apb1 for example: clock-indices = <0>, <1>, <2>, <3>, <4>, <16>, <17>, <18>, <19>, <20>, <21>; clock-output-names = "apb1_i2c0", "apb1_i2c1", "apb1_i2c2", "apb1_i2c3", "apb1_i2c4", "apb1_uart0", "apb1_uart1", ... If we have "clocks = <&apb1 16>;" in some device, and we call of_clk_get_parent_name() on said clock, it would try to get clock_output_names[16], which obviously is the wrong one. With clock-indices, of_clk_get_parent_name first looks at that array, finds an entry matching 16, then uses the index of the matching entry to get the name from clock-output-names. So, we are still using the gate bitmask to declare valid clock gates. The sunxi driver does not use clock-indices directly. Nor do I think it was intended to be used by clock drivers directly. Hope this clears things up. ChenYu
On Thu, Jan 15, 2015 at 11:35:42PM +0800, Chen-Yu Tsai wrote: > On Thu, Jan 15, 2015 at 11:20 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > On Thu, Jan 15, 2015 at 10:24:04AM +0800, Chen-Yu Tsai wrote: > >> On Thu, Jan 15, 2015 at 12:33 AM, Maxime Ripard > >> <maxime.ripard@free-electrons.com> wrote: > >> > On Tue, Jan 13, 2015 at 09:37:27AM +0800, Chen-Yu Tsai wrote: > >> >> of_clk_get_parent_name() uses the clock-indices property to resolve > >> >> clock phandle arguments in case that the argument index does not > >> >> match the clock-output-names sequence. > >> >> > >> >> This is the case on sunxi, where we use the actual bit index as the > >> >> argument to the phandle. Add the clock-indices property so that > >> >> of_clk_get_parent_name() resolves the names correctly. > >> >> > >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > >> > > >> > Applied. Are the mask in the clock driver still of any use now? I > >> > don't think they are, and if we're going that way, I'd rather have > >> > them removed from the driver. > >> > >> Yes they are still passed through factors_data, for mux_clk_ops to > >> know about the width of the mux, which is 3 bits on older SoCs vs > >> 4 bits on sun9i. > > > > Erm.... These are gates. They are not muxable and are not handled > > through clk-factors, so I'm not sure how it is relevant :) > > Sorry. I jumped to the mux mask stuff. Yes the gate masks are still > used, and the gates are still referenced by the bit offset. > > As described in the commit message, clock-indices is used by > of_clk_get_parent_name() to match the index used in the phandle > to the correct name in clock-names. > > Take apb1 for example: > > clock-indices = <0>, <1>, <2>, <3>, <4>, > <16>, <17>, <18>, <19>, <20>, <21>; > clock-output-names = "apb1_i2c0", "apb1_i2c1", > "apb1_i2c2", "apb1_i2c3", "apb1_i2c4", > "apb1_uart0", "apb1_uart1", ... > > If we have "clocks = <&apb1 16>;" in some device, and we call > of_clk_get_parent_name() on said clock, it would try to get > clock_output_names[16], which obviously is the wrong one. > > With clock-indices, of_clk_get_parent_name first looks at > that array, finds an entry matching 16, then uses the > index of the matching entry to get the name from > clock-output-names. Yeah, I know what it does, and we do agree on the fact that it's needed. > So, we are still using the gate bitmask to declare valid > clock gates. The sunxi driver does not use clock-indices > directly. Nor do I think it was intended to be used by > clock drivers directly. However, the gate bitmask itself carries exactly the same information than clock-indices. It's the exact same list of numbers, just with two different ways of defining it. If we go with clock-indices, which is the right solution, then we can just drop the other one. I actually started to do just this last evening. A31 boots without any gates bit mask but the USB clocks one so far, I intend on converting the others as well. Maxime
On Thu, Jan 15, 2015 at 11:51 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Thu, Jan 15, 2015 at 11:35:42PM +0800, Chen-Yu Tsai wrote: >> On Thu, Jan 15, 2015 at 11:20 PM, Maxime Ripard >> <maxime.ripard@free-electrons.com> wrote: >> > On Thu, Jan 15, 2015 at 10:24:04AM +0800, Chen-Yu Tsai wrote: >> >> On Thu, Jan 15, 2015 at 12:33 AM, Maxime Ripard >> >> <maxime.ripard@free-electrons.com> wrote: >> >> > On Tue, Jan 13, 2015 at 09:37:27AM +0800, Chen-Yu Tsai wrote: >> >> >> of_clk_get_parent_name() uses the clock-indices property to resolve >> >> >> clock phandle arguments in case that the argument index does not >> >> >> match the clock-output-names sequence. >> >> >> >> >> >> This is the case on sunxi, where we use the actual bit index as the >> >> >> argument to the phandle. Add the clock-indices property so that >> >> >> of_clk_get_parent_name() resolves the names correctly. >> >> >> >> >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> >> > >> >> > Applied. Are the mask in the clock driver still of any use now? I >> >> > don't think they are, and if we're going that way, I'd rather have >> >> > them removed from the driver. >> >> >> >> Yes they are still passed through factors_data, for mux_clk_ops to >> >> know about the width of the mux, which is 3 bits on older SoCs vs >> >> 4 bits on sun9i. >> > >> > Erm.... These are gates. They are not muxable and are not handled >> > through clk-factors, so I'm not sure how it is relevant :) >> >> Sorry. I jumped to the mux mask stuff. Yes the gate masks are still >> used, and the gates are still referenced by the bit offset. >> >> As described in the commit message, clock-indices is used by >> of_clk_get_parent_name() to match the index used in the phandle >> to the correct name in clock-names. >> >> Take apb1 for example: >> >> clock-indices = <0>, <1>, <2>, <3>, <4>, >> <16>, <17>, <18>, <19>, <20>, <21>; >> clock-output-names = "apb1_i2c0", "apb1_i2c1", >> "apb1_i2c2", "apb1_i2c3", "apb1_i2c4", >> "apb1_uart0", "apb1_uart1", ... >> >> If we have "clocks = <&apb1 16>;" in some device, and we call >> of_clk_get_parent_name() on said clock, it would try to get >> clock_output_names[16], which obviously is the wrong one. >> >> With clock-indices, of_clk_get_parent_name first looks at >> that array, finds an entry matching 16, then uses the >> index of the matching entry to get the name from >> clock-output-names. > > Yeah, I know what it does, and we do agree on the fact that it's > needed. > >> So, we are still using the gate bitmask to declare valid >> clock gates. The sunxi driver does not use clock-indices >> directly. Nor do I think it was intended to be used by >> clock drivers directly. > > However, the gate bitmask itself carries exactly the same information > than clock-indices. It's the exact same list of numbers, just with two > different ways of defining it. > > If we go with clock-indices, which is the right solution, then we can > just drop the other one. > > I actually started to do just this last evening. A31 boots without any > gates bit mask but the USB clocks one so far, I intend on converting > the others as well. So as I understand, you want to replace the masks in the clock drivers with clock-indices in the dt. Is this correct? This potentially makes the gates clock driver very generic, which is nice. I only see drivers/clk/shmobile/clk-mstp.c using it this way though. Didn't we have this for sun6i-apb0-gates at one time? I'm not against it. Just want to make sure everyone agrees, and we can work who and how we're going about this. Thanks ChenYu
On Fri, Jan 16, 2015 at 12:09:01AM +0800, Chen-Yu Tsai wrote: > On Thu, Jan 15, 2015 at 11:51 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > On Thu, Jan 15, 2015 at 11:35:42PM +0800, Chen-Yu Tsai wrote: > >> On Thu, Jan 15, 2015 at 11:20 PM, Maxime Ripard > >> <maxime.ripard@free-electrons.com> wrote: > >> > On Thu, Jan 15, 2015 at 10:24:04AM +0800, Chen-Yu Tsai wrote: > >> >> On Thu, Jan 15, 2015 at 12:33 AM, Maxime Ripard > >> >> <maxime.ripard@free-electrons.com> wrote: > >> >> > On Tue, Jan 13, 2015 at 09:37:27AM +0800, Chen-Yu Tsai wrote: > >> >> >> of_clk_get_parent_name() uses the clock-indices property to resolve > >> >> >> clock phandle arguments in case that the argument index does not > >> >> >> match the clock-output-names sequence. > >> >> >> > >> >> >> This is the case on sunxi, where we use the actual bit index as the > >> >> >> argument to the phandle. Add the clock-indices property so that > >> >> >> of_clk_get_parent_name() resolves the names correctly. > >> >> >> > >> >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > >> >> > > >> >> > Applied. Are the mask in the clock driver still of any use now? I > >> >> > don't think they are, and if we're going that way, I'd rather have > >> >> > them removed from the driver. > >> >> > >> >> Yes they are still passed through factors_data, for mux_clk_ops to > >> >> know about the width of the mux, which is 3 bits on older SoCs vs > >> >> 4 bits on sun9i. > >> > > >> > Erm.... These are gates. They are not muxable and are not handled > >> > through clk-factors, so I'm not sure how it is relevant :) > >> > >> Sorry. I jumped to the mux mask stuff. Yes the gate masks are still > >> used, and the gates are still referenced by the bit offset. > >> > >> As described in the commit message, clock-indices is used by > >> of_clk_get_parent_name() to match the index used in the phandle > >> to the correct name in clock-names. > >> > >> Take apb1 for example: > >> > >> clock-indices = <0>, <1>, <2>, <3>, <4>, > >> <16>, <17>, <18>, <19>, <20>, <21>; > >> clock-output-names = "apb1_i2c0", "apb1_i2c1", > >> "apb1_i2c2", "apb1_i2c3", "apb1_i2c4", > >> "apb1_uart0", "apb1_uart1", ... > >> > >> If we have "clocks = <&apb1 16>;" in some device, and we call > >> of_clk_get_parent_name() on said clock, it would try to get > >> clock_output_names[16], which obviously is the wrong one. > >> > >> With clock-indices, of_clk_get_parent_name first looks at > >> that array, finds an entry matching 16, then uses the > >> index of the matching entry to get the name from > >> clock-output-names. > > > > Yeah, I know what it does, and we do agree on the fact that it's > > needed. > > > >> So, we are still using the gate bitmask to declare valid > >> clock gates. The sunxi driver does not use clock-indices > >> directly. Nor do I think it was intended to be used by > >> clock drivers directly. > > > > However, the gate bitmask itself carries exactly the same information > > than clock-indices. It's the exact same list of numbers, just with two > > different ways of defining it. > > > > If we go with clock-indices, which is the right solution, then we can > > just drop the other one. > > > > I actually started to do just this last evening. A31 boots without any > > gates bit mask but the USB clocks one so far, I intend on converting > > the others as well. > > So as I understand, you want to replace the masks in the clock drivers > with clock-indices in the dt. Is this correct? This potentially makes > the gates clock driver very generic, which is nice. Yeah, the only thing that is not are the clocks protected, which will depend on the compatible. > I only see drivers/clk/shmobile/clk-mstp.c using it this way though. > Didn't we have this for sun6i-apb0-gates at one time? I don't think we did. Or at least, it was never merged. And since this property was introduced for shmobile iirc, I don't find it very surprising :) > I'm not against it. Just want to make sure everyone agrees, and we > can work who and how we're going about this. I did most of the work tonight, for all the sun7i, sun8i and sun9i SoCs. sun6i work, I'll test sun9i, but I'll need some tests from you for the A23 if it's ok :) Thanks! Maxime
diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi index ddc34676987d..9d0a66c14caf 100644 --- a/arch/arm/boot/dts/sun9i-a80.dtsi +++ b/arch/arm/boot/dts/sun9i-a80.dtsi @@ -260,6 +260,9 @@ compatible = "allwinner,sun9i-a80-ahb0-gates-clk"; reg = <0x06000580 0x4>; clocks = <&ahb0>; + clock-indices = <0>, <1>, <3>, <5>, <8>, <12>, <13>, + <14>, <15>, <16>, <18>, <20>, <21>, + <22>, <23>; clock-output-names = "ahb0_fd", "ahb0_ve", "ahb0_gpu", "ahb0_ss", "ahb0_sd", "ahb0_nand1", "ahb0_nand0", "ahb0_sdram", @@ -273,6 +276,7 @@ compatible = "allwinner,sun9i-a80-ahb1-gates-clk"; reg = <0x06000584 0x4>; clocks = <&ahb1>; + clock-indices = <0>, <1>, <17>, <21>, <22>, <23>, <24>; clock-output-names = "ahb1_usbotg", "ahb1_usbhci", "ahb1_gmac", "ahb1_msgbox", "ahb1_spinlock", "ahb1_hstimer", @@ -284,6 +288,8 @@ compatible = "allwinner,sun9i-a80-ahb2-gates-clk"; reg = <0x06000588 0x4>; clocks = <&ahb2>; + clock-indices = <0>, <1>, <2>, <4>, <5>, <7>, <8>, + <11>; clock-output-names = "ahb2_lcd0", "ahb2_lcd1", "ahb2_edp", "ahb2_csi", "ahb2_hdmi", "ahb2_de", "ahb2_mp", "ahb2_mipi_dsi"; @@ -294,6 +300,8 @@ compatible = "allwinner,sun9i-a80-apb0-gates-clk"; reg = <0x06000590 0x4>; clocks = <&apb0>; + clock-indices = <1>, <5>, <11>, <12>, <13>, <15>, + <17>, <18>, <19>; clock-output-names = "apb0_spdif", "apb0_pio", "apb0_ac97", "apb0_i2s0", "apb0_i2s1", "apb0_lradc", "apb0_gpadc", "apb0_twd", @@ -305,6 +313,8 @@ compatible = "allwinner,sun9i-a80-apb1-gates-clk"; reg = <0x06000594 0x4>; clocks = <&apb1>; + clock-indices = <0>, <1>, <2>, <3>, <4>, + <16>, <17>, <18>, <19>, <20>, <21>; clock-output-names = "apb1_i2c0", "apb1_i2c1", "apb1_i2c2", "apb1_i2c3", "apb1_i2c4", "apb1_uart0", "apb1_uart1",
of_clk_get_parent_name() uses the clock-indices property to resolve clock phandle arguments in case that the argument index does not match the clock-output-names sequence. This is the case on sunxi, where we use the actual bit index as the argument to the phandle. Add the clock-indices property so that of_clk_get_parent_name() resolves the names correctly. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- arch/arm/boot/dts/sun9i-a80.dtsi | 10 ++++++++++ 1 file changed, 10 insertions(+)