Re: [PATCH v3 14/14] nubus: Add support for the driver model

[Philippe Ombredanne]

Finn,

On Sun, Dec 10, 2017 at 11:03 AM, Finn Thain  wrote:
> On Thu, 7 Dec 2017, Philippe Ombredanne wrote:
>
>> > diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c
>> > new file mode 100644
>> > index ..b5045e639b4d
>> > --- /dev/null
>> > +++ b/drivers/nubus/bus.c
>> > @@ -0,0 +1,119 @@
>> > +/*
>> > + * Bus implementation for the NuBus subsystem.
>> > + *
>> > + * Copyright (C) 2017 Finn Thain
>> > + *
>> > + * SPDX-License-Identifier: GPL-2.0
>> > + */
>>
>> I think the SPDX id should be on the first line using C++-style // comments.
>
> Sure, why not.
>
> Since Linus has explicitly allowed conversion of the entire block comment
> to C++ style, I think I may prefer that as being less ugly than the
> mixture of styles (though fwiw this will always read like top posting to
> me).
>
>> See tglx's doc patches
>
> I didn't find any mention of SPDX under Documentation...
>
>> and Linus posts explaining why he wants it this way!
>>
>
> I suspect that you are referring to his posts in this thread (?)
> https://lkml.org/lkml/2017/11/2/728

Actually this is patch series: https://lkml.org/lkml/2017/12/4/934
And possibly some minor fixes pending, based on Heiko eagle eye review
and some of my nits.

-- 
Cordially
Philippe Ombredanne

+1 650 799 0949 | pombreda...@nexb.com
DejaCode - What's in your code?! - http://www.dejacode.com
AboutCode - Open source for open source - https://www.aboutcode.org
nexB Inc. - http://www.nexb.com

Re: [PATCH v3 14/14] nubus: Add support for the driver model

[Philippe Ombredanne]

Finn,

On Sun, Dec 10, 2017 at 11:03 AM, Finn Thain  wrote:
> On Thu, 7 Dec 2017, Philippe Ombredanne wrote:
>
>> > diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c
>> > new file mode 100644
>> > index ..b5045e639b4d
>> > --- /dev/null
>> > +++ b/drivers/nubus/bus.c
>> > @@ -0,0 +1,119 @@
>> > +/*
>> > + * Bus implementation for the NuBus subsystem.
>> > + *
>> > + * Copyright (C) 2017 Finn Thain
>> > + *
>> > + * SPDX-License-Identifier: GPL-2.0
>> > + */
>>
>> I think the SPDX id should be on the first line using C++-style // comments.
>
> Sure, why not.
>
> Since Linus has explicitly allowed conversion of the entire block comment
> to C++ style, I think I may prefer that as being less ugly than the
> mixture of styles (though fwiw this will always read like top posting to
> me).
>
>> See tglx's doc patches
>
> I didn't find any mention of SPDX under Documentation...
>
>> and Linus posts explaining why he wants it this way!
>>
>
> I suspect that you are referring to his posts in this thread (?)
> https://lkml.org/lkml/2017/11/2/728

Actually this is patch series: https://lkml.org/lkml/2017/12/4/934
And possibly some minor fixes pending, based on Heiko eagle eye review
and some of my nits.

-- 
Cordially
Philippe Ombredanne

+1 650 799 0949 | pombreda...@nexb.com
DejaCode - What's in your code?! - http://www.dejacode.com
AboutCode - Open source for open source - https://www.aboutcode.org
nexB Inc. - http://www.nexb.com

Re: [PATCH v3 14/14] nubus: Add support for the driver model

[Finn Thain]

On Thu, 7 Dec 2017, Philippe Ombredanne wrote:

> > diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c
> > new file mode 100644
> > index ..b5045e639b4d
> > --- /dev/null
> > +++ b/drivers/nubus/bus.c
> > @@ -0,0 +1,119 @@
> > +/*
> > + * Bus implementation for the NuBus subsystem.
> > + *
> > + * Copyright (C) 2017 Finn Thain
> > + *
> > + * SPDX-License-Identifier: GPL-2.0
> > + */
> 
> I think the SPDX id should be on the first line using C++-style // comments.

Sure, why not.

Since Linus has explicitly allowed conversion of the entire block comment 
to C++ style, I think I may prefer that as being less ugly than the 
mixture of styles (though fwiw this will always read like top posting to 
me).

> See tglx's doc patches

I didn't find any mention of SPDX under Documentation...

