lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d077971c-d7ef-d630-2d89-5a06fb481b28@broadcom.com>
Date:   Tue, 6 Nov 2018 16:40:55 -0500
From:   Al Cooper <al.cooper@...adcom.com>
To:     Alan Stern <stern@...land.harvard.edu>,
        Al Cooper <alcooperx@...il.com>
Cc:     linux-kernel@...r.kernel.org, Alban Bedel <albeu@...e.fr>,
        Alex Elder <elder@...aro.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        Avi Fishman <avifishman70@...il.com>,
        bcm-kernel-feedback-list@...adcom.com,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Chunfeng Yun <chunfeng.yun@...iatek.com>,
        "David S. Miller" <davem@...emloft.net>,
        devicetree@...r.kernel.org, Dmitry Osipenko <digetx@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        Hans de Goede <hdegoede@...hat.com>,
        James Hogan <jhogan@...nel.org>,
        Jianguo Sun <sunjianguo1@...wei.com>,
        Johan Hovold <johan@...nel.org>,
        Kees Cook <keescook@...omium.org>, linux-usb@...r.kernel.org,
        Lu Baolu <baolu.lu@...ux.intel.com>,
        Mark Rutland <mark.rutland@....com>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Mathias Nyman <mathias.nyman@...el.com>,
        Mathias Nyman <mathias.nyman@...ux.intel.com>,
        Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
        Rishabh Bhatnagar <rishabhb@...eaurora.org>,
        Rob Herring <robh+dt@...nel.org>, Roger Quadros <rogerq@...com>
Subject: Re: [PATCH V3 4/6] usb: ohci-platform: Add support for Broadcom STB
 SoC's

On 11/6/18 11:08 AM, Alan Stern wrote:
> On Mon, 5 Nov 2018, Al Cooper wrote:
> 
>> Add support for Broadcom STB SoC's to the ohci platform driver.
>>
>> Signed-off-by: Al Cooper <alcooperx@...il.com>
>> ---
> 
>> @@ -177,6 +189,8 @@ static int ohci_platform_probe(struct platform_device *dev)
>>   		ohci->flags |= OHCI_QUIRK_FRAME_NO;
>>   	if (pdata->num_ports)
>>   		ohci->num_ports = pdata->num_ports;
>> +	if (pdata->suspend_without_phy_exit)
>> +		hcd->suspend_without_phy_exit = 1;
> 
> Sorry if I missed this in the earlier discussions...  Is there any
> possibility of adding a DT binding that could express this requirement,
> instead of putting it in the platform data?
> 
> Alan Stern
> 

Alan,

That was my original approach but internal review suggested that I use 
pdata instead. Below is my original patch for:
[PATCH V3 2/6] usb: core: Add ability to skip phy exit on suspend and 
init on resume
With this patch I can then use ohci_platform.c without any 
modifications. Could you let me know what you think?

Thanks
Al

Add the ability to skip calling the PHY's exit routine on suspend
and the PHY's init routine on resume. This is to handle a USB PHY
that should have it's power_off function called on suspend but cannot
have it's exit function called because on exit it will disable the
PHY to the point where register accesses to the Host Controllers
using the PHY will be disabled and the host drivers will crash.

This is enabled with the HCD flag "suspend_without_phy_exit" which
can be set from any HCD driver or from the device-tree property
"suspend-without-phy-exit".

Signed-off-by: Al Cooper <alcooperx@...il.com>
---
  drivers/usb/core/hcd.c  |  8 ++++----
  drivers/usb/core/phy.c  | 21 +++++++++++++++------
  drivers/usb/core/phy.h  |  9 ++++++---
  include/linux/usb/hcd.h |  3 +++
  4 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 1c21955fe7c0..e67e4d6b3d21 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2263,7 +2263,7 @@ int hcd_bus_suspend(struct usb_device *rhdev, 
pm_message_t msg)
  		hcd->state = HC_STATE_SUSPENDED;

  		if (!PMSG_IS_AUTO(msg))
-			usb_phy_roothub_suspend(hcd->self.sysdev,
+			usb_phy_roothub_suspend(hcd,
  						hcd->phy_roothub);

  		/* Did we race with a root-hub wakeup event? */
@@ -2304,7 +2304,7 @@ int hcd_bus_resume(struct usb_device *rhdev, 
pm_message_t msg)
  	}

  	if (!PMSG_IS_AUTO(msg)) {
-		status = usb_phy_roothub_resume(hcd->self.sysdev,
+		status = usb_phy_roothub_resume(hcd,
  						hcd->phy_roothub);
  		if (status)
  			return status;
@@ -2347,7 +2347,7 @@ int hcd_bus_resume(struct usb_device *rhdev, 
pm_message_t msg)
  		}
  	} else {
  		hcd->state = old_state;
-		usb_phy_roothub_suspend(hcd->self.sysdev, hcd->phy_roothub);
+		usb_phy_roothub_suspend(hcd, hcd->phy_roothub);
  		dev_dbg(&rhdev->dev, "bus %s fail, err %d\n",
  				"resume", status);
  		if (status != -ESHUTDOWN)
@@ -2744,7 +2744,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
  	struct usb_device *rhdev;

  	if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) {
-		hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev);
+		hcd->phy_roothub = usb_phy_roothub_alloc(hcd);
  		if (IS_ERR(hcd->phy_roothub))
  			return PTR_ERR(hcd->phy_roothub);

diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
index 9879767452a2..0eb12566f1c3 100644
--- a/drivers/usb/core/phy.c
+++ b/drivers/usb/core/phy.c
@@ -45,10 +45,11 @@ static int usb_phy_roothub_add_phy(struct device 
*dev, int index,
  	return 0;
  }

-struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
+struct usb_phy_roothub *usb_phy_roothub_alloc(struct usb_hcd *hcd)
  {
  	struct usb_phy_roothub *phy_roothub;
  	int i, num_phys, err;
+	struct device *dev = hcd->self.sysdev;

  	if (!IS_ENABLED(CONFIG_GENERIC_PHY))
  		return NULL;
@@ -58,6 +59,9 @@ struct usb_phy_roothub *usb_phy_roothub_alloc(struct 
device *dev)
  	if (num_phys <= 0)
  		return NULL;

+	if (device_property_read_bool(dev, "suspend-without-phy-exit"))
+		hcd->suspend_without_phy_exit = 1;
+
  	phy_roothub = devm_kzalloc(dev, sizeof(*phy_roothub), GFP_KERNEL);
  	if (!phy_roothub)
  		return ERR_PTR(-ENOMEM);
@@ -161,26 +165,30 @@ void usb_phy_roothub_power_off(struct 
usb_phy_roothub *phy_roothub)
  }
  EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off);

-int usb_phy_roothub_suspend(struct device *controller_dev,
+int usb_phy_roothub_suspend(struct usb_hcd *hcd,
  			    struct usb_phy_roothub *phy_roothub)
  {
+	struct device *controller_dev = hcd->self.sysdev;
+
  	usb_phy_roothub_power_off(phy_roothub);

  	/* keep the PHYs initialized so the device can wake up the system */
-	if (device_may_wakeup(controller_dev))
+	if (device_may_wakeup(controller_dev) || hcd->suspend_without_phy_exit)
  		return 0;

  	return usb_phy_roothub_exit(phy_roothub);
  }
  EXPORT_SYMBOL_GPL(usb_phy_roothub_suspend);

-int usb_phy_roothub_resume(struct device *controller_dev,
+int usb_phy_roothub_resume(struct usb_hcd *hcd,
  			   struct usb_phy_roothub *phy_roothub)
  {
+	struct device *controller_dev = hcd->self.sysdev;
  	int err;

  	/* if the device can't wake up the system _exit was called */
-	if (!device_may_wakeup(controller_dev)) {
+	if (!device_may_wakeup(controller_dev) &&
+	    !hcd->suspend_without_phy_exit) {
  		err = usb_phy_roothub_init(phy_roothub);
  		if (err)
  			return err;
@@ -189,7 +197,8 @@ int usb_phy_roothub_resume(struct device 
*controller_dev,
  	err = usb_phy_roothub_power_on(phy_roothub);

  	/* undo _init if _power_on failed */
-	if (err && !device_may_wakeup(controller_dev))
+	if (err && !device_may_wakeup(controller_dev)
+	    && !hcd->suspend_without_phy_exit)
  		usb_phy_roothub_exit(phy_roothub);

  	return err;
diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
index 88a3c037e9df..34293e11a917 100644
--- a/drivers/usb/core/phy.h
+++ b/drivers/usb/core/phy.h
@@ -5,13 +5,16 @@
   * Copyright (C) 2018 Martin Blumenstingl 
<martin.blumenstingl@...glemail.com>
   */

+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
  #ifndef __USB_CORE_PHY_H_
  #define __USB_CORE_PHY_H_

  struct device;
  struct usb_phy_roothub;

-struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev);
+struct usb_phy_roothub *usb_phy_roothub_alloc(struct usb_hcd *hcd);

  int usb_phy_roothub_init(struct usb_phy_roothub *phy_roothub);
  int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub);
@@ -19,9 +22,9 @@
  int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub);
  void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub);

-int usb_phy_roothub_suspend(struct device *controller_dev,
+int usb_phy_roothub_suspend(struct usb_hcd *hcd,
  			    struct usb_phy_roothub *phy_roothub);
-int usb_phy_roothub_resume(struct device *controller_dev,
+int usb_phy_roothub_resume(struct usb_hcd *hcd,
  			   struct usb_phy_roothub *phy_roothub);

  #endif /* __USB_CORE_PHY_H_ */
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 97e2ddec18b1..87a104055b5e 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -157,6 +157,9 @@ struct usb_hcd {
  	 */
  	unsigned		skip_phy_initialization:1;

+	/* Some phys don't want the phy's exit/init called on suspend/resume */
+	unsigned		suspend_without_phy_exit:1;
+
  	/* The next flag is a stopgap, to be removed when all the HCDs
  	 * support the new root-hub polling mechanism. */
  	unsigned		uses_new_polling:1;
-- 
1.9.0.138.g2de3478

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