* [PATCH] i3c: master: svc: fix signed/unsigned mismatch in dynamic address assignment
@ 2025-03-09 16:43 Qasim Ijaz
0 siblings, 0 replies; 6+ messages in thread
From: Qasim Ijaz @ 2025-03-09 16:43 UTC (permalink / raw)
To: miquel.raynal, Frank.Li, alexandre.belloni; +Cc: linux-i3c, imx, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1605 bytes --]
svc_i3c_master_do_daa_locked() declares dyn_addr as an unsigned int
however it initialises it with i3c_master_get_free_addr() which
returns a signed int type and then attempts to check if dyn_addr is
less than 0. Unsigned integers cannot be less than 0, so the check
is essentially redundant. Furthermore i3c_master_get_free_addr()
could return -ENOMEM which an unsigned int cannot store.
Fix this by capturing the return value of i3c_master_get_free_addr()
in a signed int ‘dyn_addr_ret’. If that value is negative, return
an error. Otherwise, assign it to the unsigned int ‘dyn_addr’ once
we know it’s valid.
Fixes: 4008a74e0f9b ("i3c: master: svc: Fix npcm845 FIFO empty issue")
Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
---
drivers/i3c/master/svc-i3c-master.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index f22fb9e75142..eea08f00d7ce 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -998,9 +998,11 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
* filling within a few hundred nanoseconds, which is significantly
* faster compared to the 64 SCL clock cycles.
*/
- dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1);
- if (dyn_addr < 0)
+ int dyn_addr_ret = i3c_master_get_free_addr(&master->base, last_addr + 1);
+
+ if (dyn_addr_ret < 0)
return -ENOSPC;
+ dyn_addr = dyn_addr_ret;
writel(dyn_addr, master->regs + SVC_I3C_MWDATAB);
--
2.39.5
[-- Attachment #2: Type: text/plain, Size: 111 bytes --]
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] i3c: master: svc: fix signed/unsigned mismatch in dynamic address assignment
@ 2025-03-17 10:15 Qasim Ijaz
2025-03-18 13:40 ` Frank Li
0 siblings, 1 reply; 6+ messages in thread
From: Qasim Ijaz @ 2025-03-17 10:15 UTC (permalink / raw)
To: miquel.raynal, Frank.Li, alexandre.belloni; +Cc: linux-i3c, imx, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1597 bytes --]
svc_i3c_master_do_daa_locked() declares dyn_addr as an unsigned int
however it initialises it with i3c_master_get_free_addr() which
returns a signed int type and then attempts to check if dyn_addr is
less than 0. Unsigned integers cannot be less than 0, so the check
is essentially redundant. Furthermore i3c_master_get_free_addr()
could return -ENOMEM which an unsigned int cannot store.
Fix this by capturing the return value of i3c_master_get_free_addr()
in a signed int ‘dyn_addr_ret’. If that value is negative, return
an error. Otherwise, assign it to the unsigned int ‘dyn_addr’ once
we know it’s valid.
Fixes: 4008a74e0f9b ("i3c: master: svc: Fix npcm845 FIFO empty issue")
Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
---
drivers/i3c/master/svc-i3c-master.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index f22fb9e75142..eea08f00d7ce 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -998,9 +998,10 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
* filling within a few hundred nanoseconds, which is significantly
* faster compared to the 64 SCL clock cycles.
*/
- dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1);
- if (dyn_addr < 0)
+ int dyn_addr_ret = i3c_master_get_free_addr(&master->base, last_addr + 1);
+ if (dyn_addr_ret < 0)
return -ENOSPC;
+ dyn_addr = dyn_addr_ret;
writel(dyn_addr, master->regs + SVC_I3C_MWDATAB);
--
2.39.5
[-- Attachment #2: Type: text/plain, Size: 111 bytes --]
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] i3c: master: svc: fix signed/unsigned mismatch in dynamic address assignment
2025-03-17 10:15 Qasim Ijaz
@ 2025-03-18 13:40 ` Frank Li
2025-03-18 14:41 ` Qasim Ijaz
0 siblings, 1 reply; 6+ messages in thread
From: Frank Li @ 2025-03-18 13:40 UTC (permalink / raw)
To: Qasim Ijaz; +Cc: miquel.raynal, alexandre.belloni, linux-i3c, imx, linux-kernel
On Mon, Mar 17, 2025 at 10:15:16AM +0000, Qasim Ijaz wrote:
> svc_i3c_master_do_daa_locked() declares dyn_addr as an unsigned int
> however it initialises it with i3c_master_get_free_addr() which
> returns a signed int type and then attempts to check if dyn_addr is
> less than 0. Unsigned integers cannot be less than 0, so the check
> is essentially redundant. Furthermore i3c_master_get_free_addr()
> could return -ENOMEM which an unsigned int cannot store.
>
> Fix this by capturing the return value of i3c_master_get_free_addr()
> in a signed int ‘dyn_addr_ret’. If that value is negative, return
> an error. Otherwise, assign it to the unsigned int ‘dyn_addr’ once
> we know it’s valid.
>
> Fixes: 4008a74e0f9b ("i3c: master: svc: Fix npcm845 FIFO empty issue")
> Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
> ---
Thank you for your patch, but similar one was already applied
https://lore.kernel.org/linux-i3c/174225158210.1593610.10018812780731849409.b4-ty@bootlin.com/T/#m5120e1c362e7e57f4cab139a45410fde421c2f37
Frank
> drivers/i3c/master/svc-i3c-master.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index f22fb9e75142..eea08f00d7ce 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -998,9 +998,10 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
> * filling within a few hundred nanoseconds, which is significantly
> * faster compared to the 64 SCL clock cycles.
> */
> - dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1);
> - if (dyn_addr < 0)
> + int dyn_addr_ret = i3c_master_get_free_addr(&master->base, last_addr + 1);
> + if (dyn_addr_ret < 0)
> return -ENOSPC;
> + dyn_addr = dyn_addr_ret;
>
> writel(dyn_addr, master->regs + SVC_I3C_MWDATAB);
>
> --
> 2.39.5
>
--
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* Re: [PATCH] i3c: master: svc: fix signed/unsigned mismatch in dynamic address assignment
2025-03-18 13:40 ` Frank Li
@ 2025-03-18 14:41 ` Qasim Ijaz
2025-03-19 13:18 ` Frank Li
0 siblings, 1 reply; 6+ messages in thread
From: Qasim Ijaz @ 2025-03-18 14:41 UTC (permalink / raw)
To: Frank Li; +Cc: miquel.raynal, alexandre.belloni, linux-i3c, imx, linux-kernel
On Tue, Mar 18, 2025 at 09:40:17AM -0400, Frank Li wrote:
> On Mon, Mar 17, 2025 at 10:15:16AM +0000, Qasim Ijaz wrote:
> > svc_i3c_master_do_daa_locked() declares dyn_addr as an unsigned int
> > however it initialises it with i3c_master_get_free_addr() which
> > returns a signed int type and then attempts to check if dyn_addr is
> > less than 0. Unsigned integers cannot be less than 0, so the check
> > is essentially redundant. Furthermore i3c_master_get_free_addr()
> > could return -ENOMEM which an unsigned int cannot store.
> >
> > Fix this by capturing the return value of i3c_master_get_free_addr()
> > in a signed int ‘dyn_addr_ret’. If that value is negative, return
> > an error. Otherwise, assign it to the unsigned int ‘dyn_addr’ once
> > we know it’s valid.
> >
> > Fixes: 4008a74e0f9b ("i3c: master: svc: Fix npcm845 FIFO empty issue")
> > Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
> > ---
>
> Thank you for your patch, but similar one was already applied
> https://lore.kernel.org/linux-i3c/174225158210.1593610.10018812780731849409.b4-ty@bootlin.com/T/#m5120e1c362e7e57f4cab139a45410fde421c2f37
>
Hi Frank
I sent a fix for this issue on the 9th March, which is before the patch
you linked which was sent on the 10th March.
You can view my orignal patch here:
https://lore.kernel.org/all/20250309164314.15039-1-qasdev00@gmail.com/
Thanks
Qasim
> Frank
> > drivers/i3c/master/svc-i3c-master.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> > index f22fb9e75142..eea08f00d7ce 100644
> > --- a/drivers/i3c/master/svc-i3c-master.c
> > +++ b/drivers/i3c/master/svc-i3c-master.c
> > @@ -998,9 +998,10 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
> > * filling within a few hundred nanoseconds, which is significantly
> > * faster compared to the 64 SCL clock cycles.
> > */
> > - dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1);
> > - if (dyn_addr < 0)
> > + int dyn_addr_ret = i3c_master_get_free_addr(&master->base, last_addr + 1);
> > + if (dyn_addr_ret < 0)
> > return -ENOSPC;
> > + dyn_addr = dyn_addr_ret;
> >
> > writel(dyn_addr, master->regs + SVC_I3C_MWDATAB);
> >
> > --
> > 2.39.5
> >
--
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* Re: [PATCH] i3c: master: svc: fix signed/unsigned mismatch in dynamic address assignment
2025-03-18 14:41 ` Qasim Ijaz
@ 2025-03-19 13:18 ` Frank Li
2025-03-20 22:01 ` Alexandre Belloni
0 siblings, 1 reply; 6+ messages in thread
From: Frank Li @ 2025-03-19 13:18 UTC (permalink / raw)
To: Qasim Ijaz; +Cc: miquel.raynal, alexandre.belloni, linux-i3c, imx, linux-kernel
On Tue, Mar 18, 2025 at 02:41:47PM +0000, Qasim Ijaz wrote:
> On Tue, Mar 18, 2025 at 09:40:17AM -0400, Frank Li wrote:
> > On Mon, Mar 17, 2025 at 10:15:16AM +0000, Qasim Ijaz wrote:
> > > svc_i3c_master_do_daa_locked() declares dyn_addr as an unsigned int
> > > however it initialises it with i3c_master_get_free_addr() which
> > > returns a signed int type and then attempts to check if dyn_addr is
> > > less than 0. Unsigned integers cannot be less than 0, so the check
> > > is essentially redundant. Furthermore i3c_master_get_free_addr()
> > > could return -ENOMEM which an unsigned int cannot store.
> > >
> > > Fix this by capturing the return value of i3c_master_get_free_addr()
> > > in a signed int ‘dyn_addr_ret’. If that value is negative, return
> > > an error. Otherwise, assign it to the unsigned int ‘dyn_addr’ once
> > > we know it’s valid.
> > >
> > > Fixes: 4008a74e0f9b ("i3c: master: svc: Fix npcm845 FIFO empty issue")
> > > Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
> > > ---
> >
> > Thank you for your patch, but similar one was already applied
> > https://lore.kernel.org/linux-i3c/174225158210.1593610.10018812780731849409.b4-ty@bootlin.com/T/#m5120e1c362e7e57f4cab139a45410fde421c2f37
> >
>
> Hi Frank
>
> I sent a fix for this issue on the 9th March, which is before the patch
> you linked which was sent on the 10th March.
Yes, but perfer original owner to fix this type minor fix.
https://lore.kernel.org/linux-i3c/174129444617.1163689.11942276093411687387.b4-ty@bootlin.com/T/#t
Frank
>
> You can view my orignal patch here:
>
> https://lore.kernel.org/all/20250309164314.15039-1-qasdev00@gmail.com/
>
> Thanks
> Qasim
> > Frank
> > > drivers/i3c/master/svc-i3c-master.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> > > index f22fb9e75142..eea08f00d7ce 100644
> > > --- a/drivers/i3c/master/svc-i3c-master.c
> > > +++ b/drivers/i3c/master/svc-i3c-master.c
> > > @@ -998,9 +998,10 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
> > > * filling within a few hundred nanoseconds, which is significantly
> > > * faster compared to the 64 SCL clock cycles.
> > > */
> > > - dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1);
> > > - if (dyn_addr < 0)
> > > + int dyn_addr_ret = i3c_master_get_free_addr(&master->base, last_addr + 1);
> > > + if (dyn_addr_ret < 0)
> > > return -ENOSPC;
> > > + dyn_addr = dyn_addr_ret;
> > >
> > > writel(dyn_addr, master->regs + SVC_I3C_MWDATAB);
> > >
> > > --
> > > 2.39.5
> > >
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
--
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* Re: [PATCH] i3c: master: svc: fix signed/unsigned mismatch in dynamic address assignment
2025-03-19 13:18 ` Frank Li
@ 2025-03-20 22:01 ` Alexandre Belloni
0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Belloni @ 2025-03-20 22:01 UTC (permalink / raw)
To: Frank Li; +Cc: Qasim Ijaz, miquel.raynal, linux-i3c, imx, linux-kernel
On 19/03/2025 09:18:52-0400, Frank Li wrote:
> On Tue, Mar 18, 2025 at 02:41:47PM +0000, Qasim Ijaz wrote:
> > On Tue, Mar 18, 2025 at 09:40:17AM -0400, Frank Li wrote:
> > > On Mon, Mar 17, 2025 at 10:15:16AM +0000, Qasim Ijaz wrote:
> > > > svc_i3c_master_do_daa_locked() declares dyn_addr as an unsigned int
> > > > however it initialises it with i3c_master_get_free_addr() which
> > > > returns a signed int type and then attempts to check if dyn_addr is
> > > > less than 0. Unsigned integers cannot be less than 0, so the check
> > > > is essentially redundant. Furthermore i3c_master_get_free_addr()
> > > > could return -ENOMEM which an unsigned int cannot store.
> > > >
> > > > Fix this by capturing the return value of i3c_master_get_free_addr()
> > > > in a signed int ‘dyn_addr_ret’. If that value is negative, return
> > > > an error. Otherwise, assign it to the unsigned int ‘dyn_addr’ once
> > > > we know it’s valid.
> > > >
> > > > Fixes: 4008a74e0f9b ("i3c: master: svc: Fix npcm845 FIFO empty issue")
> > > > Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
> > > > ---
> > >
> > > Thank you for your patch, but similar one was already applied
> > > https://lore.kernel.org/linux-i3c/174225158210.1593610.10018812780731849409.b4-ty@bootlin.com/T/#m5120e1c362e7e57f4cab139a45410fde421c2f37
> > >
> >
> > Hi Frank
> >
> > I sent a fix for this issue on the 9th March, which is before the patch
> > you linked which was sent on the 10th March.
>
> Yes, but perfer original owner to fix this type minor fix.
>
This is absolutely not what I said, the first one that is sent and is
applicable should be applied.
See how you didn't fix this trivial issue:
https://lore.kernel.org/linux-i3c/20250319-i3c-fix-clang-fallthrough-v1-1-d8e02be1ef5c@kernel.org/T/#u
> https://lore.kernel.org/linux-i3c/174129444617.1163689.11942276093411687387.b4-ty@bootlin.com/T/#t
>
> Frank
> >
> > You can view my orignal patch here:
> >
> > https://lore.kernel.org/all/20250309164314.15039-1-qasdev00@gmail.com/
> >
> > Thanks
> > Qasim
> > > Frank
> > > > drivers/i3c/master/svc-i3c-master.c | 5 +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> > > > index f22fb9e75142..eea08f00d7ce 100644
> > > > --- a/drivers/i3c/master/svc-i3c-master.c
> > > > +++ b/drivers/i3c/master/svc-i3c-master.c
> > > > @@ -998,9 +998,10 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
> > > > * filling within a few hundred nanoseconds, which is significantly
> > > > * faster compared to the 64 SCL clock cycles.
> > > > */
> > > > - dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1);
> > > > - if (dyn_addr < 0)
> > > > + int dyn_addr_ret = i3c_master_get_free_addr(&master->base, last_addr + 1);
> > > > + if (dyn_addr_ret < 0)
> > > > return -ENOSPC;
> > > > + dyn_addr = dyn_addr_ret;
> > > >
> > > > writel(dyn_addr, master->regs + SVC_I3C_MWDATAB);
> > > >
> > > > --
> > > > 2.39.5
> > > >
> >
> > --
> > linux-i3c mailing list
> > linux-i3c@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-i3c
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
--
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, other threads:[~2025-03-20 22:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-09 16:43 [PATCH] i3c: master: svc: fix signed/unsigned mismatch in dynamic address assignment Qasim Ijaz
-- strict thread matches above, loose matches on Subject: below --
2025-03-17 10:15 Qasim Ijaz
2025-03-18 13:40 ` Frank Li
2025-03-18 14:41 ` Qasim Ijaz
2025-03-19 13:18 ` Frank Li
2025-03-20 22:01 ` Alexandre Belloni
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).