Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Jerome Brunet <jbrunet@baylibre.com>
To:     Stephen Boyd <sboyd@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>
Cc:     linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
        Neil Armstrong <narmstrong@baylibre.com>
Subject: Re: [PATCH 3/9] clk: meson: axg-audio: Don't reference clk_init_data after registration
In-Reply-To: <20190731193517.237136-4-sboyd@kernel.org>
References: <20190731193517.237136-1-sboyd@kernel.org> <20190731193517.237136-4-sboyd@kernel.org>
Date:   Tue, 06 Aug 2019 10:49:47 +0200
Message-ID: <1jwofqvftg.fsf@starbuckisacylon.baylibre.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed 31 Jul 2019 at 12:35, Stephen Boyd <sboyd@kernel.org> wrote:

> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.

Hi Stephen,

What to do you indend to do with this one ? Will you apply directly or
should we take it ?

We have several changes for the controller which may conflict with this
one. It is nothing major but the sooner I know how this changes goes in,
the sooner I can rebase the rest.

Also, We were (re)using the init_data only on register failures.
I understand that you want to guarantee .init is NULL when the clock is
registered, but it this particular case, the registeration failed so the
clock is not registered.

IMO, it would be better if devm_clk_hw_register() left the init_data
untouched if the registration fails.

>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
>
> Please ack so I can take this through clk tree
>
>  drivers/clk/meson/axg-audio.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c
> index 8028ff6f6610..db0b73d53551 100644
> --- a/drivers/clk/meson/axg-audio.c
> +++ b/drivers/clk/meson/axg-audio.c
> @@ -992,15 +992,18 @@ static int axg_audio_clkc_probe(struct platform_device *pdev)
>  
>  	/* Take care to skip the registered input clocks */
>  	for (i = AUD_CLKID_DDR_ARB; i < data->hw_onecell_data->num; i++) {
> +		const char *name;
> +
>  		hw = data->hw_onecell_data->hws[i];
>  		/* array might be sparse */
>  		if (!hw)
>  			continue;
>  
> +		name = hw->init->name;
> +
>  		ret = devm_clk_hw_register(dev, hw);
>  		if (ret) {
> -			dev_err(dev, "failed to register clock %s\n",
> -				hw->init->name);
> +			dev_err(dev, "failed to register clock %s\n", name);
>  			return ret;
>  		}
>  	}
> -- 
> Sent by a computer through tubes