linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] can: Support transceiver based bit rate limit
@ 2017-08-03  0:51 Franklin S Cooper Jr
  2017-08-03  0:51 ` [PATCH v3 1/4] can: dev: Add support for limiting configured bitrate Franklin S Cooper Jr
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Franklin S Cooper Jr @ 2017-08-03  0:51 UTC (permalink / raw)
  To: linux-kernel, devicetree, netdev, linux-can, wg, mkl, robh+dt,
	quentin.schulz, dev.kurt, andrew, sergei.shtylyov, socketcan
  Cc: Franklin S Cooper Jr

Add a new generic binding that CAN drivers can be used to specify the max
bit rate supported by a transceiver. This is useful since in some instances
since 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 3 changes:
Switch from having two "max bitrates" to one universal bitrate.

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
  dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver
    bindings
  dt-bindings: can: m_can: Include reference to new fixed transceiver
    binding
  can: m_can: Add call to of_can_transceiver_fixed

 .../bindings/net/can/fixed-transceiver.txt         | 24 +++++++++++
 .../devicetree/bindings/net/can/m_can.txt          |  9 ++++
 drivers/net/can/dev.c                              | 50 ++++++++++++++++++++++
 drivers/net/can/m_can/m_can.c                      |  2 +
 include/linux/can/dev.h                            |  5 +++
 5 files changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/fixed-transceiver.txt

-- 
2.9.4.dirty

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/4] can: dev: Add support for limiting configured bitrate
  2017-08-03  0:51 [PATCH v3 0/4] can: Support transceiver based bit rate limit Franklin S Cooper Jr
@ 2017-08-03  0:51 ` Franklin S Cooper Jr
  2017-08-03  0:51 ` [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings Franklin S Cooper Jr
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Franklin S Cooper Jr @ 2017-08-03  0:51 UTC (permalink / raw)
  To: linux-kernel, devicetree, netdev, linux-can, wg, mkl, robh+dt,
	quentin.schulz, dev.kurt, andrew, sergei.shtylyov, socketcan
  Cc: 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>
---
 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..db56914 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,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_fixed(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;
+
+	dn = of_get_child_by_name(np, "fixed-transceiver");
+	if (!dn)
+		return;
+
+	max_bitrate = 0;
+	ret = of_property_read_u32(dn, "max-bitrate", &max_bitrate);
+
+	if (max_bitrate > 0) {
+		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_fixed);
+#endif
+
 /*
  * Common close function for cleanup before the device gets closed.
  *
@@ -913,6 +947,14 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 					priv->bitrate_const_cnt);
 		if (err)
 			return err;
+
+		if (priv->is_bitrate_limited &&
+		    bt.bitrate > priv->max_bitrate) {
+			netdev_err(dev, "arbitration bitrate surpasses transceiver capabilities of %d bps\n",
+				   priv->max_bitrate);
+			return -EINVAL;
+		}
+
 		memcpy(&priv->bittiming, &bt, sizeof(bt));
 
 		if (priv->do_set_bittiming) {
@@ -997,6 +1039,14 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 					priv->data_bitrate_const_cnt);
 		if (err)
 			return err;
+
+		if (priv->is_bitrate_limited &&
+		    dbt.bitrate > priv->max_bitrate) {
+			netdev_err(dev, "canfd data bitrate surpasses transceiver capabilities of %d bps\n",
+				   priv->max_bitrate);
+			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..a6e62df 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;
 
+	unsigned int max_bitrate;
+	bool is_bitrate_limited;
+
 	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.9.4.dirty

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings
  2017-08-03  0:51 [PATCH v3 0/4] can: Support transceiver based bit rate limit Franklin S Cooper Jr
  2017-08-03  0:51 ` [PATCH v3 1/4] can: dev: Add support for limiting configured bitrate Franklin S Cooper Jr
@ 2017-08-03  0:51 ` Franklin S Cooper Jr
  2017-08-03  9:18   ` Sergei Shtylyov
  2017-08-03  0:51 ` [PATCH v3 3/4] dt-bindings: can: m_can: Include reference to new fixed transceiver binding Franklin S Cooper Jr
  2017-08-03  0:51 ` [PATCH v3 4/4] can: m_can: Add call to of_can_transceiver_fixed Franklin S Cooper Jr
  3 siblings, 1 reply; 12+ messages in thread
From: Franklin S Cooper Jr @ 2017-08-03  0:51 UTC (permalink / raw)
  To: linux-kernel, devicetree, netdev, linux-can, wg, mkl, robh+dt,
	quentin.schulz, dev.kurt, andrew, sergei.shtylyov, socketcan
  Cc: 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>
---
 .../bindings/net/can/fixed-transceiver.txt         | 24 ++++++++++++++++++++++
 1 file changed, 24 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..2f58838b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
@@ -0,0 +1,24 @@
+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.
+
+Required Properties:
+ max-bitrate:	a positive non 0 value that determines the max
+		speed that CAN/CAN-FD can run. Any other value
+		will be ignored.
+
+Examples:
+
+Based on Texas Instrument's TCAN1042HGV CAN Transceiver
+
+m_can0 {
+	....
+	fixed-transceiver@0 {
+		max-bitrate = <5000000>;
+	};
+	...
+};
-- 
2.9.4.dirty

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/4] dt-bindings: can: m_can: Include reference to new fixed transceiver binding
  2017-08-03  0:51 [PATCH v3 0/4] can: Support transceiver based bit rate limit Franklin S Cooper Jr
  2017-08-03  0:51 ` [PATCH v3 1/4] can: dev: Add support for limiting configured bitrate Franklin S Cooper Jr
  2017-08-03  0:51 ` [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings Franklin S Cooper Jr
@ 2017-08-03  0:51 ` Franklin S Cooper Jr
  2017-08-03  9:20   ` Sergei Shtylyov
  2017-08-03  0:51 ` [PATCH v3 4/4] can: m_can: Add call to of_can_transceiver_fixed Franklin S Cooper Jr
  3 siblings, 1 reply; 12+ messages in thread
From: Franklin S Cooper Jr @ 2017-08-03  0:51 UTC (permalink / raw)
  To: linux-kernel, devicetree, netdev, linux-can, wg, mkl, robh+dt,
	quentin.schulz, dev.kurt, andrew, sergei.shtylyov, socketcan
  Cc: 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>
---
 Documentation/devicetree/bindings/net/can/m_can.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt
index 9e33177..0b62f47 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/CAN-FD modes. See
+			  Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
+			  for details.
 Example:
 SoC dtsi:
 m_can1: can@020e8000 {
@@ -64,4 +69,8 @@ Board dts:
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_m_can1>;
 	status = "enabled";
+
+	fixed-transceiver@0 {
+		max-bitrate = <5000000>;
+	};
 };
-- 
2.9.4.dirty

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/4] can: m_can: Add call to of_can_transceiver_fixed
  2017-08-03  0:51 [PATCH v3 0/4] can: Support transceiver based bit rate limit Franklin S Cooper Jr
                   ` (2 preceding siblings ...)
  2017-08-03  0:51 ` [PATCH v3 3/4] dt-bindings: can: m_can: Include reference to new fixed transceiver binding Franklin S Cooper Jr
@ 2017-08-03  0:51 ` Franklin S Cooper Jr
  3 siblings, 0 replies; 12+ messages in thread
From: Franklin S Cooper Jr @ 2017-08-03  0:51 UTC (permalink / raw)
  To: linux-kernel, devicetree, netdev, linux-can, wg, mkl, robh+dt,
	quentin.schulz, dev.kurt, andrew, sergei.shtylyov, socketcan
  Cc: 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.9.4.dirty

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings
  2017-08-03  0:51 ` [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings Franklin S Cooper Jr
@ 2017-08-03  9:18   ` Sergei Shtylyov
  2017-08-03  9:48     ` Franklin S Cooper Jr
  0 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2017-08-03  9:18 UTC (permalink / raw)
  To: Franklin S Cooper Jr, linux-kernel, devicetree, netdev,
	linux-can, wg, mkl, robh+dt, quentin.schulz, dev.kurt, andrew,
	socketcan

Hello!

On 8/3/2017 3:51 AM, 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>
> ---
>   .../bindings/net/can/fixed-transceiver.txt         | 24 ++++++++++++++++++++++
>   1 file changed, 24 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..2f58838b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
> @@ -0,0 +1,24 @@
> +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.
> +
> +Required Properties:
> + max-bitrate:	a positive non 0 value that determines the max
> +		speed that CAN/CAN-FD can run. Any other value
> +		will be ignored.
> +
> +Examples:
> +
> +Based on Texas Instrument's TCAN1042HGV CAN Transceiver
> +
> +m_can0 {
> +	....
> +	fixed-transceiver@0 {

    The <unit-address> (after @) must only be specified if there's "reg" prop 
in the device node. Also, please name the node "can-transceiver@" to be more 
in line with the DT spec. which requires generic node names.

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/4] dt-bindings: can: m_can: Include reference to new fixed transceiver binding
  2017-08-03  0:51 ` [PATCH v3 3/4] dt-bindings: can: m_can: Include reference to new fixed transceiver binding Franklin S Cooper Jr
@ 2017-08-03  9:20   ` Sergei Shtylyov
  2017-08-03  9:54     ` Franklin S Cooper Jr
  0 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2017-08-03  9:20 UTC (permalink / raw)
  To: Franklin S Cooper Jr, linux-kernel, devicetree, netdev,
	linux-can, wg, mkl, robh+dt, quentin.schulz, dev.kurt, andrew,
	socketcan

On 8/3/2017 3:51 AM, 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>
> ---
>   Documentation/devicetree/bindings/net/can/m_can.txt | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt
> index 9e33177..0b62f47 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

    Subnode is not a property.

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

    Same comments here as in previous patch.

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings
  2017-08-03  9:18   ` Sergei Shtylyov
@ 2017-08-03  9:48     ` Franklin S Cooper Jr
  2017-08-03 12:22       ` Sergei Shtylyov
  0 siblings, 1 reply; 12+ messages in thread
From: Franklin S Cooper Jr @ 2017-08-03  9:48 UTC (permalink / raw)
  To: Sergei Shtylyov, linux-kernel, devicetree, netdev, linux-can, wg,
	mkl, robh+dt, quentin.schulz, dev.kurt, andrew, socketcan



On 08/03/2017 04:18 AM, Sergei Shtylyov wrote:
> Hello!
> 
> On 8/3/2017 3:51 AM, 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>
>> ---
>>   .../bindings/net/can/fixed-transceiver.txt         | 24
>> ++++++++++++++++++++++
>>   1 file changed, 24 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..2f58838b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>> @@ -0,0 +1,24 @@
>> +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.
>> +
>> +Required Properties:
>> + max-bitrate:    a positive non 0 value that determines the max
>> +        speed that CAN/CAN-FD can run. Any other value
>> +        will be ignored.
>> +
>> +Examples:
>> +
>> +Based on Texas Instrument's TCAN1042HGV CAN Transceiver
>> +
>> +m_can0 {
>> +    ....
>> +    fixed-transceiver@0 {
> 
>    The <unit-address> (after @) must only be specified if there's "reg"

Sorry. Fixed this in my v2 and some how it came back. Will fix.

> prop in the device node. Also, please name the node "can-transceiver@"
> to be more in line with the DT spec. which requires generic node names.

Its possible for future can transceivers drivers to be created. So I
thought including fixed was important to indicate that this is a "dumb"
transceiver similar to "fixed-link". So would "fixed-can-transceiver" be
ok or do you want to go with can-transceiver?
> 
> [...]

> 
> MBR, Sergei

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/4] dt-bindings: can: m_can: Include reference to new fixed transceiver binding
  2017-08-03  9:20   ` Sergei Shtylyov
@ 2017-08-03  9:54     ` Franklin S Cooper Jr
  0 siblings, 0 replies; 12+ messages in thread
From: Franklin S Cooper Jr @ 2017-08-03  9:54 UTC (permalink / raw)
  To: Sergei Shtylyov, linux-kernel, devicetree, netdev, linux-can, wg,
	mkl, robh+dt, quentin.schulz, dev.kurt, andrew, socketcan



On 08/03/2017 04:20 AM, Sergei Shtylyov wrote:
> On 8/3/2017 3:51 AM, 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>
>> ---
>>   Documentation/devicetree/bindings/net/can/m_can.txt | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt
>> b/Documentation/devicetree/bindings/net/can/m_can.txt
>> index 9e33177..0b62f47 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
> 
>    Subnode is not a property.

Ok makes sense. Several bindings refer to fixed-link as an optional
property. I thought there was some kind of weird reasoning to do so
which I decided to follow. I'll update it to say "Optional subnode".

> 
>> +              that can be used for CAN/CAN-FD modes. See
>> +             
>> Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>> +              for details.
>>   Example:
>>   SoC dtsi:
>>   m_can1: can@020e8000 {
>> @@ -64,4 +69,8 @@ Board dts:
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&pinctrl_m_can1>;
>>       status = "enabled";
>> +
>> +    fixed-transceiver@0 {
> 
>    Same comments here as in previous patch.

Will fix.
> 
> [...]
> 
> MBR, Sergei

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings
  2017-08-03  9:48     ` Franklin S Cooper Jr
@ 2017-08-03 12:22       ` Sergei Shtylyov
  2017-08-03 15:38         ` Franklin S Cooper Jr
  0 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2017-08-03 12:22 UTC (permalink / raw)
  To: Franklin S Cooper Jr, linux-kernel, devicetree, netdev,
	linux-can, wg, mkl, robh+dt, quentin.schulz, dev.kurt, andrew,
	socketcan

On 08/03/2017 12:48 PM, 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>
>>> ---
>>>    .../bindings/net/can/fixed-transceiver.txt         | 24
>>> ++++++++++++++++++++++
>>>    1 file changed, 24 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..2f58838b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>>> @@ -0,0 +1,24 @@
>>> +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.
>>> +
>>> +Required Properties:
>>> + max-bitrate:    a positive non 0 value that determines the max
>>> +        speed that CAN/CAN-FD can run. Any other value
>>> +        will be ignored.
>>> +
>>> +Examples:
>>> +
>>> +Based on Texas Instrument's TCAN1042HGV CAN Transceiver
>>> +
>>> +m_can0 {
>>> +    ....
>>> +    fixed-transceiver@0 {
>>
>>     The <unit-address> (after @) must only be specified if there's "reg"
> 
> Sorry. Fixed this in my v2 and some how it came back. Will fix.
> 
>> prop in the device node. Also, please name the node "can-transceiver@"
>> to be more in line with the DT spec. which requires generic node names.
> 
> Its possible for future can transceivers drivers to be created. So I

    So what? Ah, you are using the node name to match in the CAN drivers...

> thought including fixed was important to indicate that this is a "dumb"
> transceiver similar to "fixed-link".

    I'm not sure the "fixed-link" MAC subnode assumed any transceiver at all...

> So would "fixed-can-transceiver" be
> ok or do you want to go with can-transceiver?

    I'm somewhat perplexed at this point...

MBR, Sergei

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings
  2017-08-03 12:22       ` Sergei Shtylyov
@ 2017-08-03 15:38         ` Franklin S Cooper Jr
  2017-08-10  1:00           ` Franklin S Cooper Jr
  0 siblings, 1 reply; 12+ messages in thread
From: Franklin S Cooper Jr @ 2017-08-03 15:38 UTC (permalink / raw)
  To: Sergei Shtylyov, linux-kernel, devicetree, netdev, linux-can, wg,
	mkl, robh+dt, quentin.schulz, dev.kurt, andrew, socketcan



On 08/03/2017 07:22 AM, Sergei Shtylyov wrote:
> On 08/03/2017 12:48 PM, 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>
>>>> ---
>>>>    .../bindings/net/can/fixed-transceiver.txt         | 24
>>>> ++++++++++++++++++++++
>>>>    1 file changed, 24 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..2f58838b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>>>> @@ -0,0 +1,24 @@
>>>> +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.
>>>> +
>>>> +Required Properties:
>>>> + max-bitrate:    a positive non 0 value that determines the max
>>>> +        speed that CAN/CAN-FD can run. Any other value
>>>> +        will be ignored.
>>>> +
>>>> +Examples:
>>>> +
>>>> +Based on Texas Instrument's TCAN1042HGV CAN Transceiver
>>>> +
>>>> +m_can0 {
>>>> +    ....
>>>> +    fixed-transceiver@0 {
>>>
>>>     The <unit-address> (after @) must only be specified if there's "reg"
>>
>> Sorry. Fixed this in my v2 and some how it came back. Will fix.
>>
>>> prop in the device node. Also, please name the node "can-transceiver@"
>>> to be more in line with the DT spec. which requires generic node names.
>>
>> Its possible for future can transceivers drivers to be created. So I
> 
>    So what? Ah, you are using the node name to match in the CAN drivers...
> 
>> thought including fixed was important to indicate that this is a "dumb"
>> transceiver similar to "fixed-link".
> 
>    I'm not sure the "fixed-link" MAC subnode assumed any transceiver at
> all...

Your right. I wasn't trying to imply that it does. What I meant was that
having a node named "can-transceiver" may be a bit confusing in the
future if can transceiver drivers are created. Prefix of "fixed" atleast
to me makes it clear that this is something unique or a generic
transceiver with limitations. Similar to "fixed-link" which is for MACs
not connected to MDIO managed phy. Calling this subnode
"can-transceiver" to me would be like renaming "fixed-link" to "phy".

> 
>> So would "fixed-can-transceiver" be
>> ok or do you want to go with can-transceiver?
> 
>    I'm somewhat perplexed at this point...

If my reasoning still didn't change your views then I'll make the switch.
> 
> MBR, Sergei

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings
  2017-08-03 15:38         ` Franklin S Cooper Jr
@ 2017-08-10  1:00           ` Franklin S Cooper Jr
  0 siblings, 0 replies; 12+ messages in thread
From: Franklin S Cooper Jr @ 2017-08-10  1:00 UTC (permalink / raw)
  To: Sergei Shtylyov, linux-kernel, devicetree, netdev, linux-can, wg,
	mkl, robh+dt, quentin.schulz, dev.kurt, andrew, socketcan

Hi Sergei,

On 08/03/2017 10:38 AM, Franklin S Cooper Jr wrote:
> 
> 
> On 08/03/2017 07:22 AM, Sergei Shtylyov wrote:
>> On 08/03/2017 12:48 PM, 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>
>>>>> ---
>>>>>    .../bindings/net/can/fixed-transceiver.txt         | 24
>>>>> ++++++++++++++++++++++
>>>>>    1 file changed, 24 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..2f58838b
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>>>>> @@ -0,0 +1,24 @@
>>>>> +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.
>>>>> +
>>>>> +Required Properties:
>>>>> + max-bitrate:    a positive non 0 value that determines the max
>>>>> +        speed that CAN/CAN-FD can run. Any other value
>>>>> +        will be ignored.
>>>>> +
>>>>> +Examples:
>>>>> +
>>>>> +Based on Texas Instrument's TCAN1042HGV CAN Transceiver
>>>>> +
>>>>> +m_can0 {
>>>>> +    ....
>>>>> +    fixed-transceiver@0 {
>>>>
>>>>     The <unit-address> (after @) must only be specified if there's "reg"
>>>
>>> Sorry. Fixed this in my v2 and some how it came back. Will fix.
>>>
>>>> prop in the device node. Also, please name the node "can-transceiver@"
>>>> to be more in line with the DT spec. which requires generic node names.
>>>
>>> Its possible for future can transceivers drivers to be created. So I
>>
>>    So what? Ah, you are using the node name to match in the CAN drivers...
>>
>>> thought including fixed was important to indicate that this is a "dumb"
>>> transceiver similar to "fixed-link".
>>
>>    I'm not sure the "fixed-link" MAC subnode assumed any transceiver at
>> all...
> 
> Your right. I wasn't trying to imply that it does. What I meant was that
> having a node named "can-transceiver" may be a bit confusing in the
> future if can transceiver drivers are created. Prefix of "fixed" atleast
> to me makes it clear that this is something unique or a generic
> transceiver with limitations. Similar to "fixed-link" which is for MACs
> not connected to MDIO managed phy. Calling this subnode
> "can-transceiver" to me would be like renaming "fixed-link" to "phy".
> 
>>
>>> So would "fixed-can-transceiver" be
>>> ok or do you want to go with can-transceiver?
>>
>>    I'm somewhat perplexed at this point...
> 
> If my reasoning still didn't change your views then I'll make the switch.

I went ahead and made your suggested switch in my v4. Thanks for taking
the time to review this series.
>>
>> MBR, Sergei

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-08-10  1:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-03  0:51 [PATCH v3 0/4] can: Support transceiver based bit rate limit Franklin S Cooper Jr
2017-08-03  0:51 ` [PATCH v3 1/4] can: dev: Add support for limiting configured bitrate Franklin S Cooper Jr
2017-08-03  0:51 ` [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings Franklin S Cooper Jr
2017-08-03  9:18   ` Sergei Shtylyov
2017-08-03  9:48     ` Franklin S Cooper Jr
2017-08-03 12:22       ` Sergei Shtylyov
2017-08-03 15:38         ` Franklin S Cooper Jr
2017-08-10  1:00           ` Franklin S Cooper Jr
2017-08-03  0:51 ` [PATCH v3 3/4] dt-bindings: can: m_can: Include reference to new fixed transceiver binding Franklin S Cooper Jr
2017-08-03  9:20   ` Sergei Shtylyov
2017-08-03  9:54     ` Franklin S Cooper Jr
2017-08-03  0:51 ` [PATCH v3 4/4] can: m_can: Add call to of_can_transceiver_fixed Franklin S Cooper Jr

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).