> and Linus posts explaining why he wants it this way!
> 

I suspect that you are referring to his posts in this thread (?)
https://lkml.org/lkml/2017/11/2/728

-- 

Re: [PATCH v3 14/14] nubus: Add support for the driver model

[Finn Thain]

On Thu, 7 Dec 2017, Philippe Ombredanne wrote:

> > diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c
> > new file mode 100644
> > index ..b5045e639b4d
> > --- /dev/null
> > +++ b/drivers/nubus/bus.c
> > @@ -0,0 +1,119 @@
> > +/*
> > + * Bus implementation for the NuBus subsystem.
> > + *
> > + * Copyright (C) 2017 Finn Thain
> > + *
> > + * SPDX-License-Identifier: GPL-2.0
> > + */
> 
> I think the SPDX id should be on the first line using C++-style // comments.

Sure, why not.

Since Linus has explicitly allowed conversion of the entire block comment 
to C++ style, I think I may prefer that as being less ugly than the 
mixture of styles (though fwiw this will always read like top posting to 
me).

> See tglx's doc patches

I didn't find any mention of SPDX under Documentation...

> and Linus posts explaining why he wants it this way!
> 

I suspect that you are referring to his posts in this thread (?)
https://lkml.org/lkml/2017/11/2/728

-- 

Re: [PATCH v3 14/14] nubus: Add support for the driver model

[Philippe Ombredanne]

Finn,

On Tue, Dec 5, 2017 at 7:20 AM, Finn Thain  wrote:
> This patch brings basic support for the Linux Driver Model to the
> NuBus subsystem.
>
> For flexibility, the matching of boards with drivers is left up to the
> drivers. This is also the approach taken by NetBSD. A board may have
> many functions, and drivers may have to consider many functional
> resources and board resources in order to match a device.
>
> This implementation does not bind drivers to resources (nor does it bind
> many drivers to the same board). Apple's NuBus declaration ROM design
> is flexible enough to allow that, but I don't see a need to support it
> as we don't use the "slot zero" resources (in the main logic board ROM).
>
> Eliminate the global nubus_boards linked list by rewriting the procfs
> board iterator around bus_for_each_dev(). Hence the nubus device refcount
> can be used to determine the lifespan of board objects.
>
> Cc: Greg Kroah-Hartman 
> Tested-by: Stan Johnson 
> Signed-off-by: Finn Thain 
>
> ---
> The conversion of Mac network drivers from the Space.c convention to
> the Driver Model takes place in a separate patch series, archived at
> https://lkml.org/lkml/2017/11/11/25
> That series motivates parts of this design, such as the definition of
> 'for_each_board_func_rsrc'.
[]
> diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c
> new file mode 100644
> index ..b5045e639b4d
> --- /dev/null
> +++ b/drivers/nubus/bus.c
> @@ -0,0 +1,119 @@
> +/*
> + * Bus implementation for the NuBus subsystem.
> + *
> + * Copyright (C) 2017 Finn Thain
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */

I think the SPDX id should be on the first line using C++-style // comments.
See tglx's doc patches and Linus posts explaining why he wants it this way!

-- 
Cordially
Philippe Ombredanne

Re: [PATCH v3 14/14] nubus: Add support for the driver model

[Philippe Ombredanne]

Finn,

On Tue, Dec 5, 2017 at 7:20 AM, Finn Thain  wrote:
> This patch brings basic support for the Linux Driver Model to the
> NuBus subsystem.
>
> For flexibility, the matching of boards with drivers is left up to the
> drivers. This is also the approach taken by NetBSD. A board may have
> many functions, and drivers may have to consider many functional
> resources and board resources in order to match a device.
>
> This implementation does not bind drivers to resources (nor does it bind
> many drivers to the same board). Apple's NuBus declaration ROM design
> is flexible enough to allow that, but I don't see a need to support it
> as we don't use the "slot zero" resources (in the main logic board ROM).
>
> Eliminate the global nubus_boards linked list by rewriting the procfs
> board iterator around bus_for_each_dev(). Hence the nubus device refcount
> can be used to determine the lifespan of board objects.
>
> Cc: Greg Kroah-Hartman 
> Tested-by: Stan Johnson 
> Signed-off-by: Finn Thain 
>
> ---
> The conversion of Mac network drivers from the Space.c convention to
> the Driver Model takes place in a separate patch series, archived at
> https://lkml.org/lkml/2017/11/11/25
> That series motivates parts of this design, such as the definition of
> 'for_each_board_func_rsrc'.
[]
> diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c
> new file mode 100644
> index ..b5045e639b4d
> --- /dev/null
> +++ b/drivers/nubus/bus.c
> @@ -0,0 +1,119 @@
> +/*
> + * Bus implementation for the NuBus subsystem.
> + *
> + * Copyright (C) 2017 Finn Thain
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */

I think the SPDX id should be on the first line using C++-style // comments.
See tglx's doc patches and Linus posts explaining why he wants it this way!

-- 
Cordially
Philippe Ombredanne

Re: [PATCH v3 14/14] nubus: Add support for the driver model

[Greg Kroah-Hartman]

On Tue, Dec 05, 2017 at 01:20:02AM -0500, Finn Thain wrote:
> This patch brings basic support for the Linux Driver Model to the
> NuBus subsystem.
> 
> For flexibility, the matching of boards with drivers is left up to the
> drivers. This is also the approach taken by NetBSD. A board may have
> many functions, and drivers may have to consider many functional
> resources and board resources in order to match a device.
> 
> This implementation does not bind drivers to resources (nor does it bind
> many drivers to the same board). Apple's NuBus declaration ROM design
> is flexible enough to allow that, but I don't see a need to support it
> as we don't use the "slot zero" resources (in the main logic board ROM).
> 
> Eliminate the global nubus_boards linked list by rewriting the procfs
> board iterator around bus_for_each_dev(). Hence the nubus device refcount
> can be used to determine the lifespan of board objects.
> 
> Cc: Greg Kroah-Hartman 
> Tested-by: Stan Johnson 
> Signed-off-by: Finn Thain 

Nice job:

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v3 14/14] nubus: Add support for the driver model

[Greg Kroah-Hartman]

On Tue, Dec 05, 2017 at 01:20:02AM -0500, Finn Thain wrote:
> This patch brings basic support for the Linux Driver Model to the
> NuBus subsystem.
> 
> For flexibility, the matching of boards with drivers is left up to the
> drivers. This is also the approach taken by NetBSD. A board may have
> many functions, and drivers may have to consider many functional
> resources and board resources in order to match a device.
> 
> This implementation does not bind drivers to resources (nor does it bind
> many drivers to the same board). Apple's NuBus declaration ROM design
> is flexible enough to allow that, but I don't see a need to support it
> as we don't use the "slot zero" resources (in the main logic board ROM).
> 
> Eliminate the global nubus_boards linked list by rewriting the procfs
> board iterator around bus_for_each_dev(). Hence the nubus device refcount
> can be used to determine the lifespan of board objects.
> 
> Cc: Greg Kroah-Hartman 
> Tested-by: Stan Johnson 
> Signed-off-by: Finn Thain 

Nice job:

Reviewed-by: Greg Kroah-Hartman 

[PATCH v3 14/14] nubus: Add support for the driver model

[Finn Thain]

This patch brings basic support for the Linux Driver Model to the
NuBus subsystem.

For flexibility, the matching of boards with drivers is left up to the
drivers. This is also the approach taken by NetBSD. A board may have
many functions, and drivers may have to consider many functional
resources and board resources in order to match a device.

This implementation does not bind drivers to resources (nor does it bind
many drivers to the same board). Apple's NuBus declaration ROM design
is flexible enough to allow that, but I don't see a need to support it
as we don't use the "slot zero" resources (in the main logic board ROM).

Eliminate the global nubus_boards linked list by rewriting the procfs
board iterator around bus_for_each_dev(). Hence the nubus device refcount
can be used to determine the lifespan of board objects.

Cc: Greg Kroah-Hartman 
Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 

---
The conversion of Mac network drivers from the Space.c convention to
the Driver Model takes place in a separate patch series, archived at
https://lkml.org/lkml/2017/11/11/25
That series motivates parts of this design, such as the definition of
'for_each_board_func_rsrc'.
---
 drivers/nubus/Makefile |   2 +-
 drivers/nubus/bus.c| 119 +
 drivers/nubus/nubus.c  |  24 +-
 drivers/nubus/proc.c   |  55 +--
 include/linux/nubus.h  |  33 --
 5 files changed, 163 insertions(+), 70 deletions(-)
 create mode 100644 drivers/nubus/bus.c

diff --git a/drivers/nubus/Makefile b/drivers/nubus/Makefile
index 21bda2031e7e..6d063cde39d1 100644
--- a/drivers/nubus/Makefile
+++ b/drivers/nubus/Makefile
@@ -2,6 +2,6 @@
 # Makefile for the nubus specific drivers.
 #
 
-obj-y   := nubus.o
+obj-y := nubus.o bus.o
 
 obj-$(CONFIG_PROC_FS) += proc.o
diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c
new file mode 100644
index ..b5045e639b4d
--- /dev/null
+++ b/drivers/nubus/bus.c
@@ -0,0 +1,119 @@
+/*
+ * Bus implementation for the NuBus subsystem.
+ *
+ * Copyright (C) 2017 Finn Thain
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define to_nubus_board(d)   container_of(d, struct nubus_board, dev)
+#define to_nubus_driver(d)  container_of(d, struct nubus_driver, driver)
+
+static int nubus_bus_match(struct device *dev, struct device_driver *driver)
+{
+   return 1;
+}
+
+static int nubus_device_probe(struct device *dev)
+{
+   struct nubus_driver *ndrv = to_nubus_driver(dev->driver);
+   int err = -ENODEV;
+
+   if (ndrv->probe)
+   err = ndrv->probe(to_nubus_board(dev));
+   return err;
+}
+
+static int nubus_device_remove(struct device *dev)
+{
+   struct nubus_driver *ndrv = to_nubus_driver(dev->driver);
+   int err = -ENODEV;
+
+   if (dev->driver && ndrv->remove)
+   err = ndrv->remove(to_nubus_board(dev));
+   return err;
+}
+
+struct bus_type nubus_bus_type = {
+   .name   = "nubus",
+   .match  = nubus_bus_match,
+   .probe  = nubus_device_probe,
+   .remove = nubus_device_remove,
+};
+EXPORT_SYMBOL(nubus_bus_type);
+
+int nubus_driver_register(struct nubus_driver *ndrv)
+{
+   ndrv->driver.bus = _bus_type;
+   return driver_register(>driver);
+}
+EXPORT_SYMBOL(nubus_driver_register);
+
+void nubus_driver_unregister(struct nubus_driver *ndrv)
+{
+   driver_unregister(>driver);
+}
+EXPORT_SYMBOL(nubus_driver_unregister);
+
+static struct device nubus_parent = {
+   .init_name  = "nubus",
+};
+
+int __init nubus_bus_register(void)
+{
+   int err;
+
+   err = device_register(_parent);
+   if (err)
+   return err;
+
+   err = bus_register(_bus_type);
+   if (!err)
+   return 0;
+
+   device_unregister(_parent);
+   return err;
+}
+
+static void nubus_device_release(struct device *dev)
+{
+   struct nubus_board *board = to_nubus_board(dev);
+   struct nubus_rsrc *fres, *tmp;
+
+   list_for_each_entry_safe(fres, tmp, _func_rsrcs, list)
+   if (fres->board == board) {
+   list_del(>list);
+   kfree(fres);
+   }
+   kfree(board);
+}
+
+int nubus_device_register(struct nubus_board *board)
+{
+   board->dev.parent = _parent;
+   board->dev.release = nubus_device_release;
+   board->dev.bus = _bus_type;
+   dev_set_name(>dev, "slot.%X", board->slot);
+   return device_register(>dev);
+}
+
+static int nubus_print_device_name_fn(struct device *dev, void *data)
+{
+   struct nubus_board *board = to_nubus_board(dev);
+   struct seq_file *m = data;
+
+   seq_printf(m, "Slot %X: %s\n", board->slot, board->name);
+   return 0;
+}
+
+int nubus_proc_show(struct seq_file *m, void *data)
+{
+   return 

[PATCH v3 14/14] nubus: Add support for the driver model

[Finn Thain]

This patch brings basic support for the Linux Driver Model to the
NuBus subsystem.

For flexibility, the matching of boards with drivers is left up to the
drivers. This is also the approach taken by NetBSD. A board may have
many functions, and drivers may have to consider many functional
resources and board resources in order to match a device.

This implementation does not bind drivers to resources (nor does it bind
many drivers to the same board). Apple's NuBus declaration ROM design
is flexible enough to allow that, but I don't see a need to support it
as we don't use the "slot zero" resources (in the main logic board ROM).

Eliminate the global nubus_boards linked list by rewriting the procfs
board iterator around bus_for_each_dev(). Hence the nubus device refcount
can be used to determine the lifespan of board objects.

Cc: Greg Kroah-Hartman 
Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 

---
The conversion of Mac network drivers from the Space.c convention to
the Driver Model takes place in a separate patch series, archived at
https://lkml.org/lkml/2017/11/11/25
That series motivates parts of this design, such as the definition of
'for_each_board_func_rsrc'.
---
 drivers/nubus/Makefile |   2 +-
 drivers/nubus/bus.c| 119 +
 drivers/nubus/nubus.c  |  24 +-
 drivers/nubus/proc.c   |  55 +--
 include/linux/nubus.h  |  33 --
 5 files changed, 163 insertions(+), 70 deletions(-)
 create mode 100644 drivers/nubus/bus.c

diff --git a/drivers/nubus/Makefile b/drivers/nubus/Makefile
index 21bda2031e7e..6d063cde39d1 100644
--- a/drivers/nubus/Makefile
+++ b/drivers/nubus/Makefile
@@ -2,6 +2,6 @@
 # Makefile for the nubus specific drivers.
 #
 
-obj-y   := nubus.o
+obj-y := nubus.o bus.o
 
 obj-$(CONFIG_PROC_FS) += proc.o
diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c
new file mode 100644
index ..b5045e639b4d
--- /dev/null
+++ b/drivers/nubus/bus.c
@@ -0,0 +1,119 @@
+/*
+ * Bus implementation for the NuBus subsystem.
+ *
+ * Copyright (C) 2017 Finn Thain
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define to_nubus_board(d)   container_of(d, struct nubus_board, dev)
+#define to_nubus_driver(d)  container_of(d, struct nubus_driver, driver)
+
+static int nubus_bus_match(struct device *dev, struct device_driver *driver)
+{
+   return 1;
+}
+
+static int nubus_device_probe(struct device *dev)
+{
+   struct nubus_driver *ndrv = to_nubus_driver(dev->driver);
+   int err = -ENODEV;
+
+   if (ndrv->probe)
+   err = ndrv->probe(to_nubus_board(dev));
+   return err;
+}
+
+static int nubus_device_remove(struct device *dev)
+{
+   struct nubus_driver *ndrv = to_nubus_driver(dev->driver);
+   int err = -ENODEV;
+
+   if (dev->driver && ndrv->remove)
+   err = ndrv->remove(to_nubus_board(dev));
+   return err;
+}
+
+struct bus_type nubus_bus_type = {
+   .name   = "nubus",
+   .match  = nubus_bus_match,
+   .probe  = nubus_device_probe,
+   .remove = nubus_device_remove,
+};
+EXPORT_SYMBOL(nubus_bus_type);
+
+int nubus_driver_register(struct nubus_driver *ndrv)
+{
+   ndrv->driver.bus = _bus_type;
+   return driver_register(>driver);
+}
+EXPORT_SYMBOL(nubus_driver_register);
+
+void nubus_driver_unregister(struct nubus_driver *ndrv)
+{
+   driver_unregister(>driver);
+}
+EXPORT_SYMBOL(nubus_driver_unregister);
+
+static struct device nubus_parent = {
+   .init_name  = "nubus",
+};
+
+int __init nubus_bus_register(void)
+{
+   int err;
+
+   err = device_register(_parent);
+   if (err)
+   return err;
+
+   err = bus_register(_bus_type);
+   if (!err)
+   return 0;
+
+   device_unregister(_parent);
+   return err;
+}
+
+static void nubus_device_release(struct device *dev)
+{
+   struct nubus_board *board = to_nubus_board(dev);
+   struct nubus_rsrc *fres, *tmp;
+
+   list_for_each_entry_safe(fres, tmp, _func_rsrcs, list)
+   if (fres->board == board) {
+   list_del(>list);
+   kfree(fres);
+   }
+   kfree(board);
+}
+
+int nubus_device_register(struct nubus_board *board)
+{
+   board->dev.parent = _parent;
+   board->dev.release = nubus_device_release;
+   board->dev.bus = _bus_type;
+   dev_set_name(>dev, "slot.%X", board->slot);
+   return device_register(>dev);
+}
+
+static int nubus_print_device_name_fn(struct device *dev, void *data)
+{
+   struct nubus_board *board = to_nubus_board(dev);
+   struct seq_file *m = data;
+
+   seq_printf(m, "Slot %X: %s\n", board->slot, board->name);
+   return 0;
+}
+
+int nubus_proc_show(struct seq_file *m, void *data)
+{
+   return bus_for_each_dev(_bus_type, NULL, m,
+