Hi Wolfram Sang,
Thanks for the patch.
> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> Sent: Monday, September 30, 2024 4:00 PM
> Subject: [PATCH 1/3] dmaengine: sh: rz-dmac: handle configs where one address is zero
>
> Configs like the ones coming from the MMC subsystem will have either 'src' or 'dst' zeroed, resulting
> in an unknown bus width. This will bail out on the RZ DMA driver because of the sanity check for a
> valid bus width. Reorder the code, so that the check will only be applied when the corresponding
> address is non-zero.
>
> Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC")
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/dma/sh/rz-dmac.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 65a27c5a7bce..811389fc9cb8
> 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -601,22 +601,25 @@ static int rz_dmac_config(struct dma_chan *chan,
> struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> u32 val;
>
> - channel->src_per_address = config->src_addr;
> channel->dst_per_address = config->dst_addr;
> -
> - val = rz_dmac_ds_to_val_mapping(config->dst_addr_width);
> - if (val == CHCFG_DS_INVALID)
> - return -EINVAL;
> -
> channel->chcfg &= ~CHCFG_FILL_DDS_MASK;
> - channel->chcfg |= FIELD_PREP(CHCFG_FILL_DDS_MASK, val);
> + if (channel->dst_per_address) {
> + val = rz_dmac_ds_to_val_mapping(config->dst_addr_width);
> + if (val == CHCFG_DS_INVALID)
> + return -EINVAL;
>
> - val = rz_dmac_ds_to_val_mapping(config->src_addr_width);
> - if (val == CHCFG_DS_INVALID)
> - return -EINVAL;
> + channel->chcfg |= FIELD_PREP(CHCFG_FILL_DDS_MASK, val);
> + }
>
> + channel->src_per_address = config->src_addr;
> channel->chcfg &= ~CHCFG_FILL_SDS_MASK;
> - channel->chcfg |= FIELD_PREP(CHCFG_FILL_SDS_MASK, val);
> + if (channel->src_per_address) {
> + val = rz_dmac_ds_to_val_mapping(config->src_addr_width);
> + if (val == CHCFG_DS_INVALID)
> + return -EINVAL;
> +
> + channel->chcfg |= FIELD_PREP(CHCFG_FILL_SDS_MASK, val);
> + }
Now both code paths are identical, not sure may be introducing a helper
by passing channel, CHCFG_FILL_*_MASK and *_addr_width
will save some code??
Anyway, current code LGTM. So,
Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
Cheers,
Biju
>
> return 0;
> }
> --
> 2.45.2
