diff mbox

[v1,1/4] PCI: imx6: enable pcie on imx6qdl sabreauto

Message ID 1411382705-15301-2-git-send-email-r65037@freescale.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Richard Zhu Sept. 22, 2014, 10:45 a.m. UTC 
- enable pcie on imx6qdl sabreauto boards.
- wait the clocks to stabilize after the pcie_ref_en
(IMX6Q_GPR1_PCIE_REF_CLK_EN) is set.

Signed-off-by: Richard Zhu <r65037@freescale.com>
---
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 4 ++++
 drivers/pci/host/pci-imx6.c              | 6 +++---
 2 files changed, 7 insertions(+), 3 deletions(-)

Comments

Lucas Stach Sept. 22, 2014, 12:06 p.m. UTC | #1 
Am Montag, den 22.09.2014, 18:45 +0800 schrieb Richard Zhu:
> - enable pcie on imx6qdl sabreauto boards.
> - wait the clocks to stabilize after the pcie_ref_en
> (IMX6Q_GPR1_PCIE_REF_CLK_EN) is set.
> 

Those are two completely independent changes, even if you enable/fix a
specific board with those. So please split into DT changes (to be merged
by Shawn) and PCI changes (to be merged by Bjorn).

> Signed-off-by: Richard Zhu <r65037@freescale.com>
> ---
>  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 4 ++++
>  drivers/pci/host/pci-imx6.c              | 6 +++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> index 009abd6..d6040a5 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> @@ -410,6 +410,10 @@
>  	};
>  };
>  
> +&pcie {
> +	status = "okay";
> +};
> +
>  &pwm3 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_pwm3>;
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 233fe8a..bc4222b 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -275,15 +275,15 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
>  		goto err_pcie;
>  	}
>  
> -	/* allow the clocks to stabilize */
> -	usleep_range(200, 500);
> -
>  	/* power up core phy and enable ref clock */
>  	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>  			IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
>  	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>  			IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
>  
> +	/* allow the clocks to stabilize */
> +	usleep_range(200, 500);
> +
>  	/* Some boards don't have PCIe reset GPIO. */
>  	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
>  		gpio_set_value(imx6_pcie->reset_gpio, 0);

Are you fixing a specific issue with this? Otherwise this change doesn't
look good to me. The reference manual states that the clock must be
stable before enabling the REF_CLK_EN bit in GPR1. So moving the delay
without proof that it doesn't have adverse effects and without a good
justification seems wrong.

If this fixes a specific issue please mention it in the commit message.
Also I would like at least a confirmation from Tim Harvey that this
doesn't regress his setup. So please take him on CC for the next version
of this patch.

Regards,
Lucas
Richard Zhu Sept. 23, 2014, 3:25 a.m. UTC | #2 
SGkgTHVjYXM6DQpUaGFua3MgZm9yIHlvdXIgY29tbWVudHMuDQoNCj4gLS0tLS1PcmlnaW5hbCBN
ZXNzYWdlLS0tLS0NCj4gRnJvbTogTHVjYXMgU3RhY2ggW21haWx0bzpsLnN0YWNoQHBlbmd1dHJv
bml4LmRlXQ0KPiBTZW50OiBNb25kYXksIFNlcHRlbWJlciAyMiwgMjAxNCA4OjA3IFBNDQo+IFRv
OiBaaHUgUmljaGFyZC1SNjUwMzcNCj4gQ2M6IGxpbnV4LXBjaS1vd25lckB2Z2VyLmtlcm5lbC5v
cmc7IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IEd1byBTaGF3bi0NCj4gUjY1MDczOyBmZXN0
ZXZhbUBnbWFpbC5jb20NCj4gU3ViamVjdDogUmU6IFtQQVRDSCB2MSAxLzRdIFBDSTogaW14Njog
ZW5hYmxlIHBjaWUgb24gaW14NnFkbCBzYWJyZWF1dG8NCj4gDQo+IEFtIE1vbnRhZywgZGVuIDIy
LjA5LjIwMTQsIDE4OjQ1ICswODAwIHNjaHJpZWIgUmljaGFyZCBaaHU6DQo+ID4gLSBlbmFibGUg
cGNpZSBvbiBpbXg2cWRsIHNhYnJlYXV0byBib2FyZHMuDQo+ID4gLSB3YWl0IHRoZSBjbG9ja3Mg
dG8gc3RhYmlsaXplIGFmdGVyIHRoZSBwY2llX3JlZl9lbg0KPiA+IChJTVg2UV9HUFIxX1BDSUVf
UkVGX0NMS19FTikgaXMgc2V0Lg0KPiA+DQo+IA0KPiBUaG9zZSBhcmUgdHdvIGNvbXBsZXRlbHkg
aW5kZXBlbmRlbnQgY2hhbmdlcywgZXZlbiBpZiB5b3UgZW5hYmxlL2ZpeCBhDQo+IHNwZWNpZmlj
IGJvYXJkIHdpdGggdGhvc2UuIFNvIHBsZWFzZSBzcGxpdCBpbnRvIERUIGNoYW5nZXMgKHRvIGJl
IG1lcmdlZCBieQ0KPiBTaGF3bikgYW5kIFBDSSBjaGFuZ2VzICh0byBiZSBtZXJnZWQgYnkgQmpv
cm4pLg0KPiANCltSaWNoYXJkXSBPaywgbm8gcHJvYmxlbS4gV291bGQgYmUgc2VwYXJhdGVkIGxh
dGVyLg0KDQo+ID4gU2lnbmVkLW9mZi1ieTogUmljaGFyZCBaaHUgPHI2NTAzN0BmcmVlc2NhbGUu
Y29tPg0KPiA+IC0tLQ0KPiA+ICBhcmNoL2FybS9ib290L2R0cy9pbXg2cWRsLXNhYnJlYXV0by5k
dHNpIHwgNCArKysrDQo+ID4gIGRyaXZlcnMvcGNpL2hvc3QvcGNpLWlteDYuYyAgICAgICAgICAg
ICAgfCA2ICsrKy0tLQ0KPiA+ICAyIGZpbGVzIGNoYW5nZWQsIDcgaW5zZXJ0aW9ucygrKSwgMyBk
ZWxldGlvbnMoLSkNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9hcmNoL2FybS9ib290L2R0cy9pbXg2
cWRsLXNhYnJlYXV0by5kdHNpDQo+ID4gYi9hcmNoL2FybS9ib290L2R0cy9pbXg2cWRsLXNhYnJl
YXV0by5kdHNpDQo+ID4gaW5kZXggMDA5YWJkNi4uZDYwNDBhNSAxMDA2NDQNCj4gPiAtLS0gYS9h
cmNoL2FybS9ib290L2R0cy9pbXg2cWRsLXNhYnJlYXV0by5kdHNpDQo+ID4gKysrIGIvYXJjaC9h
cm0vYm9vdC9kdHMvaW14NnFkbC1zYWJyZWF1dG8uZHRzaQ0KPiA+IEBAIC00MTAsNiArNDEwLDEw
IEBADQo+ID4gIAl9Ow0KPiA+ICB9Ow0KPiA+DQo+ID4gKyZwY2llIHsNCj4gPiArCXN0YXR1cyA9
ICJva2F5IjsNCj4gPiArfTsNCj4gPiArDQo+ID4gICZwd20zIHsNCj4gPiAgCXBpbmN0cmwtbmFt
ZXMgPSAiZGVmYXVsdCI7DQo+ID4gIAlwaW5jdHJsLTAgPSA8JnBpbmN0cmxfcHdtMz47DQo+ID4g
ZGlmZiAtLWdpdCBhL2RyaXZlcnMvcGNpL2hvc3QvcGNpLWlteDYuYyBiL2RyaXZlcnMvcGNpL2hv
c3QvcGNpLWlteDYuYw0KPiA+IGluZGV4IDIzM2ZlOGEuLmJjNDIyMmIgMTAwNjQ0DQo+ID4gLS0t
IGEvZHJpdmVycy9wY2kvaG9zdC9wY2ktaW14Ni5jDQo+ID4gKysrIGIvZHJpdmVycy9wY2kvaG9z
dC9wY2ktaW14Ni5jDQo+ID4gQEAgLTI3NSwxNSArMjc1LDE1IEBAIHN0YXRpYyBpbnQgaW14Nl9w
Y2llX2RlYXNzZXJ0X2NvcmVfcmVzZXQoc3RydWN0DQo+IHBjaWVfcG9ydCAqcHApDQo+ID4gIAkJ
Z290byBlcnJfcGNpZTsNCj4gPiAgCX0NCj4gPg0KPiA+IC0JLyogYWxsb3cgdGhlIGNsb2NrcyB0
byBzdGFiaWxpemUgKi8NCj4gPiAtCXVzbGVlcF9yYW5nZSgyMDAsIDUwMCk7DQo+ID4gLQ0KPiA+
ICAJLyogcG93ZXIgdXAgY29yZSBwaHkgYW5kIGVuYWJsZSByZWYgY2xvY2sgKi8NCj4gPiAgCXJl
Z21hcF91cGRhdGVfYml0cyhpbXg2X3BjaWUtPmlvbXV4Y19ncHIsIElPTVVYQ19HUFIxLA0KPiA+
ICAJCQlJTVg2UV9HUFIxX1BDSUVfVEVTVF9QRCwgMCA8PCAxOCk7DQo+ID4gIAlyZWdtYXBfdXBk
YXRlX2JpdHMoaW14Nl9wY2llLT5pb211eGNfZ3ByLCBJT01VWENfR1BSMSwNCj4gPiAgCQkJSU1Y
NlFfR1BSMV9QQ0lFX1JFRl9DTEtfRU4sIDEgPDwgMTYpOw0KPiA+DQo+ID4gKwkvKiBhbGxvdyB0
aGUgY2xvY2tzIHRvIHN0YWJpbGl6ZSAqLw0KPiA+ICsJdXNsZWVwX3JhbmdlKDIwMCwgNTAwKTsN
Cj4gPiArDQo+ID4gIAkvKiBTb21lIGJvYXJkcyBkb24ndCBoYXZlIFBDSWUgcmVzZXQgR1BJTy4g
Ki8NCj4gPiAgCWlmIChncGlvX2lzX3ZhbGlkKGlteDZfcGNpZS0+cmVzZXRfZ3BpbykpIHsNCj4g
PiAgCQlncGlvX3NldF92YWx1ZShpbXg2X3BjaWUtPnJlc2V0X2dwaW8sIDApOw0KPiANCj4gQXJl
IHlvdSBmaXhpbmcgYSBzcGVjaWZpYyBpc3N1ZSB3aXRoIHRoaXM/IE90aGVyd2lzZSB0aGlzIGNo
YW5nZSBkb2Vzbid0IGxvb2sNCj4gZ29vZCB0byBtZS4gVGhlIHJlZmVyZW5jZSBtYW51YWwgc3Rh
dGVzIHRoYXQgdGhlIGNsb2NrIG11c3QgYmUgc3RhYmxlIGJlZm9yZQ0KPiBlbmFibGluZyB0aGUg
UkVGX0NMS19FTiBiaXQgaW4gR1BSMS4gU28gbW92aW5nIHRoZSBkZWxheSB3aXRob3V0IHByb29m
IHRoYXQgaXQNCj4gZG9lc24ndCBoYXZlIGFkdmVyc2UgZWZmZWN0cyBhbmQgd2l0aG91dCBhIGdv
b2QganVzdGlmaWNhdGlvbiBzZWVtcyB3cm9uZy4NCj4gDQo+IElmIHRoaXMgZml4ZXMgYSBzcGVj
aWZpYyBpc3N1ZSBwbGVhc2UgbWVudGlvbiBpdCBpbiB0aGUgY29tbWl0IG1lc3NhZ2UuDQo+IEFs
c28gSSB3b3VsZCBsaWtlIGF0IGxlYXN0IGEgY29uZmlybWF0aW9uIGZyb20gVGltIEhhcnZleSB0
aGF0IHRoaXMgZG9lc24ndA0KPiByZWdyZXNzIGhpcyBzZXR1cC4gU28gcGxlYXNlIHRha2UgaGlt
IG9uIENDIGZvciB0aGUgbmV4dCB2ZXJzaW9uIG9mIHRoaXMgcGF0Y2guDQpbUmljaGFyZF0geWVz
LCBpdCBpcyB1c2VkIHRvIGZpeCBvbmUgaXNzdWUgb24gaW14NnFkbCBhcmQgYm9hcmRzLg0KIEFj
dHVhbGx5LCB0aGUgYSB3aGlsZSBkZWxheSBpcyBtYW5kYXRvcnkgcmVxdWlyZWQgYWZ0ZXIgcGNp
ZV9yZWZfY2xrX2VuIGlzIHNldC4NCk90aGVyd2lzZSwgdGhlIHN5c3RlbSB3b3VsZCBiZSBoYW5n
IG9uIGlteDZxZGwgYXJkIGJvYXJkcywgYmVjYXVzZSB0aGF0IGlteDZxZGwgYm9hcmRzDQogZG9u
J3QgaGF2ZSB0aGUgcmVzZXRfZ3Bpby4NCkJUVywgYXMgSSBrbm93IHRoYXQgdGhlIGNsb2NrcyBz
aG91bGQgYmUgc3RhYmxlIGFscmVhZHkgYWZ0ZXIgdGhlICJjbGtfcHJlcGFyZV9lbmFibGUiIGlz
DQpyZXR1cm4uDQpPaywgSSB3b3VsZCBDQyBUaW0gSGFydmV5IGluIHRoZSBuZXh0IHZlcnNpb24g
b2YgdGhpcyBwYXRjaC1zZXQuDQpUaGFua3MuDQoNCkJlc3QgUmVnYXJkcw0KUmljaGFyZCBaaHUN
Cj4gDQo+IFJlZ2FyZHMsDQo+IEx1Y2FzDQo+IC0tDQo+IFBlbmd1dHJvbml4IGUuSy4gICAgICAg
ICAgICAgfCBMdWNhcyBTdGFjaCAgICAgICAgICAgICAgICAgfA0KPiBJbmR1c3RyaWFsIExpbnV4
IFNvbHV0aW9ucyAgIHwgaHR0cDovL3d3dy5wZW5ndXRyb25peC5kZS8gIHwNCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..d6040a5 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -410,6 +410,10 @@ 
 	};
 };
 
+&pcie {
+	status = "okay";
+};
+
 &pwm3 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_pwm3>;
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 233fe8a..bc4222b 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -275,15 +275,15 @@  static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 		goto err_pcie;
 	}
 
-	/* allow the clocks to stabilize */
-	usleep_range(200, 500);
-
 	/* power up core phy and enable ref clock */
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
 
+	/* allow the clocks to stabilize */
+	usleep_range(200, 500);
+
 	/* Some boards don't have PCIe reset GPIO. */
 	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
 		gpio_set_value(imx6_pcie->reset_gpio, 0);