Linux-i2c Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 0/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land
@ 2020-10-16 22:25 Evan Green
  2020-10-16 22:25 ` [PATCH v3 1/2] i2c: i2c-mux-gpio: Factor out pdev->dev in _probe_dt() Evan Green
  2020-10-16 22:25 ` [PATCH v3 2/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land Evan Green
  0 siblings, 2 replies; 6+ messages in thread
From: Evan Green @ 2020-10-16 22:25 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Wolfram Sang, Randy Dunlap, Evan Green, Peter Korsgaard,
	linux-i2c, linux-kernel

The i2c-mux-gpio driver is a handy driver to have in your bag of tricks,
but it currently only works with DT-based firmware. Enable this driver
on ACPI platforms as well.

The first patch is a little dinky. Peter, if it turns out you'd rather
just take this all as a single patch, feel free to squash the first
patch into the second. Or I can resend a squashed patch if needed.

Changes in v3:
 - Introduced minor &pdev->dev to dev refactor (Peter)
 - Update commit message again (Peter)
 - Added missing \n (Peter)
 - adr64 overflow check (Peter)
 - Don't initialize child (Peter)
 - Limit scope of dev_handle (Peter)

Changes in v2:
 - Make it compile properly when !CONFIG_ACPI (Randy)
 - Update commit message regarding i2c-parent (Peter)

Evan Green (2):
  i2c: i2c-mux-gpio: Factor out pdev->dev in _probe_dt()
  i2c: i2c-mux-gpio: Enable this driver in ACPI land

 drivers/i2c/muxes/i2c-mux-gpio.c | 112 ++++++++++++++++++++++---------
 1 file changed, 82 insertions(+), 30 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] i2c: i2c-mux-gpio: Factor out pdev->dev in _probe_dt()
  2020-10-16 22:25 [PATCH v3 0/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land Evan Green
@ 2020-10-16 22:25 ` Evan Green
  2020-10-16 22:25 ` [PATCH v3 2/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land Evan Green
  1 sibling, 0 replies; 6+ messages in thread
From: Evan Green @ 2020-10-16 22:25 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Wolfram Sang, Randy Dunlap, Evan Green, Peter Korsgaard,
	linux-i2c, linux-kernel

Factor out &pdev->dev into a local variable in preparation for
the ACPI enablement of this function, which will utilize the variable
more.

Signed-off-by: Evan Green <evgreen@chromium.org>
---

Changes in v3:
 - Introduced minor &pdev->dev to dev refactor (Peter)

 drivers/i2c/muxes/i2c-mux-gpio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 4effe563e9e8d..caaa782b50d83 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -53,6 +53,7 @@ static int i2c_mux_gpio_deselect(struct i2c_mux_core *muxc, u32 chan)
 static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
 					struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *adapter_np, *child;
 	struct i2c_adapter *adapter;
@@ -77,11 +78,11 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
 
 	mux->data.n_values = of_get_child_count(np);
 
-	values = devm_kcalloc(&pdev->dev,
+	values = devm_kcalloc(dev,
 			      mux->data.n_values, sizeof(*mux->data.values),
 			      GFP_KERNEL);
 	if (!values) {
-		dev_err(&pdev->dev, "Cannot allocate values array");
+		dev_err(dev, "Cannot allocate values array");
 		return -ENOMEM;
 	}
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land
  2020-10-16 22:25 [PATCH v3 0/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land Evan Green
  2020-10-16 22:25 ` [PATCH v3 1/2] i2c: i2c-mux-gpio: Factor out pdev->dev in _probe_dt() Evan Green
@ 2020-10-16 22:25 ` Evan Green
  2020-10-18 18:58   ` Andy Shevchenko
  1 sibling, 1 reply; 6+ messages in thread
From: Evan Green @ 2020-10-16 22:25 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Wolfram Sang, Randy Dunlap, Evan Green, Peter Korsgaard,
	linux-i2c, linux-kernel

Enable i2c-mux-gpio devices to be defined via ACPI. The idle-state
property translates directly to a fwnode_property_*() call. The child
reg property translates naturally into _ADR in ACPI.

The i2c-parent binding is a relic from the days when the bindings
dictated that all direct children of an I2C controller had to be I2C
devices. These days that's no longer required. The i2c-mux can sit as a
direct child of its parent controller, which is where it makes the most
sense from a hardware description perspective. For the ACPI
implementation we'll assume that's always how the i2c-mux-gpio is
instantiated.

Signed-off-by: Evan Green <evgreen@chromium.org>
---

Changes in v3:
 - Update commit message again (Peter)
 - Added missing \n (Peter)
 - adr64 overflow check (Peter)
 - Don't initialize child (Peter)
 - Limit scope of dev_handle (Peter)

Changes in v2:
 - Make it compile properly when !CONFIG_ACPI (Randy)
 - Update commit message regarding i2c-parent (Peter)

 drivers/i2c/muxes/i2c-mux-gpio.c | 107 +++++++++++++++++++++++--------
 1 file changed, 79 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index caaa782b50d83..bac415a52b780 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -49,35 +49,85 @@ static int i2c_mux_gpio_deselect(struct i2c_mux_core *muxc, u32 chan)
 	return 0;
 }
 
-#ifdef CONFIG_OF
-static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
-					struct platform_device *pdev)
+#ifdef CONFIG_ACPI
+
+static int i2c_mux_gpio_get_acpi_adr(struct device *dev,
+				     struct fwnode_handle *fwdev,
+				     unsigned int *adr)
+
+{
+	unsigned long long adr64;
+	acpi_status status;
+
+	status = acpi_evaluate_integer(ACPI_HANDLE_FWNODE(fwdev),
+				       METHOD_NAME__ADR,
+				       NULL, &adr64);
+
+	if (!ACPI_SUCCESS(status)) {
+		dev_err(dev, "Cannot get address\n");
+		return -EINVAL;
+	}
+
+	*adr = adr64;
+	if (*adr != adr64) {
+		dev_err(dev, "Address out of range\n");
+		return -ERANGE;
+	}
+
+	return 0;
+}
+
+#else
+
+static int i2c_mux_gpio_get_acpi_adr(struct device *dev,
+				     struct fwnode_handle *fwdev,
+				     unsigned int *adr)
+{
+	return -EINVAL;
+}
+
+#endif
+
+static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
+				 struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *np = pdev->dev.of_node;
-	struct device_node *adapter_np, *child;
-	struct i2c_adapter *adapter;
+	struct device_node *np = dev->of_node;
+	struct device_node *adapter_np;
+	struct i2c_adapter *adapter = NULL;
+	struct fwnode_handle *child;
 	unsigned *values;
-	int i = 0;
+	int rc, i = 0;
+
+	if (is_of_node(dev->fwnode)) {
+		if (!np)
+			return -ENODEV;
 
-	if (!np)
-		return -ENODEV;
+		adapter_np = of_parse_phandle(np, "i2c-parent", 0);
+		if (!adapter_np) {
+			dev_err(&pdev->dev, "Cannot parse i2c-parent\n");
+			return -ENODEV;
+		}
+		adapter = of_find_i2c_adapter_by_node(adapter_np);
+		of_node_put(adapter_np);
+
+	} else if (is_acpi_node(dev->fwnode)) {
+		/*
+		 * In ACPI land the mux should be a direct child of the i2c
+		 * bus it muxes.
+		 */
+		acpi_handle dev_handle = ACPI_HANDLE(dev->parent);
 
-	adapter_np = of_parse_phandle(np, "i2c-parent", 0);
-	if (!adapter_np) {
-		dev_err(&pdev->dev, "Cannot parse i2c-parent\n");
-		return -ENODEV;
+		adapter = i2c_acpi_find_adapter_by_handle(dev_handle);
 	}
-	adapter = of_find_i2c_adapter_by_node(adapter_np);
-	of_node_put(adapter_np);
+
 	if (!adapter)
 		return -EPROBE_DEFER;
 
 	mux->data.parent = i2c_adapter_id(adapter);
 	put_device(&adapter->dev);
 
-	mux->data.n_values = of_get_child_count(np);
-
+	mux->data.n_values = device_get_child_node_count(dev);
 	values = devm_kcalloc(dev,
 			      mux->data.n_values, sizeof(*mux->data.values),
 			      GFP_KERNEL);
@@ -86,24 +136,25 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
 		return -ENOMEM;
 	}
 
-	for_each_child_of_node(np, child) {
-		of_property_read_u32(child, "reg", values + i);
+	device_for_each_child_node(dev, child) {
+		if (is_of_node(child)) {
+			fwnode_property_read_u32(child, "reg", values + i);
+
+		} else if (is_acpi_node(child)) {
+			rc = i2c_mux_gpio_get_acpi_adr(dev, child, values + i);
+			if (rc)
+				return rc;
+		}
+
 		i++;
 	}
 	mux->data.values = values;
 
-	if (of_property_read_u32(np, "idle-state", &mux->data.idle))
+	if (fwnode_property_read_u32(dev->fwnode, "idle-state", &mux->data.idle))
 		mux->data.idle = I2C_MUX_GPIO_NO_IDLE;
 
 	return 0;
 }
-#else
-static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
-					struct platform_device *pdev)
-{
-	return 0;
-}
-#endif
 
 static int i2c_mux_gpio_probe(struct platform_device *pdev)
 {
@@ -119,7 +170,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	if (!dev_get_platdata(&pdev->dev)) {
-		ret = i2c_mux_gpio_probe_dt(mux, pdev);
+		ret = i2c_mux_gpio_probe_fw(mux, pdev);
 		if (ret < 0)
 			return ret;
 	} else {
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land
  2020-10-16 22:25 ` [PATCH v3 2/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land Evan Green
@ 2020-10-18 18:58   ` Andy Shevchenko
  2020-10-19 16:53     ` Evan Green
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2020-10-18 18:58 UTC (permalink / raw)
  To: Evan Green
  Cc: Peter Rosin, Wolfram Sang, Randy Dunlap, Peter Korsgaard,
	linux-i2c, Linux Kernel Mailing List

On Sat, Oct 17, 2020 at 8:30 AM Evan Green <evgreen@chromium.org> wrote:
>
> Enable i2c-mux-gpio devices to be defined via ACPI. The idle-state
> property translates directly to a fwnode_property_*() call. The child
> reg property translates naturally into _ADR in ACPI.
>
> The i2c-parent binding is a relic from the days when the bindings
> dictated that all direct children of an I2C controller had to be I2C
> devices. These days that's no longer required. The i2c-mux can sit as a
> direct child of its parent controller, which is where it makes the most
> sense from a hardware description perspective. For the ACPI
> implementation we'll assume that's always how the i2c-mux-gpio is
> instantiated.

Can you tell me if the following is relevant to what you are looking for?
https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-mux.c#L393

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land
  2020-10-18 18:58   ` Andy Shevchenko
@ 2020-10-19 16:53     ` Evan Green
  2020-10-27 23:00       ` Evan Green
  0 siblings, 1 reply; 6+ messages in thread
From: Evan Green @ 2020-10-19 16:53 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Peter Rosin, Wolfram Sang, Randy Dunlap, Peter Korsgaard,
	linux-i2c, Linux Kernel Mailing List

On Sun, Oct 18, 2020 at 11:58 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Sat, Oct 17, 2020 at 8:30 AM Evan Green <evgreen@chromium.org> wrote:
> >
> > Enable i2c-mux-gpio devices to be defined via ACPI. The idle-state
> > property translates directly to a fwnode_property_*() call. The child
> > reg property translates naturally into _ADR in ACPI.
> >
> > The i2c-parent binding is a relic from the days when the bindings
> > dictated that all direct children of an I2C controller had to be I2C
> > devices. These days that's no longer required. The i2c-mux can sit as a
> > direct child of its parent controller, which is where it makes the most
> > sense from a hardware description perspective. For the ACPI
> > implementation we'll assume that's always how the i2c-mux-gpio is
> > instantiated.
>
> Can you tell me if the following is relevant to what you are looking for?
> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-mux.c#L393

I don't think so, but let me know if I'm reading between the lines incorrectly.

The code you pointed to links the newly-minted fake i2c controller
back together with its ACPI node. This is important, since I think
that's how child I2C devices underneath the fake busses get populated
in ACPI land. But the paragraph above is discussing how to identify
the parent adapter (ie the real hardware) for an i2c-mux-gpio device.

In DT-land, the i2c-mux-gpio floats at the top of the tree directly
under /, and then uses a phandle to point to where transactions should
be forwarded. I'm told the reason for this is historical limitations
with the DT bindings. Rather than trying to translate the phandle over
1:1 into ACPI-land, I'm asserting that the mux device should live
underneath the adapter it wants to forward traffic to.

-Evan

>
> --
> With Best Regards,
> Andy Shevchenko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land
  2020-10-19 16:53     ` Evan Green
@ 2020-10-27 23:00       ` Evan Green
  0 siblings, 0 replies; 6+ messages in thread
From: Evan Green @ 2020-10-27 23:00 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Peter Rosin, Wolfram Sang, Randy Dunlap, Peter Korsgaard,
	linux-i2c, Linux Kernel Mailing List

On Mon, Oct 19, 2020 at 9:53 AM Evan Green <evgreen@chromium.org> wrote:
>
> On Sun, Oct 18, 2020 at 11:58 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Sat, Oct 17, 2020 at 8:30 AM Evan Green <evgreen@chromium.org> wrote:
> > >
> > > Enable i2c-mux-gpio devices to be defined via ACPI. The idle-state
> > > property translates directly to a fwnode_property_*() call. The child
> > > reg property translates naturally into _ADR in ACPI.
> > >
> > > The i2c-parent binding is a relic from the days when the bindings
> > > dictated that all direct children of an I2C controller had to be I2C
> > > devices. These days that's no longer required. The i2c-mux can sit as a
> > > direct child of its parent controller, which is where it makes the most
> > > sense from a hardware description perspective. For the ACPI
> > > implementation we'll assume that's always how the i2c-mux-gpio is
> > > instantiated.
> >
> > Can you tell me if the following is relevant to what you are looking for?
> > https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-mux.c#L393
>
> I don't think so, but let me know if I'm reading between the lines incorrectly.
>
> The code you pointed to links the newly-minted fake i2c controller
> back together with its ACPI node. This is important, since I think
> that's how child I2C devices underneath the fake busses get populated
> in ACPI land. But the paragraph above is discussing how to identify
> the parent adapter (ie the real hardware) for an i2c-mux-gpio device.
>
> In DT-land, the i2c-mux-gpio floats at the top of the tree directly
> under /, and then uses a phandle to point to where transactions should
> be forwarded. I'm told the reason for this is historical limitations
> with the DT bindings. Rather than trying to translate the phandle over
> 1:1 into ACPI-land, I'm asserting that the mux device should live
> underneath the adapter it wants to forward traffic to.

Andy or Peter, Any other thoughts on this series?
-Evan

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-16 22:25 [PATCH v3 0/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land Evan Green
2020-10-16 22:25 ` [PATCH v3 1/2] i2c: i2c-mux-gpio: Factor out pdev->dev in _probe_dt() Evan Green
2020-10-16 22:25 ` [PATCH v3 2/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land Evan Green
2020-10-18 18:58   ` Andy Shevchenko
2020-10-19 16:53     ` Evan Green
2020-10-27 23:00       ` Evan Green

Linux-i2c Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-i2c/0 linux-i2c/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-i2c linux-i2c/ https://lore.kernel.org/linux-i2c \
		linux-i2c@vger.kernel.org
	public-inbox-index linux-i2c

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-i2c


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git