[PATCH 0/4] clock fixes for at91

[PATCH 0/4] clock fixes for at91

[Claudiu Beznea]

Hi,

This series contains some fixes for at91 clocks (usb + sam9x60).

Thank you,
Claudiu Beznea

Claudiu Beznea (4):
  clk: at91: usb: continue if clk_hw_round_rate() returned zero
  clk: at91: sam9x60: fix usb clock parents
  clk: at91: usb: use proper usbs_mask
  clk: at91: usb: introduce num_parents in driver's structure

 drivers/clk/at91/clk-usb.c | 9 +++++++--
 drivers/clk/at91/sam9x60.c | 5 ++---
 2 files changed, 9 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH 1/4] clk: at91: usb: continue if clk_hw_round_rate() return zero

[Claudiu Beznea]

clk_hw_round_rate() may call round rate function of its parents. In case
of SAM9X60 two of USB parrents are PLLA and UPLL. These clocks are
controlled by clk-sam9x60-pll.c driver. The round rate function for this
driver is sam9x60_pll_round_rate() which call in turn
sam9x60_pll_get_best_div_mul(). In case the requested rate is not in the
proper range (rate < characteristics->output[0].min &&
rate > characteristics->output[0].max) the sam9x60_pll_round_rate() will
return a negative number to its caller (called by
clk_core_round_rate_nolock()). clk_hw_round_rate() will return zero in
case a negative number is returned by clk_core_round_rate_nolock(). With
this, the USB clock will continue its rate computation even caller of
clk_hw_round_rate() returned an error. With this, the USB clock on SAM9X60
may not chose the best parent. I detected this after a suspend/resume
cycle on SAM9X60.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/clk/at91/clk-usb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/at91/clk-usb.c b/drivers/clk/at91/clk-usb.c
index 22aede42a336..3c0bd7e51b09 100644
--- a/drivers/clk/at91/clk-usb.c
+++ b/drivers/clk/at91/clk-usb.c
@@ -75,6 +75,9 @@ static int at91sam9x5_clk_usb_determine_rate(struct clk_hw *hw,
 			tmp_parent_rate = req->rate * div;
 			tmp_parent_rate = clk_hw_round_rate(parent,
 							   tmp_parent_rate);
+			if (!tmp_parent_rate)
+				continue;
+
 			tmp_rate = DIV_ROUND_CLOSEST(tmp_parent_rate, div);
 			if (tmp_rate < req->rate)
 				tmp_diff = req->rate - tmp_rate;
-- 
2.7.4

Re: [PATCH 1/4] clk: at91: usb: continue if clk_hw_round_rate() return zero

[Stephen Boyd]

Quoting Claudiu Beznea (2020-01-17 03:36:46)
> clk_hw_round_rate() may call round rate function of its parents. In case
> of SAM9X60 two of USB parrents are PLLA and UPLL. These clocks are
> controlled by clk-sam9x60-pll.c driver. The round rate function for this
> driver is sam9x60_pll_round_rate() which call in turn
> sam9x60_pll_get_best_div_mul(). In case the requested rate is not in the
> proper range (rate < characteristics->output[0].min &&
> rate > characteristics->output[0].max) the sam9x60_pll_round_rate() will
> return a negative number to its caller (called by
> clk_core_round_rate_nolock()). clk_hw_round_rate() will return zero in
> case a negative number is returned by clk_core_round_rate_nolock(). With
> this, the USB clock will continue its rate computation even caller of
> clk_hw_round_rate() returned an error. With this, the USB clock on SAM9X60
> may not chose the best parent. I detected this after a suspend/resume
> cycle on SAM9X60.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---

Applied to clk-next

[PATCH 2/4] clk: at91: sam9x60: fix usb clock parents

[Claudiu Beznea]

SAM9X60's USB clock has 3 parents: plla, upll and main_osc.

Fixes: 01e2113de9a5 ("clk: at91: add sam9x60 pmc driver")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/clk/at91/sam9x60.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c
index 77398aefeb6d..7338a3bc71eb 100644
--- a/drivers/clk/at91/sam9x60.c
+++ b/drivers/clk/at91/sam9x60.c
@@ -237,9 +237,8 @@ static void __init sam9x60_pmc_setup(struct device_node *np)
 
 	parent_names[0] = "pllack";
 	parent_names[1] = "upllck";
-	parent_names[2] = "mainck";
-	parent_names[3] = "mainck";
-	hw = sam9x60_clk_register_usb(regmap, "usbck", parent_names, 4);
+	parent_names[2] = "main_osc";
+	hw = sam9x60_clk_register_usb(regmap, "usbck", parent_names, 3);
 	if (IS_ERR(hw))
 		goto err_free;
 
-- 
2.7.4

Re: [PATCH 2/4] clk: at91: sam9x60: fix usb clock parents

[Alexandre Belloni]

On 17/01/2020 13:36:47+0200, Claudiu Beznea wrote:
> SAM9X60's USB clock has 3 parents: plla, upll and main_osc.
> 
> Fixes: 01e2113de9a5 ("clk: at91: add sam9x60 pmc driver")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/clk/at91/sam9x60.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c
> index 77398aefeb6d..7338a3bc71eb 100644
> --- a/drivers/clk/at91/sam9x60.c
> +++ b/drivers/clk/at91/sam9x60.c
> @@ -237,9 +237,8 @@ static void __init sam9x60_pmc_setup(struct device_node *np)
>  
>  	parent_names[0] = "pllack";
>  	parent_names[1] = "upllck";
> -	parent_names[2] = "mainck";
> -	parent_names[3] = "mainck";
> -	hw = sam9x60_clk_register_usb(regmap, "usbck", parent_names, 4);
> +	parent_names[2] = "main_osc";
> +	hw = sam9x60_clk_register_usb(regmap, "usbck", parent_names, 3);
>  	if (IS_ERR(hw))
>  		goto err_free;
>  
> -- 
> 2.7.4
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/4] clk: at91: sam9x60: fix usb clock parents

