From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="R55KsC/N" Received: from mail-ej1-x636.google.com (mail-ej1-x636.google.com [IPv6:2a00:1450:4864:20::636]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7751BA6; Thu, 14 Dec 2023 04:57:39 -0800 (PST) Received: by mail-ej1-x636.google.com with SMTP id a640c23a62f3a-a1db6c63028so935949866b.2; Thu, 14 Dec 2023 04:57:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1702558658; x=1703163458; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=g2GKWz1rRyCNwha7gGcRolq2RILUdPIiIt5FzDdvILQ=; b=R55KsC/NDMFJnCucTbjK5g5o6ELzDY2jFcfTZR678xc3Cyqzv4YZEAfkohXIY2IU07 4dl4paNF15fmPy/hDhPj4ZT9E/ws8+OPCaaEZXSf745M3Mza6oET9Y+g0BvZub2aDCKH lHVcwFLKwhyCwvdWLhDXy6mUPqgAe7lfRhv0ubP7boMtSjypehMixvZk41QdZXx2NyM/ RrI7cu6nbU3pT9XRYB/PtgZUKhsJg+O4JqxnSS1H9uloO3a794NcJJy3ijw+CpFR99tL fvRd7A/zUqUO0zZCvA1kM/ZfLPRFL//PzPpphkn4gvi2h/VVWCi4WBOQDKAyhvvQeWZE sQSw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702558658; x=1703163458; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=g2GKWz1rRyCNwha7gGcRolq2RILUdPIiIt5FzDdvILQ=; b=fnXUPG4rDFyD+WBn32ix6SI74MeCIA64XP/p0HXV+lPPaNrNkMYKKigvQvLSrgan10 MNbXKTPo3YdP8gZiqPPs8p11Tvwi860UvQN6cN91+EMsj8oUE0SdwZB6j5Dg15KVNLF/ 1krsZgZ1qo88P+JxiET/QX7UAh5o9KsNo0nUfOPW7LIffr1jpmWNp+8RPf3hcewuIRI2 5pr8jvlkydqj+9pqCx9VQ27meRr5TN6di+wabj3Xa+xG1HMpnU9e07DN0K5GMQ0992hK yojN3OiqGXLHEB4lTfQdN3puUoUlTGN9DJNONA/VlLBdz9OhodWMxBs259ElAl0enwfS Swmw== X-Gm-Message-State: AOJu0YyTmFIzCznmuAM9dzM5NctEhmlUlOFmuQXcXqXEZeTOfv0smpyU EmHXAhpHTWNxtcV3CMLbtlU= X-Google-Smtp-Source: AGHT+IHtIY7rFcQC93tfnJBXkLGQRNbPQabINCfXb9w5tCgijrk98Ge2WRXokyVfdLbqczA2V1R56A== X-Received: by 2002:a17:906:4546:b0:a01:fc1b:8197 with SMTP id s6-20020a170906454600b00a01fc1b8197mr4262851ejq.62.1702558657491; Thu, 14 Dec 2023 04:57:37 -0800 (PST) Received: from [172.25.98.130] ([5.2.194.157]) by smtp.gmail.com with ESMTPSA id ub14-20020a170907c80e00b00a1da5d9a602sm9368347ejc.138.2023.12.14.04.57.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 14 Dec 2023 04:57:36 -0800 (PST) Message-ID: <375bf803-a5d5-4778-938a-b8218b116375@gmail.com> Date: Thu, 14 Dec 2023 14:57:35 +0200 Precedence: bulk X-Mailing-List: linux-gpio@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v8 2/2] iio: adc: ad7173: add AD7173 driver Content-Language: en-US To: Jonathan Cameron Cc: linus.walleij@linaro.org, brgl@bgdev.pl, andy@kernel.org, linux-gpio@vger.kernel.org, Lars-Peter Clausen , Jonathan Cameron , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Michael Walle , Andy Shevchenko , Arnd Bergmann , ChiaEn Wu , Niklas Schnelle , =?UTF-8?Q?Leonard_G=C3=B6hrs?= , Mike Looijmans , Haibo Chen , Hugo Villeneuve , Ceclan Dumitru , linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org References: <20231212104451.22522-1-mitrutzceclan@gmail.com> <20231212104451.22522-2-mitrutzceclan@gmail.com> <20231214123029.000002f1@Huawei.com> From: Ceclan Dumitru In-Reply-To: <20231214123029.000002f1@Huawei.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 12/14/23 14:30, Jonathan Cameron wrote: > On Tue, 12 Dec 2023 12:44:36 +0200 > Dumitru Ceclan wrote: > >> The AD7173 family offer a complete integrated Sigma-Delta ADC solution >> which can be used in high precision, low noise single channel >> applications or higher speed multiplexed applications. The Sigma-Delta >> ADC is intended primarily for measurement of signals close to DC but also >> delivers outstanding performance with input bandwidths out to ~10kHz. >> >> Reviewed-by: Michael Walle # for gpio-regmap >> Signed-off-by: Dumitru Ceclan > Hi > > Given it seems like you'll be doing a v9, one quick comment from me below. > > Jonathan > >> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c >> new file mode 100644 >> index 000000000000..96918b24a10a >> --- /dev/null >> +++ b/drivers/iio/adc/ad7173.c >> @@ -0,0 +1,964 @@ > ... > >> +static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev) >> +{ > > ... > >> + >> + if (st->info->has_temp) { >> + chan_arr[chan_index] = ad7173_temp_iio_channel_template; >> + chan_st_priv = &channels_st_priv_arr[chan_index]; >> + chan_st_priv->ain = >> + AD7173_CH_ADDRESS(chan_arr[chan_index].channel, chan_arr[chan_index].channel2); >> + chan_st_priv->cfg.bipolar = false; >> + chan_st_priv->cfg.input_buf = true; >> + chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF; >> + st->adc_mode |= AD7173_ADC_MODE_REF_EN; >> + >> + chan_index++; >> + } >> + >> + device_for_each_child_node(dev, child) { >> + chan = &chan_arr[chan_index]; >> + chan_st_priv = &channels_st_priv_arr[chan_index]; >> + ret = fwnode_property_read_u32_array(child, "diff-channels", >> + ain, ARRAY_SIZE(ain)); >> + if (ret) { >> + fwnode_handle_put(child); >> + return ret; >> + } >> + >> + if (ain[0] >= st->info->num_inputs || >> + ain[1] >= st->info->num_inputs) { >> + fwnode_handle_put(child); >> + return dev_err_probe(dev, -EINVAL, >> + "Input pin number out of range for pair (%d %d).\n", >> + ain[0], ain[1]); >> + } >> + >> + ret = fwnode_property_match_property_string(child, >> + "adi,reference-select", >> + ad7173_ref_sel_str, >> + ARRAY_SIZE(ad7173_ref_sel_str)); >> + >> + if (ret < 0) >> + ref_sel = AD7173_SETUP_REF_SEL_INT_REF; >> + else >> + ref_sel = ret; > Simpler pattern for properties with a default is not to check the error code. > > ref_sel = AD7173_SETUP_REF_SEL_INT_REF; > > fwnode_property_match_property_String(child, ... > > so only if it succeeds is the value overridden. Where exactly would the value be overridden, the function does not have an argument passed for the found index. The function is written to return either the found index or a negative error. The proposed pattern would just ignore the returned index and would always leave ref_sel to default. Am I missing something? I can see in the thread where it was introduced that you proposed: "Looking at the usecases I wonder if it would be better to pass in an unsigned int *ret which is only updated on a match?" But on the iio togreg branch that was suggested I could the function on, it does not have that parameter.