I got the following errors building 2.4.8-pre1.
Snippet from .config:
CONFIG_PARPORT=y
CONFIG_PARPORT_PC=y
CONFIG_PARPORT_PC_CML1=y
Steven
parport_pc.c:257: redefinition of `parport_pc_write_data'
/usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:45: `parport_pc_write_data' previously defined here
parport_pc.c:262: redefinition of `parport_pc_read_data'
/usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:53: `parport_pc_read_data' previously defined here
parport_pc.c:267: redefinition of `parport_pc_write_control'
/usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:139: `parport_pc_write_control' previously defined here
parport_pc.c:284: redefinition of `parport_pc_read_control'
/usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:156: `parport_pc_read_control' previously defined here
parport_pc.c:295: redefinition of `parport_pc_frob_control'
/usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:168: `parport_pc_frob_control' previously defined here
parport_pc.c:320: redefinition of `parport_pc_read_status'
/usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:193: `parport_pc_read_status' previously defined here
parport_pc.c:325: redefinition of `parport_pc_disable_irq'
/usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:199: `parport_pc_disable_irq' previously defined here
parport_pc.c:330: redefinition of `parport_pc_enable_irq'
/usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:204: `parport_pc_enable_irq' previously defined here
parport_pc.c:335: redefinition of `parport_pc_data_forward'
/usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:133: `parport_pc_data_forward' previously defined here
parport_pc.c:340: redefinition of `parport_pc_data_reverse'
/usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:128: `parport_pc_data_reverse' previously defined here
make[3]: *** [parport_pc.o] Error 1
make[3]: Leaving directory `/usr/src/linux-2.4.8-pre1/drivers/parport'
On Thu, Jul 26, 2001 at 07:53:11PM -0600, Steven Cole wrote:
> I got the following errors building 2.4.8-pre1.
>
> parport_pc.c:257: redefinition of `parport_pc_write_data'
> /usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:45: `parport_pc_write_data' previously defined here
> parport_pc.c:262: redefinition of `parport_pc_read_data'
> /usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:53: `parport_pc_read_data' previously defined here
> ...
> make[3]: *** [parport_pc.o] Error 1
> make[3]: Leaving directory `/usr/src/linux-2.4.8-pre1/drivers/parport'
Hmm, these functions are multiply defined, namely in the c source and
in it's header file. I see no reason why someone should do this. The
problem was hidden in older kernel releases by the fact that these
functions were declared "extern __inline__" which is absolutely
nonsense in my opinion. So the solution should be to just remove these
inline functions from the c source file, which can be done with the
following simple and stupid patch.
This should be the correct solution, or did I miss the vital point?
Robert
diff -u --recursive --new-file v2.4.7/linux/drivers/parport/parport_pc.c linux/drivers/parport/parport_pc.c
--- v2.4.7/linux/drivers/parport/parport_pc.c Wed Jul 11 01:07:46 2001
+++ linux/drivers/parport/parport_pc.c Fri Jul 27 09:24:50 2001
@@ -253,94 +253,6 @@
parport_generic_irq(irq, (struct parport *) dev_id, regs);
}
-void parport_pc_write_data(struct parport *p, unsigned char d)
-{
- outb (d, DATA (p));
-}
-
-unsigned char parport_pc_read_data(struct parport *p)
-{
- return inb (DATA (p));
-}
-
-void parport_pc_write_control(struct parport *p, unsigned char d)
-{
- const unsigned char wm = (PARPORT_CONTROL_STROBE |
- PARPORT_CONTROL_AUTOFD |
- PARPORT_CONTROL_INIT |
- PARPORT_CONTROL_SELECT);
-
- /* Take this out when drivers have adapted to the newer interface. */
- if (d & 0x20) {
- printk (KERN_DEBUG "%s (%s): use data_reverse for this!\n",
- p->name, p->cad->name);
- parport_pc_data_reverse (p);
- }
-
- __parport_pc_frob_control (p, wm, d & wm);
-}
-
-unsigned char parport_pc_read_control(struct parport *p)
-{
- const unsigned char wm = (PARPORT_CONTROL_STROBE |
- PARPORT_CONTROL_AUTOFD |
- PARPORT_CONTROL_INIT |
- PARPORT_CONTROL_SELECT);
- const struct parport_pc_private *priv = p->physport->private_data;
- return priv->ctr & wm; /* Use soft copy */
-}
-
-unsigned char parport_pc_frob_control (struct parport *p, unsigned char mask,
- unsigned char val)
-{
- const unsigned char wm = (PARPORT_CONTROL_STROBE |
- PARPORT_CONTROL_AUTOFD |
- PARPORT_CONTROL_INIT |
- PARPORT_CONTROL_SELECT);
-
- /* Take this out when drivers have adapted to the newer interface. */
- if (mask & 0x20) {
- printk (KERN_DEBUG "%s (%s): use data_%s for this!\n",
- p->name, p->cad->name,
- (val & 0x20) ? "reverse" : "forward");
- if (val & 0x20)
- parport_pc_data_reverse (p);
- else
- parport_pc_data_forward (p);
- }
-
- /* Restrict mask and val to control lines. */
- mask &= wm;
- val &= wm;
-
- return __parport_pc_frob_control (p, mask, val);
-}
-
-unsigned char parport_pc_read_status(struct parport *p)
-{
- return inb (STATUS (p));
-}
-
-void parport_pc_disable_irq(struct parport *p)
-{
- __parport_pc_frob_control (p, 0x10, 0);
-}
-
-void parport_pc_enable_irq(struct parport *p)
-{
- __parport_pc_frob_control (p, 0x10, 0x10);
-}
-
-void parport_pc_data_forward (struct parport *p)
-{
- __parport_pc_frob_control (p, 0x20, 0);
-}
-
-void parport_pc_data_reverse (struct parport *p)
-{
- __parport_pc_frob_control (p, 0x20, 0x20);
-}
-
void parport_pc_init_state(struct pardevice *dev, struct parport_state *s)
{
s->u.pc.ctr = 0xc | (dev->irq_func ? 0x10 : 0x0);
--
Robert Schiele mailto:[email protected]
Tel./Fax: +49-621-10059 http://webrum.uni-mannheim.de/math/rschiele/
On Friday 27 July 2001 02:12, Robert Schiele wrote:
> On Thu, Jul 26, 2001 at 07:53:11PM -0600, Steven Cole wrote:
> > I got the following errors building 2.4.8-pre1.
> >
> > parport_pc.c:257: redefinition of `parport_pc_write_data'
> > /usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:45:
> > `parport_pc_write_data' previously defined here parport_pc.c:262:
> > redefinition of `parport_pc_read_data'
> > /usr/src/linux-2.4.8-pre1/include/linux/parport_pc.h:53:
> > `parport_pc_read_data' previously defined here ...
> > make[3]: *** [parport_pc.o] Error 1
> > make[3]: Leaving directory `/usr/src/linux-2.4.8-pre1/drivers/parport'
>
> Hmm, these functions are multiply defined, namely in the c source and
> in it's header file. I see no reason why someone should do this. The
> problem was hidden in older kernel releases by the fact that these
> functions were declared "extern __inline__" which is absolutely
> nonsense in my opinion. So the solution should be to just remove these
> inline functions from the c source file, which can be done with the
> following simple and stupid patch.
>
> This should be the correct solution, or did I miss the vital point?
>
> Robert
Your patch works for me. Thank you. I can now print from my parallel port
printer using 2.4.8-pre1. FWIW, here is my gcc version:
[steven@localhost steven]$ gcc -v
Reading specs from /usr/lib/gcc-lib/i586-mandrake-linux/2.96/specs
gcc version 2.96 20000731 (Linux-Mandrake 8.0 2.96-0.47mdk)
Steven
>Hmm, these functions are multiply defined, namely in the c source and
>in it's header file. I see no reason why someone should do this. The
>problem was hidden in older kernel releases by the fact that these
>functions were declared "extern __inline__" which is absolutely
>nonsense in my opinion. So the solution should be to just remove these
>inline functions from the c source file, which can be done with the
>following simple and stupid patch.
>
>This should be the correct solution, or did I miss the vital point?
I think you did miss the vital point: this will probably break with
CONFIG_PARPORT_OTHER.
Declaring them "extern inline" in parport_pc.h is exactly the right thing to
do. What do you think is wrong with that?
p.
On Sat, Jul 28, 2001 at 05:39:22PM +0100, Philip Blundell wrote:
> I think you did miss the vital point: this will probably break with
> CONFIG_PARPORT_OTHER.
No this cannot happen. These functions are only used from source files
that also include parport_pc.h. If this were not the case, it would
have been a bug anyway.
>
> Declaring them "extern inline" in parport_pc.h is exactly the right thing to
> do. What do you think is wrong with that?
The "extern" was only an escape for the case that the compiler cannot
inline the function. Due to the fact, that current gcc has "static
inline" it is better to use this, because with "static inline" we do
not need to keep a global symbol just for the case the compiler is not
capable to inline the function in some place.
Let's turn the tables: What do you think is wrong with "static
inline"? In my opinion it's a much cleaner solution than "extern
inline".
Robert
--
Robert Schiele mailto:[email protected]
Tel./Fax: +49-621-10059 http://webrum.uni-mannheim.de/math/rschiele/
>The "extern" was only an escape for the case that the compiler cannot
>inline the function. Due to the fact, that current gcc has "static
>inline" it is better to use this, because with "static inline" we do
>not need to keep a global symbol just for the case the compiler is not
>capable to inline the function in some place.
The versions in the .c file are there so that the "ops" structure can point to
them. The ones in the .h file are purely an optimisation to allow you to
short-circuit the ops struct if you know only one driver is involved.
Changing this stuff to "static inline" still offends my sense of aesthetics
somewhat, but I guess it's okay if you have checked that it still does the
right thing in the CONFIG_PARPORT_OTHER case.
p.
On Sat, Jul 28, 2001 at 10:06:43PM +0100, Philip Blundell wrote:
> >The "extern" was only an escape for the case that the compiler cannot
> >inline the function. Due to the fact, that current gcc has "static
> >inline" it is better to use this, because with "static inline" we do
> >not need to keep a global symbol just for the case the compiler is not
> >capable to inline the function in some place.
>
> The versions in the .c file are there so that the "ops" structure can point to
> them. The ones in the .h file are purely an optimisation to allow you to
> short-circuit the ops struct if you know only one driver is involved.
Because of that the compiler has to create a "real" function from the
code anyway. The way this is handled by gcc is stated in the gcc info
pages:
"When a function is both inline and `static', if all calls to the
function are integrated into the caller, and the function's
address is never used, then the function's own assembler code is
never referenced. In this case, GNU CC does not actually output
assembler code for the function, unless you specify the option
`-fkeep-inline-functions'. Some calls cannot be integrated for
various reasons (in particular, calls that precede the function's
definition cannot be integrated, and neither can recursive calls
within the definition). If there is a nonintegrated call, then
the function is compiled to assembler code as usual. The function
must also be compiled as usual if the program refers to its
address, because that can't be inlined."
>
> Changing this stuff to "static inline" still offends my sense of aesthetics
> somewhat, but I guess it's okay if you have checked that it still does the
> right thing in the CONFIG_PARPORT_OTHER case.
I didn't test this before your mail. But to be absolutely sure, I
tested it now. --- And it works perfectly.
Robert
--
Robert Schiele mailto:[email protected]
Tel./Fax: +49-621-10059 http://webrum.uni-mannheim.de/math/rschiele/