Received-SPF: pass (google.com: domain of linux-kernel+bounces-214849-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <104f23d7-9832-4c06-92c7-d3ea84fbf186@gmx.net>
Date: Fri, 14 Jun 2024 13:52:53 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] staging: vc04_services: vchiq_arm: Fix initialisation
 check
To: Kieran Bingham <kieran.bingham@ideasonboard.com>,
 Florian Fainelli <florian.fainelli@broadcom.com>,
 Umang Jain <umang.jain@ideasonboard.com>,
 linux-rpi-kernel@lists.infradead.org
Cc: Broadcom internal kernel review list
 <bcm-kernel-feedback-list@broadcom.com>,
 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
 Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
 Dave Stevenson <dave.stevenson@raspberrypi.org>, detule
 <ogjoneski@gmail.com>, Dan Carpenter <dan.carpenter@linaro.org>,
 BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
 <linux-arm-kernel@lists.infradead.org>,
 STAGING SUBSYSTEM <linux-staging@lists.linux.dev>,
 open list <linux-kernel@vger.kernel.org>
References: <20240613194150.2915202-1-kieran.bingham@ideasonboard.com>
 <20240613194150.2915202-2-kieran.bingham@ideasonboard.com>
 <082d9df0-0947-4452-a3fc-87eab2019e01@gmx.net>
 <171836496531.2248009.11650291484570726735@ping.linuxembedded.co.uk>
Content-Language: en-US
From: Stefan Wahren <wahrenst@gmx.net>
In-Reply-To: <171836496531.2248009.11650291484570726735@ping.linuxembedded.co.uk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:IXSy47jmsJA=;XxNo+wybPSb96wXOawF84Ikkh0p
 Rxq85Y5W1LmlQ75ZCF9PeU1SS2wWfKQMNXGf42CYSTlhVWdvOLCJbv2fb3YP3EY4He+OycHuy
 lOM8LIF/YpXDy3WjGtmx03wFUMPeas9OK7kx+g6SUVNlG7tHTRmvrxuXXaMz/HRwgd8NTfGH3
 bOfem205pxAzu7yigz32OckVyvS7B3Trw7pkLoIN/152O6BoqcyQB5dPMyFtwM9Q/gZ0i3Rqo
 yCVBsF87VeBHrFJfzqn+JTncXlbXvHZ1/i7ypDibf6o6kJ+GdLUtbiaW7XbXcqt64RIA//xFC
 y/jJ8nQb0ppv+3qDpy5zwh8RPdQ8nO+ZfGO3Y/15VMBqGJ1sgiWQXImyqcC2UQ9SOfbeKyIUr
 vW3bW5oqXdOe2N+GVd4qzcRAFMqw2OeXiehLmD9ErcWr799OriQsdhpHO9LhN/9laFD2jb1ZI
 bhe1+S5kUUvs3xYOthYFSHw6OTvdHZ9vTHt58hMZ0Vq1OBW/J6baViuL1TcqHo4sTzRrxsS+x
 k9EtI5800/fxANiC1UAi8KocV0oeN54XtoevGFKkriB1nbB0VCikWr88rXeZDbSeu1FJKq+78
 b0KuD4ozxrMz02vqhiXnTjH08kdh1gykQSfLlR/b3IZi3xk955MHI2zqZId2labtlCKK9YEKR
 /nOecab2yKKLHBRj0d9BC65u5BdVU9j+4G8DRoaQG1J6PpsQ4jKx+qdOCB5R4ldMlWQmloLSr
 SRnvqX8rhCzRF3zYQqaEWX/4vWCKE6SMCXpmL4/numP8be0dQwYpdGnpUm0Wwxsb6FJIE58WU
 x45/5ZQVFnDiSFgl4NQmpYGy2gHdmP1iuCfEGQ4A3x8ts=

Hi Kieran,

Am 14.06.24 um 13:36 schrieb Kieran Bingham:
> Hi Stefan,
>
> Sorry, indeed I completely missed this mail.
>
> Quoting Stefan Wahren (2024-06-13 21:01:42)
>> Hi Kieran,
>>
>> Am 13.06.24 um 21:41 schrieb Kieran Bingham:
>>> The vchiq_state used to be obtained through an accessor
>>> which would validate that the VCHIQ had been initialised
>>> correctly with the remote.
>>>
>>> In commit 42a2f6664e18 ("staging: vc04_services: Move global g_state t=
o
>>> vchiq_state") the global state was moved to the vchiq_mgnt structures
>>> stored as a vchiq instance specific context. This conversion removed t=
he
>>> helpers and instead replaced users of this helper with the assumption
>>> that the state is always available and the remote connected.
>>>
>>> Fix this broken assumption by re-introducing the logic that was lost
>>> during the conversion.
>> thank you for sending this patch. Maybe it's worth to mention that this
>> patch also drop some unnecessary NULL checks of state.
> I don't understand this comment. Nothing is dropped is it?
>
> The newly added vchiq_remote_initialised() is itself a null-check too!
the vchiq_remote_initialised() only checks the member remote, but not
state itself. From my point of view the null-check for state is
unnecessary, because most of the code already assumed that state is not
null like e.g. in vchiq_open().