Re: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

> I can reproduce this now when the device does not have primary fwnode
> (of_node or ACPI). Everything seems to work fine if there is the
> primary fwnode and when the build-in properties are used as the
> secondary fwnode (fallback).
> 
> This is a regression in drivers/base/property.c. Thanks for the
> report. I'll try to prepare a fix for it today. I'll let you know
> then so you can test it.

OK, I found a few problems with the fwnode handling. One problem is
that ACPI_COMPANION_SET macro will simply clear the primary fwnode
unconditionally without checking the fwnode type if NULL is passed to
it as parameter.

An other problem is that the secondary fwnode will have ERR_PTR in
some cases, I'm not completely sure why, but in any case,
drivers/base/property.c is not considering that at all.

I've prepared a small diff where I solve temporarily the first issue
by making sure the ACPI companion is set before the build-in
properties are attached to the platform device, and the second issue
quite simply by considering that the secondary fwnode may contain
ERR_PTR.

It can be applied on top of the two patches. Please give it a try and
let me know if it works for you.


Thanks,

-- 
heikki
diff --git a/drivers/base/property.c b/drivers/base/property.c
index c359351..a163f2c 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -218,7 +218,7 @@ bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
 	bool ret;
 
 	ret = __fwnode_property_present(fwnode, propname);
-	if (ret == false && fwnode && fwnode->secondary)
+	if (ret == false && fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
 		ret = __fwnode_property_present(fwnode->secondary, propname);
 	return ret;
 }
@@ -423,7 +423,7 @@ EXPORT_SYMBOL_GPL(device_property_match_string);
 	int _ret_;									\
 	_ret_ = FWNODE_PROP_READ(_fwnode_, _propname_, _type_, _proptype_,		\
 				 _val_, _nval_);					\
-	if (_ret_ == -EINVAL && _fwnode_ && _fwnode_->secondary)			\
+	if (_ret_ == -EINVAL && _fwnode_ && !IS_ERR_OR_NULL(_fwnode_->secondary))	\
 		_ret_ = FWNODE_PROP_READ(_fwnode_->secondary, _propname_, _type_,	\
 				_proptype_, _val_, _nval_);				\
 	_ret_;										\
@@ -593,7 +593,7 @@ int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
 	int ret;
 
 	ret = __fwnode_property_read_string_array(fwnode, propname, val, nval);
-	if (ret == -EINVAL && fwnode && fwnode->secondary)
+	if (ret == -EINVAL && fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
 		ret = __fwnode_property_read_string_array(fwnode->secondary,
 							  propname, val, nval);
 	return ret;
@@ -621,7 +621,7 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 	int ret;
 
 	ret = __fwnode_property_read_string(fwnode, propname, val);
-	if (ret == -EINVAL && fwnode && fwnode->secondary)
+	if (ret == -EINVAL && fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
 		ret = __fwnode_property_read_string(fwnode->secondary,
 						    propname, val);
 	return ret;
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 4a0dc81..c4628e9 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -927,6 +927,10 @@ static int dwc3_probe(struct platform_device *pdev)
 	device_property_read_u32(dev, "snps,quirk-frame-length-adjustment",
 				 &fladj);
 
+	dev_info(dev, "usb3_lpm_capable=%d\n", dwc->usb3_lpm_capable);
+	dev_info(dev, "has_lpm_erratum=%d\n", dwc->has_lpm_erratum);
+	dev_info(dev, "dis_enblslpm_quirk=%d\n", dwc->dis_enblslpm_quirk);
+
 	/* default to superspeed if no maximum_speed passed */
 	if (dwc->maximum_speed == USB_SPEED_UNKNOWN)
 		dwc->maximum_speed = USB_SPEED_SUPER;
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 96dbf24..de49816 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -56,6 +56,15 @@ dwc3_pci_add_pset(struct platform_device *dwc3, struct property_entry *ent)
 
 static int dwc3_pci_quirks(struct pci_dev *pdev)
 {
+	struct property_entry pentry[] = {
+		PROPERTY_ENTRY_BOOL("snps,usb3_lpm_capable"),
+		PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
+		PROPERTY_ENTRY_BOOL("snps,dis_enblslpm_quirk"),
+		{ },
+	};
+
+	return dwc3_pci_add_pset(pci_get_drvdata(pdev), pentry);
+
 	if (pdev->vendor == PCI_VENDOR_ID_AMD &&
 	    pdev->device == PCI_DEVICE_ID_AMD_NL_USB) {
 		struct property_entry pentry[] = {
@@ -169,14 +178,14 @@ static int dwc3_pci_probe(struct pci_dev *pci,
 		return ret;
 	}
 
+	dwc3->dev.parent = dev;
+	ACPI_COMPANION_SET(&dwc3->dev, ACPI_COMPANION(dev));
+
 	pci_set_drvdata(pci, dwc3);
 	ret = dwc3_pci_quirks(pci);
 	if (ret)
 		goto err;
 
-	dwc3->dev.parent = dev;
-	ACPI_COMPANION_SET(&dwc3->dev, ACPI_COMPANION(dev));
-
 	ret = platform_device_add(dwc3);
 	if (ret) {
 		dev_err(dev, "failed to register dwc3 device\n");

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux