[PATCH 02/14] ARM : SAMSUNG : Add RS485 support.
Paul Schilling
paul.s.schilling at gmail.com
Tue Nov 1 15:22:59 EDT 2011
Sorry for the formatting mess on the previous email. Lets try
this again.
I need an opinion on two issues before I resubmit the Samsung
RS-485 driver.
First of all the driver works. I have tested it on a logic analyser
and I found that it switches from transmit to receive in token
mode exactly after 1 bit of idle time as long as interrupts are kept
enabled and nothing tries to wait or sleep inside of an interrupt.
I found 2 maybe 3 places where this occurs up to 22 milliseconds.
The opinion part... I needed a timer to switch from transmit to
receive after the FIFO was empty. I started by using the low
resolution timer using jiffies first. I found that wasn't high enough
resolution, so I switched to the Linux HRT. Currently I have both
versions that can be selected by conditional compile. Should I
just remove the low resolution timer completely or leave it in.
Second, I have a chunk of code that if it could be made to work
could off load the receiving to DMA up to the last couple of bytes
then switch back to interrupts for the token byte. Should I leave
that code in a #if 0 statement or should I just delete it.
Thanks,
Paul Schilling
> On Tue, Nov 1, 2011 at 9:57 AM, Paul Schilling
> <paul.s.schilling at gmail.com> wrote:
>> I am working on resubmitting this stuff right now. I have 14 patches
>> that should be
>> broken into more like 20 patches. Most of them are ARM S3C and
>> specifically S3C2416 patches.
>> I am working on a patch right now to fix a Atomic wait in the DMA
>> code. That causes
>> 20 milliseconds of no interrupts whenever a sound is played on the samsung S3C
>> platform.
>>
>> The code below wasn't supposed to go out and I have patch that removes
>> that chunk
>> of code almost ready that just needs testing before I send it.
>>
>> On Tue, Nov 1, 2011 at 7:18 AM, Alan Cox <alan at linux.intel.com> wrote:
>>>> +config SAMSUNG_HAS_RS485
>>>> + bool "Enable RS485 support for Samsung"
>>>> + depends on SERIAL_SAMSUNG && (MACH_CONDOR2440 ||
>>>> MACH_CONDOR2416 || MACH_MINI2440)
>>>> + default y if (MACH_CONDOR2440 || MACH_CONDOR2416)
>>>> + default n if (MACH_MINI2440)
>>>
>>> There really needs to be more ARM thinking about doing this sort of
>>> stuff at runtime. If you can detect the board type at runtime then you
>>> need to be moving towards kicking board specifics out of the depends,
>>> and treating it as an "include support for this, if the board has it"
>>> so you can move towards less build time only configuration.
>>>
>>
>> The chunk of code that says FIXME is an optimization that could be
>> implemented to
>> decrease interrupts by using DMA until the last byte is ready to be
>> sent. I have a new
>> patch that explains in a comment what the code is for.
>>
>>>> +/* FIXME */
>>>> + #if 0
>>>> + if (last_state != en) {
>>>> +
>>>
>>> So this doesn't look ready to submit either...
>>>
>>
>> I have a patch that I haven't sent yet that fixes the code formating
>> issues. I need to compile and test it before I send it out.
>>
>>>
>>>> + if ((utrstat & S3C2410_UTRSTAT_TXE) ? 1 : 0) {
>>>> + if (cfg->gpio_transmit_en > -1) {
>>>> +
>>>> gpio_set_value(cfg->gpio_transmit_en, 0);
>>>> + }
>>>
>>> Lots of excess brackets (see CodingStyle) - removing a few of the
>>> excess ones would make it easier to read later.
>>>
>>> The later bits become a real mess of ifdefs - much not your fault, the
>>> combination of your ifdefs and existing lack of design push it beyond
>>> ugly and really want addressing.
>>>
>>>
>>
>
More information about the linux-arm-kernel
mailing list