Peter Stuge wrote:
On Sun, Oct 28, 2007 at 08:04:26PM -0400, Corey Osgood wrote:
+void f71805f_pnp_enable(struct device *dev) +{
- pnp_enter_conf_state(dev);
- pnp_set_logical_device(dev);
- if(dev->enabled) {
pnp_set_enable(dev, 1);- }
- else {
pnp_set_enable(dev, 0);- }
- pnp_exit_conf_state(dev);
+}
How about just pnp_set_enable(dev, dev->enabled) ?
Or !!dev->enabled if it needs to be 1 specifically.
Ok.
+static void f71805f_init(struct device *dev) +{
- struct superio_smsc_f71805f_config *conf = dev->chip_info;
- struct resource *res0, *res1;
- if (!dev->enabled)
return;- switch(dev->path.u.pnp.device) {
Since switch isn't a function I think the style is to have a space before ( though I don't care much for all the spaces myself.
Sounds right to me.
- case F71805F_SP1:
res0 = find_resource(dev, PNP_IDX_IO0);//init_uart8250(res0->base, &conf->com1);Why commented out? Can it be removed?
Good question, wish I could give you a good answer. It was commented in the v2 code, and is also commented in the winbond. I think it's something that should work in the future, but doesn't at the moment. I've put in a comment to that effect.
- /* COM1 */
- com1dev = "2";
Are the ports called COM1 and 2 in the data sheet? They're SP1/2 above. I don't particularly like the COMn names.
OK. But does renaming them to spn break the dts? Guess we'll find out later.
+++ superio/fintek/f71805f/Makefile (revision 0) @@ -0,0 +1,30 @@ +## +## This file is part of the LinuxBIOS project. +## +## Copyright (C) 2007 coresystems GmbH +## (Written by Stefan Reinauer stepan@coresystems.de for coresystems GmbH)
Did Stefan really write this?
Nope, my bad, copy-paste error. Updated patch attached.
-Corey