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 4AC83C433EF for ; Fri, 6 May 2022 07:39:41 +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:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=wfqeJAevUWE9xPlptMJO7Bse0LeertAf/60P6GNfw0Y=; b=DfF+/EPomtjyTR wd6NQt+Dv67BEZ2ORfy0aqkC1WbPZKKn01FSToJ6hQdkFGYdthWRlp2rDHvQfVKIHvqzXDE69IG3z /HevefBqTAvs8WH8Fq6kjAV55yxKoAwmncq7wFZKwMO0KCVEXmhdLVhjGT5tBE3kfROfZwI8DN5Nt 12xt1biDfkx+UYzeVPJ2dLjFCrlj/0BwR+jGrsc0dHddYvhafceCzgJZ6ZRO6geRW3oRpjK1jzrMH WP6JmuvmFz+Spl9NrYVRHjRaQdRO62eRa0WluJiIRDkSRYNwOhAbhFFvbTADgrK/6cVGW3043PWgp O0JTKCFIYvUeG7Q7GgoA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nmsXf-001sGo-0n; Fri, 06 May 2022 07:38:23 +0000 Received: from mx1.tq-group.com ([93.104.207.81]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nmsXb-001sEV-Hp for linux-arm-kernel@lists.infradead.org; Fri, 06 May 2022 07:38:22 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1651822699; x=1683358699; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=x3UR8vxd+AhKyBXzZmyGlDsth1DjOBHx8a9Q9fUvtDU=; b=TTKemkEnWDuqRM015nw5Z5WS4Q03xoVmhtPktRnI59a8ksOmU8Dnu+IO 6xBQt7M80Vr1IJyn7U139kZDCz5CaRi9/X/yXsuoYITYg2ZVsO4k6/ukF PhxeSqZJuPeYGlCcBVceujVb4kUxFXSneFQwNkvubZkOJNz0t2PY/0Fql tAxzeM+ItjtXHA3O2S6uStYzAdH/wv3x7hZ51FqZpAi9iXiU1gPKWc8KO 5ihbtZH1fy4bKhD1BQlpPz/JJxzMh0hEUsBY+zYvla1oHwhDwIdu9SyOx wzrea0/MSDzYaWxKhSQHOrfMyPN2wTaWwgFSQKCceT/qJQoiJd/EPvH9f A==; X-IronPort-AV: E=Sophos;i="5.91,203,1647298800"; d="scan'208";a="23712750" Received: from unknown (HELO tq-pgp-pr1.tq-net.de) ([192.168.6.15]) by mx1-pgp.tq-group.com with ESMTP; 06 May 2022 09:38:11 +0200 Received: from mx1.tq-group.com ([192.168.6.7]) by tq-pgp-pr1.tq-net.de (PGP Universal service); Fri, 06 May 2022 09:38:11 +0200 X-PGP-Universal: processed; by tq-pgp-pr1.tq-net.de on Fri, 06 May 2022 09:38:11 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1651822691; x=1683358691; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=x3UR8vxd+AhKyBXzZmyGlDsth1DjOBHx8a9Q9fUvtDU=; b=PYWfYDpFr7TLdakdGnyLzUITu1DUwb5fDNs8/5aFcFwknT6EJS66W3/L qn7lAXOBwOjdQVzGr4Xc8ticavG3NPbfMKfSIbNTACY53m27U6L1SzA+p omEVoeWJzHcqjZrnDbNtJKWDZ5jTa0PBAGI5vTyEtEZimT8W4Z0tmQu9P wGvLbYeYfEb4Hlk8zdVusZZhbQn1uzGtsYTL+PJ9mbJBtzhevB857pm8R RG+HnkasUnIGd+nbLLEUclMVbTPpowPLKkv9A3jp7wv8aWgWvoYDfTHx0 EE7hQQfItXIowoAyPdktWZnsqsNVinkCrVB9D0NqEav61rw9BiclhaAW6 A==; X-IronPort-AV: E=Sophos;i="5.91,203,1647298800"; d="scan'208";a="23712749" Received: from vtuxmail01.tq-net.de ([10.115.0.20]) by mx1.tq-group.com with ESMTP; 06 May 2022 09:38:11 +0200 Received: from steina-w.localnet (unknown [10.123.49.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by vtuxmail01.tq-net.de (Postfix) with ESMTPSA id 3592D280070; Fri, 6 May 2022 09:38:11 +0200 (CEST) From: Alexander Stein To: Peter Chen , Jun Li Cc: Peter Chen , Greg Kroah-Hartman , Shawn Guo , Sascha Hauer , Fabio Estevam , Pengutronix Kernel Team , dl-linux-imx , USB list , "linux-arm-kernel@lists.infradead.org" Subject: Re: (EXT) RE: (EXT) RE: (EXT) Re: [RFC 1/1] usb: chipidea: ci_hdrc_imx: disable runtime pm for HSIC interface Date: Fri, 06 May 2022 09:38:08 +0200 Message-ID: <1792784.atdPhlSkOF@steina-w> Organization: TQ-Systems GmbH In-Reply-To: References: <20220302094239.3075014-1-alexander.stein@ew.tq-group.com> <5566202.DvuYhMxLoT@steina-w> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220506_003820_095093_E5480C13 X-CRM114-Status: GOOD ( 56.44 ) 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="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Am Freitag, 6. Mai 2022, 09:09:22 CEST schrieb Jun Li: > > -----Original Message----- > > From: Alexander Stein > > Sent: Wednesday, May 4, 2022 3:06 PM > > To: Peter Chen ; Jun Li > > Cc: Peter Chen ; Greg Kroah-Hartman > > ; Shawn Guo ; Sascha > > Hauer ; Fabio Estevam ; > > Pengutronix Kernel Team ; dl-linux-imx > > ; USB list ; > > linux-arm-kernel@lists.infradead.org > > Subject: Re: (EXT) RE: (EXT) Re: [RFC 1/1] usb: chipidea: ci_hdrc_imx: > > disable runtime pm for HSIC interface > > = > > Helllo, > > = > > Am Dienstag, 12. April 2022, 13:36:55 CEST schrieb Jun Li: > > > > -----Original Message----- > > > > From: Alexander Stein > > > > Sent: Monday, April 11, 2022 9:52 PM > > > > To: Peter Chen ; Jun Li > > > > Cc: Peter Chen ; Greg Kroah-Hartman > > > > ; Shawn Guo ; > > > > Sascha Hauer ; Fabio Estevam > > > > ; Pengutronix Kernel Team > > > > ; dl-linux-imx ; USB list > > > > ; linux-arm-kernel@lists.infradead.org > > > > Subject: Re: (EXT) RE: (EXT) Re: [RFC 1/1] usb: chipidea: ci_hdrc_i= mx: > > > > disable runtime pm for HSIC interface > > > > = > > > > Hi, > > > > = > > > > Am Samstag, 9. April 2022, 06:49:54 CEST schrieb Jun Li: > > > > > > -----Original Message----- > > > > > > From: Peter Chen > > > > > > Sent: Saturday, April 9, 2022 10:20 AM > > > > > > To: Alexander Stein > > > > > > Cc: Peter Chen ; Greg Kroah-Hartman > > > > > > ; Shawn Guo ; > > > > > > Sascha Hauer ; Fabio Estevam > > > > > > ; Pengutronix Kernel Team > > > > > > ; dl-linux-imx ; USB > > > > > > list ; > > > > > > linux-arm-kernel@lists.infradead.org > > > > > > Subject: Re: (EXT) Re: [RFC 1/1] usb: chipidea: ci_hdrc_imx: > > > > > > disable runtime pm for HSIC interface > > > > > > = > > > > > > On 22-03-29 10:14:36, Alexander Stein wrote: > > > > > > > Hello Peter, > > > > > > > = > > > > > > > Am Dienstag, 15. M=E4rz 2022, 02:23:23 CEST schrieb Peter Che= n: > > > > > > > > On Wed, Mar 2, 2022 at 5:42 PM Alexander Stein > > > > > > > > = > > > > > > > > wrote: > > > > > > > > > With the add of power-domain support in commit > > > > > > > > > 02f8eb40ef7b > > = > > ("ARM: > > > > > > dts: > > > > > > > > > imx7s: Add power domain for imx7d HSIC") runtime suspend > > > > > > > > > will disable the power-domain. This prevents IRQs to occur > > > > > > > > > when a new device is attached on a downstream hub. > > > > > > > > > = > > > > > > > > > Signed-off-by: Alexander Stein > > > > > > > > > > > > > > > > > > --- > > > > > > > > > Our board TQMa7x + MBa7x (i.MX7 based) uses a HSIC link to > > > > > > > > > mounted > > > > > > = > > > > > > USB HUB > > > > > > = > > > > > > > > > on usbh device. Cold plugging an USB mass storage device > > > > > > > > > is working > > > > > > = > > > > > > fine. > > > > > > = > > > > > > > > > But once the last non-HUB device is disconnected the > > > > > > > > > ci_hdrc device > > > > > > = > > > > > > goes > > > > > > = > > > > > > > > > into runtime suspend. > > > > > > > > = > > > > > > > > Would you please show the difference between cold boot and > > > > > > > > runtime suspend after disconnecting the last USB device? > > > > > > > > = > > > > > > > > - Power domain on/off status for HUB device > > > > > > > > - Runtime suspend status at /sys entry for HUB device > > > > > > > > - "/sys/..power/wakeup" /sys entry for HUB device > > > > > > > = > > > > > > > I hope I got all entries you requested. > > > > > > > = > > > > > > > For reference this is the bus topology: > > > > > > > lsusb -t > > > > > > > /: Bus 02.Port 1: Dev 1, Class=3Droot_hub, Driver=3Dci_hdrc/= 1p, > > > > > > > 480M > > > > > > > /: Bus 01.Port 1: Dev 1, Class=3Droot_hub, Driver=3Dci_hdrc/= 1p, > > > > > > > 480M > > > > > > > = > > > > > > > |__ Port 1: Dev 2, If 0, Class=3DHub, Driver=3Dhub/4p, 48= 0M > > > > > > > | > > > > > > > |__ Port 2: Dev 3, If 0, Class=3DMass Storage, > > > > > > > = > > > > > > > Driver=3Dusb-storage, > > > > > > = > > > > > > 480M > > > > > > = > > > > > > > Bus 2 is a different connector and doesn't matter here. I'm > > > > > > > disconnecting > > > > > > = > > > > > > 'Dev > > > > > > = > > > > > > > 3' in this scenario. > > > > > > > = > > > > > > > After boot up with the bus as shown above: > > > > > > > $ cat /sys/bus/usb/devices/1-1/power/wakeup > > > > > > > disabled > > > > > > > $ cat /sys/bus/usb/devices/1-1/power/runtime_status > > > > > > > active > > > > > > > $ cat /sys/kernel/debug/pm_genpd/usb-hsic-phy/current_state > > > > > > > on > > > > > > > = > > > > > > > After disconnecting Dev 3 from the bus ('usb 1-1.2: USB > > > > > > > disconnect, device number 3' in dmesg) the status changes as > > > > > > > follows (without the patch): > > > > > > > $ cat /sys/bus/usb/devices/1-1/power/wakeup > > > > > > > disabled > > > > > > > $ cat /sys/bus/usb/devices/1-1/power/runtime_status > > > > > > > suspended > > > > > > > $ cat /sys/kernel/debug/pm_genpd/usb-hsic-phy/current_state > > > > > > > off-0 > > > > > > > = > > > > > > > For the record, when applying the posted patch this changes > > > > > > > into: > > > > > > > $ cat /sys/bus/usb/devices/1-1/power/wakeup > > > > > > > disabled > > > > > > > $ cat /sys/bus/usb/devices/1-1/power/runtime_status > > > > > > > suspended > > > > > > > $ cat /sys/kernel/debug/pm_genpd/usb-hsic-phy/current_state > > > > > > > on > > > > > > = > > > > > > Okay, I think the problem here is the power domain for USB > > > > > > controller is off at runtime, but USB controller/PHY needs to > > > > > > detect the USB wakeup signal at runtime, so the USB > > > > > > controller/PHY's power domain should be not off. The proper > > > > > > change may keep power domain on at runtime, and the power domain > > = > > could be off at system suspend. > > = > > > > > Can this "hsic phy power domain off breaks wakeup" be confirmed? > > > > > Like with some hack to move hsic phy power domain on some other > > > > > device > > > > > node: > > > > > = > > > > > non-usb-node { > > > > > = > > > > > ... > > > > > power-domains =3D <&pgc_hsic_phy>; > > > > > status =3D "okay"; > > > > > = > > > > > }; > > > > > = > > > > > Just make sure this non-usb-node to be runtime active when do hsic > > > > > hub test. > > > > = > > > > Thanks for that suggestion. I apparently does work. Using the this > > > > small patch > > > > = > > > > --->8--- > > > > diff --git a/arch/arm/boot/dts/imx7-mba7.dtsi > > > > b/arch/arm/boot/dts/imx7- mba7.dtsi index b05f662aa87b..cba2f9efa17e > > > > 100644 > > > > --- a/arch/arm/boot/dts/imx7-mba7.dtsi > > > > +++ b/arch/arm/boot/dts/imx7-mba7.dtsi > > > > @@ -580,6 +580,7 @@ &uart3 { > > > > = > > > > assigned-clocks =3D <&clks IMX7D_UART3_ROOT_SRC>; > > > > assigned-clock-parents =3D <&clks IMX7D_OSC_24M_CLK>; > > > > status =3D "okay"; > > > > = > > > > + power-domains =3D <&pgc_hsic_phy>; > > > > = > > > > }; > > > > = > > > > &uart4 { > > > > = > > > > --->8--- > > > > = > > > > The HSIC power domain is also attached to to uart3. > > > > = > > > > $ cat /sys/kernel/debug/pm_genpd/usb-hsic-phy/devices > > > > /devices/platform/soc/30800000.bus/30800000.spba-bus/30880000.serial > > > > /devices/platform/soc/30800000.bus/30b30000.usb > > > > /devices/platform/soc/30800000.bus/30b30000.usb/ci_hdrc.1 > > > > $ cat /sys/kernel/debug/pm_genpd/usb-hsic-phy/current_state > > > > on > > > > $ echo on > > > > > /sys/devices/platform/soc/30800000.bus/30800000.spba-bus/ > > > > 30880000.serial/power/control > > > > $ lsusb -t > > > > /: Bus 02.Port 1: Dev 1, Class=3Droot_hub, Driver=3Dci_hdrc/1p, 48= 0M > > > > /: Bus 01.Port 1: Dev 1, Class=3Droot_hub, Driver=3Dci_hdrc/1p, 48= 0M > > > > = > > > > |__ Port 1: Dev 2, If 0, Class=3DHub, Driver=3Dhub/4p, 480M > > > > | > > > > |__ Port 2: Dev 3, If 0, Class=3DMass Storage, Driver=3D, 4= 80M > > > > = > > > > [USB > > > > = > > > > disconnect] $ cat > > > > /sys/kernel/debug/pm_genpd/usb-hsic-phy/current_state > > > > on > > > = > > > Just want to be sure this was done with hdrc imx runtime PM enabled. > > > = > > > > [USB reconnect] > > > > $ lsusb -t > > > > /: Bus 02.Port 1: Dev 1, Class=3Droot_hub, Driver=3Dci_hdrc/1p, 48= 0M > > > > /: Bus 01.Port 1: Dev 1, Class=3Droot_hub, Driver=3Dci_hdrc/1p, 48= 0M > > > > = > > > > |__ Port 1: Dev 2, If 0, Class=3DHub, Driver=3Dhub/4p, 480M > > > > | > > > > |__ Port 2: Dev 4, If 0, Class=3DMass Storage, Driver=3D, 4= 80M > > > > = > > > > Hot plug detecting still works as you can see the USB device number > > > > increased. > > > > = > > > > For the records, there is no difference to this patch in removing > > > > the power domain from USB HSIC device. I just wanted to keep the > > > > diff small. > > > = > > > This is good enough to confirm this, thanks. > > > = > > > I don't have a HW with HSIC enabled for test, and I am not sure the > > > initial state of power domain is on, can something like below deserve= a > > = > > try? > > = > > > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c index > > > 3cb123016b3e..f5467ef18e33 100644 > > > --- a/drivers/soc/imx/gpcv2.c > > > +++ b/drivers/soc/imx/gpcv2.c > > > @@ -416,6 +416,7 @@ static const struct imx_pgc_domain > > > imx7_pgc_domains[] =3D { [IMX7_POWER_DOMAIN_USB_HSIC_PHY] =3D { > > > = > > > .genpd =3D { > > > = > > > .name =3D "usb-hsic-phy", > > > = > > > + .flags =3D GENPD_FLAG_RPM_ALWAYS_ON, > > > = > > > }, > > > .bits =3D { > > > = > > > .pxx =3D IMX7_USB_HSIC_PHY_SW_Pxx_REQ, @@ -93= 0,7 > > > = > > > +931,7 @@ static int imx_pgc_domain_probe(struct platform_device > > > *pdev) regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING, > > > domain->bits.map, domain->bits.map); > > > = > > > - ret =3D pm_genpd_init(&domain->genpd, NULL, true); > > > + ret =3D pm_genpd_init(&domain->genpd, NULL, > > > + !(domain->genpd.flags & > > > GENPD_FLAG_RPM_ALWAYS_ON)); if (ret) { > > > = > > > dev_err(domain->dev, "Failed to init power domain\n"); > > > goto out_domain_unmap; > > = > > This does indeed the trick. But AFAICS the downside is that the > > powerdomain > > is enabled, even if USB is not used. Not sure if this is acceptable > > though. > = > I think GENPD_FLAG_RPM_ALWAYS_ON is the right thing to do if the HSIC port > need the power domain on to detect remote wakeup, what's the exact meaning > of "USB is not used"? Exactly, GENPD_FLAG_RPM_ALWAYS_ON is the right thing to to iff the HSIC por= t = needs the powerdomain. But what about the case when HSIC is not enabled at = all? That's what I meant by "USB is not used". AFAICS setting GENPD_FLAG_RPM_ALWAYS_ON enables the powerdomain = unconditionally from any user. Alexander _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel