* [PATCH][next] taprio: Use struct_size() in kzalloc()
From: Gustavo A. R. Silva @ 2020-06-18 14:46 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller,
Jakub Kicinski
Cc: netdev, linux-kernel, Gustavo A. R. Silva
Make use of the struct_size() helper instead of an open-coded version
in order to avoid any potential type mistakes. Also, remove unnecessary
variable _size_.
This code was detected with the help of Coccinelle and, audited and
fixed manually.
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
net/sched/sch_taprio.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index b1eb12d33b9a..e981992634dd 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1108,11 +1108,10 @@ static void setup_txtime(struct taprio_sched *q,
static struct tc_taprio_qopt_offload *taprio_offload_alloc(int num_entries)
{
- size_t size = sizeof(struct tc_taprio_sched_entry) * num_entries +
- sizeof(struct __tc_taprio_qopt_offload);
struct __tc_taprio_qopt_offload *__offload;
- __offload = kzalloc(size, GFP_KERNEL);
+ __offload = kzalloc(struct_size(__offload, offload.entries, num_entries),
+ GFP_KERNEL);
if (!__offload)
return NULL;
--
2.27.0
^ permalink raw reply related
* [PATCH][next] net/sched: cls_u32: Use struct_size() in kzalloc()
From: Gustavo A. R. Silva @ 2020-06-18 14:53 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller,
Jakub Kicinski
Cc: netdev, linux-kernel, Gustavo A. R. Silva
Make use of the struct_size() helper instead of an open-coded version
in order to avoid any potential type mistakes.
This code was detected with the help of Coccinelle and, audited and
fixed manually.
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
net/sched/cls_u32.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index e15ff335953d..771b068f8254 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -796,9 +796,7 @@ static struct tc_u_knode *u32_init_knode(struct net *net, struct tcf_proto *tp,
struct tc_u32_sel *s = &n->sel;
struct tc_u_knode *new;
- new = kzalloc(sizeof(*n) + s->nkeys*sizeof(struct tc_u32_key),
- GFP_KERNEL);
-
+ new = kzalloc(struct_size(new, sel.keys, s->nkeys), GFP_KERNEL);
if (!new)
return NULL;
--
2.27.0
^ permalink raw reply related
* Re: qmi_wwan not using netif_carrier_*()
From: Tanjeff-Nicolai Moos @ 2020-06-18 10:08 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Dan Williams, netdev
In-Reply-To: <20200617172434.GH205574@lunn.ch>
On Wed, 17 Jun 2020 19:24:34 +0200
Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Jun 17, 2020 at 11:59:33AM -0500, Dan Williams wrote:
> > On Wed, 2020-06-17 at 18:48 +0200, Andrew Lunn wrote:
> > > On Wed, Jun 17, 2020 at 03:21:53PM +0200, Tanjeff-Nicolai Moos wrote:
> > > > Hi netdevs,
> > > >
> > > > Kernel version:
> > > >
> > > > I'm working with kernel 4.14.137 (OpenWRT project). But I looked
> > > > at
> > > > the source of kernel 5.7 and found the same situation.
> > > >
> > > > Problem:
> > > >
> > > > I'm using the qmi_wwan driver for a Sierra Wireless EM7455 LTE
> > > > modem. This driver does not use
> > > > netif_carrier_on()/netif_carrier_off() to update its link status.
> > > > This confuses ledtrig_netdev which uses netif_carrier_ok() to
> > > > obtain
> > > > the link status.
> > > >
> > > > My solution:
> > > >
> > > > As a solution (or workaround?) I would try:
> > > >
> > > > 1) In drivers/net/usb/qmi_wwan.c, lines 904/913: Add the flag
> > > > FLAG_LINK_INTR.
> > > >
> > > > 2) In drivers/net/usb/usbnet.c, functions usbnet_open() and
> > > > usbnet_stop(): Add a call to netif_carrier_*(),
> > > > but only if FLAG_LINK_INTR is set.
> > > >
> > > > Question:
> > > >
> > > > Is this the intended way to use FLAG_LINK_INTR and
> > > > netif_carrier_*()?
> > > > Or is there another recommended way to obtain the link status of
> > > > network devices (I could change ledtrig_netdev)?
> > >
> > > Hi Tanjeff
> > >
> > > With Ethernet, having a carrier means there is a link partner, the
> > > layer 2 of the OSI 7 layer stack model is working. If the interface
> > > is
> > > not open()ed, it clearly should not have carrier. However, just
> > > because it is open, does not mean it has carrier. The cable could be
> > > unplugged, etc.
> > >
> > > This is an LTE modem. What does carrier mean here? I don't know if it
> > > is well defined, but i would guess it is connected to a base station
> > > which is offering service. I'm assuming you are interested in data
> > > here, not wanting to make a 911/999/112/$EMERGENCY_SERVICE call which
> > > in theory all base stations should accept.
> > >
> > > Is there a way to get this state information from the hardware? That
> > > would be the correct way to set the carrier.
> >
> > There isn't. All the setup that would result in IFF_LOWER_UP (eg
> > ability to pass packets to the cellular network) happens over channels
> > *other* than the ethernet one. eg CDC-WDM, CDC-ACM, CDC-MBIM, AT
> > commands, QMI commands, MBIM commands, etc.
> >
> > Something in userspace handles the actual IP-level connection setup and
> > once that's done, only then do you really have IFF_LOWER_UP. One way to
> > solve this could be to require userspace connection managers to manage
> > the carrier state of the device, which is possible for some drivers
> > already IIRC.
>
> So Tanjeff, what is you real use case here? I assume you want to
> control an LED so it is on when the LTE modem is connected? Could you
> export the LED to user space and have a dhclient-enter/exit script change
> the state of the LED?
The LED should show whether the link is up (data transfer is possible),
and it should blink when data is being transferred. This functionality
is provided by the ledtrig_netdev driver. The blinking works already,
but the indicated link state is wrong, because the netif_carrier_ok()
function /always/ reports true.
When I control the LED by userspace software, it would probably not
blink during xfer. Therefore I would prefer to stick with
ledtrig_netdev (also, it will give me the same behavior as for WLAN,
where I also use ledtrig_netdev).
I observed that sierra.c does call netif_carrier_off(). Since I'm using
a Sierra modem, maybe I'm actually using this driver (sorry for my
limited knowledge), and things should already work? Then I would have
some misconfiguration...
I also observed that "ip address" shows the flags "UP" and "LOWER_UP"
when the interface is up. The kernel seems to know whether "the link"
is up, although I don't know which link is considered. Maybe it is
possible to get that knowledge from within qmi_wwan or usbnet and to
set the carrier accordingly.
Kind regards, tanjeff
--
Tanjeff-Nicolai Moos
Dipl.-Inf. (FH)
Senior Software Engineer
ELTEC Elektronik AG, Mainz
_________________________
Fon +49 6131 918 342
Fax +49 6131 918 195
Email tmoos@eltec.de
Web www.eltec.de
________________________________
*********************************************************
ELTEC Elektronik AG
Galileo-Galilei-Straße 11
D-55129 Mainz
Vorstand: Peter Albert
Aufsichtsratsvorsitzender: Andreas Kochhäuser
Registergericht: Amtsgericht Mainz
Registernummer: HRB 7038
Ust-ID: DE 149 049 790
*********************************************************
Wichtiger Hinweis:
Diese E-Mail kann Betriebs- oder Geschäftsgeheimnisse oder sonstige vertrauliche Informationen enthalten. Sollten Sie diese E-Mail irrtümlich erhalten haben, ist Ihnen eine Kenntnisnahme des Inhalts, eine Vervielfältigung oder Weitergabe der E-Mail ausdrücklich untersagt.
Bitte benachrichtigen Sie uns und vernichten Sie die empfangene E-Mail. Evtl. Anhänge dieser Nachricht wurden auf Viren überprüft!
Jede Form von Vervielfältigung, Abänderung, Verbreitung oder Veröffentlichung dieser E-Mail Nachricht ist untersagt! Das Verwenden von Informationen aus dieser Nachricht für irgendwelche Zwecke ist strengstens untersagt.
Es gelten unsere Allgemeinen Geschäftsbedingungen, zu finden unter www.eltec.de.
^ permalink raw reply
* Re: [PATCH] [net/sched]: Remove redundant condition in qdisc_graft
From: David Miller @ 2020-06-18 14:53 UTC (permalink / raw)
To: gaurav1086; +Cc: jhs, xiyou.wangcong, jiri, kuba, netdev, linux-kernel
In-Reply-To: <20200618040056.30792-1-gaurav1086@gmail.com>
From: Gaurav Singh <gaurav1086@gmail.com>
Date: Thu, 18 Jun 2020 00:00:56 -0400
> parent cannot be NULL here since its in the else part
> of the if (parent == NULL) condition. Remove the extra
> check on parent pointer.
>
> Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
> ---
> net/sched/sch_api.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index 9a3449b56bd6..be93ebcdb18d 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1094,7 +1094,7 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
>
> /* Only support running class lockless if parent is lockless */
> if (new && (new->flags & TCQ_F_NOLOCK) &&
> - parent && !(parent->flags & TCQ_F_NOLOCK))
> + !(parent->flags & TCQ_F_NOLOCK))
You've broken the indentation of this line, it was correctly indented
before your change.
^ permalink raw reply
* Re: [PATCH] [net/sched] Fix null pointer deref skb in tc_ctl_action
From: David Miller @ 2020-06-18 14:54 UTC (permalink / raw)
To: gaurav1086; +Cc: jhs, xiyou.wangcong, jiri, kuba, netdev, linux-kernel
In-Reply-To: <20200618014328.28668-1-gaurav1086@gmail.com>
From: Gaurav Singh <gaurav1086@gmail.com>
Date: Wed, 17 Jun 2020 21:43:28 -0400
> Add null check for skb
>
> Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
> ---
> net/sched/act_api.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 8ac7eb0a8309..fd584821d75a 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -1473,9 +1473,12 @@ static const struct nla_policy tcaa_policy[TCA_ROOT_MAX + 1] = {
> static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
> struct netlink_ext_ack *extack)
> {
> + if (!skb)
> + return 0;
> +
> struct net *net = sock_net(skb->sk);
You're adding code before variable declarations, this is not correct.
I find your work to be very rushed and sloppy, please take your time
and submit well formed and tested changes.
Thank you.
^ permalink raw reply
* [PATCH net] selftests/net: report etf errors correctly
From: Willem de Bruijn @ 2020-06-18 14:55 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
The ETF qdisc can queue skbs that it could not pace on the errqueue.
Address a few issues in the selftest
- recv buffer size was too small, and incorrectly calculated
- compared errno to ee_code instead of ee_errno
- missed error type invalid request
Fixes: ea6a547669b3 ("selftests/net: make so_txtime more robust to timer variance")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
tools/testing/selftests/net/so_txtime.c | 33 +++++++++++++++++++------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/net/so_txtime.c b/tools/testing/selftests/net/so_txtime.c
index 383bac05ac32..3400afff0c3c 100644
--- a/tools/testing/selftests/net/so_txtime.c
+++ b/tools/testing/selftests/net/so_txtime.c
@@ -15,8 +15,9 @@
#include <inttypes.h>
#include <linux/net_tstamp.h>
#include <linux/errqueue.h>
+#include <linux/if_ether.h>
#include <linux/ipv6.h>
-#include <linux/tcp.h>
+#include <linux/udp.h>
#include <stdbool.h>
#include <stdlib.h>
#include <stdio.h>
@@ -140,8 +141,8 @@ static void do_recv_errqueue_timeout(int fdt)
{
char control[CMSG_SPACE(sizeof(struct sock_extended_err)) +
CMSG_SPACE(sizeof(struct sockaddr_in6))] = {0};
- char data[sizeof(struct ipv6hdr) +
- sizeof(struct tcphdr) + 1];
+ char data[sizeof(struct ethhdr) + sizeof(struct ipv6hdr) +
+ sizeof(struct udphdr) + 1];
struct sock_extended_err *err;
struct msghdr msg = {0};
struct iovec iov = {0};
@@ -159,6 +160,8 @@ static void do_recv_errqueue_timeout(int fdt)
msg.msg_controllen = sizeof(control);
while (1) {
+ const char *reason;
+
ret = recvmsg(fdt, &msg, MSG_ERRQUEUE);
if (ret == -1 && errno == EAGAIN)
break;
@@ -176,14 +179,30 @@ static void do_recv_errqueue_timeout(int fdt)
err = (struct sock_extended_err *)CMSG_DATA(cm);
if (err->ee_origin != SO_EE_ORIGIN_TXTIME)
error(1, 0, "errqueue: origin 0x%x\n", err->ee_origin);
- if (err->ee_code != ECANCELED)
- error(1, 0, "errqueue: code 0x%x\n", err->ee_code);
+
+ switch (err->ee_errno) {
+ case ECANCELED:
+ if (err->ee_code != SO_EE_CODE_TXTIME_MISSED)
+ error(1, 0, "errqueue: unknown ECANCELED %u\n",
+ err->ee_code);
+ reason = "missed txtime";
+ break;
+ case EINVAL:
+ if (err->ee_code != SO_EE_CODE_TXTIME_INVALID_PARAM)
+ error(1, 0, "errqueue: unknown EINVAL %u\n",
+ err->ee_code);
+ reason = "invalid txtime";
+ break;
+ default:
+ error(1, 0, "errqueue: errno %u code %u\n",
+ err->ee_errno, err->ee_code);
+ };
tstamp = ((int64_t) err->ee_data) << 32 | err->ee_info;
tstamp -= (int64_t) glob_tstart;
tstamp /= 1000 * 1000;
- fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped\n",
- data[ret - 1], tstamp);
+ fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
+ data[ret - 1], tstamp, reason);
msg.msg_flags = 0;
msg.msg_controllen = sizeof(control);
--
2.27.0.290.gba653c62da-goog
^ permalink raw reply related
* Re: [PATCH net v1] net: phy: smsc: fix printing too many logs
From: Dejin Zheng @ 2020-06-18 15:08 UTC (permalink / raw)
To: Andrew Lunn
Cc: Russell King - ARM Linux admin, f.fainelli, hkallweit1, davem,
kuba, netdev, linux-kernel, Kevin Groeneveld
In-Reply-To: <20200617213642.GE240559@lunn.ch>
On Wed, Jun 17, 2020 at 11:36:42PM +0200, Andrew Lunn wrote:
> On Wed, Jun 17, 2020 at 09:24:50PM +0100, Russell King - ARM Linux admin wrote:
> > On Wed, Jun 17, 2020 at 08:43:34PM +0200, Andrew Lunn wrote:
> > > You have explained what the change does. But not why it is
> > > needed. What exactly is happening. To me, the key thing is
> > > understanding why we get -110, and why it is not an actual error we
> > > should be reporting as an error. That is what needs explaining.
> >
> > The patch author really ought to be explaining this... but let me
> > have a go. It's worth pointing out that the comments in the file
> > aren't good English either, so don't really describe what is going
> > on.
> >
> > When this PHY is in EDPD mode, it doesn't always detect a connected
> > cable. The workaround for it involves, when the link is down, and
> > at each read_status() call:
> >
> > - disable EDPD mode, forcing the PHY out of low-power mode
> > - waiting 640ms to see if we have any energy detected from the media
> > - re-enable entry to EDPD mode
> >
> > This is presumably enough to allow the PHY to notice that a cable is
> > connected, and resume normal operations to negotiate with the partner.
> >
> > The problem is that when no media is detected, the 640ms wait times
> > out (as it should, we don't want to wait forever) and the kernel
> > prints a warning.
>
> Hi Russell
>
> Yes, that is what i was thinking.
>
> There probably should be a comment added just to prevent somebody
> swapping it back to phy_read_poll_timeout(). It is not clear that
> -ETIMEOUT is expected under some conditions.
>
> Andrew
Hi Andrew and Russell,
First of all, thanks very much for your comment!
I did not understand Andrew's comments correctly in the previous email,
sorry for my bad English. I found something in the commit 776829de90c597
("net: phy: workaround for buggy cable detection by LAN8700 after cable
plugging") about why it is not an actual error when get a timeout error
in this driver. the commit's link is here:
https://lore.kernel.org/patchwork/patch/590285/
As Russell said, it just for fix a hardware bug on LAN8700 device by wait
640ms, so it should not as an actual error when got timeout.
The following is my understanding:
It leads to unstable detection of plugging in Ethernet cable when LAN87xx
is in Energy Detect Power-Down mode. Because the ENERGYON bit is not set.
For fix this issue, it will disables Energy Detect Power-Down mode and
check ENERGYON bit to waiting for response on link pulses to detect
presence of plugged Ethernet cable in the 640ms. if got the ENERGYON bit
is set, I guess the cable plug-in event was detected and will report an
interrupt after exit lan87xx_read_status() funtion, if the ENERGYON bit
is not set, the plug-in event was not detected. after that, entry Energy
Detect Power-Down mode again.
it will re-call lan87xx_read_status() funtion by interrupt when got the
ENERGYON bit is set. and check link status again. then, It will get a
stable link status for LAN87xx device.
So the timeout for wait 640ms should not as an actual error.
Thanks!
Dejin
^ permalink raw reply
* Re: [PATCH net] geneve: allow changing DF behavior after creation
From: Stefano Brivio @ 2020-06-18 15:20 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: netdev
In-Reply-To: <20200618130347.GA2557669@bistromath.localdomain>
On Thu, 18 Jun 2020 15:03:47 +0200
Sabrina Dubroca <sd@queasysnail.net> wrote:
> 2020-06-18, 12:26:29 +0200, Stefano Brivio wrote:
> > On Thu, 18 Jun 2020 12:13:22 +0200
> > Sabrina Dubroca <sd@queasysnail.net> wrote:
> >
> > > Currently, trying to change the DF parameter of a geneve device does
> > > nothing:
> > >
> > > # ip -d link show geneve1
> > > 14: geneve1: <snip>
> > > link/ether <snip>
> > > geneve id 1 remote 10.0.0.1 ttl auto df set dstport 6081 <snip>
> > > # ip link set geneve1 type geneve id 1 df unset
> > > # ip -d link show geneve1
> > > 14: geneve1: <snip>
> > > link/ether <snip>
> > > geneve id 1 remote 10.0.0.1 ttl auto df set dstport 6081 <snip>
> > >
> > > We just need to update the value in geneve_changelink.
> > >
> > > Fixes: a025fb5f49ad ("geneve: Allow configuration of DF behaviour")
> > > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> > > ---
> > > drivers/net/geneve.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> > > index 75266580b586..4661ef865807 100644
> > > --- a/drivers/net/geneve.c
> > > +++ b/drivers/net/geneve.c
> > > @@ -1649,6 +1649,7 @@ static int geneve_changelink(struct net_device *dev, struct nlattr *tb[],
> > > geneve->collect_md = metadata;
> > > geneve->use_udp6_rx_checksums = use_udp6_rx_checksums;
> > > geneve->ttl_inherit = ttl_inherit;
> > > + geneve->df = df;
> >
> > I introduced this bug as I didn't notice the asymmetry with VXLAN,
> > where vxlan_nl2conf() takes care of this for both new links and link
> > changes.
>
> Yeah, I didn't notice either :/
>
> > Here, this block is duplicated in geneve_configure(), which,
> > somewhat surprisingly given the name, is not called from
> > geneve_changelink(). Did you consider factoring out (at least) this
> > block to have it shared?
>
> Then I'd have to introduce another lovely function with an absurdly
> long argument list. I'd rather clean that up in all of geneve and
> introduce something like struct vxlan_config, but it's a bit much for
> net. I'll do that once this fix finds its way into net-next.
Yeah, sure, I didn't mean you should simply copy and paste that
somewhere. Either something like struct vxlan_config used around the
current logic, or perhaps even better, something like vxlan_nl2conf()
does.
I didn't check whether something in GENEVE is so special compared to
VXLAN as to prevent this, though.
--
Stefano
^ permalink raw reply
* Re: [RFC PATCH 2/9] net: dsa: Add DSA driver for Hirschmann Hellcreek switches
From: Andrew Lunn @ 2020-06-18 15:22 UTC (permalink / raw)
To: Kurt Kanzenbach
Cc: Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski,
netdev, Rob Herring, devicetree, Sebastian Andrzej Siewior,
Richard Cochran, Kamil Alkhouri, ilias.apalodimas
In-Reply-To: <20200618064029.32168-3-kurt@linutronix.de>
> +static void __hellcreek_select_port(struct hellcreek *hellcreek, int port)
Hi Kurt
In general, please can you drop all these __ prefixes. They are useful
when you have for example hellcreek_select_port() takes a lock and
then calls _hellcreek_select_port(), but here, there is no need for
them.
> +static int hellcreek_detect(struct hellcreek *hellcreek)
> +{
> + u8 tgd_maj, tgd_min;
> + u16 id, rel_low, rel_high, date_low, date_high, tgd_ver;
> + u32 rel, date;
Reverse Christmas tree please. Here and everywhere else.
> +
> + id = hellcreek_read(hellcreek, HR_MODID_C);
> + rel_low = hellcreek_read(hellcreek, HR_REL_L_C);
> + rel_high = hellcreek_read(hellcreek, HR_REL_H_C);
> + date_low = hellcreek_read(hellcreek, HR_BLD_L_C);
> + date_high = hellcreek_read(hellcreek, HR_BLD_H_C);
> + tgd_ver = hellcreek_read(hellcreek, TR_TGDVER);
> +
> + if (id != HELLCREEK_MODULE_ID)
> + return -ENODEV;
> +
> + rel = rel_low | (rel_high << 16);
> + date = date_low | (date_high << 16);
> + tgd_maj = (tgd_ver & TR_TGDVER_REV_MAJ_MASK) >> TR_TGDVER_REV_MAJ_SHIFT;
> + tgd_min = (tgd_ver & TR_TGDVER_REV_MIN_MASK) >> TR_TGDVER_REV_MIN_SHIFT;
> +
> + dev_info(hellcreek->dev, "Module ID=%02x Release=%04x Date=%04x TGD Version=%02x.%02x\n",
> + id, rel, date, tgd_maj, tgd_min);
> +
> + return 0;
> +}
> +static int hellcreek_port_enable(struct dsa_switch *ds, int port,
> + struct phy_device *phy)
> +{
> + struct hellcreek *hellcreek = ds->priv;
> + struct hellcreek_port *hellcreek_port = &hellcreek->ports[port];
> + unsigned long flags;
> + u16 val;
> +
> + if (port >= NUM_PORTS)
> + return -EINVAL;
> +
> + dev_info(hellcreek->dev, "Enable port %d\n", port);
dev_dbg().
+
> + spin_lock_irqsave(&hellcreek->reg_lock, flags);
I've not seen any interrupt handling code in the driver, and there is
no mention of an interrupt in the DT binding. Do you really need
_irqsave spin locks?
> +
> + __hellcreek_select_port(hellcreek, port);
> + val = hellcreek_port->ptcfg;
> + val |= HR_PTCFG_ADMIN_EN;
> + hellcreek_write(hellcreek, val, HR_PTCFG);
> + hellcreek_port->ptcfg = val;
> +
> + spin_unlock_irqrestore(&hellcreek->reg_lock, flags);
> +
> + return 0;
> +}
> +
> +static void hellcreek_port_disable(struct dsa_switch *ds, int port)
> +{
> + struct hellcreek *hellcreek = ds->priv;
> + struct hellcreek_port *hellcreek_port = &hellcreek->ports[port];
> + unsigned long flags;
> + u16 val;
> +
> + if (port >= NUM_PORTS)
> + return;
I don't think this test is actually needed, here or in any of the
other callbacks. If it does happen, it means we have a core bug we
should fix.
> +
> + dev_info(hellcreek->dev, "Disable port %d\n", port);
dev_dbg()
> + spin_lock_irqsave(&hellcreek->reg_lock, flags);
> +
> + __hellcreek_select_port(hellcreek, port);
> + val = hellcreek_port->ptcfg;
> + val &= ~HR_PTCFG_ADMIN_EN;
> + hellcreek_write(hellcreek, val, HR_PTCFG);
> + hellcreek_port->ptcfg = val;
> +
> + spin_unlock_irqrestore(&hellcreek->reg_lock, flags);
> +}
> +
> +static void hellcreek_get_ethtool_stats(struct dsa_switch *ds, int port,
> + uint64_t *data)
> +{
> + struct hellcreek *hellcreek = ds->priv;
> + struct hellcreek_port *hellcreek_port = &hellcreek->ports[port];
> + unsigned long flags;
> + int i;
> +
> + if (port >= NUM_PORTS)
> + return;
> +
> + spin_lock_irqsave(&hellcreek->reg_lock, flags);
> + for (i = 0; i < ARRAY_SIZE(hellcreek_counter); ++i) {
> + const struct hellcreek_counter *counter = &hellcreek_counter[i];
> + u8 offset = counter->offset + port * 64;
> + u16 high, low;
> + u64 value = 0;
> +
> + __hellcreek_select_counter(hellcreek, offset);
> +
> + high = hellcreek_read(hellcreek, HR_CRDH);
> + low = hellcreek_read(hellcreek, HR_CRDL);
> + value = (high << 16) | low;
Is there some sort of snapshot happening here? Or do you need to read
high again, to make sure it has not incremented when low was being
read?
> +
> + hellcreek_port->counter_values[i] += value;
> + data[i] = hellcreek_port->counter_values[i];
> + }
> + spin_unlock_irqrestore(&hellcreek->reg_lock, flags);
> +}
> +
> +static int hellcreek_vlan_prepare(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_vlan *vlan)
> +{
> + struct hellcreek *hellcreek = ds->priv;
> +
> + /* Nothing todo */
> + dev_info(hellcreek->dev, "VLAN prepare for port %d\n", port);
dev_dbg()
> +
> + return 0;
> +}
> +
> +static void hellcreek_vlan_add(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_vlan *vlan)
> +{
> + struct hellcreek *hellcreek = ds->priv;
> + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
> + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
> + unsigned long flags;
> + u16 vid;
> +
> + if (port >= NUM_PORTS || vlan->vid_end >= 4096)
Again, if vlan->vid_end >= 4096 is true, there is a core bug which
needs fixing.
> + return;
> +
> + dev_info(hellcreek->dev, "Add VLANs (%d -- %d) on port %d, %s, %s\n",
> + vlan->vid_begin, vlan->vid_end, port,
> + untagged ? "untagged" : "tagged",
> + pvid ? "PVID" : "no PVID");
Please go thought the driver and consider all you dev_info()
statements. Should they be dev_dbg()?
Andrew
^ permalink raw reply
* Re: net/mlx5e: bind() always returns EINVAL with XDP_ZEROCOPY
From: Jonathan Lemon @ 2020-06-18 15:23 UTC (permalink / raw)
To: Kal Cutter Conley
Cc: Saeed Mahameed, brouer@redhat.com, Maxim Mikityanskiy,
magnus.karlsson@intel.com, toke.hoiland-jorgensen@kau.se,
xdp-newbies@vger.kernel.org, Tariq Toukan, gospo@broadcom.com,
jakub.kicinski@netronome.com, netdev@vger.kernel.org,
bjorn.topel@intel.com
In-Reply-To: <CAHApi-k=9Szxm0QMD4N4PW9Lq8L4hW6e7VfyBePzrTgvKGRs5Q@mail.gmail.com>
On Sun, Jun 14, 2020 at 10:55:30AM +0200, Kal Cutter Conley wrote:
> Hi Saeed,
> Thanks for explaining the reasoning behind the special mlx5 queue
> numbering with XDP zerocopy.
>
> We have a process using AF_XDP that also shares the network interface
> with other processes on the system. ethtool rx flow classification
> rules are used to route the traffic to the appropriate XSK queue
> N..(2N-1). The issue is these queues are only valid as long they are
> active (as far as I can tell). This means if my AF_XDP process dies
> other processes no longer receive ingress traffic routed over queues
> N..(2N-1) even though my XDP program is still loaded and would happily
> always return XDP_PASS. Other drivers do not have this usability issue
> because they use queues that are always valid. Is there a simple
> workaround for this issue? It seems to me queues N..(2N-1) should
> simply map to 0..(N-1) when they are not active?
If your XDP program returns XDP_PASS, the packet should be delivered to
the xsk socket. If the application isn't running, where would it go?
I do agree that the usability of this can be improved. What if the flow
rules are inserted and removed along with queue creatioin/destruction?
--
Jonathan
^ permalink raw reply
* [PATCH net-next 0/4] Marvell mvpp2 improvements
From: Russell King - ARM Linux admin @ 2020-06-18 15:38 UTC (permalink / raw)
To: Antoine Tenart, Alexandre Belloni; +Cc: David S. Miller, Jakub Kicinski, netdev
Hi,
This series primarily cleans up mvpp2, but also fixes a left-over from
91a208f2185a ("net: phylink: propagate resolved link config via
mac_link_up()").
Patch 1 introduces some port helpers:
mvpp2_port_supports_xlg() - does the port support the XLG MAC
mvpp2_port_supports_rgmii() - does the port support RGMII modes
Patch 2 introduces mvpp2_phylink_to_port(), rather than having repeated
open coding of container_of().
Patch 3 introduces mvpp2_modify(), which reads-modifies-writes a
register - I've converted the phylink specific code to use this
helper.
Patch 4 moves the hardware control of the pause modes from
mvpp2_xlg_config() (which is called via the phylink_config method)
to mvpp2_mac_link_up() - a change that was missed in the above
referenced commit.
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 164 +++++++++++++-----------
1 file changed, 89 insertions(+), 75 deletions(-)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* [PATCH net-next 1/4] net: mvpp2: add port support helpers
From: Russell King @ 2020-06-18 15:38 UTC (permalink / raw)
To: Antoine Tenart, Alexandre Belloni; +Cc: David S. Miller, Jakub Kicinski, netdev
In-Reply-To: <20200618153818.GD1551@shell.armlinux.org.uk>
The mvpp2 code has tests scattered amongst the code to determine
whether the port supports the XLG, and whether the port supports
RGMII mode.
Rather than having these tests scattered, provide a couple of helper
functions, so that future additions can ensure that they get these
tests correct.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 43 ++++++++++++-------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 24f4d8e0da98..7653277d03b7 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1114,6 +1114,17 @@ mvpp2_shared_interrupt_mask_unmask(struct mvpp2_port *port, bool mask)
}
}
+/* Only GOP port 0 has an XLG MAC */
+static bool mvpp2_port_supports_xlg(struct mvpp2_port *port)
+{
+ return port->gop_id == 0;
+}
+
+static bool mvpp2_port_supports_rgmii(struct mvpp2_port *port)
+{
+ return !(port->priv->hw_version == MVPP22 && port->gop_id == 0);
+}
+
/* Port configuration routines */
static bool mvpp2_is_xlg(phy_interface_t interface)
{
@@ -1194,7 +1205,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port)
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_RXID:
case PHY_INTERFACE_MODE_RGMII_TXID:
- if (port->gop_id == 0)
+ if (!mvpp2_port_supports_rgmii(port))
goto invalid_conf;
mvpp22_gop_init_rgmii(port);
break;
@@ -1204,7 +1215,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port)
mvpp22_gop_init_sgmii(port);
break;
case PHY_INTERFACE_MODE_10GBASER:
- if (port->gop_id != 0)
+ if (!mvpp2_port_supports_xlg(port))
goto invalid_conf;
mvpp22_gop_init_10gkr(port);
break;
@@ -1246,7 +1257,7 @@ static void mvpp22_gop_unmask_irq(struct mvpp2_port *port)
writel(val, port->base + MVPP22_GMAC_INT_SUM_MASK);
}
- if (port->gop_id == 0) {
+ if (mvpp2_port_supports_xlg(port)) {
/* Enable the XLG/GIG irqs for this port */
val = readl(port->base + MVPP22_XLG_EXT_INT_MASK);
if (mvpp2_is_xlg(port->phy_interface))
@@ -1261,7 +1272,7 @@ static void mvpp22_gop_mask_irq(struct mvpp2_port *port)
{
u32 val;
- if (port->gop_id == 0) {
+ if (mvpp2_port_supports_xlg(port)) {
val = readl(port->base + MVPP22_XLG_EXT_INT_MASK);
val &= ~(MVPP22_XLG_EXT_INT_MASK_XLG |
MVPP22_XLG_EXT_INT_MASK_GIG);
@@ -1290,7 +1301,7 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
writel(val, port->base + MVPP22_GMAC_INT_MASK);
}
- if (port->gop_id == 0) {
+ if (mvpp2_port_supports_xlg(port)) {
val = readl(port->base + MVPP22_XLG_INT_MASK);
val |= MVPP22_XLG_INT_MASK_LINK;
writel(val, port->base + MVPP22_XLG_INT_MASK);
@@ -1328,8 +1339,8 @@ static void mvpp2_port_enable(struct mvpp2_port *port)
{
u32 val;
- /* Only GOP port 0 has an XLG MAC */
- if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface)) {
+ if (mvpp2_port_supports_xlg(port) &&
+ mvpp2_is_xlg(port->phy_interface)) {
val = readl(port->base + MVPP22_XLG_CTRL0_REG);
val |= MVPP22_XLG_CTRL0_PORT_EN;
val &= ~MVPP22_XLG_CTRL0_MIB_CNT_DIS;
@@ -1346,8 +1357,8 @@ static void mvpp2_port_disable(struct mvpp2_port *port)
{
u32 val;
- /* Only GOP port 0 has an XLG MAC */
- if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface)) {
+ if (mvpp2_port_supports_xlg(port) &&
+ mvpp2_is_xlg(port->phy_interface)) {
val = readl(port->base + MVPP22_XLG_CTRL0_REG);
val &= ~MVPP22_XLG_CTRL0_PORT_EN;
writel(val, port->base + MVPP22_XLG_CTRL0_REG);
@@ -2740,7 +2751,8 @@ static irqreturn_t mvpp2_link_status_isr(int irq, void *dev_id)
mvpp22_gop_mask_irq(port);
- if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface)) {
+ if (mvpp2_port_supports_xlg(port) &&
+ mvpp2_is_xlg(port->phy_interface)) {
val = readl(port->base + MVPP22_XLG_INT_STAT);
if (val & MVPP22_XLG_INT_STAT_LINK) {
event = true;
@@ -3430,8 +3442,7 @@ static void mvpp22_mode_reconfigure(struct mvpp2_port *port)
mvpp22_pcs_reset_deassert(port);
- /* Only GOP port 0 has an XLG MAC */
- if (port->gop_id == 0) {
+ if (mvpp2_port_supports_xlg(port)) {
ctrl3 = readl(port->base + MVPP22_XLG_CTRL3_REG);
ctrl3 &= ~MVPP22_XLG_CTRL3_MACMODESELECT_MASK;
@@ -3443,7 +3454,7 @@ static void mvpp22_mode_reconfigure(struct mvpp2_port *port)
writel(ctrl3, port->base + MVPP22_XLG_CTRL3_REG);
}
- if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface))
+ if (mvpp2_port_supports_xlg(port) && mvpp2_is_xlg(port->phy_interface))
mvpp2_xlg_max_rx_size_set(port);
else
mvpp2_gmac_max_rx_size_set(port);
@@ -4768,14 +4779,14 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
switch (state->interface) {
case PHY_INTERFACE_MODE_10GBASER:
case PHY_INTERFACE_MODE_XAUI:
- if (port->gop_id != 0)
+ if (!mvpp2_port_supports_xlg(port))
goto empty_set;
break;
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_RXID:
case PHY_INTERFACE_MODE_RGMII_TXID:
- if (port->priv->hw_version == MVPP22 && port->gop_id == 0)
+ if (!mvpp2_port_supports_rgmii(port))
goto empty_set;
break;
default:
@@ -4791,7 +4802,7 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
case PHY_INTERFACE_MODE_10GBASER:
case PHY_INTERFACE_MODE_XAUI:
case PHY_INTERFACE_MODE_NA:
- if (port->gop_id == 0) {
+ if (mvpp2_port_supports_xlg(port)) {
phylink_set(mask, 10000baseT_Full);
phylink_set(mask, 10000baseCR_Full);
phylink_set(mask, 10000baseSR_Full);
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper
From: Russell King @ 2020-06-18 15:38 UTC (permalink / raw)
To: Antoine Tenart, Alexandre Belloni; +Cc: David S. Miller, Jakub Kicinski, netdev
In-Reply-To: <20200618153818.GD1551@shell.armlinux.org.uk>
Add a helper to convert the struct phylink_config pointer passed in
from phylink to the drivers internal struct mvpp2_port.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 30 +++++++++----------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 7653277d03b7..8c8314715efd 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4767,12 +4767,17 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
eth_hw_addr_random(dev);
}
+static inline struct mvpp2_port *
+mvpp2_phylink_to_port(struct phylink_config *config)
+{
+ return container_of(config, struct mvpp2_port, phylink_config);
+}
+
static void mvpp2_phylink_validate(struct phylink_config *config,
unsigned long *supported,
struct phylink_link_state *state)
{
- struct mvpp2_port *port = container_of(config, struct mvpp2_port,
- phylink_config);
+ struct mvpp2_port *port = mvpp2_phylink_to_port(config);
__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
/* Invalid combinations */
@@ -4913,8 +4918,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
struct phylink_link_state *state)
{
- struct mvpp2_port *port = container_of(config, struct mvpp2_port,
- phylink_config);
+ struct mvpp2_port *port = mvpp2_phylink_to_port(config);
if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
@@ -4931,8 +4935,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
static void mvpp2_mac_an_restart(struct phylink_config *config)
{
- struct mvpp2_port *port = container_of(config, struct mvpp2_port,
- phylink_config);
+ struct mvpp2_port *port = mvpp2_phylink_to_port(config);
u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
@@ -5105,13 +5108,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
const struct phylink_link_state *state)
{
- struct net_device *dev = to_net_dev(config->dev);
- struct mvpp2_port *port = netdev_priv(dev);
+ struct mvpp2_port *port = mvpp2_phylink_to_port(config);
bool change_interface = port->phy_interface != state->interface;
/* Check for invalid configuration */
if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
- netdev_err(dev, "Invalid mode on %s\n", dev->name);
+ netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
return;
}
@@ -5151,8 +5153,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
int speed, int duplex,
bool tx_pause, bool rx_pause)
{
- struct net_device *dev = to_net_dev(config->dev);
- struct mvpp2_port *port = netdev_priv(dev);
+ struct mvpp2_port *port = mvpp2_phylink_to_port(config);
u32 val;
if (mvpp2_is_xlg(interface)) {
@@ -5199,14 +5200,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
mvpp2_egress_enable(port);
mvpp2_ingress_enable(port);
- netif_tx_wake_all_queues(dev);
+ netif_tx_wake_all_queues(port->dev);
}
static void mvpp2_mac_link_down(struct phylink_config *config,
unsigned int mode, phy_interface_t interface)
{
- struct net_device *dev = to_net_dev(config->dev);
- struct mvpp2_port *port = netdev_priv(dev);
+ struct mvpp2_port *port = mvpp2_phylink_to_port(config);
u32 val;
if (!phylink_autoneg_inband(mode)) {
@@ -5223,7 +5223,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
}
}
- netif_tx_stop_all_queues(dev);
+ netif_tx_stop_all_queues(port->dev);
mvpp2_egress_disable(port);
mvpp2_ingress_disable(port);
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 3/4] net: mvpp2: add register modification helper
From: Russell King @ 2020-06-18 15:38 UTC (permalink / raw)
To: Antoine Tenart, Alexandre Belloni; +Cc: David S. Miller, Jakub Kicinski, netdev
In-Reply-To: <20200618153818.GD1551@shell.armlinux.org.uk>
Add a helper to read-modify-write a register, and use it in the phylink
helpers.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 88 ++++++++++---------
1 file changed, 46 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 8c8314715efd..9edd8fbf18a6 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1132,6 +1132,17 @@ static bool mvpp2_is_xlg(phy_interface_t interface)
interface == PHY_INTERFACE_MODE_XAUI;
}
+static void mvpp2_modify(void __iomem *ptr, u32 mask, u32 set)
+{
+ u32 old, val;
+
+ old = val = readl(ptr);
+ val &= ~mask;
+ val |= set;
+ if (old != val)
+ writel(val, ptr);
+}
+
static void mvpp22_gop_init_rgmii(struct mvpp2_port *port)
{
struct mvpp2 *priv = port->priv;
@@ -4947,38 +4958,29 @@ static void mvpp2_mac_an_restart(struct phylink_config *config)
static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
const struct phylink_link_state *state)
{
- u32 old_ctrl0, ctrl0;
- u32 old_ctrl4, ctrl4;
-
- old_ctrl0 = ctrl0 = readl(port->base + MVPP22_XLG_CTRL0_REG);
- old_ctrl4 = ctrl4 = readl(port->base + MVPP22_XLG_CTRL4_REG);
-
- ctrl0 |= MVPP22_XLG_CTRL0_MAC_RESET_DIS;
+ u32 val;
+ val = MVPP22_XLG_CTRL0_MAC_RESET_DIS;
if (state->pause & MLO_PAUSE_TX)
- ctrl0 |= MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
- else
- ctrl0 &= ~MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
+ val |= MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
if (state->pause & MLO_PAUSE_RX)
- ctrl0 |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
- else
- ctrl0 &= ~MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
-
- ctrl4 &= ~(MVPP22_XLG_CTRL4_MACMODSELECT_GMAC |
- MVPP22_XLG_CTRL4_EN_IDLE_CHECK);
- ctrl4 |= MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC;
-
- if (old_ctrl0 != ctrl0)
- writel(ctrl0, port->base + MVPP22_XLG_CTRL0_REG);
- if (old_ctrl4 != ctrl4)
- writel(ctrl4, port->base + MVPP22_XLG_CTRL4_REG);
-
- if (!(old_ctrl0 & MVPP22_XLG_CTRL0_MAC_RESET_DIS)) {
- while (!(readl(port->base + MVPP22_XLG_CTRL0_REG) &
- MVPP22_XLG_CTRL0_MAC_RESET_DIS))
- continue;
- }
+ val |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
+
+ mvpp2_modify(port->base + MVPP22_XLG_CTRL0_REG,
+ MVPP22_XLG_CTRL0_MAC_RESET_DIS |
+ MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN |
+ MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN, val);
+ mvpp2_modify(port->base + MVPP22_XLG_CTRL4_REG,
+ MVPP22_XLG_CTRL4_MACMODSELECT_GMAC |
+ MVPP22_XLG_CTRL4_EN_IDLE_CHECK |
+ MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC,
+ MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC);
+
+ /* Wait for reset to deassert */
+ do {
+ val = readl(port->base + MVPP22_XLG_CTRL0_REG);
+ } while (!(val & MVPP22_XLG_CTRL0_MAC_RESET_DIS));
}
static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
@@ -5158,19 +5160,14 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
if (mvpp2_is_xlg(interface)) {
if (!phylink_autoneg_inband(mode)) {
- val = readl(port->base + MVPP22_XLG_CTRL0_REG);
- val &= ~MVPP22_XLG_CTRL0_FORCE_LINK_DOWN;
- val |= MVPP22_XLG_CTRL0_FORCE_LINK_PASS;
- writel(val, port->base + MVPP22_XLG_CTRL0_REG);
+ mvpp2_modify(port->base + MVPP22_XLG_CTRL0_REG,
+ MVPP22_XLG_CTRL0_FORCE_LINK_DOWN |
+ MVPP22_XLG_CTRL0_FORCE_LINK_PASS,
+ MVPP22_XLG_CTRL0_FORCE_LINK_PASS);
}
} else {
if (!phylink_autoneg_inband(mode)) {
- val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
- val &= ~(MVPP2_GMAC_FORCE_LINK_DOWN |
- MVPP2_GMAC_CONFIG_MII_SPEED |
- MVPP2_GMAC_CONFIG_GMII_SPEED |
- MVPP2_GMAC_CONFIG_FULL_DUPLEX);
- val |= MVPP2_GMAC_FORCE_LINK_PASS;
+ val = MVPP2_GMAC_FORCE_LINK_PASS;
if (speed == SPEED_1000 || speed == SPEED_2500)
val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
@@ -5180,20 +5177,27 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
if (duplex == DUPLEX_FULL)
val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
- writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+ mvpp2_modify(port->base + MVPP2_GMAC_AUTONEG_CONFIG,
+ MVPP2_GMAC_FORCE_LINK_DOWN |
+ MVPP2_GMAC_FORCE_LINK_PASS |
+ MVPP2_GMAC_CONFIG_MII_SPEED |
+ MVPP2_GMAC_CONFIG_GMII_SPEED |
+ MVPP2_GMAC_CONFIG_FULL_DUPLEX, val);
}
/* We can always update the flow control enable bits;
* these will only be effective if flow control AN
* (MVPP2_GMAC_FLOW_CTRL_AUTONEG) is disabled.
*/
- val = readl(port->base + MVPP22_GMAC_CTRL_4_REG);
- val &= ~(MVPP22_CTRL4_RX_FC_EN | MVPP22_CTRL4_TX_FC_EN);
+ val = 0;
if (tx_pause)
val |= MVPP22_CTRL4_TX_FC_EN;
if (rx_pause)
val |= MVPP22_CTRL4_RX_FC_EN;
- writel(val, port->base + MVPP22_GMAC_CTRL_4_REG);
+
+ mvpp2_modify(port->base + MVPP22_GMAC_CTRL_4_REG,
+ MVPP22_CTRL4_RX_FC_EN | MVPP22_CTRL4_TX_FC_EN,
+ val);
}
mvpp2_port_enable(port);
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 4/4] net: mvpp2: set xlg flow control in mvpp2_mac_link_up()
From: Russell King @ 2020-06-18 15:39 UTC (permalink / raw)
To: Antoine Tenart, Alexandre Belloni; +Cc: David S. Miller, Jakub Kicinski, netdev
In-Reply-To: <20200618153818.GD1551@shell.armlinux.org.uk>
Set the flow control settings in mvpp2_mac_link_up() for 10G links
just as we do for 1G and slower links. This is now the preferred
location.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 23 +++++++++----------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 9edd8fbf18a6..1eb5652cd674 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4960,17 +4960,9 @@ static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
{
u32 val;
- val = MVPP22_XLG_CTRL0_MAC_RESET_DIS;
- if (state->pause & MLO_PAUSE_TX)
- val |= MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
-
- if (state->pause & MLO_PAUSE_RX)
- val |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
-
mvpp2_modify(port->base + MVPP22_XLG_CTRL0_REG,
- MVPP22_XLG_CTRL0_MAC_RESET_DIS |
- MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN |
- MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN, val);
+ MVPP22_XLG_CTRL0_MAC_RESET_DIS,
+ MVPP22_XLG_CTRL0_MAC_RESET_DIS);
mvpp2_modify(port->base + MVPP22_XLG_CTRL4_REG,
MVPP22_XLG_CTRL4_MACMODSELECT_GMAC |
MVPP22_XLG_CTRL4_EN_IDLE_CHECK |
@@ -5160,10 +5152,17 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
if (mvpp2_is_xlg(interface)) {
if (!phylink_autoneg_inband(mode)) {
+ val = MVPP22_XLG_CTRL0_FORCE_LINK_PASS;
+ if (tx_pause)
+ val |= MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
+ if (rx_pause)
+ val |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
+
mvpp2_modify(port->base + MVPP22_XLG_CTRL0_REG,
MVPP22_XLG_CTRL0_FORCE_LINK_DOWN |
- MVPP22_XLG_CTRL0_FORCE_LINK_PASS,
- MVPP22_XLG_CTRL0_FORCE_LINK_PASS);
+ MVPP22_XLG_CTRL0_FORCE_LINK_PASS |
+ MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN |
+ MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN, val);
}
} else {
if (!phylink_autoneg_inband(mode)) {
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v5 3/3] net: phy: mscc: handle the clkout control on some phy variants
From: Heiko Stübner @ 2020-06-18 15:41 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Andrew Lunn, davem, kuba, robh+dt, f.fainelli, hkallweit1, netdev,
devicetree, linux-kernel, christoph.muellner
In-Reply-To: <20200618134102.GA1551@shell.armlinux.org.uk>
Am Donnerstag, 18. Juni 2020, 15:41:02 CEST schrieb Russell King - ARM Linux admin:
> On Thu, Jun 18, 2020 at 03:28:22PM +0200, Andrew Lunn wrote:
> > On Thu, Jun 18, 2020 at 02:11:39PM +0200, Heiko Stuebner wrote:
> > > From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > >
> > > At least VSC8530/8531/8540/8541 contain a clock output that can emit
> > > a predefined rate of 25, 50 or 125MHz.
> > >
> > > This may then feed back into the network interface as source clock.
> > > So expose a clock-provider from the phy using the common clock framework
> > > to allow setting the rate.
> > >
> > > Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > > ---
> > > drivers/net/phy/mscc/mscc.h | 13 +++
> > > drivers/net/phy/mscc/mscc_main.c | 182 +++++++++++++++++++++++++++++--
> > > 2 files changed, 187 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
> > > index fbcee5fce7b2..94883dab5cc1 100644
> > > --- a/drivers/net/phy/mscc/mscc.h
> > > +++ b/drivers/net/phy/mscc/mscc.h
> > > @@ -218,6 +218,13 @@ enum rgmii_clock_delay {
> > > #define INT_MEM_DATA_M 0x00ff
> > > #define INT_MEM_DATA(x) (INT_MEM_DATA_M & (x))
> > >
> > > +#define MSCC_CLKOUT_CNTL 13
> > > +#define CLKOUT_ENABLE BIT(15)
> > > +#define CLKOUT_FREQ_MASK GENMASK(14, 13)
> > > +#define CLKOUT_FREQ_25M (0x0 << 13)
> > > +#define CLKOUT_FREQ_50M (0x1 << 13)
> > > +#define CLKOUT_FREQ_125M (0x2 << 13)
> > > +
> > > #define MSCC_PHY_PROC_CMD 18
> > > #define PROC_CMD_NCOMPLETED 0x8000
> > > #define PROC_CMD_FAILED 0x4000
> > > @@ -360,6 +367,12 @@ struct vsc8531_private {
> > > */
> > > unsigned int base_addr;
> > >
> > > +#ifdef CONFIG_COMMON_CLK
> > > + struct clk_hw clkout_hw;
> > > +#endif
> > > + u32 clkout_rate;
> > > + int clkout_enabled;
> > > +
> > > #if IS_ENABLED(CONFIG_MACSEC)
> > > /* MACsec fields:
> > > * - One SecY per device (enforced at the s/w implementation level)
> > > diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
> > > index 5d2777522fb4..727a9dd58403 100644
> > > --- a/drivers/net/phy/mscc/mscc_main.c
> > > +++ b/drivers/net/phy/mscc/mscc_main.c
> > > @@ -7,6 +7,7 @@
> > > * Copyright (c) 2016 Microsemi Corporation
> > > */
> > >
> > > +#include <linux/clk-provider.h>
> > > #include <linux/firmware.h>
> > > #include <linux/jiffies.h>
> > > #include <linux/kernel.h>
> > > @@ -431,7 +432,6 @@ static int vsc85xx_dt_led_mode_get(struct phy_device *phydev,
> > >
> > > return led_mode;
> > > }
> > > -
> > > #else
> > > static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
> > > {
> > > @@ -1508,6 +1508,43 @@ static int vsc85xx_config_init(struct phy_device *phydev)
> > > return 0;
> > > }
> > >
> > > +static int vsc8531_config_init(struct phy_device *phydev)
> > > +{
> > > + struct vsc8531_private *vsc8531 = phydev->priv;
> > > + u16 val;
> > > + int rc;
> > > +
> > > + rc = vsc85xx_config_init(phydev);
> > > + if (rc)
> > > + return rc;
> > > +
> > > +#ifdef CONFIG_COMMON_CLK
> > > + switch (vsc8531->clkout_rate) {
> > > + case 25000000:
> > > + val = CLKOUT_FREQ_25M;
> > > + break;
> > > + case 50000000:
> > > + val = CLKOUT_FREQ_50M;
> > > + break;
> > > + case 125000000:
> > > + val = CLKOUT_FREQ_125M;
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (vsc8531->clkout_enabled)
> > > + val |= CLKOUT_ENABLE;
> > > +
> > > + rc = phy_write_paged(phydev, MSCC_PHY_PAGE_EXTENDED_GPIO,
> > > + MSCC_CLKOUT_CNTL, val);
> > > + if (rc)
> > > + return rc;
> > > +#endif
> > > +
> > > + return 0;
> > > +}
> > > +
> >
> > > +static int vsc8531_clkout_prepare(struct clk_hw *hw)
> > > +{
> > > + struct vsc8531_private *vsc8531 = clkout_hw_to_vsc8531(hw);
> > > +
> > > + vsc8531->clkout_enabled = true;
> > > + return 0;
> > > +}
> > > +
> > > +static void vsc8531_clkout_unprepare(struct clk_hw *hw)
> > > +{
> > > + struct vsc8531_private *vsc8531 = clkout_hw_to_vsc8531(hw);
> > > +
> > > + vsc8531->clkout_enabled = false;
> > > +}
> > > +
> >
> > > +static const struct clk_ops vsc8531_clkout_ops = {
> > > + .prepare = vsc8531_clkout_prepare,
> > > + .unprepare = vsc8531_clkout_unprepare,
> > > + .is_prepared = vsc8531_clkout_is_prepared,
> > > + .recalc_rate = vsc8531_clkout_recalc_rate,
> > > + .round_rate = vsc8531_clkout_round_rate,
> > > + .set_rate = vsc8531_clkout_set_rate,
> >
> > I'm not sure this is the expected behaviour. The clk itself should
> > only start ticking when the enable callback is called. But this code
> > will enable the clock when config_init() is called. I think you should
> > implement the enable and disable methods.
>
> That is actually incorrect. The whole "prepare" vs "enable" difference
> is that prepare can schedule, enable isn't permitted. So, if you need
> to sleep to enable the clock, then enabling the clock in the prepare
> callback is the right thing to do.
>
> However, the above driver just sets a flag, which only gets used when
> the PHY's config_init method is called; that really doesn't seem to be
> sane - the clock is available from the point that the PHY has been
> probed, and it'll be expected that once the clock is published, it can
> be made functional.
Though I'm not sure how this fits in the whole bringup of ethernet phys.
Like the phy is dependent on the underlying ethernet controller to
actually turn it on.
I guess we should check the phy-state and if it's not accessible, just
keep the values and if it's in a suitable state do the configuration.
Calling a vsc8531_config_clkout() from both the vsc8531_config_init()
as well as the clk_(un-)prepare and clk_set_rate functions and being
protected by a check against phy_is_started() ?
Heiko
^ permalink raw reply
* Re: [net-next 14/15] iecm: Add iecm to the kernel build system
From: Jakub Kicinski @ 2020-06-18 15:43 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem, Alice Michael, netdev, nhorman, sassmann, Alan Brady,
Phani Burra, Joshua Hay, Madhu Chittim, Pavan Kumar Linga,
Donald Skidmore, Jesse Brandeburg, Sridhar Samudrala
In-Reply-To: <20200618051344.516587-15-jeffrey.t.kirsher@intel.com>
On Wed, 17 Jun 2020 22:13:43 -0700 Jeff Kirsher wrote:
> From: Alice Michael <alice.michael@intel.com>
>
> This introduces iecm as a module to the kernel, and adds
> relevant documentation.
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:45:17: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:45:17: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:45:17: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:47:17: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:47:17: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:47:17: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:54:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:54:9: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:54:9: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:57:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:57:9: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:57:9: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:58:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:58:9: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:58:9: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:59:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:59:9: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:59:9: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:62:15: warning: incorrect type in argument 1 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:62:15: expected void const volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:62:15: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:321:33: warning: incorrect type in assignment (different base types)
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:321:33: expected restricted __le16 [usertype] pfid_vfid
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:321:33: got unsigned short [usertype] func_id
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:364:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:364:9: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:364:9: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:563:17: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:563:17: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:563:17: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_lib.c:49:13: warning: symbol 'iecm_mb_intr_clean' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_lib.c:70:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_lib.c:70:9: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_lib.c:70:9: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_lib.c:63:6: warning: symbol 'iecm_mb_irq_enable' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_lib.c:77:5: warning: symbol 'iecm_mb_intr_req_irq' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_lib.c:104:6: warning: symbol 'iecm_get_mb_vec_id' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_lib.c:122:5: warning: symbol 'iecm_mb_intr_init' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_lib.c:140:6: warning: symbol 'iecm_intr_distribute' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_lib.c:299:21: warning: incorrect type in assignment (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_lib.c:299:21: expected unsigned char [usertype] *hw_addr
../drivers/net/ethernet/intel/iecm/iecm_lib.c:299:21: got void [noderef] <asn:2> *
../drivers/net/ethernet/intel/iecm/iecm_lib.c:417:5: warning: symbol 'iecm_vport_rel' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_lib.c:748:6: warning: symbol 'iecm_deinit_task' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:84:6: warning: symbol 'iecm_tx_buf_rel_all' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:115:6: warning: symbol 'iecm_tx_desc_rel' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:136:6: warning: symbol 'iecm_tx_desc_rel_all' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:312:6: warning: symbol 'iecm_rx_buf_rel_all' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:341:6: warning: symbol 'iecm_rx_desc_rel' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:371:6: warning: symbol 'iecm_rx_desc_rel_all' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:486:6: warning: symbol 'iecm_rx_hdr_buf_hw_alloc' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:564:47: warning: incorrect type in assignment (different base types)
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:564:47: expected restricted __le16 [usertype] buf_id
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:564:47: got restricted __le64 [usertype]
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:2072:50: warning: Using plain integer as NULL pointer
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:2319:27: warning: cast to restricted __le32
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:2319:27: warning: cast from restricted __le16
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:2852:23: warning: cast to restricted __le16
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3086:17: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3086:17: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3086:17: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3414:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3414:9: expected void volatile [noderef] <asn:2> *addr
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3414:9: got unsigned char [usertype] *
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3565:5: warning: symbol 'iecm_vport_splitq_napi_poll' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3603:6: warning: symbol 'iecm_vport_intr_map_vector_to_qs' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3715:5: warning: symbol 'iecm_vport_intr_alloc' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:1702:34: warning: incorrect type in assignment (different base types)
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:1702:34: expected unsigned char [usertype] cmd_dtype
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:1702:34: got restricted __le16 [usertype]
../drivers/net/ethernet/intel/iecm/iecm_virtchnl.c:12:6: warning: symbol 'iecm_recv_event_msg' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_virtchnl.c:61:1: warning: symbol 'iecm_mb_clean' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_main.c:17:5: warning: symbol 'debug' was not declared. Should it be static?
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:144: warning: Function parameter or member 'qinfo' not described in 'iecm_ctlq_add'
../drivers/net/ethernet/intel/iecm/iecm_controlq.c:144: warning: Excess function parameter 'q_info' description in 'iecm_ctlq_add'
In file included from ../include/linux/net/intel/iecm.h:50,
from ../drivers/net/ethernet/intel/iecm/iecm_osdep.c:5:
../include/linux/net/intel/iecm_txrx.h:293:30: warning: ‘iecm_rx_ptype_lkup’ defined but not used [-Wunused-const-variable=]
293 | struct iecm_rx_ptype_decoded iecm_rx_ptype_lkup[IECM_RX_SUPP_PTYPE] = {
| ^~~~~~~~~~~~~~~~~~
In file included from ../include/linux/net/intel/iecm.h:50,
from ../drivers/net/ethernet/intel/iecm/iecm_singleq_txrx.c:5:
../include/linux/net/intel/iecm_txrx.h:293:30: warning: ‘iecm_rx_ptype_lkup’ defined but not used [-Wunused-const-variable=]
293 | struct iecm_rx_ptype_decoded iecm_rx_ptype_lkup[IECM_RX_SUPP_PTYPE] = {
| ^~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_singleq_txrx.c:584: warning: Function parameter or member 'rx_desc' not described in 'iecm_rx_singleq_process_skb_fields'
../drivers/net/ethernet/intel/iecm/iecm_singleq_txrx.c:584: warning: Function parameter or member 'ptype' not described in 'iecm_rx_singleq_process_skb_fields'
../drivers/net/ethernet/intel/iecm/iecm_singleq_txrx.c:645: warning: bad line:
../drivers/net/ethernet/intel/iecm/iecm_singleq_txrx.c:689: warning: Function parameter or member 'dev' not described in 'iecm_singleq_rx_get_buf_page'
../drivers/net/ethernet/intel/iecm/iecm_lib.c:49:13: warning: no previous prototype for ‘iecm_mb_intr_clean’ [-Wmissing-prototypes]
49 | irqreturn_t iecm_mb_intr_clean(int __always_unused irq, void *data)
| ^~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_lib.c:63:6: warning: no previous prototype for ‘iecm_mb_irq_enable’ [-Wmissing-prototypes]
63 | void iecm_mb_irq_enable(struct iecm_adapter *adapter)
| ^~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_lib.c:77:5: warning: no previous prototype for ‘iecm_mb_intr_req_irq’ [-Wmissing-prototypes]
77 | int iecm_mb_intr_req_irq(struct iecm_adapter *adapter)
| ^~~~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_lib.c:104:6: warning: no previous prototype for ‘iecm_get_mb_vec_id’ [-Wmissing-prototypes]
104 | void iecm_get_mb_vec_id(struct iecm_adapter *adapter)
| ^~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_lib.c:122:5: warning: no previous prototype for ‘iecm_mb_intr_init’ [-Wmissing-prototypes]
122 | int iecm_mb_intr_init(struct iecm_adapter *adapter)
| ^~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_lib.c:140:6: warning: no previous prototype for ‘iecm_intr_distribute’ [-Wmissing-prototypes]
140 | void iecm_intr_distribute(struct iecm_adapter *adapter)
| ^~~~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_lib.c:417:5: warning: no previous prototype for ‘iecm_vport_rel’ [-Wmissing-prototypes]
417 | int iecm_vport_rel(struct iecm_vport *vport)
| ^~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_lib.c:748:6: warning: no previous prototype for ‘iecm_deinit_task’ [-Wmissing-prototypes]
748 | void iecm_deinit_task(struct iecm_adapter *adapter)
| ^~~~~~~~~~~~~~~~
In file included from ../include/linux/net/intel/iecm.h:50,
from ../drivers/net/ethernet/intel/iecm/iecm_lib.c:6:
../include/linux/net/intel/iecm_txrx.h:293:30: warning: ‘iecm_rx_ptype_lkup’ defined but not used [-Wunused-const-variable=]
293 | struct iecm_rx_ptype_decoded iecm_rx_ptype_lkup[IECM_RX_SUPP_PTYPE] = {
| ^~~~~~~~~~~~~~~~~~
In file included from ../include/linux/net/intel/iecm.h:50,
from ../drivers/net/ethernet/intel/iecm/iecm_ethtool.c:4:
../include/linux/net/intel/iecm_txrx.h:293:30: warning: ‘iecm_rx_ptype_lkup’ defined but not used [-Wunused-const-variable=]
293 | struct iecm_rx_ptype_decoded iecm_rx_ptype_lkup[IECM_RX_SUPP_PTYPE] = {
| ^~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_lib.c:490: warning: Function parameter or member 'vport_id' not described in 'iecm_vport_alloc'
../drivers/net/ethernet/intel/iecm/iecm_lib.c:490: warning: Excess function parameter 'vport_type' description in 'iecm_vport_alloc'
../drivers/net/ethernet/intel/iecm/iecm_ethtool.c:80: warning: Function parameter or member 'abs_rx_qid' not described in 'iecm_find_virtual_qid'
../drivers/net/ethernet/intel/iecm/iecm_ethtool.c:1031: warning: Function parameter or member 'cmd' not described in 'iecm_get_link_ksettings'
../drivers/net/ethernet/intel/iecm/iecm_ethtool.c:1031: warning: Excess function parameter 'ecmd' description in 'iecm_get_link_ksettings'
In file included from ../include/linux/net/intel/iecm.h:50,
from ../drivers/net/ethernet/intel/iecm/iecm_main.c:6:
../include/linux/net/intel/iecm_txrx.h:293:30: warning: ‘iecm_rx_ptype_lkup’ defined but not used [-Wunused-const-variable=]
293 | struct iecm_rx_ptype_decoded iecm_rx_ptype_lkup[IECM_RX_SUPP_PTYPE] = {
| ^~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_virtchnl.c:12:6: warning: no previous prototype for ‘iecm_recv_event_msg’ [-Wmissing-prototypes]
12 | void iecm_recv_event_msg(struct iecm_vport *vport)
| ^~~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_virtchnl.c:61:1: warning: no previous prototype for ‘iecm_mb_clean’ [-Wmissing-prototypes]
61 | iecm_mb_clean(struct iecm_adapter *adapter)
| ^~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_virtchnl.c:1422: warning: Function parameter or member 'vport' not described in 'iecm_send_get_stats_msg'
../drivers/net/ethernet/intel/iecm/iecm_virtchnl.c:1422: warning: Excess function parameter 'adapter' description in 'iecm_send_get_stats_msg'
../drivers/net/ethernet/intel/iecm/iecm_virtchnl.c:1701: warning: Function parameter or member 'hw' not described in 'iecm_find_ctlq'
../drivers/net/ethernet/intel/iecm/iecm_virtchnl.c:1701: warning: Excess function parameter 'adapter' description in 'iecm_find_ctlq'
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:84:6: warning: no previous prototype for ‘iecm_tx_buf_rel_all’ [-Wmissing-prototypes]
84 | void iecm_tx_buf_rel_all(struct iecm_queue *txq)
| ^~~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:115:6: warning: no previous prototype for ‘iecm_tx_desc_rel’ [-Wmissing-prototypes]
115 | void iecm_tx_desc_rel(struct iecm_queue *txq, bool bufq)
| ^~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:136:6: warning: no previous prototype for ‘iecm_tx_desc_rel_all’ [-Wmissing-prototypes]
136 | void iecm_tx_desc_rel_all(struct iecm_vport *vport)
| ^~~~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:312:6: warning: no previous prototype for ‘iecm_rx_buf_rel_all’ [-Wmissing-prototypes]
312 | void iecm_rx_buf_rel_all(struct iecm_queue *rxq)
| ^~~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:341:6: warning: no previous prototype for ‘iecm_rx_desc_rel’ [-Wmissing-prototypes]
341 | void iecm_rx_desc_rel(struct iecm_queue *rxq, bool bufq,
| ^~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:371:6: warning: no previous prototype for ‘iecm_rx_desc_rel_all’ [-Wmissing-prototypes]
371 | void iecm_rx_desc_rel_all(struct iecm_vport *vport)
| ^~~~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:486:6: warning: no previous prototype for ‘iecm_rx_hdr_buf_hw_alloc’ [-Wmissing-prototypes]
486 | bool iecm_rx_hdr_buf_hw_alloc(struct iecm_queue *rxq,
| ^~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3565:5: warning: no previous prototype for ‘iecm_vport_splitq_napi_poll’ [-Wmissing-prototypes]
3565 | int iecm_vport_splitq_napi_poll(struct napi_struct *napi, int budget)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3603:6: warning: no previous prototype for ‘iecm_vport_intr_map_vector_to_qs’ [-Wmissing-prototypes]
3603 | void iecm_vport_intr_map_vector_to_qs(struct iecm_vport *vport)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3715:5: warning: no previous prototype for ‘iecm_vport_intr_alloc’ [-Wmissing-prototypes]
3715 | int iecm_vport_intr_alloc(struct iecm_vport *vport)
| ^~~~~~~~~~~~~~~~~~~~~
In file included from ../include/linux/net/intel/iecm.h:50,
from ../drivers/net/ethernet/intel/iecm/iecm_txrx.c:4:
../include/linux/net/intel/iecm_txrx.h:293:30: warning: ‘iecm_rx_ptype_lkup’ defined but not used [-Wunused-const-variable=]
293 | struct iecm_rx_ptype_decoded iecm_rx_ptype_lkup[IECM_RX_SUPP_PTYPE] = {
| ^~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:2593: warning: Function parameter or member 'dev' not described in 'iecm_rx_get_buf_page'
../drivers/net/ethernet/intel/iecm/iecm_txrx.c:3841: warning: Function parameter or member 'qid_list' not described in 'iecm_get_rx_qid_list'
^ permalink raw reply
* [PATCH net] ibmveth: Fix max MTU limit
From: Thomas Falcon @ 2020-06-18 15:43 UTC (permalink / raw)
To: netdev; +Cc: linuxppc-dev, Thomas Falcon
The max MTU limit defined for ibmveth is not accounting for
virtual ethernet buffer overhead, which is twenty-two additional
bytes set aside for the ethernet header and eight additional bytes
of an opaque handle reserved for use by the hypervisor. Update the
max MTU to reflect this overhead.
Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
drivers/net/ethernet/ibm/ibmveth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index 96d36ae5049e..c5c732601e35 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1715,7 +1715,7 @@ static int ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id)
}
netdev->min_mtu = IBMVETH_MIN_MTU;
- netdev->max_mtu = ETH_MAX_MTU;
+ netdev->max_mtu = ETH_MAX_MTU - IBMVETH_BUFF_OH;
memcpy(netdev->dev_addr, mac_addr_p, ETH_ALEN);
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v1 2/3] net/fsl: acpize xgmac_mdio
From: Jeremy Linton @ 2020-06-18 15:43 UTC (permalink / raw)
To: Andrew Lunn, Calvin Johnson
Cc: Russell King - ARM Linux admin, Jon, Cristi Sovaiala,
Ioana Ciornei, Andy Shevchenko, Florian Fainelli, Madalin Bucur,
netdev, linux.cj
In-Reply-To: <20200617173414.GI205574@lunn.ch>
Hi,
On 6/17/20 12:34 PM, Andrew Lunn wrote:
> On Wed, Jun 17, 2020 at 10:45:34PM +0530, Calvin Johnson wrote:
>> From: Jeremy Linton <jeremy.linton@arm.com>
>
>> +static const struct acpi_device_id xgmac_acpi_match[] = {
>> + { "NXP0006", (kernel_ulong_t)NULL },
>
> Hi Jeremy
>
> What exactly does NXP0006 represent? An XGMAC MDIO bus master? Some
> NXP MDIO bus master? An XGMAC Ethernet controller which has an NXP
> MDIO bus master? A cluster of Ethernet controllers?
Strictly speaking its a NXP defined (they own the "NXP" prefix per
https://uefi.org/pnp_id_list) id. So, they have tied it to a specific
bit of hardware. In this case it appears to be a shared MDIO master
which isn't directly contained in an Ethernet controller. Its somewhat
similar to a "nxp,xxxxx" compatible id, depending on how they are using
it to identify an ACPI device object (_HID()/_CID()).
So AFAIK, this is all valid ACPI usage as long as the ID maps to a
unique device/object.
>
> Is this documented somewhere? In the DT world we have a clear
> documentation for all the compatible strings. Is there anything
> similar in the ACPI world for these magic numbers?
Sadly not fully. The mentioned PNP and ACPI
(https://uefi.org/acpi_id_list) ids lists are requested and registered
to a given organization. But, once the prefix is owned, it becomes the
responsibility of that organization to assign & manage the ID's with
their prefix. There are various individuals/etc which have collected
lists, though like PCI ids, there aren't any formal publishing requirements.
^ permalink raw reply
* Re: [RFC PATCH 3/9] net: dsa: hellcreek: Add PTP clock support
From: Jakub Kicinski @ 2020-06-18 15:46 UTC (permalink / raw)
To: Kurt Kanzenbach
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev, Rob Herring, devicetree, Sebastian Andrzej Siewior,
Richard Cochran, Kamil Alkhouri, ilias.apalodimas
In-Reply-To: <20200618064029.32168-4-kurt@linutronix.de>
On Thu, 18 Jun 2020 08:40:23 +0200 Kurt Kanzenbach wrote:
> From: Kamil Alkhouri <kamil.alkhouri@hs-offenburg.de>
>
> The switch has internal PTP hardware clocks. Add support for it. There are three
> clocks:
>
> * Synchronized
> * Syntonized
> * Free running
>
> Currently the synchronized clock is exported to user space which is a good
> default for the beginning. The free running clock might be exported later
> e.g. for implementing 802.1AS-2011/2020 Time Aware Bridges (TAB). The switch
> also supports cross time stamping for that purpose.
>
> The implementation adds support setting/getting the time as well as offset and
> frequency adjustments. However, the clock only holds a partial timeofday
> timestamp. This is why we track the seconds completely in software (see overflow
> work and last_ts).
>
> Furthermore, add the PTP multicast addresses into the FDB to forward that
> packages only to the CPU port where they are processed by a PTP program.
>
> Signed-off-by: Kamil Alkhouri <kamil.alkhouri@hs-offenburg.de>
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
Please make sure each patch in the series builds cleanly with the W=1
C=1 flags. Here we have:
../drivers/net/dsa/hirschmann/hellcreek_ptp.c: In function ‘__hellcreek_ptp_clock_read’:
../drivers/net/dsa/hirschmann/hellcreek_ptp.c:30:28: warning: variable ‘sech’ set but not used [-Wunused-but-set-variable]
30 | u16 nsl, nsh, secl, secm, sech;
| ^~~~
../drivers/net/dsa/hirschmann/hellcreek_ptp.c:30:22: warning: variable ‘secm’ set but not used [-Wunused-but-set-variable]
30 | u16 nsl, nsh, secl, secm, sech;
| ^~~~
../drivers/net/dsa/hirschmann/hellcreek_ptp.c:30:16: warning: variable ‘secl’ set but not used [-Wunused-but-set-variable]
30 | u16 nsl, nsh, secl, secm, sech;
| ^~~~
Next patch adds a few more.
^ permalink raw reply
* Re: [PATCH net-next 4/5] net: phy: add Lynx PCS MDIO module
From: Jakub Kicinski @ 2020-06-18 15:47 UTC (permalink / raw)
To: Ioana Ciornei
Cc: netdev, davem, vladimir.oltean, claudiu.manoil,
alexandru.marginean, michael, andrew, linux, f.fainelli
In-Reply-To: <20200618120837.27089-5-ioana.ciornei@nxp.com>
On Thu, 18 Jun 2020 15:08:36 +0300 Ioana Ciornei wrote:
> +#if IS_ENABLED(CONFIG_MDIO_LYNX_PCS)
> +struct mdio_lynx_pcs *mdio_lynx_pcs_create(struct mdio_device *mdio_dev);
> +
> +void mdio_lynx_pcs_free(struct mdio_lynx_pcs *pcs);
> +#else
> +static struct mdio_lynx_pcs *mdio_lynx_pcs_create(struct mdio_device *mdio_dev)
> +{
> + return NULL;
> +}
> +
> +static void mdio_lynx_pcs_free(struct mdio_lynx_pcs *pcs)
> +{
> +}
> +#endif
Do you want these to be static inline?
^ permalink raw reply
* Re: [PATCH v5 3/3] net: phy: mscc: handle the clkout control on some phy variants
From: Russell King - ARM Linux admin @ 2020-06-18 15:47 UTC (permalink / raw)
To: Heiko Stübner
Cc: Andrew Lunn, davem, kuba, robh+dt, f.fainelli, hkallweit1, netdev,
devicetree, linux-kernel, christoph.muellner
In-Reply-To: <2277698.LFZWc9m3Y3@diego>
On Thu, Jun 18, 2020 at 05:41:54PM +0200, Heiko Stübner wrote:
> Am Donnerstag, 18. Juni 2020, 15:41:02 CEST schrieb Russell King - ARM Linux admin:
> > On Thu, Jun 18, 2020 at 03:28:22PM +0200, Andrew Lunn wrote:
> > > On Thu, Jun 18, 2020 at 02:11:39PM +0200, Heiko Stuebner wrote:
> > > > From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > > >
> > > > At least VSC8530/8531/8540/8541 contain a clock output that can emit
> > > > a predefined rate of 25, 50 or 125MHz.
> > > >
> > > > This may then feed back into the network interface as source clock.
> > > > So expose a clock-provider from the phy using the common clock framework
> > > > to allow setting the rate.
> > > >
> > > > Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > > > ---
> > > > drivers/net/phy/mscc/mscc.h | 13 +++
> > > > drivers/net/phy/mscc/mscc_main.c | 182 +++++++++++++++++++++++++++++--
> > > > 2 files changed, 187 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
> > > > index fbcee5fce7b2..94883dab5cc1 100644
> > > > --- a/drivers/net/phy/mscc/mscc.h
> > > > +++ b/drivers/net/phy/mscc/mscc.h
> > > > @@ -218,6 +218,13 @@ enum rgmii_clock_delay {
> > > > #define INT_MEM_DATA_M 0x00ff
> > > > #define INT_MEM_DATA(x) (INT_MEM_DATA_M & (x))
> > > >
> > > > +#define MSCC_CLKOUT_CNTL 13
> > > > +#define CLKOUT_ENABLE BIT(15)
> > > > +#define CLKOUT_FREQ_MASK GENMASK(14, 13)
> > > > +#define CLKOUT_FREQ_25M (0x0 << 13)
> > > > +#define CLKOUT_FREQ_50M (0x1 << 13)
> > > > +#define CLKOUT_FREQ_125M (0x2 << 13)
> > > > +
> > > > #define MSCC_PHY_PROC_CMD 18
> > > > #define PROC_CMD_NCOMPLETED 0x8000
> > > > #define PROC_CMD_FAILED 0x4000
> > > > @@ -360,6 +367,12 @@ struct vsc8531_private {
> > > > */
> > > > unsigned int base_addr;
> > > >
> > > > +#ifdef CONFIG_COMMON_CLK
> > > > + struct clk_hw clkout_hw;
> > > > +#endif
> > > > + u32 clkout_rate;
> > > > + int clkout_enabled;
> > > > +
> > > > #if IS_ENABLED(CONFIG_MACSEC)
> > > > /* MACsec fields:
> > > > * - One SecY per device (enforced at the s/w implementation level)
> > > > diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
> > > > index 5d2777522fb4..727a9dd58403 100644
> > > > --- a/drivers/net/phy/mscc/mscc_main.c
> > > > +++ b/drivers/net/phy/mscc/mscc_main.c
> > > > @@ -7,6 +7,7 @@
> > > > * Copyright (c) 2016 Microsemi Corporation
> > > > */
> > > >
> > > > +#include <linux/clk-provider.h>
> > > > #include <linux/firmware.h>
> > > > #include <linux/jiffies.h>
> > > > #include <linux/kernel.h>
> > > > @@ -431,7 +432,6 @@ static int vsc85xx_dt_led_mode_get(struct phy_device *phydev,
> > > >
> > > > return led_mode;
> > > > }
> > > > -
> > > > #else
> > > > static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
> > > > {
> > > > @@ -1508,6 +1508,43 @@ static int vsc85xx_config_init(struct phy_device *phydev)
> > > > return 0;
> > > > }
> > > >
> > > > +static int vsc8531_config_init(struct phy_device *phydev)
> > > > +{
> > > > + struct vsc8531_private *vsc8531 = phydev->priv;
> > > > + u16 val;
> > > > + int rc;
> > > > +
> > > > + rc = vsc85xx_config_init(phydev);
> > > > + if (rc)
> > > > + return rc;
> > > > +
> > > > +#ifdef CONFIG_COMMON_CLK
> > > > + switch (vsc8531->clkout_rate) {
> > > > + case 25000000:
> > > > + val = CLKOUT_FREQ_25M;
> > > > + break;
> > > > + case 50000000:
> > > > + val = CLKOUT_FREQ_50M;
> > > > + break;
> > > > + case 125000000:
> > > > + val = CLKOUT_FREQ_125M;
> > > > + break;
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + if (vsc8531->clkout_enabled)
> > > > + val |= CLKOUT_ENABLE;
> > > > +
> > > > + rc = phy_write_paged(phydev, MSCC_PHY_PAGE_EXTENDED_GPIO,
> > > > + MSCC_CLKOUT_CNTL, val);
> > > > + if (rc)
> > > > + return rc;
> > > > +#endif
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > >
> > > > +static int vsc8531_clkout_prepare(struct clk_hw *hw)
> > > > +{
> > > > + struct vsc8531_private *vsc8531 = clkout_hw_to_vsc8531(hw);
> > > > +
> > > > + vsc8531->clkout_enabled = true;
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static void vsc8531_clkout_unprepare(struct clk_hw *hw)
> > > > +{
> > > > + struct vsc8531_private *vsc8531 = clkout_hw_to_vsc8531(hw);
> > > > +
> > > > + vsc8531->clkout_enabled = false;
> > > > +}
> > > > +
> > >
> > > > +static const struct clk_ops vsc8531_clkout_ops = {
> > > > + .prepare = vsc8531_clkout_prepare,
> > > > + .unprepare = vsc8531_clkout_unprepare,
> > > > + .is_prepared = vsc8531_clkout_is_prepared,
> > > > + .recalc_rate = vsc8531_clkout_recalc_rate,
> > > > + .round_rate = vsc8531_clkout_round_rate,
> > > > + .set_rate = vsc8531_clkout_set_rate,
> > >
> > > I'm not sure this is the expected behaviour. The clk itself should
> > > only start ticking when the enable callback is called. But this code
> > > will enable the clock when config_init() is called. I think you should
> > > implement the enable and disable methods.
> >
> > That is actually incorrect. The whole "prepare" vs "enable" difference
> > is that prepare can schedule, enable isn't permitted. So, if you need
> > to sleep to enable the clock, then enabling the clock in the prepare
> > callback is the right thing to do.
> >
> > However, the above driver just sets a flag, which only gets used when
> > the PHY's config_init method is called; that really doesn't seem to be
> > sane - the clock is available from the point that the PHY has been
> > probed, and it'll be expected that once the clock is published, it can
> > be made functional.
>
> Though I'm not sure how this fits in the whole bringup of ethernet phys.
> Like the phy is dependent on the underlying ethernet controller to
> actually turn it on.
>
> I guess we should check the phy-state and if it's not accessible, just
> keep the values and if it's in a suitable state do the configuration.
>
> Calling a vsc8531_config_clkout() from both the vsc8531_config_init()
> as well as the clk_(un-)prepare and clk_set_rate functions and being
> protected by a check against phy_is_started() ?
It sounds like it doesn't actually fit the clk API paradym then. I
see that Rob suggested it, and from the DT point of view, it makes
complete sense, but then if the hardware can't actually be used in
the way the clk API expects it to be used, then there's a semantic
problem.
What is this clock used for?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH net-next 5/5] net: dsa: felix: use the Lynx PCS helpers
From: Jakub Kicinski @ 2020-06-18 15:48 UTC (permalink / raw)
To: Ioana Ciornei
Cc: netdev, davem, vladimir.oltean, claudiu.manoil,
alexandru.marginean, michael, andrew, linux, f.fainelli
In-Reply-To: <20200618120837.27089-6-ioana.ciornei@nxp.com>
On Thu, 18 Jun 2020 15:08:37 +0300 Ioana Ciornei wrote:
> Use the helper functions introduced by the newly added
> Lynx PCS MDIO module.
>
> Instead of representing the PCS as a phy_device, a mdio_device structure
> will be passed to the Lynx module which is now actually implementing all
> the PCS configuration and status reporting.
>
> All code previously used for PCS momnitoring and runtime configuration
> is removed and replaced will calls to the Lynx PCS operations.
>
> Tested on the following SERDES protocols of LS1028A: 0x7777
> (2500Base-X), 0x85bb (QSGMII), 0x9999 (SGMII) and 0x13bb (USXGMII).
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Hm, this does not build with allmodconfig.
drivers/net/phy/mdio-lynx-pcs.o: In function `lynx_pcs_link_up':
mdio-lynx-pcs.c:(.text+0x115): undefined reference to `mdiobus_modify'
mdio-lynx-pcs.c:(.text+0x1a3): undefined reference to `mdiobus_write'
drivers/net/phy/mdio-lynx-pcs.o: In function `lynx_pcs_config':
mdio-lynx-pcs.c:(.text+0x33a): undefined reference to `mdiobus_write'
mdio-lynx-pcs.c:(.text+0x371): undefined reference to `mdiobus_modify'
mdio-lynx-pcs.c:(.text+0x384): undefined reference to `phylink_mii_c22_pcs_config'
mdio-lynx-pcs.c:(.text+0x3e4): undefined reference to `mdiobus_write'
mdio-lynx-pcs.c:(.text+0x40d): undefined reference to `mdiobus_write'
mdio-lynx-pcs.c:(.text+0x422): undefined reference to `mdiobus_write'
drivers/net/phy/mdio-lynx-pcs.o: In function `lynx_pcs_get_state_usxgmii.isra.0':
mdio-lynx-pcs.c:(.text+0x457): undefined reference to `mdiobus_read'
mdio-lynx-pcs.c:(.text+0x4f1): undefined reference to `mdiobus_read'
drivers/net/phy/mdio-lynx-pcs.o: In function `lynx_pcs_get_state':
mdio-lynx-pcs.c:(.text+0x650): undefined reference to `phylink_mii_c22_pcs_get_state'
mdio-lynx-pcs.c:(.text+0x6fb): undefined reference to `phy_duplex_to_str'
mdio-lynx-pcs.c:(.text+0x711): undefined reference to `phy_speed_to_str'
mdio-lynx-pcs.c:(.text+0x7c0): undefined reference to `mdiobus_read'
mdio-lynx-pcs.c:(.text+0x7d4): undefined reference to `mdiobus_read'
drivers/net/phy/mdio-lynx-pcs.o: In function `lynx_pcs_an_restart':
mdio-lynx-pcs.c:(.text+0x954): undefined reference to `mdiobus_write'
mdio-lynx-pcs.c:(.text+0x978): undefined reference to `phylink_mii_c22_pcs_an_restart'
make[1]: *** [vmlinux] Error 1
make: *** [__sub-make] Error 2
^ permalink raw reply
* Re: [PATCH v5 3/3] net: phy: mscc: handle the clkout control on some phy variants
From: Jakub Kicinski @ 2020-06-18 15:49 UTC (permalink / raw)
To: Heiko Stuebner
Cc: davem, robh+dt, andrew, f.fainelli, hkallweit1, linux, netdev,
devicetree, linux-kernel, christoph.muellner, Heiko Stuebner
In-Reply-To: <20200618121139.1703762-4-heiko@sntech.de>
On Thu, 18 Jun 2020 14:11:39 +0200 Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> At least VSC8530/8531/8540/8541 contain a clock output that can emit
> a predefined rate of 25, 50 or 125MHz.
>
> This may then feed back into the network interface as source clock.
> So expose a clock-provider from the phy using the common clock framework
> to allow setting the rate.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Doesn't build with allmodconfig:
../drivers/net/phy/mscc/mscc_macsec.c:391:42: warning: cast from restricted sci_t
../drivers/net/phy/mscc/mscc_macsec.c:393:42: warning: restricted sci_t degrades to integer
../drivers/net/phy/mscc/mscc_macsec.c:400:42: warning: restricted __be16 degrades to integer
../drivers/net/phy/mscc/mscc_macsec.c:606:34: warning: cast from restricted sci_t
../drivers/net/phy/mscc/mscc_macsec.c:608:34: warning: restricted sci_t degrades to integer
../drivers/net/phy/mscc/mscc_macsec.c:391:42: warning: cast from restricted sci_t
../drivers/net/phy/mscc/mscc_macsec.c:393:42: warning: restricted sci_t degrades to integer
../drivers/net/phy/mscc/mscc_macsec.c:400:42: warning: restricted __be16 degrades to integer
../drivers/net/phy/mscc/mscc_macsec.c:606:34: warning: cast from restricted sci_t
../drivers/net/phy/mscc/mscc_macsec.c:608:34: warning: restricted sci_t degrades to integer
In file included from ../drivers/net/phy/mscc/mscc_macsec.c:17:
../drivers/net/phy/mscc/mscc.h:371:16: error: field ‘clkout_hw’ has incomplete type
371 | struct clk_hw clkout_hw;
| ^~~~~~~~~
make[5]: *** [drivers/net/phy/mscc/mscc_macsec.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [drivers/net/phy/mscc] Error 2
make[3]: *** [drivers/net/phy] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [drivers/net] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [drivers] Error 2
make: *** [__sub-make] Error 2
^ permalink raw reply
* Re: [PATCH v2] net: macb: undo operations in case of failure
From: Nicolas Ferre @ 2020-06-18 15:53 UTC (permalink / raw)
To: Claudiu Beznea, davem, kuba, linux; +Cc: antoine.tenart, netdev, linux-kernel
In-Reply-To: <1592469460-17825-1-git-send-email-claudiu.beznea@microchip.com>
On 18/06/2020 at 10:37, Claudiu Beznea wrote:
> Undo previously done operation in case macb_phylink_connect()
> fails. Since macb_reset_hw() is the 1st undo operation the
> napi_exit label was renamed to reset_hw.
>
> Fixes: 7897b071ac3b ("net: macb: convert to phylink")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Thanks Claudiu.
Regards,
Nicolas
> ---
>
> Changes in v2:
> - corrected fixes SHA1
>
> drivers/net/ethernet/cadence/macb_main.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 67933079aeea..257c4920cb88 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2558,7 +2558,7 @@ static int macb_open(struct net_device *dev)
>
> err = macb_phylink_connect(bp);
> if (err)
> - goto napi_exit;
> + goto reset_hw;
>
> netif_tx_start_all_queues(dev);
>
> @@ -2567,9 +2567,11 @@ static int macb_open(struct net_device *dev)
>
> return 0;
>
> -napi_exit:
> +reset_hw:
> + macb_reset_hw(bp);
> for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue)
> napi_disable(&queue->napi);
> + macb_free_consistent(bp);
> pm_exit:
> pm_runtime_put_sync(&bp->pdev->dev);
> return err;
>
--
Nicolas Ferre
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox