From: Gregory CLEMENT <gregory.clement@bootlin.com>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: "Andrew Lunn" <andrew@lunn.ch>,
"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
"Jason Cooper" <jason@lakedaemon.net>,
"Antoine Tenart" <antoine.tenart@bootlin.com>,
linux-pci@vger.kernel.org, "Hanna Hawa" <hannah@marvell.com>,
"Omri Itach" <omrii@marvell.com>,
"Nadav Haklai" <nadavh@marvell.com>,
"Shadi Ammouri" <shadi@marvell.com>,
"Igal Liberman" <igall@marvell.com>,
"Miquèl Raynal" <miquel.raynal@bootlin.com>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Marcin Wojtas" <mw@semihalf.com>,
linux-arm-kernel@lists.infradead.org,
"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH 2/2] PCI: armada8k: Fix clock resource by adding a register clock
Date: Wed, 28 Feb 2018 16:37:24 +0100 [thread overview]
Message-ID: <874lm1f7m3.fsf@bootlin.com> (raw)
In-Reply-To: <20180228155350.0221a90f@windsurf.lan> (Thomas Petazzoni's message of "Wed, 28 Feb 2018 15:53:50 +0100")
SGkgVGhvbWFzLAogCiBPbiBtZXIuLCBmw6l2ci4gMjggMjAxOCwgVGhvbWFzIFBldGF6em9uaSA8
dGhvbWFzLnBldGF6em9uaUBib290bGluLmNvbT4gd3JvdGU6Cgo+IEhlbGxvLAo+Cj4gT24gV2Vk
LCAyOCBGZWIgMjAxOCAxNTo0NzowNCArMDEwMCwgR3JlZ29yeSBDTEVNRU5UIHdyb3RlOgo+PiBP
biBBcm1hZGEgN0svOEsgd2UgbmVlZCB0byBleHBsaWNpdGx5IGVuYWJsZSB0aGUgcmVnaXN0ZXIg
Y2xvY2suIFRoaXMKPj4gY2xvY2sgaXMgb3B0aW9uYWwgYmVjYXVzZSBub3QgYWxsIHRoZSBTb0Nz
IHVzaW5nIHRoaXMgSVAgbmVlZCBpdCBidXQgYXQKPj4gbGVhc3QgZm9yIEFybWFkYSA3Sy84SyBp
dCBpcyBhY3R1YWxseSBtYW5kYXRvcnkuCj4+IAo+PiBUaGUgYmluZGluZyBkb2N1bWVudGF0aW9u
IGlzIHVwZGF0ZWQgYWNjb3JkaW5nbHkuCj4+IAo+PiBTaWduZWQtb2ZmLWJ5OiBHcmVnb3J5IENM
RU1FTlQgPGdyZWdvcnkuY2xlbWVudEBib290bGluLmNvbT4KPj4gLS0tCj4+ICBEb2N1bWVudGF0
aW9uL2RldmljZXRyZWUvYmluZGluZ3MvcGNpL3BjaS1hcm1hZGE4ay50eHQgfCAgNiArKysrKy0K
Pj4gIGRyaXZlcnMvcGNpL2R3Yy9wY2llLWFybWFkYThrLmMgICAgICAgICAgICAgICAgICAgICAg
ICB8IDExICsrKysrKysrKysrCj4+ICAyIGZpbGVzIGNoYW5nZWQsIDE2IGluc2VydGlvbnMoKyks
IDEgZGVsZXRpb24oLSkKPj4gCj4+IGRpZmYgLS1naXQgYS9Eb2N1bWVudGF0aW9uL2RldmljZXRy
ZWUvYmluZGluZ3MvcGNpL3BjaS1hcm1hZGE4ay50eHQgYi9Eb2N1bWVudGF0aW9uL2RldmljZXRy
ZWUvYmluZGluZ3MvcGNpL3BjaS1hcm1hZGE4ay50eHQKPj4gaW5kZXggYzFlNGMzZDEwYTc0Li45
OTQ4YjFlOWE4ZTUgMTAwNjQ0Cj4+IC0tLSBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5k
aW5ncy9wY2kvcGNpLWFybWFkYThrLnR4dAo+PiArKysgYi9Eb2N1bWVudGF0aW9uL2RldmljZXRy
ZWUvYmluZGluZ3MvcGNpL3BjaS1hcm1hZGE4ay50eHQKPj4gQEAgLTEyLDcgKzEyLDExIEBAIFJl
cXVpcmVkIHByb3BlcnRpZXM6Cj4+ICAgICAtICJjdHJsIiBmb3IgdGhlIGNvbnRyb2wgcmVnaXN0
ZXIgcmVnaW9uCj4+ICAgICAtICJjb25maWciIGZvciB0aGUgY29uZmlnIHNwYWNlIHJlZ2lvbgo+
PiAgLSBpbnRlcnJ1cHRzOiBJbnRlcnJ1cHQgc3BlY2lmaWVyIGZvciB0aGUgUENJZSBjb250cm9s
ZXIKPj4gLS0gY2xvY2tzOiByZWZlcmVuY2UgdG8gdGhlIFBDSWUgY29udHJvbGxlciBjbG9jawo+
PiArLSBjbG9ja3M6IHJlZmVyZW5jZSB0byB0aGUgUENJZSBjb250cm9sbGVyIGNsb2Nrcwo+PiAr
LSBjbG9jay1uYW1lczogbWFuZGF0b3J5IGlmIHRoZXJlIGlzIGEgc2Vjb25kIGNsb2NrLCBpbiB0
aGlzIGNhc2UgdGhlCj4+ICsgICBuYW1lIG11c3QgYmUgImNvcmUiIGZvciB0aGUgZmlyc3QgY2xv
Y2sgYW5kICJyZWciIGZvciB0aGUgc2Vjb25kCj4+ICsgICBvbmUKPj4gKwo+Cj4gVW5uZWVkZWQg
bmV3IGxpbmUgYWRkZWQgaGVyZS4Kd2lsbCByZW1vdmVkIGl0Cgo+Cj4+ICAKPj4gIEV4YW1wbGU6
Cj4+ICAKPj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvcGNpL2R3Yy9wY2llLWFybWFkYThrLmMgYi9k
cml2ZXJzL3BjaS9kd2MvcGNpZS1hcm1hZGE4ay5jCj4+IGluZGV4IGY5YjFhZWMyNWM1Yy4uYWE0
ZTVjYzRhYjdiIDEwMDY0NAo+PiAtLS0gYS9kcml2ZXJzL3BjaS9kd2MvcGNpZS1hcm1hZGE4ay5j
Cj4+ICsrKyBiL2RyaXZlcnMvcGNpL2R3Yy9wY2llLWFybWFkYThrLmMKPj4gQEAgLTI4LDYgKzI4
LDcgQEAKPj4gIHN0cnVjdCBhcm1hZGE4a19wY2llIHsKPj4gIAlzdHJ1Y3QgZHdfcGNpZSAqcGNp
Owo+PiAgCXN0cnVjdCBjbGsgKmNsazsKPj4gKwlzdHJ1Y3QgY2xrICpjbGtfcmVnOwo+PiAgfTsK
Pj4gIAo+PiAgI2RlZmluZSBQQ0lFX1ZFTkRPUl9SRUdTX09GRlNFVAkJMHg4MDAwCj4+IEBAIC0y
MjksNiArMjMwLDE1IEBAIHN0YXRpYyBpbnQgYXJtYWRhOGtfcGNpZV9wcm9iZShzdHJ1Y3QgcGxh
dGZvcm1fZGV2aWNlICpwZGV2KQo+PiAgCWlmIChyZXQpCj4+ICAJCXJldHVybiByZXQ7Cj4+ICAK
Pj4gKwlpZiAoSVNfRVJSKHBjaWUtPmNsa19yZWcpICYmIFBUUl9FUlIocGNpZS0+Y2xrX3JlZykg
PT0gLUVQUk9CRV9ERUZFUikgewo+PiArCQljbGtfZGlzYWJsZV91bnByZXBhcmUocGNpZS0+Y2xr
KTsKPj4gKwkJcmV0dXJuIC1FUFJPQkVfREVGRVI7Cj4+ICsJfQo+PiArCWlmICghSVNfRVJSKHBj
aWUtPmNsa19yZWcpKSB7Cj4+ICsJCXJldCA9IGNsa19wcmVwYXJlX2VuYWJsZShwY2llLT5jbGtf
cmVnKTsKPj4gKwkJaWYgKHJldCkKPj4gKwkJCWdvdG8gZmFpbDsKPj4gKwl9Cj4+ICAJLyogR2V0
IHRoZSBkdy1wY2llIHVuaXQgY29uZmlndXJhdGlvbi9jb250cm9sIHJlZ2lzdGVycyBiYXNlLiAq
Lwo+Cj4gTWlzc2luZyBuZXcgbGluZSBiZXR3ZWVuIHRoZSBlbmQgb2YgdGhlIGJsb2NrIGFuZCB0
aGUgbmV4dCBjb21tZW50Lgo+Ck9LCgo+IFJlZ2FyZGluZyB0aGUgZXJyb3IgaGFuZGxpbmcsIGRv
ZXNuJ3QgaXQgbWFrZSBtb3JlIHNlbnNlIHRvIGFsc28gdXNlIGEKPiBnb3RvIGxhYmVsIHRvIGRp
c2FibGUgcGNpZS0+Y2xrIHdoZW4gZ2V0dGluZyB0aGUgc2Vjb25kIGNsb2NrIGdldHMgYQo+IC1F
UFJPQkVfREVGRVIgPwo+Cj4+ICAJYmFzZSA9IHBsYXRmb3JtX2dldF9yZXNvdXJjZV9ieW5hbWUo
cGRldiwgSU9SRVNPVVJDRV9NRU0sICJjdHJsIik7Cj4+ICAJcGNpLT5kYmlfYmFzZSA9IGRldm1f
cGNpX3JlbWFwX2NmZ19yZXNvdXJjZShkZXYsIGJhc2UpOwo+PiBAQCAtMjQ3LDYgKzI1Nyw3IEBA
IHN0YXRpYyBpbnQgYXJtYWRhOGtfcGNpZV9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpw
ZGV2KQo+PiAgCXJldHVybiAwOwo+PiAgCj4+ICBmYWlsOgo+PiArCWNsa19kaXNhYmxlX3VucHJl
cGFyZShwY2llLT5jbGtfcmVnKTsKPgo+IFNvIHlvdSBhcmUgZGlzYWJsaW5nL3VucHJlcGFyaW5n
IHRoZSBjbG9jaywgd2hpY2ggZmFpbGVkIHRvCj4gcHJlcGFyZS9lbmFibGUgPwoKSSB3YXMgdGhp
bmtpbmcgdG8gYSBzaW5nbGUgdXNlciwgaW4gdGhpcyBjYXNlIGlmIHRoZSBwcmVwYXJlL2VuYWJs
ZQpmYWlsZWQgdGhlbiB0aGUgZGlzYWJsaW5nL3VucHJlcGFyaW5nIGRvIG5vdGhpbmcgYmVjYXVz
ZSB0aGUgY291bnRlciBpcwphbHJlYWR5IHRvIDAuIEJ1dCBpbmRlZWQgaW4gY2FzZSBvZiBtdWx0
aXBsZSB1c2VycyBvZiB0aGUgY2xvY2ssIHRoZW4gdGhlCmNvdW50ZXIgY291bGQgYmUgd3Jvbmds
eSBkZWNyZWFzZS4gSSB3aWxsIG1vZGlmeSBpdCBieSBhZGRpbmcgYSBvdGhlcgpsYWJlbC4KClRo
YW5rcywKCkdyZWdvcnkKCj4KPj4gIAljbGtfZGlzYWJsZV91bnByZXBhcmUocGNpZS0+Y2xrKTsK
Pj4gIAo+PiAgCXJldHVybiByZXQ7Cj4KPiBUaG9tYXMKPiAtLSAKPiBUaG9tYXMgUGV0YXp6b25p
LCBDVE8sIEJvb3RsaW4gKGZvcm1lcmx5IEZyZWUgRWxlY3Ryb25zKQo+IEVtYmVkZGVkIExpbnV4
IGFuZCBLZXJuZWwgZW5naW5lZXJpbmcKPiBodHRwOi8vYm9vdGxpbi5jb20KCi0tIApHcmVnb3J5
IENsZW1lbnQsIEJvb3RsaW4gKGZvcm1lcmx5IEZyZWUgRWxlY3Ryb25zKQpFbWJlZGRlZCBMaW51
eCBhbmQgS2VybmVsIGVuZ2luZWVyaW5nCmh0dHA6Ly9ib290bGluLmNvbQoKX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbGludXgtYXJtLWtlcm5lbCBtYWls
aW5nIGxpc3QKbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0
cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5lbAo=
WARNING: multiple messages have this Message-ID (diff)
From: gregory.clement@bootlin.com (Gregory CLEMENT)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] PCI: armada8k: Fix clock resource by adding a register clock
Date: Wed, 28 Feb 2018 16:37:24 +0100 [thread overview]
Message-ID: <874lm1f7m3.fsf@bootlin.com> (raw)
In-Reply-To: <20180228155350.0221a90f@windsurf.lan> (Thomas Petazzoni's message of "Wed, 28 Feb 2018 15:53:50 +0100")
Hi Thomas,
On mer., f?vr. 28 2018, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
> Hello,
>
> On Wed, 28 Feb 2018 15:47:04 +0100, Gregory CLEMENT wrote:
>> On Armada 7K/8K we need to explicitly enable the register clock. This
>> clock is optional because not all the SoCs using this IP need it but at
>> least for Armada 7K/8K it is actually mandatory.
>>
>> The binding documentation is updated accordingly.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> ---
>> Documentation/devicetree/bindings/pci/pci-armada8k.txt | 6 +++++-
>> drivers/pci/dwc/pcie-armada8k.c | 11 +++++++++++
>> 2 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pci-armada8k.txt b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
>> index c1e4c3d10a74..9948b1e9a8e5 100644
>> --- a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
>> +++ b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
>> @@ -12,7 +12,11 @@ Required properties:
>> - "ctrl" for the control register region
>> - "config" for the config space region
>> - interrupts: Interrupt specifier for the PCIe controler
>> -- clocks: reference to the PCIe controller clock
>> +- clocks: reference to the PCIe controller clocks
>> +- clock-names: mandatory if there is a second clock, in this case the
>> + name must be "core" for the first clock and "reg" for the second
>> + one
>> +
>
> Unneeded new line added here.
will removed it
>
>>
>> Example:
>>
>> diff --git a/drivers/pci/dwc/pcie-armada8k.c b/drivers/pci/dwc/pcie-armada8k.c
>> index f9b1aec25c5c..aa4e5cc4ab7b 100644
>> --- a/drivers/pci/dwc/pcie-armada8k.c
>> +++ b/drivers/pci/dwc/pcie-armada8k.c
>> @@ -28,6 +28,7 @@
>> struct armada8k_pcie {
>> struct dw_pcie *pci;
>> struct clk *clk;
>> + struct clk *clk_reg;
>> };
>>
>> #define PCIE_VENDOR_REGS_OFFSET 0x8000
>> @@ -229,6 +230,15 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>> if (ret)
>> return ret;
>>
>> + if (IS_ERR(pcie->clk_reg) && PTR_ERR(pcie->clk_reg) == -EPROBE_DEFER) {
>> + clk_disable_unprepare(pcie->clk);
>> + return -EPROBE_DEFER;
>> + }
>> + if (!IS_ERR(pcie->clk_reg)) {
>> + ret = clk_prepare_enable(pcie->clk_reg);
>> + if (ret)
>> + goto fail;
>> + }
>> /* Get the dw-pcie unit configuration/control registers base. */
>
> Missing new line between the end of the block and the next comment.
>
OK
> Regarding the error handling, doesn't it make more sense to also use a
> goto label to disable pcie->clk when getting the second clock gets a
> -EPROBE_DEFER ?
>
>> base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl");
>> pci->dbi_base = devm_pci_remap_cfg_resource(dev, base);
>> @@ -247,6 +257,7 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>> return 0;
>>
>> fail:
>> + clk_disable_unprepare(pcie->clk_reg);
>
> So you are disabling/unpreparing the clock, which failed to
> prepare/enable ?
I was thinking to a single user, in this case if the prepare/enable
failed then the disabling/unpreparing do nothing because the counter is
already to 0. But indeed in case of multiple users of the clock, then the
counter could be wrongly decrease. I will modify it by adding a other
label.
Thanks,
Gregory
>
>> clk_disable_unprepare(pcie->clk);
>>
>> return ret;
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> http://bootlin.com
--
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
next prev parent reply other threads:[~2018-02-28 15:37 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-28 14:47 [PATCH 0/2] PCI: armada8k: Fix clock resource for Armada 7K/8K Gregory CLEMENT
2018-02-28 14:47 ` Gregory CLEMENT
2018-02-28 14:47 ` [PATCH 1/2] PCI: armada8k: Remove useless test before clk_disable_unprepare Gregory CLEMENT
2018-02-28 14:47 ` Gregory CLEMENT
2018-02-28 14:47 ` [PATCH 2/2] PCI: armada8k: Fix clock resource by adding a register clock Gregory CLEMENT
2018-02-28 14:47 ` Gregory CLEMENT
2018-02-28 14:53 ` Thomas Petazzoni
2018-02-28 14:53 ` Thomas Petazzoni
2018-02-28 15:37 ` Gregory CLEMENT [this message]
2018-02-28 15:37 ` Gregory CLEMENT
2018-02-28 15:27 ` Russell King - ARM Linux
2018-02-28 15:27 ` Russell King - ARM Linux
2018-02-28 15:31 ` Gregory CLEMENT
2018-02-28 15:31 ` Gregory CLEMENT
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874lm1f7m3.fsf@bootlin.com \
--to=gregory.clement@bootlin.com \
--cc=andrew@lunn.ch \
--cc=antoine.tenart@bootlin.com \
--cc=bhelgaas@google.com \
--cc=hannah@marvell.com \
--cc=igall@marvell.com \
--cc=jason@lakedaemon.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=miquel.raynal@bootlin.com \
--cc=mw@semihalf.com \
--cc=nadavh@marvell.com \
--cc=omrii@marvell.com \
--cc=sebastian.hesselbarth@gmail.com \
--cc=shadi@marvell.com \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.