[PATCH v2 0/4] can: Add new binding to limit bit rate used

[PATCH v2 0/4] can: Add new binding to limit bit rate used

[Franklin S Cooper Jr]

Add a new generic binding that CAN drivers can use to specify the max
arbitration and data bit rate supported by a transceiver. This is
useful since in some instances the maximum speeds may be limited by
the transceiver used. However, transceivers may not provide a means
to determine this limitation at runtime. Therefore, create a new binding
that mimics "fixed-link" that allows a user to hardcode the max speeds
that can be used.

Also add support for this new binding in the MCAN driver.

Note this is an optional subnode so even if a driver adds support for
parsing fixed-transceiver the user does not have to define it in their
device tree.

Version 2 changes:
Rename function
Define proper variable default
Drop unit address
Move check to changelink function
Reword commit message
Reword documentation

Franklin S Cooper Jr (4):
  can: dev: Add support for limiting configured bitrate
  can: fixed-transceiver: Add documentation for CAN fixed transceiver
    bindings
  can: m_can: Update documentation to mention new fixed transceiver
    binding
  can: m_can: Add call to of_can_transceiver_fixed

 .../bindings/net/can/fixed-transceiver.txt         | 42 +++++++++++++++
 .../devicetree/bindings/net/can/m_can.txt          | 10 ++++
 drivers/net/can/dev.c                              | 59 ++++++++++++++++++++++
 drivers/net/can/m_can/m_can.c                      |  2 +
 include/linux/can/dev.h                            |  5 ++
 5 files changed, 118 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/fixed-transceiver.txt

-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/4] can: Add new binding to limit bit rate used

[Franklin S Cooper Jr]

Add a new generic binding that CAN drivers can use to specify the max
arbitration and data bit rate supported by a transceiver. This is
useful since in some instances the maximum speeds may be limited by
the transceiver used. However, transceivers may not provide a means
to determine this limitation at runtime. Therefore, create a new binding
that mimics "fixed-link" that allows a user to hardcode the max speeds
that can be used.

Also add support for this new binding in the MCAN driver.

Note this is an optional subnode so even if a driver adds support for
parsing fixed-transceiver the user does not have to define it in their
device tree.

Version 2 changes:
Rename function
Define proper variable default
Drop unit address
Move check to changelink function
Reword commit message
Reword documentation

Franklin S Cooper Jr (4):
  can: dev: Add support for limiting configured bitrate
  can: fixed-transceiver: Add documentation for CAN fixed transceiver
    bindings
  can: m_can: Update documentation to mention new fixed transceiver
    binding
  can: m_can: Add call to of_can_transceiver_fixed

 .../bindings/net/can/fixed-transceiver.txt         | 42 +++++++++++++++
 .../devicetree/bindings/net/can/m_can.txt          | 10 ++++
 drivers/net/can/dev.c                              | 59 ++++++++++++++++++++++
 drivers/net/can/m_can/m_can.c                      |  2 +
 include/linux/can/dev.h                            |  5 ++
 5 files changed, 118 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/fixed-transceiver.txt

-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/4] can: Add new binding to limit bit rate used

[Franklin S Cooper Jr]

Add a new generic binding that CAN drivers can use to specify the max
arbitration and data bit rate supported by a transceiver. This is
useful since in some instances the maximum speeds may be limited by
the transceiver used. However, transceivers may not provide a means
to determine this limitation at runtime. Therefore, create a new binding
that mimics "fixed-link" that allows a user to hardcode the max speeds
that can be used.

Also add support for this new binding in the MCAN driver.

Note this is an optional subnode so even if a driver adds support for
parsing fixed-transceiver the user does not have to define it in their
device tree.

Version 2 changes:
Rename function
Define proper variable default
Drop unit address
Move check to changelink function
Reword commit message
Reword documentation

Franklin S Cooper Jr (4):
  can: dev: Add support for limiting configured bitrate
  can: fixed-transceiver: Add documentation for CAN fixed transceiver
    bindings
  can: m_can: Update documentation to mention new fixed transceiver
    binding
  can: m_can: Add call to of_can_transceiver_fixed

 .../bindings/net/can/fixed-transceiver.txt         | 42 +++++++++++++++
 .../devicetree/bindings/net/can/m_can.txt          | 10 ++++
 drivers/net/can/dev.c                              | 59 ++++++++++++++++++++++
 drivers/net/can/m_can/m_can.c                      |  2 +
 include/linux/can/dev.h                            |  5 ++
 5 files changed, 118 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/fixed-transceiver.txt

-- 
2.10.0

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

[Franklin S Cooper Jr]

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 fixed-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@ti.com>
---
Version 2 changes:
Rename new function to of_can_transceiver_fixed
Use version of of_property_read that supports signed/negative values
Return error when user tries to use CAN-FD if the transceiver doesn't
support it (max-data-speed = -1).

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

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 365a8cc..c046631 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -27,6 +27,7 @@
 #include <linux/can/skb.h>
 #include <linux/can/netlink.h>
 #include <linux/can/led.h>
+#include <linux/of.h>
 #include <net/rtnetlink.h>
 
 #define MOD_DESC "CAN device driver interface"
@@ -814,6 +815,41 @@ int open_candev(struct net_device *dev)
 }
 EXPORT_SYMBOL_GPL(open_candev);
 
+#ifdef CONFIG_OF
+void of_can_transceiver_fixed(struct net_device *dev)
+{
+	struct device_node *dn;
+	struct can_priv *priv = netdev_priv(dev);
+	int max_frequency;
+	struct device_node *np;
+
+	np = dev->dev.parent->of_node;
+
+	dn = of_get_child_by_name(np, "fixed-transceiver");
+	if (!dn)
+		return;
+
+	/* Value of 0 implies ignore max speed constraint */
+	max_frequency = 0;
+	of_property_read_s32(dn, "max-arbitration-speed", &max_frequency);
+
+	if (max_frequency >= 0)
+		priv->max_trans_arbitration_speed = max_frequency;
+	else
+		priv->max_trans_arbitration_speed = 0;
+
+	max_frequency = 0;
+
+	of_property_read_s32(dn, "max-data-speed", &max_frequency);
+
+	if (max_frequency >= -1)
+		priv->max_trans_data_speed = max_frequency;
+	else
+		priv->max_trans_data_speed = 0;
+}
+EXPORT_SYMBOL(of_can_transceiver_fixed);
+#endif
+
 /*
  * Common close function for cleanup before the device gets closed.
  *
@@ -913,6 +949,14 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 					priv->bitrate_const_cnt);
 		if (err)
 			return err;
+
+		if (priv->max_trans_arbitration_speed > 0 &&
+		    bt.bitrate > priv->max_trans_arbitration_speed) {
+			netdev_err(dev, "arbitration bitrate surpasses transceiver capabilities of %d bps\n",
+				   priv->max_trans_arbitration_speed);
+			return -EINVAL;
+		}
+
 		memcpy(&priv->bittiming, &bt, sizeof(bt));
 
 		if (priv->do_set_bittiming) {
@@ -989,6 +1033,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 		if (!priv->data_bittiming_const && !priv->do_set_data_bittiming)
 			return -EOPNOTSUPP;
 
+		if ((priv->ctrlmode & CAN_CTRLMODE_FD) &&
+		    priv->max_trans_data_speed == -1) {
+			netdev_err(dev, "canfd mode is not supported by transceiver\n");
+			return -EINVAL;
+		}
+
 		memcpy(&dbt, nla_data(data[IFLA_CAN_DATA_BITTIMING]),
 		       sizeof(dbt));
 		err = can_get_bittiming(dev, &dbt,
@@ -997,6 +1047,15 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 					priv->data_bitrate_const_cnt);
 		if (err)
 			return err;
+
+		if (priv->max_trans_data_speed  > 0 &&
+		    (priv->ctrlmode & CAN_CTRLMODE_FD) &&
+		    (dbt.bitrate > priv->max_trans_data_speed)) {
+			netdev_err(dev, "canfd data bitrate surpasses transceiver capabilities of %d bps\n",
+				   priv->max_trans_data_speed);
+			return -EINVAL;
+		}
+
 		memcpy(&priv->data_bittiming, &dbt, sizeof(dbt));
 
 		if (priv->do_set_data_bittiming) {
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 141b05a..926fc7e 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -47,6 +47,9 @@ struct can_priv {
 	unsigned int data_bitrate_const_cnt;
 	struct can_clock clock;
 
+	int max_trans_arbitration_speed;
+	int max_trans_data_speed;
+
 	enum can_state state;
 
 	/* CAN controller features - see include/uapi/linux/can/netlink.h */
