Patchwork [RFC,4/7] sparc: rename hw/iommu.c

login
register
mail settings
Submitter Eduard - Gabriel Munteanu
Date March 30, 2010, 8:20 a.m.
Message ID <d9001abc473c4c13180637d00643cbafc587e13a.1269936879.git.eduard.munteanu@linux360.ro>
Download mbox | patch
Permalink /patch/48971/
State New
Headers show

Comments

Eduard - Gabriel Munteanu - March 30, 2010, 8:20 a.m.
hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
AMD IOMMU, which could lead to confusion unless we rename the former.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 Makefile.target               |    2 +-
 hw/{iommu.c => sparc_iommu.c} |    0
 hw/sun4m.h                    |    2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename hw/{iommu.c => sparc_iommu.c} (100%)
Blue Swirl - March 30, 2010, 5:06 p.m.
On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
>  AMD IOMMU, which could lead to confusion unless we rename the former.

I was also thinking of renaming the file some time ago. The correct
name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
several IO-UNITs instead. All Sun4m machines had an IOMMU.

But the qdev name of the device is still "iommu" and we can't change
that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
in amd_iommu.c?
Joerg Roedel - March 30, 2010, 7:28 p.m.
On Tue, Mar 30, 2010 at 08:06:36PM +0300, Blue Swirl wrote:
> On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> > hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
> >  AMD IOMMU, which could lead to confusion unless we rename the former.
> 
> I was also thinking of renaming the file some time ago. The correct
> name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
> different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
> several IO-UNITs instead. All Sun4m machines had an IOMMU.
> 
> But the qdev name of the device is still "iommu" and we can't change
> that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
> in amd_iommu.c?

Keeping the plain name 'iommu' will likely cause confusion when more
iommu implementations are added. It is better to rename it so that the
name better describes what the file implements. So this change makes
sense for me.

	Joerg
Blue Swirl - March 30, 2010, 8 p.m.
On 3/30/10, Joerg Roedel <joro@8bytes.org> wrote:
> On Tue, Mar 30, 2010 at 08:06:36PM +0300, Blue Swirl wrote:
>  > On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
>  > > hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
>  > >  AMD IOMMU, which could lead to confusion unless we rename the former.
>  >
>  > I was also thinking of renaming the file some time ago. The correct
>  > name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
>  > different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
>  > several IO-UNITs instead. All Sun4m machines had an IOMMU.
>  >
>  > But the qdev name of the device is still "iommu" and we can't change
>  > that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
>  > in amd_iommu.c?
>
>
> Keeping the plain name 'iommu' will likely cause confusion when more
>  iommu implementations are added. It is better to rename it so that the
>  name better describes what the file implements. So this change makes
>  sense for me.

I see. I'm OK then with "sun4m_iommu.c".
Eduard - Gabriel Munteanu - March 30, 2010, 8:15 p.m.
On Tue, Mar 30, 2010 at 11:00:10PM +0300, Blue Swirl wrote:
> On 3/30/10, Joerg Roedel <joro@8bytes.org> wrote:
> > On Tue, Mar 30, 2010 at 08:06:36PM +0300, Blue Swirl wrote:
> >  > On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> >  > > hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
> >  > >  AMD IOMMU, which could lead to confusion unless we rename the former.
> >  >
> >  > I was also thinking of renaming the file some time ago. The correct
> >  > name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
> >  > different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
> >  > several IO-UNITs instead. All Sun4m machines had an IOMMU.
> >  >
> >  > But the qdev name of the device is still "iommu" and we can't change
> >  > that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
> >  > in amd_iommu.c?
> >
> >
> > Keeping the plain name 'iommu' will likely cause confusion when more
> >  iommu implementations are added. It is better to rename it so that the
> >  name better describes what the file implements. So this change makes
> >  sense for me.
> 
> I see. I'm OK then with "sun4m_iommu.c".

Yes, I think it's enough to just change the filename, since multiple
such devices aren't likely to conflict, in any configuration.

Then "sun4m_iommu.c" it is. Will resubmit with the next patchset.


	Eduard
Gerd Hoffmann - March 31, 2010, 7:27 a.m.
On 03/30/10 19:06, Blue Swirl wrote:
> On 3/30/10, Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>  wrote:
>> hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
>>   AMD IOMMU, which could lead to confusion unless we rename the former.
>
> I was also thinking of renaming the file some time ago. The correct
> name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
> different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
> several IO-UNITs instead. All Sun4m machines had an IOMMU.
>
> But the qdev name of the device is still "iommu" and we can't change
> that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
> in amd_iommu.c?

I'd go for the (filename) rename.  The qdev name shouldn't cause 
conflicts due to the different targets.

cheers,
   Gerd

Patch

diff --git a/Makefile.target b/Makefile.target
index 4d88543..cbe19a6 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -305,7 +305,7 @@  obj-sparc-y += vga.o vga-pci.o
 obj-sparc-y += fdc.o mc146818rtc.o serial.o
 obj-sparc-y += cirrus_vga.o parallel.o
 else
-obj-sparc-y = sun4m.o lance.o tcx.o iommu.o slavio_intctl.o
+obj-sparc-y = sun4m.o lance.o tcx.o sparc_iommu.o slavio_intctl.o
 obj-sparc-y += slavio_timer.o slavio_misc.o fdc.o sparc32_dma.o
 obj-sparc-y += cs4231.o eccmemctl.o sbi.o sun4c_intctl.o
 endif
diff --git a/hw/iommu.c b/hw/sparc_iommu.c
similarity index 100%
rename from hw/iommu.c
rename to hw/sparc_iommu.c
diff --git a/hw/sun4m.h b/hw/sun4m.h
index ce97ee5..5007924 100644
--- a/hw/sun4m.h
+++ b/hw/sun4m.h
@@ -5,7 +5,7 @@ 
 
 /* Devices used by sparc32 system.  */
 
-/* iommu.c */
+/* sparc_iommu.c */
 void sparc_iommu_memory_rw(void *opaque, target_phys_addr_t addr,
                                  uint8_t *buf, int len, int is_write);
 static inline void sparc_iommu_memory_read(void *opaque,