[LinuxPPS] [PATCH 2/2] pps: new client driver using IRQs

[James Nuss]

Hi Igor,

I agree, platform_data is a good candidate for this type of parameter.
Where is the appropriate place to keep the header file containing the
struct declaration(s)? e.g in your case "struct
pps_client_gpio_platform_data" and "struct pps_client_gpio" Presumably
it needs to be in a readily accessible header file so the board setup
code can use it.

cheers,
James

>
> This way can lead to problems when developer tries to debug PPS subsystem
> with some specific signal parameters.
> In my opinion it much safer to explicitly declare ".active_low = 0" or
> ".active_low = 1" in the platform initialization code.
> See example:
>
> /*
>  * pps client gpio
>  */
> static struct pps_client_gpio pps_client_gpios[] = {
>    {
>        .gpio            = GPI_PPS0_IN,
>        .active_low        = 0, /* ASSERT is a _/ */
>        .desc            = "pps0 source",
>        .decoder_type        = PPS_DECODER_GEOSIG_T1PPS,
>    },
>    {
>        .gpio            = GPI_PPS1_IN,
>        .active_low        = 1, /* ASSERT is a \_ */
>        .desc            = "pps1 source",
>    }
> };
>
> static struct pps_client_gpio_platform_data pps_client_gpios_data = {
>    .pclient    = pps_client_gpios,
>    .num_gpios    = ARRAY_SIZE(pps_client_gpios),
> };
>
> static struct platform_device pps_client_gpio_device = {
>    .name        = "pps-client-gpio",
>    .id        = 0,
>    .dev        = {
>        .platform_data    = &pps_client_gpios_data,
>    }
> };
>
>
>> If I was to implement the driver this way then you would have exactly
>> 3 ways to register a device to use this driver:
>>
>> 1) Register an IRQ with only IORESOURCE_IRQ_HIGHEDGE set:
>> This will generate an ASSERT event on rising edges (no CLEAR events)
>>
>> 2) Register an IRQ with only IORESOURCE_IRQ_LOWEDGE set:
>> This will generate an ASSERT event on falling edges (no CLEAR events)
>>
>> 3) Register an IRQ with both IORESOURCE_IRQ_LOWEDGE and
>> IORESOURCE_IRQ_HIGHEDGE set:
>> This will generate ASSERT and CLEAR events with the driver dynamically
>> determining which edge is the ASSERT based on the logic above.
>>
>> I hope this covers all the potential use cases.
>>
>> cheers,
>> James
>>
>>
>> On Fri, Apr 29, 2011 at 4:26 AM, Rodolfo Giometti<giometti at enneenne.com>
>>  wrote:
>>>
>>> On Fri, Apr 29, 2011 at 08:29:55AM +0400, Igor Plyatov wrote:
>>>
>>>>>> The latter will definitely mess things up, right?
>>>>>
>>>>> I mean, one surely can register an IRQ resource with both flags set.
>>>>> And
>>>>> if the underlying hardware works as it is described (i.e. raises an irq
>>>>> on both edges) then it will be a problem.
>>>>
>>>> Please don't try to abandon one of ASSERT or CLEAR events!
>>>> It is very useful to register both of them, because in this case its
>>>> possible to measure pulse width and decode PPS signals like DCF77.
>>>
>>> At this point I suppose we should add both ASSERT and CLEAR events...
>>>
>>> Ciao,
>>>
>>> Rodolfo
>>>
>>> --
>>>
>>> GNU/Linux Solutions                  e-mail: giometti at enneenne.com
>>> Linux Device Driver                          giometti at linux.it
>>> Embedded Systems                     phone:  +39 349 2432127
>>> UNIX programming                     skype:  rodolfo.giometti
>>> Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel"
>>> in
>>> the body of a message to majordomo at vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>>>
>
> Best regards!
>
> --
> Igor Plyatov
>
>