linux-i3c.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/6] I3C device addresing adjustments
@ 2019-12-10 10:14 Przemysław Gaj
  2019-12-10 10:14 ` [PATCH v4 1/6] i3c: master: make sure ->boardinfo is initialized in add_i3c_dev_locked() Przemysław Gaj
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Przemysław Gaj @ 2019-12-10 10:14 UTC (permalink / raw)
  To: bbrezillon; +Cc: linux-i3c, Przemysław Gaj, rafalc, vitor.soares

This patch series is based on Vitor's Soares previous patch series.
I'm trying to meet requirements and comments which came up during the
code review.

Description related to things which were taken from v3:
Propagate i3c_boardinfo, if available, to i3c_dev_desc during
i3c_master_add_i3c_dev_locked(). 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.

Sending the devices without DA in DEFSLVS frame will be fixed in separate
patch if needed.

There are also some improvements related to DT bindings documentation.

Changes in v4:
  - Remove device detach/free
  - Add PID check before registering the device
  - Add pre-reservation of init_dyn_addr

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

Przemysław Gaj (2):
  i3c: master: pre-reserve boardinfo->init_dyn_addr when available
  i3c: master: make sure the PID is set before registering the device

Vitor Soares (4):
  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                          | 29 +++++++++++++++++++++++++--
 drivers/i3c/master/dw-i3c-master.c            | 16 +++++++++++++++
 3 files changed, 55 insertions(+), 5 deletions(-)
 mode change 100644 => 100755 drivers/i3c/master.c

-- 
2.14.0


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

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

* [PATCH v4 1/6] i3c: master: make sure ->boardinfo is initialized in add_i3c_dev_locked()
  2019-12-10 10:14 [PATCH v4 0/6] I3C device addresing adjustments Przemysław Gaj
@ 2019-12-10 10:14 ` Przemysław Gaj
  2019-12-10 10:14 ` [PATCH v4 2/6] i3c: master: pre-reserve boardinfo->init_dyn_addr when available Przemysław Gaj
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Przemysław Gaj @ 2019-12-10 10:14 UTC (permalink / raw)
  To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, stable, vitor.soares

From: Vitor Soares <vitor.soares@synopsys.com>

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>
Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
---
Change in v4:
  - Remove addrstatus check, this will be sent as a separate patch

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 | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 mode change 100644 => 100755 drivers/i3c/master.c

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 043691656245..5c06c41e6660
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1774,6 +1774,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
@@ -1854,6 +1870,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:
-- 
2.14.0


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

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

* [PATCH v4 2/6] i3c: master: pre-reserve boardinfo->init_dyn_addr when available
  2019-12-10 10:14 [PATCH v4 0/6] I3C device addresing adjustments Przemysław Gaj
  2019-12-10 10:14 ` [PATCH v4 1/6] i3c: master: make sure ->boardinfo is initialized in add_i3c_dev_locked() Przemysław Gaj
@ 2019-12-10 10:14 ` Przemysław Gaj
  2019-12-10 13:29   ` Boris Brezillon
  2019-12-10 15:24   ` Vitor Soares
  2019-12-10 10:14 ` [PATCH v4 3/6] i3c: master: make sure the PID is set before registering the device Przemysław Gaj
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 13+ messages in thread
From: Przemysław Gaj @ 2019-12-10 10:14 UTC (permalink / raw)
  To: bbrezillon; +Cc: linux-i3c, Przemysław Gaj, rafalc, vitor.soares

It may be the case that SETDASA fails for some reason. Reserve
->init_dyn_addr when it's defined to prevent assigning this address
to another slave device. This way when device shows up we don't
have to re-assign addresses.

Signed-off-by: Przemysław Gaj <pgaj@cadence.com>
---
 drivers/i3c/master.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 5c06c41e6660..fab6e0609fca 100755
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1263,7 +1263,8 @@ static void i3c_master_put_i3c_addrs(struct i3c_dev_desc *dev)
 					     I3C_ADDR_SLOT_FREE);
 
 	if (dev->boardinfo && dev->boardinfo->init_dyn_addr)
-		i3c_bus_set_addr_slot_status(&master->bus, dev->info.dyn_addr,
+		i3c_bus_set_addr_slot_status(&master->bus,
+					     dev->boardinfo->init_dyn_addr,
 					     I3C_ADDR_SLOT_FREE);
 }
 
@@ -1675,6 +1676,11 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
 				ret = -EBUSY;
 				goto err_detach_devs;
 			}
+
+			/* Reserve the slot. */
+			i3c_bus_set_addr_slot_status(&master->bus,
+						     i3cboardinfo->init_dyn_addr,
+						     I3C_ADDR_SLOT_I3C_DEV);
 		}
 
 		i3cdev = i3c_master_alloc_i3c_dev(master, &info);
-- 
2.14.0


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

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

* [PATCH v4 3/6] i3c: master: make sure the PID is set before registering the device
  2019-12-10 10:14 [PATCH v4 0/6] I3C device addresing adjustments Przemysław Gaj
  2019-12-10 10:14 ` [PATCH v4 1/6] i3c: master: make sure ->boardinfo is initialized in add_i3c_dev_locked() Przemysław Gaj
  2019-12-10 10:14 ` [PATCH v4 2/6] i3c: master: pre-reserve boardinfo->init_dyn_addr when available Przemysław Gaj
@ 2019-12-10 10:14 ` Przemysław Gaj
  2019-12-10 10:15 ` [PATCH v4 4/6] dt-bindings: i3c: Make 'assigned-address' valid if static address == 0 Przemysław Gaj
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Przemysław Gaj @ 2019-12-10 10:14 UTC (permalink / raw)
  To: bbrezillon; +Cc: linux-i3c, Przemysław Gaj, rafalc, vitor.soares

If SETDASA failed for some reason or Provisioned ID (PID) retrieval
failed PID may not be set. Check that condition before registering
the device.

Signed-off-by: Przemysław Gaj <pgaj@cadence.com>
---
 drivers/i3c/master.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index fab6e0609fca..4b3d1c0f778d 100755
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1450,7 +1450,8 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 		return;
 
 	i3c_bus_for_each_i3cdev(&master->bus, desc) {
-		if (desc->dev || !desc->info.dyn_addr || desc == master->this)
+		if (desc->dev || !desc->info.dyn_addr ||
+		    desc == master->this || !desc->info.pid)
 			continue;
 
 		desc->dev = kzalloc(sizeof(*desc->dev), GFP_KERNEL);
-- 
2.14.0


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

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

* [PATCH v4 4/6] dt-bindings: i3c: Make 'assigned-address' valid if static address == 0
  2019-12-10 10:14 [PATCH v4 0/6] I3C device addresing adjustments Przemysław Gaj
                   ` (2 preceding siblings ...)
  2019-12-10 10:14 ` [PATCH v4 3/6] i3c: master: make sure the PID is set before registering the device Przemysław Gaj
@ 2019-12-10 10:15 ` Przemysław Gaj
  2019-12-10 10:15 ` [PATCH v4 5/6] dt-bindings: i3c: add a note for no guarantee of 'assigned-address' use Przemysław Gaj
  2019-12-10 10:15 ` [PATCH v4 6/6] i3c: master: dw: reattach device on first available location of address table Przemysław Gaj
  5 siblings, 0 replies; 13+ messages in thread
From: Przemysław Gaj @ 2019-12-10 10:15 UTC (permalink / raw)
  To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares

From: Vitor Soares <vitor.soares@synopsys.com>

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>
Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
---
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 4ffe059f0fec..7fee9b4dfba0 100644
--- a/Documentation/devicetree/bindings/i3c/i3c.txt
+++ b/Documentation/devicetree/bindings/i3c/i3c.txt
@@ -100,9 +100,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:
 			assigned-address = <0xa>;
 		};
 
+		/*
+		 * 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.
-- 
2.14.0


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

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

* [PATCH v4 5/6] dt-bindings: i3c: add a note for no guarantee of 'assigned-address' use
  2019-12-10 10:14 [PATCH v4 0/6] I3C device addresing adjustments Przemysław Gaj
                   ` (3 preceding siblings ...)
  2019-12-10 10:15 ` [PATCH v4 4/6] dt-bindings: i3c: Make 'assigned-address' valid if static address == 0 Przemysław Gaj
@ 2019-12-10 10:15 ` Przemysław Gaj
  2019-12-10 10:15 ` [PATCH v4 6/6] i3c: master: dw: reattach device on first available location of address table Przemysław Gaj
  5 siblings, 0 replies; 13+ messages in thread
From: Przemysław Gaj @ 2019-12-10 10:15 UTC (permalink / raw)
  To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares

From: Vitor Soares <vitor.soares@synopsys.com>

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>
Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
---
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 7fee9b4dfba0..e7ac9756c731 100644
--- a/Documentation/devicetree/bindings/i3c/i3c.txt
+++ b/Documentation/devicetree/bindings/i3c/i3c.txt
@@ -100,7 +100,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.14.0


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

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

* [PATCH v4 6/6] i3c: master: dw: reattach device on first available location of address table
  2019-12-10 10:14 [PATCH v4 0/6] I3C device addresing adjustments Przemysław Gaj
                   ` (4 preceding siblings ...)
  2019-12-10 10:15 ` [PATCH v4 5/6] dt-bindings: i3c: add a note for no guarantee of 'assigned-address' use Przemysław Gaj
@ 2019-12-10 10:15 ` Przemysław Gaj
  2019-12-10 14:40   ` Vitor Soares
  5 siblings, 1 reply; 13+ messages in thread
From: Przemysław Gaj @ 2019-12-10 10:15 UTC (permalink / raw)
  To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares

From: Vitor Soares <vitor.soares@synopsys.com>

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>
Signed-off-by: Przemyslaw Gaj <pgaj@cadence.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 b0ff0e12d84c..c6caba39a34b 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -899,6 +899,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.14.0


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

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

* Re: [PATCH v4 2/6] i3c: master: pre-reserve boardinfo->init_dyn_addr when available
  2019-12-10 10:14 ` [PATCH v4 2/6] i3c: master: pre-reserve boardinfo->init_dyn_addr when available Przemysław Gaj
@ 2019-12-10 13:29   ` Boris Brezillon
  2019-12-10 15:24   ` Vitor Soares
  1 sibling, 0 replies; 13+ messages in thread
From: Boris Brezillon @ 2019-12-10 13:29 UTC (permalink / raw)
  To: Przemysław Gaj; +Cc: linux-i3c, vitor.soares, rafalc, bbrezillon

On Tue, 10 Dec 2019 11:14:58 +0100
Przemysław Gaj <pgaj@cadence.com> wrote:

> It may be the case that SETDASA fails for some reason. Reserve
> ->init_dyn_addr when it's defined to prevent assigning this address  
> to another slave device. This way when device shows up we don't
> have to re-assign addresses.
> 
> Signed-off-by: Przemysław Gaj <pgaj@cadence.com>
> ---
>  drivers/i3c/master.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5c06c41e6660..fab6e0609fca 100755
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1263,7 +1263,8 @@ static void i3c_master_put_i3c_addrs(struct i3c_dev_desc *dev)
>  					     I3C_ADDR_SLOT_FREE);
>  
>  	if (dev->boardinfo && dev->boardinfo->init_dyn_addr)
> -		i3c_bus_set_addr_slot_status(&master->bus, dev->info.dyn_addr,
> +		i3c_bus_set_addr_slot_status(&master->bus,
> +					     dev->boardinfo->init_dyn_addr,
>  					     I3C_ADDR_SLOT_FREE);

Should be fixed in a separate patch, or at least mentioned in the
commit message.

>  }
>  
> @@ -1675,6 +1676,11 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
>  				ret = -EBUSY;
>  				goto err_detach_devs;
>  			}
> +
> +			/* Reserve the slot. */
> +			i3c_bus_set_addr_slot_status(&master->bus,
> +						     i3cboardinfo->init_dyn_addr,
> +						     I3C_ADDR_SLOT_I3C_DEV);

