Received-SPF: pass (google.com: domain of linux-kernel+bounces-31345-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20230804104602.395892-1-ckeepax@opensource.cirrus.com>
 <20230804104602.395892-6-ckeepax@opensource.cirrus.com> <ZalahZkCrBm-BXwz@surfacebook.localdomain>
 <20240119114917.GB16899@ediswmail.ad.cirrus.com>
In-Reply-To: <20240119114917.GB16899@ediswmail.ad.cirrus.com>
From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Fri, 19 Jan 2024 18:09:28 +0200
Message-ID: <CAHp75Vf0BdOj_Bcxs3L=aznUzoMptPF+tDBpOcBKOcVTH45+Hg@mail.gmail.com>
Subject: Re: [PATCH v7 5/6] spi: cs42l43: Add SPI controller support
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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Jan 19, 2024 at 1:49=E2=80=AFPM 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 =3D 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 =3D realloc(x...)` mistake).

--=20
With Best Regards,
Andy Shevchenko