2023-11-20 06:32:33

by Siddharth Vadapalli

Subject: [PATCH] arm64: dts: ti: k3-am654-icssg2: Enable PHY interrupts for ICSSG2

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 <[email protected]>
---

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>;
};
--
2.34.1

2023-11-20 11:29:49

by MD Danish Anwar

Subject: Re: [PATCH] arm64: dts: ti: k3-am654-icssg2: Enable PHY interrupts for ICSSG2

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 <[email protected]>
Reviewed-by: MD Danish Anwar <[email protected]>

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

--
Thanks and Regards,
Danish

2023-12-04 13:21:23

by Nishanth Menon

Subject: Re: [PATCH] arm64: dts: ti: k3-am654-icssg2: Enable PHY interrupts for ICSSG2

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 <[email protected]>
> ---
>
> 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
>

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2023-12-13 08:16:55

by Siddharth Vadapalli

Subject: Re: [PATCH] arm64: dts: ti: k3-am654-icssg2: Enable PHY interrupts for ICSSG2

Hello Nishanth,

Thank you for reviewing the patch. I have addressed your feedback in the v2
patch at:
https://lore.kernel.org/r/[email protected]/

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 <[email protected]>
>> ---
>>
>> 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
>>
>

--
Regards,
Siddharth.