* Re: [PATCH] [v2] arm64: fix unreachable code issue with cmpxchg
From: Nathan Chancellor @ 2019-09-17 20:34 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mark Rutland, Peter Zijlstra (Intel), Catalin Marinas,
Nick Desaulniers, linux-kernel, clang-built-linux, Andrew Murray,
Thomas Gleixner, Will Deacon, linux-arm-kernel
In-Reply-To: <20190910115643.391995-1-arnd@arndb.de>
On Tue, Sep 10, 2019 at 01:56:22PM +0200, Arnd Bergmann wrote:
> On arm64 build with clang, sometimes the __cmpxchg_mb is not inlined
> when CONFIG_OPTIMIZE_INLINING is set.
> Clang then fails a compile-time assertion, because it cannot tell at
> compile time what the size of the argument is:
>
> mm/memcontrol.o: In function `__cmpxchg_mb':
> memcontrol.c:(.text+0x1a4c): undefined reference to `__compiletime_assert_175'
> memcontrol.c:(.text+0x1a4c): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `__compiletime_assert_175'
>
> Mark all of the cmpxchg() style functions as __always_inline to
> ensure that the compiler can see the result.
>
> Acked-by: Nick Desaulniers <ndesaulniers@google.com>
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/648
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> Tested-by: Andrew Murray <andrew.murray@arm.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: skip unneeded changes, as suggested by Andrew Murray
> ---
> arch/arm64/include/asm/cmpxchg.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
> index a1398f2f9994..f9bef42c1411 100644
> --- a/arch/arm64/include/asm/cmpxchg.h
> +++ b/arch/arm64/include/asm/cmpxchg.h
> @@ -62,7 +62,7 @@ __XCHG_CASE( , , mb_, 64, dmb ish, nop, , a, l, "memory")
> #undef __XCHG_CASE
>
> #define __XCHG_GEN(sfx) \
> -static inline unsigned long __xchg##sfx(unsigned long x, \
> +static __always_inline unsigned long __xchg##sfx(unsigned long x, \
> volatile void *ptr, \
> int size) \
> { \
> @@ -148,7 +148,7 @@ __CMPXCHG_DBL(_mb)
> #undef __CMPXCHG_DBL
>
> #define __CMPXCHG_GEN(sfx) \
> -static inline unsigned long __cmpxchg##sfx(volatile void *ptr, \
> +static __always_inline unsigned long __cmpxchg##sfx(volatile void *ptr, \
> unsigned long old, \
> unsigned long new, \
> int size) \
> @@ -255,7 +255,7 @@ __CMPWAIT_CASE( , , 64);
> #undef __CMPWAIT_CASE
>
> #define __CMPWAIT_GEN(sfx) \
> -static inline void __cmpwait##sfx(volatile void *ptr, \
> +static __always_inline void __cmpwait##sfx(volatile void *ptr, \
> unsigned long val, \
> int size) \
> { \
> --
> 2.20.0
>
Looks like the arm64 pull request happened without this patch so clang
all{mod,yes}config builds are broken. Did the maintainers have any
further comments on it or could this make it in with the next one?
Cheers,
Nathan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 2/8] clk: Warn if clk_init_data is not zero initialized
From: Stephen Boyd @ 2019-09-17 20:38 UTC (permalink / raw)
To: Manivannan Sadhasivam, mturquette, robh+dt
Cc: devicetree, Manivannan Sadhasivam, darren.tsao, linux-kernel,
linux-arm-kernel, fisher.cheng, alec.lin, linux-clk, haitao.suo
In-Reply-To: <20190916161447.32715-3-manivannan.sadhasivam@linaro.org>
Quoting Manivannan Sadhasivam (2019-09-16 09:14:41)
> The new implementation for determining parent map uses multiple ways
> to pass parent info. The order in which it gets processed depends on
> the first available member. Hence, it is necessary to zero init the
> clk_init_data struct so that the expected member gets processed correctly.
> So, add a warning if multiple clk_init_data members are available during
> clk registration.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> drivers/clk/clk.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c0990703ce54..7d6d6984c979 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3497,6 +3497,14 @@ static int clk_core_populate_parent_map(struct clk_core *core)
> if (!num_parents)
> return 0;
>
> + /*
> + * Check for non-zero initialized clk_init_data struct. This is
> + * required because, we only require one of the (parent_names/
> + * parent_data/parent_hws) to be set at a time. Otherwise, the
> + * current code would use first available member.
> + */
> + WARN_ON((parent_names && parent_data) || (parent_names && parent_hws));
> +
This will warn for many drivers because they set clk_init_data on the
stack and assign parent_names but let junk from the stack be assigned to
parent_data. The code uses parent_names first and then looks for
parent_data or parent_hws because of this fact of life that we've never
required clk_init_data to be initialized to all zero.
> /*
> * Avoid unnecessary string look-ups of clk_core's possible parents by
> * having a cache of names/clk_hw pointers to clk_core pointers.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 1/8] clk: Zero init clk_init_data in helpers
From: Stephen Boyd @ 2019-09-17 20:39 UTC (permalink / raw)
To: Manivannan Sadhasivam, mturquette, robh+dt
Cc: devicetree, Manivannan Sadhasivam, darren.tsao, linux-kernel,
linux-arm-kernel, fisher.cheng, alec.lin, linux-clk, haitao.suo
In-Reply-To: <20190916161447.32715-2-manivannan.sadhasivam@linaro.org>
Quoting Manivannan Sadhasivam (2019-09-16 09:14:40)
> The clk_init_data struct needs to be initialized to zero for the new
> parent_map implementation to work correctly. Otherwise, the member which
> is available first will get processed.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> drivers/clk/clk-composite.c | 2 +-
> drivers/clk/clk-divider.c | 2 +-
> drivers/clk/clk-fixed-rate.c | 2 +-
> drivers/clk/clk-gate.c | 2 +-
> drivers/clk/clk-mux.c | 2 +-
> 5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> index b06038b8f658..4d579f9d20f6 100644
> --- a/drivers/clk/clk-composite.c
> +++ b/drivers/clk/clk-composite.c
> @@ -208,7 +208,7 @@ struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name,
> unsigned long flags)
> {
> struct clk_hw *hw;
> - struct clk_init_data init;
> + struct clk_init_data init = { NULL };
I'd prefer { } because then we don't have to worry about ordering the
struct to have a pointer vs. something else first.
> struct clk_composite *composite;
> struct clk_ops *clk_composite_ops;
> int ret;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 3/8] clk: Add clk_hw_unregister_composite helper function definition
From: Stephen Boyd @ 2019-09-17 20:40 UTC (permalink / raw)
To: Manivannan Sadhasivam, mturquette, robh+dt
Cc: devicetree, Manivannan Sadhasivam, darren.tsao, linux-kernel,
linux-arm-kernel, fisher.cheng, alec.lin, linux-clk, haitao.suo
In-Reply-To: <20190916161447.32715-4-manivannan.sadhasivam@linaro.org>
Quoting Manivannan Sadhasivam (2019-09-16 09:14:42)
> This function has been delcared but not defined anywhere. Hence, this
> commit adds definition for it.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
Can you add a fixes tag?
Fixes: 49cb392d3639 ("clk: composite: Add hw based registration APIs")
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V2, 1/2] media: i2c: dw9768: Add DT support and MAINTAINERS entry
From: Rob Herring @ 2019-09-17 20:47 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mark Rutland, devicetree, drinkcat, srv_heupstream, sam.hung,
shengnan.wang, Tomasz Figa, sj.huang, linux-mediatek,
dongchun.zhu, Sakari Ailus, Matthias Brugger, bingbu.cao,
Javier Martinez Canillas, Mauro Carvalho Chehab, linux-arm-kernel,
Linux Media Mailing List
In-Reply-To: <20190905122449.GG2680@smile.fi.intel.com>
On Thu, Sep 05, 2019 at 03:24:49PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 05, 2019 at 03:00:12PM +0300, Sakari Ailus wrote:
>
> > And I see no reason to add a separate patch just for
> > MAINTAINERS change.
>
> It's up to maintainers of the subsystem.
Just update MAINTAINERS with the driver. It doesn't need to be so
complicated or per subsystem. There's enough of what each maintainer
wants already.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v12 05/12] namei: obey trailing magic-link DAC permissions
From: Jann Horn @ 2019-09-17 21:30 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, kernel list, David Howells,
open list:KERNEL SELFTEST FRAMEWORK, sparclinux, Jiri Olsa,
linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai, Shuah Khan,
Alexander Shishkin, Ingo Molnar, linux-arm-kernel, linux-mips,
linux-xtensa, Kees Cook, Arnd Bergmann, linuxppc-dev, linux-m68k,
Al Viro, Andy Lutomirski, Shuah Khan, Namhyung Kim,
David Drysdale, Christian Brauner, J. Bruce Fields, linux-parisc,
Linux API, Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman,
linux-alpha, linux-fsdevel, Andrew Morton, Linus Torvalds,
Linux Containers
In-Reply-To: <20190904201933.10736-6-cyphar@cyphar.com>
On Wed, Sep 4, 2019 at 10:21 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> The ability for userspace to "re-open" file descriptors through
> /proc/self/fd has been a very useful tool for all sorts of usecases
> (container runtimes are one common example). However, the current
> interface for doing this has resulted in some pretty subtle security
> holes. Userspace can re-open a file descriptor with more permissions
> than the original, which can result in cases such as /proc/$pid/exe
> being re-opened O_RDWR at a later date even though (by definition)
> /proc/$pid/exe cannot be opened for writing. When combined with O_PATH
> the results can get even more confusing.
[...]
> Instead we have to restrict it in such a way that it doesn't break
> (good) users but does block potential attackers. The solution applied in
> this patch is to restrict *re-opening* (not resolution through)
> magic-links by requiring that mode of the link be obeyed. Normal
> symlinks have modes of a+rwx but magic-links have other modes. These
> magic-link modes were historically ignored during path resolution, but
> they've now been re-purposed for more useful ends.
Thanks for dealing with this issue!
[...]
> diff --git a/fs/namei.c b/fs/namei.c
> index 209c51a5226c..54d57dad0f91 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -872,7 +872,7 @@ void nd_jump_link(struct path *path)
>
> nd->path = *path;
> nd->inode = nd->path.dentry->d_inode;
> - nd->flags |= LOOKUP_JUMPED;
> + nd->flags |= LOOKUP_JUMPED | LOOKUP_MAGICLINK_JUMPED;
> }
[...]
> +static int trailing_magiclink(struct nameidata *nd, int acc_mode,
> + fmode_t *opath_mask)
> +{
> + struct inode *inode = nd->link_inode;
> + fmode_t upgrade_mask = 0;
> +
> + /* Was the trailing_symlink() a magic-link? */
> + if (!(nd->flags & LOOKUP_MAGICLINK_JUMPED))
> + return 0;
> +
> + /*
> + * Figure out the upgrade-mask of the link_inode. Since these aren't
> + * strictly POSIX semantics we don't do an acl_permission_check() here,
> + * so we only care that at least one bit is set for each upgrade-mode.
> + */
> + if (inode->i_mode & S_IRUGO)
> + upgrade_mask |= FMODE_PATH_READ;
> + if (inode->i_mode & S_IWUGO)
> + upgrade_mask |= FMODE_PATH_WRITE;
> + /* Restrict the O_PATH upgrade-mask of the caller. */
> + if (opath_mask)
> + *opath_mask &= upgrade_mask;
> + return may_open_magiclink(upgrade_mask, acc_mode);
> }
This looks racy because entries in the file descriptor table can be
switched out as long as task->files->file_lock isn't held. Unless I'm
missing something, something like the following (untested) would
bypass this restriction:
int readonly_fd = ...; /* some read-only fd we want to reopen as writable */
int writable_fd = open("/dev/null", O_RDWR);
int flippy_fd = dup(writable_fd);
char flippy_fd_path[100];
sprintf(flippy_fd_path, "/proc/%d/fd/%d", getpid(), flippy_fd);
if (fork() == 0) {
while (1) {
int reopened_fd = open(flippy_fd_path, O_RDWR);
if (reopened_fd == -1) continue;
char reopened_fd_path[100];
sprintf(reopened_fd_path, "/proc/self/fd/%d", reopened_fd);
char reopened_fd_target[1000];
int target_len = readlink(reopened_fd_path, reopened_fd_target,
sizeof(reopened_fd_target)-1);
reopened_fd_target[target_len] = 0;
if (strcmp(reopened_fd_target, "/dev/null"))
printf("managed to reopen as writable\n");
close(reopened_fd);
}
} else {
while (1) {
dup2(readonly_fd, flippy_fd);
dup2(writable_fd, flippy_fd);
}
}
Perhaps you could change nd_jump_link() to "void nd_jump_link(struct
path *path, umode_t link_mode)", and let proc_pid_get_link() pass the
link_mode through from an out-argument of .proc_get_link()? Then
proc_fd_link() could grab the proper mode in a race-free manner. And
nd_jump_link() could stash the mode in the nameidata.
A sketch of how I imagine that:
===============================
diff --git a/fs/namei.c b/fs/namei.c
index 6b936038319b..14c6790203c7 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -506,6 +506,7 @@ struct nameidata {
struct inode *link_inode;
unsigned root_seq;
int dfd;
+ umode_t last_link_mode;
} __randomize_layout;
static void set_nameidata(struct nameidata *p, int dfd, struct filename *name)
@@ -890,7 +891,7 @@ static int nd_jump_root(struct nameidata *nd)
* Helper to directly jump to a known parsed path from ->get_link,
* caller must have taken a reference to path beforehand.
*/
-void nd_jump_link(struct path *path)
+void nd_jump_link(struct path *path, umode_t link_mode)
{
struct nameidata *nd = current->nameidata;
path_put(&nd->path);
@@ -898,6 +899,7 @@ void nd_jump_link(struct path *path)
nd->path = *path;
nd->inode = nd->path.dentry->d_inode;
nd->flags |= LOOKUP_JUMPED | LOOKUP_MAGICLINK_JUMPED;
+ nd->last_link_mode = link_mode;
}
static inline void put_link(struct nameidata *nd)
@@ -3654,9 +3656,9 @@ static int trailing_magiclink(struct nameidata
*nd, int acc_mode,
* strictly POSIX semantics we don't do an acl_permission_check() here,
* so we only care that at least one bit is set for each upgrade-mode.
*/
- if (inode->i_mode & S_IRUGO)
+ if (nd->last_link_mode & S_IRUGO)
upgrade_mask |= FMODE_PATH_READ;
- if (inode->i_mode & S_IWUGO)
+ if (nd->last_link_mode & S_IWUGO)
upgrade_mask |= FMODE_PATH_WRITE;
/* Restrict the O_PATH upgrade-mask of the caller. */
if (opath_mask)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 297242174402..af0218447571 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1614,6 +1614,7 @@ static const char *proc_pid_get_link(struct
dentry *dentry,
{
struct path path;
int error = -EACCES;
+ umode_t link_mode;
if (!dentry)
return ERR_PTR(-ECHILD);
@@ -1622,11 +1623,11 @@ static const char *proc_pid_get_link(struct
dentry *dentry,
if (!proc_fd_access_allowed(inode))
goto out;
- error = PROC_I(inode)->op.proc_get_link(dentry, &path);
+ error = PROC_I(inode)->op.proc_get_link(dentry, &path, &link_mode);
if (error)
goto out;
- nd_jump_link(&path);
+ nd_jump_link(&path, link_mode);
return NULL;
out:
return ERR_PTR(error);
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 9b7d8becb002..9c1d247177b1 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -163,7 +163,8 @@ static const struct dentry_operations
tid_fd_dentry_operations = {
.d_delete = pid_delete_dentry,
};
-static int proc_fd_link(struct dentry *dentry, struct path *path)
+static int proc_fd_link(struct dentry *dentry, struct path *path,
+ umode_t *link_mode)
{
struct files_struct *files = NULL;
struct task_struct *task;
@@ -184,6 +185,7 @@ static int proc_fd_link(struct dentry *dentry,
struct path *path)
if (fd_file) {
*path = fd_file->f_path;
path_get(&fd_file->f_path);
+ *link_mode = /* something based on fd_file->f_mode */;
ret = 0;
}
spin_unlock(&files->file_lock);
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index cd0c8d5ce9a1..a090fff984ed 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -74,7 +74,7 @@ extern struct kmem_cache *proc_dir_entry_cache;
void pde_free(struct proc_dir_entry *pde);
union proc_op {
- int (*proc_get_link)(struct dentry *, struct path *);
+ int (*proc_get_link)(struct dentry *, struct path *, umode_t *);
int (*proc_show)(struct seq_file *m,
struct pid_namespace *ns, struct pid *pid,
struct task_struct *task);
===============================
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] ARM: dts: imx6dl: SolidRun: add phy node with 100Mb/s max-speed
From: Russell King - ARM Linux admin @ 2019-09-17 21:42 UTC (permalink / raw)
To: tinywrkb
Cc: Mark Rutland, Andrew Lunn, Baruch Siach,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Shawn Guo, Sascha Hauer, open list, Rob Herring, NXP Linux Team,
Pengutronix Kernel Team, Fabio Estevam, linux-arm-kernel
In-Reply-To: <20190917171913.GY25745@shell.armlinux.org.uk>
On Tue, Sep 17, 2019 at 06:19:13PM +0100, Russell King - ARM Linux admin wrote:
> whether you can get the link to come up at all. You might need to see
> whether wiggling the RJ45 helps (I've had that sort of thing with some
> cables.)
>
> You might also need "ethtool -s eth0 advertise ffcf" after trying that
> if it doesn't work to take the gigabit speeds out of the advertisement.
>
> Thanks.
>
> drivers/net/phy/at803x.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index b3893347804d..85cf4a4a5e81 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -296,6 +296,11 @@ static int at803x_config_init(struct phy_device *phydev)
> if (ret < 0)
> return ret;
>
> + /* Disable smartspeed */
> + ret = phy_modify(phydev, 0x14, BIT(5), 0);
> + if (ret < 0)
> + return ret;
> +
> /* The RX and TX delay default is:
> * after HW reset: RX delay enabled and TX delay disabled
> * after SW reset: RX delay enabled, while TX delay retains the
Hi,
Could you try this patch instead - it seems that the PHY needs to be
soft-reset for the write to take effect, and _even_ for the clearance
of the bit to become visible in the register.
I'm not expecting this on its own to solve anything, but it should at
least mean that the at803x doesn't modify the advertisement registers
itself. It may mean that the link doesn't even come up without forcing
the advertisement via the ethtool command I mentioned before.
Thanks.
drivers/net/phy/at803x.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index b3893347804d..69a58c0e6b42 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -296,6 +296,16 @@ static int at803x_config_init(struct phy_device *phydev)
if (ret < 0)
return ret;
+ /* Disable smartspeed */
+ ret = phy_modify(phydev, 0x14, BIT(5), 0);
+ if (ret < 0)
+ return ret;
+
+ /* Must soft-reset the PHY for smartspeed disable to take effect */
+ ret = genphy_soft_reset(phydev);
+ if (ret < 0)
+ return ret;
+
/* The RX and TX delay default is:
* after HW reset: RX delay enabled and TX delay disabled
* after SW reset: RX delay enabled, while TX delay retains the
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] ARM: dts: imx6dl: SolidRun: add phy node with 100Mb/s max-speed
From: Russell King - ARM Linux admin @ 2019-09-17 22:30 UTC (permalink / raw)
To: tinywrkb
Cc: Mark Rutland, Andrew Lunn, Baruch Siach,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Fabio Estevam, Sascha Hauer, open list, Rob Herring,
NXP Linux Team, Pengutronix Kernel Team, Shawn Guo,
linux-arm-kernel
In-Reply-To: <20190917133253.GA1210141@arch-dsk-01>
On Tue, Sep 17, 2019 at 04:32:53PM +0300, tinywrkb wrote:
> Here's the output of # mii-tool -v -v eth0
>
> * linux-test-5.1rc1-a2703de70942-without_bad_commit
>
> Using SIOCGMIIPHY=0x8947
> eth0: negotiated 100baseTx-FD flow-control, link ok
> registers for MII PHY 0:
> 3100 796d 004d d072 15e1 c5e1 000f 0000
> 0000 0000 0800 0000 0000 0000 0000 a000
> 0000 0000 0000 f420 082c 0000 04e8 0000
> 3200 3000 0000 063d 0000 0000 0000 0000
I'll also mention some other discrepencies that I've just spotted in
this register set.
The BMSR is 0x796d. Bit 2 is the link status, which is indicating
that link is up. Bit 5 indicates negotiation complete, which it
claims it is.
The PHY has a second status register at 0x11 which gives real time
information. That is 0x0000. Bit 10 indicates link up, and is
indicating that the link is down. Bit 11 is saying that the speed
and duplex is not resolved either.
So, there's contradictory information being reported by this PHY.
This brings up several questions:
1. what is the _true_ state of the link? Is the link up or down?
2. what does the link partner think is the current link state and
results of negotiation?
3. should we be reading the register at 0x11 to determine the
negotiation results and link state (maybe logically anding the
present state with the BMSR link state)?
Compare that to a correctly functioning AR8035 such as I have in my
cubox-i4 connected to a Netgear GS116 switch:
3100 796d 004d d072 15e1 c5e1 000d 2001
0000 0200 3c00 0000 0000 4007 b29a a000
0862 bc1c 0000 0000 082c 0000 07e8 0000
3200 3000 0000 063e 0000 0005 2d47 8100.
BMSR is again 0x796d. The PHY specific status register this time
is 0xbc1c, which indicates 1G, full duplex, resolved, link up, no
smartspeed downgrade, tx/rx pause.
The register at 0x10 is a control register, which is strangely also
different between our two. Apparently in your PHY configuration,
auto-MDI crossover mode is disabled, you are forced to MDI mode.
On hardware reset, this register contains 0x0862, as per my
example above, but yours is zero.
I don't think the difference in register 0x10 can be explained away
by operation of the smartspeed feature - so maybe my theory about
the advertisement registers being cleared by the PHY is wrong. The
question is: how is 0x10 getting reset to zero in your setup? Maybe
something has corrupted the configuration of the PHY in ways that
Linux doesn't know how to reprogram?
Have you tried power-cycling the cubox-i?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM: dts: imx6dl: SolidRun: add phy node with 100Mb/s max-speed
From: Russell King - ARM Linux admin @ 2019-09-17 22:43 UTC (permalink / raw)
To: tinywrkb
Cc: Mark Rutland, Andrew Lunn, Baruch Siach,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Shawn Guo, Sascha Hauer, open list, Rob Herring, NXP Linux Team,
Pengutronix Kernel Team, Fabio Estevam, linux-arm-kernel
In-Reply-To: <20190917223013.GC25745@shell.armlinux.org.uk>
On Tue, Sep 17, 2019 at 11:30:13PM +0100, Russell King - ARM Linux admin wrote:
> On Tue, Sep 17, 2019 at 04:32:53PM +0300, tinywrkb wrote:
> > Here's the output of # mii-tool -v -v eth0
> >
> > * linux-test-5.1rc1-a2703de70942-without_bad_commit
> >
> > Using SIOCGMIIPHY=0x8947
> > eth0: negotiated 100baseTx-FD flow-control, link ok
> > registers for MII PHY 0:
> > 3100 796d 004d d072 15e1 c5e1 000f 0000
> > 0000 0000 0800 0000 0000 0000 0000 a000
> > 0000 0000 0000 f420 082c 0000 04e8 0000
> > 3200 3000 0000 063d 0000 0000 0000 0000
>
> I'll also mention some other discrepencies that I've just spotted in
> this register set.
>
> The BMSR is 0x796d. Bit 2 is the link status, which is indicating
> that link is up. Bit 5 indicates negotiation complete, which it
> claims it is.
>
> The PHY has a second status register at 0x11 which gives real time
> information. That is 0x0000. Bit 10 indicates link up, and is
> indicating that the link is down. Bit 11 is saying that the speed
> and duplex is not resolved either.
>
> So, there's contradictory information being reported by this PHY.
>
> This brings up several questions:
> 1. what is the _true_ state of the link? Is the link up or down?
>
> 2. what does the link partner think is the current link state and
> results of negotiation?
>
> 3. should we be reading the register at 0x11 to determine the
> negotiation results and link state (maybe logically anding the
> present state with the BMSR link state)?
>
>
> Compare that to a correctly functioning AR8035 such as I have in my
> cubox-i4 connected to a Netgear GS116 switch:
>
> 3100 796d 004d d072 15e1 c5e1 000d 2001
> 0000 0200 3c00 0000 0000 4007 b29a a000
> 0862 bc1c 0000 0000 082c 0000 07e8 0000
> 3200 3000 0000 063e 0000 0005 2d47 8100.
>
> BMSR is again 0x796d. The PHY specific status register this time
> is 0xbc1c, which indicates 1G, full duplex, resolved, link up, no
> smartspeed downgrade, tx/rx pause.
>
> The register at 0x10 is a control register, which is strangely also
> different between our two. Apparently in your PHY configuration,
> auto-MDI crossover mode is disabled, you are forced to MDI mode.
> On hardware reset, this register contains 0x0862, as per my
> example above, but yours is zero.
>
> I don't think the difference in register 0x10 can be explained away
> by operation of the smartspeed feature - so maybe my theory about
> the advertisement registers being cleared by the PHY is wrong. The
> question is: how is 0x10 getting reset to zero in your setup? Maybe
> something has corrupted the configuration of the PHY in ways that
> Linux doesn't know how to reprogram?
>
> Have you tried power-cycling the cubox-i?
Hopefully one last thing, which will explain why you may not be able
to get an IP address even with some of these tweaks I've been getting
you to try. Do you have either none or both of these commits in your
kernel?
0672d22a1924 ("ARM: dts: imx: Fix the AR803X phy-mode")
6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode")
I think you'll have the latter but not the former. You will need the
former if you have the latter.
I think this thread is a good illustration why breaking existing DT
compatibility - even for the sake of fixing a bug - is just bad news.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 2/8] PM / devfreq: Don't fail devfreq_dev_release if not in list
From: Leonard Crestez @ 2019-09-18 0:18 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, Krzysztof Kozlowski, Chanwoo Choi, Alexandre Bailon,
Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <cover.1568764439.git.leonard.crestez@nxp.com>
Right now devfreq_dev_release will print a warning and abort the rest of
the cleanup if the devfreq instance is not part of the global
devfreq_list. But this is a valid scenario, for example it can happen if
the governor can't be found or on any other init error that happens
after device_register.
Initialize devfreq->node to an empty list head in devfreq_add_device so
that list_del becomes a safe noop inside devfreq_dev_release and we can
continue the rest of the cleanup.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/devfreq/devfreq.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 665575228c4f..a715f27f35fd 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -582,15 +582,10 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
static void devfreq_dev_release(struct device *dev)
{
struct devfreq *devfreq = to_devfreq(dev);
mutex_lock(&devfreq_list_lock);
- if (IS_ERR(find_device_devfreq(devfreq->dev.parent))) {
- mutex_unlock(&devfreq_list_lock);
- dev_warn(&devfreq->dev, "releasing devfreq which doesn't exist\n");
- return;
- }
list_del(&devfreq->node);
mutex_unlock(&devfreq_list_lock);
if (devfreq->profile->exit)
devfreq->profile->exit(devfreq->dev.parent);
@@ -641,10 +636,11 @@ struct devfreq *devfreq_add_device(struct device *dev,
mutex_init(&devfreq->lock);
mutex_lock(&devfreq->lock);
devfreq->dev.parent = dev;
devfreq->dev.class = devfreq_class;
devfreq->dev.release = devfreq_dev_release;
+ INIT_LIST_HEAD(&devfreq->node);
devfreq->profile = profile;
strncpy(devfreq->governor_name, governor_name, DEVFREQ_NAME_LEN);
devfreq->previous_freq = profile->initial_freq;
devfreq->last_status.current_frequency = profile->initial_freq;
devfreq->data = data;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/8] PM / devfreq: Lock devfreq in trans_stat_show
From: Leonard Crestez @ 2019-09-18 0:18 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, Krzysztof Kozlowski, Chanwoo Choi, Alexandre Bailon,
Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <cover.1568764439.git.leonard.crestez@nxp.com>
There is no locking in this sysfs show function so stats printing can
race with a devfreq_update_status called as part of freq switching or
with initialization.
Also add an assert in devfreq_update_status to make it clear that lock
must be held by caller.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/devfreq/devfreq.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 2494ee16f502..665575228c4f 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -159,10 +159,11 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
{
int lev, prev_lev, ret = 0;
unsigned long cur_time;
cur_time = jiffies;
+ lockdep_assert_held(&devfreq->lock);
/* Immediately exit if previous_freq is not initialized yet. */
if (!devfreq->previous_freq)
goto out;
@@ -1415,15 +1416,20 @@ static ssize_t trans_stat_show(struct device *dev,
struct devfreq *devfreq = to_devfreq(dev);
ssize_t len;
int i, j;
unsigned int max_state = devfreq->profile->max_state;
+ mutex_lock(&devfreq->lock);
if (!devfreq->stop_polling &&
- devfreq_update_status(devfreq, devfreq->previous_freq))
- return 0;
- if (max_state == 0)
- return sprintf(buf, "Not Supported.\n");
+ devfreq_update_status(devfreq, devfreq->previous_freq)) {
+ len = 0;
+ goto out;
+ }
+ if (max_state == 0) {
+ len = sprintf(buf, "Not Supported.\n");
+ goto out;
+ }
len = sprintf(buf, " From : To\n");
len += sprintf(buf + len, " :");
for (i = 0; i < max_state; i++)
len += sprintf(buf + len, "%10lu",
@@ -1447,10 +1453,13 @@ static ssize_t trans_stat_show(struct device *dev,
jiffies_to_msecs(devfreq->time_in_state[i]));
}
len += sprintf(buf + len, "Total transition : %u\n",
devfreq->total_trans);
+
+out:
+ mutex_unlock(&devfreq->lock);
return len;
}
static DEVICE_ATTR_RO(trans_stat);
static struct attribute *devfreq_attrs[] = {
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 4/8] PM / devfreq: Don't take lock in devfreq_add_device
From: Leonard Crestez @ 2019-09-18 0:18 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, Krzysztof Kozlowski, Chanwoo Choi, Alexandre Bailon,
Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <cover.1568764439.git.leonard.crestez@nxp.com>
A device usually doesn't need to lock itself during initialization
because it is not yet reachable from other threads.
This simplifies the code and helps avoid recursive lock warnings.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/devfreq/devfreq.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 57a217fc92de..860cbbab476c 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -634,11 +634,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
err = -ENOMEM;
goto err_out;
}
mutex_init(&devfreq->lock);
- mutex_lock(&devfreq->lock);
devfreq->dev.parent = dev;
devfreq->dev.class = devfreq_class;
devfreq->dev.release = devfreq_dev_release;
INIT_LIST_HEAD(&devfreq->node);
devfreq->profile = profile;
@@ -647,28 +646,24 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->last_status.current_frequency = profile->initial_freq;
devfreq->data = data;
devfreq->nb.notifier_call = devfreq_notifier_call;
if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
- mutex_unlock(&devfreq->lock);
err = set_freq_table(devfreq);
if (err < 0)
goto err_dev;
- mutex_lock(&devfreq->lock);
}
devfreq->scaling_min_freq = find_available_min_freq(devfreq);
if (!devfreq->scaling_min_freq) {
- mutex_unlock(&devfreq->lock);
err = -EINVAL;
goto err_dev;
}
devfreq->min_freq = devfreq->scaling_min_freq;
devfreq->scaling_max_freq = find_available_max_freq(devfreq);
if (!devfreq->scaling_max_freq) {
- mutex_unlock(&devfreq->lock);
err = -EINVAL;
goto err_dev;
}
devfreq->max_freq = devfreq->scaling_max_freq;
@@ -679,20 +674,18 @@ struct devfreq *devfreq_add_device(struct device *dev,
array3_size(sizeof(unsigned int),
devfreq->profile->max_state,
devfreq->profile->max_state),
GFP_KERNEL);
if (!devfreq->trans_table) {
- mutex_unlock(&devfreq->lock);
err = -ENOMEM;
goto err_dev;
}
devfreq->time_in_state = kcalloc(devfreq->profile->max_state,
sizeof(unsigned long),
GFP_KERNEL);
if (!devfreq->time_in_state) {
- mutex_unlock(&devfreq->lock);
err = -ENOMEM;
goto err_dev;
}
devfreq->last_stat_updated = jiffies;
@@ -701,17 +694,14 @@ struct devfreq *devfreq_add_device(struct device *dev,
dev_set_name(&devfreq->dev, "devfreq%d",
atomic_inc_return(&devfreq_no));
err = device_register(&devfreq->dev);
if (err) {
- mutex_unlock(&devfreq->lock);
put_device(&devfreq->dev);
goto err_out;
}
- mutex_unlock(&devfreq->lock);
-
mutex_lock(&devfreq_list_lock);
governor = try_then_request_governor(devfreq->governor_name);
if (IS_ERR(governor)) {
dev_err(dev, "%s: Unable to find governor for the device\n",
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 0/8] PM / devfreq: Add dev_pm_qos support
From: Leonard Crestez @ 2019-09-18 0:18 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, Krzysztof Kozlowski, Chanwoo Choi, Alexandre Bailon,
Georgi Djakov, linux-arm-kernel, Jacky Bai
Add dev_pm_qos notifiers to devfreq core in order to support frequency
limits via dev_pm_qos_add_request.
Unlike the rest of devfreq the dev_pm_qos frequency is measured in Khz,
this is consistent with current dev_pm_qos usage for cpufreq and
allows frequencies above 2Ghz (pm_qos expresses limits as s32).
Like with cpufreq the handling of min_freq/max_freq is moved to the
dev_pm_qos mechanism. Constraints from userspace are no longer clamped on
store, instead all values can be written and we only check against OPPs in a
new devfreq_get_freq_range function. This is consistent with the design of
dev_pm_qos.
Notifiers from pm_qos are executed under a single global dev_pm_qos_mtx and
need to take devfreq->lock. Notifier registration takes the same dev_pm_qos_mtx
so in order to prevent lockdep warnings it must be done outside devfreq->lock.
Current devfreq_add_device does *all* initialization under devfreq->lock
and that needs to be relaxed.
---
Changes since v4:
* Move more devfreq_add_device init ahead of device_register.
* Make devfreq_dev_release cleanup devices not yet in devfreq_list. This is
simpler than previous attempt to add to devfreq_list sonner.
* Take devfreq->lock in trans_stat_show
* Register dev_pm_opp notifier on devfreq parent dev (which has OPPs)
Link to v4: https://patchwork.kernel.org/cover/11114657/
Changes since v3:
* Cleanup locking and error-handling in devfreq_add_device
* Register notifiers after device registration but before governor start
* Keep the initialization of min_req/max_req ahead of device_register
because it's used for sysfs handling
* Use HZ_PER_KHZ instead of 1000
* Add kernel-doc comments
* Move OPP notifier to core
Link to v3: https://patchwork.kernel.org/cover/11104061/
Changes since v2:
* Handle sysfs via dev_pm_qos (in separate patch)
* Add locking to {min,max}_freq_show
* Fix checkpatch issues (long lines etc)
Link to v2: https://patchwork.kernel.org/patch/11084279/
Changes since v1:
* Add doxygen comments for min_nb/max_nb
* Remove notifiers on error/cleanup paths. Keep gotos simple by relying on
dev_pm_qos_remove_notifier ignoring notifiers which were not added.
Link to v1: https://patchwork.kernel.org/patch/11078475/
Leonard Crestez (8):
PM / devfreq: Lock devfreq in trans_stat_show
PM / devfreq: Don't fail devfreq_dev_release if not in list
PM / devfreq: Move more initialization before registration
PM / devfreq: Don't take lock in devfreq_add_device
PM / devfreq: Introduce devfreq_get_freq_range
PM / devfreq: Add dev_pm_qos support
PM / devfreq: Use dev_pm_qos for sysfs min/max_freq
PM / devfreq: Move opp notifier registration to core
drivers/devfreq/devfreq.c | 362 +++++++++++++++++++----------------
drivers/devfreq/exynos-bus.c | 7 -
drivers/devfreq/rk3399_dmc.c | 6 -
include/linux/devfreq.h | 22 +--
4 files changed, 204 insertions(+), 193 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 3/8] PM / devfreq: Move more initialization before registration
From: Leonard Crestez @ 2019-09-18 0:18 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, Krzysztof Kozlowski, Chanwoo Choi, Alexandre Bailon,
Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <cover.1568764439.git.leonard.crestez@nxp.com>
In general it is a better to initialize an object before making it
accessible externally (through device_register).
This make it possible to avoid relying on locking a partially
initialized object.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/devfreq/devfreq.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index a715f27f35fd..57a217fc92de 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -589,10 +589,12 @@ static void devfreq_dev_release(struct device *dev)
if (devfreq->profile->exit)
devfreq->profile->exit(devfreq->dev.parent);
mutex_destroy(&devfreq->lock);
+ kfree(devfreq->time_in_state);
+ kfree(devfreq->trans_table);
kfree(devfreq);
}
/**
* devfreq_add_device() - Add devfreq feature to the device
@@ -671,44 +673,43 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->max_freq = devfreq->scaling_max_freq;
devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
atomic_set(&devfreq->suspend_count, 0);
- dev_set_name(&devfreq->dev, "devfreq%d",
- atomic_inc_return(&devfreq_no));
- err = device_register(&devfreq->dev);
- if (err) {
- mutex_unlock(&devfreq->lock);
- put_device(&devfreq->dev);
- goto err_out;
- }
-
- devfreq->trans_table = devm_kzalloc(&devfreq->dev,
+ devfreq->trans_table = kzalloc(
array3_size(sizeof(unsigned int),
devfreq->profile->max_state,
devfreq->profile->max_state),
GFP_KERNEL);
if (!devfreq->trans_table) {
mutex_unlock(&devfreq->lock);
err = -ENOMEM;
- goto err_devfreq;
+ goto err_dev;
}
- devfreq->time_in_state = devm_kcalloc(&devfreq->dev,
- devfreq->profile->max_state,
- sizeof(unsigned long),
- GFP_KERNEL);
+ devfreq->time_in_state = kcalloc(devfreq->profile->max_state,
+ sizeof(unsigned long),
+ GFP_KERNEL);
if (!devfreq->time_in_state) {
mutex_unlock(&devfreq->lock);
err = -ENOMEM;
- goto err_devfreq;
+ goto err_dev;
}
devfreq->last_stat_updated = jiffies;
srcu_init_notifier_head(&devfreq->transition_notifier_list);
+ dev_set_name(&devfreq->dev, "devfreq%d",
+ atomic_inc_return(&devfreq_no));
+ err = device_register(&devfreq->dev);
+ if (err) {
+ mutex_unlock(&devfreq->lock);
+ put_device(&devfreq->dev);
+ goto err_out;
+ }
+
mutex_unlock(&devfreq->lock);
mutex_lock(&devfreq_list_lock);
governor = try_then_request_governor(devfreq->governor_name);
@@ -734,14 +735,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
return devfreq;
err_init:
mutex_unlock(&devfreq_list_lock);
-err_devfreq:
devfreq_remove_device(devfreq);
- devfreq = NULL;
+ return ERR_PTR(err);
err_dev:
+ kfree(devfreq->time_in_state);
+ kfree(devfreq->trans_table);
kfree(devfreq);
err_out:
return ERR_PTR(err);
}
EXPORT_SYMBOL(devfreq_add_device);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 5/8] PM / devfreq: Introduce devfreq_get_freq_range
From: Leonard Crestez @ 2019-09-18 0:18 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, Krzysztof Kozlowski, Chanwoo Choi, Alexandre Bailon,
Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <cover.1568764439.git.leonard.crestez@nxp.com>
Moving handling of min/max freq to a single function and call it from
update_devfreq and for printing min/max freq values in sysfs.
This changes the behavior of out-of-range min_freq/max_freq: clamping
is now done at evaluation time. This means that if an out-of-range
constraint is imposed by sysfs and it later becomes valid then it will
be enforced.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/devfreq/devfreq.c | 111 +++++++++++++++++++++-----------------
1 file changed, 63 insertions(+), 48 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 860cbbab476c..51a4179e2c69 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -24,10 +24,12 @@
#include <linux/printk.h>
#include <linux/hrtimer.h>
#include <linux/of.h>
#include "governor.h"
+#define HZ_PER_KHZ 1000
+
#define CREATE_TRACE_POINTS
#include <trace/events/devfreq.h>
static struct class *devfreq_class;
@@ -96,10 +98,50 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
dev_pm_opp_put(opp);
return max_freq;
}
+/**
+ * devfreq_get_freq_range() - Get the current freq range
+ * @devfreq: the devfreq instance
+ * @min_freq: the min frequency
+ * @max_freq: the max frequency
+ *
+ * This takes into consideration all constraints.
+ */
+static void devfreq_get_freq_range(struct devfreq *devfreq,
+ unsigned long *min_freq,
+ unsigned long *max_freq)
+{
+ unsigned long *freq_table = devfreq->profile->freq_table;
+
+ lockdep_assert_held(&devfreq->lock);
+
+ /* Init min/max frequency from freq table */
+ if (freq_table[0] < freq_table[devfreq->profile->max_state - 1]) {
+ *min_freq = freq_table[0];
+ *max_freq = freq_table[devfreq->profile->max_state - 1];
+ } else {
+ *min_freq = freq_table[devfreq->profile->max_state - 1];
+ *max_freq = freq_table[0];
+ }
+
+ /* constraints from sysfs: */
+ *min_freq = max(*min_freq, devfreq->min_freq);
+ *max_freq = min(*max_freq, devfreq->max_freq);
+
+ /* constraints from opp interface: */
+ *min_freq = max(*min_freq, devfreq->scaling_min_freq);
+ /* scaling_max_freq can be zero on error */
+ if (devfreq->scaling_max_freq)
+ *max_freq = min(*max_freq, devfreq->scaling_max_freq);
+
+ /* max_freq takes precedence over min_freq */
+ if (*min_freq > *max_freq)
+ *min_freq = *max_freq;
+}
+
/**
* devfreq_get_freq_level() - Lookup freq_table for the frequency
* @devfreq: the devfreq instance
* @freq: the target frequency
*/
@@ -349,21 +391,13 @@ int update_devfreq(struct devfreq *devfreq)
/* Reevaluate the proper frequency */
err = devfreq->governor->get_target_freq(devfreq, &freq);
if (err)
return err;
+ devfreq_get_freq_range(devfreq, &min_freq, &max_freq);
- /*
- * Adjust the frequency with user freq, QoS and available freq.
- *
- * List from the highest priority
- * max_freq
- * min_freq
- */
- max_freq = min(devfreq->scaling_max_freq, devfreq->max_freq);
- min_freq = max(devfreq->scaling_min_freq, devfreq->min_freq);
-
+ /* max freq takes priority over min freq */
if (freq < min_freq) {
freq = min_freq;
flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
}
if (freq > max_freq) {
@@ -1293,40 +1327,28 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
ret = sscanf(buf, "%lu", &value);
if (ret != 1)
return -EINVAL;
mutex_lock(&df->lock);
-
- if (value) {
- if (value > df->max_freq) {
- ret = -EINVAL;
- goto unlock;
- }
- } else {
- unsigned long *freq_table = df->profile->freq_table;
-
- /* Get minimum frequency according to sorting order */
- if (freq_table[0] < freq_table[df->profile->max_state - 1])
- value = freq_table[0];
- else
- value = freq_table[df->profile->max_state - 1];
- }
-
df->min_freq = value;
update_devfreq(df);
- ret = count;
-unlock:
mutex_unlock(&df->lock);
- return ret;
+
+ return count;
}
static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct devfreq *df = to_devfreq(dev);
+ unsigned long min_freq, max_freq;
- return sprintf(buf, "%lu\n", max(df->scaling_min_freq, df->min_freq));
+ mutex_lock(&df->lock);
+ devfreq_get_freq_range(df, &min_freq, &max_freq);
+ mutex_unlock(&df->lock);
+
+ return sprintf(buf, "%lu\n", min_freq);
}
static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
@@ -1338,40 +1360,33 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
if (ret != 1)
return -EINVAL;
mutex_lock(&df->lock);
- if (value) {
- if (value < df->min_freq) {
- ret = -EINVAL;
- goto unlock;
- }
- } else {
- unsigned long *freq_table = df->profile->freq_table;
-
- /* Get maximum frequency according to sorting order */
- if (freq_table[0] < freq_table[df->profile->max_state - 1])
- value = freq_table[df->profile->max_state - 1];
- else
- value = freq_table[0];
- }
+ /* Interpret zero as "don't care" */
+ if (!value)
+ value = ULONG_MAX;
df->max_freq = value;
update_devfreq(df);
- ret = count;
-unlock:
mutex_unlock(&df->lock);
- return ret;
+
+ return count;
}
static DEVICE_ATTR_RW(min_freq);
static ssize_t max_freq_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct devfreq *df = to_devfreq(dev);
+ unsigned long min_freq, max_freq;
+
+ mutex_lock(&df->lock);
+ devfreq_get_freq_range(df, &min_freq, &max_freq);
+ mutex_unlock(&df->lock);
- return sprintf(buf, "%lu\n", min(df->scaling_max_freq, df->max_freq));
+ return sprintf(buf, "%lu\n", max_freq);
}
static DEVICE_ATTR_RW(max_freq);
static ssize_t available_frequencies_show(struct device *d,
struct device_attribute *attr,
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 6/8] PM / devfreq: Add dev_pm_qos support
From: Leonard Crestez @ 2019-09-18 0:18 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, Krzysztof Kozlowski, Chanwoo Choi, Alexandre Bailon,
Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <cover.1568764439.git.leonard.crestez@nxp.com>
Register notifiers with the pm_qos framework in order to respond to
requests for MIN_FREQUENCY and MAX_FREQUENCY.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/devfreq/devfreq.c | 71 +++++++++++++++++++++++++++++++++++++++
include/linux/devfreq.h | 5 +++
2 files changed, 76 insertions(+)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 51a4179e2c69..d8d57318b12c 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -22,17 +22,20 @@
#include <linux/platform_device.h>
#include <linux/list.h>
#include <linux/printk.h>
#include <linux/hrtimer.h>
#include <linux/of.h>
+#include <linux/pm_qos.h>
#include "governor.h"
#define HZ_PER_KHZ 1000
#define CREATE_TRACE_POINTS
#include <trace/events/devfreq.h>
+#define HZ_PER_KHZ 1000
+
static struct class *devfreq_class;
/*
* devfreq core provides delayed work based load monitoring helper
* functions. Governors can use these or can implement their own
@@ -123,10 +126,16 @@ static void devfreq_get_freq_range(struct devfreq *devfreq,
} else {
*min_freq = freq_table[devfreq->profile->max_state - 1];
*max_freq = freq_table[0];
}
+ /* constraints from dev_pm_qos: */
+ *min_freq = max(*min_freq, HZ_PER_KHZ * (unsigned long)dev_pm_qos_read_value(
+ devfreq->dev.parent, DEV_PM_QOS_MIN_FREQUENCY));
+ *max_freq = min(*max_freq, HZ_PER_KHZ * (unsigned long)dev_pm_qos_read_value(
+ devfreq->dev.parent, DEV_PM_QOS_MAX_FREQUENCY));
+
/* constraints from sysfs: */
*min_freq = max(*min_freq, devfreq->min_freq);
*max_freq = min(*max_freq, devfreq->max_freq);
/* constraints from opp interface: */
@@ -605,10 +614,49 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
mutex_unlock(&devfreq->lock);
return ret;
}
+/**
+ * devfreq_qos_notifier_call() - Common handler for qos freq changes.
+ * @devfreq: the devfreq instance.
+ */
+static int devfreq_qos_notifier_call(struct devfreq *devfreq)
+{
+ int ret;
+
+ mutex_lock(&devfreq->lock);
+ ret = update_devfreq(devfreq);
+ mutex_unlock(&devfreq->lock);
+
+ return ret;
+}
+
+/**
+ * devfreq_qos_min_notifier_call() - Callback for qos min_freq changes.
+ * @nb: Should to be devfreq->nb_min
+ */
+static int devfreq_qos_min_notifier_call(struct notifier_block *nb,
+ unsigned long val, void *ptr)
+{
+ struct devfreq *devfreq = container_of(nb, struct devfreq, nb_min);
+
+ return devfreq_qos_notifier_call(devfreq);
+}
+
+/**
+ * devfreq_qos_max_notifier_call() - Callback for qos min_freq changes.
+ * @nb: Should to be devfreq->nb_max
+ */
+static int devfreq_qos_max_notifier_call(struct notifier_block *nb,
+ unsigned long val, void *ptr)
+{
+ struct devfreq *devfreq = container_of(nb, struct devfreq, nb_max);
+
+ return devfreq_qos_notifier_call(devfreq);
+}
+
/**
* devfreq_dev_release() - Callback for struct device to release the device.
* @dev: the devfreq device
*
* Remove devfreq from the list and release its resources.
@@ -619,10 +667,15 @@ static void devfreq_dev_release(struct device *dev)
mutex_lock(&devfreq_list_lock);
list_del(&devfreq->node);
mutex_unlock(&devfreq_list_lock);
+ dev_pm_qos_remove_notifier(devfreq->dev.parent, &devfreq->nb_max,
+ DEV_PM_QOS_MAX_FREQUENCY);
+ dev_pm_qos_remove_notifier(devfreq->dev.parent, &devfreq->nb_min,
+ DEV_PM_QOS_MIN_FREQUENCY);
+
if (devfreq->profile->exit)
devfreq->profile->exit(devfreq->dev.parent);
mutex_destroy(&devfreq->lock);
kfree(devfreq->time_in_state);
@@ -732,10 +785,27 @@ struct devfreq *devfreq_add_device(struct device *dev,
if (err) {
put_device(&devfreq->dev);
goto err_out;
}
+ /*
+ * Register notifiers for updates to min_freq/max_freq after device is
+ * initialized (and we can handle notifications) but before the governor
+ * is started (which should do an initial enforcement of constraints)
+ */
+ devfreq->nb_min.notifier_call = devfreq_qos_min_notifier_call;
+ err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_min,
+ DEV_PM_QOS_MIN_FREQUENCY);
+ if (err)
+ goto err_devfreq;
+
+ devfreq->nb_max.notifier_call = devfreq_qos_max_notifier_call;
+ err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_max,
+ DEV_PM_QOS_MAX_FREQUENCY);
+ if (err)
+ goto err_devfreq;
+
mutex_lock(&devfreq_list_lock);
governor = try_then_request_governor(devfreq->governor_name);
if (IS_ERR(governor)) {
dev_err(dev, "%s: Unable to find governor for the device\n",
@@ -759,10 +829,11 @@ struct devfreq *devfreq_add_device(struct device *dev,
return devfreq;
err_init:
mutex_unlock(&devfreq_list_lock);
+err_devfreq:
devfreq_remove_device(devfreq);
return ERR_PTR(err);
err_dev:
kfree(devfreq->time_in_state);
kfree(devfreq->trans_table);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index c3cbc15fdf08..dac0dffeabb4 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -134,10 +134,12 @@ struct devfreq_dev_profile {
* @total_trans: Number of devfreq transitions
* @trans_table: Statistics of devfreq transitions
* @time_in_state: Statistics of devfreq states
* @last_stat_updated: The last time stat updated
* @transition_notifier_list: list head of DEVFREQ_TRANSITION_NOTIFIER notifier
+ * @nb_min: Notifier block for DEV_PM_QOS_MIN_FREQUENCY
+ * @nb_max: Notifier block for DEV_PM_QOS_MAX_FREQUENCY
*
* This structure stores the devfreq information for a give device.
*
* Note that when a governor accesses entries in struct devfreq in its
* functions except for the context of callbacks defined in struct
@@ -176,10 +178,13 @@ struct devfreq {
unsigned int *trans_table;
unsigned long *time_in_state;
unsigned long last_stat_updated;
struct srcu_notifier_head transition_notifier_list;
+
+ struct notifier_block nb_min;
+ struct notifier_block nb_max;
};
struct devfreq_freqs {
unsigned long old;
unsigned long new;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 7/8] PM / devfreq: Use dev_pm_qos for sysfs min/max_freq
From: Leonard Crestez @ 2019-09-18 0:18 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, Krzysztof Kozlowski, Chanwoo Choi, Alexandre Bailon,
Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <cover.1568764439.git.leonard.crestez@nxp.com>
Switch the handling of min_freq and max_freq from sysfs to use the
dev_pm_qos interface.
Since dev_pm_qos handles frequencies as kHz this change reduces the
precision of min_freq and max_freq. This shouldn't introduce problems
because frequencies which are not an integer number of kHz are likely
not an integer number of Hz either.
Try to ensure compatibilitity by rounding min values down and rounding
max values up.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/devfreq/devfreq.c | 51 +++++++++++++++++++++++++--------------
include/linux/devfreq.h | 9 ++++---
2 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index d8d57318b12c..7977bad93949 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -132,14 +132,10 @@ static void devfreq_get_freq_range(struct devfreq *devfreq,
*min_freq = max(*min_freq, HZ_PER_KHZ * (unsigned long)dev_pm_qos_read_value(
devfreq->dev.parent, DEV_PM_QOS_MIN_FREQUENCY));
*max_freq = min(*max_freq, HZ_PER_KHZ * (unsigned long)dev_pm_qos_read_value(
devfreq->dev.parent, DEV_PM_QOS_MAX_FREQUENCY));
- /* constraints from sysfs: */
- *min_freq = max(*min_freq, devfreq->min_freq);
- *max_freq = min(*max_freq, devfreq->max_freq);
-
/* constraints from opp interface: */
*min_freq = max(*min_freq, devfreq->scaling_min_freq);
/* scaling_max_freq can be zero on error */
if (devfreq->scaling_max_freq)
*max_freq = min(*max_freq, devfreq->scaling_max_freq);
@@ -675,10 +671,12 @@ static void devfreq_dev_release(struct device *dev)
DEV_PM_QOS_MIN_FREQUENCY);
if (devfreq->profile->exit)
devfreq->profile->exit(devfreq->dev.parent);
+ dev_pm_qos_remove_request(&devfreq->max_freq_req);
+ dev_pm_qos_remove_request(&devfreq->min_freq_req);
mutex_destroy(&devfreq->lock);
kfree(devfreq->time_in_state);
kfree(devfreq->trans_table);
kfree(devfreq);
}
@@ -743,18 +741,28 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->scaling_min_freq = find_available_min_freq(devfreq);
if (!devfreq->scaling_min_freq) {
err = -EINVAL;
goto err_dev;
}
- devfreq->min_freq = devfreq->scaling_min_freq;
devfreq->scaling_max_freq = find_available_max_freq(devfreq);
if (!devfreq->scaling_max_freq) {
err = -EINVAL;
goto err_dev;
}
- devfreq->max_freq = devfreq->scaling_max_freq;
+
+ /* dev_pm_qos requests for min/max freq from sysfs */
+ err = dev_pm_qos_add_request(dev, &devfreq->min_freq_req,
+ DEV_PM_QOS_MIN_FREQUENCY, 0);
+ if (err < 0) {
+ goto err_dev;
+ }
+ err = dev_pm_qos_add_request(dev, &devfreq->max_freq_req,
+ DEV_PM_QOS_MAX_FREQUENCY, S32_MAX);
+ if (err < 0) {
+ goto err_dev;
+ }
devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
atomic_set(&devfreq->suspend_count, 0);
devfreq->trans_table = kzalloc(
@@ -833,10 +841,14 @@ struct devfreq *devfreq_add_device(struct device *dev,
mutex_unlock(&devfreq_list_lock);
err_devfreq:
devfreq_remove_device(devfreq);
return ERR_PTR(err);
err_dev:
+ if (dev_pm_qos_request_active(&devfreq->max_freq_req))
+ dev_pm_qos_remove_request(&devfreq->max_freq_req);
+ if (dev_pm_qos_request_active(&devfreq->min_freq_req))
+ dev_pm_qos_remove_request(&devfreq->min_freq_req);
kfree(devfreq->time_in_state);
kfree(devfreq->trans_table);
kfree(devfreq);
err_out:
return ERR_PTR(err);
@@ -1397,14 +1409,17 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
ret = sscanf(buf, "%lu", &value);
if (ret != 1)
return -EINVAL;
- mutex_lock(&df->lock);
- df->min_freq = value;
- update_devfreq(df);
- mutex_unlock(&df->lock);
+ /* round down to kHz for dev_pm_qos */
+ if (value)
+ value = value / HZ_PER_KHZ;
+
+ ret = dev_pm_qos_update_request(&df->min_freq_req, value);
+ if (ret < 0)
+ return ret;
return count;
}
static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
@@ -1429,19 +1444,19 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
ret = sscanf(buf, "%lu", &value);
if (ret != 1)
return -EINVAL;
- mutex_lock(&df->lock);
-
- /* Interpret zero as "don't care" */
- if (!value)
- value = ULONG_MAX;
+ /* round up to kHz for dev_pm_qos and interpret zero as "don't care" */
+ if (value)
+ value = DIV_ROUND_UP(value, HZ_PER_KHZ);
+ else
+ value = S32_MAX;
- df->max_freq = value;
- update_devfreq(df);
- mutex_unlock(&df->lock);
+ ret = dev_pm_qos_update_request(&df->max_freq_req, value);
+ if (ret < 0)
+ return ret;
return count;
}
static DEVICE_ATTR_RW(min_freq);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index dac0dffeabb4..4b5cc80abbe3 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -11,10 +11,11 @@
#define __LINUX_DEVFREQ_H__
#include <linux/device.h>
#include <linux/notifier.h>
#include <linux/pm_opp.h>
+#include <linux/pm_qos.h>
#define DEVFREQ_NAME_LEN 16
/* DEVFREQ governor name */
#define DEVFREQ_GOV_SIMPLE_ONDEMAND "simple_ondemand"
@@ -121,12 +122,12 @@ struct devfreq_dev_profile {
* devfreq.nb to the corresponding register notifier call chain.
* @work: delayed work for load monitoring.
* @previous_freq: previously configured frequency value.
* @data: Private data of the governor. The devfreq framework does not
* touch this.
- * @min_freq: Limit minimum frequency requested by user (0: none)
- * @max_freq: Limit maximum frequency requested by user (0: none)
+ * @min_freq_req: Limit minimum frequency requested by user (0: none)
+ * @max_freq_req: Limit maximum frequency requested by user (0: none)
* @scaling_min_freq: Limit minimum frequency requested by OPP interface
* @scaling_max_freq: Limit maximum frequency requested by OPP interface
* @stop_polling: devfreq polling status of a device.
* @suspend_freq: frequency of a device set during suspend phase.
* @resume_freq: frequency of a device set in resume phase.
@@ -161,12 +162,12 @@ struct devfreq {
unsigned long previous_freq;
struct devfreq_dev_status last_status;
void *data; /* private data for governors */
- unsigned long min_freq;
- unsigned long max_freq;
+ struct dev_pm_qos_request min_freq_req;
+ struct dev_pm_qos_request max_freq_req;
unsigned long scaling_min_freq;
unsigned long scaling_max_freq;
bool stop_polling;
unsigned long suspend_freq;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 8/8] PM / devfreq: Move opp notifier registration to core
From: Leonard Crestez @ 2019-09-18 0:18 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, Krzysztof Kozlowski, Chanwoo Choi, Alexandre Bailon,
Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <cover.1568764439.git.leonard.crestez@nxp.com>
An opp notifier can be registered by devfreq in order to respond to OPPs
being enabled or disabled at runtime (for example by thermal). This is
currently handled explicitly by drivers.
Move notifier handling to devfreq_add_device because this shouldn't be
hardware-specific.
Handling this inside the core also takes less code.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/devfreq/devfreq.c | 84 +++---------------------------------
drivers/devfreq/exynos-bus.c | 7 ---
drivers/devfreq/rk3399_dmc.c | 6 ---
include/linux/devfreq.h | 8 ----
4 files changed, 6 insertions(+), 99 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 7977bad93949..b9177430ae8f 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -663,10 +663,11 @@ static void devfreq_dev_release(struct device *dev)
mutex_lock(&devfreq_list_lock);
list_del(&devfreq->node);
mutex_unlock(&devfreq_list_lock);
+ dev_pm_opp_unregister_notifier(devfreq->dev.parent, &devfreq->nb);
dev_pm_qos_remove_notifier(devfreq->dev.parent, &devfreq->nb_max,
DEV_PM_QOS_MAX_FREQUENCY);
dev_pm_qos_remove_notifier(devfreq->dev.parent, &devfreq->nb_min,
DEV_PM_QOS_MIN_FREQUENCY);
@@ -728,11 +729,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->profile = profile;
strncpy(devfreq->governor_name, governor_name, DEVFREQ_NAME_LEN);
devfreq->previous_freq = profile->initial_freq;
devfreq->last_status.current_frequency = profile->initial_freq;
devfreq->data = data;
- devfreq->nb.notifier_call = devfreq_notifier_call;
if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
err = set_freq_table(devfreq);
if (err < 0)
goto err_dev;
@@ -810,10 +810,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_max,
DEV_PM_QOS_MAX_FREQUENCY);
if (err)
goto err_devfreq;
+ devfreq->nb.notifier_call = devfreq_notifier_call;
+ err = dev_pm_opp_register_notifier(devfreq->dev.parent, &devfreq->nb);
+ if (err)
+ goto err_devfreq;
+
mutex_lock(&devfreq_list_lock);
governor = try_then_request_governor(devfreq->governor_name);
if (IS_ERR(governor)) {
dev_err(dev, "%s: Unable to find governor for the device\n",
@@ -1624,87 +1629,10 @@ struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
return opp;
}
EXPORT_SYMBOL(devfreq_recommended_opp);
-/**
- * devfreq_register_opp_notifier() - Helper function to get devfreq notified
- * for any changes in the OPP availability
- * changes
- * @dev: The devfreq user device. (parent of devfreq)
- * @devfreq: The devfreq object.
- */
-int devfreq_register_opp_notifier(struct device *dev, struct devfreq *devfreq)
-{
- return dev_pm_opp_register_notifier(dev, &devfreq->nb);
-}
-EXPORT_SYMBOL(devfreq_register_opp_notifier);
-
-/**
- * devfreq_unregister_opp_notifier() - Helper function to stop getting devfreq
- * notified for any changes in the OPP
- * availability changes anymore.
- * @dev: The devfreq user device. (parent of devfreq)
- * @devfreq: The devfreq object.
- *
- * At exit() callback of devfreq_dev_profile, this must be included if
- * devfreq_recommended_opp is used.
- */
-int devfreq_unregister_opp_notifier(struct device *dev, struct devfreq *devfreq)
-{
- return dev_pm_opp_unregister_notifier(dev, &devfreq->nb);
-}
-EXPORT_SYMBOL(devfreq_unregister_opp_notifier);
-
-static void devm_devfreq_opp_release(struct device *dev, void *res)
-{
- devfreq_unregister_opp_notifier(dev, *(struct devfreq **)res);
-}
-
-/**
- * devm_devfreq_register_opp_notifier() - Resource-managed
- * devfreq_register_opp_notifier()
- * @dev: The devfreq user device. (parent of devfreq)
- * @devfreq: The devfreq object.
- */
-int devm_devfreq_register_opp_notifier(struct device *dev,
- struct devfreq *devfreq)
-{
- struct devfreq **ptr;
- int ret;
-
- ptr = devres_alloc(devm_devfreq_opp_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return -ENOMEM;
-
- ret = devfreq_register_opp_notifier(dev, devfreq);
- if (ret) {
- devres_free(ptr);
- return ret;
- }
-
- *ptr = devfreq;
- devres_add(dev, ptr);
-
- return 0;
-}
-EXPORT_SYMBOL(devm_devfreq_register_opp_notifier);
-
-/**
- * devm_devfreq_unregister_opp_notifier() - Resource-managed
- * devfreq_unregister_opp_notifier()
- * @dev: The devfreq user device. (parent of devfreq)
- * @devfreq: The devfreq object.
- */
-void devm_devfreq_unregister_opp_notifier(struct device *dev,
- struct devfreq *devfreq)
-{
- WARN_ON(devres_release(dev, devm_devfreq_opp_release,
- devm_devfreq_dev_match, devfreq));
-}
-EXPORT_SYMBOL(devm_devfreq_unregister_opp_notifier);
-
/**
* devfreq_register_notifier() - Register a driver with devfreq
* @devfreq: The devfreq object.
* @nb: The notifier block to register.
* @list: DEVFREQ_TRANSITION_NOTIFIER.
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index c832673273a2..29f422469960 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -356,17 +356,10 @@ static int exynos_bus_probe(struct platform_device *pdev)
dev_err(dev, "failed to add devfreq device\n");
ret = PTR_ERR(bus->devfreq);
goto err;
}
- /* Register opp_notifier to catch the change of OPP */
- ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq);
- if (ret < 0) {
- dev_err(dev, "failed to register opp notifier\n");
- goto err;
- }
-
/*
* Enable devfreq-event to get raw data which is used to determine
* current bus load.
*/
ret = exynos_bus_enable_edev(bus);
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 2e65d7279d79..f660d2031e8a 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -454,12 +454,10 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
if (IS_ERR(data->devfreq)) {
ret = PTR_ERR(data->devfreq);
goto err_free_opp;
}
- devm_devfreq_register_opp_notifier(dev, data->devfreq);
-
data->dev = dev;
platform_set_drvdata(pdev, data);
return 0;
@@ -470,14 +468,10 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
static int rk3399_dmcfreq_remove(struct platform_device *pdev)
{
struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(&pdev->dev);
- /*
- * Before remove the opp table we need to unregister the opp notifier.
- */
- devm_devfreq_unregister_opp_notifier(dmcfreq->dev, dmcfreq->devfreq);
dev_pm_opp_of_remove_table(dmcfreq->dev);
return 0;
}
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 4b5cc80abbe3..bf6ebfbc1e8a 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -220,18 +220,10 @@ extern void devfreq_resume(void);
extern int update_devfreq(struct devfreq *devfreq);
/* Helper functions for devfreq user device driver with OPP. */
extern struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
unsigned long *freq, u32 flags);
-extern int devfreq_register_opp_notifier(struct device *dev,
- struct devfreq *devfreq);
-extern int devfreq_unregister_opp_notifier(struct device *dev,
- struct devfreq *devfreq);
-extern int devm_devfreq_register_opp_notifier(struct device *dev,
- struct devfreq *devfreq);
-extern void devm_devfreq_unregister_opp_notifier(struct device *dev,
- struct devfreq *devfreq);
extern int devfreq_register_notifier(struct devfreq *devfreq,
struct notifier_block *nb,
unsigned int list);
extern int devfreq_unregister_notifier(struct devfreq *devfreq,
struct notifier_block *nb,
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v4 2/3] dt-bindings: rng: add bindings for MediaTek ARMv8 SoCs
From: Neal Liu @ 2019-09-18 1:44 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree@vger.kernel.org, Herbert Xu,
wsd_upstream, Sean Wang, linux-kernel@vger.kernel.org, Neal Liu,
Crystal Guo (郭晶), linux-crypto@vger.kernel.org,
Matt Mackall, Matthias Brugger,
linux-mediatek@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAL_Jsq+SRhd=-5O2G_CMfJX9Z188kvA05MQOXaU1J8iExwUixQ@mail.gmail.com>
On Tue, 2019-07-23 at 22:35 +0800, Rob Herring wrote:
> On Mon, Jul 22, 2019 at 8:21 PM Neal Liu <neal.liu@mediatek.com> wrote:
> >
>
> Please don't top post to lists.
>
> > Dear Rob,
> > You can check my driver for detail:
> > http://patchwork.kernel.org/patch/11012475/ or patchset 3/3
>
> I could, or you could just answer my question.
>
> >
> > This driver is registered as hardware random number generator, and
> > combines with rng-core.
> > We want to add one rng hw based on the dts. Is this proper or do you
> > have other suggestion to meet this requirement?
>
> It depends. There doesn't appear to be any resource configuration, so
> why does it need to be in DT. DT is not the only way instantiate
> drivers.
>
> Rob
>
We would like to consult more about this patch.
We cannot figure out what method should be used instead of DT.
The interface to access firmware is "smc" and firmware function only
exists on certain platforms.
Some DT has similar way, like:
http://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts#L470
firmware {
optee {
compatible = "linaro,optee-tz";
method = "smc";
};
};
Is there any way to instantiate driver on certain platforms without DT?
Could you give us some examples?
Thanks
> >
> > Thanks
> >
> >
> > On Tue, 2019-07-23 at 01:13 +0800, Rob Herring wrote:
> > > On Mon, Jun 24, 2019 at 03:24:11PM +0800, Neal Liu wrote:
> > > > Document the binding used by the MediaTek ARMv8 SoCs random
> > > > number generator with TrustZone enabled.
> > > >
> > > > Signed-off-by: Neal Liu <neal.liu@mediatek.com>
> > > > ---
> > > > .../devicetree/bindings/rng/mtk-sec-rng.txt | 10 ++++++++++
> > > > 1 file changed, 10 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/rng/mtk-sec-rng.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/rng/mtk-sec-rng.txt b/Documentation/devicetree/bindings/rng/mtk-sec-rng.txt
> > > > new file mode 100644
> > > > index 0000000..c04ce15
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/rng/mtk-sec-rng.txt
> > > > @@ -0,0 +1,10 @@
> > > > +MediaTek random number generator with TrustZone enabled
> > > > +
> > > > +Required properties:
> > > > +- compatible : Should be "mediatek,mtk-sec-rng"
> > >
> > > What's the interface to access this?
> > >
> > > A node with a 'compatible' and nothing else is a sign of something that
> > > a parent device should instantiate and doesn't need to be in DT. IOW,
> > > what do complete bindings for firmware functions look like?
> > >
> > > > +
> > > > +Example:
> > > > +
> > > > +hwrng: hwrng {
> > > > + compatible = "mediatek,mtk-sec-rng";
> > > > +}
> > > > --
> > > > 1.7.9.5
> > > >
> > >
> > > _______________________________________________
> > > Linux-mediatek mailing list
> > > Linux-mediatek@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-mediatek
> >
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL 1/5] ARM: SoC platform updates for v5.4
From: Linus Torvalds @ 2019-09-18 2:08 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: SoC Team, Linux ARM
In-Reply-To: <CAK8P3a1Xy7=o7KSawy6d7JkEm9VqT57vZ-zc45S85SXB7=pc+w@mail.gmail.com>
On Tue, Sep 17, 2019 at 3:03 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> There is a fix in the dmaengine tree, commit d17d9ea95727 ("dmaengine:
> iop-adma.c: fix printk format warning") from Randy Dunlap. I fixed
> some related issues in the same driver but missed this one because of
> a clang bug that is now fixed.
Ok, I just pulled the dmaengine tree, and re-enabled the
INTEL_IOP_ADMA driver since it now builds cleanly.
The lpc32xx_udc driver is still disabled for compile-testing.
Linus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH 3/6] Timer: expose monotonic clock and counter value
From: Jianyong Wu (Arm Technology China) @ 2019-09-18 2:50 UTC (permalink / raw)
To: Marc Zyngier, netdev@vger.kernel.org, pbonzini@redhat.com,
sean.j.christopherson@intel.com, richardcochran@gmail.com,
Mark Rutland, Will Deacon, Suzuki Poulose
Cc: Justin He (Arm Technology China), Steve Capper,
linux-kernel@vger.kernel.org, Kaly Xin (Arm Technology China), nd,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <ad38f692-a7c4-34e0-8236-ebd2d237bd93@kernel.org>
Hi Marc,
> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: Wednesday, September 18, 2019 1:10 AM
> To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@arm.com>;
> netdev@vger.kernel.org; pbonzini@redhat.com;
> sean.j.christopherson@intel.com; richardcochran@gmail.com; Mark Rutland
> <Mark.Rutland@arm.com>; Will Deacon <Will.Deacon@arm.com>; Suzuki
> Poulose <Suzuki.Poulose@arm.com>
> Cc: linux-kernel@vger.kernel.org; Steve Capper <Steve.Capper@arm.com>;
> Kaly Xin (Arm Technology China) <Kaly.Xin@arm.com>; Justin He (Arm
> Technology China) <Justin.He@arm.com>; nd <nd@arm.com>; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH 3/6] Timer: expose monotonic clock and counter value
>
> On 17/09/2019 12:24, Jianyong Wu wrote:
> > A number of PTP drivers (such as ptp-kvm) are assuming what the
> > current clock source is, which could lead to interesting effects on
> > systems where the clocksource can change depending on external events.
> >
> > For this purpose, add a new API that retrives both the current
> > monotonic clock as well as its counter value.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
>
> There must be something wrong with the way you've taken this patch in your
> tree. My authorship is gone (not that I deeply care about it, but it is good
> practice to keep attributions), and the subject line has been rewritten.
>
> I'd appreciate it if you could fix this in a future revision of this series. For
> reference, the original patch is here[1].
>
Sorry for "steal" your patch, I'm not familiar with it and neglect this important change.
I just copy this patch from your email and add the subject myself.
I will fix all of them later in v3.
> > ---
> > include/linux/timekeeping.h | 3 +++
> > kernel/time/timekeeping.c | 13 +++++++++++++
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
> > index a8ab0f143ac4..a5389adaa8bc 100644
> > --- a/include/linux/timekeeping.h
> > +++ b/include/linux/timekeeping.h
> > @@ -247,6 +247,9 @@ extern int get_device_system_crosststamp(
> > struct system_time_snapshot *history,
> > struct system_device_crosststamp *xtstamp);
> >
> > +/* Obtain current monotonic clock and its counter value */ extern
> > +void get_current_counterval(struct system_counterval_t *sc);
> > +
> > /*
> > * Simultaneously snapshot realtime and monotonic raw clocks
> > */
> > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> > index 44b726bab4bd..07a0969625b1 100644
> > --- a/kernel/time/timekeeping.c
> > +++ b/kernel/time/timekeeping.c
> > @@ -1098,6 +1098,19 @@ static bool cycle_between(u64 before, u64 test,
> u64 after)
> > return false;
> > }
> >
> > +/**
> > + * get_current_counterval - Snapshot the current clocksource and counter
> value
> > + * @sc: Pointer to a struct containing the current clocksource and its
> value
> > + */
> > +void get_current_counterval(struct system_counterval_t *sc) {
> > + struct timekeeper *tk = &tk_core.timekeeper;
> > +
> > + sc->cs = READ_ONCE(tk->tkr_mono.clock);
> > + sc->cycles = sc->cs->read(sc->cs);
> > +}
> > +EXPORT_SYMBOL_GPL(get_current_counterval);
>
> This export wasn't in my original patch. I guess you need it because your ptp
> driver builds as a module? It'd be good to mention it in the commit log.
>
Yeah, ptp_kvm will be a module, so export is necessary. I will mention this change
in commit log.
Thanks
Jianyong Wu
> > +
> > /**
> > * get_device_system_crosststamp - Synchronously capture
> system/device timestamp
> > * @get_time_fn: Callback to get simultaneous device time and
> >
>
> Thanks,
>
> M.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-
> platforms.git/commit/?h=timer/counterval&id=a6e8abce025691b6a55e1c195
> 878d7f76bfeb9d1
> --
> Jazz is not dead, it just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] arm:unwind: fix incorrect backtrace with unwind_table
From: chenzefeng @ 2019-09-18 3:09 UTC (permalink / raw)
To: linux, matthias.schiffer, tglx, info, gregkh
Cc: cj.chengjian, linux-kernel, chenzefeng2, nixiaoming,
linux-arm-kernel, liucheng32
For arm, if the CONFIG_ARM_UNWIND is open, when insmod a module,
the init section add to the unwind_table, the code path as follow:
load_module
--->post_relocation
------->module_finalize
----------->maps[ARM_SEC_INIT].txt_sec = s
----------->unwind_table_add
Later if load_module success, the init section's memory will be
vfree, the code path as follow:
load_module
--->do_init_module
------->freeinit->module_init = mod->init_layout.base
------->schedule_work(&init_free_wq)
----------->do_free_init
--------------->vfree(freeinit->module_init)
But after the init section's had been vfree, but it's unwind_table
is not removed.
The issue as follow:
When insmod module A, the system alloc the "Addr1" for it's init
text section, and add it to the unwind_table list, after insmod
success, the "Addr1" would be vfreed.
Unfortunately, later insmod module B, the system alloc the "Addr1"
for it's text section, and add it to the unwind_table list, too.
And we dumpstack in module B, we may get a incorrect backtrace.
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
---
arch/arm/kernel/module.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index deef17f..438ed67 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -410,7 +410,20 @@ int module_finalize(const Elf32_Ehdr *hdr, const Elf_Shdr *sechdrs,
int i;
for (i = 0; i < ARM_SEC_MAX; i++)
- if (mod->arch.unwind[i])
+ if (mod->arch.unwind[i]) {
unwind_table_del(mod->arch.unwind[i]);
+ mod->arch.unwind[i] = NULL;
+ }
+#endif
+}
+
+void
+module_arch_freeing_init(struct module *mod)
+{
+#ifdef CONFIG_ARM_UNWIND
+ if (mod->arch.unwind[ARM_SEC_INIT]) {
+ unwind_table_del(mod->arch.unwind[ARM_SEC_INIT]);
+ mod->arch.unwind[ARM_SEC_INIT] = NULL;
+ }
#endif
}
--
1.8.5.6
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* RE: [PATCH v3 09/11] PCI: layerscape: Add EP mode support for ls1088a and ls2088a
From: Xiaowei Bao @ 2019-09-18 3:17 UTC (permalink / raw)
To: Andrew Murray, robh+dt@kernel.org
Cc: mark.rutland@arm.com, Roy Zang, lorenzo.pieralisi@arm.com,
arnd@arndb.de, devicetree@vger.kernel.org, jingoohan1@gmail.com,
Z.q. Hou, linuxppc-dev@lists.ozlabs.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
kishon@ti.com, M.h. Lian, robh+dt@kernel.org,
gregkh@linuxfoundation.org, linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, Leo Li, shawnguo@kernel.org,
Mingkai Hu
In-Reply-To: <20190916143746.GU9720@e119886-lin.cambridge.arm.com>
> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年9月16日 22:38
> To: Xiaowei Bao <xiaowei.bao@nxp.com>; robh+dt@kernel.org
> Cc: robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org; Leo
> Li <leoyang.li@nxp.com>; kishon@ti.com; lorenzo.pieralisi@arm.com; M.h.
> Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy
> Zang <roy.zang@nxp.com>; jingoohan1@gmail.com;
> gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org;
> arnd@arndb.de; gregkh@linuxfoundation.org; Z.q. Hou
> <zhiqiang.hou@nxp.com>
> Subject: Re: [PATCH v3 09/11] PCI: layerscape: Add EP mode support for
> ls1088a and ls2088a
>
> On Sat, Sep 14, 2019 at 04:10:22AM +0000, Xiaowei Bao wrote:
> >
> >
> > > -----Original Message-----
> > > From: Andrew Murray <andrew.murray@arm.com>
> > > Sent: 2019年9月12日 20:50
> > > To: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > Cc: robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org;
> > > Leo Li <leoyang.li@nxp.com>; kishon@ti.com; lorenzo.pieralisi@arm.com;
> M.h.
> > > Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy
> > > Zang <roy.zang@nxp.com>; jingoohan1@gmail.com;
> > > gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org;
> > > arnd@arndb.de; gregkh@linuxfoundation.org; Z.q. Hou
> > > <zhiqiang.hou@nxp.com>
> > > Subject: Re: [PATCH v3 09/11] PCI: layerscape: Add EP mode support
> > > for ls1088a and ls2088a
> > >
> > > On Tue, Sep 03, 2019 at 01:47:36AM +0000, Xiaowei Bao wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Andrew Murray <andrew.murray@arm.com>
> > > > > Sent: 2019年9月2日 20:46
> > > > > To: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > > > Cc: robh+dt@kernel.org; mark.rutland@arm.com;
> > > > > shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> > > > > lorenzo.pieralisi@arm.com;
> > > M.h.
> > > > > Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> > > > > Roy Zang <roy.zang@nxp.com>; jingoohan1@gmail.com;
> > > > > gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
> > > > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > > linux-arm-kernel@lists.infradead.org;
> > > > > linuxppc-dev@lists.ozlabs.org; arnd@arndb.de;
> > > > > gregkh@linuxfoundation.org; Z.q. Hou <zhiqiang.hou@nxp.com>
> > > > > Subject: Re: [PATCH v3 09/11] PCI: layerscape: Add EP mode
> > > > > support for ls1088a and ls2088a
> > > > >
> > > > > On Mon, Sep 02, 2019 at 11:17:14AM +0800, Xiaowei Bao wrote:
> > > > > > Add PCIe EP mode support for ls1088a and ls2088a, there are
> > > > > > some difference between LS1 and LS2 platform, so refactor the
> > > > > > code of the EP driver.
> > > > > >
> > > > > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > > > > ---
> > > > > > v2:
> > > > > > - This is a new patch for supporting the ls1088a and ls2088a
> platform.
> > > > > > v3:
> > > > > > - Adjust the some struct assignment order in probe function.
> > > > > >
> > > > > > drivers/pci/controller/dwc/pci-layerscape-ep.c | 72
> > > > > > +++++++++++++++++++-------
> > > > > > 1 file changed, 53 insertions(+), 19 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > index 5f0cb99..723bbe5 100644
> > > > > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > @@ -20,27 +20,29 @@
> > > > > >
> > > > > > #define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
> > > > > >
> > > > > > -struct ls_pcie_ep {
> > > > > > - struct dw_pcie *pci;
> > > > > > - struct pci_epc_features *ls_epc;
> > > > > > +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > > > > > +
> > > > > > +struct ls_pcie_ep_drvdata {
> > > > > > + u32 func_offset;
> > > > > > + const struct dw_pcie_ep_ops *ops;
> > > > > > + const struct dw_pcie_ops *dw_pcie_ops;
> > > > > > };
> > > > > >
> > > > > > -#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > > > > > +struct ls_pcie_ep {
> > > > > > + struct dw_pcie *pci;
> > > > > > + struct pci_epc_features *ls_epc;
> > > > > > + const struct ls_pcie_ep_drvdata *drvdata; };
> > > > > >
> > > > > > static int ls_pcie_establish_link(struct dw_pcie *pci) {
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> > > > > > +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
> > > > > > .start_link = ls_pcie_establish_link, };
> > > > > >
> > > > > > -static const struct of_device_id ls_pcie_ep_of_match[] = {
> > > > > > - { .compatible = "fsl,ls-pcie-ep",},
> > > > > > - { },
> > > > > > -};
> > > > > > -
> > > > > > static const struct pci_epc_features*
> > > > > > ls_pcie_ep_get_features(struct dw_pcie_ep *ep) { @@ -87,10
> > > > > > +89,39 @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep,
> u8 func_no,
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > -static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > > > > > +static unsigned int ls_pcie_ep_func_conf_select(struct
> > > > > > +dw_pcie_ep
> > > *ep,
> > > > > > + u8 func_no)
> > > > > > +{
> > > > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > > > > > +
> > > > > > + WARN_ON(func_no && !pcie->drvdata->func_offset);
> > > > > > + return pcie->drvdata->func_offset * func_no; }
> > > > > > +
> > > > > > +static const struct dw_pcie_ep_ops ls_pcie_ep_ops = {
> > > > > > .ep_init = ls_pcie_ep_init,
> > > > > > .raise_irq = ls_pcie_ep_raise_irq,
> > > > > > .get_features = ls_pcie_ep_get_features,
> > > > > > + .func_conf_select = ls_pcie_ep_func_conf_select, };
> > > > > > +
> > > > > > +static const struct ls_pcie_ep_drvdata ls1_ep_drvdata = {
> > > > > > + .ops = &ls_pcie_ep_ops,
> > > > > > + .dw_pcie_ops = &dw_ls_pcie_ep_ops, };
> > > > > > +
> > > > > > +static const struct ls_pcie_ep_drvdata ls2_ep_drvdata = {
> > > > > > + .func_offset = 0x20000,
> > > > > > + .ops = &ls_pcie_ep_ops,
> > > > > > + .dw_pcie_ops = &dw_ls_pcie_ep_ops, };
> > > > > > +
> > > > > > +static const struct of_device_id ls_pcie_ep_of_match[] = {
> > > > > > + { .compatible = "fsl,ls1046a-pcie-ep", .data =
> &ls1_ep_drvdata },
> > > > > > + { .compatible = "fsl,ls1088a-pcie-ep", .data =
> &ls2_ep_drvdata },
> > > > > > + { .compatible = "fsl,ls2088a-pcie-ep", .data =
> &ls2_ep_drvdata },
> > > > > > + { },
> > > > >
> > > > > This removes support for "fsl,ls-pcie-ep" - was that
> > > > > intentional? If you do plan to drop it please make sure you
> > > > > explain why in the commit message. See also my comments in your
> dt-binding patch.
> > > >
> > > > In fact, the u-boot will fixup the status property to 'status =
> > > > enabled' in PCI node of the DTS base on "fsl,ls-pcie-ep"
> > > > compatible, so "fsl,ls-pcie-ep" is used, I used this compatible
> > > > before, because the driver only support the LS1046a, but this
> > > > time, I add the LS1088a and LS2088a support, and these two boards
> > > > have some difference form
> > > LS1046a, so I changed the compatible. I am not sure whether need to
> > > add "fsl,ls-pcie-ep"
> > > > in there, could you give some advice, thanks a lot.
> > >
> > > It sounds like "fsl,ls-pcie-ep" can be a fallback for "fsl,ls1046a-pcie-ep".
> >
> > This is not a fallback, the compatible "fsl,ls1046a-pcie-ep" is used
> > by bootloader, the bootloader will modify the status property, the
> > bootloader code get the
> > PCI_HEADER_TYPE(0xe) of config space to decide enable which node(EP or
> > RC) status property. At the beginning, we plan to use one compatible
> "fsl,ls1046a-pcie-ep"
> > support all NXP's platform, but actually, due to the difference of
> > each platform, it is difficult.
>
> I've looked at the U-Boot source [1] and device trees, I think I understand
> what happens here.
>
> The DT describes disabled nodes for both fsl,lsXXXXX-pcie and
> fsl,lxXXXXX-pcie-ep. U-Boot looks at the nodes and compares with the actual
> PCI config space to determine the current hardware configuration type. It will
> then *enable* either the RC or EP.
Yes, you are correct.
>
> However U-Boot currently only looks for a compatible string with "fsl,ls-pcie"
> or "fsl,ls-pcie-ep". This is why the DT needs to describe a PCI node as both
> "fsl,lsXXXXX-pcie-ep" and "fsl,ls-pcie" - the first for kernel and the second for
> the U-Boot. (The second is no longer needed by the kernel driver as you are
> now using the more specific names).
The fact is that the compatible "fsl,ls-pcie-ep" is used for match the driver at first,
and community of opensource advice reserve the compatible "fsl,lsXXXXX-pcie-ep"
for other platforms, at present, the compatible "fsl,ls-pcie-ep" is also used to
match the driver, and "fsl,lsXXXXX-pcie-ep" used to difference the platforms, please
refer to the drivers/pci/controller/dwc/pci-layerscape.c, it is similar.
>
> Looking again at your bindings patch...
>
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index e20ceaa..762ae41 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -22,7 +22,9 @@ Required properties:
> "fsl,ls1043a-pcie"
> "fsl,ls1012a-pcie"
> EP mode:
> - "fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep"
> + "fsl,ls1046a-pcie-ep" "fsl,ls-pcie-ep"
> + "fsl,ls1088a-pcie-ep" "fsl,ls-pcie-ep"
> + "fsl,ls2088a-pcie-ep" "fsl,ls-pcie-ep"
>
> ... the "fsl,ls-pcie-ep" is added *only* to the EP mode.
>
> But doesn't U-Boot need "fsl,ls-pcie-ep" added to each of the RC modes as
> well, to ensure they are set to enabled before booting the kernel?
>
> Rob - Do we document compatible names like this that are used in the DT but
> not used by the kernel?
>
> In any case, prior to this series it would have been possible to use a ls1046a
> device with a DT that has only string "fsl,ls-pcie-ep" - now that doesn't work.
> If this is of concern then &ls1_ep_drvdata should also be used for
> fsl,ls-pcie-ep.
>
> Thanks,
>
> Andrew Murray
>
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.
> denx.de%2Fu-boot%2Fu-boot%2Fblob%2Fmaster%2Fdrivers%2Fpci%2Fpcie_l
> ayerscape_fixup.c&data=02%7C01%7Cxiaowei.bao%40nxp.com%7Ccf3
> 3c1b76cb74f1cb41308d73ab37330%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5%7C0%7C1%7C637042414749368155&sdata=uVvfeT8AnOlPS%2ByBZ9
> Y4%2BHNJIA6MPx2sfWUBiD75IRY%3D&reserved=0
>
> >
> > >
> > > I'm assuming that if someone used "fsl,ls1046a-pcie-ep" on ls1088a
> > > or ls2088a hardware it would still work, but without the multiple PF
> support.
> > >
> >
> > I think the EP driver will not work if use current code, due to the
> > current driver need driver data.
> >
> > > I.e. if "fsl,ls-pcie-ep" is given, treat it as ls1046a.
> >
> >
> >
> > >
> > > Thanks,
> > >
> > > Andrew Murray
> > >
> > > >
> > > > Thanks
> > > > Xiaowei
> > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Andrew Murray
> > > > >
> > > > > > };
> > > > > >
> > > > > > static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie, @@
> > > > > > -103,7
> > > > > > +134,7 @@ static int __init ls_add_pcie_ep(struct ls_pcie_ep
> > > > > > +*pcie,
> > > > > > int ret;
> > > > > >
> > > > > > ep = &pci->ep;
> > > > > > - ep->ops = &pcie_ep_ops;
> > > > > > + ep->ops = pcie->drvdata->ops;
> > > > > >
> > > > > > res = platform_get_resource_byname(pdev,
> IORESOURCE_MEM,
> > > > > "addr_space");
> > > > > > if (!res)
> > > > > > @@ -142,20 +173,23 @@ static int __init
> > > > > > ls_pcie_ep_probe(struct
> > > > > platform_device *pdev)
> > > > > > if (!ls_epc)
> > > > > > return -ENOMEM;
> > > > > >
> > > > > > - dbi_base = platform_get_resource_byname(pdev,
> > > IORESOURCE_MEM,
> > > > > "regs");
> > > > > > - pci->dbi_base = devm_pci_remap_cfg_resource(dev,
> dbi_base);
> > > > > > - if (IS_ERR(pci->dbi_base))
> > > > > > - return PTR_ERR(pci->dbi_base);
> > > > > > + pcie->drvdata = of_device_get_match_data(dev);
> > > > > >
> > > > > > - pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> > > > > > pci->dev = dev;
> > > > > > - pci->ops = &ls_pcie_ep_ops;
> > > > > > - pcie->pci = pci;
> > > > > > + pci->ops = pcie->drvdata->dw_pcie_ops;
> > > > > >
> > > > > > ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > > > > >
> > > > > > + pcie->pci = pci;
> > > > > > pcie->ls_epc = ls_epc;
> > > > > >
> > > > > > + dbi_base = platform_get_resource_byname(pdev,
> > > IORESOURCE_MEM,
> > > > > "regs");
> > > > > > + pci->dbi_base = devm_pci_remap_cfg_resource(dev,
> dbi_base);
> > > > > > + if (IS_ERR(pci->dbi_base))
> > > > > > + return PTR_ERR(pci->dbi_base);
> > > > > > +
> > > > > > + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> > > > > > +
> > > > > > platform_set_drvdata(pdev, pcie);
> > > > > >
> > > > > > ret = ls_add_pcie_ep(pcie, pdev);
> > > > > > --
> > > > > > 2.9.5
> > > > > >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/6] Timer: expose monotonic clock and counter value
From: Richard Cochran @ 2019-09-18 3:42 UTC (permalink / raw)
To: Jianyong Wu
Cc: Mark.Rutland, justin.he, suzuki.poulose, netdev, Steve.Capper,
Will.Deacon, linux-kernel, sean.j.christopherson, Kaly.Xin, maz,
pbonzini, nd, linux-arm-kernel
In-Reply-To: <20190917112430.45680-4-jianyong.wu@arm.com>
On Tue, Sep 17, 2019 at 07:24:27AM -0400, Jianyong Wu wrote:
> A number of PTP drivers (such as ptp-kvm) are assuming what the
> current clock source is, which could lead to interesting effects on
> systems where the clocksource can change depending on external events.
>
> For this purpose, add a new API that retrives both the current
> monotonic clock as well as its counter value.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
> include/linux/timekeeping.h | 3 +++
> kernel/time/timekeeping.c | 13 +++++++++++++
> 2 files changed, 16 insertions(+)
For core time keeping changes, you must CC lkml, tglx, and John Stultz.
Thanks,
Richard
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v1] scsi: ufs: skip shutdown if hba is not powered
From: Stanley Chu @ 2019-09-18 4:20 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar,
pedrom.sousa, jejb
Cc: andy.teng, chun-hung.wu, kuohong.wang, linux-mediatek, peter.wang,
matthias.bgg, Stanley Chu, linux-arm-kernel, beanhuo
In some cases, hba may go through shutdown flow without successful
initialization and then make system hang.
For example, if ufshcd_change_power_mode() gets error and
leads to ufshcd_hba_exit() to release resources of the host,
future shutdown flow may hang the system since the host register
will be accessed in unpowered state.
To solve this issue, simply add checking to skip shutdown for above
kind of situation.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufshcd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 60a24fe908e7..a601ce0f6195 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8121,6 +8121,9 @@ int ufshcd_shutdown(struct ufs_hba *hba)
{
int ret = 0;
+ if (!hba->is_powered)
+ goto out;
+
if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
goto out;
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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