* Re: [PATCH net-next 2/3] net: dsa: mediatek: combine MediaTek tag with VLAN tag
From: Andrew Lunn @ 2017-12-07 15:30 UTC (permalink / raw)
To: sean.wang
Cc: davem, f.fainelli, vivien.didelot, netdev, linux-kernel,
linux-mediatek
In-Reply-To: <dffc711dc88898f8a5f5387d1fa7d14e0faf9ed5.1512625814.git.sean.wang@mediatek.com>
> @@ -25,20 +28,37 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
> {
> struct dsa_port *dp = dsa_slave_to_port(dev);
> u8 *mtk_tag;
> + bool is_vlan_skb = true;
..
> + /* Mark tag attribute on special tag insertion to notify hardware
> + * whether that's a combined special tag with 802.1Q header.
> + */
> + mtk_tag[0] = is_vlan_skb ? MTK_HDR_XMIT_TAGGED_TPID_8100 :
> + MTK_HDR_XMIT_UNTAGGED;
> mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK;
> - mtk_tag[2] = 0;
> - mtk_tag[3] = 0;
> +
> + /* Tag control information is kept for 802.1Q */
> + if (!is_vlan_skb) {
> + mtk_tag[2] = 0;
> + mtk_tag[3] = 0;
> + }
>
> return skb;
> }
Hi Sean
So you can mark a packet for egress. What about ingress? How do you
know the VLAN/PORT combination for packets the CPU receives? I would
of expected a similar change to mtk_tag_rcv().
Andrew
^ permalink raw reply
* Re: [crypto 6/8] chtls: TCB and Key program
From: Stephan Mueller @ 2017-12-07 15:27 UTC (permalink / raw)
To: Atul Gupta
Cc: herbert@gondor.apana.org.au, linux-crypto@vger.kernel.org,
netdev@vger.kernel.org, davem@davemloft.net, davejwatson@fb.com,
Ganesh GR, Harsh Jain
In-Reply-To: <MWHPR1201MB0238C75889F4E986F338B05297330@MWHPR1201MB0238.namprd12.prod.outlook.com>
Am Donnerstag, 7. Dezember 2017, 16:08:04 CET schrieb Atul Gupta:
Hi Atul,
> > As far as I see, the key is part of the skb (via kctx). This skb is
> > released after being processed. The release calls kfree_skb which does
> > not zeroize the key. Wouldn't it make sense to clear the memory of the
> > key when the skb is released? [Atul] we should perhaps memset the info
> > received from user so that driver has no info on key once its written on
> > chip memory. memset(gcm_ctx->key, 0, keylen);
>
> Are you saying that the skb (via kctx) above does not obtain a copy of the
> key? If not, what is done in chtls_key_info? It does have a key copy, I was
> not sure how key info is accessed once skb is released.
All I am saying is that this key copy should be zapped when the memory is
released. Thus, if the skb has a copy of the key, at least at the time of
free() of the skb, a memset() of the key memory should be done (or before).
>
>
> Ciao
> Stephan
>
> Thanks
> Atul
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: dsa: mediatek: add VLAN support for MT7530
From: Andrew Lunn @ 2017-12-07 15:24 UTC (permalink / raw)
To: sean.wang
Cc: davem, f.fainelli, vivien.didelot, netdev, linux-kernel,
linux-mediatek
In-Reply-To: <792c2d52b8da717b55ead9d480863ca390fe91b3.1512625814.git.sean.wang@mediatek.com>
> static void
> +mt7530_port_set_vlan_unware(struct dsa_switch *ds, int port)
> +{
> + struct mt7530_priv *priv = ds->priv;
> + int i;
> + bool all_user_ports_removed = true;
Hi Sean
Reverse Christmas tree please.
> +static int
> +mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid)
> +{
> + u32 val;
> + int ret;
> + struct mt7530_dummy_poll p;
Here too.
> +static int
> +mt7530_port_vlan_prepare(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_vlan *vlan,
> + struct switchdev_trans *trans)
> +{
> + struct mt7530_priv *priv = ds->priv;
> +
> + /* The port is being kept as VLAN-unware port when bridge is set up
> + * with vlan_filtering not being set, Otherwise, the port and the
> + * corresponding CPU port is required the setup for becoming a
> + * VLAN-ware port.
> + */
> + if (!priv->ports[port].vlan_filtering)
> + return 0;
> +
> + mt7530_port_set_vlan_ware(ds, port);
> + mt7530_port_set_vlan_ware(ds, MT7530_CPU_PORT);
A prepare function should just validate that it is possible to carry
out the operation. It should not change any state. These two last
lines probably don't belong here.
> +
> + return 0;
> +}
> +
> +static void
> +mt7530_hw_vlan_add(struct mt7530_priv *priv,
> + struct mt7530_hw_vlan_entry *entry)
> +{
> + u32 val;
> + u8 new_members;
Reverse Christmas tree. Please check the whole patch.
> +static inline void INIT_MT7530_HW_ENTRY(struct mt7530_hw_vlan_entry *e,
> + int port, bool untagged)
> +{
> + e->port = port;
> + e->untagged = untagged;
> +}
All CAPITAL letters is for #defines. This is just a normal
function. Please use lower case.
Andrew
^ permalink raw reply
* RE: [crypto 6/8] chtls: TCB and Key program
From: Atul Gupta @ 2017-12-07 15:08 UTC (permalink / raw)
To: Stephan Mueller
Cc: herbert@gondor.apana.org.au, linux-crypto@vger.kernel.org,
netdev@vger.kernel.org, davem@davemloft.net, davejwatson@fb.com,
Ganesh GR, Harsh Jain
In-Reply-To: <2305475.IxNZ3AiDlf@tauon.chronox.de>
-----Original Message-----
From: linux-crypto-owner@vger.kernel.org [mailto:linux-crypto-owner@vger.kernel.org] On Behalf Of Stephan Mueller
Sent: Thursday, December 7, 2017 8:13 PM
To: Atul Gupta <atul.gupta@chelsio.com>
Cc: herbert@gondor.apana.org.au; linux-crypto@vger.kernel.org; netdev@vger.kernel.org; davem@davemloft.net; davejwatson@fb.com; Ganesh GR <ganeshgr@chelsio.com>; Harsh Jain <Harsh@chelsio.com>
Subject: Re: [crypto 6/8] chtls: TCB and Key program
Am Donnerstag, 7. Dezember 2017, 15:21:03 CET schrieb Atul Gupta:
Hi Atul,
>
> memzero_explicit(key)?
> [Atul] may not be required as entire info of size keylen and
> AEAD_H_SIZE is copied onto kctx->key. Key data is received from user,
> while ghash is memset and locally generated
Sure, but wouldn't it make sense to zap all instances where key material was stored?
Agree, Its safe to memset where keylen is variable, perhaps in future where we support different keylen. In current case key len is same as buffer size hence may not cause issue.
>
> As far as I see, the key is part of the skb (via kctx). This skb is
> released after being processed. The release calls kfree_skb which does
> not zeroize the key. Wouldn't it make sense to clear the memory of the
> key when the skb is released? [Atul] we should perhaps memset the info
> received from user so that driver has no info on key once its written on chip memory.
> memset(gcm_ctx->key, 0, keylen);
Are you saying that the skb (via kctx) above does not obtain a copy of the key? If not, what is done in chtls_key_info?
It does have a key copy, I was not sure how key info is accessed once skb is released.
Ciao
Stephan
Thanks
Atul
^ permalink raw reply
* Re: [PATCH net-next v4 1/4] phylib: Add device reset delay support
From: Richard Leitner @ 2017-12-07 15:01 UTC (permalink / raw)
To: Geert Uytterhoeven, Richard Leitner
Cc: Rob Herring, Mark Rutland, Fugang Duan, Andrew Lunn,
Florian Fainelli, Frank Rowand, David S. Miller,
Geert Uytterhoeven, Sergei Shtylyov, Baruch Siach, David Wu,
lukma-ynQEQJNshbs, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMuHMdVgWif=1K-S2j1Y39B9yDNvV12sUm7nrFLOP-=ndSYtVA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Geert,
On 12/07/2017 03:52 PM, Geert Uytterhoeven wrote:
> Hi Richard,
>
> On Thu, Dec 7, 2017 at 3:43 PM, Richard Leitner <dev-M/VWbR8SM2SsTnJN9+BGXg@public.gmane.org> wrote:
>> --- a/drivers/net/phy/mdio_device.c
>> +++ b/drivers/net/phy/mdio_device.c
>> @@ -24,6 +24,7 @@
>> #include <linux/slab.h>
>> #include <linux/string.h>
>> #include <linux/unistd.h>
>> +#include <linux/delay.h>
>>
>> void mdio_device_free(struct mdio_device *mdiodev)
>> {
>> @@ -118,8 +119,16 @@ EXPORT_SYMBOL(mdio_device_remove);
>>
>> void mdio_device_reset(struct mdio_device *mdiodev, int value)
>> {
>> - if (mdiodev->reset)
>> - gpiod_set_value(mdiodev->reset, value);
>> + unsigned int d;
>> +
>> + if (!mdiodev->reset)
>> + return;
>> +
>> + gpiod_set_value(mdiodev->reset, value);
>> +
>> + d = value ? mdiodev->reset_delay : mdiodev->reset_post_delay;
>> + if (d)
>> + usleep_range(d, d + min_t(unsigned int, d / 10, 100));
>
> Oops, I meant "max_t", not "min_t", else the upper limit can be "d + 0",
> which is not what we want.
You're right...
> Sorry, my fault.
I just copied it over from you suggestion without thinking about it...
So it's definitely my fault too ;-)
I'll wait for some more comments and send a new version next week.
regards;Richard.L
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [Patch net-next 1/2] netlink: make netlink tap per netns
From: Daniel Borkmann @ 2017-12-07 14:59 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: Kevin Cernekee
In-Reply-To: <20171206230320.22191-2-xiyou.wangcong@gmail.com>
On 12/07/2017 12:03 AM, Cong Wang wrote:
> nlmon device is not supposed to capture netlink events from
> other netns, so instead of filtering events, we can simply
> make netlink tap itself per netns.
>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Kevin Cernekee <cernekee@chromium.org>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Yeah, that should optimize it a bit further, looks fine to me.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next v4 1/4] phylib: Add device reset delay support
From: Geert Uytterhoeven @ 2017-12-07 14:52 UTC (permalink / raw)
To: Richard Leitner
Cc: Rob Herring, Mark Rutland, Fugang Duan, Andrew Lunn,
Florian Fainelli, Frank Rowand, David S. Miller,
Geert Uytterhoeven, Sergei Shtylyov, Baruch Siach, David Wu,
lukma, netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Richard Leitner
In-Reply-To: <20171207144358.3351-2-dev@g0hl1n.net>
Hi Richard,
On Thu, Dec 7, 2017 at 3:43 PM, Richard Leitner <dev@g0hl1n.net> wrote:
> --- a/drivers/net/phy/mdio_device.c
> +++ b/drivers/net/phy/mdio_device.c
> @@ -24,6 +24,7 @@
> #include <linux/slab.h>
> #include <linux/string.h>
> #include <linux/unistd.h>
> +#include <linux/delay.h>
>
> void mdio_device_free(struct mdio_device *mdiodev)
> {
> @@ -118,8 +119,16 @@ EXPORT_SYMBOL(mdio_device_remove);
>
> void mdio_device_reset(struct mdio_device *mdiodev, int value)
> {
> - if (mdiodev->reset)
> - gpiod_set_value(mdiodev->reset, value);
> + unsigned int d;
> +
> + if (!mdiodev->reset)
> + return;
> +
> + gpiod_set_value(mdiodev->reset, value);
> +
> + d = value ? mdiodev->reset_delay : mdiodev->reset_post_delay;
> + if (d)
> + usleep_range(d, d + min_t(unsigned int, d / 10, 100));
Oops, I meant "max_t", not "min_t", else the upper limit can be "d + 0",
which is not what we want.
Sorry, my fault.
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* RE: [crypto 4/8] chtls: CPL handler definition
From: Atul Gupta @ 2017-12-07 14:50 UTC (permalink / raw)
To: Stefano Brivio
Cc: herbert@gondor.apana.org.au, linux-crypto@vger.kernel.org,
netdev@vger.kernel.org, davem@davemloft.net, davejwatson@fb.com,
Ganesh GR, Harsh Jain
In-Reply-To: <20171205161945.768db3b5@elisabeth>
-----Original Message-----
From: linux-crypto-owner@vger.kernel.org [mailto:linux-crypto-owner@vger.kernel.org] On Behalf Of Stefano Brivio
Sent: Tuesday, December 5, 2017 8:54 PM
To: Atul Gupta <atul.gupta@chelsio.com>
Cc: herbert@gondor.apana.org.au; linux-crypto@vger.kernel.org; netdev@vger.kernel.org; davem@davemloft.net; davejwatson@fb.com; Ganesh GR <ganeshgr@chelsio.com>; Harsh Jain <Harsh@chelsio.com>
Subject: Re: [crypto 4/8] chtls: CPL handler definition
On Tue, 5 Dec 2017 17:10:00 +0530
Atul Gupta <atul.gupta@chelsio.com> wrote:
> CPL handlers for TLS session, record transmit and receive
>
> Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
> ---
> drivers/crypto/chelsio/chtls/chtls_cm.c | 2048 +++++++++++++++++++++++++++++++
> 1 file changed, 2048 insertions(+)
> create mode 100644 drivers/crypto/chelsio/chtls/chtls_cm.c
>
> diff --git a/drivers/crypto/chelsio/chtls/chtls_cm.c b/drivers/crypto/chelsio/chtls/chtls_cm.c
> new file mode 100644
> index 0000000..ea1c301
> --- /dev/null
> +++ b/drivers/crypto/chelsio/chtls/chtls_cm.c
> @@ -0,0 +1,2048 @@
> +/*
> + * Copyright (c) 2017 Chelsio Communications, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Written by: Atul Gupta (atul.gupta@chelsio.com)
> + */
> +
> +#include <linux/module.h>
> +#include <linux/list.h>
> +#include <linux/workqueue.h>
> +#include <linux/skbuff.h>
> +#include <linux/timer.h>
> +#include <linux/notifier.h>
> +#include <linux/inetdevice.h>
> +#include <linux/ip.h>
> +#include <linux/tcp.h>
> +#include <linux/sched/signal.h>
> +#include <linux/kallsyms.h>
> +#include <linux/kprobes.h>
> +#include <linux/if_vlan.h>
> +#include <net/tcp.h>
> +#include <net/dst.h>
> +
> +#include "chtls.h"
> +#include "chtls_cm.h"
> +
> +extern struct request_sock_ops chtls_rsk_ops;
> +static void (*tcp_time_wait_p)(struct sock *sk, int state, int timeo);
> +
> +/*
> + * State transitions and actions for close. Note that if we are in SYN_SENT
> + * we remain in that state as we cannot control a connection while it's in
> + * SYN_SENT; such connections are allowed to establish and are then aborted.
> + */
> +static unsigned char new_state[16] = {
> + /* current state: new state: action: */
> + /* (Invalid) */ TCP_CLOSE,
> + /* TCP_ESTABLISHED */ TCP_FIN_WAIT1 | TCP_ACTION_FIN,
> + /* TCP_SYN_SENT */ TCP_SYN_SENT,
> + /* TCP_SYN_RECV */ TCP_FIN_WAIT1 | TCP_ACTION_FIN,
> + /* TCP_FIN_WAIT1 */ TCP_FIN_WAIT1,
> + /* TCP_FIN_WAIT2 */ TCP_FIN_WAIT2,
> + /* TCP_TIME_WAIT */ TCP_CLOSE,
> + /* TCP_CLOSE */ TCP_CLOSE,
> + /* TCP_CLOSE_WAIT */ TCP_LAST_ACK | TCP_ACTION_FIN,
> + /* TCP_LAST_ACK */ TCP_LAST_ACK,
> + /* TCP_LISTEN */ TCP_CLOSE,
> + /* TCP_CLOSING */ TCP_CLOSING,
> +};
> +
> +static struct chtls_sock *chtls_sock_create(struct chtls_dev *cdev)
> +{
> + struct chtls_sock *csk = kzalloc(sizeof(*csk), GFP_NOIO);
> +
> + if (!csk)
> + return NULL;
> +
> + csk->txdata_skb_cache = alloc_skb(TXDATA_SKB_LEN, GFP_ATOMIC);
Excess whitespace.
[Atul] Will take care in v2
> + if (!csk->txdata_skb_cache) {
> + kfree(csk);
> + return NULL;
> + }
> +
> + kref_init(&csk->kref);
> + csk->cdev = cdev;
> + skb_queue_head_init(&csk->txq);
> + csk->wr_skb_head = NULL;
> + csk->wr_skb_tail = NULL;
> + csk->mss = MAX_MSS;
> + csk->tlshws.ofld = 1;
> + csk->tlshws.txkey = -1;
> + csk->tlshws.rxkey = -1;
> + csk->tlshws.mfs = TLS_MFS;
> + skb_queue_head_init(&csk->tlshws.sk_recv_queue);
> + return csk;
> +}
> +
> +void chtls_sock_release(struct kref *ref)
> +{
> + struct chtls_sock *csk =
> + container_of(ref, struct chtls_sock, kref);
> +
> + kfree(csk);
> +}
> +
> +void get_tcp_symbol(void)
> +{
> + tcp_time_wait_p = (void *)kallsyms_lookup_name("tcp_time_wait");
> + if (!tcp_time_wait_p)
> + pr_info("could not locate tcp_time_wait");
Probably not something that should be used here. Why do you need this?
[Atul] using it to call tcp_time_wait, as used in tcp_rcv_state_process
> +}
> +
> +static struct net_device *chtls_ipv4_netdev(struct chtls_dev *cdev,
> + struct sock *sk)
> +{
> + struct net_device *ndev = cdev->ports[0];
> +
> + if (likely(!inet_sk(sk)->inet_rcv_saddr))
> + return ndev;
> +
> + ndev = ip_dev_find(&init_net, inet_sk(sk)->inet_rcv_saddr);
> + if (!ndev)
> + return NULL;
> +
> + if (is_vlan_dev(ndev))
> + return vlan_dev_real_dev(ndev);
> + return ndev;
> +}
> +
> +static void assign_rxopt(struct sock *sk, unsigned int opt)
> +{
> + const struct chtls_dev *cdev;
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + cdev = csk->cdev;
> + tp->tcp_header_len = sizeof(struct tcphdr);
> + tp->rx_opt.mss_clamp = cdev->mtus[TCPOPT_MSS_G(opt)] - 40;
> + tp->mss_cache = tp->rx_opt.mss_clamp;
> + tp->rx_opt.tstamp_ok = TCPOPT_TSTAMP_G(opt);
> + tp->rx_opt.snd_wscale = TCPOPT_SACK_G(opt);
> + tp->rx_opt.wscale_ok = TCPOPT_WSCALE_OK_G(opt);
> + SND_WSCALE(tp) = TCPOPT_SND_WSCALE_G(opt);
> + if (!tp->rx_opt.wscale_ok)
> + tp->rx_opt.rcv_wscale = 0;
> + if (tp->rx_opt.tstamp_ok) {
> + tp->tcp_header_len += TCPOLEN_TSTAMP_ALIGNED;
> + tp->rx_opt.mss_clamp -= TCPOLEN_TSTAMP_ALIGNED;
> + } else if (csk->opt2 & TSTAMPS_EN_F) {
> + csk->opt2 &= ~TSTAMPS_EN_F;
> + csk->mtu_idx = TCPOPT_MSS_G(opt);
> + }
> +}
> +
> +static void chtls_purge_rcv_queue(struct sock *sk)
> +{
> + struct sk_buff *skb;
> +
> + while ((skb = __skb_dequeue(&sk->sk_receive_queue)) != NULL) {
> + skb_dst_set(skb, (void *)NULL);
> + kfree_skb(skb);
> + }
> +}
> +
> +static void chtls_purge_write_queue(struct sock *sk)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct sk_buff *skb;
> +
> + while ((skb = __skb_dequeue(&csk->txq))) {
> + sk->sk_wmem_queued -= skb->truesize;
> + __kfree_skb(skb);
> + }
> +}
> +
> +static void chtls_purge_receive_queue(struct sock *sk)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct chtls_hws *tlsk = &csk->tlshws;
> + struct sk_buff *skb;
> +
> + while ((skb = __skb_dequeue(&tlsk->sk_recv_queue)) != NULL) {
> + skb_dst_set(skb, NULL);
> + kfree_skb(skb);
> + }
> +}
> +
> +static void abort_arp_failure(void *handle, struct sk_buff *skb)
> +{
> + struct cpl_abort_req *req = cplhdr(skb);
> + struct chtls_dev *cdev = (struct chtls_dev *)handle;
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + req->cmd = CPL_ABORT_NO_RST;
> + cxgb4_ofld_send(cdev->lldi->ports[0], skb);
> +}
> +
> +struct sk_buff *alloc_ctrl_skb(struct sk_buff *skb, int len)
> +{
> + if (likely(skb && !skb_shared(skb) && !skb_cloned(skb))) {
> + __skb_trim(skb, 0);
> + refcount_add(2, &skb->users);
> + } else {
> + skb = alloc_skb(len, GFP_KERNEL | __GFP_NOFAIL);
> + }
> + return skb;
> +}
> +
> +static void chtls_send_abort(struct sock *sk, int mode, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_abort_req *req;
> +
> + if (!skb)
> + skb = alloc_ctrl_skb(csk->txdata_skb_cache, sizeof(*req));
What if alloc_ctrl_skb() fails?
[Atul] alloc_ctrl_skb does alloc_skb with GFP_NOFAIL
> + req = (struct cpl_abort_req *)skb_put(skb, sizeof(*req));
> + INIT_TP_WR_CPL(req, CPL_ABORT_REQ, csk->tid);
> + set_queue(skb, (csk->txq_idx << 1) | CPL_PRIORITY_DATA, sk);
> + req->rsvd0 = htonl(tp->snd_nxt);
> + req->rsvd1 = !csk_flag_nochk(csk, CSK_TX_DATA_SENT);
> + req->cmd = mode;
> + t4_set_arp_err_handler(skb, csk->cdev, abort_arp_failure);
> + send_or_defer(sk, tp, skb, mode == CPL_ABORT_SEND_RST);
> +}
> +
> +int chtls_send_reset(struct sock *sk, int mode, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> +
> + if (unlikely(csk_flag_nochk(csk, CSK_ABORT_SHUTDOWN) ||
> + !csk->cdev)) {
> + if (sk->sk_state == TCP_SYN_RECV)
> + csk_set_flag(csk, CSK_RST_ABORTED);
> + goto out;
> + }
> +
> + if (!csk_flag_nochk(csk, CSK_TX_DATA_SENT)) {
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + if (send_tx_flowc_wr(sk, 0, tp->snd_nxt, tp->rcv_nxt) < 0)
> + WARN_ONCE(1, "send tx flowc error");
> + csk_set_flag(csk, CSK_TX_DATA_SENT);
> + }
> +
> + csk_set_flag(csk, CSK_ABORT_RPL_PENDING);
> + chtls_purge_write_queue(sk);
> +
> + csk_set_flag(csk, CSK_ABORT_SHUTDOWN);
> + if (sk->sk_state != TCP_SYN_RECV)
> + chtls_send_abort(sk, mode, skb);
If sk->sk_state == TCP_SYN_RECV, aren't we leaking skb, coming e.g.
from reset_listen_child()?
[Atul] If (sk->sk_state == TCP_SYN_RECV) we free the skb, else we call the send abort where skb is freed on completion.
> +
> + return 0;
> +out:
> + if (skb)
> + kfree_skb(skb);
> + return 1;
> +}
> +
> +static void release_tcp_port(struct sock *sk)
> +{
> + if (inet_csk(sk)->icsk_bind_hash)
> + inet_put_port(sk);
> +}
> +
> +static void tcp_uncork(struct sock *sk)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + if (tp->nonagle & TCP_NAGLE_CORK) {
> + tp->nonagle &= ~TCP_NAGLE_CORK;
> + chtls_tcp_push(sk, 0);
> + }
> +}
> +
> +static void chtls_close_conn(struct sock *sk)
> +{
> + struct sk_buff *skb;
> + struct cpl_close_con_req *req;
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + unsigned int tid = csk->tid;
> + unsigned int len = roundup(sizeof(struct cpl_close_con_req), 16);
> +
> + skb = alloc_skb(len, GFP_KERNEL | __GFP_NOFAIL);
> + req = (struct cpl_close_con_req *)__skb_put(skb, len);
> + memset(req, 0, len);
> + req->wr.wr_hi = htonl(FW_WR_OP_V(FW_TP_WR) |
> + FW_WR_IMMDLEN_V(sizeof(*req) -
> + sizeof(req->wr)));
> + req->wr.wr_mid = htonl(FW_WR_LEN16_V(DIV_ROUND_UP(sizeof(*req), 16)) |
> + FW_WR_FLOWID_V(tid));
> +
> + OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_CLOSE_CON_REQ, tid));
> +
> + tcp_uncork(sk);
> + skb_entail(sk, skb, ULPCB_FLAG_NO_HDR | ULPCB_FLAG_NO_APPEND);
> + if (sk->sk_state != TCP_SYN_SENT)
> + chtls_push_frames(csk, 1);
> +}
> +
> +/*
> + * Perform a state transition during close and return the actions indicated
> + * for the transition. Do not make this function inline, the main reason
> + * it exists at all is to avoid multiple inlining of tcp_set_state.
> + */
> +static int make_close_transition(struct sock *sk)
> +{
> + int next = (int)new_state[sk->sk_state];
> +
> + tcp_set_state(sk, next & TCP_STATE_MASK);
> + return next & TCP_ACTION_FIN;
> +}
> +
> +void chtls_close(struct sock *sk, long timeout)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + int data_lost, prev_state;
> +
> + lock_sock(sk);
> + if (sk->sk_prot->close != chtls_close) {
> + release_sock(sk);
> + return sk->sk_prot->close(sk, timeout);
> + }
> +
> + sk->sk_shutdown |= SHUTDOWN_MASK;
> +
> + data_lost = skb_queue_len(&sk->sk_receive_queue);
> + data_lost |= skb_queue_len(&csk->tlshws.sk_recv_queue);
> + chtls_purge_receive_queue(sk);
> + chtls_purge_rcv_queue(sk);
> +
> + if (sk->sk_state == TCP_CLOSE) {
> + goto wait;
> + } else if (data_lost || sk->sk_state == TCP_SYN_SENT) {
> + chtls_send_reset(sk, CPL_ABORT_SEND_RST, NULL);
> + release_tcp_port(sk);
> + goto unlock;
> + } else if (sock_flag(sk, SOCK_LINGER) && !sk->sk_lingertime) {
> + sk->sk_prot->disconnect(sk, 0);
> + } else if (make_close_transition(sk)) {
> + chtls_close_conn(sk);
> + }
> +wait:
> + if (timeout)
> + sk_stream_wait_close(sk, timeout);
> +
> +unlock:
> + prev_state = sk->sk_state;
> + sock_hold(sk);
> + sock_orphan(sk);
> +
> + release_sock(sk);
> +
> + local_bh_disable();
> + bh_lock_sock(sk);
> +
> + if (prev_state != TCP_CLOSE && sk->sk_state == TCP_CLOSE)
> + goto out;
> +
> + if (sk->sk_state == TCP_FIN_WAIT2 && tcp_sk(sk)->linger2 < 0 &&
> + !csk_flag(sk, CSK_ABORT_SHUTDOWN)) {
> + struct sk_buff *skb;
> +
> + skb = alloc_skb(sizeof(struct cpl_abort_req), GFP_ATOMIC);
> + if (skb)
> + chtls_send_reset(sk, CPL_ABORT_SEND_RST, skb);
> + }
> +
> + if (sk->sk_state == TCP_CLOSE)
> + inet_csk_destroy_sock(sk);
> +
> +out:
> + bh_unlock_sock(sk);
> + local_bh_enable();
> + sock_put(sk);
> +}
> +
> +/*
> + * Wait until a socket enters on of the given states.
> + */
> +static int wait_for_states(struct sock *sk, unsigned int states)
> +{
> + struct socket_wq _sk_wq;
> + long current_timeo = 200;
> + DECLARE_WAITQUEUE(wait, current);
> + int err = 0;
> +
> + /*
> + * We want this to work even when there's no associated struct socket.
> + * In that case we provide a temporary wait_queue_head_t.
> + */
> + if (!sk->sk_wq) {
> + init_waitqueue_head(&_sk_wq.wait);
> + _sk_wq.fasync_list = NULL;
> + init_rcu_head_on_stack(&_sk_wq.rcu);
> + sk->sk_wq = &_sk_wq;
> + }
> +
> + add_wait_queue(sk_sleep(sk), &wait);
> + while (!sk_in_state(sk, states)) {
> + if (!current_timeo) {
> + err = -EBUSY;
> + break;
> + }
> + if (signal_pending(current)) {
> + err = sock_intr_errno(current_timeo);
> + break;
> + }
> + set_current_state(TASK_UNINTERRUPTIBLE);
> + release_sock(sk);
> + if (!sk_in_state(sk, states))
> + current_timeo = schedule_timeout(current_timeo);
> + __set_current_state(TASK_RUNNING);
> + lock_sock(sk);
> + }
> + remove_wait_queue(sk_sleep(sk), &wait);
> +
> + if (sk->sk_wq == &_sk_wq)
> + sk->sk_wq = NULL;
> + return err;
> +}
> +
> +int chtls_disconnect(struct sock *sk, int flags)
> +{
> + struct chtls_sock *csk;
> + struct tcp_sock *tp = tcp_sk(sk);
> + int err;
> +
> + if (sk->sk_prot->disconnect != chtls_disconnect)
> + return sk->sk_prot->disconnect(sk, flags);
> +
> + csk = rcu_dereference_sk_user_data(sk);
> + chtls_purge_receive_queue(sk);
> + chtls_purge_rcv_queue(sk);
> + chtls_purge_write_queue(sk);
> +
> + if (sk->sk_state != TCP_CLOSE) {
> + sk->sk_err = ECONNRESET;
> + chtls_send_reset(sk, CPL_ABORT_SEND_RST, NULL);
> + err = wait_for_states(sk, TCPF_CLOSE);
> + if (err)
> + return err;
> + }
> + if (sk->sk_prot->disconnect != chtls_disconnect)
> + return sk->sk_prot->disconnect(sk, flags);
> +
> + chtls_purge_receive_queue(sk);
> + chtls_purge_rcv_queue(sk);
> + tp->max_window = 0xFFFF << (tp->rx_opt.snd_wscale);
> + return tcp_disconnect(sk, flags);
> +}
> +
> +#define SHUTDOWN_ELIGIBLE_STATE (TCPF_ESTABLISHED | \
> + TCPF_SYN_RECV | TCPF_CLOSE_WAIT)
> +void chtls_shutdown(struct sock *sk, int how)
> +{
> + if (sk->sk_prot->shutdown != chtls_shutdown)
> + return sk->sk_prot->shutdown(sk, how);
> +
> + if ((how & SEND_SHUTDOWN) &&
> + sk_in_state(sk, SHUTDOWN_ELIGIBLE_STATE) &&
> + make_close_transition(sk))
> + chtls_close_conn(sk);
> +}
> +
> +void chtls_destroy_sock(struct sock *sk)
> +{
> + struct chtls_sock *csk;
> +
> + if (sk->sk_prot->destroy != chtls_destroy_sock)
> + return sk->sk_prot->destroy(sk);
> +
> + csk = rcu_dereference_sk_user_data(sk);
> + chtls_purge_receive_queue(sk);
> + csk->ulp_mode = ULP_MODE_NONE;
> + chtls_purge_write_queue(sk);
> + free_tls_keyid(sk);
> + kref_put(&csk->kref, chtls_sock_release);
> +
> + sk->sk_prot = &tcp_prot;
> + sk->sk_prot->destroy(sk);
> +}
> +
> +static void reset_listen_child(struct sock *child)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(child);
> + struct sk_buff *skb;
> +
> + skb = alloc_ctrl_skb(csk->txdata_skb_cache,
> + sizeof(struct cpl_abort_req));
> +
> + chtls_send_reset(child, CPL_ABORT_SEND_RST, skb);
> + sock_orphan(child);
> + INC_ORPHAN_COUNT(child);
> + if (child->sk_state == TCP_CLOSE)
> + inet_csk_destroy_sock(child);
> +}
> +
> +void chtls_disconnect_acceptq(struct sock *listen_sk)
> +{
> + struct request_sock **pprev;
> +
> + pprev = ACCEPT_QUEUE(listen_sk);
> + while (*pprev) {
> + struct request_sock *req = *pprev;
> +
> + if (req->rsk_ops == &chtls_rsk_ops) {
> + struct sock *child = req->sk;
> +
> + *pprev = req->dl_next;
> + sk_acceptq_removed(listen_sk);
> + reqsk_put(req);
> + sock_hold(child);
> + local_bh_disable();
> + bh_lock_sock(child);
> + release_tcp_port(child);
> + reset_listen_child(child);
> + bh_unlock_sock(child);
> + local_bh_enable();
> + sock_put(child);
> + } else {
> + pprev = &req->dl_next;
> + }
> + }
> +}
> +
> +static int listen_hashfn(const struct sock *sk)
> +{
> + return ((unsigned long)sk >> 10) & (LISTEN_INFO_HASH_SIZE - 1);
> +}
> +
> +static struct listen_info *listen_hash_add(struct chtls_dev *cdev,
> + struct sock *sk,
> + unsigned int stid)
> +{
> + struct listen_info *p = kmalloc(sizeof(*p), GFP_KERNEL);
> +
> + if (p) {
> + int key = listen_hashfn(sk);
> +
> + p->sk = sk;
> + p->stid = stid;
> + spin_lock(&cdev->listen_lock);
> + p->next = cdev->listen_hash_tab[key];
> + cdev->listen_hash_tab[key] = p;
> + spin_unlock(&cdev->listen_lock);
> + }
> + return p;
> +}
> +
> +static int listen_hash_find(struct chtls_dev *cdev,
> + struct sock *sk)
> +{
> + struct listen_info *p;
> + int key = listen_hashfn(sk);
> + int stid = -1;
Reverse christmas tree format?
[Atul] Will take care in v2
> +
> + spin_lock(&cdev->listen_lock);
> + for (p = cdev->listen_hash_tab[key]; p; p = p->next)
> + if (p->sk == sk) {
> + stid = p->stid;
> + break;
> + }
> + spin_unlock(&cdev->listen_lock);
> + return stid;
> +}
> +
> +static int listen_hash_del(struct chtls_dev *cdev,
> + struct sock *sk)
> +{
> + int key = listen_hashfn(sk);
> + struct listen_info *p, **prev = &cdev->listen_hash_tab[key];
> + int stid = -1;
> +
> + spin_lock(&cdev->listen_lock);
> + for (p = *prev; p; prev = &p->next, p = p->next)
> + if (p->sk == sk) {
> + stid = p->stid;
> + *prev = p->next;
> + kfree(p);
> + break;
> + }
> + spin_unlock(&cdev->listen_lock);
> + return stid;
> +}
> +
> +int chtls_listen_start(struct chtls_dev *cdev, struct sock *sk)
> +{
> + struct net_device *ndev;
> + struct port_info *pi;
> + struct adapter *adap;
> + struct listen_ctx *ctx;
> + int stid;
> + int ret;
Reverse christmas tree format?
[Atul] Will take care in v2
> +
> + if (sk->sk_family != PF_INET)
> + return -EAGAIN;
> +
> + rcu_read_lock();
> + ndev = chtls_ipv4_netdev(cdev, sk);
> + rcu_read_unlock();
> + if (!ndev)
> + return -EBADF;
> +
> + pi = netdev_priv(ndev);
> + adap = pi->adapter;
> + if (!(adap->flags & FULL_INIT_DONE))
> + return -EBADF;
> +
> + if (listen_hash_find(cdev, sk) >= 0) /* already have it */
> + return -EADDRINUSE;
> +
> + ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + __module_get(THIS_MODULE);
> + ctx->lsk = sk;
> + ctx->cdev = cdev;
> + ctx->state = T4_LISTEN_START_PENDING;
> +
> + if (cdev->lldi->enable_fw_ofld_conn &&
> + sk->sk_family == PF_INET)
> + stid = cxgb4_alloc_sftid(cdev->tids, sk->sk_family, ctx);
> + else
> + stid = cxgb4_alloc_stid(cdev->tids, sk->sk_family, ctx);
> +
> + if (stid < 0)
> + goto free_ctx;
> +
> + sock_hold(sk);
> + if (!listen_hash_add(cdev, sk, stid))
> + goto free_stid;
> +
> + if (cdev->lldi->enable_fw_ofld_conn) {
> + ret = cxgb4_create_server_filter(ndev, stid,
> + inet_sk(sk)->inet_rcv_saddr,
> + inet_sk(sk)->inet_sport, 0,
> + cdev->lldi->rxq_ids[0], 0, 0);
> + } else {
> + ret = cxgb4_create_server(ndev, stid,
> + inet_sk(sk)->inet_rcv_saddr,
> + inet_sk(sk)->inet_sport, 0,
> + cdev->lldi->rxq_ids[0]);
> + }
> + if (ret > 0)
> + ret = net_xmit_errno(ret);
> + if (ret)
> + goto del_hash;
> +
> + if (!ret)
Not needed I guess?
[Atul] its required, cxgb4_create_server calls net_xmit_eval where ret can be NET_XMIT_SUCCESS/DROP/CN.
net_xmit_eval can return 0 or 1.
If 1, net_xmit_errno is called which returns ENOBUF or 0. If ENOBUF goto del_hash else return 0
> + return 0;
> +del_hash:
> + listen_hash_del(cdev, sk);
> +free_stid:
> + cxgb4_free_stid(cdev->tids, stid, sk->sk_family);
> + sock_put(sk);
> +free_ctx:
> + kfree(ctx);
> + module_put(THIS_MODULE);
> + return -EBADF;
> +}
> +
> +void chtls_listen_stop(struct chtls_dev *cdev, struct sock *sk)
> +{
> + int stid;
> +
> + stid = listen_hash_del(cdev, sk);
> + if (stid < 0)
> + return;
> +
> + if (cdev->lldi->enable_fw_ofld_conn) {
> + cxgb4_remove_server_filter(cdev->lldi->ports[0], stid,
> + cdev->lldi->rxq_ids[0], 0);
> + } else {
> + cxgb4_remove_server(cdev->lldi->ports[0], stid,
> + cdev->lldi->rxq_ids[0], 0);
> + }
> + chtls_disconnect_acceptq(sk);
> +}
> +
> +static int chtls_pass_open_rpl(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct cpl_pass_open_rpl *rpl = cplhdr(skb) + RSS_HDR;
> + unsigned int stid = GET_TID(rpl);
> + struct listen_ctx *listen_ctx;
> +
> + listen_ctx = (struct listen_ctx *)lookup_stid(cdev->tids, stid);
> + if (!listen_ctx)
> + return 1;
> +
> + if (listen_ctx->state == T4_LISTEN_START_PENDING) {
> + listen_ctx->state = T4_LISTEN_STARTED;
> + return 1;
> + }
> +
> + if (rpl->status != CPL_ERR_NONE) {
> + pr_info("Unexpected PASS_OPEN_RPL status %u for STID %u\n",
> + rpl->status, stid);
Why do we return 0? How is this error handled?
[Atul] Will take care in v2
> + } else {
> + cxgb4_free_stid(cdev->tids, stid, listen_ctx->lsk->sk_family);
> + sock_put(listen_ctx->lsk);
> + kfree(listen_ctx);
> + module_put(THIS_MODULE);
> + }
> +
> + return 0;
> +}
> +
> +static int chtls_close_listsrv_rpl(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct cpl_close_listsvr_rpl *rpl = cplhdr(skb) + RSS_HDR;
> + unsigned int stid = GET_TID(rpl);
> + void *data = lookup_stid(cdev->tids, stid);
> +
> + if (rpl->status != CPL_ERR_NONE) {
> + pr_info("Unexpected CLOSE_LISTSRV_RPL status %u for STID %u\n",
> + rpl->status, stid);
> + } else {
> + struct listen_ctx *listen_ctx = (struct listen_ctx *)data;
> +
> + cxgb4_free_stid(cdev->tids, stid, listen_ctx->lsk->sk_family);
> + sock_put(listen_ctx->lsk);
> + kfree(listen_ctx);
> + module_put(THIS_MODULE);
> + }
> +
> + return 0;
> +}
> +
> +static void conn_remove_handle(struct chtls_dev *cdev,
> + int tid)
> +{
> + spin_lock_bh(&cdev->aidr_lock);
> + idr_remove(&cdev->aidr, tid);
> + spin_unlock_bh(&cdev->aidr_lock);
> +}
> +
> +void free_atid(struct chtls_sock *csk, struct chtls_dev *cdev,
> + unsigned int atid)
> +{
> + struct tid_info *tids = cdev->tids;
> +
> + conn_remove_handle(cdev, atid);
> + cxgb4_free_atid(tids, atid);
> + sock_put(csk->sk);
> + kref_put(&csk->kref, chtls_sock_release);
> +}
> +
> +static void chtls_release_resources(struct sock *sk)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct chtls_dev *cdev = csk->cdev;
> + struct tid_info *tids;
> + unsigned int tid = csk->tid;
> +
> + if (!cdev)
> + return;
> +
> + tids = cdev->tids;
> + kfree_skb(csk->txdata_skb_cache);
> + csk->txdata_skb_cache = NULL;
> +
> + if (csk->l2t_entry) {
> + cxgb4_l2t_release(csk->l2t_entry);
> + csk->l2t_entry = NULL;
> + }
> +
> + if (sk->sk_state == TCP_SYN_SENT) {
> + free_atid(csk, cdev, tid);
> + __skb_queue_purge(&csk->ooo_queue);
> + } else {
> + cxgb4_remove_tid(tids, csk->port_id, tid, sk->sk_family);
> + sock_put(sk);
> + }
> +}
> +
> +static void cleanup_syn_rcv_conn(struct sock *child, struct sock *parent)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(child);
> + struct request_sock *req = csk->passive_reap_next;
> +
> + reqsk_queue_removed(&inet_csk(parent)->icsk_accept_queue, req);
> + chtls_reqsk_free(req);
> + csk->passive_reap_next = NULL;
> +}
> +
> +static void chtls_conn_done(struct sock *sk)
> +{
> + if (sock_flag(sk, SOCK_DEAD))
> + chtls_purge_rcv_queue(sk);
> + sk_wakeup_sleepers(sk, 0);
> + tcp_done(sk);
> +}
> +
> +static void do_abort_syn_rcv(struct sock *child, struct sock *parent)
> +{
> + /*
> + * If the server is still open we clean up the child connection,
> + * otherwise the server already did the clean up as it was purging
> + * its SYN queue and the skb was just sitting in its backlog.
> + */
> + if (likely(parent->sk_state == TCP_LISTEN)) {
> + cleanup_syn_rcv_conn(child, parent);
> + /* Without the below call to sock_orphan,
> + * we leak the socket resource with syn_flood test
> + * as inet_csk_destroy_sock will not be called
> + * in tcp_done since SOCK_DEAD flag is not set.
> + * Kernel handles this differently where new socket is
> + * created only after 3 way handshake is done.
> + */
> + sock_orphan(child);
> + percpu_counter_inc((child)->sk_prot->orphan_count);
> + chtls_release_resources(child);
> + chtls_conn_done(child);
> + } else {
> + if (csk_flag(child, CSK_RST_ABORTED)) {
> + chtls_release_resources(child);
> + chtls_conn_done(child);
> + }
> + }
> +}
> +
> +static void pass_open_abort(struct sock *child, struct sock *parent,
> + struct sk_buff *skb)
> +{
> + do_abort_syn_rcv(child, parent);
> + kfree_skb(skb);
> +}
> +
> +static void bl_pass_open_abort(struct sock *lsk, struct sk_buff *skb)
> +{
> + pass_open_abort(skb->sk, lsk, skb);
> +}
> +
> +static void chtls_pass_open_arp_failure(struct sock *sk,
> + struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct chtls_dev *cdev = csk->cdev;
> + struct sock *parent;
> + const struct request_sock *oreq;
> + void *data;
> +
> + /*
> + * If the connection is being aborted due to the parent listening
> + * socket going away there's nothing to do, the ABORT_REQ will close
> + * the connection.
> + */
> + if (csk_flag(sk, CSK_ABORT_RPL_PENDING)) {
> + kfree_skb(skb);
> + return;
> + }
> +
> + oreq = csk->passive_reap_next;
> + data = lookup_stid(cdev->tids, oreq->ts_recent);
> + parent = ((struct listen_ctx *)data)->lsk;
> +
> + bh_lock_sock(parent);
> + if (!sock_owned_by_user(parent)) {
> + pass_open_abort(sk, parent, skb);
> + } else {
> + BLOG_SKB_CB(skb)->backlog_rcv = bl_pass_open_abort;
> + __sk_add_backlog(parent, skb);
> + }
> + bh_unlock_sock(parent);
> +}
> +
> +static void chtls_accept_rpl_arp_failure(void *handle,
> + struct sk_buff *skb)
> +{
> + struct sock *sk = (struct sock *)handle;
> +
> + sock_hold(sk);
> + process_cpl_msg(chtls_pass_open_arp_failure, sk, skb);
> + sock_put(sk);
> +}
> +
> +static unsigned int chtls_select_mss(const struct chtls_sock *csk,
> + unsigned int pmtu,
> + struct cpl_pass_accept_req *req)
> +{
> + struct sock *sk = csk->sk;
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct dst_entry *dst = __sk_dst_get(sk);
> + struct chtls_dev *cdev = csk->cdev;
> + unsigned int iphdrsz;
> + unsigned int tcpoptsz = 0;
> + unsigned int mtu_idx;
> + unsigned int mss = ntohs(req->tcpopt.mss);
> +
> + iphdrsz = sizeof(struct iphdr) + sizeof(struct tcphdr);
> + if (req->tcpopt.tstamp)
> + tcpoptsz += round_up(TCPOLEN_TIMESTAMP, 4);
> +
> + tp->advmss = dst_metric_advmss(dst);
> + if (USER_MSS(tp) && tp->advmss > USER_MSS(tp))
> + tp->advmss = USER_MSS(tp);
> + if (tp->advmss > pmtu - iphdrsz)
> + tp->advmss = pmtu - iphdrsz;
> + if (mss && tp->advmss > mss)
> + tp->advmss = mss;
> +
> + tp->advmss = cxgb4_best_aligned_mtu(cdev->lldi->mtus,
> + iphdrsz + tcpoptsz,
> + tp->advmss - tcpoptsz,
> + 8, &mtu_idx);
> + tp->advmss -= iphdrsz;
> +
> + inet_csk(sk)->icsk_pmtu_cookie = pmtu;
> + return mtu_idx;
> +}
> +
> +static unsigned int select_rcv_wnd(struct chtls_sock *csk)
> +{
> + struct sock *sk = csk->sk;
> + unsigned int wnd = tcp_full_space(sk);
> + unsigned int rcvwnd;
Reverse christmas tree format?
[Atul] Will take care in v2
> + if (wnd < MIN_RCV_WND)
> + wnd = MIN_RCV_WND;
> +
> + rcvwnd = MAX_RCV_WND;
> +
> + csk_set_flag(csk, CSK_UPDATE_RCV_WND);
> + return min(wnd, rcvwnd);
> +}
> +
> +static void chtls_pass_accept_rpl(struct sk_buff *skb,
> + struct cpl_pass_accept_req *req,
> + unsigned int tid)
> +
> +{
> + struct sock *sk;
> + struct chtls_sock *csk;
> + const struct tcphdr *tcph;
> + struct cxgb4_lld_info *lldi;
> + struct cpl_t5_pass_accept_rpl *rpl5;
> + unsigned int len = roundup(sizeof(*rpl5), 16);
> + const struct tcp_sock *tp;
> + u64 opt0;
> + u32 opt2, hlen;
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + sk = skb->sk;
> + tp = tcp_sk(sk);
> + csk = sk->sk_user_data;
> + csk->tid = tid;
> + lldi = csk->cdev->lldi;
> +
> + rpl5 = __skb_put_zero(skb, len);
> + INIT_TP_WR(rpl5, tid);
> +
> + OPCODE_TID(rpl5) = cpu_to_be32(MK_OPCODE_TID(CPL_PASS_ACCEPT_RPL,
> + csk->tid));
> + csk->mtu_idx = chtls_select_mss(csk, dst_mtu(__sk_dst_get(sk)),
> + req);
> + opt0 = TCAM_BYPASS_F |
Excess whitespace.
[Atul] Will take care in v2
> + WND_SCALE_V((tp)->rx_opt.rcv_wscale) |
> + MSS_IDX_V(csk->mtu_idx) |
> + L2T_IDX_V(csk->l2t_entry->idx) |
> + NAGLE_V(!(tp->nonagle & TCP_NAGLE_OFF)) |
> + TX_CHAN_V(csk->tx_chan) |
> + SMAC_SEL_V(csk->smac_idx) |
> + DSCP_V(csk->tos >> 2) |
> + ULP_MODE_V(ULP_MODE_TLS) |
> + RCV_BUFSIZ_V(min(tp->rcv_wnd >> 10, RCV_BUFSIZ_M));
> +
> + opt2 = RX_CHANNEL_V(0) |
> + RSS_QUEUE_VALID_F | RSS_QUEUE_V(csk->rss_qid);
> +
> + if (!is_t5(lldi->adapter_type))
> + opt2 |= RX_FC_DISABLE_F;
> + if (req->tcpopt.tstamp)
> + opt2 |= TSTAMPS_EN_F;
> + if (req->tcpopt.sack)
> + opt2 |= SACK_EN_F;
> + hlen = ntohl(req->hdr_len);
> +
> + tcph = (struct tcphdr *)((u8 *)(req + 1) +
> + T6_ETH_HDR_LEN_G(hlen) + T6_IP_HDR_LEN_G(hlen));
> + if (tcph->ece && tcph->cwr)
> + opt2 |= CCTRL_ECN_V(1);
> + opt2 |= CONG_CNTRL_V(CONG_ALG_NEWRENO);
> + opt2 |= T5_ISS_F;
> + opt2 |= T5_OPT_2_VALID_F;
> + rpl5->opt0 = cpu_to_be64(opt0);
> + rpl5->opt2 = cpu_to_be32(opt2);
> + rpl5->iss = cpu_to_be32((prandom_u32() & ~7UL) - 1);
> + set_wr_txq(skb, CPL_PRIORITY_SETUP, csk->port_id);
> + t4_set_arp_err_handler(skb, sk, chtls_accept_rpl_arp_failure);
> + cxgb4_l2t_send(csk->egress_dev, skb, csk->l2t_entry);
> +}
> +
> +static void inet_inherit_port(struct inet_hashinfo *hash_info,
> + struct sock *lsk, struct sock *newsk)
> +{
> + local_bh_disable();
> + __inet_inherit_port(lsk, newsk);
> + local_bh_enable();
> +}
> +
> +static int chtls_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> +{
> + if (skb->protocol) {
> + kfree_skb(skb);
> + return 0;
> + }
> + BLOG_SKB_CB(skb)->backlog_rcv(sk, skb);
> + return 0;
> +}
> +
> +static struct sock *chtls_recv_sock(struct sock *lsk,
> + struct request_sock *oreq,
> + void *network_hdr,
> + const struct cpl_pass_accept_req *req,
> + struct chtls_dev *cdev)
> +
> +{
> + struct sock *newsk;
> + struct dst_entry *dst = NULL;
> + const struct tcphdr *tcph;
> + struct neighbour *n;
> + struct net_device *ndev;
> + struct chtls_sock *csk;
> + struct tcp_sock *tp;
> + struct inet_sock *newinet;
> + u16 port_id;
> + int step;
> + int rxq_idx;
> + const struct iphdr *iph = (const struct iphdr *)network_hdr;
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + newsk = tcp_create_openreq_child(lsk, oreq, cdev->askb);
> + if (!newsk)
> + goto free_oreq;
> +
> + dst = inet_csk_route_child_sock(lsk, newsk, oreq);
> + if (!dst)
> + goto free_sk;
> +
> + tcph = (struct tcphdr *)(iph + 1);
> + n = dst_neigh_lookup(dst, &iph->saddr);
> + if (!n)
> + goto free_sk;
> +
> + ndev = n->dev;
> + if (!ndev)
> + goto free_sk;
> + port_id = cxgb4_port_idx(ndev);
> +
> + csk = chtls_sock_create(cdev);
> + if (!csk)
> + goto free_sk;
> +
> + csk->l2t_entry = cxgb4_l2t_get(cdev->lldi->l2t, n, ndev, 0);
> + if (!csk->l2t_entry)
> + goto free_csk;
> +
> + newsk->sk_user_data = csk;
> + newsk->sk_backlog_rcv = chtls_backlog_rcv;
> +
> + tp = tcp_sk(newsk);
> + newinet = inet_sk(newsk);
> +
> + newinet->inet_daddr = iph->saddr;
> + newinet->inet_rcv_saddr = iph->daddr;
> + newinet->inet_saddr = iph->daddr;
> +
> + oreq->ts_recent = PASS_OPEN_TID_G(ntohl(req->tos_stid));
> + sk_setup_caps(newsk, dst);
> + csk->sk = newsk;
> + csk->passive_reap_next = oreq;
> + csk->tx_chan = cxgb4_port_chan(ndev);
> + csk->port_id = port_id;
> + csk->egress_dev = ndev;
> + csk->tos = PASS_OPEN_TOS_G(ntohl(req->tos_stid));
> + csk->ulp_mode = ULP_MODE_TLS;
> + step = cdev->lldi->nrxq / cdev->lldi->nchan;
> + csk->rss_qid = cdev->lldi->rxq_ids[port_id * step];
> + rxq_idx = port_id * step;
> + csk->txq_idx = (rxq_idx < cdev->lldi->ntxq) ? rxq_idx :
> + port_id * step;
> + csk->sndbuf = newsk->sk_sndbuf;
> + csk->smac_idx = cxgb4_tp_smt_idx(cdev->lldi->adapter_type,
> + cxgb4_port_viid(ndev));
> + tp->rcv_wnd = select_rcv_wnd(csk);
> +
> + neigh_release(n);
> + lsk->sk_prot->hash(newsk);
> + inet_inherit_port(&tcp_hashinfo, lsk, newsk);
> + bh_unlock_sock(newsk);
Where is this locked?
[Atul] tcp_create_openreq_child ->sk_clone_lock
> +
> + return newsk;
> +free_csk:
> + chtls_sock_release(&csk->kref);
> +free_sk:
> + dst_release(dst);
> +free_oreq:
> + chtls_reqsk_free(oreq);
> + return NULL;
> +}
> +
> +/*
> + * Populate a TID_RELEASE WR. The skb must be already propely sized.
> + */
> +static void mk_tid_release(struct sk_buff *skb,
> + unsigned int chan, unsigned int tid)
> +{
> + struct cpl_tid_release *req;
> + unsigned int len = roundup(sizeof(struct cpl_tid_release), 16);
Reverse christmas tree format?
[Atul] will take care in v2
> + req = (struct cpl_tid_release *)__skb_put(skb, len);
> + memset(req, 0, len);
> + set_wr_txq(skb, CPL_PRIORITY_SETUP, chan);
> + INIT_TP_WR_CPL(req, CPL_TID_RELEASE, tid);
> +}
> +
> +static int chtls_get_module(struct sock *sk)
> +{
> + struct inet_connection_sock *icsk = inet_csk(sk);
> +
> + if (!try_module_get(icsk->icsk_ulp_ops->owner))
> + return -1;
> +
> + return 0;
> +}
> +
> +static void chtls_pass_accept_request(struct sock *sk,
> + struct sk_buff *skb)
> +{
> + struct sock *newsk;
> + struct sk_buff *reply_skb;
> + struct cpl_t5_pass_accept_rpl *rpl;
> + struct cpl_pass_accept_req *req = cplhdr(skb) + RSS_HDR;
> + unsigned int tid = GET_TID(req);
> + struct ethhdr *eh;
> + struct iphdr *iph;
> + struct tcphdr *tcph;
> + struct request_sock *oreq = NULL;
> + void *network_hdr;
> + unsigned int len = roundup(sizeof(*rpl), 16);
> + struct chtls_dev *cdev = BLOG_SKB_CB(skb)->cdev;
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + newsk = lookup_tid(cdev->tids, tid);
> + if (newsk) {
> + pr_info("tid (%d) already in use\n", tid);
> + return;
> + }
> +
> + reply_skb = alloc_skb(len, GFP_ATOMIC);
> + if (!reply_skb) {
> + cxgb4_remove_tid(cdev->tids, 0, tid, sk->sk_family);
> + kfree_skb(skb);
> + return;
> + }
> +
> + if (sk->sk_state != TCP_LISTEN)
> + goto reject;
> +
> + if (inet_csk_reqsk_queue_is_full(sk))
> + goto reject;
> +
> + if (sk_acceptq_is_full(sk))
> + goto reject;
> +
> + oreq = inet_reqsk_alloc(&chtls_rsk_ops, sk, true);
> + if (!oreq)
> + goto reject;
> +
> + oreq->rsk_rcv_wnd = 0;
> + oreq->rsk_window_clamp = 0;
> + oreq->cookie_ts = 0;
> + oreq->mss = 0;
> + oreq->ts_recent = 0;
> +
> + eh = (struct ethhdr *)(req + 1);
> + iph = (struct iphdr *)(eh + 1);
> + if (iph->version != 0x4)
> + goto reject;
> +
> + network_hdr = (void *)(eh + 1);
> + tcph = (struct tcphdr *)(iph + 1);
> +
> + tcp_rsk(oreq)->tfo_listener = false;
> + tcp_rsk(oreq)->rcv_isn = ntohl(tcph->seq);
> + chtls_set_req_port(oreq, tcph->source, tcph->dest);
> + inet_rsk(oreq)->ecn_ok = 0;
> + chtls_set_req_addr(oreq, iph->daddr, iph->saddr);
> + chtls_set_req_opt(oreq, NULL);
> + if (req->tcpopt.wsf <= 14) {
> + inet_rsk(oreq)->wscale_ok = 1;
> + inet_rsk(oreq)->snd_wscale = req->tcpopt.wsf;
> + }
> + inet_rsk(oreq)->ir_iif = sk->sk_bound_dev_if;
> +
> + newsk = chtls_recv_sock(sk, oreq, network_hdr, req, cdev);
> + if (!newsk)
> + goto reject;
> +
> + if (chtls_get_module(newsk))
> + goto reject;
> + inet_csk_reqsk_queue_added(sk);
> + reply_skb->sk = newsk;
> + chtls_install_cpl_ops(newsk);
> + cxgb4_insert_tid(cdev->tids, newsk, tid, newsk->sk_family);
> + chtls_pass_accept_rpl(reply_skb, req, tid);
> + kfree_skb(skb);
> + return;
> +
> +reject:
Is this leaking oreq from inet_reqsk_alloc() above?
[Atul] will take care in v2
> + mk_tid_release(reply_skb, 0, tid);
> + cxgb4_ofld_send(cdev->lldi->ports[0], reply_skb);
> + kfree_skb(skb);
> +}
> +
> +/*
> + * Handle a CPL_PASS_ACCEPT_REQ message.
> + */
> +static int chtls_pass_accept_req(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct cpl_pass_accept_req *req = cplhdr(skb) + RSS_HDR;
> + unsigned int stid = PASS_OPEN_TID_G(ntohl(req->tos_stid));
> + unsigned int tid = GET_TID(req);
> + void *data;
> + struct listen_ctx *ctx;
> + struct sock *lsk;
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + data = lookup_stid(cdev->tids, stid);
> + if (!data)
> + return 1;
> +
> + ctx = (struct listen_ctx *)data;
> + lsk = ctx->lsk;
> +
> + if (unlikely(tid >= cdev->tids->ntids)) {
> + pr_info("passive open TID %u too large\n", tid);
> + return 1;
> + }
> +
> + BLOG_SKB_CB(skb)->cdev = cdev;
> + process_cpl_msg(chtls_pass_accept_request, lsk, skb);
> + return 0;
> +}
> +
> +/*
> + * Completes some final bits of initialization for just established connections
> + * and changes their state to TCP_ESTABLISHED.
> + *
> + * snd_isn here is the ISN after the SYN, i.e., the true ISN + 1.
> + */
> +static void make_established(struct sock *sk, u32 snd_isn, unsigned int opt)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + tp->pushed_seq = snd_isn;
> + tp->write_seq = snd_isn;
> + tp->snd_nxt = snd_isn;
> + tp->snd_una = snd_isn;
> + inet_sk(sk)->inet_id = tp->write_seq ^ jiffies;
What is the purpose of xor'ing this with jiffies?
[Atul] borrowed from tcp_v4_syn_recv_sock [inet->inet_id = tp->write_seq ^ jiffies;]
> + assign_rxopt(sk, opt);
Reverse christmas tree format?
> +
> + if (tp->rcv_wnd > (RCV_BUFSIZ_M << 10))
> + tp->rcv_wup -= tp->rcv_wnd - (RCV_BUFSIZ_M << 10);
> +
> + dst_confirm(sk->sk_dst_cache);
> +
> + smp_mb();
> + tcp_set_state(sk, TCP_ESTABLISHED);
> +}
> +
> +static void chtls_abort_conn(struct sock *sk, struct sk_buff *skb)
> +{
> + struct sk_buff *abort_skb;
> +
> + abort_skb = alloc_skb(sizeof(struct cpl_abort_req), GFP_ATOMIC);
> + if (abort_skb)
> + chtls_send_reset(sk, CPL_ABORT_SEND_RST, abort_skb);
> +}
> +
> +static struct sock *reap_list;
> +static DEFINE_SPINLOCK(reap_list_lock);
> +
> +/*
> + * Process the reap list.
> + */
> +DECLARE_TASK_FUNC(process_reap_list, task_param)
> +{
> + spin_lock_bh(&reap_list_lock);
> + while (reap_list) {
> + struct sock *sk = reap_list;
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> +
> + reap_list = csk->passive_reap_next;
> + csk->passive_reap_next = NULL;
> + spin_unlock(&reap_list_lock);
> + sock_hold(sk);
> +
> + bh_lock_sock(sk);
> + chtls_abort_conn(sk, NULL);
> + sock_orphan(sk);
> + if (sk->sk_state == TCP_CLOSE)
> + inet_csk_destroy_sock(sk);
> + bh_unlock_sock(sk);
> + sock_put(sk);
> + spin_lock(&reap_list_lock);
Could it be that you are missing a spin_unlock() here?
[Atul] unlock is called in add_to_reap_list where reap task is called
> + }
> + spin_unlock_bh(&reap_list_lock);
> +}
> +
> +static DECLARE_WORK(reap_task, process_reap_list);
> +
> +static void add_to_reap_list(struct sock *sk)
> +{
> + struct chtls_sock *csk = sk->sk_user_data;
> +
> + local_bh_disable();
> + bh_lock_sock(sk);
> + release_tcp_port(sk); /* release the port immediately */
> +
> + spin_lock(&reap_list_lock);
> + csk->passive_reap_next = reap_list;
> + reap_list = sk;
> + if (!csk->passive_reap_next)
> + schedule_work(&reap_task);
> + spin_unlock(&reap_list_lock);
> + bh_unlock_sock(sk);
> + local_bh_enable();
> +}
> +
> +static void add_pass_open_to_parent(struct sock *child, struct sock *lsk,
> + struct chtls_dev *cdev)
> +{
> + struct chtls_sock *csk = child->sk_user_data;
> + struct request_sock *oreq;
> +
> + if (lsk->sk_state != TCP_LISTEN)
> + return;
> +
> + oreq = csk->passive_reap_next;
> + csk->passive_reap_next = NULL;
> +
> + reqsk_queue_removed(&inet_csk(lsk)->icsk_accept_queue, oreq);
> +
> + if (sk_acceptq_is_full(lsk)) {
> + chtls_reqsk_free(oreq);
> + add_to_reap_list(child);
> + } else {
> + refcount_set(&oreq->rsk_refcnt, 1);
> + inet_csk_reqsk_queue_add(lsk, oreq, child);
> + lsk->sk_data_ready(lsk);
> + }
> +}
> +
> +static void bl_add_pass_open_to_parent(struct sock *lsk, struct sk_buff *skb)
> +{
> + struct sock *child = skb->sk;
> +
> + skb->sk = NULL;
> + add_pass_open_to_parent(child, lsk, BLOG_SKB_CB(skb)->cdev);
> + kfree_skb(skb);
> +}
> +
> +static int chtls_pass_establish(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct cpl_pass_establish *req = cplhdr(skb) + RSS_HDR;
> + struct chtls_sock *csk;
> + struct sock *lsk, *sk;
> + unsigned int hwtid = GET_TID(req);
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + if (!sk)
> + return 1;
> +
> + bh_lock_sock(sk);
> + if (unlikely(sock_owned_by_user(sk))) {
> + kfree_skb(skb);
> + } else {
> + void *data;
> + unsigned int stid;
> +
> + csk = sk->sk_user_data;
> + csk->wr_max_credits = 64;
> + csk->wr_credits = 64;
> + csk->wr_unacked = 0;
> + make_established(sk, ntohl(req->snd_isn), ntohs(req->tcp_opt));
> + stid = PASS_OPEN_TID_G(ntohl(req->tos_stid));
> + sk->sk_state_change(sk);
> + if (unlikely(sk->sk_socket))
> + sk_wake_async(sk, 0, POLL_OUT);
> +
> + data = lookup_stid(cdev->tids, stid);
> + lsk = ((struct listen_ctx *)data)->lsk;
> +
> + bh_lock_sock(lsk);
> + if (likely(!sock_owned_by_user(lsk))) {
> + kfree_skb(skb);
> + add_pass_open_to_parent(sk, lsk, cdev);
> + } else {
> + skb->sk = sk;
> + BLOG_SKB_CB(skb)->cdev = cdev;
> + BLOG_SKB_CB(skb)->backlog_rcv =
> + bl_add_pass_open_to_parent;
> + __sk_add_backlog(lsk, skb);
> + }
> + bh_unlock_sock(lsk);
> + }
> + bh_unlock_sock(sk);
> + return 0;
> +}
> +
> +/*
> + * Handle receipt of an urgent pointer.
> + */
> +static void handle_urg_ptr(struct sock *sk, u32 urg_seq)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + urg_seq--;
> + if (tp->urg_data && !after(urg_seq, tp->urg_seq))
> + return; /* duplicate pointer */
> +
> + sk_send_sigurg(sk);
> + if (tp->urg_seq == tp->copied_seq && tp->urg_data &&
> + !sock_flag(sk, SOCK_URGINLINE) &&
> + tp->copied_seq != tp->rcv_nxt) {
> + struct sk_buff *skb = skb_peek(&sk->sk_receive_queue);
> +
> + tp->copied_seq++;
> + if (skb && tp->copied_seq - ULP_SKB_CB(skb)->seq >= skb->len)
> + chtls_free_skb(sk, skb);
> + }
> +
> + tp->urg_data = TCP_URG_NOTYET;
> + tp->urg_seq = urg_seq;
> +}
> +
> +static void check_sk_callbacks(struct chtls_sock *csk)
> +{
> + struct sock *sk = csk->sk;
> +
> + if (unlikely(sk->sk_user_data &&
> + !csk_flag_nochk(csk, CSK_CALLBACKS_CHKD)))
> + csk_set_flag(csk, CSK_CALLBACKS_CHKD);
> +}
> +
> +/*
> + * Handles Rx data that arrives in a state where the socket isn't accepting
> + * new data.
> + */
> +static void handle_excess_rx(struct sock *sk, struct sk_buff *skb)
> +{
> + if (!csk_flag(sk, CSK_ABORT_SHUTDOWN))
> + chtls_abort_conn(sk, skb);
> +
> + kfree_skb(skb);
> +}
> +
> +static void chtls_recv_data(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_rx_data *hdr = cplhdr(skb) + RSS_HDR;
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + if (unlikely(sk->sk_shutdown & RCV_SHUTDOWN)) {
> + handle_excess_rx(sk, skb);
> + return;
> + }
> +
> + ULP_SKB_CB(skb)->seq = ntohl(hdr->seq);
> + ULP_SKB_CB(skb)->psh = hdr->psh;
> + skb_ulp_mode(skb) = ULP_MODE_NONE;
> +
> + skb_reset_transport_header(skb);
> + __skb_pull(skb, sizeof(*hdr) + RSS_HDR);
> + if (!skb->data_len)
> + __skb_trim(skb, ntohs(hdr->len));
> +
> + if (unlikely(hdr->urg))
> + handle_urg_ptr(sk, tp->rcv_nxt + ntohs(hdr->urg));
> + if (unlikely(tp->urg_data == TCP_URG_NOTYET &&
> + tp->urg_seq - tp->rcv_nxt < skb->len))
> + tp->urg_data = TCP_URG_VALID |
> + skb->data[tp->urg_seq - tp->rcv_nxt];
> +
> + if (unlikely(hdr->dack_mode != csk->delack_mode)) {
> + csk->delack_mode = hdr->dack_mode;
> + csk->delack_seq = tp->rcv_nxt;
> + }
> +
> + tcp_hdr(skb)->fin = 0;
> + tp->rcv_nxt += skb->len;
> +
> + __skb_queue_tail(&sk->sk_receive_queue, skb);
> +
> + if (!sock_flag(sk, SOCK_DEAD)) {
> + check_sk_callbacks(csk);
> + sk->sk_data_ready(sk);
> + }
> +}
> +
> +static int chtls_rx_data(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct sock *sk;
> + struct cpl_rx_data *req = cplhdr(skb) + RSS_HDR;
> + unsigned int hwtid = GET_TID(req);
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + skb_dst_set(skb, NULL);
> + process_cpl_msg(chtls_recv_data, sk, skb);
> + return 0;
> +}
> +
> +static void chtls_recv_pdu(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct chtls_hws *tlsk = &csk->tlshws;
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_tls_data *hdr = cplhdr(skb);
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + if (unlikely(sk->sk_shutdown & RCV_SHUTDOWN)) {
> + handle_excess_rx(sk, skb);
> + return;
> + }
> +
> + ULP_SKB_CB(skb)->seq = ntohl(hdr->seq);
> + ULP_SKB_CB(skb)->flags = 0;
> + skb_ulp_mode(skb) = ULP_MODE_TLS;
> +
> + skb_reset_transport_header(skb);
> + __skb_pull(skb, sizeof(*hdr));
> + if (!skb->data_len)
> + __skb_trim(skb,
> + CPL_TLS_DATA_LENGTH_G(ntohl(hdr->length_pkd)));
> +
> + if (unlikely(tp->urg_data == TCP_URG_NOTYET && tp->urg_seq -
> + tp->rcv_nxt < skb->len))
> + tp->urg_data = TCP_URG_VALID |
> + skb->data[tp->urg_seq - tp->rcv_nxt];
> +
> + tcp_hdr(skb)->fin = 0;
> + tlsk->pldlen = CPL_TLS_DATA_LENGTH_G(ntohl(hdr->length_pkd));
> + __skb_queue_tail(&tlsk->sk_recv_queue, skb);
> +}
> +
> +static int chtls_rx_pdu(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct sock *sk;
> + struct cpl_tls_data *req = cplhdr(skb);
> + unsigned int hwtid = GET_TID(req);
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + skb_dst_set(skb, NULL);
> + process_cpl_msg(chtls_recv_pdu, sk, skb);
> + return 0;
> +}
> +
> +static void chtls_set_hdrlen(struct sk_buff *skb, unsigned int nlen)
> +{
> + struct tlsrx_cmp_hdr *tls_cmp_hdr = cplhdr(skb);
> +
> + skb->hdr_len = ntohs(tls_cmp_hdr->length);
> + tls_cmp_hdr->length = ntohs(nlen);
> +}
> +
> +static void chtls_rx_hdr(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct chtls_hws *tlsk = &csk->tlshws;
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_rx_tls_cmp *cmp_cpl = cplhdr(skb);
> + struct sk_buff *skb_rec = NULL;
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + ULP_SKB_CB(skb)->seq = ntohl(cmp_cpl->seq);
> + ULP_SKB_CB(skb)->flags = 0;
> +
> + skb_reset_transport_header(skb);
> + __skb_pull(skb, sizeof(*cmp_cpl));
> + if (!skb->data_len)
> + __skb_trim(skb, CPL_RX_TLS_CMP_LENGTH_G
> + (ntohl(cmp_cpl->pdulength_length)));
> +
> + tp->rcv_nxt +=
> + CPL_RX_TLS_CMP_PDULENGTH_G(ntohl(cmp_cpl->pdulength_length));
> +
> + skb_rec = __skb_dequeue(&tlsk->sk_recv_queue);
> + if (!skb_rec) {
> + ULP_SKB_CB(skb)->flags |= ULPCB_FLAG_TLS_ND;
> + __skb_queue_tail(&sk->sk_receive_queue, skb);
> + } else {
> + chtls_set_hdrlen(skb, tlsk->pldlen);
> + tlsk->pldlen = 0;
> + __skb_queue_tail(&sk->sk_receive_queue, skb);
> + __skb_queue_tail(&sk->sk_receive_queue, skb_rec);
> + }
> +
> + if (!sock_flag(sk, SOCK_DEAD)) {
> + check_sk_callbacks(csk);
> + sk->sk_data_ready(sk);
> + }
> +}
> +
> +static int chtls_rx_cmp(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct sock *sk;
> + struct cpl_rx_tls_cmp *req = cplhdr(skb);
> + unsigned int hwtid = GET_TID(req);
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + skb_dst_set(skb, NULL);
> + process_cpl_msg(chtls_rx_hdr, sk, skb);
> +
> + return 0;
> +}
> +
> +static void chtls_timewait(struct sock *sk)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + tp->rcv_nxt++;
> + tp->rx_opt.ts_recent_stamp = get_seconds();
> + tp->srtt_us = 0;
> + tcp_time_wait_p(sk, TCP_TIME_WAIT, 0);
> +}
> +
> +static void chtls_peer_close(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> +
> + sk->sk_shutdown |= RCV_SHUTDOWN;
> + sock_set_flag(sk, SOCK_DONE);
> +
> + switch (sk->sk_state) {
> + case TCP_SYN_RECV:
> + case TCP_ESTABLISHED:
> + tcp_set_state(sk, TCP_CLOSE_WAIT);
> + break;
> + case TCP_FIN_WAIT1:
> + tcp_set_state(sk, TCP_CLOSING);
> + break;
> + case TCP_FIN_WAIT2:
> + chtls_release_resources(sk);
> + if (csk_flag_nochk(csk, CSK_ABORT_RPL_PENDING))
> + chtls_conn_done(sk);
> + else
> + chtls_timewait(sk);
> + break;
> + default:
> + pr_info("cpl_peer_close in bad state %d\n", sk->sk_state);
> + }
> +
> + if (!sock_flag(sk, SOCK_DEAD)) {
> + sk->sk_state_change(sk);
> + /* Do not send POLL_HUP for half duplex close. */
> +
> + if ((sk->sk_shutdown & SEND_SHUTDOWN) ||
> + sk->sk_state == TCP_CLOSE)
> + sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_HUP);
> + else
> + sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN);
> + }
> +}
> +
> +static void chtls_close_con_rpl(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_close_con_rpl *rpl = cplhdr(skb) + RSS_HDR;
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + tp->snd_una = ntohl(rpl->snd_nxt) - 1; /* exclude FIN */
> +
> + switch (sk->sk_state) {
> + case TCP_CLOSING:
> + chtls_release_resources(sk);
> + if (csk_flag_nochk(csk, CSK_ABORT_RPL_PENDING))
> + chtls_conn_done(sk);
> + else
> + chtls_timewait(sk);
> + break;
> + case TCP_LAST_ACK:
> + chtls_release_resources(sk);
> + chtls_conn_done(sk);
> + break;
> + case TCP_FIN_WAIT1:
> + tcp_set_state(sk, TCP_FIN_WAIT2);
> + sk->sk_shutdown |= SEND_SHUTDOWN;
> + dst_confirm(sk->sk_dst_cache);
> +
> + if (!sock_flag(sk, SOCK_DEAD))
> + sk->sk_state_change(sk);
> + else if (tcp_sk(sk)->linger2 < 0 &&
> + !csk_flag_nochk(csk, CSK_ABORT_SHUTDOWN))
> + chtls_abort_conn(sk, skb);
> + break;
> + default:
> + pr_info("close_con_rpl in bad state %d\n", sk->sk_state);
> + }
> + kfree_skb(skb);
> +}
> +
> +static struct sk_buff *get_cpl_skb(struct sk_buff *skb,
> + size_t len, gfp_t gfp)
> +{
> + if (likely(!skb_is_nonlinear(skb) && !skb_cloned(skb))) {
> + WARN_ONCE(skb->len < len, "skb alloc error");
> + __skb_trim(skb, len);
> + skb_get(skb);
> + } else {
> + skb = alloc_skb(len, gfp);
> + if (skb)
> + __skb_put(skb, len);
> + }
> + return skb;
> +}
> +
> +static void set_abort_rpl_wr(struct sk_buff *skb, unsigned int tid,
> + int cmd)
> +{
> + struct cpl_abort_rpl *rpl = cplhdr(skb);
> +
> + INIT_TP_WR_CPL(rpl, CPL_ABORT_RPL, tid);
> + rpl->cmd = cmd;
> +}
> +
> +static void send_defer_abort_rpl(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct sk_buff *reply_skb;
> + struct cpl_abort_req_rss *req = cplhdr(skb);
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + reply_skb = alloc_skb(sizeof(struct cpl_abort_rpl),
> + GFP_KERNEL | __GFP_NOFAIL);
> + if (!reply_skb)
> + return;
> +
> + __skb_put(reply_skb, sizeof(struct cpl_abort_rpl));
> + set_abort_rpl_wr(reply_skb, GET_TID(req),
> + (req->status & CPL_ABORT_NO_RST));
> + set_wr_txq(reply_skb, CPL_PRIORITY_DATA, req->status >> 1);
> + cxgb4_ofld_send(cdev->lldi->ports[0], reply_skb);
> + kfree_skb(skb);
> +}
> +
> +static void send_abort_rpl(struct sock *sk, struct sk_buff *skb,
> + struct chtls_dev *cdev, int status, int queue)
> +{
> + struct sk_buff *reply_skb;
> + struct cpl_abort_req_rss *req = cplhdr(skb);
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + reply_skb = alloc_skb(sizeof(struct cpl_abort_rpl),
> + GFP_KERNEL);
> +
> + if (!reply_skb) {
> + req->status = (queue << 1);
> + send_defer_abort_rpl(cdev, skb);
> + return;
> + }
> +
> + set_abort_rpl_wr(reply_skb, GET_TID(req), status);
> + kfree_skb(skb);
> +
> + set_wr_txq(reply_skb, CPL_PRIORITY_DATA, queue);
> + if (sock_flag(sk, SOCK_INLINE)) {
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct l2t_entry *e = csk->l2t_entry;
> +
> + if (e && sk->sk_state != TCP_SYN_RECV) {
> + cxgb4_l2t_send(csk->egress_dev, reply_skb, e);
> + return;
> + }
> + }
> + cxgb4_ofld_send(cdev->lldi->ports[0], reply_skb);
> +}
> +
> +/*
> + * Add an skb to the deferred skb queue for processing from process context.
> + */
> +void t4_defer_reply(struct sk_buff *skb, struct chtls_dev *cdev,
> + defer_handler_t handler)
> +{
> + DEFERRED_SKB_CB(skb)->handler = handler;
> + spin_lock_bh(&cdev->deferq.lock);
> + __skb_queue_tail(&cdev->deferq, skb);
> + if (skb_queue_len(&cdev->deferq) == 1)
> + schedule_work(&cdev->deferq_task);
> + spin_unlock_bh(&cdev->deferq.lock);
> +}
> +
> +static void chtls_send_abort_rpl(struct sock *sk, struct sk_buff *skb,
> + struct chtls_dev *cdev,
> + int status, int queue)
> +{
> + struct sk_buff *reply_skb;
> + struct cpl_abort_req_rss *req = cplhdr(skb) + RSS_HDR;
> + unsigned int tid = GET_TID(req);
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + reply_skb = get_cpl_skb(skb, sizeof(struct cpl_abort_rpl), gfp_any());
> + if (!reply_skb) {
> + req->status = (queue << 1) | status;
> + t4_defer_reply(skb, cdev, send_defer_abort_rpl);
> + return;
> + }
> +
> + set_abort_rpl_wr(reply_skb, tid, status);
> + set_wr_txq(reply_skb, CPL_PRIORITY_DATA, queue);
> + if (sock_flag(sk, SOCK_INLINE)) {
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct l2t_entry *e = csk->l2t_entry;
> +
> + if (e && sk->sk_state != TCP_SYN_RECV) {
> + cxgb4_l2t_send(csk->egress_dev, reply_skb, e);
> + return;
> + }
> + }
> + cxgb4_ofld_send(cdev->lldi->ports[0], reply_skb);
> + kfree_skb(skb);
> +}
> +
> +/*
> + * This is run from a listener's backlog to abort a child connection in
> + * SYN_RCV state (i.e., one on the listener's SYN queue).
> + */
> +static void bl_abort_syn_rcv(struct sock *lsk, struct sk_buff *skb)
> +{
> + struct sock *child = skb->sk;
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(child);
> + int queue = csk->txq_idx;
> +
> + skb->sk = NULL;
> + do_abort_syn_rcv(child, lsk);
> + send_abort_rpl(child, skb, BLOG_SKB_CB(skb)->cdev,
> + CPL_ABORT_NO_RST, queue);
> +}
> +
> +static int abort_syn_rcv(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = sk->sk_user_data;
> + struct chtls_dev *cdev = csk->cdev;
> + const struct request_sock *oreq = csk->passive_reap_next;
> + struct listen_ctx *listen_ctx;
> + struct sock *psk;
> + void *ctx;
> +
> + if (!oreq)
> + return -1;
> +
> + ctx = lookup_stid(cdev->tids, oreq->ts_recent);
> + if (!ctx)
> + return -1;
> +
> + listen_ctx = (struct listen_ctx *)ctx;
> + psk = listen_ctx->lsk;
> +
> + bh_lock_sock(psk);
> + if (!sock_owned_by_user(psk)) {
> + int queue = csk->txq_idx;
> +
> + do_abort_syn_rcv(sk, psk);
> + send_abort_rpl(sk, skb, cdev, CPL_ABORT_NO_RST, queue);
> + } else {
> + skb->sk = sk;
> + BLOG_SKB_CB(skb)->backlog_rcv = bl_abort_syn_rcv;
> + __sk_add_backlog(psk, skb);
> + }
> + bh_unlock_sock(psk);
> + return 0;
> +}
> +
> +static void chtls_abort_req_rss(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = sk->sk_user_data;
> + const struct cpl_abort_req_rss *req = cplhdr(skb) + RSS_HDR;
> + int queue = csk->txq_idx;
> + int rst_status = CPL_ABORT_NO_RST;
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + if (is_neg_adv(req->status)) {
> + if (sk->sk_state == TCP_SYN_RECV)
> + chtls_set_tcb_tflag(sk, 0, 0);
> +
> + kfree_skb(skb);
> + return;
> + }
> +
> + csk_reset_flag(csk, CSK_ABORT_REQ_RCVD);
> +
> + if (!csk_flag_nochk(csk, CSK_ABORT_SHUTDOWN) &&
> + !csk_flag_nochk(csk, CSK_TX_DATA_SENT)) {
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + if (send_tx_flowc_wr(sk, 0, tp->snd_nxt, tp->rcv_nxt) < 0)
> + WARN_ONCE(1, "send_tx_flowc error");
> + csk_set_flag(csk, CSK_TX_DATA_SENT);
> + }
> +
> + csk_set_flag(csk, CSK_ABORT_SHUTDOWN);
> +
> + if (!csk_flag_nochk(csk, CSK_ABORT_RPL_PENDING)) {
> + sk->sk_err = ETIMEDOUT;
> +
> + if (!sock_flag(sk, SOCK_DEAD))
> + sk->sk_error_report(sk);
> +
> + if (sk->sk_state == TCP_SYN_RECV && !abort_syn_rcv(sk, skb))
> + return;
> +
> + chtls_release_resources(sk);
> + chtls_conn_done(sk);
> + }
> +
> + chtls_send_abort_rpl(sk, skb, csk->cdev, rst_status, queue);
> +}
> +
> +static void chtls_abort_rpl_rss(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct cpl_abort_rpl_rss *rpl = cplhdr(skb) + RSS_HDR;
> + struct chtls_dev *cdev = csk->cdev;
> +
> + if (csk_flag_nochk(csk, CSK_ABORT_RPL_PENDING)) {
> + csk_reset_flag(csk, CSK_ABORT_RPL_PENDING);
> + if (!csk_flag_nochk(csk, CSK_ABORT_REQ_RCVD)) {
> + if (sk->sk_state == TCP_SYN_SENT) {
> + cxgb4_remove_tid(cdev->tids,
> + csk->port_id,
> + GET_TID(rpl),
> + sk->sk_family);
> + sock_put(sk);
> + }
> + chtls_release_resources(sk);
> + chtls_conn_done(sk);
> + }
> + }
> + kfree_skb(skb);
> +}
> +
> +static int chtls_conn_cpl(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct sock *sk;
> + struct cpl_peer_close *req = cplhdr(skb) + RSS_HDR;
> + unsigned int hwtid = GET_TID(req);
> + void (*fn)(struct sock *sk, struct sk_buff *skb);
> + u8 opcode = ((const struct rss_header *)cplhdr(skb))->opcode;
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + if (!sk)
> + goto rel_skb;
> +
> + switch (opcode) {
> + case CPL_PEER_CLOSE:
> + fn = chtls_peer_close;
> + break;
> + case CPL_CLOSE_CON_RPL:
> + fn = chtls_close_con_rpl;
> + break;
> + case CPL_ABORT_REQ_RSS:
> + fn = chtls_abort_req_rss;
> + break;
> + case CPL_ABORT_RPL_RSS:
> + fn = chtls_abort_rpl_rss;
> + break;
> + default:
> + goto rel_skb;
> + }
> +
> + process_cpl_msg(fn, sk, skb);
> + return 0;
> +
> +rel_skb:
> + kfree_skb(skb);
> + return 0;
> +}
> +
> +static struct sk_buff *dequeue_wr(struct sock *sk)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct sk_buff *skb = csk->wr_skb_head;
> +
> + if (likely(skb)) {
> + /* Don't bother clearing the tail */
> + csk->wr_skb_head = WR_SKB_CB(skb)->next_wr;
> + WR_SKB_CB(skb)->next_wr = NULL;
> + }
> + return skb;
> +}
> +
> +static void chtls_rx_ack(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = sk->sk_user_data;
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_fw4_ack *hdr = cplhdr(skb) + RSS_HDR;
> + u8 credits = hdr->credits;
> + u32 snd_una = ntohl(hdr->snd_una);
Reverse christmas tree format?
[Atul] will take care in v2
> +
> + csk->wr_credits += credits;
> +
> + if (csk->wr_unacked > csk->wr_max_credits - csk->wr_credits)
> + csk->wr_unacked = csk->wr_max_credits - csk->wr_credits;
> +
> + while (credits) {
> + struct sk_buff *pskb = csk->wr_skb_head;
> +
> + if (unlikely(!pskb)) {
> + if (csk->wr_nondata)
> + csk->wr_nondata -= credits;
> + break;
> + }
> + if (unlikely(credits < pskb->csum)) {
> + pskb->csum -= credits;
> + break;
> + }
> + dequeue_wr(sk);
> + credits -= pskb->csum;
> + kfree_skb(pskb);
> + }
> + if (hdr->seq_vld & CPL_FW4_ACK_FLAGS_SEQVAL) {
> + if (unlikely(before(snd_una, tp->snd_una))) {
> + kfree_skb(skb);
> + return;
> + }
> +
> + if (tp->snd_una != snd_una) {
> + tp->snd_una = snd_una;
> + dst_confirm(sk->sk_dst_cache);
> + tp->rcv_tstamp = tcp_time_stamp(tp);
> + if (tp->snd_una == tp->snd_nxt &&
> + !csk_flag_nochk(csk, CSK_TX_FAILOVER))
> + csk_reset_flag(csk, CSK_TX_WAIT_IDLE);
> + }
> + }
> +
> + if (hdr->seq_vld & CPL_FW4_ACK_FLAGS_CH) {
> + unsigned int fclen16 = roundup(failover_flowc_wr_len, 16);
> +
> + csk->wr_credits -= fclen16;
> + csk_reset_flag(csk, CSK_TX_WAIT_IDLE);
> + csk_reset_flag(csk, CSK_TX_FAILOVER);
> + }
> + if (skb_queue_len(&csk->txq) && chtls_push_frames(csk, 0))
> + sk->sk_write_space(sk);
> + kfree_skb(skb);
I guess you actually always want to kfree_skb(skb) here, right?
[Atul] yes
> +}
> +
> +static int chtls_wr_ack(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct cpl_fw4_ack *rpl = cplhdr(skb) + RSS_HDR;
> + unsigned int hwtid = GET_TID(rpl);
> + struct sock *sk;
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + process_cpl_msg(chtls_rx_ack, sk, skb);
> +
> + return 0;
> +}
> +
> +chtls_handler_func chtls_handlers[NUM_CPL_CMDS] = {
> + [CPL_PASS_OPEN_RPL] = chtls_pass_open_rpl,
> + [CPL_CLOSE_LISTSRV_RPL] = chtls_close_listsrv_rpl,
> + [CPL_PASS_ACCEPT_REQ] = chtls_pass_accept_req,
> + [CPL_PASS_ESTABLISH] = chtls_pass_establish,
> + [CPL_RX_DATA] = chtls_rx_data,
> + [CPL_TLS_DATA] = chtls_rx_pdu,
> + [CPL_RX_TLS_CMP] = chtls_rx_cmp,
> + [CPL_PEER_CLOSE] = chtls_conn_cpl,
> + [CPL_CLOSE_CON_RPL] = chtls_conn_cpl,
> + [CPL_ABORT_REQ_RSS] = chtls_conn_cpl,
> + [CPL_ABORT_RPL_RSS] = chtls_conn_cpl,
> + [CPL_FW4_ACK] = chtls_wr_ack,
> +};
--
Stefano
Thanks
Atul
^ permalink raw reply
* [PATCH net-next v4 3/4] net: phy: smsc: LAN8710/20: add PHY_RST_AFTER_CLK_EN flag
From: Richard Leitner @ 2017-12-07 14:43 UTC (permalink / raw)
To: robh+dt, mark.rutland, fugang.duan, andrew, f.fainelli,
frowand.list
Cc: davem, geert+renesas, sergei.shtylyov, baruch, david.wu, lukma,
netdev, devicetree, linux-kernel, richard.leitner
In-Reply-To: <20171207144358.3351-1-dev@g0hl1n.net>
From: Richard Leitner <richard.leitner@skidata.com>
The Microchip/SMSC LAN8710/LAN8720 PHYs need (according to their
datasheet [1]) a continuous REF_CLK when configured to "REF_CLK In Mode".
Therefore set the PHY_RST_AFTER_CLK_EN flag for those PHYs to let the
ETH driver reset them after the REF_CLK is enabled.
[1] http://ww1.microchip.com/downloads/en/DeviceDoc/00002165B.pdf
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/smsc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index a1961ba87e2b..be399d645224 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -312,7 +312,7 @@ static struct phy_driver smsc_phy_driver[] = {
.name = "SMSC LAN8710/LAN8720",
.features = PHY_BASIC_FEATURES,
- .flags = PHY_HAS_INTERRUPT,
+ .flags = PHY_HAS_INTERRUPT | PHY_RST_AFTER_CLK_EN,
.probe = smsc_phy_probe,
--
2.11.0
^ permalink raw reply related
* [PATCH net-next v4 4/4] net: fec: add phy_reset_after_clk_enable() support
From: Richard Leitner @ 2017-12-07 14:43 UTC (permalink / raw)
To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
fugang.duan-3arQi8VN3Tc, andrew-g2DYL2Zd6BY,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8,
baruch-NswTu9S1W3P6gbPvEgmw2w, david.wu-TNX95d0MmH7DzftRWevZcw,
lukma-ynQEQJNshbs, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
richard.leitner-WcANXNA0UjBBDgjK7y7TUQ
In-Reply-To: <20171207144358.3351-1-dev-M/VWbR8SM2SsTnJN9+BGXg@public.gmane.org>
From: Richard Leitner <richard.leitner-WcANXNA0UjBBDgjK7y7TUQ@public.gmane.org>
Some PHYs (for example the SMSC LAN8710/LAN8720) doesn't allow turning
the refclk on and off again during operation (according to their
datasheet). Nonetheless exactly this behaviour was introduced for power
saving reasons by commit e8fcfcd5684a ("net: fec: optimize the clock management to save power").
Therefore add support for the phy_reset_after_clk_enable function from
phylib to mitigate this issue.
Generally speaking this issue is only relevant if the ref clk for the
PHY is generated by the SoC and therefore the PHY is configured to
"REF_CLK In Mode". In our specific case (PCB) this problem does occur at
about every 10th to 50th POR of an LAN8710 connected to an i.MX6SOLO
SoC. The typical symptom of this problem is a "swinging" ethernet link.
Similar issues were reported by users of the NXP forum:
https://community.nxp.com/thread/389902
https://community.nxp.com/message/309354
With this patch applied the issue didn't occur for at least a few
hundret PORs of our board.
Fixes: e8fcfcd5684a ("net: fec: optimize the clock management to save power")
Signed-off-by: Richard Leitner <richard.leitner-WcANXNA0UjBBDgjK7y7TUQ@public.gmane.org>
---
drivers/net/ethernet/freescale/fec_main.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 610573855213..2d1b06579c1a 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1862,6 +1862,8 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
ret = clk_prepare_enable(fep->clk_ref);
if (ret)
goto failed_clk_ref;
+
+ phy_reset_after_clk_enable(ndev->phydev);
} else {
clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_enet_out);
@@ -2834,6 +2836,7 @@ fec_enet_open(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
int ret;
+ bool reset_again;
ret = pm_runtime_get_sync(&fep->pdev->dev);
if (ret < 0)
@@ -2844,6 +2847,17 @@ fec_enet_open(struct net_device *ndev)
if (ret)
goto clk_enable;
+ /* During the first fec_enet_open call the PHY isn't probed at this
+ * point. Therefore the phy_reset_after_clk_enable() call within
+ * fec_enet_clk_enable() fails. As we need this reset in order to be
+ * sure the PHY is working correctly we check if we need to reset again
+ * later when the PHY is probed
+ */
+ if (ndev->phydev && ndev->phydev->drv)
+ reset_again = false;
+ else
+ reset_again = true;
+
/* I should reset the ring buffers here, but I don't yet know
* a simple way to do that.
*/
@@ -2860,6 +2874,12 @@ fec_enet_open(struct net_device *ndev)
if (ret)
goto err_enet_mii_probe;
+ /* Call phy_reset_after_clk_enable() again if it failed during
+ * phy_reset_after_clk_enable() before because the PHY wasn't probed.
+ */
+ if (reset_again)
+ phy_reset_after_clk_enable(ndev->phydev);
+
if (fep->quirks & FEC_QUIRK_ERR006687)
imx6q_cpuidle_fec_irqs_used();
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next v4 2/4] phylib: add reset after clk enable support
From: Richard Leitner @ 2017-12-07 14:43 UTC (permalink / raw)
To: robh+dt, mark.rutland, fugang.duan, andrew, f.fainelli,
frowand.list
Cc: davem, geert+renesas, sergei.shtylyov, baruch, david.wu, lukma,
netdev, devicetree, linux-kernel, richard.leitner
In-Reply-To: <20171207144358.3351-1-dev@g0hl1n.net>
From: Richard Leitner <richard.leitner@skidata.com>
Some PHYs need the refclk to be a continuous clock. Therefore they don't
allow turning it off and on again during operation. Nonetheless such a
clock switching is performed by some ETH drivers (namely FEC [1]) for
power saving reasons. An example for an affected PHY is the
SMSC/Microchip LAN8720 in "REF_CLK In Mode".
In order to provide a uniform method to overcome this problem this patch
adds a new phy_driver flag (PHY_RST_AFTER_CLK_EN) and corresponding
function phy_reset_after_clk_enable() to the phylib. These should be
used to trigger reset of the PHY after the refclk is switched on again.
[1] commit e8fcfcd5684a ("net: fec: optimize the clock management to save power")
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/phy_device.c | 24 ++++++++++++++++++++++++
include/linux/phy.h | 2 ++
2 files changed, 26 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 1de5e242b8b4..462c17ed87b8 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1218,6 +1218,30 @@ int phy_loopback(struct phy_device *phydev, bool enable)
}
EXPORT_SYMBOL(phy_loopback);
+/**
+ * phy_reset_after_clk_enable - perform a PHY reset if needed
+ * @phydev: target phy_device struct
+ *
+ * Description: Some PHYs are known to need a reset after their refclk was
+ * enabled. This function evaluates the flags and perform the reset if it's
+ * needed. Returns < 0 on error, 0 if the phy wasn't reset and 1 if the phy
+ * was reset.
+ */
+int phy_reset_after_clk_enable(struct phy_device *phydev)
+{
+ if (!phydev || !phydev->drv)
+ return -ENODEV;
+
+ if (phydev->drv->flags & PHY_RST_AFTER_CLK_EN) {
+ phy_device_reset(phydev, 1);
+ phy_device_reset(phydev, 0);
+ return 1;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(phy_reset_after_clk_enable);
+
/* Generic PHY support and helper functions */
/**
diff --git a/include/linux/phy.h b/include/linux/phy.h
index d3037e2ffbc4..c4b4715caa21 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -59,6 +59,7 @@
#define PHY_HAS_INTERRUPT 0x00000001
#define PHY_IS_INTERNAL 0x00000002
+#define PHY_RST_AFTER_CLK_EN 0x00000004
#define MDIO_DEVICE_IS_PHY 0x80000000
/* Interface Mode definitions */
@@ -853,6 +854,7 @@ int phy_aneg_done(struct phy_device *phydev);
int phy_stop_interrupts(struct phy_device *phydev);
int phy_restart_aneg(struct phy_device *phydev);
+int phy_reset_after_clk_enable(struct phy_device *phydev);
static inline void phy_device_reset(struct phy_device *phydev, int value)
{
--
2.11.0
^ permalink raw reply related
* [PATCH net-next v4 1/4] phylib: Add device reset delay support
From: Richard Leitner @ 2017-12-07 14:43 UTC (permalink / raw)
To: robh+dt, mark.rutland, fugang.duan, andrew, f.fainelli,
frowand.list
Cc: davem, geert+renesas, sergei.shtylyov, baruch, david.wu, lukma,
netdev, devicetree, linux-kernel, richard.leitner
In-Reply-To: <20171207144358.3351-1-dev@g0hl1n.net>
From: Richard Leitner <richard.leitner@skidata.com>
Some PHYs need a minimum time after the reset gpio was asserted and/or
deasserted. To ensure we meet these timing requirements add two new
optional devicetree parameters for the phy: reset-delay-us and
reset-post-delay-us.
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Documentation/devicetree/bindings/net/phy.txt | 10 ++++++++++
drivers/net/phy/mdio_device.c | 13 +++++++++++--
drivers/of/of_mdio.c | 4 ++++
include/linux/mdio.h | 2 ++
4 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index c05479f5ac7c..72860ce7f610 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -55,6 +55,12 @@ Optional Properties:
- reset-gpios: The GPIO phandle and specifier for the PHY reset signal.
+- reset-delay-us: Delay after the reset was asserted in microseconds.
+ If this property is missing the delay will be skipped.
+
+- reset-post-delay-us: Delay after the reset was deasserted in microseconds.
+ If this property is missing the delay will be skipped.
+
Example:
ethernet-phy@0 {
@@ -62,4 +68,8 @@ ethernet-phy@0 {
interrupt-parent = <&PIC>;
interrupts = <35 IRQ_TYPE_EDGE_RISING>;
reg = <0>;
+
+ reset-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;
+ reset-delay-us = <1000>;
+ reset-post-delay-us = <2000>;
};
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index 75d97dd9fb28..0423280c88fe 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -24,6 +24,7 @@
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/unistd.h>
+#include <linux/delay.h>
void mdio_device_free(struct mdio_device *mdiodev)
{
@@ -118,8 +119,16 @@ EXPORT_SYMBOL(mdio_device_remove);
void mdio_device_reset(struct mdio_device *mdiodev, int value)
{
- if (mdiodev->reset)
- gpiod_set_value(mdiodev->reset, value);
+ unsigned int d;
+
+ if (!mdiodev->reset)
+ return;
+
+ gpiod_set_value(mdiodev->reset, value);
+
+ d = value ? mdiodev->reset_delay : mdiodev->reset_post_delay;
+ if (d)
+ usleep_range(d, d + min_t(unsigned int, d / 10, 100));
}
EXPORT_SYMBOL(mdio_device_reset);
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 98258583abb0..7c8767176315 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -77,6 +77,10 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
if (of_property_read_bool(child, "broken-turn-around"))
mdio->phy_ignore_ta_mask |= 1 << addr;
+ of_property_read_u32(child, "reset-delay-us", &phy->mdio.reset_delay);
+ of_property_read_u32(child, "reset-post-delay-us",
+ &phy->mdio.reset_post_delay);
+
/* Associate the OF node with the device structure so it
* can be looked up later */
of_node_get(child);
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 92d4e55ffe67..e37c21d8eb19 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -41,6 +41,8 @@ struct mdio_device {
int addr;
int flags;
struct gpio_desc *reset;
+ unsigned int reset_delay;
+ unsigned int reset_post_delay;
};
#define to_mdio_device(d) container_of(d, struct mdio_device, dev)
--
2.11.0
^ permalink raw reply related
* [PATCH net-next v4 0/4] net: fec: fix refclk enable for SMSC LAN8710/20
From: Richard Leitner @ 2017-12-07 14:43 UTC (permalink / raw)
To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
fugang.duan-3arQi8VN3Tc, andrew-g2DYL2Zd6BY,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8,
baruch-NswTu9S1W3P6gbPvEgmw2w, david.wu-TNX95d0MmH7DzftRWevZcw,
lukma-ynQEQJNshbs, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
richard.leitner-WcANXNA0UjBBDgjK7y7TUQ
From: Richard Leitner <richard.leitner-WcANXNA0UjBBDgjK7y7TUQ@public.gmane.org>
This patch series fixes the use of the SMSC LAN8710/20 with a Freescale ETH
when the refclk is generated by the FSL.
This patchset depends on the "phylib: Add device reset GPIO support" patch
submitted by Geert Uytterhoeven/Sergei Shtylyov, which was merged to
net-next as commit bafbdd527d56 ("phylib: Add device reset GPIO support").
Changes v4:
- simplify dts parsing
- simplify reset delay evaluation and execution
- fec: ensure to only reset once during fec_enet_open()
- remove dependency notes from commit message
- add reviews and acks
Changes v3:
- use phylib to hard-reset the PHY
- implement reset delays in phylib
- add new phylib API & flag (PHY_RST_AFTER_CLK_EN) to determine if
a PHY is affected
Changes v2:
- simplify and fix fec_reset_phy function to support multiple calls
- include: linux: phy: harmonize phy_id{,_mask} type
- reset the phy instead of not turning the clock on and off
(which would have caused a power consumption regression)
Richard Leitner (4):
phylib: Add device reset delay support
phylib: add reset after clk enable support
net: phy: smsc: LAN8710/20: add PHY_RST_AFTER_CLK_EN flag
net: fec: add phy_reset_after_clk_enable() support
Documentation/devicetree/bindings/net/phy.txt | 10 ++++++++++
drivers/net/ethernet/freescale/fec_main.c | 20 ++++++++++++++++++++
drivers/net/phy/mdio_device.c | 13 +++++++++++--
drivers/net/phy/phy_device.c | 24 ++++++++++++++++++++++++
drivers/net/phy/smsc.c | 2 +-
drivers/of/of_mdio.c | 4 ++++
include/linux/mdio.h | 2 ++
include/linux/phy.h | 2 ++
8 files changed, 74 insertions(+), 3 deletions(-)
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [crypto 6/8] chtls: TCB and Key program
From: Stephan Mueller @ 2017-12-07 14:42 UTC (permalink / raw)
To: Atul Gupta
Cc: herbert@gondor.apana.org.au, linux-crypto@vger.kernel.org,
netdev@vger.kernel.org, davem@davemloft.net, davejwatson@fb.com,
Ganesh GR, Harsh Jain
In-Reply-To: <MWHPR1201MB02382E04C3603A5B5F531CB897330@MWHPR1201MB0238.namprd12.prod.outlook.com>
Am Donnerstag, 7. Dezember 2017, 15:21:03 CET schrieb Atul Gupta:
Hi Atul,
>
> memzero_explicit(key)?
> [Atul] may not be required as entire info of size keylen and AEAD_H_SIZE is
> copied onto kctx->key. Key data is received from user, while ghash is
> memset and locally generated
Sure, but wouldn't it make sense to zap all instances where key material was
stored?
>
> As far as I see, the key is part of the skb (via kctx). This skb is released
> after being processed. The release calls kfree_skb which does not zeroize
> the key. Wouldn't it make sense to clear the memory of the key when the skb
> is released? [Atul] we should perhaps memset the info received from user so
> that driver has no info on key once its written on chip memory.
> memset(gcm_ctx->key, 0, keylen);
Are you saying that the skb (via kctx) above does not obtain a copy of the
key? If not, what is done in chtls_key_info?
Ciao
Stephan
^ permalink raw reply
* Re: [2/3] ath9k: move spectral scan support under a separate config symbol
From: Kalle Valo @ 2017-12-07 14:32 UTC (permalink / raw)
To: Matthias Schiffer
Cc: ath9k-devel, ath10k, linux-wireless, netdev, linux-kernel, nbd
In-Reply-To: <1c982c915065a9b5ed3232eedb567f589eca261b.1511804622.git.mschiffer@universe-factory.net>
Matthias Schiffer <mschiffer@universe-factory.net> wrote:
> At the moment, spectral scan support, and with it RELAY, is always enabled
> with ATH9K[_HTC]_DEBUGFS. Spectral scan support is currently the only user
> of RELAY in ath9k, and it unconditionally reserves a relay channel.
>
> Having debugfs support in ath9k is often useful even on very small embedded
> routers, where we'd rather like to avoid the code size and RAM usage of the
> relay support.
>
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
2 patches applied to ath-next branch of ath.git, thanks.
9df7ddc3ed25 ath9k: move spectral scan support under a separate config symbol
42e01cb9cb10 ath10k: move spectral scan support under a separate config symbol
--
https://patchwork.kernel.org/patch/10077735/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/3] ath9k: remove stray backslash in Makefile
From: Kalle Valo @ 2017-12-07 14:30 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Matthias Schiffer, ath9k-devel, ath10k@lists.infradead.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, nbd@nbd.name
In-Reply-To: <20171127193516.682827ea@cakuba.netronome.com>
Jakub Kicinski <kubakici@wp.pl> writes:
> On Mon, 27 Nov 2017 18:56:21 +0100, Matthias Schiffer wrote:
>> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
>> ---
>> drivers/net/wireless/ath/ath9k/Makefile | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/Makefile b/drivers/net/wireless/ath/ath9k/Makefile
>> index 36a40ffdce15..90e4a341076c 100644
>> --- a/drivers/net/wireless/ath/ath9k/Makefile
>> +++ b/drivers/net/wireless/ath/ath9k/Makefile
>> @@ -59,7 +59,7 @@ obj-$(CONFIG_ATH9K_HW) += ath9k_hw.o
>> obj-$(CONFIG_ATH9K_COMMON) += ath9k_common.o
>> ath9k_common-y:= common.o \
>> common-init.o \
>> - common-beacon.o \
>> + common-beacon.o
>>
>
> It's not necessarily stray, there is nothing on the next line so it's
> OK, and if you add \ at the end of all lines, you don't have to touch
> the last line every time you add/remove something. Sort of like
> putting a , after last enum value.
I agree with Jakub, I think the backslash is there on purpose so I
dropped this patch.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v4 6/8] staging: octeon: Remove USE_ASYNC_IOBDMA macro.
From: Greg Kroah-Hartman @ 2017-12-07 14:28 UTC (permalink / raw)
To: David Daney
Cc: Mark Rutland, linux-mips, devel, devicetree, netdev, linux-kernel,
ralf, Rob Herring, Andrew Lunn, Steven J. Hill, Florian Fainelli,
James Hogan, David S. Miller
In-Reply-To: <20171129005540.28829-7-david.daney@cavium.com>
On Tue, Nov 28, 2017 at 04:55:38PM -0800, David Daney wrote:
> Previous patch sets USE_ASYNC_IOBDMA to 1 unconditionally. Remove
> USE_ASYNC_IOBDMA from all if statements. Remove dead code caused by
> the change.
>
> Signed-off-by: David Daney <david.daney@cavium.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* [PATCH net-next v2 8/8] net: phy: meson-gxl: join the authors
From: Jerome Brunet @ 2017-12-07 14:27 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: Jerome Brunet, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-1-jbrunet@baylibre.com>
Following previous changes, join the other authors of this driver and
take the blame with them
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/net/phy/meson-gxl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 861b021b9758..4cd5b2622ae1 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -256,4 +256,5 @@ MODULE_DEVICE_TABLE(mdio, meson_gxl_tbl);
MODULE_DESCRIPTION("Amlogic Meson GXL Internal PHY driver");
MODULE_AUTHOR("Baoqi wang");
MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
MODULE_LICENSE("GPL");
--
2.14.3
^ permalink raw reply related
* [PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support
From: Jerome Brunet @ 2017-12-07 14:27 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: Jerome Brunet, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-1-jbrunet@baylibre.com>
Enable interrupt support in meson-gxl PHY driver
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/net/phy/meson-gxl.c | 37 ++++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 5325940fe899..861b021b9758 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -33,6 +33,14 @@
#define TSTCNTL_WRITE_ADDRESS GENMASK(4, 0)
#define TSTREAD1 21
#define TSTWRITE 23
+#define INTSRC_FLAG 29
+#define INTSRC_ANEG_PR BIT(1)
+#define INTSRC_PARALLEL_FAULT BIT(2)
+#define INTSRC_ANEG_LP_ACK BIT(3)
+#define INTSRC_LINK_DOWN BIT(4)
+#define INTSRC_REMOTE_FAULT BIT(5)
+#define INTSRC_ANEG_COMPLETE BIT(6)
+#define INTSRC_MASK 30
#define BANK_ANALOG_DSP 0
#define BANK_WOL 1
@@ -193,17 +201,44 @@ int meson_gxl_read_status(struct phy_device *phydev)
return genphy_read_status(phydev);
}
+static int meson_gxl_ack_interrupt(struct phy_device *phydev)
+{
+ int ret = phy_read(phydev, INTSRC_FLAG);
+
+ return ret < 0 ? ret : 0;
+}
+
+static int meson_gxl_config_intr(struct phy_device *phydev)
+{
+ u16 val;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ val = INTSRC_ANEG_PR
+ | INTSRC_PARALLEL_FAULT
+ | INTSRC_ANEG_LP_ACK
+ | INTSRC_LINK_DOWN
+ | INTSRC_REMOTE_FAULT
+ | INTSRC_ANEG_COMPLETE;
+ } else {
+ val = 0;
+ }
+
+ return phy_write(phydev, INTSRC_MASK, val);
+}
+
static struct phy_driver meson_gxl_phy[] = {
{
.phy_id = 0x01814400,
.phy_id_mask = 0xfffffff0,
.name = "Meson GXL Internal PHY",
.features = PHY_BASIC_FEATURES,
- .flags = PHY_IS_INTERNAL,
+ .flags = PHY_IS_INTERNAL | PHY_HAS_INTERRUPT,
.config_init = meson_gxl_config_init,
.config_aneg = genphy_config_aneg,
.aneg_done = genphy_aneg_done,
.read_status = meson_gxl_read_status,
+ .ack_interrupt = meson_gxl_ack_interrupt,
+ .config_intr = meson_gxl_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
},
--
2.14.3
^ permalink raw reply related
* [PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched
From: Jerome Brunet @ 2017-12-07 14:27 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: Jerome Brunet, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-1-jbrunet@baylibre.com>
The PHY performs just as well when left in its default configuration and
it makes senses because this poke gets reset just after init.
According to the documentation, all registers in the Analog/DSP bank are
reset when there is a mode switch from 10BT to 100BT.
In the end, we have used the default configuration so far and there is no
reason to change now. Remove CONFIG_A6 poke to make this clear.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
Out of curiosity, I tried to re-apply the ANALOG/DSP settings on speed
changes (patch available here [0] if someone wants to try) but I did
not notice any change as a result. In the end, I thought it was safer
to keep on using the ANALOG settings we have been actually using so far,
everybody seems to be happy with them
[0]: https://github.com/jeromebrunet/linux/commit/b594288e629a61574e76112497474fd3cf46c781
drivers/net/phy/meson-gxl.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 726e0eeed475..5325940fe899 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -38,9 +38,6 @@
#define BANK_WOL 1
#define BANK_BIST 3
-/* Analog/DSP Registers */
-#define A6_CONFIG_REG 0x17
-
/* WOL Registers */
#define LPI_STATUS 0xc
#define LPI_STATUS_RSV12 BIT(12)
@@ -126,12 +123,6 @@ static int meson_gxl_config_init(struct phy_device *phydev)
{
int ret;
- /* Write CONFIG_A6*/
- ret = meson_gxl_write_reg(phydev, BANK_ANALOG_DSP, A6_CONFIG_REG,
- 0x8e0d);
- if (ret)
- return ret;
-
/* Enable fractional PLL */
ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_CONTROL, 0x5);
if (ret)
--
2.14.3
^ permalink raw reply related
* [PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption
From: Jerome Brunet @ 2017-12-07 14:27 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: Jerome Brunet, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-1-jbrunet@baylibre.com>
The purpose of this change is to fix the incorrect detection of the link
partner (LP) advertised capabilities which sometimes happens with this PHY
(roughly 1 time in a dozen)
This issue may cause the link to be negotiated at 10Mbps/Full or
10Mbps/Half when 100MBps/Full is actually possible. In some case, the link
is even completely broken and no communication is possible.
To detect the corruption, we must look for a magic undocumented bit in the
WOL bank (hint given by the SoC vendor kernel) but this is not enough to
cover all cases. We also have to look at the LPA ack. If the LP supports
Aneg but did not ack our base code when aneg is completed, we assume
something went wrong.
The detection of a corrupted LPA triggers a restart of the aneg process.
This solves the problem but may take up to 6 retries to complete.
Fixes: 7334b3e47aee ("net: phy: Add Meson GXL Internal PHY driver")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
I suppose this patch probably seems a bit hacky, especially the part
about the link partner acknowledge. I'm trying to figure out if the
value in MII_LPA makes sense but I don't have such a deep knowledge
of the ethernet spec.
To me, it does not makes sense for the LP to support ANEG (Bit 1 in
MII_EXPENSION), the aneg to have successfully complete and, at the
same time, LP does not ACK our base code word, which we should have
sent during this aneg.
If you think this may have unintended consequences or if you have
an idea to this differently, feel free to let me know.
drivers/net/phy/meson-gxl.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 58 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 2e8c40df33c2..726e0eeed475 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -35,11 +35,16 @@
#define TSTWRITE 23
#define BANK_ANALOG_DSP 0
+#define BANK_WOL 1
#define BANK_BIST 3
/* Analog/DSP Registers */
#define A6_CONFIG_REG 0x17
+/* WOL Registers */
+#define LPI_STATUS 0xc
+#define LPI_STATUS_RSV12 BIT(12)
+
/* BIST Registers */
#define FR_PLL_CONTROL 0x1b
#define FR_PLL_DIV0 0x1c
@@ -145,6 +150,58 @@ static int meson_gxl_config_init(struct phy_device *phydev)
return genphy_config_init(phydev);
}
+/* This specific function is provided to cope with the possible failures of
+ * this phy during aneg process. When aneg fails, the PHY reports that aneg
+ * is done but the value found in MII_LPA is wrong:
+ * - Early failures: MII_LPA is just 0x0001. if MII_EXPANSION reports that
+ * the link partner (LP) supports aneg but the LP never acked our base
+ * code word, it is likely that we never sent it to begin with.
+ * - Late failures: MII_LPA is filled with a value which seems to make sense
+ * but it actually is not what the LP is advertising. It seems that we
+ * can detect this using a magic bit in the WOL bank (reg 12 - bit 12).
+ * If this particular bit is not set when aneg is reported being done,
+ * it means MII_LPA is likely to be wrong.
+ *
+ * In both case, forcing a restart of the aneg process solve the problem.
+ * When this failure happens, the first retry is usually successful but,
+ * in some cases, it may take up to 6 retries to get a decent result
+ */
+int meson_gxl_read_status(struct phy_device *phydev)
+{
+ int ret, wol, lpa, exp;
+
+ if (phydev->autoneg == AUTONEG_ENABLE) {
+ ret = genphy_aneg_done(phydev);
+ if (ret < 0)
+ return ret;
+ else if (!ret)
+ goto read_status_continue;
+
+ /* Aneg is done, let's check everything is fine */
+ wol = meson_gxl_read_reg(phydev, BANK_WOL, LPI_STATUS);
+ if (wol < 0)
+ return wol;
+
+ lpa = phy_read(phydev, MII_LPA);
+ if (lpa < 0)
+ return lpa;
+
+ exp = phy_read(phydev, MII_EXPANSION);
+ if (exp < 0)
+ return exp;
+
+ if (!(wol & LPI_STATUS_RSV12) ||
+ ((exp & EXPANSION_NWAY) && !(lpa & LPA_LPACK))) {
+ /* Looks like aneg failed after all */
+ phydev_dbg(phydev, "LPA corruption - aneg restart\n");
+ return genphy_restart_aneg(phydev);
+ }
+ }
+
+read_status_continue:
+ return genphy_read_status(phydev);
+}
+
static struct phy_driver meson_gxl_phy[] = {
{
.phy_id = 0x01814400,
@@ -155,7 +212,7 @@ static struct phy_driver meson_gxl_phy[] = {
.config_init = meson_gxl_config_init,
.config_aneg = genphy_config_aneg,
.aneg_done = genphy_aneg_done,
- .read_status = genphy_read_status,
+ .read_status = meson_gxl_read_status,
.suspend = genphy_suspend,
.resume = genphy_resume,
},
--
2.14.3
^ permalink raw reply related
* [PATCH net-next v2 4/8] net: phy: meson-gxl: use genphy_config_init
From: Jerome Brunet @ 2017-12-07 14:27 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: Kevin Hilman, linux-kernel, netdev, linux-amlogic,
linux-arm-kernel, Jerome Brunet
In-Reply-To: <20171207142715.32578-1-jbrunet@baylibre.com>
Use the generic init function to populate some of the phydev
structure fields
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/net/phy/meson-gxl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 05054770aefb..2e8c40df33c2 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -142,7 +142,7 @@ static int meson_gxl_config_init(struct phy_device *phydev)
if (ret)
return ret;
- return 0;
+ return genphy_config_init(phydev);
}
static struct phy_driver meson_gxl_phy[] = {
--
2.14.3
^ permalink raw reply related
* [PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers
From: Jerome Brunet @ 2017-12-07 14:27 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: Jerome Brunet, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-1-jbrunet@baylibre.com>
Add read and write helpers to manipulate banked registers on this PHY
This helps clarify the settings applied to these registers in the init
function and upcoming changes.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
1 file changed, 67 insertions(+), 36 deletions(-)
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index d82aa8cea401..05054770aefb 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -45,11 +45,13 @@
#define FR_PLL_DIV0 0x1c
#define FR_PLL_DIV1 0x1d
-static int meson_gxl_config_init(struct phy_device *phydev)
+static int meson_gxl_open_banks(struct phy_device *phydev)
{
int ret;
- /* Enable Analog and DSP register Bank access by */
+ /* Enable Analog and DSP register Bank access by
+ * toggling TSTCNTL_TEST_MODE bit in the TSTCNTL register
+ */
ret = phy_write(phydev, TSTCNTL, 0);
if (ret)
return ret;
@@ -59,55 +61,84 @@ static int meson_gxl_config_init(struct phy_device *phydev)
ret = phy_write(phydev, TSTCNTL, 0);
if (ret)
return ret;
- ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
- if (ret)
- return ret;
+ return phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
+}
- /* Write CONFIG_A6*/
- ret = phy_write(phydev, TSTWRITE, 0x8e0d)
+static void meson_gxl_close_banks(struct phy_device *phydev)
+{
+ phy_write(phydev, TSTCNTL, 0);
+}
+
+static int meson_gxl_read_reg(struct phy_device *phydev,
+ unsigned int bank, unsigned int reg)
+{
+ int ret;
+
+ ret = meson_gxl_open_banks(phydev);
if (ret)
- return ret;
- ret = phy_write(phydev, TSTCNTL,
- TSTCNTL_WRITE
- | FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_ANALOG_DSP)
- | TSTCNTL_TEST_MODE
- | FIELD_PREP(TSTCNTL_WRITE_ADDRESS, A6_CONFIG_REG));
+ goto out;
+
+ ret = phy_write(phydev, TSTCNTL, TSTCNTL_READ |
+ FIELD_PREP(TSTCNTL_REG_BANK_SEL, bank) |
+ TSTCNTL_TEST_MODE |
+ FIELD_PREP(TSTCNTL_READ_ADDRESS, reg));
if (ret)
- return ret;
+ goto out;
- /* Enable fractional PLL */
- ret = phy_write(phydev, TSTWRITE, 0x0005);
+ ret = phy_read(phydev, TSTREAD1);
+out:
+ /* Close the bank access on our way out */
+ meson_gxl_close_banks(phydev);
+ return ret;
+}
+
+static int meson_gxl_write_reg(struct phy_device *phydev,
+ unsigned int bank, unsigned int reg,
+ uint16_t value)
+{
+ int ret;
+
+ ret = meson_gxl_open_banks(phydev);
if (ret)
- return ret;
- ret = phy_write(phydev, TSTCNTL,
- TSTCNTL_WRITE
- | FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
- | TSTCNTL_TEST_MODE
- | FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_CONTROL));
+ goto out;
+
+ ret = phy_write(phydev, TSTWRITE, value);
if (ret)
- return ret;
+ goto out;
- /* Program fraction FR_PLL_DIV1 */
- ret = phy_write(phydev, TSTWRITE, 0x029a);
+ ret = phy_write(phydev, TSTCNTL, TSTCNTL_WRITE |
+ FIELD_PREP(TSTCNTL_REG_BANK_SEL, bank) |
+ TSTCNTL_TEST_MODE |
+ FIELD_PREP(TSTCNTL_WRITE_ADDRESS, reg));
+
+out:
+ /* Close the bank access on our way out */
+ meson_gxl_close_banks(phydev);
+ return ret;
+}
+
+static int meson_gxl_config_init(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Write CONFIG_A6*/
+ ret = meson_gxl_write_reg(phydev, BANK_ANALOG_DSP, A6_CONFIG_REG,
+ 0x8e0d);
if (ret)
return ret;
- ret = phy_write(phydev, TSTCNTL,
- TSTCNTL_WRITE
- | FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
- | TSTCNTL_TEST_MODE
- | FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV1));
+
+ /* Enable fractional PLL */
+ ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_CONTROL, 0x5);
if (ret)
return ret;
/* Program fraction FR_PLL_DIV1 */
- ret = phy_write(phydev, TSTWRITE, 0xaaaa);
+ ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_DIV1, 0x029a);
if (ret)
return ret;
- ret = phy_write(phydev, TSTCNTL,
- TSTCNTL_WRITE
- | FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
- | TSTCNTL_TEST_MODE
- | FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV0));
+
+ /* Program fraction FR_PLL_DIV1 */
+ ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_DIV0, 0xaaaa);
if (ret)
return ret;
--
2.14.3
^ permalink raw reply related
* [PATCH net-next v2 2/8] net: phy: meson-gxl: define control registers
From: Jerome Brunet @ 2017-12-07 14:27 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: Jerome Brunet, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel, Neil Armstrong
In-Reply-To: <20171207142715.32578-1-jbrunet@baylibre.com>
Define registers and bits in meson-gxl PHY driver to make a bit
more human friendly. No functional change.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/net/phy/meson-gxl.c | 64 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 51 insertions(+), 13 deletions(-)
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 7ddb709f69fc..d82aa8cea401 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -22,54 +22,92 @@
#include <linux/ethtool.h>
#include <linux/phy.h>
#include <linux/netdevice.h>
+#include <linux/bitfield.h>
+
+#define TSTCNTL 20
+#define TSTCNTL_READ BIT(15)
+#define TSTCNTL_WRITE BIT(14)
+#define TSTCNTL_REG_BANK_SEL GENMASK(12, 11)
+#define TSTCNTL_TEST_MODE BIT(10)
+#define TSTCNTL_READ_ADDRESS GENMASK(9, 5)
+#define TSTCNTL_WRITE_ADDRESS GENMASK(4, 0)
+#define TSTREAD1 21
+#define TSTWRITE 23
+
+#define BANK_ANALOG_DSP 0
+#define BANK_BIST 3
+
+/* Analog/DSP Registers */
+#define A6_CONFIG_REG 0x17
+
+/* BIST Registers */
+#define FR_PLL_CONTROL 0x1b
+#define FR_PLL_DIV0 0x1c
+#define FR_PLL_DIV1 0x1d
static int meson_gxl_config_init(struct phy_device *phydev)
{
int ret;
/* Enable Analog and DSP register Bank access by */
- ret = phy_write(phydev, 0x14, 0x0000);
+ ret = phy_write(phydev, TSTCNTL, 0);
if (ret)
return ret;
- ret = phy_write(phydev, 0x14, 0x0400);
+ ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
if (ret)
return ret;
- ret = phy_write(phydev, 0x14, 0x0000);
+ ret = phy_write(phydev, TSTCNTL, 0);
if (ret)
return ret;
- ret = phy_write(phydev, 0x14, 0x0400);
+ ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
if (ret)
return ret;
- /* Write Analog register 23 */
- ret = phy_write(phydev, 0x17, 0x8E0D);
+ /* Write CONFIG_A6*/
+ ret = phy_write(phydev, TSTWRITE, 0x8e0d)
if (ret)
return ret;
- ret = phy_write(phydev, 0x14, 0x4417);
+ ret = phy_write(phydev, TSTCNTL,
+ TSTCNTL_WRITE
+ | FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_ANALOG_DSP)
+ | TSTCNTL_TEST_MODE
+ | FIELD_PREP(TSTCNTL_WRITE_ADDRESS, A6_CONFIG_REG));
if (ret)
return ret;
/* Enable fractional PLL */
- ret = phy_write(phydev, 0x17, 0x0005);
+ ret = phy_write(phydev, TSTWRITE, 0x0005);
if (ret)
return ret;
- ret = phy_write(phydev, 0x14, 0x5C1B);
+ ret = phy_write(phydev, TSTCNTL,
+ TSTCNTL_WRITE
+ | FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+ | TSTCNTL_TEST_MODE
+ | FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_CONTROL));
if (ret)
return ret;
/* Program fraction FR_PLL_DIV1 */
- ret = phy_write(phydev, 0x17, 0x029A);
+ ret = phy_write(phydev, TSTWRITE, 0x029a);
if (ret)
return ret;
- ret = phy_write(phydev, 0x14, 0x5C1D);
+ ret = phy_write(phydev, TSTCNTL,
+ TSTCNTL_WRITE
+ | FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+ | TSTCNTL_TEST_MODE
+ | FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV1));
if (ret)
return ret;
/* Program fraction FR_PLL_DIV1 */
- ret = phy_write(phydev, 0x17, 0xAAAA);
+ ret = phy_write(phydev, TSTWRITE, 0xaaaa);
if (ret)
return ret;
- ret = phy_write(phydev, 0x14, 0x5C1C);
+ ret = phy_write(phydev, TSTCNTL,
+ TSTCNTL_WRITE
+ | FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+ | TSTCNTL_TEST_MODE
+ | FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV0));
if (ret)
return ret;
--
2.14.3
^ permalink raw reply related
* [PATCH net-next v2 1/8] net: phy: meson-gxl: check phy_write return value
From: Jerome Brunet @ 2017-12-07 14:27 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: Jerome Brunet, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-1-jbrunet@baylibre.com>
Always check phy_write return values. Better to be safe than sorry
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/net/phy/meson-gxl.c | 50 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 38 insertions(+), 12 deletions(-)
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 1ea69b7585d9..7ddb709f69fc 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -25,27 +25,53 @@
static int meson_gxl_config_init(struct phy_device *phydev)
{
+ int ret;
+
/* Enable Analog and DSP register Bank access by */
- phy_write(phydev, 0x14, 0x0000);
- phy_write(phydev, 0x14, 0x0400);
- phy_write(phydev, 0x14, 0x0000);
- phy_write(phydev, 0x14, 0x0400);
+ ret = phy_write(phydev, 0x14, 0x0000);
+ if (ret)
+ return ret;
+ ret = phy_write(phydev, 0x14, 0x0400);
+ if (ret)
+ return ret;
+ ret = phy_write(phydev, 0x14, 0x0000);
+ if (ret)
+ return ret;
+ ret = phy_write(phydev, 0x14, 0x0400);
+ if (ret)
+ return ret;
/* Write Analog register 23 */
- phy_write(phydev, 0x17, 0x8E0D);
- phy_write(phydev, 0x14, 0x4417);
+ ret = phy_write(phydev, 0x17, 0x8E0D);
+ if (ret)
+ return ret;
+ ret = phy_write(phydev, 0x14, 0x4417);
+ if (ret)
+ return ret;
/* Enable fractional PLL */
- phy_write(phydev, 0x17, 0x0005);
- phy_write(phydev, 0x14, 0x5C1B);
+ ret = phy_write(phydev, 0x17, 0x0005);
+ if (ret)
+ return ret;
+ ret = phy_write(phydev, 0x14, 0x5C1B);
+ if (ret)
+ return ret;
/* Program fraction FR_PLL_DIV1 */
- phy_write(phydev, 0x17, 0x029A);
- phy_write(phydev, 0x14, 0x5C1D);
+ ret = phy_write(phydev, 0x17, 0x029A);
+ if (ret)
+ return ret;
+ ret = phy_write(phydev, 0x14, 0x5C1D);
+ if (ret)
+ return ret;
/* Program fraction FR_PLL_DIV1 */
- phy_write(phydev, 0x17, 0xAAAA);
- phy_write(phydev, 0x14, 0x5C1C);
+ ret = phy_write(phydev, 0x17, 0xAAAA);
+ if (ret)
+ return ret;
+ ret = phy_write(phydev, 0x14, 0x5C1C);
+ if (ret)
+ return ret;
return 0;
}
--
2.14.3
^ permalink raw reply related
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