Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <099c1084-4093-860b-587d-2942a5da7e6c@ti.com>
Date:   Mon, 12 Dec 2022 10:21:22 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [PATCH 5/5] dmaengine: ti: k3-udma: Add support for BCDMA CSI RX
Content-Language: en-US
To:     =?UTF-8?Q?P=c3=a9ter_Ujfalusi?= <peter.ujfalusi@gmail.com>,
        Vinod Koul <vkoul@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
CC:     <dmaengine@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20221206043554.1521522-1-vigneshr@ti.com>
 <20221206043554.1521522-6-vigneshr@ti.com>
 <46e1321e-ff27-7b95-6f93-975d08802626@gmail.com>
From:   Vignesh Raghavendra <vigneshr@ti.com>
In-Reply-To: <46e1321e-ff27-7b95-6f93-975d08802626@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 10/12/22 14:19, Péter Ujfalusi wrote:
> 
> 
> On 12/6/22 06:35, Vignesh Raghavendra wrote:
>> BCDMA CSI RX present on AM62Ax SoC is a dedicated DMA for servicing
>> Camera Serial Interface (CSI) IP. Add support for the same.
>>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> ---
>>  drivers/dma/ti/k3-udma.c | 37 ++++++++++++++++++++++++++++++++-----
>>  1 file changed, 32 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
>> index 19fce52a9b53..a8b497ed3f30 100644
>> --- a/drivers/dma/ti/k3-udma.c
>> +++ b/drivers/dma/ti/k3-udma.c
>> @@ -135,6 +135,7 @@ struct udma_match_data {
>>  	u32 flags;
>>  	u32 statictr_z_mask;
>>  	u8 burst_size[3];
>> +	struct udma_soc_data *soc_data;
>>  };
>>  
>>  struct udma_soc_data {
>> @@ -4295,6 +4296,25 @@ static struct udma_match_data j721e_mcu_data = {
>>  	},
>>  };
>>  
>> +static struct udma_soc_data am62a_dmss_csi_soc_data = {
>> +	.oes = {
>> +		.bcdma_rchan_data = 0xe00,
>> +		.bcdma_rchan_ring = 0x1000,
>> +	},
>> +};
>> +
>> +static struct udma_match_data am62a_bcdma_csirx_data = {
>> +	.type = DMA_TYPE_BCDMA,
>> +	.psil_base = 0x3100,
>> +	.enable_memcpy_support = false,
>> +	.burst_size = {
>> +		TI_SCI_RM_UDMAP_CHAN_BURST_SIZE_64_BYTES, /* Normal Channels */
>> +		0, /* No H Channels */
>> +		0, /* No UH Channels */
>> +	},
>> +	.soc_data = &am62a_dmss_csi_soc_data,
>> +};
>> +
>>  static struct udma_match_data am64_bcdma_data = {
>>  	.type = DMA_TYPE_BCDMA,
>>  	.psil_base = 0x2000, /* for tchan and rchan, not applicable to bchan */
>> @@ -4344,6 +4364,10 @@ static const struct of_device_id udma_of_match[] = {
>>  		.compatible = "ti,am64-dmss-pktdma",
>>  		.data = &am64_pktdma_data,
>>  	},
>> +	{
>> +		.compatible = "ti,am62a-dmss-bcdma-csirx",
>> +		.data = &am62a_bcdma_csirx_data,
>> +	},
>>  	{ /* Sentinel */ },
>>  };
>>  
>> @@ -5272,12 +5296,15 @@ static int udma_probe(struct platform_device *pdev)
>>  	}
>>  	ud->match_data = match->data;
>>  
>> -	soc = soc_device_match(k3_soc_devices);
>> -	if (!soc) {
>> -		dev_err(dev, "No compatible SoC found\n");
>> -		return -ENODEV;
>> +	ud->soc_data = ud->match_data->soc_data;
>> +	if (!ud->soc_data) {
>> +		soc = soc_device_match(k3_soc_devices);
>> +		if (!soc) {
>> +			dev_err(dev, "No compatible SoC found\n");
>> +			return -ENODEV;
>> +		}
>> +		ud->soc_data = soc->data;
>>  	}
>> -	ud->soc_data = soc->data;
> 
> Right, the original design was based on the promise that a DMSS will
> contain maximum 1 BCDMA and/or 1 PKTDMA, looks like now a DMSS have 2
> BCDMAs?
> 

Thats correct. IPs requiring dedicated B/W paths (its just CSI today)
would probably have dedicated BCDMA.

> The only possible issue I can see is that if in future SoCs the Output
> Event Offsets got shuffled for the BCDMAs, but then a new compatible for
> each SoC might just work.
> 

We have ensured all BCDMAs for CSI follow the same offsets across the
SoCs. So, this compatible should work for all BCDMA_CSIs. But yes, this
can be expanded by adding new compatible is third BCDMA variant shows up
(doesn't exist today though).

> Nice solution with minimal change! ;)

Thanks! appreciate the review comments.


> 
>>  
>>  	ret = udma_get_mmrs(pdev, ud);
>>  	if (ret)
> 

-- 
Regards
Vignesh