Linux-i3c Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 0/5] i3c: detach/free device fail pre_assign_dyn_addr()
@ 2019-09-05 10:00 Vitor Soares
  2019-09-05 10:00 ` [PATCH v3 1/5] i3c: master: detach/free devices fail on pre_assign_dyn_addr() Vitor Soares
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Vitor Soares @ 2019-09-05 10:00 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-i3c
  Cc: mark.rutland, Joao.Pinto, bbrezillon, Vitor Soares, pgaj, robh+dt

As for today, the I3C framework is keeping in memory and master->bus.devs
list the devices that fail during pre_assign_dyn_addr() and send them on
DEFSLVS command.

According to MIPI I3C Bus spec the DEFSLVS command is used to inform any
Secondary Master about the Dynamic Addresses that were assigned to I3C
devices and the I2C devices present on the bus.

This issue could be fixed by changing i3c_master_defslvs_locked() to
ignore unaddressed i3c devices but the i3c_dev_desc would be allocated and
attached to HC unnecessarily. This can cause that some HC aren't able to
do DAA for HJ capable devices due of lack of space.

This patch-series propose to detach/free devices that are failing during
pre_assign_dyn_addr() and to propagate i3c_boardinfo, if available, to
i3c_dev_desc during i3c_master_add_i3c_dev_locked(). Besides the fix for
the problem mention above, this change will permit to describe devices
with a preferable dynamic address (important due to priority reason) but
without a static address in DT.

In addition, I'm improving the management of the Data Address Table in
DW I3C Master by keeping the free slots consecutive.

Change in v3:
  - Change cover letter
  - Change commit message for patch 1
  - Add Rob rb-tags

Change in v2:
  - Move out detach/free the i3c_dev_desc from pre_assign_dyn_addr()
  - Change i3c_master_search_i3c_boardinfo(newdev) to
  i3c_master_init_i3c_dev_boardinfo(newdev)
  - Add fixes, stable tags on patch 2
  - Add a note for no guarantee of 'assigned-address' use

Vitor Soares (5):
  i3c: master: detach and free device if pre_assign_dyn_addr() fails
  i3c: master: make sure ->boardinfo is initialized in
    add_i3c_dev_locked()
  dt-bindings: i3c: Make 'assigned-address' valid if static address == 0
  dt-bindings: i3c: add a note for no guarantee of 'assigned-address'
    use
  i3c: master: dw: reattach device on first available location of
    address table

 Documentation/devicetree/bindings/i3c/i3c.txt | 15 ++++++--
 drivers/i3c/master.c                          | 49 ++++++++++++++++++++++-----
 drivers/i3c/master/dw-i3c-master.c            | 16 +++++++++
 3 files changed, 68 insertions(+), 12 deletions(-)

-- 
2.7.4


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* [PATCH v3 1/5] i3c: master: detach/free devices fail on pre_assign_dyn_addr()
  2019-09-05 10:00 [PATCH v3 0/5] i3c: detach/free device fail pre_assign_dyn_addr() Vitor Soares
@ 2019-09-05 10:00 ` Vitor Soares
  2019-09-05 10:00 ` [PATCH v3 2/5] i3c: master: make sure ->boardinfo is initialized in add_i3c_dev_locked() Vitor Soares
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Vitor Soares @ 2019-09-05 10:00 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-i3c
  Cc: mark.rutland, Joao.Pinto, bbrezillon, Vitor Soares, pgaj, robh+dt

As for today, the I3C framework is keeping in memory and master->bus.devs
list the devices that fail during pre_assign_dyn_addr() and send them on
DEFSLVS command.

According to MIPI I3C Bus spec the DEFSLVS command is used to inform any
Secondary Master about the Dynamic Addresses that were assigned to I3C
devices and the I2C devices present on the bus.

This issue could be fixed by changing i3c_master_defslvs_locked() to
ignore unaddressed i3c devices but the i3c_dev_desc would be allocated and
attached to HC unnecessarily. This can cause that some HC aren't able to
do DAA for HJ capable devices due to lack of space in controller.

Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
---
Changes in v3:
  - Change commit message

Changes in v2:
  - Move out detach/free the i3c_dev_desc from pre_assign_dyn_addr()
  - Convert i3c_master_pre_assign_dyn_addr() to return an int

 drivers/i3c/master.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 5f4bd52..586e34f 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1426,19 +1426,19 @@ static void i3c_master_detach_i2c_dev(struct i2c_dev_desc *dev)
 		master->ops->detach_i2c_dev(dev);
 }
 
-static void i3c_master_pre_assign_dyn_addr(struct i3c_dev_desc *dev)
+static int i3c_master_pre_assign_dyn_addr(struct i3c_dev_desc *dev)
 {
 	struct i3c_master_controller *master = i3c_dev_get_master(dev);
 	int ret;
 
 	if (!dev->boardinfo || !dev->boardinfo->init_dyn_addr ||
 	    !dev->boardinfo->static_addr)
-		return;
+		return 0;
 
 	ret = i3c_master_setdasa_locked(master, dev->info.static_addr,
 					dev->boardinfo->init_dyn_addr);
 	if (ret)
-		return;
+		return ret;
 
 	dev->info.dyn_addr = dev->boardinfo->init_dyn_addr;
 	ret = i3c_master_reattach_i3c_dev(dev, 0);
@@ -1449,10 +1449,12 @@ static void i3c_master_pre_assign_dyn_addr(struct i3c_dev_desc *dev)
 	if (ret)
 		goto err_rstdaa;
 
-	return;
+	return 0;
 
 err_rstdaa:
 	i3c_master_rstdaa_locked(master, dev->boardinfo->init_dyn_addr);
+
+	return ret;
 }
 
 static void
@@ -1647,7 +1649,7 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
 	enum i3c_addr_slot_status status;
 	struct i2c_dev_boardinfo *i2cboardinfo;
 	struct i3c_dev_boardinfo *i3cboardinfo;
-	struct i3c_dev_desc *i3cdev;
+	struct i3c_dev_desc *i3cdev, *i3ctmp;
 	struct i2c_dev_desc *i2cdev;
 	int ret;
 
@@ -1746,8 +1748,14 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
 	 * Pre-assign dynamic address and retrieve device information if
 	 * needed.
 	 */
-	i3c_bus_for_each_i3cdev(&master->bus, i3cdev)
-		i3c_master_pre_assign_dyn_addr(i3cdev);
+	list_for_each_entry_safe(i3cdev, i3ctmp, &master->bus.devs.i3c,
+				 common.node) {
+		ret = i3c_master_pre_assign_dyn_addr(i3cdev);
+		if (ret) {
+			i3c_master_detach_i3c_dev(i3cdev);
+			i3c_master_free_i3c_dev(i3cdev);
+		}
+	}
 
 	ret = i3c_master_do_daa(master);
 	if (ret)
-- 
2.7.4


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* [PATCH v3 2/5] i3c: master: make sure ->boardinfo is initialized in add_i3c_dev_locked()
  2019-09-05 10:00 [PATCH v3 0/5] i3c: detach/free device fail pre_assign_dyn_addr() Vitor Soares
  2019-09-05 10:00 ` [PATCH v3 1/5] i3c: master: detach/free devices fail on pre_assign_dyn_addr() Vitor Soares
