[PATCH v1] net: phy: at803x: add ar9331 support

[PATCH v1] net: phy: at803x: add ar9331 support

[Oleksij Rempel]

Mostly this hardware can work with generic PHY driver, but this change
is needed to provided interrupt handling support.
Tested with dsa ar9331-switch driver.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/at803x.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 6ad8b1c63c34..d62a77adb8e7 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -53,6 +53,7 @@
 #define AT803X_DEBUG_REG_5			0x05
 #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
 
+#define AR9331_PHY_ID 0x004dd041
 #define ATH8030_PHY_ID 0x004dd076
 #define ATH8031_PHY_ID 0x004dd074
 #define ATH8035_PHY_ID 0x004dd072
@@ -406,11 +407,24 @@ static struct phy_driver at803x_driver[] = {
 	.aneg_done		= at803x_aneg_done,
 	.ack_interrupt		= &at803x_ack_interrupt,
 	.config_intr		= &at803x_config_intr,
+}, {
+	/* ATHEROS AR9331 */
+	.phy_id			= AR9331_PHY_ID,
+	.name			= "Atheros AR9331 built-in PHY",
+	.phy_id_mask		= AT803X_PHY_ID_MASK,
+	.probe			= at803x_probe,
+	.config_init		= at803x_config_init,
+	.suspend		= at803x_suspend,
+	.resume			= at803x_resume,
+	/* PHY_BASIC_FEATURES */
+	.ack_interrupt		= &at803x_ack_interrupt,
+	.config_intr		= &at803x_config_intr,
 } };
 
 module_phy_driver(at803x_driver);
 
 static struct mdio_device_id __maybe_unused atheros_tbl[] = {
+	{ AR9331_PHY_ID, AT803X_PHY_ID_MASK },
 	{ ATH8030_PHY_ID, AT803X_PHY_ID_MASK },
 	{ ATH8031_PHY_ID, AT803X_PHY_ID_MASK },
 	{ ATH8035_PHY_ID, AT803X_PHY_ID_MASK },
-- 
2.23.0

Re: [PATCH v1] net: phy: at803x: add ar9331 support

[Andrew Lunn]

On Mon, Sep 30, 2019 at 11:27:10AM +0200, Oleksij Rempel wrote:
> Mostly this hardware can work with generic PHY driver, but this change
> is needed to provided interrupt handling support.
> Tested with dsa ar9331-switch driver.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/phy/at803x.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 6ad8b1c63c34..d62a77adb8e7 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -53,6 +53,7 @@
>  #define AT803X_DEBUG_REG_5			0x05
>  #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
>  
> +#define AR9331_PHY_ID 0x004dd041
>  #define ATH8030_PHY_ID 0x004dd076
>  #define ATH8031_PHY_ID 0x004dd074
>  #define ATH8035_PHY_ID 0x004dd072

Hi Oleksij

I wonder if we should call this ATH9331_PHY_ID, to keep with the
naming convention? Why did you choose AR, not ATH?

Thanks
	Andrew

Re: [PATCH v1] net: phy: at803x: add ar9331 support

[Oleksij Rempel]

On Mon, Sep 30, 2019 at 03:20:41PM +0200, Andrew Lunn wrote:
> On Mon, Sep 30, 2019 at 11:27:10AM +0200, Oleksij Rempel wrote:
> > Mostly this hardware can work with generic PHY driver, but this change
> > is needed to provided interrupt handling support.
> > Tested with dsa ar9331-switch driver.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  drivers/net/phy/at803x.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> > index 6ad8b1c63c34..d62a77adb8e7 100644
> > --- a/drivers/net/phy/at803x.c
> > +++ b/drivers/net/phy/at803x.c
> > @@ -53,6 +53,7 @@
> >  #define AT803X_DEBUG_REG_5			0x05
> >  #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
> >  
> > +#define AR9331_PHY_ID 0x004dd041
> >  #define ATH8030_PHY_ID 0x004dd076
> >  #define ATH8031_PHY_ID 0x004dd074
> >  #define ATH8035_PHY_ID 0x004dd072
> 
> Hi Oleksij
> 
> I wonder if we should call this ATH9331_PHY_ID, to keep with the
> naming convention? Why did you choose AR, not ATH?

Hi Andrew,

i have no strong opinion here. Taking what ever name was on the SoC
package, was safe enough for me. If you say, it should be ATH9331, i'll
rename it. Should i?

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v1] net: phy: at803x: add ar9331 support

[Heiner Kallweit]

On 30.09.2019 11:27, Oleksij Rempel wrote:
> Mostly this hardware can work with generic PHY driver, but this change
> is needed to provided interrupt handling support.
> Tested with dsa ar9331-switch driver.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/phy/at803x.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 6ad8b1c63c34..d62a77adb8e7 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -53,6 +53,7 @@
>  #define AT803X_DEBUG_REG_5			0x05
>  #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
>  
> +#define AR9331_PHY_ID 0x004dd041
>  #define ATH8030_PHY_ID 0x004dd076
>  #define ATH8031_PHY_ID 0x004dd074
>  #define ATH8035_PHY_ID 0x004dd072
> @@ -406,11 +407,24 @@ static struct phy_driver at803x_driver[] = {
>  	.aneg_done		= at803x_aneg_done,
>  	.ack_interrupt		= &at803x_ack_interrupt,
>  	.config_intr		= &at803x_config_intr,
> +}, {
> +	/* ATHEROS AR9331 */
> +	.phy_id			= AR9331_PHY_ID,
> +	.name			= "Atheros AR9331 built-in PHY",
> +	.phy_id_mask		= AT803X_PHY_ID_MASK,

The ID mask of 0xffffffef is quite strange, it ignores the last
bit of the model number and requires exact match of the revision
number. Unfortunately the original commit doesn't explain why
this mask was chosen. It would only make sense if there are
functionally identical PHY's with e.g. id 0x004dd051.
If in doubt I'd suggest you use macro PHY_ID_MATCH_EXACT.

> +	.probe			= at803x_probe,

After 5c5f626bcace ("net: phy: improve handling link_change_notify
callback") struct at803x_priv isn't used any longer and the probe
callback could be removed. I didn't do that as part of this commit
because I could compile-test the change only.

> +	.config_init		= at803x_config_init,
> +	.suspend		= at803x_suspend,
> +	.resume			= at803x_resume,
> +	/* PHY_BASIC_FEATURES */
> +	.ack_interrupt		= &at803x_ack_interrupt,
> +	.config_intr		= &at803x_config_intr,
>  } };
>  
>  module_phy_driver(at803x_driver);
>  
>  static struct mdio_device_id __maybe_unused atheros_tbl[] = {
> +	{ AR9331_PHY_ID, AT803X_PHY_ID_MASK },

See comment above regarding the id mask.

>  	{ ATH8030_PHY_ID, AT803X_PHY_ID_MASK },
>  	{ ATH8031_PHY_ID, AT803X_PHY_ID_MASK },
>  	{ ATH8035_PHY_ID, AT803X_PHY_ID_MASK },
> 
Heiner