Re: [patch] hdaps driver update.

[Greg KH]

On Wed, Sep 14, 2005 at 12:09:35PM -0400, Robert Love wrote:
> On Wed, 2005-09-14 at 09:05 -0700, Greg KH wrote:
> 
> > Woah, no, this is not ok.  Please see my objections to this the zillion
> > other times people have tried to do this...
> > 
> > Why is this static?  Shouldn't it be dynamic and then your release would
> > be able to free the memory?
> 
> The release only happens on module unload, the device is not
> hotpluggable, and thus we'd gain the memory anyhow.
> 
> So it is static the way any other no-need-to-dynamically-create data
> structure would be.
> 
> No?

But you are reference counting a static object, right?  Which isn't the
nicest thing to have done.  Why not just dynamically create it?

> > >  static struct device_driver hdaps_driver = {
> > >  	.name = "hdaps",
> > >  	.bus = &platform_bus_type,
> > > -	.owner = THIS_MODULE,
> > >  	.probe = hdaps_probe,
> > >  	.resume = hdaps_resume
> > >  };
> > 
> > Why delete that?  You just lost your symlink in sysfs then :(
> 
> I don't follow.
> 
> Wouldn't we want to be removed from sysfs?

No, if you have that .owner field in your driver, you get a symlink in
sysfs that points from your driver to the module that controls it.  You
just removed that symlink, which is not what I think you wanted to have
happen :(

I also think you don't get the module reference counting for your
driver's and devices sysfs files but haven't looked deep enough to see
if this is true for your code or not.  Should be easy for you to test,
just open a sysfs file for your device and see if the module reference
is incremented or not.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/