On 07/26/2017 12:05 PM, Oliver Hartkopp wrote: > On 07/26/2017 06:41 PM, Andrew Lunn wrote: >> On Mon, Jul 24, 2017 at 06:05:19PM -0500, Franklin S Cooper Jr wrote: > >>> + >>> +Optional: >>> + max-arbitration-speed: a positive non 0 value that determines the max >>> + speed that CAN can run in non CAN-FD mode or during the >>> + arbitration phase in CAN-FD mode. >> >> Hi Franklin >> >> Since this and the next property are optional, it is good to document >> what happens when they are not in the DT blob. Also document what 0 >> means. The driver ignores values less than 0 with the exception being max-data-speed which supports a value of -1. Not sure what I'm documenting when the binding specifically says to use a positive non zero value. Its the same reason I don't document what happens if you give it a negative value. >> >>> + >>> + max-data-speed: a positive non 0 value that determines the max >>> data rate >>> + that can be used in CAN-FD mode. A value of -1 implies >>> + CAN-FD is not supported by the transceiver. >> >> -1 is ugly. I think it would be better to have a missing >> max-data-speed property indicate that CAN-FD is not supported. > Although this leads to your later point I don't think this is the right approach. Its an optional property. Not including the property should not assume it isn't supported. > Thanks Andrew! I had the same feeling about '-1' :-) Ok I'll go back to having 0 = not supported. Which will handle the documentation comment above. > >> And >> maybe put 'fd' into the property name. > > Good point. In fact the common naming to set bitrates for CAN(FD) > controllers are 'bitrate' and 'data bitrate'. > > 'speed' is not really a good word for that. I'm fine with switching to using bitrate instead of speed. Kurk was originally the one that suggested to use the term arbitration and data since thats how the spec refers to it. Which I do agree with. But your right that in the drivers (struct can_priv) we just use bittiming and data_bittiming (CAN-FD timings). I don't think adding "fd" into the property name makes sense unless we are calling it something like "max-canfd-bitrate" which I would agree is the easiest to understand. So what is the preference if we end up sticking with two properties? Option 1 or 2? 1) max-bitrate max-data-bitrate 2) max-bitrate max-canfd-bitrate > > Finally, @Franklin: > > A CAN transceiver is limited in bandwidth. But you only have one RX and > one TX line between the CAN controller and the CAN transceiver. The > transceiver does not know about CAN FD - it has just a physical(!) layer > with a limited bandwidth. This is ONE limitation. > > So I tend to specify only ONE 'max-bitrate' property for the > fixed-transceiver binding. > > The fact whether the CAN controller is CAN FD capable or not is provided > by the netlink configuration interface for CAN controllers. Part of the reasoning to have two properties is to indicate that you don't support CAN FD while limiting the "arbitration" bit rate. With one property you can not determine this and end up having to make some assumptions that can quickly end up biting people. > > Regards, > Oliver > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
