arm64: dts: ti: k3-am654-icssg2: Enable PHY interrupts for ICSSG2

Message ID 20231120063159.539306-1-s-vadapalli@ti.com
State New
Headers
Series arm64: dts: ti: k3-am654-icssg2: Enable PHY interrupts for ICSSG2 |

Commit Message

Siddharth Vadapalli Nov. 20, 2023, 6:31 a.m. UTC
  Enable interrupt mode of operation of the DP83867 Ethernet PHY which is
used by ICSSG2. The DP83867 PHY driver already supports interrupt handling
for interrupts generated by the PHY. Thus, add the necessary device-tree
support to enable it.

Since the GPIO1_87 line is muxed with EXT_REFCLK1 and SYNC1_OUT, update
the pinmux to select GPIO1_87 for routing the interrupt.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

This patch is based on linux-next tagged next-20231120.

Regards,
Siddharth.

 arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
  

Comments

MD Danish Anwar Nov. 20, 2023, 11:29 a.m. UTC | #1
On 20/11/23 12:01 pm, Siddharth Vadapalli wrote:
> Enable interrupt mode of operation of the DP83867 Ethernet PHY which is
> used by ICSSG2. The DP83867 PHY driver already supports interrupt handling
> for interrupts generated by the PHY. Thus, add the necessary device-tree
> support to enable it.
> 
> Since the GPIO1_87 line is muxed with EXT_REFCLK1 and SYNC1_OUT, update
> the pinmux to select GPIO1_87 for routing the interrupt.
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
Reviewed-by: MD Danish Anwar <danishanwar@ti.com>

> ---
> 
> This patch is based on linux-next tagged next-20231120.
> 
> Regards,
> Siddharth.
> 
>  arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso b/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
> index ec8cf20ca3ac..9f723592d0f4 100644
> --- a/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
> +++ b/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
> @@ -124,21 +124,34 @@ AM65X_IOPAD(0x0088, PIN_INPUT, 2) /* (AG17) PRG2_PRU0_GPO4.PRG2_RGMII1_RX_CTL */
>  	};
>  };
>  
> +&main_pmx1 {
> +	/* Select GPIO1_87 for ICSSG2 PHY interrupt */
> +	icssg2_phy_irq_pins_default: icssg2-phy-irq-default-pins {
> +		pinctrl-single,pins = <
> +			AM65X_IOPAD(0x0014, PIN_INPUT, 7) /* (A22) EXT_REFCLK1.GPIO1_87 */
> +		>;
> +	};
> +};
> +
>  &icssg2_mdio {
>  	status = "okay";
> -	pinctrl-names = "default";
> -	pinctrl-0 = <&icssg2_mdio_pins_default>;
> +	pinctrl-names = "default", "icssg2-phy-irq";
> +	pinctrl-0 = <&icssg2_mdio_pins_default>, <&icssg2_phy_irq_pins_default>;
>  	#address-cells = <1>;
>  	#size-cells = <0>;
>  
>  	icssg2_phy0: ethernet-phy@0 {
>  		reg = <0>;
> +		interrupt-parent = <&main_gpio1>;
> +		interrupts = <87 0x2>;
>  		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
>  		ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
>  	};
>  
>  	icssg2_phy1: ethernet-phy@3 {
>  		reg = <3>;
> +		interrupt-parent = <&main_gpio1>;
> +		interrupts = <87 0x2>;
>  		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
>  		ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
>  	};
  
Nishanth Menon Dec. 4, 2023, 1:21 p.m. UTC | #2
On 12:01-20231120, Siddharth Vadapalli wrote:
> Enable interrupt mode of operation of the DP83867 Ethernet PHY which is
> used by ICSSG2. The DP83867 PHY driver already supports interrupt handling
> for interrupts generated by the PHY. Thus, add the necessary device-tree
> support to enable it.
> 
> Since the GPIO1_87 line is muxed with EXT_REFCLK1 and SYNC1_OUT, update
> the pinmux to select GPIO1_87 for routing the interrupt.
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
> 
> This patch is based on linux-next tagged next-20231120.
> 
> Regards,
> Siddharth.
> 
>  arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso b/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
> index ec8cf20ca3ac..9f723592d0f4 100644
> --- a/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
> +++ b/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
> @@ -124,21 +124,34 @@ AM65X_IOPAD(0x0088, PIN_INPUT, 2) /* (AG17) PRG2_PRU0_GPO4.PRG2_RGMII1_RX_CTL */
>  	};
>  };
>  
> +&main_pmx1 {
> +	/* Select GPIO1_87 for ICSSG2 PHY interrupt */
> +	icssg2_phy_irq_pins_default: icssg2-phy-irq-default-pins {
> +		pinctrl-single,pins = <
> +			AM65X_IOPAD(0x0014, PIN_INPUT, 7) /* (A22) EXT_REFCLK1.GPIO1_87 */
> +		>;
> +	};
> +};
> +
>  &icssg2_mdio {
>  	status = "okay";
> -	pinctrl-names = "default";
> -	pinctrl-0 = <&icssg2_mdio_pins_default>;
> +	pinctrl-names = "default", "icssg2-phy-irq";
> +	pinctrl-0 = <&icssg2_mdio_pins_default>, <&icssg2_phy_irq_pins_default>;

why should the pins be part of mdio pinctrl instead of phy?

>  	#address-cells = <1>;
>  	#size-cells = <0>;
>  
>  	icssg2_phy0: ethernet-phy@0 {
>  		reg = <0>;
> +		interrupt-parent = <&main_gpio1>;
> +		interrupts = <87 0x2>;
>  		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
>  		ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
>  	};
>  
>  	icssg2_phy1: ethernet-phy@3 {
>  		reg = <3>;
> +		interrupt-parent = <&main_gpio1>;
> +		interrupts = <87 0x2>;

Shouldn't you be using macros for interrupt level like IRQ_TYPE_EDGE_FALLING?

>  		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
>  		ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
>  	};
> -- 
> 2.34.1
>
  
Siddharth Vadapalli Dec. 13, 2023, 8:16 a.m. UTC | #3
Hello Nishanth,

Thank you for reviewing the patch. I have addressed your feedback in the v2
patch at:
https://lore.kernel.org/r/20231213080216.1710730-1-s-vadapalli@ti.com/

On 04/12/23 18:51, Nishanth Menon wrote:
> On 12:01-20231120, Siddharth Vadapalli wrote:
>> Enable interrupt mode of operation of the DP83867 Ethernet PHY which is
>> used by ICSSG2. The DP83867 PHY driver already supports interrupt handling
>> for interrupts generated by the PHY. Thus, add the necessary device-tree
>> support to enable it.
>>
>> Since the GPIO1_87 line is muxed with EXT_REFCLK1 and SYNC1_OUT, update
>> the pinmux to select GPIO1_87 for routing the interrupt.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>
>> This patch is based on linux-next tagged next-20231120.
>>
>> Regards,
>> Siddharth.
>>
>>  arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso b/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
>> index ec8cf20ca3ac..9f723592d0f4 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
>> +++ b/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
>> @@ -124,21 +124,34 @@ AM65X_IOPAD(0x0088, PIN_INPUT, 2) /* (AG17) PRG2_PRU0_GPO4.PRG2_RGMII1_RX_CTL */
>>  	};
>>  };
>>  
>> +&main_pmx1 {
>> +	/* Select GPIO1_87 for ICSSG2 PHY interrupt */
>> +	icssg2_phy_irq_pins_default: icssg2-phy-irq-default-pins {
>> +		pinctrl-single,pins = <
>> +			AM65X_IOPAD(0x0014, PIN_INPUT, 7) /* (A22) EXT_REFCLK1.GPIO1_87 */
>> +		>;
>> +	};
>> +};
>> +
>>  &icssg2_mdio {
>>  	status = "okay";
>> -	pinctrl-names = "default";
>> -	pinctrl-0 = <&icssg2_mdio_pins_default>;
>> +	pinctrl-names = "default", "icssg2-phy-irq";
>> +	pinctrl-0 = <&icssg2_mdio_pins_default>, <&icssg2_phy_irq_pins_default>;
> 
> why should the pins be part of mdio pinctrl instead of phy?
> 
>>  	#address-cells = <1>;
>>  	#size-cells = <0>;
>>  
>>  	icssg2_phy0: ethernet-phy@0 {
>>  		reg = <0>;
>> +		interrupt-parent = <&main_gpio1>;
>> +		interrupts = <87 0x2>;
>>  		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
>>  		ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
>>  	};
>>  
>>  	icssg2_phy1: ethernet-phy@3 {
>>  		reg = <3>;
>> +		interrupt-parent = <&main_gpio1>;
>> +		interrupts = <87 0x2>;
> 
> Shouldn't you be using macros for interrupt level like IRQ_TYPE_EDGE_FALLING?
> 
>>  		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
>>  		ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
>>  	};
>> -- 
>> 2.34.1
>>
>
  

Patch

diff --git a/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso b/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
index ec8cf20ca3ac..9f723592d0f4 100644
--- a/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
+++ b/arch/arm64/boot/dts/ti/k3-am654-icssg2.dtso
@@ -124,21 +124,34 @@  AM65X_IOPAD(0x0088, PIN_INPUT, 2) /* (AG17) PRG2_PRU0_GPO4.PRG2_RGMII1_RX_CTL */
 	};
 };
 
+&main_pmx1 {
+	/* Select GPIO1_87 for ICSSG2 PHY interrupt */
+	icssg2_phy_irq_pins_default: icssg2-phy-irq-default-pins {
+		pinctrl-single,pins = <
+			AM65X_IOPAD(0x0014, PIN_INPUT, 7) /* (A22) EXT_REFCLK1.GPIO1_87 */
+		>;
+	};
+};
+
 &icssg2_mdio {
 	status = "okay";
-	pinctrl-names = "default";
-	pinctrl-0 = <&icssg2_mdio_pins_default>;
+	pinctrl-names = "default", "icssg2-phy-irq";
+	pinctrl-0 = <&icssg2_mdio_pins_default>, <&icssg2_phy_irq_pins_default>;
 	#address-cells = <1>;
 	#size-cells = <0>;
 
 	icssg2_phy0: ethernet-phy@0 {
 		reg = <0>;
+		interrupt-parent = <&main_gpio1>;
+		interrupts = <87 0x2>;
 		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
 		ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
 	};
 
 	icssg2_phy1: ethernet-phy@3 {
 		reg = <3>;
+		interrupt-parent = <&main_gpio1>;
+		interrupts = <87 0x2>;
 		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
 		ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
 	};