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: <CAP6Zq1iPmy-fvqqAwBuoskR18v0dPVwYm0tEcE5h1g8fOiOQvg@mail.gmail.com>
Date:   Tue, 17 Jan 2023 19:35:33 +0200
From:   Tomer Maimon <tmaimon77@...il.com>
To:     Stephen Boyd <sboyd@...nel.org>
Cc:     avifishman70@...il.com, benjaminfair@...gle.com, joel@....id.au,
        mturquette@...libre.com, tali.perry1@...il.com, venture@...gle.com,
        yuenn@...gle.com, openbmc@...ts.ozlabs.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v14 1/1] clk: npcm8xx: add clock controller

Hi Stephen,

Very sorry for the late reply.

On Fri, 16 Dec 2022 at 20:44, Stephen Boyd <sboyd@...nel.org> wrote:
>
> Quoting Tomer Maimon (2022-12-11 12:43:24)
> > diff --git a/drivers/clk/clk-npcm8xx.c b/drivers/clk/clk-npcm8xx.c
> > new file mode 100644
> > index 000000000000..08ee7bea6f3a
> > --- /dev/null
> > +++ b/drivers/clk/clk-npcm8xx.c
> > @@ -0,0 +1,650 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> [...]
> > +#define NPCM8XX_CLK_S_RCP        "rcp"
> > +
> > +static const u32 pll_mux_table[] = { 0, 1, 2, 3 };
> > +static const struct clk_parent_data pll_mux_parents[] = {
> > +       { .fw_name = NPCM8XX_CLK_S_PLL0, .name = NPCM8XX_CLK_S_PLL0 },
>
> As this is a new driver either you should only have .fw_name here. The
> .name field is a backup to migrate code over to a new binding. When
> .fw_name is used there should be an associated DT binding update. I
What do you mean by associated DT binding? does the.fw_name, for
example, NPCM8XX_CLK_S_PLL0 need to represent in the device tree?
> doubt the usage of .fw_name is correct though, because aren't these clks
> internal to the controller? The .fw_name field is about describing does the
yes the PLL clocks are internal.
> parents that are an input to the clk controller node in DT (because the
> controller is a consumer of these clks that are external to the device).
>
> So can you use the .hw field for these internal clks? Check out
> CLK_HW_INIT_HWS() macro and friends for a possible way to initialize
> this.
but still, I have used devm_clk_hw_register_mux_parent_data_table
function to register the clock mux,
should I use  devm_clk_hw_register_mux_parent_hws function instead?
Does this modification need to be done in all the mux parent struct?
could you point me to some example in the Linux kernel how do you
think that I should represent the mux clock in the NPCM8XX clock
driver?
>
> > +       { .fw_name = NPCM8XX_CLK_S_PLL1, .name = NPCM8XX_CLK_S_PLL1 },
> > +       { .fw_name = NPCM8XX_CLK_S_REFCLK, .name = NPCM8XX_CLK_S_REFCLK },
>
> Maybe this is external? If so, it would be great to have this in the
> binding as a `clocks` property.
O.K.

Thanks,

Tomer

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