* [RFC] replace ->get_poll_head with a waitqueue pointer in struct file
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
Introducing the new poll methods showed up a regression in the
will-it-scale ltp tests. One reason for that is that indirect function
calls are very expensive now with the spectre mitigations. I'm waiting
for better numbers, but this series has shown a 5% improvements in the
ops per second so far, while for the get_poll_head addition we had
regressions of 3.7% or 8.8% depending on the measurement.
This series removes the get_poll_head method again and instead stores an
optional wait_queue_head pointer in struct file, on which the poll_mask
method can be used if it is set. The only complication is the networking
poll code which not only does some interesting gymnastics to get at the
wait queue pointer, but also has a mode to to hardware polling before
waiting for an even from poll or epoll. Because of that this series has
a few net prep patches that need careful review.
^ permalink raw reply
* [PATCH 1/6] net: remove sock_poll_busy_flag
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
Can simplify be inlined into the only caller.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/net/busy_poll.h | 6 ------
net/socket.c | 5 ++++-
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index c5187438af38..4a459a0d70d1 100644
--- a/include/net/busy_poll.h
+++ b/include/net/busy_poll.h
@@ -130,12 +130,6 @@ static inline void sock_poll_busy_loop(struct socket *sock, __poll_t events)
}
}
-/* if this socket can poll_ll, tell the system call */
-static inline __poll_t sock_poll_busy_flag(struct socket *sock)
-{
- return sk_can_busy_loop(sock->sk) ? POLL_BUSY_LOOP : 0;
-}
-
/* used in the NIC receive handler to mark the skb */
static inline void skb_mark_napi_id(struct sk_buff *skb,
struct napi_struct *napi)
diff --git a/net/socket.c b/net/socket.c
index 8a109012608a..0f397fa33614 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1171,7 +1171,10 @@ static __poll_t sock_poll(struct file *file, poll_table *wait)
mask = sock->ops->poll_mask(sock, events);
}
- return mask | sock_poll_busy_flag(sock);
+ /* this socket can poll_ll so tell the system call */
+ if (sk_can_busy_loop(sock->sk))
+ mask |= POLL_BUSY_LOOP;
+ return mask;
}
static int sock_mmap(struct file *file, struct vm_area_struct *vma)
--
2.17.1
^ permalink raw reply related
* [PATCH 2/6] net: remove bogus RCU annotations on socket.wq
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
We never use RCU protection for it, just a lot of cargo-cult
rcu_deference_protects calls.
Note that we do keep the kfree_rcu call for it, as the references through
struct sock are RCU protected and thus might require a grace period before
freeing.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/net.h | 2 +-
include/net/sock.h | 2 +-
net/socket.c | 10 ++++------
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/include/linux/net.h b/include/linux/net.h
index 08b6eb964dd6..2be0d5368315 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -114,7 +114,7 @@ struct socket {
unsigned long flags;
- struct socket_wq __rcu *wq;
+ struct socket_wq *wq;
struct file *file;
struct sock *sk;
diff --git a/include/net/sock.h b/include/net/sock.h
index b3b75419eafe..ad85d37c83c8 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1725,7 +1725,7 @@ static inline void sock_graft(struct sock *sk, struct socket *parent)
{
WARN_ON(parent->sk);
write_lock_bh(&sk->sk_callback_lock);
- sk->sk_wq = parent->wq;
+ rcu_assign_pointer(sk->sk_wq, parent->wq);
parent->sk = sk;
sk_set_socket(sk, parent);
sk->sk_uid = SOCK_INODE(parent)->i_uid;
diff --git a/net/socket.c b/net/socket.c
index 0f397fa33614..fe6620607b07 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -255,7 +255,7 @@ static struct inode *sock_alloc_inode(struct super_block *sb)
init_waitqueue_head(&wq->wait);
wq->fasync_list = NULL;
wq->flags = 0;
- RCU_INIT_POINTER(ei->socket.wq, wq);
+ ei->socket.wq = wq;
ei->socket.state = SS_UNCONNECTED;
ei->socket.flags = 0;
@@ -269,11 +269,9 @@ static struct inode *sock_alloc_inode(struct super_block *sb)
static void sock_destroy_inode(struct inode *inode)
{
struct socket_alloc *ei;
- struct socket_wq *wq;
ei = container_of(inode, struct socket_alloc, vfs_inode);
- wq = rcu_dereference_protected(ei->socket.wq, 1);
- kfree_rcu(wq, rcu);
+ kfree_rcu(ei->socket.wq, rcu);
kmem_cache_free(sock_inode_cachep, ei);
}
@@ -607,7 +605,7 @@ static void __sock_release(struct socket *sock, struct inode *inode)
module_put(owner);
}
- if (rcu_dereference_protected(sock->wq, 1)->fasync_list)
+ if (sock->wq->fasync_list)
pr_err("%s: fasync list not empty!\n", __func__);
if (!sock->file) {
@@ -1211,7 +1209,7 @@ static int sock_fasync(int fd, struct file *filp, int on)
return -EINVAL;
lock_sock(sk);
- wq = rcu_dereference_protected(sock->wq, lockdep_sock_is_held(sk));
+ wq = sock->wq;
fasync_helper(fd, filp, on, &wq->fasync_list);
if (!wq->fasync_list)
--
2.17.1
^ permalink raw reply related
* [PATCH 3/6] net: don't detour through struct to find the poll head
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
As far as I can tell sock->sk->sk_wq->wait will always point to
sock->wq->wait. That means we can do the shorter dereference and
not worry about any RCU protection.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
net/socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/socket.c b/net/socket.c
index fe6620607b07..7cf037d21805 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1136,7 +1136,7 @@ static struct wait_queue_head *sock_get_poll_head(struct file *file,
if (!sock->ops->poll_mask)
return NULL;
sock_poll_busy_loop(sock, events);
- return sk_sleep(sock->sk);
+ return &sock->wq->wait;
}
static __poll_t sock_poll_mask(struct file *file, __poll_t events)
--
2.17.1
^ permalink raw reply related
* [PATCH 4/6] net: remove busy polling from sock_get_poll_head
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
Busy polling always comes from a synchronous poll context, so for now we
can assume that it calls ->poll if present. Move the busy polling in
sock_poll to the common block and remove it from sock_get_poll_head to
prepare for the removal of the get_poll_head method.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
net/socket.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index 7cf037d21805..ca300c97b7ba 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1135,7 +1135,6 @@ static struct wait_queue_head *sock_get_poll_head(struct file *file,
if (!sock->ops->poll_mask)
return NULL;
- sock_poll_busy_loop(sock, events);
return &sock->wq->wait;
}
@@ -1161,8 +1160,9 @@ static __poll_t sock_poll(struct file *file, poll_table *wait)
struct socket *sock = file->private_data;
__poll_t events = poll_requested_events(wait), mask = 0;
+ sock_poll_busy_loop(sock, events);
+
if (sock->ops->poll) {
- sock_poll_busy_loop(sock, events);
mask = sock->ops->poll(file, sock, wait);
} else if (sock->ops->poll_mask) {
sock_poll_wait(file, sock_get_poll_head(file, events), wait);
--
2.17.1
^ permalink raw reply related
* [PATCH 5/6] net: remove sock_poll_busy_loop
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
There is no point in hiding this logic in a helper. Also remove the
useless events != 0 check, and reorder the remaining ones so that the
cheaper test comes first.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/net/busy_poll.h | 9 ---------
net/socket.c | 4 +++-
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index 4a459a0d70d1..71c72a939bf8 100644
--- a/include/net/busy_poll.h
+++ b/include/net/busy_poll.h
@@ -121,15 +121,6 @@ static inline void sk_busy_loop(struct sock *sk, int nonblock)
#endif
}
-static inline void sock_poll_busy_loop(struct socket *sock, __poll_t events)
-{
- if (sk_can_busy_loop(sock->sk) &&
- events && (events & POLL_BUSY_LOOP)) {
- /* once, only if requested by syscall */
- sk_busy_loop(sock->sk, 1);
- }
-}
-
/* used in the NIC receive handler to mark the skb */
static inline void skb_mark_napi_id(struct sk_buff *skb,
struct napi_struct *napi)
diff --git a/net/socket.c b/net/socket.c
index ca300c97b7ba..4354afe8ad96 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1160,7 +1160,9 @@ static __poll_t sock_poll(struct file *file, poll_table *wait)
struct socket *sock = file->private_data;
__poll_t events = poll_requested_events(wait), mask = 0;
- sock_poll_busy_loop(sock, events);
+ /* poll once if requested by the syscall */
+ if ((events & POLL_BUSY_LOOP) && sk_can_busy_loop(sock->sk))
+ sk_busy_loop(sock->sk, 1);
if (sock->ops->poll) {
mask = sock->ops->poll(file, sock, wait);
--
2.17.1
^ permalink raw reply related
* [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
The doubling of the indirect calls caused a too big regression for some
benchmarks in our post-spectre world.
With some of the nastiness in the networking code moved out of the way
we can instead stick a pointer to a waitqueue into struct file and
avoid one of the indirect calls. This actually happens to simplify
the code quite a bit as well.
Note that for this removes the possibility of actually returning an
error before waiting in poll. We could still do this with an ERR_PTR
in f_poll_head with a little bit of WRITE_ONCE/READ_ONCE magic, but
I'd like to defer that until actually required.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
Documentation/filesystems/Locking | 4 +---
Documentation/filesystems/vfs.txt | 15 +++++---------
drivers/char/random.c | 8 ++++----
fs/aio.c | 22 ++++++---------------
fs/eventfd.c | 33 +++++++++++++++++++------------
fs/eventpoll.c | 9 +--------
fs/pipe.c | 12 +++--------
fs/select.c | 18 ++++-------------
fs/timerfd.c | 31 +++++++++++++++++------------
include/linux/fs.h | 2 +-
include/linux/poll.h | 7 +------
net/socket.c | 17 +++-------------
12 files changed, 67 insertions(+), 111 deletions(-)
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 2c391338c675..4d183e8258b6 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -441,7 +441,6 @@ prototypes:
int (*iterate) (struct file *, struct dir_context *);
int (*iterate_shared) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
- struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
__poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
@@ -505,8 +504,7 @@ in sys_read() and friends.
the lease within the individual filesystem to record the result of the
operation
-->poll_mask can be called with or without the waitqueue lock for the waitqueue
-returned from ->get_poll_head.
+->poll_mask can be called with or without the waitqueue lock
--------------------------- dquot_operations -------------------------------
prototypes:
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 829a7b7857a4..2d2f07acafa8 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -857,7 +857,6 @@ struct file_operations {
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
int (*iterate) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
- struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
__poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
@@ -903,16 +902,12 @@ otherwise noted.
activity on this file and (optionally) go to sleep until there
is activity. Called by the select(2) and poll(2) system calls
- get_poll_head: Returns the struct wait_queue_head that callers can
- wait on. Callers need to check the returned events using ->poll_mask
- once woken. Can return NULL to indicate polling is not supported,
- or any error code using the ERR_PTR convention to indicate that a
- grave error occured and ->poll_mask shall not be called.
-
poll_mask: return the mask of EPOLL* values describing the file descriptor
- state. Called either before going to sleep on the waitqueue returned by
- get_poll_head, or after it has been woken. If ->get_poll_head and
- ->poll_mask are implemented ->poll does not need to be implement.
+ state. Called either before going to sleep on ->f_poll_head, or after it
+ has been woken. On files that support ->poll_mask the ->f_poll_head field
+ must point to a wait_queue_head that poll can wait on. The waitqueue must
+ have the same (or a longer) life time as the struct file itself.
+ If ->poll_mask is implemented ->poll does not need to be implement.
unlocked_ioctl: called by the ioctl(2) system call.
diff --git a/drivers/char/random.c b/drivers/char/random.c
index a8fb0020ba5c..e6260a9fa3b9 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1875,10 +1875,10 @@ urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
return ret;
}
-static struct wait_queue_head *
-random_get_poll_head(struct file *file, __poll_t events)
+static int random_open(struct inode *inode, struct file *file)
{
- return &random_wait;
+ file->f_poll_head = &random_wait;
+ return 0;
}
static __poll_t
@@ -1990,9 +1990,9 @@ static int random_fasync(int fd, struct file *filp, int on)
}
const struct file_operations random_fops = {
+ .open = random_open,
.read = random_read,
.write = random_write,
- .get_poll_head = random_get_poll_head,
.poll_mask = random_poll_mask,
.unlocked_ioctl = random_ioctl,
.fasync = random_fasync,
diff --git a/fs/aio.c b/fs/aio.c
index e1d20124ec0e..84f498df8afc 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -168,7 +168,6 @@ struct fsync_iocb {
struct poll_iocb {
struct file *file;
__poll_t events;
- struct wait_queue_head *head;
union {
struct wait_queue_entry wait;
@@ -1632,7 +1631,7 @@ static int aio_poll_cancel(struct kiocb *iocb)
{
struct aio_kiocb *aiocb = container_of(iocb, struct aio_kiocb, rw);
struct poll_iocb *req = &aiocb->poll;
- struct wait_queue_head *head = req->head;
+ struct wait_queue_head *head = req->file->f_poll_head;
bool found = false;
spin_lock(&head->lock);
@@ -1655,7 +1654,7 @@ static int aio_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
struct file *file = req->file;
__poll_t mask = key_to_poll(key);
- assert_spin_locked(&req->head->lock);
+ assert_spin_locked(&file->f_poll_head->lock);
/* for instances that support it check for an event match first: */
if (mask && !(mask & req->events))
@@ -1703,30 +1702,21 @@ static ssize_t aio_poll(struct aio_kiocb *aiocb, struct iocb *iocb)
req->file = fget(iocb->aio_fildes);
if (unlikely(!req->file))
return -EBADF;
- if (!file_has_poll_mask(req->file))
+ if (!req->file->f_poll_head)
goto out_fail;
- req->head = req->file->f_op->get_poll_head(req->file, req->events);
- if (!req->head)
- goto out_fail;
- if (IS_ERR(req->head)) {
- mask = EPOLLERR;
- goto done;
- }
-
init_waitqueue_func_entry(&req->wait, aio_poll_wake);
aiocb->ki_cancel = aio_poll_cancel;
spin_lock_irq(&ctx->ctx_lock);
- spin_lock(&req->head->lock);
+ spin_lock(&req->file->f_poll_head->lock);
mask = req->file->f_op->poll_mask(req->file, req->events) & req->events;
if (!mask) {
- __add_wait_queue(req->head, &req->wait);
+ __add_wait_queue(req->file->f_poll_head, &req->wait);
list_add_tail(&aiocb->ki_list, &ctx->active_reqs);
}
- spin_unlock(&req->head->lock);
+ spin_unlock(&req->file->f_poll_head->lock);
spin_unlock_irq(&ctx->ctx_lock);
-done:
if (mask)
__aio_poll_complete(aiocb, mask);
return 0;
diff --git a/fs/eventfd.c b/fs/eventfd.c
index ceb1031f1cac..8904b127577b 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -101,14 +101,6 @@ static int eventfd_release(struct inode *inode, struct file *file)
return 0;
}
-static struct wait_queue_head *
-eventfd_get_poll_head(struct file *file, __poll_t events)
-{
- struct eventfd_ctx *ctx = file->private_data;
-
- return &ctx->wqh;
-}
-
static __poll_t eventfd_poll_mask(struct file *file, __poll_t eventmask)
{
struct eventfd_ctx *ctx = file->private_data;
@@ -311,7 +303,6 @@ static const struct file_operations eventfd_fops = {
.show_fdinfo = eventfd_show_fdinfo,
#endif
.release = eventfd_release,
- .get_poll_head = eventfd_get_poll_head,
.poll_mask = eventfd_poll_mask,
.read = eventfd_read,
.write = eventfd_write,
@@ -390,7 +381,8 @@ EXPORT_SYMBOL_GPL(eventfd_ctx_fileget);
static int do_eventfd(unsigned int count, int flags)
{
struct eventfd_ctx *ctx;
- int fd;
+ struct file *file;
+ int fd, error;
/* Check the EFD_* constants for consistency. */
BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
@@ -408,12 +400,27 @@ static int do_eventfd(unsigned int count, int flags)
ctx->count = count;
ctx->flags = flags;
- fd = anon_inode_getfd("[eventfd]", &eventfd_fops, ctx,
+ error = get_unused_fd_flags(O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
+ if (error < 0)
+ goto err_free_ctx;
+ fd = error;
+
+ file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx,
O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
- if (fd < 0)
- eventfd_free_ctx(ctx);
+ if (IS_ERR(file)) {
+ error = PTR_ERR(file);
+ goto err_put_unused_fd;
+ }
+ file->f_poll_head = &ctx->wqh;
+ fd_install(fd, file);
return fd;
+
+err_put_unused_fd:
+ put_unused_fd(fd);
+err_free_ctx:
+ eventfd_free_ctx(ctx);
+ return error;
}
SYSCALL_DEFINE2(eventfd2, unsigned int, count, int, flags)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index ea4436f409fb..a0d199be91db 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -922,13 +922,6 @@ static __poll_t ep_read_events_proc(struct eventpoll *ep, struct list_head *head
return 0;
}
-static struct wait_queue_head *ep_eventpoll_get_poll_head(struct file *file,
- __poll_t eventmask)
-{
- struct eventpoll *ep = file->private_data;
- return &ep->poll_wait;
-}
-
static __poll_t ep_eventpoll_poll_mask(struct file *file, __poll_t eventmask)
{
struct eventpoll *ep = file->private_data;
@@ -972,7 +965,6 @@ static const struct file_operations eventpoll_fops = {
.show_fdinfo = ep_show_fdinfo,
#endif
.release = ep_eventpoll_release,
- .get_poll_head = ep_eventpoll_get_poll_head,
.poll_mask = ep_eventpoll_poll_mask,
.llseek = noop_llseek,
};
@@ -1973,6 +1965,7 @@ static int do_epoll_create(int flags)
goto out_free_fd;
}
ep->file = file;
+ file->f_poll_head = &ep->poll_wait;
fd_install(fd, file);
return fd;
diff --git a/fs/pipe.c b/fs/pipe.c
index bb0840e234f3..6817a60bebc9 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -509,14 +509,6 @@ static long pipe_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
}
}
-static struct wait_queue_head *
-pipe_get_poll_head(struct file *filp, __poll_t events)
-{
- struct pipe_inode_info *pipe = filp->private_data;
-
- return &pipe->wait;
-}
-
/* No kernel lock held - fine */
static __poll_t pipe_poll_mask(struct file *filp, __poll_t events)
{
@@ -768,6 +760,7 @@ int create_pipe_files(struct file **res, int flags)
f->f_flags = O_WRONLY | (flags & (O_NONBLOCK | O_DIRECT));
f->private_data = inode->i_pipe;
+ f->f_poll_head = &inode->i_pipe->wait;
res[0] = alloc_file(&path, FMODE_READ, &pipefifo_fops);
if (IS_ERR(res[0])) {
@@ -778,6 +771,7 @@ int create_pipe_files(struct file **res, int flags)
path_get(&path);
res[0]->private_data = inode->i_pipe;
res[0]->f_flags = O_RDONLY | (flags & O_NONBLOCK);
+ res[0]->f_poll_head = &inode->i_pipe->wait;
res[1] = f;
return 0;
@@ -930,6 +924,7 @@ static int fifo_open(struct inode *inode, struct file *filp)
/* We can only do regular read/write on fifos */
filp->f_mode &= (FMODE_READ | FMODE_WRITE);
+ filp->f_poll_head = &pipe->wait;
switch (filp->f_mode) {
case FMODE_READ:
@@ -1023,7 +1018,6 @@ const struct file_operations pipefifo_fops = {
.llseek = no_llseek,
.read_iter = pipe_read,
.write_iter = pipe_write,
- .get_poll_head = pipe_get_poll_head,
.poll_mask = pipe_poll_mask,
.unlocked_ioctl = pipe_ioctl,
.release = pipe_release,
diff --git a/fs/select.c b/fs/select.c
index 317891ff8165..26e20063454a 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -38,20 +38,10 @@ __poll_t vfs_poll(struct file *file, struct poll_table_struct *pt)
{
if (file->f_op->poll) {
return file->f_op->poll(file, pt);
- } else if (file_has_poll_mask(file)) {
- unsigned int events = poll_requested_events(pt);
- struct wait_queue_head *head;
-
- if (pt && pt->_qproc) {
- head = file->f_op->get_poll_head(file, events);
- if (!head)
- return DEFAULT_POLLMASK;
- if (IS_ERR(head))
- return EPOLLERR;
- pt->_qproc(file, head, pt);
- }
-
- return file->f_op->poll_mask(file, events);
+ } else if (file->f_poll_head) {
+ if (pt && pt->_qproc)
+ pt->_qproc(file, file->f_poll_head, pt);
+ return file->f_op->poll_mask(file, poll_requested_events(pt));
} else {
return DEFAULT_POLLMASK;
}
diff --git a/fs/timerfd.c b/fs/timerfd.c
index d84a2bee4f82..08e820166c4d 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -227,14 +227,6 @@ static int timerfd_release(struct inode *inode, struct file *file)
return 0;
}
-static struct wait_queue_head *timerfd_get_poll_head(struct file *file,
- __poll_t eventmask)
-{
- struct timerfd_ctx *ctx = file->private_data;
-
- return &ctx->wqh;
-}
-
static __poll_t timerfd_poll_mask(struct file *file, __poll_t eventmask)
{
struct timerfd_ctx *ctx = file->private_data;
@@ -363,7 +355,6 @@ static long timerfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg
static const struct file_operations timerfd_fops = {
.release = timerfd_release,
- .get_poll_head = timerfd_get_poll_head,
.poll_mask = timerfd_poll_mask,
.read = timerfd_read,
.llseek = noop_llseek,
@@ -386,8 +377,9 @@ static int timerfd_fget(int fd, struct fd *p)
SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
{
- int ufd;
+ int ufd, error;
struct timerfd_ctx *ctx;
+ struct file *file;
/* Check the TFD_* constants for consistency. */
BUILD_BUG_ON(TFD_CLOEXEC != O_CLOEXEC);
@@ -424,12 +416,25 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
ctx->moffs = ktime_mono_to_real(0);
- ufd = anon_inode_getfd("[timerfd]", &timerfd_fops, ctx,
+ error = get_unused_fd_flags(O_RDWR | (flags & TFD_SHARED_FCNTL_FLAGS));
+ if (error < 0)
+ goto out_free_ctx;
+ ufd = error;
+
+ file = anon_inode_getfile("[timerfd]", &timerfd_fops, ctx,
O_RDWR | (flags & TFD_SHARED_FCNTL_FLAGS));
- if (ufd < 0)
- kfree(ctx);
+ if (IS_ERR(file)) {
+ error = PTR_ERR(file);
+ goto err_put_unused_fd;
+ }
+ file->f_poll_head = &ctx->wqh;
+ fd_install(ufd, file);
return ufd;
+err_put_unused_fd:
+ put_unused_fd(ufd);
+out_free_ctx:
+ return error;
}
static int do_timerfd_settime(int ufd, int flags,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5c91108846db..cc0fb83d3743 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -878,6 +878,7 @@ struct file {
loff_t f_pos;
struct fown_struct f_owner;
const struct cred *f_cred;
+ struct wait_queue_head *f_poll_head;
struct file_ra_state f_ra;
u64 f_version;
@@ -1720,7 +1721,6 @@ struct file_operations {
int (*iterate) (struct file *, struct dir_context *);
int (*iterate_shared) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
- struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
__poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
diff --git a/include/linux/poll.h b/include/linux/poll.h
index fdf86b4cbc71..e3dd9dfee20a 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -74,14 +74,9 @@ static inline void init_poll_funcptr(poll_table *pt, poll_queue_proc qproc)
pt->_key = ~(__poll_t)0; /* all events enabled */
}
-static inline bool file_has_poll_mask(struct file *file)
-{
- return file->f_op->get_poll_head && file->f_op->poll_mask;
-}
-
static inline bool file_can_poll(struct file *file)
{
- return file->f_op->poll || file_has_poll_mask(file);
+ return file->f_op->poll || file->f_poll_head;
}
__poll_t vfs_poll(struct file *file, struct poll_table_struct *pt);
diff --git a/net/socket.c b/net/socket.c
index 4354afe8ad96..155d4ba38645 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -117,8 +117,6 @@ static ssize_t sock_write_iter(struct kiocb *iocb, struct iov_iter *from);
static int sock_mmap(struct file *file, struct vm_area_struct *vma);
static int sock_close(struct inode *inode, struct file *file);
-static struct wait_queue_head *sock_get_poll_head(struct file *file,
- __poll_t events);
static __poll_t sock_poll_mask(struct file *file, __poll_t);
static __poll_t sock_poll(struct file *file, struct poll_table_struct *wait);
static long sock_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
@@ -143,7 +141,6 @@ static const struct file_operations socket_file_ops = {
.llseek = no_llseek,
.read_iter = sock_read_iter,
.write_iter = sock_write_iter,
- .get_poll_head = sock_get_poll_head,
.poll_mask = sock_poll_mask,
.poll = sock_poll,
.unlocked_ioctl = sock_ioctl,
@@ -422,6 +419,8 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
sock->file = file;
file->f_flags = O_RDWR | (flags & O_NONBLOCK);
+ if (sock->ops->poll_mask)
+ file->f_poll_head = &sock->wq->wait;
file->private_data = sock;
return file;
}
@@ -1128,16 +1127,6 @@ int sock_create_lite(int family, int type, int protocol, struct socket **res)
}
EXPORT_SYMBOL(sock_create_lite);
-static struct wait_queue_head *sock_get_poll_head(struct file *file,
- __poll_t events)
-{
- struct socket *sock = file->private_data;
-
- if (!sock->ops->poll_mask)
- return NULL;
- return &sock->wq->wait;
-}
-
static __poll_t sock_poll_mask(struct file *file, __poll_t events)
{
struct socket *sock = file->private_data;
@@ -1167,7 +1156,7 @@ static __poll_t sock_poll(struct file *file, poll_table *wait)
if (sock->ops->poll) {
mask = sock->ops->poll(file, sock, wait);
} else if (sock->ops->poll_mask) {
- sock_poll_wait(file, sock_get_poll_head(file, events), wait);
+ sock_poll_wait(file, &sock->wq->wait, wait);
mask = sock->ops->poll_mask(sock, events);
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v1 net-next 02/14] net: Add a new socket option for a future transmit time.
From: Willem de Bruijn @ 2018-06-28 14:26 UTC (permalink / raw)
To: Jesus Sanchez-Palencia
Cc: Network Development, Thomas Gleixner, jan.altenberg,
Vinicius Gomes, kurt.kanzenbach, Henrik Austad, Richard Cochran,
Levi Pearson, ilias.apalodimas, ivan.khoronzhuk, Miroslav Lichvar,
Willem de Bruijn, Jamal Hadi Salim, Cong Wang,
Jiří Pírko, Richard Cochran
In-Reply-To: <20180627215950.6719-3-jesus.sanchez-palencia@intel.com>
On Wed, Jun 27, 2018 at 6:08 PM Jesus Sanchez-Palencia
<jesus.sanchez-palencia@intel.com> wrote:
>
> From: Richard Cochran <rcochran@linutronix.de>
>
> This patch introduces SO_TXTIME. User space enables this option in
> order to pass a desired future transmit time in a CMSG when calling
> sendmsg(2). The argument to this socket option is a 6-bytes long struct
> defined as:
>
> struct sock_txtime {
> clockid_t clockid;
> u16 flags;
> };
clockid_t is __kernel_clockid_t is int is a variable length field.
Please use fixed
length fields. Also, as MAX_CLOCKS is 16, only 4 bits are needed. A single u16
is probably sufficient as cmsg argument. To future proof, a u32 will
allow for more
than 4 flags. But in struct sock, 16 bits should be sufficient to
encode both clock id
and flags.
> Note that two new fields were added to struct sock by filling a 4-bytes
> hole found in the struct. For that reason, neither the struct size or
> number of cachelines were altered.
>
> Signed-off-by: Richard Cochran <rcochran@linutronix.de>
> Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
> ---
> +#include <asm/unaligned.h>
> #include <linux/capability.h>
> #include <linux/errno.h>
> #include <linux/errqueue.h>
> @@ -697,6 +698,7 @@ EXPORT_SYMBOL(sk_mc_loop);
> int sock_setsockopt(struct socket *sock, int level, int optname,
> char __user *optval, unsigned int optlen)
> {
> + struct sock_txtime sk_txtime;
> struct sock *sk = sock->sk;
> int val;
> int valbool;
> @@ -1070,6 +1072,22 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
> }
> break;
>
> + case SO_TXTIME:
> + if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)) {
> + ret = -EPERM;
> + } else if (optlen != sizeof(struct sock_txtime)) {
> + ret = -EINVAL;
> + } else if (copy_from_user(&sk_txtime, optval,
> + sizeof(struct sock_txtime))) {
> + ret = -EFAULT;
> + sock_valbool_flag(sk, SOCK_TXTIME, false);
Why change sk state on failure? This is not customary.
> + } else {
> + sock_valbool_flag(sk, SOCK_TXTIME, true);
> + sk->sk_clockid = sk_txtime.clockid;
> + sk->sk_txtime_flags = sk_txtime.flags;
Validate input and fail on undefined flags.
> @@ -2137,6 +2162,13 @@ int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg,
> sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK;
> sockc->tsflags |= tsflags;
> break;
> + case SCM_TXTIME:
> + if (!sock_flag(sk, SOCK_TXTIME))
> + return -EINVAL;
Note that on lockfree datapaths like udp this test can race with the
setsockopt above.
It seems harmless here.
> + if (cmsg->cmsg_len != CMSG_LEN(sizeof(u64)))
> + return -EINVAL;
> + sockc->transmit_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
> + break;
> /* SCM_RIGHTS and SCM_CREDENTIALS are semantically in SOL_UNIX. */
> case SCM_RIGHTS:
> case SCM_CREDENTIALS:
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v1 net-next 03/14] net: ipv4: Hook into time based transmission
From: Willem de Bruijn @ 2018-06-28 14:26 UTC (permalink / raw)
To: Jesus Sanchez-Palencia
Cc: Network Development, Thomas Gleixner, jan.altenberg,
Vinicius Gomes, kurt.kanzenbach, Henrik Austad, Richard Cochran,
Levi Pearson, ilias.apalodimas, ivan.khoronzhuk, Miroslav Lichvar,
Willem de Bruijn, Jamal Hadi Salim, Cong Wang,
Jiří Pírko, Richard Cochran
In-Reply-To: <20180627215950.6719-4-jesus.sanchez-palencia@intel.com>
On Wed, Jun 27, 2018 at 6:07 PM Jesus Sanchez-Palencia
<jesus.sanchez-palencia@intel.com> wrote:
>
> Add a transmit_time field to struct inet_cork, then copy the
> timestamp from the CMSG cookie at ip_setup_cork() so we can
> safely copy it into the skb later during __ip_make_skb().
>
> For the raw fast path, just perform the copy at raw_send_hdrinc().
>
> Signed-off-by: Richard Cochran <rcochran@linutronix.de>
> Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
> ---
> include/net/inet_sock.h | 1 +
> net/ipv4/ip_output.c | 3 +++
> net/ipv4/raw.c | 2 ++
> net/ipv4/udp.c | 1 +
Also support the feature for ipv6
> 4 files changed, 7 insertions(+)
>
> diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
> index 83d5b3c2ac42..314be484c696 100644
> --- a/include/net/inet_sock.h
> +++ b/include/net/inet_sock.h
> @@ -148,6 +148,7 @@ struct inet_cork {
> __s16 tos;
> char priority;
> __u16 gso_size;
> + u64 transmit_time;
> };
>
> struct inet_cork_full {
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index b3308e9d9762..904a54a090e9 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -1153,6 +1153,7 @@ static int ip_setup_cork(struct sock *sk, struct inet_cork *cork,
> cork->tos = ipc->tos;
> cork->priority = ipc->priority;
> cork->tx_flags = ipc->tx_flags;
> + cork->transmit_time = ipc->sockc.transmit_time;
Initialize ipc->sockc.transmit_time in all possible paths to avoid bugs like the
one fixed in commit 9887cba19978 ("ip: limit use of gso_size to udp").
> return 0;
> }
> @@ -1413,6 +1414,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
>
> skb->priority = (cork->tos != -1) ? cork->priority: sk->sk_priority;
> skb->mark = sk->sk_mark;
> + skb->tstamp = cork->transmit_time;
> /*
> * Steal rt from cork.dst to avoid a pair of atomic_inc/atomic_dec
> * on dst refcount
> @@ -1495,6 +1497,7 @@ struct sk_buff *ip_make_skb(struct sock *sk,
> cork->flags = 0;
> cork->addr = 0;
> cork->opt = NULL;
> + cork->transmit_time = 0;
Not needed when unconditionally overwriting the field in ip_setup_cork.
> err = ip_setup_cork(sk, cork, ipc, rtp);
> if (err)
> return ERR_PTR(err);
^ permalink raw reply
* Re: [PATCH v1 net-next 13/14] net/sched: Enforce usage of CLOCK_TAI for sch_etf
From: Willem de Bruijn @ 2018-06-28 14:26 UTC (permalink / raw)
To: Jesus Sanchez-Palencia
Cc: Network Development, Thomas Gleixner, jan.altenberg,
Vinicius Gomes, kurt.kanzenbach, Henrik Austad, Richard Cochran,
Levi Pearson, ilias.apalodimas, ivan.khoronzhuk, Miroslav Lichvar,
Willem de Bruijn, Jamal Hadi Salim, Cong Wang,
Jiří Pírko
In-Reply-To: <20180627215950.6719-14-jesus.sanchez-palencia@intel.com>
On Wed, Jun 27, 2018 at 8:45 PM Jesus Sanchez-Palencia
<jesus.sanchez-palencia@intel.com> wrote:
>
> The qdisc and the SO_TXTIME ABIs allow for a clockid to be configured,
> but it's been decided that usage of CLOCK_TAI should be enforced until
> we decide to allow for other clockids to be used. The rationale here is
> that PTP times are usually in the TAI scale, thus no other clocks should
> be necessary.
>
> For now, the qdisc will return EINVAL if any clocks other than
> CLOCK_TAI are used.
>
> Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
> ---
> net/sched/sch_etf.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/sch_etf.c b/net/sched/sch_etf.c
> index cd6cb5b69228..5514a8aa3bd5 100644
> --- a/net/sched/sch_etf.c
> +++ b/net/sched/sch_etf.c
> @@ -56,8 +56,8 @@ static inline int validate_input_params(struct tc_etf_qopt *qopt,
> return -ENOTSUPP;
> }
>
> - if (qopt->clockid >= MAX_CLOCKS) {
> - NL_SET_ERR_MSG(extack, "Invalid clockid");
> + if (qopt->clockid != CLOCK_TAI) {
> + NL_SET_ERR_MSG(extack, "Invalid clockid. CLOCK_TAI must be used");
Similar to the comment in patch 12, this should be squashed (into
patch 6) to avoid incorrect behavior in a range of SHA1s.
^ permalink raw reply
* Re: [PATCH v1 net-next 14/14] net/sched: Make etf report drops on error_queue
From: Willem de Bruijn @ 2018-06-28 14:27 UTC (permalink / raw)
To: Jesus Sanchez-Palencia
Cc: Network Development, Thomas Gleixner, jan.altenberg,
Vinicius Gomes, kurt.kanzenbach, Henrik Austad, Richard Cochran,
Levi Pearson, ilias.apalodimas, ivan.khoronzhuk, Miroslav Lichvar,
Willem de Bruijn, Jamal Hadi Salim, Cong Wang,
Jiří Pírko
In-Reply-To: <20180627215950.6719-15-jesus.sanchez-palencia@intel.com>
On Wed, Jun 27, 2018 at 6:07 PM Jesus Sanchez-Palencia
<jesus.sanchez-palencia@intel.com> wrote:
>
> Use the socket error queue for reporting dropped packets if the
> socket has enabled that feature through the SO_TXTIME API.
>
> Packets are dropped either on enqueue() if they aren't accepted by the
> qdisc or on dequeue() if the system misses their deadline. Those are
> reported as different errors so applications can react accordingly.
>
> Userspace can retrieve the errors through the socket error queue and the
> corresponding cmsg interfaces. A struct sock_extended_err* is used for
> returning the error data, and the packet's timestamp can be retrieved by
> adding both ee_data and ee_info fields as e.g.:
>
> ((__u64) serr->ee_data << 32) + serr->ee_info
>
> This feature is disabled by default and must be explicitly enabled by
> applications. Enabling it can bring some overhead for the Tx cycles
> of the application.
>
> Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
> ---
> struct sock_txtime {
> clockid_t clockid; /* reference clockid */
> - u16 flags; /* bit 0: txtime in deadline_mode */
> + u16 flags; /* bit 0: txtime in deadline_mode
> + * bit 1: report drops on sk err queue
> + */
> };
If this is shared with userspace, should be defined in an uapi header.
Same on the flag bits below. Self documenting code is preferable over
comments.
> /*
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 73f4404e49e4..e681a45cfe7e 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -473,6 +473,7 @@ struct sock {
> u16 sk_clockid;
> u16 sk_txtime_flags;
> #define SK_TXTIME_DEADLINE_MASK BIT(0)
> +#define SK_TXTIME_RECV_ERR_MASK BIT(1)
Integer bitfields are (arguably) more readable. There is no requirement
that the user interface be the same as the in-kernel implementation. Indeed
if you can save bits in struct sock, that is preferable (but not so for the ABI,
which cannot easily be extended).
>
> struct socket *sk_socket;
> void *sk_user_data;
> diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
> index dc64cfaf13da..66fd5e443c94 100644
> --- a/include/uapi/linux/errqueue.h
> +++ b/include/uapi/linux/errqueue.h
> @@ -25,6 +25,8 @@ struct sock_extended_err {
> #define SO_EE_OFFENDER(ee) ((struct sockaddr*)((ee)+1))
>
> #define SO_EE_CODE_ZEROCOPY_COPIED 1
> +#define SO_EE_CODE_TXTIME_INVALID_PARAM 2
> +#define SO_EE_CODE_TXTIME_MISSED 3
>
> /**
> * struct scm_timestamping - timestamps exposed through cmsg
> diff --git a/net/sched/sch_etf.c b/net/sched/sch_etf.c
> index 5514a8aa3bd5..166f4b72875b 100644
> --- a/net/sched/sch_etf.c
> +++ b/net/sched/sch_etf.c
> @@ -11,6 +11,7 @@
> #include <linux/kernel.h>
> #include <linux/string.h>
> #include <linux/errno.h>
> +#include <linux/errqueue.h>
> #include <linux/rbtree.h>
> #include <linux/skbuff.h>
> #include <linux/posix-timers.h>
> @@ -124,6 +125,35 @@ static void reset_watchdog(struct Qdisc *sch)
> qdisc_watchdog_schedule_ns(&q->watchdog, ktime_to_ns(next));
> }
>
> +static void report_sock_error(struct sk_buff *skb, u32 err, u8 code)
> +{
> + struct sock_exterr_skb *serr;
> + ktime_t txtime = skb->tstamp;
> +
> + if (!skb->sk || !(skb->sk->sk_txtime_flags & SK_TXTIME_RECV_ERR_MASK))
> + return;
> +
> + skb = skb_clone_sk(skb);
> + if (!skb)
> + return;
> +
> + sock_hold(skb->sk);
Why take an extra reference? The skb holds a ref on the sk.
> +
> + serr = SKB_EXT_ERR(skb);
> + serr->ee.ee_errno = err;
> + serr->ee.ee_origin = SO_EE_ORIGIN_LOCAL;
I suggest adding a new SO_EE_ORIGIN_TXTIME as opposed to overloading
the existing
local origin. Then the EE_CODE can start at 1, as ee_code can be
demultiplexed by origin.
> + serr->ee.ee_type = 0;
> + serr->ee.ee_code = code;
> + serr->ee.ee_pad = 0;
> + serr->ee.ee_data = (txtime >> 32); /* high part of tstamp */
> + serr->ee.ee_info = txtime; /* low part of tstamp */
> +
> + if (sock_queue_err_skb(skb->sk, skb))
> + kfree_skb(skb);
> +
> + sock_put(skb->sk);
> +}
^ permalink raw reply
* Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Jiri Pirko @ 2018-06-28 14:29 UTC (permalink / raw)
To: David Ahern
Cc: netdev, davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, mlxsw, sridhar.samudrala
In-Reply-To: <3660d513-1f0c-fa7a-0ac2-9f89aec89389@gmail.com>
Thu, Jun 28, 2018 at 04:18:47PM CEST, dsahern@gmail.com wrote:
>On 6/28/18 7:08 AM, Jiri Pirko wrote:
>> Create dummy device with clsact first:
>> # ip link add type dummy
>> # tc qdisc add dev dummy0 clsact
>>
>> There is no template assigned by default:
>> # tc chaintemplate show dev dummy0 ingress
>>
>> Add a template of type flower allowing to insert rules matching on last
>> 2 bytes of destination mac address:
>> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>
>> The template is now showed in the list:
>> # tc chaintemplate show dev dummy0 ingress
>> chaintemplate flower chain 0
>> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>> eth_type ipv4
>>
>> Add another template, this time for chain number 22:
>> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>> # tc chaintemplate show dev dummy0 ingress
>> chaintemplate flower chain 0
>> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>> eth_type ipv4
>> chaintemplate flower chain 22
>> eth_type ipv4
>> dst_ip 0.0.0.0/16
>>
>> Add a filter that fits the template:
>> # tc filter add dev dummy0 ingress proto ip flower dst_mac aa:bb:cc:dd:ee:ff/00:00:00:00:00:0F action drop
>>
>> Addition of filters that does not fit the template would fail:
>> # tc filter add dev dummy0 ingress proto ip flower dst_mac aa:11:22:33:44:55/00:00:00:FF:00:00 action drop
>> Error: Mask does not fit the template.
>> We have an error talking to the kernel, -1
>> # tc filter add dev dummy0 ingress proto ip flower dst_ip 10.0.0.1 action drop
>> Error: Mask does not fit the template.
>> We have an error talking to the kernel, -1
>>
>> Additions of filters to chain 22:
>> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/8 action drop
>> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1 action drop
>> Error: Mask does not fit the template.
>> We have an error talking to the kernel, -1
>> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/24 action drop
>> Error: Mask does not fit the template.
>> We have an error talking to the kernel, -1
>>
>> Removal of a template from non-empty chain would fail:
>> # tc chaintemplate del dev dummy0 ingress
>> Error: The chain is not empty, unable to delete template.
>> We have an error talking to the kernel, -1
>
>Why this restriction? It's a template, so why can't it be removed
>regardless of whether there are filters?
That means you could start to insert filters that does not match the
original template. I wanted to avoid it. The chain is utilized in hw for
the original template, the filter insertion would have to be sanitized
in driver. With this restriction, drivers can depend on filters always
be fitting.
>
>>
>> Once the chain is flushed, the template could be removed:
>> # tc filter del dev dummy0 ingress
>> # tc chaintemplate del dev dummy0 ingress
>>
>
^ permalink raw reply
* [PATCH v3 00/18] ARM: davinci: step towards removing at24_platform_data
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Since I took over maintainership of the at24 driver I've been working
towards removing at24_platform_data in favor for device properties.
DaVinci is the only platform that's still using it - all other users
have already been converted.
One of the obstacles in case of DaVinci is removing the setup() callback
from the pdata struct, the only user of which are some davinci boards.
Most boards use the EEPROM to store the MAC address. This series adds
support for cell lookups to the nvmem framework, registers relevant
cells for all users, converts the davinci_emac driver to using them
and replaces at24_platform_data with device properties.
The only board that's still using this callback is now mityomapl138.
Unfortunately it stores more info in EEPROM than just the MAC address
and will require some more work. Unfortunately I don't have access
to this board so I can't test any actual solutions on a live hardware.
Tested on a dm365-evm board.
v1 -> v2:
- for backward compatiblity: fall back to using of_get_mac_address() if
we can't get an nvmem cell in patch 7
- add patch 15 which removes dead code
v2 -> v3:
- documented new nvmem cell lookup API
- renamed the nvmem cell lookup routines
- in order to completly remove the mac_addr from davinci's soc_info
added a patch that moves reading the MAC address from SPI to the emac
driver
- removed more dead code
Bartosz Golaszewski (18):
nvmem: add support for cell lookups
Documentation: nvmem: document lookup entries
ARM: davinci: dm365-evm: use nvmem lookup for mac address
ARM: davinci: dm644-evm: use nvmem lookup for mac address
ARM: davinci: dm646x-evm: use nvmem lookup for mac address
ARM: davinci: da830-evm: use nvmem lookup for mac address
ARM: davinci: mityomapl138: add nvmem cells lookup entries
net: davinci_emac: potentially get the MAC address from MTD
ARM: davinci: da850-evm: remove dead MTD code
net: davinci_emac: use nvmem to retrieve the mac address
ARM: davinci: mityomapl138: don't read the MAC address from machine
code
ARM: davinci: dm365-evm: use device properties for at24 eeprom
ARM: davinci: da830-evm: use device properties for at24 eeprom
ARM: davinci: dm644x-evm: use device properties for at24 eeprom
ARM: davinci: dm646x-evm: use device properties for at24 eeprom
ARM: davinci: sffsdr: fix the at24 eeprom device name
ARM: davinci: sffsdr: use device properties for at24 eeprom
ARM: davinci: remove dead code related to MAC address reading
Documentation/nvmem/nvmem.txt | 28 ++++++++
arch/arm/mach-davinci/board-da830-evm.c | 25 +++++---
arch/arm/mach-davinci/board-da850-evm.c | 28 --------
arch/arm/mach-davinci/board-dm365-evm.c | 25 +++++---
arch/arm/mach-davinci/board-dm644x-evm.c | 24 ++++---
arch/arm/mach-davinci/board-dm646x-evm.c | 25 +++++---
arch/arm/mach-davinci/board-mityomapl138.c | 30 ++++++---
arch/arm/mach-davinci/board-sffsdr.c | 13 ++--
arch/arm/mach-davinci/common.c | 15 -----
drivers/net/ethernet/ti/davinci_emac.c | 50 ++++++++++++---
drivers/nvmem/core.c | 75 +++++++++++++++++++++-
include/linux/davinci_emac.h | 2 -
include/linux/nvmem-consumer.h | 6 ++
include/linux/nvmem-provider.h | 10 +++
14 files changed, 253 insertions(+), 103 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v3 01/18] nvmem: add support for cell lookups
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: netdev, linux-omap, linux-kernel, linux-arm-kernel,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We can currently only register nvmem cells from device tree or by
manually calling nvmem_add_cells(). The latter options however forces
users to make sure that the nvmem provider with which the cells are
associated is registered before the call.
This patch proposes a new solution inspired by other frameworks that
offer resource lookups (GPIO, PWM etc.). It adds functions that allow
machine code to register nvmem lookup which are later lazily used to
add corresponding nvmem cells and remove them if no longer needed.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/nvmem/core.c | 75 +++++++++++++++++++++++++++++++++-
include/linux/nvmem-consumer.h | 6 +++
include/linux/nvmem-provider.h | 10 +++++
3 files changed, 90 insertions(+), 1 deletion(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index b5b0cdc21d01..c4889a4b26cf 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -62,6 +62,9 @@ static DEFINE_IDA(nvmem_ida);
static LIST_HEAD(nvmem_cells);
static DEFINE_MUTEX(nvmem_cells_mutex);
+static LIST_HEAD(nvmem_cell_lookups);
+static DEFINE_MUTEX(nvmem_lookup_mutex);
+
#ifdef CONFIG_DEBUG_LOCK_ALLOC
static struct lock_class_key eeprom_lock_key;
#endif
@@ -247,6 +250,41 @@ static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
NULL,
};
+/**
+ * nvmem_add_lookup_table() - register a number of nvmem cell lookup entries
+ *
+ * @lookup: array of nvmem cell lookup entries
+ * @nentries: number of lookup entries in the array
+ */
+void nvmem_add_lookup_table(struct nvmem_cell_lookup *lookup, size_t nentries)
+{
+ int i;
+
+ mutex_lock(&nvmem_lookup_mutex);
+ for (i = 0; i < nentries; i++)
+ list_add_tail(&lookup[i].list, &nvmem_cell_lookups);
+ mutex_unlock(&nvmem_lookup_mutex);
+}
+EXPORT_SYMBOL_GPL(nvmem_add_lookup_table);
+
+/**
+ * nvmem_del_lookup_table() - unregister a set of previously added nvmem cell
+ * lookup entries
+ *
+ * @lookup: array of nvmem cell lookup entries
+ * @nentries: number of lookup entries in the array
+ */
+void nvmem_del_lookup_table(struct nvmem_cell_lookup *lookup, size_t nentries)
+{
+ int i;
+
+ mutex_lock(&nvmem_lookup_mutex);
+ for (i = 0; i < nentries; i++)
+ list_del(&lookup[i].list);
+ mutex_unlock(&nvmem_lookup_mutex);
+}
+EXPORT_SYMBOL_GPL(nvmem_del_lookup_table);
+
static void nvmem_release(struct device *dev)
{
struct nvmem_device *nvmem = to_nvmem_device(dev);
@@ -916,6 +954,37 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
#endif
+static struct nvmem_cell *nvmem_cell_from_lookup(const char *cell_id)
+{
+ struct nvmem_cell *cell = ERR_PTR(-EPROBE_DEFER);
+ struct nvmem_cell_lookup *lookup;
+ struct nvmem_device *nvmem;
+ int rc;
+
+ mutex_lock(&nvmem_lookup_mutex);
+
+ list_for_each_entry(lookup, &nvmem_cell_lookups, list) {
+ if (strcmp(cell_id, lookup->info.name) == 0) {
+ nvmem = nvmem_find(lookup->nvmem_name);
+ if (!nvmem)
+ goto out;
+
+ rc = nvmem_add_cells(nvmem, &lookup->info, 1);
+ if (rc) {
+ cell = ERR_PTR(rc);
+ goto out;
+ }
+
+ cell = nvmem_cell_get_from_list(cell_id);
+ break;
+ }
+ }
+
+out:
+ mutex_unlock(&nvmem_lookup_mutex);
+ return cell;
+}
+
/**
* nvmem_cell_get() - Get nvmem cell of device form a given cell name
*
@@ -936,7 +1005,11 @@ struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *cell_id)
return cell;
}
- return nvmem_cell_get_from_list(cell_id);
+ cell = nvmem_cell_get_from_list(cell_id);
+ if (!IS_ERR(cell) || PTR_ERR(cell) == -EPROBE_DEFER)
+ return cell;
+
+ return nvmem_cell_from_lookup(cell_id);
}
EXPORT_SYMBOL_GPL(nvmem_cell_get);
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index 4e85447f7860..f4b5d3186e94 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -29,6 +29,12 @@ struct nvmem_cell_info {
unsigned int nbits;
};
+struct nvmem_cell_lookup {
+ struct nvmem_cell_info info;
+ struct list_head list;
+ const char *nvmem_name;
+};
+
#if IS_ENABLED(CONFIG_NVMEM)
/* Cell based interface */
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 24def6ad09bb..6a17d722062b 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -17,6 +17,7 @@
struct nvmem_device;
struct nvmem_cell_info;
+struct nvmem_cell_lookup;
typedef int (*nvmem_reg_read_t)(void *priv, unsigned int offset,
void *val, size_t bytes);
typedef int (*nvmem_reg_write_t)(void *priv, unsigned int offset,
@@ -72,6 +73,9 @@ struct nvmem_config {
struct nvmem_device *nvmem_register(const struct nvmem_config *cfg);
int nvmem_unregister(struct nvmem_device *nvmem);
+void nvmem_add_lookup_table(struct nvmem_cell_lookup *lookup, size_t nentries);
+void nvmem_del_lookup_table(struct nvmem_cell_lookup *lookup, size_t nentries);
+
struct nvmem_device *devm_nvmem_register(struct device *dev,
const struct nvmem_config *cfg);
@@ -92,6 +96,12 @@ static inline int nvmem_unregister(struct nvmem_device *nvmem)
return -ENOSYS;
}
+static inline void
+nvmem_add_lookup_table(struct nvmem_cell_lookup *lookup, size_t nentries) {}
+
+static inline void
+nvmem_del_lookup_table(struct nvmem_cell_lookup *lookup, size_t nentries) {}
+
static inline struct nvmem_device *
devm_nvmem_register(struct device *dev, const struct nvmem_config *c)
{
--
2.17.1
^ permalink raw reply related
* [PATCH v3 02/18] Documentation: nvmem: document lookup entries
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Describe the usage of nvmem cell lookup tables.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
Documentation/nvmem/nvmem.txt | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/Documentation/nvmem/nvmem.txt b/Documentation/nvmem/nvmem.txt
index 8d8d8f58f96f..9d5e3ca2b4f3 100644
--- a/Documentation/nvmem/nvmem.txt
+++ b/Documentation/nvmem/nvmem.txt
@@ -58,6 +58,34 @@ static int qfprom_probe(struct platform_device *pdev)
It is mandatory that the NVMEM provider has a regmap associated with its
struct device. Failure to do would return error code from nvmem_register().
+Additionally it is possible to create nvmem cell lookup entries and register
+them with the nvmem framework from machine code as shown in the example below:
+
+static struct nvmem_cell_lookup foobar_lookup = {
+ .info = {
+ .name = "mac-address",
+ .offset = 0xd000,
+ .bytes = ERH_ALEN,
+ },
+ .nvmem_name = "foobar",
+};
+
+static void foobar_register(void)
+{
+ ...
+ nvmem_add_lookup_table(&foobar_lookup, 1);
+ ...
+}
+
+A lookup entry table can be later removed if needed:
+
+static void foobar_fini(void)
+{
+ ...
+ nvmem_del_lookup_table(&foobar_lookup, 1);
+ ...
+}
+
NVMEM Consumers
+++++++++++++++
--
2.17.1
^ permalink raw reply related
* [PATCH v3 06/18] ARM: davinci: da830-evm: use nvmem lookup for mac address
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We now support nvmem lookups for machine code. Add a lookup for
mac-address.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-da830-evm.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 14a6fc061744..4a2fe8142a2f 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -29,6 +29,7 @@
#include <linux/platform_data/spi-davinci.h>
#include <linux/platform_data/usb-davinci.h>
#include <linux/regulator/machine.h>
+#include <linux/nvmem-provider.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -409,6 +410,15 @@ static inline void da830_evm_init_lcdc(int mux_mode)
static inline void da830_evm_init_lcdc(int mux_mode) { }
#endif
+static struct nvmem_cell_lookup da830_evm_mac_address_cell = {
+ .info = {
+ .name = "mac-address",
+ .offset = 0x7f00,
+ .bytes = ETH_ALEN,
+ },
+ .nvmem_name = "1-00500",
+};
+
static struct at24_platform_data da830_evm_i2c_eeprom_info = {
.byte_len = SZ_256K / 8,
.page_size = 64,
@@ -618,6 +628,8 @@ static __init void da830_evm_init(void)
pr_warn("%s: spi 0 registration failed: %d\n", __func__, ret);
regulator_has_full_constraints();
+
+ nvmem_add_lookup_table(&da830_evm_mac_address_cell, 1);
}
#ifdef CONFIG_SERIAL_8250_CONSOLE
--
2.17.1
^ permalink raw reply related
* [PATCH v3 07/18] ARM: davinci: mityomapl138: add nvmem cells lookup entries
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We now support nvmem lookups for machine code. Add a lookup for
mac-address.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-mityomapl138.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
index 37b3e48a21d1..b5be51c0451e 100644
--- a/arch/arm/mach-davinci/board-mityomapl138.c
+++ b/arch/arm/mach-davinci/board-mityomapl138.c
@@ -21,6 +21,7 @@
#include <linux/etherdevice.h>
#include <linux/spi/spi.h>
#include <linux/spi/flash.h>
+#include <linux/nvmem-provider.h>
#include <asm/io.h>
#include <asm/mach-types.h>
@@ -160,6 +161,25 @@ static void read_factory_config(struct nvmem_device *nvmem, void *context)
mityomapl138_cpufreq_init(partnum);
}
+static struct nvmem_cell_lookup mityomapl138_nvmem_cells[] = {
+ {
+ .info = {
+ .name = "factory-config",
+ .offset = 0x0,
+ .bytes = sizeof(struct factory_config),
+ },
+ .nvmem_name = "1-00500",
+ },
+ {
+ .info = {
+ .name = "mac-address",
+ .offset = 0x64,
+ .bytes = ETH_ALEN,
+ },
+ .nvmem_name = "1-00500",
+ }
+};
+
static struct at24_platform_data mityomapl138_fd_chip = {
.byte_len = 256,
.page_size = 8,
@@ -534,6 +554,8 @@ static void __init mityomapl138_init(void)
if (ret)
pr_warn("spi 1 registration failed: %d\n", ret);
+ nvmem_add_lookup_table(mityomapl138_nvmem_cells,
+ ARRAY_SIZE(mityomapl138_nvmem_cells));
mityomapl138_config_emac();
ret = da8xx_register_rtc();
--
2.17.1
^ permalink raw reply related
* [PATCH v3 08/18] net: davinci_emac: potentially get the MAC address from MTD
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
On da850-evm board we can read the MAC address from MTD. It's currently
done in the relevant board file, but we want to get rid of all the MAC
reading callbacks from the board file (SPI and NAND). Move the reading
of the MAC address from SPI to the emac driver's probe function.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/net/ethernet/ti/davinci_emac.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index a1a6445b5a7e..17cfe093b8cf 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -67,7 +67,7 @@
#include <linux/of_irq.h>
#include <linux/of_net.h>
#include <linux/mfd/syscon.h>
-
+#include <linux/mtd/mtd.h>
#include <asm/irq.h>
#include <asm/page.h>
@@ -1783,7 +1783,10 @@ static int davinci_emac_probe(struct platform_device *pdev)
struct cpdma_params dma_params;
struct clk *emac_clk;
unsigned long emac_bus_frequency;
-
+#ifdef CONFIG_MTD
+ size_t mac_addr_len;
+ struct mtd_info *mtd;
+#endif /* CONFIG_MTD */
/* obtain emac clock from kernel */
emac_clk = devm_clk_get(&pdev->dev, NULL);
@@ -1815,6 +1818,18 @@ static int davinci_emac_probe(struct platform_device *pdev)
goto err_free_netdev;
}
+#ifdef CONFIG_MTD
+ mtd = get_mtd_device_nm("MAC-Address");
+ if (!IS_ERR(mtd)) {
+ rc = mtd_read(mtd, 0, ETH_ALEN,
+ &mac_addr_len, priv->mac_addr);
+ if (rc == 0)
+ dev_info(&pdev->dev,
+ "Read MAC addr from SPI Flash: %pM\n",
+ priv->mac_addr);
+ }
+#endif /* CONFIG_MTD */
+
/* MAC addr and PHY mask , RMII enable info from platform_data */
memcpy(priv->mac_addr, pdata->mac_addr, ETH_ALEN);
priv->phy_id = pdata->phy_id;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 09/18] ARM: davinci: da850-evm: remove dead MTD code
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: netdev, linux-omap, linux-kernel, linux-arm-kernel,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We no longer need to register the MTD notifier to read the MAC address
as it's now being done in the emac address.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-da850-evm.c | 28 -------------------------
1 file changed, 28 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index e22fb40e34bc..4de075458d83 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -124,32 +124,6 @@ static struct spi_board_info da850evm_spi_info[] = {
},
};
-#ifdef CONFIG_MTD
-static void da850_evm_m25p80_notify_add(struct mtd_info *mtd)
-{
- char *mac_addr = davinci_soc_info.emac_pdata->mac_addr;
- size_t retlen;
-
- if (!strcmp(mtd->name, "MAC-Address")) {
- mtd_read(mtd, 0, ETH_ALEN, &retlen, mac_addr);
- if (retlen == ETH_ALEN)
- pr_info("Read MAC addr from SPI Flash: %pM\n",
- mac_addr);
- }
-}
-
-static struct mtd_notifier da850evm_spi_notifier = {
- .add = da850_evm_m25p80_notify_add,
-};
-
-static void da850_evm_setup_mac_addr(void)
-{
- register_mtd_user(&da850evm_spi_notifier);
-}
-#else
-static void da850_evm_setup_mac_addr(void) { }
-#endif
-
static struct mtd_partition da850_evm_norflash_partition[] = {
{
.name = "bootloaders + env",
@@ -1455,8 +1429,6 @@ static __init void da850_evm_init(void)
if (ret)
pr_warn("%s: SATA registration failed: %d\n", __func__, ret);
- da850_evm_setup_mac_addr();
-
ret = da8xx_register_rproc();
if (ret)
pr_warn("%s: dsp/rproc registration failed: %d\n",
--
2.17.1
^ permalink raw reply related
* [PATCH v3 10/18] net: davinci_emac: use nvmem to retrieve the mac address
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
All users which store the MAC address in EEPROM now register relevant
nvmem cells. Switch to retrieving the MAC address over the nvmem
framework. If we can't get the nvmem cell then fall back to using
the device tree.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/net/ethernet/ti/davinci_emac.c | 33 +++++++++++++++++++-------
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index 17cfe093b8cf..7608169e643f 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -68,6 +68,8 @@
#include <linux/of_net.h>
#include <linux/mfd/syscon.h>
#include <linux/mtd/mtd.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/nvmem-consumer.h>
#include <asm/irq.h>
#include <asm/page.h>
@@ -1696,7 +1698,6 @@ davinci_emac_of_get_pdata(struct platform_device *pdev, struct emac_priv *priv)
const struct of_device_id *match;
const struct emac_platform_data *auxdata;
struct emac_platform_data *pdata = NULL;
- const u8 *mac_addr;
if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
return dev_get_platdata(&pdev->dev);
@@ -1708,12 +1709,6 @@ davinci_emac_of_get_pdata(struct platform_device *pdev, struct emac_priv *priv)
np = pdev->dev.of_node;
pdata->version = EMAC_VERSION_2;
- if (!is_valid_ether_addr(pdata->mac_addr)) {
- mac_addr = of_get_mac_address(np);
- if (mac_addr)
- ether_addr_copy(pdata->mac_addr, mac_addr);
- }
-
of_property_read_u32(np, "ti,davinci-ctrl-reg-offset",
&pdata->ctrl_reg_offset);
@@ -1783,10 +1778,12 @@ static int davinci_emac_probe(struct platform_device *pdev)
struct cpdma_params dma_params;
struct clk *emac_clk;
unsigned long emac_bus_frequency;
-#ifdef CONFIG_MTD
+ const void *mac_addr;
size_t mac_addr_len;
+#ifdef CONFIG_MTD
struct mtd_info *mtd;
#endif /* CONFIG_MTD */
+ struct nvmem_cell *cell;
/* obtain emac clock from kernel */
emac_clk = devm_clk_get(&pdev->dev, NULL);
@@ -1830,8 +1827,26 @@ static int davinci_emac_probe(struct platform_device *pdev)
}
#endif /* CONFIG_MTD */
+ cell = nvmem_cell_get(&pdev->dev, "mac-address");
+ if (!IS_ERR(cell)) {
+ mac_addr = nvmem_cell_read(cell, &mac_addr_len);
+ if (!IS_ERR(mac_addr)) {
+ if (is_valid_ether_addr(mac_addr)) {
+ dev_info(&pdev->dev,
+ "Read MAC addr from EEPROM: %pM\n",
+ mac_addr);
+ ether_addr_copy(priv->mac_addr, mac_addr);
+ }
+ kfree(mac_addr);
+ }
+ nvmem_cell_put(cell);
+ } else {
+ mac_addr = of_get_mac_address(np);
+ if (mac_addr)
+ ether_addr_copy(priv->mac_addr, mac_addr);
+ }
+
/* MAC addr and PHY mask , RMII enable info from platform_data */
- memcpy(priv->mac_addr, pdata->mac_addr, ETH_ALEN);
priv->phy_id = pdata->phy_id;
priv->rmii_en = pdata->rmii_en;
priv->version = pdata->version;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 11/18] ARM: davinci: mityomapl138: don't read the MAC address from machine code
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: netdev, linux-omap, linux-kernel, linux-arm-kernel,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
This is now done by the emac driver using a registered nvmem cell.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-mityomapl138.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
index b5be51c0451e..5c0a0557a361 100644
--- a/arch/arm/mach-davinci/board-mityomapl138.c
+++ b/arch/arm/mach-davinci/board-mityomapl138.c
@@ -120,7 +120,6 @@ static void read_factory_config(struct nvmem_device *nvmem, void *context)
{
int ret;
const char *partnum = NULL;
- struct davinci_soc_info *soc_info = &davinci_soc_info;
if (!IS_BUILTIN(CONFIG_NVMEM)) {
pr_warn("Factory Config not available without CONFIG_NVMEM\n");
@@ -146,13 +145,6 @@ static void read_factory_config(struct nvmem_device *nvmem, void *context)
goto bad_config;
}
- pr_info("Found MAC = %pM\n", factory_config.mac);
- if (is_valid_ether_addr(factory_config.mac))
- memcpy(soc_info->emac_pdata->mac_addr,
- factory_config.mac, ETH_ALEN);
- else
- pr_warn("Invalid MAC found in factory config block\n");
-
partnum = factory_config.partnum;
pr_info("Part Number = %s\n", partnum);
--
2.17.1
^ permalink raw reply related
* [PATCH v3 12/18] ARM: davinci: dm365-evm: use device properties for at24 eeprom
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-dm365-evm.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm365-evm.c b/arch/arm/mach-davinci/board-dm365-evm.c
index cb0ac92a278e..b360d26e6caa 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -18,7 +18,7 @@
#include <linux/i2c.h>
#include <linux/io.h>
#include <linux/clk.h>
-#include <linux/platform_data/at24.h>
+#include <linux/property.h>
#include <linux/leds.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
@@ -179,18 +179,15 @@ static struct nvmem_cell_lookup dm365evm_mac_address_cell = {
.nvmem_name = "1-00500",
};
-static struct at24_platform_data eeprom_info = {
- .byte_len = (256*1024) / 8,
- .page_size = 64,
- .flags = AT24_FLAG_ADDR16,
- .setup = davinci_get_mac_addr,
- .context = (void *)0x7f00,
+static const struct property_entry eeprom_properties[] = {
+ PROPERTY_ENTRY_U32("pagesize", 64),
+ { }
};
static struct i2c_board_info i2c_info[] = {
{
I2C_BOARD_INFO("24c256", 0x50),
- .platform_data = &eeprom_info,
+ .properties = eeprom_properties,
},
{
I2C_BOARD_INFO("tlv320aic3x", 0x18),
--
2.17.1
^ permalink raw reply related
* [PATCH v3 13/18] ARM: davinci: da830-evm: use device properties for at24 eeprom
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-da830-evm.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 4a2fe8142a2f..08a23e777eca 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -18,7 +18,7 @@
#include <linux/platform_device.h>
#include <linux/i2c.h>
#include <linux/platform_data/pcf857x.h>
-#include <linux/platform_data/at24.h>
+#include <linux/property.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
#include <linux/spi/spi.h>
@@ -419,12 +419,9 @@ static struct nvmem_cell_lookup da830_evm_mac_address_cell = {
.nvmem_name = "1-00500",
};
-static struct at24_platform_data da830_evm_i2c_eeprom_info = {
- .byte_len = SZ_256K / 8,
- .page_size = 64,
- .flags = AT24_FLAG_ADDR16,
- .setup = davinci_get_mac_addr,
- .context = (void *)0x7f00,
+static const struct property_entry da830_evm_i2c_eeprom_properties[] = {
+ PROPERTY_ENTRY_U32("pagesize", 64),
+ { }
};
static int __init da830_evm_ui_expander_setup(struct i2c_client *client,
@@ -458,7 +455,7 @@ static struct pcf857x_platform_data __initdata da830_evm_ui_expander_info = {
static struct i2c_board_info __initdata da830_evm_i2c_devices[] = {
{
I2C_BOARD_INFO("24c256", 0x50),
- .platform_data = &da830_evm_i2c_eeprom_info,
+ .properties = da830_evm_i2c_eeprom_properties,
},
{
I2C_BOARD_INFO("tlv320aic3x", 0x18),
--
2.17.1
^ permalink raw reply related
* [PATCH v3 14/18] ARM: davinci: dm644x-evm: use device properties for at24 eeprom
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-dm644x-evm.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c b/arch/arm/mach-davinci/board-dm644x-evm.c
index 6d35c6e1b0bd..abfcf42da6fb 100644
--- a/arch/arm/mach-davinci/board-dm644x-evm.c
+++ b/arch/arm/mach-davinci/board-dm644x-evm.c
@@ -16,8 +16,8 @@
#include <linux/gpio/machine.h>
#include <linux/i2c.h>
#include <linux/platform_data/pcf857x.h>
-#include <linux/platform_data/at24.h>
#include <linux/platform_data/gpio-davinci.h>
+#include <linux/property.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/rawnand.h>
#include <linux/mtd/partitions.h>
@@ -486,12 +486,8 @@ static struct nvmem_cell_lookup dm6446evm_mac_address_cell = {
.nvmem_name = "1-00500",
};
-static struct at24_platform_data eeprom_info = {
- .byte_len = (256*1024) / 8,
- .page_size = 64,
- .flags = AT24_FLAG_ADDR16,
- .setup = davinci_get_mac_addr,
- .context = (void *)0x7f00,
+static const struct property_entry eeprom_properties[] = {
+ PROPERTY_ENTRY_U32("pagesize", 64),
};
/*
@@ -601,7 +597,7 @@ static struct i2c_board_info __initdata i2c_info[] = {
},
{
I2C_BOARD_INFO("24c256", 0x50),
- .platform_data = &eeprom_info,
+ .properties = eeprom_properties,
},
{
I2C_BOARD_INFO("tlv320aic33", 0x1b),
--
2.17.1
^ permalink raw reply related
* [PATCH v3 15/18] ARM: davinci: dm646x-evm: use device properties for at24 eeprom
From: Bartosz Golaszewski @ 2018-06-28 14:32 UTC (permalink / raw)
To: Sekhar Nori, Kevin Hilman, Russell King, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Dan Carpenter, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180628143244.4561-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/board-dm646x-evm.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c
index 5a9de47bc8a2..5049f0c6cd1a 100644
--- a/arch/arm/mach-davinci/board-dm646x-evm.c
+++ b/arch/arm/mach-davinci/board-dm646x-evm.c
@@ -22,7 +22,7 @@
#include <linux/gpio.h>
#include <linux/platform_device.h>
#include <linux/i2c.h>
-#include <linux/platform_data/at24.h>
+#include <linux/property.h>
#include <linux/platform_data/pcf857x.h>
#include <media/i2c/tvp514x.h>
@@ -320,12 +320,9 @@ static struct nvmem_cell_lookup dm646x_evm_mac_address_cell = {
.nvmem_name = "1-00500",
};
-static struct at24_platform_data eeprom_info = {
- .byte_len = (256*1024) / 8,
- .page_size = 64,
- .flags = AT24_FLAG_ADDR16,
- .setup = davinci_get_mac_addr,
- .context = (void *)0x7f00,
+static const struct property_entry eeprom_properties[] = {
+ PROPERTY_ENTRY_U32("pagesize", 64),
+ { }
};
#endif
@@ -396,7 +393,7 @@ static void evm_init_cpld(void)
static struct i2c_board_info __initdata i2c_info[] = {
{
I2C_BOARD_INFO("24c256", 0x50),
- .platform_data = &eeprom_info,
+ .properties = eeprom_properties,
},
{
I2C_BOARD_INFO("pcf8574a", 0x38),
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox