From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com [209.85.221.48]) (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 30433479876 for ; Thu, 13 Aug 2026 12:43:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786625020; cv=none; b=tVCGmRFpEdJrFtsveAN3zThD15xWuT9LjLKrFI0JeDWMLyYzEbKxL+herAZnn0dPt5X3hQcvN4xxR+SaIe4i5eskRIZPaHjqfXdEYieGqfnZZixgsKkwf7FW617yyNcS+FRdbSOydL6GE1+Cb8XyHCuULbxIoA1M7aetrSh5jP4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786625020; c=relaxed/simple; bh=XCRyYa9Cj2cBnzNJz+uLg9K78+0Q04t78FS8jSLyM8k=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:To:From:Subject: References:In-Reply-To; b=sTM2KMC/lnjG4SQLdLJ5xZ6Tl6s6c9QO6MgsjB+iyO5iBPhz1JAKGh3cc+wPGsfb+A4PnBi1XMODAOc8taeX5p8CLc9w145d5LCdgI7rjd3/Hj3jvt3RwKgXhs5UgKQdijnYMVm0TSQw98TeqbwLgLeifZcrD6B68JzQAHl40sU= 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=Q9+Uzqur; arc=none smtp.client-ip=209.85.221.48 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 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Q9+Uzqur" Received: by mail-wr1-f48.google.com with SMTP id ffacd0b85a97d-47fde295992so684373f8f.0 for ; Thu, 13 Aug 2026 05:43:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1786625010; x=1787229810; darn=vger.kernel.org; h=in-reply-to:references:subject:from:to:cc:message-id:date :content-type:content-transfer-encoding:mime-version:from:to:cc :subject:date:message-id:reply-to:content-type; bh=Wrg2VIt6trGfNzd/hwCmmeNh9EVzBuveLNTMhpJY8Qc=; b=Q9+Uzqur1Gz03gxp1I1gre5k12YPLSRHlWLBKEUk76UnkrMbPTv7tQM6HxziLCYfpb 9Uor95fHj1ZHVs4yeOe0mC02MxyXSWdJ8pcRZdyCphMRYmhSNoEvfU/2Qzh4Rus+69O5 vtilB0zUZJS1eArtrDXezoZ7vG51dYjXeniUG8sjUsrhMDCCW5WxOLXKDhIohHFp85ir iQKWQVHYgc38/omb0NEfyrp29noBtVHrWKeybbCJb9tIskN7sr2yvj9OEVHtD09Qc+Lk O+NiYbPNghVOxu401jV6CkKCObGpj6q4+TyaNR/iX8WQ2nM8AI8UtpAMClw3S8pKf1PP oGYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786625010; x=1787229810; h=in-reply-to:references:subject:from:to:cc:message-id:date :content-type:content-transfer-encoding:mime-version:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=Wrg2VIt6trGfNzd/hwCmmeNh9EVzBuveLNTMhpJY8Qc=; b=HRYuVBixynalRoRB6cgU0UD/2oa9BlgsjR/04U7ofhg0wgxP3TIf8TAqvQ+jZdVGgH lwm2+19c79Yv3e5Sibc/0dTdJmkV1g6pv2ts/3CnEeOjqJHgN+tX74N9NsgBN1xoDB80 z7thX9qC1F1+a4b+6IvqYBT5FJ1fJU0cKlq493eu3GktzI9oSdzrqkM8y4DbXpEUw3S1 LJLkXxnQBd+O3jI5tQ6G28CDKhT2zEEldIkuww/lX93StGX5RRudtw1ckxV6/uK1yQKz YDRkGvDQzmWvDi57zwm/GgOs40EIFA8to5Awj1nV3k15NYlJ4ZaRxlJfTcP3ZR/AQB11 5m4Q== X-Forwarded-Encrypted: i=1; AHgh+Rq9DNnLSfWtRJZo1K4QsYY/Gy2E/+jP4N12Yd/6XO2ftJ3Qrbn3kIlXolegNnavTisrd91zSuI8y1aeVEw=@vger.kernel.org X-Gm-Message-State: AOJu0YxKChrJrzeaFk5Wr/TG6LTix4bJbPlCipfBmZfy+BQ/j/IjBBLd fBxf0vIOOdY0PPCXTaWtOwbVMQx8Y0fB2VuwS588lrpKv8DIPCQMD4b8 X-Gm-Gg: AR+sD11PcjKxokAhO9mQAK7O+EyEhshQgtYJVK1Z6aE7JE3JuTT9DhdcV9c3qp3GtxZ wJwc8haye9t0sS6A81T0Ec8fo7t+f4HUFMqHKGwvOzeaAdgYCR8JqwrUsCyFRdZTzus8U+dfTfP STlpiFEyRBSK8TiHTqoUYSt/Pbe+49B2xN0soSjbH+ip/pjbAE6JZ020gm5LzIKNtwjFvBrHnQq Jhrr+Z9tQunknUNUTsJKURTi7f4bQktb2aYngFUV2XPfAsg6QXb7TT7m03TvzFx+48f4CBjv6Dx xQnnsy/MugPqfgL/FeW8rfdPloDqJ1llOUxAICfxTp7YUfdeznA0OpiP88al9V9yPVGIKQ6cfef 8xdMW8wo6C8Amzeet7tUgr8B0cXru49GCw23Pr5IBAKmKenwG+xn9Cbl73IHR+ydhMClQ5FK9p/ KV8K+c2esT5EK5wkKg4PfolIuPkS1VFpzFrti8v6rx8K8enx5QOEqHRUskhOkMUj0GL8Sg++Tkp VXnSL56A7Igx8ZjB/A= X-Received: by 2002:adf:e189:0:b0:47f:ec53:1d2d with SMTP id ffacd0b85a97d-4815a4f981fmr7957694f8f.7.1786625009500; Thu, 13 Aug 2026 05:43:29 -0700 (PDT) Received: from localhost ([2001:4bb8:16f:15e0:beeb:f51b:da5e:3a64]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4815a5c2842sm6423824f8f.35.2026.08.13.05.43.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 13 Aug 2026 05:43:29 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 13 Aug 2026 14:43:26 +0200 Message-Id: Cc: "Lars-Peter Clausen" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "David Lechner" , =?utf-8?q?Nuno_S=C3=A1?= , "Andy Shevchenko" , , , To: "Jonathan Cameron" , "Javier Carrasco" From: "Javier Carrasco" Subject: Re: [PATCH v6 4/4] iio: light: veml6031x00: add support for events and trigger X-Mailer: aerc 0.21.0-143-g2f3a2e260c09 References: <20260812-veml6031x00-v6-0-7eef6e4ce290@gmail.com> <20260812-veml6031x00-v6-4-7eef6e4ce290@gmail.com> <20260813022436.41a28d64@jic23-huawei> In-Reply-To: <20260813022436.41a28d64@jic23-huawei> Hi Jonathan, thank you for your review to the series. On Thu Aug 13, 2026 at 3:24 AM CEST, Jonathan Cameron wrote: > On Wed, 12 Aug 2026 22:27:43 +0200 > Javier Carrasco wrote: > >> The device provides a shared interrupt line for to notify events and >> data ready, which can be used as a trigger. The interrupt line is not a >> requirement for the device to work. Implement variants for the cases >> whether the interrupt line is provided or not. >> >> Signed-off-by: Javier Carrasco >> @@ -549,11 +948,78 @@ static int veml6031x00_buffer_postdisable(struct i= io_dev *iio) >> return 0; >> } >> >> +static int veml6031x00_set_trigger_state(struct iio_trigger *trig, bool= state) >> +{ >> + struct iio_dev *iio =3D iio_trigger_get_drvdata(trig); >> + struct veml6031x00_data *data =3D iio_priv(iio); >> + int ret; >> + >> + guard(mutex)(&data->irq_lock); >> + >> + if (state =3D=3D data->trig_en) >> + return 0; >> + >> + ret =3D veml6031x00_set_interrupt(data, state); >> + if (ret) >> + return ret; >> + >> + /* The AF bit must be updated before updating AF_TRIG */ >> + ret =3D regmap_update_bits(data->regmap, VEML6031X00_REG_CONF0, >> + VEML6031X00_CONF0_AF, >> + FIELD_PREP(VEML6031X00_CONF0_AF, state)); >> + if (ret) { >> + veml6031x00_set_interrupt(data, !state); >> + >> + return ret; >> + } >> + >> + ret =3D regmap_update_bits(data->regmap, VEML6031X00_REG_CONF0, >> + VEML6031X00_CONF0_AF_TRIG, >> + FIELD_PREP(VEML6031X00_CONF0_AF_TRIG, state)); >> + if (ret) { >> + regmap_update_bits(data->regmap, VEML6031X00_REG_CONF0, >> + VEML6031X00_CONF0_AF, >> + FIELD_PREP(VEML6031X00_CONF0_AF, !state)); >> + veml6031x00_set_interrupt(data, !state); > > This dance vs a goto is I guess due to the mutex. I'd clean it up > by using a helper function for the stuff done under the guard(). The > helper can do goto based cleanup and avoid repetition plus reduce chance > of missing cleaning something up on error. The outer function can > still use guard(). > Yes, that was the reason why some code was duplicated. I will add a helper function with the __must_hold() annotation and lockdep_assert_held(). >> + >> + return ret; >> + } >> + >> + data->trig_en =3D state; >> + >> + return 0; >> +} > >> >> +static int veml6031x00_setup_irq(struct i2c_client *i2c, struct iio_dev= *iio) >> +{ >> + struct veml6031x00_data *data =3D iio_priv(iio); >> + struct device *dev =3D regmap_get_device(data->regmap); >> + int ret; >> + >> + data->trig =3D devm_iio_trigger_alloc(dev, "%s-drdy%d", >> + iio->name, iio_device_id(iio)); >> + if (!data->trig) >> + return -ENOMEM; >> + >> + data->trig->ops =3D &veml6031x00_trigger_ops; >> + iio_trigger_set_drvdata(data->trig, iio); >> + >> + ret =3D devm_iio_trigger_register(dev, data->trig); >> + if (ret) >> + return ret; >> + >> + iio->trig =3D iio_trigger_get(data->trig); > > Sashiko is correct that we loose a reference here on error and right now > there is no IIO core infrastructure to solve this > > Why are we setting a default trigger? Userspace tools should be > fine looking for a data ready trigger, or choosing a different one if the= y > would prefer. Added advantage of not setting it here is the reference cou= nt > issue goes away :) > I will drop iio->trig =3D iio_trigger_get(data->trig) for v7. I added it because it is (or at least, it was) a common practice in many II= O drivers to assign their own trigger, and in the end it is by far the most common use case. Of course, we're not going to touch existing drivers to remove that, but is it then something to be advised against in the future unless there is a good reason for it? Thanks and best regards, Javier