From: Parshuram Thombare <pthombar@cadence.com> To: <bbrezillon@kernel.org>, <vitor.soares@synopsys.com> Cc: <pgaj@cadence.com>, <linux-i3c@lists.infradead.org>, <linux-kernel@vger.kernel.org>, <mparab@cadence.com>, <praneeth@ti.com>, Parshuram Thombare <pthombar@cadence.com> Subject: [PATCH] i3c: master: fix for SETDASA and DAA process Date: Thu, 14 May 2020 18:30:09 +0200 [thread overview] Message-ID: <1589473809-16708-1-git-send-email-pthombar@cadence.com> (raw) This patch fix following issues. 1. Controller slots blocked for devices with static_addr but no init_dyn_addr may limit the number of I3C devices on the bus which gets dynamic address in DAA. So instead of attaching all the devices with static_addr, now we only attach the devices which successfully complete SETDASA. Remaining devices are handled in DAA. 2. Since we alreay handled devices with init_dyn_addr, removed it's handling from i3c_master_add_i3c_dev_locked(). Now only case handled is devices already with dyn_addr participated in DAA, and again got new dyn_addr with an extra slot in the master controller. 3. Removed unnecessary i3c_master_reattach_i3c_dev() from i3c_master_add_i3c_dev_locked(), right away after finding if duplicate device exists in the I3C list. In case of different new and old dyn_addr i3c_master_reattach_i3c_dev() will fail which is wrong, and in case of same dyn_addr it doesn't add anything new. Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") Signed-off-by: Parshuram Thombare <pthombar@cadence.com> --- drivers/i3c/master.c | 111 ++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 65 deletions(-) diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 5f4bd52121fe..f1d929b58549 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -1375,6 +1375,11 @@ static int i3c_master_reattach_i3c_dev(struct i3c_dev_desc *dev, i3c_bus_set_addr_slot_status(&master->bus, dev->info.dyn_addr, I3C_ADDR_SLOT_I3C_DEV); + + if (old_dyn_addr) + i3c_bus_set_addr_slot_status(&master->bus, + old_dyn_addr, + I3C_ADDR_SLOT_FREE); } if (master->ops->reattach_i3c_dev) { @@ -1426,33 +1431,52 @@ 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 void i3c_master_pre_assign_dyn_addr(struct i3c_master_controller *master, + struct i3c_dev_boardinfo *boardinfo) { - struct i3c_master_controller *master = i3c_dev_get_master(dev); + struct i3c_device_info info = { + .static_addr = boardinfo->static_addr, + }; + struct i3c_dev_desc *i3cdev; int ret; - if (!dev->boardinfo || !dev->boardinfo->init_dyn_addr || - !dev->boardinfo->static_addr) + /* + * We anyway don't attach devices which are not addressable + * (no static_addr and dyn_addr) and devices with static_addr + * but no init_dyn_addr will participate in DAA. + */ + if (!boardinfo->static_addr || !boardinfo->init_dyn_addr) + return; + + i3cdev = i3c_master_alloc_i3c_dev(master, &info); + if (IS_ERR(i3cdev)) return; - ret = i3c_master_setdasa_locked(master, dev->info.static_addr, - dev->boardinfo->init_dyn_addr); + i3cdev->boardinfo = boardinfo; + + ret = i3c_master_attach_i3c_dev(master, i3cdev); if (ret) return; - dev->info.dyn_addr = dev->boardinfo->init_dyn_addr; - ret = i3c_master_reattach_i3c_dev(dev, 0); + ret = i3c_master_setdasa_locked(master, i3cdev->info.static_addr, + i3cdev->boardinfo->init_dyn_addr); if (ret) - goto err_rstdaa; + goto err_setdasa; - ret = i3c_master_retrieve_dev_info(dev); + i3cdev->info.dyn_addr = i3cdev->boardinfo->init_dyn_addr; + ret = i3c_master_reattach_i3c_dev(i3cdev, 0); if (ret) goto err_rstdaa; - return; + ret = i3c_master_retrieve_dev_info(i3cdev); + if (ret) + goto err_rstdaa; err_rstdaa: - i3c_master_rstdaa_locked(master, dev->boardinfo->init_dyn_addr); + i3c_master_rstdaa_locked(master, i3cdev->boardinfo->init_dyn_addr); +err_setdasa: + i3c_master_detach_i3c_dev(i3cdev); + i3c_master_free_i3c_dev(i3cdev); } static void @@ -1619,8 +1643,8 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master) * This function is following all initialisation steps described in the I3C * specification: * - * 1. Attach I2C and statically defined I3C devs to the master so that the - * master can fill its internal device table appropriately + * 1. Attach I2C devs to the master so that the master can fill its internal + * device table appropriately * * 2. Call &i3c_master_controller_ops->bus_init() method to initialize * the master controller. That's usually where the bus mode is selected @@ -1647,7 +1671,6 @@ 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 i2c_dev_desc *i2cdev; int ret; @@ -1679,34 +1702,6 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) goto err_detach_devs; } } - list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) { - struct i3c_device_info info = { - .static_addr = i3cboardinfo->static_addr, - }; - - if (i3cboardinfo->init_dyn_addr) { - status = i3c_bus_get_addr_slot_status(&master->bus, - i3cboardinfo->init_dyn_addr); - if (status != I3C_ADDR_SLOT_FREE) { - ret = -EBUSY; - goto err_detach_devs; - } - } - - i3cdev = i3c_master_alloc_i3c_dev(master, &info); - if (IS_ERR(i3cdev)) { - ret = PTR_ERR(i3cdev); - goto err_detach_devs; - } - - i3cdev->boardinfo = i3cboardinfo; - - ret = i3c_master_attach_i3c_dev(master, i3cdev); - if (ret) { - i3c_master_free_i3c_dev(i3cdev); - goto err_detach_devs; - } - } /* * Now execute the controller specific ->bus_init() routine, which @@ -1746,8 +1741,8 @@ 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(i3cboardinfo, &master->boardinfo.i3c, node) + i3c_master_pre_assign_dyn_addr(master, i3cboardinfo); ret = i3c_master_do_daa(master); if (ret) @@ -1811,7 +1806,7 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master, { struct i3c_device_info info = { .dyn_addr = addr }; struct i3c_dev_desc *newdev, *olddev; - u8 old_dyn_addr = addr, expected_dyn_addr; + u8 old_dyn_addr = addr; struct i3c_ibi_setup ibireq = { }; bool enable_ibi = false; int ret; @@ -1866,39 +1861,25 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master, i3c_master_free_i3c_dev(olddev); } - ret = i3c_master_reattach_i3c_dev(newdev, old_dyn_addr); - if (ret) - goto err_detach_dev; - /* * Depending on our previous state, the expected dynamic address might * differ: * - if the device already had a dynamic address assigned, let's try to - * re-apply this one - * - if the device did not have a dynamic address and the firmware - * requested a specific address, pick this one + * re-apply this one. Device with dyn_addr participated in DAA ? * - in any other case, keep the address automatically assigned by the * master */ - if (old_dyn_addr && old_dyn_addr != newdev->info.dyn_addr) - expected_dyn_addr = old_dyn_addr; - else if (newdev->boardinfo && newdev->boardinfo->init_dyn_addr) - expected_dyn_addr = newdev->boardinfo->init_dyn_addr; - else - expected_dyn_addr = newdev->info.dyn_addr; - - if (newdev->info.dyn_addr != expected_dyn_addr) { + if (old_dyn_addr && old_dyn_addr != newdev->info.dyn_addr) { /* * Try to apply the expected dynamic address. If it fails, keep * the address assigned by the master. */ ret = i3c_master_setnewda_locked(master, newdev->info.dyn_addr, - expected_dyn_addr); + old_dyn_addr); if (!ret) { - old_dyn_addr = newdev->info.dyn_addr; - newdev->info.dyn_addr = expected_dyn_addr; - i3c_master_reattach_i3c_dev(newdev, old_dyn_addr); + newdev->info.dyn_addr = old_dyn_addr; + i3c_master_reattach_i3c_dev(newdev, addr); } else { dev_err(&master->dev, "Failed to assign reserved/old address to device %d%llx", -- 2.17.1
WARNING: multiple messages have this Message-ID (diff)
From: Parshuram Thombare <pthombar@cadence.com> To: <bbrezillon@kernel.org>, <vitor.soares@synopsys.com> Cc: mparab@cadence.com, Parshuram Thombare <pthombar@cadence.com>, praneeth@ti.com, linux-kernel@vger.kernel.org, pgaj@cadence.com, linux-i3c@lists.infradead.org Subject: [PATCH] i3c: master: fix for SETDASA and DAA process Date: Thu, 14 May 2020 18:30:09 +0200 [thread overview] Message-ID: <1589473809-16708-1-git-send-email-pthombar@cadence.com> (raw) This patch fix following issues. 1. Controller slots blocked for devices with static_addr but no init_dyn_addr may limit the number of I3C devices on the bus which gets dynamic address in DAA. So instead of attaching all the devices with static_addr, now we only attach the devices which successfully complete SETDASA. Remaining devices are handled in DAA. 2. Since we alreay handled devices with init_dyn_addr, removed it's handling from i3c_master_add_i3c_dev_locked(). Now only case handled is devices already with dyn_addr participated in DAA, and again got new dyn_addr with an extra slot in the master controller. 3. Removed unnecessary i3c_master_reattach_i3c_dev() from i3c_master_add_i3c_dev_locked(), right away after finding if duplicate device exists in the I3C list. In case of different new and old dyn_addr i3c_master_reattach_i3c_dev() will fail which is wrong, and in case of same dyn_addr it doesn't add anything new. Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") Signed-off-by: Parshuram Thombare <pthombar@cadence.com> --- drivers/i3c/master.c | 111 ++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 65 deletions(-) diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 5f4bd52121fe..f1d929b58549 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -1375,6 +1375,11 @@ static int i3c_master_reattach_i3c_dev(struct i3c_dev_desc *dev, i3c_bus_set_addr_slot_status(&master->bus, dev->info.dyn_addr, I3C_ADDR_SLOT_I3C_DEV); + + if (old_dyn_addr) + i3c_bus_set_addr_slot_status(&master->bus, + old_dyn_addr, + I3C_ADDR_SLOT_FREE); } if (master->ops->reattach_i3c_dev) { @@ -1426,33 +1431,52 @@ 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 void i3c_master_pre_assign_dyn_addr(struct i3c_master_controller *master, + struct i3c_dev_boardinfo *boardinfo) { - struct i3c_master_controller *master = i3c_dev_get_master(dev); + struct i3c_device_info info = { + .static_addr = boardinfo->static_addr, + }; + struct i3c_dev_desc *i3cdev; int ret; - if (!dev->boardinfo || !dev->boardinfo->init_dyn_addr || - !dev->boardinfo->static_addr) + /* + * We anyway don't attach devices which are not addressable + * (no static_addr and dyn_addr) and devices with static_addr + * but no init_dyn_addr will participate in DAA. + */ + if (!boardinfo->static_addr || !boardinfo->init_dyn_addr) + return; + + i3cdev = i3c_master_alloc_i3c_dev(master, &info); + if (IS_ERR(i3cdev)) return; - ret = i3c_master_setdasa_locked(master, dev->info.static_addr, - dev->boardinfo->init_dyn_addr); + i3cdev->boardinfo = boardinfo; + + ret = i3c_master_attach_i3c_dev(master, i3cdev); if (ret) return; - dev->info.dyn_addr = dev->boardinfo->init_dyn_addr; - ret = i3c_master_reattach_i3c_dev(dev, 0); + ret = i3c_master_setdasa_locked(master, i3cdev->info.static_addr, + i3cdev->boardinfo->init_dyn_addr); if (ret) - goto err_rstdaa; + goto err_setdasa; - ret = i3c_master_retrieve_dev_info(dev); + i3cdev->info.dyn_addr = i3cdev->boardinfo->init_dyn_addr; + ret = i3c_master_reattach_i3c_dev(i3cdev, 0); if (ret) goto err_rstdaa; - return; + ret = i3c_master_retrieve_dev_info(i3cdev); + if (ret) + goto err_rstdaa; err_rstdaa: - i3c_master_rstdaa_locked(master, dev->boardinfo->init_dyn_addr); + i3c_master_rstdaa_locked(master, i3cdev->boardinfo->init_dyn_addr); +err_setdasa: + i3c_master_detach_i3c_dev(i3cdev); + i3c_master_free_i3c_dev(i3cdev); } static void @@ -1619,8 +1643,8 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master) * This function is following all initialisation steps described in the I3C * specification: * - * 1. Attach I2C and statically defined I3C devs to the master so that the - * master can fill its internal device table appropriately + * 1. Attach I2C devs to the master so that the master can fill its internal + * device table appropriately * * 2. Call &i3c_master_controller_ops->bus_init() method to initialize * the master controller. That's usually where the bus mode is selected @@ -1647,7 +1671,6 @@ 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 i2c_dev_desc *i2cdev; int ret; @@ -1679,34 +1702,6 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) goto err_detach_devs; } } - list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) { - struct i3c_device_info info = { - .static_addr = i3cboardinfo->static_addr, - }; - - if (i3cboardinfo->init_dyn_addr) { - status = i3c_bus_get_addr_slot_status(&master->bus, - i3cboardinfo->init_dyn_addr); - if (status != I3C_ADDR_SLOT_FREE) { - ret = -EBUSY; - goto err_detach_devs; - } - } - - i3cdev = i3c_master_alloc_i3c_dev(master, &info); - if (IS_ERR(i3cdev)) { - ret = PTR_ERR(i3cdev); - goto err_detach_devs; - } - - i3cdev->boardinfo = i3cboardinfo; - - ret = i3c_master_attach_i3c_dev(master, i3cdev); - if (ret) { - i3c_master_free_i3c_dev(i3cdev); - goto err_detach_devs; - } - } /* * Now execute the controller specific ->bus_init() routine, which @@ -1746,8 +1741,8 @@ 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(i3cboardinfo, &master->boardinfo.i3c, node) + i3c_master_pre_assign_dyn_addr(master, i3cboardinfo); ret = i3c_master_do_daa(master); if (ret) @@ -1811,7 +1806,7 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master, { struct i3c_device_info info = { .dyn_addr = addr }; struct i3c_dev_desc *newdev, *olddev; - u8 old_dyn_addr = addr, expected_dyn_addr; + u8 old_dyn_addr = addr; struct i3c_ibi_setup ibireq = { }; bool enable_ibi = false; int ret; @@ -1866,39 +1861,25 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master, i3c_master_free_i3c_dev(olddev); } - ret = i3c_master_reattach_i3c_dev(newdev, old_dyn_addr); - if (ret) - goto err_detach_dev; - /* * Depending on our previous state, the expected dynamic address might * differ: * - if the device already had a dynamic address assigned, let's try to - * re-apply this one - * - if the device did not have a dynamic address and the firmware - * requested a specific address, pick this one + * re-apply this one. Device with dyn_addr participated in DAA ? * - in any other case, keep the address automatically assigned by the * master */ - if (old_dyn_addr && old_dyn_addr != newdev->info.dyn_addr) - expected_dyn_addr = old_dyn_addr; - else if (newdev->boardinfo && newdev->boardinfo->init_dyn_addr) - expected_dyn_addr = newdev->boardinfo->init_dyn_addr; - else - expected_dyn_addr = newdev->info.dyn_addr; - - if (newdev->info.dyn_addr != expected_dyn_addr) { + if (old_dyn_addr && old_dyn_addr != newdev->info.dyn_addr) { /* * Try to apply the expected dynamic address. If it fails, keep * the address assigned by the master. */ ret = i3c_master_setnewda_locked(master, newdev->info.dyn_addr, - expected_dyn_addr); + old_dyn_addr); if (!ret) { - old_dyn_addr = newdev->info.dyn_addr; - newdev->info.dyn_addr = expected_dyn_addr; - i3c_master_reattach_i3c_dev(newdev, old_dyn_addr); + newdev->info.dyn_addr = old_dyn_addr; + i3c_master_reattach_i3c_dev(newdev, addr); } else { dev_err(&master->dev, "Failed to assign reserved/old address to device %d%llx", -- 2.17.1 _______________________________________________ linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c
next reply other threads:[~2020-05-14 16:30 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-14 16:30 Parshuram Thombare [this message] 2020-05-14 16:30 ` [PATCH] i3c: master: fix for SETDASA and DAA process Parshuram Thombare 2020-05-20 8:58 ` Boris Brezillon 2020-05-20 8:58 ` Boris Brezillon 2020-05-20 9:20 ` Parshuram Raju Thombare 2020-05-20 9:20 ` Parshuram Raju Thombare
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1589473809-16708-1-git-send-email-pthombar@cadence.com \ --to=pthombar@cadence.com \ --cc=bbrezillon@kernel.org \ --cc=linux-i3c@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mparab@cadence.com \ --cc=pgaj@cadence.com \ --cc=praneeth@ti.com \ --cc=vitor.soares@synopsys.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.