Hello!

On 08/17/2017 08:54 PM, Franklin S Cooper Jr wrote:

quoted

quoted

Various CAN or CAN-FD IP may be able to run at a faster rate than
what the transceiver the CAN node is connected to. This can lead to
unexpected errors. However, CAN transceivers typically have fixed
limitations and provide no means to discover these limitations at
runtime. Therefore, add support for a can-transceiver node that
can be reused by other CAN peripheral drivers to determine for both
CAN and CAN-FD what the max bitrate that can be used. If the user
tries to configure CAN to pass these maximum bitrates it will throw
an error.

Signed-off-by: Franklin S Cooper Jr <redacted>
---
Version 4 changes:
Used can-transceiver instead of fixed-transceiver.

   drivers/net/can/dev.c   | 50
+++++++++++++++++++++++++++++++++++++++++++++++++
   include/linux/can/dev.h |  5 +++++
   2 files changed, 55 insertions(+)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 365a8cc..372108f 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c

[...]

quoted

@@ -814,6 +815,39 @@ int open_candev(struct net_device *dev)
   }
   EXPORT_SYMBOL_GPL(open_candev);
   +#ifdef CONFIG_OF
+/*
+ * Common function that can be used to understand the limitation of
+ * a transceiver when it provides no means to determine these

limitations
+ * at runtime.
+ */
+void of_can_transceiver(struct net_device *dev)
+{
+    struct device_node *dn;
+    struct can_priv *priv = netdev_priv(dev);
+    struct device_node *np;
+    unsigned int max_bitrate;
+    int ret;
+
+    np = dev->dev.parent->of_node;

    I'd do that as an initializer.

Ok

quoted

quoted

+
+    dn = of_get_child_by_name(np, "can-transceiver");
+    if (!dn)
+        return;
+
+    max_bitrate = 0;
+    ret = of_property_read_u32(dn, "max-bitrate", &max_bitrate);

    I'd initialize max_bitrate to 0 as iff of_property_read_u32() fails,
it'll leave the variable unset...>

quoted

+
+    if (max_bitrate > 0) {

    You risk checking unset variable here.

Four lines up I am already setting max_bitrate to a default value of 0.

    Oops, sorry. Don't know how I missed that. Better done as initializer too 
though...

MBR, Sergei