Re[2]: [PATCH 1/2] staging/comedi/drivers: add driver for ad7739 analog to digital converter chip on an spi bus

Alexander Pazdnikov pazdnikov at list.ru
Wed Mar 21 08:04:50 UTC 2012


Tue, 20 Mar 2012 05:26:04 -0700 от Greg KH <greg at kroah.com>:
> On Tue, Mar 20, 2012 at 10:37:12AM +0400, Alexander Pazdnikov wrote:
> > Fri, 16 Mar 2012 08:28:50 -0700 от Greg KH <greg at kroah.com>:
> >
> > > You should probably take this out, along with the call below where you
> > > use it, as it's not needed anymore, right?
> > >
> > > > +	snprintf(devname, sizeof(devname), "%s%u.%u", spi_bus_type.name,
> > > > +		 (unsigned)((dev->iobase >> 8) & 0xFF),
> > > > +		 (unsigned)(dev->iobase & 0xFF));
> > > > +
> > > > +	d = bus_find_device_by_name(&spi_bus_type, NULL, devname);
> > > > +	if (!d) {
> > > > +		dev_err(dev->class_dev, "devices %s not found\n", devname);
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	priv->spi = to_spi_device(d);
> > >
> > > Is that how you register a spi device normally?  I don't see any other
> > > SPI drivers doing this, why are you not just registering the driver with
> > > the bus like others do?
> >
> > Because it is not an spi driver like other spi drivers are, it is
> > based on another common spi driver - spidev, and it is only a high
> > level protocol driver for ad7739 chip.
> > I've used platform startup code to setup platform devices.
> > On embedded systems chips are unlikly to be pluggable, socame to a
> > disicion, that it is more error safe to try to setup an ad7739 chip
> > only for already registered spi devices.
> > ad7739 device is registered from user space by issueing a command of the form:
> >
> >     comedi_config /dev/comedi10 ad7739 0x106
> >
> > After that we can use /dev/comedi10 to work with ad7739 chip by common
> > functions, i.e. to get analog data from one channel with comedi_lib
> > userspace functions.
> > Comedi is a framework for common use of data acquisition purpuses from
> > different analog converters and digital input/outputs.
> >
> > Of course, it will be more pluggable, if based spidev device will be registered here.
> > I didn't find any practical needs to make it more pluggable and to
> > register based spidev device indeed of just searching for such a
> > device, may be i'm wrong.
> 
> I still don't understand, sorry.  You shouldn't be poking around in the
> core of the SPI layer with a "simple" driver like this, especially as no
> other SPI driver does this, right?
> 
> What is wrong with using the "correct" SPI driver interface for this
> driver?  Why can't you do that?
> 
> greg k-h
> 

Hello.

I've viewed some other standalone spi drivers and weighed the pros and cons again.

So, think, that my approach is optimal for comedi_lib device driver area.
I've taken for comparison drivers from drivers/gpio because of there similarness.
ad7739 driver is not a standalone driver, like for example drivers/gpio/gpio-74x164.c

gpio-74x164.c registers also a gpio_chip device - a usefull client side part of this driver.
Note, they are also configured through platform_data in a board BSP, no plug-and-play available.

Let's back to ad7739. In the kernel there is no support code for working with analog converters, so there is no need to register any usefull client device (like chip_gpio).
My work mostly is centred on providing comedi device framework an access to analog channels of the ad7739 chip. To do this, there is a need to negotiate with ad7739 chip through an spi bus.

There is already well written code in drivers/spi/spidev.c - common user space driver, and I've decided to use it for spi device driver configuration purpuses.

Otherwise.
1. I'll have to copy-paste the portion of init code from this driver (spidev.c), and have to register my ad7739 driver inside spi bus system.
2. In my brief preview, I'll have to copy-paste some locking logic to prevent simultaneous access from different comedi_lib devices(for example when two comdei devices are assigned to a one physical device). 

Taking into account the above, think that it is optimal to use existing code driver base for spi access and configuration purposes (spidev.c), and to realize only buisness logic for comedi_lib device framework.

Waitng for your opinion.

--
Best regards,
Alexander Pazdnikov


More information about the devel mailing list