Re: [PATCH] Add support for the S-35390A RTC chip.

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

Hi Byron,

On Sat,  5 Jan 2008 22:58:14 +0000, Byron Bradley wrote:
> This adds basic get/set time support for the Seiko Instruments
> S-35390A. This chip communicates using I2C and is used on the
> QNAP TS-109/TS-209 NAS devices. It takes up eight addresses on
> the I2C bus and depends on the following patches:
> 
> 	i2c-remove-redundant-i2c_client-list.patch
> 	i2c-add-i2c_new_dummy-utility.patch
> 
> Signed-off-by: Byron Bradley <byron.bbradley-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Tested-by: Tim Ellis <tim-ReWupQYfQ/MAvxtiuMwx3w@public.gmane.org>
> Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
> ---
>  drivers/rtc/Kconfig       |    9 ++
>  drivers/rtc/Makefile      |    1 +
>  drivers/rtc/rtc-s35390a.c |  316 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 326 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/rtc/rtc-s35390a.c

Acked-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>

With just one comment:

> +	/* This chip uses multiple addresses, use dummy devices for them */
> +	for (i = 1; i < 8; ++i) {
> +		s35390a->client[i] = i2c_new_dummy(client->adapter,
> +					client->addr + i, "rtc-s35390a");
> +		if (!s35390a->client[i]) {
> +			dev_err(&client->dev, "Address %d unavailable\n",
> +						client->addr + i);
> +			err = -ENOCSI;
> +			goto exit_dummy;
> +		}
> +	}

-ENOCSI? Don't you rather mean -EBUSY?

Also, the address would be better printed as an hexadecimal value -
decimal I2C addresses isn't something people are used to.

You should get your driver reviewed by Alessandro Zummo now, and then
it can be merged upstream... After the i2c cleanups it depends on are
upstream themselves, that is (working on it.)

-- 
Jean Delvare