Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Received-SPF: Pass (protection.outlook.com: domain of seco.com designates
 20.160.56.86 as permitted sender) receiver=protection.outlook.com;
 client-ip=20.160.56.86; helo=inpost-eu.tmcas.trendmicro.com; pr=C
Authentication-Results-Original: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=seco.com;
Message-ID: <c3cda013-eab0-46c0-a89a-ed51ecfd1e1d@seco.com>
Date:   Mon, 11 Dec 2023 11:03:39 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3] rtc: abx80x: Don't warn about oscillator failure after
 PoR
Content-Language: en-US
To:     Alessandro Zummo <a.zummo@towertech.it>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        linux-rtc@vger.kernel.org
Cc:     linux-kernel@vger.kernel.org
References: <20231019163931.3682923-1-sean.anderson@seco.com>
From:   Sean Anderson <sean.anderson@seco.com>
In-Reply-To: <20231019163931.3682923-1-sean.anderson@seco.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-Microsoft-Antispam-Message-Info-Original: +lKDBNLGagjOrFxIaVVUoy4I4WsDBWowjTca5BGREoN1z7Wn2RaaH9PeyHLOHQviRoaaS1umFEfDN8STwA0OlElJ2OFNUaY/BxHa5AYUlPcbzgnfbWu82DCa/I+rwQ7yBerlHZbc5geu6PGYx6/MX2QRoydovPZFt65Zkf6I31cIXTd9cgqLSSN8BSPgcuUTsT73ngbWQlsLhO08dicRhxROv3dwFc7VASe9PNRVarTwZxtqPco8PhqDycW8sq+yucCcWnayxiUf0I0V2LUCXZLvpaeIofoIi9lN0ACzscHJXBL8tmEphRgp6FtP5QO1aJg6Srp854FE5H1m3bnZ6b/KRFxQ7WVPXXMRfPMbcnvWAueVYK2sEWI+qwCQXDYjCv9Nf8EbFNiMU5+oY5i9Q45Fs5wOEyIcGzARb1vJ6KCc1Z1umvJPaUrxfYmuW4s/7kl+T8W7XMoFRzDwhwbAzfUWDgaVGzeTFa+Dc1rviEzyNkyOTKeD2O1dDyT408lLCt/JeDniEMfc0nxWyeNlYfnbnGsUMmt3tDa1CXT1SdCKBU6i0rpMhP01va0/N483zVkKemiIG5spdL9j1p1AfVWJ4HTvfRydxH+9INDTRxoDl7+H37PZFygg8wgvizEUT9jLX6g3n1HGn+2zXlD1pyVwGGNb5M0z4qEZ8hjwN5IMYijCR6HPDpsGTQ9WAxA1+3bhAFRCsk9eWS8iKyJP3jsEVakKcYPIDbqhs0Mgchs=
X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DB9PR03MB8847.eurprd03.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(136003)(366004)(396003)(39850400004)(376002)(346002)(230273577357003)(230173577357003)(230922051799003)(1800799012)(451199024)(64100799003)(186009)(6512007)(53546011)(6506007)(5660300002)(26005)(2616005)(44832011)(36756003)(66946007)(6486002)(66476007)(66556008)(2906002)(83380400001)(31686004)(41300700001)(38350700005)(478600001)(86362001)(8676002)(8936002)(4326008)(52116002)(31696002)(110136005)(316002)(38100700002)(6666004)(43740500002)(45980500001);DIR:OUT;SFP:1101;
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBBPR03MB10395
X-EOPAttributedMessage: 0
X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM6EUR05FT063.eop-eur05.prod.protection.outlook.com
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id-Prvs: 34aa8a4b-d386-4dd5-3f44-08dbfa62c0a2
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: /dYtreir/uZYa0M+DBcQTuibEQnldZDkk3klILb72uH63Ms3/pKVfBFU7IaluHZJ72G59bjqtoEGzIyrwr9u+BLzfOcEvdFe2Z2bB8/eowsGhIl1t+715cXeHY2XC9f4qr6UAQw5meM9u/hU/4ULpJ7bxBWTvUs7cxQOiC1YNkY8hNcr4PbbhWS6yqUpMNfK1Y3wFW/ZqMS+HqzGpm1+io4RDsHxCxvKrQN4uej5yM2UCBC/1haaf9slVuAmHmkznCw+Ye3Rer2oxUYyfTT+Bt6D8EsG5hxjORdGxFfoSnYBvO4yqcwS36Mjb7TiCu8Hn8hYbkoBc1SAX4OEV0cm23ild/zKojXt1anWcED1bHKJi86cKNHH6Wv4wN/EHSP3Ze9EZru5rhyeGMJgwh9vgZvgogEtGroYmAI9OrzIiSfIDfzG8B/U42V/2d5EQm0TWdHziAL8mFjDM1Ft8FU0vB1cjGZKId6NdkNj6/F+jqUYsLOMF8wFqHx2AyTS2xP98Vz18ibsr8dtMlMs4xntCCjEH+Af1+19TkA9ImaZGF35FJX5VU8dishf//z4aKY+V4CYgSVgTQR7Ms6vZwk1I/itvkSgb7SV+QVJrek1IcKPo7e26sPQ128aCcQsQ7dbRniKC2Ok6dXIOQg8DmYjM1IFqQyu3vI+Gb1LY1BslE27DwnCDLe+6cUzZWyrELB2vVKE+CM7Ue3zkASz2ifggi5qWxSzctqRQsu615vOu4SedpmBTdbAOsEa3m79SsrDBkEfVlZebeJg5xiTjbm1gi0yK1vIkow+UbFKIOHV4KKoLPpamuKKQt+iwO9DErQt
X-Forefront-Antispam-Report: CIP:20.160.56.86;CTRY:NL;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:inpost-eu.tmcas.trendmicro.com;PTR:inpost-eu.tmcas.trendmicro.com;CAT:NONE;SFS:(13230031)(39850400004)(376002)(346002)(136003)(396003)(230922051799003)(230173577357003)(230273577357003)(451199024)(186009)(64100799003)(1800799012)(82310400011)(36840700001)(46966006)(41300700001)(34020700004)(36860700001)(47076005)(336012)(26005)(83380400001)(2616005)(82740400003)(86362001)(36756003)(31696002)(7596003)(7636003)(356005)(44832011)(5660300002)(316002)(8676002)(8936002)(4326008)(2906002)(6666004)(6512007)(6506007)(53546011)(70586007)(70206006)(110136005)(478600001)(6486002)(31686004)(40480700001)(43740500002);DIR:OUT;SFP:1101;
X-OriginatorOrg: seco.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Dec 2023 16:03:50.5163
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 8d05c5a6-123f-4250-a6b2-08dbfa62c407
X-MS-Exchange-CrossTenant-Id: bebe97c3-6438-442e-ade3-ff17aa50e733
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=bebe97c3-6438-442e-ade3-ff17aa50e733;Ip=[20.160.56.86];Helo=[inpost-eu.tmcas.trendmicro.com]
X-MS-Exchange-CrossTenant-AuthSource: AM6EUR05FT063.eop-eur05.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7PR03MB6264
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Mon, 11 Dec 2023 08:05:49 -0800 (PST)

On 10/19/23 12:39, Sean Anderson wrote:
> According to the datasheet, the "oscillator failure" bit is set
> 
>> ...on a power on reset, when both the system and battery voltages have
>> dropped below acceptable levels. It is also set if an Oscillator Failure
>> occurs....
> 
> From testing, this bit is also set if a software reset is initiated.
> 
> This bit has a confusing name; it really tells us whether the time data
> is valid. We clear it when writing the time. If it is still set, that
> means there is a persistent issue (such as an oscillator failure),
> instead of a transient one (such as power loss).
> 
> Because there are several other reasons which might cause this bit
> to be set (including booting for the first time or a battery failure),
> do not warn about oscillator failures willy-nilly. This may cause system
> integrators to waste time looking into the wrong line of investigation.
> 
> We continue printing a message about invalid time data or an oscillator
> failure. There is no voltimeter in this RTC, so this is the best
> indication that the battery is dead (or dying) and reeds replacement.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
> Note that the following drivers all warn when they detect a problem with
> the oscillator:
> 
> drivers/rtc/rtc-ds1672.c
> drivers/rtc/rtc-pcf*.c
> drivers/rtc/rtc-rs5c*.c
> drivers/rtc/rtc-sc27xx.c
> 
> So warning about such an error has good precedent.
> 
> Changes in v3:
> - Use info since this is a good indication of a battery failure
> 
> Changes in v2:
> - Use debug instead of info in the typical case (no battery)
> 
>  drivers/rtc/rtc-abx80x.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index fde2b8054c2e..f463a58a240b 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
> @@ -127,6 +127,7 @@ struct abx80x_priv {
>  	struct rtc_device *rtc;
>  	struct i2c_client *client;
>  	struct watchdog_device wdog;
> +	bool wrote_time;
>  };
>  
>  static int abx80x_write_config_key(struct i2c_client *client, u8 key)
> @@ -179,6 +180,7 @@ static int abx80x_enable_trickle_charger(struct i2c_client *client,
>  static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
> +	struct abx80x_priv *priv = i2c_get_clientdata(client);
>  	unsigned char buf[8];
>  	int err, flags, rc_mode = 0;
>  
> @@ -193,7 +195,18 @@ static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  			return flags;
>  
>  		if (flags & ABX8XX_OSS_OF) {
> -			dev_err(dev, "Oscillator failure, data is invalid.\n");
> +			/*
> +			 * The OF bit can be set either because of a reset
> +			 * (PoR/Software reset) or because of an oscillator
> +			 * failure. Effectively, it indicates that the stored
> +			 * time is invalid. When we write the time, we clear
> +			 * this bit. If it stays set, then this indicates an
> +			 * oscillator failure.
> +			 */
> +			if (priv->wrote_time)
> +				dev_err(dev, "Oscillator failure\n");
> +			else
> +				dev_info(dev, "Time data invalid\n");
>  			return -EINVAL;
>  		}
>  	}
> @@ -219,6 +232,7 @@ static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
> +	struct abx80x_priv *priv = i2c_get_clientdata(client);
>  	unsigned char buf[8];
>  	int err, flags;
>  
> @@ -252,6 +266,7 @@ static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  		dev_err(&client->dev, "Unable to write oscillator status register\n");
>  		return err;
>  	}
> +	priv->wrote_time = true;
>  
>  	return 0;
>  }

ping?