Re: [PATCH v5 07/17] dmaengine: sh: rz-dmac: Add helper to check if the channel is enabled

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Incomplete refactoring: the `rz_dmac_chan_is_enabled` helper is not used in `rz_dmac_enable_hw()`.
- [Low] Parameter naming convention mismatch in `rz_dmac_chan_is_enabled()`.
- [Low] Redundant double-negation operator (`!!`) used with a `bool` return type.
--

commit c927e2bf558812607e323c4466ef5f913dc31b55
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

dmaengine: sh: rz-dmac: Add helper to check if the channel is enabled

This commit adds a helper function to check if a DMA channel is enabled.
The new helper is then used to refactor the device pause operation.

> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index d3926ecd63ac1..c7337cf271369 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -279,6 +279,13 @@ static void rz_dmac_lmdesc_recycle(struct rz_dmac_chan *channel)
>  	channel->lmdesc.head = lmdesc;
>  }
>  
> +static bool rz_dmac_chan_is_enabled(struct rz_dmac_chan *chan)

This isn't a bug, but could we use "channel" instead of "chan" for the
parameter name?

Throughout the rest of the driver, "chan" seems reserved for
struct dma_chan *, while "channel" is used for struct rz_dmac_chan *.

> +{
> +	u32 val = rz_dmac_ch_readl(chan, CHSTAT, 1);
> +
> +	return !!(val & CHSTAT_EN);

Is the double negation necessary here?

Since the function returns a bool, the compiler will automatically convert
any non-zero value to true, so returning val & CHSTAT_EN should be
sufficient.

> +}
> +
>  static void rz_dmac_enable_hw(struct rz_dmac_chan *channel)

Could the new helper be used here in rz_dmac_enable_hw() as well?

Looking at rz_dmac_enable_hw(), it has an identical open-coded check:

static void rz_dmac_enable_hw(struct rz_dmac_chan *channel)
{
        ...
        chstat = rz_dmac_ch_readl(channel, CHSTAT, 1);
        if (!(chstat & CHSTAT_EN)) {
        ...

Using the helper there would allow removing the local chstat variable and
improve consistency.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512121219.216159-1-claudiu.beznea.uj@bp.renesas.com?part=7