Grant Likely wrote: > On Sat, May 23, 2009 at 10:44 AM, Wolfgang Grandegger <w...@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. >>>>> +- 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. > 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. > Segher, what's your opinion on this? I personally don't have a real preference. >> 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. > 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. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
