Re: [PATCH 2/5] i2c: gpio: Add support on ACPI-based system

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mika:

在 2022/9/22 20:26, Mika Westerberg 写道:
Hi,

On Thu, Sep 22, 2022 at 07:39:55PM +0800, Binbin Zhou wrote:
Add support for the ACPI-based device registration so that the driver
can be also enabled through ACPI table.

Signed-off-by: Huacai Chen <chenhuacai@xxxxxxxxxxx>
Signed-off-by: Binbin Zhou <zhoubinbin@xxxxxxxxxxx>
---
  drivers/i2c/busses/i2c-gpio.c | 41 ++++++++++++++++++++++++++++++++++-
  1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index b1985c1667e1..ccea37e755e6 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -13,6 +13,7 @@
  #include <linux/init.h>
  #include <linux/interrupt.h>
  #include <linux/module.h>
+#include <linux/acpi.h>
  #include <linux/of.h>
  #include <linux/platform_data/i2c-gpio.h>
  #include <linux/platform_device.h>
@@ -318,6 +319,24 @@ static void of_i2c_gpio_get_props(struct device_node *np,
  		of_property_read_bool(np, "i2c-gpio,scl-output-only");
  }
+static void acpi_i2c_gpio_get_props(struct device *dev,
+				  struct i2c_gpio_platform_data *pdata)
+{
+	u32 reg;
+
+	device_property_read_u32(dev, "delay-us", &pdata->udelay);
+
+	if (!device_property_read_u32(dev, "timeout-ms", &reg))
+		pdata->timeout = msecs_to_jiffies(reg);
+
+	pdata->sda_is_open_drain =
+		device_property_read_bool(dev, "sda-open-drain");
+	pdata->scl_is_open_drain =
+		device_property_read_bool(dev, "scl-open-drain");
+	pdata->scl_is_output_only =
+		device_property_read_bool(dev, "scl-output-only");
+}

I think this would work with the DT description too as it is using
device_property_xxx() so I wonder if you can just do:

	i2c_gpio_get_props(dev, pdata);

instead of

  	if (np) {
  		of_i2c_gpio_get_props(np, pdata);
	} else if (ACPI_COMPANION(dev)) {
		acpi_i2c_gpio_get_props(dev, pdata);

Sorry, I don't quite understand how to do a unified api.

We get the corresponding value by matching the propname, but obviously the propnames related in the two ways are different.

e.g. "delay-us"(ACPI) vs "i2c-gpio, delay-us"(FDT)

I think the judgment of "if..else.." is indispensable.

thanks.

Binbin

+
  static struct gpio_desc *i2c_gpio_get_desc(struct device *dev,
  					   const char *con_id,
  					   unsigned int index,
@@ -363,6 +382,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
  	struct device *dev = &pdev->dev;
  	struct device_node *np = dev->of_node;
  	enum gpiod_flags gflags;
+	acpi_status status;
+	unsigned long long id;
  	int ret;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -375,6 +396,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
if (np) {
  		of_i2c_gpio_get_props(np, pdata);
+	} else if (ACPI_COMPANION(dev)) {
+		acpi_i2c_gpio_get_props(dev, pdata);
  	} else {
  		/*
  		 * If all platform data settings are zero it is OK
@@ -445,7 +468,14 @@ static int i2c_gpio_probe(struct platform_device *pdev)
  	adap->dev.parent = dev;
  	adap->dev.of_node = np;
- adap->nr = pdev->id;
+	if (ACPI_COMPANION(dev)) {
+		status = acpi_evaluate_integer(ACPI_HANDLE(dev),
+						"_UID", NULL, &id);
+		if (ACPI_SUCCESS(status) && (id >= 0))
+			adap->nr = id;

Unrelated change? And if not then same comment about why you need the
static number in the first place ;-)




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux