> -----Original Message-----
> From: [email protected] [mailto:linux-omap-
> [email protected]] On Behalf Of Kevin Hilman
> Sent: Saturday, August 22, 2009 9:59 PM
> To: Roger Quadros
> Cc: Varadarajan, Charu Latha; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [RFC][PATCH 1/3] RTC periodic interrupts enabling and msecure
> init
>
> Roger Quadros <[email protected]> writes:
>
> > ext [email protected] wrote:
> >> Triton2 RTC code changes for fixing periodic interrupt feature in RTC.
> >> rtc-twlcore.c does initialisation of the msecure gpio pin. Board
> >> files indicate msecure gpio line through twl4030 platform
> >> data. twl4030-core.c passes this information to RTC driver.
> >> Board files does msecure gpio mux configuration.
> >>
> >>
> >> Signed-off-by: Charulatha V <[email protected]>
> >
> > Periodic interrupts and msecure are 2 different entities. I think they
> > should be implemented in different patches.
>
> Agreed. The "fix" part of this should be a separated out with a
> detailed description of both the problem and the fix.
>
> Kevin
Okay.
>
> >> ---
> >> drivers/rtc/rtc-twl4030.c | 63
> ++++++++++++++++++++++++++++++++++++++++++++-
> >> 1 files changed, 62 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/rtc/rtc-twl4030.c b/drivers/rtc/rtc-twl4030.c
> >> index 9c8c70c..614adf0 100644
> >> --- a/drivers/rtc/rtc-twl4030.c
> >> +++ b/drivers/rtc/rtc-twl4030.c
> >> @@ -29,7 +29,12 @@
> >> #include <linux/interrupt.h>
> >> #include <linux/i2c/twl4030.h>
> >> +#include <linux/gpio.h>
> >> +/*
> >> + * To find if the value is a power of two
> >> + */
> >> +#define is_power_of_two(x) (!((x) & ((x)-1)))
> >> /*
> >> * RTC block register offsets (use TWL_MODULE_RTC)
> >> @@ -86,6 +91,37 @@
> >> /*--------------------------------------------------------------------
> --*/
> >> /*
> >> + * msecure line initialisation for TWL4030 RTC registers write access
> >> + */
> >> +static int msecure_init(struct twl4030_rtc_platform_data *pdata)
> >> +{
> >> + int ret = 0;
> >> + if (pdata == NULL)
> >> + goto out;
> >> +
> >> + ret = gpio_request(pdata->msecure_gpio, "msecure");
> >> + if (ret < 0) {
> >
> > if (ret) should suffice
> >
> >> + pr_err("twl4030_rtc: can't reserve msecure GPIO:%d !\n"
> >> + "RTC functionality will not be available\n",
> >> + pdata->msecure_gpio);
> >> + goto out;
> >> + }
> >> + /*
> >> + * TWL4030 will be in secure mode if msecure line from OMAP is low.
> >> + * Make msecure line high in order to change the TWL4030 RTC time
> >> + * and calender registers.
> >> + */
> >> + ret = gpio_direction_output(pdata->msecure_gpio, 1);
> >> + if (ret < 0)
> >
> > ditto
> >
> >> + pr_err("twl4030_rtc: can't set msecure GPIO direction:%d !\n"
> >> + "RTC functionality will not be available\n",
> >> + pdata->msecure_gpio);
> >> +
> >> +out:
> >> + return ret;
> >> +}
Agreed.
> >> +
> >> +/*
> >> * Supports 1 byte read from TWL4030 RTC register.
> >> */
> >> static int twl4030_rtc_read_u8(u8 *data, u8 reg)
> >> @@ -128,7 +164,6 @@ static int set_rtc_irq_bit(unsigned char bit)
> >> int ret;
> >> val = rtc_irq_bits | bit;
> >> - val &= ~BIT_RTC_INTERRUPTS_REG_EVERY_M;
> >> ret = twl4030_rtc_write_u8(val, REG_RTC_INTERRUPTS_REG);
> >> if (ret == 0)
> >> rtc_irq_bits = val;
> >> @@ -318,6 +353,25 @@ out:
> >> return ret;
> >> }
> >> +static int twl4030_rtc_irq_set_freq(struct device *dev, int freq)
> >> +{
> >> + int ret, val = 1;
> >> + int regbit = 0;
> >> +
> >> + if ((!is_power_of_2(freq)) || (freq > 8) || (freq <= 0))
> >> + return -EINVAL;
> >
> > 0 is valid freq. it means disable periodic interrupts.
> >
> >> +
> >> + while ((freq & val) == 0) {
> >> + val = val << 1;
> >> + regbit++;
> >> + }
> >
> > as per your implementation, if user sets interrupt rate of 4 Hz then
> > you will set regbit to 2 which means interrupt every hour?
> > i.e. 0.00027 Hz. no?
> >
> >> + ret = set_rtc_irq_bit(regbit);
> >> + if (ret)
> >> + dev_err(dev, "rtc_irq_set_freq error %d\n", ret);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> static irqreturn_t twl4030_rtc_interrupt(int irq, void *rtc)
> >> {
> >> unsigned long events = 0;
> >> @@ -383,6 +437,7 @@ static struct rtc_class_ops twl4030_rtc_ops = {
> >> .set_alarm = twl4030_rtc_set_alarm,
> >> .alarm_irq_enable = twl4030_rtc_alarm_irq_enable,
> >> .update_irq_enable = twl4030_rtc_update_irq_enable,
> >> + .irq_set_freq = twl4030_rtc_irq_set_freq,
> >
> > IMHO this does not make sense.
> > twl4030 supports a max interrupt rate of 1 Hz (i.e. 1 sec). So you can
> > only support freq values of 0 and 1 i.e. 0 for disabled and 1 for 1
> > sec interrupt.
> > This functionality is already achieved by update_irq_enable.
> >
> > -roger
> > --
Agreed that twl4030_rtc_update_irq_enable is enough if the periodic interrupt
is required only for 1 Hz.
If this is enough, then any ioctl call with RTC_IRQP_SET to twl4030 chip's RTC
can do only
1 Hz freq setting. This means that "RTC_IRQP_SET" is not required at all for
twl4030 chip's RTC
and only "RTC_UIE_ON" is enough. Please clarify.
The twl4030 chip also supports interrupt every min, day and hour. Don't we need
support for this?
-V Charu Latha--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
