From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 670E5C433E6 for ; Tue, 1 Sep 2020 12:01:58 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1E02D206EF for ; Tue, 1 Sep 2020 12:01:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="rDmD02NS" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1E02D206EF Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=FR4TdqlZiT0bLuw6DSqgcQH+XdOJiaPFXk0lMZFW4pM=; b=rDmD02NS0EycyuH6vdvak3k8n 0mG2BtSNL4inEMN0t39EXb3XVEl9VOKQQSSHmibfSGxrp+ZvGbfLTxkTQX2k2xQqZHSeTo8z2xMr6 mpqTnqhNtp6/iy5ZPsAJgN3zlYyUQBZzwa7w79eBgbvhx9fCVmzqWM6UhHtJfv7lQv2EED2BcsXH9 IQXFMc+4EggoGzqeoJKLMmGWUmfaGVuTc3BM7gtWbxM750yE8cylIU8To+urhmojNg+CJGyzIrFyJ yXY/Brgpl4C3JaTTk5LjdWWfhQIjAyIRJO89qbedhBPMTaFHtGiuzp4uze97IcX5cvmnM/lU7I9A1 Q8Vwgvd4w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kD4xp-0000aU-02; Tue, 01 Sep 2020 12:00:37 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kD4xl-0000ZO-UJ for linux-arm-kernel@lists.infradead.org; Tue, 01 Sep 2020 12:00:34 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 463E01FB; Tue, 1 Sep 2020 05:00:30 -0700 (PDT) Received: from [10.57.40.122] (unknown [10.57.40.122]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 937E73F68F; Tue, 1 Sep 2020 05:00:28 -0700 (PDT) Subject: Re: [PATCH 2/2] usb: dwc3: Add driver for Xilinx platforms To: Manish Narani , "gregkh@linuxfoundation.org" , "robh+dt@kernel.org" , Michal Simek , "balbi@kernel.org" , "p.zabel@pengutronix.de" References: <1598467441-124203-1-git-send-email-manish.narani@xilinx.com> <1598467441-124203-3-git-send-email-manish.narani@xilinx.com> <0927fb9f-1044-38b3-d6f3-76edffefd99c@arm.com> From: Robin Murphy Message-ID: <98c17481-e9c5-ce03-ad30-3653ec2305d4@arm.com> Date: Tue, 1 Sep 2020 13:00:27 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200901_080034_062951_ADF4960E X-CRM114-Status: GOOD ( 23.91 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "devicetree@vger.kernel.org" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , git Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2020-08-28 18:53, Manish Narani wrote: > Hi Robin, > > Thanks for the review. Please find my comment below inline. > >> -----Original Message----- >> From: Robin Murphy >> Sent: Friday, August 28, 2020 12:17 AM >> To: Manish Narani ; gregkh@linuxfoundation.org; >> robh+dt@kernel.org; Michal Simek ; balbi@kernel.org; >> p.zabel@pengutronix.de >> Cc: devicetree@vger.kernel.org; linux-usb@vger.kernel.org; linux- >> kernel@vger.kernel.org; git ; linux-arm- >> kernel@lists.infradead.org >> Subject: Re: [PATCH 2/2] usb: dwc3: Add driver for Xilinx platforms >> >> On 2020-08-26 19:44, Manish Narani wrote: >> [...] >>> + /* >>> + * This routes the usb dma traffic to go through CCI path instead >>> + * of reaching DDR directly. This traffic routing is needed to >>> + * make SMMU and CCI work with USB dma. >>> + */ >>> + if (of_dma_is_coherent(dev->of_node) || dev->iommu_group) { >>> + reg = readl(priv_data->regs + XLNX_USB_COHERENCY); >>> + reg |= XLNX_USB_COHERENCY_ENABLE; >>> + writel(reg, priv_data->regs + XLNX_USB_COHERENCY); >>> + } >> >> This looks rather suspect - coherency should be based on coherency, not >> on whether an IOMMU group is present. If the device isn't described as >> coherent in the DT, then any SMMU mappings will end up using attributes >> that will downgrade traffic to be non-snooping anyway. And if the SMMU >> is enabled but not translating (e.g. "iommu.passthrough=1") then >> enabling hardware coherency when the DMA layer hasn't been told about it >> can potentially lead to nasty subtle problems and data loss. > > May be the description needs to be updated in this. This is not the actual coherency enabling bit, but this is needed when coherency is enabled. > This is a register inside Xilinx USB controller which handles USB (which is in LPD) traffic route switching from LPD (Low Power Domain) to FPD (Full Power Domain) path in the Xilinx SoC in either of the below scenarios: > 1. Device is described coherent in DT. > 2. SMMU is enabled. > > I will update the same in v2. Ah, OK, so it's just that the control bit itself has a terrible name :) From the available information I had assumed that this controlled the output attributes, and that the interconnect might then steer traffic based on those. Explaining a bit more clearly in the comment probably would be a good idea. In that case, I'd concur that the current logic is in fact appropriate, but please use the device_iommu_mapped() helper for cleanliness. Cheers, Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel