From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.bootlin.com ([62.4.15.54]:43034 "EHLO mail.bootlin.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751849AbeDJJyv (ORCPT ); Tue, 10 Apr 2018 05:54:51 -0400 Date: Tue, 10 Apr 2018 11:54:39 +0200 From: Alexandre Belloni To: Marcin Ziemianowicz Cc: Boris Brezillon , Nicolas Ferre , Greg Kroah-Hartman , Michael Turquette , Stephen Boyd , Alan Stern , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [PATCH v2 1/2] clk: at91: Added more information logging. Message-ID: <20180410095439.GA2745@piout.net> References: <20180410001621.GA62230@hak8or> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20180410001621.GA62230@hak8or> Sender: linux-clk-owner@vger.kernel.org List-ID: Hi, On 09/04/2018 20:16:21-0400, Marcin Ziemianowicz wrote: > I noticed that when debugging some USB clocking issue that there weren't > many ways to tell what the state of the USB clocking system was. This > adds a few logging statements to see what the relevant code is trying to > do. > > Signed-off-by: Marcin Ziemianowicz > --- > drivers/clk/at91/clk-pll.c | 6 +++++- > drivers/clk/at91/clk-usb.c | 10 ++++++++-- > drivers/usb/host/ohci-at91.c | 16 ++++++++++------ > 3 files changed, 23 insertions(+), 9 deletions(-) > > diff --git a/drivers/clk/at91/clk-pll.c b/drivers/clk/at91/clk-pll.c > index 7d3223fc7161..534961766ae5 100644 > --- a/drivers/clk/at91/clk-pll.c > +++ b/drivers/clk/at91/clk-pll.c > @@ -133,6 +133,7 @@ static unsigned long clk_pll_recalc_rate(struct clk_hw *hw, > { > struct clk_pll *pll = to_clk_pll(hw); > unsigned int pllr; > + unsigned long recalcedrate; > u16 mul; > u8 div; > > @@ -144,7 +145,10 @@ static unsigned long clk_pll_recalc_rate(struct clk_hw *hw, > if (!div || !mul) > return 0; > > - return (parent_rate / div) * (mul + 1); > + recalcedrate = (parent_rate / div) * (mul + 1); > + pr_debug("clk-pll: calculating new rate, (%lu hz / %u) * %u = %lu hz\n", > + parent_rate, div, mul, recalcedrate); This is not properly aligned. Also, I'm not a huge fan of adding that kind of debug messages. Isn't the rate already present in debugfs ? > + return recalcedrate; > } > > static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate, > diff --git a/drivers/clk/at91/clk-usb.c b/drivers/clk/at91/clk-usb.c > index 791770a563fc..2fa877e99bac 100644 > --- a/drivers/clk/at91/clk-usb.c > +++ b/drivers/clk/at91/clk-usb.c > @@ -48,11 +48,15 @@ static unsigned long at91sam9x5_clk_usb_recalc_rate(struct clk_hw *hw, > struct at91sam9x5_clk_usb *usb = to_at91sam9x5_clk_usb(hw); > unsigned int usbr; > u8 usbdiv; > + unsigned int calcdclock; > > regmap_read(usb->regmap, AT91_PMC_USB, &usbr); > usbdiv = (usbr & AT91_PMC_OHCIUSBDIV) >> SAM9X5_USB_DIV_SHIFT; > > - return DIV_ROUND_CLOSEST(parent_rate, (usbdiv + 1)); > + calcdclock = DIV_ROUND_CLOSEST(parent_rate, (usbdiv + 1)); > + pr_debug("clk-usb: calculating new rate, %lu hz / %u = %u hz\n", > + parent_rate, usbdiv + 1, calcdclock); > + return calcdclock; > } > > static int at91sam9x5_clk_usb_determine_rate(struct clk_hw *hw, > @@ -98,7 +102,6 @@ static int at91sam9x5_clk_usb_determine_rate(struct clk_hw *hw, > if (!best_diff) > break; > } > - Unrelated change > if (best_rate < 0) > return best_rate; > > @@ -142,6 +145,9 @@ static int at91sam9x5_clk_usb_set_rate(struct clk_hw *hw, unsigned long rate, > if (div > SAM9X5_USB_MAX_DIV + 1 || !div) > return -EINVAL; > > + pr_debug("clk-usb: setting USB clock divider to %lu hz / %lu = %lu hz\n", > + parent_rate, div, rate); > + > regmap_update_bits(usb->regmap, AT91_PMC_USB, AT91_PMC_OHCIUSBDIV, > (div - 1) << SAM9X5_USB_DIV_SHIFT); > > diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c > index 5ad9e9bdc8ee..c57a239918f9 100644 > --- a/drivers/usb/host/ohci-at91.c > +++ b/drivers/usb/host/ohci-at91.c > @@ -70,11 +70,13 @@ static const struct ohci_driver_overrides ohci_at91_drv_overrides __initconst = > > /*-------------------------------------------------------------------------*/ > > -static void at91_start_clock(struct ohci_at91_priv *ohci_at91) > +static void at91_start_clock(struct ohci_at91_priv *ohci_at91, > + struct device *dev) This is not properly aligned either > { > if (ohci_at91->clocked) > return; > > + dev_dbg(dev, "Enabling hclk, iclk, and setting fclk to 48 Mhz\n"); > clk_set_rate(ohci_at91->fclk, 48000000); > clk_prepare_enable(ohci_at91->hclk); > clk_prepare_enable(ohci_at91->iclk); > @@ -82,11 +84,13 @@ static void at91_start_clock(struct ohci_at91_priv *ohci_at91) > ohci_at91->clocked = true; > } > > -static void at91_stop_clock(struct ohci_at91_priv *ohci_at91) > +static void at91_stop_clock(struct ohci_at91_priv *ohci_at91, > + struct device *dev) > { > if (!ohci_at91->clocked) > return; > > + dev_dbg(dev, "Disabling hclk, iclk, and fclk\n"); > clk_disable_unprepare(ohci_at91->fclk); > clk_disable_unprepare(ohci_at91->iclk); > clk_disable_unprepare(ohci_at91->hclk); > @@ -104,7 +108,7 @@ static void at91_start_hc(struct platform_device *pdev) > /* > * Start the USB clocks. > */ > - at91_start_clock(ohci_at91); > + at91_start_clock(ohci_at91, &pdev->dev); > > /* > * The USB host controller must remain in reset. > @@ -128,7 +132,7 @@ static void at91_stop_hc(struct platform_device *pdev) > /* > * Stop the USB clocks. > */ > - at91_stop_clock(ohci_at91); > + at91_stop_clock(ohci_at91, &pdev->dev); > } > > > @@ -623,7 +627,7 @@ ohci_hcd_at91_drv_suspend(struct device *dev) > > /* flush the writes */ > (void) ohci_readl (ohci, &ohci->regs->control); > - at91_stop_clock(ohci_at91); > + at91_stop_clock(ohci_at91, dev); > } > > return ret; > @@ -638,7 +642,7 @@ ohci_hcd_at91_drv_resume(struct device *dev) > if (ohci_at91->wakeup) > disable_irq_wake(hcd->irq); > > - at91_start_clock(ohci_at91); > + at91_start_clock(ohci_at91, dev); > > ohci_resume(hcd, false); > > -- > 2.17.0 > > -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v2,1/2] clk: at91: Added more information logging. From: Alexandre Belloni Message-Id: <20180410095439.GA2745@piout.net> Date: Tue, 10 Apr 2018 11:54:39 +0200 To: Marcin Ziemianowicz Cc: Boris Brezillon , Nicolas Ferre , Greg Kroah-Hartman , Michael Turquette , Stephen Boyd , Alan Stern , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org List-ID: SGksCgpPbiAwOS8wNC8yMDE4IDIwOjE2OjIxLTA0MDAsIE1hcmNpbiBaaWVtaWFub3dpY3ogd3Jv dGU6Cj4gSSBub3RpY2VkIHRoYXQgd2hlbiBkZWJ1Z2dpbmcgc29tZSBVU0IgY2xvY2tpbmcgaXNz dWUgdGhhdCB0aGVyZSB3ZXJlbid0Cj4gbWFueSB3YXlzIHRvIHRlbGwgd2hhdCB0aGUgc3RhdGUg b2YgdGhlIFVTQiBjbG9ja2luZyBzeXN0ZW0gd2FzLiBUaGlzCj4gYWRkcyBhIGZldyBsb2dnaW5n IHN0YXRlbWVudHMgdG8gc2VlIHdoYXQgdGhlIHJlbGV2YW50IGNvZGUgaXMgdHJ5aW5nIHRvCj4g ZG8uCj4gCj4gU2lnbmVkLW9mZi1ieTogTWFyY2luIFppZW1pYW5vd2ljeiA8bWFyY2luQHppZW1p YW5vd2ljei5jb20+Cj4gLS0tCj4gIGRyaXZlcnMvY2xrL2F0OTEvY2xrLXBsbC5jICAgfCAgNiAr KysrKy0KPiAgZHJpdmVycy9jbGsvYXQ5MS9jbGstdXNiLmMgICB8IDEwICsrKysrKysrLS0KPiAg ZHJpdmVycy91c2IvaG9zdC9vaGNpLWF0OTEuYyB8IDE2ICsrKysrKysrKystLS0tLS0KPiAgMyBm aWxlcyBjaGFuZ2VkLCAyMyBpbnNlcnRpb25zKCspLCA5IGRlbGV0aW9ucygtKQo+IAo+IGRpZmYg LS1naXQgYS9kcml2ZXJzL2Nsay9hdDkxL2Nsay1wbGwuYyBiL2RyaXZlcnMvY2xrL2F0OTEvY2xr LXBsbC5jCj4gaW5kZXggN2QzMjIzZmM3MTYxLi41MzQ5NjE3NjZhZTUgMTAwNjQ0Cj4gLS0tIGEv ZHJpdmVycy9jbGsvYXQ5MS9jbGstcGxsLmMKPiArKysgYi9kcml2ZXJzL2Nsay9hdDkxL2Nsay1w bGwuYwo+IEBAIC0xMzMsNiArMTMzLDcgQEAgc3RhdGljIHVuc2lnbmVkIGxvbmcgY2xrX3BsbF9y ZWNhbGNfcmF0ZShzdHJ1Y3QgY2xrX2h3ICpodywKPiAgewo+ICAJc3RydWN0IGNsa19wbGwgKnBs bCA9IHRvX2Nsa19wbGwoaHcpOwo+ICAJdW5zaWduZWQgaW50IHBsbHI7Cj4gKwl1bnNpZ25lZCBs b25nIHJlY2FsY2VkcmF0ZTsKPiAgCXUxNiBtdWw7Cj4gIAl1OCBkaXY7Cj4gIAo+IEBAIC0xNDQs NyArMTQ1LDEwIEBAIHN0YXRpYyB1bnNpZ25lZCBsb25nIGNsa19wbGxfcmVjYWxjX3JhdGUoc3Ry dWN0IGNsa19odyAqaHcsCj4gIAlpZiAoIWRpdiB8fCAhbXVsKQo+ICAJCXJldHVybiAwOwo+ICAK PiAtCXJldHVybiAocGFyZW50X3JhdGUgLyBkaXYpICogKG11bCArIDEpOwo+ICsJcmVjYWxjZWRy YXRlID0gKHBhcmVudF9yYXRlIC8gZGl2KSAqIChtdWwgKyAxKTsKPiArCXByX2RlYnVnKCJjbGst cGxsOiBjYWxjdWxhdGluZyBuZXcgcmF0ZSwgKCVsdSBoeiAvICV1KSAqICV1ID0gJWx1IGh6XG4i LAo+ICsJCXBhcmVudF9yYXRlLCBkaXYsIG11bCwgcmVjYWxjZWRyYXRlKTsKVGhpcyBpcyBub3Qg cHJvcGVybHkgYWxpZ25lZC4gQWxzbywgSSdtIG5vdCBhIGh1Z2UgZmFuIG9mIGFkZGluZyB0aGF0 CmtpbmQgb2YgZGVidWcgbWVzc2FnZXMuIElzbid0IHRoZSByYXRlIGFscmVhZHkgcHJlc2VudCBp biBkZWJ1Z2ZzID8KCj4gKwlyZXR1cm4gcmVjYWxjZWRyYXRlOwo+ICB9Cj4gIAo+ICBzdGF0aWMg bG9uZyBjbGtfcGxsX2dldF9iZXN0X2Rpdl9tdWwoc3RydWN0IGNsa19wbGwgKnBsbCwgdW5zaWdu ZWQgbG9uZyByYXRlLAo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2Nsay9hdDkxL2Nsay11c2IuYyBi L2RyaXZlcnMvY2xrL2F0OTEvY2xrLXVzYi5jCj4gaW5kZXggNzkxNzcwYTU2M2ZjLi4yZmE4Nzdl OTliYWMgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9jbGsvYXQ5MS9jbGstdXNiLmMKPiArKysgYi9k cml2ZXJzL2Nsay9hdDkxL2Nsay11c2IuYwo+IEBAIC00OCwxMSArNDgsMTUgQEAgc3RhdGljIHVu c2lnbmVkIGxvbmcgYXQ5MXNhbTl4NV9jbGtfdXNiX3JlY2FsY19yYXRlKHN0cnVjdCBjbGtfaHcg Kmh3LAo+ICAJc3RydWN0IGF0OTFzYW05eDVfY2xrX3VzYiAqdXNiID0gdG9fYXQ5MXNhbTl4NV9j bGtfdXNiKGh3KTsKPiAgCXVuc2lnbmVkIGludCB1c2JyOwo+ICAJdTggdXNiZGl2Owo+ICsJdW5z aWduZWQgaW50IGNhbGNkY2xvY2s7Cj4gIAo+ICAJcmVnbWFwX3JlYWQodXNiLT5yZWdtYXAsIEFU OTFfUE1DX1VTQiwgJnVzYnIpOwo+ICAJdXNiZGl2ID0gKHVzYnIgJiBBVDkxX1BNQ19PSENJVVNC RElWKSA+PiBTQU05WDVfVVNCX0RJVl9TSElGVDsKPiAgCj4gLQlyZXR1cm4gRElWX1JPVU5EX0NM T1NFU1QocGFyZW50X3JhdGUsICh1c2JkaXYgKyAxKSk7Cj4gKwljYWxjZGNsb2NrID0gRElWX1JP VU5EX0NMT1NFU1QocGFyZW50X3JhdGUsICh1c2JkaXYgKyAxKSk7Cj4gKwlwcl9kZWJ1ZygiY2xr LXVzYjogY2FsY3VsYXRpbmcgbmV3IHJhdGUsICVsdSBoeiAvICV1ID0gJXUgaHpcbiIsCj4gKwkJ cGFyZW50X3JhdGUsIHVzYmRpdiArIDEsIGNhbGNkY2xvY2spOwo+ICsJcmV0dXJuIGNhbGNkY2xv Y2s7Cj4gIH0KPiAgCj4gIHN0YXRpYyBpbnQgYXQ5MXNhbTl4NV9jbGtfdXNiX2RldGVybWluZV9y YXRlKHN0cnVjdCBjbGtfaHcgKmh3LAo+IEBAIC05OCw3ICsxMDIsNiBAQCBzdGF0aWMgaW50IGF0 OTFzYW05eDVfY2xrX3VzYl9kZXRlcm1pbmVfcmF0ZShzdHJ1Y3QgY2xrX2h3ICpodywKPiAgCQlp ZiAoIWJlc3RfZGlmZikKPiAgCQkJYnJlYWs7Cj4gIAl9Cj4gLQoKVW5yZWxhdGVkIGNoYW5nZQoK PiAgCWlmIChiZXN0X3JhdGUgPCAwKQo+ICAJCXJldHVybiBiZXN0X3JhdGU7Cj4gIAo+IEBAIC0x NDIsNiArMTQ1LDkgQEAgc3RhdGljIGludCBhdDkxc2FtOXg1X2Nsa191c2Jfc2V0X3JhdGUoc3Ry dWN0IGNsa19odyAqaHcsIHVuc2lnbmVkIGxvbmcgcmF0ZSwKPiAgCWlmIChkaXYgPiBTQU05WDVf VVNCX01BWF9ESVYgKyAxIHx8ICFkaXYpCj4gIAkJcmV0dXJuIC1FSU5WQUw7Cj4gIAo+ICsJcHJf ZGVidWcoImNsay11c2I6IHNldHRpbmcgVVNCIGNsb2NrIGRpdmlkZXIgdG8gJWx1IGh6IC8gJWx1 ID0gJWx1IGh6XG4iLAo+ICsJCXBhcmVudF9yYXRlLCBkaXYsIHJhdGUpOwo+ICsKPiAgCXJlZ21h cF91cGRhdGVfYml0cyh1c2ItPnJlZ21hcCwgQVQ5MV9QTUNfVVNCLCBBVDkxX1BNQ19PSENJVVNC RElWLAo+ICAJCQkgICAoZGl2IC0gMSkgPDwgU0FNOVg1X1VTQl9ESVZfU0hJRlQpOwo+ICAKPiBk aWZmIC0tZ2l0IGEvZHJpdmVycy91c2IvaG9zdC9vaGNpLWF0OTEuYyBiL2RyaXZlcnMvdXNiL2hv c3Qvb2hjaS1hdDkxLmMKPiBpbmRleCA1YWQ5ZTliZGM4ZWUuLmM1N2EyMzk5MThmOSAxMDA2NDQK PiAtLS0gYS9kcml2ZXJzL3VzYi9ob3N0L29oY2ktYXQ5MS5jCj4gKysrIGIvZHJpdmVycy91c2Iv aG9zdC9vaGNpLWF0OTEuYwo+IEBAIC03MCwxMSArNzAsMTMgQEAgc3RhdGljIGNvbnN0IHN0cnVj dCBvaGNpX2RyaXZlcl9vdmVycmlkZXMgb2hjaV9hdDkxX2Rydl9vdmVycmlkZXMgX19pbml0Y29u c3QgPQo+ICAKPiAgLyotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tKi8KPiAgCj4gLXN0YXRpYyB2b2lkIGF0OTFf c3RhcnRfY2xvY2soc3RydWN0IG9oY2lfYXQ5MV9wcml2ICpvaGNpX2F0OTEpCj4gK3N0YXRpYyB2 b2lkIGF0OTFfc3RhcnRfY2xvY2soc3RydWN0IG9oY2lfYXQ5MV9wcml2ICpvaGNpX2F0OTEsCj4g KwkJCQlzdHJ1Y3QgZGV2aWNlICpkZXYpCgpUaGlzIGlzIG5vdCBwcm9wZXJseSBhbGlnbmVkIGVp dGhlcgoKPiAgewo+ICAJaWYgKG9oY2lfYXQ5MS0+Y2xvY2tlZCkKPiAgCQlyZXR1cm47Cj4gIAo+ ICsJZGV2X2RiZyhkZXYsICJFbmFibGluZyBoY2xrLCBpY2xrLCBhbmQgc2V0dGluZyBmY2xrIHRv IDQ4IE1oelxuIik7Cj4gIAljbGtfc2V0X3JhdGUob2hjaV9hdDkxLT5mY2xrLCA0ODAwMDAwMCk7 Cj4gIAljbGtfcHJlcGFyZV9lbmFibGUob2hjaV9hdDkxLT5oY2xrKTsKPiAgCWNsa19wcmVwYXJl X2VuYWJsZShvaGNpX2F0OTEtPmljbGspOwo+IEBAIC04MiwxMSArODQsMTMgQEAgc3RhdGljIHZv aWQgYXQ5MV9zdGFydF9jbG9jayhzdHJ1Y3Qgb2hjaV9hdDkxX3ByaXYgKm9oY2lfYXQ5MSkKPiAg CW9oY2lfYXQ5MS0+Y2xvY2tlZCA9IHRydWU7Cj4gIH0KPiAgCj4gLXN0YXRpYyB2b2lkIGF0OTFf c3RvcF9jbG9jayhzdHJ1Y3Qgb2hjaV9hdDkxX3ByaXYgKm9oY2lfYXQ5MSkKPiArc3RhdGljIHZv aWQgYXQ5MV9zdG9wX2Nsb2NrKHN0cnVjdCBvaGNpX2F0OTFfcHJpdiAqb2hjaV9hdDkxLAo+ICsJ CQkJc3RydWN0IGRldmljZSAqZGV2KQo+ICB7Cj4gIAlpZiAoIW9oY2lfYXQ5MS0+Y2xvY2tlZCkK PiAgCQlyZXR1cm47Cj4gIAo+ICsJZGV2X2RiZyhkZXYsICJEaXNhYmxpbmcgaGNsaywgaWNsaywg YW5kIGZjbGtcbiIpOwo+ICAJY2xrX2Rpc2FibGVfdW5wcmVwYXJlKG9oY2lfYXQ5MS0+ZmNsayk7 Cj4gIAljbGtfZGlzYWJsZV91bnByZXBhcmUob2hjaV9hdDkxLT5pY2xrKTsKPiAgCWNsa19kaXNh YmxlX3VucHJlcGFyZShvaGNpX2F0OTEtPmhjbGspOwo+IEBAIC0xMDQsNyArMTA4LDcgQEAgc3Rh dGljIHZvaWQgYXQ5MV9zdGFydF9oYyhzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2KQo+ICAJ LyoKPiAgCSAqIFN0YXJ0IHRoZSBVU0IgY2xvY2tzLgo+ICAJICovCj4gLQlhdDkxX3N0YXJ0X2Ns b2NrKG9oY2lfYXQ5MSk7Cj4gKwlhdDkxX3N0YXJ0X2Nsb2NrKG9oY2lfYXQ5MSwgJnBkZXYtPmRl dik7Cj4gIAo+ICAJLyoKPiAgCSAqIFRoZSBVU0IgaG9zdCBjb250cm9sbGVyIG11c3QgcmVtYWlu IGluIHJlc2V0Lgo+IEBAIC0xMjgsNyArMTMyLDcgQEAgc3RhdGljIHZvaWQgYXQ5MV9zdG9wX2hj KHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpCj4gIAkvKgo+ICAJICogU3RvcCB0aGUgVVNC IGNsb2Nrcy4KPiAgCSAqLwo+IC0JYXQ5MV9zdG9wX2Nsb2NrKG9oY2lfYXQ5MSk7Cj4gKwlhdDkx X3N0b3BfY2xvY2sob2hjaV9hdDkxLCAmcGRldi0+ZGV2KTsKPiAgfQo+ICAKPiAgCj4gQEAgLTYy Myw3ICs2MjcsNyBAQCBvaGNpX2hjZF9hdDkxX2Rydl9zdXNwZW5kKHN0cnVjdCBkZXZpY2UgKmRl dikKPiAgCj4gIAkJLyogZmx1c2ggdGhlIHdyaXRlcyAqLwo+ICAJCSh2b2lkKSBvaGNpX3JlYWRs IChvaGNpLCAmb2hjaS0+cmVncy0+Y29udHJvbCk7Cj4gLQkJYXQ5MV9zdG9wX2Nsb2NrKG9oY2lf YXQ5MSk7Cj4gKwkJYXQ5MV9zdG9wX2Nsb2NrKG9oY2lfYXQ5MSwgZGV2KTsKPiAgCX0KPiAgCj4g IAlyZXR1cm4gcmV0Owo+IEBAIC02MzgsNyArNjQyLDcgQEAgb2hjaV9oY2RfYXQ5MV9kcnZfcmVz dW1lKHN0cnVjdCBkZXZpY2UgKmRldikKPiAgCWlmIChvaGNpX2F0OTEtPndha2V1cCkKPiAgCQlk aXNhYmxlX2lycV93YWtlKGhjZC0+aXJxKTsKPiAgCj4gLQlhdDkxX3N0YXJ0X2Nsb2NrKG9oY2lf YXQ5MSk7Cj4gKwlhdDkxX3N0YXJ0X2Nsb2NrKG9oY2lfYXQ5MSwgZGV2KTsKPiAgCj4gIAlvaGNp X3Jlc3VtZShoY2QsIGZhbHNlKTsKPiAgCj4gLS0gCj4gMi4xNy4wCj4gCj4K From mboxrd@z Thu Jan 1 00:00:00 1970 From: alexandre.belloni@bootlin.com (Alexandre Belloni) Date: Tue, 10 Apr 2018 11:54:39 +0200 Subject: [PATCH v2 1/2] clk: at91: Added more information logging. In-Reply-To: <20180410001621.GA62230@hak8or> References: <20180410001621.GA62230@hak8or> Message-ID: <20180410095439.GA2745@piout.net> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On 09/04/2018 20:16:21-0400, Marcin Ziemianowicz wrote: > I noticed that when debugging some USB clocking issue that there weren't > many ways to tell what the state of the USB clocking system was. This > adds a few logging statements to see what the relevant code is trying to > do. > > Signed-off-by: Marcin Ziemianowicz > --- > drivers/clk/at91/clk-pll.c | 6 +++++- > drivers/clk/at91/clk-usb.c | 10 ++++++++-- > drivers/usb/host/ohci-at91.c | 16 ++++++++++------ > 3 files changed, 23 insertions(+), 9 deletions(-) > > diff --git a/drivers/clk/at91/clk-pll.c b/drivers/clk/at91/clk-pll.c > index 7d3223fc7161..534961766ae5 100644 > --- a/drivers/clk/at91/clk-pll.c > +++ b/drivers/clk/at91/clk-pll.c > @@ -133,6 +133,7 @@ static unsigned long clk_pll_recalc_rate(struct clk_hw *hw, > { > struct clk_pll *pll = to_clk_pll(hw); > unsigned int pllr; > + unsigned long recalcedrate; > u16 mul; > u8 div; > > @@ -144,7 +145,10 @@ static unsigned long clk_pll_recalc_rate(struct clk_hw *hw, > if (!div || !mul) > return 0; > > - return (parent_rate / div) * (mul + 1); > + recalcedrate = (parent_rate / div) * (mul + 1); > + pr_debug("clk-pll: calculating new rate, (%lu hz / %u) * %u = %lu hz\n", > + parent_rate, div, mul, recalcedrate); This is not properly aligned. Also, I'm not a huge fan of adding that kind of debug messages. Isn't the rate already present in debugfs ? > + return recalcedrate; > } > > static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate, > diff --git a/drivers/clk/at91/clk-usb.c b/drivers/clk/at91/clk-usb.c > index 791770a563fc..2fa877e99bac 100644 > --- a/drivers/clk/at91/clk-usb.c > +++ b/drivers/clk/at91/clk-usb.c > @@ -48,11 +48,15 @@ static unsigned long at91sam9x5_clk_usb_recalc_rate(struct clk_hw *hw, > struct at91sam9x5_clk_usb *usb = to_at91sam9x5_clk_usb(hw); > unsigned int usbr; > u8 usbdiv; > + unsigned int calcdclock; > > regmap_read(usb->regmap, AT91_PMC_USB, &usbr); > usbdiv = (usbr & AT91_PMC_OHCIUSBDIV) >> SAM9X5_USB_DIV_SHIFT; > > - return DIV_ROUND_CLOSEST(parent_rate, (usbdiv + 1)); > + calcdclock = DIV_ROUND_CLOSEST(parent_rate, (usbdiv + 1)); > + pr_debug("clk-usb: calculating new rate, %lu hz / %u = %u hz\n", > + parent_rate, usbdiv + 1, calcdclock); > + return calcdclock; > } > > static int at91sam9x5_clk_usb_determine_rate(struct clk_hw *hw, > @@ -98,7 +102,6 @@ static int at91sam9x5_clk_usb_determine_rate(struct clk_hw *hw, > if (!best_diff) > break; > } > - Unrelated change > if (best_rate < 0) > return best_rate; > > @@ -142,6 +145,9 @@ static int at91sam9x5_clk_usb_set_rate(struct clk_hw *hw, unsigned long rate, > if (div > SAM9X5_USB_MAX_DIV + 1 || !div) > return -EINVAL; > > + pr_debug("clk-usb: setting USB clock divider to %lu hz / %lu = %lu hz\n", > + parent_rate, div, rate); > + > regmap_update_bits(usb->regmap, AT91_PMC_USB, AT91_PMC_OHCIUSBDIV, > (div - 1) << SAM9X5_USB_DIV_SHIFT); > > diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c > index 5ad9e9bdc8ee..c57a239918f9 100644 > --- a/drivers/usb/host/ohci-at91.c > +++ b/drivers/usb/host/ohci-at91.c > @@ -70,11 +70,13 @@ static const struct ohci_driver_overrides ohci_at91_drv_overrides __initconst = > > /*-------------------------------------------------------------------------*/ > > -static void at91_start_clock(struct ohci_at91_priv *ohci_at91) > +static void at91_start_clock(struct ohci_at91_priv *ohci_at91, > + struct device *dev) This is not properly aligned either > { > if (ohci_at91->clocked) > return; > > + dev_dbg(dev, "Enabling hclk, iclk, and setting fclk to 48 Mhz\n"); > clk_set_rate(ohci_at91->fclk, 48000000); > clk_prepare_enable(ohci_at91->hclk); > clk_prepare_enable(ohci_at91->iclk); > @@ -82,11 +84,13 @@ static void at91_start_clock(struct ohci_at91_priv *ohci_at91) > ohci_at91->clocked = true; > } > > -static void at91_stop_clock(struct ohci_at91_priv *ohci_at91) > +static void at91_stop_clock(struct ohci_at91_priv *ohci_at91, > + struct device *dev) > { > if (!ohci_at91->clocked) > return; > > + dev_dbg(dev, "Disabling hclk, iclk, and fclk\n"); > clk_disable_unprepare(ohci_at91->fclk); > clk_disable_unprepare(ohci_at91->iclk); > clk_disable_unprepare(ohci_at91->hclk); > @@ -104,7 +108,7 @@ static void at91_start_hc(struct platform_device *pdev) > /* > * Start the USB clocks. > */ > - at91_start_clock(ohci_at91); > + at91_start_clock(ohci_at91, &pdev->dev); > > /* > * The USB host controller must remain in reset. > @@ -128,7 +132,7 @@ static void at91_stop_hc(struct platform_device *pdev) > /* > * Stop the USB clocks. > */ > - at91_stop_clock(ohci_at91); > + at91_stop_clock(ohci_at91, &pdev->dev); > } > > > @@ -623,7 +627,7 @@ ohci_hcd_at91_drv_suspend(struct device *dev) > > /* flush the writes */ > (void) ohci_readl (ohci, &ohci->regs->control); > - at91_stop_clock(ohci_at91); > + at91_stop_clock(ohci_at91, dev); > } > > return ret; > @@ -638,7 +642,7 @@ ohci_hcd_at91_drv_resume(struct device *dev) > if (ohci_at91->wakeup) > disable_irq_wake(hcd->irq); > > - at91_start_clock(ohci_at91); > + at91_start_clock(ohci_at91, dev); > > ohci_resume(hcd, false); > > -- > 2.17.0 > > -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com