Netdev List
 help / color / mirror / Atom feed
* [PATCH net 4/6] bnxt_en: Suppress HWRM errors for HWRM_NVM_GET_VARIABLE command
From: Michael Chan @ 2019-08-16 22:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, Vasundhara Volam
In-Reply-To: <1565994817-6328-1-git-send-email-michael.chan@broadcom.com>

From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

For newly added NVM parameters, older firmware may not have the support.
Suppress the error message to avoid the unncessary error message which is
triggered when devlink calls the driver during initialization.

Fixes: 782a624d00fa ("bnxt_en: Add bnxt_en initial params table and register it.")
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 549c90d3..c05d663 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -98,10 +98,13 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 	if (idx)
 		req->dimensions = cpu_to_le16(1);
 
-	if (req->req_type == cpu_to_le16(HWRM_NVM_SET_VARIABLE))
+	if (req->req_type == cpu_to_le16(HWRM_NVM_SET_VARIABLE)) {
 		memcpy(data_addr, buf, bytesize);
-
-	rc = hwrm_send_message(bp, msg, msg_len, HWRM_CMD_TIMEOUT);
+		rc = hwrm_send_message(bp, msg, msg_len, HWRM_CMD_TIMEOUT);
+	} else {
+		rc = hwrm_send_message_silent(bp, msg, msg_len,
+					      HWRM_CMD_TIMEOUT);
+	}
 	if (!rc && req->req_type == cpu_to_le16(HWRM_NVM_GET_VARIABLE))
 		memcpy(buf, data_addr, bytesize);
 
-- 
2.5.1


^ permalink raw reply related

* [PATCH net 3/6] bnxt_en: Fix handling FRAG_ERR when NVM_INSTALL_UPDATE cmd fails
From: Michael Chan @ 2019-08-16 22:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, Vasundhara Volam
In-Reply-To: <1565994817-6328-1-git-send-email-michael.chan@broadcom.com>

From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

If FW returns FRAG_ERR in response error code, driver is resending the
command only when HWRM command returns success. Fix the code to resend
NVM_INSTALL_UPDATE command with DEFRAG install flags, if FW returns
FRAG_ERR in its response error code.

Fixes: cb4d1d626145 ("bnxt_en: Retry failed NVM_INSTALL_UPDATE with defragmentation flag enabled.")
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index c7ee63d..8445a0c 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -2016,21 +2016,19 @@ static int bnxt_flash_package_from_file(struct net_device *dev,
 	mutex_lock(&bp->hwrm_cmd_lock);
 	hwrm_err = _hwrm_send_message(bp, &install, sizeof(install),
 				      INSTALL_PACKAGE_TIMEOUT);
-	if (hwrm_err)
-		goto flash_pkg_exit;
-
-	if (resp->error_code) {
+	if (hwrm_err) {
 		u8 error_code = ((struct hwrm_err_output *)resp)->cmd_err;
 
-		if (error_code == NVM_INSTALL_UPDATE_CMD_ERR_CODE_FRAG_ERR) {
+		if (resp->error_code && error_code ==
+		    NVM_INSTALL_UPDATE_CMD_ERR_CODE_FRAG_ERR) {
 			install.flags |= cpu_to_le16(
 			       NVM_INSTALL_UPDATE_REQ_FLAGS_ALLOWED_TO_DEFRAG);
 			hwrm_err = _hwrm_send_message(bp, &install,
 						      sizeof(install),
 						      INSTALL_PACKAGE_TIMEOUT);
-			if (hwrm_err)
-				goto flash_pkg_exit;
 		}
+		if (hwrm_err)
+			goto flash_pkg_exit;
 	}
 
 	if (resp->result) {
-- 
2.5.1


^ permalink raw reply related

* [PATCH net 2/6] bnxt_en: Improve RX doorbell sequence.
From: Michael Chan @ 2019-08-16 22:33 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <1565994817-6328-1-git-send-email-michael.chan@broadcom.com>

When both RX buffers and RX aggregation buffers have to be
replenished at the end of NAPI, post the RX aggregation buffers first
before RX buffers.  Otherwise, we may run into a situation where
there are only RX buffers without RX aggregation buffers for a split
second.  This will cause the hardware to abort the RX packet and
report buffer errors, which will cause unnecessary cleanup by the
driver.

Ringing the Aggregation ring doorbell first before the RX ring doorbell
will prevent some of these buffer errors.  Use the same sequence during
ring initialization as well.

Fixes: 697197e5a173 ("bnxt_en: Re-structure doorbells.")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 1ef224f..8dce406 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -2021,9 +2021,9 @@ static void __bnxt_poll_work_done(struct bnxt *bp, struct bnxt_napi *bnapi)
 	if (bnapi->events & BNXT_RX_EVENT) {
 		struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
 
-		bnxt_db_write(bp, &rxr->rx_db, rxr->rx_prod);
 		if (bnapi->events & BNXT_AGG_EVENT)
 			bnxt_db_write(bp, &rxr->rx_agg_db, rxr->rx_agg_prod);
+		bnxt_db_write(bp, &rxr->rx_db, rxr->rx_prod);
 	}
 	bnapi->events = 0;
 }
@@ -5064,6 +5064,7 @@ static void bnxt_set_db(struct bnxt *bp, struct bnxt_db_info *db, u32 ring_type,
 
 static int bnxt_hwrm_ring_alloc(struct bnxt *bp)
 {
+	bool agg_rings = !!(bp->flags & BNXT_FLAG_AGG_RINGS);
 	int i, rc = 0;
 	u32 type;
 
@@ -5139,7 +5140,9 @@ static int bnxt_hwrm_ring_alloc(struct bnxt *bp)
 		if (rc)
 			goto err_out;
 		bnxt_set_db(bp, &rxr->rx_db, type, map_idx, ring->fw_ring_id);
-		bnxt_db_write(bp, &rxr->rx_db, rxr->rx_prod);
+		/* If we have agg rings, post agg buffers first. */
+		if (!agg_rings)
+			bnxt_db_write(bp, &rxr->rx_db, rxr->rx_prod);
 		bp->grp_info[map_idx].rx_fw_ring_id = ring->fw_ring_id;
 		if (bp->flags & BNXT_FLAG_CHIP_P5) {
 			struct bnxt_cp_ring_info *cpr = &bnapi->cp_ring;
@@ -5158,7 +5161,7 @@ static int bnxt_hwrm_ring_alloc(struct bnxt *bp)
 		}
 	}
 
-	if (bp->flags & BNXT_FLAG_AGG_RINGS) {
+	if (agg_rings) {
 		type = HWRM_RING_ALLOC_AGG;
 		for (i = 0; i < bp->rx_nr_rings; i++) {
 			struct bnxt_rx_ring_info *rxr = &bp->rx_ring[i];
@@ -5174,6 +5177,7 @@ static int bnxt_hwrm_ring_alloc(struct bnxt *bp)
 			bnxt_set_db(bp, &rxr->rx_agg_db, type, map_idx,
 				    ring->fw_ring_id);
 			bnxt_db_write(bp, &rxr->rx_agg_db, rxr->rx_agg_prod);
+			bnxt_db_write(bp, &rxr->rx_db, rxr->rx_prod);
 			bp->grp_info[grp_idx].agg_fw_ring_id = ring->fw_ring_id;
 		}
 	}
-- 
2.5.1


^ permalink raw reply related

* [PATCH net 1/6] bnxt_en: Fix VNIC clearing logic for 57500 chips.
From: Michael Chan @ 2019-08-16 22:33 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <1565994817-6328-1-git-send-email-michael.chan@broadcom.com>

During device shutdown, the VNIC clearing sequence needs to be modified
to free the VNIC first before freeing the RSS contexts.  The current
code is doing the reverse and we can get mis-directed RX completions
to CP ring ID 0 when the RSS contexts are freed and zeroed.  These
mis-directed packets may cause the driver to crash.  The clearing
of RSS contexts is not required with the new sequence.

Refactor the VNCI clearing logic into a new function bnxt_clear_vnic()
and do the chip specific VNIC clearing sequence.

Fixes: 7b3af4f75b81 ("bnxt_en: Add RSS support for 57500 chips.")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 7070349..1ef224f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7016,19 +7016,29 @@ static void bnxt_hwrm_clear_vnic_rss(struct bnxt *bp)
 		bnxt_hwrm_vnic_set_rss(bp, i, false);
 }
 
-static void bnxt_hwrm_resource_free(struct bnxt *bp, bool close_path,
-				    bool irq_re_init)
+static void bnxt_clear_vnic(struct bnxt *bp)
 {
-	if (bp->vnic_info) {
-		bnxt_hwrm_clear_vnic_filter(bp);
+	if (!bp->vnic_info)
+		return;
+
+	bnxt_hwrm_clear_vnic_filter(bp);
+	if (!(bp->flags & BNXT_FLAG_CHIP_P5)) {
 		/* clear all RSS setting before free vnic ctx */
 		bnxt_hwrm_clear_vnic_rss(bp);
 		bnxt_hwrm_vnic_ctx_free(bp);
-		/* before free the vnic, undo the vnic tpa settings */
-		if (bp->flags & BNXT_FLAG_TPA)
-			bnxt_set_tpa(bp, false);
-		bnxt_hwrm_vnic_free(bp);
 	}
+	/* before free the vnic, undo the vnic tpa settings */
+	if (bp->flags & BNXT_FLAG_TPA)
+		bnxt_set_tpa(bp, false);
+	bnxt_hwrm_vnic_free(bp);
+	if (bp->flags & BNXT_FLAG_CHIP_P5)
+		bnxt_hwrm_vnic_ctx_free(bp);
+}
+
+static void bnxt_hwrm_resource_free(struct bnxt *bp, bool close_path,
+				    bool irq_re_init)
+{
+	bnxt_clear_vnic(bp);
 	bnxt_hwrm_ring_free(bp, close_path);
 	bnxt_hwrm_ring_grp_free(bp);
 	if (irq_re_init) {
-- 
2.5.1


^ permalink raw reply related

* [PATCH net 0/6] bnxt_en: Bug fixes.
From: Michael Chan @ 2019-08-16 22:33 UTC (permalink / raw)
  To: davem; +Cc: netdev

2 Bug fixes related to 57500 shutdown sequence and doorbell sequence,
2 TC Flower bug fixes related to the setting of the flow direction,
1 NVRAM update bug fix, and a minor fix to suppress an unnecessary
error message.  Please queue for -stable as well.  Thanks.

Michael Chan (2):
  bnxt_en: Fix VNIC clearing logic for 57500 chips.
  bnxt_en: Improve RX doorbell sequence.

Somnath Kotur (1):
  bnxt_en: Fix to include flow direction in L2 key

Vasundhara Volam (2):
  bnxt_en: Fix handling FRAG_ERR when NVM_INSTALL_UPDATE cmd fails
  bnxt_en: Suppress HWRM errors for HWRM_NVM_GET_VARIABLE command

Venkat Duvvuru (1):
  bnxt_en: Use correct src_fid to determine direction of the flow

 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 36 ++++++++++++++++-------
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c |  9 ++++--
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 12 ++++----
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c      |  8 ++---
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.h      |  2 +-
 5 files changed, 40 insertions(+), 27 deletions(-)

-- 
2.5.1


^ permalink raw reply

* Re: [PATCH net-next] selftests: Fix get_ifidx and callers in nettest.c
From: David Miller @ 2019-08-16 22:25 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, dan.carpenter, dsahern
In-Reply-To: <20190814171151.27739-1-dsahern@kernel.org>

From: David Ahern <dsahern@kernel.org>
Date: Wed, 14 Aug 2019 10:11:51 -0700

> From: David Ahern <dsahern@gmail.com>
> 
> Dan reported:
 ...
> Update get_ifidx to return -1 on errors and cleanup callers of it.
> 
> Fixes: acda655fefae ("selftests: Add nettest")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: David Ahern <dsahern@gmail.com>

Applied, thanks David.

^ permalink raw reply

* Re: [PATCH] lan78xx: Fix memory leaks
From: David Miller @ 2019-08-16 22:24 UTC (permalink / raw)
  To: wenwen; +Cc: woojung.huh, UNGLinuxDriver, netdev, linux-usb, linux-kernel
In-Reply-To: <1565799793-7446-1-git-send-email-wenwen@cs.uga.edu>

From: Wenwen Wang <wenwen@cs.uga.edu>
Date: Wed, 14 Aug 2019 11:23:13 -0500

> In lan78xx_probe(), a new urb is allocated through usb_alloc_urb() and
> saved to 'dev->urb_intr'. However, in the following execution, if an error
> occurs, 'dev->urb_intr' is not deallocated, leading to memory leaks. To fix
> this issue, invoke usb_free_urb() to free the allocated urb before
> returning from the function.
> 
> Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>

Applied.

^ permalink raw reply

* Re: regression in ath10k dma allocation
From: Nicolin Chen @ 2019-08-16 22:25 UTC (permalink / raw)
  To: Tobias Klausmann
  Cc: Christoph Hellwig, kvalo, davem, ath10k, linux-wireless, netdev,
	linux-kernel, m.szyprowski, robin.murphy, iommu, tobias.klausmann
In-Reply-To: <af96ea6a-2b17-9b66-7aba-b7dae5bcbba5@mni.thm.de>

Hi Tobias

On Fri, Aug 16, 2019 at 10:16:45PM +0200, Tobias Klausmann wrote:
> > do you have CONFIG_DMA_CMA set in your config?  If not please make sure
> > you have this commit in your testing tree, and if the problem still
> > persists it would be a little odd and we'd have to dig deeper:
> > 
> > commit dd3dcede9fa0a0b661ac1f24843f4a1b1317fdb6
> > Author: Nicolin Chen <nicoleotsuka@gmail.com>
> > Date:   Wed May 29 17:54:25 2019 -0700
> > 
> >      dma-contiguous: fix !CONFIG_DMA_CMA version of dma_{alloc, free}_contiguous()

> yes CONFIG_DMA_CMA is set (=y, see attached config), the commit you mention
> above is included, if you have any hints how to go forward, please let me
> know!

For CONFIG_DMA_CMA=y, by judging the log with error code -12, I
feel this one should work for you. Would you please check if it
is included or try it out otherwise?

dma-contiguous: do not overwrite align in dma_alloc_contiguous()
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c6622a425acd1d2f3a443cd39b490a8777b622d7

Thanks
Nicolin

^ permalink raw reply

* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Christian Brauner @ 2019-08-16 22:22 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andy Lutomirski, Kees Cook, Andy Lutomirski, Song Liu, Networking,
	bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team,
	Lorenz Bauer, Jann Horn, Greg KH, Linux API, LSM List
In-Reply-To: <20190816214542.inpt6p655whc2ejw@ast-mbp.dhcp.thefacebook.com>

On Fri, Aug 16, 2019 at 02:45:44PM -0700, Alexei Starovoitov wrote:
> On Thu, Aug 15, 2019 at 05:54:59PM -0700, Andy Lutomirski wrote:
> > 
> > 
> > > On Aug 15, 2019, at 4:46 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > 
> > 
> > >> 
> > >> I'm not sure why you draw the line for VMs -- they're just as buggy
> > >> as anything else. Regardless, I reject this line of thinking: yes,
> > >> all software is buggy, but that isn't a reason to give up.
> > > 
> > > hmm. are you saying you want kernel community to work towards
> > > making containers (namespaces) being able to run arbitrary code
> > > downloaded from the internet?
> > 
> > Yes.

If I may weigh in here too: Yes. In fact, we already do that large
scale!

> > 
> > As an example, Sandstorm uses a combination of namespaces (user, network, mount, ipc) and a moderately permissive seccomp policy to run arbitrary code. Not just little snippets, either — node.js, Mongo, MySQL, Meteor, and other fairly heavyweight stacks can all run under Sandstorm, with the whole stack (database engine binaries, etc) supplied by entirely untrusted customers.  During the time Sandstorm was under active development, I can recall *one* bug that would have allowed a sandbox escape. That’s a pretty good track record.  (Also, Meltdown and Spectre, sigh.)
> 
> exactly: "meltdown", "spectre", "sigh".
> Side channels effectively stalled the work on secure containers.
> And killed projects like sandstorm.

If I may, Sandstorm's death has very likely nothing to do with
Meltdown/Spectre etc. since that should've also killed Qemu, Crosvm and
all the others in one fell swoop. It's also not a very good example (no
offense, Andy :)) and probably a bit dated.
We have LXD which is a full-fledged container manager that runs
*unprivileged system* containers on a large scale and is very much
alive. That is it runs systemd, openrc, what have you, i.e. simply
unmodifed distro images at this point.

It's used to run Linux workloads on all Chromebooks and in a bunch of
other places. Since its inception we did not have a single
*unprivileged* container to init userns/host breakout.
At this point in time the really bad CVEs are almost exclusively against
*privileged* containers (see this year's leading nomination for
container CVE grand mal of the year: CVE-2019-5736) which were never and
will never be considered root safe despite everyone pretending
otherwise.


> Why work on improving kaslr when there are several ways to
> get kernel addresses through hw bugs? Patch mouse holes when roof is leaking ?
> In case of unprivileged bpf I'm confident that all known holes are patched.
> Until disclosures stop happening with the frequency they do now the time
> of bpf developers is better spent on something other than unprivileged bpf.
> 
> > I’m suggesting that you engage the security community ...
> > .. so that normal users can use bpf filtering
> 
> yes, but not soon. unfortunately.

Tbh, I do not have a strong opinion on unprivileged bpf at this moment.
If the community thinks that the bits and pieces are not in place or the
timing is not right that's fine with me.
What we should make sure though is that we don't end up with something
halfbaked. And this /dev/bpf thing very much feels like a hack. If
unprivileged bpf is not a thing why bother with /dev/bpf or CAP_BPF at
all.

(The one usecase I'd care about is to extend seccomp to do pointer-based
syscall filtering. Whether or not that'd require (unprivileged) ebpf is
up for discussion at KSummit.)

Christian

^ permalink raw reply

* Re: [PATCH v6 4/4] net: phy: realtek: Add LED configuration support for RTL8211E
From: Florian Fainelli @ 2019-08-16 22:12 UTC (permalink / raw)
  To: Matthias Kaehlcke, Pavel Machek
  Cc: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
	Heiner Kallweit, netdev, devicetree, linux-kernel,
	Douglas Anderson
In-Reply-To: <20190816212728.GW250418@google.com>

On 8/16/19 2:27 PM, Matthias Kaehlcke wrote:
> On Fri, Aug 16, 2019 at 10:13:42PM +0200, Pavel Machek wrote:
>> On Tue 2019-08-13 12:11:47, Matthias Kaehlcke wrote:
>>> Add a .config_led hook which is called by the PHY core when
>>> configuration data for a PHY LED is available. Each LED can be
>>> configured to be solid 'off, solid 'on' for certain (or all)
>>> link speeds or to blink on RX/TX activity.
>>>
>>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>>
>> THis really needs to go through the LED subsystem,
> 
> Sorry, I used what get_maintainers.pl threw at me, I should have
> manually cc-ed the LED list.
> 
>> and use the same userland interfaces as the rest of the system.
> 
> With the PHY maintainers we discussed to define a binding that is
> compatible with that of the LED one, to have the option to integrate
> it with the LED subsystem later. The integration itself is beyond the
> scope of this patchset.
> 
> The PHY LED configuration is a low priority for the project I'm
> working on. I wanted to make an attempt to upstream it and spent
> already significantly more time on it than planned, if integration
> with the LED framework now is a requirement please consider this
> series abandonded.

While I have an appreciation for how hard it can be to work in a
corporate environment while doing upstream first and working with
virtually unbounded goals (in time or scope) due to maintainers and
reviewers, that kind of statement can hinder your ability to establish
trust with peers in the community as it can be read as take it or leave it.

The LED subsystem integration can definitively come in later from my 2
cents perspective and this patch series as it stands is valuable and
avoids inventing new bindings.
-- 
Florian

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH bpf-next 0/5] Add support for SKIP_BPF flag for AF_XDP sockets
From: Jonathan Lemon @ 2019-08-16 22:08 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Samudrala, Sridhar, Björn Töpel, Karlsson, Magnus,
	Netdev, bpf, intel-wired-lan, maciej.fijalkowski, tom.herbert
In-Reply-To: <CAJ+HfNj=devuEky3VwbibA-j+o=bKi4Gg=MeHsuuDGAKc9p2Vw@mail.gmail.com>



On 16 Aug 2019, at 6:32, Björn Töpel wrote:

> On Thu, 15 Aug 2019 at 18:46, Samudrala, Sridhar
> <sridhar.samudrala@intel.com> wrote:
>>
>> On 8/15/2019 5:51 AM, Björn Töpel wrote:
>>> On 2019-08-15 05:46, Sridhar Samudrala wrote:
>>>> This patch series introduces XDP_SKIP_BPF flag that can be 
>>>> specified
>>>> during the bind() call of an AF_XDP socket to skip calling the BPF
>>>> program in the receive path and pass the buffer directly to the 
>>>> socket.
>>>>
>>>> When a single AF_XDP socket is associated with a queue and a HW
>>>> filter is used to redirect the packets and the app is interested in
>>>> receiving all the packets on that queue, we don't need an 
>>>> additional
>>>> BPF program to do further filtering or lookup/redirect to a socket.
>>>>
>>>> Here are some performance numbers collected on
>>>>    - 2 socket 28 core Intel(R) Xeon(R) Platinum 8180 CPU @ 2.50GHz
>>>>    - Intel 40Gb Ethernet NIC (i40e)
>>>>
>>>> All tests use 2 cores and the results are in Mpps.
>>>>
>>>> turbo on (default)
>>>> ---------------------------------------------
>>>>                        no-skip-bpf    skip-bpf
>>>> ---------------------------------------------
>>>> rxdrop zerocopy           21.9         38.5
>>>> l2fwd  zerocopy           17.0         20.5
>>>> rxdrop copy               11.1         13.3
>>>> l2fwd  copy                1.9          2.0
>>>>
>>>> no turbo :  echo 1 > /sys/devices/system/cpu/intel_pstate/no_turbo
>>>> ---------------------------------------------
>>>>                        no-skip-bpf    skip-bpf
>>>> ---------------------------------------------
>>>> rxdrop zerocopy           15.4         29.0
>>>> l2fwd  zerocopy           11.8         18.2
>>>> rxdrop copy                8.2         10.5
>>>> l2fwd  copy                1.7          1.7
>>>> ---------------------------------------------
>>>>
>>>
>>> This work is somewhat similar to the XDP_ATTACH work [1]. Avoiding 
>>> the
>>> retpoline in the XDP program call is a nice performance boost! I 
>>> like
>>> the numbers! :-) I also like the idea of adding a flag that just 
>>> does
>>> what most AF_XDP Rx users want -- just getting all packets of a
>>> certain queue into the XDP sockets.
>>>
>>> In addition to Toke's mail, I have some more concerns with the 
>>> series:
>>>
>>> * AFAIU the SKIP_BPF only works for zero-copy enabled sockets. IMO, 
>>> it
>>>    should work for all modes (including XDP_SKB).
>>
>> This patch enables SKIP_BPF for AF_XDP sockets where an XDP program 
>> is
>> attached at driver level (both zerocopy and copy modes)
>> I tried a quick hack to see the perf benefit with generic XDP mode, 
>> but
>> i didn't see any significant improvement in performance in that
>> scenario. so i didn't include that mode.
>>
>>>
>>> * In order to work, a user still needs an XDP program running. 
>>> That's
>>>    clunky. I'd like the behavior that if no XDP program is attached,
>>>    and the option is set, the packets for a that queue end up in the
>>>    socket. If there's an XDP program attached, the program has
>>>    precedence.
>>
>> I think this would require more changes in the drivers to take XDP
>> datapath even when there is no XDP program loaded.
>>
>
> Today, from a driver perspective, to enable XDP you pass a struct
> bpf_prog pointer via the ndo_bpf. The program get executed in
> BPF_PROG_RUN (via bpf_prog_run_xdp) from include/linux/filter.h.
>
> I think it's possible to achieve what you're doing w/o *any* driver
> modification. Pass a special, invalid, pointer to the driver (say
> (void *)0x1 or smth more elegant), which has a special handling in
> BPF_RUN_PROG e.g. setting a per-cpu state and return XDP_REDIRECT. The
> per-cpu state is picked up in xdp_do_redirect and xdp_flush.
>
> An approach like this would be general, and apply to all modes
> automatically.
>
> Thoughts?

All the default program does is check that the map entry contains a xsk,
and call bpf_redirect_map().  So this is pretty much the same as above,
without any special case handling.

Why would this be so expensive?  Is the JIT compilation time being 
counted?
-- 
Jonathan

>
>>>
>>> * It requires changes in all drivers. Not nice, and scales badly. 
>>> Try
>>>    making it generic (xdp_do_redirect/xdp_flush), so it Just Works 
>>> for
>>>    all XDP capable drivers.
>>
>> I tried to make this as generic as possible and make the changes to 
>> the
>> driver very minimal, but could not find a way to avoid any changes at
>> all to the driver. xdp_do_direct() gets called based after the call 
>> to
>> bpf_prog_run_xdp() in the drivers.
>>
>>>
>>> Thanks for working on this!
>>>
>>>
>>> Björn
>>>
>>> [1]
>>> https://lore.kernel.org/netdev/20181207114431.18038-1-bjorn.topel@gmail.com/
>>>
>>>
>>>
>>>> Sridhar Samudrala (5):
>>>>    xsk: Convert bool 'zc' field in struct xdp_umem to a u32 bitmap
>>>>    xsk: Introduce XDP_SKIP_BPF bind option
>>>>    i40e: Enable XDP_SKIP_BPF option for AF_XDP sockets
>>>>    ixgbe: Enable XDP_SKIP_BPF option for AF_XDP sockets
>>>>    xdpsock_user: Add skip_bpf option
>>>>
>>>>   drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 22 +++++++++-
>>>>   drivers/net/ethernet/intel/i40e/i40e_xsk.c    |  6 +++
>>>>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 20 ++++++++-
>>>>   drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  | 16 ++++++-
>>>>   include/net/xdp_sock.h                        | 21 ++++++++-
>>>>   include/uapi/linux/if_xdp.h                   |  1 +
>>>>   include/uapi/linux/xdp_diag.h                 |  1 +
>>>>   net/xdp/xdp_umem.c                            |  9 ++--
>>>>   net/xdp/xsk.c                                 | 43 
>>>> ++++++++++++++++---
>>>>   net/xdp/xsk_diag.c                            |  5 ++-
>>>>   samples/bpf/xdpsock_user.c                    |  8 ++++
>>>>   11 files changed, 135 insertions(+), 17 deletions(-)
>>>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply

* Re: [PATCH v6 1/4] dt-bindings: net: phy: Add subnode for LED configuration
From: Matthias Kaehlcke @ 2019-08-16 22:04 UTC (permalink / raw)
  To: Pavel Machek
  Cc: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
	Florian Fainelli, Heiner Kallweit, netdev, devicetree,
	linux-kernel, Douglas Anderson
In-Reply-To: <20190816201338.GA1646@bug>

On Fri, Aug 16, 2019 at 10:13:38PM +0200, Pavel Machek wrote:
> Hi!
> 
> Please Cc led mailing lists on led issues.

sorry for missing this

> On Tue 2019-08-13 12:11:44, Matthias Kaehlcke wrote:
> > The LED behavior of some Ethernet PHYs is configurable. Add an
> > optional 'leds' subnode with a child node for each LED to be
> > configured. The binding aims to be compatible with the common
> > LED binding (see devicetree/bindings/leds/common.txt).
> > 
> > A LED can be configured to be:
> > 
> > - 'on' when a link is active, some PHYs allow configuration for
> >   certain link speeds
> >   speeds
> > - 'off'
> > - blink on RX/TX activity, some PHYs allow configuration for
> >   certain link speeds
> > 
> > For the configuration to be effective it needs to be supported by
> > the hardware and the corresponding PHY driver.
> > 
> > Suggested-by: Andrew Lunn <andrew@lunn.ch>
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> 
> > @@ -173,5 +217,20 @@ examples:
> >              reset-gpios = <&gpio1 4 1>;
> >              reset-assert-us = <1000>;
> >              reset-deassert-us = <2000>;
> > +
> > +            leds {
> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +
> > +                led@0 {
> > +                    reg = <0>;
> > +                    linux,default-trigger = "phy-link-1g";
> > +                };
> 
> Because this affects us.
> 
> Is the LED software controllable?

it might be for certain PHYs, integration with the LED framework is
not part of this series.

> Or can it do limited subset of triggers you listed?

it depends on the PHY. The one in this series (RTL8211E) only supports
a limited subset of the listed triggers.

^ permalink raw reply

* Re: [PATCH bpf-next] libbpf: remove zc variable as it is not used
From: Jonathan Lemon @ 2019-08-16 22:01 UTC (permalink / raw)
  To: Yonghong Song; +Cc: Magnus Karlsson, bjorn.topel, ast, daniel, netdev, bpf
In-Reply-To: <f3a8ea34-bd70-8ab8-9739-bb086643fa44@fb.com>



On 16 Aug 2019, at 8:37, Yonghong Song wrote:

> On 8/16/19 3:26 AM, Magnus Karlsson wrote:
>> The zc is not used in the xsk part of libbpf, so let us remove it. 
>> Not
>> good to have dead code lying around.
>>
>> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
>> Reported-by: Yonghong Song <yhs@fb.com> > ---
>>   tools/lib/bpf/xsk.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
>> index 680e630..9687da9 100644
>> --- a/tools/lib/bpf/xsk.c
>> +++ b/tools/lib/bpf/xsk.c
>> @@ -65,7 +65,6 @@ struct xsk_socket {
>>   	int xsks_map_fd;
>>   	__u32 queue_id;
>>   	char ifname[IFNAMSIZ];
>> -	bool zc;
>>   };
>>
>>   struct xsk_nl_info {
>> @@ -608,8 +607,6 @@ int xsk_socket__create(struct xsk_socket 
>> **xsk_ptr, const char *ifname,
>>   		goto out_mmap_tx;
>>   	}
>>
>> -	xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
>
> Since opts.flags usage is removed. Do you think it makes sense to
> remove
>          optlen = sizeof(opts);
>          err = getsockopt(xsk->fd, SOL_XDP, XDP_OPTIONS, &opts, 
> &optlen);
>          if (err) {
>                  err = -errno;
>                  goto out_mmap_tx;
>          }
> as well since nobody then uses opts?

IIRC, this was added specifically in 
2761ed4b6e192820760d5ba913834b2ba05fd08c
so that userland code could know whether the socket was operating in 
zero-copy
mode or not.
-- 
Jonathan

^ permalink raw reply

* Re: [PATCH net-next v2 1/3] net: phy: remove calls to genphy_config_init
From: Florian Fainelli @ 2019-08-16 21:58 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller, Kevin Hilman,
	Vivien Didelot
  Cc: netdev@vger.kernel.org, open list:ARM/Amlogic Meson...
In-Reply-To: <cf0de135-516c-c3e4-6fc7-bf4dbef6462d@gmail.com>

On 8/16/19 1:31 PM, Heiner Kallweit wrote:
> Supported PHY features are either auto-detected or explicitly set.
> In both cases calling genphy_config_init isn't needed. All that
> genphy_config_init does is removing features that are set as
> supported but can't be auto-detected. Basically it duplicates the
> code in genphy_read_abilities. Therefore remove such calls from
> all PHY drivers.
> 
> v2:
> - remove call also from new adin PHY driver
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Looks good, just one question below:

> +static int dummy_config_init(struct phy_device *phydev)
> +{
> +	return 0;
> +}
> +
>  static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
>  	{ TI_DP83848C_PHY_ID, 0xfffffff0 },
>  	{ NS_DP83848C_PHY_ID, 0xfffffff0 },
> @@ -113,13 +113,13 @@ MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
>  
>  static struct phy_driver dp83848_driver[] = {
>  	DP83848_PHY_DRIVER(TI_DP83848C_PHY_ID, "TI DP83848C 10/100 Mbps PHY",
> -			   genphy_config_init),
> +			   dummy_config_init),
>  	DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY",
> -			   genphy_config_init),
> +			   dummy_config_init),
>  	DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY",
>  			   dp83848_config_init),
>  	DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY",
> -			   genphy_config_init),
> +			   dummy_config_init),

