Re: [RFC PATCH 1/3] siox: new driver/bus framework for Eckelmann SIOX

[Uwe Kleine-König]

Hello Greg,

On Fri, Mar 11, 2016 at 02:20:39PM -0800, Greg Kroah-Hartman wrote:
> On Fri, Mar 11, 2016 at 10:52:12PM +0100, Uwe Kleine-König wrote:
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> > ---
> >  drivers/Kconfig          |   2 +
> >  drivers/Makefile         |   1 +
> >  drivers/siox/Kconfig     |   2 +
> >  drivers/siox/Makefile    |   1 +
> >  drivers/siox/siox-core.c | 572 +++++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/siox/siox.h      |  48 ++++
> >  include/linux/siox.h     |  53 +++++
> >  7 files changed, 679 insertions(+)
> >  create mode 100644 drivers/siox/Kconfig
> >  create mode 100644 drivers/siox/Makefile
> >  create mode 100644 drivers/siox/siox-core.c
> >  create mode 100644 drivers/siox/siox.h
> >  create mode 100644 include/linux/siox.h
> 
> What's the relationship between your "master" and "regular" devices
> here?  I think that's getting things confused, you are unregistering
> your 'master' yet the children it controls don't go away because you
> never unregister them.  You need to fix this heirachy up a bit, and
> maybe just drop the 'master' logic?

A master is the bus controller (i.e. it drives DOUT, DCLK and DLD). A
device is a consumer and provides DIN to the master.

The topology looks as follows:

      ,------->--DCLK-->---------------+----------------------.
      ^                                v                      v
 ,--------.                ,----------------------.       ,------
 |        |                |   ,--------------.   |       |
 |        |--->--DOUT-->---|->-|shift register|->-|--->---|
 |        |                |   `--------------'   |       |
 | master |                |        device        |       |  device
 |        |                |   ,--------------.   |       |
 |        |---<--DIN---<---|-<-|shift register|-<-|---<---|
 |        |                |   `--------------'   |       |
 `--------'                `----------------------'       `------
      v                                ^                      ^
      `----------DLD-------------------+----------------------'

So there are two chains of shift registers, one for letting the master
write to the devices, and the other to let the master read from the
devices. DCLK is the clock to shift both chains by a single bit. An edge
on DLD is used to make the devices load the inputs and sample the
outputs.

I think I need the following:

	void siox_unregister_master(struct siox_master *smaster)
	{
		for (each siox_device d of smaster)
			siox_device_remove(d);

		device_unregister(&smaster->dev);
	}

But the locking scheme that is necessary isn't obvious for me. For
example I must prevent that after the for loop is done a new device is
registered. Do I need to prevent this explicitly (e.g. by doing
something like:

	lock(smaster);
	smaster->is_going_away = true;
	for (each siox_device d of smaster)
		siox_device_remove(d);
	unlock(smaster);

	device_unregister(&smaster->dev);

and let registering new devices fail if smaster->is_going_away is true?)
Or is there something that the kobject stuff/driver core can help me
with?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |