[PATCH v2 2/2] clk: at91: Fix for PLL set_rate changes not being actually written to PLL peripheral bits

[Marcin Ziemianowicz]

When a USB device is connected to the USB host port on the SAM9N12 then
you get "-62" error which seems to indicate USB replies from the device
are timing out. Looking around, I saw the USB bus was running at half
speed. Going further, it seems that in ..._set_rate() the PLL wasn't
actually being adjusted. Writing the multiplier and divider values to
the peripheral fixes the bus running at half speed.

Signed-off-by: Marcin Ziemianowicz 
---
 drivers/clk/at91/clk-pll.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/clk/at91/clk-pll.c b/drivers/clk/at91/clk-pll.c
index 534961766ae5..db7155fe9346 100644
--- a/drivers/clk/at91/clk-pll.c
+++ b/drivers/clk/at91/clk-pll.c
@@ -288,6 +288,14 @@ static int clk_pll_set_rate(struct clk_hw *hw, unsigned 
long rate,
pll->div = div;
pll->mul = mul;
 
+   // Set the PLL as per above div and mil values.
+   regmap_update_bits(pll->regmap, AT91_CKGR_PLLBR,
+   AT91_PMC_DIV | AT91_PMC_MUL,
+   (div << 0) | (mul << 16));
+
+   pr_debug("clk-pll: setting new rate, (%lu hz / %u) * %u = %lu hz\n",
+   parent_rate, div, mul, rate);
+
return 0;
 }
 
-- 
2.17.0


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] clk: at91: Added more information logging.

[Marcin Ziemianowicz]

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);
+   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;
}
-
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)
 {
if (ohci_at91->clocked)
return;
 
+   dev_dbg(dev, "Enabling hclk, iclk, and setting fclk to 48 Mhz\n");
clk_set_rate(ohci_at91->fclk, 4800);
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_su

[PATCH v2 0/2] clk: at91: Added more information logging

[Marcin Ziemianowicz]

This is a series of patches which resolves set_rate() for the PLL not
having any effect and therefore the USB Host port not working. Also, a
few messages were added which may be helpful in the future when others
are working with USB clocking.

Changes since V1:
  Added patch set cover letter
  Shortened lines which were over >80 characters long
  > Comment by Greg Kroah-Hartman about "from" field in email addressed
  > Comment by Alan Stern about redundant debug lines addressed

hak8or (2):
  clk: at91: Added more information logging.
  clk: at91: Fix for PLL set_rate changes not being actually written to
PLL peripheral bits

 drivers/clk/at91/clk-pll.c   | 14 +-
 drivers/clk/at91/clk-usb.c   | 10 --
 drivers/usb/host/ohci-at91.c | 16 ++--
 3 files changed, 31 insertions(+), 9 deletions(-)

-- 
2.17.0


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] clk: at91: Added more information logging.

[Marcin Ziemianowicz]

On Sun, Apr 08, 2018 at 11:54:49AM +0200, Greg Kroah-Hartman wrote:
> On Sun, Apr 08, 2018 at 05:43:30AM -0400, Marcin 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 
> 
> Your "From:" line doesn't match this name :(

Ah drat, I knew I did something wrong. You suggested in IRC that I wait a day
for others to reply, so after that I will attempt to version my patchset.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html