public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb•de>
To: Benjamin Herrenschmidt <benh@kernel•crashing.org>
Cc: linuxppc-dev@ozlabs•org
Subject: Re: [PATCH/RFC] Hookable IO operations
Date: Fri, 3 Nov 2006 23:48:37 +0100	[thread overview]
Message-ID: <200611032348.38402.arnd@arndb.de> (raw)
In-Reply-To: <1162593121.10630.132.camel@localhost.localdomain>

On Friday 03 November 2006 23:32, Benjamin Herrenschmidt wrote:

> > If you leave out the 'extern', it fits into a normal line ;-)
> 
> Did I change them from the initial definition ? Anyway, I dislike
> prototypes in headers without "extern".

I prefer writing down the extern as well, but the common style
seems to have moved away from it now.

> 
> > > +
> > > +extern void (*__memset_io)(volatile void __iomem *addr, int c,
> > > +			   unsigned long n);
> > > +extern void (*__memcpy_fromio)(void *dest, const volatile void __iomem *src,
> > > +			       unsigned long n);
> > > +extern void (*__memcpy_toio)(volatile void __iomem *dest, const void *src,
> > > +			     unsigned long n);
> > 
> > Why are these all separate symbols? Wouldn't it be clearer to group them
> > in ppc_md, or a similar structure of function pointers?
> 
> Because the IO ones are separate already and I want to keep things
> consistent with them.

Actually I meant all of the I/O pointers, not just these three. When
you were describing your idea to me, I was thinking of something like

struct ppc_io {
	void (*writeb)(u8 val);
	void (*writew)(u16 val);
	void (*writel)(u32 val);
	void (*writeq)(u64 val);
	...
	void (*memset_io)(volatile void __iomem *addr, int c,
				 unsigned long n);
	void (*__memcpy_fromio)(void *dest,
			const volatile void __iomem *src, unsigned long n);
	void (*__memcpy_toio)(volatile void __iomem *dest,
			const void *src, unsigned long n);
} *ppc_io;

Did you never consider this, or did you make your mind up in the
process?

Is your current code more efficient?

> > > -static u8 iSeries_Read_Byte(const volatile void __iomem *IoAddress)
> > > +static u8 iseries_readb(const volatile void __iomem *IoAddress)
> > 
> > Since you're converting iSeries_Read_Byte from SilLycAps, shouldn't
> > you change IoAddress to something more sensible at the same time?
> 
> No, I'm only changing the prototype & name to better match my hooks,
> further cleanups of the content of these functions is something I'll do
> in a separate patch.

ok, makes sense.

	Arnd <><

  reply	other threads:[~2006-11-03 22:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-11-03 21:12 [PATCH/RFC] Hookable IO operations Benjamin Herrenschmidt
2006-11-03 22:20 ` Arnd Bergmann
2006-11-03 22:32   ` Benjamin Herrenschmidt
2006-11-03 22:48     ` Arnd Bergmann [this message]
2006-11-03 22:56       ` Benjamin Herrenschmidt
2006-11-04  0:15         ` Arnd Bergmann
2006-11-04  1:06           ` Benjamin Herrenschmidt
2006-11-04 16:28 ` Segher Boessenkool
2006-11-04 22:05   ` Benjamin Herrenschmidt
2006-11-04 22:33     ` Segher Boessenkool

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200611032348.38402.arnd@arndb.de \
    --to=arnd@arndb$(echo .)de \
    --cc=benh@kernel$(echo .)crashing.org \
    --cc=linuxppc-dev@ozlabs$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox