| Message ID | 20221219115725.11992-6-umang.jain@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: vc04_services: Remove custom return values | expand |
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. > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c > index 9c64d5de810e..ddb6d0f4daed 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c > @@ -3641,7 +3641,7 @@ vchiq_loud_error_footer(void) > int vchiq_send_remote_use(struct vchiq_state *state) > { > if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) > - return VCHIQ_RETRY; > + return -ENOTCONN; > > return queue_message(state, NULL, MAKE_REMOTE_USE, NULL, NULL, 0, 0); > } > @@ -3649,7 +3649,7 @@ int vchiq_send_remote_use(struct vchiq_state *state) > int vchiq_send_remote_use_active(struct vchiq_state *state) > { > if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) > - return VCHIQ_RETRY; > + return -ENOTCONN; > > return queue_message(state, NULL, MAKE_REMOTE_USE_ACTIVE, > NULL, NULL, 0, 0);
Hi Stefan, On 12/22/22 4:37 PM, 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. Should I send a v3 of the series with updated commit message or can you drop the tag while applying? Other option would be to send v2.1 --in-reply-to this patch. I am fine with anything as long as it aligns with the merging workflow. Thanks, Umang >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git >> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c >> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c >> index 9c64d5de810e..ddb6d0f4daed 100644 >> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c >> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c >> @@ -3641,7 +3641,7 @@ vchiq_loud_error_footer(void) >> int vchiq_send_remote_use(struct vchiq_state *state) >> { >> if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) >> - return VCHIQ_RETRY; >> + return -ENOTCONN; >> return queue_message(state, NULL, MAKE_REMOTE_USE, NULL, >> NULL, 0, 0); >> } >> @@ -3649,7 +3649,7 @@ int vchiq_send_remote_use(struct vchiq_state >> *state) >> int vchiq_send_remote_use_active(struct vchiq_state *state) >> { >> if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) >> - return VCHIQ_RETRY; >> + return -ENOTCONN; >> return queue_message(state, NULL, MAKE_REMOTE_USE_ACTIVE, >> NULL, NULL, 0, 0);
Hi Umang, Am 22.12.22 um 13:04 schrieb Umang Jain: > Hi Stefan, > > On 12/22/22 4:37 PM, 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. > > Should I send a v3 of the series with updated commit message or can > you drop the tag while applying? Greg is the maintainer of staging, so i cannot decide. But i would prefer a v3 with my tested tag added and this fixes tag dropped. Best regards > > Other option would be to send v2.1 --in-reply-to this patch. I am > fine with anything as long as it aligns with the merging workflow. > > Thanks, > Umang >>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >>> --- >>> .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git >>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c >>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c >>> index 9c64d5de810e..ddb6d0f4daed 100644 >>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c >>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c >>> @@ -3641,7 +3641,7 @@ vchiq_loud_error_footer(void) >>> int vchiq_send_remote_use(struct vchiq_state *state) >>> { >>> if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) >>> - return VCHIQ_RETRY; >>> + return -ENOTCONN; >>> return queue_message(state, NULL, MAKE_REMOTE_USE, NULL, >>> NULL, 0, 0); >>> } >>> @@ -3649,7 +3649,7 @@ int vchiq_send_remote_use(struct vchiq_state >>> *state) >>> int vchiq_send_remote_use_active(struct vchiq_state *state) >>> { >>> if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) >>> - return VCHIQ_RETRY; >>> + return -ENOTCONN; >>> return queue_message(state, NULL, MAKE_REMOTE_USE_ACTIVE, >>> NULL, NULL, 0, 0); >
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
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index 9c64d5de810e..ddb6d0f4daed 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -3641,7 +3641,7 @@ vchiq_loud_error_footer(void) int vchiq_send_remote_use(struct vchiq_state *state) { if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) - return VCHIQ_RETRY; + return -ENOTCONN; return queue_message(state, NULL, MAKE_REMOTE_USE, NULL, NULL, 0, 0); } @@ -3649,7 +3649,7 @@ int vchiq_send_remote_use(struct vchiq_state *state) int vchiq_send_remote_use_active(struct vchiq_state *state) { if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) - return VCHIQ_RETRY; + return -ENOTCONN; return queue_message(state, NULL, MAKE_REMOTE_USE_ACTIVE, NULL, NULL, 0, 0);
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") Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)