Received: by 2002:a05:7412:bbc7:b0:fc:a2b0:25d7 with SMTP id kh7csp2295175rdb; Mon, 5 Feb 2024 01:54:00 -0800 (PST) X-Google-Smtp-Source: AGHT+IG9nXsFlHJ4GSe6YLbyRXkfM/cNKizpV8ITyeQQ8GmBFlL59uTphio06bfEnBHN24pQALsH X-Received: by 2002:a17:907:7746:b0:a36:fb2c:8701 with SMTP id kx6-20020a170907774600b00a36fb2c8701mr7158297ejc.55.1707126840640; Mon, 05 Feb 2024 01:54:00 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1707126840; cv=pass; d=google.com; s=arc-20160816; b=XqJ0qZvksHZNQ4h0eQFNRji/rL/x8QXjG3Fx4p+awdlmYTQPIlM7RybnZm1XuL+AT1 aUXaR3EIiN5L18137lgdohnBQBK3GtrF08Kr/wNxiKddSLbvl7WE2e46RYtbXNZCPCpt hTyEX7tFsmJvfYhx5o9qxPSF1js0k1d8VXACTg2lMYDobYflmXudlwt8mMe+A4guAA85 P9Si5G8Wold+XJQCwXkgi4l4pe4q6hyy+wxilYPSPlBgQXP3aUsvAe+83tRX+oeHD0+L iHFybH4/gKrGHygHmSAHCEkkliZzuhAD413I1uvIKvpj+7Z4JSzl2lMpIq5Bc0SrC+qv Mx+g== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-disposition:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:message-id:subject:cc :to:from:date:dkim-signature; bh=KVR1d4N0MAmwAVoqQZoMPRejFBb2H9WAFtXUDQ1yD9E=; fh=bVSjRsLY9CIa9tTkqK4OzrsgoG8JZrDycyiYuHjsX9o=; b=XwjuwN+vkUFMTUVEtKD66foy2otRgAwti6v8jUFq6rxFqpnMBPuHG8bt+nG8Jhscfo qqNQWvbL03okyTyaL8uqCsh606uDY1nbcStYbKbmOF75tcGL3jfAcPK99lR2UXVp6M/h Iu87amExEIFZ4Dl8vnCD5cNozH5NN4r60x46Pms3ohZAGXoZsZi2AkPgvYAprYg609Bx k0YgNhJ+d+uY6W8aBVRtYh833ga4X10H7k/pT5T9VFM0WwF+8i6dyM2ci22yiZhvDrXE 7ZqHw7ZPX+hg0KpsV0jlZG6Dr8tyVk5SI055QcxZhNqRo6d0GoJK1uI2y6N1qmfa/s/e JTbg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=BbOG8hLt; arc=pass (i=1 spf=pass spfdomain=gmail.com dkim=pass dkdomain=gmail.com dmarc=pass fromdomain=gmail.com); spf=pass (google.com: domain of linux-kernel+bounces-52353-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-52353-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com X-Forwarded-Encrypted: i=1; AJvYcCUB6b9ofzaLaIJq5fjuF0KYhvTEAfa0d1dTePy16ooLeapIUKaeXB56Lk0iBa/ElLTo5QGNSbOjaasOfBTdzLuuqfEhA+9EDGrVoayrtA== Return-Path: Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [147.75.80.249]) by mx.google.com with ESMTPS id f18-20020a17090624d200b00a3782589b42si1792622ejb.806.2024.02.05.01.54.00 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Feb 2024 01:54:00 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-52353-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=BbOG8hLt; arc=pass (i=1 spf=pass spfdomain=gmail.com dkim=pass dkdomain=gmail.com dmarc=pass fromdomain=gmail.com); spf=pass (google.com: domain of linux-kernel+bounces-52353-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-52353-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 346801F2154C for ; Mon, 5 Feb 2024 09:54:00 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 14BCD12E7D; Mon, 5 Feb 2024 09:53:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="BbOG8hLt" Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 73B44134C6; Mon, 5 Feb 2024 09:53:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707126810; cv=none; b=TXDvvt0LOZz8bkprlZzmRCQjXDHT0/g37PpzTUqOA+T91SHnVuBptd1f0qVfYV1A6AZGRiH0SF8pI53/3h4lYcX/d9RbTogBJ7u9B3m21lpTxPedXUeKN9bK9FmUxv6ipNCivYP0HPBw6rPvbmJTFpnaOG5BDIfUC37OyFN0+yg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707126810; c=relaxed/simple; bh=qzWq2fEmrI0y52InyhdcUfWfRevTQvm9xcPd55lTSPM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=b+scZrqTiHtCDt/yXYFyM3Yi6/QmGgY5Z9HBfdKmAMAttLUFJ8TcG40IPMp6A8kXsmUWbDmHCyf2orzPAiqD++Wdgt5TDDN+T7pEtcxZ2XcSberW8luRyhPPxMKhy+UwBe6/Dq8YCP0wA1XzlCx4yY+EqN9+0IouzPBnEDOct9s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=BbOG8hLt; arc=none smtp.client-ip=209.85.218.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ej1-f47.google.com with SMTP id a640c23a62f3a-a2f79e79f0cso566687366b.2; Mon, 05 Feb 2024 01:53:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1707126806; x=1707731606; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=KVR1d4N0MAmwAVoqQZoMPRejFBb2H9WAFtXUDQ1yD9E=; b=BbOG8hLt5fPuxdhwdb3WBze8kgOV+ReUfM4JC4yFPvx7gIoXiLgVFIsEk7JgygPg/y 9gEZ2a+NxziUQhX8+uS0KI+WeBJszJcRSE2wyp6qSDtIhkVNCh94whW/TxmDmdXTwh5k yFa7CZqr3eOnJ4JayuS5gbeiYf2S+egXpzr9GvyOKtendOhMmbnoi9BQKLjYNQeRag7u N8bXWCvpcyp67QGLrDzzaMxGbZOa303oklfs3ZvAhrQdGS2jqYRu36GGSr+/4bw4B8Lg WTMA1XiAsefCDYM9rvnTrAsJnPN37cApyI3Aqg6DAcgMN1HXWYsOr+j7ROTd0Cf84VDZ ShNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707126806; x=1707731606; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=KVR1d4N0MAmwAVoqQZoMPRejFBb2H9WAFtXUDQ1yD9E=; b=RK985XY96Lg4kEiP/U+dQy9h/jehOY101BfwfEsq09cdQrS89Rt7qtFSEw31Yl2Uh0 Qfs7CbYOCsCcrnrfy3pvh5Xzbi+Fa47fFiGDtmg2IJ4nHzIvhzpksWiLnyYiDr0cgMFN 9K2di2L6KR3itZrpcyId8/0gz0Vz+ZDU4baXBOSMez8F00GFLAoGPzQFko4RcCw3VaUk LrnaHMJmomjjr+DM+a8YGn/t9JYb5Dz/tR3jEcgaFdm1hFWMs2xwJ+PAr9ji6S63eNG7 d6L272ouowktrAaAUg4m55kBb1SeeN1I3houcYQ2DokCSWLtYA4mVG8nHeSKmpWA3m/6 1ZMA== X-Gm-Message-State: AOJu0Yz9Li2eYqo2IYaa8fy0sWggBk+yf7ZTxVjGEHhQKwoouwS+diCT zvbeYApRtCcNFTzsNzlWKPnhRnHco/ePecTCfKk3Puz5IdcxszMq X-Received: by 2002:a17:906:8309:b0:a37:b2e6:cf65 with SMTP id j9-20020a170906830900b00a37b2e6cf65mr1879217ejx.52.1707126806411; Mon, 05 Feb 2024 01:53:26 -0800 (PST) X-Forwarded-Encrypted: i=0; AJvYcCX7k6NLnblNcyc7fQ9YFcgOWMe2KUTVWvxSmSfu0qdx4ejn58XMDppo3lUAVeZgFWmlSWsapTeOkg0EvaG6lYRULh3YKDnqLxaU2Kd6rcnZbQv0C6LlaTlbym9haj1BBKMh6Ngs0PqBUsT5y6IqAhxCW7cot5gRQaY0LVb7a0kzCH/9M2E2h3gLO4PKH+iBvunHwx0hHtjabkyRPVSM31N2dbg+cg5F5hUoHA== Received: from debian ([93.184.186.109]) by smtp.gmail.com with ESMTPSA id v8-20020a170906488800b00a37dcf17d3asm416088ejq.174.2024.02.05.01.53.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Feb 2024 01:53:25 -0800 (PST) Date: Mon, 5 Feb 2024 10:53:23 +0100 From: Dimitri Fedrau To: Jonathan Cameron Cc: Jonathan Cameron , Javier Carrasco , Li peiyu <579lpy@gmail.com>, Lars-Peter Clausen , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] iio: humidity: hdc3020: add threshold events support Message-ID: <20240205095323.GA2323766@debian> References: <20240204103710.19212-1-dima.fedrau@gmail.com> <20240204144347.7f0eb822@jic23-huawei> <20240205070421.GA2264419@debian> <20240205093349.00003e10@Huawei.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240205093349.00003e10@Huawei.com> Am Mon, Feb 05, 2024 at 09:33:49AM +0000 schrieb Jonathan Cameron: > > > > static const u8 HDC3020_S_AUTO_10HZ_MOD0[2] = { 0x27, 0x37 }; > > > > > > > > +static const u8 HDC3020_S_STATUS[2] = { 0x30, 0x41 }; > > > > + > > > > static const u8 HDC3020_EXIT_AUTO[2] = { 0x30, 0x93 }; > > > > > > > > +static const u8 HDC3020_S_T_RH_THRESH_LOW[2] = { 0x61, 0x00 }; > > > > > > Ah. missed this in original driver, but this use of capitals for > > > non #defines is really confusing and we should aim to clean that > > > up. > > > > > Could use small letters instead. > > That would avoid any confusion. > > > > > > As I mention below, I'm unconvinced that it makes sense to handle > > > these as pairs. > > > > > For the threshold I could convert it as it is for the heater registers: > > > > #define HDC3020_S_T_RH_THRESH_MSB 0x61 > > #define HDC3020_S_T_RH_THRESH_LOW 0x00 > > #define HDC3020_S_T_RH_THRESH_LOW_CLR 0x0B > > #define HDC3020_S_T_RH_THRESH_HIGH_CLR 0x16 > > #define HDC3020_S_T_RH_THRESH_HIGH 0x1D > > > > #define HDC3020_R_T_RH_THRESH_MSB 0xE1 > > #define HDC3020_R_T_RH_THRESH_LOW 0x02 > > #define HDC3020_R_T_RH_THRESH_LOW_CLR 0x09 > > #define HDC3020_R_T_RH_THRESH_HIGH_CLR 0x14 > > #define HDC3020_R_T_RH_THRESH_HIGH 0x1F > > > > or: > > > > #define HDC3020_S_T_RH_THRESH_LOW 0x6100 > > #define HDC3020_S_T_RH_THRESH_LOW_CLR 0x610B > > #define HDC3020_S_T_RH_THRESH_HIGH_CLR 0x6116 > > #define HDC3020_S_T_RH_THRESH_HIGH 0x611D > > > > #define HDC3020_R_T_RH_THRESH_LOW 0x6102 > > #define HDC3020_R_T_RH_THRESH_LOW_CLR 0x6109 > > #define HDC3020_R_T_RH_THRESH_HIGH_CLR 0x6114 > > #define HDC3020_R_T_RH_THRESH_HIGH 0x611F > > > > I don't know if it's a good idea, as we would need to make sure it is > > big endian in the buffer. Probably with a function that handles this. > I think this is the best plan with a > put_unaligned_be16() to deal with the endianness. > The compiler should be able to optimize that heavily. > I think that would require some refactoring. I would add patches that are fixing this. Have there been reasons for using the pairs ? I'm just curious. > > > > > +static int hdc3020_read_thresh(struct iio_dev *indio_dev, > > > > + const struct iio_chan_spec *chan, > > > > + enum iio_event_type type, > > > > + enum iio_event_direction dir, > > > > + enum iio_event_info info, > > > > + int *val, int *val2) > > > > +{ > > > > + struct hdc3020_data *data = iio_priv(indio_dev); > > > > + u16 *thresh; > > > > + > > > > + /* Select threshold */ > > > > + if (info == IIO_EV_INFO_VALUE) { > > > > + if (dir == IIO_EV_DIR_RISING) > > > > + thresh = &data->t_rh_thresh_high; > > > > + else > > > > + thresh = &data->t_rh_thresh_low; > > > > + } else { > > > > + if (dir == IIO_EV_DIR_RISING) > > > > + thresh = &data->t_rh_thresh_high_clr; > > > > + else > > > > + thresh = &data->t_rh_thresh_low_clr; > > > > + } > > > > + > > > > + guard(mutex)(&data->lock); > > > > > > Why take the lock here? > > > > > > you are relying on a single value that is already cached. > > > > > A single threshold value is used for humidity and temperature values. I > > didn't see a lock in "iio_ev_value_show", so there might be some > > concurrent access triggered by "in_temp_thresh_rising_value" and > > "in_humidityrelative_thresh_rising_value" sysfs files which is not > > secured by a mutex or similiar. > > Unless you going to get value tearing (very unlikely and lots of the > kernel assumes that won't happen - more of a theoretical possibility > that we don't want compilers to do!) this just protects against a race > where you read one and write the other. That doesn't really help us > as it just moves the race to which one gets the lock first. > Yes, it's very unlikely to happen. Anyway, I'm dropping the support for the caching and with it this function. Dimitri