[PATCH v2 1/6] rtc: omap: kicker mechanism support

[PATCH v2 1/6] rtc: omap: kicker mechanism support

[Sergei Shtylyov]

Hello.

On 25-07-2012 10:12, Afzal Mohammed wrote:

> OMAP RTC IP can have kicker feature. This prevents spurious
> writes to register. To write to registers kicker lock has to
> be released. Procedure to do it as follows,

> 1. write to kick0 register, 0x83e70b13
> 2. write to kick1 register, 0x95a4f1e0

> Writing value other than 0x83e70b13 to kick0 enables write
> locking, more details about kicker mechanism can be found in
> section 20.3.3.5.3 of AM335X TRM @www.ti.com/am335x

> Here id table information is added and is used to distinguish
> those that require kicker handling and the ones that doesn't
> need it. There are more features in the newer IP's compared
> to legacy ones other than kicker, which driver currently
> doesn't handle, supporting additional features would be
> easier with the addition of id table.

> Older IP (of OMAP1) doesn't have revision register as per
> TRM, so revision register can't be relied always to find
> features, hence id table is being used.

> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> ---

> v2:
>   Use device name da830-rtc instead of am1808-rtc
>   Newly added register name made similar to that existing in the driver
>   Better commit message description

>   drivers/rtc/rtc-omap.c |   39 ++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 38 insertions(+), 1 deletions(-)

> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index 0b614e3..8afbc2e 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -38,6 +38,8 @@
>    * the SoC). See the BOARD-SPECIFIC CUSTOMIZATION comment.
>    */
>
> +#define	DRIVER_NAME			"omap_rtc"
> +
>   #define OMAP_RTC_BASE			0xfffb4800
>
>   /* RTC registers */
> @@ -64,6 +66,9 @@
>   #define OMAP_RTC_COMP_MSB_REG		0x50
>   #define OMAP_RTC_OSC_REG		0x54
>
> +#define OMAP_RTC_KICK0_REG		0x6c
> +#define OMAP_RTC_KICK1_REG		0x70
> +
>   /* OMAP_RTC_CTRL_REG bit fields: */
>   #define OMAP_RTC_CTRL_SPLIT		(1<<7)
>   #define OMAP_RTC_CTRL_DISABLE		(1<<6)
> @@ -88,11 +93,19 @@
>   #define OMAP_RTC_INTERRUPTS_IT_ALARM    (1<<3)
>   #define OMAP_RTC_INTERRUPTS_IT_TIMER    (1<<2)
>
> +/* OMAP_RTC_KICKER values */
> +#define	KICK0_VALUE			(0x83e70b13)
> +#define	KICK1_VALUE			(0x95a4f1e0)

    Parens not needed around simple literals.

>   static void __iomem	*rtc_base;
>
>   #define rtc_read(addr)		__raw_readb(rtc_base + (addr))
>   #define rtc_write(val, addr)	__raw_writeb(val, rtc_base + (addr))
>
> +#define rtc_writel(val, addr)	writel(val, rtc_base + (addr))
> +

    Why not __raw_writel() like the above functions?

WBR, Sergei

[PATCH v2 1/6] rtc: omap: kicker mechanism support

[Mohammed, Afzal]

Hi Sergei,

On Wed, Jul 25, 2012 at 16:45:29, Sergei Shtylyov wrote:

> > +/* OMAP_RTC_KICKER values */
> > +#define	KICK0_VALUE			(0x83e70b13)
> > +#define	KICK1_VALUE			(0x95a4f1e0)
> 
>     Parens not needed around simple literals.

Thanks for catching it

> 
> >   static void __iomem	*rtc_base;
> >
> >   #define rtc_read(addr)		__raw_readb(rtc_base + (addr))
> >   #define rtc_write(val, addr)	__raw_writeb(val, rtc_base + (addr))
> >
> > +#define rtc_writel(val, addr)	writel(val, rtc_base + (addr))
> > +
> 
>     Why not __raw_writel() like the above functions?

This driver would be used in AM335X, it being ARMv7, writel would be
safe (existing __raw_readb/__raw_writeb too needs to be replaced)

Regards
Afzal

[PATCH v2 6/6] arm/dts: am33xx rtc node

[Sergei Shtylyov]

Hello.

On 25-07-2012 10:12, Afzal Mohammed wrote:

> Add AM33xx rtc node.

> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> ---

> v2:
>   Use compatible as ti,da830-rtc instead of ti,am1808-rtc

>   arch/arm/boot/dts/am33xx.dtsi |    5 +++++
>   1 files changed, 5 insertions(+), 0 deletions(-)

> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index bd0cff3..e1ed72d 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -159,5 +159,10 @@
>   			compatible = "ti,omap3-wdt";
>   			ti,hwmods = "wd_timer2";
>   		};
> +
> +		rtc at 44e3e000 {

    Address postfix in the node name without "reg" property?

> +			compatible = "ti,da830-rtc";
> +			ti,hwmods = "rtc";
> +		};

WBR, Sergei

[PATCH v2 6/6] arm/dts: am33xx rtc node

[Mohammed, Afzal]

Hi Sergei,

On Wed, Jul 25, 2012 at 16:50:56, Sergei Shtylyov wrote:

> > +		rtc at 44e3e000 {
> 
>     Address postfix in the node name without "reg" property?

As per [1], "The unit-address is included if the node describes
a device with an address".

Here even though reg property is not present, as via hwmod
(see below) it is getting address, isn't it better to have it

> 
> > +			compatible = "ti,da830-rtc";
> > +			ti,hwmods = "rtc";

Regards
Afzal


[1] http://devicetree.org/Device_Tree_Usage

[PATCH v2 6/6] arm/dts: am33xx rtc node

[Sergei Shtylyov]

Hello.

On 07/25/2012 06:09 PM, Mohammed, Afzal wrote:

>>> +		rtc at 44e3e000 {

>>     Address postfix in the node name without "reg" property?

> As per [1], "The unit-address is included if the node describes
> a device with an address".

   Which in this case it doesn't.

> Here even though reg property is not present, as via hwmod
> (see below) it is getting address, isn't it better to have it

   I think not.

>>> +			compatible = "ti,da830-rtc";
>>> +			ti,hwmods = "rtc";

WBR, Sergei

[PATCH v2 6/6] arm/dts: am33xx rtc node

[Mohammed, Afzal]

Hi Sergei,

On Wed, Jul 25, 2012 at 22:29:24, Sergei Shtylyov wrote:

> >>> +		rtc at 44e3e000 {
> 
> >>     Address postfix in the node name without "reg" property?
> 
> > As per [1], "The unit-address is included if the node describes
> > a device with an address".
> 
>    Which in this case it doesn't.
> 
> > Here even though reg property is not present, as via hwmod
> > (see below) it is getting address, isn't it better to have it
> 
>    I think not.

Ok, I will remove it
 
> >>> +			compatible = "ti,da830-rtc";
> >>> +			ti,hwmods = "rtc";

Regards
Afzal