lkml.org 
[lkml]   [2007]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [rtc-linux] [PATCH 1/2] rtc: add rtc-m41txx driver
    From
    On Wed, 20 Jun 2007 00:28:28 +0200, Alessandro Zummo <alessandro.zummo@towertech.it> wrote:
    > I'd prefer if you change the name in rtc-m41t80 and add
    > the names of the supported chips in the Kconfig entry.

    OK, I will do. I thought M41ST94 also can be supported but that was
    wrong. From datasheets, the driver can be used for M41T8[0123],
    M41ST8[457] chips.

    > > + unsigned char wbuf[1 + M41TXX_REG_YEAR + 1];
    >
    > would be easier to understand if you have some #defines
    > with the sizes you require for the various buffers.

    I will do.

    > the proc interface will go away sooner or later, would be better to expose
    > the battery status with a sysfs attribute.

    I will do.

    > > +++ b/include/linux/m41txx.h
    > > @@ -0,0 +1,40 @@
    > > +/*
    > > + * Definitions for the ST M41TXX family of i2c rtc chips.
    > > + *
    > > + * Based on m41t00.h by Mark A. Greer <mgreer@mvista.com>
    > > + *
    > > + * This program is free software; you can redistribute it and/or modify
    > > + * it under the terms of the GNU General Public License version 2 as
    > > + * published by the Free Software Foundation.
    >
    > Unless this include is required by other files, please place
    > it in drivers/rtc calling it rtc-drivername.h

    It is required by platform code to provide SQW setting. Also
    M41TXX_DRV_NAME and M41TXX_I2C_ADDR might be useful to declare
    i2c_board_info, which would be something like:

    static struct m41txx_platform_data pdata = {
    .sqw_freq = M41TXX_SQW_32KHZ,
    };
    static struct i2c_board_info binfo __initdata = {
    I2C_BOARD_INFO(M41TXX_DRV_NAME, M41TXX_I2C_ADDR),
    .type = "m41t80",
    .platform_data = &pdata,
    };
    return i2c_register_board_info(0, &binfo, 1);

    > Other than the comments above, the driver
    > seems fine to me.

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

    \
     
     \ /
      Last update: 2007-06-20 04:19    [W:0.023 / U:118.868 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site