linux-kernel - Re: [PATCH v3] drivers: rtc: Add driver for SD2405AL.

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] [day] [month] [year] [list]

Message-ID: <21ca87a0-6062-411d-88e4-c644c054e497@prolan.hu>
Date: Mon, 24 Jun 2024 10:09:46 +0200
From: Csókás Bence <csokas.bence@...lan.hu>
To: <gomba007@...il.com>, Alexandre Belloni <alexandre.belloni@...tlin.com>
CC: <linux-kernel@...r.kernel.org>, <linux-rtc@...r.kernel.org>
Subject: Re: [PATCH v3] drivers: rtc: Add driver for SD2405AL.

Hi!

> +static int sd2405al_setup(struct sd2405al *sd2405al)
> +{
> +	unsigned int val;
> +	int ret;

I would still remove `val` and inline everything.

> +static int sd2405al_check_state(struct sd2405al *sd2405al)
> +{
> +	u8 data[2] = { 0 };
> +	int state;
> +	int ret;
> +
> +	ret = regmap_bulk_read(sd2405al->regmap, SD2405AL_REG_CTR1, data, 2);
> +	if (ret < 0) {
> +		dev_err(sd2405al->dev, "read failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* CRT1 */
> +	state = (data[0] & (SD2405AL_BIT_WRTC2 | SD2405AL_BIT_WRTC3)) != 0;
> +
> +	/* CTR2 */
> +	state &= (data[1] & SD2405AL_BIT_WRTC1) != 0;
> +
> +	return state;

Same here.

> +static int sd2405al_read_time(struct device *dev, struct rtc_time *time)
> +{
> +	u8 hour;

This variable could also be inlined.

Other than that, LGTM. Though I don't have this chip, I can only go by 
the linked datasheet.

On 6/20/24 11:55, Tóth János via B4 Relay wrote:
 > From: Tóth János <gomba007@...il.com>
 >
 > Add support for the DFRobot SD2405AL I2C RTC Module.
 >
 > Datasheet:
 > 
https://image.dfrobot.com/image/data/TOY0021/SD2405AL%20datasheet%20(Angelo%20v0.1).pdf
 >
 > Product:
 > 	https://www.dfrobot.com/product-1600.html
 >
 > To instantiate (assuming device is connected to I2C-1)
 > as root:
 > 	echo sd2405al 0x32 > /sys/bus/i2c/devices/i2c-1/new_device
 > as user:
 > 	echo 'sd2405al 0x32' | sudo tee /sys/class/i2c-adapter/i2c-1/new_device
 >
 > The driver is tested with:
 > 	+ hwclock
 > 	+ tools/testing/selftests/rtc/setdate
 > 	+ tools/testing/selftests/rtc/rtctest
 >
 > Signed-off-by: Tóth János <gomba007@...il.com>

Reviewed-by: Csókás, Bence <csokas.bence@...lan.hu>