Re: [linuxdc-dev] [PATCHES] Started work on maple driver

[Paul M. <le...@li...>]

On Thu, Jun 17, 2004 at 12:05:09AM +0100, Adrian McMenamin wrote:
> This is pretty crude stuff and doesn't even work properly in the limited =
way=20
> it is impolemented so far, but here it is for your perusal. (NB: these ar=
e=20
> against the maple code in the linuxsh repository).
>=20
For starters, please do all of the sysfs registration and handling in a new
maple-sysfs.c, it's ok to have one or two ifdefs in maple.c to hook into it,
but it would be preferrable to keep it isolated since sysfs is now under
CONFIG_SYSFS, so we may not always want it. If you look at the dma code, it
does similar things (though not as a bus), so you may want to look at that.
Additionally, you may also want to look at arch/sh/kernel/cpu/bus.c which
implements SH-bus as a dummy bus to handle registration of things that have
nowhere better to go (or things that could be done as platform device, but
have additional requirements, etc.). Ideally, maple will have a similar
registration method from a driver point of view.

> @@ -1,7 +1,20 @@
>  /*
> + *=09
>   *	Maple Bus device driver
>   */
> =20
> +/*************************
> + * Maple Bus Driver for  *
> + * SEGA Dreamcast 	 *
> + * Copyright Paul Mundt  *
> + * 2000 - 2004           *
> + * Sections copyright    *
> + * Adrian McMenamin      *
> + * 2004                  *
> + * Licensed under	 *
> + * version 2 of the GPL  *
> + *************************/
> +
>  #include <linux/config.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>

I didn't write this, so you can drop my name entirely. I'm not sure who the
original author of this is, it's had a bunch of different people hacking on=
 it
at some point or another though.

> @@ -65,7 +97,7 @@
>  	struct list_head *lh =3D (void *)driver;
>  	list_add(lh, &maple_driver_list);
> =20
> -	MOD_INC_USE_COUNT;
> +	/* MOD_INC_USE_COUNT; Redundant under 2.6 module code */
> =20
>  	pr_info("maple: registered driver: %s (function 0x%lx)\n",
>  	       driver->name, driver->function);
> @@ -77,7 +109,7 @@
>  	struct list_head *lh =3D (void *)driver;
>  	list_del(lh);
> =20
> -	MOD_DEC_USE_COUNT;
> +	/* MOD_DEC_USE_COUNT; */
> =20
>  	pr_info("maple: unregistered driver: %s (function 0x%lx)\n",
>  	       driver->name, driver->function);

It's probably worth adding an owner to the maple dev struct and doing
__module_get()/__module_put() on that for proper refcounting.

Other then that, it looks like a decent start, though much of maple.c needs
to be rewritten regardless due to its current inability to handle subdevice=
s,
general cruftiness of the list management, etc. another good reason to keep
the sysfs stuff abstracted in maple-sysfs.c, then we can rewrite maple.c
without stepping over any of the sysfs code.