@@ -165,6 +168,8 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
 unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx);
 void can_free_echo_skb(struct net_device *dev, unsigned int idx);
 
+void of_can_transceiver_fixed(struct net_device *dev);
+
 struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf);
 struct sk_buff *alloc_canfd_skb(struct net_device *dev,
 				struct canfd_frame **cfd);
-- 
2.10.0

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

[Franklin S Cooper Jr]

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 fixed-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@ti.com>
---
Version 2 changes:
Rename new function to of_can_transceiver_fixed
Use version of of_property_read that supports signed/negative values
Return error when user tries to use CAN-FD if the transceiver doesn't
support it (max-data-speed = -1).

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

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 365a8cc..c046631 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -27,6 +27,7 @@
 #include <linux/can/skb.h>
 #include <linux/can/netlink.h>
 #include <linux/can/led.h>
+#include <linux/of.h>
 #include <net/rtnetlink.h>
 
 #define MOD_DESC "CAN device driver interface"
@@ -814,6 +815,41 @@ int open_candev(struct net_device *dev)
 }
 EXPORT_SYMBOL_GPL(open_candev);
 
+#ifdef CONFIG_OF
+void of_can_transceiver_fixed(struct net_device *dev)
+{
+	struct device_node *dn;
+	struct can_priv *priv = netdev_priv(dev);
+	int max_frequency;
+	struct device_node *np;
+
+	np = dev->dev.parent->of_node;
+
+	dn = of_get_child_by_name(np, "fixed-transceiver");
+	if (!dn)
+		return;
+
+	/* Value of 0 implies ignore max speed constraint */
+	max_frequency = 0;
+	of_property_read_s32(dn, "max-arbitration-speed", &max_frequency);
+
+	if (max_frequency >= 0)
+		priv->max_trans_arbitration_speed = max_frequency;
+	else
+		priv->max_trans_arbitration_speed = 0;
+
+	max_frequency = 0;
+
+	of_property_read_s32(dn, "max-data-speed", &max_frequency);
+
+	if (max_frequency >= -1)
+		priv->max_trans_data_speed = max_frequency;
+	else
+		priv->max_trans_data_speed = 0;
+}
+EXPORT_SYMBOL(of_can_transceiver_fixed);
+#endif
+
 /*
  * Common close function for cleanup before the device gets closed.
  *
@@ -913,6 +949,14 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 					priv->bitrate_const_cnt);
 		if (err)
 			return err;
+
+		if (priv->max_trans_arbitration_speed > 0 &&
+		    bt.bitrate > priv->max_trans_arbitration_speed) {
+			netdev_err(dev, "arbitration bitrate surpasses transceiver capabilities of %d bps\n",
+				   priv->max_trans_arbitration_speed);
+			return -EINVAL;
+		}
+
 		memcpy(&priv->bittiming, &bt, sizeof(bt));
 
 		if (priv->do_set_bittiming) {
@@ -989,6 +1033,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 		if (!priv->data_bittiming_const && !priv->do_set_data_bittiming)
 			return -EOPNOTSUPP;
 
+		if ((priv->ctrlmode & CAN_CTRLMODE_FD) &&
+		    priv->max_trans_data_speed == -1) {
+			netdev_err(dev, "canfd mode is not supported by transceiver\n");
+			return -EINVAL;
+		}
+
 		memcpy(&dbt, nla_data(data[IFLA_CAN_DATA_BITTIMING]),
 		       sizeof(dbt));
 		err = can_get_bittiming(dev, &dbt,
@@ -997,6 +1047,15 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 					priv->data_bitrate_const_cnt);
 		if (err)
 			return err;
+
+		if (priv->max_trans_data_speed  > 0 &&
+		    (priv->ctrlmode & CAN_CTRLMODE_FD) &&
+		    (dbt.bitrate > priv->max_trans_data_speed)) {
+			netdev_err(dev, "canfd data bitrate surpasses transceiver capabilities of %d bps\n",
+				   priv->max_trans_data_speed);
+			return -EINVAL;
+		}
+
 		memcpy(&priv->data_bittiming, &dbt, sizeof(dbt));
 
 		if (priv->do_set_data_bittiming) {
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 141b05a..926fc7e 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -47,6 +47,9 @@ struct can_priv {
 	unsigned int data_bitrate_const_cnt;
 	struct can_clock clock;
 
+	int max_trans_arbitration_speed;
+	int max_trans_data_speed;
+
 	enum can_state state;
 
 	/* CAN controller features - see include/uapi/linux/can/netlink.h */
@@ -165,6 +168,8 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
 unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx);
 void can_free_echo_skb(struct net_device *dev, unsigned int idx);
 
+void of_can_transceiver_fixed(struct net_device *dev);
+
 struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf);
 struct sk_buff *alloc_canfd_skb(struct net_device *dev,
 				struct canfd_frame **cfd);
-- 
2.10.0

[PATCH v2 3/4] can: m_can: Update documentation to mention new fixed transceiver binding

[Franklin S Cooper Jr]

Add information regarding fixed transceiver binding. This is especially
important for MCAN since the IP allows CAN FD mode to run significantly
faster than what most transceivers are capable of.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
Version 2 changes:
Drop unit address

 Documentation/devicetree/bindings/net/can/m_can.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt
index 9e33177..e4abd2c 100644
--- a/Documentation/devicetree/bindings/net/can/m_can.txt
+++ b/Documentation/devicetree/bindings/net/can/m_can.txt
@@ -43,6 +43,11 @@ Required properties:
 			  Please refer to 2.4.1 Message RAM Configuration in
 			  Bosch M_CAN user manual for details.
 
+Optional properties:
+- fixed-transceiver	: Fixed-transceiver subnode describing maximum speed
+			  that can be used for CAN and/or CAN-FD modes.  See
+			  Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
+			  for details.
 Example:
 SoC dtsi:
 m_can1: can@020e8000 {
@@ -64,4 +69,9 @@ Board dts:
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_m_can1>;
 	status = "enabled";
+
+	fixed-transceiver {
+		max-arbitration-speed = <1000000>;
+		max-data-speed = <5000000>;
+	};
 };
-- 
2.10.0

[PATCH v2 3/4] can: m_can: Update documentation to mention new fixed transceiver binding

