Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 23 Dec 2022 18:25:18 +0300
From:   Dan Carpenter <error27@gmail.com>
To:     Stefan Wahren <stefan.wahren@i2se.com>
Cc:     Umang Jain <umang.jain@ideasonboard.com>,
        linux-staging@lists.linux.dev,
        linux-rpi-kernel@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org,
        linux-kernel@vger.kernel.org, Kees Cook <keescook@chromium.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Adrien Thierry <athierry@redhat.com>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        Kieran Bingham <kieran.bingham@ideasonboard.com>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH v2 5/7] vc04_services: vchiq_arm: Drop VCHIQ_RETRY usage
 on disconnect
Message-ID: <Y6XIXuccQD1RuBty@kadam>
References: <20221219115725.11992-1-umang.jain@ideasonboard.com>
 <20221219115725.11992-6-umang.jain@ideasonboard.com>
 <8fcbdf54-98ae-6a06-ecaf-591a43b863a1@i2se.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8fcbdf54-98ae-6a06-ecaf-591a43b863a1@i2se.com>
Precedence: bulk

On Thu, Dec 22, 2022 at 12:07:56PM +0100, Stefan Wahren wrote:
> Hi Umang,
> 
> Am 19.12.22 um 12:57 schrieb Umang Jain:
> > Drop the usage of VCHIQ_RETRY when the vchiq has connection status
> > VCHIQ_CONNSTATE_DISCONNECTED. Disconnected status will not be valid to
> > carry on a retry, replace the VCHIQ_RETRY with -ENOTCONN.
> > 
> > This patch removes the usage of vCHIQ_RETRY completely and act as
> > intermediatory to address the TODO item:
> > 	* Get rid of custom function return values
> > for vc04_services/interface.
> > 
> > Fixes: 71bad7f08641 ("staging: add bcm2708 vchiq driver")
> please drop this fixes tag since this commit doesn't fix a real issue and
> also shouldn't be applied to stable.

I asked Umang to add the Fixes tag based on the patch description and
based on that it seemed like a behavior change just from looking at
the patch.

But actually you are right that the fixes tag is not required.

The vchiq_send_remote_use() function is never called (dead code).  The
vchiq_send_remote_use_active() function now returns a mix of custom
error codes and negative error codes.  (Ugly).  The caller only tests
for VCHIQ_SUCCESS so this code does not change behavior.  I really feel
like the commit description needs to be clearer on this.

The fixes tag is not really about stable.  Stable uses the tag, but the
fixes tag is appropriate whenever there is a bug fix.

regards,
dan carpenter