Looks like the "reserve/release init_dyn_addr slot" logic is
asymmetric. I wonder if that's not a problem. Can't we end up in a
situation where the ->init_dyn_addr is released (when SETDASA() fails)
and then re-used without being reserved (when the device is later
discovered through a regular DAA)?

So maybe I was wrong and we should indeed reserve the address in the
i3c_master_get_i3c_addrs() path.

>  		}
>  
>  		i3cdev = i3c_master_alloc_i3c_dev(master, &info);


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

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

* RE: [PATCH v4 6/6] i3c: master: dw: reattach device on first available location of address table
  2019-12-10 10:15 ` [PATCH v4 6/6] i3c: master: dw: reattach device on first available location of address table Przemysław Gaj
@ 2019-12-10 14:40   ` Vitor Soares
  2019-12-10 14:43     ` Przemysław Gaj
  2019-12-10 14:52     ` Boris Brezillon
  0 siblings, 2 replies; 13+ messages in thread
From: Vitor Soares @ 2019-12-10 14:40 UTC (permalink / raw)
  To: Przemysław Gaj, bbrezillon, Joao Pinto; +Cc: linux-i3c, rafalc

++João Pinto

Hi Przemysław,

Sorry for this, but please drop this patch from this series. Boris as 
Maintainer could already merge it when gives rb tag.

From: Przemysław Gaj <pgaj@cadence.com>
Date: Tue, Dec 10, 2019 at 10:15:02

> From: Vitor Soares <vitor.soares@synopsys.com>
> 
> 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>
> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.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 b0ff0e12d84c..c6caba39a34b 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -899,6 +899,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.14.0

Best regards,
Vitor Soares
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

* Re: [PATCH v4 6/6] i3c: master: dw: reattach device on first available location of address table
  2019-12-10 14:40   ` Vitor Soares
@ 2019-12-10 14:43     ` Przemysław Gaj
  2019-12-10 14:52     ` Boris Brezillon
  1 sibling, 0 replies; 13+ messages in thread
From: Przemysław Gaj @ 2019-12-10 14:43 UTC (permalink / raw)
  To: Vitor Soares; +Cc: linux-i3c, Joao Pinto, rafalc, bbrezillon

Hi Vitor,

The 12/10/2019 14:40, Vitor Soares wrote:
> 
> ++João Pinto
> 
> Hi Przemysław,
> 
> Sorry for this, but please drop this patch from this series. Boris as 
> Maintainer could already merge it when gives rb tag.

OK, understand. Thank you for the information.

> 
> From: Przemysław Gaj <pgaj@cadence.com>
> Date: Tue, Dec 10, 2019 at 10:15:02
> 
> > From: Vitor Soares <vitor.soares@synopsys.com>
> > 
> > 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>
> > Signed-off-by: Przemyslaw Gaj <pgaj@cadence.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 b0ff0e12d84c..c6caba39a34b 100644
> > --- a/drivers/i3c/master/dw-i3c-master.c
> > +++ b/drivers/i3c/master/dw-i3c-master.c
> > @@ -899,6 +899,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.14.0
> 
> Best regards,
> Vitor Soares

-- 
--
Regards,
Przemek

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

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

* Re: [PATCH v4 6/6] i3c: master: dw: reattach device on first available location of address table
  2019-12-10 14:40   ` Vitor Soares
  2019-12-10 14:43     ` Przemysław Gaj
@ 2019-12-10 14:52     ` Boris Brezillon
  1 sibling, 0 replies; 13+ messages in thread
