linux-kernel - Re: [PATCH] clk: qcom: gcc: Fix board clock node name

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <154178353645.88331.795912528906051068@swboyd.mtv.corp.google.com>
Date:   Fri, 09 Nov 2018 09:12:16 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     Michael Turquette <mturquette@...libre.com>,
        Vinod Koul <vkoul@...nel.org>
Cc:     Vinod Koul <vkoul@...nel.org>, Andy Gross <andy.gross@...aro.org>,
        David Brown <david.brown@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk: qcom: gcc: Fix board clock node name

Quoting Vinod Koul (2018-11-09 01:50:54)
> Device tree node name are not supposed to have "_" in them so fix the
> node name use of xo_board to xo-board
> 
> Fixes: 652f1813c113 ("clk: qcom: gcc: Add global clock controller driver for QCS404")
> Signed-off-by: Vinod Koul <vkoul@...nel.org>
> ---
> 
> Steve: RobH pointed this on DTS patches, would be great if you can pick this
> as a fix

Ok.

> 
>  drivers/clk/qcom/gcc-qcs404.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c
> index e4ca6a45f313..ef1b267cb058 100644
> --- a/drivers/clk/qcom/gcc-qcs404.c
> +++ b/drivers/clk/qcom/gcc-qcs404.c
> @@ -265,7 +265,7 @@ static struct clk_fixed_factor cxo = {
>         .div = 1,
>         .hw.init = &(struct clk_init_data){
>                 .name = "cxo",
> -               .parent_names = (const char *[]){ "xo_board" },
> +               .parent_names = (const char *[]){ "xo-board" },

We have xo_board used everywhere else in drivers/clk/qcom/ so this makes
me concerned. Wouldn't a better answer be to add clock-output-names to
the xo-board DT node and have arm-soc merge that through. I mostly want
to keep things consistent here so that if we need to inject an xo_board
clk into the system then we can do so generically instead of making it
per-platform. Of course, if we're never going to have this problem on
qcs404 then it will be fine to start differing here. So I'm leaning
towards merge this patch, just please ack my concern here and tell me it
won't be a problem and I'll be happy to merge to clk-fixes.

BTW, can you also specify a 'clocks' property in the GCC node and send
the xo_board node there? In fact, we should do that for every GCC node
in the tree. Care to do that and also add sleep_clk to each clock
controller node that uses it? This is useful to do so that we can more
easily see where clocks are going between clock controller nodes.