Grant Likely wrote:
quoted text > On Sat, May 23, 2009 at 10:44 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> Wolfgang Grandegger wrote:
>>> Grant Likely wrote:
>>>>> +- clock-frequency : CAN system clock frequency in Hz, which is normally
>>>>> + half of the oscillator clock frequency. If not specified, a
>>>>> + default value of 8000000 (8 MHz) is used.
>>>> A clock-frequency property typically refers to the bus clock
>>>> frequency. Something like can-frequency would be better.
>>> Ah, right, but I'm also not happy with "can-frequency". The manual
>>> speaks about the "internal clock", which is half of the external
>>> oscillator frequency. Maybe "internal-clock-frequency" might be better.
>
> Would it be better to specify the external clock frequency, and the
> driver know that internal freq is half that? I ask because external
> clock freq is a value the HW designer actually has control over.
I'm sharing your arguments: "external-clock-frequency". There is always
some confusion about external and internal clock frequency but the name
should make it clear.
quoted text >>>>> +- cdr-reg : value of the SJA1000 clock divider register according to
>>>>> + the SJA1000 data sheet. If not specified, a default value of
>>>>> + 0x48 is used.
>>>> Ewh. The driver should be clueful enough to derive the clock divider
>>>> value given both the bus and can frequencies. I don't like this
>>>> property.
>>> The clock divider register controls the CLKOUT frequency for the
>>> microcontroller another CAN controller and allows to deactivate the
>>> CLKOUT pin. It's not used to configure the CAN bus frequency.
>>>
>>>>> +- ocr-reg : value of the SJA1000 output control register according to
>>>>> + the SJA1000 data sheet. If not specified, a default value of
>>>>> + 0x0a is used.
>>>> Ditto here; the binding should describe the usage mode; not the
>>>> register settings to get the usage mode. What sort of settings will
>>>> the .dts author be writing here?
>>> Unfortunately, there are many:
>>>
>>> clkout-frequency
>>> bypass-comperator
>>> tx1-output-on-rx-irq
>>>
>>> #define OCR_MODE_BIPHASE 0x00
>>> #define OCR_MODE_TEST 0x01
>>> #define OCR_MODE_NORMAL 0x02
>>> #define OCR_MODE_CLOCK 0x03
>>>
>>> #define OCR_TX0_INVERT 0x04
>>> #define OCR_TX0_PULLDOWN 0x08
>>> #define OCR_TX0_PULLUP 0x10
>>> #define OCR_TX0_PUSHPULL 0x18
>>> #define OCR_TX1_INVERT 0x20
>>> #define OCR_TX1_PULLDOWN 0x40
>>> #define OCR_TX1_PULLUP 0x80
>>> #define OCR_TX1_PUSHPULL 0xc0
>>>
>>> I think implementing properties for each option is overkill.
>
> Ugh, I see what you mean.
>
>> Would the following more descriptive properties be OK?
>>
>> clock-out-frequency = <0>, // CLKOUT pin clock off
>> = <4000000>; // frequency on CLKOUT pin
>
> Or how about CLKOUT pin off if the property isn't present? Otherwise,
Yep, that will be the default anyhow.
quoted text > this looks okay. BTW, I'd consider prefixing this with 'nxp,' or
> 'sja1000,' to protect the namespace. clock-out-frequency sounds like
> one of those names which could be commonly used in the future. I'd so
> the same for the other chip-specific properties too.
quoted text > Segher, what's your opinion on this?
I personally don't have a real preference.
quoted text >> bypass-input-comparator; // allows to bypass the CAN input comparator.
>>
>> tx1-output-on-rx-irq; // allows the TX1 output to be used as a
>> // dedicated RX interrupt output.
>>
>> output-control-mode = <0x0> // bi-phase output mode
>> <0x1> // test output mode
>> <0x2> // normal output mode (default)
>> <0x3> // clock output mode
>>
>> output-pin-config = <0x01> // TX0 invert
>> <0x02> // TX0 pull-down
>> <0x04> // TX0 pull-up
>> <0x06> // TX0 push-pull
>> <0x08> // TX1 invert
>> <0x10> // TX1 pull-down
>> <0x20> // TX1 pull-up
>> <0x30> // TX1 push-pull
>
> hmmm; It is very chip specific and it is a lot of mucking around. If
> you prefix the property with the manufacturer name, then perhaps the
> explicit register setting is okay.
OK.
quoted text > Are TX0 & TX1 protocol pins or GPIOs? If gpio, then maybe it is worth
> the mucking about to then use the gpios binding to specify the pin
> mode.
These are the output from the CAN output driver 0 or 1 to the physical
bus line. E.g., in normal output mode the CAN bit sequence is sent via
TX0 or TX1. From a non-hardware expert point of view, they must be
programmed properly to get the hardware to work.
Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to
majordomo@vger.kernel.org
More majordomo info at
http://vger.kernel.org/majordomo-info.html
