Re: [PATCH v7 5/6] spi: cs42l43: Add SPI controller support

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: broonie@kernel.org, lee@kernel.org, robh+dt@kernel.org,
	 krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	 linus.walleij@linaro.org, vkoul@kernel.org, lgirdwood@gmail.com,
	 yung-chuan.liao@linux.intel.com, sanyog.r.kale@intel.com,
	 pierre-louis.bossart@linux.intel.com,
	alsa-devel@alsa-project.org,  patches@opensource.cirrus.com,
	devicetree@vger.kernel.org,  linux-gpio@vger.kernel.org,
	linux-spi@vger.kernel.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 5/6] spi: cs42l43: Add SPI controller support
Date: Fri, 19 Jan 2024 18:09:28 +0200	[thread overview]
Message-ID: <CAHp75Vf0BdOj_Bcxs3L=aznUzoMptPF+tDBpOcBKOcVTH45+Hg@mail.gmail.com> (raw)
In-Reply-To: <20240119114917.GB16899@ediswmail.ad.cirrus.com>

On Fri, Jan 19, 2024 at 1:49 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
> On Thu, Jan 18, 2024 at 07:06:13PM +0200, andy.shevchenko@gmail.com wrote:

...

> > > +   if (is_of_node(fwnode))
> > > +           fwnode = fwnode_get_named_child_node(fwnode, "spi");
> >
> > You can actually drop these is_of_node() tests and use another variable. In
> > ACPI there can't be child node in small letters.
>
> is_of_node feels pretty clear what the intent is, rather than
> relying on nodes not existing etc.
>
> > But main problem here (and in another driver where the similar is used) that
> > you bumped reference count for fwnode. I haven't seen where you drop it back.
> > Have you tested rmmod/modprobe in a loop?
>
> Yeah it should drop the reference will add that.

Note, this will require an additional variable anyway (as in the
infamous `x = realloc(x...)` mistake).

-- 
With Best Regards,
Andy Shevchenko