Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
On Sun, Feb 10, 2013 at 08:36:23AM -0800, Greg KH wrote:
> On Sun, Feb 10, 2013 at 04:32:18AM +0100, Samuel Ortiz wrote:
> > Hi Greg,
> >
> > On Fri, Feb 08, 2013 at 03:55:24PM -0800, Greg KH wrote:
> > > On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
> > > > From: Samuel Ortiz
> > > >
> > > > Signed-off-by: Samuel Ortiz
> > > > Signed-off-by: Tomas Winkler
> > > > Acked-by: Arnd Bergmann
> > > > ---
> > > > drivers/misc/mei/bus.c | 29 +
> > > > include/linux/mei_bus.h |3 +++
> > > > 2 files changed, 32 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> > > > index bb96423c..0a5e624 100644
> > > > --- a/drivers/misc/mei/bus.c
> > > > +++ b/drivers/misc/mei/bus.c
> > > > @@ -153,3 +153,32 @@ void mei_remove_device(struct mei_bus_client
> > > > *client)
> > > > device_unregister(>dev);
> > > > }
> > > > EXPORT_SYMBOL(mei_remove_device);
> > > > +
> > > > +int mei_driver_register(struct mei_bus_driver *driver)
> > > > +{
> > > > + int err;
> > > > +
> > > > + /* Can't register until after driver model init */
> > > > + if (unlikely(WARN_ON(!mei_bus_type.p)))
> > > > + return -EAGAIN;
> > >
> > > No, you really don't know what 'p' is, so don't assume you do. You have
> > > fields you can test to see if your driver model is up and registered,
> > > use them instead.
> > Unless I'm missing something, I would actually need to track if the bus type
> > is registered or not from bus.c. I can't find any field from bus_type that
> > tells me for sure if my bus is registered or not, except for p.
> > I actually think I don't need that check anyway. IIRC, this is something I
> > took from i2c-core.c but in the ME case it's most likely not needed.
>
> It's your bus, if you need to check this, then have your own flag, don't
> poke in the driver core internals. As you point out, I don't think you
> need to check this at all.
Yes, I removed the check. Tomas should sent v3 later today with all your
comments
adressed.
Thanks for the review.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
On Sun, Feb 10, 2013 at 08:36:23AM -0800, Greg KH wrote:
On Sun, Feb 10, 2013 at 04:32:18AM +0100, Samuel Ortiz wrote:
Hi Greg,
On Fri, Feb 08, 2013 at 03:55:24PM -0800, Greg KH wrote:
On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
From: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Tomas Winkler tomas.wink...@intel.com
Acked-by: Arnd Bergmann a...@arndb.de
---
drivers/misc/mei/bus.c | 29 +
include/linux/mei_bus.h |3 +++
2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index bb96423c..0a5e624 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -153,3 +153,32 @@ void mei_remove_device(struct mei_bus_client
*client)
device_unregister(client-dev);
}
EXPORT_SYMBOL(mei_remove_device);
+
+int mei_driver_register(struct mei_bus_driver *driver)
+{
+ int err;
+
+ /* Can't register until after driver model init */
+ if (unlikely(WARN_ON(!mei_bus_type.p)))
+ return -EAGAIN;
No, you really don't know what 'p' is, so don't assume you do. You have
fields you can test to see if your driver model is up and registered,
use them instead.
Unless I'm missing something, I would actually need to track if the bus type
is registered or not from bus.c. I can't find any field from bus_type that
tells me for sure if my bus is registered or not, except for p.
I actually think I don't need that check anyway. IIRC, this is something I
took from i2c-core.c but in the ME case it's most likely not needed.
It's your bus, if you need to check this, then have your own flag, don't
poke in the driver core internals. As you point out, I don't think you
need to check this at all.
Yes, I removed the check. Tomas should sent v3 later today with all your
comments
adressed.
Thanks for the review.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
On Sun, Feb 10, 2013 at 04:32:18AM +0100, Samuel Ortiz wrote:
> Hi Greg,
>
> On Fri, Feb 08, 2013 at 03:55:24PM -0800, Greg KH wrote:
> > On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
> > > From: Samuel Ortiz
> > >
> > > Signed-off-by: Samuel Ortiz
> > > Signed-off-by: Tomas Winkler
> > > Acked-by: Arnd Bergmann
> > > ---
> > > drivers/misc/mei/bus.c | 29 +
> > > include/linux/mei_bus.h |3 +++
> > > 2 files changed, 32 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> > > index bb96423c..0a5e624 100644
> > > --- a/drivers/misc/mei/bus.c
> > > +++ b/drivers/misc/mei/bus.c
> > > @@ -153,3 +153,32 @@ void mei_remove_device(struct mei_bus_client *client)
> > > device_unregister(>dev);
> > > }
> > > EXPORT_SYMBOL(mei_remove_device);
> > > +
> > > +int mei_driver_register(struct mei_bus_driver *driver)
> > > +{
> > > + int err;
> > > +
> > > + /* Can't register until after driver model init */
> > > + if (unlikely(WARN_ON(!mei_bus_type.p)))
> > > + return -EAGAIN;
> >
> > No, you really don't know what 'p' is, so don't assume you do. You have
> > fields you can test to see if your driver model is up and registered,
> > use them instead.
> Unless I'm missing something, I would actually need to track if the bus type
> is registered or not from bus.c. I can't find any field from bus_type that
> tells me for sure if my bus is registered or not, except for p.
> I actually think I don't need that check anyway. IIRC, this is something I
> took from i2c-core.c but in the ME case it's most likely not needed.
It's your bus, if you need to check this, then have your own flag, don't
poke in the driver core internals. As you point out, I don't think you
need to check this at all.
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
On Sun, Feb 10, 2013 at 04:32:18AM +0100, Samuel Ortiz wrote:
Hi Greg,
On Fri, Feb 08, 2013 at 03:55:24PM -0800, Greg KH wrote:
On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
From: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Tomas Winkler tomas.wink...@intel.com
Acked-by: Arnd Bergmann a...@arndb.de
---
drivers/misc/mei/bus.c | 29 +
include/linux/mei_bus.h |3 +++
2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index bb96423c..0a5e624 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -153,3 +153,32 @@ void mei_remove_device(struct mei_bus_client *client)
device_unregister(client-dev);
}
EXPORT_SYMBOL(mei_remove_device);
+
+int mei_driver_register(struct mei_bus_driver *driver)
+{
+ int err;
+
+ /* Can't register until after driver model init */
+ if (unlikely(WARN_ON(!mei_bus_type.p)))
+ return -EAGAIN;
No, you really don't know what 'p' is, so don't assume you do. You have
fields you can test to see if your driver model is up and registered,
use them instead.
Unless I'm missing something, I would actually need to track if the bus type
is registered or not from bus.c. I can't find any field from bus_type that
tells me for sure if my bus is registered or not, except for p.
I actually think I don't need that check anyway. IIRC, this is something I
took from i2c-core.c but in the ME case it's most likely not needed.
It's your bus, if you need to check this, then have your own flag, don't
poke in the driver core internals. As you point out, I don't think you
need to check this at all.
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
Hi Greg,
On Fri, Feb 08, 2013 at 03:55:24PM -0800, Greg KH wrote:
> On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
> > From: Samuel Ortiz
> >
> > Signed-off-by: Samuel Ortiz
> > Signed-off-by: Tomas Winkler
> > Acked-by: Arnd Bergmann
> > ---
> > drivers/misc/mei/bus.c | 29 +
> > include/linux/mei_bus.h |3 +++
> > 2 files changed, 32 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> > index bb96423c..0a5e624 100644
> > --- a/drivers/misc/mei/bus.c
> > +++ b/drivers/misc/mei/bus.c
> > @@ -153,3 +153,32 @@ void mei_remove_device(struct mei_bus_client *client)
> > device_unregister(>dev);
> > }
> > EXPORT_SYMBOL(mei_remove_device);
> > +
> > +int mei_driver_register(struct mei_bus_driver *driver)
> > +{
> > + int err;
> > +
> > + /* Can't register until after driver model init */
> > + if (unlikely(WARN_ON(!mei_bus_type.p)))
> > + return -EAGAIN;
>
> No, you really don't know what 'p' is, so don't assume you do. You have
> fields you can test to see if your driver model is up and registered,
> use them instead.
Unless I'm missing something, I would actually need to track if the bus type
is registered or not from bus.c. I can't find any field from bus_type that
tells me for sure if my bus is registered or not, except for p.
I actually think I don't need that check anyway. IIRC, this is something I
took from i2c-core.c but in the ME case it's most likely not needed.
> > + driver->driver.owner = THIS_MODULE;
>
> Nope, this should be the module that owns the driver, not this one. It
> needs to be passed in with the call, or as part of the driver structure.
That's right, I'll fix that.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
Hi Greg,
On Fri, Feb 08, 2013 at 03:55:24PM -0800, Greg KH wrote:
On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
From: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Tomas Winkler tomas.wink...@intel.com
Acked-by: Arnd Bergmann a...@arndb.de
---
drivers/misc/mei/bus.c | 29 +
include/linux/mei_bus.h |3 +++
2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index bb96423c..0a5e624 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -153,3 +153,32 @@ void mei_remove_device(struct mei_bus_client *client)
device_unregister(client-dev);
}
EXPORT_SYMBOL(mei_remove_device);
+
+int mei_driver_register(struct mei_bus_driver *driver)
+{
+ int err;
+
+ /* Can't register until after driver model init */
+ if (unlikely(WARN_ON(!mei_bus_type.p)))
+ return -EAGAIN;
No, you really don't know what 'p' is, so don't assume you do. You have
fields you can test to see if your driver model is up and registered,
use them instead.
Unless I'm missing something, I would actually need to track if the bus type
is registered or not from bus.c. I can't find any field from bus_type that
tells me for sure if my bus is registered or not, except for p.
I actually think I don't need that check anyway. IIRC, this is something I
took from i2c-core.c but in the ME case it's most likely not needed.
+ driver-driver.owner = THIS_MODULE;
Nope, this should be the module that owns the driver, not this one. It
needs to be passed in with the call, or as part of the driver structure.
That's right, I'll fix that.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
> From: Samuel Ortiz
>
> Signed-off-by: Samuel Ortiz
> Signed-off-by: Tomas Winkler
> Acked-by: Arnd Bergmann
> ---
> drivers/misc/mei/bus.c | 29 +
> include/linux/mei_bus.h |3 +++
> 2 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> index bb96423c..0a5e624 100644
> --- a/drivers/misc/mei/bus.c
> +++ b/drivers/misc/mei/bus.c
> @@ -153,3 +153,32 @@ void mei_remove_device(struct mei_bus_client *client)
> device_unregister(>dev);
> }
> EXPORT_SYMBOL(mei_remove_device);
> +
> +int mei_driver_register(struct mei_bus_driver *driver)
> +{
> + int err;
> +
> + /* Can't register until after driver model init */
> + if (unlikely(WARN_ON(!mei_bus_type.p)))
> + return -EAGAIN;
No, you really don't know what 'p' is, so don't assume you do. You have
fields you can test to see if your driver model is up and registered,
use them instead. Don't you think that something called 'p' is not
there for you to use?
> + driver->driver.owner = THIS_MODULE;
Nope, this should be the module that owns the driver, not this one. It
needs to be passed in with the call, or as part of the driver structure.
> + driver->driver.bus = _bus_type;
> +
> + err = driver_register(>driver);
> + if (err)
> + return err;
> +
> + pr_debug("mei: driver [%s] registered\n", driver->driver.name);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(mei_driver_register);
_GPL() please?
> +
> +void mei_driver_unregister(struct mei_bus_driver *driver)
> +{
> + driver_unregister(>driver);
> +
> + pr_debug("mei: driver [%s] unregistered\n", driver->driver.name);
> +}
> +EXPORT_SYMBOL(mei_driver_unregister);
Same here.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
On Friday 08 February 2013 14:28:15 Tomas Winkler wrote: > From: Samuel Ortiz > > Signed-off-by: Samuel Ortiz > Signed-off-by: Tomas Winkler Acked-by: Arnd Bergmann -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
On Friday 08 February 2013 14:28:15 Tomas Winkler wrote: From: Samuel Ortiz sa...@linux.intel.com Signed-off-by: Samuel Ortiz sa...@linux.intel.com Signed-off-by: Tomas Winkler tomas.wink...@intel.com Acked-by: Arnd Bergmann a...@arndb.de -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
From: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Tomas Winkler tomas.wink...@intel.com
Acked-by: Arnd Bergmann a...@arndb.de
---
drivers/misc/mei/bus.c | 29 +
include/linux/mei_bus.h |3 +++
2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index bb96423c..0a5e624 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -153,3 +153,32 @@ void mei_remove_device(struct mei_bus_client *client)
device_unregister(client-dev);
}
EXPORT_SYMBOL(mei_remove_device);
+
+int mei_driver_register(struct mei_bus_driver *driver)
+{
+ int err;
+
+ /* Can't register until after driver model init */
+ if (unlikely(WARN_ON(!mei_bus_type.p)))
+ return -EAGAIN;
No, you really don't know what 'p' is, so don't assume you do. You have
fields you can test to see if your driver model is up and registered,
use them instead. Don't you think that something called 'p' is not
there for you to use?
+ driver-driver.owner = THIS_MODULE;
Nope, this should be the module that owns the driver, not this one. It
needs to be passed in with the call, or as part of the driver structure.
+ driver-driver.bus = mei_bus_type;
+
+ err = driver_register(driver-driver);
+ if (err)
+ return err;
+
+ pr_debug(mei: driver [%s] registered\n, driver-driver.name);
+
+ return 0;
+}
+EXPORT_SYMBOL(mei_driver_register);
_GPL() please?
+
+void mei_driver_unregister(struct mei_bus_driver *driver)
+{
+ driver_unregister(driver-driver);
+
+ pr_debug(mei: driver [%s] unregistered\n, driver-driver.name);
+}
+EXPORT_SYMBOL(mei_driver_unregister);
Same here.
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
