| Message ID | 20240910051007.297227-1-umang.jain@ideasonboard.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | staging: vchiq_core: Remove unused function argument | expand |
Oops, messed up cover letter 'Subject' It should be : [PATCH v6 0/7] staging: vchiq_core: Refactor vchiq_bulk_transfer() logic On 10/09/24 10:40 am, Umang Jain wrote: > Series mostly refactors vchiq_bulk_transfer() code according > to Arnd's review comment from [1]: > >> You can also wrap vchiq_bulk_transfer() in order to have >> four separate functions based on the different 'mode' >> values and have them only take the arguments they actually >> need. > I didn't wrap vchiq_bulk_transfer(), instead created four > differnet function, one for each mode. > This will pave the way to address his second comment: > >> Ideally this should be cleaned up in a way that completely >> avoids passing both user and kernel data at the same time. > which is not part of this series and will be done on top as > arguments shuffling will have to fix the sparse warnings > that exists currently. > > Patch 1/7 first moves the VCHIQ_BULK_MODE_WAITING logic out > of vchiq_bulk_transfer > > Patch 2/7 then moves the core logic to vchiq_bulk_transfer() > which can be shared in subsequent patches > > Patch 3/7 and 4/7 refactors remaining bulk transfer modes > > Patch 5/7 finally drops the vchiq_bulk_transfer() as not needed > > patch 6/7 and 7/7 are drive by patches, noticed during development. > > [1]: https://lore.kernel.org/linux-staging/3d3b7368-93b2-4c0d-845e-4099c2de9dc1@app.fastmail.com/ > > Changes in v6: > - Drop forward declaration vchiq_bulk_xfer_queue_msg_interruptible() > in 2/7 > > changes in v5: > - Fix regression for VCHIQ_BULK_MODE_BLOCKING in 3/7 > (Fixes strange hang from v4 with 'vchiq_test -p') > > Changes in v4: > - In 1/7 return 0 direcly instead of assigning status = 0 > - In 3/7, fix null assignment of bulk waiter > > CHanges in v3: > - Directly return if service == NULL in 1/7 and 4/7 > - Drop set-but-unused variable from 1/7 > > Changes in v2: > - Directly return if service == NULL in 3/7 > - Prefix non-static functions with "vchiq_" (appropriate driver prefix) > - Use typed argument for 'userdata' in 1/7 > > > Umang Jain (7): > staging: vchiq: Factor out bulk transfer for VCHIQ_BULK_MODE_WAITING > staging: vchiq_core: Simplify vchiq_bulk_transfer() > staging: vchiq_core: Factor out bulk transfer for blocking mode > staging: vchiq_core: Factor out bulk transfer for (no/)callback mode > staging: vchiq_core: Drop vchiq_bulk_transfer() > staging: vchiq_core: Remove unused function argument > staging: vchiq_core: Pass enumerated flag instead of int > > .../interface/vchiq_arm/vchiq_arm.c | 20 +- > .../interface/vchiq_arm/vchiq_core.c | 323 +++++++++++------- > .../interface/vchiq_arm/vchiq_core.h | 16 +- > .../interface/vchiq_arm/vchiq_dev.c | 15 +- > 4 files changed, 232 insertions(+), 142 deletions(-) >
Am 10.09.24 um 07:10 schrieb Umang Jain: > Series mostly refactors vchiq_bulk_transfer() code according > to Arnd's review comment from [1]: > >> You can also wrap vchiq_bulk_transfer() in order to have >> four separate functions based on the different 'mode' >> values and have them only take the arguments they actually >> need. > I didn't wrap vchiq_bulk_transfer(), instead created four > differnet function, one for each mode. > This will pave the way to address his second comment: > >> Ideally this should be cleaned up in a way that completely >> avoids passing both user and kernel data at the same time. > which is not part of this series and will be done on top as > arguments shuffling will have to fix the sparse warnings > that exists currently. > > Patch 1/7 first moves the VCHIQ_BULK_MODE_WAITING logic out > of vchiq_bulk_transfer > > Patch 2/7 then moves the core logic to vchiq_bulk_transfer() > which can be shared in subsequent patches > > Patch 3/7 and 4/7 refactors remaining bulk transfer modes > > Patch 5/7 finally drops the vchiq_bulk_transfer() as not needed > > patch 6/7 and 7/7 are drive by patches, noticed during development. > > [1]: https://lore.kernel.org/linux-staging/3d3b7368-93b2-4c0d-845e-4099c2de9dc1@app.fastmail.com/ The whole series is Tested-by: Stefan Wahren <wahrenst@gmx.net> > > Changes in v6: > - Drop forward declaration vchiq_bulk_xfer_queue_msg_interruptible() > in 2/7 > > changes in v5: > - Fix regression for VCHIQ_BULK_MODE_BLOCKING in 3/7 > (Fixes strange hang from v4 with 'vchiq_test -p') > > Changes in v4: > - In 1/7 return 0 direcly instead of assigning status = 0 > - In 3/7, fix null assignment of bulk waiter > > CHanges in v3: > - Directly return if service == NULL in 1/7 and 4/7 > - Drop set-but-unused variable from 1/7 > > Changes in v2: > - Directly return if service == NULL in 3/7 > - Prefix non-static functions with "vchiq_" (appropriate driver prefix) > - Use typed argument for 'userdata' in 1/7 > > > Umang Jain (7): > staging: vchiq: Factor out bulk transfer for VCHIQ_BULK_MODE_WAITING > staging: vchiq_core: Simplify vchiq_bulk_transfer() > staging: vchiq_core: Factor out bulk transfer for blocking mode > staging: vchiq_core: Factor out bulk transfer for (no/)callback mode > staging: vchiq_core: Drop vchiq_bulk_transfer() > staging: vchiq_core: Remove unused function argument > staging: vchiq_core: Pass enumerated flag instead of int > > .../interface/vchiq_arm/vchiq_arm.c | 20 +- > .../interface/vchiq_arm/vchiq_core.c | 323 +++++++++++------- > .../interface/vchiq_arm/vchiq_core.h | 16 +- > .../interface/vchiq_arm/vchiq_dev.c | 15 +- > 4 files changed, 232 insertions(+), 142 deletions(-) >