[Franklin S Cooper Jr]

Add information regarding fixed transceiver binding. This is especially
important for MCAN since the IP allows CAN FD mode to run significantly
faster than what most transceivers are capable of.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
Version 2 changes:
Drop unit address

 Documentation/devicetree/bindings/net/can/m_can.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt
index 9e33177..e4abd2c 100644
--- a/Documentation/devicetree/bindings/net/can/m_can.txt
+++ b/Documentation/devicetree/bindings/net/can/m_can.txt
@@ -43,6 +43,11 @@ Required properties:
 			  Please refer to 2.4.1 Message RAM Configuration in
 			  Bosch M_CAN user manual for details.
 
+Optional properties:
+- fixed-transceiver	: Fixed-transceiver subnode describing maximum speed
+			  that can be used for CAN and/or CAN-FD modes.  See
+			  Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
+			  for details.
 Example:
 SoC dtsi:
 m_can1: can@020e8000 {
@@ -64,4 +69,9 @@ Board dts:
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_m_can1>;
 	status = "enabled";
+
+	fixed-transceiver {
+		max-arbitration-speed = <1000000>;
+		max-data-speed = <5000000>;
+	};
 };
-- 
2.10.0

Re: [PATCH v2 3/4] can: m_can: Update documentation to mention new fixed transceiver binding

[Rob Herring]

On Mon, Jul 24, 2017 at 06:05:20PM -0500, Franklin S Cooper Jr wrote:
> Add information regarding fixed transceiver binding. This is especially
> important for MCAN since the IP allows CAN FD mode to run significantly
> faster than what most transceivers are capable of.
> 
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> ---
> Version 2 changes:
> Drop unit address
> 
>  Documentation/devicetree/bindings/net/can/m_can.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt
> index 9e33177..e4abd2c 100644
> --- a/Documentation/devicetree/bindings/net/can/m_can.txt
> +++ b/Documentation/devicetree/bindings/net/can/m_can.txt
> @@ -43,6 +43,11 @@ Required properties:
>  			  Please refer to 2.4.1 Message RAM Configuration in
>  			  Bosch M_CAN user manual for details.
>  
> +Optional properties:
> +- fixed-transceiver	: Fixed-transceiver subnode describing maximum speed

This is a node, not a property. Sub nodes should have their own section.

> +			  that can be used for CAN and/or CAN-FD modes.  See
> +			  Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
> +			  for details.
>  Example:
>  SoC dtsi:
>  m_can1: can@020e8000 {
> @@ -64,4 +69,9 @@ Board dts:
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_m_can1>;
>  	status = "enabled";
> +
> +	fixed-transceiver {
> +		max-arbitration-speed = <1000000>;
> +		max-data-speed = <5000000>;
> +	};
>  };
> -- 
> 2.10.0
> 

Re: [PATCH v2 3/4] can: m_can: Update documentation to mention new fixed transceiver binding

[Franklin S Cooper Jr]

Hi Rob,
On 08/03/2017 12:07 PM, Rob Herring wrote:
> On Mon, Jul 24, 2017 at 06:05:20PM -0500, Franklin S Cooper Jr wrote:
>> Add information regarding fixed transceiver binding. This is especially
>> important for MCAN since the IP allows CAN FD mode to run significantly
>> faster than what most transceivers are capable of.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> ---
>> Version 2 changes:
>> Drop unit address
>>
>>  Documentation/devicetree/bindings/net/can/m_can.txt | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt
>> index 9e33177..e4abd2c 100644
>> --- a/Documentation/devicetree/bindings/net/can/m_can.txt
>> +++ b/Documentation/devicetree/bindings/net/can/m_can.txt
>> @@ -43,6 +43,11 @@ Required properties:
>>  			  Please refer to 2.4.1 Message RAM Configuration in
>>  			  Bosch M_CAN user manual for details.
>>  
>> +Optional properties:
>> +- fixed-transceiver	: Fixed-transceiver subnode describing maximum speed
> 
> This is a node, not a property. Sub nodes should have their own section.

Fixed in my v4 that I just sent.
> 
>> +			  that can be used for CAN and/or CAN-FD modes.  See
>> +			  Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>> +			  for details.
>>  Example:
>>  SoC dtsi:
>>  m_can1: can@020e8000 {
>> @@ -64,4 +69,9 @@ Board dts:
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&pinctrl_m_can1>;
>>  	status = "enabled";
>> +
>> +	fixed-transceiver {
>> +		max-arbitration-speed = <1000000>;
>> +		max-data-speed = <5000000>;
>> +	};
>>  };
>> -- 
>> 2.10.0
>>

Re: [PATCH v2 3/4] can: m_can: Update documentation to mention new fixed transceiver binding

[Franklin S Cooper Jr]

Hi Rob,
On 08/03/2017 12:07 PM, Rob Herring wrote:
> On Mon, Jul 24, 2017 at 06:05:20PM -0500, Franklin S Cooper Jr wrote:
>> Add information regarding fixed transceiver binding. This is especially
>> important for MCAN since the IP allows CAN FD mode to run significantly
>> faster than what most transceivers are capable of.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> ---
>> Version 2 changes:
>> Drop unit address
>>
>>  Documentation/devicetree/bindings/net/can/m_can.txt | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt
>> index 9e33177..e4abd2c 100644
>> --- a/Documentation/devicetree/bindings/net/can/m_can.txt
>> +++ b/Documentation/devicetree/bindings/net/can/m_can.txt
>> @@ -43,6 +43,11 @@ Required properties:
>>  			  Please refer to 2.4.1 Message RAM Configuration in
>>  			  Bosch M_CAN user manual for details.
>>  
>> +Optional properties:
>> +- fixed-transceiver	: Fixed-transceiver subnode describing maximum speed
> 
> This is a node, not a property. Sub nodes should have their own section.

Fixed in my v4 that I just sent.
> 
>> +			  that can be used for CAN and/or CAN-FD modes.  See
>> +			  Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>> +			  for details.
>>  Example:
>>  SoC dtsi:
>>  m_can1: can@020e8000 {
>> @@ -64,4 +69,9 @@ Board dts:
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&pinctrl_m_can1>;
>>  	status = "enabled";
>> +
>> +	fixed-transceiver {
>> +		max-arbitration-speed = <1000000>;
>> +		max-data-speed = <5000000>;
>> +	};
>>  };
>> -- 
>> 2.10.0
>>

[PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

Add documentation to describe usage of the new fixed transceiver binding.
This new binding is applicable for any CAN device therefore it exists as
its own document.

Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
---
Version 2:
Tweak commit message working
Tweak wording for properties
Drop unit addressing
Give example if CAN-FD isn't supported

 .../bindings/net/can/fixed-transceiver.txt         | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/fixed-transceiver.txt

diff --git a/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
new file mode 100644
index 0000000..dc7e3eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
@@ -0,0 +1,42 @@
+Fixed transceiver Device Tree binding
+------------------------------
+
+CAN transceiver typically limits the max speed in standard CAN and CAN FD
+modes. Typically these limitations are static and the transceivers themselves
+provide no way to detect this limitation at runtime. For this situation,
+the "fixed-transceiver" node can be used.
+
+Properties:
+
+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.
+
+ 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.
+
+Examples:
+
+Based on Texas Instrument's TCAN1042HGV CAN Transceiver
+
+m_can0 {
+	....
+	fixed-transceiver {
+		max-arbitration-speed = <1000000>;
+		max-data-speed = <5000000>;
+	};
+	...
+};
+
+Based on a CAN Transceiver that doesn't support CAN-FD. Only needed if the
+CAN IP supports CAN-FD but is using a transceiver that doesn't.
+
+m_can0 {
+	....
+	fixed-transceiver {
+		max-data-speed = <(-1)>;
+	};
+	...
+};
-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

Add documentation to describe usage of the new fixed transceiver binding.
This new binding is applicable for any CAN device therefore it exists as
its own document.

Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
---
Version 2:
Tweak commit message working
Tweak wording for properties
Drop unit addressing
Give example if CAN-FD isn't supported

 .../bindings/net/can/fixed-transceiver.txt         | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/fixed-transceiver.txt

diff --git a/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
new file mode 100644
index 0000000..dc7e3eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
@@ -0,0 +1,42 @@
+Fixed transceiver Device Tree binding
+------------------------------
+
+CAN transceiver typically limits the max speed in standard CAN and CAN FD
+modes. Typically these limitations are static and the transceivers themselves
+provide no way to detect this limitation at runtime. For this situation,
+the "fixed-transceiver" node can be used.
+
+Properties:
+
+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.
+
+ 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.
+
+Examples:
+
+Based on Texas Instrument's TCAN1042HGV CAN Transceiver
+
+m_can0 {
+	....
+	fixed-transceiver {
+		max-arbitration-speed = <1000000>;
+		max-data-speed = <5000000>;
+	};
+	...
+};
+
+Based on a CAN Transceiver that doesn't support CAN-FD. Only needed if the
+CAN IP supports CAN-FD but is using a transceiver that doesn't.
+
+m_can0 {
+	....
+	fixed-transceiver {
+		max-data-speed = <(-1)>;
+	};
+	...
+};
-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

Add documentation to describe usage of the new fixed transceiver binding.
This new binding is applicable for any CAN device therefore it exists as
its own document.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
Version 2:
Tweak commit message working
Tweak wording for properties
Drop unit addressing
Give example if CAN-FD isn't supported

 .../bindings/net/can/fixed-transceiver.txt         | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/fixed-transceiver.txt

diff --git a/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
new file mode 100644
index 0000000..dc7e3eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
@@ -0,0 +1,42 @@
+Fixed transceiver Device Tree binding
+------------------------------
+
+CAN transceiver typically limits the max speed in standard CAN and CAN FD
+modes. Typically these limitations are static and the transceivers themselves
+provide no way to detect this limitation at runtime. For this situation,
+the "fixed-transceiver" node can be used.
+
+Properties:
+
+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.
+
+ 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.
+
+Examples:
+
+Based on Texas Instrument's TCAN1042HGV CAN Transceiver
+
+m_can0 {
+	....
+	fixed-transceiver {
+		max-arbitration-speed = <1000000>;
+		max-data-speed = <5000000>;
+	};
+	...
+};
+
+Based on a CAN Transceiver that doesn't support CAN-FD. Only needed if the
+CAN IP supports CAN-FD but is using a transceiver that doesn't.
+
+m_can0 {
+	....
+	fixed-transceiver {
+		max-data-speed = <(-1)>;
+	};
+	...
+};
-- 
2.10.0

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Oliver Hartkopp]

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

(..)

> +	fixed-transceiver {
> +		max-data-speed = <(-1)>;

Looks ugly IMHO.

Why didn't you stay on '0' for 'not supported'??

Regards,
Oliver

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

On 07/25/2017 11:32 AM, Oliver Hartkopp wrote:
> 
>> + 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.
>> +
>> +Examples:
> 
> (..)
> 
>> +    fixed-transceiver {
>> +        max-data-speed = <(-1)>;
> 
> Looks ugly IMHO.
> 
> Why didn't you stay on '0' for 'not supported'??

Unless a driver specifically calls of_can_transceiver_fixed
priv->max_trans_data_speed will be by default 0. Therefore, all drivers
that support CAN-FD will claim that the transceiver indicates that it
isn't supported. So one option was to update every single driver to set
this property by default which I started to do but it end up becoming a
massive patch and it was risky in case I missed a driver which would of
resulted in major regressions. Its also problematic for new drivers that
miss this property or the many out of tree CAN drivers. The other option
was to create another variable to track to see if
of_can_transceiver_fixed was called but I didn't think that was the
better solution. So using signed values in DT is a bit ugly due to
syntax but was valid and I made sure I documented it so its clear.
> 
> Regards,
> Oliver
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

On 07/25/2017 11:32 AM, Oliver Hartkopp wrote:
> 
>> + 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.
>> +
>> +Examples:
> 
> (..)
> 
>> +    fixed-transceiver {
>> +        max-data-speed = <(-1)>;
> 
> Looks ugly IMHO.
> 
> Why didn't you stay on '0' for 'not supported'??

Unless a driver specifically calls of_can_transceiver_fixed
priv->max_trans_data_speed will be by default 0. Therefore, all drivers
that support CAN-FD will claim that the transceiver indicates that it
isn't supported. So one option was to update every single driver to set
this property by default which I started to do but it end up becoming a
massive patch and it was risky in case I missed a driver which would of
resulted in major regressions. Its also problematic for new drivers that
miss this property or the many out of tree CAN drivers. The other option
was to create another variable to track to see if
of_can_transceiver_fixed was called but I didn't think that was the
better solution. So using signed values in DT is a bit ugly due to
syntax but was valid and I made sure I documented it so its clear.
> 
> Regards,
> Oliver
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

On 07/25/2017 11:32 AM, Oliver Hartkopp wrote:
> 
>> + 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.
>> +
>> +Examples:
> 
> (..)
> 
>> +    fixed-transceiver {
>> +        max-data-speed = <(-1)>;
> 
> Looks ugly IMHO.
> 
> Why didn't you stay on '0' for 'not supported'??

Unless a driver specifically calls of_can_transceiver_fixed
priv->max_trans_data_speed will be by default 0. Therefore, all drivers
that support CAN-FD will claim that the transceiver indicates that it
isn't supported. So one option was to update every single driver to set
this property by default which I started to do but it end up becoming a
massive patch and it was risky in case I missed a driver which would of
resulted in major regressions. Its also problematic for new drivers that
miss this property or the many out of tree CAN drivers. The other option
was to create another variable to track to see if
of_can_transceiver_fixed was called but I didn't think that was the
better solution. So using signed values in DT is a bit ugly due to
syntax but was valid and I made sure I documented it so its clear.
> 
> Regards,
> Oliver
> 

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Andrew Lunn]

On Mon, Jul 24, 2017 at 06:05:19PM -0500, Franklin S Cooper Jr wrote:
> Add documentation to describe usage of the new fixed transceiver binding.
> This new binding is applicable for any CAN device therefore it exists as
> its own document.
> 
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> ---
> Version 2:
> Tweak commit message working
> Tweak wording for properties
> Drop unit addressing
> Give example if CAN-FD isn't supported
> 
>  .../bindings/net/can/fixed-transceiver.txt         | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
> new file mode 100644
> index 0000000..dc7e3eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
> @@ -0,0 +1,42 @@
> +Fixed transceiver Device Tree binding
> +------------------------------
> +
> +CAN transceiver typically limits the max speed in standard CAN and CAN FD
> +modes. Typically these limitations are static and the transceivers themselves
> +provide no way to detect this limitation at runtime. For this situation,
> +the "fixed-transceiver" node can be used.
> +
> +Properties:
> +
> +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.

> +
> + 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. And
maybe put 'fd' into the property name.

      Andrew

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Oliver Hartkopp]

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

Thanks Andrew! I had the same feeling about '-1' :-)

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

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.

Regards,
Oliver

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

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
> 

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Oliver Hartkopp]

