[PATCH 22/52] m68k: atari: Add and use "atari.h"

Geert Uytterhoeven posted 52 patches 2 years, 3 months ago
There is a newer version of this series
[PATCH 22/52] m68k: atari: Add and use "atari.h"
Posted by Geert Uytterhoeven 2 years, 3 months ago
When building with W=1:

    arch/m68k/atari/time.c:59:1: warning: no previous prototype for ‘atari_sched_init’ [-Wmissing-prototypes]
       59 | atari_sched_init(void)
	  | ^~~~~~~~~~~~~~~~
    arch/m68k/atari/time.c:140:5: warning: no previous prototype for ‘atari_mste_hwclk’ [-Wmissing-prototypes]
      140 | int atari_mste_hwclk( int op, struct rtc_time *t )
	  |     ^~~~~~~~~~~~~~~~
    arch/m68k/atari/time.c:199:5: warning: no previous prototype for ‘atari_tt_hwclk’ [-Wmissing-prototypes]
      199 | int atari_tt_hwclk( int op, struct rtc_time *t )
	  |     ^~~~~~~~~~~~~~
    arch/m68k/atari/ataints.c:267:13: warning: no previous prototype for ‘atari_init_IRQ’ [-Wmissing-prototypes]
      267 | void __init atari_init_IRQ(void)
	  |             ^~~~~~~~~~~~~~
    arch/m68k/atari/atasound.c:36:6: warning: no previous prototype for ‘atari_microwire_cmd’ [-Wmissing-prototypes]
       36 | void atari_microwire_cmd (int cmd)
	  |      ^~~~~~~~~~~~~~~~~~~
    arch/m68k/atari/atasound.c:53:6: warning: no previous prototype for ‘atari_mksound’ [-Wmissing-prototypes]
       53 | void atari_mksound (unsigned int hz, unsigned int ticks)
	  |      ^~~~~~~~~~~~~

Fix this by introducing a new header file "atari.h" for holding the
prototypes of functions implemented in arch/m68k/atari/.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/m68k/atari/ataints.c  |  3 +--
 arch/m68k/atari/atari.h    | 15 +++++++++++++++
 arch/m68k/atari/atasound.c |  1 +
 arch/m68k/atari/config.c   | 11 ++---------
 arch/m68k/atari/time.c     |  2 ++
 5 files changed, 21 insertions(+), 11 deletions(-)
 create mode 100644 arch/m68k/atari/atari.h

diff --git a/arch/m68k/atari/ataints.c b/arch/m68k/atari/ataints.c
index 56f02ea2c248d844..23256434191c39af 100644
--- a/arch/m68k/atari/ataints.c
+++ b/arch/m68k/atari/ataints.c
@@ -52,6 +52,7 @@
 #include <asm/entry.h>
 #include <asm/io.h>
 
+#include "atari.h"
 
 /*
  * Atari interrupt handling scheme:
@@ -81,8 +82,6 @@ __ALIGN_STR "\n\t"
 	"orw	#0x200,%sp@\n\t"	/* set saved ipl to 2 */
 	"rte");
 
