On Wednesday, April 24, 2013 5:37 PM Andrew Morton wrote:
=20
On Wed, 24 Apr 2013 10:31:57 -0400 Alexandre Bounine
[off-list ref] wrote:
=20
quoted
Rework to implement RapidIO enumeration/discovery method selection
combined with ability to use enumeration/discovery as a kernel module.
...
@@ -1421,3 +1295,46 @@ enum_done:
bail:
return -EBUSY;
}
+
+struct rio_scan rio_scan_ops =3D {
+ .enumerate =3D rio_enum_mport,
+ .discover =3D rio_disc_mport,
+};
+
+
+#ifdef MODULE
=20
Why the `ifdef MODULE'? The module parameters are still accessible if
the driver is statically linked and we do want the driver to behave in
the same way regardless of how it was linked and loaded.
Marked for review.
quoted
+static bool scan;
+module_param(scan, bool, 0);
+MODULE_PARM_DESC(scan, "Start RapidIO network enumeration/discovery
"
quoted
+ "(default =3D 1)");
+
+/**
+ * rio_basic_attach:
+ *
+ * When this enumeration/discovery method is loaded as a module this f=
unction
quoted
+ * registers its specific enumeration and discover routines for all av=
ailable
quoted
+ * RapidIO mport devices. The "scan" command line parameter controls a=
bility of
quoted
+ * the module to start RapidIO enumeration/discovery automatically.
+ *
+ * Returns 0 for success or -EIO if unable to register itself.
+ *
+ * This enumeration/discovery method cannot be unloaded and therefore =
does not
quoted
+ * provide a matching cleanup_module routine.
+ */
+
+int __init rio_basic_attach(void)
=20
static
My miss. Will fix.
quoted
+{
+ if (rio_register_scan(RIO_MPORT_ANY, &rio_scan_ops))
+ return -EIO;
+ if (scan)
+ rio_init_mports();
+ return 0;
+}
+
+module_init(rio_basic_attach);
+
+MODULE_DESCRIPTION("Basic RapidIO enumeration/discovery");
+MODULE_LICENSE("GPL");
+
+#endif /* MODULE */diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
index d553b5d..e36628a 100644
--- a/drivers/rapidio/rio.c
+++ b/drivers/rapidio/rio.c
@@ -31,6 +31,9 @@
#include "rio.h"
+LIST_HEAD(rio_devices);
=20
static?
=20
quoted
+DEFINE_SPINLOCK(rio_global_list_lock);
=20
static?
These two have been very tempting for me to make them static.
But because a goal was simple code move and they have been visible
from very beginning of RapidIO code, I decided to keep the scope "as is".
While I am against using the device list directly, I am not sure that this
patch is a good place to change the existing scope.
Because keeping them global prompted your comment I will happily make them
static and see if anyone will complain about it.
=20
quoted
+
static LIST_HEAD(rio_mports);
static unsigned char next_portid;
static DEFINE_SPINLOCK(rio_mmap_lock);
...
+/**
+ * rio_switch_init - Sets switch operations for a particular vendor sw=
itch
quoted
+ * @rdev: RIO device
+ * @do_enum: Enumeration/Discovery mode flag
+ *
+ * Searches the RIO switch ops table for known switch types. If the vi=
d
quoted
+ * and did match a switch table entry, then call switch initialization
+ * routine to setup switch-specific routines.
+ */
+void rio_switch_init(struct rio_dev *rdev, int do_enum)
+{
+ struct rio_switch_ops *cur =3D __start_rio_switch_ops;
+ struct rio_switch_ops *end =3D __end_rio_switch_ops;
=20
huh, I hadn't noticed that RIO has its very own vmlinux section. How
peculair.
Yes it is there (since 2.6.15). We will address it at some point later.
At this moment just moving the code from one file to another.
=20
quoted
+ while (cur < end) {
+ if ((cur->vid =3D=3D rdev->vid) && (cur->did =3D=3D rdev->did)) {
+ pr_debug("RIO: calling init routine for %s\n",
+ rio_name(rdev));
+ cur->init_hook(rdev, do_enum);
+ break;
+ }
+ cur++;
+ }
+
+ if ((cur >=3D end) && (rdev->pef & RIO_PEF_STD_RT)) {
+ pr_debug("RIO: adding STD routing ops for %s\n",
+ rio_name(rdev));
+ rdev->rswitch->add_entry =3D rio_std_route_add_entry;
+ rdev->rswitch->get_entry =3D rio_std_route_get_entry;
+ rdev->rswitch->clr_table =3D rio_std_route_clr_table;
+ }
+
+ if (!rdev->rswitch->add_entry || !rdev->rswitch->get_entry)
+ printk(KERN_ERR "RIO: missing routing ops for %s\n",
+ rio_name(rdev));
+}
+EXPORT_SYMBOL_GPL(rio_switch_init);
...
+int rio_register_scan(int mport_id, struct rio_scan *scan_ops)
+{
+ struct rio_mport *port;
+ int rc =3D -EBUSY;
+
+ list_for_each_entry(port, &rio_mports, node) {
=20
How come the driver has no locking for rio_mports? If a bugfix isn't
needed here then a code comment is!
Locking is not needed at this moment, but has to be added sooner or later a=
nyway.
I will add it now to avoid fixing it later.=20
=20
quoted
+ if (port->id =3D=3D mport_id || mport_id =3D=3D RIO_MPORT_ANY) {
+ if (port->nscan && mport_id =3D=3D RIO_MPORT_ANY)
+ continue;
+ else if (port->nscan)
+ break;
+
+ port->nscan =3D scan_ops;
+ rc =3D 0;
+
+ if (mport_id !=3D RIO_MPORT_ANY)
+ break;
+ }
+ }
+
+ return rc;
+}
...