Add config_init for LAN8814. This function is required for the following
reasons:
- we need to make sure that the PHY is reset,
- disable ANEG with QSGMII PCS Host side
- swap the MDI-X A,B transmit so that there will not be any link flip-flaps
when the PHY gets a link.
Signed-off-by: Horatiu Vultur <[email protected]>
---
drivers/net/phy/micrel.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 44a24b99c894..f080312032cf 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1565,6 +1565,14 @@ static int ksz886x_cable_test_get_status(struct phy_device *phydev,
#define LAN_EXT_PAGE_ACCESS_ADDRESS_DATA 0x17
#define LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC 0x4000
+#define LAN8814_QSGMII_SOFT_RESET 0x43
+#define LAN8814_QSGMII_SOFT_RESET_BIT BIT(0)
+#define LAN8814_QSGMII_PCS1G_ANEG_CONFIG 0x13
+#define LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA BIT(3)
+#define LAN8814_ALIGN_SWAP 0x4a
+#define LAN8814_ALIGN_TX_A_B_SWAP 0x1
+#define LAN8814_ALIGN_TX_A_B_SWAP_MASK GENMASK(2, 0)
+
#define LAN8804_ALIGN_SWAP 0x4a
#define LAN8804_ALIGN_TX_A_B_SWAP 0x1
#define LAN8804_ALIGN_TX_A_B_SWAP_MASK GENMASK(2, 0)
@@ -1601,6 +1609,29 @@ static int lanphy_write_page_reg(struct phy_device *phydev, int page, u16 addr,
return 0;
}
+static int lan8814_config_init(struct phy_device *phydev)
+{
+ int val;
+
+ /* Reset the PHY */
+ val = lanphy_read_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET);
+ val |= LAN8814_QSGMII_SOFT_RESET_BIT;
+ lanphy_write_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET, val);
+
+ /* Disable ANEG with QSGMII PCS Host side */
+ val = lanphy_read_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG);
+ val &= ~LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA;
+ lanphy_write_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG, val);
+
+ /* MDI-X setting for swap A,B transmit */
+ val = lanphy_read_page_reg(phydev, 2, LAN8814_ALIGN_SWAP);
+ val &= ~LAN8814_ALIGN_TX_A_B_SWAP_MASK;
+ val |= LAN8814_ALIGN_TX_A_B_SWAP;
+ lanphy_write_page_reg(phydev, 2, LAN8814_ALIGN_SWAP, val);
+
+ return 0;
+}
+
static int lan8804_config_init(struct phy_device *phydev)
{
int val;
@@ -1793,6 +1824,7 @@ static struct phy_driver ksphy_driver[] = {
.phy_id = PHY_ID_LAN8814,
.phy_id_mask = MICREL_PHY_ID_MASK,
.name = "Microchip INDY Gigabit Quad PHY",
+ .config_init = lan8814_config_init,
.driver_data = &ksz9021_type,
.probe = kszphy_probe,
.soft_reset = genphy_soft_reset,
--
2.33.0
On 26.11.2021 11:38, Horatiu Vultur wrote:
> Add config_init for LAN8814. This function is required for the following
> reasons:
> - we need to make sure that the PHY is reset,
> - disable ANEG with QSGMII PCS Host side
> - swap the MDI-X A,B transmit so that there will not be any link flip-flaps
> when the PHY gets a link.
>
> Signed-off-by: Horatiu Vultur <[email protected]>
> ---
> drivers/net/phy/micrel.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 44a24b99c894..f080312032cf 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -1565,6 +1565,14 @@ static int ksz886x_cable_test_get_status(struct phy_device *phydev,
> #define LAN_EXT_PAGE_ACCESS_ADDRESS_DATA 0x17
> #define LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC 0x4000
>
> +#define LAN8814_QSGMII_SOFT_RESET 0x43
> +#define LAN8814_QSGMII_SOFT_RESET_BIT BIT(0)
> +#define LAN8814_QSGMII_PCS1G_ANEG_CONFIG 0x13
> +#define LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA BIT(3)
> +#define LAN8814_ALIGN_SWAP 0x4a
> +#define LAN8814_ALIGN_TX_A_B_SWAP 0x1
> +#define LAN8814_ALIGN_TX_A_B_SWAP_MASK GENMASK(2, 0)
> +
> #define LAN8804_ALIGN_SWAP 0x4a
> #define LAN8804_ALIGN_TX_A_B_SWAP 0x1
> #define LAN8804_ALIGN_TX_A_B_SWAP_MASK GENMASK(2, 0)
> @@ -1601,6 +1609,29 @@ static int lanphy_write_page_reg(struct phy_device *phydev, int page, u16 addr,
> return 0;
> }
>
> +static int lan8814_config_init(struct phy_device *phydev)
> +{
> + int val;
> +
> + /* Reset the PHY */
> + val = lanphy_read_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET);
> + val |= LAN8814_QSGMII_SOFT_RESET_BIT;
> + lanphy_write_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET, val);
> +
> + /* Disable ANEG with QSGMII PCS Host side */
> + val = lanphy_read_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG);
> + val &= ~LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA;
> + lanphy_write_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG, val);
> +
> + /* MDI-X setting for swap A,B transmit */
> + val = lanphy_read_page_reg(phydev, 2, LAN8814_ALIGN_SWAP);
> + val &= ~LAN8814_ALIGN_TX_A_B_SWAP_MASK;
> + val |= LAN8814_ALIGN_TX_A_B_SWAP;
> + lanphy_write_page_reg(phydev, 2, LAN8814_ALIGN_SWAP, val);
> +
Not directly related to just this patch:
Did you consider implementing the read_page and write_page PHY driver
callbacks? Then you could use phylib functions like phy_modify_paged et al
and you wouldn't have to open-code the paged register operations.
I think write_page would just be
phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, page);
phy_write(phydev, LAN_EXT_PAGE_ACCESS_ADDRESS_DATA, addr);
phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, (page | LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC));
and read_page
phy_read(phydev, LAN_EXT_PAGE_ACCESS_CONTROL);
> + return 0;
> +}
> +
> static int lan8804_config_init(struct phy_device *phydev)
> {
> int val;
> @@ -1793,6 +1824,7 @@ static struct phy_driver ksphy_driver[] = {
> .phy_id = PHY_ID_LAN8814,
> .phy_id_mask = MICREL_PHY_ID_MASK,
> .name = "Microchip INDY Gigabit Quad PHY",
> + .config_init = lan8814_config_init,
> .driver_data = &ksz9021_type,
> .probe = kszphy_probe,
> .soft_reset = genphy_soft_reset,
>
On Fri, Nov 26, 2021 at 12:57:33PM +0100, Heiner Kallweit wrote:
> Not directly related to just this patch:
> Did you consider implementing the read_page and write_page PHY driver
> callbacks? Then you could use phylib functions like phy_modify_paged et al
> and you wouldn't have to open-code the paged register operations.
>
> I think write_page would just be
> phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, page);
> phy_write(phydev, LAN_EXT_PAGE_ACCESS_ADDRESS_DATA, addr);
> phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, (page | LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC));
>
> and read_page
> phy_read(phydev, LAN_EXT_PAGE_ACCESS_CONTROL);
Remember that read_page() and write_page() must be implemented using
the unlocked accessors since the MDIO bus lock is held prior to calling
them. So these should be __phy_write() and __phy_read().
The use of the helpers you mention above also bring greater safety to
the read-modify-write accesses, since with these accessors, the whole
set of accesses are done while holding the bus lock.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> - swap the MDI-X A,B transmit so that there will not be any link flip-flaps
> when the PHY gets a link.
Isn't this a board issue, rather than generic? Or does the datasheet
have the pins labelled wrongly and all boards following the datasheet
are wrong?
Andrew
The 11/26/2021 12:57, Heiner Kallweit wrote:
Hi Heiner,
>
> On 26.11.2021 11:38, Horatiu Vultur wrote:
> >
> > +static int lan8814_config_init(struct phy_device *phydev)
> > +{
> > + int val;
> > +
> > + /* Reset the PHY */
> > + val = lanphy_read_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET);
> > + val |= LAN8814_QSGMII_SOFT_RESET_BIT;
> > + lanphy_write_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET, val);
> > +
> > + /* Disable ANEG with QSGMII PCS Host side */
> > + val = lanphy_read_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG);
> > + val &= ~LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA;
> > + lanphy_write_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG, val);
> > +
> > + /* MDI-X setting for swap A,B transmit */
> > + val = lanphy_read_page_reg(phydev, 2, LAN8814_ALIGN_SWAP);
> > + val &= ~LAN8814_ALIGN_TX_A_B_SWAP_MASK;
> > + val |= LAN8814_ALIGN_TX_A_B_SWAP;
> > + lanphy_write_page_reg(phydev, 2, LAN8814_ALIGN_SWAP, val);
> > +
>
> Not directly related to just this patch:
> Did you consider implementing the read_page and write_page PHY driver
> callbacks? Then you could use phylib functions like phy_modify_paged et al
> and you wouldn't have to open-code the paged register operations.
>
> I think write_page would just be
> phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, page);
> phy_write(phydev, LAN_EXT_PAGE_ACCESS_ADDRESS_DATA, addr);
> phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, (page | LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC));
>
> and read_page
> phy_read(phydev, LAN_EXT_PAGE_ACCESS_CONTROL);
Thanks for the suggestion, but unfortunately it would not work.
The reason is that in the callback 'write_page' I don't actually get
also the address in the page that is needed to be read/write.
If this issue would be fixed, then there is another problem.
To read/write the data in the extended page is required to access the
register LAN_EXT_PAGE_ACCESS_ADDRESS_DATA. But that would not happen
when using the phy_read_paged, it would read actually the register in
page 0.
If I have missed something, please let me know.
>
> > + return 0;
> > +}
> > +
> > static int lan8804_config_init(struct phy_device *phydev)
> > {
> > int val;
> > @@ -1793,6 +1824,7 @@ static struct phy_driver ksphy_driver[] = {
> > .phy_id = PHY_ID_LAN8814,
> > .phy_id_mask = MICREL_PHY_ID_MASK,
> > .name = "Microchip INDY Gigabit Quad PHY",
> > + .config_init = lan8814_config_init,
> > .driver_data = &ksz9021_type,
> > .probe = kszphy_probe,
> > .soft_reset = genphy_soft_reset,
> >
>
--
/Horatiu
The 11/26/2021 16:15, Andrew Lunn wrote:
Hi Andrew,
>
> > - swap the MDI-X A,B transmit so that there will not be any link flip-flaps
> > when the PHY gets a link.
>
> Isn't this a board issue, rather than generic? Or does the datasheet
> have the pins labelled wrongly and all boards following the datasheet
> are wrong?
From my understanding it is not a board issue. This is needed in case
the link partners can't do the A/B swapping based on MDI/MDIX. It would
not harm if this is set also for link partners that can do this.
>
> Andrew
--
/Horatiu
On 29.11.2021 09:29, Horatiu Vultur wrote:
> The 11/26/2021 12:57, Heiner Kallweit wrote:
>
> Hi Heiner,
>
>>
>> On 26.11.2021 11:38, Horatiu Vultur wrote:
>>>
>>> +static int lan8814_config_init(struct phy_device *phydev)
>>> +{
>>> + int val;
>>> +
>>> + /* Reset the PHY */
>>> + val = lanphy_read_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET);
>>> + val |= LAN8814_QSGMII_SOFT_RESET_BIT;
>>> + lanphy_write_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET, val);
>>> +
>>> + /* Disable ANEG with QSGMII PCS Host side */
>>> + val = lanphy_read_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG);
>>> + val &= ~LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA;
>>> + lanphy_write_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG, val);
>>> +
>>> + /* MDI-X setting for swap A,B transmit */
>>> + val = lanphy_read_page_reg(phydev, 2, LAN8814_ALIGN_SWAP);
>>> + val &= ~LAN8814_ALIGN_TX_A_B_SWAP_MASK;
>>> + val |= LAN8814_ALIGN_TX_A_B_SWAP;
>>> + lanphy_write_page_reg(phydev, 2, LAN8814_ALIGN_SWAP, val);
>>> +
>>
>> Not directly related to just this patch:
>> Did you consider implementing the read_page and write_page PHY driver
>> callbacks? Then you could use phylib functions like phy_modify_paged et al
>> and you wouldn't have to open-code the paged register operations.
>>
>> I think write_page would just be
>> phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, page);
>> phy_write(phydev, LAN_EXT_PAGE_ACCESS_ADDRESS_DATA, addr);
>> phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, (page | LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC));
>>
>> and read_page
>> phy_read(phydev, LAN_EXT_PAGE_ACCESS_CONTROL);
>
> Thanks for the suggestion, but unfortunately it would not work.
> The reason is that in the callback 'write_page' I don't actually get
> also the address in the page that is needed to be read/write.
>
> If this issue would be fixed, then there is another problem.
> To read/write the data in the extended page is required to access the
> register LAN_EXT_PAGE_ACCESS_ADDRESS_DATA. But that would not happen
> when using the phy_read_paged, it would read actually the register in
> page 0.
>
> If I have missed something, please let me know.
>
Right, after reading the sequence in lanphy_read_page_reg() more
carefully I agree. This PHY re-uses the more complex MMD access
mechanism for paged access.
>>
>>> + return 0;
>>> +}
>>> +
>>> static int lan8804_config_init(struct phy_device *phydev)
>>> {
>>> int val;
>>> @@ -1793,6 +1824,7 @@ static struct phy_driver ksphy_driver[] = {
>>> .phy_id = PHY_ID_LAN8814,
>>> .phy_id_mask = MICREL_PHY_ID_MASK,
>>> .name = "Microchip INDY Gigabit Quad PHY",
>>> + .config_init = lan8814_config_init,
>>> .driver_data = &ksz9021_type,
>>> .probe = kszphy_probe,
>>> .soft_reset = genphy_soft_reset,
>>>
>>
>
The 11/26/2021 11:38, Horatiu Vultur wrote:
Hi,
Sorry for reviving this old thread. I can see this patch was marked as
"Changes Requested" [1]. The change that Heiner proposed, will not worked as
we already discussed. It is using a different mechanism to access extend
pages.
Should I just try to resend the patch or is possible to get this one?
[1] https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
--
/Horatiu
On Wed, 22 Dec 2021 12:48:20 +0100 Horatiu Vultur wrote:
> The 11/26/2021 11:38, Horatiu Vultur wrote:
>
> Sorry for reviving this old thread. I can see this patch was marked as
> "Changes Requested" [1]. The change that Heiner proposed, will not worked as
> we already discussed. It is using a different mechanism to access extend
> pages.
> Should I just try to resend the patch or is possible to get this one?
>
> [1] https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
Please resend (preferably with a short note on why in the change log).