* [PATCH] mtd: orion_nand: Improve error handling in orion_nand_probe
From: Emil Goode @ 2013-06-09 9:08 UTC (permalink / raw)
To: dwmw2, artem.bityutskiy, andrew, wfp5p, jg1.han
Cc: kernel-janitors, Emil Goode, linux-mtd, linux-kernel
This patch fixes some issues in the error handling and simplifies
the code by converting to devm* functions.
If the kzalloc call fails it is unnecessary to use the label no_res
and pass a NULL pointer to kfree. If the devm_kzalloc call fails on
line 110 we forgett to call iounmap for the previous ioremap call.
The following changes are introduced:
- Convert to devm_kzalloc and remove calls to kfree.
- Convert to devm_ioremap_resource that adds a missing call to
*request_mem_region and remove calls to iounmap.
- The devm_ioremap_resource function checks the passed resource so
we can remove the NULL check after the platform_get_resource call.
Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
The patch is only build tested
drivers/mtd/nand/orion_nand.c | 39 +++++++++++----------------------------
1 file changed, 11 insertions(+), 28 deletions(-)
diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
index 8fbd002..cc22f2f 100644
--- a/drivers/mtd/nand/orion_nand.c
+++ b/drivers/mtd/nand/orion_nand.c
@@ -85,34 +85,25 @@ static int __init orion_nand_probe(struct platform_device *pdev)
int ret = 0;
u32 val = 0;
- nc = kzalloc(sizeof(struct nand_chip) + sizeof(struct mtd_info), GFP_KERNEL);
+ nc = devm_kzalloc(&pdev->dev, sizeof(struct nand_chip) +
+ sizeof(struct mtd_info), GFP_KERNEL);
if (!nc) {
printk(KERN_ERR "orion_nand: failed to allocate device structure.\n");
- ret = -ENOMEM;
- goto no_res;
+ return -ENOMEM;
}
mtd = (struct mtd_info *)(nc + 1);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- ret = -ENODEV;
- goto no_res;
- }
-
- io_base = ioremap(res->start, resource_size(res));
- if (!io_base) {
- printk(KERN_ERR "orion_nand: ioremap failed\n");
- ret = -EIO;
- goto no_res;
- }
+ io_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(io_base))
+ return PTR_ERR(io_base);
if (pdev->dev.of_node) {
board = devm_kzalloc(&pdev->dev, sizeof(struct orion_nand_data),
- GFP_KERNEL);
+ GFP_KERNEL);
if (!board) {
printk(KERN_ERR "orion_nand: failed to allocate board structure.\n");
- ret = -ENOMEM;
- goto no_res;
+ return -ENOMEM;
}
if (!of_property_read_u32(pdev->dev.of_node, "cle", &val))
board->cle = (u8)val;
@@ -167,7 +158,7 @@ static int __init orion_nand_probe(struct platform_device *pdev)
if (nand_scan(mtd, 1)) {
ret = -ENXIO;
- goto no_dev;
+ goto disable_clk;
}
mtd->name = "orion_nand";
@@ -176,20 +167,17 @@ static int __init orion_nand_probe(struct platform_device *pdev)
board->parts, board->nr_parts);
if (ret) {
nand_release(mtd);
- goto no_dev;
+ goto disable_clk;
}
return 0;
-no_dev:
+disable_clk:
if (!IS_ERR(clk)) {
clk_disable_unprepare(clk);
clk_put(clk);
}
platform_set_drvdata(pdev, NULL);
- iounmap(io_base);
-no_res:
- kfree(nc);
return ret;
}
@@ -197,15 +185,10 @@ no_res:
static int orion_nand_remove(struct platform_device *pdev)
{
struct mtd_info *mtd = platform_get_drvdata(pdev);
- struct nand_chip *nc = mtd->priv;
struct clk *clk;
nand_release(mtd);
- iounmap(nc->IO_ADDR_W);
-
- kfree(nc);
-
clk = clk_get(&pdev->dev, NULL);
if (!IS_ERR(clk)) {
clk_disable_unprepare(clk);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] xattr: Constify ->name member of "struct xattr".
From: Joel Becker @ 2013-06-09 9:07 UTC (permalink / raw)
To: Tetsuo Handa
Cc: ocfs2-devel, reiserfs-devel, eparis, casey, zohar, serge.hallyn,
linux-fsdevel, linux-security-module
In-Reply-To: <201306082154.BDJ95357.MFQFVOJLSOtHFO@I-love.SAKURA.ne.jp>
I can't really comment on the concept, but if security folks agree, the
ocfs2 part looks fine.
Reviewed-by: Joel Becker <jlbec@evilplan.org>
On Sat, Jun 08, 2013 at 09:54:56PM +0900, Tetsuo Handa wrote:
> >From 96df8b4be7f8702913a0245b2312958e2eea6caf Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Sat, 8 Jun 2013 21:46:58 +0900
> Subject: [PATCH] xattr: Constify ->name member of "struct xattr".
>
> Since everybody sets kstrdup()ed constant string to "struct xattr"->name but
> nobody modifies "struct xattr"->name , we can omit kstrdup() and its failure
> checking by constifying ->name member of "struct xattr".
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> fs/ocfs2/xattr.h | 2 +-
> include/linux/security.h | 8 ++++----
> include/linux/xattr.h | 2 +-
> include/uapi/linux/reiserfs_xattr.h | 2 +-
> security/capability.c | 2 +-
> security/integrity/evm/evm_main.c | 2 +-
> security/security.c | 8 +++-----
> security/selinux/hooks.c | 17 ++++++-----------
> security/smack/smack_lsm.c | 9 +++------
> 9 files changed, 21 insertions(+), 31 deletions(-)
>
> diff --git a/fs/ocfs2/xattr.h b/fs/ocfs2/xattr.h
> index e5c7f15..19f134e 100644
> --- a/fs/ocfs2/xattr.h
> +++ b/fs/ocfs2/xattr.h
> @@ -32,7 +32,7 @@ enum ocfs2_xattr_type {
>
> struct ocfs2_security_xattr_info {
> int enable;
> - char *name;
> + const char *name;
> void *value;
> size_t value_len;
> };
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 40560f4..2e64d73 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1466,7 +1466,7 @@ struct security_operations {
> int (*inode_alloc_security) (struct inode *inode);
> void (*inode_free_security) (struct inode *inode);
> int (*inode_init_security) (struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len);
> int (*inode_create) (struct inode *dir,
> struct dentry *dentry, umode_t mode);
> @@ -1737,7 +1737,7 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> const struct qstr *qstr,
> initxattrs initxattrs, void *fs_data);
> int security_old_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len);
> int security_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode);
> int security_inode_link(struct dentry *old_dentry, struct inode *dir,
> @@ -2048,8 +2048,8 @@ static inline int security_inode_init_security(struct inode *inode,
> static inline int security_old_inode_init_security(struct inode *inode,
> struct inode *dir,
> const struct qstr *qstr,
> - char **name, void **value,
> - size_t *len)
> + const char **name,
> + void **value, size_t *len)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index fdbafc6..91b0a68 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -31,7 +31,7 @@ struct xattr_handler {
> };
>
> struct xattr {
> - char *name;
> + const char *name;
> void *value;
> size_t value_len;
> };
> diff --git a/include/uapi/linux/reiserfs_xattr.h b/include/uapi/linux/reiserfs_xattr.h
> index d8ce17c..38fdd64 100644
> --- a/include/uapi/linux/reiserfs_xattr.h
> +++ b/include/uapi/linux/reiserfs_xattr.h
> @@ -16,7 +16,7 @@ struct reiserfs_xattr_header {
> };
>
> struct reiserfs_security_handle {
> - char *name;
> + const char *name;
> void *value;
> size_t length;
> };
> diff --git a/security/capability.c b/security/capability.c
> index 1728d4e..b311ff0 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -119,7 +119,7 @@ static void cap_inode_free_security(struct inode *inode)
> }
>
> static int cap_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len)
> {
> return -EOPNOTSUPP;
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index cdbde17..2787080 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -405,7 +405,7 @@ int evm_inode_init_security(struct inode *inode,
>
> evm_xattr->value = xattr_data;
> evm_xattr->value_len = sizeof(*xattr_data);
> - evm_xattr->name = kstrdup(XATTR_EVM_SUFFIX, GFP_NOFS);
> + evm_xattr->name = XATTR_EVM_SUFFIX;
> return 0;
> out:
> kfree(xattr_data);
> diff --git a/security/security.c b/security/security.c
> index a3dce87..8849691 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -335,10 +335,10 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> if (unlikely(IS_PRIVATE(inode)))
> return 0;
>
> - memset(new_xattrs, 0, sizeof new_xattrs);
> if (!initxattrs)
> return security_ops->inode_init_security(inode, dir, qstr,
> NULL, NULL, NULL);
> + memset(new_xattrs, 0, sizeof(new_xattrs));
> lsm_xattr = new_xattrs;
> ret = security_ops->inode_init_security(inode, dir, qstr,
> &lsm_xattr->name,
> @@ -353,16 +353,14 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> goto out;
> ret = initxattrs(inode, new_xattrs, fs_data);
> out:
> - for (xattr = new_xattrs; xattr->name != NULL; xattr++) {
> - kfree(xattr->name);
> + for (xattr = new_xattrs; xattr->value != NULL; xattr++)
> kfree(xattr->value);
> - }
> return (ret == -EOPNOTSUPP) ? 0 : ret;
> }
> EXPORT_SYMBOL(security_inode_init_security);
>
> int security_old_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len)
> {
> if (unlikely(IS_PRIVATE(inode)))
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 5c6f2cd..16b8e51 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2516,7 +2516,8 @@ static void selinux_inode_free_security(struct inode *inode)
> }
>
> static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr,
> + const char **name,
> void **value, size_t *len)
> {
> const struct task_security_struct *tsec = current_security();
> @@ -2524,7 +2525,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> struct superblock_security_struct *sbsec;
> u32 sid, newsid, clen;
> int rc;
> - char *namep = NULL, *context;
> + char *context;
>
> dsec = dir->i_security;
> sbsec = dir->i_sb->s_security;
> @@ -2560,19 +2561,13 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> if (!ss_initialized || !(sbsec->flags & SE_SBLABELSUPP))
> return -EOPNOTSUPP;
>
> - if (name) {
> - namep = kstrdup(XATTR_SELINUX_SUFFIX, GFP_NOFS);
> - if (!namep)
> - return -ENOMEM;
> - *name = namep;
> - }
> + if (name)
> + *name = XATTR_SELINUX_SUFFIX;
>
> if (value && len) {
> rc = security_sid_to_context_force(newsid, &context, &clen);
> - if (rc) {
> - kfree(namep);
> + if (rc)
> return rc;
> - }
> *value = context;
> *len = clen;
> }
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index d52c780..46e5b47 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -554,7 +554,7 @@ static void smack_inode_free_security(struct inode *inode)
> * Returns 0 if it all works out, -ENOMEM if there's no memory
> */
> static int smack_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len)
> {
> struct smack_known *skp;
> @@ -564,11 +564,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
> char *dsp = smk_of_inode(dir);
> int may;
>
> - if (name) {
> - *name = kstrdup(XATTR_SMACK_SUFFIX, GFP_NOFS);
> - if (*name == NULL)
> - return -ENOMEM;
> - }
> + if (name)
> + *name = XATTR_SMACK_SUFFIX;
>
> if (value) {
> skp = smk_find_entry(csp);
> --
> 1.7.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
"Sometimes I think the surest sign intelligent
life exists elsewhere in the universe is that
none of it has tried to contact us."
-Calvin & Hobbes
http://www.jlbec.org/
jlbec@evilplan.org
^ permalink raw reply
* [Ocfs2-devel] [PATCH] xattr: Constify ->name member of "struct xattr".
From: Joel Becker @ 2013-06-09 9:07 UTC (permalink / raw)
To: Tetsuo Handa
Cc: ocfs2-devel, reiserfs-devel, eparis, casey, zohar, serge.hallyn,
linux-fsdevel, linux-security-module
In-Reply-To: <201306082154.BDJ95357.MFQFVOJLSOtHFO@I-love.SAKURA.ne.jp>
I can't really comment on the concept, but if security folks agree, the
ocfs2 part looks fine.
Reviewed-by: Joel Becker <jlbec@evilplan.org>
On Sat, Jun 08, 2013 at 09:54:56PM +0900, Tetsuo Handa wrote:
> >From 96df8b4be7f8702913a0245b2312958e2eea6caf Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Sat, 8 Jun 2013 21:46:58 +0900
> Subject: [PATCH] xattr: Constify ->name member of "struct xattr".
>
> Since everybody sets kstrdup()ed constant string to "struct xattr"->name but
> nobody modifies "struct xattr"->name , we can omit kstrdup() and its failure
> checking by constifying ->name member of "struct xattr".
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> fs/ocfs2/xattr.h | 2 +-
> include/linux/security.h | 8 ++++----
> include/linux/xattr.h | 2 +-
> include/uapi/linux/reiserfs_xattr.h | 2 +-
> security/capability.c | 2 +-
> security/integrity/evm/evm_main.c | 2 +-
> security/security.c | 8 +++-----
> security/selinux/hooks.c | 17 ++++++-----------
> security/smack/smack_lsm.c | 9 +++------
> 9 files changed, 21 insertions(+), 31 deletions(-)
>
> diff --git a/fs/ocfs2/xattr.h b/fs/ocfs2/xattr.h
> index e5c7f15..19f134e 100644
> --- a/fs/ocfs2/xattr.h
> +++ b/fs/ocfs2/xattr.h
> @@ -32,7 +32,7 @@ enum ocfs2_xattr_type {
>
> struct ocfs2_security_xattr_info {
> int enable;
> - char *name;
> + const char *name;
> void *value;
> size_t value_len;
> };
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 40560f4..2e64d73 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1466,7 +1466,7 @@ struct security_operations {
> int (*inode_alloc_security) (struct inode *inode);
> void (*inode_free_security) (struct inode *inode);
> int (*inode_init_security) (struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len);
> int (*inode_create) (struct inode *dir,
> struct dentry *dentry, umode_t mode);
> @@ -1737,7 +1737,7 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> const struct qstr *qstr,
> initxattrs initxattrs, void *fs_data);
> int security_old_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len);
> int security_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode);
> int security_inode_link(struct dentry *old_dentry, struct inode *dir,
> @@ -2048,8 +2048,8 @@ static inline int security_inode_init_security(struct inode *inode,
> static inline int security_old_inode_init_security(struct inode *inode,
> struct inode *dir,
> const struct qstr *qstr,
> - char **name, void **value,
> - size_t *len)
> + const char **name,
> + void **value, size_t *len)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index fdbafc6..91b0a68 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -31,7 +31,7 @@ struct xattr_handler {
> };
>
> struct xattr {
> - char *name;
> + const char *name;
> void *value;
> size_t value_len;
> };
> diff --git a/include/uapi/linux/reiserfs_xattr.h b/include/uapi/linux/reiserfs_xattr.h
> index d8ce17c..38fdd64 100644
> --- a/include/uapi/linux/reiserfs_xattr.h
> +++ b/include/uapi/linux/reiserfs_xattr.h
> @@ -16,7 +16,7 @@ struct reiserfs_xattr_header {
> };
>
> struct reiserfs_security_handle {
> - char *name;
> + const char *name;
> void *value;
> size_t length;
> };
> diff --git a/security/capability.c b/security/capability.c
> index 1728d4e..b311ff0 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -119,7 +119,7 @@ static void cap_inode_free_security(struct inode *inode)
> }
>
> static int cap_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len)
> {
> return -EOPNOTSUPP;
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index cdbde17..2787080 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -405,7 +405,7 @@ int evm_inode_init_security(struct inode *inode,
>
> evm_xattr->value = xattr_data;
> evm_xattr->value_len = sizeof(*xattr_data);
> - evm_xattr->name = kstrdup(XATTR_EVM_SUFFIX, GFP_NOFS);
> + evm_xattr->name = XATTR_EVM_SUFFIX;
> return 0;
> out:
> kfree(xattr_data);
> diff --git a/security/security.c b/security/security.c
> index a3dce87..8849691 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -335,10 +335,10 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> if (unlikely(IS_PRIVATE(inode)))
> return 0;
>
> - memset(new_xattrs, 0, sizeof new_xattrs);
> if (!initxattrs)
> return security_ops->inode_init_security(inode, dir, qstr,
> NULL, NULL, NULL);
> + memset(new_xattrs, 0, sizeof(new_xattrs));
> lsm_xattr = new_xattrs;
> ret = security_ops->inode_init_security(inode, dir, qstr,
> &lsm_xattr->name,
> @@ -353,16 +353,14 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> goto out;
> ret = initxattrs(inode, new_xattrs, fs_data);
> out:
> - for (xattr = new_xattrs; xattr->name != NULL; xattr++) {
> - kfree(xattr->name);
> + for (xattr = new_xattrs; xattr->value != NULL; xattr++)
> kfree(xattr->value);
> - }
> return (ret == -EOPNOTSUPP) ? 0 : ret;
> }
> EXPORT_SYMBOL(security_inode_init_security);
>
> int security_old_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len)
> {
> if (unlikely(IS_PRIVATE(inode)))
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 5c6f2cd..16b8e51 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2516,7 +2516,8 @@ static void selinux_inode_free_security(struct inode *inode)
> }
>
> static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr,
> + const char **name,
> void **value, size_t *len)
> {
> const struct task_security_struct *tsec = current_security();
> @@ -2524,7 +2525,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> struct superblock_security_struct *sbsec;
> u32 sid, newsid, clen;
> int rc;
> - char *namep = NULL, *context;
> + char *context;
>
> dsec = dir->i_security;
> sbsec = dir->i_sb->s_security;
> @@ -2560,19 +2561,13 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> if (!ss_initialized || !(sbsec->flags & SE_SBLABELSUPP))
> return -EOPNOTSUPP;
>
> - if (name) {
> - namep = kstrdup(XATTR_SELINUX_SUFFIX, GFP_NOFS);
> - if (!namep)
> - return -ENOMEM;
> - *name = namep;
> - }
> + if (name)
> + *name = XATTR_SELINUX_SUFFIX;
>
> if (value && len) {
> rc = security_sid_to_context_force(newsid, &context, &clen);
> - if (rc) {
> - kfree(namep);
> + if (rc)
> return rc;
> - }
> *value = context;
> *len = clen;
> }
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index d52c780..46e5b47 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -554,7 +554,7 @@ static void smack_inode_free_security(struct inode *inode)
> * Returns 0 if it all works out, -ENOMEM if there's no memory
> */
> static int smack_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len)
> {
> struct smack_known *skp;
> @@ -564,11 +564,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
> char *dsp = smk_of_inode(dir);
> int may;
>
> - if (name) {
> - *name = kstrdup(XATTR_SMACK_SUFFIX, GFP_NOFS);
> - if (*name == NULL)
> - return -ENOMEM;
> - }
> + if (name)
> + *name = XATTR_SMACK_SUFFIX;
>
> if (value) {
> skp = smk_find_entry(csp);
> --
> 1.7.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
"Sometimes I think the surest sign intelligent
life exists elsewhere in the universe is that
none of it has tried to contact us."
-Calvin & Hobbes
http://www.jlbec.org/
jlbec at evilplan.org
^ permalink raw reply
* Re: [PATCH v8 00/11] KVM: MMU: fast zap all shadow pages
From: Xiao Guangrong @ 2013-06-09 9:06 UTC (permalink / raw)
To: Gleb Natapov; +Cc: avi.kivity, mtosatti, pbonzini, linux-kernel, kvm
In-Reply-To: <20130609085342.GI4725@redhat.com>
On 06/09/2013 04:53 PM, Gleb Natapov wrote:
> On Fri, May 31, 2013 at 08:36:19AM +0800, Xiao Guangrong wrote:
>> Hi Gleb, Paolo, Marcelo,
>>
>> I have putted the potential controversial patches to the latter that are
>> patch 8 ~ 10, patch 11 depends on patch 9. Other patches are fully reviewed,
>> I think its are ready for being merged. If not luck enough, further discussion
>> is needed, could you please apply that patches first? :)
>>
>> Thank you in advance!
>>
>> Some points are raised during discussion but missed in this version:
>> 1) Gleb's idea that skip obsolete pages in the hast list walker
>>
>> Unfortunately, it is not safe. There has a window between updating
>> valid-gen and reloading mmu, in that window, the obsolete page can
>> be used by vcpu, but the guest page table fail to be write-protected
>> (since the obsolete page is skipped in mmu_need_write_protect()).
>>
> Can you elaborate on how this can happen. valid_gen is updated under
> mmu_lock and reloading of mmus happens under the same lock, so for all
> other vcpus this should look like atomic thing.
You're right.
Actually, i made another optimization patch in this version that moves
kvm_reload_remote_mmus() out of mmu-lock, but did not attach it into this
series. It seems my brain is not parallel-able enough. :(
^ permalink raw reply
* Re: [PATCH 2/2] i2c: designware: Add i2c-designware-hs
From: zhangfei gao @ 2013-06-09 8:59 UTC (permalink / raw)
To: Baruch Siach
Cc: Dirk Brandewie, Zhangfei Gao, device-tree, linux-arm-kernel,
Wolfram Sang
In-Reply-To: <20130609032300.GB4312@tarshish>
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt
>> @@ -0,0 +1,30 @@
>> +* Hisilicon I2C Controller
>> +
>> +Required properties :
>> +
>> + - compatible : should be "hisilicon,designware-i2c"
>> + - reg : Offset and length of the register set for the device
>> + - interrupts : <IRQ> where IRQ is the interrupt number.
>> +
>> +Example :
>> +
>> + i2c0: i2c@fcb08000 {
>> + compatible = "hs,designware-i2c";
>
> A few comments on this one:
>
> 1. You should Cc devicetree-discuss@lists.ozlabs.org on patches touching ftd
> bindings (added to Cc)
>
> 2. The convention is to use the IC block designer in the "compatible" property
> prefix, in this case Symopsys (snps)
>
> 3. This does not match the compatible property in hs_dw_i2c_of_match[] below
> where you use "hisilicon,designware-i2c"
>
> 4. Please update Documentation/devicetree/bindings/vendor-prefixes.txt when
> adding new vendor prefixes
Thanks Baruch for the kind education, really useful.
How about using .compatible = "snps,hisilicon-i2c"
>> + Client in i2c0 bus with add 0x58 could be added as example
>> + i2c0: i2c@fcb08000 {
>> + status = "ok";
>
> The convention is to use "okay".
got it.
>
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>;
>> + i2c_client1: i2c_client@58 {
>> + compatible = "hisilicon,i2c_client_tpa2028";
>> + reg = <0x58>;
>> + };
>> + };
>
> [...]
>
> The code below looks like a direct copy of i2c-designware-platdrv.c. Is there
> any reason you can't use that code instead?
Not understood i2c-designware-platdrv.c can be directly touched.
Usually, there is register function, or external function call.
It would be great if we could directly add hisilicon support in
i2c-designware-platdrv.c.
How about adding these code to distinguish.
The concern is will platdrv.c become bigger and bigger?
What in case private register have to be accessed?
struct dw_i2c_data {
u32 accessor_flags;
unsigned int tx_fifo_depth;
unsigned int rx_fifo_depth;
};
static struct dw_i2c_data hisilicon_data = {
.accessor_flags = ACCESS_32BIT,
.tx_fifo_depth = 16,
.rx_fifo_depth = 16,
};
{ .compatible = "snps,hisilicon-i2c", .data = &hisilicon_data},
^ permalink raw reply
* [PATCH 2/2] i2c: designware: Add i2c-designware-hs
From: zhangfei gao @ 2013-06-09 8:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130609032300.GB4312@tarshish>
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-designware-hs.txt
>> @@ -0,0 +1,30 @@
>> +* Hisilicon I2C Controller
>> +
>> +Required properties :
>> +
>> + - compatible : should be "hisilicon,designware-i2c"
>> + - reg : Offset and length of the register set for the device
>> + - interrupts : <IRQ> where IRQ is the interrupt number.
>> +
>> +Example :
>> +
>> + i2c0: i2c at fcb08000 {
>> + compatible = "hs,designware-i2c";
>
> A few comments on this one:
>
> 1. You should Cc devicetree-discuss at lists.ozlabs.org on patches touching ftd
> bindings (added to Cc)
>
> 2. The convention is to use the IC block designer in the "compatible" property
> prefix, in this case Symopsys (snps)
>
> 3. This does not match the compatible property in hs_dw_i2c_of_match[] below
> where you use "hisilicon,designware-i2c"
>
> 4. Please update Documentation/devicetree/bindings/vendor-prefixes.txt when
> adding new vendor prefixes
Thanks Baruch for the kind education, really useful.
How about using .compatible = "snps,hisilicon-i2c"
>> + Client in i2c0 bus with add 0x58 could be added as example
>> + i2c0: i2c at fcb08000 {
>> + status = "ok";
>
> The convention is to use "okay".
got it.
>
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>;
>> + i2c_client1: i2c_client at 58 {
>> + compatible = "hisilicon,i2c_client_tpa2028";
>> + reg = <0x58>;
>> + };
>> + };
>
> [...]
>
> The code below looks like a direct copy of i2c-designware-platdrv.c. Is there
> any reason you can't use that code instead?
Not understood i2c-designware-platdrv.c can be directly touched.
Usually, there is register function, or external function call.
It would be great if we could directly add hisilicon support in
i2c-designware-platdrv.c.
How about adding these code to distinguish.
The concern is will platdrv.c become bigger and bigger?
What in case private register have to be accessed?
struct dw_i2c_data {
u32 accessor_flags;
unsigned int tx_fifo_depth;
unsigned int rx_fifo_depth;
};
static struct dw_i2c_data hisilicon_data = {
.accessor_flags = ACCESS_32BIT,
.tx_fifo_depth = 16,
.rx_fifo_depth = 16,
};
{ .compatible = "snps,hisilicon-i2c", .data = &hisilicon_data},
^ permalink raw reply
* Re: [PATCH] KVM: x86: fix missed memory synchronization when patch hypercall
From: Gleb Natapov @ 2013-06-09 8:59 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Paolo Bonzini, Marcelo Tosatti, LKML, KVM
In-Reply-To: <51B4434A.2010402@gmail.com>
On Sun, Jun 09, 2013 at 04:56:42PM +0800, Xiao Guangrong wrote:
> On 06/09/2013 04:45 PM, Gleb Natapov wrote:
> > On Sat, Jun 08, 2013 at 11:15:37AM +0800, Xiao Guangrong wrote:
> >> From: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> >>
> >> Currently, memory synchronization is missed in emulator_fix_hypercall,
> >> please see the commit 758ccc89b83
> >> (KVM: x86: drop calling kvm_mmu_zap_all in emulator_fix_hypercall)
> >>
> >> This patch fixes it by introducing kvm_vcpus_hang_on_page_start() and
> >> kvm_vcpus_hang_on_page_end which unmap the patched page from guest
> >> and use kvm_flush_remote_tlbs() as the serializing instruction to
> >> ensure the memory coherence
> >> [ The SDM said that INVEPT, INVVPID and MOV (to control register, with
> >> the exception of MOV CR8) are the serializing instructions. ]
> >>
> >> The mmu-lock is held during host patches the page so that it stops vcpus
> >> to fix its further page fault
> >>
> > I have a patch to implement is much simple and in generic way, not
> > relying on MMU internals.
>
> I have considered this way but it seems not simple - it needs a new type of
> request and it forces all vcpus to hang when host is patching the page.
>
> My approach is just reusing the mmu code and requires vcpus to hang only when
> the patched page is bing accessed.
That's very rare, no point to optimize this code path.
--
Gleb.
^ permalink raw reply
* [PATCH] sparc: kernel: using strlcpy() instead of strcpy()
From: Zhao Hongjiang @ 2013-06-09 8:57 UTC (permalink / raw)
To: sparclinux
'boot_command_line' and 'full_boot_str' has a fix length, 'cmdline_p' and
'boot_command' maybe larger than them. So use strlcpy() instead of strcpy()
to avoid memory overflow.
Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
---
arch/sparc/kernel/ds.c | 3 ++-
arch/sparc/kernel/setup_32.c | 2 +-
arch/sparc/kernel/setup_64.c | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/sparc/kernel/ds.c b/arch/sparc/kernel/ds.c
index 75bb608..5ef48da 100644
--- a/arch/sparc/kernel/ds.c
+++ b/arch/sparc/kernel/ds.c
@@ -843,7 +843,8 @@ void ldom_reboot(const char *boot_command)
unsigned long len;
strcpy(full_boot_str, "boot ");
- strcpy(full_boot_str + strlen("boot "), boot_command);
+ strlcpy(full_boot_str + strlen("boot "), boot_command,
+ sizeof(full_boot_str + strlen("boot ")));
len = strlen(full_boot_str);
if (reboot_data_supported) {
diff --git a/arch/sparc/kernel/setup_32.c b/arch/sparc/kernel/setup_32.c
index 38bf80a..1434526 100644
--- a/arch/sparc/kernel/setup_32.c
+++ b/arch/sparc/kernel/setup_32.c
@@ -304,7 +304,7 @@ void __init setup_arch(char **cmdline_p)
/* Initialize PROM console and command line. */
*cmdline_p = prom_getbootargs();
- strcpy(boot_command_line, *cmdline_p);
+ strlcpy(boot_command_line, *cmdline_p, COMMAND_LINE_SIZE);
parse_early_param();
boot_flags_init(*cmdline_p);
diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c
index 88a127b..1378554 100644
--- a/arch/sparc/kernel/setup_64.c
+++ b/arch/sparc/kernel/setup_64.c
@@ -555,7 +555,7 @@ void __init setup_arch(char **cmdline_p)
{
/* Initialize PROM console and command line. */
*cmdline_p = prom_getbootargs();
- strcpy(boot_command_line, *cmdline_p);
+ strlcpy(boot_command_line, *cmdline_p, COMMAND_LINE_SIZE);
parse_early_param();
boot_flags_init(*cmdline_p);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] KVM: x86: fix missed memory synchronization when patch hypercall
From: Xiao Guangrong @ 2013-06-09 8:56 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Paolo Bonzini, Marcelo Tosatti, LKML, KVM
In-Reply-To: <20130609084526.GH4725@redhat.com>
On 06/09/2013 04:45 PM, Gleb Natapov wrote:
> On Sat, Jun 08, 2013 at 11:15:37AM +0800, Xiao Guangrong wrote:
>> From: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
>>
>> Currently, memory synchronization is missed in emulator_fix_hypercall,
>> please see the commit 758ccc89b83
>> (KVM: x86: drop calling kvm_mmu_zap_all in emulator_fix_hypercall)
>>
>> This patch fixes it by introducing kvm_vcpus_hang_on_page_start() and
>> kvm_vcpus_hang_on_page_end which unmap the patched page from guest
>> and use kvm_flush_remote_tlbs() as the serializing instruction to
>> ensure the memory coherence
>> [ The SDM said that INVEPT, INVVPID and MOV (to control register, with
>> the exception of MOV CR8) are the serializing instructions. ]
>>
>> The mmu-lock is held during host patches the page so that it stops vcpus
>> to fix its further page fault
>>
> I have a patch to implement is much simple and in generic way, not
> relying on MMU internals.
I have considered this way but it seems not simple - it needs a new type of
request and it forces all vcpus to hang when host is patching the page.
My approach is just reusing the mmu code and requires vcpus to hang only when
the patched page is bing accessed.
^ permalink raw reply
* Re: Immutable biovecs, dio rewrite
From: Kent Overstreet @ 2013-06-09 8:55 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jens Axboe, Theodore Ts'o, linux-kernel@vger.kernel.org,
Linux FS Devel
In-Reply-To: <CAMuHMdXsnNMGUNKXaoUYCNnrCi2jv-acktzF_+g0yfEm7Lo-Og@mail.gmail.com>
On Sun, Jun 09, 2013 at 10:34:08AM +0200, Geert Uytterhoeven wrote:
> On Sun, Jun 9, 2013 at 4:18 AM, Kent Overstreet <koverstreet@google.com> wrote:
> > * Changing all the drivers to go through the iterator means that we can
> > submit a partially completed bio to generic_make_request() - this
> > previously worked on some drivers, but worked on others.
>
> Some negation is missing in this sentence?
Heh, whoops.
Yeah, it seemed to work fine on normal request queue based drivers - and
expecting the driver to respect the current value of bi_idx isn't really
a crazy thing - but it breaks with other drivers. Both bcache and md
have hacks to work around this (c.f. md_trim_bio()) but those hacks have
downsides (you have to either modify the biovec, or clone the whole bio
and biovec).
But if drivers are using the bvec iterator primitives so they don't
modify the biovec, we get this for free.
^ permalink raw reply
* Re: [PATCH] Fix typos found by misspellings
From: Benno Schulenberg @ 2013-06-09 8:54 UTC (permalink / raw)
To: Krzysztof Żelechowski; +Cc: Util-Linux
In-Reply-To: <kotrf8$s14$1@ger.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 662 bytes --]
On Sat, Jun 8, 2013, at 1:49, Krzysztof Żelechowski wrote:
>> The
>> .BR rootcontext=
>> option allows you to explicitly label the root inode of a FS being mounted
>> -before that FS or inode because visable to userspace. This was found to be
>> +before that FS or inode because visible to userspace. This was found to be
>> useful for things like stateless linux.
>
> I still do not understand the first sentence. What is that supposed to
> mean?
Maybe s/because/becomes/?
Attached patch makes that change, plus some grammar fixes.
Regards,
Benno
--
http://www.fastmail.fm - Email service worth paying for. Try it for free
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-docs-fix-mistaken-word-and-some-grammar-in-man-page-.patch --]
[-- Type: text/x-patch; name="0001-docs-fix-mistaken-word-and-some-grammar-in-man-page-.patch", Size: 2179 bytes --]
From 753acb231f5cbbe846f0cdb0717ad91e8255cb4b Mon Sep 17 00:00:00 2001
From: Benno Schulenberg <bensberg@justemail.net>
Date: Sun, 9 Jun 2013 10:50:01 +0200
Subject: [PATCH] docs: fix mistaken word and some grammar in man page of mount
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Reported-by: Krzysztof Żelechowski <giecrilj@stegny.2a.pl>
Signed-off-by: Benno Schulenberg <bensberg@justemail.net>
---
sys-utils/mount.8 | 23 +++++++++++------------
1 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/sys-utils/mount.8 b/sys-utils/mount.8
index 3ffdb24..a9c68fd 100644
--- a/sys-utils/mount.8
+++ b/sys-utils/mount.8
@@ -946,28 +946,27 @@ filesystem that supports xattr labeling.
The
.BR rootcontext=
option allows you to explicitly label the root inode of a FS being mounted
-before that FS or inode because visible to userspace. This was found to be
+before that FS or inode becomes visible to userspace. This was found to be
useful for things like stateless linux.
-Note that kernel rejects any remount request that includes the context
-option even if unchanged from the current context.
+Note that the kernel rejects any remount request that includes the context
+option, \fBeven\fP when unchanged from the current context.
-.B Warning that \fIcontext\fP value might contains comma
-and in this case the value has to be properly quoted otherwise
+.BR "Warning: the \fIcontext\fP value might contain commas" ,
+in which case the value has to be properly quoted, otherwise
.BR mount (8)
-will interpret the comma as separator between mount options. Don't forget that
-shell strips off quotes and
-.BR "double quoting is required" ,
-for example:
+will interpret the comma as a separator between mount options. Don't forget that
+the shell strips off quotes and thus
+.BR "double quoting is required" .
+For example:
.RS
.RS
.sp
-mount -t tmpfs none /mnt \-o 'context="system_u:object_r:tmp_t:s0:c127,c456",noexec'
+.B mount -t tmpfs none /mnt \-o 'context="system_u:object_r:tmp_t:s0:c127,c456",noexec'
.sp
.RE
-
For more details, see
-.BR selinux (8)
+.BR selinux (8).
.RE
.TP
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 2/4] ipset: add "inner" flag implementation
From: Jozsef Kadlecsik @ 2013-06-09 8:54 UTC (permalink / raw)
To: Dash Four; +Cc: Pablo Neira Ayuso, Netfilter Core Team
In-Reply-To: <51B133C5.5020303@googlemail.com>
On Fri, 7 Jun 2013, Dash Four wrote:
> Jozsef Kadlecsik wrote:
> > Also, I'd prefer a single new function (or two inline functions if their
> > size permits), which'd return the success code and in pointers the inner IP
> > header and
> > the offset to the header (for proto/ports). Read my comments below.
> >
> It has to be 2 functions - one dealing with ipv4 headers and another
> implementing ipv6 header handling. Further comments and questions follow
> below.
I meant, similarly to ip_set_get_ip_port, which wraps around the two
functions. So one exported function suffices.
> > > +static inline void
> > > +ip4inneraddrptr(const struct sk_buff *skb, bool src, __be32 *addr)
> > > +{
> > > + struct iphdr _iph;
> > > + struct icmphdr _icmph;
> > > + u8 type;
> > > + const struct iphdr *ih;
> > > + const struct icmphdr *ich;
> > > + static const size_t len = 8 + sizeof(struct iphdr);
> > >
> >
> > I don't like bare numbers as magic constants.
> >
> That's the size of a "standard" header (the "magic" number 8 is also
> used in ipv6_skip_exthdr as well). I can use some more meaningful
> constant if you wish - let me know.
If it's the size of a structure, use a sizeof. (I don't like the number in
ipv6_skip_exthdr either, but that's another thing.)
> > > +#define ip6_ext_hdr(hdr) ((hdr == IPPROTO_HOPOPTS) || \
> > > + (hdr == IPPROTO_ROUTING) || \
> > > + (hdr == IPPROTO_FRAGMENT) || \
> > > + (hdr == IPPROTO_ESP) || \
> > > + (hdr == IPPROTO_AH) || \
> > > + (hdr == IPPROTO_NONE) || \
> > > + (hdr == IPPROTO_DSTOPTS))
> > > +static inline void
> > > +ip6inneraddrptr(const struct sk_buff *skb, bool src, struct in6_addr
> > > *addr)
> > > +{
> > > + u8 type, currenthdr;
> > > + bool fragment = false;
> > > + unsigned int ptr, hdrlen = 0;
> > > + struct ipv6hdr _ip6h;
> > > + struct icmp6hdr _icmp6h;
> > > + const struct ipv6hdr *ih;
> > > + const struct icmp6hdr *ic;
> > > +
> > > + ih = skb_header_pointer(skb, 0, sizeof(_ip6h), &_ip6h);
> > > + if (ih == NULL)
> > > + goto err;
> > > +
> > > + ptr = sizeof(struct ipv6hdr);
> > > + currenthdr = ih->nexthdr;
> > > + while (currenthdr != NEXTHDR_NONE && ip6_ext_hdr(currenthdr)) {
> > > + struct ipv6_opt_hdr _hdr;
> > > + const struct ipv6_opt_hdr *hp;
> > > +
> > > + hp = skb_header_pointer(skb, ptr, sizeof(_hdr), &_hdr);
> > > + if (hp == NULL)
> > > + goto err;
> > > +
> > > + switch (currenthdr) {
> > > + case IPPROTO_FRAGMENT: {
> > > + struct frag_hdr _fhdr;
> > > + const struct frag_hdr *fh;
> > > +
> > > + fh = skb_header_pointer(skb, ptr, sizeof(_fhdr),
> > > + &_fhdr);
> > > + if (fh == NULL)
> > > + goto err;
> > > + if (ntohs(fh->frag_off) & 0xFFF8)
> > > + fragment = true;
> > > +
> > > + hdrlen = 8;
> > > + break;
> > > + }
> > > + case IPPROTO_DSTOPTS:
> > > + case IPPROTO_ROUTING:
> > > + case IPPROTO_HOPOPTS:
> > > + if (fragment)
> > > + goto err;
> > > +
> > > + hdrlen = ipv6_optlen(hp);
> > > + break;
> > > + case IPPROTO_AH:
> > > + hdrlen = (hp->hdrlen+2)<<2;
> > > + break;
> > > + case IPPROTO_ESP:
> > > + default:
> > > + goto err;
> > > + } /* switch IPPROTO */
> > > +
> > > + currenthdr = hp->nexthdr;
> > > + ptr += hdrlen;
> > > + }
> > >
> >
> > The code segment above is the reimplementation of ipv6_skip_exthdr.
> >
> No, not really. Even though there are similarities between the two
> fucntions, there are 2 differences - one is that ipv6_skip_exthdr wasn't
> dealing properly (from ipset's point of view) with fragments, "frag_off"
> is not present in kernel versions < 3.3 and the other is that
> ipv6_skip_exthdr doesn't return an error when either an unknown or ESP
> header type is encountered ("case IPPROTO_ESP" and "default" above).
ipv6_skip_exthdr handles ESP as a normal protocol, stops processing there,
so the called deals with it. Also, we must process (ignore) unknown
extension headers, that's not an error condition.
Kernel versions before 3.3 are supported as is. I don't see the point in
fixing ipv6_skip_exthdr just for ipset when the same function is used
"unpatched" in the kernel itself before 3.3.
> > These all can be replaced by a function which skips the external header and
> > points to the inner one, something like:
> >
> > int
> > ipset_inner_header(const struct sk_buff *skb, u8 pf, u8 *offset,
> > void *inner)
> > {
> > /* IPv4 */
> > ...
> > /* IPv6 */
> > nexthdr = ipv6_hdr(skb)->nexthdr;
> > *offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr),
> > &nexthdr, &frag_off);
> > /* Validate offset and frag_off */
> > /* Check ICMPv6, update offset */
> > /* Copy inner IPv6 header to *inner by skb_header_pointer */
> >
> > return success/error;
> > }
> >
> IPv4 and IPv6 should be treated as separate cases, so I'll create two
> separate functions for that.
That'd require two exported functions (check the allowed size for inline
functions), and I'm unsure two exports will be accepted.
> One other thing - this type of operation is not going to be confined
> just to ipset (I plan to do the same thing when submit the changes to
> the AUDIT iptables target, which logs these attributes as well), so I am
> thinking of offloading these two functions and moving everything to
> ip[v6].h so that they can be used not just by ipset. What do you think?
You can put inline functions/macros into header files only and
function size may block using inlined.
> > Current ip[46]addr[ptr] functions of course need to be modified to use the
> > IP header structures instead of the skbuff one.
> >
> I am not sure there should be one function for this - IPv6 and IPv4 are two
> separate cases (what if I don't have IPv6 - the whole IPv6 code will never be
> executed/needed, so there isn't any reason to be there at all), so I think
> there should be two separate functions for these.
That can be solved by empty function definition. I mean, in the
ipset_inner_header example above, the IPv4/IPv6 cases are handled in
independent functions and the one for IPv6 is actually empty when there's
no IPv6 enabled at all.
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply
* Confirmed Reciept.....
From: info @ 2013-06-09 8:48 UTC (permalink / raw)
To: Recipients
Uk National Lottery
Ref: L/200-26937
Batch: 2007MJL-01
FINAL NOTIFICATION
We are pleased to inform you today 6th June, 2013 of the result
of the winners of the UK NATIONAL LOTTERY ONLINE PROMO PROGRAMME, held
on the 30th of May, 2013.
You have therefore been approved for a lump sum pay out of £1,000 000
(One Million Pounds Sterling) in cash credited to file XYL/26510460037/06.To file for your claim,
please contact our claims agent;
Agents Name: Dr.Jones Greene
Emai: claimjonesgreene@yahoo.co.uk
Provide him with the information below:
1.Full Name:
2.Full Address:
3.Marital Status:
4.Occupation:
5.Age:
6.Sex:
7.Nationality:
8.Country Of Residence:
9.Telephone Number:
Congratulations once more from all members and staff of this program.
Sincerely,
Dr.Jones Greene
UK NATIONAL LOTTERY
^ permalink raw reply
* Re: [PATCH v8 00/11] KVM: MMU: fast zap all shadow pages
From: Gleb Natapov @ 2013-06-09 8:53 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: avi.kivity, mtosatti, pbonzini, linux-kernel, kvm
In-Reply-To: <1369960590-14138-1-git-send-email-xiaoguangrong@linux.vnet.ibm.com>
On Fri, May 31, 2013 at 08:36:19AM +0800, Xiao Guangrong wrote:
> Hi Gleb, Paolo, Marcelo,
>
> I have putted the potential controversial patches to the latter that are
> patch 8 ~ 10, patch 11 depends on patch 9. Other patches are fully reviewed,
> I think its are ready for being merged. If not luck enough, further discussion
> is needed, could you please apply that patches first? :)
>
> Thank you in advance!
>
> Some points are raised during discussion but missed in this version:
> 1) Gleb's idea that skip obsolete pages in the hast list walker
>
> Unfortunately, it is not safe. There has a window between updating
> valid-gen and reloading mmu, in that window, the obsolete page can
> be used by vcpu, but the guest page table fail to be write-protected
> (since the obsolete page is skipped in mmu_need_write_protect()).
>
Can you elaborate on how this can happen. valid_gen is updated under
mmu_lock and reloading of mmus happens under the same lock, so for all
other vcpus this should look like atomic thing.
> Instead, we can only skip the zapped-obsolete page
> (is_obsolete_sp(sp) && sp->role.invalid)), the current code has already
> skip them but put the comment around the hash list walker to warn the
> further development.
>
> 2) Marcelo's comment that obsolete pages can cause the number of shadow page
> greater than the n_max_mmu_pages
>
> I am not sure this is really a problem, it only exists in the really tiny
> window and the page-reclaim path are able to handle the obsolete pages.
> Furthermore, we can properly reduce n_max_mmu_pages to make that window
> more tiny.
>
> Anyway, like commit 5d21881432 shows that "the mmu counters are for
> beancounting purposes only", maybe that window is allowed.
>
> Changlog:
> V8:
> 1): add some comments to explain FIFO around active_mmu_list address
> Marcelo's comments.
>
> 2): the page-reclaim path may fail to free zapped-obsolete pages pointed
> out by Marcelo, the patchset fixes it by listing all zapped obsolete
> pages on a global list, always free page from that list first.
>
> 3): address Marcelo's suggestion to move the "zap pages in batch" patch
> to the latter.
>
> 4): drop the previous patch which introduced
> kvm_mmu_prepare_zap_obsolete_page(), instead, we put the comments
> around hash list walker to warn the user that the zapped-obsolete
> page still live on hash list.
>
> 5): add the note into the changelog of "zap pages in batch" patch to explain
> the batch number is the speculative value based on Takuya's comments.
>
> V7:
> 1): separate some optimization into two patches which do not reuse
> the obsolete pages and collapse tlb flushes, suggested by Marcelo.
>
> 2): make the patch based on Gleb's diff change which reduce
> KVM_REQ_MMU_RELOAD when root page is being zapped.
>
> 3): remove calling kvm_mmu_zap_page when patching hypercall, investigated
> by Gleb.
>
> 4): drop the patch which deleted page from hash list at the "prepare"
> time since it can break the walk based on hash list.
>
> 5): rename kvm_mmu_invalidate_all_pages to kvm_mmu_invalidate_zap_all_pages.
>
> 6): introduce kvm_mmu_prepare_zap_obsolete_page which is used to zap obsolete
> page to collapse tlb flushes.
>
> V6:
> 1): reversely walk active_list to skip the new created pages based
> on the comments from Gleb and Paolo.
>
> 2): completely replace kvm_mmu_zap_all by kvm_mmu_invalidate_all_pages
> based on Gleb's comments.
>
> 3): improve the parameters of kvm_mmu_invalidate_all_pages based on
> Gleb's comments.
>
> 4): rename kvm_mmu_invalidate_memslot_pages to kvm_mmu_invalidate_all_pages
> 5): rename zap_invalid_pages to kvm_zap_obsolete_pages
>
> V5:
> 1): rename is_valid_sp to is_obsolete_sp
> 2): use lock-break technique to zap all old pages instead of only pages
> linked on invalid slot's rmap suggested by Marcelo.
> 3): trace invalid pages and kvm_mmu_invalidate_memslot_pages()
> 4): rename kvm_mmu_invalid_memslot_pages to kvm_mmu_invalidate_memslot_pages
> according to Takuya's comments.
>
> V4:
> 1): drop unmapping invalid rmap out of mmu-lock and use lock-break technique
> instead. Thanks to Gleb's comments.
>
> 2): needn't handle invalid-gen pages specially due to page table always
> switched by KVM_REQ_MMU_RELOAD. Thanks to Marcelo's comments.
>
> V3:
> completely redesign the algorithm, please see below.
>
> V2:
> - do not reset n_requested_mmu_pages and n_max_mmu_pages
> - batch free root shadow pages to reduce vcpu notification and mmu-lock
> contention
> - remove the first patch that introduce kvm->arch.mmu_cache since we only
> 'memset zero' on hashtable rather than all mmu cache members in this
> version
> - remove unnecessary kvm_reload_remote_mmus after kvm_mmu_zap_all
>
> * Issue
> The current kvm_mmu_zap_all is really slow - it is holding mmu-lock to
> walk and zap all shadow pages one by one, also it need to zap all guest
> page's rmap and all shadow page's parent spte list. Particularly, things
> become worse if guest uses more memory or vcpus. It is not good for
> scalability.
>
> * Idea
> KVM maintains a global mmu invalid generation-number which is stored in
> kvm->arch.mmu_valid_gen and every shadow page stores the current global
> generation-number into sp->mmu_valid_gen when it is created.
>
> When KVM need zap all shadow pages sptes, it just simply increase the
> global generation-number then reload root shadow pages on all vcpus.
> Vcpu will create a new shadow page table according to current kvm's
> generation-number. It ensures the old pages are not used any more.
>
> Then the invalid-gen pages (sp->mmu_valid_gen != kvm->arch.mmu_valid_gen)
> are zapped by using lock-break technique.
>
> Gleb Natapov (1):
> KVM: MMU: reduce KVM_REQ_MMU_RELOAD when root page is zapped
>
> Xiao Guangrong (10):
> KVM: x86: drop calling kvm_mmu_zap_all in emulator_fix_hypercall
> KVM: MMU: drop unnecessary kvm_reload_remote_mmus
> KVM: MMU: fast invalidate all pages
> KVM: x86: use the fast way to invalidate all pages
> KVM: MMU: show mmu_valid_gen in shadow page related tracepoints
> KVM: MMU: add tracepoint for kvm_mmu_invalidate_all_pages
> KVM: MMU: do not reuse the obsolete page
> KVM: MMU: zap pages in batch
> KVM: MMU: collapse TLB flushes when zap all pages
> KVM: MMU: reclaim the zapped-obsolete page first
>
> arch/x86/include/asm/kvm_host.h | 4 +
> arch/x86/kvm/mmu.c | 128 ++++++++++++++++++++++++++++++++++++---
> arch/x86/kvm/mmu.h | 1 +
> arch/x86/kvm/mmutrace.h | 42 ++++++++++---
> arch/x86/kvm/x86.c | 17 +----
> 5 files changed, 161 insertions(+), 31 deletions(-)
>
> --
> 1.7.7.6
--
Gleb.
^ permalink raw reply
* Re: [PATCH v2] ethernet/arc/arc_emac - Add new driver
From: Alexey Brodkin @ 2013-06-09 8:53 UTC (permalink / raw)
To: Francois Romieu
Cc: Alexey Brodkin, netdev@vger.kernel.org, Vineet Gupta,
Mischa Jonker, Arnd Bergmann, Grant Likely, Rob Herring,
Paul Gortmaker, David S. Miller, linux-kernel@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, joe@perches.com
In-Reply-To: <20130607203231.GA14077@electric-eye.fr.zoreil.com>
On 06/08/2013 12:33 AM, Francois Romieu wrote:
> Alexey Brodkin <Alexey.Brodkin@synopsys.com> :
[]
>> + * Vineet Gupta: Nov 2009
>> + * -Rewrote the driver register access macros so that multiple
accesses
>> + * in same function use "anchor" reg to save the base addr causing
>> + * shorter instructions
>
> The kernel has been using git for some time: even if you don't remove
this
> stuff, you shouldn't add more of it.
As replied to Joe I just want to name people contributed in this driver.
What is a appropriate way to do it?
>> +#define NAPI_WEIGHT 40 /* Workload for NAPI */
>
> ARC_EMAC_NAPI_WEIGHT ?
If it makes sense I may rename this one. I didn't do it earlier just
because this symbol is not exposed to anything outside this driver so
short and might be common name shouldn't hurt.
>> +struct arc_emac_bd_t {
>> + unsigned int info;
>> + void *data;
>
> Why no char * ?
>
> It's skb->data after all.
Makes sense - will correct.
> [...]
>> +static const struct ethtool_ops arc_emac_ethtool_ops = {
>> + .get_settings = arc_emac_get_settings,
>> + .set_settings = arc_emac_set_settings,
>> + .get_drvinfo = arc_emac_get_drvinfo,
>> + .get_link = ethtool_op_get_link,
>
> .get_settings = arc_emac_get_settings,
> .set_settings = arc_emac_set_settings,
> .get_drvinfo = arc_emac_get_drvinfo,
> .get_link = ethtool_op_get_link,
Good point. Thanks.
>> +};
>> +
>> +static int arc_emac_poll(struct napi_struct *napi, int budget)
>> +{
>> + struct net_device *net_dev = napi->dev;
>> + struct arc_emac_priv *priv = netdev_priv(net_dev);
>> + struct sk_buff *skb, *skbnew;
>> + unsigned int i, loop, len, info, work_done = 0;
>
> You may reduce the scope of several variables.
Correct.
>> +
>> + /* Make a note that we saw a packet at this BD.
>> + * So next time, driver starts from this + 1
>> + */
>> + priv->last_rx_bd = i;
>> +
>> + /* Packet fits in one BD (Non Fragmented) */
>> + if (likely((info & (FRST_MASK | LAST_MASK)) ==
>> + (FRST_MASK | LAST_MASK))) {
>
> You may #define (FRST_MASK | LAST_MASK)
This combo is used in only 2 places so is it worth to introduce another
define? With these (FRST_MASK | LAST_MASK) I suppose reader will
understand that these are 2 separate bits. But still it might be just my
vision and if #define (FRST_MASK | LAST_MASK) looks better then I'll add
it immediately.
>> +
>> + len = info & LEN_MASK;
>> + priv->stats.rx_packets++;
>> + priv->stats.rx_bytes += len;
>> + skb = priv->rx_skbuff[i];
>> +
>> + /* Get a new SKB from stack */
>> + skbnew = netdev_alloc_skb(net_dev,
>> + net_dev->mtu +
>> + EMAC_BUFFER_PAD);
>> +
>> + if (!skbnew) {
>> + netdev_err(net_dev, "Out of memory, "
>> + "dropping packet\n");
>
> Rate limit or do nothing.
Not clear what do you mean. Could you please clarify?
>> +
>> + /* Return buffer to EMAC */
>> + priv->rxbd[i].info = FOR_EMAC |
>> + (net_dev->mtu + EMAC_BUFFER_PAD);
>> + priv->stats.rx_dropped++;
>> + continue;
>> + }
>> +
>> + /* Actually preparing the BD for next cycle
>> + * IP header align, eth is 14 bytes
>> + */
>> + skb_reserve(skbnew, 2);
>
> netdev_alloc_skb_ip_align
Thanks, will use this one instead.
>> + priv->rx_skbuff[i] = skbnew;
>> +
>> + priv->rxbd[i].data = skbnew->data;
>> + priv->rxbd[i].info = FOR_EMAC |
>> + (net_dev->mtu + EMAC_BUFFER_PAD);
>> +
>> + /* Prepare arrived pkt for delivery to stack */
>> + dma_map_single(&net_dev->dev, (void *)skb->data,
>> + len, DMA_FROM_DEVICE);
>
> dma_map_single may fail.
Correct. Will add test of returned value.
> Speaking of it, where are dma_unmap and dma_sync ?
Oops, this is something I need to look at a bit now)
> [...]
>> +/**
>> + * arc_emac_intr - Global interrupt handler for EMAC.
>> + * @irq: irq number.
>> + * @net_dev: net_device pointer.
>> + *
>> + * returns: IRQ_HANDLED for all cases.
>> + *
>> + * ARC EMAC has only 1 interrupt line, and depending on bits raised in
>> + * STATUS register we may tell what is a reason for interrupt to fire.
>> + */
>> +static irqreturn_t arc_emac_intr(int irq, void *dev_instance)
>> +{
>> + struct net_device *net_dev = (struct net_device *)dev_instance;
>
> Useless cast from void *
Agree.
>> + struct arc_emac_priv *priv = netdev_priv(net_dev);
>> + unsigned int status;
>> +
>> + /* Read STATUS register from EMAC */
>> + status = EMAC_REG_GET(priv->reg_base_addr, R_STATUS);
>> +
>> + /* Mask all bits except "MDIO complete" */
>> + status &= ~MDIO_MASK;
>> +
>> + /* To reset any bit in STATUS register we need to write "1" in
>> + * corresponding bit. That's why we write only masked bits back.
>> + */
>> + EMAC_REG_SET(priv->reg_base_addr, R_STATUS, status);
>> +
>> + if (likely(status & (RXINT_MASK | TXINT_MASK))) {
>> + if (status & RXINT_MASK) {
>> + if (likely(napi_schedule_prep(&priv->napi))) {
>> + EMAC_REG_CLR(priv->reg_base_addr, R_ENABLE,
>> + RXINT_MASK);
>> + __napi_schedule(&priv->napi);
>> + }
>> + }
>> + if (status & TXINT_MASK) {
>> + unsigned int i, info;
>> + struct sk_buff *skb;
>> +
>> + for (i = 0; i < TX_BD_NUM; i++) {
>> + info = priv->txbd[priv->txbd_dirty].info;
>> +
>> + if (info & (DROP | DEFR | LTCL | UFLO))
>> + netdev_warn(net_dev,
>> + "add Tx errors to stats\n");
>> +
>> + if ((info & FOR_EMAC) ||
>> + !priv->txbd[priv->txbd_dirty].data)
>
> if ((info & FOR_EMAC) ||
> !priv->txbd[priv->txbd_dirty].data)
>
Incorrect indentation, I see this now.
>> + break;
>> +
>> + if (info & LAST_MASK) {
>> + skb = priv->tx_skbuff[priv->txbd_dirty];
>> + priv->stats.tx_packets++;
>> + priv->stats.tx_bytes += skb->len;
>> +
>> + /* return the sk_buff to system */
>> + dev_kfree_skb_irq(skb);
>> + }
>> + priv->txbd[priv->txbd_dirty].data = 0x0;
>> + priv->txbd[priv->txbd_dirty].info = 0x0;
>> + priv->txbd_dirty = (priv->txbd_dirty + 1) %
>> + TX_BD_NUM;
>> + }
>> + }
>> + } else {
>> + if (status & ERR_MASK) {
>> + /* MSER/RXCR/RXFR/RXFL interrupt fires on corresponding
>> + * 8-bit error counter overrun.
>> + * Because of this fact we add 256 items each time
>> + * overrun interrupt happens.
>> + */
>
> Please use a local &priv->stats variable.
Ok, makes sense.
>> +
>> + if (status & TXCH_MASK) {
>> + priv->stats.tx_errors++;
>> + priv->stats.tx_aborted_errors++;
>> + netdev_err(priv->net_dev,
>> + "Tx chaining err! txbd_dirty = %u\n",
>> + priv->txbd_dirty);
>> + } else if (status & MSER_MASK) {
>> + priv->stats.rx_missed_errors += 255;
>> + priv->stats.rx_errors += 255;
>> + } else if (status & RXCR_MASK) {
>> + priv->stats.rx_crc_errors += 255;
>> + priv->stats.rx_errors += 255;
>> + } else if (status & RXFR_MASK) {
>> + priv->stats.rx_frame_errors += 255;
>> + priv->stats.rx_errors += 255;
>> + } else if (status & RXFL_MASK) {
>> + priv->stats.rx_over_errors += 255;
>> + priv->stats.rx_errors += 255;
>> + } else {
>> + netdev_err(priv->net_dev,
>> + "unknown err. Status reg is 0x%x\n",
>> + status);
>> + }
>> + }
>> + }
>> + return IRQ_HANDLED;
>> +}
>> +
>> +/**
>> + * arc_emac_open - Open the network device.
>> + * @net_dev: Pointer to the network device.
>> + *
>> + * returns: 0, on success or non-zero error value on failure.
>> + *
>> + * This function sets the MAC address, requests and enables an IRQ
>> + * for the EMAC device and starts the Tx queue.
>> + * It also connects to the phy device.
>> + */
>> +int arc_emac_open(struct net_device *net_dev)
>
> static
>
Why didn't I put static for every other function? Will fix it now.
>> +{
>> + struct arc_emac_priv *priv = netdev_priv(net_dev);
>> + struct arc_emac_bd_t *bd;
>> + struct sk_buff *skb;
>> + int i;
>> +
>> + if (!priv->phy_node) {
>> + netdev_err(net_dev, "arc_emac_open: phy device is absent\n");
>> + return -ENODEV;
>> + }
>> +
>> + priv->phy_dev = of_phy_connect(net_dev, priv->phy_node,
>> + arc_emac_adjust_link, 0,
>> + PHY_INTERFACE_MODE_MII);
>> +
>> + if (!priv->phy_dev) {
>> + netdev_err(net_dev, "of_phy_connect() failed\n");
>> + return -ENODEV;
>> + }
>
> Is there a reason why it could not be done in probe and thus save
> some !priv->phy_dev checks ?
I think that I saw in couple of drivers phy connection is done in
"open". That's why I put mine here too. In general I think it's possible
to move this functionality in "probe" easily.
>> +
>> + netdev_info(net_dev, "connected to %s phy with id 0x%x\n",
>> + priv->phy_dev->drv->name, priv->phy_dev->phy_id);
>> +
>> + priv->phy_dev->autoneg = AUTONEG_ENABLE;
>> + priv->phy_dev->speed = 0;
>> + priv->phy_dev->duplex = 0;
>> +
>> + /* We support only up-to 100mbps speeds */
>> + priv->phy_dev->advertising = priv->phy_dev->supported;
>> + priv->phy_dev->advertising &= PHY_BASIC_FEATURES;
>> + priv->phy_dev->advertising |= ADVERTISED_Autoneg;
>
> I'd go for a local priv->phy_dev.
Makes sense.
>> +
>> + /* Restart auto negotiation */
>> + phy_start_aneg(priv->phy_dev);
>> +
>> + netif_start_queue(net_dev);
>
> No need to rush. Please finish software init.
Ok.
>> +
>> + /* Allocate and set buffers for Rx BD's */
>> + bd = priv->rxbd;
>> + for (i = 0; i < RX_BD_NUM; i++) {
>> + skb = netdev_alloc_skb(net_dev, net_dev->mtu + EMAC_BUFFER_PAD);
>
> Missing NULL check.
Correct. Will fix.
>> +{
>> + struct arc_emac_priv *priv = netdev_priv(net_dev);
>> +
>> + napi_disable(&priv->napi);
>> + netif_stop_queue(net_dev);
>> +
>> + /* Disable interrupts */
>> + EMAC_REG_CLR(priv->reg_base_addr, R_ENABLE,
>> + TXINT_MASK | /* Tx interrupt */
>> + RXINT_MASK | /* Rx interrupt */
>> + ERR_MASK | /* Error interrupt */
>> + TXCH_MASK); /* Transmit chaining error interrupt */
>
> Useless comments.
Why so? Is it clear that TXCH means "Transmit chaining error interrupt"?
Or those defines should just have comments where they are defined and
later just use them with no comments?
>
>> +
>> + if (priv->phy_dev)
>> + phy_disconnect(priv->phy_dev);
>
> arc_emac_open succeeded: priv->phy_dev can't be NULL.
Right. Useless check.
>> +{
>> + int len, bitmask;
>> + unsigned int info;
>> + char *pkt;
>> + struct arc_emac_priv *priv = netdev_priv(net_dev);
>> +
>> + len = skb->len < ETH_ZLEN ? ETH_ZLEN : skb->len;
>> + pkt = skb->data;
>> + net_dev->trans_start = jiffies;
>> +
>> + dma_map_single(&net_dev->dev, (void *)pkt, len, DMA_TO_DEVICE);
>
> dma_map_single may fail.
Check is needed I see.
>> +static const struct net_device_ops arc_emac_netdev_ops = {
>> + .ndo_open = arc_emac_open,
>> + .ndo_stop = arc_emac_stop,
>> + .ndo_start_xmit = arc_emac_tx,
>> + .ndo_set_mac_address = arc_emac_set_address,
>> + .ndo_get_stats = arc_emac_stats,
>
> Please use tabs before "=".
Ok.
>> +static struct platform_driver arc_emac_driver = {
>> + .probe = arc_emac_probe,
>> + .remove = arc_emac_remove,
>> + .driver = {
>> + .name = DRV_NAME,
>> + .owner = THIS_MODULE,
>> + .of_match_table = arc_emac_dt_ids,
>> + },
>
> Excess tab.
Ok.
> [...]
>> diff --git a/drivers/net/ethernet/arc/arc_emac_mdio.c
b/drivers/net/ethernet/arc/arc_emac_mdio.c
>> new file mode 100644
>> index 0000000..7d13dd5
>> --- /dev/null
>> +++ b/drivers/net/ethernet/arc/arc_emac_mdio.c
> [...]
>> +static int arc_mdio_complete_wait(struct arc_mdio_priv *priv)
>> +{
>> + unsigned int status;
>
> Excess scope.
Ok.
>> + unsigned int count = (ARC_MDIO_COMPLETE_POLL_COUNT * 40);
>> +
>> + while (1) {
>> + /* Read STATUS register from EMAC */
>> + status = EMAC_REG_GET(priv->reg_base_addr, R_STATUS);
>
> Useless comment.
Might be so.
>> +
>> + /* Mask "MDIO complete" bit */
>> + status &= MDIO_MASK;
>> +
>> + if (status) {
>> + /* Reset "MDIO complete" flag */
>> + EMAC_REG_SET(priv->reg_base_addr, R_STATUS, status);
>> + break;
>
> return 0;
I prefer 1 exit point. That's why I put here "break".
>> + }
>> +
>> + /* Make sure we never get into infinite loop */
>> + if (count-- == 0) {
>
> KISS: use a for loop ?
>
Good point. Indeed "for" fits better here.
>> + WARN_ON(1);
>> + return -ETIMEDOUT;
>> + }
>> + msleep(25);
>> + }
>> + return 0;
>> +}
> [...]
>> +static int arc_mdio_read(struct mii_bus *bus, int phy_addr, int
reg_num)
>> +{
>> + int error;
>> + unsigned int value;
>> + struct arc_mdio_priv *priv = bus->priv;
>
> Revert the xmas tree.
Not clear what does it mean? Could you please calrify?
> [...]
>> +int arc_mdio_probe(struct device_node *dev_node, struct
arc_mdio_priv *priv)
>> +{
>> + struct mii_bus *bus;
>> + int error;
>> +
>> + bus = mdiobus_alloc();
>> + if (!bus) {
>> + error = -ENOMEM;
>> + goto cleanup;
>
> Nothing needs to be cleaned up.
Seems like that's true )
> [...]
>> diff --git a/drivers/net/ethernet/arc/arc_emac_mdio.h
b/drivers/net/ethernet/arc/arc_emac_mdio.h
>> new file mode 100644
>> index 0000000..954241e
>> --- /dev/null
>> +++ b/drivers/net/ethernet/arc/arc_emac_mdio.h
>> @@ -0,0 +1,22 @@
>> +/*
>> + * Copyright (C) 2004, 2007-2010, 2011-2013 Synopsys, Inc.
(www.synopsys.com)
>> + *
>> + * Definitions for MDIO of ARC EMAC device driver
>> + */
>> +
>> +#ifndef ARC_MDIO_H
>> +#define ARC_MDIO_H
>> +
>> +#include <linux/device.h>
>> +#include <linux/phy.h>
>> +
>> +struct arc_mdio_priv {
>> + struct mii_bus *bus;
>> + struct device *dev;
>> + void __iomem *reg_base_addr; /* MAC registers base address */
>> +};
>
> Overengineered ?
Why so? Not clear, sorry.
> [...]
>> diff --git a/drivers/net/ethernet/arc/arc_emac_regs.h
b/drivers/net/ethernet/arc/arc_emac_regs.h
>> new file mode 100644
>> index 0000000..e4c8d73
>> --- /dev/null
>> +++ b/drivers/net/ethernet/arc/arc_emac_regs.h
> [...]
>> +#define EMAC_REG_SET(reg_base_addr, reg, val) \
>> + iowrite32((val), reg_base_addr + reg * sizeof(int))
>> +
>> +#define EMAC_REG_GET(reg_base_addr, reg) \
>> + ioread32(reg_base_addr + reg * sizeof(int))
>
> May use real non-caps functions.
Do you mean to use "io{read|write}32" directly without macro?
>> +
>> +#define EMAC_REG_OR(b, r, v) EMAC_REG_SET(b, r, EMAC_REG_GET(b, r)
| (v))
>> +#define EMAC_REG_CLR(b, r, v) EMAC_REG_SET(b, r, EMAC_REG_GET(b, r)
& ~(v))
>
> May use real non-caps functions.
>
> Go ahead.
>
Thanks a lot for this deep analisys.
-Alexey
^ permalink raw reply
* Re: No HDMI output on AC100
From: Thomas Meyer @ 2013-06-09 8:52 UTC (permalink / raw)
To: Marc Dietrich; +Cc: linux-tegra, Lucas Stach
In-Reply-To: <2572002.IOA8y0azF4@ax5200p>
Am Samstag, den 08.06.2013, 22:07 +0200 schrieb Marc Dietrich:
> On Saturday 08 June 2013 18:05:52 Lucas Stach wrote:
> > Am Samstag, den 08.06.2013, 16:21 +0200 schrieb Thomas Meyer:
> > > > On Saturday 08 June 2013 10:33:55 Thomas Meyer wrote:
> > > >> Kernel version: 3.10.0-0.rc4.git0.1.fc20.armv7hl
> > > >>
> > > >> This is the latest Fedora ARM kernel. Config is available here:
> > > >> http://pkgs.fedoraproject.org/cgit/kernel.git/tree/config-armv7
> > > >>
> > > >> This is what I see in the kernel log:
> > > >>
> > > >> $ dmesg |grep hdmi
> > > >> [ 33.403417] tegra-hdmi 54280000.hdmi: failed to get VDD regulator
> > > >> [ 33.403441] platform 54280000.hdmi: Driver tegra-hdmi requests probe
> > > >>
> > > >> any ideas?
> > > >
> > > > is the regulator (tps6586x) build-in?
> > >
> > > No, it's a module.
> > >
> > > > The make everything a module path isn't
> > > > well tested I guess.
> >
> > It's not the problem here. The driver requests to be probed again and
> > comes up once the regulator is there.
> >
> > Though the pixel clock looks odd for a HD HDMI monitor. Try looking at
> > the Xorg log to see what mode of your monitor get's chosen and why it
> > fails.
>
> I just checked myself. Found that tps6586x-regulator module can't be
> autoloaded, so I connected my TV, booted and modprobed the module by hand.
> All works more or less fine, so probe deferral is no problem.
>
> Thomas, did you add cma=64M to you kernel command line? HDMI needs some
> memory.
>
oops! I think everthing did always work correctly! But somehow Fedora 19
fails to spawn a getty on the tty1/fb0 console... so I just did see a
black screen :-)
> Marc
>
^ permalink raw reply
* Important Proposal!
From: Lee, leengdfe@live.cn @ 2013-06-04 6:59 UTC (permalink / raw)
To: netdev
Good-Day ,
I am Lee Kam Leung, a senior Asset Manager and Actuary with C.T.B Corporation, in
Beijing, China. I am contacting you with regards to a discreet and somewhat sensitive
business asset available to my knowledge by virtue of my position.
I will furnish you with further exclusive and of course, confidential details once I
receive your affirmatory response
Reply to this e-mail address : kamleung10@yahoo.com.hk
Faithfully,
Lee Kam Leung
>From the Asset Management Department.
^ permalink raw reply
* [PATCH 1/2] i2c-designware: support no DW_IC_COMP_TYPE platform
From: zhangfei @ 2013-06-09 8:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130609030600.GA4312@tarshish>
On 13-06-09 11:06 AM, Baruch Siach wrote:
> Hi Zhangfei Gao,
>
> On Sun, Jun 09, 2013 at 10:36:41AM +0800, Zhangfei Gao wrote:
>> Check accessor_flags before reading DW_IC_COMP_TYPE
>> Give chance to platform no such register
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>> ---
>> drivers/i2c/busses/i2c-designware-core.c | 24 +++++++++++++-----------
>> drivers/i2c/busses/i2c-designware-core.h | 1 +
>> 2 files changed, 14 insertions(+), 11 deletions(-)
>
> [...]
>
>> diff --git a/drivers/i2c/busses/i2c-designware-core.h
>> b/drivers/i2c/busses/i2c-designware-core.h
>> index e761ad1..b38b54e 100644
>> --- a/drivers/i2c/busses/i2c-designware-core.h
>> +++ b/drivers/i2c/busses/i2c-designware-core.h
>> @@ -94,6 +94,7 @@ struct dw_i2c_dev {
>>
>> #define ACCESS_SWAP 0x00000001
>> #define ACCESS_16BIT 0x00000002
>> +#define ACCESS_32BIT 0x00000004
>
> This belongs to the next patch, not to this one.
>
> baruch
>
Thanks baruch, make sense.
^ permalink raw reply
* Re: [PATCH] KVM: x86: fix missed memory synchronization when patch hypercall
From: Gleb Natapov @ 2013-06-09 8:45 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Paolo Bonzini, Marcelo Tosatti, LKML, KVM
In-Reply-To: <51B2A1D9.6060306@gmail.com>
On Sat, Jun 08, 2013 at 11:15:37AM +0800, Xiao Guangrong wrote:
> From: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
>
> Currently, memory synchronization is missed in emulator_fix_hypercall,
> please see the commit 758ccc89b83
> (KVM: x86: drop calling kvm_mmu_zap_all in emulator_fix_hypercall)
>
> This patch fixes it by introducing kvm_vcpus_hang_on_page_start() and
> kvm_vcpus_hang_on_page_end which unmap the patched page from guest
> and use kvm_flush_remote_tlbs() as the serializing instruction to
> ensure the memory coherence
> [ The SDM said that INVEPT, INVVPID and MOV (to control register, with
> the exception of MOV CR8) are the serializing instructions. ]
>
> The mmu-lock is held during host patches the page so that it stops vcpus
> to fix its further page fault
>
I have a patch to implement is much simple and in generic way, not
relying on MMU internals.
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6402951..49774ab 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5517,7 +5517,7 @@ out:
}
EXPORT_SYMBOL_GPL(kvm_emulate_hypercall);
-static int emulator_fix_hypercall(struct x86_emulate_ctxt *ctxt)
+static int emulator_fix_hypercall_cb(void *ctxt)
{
struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
char instruction[3];
@@ -5528,6 +5528,14 @@ static int emulator_fix_hypercall(struct x86_emulate_ctxt *ctxt)
return emulator_write_emulated(ctxt, rip, instruction, 3, NULL);
}
+static int emulator_fix_hypercall(struct x86_emulate_ctxt *ctxt)
+{
+ struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
+ return kvm_exec_with_stopped_vcpu(vcpu->kvm,
+ emulator_fix_hypercall_cb, ctxt);
+}
+
+
/*
* Check if userspace requested an interrupt window, and that the
* interrupt window is open.
@@ -5761,6 +5769,10 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
kvm_deliver_pmi(vcpu);
if (kvm_check_request(KVM_REQ_SCAN_IOAPIC, vcpu))
vcpu_scan_ioapic(vcpu);
+ if (kvm_check_request(KVM_REQ_STOP_VCPU, vcpu)){
+ mutex_lock(&vcpu->kvm->lock);
+ mutex_unlock(&vcpu->kvm->lock);
+ }
}
if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index e3aae6d..6c9361a 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -125,6 +125,7 @@ static inline bool is_error_page(struct page *page)
#define KVM_REQ_EPR_EXIT 20
#define KVM_REQ_SCAN_IOAPIC 21
#define KVM_REQ_GLOBAL_CLOCK_UPDATE 22
+#define KVM_REQ_STOP_VCPU 23
#define KVM_USERSPACE_IRQ_SOURCE_ID 0
#define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID 1
@@ -576,6 +577,7 @@ void kvm_flush_remote_tlbs(struct kvm *kvm);
void kvm_reload_remote_mmus(struct kvm *kvm);
void kvm_make_mclock_inprogress_request(struct kvm *kvm);
void kvm_make_scan_ioapic_request(struct kvm *kvm);
+int kvm_exec_with_stopped_vcpu(struct kvm *kvm, int (*cb)(void *), void *data);
long kvm_arch_dev_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..531e765 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -222,6 +222,18 @@ void kvm_make_scan_ioapic_request(struct kvm *kvm)
make_all_cpus_request(kvm, KVM_REQ_SCAN_IOAPIC);
}
+int kvm_exec_with_stopped_vcpu(struct kvm *kvm, int (*cb)(void *), void *data)
+{
+ int r;
+
+ mutex_lock(&kvm->lock);
+ make_all_cpus_request(kvm, KVM_REQ_STOP_VCPU);
+ r = cb(data);
+ mutex_unlock(&kvm->lock);
+
+ return r;
+}
+
int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
{
struct page *page;
--
Gleb.
^ permalink raw reply related
* Re: [PATCH] Boottime: A tool for automatic measurement of kernel/bootloader boot time
From: Steve Liu @ 2013-06-09 8:17 UTC (permalink / raw)
To: linux-kernel
In-Reply-To: <20121023071951.GA2705@gmail.com>
how about this patch, do we really need this patch
^ permalink raw reply
* [V1 7/7] video: mmp: add video layer set win/addr operations support
From: Jett.Zhou @ 2013-06-09 8:45 UTC (permalink / raw)
To: linux-arm-kernel
From: Jing Xiang <jxiang@marvell.com>
Since mmp display controller support graphic/video layer together, so
add win/addr setting operations support when set win parameters.
Signed-off-by: Jing Xiang <jxiang@marvell.com>
Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
---
drivers/video/mmp/hw/mmp_ctrl.c | 30 +++++++++++++++++++++++-------
1 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c
index 92a5f44..4da6268 100644
--- a/drivers/video/mmp/hw/mmp_ctrl.c
+++ b/drivers/video/mmp/hw/mmp_ctrl.c
@@ -142,17 +142,27 @@ static void dmafetch_set_fmt(struct mmp_overlay *overlay)
static void overlay_set_win(struct mmp_overlay *overlay, struct mmp_win *win)
{
struct lcd_regs *regs = path_regs(overlay->path);
- u32 pitch;
/* assert win supported */
memcpy(&overlay->win, win, sizeof(struct mmp_win));
mutex_lock(&overlay->access_ok);
- pitch = win->xsrc * pixfmt_to_stride(win->pix_fmt);
- writel_relaxed(pitch, ®s->g_pitch);
- writel_relaxed((win->ysrc << 16) | win->xsrc, ®s->g_size);
- writel_relaxed((win->ydst << 16) | win->xdst, ®s->g_size_z);
- writel_relaxed(0, ®s->g_start);
+
+ if (overlay_is_vid(overlay)) {
+ writel_relaxed(win->pitch[0], ®s->v_pitch_yc);
+ writel_relaxed(win->pitch[2] << 16 |
+ win->pitch[1], ®s->v_pitch_uv);
+
+ writel_relaxed((win->ysrc << 16) | win->xsrc, ®s->v_size);
+ writel_relaxed((win->ydst << 16) | win->xdst, ®s->v_size_z);
+ writel_relaxed(win->ypos << 16 | win->xpos, ®s->v_start);
+ } else {
+ writel_relaxed(win->pitch[0], ®s->g_pitch);
+
+ writel_relaxed((win->ysrc << 16) | win->xsrc, ®s->g_size);
+ writel_relaxed((win->ydst << 16) | win->xdst, ®s->g_size_z);
+ writel_relaxed(win->ypos << 16 | win->xpos, ®s->g_start);
+ }
dmafetch_set_fmt(overlay);
mutex_unlock(&overlay->access_ok);
@@ -234,7 +244,13 @@ static int overlay_set_addr(struct mmp_overlay *overlay, struct mmp_addr *addr)
/* FIXME: assert addr supported */
memcpy(&overlay->addr, addr, sizeof(struct mmp_addr));
- writel(addr->phys[0], ®s->g_0);
+
+ if (overlay_is_vid(overlay)) {
+ writel_relaxed(addr->phys[0], ®s->v_y0);
+ writel_relaxed(addr->phys[1], ®s->v_u0);
+ writel_relaxed(addr->phys[2], ®s->v_v0);
+ } else
+ writel_relaxed(addr->phys[0], ®s->g_0);
return overlay->addr.phys[0];
}
--
1.7.0.4
^ permalink raw reply related
* Re: cifs-utils VFS errors
From: steve @ 2013-06-09 8:45 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20130608202342.6e191950-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
On Sat, 2013-06-08 at 20:23 -0400, Jeff Layton wrote:
> On Sat, 08 Jun 2013 16:49:35 +0200
> steve <steve-dZ4O0aZtNmBWk0Htik3J/w@public.gmane.org> wrote:
> > > Hi
> > > Brilliant.
> > > I applied the patch, well, I edited cifs.upcall.c with the -'s and +'s
> > > at least. I then, make clean, build and make install. I now have:
> > > cifs.upcall
> > > Usage: cifs.upcall [ -d /path/to/keytab] [-k /path/to/krb5.conf] [-t]
> > > [-v] [-l] key_serial
> > >
> > > Looks good. Where do I put the -d in:
> > > mount -t cifs //altea/users /mnt -osec=krb5,multiuser,username=cifsuser
> > > or don't I?
> > > Cheers,
> > > Steve
> >
> > Here is /etc/request-key.conf:
> >
> > create cifs.spnego * * /usr/sbin/cifs.upcall -c %k
> >
> >
>
> Yes, you'll need to add the new argument there.
>
Hi
Here is the keytab:
klist -ke /etc/cifs.keytab
Keytab name: FILE:/etc/cifs.keytab
KVNO Principal
----
--------------------------------------------------------------------------
1 cifsuser-UiqEU/D402Y@public.gmane.org (arcfour-hmac)
create cifs.spnego * * /usr/sbin/cifs.upcall -d /etc/cifs.keytab -c %k
Unfortunately we are back to having to have a root cache in /tmp:
mount -t cifs //altea/shared /home/shared
-osec=krb5,multiuser,username=cifsuser
mount error(126): Required key not available
/var/log/messages for the failed key:
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)
2013-06-09T10:36:34.566409+02:00 catral cifs.upcall: user=cifsuser
2013-06-09T10:36:34.580279+02:00 catral cifs.upcall: pid=1396
2013-06-09T10:36:34.587159+02:00 catral cifs.upcall: find_krb5_cc:
scandir error on directory '/run/user/0': No such file or directory
2013-06-09T10:36:34.588382+02:00 catral cifs.upcall:
krb5_get_init_creds_keytab: -1765328174
2013-06-09T10:36:34.595349+02:00 catral cifs.upcall: handle_krb5_mech:
getting service ticket for altea
2013-06-09T10:36:34.596593+02:00 catral cifs.upcall: cifs_krb5_get_req:
unable to resolve (null) to ccache
2013-06-09T10:36:34.607253+02:00 catral cifs.upcall: handle_krb5_mech:
failed to obtain service ticket (-1765328245)
2013-06-09T10:36:34.608787+02:00 catral cifs.upcall: handle_krb5_mech:
getting service ticket for altea.hh3.site
2013-06-09T10:36:34.612720+02:00 catral cifs.upcall: cifs_krb5_get_req:
unable to resolve (null) to ccache
2013-06-09T10:36:34.614176+02:00 catral cifs.upcall: handle_krb5_mech:
failed to obtain service ticket (-1765328245)
2013-06-09T10:36:34.620231+02:00 catral cifs.upcall: Unable to obtain
service ticket
2013-06-09T10:36:34.621737+02:00 catral cifs.upcall: Exit status
-1765328245
If I now kinit cifsuser as root, it mounts fine:
kinit cifsuser
Password for cifsuser-UiqEU/D402Y@public.gmane.org:
catral:/home/steve # mount -t cifs //altea/shared /home/shared
-osec=krb5,multiuser,username=cifsuser
catral:/home/steve # mount | grep altea/shared
//altea/shared on /home/shared type cifs
(rw,relatime,vers=1.0,sec=krb5,cache=strict,unc=\\altea
\shared,multiuser,uid=0,noforceuid,gid=0,noforcegid,addr=192.168.1.100,unix,posixpaths,serverino,acl,noperm,rsize=1048576,wsize=65536,actimeo=1)
ticket
/var/log/messages for the successful mount:
2013-06-09T10:36:34.621737+02:00 catral cifs.upcall: Exit status
-1765328245
2013-06-09T10:40:06.705799+02:00 catral cifs.upcall: key description:
cifs.spnego;0;0;3f000000;ver=0x2;host=altea;ip4=192.168.1.100;sec=krb5;uid=0x0;creduid=0x0;user=cifsuser;pid=0x587
2013-06-09T10:40:06.710173+02:00 catral cifs.upcall: ver=2
2013-06-09T10:40:06.721488+02:00 catral cifs.upcall: host=altea
2013-06-09T10:40:06.725720+02:00 catral cifs.upcall: ip=192.168.1.100
2013-06-09T10:40:06.733396+02:00 catral cifs.upcall: sec=1
2013-06-09T10:40:06.742668+02:00 catral cifs.upcall: uid=0
2013-06-09T10:40:06.744518+02:00 catral cifs.upcall: creduid=0
2013-06-09T10:40:06.746116+02:00 catral cifs.upcall: user=cifsuser
2013-06-09T10:40:06.747900+02:00 catral cifs.upcall: pid=1415
2013-06-09T10:40:06.749599+02:00 catral cifs.upcall: find_krb5_cc:
scandir error on directory '/run/user/0': No such file or directory
2013-06-09T10:40:06.751559+02:00 catral cifs.upcall: find_krb5_cc:
considering /tmp/krb5cc_0
2013-06-09T10:40:06.755205+02:00 catral cifs.upcall: find_krb5_cc:
FILE:/tmp/krb5cc_0 is valid ccache
2013-06-09T10:40:06.756825+02:00 catral cifs.upcall: handle_krb5_mech:
getting service ticket for altea
2013-06-09T10:40:06.758426+02:00 catral cifs.upcall: handle_krb5_mech:
obtained service ticket
2013-06-09T10:40:06.760770+02:00 catral cifs.upcall: Exit status 0
It seems that cifs.upcall ignores /etc/reqestkey.conf Unless there is a
root cache, nothing gets mounted. I've tested without the patch and
having the key in the defaul keytab instead. The same.
This is nothing to do with the patch. cifs will not mount unless there
is a root cache available no matter which keytab is used: default keytab
or -d patch keytab.
Stuck.
^ permalink raw reply
* [V1 6/7] video: mmp: calculate pitch value when fb set win
From: Jett.Zhou @ 2013-06-09 8:44 UTC (permalink / raw)
To: linux-arm-kernel
From: Jing Xiang <jxiang@marvell.com>
Add new func mmpfb_set_win to make code clean, it will calculate pitch
value when fb set win in mmpfb_set_win.
Signed-off-by: Jing Xiang <jxiang@marvell.com>
Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
---
drivers/video/mmp/fb/mmpfb.c | 34 ++++++++++++++++++++++------------
1 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/video/mmp/fb/mmpfb.c b/drivers/video/mmp/fb/mmpfb.c
index 6d1fa96..d9e7c94 100644
--- a/drivers/video/mmp/fb/mmpfb.c
+++ b/drivers/video/mmp/fb/mmpfb.c
@@ -392,12 +392,29 @@ static int var_update(struct fb_info *info)
return 0;
}
+static void mmpfb_set_win(struct fb_info *info)
+{
+ struct mmpfb_info *fbi = info->par;
+ struct fb_var_screeninfo *var = &info->var;
+ struct mmp_win win;
+ u32 stride;
+
+ memset(&win, 0, sizeof(win));
+ win.xsrc = win.xdst = fbi->mode.xres;
+ win.ysrc = win.ydst = fbi->mode.yres;
+ win.pix_fmt = fbi->pix_fmt;
+ stride = pixfmt_to_stride(win.pix_fmt);
+ win.pitch[0] = var->xres_virtual * stride;
+ win.pitch[1] = win.pitch[2] =
+ (stride == 1) ? (var->xres_virtual >> 1) : 0;
+ mmp_overlay_set_win(fbi->overlay, &win);
+}
+
static int mmpfb_set_par(struct fb_info *info)
{
struct mmpfb_info *fbi = info->par;
struct fb_var_screeninfo *var = &info->var;
struct mmp_addr addr;
- struct mmp_win win;
struct mmp_mode mode;
int ret;
@@ -409,11 +426,8 @@ static int mmpfb_set_par(struct fb_info *info)
fbmode_to_mmpmode(&mode, &fbi->mode, fbi->output_fmt);
mmp_path_set_mode(fbi->path, &mode);
- memset(&win, 0, sizeof(win));
- win.xsrc = win.xdst = fbi->mode.xres;
- win.ysrc = win.ydst = fbi->mode.yres;
- win.pix_fmt = fbi->pix_fmt;
- mmp_overlay_set_win(fbi->overlay, &win);
+ /* set window related info */
+ mmpfb_set_win(info);
/* set address always */
memset(&addr, 0, sizeof(addr));
@@ -427,16 +441,12 @@ static int mmpfb_set_par(struct fb_info *info)
static void mmpfb_power(struct mmpfb_info *fbi, int power)
{
struct mmp_addr addr;
- struct mmp_win win;
struct fb_var_screeninfo *var = &fbi->fb_info->var;
/* for power on, always set address/window again */
if (power) {
- memset(&win, 0, sizeof(win));
- win.xsrc = win.xdst = fbi->mode.xres;
- win.ysrc = win.ydst = fbi->mode.yres;
- win.pix_fmt = fbi->pix_fmt;
- mmp_overlay_set_win(fbi->overlay, &win);
+ /* set window related info */
+ mmpfb_set_win(fbi->fb_info);
/* set address always */
memset(&addr, 0, sizeof(addr));
--
1.7.0.4
^ permalink raw reply related
* [V1 5/7] video: mmp: add pitch info in mmp_win structure
From: Jett.Zhou @ 2013-06-09 8:44 UTC (permalink / raw)
To: linux-arm-kernel
From: Jing Xiang <jxiang@marvell.com>
Add pitch length info of graphics/video layer for mmp_win, if it is
YUV format of video layer, u/v pitch will non-zero.
Signed-off-by: Jing Xiang <jxiang@marvell.com>
Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
---
include/video/mmp_disp.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/include/video/mmp_disp.h b/include/video/mmp_disp.h
index b9dd1fb..5708d26 100644
--- a/include/video/mmp_disp.h
+++ b/include/video/mmp_disp.h
@@ -91,6 +91,9 @@ struct mmp_win {
u16 up_crop;
u16 bottom_crop;
int pix_fmt;
+ /* pitch[0]: graphics/video layer line length or y pitch
+ * pitch[1]/pitch[2]: video u/v pitch if non-zero */
+ u32 pitch[3];
};
struct mmp_addr {
--
1.7.0.4
^ permalink raw reply related
* [V1 4/7] video: mmp: fix memcpy wrong size for mmp_addr issue
From: Jett.Zhou @ 2013-06-09 8:44 UTC (permalink / raw)
To: linux-arm-kernel
From: Jing Xiang <jxiang@marvell.com>
Memcpy used wrong struct of mmp_addr, fix it.
Signed-off-by: Jing Xiang <jxiang@marvell.com>
Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
---
drivers/video/mmp/hw/mmp_ctrl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c
index 8836053..92a5f44 100644
--- a/drivers/video/mmp/hw/mmp_ctrl.c
+++ b/drivers/video/mmp/hw/mmp_ctrl.c
@@ -233,7 +233,7 @@ static int overlay_set_addr(struct mmp_overlay *overlay, struct mmp_addr *addr)
struct lcd_regs *regs = path_regs(overlay->path);
/* FIXME: assert addr supported */
- memcpy(&overlay->addr, addr, sizeof(struct mmp_win));
+ memcpy(&overlay->addr, addr, sizeof(struct mmp_addr));
writel(addr->phys[0], ®s->g_0);
return overlay->addr.phys[0];
--
1.7.0.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.