Re: [PATCH v4 1/4] can: dev: Add support for limiting configured bitrate

From: Sergei Shtylyov
Date: Thu Aug 10 2017 - 06:05:24 EST


Hello!

On 8/10/2017 3:59 AM, Franklin S Cooper Jr wrote:

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 <fcooper@xxxxxx>
---
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
[...]
@@ -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.

+
+ 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...

+
+ if (max_bitrate > 0) {

You risk checking unset variable here.

+ priv->max_bitrate = max_bitrate;
+ priv->is_bitrate_limited = true;
+ } else if (ret != -EINVAL) {
+ netdev_warn(dev, "Invalid value for transceiver max bitrate\n");
+ }
+}
+EXPORT_SYMBOL(of_can_transceiver);
+#endif
+
/*
* Common close function for cleanup before the device gets closed.
*
[...]

MBR, Sergei