diff mbox

[v2,3/6] clk: ti: dra7: Add clkctrl clock data for the mcan clocks

Message ID 20180530141133.3711-4-faiz_abbas@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Faiz Abbas May 30, 2018, 2:11 p.m. UTC
Add clkctrl data for the m_can clocks and register it within the
clkctrl driver

CC: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 drivers/clk/ti/clk-7xx.c         | 1 +
 include/dt-bindings/clock/dra7.h | 1 +
 2 files changed, 2 insertions(+)

Comments

Rob Herring (Arm) May 31, 2018, 4:03 a.m. UTC | #1
On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
> Add clkctrl data for the m_can clocks and register it within the
> clkctrl driver
> 
> CC: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
>  drivers/clk/ti/clk-7xx.c         | 1 +
>  include/dt-bindings/clock/dra7.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
> index fb249a1637a5..71a122b2dc67 100644
> --- a/drivers/clk/ti/clk-7xx.c
> +++ b/drivers/clk/ti/clk-7xx.c
> @@ -708,6 +708,7 @@ static const struct omap_clkctrl_reg_data dra7_wkupaon_clkctrl_regs[] __initcons
>  	{ DRA7_COUNTER_32K_CLKCTRL, NULL, 0, "wkupaon_iclk_mux" },
>  	{ DRA7_UART10_CLKCTRL, dra7_uart10_bit_data, CLKF_SW_SUP, "wkupaon_cm:clk:0060:24" },
>  	{ DRA7_DCAN1_CLKCTRL, dra7_dcan1_bit_data, CLKF_SW_SUP, "wkupaon_cm:clk:0068:24" },
> +	{ DRA7_ADC_CLKCTRL, NULL, CLKF_SW_SUP, "mcan_clk" },
>  	{ 0 },
>  };
>  
> diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
> index 5e1061b15aed..d7549c57cac3 100644
> --- a/include/dt-bindings/clock/dra7.h
> +++ b/include/dt-bindings/clock/dra7.h
> @@ -168,5 +168,6 @@
>  #define DRA7_COUNTER_32K_CLKCTRL	DRA7_CLKCTRL_INDEX(0x50)
>  #define DRA7_UART10_CLKCTRL	DRA7_CLKCTRL_INDEX(0x80)
>  #define DRA7_DCAN1_CLKCTRL	DRA7_CLKCTRL_INDEX(0x88)
> +#define DRA7_ADC_CLKCTRL	DRA7_CLKCTRL_INDEX(0xa0)

ADC and mcan are the same thing?

>  
>  #endif
> -- 
> 2.17.0
>
Faiz Abbas May 31, 2018, 10:14 a.m. UTC | #2
Hi,

On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
>> Add clkctrl data for the m_can clocks and register it within the
...
>>  
>> diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
>> index 5e1061b15aed..d7549c57cac3 100644
>> --- a/include/dt-bindings/clock/dra7.h
>> +++ b/include/dt-bindings/clock/dra7.h
>> @@ -168,5 +168,6 @@
>>  #define DRA7_COUNTER_32K_CLKCTRL	DRA7_CLKCTRL_INDEX(0x50)
>>  #define DRA7_UART10_CLKCTRL	DRA7_CLKCTRL_INDEX(0x80)
>>  #define DRA7_DCAN1_CLKCTRL	DRA7_CLKCTRL_INDEX(0x88)
>> +#define DRA7_ADC_CLKCTRL	DRA7_CLKCTRL_INDEX(0xa0)
> 
> ADC and mcan are the same thing?
> 

The register to control MCAN clocks is called ADC_CLKCTRL, Yes.

Thanks,
Faiz
Tero Kristo May 31, 2018, 1:29 p.m. UTC | #3
On 31/05/18 13:14, Faiz Abbas wrote:
> Hi,
> 
> On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
>> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
>>> Add clkctrl data for the m_can clocks and register it within the
> ...
>>>   
>>> diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
>>> index 5e1061b15aed..d7549c57cac3 100644
>>> --- a/include/dt-bindings/clock/dra7.h
>>> +++ b/include/dt-bindings/clock/dra7.h
>>> @@ -168,5 +168,6 @@
>>>   #define DRA7_COUNTER_32K_CLKCTRL	DRA7_CLKCTRL_INDEX(0x50)
>>>   #define DRA7_UART10_CLKCTRL	DRA7_CLKCTRL_INDEX(0x80)
>>>   #define DRA7_DCAN1_CLKCTRL	DRA7_CLKCTRL_INDEX(0x88)
>>> +#define DRA7_ADC_CLKCTRL	DRA7_CLKCTRL_INDEX(0xa0)
>>
>> ADC and mcan are the same thing?
>>
> 
> The register to control MCAN clocks is called ADC_CLKCTRL, Yes.

