| Message ID | 20240829-i3c_fix-v4-0-ebcbd5efceba@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | i3c: master: some fix and improvemnt for hotjoin | expand |
On Thu, Aug 29, 2024 at 05:13:57PM -0400, Frank Li wrote: > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- Alex: https://lore.kernel.org/linux-i3c/20240819-i3c_fix-v3-0-7d69f7b0a05e@nxp.com/T/#m16fa9bb875b0ae9d37c5f6e91f90e375551c6366 There is some discuss about assiged address, I already explain the reason and ping 3 times, These patch are actual fixed hot join issues. How move forward? best regard Frank Li > Changes in v4: > - See each patch > - Link to v3: https://lore.kernel.org/r/20240819-i3c_fix-v3-0-7d69f7b0a05e@nxp.com > > Changes in v3: > - Fix build warning > kernel test robot noticed the following build warnings: > > url: https://github.com/intel-lab-lkp/linux/commits/Frank-Li/i3c-master-Remove-i3c_dev_disable_ibi_locked-olddev-on-device-hotjoin/20240814-234209 > base: 41c196e567fb1ea97f68a2ffb7faab451cd90854 > patch link: https://lore.kernel.org/r/20240813-i3c_fix-v2-10-68fe4a050188%40nxp.com > patch subject: [PATCH v2 10/11] i3c: master: svc: wait for Manual ACK/NACK Done before next step > config: x86_64-randconfig-161-20240817 (https://download.01.org/0day-ci/archive/20240818/202408180012.ifcIOjgX-lkp@intel.com/config) > compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > | Closes: https://lore.kernel.org/r/202408180012.ifcIOjgX-lkp@intel.com/ > > - Link to v2: https://lore.kernel.org/r/20240813-i3c_fix-v2-0-68fe4a050188@nxp.com > > Changes in v2: > - add help function at i3c: master: svc: manually emit NACK/ACK for hotjoin F > Add below new fix patch > i3c: master: svc: fix possible assignment of the same address to two devices > i3c: master: svc: wait for Manual ACK/NACK Done before next step > i3c: master: svc: use spinlock_saveirq at svc_i3c_master_ibi_work() > i3c: master: svc: need check IBIWON for dynamtica address assign > > - Link to v1: https://lore.kernel.org/r/20240724-i3c_fix-v1-0-bfa500b023d6@nxp.com > > --- > Frank Li (11): > i3c: master: Remove i3c_dev_disable_ibi_locked(olddev) on device hotjoin > i3c: master: Replace hard code 2 with macro I3C_ADDR_SLOT_STATUS_BITS > i3c: master: Extend address status bit to 4 and add I3C_ADDR_SLOT_EXT_INIT > i3c: master: Fix dynamic address leak when 'assigned-address' is present > i3c: master: Fix miss free init_dyn_addr at i3c_master_put_i3c_addrs() > i3c: master: svc: use repeat start when IBI WIN happens > i3c: master: svc: manually emit NACK/ACK for hotjoin > i3c: master: svc: need check IBIWON for dynamtica address assign > i3c: master: svc: use spin_lock_irqsave at svc_i3c_master_ibi_work() > i3c: master: svc: wait for Manual ACK/NACK Done before next step > i3c: master: svc: fix possible assignment of the same address to two devices > > drivers/i3c/master.c | 100 +++++++++++++++++++++-------- > drivers/i3c/master/svc-i3c-master.c | 122 +++++++++++++++++++++++++++--------- > include/linux/i3c/master.h | 9 ++- > 3 files changed, 175 insertions(+), 56 deletions(-) > --- > base-commit: f2b9f0aeff2b3bb0446c955f0d8fac7659644c75 > change-id: 20240724-i3c_fix-371bf8fa9e00 > > Best regards, > --- > Frank Li <Frank.Li@nxp.com> >
Hi Frank, Frank.li@nxp.com wrote on Thu, 26 Sep 2024 13:02:06 -0400: > On Thu, Aug 29, 2024 at 05:13:57PM -0400, Frank Li wrote: > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > Alex: > > https://lore.kernel.org/linux-i3c/20240819-i3c_fix-v3-0-7d69f7b0a05e@nxp.com/T/#m16fa9bb875b0ae9d37c5f6e91f90e375551c6366 > > There is some discuss about assiged address, I already explain the reason > and ping 3 times, Pinging will not help as long as the code is not clear and simplified. I honestly think the PID should be retrieved earlier in the HCI part and that would solve most of your issues. Otherwise if that is not possible I still find hard to understand the big picture, the comments and the code. I've already spent quite a bit of time trying to improve it with you, but the logic is still a little bit too specific and complex from my point of view. > These patch are actual fixed hot join issues. It should not be the case. Hot-join should work without the devices having all the time the preferred address. If hot-join really does not work, then please split the series with the "preferred address" handling being done apart in a second series. Thanks, Miquèl
On Mon, Sep 30, 2024 at 11:34:36AM +0200, Miquel Raynal wrote: > Hi Frank, > > Frank.li@nxp.com wrote on Thu, 26 Sep 2024 13:02:06 -0400: > > > On Thu, Aug 29, 2024 at 05:13:57PM -0400, Frank Li wrote: > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > --- > > > > Alex: > > > > https://lore.kernel.org/linux-i3c/20240819-i3c_fix-v3-0-7d69f7b0a05e@nxp.com/T/#m16fa9bb875b0ae9d37c5f6e91f90e375551c6366 > > > > There is some discuss about assiged address, I already explain the reason > > and ping 3 times, > > Pinging will not help as long as the code is not clear and simplified. The problem is that no reply after I posted last reply and ping 3 times have not response about it. > I honestly think the PID should be retrieved earlier in the HCI part > and that would solve most of your issues. If there are problem or difference opinion, we can continue to discuss it. But no reply will stop the whole thing move forward. I3C HCI Spec 1.2, sec 6.4.1, when do DAA, "DAA CMD and dynmatic address" queue to cmd together. We don't know PID before DAA CMD. So dynmatic address can NOT get based on PID. If I am wrong about HCI, let me know since I have not worked HCI before. > > Otherwise if that is not possible I still find hard to understand the > big picture, the comments and the code. I've already spent quite a bit > of time trying to improve it with you, but the logic is still a little > bit too specific and complex from my point of view. > > > These patch are actual fixed hot join issues. > > It should not be the case. Hot-join should work without the devices > having all the time the preferred address. If hot-join really does > not work, then please split the series with the "preferred address" > handling being done apart in a second series. I can do that, could you check svc part and most already acked you? I can post svc part only later by addressing your comments if you have. Frank > > Thanks, > Miquèl
On Thu, Aug 29, 2024 at 05:13:57PM -0400, Frank Li wrote: > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- Alex: you can superseded this serie. According to mirquel's suggestion, I splitted into 4 parts: 1. svc driver fix patches: https://lore.kernel.org/imx/ZvwpMBWzNjTXpPZF@lizhi-Precision-Tower-5810/T/#t 2. i3c master fix patches with miquel's ack a: https://lore.kernel.org/imx/20241001162608.224039-1-Frank.Li@nxp.com/T/#u b: https://lore.kernel.org/imx/20241001162232.223724-1-Frank.Li@nxp.com/T/#u 3. ongoing discussion on about dt assigne address: https://lore.kernel.org/imx/20241001-i3c_dts_assign-v1-0-6ba83dc15eb8@nxp.com/T/#t I hope above help you manage these patches. Frank > Changes in v4: > - See each patch > - Link to v3: https://lore.kernel.org/r/20240819-i3c_fix-v3-0-7d69f7b0a05e@nxp.com > > Changes in v3: > - Fix build warning > kernel test robot noticed the following build warnings: > > url: https://github.com/intel-lab-lkp/linux/commits/Frank-Li/i3c-master-Remove-i3c_dev_disable_ibi_locked-olddev-on-device-hotjoin/20240814-234209 > base: 41c196e567fb1ea97f68a2ffb7faab451cd90854 > patch link: https://lore.kernel.org/r/20240813-i3c_fix-v2-10-68fe4a050188%40nxp.com > patch subject: [PATCH v2 10/11] i3c: master: svc: wait for Manual ACK/NACK Done before next step > config: x86_64-randconfig-161-20240817 (https://download.01.org/0day-ci/archive/20240818/202408180012.ifcIOjgX-lkp@intel.com/config) > compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > | Closes: https://lore.kernel.org/r/202408180012.ifcIOjgX-lkp@intel.com/ > > - Link to v2: https://lore.kernel.org/r/20240813-i3c_fix-v2-0-68fe4a050188@nxp.com > > Changes in v2: > - add help function at i3c: master: svc: manually emit NACK/ACK for hotjoin F > Add below new fix patch > i3c: master: svc: fix possible assignment of the same address to two devices > i3c: master: svc: wait for Manual ACK/NACK Done before next step > i3c: master: svc: use spinlock_saveirq at svc_i3c_master_ibi_work() > i3c: master: svc: need check IBIWON for dynamtica address assign > > - Link to v1: https://lore.kernel.org/r/20240724-i3c_fix-v1-0-bfa500b023d6@nxp.com > > --- > Frank Li (11): > i3c: master: Remove i3c_dev_disable_ibi_locked(olddev) on device hotjoin > i3c: master: Replace hard code 2 with macro I3C_ADDR_SLOT_STATUS_BITS > i3c: master: Extend address status bit to 4 and add I3C_ADDR_SLOT_EXT_INIT > i3c: master: Fix dynamic address leak when 'assigned-address' is present > i3c: master: Fix miss free init_dyn_addr at i3c_master_put_i3c_addrs() > i3c: master: svc: use repeat start when IBI WIN happens > i3c: master: svc: manually emit NACK/ACK for hotjoin > i3c: master: svc: need check IBIWON for dynamtica address assign > i3c: master: svc: use spin_lock_irqsave at svc_i3c_master_ibi_work() > i3c: master: svc: wait for Manual ACK/NACK Done before next step > i3c: master: svc: fix possible assignment of the same address to two devices > > drivers/i3c/master.c | 100 +++++++++++++++++++++-------- > drivers/i3c/master/svc-i3c-master.c | 122 +++++++++++++++++++++++++++--------- > include/linux/i3c/master.h | 9 ++- > 3 files changed, 175 insertions(+), 56 deletions(-) > --- > base-commit: f2b9f0aeff2b3bb0446c955f0d8fac7659644c75 > change-id: 20240724-i3c_fix-371bf8fa9e00 > > Best regards, > --- > Frank Li <Frank.Li@nxp.com> >
Signed-off-by: Frank Li <Frank.Li@nxp.com> --- Changes in v4: - See each patch - Link to v3: https://lore.kernel.org/r/20240819-i3c_fix-v3-0-7d69f7b0a05e@nxp.com Changes in v3: - Fix build warning kernel test robot noticed the following build warnings: url: https://github.com/intel-lab-lkp/linux/commits/Frank-Li/i3c-master-Remove-i3c_dev_disable_ibi_locked-olddev-on-device-hotjoin/20240814-234209 base: 41c196e567fb1ea97f68a2ffb7faab451cd90854 patch link: https://lore.kernel.org/r/20240813-i3c_fix-v2-10-68fe4a050188%40nxp.com patch subject: [PATCH v2 10/11] i3c: master: svc: wait for Manual ACK/NACK Done before next step config: x86_64-randconfig-161-20240817 (https://download.01.org/0day-ci/archive/20240818/202408180012.ifcIOjgX-lkp@intel.com/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> | Closes: https://lore.kernel.org/r/202408180012.ifcIOjgX-lkp@intel.com/ - Link to v2: https://lore.kernel.org/r/20240813-i3c_fix-v2-0-68fe4a050188@nxp.com Changes in v2: - add help function at i3c: master: svc: manually emit NACK/ACK for hotjoin F Add below new fix patch i3c: master: svc: fix possible assignment of the same address to two devices i3c: master: svc: wait for Manual ACK/NACK Done before next step i3c: master: svc: use spinlock_saveirq at svc_i3c_master_ibi_work() i3c: master: svc: need check IBIWON for dynamtica address assign - Link to v1: https://lore.kernel.org/r/20240724-i3c_fix-v1-0-bfa500b023d6@nxp.com --- Frank Li (11): i3c: master: Remove i3c_dev_disable_ibi_locked(olddev) on device hotjoin i3c: master: Replace hard code 2 with macro I3C_ADDR_SLOT_STATUS_BITS i3c: master: Extend address status bit to 4 and add I3C_ADDR_SLOT_EXT_INIT i3c: master: Fix dynamic address leak when 'assigned-address' is present i3c: master: Fix miss free init_dyn_addr at i3c_master_put_i3c_addrs() i3c: master: svc: use repeat start when IBI WIN happens i3c: master: svc: manually emit NACK/ACK for hotjoin i3c: master: svc: need check IBIWON for dynamtica address assign i3c: master: svc: use spin_lock_irqsave at svc_i3c_master_ibi_work() i3c: master: svc: wait for Manual ACK/NACK Done before next step i3c: master: svc: fix possible assignment of the same address to two devices drivers/i3c/master.c | 100 +++++++++++++++++++++-------- drivers/i3c/master/svc-i3c-master.c | 122 +++++++++++++++++++++++++++--------- include/linux/i3c/master.h | 9 ++- 3 files changed, 175 insertions(+), 56 deletions(-) --- base-commit: f2b9f0aeff2b3bb0446c955f0d8fac7659644c75 change-id: 20240724-i3c_fix-371bf8fa9e00 Best regards, --- Frank Li <Frank.Li@nxp.com>