[Stephen Boyd]

Quoting Claudiu Beznea (2020-01-17 03:36:47)
> SAM9X60's USB clock has 3 parents: plla, upll and main_osc.
> 
> Fixes: 01e2113de9a5 ("clk: at91: add sam9x60 pmc driver")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---

Applied to clk-next

[PATCH 3/4] clk: at91: usb: use proper usbs_mask

[Claudiu Beznea]

Use usbs_mask passed as argument. The usbs_mask is different for
SAM9X60.

Fixes: 2423eeaead6f8 ("clk: at91: usb: Add sam9x60 support")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/clk/at91/clk-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/at91/clk-usb.c b/drivers/clk/at91/clk-usb.c
index 3c0bd7e51b09..c0895c993cce 100644
--- a/drivers/clk/at91/clk-usb.c
+++ b/drivers/clk/at91/clk-usb.c
@@ -214,7 +214,7 @@ _at91sam9x5_clk_register_usb(struct regmap *regmap, const char *name,
 
 	usb->hw.init = &init;
 	usb->regmap = regmap;
-	usb->usbs_mask = SAM9X5_USBS_MASK;
+	usb->usbs_mask = usbs_mask;
 
 	hw = &usb->hw;
 	ret = clk_hw_register(NULL, &usb->hw);
-- 
2.7.4

Re: [PATCH 3/4] clk: at91: usb: use proper usbs_mask

[Alexandre Belloni]

On 17/01/2020 13:36:48+0200, Claudiu Beznea wrote:
> Use usbs_mask passed as argument. The usbs_mask is different for
> SAM9X60.
> 
> Fixes: 2423eeaead6f8 ("clk: at91: usb: Add sam9x60 support")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/clk/at91/clk-usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/at91/clk-usb.c b/drivers/clk/at91/clk-usb.c
> index 3c0bd7e51b09..c0895c993cce 100644
> --- a/drivers/clk/at91/clk-usb.c
> +++ b/drivers/clk/at91/clk-usb.c
> @@ -214,7 +214,7 @@ _at91sam9x5_clk_register_usb(struct regmap *regmap, const char *name,
>  
>  	usb->hw.init = &init;
>  	usb->regmap = regmap;
> -	usb->usbs_mask = SAM9X5_USBS_MASK;
> +	usb->usbs_mask = usbs_mask;
>  
>  	hw = &usb->hw;
>  	ret = clk_hw_register(NULL, &usb->hw);
> -- 
> 2.7.4
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 3/4] clk: at91: usb: use proper usbs_mask

[Stephen Boyd]

Quoting Claudiu Beznea (2020-01-17 03:36:48)
> Use usbs_mask passed as argument. The usbs_mask is different for
> SAM9X60.
> 
> Fixes: 2423eeaead6f8 ("clk: at91: usb: Add sam9x60 support")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---

Applied to clk-next

[PATCH 4/4] clk: at91: usb: introduce num_parents in driver's structure

[Claudiu Beznea]

SAM9X60 USB clock may have up to 3 parents. Save the number of parents in
driver's data structure and validate against it when setting parent.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/clk/at91/clk-usb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/at91/clk-usb.c b/drivers/clk/at91/clk-usb.c
index c0895c993cce..31d5c45e30d7 100644
--- a/drivers/clk/at91/clk-usb.c
+++ b/drivers/clk/at91/clk-usb.c
@@ -25,6 +25,7 @@ struct at91sam9x5_clk_usb {
 	struct clk_hw hw;
 	struct regmap *regmap;
 	u32 usbs_mask;
+	u8 num_parents;
 };
 
 #define to_at91sam9x5_clk_usb(hw) \
@@ -110,7 +111,7 @@ static int at91sam9x5_clk_usb_set_parent(struct clk_hw *hw, u8 index)
 {
 	struct at91sam9x5_clk_usb *usb = to_at91sam9x5_clk_usb(hw);
 
-	if (index > 1)
+	if (index >= usb->num_parents)
 		return -EINVAL;
 
 	regmap_update_bits(usb->regmap, AT91_PMC_USB, usb->usbs_mask, index);
@@ -215,6 +216,7 @@ _at91sam9x5_clk_register_usb(struct regmap *regmap, const char *name,
 	usb->hw.init = &init;
 	usb->regmap = regmap;
 	usb->usbs_mask = usbs_mask;
+	usb->num_parents = num_parents;
 
 	hw = &usb->hw;
 	ret = clk_hw_register(NULL, &usb->hw);
-- 
2.7.4

Re: [PATCH 4/4] clk: at91: usb: introduce num_parents in driver's structure

[Alexandre Belloni]

On 17/01/2020 13:36:49+0200, Claudiu Beznea wrote:
> SAM9X60 USB clock may have up to 3 parents. Save the number of parents in
> driver's data structure and validate against it when setting parent.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/clk/at91/clk-usb.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/at91/clk-usb.c b/drivers/clk/at91/clk-usb.c
> index c0895c993cce..31d5c45e30d7 100644
> --- a/drivers/clk/at91/clk-usb.c
> +++ b/drivers/clk/at91/clk-usb.c
> @@ -25,6 +25,7 @@ struct at91sam9x5_clk_usb {
>  	struct clk_hw hw;
>  	struct regmap *regmap;
>  	u32 usbs_mask;
> +	u8 num_parents;
>  };
>  
>  #define to_at91sam9x5_clk_usb(hw) \
> @@ -110,7 +111,7 @@ static int at91sam9x5_clk_usb_set_parent(struct clk_hw *hw, u8 index)
>  {
>  	struct at91sam9x5_clk_usb *usb = to_at91sam9x5_clk_usb(hw);
>  
> -	if (index > 1)
> +	if (index >= usb->num_parents)
>  		return -EINVAL;
>  
>  	regmap_update_bits(usb->regmap, AT91_PMC_USB, usb->usbs_mask, index);
> @@ -215,6 +216,7 @@ _at91sam9x5_clk_register_usb(struct regmap *regmap, const char *name,
>  	usb->hw.init = &init;
>  	usb->regmap = regmap;
>  	usb->usbs_mask = usbs_mask;
> +	usb->num_parents = num_parents;
>  
>  	hw = &usb->hw;
>  	ret = clk_hw_register(NULL, &usb->hw);
> -- 
> 2.7.4
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 4/4] clk: at91: usb: introduce num_parents in driver's structure

[Stephen Boyd]

Quoting Claudiu Beznea (2020-01-17 03:36:49)
> SAM9X60 USB clock may have up to 3 parents. Save the number of parents in
> driver's data structure and validate against it when setting parent.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---

Applied to clk-next