* [PATCH] rhashtable: remove duplicated include from rhashtable.c
From: Yue Haibing @ 2018-08-21 1:41 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: Yue Haibing, netdev, kernel-janitors
Remove duplicated include.
Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
---
lib/rhashtable.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index ae4223e..672eecd 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -28,7 +28,6 @@
#include <linux/rhashtable.h>
#include <linux/err.h>
#include <linux/export.h>
-#include <linux/rhashtable.h>
#define HASH_DEFAULT_SIZE 64UL
#define HASH_MIN_SIZE 4U
^ permalink raw reply related
* Re: [PATCH net] net/ipv6: Put lwtstate when destroying fib6_info
From: David Miller @ 2018-08-21 1:19 UTC (permalink / raw)
To: dsahern; +Cc: netdev, dsahern
In-Reply-To: <20180820200241.19876-1-dsahern@kernel.org>
From: dsahern@kernel.org
Date: Mon, 20 Aug 2018 13:02:41 -0700
> From: David Ahern <dsahern@gmail.com>
>
> Prior to the introduction of fib6_info lwtstate was managed by the dst
> code. With fib6_info releasing lwtstate needs to be done when the struct
> is freed.
>
> Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> Signed-off-by: David Ahern <dsahern@gmail.com>
Applied and queued up for -stable, thanks David.
^ permalink raw reply
* Re: [PATCH iproute2] iproute: make clang happy
From: Stephen Hemminger @ 2018-08-21 0:52 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: Mahesh Bandewar, netdev
In-Reply-To: <CAF2d9jh3xq_F799Dn0=R0XK7NAkrQxmREg++sT4e3MpVLsnPpw@mail.gmail.com>
On Mon, 20 Aug 2018 16:44:28 -0700
Mahesh Bandewar (महेश बंडेवार) <maheshb@google.com> wrote:
> On Mon, Aug 20, 2018 at 4:38 PM, Mahesh Bandewar (महेश बंडेवार)
> <maheshb@google.com> wrote:
> > On Mon, Aug 20, 2018 at 3:52 PM, Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> >> On Mon, 20 Aug 2018 14:42:15 -0700
> >> Mahesh Bandewar <mahesh@bandewar.net> wrote:
> >>
> >>> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
> >>> index ace4b3dd738b..a524b520b276 100644
> >>> --- a/tc/m_ematch.c
> >>> +++ b/tc/m_ematch.c
> >>> @@ -277,6 +277,7 @@ static int flatten_tree(struct ematch *head, struct ematch *tree)
> >>> return count;
> >>> }
> >>>
> >>> +__attribute__((format(printf, 5, 6)))
> >>> int em_parse_error(int err, struct bstr *args, struct bstr *carg,
> >>> struct ematch_util *e, char *fmt, ...)
> >>
> >> I think the printf attribute needs to go on the function prototype
> >> here:
> >> tc/m_ematch.h:extern int em_parse_error(int err, struct bstr *args, struct bstr *carg,
> >>
> > The attributes are attached to the definitions only and not prototype
> > declarations. Please see the definition/declaration for jsonw_printf()
> > in the same patch.
> I take that back. Seems like it's fine either way.
The reason to put the attributes in the .h file is that then the compiler
can test for misuse in other files. For example if em_parse_error had
a bad format in em_u32.c, then the warning would not happen unless
the attribute was on the function prototype.
^ permalink raw reply
* Re: [PATCH net-next v8 7/7] net: vhost: make busyloop_intr more accurate
From: Jason Wang @ 2018-08-21 0:47 UTC (permalink / raw)
To: xiangxia.m.yue, mst, makita.toshiaki; +Cc: netdev, virtualization
In-Reply-To: <f85bfa97-ab9c-2d51-2053-1fe6bb3d45bc@redhat.com>
On 2018年08月21日 08:33, Jason Wang wrote:
>
>
> On 2018年08月19日 20:11, xiangxia.m.yue@gmail.com wrote:
>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>
>> The patch uses vhost_has_work_pending() to check if
>> the specified handler is scheduled, because in the most case,
>> vhost_has_work() return true when other side handler is added
>> to worker list. Use the vhost_has_work_pending() insead of
>> vhost_has_work().
>>
>> Topology:
>> [Host] ->linux bridge -> tap vhost-net ->[Guest]
>>
>> TCP_STREAM (netperf):
>> * Without the patch: 38035.39 Mbps, 3.37 us mean latency
>> * With the patch: 38409.44 Mbps, 3.34 us mean latency
>
> The improvement is not obvious as last version. Do you imply there's
> some recent changes of vhost that make it faster?
>
I misunderstood the numbers, please ignore this.
It shows less than 1% improvement. I'm not sure it's worth to do so. Can
you try bi-directional pktgen to see if it has more obvious effect?
Thanks
> Thanks
>
>>
>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> ---
>> drivers/vhost/net.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index db63ae2..b6939ef 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -487,10 +487,8 @@ static void vhost_net_busy_poll(struct vhost_net
>> *net,
>> endtime = busy_clock() + busyloop_timeout;
>> while (vhost_can_busy_poll(endtime)) {
>> - if (vhost_has_work(&net->dev)) {
>> - *busyloop_intr = true;
>> + if (vhost_has_work(&net->dev))
>> break;
>> - }
>> if ((sock_has_rx_data(sock) &&
>> !vhost_vq_avail_empty(&net->dev, rvq)) ||
>> @@ -513,6 +511,11 @@ static void vhost_net_busy_poll(struct vhost_net
>> *net,
>> !vhost_has_work_pending(&net->dev, VHOST_NET_VQ_RX))
>> vhost_net_enable_vq(net, rvq);
>> + if (vhost_has_work_pending(&net->dev,
>> + poll_rx ?
>> + VHOST_NET_VQ_RX: VHOST_NET_VQ_TX))
>> + *busyloop_intr = true;
>> +
>> mutex_unlock(&vq->mutex);
>> }
>
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next v8 5/7] net: vhost: introduce bitmap for vhost_poll
From: Jason Wang @ 2018-08-21 0:45 UTC (permalink / raw)
To: xiangxia.m.yue, mst, makita.toshiaki; +Cc: virtualization, netdev
In-Reply-To: <1534680686-3108-6-git-send-email-xiangxia.m.yue@gmail.com>
On 2018年08月19日 20:11, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> The bitmap of vhost_dev can help us to check if the
> specified poll is scheduled. This patch will be used
> for next two patches.
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/vhost/net.c | 11 +++++++++--
> drivers/vhost/vhost.c | 17 +++++++++++++++--
> drivers/vhost/vhost.h | 7 ++++++-
> 3 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 1eff72d..23d7ffc 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -1135,8 +1135,15 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> }
> vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX);
>
> - vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, EPOLLOUT, dev);
> - vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, EPOLLIN, dev);
> + vhost_poll_init(n->poll + VHOST_NET_VQ_TX,
> + handle_tx_net,
> + VHOST_NET_VQ_TX,
> + EPOLLOUT, dev);
> +
> + vhost_poll_init(n->poll + VHOST_NET_VQ_RX,
> + handle_rx_net,
> + VHOST_NET_VQ_RX,
> + EPOLLIN, dev);
>
> f->private_data = n;
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index a1c06e7..dc88a60 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -186,7 +186,7 @@ void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn)
>
> /* Init poll structure */
> void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> - __poll_t mask, struct vhost_dev *dev)
> + __u8 poll_id, __poll_t mask, struct vhost_dev *dev)
> {
> init_waitqueue_func_entry(&poll->wait, vhost_poll_wakeup);
> init_poll_funcptr(&poll->table, vhost_poll_func);
> @@ -194,6 +194,7 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> poll->dev = dev;
> poll->wqh = NULL;
>
> + poll->poll_id = poll_id;
> vhost_work_init(&poll->work, fn);
> }
> EXPORT_SYMBOL_GPL(vhost_poll_init);
> @@ -276,8 +277,16 @@ bool vhost_has_work(struct vhost_dev *dev)
> }
> EXPORT_SYMBOL_GPL(vhost_has_work);
>
> +bool vhost_has_work_pending(struct vhost_dev *dev, int poll_id)
> +{
> + return !llist_empty(&dev->work_list) &&
> + test_bit(poll_id, dev->work_pending);
I think we've already had something similar. E.g can we test
VHOST_WORK_QUEUED instead?
Thanks
> +}
> +EXPORT_SYMBOL_GPL(vhost_has_work_pending);
> +
> void vhost_poll_queue(struct vhost_poll *poll)
> {
> + set_bit(poll->poll_id, poll->dev->work_pending);
> vhost_work_queue(poll->dev, &poll->work);
> }
> EXPORT_SYMBOL_GPL(vhost_poll_queue);
> @@ -354,6 +363,7 @@ static int vhost_worker(void *data)
> if (!node)
> schedule();
>
> + bitmap_zero(dev->work_pending, VHOST_DEV_MAX_VQ);
> node = llist_reverse_order(node);
> /* make sure flag is seen after deletion */
> smp_wmb();
> @@ -420,6 +430,8 @@ void vhost_dev_init(struct vhost_dev *dev,
> struct vhost_virtqueue *vq;
> int i;
>
> + BUG_ON(nvqs > VHOST_DEV_MAX_VQ);
> +
> dev->vqs = vqs;
> dev->nvqs = nvqs;
> mutex_init(&dev->mutex);
> @@ -428,6 +440,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> dev->iotlb = NULL;
> dev->mm = NULL;
> dev->worker = NULL;
> + bitmap_zero(dev->work_pending, VHOST_DEV_MAX_VQ);
> init_llist_head(&dev->work_list);
> init_waitqueue_head(&dev->wait);
> INIT_LIST_HEAD(&dev->read_list);
> @@ -445,7 +458,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> vhost_vq_reset(dev, vq);
> if (vq->handle_kick)
> vhost_poll_init(&vq->poll, vq->handle_kick,
> - EPOLLIN, dev);
> + i, EPOLLIN, dev);
> }
> }
> EXPORT_SYMBOL_GPL(vhost_dev_init);
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 6c844b9..60b6f6d 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -30,6 +30,7 @@ struct vhost_poll {
> wait_queue_head_t *wqh;
> wait_queue_entry_t wait;
> struct vhost_work work;
> + __u8 poll_id;
> __poll_t mask;
> struct vhost_dev *dev;
> };
> @@ -37,9 +38,10 @@ struct vhost_poll {
> void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn);
> void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
> bool vhost_has_work(struct vhost_dev *dev);
> +bool vhost_has_work_pending(struct vhost_dev *dev, int poll_id);
>
> void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> - __poll_t mask, struct vhost_dev *dev);
> + __u8 id, __poll_t mask, struct vhost_dev *dev);
> int vhost_poll_start(struct vhost_poll *poll, struct file *file);
> void vhost_poll_stop(struct vhost_poll *poll);
> void vhost_poll_flush(struct vhost_poll *poll);
> @@ -152,6 +154,8 @@ struct vhost_msg_node {
> struct list_head node;
> };
>
> +#define VHOST_DEV_MAX_VQ 128
> +
> struct vhost_dev {
> struct mm_struct *mm;
> struct mutex mutex;
> @@ -159,6 +163,7 @@ struct vhost_dev {
> int nvqs;
> struct eventfd_ctx *log_ctx;
> struct llist_head work_list;
> + DECLARE_BITMAP(work_pending, VHOST_DEV_MAX_VQ);
> struct task_struct *worker;
> struct vhost_umem *umem;
> struct vhost_umem *iotlb;
^ permalink raw reply
* Re: [PATCH iproute2] iproute: make clang happy
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-08-21 0:33 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Mahesh Bandewar, netdev
In-Reply-To: <CAF2d9jh3xq_F799Dn0=R0XK7NAkrQxmREg++sT4e3MpVLsnPpw@mail.gmail.com>
On Mon, Aug 20, 2018 at 4:44 PM, Mahesh Bandewar (महेश बंडेवार)
<maheshb@google.com> wrote:
> On Mon, Aug 20, 2018 at 4:38 PM, Mahesh Bandewar (महेश बंडेवार)
> <maheshb@google.com> wrote:
>> On Mon, Aug 20, 2018 at 3:52 PM, Stephen Hemminger
>> <stephen@networkplumber.org> wrote:
>>> On Mon, 20 Aug 2018 14:42:15 -0700
>>> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>>>
>>>> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
>>>> index ace4b3dd738b..a524b520b276 100644
>>>> --- a/tc/m_ematch.c
>>>> +++ b/tc/m_ematch.c
>>>> @@ -277,6 +277,7 @@ static int flatten_tree(struct ematch *head, struct ematch *tree)
>>>> return count;
>>>> }
>>>>
>>>> +__attribute__((format(printf, 5, 6)))
>>>> int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>>>> struct ematch_util *e, char *fmt, ...)
>>>
>>> I think the printf attribute needs to go on the function prototype
>>> here:
>>> tc/m_ematch.h:extern int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>>>
>> The attributes are attached to the definitions only and not prototype
>> declarations. Please see the definition/declaration for jsonw_printf()
>> in the same patch.
> I take that back. Seems like it's fine either way.
>>>
>>> PS: I need to take the extern of those function prototypes.
I can take care of this in v2
^ permalink raw reply
* Re: [PATCH net-next v8 7/7] net: vhost: make busyloop_intr more accurate
From: Jason Wang @ 2018-08-21 0:33 UTC (permalink / raw)
To: xiangxia.m.yue, mst, makita.toshiaki; +Cc: virtualization, netdev
In-Reply-To: <1534680686-3108-8-git-send-email-xiangxia.m.yue@gmail.com>
On 2018年08月19日 20:11, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> The patch uses vhost_has_work_pending() to check if
> the specified handler is scheduled, because in the most case,
> vhost_has_work() return true when other side handler is added
> to worker list. Use the vhost_has_work_pending() insead of
> vhost_has_work().
>
> Topology:
> [Host] ->linux bridge -> tap vhost-net ->[Guest]
>
> TCP_STREAM (netperf):
> * Without the patch: 38035.39 Mbps, 3.37 us mean latency
> * With the patch: 38409.44 Mbps, 3.34 us mean latency
The improvement is not obvious as last version. Do you imply there's
some recent changes of vhost that make it faster?
Thanks
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/vhost/net.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index db63ae2..b6939ef 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -487,10 +487,8 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> endtime = busy_clock() + busyloop_timeout;
>
> while (vhost_can_busy_poll(endtime)) {
> - if (vhost_has_work(&net->dev)) {
> - *busyloop_intr = true;
> + if (vhost_has_work(&net->dev))
> break;
> - }
>
> if ((sock_has_rx_data(sock) &&
> !vhost_vq_avail_empty(&net->dev, rvq)) ||
> @@ -513,6 +511,11 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> !vhost_has_work_pending(&net->dev, VHOST_NET_VQ_RX))
> vhost_net_enable_vq(net, rvq);
>
> + if (vhost_has_work_pending(&net->dev,
> + poll_rx ?
> + VHOST_NET_VQ_RX: VHOST_NET_VQ_TX))
> + *busyloop_intr = true;
> +
> mutex_unlock(&vq->mutex);
> }
>
^ permalink raw reply
* Re: [PATCH iproute2] iproute: make clang happy
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-08-21 0:32 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Mahesh Bandewar, netdev
In-Reply-To: <20180820155038.30d0f08c@xeon-e3>
On Mon, Aug 20, 2018 at 3:50 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Mon, 20 Aug 2018 14:42:15 -0700
> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>
>>
>> if (is_json_context()) {
>> + json_writer_t *jw;
>> +
>> open_json_object("bittiming");
>> print_int(PRINT_ANY, "bitrate", NULL, bt->bitrate);
>> - jsonw_float_field_fmt(get_json_writer(),
>> - "sample_point", "%.3f",
>> - (float) bt->sample_point / 1000.);
>> + jw = get_json_writer();
>> + jsonw_name(jw, "sample_point");
>> + jsonw_printf(jw, "%.3f",
>> + (float) bt->sample_point / 1000);
>
> I think it would be better to get rid of the is_json_context() here in the CAN code
> and just use the print_json functions completely. Most of the other code is able to
> do that already.
Seems like this is not the only location and need to be taken care
every where in the CAN code. I'll leave it to some JSON / print expert
^ permalink raw reply
* Re: [PATCH net-next,v4] net/tls: Calculate nsg for zerocopy path without skb_cow_data.
From: David Miller @ 2018-08-21 0:30 UTC (permalink / raw)
To: doronrk; +Cc: davejwatson, vakul.garg, borisp, aviadye, netdev
In-Reply-To: <20180821002723.GA79644@doronrk-mbp>
From: Doron Roberts-Kedes <doronrk@fb.com>
Date: Mon, 20 Aug 2018 17:27:23 -0700
> Given that frag_lists are not unlikely in this case, I believe the only
> remaining feedback on the original patch was the recursive
> implementation. If you'd like, I can re-submit with an iterative
> implementation, but I noticed that goes against the existing recursive
> pattern in functions like skb_release_data -> kfree_skb_list -> kfree_skb
> -> __kfree_skb -> skb_release_all -> skb_release_data, as well as
> skb_to_sgvec. Let me know whether an iterative implementation is
> preferred here, or whether I can simply rebase and resubmit a patch
> similar to the original (modulo some variable renaming improvements).
Ok, I guess staying with the recursive implementation is fine.
It's a real shame that frag lists are so common in this code path,
especially nested ones :-/
In the long term, perhaps we can do something about that.
In the short term, I guess this means your original change is OK.
Please resubmit when the net-next tree opens back up, thanks.
^ permalink raw reply
* Re: [PATCH net-next,v4] net/tls: Calculate nsg for zerocopy path without skb_cow_data.
From: Doron Roberts-Kedes @ 2018-08-21 0:27 UTC (permalink / raw)
To: David Miller; +Cc: davejwatson, vakul.garg, borisp, aviadye, netdev
In-Reply-To: <20180811.115453.2016294695634012705.davem@davemloft.net>
On Sat, Aug 11, 2018 at 11:54:53AM -0700, David Miller wrote:
> From: Doron Roberts-Kedes <doronrk@fb.com>
> Date: Thu, 9 Aug 2018 15:43:44 -0700
>
> The reason is that we usually never need to "map" an SKB on receive,
> and on transmit the SKB geometry is normalized by the destination
> device features which means no frag_list.
>
> And the other existing receive path doing SW crypto, IPSEC, always
> COWs the data so get the number of SG array entries needed from
> skb_cow_data().
Makes sense, thanks!
> Frag lists on RX should be pretty rare. They occur when GRO
> segmentation encouters poorly arranged incoming SKBs. For example
> this happens if the incoming frames use lots of tiny SKB page frags,
> and therefore prevent accumulation into the page frag array of the
> head GRO skb.
>
> So yes it can happen, and we have to account for it.
>
> So I guess you really do need to accomodate this situation. Why
> don't you try to rearrange your new function with some likely()
> and unlikely() tests so that the straight code path optimizes for
> the non-frag-list case?
So I did some poking around and found that basically 100% of skb's
recieved by ktls have a frag_list because of how strparser is
implemented. skb's from TCP that do not a have a frag_list are
accumulated by strparser using frag_list of a new skb. skb's from TCP
that do have a frag_list can become part of skb's with nested frag_lists
(skb's with non-NULL frag_list that are themselves part of a frag_list).
Unfortunatley, frag_list seems to be the common case, so is probably not a
good candidate for an unlikely() test.
Regarding nested frag_list's more generally, is a good rule of thumb
that multiple layers of frag_list will not occur except for
post-processing such as in strparser? When should skb-handling code be
prepared for nested frag_lists?
Given that frag_lists are not unlikely in this case, I believe the only
remaining feedback on the original patch was the recursive
implementation. If you'd like, I can re-submit with an iterative
implementation, but I noticed that goes against the existing recursive
pattern in functions like skb_release_data -> kfree_skb_list -> kfree_skb
-> __kfree_skb -> skb_release_all -> skb_release_data, as well as
skb_to_sgvec. Let me know whether an iterative implementation is
preferred here, or whether I can simply rebase and resubmit a patch
similar to the original (modulo some variable renaming improvements).
^ permalink raw reply
* Re: [PATCH net-next v8 3/7] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
From: Jason Wang @ 2018-08-21 3:15 UTC (permalink / raw)
To: xiangxia.m.yue, mst, makita.toshiaki; +Cc: netdev, virtualization
In-Reply-To: <1534680686-3108-4-git-send-email-xiangxia.m.yue@gmail.com>
On 2018年08月19日 20:11, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> Factor out generic busy polling logic and will be
> used for in tx path in the next patch. And with the patch,
> qemu can set differently the busyloop_timeout for rx queue.
>
> To avoid duplicate codes, introduce the helper functions:
> * sock_has_rx_data(changed from sk_has_rx_data)
> * vhost_net_busy_poll_try_queue
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/vhost/net.c | 111 +++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 71 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 32c1b52..453c061 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -440,6 +440,75 @@ static void vhost_net_signal_used(struct vhost_net_virtqueue *nvq)
> nvq->done_idx = 0;
> }
>
> +static int sock_has_rx_data(struct socket *sock)
> +{
> + if (unlikely(!sock))
> + return 0;
> +
> + if (sock->ops->peek_len)
> + return sock->ops->peek_len(sock);
> +
> + return skb_queue_empty(&sock->sk->sk_receive_queue);
> +}
> +
> +static void vhost_net_busy_poll_try_queue(struct vhost_net *net,
> + struct vhost_virtqueue *vq)
> +{
> + if (!vhost_vq_avail_empty(&net->dev, vq)) {
> + vhost_poll_queue(&vq->poll);
> + } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> + vhost_disable_notify(&net->dev, vq);
> + vhost_poll_queue(&vq->poll);
> + }
> +}
> +
> +static void vhost_net_busy_poll(struct vhost_net *net,
> + struct vhost_virtqueue *rvq,
> + struct vhost_virtqueue *tvq,
> + bool *busyloop_intr,
> + bool poll_rx)
> +{
> + unsigned long busyloop_timeout;
> + unsigned long endtime;
> + struct socket *sock;
> + struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
> +
> + mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
> + vhost_disable_notify(&net->dev, vq);
> + sock = rvq->private_data;
> +
> + busyloop_timeout = poll_rx ? rvq->busyloop_timeout:
> + tvq->busyloop_timeout;
> +
> + preempt_disable();
> + endtime = busy_clock() + busyloop_timeout;
> +
> + while (vhost_can_busy_poll(endtime)) {
> + if (vhost_has_work(&net->dev)) {
> + *busyloop_intr = true;
> + break;
> + }
> +
> + if ((sock_has_rx_data(sock) &&
> + !vhost_vq_avail_empty(&net->dev, rvq)) ||
> + !vhost_vq_avail_empty(&net->dev, tvq))
> + break;
> +
> + cpu_relax();
> + }
> +
> + preempt_enable();
> +
> + if (poll_rx)
> + vhost_net_busy_poll_try_queue(net, tvq);
> + else if (sock_has_rx_data(sock))
> + vhost_net_busy_poll_try_queue(net, rvq);
This could be simplified like:
if (poll_rx || sock_has_rx_data(sock))
vhost_net_busy_poll_try_queue(net, vq);
Thanks
> + else /* On tx here, sock has no rx data. */
> + vhost_enable_notify(&net->dev, rvq);
> +
> + mutex_unlock(&vq->mutex);
> +}
> +
> static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> struct vhost_net_virtqueue *nvq,
> unsigned int *out_num, unsigned int *in_num,
> @@ -753,16 +822,6 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
> return len;
> }
>
> -static int sk_has_rx_data(struct sock *sk)
> -{
> - struct socket *sock = sk->sk_socket;
> -
> - if (sock->ops->peek_len)
> - return sock->ops->peek_len(sock);
> -
> - return skb_queue_empty(&sk->sk_receive_queue);
> -}
> -
> static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
> bool *busyloop_intr)
> {
> @@ -770,41 +829,13 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
> struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
> struct vhost_virtqueue *rvq = &rnvq->vq;
> struct vhost_virtqueue *tvq = &tnvq->vq;
> - unsigned long uninitialized_var(endtime);
> int len = peek_head_len(rnvq, sk);
>
> - if (!len && tvq->busyloop_timeout) {
> + if (!len && rvq->busyloop_timeout) {
> /* Flush batched heads first */
> vhost_net_signal_used(rnvq);
> /* Both tx vq and rx socket were polled here */
> - mutex_lock_nested(&tvq->mutex, VHOST_NET_VQ_TX);
> - vhost_disable_notify(&net->dev, tvq);
> -
> - preempt_disable();
> - endtime = busy_clock() + tvq->busyloop_timeout;
> -
> - while (vhost_can_busy_poll(endtime)) {
> - if (vhost_has_work(&net->dev)) {
> - *busyloop_intr = true;
> - break;
> - }
> - if ((sk_has_rx_data(sk) &&
> - !vhost_vq_avail_empty(&net->dev, rvq)) ||
> - !vhost_vq_avail_empty(&net->dev, tvq))
> - break;
> - cpu_relax();
> - }
> -
> - preempt_enable();
> -
> - if (!vhost_vq_avail_empty(&net->dev, tvq)) {
> - vhost_poll_queue(&tvq->poll);
> - } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) {
> - vhost_disable_notify(&net->dev, tvq);
> - vhost_poll_queue(&tvq->poll);
> - }
> -
> - mutex_unlock(&tvq->mutex);
> + vhost_net_busy_poll(net, rvq, tvq, busyloop_intr, true);
>
> len = peek_head_len(rnvq, sk);
> }
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [Patch net 8/9] act_ife: move tcfa_lock down to where necessary
From: Cong Wang @ 2018-08-20 23:57 UTC (permalink / raw)
To: David Miller
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Vlad Buslov
In-Reply-To: <20180820.112952.2052634913677104782.davem@davemloft.net>
On Mon, Aug 20, 2018 at 11:29 AM David Miller <davem@davemloft.net> wrote:
>
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Sun, 19 Aug 2018 12:22:12 -0700
>
> > The only time we need to take tcfa_lock is when adding
> > a new metainfo to an existing ife->metalist. We don't need
> > to take tcfa_lock so early and so broadly in tcf_ife_init().
> >
> > This means we can always take ife_mod_lock first, avoid the
> > reverse locking ordering warning as reported by Vlad.
> >
> > Reported-by: Vlad Buslov <vladbu@mellanox.com>
> > Tested-by: Vlad Buslov <vladbu@mellanox.com>
> > Cc: Vlad Buslov <vladbu@mellanox.com>
> > Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>
> After this change we no longer call populate_metalist() in an atomic
> context via tcf_ife_init(), and populate_metalist passes 'exists'
> down to add_metainfo() as an 'atomic' indicator. It doesn't have this
> meaning if you aren't holding the tcfa_lock in the callers with BH
> disabled.
Passing 'exists' as 'atomic' is prior to my change. With my change,
they are separated as two parameters:
static int add_metainfo(struct tcf_ife_info *ife, u32 metaid, void *metaval,
- int len, bool atomic)
+ int len, bool atomic, bool exists)
Or I misunderstand your point here?
>
> Therefore, add_metainfo()'s 'atomic' indication is inaccurate in this
> call chain and will use GFP_ATOMIC unnecessarily.
>
> Probably the thing to just is just pass 'false' down to add_metainfo()
> in populate_metalist().
This is exactly what this patch does?
static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb,
bool exists, bool rtnl_held)
{
@@ -436,12 +431,11 @@ static int populate_metalist(struct tcf_ife_info
*ife, struct nlattr **tb,
...
- rc = add_metainfo(ife, i, val, len, exists);
+ rc = add_metainfo(ife, i, val, len, false, exists);
^ permalink raw reply
* Re: [PATCH iproute2] iproute: make clang happy
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-08-20 23:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Mahesh Bandewar, netdev
In-Reply-To: <CAF2d9jj5EpLjGgpe94vYj9jt-MJHyZEcNaLOUTtYtibgEBpzww@mail.gmail.com>
On Mon, Aug 20, 2018 at 4:38 PM, Mahesh Bandewar (महेश बंडेवार)
<maheshb@google.com> wrote:
> On Mon, Aug 20, 2018 at 3:52 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
>> On Mon, 20 Aug 2018 14:42:15 -0700
>> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>>
>>> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
>>> index ace4b3dd738b..a524b520b276 100644
>>> --- a/tc/m_ematch.c
>>> +++ b/tc/m_ematch.c
>>> @@ -277,6 +277,7 @@ static int flatten_tree(struct ematch *head, struct ematch *tree)
>>> return count;
>>> }
>>>
>>> +__attribute__((format(printf, 5, 6)))
>>> int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>>> struct ematch_util *e, char *fmt, ...)
>>
>> I think the printf attribute needs to go on the function prototype
>> here:
>> tc/m_ematch.h:extern int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>>
> The attributes are attached to the definitions only and not prototype
> declarations. Please see the definition/declaration for jsonw_printf()
> in the same patch.
I take that back. Seems like it's fine either way.
>>
>> PS: I need to take the extern of those function prototypes.
^ permalink raw reply
* [RFC RFT PATCH v4 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Janusz Krzysztofik @ 2018-08-20 23:43 UTC (permalink / raw)
To: Linus Walleij
Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
David S. Miller, Dominik Brodowski, Kishon Vijay Abraham I,
Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
Jiri Slaby, linux-gpio, linux-doc, linux-i2c
In-Reply-To: <20180820234341.5271-1-jmkrzyszt@gmail.com>
Most users of get/set array functions iterate consecutive bits of data,
usually a single integer, while or processing array of results obtained
from or building an array of values to be passed to those functions.
Save time wasted on those iterations by changing the functions' API to
accept bitmaps.
All current users are updated as well.
More benefits from the change are expected as soon as planned support
for accepting/passing those bitmaps directly from/to respective GPIO
chip callbacks if applicable is implemented.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Documentation/driver-api/gpio/consumer.rst | 22 ++++----
drivers/auxdisplay/hd44780.c | 52 +++++++++--------
drivers/bus/ts-nbus.c | 19 ++-----
drivers/gpio/gpio-max3191x.c | 17 +++---
drivers/gpio/gpiolib.c | 86 +++++++++++++++--------------
drivers/gpio/gpiolib.h | 4 +-
drivers/i2c/muxes/i2c-mux-gpio.c | 3 +-
drivers/mmc/core/pwrseq_simple.c | 13 ++---
drivers/mux/gpio.c | 4 +-
drivers/net/phy/mdio-mux-gpio.c | 3 +-
drivers/pcmcia/soc_common.c | 11 ++--
drivers/phy/motorola/phy-mapphone-mdm6600.c | 17 +++---
drivers/staging/iio/adc/ad7606.c | 9 +--
drivers/tty/serial/serial_mctrl_gpio.c | 7 ++-
include/linux/gpio/consumer.h | 18 +++---
15 files changed, 138 insertions(+), 147 deletions(-)
diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index aa03f389d41d..ed68042ddccf 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -323,29 +323,29 @@ The following functions get or set the values of an array of GPIOs::
int gpiod_get_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
void gpiod_set_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
The array can be an arbitrary set of GPIOs. The functions will try to access
GPIOs belonging to the same bank or chip simultaneously if supported by the
@@ -356,8 +356,8 @@ accessed sequentially.
The functions take three arguments:
* array_size - the number of array elements
* desc_array - an array of GPIO descriptors
- * value_array - an array to store the GPIOs' values (get) or
- an array of values to assign to the GPIOs (set)
+ * value_bitmap - a bitmap to store the GPIOs' values (get) or
+ a bitmap of values to assign to the GPIOs (set)
The descriptor array can be obtained using the gpiod_get_array() function
or one of its variants. If the group of descriptors returned by that function
@@ -366,7 +366,7 @@ the struct gpio_descs returned by gpiod_get_array()::
struct gpio_descs *my_gpio_descs = gpiod_get_array(...);
gpiod_set_array_value(my_gpio_descs->ndescs, my_gpio_descs->desc,
- my_gpio_values);
+ my_gpio_value_bitmap);
It is also possible to access a completely arbitrary array of descriptors. The
descriptors may be obtained using any combination of gpiod_get() and
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index f1a42f0f1ded..d340473aa142 100644
--- a/drivers/auxdisplay/hd44780.c
+++ b/drivers/auxdisplay/hd44780.c
@@ -62,20 +62,19 @@ static void hd44780_strobe_gpio(struct hd44780 *hd)
/* write to an LCD panel register in 8 bit GPIO mode */
static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
{
- int values[10]; /* for DATA[0-7], RS, RW */
- unsigned int i, n;
+ unsigned long value_bitmap[1]; /* for DATA[0-7], RS, RW */
+ unsigned int n;
- for (i = 0; i < 8; i++)
- values[PIN_DATA0 + i] = !!(val & BIT(i));
- values[PIN_CTRL_RS] = rs;
+ value_bitmap[0] = val;
+ __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
n = 9;
if (hd->pins[PIN_CTRL_RW]) {
- values[PIN_CTRL_RW] = 0;
+ __clear_bit(PIN_CTRL_RW, value_bitmap);
n++;
}
/* Present the data to the port */
- gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], values);
+ gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], value_bitmap);
hd44780_strobe_gpio(hd);
}
@@ -83,32 +82,31 @@ static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
/* write to an LCD panel register in 4 bit GPIO mode */
static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int rs)
{
- int values[10]; /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
- unsigned int i, n;
+ /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
+ unsigned long value_bitmap[0];
+ unsigned int n;
/* High nibble + RS, RW */
- for (i = 4; i < 8; i++)
- values[PIN_DATA0 + i] = !!(val & BIT(i));
- values[PIN_CTRL_RS] = rs;
+ value_bitmap[0] = val;
+ __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
n = 5;
if (hd->pins[PIN_CTRL_RW]) {
- values[PIN_CTRL_RW] = 0;
+ __clear_bit(PIN_CTRL_RW, value_bitmap);
n++;
}
+ value_bitmap[0] = value_bitmap[0] >> PIN_DATA4;
/* Present the data to the port */
- gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
- &values[PIN_DATA4]);
+ gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
hd44780_strobe_gpio(hd);
/* Low nibble */
- for (i = 0; i < 4; i++)
- values[PIN_DATA4 + i] = !!(val & BIT(i));
+ value_bitmap[0] &= ~((1 << PIN_DATA4) - 1);
+ value_bitmap[0] |= val & ~((1 << PIN_DATA4) - 1);
/* Present the data to the port */
- gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
- &values[PIN_DATA4]);
+ gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
hd44780_strobe_gpio(hd);
}
@@ -155,23 +153,23 @@ static void hd44780_write_cmd_gpio4(struct charlcd *lcd, int cmd)
/* Send 4-bits of a command to the LCD panel in raw 4 bit GPIO mode */
static void hd44780_write_cmd_raw_gpio4(struct charlcd *lcd, int cmd)
{
- int values[10]; /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
+ /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
+ unsigned long value_bitmap[1];
struct hd44780 *hd = lcd->drvdata;
- unsigned int i, n;
+ unsigned int n;
/* Command nibble + RS, RW */
- for (i = 0; i < 4; i++)
- values[PIN_DATA4 + i] = !!(cmd & BIT(i));
- values[PIN_CTRL_RS] = 0;
+ value_bitmap[0] = cmd << PIN_DATA4;
+ __clear_bit(PIN_CTRL_RS, value_bitmap);
n = 5;
if (hd->pins[PIN_CTRL_RW]) {
- values[PIN_CTRL_RW] = 0;
+ __clear_bit(PIN_CTRL_RW, value_bitmap);
n++;
}
+ value_bitmap[0] = value_bitmap[0] >> PIN_DATA4;
/* Present the data to the port */
- gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
- &values[PIN_DATA4]);
+ gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
hd44780_strobe_gpio(hd);
}
diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index 073fd9011154..ce6c1e89236d 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -110,13 +110,9 @@ static void ts_nbus_set_direction(struct ts_nbus *ts_nbus, int direction)
*/
static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
{
- int i;
- int values[8];
-
- for (i = 0; i < 8; i++)
- values[i] = 0;
+ unsigned long value_bitmap[1] = { 0, };
- gpiod_set_array_value_cansleep(8, ts_nbus->data->desc, values);
+ gpiod_set_array_value_cansleep(8, ts_nbus->data->desc, value_bitmap);
gpiod_set_value_cansleep(ts_nbus->csn, 0);
gpiod_set_value_cansleep(ts_nbus->strobe, 0);
gpiod_set_value_cansleep(ts_nbus->ale, 0);
@@ -157,16 +153,9 @@ static int ts_nbus_read_byte(struct ts_nbus *ts_nbus, u8 *val)
static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
{
struct gpio_descs *gpios = ts_nbus->data;
- int i;
- int values[8];
-
- for (i = 0; i < 8; i++)
- if (byte & BIT(i))
- values[i] = 1;
- else
- values[i] = 0;
+ unsigned long value_bitmap[1] = { byte, };
- gpiod_set_array_value_cansleep(8, gpios->desc, values);
+ gpiod_set_array_value_cansleep(8, gpios->desc, value_bitmap);
}
/*
diff --git a/drivers/gpio/gpio-max3191x.c b/drivers/gpio/gpio-max3191x.c
index b5b9cb1fda50..c4ec1c82af27 100644
--- a/drivers/gpio/gpio-max3191x.c
+++ b/drivers/gpio/gpio-max3191x.c
@@ -315,17 +315,20 @@ static void gpiod_set_array_single_value_cansleep(unsigned int ndescs,
struct gpio_desc **desc,
int value)
{
- int i, *values;
+ unsigned long *value_bitmap;
- values = kmalloc_array(ndescs, sizeof(*values), GFP_KERNEL);
- if (!values)
+ value_bitmap = kmalloc_array(BITS_TO_LONGS(ndescs),
+ sizeof(*value_bitmap), GFP_KERNEL);
+ if (!value_bitmap)
return;
- for (i = 0; i < ndescs; i++)
- values[i] = value;
+ if (value)
+ bitmap_fill(value_bitmap, ndescs);
+ else
+ bitmap_zero(value_bitmap, ndescs);
- gpiod_set_array_value_cansleep(ndescs, desc, values);
- kfree(values);
+ gpiod_set_array_value_cansleep(ndescs, desc, value_bitmap);
+ kfree(value_bitmap);
}
static struct gpio_descs *devm_gpiod_get_array_optional_count(
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e8f8a1999393..f0e9ffa8cab6 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -427,7 +427,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
struct linehandle_state *lh = filep->private_data;
void __user *ip = (void __user *)arg;
struct gpiohandle_data ghd;
- int vals[GPIOHANDLES_MAX];
+ unsigned long value_bitmap[BITS_TO_LONGS(GPIOHANDLES_MAX)];
int i;
if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) {
@@ -436,13 +436,13 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
true,
lh->numdescs,
lh->descs,
- vals);
+ value_bitmap);
if (ret)
return ret;
memset(&ghd, 0, sizeof(ghd));
for (i = 0; i < lh->numdescs; i++)
- ghd.values[i] = vals[i];
+ ghd.values[i] = test_bit(i, value_bitmap);
if (copy_to_user(ip, &ghd, sizeof(ghd)))
return -EFAULT;
@@ -461,14 +461,14 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
/* Clamp all values to [0,1] */
for (i = 0; i < lh->numdescs; i++)
- vals[i] = !!ghd.values[i];
+ __assign_bit(i, value_bitmap, !!ghd.values[i]);
/* Reuse the array setting function */
return gpiod_set_array_value_complex(false,
true,
lh->numdescs,
lh->descs,
- vals);
+ value_bitmap);
}
return -EINVAL;
}
@@ -2784,7 +2784,7 @@ static int gpio_chip_get_multiple(struct gpio_chip *chip,
int gpiod_get_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
int i = 0;
@@ -2835,7 +2835,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags))
value = !value;
- value_array[j] = value;
+ __assign_bit(j, value_bitmap, value);
trace_gpio_value(desc_to_gpio(desc), 1, value);
}
@@ -2895,9 +2895,9 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
/**
* gpiod_get_raw_array_value() - read raw values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitmap: bitmap to store the read values
*
* Read the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status. Return 0 in case of success,
@@ -2907,20 +2907,21 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
* and it will complain if the GPIO chip functions potentially sleep.
*/
int gpiod_get_raw_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(true, false, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
/**
* gpiod_get_array_value() - read values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitnap: bitmap to store the read values
*
* Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account. Return 0 in case of success, else an error code.
@@ -2929,12 +2930,13 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
* and it will complain if the GPIO chip functions potentially sleep.
*/
int gpiod_get_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(false, false, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_array_value);
@@ -3027,7 +3029,7 @@ static void gpio_chip_set_multiple(struct gpio_chip *chip,
int gpiod_set_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
int i = 0;
@@ -3056,7 +3058,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
do {
struct gpio_desc *desc = desc_array[i];
int hwgpio = gpio_chip_hwgpio(desc);
- int value = value_array[i];
+ int value = test_bit(i, value_bitmap);
if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags))
value = !value;
@@ -3152,9 +3154,9 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
/**
* gpiod_set_raw_array_value() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status.
@@ -3163,20 +3165,21 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
* complain if the GPIO chip functions potentially sleep.
*/
int gpiod_set_raw_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return -EINVAL;
return gpiod_set_array_value_complex(true, false, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
/**
* gpiod_set_array_value() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account.
@@ -3185,12 +3188,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
* complain if the GPIO chip functions potentially sleep.
*/
void gpiod_set_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return;
gpiod_set_array_value_complex(false, false, array_size, desc_array,
- value_array);
+ value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_array_value);
@@ -3410,9 +3414,9 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
/**
* gpiod_get_raw_array_value_cansleep() - read raw values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitmap: bitmap to store the read values
*
* Read the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status. Return 0 in case of success,
@@ -3422,21 +3426,21 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
*/
int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(true, true, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
/**
* gpiod_get_array_value_cansleep() - read values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitmap: bitmap to store the read values
*
* Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account. Return 0 in case of success, else an error code.
@@ -3445,13 +3449,13 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
*/
int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(false, true, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_array_value_cansleep);
@@ -3493,9 +3497,9 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
/**
* gpiod_set_raw_array_value_cansleep() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status.
@@ -3504,13 +3508,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
*/
int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return -EINVAL;
return gpiod_set_array_value_complex(true, true, array_size, desc_array,
- value_array);
+ value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value_cansleep);
@@ -3533,9 +3537,9 @@ void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n)
/**
* gpiod_set_array_value_cansleep() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account.
@@ -3544,13 +3548,13 @@ void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n)
*/
void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return;
gpiod_set_array_value_complex(false, true, array_size, desc_array,
- value_array);
+ value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_array_value_cansleep);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index a7e49fef73d4..11e83d2eef89 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -187,11 +187,11 @@ struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip, u16 hwnum);
int gpiod_get_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_set_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
/* This is just passed between gpiolib and devres */
struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 401308e3d036..d675e0ca2fa4 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -27,13 +27,14 @@ struct gpiomux {
static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
{
+ unsigned long value_bitmap[1] = { val, };
int i;
for (i = 0; i < mux->data.n_gpios; i++)
mux->values[i] = (val >> i) & 1;
gpiod_set_array_value_cansleep(mux->data.n_gpios,
- mux->gpios, mux->values);
+ mux->gpios, value_bitmap);
}
static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index a8b9fee4d62a..0d6e3a5be3ba 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -40,18 +40,13 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
struct gpio_descs *reset_gpios = pwrseq->reset_gpios;
if (!IS_ERR(reset_gpios)) {
- int i, *values;
+ unsigned long value_bitmap[1];
int nvalues = reset_gpios->ndescs;
- values = kmalloc_array(nvalues, sizeof(int), GFP_KERNEL);
- if (!values)
- return;
+ value_bitmap[0] = value;
- for (i = 0; i < nvalues; i++)
- values[i] = value;
-
- gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc, values);
- kfree(values);
+ gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
+ value_bitmap);
}
}
diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
index 6fdd9316db8b..cc2d5f50472a 100644
--- a/drivers/mux/gpio.c
+++ b/drivers/mux/gpio.c
@@ -23,14 +23,14 @@ struct mux_gpio {
static int mux_gpio_set(struct mux_control *mux, int state)
{
struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
+ unsigned long value_bitmap[1] = { state, };
int i;
for (i = 0; i < mux_gpio->gpios->ndescs; i++)
mux_gpio->val[i] = (state >> i) & 1;
gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
- mux_gpio->gpios->desc,
- mux_gpio->val);
+ mux_gpio->gpios->desc, value_bitmap);
return 0;
}
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index bc90764a8b8d..8e1ec750277e 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -27,6 +27,7 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
void *data)
{
struct mdio_mux_gpio_state *s = data;
+ unsigned long value_bitmap[1] = { desired_child, };
unsigned int n;
if (current_child == desired_child)
@@ -36,7 +37,7 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
s->values[n] = (desired_child >> n) & 1;
gpiod_set_array_value_cansleep(s->gpios->ndescs, s->gpios->desc,
- s->values);
+ value_bitmap);
return 0;
}
diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index c5f2344c189b..e0f89155c474 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -351,19 +351,22 @@ static int soc_common_pcmcia_config_skt(
if (ret == 0) {
struct gpio_desc *descs[2];
- int values[2], n = 0;
+ unsigned long value_bitmap[1];
+ int n = 0;
if (skt->gpio_reset) {
descs[n] = skt->gpio_reset;
- values[n++] = !!(state->flags & SS_RESET);
+ __assign_bit(n++, value_bitmap,
+ !!(state->flags & SS_RESET));
}
if (skt->gpio_bus_enable) {
descs[n] = skt->gpio_bus_enable;
- values[n++] = !!(state->flags & SS_OUTPUT_ENA);
+ __assign_bit(n++, value_bitmap,
+ !!(state->flags & SS_OUTPUT_ENA));
}
if (n)
- gpiod_set_array_value_cansleep(n, descs, values);
+ gpiod_set_array_value_cansleep(n, descs, value_bitmap);
/*
* This really needs a better solution. The IRQ
diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
index 0075fb0bef8c..b6477c3599c4 100644
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -157,15 +157,12 @@ static const struct phy_ops gpio_usb_ops = {
*/
static void phy_mdm6600_cmd(struct phy_mdm6600 *ddata, int val)
{
- int values[PHY_MDM6600_NR_CMD_LINES];
- int i;
+ unsigned long value_bitmap[1];
- val &= (1 << PHY_MDM6600_NR_CMD_LINES) - 1;
- for (i = 0; i < PHY_MDM6600_NR_CMD_LINES; i++)
- values[i] = (val & BIT(i)) >> i;
+ value_bitmap[0] = val & ((1 << PHY_MDM6600_NR_CMD_LINES) - 1);
gpiod_set_array_value_cansleep(PHY_MDM6600_NR_CMD_LINES,
- ddata->cmd_gpios->desc, values);
+ ddata->cmd_gpios->desc, value_bitmap);
}
/**
@@ -176,7 +173,7 @@ static void phy_mdm6600_status(struct work_struct *work)
{
struct phy_mdm6600 *ddata;
struct device *dev;
- int values[PHY_MDM6600_NR_STATUS_LINES];
+ unsigned long value_bitmap[1] = { 0, };
int error, i, val = 0;
ddata = container_of(work, struct phy_mdm6600, status_work.work);
@@ -184,14 +181,14 @@ static void phy_mdm6600_status(struct work_struct *work)
error = gpiod_get_array_value_cansleep(PHY_MDM6600_NR_STATUS_LINES,
ddata->status_gpios->desc,
- values);
+ value_bitmap);
if (error)
return;
for (i = 0; i < PHY_MDM6600_NR_STATUS_LINES; i++) {
- val |= values[i] << i;
+ val |= test_bit(i, value_bitmap) << i;
dev_dbg(ddata->dev, "XXX %s: i: %i values[i]: %i val: %i\n",
- __func__, i, values[i], val);
+ __func__, i, test_bit(i, value_bitmap), val);
}
ddata->status = val;
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 25b9fcd5e3a4..0eca047bc1cc 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -202,7 +202,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
long mask)
{
struct ad7606_state *st = iio_priv(indio_dev);
- int values[3];
+ unsigned long value_bitmap[1];
int ret, i;
switch (mask) {
@@ -227,13 +227,10 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
if (ret < 0)
return ret;
- values[0] = (ret >> 0) & 1;
- values[1] = (ret >> 1) & 1;
- values[2] = (ret >> 2) & 1;
+ value_bitmap[0] = ret;
mutex_lock(&st->lock);
- gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
- values);
+ gpiod_set_array_value(3, st->gpio_os->desc, value_bitmap);
st->oversampling = val;
mutex_unlock(&st->lock);
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 1c06325beaca..bb8b4756d72d 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -40,7 +40,7 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
{
enum mctrl_gpio_idx i;
struct gpio_desc *desc_array[UART_GPIO_MAX];
- int value_array[UART_GPIO_MAX];
+ unsigned long value_bitmap[BITS_TO_LONGS(UART_GPIO_MAX)];
unsigned int count = 0;
if (gpios == NULL)
@@ -49,10 +49,11 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
for (i = 0; i < UART_GPIO_MAX; i++)
if (gpios->gpio[i] && mctrl_gpios_desc[i].dir_out) {
desc_array[count] = gpios->gpio[i];
- value_array[count] = !!(mctrl & mctrl_gpios_desc[i].mctrl);
+ __assign_bit(count, value_bitmap,
+ !!(mctrl & mctrl_gpios_desc[i].mctrl));
count++;
}
- gpiod_set_array_value(count, desc_array, value_array);
+ gpiod_set_array_value(count, desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(mctrl_gpio_set);
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 21ddbe440030..1b21dc7b0fad 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -104,36 +104,38 @@ int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
/* Value get/set from non-sleeping context */
int gpiod_get_value(const struct gpio_desc *desc);
int gpiod_get_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array);
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap);
void gpiod_set_value(struct gpio_desc *desc, int value);
void gpiod_set_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array);
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap);
int gpiod_get_raw_value(const struct gpio_desc *desc);
int gpiod_get_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_raw_value(struct gpio_desc *desc, int value);
int gpiod_set_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
/* Value get/set from sleeping context */
int gpiod_get_value_cansleep(const struct gpio_desc *desc);
int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc);
int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);
int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
--
2.16.4
^ permalink raw reply related
* Re: [PATCH iproute2] iproute: make clang happy
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-08-20 23:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Mahesh Bandewar, netdev
In-Reply-To: <20180820155223.0569b849@xeon-e3>
On Mon, Aug 20, 2018 at 3:52 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Mon, 20 Aug 2018 14:42:15 -0700
> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>
>> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
>> index ace4b3dd738b..a524b520b276 100644
>> --- a/tc/m_ematch.c
>> +++ b/tc/m_ematch.c
>> @@ -277,6 +277,7 @@ static int flatten_tree(struct ematch *head, struct ematch *tree)
>> return count;
>> }
>>
>> +__attribute__((format(printf, 5, 6)))
>> int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>> struct ematch_util *e, char *fmt, ...)
>
> I think the printf attribute needs to go on the function prototype
> here:
> tc/m_ematch.h:extern int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>
The attributes are attached to the definitions only and not prototype
declarations. Please see the definition/declaration for jsonw_printf()
in the same patch.
>
> PS: I need to take the extern of those function prototypes.
^ permalink raw reply
* Re: r8169 needs CONFIG_REALTEK_PHY
From: Marc Dionne @ 2018-08-20 23:33 UTC (permalink / raw)
To: Florian Fainelli; +Cc: hkallweit1, netdev, David Miller
In-Reply-To: <29f25302-6a60-4a44-fd65-26b5a15ff35a@gmail.com>
On Mon, Aug 20, 2018 at 5:39 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 08/20/2018 12:44 PM, Marc Dionne wrote:
>> The r8169 adapter in one of my machines was not working after updating
>> to a current kernel from the merge window, which was fixed by enabling
>> CONFIG_REALTEK_PHY.
>>
>> So in addition to "select PHYLIB", should CONFIG_R8169 not also be
>> doing "select CONFIG_REALTEK_PHY" ?
>
> This is fixed in Linus' tree now with:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bfdd19ad80f203f42f05fd32a31c678c9c524ef9
> --
> Florian
Ah thanks for the quick reply, thought I had done a fresh pull before
reporting the problem; it is indeed fixed in Linus' current tree.
Sorry for the noise,
Marc
^ permalink raw reply
* Re: [PATCH] tipc: fix issue that tipc_dest neglects of big-endian
From: David Miller @ 2018-08-21 2:20 UTC (permalink / raw)
To: Haiqing.Bai; +Cc: jon.maloy, ying.xue, zhenbo.gao, netdev, linux-kernel
In-Reply-To: <1534760761-30206-1-git-send-email-Haiqing.Bai@windriver.com>
From: Haiqing Bai <Haiqing.Bai@windriver.com>
Date: Mon, 20 Aug 2018 18:26:01 +0800
> The tipc multicast demo in tipcutils fails to work on big-endian hardware.
> The tipc multicast server can not receive the packets sent by the multicast
> client for that the dest port is always zero after tipc_dest_pop, then it
> is found that the struct tipc_dest fails to take big/little endian into
> account.
>
> Signed-off-by: Haiqing Bai <Haiqing.Bai@windriver.com>
> Signed-off-by: Zhenbo Gao <zhenbo.gao@windriver.com>
Jon and Ying, please review.
thank you.
^ permalink raw reply
* [PATCH iproute2 4/4] tc: drop extern from function prototypes
From: Stephen Hemminger @ 2018-08-20 23:02 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
In-Reply-To: <20180820230231.24723-1-stephen@networkplumber.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
tc/m_ematch.h | 35 +++++++++++++++--------------------
tc/m_pedit.h | 31 +++++++++++++++----------------
tc/tc_common.h | 30 +++++++++++++++---------------
tc/tc_red.h | 7 ++++---
4 files changed, 49 insertions(+), 54 deletions(-)
diff --git a/tc/m_ematch.h b/tc/m_ematch.h
index f634f19164fa..ff02d7ac9112 100644
--- a/tc/m_ematch.h
+++ b/tc/m_ematch.h
@@ -12,17 +12,16 @@
#define EMATCHKINDSIZ 16
-struct bstr
-{
+struct bstr {
char *data;
unsigned int len;
int quoted;
struct bstr *next;
};
-extern struct bstr * bstr_alloc(const char *text);
+struct bstr *bstr_alloc(const char *text);
-static inline struct bstr * bstr_new(char *data, unsigned int len)
+static inline struct bstr *bstr_new(char *data, unsigned int len)
{
struct bstr *b = calloc(1, sizeof(*b));
@@ -35,7 +34,7 @@ static inline struct bstr * bstr_new(char *data, unsigned int len)
return b;
}
-static inline int bstrcmp(struct bstr *b, const char *text)
+static inline int bstrcmp(const struct bstr *b, const char *text)
{
int len = strlen(text);
int d = b->len - len;
@@ -51,12 +50,10 @@ static inline struct bstr *bstr_next(struct bstr *b)
return b->next;
}
-extern unsigned long bstrtoul(const struct bstr *b);
-extern void bstr_print(FILE *fd, const struct bstr *b, int ascii);
-
+unsigned long bstrtoul(const struct bstr *b);
+void bstr_print(FILE *fd, const struct bstr *b, int ascii);
-struct ematch
-{
+struct ematch {
struct bstr *args;
int index;
int inverted;
@@ -66,7 +63,7 @@ struct ematch
struct ematch *next;
};
-static inline struct ematch * new_ematch(struct bstr *args, int inverted)
+static inline struct ematch *new_ematch(struct bstr *args, int inverted)
{
struct ematch *e = calloc(1, sizeof(*e));
@@ -79,14 +76,12 @@ static inline struct ematch * new_ematch(struct bstr *args, int inverted)
return e;
}
-extern void print_ematch_tree(const struct ematch *tree);
+void print_ematch_tree(const struct ematch *tree);
-
-struct ematch_util
-{
+struct ematch_util {
char kind[EMATCHKINDSIZ];
int kind_num;
- int (*parse_eopt)(struct nlmsghdr *,struct tcf_ematch_hdr *,
+ int (*parse_eopt)(struct nlmsghdr *, struct tcf_ematch_hdr *,
struct bstr *);
int (*parse_eopt_argv)(struct nlmsghdr *, struct tcf_ematch_hdr *,
int, char **);
@@ -95,7 +90,7 @@ struct ematch_util
struct ematch_util *next;
};
-static inline int parse_layer(struct bstr *b)
+static inline int parse_layer(const struct bstr *b)
{
if (*((char *) b->data) == 'l')
return TCF_LAYER_LINK;
@@ -107,9 +102,9 @@ static inline int parse_layer(struct bstr *b)
return INT_MAX;
}
-extern int em_parse_error(int err, struct bstr *args, struct bstr *carg,
+int em_parse_error(int err, struct bstr *args, struct bstr *carg,
struct ematch_util *, char *fmt, ...);
-extern int print_ematch(FILE *, const struct rtattr *);
-extern int parse_ematch(int *, char ***, int, struct nlmsghdr *);
+int print_ematch(FILE *, const struct rtattr *);
+int parse_ematch(int *, char ***, int, struct nlmsghdr *);
#endif
diff --git a/tc/m_pedit.h b/tc/m_pedit.h
index a8b069581509..b6b274bd08c7 100644
--- a/tc/m_pedit.h
+++ b/tc/m_pedit.h
@@ -71,23 +71,22 @@ struct m_pedit_util {
struct m_pedit_key *tkey);
};
-extern int pack_key(struct m_pedit_sel *sel, struct m_pedit_key *tkey);
-extern int pack_key32(__u32 retain, struct m_pedit_sel *sel,
- struct m_pedit_key *tkey);
-extern int pack_key16(__u32 retain, struct m_pedit_sel *sel,
- struct m_pedit_key *tkey);
-extern int pack_key8(__u32 retain, struct m_pedit_sel *sel,
+int pack_key(struct m_pedit_sel *sel, struct m_pedit_key *tkey);
+int pack_key32(__u32 retain, struct m_pedit_sel *sel,
+ struct m_pedit_key *tkey);
+int pack_key16(__u32 retain, struct m_pedit_sel *sel,
+ struct m_pedit_key *tkey);
+int pack_key8(__u32 retain, struct m_pedit_sel *sel,
struct m_pedit_key *tkey);
-extern int parse_val(int *argc_p, char ***argv_p, __u32 *val, int type);
-extern int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type,
- __u32 retain,
- struct m_pedit_sel *sel, struct m_pedit_key *tkey);
-extern int parse_offset(int *argc_p, char ***argv_p,
- struct m_pedit_sel *sel, struct m_pedit_key *tkey);
+int parse_val(int *argc_p, char ***argv_p, __u32 *val, int type);
+int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type,
+ __u32 retain,
+ struct m_pedit_sel *sel, struct m_pedit_key *tkey);
+int parse_offset(int *argc_p, char ***argv_p,
+ struct m_pedit_sel *sel, struct m_pedit_key *tkey);
int parse_pedit(struct action_util *a, int *argc_p, char ***argv_p,
int tca_id, struct nlmsghdr *n);
-extern int print_pedit(struct action_util *au, FILE *f, struct rtattr *arg);
-extern int pedit_print_xstats(struct action_util *au, FILE *f,
- struct rtattr *xstats);
-
+int print_pedit(struct action_util *au, FILE *f, struct rtattr *arg);
+int pedit_print_xstats(struct action_util *au, FILE *f,
+ struct rtattr *xstats);
#endif
diff --git a/tc/tc_common.h b/tc/tc_common.h
index 272d1727027d..371ca7d04602 100644
--- a/tc/tc_common.h
+++ b/tc/tc_common.h
@@ -5,26 +5,26 @@
extern struct rtnl_handle rth;
-extern int do_qdisc(int argc, char **argv);
-extern int do_class(int argc, char **argv);
-extern int do_filter(int argc, char **argv, void *buf, size_t buflen);
-extern int do_chain(int argc, char **argv, void *buf, size_t buflen);
-extern int do_action(int argc, char **argv, void *buf, size_t buflen);
-extern int do_tcmonitor(int argc, char **argv);
-extern int do_exec(int argc, char **argv);
+int do_qdisc(int argc, char **argv);
+int do_class(int argc, char **argv);
+int do_filter(int argc, char **argv, void *buf, size_t buflen);
+int do_chain(int argc, char **argv, void *buf, size_t buflen);
+int do_action(int argc, char **argv, void *buf, size_t buflen);
+int do_tcmonitor(int argc, char **argv);
+int do_exec(int argc, char **argv);
-extern int print_action(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg);
-extern int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg);
-extern int print_qdisc(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg);
-extern int print_class(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg);
-extern void print_size_table(FILE *fp, const char *prefix, struct rtattr *rta);
+int print_action(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg);
+int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg);
+int print_qdisc(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg);
+int print_class(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg);
+void print_size_table(FILE *fp, const char *prefix, struct rtattr *rta);
struct tc_estimator;
-extern int parse_estimator(int *p_argc, char ***p_argv, struct tc_estimator *est);
+int parse_estimator(int *p_argc, char ***p_argv, struct tc_estimator *est);
struct tc_sizespec;
-extern int parse_size_table(int *p_argc, char ***p_argv, struct tc_sizespec *s);
-extern int check_size_table_opts(struct tc_sizespec *s);
+int parse_size_table(int *p_argc, char ***p_argv, struct tc_sizespec *s);
+int check_size_table_opts(struct tc_sizespec *s);
extern int show_graph;
extern bool use_names;
diff --git a/tc/tc_red.h b/tc/tc_red.h
index 88fba58b7416..6c6e6b039732 100644
--- a/tc/tc_red.h
+++ b/tc/tc_red.h
@@ -2,8 +2,9 @@
#ifndef _TC_RED_H_
#define _TC_RED_H_ 1
-extern int tc_red_eval_P(unsigned qmin, unsigned qmax, double prob);
-extern int tc_red_eval_ewma(unsigned qmin, unsigned burst, unsigned avpkt);
-extern int tc_red_eval_idle_damping(int wlog, unsigned avpkt, unsigned bandwidth, __u8 *sbuf);
+int tc_red_eval_P(unsigned qmin, unsigned qmax, double prob);
+int tc_red_eval_ewma(unsigned qmin, unsigned burst, unsigned avpkt);
+int tc_red_eval_idle_damping(int wlog, unsigned avpkt, unsigned bandwidth,
+ __u8 *sbuf);
#endif
--
2.18.0
^ permalink raw reply related
* [PATCH iproute2 3/4] genl: drop extern from function prototypes
From: Stephen Hemminger @ 2018-08-20 23:02 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
In-Reply-To: <20180820230231.24723-1-stephen@networkplumber.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
genl/genl_utils.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/genl/genl_utils.h b/genl/genl_utils.h
index 6e6f44501aba..3de0da34bdba 100644
--- a/genl/genl_utils.h
+++ b/genl/genl_utils.h
@@ -10,9 +10,10 @@ struct genl_util
struct genl_util *next;
char name[16];
int (*parse_genlopt)(struct genl_util *fu, int argc, char **argv);
- int (*print_genlopt)(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg);
+ int (*print_genlopt)(const struct sockaddr_nl *who,
+ struct nlmsghdr *n, void *arg);
};
-extern int genl_ctrl_resolve_family(const char *family);
+int genl_ctrl_resolve_family(const char *family);
#endif
--
2.18.0
^ permalink raw reply related
* [PATCH iproute2 2/4] bridge: drop extern from function prototypes
From: Stephen Hemminger @ 2018-08-20 23:02 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
In-Reply-To: <20180820230231.24723-1-stephen@networkplumber.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
bridge/br_common.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/bridge/br_common.h b/bridge/br_common.h
index 2f1cb8fd9f3d..7bf15e9548fc 100644
--- a/bridge/br_common.h
+++ b/bridge/br_common.h
@@ -6,20 +6,20 @@
#define MDB_RTR_RTA(r) \
((struct rtattr *)(((char *)(r)) + RTA_ALIGN(sizeof(__u32))))
-extern void print_vlan_info(FILE *fp, struct rtattr *tb);
-extern int print_linkinfo(const struct sockaddr_nl *who,
+void print_vlan_info(FILE *fp, struct rtattr *tb);
+int print_linkinfo(const struct sockaddr_nl *who,
struct nlmsghdr *n,
void *arg);
-extern int print_fdb(const struct sockaddr_nl *who,
+int print_fdb(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg);
-extern int print_mdb(const struct sockaddr_nl *who,
+int print_mdb(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg);
-extern int do_fdb(int argc, char **argv);
-extern int do_mdb(int argc, char **argv);
-extern int do_monitor(int argc, char **argv);
-extern int do_vlan(int argc, char **argv);
-extern int do_link(int argc, char **argv);
+int do_fdb(int argc, char **argv);
+int do_mdb(int argc, char **argv);
+int do_monitor(int argc, char **argv);
+int do_vlan(int argc, char **argv);
+int do_link(int argc, char **argv);
extern int preferred_family;
extern int show_stats;
--
2.18.0
^ permalink raw reply related
* [PATCH iproute2 1/4] ip: drop extern from function prototype
From: Stephen Hemminger @ 2018-08-20 23:02 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
In-Reply-To: <20180820230231.24723-1-stephen@networkplumber.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
ip/ip_common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ip/ip_common.h b/ip/ip_common.h
index 4d3227cbc389..200be5e23dd1 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -82,7 +82,7 @@ int do_netns(int argc, char **argv);
int do_xfrm(int argc, char **argv);
int do_ipl2tp(int argc, char **argv);
int do_ipfou(int argc, char **argv);
-extern int do_ipila(int argc, char **argv);
+int do_ipila(int argc, char **argv);
int do_tcp_metrics(int argc, char **argv);
int do_ipnetconf(int argc, char **argv);
int do_iptoken(int argc, char **argv);
--
2.18.0
^ permalink raw reply related
* [PATCH iproute2 0/4] drop extern from function prototypes
From: Stephen Hemminger @ 2018-08-20 23:02 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
iproute2 uses kernel style guidelines
Stephen Hemminger (4):
ip: drop extern from function prototype
bridge: drop extern from function prototypes
genl: drop extern from function prototypes
tc: drop extern from function prototypes
bridge/br_common.h | 18 +++++++++---------
genl/genl_utils.h | 5 +++--
ip/ip_common.h | 2 +-
tc/m_ematch.h | 35 +++++++++++++++--------------------
tc/m_pedit.h | 31 +++++++++++++++----------------
tc/tc_common.h | 30 +++++++++++++++---------------
tc/tc_red.h | 7 ++++---
7 files changed, 62 insertions(+), 66 deletions(-)
--
2.18.0
^ permalink raw reply
* Re: [PATCH iproute2] iproute: make clang happy
From: Stephen Hemminger @ 2018-08-20 22:52 UTC (permalink / raw)
To: Mahesh Bandewar; +Cc: netdev, Mahesh Bandewar
In-Reply-To: <20180820214215.218127-1-mahesh@bandewar.net>
On Mon, 20 Aug 2018 14:42:15 -0700
Mahesh Bandewar <mahesh@bandewar.net> wrote:
> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
> index ace4b3dd738b..a524b520b276 100644
> --- a/tc/m_ematch.c
> +++ b/tc/m_ematch.c
> @@ -277,6 +277,7 @@ static int flatten_tree(struct ematch *head, struct ematch *tree)
> return count;
> }
>
> +__attribute__((format(printf, 5, 6)))
> int em_parse_error(int err, struct bstr *args, struct bstr *carg,
> struct ematch_util *e, char *fmt, ...)
I think the printf attribute needs to go on the function prototype
here:
tc/m_ematch.h:extern int em_parse_error(int err, struct bstr *args, struct bstr *carg,
PS: I need to take the extern of those function prototypes.
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Alban @ 2018-08-20 22:53 UTC (permalink / raw)
To: Boris Brezillon
Cc: Aban Bedel, Srinivas Kandagatla, Bartosz Golaszewski,
Jonathan Corbet, Sekhar Nori, Kevin Hilman, Russell King,
Arnd Bergmann, Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Marek Vasut, Richard Weinberger, Grygorii Strashko,
David S . Miller, Naren, Mauro Carvalho Chehab, Andrew Morton,
Lukas Wunner, Dan
In-Reply-To: <20180819184609.6dcdbb9a@bbrezillon>
[-- Attachment #1: Type: text/plain, Size: 6159 bytes --]
On Sun, 19 Aug 2018 18:46:09 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> On Sun, 19 Aug 2018 13:31:06 +0200
> Alban <albeu@free.fr> wrote:
>
> > On Fri, 17 Aug 2018 18:27:20 +0200
> > Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> >
> > > Hi Bartosz,
> > >
> > > On Fri, 10 Aug 2018 10:05:03 +0200
> > > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > > From: Alban Bedel <albeu@free.fr>
> > > >
> > > > Allow drivers that use the nvmem API to read data stored on MTD devices.
> > > > For this the mtd devices are registered as read-only NVMEM providers.
> > > >
> > > > Signed-off-by: Alban Bedel <albeu@free.fr>
> > > > [Bartosz:
> > > > - use the managed variant of nvmem_register(),
> > > > - set the nvmem name]
> > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > What happened to the 2 other patches of Alban's series? I'd really
> > > like the DT case to be handled/agreed on in the same patchset, but
> > > IIRC, Alban and Srinivas disagreed on how this should be represented.
> > > I hope this time we'll come to an agreement, because the MTD <-> NVMEM
> > > glue has been floating around for quite some time...
> >
> > These other patches were to fix what I consider a fundamental flaw in
> > the generic NVMEM bindings, however we couldn't agree on this point.
> > Bartosz later contacted me to take over this series and I suggested to
> > just change the MTD NVMEM binding to use a compatible string on the
> > NVMEM cells as an alternative solution to fix the clash with the old
> > style MTD partition.
> >
> > However all this has no impact on the code needed to add NVMEM support
> > to MTD, so the above patch didn't change at all.
>
> It does have an impact on the supported binding though.
> nvmem->dev.of_node is automatically assigned to mtd->dev.of_node, which
> means people will be able to define their NVMEM cells directly under
> the MTD device and reference them from other nodes (even if it's not
> documented), and as you said, it conflict with the old MTD partition
> bindings. So we'd better agree on this binding before merging this
> patch.
Unless the nvmem cell node has a compatible string, then it won't be
considered as a partition by the MTD code. That is were the clash is,
both bindings allow free named child nodes without a compatible string.
> I see several options:
>
> 1/ provide a way to tell the NVMEM framework not to use parent->of_node
> even if it's != NULL. This way we really don't support defining
> NVMEM cells in the DT, and also don't support referencing the nvmem
> device using a phandle.
I really don't get what the point of this would be. Make the whole API
useless?
> 2/ define a new binding where all nvmem-cells are placed in an
> "nvmem" subnode (just like we have this "partitions" subnode for
> partitions), and then add a config->of_node field so that the
> nvmem provider can explicitly specify the DT node representing the
> nvmem device. We'll also need to set this field to ERR_PTR(-ENOENT)
> in case this node does not exist so that the nvmem framework knows
> that it should not assign nvmem->dev.of_node to parent->of_node
This is not good. First the NVMEM device is only a virtual concept of
the Linux kernel, it has no place in the DT. Secondly the NVMEM
provider (here the MTD device) then has to manually parse its DT node to
find this subnode, pass it to the NVMEM framework to later again
resolve it back to the MTD device. Not very complex but still a lot of
useless code, just registering the MTD device is a lot simpler and much
more inline with most other kernel API that register a "service"
available from a device.
> 3/ only declare partitions as nvmem providers. This would solve the
> problem we have with partitions defined in the DT since
> defining sub-partitions in the DT is not (yet?) supported and
> partition nodes are supposed to be leaf nodes. Still, I'm not a big
> fan of this solution because it will prevent us from supporting
> sub-partitions if we ever want/need to.
That sound like a poor workaround. Remember that this problem could
appear with any device that has a binding that use child nodes.
> 4/ Add a ->of_xlate() hook that would be called if present by the
> framework instead of using the default parsing we have right now.
That is a bit cleaner, but I don't think it would be worse the
complexity. Furthermore xlate functions are more about converting
from hardware parameters to internal kernel representation than to hide
extra DT parsing.
> 5/ Tell the nvmem framework the name of the subnode containing nvmem
> cell definitions (if NULL that means cells are directly defined
> under the nvmem provider node). We would set it to "nvmem-cells" (or
> whatever you like) for the MTD case.
If so please match on compatible and not on the node name.
6/ Extend the current NVMEM cell lookup to check if the parent node of
the cell has a compatible string set to "nvmem-cells". If it doesn't it
mean we have the current binding and this node is the NVMEM device. If
it does the device node is just the next parent. This is trivial to
implement (literally 2 lines of code) and cover all the cases currently
known.
7/ Just add a compatible string to the nvmem cell. No code change is
needed, however as the nvmem cells have an address space (the offset in
byte in the storage) it might still clash with another address space
used by the main device biding (for example a number of child
functions).
> There are probably other options (some were proposed by Alban and
> Srinivas already), but I'd like to get this sorted out before we merge
> this patch.
>
> Alban, Srinivas, any opinion?
My preference goes to 6/ as it is trivial to implement, solves all
known shortcomings and is backward compatible with the current binding.
All other solutions have limitations and/or require too complex
implementations compared to what they try to solve.
Alban
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH iproute2] iproute: make clang happy
From: Stephen Hemminger @ 2018-08-20 22:50 UTC (permalink / raw)
To: Mahesh Bandewar; +Cc: netdev, Mahesh Bandewar
In-Reply-To: <20180820214215.218127-1-mahesh@bandewar.net>
On Mon, 20 Aug 2018 14:42:15 -0700
Mahesh Bandewar <mahesh@bandewar.net> wrote:
>
> if (is_json_context()) {
> + json_writer_t *jw;
> +
> open_json_object("bittiming");
> print_int(PRINT_ANY, "bitrate", NULL, bt->bitrate);
> - jsonw_float_field_fmt(get_json_writer(),
> - "sample_point", "%.3f",
> - (float) bt->sample_point / 1000.);
> + jw = get_json_writer();
> + jsonw_name(jw, "sample_point");
> + jsonw_printf(jw, "%.3f",
> + (float) bt->sample_point / 1000);
I think it would be better to get rid of the is_json_context() here in the CAN code
and just use the print_json functions completely. Most of the other code is able to
do that already.
^ 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