Netdev List
 help / color / mirror / Atom feed
* [PATCH mlx5-next v2 1/4] net/mlx5: Set ODP capabilities for DC transport
From: Leon Romanovsky @ 2019-08-06  7:48 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Michael Guralnik, Moni Shoua,
	Saeed Mahameed, linux-netdev
In-Reply-To: <20190806074807.9111-1-leon@kernel.org>

From: Michael Guralnik <michaelgur@mellanox.com>

Query ODP capabilities for DC transport from FW.

Signed-off-by: Michael Guralnik <michaelgur@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 6 ++++++
 include/linux/mlx5/mlx5_ifc.h                  | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index b15b27a497fc..3995fc6d4d34 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -495,6 +495,12 @@ static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
 	ODP_CAP_SET_MAX(dev, xrc_odp_caps.write);
 	ODP_CAP_SET_MAX(dev, xrc_odp_caps.read);
 	ODP_CAP_SET_MAX(dev, xrc_odp_caps.atomic);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.srq_receive);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.send);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.receive);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.write);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.read);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.atomic);
 
 	if (do_set)
 		err = set_caps(dev, set_ctx, set_sz,
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index ec571fd7fcf8..b96d5c37f70f 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -944,7 +944,9 @@ struct mlx5_ifc_odp_cap_bits {
 
 	struct mlx5_ifc_odp_per_transport_service_cap_bits xrc_odp_caps;
 
-	u8         reserved_at_100[0x700];
+	struct mlx5_ifc_odp_per_transport_service_cap_bits dc_odp_caps;
+
+	u8         reserved_at_120[0x6E0];
 };
 
 struct mlx5_ifc_calc_op {
-- 
2.20.1


^ permalink raw reply related

* [PATCH rdma-next v2 0/4] ODP support for mlx5 DC QPs
From: Leon Romanovsky @ 2019-08-06  7:48 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Michael Guralnik, Moni Shoua,
	Saeed Mahameed, linux-netdev

From: Leon Romanovsky <leonro@mellanox.com>

Changelog
 v2:
 * Fixed reserved_* field wrong name (Saeed M.)
 * Split first patch to two patches, one for mlx5-next and one for rdma-next. (Saeed M.)
 v1:
 * Fixed alignment to u64 in mlx5-abi.h (Gal P.)
 * https://lore.kernel.org/linux-rdma/20190804100048.32671-1-leon@kernel.org
 v0:
 * https://lore.kernel.org/linux-rdma/20190801122139.25224-1-leon@kernel.org

---------------------------------------------------------------------------------
From Michael,

The series adds support for on-demand paging for DC transport.
Adding handling of DC WQE parsing upon page faults and exposing
capabilities.

As DC is mlx-only transport, the capabilities are exposed to the user
using the direct-verbs mechanism. Namely through the
mlx5dv_query_device.

Thanks

Michael Guralnik (4):
  net/mlx5: Set ODP capabilities for DC transport
  IB/mlx5: Query ODP capabilities for DC
  IB/mlx5: Expose ODP for DC capabilities to user
  IB/mlx5: Add page fault handler for DC initiator WQE

 drivers/infiniband/hw/mlx5/main.c             |  6 +++++
 drivers/infiniband/hw/mlx5/mlx5_ib.h          |  1 +
 drivers/infiniband/hw/mlx5/odp.c              | 27 ++++++++++++++++++-
 .../net/ethernet/mellanox/mlx5/core/main.c    |  6 +++++
 include/linux/mlx5/mlx5_ifc.h                 |  4 ++-
 include/uapi/rdma/mlx5-abi.h                  |  3 +++
 6 files changed, 45 insertions(+), 2 deletions(-)

--
2.20.1


^ permalink raw reply

* [PATCH net] net: ethernet: sun4i-emac: Support phy-handle property for finding PHYs
From: Chen-Yu Tsai @ 2019-08-06  7:35 UTC (permalink / raw)
  To: David S. Miller, Maxime Ripard
  Cc: Chen-Yu Tsai, netdev, linux-arm-kernel, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The sun4i-emac uses the "phy" property to find the PHY it's supposed to
use. This property was deprecated in favor of "phy-handle" in commit
8c5b09447625 ("dt-bindings: net: sun4i-emac: Convert the binding to a
schemas").

Add support for this new property name, and fall back to the old one in
case the device tree hasn't been updated.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---

The aforementioned commit is in v5.3-rc1. It would be nice to have the
driver fix in the same release. In addition, an update for the device
tree has been queued up for v5.4, which made us realize the driver needs
an update.

---
 drivers/net/ethernet/allwinner/sun4i-emac.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 3434730a7699..0537df06a9b5 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -860,7 +860,9 @@ static int emac_probe(struct platform_device *pdev)
 		goto out_clk_disable_unprepare;
 	}
 
-	db->phy_node = of_parse_phandle(np, "phy", 0);
+	db->phy_node = of_parse_phandle(np, "phy-handle", 0);
+	if (!db->phy_node)
+		db->phy_node = of_parse_phandle(np, "phy", 0);
 	if (!db->phy_node) {
 		dev_err(&pdev->dev, "no associated PHY\n");
 		ret = -ENODEV;
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH 15/16] net: phy: adin: add ethtool get_stats support
From: Ardelean, Alexandru @ 2019-08-06  7:18 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805153058.GU24275@lunn.ch>

On Mon, 2019-08-05 at 17:30 +0200, Andrew Lunn wrote:
> [External]
> 
> On Mon, Aug 05, 2019 at 07:54:52PM +0300, Alexandru Ardelean wrote:
> > This change implements retrieving all the error counters from the PHY.
> > The PHY supports several error counters/stats. The `Mean Square Errors`
> > status values are only valie when a link is established, and shouldn't be
> > incremented. These values characterize the quality of a signal.
> 
> I think you mean accumulated, not incremented?

accumulated sounds better;


> > The rest of the error counters are self-clearing on read.
> > Most of them are reports from the Frame Checker engine that the PHY has.
> > 
> > Not retrieving the `LPI Wake Error Count Register` here, since that is used
> > by the PHY framework to check for any EEE errors. And that register is
> > self-clearing when read (as per IEEE spec).
> > 
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >  drivers/net/phy/adin.c | 108 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 108 insertions(+)
> > 
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > index a1f3456a8504..04896547dac8 100644
> > --- a/drivers/net/phy/adin.c
> > +++ b/drivers/net/phy/adin.c
> > @@ -103,6 +103,32 @@ static struct clause22_mmd_map clause22_mmd_map[] = {
> >  	{ MDIO_MMD_PCS, MDIO_PCS_EEE_WK_ERR,	ADIN1300_LPI_WAKE_ERR_CNT_REG },
> >  };
> >  
> > +struct adin_hw_stat {
> > +	const char *string;
> > +	u16 reg1;
> > +	u16 reg2;
> > +	bool do_not_inc;
> 
> do_not_accumulate? or reverse its meaning, clear_on_read?

do_not_accumulate works;
there are only 4 regs that need this property set to true

> 
>    Andrew

^ permalink raw reply

* Re: [PATCH 15/16] net: phy: adin: add ethtool get_stats support
From: Ardelean, Alexandru @ 2019-08-06  7:11 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805152832.GT24275@lunn.ch>

On Mon, 2019-08-05 at 17:28 +0200, Andrew Lunn wrote:
> [External]
> 
> > +struct adin_hw_stat {
> > +	const char *string;
> > +static void adin_get_strings(struct phy_device *phydev, u8 *data)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(adin_hw_stats); i++) {
> > +		memcpy(data + i * ETH_GSTRING_LEN,
> > +		       adin_hw_stats[i].string, ETH_GSTRING_LEN);
> 
> You define string as a char *. So it will be only as long as it should
> be. However memcpy always copies ETH_GSTRING_LEN bytes, doing off the
> end of the string and into whatever follows.
> 

hmm, will use strlcpy()
i blindedly copied memcpy() from some other driver

> 
> > +	}
> > +}
> > +
> > +static int adin_read_mmd_stat_regs(struct phy_device *phydev,
> > +				   struct adin_hw_stat *stat,
> > +				   u32 *val)
> > +{
> > +	int ret;
> > +
> > +	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, stat->reg1);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	*val = (ret & 0xffff);
> > +
> > +	if (stat->reg2 == 0)
> > +		return 0;
> > +
> > +	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, stat->reg2);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	*val <<= 16;
> > +	*val |= (ret & 0xffff);
> 
> Does the hardware have a snapshot feature? Is there a danger that
> between the two reads stat->reg1 rolls over and you end up with too
> big a value?

i'm afraid i don't understand about the snapshot feature you are mentioning;
i.e. i don't remember seeing it in other chips;

regarding the danger that stat->reg1 rolls over, i guess that is possible, but it's a bit hard to guard against;
i guess if it ends up in that scenario, [for many counters] things would be horribly bad, and the chip, or cabling would
be unusable;

not sure if this answer is sufficient/satisfactory;

thanks

> 
>     Andrew

^ permalink raw reply

* Re: [PATCH 16/16] dt-bindings: net: add bindings for ADIN PHY driver
From: Ardelean, Alexandru @ 2019-08-06  7:03 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805141100.GG24275@lunn.ch>

On Mon, 2019-08-05 at 16:11 +0200, Andrew Lunn wrote:
> [External]
> 
> > +  adi,rx-internal-delay:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode
> > +      is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
> > +      default value is 0 (which represents 2 ns)
> > +    enum: [ 0, 1, 2, 6, 7 ]
> 
> We want these numbers to be in ns. So the default value would actually
> be 2. The driver needs to convert the number in DT to a value to poke
> into a PHY register. Please rename the property adi,rx-internal-delay-ns.

ack;
also, good point about ns units and PHY driver to convert it to reg values;

> 
> > +
> > +  adi,tx-internal-delay:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode
> > +      is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
> > +      default value is 0 (which represents 2 ns)
> > +    enum: [ 0, 1, 2, 6, 7 ]
> 
> Same here.
> 
> > +
> > +  adi,fifo-depth:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      When operating in RMII mode, this option configures the FIFO depth.
> > +      See `dt-bindings/net/adin.h`.
> > +    enum: [ 0, 1, 2, 3, 4, 5 ]
> 
> Units? You should probably rename this adi,fifo-depth-bits and list
> the valid values in bits.

units are bits;
will adapt this

> 
> > +
> > +  adi,eee-enabled:
> > +    description: |
> > +      Advertise EEE capabilities on power-up/init (default disabled)
> > +    type: boolean
> 
> It is not the PHY which decides this. The MAC indicates if it is EEE
> capable to phylib. phylib looks into the PHY registers to determine if
> the PHY supports EEE. phylib will then enable EEE
> advertisement. Please remove this, and ensure EEE is disabled by
> default.

ack;
will remove

> 
> 	Andrew

^ permalink raw reply

* Re: [PATCH mlx5-next v1 1/3] IB/mlx5: Query ODP capabilities for DC
From: Leon Romanovsky @ 2019-08-06  7:02 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: Jason Gunthorpe, dledford@redhat.com, Michael Guralnik,
	Moni Shoua, netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <d3b21502d398fc3bf2cf38231ca84c1bb0386b17.camel@mellanox.com>

On Mon, Aug 05, 2019 at 06:23:04PM +0000, Saeed Mahameed wrote:
> On Sun, 2019-08-04 at 13:00 +0300, Leon Romanovsky wrote:
> > From: Michael Guralnik <michaelgur@mellanox.com>
> >
> > Set current capabilities of ODP for DC to max capabilities and cache
> > them in mlx5_ib.
> >
> > Signed-off-by: Michael Guralnik <michaelgur@mellanox.com>
> > Reviewed-by: Moni Shoua <monis@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> >  drivers/infiniband/hw/mlx5/mlx5_ib.h           |  1 +
> >  drivers/infiniband/hw/mlx5/odp.c               | 18
> > ++++++++++++++++++
> >  drivers/net/ethernet/mellanox/mlx5/core/main.c |  6 ++++++
> >  include/linux/mlx5/mlx5_ifc.h                  |  4 +++-
>
> Please avoid cross tree changes when you can..
> Here you do can avoid it, so please separate to two stage patches,
> mlx5_ifc and core, then mlx5_ib.
>
>
> >  4 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > index cb41a7e6255a..f99c71b3c876 100644
> > --- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > @@ -967,6 +967,7 @@ struct mlx5_ib_dev {
> >  	struct mutex			slow_path_mutex;
> >  	int				fill_delay;
> >  	struct ib_odp_caps	odp_caps;
> > +	uint32_t		dc_odp_caps;
> >  	u64			odp_max_size;
> >  	struct mlx5_ib_pf_eq	odp_pf_eq;
> >
> > diff --git a/drivers/infiniband/hw/mlx5/odp.c
> > b/drivers/infiniband/hw/mlx5/odp.c
> > index b0c5de39d186..5e87a5e25574 100644
> > --- a/drivers/infiniband/hw/mlx5/odp.c
> > +++ b/drivers/infiniband/hw/mlx5/odp.c
> > @@ -353,6 +353,24 @@ void mlx5_ib_internal_fill_odp_caps(struct
> > mlx5_ib_dev *dev)
> >  	if (MLX5_CAP_ODP(dev->mdev, xrc_odp_caps.srq_receive))
> >  		caps->per_transport_caps.xrc_odp_caps |=
> > IB_ODP_SUPPORT_SRQ_RECV;
> >
> > +	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.send))
> > +		dev->dc_odp_caps |= IB_ODP_SUPPORT_SEND;
> > +
> > +	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.receive))
> > +		dev->dc_odp_caps |= IB_ODP_SUPPORT_RECV;
> > +
> > +	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.write))
> > +		dev->dc_odp_caps |= IB_ODP_SUPPORT_WRITE;
> > +
> > +	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.read))
> > +		dev->dc_odp_caps |= IB_ODP_SUPPORT_READ;
> > +
> > +	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.atomic))
> > +		dev->dc_odp_caps |= IB_ODP_SUPPORT_ATOMIC;
> > +
> > +	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.srq_receive))
> > +		dev->dc_odp_caps |= IB_ODP_SUPPORT_SRQ_RECV;
> > +
> >  	if (MLX5_CAP_GEN(dev->mdev, fixed_buffer_size) &&
> >  	    MLX5_CAP_GEN(dev->mdev, null_mkey) &&
> >  	    MLX5_CAP_GEN(dev->mdev, umr_extended_translation_offset))
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > index b15b27a497fc..3995fc6d4d34 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > @@ -495,6 +495,12 @@ static int handle_hca_cap_odp(struct
> > mlx5_core_dev *dev)
> >  	ODP_CAP_SET_MAX(dev, xrc_odp_caps.write);
> >  	ODP_CAP_SET_MAX(dev, xrc_odp_caps.read);
> >  	ODP_CAP_SET_MAX(dev, xrc_odp_caps.atomic);
> > +	ODP_CAP_SET_MAX(dev, dc_odp_caps.srq_receive);
> > +	ODP_CAP_SET_MAX(dev, dc_odp_caps.send);
> > +	ODP_CAP_SET_MAX(dev, dc_odp_caps.receive);
> > +	ODP_CAP_SET_MAX(dev, dc_odp_caps.write);
> > +	ODP_CAP_SET_MAX(dev, dc_odp_caps.read);
> > +	ODP_CAP_SET_MAX(dev, dc_odp_caps.atomic);
> >
> >  	if (do_set)
> >  		err = set_caps(dev, set_ctx, set_sz,
> > diff --git a/include/linux/mlx5/mlx5_ifc.h
> > b/include/linux/mlx5/mlx5_ifc.h
> > index ec571fd7fcf8..5eae8d734435 100644
> > --- a/include/linux/mlx5/mlx5_ifc.h
> > +++ b/include/linux/mlx5/mlx5_ifc.h
> > @@ -944,7 +944,9 @@ struct mlx5_ifc_odp_cap_bits {
> >
> >  	struct mlx5_ifc_odp_per_transport_service_cap_bits
> > xrc_odp_caps;
> >
> > -	u8         reserved_at_100[0x700];
> > +	struct mlx5_ifc_odp_per_transport_service_cap_bits dc_odp_caps;
> > +
> > +	u8         reserved_at_100[0x6E0];
>
> reserved_at_100 should move 20 bit forward. i.e reserved_at_120

Thanks for pointing it, I'm sending new version now.

>
>

^ permalink raw reply

* Re: [PATCH v2] net/mlx5e: Use refcount_t for refcount
From: Leon Romanovsky @ 2019-08-06  6:59 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: hslester96@gmail.com, davem@davemloft.net, netdev@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <b19b7cd49d373cc51d3e745a6444b27166b88304.camel@mellanox.com>

On Mon, Aug 05, 2019 at 08:06:36PM +0000, Saeed Mahameed wrote:
> On Mon, 2019-08-05 at 14:55 +0800, Chuhong Yuan wrote:
> > On Mon, Aug 5, 2019 at 2:13 PM Leon Romanovsky <leon@kernel.org>
> > wrote:
> > > On Sun, Aug 04, 2019 at 10:44:47PM +0800, Chuhong Yuan wrote:
> > > > On Sun, Aug 4, 2019 at 8:59 PM Leon Romanovsky <leon@kernel.org>
> > > > wrote:
> > > > > On Sat, Aug 03, 2019 at 12:48:28AM +0800, Chuhong Yuan wrote:
> > > > > > refcount_t is better for reference counters since its
> > > > > > implementation can prevent overflows.
> > > > > > So convert atomic_t ref counters to refcount_t.
> > > > >
> > > > > I'm not thrilled to see those automatic conversion patches,
> > > > > especially
> > > > > for flows which can't overflow. There is nothing wrong in using
> > > > > atomic_t
> > > > > type of variable, do you have in mind flow which will cause to
> > > > > overflow?
> > > > >
> > > > > Thanks
> > > >
> > > > I have to say that these patches are not done automatically...
> > > > Only the detection of problems is done by a script.
> > > > All conversions are done manually.
> > >
> > > Even worse, you need to audit usage of atomic_t and replace there
> > > it can overflow.
> > >
> > > > I am not sure whether the flow can cause an overflow.
> > >
> > > It can't.
> > >
> > > > But I think it is hard to ensure that a data path is impossible
> > > > to have problems in any cases including being attacked.
> > >
> > > It is not data path, and I doubt that such conversion will be
> > > allowed
> > > in data paths without proving that no performance regression is
> > > introduced.
> > > > So I think it is better to do this minor revision to prevent
> > > > potential risk, just like we have done in mlx5/core/cq.c.
> > >
> > > mlx5/core/cq.c is a different beast, refcount there means actual
> > > users
> > > of CQ which are limited in SW, so in theory, they have potential
> > > to be overflown.
> > >
> > > It is not the case here, there your are adding new port.
> > > There is nothing wrong with atomic_t.
> > >
> >
> > Thanks for your explanation!
> > I will pay attention to this point in similar cases.
> > But it seems that the semantic of refcount is not always as clear as
> > here...
> >
>
> Semantically speaking, there is nothing wrong with moving to refcount_t
> in the case of vxlan ports.. it also seems more accurate and will
> provide the type protection, even if it is not necessary. Please let me
> know what is the verdict here, i can apply this patch to net-next-mlx5.

There is no verdict here, it is up to you., if you like code churn, go
for it.

Thanks

>
> Thanks,
> Saeed.

^ permalink raw reply

* [PATCH net-next v2 1/1] qed: Add new ethtool supported port types based on media.
From: Rahul Verma @ 2019-08-06  6:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, aelior, mkalderon

Supported ports in ethtool <eth1> are displayed based on media type.
For media type fibre and twinaxial, port type is "FIBRE". Media type
Base-T is "TP" and media KR is "Backplane".

V1->V2:
Corrected the subject.

Signed-off-by: Rahul Verma <rahulv@marvell.com>
Signed-off-by: Michal Kalderon <michal.kalderon@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed_main.c      | 6 +++++-
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 3 ++-
 include/linux/qed/qed_if.h                      | 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 829dd60..e5ac8bd 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -1688,6 +1688,7 @@ static void qed_fill_link_capability(struct qed_hwfn *hwfn,
 
 	switch (media_type) {
 	case MEDIA_DA_TWINAX:
+		*if_capability |= QED_LM_FIBRE_BIT;
 		if (capability & NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_20G)
 			*if_capability |= QED_LM_20000baseKR2_Full_BIT;
 		/* For DAC media multiple speed capabilities are supported*/
@@ -1707,6 +1708,7 @@ static void qed_fill_link_capability(struct qed_hwfn *hwfn,
 			*if_capability |= QED_LM_100000baseCR4_Full_BIT;
 		break;
 	case MEDIA_BASE_T:
+		*if_capability |= QED_LM_TP_BIT;
 		if (board_cfg & NVM_CFG1_PORT_PORT_TYPE_EXT_PHY) {
 			if (capability &
 			    NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_1G) {
@@ -1718,6 +1720,7 @@ static void qed_fill_link_capability(struct qed_hwfn *hwfn,
 			}
 		}
 		if (board_cfg & NVM_CFG1_PORT_PORT_TYPE_MODULE) {
+			*if_capability |= QED_LM_FIBRE_BIT;
 			if (tcvr_type == ETH_TRANSCEIVER_TYPE_1000BASET)
 				*if_capability |= QED_LM_1000baseT_Full_BIT;
 			if (tcvr_type == ETH_TRANSCEIVER_TYPE_10G_BASET)
@@ -1728,6 +1731,7 @@ static void qed_fill_link_capability(struct qed_hwfn *hwfn,
 	case MEDIA_SFPP_10G_FIBER:
 	case MEDIA_XFP_FIBER:
 	case MEDIA_MODULE_FIBER:
+		*if_capability |= QED_LM_FIBRE_BIT;
 		if (capability &
 		    NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_1G) {
 			if ((tcvr_type == ETH_TRANSCEIVER_TYPE_1G_LX) ||
@@ -1770,6 +1774,7 @@ static void qed_fill_link_capability(struct qed_hwfn *hwfn,
 
 		break;
 	case MEDIA_KR:
+		*if_capability |= QED_LM_Backplane_BIT;
 		if (capability & NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_20G)
 			*if_capability |= QED_LM_20000baseKR2_Full_BIT;
 		if (capability &
@@ -1821,7 +1826,6 @@ static void qed_fill_link(struct qed_hwfn *hwfn,
 		if_link->link_up = true;
 
 	/* TODO - at the moment assume supported and advertised speed equal */
-	if_link->supported_caps = QED_LM_FIBRE_BIT;
 	if (link_caps.default_speed_autoneg)
 		if_link->supported_caps |= QED_LM_Autoneg_BIT;
 	if (params.pause.autoneg ||
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index e85f9fe..abcee47 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -424,12 +424,13 @@ struct qede_link_mode_mapping {
 };
 
 static const struct qede_link_mode_mapping qed_lm_map[] = {
+	{QED_LM_FIBRE_BIT, ETHTOOL_LINK_MODE_FIBRE_BIT},
 	{QED_LM_Autoneg_BIT, ETHTOOL_LINK_MODE_Autoneg_BIT},
 	{QED_LM_Asym_Pause_BIT, ETHTOOL_LINK_MODE_Asym_Pause_BIT},
 	{QED_LM_Pause_BIT, ETHTOOL_LINK_MODE_Pause_BIT},
 	{QED_LM_1000baseT_Full_BIT, ETHTOOL_LINK_MODE_1000baseT_Full_BIT},
 	{QED_LM_10000baseT_Full_BIT, ETHTOOL_LINK_MODE_10000baseT_Full_BIT},
-	{QED_LM_2500baseX_Full_BIT, ETHTOOL_LINK_MODE_2500baseX_Full_BIT},
+	{QED_LM_TP_BIT, ETHTOOL_LINK_MODE_TP_BIT},
 	{QED_LM_Backplane_BIT, ETHTOOL_LINK_MODE_Backplane_BIT},
 	{QED_LM_1000baseKX_Full_BIT, ETHTOOL_LINK_MODE_1000baseKX_Full_BIT},
 	{QED_LM_10000baseKX4_Full_BIT, ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT},
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index eef02e6..2302136 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -689,7 +689,7 @@ enum qed_link_mode_bits {
 	QED_LM_40000baseLR4_Full_BIT = BIT(9),
 	QED_LM_50000baseKR2_Full_BIT = BIT(10),
 	QED_LM_100000baseKR4_Full_BIT = BIT(11),
-	QED_LM_2500baseX_Full_BIT = BIT(12),
+	QED_LM_TP_BIT = BIT(12),
 	QED_LM_Backplane_BIT = BIT(13),
 	QED_LM_1000baseKX_Full_BIT = BIT(14),
 	QED_LM_10000baseKX4_Full_BIT = BIT(15),
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH 16/16] dt-bindings: net: add bindings for ADIN PHY driver
From: Ardelean, Alexandru @ 2019-08-06  6:57 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805142754.GL24275@lunn.ch>

On Mon, 2019-08-05 at 16:27 +0200, Andrew Lunn wrote:
> [External]
> 
> On Mon, Aug 05, 2019 at 07:54:53PM +0300, Alexandru Ardelean wrote:
> > This change adds bindings for the Analog Devices ADIN PHY driver, detailing
> > all the properties implemented by the driver.
> > 
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >  .../devicetree/bindings/net/adi,adin.yaml     | 93 +++++++++++++++++++
> >  MAINTAINERS                                   |  2 +
> >  include/dt-bindings/net/adin.h                | 26 ++++++
> >  3 files changed, 121 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/adi,adin.yaml
> >  create mode 100644 include/dt-bindings/net/adin.h
> > 
> > diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml
> > b/Documentation/devicetree/bindings/net/adi,adin.yaml
> > new file mode 100644
> > index 000000000000..fcf884bb86f7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
> > @@ -0,0 +1,93 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/adi,adin.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices ADIN1200/ADIN1300 PHY
> > +
> > +maintainers:
> > +  - Alexandru Ardelean <alexandru.ardelean@analog.com>
> > +
> > +description: |
> > +  Bindings for Analog Devices Industrial Ethernet PHYsphy
> > +
> > +properties:
> > +  compatible:
> > +    description: |
> > +      Compatible list, may contain "ethernet-phy-ieee802.3-c45" in which case
> > +      Clause 45 will be used to access device management registers. If
> > +      unspecified, Clause 22 will be used. Use this only when MDIO supports
> > +      Clause 45 access, but there is no other way to determine this.
> > +    enum:
> > +      - ethernet-phy-ieee802.3-c45
> 
> It is valid to list ethernet-phy-ieee802.3-c22, it is just not
> required. So maybe you should list it here to keep the DT validater happy?

ack

> 
> 	  Andrew

^ permalink raw reply

* Re: [PATCH iproute2-next] rdma: Add driver QP type string
From: Leon Romanovsky @ 2019-08-06  6:54 UTC (permalink / raw)
  To: Gal Pressman; +Cc: David Ahern, Stephen Hemminger, netdev, linux-rdma
In-Reply-To: <d156ece6-79bf-f9a4-8b79-a5abf738476d@amazon.com>

On Tue, Aug 06, 2019 at 09:41:37AM +0300, Gal Pressman wrote:
> On 05/08/2019 22:08, David Ahern wrote:
> > On 8/4/19 2:07 AM, Gal Pressman wrote:
> >> RDMA resource tracker now tracks driver QPs as well, add driver QP type
> >> string to qp_types_to_str function.
> >
> > "now" means which kernel release? Leon: should this be in master or -next?
>
> Now means the patch is merged to RDMA's for-rc branch (5.3).

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/infiniband?id=52e0a118a20308dd6aa531e20a5ab5907d2264c8

David,

I think that it is better to apply this patch to iproute2-rc just
to be on the same page with kernel patch.

Thanks

^ permalink raw reply

* Re: [PATCH 14/16] net: phy: adin: make sure down-speed auto-neg is enabled
From: Ardelean, Alexandru @ 2019-08-06  6:53 UTC (permalink / raw)
  To: devicetree@vger.kernel.org, hkallweit1@gmail.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: f.fainelli@gmail.com, davem@davemloft.net, mark.rutland@arm.com,
	robh+dt@kernel.org, andrew@lunn.ch
In-Reply-To: <baac3842-bd9c-75af-83e3-9e89def1c429@gmail.com>

On Tue, 2019-08-06 at 07:52 +0200, Heiner Kallweit wrote:
> [External]
> 
> On 05.08.2019 18:54, Alexandru Ardelean wrote:
> > Down-speed auto-negotiation may not always be enabled, in which case the
> > PHY won't down-shift to 100 or 10 during auto-negotiation.
> > 
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >  drivers/net/phy/adin.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > index 86848444bd98..a1f3456a8504 100644
> > --- a/drivers/net/phy/adin.c
> > +++ b/drivers/net/phy/adin.c
> > @@ -32,6 +32,13 @@
> >  #define   ADIN1300_NRG_PD_TX_EN			BIT(2)
> >  #define   ADIN1300_NRG_PD_STATUS		BIT(1)
> >  
> > +#define ADIN1300_PHY_CTRL2			0x0016
> > +#define   ADIN1300_DOWNSPEED_AN_100_EN		BIT(11)
> > +#define   ADIN1300_DOWNSPEED_AN_10_EN		BIT(10)
> > +#define   ADIN1300_GROUP_MDIO_EN		BIT(6)
> > +#define   ADIN1300_DOWNSPEEDS_EN	\
> > +	(ADIN1300_DOWNSPEED_AN_100_EN | ADIN1300_DOWNSPEED_AN_10_EN)
> > +
> >  #define ADIN1300_INT_MASK_REG			0x0018
> >  #define   ADIN1300_INT_MDIO_SYNC_EN		BIT(9)
> >  #define   ADIN1300_INT_ANEG_STAT_CHNG_EN	BIT(8)
> > @@ -425,6 +432,22 @@ static int adin_config_mdix(struct phy_device *phydev)
> >  	return phy_write(phydev, ADIN1300_PHY_CTRL1, reg);
> >  }
> >  
> > +static int adin_config_downspeeds(struct phy_device *phydev)
> > +{
> > +	int reg;
> > +
> > +	reg = phy_read(phydev, ADIN1300_PHY_CTRL2);
> > +	if (reg < 0)
> > +		return reg;
> > +
> > +	if ((reg & ADIN1300_DOWNSPEEDS_EN) == ADIN1300_DOWNSPEEDS_EN)
> > +		return 0;
> > +
> > +	reg |= ADIN1300_DOWNSPEEDS_EN;
> > +
> > +	return phy_write(phydev, ADIN1300_PHY_CTRL2, reg);
> 
> Using phy_set_bits() would be easier.

ack;
missed this;

thanks

> 
> > +}
> > +
> >  static int adin_config_aneg(struct phy_device *phydev)
> >  {
> >  	int ret;
> > @@ -433,6 +456,10 @@ static int adin_config_aneg(struct phy_device *phydev)
> >  	if (ret)
> >  		return ret;
> >  
> > +	ret = adin_config_downspeeds(phydev);
> > +	if (ret < 0)
> > +		return ret;
> > +
> >  	return genphy_config_aneg(phydev);
> >  }
> >  
> > 

^ permalink raw reply

* Re: [PATCH 14/16] net: phy: adin: make sure down-speed auto-neg is enabled
From: Ardelean, Alexandru @ 2019-08-06  6:53 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805152214.GS24275@lunn.ch>

On Mon, 2019-08-05 at 17:22 +0200, Andrew Lunn wrote:
> [External]
> 
> On Mon, Aug 05, 2019 at 07:54:51PM +0300, Alexandru Ardelean wrote:
> > Down-speed auto-negotiation may not always be enabled, in which case the
> > PHY won't down-shift to 100 or 10 during auto-negotiation.
> 
> Please look at how the marvell driver enables and configures this
> feature. Ideally we want all PHY drivers to use the same configuration
> API for features like this.

ack

> 
>     Andrew

^ permalink raw reply

* Re: [PATCH 12/16] net: phy: adin: read EEE setting from device-tree
From: Ardelean, Alexandru @ 2019-08-06  6:52 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805151909.GR24275@lunn.ch>

On Mon, 2019-08-05 at 17:19 +0200, Andrew Lunn wrote:
> [External]
> 
> On Mon, Aug 05, 2019 at 07:54:49PM +0300, Alexandru Ardelean wrote:
> > By default, EEE is not advertised on system init. This change allows the
> > user to specify a device property to enable EEE advertisements when the PHY
> > initializes.
>  
> This patch is not required. If EEE is not being advertised when it
> should, it means there is a MAC driver bug.

ack;
same thing about PHY specifics ignoring MAC specifics

thanks

> 
> 	Andrew

^ permalink raw reply

* Re: [PATCH 11/16] net: phy: adin: PHY reset mechanisms
From: Ardelean, Alexandru @ 2019-08-06  6:50 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805151500.GP24275@lunn.ch>

On Mon, 2019-08-05 at 17:15 +0200, Andrew Lunn wrote:
> [External]
> 
> On Mon, Aug 05, 2019 at 07:54:48PM +0300, Alexandru Ardelean wrote:
> > The ADIN PHYs supports 4 types of reset:
> > 1. The standard PHY reset via BMCR_RESET bit in MII_BMCR reg
> > 2. Reset via GPIO
> > 3. Reset via reg GeSftRst (0xff0c) & reload previous pin configs
> > 4. Reset via reg GeSftRst (0xff0c) & request new pin configs
> > 
> > Resets 2 & 4 are almost identical, with the exception that the crystal
> > oscillator is available during reset for 2.
> > 
> > Resetting via GeSftRst or via GPIO is useful when doing a warm reboot. If
> > doing various settings via phytool or ethtool, the sub-system registers
> > don't reset just via BMCR_RESET.
> > 
> > This change implements resetting the entire PHY subsystem during probe.
> > During PHY HW init (phy_hw_init() logic) the PHY core regs will be reset
> > again via BMCR_RESET. This will also need to happen during a PM resume.
> 
> phylib already has support for GPIO reset. So if possible, you should
> not repeat that code here.
> 
> What is the difference between a GPIO reset, and a GPIO reset followed
> by a subsystem soft reset?

there shouldn't be any difference;
it's just 2 consecutive resets;
i'll take a closer look at phylib's GPIO reset and see

> 
>    Andrew

^ permalink raw reply

* Re: [PATCH 10/16] net: phy: adin: add EEE translation layer for Clause 22
From: Ardelean, Alexandru @ 2019-08-06  6:47 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805221150.GE25700@lunn.ch>

On Tue, 2019-08-06 at 00:11 +0200, Andrew Lunn wrote:
> [External]
> 
> > +static int adin_cl22_to_adin_reg(int devad, u16 cl22_regnum)
> > +{
> > +	struct clause22_mmd_map *m;
> > +	int i;
> > +
> > +	if (devad == MDIO_MMD_VEND1)
> > +		return cl22_regnum;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(clause22_mmd_map); i++) {
> > +		m = &clause22_mmd_map[i];
> > +		if (m->devad == devad && m->cl22_regnum == cl22_regnum)
> > +			return m->adin_regnum;
> > +	}
> > +
> > +	pr_err("No translation available for devad: %d reg: %04x\n",
> > +	       devad, cl22_regnum);
> 
> phydev_err(). 

ack

> 
> 	      Andrew

^ permalink raw reply

* Re: [PATCH 06/16] net: phy: adin: support PHY mode converters
From: Ardelean, Alexandru @ 2019-08-06  6:47 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805145105.GN24275@lunn.ch>

On Mon, 2019-08-05 at 16:51 +0200, Andrew Lunn wrote:
> [External]
> 
> On Mon, Aug 05, 2019 at 07:54:43PM +0300, Alexandru Ardelean wrote:
> > Sometimes, the connection between a MAC and PHY is done via a
> > mode/interface converter. An example is a GMII-to-RGMII converter, which
> > would mean that the MAC operates in GMII mode while the PHY operates in
> > RGMII. In this case there is a discrepancy between what the MAC expects &
> > what the PHY expects and both need to be configured in their respective
> > modes.
> > 
> > Sometimes, this converter is specified via a board/system configuration (in
> > the device-tree for example). But, other times it can be left unspecified.
> > The use of these converters is common in boards that have FPGA on them.
> > 
> > This patch also adds support for a `adi,phy-mode-internal` property that
> > can be used in these (implicit convert) cases. The internal PHY mode will
> > be used to specify the correct register settings for the PHY.
> > 
> > `fwnode_handle` is used, since this property may be specified via ACPI as
> > well in other setups, but testing has been done in DT context.
> 
> Looking at the patch, you seems to assume phy-mode is what the MAC is
> using? That seems rather odd, given the name. It seems like a better
> solution would be to add a mac-mode, which the MAC uses to configure
> its side of the link. The MAC driver would then implement this
> property.
> 

actually, that's a pretty good idea;
i guess i was narrow-minded when writing the driver, and got stuck on phy specifics, and forgot about the MAC-side;
[ i also catch these design elements when reviewing, but i also seem to miss them when writing stuff sometimes ]

thanks

> I don't see a need for this. phy-mode indicates what the PHY should
> use. End of story.
> 
>      Andrew

^ permalink raw reply

* Re: [PATCH 05/16] net: phy: adin: configure RGMII/RMII/MII modes on config
From: Ardelean, Alexandru @ 2019-08-06  6:43 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805143935.GM24275@lunn.ch>

On Mon, 2019-08-05 at 16:39 +0200, Andrew Lunn wrote:
> [External]
> 
> On Mon, Aug 05, 2019 at 07:54:42PM +0300, Alexandru Ardelean wrote:
> > The ADIN1300 chip supports RGMII, RMII & MII modes. Default (if
> > unconfigured) is RGMII.
> > This change adds support for configuring these modes via the device
> > registers.
> > 
> > For RGMII with internal delays (modes RGMII_ID,RGMII_TXID, RGMII_RXID),
> 
> It would be nice to add the missing space.
> 
> > the default delay is 2 ns. This can be configurable and will be done in
> > a subsequent change.
> > 
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >  drivers/net/phy/adin.c | 79 +++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 78 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > index 3dd9fe50f4c8..dbdb8f60741c 100644
> > --- a/drivers/net/phy/adin.c
> > +++ b/drivers/net/phy/adin.c
> > @@ -33,14 +33,91 @@
> >  	 ADIN1300_INT_HW_IRQ_EN)
> >  #define ADIN1300_INT_STATUS_REG			0x0019
> >  
> > +#define ADIN1300_GE_RGMII_CFG_REG		0xff23
> > +#define   ADIN1300_GE_RGMII_RXID_EN		BIT(2)
> > +#define   ADIN1300_GE_RGMII_TXID_EN		BIT(1)
> > +#define   ADIN1300_GE_RGMII_EN			BIT(0)
> > +
> > +#define ADIN1300_GE_RMII_CFG_REG		0xff24
> > +#define   ADIN1300_GE_RMII_EN			BIT(0)
> > +
> > +static int adin_config_rgmii_mode(struct phy_device *phydev,
> > +				  phy_interface_t intf)
> > +{
> > +	int reg;
> > +
> > +	reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_GE_RGMII_CFG_REG);
> > +	if (reg < 0)
> > +		return reg;
> > +
> > +	if (!phy_interface_mode_is_rgmii(intf)) {
> > +		reg &= ~ADIN1300_GE_RGMII_EN;
> > +		goto write;
> > +	}
> > +
> > +	reg |= ADIN1300_GE_RGMII_EN;
> > +
> > +	if (intf == PHY_INTERFACE_MODE_RGMII_ID ||
> > +	    intf == PHY_INTERFACE_MODE_RGMII_RXID) {
> > +		reg |= ADIN1300_GE_RGMII_RXID_EN;
> > +	} else {
> > +		reg &= ~ADIN1300_GE_RGMII_RXID_EN;
> > +	}
> > +
> > +	if (intf == PHY_INTERFACE_MODE_RGMII_ID ||
> > +	    intf == PHY_INTERFACE_MODE_RGMII_TXID) {
> > +		reg |= ADIN1300_GE_RGMII_TXID_EN;
> > +	} else {
> > +		reg &= ~ADIN1300_GE_RGMII_TXID_EN;
> > +	}
> 
> Nice. Often driver writers forget to clear the delay, they only set
> it. Not so here.
> 
> However, is checkpatch happy with this? Each half of the if/else is a
> single statement, so the {} are not needed.

it did not complain;
this whole series is checkpatch friendly [with the version of checkpatch in net-next]
i think it complained about un-balanced if-block; something like:

```
if () {

} else
  single-statement
```

but checkpatch is also a moving target;
so ¯\_(ツ)_/¯

> 
> > +
> > +write:
> > +	return phy_write_mmd(phydev, MDIO_MMD_VEND1,
> > +			     ADIN1300_GE_RGMII_CFG_REG, reg);
> > +}
> > +
> > +static int adin_config_rmii_mode(struct phy_device *phydev,
> > +				 phy_interface_t intf)
> > +{
> > +	int reg;
> > +
> > +	reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_GE_RMII_CFG_REG);
> > +	if (reg < 0)
> > +		return reg;
> > +
> > +	if (intf != PHY_INTERFACE_MODE_RMII) {
> > +		reg &= ~ADIN1300_GE_RMII_EN;
> > +		goto write;
> 
> goto? Really?

yep;
personally, i used to not like it all that much up until a few years, but sometimes it feels it can help with creating
cleaner patches in certain contexts;

i'll re-spin without it;

> 
> > +	}
> > +
> > +	reg |= ADIN1300_GE_RMII_EN;
> > +
> > +write:
> > +	return phy_write_mmd(phydev, MDIO_MMD_VEND1,
> > +			     ADIN1300_GE_RMII_CFG_REG, reg);
> > +}
> > +
> >  static int adin_config_init(struct phy_device *phydev)
> >  {
> > -	int rc;
> > +	phy_interface_t interface, rc;
> 
> genphy_config_init() does not return a phy_interface_t!

good point;
will check;

> 
> >  
> >  	rc = genphy_config_init(phydev);
> >  	if (rc < 0)
> >  		return rc;
> >  
> > +	interface = phydev->interface;
> > +
> > +	rc = adin_config_rgmii_mode(phydev, interface);
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	rc = adin_config_rmii_mode(phydev, interface);
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	dev_info(&phydev->mdio.dev, "PHY is using mode '%s'\n",
> > +		 phy_modes(phydev->interface));
> 
> phydev_dbg(), or not at all.

ack

> 
> 	      Andrew

^ permalink raw reply

* Re: [PATCH iproute2-next] rdma: Add driver QP type string
From: Gal Pressman @ 2019-08-06  6:41 UTC (permalink / raw)
  To: David Ahern, Stephen Hemminger; +Cc: netdev, linux-rdma, Leon Romanovsky
In-Reply-To: <fd623a4e-d076-3eea-2d1e-7702812b0dfc@gmail.com>

On 05/08/2019 22:08, David Ahern wrote:
> On 8/4/19 2:07 AM, Gal Pressman wrote:
>> RDMA resource tracker now tracks driver QPs as well, add driver QP type
>> string to qp_types_to_str function.
> 
> "now" means which kernel release? Leon: should this be in master or -next?

Now means the patch is merged to RDMA's for-rc branch (5.3).

^ permalink raw reply

* Re: [PATCH 04/16] net: phy: adin: add {write,read}_mmd hooks
From: Ardelean, Alexandru @ 2019-08-06  6:38 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805142513.GK24275@lunn.ch>

On Mon, 2019-08-05 at 16:25 +0200, Andrew Lunn wrote:
> [External]
> 
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > index b75c723bda79..3dd9fe50f4c8 100644
> > --- a/drivers/net/phy/adin.c
> > +++ b/drivers/net/phy/adin.c
> > @@ -14,6 +14,9 @@
> >  #define PHY_ID_ADIN1200				0x0283bc20
> >  #define PHY_ID_ADIN1300				0x0283bc30
> >  
> > +#define ADIN1300_MII_EXT_REG_PTR		0x10
> > +#define ADIN1300_MII_EXT_REG_DATA		0x11
> > +
> >  #define ADIN1300_INT_MASK_REG			0x0018
> 
> Please be consistent with registers. Either use 4 digits, or 2 digits.

ack;

> 
>        Andrew

^ permalink raw reply

* Re: [PATCH 03/16] net: phy: adin: add support for interrupts
From: Ardelean, Alexandru @ 2019-08-06  6:38 UTC (permalink / raw)
  To: devicetree@vger.kernel.org, hkallweit1@gmail.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: f.fainelli@gmail.com, davem@davemloft.net, mark.rutland@arm.com,
	robh+dt@kernel.org, andrew@lunn.ch
In-Reply-To: <4f539572-4c59-0450-fcd4-0bbc3eece9c8@gmail.com>

On Mon, 2019-08-05 at 23:02 +0200, Heiner Kallweit wrote:
> [External]
> 
> On 05.08.2019 18:54, Alexandru Ardelean wrote:
> > This change adds support for enabling PHY interrupts that can be used by
> > the PHY framework to get signal for link/speed/auto-negotiation changes.
> > 
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >  drivers/net/phy/adin.c | 44 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> > 
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > index c100a0dd95cd..b75c723bda79 100644
> > --- a/drivers/net/phy/adin.c
> > +++ b/drivers/net/phy/adin.c
> > @@ -14,6 +14,22 @@
> >  #define PHY_ID_ADIN1200				0x0283bc20
> >  #define PHY_ID_ADIN1300				0x0283bc30
> >  
> > +#define ADIN1300_INT_MASK_REG			0x0018
> > +#define   ADIN1300_INT_MDIO_SYNC_EN		BIT(9)
> > +#define   ADIN1300_INT_ANEG_STAT_CHNG_EN	BIT(8)
> > +#define   ADIN1300_INT_ANEG_PAGE_RX_EN		BIT(6)
> > +#define   ADIN1300_INT_IDLE_ERR_CNT_EN		BIT(5)
> > +#define   ADIN1300_INT_MAC_FIFO_OU_EN		BIT(4)
> > +#define   ADIN1300_INT_RX_STAT_CHNG_EN		BIT(3)
> > +#define   ADIN1300_INT_LINK_STAT_CHNG_EN	BIT(2)
> > +#define   ADIN1300_INT_SPEED_CHNG_EN		BIT(1)
> > +#define   ADIN1300_INT_HW_IRQ_EN		BIT(0)
> > +#define ADIN1300_INT_MASK_EN	\
> > +	(ADIN1300_INT_ANEG_STAT_CHNG_EN | ADIN1300_INT_ANEG_PAGE_RX_EN | \
> > +	 ADIN1300_INT_LINK_STAT_CHNG_EN | ADIN1300_INT_SPEED_CHNG_EN | \
> > +	 ADIN1300_INT_HW_IRQ_EN)
> > +#define ADIN1300_INT_STATUS_REG			0x0019
> > +
> >  static int adin_config_init(struct phy_device *phydev)
> >  {
> >  	int rc;
> > @@ -25,15 +41,40 @@ static int adin_config_init(struct phy_device *phydev)
> >  	return 0;
> >  }
> >  
> > +static int adin_phy_ack_intr(struct phy_device *phydev)
> > +{
> > +	int ret;
> > +
> > +	/* Clear pending interrupts.  */
> > +	ret = phy_read(phydev, ADIN1300_INT_STATUS_REG);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +
> > +static int adin_phy_config_intr(struct phy_device *phydev)
> > +{
> > +	if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
> > +		return phy_set_bits(phydev, ADIN1300_INT_MASK_REG,
> > +				    ADIN1300_INT_MASK_EN);
> > +
> > +	return phy_clear_bits(phydev, ADIN1300_INT_MASK_REG,
> > +			      ADIN1300_INT_MASK_EN);
> > +}
> > +
> >  static struct phy_driver adin_driver[] = {
> >  	{
> >  		.phy_id		= PHY_ID_ADIN1200,
> >  		.name		= "ADIN1200",
> >  		.phy_id_mask	= 0xfffffff0,
> >  		.features	= PHY_BASIC_FEATURES,
> > +		.flags		= PHY_HAS_INTERRUPT,
> 
> This flag doesn't exist any longer. This indicates that you
> develop against an older kernel version. Please develop
> against net-next. Check up-to-date drivers like the one
> for Realtek PHY's for hints.

ack;

> 
> >  		.config_init	= adin_config_init,
> >  		.config_aneg	= genphy_config_aneg,
> >  		.read_status	= genphy_read_status,
> > +		.ack_interrupt	= adin_phy_ack_intr,
> > +		.config_intr	= adin_phy_config_intr,
> >  		.resume		= genphy_resume,
> >  		.suspend	= genphy_suspend,
> >  	},
> > @@ -42,9 +83,12 @@ static struct phy_driver adin_driver[] = {
> >  		.name		= "ADIN1300",
> >  		.phy_id_mask	= 0xfffffff0,
> >  		.features	= PHY_GBIT_FEATURES,
> > +		.flags		= PHY_HAS_INTERRUPT,
> >  		.config_init	= adin_config_init,
> >  		.config_aneg	= genphy_config_aneg,
> >  		.read_status	= genphy_read_status,
> > +		.ack_interrupt	= adin_phy_ack_intr,
> > +		.config_intr	= adin_phy_config_intr,
> >  		.resume		= genphy_resume,
> >  		.suspend	= genphy_suspend,
> >  	},
> > 

^ permalink raw reply

* Re: [PATCH 03/16] net: phy: adin: add support for interrupts
From: Ardelean, Alexandru @ 2019-08-06  6:37 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805142123.GJ24275@lunn.ch>

On Mon, 2019-08-05 at 16:21 +0200, Andrew Lunn wrote:
> [External]
> 
> On Mon, Aug 05, 2019 at 07:54:40PM +0300, Alexandru Ardelean wrote:
> > This change adds support for enabling PHY interrupts that can be used by
> > the PHY framework to get signal for link/speed/auto-negotiation changes.
> > 
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >  drivers/net/phy/adin.c | 44 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> > 
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > index c100a0dd95cd..b75c723bda79 100644
> > --- a/drivers/net/phy/adin.c
> > +++ b/drivers/net/phy/adin.c
> > @@ -14,6 +14,22 @@
> >  #define PHY_ID_ADIN1200				0x0283bc20
> >  #define PHY_ID_ADIN1300				0x0283bc30
> >  
> > +#define ADIN1300_INT_MASK_REG			0x0018
> > +#define   ADIN1300_INT_MDIO_SYNC_EN		BIT(9)
> > +#define   ADIN1300_INT_ANEG_STAT_CHNG_EN	BIT(8)
> > +#define   ADIN1300_INT_ANEG_PAGE_RX_EN		BIT(6)
> > +#define   ADIN1300_INT_IDLE_ERR_CNT_EN		BIT(5)
> > +#define   ADIN1300_INT_MAC_FIFO_OU_EN		BIT(4)
> > +#define   ADIN1300_INT_RX_STAT_CHNG_EN		BIT(3)
> > +#define   ADIN1300_INT_LINK_STAT_CHNG_EN	BIT(2)
> > +#define   ADIN1300_INT_SPEED_CHNG_EN		BIT(1)
> > +#define   ADIN1300_INT_HW_IRQ_EN		BIT(0)
> > +#define ADIN1300_INT_MASK_EN	\
> > +	(ADIN1300_INT_ANEG_STAT_CHNG_EN | ADIN1300_INT_ANEG_PAGE_RX_EN | \
> > +	 ADIN1300_INT_LINK_STAT_CHNG_EN | ADIN1300_INT_SPEED_CHNG_EN | \
> > +	 ADIN1300_INT_HW_IRQ_EN)
> > +#define ADIN1300_INT_STATUS_REG			0x0019
> > +
> >  static int adin_config_init(struct phy_device *phydev)
> >  {
> >  	int rc;
> > @@ -25,15 +41,40 @@ static int adin_config_init(struct phy_device *phydev)
> >  	return 0;
> >  }
> >  
> > +static int adin_phy_ack_intr(struct phy_device *phydev)
> > +{
> > +	int ret;
> > +
> > +	/* Clear pending interrupts.  */
> > +	ret = phy_read(phydev, ADIN1300_INT_STATUS_REG);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return 0;
> 
> Please go through the whole driver and throw out all the needless

ack;
i'll re-visit;

> 
> 	if (ret < 0)
> 		return ret;
> 
> 	return 0;
> 
> Thanks
> 	Andrew

^ permalink raw reply

* Re: [PATCH 01/16] net: phy: adin: add support for Analog Devices PHYs
From: Ardelean, Alexandru @ 2019-08-06  6:35 UTC (permalink / raw)
  To: devicetree@vger.kernel.org, hkallweit1@gmail.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: f.fainelli@gmail.com, davem@davemloft.net, mark.rutland@arm.com,
	robh+dt@kernel.org, andrew@lunn.ch
In-Reply-To: <206ec97f-3115-9a2c-91a0-e5f7aec4a39e@gmail.com>

On Mon, 2019-08-05 at 22:54 +0200, Heiner Kallweit wrote:
> [External]
> 
> On 05.08.2019 18:54, Alexandru Ardelean wrote:
> > This change adds support for Analog Devices Industrial Ethernet PHYs.
> > Particularly the PHYs this driver adds support for:
> >  * ADIN1200 - Robust, Industrial, Low Power 10/100 Ethernet PHY
> >  * ADIN1300 - Robust, Industrial, Low Latency 10/100/1000 Gigabit
> >    Ethernet PHY
> > 
> > The 2 chips are pin & register compatible with one another. The main
> > difference being that ADIN1200 doesn't operate in gigabit mode.
> > 
> > The chips can be operated by the Generic PHY driver as well via the
> > standard IEEE PHY registers (0x0000 - 0x000F) which are supported by the
> > kernel as well. This assumes that configuration of the PHY has been done
> > required.
> > 
> > Configuration can also be done via registers, which will be implemented by
> > the driver in the next changes.
> > 
> > Datasheets:
> >   https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1300.pdf
> >   https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1200.pdf
> > 
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >  MAINTAINERS              |  7 +++++
> >  drivers/net/phy/Kconfig  |  9 ++++++
> >  drivers/net/phy/Makefile |  1 +
> >  drivers/net/phy/adin.c   | 59 ++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 76 insertions(+)
> >  create mode 100644 drivers/net/phy/adin.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ee663e0e2f2e..faf5723610c8 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -938,6 +938,13 @@ S:	Supported
> >  F:	drivers/mux/adgs1408.c
> >  F:	Documentation/devicetree/bindings/mux/adi,adgs1408.txt
> >  
> > +ANALOG DEVICES INC ADIN DRIVER
> > +M:	Alexandru Ardelean <alexaundru.ardelean@analog.com>
> > +L:	netdev@vger.kernel.org
> > +W:	http://ez.analog.com/community/linux-device-drivers
> > +S:	Supported
> > +F:	drivers/net/phy/adin.c
> > +
> >  ANALOG DEVICES INC ADIS DRIVER LIBRARY
> >  M:	Alexandru Ardelean <alexandru.ardelean@analog.com>
> >  S:	Supported
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 206d8650ee7f..5966d3413676 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -257,6 +257,15 @@ config SFP
> >  	depends on HWMON || HWMON=n
> >  	select MDIO_I2C
> >  
> > +config ADIN_PHY
> > +	tristate "Analog Devices Industrial Ethernet PHYs"
> > +	help
> > +	  Adds support for the Analog Devices Industrial Ethernet PHYs.
> > +	  Currently supports the:
> > +	  - ADIN1200 - Robust,Industrial, Low Power 10/100 Ethernet PHY
> > +	  - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
> > +	    Ethernet PHY
> > +
> >  config AMD_PHY
> >  	tristate "AMD PHYs"
> >  	---help---
> > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> > index ba07c27e4208..a03437e091f3 100644
> > --- a/drivers/net/phy/Makefile
> > +++ b/drivers/net/phy/Makefile
> > @@ -47,6 +47,7 @@ obj-$(CONFIG_SFP)		+= sfp.o
> >  sfp-obj-$(CONFIG_SFP)		+= sfp-bus.o
> >  obj-y				+= $(sfp-obj-y) $(sfp-obj-m)
> >  
> > +obj-$(CONFIG_ADIN_PHY)		+= adin.o
> >  obj-$(CONFIG_AMD_PHY)		+= amd.o
> >  aquantia-objs			+= aquantia_main.o
> >  ifdef CONFIG_HWMON
> > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> > new file mode 100644
> > index 000000000000..6a610d4563c3
> > --- /dev/null
> > +++ b/drivers/net/phy/adin.c
> > @@ -0,0 +1,59 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/**
> > + *  Driver for Analog Devices Industrial Ethernet PHYs
> > + *
> > + * Copyright 2019 Analog Devices Inc.
> > + */
> > +#include <linux/kernel.h>
> > +#include <linux/errno.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/mii.h>
> > +#include <linux/phy.h>
> > +
> > +#define PHY_ID_ADIN1200				0x0283bc20
> > +#define PHY_ID_ADIN1300				0x0283bc30
> > +
> > +static int adin_config_init(struct phy_device *phydev)
> > +{
> > +	int rc;
> > +
> > +	rc = genphy_config_init(phydev);
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	return 0;
> > +}
> > +
> > +static struct phy_driver adin_driver[] = {
> > +	{
> > +		.phy_id		= PHY_ID_ADIN1200,
> 
> You could use PHY_ID_MATCH_MODEL here.
> 
> > +		.name		= "ADIN1200",
> > +		.phy_id_mask	= 0xfffffff0,
> > +		.features	= PHY_BASIC_FEATURES,
> 
> Setting features is deprecated, instead the get_features callback
> should be implemented if the default genphy_read_abilities needs
> to be extended / replaced. You say that the PHY's work with the
> genphy driver, so I suppose the default feature detection is ok
> in your case. Then you could simply remove setting "features".

ack;
thanks for the info

> 
> > +		.config_init	= adin_config_init,
> > +		.config_aneg	= genphy_config_aneg,
> > +		.read_status	= genphy_read_status,
> > +	},
> > +	{
> > +		.phy_id		= PHY_ID_ADIN1300,
> > +		.name		= "ADIN1300",
> > +		.phy_id_mask	= 0xfffffff0,
> > +		.features	= PHY_GBIT_FEATURES,
> > +		.config_init	= adin_config_init,
> > +		.config_aneg	= genphy_config_aneg,
> > +		.read_status	= genphy_read_status,
> > +	},
> > +};
> > +
> > +module_phy_driver(adin_driver);
> > +
> > +static struct mdio_device_id __maybe_unused adin_tbl[] = {
> > +	{ PHY_ID_ADIN1200, 0xfffffff0 },
> > +	{ PHY_ID_ADIN1300, 0xfffffff0 },
> 
> PHY_ID_MATCH_MODEL could be used here too.

ack;
will take a look

> 
> > +	{ }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(mdio, adin_tbl);
> > +MODULE_DESCRIPTION("Analog Devices Industrial Ethernet PHY driver");
> > +MODULE_LICENSE("GPL");
> > 

^ permalink raw reply

* Re: [PATCH 01/16] net: phy: adin: add support for Analog Devices PHYs
From: Ardelean, Alexandru @ 2019-08-06  6:35 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805151736.GQ24275@lunn.ch>

On Mon, 2019-08-05 at 17:17 +0200, Andrew Lunn wrote:
> [External]
> 
> > +static struct phy_driver adin_driver[] = {
> > +	{
> > +		.phy_id		= PHY_ID_ADIN1200,
> > +		.name		= "ADIN1200",
> > +		.phy_id_mask	= 0xfffffff0,
> > +		.features	= PHY_BASIC_FEATURES,
> 
> Do you need this? If the device implements the registers correctly,
> phylib can determine this from the registers.

ack;
will take a look;

> 
> > +		.config_init	= adin_config_init,
> > +		.config_aneg	= genphy_config_aneg,
> > +		.read_status	= genphy_read_status,
> > +	},
> > +	{
> > +		.phy_id		= PHY_ID_ADIN1300,
> > +		.name		= "ADIN1300",
> > +		.phy_id_mask	= 0xfffffff0,
> > +		.features	= PHY_GBIT_FEATURES,
> 
> same here.

ack;

> 
> > +		.config_init	= adin_config_init,
> > +		.config_aneg	= genphy_config_aneg,
> > +		.read_status	= genphy_read_status,
> > +	},
> > +};
> > +
> > +module_phy_driver(adin_driver);
> > +
> > +static struct mdio_device_id __maybe_unused adin_tbl[] = {
> > +	{ PHY_ID_ADIN1200, 0xfffffff0 },
> > +	{ PHY_ID_ADIN1300, 0xfffffff0 },
> 
> PHY_ID_MATCH_VENDOR().

ack;

> 
> 	Andrew

^ permalink raw reply

* Re: [PATCH 01/16] net: phy: adin: add support for Analog Devices PHYs
From: Ardelean, Alexandru @ 2019-08-06  6:32 UTC (permalink / raw)
  To: andrew@lunn.ch
  Cc: davem@davemloft.net, hkallweit1@gmail.com,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
	netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <20190805141644.GH24275@lunn.ch>

On Mon, 2019-08-05 at 16:16 +0200, Andrew Lunn wrote:
> [External]
> 
> > +static int adin_config_init(struct phy_device *phydev)
> > +{
> > +	int rc;
> > +
> > +	rc = genphy_config_init(phydev);
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	return 0;
> > +}
> 
> Why not just
> 
>     return genphy_config_init(phydev);

Because stuff will get added after this return statement in the next patches.
I thought maybe this would be a good idea to keep the git changes minimal, but I can do a direct return and update it in
the next patches when needed.

> 
>     Andrew
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox