Re: [PATCH] phy: nxp-c45: add driver for tja1103
> nxp-c45-tja11xx is acceptable from my point of view. Great. Enough bike shedding, nxp-c45-tja11xx it is. Andrew
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On 4/13/2021 3:57 PM, Andrew Lunn wrote: Ok, we can agree that there will not be a perfect naming. Would it be a possibility to rename the existing TJA11xx driver to TJA1100-1-2 or is that unwanted? It is generally a bad idea. It makes back porting fixing harder if the file changes name. If nxp-c45.c is to generic (I take from your comments that' your conclusion), we could at least lean towards nxp-c45-bt1.c? Unfortunately, the product naming schemes are not sufficiently methodical to have a a good driver name based on product names. And what does bt1 stand for? How about nxp-c45-tja11xx.c. It is not ideal, but it does at least give an indication of what devices it does cover, even if there is a big overlap with nxp-tja11xx.c, in terms of pattern matching. And if you do decide to have a major change of registers, your can call the device tja1201 and have a new driver nxp-c45-tja12xx. Andrew bt1 standing for BASE-T1. As you can see from the current situation, it could well happen that a future PHY is SW incompatible (right now I would say it is unlikely, but ok), and the device is still a TJA11xx. nxp-c45-tja11xx is acceptable from my point of view.
Re: [PATCH] phy: nxp-c45: add driver for tja1103
> Ok, we can agree that there will not be a perfect naming. Would it be a > possibility to rename the existing TJA11xx driver to TJA1100-1-2 or is that > unwanted? It is generally a bad idea. It makes back porting fixing harder if the file changes name. > If nxp-c45.c is to generic (I take from your comments that' your > conclusion), we could at least lean towards nxp-c45-bt1.c? Unfortunately, > the product naming schemes are not sufficiently methodical to have a a good > driver name based on product names. And what does bt1 stand for? How about nxp-c45-tja11xx.c. It is not ideal, but it does at least give an indication of what devices it does cover, even if there is a big overlap with nxp-tja11xx.c, in terms of pattern matching. And if you do decide to have a major change of registers, your can call the device tja1201 and have a new driver nxp-c45-tja12xx. Andrew
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Mon, 2021-04-12 at 10:50 +0100, Russell King - ARM Linux admin
wrote:
> On Fri, Apr 09, 2021 at 09:41:06PM +0300, Radu Pirea (NXP OSS) wrote:
> > +#define B100T1_PMAPMD_CTL 0x0834
> > +#define B100T1_PMAPMD_CONFIG_ENBIT(15)
> > +#define B100T1_PMAPMD_MASTER BIT(14)
> > +#define MASTER_MODE(B100T1_PMAPMD_CONFIG_EN |
> > B100T1_PMAPMD_MASTER)
> > +#define SLAVE_MODE (B100T1_PMAPMD_CONFIG_EN)
> > +
> > +#define DEVICE_CONTROL 0x0040
> > +#define DEVICE_CONTROL_RESET BIT(15)
> > +#define DEVICE_CONTROL_CONFIG_GLOBAL_ENBIT(14)
> > +#define DEVICE_CONTROL_CONFIG_ALL_EN BIT(13)
> > +#define RESET_POLL_NS (250 * NSEC_PER_MSEC)
> > +
> > +#define PHY_CONTROL0x8100
> > +#define PHY_CONFIG_EN BIT(14)
> > +#define PHY_START_OP BIT(0)
> > +
> > +#define PHY_CONFIG 0x8108
> > +#define PHY_CONFIG_AUTOBIT(0)
> > +
> > +#define SIGNAL_QUALITY 0x8320
> > +#define SQI_VALID BIT(14)
> > +#define SQI_MASK GENMASK(2, 0)
> > +#define MAX_SQISQI_MASK
> > +
> > +#define CABLE_TEST 0x8330
> > +#define CABLE_TEST_ENABLE BIT(15)
> > +#define CABLE_TEST_START BIT(14)
> > +#define CABLE_TEST_VALID BIT(13)
> > +#define CABLE_TEST_OK 0x00
> > +#define CABLE_TEST_SHORTED 0x01
> > +#define CABLE_TEST_OPEN0x02
> > +#define CABLE_TEST_UNKNOWN 0x07
> > +
> > +#define PORT_CONTROL 0x8040
> > +#define PORT_CONTROL_ENBIT(14)
> > +
> > +#define PORT_INFRA_CONTROL 0xAC00
> > +#define PORT_INFRA_CONTROL_EN BIT(14)
> > +
> > +#define VND1_RXID 0xAFCC
> > +#define VND1_TXID 0xAFCD
> > +#define ID_ENABLE BIT(15)
> > +
> > +#define ABILITIES 0xAFC4
> > +#define RGMII_ID_ABILITY BIT(15)
> > +#define RGMII_ABILITY BIT(14)
> > +#define RMII_ABILITY BIT(10)
> > +#define REVMII_ABILITY BIT(9)
> > +#define MII_ABILITYBIT(8)
> > +#define SGMII_ABILITY BIT(0)
> > +
> > +#define MII_BASIC_CONFIG 0xAFC6
> > +#define MII_BASIC_CONFIG_REV BIT(8)
> > +#define MII_BASIC_CONFIG_SGMII 0x9
> > +#define MII_BASIC_CONFIG_RGMII 0x7
> > +#define MII_BASIC_CONFIG_RMII 0x5
> > +#define MII_BASIC_CONFIG_MII 0x4
> > +
> > +#define SYMBOL_ERROR_COUNTER 0x8350
> > +#define LINK_DROP_COUNTER 0x8352
> > +#define LINK_LOSSES_AND_FAILURES 0x8353
> > +#define R_GOOD_FRAME_CNT 0xA950
> > +#define R_BAD_FRAME_CNT0xA952
> > +#define R_RXER_FRAME_CNT 0xA954
> > +#define RX_PREAMBLE_COUNT 0xAFCE
> > +#define TX_PREAMBLE_COUNT 0xAFCF
> > +#define RX_IPG_LENGTH 0xAFD0
> > +#define TX_IPG_LENGTH 0xAFD1
> > +#define COUNTERS_ENBIT(15)
> > +
> > +#define CLK_25MHZ_PS_PERIOD4UL
> > +#define PS_PER_DEGREE (CLK_25MHZ_PS_PERIOD / 360)
> > +#define MIN_ID_PS 8222U
> > +#define MAX_ID_PS 11300U
>
> Maybe include some prefix as to which MMD each of these registers is
> located?
I will add the MMD as prefix. Thank you.
>
> > +static bool nxp_c45_can_sleep(struct phy_device *phydev)
> > +{
> > + int reg;
> > +
> > + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT1);
> > + if (reg < 0)
> > + return false;
> > +
> > + return !!(reg & MDIO_STAT1_LPOWERABLE);
> > +}
>
> This looks like it could be useful as a generic helper function -
> nothing in this function is specific to this PHY.
>
> > +static int nxp_c45_resume(struct phy_device *phydev)
> > +{
> > + int reg;
> > +
> > + if (!nxp_c45_can_sleep(phydev))
> > + return -EOPNOTSUPP;
> > +
> > + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
> > + reg &= ~MDIO_CTRL1_LPOWER;
> > + phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, reg);
> > +
> > + return 0;
> > +}
> > +
> > +static int nxp_c45_suspend(struct phy_device *phydev)
> > +{
> > + int reg;
> > +
> > + if (!nxp_c45_can_sleep(phydev))
> > + return -EOPNOTSUPP;
> > +
> > + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
> > + reg |= MDIO_CTRL1_LPOWER;
> > + phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, reg);
> > +
> > + return 0;
> > +}
>
> These too look like potential generic helper functions.
That's true.
Should I implement them as genphy_c45_pma_sus
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On 4/13/2021 3:30 PM, Andrew Lunn wrote: On Tue, Apr 13, 2021 at 08:56:30AM +0200, Christian Herber wrote: Hi Andrew, On 4/12/2021 6:52 PM, Andrew Lunn wrote: So what you are say is, you don't care if the IP is completely different, it all goes in one driver. So lets put this driver into nxp-tja11xx.c. And then we avoid all the naming issues. Andrew As this seems to be a key question, let me try and shed some more light on this. The original series of BASE-T1 PHYs includes TJA110, TJA1101, and TJA1102. They are covered by the existing driver, which has the unfortunate naming TJA11xx. Unfortunate, because the use of wildcards is a bit to generous. Yes, that does happen. Naming is difficult. But i really think nxp-c45.c is much worse. It gives no idea at all what it supports. Or in the future, what it does not support, and you actually need nxp-c45-ng.c. Developers are going to look at a board, see a tja1XYZ chip, see the nxp-tja11xx.c and enable it. Does the chip have a big C45 symbol on it? Anything to give the idea it should use the nxp-c45 driver? Maybe we should actually swing the complete opposite direction. Name it npx-tja1103.c. There are lots of drivers which have a specific name, but actually support a lot more devices. The developer sees they have an tja1XYZ, there are two drivers which look about right, and enable them both? Andrew Ok, we can agree that there will not be a perfect naming. Would it be a possibility to rename the existing TJA11xx driver to TJA1100-1-2 or is that unwanted? I agree that it should be easy to find the right driver. Right now, there is another device called the SJA1110, which has a very similar IP integrated. It would be possible to use the driver for that device also, even if this is outside of the scope of this submission. Going for wildcards, we get to xJA11xx, which is really undesirable to me. In the end my hope was that people will look up the correct driver through LKDDb or similar and will find the matching devices from there. I am open to any suggestion that leads to users finding the right driver and that also work for future devices. Using your example of an NG device: My assumption is that the things that change are covered by IEEE standardized registers, and thus should be implemented as part of generic helper functions. The things that are outside of the IEEE scope, e.g xMII interface configuration are generic and can be contained in a single driver if the registers are kept software compatible which we intend to do. If nxp-c45.c is to generic (I take from your comments that' your conclusion), we could at least lean towards nxp-c45-bt1.c? Unfortunately, the product naming schemes are not sufficiently methodical to have a a good driver name based on product names. Christian
Re: Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Tue, Apr 13, 2021 at 08:56:30AM +0200, Christian Herber wrote: > Hi Andrew, > > On 4/12/2021 6:52 PM, Andrew Lunn wrote: > > > > So what you are say is, you don't care if the IP is completely > > different, it all goes in one driver. So lets put this driver into > > nxp-tja11xx.c. And then we avoid all the naming issues. > > > > Andrew > > > > As this seems to be a key question, let me try and shed some more light on > this. > The original series of BASE-T1 PHYs includes TJA110, TJA1101, and TJA1102. > They are covered by the existing driver, which has the unfortunate naming > TJA11xx. Unfortunate, because the use of wildcards is a bit to generous. Yes, that does happen. Naming is difficult. But i really think nxp-c45.c is much worse. It gives no idea at all what it supports. Or in the future, what it does not support, and you actually need nxp-c45-ng.c. Developers are going to look at a board, see a tja1XYZ chip, see the nxp-tja11xx.c and enable it. Does the chip have a big C45 symbol on it? Anything to give the idea it should use the nxp-c45 driver? Maybe we should actually swing the complete opposite direction. Name it npx-tja1103.c. There are lots of drivers which have a specific name, but actually support a lot more devices. The developer sees they have an tja1XYZ, there are two drivers which look about right, and enable them both? Andrew
Re: Re: [PATCH] phy: nxp-c45: add driver for tja1103
Hi Andrew, On 4/12/2021 6:52 PM, Andrew Lunn wrote: So what you are say is, you don't care if the IP is completely different, it all goes in one driver. So lets put this driver into nxp-tja11xx.c. And then we avoid all the naming issues. Andrew As this seems to be a key question, let me try and shed some more light on this. The original series of BASE-T1 PHYs includes TJA110, TJA1101, and TJA1102. They are covered by the existing driver, which has the unfortunate naming TJA11xx. Unfortunate, because the use of wildcards is a bit to generous. E.g. the naming would also include a TJA1145, which is a high-speed CAN transceiver. The truth is, extrapolating wildcards in product names doesn't work as there is not guarantee of future product names. The mentioned TJA1100/1/2 are *fairly* software-compatible, which is why it makes sense to have a shared driver. When it gets to TJA1103, there is no SW compatibility, which is why we decided to create a new driver. We want to support all future Ethernet PHY devices with this codebase, and that is why the naming is that generic. The common denominator of the devices is that they are NXP products and use clause 45 addressing. When you say we don't care that the IP is different, that doesn't quite fit. Just because the MDI is different, the register map does not need to change much, so it will be easy to support future PHYs also when using different PHY technology. Moving the code into TJA11xx is creating more issues, as it assumes that the devices which are managed by the driver are always TJA... devices which may not be true. Christian
Re: [PATCH] phy: nxp-c45: add driver for tja1103
> +static const struct nxp_c45_phy_stats nxp_c45_hw_stats[] = {
> + { "phy_symbol_error_cnt", MDIO_MMD_VEND1, SYMBOL_ERROR_COUNTER, 0,
> GENMASK(15, 0) },
> + { "phy_link_status_drop_cnt", MDIO_MMD_VEND1, LINK_DROP_COUNTER, 8,
> GENMASK(13, 8) },
> + { "phy_link_availability_drop_cnt", MDIO_MMD_VEND1, LINK_DROP_COUNTER,
> 0, GENMASK(5, 0) },
netdev tries to keep with the old 80 character limit. Please wrap the
long lines.
> +static void nxp_c45_set_delays(struct phy_device *phydev)
> +{
> + struct nxp_c45_phy *priv = phydev->priv;
> + u64 tx_delay = priv->tx_delay;
> + u64 rx_delay = priv->rx_delay;
> + u64 degree;
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> + degree = tx_delay / PS_PER_DEGREE;
> + phy_write_mmd(phydev, MDIO_MMD_VEND1, VND1_TXID,
> + ID_ENABLE | nxp_c45_get_phase_shift(degree));
> + }
You are missing an else clause. You need to ensure the delay is 0 if
delays are not required. You have no idea what the bootloader has
done.
> +static int nxp_c45_get_delays(struct phy_device *phydev)
> +{
> + struct nxp_c45_phy *priv = phydev->priv;
> + int ret;
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> + ret = device_property_read_u32(&phydev->mdio.dev,
> "tx-internal-delay-ps",
> +&priv->tx_delay);
> + if (ret) {
> + phydev_err(phydev, "tx-internal-delay-ps property
> missing\n");
This is not normally mandatory. Default to 2ns if not specified in DT.
> +static int nxp_c45_set_phy_mode(struct phy_device *phydev)
> +{
> + int ret;
> +
> + ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, ABILITIES);
> + phydev_dbg(phydev, "Clause 45 managed PHY abilities 0x%x\n", ret);
> +
> + switch (phydev->interface) {
> + case PHY_INTERFACE_MODE_RGMII:
> + if (!(ret & RGMII_ABILITY)) {
> + phydev_err(phydev, "rgmii mode not supported\n");
> + return -EINVAL;
> + }
> + phy_write_mmd(phydev, MDIO_MMD_VEND1, MII_BASIC_CONFIG,
> MII_BASIC_CONFIG_RGMII);
> + break;
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + case PHY_INTERFACE_MODE_RGMII_TXID:
> + case PHY_INTERFACE_MODE_RGMII_RXID:
> + if (!(ret & RGMII_ID_ABILITY)) {
> + phydev_err(phydev, "rgmii-id, rgmii-txid, rgmii-rxid
> modes are not supported\n");
> + return -EINVAL;
> + }
> + phy_write_mmd(phydev, MDIO_MMD_VEND1, MII_BASIC_CONFIG,
> MII_BASIC_CONFIG_RGMII);
> + ret = nxp_c45_get_delays(phydev);
> + if (ret)
> + return ret;
> +
> + nxp_c45_set_delays(phydev);
> + break;
Again, for PHY_INTERFACE_MODE_RGMII you need to ensure the hardware is
not inserting a delay.
> + case PHY_INTERFACE_MODE_SGMII:
> + if (!(ret & SGMII_ABILITY)) {
> + phydev_err(phydev, "sgmii mode not supported\n");
> + return -EINVAL;
> + }
> + phy_write_mmd(phydev, MDIO_MMD_VEND1, MII_BASIC_CONFIG,
> MII_BASIC_CONFIG_SGMII);
> + break;
Interested. What gets reported over the inband signalling?
Andrew
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Mon, Apr 12, 2021 at 05:49:04PM +0300, Radu Nicolae Pirea (NXP OSS) wrote: > On Mon, 2021-04-12 at 16:23 +0200, Andrew Lunn wrote: > > > It is purely a C45 device. > > > > > Even if the PHY will be based on the same IP or not, if it is a C45 > > > PHY, it will be supported by this driver. We are not talking about > > > 2 or > > > 3 PHYs. This driver will support all future C45 PHYs. That's why we > > > named it "NXP C45". > > > > So if in future you produce C45 multi-gige PHYs, which have nothing > > in > > common with the T1 automative PHY, it will still be in this driver? > Yes. C45 is robust and, if the PHY interface is standard, you can > support Base-T, Base-T1, and so on in the same register interface. So what you are say is, you don't care if the IP is completely different, it all goes in one driver. So lets put this driver into nxp-tja11xx.c. And then we avoid all the naming issues. Andrew
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Mon, 2021-04-12 at 16:23 +0200, Andrew Lunn wrote: > > It is purely a C45 device. > > > Even if the PHY will be based on the same IP or not, if it is a C45 > > PHY, it will be supported by this driver. We are not talking about > > 2 or > > 3 PHYs. This driver will support all future C45 PHYs. That's why we > > named it "NXP C45". > > So if in future you produce C45 multi-gige PHYs, which have nothing > in > common with the T1 automative PHY, it will still be in this driver? Yes. C45 is robust and, if the PHY interface is standard, you can support Base-T, Base-T1, and so on in the same register interface. > > Andrew
Re: [PATCH] phy: nxp-c45: add driver for tja1103
> It is purely a C45 device. > Even if the PHY will be based on the same IP or not, if it is a C45 > PHY, it will be supported by this driver. We are not talking about 2 or > 3 PHYs. This driver will support all future C45 PHYs. That's why we > named it "NXP C45". So if in future you produce C45 multi-gige PHYs, which have nothing in common with the T1 automative PHY, it will still be in this driver? Andrew
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Mon, 2021-04-12 at 14:57 +0200, Andrew Lunn wrote: > On Mon, Apr 12, 2021 at 01:02:07PM +0300, Radu Nicolae Pirea (NXP > OSS) wrote: > > On Fri, 2021-04-09 at 21:36 +0200, Andrew Lunn wrote: > > > On Fri, Apr 09, 2021 at 09:41:06PM +0300, Radu Pirea (NXP OSS) > > > wrote: > > > > Add driver for tja1103 driver and for future NXP C45 PHYs. > > > > > > So apart from c45 vs c22, how does this differ to nxp-tja11xx.c? > > > Do we really want two different drivers for the same hardware? > > > Can we combine them somehow? > > It looks like the PHYs are the same hardware, but that's not > > entirely > > true. Just the naming is the same. TJA1103 is using a different IP > > and > > is having timestamping support(I will add it later). > > Is the IP very different? You often see different generations of a > PHY > supported by the same driver, if the generations are similar. Yes. It's very different. I know what you mean, but that's not the case. That's why we decided to write a new driver from scratch. > > Does it support C22 or it is purely a C45 device? It is purely a C45 device. > > > TJA is also not an Ethernet PHY series, but a general prefix for > > media > > interfaces including also CAN, LIN, etc. > > > > > > > +config NXP_C45_PHY > > > > + tristate "NXP C45 PHYs" > > > > > > This is also very vague. So in the future it will support PHYs > > > other > > > than the TJA series? > > Yes, in the future this driver will support other PHYs too. > > Based on the same IP? > Or different IP? Are we talking about 2 more > PHYs, so like the nxp-tja11xx.c will support 3 PHYs. And then the > tja1106 will come along and need a new driver? > What will you call > that? > I just don't like 'NXP C45 PHYs", it gives no clue as to what it > actually supports, and it gives you problems when you need to add yet > another driver. Even if the PHY will be based on the same IP or not, if it is a C45 PHY, it will be supported by this driver. We are not talking about 2 or 3 PHYs. This driver will support all future C45 PHYs. That's why we named it "NXP C45". > > At minimum, there needs to be a patch to add tja1102 to the help for > the nxp-tja11xx.c driver. And this driver needs to list tja1103. I will make the changes then. Thank you. > > Andrew
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Mon, Apr 12, 2021 at 01:02:07PM +0300, Radu Nicolae Pirea (NXP OSS) wrote: > On Fri, 2021-04-09 at 21:36 +0200, Andrew Lunn wrote: > > On Fri, Apr 09, 2021 at 09:41:06PM +0300, Radu Pirea (NXP OSS) wrote: > > > Add driver for tja1103 driver and for future NXP C45 PHYs. > > > > So apart from c45 vs c22, how does this differ to nxp-tja11xx.c? > > Do we really want two different drivers for the same hardware? > > Can we combine them somehow? > It looks like the PHYs are the same hardware, but that's not entirely > true. Just the naming is the same. TJA1103 is using a different IP and > is having timestamping support(I will add it later). Is the IP very different? You often see different generations of a PHY supported by the same driver, if the generations are similar. Does it support C22 or it is purely a C45 device? > TJA is also not an Ethernet PHY series, but a general prefix for media > interfaces including also CAN, LIN, etc. > > > > > +config NXP_C45_PHY > > > + tristate "NXP C45 PHYs" > > > > This is also very vague. So in the future it will support PHYs other > > than the TJA series? > Yes, in the future this driver will support other PHYs too. Based on the same IP? Or different IP? Are we talking about 2 more PHYs, so like the nxp-tja11xx.c will support 3 PHYs. And then the tja1106 will come along and need a new driver? What will you call that? I just don't like 'NXP C45 PHYs", it gives no clue as to what it actually supports, and it gives you problems when you need to add yet another driver. At minimum, there needs to be a patch to add tja1102 to the help for the nxp-tja11xx.c driver. And this driver needs to list tja1103. Andrew
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Fri, 2021-04-09 at 21:36 +0200, Andrew Lunn wrote: > On Fri, Apr 09, 2021 at 09:41:06PM +0300, Radu Pirea (NXP OSS) wrote: > > Add driver for tja1103 driver and for future NXP C45 PHYs. > > So apart from c45 vs c22, how does this differ to nxp-tja11xx.c? > Do we really want two different drivers for the same hardware? > Can we combine them somehow? It looks like the PHYs are the same hardware, but that's not entirely true. Just the naming is the same. TJA1103 is using a different IP and is having timestamping support(I will add it later). TJA is also not an Ethernet PHY series, but a general prefix for media interfaces including also CAN, LIN, etc. > > > +config NXP_C45_PHY > > + tristate "NXP C45 PHYs" > > This is also very vague. So in the future it will support PHYs other > than the TJA series? Yes, in the future this driver will support other PHYs too. > > Andrew
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Fri, Apr 09, 2021 at 09:41:06PM +0300, Radu Pirea (NXP OSS) wrote:
> +#define B100T1_PMAPMD_CTL0x0834
> +#define B100T1_PMAPMD_CONFIG_EN BIT(15)
> +#define B100T1_PMAPMD_MASTER BIT(14)
> +#define MASTER_MODE (B100T1_PMAPMD_CONFIG_EN |
> B100T1_PMAPMD_MASTER)
> +#define SLAVE_MODE (B100T1_PMAPMD_CONFIG_EN)
> +
> +#define DEVICE_CONTROL 0x0040
> +#define DEVICE_CONTROL_RESET BIT(15)
> +#define DEVICE_CONTROL_CONFIG_GLOBAL_EN BIT(14)
> +#define DEVICE_CONTROL_CONFIG_ALL_EN BIT(13)
> +#define RESET_POLL_NS(250 * NSEC_PER_MSEC)
> +
> +#define PHY_CONTROL 0x8100
> +#define PHY_CONFIG_ENBIT(14)
> +#define PHY_START_OP BIT(0)
> +
> +#define PHY_CONFIG 0x8108
> +#define PHY_CONFIG_AUTO BIT(0)
> +
> +#define SIGNAL_QUALITY 0x8320
> +#define SQI_VALIDBIT(14)
> +#define SQI_MASK GENMASK(2, 0)
> +#define MAX_SQI SQI_MASK
> +
> +#define CABLE_TEST 0x8330
> +#define CABLE_TEST_ENABLEBIT(15)
> +#define CABLE_TEST_START BIT(14)
> +#define CABLE_TEST_VALID BIT(13)
> +#define CABLE_TEST_OK0x00
> +#define CABLE_TEST_SHORTED 0x01
> +#define CABLE_TEST_OPEN 0x02
> +#define CABLE_TEST_UNKNOWN 0x07
> +
> +#define PORT_CONTROL 0x8040
> +#define PORT_CONTROL_EN BIT(14)
> +
> +#define PORT_INFRA_CONTROL 0xAC00
> +#define PORT_INFRA_CONTROL_ENBIT(14)
> +
> +#define VND1_RXID0xAFCC
> +#define VND1_TXID0xAFCD
> +#define ID_ENABLEBIT(15)
> +
> +#define ABILITIES0xAFC4
> +#define RGMII_ID_ABILITY BIT(15)
> +#define RGMII_ABILITYBIT(14)
> +#define RMII_ABILITY BIT(10)
> +#define REVMII_ABILITY BIT(9)
> +#define MII_ABILITY BIT(8)
> +#define SGMII_ABILITYBIT(0)
> +
> +#define MII_BASIC_CONFIG 0xAFC6
> +#define MII_BASIC_CONFIG_REV BIT(8)
> +#define MII_BASIC_CONFIG_SGMII 0x9
> +#define MII_BASIC_CONFIG_RGMII 0x7
> +#define MII_BASIC_CONFIG_RMII0x5
> +#define MII_BASIC_CONFIG_MII 0x4
> +
> +#define SYMBOL_ERROR_COUNTER 0x8350
> +#define LINK_DROP_COUNTER0x8352
> +#define LINK_LOSSES_AND_FAILURES 0x8353
> +#define R_GOOD_FRAME_CNT 0xA950
> +#define R_BAD_FRAME_CNT 0xA952
> +#define R_RXER_FRAME_CNT 0xA954
> +#define RX_PREAMBLE_COUNT0xAFCE
> +#define TX_PREAMBLE_COUNT0xAFCF
> +#define RX_IPG_LENGTH0xAFD0
> +#define TX_IPG_LENGTH0xAFD1
> +#define COUNTERS_EN BIT(15)
> +
> +#define CLK_25MHZ_PS_PERIOD 4UL
> +#define PS_PER_DEGREE(CLK_25MHZ_PS_PERIOD / 360)
> +#define MIN_ID_PS8222U
> +#define MAX_ID_PS11300U
Maybe include some prefix as to which MMD each of these registers is
located?
> +static bool nxp_c45_can_sleep(struct phy_device *phydev)
> +{
> + int reg;
> +
> + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT1);
> + if (reg < 0)
> + return false;
> +
> + return !!(reg & MDIO_STAT1_LPOWERABLE);
> +}
This looks like it could be useful as a generic helper function -
nothing in this function is specific to this PHY.
> +static int nxp_c45_resume(struct phy_device *phydev)
> +{
> + int reg;
> +
> + if (!nxp_c45_can_sleep(phydev))
> + return -EOPNOTSUPP;
> +
> + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
> + reg &= ~MDIO_CTRL1_LPOWER;
> + phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, reg);
> +
> + return 0;
> +}
> +
> +static int nxp_c45_suspend(struct phy_device *phydev)
> +{
> + int reg;
> +
> + if (!nxp_c45_can_sleep(phydev))
> + return -EOPNOTSUPP;
> +
> + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
> + reg |= MDIO_CTRL1_LPOWER;
> + phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, reg);
> +
> + return 0;
> +}
These too look like potential generic helper functions.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Fri, 2021-04-09 at 21:18 +0200, Heiner Kallweit wrote: > On 09.04.2021 20:41, Radu Pirea (NXP OSS) wrote: > > Add driver for tja1103 driver and for future NXP C45 PHYs. > > > > Signed-off-by: Radu Pirea (NXP OSS) > > > > --- > > MAINTAINERS | 6 + > > drivers/net/phy/Kconfig | 6 + > > drivers/net/phy/Makefile | 1 + > > drivers/net/phy/nxp-c45.c | 622 > > ++ > > 4 files changed, 635 insertions(+) > > create mode 100644 drivers/net/phy/nxp-c45.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index a008b70f3c16..082a5eca8913 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -12518,6 +12518,12 @@ F: drivers/nvmem/ > > F: include/linux/nvmem-consumer.h > > F: include/linux/nvmem-provider.h > > > > +NXP C45 PHY DRIVER > > +M: Radu Pirea > > +L: net...@vger.kernel.org > > +S: Maintained > > +F: drivers/net/phy/nxp-c45.c > > + > > NXP FSPI DRIVER > > M: Ashish Kumar > > R: Yogesh Gaur > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > > index 698bea312adc..fd2da80b5339 100644 > > --- a/drivers/net/phy/Kconfig > > +++ b/drivers/net/phy/Kconfig > > @@ -228,6 +228,12 @@ config NATIONAL_PHY > > help > > Currently supports the DP83865 PHY. > > > > +config NXP_C45_PHY > > + tristate "NXP C45 PHYs" > > + help > > + Enable support for NXP C45 PHYs. > > + Currently supports only the TJA1103 PHY. > > + > > config NXP_TJA11XX_PHY > > tristate "NXP TJA11xx PHYs support" > > depends on HWMON > > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > > index a13e402074cf..a18f095748b5 100644 > > --- a/drivers/net/phy/Makefile > > +++ b/drivers/net/phy/Makefile > > @@ -70,6 +70,7 @@ obj-$(CONFIG_MICROCHIP_PHY) += microchip.o > > obj-$(CONFIG_MICROCHIP_T1_PHY) += microchip_t1.o > > obj-$(CONFIG_MICROSEMI_PHY)+= mscc/ > > obj-$(CONFIG_NATIONAL_PHY) += national.o > > +obj-$(CONFIG_NXP_C45_PHY) += nxp-c45.o > > obj-$(CONFIG_NXP_TJA11XX_PHY) += nxp-tja11xx.o > > obj-$(CONFIG_QSEMI_PHY)+= qsemi.o > > obj-$(CONFIG_REALTEK_PHY) += realtek.o > > diff --git a/drivers/net/phy/nxp-c45.c b/drivers/net/phy/nxp-c45.c > > new file mode 100644 > > index ..2961799f7d05 > > --- /dev/null > > +++ b/drivers/net/phy/nxp-c45.c > > @@ -0,0 +1,622 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* NXP C45 PHY driver > > + * Copyright (C) 2021 NXP > > + * Copyright (C) 2021 Radu Pirea > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define PHY_ID_BASE_T1 0x001BB010 > > + > > +#define B100T1_PMAPMD_CTL 0x0834 > > +#define B100T1_PMAPMD_CONFIG_ENBIT(15) > > +#define B100T1_PMAPMD_MASTER BIT(14) > > +#define MASTER_MODE(B100T1_PMAPMD_CONFIG_EN | > > B100T1_PMAPMD_MASTER) > > +#define SLAVE_MODE (B100T1_PMAPMD_CONFIG_EN) > > + > > +#define DEVICE_CONTROL 0x0040 > > +#define DEVICE_CONTROL_RESET BIT(15) > > +#define DEVICE_CONTROL_CONFIG_GLOBAL_ENBIT(14) > > +#define DEVICE_CONTROL_CONFIG_ALL_EN BIT(13) > > +#define RESET_POLL_NS (250 * NSEC_PER_MSEC) > > + > > +#define PHY_CONTROL0x8100 > > +#define PHY_CONFIG_EN BIT(14) > > +#define PHY_START_OP BIT(0) > > + > > +#define PHY_CONFIG 0x8108 > > +#define PHY_CONFIG_AUTOBIT(0) > > + > > +#define SIGNAL_QUALITY 0x8320 > > +#define SQI_VALID BIT(14) > > +#define SQI_MASK GENMASK(2, 0) > > +#define MAX_SQISQI_MASK > > + > > +#define CABLE_TEST 0x8330 > > +#define CABLE_TEST_ENABLE BIT(15) > > +#define CABLE_TEST_START BIT(14) > > +#define CABLE_TEST_VALID BIT(13) > > +#define CABLE_TEST_OK 0x00 > > +#define CABLE_TEST_SHORTED 0x01 > > +#define CABLE_TEST_OPEN0x02 > > +#define CABLE_TEST_UNKNOWN 0x07 > > + > > +#define PORT_CONTROL 0x8040 > > +#define PORT_CONTROL_ENBIT(14) > > + > > +#define PORT_INFRA_CONTROL 0xAC00 > > +#define PORT_INFRA_CONTROL_EN BIT(14) > > + > > +#define VND1_RXID 0xAFCC > > +#define VND1_TXID 0xAFCD > > +#define ID_ENABLE BIT(15) > > + > > +#define ABILITIES 0xAFC4 > > +#define RGMII_ID_ABILITY BIT(15) > > +#define RGMII_ABILITY BIT(14) > > +#define RMII_ABILITY BIT(10) > > +#define REVMII_ABILITY BIT(9) > > +#define MII_ABILITY
Re: [PATCH] phy: nxp-c45: add driver for tja1103
Hi "Radu, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v5.12-rc6 next-20210409] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Radu-Pirea-NXP-OSS/phy-nxp-c45-add-driver-for-tja1103/20210410-024203 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 17e7124aad766b3f158943acb51467f86220afe9 config: arm-allyesconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/436e508ba7a4bbe24891acf430d7722ed1f5e128 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Radu-Pirea-NXP-OSS/phy-nxp-c45-add-driver-for-tja1103/20210410-024203 git checkout 436e508ba7a4bbe24891acf430d7722ed1f5e128 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): arm-linux-gnueabi-ld: drivers/net/phy/nxp-c45.o: in function `nxp_c45_config_init': >> nxp-c45.c:(.text+0xe2c): undefined reference to `__aeabi_uldivmod' >> arm-linux-gnueabi-ld: nxp-c45.c:(.text+0xe64): undefined reference to >> `__aeabi_uldivmod' arm-linux-gnueabi-ld: nxp-c45.c:(.text+0xec0): undefined reference to `__aeabi_uldivmod' arm-linux-gnueabi-ld: nxp-c45.c:(.text+0xef8): undefined reference to `__aeabi_uldivmod' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Fri, Apr 09, 2021 at 09:41:06PM +0300, Radu Pirea (NXP OSS) wrote: > Add driver for tja1103 driver and for future NXP C45 PHYs. So apart from c45 vs c22, how does this differ to nxp-tja11xx.c? Do we really want two different drivers for the same hardware? Can we combine them somehow? > +config NXP_C45_PHY > + tristate "NXP C45 PHYs" This is also very vague. So in the future it will support PHYs other than the TJA series? Andrew
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Fri, 9 Apr 2021 21:41:06 +0300 Radu Pirea (NXP OSS) wrote: > Add driver for tja1103 driver and for future NXP C45 PHYs. > > Signed-off-by: Radu Pirea (NXP OSS) drivers/net/phy/nxp-c45: struct mdio_device_id is 8 bytes. The last of 1 is: 0x10 0xb0 0x1b 0x00 0xf0 0xff 0xff 0xff FATAL: modpost: drivers/net/phy/nxp-c45: struct mdio_device_id is not terminated with a NULL entry! make[2]: *** [Module.symvers] Error 1 make[1]: *** [modules] Error 2 make: *** [__sub-make] Error 2
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On 09.04.2021 20:41, Radu Pirea (NXP OSS) wrote: > Add driver for tja1103 driver and for future NXP C45 PHYs. > > Signed-off-by: Radu Pirea (NXP OSS) > --- > MAINTAINERS | 6 + > drivers/net/phy/Kconfig | 6 + > drivers/net/phy/Makefile | 1 + > drivers/net/phy/nxp-c45.c | 622 ++ > 4 files changed, 635 insertions(+) > create mode 100644 drivers/net/phy/nxp-c45.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index a008b70f3c16..082a5eca8913 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12518,6 +12518,12 @@ F: drivers/nvmem/ > F: include/linux/nvmem-consumer.h > F: include/linux/nvmem-provider.h > > +NXP C45 PHY DRIVER > +M: Radu Pirea > +L: net...@vger.kernel.org > +S: Maintained > +F: drivers/net/phy/nxp-c45.c > + > NXP FSPI DRIVER > M: Ashish Kumar > R: Yogesh Gaur > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 698bea312adc..fd2da80b5339 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -228,6 +228,12 @@ config NATIONAL_PHY > help > Currently supports the DP83865 PHY. > > +config NXP_C45_PHY > + tristate "NXP C45 PHYs" > + help > + Enable support for NXP C45 PHYs. > + Currently supports only the TJA1103 PHY. > + > config NXP_TJA11XX_PHY > tristate "NXP TJA11xx PHYs support" > depends on HWMON > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index a13e402074cf..a18f095748b5 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -70,6 +70,7 @@ obj-$(CONFIG_MICROCHIP_PHY) += microchip.o > obj-$(CONFIG_MICROCHIP_T1_PHY) += microchip_t1.o > obj-$(CONFIG_MICROSEMI_PHY) += mscc/ > obj-$(CONFIG_NATIONAL_PHY) += national.o > +obj-$(CONFIG_NXP_C45_PHY)+= nxp-c45.o > obj-$(CONFIG_NXP_TJA11XX_PHY)+= nxp-tja11xx.o > obj-$(CONFIG_QSEMI_PHY) += qsemi.o > obj-$(CONFIG_REALTEK_PHY)+= realtek.o > diff --git a/drivers/net/phy/nxp-c45.c b/drivers/net/phy/nxp-c45.c > new file mode 100644 > index ..2961799f7d05 > --- /dev/null > +++ b/drivers/net/phy/nxp-c45.c > @@ -0,0 +1,622 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* NXP C45 PHY driver > + * Copyright (C) 2021 NXP > + * Copyright (C) 2021 Radu Pirea > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define PHY_ID_BASE_T1 0x001BB010 > + > +#define B100T1_PMAPMD_CTL0x0834 > +#define B100T1_PMAPMD_CONFIG_EN BIT(15) > +#define B100T1_PMAPMD_MASTER BIT(14) > +#define MASTER_MODE (B100T1_PMAPMD_CONFIG_EN | > B100T1_PMAPMD_MASTER) > +#define SLAVE_MODE (B100T1_PMAPMD_CONFIG_EN) > + > +#define DEVICE_CONTROL 0x0040 > +#define DEVICE_CONTROL_RESET BIT(15) > +#define DEVICE_CONTROL_CONFIG_GLOBAL_EN BIT(14) > +#define DEVICE_CONTROL_CONFIG_ALL_EN BIT(13) > +#define RESET_POLL_NS(250 * NSEC_PER_MSEC) > + > +#define PHY_CONTROL 0x8100 > +#define PHY_CONFIG_ENBIT(14) > +#define PHY_START_OP BIT(0) > + > +#define PHY_CONFIG 0x8108 > +#define PHY_CONFIG_AUTO BIT(0) > + > +#define SIGNAL_QUALITY 0x8320 > +#define SQI_VALIDBIT(14) > +#define SQI_MASK GENMASK(2, 0) > +#define MAX_SQI SQI_MASK > + > +#define CABLE_TEST 0x8330 > +#define CABLE_TEST_ENABLEBIT(15) > +#define CABLE_TEST_START BIT(14) > +#define CABLE_TEST_VALID BIT(13) > +#define CABLE_TEST_OK0x00 > +#define CABLE_TEST_SHORTED 0x01 > +#define CABLE_TEST_OPEN 0x02 > +#define CABLE_TEST_UNKNOWN 0x07 > + > +#define PORT_CONTROL 0x8040 > +#define PORT_CONTROL_EN BIT(14) > + > +#define PORT_INFRA_CONTROL 0xAC00 > +#define PORT_INFRA_CONTROL_ENBIT(14) > + > +#define VND1_RXID0xAFCC > +#define VND1_TXID0xAFCD > +#define ID_ENABLEBIT(15) > + > +#define ABILITIES0xAFC4 > +#define RGMII_ID_ABILITY BIT(15) > +#define RGMII_ABILITYBIT(14) > +#define RMII_ABILITY BIT(10) > +#define REVMII_ABILITY BIT(9) > +#define MII_ABILITY BIT(8) > +#define SGMII_ABILITYBIT(0) > + > +#define MII_BASIC_CONFIG 0xAFC6 > +#define MII_BASIC_CONFIG_REV BIT(8) > +#define MII_BASIC_CONFIG_SGMII 0x9 > +#define MII_BASIC_CONFIG_RGMII 0x7 > +#define MII_BASIC_CONFIG_RMII0x5 > +#define MII_BASIC_CONFIG_MII 0x4 > + >
[PATCH] phy: nxp-c45: add driver for tja1103
Add driver for tja1103 driver and for future NXP C45 PHYs. Signed-off-by: Radu Pirea (NXP OSS) --- MAINTAINERS | 6 + drivers/net/phy/Kconfig | 6 + drivers/net/phy/Makefile | 1 + drivers/net/phy/nxp-c45.c | 622 ++ 4 files changed, 635 insertions(+) create mode 100644 drivers/net/phy/nxp-c45.c diff --git a/MAINTAINERS b/MAINTAINERS index a008b70f3c16..082a5eca8913 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12518,6 +12518,12 @@ F: drivers/nvmem/ F: include/linux/nvmem-consumer.h F: include/linux/nvmem-provider.h +NXP C45 PHY DRIVER +M: Radu Pirea +L: net...@vger.kernel.org +S: Maintained +F: drivers/net/phy/nxp-c45.c + NXP FSPI DRIVER M: Ashish Kumar R: Yogesh Gaur diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 698bea312adc..fd2da80b5339 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -228,6 +228,12 @@ config NATIONAL_PHY help Currently supports the DP83865 PHY. +config NXP_C45_PHY + tristate "NXP C45 PHYs" + help + Enable support for NXP C45 PHYs. + Currently supports only the TJA1103 PHY. + config NXP_TJA11XX_PHY tristate "NXP TJA11xx PHYs support" depends on HWMON diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index a13e402074cf..a18f095748b5 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -70,6 +70,7 @@ obj-$(CONFIG_MICROCHIP_PHY) += microchip.o obj-$(CONFIG_MICROCHIP_T1_PHY) += microchip_t1.o obj-$(CONFIG_MICROSEMI_PHY)+= mscc/ obj-$(CONFIG_NATIONAL_PHY) += national.o +obj-$(CONFIG_NXP_C45_PHY) += nxp-c45.o obj-$(CONFIG_NXP_TJA11XX_PHY) += nxp-tja11xx.o obj-$(CONFIG_QSEMI_PHY)+= qsemi.o obj-$(CONFIG_REALTEK_PHY) += realtek.o diff --git a/drivers/net/phy/nxp-c45.c b/drivers/net/phy/nxp-c45.c new file mode 100644 index ..2961799f7d05 --- /dev/null +++ b/drivers/net/phy/nxp-c45.c @@ -0,0 +1,622 @@ +// SPDX-License-Identifier: GPL-2.0 +/* NXP C45 PHY driver + * Copyright (C) 2021 NXP + * Copyright (C) 2021 Radu Pirea + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define PHY_ID_BASE_T1 0x001BB010 + +#define B100T1_PMAPMD_CTL 0x0834 +#define B100T1_PMAPMD_CONFIG_ENBIT(15) +#define B100T1_PMAPMD_MASTER BIT(14) +#define MASTER_MODE(B100T1_PMAPMD_CONFIG_EN | B100T1_PMAPMD_MASTER) +#define SLAVE_MODE (B100T1_PMAPMD_CONFIG_EN) + +#define DEVICE_CONTROL 0x0040 +#define DEVICE_CONTROL_RESET BIT(15) +#define DEVICE_CONTROL_CONFIG_GLOBAL_ENBIT(14) +#define DEVICE_CONTROL_CONFIG_ALL_EN BIT(13) +#define RESET_POLL_NS (250 * NSEC_PER_MSEC) + +#define PHY_CONTROL0x8100 +#define PHY_CONFIG_EN BIT(14) +#define PHY_START_OP BIT(0) + +#define PHY_CONFIG 0x8108 +#define PHY_CONFIG_AUTOBIT(0) + +#define SIGNAL_QUALITY 0x8320 +#define SQI_VALID BIT(14) +#define SQI_MASK GENMASK(2, 0) +#define MAX_SQISQI_MASK + +#define CABLE_TEST 0x8330 +#define CABLE_TEST_ENABLE BIT(15) +#define CABLE_TEST_START BIT(14) +#define CABLE_TEST_VALID BIT(13) +#define CABLE_TEST_OK 0x00 +#define CABLE_TEST_SHORTED 0x01 +#define CABLE_TEST_OPEN0x02 +#define CABLE_TEST_UNKNOWN 0x07 + +#define PORT_CONTROL 0x8040 +#define PORT_CONTROL_ENBIT(14) + +#define PORT_INFRA_CONTROL 0xAC00 +#define PORT_INFRA_CONTROL_EN BIT(14) + +#define VND1_RXID 0xAFCC +#define VND1_TXID 0xAFCD +#define ID_ENABLE BIT(15) + +#define ABILITIES 0xAFC4 +#define RGMII_ID_ABILITY BIT(15) +#define RGMII_ABILITY BIT(14) +#define RMII_ABILITY BIT(10) +#define REVMII_ABILITY BIT(9) +#define MII_ABILITYBIT(8) +#define SGMII_ABILITY BIT(0) + +#define MII_BASIC_CONFIG 0xAFC6 +#define MII_BASIC_CONFIG_REV BIT(8) +#define MII_BASIC_CONFIG_SGMII 0x9 +#define MII_BASIC_CONFIG_RGMII 0x7 +#define MII_BASIC_CONFIG_RMII 0x5 +#define MII_BASIC_CONFIG_MII 0x4 + +#define SYMBOL_ERROR_COUNTER 0x8350 +#define LINK_DROP_COUNTER 0x8352 +#define LINK_LOSSES_AND_FAILURES 0x8353 +#define R_GOOD_FRAME_CNT 0xA950 +#define R_BAD_FRAME_CNT0xA952 +#define R_RXER_FRAME_CNT 0xA954 +#define RX_PRE
