* Re: [PATCH net-next 0/4] igb: auxiliary PHC functions for the i210.
From: Jeff Kirsher @ 2014-11-19 8:43 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, David Miller, bruce.w.allan, Jacob Keller, John Ronciak,
Matthew Vick
In-Reply-To: <20141119063103.GA4109@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 396 bytes --]
On Wed, 2014-11-19 at 07:31 +0100, Richard Cochran wrote:
> On Mon, Nov 17, 2014 at 03:15:18PM -0800, Jeff Kirsher wrote:
> > Thanks Richard, I have added your series of igb patches to my queue.
>
> Jeff, please hold off on these. I found a bug in the PPS code. I will
> re-submit with a fix.
Ok, I will drop the series that I currently have in my queue and will
await your v2 series.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: net: xfrm: Deletion of an unnecessary check before the function call "ipcomp_free_tfms"
From: SF Markus Elfring @ 2014-11-19 9:51 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Herbert Xu, Steffen Klassert, netdev, LKML,
kernel-janitors, Julia Lawall
In-Reply-To: <20141119084542.GK4905@mwanda>
>> The ipcomp_free_tfms() function tests whether its argument is NULL and then
>> returns immediately. Thus the test around the call is not needed.
>
> It doesn't though...
You are right that this function implementation does a bit more before
returning because of a detected null pointer.
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/net/xfrm/xfrm_ipcomp.c?id=394efd19d5fcae936261bd48e5b33b21897aacf8#n247
Can you agree that input parameter validation is also performed there?
Do you want that I resend my patch with a corrected commit message?
Regards,
Markus
^ permalink raw reply
* Re: [net-next 12/15] i40evf: make checkpatch happy
From: Jeff Kirsher @ 2014-11-19 9:55 UTC (permalink / raw)
To: Joe Perches; +Cc: davem, Mitch Williams, netdev, nhorman, sassmann, jogreene
In-Reply-To: <1416373185.6651.12.camel@perches.com>
[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]
On Tue, 2014-11-18 at 20:59 -0800, Joe Perches wrote:
> On Tue, 2014-11-18 at 20:10 -0800, Jeff Kirsher wrote:
> > This patch is the result of running checkpatch on the i40evf driver with
> > the --strict option. The vast majority of changes are adding/removing
> > blank lines, aligning function parameters, and correcting over-long
> > lines.
>
> Hey Mitch, Jeff:
>
> > diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
> []
> > @@ -1265,8 +1269,8 @@ int i40evf_init_interrupt_scheme(struct i40evf_adapter *adapter)
> > }
> >
> > dev_info(&adapter->pdev->dev, "Multiqueue %s: Queue pair count = %u",
> > - (adapter->num_active_queues > 1) ? "Enabled" :
> > - "Disabled", adapter->num_active_queues);
> > + (adapter->num_active_queues > 1) ? "Enabled" : "Disabled",
> > + adapter->num_active_queues);
>
> You could add a newline to that format one day.
>
> > diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
> []
> > @@ -711,7 +712,6 @@ void i40evf_virtchnl_completion(struct i40evf_adapter *adapter,
> > "%s: Unknown event %d from pf\n",
> > __func__, vpe->event);
> > break;
> > -
> > }
> > return;
> > }
> > @@ -776,7 +776,7 @@ void i40evf_virtchnl_completion(struct i40evf_adapter *adapter,
> > break;
> > default:
> > dev_warn(&adapter->pdev->dev, "%s: Received unexpected message %d from PF\n",
> > - __func__, v_opcode);
> > + __func__, v_opcode);
> > break;
> > } /* switch v_opcode */
> > adapter->current_op = I40E_VIRTCHNL_OP_UNKNOWN;
>
> And be consistent with PF vs pf usage too.
Thanks for noticing, I had not caught that there was inconsistent use.
I try to catch acronyms that are not capitalized, and after looking
through the code, I see that there is inconsistent use of VF vs vf as
well. I am putting together a patch now that will make the use of both
acronyms consistent throughout our code.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: net: xfrm: Deletion of an unnecessary check before the function call "ipcomp_free_tfms"
From: Julia Lawall @ 2014-11-19 9:58 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Dan Carpenter, David S. Miller, Herbert Xu, Steffen Klassert,
netdev, LKML, kernel-janitors
In-Reply-To: <546C6823.50900@users.sourceforge.net>
On Wed, 19 Nov 2014, SF Markus Elfring wrote:
> >> The ipcomp_free_tfms() function tests whether its argument is NULL and then
> >> returns immediately. Thus the test around the call is not needed.
> >
> > It doesn't though...
>
> You are right that this function implementation does a bit more before
> returning because of a detected null pointer.
> https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/net/xfrm/xfrm_ipcomp.c?id=394efd19d5fcae936261bd48e5b33b21897aacf8#n247
>
> Can you agree that input parameter validation is also performed there?
> Do you want that I resend my patch with a corrected commit message?
This is completely crazy. The function performs a side effect on a data
structure. If the call site doesn't want that done in a certain case,
then it should not be done.
julia
^ permalink raw reply
* [PATCH net-next] net: use skb_get_hash_raw instead of skb_get_hash in set_rps_cpu
From: Govindarajulu Varadarajan @ 2014-11-19 10:00 UTC (permalink / raw)
To: davem, netdev; +Cc: Govindarajulu Varadarajan
In set_rps_cpu, we call skb_get_hash to get the hash of skb. The caller
get_rps_cpu has already determined the hash of skb by calling skb_get_hash.
Since get_rps_cpu is the only caller of set_rps_cpu, we can use skb_get_hash_raw
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 1ab168e..b4ad4d1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3055,7 +3055,7 @@ set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
flow_table = rcu_dereference(rxqueue->rps_flow_table);
if (!flow_table)
goto out;
- flow_id = skb_get_hash(skb) & flow_table->mask;
+ flow_id = skb_get_hash_raw(skb) & flow_table->mask;
rc = dev->netdev_ops->ndo_rx_flow_steer(dev, skb,
rxq_index, flow_id);
if (rc < 0)
--
2.1.0
^ permalink raw reply related
* Re: [patch net-next v3 8/9] net: move vlan pop/push functions into common code
From: Jiri Pirko @ 2014-11-19 10:03 UTC (permalink / raw)
To: Pravin Shelar
Cc: netdev, David Miller, Jamal Hadi Salim, Tom Herbert, Eric Dumazet,
Willem de Bruijn, Daniel Borkmann, mst, fw, Paul.Durrant,
Thomas Graf, Cong Wang
In-Reply-To: <CALnjE+rJEtyL-QDrp0x+N_pCxDuR6soHkaY-OrswWrdZwfE2sQ@mail.gmail.com>
Wed, Nov 19, 2014 at 09:06:19AM CET, pshelar@nicira.com wrote:
>On Tue, Nov 18, 2014 at 1:37 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> So it can be used from out of openvswitch code.
>> Did couple of cosmetic changes on the way, namely variable naming and
>> adding support for 8021AD proto.
>>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>
>> v2->v3
>> - used previously introduced __vlan_insert_tag helper
>> - used skb_push/pull to get skb->data into needed point
>> - fixed skb->mac_len computation in skb_vlan_push pointed out by Pravin
>> v1->v2:
>> - adjusted to fix recent ovs changes
>> - included change to use make_writable suggested by Eric
>>
>> include/linux/skbuff.h | 2 +
>> net/core/skbuff.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
>> net/openvswitch/actions.c | 88 ++++++-------------------------------------
>> 3 files changed, 109 insertions(+), 76 deletions(-)
>>
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index e045516..78c299f 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -2679,6 +2679,8 @@ unsigned int skb_gso_transport_seglen(const struct sk_buff *skb);
>> struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
>> struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
>> int skb_ensure_writable(struct sk_buff *skb, int write_len);
>> +int skb_vlan_pop(struct sk_buff *skb);
>> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
>>
>> struct skb_checksum_ops {
>> __wsum (*update)(const void *mem, int len, __wsum wsum);
>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index d11bbe0..2f72e62 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -4163,6 +4163,101 @@ int skb_ensure_writable(struct sk_buff *skb, int write_len)
>> }
>> EXPORT_SYMBOL(skb_ensure_writable);
>>
>> +/* remove VLAN header from packet and update csum accordingly. */
>> +static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
>> +{
>> + struct vlan_hdr *vhdr;
>> + unsigned int offset = skb->data - skb_mac_header(skb);
>> + int err;
>> +
>> + __skb_push(skb, offset);
>> + err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
>> + if (unlikely(err))
>> + goto pull;
>> +
>> + skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
>> +
>> + vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
>> + *vlan_tci = ntohs(vhdr->h_vlan_TCI);
>> +
>> + memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
>> + __skb_pull(skb, VLAN_HLEN);
>> +
>> + vlan_set_encap_proto(skb, vhdr);
>> + skb->mac_header += VLAN_HLEN;
>> +
>> + if (skb_network_offset(skb) < ETH_HLEN)
>> + skb_set_network_header(skb, ETH_HLEN);
>> +
>> + skb_reset_mac_len(skb);
>
>skb_reset_mac_len() sets length according to ethernet and network
>offsets, but mpls expects mac-length to be offset to mpls header (ref.
>skb_mpls_header()). Therefore rather than reset we need to subtract
>VLAN_HLEN from mac_len. Same goes for vlan-push(), we can add
>VLAN_HLEN.
Hmm, I copied this bits from the original openvswitch code. So you say
that it is broken?
For the push case, __vlan_insert_tag is called which does:
skb->mac_header -= VLAN_HLEN;
then skb_reset_mac_len does:
skb->mac_len = skb->network_header - skb->mac_header
Should be the same as skb->mac_len += VLAN_HLEN, but nicer.
So for push, I think this is okay.
>
>> +pull:
>> + __skb_pull(skb, offset);
>> +
>> + return err;
>> +}
>> +
>> +int skb_vlan_pop(struct sk_buff *skb)
>> +{
>> + u16 vlan_tci;
>> + __be16 vlan_proto;
>> + int err;
>> +
>> + if (likely(vlan_tx_tag_present(skb))) {
>> + skb->vlan_tci = 0;
>> + } else {
>> + if (unlikely((skb->protocol != htons(ETH_P_8021Q) &&
>> + skb->protocol != htons(ETH_P_8021AD)) ||
>> + skb->len < VLAN_ETH_HLEN))
>> + return 0;
>> +
>> + err = __skb_vlan_pop(skb, &vlan_tci);
>> + if (err)
>> + return err;
>> + }
>> + /* move next vlan tag to hw accel tag */
>> + if (likely((skb->protocol != htons(ETH_P_8021Q) &&
>> + skb->protocol != htons(ETH_P_8021AD)) ||
>> + skb->len < VLAN_ETH_HLEN))
>> + return 0;
>> +
>> + vlan_proto = skb->protocol;
>> + err = __skb_vlan_pop(skb, &vlan_tci);
>> + if (unlikely(err))
>> + return err;
>> +
>> + __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(skb_vlan_pop);
>> +
>> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
>> +{
>> + if (vlan_tx_tag_present(skb)) {
>> + unsigned int offset = skb->data - skb_mac_header(skb);
>> + int err;
>> +
>> + /* __vlan_insert_tag expect skb->data pointing to mac header.
>> + * So change skb->data before calling it and change back to
>> + * original position later
>> + */
>> + __skb_push(skb, offset);
>> + err = __vlan_insert_tag(skb, skb->vlan_proto,
>> + vlan_tx_tag_get(skb));
>> + if (err)
>> + return err;
>> + skb->protocol = skb->vlan_proto;
>> + skb_reset_mac_len(skb);
>> + __skb_pull(skb, offset);
>> +
>> + if (skb->ip_summed == CHECKSUM_COMPLETE)
>> + skb->csum = csum_add(skb->csum, csum_partial(skb->data
>> + + (2 * ETH_ALEN), VLAN_HLEN, 0));
>> + }
>> + __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(skb_vlan_push);
>> +
>> /**
>> * alloc_skb_with_frags - allocate skb with page frags
>> *
>> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> index 7ffa377..ae7e1b2 100644
>> --- a/net/openvswitch/actions.c
>> +++ b/net/openvswitch/actions.c
>> @@ -206,93 +206,27 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
>> return 0;
>> }
>>
>> -/* remove VLAN header from packet and update csum accordingly. */
>> -static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
>> -{
>> - struct vlan_hdr *vhdr;
>> - int err;
>> -
>> - err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
>> - if (unlikely(err))
>> - return err;
>> -
>> - skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
>> -
>> - vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
>> - *current_tci = vhdr->h_vlan_TCI;
>> -
>> - memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
>> - __skb_pull(skb, VLAN_HLEN);
>> -
>> - vlan_set_encap_proto(skb, vhdr);
>> - skb->mac_header += VLAN_HLEN;
>> -
>> - if (skb_network_offset(skb) < ETH_HLEN)
>> - skb_set_network_header(skb, ETH_HLEN);
>> -
>> - /* Update mac_len for subsequent MPLS actions */
>> - skb_reset_mac_len(skb);
>> - return 0;
>> -}
>> -
>> static int pop_vlan(struct sk_buff *skb, struct sw_flow_key *key)
>> {
>> - __be16 tci;
>> int err;
>>
>> - if (likely(vlan_tx_tag_present(skb))) {
>> - skb->vlan_tci = 0;
>> - } else {
>> - if (unlikely(skb->protocol != htons(ETH_P_8021Q) ||
>> - skb->len < VLAN_ETH_HLEN))
>> - return 0;
>> -
>> - err = __pop_vlan_tci(skb, &tci);
>> - if (err)
>> - return err;
>> - }
>> - /* move next vlan tag to hw accel tag */
>> - if (likely(skb->protocol != htons(ETH_P_8021Q) ||
>> - skb->len < VLAN_ETH_HLEN)) {
>> + err = skb_vlan_pop(skb);
>> + if (vlan_tx_tag_present(skb))
>> + invalidate_flow_key(key);
>> + else
>> key->eth.tci = 0;
>> - return 0;
>> - }
>> -
>> - invalidate_flow_key(key);
>> - err = __pop_vlan_tci(skb, &tci);
>> - if (unlikely(err))
>> - return err;
>> -
>> - __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(tci));
>> - return 0;
>> + return err;
>> }
>>
>> static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
>> const struct ovs_action_push_vlan *vlan)
>> {
>> - if (unlikely(vlan_tx_tag_present(skb))) {
>> - u16 current_tag;
>> -
>> - /* push down current VLAN tag */
>> - current_tag = vlan_tx_tag_get(skb);
>> -
>> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
>> - current_tag);
>> - if (!skb)
>> - return -ENOMEM;
>> - /* Update mac_len for subsequent MPLS actions */
>> - skb->mac_len += VLAN_HLEN;
>> -
>> - if (skb->ip_summed == CHECKSUM_COMPLETE)
>> - skb->csum = csum_add(skb->csum, csum_partial(skb->data
>> - + (2 * ETH_ALEN), VLAN_HLEN, 0));
>> -
>> + if (vlan_tx_tag_present(skb))
>> invalidate_flow_key(key);
>> - } else {
>> + else
>> key->eth.tci = vlan->vlan_tci;
>> - }
>> - __vlan_hwaccel_put_tag(skb, vlan->vlan_tpid, ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
>> - return 0;
>> + return skb_vlan_push(skb, vlan->vlan_tpid,
>> + ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
>> }
>>
>> static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
>> @@ -858,8 +792,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>>
>> case OVS_ACTION_ATTR_PUSH_VLAN:
>> err = push_vlan(skb, key, nla_data(a));
>> - if (unlikely(err)) /* skb already freed. */
>> + if (unlikely(err)) {
>> + dev_kfree_skb_any(skb);
>> return err;
>> + }
>There is no need to check for error. Just break is sufficient. Error
>checking is done after switch case.
Fixed.
>> break;
>>
>> case OVS_ACTION_ATTR_POP_VLAN:
>> --
>> 1.9.3
>>
^ permalink raw reply
* Re: net: xfrm: Deletion of an unnecessary check before the function call "ipcomp_free_tfms"
From: Dan Carpenter @ 2014-11-19 10:10 UTC (permalink / raw)
To: SF Markus Elfring
Cc: David S. Miller, Herbert Xu, Steffen Klassert, netdev, LKML,
kernel-janitors, Julia Lawall
In-Reply-To: <546C6823.50900@users.sourceforge.net>
I have come to view you as a very clever troll. You will say
infuriating things like "That is an interesting background information
but the function implementation update suggestion is correct" and
pretend to not see the bug for nine emails. Then you'll say something
"Ha ha, I audited all the call trees and it can't actually be NULL so
you were getting angry for no reason." Except you'll say it in a
more obfuscated way than that. This is what you did last time.
regards,
dan carpenter
^ permalink raw reply
* Re: ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Jiri Bohac @ 2014-11-19 10:34 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Jiri Bohac, Arnaldo Carvalho de Melo, netdev, David Miller
In-Reply-To: <1609909.1jJeqOzgZ8@wuerfel>
On Wed, Nov 19, 2014 at 09:32:54AM +0100, Arnd Bergmann wrote:
> I'm more interested in the code structure, in particular this bit
>
> @@ -1807,8 +1812,10 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
>
> rc = skb_copy_datagram_iovec(skb, sizeof(struct ipxhdr), msg->msg_iov,
> copied);
> - if (rc)
> - goto out_free;
> + if (rc) {
> + skb_free_datagram(sk, skb);
> + goto out;
> + }
>
>
> after your change mixes coding styles: in some failure cases you just goto
> a cleanup part, in other cases you do the cleanup before the goto.
>
> If I'm reading the patch correctly, this change has introduced a leak
Very lame of me - thanks so much for spotitng this!
Sending a restructured v3.
--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ
^ permalink raw reply
* [PATCH v3] fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Jiri Bohac @ 2014-11-19 10:38 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: jbohac, Arnaldo Carvalho de Melo, netdev, David Miller
In-Reply-To: <20141119103413.GA19092@midget.suse.cz>
This fixes an old regression introduced by commit
b0d0d915 (ipx: remove the BKL).
When a recvmsg syscall blocks waiting for new data, no data can be sent on the
same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.
This breaks mars-nwe (NetWare emulator):
- the ncpserv process reads the request using recvmsg
- ncpserv forks and spawns nwconn
- ncpserv calls a (blocking) recvmsg and waits for new requests
- nwconn deadlocks in sendmsg on the same socket
Commit b0d0d915 has simply replaced BKL locking with
lock_sock/release_sock. Unlike now, BKL got unlocked while
sleeping, so a blocking recvmsg did not block a concurrent
sendmsg.
Only keep the socket locked while actually working with the socket data and
release it prior to calling skb_recv_datagram().
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index a0c7536..d0725d9 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1764,6 +1764,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
struct ipxhdr *ipx = NULL;
struct sk_buff *skb;
int copied, rc;
+ int locked = 1;
lock_sock(sk);
/* put the autobinding in */
@@ -1790,6 +1791,8 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
if (sock_flag(sk, SOCK_ZAPPED))
goto out;
+ release_sock(sk);
+ locked = 0;
skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
flags & MSG_DONTWAIT, &rc);
if (!skb) {
@@ -1825,7 +1828,8 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
out_free:
skb_free_datagram(sk, skb);
out:
- release_sock(sk);
+ if (locked)
+ release_sock(sk);
return rc;
}
--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ
^ permalink raw reply related
* Re: [PATCH 2/7] phy: miphy365x: Pass sysconfig register offsets via syscfg dt property.
From: Lee Jones @ 2014-11-19 10:42 UTC (permalink / raw)
To: Peter Griffin
Cc: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
maxime.coquelin, patrice.chotard, peppe.cavallaro, kishon, arnd,
netdev, devicetree, alexandre.torgue
In-Reply-To: <1416385632-5832-3-git-send-email-peter.griffin@linaro.org>
On Wed, 19 Nov 2014, Peter Griffin wrote:
> Based on Arnds review comments here https://lkml.org/lkml/2014/11/13/161, update
> the miphy365 phy driver to access sysconfig register offsets via syscfg dt property.
>
> This is because the reg property should not be mixing address spaces like it does
> currently for miphy365. This change then also aligns us to how other platforms such
> as keystone and bcm7445 pass there syscon offsets via DT.
>
> This patch breaks DT compatibility, but this platform is considered WIP, and is only
> used by a few developers who are upstreaming support for it. This change has been done
> as a single atomic commit to ensure it is bisectable.
You should wrap your lines sooner (70-75 chars).
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
> .../devicetree/bindings/phy/phy-miphy365x.txt | 15 +++++------
> arch/arm/boot/dts/stih416.dtsi | 10 ++++----
> drivers/phy/phy-miphy365x.c | 29 ++++++++--------------
> 3 files changed, 23 insertions(+), 31 deletions(-)
[...]
> diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
> index 801afaf..7308afe 100644
> --- a/drivers/phy/phy-miphy365x.c
> +++ b/drivers/phy/phy-miphy365x.c
[...]
> phy_set_drvdata(phy, miphy_dev->phys[port]);
> +
> port++;
> + /*sysconfig offsets are not indexed from zero */
You're missing a ' ' afrer '/*'.
Probably better do say what they _are_ indexed from, rather than what
they're not.
> + ret = of_property_read_u32_index(np, "st,syscfg", port,
> + &miphy_phy->ctrlreg);
> + if (ret) {
> + dev_err(&pdev->dev, "No sysconfig offset found\n");
> + return ret;
> + }
> }
>
> provider = devm_of_phy_provider_register(&pdev->dev, miphy365x_xlate);
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v3] fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Arnd Bergmann @ 2014-11-19 10:50 UTC (permalink / raw)
To: Jiri Bohac; +Cc: Arnaldo Carvalho de Melo, netdev, David Miller
In-Reply-To: <20141119103814.GB19092@midget.suse.cz>
On Wednesday 19 November 2014 11:38:14 Jiri Bohac wrote:
> This fixes an old regression introduced by commit
> b0d0d915 (ipx: remove the BKL).
>
> When a recvmsg syscall blocks waiting for new data, no data can be sent on the
> same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.
>
> This breaks mars-nwe (NetWare emulator):
> - the ncpserv process reads the request using recvmsg
> - ncpserv forks and spawns nwconn
> - ncpserv calls a (blocking) recvmsg and waits for new requests
> - nwconn deadlocks in sendmsg on the same socket
>
> Commit b0d0d915 has simply replaced BKL locking with
> lock_sock/release_sock. Unlike now, BKL got unlocked while
> sleeping, so a blocking recvmsg did not block a concurrent
> sendmsg.
>
> Only keep the socket locked while actually working with the socket data and
> release it prior to calling skb_recv_datagram().
>
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Looks correct to me and simple enough,
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
> index a0c7536..d0725d9 100644
> --- a/net/ipx/af_ipx.c
> +++ b/net/ipx/af_ipx.c
> @@ -1764,6 +1764,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
> struct ipxhdr *ipx = NULL;
> struct sk_buff *skb;
> int copied, rc;
> + int locked = 1;
>
> lock_sock(sk);
> /* put the autobinding in */
> @@ -1790,6 +1791,8 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
> if (sock_flag(sk, SOCK_ZAPPED))
> goto out;
>
> + release_sock(sk);
> + locked = 0;
> skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
> flags & MSG_DONTWAIT, &rc);
> if (!skb) {
> @@ -1825,7 +1828,8 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
> out_free:
> skb_free_datagram(sk, skb);
> out:
> - release_sock(sk);
> + if (locked)
> + release_sock(sk);
> return rc;
> }
I don't like the idea of having a local flag for this, and would still
prefer the simpler version of taking the lock again even if it's not
needed, but your version is probably good enough unless Dave wants
you to do a v4 for this.
Arnd
^ permalink raw reply
* hello
From: hi @ 2014-11-19 11:42 UTC (permalink / raw)
this is amazing
iphone 6plus 400euro, ipad,samsung s5,moto,laptop....
w eb : loooooo. com
^ permalink raw reply
* [PATCH 00/10] net: phy: add device-type abstraction
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
This series adds device and device-type abstractions to the micrel
driver, and enables support for RMII-reference clock selection for
KSZ8081 and KSZ8091 devices.
While adding support for more features for the Micrel PHYs mentioned
above, it became apparent that the configuration space is much too large
and that adding type-specific callbacks will simply not scale. Instead I
added a driver_data field to struct phy_device, which can be used to
store static device type data that can be parsed and acted on in
generic driver callbacks. This allows a lot of duplicated code to be
removed, and should make it much easier to add new features or deal with
device-type quirks in the future.
The series has been tested on a dual KSZ8081 setup. Further testing on
other Micrel PHYs would be much appreciated.
The recent commit a95a18afe4c8 ("phy/micrel: KSZ8031RNL RMII clock
reconfiguration bug") currently prevents KSZ8031 PHYs from using the
generic config-init. Bruno, who is the author of that patch, has agreed
to test this series and some follow-up diagnostic patches to determine
how best to incorporate these devices as well. I intend to send a
follow-up patch that removes the custom 8031 config-init and documents
this quirk, but the current series can be applied meanwhile.
These patches are against net-next which contains some already merged
prerequisite patches to the driver.
Johan
Johan Hovold (10):
net: phy: add static data field to struct phy_driver
net: phy: micrel: add device-type abstraction
net: phy: micrel: parse of nodes at probe
net: phy: micrel: add has-broadcast-disable flag to type data
net: phy: micrel: add generic clock-mode-select support
net: phy: micrel: add support for clock-mode select to KSZ8081/KSZ8091
dt/bindings: reformat micrel eth-phy documentation
dt/bindings: add clock-select function property to micrel phy binding
net: phy: micrel: refactor interrupt config
net: phy: micrel: add copyright entry
Documentation/devicetree/bindings/net/micrel.txt | 37 ++--
drivers/net/phy/micrel.c | 262 ++++++++++++++---------
include/linux/micrel_phy.h | 1 -
include/linux/phy.h | 2 +
4 files changed, 184 insertions(+), 118 deletions(-)
--
2.0.4
^ permalink raw reply
* [PATCH 02/10] net: phy: micrel: add device-type abstraction
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add structured device-type information and support for generic led-mode
setup to the generic config_init callback.
This is a first step in ultimately getting rid of device-type specific
callbacks.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 83 ++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 70 insertions(+), 13 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 30e894d6ffbd..1b528137afd9 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -73,6 +73,30 @@
#define PS_TO_REG 200
+struct kszphy_type {
+ u32 led_mode_reg;
+};
+
+struct kszphy_priv {
+ const struct kszphy_type *type;
+};
+
+static const struct kszphy_type ksz8021_type = {
+ .led_mode_reg = MII_KSZPHY_CTRL_2,
+};
+
+static const struct kszphy_type ksz8041_type = {
+ .led_mode_reg = MII_KSZPHY_CTRL_1,
+};
+
+static const struct kszphy_type ksz8051_type = {
+ .led_mode_reg = MII_KSZPHY_CTRL_2,
+};
+
+static const struct kszphy_type ksz8081_type = {
+ .led_mode_reg = MII_KSZPHY_CTRL_2,
+};
+
static int ksz_config_flags(struct phy_device *phydev)
{
int regval;
@@ -229,19 +253,25 @@ out:
static int kszphy_config_init(struct phy_device *phydev)
{
- return 0;
-}
+ struct kszphy_priv *priv = phydev->priv;
+ const struct kszphy_type *type;
-static int kszphy_config_init_led8041(struct phy_device *phydev)
-{
- return kszphy_setup_led(phydev, MII_KSZPHY_CTRL_1);
+ if (!priv)
+ return 0;
+
+ type = priv->type;
+
+ if (type->led_mode_reg)
+ kszphy_setup_led(phydev, type->led_mode_reg);
+
+ return 0;
}
static int ksz8021_config_init(struct phy_device *phydev)
{
int rc;
- kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
+ kszphy_config_init(phydev);
rc = ksz_config_flags(phydev);
if (rc < 0)
@@ -256,7 +286,7 @@ static int ks8051_config_init(struct phy_device *phydev)
{
int rc;
- kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
+ kszphy_config_init(phydev);
rc = ksz_config_flags(phydev);
return rc < 0 ? rc : 0;
@@ -265,9 +295,8 @@ static int ks8051_config_init(struct phy_device *phydev)
static int ksz8081_config_init(struct phy_device *phydev)
{
kszphy_broadcast_disable(phydev);
- kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
- return 0;
+ return kszphy_config_init(phydev);
}
static int ksz9021_load_values_from_of(struct phy_device *phydev,
@@ -499,6 +528,22 @@ ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
{
}
+static int kszphy_probe(struct phy_device *phydev)
+{
+ const struct kszphy_type *type = phydev->drv->driver_data;
+ struct kszphy_priv *priv;
+
+ priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ phydev->priv = priv;
+
+ priv->type = type;
+
+ return 0;
+}
+
static int ksz8021_probe(struct phy_device *phydev)
{
struct clk *clk;
@@ -517,7 +562,7 @@ static int ksz8021_probe(struct phy_device *phydev)
}
}
- return 0;
+ return kszphy_probe(phydev);
}
static struct phy_driver ksphy_driver[] = {
@@ -542,6 +587,7 @@ static struct phy_driver ksphy_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause |
SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz8021_type,
.probe = ksz8021_probe,
.config_init = ksz8021_config_init,
.config_aneg = genphy_config_aneg,
@@ -558,6 +604,7 @@ static struct phy_driver ksphy_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause |
SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz8021_type,
.probe = ksz8021_probe,
.config_init = ksz8021_config_init,
.config_aneg = genphy_config_aneg,
@@ -574,7 +621,9 @@ static struct phy_driver ksphy_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
| SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
- .config_init = kszphy_config_init_led8041,
+ .driver_data = &ksz8041_type,
+ .probe = kszphy_probe,
+ .config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
@@ -589,7 +638,9 @@ static struct phy_driver ksphy_driver[] = {
.features = PHY_BASIC_FEATURES |
SUPPORTED_Pause | SUPPORTED_Asym_Pause,
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
- .config_init = kszphy_config_init_led8041,
+ .driver_data = &ksz8041_type,
+ .probe = kszphy_probe,
+ .config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
@@ -604,6 +655,8 @@ static struct phy_driver ksphy_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
| SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz8051_type,
+ .probe = kszphy_probe,
.config_init = ks8051_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
@@ -618,7 +671,9 @@ static struct phy_driver ksphy_driver[] = {
.phy_id_mask = 0x00ffffff,
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
- .config_init = kszphy_config_init_led8041,
+ .driver_data = &ksz8041_type,
+ .probe = kszphy_probe,
+ .config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
@@ -632,6 +687,8 @@ static struct phy_driver ksphy_driver[] = {
.phy_id_mask = 0x00fffff0,
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz8081_type,
+ .probe = kszphy_probe,
.config_init = ksz8081_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
--
2.0.4
^ permalink raw reply related
* [PATCH 03/10] net: phy: micrel: parse of nodes at probe
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Parse the "micrel,led-mode" property at probe, rather than at config_init
time in the led-setup helper itself.
Note that the bogus parent->of_node bit is removed.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 1b528137afd9..6a81aaca5b1c 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -79,6 +79,7 @@ struct kszphy_type {
struct kszphy_priv {
const struct kszphy_type *type;
+ int led_mode;
};
static const struct kszphy_type ksz8021_type = {
@@ -186,24 +187,9 @@ static int ks8737_config_intr(struct phy_device *phydev)
return rc < 0 ? rc : 0;
}
-static int kszphy_setup_led(struct phy_device *phydev, u32 reg)
+static int kszphy_setup_led(struct phy_device *phydev, u32 reg, int val)
{
-
- struct device *dev = &phydev->dev;
- struct device_node *of_node = dev->of_node;
int rc, temp, shift;
- u32 val;
-
- if (!of_node && dev->parent->of_node)
- of_node = dev->parent->of_node;
-
- if (of_property_read_u32(of_node, "micrel,led-mode", &val))
- return 0;
-
- if (val > 3) {
- dev_err(&phydev->dev, "invalid led mode: 0x%02x\n", val);
- return -EINVAL;
- }
switch (reg) {
case MII_KSZPHY_CTRL_1:
@@ -261,8 +247,8 @@ static int kszphy_config_init(struct phy_device *phydev)
type = priv->type;
- if (type->led_mode_reg)
- kszphy_setup_led(phydev, type->led_mode_reg);
+ if (priv->led_mode >= 0)
+ kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
return 0;
}
@@ -531,7 +517,9 @@ ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
static int kszphy_probe(struct phy_device *phydev)
{
const struct kszphy_type *type = phydev->drv->driver_data;
+ struct device_node *np = phydev->dev.of_node;
struct kszphy_priv *priv;
+ int ret;
priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -541,6 +529,21 @@ static int kszphy_probe(struct phy_device *phydev)
priv->type = type;
+ if (type->led_mode_reg) {
+ ret = of_property_read_u32(np, "micrel,led-mode",
+ &priv->led_mode);
+ if (ret)
+ priv->led_mode = -1;
+
+ if (priv->led_mode > 3) {
+ dev_err(&phydev->dev, "invalid led mode: 0x%02x\n",
+ priv->led_mode);
+ priv->led_mode = -1;
+ }
+ } else {
+ priv->led_mode = -1;
+ }
+
return 0;
}
--
2.0.4
^ permalink raw reply related
* [PATCH 06/10] net: phy: micrel: add support for clock-mode select to KSZ8081/KSZ8091
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Micrel KSZ8081 and KSZ8091 PHYs have the RMII Reference Clock Select
bit, which is used to select 25 or 50 MHz clock mode.
Note that on some revisions of the PHY (e.g. KSZ8081RND) the function of
this bit is inverted so that setting it enables 25 rather than 50 MHz
mode. Add a new device-tree property
"micrel,rmii-reference-clock-select-25-mhz" to describe this.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
Documentation/devicetree/bindings/net/micrel.txt | 4 ++--
drivers/net/phy/micrel.c | 11 ++++++-----
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
index 30062fae5623..a1bab5eaae02 100644
--- a/Documentation/devicetree/bindings/net/micrel.txt
+++ b/Documentation/devicetree/bindings/net/micrel.txt
@@ -22,5 +22,5 @@ Optional properties:
- clocks, clock-names: contains clocks according to the common clock bindings.
supported clocks:
- - KSZ8021, KSZ8031: "rmii-ref": The RMII refence input clock. Used
- to determine the XI input clock.
+ - KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII
+ refence input clock. Used to determine the XI input clock.
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d2e790cd3651..04fbee846b66 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -102,6 +102,7 @@ static const struct kszphy_type ksz8051_type = {
static const struct kszphy_type ksz8081_type = {
.led_mode_reg = MII_KSZPHY_CTRL_2,
.has_broadcast_disable = true,
+ .has_rmii_ref_clk_sel = true,
};
static int kszphy_extended_write(struct phy_device *phydev,
@@ -548,16 +549,16 @@ static int kszphy_probe(struct phy_device *phydev)
clk = devm_clk_get(&phydev->dev, "rmii-ref");
if (!IS_ERR(clk)) {
unsigned long rate = clk_get_rate(clk);
+ bool rmii_ref_clk_sel_25_mhz;
priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
+ rmii_ref_clk_sel_25_mhz = of_property_read_bool(np,
+ "micrel,rmii-reference-clock-select-25-mhz");
- /* FIXME: add support for PHY revisions that have this bit
- * inverted (e.g. through new property or based on PHY ID).
- */
if (rate > 24500000 && rate < 25500000) {
- priv->rmii_ref_clk_sel_val = false;
+ priv->rmii_ref_clk_sel_val = rmii_ref_clk_sel_25_mhz;
} else if (rate > 49500000 && rate < 50500000) {
- priv->rmii_ref_clk_sel_val = true;
+ priv->rmii_ref_clk_sel_val = !rmii_ref_clk_sel_25_mhz;
} else {
dev_err(&phydev->dev, "Clock rate out of range: %ld\n", rate);
return -EINVAL;
--
2.0.4
^ permalink raw reply related
* [PATCH 07/10] dt/bindings: reformat micrel eth-phy documentation
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold, devicetree
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Reduce indentation of Micrel PHY binding documentations somewhat.
Also fix "reference input clock" typo while at it.
Cc: devicetree@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
Documentation/devicetree/bindings/net/micrel.txt | 26 ++++++++++++------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
index a1bab5eaae02..20a6cac7abc6 100644
--- a/Documentation/devicetree/bindings/net/micrel.txt
+++ b/Documentation/devicetree/bindings/net/micrel.txt
@@ -6,21 +6,21 @@ Optional properties:
- micrel,led-mode : LED mode value to set for PHYs with configurable LEDs.
- Configure the LED mode with single value. The list of PHYs and
- the bits that are currently supported:
+ Configure the LED mode with single value. The list of PHYs and the
+ bits that are currently supported:
- KSZ8001: register 0x1e, bits 15..14
- KSZ8041: register 0x1e, bits 15..14
- KSZ8021: register 0x1f, bits 5..4
- KSZ8031: register 0x1f, bits 5..4
- KSZ8051: register 0x1f, bits 5..4
- KSZ8081: register 0x1f, bits 5..4
- KSZ8091: register 0x1f, bits 5..4
+ KSZ8001: register 0x1e, bits 15..14
+ KSZ8041: register 0x1e, bits 15..14
+ KSZ8021: register 0x1f, bits 5..4
+ KSZ8031: register 0x1f, bits 5..4
+ KSZ8051: register 0x1f, bits 5..4
+ KSZ8081: register 0x1f, bits 5..4
+ KSZ8091: register 0x1f, bits 5..4
- See the respective PHY datasheet for the mode values.
+ See the respective PHY datasheet for the mode values.
- clocks, clock-names: contains clocks according to the common clock bindings.
- supported clocks:
- - KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII
- refence input clock. Used to determine the XI input clock.
+ supported clocks:
+ - KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII reference
+ input clock. Used to determine the XI input clock.
--
2.0.4
^ permalink raw reply related
* [PATCH 09/10] net: phy: micrel: refactor interrupt config
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add generic interrupt-config callback and store interrupt-level bitmask
in type data for PHY types not using bit 9.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 71 ++++++++++++++++++++----------------------------
1 file changed, 29 insertions(+), 42 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 04fbee846b66..d7eb7b0e6d4e 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -55,8 +55,6 @@
#define MII_KSZPHY_CTRL MII_KSZPHY_CTRL_2
/* bitmap of PHY register to set interrupt mode */
#define KSZPHY_CTRL_INT_ACTIVE_HIGH BIT(9)
-#define KSZ9021_CTRL_INT_ACTIVE_HIGH BIT(14)
-#define KS8737_CTRL_INT_ACTIVE_HIGH BIT(14)
#define KSZPHY_RMII_REF_CLK_SEL BIT(7)
/* Write/read to/from extended registers */
@@ -75,6 +73,7 @@
struct kszphy_type {
u32 led_mode_reg;
+ u16 interrupt_level_mask;
bool has_broadcast_disable;
bool has_rmii_ref_clk_sel;
};
@@ -105,6 +104,14 @@ static const struct kszphy_type ksz8081_type = {
.has_rmii_ref_clk_sel = true,
};
+static const struct kszphy_type ks8737_type = {
+ .interrupt_level_mask = BIT(14),
+};
+
+static const struct kszphy_type ksz9021_type = {
+ .interrupt_level_mask = BIT(14),
+};
+
static int kszphy_extended_write(struct phy_device *phydev,
u32 regnum, u16 val)
{
@@ -129,54 +136,31 @@ static int kszphy_ack_interrupt(struct phy_device *phydev)
return (rc < 0) ? rc : 0;
}
-static int kszphy_set_interrupt(struct phy_device *phydev)
-{
- int temp;
- temp = (PHY_INTERRUPT_ENABLED == phydev->interrupts) ?
- KSZPHY_INTCS_ALL : 0;
- return phy_write(phydev, MII_KSZPHY_INTCS, temp);
-}
-
static int kszphy_config_intr(struct phy_device *phydev)
{
- int temp, rc;
-
- /* set the interrupt pin active low */
- temp = phy_read(phydev, MII_KSZPHY_CTRL);
- if (temp < 0)
- return temp;
- temp &= ~KSZPHY_CTRL_INT_ACTIVE_HIGH;
- phy_write(phydev, MII_KSZPHY_CTRL, temp);
- rc = kszphy_set_interrupt(phydev);
- return rc < 0 ? rc : 0;
-}
+ const struct kszphy_type *type = phydev->drv->driver_data;
+ int temp;
+ u16 mask;
-static int ksz9021_config_intr(struct phy_device *phydev)
-{
- int temp, rc;
+ if (type && type->interrupt_level_mask)
+ mask = type->interrupt_level_mask;
+ else
+ mask = KSZPHY_CTRL_INT_ACTIVE_HIGH;
/* set the interrupt pin active low */
temp = phy_read(phydev, MII_KSZPHY_CTRL);
if (temp < 0)
return temp;
- temp &= ~KSZ9021_CTRL_INT_ACTIVE_HIGH;
+ temp &= ~mask;
phy_write(phydev, MII_KSZPHY_CTRL, temp);
- rc = kszphy_set_interrupt(phydev);
- return rc < 0 ? rc : 0;
-}
-static int ks8737_config_intr(struct phy_device *phydev)
-{
- int temp, rc;
+ /* enable / disable interrupts */
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ temp = KSZPHY_INTCS_ALL;
+ else
+ temp = 0;
- /* set the interrupt pin active low */
- temp = phy_read(phydev, MII_KSZPHY_CTRL);
- if (temp < 0)
- return temp;
- temp &= ~KS8737_CTRL_INT_ACTIVE_HIGH;
- phy_write(phydev, MII_KSZPHY_CTRL, temp);
- rc = kszphy_set_interrupt(phydev);
- return rc < 0 ? rc : 0;
+ return phy_write(phydev, MII_KSZPHY_INTCS, temp);
}
static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)
@@ -581,11 +565,12 @@ static struct phy_driver ksphy_driver[] = {
.name = "Micrel KS8737",
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ks8737_type,
.config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
- .config_intr = ks8737_config_intr,
+ .config_intr = kszphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
@@ -726,11 +711,12 @@ static struct phy_driver ksphy_driver[] = {
.name = "Micrel KSZ9021 Gigabit PHY",
.features = (PHY_GBIT_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz9021_type,
.config_init = ksz9021_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
- .config_intr = ksz9021_config_intr,
+ .config_intr = kszphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
.read_mmd_indirect = ksz9021_rd_mmd_phyreg,
@@ -742,11 +728,12 @@ static struct phy_driver ksphy_driver[] = {
.name = "Micrel KSZ9031 Gigabit PHY",
.features = (PHY_GBIT_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz9021_type,
.config_init = ksz9031_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
- .config_intr = ksz9021_config_intr,
+ .config_intr = kszphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
.driver = { .owner = THIS_MODULE, },
--
2.0.4
^ permalink raw reply related
* [PATCH 10/10] net: phy: micrel: add copyright entry
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add myself to the list of copyright holders.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d7eb7b0e6d4e..c530de1e63f5 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -6,6 +6,7 @@
* Author: David J. Choi
*
* Copyright (c) 2010-2013 Micrel, Inc.
+ * Copyright (c) 2014 Johan Hovold <johan@kernel.org>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
--
2.0.4
^ permalink raw reply related
* [PATCH 01/10] net: phy: add static data field to struct phy_driver
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add static driver-data field to struct phy_driver, which can be used to
store structured device-type information.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
include/linux/phy.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 07794e720139..22af8f8f5802 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -433,6 +433,7 @@ struct phy_device {
* by this PHY
* flags: A bitfield defining certain other features this PHY
* supports (like interrupts)
+ * driver_data: static driver data
*
* The drivers must implement config_aneg and read_status. All
* other functions are optional. Note that none of these
@@ -448,6 +449,7 @@ struct phy_driver {
unsigned int phy_id_mask;
u32 features;
u32 flags;
+ const void *driver_data;
/*
* Called to issue a PHY software reset
--
2.0.4
^ permalink raw reply related
* [PATCH 05/10] net: phy: micrel: add generic clock-mode-select support
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add generic RMII-Reference-Clock-Select support.
Several Micrel PHY have an RMII-Reference-Clock-Select bit to select
25 MHz or 50 MHz clock mode. Recently, support for configuring this
through device tree for KSZ8021 and KSZ8031 was added.
Generalise this support so that it can be configured for other PHY types
as well.
Note that some PHY revisions (of the same type) has this bit inverted.
This should be either configurable through a new device-tree property,
or preferably, determined based on PHY ID if possible.
Also note that this removes support for setting 25 MHz mode from board
files which was also added by the above mentioned commit 45f56cb82e45
("net/phy: micrel: Add clock support for KSZ8021/KSZ8031").
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 93 +++++++++++++++++++++++++---------------------
include/linux/micrel_phy.h | 1 -
2 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index a0e944099020..d2e790cd3651 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -57,7 +57,7 @@
#define KSZPHY_CTRL_INT_ACTIVE_HIGH BIT(9)
#define KSZ9021_CTRL_INT_ACTIVE_HIGH BIT(14)
#define KS8737_CTRL_INT_ACTIVE_HIGH BIT(14)
-#define KSZ8051_RMII_50MHZ_CLK BIT(7)
+#define KSZPHY_RMII_REF_CLK_SEL BIT(7)
/* Write/read to/from extended registers */
#define MII_KSZPHY_EXTREG 0x0b
@@ -76,15 +76,19 @@
struct kszphy_type {
u32 led_mode_reg;
bool has_broadcast_disable;
+ bool has_rmii_ref_clk_sel;
};
struct kszphy_priv {
const struct kszphy_type *type;
int led_mode;
+ bool rmii_ref_clk_sel;
+ bool rmii_ref_clk_sel_val;
};
static const struct kszphy_type ksz8021_type = {
.led_mode_reg = MII_KSZPHY_CTRL_2,
+ .has_rmii_ref_clk_sel = true,
};
static const struct kszphy_type ksz8041_type = {
@@ -100,21 +104,6 @@ static const struct kszphy_type ksz8081_type = {
.has_broadcast_disable = true,
};
-static int ksz_config_flags(struct phy_device *phydev)
-{
- int regval;
-
- if (phydev->dev_flags & (MICREL_PHY_50MHZ_CLK | MICREL_PHY_25MHZ_CLK)) {
- regval = phy_read(phydev, MII_KSZPHY_CTRL);
- if (phydev->dev_flags & MICREL_PHY_50MHZ_CLK)
- regval |= KSZ8051_RMII_50MHZ_CLK;
- else
- regval &= ~KSZ8051_RMII_50MHZ_CLK;
- return phy_write(phydev, MII_KSZPHY_CTRL, regval);
- }
- return 0;
-}
-
static int kszphy_extended_write(struct phy_device *phydev,
u32 regnum, u16 val)
{
@@ -189,6 +178,22 @@ static int ks8737_config_intr(struct phy_device *phydev)
return rc < 0 ? rc : 0;
}
+static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)
+{
+ int ctrl;
+
+ ctrl = phy_read(phydev, MII_KSZPHY_CTRL);
+ if (ctrl < 0)
+ return ctrl;
+
+ if (val)
+ ctrl |= KSZPHY_RMII_REF_CLK_SEL;
+ else
+ ctrl &= ~KSZPHY_RMII_REF_CLK_SEL;
+
+ return phy_write(phydev, MII_KSZPHY_CTRL, ctrl);
+}
+
static int kszphy_setup_led(struct phy_device *phydev, u32 reg, int val)
{
int rc, temp, shift;
@@ -243,6 +248,7 @@ static int kszphy_config_init(struct phy_device *phydev)
{
struct kszphy_priv *priv = phydev->priv;
const struct kszphy_type *type;
+ int ret;
if (!priv)
return 0;
@@ -252,6 +258,14 @@ static int kszphy_config_init(struct phy_device *phydev)
if (type->has_broadcast_disable)
kszphy_broadcast_disable(phydev);
+ if (priv->rmii_ref_clk_sel) {
+ ret = kszphy_rmii_clk_sel(phydev, priv->rmii_ref_clk_sel_val);
+ if (ret) {
+ dev_err(&phydev->dev, "failed to set rmii reference clock\n");
+ return ret;
+ }
+ }
+
if (priv->led_mode >= 0)
kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
@@ -262,10 +276,8 @@ static int ksz8021_config_init(struct phy_device *phydev)
{
int rc;
- kszphy_config_init(phydev);
-
- rc = ksz_config_flags(phydev);
- if (rc < 0)
+ rc = kszphy_config_init(phydev);
+ if (rc)
return rc;
rc = kszphy_broadcast_disable(phydev);
@@ -273,16 +285,6 @@ static int ksz8021_config_init(struct phy_device *phydev)
return rc < 0 ? rc : 0;
}
-static int ks8051_config_init(struct phy_device *phydev)
-{
- int rc;
-
- kszphy_config_init(phydev);
-
- rc = ksz_config_flags(phydev);
- return rc < 0 ? rc : 0;
-}
-
static int ksz9021_load_values_from_of(struct phy_device *phydev,
struct device_node *of_node, u16 reg,
char *field1, char *field2,
@@ -517,6 +519,7 @@ static int kszphy_probe(struct phy_device *phydev)
const struct kszphy_type *type = phydev->drv->driver_data;
struct device_node *np = phydev->dev.of_node;
struct kszphy_priv *priv;
+ struct clk *clk;
int ret;
priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
@@ -542,28 +545,32 @@ static int kszphy_probe(struct phy_device *phydev)
priv->led_mode = -1;
}
- return 0;
-}
-
-static int ksz8021_probe(struct phy_device *phydev)
-{
- struct clk *clk;
-
clk = devm_clk_get(&phydev->dev, "rmii-ref");
if (!IS_ERR(clk)) {
unsigned long rate = clk_get_rate(clk);
+ priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
+
+ /* FIXME: add support for PHY revisions that have this bit
+ * inverted (e.g. through new property or based on PHY ID).
+ */
if (rate > 24500000 && rate < 25500000) {
- phydev->dev_flags |= MICREL_PHY_25MHZ_CLK;
+ priv->rmii_ref_clk_sel_val = false;
} else if (rate > 49500000 && rate < 50500000) {
- phydev->dev_flags |= MICREL_PHY_50MHZ_CLK;
+ priv->rmii_ref_clk_sel_val = true;
} else {
dev_err(&phydev->dev, "Clock rate out of range: %ld\n", rate);
return -EINVAL;
}
}
- return kszphy_probe(phydev);
+ /* Support legacy board-file configuration */
+ if (phydev->dev_flags & MICREL_PHY_50MHZ_CLK) {
+ priv->rmii_ref_clk_sel = true;
+ priv->rmii_ref_clk_sel_val = true;
+ }
+
+ return 0;
}
static struct phy_driver ksphy_driver[] = {
@@ -589,7 +596,7 @@ static struct phy_driver ksphy_driver[] = {
SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
.driver_data = &ksz8021_type,
- .probe = ksz8021_probe,
+ .probe = kszphy_probe,
.config_init = ksz8021_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
@@ -606,7 +613,7 @@ static struct phy_driver ksphy_driver[] = {
SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
.driver_data = &ksz8021_type,
- .probe = ksz8021_probe,
+ .probe = kszphy_probe,
.config_init = ksz8021_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
@@ -658,7 +665,7 @@ static struct phy_driver ksphy_driver[] = {
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
.driver_data = &ksz8051_type,
.probe = kszphy_probe,
- .config_init = ks8051_config_init,
+ .config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 53d33dee70e1..2e5b194b9b19 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -37,7 +37,6 @@
/* struct phy_device dev_flags definitions */
#define MICREL_PHY_50MHZ_CLK 0x00000001
-#define MICREL_PHY_25MHZ_CLK 0x00000002
#define MICREL_KSZ9021_EXTREG_CTRL 0xB
#define MICREL_KSZ9021_EXTREG_DATA_WRITE 0xC
--
2.0.4
^ permalink raw reply related
* [PATCH 04/10] net: phy: micrel: add has-broadcast-disable flag to type data
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add has_broadcast_disable flag to type-data and generic config_init.
This allows us to remove the ksz8081 config_init callback.
Note that ksz8021_config_init is kept for now due to a95a18afe4c8
("phy/micrel: KSZ8031RNL RMII clock reconfiguration bug").
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 6a81aaca5b1c..a0e944099020 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -75,6 +75,7 @@
struct kszphy_type {
u32 led_mode_reg;
+ bool has_broadcast_disable;
};
struct kszphy_priv {
@@ -96,6 +97,7 @@ static const struct kszphy_type ksz8051_type = {
static const struct kszphy_type ksz8081_type = {
.led_mode_reg = MII_KSZPHY_CTRL_2,
+ .has_broadcast_disable = true,
};
static int ksz_config_flags(struct phy_device *phydev)
@@ -247,6 +249,9 @@ static int kszphy_config_init(struct phy_device *phydev)
type = priv->type;
+ if (type->has_broadcast_disable)
+ kszphy_broadcast_disable(phydev);
+
if (priv->led_mode >= 0)
kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
@@ -278,13 +283,6 @@ static int ks8051_config_init(struct phy_device *phydev)
return rc < 0 ? rc : 0;
}
-static int ksz8081_config_init(struct phy_device *phydev)
-{
- kszphy_broadcast_disable(phydev);
-
- return kszphy_config_init(phydev);
-}
-
static int ksz9021_load_values_from_of(struct phy_device *phydev,
struct device_node *of_node, u16 reg,
char *field1, char *field2,
@@ -692,7 +690,7 @@ static struct phy_driver ksphy_driver[] = {
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
.driver_data = &ksz8081_type,
.probe = kszphy_probe,
- .config_init = ksz8081_config_init,
+ .config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
--
2.0.4
^ permalink raw reply related
* [PATCH 08/10] dt/bindings: add clock-select function property to micrel phy binding
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold, devicetree
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add "micrel,rmii-reference-clock-select-25-mhz" to Micrel ethernet PHY
binding documentation.
This property is needed to properly describe some revisions of Micrel
PHYs which has the function of this configuration bit inverted so that
setting it enables 25 MHz rather than 50 MHz clock mode.
Note that a clock reference ("rmii-ref") is still needed to actually
select either mode.
Cc: devicetree@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
Documentation/devicetree/bindings/net/micrel.txt | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
index 20a6cac7abc6..87496a8c64ab 100644
--- a/Documentation/devicetree/bindings/net/micrel.txt
+++ b/Documentation/devicetree/bindings/net/micrel.txt
@@ -19,6 +19,17 @@ Optional properties:
See the respective PHY datasheet for the mode values.
+ - micrel,rmii-reference-clock-select-25-mhz: RMII Reference Clock Select
+ bit selects 25 MHz mode
+
+ Setting the RMII Reference Clock Select bit enables 25 MHz rather
+ than 50 MHz clock mode.
+
+ Note that this option in only needed for certain PHY revisions with a
+ non-standard, inverted function of this configuration bit.
+ Specifically, a clock reference ("rmii-ref" below) is always needed to
+ actually select a mode.
+
- clocks, clock-names: contains clocks according to the common clock bindings.
supported clocks:
--
2.0.4
^ permalink raw reply related
* Re: [PATCH] net: team: expose sysfs attributes for each team option
From: Hamad Kadmany @ 2014-11-19 12:28 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Hamad Kadmany, netdev
In-Reply-To: <20141117130201.GB1872@nanopsycho.orion>
On Mon, November 17, 2014 1:02 pm, Jiri Pirko wrote:
>
> Nak.
>
> I don't like this patch. The plan for team was keep things simple and to
> have single userspace api using netlink, as that is the correct way to
> deal with things. Sysfs for this use-case is not. Please, use netlink api.
Using team driver requires using libnl3. In Android based platforms, libnl3 is not supported, and libnl3 license poses constraints in Android's user-space.
This is the reason for the this change, to have at least the team options exposed through sysfs.
>
>>
>>Signed-off-by: Hamad Kadmany <hkadmany@codeaurora.org>
>>---
>> drivers/net/team/team.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++--
>> include/linux/if_team.h | 3 +
>> 2 files changed, 278 insertions(+), 7 deletions(-)
>>
>>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>>index 1222229..afd2f8f 100644
>>--- a/drivers/net/team/team.c
>>+++ b/drivers/net/team/team.c
>>@@ -110,8 +110,198 @@ struct team_option_inst { /* One for each option instance */
>> struct team_option_inst_info info;
>> bool changed;
>> bool removed;
>>+ bool dev_attr_file_exist;
>>+ struct device_attribute dev_attr; /* corresponding sysfs attribute */
>> };
>>
>>+static struct attribute *team_sysfs_attrs[] = {
>>+ NULL,
>>+};
>>+
>>+static struct attribute_group team_sysfs_attr_group = {
>>+ .attrs = team_sysfs_attrs,
>>+};
>>+
>>+/* Device attributes (sysfs) */
>>+
>>+static ssize_t show_attribute(struct device *dev,
>>+ struct device_attribute *attr,
>>+ char *buf)
>>+{
>>+ struct team *team = dev_get_drvdata(dev);
>>+ struct team_option_inst *opt_inst;
>>+ ssize_t ret;
>>+ struct team_option *option;
>>+ struct team_gsetter_ctx ctx;
>>+
>>+ if (mutex_lock_interruptible(&team->lock))
>>+ return -ERESTARTSYS;
>>+
>>+ opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>+ option = opt_inst->option;
>>+ if (!option->getter) {
>>+ ret = -EOPNOTSUPP;
>>+ netdev_err(team->dev,
>>+ "Option %s is write only\n", attr->attr.name);
>>+ goto exit;
>>+ }
>>+
>>+ ctx.info = &opt_inst->info;
>>+ /* let the option getter do its job */
>>+ ret = option->getter(team, &ctx);
>>+ if (ret)
>>+ goto exit;
>>+
>>+ /* translate option's output into sysfs output */
>>+ switch (option->type) {
>>+ case TEAM_OPTION_TYPE_U32:
>>+ ret = scnprintf(buf, PAGE_SIZE, "%u\n", ctx.data.u32_val);
>>+ break;
>>+ case TEAM_OPTION_TYPE_STRING:
>>+ ret = scnprintf(buf, PAGE_SIZE, "%s\n", ctx.data.str_val);
>>+ break;
>>+ case TEAM_OPTION_TYPE_BINARY:
>>+ if (ctx.data.bin_val.len > PAGE_SIZE) {
>>+ netdev_err(team->dev,
>>+ "Option output is too long (%d)\n",
>>+ ctx.data.bin_val.len);
>>+ break;
>>+ }
>>+
>>+ memcpy(buf, ctx.data.bin_val.ptr, ctx.data.bin_val.len);
>>+ ret = ctx.data.bin_val.len;
>>+ break;
>>+ case TEAM_OPTION_TYPE_BOOL:
>>+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.bool_val);
>>+ break;
>>+ case TEAM_OPTION_TYPE_S32:
>>+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.s32_val);
>>+ break;
>>+ default:
>>+ BUG();
>>+ }
>>+
>>+exit:
>>+ mutex_unlock(&team->lock);
>>+
>>+ return ret;
>>+}
>>+
>>+static int team_nl_send_event_options_get(struct team *team,
>>+ struct list_head *sel_opt_inst_list);
>>+
>>+static ssize_t set_attribute(struct device *dev,
>>+ struct device_attribute *attr,
>>+ const char *buf, size_t count)
>>+{
>>+ struct team_option_inst *opt_inst;
>>+ struct team *team = dev_get_drvdata(dev);
>>+ int err = 0;
>>+ struct team_option *option = NULL;
>>+ struct team_gsetter_ctx ctx;
>>+
>>+ LIST_HEAD(opt_inst_list);
>>+
>>+ if (mutex_lock_interruptible(&team->lock))
>>+ return -ERESTARTSYS;
>>+
>>+ opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>+ option = opt_inst->option;
>>+ if (!option->setter) {
>>+ netdev_err(team->dev,
>>+ "Option %s is read only\n", attr->attr.name);
>>+ err = -EOPNOTSUPP;
>>+ goto exit;
>>+ }
>>+
>>+ ctx.info = &opt_inst->info;
>>+
>>+ /* translate sysfs input into option's input */
>>+ switch (option->type) {
>>+ case TEAM_OPTION_TYPE_U32:
>>+ err = kstrtoint(buf, 0, &ctx.data.u32_val);
>>+ break;
>>+ case TEAM_OPTION_TYPE_STRING:
>>+ if (count > TEAM_STRING_MAX_LEN) {
>>+ netdev_err(team->dev,
>>+ "Input buffer too long (%zu)\n", count);
>>+ err = -EINVAL;
>>+ break;
>>+ }
>>+ ctx.data.str_val = buf;
>>+ break;
>>+ case TEAM_OPTION_TYPE_BINARY:
>>+ ctx.data.bin_val.len = count;
>>+ ctx.data.bin_val.ptr = buf;
>>+ break;
>>+ case TEAM_OPTION_TYPE_BOOL:
>>+ err = strtobool(buf, &ctx.data.bool_val);
>>+ break;
>>+ case TEAM_OPTION_TYPE_S32:
>>+ err = kstrtoint(buf, 0, &ctx.data.s32_val);
>>+ break;
>>+ default:
>>+ BUG();
>>+ }
>>+
>>+ if (err) {
>>+ netdev_err(team->dev, "Failed to translate input buffer\n");
>>+ goto exit;
>>+ }
>>+
>>+ /* let the option setter do its job */
>>+ err = option->setter(team, &ctx);
>>+ if (err)
>>+ goto exit;
>>+
>>+ /* propagate option changed event */
>>+ opt_inst->changed = true;
>>+ list_add(&opt_inst->tmp_list, &opt_inst_list);
>>+ err = team_nl_send_event_options_get(team, &opt_inst_list);
>>+ if (err == -ESRCH) /* no listeners, not a real error */
>>+ err = 0;
>>+
>>+exit:
>>+ mutex_unlock(&team->lock);
>>+
>>+ if (!err)
>>+ err = count;
>>+ return err;
>>+}
>>+
>>+/* create sysfs attribute for each option that is being registered */
>>+static int __team_option_add_sysfs_attr(struct team *team,
>>+ struct team_option_inst *opt_inst,
>>+ bool create_sysfs_file)
>>+{
>>+ int err = 0;
>>+ struct device_attribute *new_dev_attr = &opt_inst->dev_attr;
>>+
>>+ new_dev_attr->attr.name = opt_inst->option->name;
>>+ new_dev_attr->attr.mode = S_IRUGO | S_IWUSR;
>>+ new_dev_attr->show = show_attribute;
>>+ new_dev_attr->store = set_attribute;
>>+
>>+ if (create_sysfs_file) {
>>+ err = sysfs_create_file(&team->dev->dev.kobj,
>>+ &new_dev_attr->attr);
>>+ if (err)
>>+ netdev_err(team->dev,
>>+ "Failed to create sysfs attribute %s\n",
>>+ new_dev_attr->attr.name);
>>+ }
>>+
>>+ return err;
>>+}
>>+
>>+static void __team_option_del_sysfs_attr(struct team *team,
>>+ struct team_option_inst *opt_inst)
>>+{
>>+ if (opt_inst->dev_attr_file_exist)
>>+ sysfs_remove_file(&team->dev->dev.kobj,
>>+ &opt_inst->dev_attr.attr);
>>+}
>>+
>> static struct team_option *__team_find_option(struct team *team,
>> const char *opt_name)
>> {
>>@@ -124,8 +314,10 @@ static struct team_option *__team_find_option(struct team *team,
>> return NULL;
>> }
>>
>>-static void __team_option_inst_del(struct team_option_inst *opt_inst)
>>+static void __team_option_inst_del(struct team *team,
>>+ struct team_option_inst *opt_inst)
>> {
>>+ __team_option_del_sysfs_attr(team, opt_inst);
>> list_del(&opt_inst->list);
>> kfree(opt_inst);
>> }
>>@@ -137,7 +329,7 @@ static void __team_option_inst_del_option(struct team *team,
>>
>> list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>> if (opt_inst->option == option)
>>- __team_option_inst_del(opt_inst);
>>+ __team_option_inst_del(team, opt_inst);
>> }
>> }
>>
>>@@ -162,6 +354,7 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>> opt_inst->info.array_index = i;
>> opt_inst->changed = true;
>> opt_inst->removed = false;
>>+ opt_inst->dev_attr_file_exist = false;
>> list_add_tail(&opt_inst->list, &team->option_inst_list);
>> if (option->init) {
>> err = option->init(team, &opt_inst->info);
>>@@ -170,6 +363,20 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>> }
>>
>> }
>>+
>>+ /* add sysfs attribute. per-port and array options are skipped */
>>+ if (!option->per_port && !option->array_size) {
>>+ /* create the sysfs file only if our state allows it */
>>+ bool create_sysfs_file = device_is_registered(&team->dev->dev);
>>+
>>+ err = __team_option_add_sysfs_attr(team, opt_inst,
>>+ create_sysfs_file);
>>+ if (err)
>>+ return err;
>>+
>>+ opt_inst->dev_attr_file_exist = true;
>>+ }
>>+
>> return 0;
>> }
>>
>>@@ -218,7 +425,7 @@ static void __team_option_inst_del_port(struct team *team,
>> list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>> if (opt_inst->option->per_port &&
>> opt_inst->info.port == port)
>>- __team_option_inst_del(opt_inst);
>>+ __team_option_inst_del(team, opt_inst);
>> }
>> }
>>
>>@@ -337,6 +544,51 @@ static void __team_options_unregister(struct team *team,
>>
>> static void __team_options_change_check(struct team *team);
>>
>>+static void team_attr_grp_free(struct team *team)
>>+{
>>+ kfree(team->attr_grp.attrs);
>>+}
>>+
>>+/* allocate attribute group for creating sysfs for team's own options */
>>+static int team_attr_grp_alloc(struct team *team)
>>+{
>>+ struct attribute **attributes;
>>+ struct team_option_inst *opt_inst;
>>+ int num_attr = 0;
>>+ struct team_option *option;
>>+
>>+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>+ option = opt_inst->option;
>>+ /* per-port and array options currently not supported as
>>+ * sysfs attributes
>>+ */
>>+ if (option->per_port || option->array_size)
>>+ continue;
>>+
>>+ num_attr++;
>>+ }
>>+
>>+ /* +1 for having NULL as last item in the array */
>>+ attributes = kzalloc((num_attr + 1) * sizeof(*attributes), GFP_KERNEL);
>>+ if (!attributes)
>>+ return -ENOMEM;
>>+ team->attr_grp.attrs = attributes;
>>+
>>+ num_attr = 0;
>>+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>+ option = opt_inst->option;
>>+ /* per-port and array options currently not supported as
>>+ * sysfs attributes
>>+ */
>>+ if (option->per_port || option->array_size)
>>+ continue;
>>+
>>+ attributes[num_attr++] = &opt_inst->dev_attr.attr;
>>+ }
>>+
>>+ return 0;
>>+}
>>+
>> int team_options_register(struct team *team,
>> const struct team_option *option,
>> size_t option_count)
>>@@ -1380,15 +1632,28 @@ static int team_init(struct net_device *dev)
>>
>> INIT_LIST_HEAD(&team->option_list);
>> INIT_LIST_HEAD(&team->option_inst_list);
>>- err = team_options_register(team, team_options, ARRAY_SIZE(team_options));
>>+
>>+ err = team_options_register(team, team_options,
>>+ ARRAY_SIZE(team_options));
>> if (err)
>> goto err_options_register;
>> netif_carrier_off(dev);
>>
>> team_set_lockdep_class(dev);
>>
>>+ /* store team context, to be used by Device attributes getter/setter */
>>+ dev_set_drvdata(&dev->dev, team);
>>+
>>+ /* allocate and register sysfs attributes for team's own options */
>>+ err = team_attr_grp_alloc(team);
>>+ if (err)
>>+ goto err_grp_alloc;
>>+ dev->sysfs_groups[0] = &team->attr_grp;
>>+
>> return 0;
>>
>>+err_grp_alloc:
>>+ team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>> err_options_register:
>> team_queue_override_fini(team);
>> err_team_queue_override_init:
>>@@ -1407,9 +1672,15 @@ static void team_uninit(struct net_device *dev)
>> list_for_each_entry_safe(port, tmp, &team->port_list, list)
>> team_port_del(team, port->dev);
>>
>>+ sysfs_remove_group(&team->dev->dev.kobj, &team->attr_grp);
>>+ team_attr_grp_free(team);
>>+ /* set to dummy group to avoid double free by core */
>>+ dev->sysfs_groups[0] = &team_sysfs_attr_group;
>>+
>> __team_change_mode(team, NULL); /* cleanup */
>> __team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>> team_queue_override_fini(team);
>>+
>> mutex_unlock(&team->lock);
>> }
>>
>>@@ -2194,9 +2465,6 @@ static int team_nl_cmd_options_get(struct sk_buff *skb, struct genl_info *info)
>> return err;
>> }
>>
>>-static int team_nl_send_event_options_get(struct team *team,
>>- struct list_head *sel_opt_inst_list);
>>-
>> static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
>> {
>> struct team *team;
>>diff --git a/include/linux/if_team.h b/include/linux/if_team.h
>>index 25b8b15..2e9fb2a 100644
>>--- a/include/linux/if_team.h
>>+++ b/include/linux/if_team.h
>>@@ -188,6 +188,9 @@ struct team {
>> struct list_head option_list;
>> struct list_head option_inst_list; /* list of option instances */
>>
>>+ /* attribute group for registering team's own options at init */
>>+ struct attribute_group attr_grp;
>>+
>> const struct team_mode *mode;
>> struct team_mode_ops ops;
>> bool user_carrier_enabled;
>>--
>>1.8.5.2
>>--
>>Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
>>The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>>a Linux Foundation Collaborative Project
>>
>
^ permalink raw reply
* Re: [PATCH] net: team: expose sysfs attributes for each team option
From: Jiri Pirko @ 2014-11-19 12:40 UTC (permalink / raw)
To: Hamad Kadmany; +Cc: netdev
In-Reply-To: <03067d01f18e264b30f86d0307d11b07.squirrel@www.codeaurora.org>
Wed, Nov 19, 2014 at 01:28:19PM CET, hkadmany@codeaurora.org wrote:
>On Mon, November 17, 2014 1:02 pm, Jiri Pirko wrote:
>>
>> Nak.
>>
>> I don't like this patch. The plan for team was keep things simple and to
>> have single userspace api using netlink, as that is the correct way to
>> deal with things. Sysfs for this use-case is not. Please, use netlink api.
>
>Using team driver requires using libnl3. In Android based platforms, libnl3 is not supported, and libnl3 license poses constraints in Android's user-space.
>
>This is the reason for the this change, to have at least the team options exposed through sysfs.
No, that is certainly not the reason. If libnl3 does not suit you, use a
different lib. It is in my opinion unacceptaptable to work around
certain licence issues by adding parallel sysfs api (not to mention that
sysfs is totally unsuitable for that).
You should use existing Netlink API. That's it.
>
>
>>
>>>
>>>Signed-off-by: Hamad Kadmany <hkadmany@codeaurora.org>
>>>---
>>> drivers/net/team/team.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++--
>>> include/linux/if_team.h | 3 +
>>> 2 files changed, 278 insertions(+), 7 deletions(-)
>>>
>>>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>>>index 1222229..afd2f8f 100644
>>>--- a/drivers/net/team/team.c
>>>+++ b/drivers/net/team/team.c
>>>@@ -110,8 +110,198 @@ struct team_option_inst { /* One for each option instance */
>>> struct team_option_inst_info info;
>>> bool changed;
>>> bool removed;
>>>+ bool dev_attr_file_exist;
>>>+ struct device_attribute dev_attr; /* corresponding sysfs attribute */
>>> };
>>>
>>>+static struct attribute *team_sysfs_attrs[] = {
>>>+ NULL,
>>>+};
>>>+
>>>+static struct attribute_group team_sysfs_attr_group = {
>>>+ .attrs = team_sysfs_attrs,
>>>+};
>>>+
>>>+/* Device attributes (sysfs) */
>>>+
>>>+static ssize_t show_attribute(struct device *dev,
>>>+ struct device_attribute *attr,
>>>+ char *buf)
>>>+{
>>>+ struct team *team = dev_get_drvdata(dev);
>>>+ struct team_option_inst *opt_inst;
>>>+ ssize_t ret;
>>>+ struct team_option *option;
>>>+ struct team_gsetter_ctx ctx;
>>>+
>>>+ if (mutex_lock_interruptible(&team->lock))
>>>+ return -ERESTARTSYS;
>>>+
>>>+ opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>>+ option = opt_inst->option;
>>>+ if (!option->getter) {
>>>+ ret = -EOPNOTSUPP;
>>>+ netdev_err(team->dev,
>>>+ "Option %s is write only\n", attr->attr.name);
>>>+ goto exit;
>>>+ }
>>>+
>>>+ ctx.info = &opt_inst->info;
>>>+ /* let the option getter do its job */
>>>+ ret = option->getter(team, &ctx);
>>>+ if (ret)
>>>+ goto exit;
>>>+
>>>+ /* translate option's output into sysfs output */
>>>+ switch (option->type) {
>>>+ case TEAM_OPTION_TYPE_U32:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%u\n", ctx.data.u32_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_STRING:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%s\n", ctx.data.str_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BINARY:
>>>+ if (ctx.data.bin_val.len > PAGE_SIZE) {
>>>+ netdev_err(team->dev,
>>>+ "Option output is too long (%d)\n",
>>>+ ctx.data.bin_val.len);
>>>+ break;
>>>+ }
>>>+
>>>+ memcpy(buf, ctx.data.bin_val.ptr, ctx.data.bin_val.len);
>>>+ ret = ctx.data.bin_val.len;
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BOOL:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.bool_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_S32:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.s32_val);
>>>+ break;
>>>+ default:
>>>+ BUG();
>>>+ }
>>>+
>>>+exit:
>>>+ mutex_unlock(&team->lock);
>>>+
>>>+ return ret;
>>>+}
>>>+
>>>+static int team_nl_send_event_options_get(struct team *team,
>>>+ struct list_head *sel_opt_inst_list);
>>>+
>>>+static ssize_t set_attribute(struct device *dev,
>>>+ struct device_attribute *attr,
>>>+ const char *buf, size_t count)
>>>+{
>>>+ struct team_option_inst *opt_inst;
>>>+ struct team *team = dev_get_drvdata(dev);
>>>+ int err = 0;
>>>+ struct team_option *option = NULL;
>>>+ struct team_gsetter_ctx ctx;
>>>+
>>>+ LIST_HEAD(opt_inst_list);
>>>+
>>>+ if (mutex_lock_interruptible(&team->lock))
>>>+ return -ERESTARTSYS;
>>>+
>>>+ opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>>+ option = opt_inst->option;
>>>+ if (!option->setter) {
>>>+ netdev_err(team->dev,
>>>+ "Option %s is read only\n", attr->attr.name);
>>>+ err = -EOPNOTSUPP;
>>>+ goto exit;
>>>+ }
>>>+
>>>+ ctx.info = &opt_inst->info;
>>>+
>>>+ /* translate sysfs input into option's input */
>>>+ switch (option->type) {
>>>+ case TEAM_OPTION_TYPE_U32:
>>>+ err = kstrtoint(buf, 0, &ctx.data.u32_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_STRING:
>>>+ if (count > TEAM_STRING_MAX_LEN) {
>>>+ netdev_err(team->dev,
>>>+ "Input buffer too long (%zu)\n", count);
>>>+ err = -EINVAL;
>>>+ break;
>>>+ }
>>>+ ctx.data.str_val = buf;
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BINARY:
>>>+ ctx.data.bin_val.len = count;
>>>+ ctx.data.bin_val.ptr = buf;
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BOOL:
>>>+ err = strtobool(buf, &ctx.data.bool_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_S32:
>>>+ err = kstrtoint(buf, 0, &ctx.data.s32_val);
>>>+ break;
>>>+ default:
>>>+ BUG();
>>>+ }
>>>+
>>>+ if (err) {
>>>+ netdev_err(team->dev, "Failed to translate input buffer\n");
>>>+ goto exit;
>>>+ }
>>>+
>>>+ /* let the option setter do its job */
>>>+ err = option->setter(team, &ctx);
>>>+ if (err)
>>>+ goto exit;
>>>+
>>>+ /* propagate option changed event */
>>>+ opt_inst->changed = true;
>>>+ list_add(&opt_inst->tmp_list, &opt_inst_list);
>>>+ err = team_nl_send_event_options_get(team, &opt_inst_list);
>>>+ if (err == -ESRCH) /* no listeners, not a real error */
>>>+ err = 0;
>>>+
>>>+exit:
>>>+ mutex_unlock(&team->lock);
>>>+
>>>+ if (!err)
>>>+ err = count;
>>>+ return err;
>>>+}
>>>+
>>>+/* create sysfs attribute for each option that is being registered */
>>>+static int __team_option_add_sysfs_attr(struct team *team,
>>>+ struct team_option_inst *opt_inst,
>>>+ bool create_sysfs_file)
>>>+{
>>>+ int err = 0;
>>>+ struct device_attribute *new_dev_attr = &opt_inst->dev_attr;
>>>+
>>>+ new_dev_attr->attr.name = opt_inst->option->name;
>>>+ new_dev_attr->attr.mode = S_IRUGO | S_IWUSR;
>>>+ new_dev_attr->show = show_attribute;
>>>+ new_dev_attr->store = set_attribute;
>>>+
>>>+ if (create_sysfs_file) {
>>>+ err = sysfs_create_file(&team->dev->dev.kobj,
>>>+ &new_dev_attr->attr);
>>>+ if (err)
>>>+ netdev_err(team->dev,
>>>+ "Failed to create sysfs attribute %s\n",
>>>+ new_dev_attr->attr.name);
>>>+ }
>>>+
>>>+ return err;
>>>+}
>>>+
>>>+static void __team_option_del_sysfs_attr(struct team *team,
>>>+ struct team_option_inst *opt_inst)
>>>+{
>>>+ if (opt_inst->dev_attr_file_exist)
>>>+ sysfs_remove_file(&team->dev->dev.kobj,
>>>+ &opt_inst->dev_attr.attr);
>>>+}
>>>+
>>> static struct team_option *__team_find_option(struct team *team,
>>> const char *opt_name)
>>> {
>>>@@ -124,8 +314,10 @@ static struct team_option *__team_find_option(struct team *team,
>>> return NULL;
>>> }
>>>
>>>-static void __team_option_inst_del(struct team_option_inst *opt_inst)
>>>+static void __team_option_inst_del(struct team *team,
>>>+ struct team_option_inst *opt_inst)
>>> {
>>>+ __team_option_del_sysfs_attr(team, opt_inst);
>>> list_del(&opt_inst->list);
>>> kfree(opt_inst);
>>> }
>>>@@ -137,7 +329,7 @@ static void __team_option_inst_del_option(struct team *team,
>>>
>>> list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>>> if (opt_inst->option == option)
>>>- __team_option_inst_del(opt_inst);
>>>+ __team_option_inst_del(team, opt_inst);
>>> }
>>> }
>>>
>>>@@ -162,6 +354,7 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>>> opt_inst->info.array_index = i;
>>> opt_inst->changed = true;
>>> opt_inst->removed = false;
>>>+ opt_inst->dev_attr_file_exist = false;
>>> list_add_tail(&opt_inst->list, &team->option_inst_list);
>>> if (option->init) {
>>> err = option->init(team, &opt_inst->info);
>>>@@ -170,6 +363,20 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>>> }
>>>
>>> }
>>>+
>>>+ /* add sysfs attribute. per-port and array options are skipped */
>>>+ if (!option->per_port && !option->array_size) {
>>>+ /* create the sysfs file only if our state allows it */
>>>+ bool create_sysfs_file = device_is_registered(&team->dev->dev);
>>>+
>>>+ err = __team_option_add_sysfs_attr(team, opt_inst,
>>>+ create_sysfs_file);
>>>+ if (err)
>>>+ return err;
>>>+
>>>+ opt_inst->dev_attr_file_exist = true;
>>>+ }
>>>+
>>> return 0;
>>> }
>>>
>>>@@ -218,7 +425,7 @@ static void __team_option_inst_del_port(struct team *team,
>>> list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>>> if (opt_inst->option->per_port &&
>>> opt_inst->info.port == port)
>>>- __team_option_inst_del(opt_inst);
>>>+ __team_option_inst_del(team, opt_inst);
>>> }
>>> }
>>>
>>>@@ -337,6 +544,51 @@ static void __team_options_unregister(struct team *team,
>>>
>>> static void __team_options_change_check(struct team *team);
>>>
>>>+static void team_attr_grp_free(struct team *team)
>>>+{
>>>+ kfree(team->attr_grp.attrs);
>>>+}
>>>+
>>>+/* allocate attribute group for creating sysfs for team's own options */
>>>+static int team_attr_grp_alloc(struct team *team)
>>>+{
>>>+ struct attribute **attributes;
>>>+ struct team_option_inst *opt_inst;
>>>+ int num_attr = 0;
>>>+ struct team_option *option;
>>>+
>>>+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>>+ option = opt_inst->option;
>>>+ /* per-port and array options currently not supported as
>>>+ * sysfs attributes
>>>+ */
>>>+ if (option->per_port || option->array_size)
>>>+ continue;
>>>+
>>>+ num_attr++;
>>>+ }
>>>+
>>>+ /* +1 for having NULL as last item in the array */
>>>+ attributes = kzalloc((num_attr + 1) * sizeof(*attributes), GFP_KERNEL);
>>>+ if (!attributes)
>>>+ return -ENOMEM;
>>>+ team->attr_grp.attrs = attributes;
>>>+
>>>+ num_attr = 0;
>>>+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>>+ option = opt_inst->option;
>>>+ /* per-port and array options currently not supported as
>>>+ * sysfs attributes
>>>+ */
>>>+ if (option->per_port || option->array_size)
>>>+ continue;
>>>+
>>>+ attributes[num_attr++] = &opt_inst->dev_attr.attr;
>>>+ }
>>>+
>>>+ return 0;
>>>+}
>>>+
>>> int team_options_register(struct team *team,
>>> const struct team_option *option,
>>> size_t option_count)
>>>@@ -1380,15 +1632,28 @@ static int team_init(struct net_device *dev)
>>>
>>> INIT_LIST_HEAD(&team->option_list);
>>> INIT_LIST_HEAD(&team->option_inst_list);
>>>- err = team_options_register(team, team_options, ARRAY_SIZE(team_options));
>>>+
>>>+ err = team_options_register(team, team_options,
>>>+ ARRAY_SIZE(team_options));
>>> if (err)
>>> goto err_options_register;
>>> netif_carrier_off(dev);
>>>
>>> team_set_lockdep_class(dev);
>>>
>>>+ /* store team context, to be used by Device attributes getter/setter */
>>>+ dev_set_drvdata(&dev->dev, team);
>>>+
>>>+ /* allocate and register sysfs attributes for team's own options */
>>>+ err = team_attr_grp_alloc(team);
>>>+ if (err)
>>>+ goto err_grp_alloc;
>>>+ dev->sysfs_groups[0] = &team->attr_grp;
>>>+
>>> return 0;
>>>
>>>+err_grp_alloc:
>>>+ team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>>> err_options_register:
>>> team_queue_override_fini(team);
>>> err_team_queue_override_init:
>>>@@ -1407,9 +1672,15 @@ static void team_uninit(struct net_device *dev)
>>> list_for_each_entry_safe(port, tmp, &team->port_list, list)
>>> team_port_del(team, port->dev);
>>>
>>>+ sysfs_remove_group(&team->dev->dev.kobj, &team->attr_grp);
>>>+ team_attr_grp_free(team);
>>>+ /* set to dummy group to avoid double free by core */
>>>+ dev->sysfs_groups[0] = &team_sysfs_attr_group;
>>>+
>>> __team_change_mode(team, NULL); /* cleanup */
>>> __team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>>> team_queue_override_fini(team);
>>>+
>>> mutex_unlock(&team->lock);
>>> }
>>>
>>>@@ -2194,9 +2465,6 @@ static int team_nl_cmd_options_get(struct sk_buff *skb, struct genl_info *info)
>>> return err;
>>> }
>>>
>>>-static int team_nl_send_event_options_get(struct team *team,
>>>- struct list_head *sel_opt_inst_list);
>>>-
>>> static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
>>> {
>>> struct team *team;
>>>diff --git a/include/linux/if_team.h b/include/linux/if_team.h
>>>index 25b8b15..2e9fb2a 100644
>>>--- a/include/linux/if_team.h
>>>+++ b/include/linux/if_team.h
>>>@@ -188,6 +188,9 @@ struct team {
>>> struct list_head option_list;
>>> struct list_head option_inst_list; /* list of option instances */
>>>
>>>+ /* attribute group for registering team's own options at init */
>>>+ struct attribute_group attr_grp;
>>>+
>>> const struct team_mode *mode;
>>> struct team_mode_ops ops;
>>> bool user_carrier_enabled;
>>>--
>>>1.8.5.2
>>>--
>>>Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
>>>The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>>>a Linux Foundation Collaborative Project
>>>
>>
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox