Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20220120052303.2098394-1-sboyd@kernel.org> <CAGS_qxoMRsvx8HDex9Aq9KUALAQrdNe3VVYikyzT59TEYfbLdg@mail.gmail.com>
 <20220120175140.D8989C340E3@smtp.kernel.org>
In-Reply-To: <20220120175140.D8989C340E3@smtp.kernel.org>
From:   Daniel Latypov <dlatypov@google.com>
Date:   Thu, 20 Jan 2022 09:58:53 -0800
Message-ID: <CAGS_qxqUUnvSOs8ihGQJLVgZtUfKbjj7E_2RiMTQT4u=WmKCXw@mail.gmail.com>
Subject: Re: [PATCH v2] clk: gate: Add some kunit test suites
To:     Stephen Boyd <sboyd@kernel.org>
Cc:     Michael Turquette <mturquette@baylibre.com>,
        linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
        Brendan Higgins <brendanhiggins@google.com>,
        kunit-dev@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jan 20, 2022 at 9:51 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Daniel Latypov (2022-01-19 22:26:50)
> >  On Wed, Jan 19, 2022 at 9:23 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > > +
> > > +static void clk_gate_test_exit(struct kunit *test)
> > > +{
> > > +       struct clk_gate_test_context *ctx = test->priv;
> > > +
> > > +       clk_hw_unregister_gate(ctx->hw);
> > > +       clk_hw_unregister_fixed_rate(ctx->parent);
> > > +       kfree(ctx);
> >
> > I think we forgot to drop some kfree's now that we're using
> > test->priv = ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> >
> > This will result in duplicate kfree's.
>
> Good catch! I forgot that it was a managed allocation like that. Doesn't
> help that I had to go all the way to the kunit_kmalloc_array()
> kernel-doc to read that test managed means it is freed when the test is
> done.

Good point.
We should at least have kunit_kmalloc() doc comments make that clear.

>
> > > +
> > > +static void clk_gate_test_is_enabled(struct kunit *test)
> > > +{
> > > +       struct clk_hw *hw;
> > > +       struct clk_gate_test_context *ctx;
> > > +
> > > +       ctx = clk_gate_test_alloc_ctx(test);
> > > +       ctx->fake_reg = BIT(7);
> > > +       hw = clk_hw_register_gate(NULL, "test_gate", NULL, 0, ctx->fake_mem, 7,
> > > +                                 0, NULL);
> > > +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, hw);
> > > +       KUNIT_ASSERT_TRUE(test, clk_hw_is_enabled(hw));
> > > +
> > > +       clk_hw_unregister_gate(hw);
> > > +       kfree(ctx);
> >
> > ditto
> >
> >
>
> Thanks! Next time can you trim your mails? It's quite long and I have to
> scroll down to find everything.

Thanks for the tip.
In the past I was wary of doing so since gmail can behave pretty
erratically when composing plain-text emails (it randomly jumps back
to the top at times, sometimes while typing???).
But I'll keep that in mind going forward. Hopefully having less text
will make gmail less buggy too...