| Message ID | 20240213105730.5287-1-tony@atomide.com |
|---|---|
| Headers | show |
| Series | Use reg instead of ti,bit-shift for clksel | expand |
On Tue, 13 Feb 2024 12:56:40 +0200 Tony Lindgren <tony@atomide.com> wrote: > Hi all, > > This series updates the clksel clocks to use the standard reg property > instead of ti,bit-shift. > > I'd like to apply these before we make further use of the clksel clocks > to reduce the dtb check warnings. > hmm, we still have ti,bit-shift if these clocks are not used below a ti,clksel. Just wondering, can we completely deorbit ti,bit-shift if we used #address-cells = <2>; in those cases? I wait a bit with further txt->yaml conversions until this is settled. Regards, Andreas
* Andreas Kemnade <andreas@kemnade.info> [240213 23:11]: > On Tue, 13 Feb 2024 12:56:40 +0200 > Tony Lindgren <tony@atomide.com> wrote: > > > Hi all, > > > > This series updates the clksel clocks to use the standard reg property > > instead of ti,bit-shift. > > > > I'd like to apply these before we make further use of the clksel clocks > > to reduce the dtb check warnings. > > > > hmm, we still have ti,bit-shift if these clocks are not used below a ti,clksel. > Just wondering, can we completely deorbit ti,bit-shift if we used #address-cells = <2>; > in those cases? I wait a bit with further txt->yaml conversions until > this is settled. No need to wait on the yaml conversion I think :) How about just tag the ti,bit-shift property as deprecated? And add a comment saying it is only needed for the remaining unconnected clocks. Eventually we can move all the component clocks under clksel clocks, or the related clock such as the dpll clock for the clkdcoldo clocks. Regards, Tony
Quoting Tony Lindgren (2024-02-13 02:56:41) > In order to use #address-cells = <1> and start making use of the > standard reg property, let's prepare things to ignore the possible > address in the clock node name. > > Unless the clock-output-names property is used, the legacy clocks still > fall back to matching the clock data based on the node name. > > We use cleanup.h to simplify the return path for freeing tmp. > > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- Acked-by: Stephen Boyd <sboyd@kernel.org>
* Tony Lindgren <tony@atomide.com> [240214 05:41]: > * Andreas Kemnade <andreas@kemnade.info> [240213 23:11]: > > On Tue, 13 Feb 2024 12:56:40 +0200 > > Tony Lindgren <tony@atomide.com> wrote: > > > > > Hi all, > > > > > > This series updates the clksel clocks to use the standard reg property > > > instead of ti,bit-shift. > > > > > > I'd like to apply these before we make further use of the clksel clocks > > > to reduce the dtb check warnings. > > > > > > > hmm, we still have ti,bit-shift if these clocks are not used below a ti,clksel. > > Just wondering, can we completely deorbit ti,bit-shift if we used #address-cells = <2>; > > in those cases? I wait a bit with further txt->yaml conversions until > > this is settled. > > No need to wait on the yaml conversion I think :) How about just tag the > ti,bit-shift property as deprecated? And add a comment saying it is only > needed for the remaining unconnected clocks. > > Eventually we can move all the component clocks under clksel clocks, or the > related clock such as the dpll clock for the clkdcoldo clocks. Oh and yes, using #clock-cells = <2> would be nice eventually :) I think the clkcel binding already supports that. But that still leaves the issue of unconnected composite clocks.. I'm pretty sure they all have some real parent like clksel for dpll though. If you had some good idea in mind for the #address-cells = <2> for the remaining unconnected composite clocks maybe clarify it a bit. Regards, Tony
Hi Tony, On Thu, 29 Feb 2024 09:06:26 +0200 Tony Lindgren <tony@atomide.com> wrote: > * Tony Lindgren <tony@atomide.com> [240214 05:41]: > > * Andreas Kemnade <andreas@kemnade.info> [240213 23:11]: > > > On Tue, 13 Feb 2024 12:56:40 +0200 > > > Tony Lindgren <tony@atomide.com> wrote: > > > > > > > Hi all, > > > > > > > > This series updates the clksel clocks to use the standard reg property > > > > instead of ti,bit-shift. > > > > > > > > I'd like to apply these before we make further use of the clksel clocks > > > > to reduce the dtb check warnings. > > > > > > > > > > hmm, we still have ti,bit-shift if these clocks are not used below a ti,clksel. > > > Just wondering, can we completely deorbit ti,bit-shift if we used #address-cells = <2>; > > > in those cases? I wait a bit with further txt->yaml conversions until > > > this is settled. > > > > No need to wait on the yaml conversion I think :) How about just tag the > > ti,bit-shift property as deprecated? And add a comment saying it is only > > needed for the remaining unconnected clocks. > > > > Eventually we can move all the component clocks under clksel clocks, or the > > related clock such as the dpll clock for the clkdcoldo clocks. > > Oh and yes, using #clock-cells = <2> would be nice eventually :) I think > the clkcel binding already supports that. But that still leaves the issue > of unconnected composite clocks.. I'm pretty sure they all have some real > parent like clksel for dpll though. > > If you had some good idea in mind for the #address-cells = <2> for the > remaining unconnected composite clocks maybe clarify it a bit. > I was just wondering whether we could do reg = <register bit> then. Regards, Andreas
* Andreas Kemnade <andreas@kemnade.info> [240302 19:18]: > Tony Lindgren <tony@atomide.com> wrote: > > If you had some good idea in mind for the #address-cells = <2> for the > > remaining unconnected composite clocks maybe clarify it a bit. > > > I was just wondering whether we could do reg = <register bit> then. Yeah sure nothing stopping us from doing that too if it helps :) Regards, Tony
Hi Tony, On Tue, Feb 13, 2024 at 11:59 AM Tony Lindgren <tony@atomide.com> wrote: > In order to use #address-cells = <1> and start making use of the > standard reg property, let's prepare things to ignore the possible > address in the clock node name. > > Unless the clock-output-names property is used, the legacy clocks still > fall back to matching the clock data based on the node name. > > We use cleanup.h to simplify the return path for freeing tmp. > > Signed-off-by: Tony Lindgren <tony@atomide.com> Thanks for your patch, which is now commit 3516338543cafb65 ("clk: ti: Handle possible address in the node name") in v6.9-rc1. This causes an early boot crash on BeagleBone Black: ti_dt_clocks_register: failed to lookup clock node clk-24mhz-clkctrl:0000:0, ret=-517 ti_dt_clocks_register: failed to lookup clock node clk-24mhz-clkctrl:0000:0, ret=-517 ti_dt_clocks_register: failed to lookup clock node l3-aon-clkctrl:0000:30, ret=-517 ti_dt_clocks_register: failed to lookup clock node l3-aon-clkctrl:0000:19, ret=-517 ti_dt_clocks_register: failed to lookup clock node l4-wkup-clkctrl:0008:18, ret=-517 ti_dt_clocks_register: failed to lookup clock node l4ls-clkctrl:0074:18, ret=-517 ti_dt_clocks_register: failed to lookup clock node l4ls-clkctrl:0078:18, ret=-517 ti_dt_clocks_register: failed to lookup clock node l4ls-clkctrl:007c:18, ret=-517 ti_dt_clocks_register: failed to lookup clock node l3-aon-clkctrl:0000:27, ret=-517 ti_dt_clocks_register: failed to lookup clock node l3-aon-clkctrl:0000:22, ret=-517 ti_dt_clocks_register: failed to lookup clock node l3-aon-clkctrl:0000:24, ret=-517 ti_dt_clocks_register: failed to lookup clock node l3-aon-clkctrl:0000:20, ret=-517 8<--- cut here --- Unable to handle kernel paging request at virtual address fffffffe when read [fffffffe] *pgd=9fdf6841, *pte=00000000, *ppte=00000000 Internal error: Oops: 27 [#1] SMP ARM CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.8.0-rc1-boneblack-00001-g3516338543ca #119 Hardware name: Generic AM33XX (Flattened Device Tree) PC is at clk_set_parent+0x2c/0x6c LR is at __lock_acquire+0x3f8/0x29d4 pc : [<c06ecd4c>] lr : [<c01b2b14>] psr: a0000093 sp : c1001fb0 ip : 00000000 fp : 00000000 r10: c0f5da88 r9 : 00000000 r8 : 00000078 r7 : ffffffff r6 : c11ba000 r5 : fffffffe r4 : c20a0700 r3 : 00000000 r2 : c100c580 r1 : 00000001 r0 : c209ac00 Flags: NzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 80004019 DAC: 00000051 Register r0 information: slab kmalloc-192 start c209ac00 pointer offset 0 size 192 Register r1 information: non-paged memory Register r2 information: non-slab/vmalloc memory Register r3 information: NULL pointer Register r4 information: slab kmalloc-64 start c20a0700 pointer offset 0 size 64 Register r5 information: non-paged memory Register r6 information: non-slab/vmalloc memory Register r7 information: non-paged memory Register r8 information: non-paged memory Register r9 information: NULL pointer Register r10 information: non-slab/vmalloc memory Register r11 information: NULL pointer Register r12 information: NULL pointer Process swapper/0 (pid: 0, stack limit = 0x(ptrval)) Stack: (0xc1001fb0 to 0xc1002000) 1fa0: c20a0700 c10093c0 c11ba000 c0f2ebdc 1fc0: dfdffc40 c0f11380 dfdffc40 c0f01074 ffffffff ffffffff 00000000 c0f006f0 1fe0: 00000000 c0f5da88 00000000 00000e05 00000000 00000000 00000000 00000000 clk_set_parent from am33xx_dt_clk_init+0x84/0xa4 am33xx_dt_clk_init from omap_init_time_of+0x8/0x10 omap_init_time_of from start_kernel+0x430/0x638 start_kernel from 0x0 Code: e3530000 1a00000e e3550000 e5940000 (15955000) ---[ end trace 0000000000000000 ]--- Kernel panic - not syncing: Attempted to kill the idle task! ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]--- Reverting the commit on top of a recent v6.11-rcX-based tree fixes the issue. BTW, bisecting this took a while as: 1. The OMAP serial driver locks up when booted with "earlycon keep_bootcon", 2. The TI SYSC sometimes crashes during early boot, too: Unhandled fault: external abort on non-linefetch (0x1008) at 0xe036d010 [e036d010] *pgd=8276e811, *pte=47400653, *ppte=47400453 Internal error: : 1008 [#1] SMP ARM Modules linked in: CPU: 0 PID: 33 Comm: kworker/u4:3 Not tainted 6.8.0-boneblack-05567-gaa7d6513d68b #78 Hardware name: Generic AM33XX (Flattened Device Tree) Workqueue: events_unbound deferred_probe_work_func PC is at sysc_reset+0x118/0x210 LR is at sysc_probe+0xe08/0x1440 pc : [<c06d0ba8>] lr : [<c06d1cd8>] psr: 60000013 > --- a/drivers/clk/ti/clk.c > +++ b/drivers/clk/ti/clk.c > @@ -7,6 +7,7 @@ > * Tero Kristo <t-kristo@ti.com> > */ > > +#include <linux/cleanup.h> > #include <linux/clk.h> > #include <linux/clk-provider.h> > #include <linux/clkdev.h> > @@ -114,20 +115,26 @@ int ti_clk_setup_ll_ops(struct ti_clk_ll_ops *ops) > > /* > * Eventually we could standardize to using '_' for clk-*.c files to follow the > - * TRM naming and leave out the tmp name here. > + * TRM naming. > */ > static struct device_node *ti_find_clock_provider(struct device_node *from, > const char *name) > { > + char *tmp __free(kfree) = NULL; > struct device_node *np; > bool found = false; > const char *n; > - char *tmp; > + char *p; > > tmp = kstrdup_and_replace(name, '-', '_', GFP_KERNEL); > if (!tmp) > return NULL; > > + /* Ignore a possible address for the node name */ > + p = strchr(tmp, '@'); > + if (p) > + *p = '\0'; > + > /* Node named "clock" with "clock-output-names" */ > for_each_of_allnodes_from(from, np) { > if (of_property_read_string_index(np, "clock-output-names", > @@ -140,7 +147,6 @@ static struct device_node *ti_find_clock_provider(struct device_node *from, > break; > } > } > - kfree(tmp); > > if (found) { > of_node_put(from); > @@ -148,7 +154,7 @@ static struct device_node *ti_find_clock_provider(struct device_node *from, > } > > /* Fall back to using old node name base provider name */ > - return of_find_node_by_name(from, name); > + return of_find_node_by_name(from, tmp); > } > > /** Gr{oetje,eeting}s, Geert
On Mon, Sep 2, 2024 at 4:03 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Tue, Feb 13, 2024 at 11:59 AM Tony Lindgren <tony@atomide.com> wrote: > > In order to use #address-cells = <1> and start making use of the > > standard reg property, let's prepare things to ignore the possible > > address in the clock node name. > > > > Unless the clock-output-names property is used, the legacy clocks still > > fall back to matching the clock data based on the node name. > > > > We use cleanup.h to simplify the return path for freeing tmp. > > > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > Thanks for your patch, which is now commit 3516338543cafb65 ("clk: ti: > Handle possible address in the node name") in v6.9-rc1. > This causes an early boot crash on BeagleBone Black: > > ti_dt_clocks_register: failed to lookup clock node > clk-24mhz-clkctrl:0000:0, ret=-517 I found the culprit: after the move of .dts files to vendor sub-directories, I had updated my boot script to: DTB=arch/arm/boot/dts/ti/am335x-boneblack.dtb if [ ! -e $DTB ]; then DTB=arch/arm/boot/dts/am335x-boneblack.dtb fi I.e. I missed the "/omap" part, causing the install to fall back to an old DTB file that no longer works with modern kernels. Sorry for the noise. Gr{oetje,eeting}s, Geert
Hi all, This series updates the clksel clocks to use the standard reg property instead of ti,bit-shift. I'd like to apply these before we make further use of the clksel clocks to reduce the dtb check warnings. Regards, Tony Tony Lindgren (4): clk: ti: Handle possible address in the node name clk: ti: Improve clksel clock bit parsing for reg property ARM: dts: am3: Update clksel clocks to use reg instead of ti,bit-shift ARM: dts: omap3: Update clksel clocks to use reg instead of ti,bit-shift arch/arm/boot/dts/ti/omap/am33xx-clocks.dtsi | 39 +- arch/arm/boot/dts/ti/omap/am35xx-clocks.dtsi | 18 +- .../boot/dts/ti/omap/omap3430es1-clocks.dtsi | 52 +- .../dts/ti/omap/omap34xx-omap36xx-clocks.dtsi | 86 +-- ...map36xx-am35xx-omap3430es2plus-clocks.dtsi | 28 +- .../arm/boot/dts/ti/omap/omap36xx-clocks.dtsi | 7 +- .../omap/omap36xx-omap3430es2plus-clocks.dtsi | 46 +- .../arm/boot/dts/ti/omap/omap3xxx-clocks.dtsi | 510 +++++++++--------- drivers/clk/ti/apll.c | 11 +- drivers/clk/ti/clk.c | 71 ++- drivers/clk/ti/clock.h | 1 + drivers/clk/ti/divider.c | 5 +- drivers/clk/ti/gate.c | 9 +- drivers/clk/ti/interface.c | 4 +- drivers/clk/ti/mux.c | 6 +- include/linux/clk/ti.h | 3 + 16 files changed, 491 insertions(+), 405 deletions(-)