Roger Quadros <ext-roger.quadros@xxxxxxxxx> writes:
> ext charu@xxxxxx 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 <charu@xxxxxx>
>
> 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 majordomo@xxxxxxxxxxxxxxx
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
