2022-03-25 17:18:31

by Jakob Koschel

[permalink] [raw]
Subject: [PATCH] soc: ti: replace usage of found with dedicated list iterator variable

To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.

To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].

This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/
Signed-off-by: Jakob Koschel <[email protected]>
---
drivers/soc/ti/knav_dma.c | 26 ++++++++++++--------------
drivers/soc/ti/knav_qmss_queue.c | 16 +++++++---------
2 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/drivers/soc/ti/knav_dma.c b/drivers/soc/ti/knav_dma.c
index 700d8eecd8c4..7e126a73e56e 100644
--- a/drivers/soc/ti/knav_dma.c
+++ b/drivers/soc/ti/knav_dma.c
@@ -415,9 +415,8 @@ static int of_channel_match_helper(struct device_node *np, const char *name,
void *knav_dma_open_channel(struct device *dev, const char *name,
struct knav_dma_cfg *config)
{
- struct knav_dma_chan *chan;
- struct knav_dma_device *dma;
- bool found = false;
+ struct knav_dma_device *dma = NULL, *iter1;
+ struct knav_dma_chan *chan = NULL, *iter2;
int chan_num = -1;
const char *instance;

@@ -444,33 +443,32 @@ void *knav_dma_open_channel(struct device *dev, const char *name,
}

/* Look for correct dma instance */
- list_for_each_entry(dma, &kdev->list, list) {
- if (!strcmp(dma->name, instance)) {
- found = true;
+ list_for_each_entry(iter1, &kdev->list, list) {
+ if (!strcmp(iter1->name, instance)) {
+ dma = iter1;
break;
}
}
- if (!found) {
+ if (!dma) {
dev_err(kdev->dev, "No DMA instance with name %s\n", instance);
return (void *)-EINVAL;
}

/* Look for correct dma channel from dma instance */
- found = false;
- list_for_each_entry(chan, &dma->chan_list, list) {
+ list_for_each_entry(iter2, &dma->chan_list, list) {
if (config->direction == DMA_MEM_TO_DEV) {
- if (chan->channel == chan_num) {
- found = true;
+ if (iter2->channel == chan_num) {
+ chan = iter2;
break;
}
} else {
- if (chan->flow == chan_num) {
- found = true;
+ if (iter2->flow == chan_num) {
+ chan = iter2;
break;
}
}
}
- if (!found) {
+ if (!chan) {
dev_err(kdev->dev, "channel %d is not in DMA %s\n",
chan_num, instance);
return (void *)-EINVAL;
diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index 2ac3856b8d42..4dbaa8c3636c 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -758,10 +758,9 @@ void *knav_pool_create(const char *name,
int num_desc, int region_id)
{
struct knav_region *reg_itr, *region = NULL;
- struct knav_pool *pool, *pi;
+ struct knav_pool *pool, *pi = NULL, *iter;
struct list_head *node;
unsigned last_offset;
- bool slot_found;
int ret;

if (!kdev)
@@ -816,18 +815,17 @@ void *knav_pool_create(const char *name,
* the request
*/
last_offset = 0;
- slot_found = false;
node = &region->pools;
- list_for_each_entry(pi, &region->pools, region_inst) {
- if ((pi->region_offset - last_offset) >= num_desc) {
- slot_found = true;
+ list_for_each_entry(iter, &region->pools, region_inst) {
+ if ((iter->region_offset - last_offset) >= num_desc) {
+ pi = iter;
break;
}
- last_offset = pi->region_offset + pi->num_desc;
+ last_offset = iter->region_offset + iter->num_desc;
}
- node = &pi->region_inst;

- if (slot_found) {
+ if (pi) {
+ node = &pi->region_inst;
pool->region = region;
pool->num_desc = num_desc;
pool->region_offset = last_offset;

base-commit: f443e374ae131c168a065ea1748feac6b2e76613
--
2.25.1


2022-03-26 17:09:31

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH] soc: ti: replace usage of found with dedicated list iterator variable



On 24/03/22 12:55 pm, Jakob Koschel wrote:
> To move the list iterator variable into the list_for_each_entry_*()
> macro in the future it should be avoided to use the list iterator
> variable after the loop body.
>
> To *never* use the list iterator variable after the loop it was
> concluded to use a separate iterator variable instead of a
> found boolean [1].
>
> This removes the need to use a found variable and simply checking if
> the variable was set, can determine if the break/goto was hit.
>
> Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/
> Signed-off-by: Jakob Koschel <[email protected]>
> ---

Reviewed-by: Vignesh Raghavendra <[email protected]>

[...]

Regards
Vignesh

2022-04-12 12:48:27

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH] soc: ti: replace usage of found with dedicated list iterator variable

Hi Jakob Koschel,

On Thu, 24 Mar 2022 08:25:03 +0100, Jakob Koschel wrote:
> To move the list iterator variable into the list_for_each_entry_*()
> macro in the future it should be avoided to use the list iterator
> variable after the loop body.
>
> To *never* use the list iterator variable after the loop it was
> concluded to use a separate iterator variable instead of a
> found boolean [1].
>
> [...]

I have applied the following to branch ti-drivers-soc-next on [1].
Thank you!

[1/1] soc: ti: replace usage of found with dedicated list iterator variable
commit: d281a982c2693c6a7bffaa1ae1ff3b400d6e6c74

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent up the chain during
the next merge window (or sooner if it is a relevant bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

[1] git://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D