On 07/26/2017 08:29 PM, Franklin S Cooper Jr wrote:
>

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

1

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

??

It's a physical layer device which only has a bandwidth limitation.
The transceiver does not know about CAN FD.

> With one
> property you can not determine this and end up having to make some
> assumptions that can quickly end up biting people.

Despite the fact that the transceiver does not know anything about ISO 
layer 2 (CAN/CAN FD) the properties should look like

	max-bitrate
	canfd-capable

then.

But when the tranceiver is 'canfd-capable' agnostic, why provide a 
property for it?

Maybe I'm wrong but I still can't follow your argumentation ideas.

Regards,
Oliver
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Oliver Hartkopp]

On 07/26/2017 08:29 PM, Franklin S Cooper Jr wrote:
>

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

1

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

??

It's a physical layer device which only has a bandwidth limitation.
The transceiver does not know about CAN FD.

> With one
> property you can not determine this and end up having to make some
> assumptions that can quickly end up biting people.

Despite the fact that the transceiver does not know anything about ISO 
layer 2 (CAN/CAN FD) the properties should look like

	max-bitrate
	canfd-capable

then.

But when the tranceiver is 'canfd-capable' agnostic, why provide a 
property for it?

Maybe I'm wrong but I still can't follow your argumentation ideas.

Regards,
Oliver

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

On 07/27/2017 01:47 PM, Oliver Hartkopp wrote:
> On 07/26/2017 08:29 PM, Franklin S Cooper Jr wrote:
>>
> 
>> 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
>>
>>
> 
> 1
> 
>>> 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.
> 
> ??
> 
> It's a physical layer device which only has a bandwidth limitation.
> The transceiver does not know about CAN FD.
> 
>> With one
>> property you can not determine this and end up having to make some
>> assumptions that can quickly end up biting people.
> 
> Despite the fact that the transceiver does not know anything about ISO
> layer 2 (CAN/CAN FD) the properties should look like
> 
>     max-bitrate
>     canfd-capable
> 
> then.
> 
> But when the tranceiver is 'canfd-capable' agnostic, why provide a
> property for it?
> 
> Maybe I'm wrong but I still can't follow your argumentation ideas.

Your right. I spoke to our CAN transceiver team and I finally get your
points.

So yes using "max-bitrate" alone is all we need. Sorry for the confusion
and I'll create a new rev using this approach.
> 
> Regards,
> Oliver

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

On 07/27/2017 01:47 PM, Oliver Hartkopp wrote:
> On 07/26/2017 08:29 PM, Franklin S Cooper Jr wrote:
>>
> 
>> 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
>>
>>
> 
> 1
> 
>>> 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.
> 
> ??
> 
> It's a physical layer device which only has a bandwidth limitation.
> The transceiver does not know about CAN FD.
> 
>> With one
>> property you can not determine this and end up having to make some
>> assumptions that can quickly end up biting people.
> 
> Despite the fact that the transceiver does not know anything about ISO
> layer 2 (CAN/CAN FD) the properties should look like
> 
>     max-bitrate
>     canfd-capable
> 
> then.
> 
> But when the tranceiver is 'canfd-capable' agnostic, why provide a
> property for it?
> 
> Maybe I'm wrong but I still can't follow your argumentation ideas.

Your right. I spoke to our CAN transceiver team and I finally get your
points.

So yes using "max-bitrate" alone is all we need. Sorry for the confusion
and I'll create a new rev using this approach.
> 
> Regards,
> Oliver

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Kurt Van Dijck]

> 
> On 07/27/2017 01:47 PM, Oliver Hartkopp wrote:
> > On 07/26/2017 08:29 PM, Franklin S Cooper Jr wrote:
> >>
> > 
> >> 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
> >>
> >>
> > 
> > 1
> > 
> >>> 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.
> > 
> > ??
> > 
> > It's a physical layer device which only has a bandwidth limitation.
> > The transceiver does not know about CAN FD.
> > 
> >> With one
> >> property you can not determine this and end up having to make some
> >> assumptions that can quickly end up biting people.
> > 
> > Despite the fact that the transceiver does not know anything about ISO
> > layer 2 (CAN/CAN FD) the properties should look like
> > 
> >     max-bitrate
> >     canfd-capable
> > 
> > then.
> > 
> > But when the tranceiver is 'canfd-capable' agnostic, why provide a
> > property for it?
> > 
> > Maybe I'm wrong but I still can't follow your argumentation ideas.
> 

The transceiver does not know about CAN FD, but CAN FD uses
the different restrictions of the arbitration & data phase in the CAN
frame, i.e. during arbitration, the RX must indicate the wire
(dominant/recessive) within 1 bit time, during data in CAN FD, this is
not necessary.

So while _a_ transceiver may be spec'd to 1MBit during arbitration,
CAN FD packets may IMHO exceed that speed during data phase.
That was the whole point of CAN FD: exceed the limits required for
correct arbitration on transceiver & wire.

So I do not agree on the single bandwidth limitation.

The word 'max-arbitration-bitrate' makes the difference very clear.

> Your right. I spoke to our CAN transceiver team and I finally get your
> points.
> 
> So yes using "max-bitrate" alone is all we need. Sorry for the confusion
> and I'll create a new rev using this approach.
> > 
> > Regards,
> > Oliver

Kind regards,
Kurt

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Oliver Hartkopp]

On 07/28/2017 06:57 AM, Kurt Van Dijck wrote:

> So while _a_ transceiver may be spec'd to 1MBit during arbitration,
> CAN FD packets may IMHO exceed that speed during data phase.

When the bitrate is limited to 1Mbit/s you are ONLY allowed to use 
1Mbit/s in the data section too (either with CAN or CAN FD).

> That was the whole point of CAN FD: exceed the limits required for
> correct arbitration on transceiver & wire.

No. CAN FD is about a different frame format with up to 64 bytes AND the 
possibility to increase the bitrate in the data section of the frame.

> So I do not agree on the single bandwidth limitation.

The transceiver provides a single maximum bandwidth. It's an ISO Layer 1 
device.

> The word 'max-arbitration-bitrate' makes the difference very clear.

I think you are mixing up ISO layer 1 and ISO layer 2.

Regards,
Oliver

[PATCH v2 4/4] can: m_can: Add call to of_can_transceiver_fixed

[Franklin S Cooper Jr]

Add call to new generic functions that provides support via a binding
to limit the arbitration rate and/or data rate imposed by the physical
transceiver connected to the MCAN peripheral.

Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
---
 drivers/net/can/m_can/m_can.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index f4947a7..bd75df1 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1649,6 +1649,8 @@ static int m_can_plat_probe(struct platform_device *pdev)
 
 	devm_can_led_init(dev);
 
+	of_can_transceiver_fixed(dev);
+
 	dev_info(&pdev->dev, "%s device registered (irq=%d, version=%d)\n",
 		 KBUILD_MODNAME, dev->irq, priv->version);
 
-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/4] can: m_can: Add call to of_can_transceiver_fixed

[Franklin S Cooper Jr]

Add call to new generic functions that provides support via a binding
to limit the arbitration rate and/or data rate imposed by the physical
transceiver connected to the MCAN peripheral.

Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
---
 drivers/net/can/m_can/m_can.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index f4947a7..bd75df1 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1649,6 +1649,8 @@ static int m_can_plat_probe(struct platform_device *pdev)
 
 	devm_can_led_init(dev);
 
+	of_can_transceiver_fixed(dev);
+
 	dev_info(&pdev->dev, "%s device registered (irq=%d, version=%d)\n",
 		 KBUILD_MODNAME, dev->irq, priv->version);
 
-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/4] can: m_can: Add call to of_can_transceiver_fixed

[Franklin S Cooper Jr]

Add call to new generic functions that provides support via a binding
to limit the arbitration rate and/or data rate imposed by the physical
transceiver connected to the MCAN peripheral.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
 drivers/net/can/m_can/m_can.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index f4947a7..bd75df1 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1649,6 +1649,8 @@ static int m_can_plat_probe(struct platform_device *pdev)
 
 	devm_can_led_init(dev);
 
+	of_can_transceiver_fixed(dev);
+
 	dev_info(&pdev->dev, "%s device registered (irq=%d, version=%d)\n",
 		 KBUILD_MODNAME, dev->irq, priv->version);
 
-- 
2.10.0

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Kurt Van Dijck]

> 
> On 07/28/2017 06:57 AM, Kurt Van Dijck wrote:
> 
> >So while _a_ transceiver may be spec'd to 1MBit during arbitration,
> >CAN FD packets may IMHO exceed that speed during data phase.
> 
> When the bitrate is limited to 1Mbit/s you are ONLY allowed to use 1Mbit/s
> in the data section too (either with CAN or CAN FD).

My point is that the requirements posed to a transceiver
differ between arbitration & data phase for CAN FD.
So while a transceiver does not know about CAN FD, it may allow
higher bitrates for the data phase.

> 
> >That was the whole point of CAN FD: exceed the limits required for
> >correct arbitration on transceiver & wire.
> 
> No. CAN FD is about a different frame format with up to 64 bytes AND the
> possibility to increase the bitrate in the data section of the frame.
> 
> >So I do not agree on the single bandwidth limitation.
> 
> The transceiver provides a single maximum bandwidth. It's an ISO Layer 1
> device.
> 
> >The word 'max-arbitration-bitrate' makes the difference very clear.
> 
> I think you are mixing up ISO layer 1 and ISO layer 2.

In order to provide higher data throughput without putting extra limits
on transceiver & wire, the requirement for the round-trip delay to be
within 1 bittime has been eliminated, but only for the data phase when
arbitration is over.
So layer 2 (CAN FD) has been adapted to circumvent the layer 1
(transceiver + wire) limitations.

In fact, the round-trip delay requirement never actually did matter for
plain CAN during data bits either. CAN FD just makes use of that,
but is therefore incompatible on the wire.

I forgot the precise wording, but this is the principle that Bosch
explained on the CAN conference in Nurnberg several years ago, or at
least this is how I remembered it :-)

I haven't followed the developments of transceivers, but with the above
principle in mind, it's obvious that any transceiver allows higher
bitrates during the data segment because the TX-to-RX line delay must
not scale with the bitrate.
In reality, maybe not all transceivers will mention this in their
datasheet.

So whether you call it 'max-arbitration-bitrate' & 'max-data-bitrate'
or 'max-bitrate' & 'max-data-bitrate' does not really matter (I prefer
1st) but you will one day need 2 bitrates.

Kind regards,
Kurt

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Oliver Hartkopp]

Hi Kurt,

On 07/28/2017 03:02 PM, Kurt Van Dijck wrote:

>>> The word 'max-arbitration-bitrate' makes the difference very clear.
>>
>> I think you are mixing up ISO layer 1 and ISO layer 2.
> 
> In order to provide higher data throughput without putting extra limits
> on transceiver & wire, the requirement for the round-trip delay to be
> within 1 bittime has been eliminated, but only for the data phase when
> arbitration is over.
> So layer 2 (CAN FD) has been adapted to circumvent the layer 1
> (transceiver + wire) limitations.
> 
> In fact, the round-trip delay requirement never actually did matter for
> plain CAN during data bits either. CAN FD just makes use of that,
> but is therefore incompatible on the wire.
> 
> I forgot the precise wording, but this is the principle that Bosch
> explained on the CAN conference in Nurnberg several years ago, or at
> least this is how I remembered it :-)

I just checked an example for a CAN FD qualified transceiver

http://www.nxp.com/products/automotive-products/energy-power-management/can-transceivers/high-speed-can-transceiver-with-standby-mode:TJA1044

where it states:

The TJA1044T is specified for data rates up to 1 Mbit/s. Pending the 
release of ISO11898-2:2016 including CAN FD and SAE-J2284-4/5, 
additional timing parameters defining loop delay symmetry are specified 
for the TJA1044GT and TJA1044GTK. This implementation enables reliable 
communication in the CAN FD fast phase at data rates up to 5 Mbit/s.

and

TJA1044GT/TJA1044GTK

- Timing guaranteed for data rates up to 5 Mbit/s
- Improved TXD to RXD propagation delay of 210 ns

> I haven't followed the developments of transceivers, but with the above
> principle in mind, it's obvious that any transceiver allows higher
> bitrates during the data segment because the TX-to-RX line delay must
> not scale with the bitrate.
> In reality, maybe not all transceivers will mention this in their
> datasheet.
> 
> So whether you call it 'max-arbitration-bitrate' & 'max-data-bitrate'
> or 'max-bitrate' & 'max-data-bitrate' does not really matter (I prefer
> 1st) but you will one day need 2 bitrates.

The question to me is whether it is right option to specify two bitrates 
OR to specify one maximum bitrate and provide a property that a CAN FD 
capable propagation delay is available.

E.g.

	max-bitrate
	max-data-bitrate

or

	max-bitrate
	canfd-capable	// CAN FD capable propagation delay available


I assume the optimized propagation delay is 'always on' as the 
transceiver is not able to detect which kind of bits it is processing.
That's why I think providing two bitrates leads to a wrong view on the 
transceiver.

Regards,
Oliver

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

