LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] firewire: ohci: make reg_(read|write) unsigned
@ 2021-07-31 10:41 Jordy Zomer
  2021-08-01  6:24 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Jordy Zomer @ 2021-07-31 10:41 UTC (permalink / raw)
  To: linux-kernel, linux1394-devel
  Cc: Greg Kroah-Hartman, Jordy Zomer, Stefan Richter

The reg_(read|write) functions used to
take a signed integer as an offset parameter.
The callers of this function only pass an unsigned integer to it.
Therefore to make it obviously safe, let's just make this an unsgined
integer as this is used in pointer arithmetics.

Signed-off-by: Jordy Zomer <jordy@pwning.systems>
---
 drivers/firewire/ohci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 17c9d825188b..0119aa9cbc7c 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -524,12 +524,12 @@ static void log_ar_at_event(struct fw_ohci *ohci,
 	}
 }
 
-static inline void reg_write(const struct fw_ohci *ohci, int offset, u32 data)
+static inline void reg_write(const struct fw_ohci *ohci, u32 offset, u32 data)
 {
 	writel(data, ohci->registers + offset);
 }
 
-static inline u32 reg_read(const struct fw_ohci *ohci, int offset)
+static inline u32 reg_read(const struct fw_ohci *ohci, u32 offset)
 {
 	return readl(ohci->registers + offset);
 }
-- 
2.27.0


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

* Re: [PATCH] firewire: ohci: make reg_(read|write) unsigned
  2021-07-31 10:41 [PATCH] firewire: ohci: make reg_(read|write) unsigned Jordy Zomer
@ 2021-08-01  6:24 ` Greg Kroah-Hartman
  2021-08-01  6:25   ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2021-08-01  6:24 UTC (permalink / raw)
  To: Jordy Zomer; +Cc: linux-kernel, linux1394-devel, Stefan Richter

On Sat, Jul 31, 2021 at 12:41:12PM +0200, Jordy Zomer wrote:
> The reg_(read|write) functions used to
> take a signed integer as an offset parameter.
> The callers of this function only pass an unsigned integer to it.
> Therefore to make it obviously safe, let's just make this an unsgined
> integer as this is used in pointer arithmetics.
> 
> Signed-off-by: Jordy Zomer <jordy@pwning.systems>
> ---
>  drivers/firewire/ohci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Same thing should probably also be done in
drivers/firewire/init_ohci1394_dma.c for the same inline functions,
right?

Anyway, nice cleanup:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

* Re: [PATCH] firewire: ohci: make reg_(read|write) unsigned
  2021-08-01  6:24 ` Greg Kroah-Hartman
@ 2021-08-01  6:25   ` Greg Kroah-Hartman
  2021-08-01 13:30     ` Stefan Richter
  0 siblings, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2021-08-01  6:25 UTC (permalink / raw)
  To: Jordy Zomer; +Cc: linux-kernel, linux1394-devel, Stefan Richter

On Sun, Aug 01, 2021 at 08:24:04AM +0200, Greg Kroah-Hartman wrote:
> On Sat, Jul 31, 2021 at 12:41:12PM +0200, Jordy Zomer wrote:
> > The reg_(read|write) functions used to
> > take a signed integer as an offset parameter.
> > The callers of this function only pass an unsigned integer to it.
> > Therefore to make it obviously safe, let's just make this an unsgined
> > integer as this is used in pointer arithmetics.
> > 
> > Signed-off-by: Jordy Zomer <jordy@pwning.systems>
> > ---
> >  drivers/firewire/ohci.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Same thing should probably also be done in
> drivers/firewire/init_ohci1394_dma.c for the same inline functions,
> right?

And sound/firewire/isight.c also could use this.  Seems like there was
some copy/paste in firewire drivers :)

thanks,

greg k-h

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

* Re: [PATCH] firewire: ohci: make reg_(read|write) unsigned
  2021-08-01  6:25   ` Greg Kroah-Hartman
@ 2021-08-01 13:30     ` Stefan Richter
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Richter @ 2021-08-01 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jordy Zomer, linux-kernel, linux1394-devel

On Aug 01 Greg Kroah-Hartman wrote:
> On Sun, Aug 01, 2021 at 08:24:04AM +0200, Greg Kroah-Hartman wrote:
> > On Sat, Jul 31, 2021 at 12:41:12PM +0200, Jordy Zomer wrote:  
> > > The reg_(read|write) functions used to
> > > take a signed integer as an offset parameter.
> > > The callers of this function only pass an unsigned integer to it.
> > > Therefore to make it obviously safe, let's just make this an unsgined
> > > integer as this is used in pointer arithmetics.
> > > 
> > > Signed-off-by: Jordy Zomer <jordy@pwning.systems>
> > > ---
> > >  drivers/firewire/ohci.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)  
> > 
> > Same thing should probably also be done in
> > drivers/firewire/init_ohci1394_dma.c for the same inline functions,
> > right?  

Yes, register offsets are always non-negative; the lowest register address
is used as the base address.  All of the offset values are taken from
macros which are define in ohci.h.

> And sound/firewire/isight.c also could use this.  Seems like there was
> some copy/paste in firewire drivers :)

These offsets are non-negative too; they defined as macros at the top of
isight.c.  However, here we aren't in the 32 bit (?) PCI register space but
in the 48 bit FireWire node address space, which is why the functions which
are wrapped up by reg_read/write() --- snd_fw_transaction() and
fw_run_transaction() --- use u64 or unsigned long long for @offset.

Long story short, isight.c::reg_read/write() could use u32 or unsigned int
or u64 or unsigned long long for @offset; it's going to be added to an u64
so maybe that's what we should use right away?
-- 
Stefan Richter
-======--=-= =--- ----=
http://arcgraph.de/sr/

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

end of thread, other threads:[~2021-08-01 13:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-31 10:41 [PATCH] firewire: ohci: make reg_(read|write) unsigned Jordy Zomer
2021-08-01  6:24 ` Greg Kroah-Hartman
2021-08-01  6:25   ` Greg Kroah-Hartman
2021-08-01 13:30     ` Stefan Richter

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox