| Message ID | 20210414140521.11463-1-a-govindraju@ti.com |
|---|---|
| Headers | show |
| Series | CAN TRANSCEIVER: Add support for CAN transceivers | expand |
Hi Marc, On 14/04/21 9:03 pm, Marc Kleine-Budde wrote: > On 14.04.2021 19:35:18, Aswath Govindraju wrote: >> Add binding documentation for TI TCAN104x CAN transceivers. >> >> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> >> --- >> .../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..4abfc30a97d0 >> --- /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: "^tcan104x-phy" >> + >> + compatible: >> + enum: >> + - ti,tcan1042 >> + - ti,tcan1043 > > Can you ensure that the 1042 has only the standby gpio and the 1043 has both? > In the driver, it is the way the flags have been set for ti,tcan1042 and 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: tcan104x-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_LOW>; > > AFAICS the enable gpio is active high. > I will correct this in the respin. Thanks, Aswath > Marc >
On 15.04.2021 11:57:20, Aswath Govindraju wrote: > Hi Marc, > > On 14/04/21 9:03 pm, Marc Kleine-Budde wrote: > > On 14.04.2021 19:35:18, Aswath Govindraju wrote: > >> Add binding documentation for TI TCAN104x CAN transceivers. > >> > >> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> > >> --- > >> .../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..4abfc30a97d0 > >> --- /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> Can you create a maintainers entry for this file with your address? > >> + > >> +properties: > >> + $nodename: > >> + pattern: "^tcan104x-phy" > >> + > >> + compatible: > >> + enum: > >> + - ti,tcan1042 > >> + - ti,tcan1043 > > > > Can you ensure that the 1042 has only the standby gpio and the 1043 has both? > > > > In the driver, it is the way the flags have been set for ti,tcan1042 and > ti,tcan1043. I was wondering if we would enforce in the DT the 1042 has exactly one the standby GPIO and the 1043 has exactly the standby and the enable GPIO. On the other hand the HW might have pulled one or the other pin high or low and only one of the pins is connected to a GPIO. > >> + > >> + '#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: tcan104x-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_LOW>; > > > > AFAICS the enable gpio is active high. > > > > I will correct this in the respin. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Marc, On 15/04/21 1:08 pm, Marc Kleine-Budde wrote: > On 15.04.2021 11:57:20, Aswath Govindraju wrote: >> Hi Marc, >> >> On 14/04/21 9:03 pm, Marc Kleine-Budde wrote: >>> On 14.04.2021 19:35:18, Aswath Govindraju wrote: >>>> Add binding documentation for TI TCAN104x CAN transceivers. >>>> >>>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> >>>> --- >>>> .../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..4abfc30a97d0 >>>> --- /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> > > Can you create a maintainers entry for this file with your address? I don't see this being done for other phy yamls in the Documentation/devicetree/bindings/phy folder. Also, scripts/get_maintainer.pl is giving the names of maintainers after reading the yaml files too. Thanks, Aswath > >>>> + >>>> +properties: >>>> + $nodename: >>>> + pattern: "^tcan104x-phy" >>>> + >>>> + compatible: >>>> + enum: >>>> + - ti,tcan1042 >>>> + - ti,tcan1043 >>> >>> Can you ensure that the 1042 has only the standby gpio and the 1043 has both? >>> >> >> In the driver, it is the way the flags have been set for ti,tcan1042 and >> ti,tcan1043. > > I was wondering if we would enforce in the DT the 1042 has exactly one > the standby GPIO and the 1043 has exactly the standby and the enable > GPIO. > > On the other hand the HW might have pulled one or the other pin high or > low and only one of the pins is connected to a GPIO. > >>>> + >>>> + '#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: tcan104x-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_LOW>; >>> >>> AFAICS the enable gpio is active high. >>> >> >> I will correct this in the respin. > > Marc >
On 15.04.2021 14:41:57, Aswath Govindraju wrote: > >>>> +++ b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml > > Can you create a maintainers entry for this file with your address? > > I don't see this being done for other phy yamls in the > Documentation/devicetree/bindings/phy folder. Also, > scripts/get_maintainer.pl is giving the names of maintainers after > reading the yaml files too. Nice! Clever script. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
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 adds an API for devm_of_phy_optional_get_by_index Patch 3 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 4 adds a generic driver to support CAN transceivers. Patches 5 & 6 add support for implementing the transceiver as a phy of m_can_platform driver. 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 (4): phy: core: Reword the comment specifying the units of max_link_rate to be Mbps phy: Add API for devm_of_phy_optional_get_by_index dt-bindings: phy: Add binding for TI TCAN104x CAN transceivers phy: phy-can-transceiver: Add support for generic CAN transceiver driver Faiz Abbas (2): dt-bindings: net: can: Document transceiver implementation as phy can: m_can: Add support for transceiver as phy .../bindings/net/can/bosch,m_can.yaml | 3 + .../bindings/phy/ti,tcan104x-can.yaml | 56 +++++++ MAINTAINERS | 2 + drivers/net/can/m_can/m_can.c | 18 +++ drivers/net/can/m_can/m_can.h | 2 + drivers/net/can/m_can/m_can_platform.c | 15 ++ drivers/phy/Kconfig | 9 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-can-transceiver.c | 146 ++++++++++++++++++ drivers/phy/phy-core.c | 26 ++++ include/linux/phy/phy.h | 4 +- 11 files changed, 281 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml create mode 100644 drivers/phy/phy-can-transceiver.c