Is there any reason for this or is that just a documentation bug?

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Faiz Abbas June 1, 2018, 6:48 a.m. UTC | #4
Hi,

On Thursday 31 May 2018 06:59 PM, Tero Kristo wrote:
> On 31/05/18 13:14, Faiz Abbas wrote:
>> Hi,
>>
>> On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
>>> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
>>>> Add clkctrl data for the m_can clocks and register it within the
>> ...
>>>>   diff --git a/include/dt-bindings/clock/dra7.h
>>>> b/include/dt-bindings/clock/dra7.h
>>>> index 5e1061b15aed..d7549c57cac3 100644
>>>> --- a/include/dt-bindings/clock/dra7.h
>>>> +++ b/include/dt-bindings/clock/dra7.h
>>>> @@ -168,5 +168,6 @@
>>>>   #define DRA7_COUNTER_32K_CLKCTRL    DRA7_CLKCTRL_INDEX(0x50)
>>>>   #define DRA7_UART10_CLKCTRL    DRA7_CLKCTRL_INDEX(0x80)
>>>>   #define DRA7_DCAN1_CLKCTRL    DRA7_CLKCTRL_INDEX(0x88)
>>>> +#define DRA7_ADC_CLKCTRL    DRA7_CLKCTRL_INDEX(0xa0)
>>>
>>> ADC and mcan are the same thing?
>>>
>>
>> The register to control MCAN clocks is called ADC_CLKCTRL, Yes.
> 
> Is there any reason for this or is that just a documentation bug?
> 

Looks like they meant to have an ADC in dra74 or dra72 but decided
against it and then many years later used the same registers for MCAN
instead. You can see ADC_CLKCTRL exists in dra72 TRM but is explicitly
disabled.

http://www.ti.com/lit/ug/spruic2b/spruic2b.pdf pg:1524


Thanks,
Faiz
Tony Lindgren June 1, 2018, 2:26 p.m. UTC | #5
* Faiz Abbas <faiz_abbas@ti.com> [180601 06:49]:
> Hi,
> 
> On Thursday 31 May 2018 06:59 PM, Tero Kristo wrote:
> > On 31/05/18 13:14, Faiz Abbas wrote:
> >> Hi,
> >>
> >> On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
> >>> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
> >>>> Add clkctrl data for the m_can clocks and register it within the
> >> ...
> >>>>   diff --git a/include/dt-bindings/clock/dra7.h
> >>>> b/include/dt-bindings/clock/dra7.h
> >>>> index 5e1061b15aed..d7549c57cac3 100644
> >>>> --- a/include/dt-bindings/clock/dra7.h
> >>>> +++ b/include/dt-bindings/clock/dra7.h
> >>>> @@ -168,5 +168,6 @@
> >>>>   #define DRA7_COUNTER_32K_CLKCTRL    DRA7_CLKCTRL_INDEX(0x50)
> >>>>   #define DRA7_UART10_CLKCTRL    DRA7_CLKCTRL_INDEX(0x80)
> >>>>   #define DRA7_DCAN1_CLKCTRL    DRA7_CLKCTRL_INDEX(0x88)
> >>>> +#define DRA7_ADC_CLKCTRL    DRA7_CLKCTRL_INDEX(0xa0)
> >>>
> >>> ADC and mcan are the same thing?
> >>>
> >>
> >> The register to control MCAN clocks is called ADC_CLKCTRL, Yes.
> > 
> > Is there any reason for this or is that just a documentation bug?
> > 
> 
> Looks like they meant to have an ADC in dra74 or dra72 but decided
> against it and then many years later used the same registers for MCAN
> instead. You can see ADC_CLKCTRL exists in dra72 TRM but is explicitly
> disabled.
> 
> http://www.ti.com/lit/ug/spruic2b/spruic2b.pdf pg:1524

How about make add also something like to dra7.h:

