Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v5 2/2] Add Intel LGM soc DMA support.
From:   Peter Ujfalusi <peter.ujfalusi@ti.com>
To:     Amireddy Mallikarjuna reddy <mallikarjunax.reddy@linux.intel.com>,
        <dmaengine@vger.kernel.org>, <vkoul@kernel.org>,
        <devicetree@vger.kernel.org>, <robh+dt@kernel.org>
CC:     <linux-kernel@vger.kernel.org>, <andriy.shevchenko@intel.com>,
        <cheol.yong.kim@intel.com>, <qi-ming.wu@intel.com>,
        <chuanhua.lei@linux.intel.com>, <malliamireddy009@gmail.com>
References: <cover.1597381889.git.mallikarjunax.reddy@linux.intel.com>
 <cdd26d104000c060d85a0c5f8abe8492e4103de5.1597381889.git.mallikarjunax.reddy@linux.intel.com>
 <fbc98cdb-3b50-cbcc-0e90-c9d6116566d1@ti.com>
Message-ID: <70848117-ec29-d293-6603-cbf47dec35d4@ti.com>
Date:   Tue, 18 Aug 2020 13:29:12 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <fbc98cdb-3b50-cbcc-0e90-c9d6116566d1@ti.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 18/08/2020 13.16, Peter Ujfalusi wrote:

=2E..

>> +static void dma_issue_pending(struct dma_chan *chan)
>> +{
>> +	struct ldma_chan *c =3D to_ldma_chan(chan);
>> +	struct ldma_dev *d =3D to_ldma_dev(c->vchan.chan.device);
>> +	unsigned long flags;
>> +
>> +	if (d->ver =3D=3D DMA_VER22) {
>> +		spin_lock_irqsave(&c->vchan.lock, flags);
>> +		if (vchan_issue_pending(&c->vchan)) {
>> +			struct virt_dma_desc *vdesc;
>> +
>> +			/* Get the next descriptor */
>> +			vdesc =3D vchan_next_desc(&c->vchan);
>> +			if (!vdesc) {
>> +				c->ds =3D NULL;
>> +				return;
>> +			}
>> +			list_del(&vdesc->node);
>> +			c->ds =3D to_lgm_dma_desc(vdesc);
>=20
> you have set c->ds in dma_prep_slave_sg and the only way I can see that=

> you will not leak memory is that the client must terminate_sync() after=

> each transfer so that the synchronize callback is invoked between each
> prep_sg/issue_pending/competion.
>=20
>> +			spin_unlock_irqrestore(&c->vchan.lock, flags);
>> +			ldma_chan_desc_hw_cfg(c, c->ds->desc_phys, c->ds->desc_cnt);
>> +			ldma_chan_irq_en(c);
>> +		}
>=20
> If there is nothing pending, you will leave the spinlock wide open...

you leave it locked...

>=20
>> +	}
>> +	ldma_chan_on(c);
>> +}

- P=C3=A9ter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki