[PATCH v5 0/2] iio: proximity: driver for vcnl3020

[PATCH v5 0/2] iio: proximity: driver for vcnl3020

[Ivan Mikhaylov]

Add proximity sensor driver for Vishay vcnl3020. Only on-demand
measurement is supported for now.

Changes from v4:
   1. add vdd-supply,vddio-supply,interrupts properties into yaml.
   2. led-current -> vishay,led-current-milliamp.
   3. add possible values enum list.
   4. add bulk_read for result hi/lo registers.
   5. add description of vcnl3020_data structure.
   6. vcnl3020 id table is removed.
   7. make "vishay,led-current-milliamp" optional in yaml and code.

Changes from v3:
   1. minor changes.
   2. add i2c block to fix dts section in yaml.

Changes from v2:
   1. using regmap_read_poll_timeout instead of do-while in measurement
      function.
   2. change struct i2client* in vcnl3020_data to struct dev*
   3. enable REGMAP_I2C in Kconfig

Changes from v1:
   1. using regmap interface instead of i2c_smbus_* calls.
   2. switch from probe to probe_new.
   3. s32/int32_t -> int

Ivan Mikhaylov (2):
  iio: proximity: provide device tree binding document
  iio: proximity: Add driver support for vcnl3020 proximity sensor

 .../bindings/iio/proximity/vcnl3020.yaml      |  65 +++++
 drivers/iio/proximity/Kconfig                 |  11 +
 drivers/iio/proximity/Makefile                |   1 +
 drivers/iio/proximity/vcnl3020.c              | 225 ++++++++++++++++++
 4 files changed, 302 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/proximity/vcnl3020.yaml
 create mode 100644 drivers/iio/proximity/vcnl3020.c

-- 
2.21.1

[PATCH v5 1/2] iio: proximity: provide device tree binding document

[Ivan Mikhaylov]

Mostly standard i2c driver with some additional led-current option
for vcnl3020.

Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
 .../bindings/iio/proximity/vcnl3020.yaml      | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/proximity/vcnl3020.yaml

diff --git a/Documentation/devicetree/bindings/iio/proximity/vcnl3020.yaml b/Documentation/devicetree/bindings/iio/proximity/vcnl3020.yaml
new file mode 100644
index 000000000000..29ab2dee1694
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/vcnl3020.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/proximity/vcnl3020.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Integrated Proximity Sensor With Infrared Emitter
+
+maintainers:
+  - Ivan Mikhaylov <i.mikhaylov@yadro.com>
+
+description: |
+  The VCNL3020 is a fully integrated proximity sensor. Fully integrated means
+  that the infrared emitter is included in the package. It has 16-bit
+  resolution. It includes a signal processing IC and features standard I2C
+  communication interface. It features an interrupt function.
+
+  Specifications about the devices can be found at:
+  https://www.vishay.com/docs/84150/vcnl3020.pdf
+
+properties:
+  compatible:
+    enum:
+      - vishay,vcnl3020
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description: IRQ line for the proximity sensor
+    maxItems: 1
+
+  vdd-supply:
+    description: Regulator that provides power to the sensor
+
+  vddio-supply:
+    description: Regulator that provides power to the bus
+
+  vishay,led-current-milliamp:
+    description:
+        IR LED current value with valid Range = 0 to 20d. e.g. 0 = 0 mA,
+        1 = 10 mA, 20 = 200 mA (2 = 20 mA = DEFAULT). LED Current is
+        limited to 200 mA for values higher than decimal 20.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
+              18, 19, 20]
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    i2c {
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        proximity@13 {
+              compatible = "vishay,vcnl3020";
+              reg = <0x13>;
+              vishay,led-current-milliamp = <0x14>;
+        };
+    };
-- 
2.21.1

[PATCH v5 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor

[Ivan Mikhaylov]

Proximity sensor driver based on light/vcnl4000.c code.
For now supports only the single on-demand measurement.

The VCNL3020 is a fully integrated proximity sensor. Fully
integrated means that the infrared emitter is included in the
package. It has 16-bit resolution. It includes a signal
processing IC and features standard I2C communication
interface. It features an interrupt function.

Datasheet: http://www.vishay.com/docs/84150/vcnl3020.pdf
Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
 drivers/iio/proximity/Kconfig    |  11 ++
 drivers/iio/proximity/Makefile   |   1 +
 drivers/iio/proximity/vcnl3020.c | 226 +++++++++++++++++++++++++++++++
 3 files changed, 238 insertions(+)
 create mode 100644 drivers/iio/proximity/vcnl3020.c

diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index d53601447da4..b8d2b17e60ac 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -112,6 +112,17 @@ config SRF08
 	  To compile this driver as a module, choose M here: the
 	  module will be called srf08.
 
+config VCNL3020
+	tristate "VCNL3020 proximity sensor"
+	select REGMAP_I2C
+	depends on I2C
+	help
+	  Say Y here if you want to build a driver for the Vishay VCNL3020
+	  proximity sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called vcnl3020.
+
 config VL53L0X_I2C
 	tristate "STMicroelectronics VL53L0X ToF ranger sensor (I2C)"
 	depends on I2C
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index 0bb5f9de13d6..8245978ced30 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -12,5 +12,6 @@ obj-$(CONFIG_RFD77402)		+= rfd77402.o
 obj-$(CONFIG_SRF04)		+= srf04.o
 obj-$(CONFIG_SRF08)		+= srf08.o
 obj-$(CONFIG_SX9500)		+= sx9500.o
+obj-$(CONFIG_VCNL3020)		+= vcnl3020.o
 obj-$(CONFIG_VL53L0X_I2C)	+= vl53l0x-i2c.o
 
diff --git a/drivers/iio/proximity/vcnl3020.c b/drivers/iio/proximity/vcnl3020.c
new file mode 100644
index 000000000000..d4b0684c39fe
--- /dev/null
+++ b/drivers/iio/proximity/vcnl3020.c
@@ -0,0 +1,226 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Support for Vishay VCNL3020 proximity sensor on i2c bus.
+ * Based on Vishay VCNL4000 driver code.
+ *
+ * TODO: interrupts.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/regmap.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define VCNL_DRV_NAME		"vcnl3020"
+#define VCNL_REGMAP_NAME	"vcnl3020_regmap"
+#define VCNL3020_PROD_ID	0x21
+
+#define VCNL_COMMAND		0x80 /* Command register */
+#define VCNL_PROD_REV		0x81 /* Product ID and Revision ID */
+#define VCNL_PROXIMITY_RATE	0x82 /* Rate of Proximity Measurement */
+#define VCNL_LED_CURRENT	0x83 /* IR LED current for proximity mode */
+#define VCNL_PS_RESULT_HI	0x87 /* Proximity result register, MSB */
+#define VCNL_PS_RESULT_LO	0x88 /* Proximity result register, LSB */
+#define VCNL_PS_ICR		0x89 /* Interrupt Control Register  */
+
+#define VCNL_PS_LO_THR_HI	0x8a /* High byte of low threshold value */
+#define VCNL_PS_LO_THR_LO	0x8b /* Low byte of low threshold value */
+#define VCNL_PS_HI_THR_HI	0x8c /* High byte of high threshold value */
+#define VCNL_PS_HI_THR_LO	0x8d /* Low byte of high threshold value */
+#define VCNL_ISR		0x8e /* Interrupt Status Register */
+#define VCNL_PS_MOD_ADJ		0x8f /* Proximity Modulator Timing Adjustment */
+
+/* Bit masks for COMMAND register */
+#define VCNL_PS_RDY		BIT(5) /* proximity data ready? */
+#define VCNL_PS_OD		BIT(3) /* start on-demand proximity
+					* measurement
+					*/
+
+#define VCNL_ON_DEMAND_TIMEOUT_US	100000
+#define VCNL_POLL_US			20000
+
+/**
+ * struct vcnl3020_data - vcnl3020 specific data.
+ * @regmap:	device register map.
+ * @dev:	vcnl3020 device.
+ * @rev:	revision id.
+ * @lock:	lock for protecting access to device hardware registers.
+ */
+struct vcnl3020_data {
+	struct regmap *regmap;
+	struct device *dev;
+	u8 rev;
+	struct mutex lock;
+};
+
+static int vcnl3020_init(struct vcnl3020_data *data)
+{
+	int rc;
+	unsigned int reg;
+	u32 led_current;
+
+	rc = regmap_read(data->regmap, VCNL_PROD_REV, &reg);
+	if (rc) {
+		dev_err(data->dev,
+			"Error (%d) reading product revision", rc);
+		return rc;
+	}
+
+	if (reg != VCNL3020_PROD_ID) {
+		dev_err(data->dev,
+			"Product id (%x) did not match vcnl3020 (%x)", reg,
+			VCNL3020_PROD_ID);
+		return -ENODEV;
+	}
+
+	data->rev = reg;
+	mutex_init(&data->lock);
+
+	rc = device_property_read_u32(data->dev, "vishay,led-current-milliamp",
+				      &led_current);
+	if (rc == 0) {
+		rc = regmap_write(data->regmap, VCNL_LED_CURRENT, led_current);
+		if (rc)
+			dev_err(data->dev,
+				"Error (%d) setting LED current", rc);
+	}
+
+	return rc;
+};
+
+static int vcnl3020_measure_proximity(struct vcnl3020_data *data, int *val)
+{
+	int rc;
+	unsigned int reg;
+	__be16 res;
+
+	mutex_lock(&data->lock);
+
+	rc = regmap_write(data->regmap, VCNL_COMMAND, VCNL_PS_OD);
+	if (rc)
+		goto err_unlock;
+
+	/* wait for data to become ready */
+	rc = regmap_read_poll_timeout(data->regmap, VCNL_COMMAND, reg,
+				      reg & VCNL_PS_RDY, VCNL_POLL_US,
+				      VCNL_ON_DEMAND_TIMEOUT_US);
+	if (rc) {
+		dev_err(data->dev,
+			"vcnl3020_measure() failed with error (%d)", rc);
+		goto err_unlock;
+	}
+
+	/* high & low result bytes read */
+	rc = regmap_bulk_read(data->regmap, VCNL_PS_RESULT_HI, &res, 2);
+	if (rc)
+		goto err_unlock;
+
+	*val = be16_to_cpu(res);
+
+err_unlock:
+	mutex_unlock(&data->lock);
+
+	return rc;
+}
+
+static const struct iio_chan_spec vcnl3020_channels[] = {
+	{
+		.type = IIO_PROXIMITY,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+	},
+};
+
+static int vcnl3020_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, int *val,
+			     int *val2, long mask)
+{
+	int rc;
+	struct vcnl3020_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->type) {
+		case IIO_PROXIMITY:
+			rc = vcnl3020_measure_proximity(data, val);
+			if (rc)
+				return rc;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info vcnl3020_info = {
+	.read_raw = vcnl3020_read_raw,
+};
+
+static const struct regmap_config vcnl3020_regmap_config = {
+	.name		= VCNL_REGMAP_NAME,
+	.reg_bits	= 8,
+	.val_bits	= 8,
+	.max_register	= VCNL_PS_MOD_ADJ,
+};
+
+static int vcnl3020_probe(struct i2c_client *client)
+{
+	struct vcnl3020_data *data;
+	struct iio_dev *indio_dev;
+	struct regmap *regmap;
+	int rc;
+
+	regmap = devm_regmap_init_i2c(client, &vcnl3020_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "regmap_init failed!");
+		return PTR_ERR(regmap);
+	}
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->regmap = regmap;
+	data->dev = &client->dev;
+
+	rc = vcnl3020_init(data);
+	if (rc)
+		return rc;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &vcnl3020_info;
+	indio_dev->channels = vcnl3020_channels;
+	indio_dev->num_channels = ARRAY_SIZE(vcnl3020_channels);
+	indio_dev->name = VCNL_DRV_NAME;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct of_device_id vcnl3020_of_match[] = {
+	{
+		.compatible = "vishay,vcnl3020",
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, vcnl3020_of_match);
+
+static struct i2c_driver vcnl3020_driver = {
+	.driver = {
+		.name   = VCNL_DRV_NAME,
+		.of_match_table = vcnl3020_of_match,
+	},
+	.probe_new  = vcnl3020_probe,
+};
+module_i2c_driver(vcnl3020_driver);
+
+MODULE_AUTHOR("Ivan Mikhaylov <i.mikhaylov@yadro.com>");
+MODULE_DESCRIPTION("Vishay VCNL3020 proximity sensor driver");
+MODULE_LICENSE("GPL");
-- 
2.21.1

Re: [PATCH v5 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor

[Andy Shevchenko]

On Mon, Mar 30, 2020 at 6:27 PM Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote:
>
> Proximity sensor driver based on light/vcnl4000.c code.
> For now supports only the single on-demand measurement.
>
> The VCNL3020 is a fully integrated proximity sensor. Fully
> integrated means that the infrared emitter is included in the
> package. It has 16-bit resolution. It includes a signal
> processing IC and features standard I2C communication
> interface. It features an interrupt function.

Still my tag applies, but
I have few more comments below.

...

> +#define VCNL_DRV_NAME          "vcnl3020"
> +#define VCNL_REGMAP_NAME       "vcnl3020_regmap"

I'm wondering why you need the second one.

...

> +       rc = device_property_read_u32(data->dev, "vishay,led-current-milliamp",
> +                                     &led_current);

> +       if (rc == 0) {
> +               rc = regmap_write(data->regmap, VCNL_LED_CURRENT, led_current);
> +               if (rc)
> +                       dev_err(data->dev,
> +                               "Error (%d) setting LED current", rc);
> +       }
> +
> +       return rc;

Why not to use standard pattern, i.e.

  if (rc)
    return rc;
  ...
  rc = regmap_write(...);

?

...

> +       if (rc) {
> +               dev_err(data->dev,

> +                       "vcnl3020_measure() failed with error (%d)", rc);

Perhaps you keep same pattern for error messages as in previous function(s).

> +               goto err_unlock;
> +       }

> +       rc = regmap_bulk_read(data->regmap, VCNL_PS_RESULT_HI, &res, 2);

sizeof(res)

> +       if (rc)
> +               goto err_unlock;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor

[Ivan Mikhaylov]

On Mon, 2020-03-30 at 22:07 +0300, Andy Shevchenko wrote:
> On Mon, Mar 30, 2020 at 6:27 PM Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote:
> > Proximity sensor driver based on light/vcnl4000.c code.
> > For now supports only the single on-demand measurement.
> > 
> > The VCNL3020 is a fully integrated proximity sensor. Fully
> > integrated means that the infrared emitter is included in the
> > package. It has 16-bit resolution. It includes a signal
> > processing IC and features standard I2C communication
> > interface. It features an interrupt function.
> 
> Still my tag applies, but
> I have few more comments below.
> 
> ...
> 
> > +#define VCNL_DRV_NAME          "vcnl3020"
> > +#define VCNL_REGMAP_NAME       "vcnl3020_regmap"
> 
> I'm wondering why you need the second one.

For regmap initialize as name in 
static const struct regmap_config vcnl3020_regmap_config = {
       .name           = VCNL_REGMAP_NAME,

I can get rid of it from struct with name definition.

> > +       rc = device_property_read_u32(data->dev, "vishay,led-current-
> > milliamp",
> > +                                     &led_current);
> > +       if (rc == 0) {
> > +               rc = regmap_write(data->regmap, VCNL_LED_CURRENT,
> > led_current);
> > +               if (rc)
> > +                       dev_err(data->dev,
> > +                               "Error (%d) setting LED current", rc);
> > +       }
> > +
> > +       return rc;
> 
> Why not to use standard pattern, i.e.
> 
>   if (rc)
>     return rc;
>   ...
>   rc = regmap_write(...);
> 
> ?

Optional parameter. There exists a lot of ways to do it:

rc = device_property_read_u32(dev, "milliamp", &led_current);
rc = regmap_write(regmap, VCNL_LED_CURRENT, (!rc) : led_current ? 0);
or

rc = regmap_write(regmap, VCNL_LED_CURRENT, (!rc) : led_current ?
VCNL_LED_CURRENT_DEFAULT);
or even maybe make a function like get_led_current:

rc = regmap_write(regmap, VCNL_LED_CURRENT, get_led_curent(dev));
where 
static u32 get_led_current(dev) {
	u32 led_current;
	rc = device_property_read_u32(dev, "milliamp", &led_current);
	return (!rc) : led_current ? VCNL_LED_CURRENT_DEFAULT;
}
Which one would be more preferable?

> ...
> 
> > +       if (rc) {
> > +               dev_err(data->dev,
> > +                       "vcnl3020_measure() failed with error (%d)", rc);
> 
> Perhaps you keep same pattern for error messages as in previous function(s).

Sure.

> 
> > +               goto err_unlock;
> > +       }
> > +       rc = regmap_bulk_read(data->regmap, VCNL_PS_RESULT_HI, &res, 2);
> 
> sizeof(res)

Oops.

Re: [PATCH v5 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor

[Andy Shevchenko]

On Mon, Mar 30, 2020 at 11:41 PM Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote:
> On Mon, 2020-03-30 at 22:07 +0300, Andy Shevchenko wrote:
> > On Mon, Mar 30, 2020 at 6:27 PM Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote:

...

> > > +#define VCNL_DRV_NAME          "vcnl3020"
> > > +#define VCNL_REGMAP_NAME       "vcnl3020_regmap"
> >
> > I'm wondering why you need the second one.
>
> For regmap initialize as name in
> static const struct regmap_config vcnl3020_regmap_config = {
>        .name           = VCNL_REGMAP_NAME,
>
> I can get rid of it from struct with name definition.

I don't think we need a specific suffix. When somebody will look at it
they will already know that they are looking into regmap realm.

...

> > > +       rc = device_property_read_u32(data->dev, "vishay,led-current-
> > > milliamp",
> > > +                                     &led_current);
> > > +       if (rc == 0) {
> > > +               rc = regmap_write(data->regmap, VCNL_LED_CURRENT,
> > > led_current);
> > > +               if (rc)
> > > +                       dev_err(data->dev,
> > > +                               "Error (%d) setting LED current", rc);
> > > +       }
> > > +
> > > +       return rc;
> >
> > Why not to use standard pattern, i.e.
> >
> >   if (rc)
> >     return rc;
> >   ...
> >   rc = regmap_write(...);
> >
> > ?
>
> Optional parameter. There exists a lot of ways to do it:

I'm simple reading the code. And I believe the above I suggested is
cleaner equivalent.
Is it?

> rc = device_property_read_u32(dev, "milliamp", &led_current);
> rc = regmap_write(regmap, VCNL_LED_CURRENT, (!rc) : led_current ? 0);

This seems not equal to above.

> Which one would be more preferable?

One which has better readability and smallest indentation level.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor

[Ivan Mikhaylov]

On Tue, 2020-03-31 at 14:00 +0300, Andy Shevchenko wrote:
> > > Why not to use standard pattern, i.e.
> > > 
> > >   if (rc)
> > >     return rc;
> > >   ...
> > >   rc = regmap_write(...);
> > > 
> > > ?
> > 
> > Optional parameter. There exists a lot of ways to do it:
> 
> I'm simple reading the code. And I believe the above I suggested is
> cleaner equivalent.
> Is it?
> 
> > rc = device_property_read_u32(dev, "milliamp", &led_current);
> > rc = regmap_write(regmap, VCNL_LED_CURRENT, (!rc) : led_current ? 0);
> 
> This seems not equal to above.

Yes, it is not equal. Error will be returned in case of non existent parameter
in vcnl3020_init but parameter is optional. rc shouldn't be checked or should
return 0 with your suggestion.

rc = device_property_read_u32(...);
if (rc)
	return 0;
rc = regmap_write(...);
if (rc)
	dev_err(...);
return rc;

Re: [PATCH v5 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor

[Andy Shevchenko]

On Tue, Mar 31, 2020 at 7:56 PM Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote:
> On Tue, 2020-03-31 at 14:00 +0300, Andy Shevchenko wrote:
> > > > Why not to use standard pattern, i.e.
> > > >
> > > >   if (rc)
> > > >     return rc;
> > > >   ...
> > > >   rc = regmap_write(...);
> > > >
> > > > ?
> > >
> > > Optional parameter. There exists a lot of ways to do it:
> >
> > I'm simple reading the code. And I believe the above I suggested is
> > cleaner equivalent.
> > Is it?
> >
> > > rc = device_property_read_u32(dev, "milliamp", &led_current);
> > > rc = regmap_write(regmap, VCNL_LED_CURRENT, (!rc) : led_current ? 0);
> >
> > This seems not equal to above.
>
> Yes, it is not equal. Error will be returned in case of non existent parameter
> in vcnl3020_init but parameter is optional. rc shouldn't be checked

Ah it makes sense.

> or should
> return 0 with your suggestion.
>
> rc = device_property_read_u32(...);
> if (rc)
>         return 0;
> rc = regmap_write(...);
> if (rc)
>         dev_err(...);
> return rc;

In case we would like to have more optional parameters above will be a burden.
Perhaps

static int get_and_apply_property_x(...)
{
  ...Above code...
}

...and in the function

rc = get_and_apply_property_x(...)
if (rc)
  return rc;

or if it's the last one,

return get_and_apply_property_x(...);

-- 
With Best Regards,
Andy Shevchenko