On 07/28/2017 01:33 PM, Oliver Hartkopp wrote:
> Hi Kurt,
> 
> On 07/28/2017 03:02 PM, Kurt Van Dijck wrote:
> 
>>>> The word 'max-arbitration-bitrate' makes the difference very clear.
>>>
>>> I think you are mixing up ISO layer 1 and ISO layer 2.
>>
>> In order to provide higher data throughput without putting extra limits
>> on transceiver & wire, the requirement for the round-trip delay to be
>> within 1 bittime has been eliminated, but only for the data phase when
>> arbitration is over.
>> So layer 2 (CAN FD) has been adapted to circumvent the layer 1
>> (transceiver + wire) limitations.
>>
>> In fact, the round-trip delay requirement never actually did matter for
>> plain CAN during data bits either. CAN FD just makes use of that,
>> but is therefore incompatible on the wire.
>>
>> I forgot the precise wording, but this is the principle that Bosch
>> explained on the CAN conference in Nurnberg several years ago, or at
>> least this is how I remembered it :-)
> 
> I just checked an example for a CAN FD qualified transceiver
> 
> http://www.nxp.com/products/automotive-products/energy-power-management/can-transceivers/high-speed-can-transceiver-with-standby-mode:TJA1044
> 
> 
> where it states:
> 
> The TJA1044T is specified for data rates up to 1 Mbit/s. Pending the
> release of ISO11898-2:2016 including CAN FD and SAE-J2284-4/5,
> additional timing parameters defining loop delay symmetry are specified
> for the TJA1044GT and TJA1044GTK. This implementation enables reliable
> communication in the CAN FD fast phase at data rates up to 5 Mbit/s.
> 
> and
> 
> TJA1044GT/TJA1044GTK
> 
> - Timing guaranteed for data rates up to 5 Mbit/s
> - Improved TXD to RXD propagation delay of 210 ns
> 
>> I haven't followed the developments of transceivers, but with the above
>> principle in mind, it's obvious that any transceiver allows higher
>> bitrates during the data segment because the TX-to-RX line delay must
>> not scale with the bitrate.
>> In reality, maybe not all transceivers will mention this in their
>> datasheet.
>>
>> So whether you call it 'max-arbitration-bitrate' & 'max-data-bitrate'
>> or 'max-bitrate' & 'max-data-bitrate' does not really matter (I prefer
>> 1st) but you will one day need 2 bitrates.
> 
> The question to me is whether it is right option to specify two bitrates
> OR to specify one maximum bitrate and provide a property that a CAN FD
> capable propagation delay is available.
> 
> E.g.
> 
>     max-bitrate
>     max-data-bitrate
> 
> or
> 
>     max-bitrate
>     canfd-capable    // CAN FD capable propagation delay available
> 
> 
> I assume the optimized propagation delay is 'always on' as the
> transceiver is not able to detect which kind of bits it is processing.
> That's why I think providing two bitrates leads to a wrong view on the
> transceiver.

I agree with this.

The transceiver is an analog device that needs to support faster
switching frequency (FETs) including minimizing delay to support CAN-FD
ie higher bitrate. From the transceiver perspective the bits for
"arbitration" and "data" look exactly the same. Since it can't
differentiate between the two (at the physical layer) then the actual
limit isn't specific to which part/type of the CAN message is being
sent. Rather its just a single overall max bitrate limit.

> 
> Regards,
> Oliver
> 

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Franklin S Cooper Jr]

On 07/28/2017 01:33 PM, Oliver Hartkopp wrote:
> Hi Kurt,
> 
> On 07/28/2017 03:02 PM, Kurt Van Dijck wrote:
> 
>>>> The word 'max-arbitration-bitrate' makes the difference very clear.
>>>
>>> I think you are mixing up ISO layer 1 and ISO layer 2.
>>
>> In order to provide higher data throughput without putting extra limits
>> on transceiver & wire, the requirement for the round-trip delay to be
>> within 1 bittime has been eliminated, but only for the data phase when
>> arbitration is over.
>> So layer 2 (CAN FD) has been adapted to circumvent the layer 1
>> (transceiver + wire) limitations.
>>
>> In fact, the round-trip delay requirement never actually did matter for
>> plain CAN during data bits either. CAN FD just makes use of that,
>> but is therefore incompatible on the wire.
>>
>> I forgot the precise wording, but this is the principle that Bosch
>> explained on the CAN conference in Nurnberg several years ago, or at
>> least this is how I remembered it :-)
> 
> I just checked an example for a CAN FD qualified transceiver
> 
> http://www.nxp.com/products/automotive-products/energy-power-management/can-transceivers/high-speed-can-transceiver-with-standby-mode:TJA1044
> 
> 
> where it states:
> 
> The TJA1044T is specified for data rates up to 1 Mbit/s. Pending the
> release of ISO11898-2:2016 including CAN FD and SAE-J2284-4/5,
> additional timing parameters defining loop delay symmetry are specified
> for the TJA1044GT and TJA1044GTK. This implementation enables reliable
> communication in the CAN FD fast phase at data rates up to 5 Mbit/s.
> 
> and
> 
> TJA1044GT/TJA1044GTK
> 
> - Timing guaranteed for data rates up to 5 Mbit/s
> - Improved TXD to RXD propagation delay of 210 ns
> 
>> I haven't followed the developments of transceivers, but with the above
>> principle in mind, it's obvious that any transceiver allows higher
>> bitrates during the data segment because the TX-to-RX line delay must
>> not scale with the bitrate.
>> In reality, maybe not all transceivers will mention this in their
>> datasheet.
>>
>> So whether you call it 'max-arbitration-bitrate' & 'max-data-bitrate'
>> or 'max-bitrate' & 'max-data-bitrate' does not really matter (I prefer
>> 1st) but you will one day need 2 bitrates.
> 
> The question to me is whether it is right option to specify two bitrates
> OR to specify one maximum bitrate and provide a property that a CAN FD
> capable propagation delay is available.
> 
> E.g.
> 
>     max-bitrate
>     max-data-bitrate
> 
> or
> 
>     max-bitrate
>     canfd-capable    // CAN FD capable propagation delay available
> 
> 
> I assume the optimized propagation delay is 'always on' as the
> transceiver is not able to detect which kind of bits it is processing.
> That's why I think providing two bitrates leads to a wrong view on the
> transceiver.

I agree with this.

The transceiver is an analog device that needs to support faster
switching frequency (FETs) including minimizing delay to support CAN-FD
ie higher bitrate. From the transceiver perspective the bits for
"arbitration" and "data" look exactly the same. Since it can't
differentiate between the two (at the physical layer) then the actual
limit isn't specific to which part/type of the CAN message is being
sent. Rather its just a single overall max bitrate limit.

> 
> Regards,
> Oliver
> 

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Kurt Van Dijck]

> 
> On 07/28/2017 01:33 PM, Oliver Hartkopp wrote:
> > Hi Kurt,
> > 
> > On 07/28/2017 03:02 PM, Kurt Van Dijck wrote:
> > 
> >>>> The word 'max-arbitration-bitrate' makes the difference very clear.
> >>>
> >>> I think you are mixing up ISO layer 1 and ISO layer 2.
> >>
> >> In order to provide higher data throughput without putting extra limits
> >> on transceiver & wire, the requirement for the round-trip delay to be
> >> within 1 bittime has been eliminated, but only for the data phase when
> >> arbitration is over.
> >> So layer 2 (CAN FD) has been adapted to circumvent the layer 1
> >> (transceiver + wire) limitations.
> >>
> >> In fact, the round-trip delay requirement never actually did matter for
> >> plain CAN during data bits either. CAN FD just makes use of that,
> >> but is therefore incompatible on the wire.
> >>
> >> I forgot the precise wording, but this is the principle that Bosch
> >> explained on the CAN conference in Nurnberg several years ago, or at
> >> least this is how I remembered it :-)
> > 
> > I just checked an example for a CAN FD qualified transceiver
> > 
> > http://www.nxp.com/products/automotive-products/energy-power-management/can-transceivers/high-speed-can-transceiver-with-standby-mode:TJA1044
> > 
> > 
> > where it states:
> > 
> > The TJA1044T is specified for data rates up to 1 Mbit/s. Pending the
> > release of ISO11898-2:2016 including CAN FD and SAE-J2284-4/5,
> > additional timing parameters defining loop delay symmetry are specified
> > for the TJA1044GT and TJA1044GTK. This implementation enables reliable
> > communication in the CAN FD fast phase at data rates up to 5 Mbit/s.
> > 
> > and
> > 
> > TJA1044GT/TJA1044GTK
> > 
> > - Timing guaranteed for data rates up to 5 Mbit/s
> > - Improved TXD to RXD propagation delay of 210 ns

Note the words "loop delay symmetry" and "CAN FD fast phase".
I didn't dig into the ISO's, but I read this as:
the TJA1044GT (not the TJA1044T) supports CAN FD data bitrate up to
5MBit/s. The overall (and thus arbitration) bitrate is max 1MBit/s.

What did I miss?

> > 
> >> I haven't followed the developments of transceivers, but with the above
> >> principle in mind, it's obvious that any transceiver allows higher
> >> bitrates during the data segment because the TX-to-RX line delay must
> >> not scale with the bitrate.
> >> In reality, maybe not all transceivers will mention this in their
> >> datasheet.
> >>
> >> So whether you call it 'max-arbitration-bitrate' & 'max-data-bitrate'
> >> or 'max-bitrate' & 'max-data-bitrate' does not really matter (I prefer
> >> 1st) but you will one day need 2 bitrates.
> > 
> > The question to me is whether it is right option to specify two bitrates
> > OR to specify one maximum bitrate and provide a property that a CAN FD
> > capable propagation delay is available.
> > 
> > E.g.
> > 
> >     max-bitrate
> >     max-data-bitrate
> > 
> > or
> > 
> >     max-bitrate
> >     canfd-capable    // CAN FD capable propagation delay available

When you say: 'there is a CAN FD capable propagation delay available',
this actually means that you the transceiver specified a seperate
max-data-bitrate, but you don't mention what it is.
How can linux determine the max-data-bitrate to use?

> > 
> > 
> > I assume the optimized propagation delay is 'always on' as the
> > transceiver is not able to detect which kind of bits it is processing.
> > That's why I think providing two bitrates leads to a wrong view on the
> > transceiver.
> 
> I agree with this.

I still disagree as I still think that CAN FD data phase (or fast phase)
has different round-trip delay (or "loop-delay symmetry") requirements
than the arbitration phase.
The CAN chip during data-phase does not need RX to follow TX as it does during
arbitration phase.  The optimized propagation delay is 'always on' but
only applicable or sufficient during data phase. during arbitration
phase, you (the CAN FD chip) requires a better round-trip delay, and the
transceiver cannot deliver this.

> 
> The transceiver is an analog device that needs to support faster
> switching frequency (FETs) including minimizing delay to support CAN-FD
> ie higher bitrate. From the transceiver perspective the bits for
> "arbitration" and "data" look exactly the same. Since it can't
> differentiate between the two (at the physical layer) then the actual
> limit isn't specific to which part/type of the CAN message is being
> sent. Rather its just a single overall max bitrate limit.

I must disagree here.
The transceiver is an analog device that performs 2 functions:
propagate tx bits to CAN wire, and propagate CAN wire state
(dominant/recesive) to rx bits.

I'll rephrase the above explanation to fit your argument:
During arbitration, both directions are required, and needs to propagate
within 1 bit time. The transceiver doesn't know, it just performs to
best effort.
During data, the round-trip timing requirement of layer2 is relaxed.
The transceiver still doesn't know, it still performs to best effort.
Due to the relaxed round-trip timing requirement, the same transceiver
can suddenly allow higher bitrates. The transceiver didn't change, the
requirement did change.
This is what I meant earlier with "layer2 has been adapted to circumvent
layer1 limitations"

Was I successfull in transcoding my thoughts onto email :-) ?

Kind regards,
Kurt

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Oliver Hartkopp]

Hi Kurt,

On 07/28/2017 09:41 PM, Kurt Van Dijck wrote:

>> The transceiver is an analog device that needs to support faster
>> switching frequency (FETs) including minimizing delay to support CAN-FD
>> ie higher bitrate. From the transceiver perspective the bits for
>> "arbitration" and "data" look exactly the same. Since it can't
>> differentiate between the two (at the physical layer) then the actual
>> limit isn't specific to which part/type of the CAN message is being
>> sent. Rather its just a single overall max bitrate limit.
> 
> I must disagree here.
> The transceiver is an analog device that performs 2 functions:
> propagate tx bits to CAN wire, and propagate CAN wire state
> (dominant/recesive) to rx bits.
> 
> I'll rephrase the above explanation to fit your argument:
> During arbitration, both directions are required, and needs to propagate
> within 1 bit time. The transceiver doesn't know, it just performs to
> best effort.
> During data, the round-trip timing requirement of layer2 is relaxed.
> The transceiver still doesn't know, it still performs to best effort.
> Due to the relaxed round-trip timing requirement, the same transceiver
> can suddenly allow higher bitrates. The transceiver didn't change, the
> requirement did change.
> This is what I meant earlier with "layer2 has been adapted to circumvent
> layer1 limitations"
> 
> Was I successfull in transcoding my thoughts onto email :-) ?

Yes. But it's not relevant.

I talked to our CAN transceiver & CAN physical layer specialist who was 
involved in the ISO activities too.

We just need ONE value: max-bitrate

The CAN transceiver is qualified to provide this bitrate. That's it.
There's nothing special with the arbitration bitrate - we don't even 
need to outline any CAN FD capability.

The other things like 'loop delay compensation' are done in the CAN 
controller. The better the transceiver get's the bits 'in shape' the 
higher it can be qualified. But from the SoC/Controller/Linux view we 
only need the max-bitrate value to double check with the CAN controllers 
bitrate configuration request (which was Franklins intention).

Best regards,
Oliver

Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings

[Kurt Van Dijck]

> Hi Kurt,
> 
> On 07/28/2017 09:41 PM, Kurt Van Dijck wrote:
> 
> >>The transceiver is an analog device that needs to support faster
> >>switching frequency (FETs) including minimizing delay to support CAN-FD
> >>ie higher bitrate. From the transceiver perspective the bits for
> >>"arbitration" and "data" look exactly the same. Since it can't
> >>differentiate between the two (at the physical layer) then the actual
> >>limit isn't specific to which part/type of the CAN message is being
> >>sent. Rather its just a single overall max bitrate limit.
> >
> >I must disagree here.
> >The transceiver is an analog device that performs 2 functions:
> >propagate tx bits to CAN wire, and propagate CAN wire state
> >(dominant/recesive) to rx bits.
> >
> >I'll rephrase the above explanation to fit your argument:
> >During arbitration, both directions are required, and needs to propagate
> >within 1 bit time. The transceiver doesn't know, it just performs to
> >best effort.
> >During data, the round-trip timing requirement of layer2 is relaxed.
> >The transceiver still doesn't know, it still performs to best effort.
> >Due to the relaxed round-trip timing requirement, the same transceiver
> >can suddenly allow higher bitrates. The transceiver didn't change, the
> >requirement did change.
> >This is what I meant earlier with "layer2 has been adapted to circumvent
> >layer1 limitations"
> >

> I talked to our CAN transceiver & CAN physical layer specialist who was
> involved in the ISO activities too.
> 
> We just need ONE value: max-bitrate
> 
> The CAN transceiver is qualified to provide this bitrate. That's it.
> There's nothing special with the arbitration bitrate - we don't even need to
> outline any CAN FD capability.
> 
> The other things like 'loop delay compensation' are done in the CAN
> controller. The better the transceiver get's the bits 'in shape' the higher
> it can be qualified. But from the SoC/Controller/Linux view we only need the
> max-bitrate value to double check with the CAN controllers bitrate
> configuration request (which was Franklins intention).

I bet your physical layer specialist understands the details way better
than I do :-)

1 value it is then.

Kind regards,
Kurt