Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Anurag Kumar Vulisha <anuragku@xilinx.com>
To:     Jack Pham <jackp@codeaurora.org>, Felipe Balbi <balbi@kernel.org>
CC:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "v.anuragkumar@gmail.com" <v.anuragkumar@gmail.com>
Subject: RE: [PATCH] usb: dwc3: gadget: Correct the logic for finding last SG
 entry
Thread-Topic: [PATCH] usb: dwc3: gadget: Correct the logic for finding last SG
 entry
Thread-Index: AQHVFi8wapUkz2GgO0KPkbPlYU1PoKaMvgMAgAH+Z2CAARGqgIDZi58AgALQ1iA=
Date:   Fri, 25 Oct 2019 12:01:50 +0000
Message-ID: <BYAPR02MB5591EFB659784B6A017B446EA7650@BYAPR02MB5591.namprd02.prod.outlook.com>
References: <1559141985-17104-1-git-send-email-anurag.kumar.vulisha@xilinx.com>
 <87y32gcvc1.fsf@linux.intel.com>
 <BYAPR02MB559181C009B74446A797838DA7170@BYAPR02MB5591.namprd02.prod.outlook.com>
 <87ftoldh48.fsf@linux.intel.com>
 <20191023165804.GD9754@jackp-linux.qualcomm.com>
In-Reply-To: <20191023165804.GD9754@jackp-linux.qualcomm.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: xilinx.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 669c1913-0380-4293-f991-08d759431eb9
X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Oct 2019 12:01:50.7770
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: K5RY6QbWIPmUznlB0Mp5Y5npXu4S2eiON8uT+Wo7itvJLr8yHFZnYPXcsMAKPN1uQdhpOUOcpVIvCcX6RAZE2g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR02MB5655
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


Hi Jack,

>-----Original Message-----
>From: Jack Pham [mailto:jackp@codeaurora.org]
>Sent: Wednesday, October 23, 2019 10:28 PM
>To: Felipe Balbi <balbi@kernel.org>
>Cc: Anurag Kumar Vulisha <anuragku@xilinx.com>; Greg Kroah-Hartman
><gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; linux-
>kernel@vger.kernel.org; v.anuragkumar@gmail.com
>Subject: Re: [PATCH] usb: dwc3: gadget: Correct the logic for finding last=
 SG
>entry
>
>Hi Anurag,
>
>On Fri, Jun 07, 2019 at 09:49:59AM +0300, Felipe Balbi wrote:
>> Anurag Kumar Vulisha <anuragku@xilinx.com> writes:
>> >>> The dma_map_sg() merges sg1 & sg2 memory regions into sg1-
>> >>>dma_address.
>> >>> Similarly sg3 & sg4 into sg2->dma_address, sg5 & sg6 into the
>> >>> sg3->dma_address and sg6 & sg8 into sg4->dma_address. Here the
>> >>memory
>> >>> regions are merged but the page_link properties like SG_END are not
>> >>> retained into the merged sgs.
>> >>
>> >>isn't this a bug in the scatterlist mapping code? Why doesn't it keep
>> >>SG_END?
>> >>
>> >
>> > Thanks for providing your comment.
>> >
>> > I don't think it is a bug, instead I feel some enhancement needs to be=
 done
>in
>> > dma-mapping code.
>> >
>> > SG_END represents the last sg entry in the sglist and it is correctly =
getting
>> > set to the last sg entry.
>> >
>> > The issue happens only when 2 or more sg entry pages are merged into
>> > contiguous dma-able address and sg_is_last() is used to find the last =
sg
>entry
>> > with valid dma address.
>>
>> Right, and that's something that's bound to happen. I'm arguing that,
>perhaps,
>> dma API should move SG_END in case entries are merged.
>>
>> > I think that along with sg_is_last() a new flag (SG_DMA_END) and funct=
ion
>> > (something like sg_dma_is_last() ) needs to be added into dma-mapping
>code for
>> > identifying the last valid sg entry with valid dma address. So that we=
 can
>> > make use of that function instead of sg_is_last().
>>
>> Sure, propose a patch to DMA API.
>
>I'm curious if this was ever resolved. I just ran into this exact issue
>with Android ADB which uses 16KB buffers, along with f_fs supporting
>S/G since 5.0, combined with our IOMMU which performs this merging
>behavior, so it resulted in a single TRB getting queued with CHN=3D1 and
>LST=3D0 and thus the transfer never completes. Your initial patch resolves
>the issue for me, but upon revisiting this discussion I couldn't tell if
>you had attempted to patch DMA API instead as per Felipe's suggestion.
>

We were stuck with some internal priority tasks, so couldn't initiate the
discussion with IOMMU maintainers. Will work on preparing the patch
and  sending the same very soon.

Thanks,
Anurag Kumar Vulisha

>Thanks,
>Jack
>--
>The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>a Linux Foundation Collaborative Project