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

>> ---
>>  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;
>> +}
>> +
>> +/*
>>   * 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
> --
> 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
--
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

Reply via email to