linux-kernel - Re: [PATCH v3] rtc: omap: add support for pmic_power

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20141027154031.4492ea11d401045ca04a3ff8@linux-foundation.org>
Date:	Mon, 27 Oct 2014 15:40:31 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Johan Hovold <johan@...nel.org>
Cc:	Alessandro Zummo <a.zummo@...ertech.it>,
	Tony Lindgren <tony@...mide.com>,
	Benoît Cousson <bcousson@...libre.com>,
	Felipe Balbi <balbi@...com>, Lokesh Vutla <lokeshvutla@...com>,
	Guenter Roeck <linux@...ck-us.net>, nsekhar@...com,
	t-kristo@...com, j-keerthy@...com, linux-omap@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
	rtc-linux@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] rtc: omap: add support for pmic_power_en

On Mon, 27 Oct 2014 09:09:28 +0100 Johan Hovold <johan@...nel.org> wrote:

> Add new property "ti,system-power-controller" to register the RTC as a
> power-off handler.
> 
> Some RTC IP revisions can control an external PMIC via the pmic_power_en
> pin, which can be configured to transition to OFF on ALARM2 events and
> back to ON on subsequent ALARM (wakealarm) events.
> 
> This is based on earlier work by Colin Foe-Parker and AnilKumar Ch. [1]
> 
> [1] https://www.mail-archive.com/linux-omap@vger.kernel.org/msg82127.html
> 
> Tested-by: Felipe Balbi <balbi@...com>
> Signed-off-by: Johan Hovold <johan@...nel.org>
> ---
> 
> Changes since v2:
>  - add two-second delay to allow alarm to trigger before returning

hmpf.  As this sentence is below the ^--- it doesn't get into the
changelog.

> ...
>
> +static void omap_rtc_power_off(void)
> +{
> +	struct omap_rtc *rtc = omap_rtc_power_off_rtc;
> +	struct rtc_time tm;
> +	unsigned long now;
> +	u32 val;
> +
> +	/* enable pmic_power_en control */
> +	val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
> +	rtc_writel(rtc, OMAP_RTC_PMIC_REG, val | OMAP_RTC_PMIC_POWER_EN_EN);
> +
> +	/* set alarm two seconds from now */
> +	omap_rtc_read_time_raw(rtc, &tm);
> +	bcd2tm(&tm);
> +	rtc_tm_to_time(&tm, &now);
> +	rtc_time_to_tm(now + 2, &tm);
> +
> +	if (tm2bcd(&tm) < 0) {
> +		dev_err(&rtc->rtc->dev, "power off failed\n");
> +		return;
> +	}
> +
> +	rtc_wait_not_busy(rtc);
> +
> +	rtc_write(rtc, OMAP_RTC_ALARM2_SECONDS_REG, tm.tm_sec);
> +	rtc_write(rtc, OMAP_RTC_ALARM2_MINUTES_REG, tm.tm_min);
> +	rtc_write(rtc, OMAP_RTC_ALARM2_HOURS_REG, tm.tm_hour);
> +	rtc_write(rtc, OMAP_RTC_ALARM2_DAYS_REG, tm.tm_mday);
> +	rtc_write(rtc, OMAP_RTC_ALARM2_MONTHS_REG, tm.tm_mon);
> +	rtc_write(rtc, OMAP_RTC_ALARM2_YEARS_REG, tm.tm_year);
> +
> +	/*
> +	 * enable ALARM2 interrupt
> +	 *
> +	 * NOTE: this fails on AM3352 if rtc_write (writeb) is used
> +	 */
> +	val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
> +	rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
> +			val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
> +
> +	mdelay(2000);

And it is uncommented.

How on earth is a reader to know why this is here?

I can do this

--- a/drivers/rtc/rtc-omap.c~rtc-omap-add-support-for-pmic_power_en-v3-fix
+++ a/drivers/rtc/rtc-omap.c
@@ -417,6 +417,7 @@ static void omap_rtc_power_off(void)
 	rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
 			val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
 
+	/* Allow alarm to trigger before returning */
 	mdelay(2000);
 }
 


But it doesn't explain *why* we want the alarm to trigger before
returning.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/