From: Boris Brezillon @ 2019-12-10 14:52 UTC (permalink / raw)
  To: Vitor Soares
  Cc: Przemysław Gaj, Joao Pinto, linux-i3c, rafalc, bbrezillon

On Tue, 10 Dec 2019 14:40:49 +0000
Vitor Soares <Vitor.Soares@synopsys.com> wrote:

> ++João Pinto
> 
> Hi Przemysław,
> 
> Sorry for this, but please drop this patch from this series. Boris as 
> Maintainer could already merge it when gives rb tag.

Noted, I'll queue that one separately (IIUC, it's independent from the
rest of the series).

> 
> From: Przemysław Gaj <pgaj@cadence.com>
> Date: Tue, Dec 10, 2019 at 10:15:02
> 
> > From: Vitor Soares <vitor.soares@synopsys.com>
> > 
> > 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>
> > Signed-off-by: Przemyslaw Gaj <pgaj@cadence.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 b0ff0e12d84c..c6caba39a34b 100644
> > --- a/drivers/i3c/master/dw-i3c-master.c
> > +++ b/drivers/i3c/master/dw-i3c-master.c
> > @@ -899,6 +899,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.14.0  
> 
> Best regards,
> Vitor Soares


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

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

* RE: [PATCH v4 2/6] i3c: master: pre-reserve boardinfo->init_dyn_addr when available
  2019-12-10 10:14 ` [PATCH v4 2/6] i3c: master: pre-reserve boardinfo->init_dyn_addr when available Przemysław Gaj
  2019-12-10 13:29   ` Boris Brezillon
@ 2019-12-10 15:24   ` Vitor Soares
  2019-12-11  8:51     ` Przemysław Gaj
  1 sibling, 1 reply; 13+ messages in thread
From: Vitor Soares @ 2019-12-10 15:24 UTC (permalink / raw)
  To: Przemysław Gaj, bbrezillon; +Cc: linux-i3c, rafalc

Hi Przemysław,

From: Przemysław Gaj <pgaj@cadence.com>
Date: Tue, Dec 10, 2019 at 10:14:58

> It may be the case that SETDASA fails for some reason. Reserve
> ->init_dyn_addr when it's defined to prevent assigning this address
> to another slave device. This way when device shows up we don't
> have to re-assign addresses.
> 
> Signed-off-by: Przemysław Gaj <pgaj@cadence.com>
> ---
>  drivers/i3c/master.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5c06c41e6660..fab6e0609fca 100755
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1263,7 +1263,8 @@ static void i3c_master_put_i3c_addrs(struct i3c_dev_desc *dev)
>  					     I3C_ADDR_SLOT_FREE);
>  
>  	if (dev->boardinfo && dev->boardinfo->init_dyn_addr)
> -		i3c_bus_set_addr_slot_status(&master->bus, dev->info.dyn_addr,
> +		i3c_bus_set_addr_slot_status(&master->bus,
> +					     dev->boardinfo->init_dyn_addr,
>  					     I3C_ADDR_SLOT_FREE);
>  }
>  
> @@ -1675,6 +1676,11 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
>  				ret = -EBUSY;
>  				goto err_detach_devs;
>  			}
> +
> +			/* Reserve the slot. */
> +			i3c_bus_set_addr_slot_status(&master->bus,
> +						     i3cboardinfo->init_dyn_addr,
> +						     I3C_ADDR_SLOT_I3C_DEV);
>  		}
>  
>  		i3cdev = i3c_master_alloc_i3c_dev(master, &info);
> -- 
> 2.14.0

In my experience we should pre-reserve all DT DA entries and not use them 
during the ENTDAA for safety reasons.
That would be one of my next steps in this patch series.

Best regards,
Vitor Soares

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

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

* Re: [PATCH v4 2/6] i3c: master: pre-reserve boardinfo->init_dyn_addr when available
  2019-12-10 15:24   ` Vitor Soares
@ 2019-12-11  8:51     ` Przemysław Gaj
  0 siblings, 0 replies; 13+ messages in thread
