| Message ID | 20221219115725.11992-1-umang.jain@ideasonboard.com |
|---|---|
| Headers | show |
| Series | staging: vc04_services: Remove custom return values | expand |
Am 19.12.22 um 12:57 schrieb Umang Jain: > The series removes the custom return values from functions > and replaces them with linux error codes. This address the TODO > vchiq interface: > * Get rid of custom function return values > > Changes in V2: > - Patch 3/7 now reports for specific errors like -ENOMEM, -EHOSTDOWN > - Patch 5/7 reports -ENOTCONN instead of -EINVAL and adds a "Fixes" tag > > Umang Jain (7): > staging: vc04_services: Replace vchiq_status return type to int > staging: vc04_services: Drop VCHIQ_SUCCESS usage > staging: vc04_services: Drop VCHIQ_ERROR usage > staging: vc04_services: Drop VCHIQ_RETRY usage > vc04_services: vchiq_arm: Drop VCHIQ_RETRY usage on disconnect > staging: vc04_services: Drop enum vchiq_status remnants > staging: vc04_services: vchiq: Drop custom return values from TODO The whole series is: Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
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);
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