The patch allows calling request_firmware without a 'struct device'.
It appears we just need a name here from 'struct device'. I changed it
to use a kobject as Patrick suggested.
Next patch will use the new API to request firmware (microcode) for a CPU.
Signed-off-by: Shaohua Li <[email protected]>
---
linux-2.6.17-rc4-root/drivers/base/firmware_class.c | 32 ++++++++++++--------
linux-2.6.17-rc4-root/include/linux/firmware.h | 2 +
2 files changed, 22 insertions(+), 12 deletions(-)
diff -puN drivers/base/firmware_class.c~request_firmware_nodevice drivers/base/firmware_class.c
--- linux-2.6.17-rc4/drivers/base/firmware_class.c~request_firmware_nodevice 2006-05-15 14:25:09.000000000 +0800
+++ linux-2.6.17-rc4-root/drivers/base/firmware_class.c 2006-05-22 07:30:51.000000000 +0800
@@ -301,15 +301,15 @@ firmware_class_timeout(u_long data)
}
static inline void
-fw_setup_class_device_id(struct class_device *class_dev, struct device *dev)
+fw_setup_class_device_id(struct class_device *class_dev, struct kobject *kobj)
{
/* XXX warning we should watch out for name collisions */
- strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
+ strlcpy(class_dev->class_id, kobj->k_name, BUS_ID_SIZE);
}
static int
fw_register_class_device(struct class_device **class_dev_p,
- const char *fw_name, struct device *device)
+ const char *fw_name, struct kobject *kobj)
{
int retval;
struct firmware_priv *fw_priv = kzalloc(sizeof(*fw_priv),
@@ -333,8 +333,7 @@ fw_register_class_device(struct class_de
fw_priv->timeout.data = (u_long) fw_priv;
init_timer(&fw_priv->timeout);
- fw_setup_class_device_id(class_dev, device);
- class_dev->dev = device;
+ fw_setup_class_device_id(class_dev, kobj);
class_dev->class = &firmware_class;
class_set_devdata(class_dev, fw_priv);
retval = class_device_register(class_dev);
@@ -354,14 +353,14 @@ error_kfree:
static int
fw_setup_class_device(struct firmware *fw, struct class_device **class_dev_p,
- const char *fw_name, struct device *device, int uevent)
+ const char *fw_name, struct kobject *kobj, int uevent)
{
struct class_device *class_dev;
struct firmware_priv *fw_priv;
int retval;
*class_dev_p = NULL;
- retval = fw_register_class_device(&class_dev, fw_name, device);
+ retval = fw_register_class_device(&class_dev, fw_name, kobj);
if (retval)
goto out;
@@ -401,7 +400,7 @@ out:
static int
_request_firmware(const struct firmware **firmware_p, const char *name,
- struct device *device, int uevent)
+ struct kobject *kobj, int uevent)
{
struct class_device *class_dev;
struct firmware_priv *fw_priv;
@@ -419,7 +418,7 @@ _request_firmware(const struct firmware
goto out;
}
- retval = fw_setup_class_device(firmware, &class_dev, name, device,
+ retval = fw_setup_class_device(firmware, &class_dev, name, kobj,
uevent);
if (retval)
goto error_kfree_fw;
@@ -477,7 +476,15 @@ request_firmware(const struct firmware *
struct device *device)
{
int uevent = 1;
- return _request_firmware(firmware_p, name, device, uevent);
+ return _request_firmware(firmware_p, name, &device->kobj, uevent);
+}
+
+int
+request_firmware_kobj(const struct firmware **firmware_p, const char *name,
+ struct kobject *kobj)
+{
+ int uevent = 1;
+ return _request_firmware(firmware_p, name, kobj, uevent);
}
/**
@@ -534,7 +541,7 @@ request_firmware_work_func(void *arg)
return 0;
}
daemonize("%s/%s", "firmware", fw_work->name);
- ret = _request_firmware(&fw, fw_work->name, fw_work->device,
+ ret = _request_firmware(&fw, fw_work->name, &fw_work->device->kobj,
fw_work->uevent);
if (ret < 0)
fw_work->cont(NULL, fw_work->context);
@@ -624,10 +631,11 @@ firmware_class_exit(void)
class_unregister(&firmware_class);
}
-module_init(firmware_class_init);
+fs_initcall(firmware_class_init);
module_exit(firmware_class_exit);
EXPORT_SYMBOL(release_firmware);
EXPORT_SYMBOL(request_firmware);
+EXPORT_SYMBOL(request_firmware_kobj);
EXPORT_SYMBOL(request_firmware_nowait);
EXPORT_SYMBOL(register_firmware);
diff -puN include/linux/firmware.h~request_firmware_nodevice include/linux/firmware.h
--- linux-2.6.17-rc4/include/linux/firmware.h~request_firmware_nodevice 2006-05-15 14:26:34.000000000 +0800
+++ linux-2.6.17-rc4-root/include/linux/firmware.h 2006-05-22 07:24:08.000000000 +0800
@@ -13,6 +13,8 @@ struct firmware {
struct device;
int request_firmware(const struct firmware **fw, const char *name,
struct device *device);
+int request_firmware_kobj(const struct firmware **fw, const char *name,
+ struct kobject *kobj);
int request_firmware_nowait(
struct module *module, int uevent,
const char *name, struct device *device, void *context,
_
On Thu, May 25, 2006 at 11:50:45AM +0800, Shaohua Li wrote:
> The patch allows calling request_firmware without a 'struct device'.
> It appears we just need a name here from 'struct device'. I changed it
> to use a kobject as Patrick suggested.
> Next patch will use the new API to request firmware (microcode) for a CPU.
But a cpu does have a struct device. Why not just use that?
> +fw_setup_class_device_id(struct class_device *class_dev, struct kobject *kobj)
> {
> /* XXX warning we should watch out for name collisions */
> - strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
> + strlcpy(class_dev->class_id, kobj->k_name, BUS_ID_SIZE);
There's a function for this, kobject_name(), please never touch k_name
directly.
> +EXPORT_SYMBOL(request_firmware_kobj);
Ick, if you really want to do this, just fix up all callers of
request_firmware(), there aren't that many of them.
But I don't recommend it anyway.
thanks,
greg k-h
On Wed, 2006-05-24 at 21:01 -0700, Greg KH wrote:
> On Thu, May 25, 2006 at 11:50:45AM +0800, Shaohua Li wrote:
> > The patch allows calling request_firmware without a 'struct device'.
> > It appears we just need a name here from 'struct device'. I changed it
> > to use a kobject as Patrick suggested.
> > Next patch will use the new API to request firmware (microcode) for a CPU.
>
> But a cpu does have a struct device. Why not just use that?
It's a sysdev, no 'struct device' in it, IIRC.
> > +fw_setup_class_device_id(struct class_device *class_dev, struct kobject *kobj)
> > {
> > /* XXX warning we should watch out for name collisions */
> > - strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
> > + strlcpy(class_dev->class_id, kobj->k_name, BUS_ID_SIZE);
>
> There's a function for this, kobject_name(), please never touch k_name
> directly.
Ok, will do.
> > +EXPORT_SYMBOL(request_firmware_kobj);
>
> Ick, if you really want to do this, just fix up all callers of
> request_firmware(), there aren't that many of them.
>
> But I don't recommend it anyway.
I didn't see why we need a 'struct device' for request_firmware. It just
needs a name to me.
Thanks,
Shaohua
Hi Greg,
> > The patch allows calling request_firmware without a 'struct device'.
> > It appears we just need a name here from 'struct device'. I changed it
> > to use a kobject as Patrick suggested.
> > Next patch will use the new API to request firmware (microcode) for a CPU.
>
> But a cpu does have a struct device. Why not just use that?
>
> > +fw_setup_class_device_id(struct class_device *class_dev, struct kobject *kobj)
> > {
> > /* XXX warning we should watch out for name collisions */
> > - strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
> > + strlcpy(class_dev->class_id, kobj->k_name, BUS_ID_SIZE);
>
> There's a function for this, kobject_name(), please never touch k_name
> directly.
>
> > +EXPORT_SYMBOL(request_firmware_kobj);
>
> Ick, if you really want to do this, just fix up all callers of
> request_firmware(), there aren't that many of them.
>
> But I don't recommend it anyway.
I also disagree with this change at all. The callers of request_firmware
should not fiddle around with kobject's to make this work. All of them
have their struct device and they should use it.
So I would propose that we fix the caller and the not request_firmware
code. However one option would be calling it with NULL as device
argument and it registers itself a dummy device for the operation.
Regards
Marcel
Hi Shaohua,
> > > The patch allows calling request_firmware without a 'struct device'.
> > > It appears we just need a name here from 'struct device'. I changed it
> > > to use a kobject as Patrick suggested.
> > > Next patch will use the new API to request firmware (microcode) for a CPU.
> >
> > But a cpu does have a struct device. Why not just use that?
> It's a sysdev, no 'struct device' in it, IIRC.
do we really need to differentiate between sysdev and device anymore. I
recall a plan to unify all devices, but I might be wrong.
Regards
Marcel
On Thu, May 25, 2006 at 12:26:21PM +0200, Marcel Holtmann wrote:
> Hi Shaohua,
>
> > > > The patch allows calling request_firmware without a 'struct device'.
> > > > It appears we just need a name here from 'struct device'. I changed it
> > > > to use a kobject as Patrick suggested.
> > > > Next patch will use the new API to request firmware (microcode) for a CPU.
> > >
> > > But a cpu does have a struct device. Why not just use that?
> > It's a sysdev, no 'struct device' in it, IIRC.
>
> do we really need to differentiate between sysdev and device anymore. I
> recall a plan to unify all devices, but I might be wrong.
I don't think we need to keep them different anymore, they should be
made the same. Then issues like this will not happen.
And it will make your driver code smaller :)
thanks,
greg k-h
On Thu, 2006-05-25 at 12:24 +0200, Marcel Holtmann wrote:
> Hi Greg,
>
> > > The patch allows calling request_firmware without a 'struct device'.
> > > It appears we just need a name here from 'struct device'. I changed it
> > > to use a kobject as Patrick suggested.
> > > Next patch will use the new API to request firmware (microcode) for a CPU.
> >
> > But a cpu does have a struct device. Why not just use that?
> >
> > > +fw_setup_class_device_id(struct class_device *class_dev, struct kobject *kobj)
> > > {
> > > /* XXX warning we should watch out for name collisions */
> > > - strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
> > > + strlcpy(class_dev->class_id, kobj->k_name, BUS_ID_SIZE);
> >
> > There's a function for this, kobject_name(), please never touch k_name
> > directly.
> >
> > > +EXPORT_SYMBOL(request_firmware_kobj);
> >
> > Ick, if you really want to do this, just fix up all callers of
> > request_firmware(), there aren't that many of them.
> >
> > But I don't recommend it anyway.
>
> I also disagree with this change at all. The callers of request_firmware
> should not fiddle around with kobject's to make this work. All of them
> have their struct device and they should use it.
So why we need a 'struct device'? I didn't see any point we need it. We
just need a 'name'.
> So I would propose that we fix the caller and the not request_firmware
> code. However one option would be calling it with NULL as device
> argument and it registers itself a dummy device for the operation.
This doesn't work, as we need a 'name'.
do we really need to differentiate between sysdev and device anymore. I
> recall a plan to unify all devices, but I might be wrong.
I'd like this idea. But it means many works. In addition, a sysdev could
have multiple drivers, and a 'device' can't to me.
Thanks,
Shaohua
On Fri, May 26, 2006 at 11:40:39AM +0800, Shaohua Li wrote:
> On Thu, 2006-05-25 at 12:24 +0200, Marcel Holtmann wrote:
> > Hi Greg,
> >
> > > > The patch allows calling request_firmware without a 'struct device'.
> > > > It appears we just need a name here from 'struct device'. I changed it
> > > > to use a kobject as Patrick suggested.
> > > > Next patch will use the new API to request firmware (microcode) for a CPU.
> > >
> > > But a cpu does have a struct device. Why not just use that?
> > >
> > > > +fw_setup_class_device_id(struct class_device *class_dev, struct kobject *kobj)
> > > > {
> > > > /* XXX warning we should watch out for name collisions */
> > > > - strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
> > > > + strlcpy(class_dev->class_id, kobj->k_name, BUS_ID_SIZE);
> > >
> > > There's a function for this, kobject_name(), please never touch k_name
> > > directly.
> > >
> > > > +EXPORT_SYMBOL(request_firmware_kobj);
> > >
> > > Ick, if you really want to do this, just fix up all callers of
> > > request_firmware(), there aren't that many of them.
> > >
> > > But I don't recommend it anyway.
> >
> > I also disagree with this change at all. The callers of request_firmware
> > should not fiddle around with kobject's to make this work. All of them
> > have their struct device and they should use it.
> So why we need a 'struct device'? I didn't see any point we need it. We
> just need a 'name'.
You need a kobject, as ideally we would have a symlink back to the
"real" kobject. So far, only "devices" need firmware, that's why it is
that way.
> > So I would propose that we fix the caller and the not request_firmware
> > code. However one option would be calling it with NULL as device
> > argument and it registers itself a dummy device for the operation.
> This doesn't work, as we need a 'name'.
>
> do we really need to differentiate between sysdev and device anymore. I
> > recall a plan to unify all devices, but I might be wrong.
> I'd like this idea. But it means many works. In addition, a sysdev could
> have multiple drivers, and a 'device' can't to me.
Today a sysdev can have that? Ick. Any examples of ones that really
do?
thanks,
greg k-h
On Thu, 2006-05-25 at 21:06 -0700, Greg KH wrote:
> On Fri, May 26, 2006 at 11:40:39AM +0800, Shaohua Li wrote:
> > On Thu, 2006-05-25 at 12:24 +0200, Marcel Holtmann wrote:
> > > Hi Greg,
> > >
> > > > > The patch allows calling request_firmware without a 'struct device'.
> > > > > It appears we just need a name here from 'struct device'. I changed it
> > > > > to use a kobject as Patrick suggested.
> > > > > Next patch will use the new API to request firmware (microcode) for a CPU.
> > > >
> > > > But a cpu does have a struct device. Why not just use that?
> > > >
> > > > > +fw_setup_class_device_id(struct class_device *class_dev, struct kobject *kobj)
> > > > > {
> > > > > /* XXX warning we should watch out for name collisions */
> > > > > - strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
> > > > > + strlcpy(class_dev->class_id, kobj->k_name, BUS_ID_SIZE);
> > > >
> > > > There's a function for this, kobject_name(), please never touch k_name
> > > > directly.
> > > >
> > > > > +EXPORT_SYMBOL(request_firmware_kobj);
> > > >
> > > > Ick, if you really want to do this, just fix up all callers of
> > > > request_firmware(), there aren't that many of them.
> > > >
> > > > But I don't recommend it anyway.
> > >
> > > I also disagree with this change at all. The callers of request_firmware
> > > should not fiddle around with kobject's to make this work. All of them
> > > have their struct device and they should use it.
> > So why we need a 'struct device'? I didn't see any point we need it. We
> > just need a 'name'.
>
> You need a kobject, as ideally we would have a symlink back to the
> "real" kobject. So far, only "devices" need firmware, that's why it is
> that way.
> > > So I would propose that we fix the caller and the not request_firmware
> > > code. However one option would be calling it with NULL as device
> > > argument and it registers itself a dummy device for the operation.
> > This doesn't work, as we need a 'name'.
> >
> > do we really need to differentiate between sysdev and device anymore. I
> > > recall a plan to unify all devices, but I might be wrong.
> > I'd like this idea. But it means many works. In addition, a sysdev could
> > have multiple drivers, and a 'device' can't to me.
>
> Today a sysdev can have that? Ick. Any examples of ones that really
> do?
At least cpu sysdev has two drivers currently. one is mtrr driver the
other is cpurfreq.
Thanks,
Shaohua