From: Przemysław Gaj @ 2019-12-11  8:51 UTC (permalink / raw)
  To: Vitor Soares; +Cc: linux-i3c, rafalc, bbrezillon

Hi Vitor,

The 12/10/2019 15:24, Vitor Soares wrote:
> 
> Hi Przemysław,
> 
> From: Przemysław Gaj <pgaj@cadence.com>
> Date: Tue, Dec 10, 2019 at 10:14:58
> 
> > It may be the case that SETDASA fails for some reason. Reserve
> > ->init_dyn_addr when it's defined to prevent assigning this address
> > to another slave device. This way when device shows up we don't
> > have to re-assign addresses.
> > 
> > Signed-off-by: Przemysław Gaj <pgaj@cadence.com>
> > ---
> >  drivers/i3c/master.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > index 5c06c41e6660..fab6e0609fca 100755
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
> > @@ -1263,7 +1263,8 @@ static void i3c_master_put_i3c_addrs(struct i3c_dev_desc *dev)
> >  					     I3C_ADDR_SLOT_FREE);
> >  
> >  	if (dev->boardinfo && dev->boardinfo->init_dyn_addr)
> > -		i3c_bus_set_addr_slot_status(&master->bus, dev->info.dyn_addr,
> > +		i3c_bus_set_addr_slot_status(&master->bus,
> > +					     dev->boardinfo->init_dyn_addr,
> >  					     I3C_ADDR_SLOT_FREE);
> >  }
> >  
> > @@ -1675,6 +1676,11 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> >  				ret = -EBUSY;
> >  				goto err_detach_devs;
> >  			}
> > +
> > +			/* Reserve the slot. */
> > +			i3c_bus_set_addr_slot_status(&master->bus,
> > +						     i3cboardinfo->init_dyn_addr,
> > +						     I3C_ADDR_SLOT_I3C_DEV);
> >  		}
> >  
> >  		i3cdev = i3c_master_alloc_i3c_dev(master, &info);
> > -- 
> > 2.14.0
> 
> In my experience we should pre-reserve all DT DA entries and not use them 
> during the ENTDAA for safety reasons.
> That would be one of my next steps in this patch series.

Actually, I reserve DT DA address above. The only thing I will rework, I
won't free up the address. That will be documented in the code.

We can rework it later if really needed.

> 
> Best regards,
> Vitor Soares
> 

-- 
-- 
Regards,
Przemek

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

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

end of thread, other threads:[~2019-12-11  8:52 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 10:14 [PATCH v4 0/6] I3C device addresing adjustments Przemysław Gaj
2019-12-10 10:14 ` [PATCH v4 1/6] i3c: master: make sure ->boardinfo is initialized in add_i3c_dev_locked() Przemysław Gaj
2019-12-10 10:14 ` [PATCH v4 2/6] i3c: master: pre-reserve boardinfo->init_dyn_addr when available Przemysław Gaj
2019-12-10 13:29   ` Boris Brezillon
2019-12-10 15:24   ` Vitor Soares
2019-12-11  8:51     ` Przemysław Gaj
2019-12-10 10:14 ` [PATCH v4 3/6] i3c: master: make sure the PID is set before registering the device Przemysław Gaj
2019-12-10 10:15 ` [PATCH v4 4/6] dt-bindings: i3c: Make 'assigned-address' valid if static address == 0 Przemysław Gaj
2019-12-10 10:15 ` [PATCH v4 5/6] dt-bindings: i3c: add a note for no guarantee of 'assigned-address' use Przemysław Gaj
2019-12-10 10:15 ` [PATCH v4 6/6] i3c: master: dw: reattach device on first available location of address table Przemysław Gaj
2019-12-10 14:40   ` Vitor Soares
2019-12-10 14:43     ` Przemysław Gaj
2019-12-10 14:52     ` Boris Brezillon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).