@ 2019-09-05 10:00 ` Vitor Soares
  2019-09-05 10:00 ` [PATCH v3 3/5] dt-bindings: i3c: Make 'assigned-address' valid if static address == 0 Vitor Soares
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Vitor Soares @ 2019-09-05 10:00 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-i3c
  Cc: mark.rutland, Joao.Pinto, bbrezillon, stable, Vitor Soares, pgaj,
	robh+dt

The newdev->boardinfo assignment was missing in
i3c_master_add_i3c_dev_locked() and hence the ->of_node info isn't
propagated to i3c_dev_desc.

Fix this by trying to initialize device i3c_dev_boardinfo if available.

Cc: <stable@vger.kernel.org>
Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
---
Change in v3:
  - None

Changes in v2:
  - Change commit message
  - Change i3c_master_search_i3c_boardinfo(newdev) to
  i3c_master_init_i3c_dev_boardinfo(newdev)
  - Add fixes, stable tags

 drivers/i3c/master.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 586e34f..9fb99bc 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1798,6 +1798,22 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
 	return NULL;
 }
 
+static void i3c_master_init_i3c_dev_boardinfo(struct i3c_dev_desc *dev)
+{
+	struct i3c_master_controller *master = i3c_dev_get_master(dev);
+	struct i3c_dev_boardinfo *boardinfo;
+
+	if (dev->boardinfo)
+		return;
+
+	list_for_each_entry(boardinfo, &master->boardinfo.i3c, node) {
+		if (dev->info.pid == boardinfo->pid) {
+			dev->boardinfo = boardinfo;
+			return;
+		}
+	}
+}
+
 /**
  * i3c_master_add_i3c_dev_locked() - add an I3C slave to the bus
  * @master: master used to send frames on the bus
@@ -1818,8 +1834,9 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
 				  u8 addr)
 {
 	struct i3c_device_info info = { .dyn_addr = addr };
-	struct i3c_dev_desc *newdev, *olddev;
 	u8 old_dyn_addr = addr, expected_dyn_addr;
+	enum i3c_addr_slot_status addrstatus;
+	struct i3c_dev_desc *newdev, *olddev;
 	struct i3c_ibi_setup ibireq = { };
 	bool enable_ibi = false;
 	int ret;
@@ -1878,6 +1895,8 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
 	if (ret)
 		goto err_detach_dev;
 
+	i3c_master_init_i3c_dev_boardinfo(newdev);
+
 	/*
 	 * Depending on our previous state, the expected dynamic address might
 	 * differ:
@@ -1895,7 +1914,11 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
 	else
 		expected_dyn_addr = newdev->info.dyn_addr;
 
-	if (newdev->info.dyn_addr != expected_dyn_addr) {
+	addrstatus = i3c_bus_get_addr_slot_status(&master->bus,
+						  expected_dyn_addr);
+
+	if (newdev->info.dyn_addr != expected_dyn_addr &&
+	    addrstatus == I3C_ADDR_SLOT_FREE) {
 		/*
 		 * Try to apply the expected dynamic address. If it fails, keep
 		 * the address assigned by the master.
-- 
2.7.4


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* [PATCH v3 3/5] dt-bindings: i3c: Make 'assigned-address' valid if static address == 0
  2019-09-05 10:00 [PATCH v3 0/5] i3c: detach/free device fail pre_assign_dyn_addr() Vitor Soares
  2019-09-05 10:00 ` [PATCH v3 1/5] i3c: master: detach/free devices fail on pre_assign_dyn_addr() Vitor Soares
  2019-09-05 10:00 ` [PATCH v3 2/5] i3c: master: make sure ->boardinfo is initialized in add_i3c_dev_locked() Vitor Soares
@ 2019-09-05 10:00 ` Vitor Soares
  2019-09-05 10:00 ` [PATCH v3 4/5] dt-bindings: i3c: add a note for no guarantee of 'assigned-address' use Vitor Soares
  2019-09-05 10:00 ` [PATCH v3 5/5] i3c: master: dw: reattach device on first available location of address table Vitor Soares
  4 siblings, 0 replies; 6+ messages in thread
From: Vitor Soares @ 2019-09-05 10:00 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-i3c
  Cc: mark.rutland, Joao.Pinto, bbrezillon, Vitor Soares, pgaj, robh+dt

The I3C devices without a static address can require a specific dynamic
address for priority reasons.

Let's update the binding document to make the 'assigned-address' property
valid if static address == 0 and add an example with this use case.

Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Change in v3:
  - Add Rob rb-tag

Change in v2:
  - Fix typo in commit message

 Documentation/devicetree/bindings/i3c/i3c.txt | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/i3c/i3c.txt b/Documentation/devicetree/bindings/i3c/i3c.txt
index ab729a0..c851e75 100644
--- a/Documentation/devicetree/bindings/i3c/i3c.txt
+++ b/Documentation/devicetree/bindings/i3c/i3c.txt
@@ -98,9 +98,7 @@ Required properties
 
 Optional properties
 -------------------
-- assigned-address: dynamic address to be assigned to this device. This
-		    property is only valid if the I3C device has a static
-		    address (first cell of the reg property != 0).
+- assigned-address: dynamic address to be assigned to this device.
 
 
 Example:
@@ -129,6 +127,15 @@ Example:
 
 		/*
 		 * I3C device without a static I2C address but requiring
+		 * specific dynamic address.
+		 */
+		sensor@0,39200154004 {
+			reg = <0x0 0x6072 0x303904d2>;
+			assigned-address = <0xb>;
+		};
+
+		/*
+		 * I3C device without a static I2C address but requiring
 		 * resources described in the DT.
 		 */
 		sensor@0,39200154004 {
-- 
2.7.4


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* [PATCH v3 4/5] dt-bindings: i3c: add a note for no guarantee of 'assigned-address' use
  2019-09-05 10:00 [PATCH v3 0/5] i3c: detach/free device fail pre_assign_dyn_addr() Vitor Soares
                   ` (2 preceding siblings ...)
  2019-09-05 10:00 ` [PATCH v3 3/5] dt-bindings: i3c: Make 'assigned-address' valid if static address == 0 Vitor Soares
@ 2019-09-05 10:00 ` Vitor Soares
  2019-09-05 10:00 ` [PATCH v3 5/5] i3c: master: dw: reattach device on first available location of address table Vitor Soares
  4 siblings, 0 replies; 6+ messages in thread
From: Vitor Soares @ 2019-09-05 10:00 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-i3c
  Cc: mark.rutland, Joao.Pinto, bbrezillon, Vitor Soares, pgaj, robh+dt

By default, the framework will try to assign the 'assigned-address' to the
device but if the address is already in use the device dynamic address
will be different.

Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Change in v3:
  - Add Rob rb-tag

 Documentation/devicetree/bindings/i3c/i3c.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i3c/i3c.txt b/Documentation/devicetree/bindings/i3c/i3c.txt
index c851e75..e777f09 100644
--- a/Documentation/devicetree/bindings/i3c/i3c.txt
+++ b/Documentation/devicetree/bindings/i3c/i3c.txt
@@ -98,7 +98,9 @@ Required properties
 
 Optional properties
 -------------------
-- assigned-address: dynamic address to be assigned to this device.
+- assigned-address: dynamic address to be assigned to this device. The framework
+		    will try to assign this dynamic address but if something
+		    fails the device dynamic address will be different.
 
 
 Example:
-- 
2.7.4


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* [PATCH v3 5/5] i3c: master: dw: reattach device on first available location of address table
  2019-09-05 10:00 [PATCH v3 0/5] i3c: detach/free device fail pre_assign_dyn_addr() Vitor Soares
                   ` (3 preceding siblings ...)
  2019-09-05 10:00 ` [PATCH v3 4/5] dt-bindings: i3c: add a note for no guarantee of 'assigned-address' use Vitor Soares
@ 2019-09-05 10:00 ` Vitor Soares
  4 siblings, 0 replies; 6+ messages in thread
From: Vitor Soares @ 2019-09-05 10:00 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-i3c
  Cc: mark.rutland, Joao.Pinto, bbrezillon, Vitor Soares, pgaj, robh+dt

For today the reattach function only update the device address on the
controller.

Update the location to the first available too, will optimize the
enumeration process avoiding additional checks to keep the available
positions on address table consecutive.

Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Change in v3:
  - None

Change in v2:
  - Add Boris rb-tag

 drivers/i3c/master/dw-i3c-master.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 1d83c97..62261ac 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -898,6 +898,22 @@ static int dw_i3c_master_reattach_i3c_dev(struct i3c_dev_desc *dev,
 	struct dw_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
 	struct i3c_master_controller *m = i3c_dev_get_master(dev);
 	struct dw_i3c_master *master = to_dw_i3c_master(m);
+	int pos;
+
+	pos = dw_i3c_master_get_free_pos(master);
+
+	if (data->index > pos && pos > 0) {
+		writel(0,
+		       master->regs +
+		       DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
+
+		master->addrs[data->index] = 0;
+		master->free_pos |= BIT(data->index);
+
+		data->index = pos;
+		master->addrs[pos] = dev->info.dyn_addr;
+		master->free_pos &= ~BIT(pos);
+	}
 
 	writel(DEV_ADDR_TABLE_DYNAMIC_ADDR(dev->info.dyn_addr),
 	       master->regs +
-- 
2.7.4


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

^ 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 --
2019-09-05 10:00 [PATCH v3 0/5] i3c: detach/free device fail pre_assign_dyn_addr() Vitor Soares
2019-09-05 10:00 ` [PATCH v3 1/5] i3c: master: detach/free devices fail on pre_assign_dyn_addr() Vitor Soares
2019-09-05 10:00 ` [PATCH v3 2/5] i3c: master: make sure ->boardinfo is initialized in add_i3c_dev_locked() Vitor Soares
2019-09-05 10:00 ` [PATCH v3 3/5] dt-bindings: i3c: Make 'assigned-address' valid if static address == 0 Vitor Soares
2019-09-05 10:00 ` [PATCH v3 4/5] dt-bindings: i3c: add a note for no guarantee of 'assigned-address' use Vitor Soares
2019-09-05 10:00 ` [PATCH v3 5/5] i3c: master: dw: reattach device on first available location of address table Vitor Soares

Linux-i3c Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-i3c/0 linux-i3c/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-i3c linux-i3c/ https://lore.kernel.org/linux-i3c \
		linux-i3c@lists.infradead.org linux-i3c@archiver.kernel.org
	public-inbox-index linux-i3c


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-i3c


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