diff mbox series

[3/9] net: mdio: ipq4019: Enable GPIO reset for ipq5332 platform

Message ID 20231115032515.4249-4-quic_luoj@quicinc.com
State New
Headers
Series add MDIO changes on ipq5332 platform |

Commit Message

Jie Luo Nov. 15, 2023, 3:25 a.m. UTC 
  Before doing GPIO reset on the MDIO slave devices, the common clock
output to MDIO slave device should be enabled, and the related GCC
clocks also need to be configured.

Because of these extra configurations, the MDIO bus level GPIO and
PHY device level GPIO can't be leveraged. Need to add the device
tree property "phy-reset-gpio" of MDIO node to enable this special
GPIO reset.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/mdio/mdio-ipq4019.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

Andrew Lunn Nov. 15, 2023, 3:11 p.m. UTC | #1 
On Wed, Nov 15, 2023 at 11:25:09AM +0800, Luo Jie wrote:
> Before doing GPIO reset on the MDIO slave devices, the common clock
> output to MDIO slave device should be enabled, and the related GCC
> clocks also need to be configured.
> 
> Because of these extra configurations, the MDIO bus level GPIO and
> PHY device level GPIO can't be leveraged.

Its not clear to me why the normal reset cannot be used. The MBIO bus
driver can probe, setup the clocks, and then register the MDIO bus to
the core. The core can then use the GPIO resets.

What am i missing?

     Andrew
Jie Luo Nov. 16, 2023, 11:13 a.m. UTC | #2 
On 11/15/2023 11:11 PM, Andrew Lunn wrote:
> On Wed, Nov 15, 2023 at 11:25:09AM +0800, Luo Jie wrote:
>> Before doing GPIO reset on the MDIO slave devices, the common clock
>> output to MDIO slave device should be enabled, and the related GCC
>> clocks also need to be configured.
>>
>> Because of these extra configurations, the MDIO bus level GPIO and
>> PHY device level GPIO can't be leveraged.
> 
> Its not clear to me why the normal reset cannot be used. The MBIO bus
> driver can probe, setup the clocks, and then register the MDIO bus to
> the core. The core can then use the GPIO resets.
> 
> What am i missing?
> 
>       Andrew

Hi Andrew,
Looks we can leverage the MDIO bus GPIO to reset qca8084 PHY, but the
mdio bus gpio only supports one GPIO number.

Here are the reasons i put the GPIO reset here.
1. Currently one MDIO bus instance only connects one qca8084 PHY as
MDIO slave device on IPQ5332 platform, since the MDIO address
occupied by qca8084. if the other type PHY also needs to use MDIO
bus GPIO reset, then we can't cover this case.

2. Before doing the GPIO reset on qca8084, we need to enable the clock
output to qca8084 by configuring eth_ldo_rdy register, and the mdio
bus->reset is called after the mdio bus level reset.

3. program the mdio address of qca8084 PHY and the initialization
configurations needed before the registers of qca8084 can be accessed.
if we take the PHY level GPIO reset for qca8084, there is no call point
to do the initialization configurations and programing PHY address in
the MDIO driver code.

i will check the feasibility of taking the PHY level GPIO reset and do
the initial configurations in the PHY probe function.

FYI, here is the sequence to bring up qca8084.
a. enable clock output to qca8084.
b. do gpio reset of qca8084.
c. customize MDIO address and initialization configurations.
d. the PHY ID can be acquired.


Thanks,
Jie.
Robert Marko Nov. 16, 2023, 11:19 a.m. UTC | #3 
On Thu, Nov 16, 2023 at 12:14 PM Jie Luo <quic_luoj@quicinc.com> wrote:
>
>
>
> On 11/15/2023 11:11 PM, Andrew Lunn wrote:
> > On Wed, Nov 15, 2023 at 11:25:09AM +0800, Luo Jie wrote:
> >> Before doing GPIO reset on the MDIO slave devices, the common clock
> >> output to MDIO slave device should be enabled, and the related GCC
> >> clocks also need to be configured.
> >>
> >> Because of these extra configurations, the MDIO bus level GPIO and
> >> PHY device level GPIO can't be leveraged.
> >
> > Its not clear to me why the normal reset cannot be used. The MBIO bus
> > driver can probe, setup the clocks, and then register the MDIO bus to
> > the core. The core can then use the GPIO resets.
> >
> > What am i missing?
> >
> >       Andrew
>
> Hi Andrew,
> Looks we can leverage the MDIO bus GPIO to reset qca8084 PHY, but the
> mdio bus gpio only supports one GPIO number.

But, you can specify a PHY specific reset-gpio under the PHY subnode.
However, you must specify the PHY ID via compatible then, please look at:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/net/ethernet-phy.yaml?h=next-20231116#n36

I do this commonly when there are multiple reset GPIO-s for different ethernet
PHY-s.

Regards,
Robert
>
> Here are the reasons i put the GPIO reset here.
> 1. Currently one MDIO bus instance only connects one qca8084 PHY as
> MDIO slave device on IPQ5332 platform, since the MDIO address
> occupied by qca8084. if the other type PHY also needs to use MDIO
> bus GPIO reset, then we can't cover this case.
>
> 2. Before doing the GPIO reset on qca8084, we need to enable the clock
> output to qca8084 by configuring eth_ldo_rdy register, and the mdio
> bus->reset is called after the mdio bus level reset.
>
> 3. program the mdio address of qca8084 PHY and the initialization
> configurations needed before the registers of qca8084 can be accessed.
> if we take the PHY level GPIO reset for qca8084, there is no call point
> to do the initialization configurations and programing PHY address in
> the MDIO driver code.
>
> i will check the feasibility of taking the PHY level GPIO reset and do
> the initial configurations in the PHY probe function.
>
> FYI, here is the sequence to bring up qca8084.
> a. enable clock output to qca8084.
> b. do gpio reset of qca8084.
> c. customize MDIO address and initialization configurations.
> d. the PHY ID can be acquired.
>
>
> Thanks,
> Jie.
Jie Luo Nov. 16, 2023, 11:29 a.m. UTC | #4 
On 11/16/2023 7:19 PM, Robert Marko wrote:
> On Thu, Nov 16, 2023 at 12:14 PM Jie Luo <quic_luoj@quicinc.com> wrote:
>>
>>
>>
>> On 11/15/2023 11:11 PM, Andrew Lunn wrote:
>>> On Wed, Nov 15, 2023 at 11:25:09AM +0800, Luo Jie wrote:
>>>> Before doing GPIO reset on the MDIO slave devices, the common clock
>>>> output to MDIO slave device should be enabled, and the related GCC
>>>> clocks also need to be configured.
>>>>
>>>> Because of these extra configurations, the MDIO bus level GPIO and
>>>> PHY device level GPIO can't be leveraged.
>>>
>>> Its not clear to me why the normal reset cannot be used. The MBIO bus
>>> driver can probe, setup the clocks, and then register the MDIO bus to
>>> the core. The core can then use the GPIO resets.
>>>
>>> What am i missing?
>>>
>>>        Andrew
>>
>> Hi Andrew,
>> Looks we can leverage the MDIO bus GPIO to reset qca8084 PHY, but the
>> mdio bus gpio only supports one GPIO number.
> 
> But, you can specify a PHY specific reset-gpio under the PHY subnode.
> However, you must specify the PHY ID via compatible then, please look at:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/net/ethernet-phy.yaml?h=next-20231116#n36
> 
> I do this commonly when there are multiple reset GPIO-s for different ethernet
> PHY-s.
> 
> Regards,
> Robert

Got it, thanks Robert for the information, i will try the GPIO reset of
PHY DT node, and update it in the next patch set.

>>
>> Here are the reasons i put the GPIO reset here.
>> 1. Currently one MDIO bus instance only connects one qca8084 PHY as
>> MDIO slave device on IPQ5332 platform, since the MDIO address
>> occupied by qca8084. if the other type PHY also needs to use MDIO
>> bus GPIO reset, then we can't cover this case.
>>
>> 2. Before doing the GPIO reset on qca8084, we need to enable the clock
>> output to qca8084 by configuring eth_ldo_rdy register, and the mdio
>> bus->reset is called after the mdio bus level reset.
>>
>> 3. program the mdio address of qca8084 PHY and the initialization
>> configurations needed before the registers of qca8084 can be accessed.
>> if we take the PHY level GPIO reset for qca8084, there is no call point
>> to do the initialization configurations and programing PHY address in
>> the MDIO driver code.
>>
>> i will check the feasibility of taking the PHY level GPIO reset and do
>> the initial configurations in the PHY probe function.
>>
>> FYI, here is the sequence to bring up qca8084.
>> a. enable clock output to qca8084.
>> b. do gpio reset of qca8084.
>> c. customize MDIO address and initialization configurations.
>> d. the PHY ID can be acquired.
>>
>>
>> Thanks,
>> Jie.
> 
> 
>
Andrew Lunn Nov. 16, 2023, 5:20 p.m. UTC | #5 
> FYI, here is the sequence to bring up qca8084.
> a. enable clock output to qca8084.
> b. do gpio reset of qca8084.
> c. customize MDIO address and initialization configurations.
> d. the PHY ID can be acquired.

This all sounds like it is specific to the qca8084, so it should be in
the driver for the qca8084.

Its been pointed out you can get the driver to load by using the PHY
ID in the compatible. You want the SoC clock driver to export a CCF
clock, which the PHY driver can use. The PHY driver should also be
able to get the GPIO. So i think the PHY driver can do all this.

     Andrew
Jie Luo Nov. 17, 2023, 9:59 a.m. UTC | #6 
On 11/17/2023 1:20 AM, Andrew Lunn wrote:
>> FYI, here is the sequence to bring up qca8084.
>> a. enable clock output to qca8084.
>> b. do gpio reset of qca8084.
>> c. customize MDIO address and initialization configurations.
>> d. the PHY ID can be acquired.
> 
> This all sounds like it is specific to the qca8084, so it should be in
> the driver for the qca8084.
> 
> Its been pointed out you can get the driver to load by using the PHY
> ID in the compatible. You want the SoC clock driver to export a CCF
> clock, which the PHY driver can use. The PHY driver should also be
> able to get the GPIO. So i think the PHY driver can do all this.
> 
>       Andrew

Yes, Andrew, that is feasible, i will update the patches to move the
initialized clock configs in the PHY probe function.
Jie Luo Dec. 4, 2023, 8:53 a.m. UTC | #7 
On 11/17/2023 1:20 AM, Andrew Lunn wrote:
>> FYI, here is the sequence to bring up qca8084.
>> a. enable clock output to qca8084.
>> b. do gpio reset of qca8084.
>> c. customize MDIO address and initialization configurations.
>> d. the PHY ID can be acquired.
> 
> This all sounds like it is specific to the qca8084, so it should be in
> the driver for the qca8084.
> 
> Its been pointed out you can get the driver to load by using the PHY
> ID in the compatible. You want the SoC clock driver to export a CCF
> clock, which the PHY driver can use. The PHY driver should also be
> able to get the GPIO. So i think the PHY driver can do all this.
> 
>       Andrew

Hi Andrew,
If i put the GPIO reset in the PHY device tree node, the PHY probe
function will be postponed to be called instead of being called
during the MDIO bus register, which leads to the PCS can't be
created correctly because of reading PHY capability failed before
the PHY probe function called.

my device tree nodes are as below.

ethernet_device {
	phy-handle = <&phy3>;
	phy-mode = "2500base-x";
	...
};

mdio@90000 {
	phy3: ethernet-phy@3 {
		compatible = "ethernet-phy-id004d.d180";
		reg = <4>;
		reset-gpios = <&tlmm 51 GPIO_ACTIVE_LOW>;
		reset-assert-us = <100000>;
		reset-deassert-us = <100000>;
		clocks = <...>;
		clock-names = "...";
	};
};

Since the PHY probe function of phy3 is postponed instead of
called during the MDIO bus driver register, and the initialization
of qca8084 is not called when the ethernet_device driver is called
to create PCS, where the phy3 capability is checked, which is failed
since the qca8084 PHY probe is not called.

Any idea to resolve this call sequence issue?
Thanks.
diff mbox series

Patch

diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
index a77982a1a1e1..93ae4684de31 100644
--- a/drivers/net/mdio/mdio-ipq4019.c
+++ b/drivers/net/mdio/mdio-ipq4019.c
@@ -12,6 +12,7 @@ 
 #include <linux/phy.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
+#include <linux/gpio/consumer.h>
 
 #define MDIO_MODE_REG				0x40
 #define MDIO_ADDR_REG				0x44
@@ -55,6 +56,7 @@  struct ipq4019_mdio_data {
 	void __iomem *membase;
 	void __iomem *eth_ldo_rdy[ETH_LDO_RDY_CNT];
 	struct clk *clk[MDIO_CLK_CNT];
+	struct gpio_descs *reset_gpios;
 };
 
 const char *const mdio_clk_name[] = {
@@ -275,6 +277,24 @@  static int ipq_mdio_reset(struct mii_bus *bus)
 		}
 	}
 
+	/* Do the optional reset on the devices connected with MDIO bus */
+	if (priv->reset_gpios) {
+		unsigned long *values = bitmap_zalloc(priv->reset_gpios->ndescs, GFP_KERNEL);
+
+		if (!values)
+			return -ENOMEM;
+
+		bitmap_fill(values, priv->reset_gpios->ndescs);
+		gpiod_set_array_value_cansleep(priv->reset_gpios->ndescs, priv->reset_gpios->desc,
+					       priv->reset_gpios->info, values);
+
+		fsleep(IPQ_PHY_SET_DELAY_US);
+		bitmap_zero(values, priv->reset_gpios->ndescs);
+		gpiod_set_array_value_cansleep(priv->reset_gpios->ndescs, priv->reset_gpios->desc,
+					       priv->reset_gpios->info, values);
+		bitmap_free(values);
+	}
+
 	/* Configure MDIO clock source frequency if clock is specified in the device tree */
 	ret = clk_set_rate(priv->clk[MDIO_CLK_MDIO_AHB], IPQ_MDIO_CLK_RATE);
 	if (ret)
@@ -319,6 +339,19 @@  static int ipq4019_mdio_probe(struct platform_device *pdev)
 			return PTR_ERR(priv->clk[ret]);
 	}
 
+	/* This GPIO reset is for qca8084 PHY, which is only probeable by MDIO bus
+	 * after the following steps completed.
+	 *
+	 * 1. Enable LDO to provide clock for qca8084 and enable SoC GCC uniphy related clocks.
+	 * 2. Do GPIO reset on the qca8084 PHY.
+	 * 3. Configure the PHY address that is customized according to device treee.
+	 * 4. Configure the related qca8084 GCC clock & reset.
+	 */
+	priv->reset_gpios = devm_gpiod_get_array_optional(&pdev->dev, "phy-reset", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->reset_gpios))
+		return dev_err_probe(&pdev->dev, PTR_ERR(priv->reset_gpios),
+				     "mii_bus %s couldn't get reset GPIO\n", bus->id);
+
 	bus->name = "ipq4019_mdio";
 	bus->read = ipq4019_mdio_read_c22;
 	bus->write = ipq4019_mdio_write_c22;