[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20080912115530.79a3c1a3.akpm@linux-foundation.org>
Date: Fri, 12 Sep 2008 11:55:30 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: David Brownell <david-b@...bell.net>
Cc: rtc-linux@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [patch 2.6.27 mmotm] rtc-cmos: export second NVRAM bank
On Fri, 12 Sep 2008 09:57:55 -0700
David Brownell <david-b@...bell.net> wrote:
> From: David Brownell <dbrownell@...rs.sourceforge.net>
>
> Teach rtc-cmos about the second bank of registers found on most
> modern x86 systems, giving access to 128 bytes more NVRAM.
>
> This version only sees that extra NVRAM when both register banks
> are provided as part of *one* PNP resource. Since BIOS on some
> systems presents them using two IO resources, and nothing merges
> them, this can't always show all the NVRAM. (We're supposed to
> be able to use PNP id PNP0b01 too, but BIOS tables doesn't often
> seem to use that particular option.)
>
> Signed-off-by: David Brownell <dbrownell@...rs.sourceforge.net>
> ---
> For 2.6.28; applies after other pending patches
>
> drivers/rtc/rtc-cmos.c | 70 ++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 63 insertions(+), 7 deletions(-)
>
> --- a/drivers/rtc/rtc-cmos.c
> +++ b/drivers/rtc/rtc-cmos.c
> @@ -153,6 +153,43 @@ static inline int hpet_unregister_irq_ha
>
> /*----------------------------------------------------------------*/
>
> +#ifdef RTC_PORT
> +
> +/* Most newer x86 systems have two register banks, the first used
> + * for RTC and NVRAM and the second only for NVRAM. Caller must
> + * own rtc_lock ... and we won't worry about access during NMI.
> + */
> +#define can_bank2 true
It would be more idiomatic to make this upper-case: CAN_BANK2.
> +static inline unsigned char cmos_read_bank2(unsigned char addr)
> +{
> + outb(addr, RTC_PORT(2));
> + return inb(RTC_PORT(3));
> +}
> +
> +static inline void cmos_write_bank2(unsigned char val, unsigned char addr)
> +{
> + outb(addr, RTC_PORT(2));
> + outb(val, RTC_PORT(2));
> +}
> +
> +#else
> +
> +#define can_bank2 false
> +
> +static inline unsigned char cmos_read_bank2(unsigned char addr)
> +{
> + return 0;
> +}
> +
> +static inline void cmos_write_bank2(unsigned char val, unsigned char addr)
> +{
> +}
> +
> +#endif
> +
> +/*----------------------------------------------------------------*/
> +
> static int cmos_read_time(struct device *dev, struct rtc_time *t)
> {
> /* REVISIT: if the clock has a "century" register, use
> @@ -511,12 +548,21 @@ cmos_nvram_read(struct kobject *kobj, st
>
> if (unlikely(off >= attr->size))
> return 0;
> + if (unlikely(off < 0))
> + return -EINVAL;
> if ((off + count) > attr->size)
> count = attr->size - off;
> + off += NVRAM_OFFSET;
hm, now what's happening in here.
: static ssize_t
: cmos_nvram_read(struct kobject *kobj, struct bin_attribute *attr,
: char *buf, loff_t off, size_t count)
: {
: int retval;
:
: if (unlikely(off >= attr->size))
: return 0;
: if (unlikely(off < 0))
: return -EINVAL;
: if ((off + count) > attr->size)
: count = attr->size - off;
:
: off += NVRAM_OFFSET;
:
The VFS will (hopefully) prevent ->read methods from being called with
a negative file offset. What prompted the additional test for that?
I did't look at it exhaustively but I suspect that the above code won't
work right if attr->size has a value of around (2^31 - 42) and `offset'
is (2^31 - 54) and NVRAM_OFFSET==54. Or something like that. It looks
holey ;)
Of course, the assumption that attr->size is not insanely large is a
good one, but still..
> spin_lock_irq(&rtc_lock);
> - for (retval = 0, off += NVRAM_OFFSET; count--; retval++, off++)
> - *buf++ = CMOS_READ(off);
> + for (retval = 0; count; count--, off++, retval++) {
> + if (off < 128)
> + *buf++ = CMOS_READ(off);
> + else if (can_bank2)
> + *buf++ = cmos_read_bank2(off);
> + else
> + break;
> + }
--
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/
Powered by blists - more mailing lists