drv->config_init is an optional callback so you could just either pass
NULL as an argument to the macro, or simply remove that parameter?
-- 
Florian

^ permalink raw reply

* Re: IPv6 addr and route is gone after adding port to vrf (5.2.0+)
From: David Ahern @ 2019-08-16 21:48 UTC (permalink / raw)
  To: Ben Greear, netdev
In-Reply-To: <ca625841-6de8-addb-9b85-8da90715868c@candelatech.com>

On 8/16/19 3:28 PM, Ben Greear wrote:
> On 8/16/19 12:15 PM, David Ahern wrote:
>> On 8/16/19 1:13 PM, Ben Greear wrote:
>>> I have a problem with a VETH port when setting up a somewhat complicated
>>> VRF setup. I am loosing the global IPv6 addr, and also the route,
>>> apparently
>>> when I add the veth device to a vrf.  From my script's output:
>>
>> Either enslave the device before adding the address or enable the
>> retention of addresses:
>>
>> sysctl -q -w net.ipv6.conf.all.keep_addr_on_down=1
>>
> 
> Thanks, I added it to the vrf first just in case some other logic was
> expecting the routes to go away on network down.
> 
> That part now seems to be working.
> 

The down-up cycling is done on purpose - to clear out neigh entries and
routes associated with the device under the old VRF. All entries must be
created with the device in the new VRF.

^ permalink raw reply

* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Alexei Starovoitov @ 2019-08-16 21:45 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Kees Cook, Andy Lutomirski, Song Liu, Networking, bpf,
	Alexei Starovoitov, Daniel Borkmann, Kernel Team, Lorenz Bauer,
	Jann Horn, Greg KH, Linux API, LSM List
In-Reply-To: <B0364660-AD6A-4E5C-B04F-3B6DA78B4BBE@amacapital.net>

On Thu, Aug 15, 2019 at 05:54:59PM -0700, Andy Lutomirski wrote:
> 
> 
> > On Aug 15, 2019, at 4:46 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
> 
> >> 
> >> I'm not sure why you draw the line for VMs -- they're just as buggy
> >> as anything else. Regardless, I reject this line of thinking: yes,
> >> all software is buggy, but that isn't a reason to give up.
> > 
> > hmm. are you saying you want kernel community to work towards
> > making containers (namespaces) being able to run arbitrary code
> > downloaded from the internet?
> 
> Yes.
> 
> As an example, Sandstorm uses a combination of namespaces (user, network, mount, ipc) and a moderately permissive seccomp policy to run arbitrary code. Not just little snippets, either — node.js, Mongo, MySQL, Meteor, and other fairly heavyweight stacks can all run under Sandstorm, with the whole stack (database engine binaries, etc) supplied by entirely untrusted customers.  During the time Sandstorm was under active development, I can recall *one* bug that would have allowed a sandbox escape. That’s a pretty good track record.  (Also, Meltdown and Spectre, sigh.)

exactly: "meltdown", "spectre", "sigh".
Side channels effectively stalled the work on secure containers.
And killed projects like sandstorm.
Why work on improving kaslr when there are several ways to
get kernel addresses through hw bugs? Patch mouse holes when roof is leaking ?
In case of unprivileged bpf I'm confident that all known holes are patched.
Until disclosures stop happening with the frequency they do now the time
of bpf developers is better spent on something other than unprivileged bpf.

> I’m suggesting that you engage the security community ...
> .. so that normal users can use bpf filtering

yes, but not soon. unfortunately.


^ permalink raw reply

* Re: IPv6 addr and route is gone after adding port to vrf (5.2.0+)
From: Ben Greear @ 2019-08-16 21:28 UTC (permalink / raw)
  To: David Ahern, netdev
In-Reply-To: <2a53ff58-9d5d-ac22-dd23-b4225682c944@gmail.com>

On 8/16/19 12:15 PM, David Ahern wrote:
> On 8/16/19 1:13 PM, Ben Greear wrote:
>> I have a problem with a VETH port when setting up a somewhat complicated
>> VRF setup. I am loosing the global IPv6 addr, and also the route,
>> apparently
>> when I add the veth device to a vrf.  From my script's output:
> 
> Either enslave the device before adding the address or enable the
> retention of addresses:
> 
> sysctl -q -w net.ipv6.conf.all.keep_addr_on_down=1
> 

Thanks, I added it to the vrf first just in case some other logic was
expecting the routes to go away on network down.

That part now seems to be working.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply

* Re: [PATCH v6 4/4] net: phy: realtek: Add LED configuration support for RTL8211E
From: Matthias Kaehlcke @ 2019-08-16 21:27 UTC (permalink / raw)
  To: Pavel Machek
  Cc: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
	Florian Fainelli, Heiner Kallweit, netdev, devicetree,
	linux-kernel, Douglas Anderson
In-Reply-To: <20190816201342.GB1646@bug>

On Fri, Aug 16, 2019 at 10:13:42PM +0200, Pavel Machek wrote:
> On Tue 2019-08-13 12:11:47, Matthias Kaehlcke wrote:
> > Add a .config_led hook which is called by the PHY core when
> > configuration data for a PHY LED is available. Each LED can be
> > configured to be solid 'off, solid 'on' for certain (or all)
> > link speeds or to blink on RX/TX activity.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> 
> THis really needs to go through the LED subsystem,

Sorry, I used what get_maintainers.pl threw at me, I should have
manually cc-ed the LED list.

> and use the same userland interfaces as the rest of the system.

With the PHY maintainers we discussed to define a binding that is
compatible with that of the LED one, to have the option to integrate
it with the LED subsystem later. The integration itself is beyond the
scope of this patchset.

The PHY LED configuration is a low priority for the project I'm
working on. I wanted to make an attempt to upstream it and spent
already significantly more time on it than planned, if integration
with the LED framework now is a requirement please consider this
series abandonded.

^ permalink raw reply

* Re: [PATCH net-next 1/1] Added BASE-T1 PHY support to PHY Subsystem
From: Heiner Kallweit @ 2019-08-16 21:13 UTC (permalink / raw)
  To: Christian Herber, davem@davemloft.net
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190815153209.21529-2-christian.herber@nxp.com>

On 15.08.2019 17:32, Christian Herber wrote:
> BASE-T1 is a category of Ethernet PHYs.
> They use a single copper pair for transmission.
> This patch add basic support for this category of PHYs.
> It coveres the discovery of abilities and basic configuration.
> It includes setting fixed speed and enabling auto-negotiation.
> BASE-T1 devices should always Clause-45 managed.
> Therefore, this patch extends phy-c45.c.
> While for some functions like auto-neogtiation different registers are
> used, the layout of these registers is the same for the used fields.
> Thus, much of the logic of basic Clause-45 devices can be reused.
> 
> Signed-off-by: Christian Herber <christian.herber@nxp.com>
> ---
>  drivers/net/phy/phy-c45.c    | 113 +++++++++++++++++++++++++++++++----
>  drivers/net/phy/phy-core.c   |   4 +-
>  include/uapi/linux/ethtool.h |   2 +
>  include/uapi/linux/mdio.h    |  21 +++++++
>  4 files changed, 129 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index b9d4145781ca..9ff0b8c785de 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -8,13 +8,23 @@
>  #include <linux/mii.h>
>  #include <linux/phy.h>
>  
> +#define IS_100BASET1(phy) (linkmode_test_bit( \
> +			   ETHTOOL_LINK_MODE_100baseT1_Full_BIT, \
> +			   (phy)->supported))
> +#define IS_1000BASET1(phy) (linkmode_test_bit( \
> +			    ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, \
> +			    (phy)->supported))
> +
Why is there no such macro for 10BaseT1?

> +static u32 get_aneg_ctrl(struct phy_device *phydev);
> +static u32 get_aneg_stat(struct phy_device *phydev);
> +
>  /**
>   * genphy_c45_setup_forced - configures a forced speed
>   * @phydev: target phy_device struct
>   */
>  int genphy_c45_pma_setup_forced(struct phy_device *phydev)
>  {
> -	int ctrl1, ctrl2, ret;
> +	int ctrl1, ctrl2, base_t1_ctrl = 0, ret;
>  
>  	/* Half duplex is not supported */
>  	if (phydev->duplex != DUPLEX_FULL)
> @@ -28,6 +38,16 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev)
>  	if (ctrl2 < 0)
>  		return ctrl2;
>  
> +	if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) {

10BaseT1 doesn't need to be considered here?
And maybe it would be better to have a flag for BaseT1 at phy_device level
(based on bit MDIO_PMA_EXTABLE_BASET1?) instead of checking whether certain
T1 modes are supported. Then we would be more future-proof in case of new
T1 modes.

> +		base_t1_ctrl = phy_read_mmd(phydev,
> +					    MDIO_MMD_PMAPMD,
> +					    MDIO_PMA_BASET1CTRL);
> +		if (base_t1_ctrl < 0)
> +			return base_t1_ctrl;
> +
> +		base_t1_ctrl &= ~(MDIO_PMA_BASET1CTRL_TYPE);
> +	}
> +
>  	ctrl1 &= ~MDIO_CTRL1_SPEEDSEL;
>  	/*
>  	 * PMA/PMD type selection is 1.7.5:0 not 1.7.3:0.  See 45.2.1.6.1
> @@ -41,12 +61,21 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev)
>  		break;
>  	case SPEED_100:
>  		ctrl1 |= MDIO_PMA_CTRL1_SPEED100;
> -		ctrl2 |= MDIO_PMA_CTRL2_100BTX;
> +		if (IS_100BASET1(phydev)) {
> +			ctrl2 |= MDIO_PMA_CTRL2_BT1;
> +			base_t1_ctrl |= MDIO_PMA_BASET1CTRL_TYPE_100BT1;
> +		} else {
> +			ctrl2 |= MDIO_PMA_CTRL2_100BTX;
> +		}
>  		break;
>  	case SPEED_1000:
>  		ctrl1 |= MDIO_PMA_CTRL1_SPEED1000;
> -		/* Assume 1000base-T */
> -		ctrl2 |= MDIO_PMA_CTRL2_1000BT;
> +		if (IS_1000BASET1(phydev)) {
> +			ctrl2 |= MDIO_PMA_CTRL2_BT1;
> +			base_t1_ctrl |= MDIO_PMA_BASET1CTRL_TYPE_1000BT1;
> +		} else {
> +			ctrl2 |= MDIO_PMA_CTRL2_1000BT;
> +		}
>  		break;
>  	case SPEED_2500:
>  		ctrl1 |= MDIO_CTRL1_SPEED2_5G;
> @@ -75,6 +104,14 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev)
>  	if (ret < 0)
>  		return ret;
>  
> +	if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) {
> +		ret = phy_write_mmd(phydev,
> +				    MDIO_MMD_PMAPMD,
> +				    MDIO_PMA_BASET1CTRL,
> +				    base_t1_ctrl);
> +		if (ret < 0)
> +			return ret;
> +	}
>  	return genphy_c45_an_disable_aneg(phydev);
>  }
>  EXPORT_SYMBOL_GPL(genphy_c45_pma_setup_forced);
> @@ -135,8 +172,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_an_config_aneg);
>   */
>  int genphy_c45_an_disable_aneg(struct phy_device *phydev)
>  {
> -
> -	return phy_clear_bits_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1,
> +	return phy_clear_bits_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev),
>  				  MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART);
>  }
>  EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg);
> @@ -151,7 +187,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg);
>   */
>  int genphy_c45_restart_aneg(struct phy_device *phydev)
>  {
> -	return phy_set_bits_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1,
> +	return phy_set_bits_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev),
>  				MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART);
>  }
>  EXPORT_SYMBOL_GPL(genphy_c45_restart_aneg);
> @@ -171,7 +207,7 @@ int genphy_c45_check_and_restart_aneg(struct phy_device *phydev, bool restart)
>  
>  	if (!restart) {
>  		/* Configure and restart aneg if it wasn't set before */
> -		ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
> +		ret = phy_read_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev));
>  		if (ret < 0)
>  			return ret;
>  
> @@ -199,7 +235,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_check_and_restart_aneg);
>   */
>  int genphy_c45_aneg_done(struct phy_device *phydev)
>  {
> -	int val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> +	int val = phy_read_mmd(phydev, MDIO_MMD_AN, get_aneg_stat(phydev));
>  
>  	return val < 0 ? val : val & MDIO_AN_STAT1_COMPLETE ? 1 : 0;
>  }
> @@ -385,7 +421,9 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);
>   * PMA Extended Abilities (1.11) register, indicating 1000BASET an 10G related
>   * modes. If bit 1.11.14 is set, then the list is also extended with the modes
>   * in the 2.5G/5G PMA Extended register (1.21), indicating if 2.5GBASET and
> - * 5GBASET are supported.
> + * 5GBASET are supported. If bit 1.11.11 is set, then the list is also extended
> + * with the modes in the BASE-T1 PMA Extended register (1.18), indicating if
> + * 10/100/1000BASET-1 are supported.
>   */
>  int genphy_c45_pma_read_abilities(struct phy_device *phydev)
>  {
> @@ -470,6 +508,29 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev)
>  					 phydev->supported,
>  					 val & MDIO_PMA_NG_EXTABLE_5GBT);
>  		}
> +
> +		if (val & MDIO_PMA_EXTABLE_BASET1) {
> +			val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD,
> +					   MDIO_PMA_BASET1_EXTABLE);
> +			if (val < 0)
> +				return val;
> +
> +			linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT1_Full_BIT,
> +					 phydev->supported,
> +					 val & MDIO_PMA_BASET1_EXTABLE_100BT1);
> +
> +			linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT1_Full_BIT,
> +					 phydev->supported,
> +					 val & MDIO_PMA_BASET1_EXTABLE_1000BT1);
> +
> +			linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
> +					 phydev->supported,
> +					 val & MDIO_PMA_BASET1_EXTABLE_10BT1L);
> +
> +			linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1S_Full_BIT,
> +					 phydev->supported,
> +					 val & MDIO_PMA_BASET1_EXTABLE_10BT1S);
> +		}
>  	}
>  
>  	return 0;
> @@ -509,6 +570,38 @@ int genphy_c45_read_status(struct phy_device *phydev)
>  }
>  EXPORT_SYMBOL_GPL(genphy_c45_read_status);
>  
> +/**
> + * get_aneg_ctrl - Get the register address for auto-
> + * negotiation control register
> + * @phydev: target phy_device struct
> + *
> + */
> +static u32 get_aneg_ctrl(struct phy_device *phydev)
> +{
> +	u32 ctrl = MDIO_CTRL1;
> +
> +	if (IS_100BASET1(phydev) || IS_1000BASET1(phydev))
> +		ctrl = MDIO_AN_BT1_CTRL;
> +
AFAICS 10BaseT1 has separate aneg registers (526/527).
To be considered here?

> +	return ctrl;
> +}
> +
> +/**
> + * get_aneg_ctrl - Get the register address for auto-
> + * negotiation status register
> + * @phydev: target phy_device struct
> + *
> + */
> +static u32 get_aneg_stat(struct phy_device *phydev)
> +{
> +	u32 stat = MDIO_STAT1;
> +
> +	if (IS_100BASET1(phydev) || IS_1000BASET1(phydev))
> +		stat = MDIO_AN_BT1_STAT;
> +
> +	return stat;
> +}
> +
>  /* The gen10g_* functions are the old Clause 45 stub */
>  
>  int gen10g_config_aneg(struct phy_device *phydev)
> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
> index 369903d9b6ec..b50576f7709a 100644
> --- a/drivers/net/phy/phy-core.c
> +++ b/drivers/net/phy/phy-core.c
> @@ -8,7 +8,7 @@
>  
>  const char *phy_speed_to_str(int speed)
>  {
> -	BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 69,
> +	BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 71,
>  		"Enum ethtool_link_mode_bit_indices and phylib are out of sync. "
>  		"If a speed or mode has been added please update phy_speed_to_str "
>  		"and the PHY settings array.\n");
> @@ -140,6 +140,8 @@ static const struct phy_setting settings[] = {
>  	/* 10M */
>  	PHY_SETTING(     10, FULL,     10baseT_Full		),
>  	PHY_SETTING(     10, HALF,     10baseT_Half		),
> +	PHY_SETTING(     10, FULL,     10baseT1L_Full		),
> +	PHY_SETTING(     10, FULL,     10baseT1S_Full		),
>  };
>  #undef PHY_SETTING
>  
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index dd06302aa93e..e429cc8da31a 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1485,6 +1485,8 @@ enum ethtool_link_mode_bit_indices {
>  	ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT	 = 66,
>  	ETHTOOL_LINK_MODE_100baseT1_Full_BIT		 = 67,
>  	ETHTOOL_LINK_MODE_1000baseT1_Full_BIT		 = 68,
> +	ETHTOOL_LINK_MODE_10baseT1L_Full_BIT		 = 69,
> +	ETHTOOL_LINK_MODE_10baseT1S_Full_BIT		 = 70,
>  
>  	/* must be last entry */
>  	__ETHTOOL_LINK_MODE_MASK_NBITS
> diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
> index 0a552061ff1c..6fd5ff632b8e 100644
> --- a/include/uapi/linux/mdio.h
> +++ b/include/uapi/linux/mdio.h
> @@ -43,6 +43,7 @@
>  #define MDIO_PKGID1		14	/* Package identifier */
>  #define MDIO_PKGID2		15
>  #define MDIO_AN_ADVERTISE	16	/* AN advertising (base page) */
> +#define MDIO_PMA_BASET1_EXTABLE	18	/* BASE-T1 PMA/PMD extended ability */
>  #define MDIO_AN_LPA		19	/* AN LP abilities (base page) */
>  #define MDIO_PCS_EEE_ABLE	20	/* EEE Capability register */
>  #define MDIO_PMA_NG_EXTABLE	21	/* 2.5G/5G PMA/PMD extended ability */
> @@ -57,11 +58,16 @@
>  #define MDIO_PMA_10GBT_SNR	133	/* 10GBASE-T SNR margin, lane A.
>  					 * Lanes B-D are numbered 134-136. */
>  #define MDIO_PMA_10GBR_FECABLE	170	/* 10GBASE-R FEC ability */
> +#define MDIO_PMA_BASET1CTRL     2100 /* BASE-T1 PMA/PMD control */
>  #define MDIO_PCS_10GBX_STAT1	24	/* 10GBASE-X PCS status 1 */
>  #define MDIO_PCS_10GBRT_STAT1	32	/* 10GBASE-R/-T PCS status 1 */
>  #define MDIO_PCS_10GBRT_STAT2	33	/* 10GBASE-R/-T PCS status 2 */
>  #define MDIO_AN_10GBT_CTRL	32	/* 10GBASE-T auto-negotiation control */
>  #define MDIO_AN_10GBT_STAT	33	/* 10GBASE-T auto-negotiation status */
> +#define MDIO_AN_BT1_CTRL	512	/* BASE-T1 auto-negotiation control */
> +#define MDIO_AN_BT1_STAT	513	/* BASE-T1 auto-negotiation status */
> +#define MDIO_AN_10BT1_CTRL	526	/* 10BASE-T1 auto-negotiation control */
> +#define MDIO_AN_10BT1_STAT	527	/* 10BASE-T1 auto-negotiation status */
>  
>  /* LASI (Link Alarm Status Interrupt) registers, defined by XENPAK MSA. */
>  #define MDIO_PMA_LASI_RXCTRL	0x9000	/* RX_ALARM control */
> @@ -151,6 +157,7 @@
>  #define MDIO_PMA_CTRL2_100BTX		0x000e	/* 100BASE-TX type */
>  #define MDIO_PMA_CTRL2_10BT		0x000f	/* 10BASE-T type */
>  #define MDIO_PMA_CTRL2_2_5GBT		0x0030  /* 2.5GBaseT type */
> +#define MDIO_PMA_CTRL2_BT1	        0x003D	/* BASE-T1 type */
>  #define MDIO_PMA_CTRL2_5GBT		0x0031  /* 5GBaseT type */
>  #define MDIO_PCS_CTRL2_TYPE		0x0003	/* PCS type selection */
>  #define MDIO_PCS_CTRL2_10GBR		0x0000	/* 10GBASE-R type */
> @@ -205,8 +212,16 @@
>  #define MDIO_PMA_EXTABLE_1000BKX	0x0040	/* 1000BASE-KX ability */
>  #define MDIO_PMA_EXTABLE_100BTX		0x0080	/* 100BASE-TX ability */
>  #define MDIO_PMA_EXTABLE_10BT		0x0100	/* 10BASE-T ability */
> +#define MDIO_PMA_EXTABLE_BASET1		0x0800  /* BASE-T1 ability */
>  #define MDIO_PMA_EXTABLE_NBT		0x4000  /* 2.5/5GBASE-T ability */
>  
> +/* PMA BASE-T1 control register. */
> +#define MDIO_PMA_BASET1CTRL_TYPE         0x000f /* PMA/PMD BASE-T1 type sel. */
> +#define MDIO_PMA_BASET1CTRL_TYPE_100BT1  0x0000 /* 100BASE-T1 */
> +#define MDIO_PMA_BASET1CTRL_TYPE_1000BT1 0x0001 /* 1000BASE-T1 */
> +#define MDIO_PMA_BASET1CTRL_TYPE_10BT1L  0x0002 /* 10BASE-T1L */
> +#define MDIO_PMA_BASET1CTRL_TYPE_10BT1S  0x0003 /* 10BASE-T1S */
> +
>  /* PHY XGXS lane state register. */
>  #define MDIO_PHYXS_LNSTAT_SYNC0		0x0001
>  #define MDIO_PHYXS_LNSTAT_SYNC1		0x0002
> @@ -281,6 +296,12 @@
>  #define MDIO_PMA_NG_EXTABLE_2_5GBT	0x0001	/* 2.5GBASET ability */
>  #define MDIO_PMA_NG_EXTABLE_5GBT	0x0002	/* 5GBASET ability */
>  
> +/* BASE-T1 Extended abilities register. */
> +#define MDIO_PMA_BASET1_EXTABLE_100BT1   0x0001  /* 100BASE-T1 ability */
> +#define MDIO_PMA_BASET1_EXTABLE_1000BT1  0x0002  /* 1000BASE-T1 ability */
> +#define MDIO_PMA_BASET1_EXTABLE_10BT1L   0x0004  /* 10BASE-T1L ability */
> +#define MDIO_PMA_BASET1_EXTABLE_10BT1S   0x0008  /* 10BASE-T1S ability */
> +
>  /* LASI RX_ALARM control/status registers. */
>  #define MDIO_PMA_LASI_RX_PHYXSLFLT	0x0001	/* PHY XS RX local fault */
>  #define MDIO_PMA_LASI_RX_PCSLFLT	0x0008	/* PCS RX local fault */
> 


^ permalink raw reply

* Re: [PATCH] ath10k: Fix HOST capability QMI incompatibility
From: Rob Herring @ 2019-08-16 21:11 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Kalle Valo, David S. Miller, Mark Rutland, linux-wireless, netdev,
	devicetree, linux-kernel, ath10k, stable
In-Reply-To: <20190725063108.15790-1-bjorn.andersson@linaro.org>

On Wed, Jul 24, 2019 at 11:31:08PM -0700, Bjorn Andersson wrote:
> The introduction of 768ec4c012ac ("ath10k: update HOST capability QMI
> message") served the purpose of supporting the new and extended HOST
> capability QMI message.
> 
> But while the new message adds a slew of optional members it changes the
> data type of the "daemon_support" member, which means that older
> versions of the firmware will fail to decode the incoming request
> message.
> 
> There is no way to detect this breakage from Linux and there's no way to
> recover from sending the wrong message (i.e. we can't just try one
> format and then fallback to the other), so a quirk is introduced in
> DeviceTree to indicate to the driver that the firmware requires the 8bit
> version of this message.
> 
> Cc: stable@vger.kernel.org
> Fixes: 768ec4c012ac ("ath10k: update HOST capability qmi message")
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  .../bindings/net/wireless/qcom,ath10k.txt     |  6 +++++

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/net/wireless/ath/ath10k/qmi.c         | 13 ++++++++---
>  .../net/wireless/ath/ath10k/qmi_wlfw_v01.c    | 22 +++++++++++++++++++
>  .../net/wireless/ath/ath10k/qmi_wlfw_v01.h    |  1 +
>  drivers/net/wireless/ath/ath10k/snoc.c        | 11 ++++++++++
>  drivers/net/wireless/ath/ath10k/snoc.h        |  1 +
>  6 files changed, 51 insertions(+), 3 deletions(-)

^ permalink raw reply

* Re: [PATCH net-next 0/1] Add BASE-T1 PHY support
From: Heiner Kallweit @ 2019-08-16 20:59 UTC (permalink / raw)
  To: Christian Herber, davem@davemloft.net
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190815153209.21529-1-christian.herber@nxp.com>

On 15.08.2019 17:32, Christian Herber wrote:
> This patch adds basic support for BASE-T1 PHYs in the framework.
> BASE-T1 PHYs main area of application are automotive and industrial.
> BASE-T1 is standardized in IEEE 802.3, namely
> - IEEE 802.3bw: 100BASE-T1
> - IEEE 802.3bp 1000BASE-T1
> - IEEE 802.3cg: 10BASE-T1L and 10BASE-T1S
> 
> There are no products which contain BASE-T1 and consumer type PHYs like
> 1000BASE-T. However, devices exist which combine 100BASE-T1 and 1000BASE-T1
> PHYs with auto-negotiation.

Is this meant in a way that *currently* there are no PHY's combining Base-T1
with normal Base-T modes? Or are there reasons why this isn't possible in
general? I'm asking because we have PHY's combining copper and fiber, and e.g.
the mentioned Aquantia PHY that combines NBase-T with 1000Base-T2.

> 
> The intention of this patch is to make use of the existing Clause 45 functions.
> BASE-T1 adds some additional registers e.g. for aneg control, which follow a
> similiar register layout as the existing devices. The bits which are used in
> BASE-T1 specific registers are the same as in basic registers, thus the
> existing functions can be resued, with get_aneg_ctrl() selecting the correct
> register address.
> 
If Base-T1 can't be combined with other modes then at a first glance I see no
benefit in defining new registers e.g. for aneg control, and the standard ones
are unused. Why not using the standard registers? Can you shed some light on that?

Are the new registers internally shadowed to the standard location?
That's something I've seen on other PHY's: one register appears in different
places in different devices.

> The current version of ethtool has been prepared for 100/1000BASE-T1 and works
> with this patch. 10BASE-T1 needs to be added to ethtool.
> 
> Christian Herber (1):
>   Added BASE-T1 PHY support to PHY Subsystem
> 
>  drivers/net/phy/phy-c45.c    | 113 +++++++++++++++++++++++++++++++----
>  drivers/net/phy/phy-core.c   |   4 +-
>  include/uapi/linux/ethtool.h |   2 +
>  include/uapi/linux/mdio.h    |  21 +++++++
>  4 files changed, 129 insertions(+), 11 deletions(-)
> 

Heiner

^ permalink raw reply

* [PATCH net-next v2 3/3] net: phy: remove genphy_config_init
From: Heiner Kallweit @ 2019-08-16 20:32 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller, Kevin Hilman,
	Vivien Didelot
  Cc: netdev@vger.kernel.org, open list:ARM/Amlogic Meson...
In-Reply-To: <62de47ba-0624-28c0-56a1-e2fc39a36061@gmail.com>

Now that all users have been removed we can remove genphy_config_init.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 51 ------------------------------------
 include/linux/phy.h          |  1 -
 2 files changed, 52 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9c546bae9..d5db7604d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1885,57 +1885,6 @@ int genphy_soft_reset(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_soft_reset);
 
-int genphy_config_init(struct phy_device *phydev)
-{
-	int val;
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(features) = { 0, };
-
-	linkmode_set_bit_array(phy_basic_ports_array,
-			       ARRAY_SIZE(phy_basic_ports_array),
-			       features);
-	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, features);
-	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, features);
-
-	/* Do we support autonegotiation? */
-	val = phy_read(phydev, MII_BMSR);
-	if (val < 0)
-		return val;
-
-	if (val & BMSR_ANEGCAPABLE)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, features);
-
-	if (val & BMSR_100FULL)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, features);
-	if (val & BMSR_100HALF)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, features);
-	if (val & BMSR_10FULL)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, features);
-	if (val & BMSR_10HALF)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, features);
-
-	if (val & BMSR_ESTATEN) {
-		val = phy_read(phydev, MII_ESTATUS);
-		if (val < 0)
-			return val;
-
-		if (val & ESTATUS_1000_TFULL)
-			linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
-					 features);
-		if (val & ESTATUS_1000_THALF)
-			linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
-					 features);
-		if (val & ESTATUS_1000_XFULL)
-			linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
-					 features);
-	}
-
-	linkmode_and(phydev->supported, phydev->supported, features);
-	linkmode_and(phydev->advertising, phydev->advertising, features);
-
-	return 0;
-}
-EXPORT_SYMBOL(genphy_config_init);
-
 /**
  * genphy_read_abilities - read PHY abilities from Clause 22 registers
  * @phydev: target phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5ac7d2137..d26779f1f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1069,7 +1069,6 @@ void phy_attached_print(struct phy_device *phydev, const char *fmt, ...)
 void phy_attached_info(struct phy_device *phydev);
 
 /* Clause 22 PHY */
-int genphy_config_init(struct phy_device *phydev);
 int genphy_read_abilities(struct phy_device *phydev);
 int genphy_setup_forced(struct phy_device *phydev);
 int genphy_restart_aneg(struct phy_device *phydev);
-- 
2.22.1



^ permalink raw reply related

* [PATCH net-next v2 2/3] net: dsa: remove calls to genphy_config_init
From: Heiner Kallweit @ 2019-08-16 20:31 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller, Kevin Hilman,
	Vivien Didelot
  Cc: netdev@vger.kernel.org, open list:ARM/Amlogic Meson...
In-Reply-To: <62de47ba-0624-28c0-56a1-e2fc39a36061@gmail.com>

Supported PHY features are either auto-detected or explicitly set.
In both cases calling genphy_config_init isn't needed.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 net/dsa/port.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index f071acf28..f75301456 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -538,10 +538,6 @@ static int dsa_port_setup_phy_of(struct dsa_port *dp, bool enable)
 		return PTR_ERR(phydev);
 
 	if (enable) {
-		err = genphy_config_init(phydev);
-		if (err < 0)
-			goto err_put_dev;
-
 		err = genphy_resume(phydev);
 		if (err < 0)
 			goto err_put_dev;
@@ -589,7 +585,6 @@ static int dsa_port_fixed_link_register_of(struct dsa_port *dp)
 		mode = PHY_INTERFACE_MODE_NA;
 	phydev->interface = mode;
 
-	genphy_config_init(phydev);
 	genphy_read_status(phydev);
 
 	if (ds->ops->adjust_link)
-- 
2.22.1



^ permalink raw reply related

* [PATCH net-next v2 1/3] net: phy: remove calls to genphy_config_init
From: Heiner Kallweit @ 2019-08-16 20:31 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller, Kevin Hilman,
	Vivien Didelot
  Cc: netdev@vger.kernel.org, open list:ARM/Amlogic Meson...
In-Reply-To: <62de47ba-0624-28c0-56a1-e2fc39a36061@gmail.com>

Supported PHY features are either auto-detected or explicitly set.
In both cases calling genphy_config_init isn't needed. All that
genphy_config_init does is removing features that are set as
supported but can't be auto-detected. Basically it duplicates the
code in genphy_read_abilities. Therefore remove such calls from
all PHY drivers.

v2:
- remove call also from new adin PHY driver

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/adin.c         |  4 ----
 drivers/net/phy/at803x.c       |  4 ----
 drivers/net/phy/dp83822.c      |  5 -----
 drivers/net/phy/dp83848.c      | 16 ++++++++--------
 drivers/net/phy/dp83tc811.c    |  4 ----
 drivers/net/phy/meson-gxl.c    |  2 +-
 drivers/net/phy/microchip.c    |  1 -
 drivers/net/phy/microchip_t1.c |  1 -
 drivers/net/phy/mscc.c         |  4 ++--
 drivers/net/phy/vitesse.c      |  6 +++---
 10 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index ac79e16cd..4dec83df0 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -356,10 +356,6 @@ static int adin_config_init(struct phy_device *phydev)
 
 	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
 
-	rc = genphy_config_init(phydev);
-	if (rc < 0)
-		return rc;
-
 	rc = adin_config_rgmii_mode(phydev);
 	if (rc < 0)
 		return rc;
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 222ccd9ec..d98aa5671 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -249,10 +249,6 @@ static int at803x_config_init(struct phy_device *phydev)
 {
 	int ret;
 
-	ret = genphy_config_init(phydev);
-	if (ret < 0)
-		return ret;
-
 	/* The RX and TX delay default is:
 	 *   after HW reset: RX delay enabled and TX delay disabled
 	 *   after SW reset: RX delay enabled, while TX delay retains the
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 7ed4760fb..8a4b1d167 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -254,13 +254,8 @@ static int dp83822_config_intr(struct phy_device *phydev)
 
 static int dp83822_config_init(struct phy_device *phydev)
 {
-	int err;
 	int value;
 
-	err = genphy_config_init(phydev);
-	if (err < 0)
-		return err;
-
 	value = DP83822_WOL_MAGIC_EN | DP83822_WOL_SECURE_ON | DP83822_WOL_EN;
 
 	return phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
index 6f9bc7d91..11644579a 100644
--- a/drivers/net/phy/dp83848.c
+++ b/drivers/net/phy/dp83848.c
@@ -68,13 +68,8 @@ static int dp83848_config_intr(struct phy_device *phydev)
 
 static int dp83848_config_init(struct phy_device *phydev)
 {
-	int err;
 	int val;
 
-	err = genphy_config_init(phydev);
-	if (err < 0)
-		return err;
-
 	/* DP83620 always reports Auto Negotiation Ability on BMSR. Instead,
 	 * we check initial value of BMCR Auto negotiation enable bit
 	 */
@@ -85,6 +80,11 @@ static int dp83848_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int dummy_config_init(struct phy_device *phydev)
+{
+	return 0;
+}
+
 static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
 	{ TI_DP83848C_PHY_ID, 0xfffffff0 },
 	{ NS_DP83848C_PHY_ID, 0xfffffff0 },
@@ -113,13 +113,13 @@ MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
 
 static struct phy_driver dp83848_driver[] = {
 	DP83848_PHY_DRIVER(TI_DP83848C_PHY_ID, "TI DP83848C 10/100 Mbps PHY",
-			   genphy_config_init),
+			   dummy_config_init),
 	DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY",
-			   genphy_config_init),
+			   dummy_config_init),
 	DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY",
 			   dp83848_config_init),
 	DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY",
-			   genphy_config_init),
+			   dummy_config_init),
 };
 module_phy_driver(dp83848_driver);
 
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
index ac27da168..06f08832e 100644
--- a/drivers/net/phy/dp83tc811.c
+++ b/drivers/net/phy/dp83tc811.c
@@ -277,10 +277,6 @@ static int dp83811_config_init(struct phy_device *phydev)
 {
 	int value, err;
 
-	err = genphy_config_init(phydev);
-	if (err < 0)
-		return err;
-
 	value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
 	if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
 		err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index fa80d6dce..e8f2ca625 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -136,7 +136,7 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	return genphy_config_init(phydev);
+	return 0;
 }
 
 /* This function is provided to cope with the possible failures of this phy
diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c
index eb1b3287f..a644e8e50 100644
--- a/drivers/net/phy/microchip.c
+++ b/drivers/net/phy/microchip.c
@@ -305,7 +305,6 @@ static int lan88xx_config_init(struct phy_device *phydev)
 {
 	int val;
 
-	genphy_config_init(phydev);
 	/*Zerodetect delay enable */
 	val = phy_read_mmd(phydev, MDIO_MMD_PCS,
 			   PHY_ARDENNES_MMD_DEV_3_PHY_CFG);
diff --git a/drivers/net/phy/microchip_t1.c b/drivers/net/phy/microchip_t1.c
index 3d09b4716..001def450 100644
--- a/drivers/net/phy/microchip_t1.c
+++ b/drivers/net/phy/microchip_t1.c
@@ -48,7 +48,6 @@ static struct phy_driver microchip_t1_phy_driver[] = {
 
 		.features       = PHY_BASIC_T1_FEATURES,
 
-		.config_init    = genphy_config_init,
 		.config_aneg    = genphy_config_aneg,
 
 		.ack_interrupt  = lan87xx_phy_ack_interrupt,
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 645d354ff..7ada1fd9c 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -1725,7 +1725,7 @@ static int vsc8584_config_init(struct phy_device *phydev)
 			return ret;
 	}
 
-	return genphy_config_init(phydev);
+	return 0;
 
 err:
 	mutex_unlock(&phydev->mdio.bus->mdio_lock);
@@ -1767,7 +1767,7 @@ static int vsc85xx_config_init(struct phy_device *phydev)
 			return rc;
 	}
 
-	return genphy_config_init(phydev);
+	return 0;
 }
 
 static int vsc8584_did_interrupt(struct phy_device *phydev)
diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
index 43691b1ac..bb6803527 100644
--- a/drivers/net/phy/vitesse.c
+++ b/drivers/net/phy/vitesse.c
@@ -197,7 +197,7 @@ static int vsc738x_config_init(struct phy_device *phydev)
 
 	vsc73xx_config_init(phydev);
 
-	return genphy_config_init(phydev);
+	return 0;
 }
 
 static int vsc739x_config_init(struct phy_device *phydev)
@@ -229,7 +229,7 @@ static int vsc739x_config_init(struct phy_device *phydev)
 
 	vsc73xx_config_init(phydev);
 
-	return genphy_config_init(phydev);
+	return 0;
 }
 
 static int vsc73xx_config_aneg(struct phy_device *phydev)
@@ -267,7 +267,7 @@ static int vsc8601_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	return genphy_config_init(phydev);
+	return 0;
 }
 
 static int vsc824x_ack_interrupt(struct phy_device *phydev)
-- 
2.22.1



^ permalink raw reply related

* [PATCH net-next v2 0/3] net: phy: remove genphy_config_init
From: Heiner Kallweit @ 2019-08-16 20:30 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller, Kevin Hilman,
	Vivien Didelot
  Cc: netdev@vger.kernel.org, open list:ARM/Amlogic Meson...

Supported PHY features are either auto-detected or explicitly set.
In both cases calling genphy_config_init isn't needed. All that
genphy_config_init does is removing features that are set as
supported but can't be auto-detected. Basically it duplicates the
code in genphy_read_abilities. Therefore remove genphy_config_init.

v2:
- remove call also from new adin driver

Heiner Kallweit (3):
  net: phy: remove calls to genphy_config_init
  net: dsa: remove calls to genphy_config_init
  net: phy: remove genphy_config_init

 drivers/net/phy/adin.c         |  4 ---
 drivers/net/phy/at803x.c       |  4 ---
 drivers/net/phy/dp83822.c      |  5 ----
 drivers/net/phy/dp83848.c      | 16 +++++------
 drivers/net/phy/dp83tc811.c    |  4 ---
 drivers/net/phy/meson-gxl.c    |  2 +-
 drivers/net/phy/microchip.c    |  1 -
 drivers/net/phy/microchip_t1.c |  1 -
 drivers/net/phy/mscc.c         |  4 +--
 drivers/net/phy/phy_device.c   | 51 ----------------------------------
 drivers/net/phy/vitesse.c      |  6 ++--
 include/linux/phy.h            |  1 -
 net/dsa/port.c                 |  5 ----
 13 files changed, 14 insertions(+), 90 deletions(-)

-- 
2.22.1


^ 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