#define DRA7_MCAN_CLKCTRL DRA7_ADC_CLKCTRL

And you can add a comment to the dts file to avoid people
getting confused with this constantly.

Regards,

Tony
Faiz Abbas June 4, 2018, 1:58 p.m. UTC | #6
Hi,

On Friday 01 June 2018 07:56 PM, Tony Lindgren wrote:
> * Faiz Abbas <faiz_abbas@ti.com> [180601 06:49]:
>> Hi,
>>
>> On Thursday 31 May 2018 06:59 PM, Tero Kristo wrote:
>>> On 31/05/18 13:14, Faiz Abbas wrote:
>>>> Hi,
>>>>
>>>> On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
>>>>> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
>>>>>> Add clkctrl data for the m_can clocks and register it within the
>>>> ...
>>>>>>   diff --git a/include/dt-bindings/clock/dra7.h
>>>>>> b/include/dt-bindings/clock/dra7.h
>>>>>> index 5e1061b15aed..d7549c57cac3 100644
>>>>>> --- a/include/dt-bindings/clock/dra7.h
>>>>>> +++ b/include/dt-bindings/clock/dra7.h
>>>>>> @@ -168,5 +168,6 @@
>>>>>>   #define DRA7_COUNTER_32K_CLKCTRL    DRA7_CLKCTRL_INDEX(0x50)
>>>>>>   #define DRA7_UART10_CLKCTRL    DRA7_CLKCTRL_INDEX(0x80)
>>>>>>   #define DRA7_DCAN1_CLKCTRL    DRA7_CLKCTRL_INDEX(0x88)
>>>>>> +#define DRA7_ADC_CLKCTRL    DRA7_CLKCTRL_INDEX(0xa0)
>>>>>
>>>>> ADC and mcan are the same thing?
>>>>>
>>>>
>>>> The register to control MCAN clocks is called ADC_CLKCTRL, Yes.
>>>
>>> Is there any reason for this or is that just a documentation bug?
>>>
>>
>> Looks like they meant to have an ADC in dra74 or dra72 but decided
>> against it and then many years later used the same registers for MCAN
>> instead. You can see ADC_CLKCTRL exists in dra72 TRM but is explicitly
>> disabled.
>>
>> http://www.ti.com/lit/ug/spruic2b/spruic2b.pdf pg:1524
> 
> How about make add also something like to dra7.h:
> 
> #define DRA7_MCAN_CLKCTRL DRA7_ADC_CLKCTRL
> 
> And you can add a comment to the dts file to avoid people
> getting confused with this constantly.
> 

I would prefer to follow the TRM so that people don't look for registers
that don't exist at all.

Thanks,
Faiz
diff mbox

Patch

diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
index fb249a1637a5..71a122b2dc67 100644
--- a/drivers/clk/ti/clk-7xx.c
+++ b/drivers/clk/ti/clk-7xx.c
@@ -708,6 +708,7 @@  static const struct omap_clkctrl_reg_data dra7_wkupaon_clkctrl_regs[] __initcons
 	{ DRA7_COUNTER_32K_CLKCTRL, NULL, 0, "wkupaon_iclk_mux" },
 	{ DRA7_UART10_CLKCTRL, dra7_uart10_bit_data, CLKF_SW_SUP, "wkupaon_cm:clk:0060:24" },
 	{ DRA7_DCAN1_CLKCTRL, dra7_dcan1_bit_data, CLKF_SW_SUP, "wkupaon_cm:clk:0068:24" },
+	{ DRA7_ADC_CLKCTRL, NULL, CLKF_SW_SUP, "mcan_clk" },
 	{ 0 },
 };
 
diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
index 5e1061b15aed..d7549c57cac3 100644
--- a/include/dt-bindings/clock/dra7.h
+++ b/include/dt-bindings/clock/dra7.h
@@ -168,5 +168,6 @@ 
 #define DRA7_COUNTER_32K_CLKCTRL	DRA7_CLKCTRL_INDEX(0x50)
 #define DRA7_UART10_CLKCTRL	DRA7_CLKCTRL_INDEX(0x80)
 #define DRA7_DCAN1_CLKCTRL	DRA7_CLKCTRL_INDEX(0x88)
+#define DRA7_ADC_CLKCTRL	DRA7_CLKCTRL_INDEX(0xa0)
 
 #endif