Hi Humberto,
Somehow the original message didn't arrive on linux-samsung-soc...
Am 31.07.2014 06:51, schrieb Yadwinder Singh Brar:
>> ------- Original Message -------
>> Sender : Humberto Naves<hsnaves@xxxxxxxxx>
>> Date : Jul 31, 2014 00:16 (GMT+09:00)
>> Title : Role of PLL_ENABLE_BIT
>>
>> Hi,
>>
>>
>> I am using an ODROID-XU board, and I was trying to change the rates of some PLL clocks for a while, but I could not. After a while, I realized that the cpu was trapped in an infinite loop waiting for the PLL35XX_LOCK_STAT bit to be set. Furthermore, I discovered that the reason was that the the PLL35XX_ENABLE_BIT was not set.
>>
>>
>> I wonder if this is really a driver problem, since in the set_rate functions, the ENABLE_BIT is not set. Can someone confirm if this is indeed the expected behavior of the driver?
>>
>
> hmm .. actually it doesn't clear also ENABLE_BIT, I wonder how its getting clear
> in your case since we don't have enable/disable() implemented yet.
>
>>
>>
>> By the way, In order to solve this issue, I changed the files according to the following diff
>>
>>
>> diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
>> index b07fad2..9300274 100644
>> --- a/drivers/clk/samsung/clk-pll.c
>> +++ b/drivers/clk/samsung/clk-pll.c
>> @@ -131,14 +131,15 @@ static const struct clk_ops samsung_pll3000_clk_ops = {
>> /* Maximum lock time can be 270 * PDIV cycles */
>> #define PLL35XX_LOCK_FACTOR (270)
>>
>> -#define PLL35XX_MDIV_MASK (0x3FF)
>> -#define PLL35XX_PDIV_MASK (0x3F)
>> -#define PLL35XX_SDIV_MASK (0x7)
>> +#define PLL35XX_MDIV_MASK (0x3FF)
>> +#define PLL35XX_PDIV_MASK (0x3F)
>> +#define PLL35XX_SDIV_MASK (0x7)
>> #define PLL35XX_LOCK_STAT_MASK (0x1)
>> -#define PLL35XX_MDIV_SHIFT (16)
>> -#define PLL35XX_PDIV_SHIFT (8)
>> -#define PLL35XX_SDIV_SHIFT (0)
>> +#define PLL35XX_MDIV_SHIFT (16)
>> +#define PLL35XX_PDIV_SHIFT (8)
>> +#define PLL35XX_SDIV_SHIFT (0)
>> #define PLL35XX_LOCK_STAT_SHIFT (29)
>> +#define PLL35XX_ENABLE_SHIFT (31)
>>
>> static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,
>> unsigned long parent_rate)
>> @@ -190,6 +191,7 @@ static int samsung_pll35xx_set_rate(struct clk_hw *hw, unsigned long drate,
>> /* If only s change, change just s value only*/
>> tmp &= ~(PLL35XX_SDIV_MASK << PLL35XX_SDIV_SHIFT);
>> tmp |= rate->sdiv << PLL35XX_SDIV_SHIFT;
>> + tmp |= (1 << PLL35XX_ENABLE_SHIFT);
>> __raw_writel(tmp, pll->con_reg);
>>
>> return 0;
>> @@ -206,6 +208,7 @@ static int samsung_pll35xx_set_rate(struct clk_hw *hw, unsigned long drate,
>> tmp |= (rate->mdiv << PLL35XX_MDIV_SHIFT) |
>> (rate->pdiv << PLL35XX_PDIV_SHIFT) |
>> (rate->sdiv << PLL35XX_SDIV_SHIFT);
>> + tmp |= (1 << PLL35XX_ENABLE_SHIFT);
It appears tmp is a u32. Does that make 1 << 31 a safe expression wrt
signedness, or does it need to be 1u << PLL35XX_ENABLE_SHIFT?
Did you verify the result?
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