-extern void atari_microwire_cmd(int cmd);
-
 static unsigned int atari_irq_startup(struct irq_data *data)
 {
 	unsigned int irq = data->irq;
diff --git a/arch/m68k/atari/atari.h b/arch/m68k/atari/atari.h
new file mode 100644
index 0000000000000000..494a03ddac3d16ae
--- /dev/null
+++ b/arch/m68k/atari/atari.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+struct rtc_time;
+
+/* ataints.c */
+void atari_init_IRQ(void);
+
+/* atasound.c */
+void atari_microwire_cmd(int cmd);
+void atari_mksound(unsigned int hz, unsigned int ticks);
+
+/* time.c */
+void atari_sched_init(void);
+int atari_mste_hwclk(int op, struct rtc_time *t);
+int atari_tt_hwclk(int op, struct rtc_time *t);
diff --git a/arch/m68k/atari/atasound.c b/arch/m68k/atari/atasound.c
index a8724d998c39fcfa..c38ef0e6078e7260 100644
--- a/arch/m68k/atari/atasound.c
+++ b/arch/m68k/atari/atasound.c
@@ -28,6 +28,7 @@
 #include <asm/irq.h>
 #include <asm/atariints.h>
 
+#include "atari.h"
 
 /*
  * stuff from the old atasound.c
diff --git a/arch/m68k/atari/config.c b/arch/m68k/atari/config.c
index b4fe4273ad912ebe..b48a0606a00068b9 100644
--- a/arch/m68k/atari/config.c
+++ b/arch/m68k/atari/config.c
@@ -48,6 +48,8 @@
 #include <asm/io.h>
 #include <asm/config.h>
 
+#include "atari.h"
+
 u_long atari_mch_cookie;
 EXPORT_SYMBOL(atari_mch_cookie);
 
@@ -69,19 +71,10 @@ int atari_rtc_year_offset;
 static void atari_reset(void);
 static void atari_get_model(char *model);
 static void atari_get_hardware_list(struct seq_file *m);
-
-/* atari specific irq functions */
-extern void atari_init_IRQ (void);
-extern void atari_mksound(unsigned int count, unsigned int ticks);
 #ifdef CONFIG_HEARTBEAT
 static void atari_heartbeat(int on);
 #endif
 
-/* atari specific timer functions (in time.c) */
-extern void atari_sched_init(void);
-extern int atari_mste_hwclk (int, struct rtc_time *);
-extern int atari_tt_hwclk (int, struct rtc_time *);
-
 /* ++roman: This is a more elaborate test for an SCC chip, since the plain
  * Medusa board generates DTACK at the SCC's standard addresses, but a SCC
  * board in the Medusa is possible. Also, the addresses where the ST_ESCC
diff --git a/arch/m68k/atari/time.c b/arch/m68k/atari/time.c
index 7e44d0e9d0f8a902..3453c6dc6b41d3c9 100644
--- a/arch/m68k/atari/time.c
+++ b/arch/m68k/atari/time.c
@@ -23,6 +23,8 @@
 #include <asm/atariints.h>
 #include <asm/machdep.h>
 
+#include "atari.h"
+
 DEFINE_SPINLOCK(rtc_lock);
 EXPORT_SYMBOL_GPL(rtc_lock);
 
-- 
2.34.1

Re: [PATCH 22/52] m68k: atari: Add and use "atari.h"
Posted by Finn Thain 2 years, 3 months ago
On Thu, 7 Sep 2023, Geert Uytterhoeven wrote:

> diff --git a/arch/m68k/atari/atari.h b/arch/m68k/atari/atari.h
> new file mode 100644
> index 0000000000000000..494a03ddac3d16ae
> --- /dev/null
> +++ b/arch/m68k/atari/atari.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +struct rtc_time;
> +
> +/* ataints.c */
> +void atari_init_IRQ(void);
> +
> +/* atasound.c */
> +void atari_microwire_cmd(int cmd);
> +void atari_mksound(unsigned int hz, unsigned int ticks);
> +
> +/* time.c */
> +void atari_sched_init(void);
> +int atari_mste_hwclk(int op, struct rtc_time *t);
> +int atari_tt_hwclk(int op, struct rtc_time *t);

Wouldn't atariints.h and atarihw.h be more appropriate places for some of 
these?
Re: [PATCH 22/52] m68k: atari: Add and use "atari.h"
Posted by Michael Schmitz 2 years, 3 months ago
Hi Finn,

On 8/09/23 11:57, Finn Thain wrote:
> On Thu, 7 Sep 2023, Geert Uytterhoeven wrote:
>
>> diff --git a/arch/m68k/atari/atari.h b/arch/m68k/atari/atari.h
>> new file mode 100644
>> index 0000000000000000..494a03ddac3d16ae
>> --- /dev/null
>> +++ b/arch/m68k/atari/atari.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +struct rtc_time;
>> +
>> +/* ataints.c */
>> +void atari_init_IRQ(void);
>> +
>> +/* atasound.c */
>> +void atari_microwire_cmd(int cmd);
>> +void atari_mksound(unsigned int hz, unsigned int ticks);
>> +
>> +/* time.c */
>> +void atari_sched_init(void);
>> +int atari_mste_hwclk(int op, struct rtc_time *t);
>> +int atari_tt_hwclk(int op, struct rtc_time *t);
> Wouldn't atariints.h and atarihw.h be more appropriate places for some of
> these?

atariints.h already has some prototypes, so yes on that account.

atarihw.h only has inlines, but sound and time related prototypes could 
be added there, too.

Geert's intentions might have been avoiding inclusion of all the 
hardware specific data in those two files, but the only source file to 
benefit from this is config.c (the other three already include 
atariints.h and atarihw.h).

OTOH, considering this patch series adds a lot of other headers that 
only contain prototypes, it might be better to keep to that pattern 
everywhere.

Cheers,

     Michael


Re: [PATCH 22/52] m68k: atari: Add and use "atari.h"
Posted by Finn Thain 2 years, 3 months ago
On Fri, 8 Sep 2023, Michael Schmitz wrote:

> 
> atariints.h already has some prototypes, so yes on that account.
> 
> atarihw.h only has inlines, but sound and time related prototypes could 
> be added there, too.
> 
> Geert's intentions might have been avoiding inclusion of all the 
> hardware specific data in those two files, but the only source file to 
> benefit from this is config.c (the other three already include 
> atariints.h and atarihw.h).
> 
> OTOH, considering this patch series adds a lot of other headers that 
> only contain prototypes, it might be better to keep to that pattern 
> everywhere.
> 

I think Geert's intention may have been to avoid adding definitions to the 
asm/foo.h headers shipped with "make headers_install". If that's the 
intention, there would seem to be a mess to be cleaned up, for which I'm 
partly to blame...
Re: [PATCH 22/52] m68k: atari: Add and use "atari.h"
Posted by Geert Uytterhoeven 2 years, 3 months ago
Hi Finn,

On Fri, Sep 8, 2023 at 3:05 AM Finn Thain <fthain@linux-m68k.org> wrote:
> On Fri, 8 Sep 2023, Michael Schmitz wrote:
> > atariints.h already has some prototypes, so yes on that account.
> >
> > atarihw.h only has inlines, but sound and time related prototypes could
> > be added there, too.
> >
> > Geert's intentions might have been avoiding inclusion of all the
> > hardware specific data in those two files, but the only source file to
> > benefit from this is config.c (the other three already include
> > atariints.h and atarihw.h).
> >
> > OTOH, considering this patch series adds a lot of other headers that
> > only contain prototypes, it might be better to keep to that pattern
> > everywhere.
>
> I think Geert's intention may have been to avoid adding definitions to the
> asm/foo.h headers shipped with "make headers_install". If that's the
> intention, there would seem to be a mess to be cleaned up, for which I'm
> partly to blame...

arch/m68k/include/asm/ is not shipped, (arch/m68k/include/uapi/asm/ is).
However, arch/m68k/include/asm/ is shared with the whole tree, while
only the core code under arch/m68k/ needs these definitions.

There is (are) definitely (an) opportunit{y,ies} for moving more stuff
around (from <asm/...> to "...", more extern declarations in C files, ...).
Not to mention the more substantial rework...
But Linux/m68k^WRome wasn't built in a decade neither... ;-)

And TBH, I also went for the "minimum amount of work" ;-)
(which didn't work out that well, yet another rabbit hole, as usual...).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds