RE: Update to CYGPKG_IO_ADC

["Christophe Coutand" <ccoutand at stmi dot com>]

Hi Sergei,

Thanks for your inputs. Attached is the updated patch. Let me know if more changes are required.

Christophe

-----Original Message-----
From: Sergei Gavrikov [mailto:sergei.gavrikov@gmail.com] 
Sent: 25. mai 2010 22:21
To: Christophe Coutand
Cc: eCos Patches
Subject: RE: Update to CYGPKG_IO_ADC

On Tue, 25 May 2010, Christophe Coutand wrote:
> Hi Sergei,
>
> I have generated a new patch using CYG_IO_SET_CONFIG_ADC_DATA_FLUSH 
> instead.

Hi Christophe,

Thank you, I will take a look.

> The second patch is pending. I have fixed the compilation error due to 
> CYGPKG_DEVS_ADC_ARM_AT91_DEBUG_LEVEL == 0, remove the CRs and cleanup 
> the CDL file.

Okay, no issue.

> I am however not sure what you mean by placing 
> CYGNUM_IO_ADC_SAMPLE_SIZE inside a CDL component.
> CYGNUM_IO_ADC_SAMPLE_SIZE is defined from the IO/ADC package, am I 
> allowed to redefine it inside a CDL component in the DEVS/ADC?

There are 2 lines on top-level the package:

     requires    {CYGNUM_IO_ADC_SAMPLE_SIZE <= 12}
     requires    {CYGNUM_IO_ADC_SAMPLE_SIZE >= 10}

And I had a thought about some CDL wrapping the limit, e.g.

     cdl_component CYGSEM_DEVS_ADC_ARM_AT91_SAMPLE_SIZE_LIMIT {
         display       "Sample size limit"
         flavor        bool
         calculated    1
         requires      {CYGNUM_IO_ADC_SAMPLE_SIZE >= 10}
         requires      {CYGNUM_IO_ADC_SAMPLE_SIZE <= 12}
         description   "
             Tell user about the limitation."
     }

> Also I never use tabulation so if you can pin-point where you have 
> find one, I will investigate.

I believe that you don't, but your $EDITOR does. I attach those lines if you ask.  It exists many ways to catch TABs/CRs, the simplest CLI way (on Linux I think):

grep '[press ^V][press TAB]' input       ;# to catch TABs
grep '[press ^V][press RETURN]' input    ;# to catch CRs

Sometimes you can see in a pager [more|less] a wrong indenting lines (my editor can highlight TABS too if I want).

Regards,
Sergei

> Regards,
> Christophe
>
> -----Original Message-----
> From: Sergei Gavrikov [mailto:sergei.gavrikov@gmail.com]
> Sent: 25. mai 2010 12:50
> To: Christophe Coutand
> Cc: ecos-patches@ecos.sourceware.org
> Subject: Re: Update to CYGPKG_IO_ADC
>
> On Sat, 22 May 2010, Christophe Coutand wrote:
>> Added option to flush the ADC sample buffer.
>>
>> Christophe
>
> Hi Christophe,
>
> First, thank you for your contribution! Some things so far (first look 
> on both patches)
>
> 1) I could not compile the driver, if cdl_option 
> CYGPKG_DEVS_ADC_ARM_AT91_DEBUG_LEVEL != 0: (test target to build the 
> driver was sam7ex256):
>
> ecos/packages/devs/adc/arm/at91/current/src/adc_at91.c: In function ‘at91_adc_enable’:
> ecos/packages/devs/adc/arm/at91/current/src/adc_at91.c:342: error: 
> ‘at91_adc_info’ has no member named ‘zero_time’
>
> Could you fix this, please?
>
> 2) Minor
>
> Could you please, clean-up the first patch (src), i.e. remove CRs, 
> expand TABs, fix some indents there; format (wrap on) a few long lines in CDL.
>
> It seemed for me those direct requirements of a range ([10; 12]) for 
> CYGNUM_IO_ADC_SAMPLE_SIZE on top your CDL can be placed in a separate 
> CDL component.
>
> About new I/O key for config_key.h (your second patch), I would prefer 
> rename your CYG_IO_SET_CONFIG_ADC_FLUSH_DATABUF, because all *flush* 
> key names end itself on _FLUSH. What would you say about:
> CYG_IO_SET_CONFIG_ADC_DATA_FLUSH?
>
> Note: In the next patches do not diff ecos.db, please. Send new 
> db-entries as a separate file.
>
> Thank you,
> Sergei
>

Attachment: at91_adc_v2
Description: at91_adc_v2

Attachment: at91_adc.db
Description: at91_adc.db