Re: [PATCH 7/8] hwmon: (lm90) read the channel's label from device-tree

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Slawomir Stepien <sst@poczta.fm>
Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
	jdelvare@suse.com, linux@roeck-us.net, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, przemyslaw.cencner@nokia.com,
	krzysztof.adamski@nokia.com, alexander.sverdlin@nokia.com,
	Slawomir Stepien <slawomir.stepien@nokia.com>
Subject: Re: [PATCH 7/8] hwmon: (lm90) read the channel's label from device-tree
Date: Fri, 20 May 2022 13:41:34 +0200	[thread overview]
Message-ID: <edfc8e63-74d0-0f4c-20a0-7e6c8d7d9a5a@linaro.org> (raw)
In-Reply-To: <Yod07jXUCEQIYcRj@t480s.localdomain>

On 20/05/2022 13:01, Slawomir Stepien wrote:

>>> +static int lm90_probe_channel_from_dt(struct i2c_client *client,
>>> +				      struct device_node *child,
>>> +				      struct lm90_data *data)
>>> +{
>>> +	u32 id;
>>> +	int err;
>>> +	struct device *dev = &client->dev;
>>> +
>>> +	err = of_property_read_u32(child, "reg", &id);
>>> +	if (err) {
>>> +		dev_err(dev, "missing reg property of %pOFn\n", child);
>>> +		return err;
>>> +	}
>>> +
>>> +	if (id >= MAX_CHANNELS) {
>>> +		dev_err(dev, "invalid reg %d in %pOFn\n", id, child);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	err = of_property_read_string(child, "label", &data->channel_label[id]);
>>> +	if (err == -ENODATA || err == -EILSEQ) {
>>> +		dev_err(dev, "invalid label in %pOFn\n", child);
>>
>> You did not make it a required property, so why failing?
> 
> But if you have it in device-tree, then at least inform the user that there is some problem with it.

Hm, true, the missing property would return EINVAL. Sounds good then.
x

Best regards,
Krzysztof