* [U-Boot] [RFC PATCH] net: ag7xxx: Clean up some issues with phy access
@ 2017-06-12 20:20 Joe Hershberger
2017-06-13 9:24 ` Marek Vasut
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names Joe Hershberger
0 siblings, 2 replies; 17+ messages in thread
From: Joe Hershberger @ 2017-06-12 20:20 UTC (permalink / raw)
To: u-boot
Don't wait forever, Pass errors back, etc.
Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
This is a pass at improving the code quality.
This has not been tested in any way.
drivers/net/ag7xxx.c | 63 +++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 50 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
index cf60d11..c8352d1 100644
--- a/drivers/net/ag7xxx.c
+++ b/drivers/net/ag7xxx.c
@@ -26,6 +26,7 @@ enum ag7xxx_model {
AG7XXX_MODEL_AG934X,
};
+/* MAC Configuration 1 */
#define AG7XXX_ETH_CFG1 0x00
#define AG7XXX_ETH_CFG1_SOFT_RST BIT(31)
#define AG7XXX_ETH_CFG1_RX_RST BIT(19)
@@ -34,6 +35,7 @@ enum ag7xxx_model {
#define AG7XXX_ETH_CFG1_RX_EN BIT(2)
#define AG7XXX_ETH_CFG1_TX_EN BIT(0)
+/* MAC Configuration 2 */
#define AG7XXX_ETH_CFG2 0x04
#define AG7XXX_ETH_CFG2_IF_1000 BIT(9)
#define AG7XXX_ETH_CFG2_IF_10_100 BIT(8)
@@ -43,26 +45,34 @@ enum ag7xxx_model {
#define AG7XXX_ETH_CFG2_PAD_CRC_EN BIT(2)
#define AG7XXX_ETH_CFG2_FDX BIT(0)
+/* MII Configuration */
#define AG7XXX_ETH_MII_MGMT_CFG 0x20
#define AG7XXX_ETH_MII_MGMT_CFG_RESET BIT(31)
+/* MII Command */
#define AG7XXX_ETH_MII_MGMT_CMD 0x24
#define AG7XXX_ETH_MII_MGMT_CMD_READ 0x1
+/* MII Address */
#define AG7XXX_ETH_MII_MGMT_ADDRESS 0x28
#define AG7XXX_ETH_MII_MGMT_ADDRESS_SHIFT 8
+/* MII Control */
#define AG7XXX_ETH_MII_MGMT_CTRL 0x2c
+/* MII Status */
#define AG7XXX_ETH_MII_MGMT_STATUS 0x30
+/* MII Indicators */
#define AG7XXX_ETH_MII_MGMT_IND 0x34
#define AG7XXX_ETH_MII_MGMT_IND_INVALID BIT(2)
#define AG7XXX_ETH_MII_MGMT_IND_BUSY BIT(0)
+/* STA Address 1 & 2 */
#define AG7XXX_ETH_ADDR1 0x40
#define AG7XXX_ETH_ADDR2 0x44
+/* ETH Configuration 0 - 5 */
#define AG7XXX_ETH_FIFO_CFG_0 0x48
#define AG7XXX_ETH_FIFO_CFG_1 0x4c
#define AG7XXX_ETH_FIFO_CFG_2 0x50
@@ -70,20 +80,32 @@ enum ag7xxx_model {
#define AG7XXX_ETH_FIFO_CFG_4 0x58
#define AG7XXX_ETH_FIFO_CFG_5 0x5c
+/* DMA Transfer Control for Queue 0 */
#define AG7XXX_ETH_DMA_TX_CTRL 0x180
#define AG7XXX_ETH_DMA_TX_CTRL_TXE BIT(0)
+/* Descriptor Address for Queue 0 Tx */
#define AG7XXX_ETH_DMA_TX_DESC 0x184
+/* DMA Tx Status */
#define AG7XXX_ETH_DMA_TX_STATUS 0x188
+/* Rx Control */
#define AG7XXX_ETH_DMA_RX_CTRL 0x18c
#define AG7XXX_ETH_DMA_RX_CTRL_RXE BIT(0)
+/* Pointer to Rx Descriptor */
#define AG7XXX_ETH_DMA_RX_DESC 0x190
+/* Rx Status */
#define AG7XXX_ETH_DMA_RX_STATUS 0x194
+/* PHY Control Registers */
+
+/* PHY Specific Status Register */
+#define AG7XXX_PHY_PSSR 0x11
+#define AG7XXX_PHY_PSSR_LINK_UP BIT(10)
+
/* Custom register at 0x18070000 */
#define AG7XXX_GMAC_ETH_CFG 0x00
#define AG7XXX_ETH_CFG_SW_PHY_ADDR_SWAP BIT(8)
@@ -269,18 +291,33 @@ static int ag7xxx_switch_reg_write(struct mii_dev *bus, int reg, u32 val)
return 0;
}
-static u16 ag7xxx_mdio_rw(struct mii_dev *bus, int addr, int reg, u32 val)
+static int ag7xxx_mdio_rw(struct mii_dev *bus, int addr, int reg, u32 val)
{
u32 data;
+ unsigned long start;
+ int ret;
+ /* No idea if this is long enough or too long */
+ int timeout_ms = 1000;
/* Dummy read followed by PHY read/write command. */
- ag7xxx_switch_reg_read(bus, 0x98, &data);
+ ret = ag7xxx_switch_reg_read(bus, 0x98, &data);
+ if (ret < 0)
+ return ret;
data = val | (reg << 16) | (addr << 21) | BIT(30) | BIT(31);
- ag7xxx_switch_reg_write(bus, 0x98, data);
+ ret = ag7xxx_switch_reg_write(bus, 0x98, data);
+ if (ret < 0)
+ return ret;
+
+ start = get_timer(0);
/* Wait for operation to finish */
do {
- ag7xxx_switch_reg_read(bus, 0x98, &data);
+ ret = ag7xxx_switch_reg_read(bus, 0x98, &data);
+ if (ret < 0)
+ return ret;
+
+ if (get_timer(start) > timeout_ms)
+ return -ETIMEDOUT;
} while (data & BIT(31));
return data & 0xffff;
@@ -294,7 +331,11 @@ static int ag7xxx_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
static int ag7xxx_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
u16 val)
{
- ag7xxx_mdio_rw(bus, addr, reg, val);
+ int ret;
+
+ ret = ag7xxx_mdio_rw(bus, addr, reg, val);
+ if (ret < 0)
+ return ret;
return 0;
}
@@ -723,10 +764,13 @@ static int ag933x_phy_setup_common(struct udevice *dev)
return ret;
/* Read out link status */
- ret = ag7xxx_mdio_read(priv->bus, phymax, 0, MII_MIPSCR);
+ ret = ag7xxx_mdio_read(priv->bus, phymax, 0, AG7XXX_PHY_PSSR);
if (ret < 0)
return ret;
+ if (!(ret & AG7XXX_PHY_PSSR_LINK_UP))
+ return -ENOLINK;
+
return 0;
}
@@ -743,13 +787,6 @@ static int ag933x_phy_setup_common(struct udevice *dev)
return ret;
}
- for (i = 0; i < phymax; i++) {
- /* Read out link status */
- ret = ag7xxx_mdio_read(priv->bus, i, 0, MII_MIPSCR);
- if (ret < 0)
- return ret;
- }
-
return 0;
}
--
1.7.11.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH] net: ag7xxx: Clean up some issues with phy access
2017-06-12 20:20 [U-Boot] [RFC PATCH] net: ag7xxx: Clean up some issues with phy access Joe Hershberger
@ 2017-06-13 9:24 ` Marek Vasut
2017-06-13 16:28 ` Joe Hershberger
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names Joe Hershberger
1 sibling, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2017-06-13 9:24 UTC (permalink / raw)
To: u-boot
On 06/12/2017 10:20 PM, Joe Hershberger wrote:
> Don't wait forever, Pass errors back, etc.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>
> ---
> This is a pass at improving the code quality.
> This has not been tested in any way.
>
> drivers/net/ag7xxx.c | 63 +++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
> index cf60d11..c8352d1 100644
> --- a/drivers/net/ag7xxx.c
> +++ b/drivers/net/ag7xxx.c
> @@ -26,6 +26,7 @@ enum ag7xxx_model {
> AG7XXX_MODEL_AG934X,
> };
>
> +/* MAC Configuration 1 */
> #define AG7XXX_ETH_CFG1 0x00
> #define AG7XXX_ETH_CFG1_SOFT_RST BIT(31)
> #define AG7XXX_ETH_CFG1_RX_RST BIT(19)
> @@ -34,6 +35,7 @@ enum ag7xxx_model {
> #define AG7XXX_ETH_CFG1_RX_EN BIT(2)
> #define AG7XXX_ETH_CFG1_TX_EN BIT(0)
>
> +/* MAC Configuration 2 */
> #define AG7XXX_ETH_CFG2 0x04
> #define AG7XXX_ETH_CFG2_IF_1000 BIT(9)
> #define AG7XXX_ETH_CFG2_IF_10_100 BIT(8)
> @@ -43,26 +45,34 @@ enum ag7xxx_model {
> #define AG7XXX_ETH_CFG2_PAD_CRC_EN BIT(2)
> #define AG7XXX_ETH_CFG2_FDX BIT(0)
>
> +/* MII Configuration */
> #define AG7XXX_ETH_MII_MGMT_CFG 0x20
> #define AG7XXX_ETH_MII_MGMT_CFG_RESET BIT(31)
>
> +/* MII Command */
> #define AG7XXX_ETH_MII_MGMT_CMD 0x24
> #define AG7XXX_ETH_MII_MGMT_CMD_READ 0x1
>
> +/* MII Address */
> #define AG7XXX_ETH_MII_MGMT_ADDRESS 0x28
> #define AG7XXX_ETH_MII_MGMT_ADDRESS_SHIFT 8
>
> +/* MII Control */
> #define AG7XXX_ETH_MII_MGMT_CTRL 0x2c
>
> +/* MII Status */
> #define AG7XXX_ETH_MII_MGMT_STATUS 0x30
>
> +/* MII Indicators */
> #define AG7XXX_ETH_MII_MGMT_IND 0x34
> #define AG7XXX_ETH_MII_MGMT_IND_INVALID BIT(2)
> #define AG7XXX_ETH_MII_MGMT_IND_BUSY BIT(0)
>
> +/* STA Address 1 & 2 */
> #define AG7XXX_ETH_ADDR1 0x40
> #define AG7XXX_ETH_ADDR2 0x44
>
> +/* ETH Configuration 0 - 5 */
> #define AG7XXX_ETH_FIFO_CFG_0 0x48
> #define AG7XXX_ETH_FIFO_CFG_1 0x4c
> #define AG7XXX_ETH_FIFO_CFG_2 0x50
> @@ -70,20 +80,32 @@ enum ag7xxx_model {
> #define AG7XXX_ETH_FIFO_CFG_4 0x58
> #define AG7XXX_ETH_FIFO_CFG_5 0x5c
>
> +/* DMA Transfer Control for Queue 0 */
> #define AG7XXX_ETH_DMA_TX_CTRL 0x180
> #define AG7XXX_ETH_DMA_TX_CTRL_TXE BIT(0)
>
> +/* Descriptor Address for Queue 0 Tx */
> #define AG7XXX_ETH_DMA_TX_DESC 0x184
>
> +/* DMA Tx Status */
> #define AG7XXX_ETH_DMA_TX_STATUS 0x188
>
> +/* Rx Control */
> #define AG7XXX_ETH_DMA_RX_CTRL 0x18c
> #define AG7XXX_ETH_DMA_RX_CTRL_RXE BIT(0)
>
> +/* Pointer to Rx Descriptor */
> #define AG7XXX_ETH_DMA_RX_DESC 0x190
>
> +/* Rx Status */
> #define AG7XXX_ETH_DMA_RX_STATUS 0x194
>
> +/* PHY Control Registers */
> +
> +/* PHY Specific Status Register */
> +#define AG7XXX_PHY_PSSR 0x11
> +#define AG7XXX_PHY_PSSR_LINK_UP BIT(10)
> +
> /* Custom register at 0x18070000 */
> #define AG7XXX_GMAC_ETH_CFG 0x00
> #define AG7XXX_ETH_CFG_SW_PHY_ADDR_SWAP BIT(8)
> @@ -269,18 +291,33 @@ static int ag7xxx_switch_reg_write(struct mii_dev *bus, int reg, u32 val)
> return 0;
> }
>
> -static u16 ag7xxx_mdio_rw(struct mii_dev *bus, int addr, int reg, u32 val)
> +static int ag7xxx_mdio_rw(struct mii_dev *bus, int addr, int reg, u32 val)
> {
> u32 data;
> + unsigned long start;
> + int ret;
> + /* No idea if this is long enough or too long */
> + int timeout_ms = 1000;
>
> /* Dummy read followed by PHY read/write command. */
> - ag7xxx_switch_reg_read(bus, 0x98, &data);
> + ret = ag7xxx_switch_reg_read(bus, 0x98, &data);
> + if (ret < 0)
> + return ret;
> data = val | (reg << 16) | (addr << 21) | BIT(30) | BIT(31);
> - ag7xxx_switch_reg_write(bus, 0x98, data);
> + ret = ag7xxx_switch_reg_write(bus, 0x98, data);
> + if (ret < 0)
> + return ret;
> +
> + start = get_timer(0);
>
> /* Wait for operation to finish */
> do {
> - ag7xxx_switch_reg_read(bus, 0x98, &data);
> + ret = ag7xxx_switch_reg_read(bus, 0x98, &data);
> + if (ret < 0)
> + return ret;
> +
> + if (get_timer(start) > timeout_ms)
> + return -ETIMEDOUT;
> } while (data & BIT(31));
>
> return data & 0xffff;
> @@ -294,7 +331,11 @@ static int ag7xxx_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
> static int ag7xxx_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
> u16 val)
> {
> - ag7xxx_mdio_rw(bus, addr, reg, val);
> + int ret;
> +
> + ret = ag7xxx_mdio_rw(bus, addr, reg, val);
> + if (ret < 0)
> + return ret;
> return 0;
> }
>
> @@ -723,10 +764,13 @@ static int ag933x_phy_setup_common(struct udevice *dev)
> return ret;
>
> /* Read out link status */
> - ret = ag7xxx_mdio_read(priv->bus, phymax, 0, MII_MIPSCR);
> + ret = ag7xxx_mdio_read(priv->bus, phymax, 0, AG7XXX_PHY_PSSR);
> if (ret < 0)
> return ret;
>
> + if (!(ret & AG7XXX_PHY_PSSR_LINK_UP))
> + return -ENOLINK;
Are you sure about this ?
> return 0;
> }
>
> @@ -743,13 +787,6 @@ static int ag933x_phy_setup_common(struct udevice *dev)
> return ret;
> }
>
> - for (i = 0; i < phymax; i++) {
> - /* Read out link status */
> - ret = ag7xxx_mdio_read(priv->bus, i, 0, MII_MIPSCR);
> - if (ret < 0)
> - return ret;
> - }
And this ?
> return 0;
> }
>
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH] net: ag7xxx: Clean up some issues with phy access
2017-06-13 9:24 ` Marek Vasut
@ 2017-06-13 16:28 ` Joe Hershberger
2017-06-19 9:37 ` Marek Vasut
0 siblings, 1 reply; 17+ messages in thread
From: Joe Hershberger @ 2017-06-13 16:28 UTC (permalink / raw)
To: u-boot
On Tue, Jun 13, 2017 at 4:24 AM, Marek Vasut <marex@denx.de> wrote:
> On 06/12/2017 10:20 PM, Joe Hershberger wrote:
>> Don't wait forever, Pass errors back, etc.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>>
>> ---
>> This is a pass at improving the code quality.
>> This has not been tested in any way.
>>
>> drivers/net/ag7xxx.c | 63 +++++++++++++++++++++++++++++++++++++++++-----------
>> 1 file changed, 50 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
>> index cf60d11..c8352d1 100644
>> --- a/drivers/net/ag7xxx.c
>> +++ b/drivers/net/ag7xxx.c
[...] SNIP
>> @@ -723,10 +764,13 @@ static int ag933x_phy_setup_common(struct udevice *dev)
>> return ret;
>>
>> /* Read out link status */
>> - ret = ag7xxx_mdio_read(priv->bus, phymax, 0, MII_MIPSCR);
>> + ret = ag7xxx_mdio_read(priv->bus, phymax, 0, AG7XXX_PHY_PSSR);
>> if (ret < 0)
>> return ret;
>>
>> + if (!(ret & AG7XXX_PHY_PSSR_LINK_UP))
>> + return -ENOLINK;
>
> Are you sure about this ?
It seems reasonable to me, but I don't have the HW to test against as
noted above.
>> return 0;
>> }
>>
>> @@ -743,13 +787,6 @@ static int ag933x_phy_setup_common(struct udevice *dev)
>> return ret;
>> }
>>
>> - for (i = 0; i < phymax; i++) {
>> - /* Read out link status */
>> - ret = ag7xxx_mdio_read(priv->bus, i, 0, MII_MIPSCR);
>> - if (ret < 0)
>> - return ret;
>> - }
>
> And this ?
This was based on your comment: "Actually, I think this is only for
the switch ports, so we don't care about the link status."
>> return 0;
>> }
>>
>>
>
>
> --
> Best regards,
> Marek Vasut
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH] net: ag7xxx: Clean up some issues with phy access
2017-06-13 16:28 ` Joe Hershberger
@ 2017-06-19 9:37 ` Marek Vasut
2017-06-19 15:55 ` Joe Hershberger
0 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2017-06-19 9:37 UTC (permalink / raw)
To: u-boot
On 06/13/2017 06:28 PM, Joe Hershberger wrote:
> On Tue, Jun 13, 2017 at 4:24 AM, Marek Vasut <marex@denx.de> wrote:
>> On 06/12/2017 10:20 PM, Joe Hershberger wrote:
>>> Don't wait forever, Pass errors back, etc.
>>>
>>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>>>
>>> ---
>>> This is a pass at improving the code quality.
>>> This has not been tested in any way.
>>>
>>> drivers/net/ag7xxx.c | 63 +++++++++++++++++++++++++++++++++++++++++-----------
>>> 1 file changed, 50 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
>>> index cf60d11..c8352d1 100644
>>> --- a/drivers/net/ag7xxx.c
>>> +++ b/drivers/net/ag7xxx.c
>
> [...] SNIP
>
>>> @@ -723,10 +764,13 @@ static int ag933x_phy_setup_common(struct udevice *dev)
>>> return ret;
>>>
>>> /* Read out link status */
>>> - ret = ag7xxx_mdio_read(priv->bus, phymax, 0, MII_MIPSCR);
>>> + ret = ag7xxx_mdio_read(priv->bus, phymax, 0, AG7XXX_PHY_PSSR);
>>> if (ret < 0)
>>> return ret;
>>>
>>> + if (!(ret & AG7XXX_PHY_PSSR_LINK_UP))
>>> + return -ENOLINK;
>>
>> Are you sure about this ?
>
> It seems reasonable to me, but I don't have the HW to test against as
> noted above.
CCing Wills . I wouldn't be surprised if the hardware was somehow
magically screwed up, so I'd prefer to stick with equivalent changes and
maybe changes of the logic in a separate patch.
>>> return 0;
>>> }
>>>
>>> @@ -743,13 +787,6 @@ static int ag933x_phy_setup_common(struct udevice *dev)
>>> return ret;
>>> }
>>>
>>> - for (i = 0; i < phymax; i++) {
>>> - /* Read out link status */
>>> - ret = ag7xxx_mdio_read(priv->bus, i, 0, MII_MIPSCR);
>>> - if (ret < 0)
>>> - return ret;
>>> - }
>>
>> And this ?
>
> This was based on your comment: "Actually, I think this is only for
> the switch ports, so we don't care about the link status."
Just so I understand it correctly, the code reads link status and does
nothing with it ?
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH] net: ag7xxx: Clean up some issues with phy access
2017-06-19 9:37 ` Marek Vasut
@ 2017-06-19 15:55 ` Joe Hershberger
2017-06-20 9:25 ` Marek Vasut
0 siblings, 1 reply; 17+ messages in thread
From: Joe Hershberger @ 2017-06-19 15:55 UTC (permalink / raw)
To: u-boot
On Mon, Jun 19, 2017 at 4:37 AM, Marek Vasut <marex@denx.de> wrote:
> On 06/13/2017 06:28 PM, Joe Hershberger wrote:
>> On Tue, Jun 13, 2017 at 4:24 AM, Marek Vasut <marex@denx.de> wrote:
>>> On 06/12/2017 10:20 PM, Joe Hershberger wrote:
>>>> Don't wait forever, Pass errors back, etc.
>>>>
>>>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>>>>
>>>> ---
>>>> This is a pass at improving the code quality.
>>>> This has not been tested in any way.
>>>>
>>>> drivers/net/ag7xxx.c | 63 +++++++++++++++++++++++++++++++++++++++++-----------
>>>> 1 file changed, 50 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
>>>> index cf60d11..c8352d1 100644
>>>> --- a/drivers/net/ag7xxx.c
>>>> +++ b/drivers/net/ag7xxx.c
>>
>> [...] SNIP
>>
>>>> @@ -723,10 +764,13 @@ static int ag933x_phy_setup_common(struct udevice *dev)
>>>> return ret;
>>>>
>>>> /* Read out link status */
>>>> - ret = ag7xxx_mdio_read(priv->bus, phymax, 0, MII_MIPSCR);
>>>> + ret = ag7xxx_mdio_read(priv->bus, phymax, 0, AG7XXX_PHY_PSSR);
>>>> if (ret < 0)
>>>> return ret;
>>>>
>>>> + if (!(ret & AG7XXX_PHY_PSSR_LINK_UP))
>>>> + return -ENOLINK;
>>>
>>> Are you sure about this ?
>>
>> It seems reasonable to me, but I don't have the HW to test against as
>> noted above.
>
> CCing Wills . I wouldn't be surprised if the hardware was somehow
> magically screwed up, so I'd prefer to stick with equivalent changes and
> maybe changes of the logic in a separate patch.
OK.
>>>> return 0;
>>>> }
>>>>
>>>> @@ -743,13 +787,6 @@ static int ag933x_phy_setup_common(struct udevice *dev)
>>>> return ret;
>>>> }
>>>>
>>>> - for (i = 0; i < phymax; i++) {
>>>> - /* Read out link status */
>>>> - ret = ag7xxx_mdio_read(priv->bus, i, 0, MII_MIPSCR);
>>>> - if (ret < 0)
>>>> - return ret;
>>>> - }
>>>
>>> And this ?
>>
>> This was based on your comment: "Actually, I think this is only for
>> the switch ports, so we don't care about the link status."
>
> Just so I understand it correctly, the code reads link status and does
> nothing with it ?
That's how I read it.
>
> --
> Best regards,
> Marek Vasut
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH] net: ag7xxx: Clean up some issues with phy access
2017-06-19 15:55 ` Joe Hershberger
@ 2017-06-20 9:25 ` Marek Vasut
0 siblings, 0 replies; 17+ messages in thread
From: Marek Vasut @ 2017-06-20 9:25 UTC (permalink / raw)
To: u-boot
On 06/19/2017 05:55 PM, Joe Hershberger wrote:
> On Mon, Jun 19, 2017 at 4:37 AM, Marek Vasut <marex@denx.de> wrote:
>> On 06/13/2017 06:28 PM, Joe Hershberger wrote:
>>> On Tue, Jun 13, 2017 at 4:24 AM, Marek Vasut <marex@denx.de> wrote:
>>>> On 06/12/2017 10:20 PM, Joe Hershberger wrote:
>>>>> Don't wait forever, Pass errors back, etc.
>>>>>
>>>>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>>>>>
>>>>> ---
>>>>> This is a pass at improving the code quality.
>>>>> This has not been tested in any way.
>>>>>
>>>>> drivers/net/ag7xxx.c | 63 +++++++++++++++++++++++++++++++++++++++++-----------
>>>>> 1 file changed, 50 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
>>>>> index cf60d11..c8352d1 100644
>>>>> --- a/drivers/net/ag7xxx.c
>>>>> +++ b/drivers/net/ag7xxx.c
>>>
>>> [...] SNIP
>>>
>>>>> @@ -723,10 +764,13 @@ static int ag933x_phy_setup_common(struct udevice *dev)
>>>>> return ret;
>>>>>
>>>>> /* Read out link status */
>>>>> - ret = ag7xxx_mdio_read(priv->bus, phymax, 0, MII_MIPSCR);
>>>>> + ret = ag7xxx_mdio_read(priv->bus, phymax, 0, AG7XXX_PHY_PSSR);
>>>>> if (ret < 0)
>>>>> return ret;
>>>>>
>>>>> + if (!(ret & AG7XXX_PHY_PSSR_LINK_UP))
>>>>> + return -ENOLINK;
>>>>
>>>> Are you sure about this ?
>>>
>>> It seems reasonable to me, but I don't have the HW to test against as
>>> noted above.
>>
>> CCing Wills . I wouldn't be surprised if the hardware was somehow
>> magically screwed up, so I'd prefer to stick with equivalent changes and
>> maybe changes of the logic in a separate patch.
>
> OK.
>
>>>>> return 0;
>>>>> }
>>>>>
>>>>> @@ -743,13 +787,6 @@ static int ag933x_phy_setup_common(struct udevice *dev)
>>>>> return ret;
>>>>> }
>>>>>
>>>>> - for (i = 0; i < phymax; i++) {
>>>>> - /* Read out link status */
>>>>> - ret = ag7xxx_mdio_read(priv->bus, i, 0, MII_MIPSCR);
>>>>> - if (ret < 0)
>>>>> - return ret;
>>>>> - }
>>>>
>>>> And this ?
>>>
>>> This was based on your comment: "Actually, I think this is only for
>>> the switch ports, so we don't care about the link status."
>>
>> Just so I understand it correctly, the code reads link status and does
>> nothing with it ?
>
> That's how I read it.
Sigh, well ... if you could split the patch in two, that'd be nice. I
will try to test it as soon as I have a chance. If it's broken, I'll try
to bisect it then.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names
2017-06-12 20:20 [U-Boot] [RFC PATCH] net: ag7xxx: Clean up some issues with phy access Joe Hershberger
2017-06-13 9:24 ` Marek Vasut
@ 2017-06-26 19:40 ` Joe Hershberger
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 2/3] net: ag7xxx: Propagate errors on phy access Joe Hershberger
` (3 more replies)
1 sibling, 4 replies; 17+ messages in thread
From: Joe Hershberger @ 2017-06-26 19:40 UTC (permalink / raw)
To: u-boot
The register constants don't use the exact names that are used in the
TRM, so add comments that use the exact names so that it is clear what
register is being referred to.
https://www.atheros-drivers.com/qualcomm-atheros-datasheets-for-AR9331.html
Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v2:
- New - Comments split into a separate change
drivers/net/ag7xxx.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
index cf60d11..30771b9 100644
--- a/drivers/net/ag7xxx.c
+++ b/drivers/net/ag7xxx.c
@@ -26,6 +26,7 @@ enum ag7xxx_model {
AG7XXX_MODEL_AG934X,
};
+/* MAC Configuration 1 */
#define AG7XXX_ETH_CFG1 0x00
#define AG7XXX_ETH_CFG1_SOFT_RST BIT(31)
#define AG7XXX_ETH_CFG1_RX_RST BIT(19)
@@ -34,6 +35,7 @@ enum ag7xxx_model {
#define AG7XXX_ETH_CFG1_RX_EN BIT(2)
#define AG7XXX_ETH_CFG1_TX_EN BIT(0)
+/* MAC Configuration 2 */
#define AG7XXX_ETH_CFG2 0x04
#define AG7XXX_ETH_CFG2_IF_1000 BIT(9)
#define AG7XXX_ETH_CFG2_IF_10_100 BIT(8)
@@ -43,26 +45,34 @@ enum ag7xxx_model {
#define AG7XXX_ETH_CFG2_PAD_CRC_EN BIT(2)
#define AG7XXX_ETH_CFG2_FDX BIT(0)
+/* MII Configuration */
#define AG7XXX_ETH_MII_MGMT_CFG 0x20
#define AG7XXX_ETH_MII_MGMT_CFG_RESET BIT(31)
+/* MII Command */
#define AG7XXX_ETH_MII_MGMT_CMD 0x24
#define AG7XXX_ETH_MII_MGMT_CMD_READ 0x1
+/* MII Address */
#define AG7XXX_ETH_MII_MGMT_ADDRESS 0x28
#define AG7XXX_ETH_MII_MGMT_ADDRESS_SHIFT 8
+/* MII Control */
#define AG7XXX_ETH_MII_MGMT_CTRL 0x2c
+/* MII Status */
#define AG7XXX_ETH_MII_MGMT_STATUS 0x30
+/* MII Indicators */
#define AG7XXX_ETH_MII_MGMT_IND 0x34
#define AG7XXX_ETH_MII_MGMT_IND_INVALID BIT(2)
#define AG7XXX_ETH_MII_MGMT_IND_BUSY BIT(0)
+/* STA Address 1 & 2 */
#define AG7XXX_ETH_ADDR1 0x40
#define AG7XXX_ETH_ADDR2 0x44
+/* ETH Configuration 0 - 5 */
#define AG7XXX_ETH_FIFO_CFG_0 0x48
#define AG7XXX_ETH_FIFO_CFG_1 0x4c
#define AG7XXX_ETH_FIFO_CFG_2 0x50
@@ -70,18 +80,24 @@ enum ag7xxx_model {
#define AG7XXX_ETH_FIFO_CFG_4 0x58
#define AG7XXX_ETH_FIFO_CFG_5 0x5c
+/* DMA Transfer Control for Queue 0 */
#define AG7XXX_ETH_DMA_TX_CTRL 0x180
#define AG7XXX_ETH_DMA_TX_CTRL_TXE BIT(0)
+/* Descriptor Address for Queue 0 Tx */
#define AG7XXX_ETH_DMA_TX_DESC 0x184
+/* DMA Tx Status */
#define AG7XXX_ETH_DMA_TX_STATUS 0x188
+/* Rx Control */
#define AG7XXX_ETH_DMA_RX_CTRL 0x18c
#define AG7XXX_ETH_DMA_RX_CTRL_RXE BIT(0)
+/* Pointer to Rx Descriptor */
#define AG7XXX_ETH_DMA_RX_DESC 0x190
+/* Rx Status */
#define AG7XXX_ETH_DMA_RX_STATUS 0x194
/* Custom register at 0x18070000 */
--
1.7.11.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH v2 2/3] net: ag7xxx: Propagate errors on phy access
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names Joe Hershberger
@ 2017-06-26 19:40 ` Joe Hershberger
2017-06-27 9:32 ` Marek Vasut
2017-08-07 20:32 ` [U-Boot] " Joe Hershberger
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 3/3] net: ag7xxx: No longer ignore link status Joe Hershberger
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Joe Hershberger @ 2017-06-26 19:40 UTC (permalink / raw)
To: u-boot
Don't wait forever.
Pass errors back to the caller.
Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v2:
- Isolate error propagation changes
drivers/net/ag7xxx.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
index 30771b9..00e6806 100644
--- a/drivers/net/ag7xxx.c
+++ b/drivers/net/ag7xxx.c
@@ -285,18 +285,33 @@ static int ag7xxx_switch_reg_write(struct mii_dev *bus, int reg, u32 val)
return 0;
}
-static u16 ag7xxx_mdio_rw(struct mii_dev *bus, int addr, int reg, u32 val)
+static int ag7xxx_mdio_rw(struct mii_dev *bus, int addr, int reg, u32 val)
{
u32 data;
+ unsigned long start;
+ int ret;
+ /* No idea if this is long enough or too long */
+ int timeout_ms = 1000;
/* Dummy read followed by PHY read/write command. */
- ag7xxx_switch_reg_read(bus, 0x98, &data);
+ ret = ag7xxx_switch_reg_read(bus, 0x98, &data);
+ if (ret < 0)
+ return ret;
data = val | (reg << 16) | (addr << 21) | BIT(30) | BIT(31);
- ag7xxx_switch_reg_write(bus, 0x98, data);
+ ret = ag7xxx_switch_reg_write(bus, 0x98, data);
+ if (ret < 0)
+ return ret;
+
+ start = get_timer(0);
/* Wait for operation to finish */
do {
- ag7xxx_switch_reg_read(bus, 0x98, &data);
+ ret = ag7xxx_switch_reg_read(bus, 0x98, &data);
+ if (ret < 0)
+ return ret;
+
+ if (get_timer(start) > timeout_ms)
+ return -ETIMEDOUT;
} while (data & BIT(31));
return data & 0xffff;
@@ -310,7 +325,11 @@ static int ag7xxx_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
static int ag7xxx_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
u16 val)
{
- ag7xxx_mdio_rw(bus, addr, reg, val);
+ int ret;
+
+ ret = ag7xxx_mdio_rw(bus, addr, reg, val);
+ if (ret < 0)
+ return ret;
return 0;
}
--
1.7.11.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH v2 3/3] net: ag7xxx: No longer ignore link status
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names Joe Hershberger
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 2/3] net: ag7xxx: Propagate errors on phy access Joe Hershberger
@ 2017-06-26 19:40 ` Joe Hershberger
2017-06-27 9:32 ` Marek Vasut
2017-06-27 9:31 ` [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names Marek Vasut
2017-08-07 20:32 ` [U-Boot] " Joe Hershberger
3 siblings, 1 reply; 17+ messages in thread
From: Joe Hershberger @ 2017-06-26 19:40 UTC (permalink / raw)
To: u-boot
In the case of the WAN port, pay attention to the link status.
In the case of LAN ports, stop reading the link status since we don't
care.
Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
This is a pass at improving the code quality.
This has not been tested in any way.
Changes in v2:
- New - Split link status change into its own patch (so it can be dropped if it affects behavior)
drivers/net/ag7xxx.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
index 00e6806..c8352d1 100644
--- a/drivers/net/ag7xxx.c
+++ b/drivers/net/ag7xxx.c
@@ -100,6 +100,12 @@ enum ag7xxx_model {
/* Rx Status */
#define AG7XXX_ETH_DMA_RX_STATUS 0x194
+/* PHY Control Registers */
+
+/* PHY Specific Status Register */
+#define AG7XXX_PHY_PSSR 0x11
+#define AG7XXX_PHY_PSSR_LINK_UP BIT(10)
+
/* Custom register at 0x18070000 */
#define AG7XXX_GMAC_ETH_CFG 0x00
#define AG7XXX_ETH_CFG_SW_PHY_ADDR_SWAP BIT(8)
@@ -758,10 +764,13 @@ static int ag933x_phy_setup_common(struct udevice *dev)
return ret;
/* Read out link status */
- ret = ag7xxx_mdio_read(priv->bus, phymax, 0, MII_MIPSCR);
+ ret = ag7xxx_mdio_read(priv->bus, phymax, 0, AG7XXX_PHY_PSSR);
if (ret < 0)
return ret;
+ if (!(ret & AG7XXX_PHY_PSSR_LINK_UP))
+ return -ENOLINK;
+
return 0;
}
@@ -778,13 +787,6 @@ static int ag933x_phy_setup_common(struct udevice *dev)
return ret;
}
- for (i = 0; i < phymax; i++) {
- /* Read out link status */
- ret = ag7xxx_mdio_read(priv->bus, i, 0, MII_MIPSCR);
- if (ret < 0)
- return ret;
- }
-
return 0;
}
--
1.7.11.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names Joe Hershberger
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 2/3] net: ag7xxx: Propagate errors on phy access Joe Hershberger
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 3/3] net: ag7xxx: No longer ignore link status Joe Hershberger
@ 2017-06-27 9:31 ` Marek Vasut
2017-08-07 20:32 ` [U-Boot] " Joe Hershberger
3 siblings, 0 replies; 17+ messages in thread
From: Marek Vasut @ 2017-06-27 9:31 UTC (permalink / raw)
To: u-boot
On 06/26/2017 09:40 PM, Joe Hershberger wrote:
> The register constants don't use the exact names that are used in the
> TRM, so add comments that use the exact names so that it is clear what
> register is being referred to.
>
> https://www.atheros-drivers.com/qualcomm-atheros-datasheets-for-AR9331.html
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
Acked-by: Marek Vasut <marex@denx.de>
> ---
>
> Changes in v2:
> - New - Comments split into a separate change
>
> drivers/net/ag7xxx.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
> index cf60d11..30771b9 100644
> --- a/drivers/net/ag7xxx.c
> +++ b/drivers/net/ag7xxx.c
> @@ -26,6 +26,7 @@ enum ag7xxx_model {
> AG7XXX_MODEL_AG934X,
> };
>
> +/* MAC Configuration 1 */
> #define AG7XXX_ETH_CFG1 0x00
> #define AG7XXX_ETH_CFG1_SOFT_RST BIT(31)
> #define AG7XXX_ETH_CFG1_RX_RST BIT(19)
> @@ -34,6 +35,7 @@ enum ag7xxx_model {
> #define AG7XXX_ETH_CFG1_RX_EN BIT(2)
> #define AG7XXX_ETH_CFG1_TX_EN BIT(0)
>
> +/* MAC Configuration 2 */
> #define AG7XXX_ETH_CFG2 0x04
> #define AG7XXX_ETH_CFG2_IF_1000 BIT(9)
> #define AG7XXX_ETH_CFG2_IF_10_100 BIT(8)
> @@ -43,26 +45,34 @@ enum ag7xxx_model {
> #define AG7XXX_ETH_CFG2_PAD_CRC_EN BIT(2)
> #define AG7XXX_ETH_CFG2_FDX BIT(0)
>
> +/* MII Configuration */
> #define AG7XXX_ETH_MII_MGMT_CFG 0x20
> #define AG7XXX_ETH_MII_MGMT_CFG_RESET BIT(31)
>
> +/* MII Command */
> #define AG7XXX_ETH_MII_MGMT_CMD 0x24
> #define AG7XXX_ETH_MII_MGMT_CMD_READ 0x1
>
> +/* MII Address */
> #define AG7XXX_ETH_MII_MGMT_ADDRESS 0x28
> #define AG7XXX_ETH_MII_MGMT_ADDRESS_SHIFT 8
>
> +/* MII Control */
> #define AG7XXX_ETH_MII_MGMT_CTRL 0x2c
>
> +/* MII Status */
> #define AG7XXX_ETH_MII_MGMT_STATUS 0x30
>
> +/* MII Indicators */
> #define AG7XXX_ETH_MII_MGMT_IND 0x34
> #define AG7XXX_ETH_MII_MGMT_IND_INVALID BIT(2)
> #define AG7XXX_ETH_MII_MGMT_IND_BUSY BIT(0)
>
> +/* STA Address 1 & 2 */
> #define AG7XXX_ETH_ADDR1 0x40
> #define AG7XXX_ETH_ADDR2 0x44
>
> +/* ETH Configuration 0 - 5 */
> #define AG7XXX_ETH_FIFO_CFG_0 0x48
> #define AG7XXX_ETH_FIFO_CFG_1 0x4c
> #define AG7XXX_ETH_FIFO_CFG_2 0x50
> @@ -70,18 +80,24 @@ enum ag7xxx_model {
> #define AG7XXX_ETH_FIFO_CFG_4 0x58
> #define AG7XXX_ETH_FIFO_CFG_5 0x5c
>
> +/* DMA Transfer Control for Queue 0 */
> #define AG7XXX_ETH_DMA_TX_CTRL 0x180
> #define AG7XXX_ETH_DMA_TX_CTRL_TXE BIT(0)
>
> +/* Descriptor Address for Queue 0 Tx */
> #define AG7XXX_ETH_DMA_TX_DESC 0x184
>
> +/* DMA Tx Status */
> #define AG7XXX_ETH_DMA_TX_STATUS 0x188
>
> +/* Rx Control */
> #define AG7XXX_ETH_DMA_RX_CTRL 0x18c
> #define AG7XXX_ETH_DMA_RX_CTRL_RXE BIT(0)
>
> +/* Pointer to Rx Descriptor */
> #define AG7XXX_ETH_DMA_RX_DESC 0x190
>
> +/* Rx Status */
> #define AG7XXX_ETH_DMA_RX_STATUS 0x194
>
> /* Custom register at 0x18070000 */
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH v2 2/3] net: ag7xxx: Propagate errors on phy access
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 2/3] net: ag7xxx: Propagate errors on phy access Joe Hershberger
@ 2017-06-27 9:32 ` Marek Vasut
2017-08-07 20:32 ` [U-Boot] " Joe Hershberger
1 sibling, 0 replies; 17+ messages in thread
From: Marek Vasut @ 2017-06-27 9:32 UTC (permalink / raw)
To: u-boot
On 06/26/2017 09:40 PM, Joe Hershberger wrote:
> Don't wait forever.
> Pass errors back to the caller.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
Acked-by: Marek Vasut <marex@denx.de>
> ---
>
> Changes in v2:
> - Isolate error propagation changes
>
> drivers/net/ag7xxx.c | 29 ++++++++++++++++++++++++-----
> 1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
> index 30771b9..00e6806 100644
> --- a/drivers/net/ag7xxx.c
> +++ b/drivers/net/ag7xxx.c
> @@ -285,18 +285,33 @@ static int ag7xxx_switch_reg_write(struct mii_dev *bus, int reg, u32 val)
> return 0;
> }
>
> -static u16 ag7xxx_mdio_rw(struct mii_dev *bus, int addr, int reg, u32 val)
> +static int ag7xxx_mdio_rw(struct mii_dev *bus, int addr, int reg, u32 val)
> {
> u32 data;
> + unsigned long start;
> + int ret;
> + /* No idea if this is long enough or too long */
> + int timeout_ms = 1000;
>
> /* Dummy read followed by PHY read/write command. */
> - ag7xxx_switch_reg_read(bus, 0x98, &data);
> + ret = ag7xxx_switch_reg_read(bus, 0x98, &data);
> + if (ret < 0)
> + return ret;
> data = val | (reg << 16) | (addr << 21) | BIT(30) | BIT(31);
> - ag7xxx_switch_reg_write(bus, 0x98, data);
> + ret = ag7xxx_switch_reg_write(bus, 0x98, data);
> + if (ret < 0)
> + return ret;
> +
> + start = get_timer(0);
>
> /* Wait for operation to finish */
> do {
> - ag7xxx_switch_reg_read(bus, 0x98, &data);
> + ret = ag7xxx_switch_reg_read(bus, 0x98, &data);
> + if (ret < 0)
> + return ret;
> +
> + if (get_timer(start) > timeout_ms)
> + return -ETIMEDOUT;
> } while (data & BIT(31));
>
> return data & 0xffff;
> @@ -310,7 +325,11 @@ static int ag7xxx_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
> static int ag7xxx_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
> u16 val)
> {
> - ag7xxx_mdio_rw(bus, addr, reg, val);
> + int ret;
> +
> + ret = ag7xxx_mdio_rw(bus, addr, reg, val);
> + if (ret < 0)
> + return ret;
> return 0;
> }
>
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH v2 3/3] net: ag7xxx: No longer ignore link status
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 3/3] net: ag7xxx: No longer ignore link status Joe Hershberger
@ 2017-06-27 9:32 ` Marek Vasut
2017-06-27 15:46 ` Joe Hershberger
0 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2017-06-27 9:32 UTC (permalink / raw)
To: u-boot
On 06/26/2017 09:40 PM, Joe Hershberger wrote:
> In the case of the WAN port, pay attention to the link status.
> In the case of LAN ports, stop reading the link status since we don't
> care.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
Well, let's see how that works, ev. I'll have to bisect.
btw I hope all this is post-2017.07 material.
> ---
> This is a pass at improving the code quality.
> This has not been tested in any way.
>
> Changes in v2:
> - New - Split link status change into its own patch (so it can be dropped if it affects behavior)
>
> drivers/net/ag7xxx.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
> index 00e6806..c8352d1 100644
> --- a/drivers/net/ag7xxx.c
> +++ b/drivers/net/ag7xxx.c
> @@ -100,6 +100,12 @@ enum ag7xxx_model {
> /* Rx Status */
> #define AG7XXX_ETH_DMA_RX_STATUS 0x194
>
> +/* PHY Control Registers */
> +
> +/* PHY Specific Status Register */
> +#define AG7XXX_PHY_PSSR 0x11
> +#define AG7XXX_PHY_PSSR_LINK_UP BIT(10)
> +
> /* Custom register at 0x18070000 */
> #define AG7XXX_GMAC_ETH_CFG 0x00
> #define AG7XXX_ETH_CFG_SW_PHY_ADDR_SWAP BIT(8)
> @@ -758,10 +764,13 @@ static int ag933x_phy_setup_common(struct udevice *dev)
> return ret;
>
> /* Read out link status */
> - ret = ag7xxx_mdio_read(priv->bus, phymax, 0, MII_MIPSCR);
> + ret = ag7xxx_mdio_read(priv->bus, phymax, 0, AG7XXX_PHY_PSSR);
> if (ret < 0)
> return ret;
>
> + if (!(ret & AG7XXX_PHY_PSSR_LINK_UP))
> + return -ENOLINK;
> +
> return 0;
> }
>
> @@ -778,13 +787,6 @@ static int ag933x_phy_setup_common(struct udevice *dev)
> return ret;
> }
>
> - for (i = 0; i < phymax; i++) {
> - /* Read out link status */
> - ret = ag7xxx_mdio_read(priv->bus, i, 0, MII_MIPSCR);
> - if (ret < 0)
> - return ret;
> - }
> -
> return 0;
> }
>
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH v2 3/3] net: ag7xxx: No longer ignore link status
2017-06-27 9:32 ` Marek Vasut
@ 2017-06-27 15:46 ` Joe Hershberger
2017-06-27 16:10 ` Marek Vasut
0 siblings, 1 reply; 17+ messages in thread
From: Joe Hershberger @ 2017-06-27 15:46 UTC (permalink / raw)
To: u-boot
On Tue, Jun 27, 2017 at 4:32 AM, Marek Vasut <marex@denx.de> wrote:
> On 06/26/2017 09:40 PM, Joe Hershberger wrote:
>> In the case of the WAN port, pay attention to the link status.
>> In the case of LAN ports, stop reading the link status since we don't
>> care.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>
> Well, let's see how that works, ev. I'll have to bisect.
> btw I hope all this is post-2017.07 material.
I'm not intending to apply any of these until you sign off it's functionality.
-Joe
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH v2 3/3] net: ag7xxx: No longer ignore link status
2017-06-27 15:46 ` Joe Hershberger
@ 2017-06-27 16:10 ` Marek Vasut
2017-08-01 16:34 ` Joe Hershberger
0 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2017-06-27 16:10 UTC (permalink / raw)
To: u-boot
On 06/27/2017 05:46 PM, Joe Hershberger wrote:
> On Tue, Jun 27, 2017 at 4:32 AM, Marek Vasut <marex@denx.de> wrote:
>> On 06/26/2017 09:40 PM, Joe Hershberger wrote:
>>> In the case of the WAN port, pay attention to the link status.
>>> In the case of LAN ports, stop reading the link status since we don't
>>> care.
>>>
>>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>>
>> Well, let's see how that works, ev. I'll have to bisect.
>> btw I hope all this is post-2017.07 material.
>
> I'm not intending to apply any of these until you sign off it's functionality.
Hm, maybe Wills can help with that ... CCed
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [RFC PATCH v2 3/3] net: ag7xxx: No longer ignore link status
2017-06-27 16:10 ` Marek Vasut
@ 2017-08-01 16:34 ` Joe Hershberger
0 siblings, 0 replies; 17+ messages in thread
From: Joe Hershberger @ 2017-08-01 16:34 UTC (permalink / raw)
To: u-boot
On Tue, Jun 27, 2017 at 11:10 AM, Marek Vasut <marex@denx.de> wrote:
> On 06/27/2017 05:46 PM, Joe Hershberger wrote:
>> On Tue, Jun 27, 2017 at 4:32 AM, Marek Vasut <marex@denx.de> wrote:
>>> On 06/26/2017 09:40 PM, Joe Hershberger wrote:
>>>> In the case of the WAN port, pay attention to the link status.
>>>> In the case of LAN ports, stop reading the link status since we don't
>>>> care.
>>>>
>>>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>>>
>>> Well, let's see how that works, ev. I'll have to bisect.
>>> btw I hope all this is post-2017.07 material.
>>
>> I'm not intending to apply any of these until you sign off it's functionality.
>
> Hm, maybe Wills can help with that ... CCed
So... still functional?
Thanks,
-Joe
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] net: ag7xxx: Comment register names
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names Joe Hershberger
` (2 preceding siblings ...)
2017-06-27 9:31 ` [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names Marek Vasut
@ 2017-08-07 20:32 ` Joe Hershberger
3 siblings, 0 replies; 17+ messages in thread
From: Joe Hershberger @ 2017-08-07 20:32 UTC (permalink / raw)
To: u-boot
Hi Joe,
https://patchwork.ozlabs.org/patch/780854/ was applied to u-boot-net.git.
Thanks!
-Joe
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] net: ag7xxx: Propagate errors on phy access
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 2/3] net: ag7xxx: Propagate errors on phy access Joe Hershberger
2017-06-27 9:32 ` Marek Vasut
@ 2017-08-07 20:32 ` Joe Hershberger
1 sibling, 0 replies; 17+ messages in thread
From: Joe Hershberger @ 2017-08-07 20:32 UTC (permalink / raw)
To: u-boot
Hi Joe,
https://patchwork.ozlabs.org/patch/780856/ was applied to u-boot-net.git.
Thanks!
-Joe
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-08-07 20:32 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-12 20:20 [U-Boot] [RFC PATCH] net: ag7xxx: Clean up some issues with phy access Joe Hershberger
2017-06-13 9:24 ` Marek Vasut
2017-06-13 16:28 ` Joe Hershberger
2017-06-19 9:37 ` Marek Vasut
2017-06-19 15:55 ` Joe Hershberger
2017-06-20 9:25 ` Marek Vasut
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names Joe Hershberger
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 2/3] net: ag7xxx: Propagate errors on phy access Joe Hershberger
2017-06-27 9:32 ` Marek Vasut
2017-08-07 20:32 ` [U-Boot] " Joe Hershberger
2017-06-26 19:40 ` [U-Boot] [RFC PATCH v2 3/3] net: ag7xxx: No longer ignore link status Joe Hershberger
2017-06-27 9:32 ` Marek Vasut
2017-06-27 15:46 ` Joe Hershberger
2017-06-27 16:10 ` Marek Vasut
2017-08-01 16:34 ` Joe Hershberger
2017-06-27 9:31 ` [U-Boot] [RFC PATCH v2 1/3] net: ag7xxx: Comment register names Marek Vasut
2017-08-07 20:32 ` [U-Boot] " Joe Hershberger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.