Re: [PATCH v4 WIP 2/4] i2c-parport: modify driver to use new parport device model

[Sudip Mukherjee <sudipm.mukherjee@gmail.com>]

On Mon, May 04, 2015 at 08:58:58AM +0200, Jean Delvare wrote:
> On Mon, 4 May 2015 11:10:12 +0530, Sudip Mukherjee wrote:
> > On Sun, May 03, 2015 at 03:33:40PM +0200, Jean Delvare wrote:
> > > On Tue, 28 Apr 2015 17:00:21 +0530, Sudip Mukherjee wrote:
<snip>
> > I didnot get this warning, maybe I need to upgrade my gcc or will
> > W=1 show it?
> 
> I see it without W=1, as the message says this type of warning is
> enabled by default (gcc 4.8.1.) The check on const vs. non-const
> pointers is very old so I am very surprised that you don't see it, even
> if your gcc isn't recent.
then maybe I have not looked closely. I will pay attention and look for
it this time. My version is 4.7.3.
> 
<snip>
> > remains registered, then what happens to the driver?
> 
> This was a key design decision of the (then) new device driver model in
> kernel v2.5 that the lifetime of drivers should be independent from
> the lifetime of device instances. Ideally, devices are even created
> and deleted outside their driver. That works well for enumerated
> devices such as PCI or USB devices. That won't work in your case
> because parallel port devices have no unique ID so they can't be
> enumerated.
> 
> Still, the lifetime of devices should be independent from the lifetime
> of the driver. The driver should be registered as long as the module is
> loaded. The devices, however, must be created and deleted dynamically
> whenever the relevant parallel ports appear or disappear from the
> system.
> 
> So basically the module's __init function should only register the
> driver, and let the core call back its probe function for every parallel
> port available on the system at that time. And likewise, the __exit
> function should only unregister the driver, and let the core call back
> its remove function for every device that still exists at that point in
> time. This is what the platform bus subsystem does (see for example
> drivers/i2c/busses/i2c-viperboard.c but there are many many others.)
Thanks. This explained many of the doubts I was having. And I have one
more doubt and I need some suggestion for it too.
This current version of the code will register devices like :
If i register i2c-parport0 with parport0 then the sys tree will be:
		sys
	 ________|____________
		| 
	     parport
	 _____|_______    
         |           |
     parport0   i2c-parport0
	|
   i2c-parport0

so basically it registers as a subdevice of parport0 and also a device
in the bus. And this is the reason why i needed the device_type.
But i think it is wrong. I think it should have been just:
		sys
	 ________|____________
		| 
	     parport
	 _____|_______    
	      |          
	 parport0   
            |
      i2c-parport0

so, which one is actually correct?

Thanks again for that explanation, that really helped a lot.

regards
sudip


> 
> > my next WIP will have some changes in the core level also, so I shouldnot
> > add your Tested-by: to it. And I will again request you to check that.
> 
> I will do, no problem.
> 
> -- 
> Jean Delvare
> SUSE L3 Support