On 02/04/11 15:34, Hennerich, Michael wrote:
> Greg KH wrote on 2011-02-04:
>> On Fri, Feb 04, 2011 at 08:38:16AM +0000, Hennerich, Michael wrote:
>>> Greg KH wrote on 2011-02-03:
>>>> On Thu, Feb 03, 2011 at 09:58:33AM +0000, Hennerich, Michael wrote:
>>>>> Greg KH wrote on 2011-02-02:
>>>>>> On Wed, Feb 02, 2011 at 08:36:27PM +0000, Hennerich, Michael
>> wrote:
>>>>>>> Greg KH wrote on 2011-02-02:
>>>>>>>> On Wed, Feb 02, 2011 at 08:21:08PM +0100,
>>>>>>>> michael.hennerich@xxxxxxxxxx
>>>>>>>> wrote:
>>>>>>>>> From: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
>>>>>>>>>
>>>>>>>>> This patch adds a new trigger that can be invoked by writing the
>>>>>>>>> sysfs file: trigger_now. This approach can be valuable during
>>>>>>>>> automated testing or in situations, where other trigger methods
>>>>>>>>> are not applicable. For example no RTC or spare GPIOs. Last but
>>>>>>>>> not least we can allow user space applications to produce
>>>>>>>>> triggers.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
>>>>>>>>> Acked-by: Jonathan Cameron <jic23@xxxxxxxxx>
>>>>>>>>
>>>>>>>> If you add a new sysfs file, you need to document it. I'll
>>>>>>>> not take this patch as-is because of that.
>>>>>>>>
>>>>>>>>> ---
>>>>>>>>> drivers/staging/iio/trigger/Kconfig | 6 ++
>>>>>>>>> drivers/staging/iio/trigger/Makefile | 1 +
>>>>>>>>> drivers/staging/iio/trigger/iio-trig-sysfs.c | 108
>>>>>>>>> ++++++++++++++++++++++++++ 3 files changed, 115
>>>>>>>>> ++++++++++++++++++++++++++ insertions(+), 0
>>>>>>>>> deletions(-) create mode
>>>>>>>>> 100644 drivers/staging/iio/trigger/iio-trig-sysfs.c
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/staging/iio/trigger/Kconfig
>>>>>>>>> b/drivers/staging/iio/trigger/Kconfig
>>>>>>>>> index d842a58..c185e47 100644
>>>>>>>>> --- a/drivers/staging/iio/trigger/Kconfig
>>>>>>>>> +++ b/drivers/staging/iio/trigger/Kconfig
>>>>>>>>> @@ -18,4 +18,10 @@ config IIO_GPIO_TRIGGER
>>>>>>>>> help
>>>>>>>>> Provides support for using GPIO pins as IIO triggers.
>>>>>>>>> +config IIO_SYSFS_TRIGGER
>>>>>>>>> + tristate "SYSFS trigger"
>>>>>>>>> + depends on SYSFS
>>>>>>>>> + help
>>>>>>>>> + Provides support for using SYSFS entry as IIO triggers.
>>>>>>>>
>>>>>>>> Why would you ever want to not have this enabled? Why is it a
>>>>>>>> config option? And shouldn't it depend on the IIO subsystem?
>>>>>>>
>>>>>>> You won't see this if you don't have IIO + triggers enabled.
>>>>>>
>>>>>> Ok, the dependancy on IIO comes from other parts of the Kconfig
>>>>>> file, my mistake.
>>>>>>
>>>>>>> Are you asking me to add more help text here - or you didn't read
>>>>>>> the commit log? Alternatively are you asking why IIO common trigger
>>>>>>> core doesn't provide this flexibility?
>>>>>>>
>>>>>>> Sorry - please explain.
>>>>>>
>>>>>> My question is, why is this a config option at all? Why would
>>>>>> it not always be enabled? What benefit is it if it is not enabled?
>>>>>
>>>>> It's a config option, since the user must provide a platform
>>>>> resource for it.
>>>>
>>>> What platform resource?
>>>
>>> Resource is probably a bit misleading - But your platform/board
>>> dependant init code must register a struct platform_device using
>>> platform_add_device() and friends. If it does it multiple times, with
>>> different ids. multiple triggers are created.
>>
>> That's fine, but it's not what you are doing here, right?
>
> That's what I'm doing in my board setup code, it's just not part of this patch.
>
> static struct platform_device iio_sysfs_trigger = {
> .name = "iio_sysfs_trigger",
> .id = 0,
> };
>
> static struct platform_device iio_sysfs_trigger1 = {
> .name = "iio_sysfs_trigger",
> .id = 1,
> };
>
> static struct platform_device *my_devices[] __initdata = {
>
> #if defined(CONFIG_IIO_SYSFS_TRIGGER) || defined(CONFIG_IIO_SYSFS_TRIGGER _MODULE)
> &iio_sysfs_trigger,
> &iio_sysfs_trigger1,
> #endif
> }
>
>>>>> If you're not planning to use it, why compile and include it at
>> all?
>>>>
>>>> If you are a distro, how are you supposed to know if this is
>>>> something you want or not?
>>>
>>> It's likely something that you don't want.
>>
>> Then say so in the help section of the Kconfig file.
>
> ok
>
>> And also consider not including the file at all, if it's not something
>> you really want :)
>
> It's useful for everyone doing automated driver tests.
> And this driver is provided in the hope it will be useful for others too.
>
> Jonathan - can do you think it is useful?
It is. I'll probably try adding dynamic instantiation when I have a chance,
but in the meantime definitely a handy bit of functionality to have.
>
>
>>>> If this is going to be the proper API for interacting with iio
>>>> devices, then it needs to always be present in the core, otherwise
>>>> your userspace tools are going to get messy, right?
>>>
>>> Consider it as something like all the other iio drivers.
>>> The user has the option to enable the drivers that are required for
>>> its application. The hardware interacting with these drivers need to
>>> be present, and the user has to provide information on how these are
>> physically connected.
>>
>> No, the "other" drivers get loaded on demand if the hardware is
>> present or not, which is a big difference, right?
>
> None of the IIO drivers work that way.
>
> Unlike PCI or USB devices, SPI or I2C devices are not enumerated at the hardware level.
> Instead, the software must know which devices are connected on each SPI bus segment,
> and what slave selects/address these devices are using.
> For this reason, the kernel code must instantiate SPI devices explicitly.
> The most common method is to declare the SPI devices by bus number.
>
>>> If you don't enable a IIO input driver that supports triggered sampling
>>> support. You won't need any trigger driver. If you enable multiple of
>>> such drivers. You may want to trigger a few with GPIO some others with
>>> RTC and maybe one with this sysfs based trigger.
>>
>> And how does a user know to do one vs. the other?
>
> It's purely application specific.
> It's like connecting wires on a PCB.
Not quite. There are generic boards which get used for a number of applications
(take the memsic imote2's or gumstix for example). It's this that would motivate
the addition of on demand registration of these.
<snip>
> Isn't is still an argument that you don't enable kernel options or drivers
> that you don't understand or likely going to use?
Not everyone seems to keep to that or to take a look at whether the
things they build are applicable to any box their distro will actually work on.
Ubuntu 10.10 on one of my x86 boxes (2.6.35) currently has:
iio/accel:
adis16209.ko adis16220.ko adis16240.ko kxsd9.ko lis3l02dq.ko sca3000.ko
iio/adc:
max1363.ko
iio/gyro:
adis16260.ko
iio/imu:
adis16300.ko adis16350.ko adis16400.ko
iio/light:
tsl2563.ko
iio/trigger:
iio-trig-gpio.ko iio-trig-periodic-rtc.ko
Looking at it another way... We get a lot of free build testing from the distros ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
