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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AD21FC433EF for ; Tue, 28 Jun 2022 11:00:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=HoJIvGqFrVGSl4z16mjH22q9b30hSjsSyioJuS0e5EQ=; b=BzXp5t9jtZ8fno MN4V03u4/OElbLbc39oeTIalO1dh58r3yHTFdHm6ofucwzT38cEbLIml4T33Q+JKWQuA1kfwu2kOE tBDPwCfUZg+F9Mi7V/b6TY8ykSeH2LtW4BU6orwqWUZhw+N48H1jVdkglBkN4z4doIvq4akQDbNST Ko8sLoZ3Jz0d2GcuYTQeizTf7fXvEpobbQ63vQcs1S5unHrkkn1PnRUYJjpITTyYdOdb3EQrpk3QA Td5L5/Sphv8nE1ugi+I+aUrkBz0LedA+B6+WW3Xvfv6XLzwRylOF5inz1gwOqigcVMGG7ZNS9M+dB XoONM+5tKNvy1q7Q1EoQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o68wU-005oWy-8R; Tue, 28 Jun 2022 10:59:38 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o68wQ-005oVy-F1 for linux-arm-kernel@lists.infradead.org; Tue, 28 Jun 2022 10:59:36 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id E2011B81C53; Tue, 28 Jun 2022 10:59:32 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E5993C3411D; Tue, 28 Jun 2022 10:59:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656413971; bh=Wqg0ETp02r05Seu7ujmQUQ414cg/uxWo81xExN0cy1Q=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=nrh0nVqaQvbXB/U1O1AFBvjEHOWtM9Kmuc/cru4Q2as1yYm5yig+ESS0p35Ay3h1K 174YJ8Bvo966syKkPSc4BVmTpPH6hYMWyHOUE84kt29ECeKnC7GStSNUZJRg52gr7J NhEWDbdOxwwnkdFdtBgNf+eygbATz2hJqKalAhUGO29zs7nyMidkFd9srg9DLaHRPM 78Vvmuy284U4syr9bQlcPhQe3jXNROMmFt54M8s8KRDAseaAQttKxFJ8QN6EJ+GJb4 DgeVmtRl7ckAuZIysrcEp4u6IBVCUpBZLRJStIUIGcrRZgds7GT2ubwVd9CJyLVQSw 7YonD4CETuCpw== Date: Tue, 28 Jun 2022 05:59:29 -0500 From: Bjorn Helgaas To: Krzysztof Kozlowski Cc: Marek Szyprowski , Jaehoon Chung , Jingoo Han , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Alim Akhtar , linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org Subject: Re: pci-exynos.c phy_init() usage Message-ID: <20220628105929.GA1819457@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <7ab52c5f-6c48-6381-e0f3-a1d9572dc2a9@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220628_035934_837671_C156EC06 X-CRM114-Status: GOOD ( 28.45 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Jun 28, 2022 at 10:27:31AM +0200, Krzysztof Kozlowski wrote: > On 28/06/2022 10:13, Marek Szyprowski wrote: > > On 27.06.2022 12:47, Krzysztof Kozlowski wrote: > >> On 27/06/2022 12:30, Marek Szyprowski wrote: > >>> On 24.06.2022 20:07, Krzysztof Kozlowski wrote: > >>>> On 24/06/2022 19:35, Bjorn Helgaas wrote: > >>>>> In exynos_pcie_host_init() [1], we call: > >>>>> > >>>>> phy_reset(ep->phy); > >>>>> phy_power_on(ep->phy); > >>>>> phy_init(ep->phy); > >>>>> > >>>>> The phy_init() function comment [2] says it must be called before > >>>>> phy_power_on(). Is exynos doing this backwards? > >>>> Looks like. I don't have Exynos hardware with a PCI, so cannot > >>>> test/fix/verify. > >>>> > >>>> Luckily for Exynos ;-) it's not alone in this pattern: > >>>> drivers/net/ethernet/marvell/sky2.c > >>>> drivers/usb/dwc2/platform.c > >>> I've checked that on the real hardware. Swapping the order of > >>> phy_power_on and phy_init breaks driver operation. > >>> > >>> However pci-exynos is the only driver that uses the phy-exynos-pcie, so > >>> we can simply swap the content of the init and power_on in the phy > >>> driver to adjust the code to the right order. power_on/init and > >>> exit/power_off are also called one after the other in pci-exynos, > >>> without any activity between them, so we can also simply move all > >>> operation to one pair of the callback, like power_on/off. > >>> > >>> Krzysztof, which solution would you prefer? > >> I think the real problem is that the Exynos PCIe phy init > >> (exynos5433_pcie_phy_init) performs parts of power on procedure, so the > >> code is mixed. Probably also the phy init could not happen earlier due > >> to gated clocks (ungated in exynos5433_pcie_phy_power_on). > >> > >> I would prefer to clean it up while ordering init+power_on, so figure > >> out more or less correct procedure. > >> > >> You can also look at Artpec-8 PHY - it seems using correct order > >> (init+reset): > >> https://lore.kernel.org/all/20220614011616epcms2p7dcaa67c53b7df5802dd7a697e2d472d7@epcms2p7/ > > > > I've played a bit with those register writes in exynos_pcie_phy and > > frankly speaking the currenly used (power_on + init) is the only > > sequence that works properly. I'm leaning to move everything to > > phy_init/exit. I really don't see how to split it into init + power_on > > callbacks. > > I was afraid it will be like this. I imagine that certain (not > explicitly documented) init operations cannot even happen before power > on, so this would be a lot of tries. > > I am fine with it. Thanks for doing it. If nothing can be improved, a comment to this effect might make it look less like a mistake. Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel