Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   =?utf-8?Q?=C5=81ukasz_Stelmach?= <l.stelmach@samsung.com>
To:     Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc:     linux-rtc@vger.kernel.org,
        =?utf-8?Q?Bart=C5=82omiej_=C5=BBolnierkiew?= =?utf-8?Q?icz?= 
        <b.zolnierkie@samsung.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] rtc: ds1307: replace HAS_ALARM by RTC_FEATURE_ALARM
Date:   Fri, 02 Apr 2021 12:26:38 +0200
In-Reply-To: <20210330000343.801566-1-alexandre.belloni@bootlin.com>
        (Alexandre Belloni's message of "Tue, 30 Mar 2021 02:03:41 +0200")
Message-ID: <dleftjv995dlsx.fsf%l.stelmach@samsung.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-="; micalg="pgp-sha256";
        protocol="application/pgp-signature"
CMS-TYPE: 201P
References: <20210330000343.801566-1-alexandre.belloni@bootlin.com>
        <CGME20210402102655eucas1p24ee7e879816089921bf9752c3c483122@eucas1p2.samsung.com>
Precedence: bulk

--=-=-=
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

It was <2021-03-30 wto 02:03>, when Alexandre Belloni wrote:
> The core now has RTC_FEATURE_ALARM for the driver to indicate whether
> alarms are available. Use that instead of HAS_ALARM to ensure the alarm
> callbacks are not even called.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  drivers/rtc/rtc-ds1307.c | 42 +++++++---------------------------------
>  1 file changed, 7 insertions(+), 35 deletions(-)
>

Tested-by: =C5=81ukasz Stelmach <l.stelmach@samsung.com>
Reviewed-by: =C5=81ukasz Stelmach <l.stelmach@samsung.com>

> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index cd8e438bc9c4..76d67c419f7d 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -171,7 +171,6 @@ struct ds1307 {
>  	enum ds_type		type;
>  	unsigned long		flags;
>  #define HAS_NVRAM	0		/* bit 0 =3D=3D sysfs file active */
> -#define HAS_ALARM	1		/* bit 1 =3D=3D irq claimed */
>  	struct device		*dev;
>  	struct regmap		*regmap;
>  	const char		*name;
> @@ -411,9 +410,6 @@ static int ds1337_read_alarm(struct device *dev, stru=
ct rtc_wkalrm *t)
>  	int			ret;
>  	u8			regs[9];
>=20=20
> -	if (!test_bit(HAS_ALARM, &ds1307->flags))
> -		return -EINVAL;
> -
>  	/* read all ALARM1, ALARM2, and status registers at once */
>  	ret =3D regmap_bulk_read(ds1307->regmap, DS1339_REG_ALARM1_SECS,
>  			       regs, sizeof(regs));
> @@ -454,9 +450,6 @@ static int ds1337_set_alarm(struct device *dev, struc=
t rtc_wkalrm *t)
>  	u8			control, status;
>  	int			ret;
>=20=20
> -	if (!test_bit(HAS_ALARM, &ds1307->flags))
> -		return -EINVAL;
> -
>  	dev_dbg(dev, "%s secs=3D%d, mins=3D%d, "
>  		"hours=3D%d, mday=3D%d, enabled=3D%d, pending=3D%d\n",
>  		"alarm set", t->time.tm_sec, t->time.tm_min,
> @@ -512,9 +505,6 @@ static int ds1307_alarm_irq_enable(struct device *dev=
, unsigned int enabled)
>  {
>  	struct ds1307		*ds1307 =3D dev_get_drvdata(dev);
>=20=20
> -	if (!test_bit(HAS_ALARM, &ds1307->flags))
> -		return -ENOTTY;
> -
>  	return regmap_update_bits(ds1307->regmap, DS1337_REG_CONTROL,
>  				  DS1337_BIT_A1IE,
>  				  enabled ? DS1337_BIT_A1IE : 0);
> @@ -592,9 +582,6 @@ static int rx8130_read_alarm(struct device *dev, stru=
ct rtc_wkalrm *t)
>  	u8 ald[3], ctl[3];
>  	int ret;
>=20=20
> -	if (!test_bit(HAS_ALARM, &ds1307->flags))
> -		return -EINVAL;
> -
>  	/* Read alarm registers. */
>  	ret =3D regmap_bulk_read(ds1307->regmap, RX8130_REG_ALARM_MIN, ald,
>  			       sizeof(ald));
> @@ -634,9 +621,6 @@ static int rx8130_set_alarm(struct device *dev, struc=
t rtc_wkalrm *t)
>  	u8 ald[3], ctl[3];
>  	int ret;
>=20=20
> -	if (!test_bit(HAS_ALARM, &ds1307->flags))
> -		return -EINVAL;
> -
>  	dev_dbg(dev, "%s, sec=3D%d min=3D%d hour=3D%d wday=3D%d mday=3D%d mon=
=3D%d "
>  		"enabled=3D%d pending=3D%d\n", __func__,
>  		t->time.tm_sec, t->time.tm_min, t->time.tm_hour,
> @@ -681,9 +665,6 @@ static int rx8130_alarm_irq_enable(struct device *dev=
, unsigned int enabled)
>  	struct ds1307 *ds1307 =3D dev_get_drvdata(dev);
>  	int ret, reg;
>=20=20
> -	if (!test_bit(HAS_ALARM, &ds1307->flags))
> -		return -EINVAL;
> -
>  	ret =3D regmap_read(ds1307->regmap, RX8130_REG_CONTROL0, &reg);
>  	if (ret < 0)
>  		return ret;
> @@ -735,9 +716,6 @@ static int mcp794xx_read_alarm(struct device *dev, st=
ruct rtc_wkalrm *t)
>  	u8 regs[10];
>  	int ret;
>=20=20
> -	if (!test_bit(HAS_ALARM, &ds1307->flags))
> -		return -EINVAL;
> -
>  	/* Read control and alarm 0 registers. */
>  	ret =3D regmap_bulk_read(ds1307->regmap, MCP794XX_REG_CONTROL, regs,
>  			       sizeof(regs));
> @@ -793,9 +771,6 @@ static int mcp794xx_set_alarm(struct device *dev, str=
uct rtc_wkalrm *t)
>  	unsigned char regs[10];
>  	int wday, ret;
>=20=20
> -	if (!test_bit(HAS_ALARM, &ds1307->flags))
> -		return -EINVAL;
> -
>  	wday =3D mcp794xx_alm_weekday(dev, &t->time);
>  	if (wday < 0)
>  		return wday;
> @@ -842,9 +817,6 @@ static int mcp794xx_alarm_irq_enable(struct device *d=
ev, unsigned int enabled)
>  {
>  	struct ds1307 *ds1307 =3D dev_get_drvdata(dev);
>=20=20
> -	if (!test_bit(HAS_ALARM, &ds1307->flags))
> -		return -EINVAL;
> -
>  	return regmap_update_bits(ds1307->regmap, MCP794XX_REG_CONTROL,
>  				  MCP794XX_BIT_ALM0_EN,
>  				  enabled ? MCP794XX_BIT_ALM0_EN : 0);
> @@ -1641,7 +1613,7 @@ static int ds3231_clks_register(struct ds1307 *ds13=
07)
>  		 * Interrupt signal due to alarm conditions and square-wave
>  		 * output share same pin, so don't initialize both.
>  		 */
> -		if (i =3D=3D DS3231_CLK_SQW && test_bit(HAS_ALARM, &ds1307->flags))
> +		if (i =3D=3D DS3231_CLK_SQW && test_bit(RTC_FEATURE_ALARM, ds1307->rtc=
->features))
>  			continue;
>=20=20
>  		init.name =3D ds3231_clks_names[i];
> @@ -1964,15 +1936,15 @@ static int ds1307_probe(struct i2c_client *client,
>  			     bin2bcd(tmp));
>  	}
>=20=20
> -	if (want_irq || ds1307_can_wakeup_device) {
> -		device_set_wakeup_capable(ds1307->dev, true);
> -		set_bit(HAS_ALARM, &ds1307->flags);
> -	}
> -
>  	ds1307->rtc =3D devm_rtc_allocate_device(ds1307->dev);
>  	if (IS_ERR(ds1307->rtc))
>  		return PTR_ERR(ds1307->rtc);
>=20=20
> +	if (want_irq || ds1307_can_wakeup_device)
> +		device_set_wakeup_capable(ds1307->dev, true);
> +	else
> +		clear_bit(RTC_FEATURE_ALARM, ds1307->rtc->features);
> +
>  	if (ds1307_can_wakeup_device && !want_irq) {
>  		dev_info(ds1307->dev,
>  			 "'wakeup-source' is set, request for an IRQ is disabled!\n");
> @@ -1988,7 +1960,7 @@ static int ds1307_probe(struct i2c_client *client,
>  		if (err) {
>  			client->irq =3D 0;
>  			device_set_wakeup_capable(ds1307->dev, false);
> -			clear_bit(HAS_ALARM, &ds1307->flags);
> +			clear_bit(RTC_FEATURE_ALARM, ds1307->rtc->features);
>  			dev_err(ds1307->dev, "unable to request IRQ!\n");
>  		} else {
>  			dev_dbg(ds1307->dev, "got IRQ %d\n", client->irq);

=2D-=20
=C5=81ukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEXpuyqjq9kGEVr9UQsK4enJilgBAFAmBm8V4ACgkQsK4enJil
gBB8Wwf/e8msV8AcisRqv2L0td0Tfvj+L0VjMSXIlwH9u4ie+lka1fXwmePfCm1q
jRPdU2KXVqxMx9okrytV+yWue8frfgxJNOpYfHmLZsHNnyVZ/IrmuxlN+sI4iUwG
AWd/KreKX5P73/86I4HdECOA0+eM9OXesnw9OfwvrsgIbFaHID89yE8778ziN3up
Gy4rgZf/lkQiJqSM2OcQj4SrzVALhAGZcxPknBBA7rFHzoZk/wEVEEgtfQomj+km
NmG0TO0G3g5Ll8anPIs/++fOhVCoM+2USPTgm9Dj6tmFqBkf7yHil/fLo3pEDbhn
vs63o/fXKhoBmcrVj8xT83ldK1FJDw==
=cIa1
-----END PGP SIGNATURE-----
--=-=-=--