From: Andrew Morton <akpm@linux-foundation.org> To: Ben Dooks <ben-linux@fluff.org> Cc: linux-kernel@vger.kernel.org, linux-fbdev-devel@lists.sourceforge.net, Greg KH <greg@kroah.com> Subject: Re: [PATCH] mfd: SM501 core driver Date: Tue, 6 Feb 2007 21:09:30 -0800 [thread overview] Message-ID: <20070206210930.fc814bf6.akpm@linux-foundation.org> (raw) In-Reply-To: <20070206192628.GA13644@fluff.org.uk> On Tue, 6 Feb 2007 19:26:28 +0000 Ben Dooks <ben-linux@fluff.org> wrote: > This patch is an update patch, ready for merging > for the Silicon Motion SM501 multi-function device > core. > > This driver handles the core function of the chip, > including the clock, power control and allocation > of resources for drivers. It also exports a series > of platform devices for the function drivers to > attach to. > > This patch supports both platform and PCI bus > attached devices. > > Signed-off-by: Ben Dooks <ben-linux@fluff.org> Can we get Vincent's signoff here? > +#ifdef CONFIG_DEBUG This doesn't appear to be defined anywhere, and nor should it be. Something subsystem-specific should be used here? > +static const unsigned int misc_div[] = { > + [0] = 1, > + [1] = 2, > + [2] = 4, > + [3] = 8, > + [4] = 16, > + [5] = 32, > + [6] = 64, > + [7] = 128, > + [8] = 3, > + [9] = 6, > + [10] = 12, > + [11] = 24, > + [12] = 48, > + [13] = 96, > + [14] = 192, > + [15] = 384, > +}; > + > +static const unsigned int px_div[] = { > + [0] = 1, > + [1] = 2, > + [2] = 4, > + [3] = 8, > + [4] = 16, > + [5] = 32, > + [6] = 64, > + [7] = 128, > + [8] = 3, > + [9] = 6, > + [10] = 12, > + [11] = 24, > + [12] = 48, > + [13] = 96, > + [14] = 192, > + [15] = 384, > + [16] = 5, > + [17] = 10, > + [18] = 20, > + [19] = 40, > + [20] = 80, > + [21] = 160, > + [22] = 320, > + [23] = 604, > +}; > +#endif > + > +#define decode_div(val, lshft, selbit, mask, dtab) \ > + ((((val) & (selbit)) ? pll2 : 288 * MHZ) / dtab[(((val) >> (lshft)) & (mask))]) A C function would be nicer here, if poss. > +#define fmt_freq(x) ((x) / MHZ), ((x) % MHZ), (x) eww. > + (void)readl(sm->regs); Is there any benefit in all those casts? Generally we prefer to avoid them. > +EXPORT_SYMBOL_GPL(sm501_misc_control); The driver exports a lot of symbols. Does it really need to? > + down(&sm->clock_lock); Please convert to a mutex, if possible. > +/* sm501_null_release > + * > + * A release function for the platform devices we create to keep the > + * driver core happy, and stop any crashed when the devices are removed > +*/ > + > +static void sm501_null_release(struct device *dev) > +{ > +} Greg might have an opinion on that ;) > +static void sm501_create_mem(struct sm501_devdata *sm, > + struct resource *res, > + unsigned long *offs, > + unsigned long size) > +{ > + *offs -= size; /* adjust memory size */ > + > + res->flags = IORESOURCE_MEM; > + res->parent = sm->mem_res; > + res->start = sm->mem_res->start + *offs; > + res->end = res->start + size - 1; > +} Please use resource_size_t throughout, test with CONFIG_RESOURCES_64BIT on and off. > +static inline void sm501_init_reg(struct sm501_devdata *sm, > + unsigned long reg, > + struct sm501_reg_init *r) > +{ > + unsigned long tmp; > + > + tmp = readl(sm->regs + reg); > + tmp |= r->set; > + tmp &= ~r->mask; > + writel(tmp, sm->regs + reg); > +} This might be too large to inline.
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org> To: Ben Dooks <ben-linux@fluff.org> Cc: Greg KH <greg@kroah.com>, linux-fbdev-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mfd: SM501 core driver Date: Tue, 6 Feb 2007 21:09:30 -0800 [thread overview] Message-ID: <20070206210930.fc814bf6.akpm@linux-foundation.org> (raw) In-Reply-To: <20070206192628.GA13644@fluff.org.uk> On Tue, 6 Feb 2007 19:26:28 +0000 Ben Dooks <ben-linux@fluff.org> wrote: > This patch is an update patch, ready for merging > for the Silicon Motion SM501 multi-function device > core. > > This driver handles the core function of the chip, > including the clock, power control and allocation > of resources for drivers. It also exports a series > of platform devices for the function drivers to > attach to. > > This patch supports both platform and PCI bus > attached devices. > > Signed-off-by: Ben Dooks <ben-linux@fluff.org> Can we get Vincent's signoff here? > +#ifdef CONFIG_DEBUG This doesn't appear to be defined anywhere, and nor should it be. Something subsystem-specific should be used here? > +static const unsigned int misc_div[] = { > + [0] = 1, > + [1] = 2, > + [2] = 4, > + [3] = 8, > + [4] = 16, > + [5] = 32, > + [6] = 64, > + [7] = 128, > + [8] = 3, > + [9] = 6, > + [10] = 12, > + [11] = 24, > + [12] = 48, > + [13] = 96, > + [14] = 192, > + [15] = 384, > +}; > + > +static const unsigned int px_div[] = { > + [0] = 1, > + [1] = 2, > + [2] = 4, > + [3] = 8, > + [4] = 16, > + [5] = 32, > + [6] = 64, > + [7] = 128, > + [8] = 3, > + [9] = 6, > + [10] = 12, > + [11] = 24, > + [12] = 48, > + [13] = 96, > + [14] = 192, > + [15] = 384, > + [16] = 5, > + [17] = 10, > + [18] = 20, > + [19] = 40, > + [20] = 80, > + [21] = 160, > + [22] = 320, > + [23] = 604, > +}; > +#endif > + > +#define decode_div(val, lshft, selbit, mask, dtab) \ > + ((((val) & (selbit)) ? pll2 : 288 * MHZ) / dtab[(((val) >> (lshft)) & (mask))]) A C function would be nicer here, if poss. > +#define fmt_freq(x) ((x) / MHZ), ((x) % MHZ), (x) eww. > + (void)readl(sm->regs); Is there any benefit in all those casts? Generally we prefer to avoid them. > +EXPORT_SYMBOL_GPL(sm501_misc_control); The driver exports a lot of symbols. Does it really need to? > + down(&sm->clock_lock); Please convert to a mutex, if possible. > +/* sm501_null_release > + * > + * A release function for the platform devices we create to keep the > + * driver core happy, and stop any crashed when the devices are removed > +*/ > + > +static void sm501_null_release(struct device *dev) > +{ > +} Greg might have an opinion on that ;) > +static void sm501_create_mem(struct sm501_devdata *sm, > + struct resource *res, > + unsigned long *offs, > + unsigned long size) > +{ > + *offs -= size; /* adjust memory size */ > + > + res->flags = IORESOURCE_MEM; > + res->parent = sm->mem_res; > + res->start = sm->mem_res->start + *offs; > + res->end = res->start + size - 1; > +} Please use resource_size_t throughout, test with CONFIG_RESOURCES_64BIT on and off. > +static inline void sm501_init_reg(struct sm501_devdata *sm, > + unsigned long reg, > + struct sm501_reg_init *r) > +{ > + unsigned long tmp; > + > + tmp = readl(sm->regs + reg); > + tmp |= r->set; > + tmp &= ~r->mask; > + writel(tmp, sm->regs + reg); > +} This might be too large to inline. ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier. Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
next prev parent reply other threads:[~2007-02-07 5:09 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2007-02-06 19:26 [PATCH] mfd: SM501 core driver Ben Dooks 2007-02-07 5:09 ` Andrew Morton [this message] 2007-02-07 5:09 ` Andrew Morton 2007-02-07 11:48 ` Ben Dooks 2007-02-07 11:48 ` Ben Dooks 2007-02-07 14:24 ` Greg KH 2007-02-07 15:29 ` Ben Dooks 2007-02-07 16:52 ` Andrew Morton 2007-02-07 17:09 ` Ben Dooks 2007-02-07 17:09 ` Ben Dooks 2007-02-08 17:56 ` James Simmons 2007-02-08 17:56 ` James Simmons 2007-02-12 11:12 ` [Linux-fbdev-devel] " Ben Dooks 2007-02-07 17:38 ` Vincent Sanders 2007-02-07 17:38 ` Vincent Sanders
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=20070206210930.fc814bf6.akpm@linux-foundation.org \ --to=akpm@linux-foundation.org \ --cc=ben-linux@fluff.org \ --cc=greg@kroah.com \ --cc=linux-fbdev-devel@lists.sourceforge.net \ --cc=linux-kernel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.