[PATCH 0/2] Add SMBus features to Tegra I2C

[PATCH 0/2] Add SMBus features to Tegra I2C

[Akhil R]

Add support for SMBus Alert and SMBus block read functions to
i2c-tegra driver

Akhil R (2):
  dt-bindings: i2c: tegra: Add SMBus feature properties
  i2c: tegra: Add SMBus block read and SMBus alert functions

 .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  4 ++
 drivers/i2c/busses/i2c-tegra.c                     | 54 +++++++++++++++++++++-
 2 files changed, 57 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH 1/2] dt-bindings: i2c: tegra: Add SMBus feature properties

[Akhil R]

Tegra I2C can use a gpio as an smbus-alert. Document the usage of
the same.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
index 3f2f990..71ee79b 100644
--- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
@@ -70,6 +70,10 @@ Required properties:
   - rx
   - tx
 
+optional properties:
+- smbalert-gpio: Must contain an entry for the gpio to be used as smbus alert.
+  It will be used only if optional smbus-alert property is present.
+
 Example:
 
 	i2c@7000c000 {
-- 
2.7.4

[PATCH 2/2] i2c: tegra: Add SMBus block read and SMBus alert functions

[Akhil R]

Emulate the SMBus block read using ContinueXfer and SMBus using GPIO
interrupt.

For SMBus block read, the driver  reads the first byte with ContinueXfer
set which will help to parse the data count and read the remaining bytes
without stop condition in between.
SMBus alert is implemented using external gpio interrupt.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 54 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index a5be8f0..3b70013 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -14,6 +14,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -226,6 +227,11 @@ struct tegra_i2c_hw_feature {
 	bool has_interface_timing_reg;
 };
 
+struct tegra_i2c_smbalert {
+	struct i2c_smbus_alert_setup alert_data;
+	struct i2c_client *ara;
+};
+
 /**
  * struct tegra_i2c_dev - per device I2C context
  * @dev: device reference for power management
@@ -280,6 +286,8 @@ struct tegra_i2c_dev {
 	int msg_err;
 	u8 *msg_buf;
 
+	struct tegra_i2c_smbalert smbalert;
+
 	struct completion dma_complete;
 	struct dma_chan *tx_dma_chan;
 	struct dma_chan *rx_dma_chan;
@@ -1232,6 +1240,11 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 		return err;
 
 	i2c_dev->msg_buf = msg->buf;
+
+	/* The condition true implies smbus block read and len is already read*/
+	if (msg->flags & I2C_M_RECV_LEN && end_state != MSG_END_CONTINUE)
+		i2c_dev->msg_buf = msg->buf + 1;
+
 	i2c_dev->msg_buf_remaining = msg->len;
 	i2c_dev->msg_err = I2C_ERR_NONE;
 	i2c_dev->msg_read = !!(msg->flags & I2C_M_RD);
@@ -1388,6 +1401,15 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 			else
 				end_type = MSG_END_REPEAT_START;
 		}
+		/* If M_RECV_LEN use ContinueXfer to read the first byte */
+		if (msgs[i].flags & I2C_M_RECV_LEN) {
+			ret = tegra_i2c_xfer_msg(i2c_dev, &msgs[i], MSG_END_CONTINUE);
+			if (ret)
+				break;
+			/* Set the read byte as msg len */
+			msgs[i].len = msgs[i].buf[0];
+			dev_dbg(i2c_dev->dev, "reading %d bytes\n", msgs[i].len);
+		}
 		ret = tegra_i2c_xfer_msg(i2c_dev, &msgs[i], end_type);
 		if (ret)
 			break;
@@ -1415,7 +1437,8 @@ static u32 tegra_i2c_func(struct i2c_adapter *adap)
 {
 	struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
 	u32 ret = I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK) |
-		  I2C_FUNC_10BIT_ADDR |	I2C_FUNC_PROTOCOL_MANGLING;
+		  I2C_FUNC_SMBUS_READ_BLOCK_DATA | I2C_FUNC_10BIT_ADDR |
+		  I2C_FUNC_PROTOCOL_MANGLING;
 
 	if (i2c_dev->hw->has_continue_xfer_support)
 		ret |= I2C_FUNC_NOSTART;
@@ -1727,6 +1750,29 @@ static int tegra_i2c_init_hardware(struct tegra_i2c_dev *i2c_dev)
 	return ret;
 }
 
+static int tegra_i2c_setup_smbalert(struct tegra_i2c_dev *i2c_dev)
+{
+	struct tegra_i2c_smbalert *smbalert = &i2c_dev->smbalert;
+	struct gpio_desc *alert_gpiod;
+	struct i2c_client *ara;
+
+	alert_gpiod = devm_gpiod_get(i2c_dev->dev, "smbalert", GPIOD_IN);
+	if (IS_ERR(alert_gpiod))
+		return PTR_ERR(alert_gpiod);
+
+	smbalert->alert_data.irq = gpiod_to_irq(alert_gpiod);
+	if (smbalert->alert_data.irq <= 0)
+		return smbalert->alert_data.irq;
+
+	ara = i2c_new_smbus_alert_device(&i2c_dev->adapter, &smbalert->alert_data);
+	if (IS_ERR(ara))
+		return PTR_ERR(ara);
+
+	smbalert->ara = ara;
+
+	return 0;
+}
+
 static int tegra_i2c_probe(struct platform_device *pdev)
 {
 	struct tegra_i2c_dev *i2c_dev;
@@ -1821,6 +1867,12 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	if (err)
 		goto release_rpm;
 
+	if (device_property_read_bool(i2c_dev->dev, "smbus-alert")) {
+		err = tegra_i2c_setup_smbalert(i2c_dev);
+		if (err)
+			dev_warn(&pdev->dev, "smbus-alert setup failed: %d\n", err);
+	}
+
 	return 0;
 
 release_rpm:
-- 
2.7.4

Re: [PATCH 2/2] i2c: tegra: Add SMBus block read and SMBus alert functions

[Dmitry Osipenko]

09.12.2021 18:05, Akhil R пишет:
> Emulate the SMBus block read using ContinueXfer and SMBus using GPIO
> interrupt.
> 
> For SMBus block read, the driver  reads the first byte with ContinueXfer
> set which will help to parse the data count and read the remaining bytes
> without stop condition in between.
> SMBus alert is implemented using external gpio interrupt.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 54 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index a5be8f0..3b70013 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -14,6 +14,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/err.h>
>  #include <linux/i2c.h>
> +#include <linux/i2c-smbus.h>
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -226,6 +227,11 @@ struct tegra_i2c_hw_feature {
>  	bool has_interface_timing_reg;
>  };
>  
> +struct tegra_i2c_smbalert {

smbalert isn't a word, should be smbus_alert. Same for the GPIO name and
other places.

> +	struct i2c_smbus_alert_setup alert_data;
> +	struct i2c_client *ara;

What "ara" stands for? Please use meaningful names, like alert_dev for
example.

I don't see where this member is used at all, please remove it.

> +};
> +
>  /**
>   * struct tegra_i2c_dev - per device I2C context
>   * @dev: device reference for power management
> @@ -280,6 +286,8 @@ struct tegra_i2c_dev {
>  	int msg_err;
>  	u8 *msg_buf;
>  
> +	struct tegra_i2c_smbalert smbalert;

All properties must have doc comment.

>  	struct completion dma_complete;
>  	struct dma_chan *tx_dma_chan;
>  	struct dma_chan *rx_dma_chan;
> @@ -1232,6 +1240,11 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
>  		return err;
>  
>  	i2c_dev->msg_buf = msg->buf;
> +
> +	/* The condition true implies smbus block read and len is already read*/

Proper SMBus capitalization in comments. Mussing whitespace in the end
of the comment.

> +	if (msg->flags & I2C_M_RECV_LEN && end_state != MSG_END_CONTINUE)
> +		i2c_dev->msg_buf = msg->buf + 1;
> +
>  	i2c_dev->msg_buf_remaining = msg->len;
>  	i2c_dev->msg_err = I2C_ERR_NONE;
>  	i2c_dev->msg_read = !!(msg->flags & I2C_M_RD);
> @@ -1388,6 +1401,15 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
>  			else
>  				end_type = MSG_END_REPEAT_START;
>  		}
> +		/* If M_RECV_LEN use ContinueXfer to read the first byte */
> +		if (msgs[i].flags & I2C_M_RECV_LEN) {
> +			ret = tegra_i2c_xfer_msg(i2c_dev, &msgs[i], MSG_END_CONTINUE);
> +			if (ret)
> +				break;
> +			/* Set the read byte as msg len */
> +			msgs[i].len = msgs[i].buf[0];
> +			dev_dbg(i2c_dev->dev, "reading %d bytes\n", msgs[i].len);
> +		}
>  		ret = tegra_i2c_xfer_msg(i2c_dev, &msgs[i], end_type);
>  		if (ret)
>  			break;
> @@ -1415,7 +1437,8 @@ static u32 tegra_i2c_func(struct i2c_adapter *adap)
>  {
>  	struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
>  	u32 ret = I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK) |
> -		  I2C_FUNC_10BIT_ADDR |	I2C_FUNC_PROTOCOL_MANGLING;
> +		  I2C_FUNC_SMBUS_READ_BLOCK_DATA | I2C_FUNC_10BIT_ADDR |
> +		  I2C_FUNC_PROTOCOL_MANGLING;
>  
>  	if (i2c_dev->hw->has_continue_xfer_support)
>  		ret |= I2C_FUNC_NOSTART;
> @@ -1727,6 +1750,29 @@ static int tegra_i2c_init_hardware(struct tegra_i2c_dev *i2c_dev)
>  	return ret;
>  }
>  
> +static int tegra_i2c_setup_smbalert(struct tegra_i2c_dev *i2c_dev)
> +{
> +	struct tegra_i2c_smbalert *smbalert = &i2c_dev->smbalert;
> +	struct gpio_desc *alert_gpiod;
> +	struct i2c_client *ara;
> +
> +	alert_gpiod = devm_gpiod_get(i2c_dev->dev, "smbalert", GPIOD_IN);
> +	if (IS_ERR(alert_gpiod))
> +		return PTR_ERR(alert_gpiod);
> +
> +	smbalert->alert_data.irq = gpiod_to_irq(alert_gpiod);
> +	if (smbalert->alert_data.irq <= 0)
> +		return smbalert->alert_data.irq;
> +
> +	ara = i2c_new_smbus_alert_device(&i2c_dev->adapter, &smbalert->alert_data);
> +	if (IS_ERR(ara))
> +		return PTR_ERR(ara);
> +
> +	smbalert->ara = ara;
> +
> +	return 0;
> +}
> +
>  static int tegra_i2c_probe(struct platform_device *pdev)
>  {
>  	struct tegra_i2c_dev *i2c_dev;
> @@ -1821,6 +1867,12 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>  	if (err)
>  		goto release_rpm;
>  
> +	if (device_property_read_bool(i2c_dev->dev, "smbus-alert")) {

I'd move this device_property_read_bool() inside of
tegra_i2c_setup_smbus_alert(), for consistency with the rest of the code
in this driver.

Although, you shouldn't need it at all, use devm_gpiod_get_optional().

> +		err = tegra_i2c_setup_smbalert(i2c_dev);
> +		if (err)
> +			dev_warn(&pdev->dev, "smbus-alert setup failed: %d\n", err);

GPIO may probe-defer, must be dev_err_probe() here.

Re: [PATCH 2/2] i2c: tegra: Add SMBus block read and SMBus alert functions

[Dmitry Osipenko]

09.12.2021 18:05, Akhil R пишет:
> +static int tegra_i2c_setup_smbalert(struct tegra_i2c_dev *i2c_dev)
> +{
> +	struct tegra_i2c_smbalert *smbalert = &i2c_dev->smbalert;
> +	struct gpio_desc *alert_gpiod;
> +	struct i2c_client *ara;
> +
> +	alert_gpiod = devm_gpiod_get(i2c_dev->dev, "smbalert", GPIOD_IN);
> +	if (IS_ERR(alert_gpiod))
> +		return PTR_ERR(alert_gpiod);
> +
> +	smbalert->alert_data.irq = gpiod_to_irq(alert_gpiod);
> +	if (smbalert->alert_data.irq <= 0)
> +		return smbalert->alert_data.irq;

0 is the error condition.

Re: [PATCH 1/2] dt-bindings: i2c: tegra: Add SMBus feature properties

[Andy Shevchenko]

On Thu, Dec 9, 2021 at 5:05 PM Akhil R <akhilrajeev@nvidia.com> wrote:
>
> Tegra I2C can use a gpio as an smbus-alert. Document the usage of
> the same.

...

> +optional properties:

Optional

> +- smbalert-gpio: Must contain an entry for the gpio to be used as smbus alert.

-gpios (the suffix in plural form, the singular is a legacy one)

> +  It will be used only if optional smbus-alert property is present.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] i2c: tegra: Add SMBus block read and SMBus alert functions

[Andy Shevchenko]

On Thu, Dec 9, 2021 at 5:30 PM Dmitry Osipenko <digetx@gmail.com> wrote:
> 09.12.2021 18:05, Akhil R пишет:
> > +static int tegra_i2c_setup_smbalert(struct tegra_i2c_dev *i2c_dev)
> > +{
> > +     struct tegra_i2c_smbalert *smbalert = &i2c_dev->smbalert;
> > +     struct gpio_desc *alert_gpiod;
> > +     struct i2c_client *ara;
> > +
> > +     alert_gpiod = devm_gpiod_get(i2c_dev->dev, "smbalert", GPIOD_IN);
> > +     if (IS_ERR(alert_gpiod))
> > +             return PTR_ERR(alert_gpiod);
> > +
> > +     smbalert->alert_data.irq = gpiod_to_irq(alert_gpiod);
> > +     if (smbalert->alert_data.irq <= 0)
> > +             return smbalert->alert_data.irq;
>
> 0 is the error condition.

I'm not sure what you implied here. gpiod_to_irq() returns 0 if and
only if it goes to the architectures where it might be possible to
have valid vIRQ 0, but this is not the case (at least I never heard of
a such) for GPIO controllers on such platforms. So, looking at the
above code I may tell that the '=' part is redundant.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] i2c: tegra: Add SMBus block read and SMBus alert functions

[Dmitry Osipenko]

09.12.2021 18:33, Andy Shevchenko пишет:
> On Thu, Dec 9, 2021 at 5:30 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>> 09.12.2021 18:05, Akhil R пишет:
>>> +static int tegra_i2c_setup_smbalert(struct tegra_i2c_dev *i2c_dev)
>>> +{
>>> +     struct tegra_i2c_smbalert *smbalert = &i2c_dev->smbalert;
>>> +     struct gpio_desc *alert_gpiod;
>>> +     struct i2c_client *ara;
>>> +
>>> +     alert_gpiod = devm_gpiod_get(i2c_dev->dev, "smbalert", GPIOD_IN);
>>> +     if (IS_ERR(alert_gpiod))
>>> +             return PTR_ERR(alert_gpiod);
>>> +
>>> +     smbalert->alert_data.irq = gpiod_to_irq(alert_gpiod);
>>> +     if (smbalert->alert_data.irq <= 0)
>>> +             return smbalert->alert_data.irq;
>>
>> 0 is the error condition.
> 
> I'm not sure what you implied here. gpiod_to_irq() returns 0 if and
> only if it goes to the architectures where it might be possible to
> have valid vIRQ 0, but this is not the case (at least I never heard of
> a such) for GPIO controllers on such platforms. So, looking at the
> above code I may tell that the '=' part is redundant.
> 

Yes, removal of the '=' should be enough here.

Re: [PATCH 2/2] i2c: tegra: Add SMBus block read and SMBus alert functions

[Dmitry Osipenko]

09.12.2021 18:05, Akhil R пишет:
> +static int tegra_i2c_setup_smbalert(struct tegra_i2c_dev *i2c_dev)
> +{
> +	struct tegra_i2c_smbalert *smbalert = &i2c_dev->smbalert;
> +	struct gpio_desc *alert_gpiod;
> +	struct i2c_client *ara;
> +
> +	alert_gpiod = devm_gpiod_get(i2c_dev->dev, "smbalert", GPIOD_IN);
> +	if (IS_ERR(alert_gpiod))
> +		return PTR_ERR(alert_gpiod);
> +
> +	smbalert->alert_data.irq = gpiod_to_irq(alert_gpiod);
> +	if (smbalert->alert_data.irq <= 0)
> +		return smbalert->alert_data.irq;

Why GPIO is needed at all if what's actually needed is the interrupt?

Re: [PATCH 0/2] Add SMBus features to Tegra I2C

[Dmitry Osipenko]

09.12.2021 18:05, Akhil R пишет:
> Add support for SMBus Alert and SMBus block read functions to
> i2c-tegra driver
> 
> Akhil R (2):
>   dt-bindings: i2c: tegra: Add SMBus feature properties
>   i2c: tegra: Add SMBus block read and SMBus alert functions
> 
>  .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  4 ++
>  drivers/i2c/busses/i2c-tegra.c                     | 54 +++++++++++++++++++++-
>  2 files changed, 57 insertions(+), 1 deletion(-)
> 

How this was tested? This series must include the DT patch. If there is
no real user in upstream for this feature, then I don't think that we
should bother at all about it.

Re: [PATCH 0/2] Add SMBus features to Tegra I2C

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]

On Thu, Dec 09, 2021 at 07:04:30PM +0300, Dmitry Osipenko wrote:
> 09.12.2021 18:05, Akhil R пишет:
> > Add support for SMBus Alert and SMBus block read functions to
> > i2c-tegra driver
> > 
> > Akhil R (2):
> >   dt-bindings: i2c: tegra: Add SMBus feature properties
> >   i2c: tegra: Add SMBus block read and SMBus alert functions
> > 
> >  .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  4 ++
> >  drivers/i2c/busses/i2c-tegra.c                     | 54 +++++++++++++++++++++-
> >  2 files changed, 57 insertions(+), 1 deletion(-)
> > 
> 
> How this was tested? This series must include the DT patch. If there is
> no real user in upstream for this feature, then I don't think that we
> should bother at all about it.

This is primarily used by a device that uses ACPI and the driver uses
the firmware-agnostic APIs to get at this. However, it also means that
the driver effectively provides this same support for DT via those APIs
and therefore it makes sense to document that part even if there are no
current users of the DT bits.

One big advantage of this is that it helps keep the ACPI and DT bindings
in sync, and document this on the DT side also allows us to document the
ACPI side of things where no formal documentation exists, as far as I
know.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] dt-bindings: i2c: tegra: Add SMBus feature properties

[Rob Herring]

On Thu, Dec 09, 2021 at 08:35:20PM +0530, Akhil R wrote:
> Tegra I2C can use a gpio as an smbus-alert. Document the usage of
> the same.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
> index 3f2f990..71ee79b 100644
> --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
> @@ -70,6 +70,10 @@ Required properties:
>    - rx
>    - tx
>  
> +optional properties:
> +- smbalert-gpio: Must contain an entry for the gpio to be used as smbus alert.
> +  It will be used only if optional smbus-alert property is present.

There's already a standard way to do this with interrupts. And GPIOs can 
be interrupts usually.

> +
>  Example:
>  
>  	i2c@7000c000 {
> -- 
> 2.7.4
> 
> 

Re: [PATCH 0/2] Add SMBus features to Tegra I2C

[Rob Herring]

On Fri, Dec 10, 2021 at 09:56:28AM +0100, Thierry Reding wrote:
> On Thu, Dec 09, 2021 at 07:04:30PM +0300, Dmitry Osipenko wrote:
> > 09.12.2021 18:05, Akhil R пишет:
> > > Add support for SMBus Alert and SMBus block read functions to
> > > i2c-tegra driver
> > > 
> > > Akhil R (2):
> > >   dt-bindings: i2c: tegra: Add SMBus feature properties
> > >   i2c: tegra: Add SMBus block read and SMBus alert functions
> > > 
> > >  .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  4 ++
> > >  drivers/i2c/busses/i2c-tegra.c                     | 54 +++++++++++++++++++++-
> > >  2 files changed, 57 insertions(+), 1 deletion(-)
> > > 
> > 
> > How this was tested? This series must include the DT patch. If there is
> > no real user in upstream for this feature, then I don't think that we
> > should bother at all about it.
> 
> This is primarily used by a device that uses ACPI and the driver uses
> the firmware-agnostic APIs to get at this. However, it also means that
> the driver effectively provides this same support for DT via those APIs
> and therefore it makes sense to document that part even if there are no
> current users of the DT bits.

Then definitely a NAK.

> One big advantage of this is that it helps keep the ACPI and DT bindings
> in sync, and document this on the DT side also allows us to document the
> ACPI side of things where no formal documentation exists, as far as I
> know.

I have no bandwidth to review ACPI bindings and don't think the whole 
use DT bindings in ACPI is a good idea either. If someone wants this to 
be a thing, then they need to step up and review bindings.

Rob