[PATCH v6 0/3] CAN TRANSCEIVER: Add support for CAN transceivers

[PATCH v6 0/3] CAN TRANSCEIVER: Add support for CAN transceivers

[Aswath Govindraju]

The following series of patches add support for CAN transceivers.

TCAN1042 has a standby signal that needs to be pulled high for
sending/receiving messages[1]. TCAN1043 has a enable signal along with
standby signal that needs to be pulled up for sending/receiving
messages[2], and other combinations of the two lines can be used to put the
transceiver in different states to reduce power consumption. On boards
like the AM654-idk and J721e-evm these signals are controlled using gpios.

Patch 1 rewords the comment that restricts max_link_rate attribute to have
units of Mbps.

Patch 2 models the transceiver as a phy device tree node with properties
for max bit rate supported, gpio properties for indicating gpio pin numbers
to which standby and enable signals are connected.

Patch 2 adds a generic driver to support CAN transceivers.

changes since v5:
- Updated copyright year to 2021 in patch 3
- Rebased the series on top of latest linux next
- picked-up Marc Kleine-Budde's acked-by

changes since v4:
- In patch 3 made the correction from mcan to CAN, in Kconfig help

changes since v3:
- dropped patch 2(in v3)
- changed the node name property in patch 3(in v3)
- picked up Rob Herring's reviewed-by for patch 3(in v3)

changes since v2:
- dropped 5 and 6 patches and to be sent via linux-can-next
- added static keyword for can_transceiver_phy_probe()
- changed enable gpio example to active high in patch 3
- Rearranged the file names in alphabetical order in Makefile
  and MAINTAINERS file

changes since v1:
- Added patch 1 (in v2) that rewords the comment that restrict
  max_link_rate attribute to have units of Mbps.
- Added patch 2 (in v2) that adds an API for
  devm_of_phy_optional_get_by_index
- Patch 1 (in v1)
  - updated MAINTAINERS file
- Patch 2 (in v1)
  - replaced m_can with CAN to make the driver independent of CAN driver
  - Added prefix CAN_TRANSCEIVER for EN_PRESENT and STB_PRESENT
  - Added new line before return statements in power_on() and power_off
  - Added error handling patch for devm_kzalloc()
  - used the max_link_rate attribute directly instead of dividing it by
    1000000
  - removed the spaces before GPIOD_OUT_LOW in devm_gpiod_get()
  - Corrected requested value for standby-gpios to GPIOD_OUT_HIGH
  - Updated MAINTAINERS file
- Patch 3 (in v1)
  - replaced minItems with maxItems
  - Removed phy-names property as there is only one phy
- Patch 4 (in v1)
  - replaced dev_warn with dev_info when no transceiver is found
  - Added struct phy * field in m_can_classdev struct
  - moved phy_power_on and phy_power_off to m_can_open and m_can_close
    respectively
  - Moved the check for max_bit_rate to generice transceiver driver

[1] - https://www.ti.com/lit/ds/symlink/tcan1042h.pdf
[2] - https://www.ti.com/lit/ds/symlink/tcan1043-q1.pdf

Aswath Govindraju (3):
  phy: core: Reword the comment specifying the units of max_link_rate to
    be Mbps
  dt-bindings: phy: Add binding for TI TCAN104x CAN transceivers
  phy: phy-can-transceiver: Add support for generic CAN transceiver
    driver

 .../bindings/phy/ti,tcan104x-can.yaml         |  56 +++++++
 MAINTAINERS                                   |   2 +
 drivers/phy/Kconfig                           |   9 ++
 drivers/phy/Makefile                          |   1 +
 drivers/phy/phy-can-transceiver.c             | 146 ++++++++++++++++++
 include/linux/phy/phy.h                       |   2 +-
 6 files changed, 215 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
 create mode 100644 drivers/phy/phy-can-transceiver.c

-- 
2.17.1

[PATCH v6 1/3] phy: core: Reword the comment specifying the units of max_link_rate to be Mbps

[Aswath Govindraju]

In some subsystems (eg. CAN, SPI), the max link rate supported can be less
than 1 Mbps and if the unit for max_link_rate is Mbps then it can't be
used. Therefore, leave the decision of units to be used, to the producer
and consumer.

Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 include/linux/phy/phy.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 0ed434d02196..f3286f4cd306 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -125,7 +125,7 @@ struct phy_ops {
 /**
  * struct phy_attrs - represents phy attributes
  * @bus_width: Data path width implemented by PHY
- * @max_link_rate: Maximum link rate supported by PHY (in Mbps)
+ * @max_link_rate: Maximum link rate supported by PHY (units to be decided by producer and consumer)
  * @mode: PHY mode
  */
 struct phy_attrs {
-- 
2.17.1

[PATCH v6 2/3] dt-bindings: phy: Add binding for TI TCAN104x CAN transceivers

[Aswath Govindraju]

Add binding documentation for TI TCAN104x CAN transceivers.

Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 .../bindings/phy/ti,tcan104x-can.yaml         | 56 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml

diff --git a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
new file mode 100644
index 000000000000..6107880e5246
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/phy/ti,tcan104x-can.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: TCAN104x CAN TRANSCEIVER PHY
+
+maintainers:
+  - Aswath Govindraju <a-govindraju@ti.com>
+
+properties:
+  $nodename:
+    pattern: "^can-phy"
+
+  compatible:
+    enum:
+      - ti,tcan1042
+      - ti,tcan1043
+
+  '#phy-cells':
+    const: 0
+
+  standby-gpios:
+    description:
+      gpio node to toggle standby signal on transceiver
+    maxItems: 1
+
+  enable-gpios:
+    description:
+      gpio node to toggle enable signal on transceiver
+    maxItems: 1
+
+  max-bitrate:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      max bit rate supported in bps
+    minimum: 1
+
+required:
+  - compatible
+  - '#phy-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    transceiver1: can-phy {
+      compatible = "ti,tcan1043";
+      #phy-cells = <0>;
+      max-bitrate = <5000000>;
+      standby-gpios = <&wakeup_gpio1 16 GPIO_ACTIVE_LOW>;
+      enable-gpios = <&main_gpio1 67 GPIO_ACTIVE_HIGH>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 601b5ae0368a..492eb53c0db0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4040,6 +4040,7 @@ W:	https://github.com/linux-can
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
 F:	Documentation/devicetree/bindings/net/can/
+F:	Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
 F:	drivers/net/can/
 F:	include/linux/can/bittiming.h
 F:	include/linux/can/dev.h
-- 
2.17.1

[PATCH v6 3/3] phy: phy-can-transceiver: Add support for generic CAN transceiver driver

[Aswath Govindraju]

The driver adds support for generic CAN transceivers. Currently
the modes supported by this driver are standby and normal modes for TI
TCAN1042 and TCAN1043 CAN transceivers.

The transceiver is modelled as a phy with pins controlled by gpios, to put
the transceiver in various device functional modes. It also gets the phy
attribute max_link_rate for the usage of CAN drivers.

Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 MAINTAINERS                       |   1 +
 drivers/phy/Kconfig               |   9 ++
 drivers/phy/Makefile              |   1 +
 drivers/phy/phy-can-transceiver.c | 146 ++++++++++++++++++++++++++++++
 4 files changed, 157 insertions(+)
 create mode 100644 drivers/phy/phy-can-transceiver.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 492eb53c0db0..e7bd53caf5ff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4042,6 +4042,7 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
 F:	Documentation/devicetree/bindings/net/can/
 F:	Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
 F:	drivers/net/can/
+F:	drivers/phy/phy-can-transceiver.c
 F:	include/linux/can/bittiming.h
 F:	include/linux/can/dev.h
 F:	include/linux/can/led.h
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 54c1f2f0985f..7dd35f1b9cc5 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -61,6 +61,15 @@ config USB_LGM_PHY
 	  interface to interact with USB GEN-II and USB 3.x PHY that is part
 	  of the Intel network SOC.
 
+config PHY_CAN_TRANSCEIVER
+	tristate "CAN transceiver PHY"
+	select GENERIC_PHY
+	help
+	  This option enables support for CAN transceivers as a PHY. This
+	  driver provides function for putting the transceivers in various
+	  functional modes using gpios and sets the attribute max link
+	  rate, for CAN drivers.
+
 source "drivers/phy/allwinner/Kconfig"
 source "drivers/phy/amlogic/Kconfig"
 source "drivers/phy/broadcom/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index adac1b1a39d1..01e9efffc726 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -5,6 +5,7 @@
 
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
 obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY)	+= phy-core-mipi-dphy.o
+obj-$(CONFIG_PHY_CAN_TRANSCEIVER)	+= phy-can-transceiver.o
 obj-$(CONFIG_PHY_LPC18XX_USB_OTG)	+= phy-lpc18xx-usb-otg.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
 obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
new file mode 100644
index 000000000000..c2cb93b4df71
--- /dev/null
+++ b/drivers/phy/phy-can-transceiver.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * phy-can-transceiver.c - phy driver for CAN transceivers
+ *
+ * Copyright (C) 2021 Texas Instruments Incorporated - https://www.ti.com
+ *
+ */
+#include<linux/phy/phy.h>
+#include<linux/platform_device.h>
+#include<linux/module.h>
+#include<linux/gpio.h>
+#include<linux/gpio/consumer.h>
+
+struct can_transceiver_data {
+	u32 flags;
+#define CAN_TRANSCEIVER_STB_PRESENT	BIT(0)
+#define CAN_TRANSCEIVER_EN_PRESENT	BIT(1)
+};
+
+struct can_transceiver_phy {
+	struct phy *generic_phy;
+	struct gpio_desc *standby_gpio;
+	struct gpio_desc *enable_gpio;
+};
+
+/* Power on function */
+static int can_transceiver_phy_power_on(struct phy *phy)
+{
+	struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy);
+
+	if (can_transceiver_phy->standby_gpio)
+		gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 0);
+	if (can_transceiver_phy->enable_gpio)
+		gpiod_set_value_cansleep(can_transceiver_phy->enable_gpio, 1);
+
+	return 0;
+}
+
+/* Power off function */
+static int can_transceiver_phy_power_off(struct phy *phy)
+{
+	struct can_transceiver_phy *can_transceiver_phy = phy_get_drvdata(phy);
+
+	if (can_transceiver_phy->standby_gpio)
+		gpiod_set_value_cansleep(can_transceiver_phy->standby_gpio, 1);
+	if (can_transceiver_phy->enable_gpio)
+		gpiod_set_value_cansleep(can_transceiver_phy->enable_gpio, 0);
+
+	return 0;
+}
+
+static const struct phy_ops can_transceiver_phy_ops = {
+	.power_on	= can_transceiver_phy_power_on,
+	.power_off	= can_transceiver_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static const struct can_transceiver_data tcan1042_drvdata = {
+	.flags = CAN_TRANSCEIVER_STB_PRESENT,
+};
+
+static const struct can_transceiver_data tcan1043_drvdata = {
+	.flags = CAN_TRANSCEIVER_STB_PRESENT | CAN_TRANSCEIVER_EN_PRESENT,
+};
+
+static const struct of_device_id can_transceiver_phy_ids[] = {
+	{
+		.compatible = "ti,tcan1042",
+		.data = &tcan1042_drvdata
+	},
+	{
+		.compatible = "ti,tcan1043",
+		.data = &tcan1043_drvdata
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, can_transceiver_phy_ids);
+
+static int can_transceiver_phy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct device *dev = &pdev->dev;
+	struct can_transceiver_phy *can_transceiver_phy;
+	const struct can_transceiver_data *drvdata;
+	const struct of_device_id *match;
+	struct phy *phy;
+	struct gpio_desc *standby_gpio;
+	struct gpio_desc *enable_gpio;
+	u32 max_bitrate = 0;
+
+	can_transceiver_phy = devm_kzalloc(dev, sizeof(struct can_transceiver_phy), GFP_KERNEL);
+	if (!can_transceiver_phy)
+		return -ENOMEM;
+
+	match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node);
+	drvdata = match->data;
+
+	phy = devm_phy_create(dev, dev->of_node,
+			      &can_transceiver_phy_ops);
+	if (IS_ERR(phy)) {
+		dev_err(dev, "failed to create can transceiver phy\n");
+		return PTR_ERR(phy);
+	}
+
+	device_property_read_u32(dev, "max-bitrate", &max_bitrate);
+	if (!max_bitrate)
+		dev_warn(dev, "Invalid value for transceiver max bitrate. Ignoring bitrate limit\n");
+	phy->attrs.max_link_rate = max_bitrate;
+
+	can_transceiver_phy->generic_phy = phy;
+
+	if (drvdata->flags & CAN_TRANSCEIVER_STB_PRESENT) {
+		standby_gpio = devm_gpiod_get(dev, "standby", GPIOD_OUT_HIGH);
+		if (IS_ERR(standby_gpio))
+			return PTR_ERR(standby_gpio);
+		can_transceiver_phy->standby_gpio = standby_gpio;
+	}
+
+	if (drvdata->flags & CAN_TRANSCEIVER_EN_PRESENT) {
+		enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
+		if (IS_ERR(enable_gpio))
+			return PTR_ERR(enable_gpio);
+		can_transceiver_phy->enable_gpio = enable_gpio;
+	}
+
+	phy_set_drvdata(can_transceiver_phy->generic_phy, can_transceiver_phy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static struct platform_driver can_transceiver_phy_driver = {
+	.probe = can_transceiver_phy_probe,
+	.driver = {
+		.name = "can-transceiver-phy",
+		.of_match_table = can_transceiver_phy_ids,
+	},
+};
+
+module_platform_driver(can_transceiver_phy_driver);
+
+MODULE_AUTHOR("Faiz Abbas <faiz_abbas@ti.com>");
+MODULE_AUTHOR("Aswath Govindraju <a-govindraju@ti.com>");
+MODULE_DESCRIPTION("CAN TRANSCEIVER PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

Re: [PATCH v6 1/3] phy: core: Reword the comment specifying the units of max_link_rate to be Mbps

[Vinod Koul]

On 10-05-21, 10:40, Aswath Govindraju wrote:
> In some subsystems (eg. CAN, SPI), the max link rate supported can be less
> than 1 Mbps and if the unit for max_link_rate is Mbps then it can't be
> used. Therefore, leave the decision of units to be used, to the producer
> and consumer.
> 
> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  include/linux/phy/phy.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 0ed434d02196..f3286f4cd306 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -125,7 +125,7 @@ struct phy_ops {
>  /**
>   * struct phy_attrs - represents phy attributes
>   * @bus_width: Data path width implemented by PHY
> - * @max_link_rate: Maximum link rate supported by PHY (in Mbps)
> + * @max_link_rate: Maximum link rate supported by PHY (units to be decided by producer and consumer)

So there are a few users of max_link_rate. It would be better that we
document all previous users of max_link_rate that unit is in Mbps and
then modify it here

-- 
~Vinod

Re: [PATCH v6 1/3] phy: core: Reword the comment specifying the units of max_link_rate to be Mbps

[Aswath Govindraju]

Hi Vinod,

On 31/05/21 12:48 pm, Vinod Koul wrote:
> On 10-05-21, 10:40, Aswath Govindraju wrote:
>> In some subsystems (eg. CAN, SPI), the max link rate supported can be less
>> than 1 Mbps and if the unit for max_link_rate is Mbps then it can't be
>> used. Therefore, leave the decision of units to be used, to the producer
>> and consumer.
>>
>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>>  include/linux/phy/phy.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>> index 0ed434d02196..f3286f4cd306 100644
>> --- a/include/linux/phy/phy.h
>> +++ b/include/linux/phy/phy.h
>> @@ -125,7 +125,7 @@ struct phy_ops {
>>  /**
>>   * struct phy_attrs - represents phy attributes
>>   * @bus_width: Data path width implemented by PHY
>> - * @max_link_rate: Maximum link rate supported by PHY (in Mbps)
>> + * @max_link_rate: Maximum link rate supported by PHY (units to be decided by producer and consumer)
> 
> So there are a few users of max_link_rate. It would be better that we
> document all previous users of max_link_rate that unit is in Mbps and
> then modify it here
> 

I was able to see that the max_link_rate attribute was used at,

drivers/phy/cadence/phy-cadence-torrent.c:2514:
gphy->attrs.max_link_rate = cdns_phy->max_bit_rate;

and in the bindings there is indication that the units to be used is Mbps.

Can you please point me if there is any other place that I might have
missed to look at or that might need documentation update?

Thanks,
Aswath

Re: [PATCH v6 1/3] phy: core: Reword the comment specifying the units of max_link_rate to be Mbps

[Aswath Govindraju]

Hi Vinod,

On 31/05/21 2:04 pm, Aswath Govindraju wrote:
> Hi Vinod,
> 
> On 31/05/21 12:48 pm, Vinod Koul wrote:
>> On 10-05-21, 10:40, Aswath Govindraju wrote:
>>> In some subsystems (eg. CAN, SPI), the max link rate supported can be less
>>> than 1 Mbps and if the unit for max_link_rate is Mbps then it can't be
>>> used. Therefore, leave the decision of units to be used, to the producer
>>> and consumer.
>>>
>>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>> ---
>>>  include/linux/phy/phy.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>>> index 0ed434d02196..f3286f4cd306 100644
>>> --- a/include/linux/phy/phy.h
>>> +++ b/include/linux/phy/phy.h
>>> @@ -125,7 +125,7 @@ struct phy_ops {
>>>  /**
>>>   * struct phy_attrs - represents phy attributes
>>>   * @bus_width: Data path width implemented by PHY
>>> - * @max_link_rate: Maximum link rate supported by PHY (in Mbps)
>>> + * @max_link_rate: Maximum link rate supported by PHY (units to be decided by producer and consumer)
>>
>> So there are a few users of max_link_rate. It would be better that we
>> document all previous users of max_link_rate that unit is in Mbps and
>> then modify it here
>>
> 
> I was able to see that the max_link_rate attribute was used at,
> 
> drivers/phy/cadence/phy-cadence-torrent.c:2514:
> gphy->attrs.max_link_rate = cdns_phy->max_bit_rate;
> 
> and in the bindings there is indication that the units to be used is Mbps.
> 
> Can you please point me if there is any other place that I might have
> missed to look at or that might need documentation update?
> 

May I know if this patch series is good to be merged ?

Thanks,
Aswath

> Thanks,
> Aswath
> 
> 

Re: [PATCH v6 1/3] phy: core: Reword the comment specifying the units of max_link_rate to be Mbps

[Kishon Vijay Abraham I]

Hi,

On 11/06/21 6:12 pm, Aswath Govindraju wrote:
> Hi Vinod,
> 
> On 31/05/21 2:04 pm, Aswath Govindraju wrote:
>> Hi Vinod,
>>
>> On 31/05/21 12:48 pm, Vinod Koul wrote:
>>> On 10-05-21, 10:40, Aswath Govindraju wrote:
>>>> In some subsystems (eg. CAN, SPI), the max link rate supported can be less
>>>> than 1 Mbps and if the unit for max_link_rate is Mbps then it can't be
>>>> used. Therefore, leave the decision of units to be used, to the producer
>>>> and consumer.
>>>>
>>>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>>>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>>> ---
>>>>  include/linux/phy/phy.h | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>>>> index 0ed434d02196..f3286f4cd306 100644
>>>> --- a/include/linux/phy/phy.h
>>>> +++ b/include/linux/phy/phy.h
>>>> @@ -125,7 +125,7 @@ struct phy_ops {
>>>>  /**
>>>>   * struct phy_attrs - represents phy attributes
>>>>   * @bus_width: Data path width implemented by PHY
>>>> - * @max_link_rate: Maximum link rate supported by PHY (in Mbps)
>>>> + * @max_link_rate: Maximum link rate supported by PHY (units to be decided by producer and consumer)
>>>
>>> So there are a few users of max_link_rate. It would be better that we
>>> document all previous users of max_link_rate that unit is in Mbps and
>>> then modify it here
>>>
>>
>> I was able to see that the max_link_rate attribute was used at,
>>
>> drivers/phy/cadence/phy-cadence-torrent.c:2514:
>> gphy->attrs.max_link_rate = cdns_phy->max_bit_rate;
>>
>> and in the bindings there is indication that the units to be used is Mbps.
>>
>> Can you please point me if there is any other place that I might have
>> missed to look at or that might need documentation update?
>>

The only user seems to be Torrent and max_bit_rate is documented well in
Torrent.

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>

Thanks
Kishon

Re: [PATCH v6 1/3] phy: core: Reword the comment specifying the units of max_link_rate to be Mbps

[Vinod Koul]

On 11-06-21, 18:31, Kishon Vijay Abraham I wrote:

> >>> So there are a few users of max_link_rate. It would be better that we
> >>> document all previous users of max_link_rate that unit is in Mbps and
> >>> then modify it here
> >>>
> >>
> >> I was able to see that the max_link_rate attribute was used at,
> >>
> >> drivers/phy/cadence/phy-cadence-torrent.c:2514:
> >> gphy->attrs.max_link_rate = cdns_phy->max_bit_rate;
> >>
> >> and in the bindings there is indication that the units to be used is Mbps.
> >>
> >> Can you please point me if there is any other place that I might have
> >> missed to look at or that might need documentation update?
> >>
> 
> The only user seems to be Torrent and max_bit_rate is documented well in
> Torrent.
> 
> Acked-by: Kishon Vijay Abraham I <kishon@ti.com>

Series Applied, thanks

-- 
~Vinod