On Wed, Mar 21, 2012 at 12:04:50PM +0400, Alexander Pazdnikov wrote:
> Tue, 20 Mar 2012 05:26:04 -0700 от Greg KH <greg@xxxxxxxxx>:
> > On Tue, Mar 20, 2012 at 10:37:12AM +0400, Alexander Pazdnikov wrote:
> > > Fri, 16 Mar 2012 08:28:50 -0700 от Greg KH <greg@xxxxxxxxx>:
> > >
> > > > 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.
So you are duplicating that driver?
> 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.
If you can get the SPI developers to ack your patch, then I'll accept
it, otherwise I still don't feel this is the correct way to handle this
type of driver, sorry.
So please work with the SPI developers on this, on their mailing list.
When finished, please resend it to me.
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
