Re: [PATCH 02/14] dmaengine: dma-jz4780: Separate chan/ctrl registers

[Paul Cercueil <paul@crapouillou.net>]

Hi Paul,

> Hi Paul,
> 
> On Tue, Jul 03, 2018 at 02:32:02PM +0200, Paul Cercueil wrote:
>>  @@ -804,9 +818,19 @@ static int jz4780_dma_probe(struct 
>> platform_device *pdev)
>>   		return -EINVAL;
>>   	}
>> 
>>  -	jzdma->base = devm_ioremap_resource(dev, res);
>>  -	if (IS_ERR(jzdma->base))
>>  -		return PTR_ERR(jzdma->base);
>>  +	jzdma->chn_base = devm_ioremap_resource(dev, res);
>>  +	if (IS_ERR(jzdma->chn_base))
>>  +		return PTR_ERR(jzdma->chn_base);
>>  +
>>  +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>  +	if (!res) {
>>  +		dev_err(dev, "failed to get I/O memory\n");
>>  +		return -EINVAL;
>>  +	}
>>  +
>>  +	jzdma->ctrl_base = devm_ioremap_resource(dev, res);
>>  +	if (IS_ERR(jzdma->ctrl_base))
>>  +		return PTR_ERR(jzdma->ctrl_base);
> 
> Could we have this failure case fall back to the existing behaviour
> where we only have a single resource covering all the registers? That
> would avoid breaking bisection between this patch & the one that 
> updates
> the JZ4780 DT.
> 
> For example:
> 
> 	#define JZ4780_DMA_CTRL_OFFSET	0x1000
> 
> 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> 	if (res) {
> 		jzdma->ctrl_base = devm_ioremap_resource(dev, res);
> 		if (IS_ERR(jzdma->ctrl_base))
> 			return PTR_ERR(jzdma->ctrl_base);
> 	} else {
> 		jzdma->ctrl_base = jzdma->chn_base + JZ4780_DMA_CTRL_OFFSET;
> 	}
> 
> Then you could remove the fallback after patch 13, to end up with the
> same code you have now but without breaking bisection.
> 
> Most correct might be to move patch 13 to right after this one, so 
> that
> the JZ4780-specific fallback can be removed before adding support for
> any of the other SoCs.

Sure, I can do that for the V2.

Thanks,
-Paul