From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Frank Li <Frank.li@nxp.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
Boris Brezillon <boris.brezillon@collabora.com>,
Parshuram Thombare <pthombar@cadence.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Boris Brezillon <bbrezillon@kernel.org>,
Arnd Bergmann <arnd@arndb.de>,
Conor Culhane <conor.culhane@silvaco.com>,
linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org,
imx@lists.linux.dev
Subject: Re: [PATCH v3 03/11] i3c: master: Extend address status bit to 4 and add I3C_ADDR_SLOT_EXT_INIT
Date: Tue, 3 Sep 2024 15:00:38 +0200 [thread overview]
Message-ID: <20240903150038.3f224ec9@xps-13> (raw)
In-Reply-To: <ZtYCA9cYywmUaJSQ@lizhi-Precision-Tower-5810>
Hi Frank,
Frank.li@nxp.com wrote on Mon, 2 Sep 2024 14:20:51 -0400:
> On Mon, Sep 02, 2024 at 04:12:50PM +0200, Miquel Raynal wrote:
> > Hi Frank,
> >
> > > > > switch to this address if it is free.
> > > > > *
> > > > > In step 1, i3c_bus_get_free_addr() is called. To optimize for step 2b, this
> > > > > function should return an address that is not pre-reserved by any target
> > > > > device with an assigned address in the device tree (DT).
> > > >
> > > > This does not make sense, if you want to optimize for 2b, why not
> > > > selecting the assigned-address property in the first place if it's
> > > > available?
> > >
> > > This is my first idea. But I gived up this way.
> > >
> > > Select an assigned-address here will involve a big change in i3c framework.
> > > There are no PID information in i3c_master_get_free_addr().
> > >
> > > In DAA flow:
> > > - SVC is get PID first, the get_free_addr(). This case, we can use PID to
> > > get dt assigned address.(if change/add API)
> > > - But HCI, it is difference, hci_cmd_v2_daa(), get_free_addr() firstly then
> > > send out DAA command. So no PID information when call get_free_addr().
> > >
> > > To cover both case, return a *real* free address here is simplest solution.
> >
> > But this is a limitation of the HCI driver? So why not addressing this
> > in the HCI driver instead? It would greatly simplify the core logic
> > which becomes complex for wrong reasons.
>
> It is reasonable requirement to reduce stall SCL time. After get PID, SCL
> have to stall low to wait for software get dynamtic address, I3C spec allow
> relative long time for this, but still better if hardware can send out PID
> and dynamatic address together withoug stall SCL low. Pre-alloc adddress is
> good method if consider this.
I don't think it is worth the trouble, given the complexity of all
the changes. I prefer to simplify a bit the software and keep it
readable than gaining few us with SCL low. In this case you also spend
more time on the configuration I guess, so is it better than keeping
SCL low (it will be low for some time anyway).
> > > > Also, I don't understand why you would care to specifically
> > > > *not* return an address that might be the default one for another
> > > > device in the first place.
> > >
> > > If devices A (want adddress 0xA), device B (want address 0xB), if both
> > > device send hot join at the same time. device B's PID less than device A,
> > >
> > > Device B will be found firstly, call get_free_addr(), 0xA will be return
> > > if no this patch.
> > >
> > > Device A, call try_get_freeaddr() to get 0xB.
> > >
> > > So Devcie B will be assign to 0xA, and Device A will be assign to address 0xB.
> > >
> > > After do_daa command, framework will add device B and device A into i3c bus.
> > >
> > > When framework try to add device B to i3c bus, framework will try switch
> > > device B's address to 0xB from 0xA, but it will be fail because 0xB already
> > > assigned to device A.
> >
> > Well, okay, but that's exactly the situation that will happen if these
> > devices are not described in your DT. I guess it's expected that a
> > device not described in your DT can be connected, thanks to the
> > hot-join feature. In this case you cannot know what is this device
> > preferred address and you might end-up in the exact same situation.
>
> If not descript in DT, it means that any dynmatic address can be assigned.
That's the point of view of the host. But a device might be "critical"
and expect a low address, but the host not being aware. This is the
same situation as your A and B conflict example.
> > May I question the need for preferred addresses at all? Is this even
> > part of the spec? What is the use-case?
>
> It is implements detail. I3C spec said lower dynamtic address have high IBI
> priority. Spec just said assign lower dynamtic address if want to higher
> IBI prioerity. Using DT assign-address just is one implement method.
Thanks for all the information, for me the HCI driver must be modified
to retrieve the PID before assigning the dynamic address.
Thanks,
Miquèl
next prev parent reply other threads:[~2024-09-03 13:00 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-19 16:01 [PATCH v3 00/11] i3c: master: some fix and improvemnt for hotjoin Frank Li
2024-08-19 16:01 ` [PATCH v3 01/11] i3c: master: Remove i3c_dev_disable_ibi_locked(olddev) on device hotjoin Frank Li
2024-08-20 1:34 ` Stanley Chu
2024-08-20 14:45 ` Frank Li
2024-08-23 15:53 ` Miquel Raynal
2024-08-19 16:01 ` [PATCH v3 02/11] i3c: master: Replace hard code 2 with macro I3C_ADDR_SLOT_BITS Frank Li
2024-08-23 15:55 ` Miquel Raynal
2024-08-23 17:57 ` Frank Li
2024-08-26 8:05 ` Miquel Raynal
2024-08-19 16:01 ` [PATCH v3 03/11] i3c: master: Extend address status bit to 4 and add I3C_ADDR_SLOT_EXT_INIT Frank Li
2024-08-23 16:04 ` Miquel Raynal
2024-08-23 17:55 ` Frank Li
2024-08-26 8:04 ` Miquel Raynal
2024-08-26 15:56 ` Frank Li
2024-08-26 16:49 ` Miquel Raynal
2024-08-26 18:55 ` Frank Li
2024-09-02 14:12 ` Miquel Raynal
2024-09-02 18:20 ` Frank Li
2024-09-03 13:00 ` Miquel Raynal [this message]
2024-09-03 15:06 ` Frank Li
2024-09-09 20:01 ` Frank Li
2024-09-16 15:14 ` Frank Li
2024-08-19 16:01 ` [PATCH v3 04/11] i3c: master: Fix dynamic address leak when 'assigned-address' is present Frank Li
2024-08-19 16:01 ` [PATCH v3 05/11] i3c: master: Fix miss free init_dyn_addr at i3c_master_put_i3c_addrs() Frank Li
2024-08-23 16:07 ` Miquel Raynal
2024-08-19 16:02 ` [PATCH v3 06/11] i3c: master: svc: use repeat start when IBI WIN happens Frank Li
2024-08-23 16:09 ` Miquel Raynal
2024-08-19 16:02 ` [PATCH v3 07/11] i3c: master: svc: manually emit NACK/ACK for hotjoin Frank Li
2024-08-23 16:10 ` Miquel Raynal
2024-08-19 16:02 ` [PATCH v3 08/11] i3c: master: svc: need check IBIWON for dynamtica address assign Frank Li
2024-08-23 16:12 ` Miquel Raynal
2024-08-19 16:02 ` [PATCH v3 09/11] i3c: master: svc: use spinlock_saveirq at svc_i3c_master_ibi_work() Frank Li
2024-08-23 16:19 ` Miquel Raynal
2024-08-23 16:53 ` Frank Li
2024-08-19 16:02 ` [PATCH v3 10/11] i3c: master: svc: wait for Manual ACK/NACK Done before next step Frank Li
2024-08-23 16:22 ` Miquel Raynal
2024-08-23 16:45 ` Frank Li
2024-08-19 16:02 ` [PATCH v3 11/11] i3c: master: svc: fix possible assignment of the same address to two devices Frank Li
2024-08-23 16:24 ` Miquel Raynal
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=20240903150038.3f224ec9@xps-13 \
--to=miquel.raynal@bootlin.com \
--cc=Frank.li@nxp.com \
--cc=alexandre.belloni@bootlin.com \
--cc=arnd@arndb.de \
--cc=bbrezillon@kernel.org \
--cc=boris.brezillon@collabora.com \
--cc=conor.culhane@silvaco.com \
--cc=gregkh@linuxfoundation.org \
--cc=imx@lists.linux.dev \
--cc=linux-i3c@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pthombar@cadence.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox