Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
On Wed, May 30, 2018 at 07:46:50AM -0700, Doug Anderson wrote: > On Wed, May 30, 2018 at 2:37 AM, Mark Brown wrote: > >> Linux vote for the lowest voltage it's comfortable with. Linux keeps > >> track of the true voltage that the driver wants and will always change > >> its vote back to that before enabling. Thus (assuming Linux is OK > >> with 1.2 V - 1.4 V for a rail): > > That's pretty much what it should do anyway with normally designed > > hardware. > I guess the question is: do we insist that the driver include this > workaround, or are we OK with letting the hardware behave as the > hardware does? What you're describing sounds like what we should be doing normally, if we're not doing that we should probably be fixing the core. signature.asc Description: PGP signature
Re: [PATCH 15/28] ovl: Open file with data except for the case of fsync
On Wed, May 30, 2018 at 4:30 PM, Vivek Goyal wrote:
> On Tue, May 29, 2018 at 04:45:59PM +0200, Miklos Szeredi wrote:
>> From: Vivek Goyal
>>
>> ovl_open() should open file which contains data and not open metacopy
>> inode. With the introduction of metacopy inodes, with current
>> implementaion we will end up opening metacopy inode as well.
>>
>> But there can be certain circumstances like ovl_fsync() where we want to
>> allow opening a metacopy inode instead.
>>
>> Hence, change ovl_open_realfile() and add _ovl_open_real() and add extra
>> parameter which specifies whether to allow opening metacopy inode or not.
>> If this parameter is false, we look for data inode and open that.
>>
>> This should allow covering both the cases.
>>
>> Signed-off-by: Vivek Goyal
>> Reviewed-by: Amir Goldstein
>> Signed-off-by: Miklos Szeredi
>> ---
>> fs/overlayfs/file.c | 39 ++-
>> 1 file changed, 30 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
>> index 266692ce9a9a..c7738ef492c8 100644
>> --- a/fs/overlayfs/file.c
>> +++ b/fs/overlayfs/file.c
>> @@ -14,11 +14,20 @@
>> #include
>> #include "overlayfs.h"
>>
>> -static struct file *ovl_open_realfile(const struct file *file)
>> +static char ovl_whatisit(struct inode *inode, struct inode *realinode)
>> +{
>> + if (realinode != ovl_inode_upper(inode))
>> + return 'l';
>> + if (ovl_has_upperdata(inode))
>> + return 'u';
>> + else
>> + return 'm';
>> +}
>> +
>> +static struct file *ovl_open_realfile(const struct file *file,
>> + struct inode *realinode)
>> {
>> struct inode *inode = file_inode(file);
>> - struct inode *upperinode = ovl_inode_upper(inode);
>> - struct inode *realinode = upperinode ?: ovl_inode_lower(inode);
>> struct file *realfile;
>> const struct cred *old_cred;
>>
>> @@ -28,7 +37,7 @@ static struct file *ovl_open_realfile(const struct file
>> *file)
>> revert_creds(old_cred);
>>
>> pr_debug("open(%p[%pD2/%c], 0%o) -> (%p, 0%o)\n",
>> - file, file, upperinode ? 'u' : 'l', file->f_flags,
>> + file, file, ovl_whatisit(inode, realinode), file->f_flags,
>>realfile, IS_ERR(realfile) ? 0 : realfile->f_flags);
>>
>> return realfile;
>> @@ -72,17 +81,24 @@ static int ovl_change_flags(struct file *file, unsigned
>> int flags)
>> return 0;
>> }
>>
>> -static int ovl_real_fdget(const struct file *file, struct fd *real)
>> +static int ovl_real_fdget_meta(const struct file *file, struct fd *real,
>> +bool allow_meta)
>> {
>> struct inode *inode = file_inode(file);
>> + struct inode *realinode;
>>
>> real->flags = 0;
>> real->file = file->private_data;
>>
>> + if (allow_meta)
>> + realinode = ovl_inode_real(inode);
>> + else
>> + realinode = ovl_inode_realdata(inode);
>> +
>> /* Has it been copied up since we'd opened it? */
>> - if (unlikely(file_inode(real->file) != ovl_inode_real(inode))) {
>> + if (unlikely(file_inode(real->file) != realinode)) {
>> real->flags = FDPUT_FPUT;
>> - real->file = ovl_open_realfile(file);
>> + real->file = ovl_open_realfile(file, realinode);
>>
>> return PTR_ERR_OR_ZERO(real->file);
>> }
>> @@ -94,6 +110,11 @@ static int ovl_real_fdget(const struct file *file,
>> struct fd *real)
>> return 0;
>> }
>>
>> +static int ovl_real_fdget(const struct file *file, struct fd *real)
>> +{
>> + return ovl_real_fdget_meta(file, real, false);
>> +}
>> +
>> static int ovl_open(struct inode *inode, struct file *file)
>> {
>> struct dentry *dentry = file_dentry(file);
>> @@ -107,7 +128,7 @@ static int ovl_open(struct inode *inode, struct file
>> *file)
>> /* No longer need these flags, so don't pass them on to underlying fs
>> */
>> file->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
>>
>> - realfile = ovl_open_realfile(file);
>> + realfile = ovl_open_realfile(file, ovl_inode_real(file_inode(file)));
That was meant to be
+ realfile = ovl_open_realfile(file, ovl_inode_realdata(file_inode(file)));
Is that correct?
> Hmm...so there have been some changes in this patch. My original intention
> was that to always open data inode (lower/upper) in ovl_open(). So if upper
> inode is a metacopy only, I will open lower inode instead.
>
> But new logic seems to be to always open real inode (that means upper
> metacopy inode as well). And that means that later when read happens
> on the file we will end up opening lower file, read data and close
> lower file.
>
> I am concerned a bit if there are performance implications to this.
> This will be hot path for containers.
Right. Unfortunately not detected with automatic testing...
Thanks for spotting!
Miklos
[PATCH v1 1/4] dt: bindings: tegra20-emc: Document interrupt property
EMC has a dedicated interrupt that is used to notify about completion of HW operations. Document the interrupt property. Signed-off-by: Dmitry Osipenko --- .../devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt| 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt index 4c33b29dc660..a6fe401d0d48 100644 --- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt @@ -10,6 +10,7 @@ Properties: and chosen using the ramcode board selector. If omitted, only one set of tables can be present and said tables will be used irrespective of ram-code configuration. +- interrupts : Should contain EMC General interrupt. Child device nodes describe the memory settings for different configurations and clock rates. @@ -20,6 +21,7 @@ Example: #size-cells = < 0 >; compatible = "nvidia,tegra20-emc"; reg = <0x7000f4000 0x200>; + interrupts = <0 78 0x04>; } -- 2.17.0
[PATCH] semaphore: use raw_spin_lock_irq instead of raw_spin_lock_irqsave
The sleeping functions down, down_interruptible, down_killable and
down_timeout can't be called with interrupts disabled, so we don't have to
save and restore interrupt flag.
This patch avoids the costly pushf and popf instructions on the semaphore
path.
Signed-off-by: Mikulas Patocka
---
kernel/locking/semaphore.c | 21 -
1 file changed, 8 insertions(+), 13 deletions(-)
Index: linux-2.6/kernel/locking/semaphore.c
===
--- linux-2.6.orig/kernel/locking/semaphore.c 2017-06-03 00:22:44.0
+0200
+++ linux-2.6/kernel/locking/semaphore.c2018-05-30 14:46:35.0
+0200
@@ -53,14 +53,12 @@ static noinline void __up(struct semapho
*/
void down(struct semaphore *sem)
{
- unsigned long flags;
-
- raw_spin_lock_irqsave(>lock, flags);
+ raw_spin_lock_irq(>lock);
if (likely(sem->count > 0))
sem->count--;
else
__down(sem);
- raw_spin_unlock_irqrestore(>lock, flags);
+ raw_spin_unlock_irq(>lock);
}
EXPORT_SYMBOL(down);
@@ -75,15 +73,14 @@ EXPORT_SYMBOL(down);
*/
int down_interruptible(struct semaphore *sem)
{
- unsigned long flags;
int result = 0;
- raw_spin_lock_irqsave(>lock, flags);
+ raw_spin_lock_irq(>lock);
if (likely(sem->count > 0))
sem->count--;
else
result = __down_interruptible(sem);
- raw_spin_unlock_irqrestore(>lock, flags);
+ raw_spin_unlock_irq(>lock);
return result;
}
@@ -101,15 +98,14 @@ EXPORT_SYMBOL(down_interruptible);
*/
int down_killable(struct semaphore *sem)
{
- unsigned long flags;
int result = 0;
- raw_spin_lock_irqsave(>lock, flags);
+ raw_spin_lock_irq(>lock);
if (likely(sem->count > 0))
sem->count--;
else
result = __down_killable(sem);
- raw_spin_unlock_irqrestore(>lock, flags);
+ raw_spin_unlock_irq(>lock);
return result;
}
@@ -155,15 +151,14 @@ EXPORT_SYMBOL(down_trylock);
*/
int down_timeout(struct semaphore *sem, long timeout)
{
- unsigned long flags;
int result = 0;
- raw_spin_lock_irqsave(>lock, flags);
+ raw_spin_lock_irq(>lock);
if (likely(sem->count > 0))
sem->count--;
else
result = __down_timeout(sem, timeout);
- raw_spin_unlock_irqrestore(>lock, flags);
+ raw_spin_unlock_irq(>lock);
return result;
}
Re: [PATCH 8/8] ima: Differentiate auditing policy rules from "audit" actions
On 05/30/2018 11:15 AM, Steve Grubb wrote: On Wednesday, May 30, 2018 9:54:00 AM EDT Stefan Berger wrote: On 05/29/2018 05:30 PM, Steve Grubb wrote: Hello, On Thursday, May 24, 2018 4:11:05 PM EDT Stefan Berger wrote: The AUDIT_INTEGRITY_RULE is used for auditing IMA policy rules and the IMA "audit" policy action. This patch defines AUDIT_INTEGRITY_POLICY_RULE to reflect the IMA policy rules. With this change we now call integrity_audit_msg_common() to get common integrity auditing fields. This now produces the following record when parsing an IMA policy rule: type=UNKNOWN[1806] msg=audit(1527004216.690:311): action=dont_measure \ fsmagic=0x9fa0 pid=1613 uid=0 auid=0 ses=2 \ subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 \ op=policy_update cause=parse_rule comm="echo" exe="/usr/bin/echo" \ tty=tty2 res=1 Since this is a new event, do you mind moving the tty field to be between auid= and ses= ? That is the more natural place for it. 6/8 refactors the code so that the integrity audit records produced by IMA follow one format in terms of ordering of the fields, with fields like inode optional, though, and AUDIT_INTEGRITY_RULE in the end being the only one with a different format. Do we really want to change that order just for 1806? 5/8 now produces the following: type=INTEGRITY_PCR msg=audit(1527685075.941:502): pid=2431 \ uid=0 auid=1000 ses=5 \ subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 \ op=invalid_pcr cause=open_writers comm="grep" \ name="/var/log/audit/audit.log" dev="dm-0" ino=1962494 \ exe="/usr/bin/grep" tty=pts0 res=1 Comparing the two: 1806: action, fsmagic, pid, uid, auid, ses, subj, op, cause, comm,exe, tty, res INTEGRITY_PCR: pid, uid, auid, ses, subj, op, cause, comm, name, dev, ino, exe, tty, res OK. I guess go with it as is. It passes testing. What about the position of 'res' field relative to the two new fields 'exe' and 'tty'? Do we want to keep them as shown or strictly append the two new fields 'exe' and 'tty'? Paul seems to request that they appear after 'res'. Stefan -Steve Also, it might be more natural for the op= and cause= fields to be before the pid= portion. This doesn't matter as much to me because those are not searchable fields and they are skipped right over. But moving the tty field is the main comment from me. With the refactoring in 6/8 we at least have consistency among the INTEGRITY_* records, with the only exception being AUDIT_INTEGRITY_RULE that has its own format: https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_a pi.c#L324 The other ones currently all format using integrity_audit_msg(). Thanks, -Steve Signed-off-by: Stefan Berger --- include/uapi/linux/audit.h | 3 ++- security/integrity/ima/ima_policy.c | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h index 4e61a9e05132..776e0abd35cf 100644 --- a/include/uapi/linux/audit.h +++ b/include/uapi/linux/audit.h @@ -146,7 +146,8 @@ #define AUDIT_INTEGRITY_STATUS 1802 /* Integrity enable status */ #define AUDIT_INTEGRITY_HASH 1803 /* Integrity HASH type */ #define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs */ -#define AUDIT_INTEGRITY_RULE 1805 /* policy rule */ +#define AUDIT_INTEGRITY_RULE 1805 /* IMA "audit" action policy msgs */ +#define AUDIT_INTEGRITY_POLICY_RULE 1806 /* IMA policy rules */ #define AUDIT_KERNEL 2000/* Asynchronous audit record. NOT A REQUEST. */ diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 3aed25a7178a..a8ae47a386b4 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -634,7 +634,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) int result = 0; ab = integrity_audit_log_start(NULL, GFP_KERNEL, - AUDIT_INTEGRITY_RULE); + AUDIT_INTEGRITY_POLICY_RULE); entry->uid = INVALID_UID; entry->fowner = INVALID_UID; @@ -926,7 +926,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) temp_ima_appraise |= IMA_APPRAISE_FIRMWARE; else if (entry->func == POLICY_CHECK) temp_ima_appraise |= IMA_APPRAISE_POLICY; - audit_log_format(ab, "res=%d", !result); + integrity_audit_msg_common(ab, NULL, NULL, + "policy_update", "parse_rule", result); audit_log_end(ab); return result; }
Re: [PATCH v2 1/6] ARM: dra762: hwmod: Add MCAN support
On 30/05/18 18:28, Tony Lindgren wrote:
* Tero Kristo [180530 15:18]:
For the OCP if part, I think that is still needed until we switch over to
full sysc driver. clkctrl_offs you probably also need because that is used
for mapping the omap_hwmod against a specific clkctrl clock. Those can be
only removed once we are done with hwmod (or figure out some other way to
assign the clkctrl clock to a hwmod.)
Hmm might be worth testing. I thought your commit 70f05be32133
("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
already parses the clkctrl from dts?
It maps the clkctrl clock to be used by hwmod, if those are available.
We didn't add any specific clock entries to DT for mapping the actual
clkctrl clock without the hwmod_data hints yet though, as that was
deemed temporary solution only due to transition to interconnect driver.
I.e., you would need something like this in DT for every device node:
{
clocks = ;
clock-names = "clkctrl";
};
... which is currently not present.
Alternatively you could rename the main_clk in the hwmod_data to point
towards the clkctrl clock, but again, that would be a temporary solution
only.
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH 15/28] ovl: Open file with data except for the case of fsync
On Wed, May 30, 2018 at 05:12:16PM +0200, Miklos Szeredi wrote:
> >> From: Vivek Goyal
> >>
> >> ovl_open() should open file which contains data and not open metacopy
> >> inode. With the introduction of metacopy inodes, with current
> >> implementaion we will end up opening metacopy inode as well.
> >>
> >> But there can be certain circumstances like ovl_fsync() where we want to
> >> allow opening a metacopy inode instead.
> >>
> >> Hence, change ovl_open_realfile() and add _ovl_open_real() and add extra
> >> parameter which specifies whether to allow opening metacopy inode or not.
> >> If this parameter is false, we look for data inode and open that.
> >>
> >> This should allow covering both the cases.
> >>
> >> Signed-off-by: Vivek Goyal
> >> Reviewed-by: Amir Goldstein
> >> Signed-off-by: Miklos Szeredi
> >> ---
> >> fs/overlayfs/file.c | 39 ++-
> >> 1 file changed, 30 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> >> index 266692ce9a9a..c7738ef492c8 100644
> >> --- a/fs/overlayfs/file.c
> >> +++ b/fs/overlayfs/file.c
> >> @@ -14,11 +14,20 @@
> >> #include
> >> #include "overlayfs.h"
> >>
> >> -static struct file *ovl_open_realfile(const struct file *file)
> >> +static char ovl_whatisit(struct inode *inode, struct inode *realinode)
> >> +{
> >> + if (realinode != ovl_inode_upper(inode))
> >> + return 'l';
> >> + if (ovl_has_upperdata(inode))
> >> + return 'u';
> >> + else
> >> + return 'm';
> >> +}
> >> +
> >> +static struct file *ovl_open_realfile(const struct file *file,
> >> + struct inode *realinode)
> >> {
> >> struct inode *inode = file_inode(file);
> >> - struct inode *upperinode = ovl_inode_upper(inode);
> >> - struct inode *realinode = upperinode ?: ovl_inode_lower(inode);
> >> struct file *realfile;
> >> const struct cred *old_cred;
> >>
> >> @@ -28,7 +37,7 @@ static struct file *ovl_open_realfile(const struct file
> >> *file)
> >> revert_creds(old_cred);
> >>
> >> pr_debug("open(%p[%pD2/%c], 0%o) -> (%p, 0%o)\n",
> >> - file, file, upperinode ? 'u' : 'l', file->f_flags,
> >> + file, file, ovl_whatisit(inode, realinode), file->f_flags,
> >>realfile, IS_ERR(realfile) ? 0 : realfile->f_flags);
> >>
> >> return realfile;
> >> @@ -72,17 +81,24 @@ static int ovl_change_flags(struct file *file,
> >> unsigned int flags)
> >> return 0;
> >> }
> >>
> >> -static int ovl_real_fdget(const struct file *file, struct fd *real)
> >> +static int ovl_real_fdget_meta(const struct file *file, struct fd *real,
> >> +bool allow_meta)
> >> {
> >> struct inode *inode = file_inode(file);
> >> + struct inode *realinode;
> >>
> >> real->flags = 0;
> >> real->file = file->private_data;
> >>
> >> + if (allow_meta)
> >> + realinode = ovl_inode_real(inode);
> >> + else
> >> + realinode = ovl_inode_realdata(inode);
> >> +
> >> /* Has it been copied up since we'd opened it? */
> >> - if (unlikely(file_inode(real->file) != ovl_inode_real(inode))) {
> >> + if (unlikely(file_inode(real->file) != realinode)) {
> >> real->flags = FDPUT_FPUT;
> >> - real->file = ovl_open_realfile(file);
> >> + real->file = ovl_open_realfile(file, realinode);
> >>
> >> return PTR_ERR_OR_ZERO(real->file);
> >> }
> >> @@ -94,6 +110,11 @@ static int ovl_real_fdget(const struct file *file,
> >> struct fd *real)
> >> return 0;
> >> }
> >>
> >> +static int ovl_real_fdget(const struct file *file, struct fd *real)
> >> +{
> >> + return ovl_real_fdget_meta(file, real, false);
> >> +}
> >> +
> >> static int ovl_open(struct inode *inode, struct file *file)
> >> {
> >> struct dentry *dentry = file_dentry(file);
> >> @@ -107,7 +128,7 @@ static int ovl_open(struct inode *inode, struct file
> >> *file)
> >> /* No longer need these flags, so don't pass them on to underlying
> >> fs */
> >> file->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
> >>
> >> - realfile = ovl_open_realfile(file);
> >> + realfile = ovl_open_realfile(file, ovl_inode_real(file_inode(file)));
>
> That was meant to be
>
> + realfile = ovl_open_realfile(file,
> ovl_inode_realdata(file_inode(file)));
>
> Is that correct?
Yes, that's correct.
While we are at it, can we also update patch changelog. I see that
_ovl_open_real() is not there anymore.
Thanks
Vivek
Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
On Wed, May 30, 2018 at 08:34:50AM -0700, Doug Anderson wrote: > On Wed, May 30, 2018 at 8:02 AM, Mark Brown wrote: > > What you're describing sounds like what we should be doing normally, if > > we're not doing that we should probably be fixing the core. > I'm not convinced that this behavior makes sense to move to the core. > On most regulators I'd expect that when the regulator driver says to > turn them off that they will output no voltage. The reason RPMh is Oh, you mean while the regulator is off... TBH I don't see a huge problem doing that anyway and just reverting to the bottom of the constraints when everything gets turned off since we have to see if we need to adjust the voltage every time we enabled anyway. > In any other system when Linux disabled the regulator it wouldn't > matter that you left it set at 1.4V. Thus RPMh is special and IMO the > workaround belongs there. Without the core doing something the regulator isn't going to get told that anything updated voltages anyway... signature.asc Description: PGP signature
Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
On Wed, May 30, 2018 at 09:09:02AM -0700, Doug Anderson wrote: > On Wed, May 30, 2018 at 9:07 AM, Mark Brown wrote: > > It needs something to tell it what the new voltage to set is. > The regulator driver has its own cache of what voltage was most > recently requested by Linux. It can use that, can't it? If we're just going to use the most recently set voltage then hopefully the hardware already knew that, and it might not be the lowest available voltage if the last consumer to get turned off was holding the voltage higher. signature.asc Description: PGP signature
Re: [PATCH][next] aio: fix missing break in switch statement
On Wed, May 30, 2018 at 07:04:15PM +0100, Colin King wrote:
> From: Colin Ian King
>
> The addition of the IOCB_CMD_POLL command removed the break
> statement for the IOCM_CMD_FDSYNC. From my understanding, this
> should not have been removed as the fall-through does not seem
> to make sense. Fix this by adding the break back again.
>
> Detected by CoverityScan, CID#1469469 ("Missing break in switch")
>
> Fixes: 2c14fa838cbe ("aio: implement IOCB_CMD_POLL")
> Signed-off-by: Colin Ian King
Makes sense to me...
Reviewed-by: Darrick J. Wong
--D
> ---
> fs/aio.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 8274d09d44a2..e0b2f183fa1c 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1785,6 +1785,7 @@ static int io_submit_one(struct kioctx *ctx, struct
> iocb __user *user_iocb,
> break;
> case IOCB_CMD_FDSYNC:
> ret = aio_fsync(>fsync, iocb, true);
> + break;
> case IOCB_CMD_POLL:
> ret = aio_poll(req, iocb);
> break;
> --
> 2.17.0
>
Re: LKMM litmus test for Roman Penyaev's rcu-rr
On Wed, May 30, 2018 at 09:59:28AM -0500, Linus Torvalds wrote: > On Wed, May 30, 2018 at 9:29 AM Alan Stern > wrote: > > > > > > > Can't we simplify the whole sequence as basically > > > > > > A > > > if (!B) > > > D > > > > > > for that "not B" case, and just think about that. IOW, let's ignore the > > > whole "not executed" code. > > > Your listing is slightly misleading. > > No. You're confused. > > You're confused because you're conflating two *entirely* different things. > > You're conflating the static source code with the dynamic execution. They > are NOT THE SAME. I am going to go out on a limb and assert that Linus is talking about what gcc and hardware do, while Alan is talking about what the tool and memory model do. In a perfect world, these would be the same, but it appears that the world might not be perfect just now. My current guess is that we need to change the memory-model tool. If that is the case, here are some possible resolutions: 1. Make herd's C-language control dependencies work the same as its assembly language, so that they extend beyond the end of the "if" statement. I believe that this would make Roman's case work, but it could claim that other situations are safe that are actually problematic due to compiler optimizations. The fact that the model currently handles only READ_ONCE() and WRITE_ONCE() and not unmarked reads and writes make this option more attractive than it otherwise be, compilers not being allowed to reorder volatile accesses, but we are likely to introduce unmarked accesses sometime in the future. 2. Like #1 above, but only if something in one of the "if"'s branches would prevent the compiler from reordering (smp_mb(), synchronize_rcu(), value-returning non-relaxed RMW atomic, ...). Easy for me to say, but I am guessing that much violence would be needed to the tooling to make this work. ;-) If I understand Alan correctly, there is not an obvious way to make this change within the confines of the memory model's .bell and .cat files. Thanx, Paul >It really should be: > > > A > > if (!B) > > ; // NOP > > D > > No it really really shouldn't. > > There are two completely different situations: > > (1) there is the source code: > > A > if (B) > C > D > > where C contains a barrier, and B depends on A and is not statically > determinable. > > In the source code, 'D' looks unconditional BUT IT DAMN WELL IS NOT. > > It's not unconditional - it's just done in both conditions! That's a big > big difference. > > > In other words, D should be beyond the end of the "if" statement, not > > inside one of the branches. > > You're just completely confused. > > What you are stating makes no sense at all. > > Seriously, your reading of the code is entirely monsenscal, and seems to be > about syntax, not about semantics. Which is crazy. > > Lookie here, you can change the syntactic model of that code to just be > > A > if (B) > C > D > else > D > > and that code obviously has the EXACT SAME SEMANTICS. > > So if you get hung up on trivial syntactic issues, you are by definition > confused, and your tool is garbage. You're doing memory ordering analysis, > not syntax parsing, for chrissake! > > > At run time, of course, it doesn't matter; > > CPUs don't try to detect where the two branches of an "if" recombine. > > (Leaving aside issues like implementing an "if" as a move-conditional.) > > You cannot do it as a move-conditional, since that code generation would > have been buggy shit, exactly because of C. But that's a code generation > issue, not a run-time decision. > > So at run-time, the code ends up being > > A > if (!B) > D > > and D cannot be written before A has been read, because B depends on A, and > you cannot expose specutive writes before the preconditions have been > evaluated. > > > Remember, the original code was: > > > A > > if (!B) > > C > > D > > > So the execution of D is _not_ conditional, and it doesn't depend on A > > or B. (Again, CPUs don't make this distinction, but compilers do.) > > Again, the above is nothing but confused bullshit. > > D depends on B, which depends on A. > > Really. Really really. > > Anybody - or any tool - that doesn't see that is fundamentally wrong, and > has been confused by syntax. > > A *compiler* will very much also make that distinction. If it doesn't make > that distinction, it's not a compiler, it's a buggy piece of shit. > > Because semantics matter. > > Think about it. > > Linus >
Re: [PATCH v2 4/6] bus: ti-sysc: Add support for using ti-sysc for MCAN on dra76x
* Faiz Abbas [180530 14:12]:
> The dra76x MCAN generic interconnect module has a its own
> format for the bits in the control registers.
...
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -1262,6 +1262,22 @@ static const struct sysc_capabilities
> sysc_omap4_usb_host_fs = {
> .regbits = _regbits_omap4_usb_host_fs,
> };
>
> +static const struct sysc_regbits sysc_regbits_dra7_mcan = {
> + .dmadisable_shift = -ENODEV,
> + .midle_shift = -ENODEV,
> + .sidle_shift = -ENODEV,
> + .clkact_shift = -ENODEV,
> + .enwkup_shift = 4,
> + .srst_shift = 0,
> + .emufree_shift = -ENODEV,
> + .autoidle_shift = -ENODEV,
> +};
> +
> +static const struct sysc_capabilities sysc_dra7_mcan = {
> + .type = TI_SYSC_DRA7_MCAN,
> + .regbits = _regbits_dra7_mcan,
> +};
> +
> static int sysc_init_pdata(struct sysc *ddata)
> {
> struct ti_sysc_platform_data *pdata = dev_get_platdata(ddata->dev);
> @@ -1441,6 +1457,7 @@ static const struct of_device_id sysc_match[] = {
> { .compatible = "ti,sysc-mcasp", .data = _omap4_mcasp, },
> { .compatible = "ti,sysc-usb-host-fs",
> .data = _omap4_usb_host_fs, },
> + { .compatible = "ti,sysc-dra7-mcan", .data = _dra7_mcan, },
> { },
> };
Looks good to me. And presumably you checked that no other dra7 modules
use sysconfig register bit layout like this?
Regards,
Tony
[PATCH v2 1/1] mmc: sdhci-pci-dwc-mshc: synopsys dwc mshc support
Synopsys has DWC MSHC controller on HPAS-DX platform connected using PCIe
interface with SD card slot and eMMC device slots. This patch is to
enable SD cards connected on this platform. As Clock generation logic
is implemented using MMCM module of HAPS-DX platform, we have separate
functions to control the MMCM to generate required clocks with respect
to speed mode.
Signed-off-by: Prabu Thangamuthu
---
V2 - Removed sdhci-pci-dwc-mshc.h and moved into sdhci-pci-dwc-mshc.c
Fixed coding style issue.
Removed sdhci_snps_set_power and new approach to support eMMC device
voltages will be submitted after completeing validations.
V1 - Initial Patch.
MAINTAINERS | 7 +++
drivers/mmc/host/Makefile | 3 +-
drivers/mmc/host/sdhci-pci-core.c | 1 +
drivers/mmc/host/sdhci-pci-dwc-mshc.c | 88 +++
drivers/mmc/host/sdhci-pci.h | 3 ++
5 files changed, 101 insertions(+), 1 deletion(-)
create mode 100644 drivers/mmc/host/sdhci-pci-dwc-mshc.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 4863175..aba98b6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12684,6 +12684,13 @@ S: Maintained
F: drivers/mmc/host/sdhci*
F: include/linux/mmc/sdhci*
+SYNOPSYS SDHCI COMPLIANT DWC MSHC DRIVER
+M: Prabu Thangamuthu
+M: Manjunath M B
+L: linux-...@vger.kernel.org
+S: Maintained
+F: drivers/mmc/host/sdhci-pci-dwc-mshc.c
+
SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) SAMSUNG DRIVER
M: Ben Dooks
M: Jaehoon Chung
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 85dc132..20490f3 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -11,7 +11,8 @@ obj-$(CONFIG_MMC_MXC) += mxcmmc.o
obj-$(CONFIG_MMC_MXS) += mxs-mmc.o
obj-$(CONFIG_MMC_SDHCI) += sdhci.o
obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o
-sdhci-pci-y += sdhci-pci-core.o sdhci-pci-o2micro.o sdhci-pci-arasan.o
+sdhci-pci-y += sdhci-pci-core.o sdhci-pci-o2micro.o sdhci-pci-arasan.o \
+ sdhci-pci-dwc-mshc.o
obj-$(subst m,y,$(CONFIG_MMC_SDHCI_PCI)) += sdhci-pci-data.o
obj-$(CONFIG_MMC_SDHCI_ACPI) += sdhci-acpi.o
obj-$(CONFIG_MMC_SDHCI_PXAV3) += sdhci-pxav3.o
diff --git a/drivers/mmc/host/sdhci-pci-core.c
b/drivers/mmc/host/sdhci-pci-core.c
index 77dd352..ca1d4f7 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -1511,6 +1511,7 @@ static int amd_probe(struct sdhci_pci_chip *chip)
SDHCI_PCI_DEVICE(O2, SEABIRD0, o2),
SDHCI_PCI_DEVICE(O2, SEABIRD1, o2),
SDHCI_PCI_DEVICE(ARASAN, PHY_EMMC, arasan),
+ SDHCI_PCI_DEVICE(SYNOPSYS, DWC_MSHC, snps),
SDHCI_PCI_DEVICE_CLASS(AMD, SYSTEM_SDHCI, PCI_CLASS_MASK, amd),
/* Generic SD host controller */
{PCI_DEVICE_CLASS(SYSTEM_SDHCI, PCI_CLASS_MASK)},
diff --git a/drivers/mmc/host/sdhci-pci-dwc-mshc.c
b/drivers/mmc/host/sdhci-pci-dwc-mshc.c
new file mode 100644
index 000..0706055
--- /dev/null
+++ b/drivers/mmc/host/sdhci-pci-dwc-mshc.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SDHCI driver for Synopsys DWC_MSHC controller
+ *
+ * Copyright (C) 2018 Synopsys, Inc. (www.synopsys.com)
+ *
+ * Authors:
+ * Prabu Thangamuthu
+ * Manjunath M B
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "sdhci.h"
+#include "sdhci-pci.h"
+
+#define SDHCI_VENDOR_PTR_R 0xE8
+
+/* Synopsys vendor specific registers */
+#define SDHC_GPIO_OUT 0x34
+#define SDHC_AT_CTRL_R 0x40
+#define SDHC_SW_TUNE_EN 0x0010
+
+/* MMCM DRP */
+#define SDHC_MMCM_DIV_REG 0x1020
+#define DIV_REG_100_MHZ 0x1145
+#define DIV_REG_200_MHZ 0x1083
+#define SDHC_MMCM_CLKFBOUT 0x1024
+#define CLKFBOUT_100_MHZ 0x
+#define CLKFBOUT_200_MHZ 0x0080
+#define SDHC_CCLK_MMCM_RST 0x0001
+
+static void sdhci_snps_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+ u16 clk;
+ u32 reg, vendor_ptr;
+
+ vendor_ptr = sdhci_readw(host, SDHCI_VENDOR_PTR_R);
+
+ /* Disable software managed rx tuning */
+ reg = sdhci_readl(host, (SDHC_AT_CTRL_R + vendor_ptr));
+ reg &= ~SDHC_SW_TUNE_EN;
+ sdhci_writel(host, reg, (SDHC_AT_CTRL_R + vendor_ptr));
+
+ if (clock <= 5200) {
+ sdhci_set_clock(host, clock);
+ } else {
+ /* Assert reset to MMCM */
+ reg = sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr));
+ reg |= SDHC_CCLK_MMCM_RST;
+ sdhci_writel(host, reg, (SDHC_GPIO_OUT + vendor_ptr));
+
+ /* Configure MMCM */
+ if (clock == 1) {
+ sdhci_writel(host, DIV_REG_100_MHZ, SDHC_MMCM_DIV_REG);
+ sdhci_writel(host, CLKFBOUT_100_MHZ,
+ SDHC_MMCM_CLKFBOUT);
+ } else {
+ sdhci_writel(host, DIV_REG_200_MHZ, SDHC_MMCM_DIV_REG);
+ sdhci_writel(host, CLKFBOUT_200_MHZ,
+ SDHC_MMCM_CLKFBOUT);
+ }
+
+ /* De-assert reset to MMCM */
+ reg = sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr));
+ reg &= ~SDHC_CCLK_MMCM_RST;
+ sdhci_writel(host, reg, (SDHC_GPIO_OUT + vendor_ptr));
+
+ /* Enable clock */
+ clk = SDHCI_PROG_CLOCK_MODE | SDHCI_CLOCK_INT_EN |
+ SDHCI_CLOCK_CARD_EN;
+ sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+ }
Re: [PATCH v2 6/6] ARM: dts: dra76x: Add MCAN node
* Faiz Abbas [180530 14:12]:
> From: Franklin S Cooper Jr
>
> Add support for the MCAN peripheral which supports both classic
> CAN messages along with the new CAN-FD message.
...
> --- a/arch/arm/boot/dts/dra76x.dtsi
> +++ b/arch/arm/boot/dts/dra76x.dtsi
> @@ -27,6 +27,21 @@
> ti,syss-mask = <1>;
> clocks = <_clkctrl DRA7_ADC_CLKCTRL 0>;
> clock-names = "fck";
> +
> + m_can0: mcan@42c01a00 {
> + compatible = "bosch,m_can";
> + reg = <0x1a00 0x4000>, <0x0 0x18FC>;
> + reg-names = "m_can", "message_ram";
> + interrupt-parent = <>;
> + interrupts = ,
> + ;
> + interrupt-names = "int0", "int1";
> + ti,hwmods = "mcan";
The "ti,hwmods" should be in the parent node now. But you may not even need
it, see the reset comment I made for the parent node patch.
> + clocks = <_clk>, <_iclk_div>;
> + clock-names = "cclk", "hclk";
> + bosch,mram-cfg = <0x0 0 0 32 0 0 1 1>;
> + status = "disabled";
And then you should be able to also leave out status = "disabled" as the
hardware is there for sure and we can idle it. Then a board specific
dts file can set it to "disabled" if reallly needed. Setting everything
manually to "disabled" and then again to "enabled" is the wrong way
around, the default is "enabled".
Regards,
Tony
Re: [PATCH v3 3/3] x86/mm: add TLB purge to free pmd/pte page interfaces
On Wed, 2018-05-30 at 06:59 +0200, j...@8bytes.org wrote: > On Tue, May 29, 2018 at 04:10:24PM +, Kani, Toshi wrote: > > Can you explain why you think allocating a page here is a major problem? > > Because a larger allocation is more likely to fail. And if you fail the > allocation, you also fail to free more pages, which _is_ a problem. So > better avoid any allocations in code paths that are about freeing > memory. It only allocates a single page. If it fails to allocate a single page, this pud mapping request simply falls to pmd mappings, which is only as bad as your suggested plain revert always does for both pud and pmd mappings. Also, this func is called from ioremap() when a driver initializes its mapping. If the system does not have a single page to allocate, the driver has a much bigger issue to deal with than its request falling back to pmd mappings. Please also note that this func is not called from free-memory path. > > If we just revert, please apply patch 1/3 first. This patch address the > > BUG_ON issue on PAE. This is a real issue that needs a fix ASAP. > > It does not address the problem of dirty page-walk caches on x86-64. This patch 3/3 fixes it. Hope my explanation above clarified. > > The page-directory cache issue on x64, which is addressed by patch 3/3, > > is a theoretical issue that I could not hit by putting ioremap() calls > > into a loop for a whole day. Nobody hit this issue, either. > > How do you know you didn't hit that issue? It might cause silent data > corruption, which might not be easily detected. If the test hit that issue, it would have caused a kernel page fault (freed & cleared pte page still unmapped, or this page reused and attribute data invalid) or MCE (pte page reused and phys addr invalid) when it accessed to ioremap'd address. Causing silent data corruption requires that this freed & cleared pte page to be reused and re-initialized with a valid pte entry data. While this case is possible, the chance of my test only hitting this case without ever hitting much more likely cases of page fault or MCE is basically zero. > > The simple revert patch Joerg posted a while ago causes > > pmd_free_pte_page() to fail on x64. This causes multiple pmd mappings > > to fall into pte mappings on my test systems. This can be seen as a > > degradation, and I am afraid that it is more harmful than good. > > The plain revert just removes all the issues with the dirty TLB that the > original patch introduced and prevents huge mappings from being > established when there have been smaller mappings before. This is not > ideal, but at least its is consistent and does not leak pages and leaves > no dirty TLBs. So this is the easiest and most reliable fix for this > stage in the release process. If you think the page directory issue needs a fix ASAP, I believe taking patch 3/3 is much better option than the plain revert, which will introduce the fall-back issue I explained. Thanks, -Toshi
Re: [PATCH] PCI/portdrv: Remove unused pcie_port_acpi_setup()
On Thu, May 10, 2018 at 04:46:15PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas
>
> 02bfeb484230 ("PCI/portdrv: Simplify PCIe feature permission checking")
> removed the only call of pcie_port_acpi_setup() and removed portdrv_acpi.o
> from the Makefile, but I forgot to remove pcie_port_acpi_setup() itself.
>
> Remove pcie_port_acpi_setup() and the drivers/pci/pcie/portdrv_acpi.c file.
>
> Signed-off-by: Bjorn Helgaas
I applied this to pci/portdrv for v4.18.
> ---
> drivers/pci/pcie/portdrv_acpi.c | 57
> ---
> 1 file changed, 57 deletions(-)
> delete mode 100644 drivers/pci/pcie/portdrv_acpi.c
>
> diff --git a/drivers/pci/pcie/portdrv_acpi.c b/drivers/pci/pcie/portdrv_acpi.c
> deleted file mode 100644
> index 8ab5d434b9c6..
> --- a/drivers/pci/pcie/portdrv_acpi.c
> +++ /dev/null
> @@ -1,57 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * PCIe Port Native Services Support, ACPI-Related Part
> - *
> - * Copyright (C) 2010 Rafael J. Wysocki , Novell Inc.
> - */
> -
> -#include
> -#include
> -#include
> -#include
> -#include
> -
> -#include "aer/aerdrv.h"
> -#include "../pci.h"
> -#include "portdrv.h"
> -
> -/**
> - * pcie_port_acpi_setup - Request the BIOS to release control of PCIe
> services.
> - * @port: PCIe Port service for a root port or event collector.
> - * @srv_mask: Bit mask of services that can be enabled for @port.
> - *
> - * Invoked when @port is identified as a PCIe port device. To avoid
> conflicts
> - * with the BIOS PCIe port native services support requires the BIOS to yield
> - * control of these services to the kernel. The mask of services that the
> BIOS
> - * allows to be enabled for @port is written to @srv_mask.
> - *
> - * NOTE: It turns out that we cannot do that for individual port services
> - * separately, because that would make some systems work incorrectly.
> - */
> -void pcie_port_acpi_setup(struct pci_dev *port, int *srv_mask)
> -{
> - struct acpi_pci_root *root;
> - acpi_handle handle;
> - u32 flags;
> -
> - if (acpi_pci_disabled)
> - return;
> -
> - handle = acpi_find_root_bridge_handle(port);
> - if (!handle)
> - return;
> -
> - root = acpi_pci_find_root(handle);
> - if (!root)
> - return;
> -
> - flags = root->osc_control_set;
> -
> - *srv_mask = 0;
> - if (flags & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)
> - *srv_mask |= PCIE_PORT_SERVICE_HP;
> - if (flags & OSC_PCI_EXPRESS_PME_CONTROL)
> - *srv_mask |= PCIE_PORT_SERVICE_PME;
> - if (flags & OSC_PCI_EXPRESS_AER_CONTROL)
> - *srv_mask |= PCIE_PORT_SERVICE_AER | PCIE_PORT_SERVICE_DPC;
> -}
>
Re: checkpatch.pl: Perl regression in "improve patch recognition"
On Wed, 2018-05-30 at 18:25 +0200, Charlemagne Lasse wrote:
> Since the commit 9dd0c31c6cc0 ("checkpatch: improve patch
> recognition"), the checkpatch.pl in linux-next is only printing a lot
> of error messages when started (with and without arguments):
https://lkml.org/lkml/2018/5/29/932
Re: [PATCH v2 1/6] soc: qcom: rpmpd: Add a powerdomain driver to model corners
Hello Rajendra, On 05/30/2018 03:14 AM, Rajendra Nayak wrote: > On 05/30/2018 02:47 PM, Ulf Hansson wrote: >> On 25 May 2018 at 12:01, Rajendra Nayak wrote: ... >>> + pm_genpd_init([i]->pd, NULL, true); >> >> Question: Is there no hierarchical topology of the PM domains. No >> genpd subdomains? > > The hierarchy if any is all handled by the remote core (RPM in this case). > For Linux its just a flat view. There is one special case that we'll need to handle somehow. The APPS vlvl request for VDD_MX needs to be greater than or equal to the vlvl request for VDD_CX. Can you please add the necessary code to achieve this? RPMh hardware doesn't handle this hardware requirement due to concerns about modem use case latency. Please note that this is handled in a somewhat hacky manner [1] with the downstream rpmh-regulator driver by specifying VDD_MX as the parent of VDD_CX and VDD_MX_AO as the parent of VDD_CX_AO with a dropout voltage of -1. That way, enabling CX causes MX to be enabled and voltage level requests are propagated from CX to MX (the -1 is ignored because it is rounded up within the sparse vlvl numbering space). Thanks, David [1]: https://source.codeaurora.org/quic/la/kernel/msm-4.9/tree/arch/arm64/boot/dts/qcom/sdm845-regulator.dtsi?h=msm-4.9#n135 -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 2/2] fs, elf: drop MAP_FIXED usage from elf_map
On 05/30/2018 01:02 AM, Michal Hocko wrote: > On Tue 29-05-18 15:21:14, Mike Kravetz wrote: >> Just a quick heads up. I noticed a change in libhugetlbfs testing starting >> with v4.17-rc1. >> >> V4.16 libhugetlbfs test results >> ** TEST SUMMARY >> * 2M >> * 32-bit 64-bit >> * Total testcases: 110113 >> * Skipped: 0 0 >> *PASS: 105111 >> *FAIL: 0 0 >> *Killed by signal: 4 1 >> * Bad configuration: 1 1 >> * Expected FAIL: 0 0 >> * Unexpected PASS: 0 0 >> *Test not present: 0 0 >> * Strange test result: 0 0 >> ** >> >> v4.17-rc1 (and later) libhugetlbfs test results >> ** TEST SUMMARY >> * 2M >> * 32-bit 64-bit >> * Total testcases: 110113 >> * Skipped: 0 0 >> *PASS:98111 >> *FAIL: 0 0 >> *Killed by signal:11 1 >> * Bad configuration: 1 1 >> * Expected FAIL: 0 0 >> * Unexpected PASS: 0 0 >> *Test not present: 0 0 >> * Strange test result: 0 0 >> ** >> >> I traced the 7 additional (32-bit) killed by signal results to this >> commit 4ed28639519c fs, elf: drop MAP_FIXED usage from elf_map. >> >> libhugetlbfs does unusual things and even provides custom linker scripts. >> So, in hindsight this change in behavior does not seem too unexpected. I >> JUST discovered this while running libhugetlbfs tests for an unrelated >> issue/change and, will do some analysis to see exactly what is happening. > > I am definitely interested about further details. Are there any messages > in the kernel log? > Yes, new messages associated with the failures. [ 47.570451] 1368 (xB.linkhuge_nof): Uhuuh, elf segment at a731413b requested but the memory is mapped already [ 47.606991] 1372 (xB.linkhuge_nof): Uhuuh, elf segment at a731413b requested but the memory is mapped already [ 47.641351] 1376 (xB.linkhuge_nof): Uhuuh, elf segment at a731413b requested but the memory is mapped already [ 47.726138] 1384 (xB.linkhuge): Uhuuh, elf segment at 90b9eaf6 requested but the memory is mapped already [ 47.773169] 1393 (xB.linkhuge): Uhuuh, elf segment at 90b9eaf6 requested but the memory is mapped already [ 47.817788] 1402 (xB.linkhuge): Uhuuh, elf segment at 90b9eaf6 requested but the memory is mapped already [ 47.857338] 1406 (xB.linkshare): Uhuuh, elf segment at 18430471 requested but the memory is mapped already [ 47.956355] 1427 (xB.linkshare): Uhuuh, elf segment at 18430471 requested but the memory is mapped already [ 48.054894] 1448 (xB.linkhuge): Uhuuh, elf segment at 90b9eaf6 requested but the memory is mapped already [ 48.071221] 1451 (xB.linkhuge): Uhuuh, elf segment at 90b9eaf6 requested but the memory is mapped already Just curious, the addresses printed in those messages does not seem correct. They should be page aligned. Correct? I think that %p conversion in the pr_info() may doing something wrong. Also, the new failures in question are indeed being built with custom linker scripts designed for use with binutils older than 2.16 (really old). So, no new users should encounter this issue (I think). It appears that this may only impact old applications built long ago with pre-2.16 binutils. -- Mike Kravetz
[PATCH v1 0/4] Tegra20 External Memory Controller driver
Hello, Couple years ago the Tegra20 EMC driver was removed from the kernel due to incompatible changes in the Tegra's clock driver. This patchset introduces a modernized EMC driver. Currently the sole purpose of the driver is to initialize DRAM frequency to maximum rate during of the kernels boot-up. Later we may consider implementing dynamic memory frequency scaling, utilizing functionality provided by this driver. Dmitry Osipenko (4): dt: bindings: tegra20-emc: Document interrupt property ARM: dts: tegra20: Add interrupt to External Memory Controller clk: tegra20: Turn EMC clock gate into divider memory: tegra: Introduce Tegra20 EMC driver .../bindings/arm/tegra/nvidia,tegra20-emc.txt | 2 + arch/arm/boot/dts/tegra20.dtsi| 1 + drivers/clk/tegra/clk-tegra20.c | 39 +- drivers/memory/tegra/Kconfig | 10 + drivers/memory/tegra/Makefile | 1 + drivers/memory/tegra/tegra20-emc.c| 573 ++ 6 files changed, 616 insertions(+), 10 deletions(-) create mode 100644 drivers/memory/tegra/tegra20-emc.c -- 2.17.0
[PATCH v1 4/4] memory: tegra: Introduce Tegra20 EMC driver
Introduce driver for the External Memory Controller (EMC) found on Tegra20 chips, which controls the external DRAM on the board. The purpose of this driver is to program memory timings for external memory on the EMC clock rate change. Signed-off-by: Dmitry Osipenko --- drivers/memory/tegra/Kconfig | 10 + drivers/memory/tegra/Makefile | 1 + drivers/memory/tegra/tegra20-emc.c | 573 + 3 files changed, 584 insertions(+) create mode 100644 drivers/memory/tegra/tegra20-emc.c diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig index 6d74e499e18d..34e0b70f5c5f 100644 --- a/drivers/memory/tegra/Kconfig +++ b/drivers/memory/tegra/Kconfig @@ -6,6 +6,16 @@ config TEGRA_MC This driver supports the Memory Controller (MC) hardware found on NVIDIA Tegra SoCs. +config TEGRA20_EMC + bool "NVIDIA Tegra20 External Memory Controller driver" + default y + depends on ARCH_TEGRA_2x_SOC + help + This driver is for the External Memory Controller (EMC) found on + Tegra20 chips. The EMC controls the external DRAM on the board. + This driver is required to change memory timings / clock rate for + external memory. + config TEGRA124_EMC bool "NVIDIA Tegra124 External Memory Controller driver" default y diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile index 94ab16ba075b..3971a6b7c487 100644 --- a/drivers/memory/tegra/Makefile +++ b/drivers/memory/tegra/Makefile @@ -10,5 +10,6 @@ tegra-mc-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o obj-$(CONFIG_TEGRA_MC) += tegra-mc.o +obj-$(CONFIG_TEGRA20_EMC) += tegra20-emc.o obj-$(CONFIG_TEGRA124_EMC) += tegra124-emc.o obj-$(CONFIG_ARCH_TEGRA_186_SOC) += tegra186.o diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c new file mode 100644 index ..fbd1f85edf23 --- /dev/null +++ b/drivers/memory/tegra/tegra20-emc.c @@ -0,0 +1,573 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Tegra20 External Memory Controller driver + * + * Author: Dmitry Osipenko + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define EMC_INTSTATUS 0x000 +#define EMC_INTMASK0x004 +#define EMC_TIMING_CONTROL 0x028 +#define EMC_RC 0x02c +#define EMC_RFC0x030 +#define EMC_RAS0x034 +#define EMC_RP 0x038 +#define EMC_R2W0x03c +#define EMC_W2R0x040 +#define EMC_R2P0x044 +#define EMC_W2P0x048 +#define EMC_RD_RCD 0x04c +#define EMC_WR_RCD 0x050 +#define EMC_RRD0x054 +#define EMC_REXT 0x058 +#define EMC_WDV0x05c +#define EMC_QUSE 0x060 +#define EMC_QRST 0x064 +#define EMC_QSAFE 0x068 +#define EMC_RDV0x06c +#define EMC_REFRESH0x070 +#define EMC_BURST_REFRESH_NUM 0x074 +#define EMC_PDEX2WR0x078 +#define EMC_PDEX2RD0x07c +#define EMC_PCHG2PDEN 0x080 +#define EMC_ACT2PDEN 0x084 +#define EMC_AR2PDEN0x088 +#define EMC_RW2PDEN0x08c +#define EMC_TXSR 0x090 +#define EMC_TCKE 0x094 +#define EMC_TFAW 0x098 +#define EMC_TRPAB 0x09c +#define EMC_TCLKSTABLE 0x0a0 +#define EMC_TCLKSTOP 0x0a4 +#define EMC_TREFBW 0x0a8 +#define EMC_QUSE_EXTRA 0x0ac +#define EMC_ODT_WRITE 0x0b0 +#define EMC_ODT_READ 0x0b4 +#define EMC_FBIO_CFG5 0x104 +#define EMC_FBIO_CFG6 0x114 +#define EMC_AUTO_CAL_INTERVAL 0x2a8 +#define EMC_CFG_2 0x2b8 +#define EMC_CFG_DIG_DLL0x2bc +#define EMC_DLL_XFORM_DQS 0x2c0 +#define EMC_DLL_XFORM_QUSE 0x2c4 +#define EMC_ZCAL_REF_CNT 0x2e0 +#define EMC_ZCAL_WAIT_CNT 0x2e4 +#define EMC_CFG_CLKTRIM_0 0x2d0 +#define EMC_CFG_CLKTRIM_1 0x2d4
Re: [PATCH] IB/mad: Use ID allocator routines to allocate agent number
On Wed, May 30, 2018 at 02:22:56PM +0200, Hans Westgaard Ry wrote: > We came up with this code snippet which we think handles both preventing > immediate re-use and too big/wrapping... Isn't this basically the same as idr_alloc_cyclic ? Jason
Re: [PATCH v7 3/7] drivers/i2c: Add port structure to FSI algorithm
On 05/29/2018 06:19 PM, Andy Shevchenko wrote:
On Wed, May 30, 2018 at 1:24 AM, Eddie James wrote:
From: "Edward A. James"
Add and initialize I2C adapters for each port on the FSI-attached I2C
master. Ports for each master are defined in the devicetree.
+#include
+static int fsi_i2c_set_port(struct fsi_i2c_port *port)
+{
+ int rc;
+ struct fsi_device *fsi = port->master->fsi;
+ u32 mode, dummy = 0;
+ u16 old_port;
+
+ rc = fsi_i2c_read_reg(fsi, I2C_FSI_MODE, );
+ if (rc)
+ return rc;
+
+ old_port = GETFIELD(I2C_MODE_PORT, mode);
+
+ if (old_port != port->port) {
Why not simple
if (port->port == GETFIELD())
return 0;
?
Sure.
+ /* reset engine when port is changed */
+ rc = fsi_i2c_write_reg(fsi, I2C_FSI_RESET_ERR, );
+ if (rc)
+ return rc;
+ }
+ return rc;
It's hardly would be non-zero, right?
No, fsi_i2c_read_reg and fsi_i2c_write_reg both return 0 on success and
non-zero on error. That is the desired behavior of this function also.
+}
static int fsi_i2c_probe(struct device *dev)
{
Isn't below somehow repeats of_i2c_register_devices() ?
Why not to use it?
Because I need to assign all these port structure fields. Also looks
like of_i2c_register_devices creates new devices; I just want an adapter
for each port.
+ /* Add adapter for each i2c port of the master. */
+ for_each_available_child_of_node(dev->of_node, np) {
+ rc = of_property_read_u32(np, "reg", _no);
+ if (rc || port_no > USHRT_MAX)
+ continue;
+
+ port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+ if (!port)
+ break;
+
+ port->master = i2c;
+ port->port = port_no;
+
+ port->adapter.owner = THIS_MODULE;
+ port->adapter.dev.of_node = np;
+ port->adapter.dev.parent = dev;
+ port->adapter.algo = _i2c_algorithm;
+ port->adapter.algo_data = port;
+
+ snprintf(port->adapter.name, sizeof(port->adapter.name),
+"i2c_bus-%u", port_no);
+
+ rc = i2c_add_adapter(>adapter);
+ if (rc < 0) {
+ dev_err(dev, "Failed to register adapter: %d\n", rc);
+ devm_kfree(dev, port);
This hurts my eyes. Why?!
What would you suggest instead?
+ continue;
+ }
+
+ list_add(>list, >ports);
+ }
+
dev_set_drvdata(dev, i2c);
return 0;
}
+ if (!list_empty(>ports)) {
My gosh, this is done already in list_for_each*()
No, list_for_each_entry does NOT check if the list is empty or if the
first entry is NULL.
Thanks,
Eddie
+ list_for_each_entry(port, >ports, list) {
+ i2c_del_adapter(>adapter);
+ }
+ }
Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq
Hi Vijay, On 05/30/2018 10:11 AM, Vijay Viswanath wrote: > Hi Georgi, > > Thanks for testing the patch on 8096 and pointing out this issue. > The issue is coming because, when card is removed, the HOST_CONTROL2 > register is retaining the 1.8V Signalling enable bit till SDHCI reset > happens after a new card is inserted. > > Adding the change you suggested can avoid this wait, but I feel a better > solution is to clear the 1.8V signalling bit when the card is removed. > When a new card is inserted, we shouldn't be keeping the 1.8V bit set > until we send cmd11 to the SD card. A new SD card should start with 3V. > > One solution is to explicitly clear the HOST_CONTROL2 register when card > is removed. > > Other way is to revert the commit: > 9718f84b85396e090ca42fafa730410d286d61e3 "mmc: sdhci-msm: Do not reset > the controller if no card in the slot" > > The sdhci-msm doesn't require "SDHCI_QUIRK_NO_CARD_NO_RESET". The issue > which above commit is trying to avoid is fixed by the pwr-irq patches. > Resetting the controller will clear the HOST_CONTROL2 register and avoid > this issue. > > Can you please try this ? I tested reverting the QUIRK on two platforms: > db410c(8916) and sdm845. SD card insert/remove worked fine after that > and I didn't get any "Reset 0x1 never completed" error during card > insert/remove or shutdown. Thank you! I have submitted a patch to remove the quirk and tested it on db410c and db820c. BR, Georgi
[PATCH] sched/deadline: Fix missing clock update
A missing clock update is causing the below warning:
[ cut here ]
rq->clock_update_flags < RQCF_ACT_SKIP
WARNING: CPU: 10 PID: 0 at kernel/sched/sched.h:963
inactive_task_timer+0x5d6/0x720
Modules linked in: xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun
ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack devlink ip_set nfnetlink
ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6
nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_raw ip6table_security
iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
iptable_mangle iptable_raw iptable_security ebtable_filter ebtables
ip6table_filter ip6_tables cmac bnep sunrpc arc4 iwlmvm intel_rapl sb_edac
x86_pkg_temp_thermal intel_powerclamp coretemp mac80211 kvm_intel kvm irqbypass
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel intel_cstate intel_uncore
iwlwifi intel_rapl_perf snd_hda_codec_hdmi cfg80211 mei_wdt
snd_hda_codec_realtek iTCO_wdt iTCO_vendor_support snd_hda_codec_generic
snd_hda_intel btusb snd_hda_codec btrtl btbcm btintel wmi_bmof
intel_wmi_thunderbolt snd_hwdep bluetooth snd_hda_core snd_seq snd_seq_device
snd_pcm rtsx_usb_ms mei_me ecdh_generic snd_timer memstick rfkill snd mei
lpc_ich i2c_i801 shpchp tpm_tis soundcore tpm_tis_core tpm xfs libcrc32c
nouveau video drm_kms_helper e1000e ttm rtsx_usb_sdmmc drm mmc_core mxm_wmi igb
crc32c_intel dca rtsx_usb ptp ata_generic r8169 i2c_algo_bit pps_core pata_acpi
mii wmi
CPU: 10 PID: 0 Comm: swapper/10 Not tainted 4.17.0-rc6+ #33
Hardware name: LENOVO 30B6S2F900/1030, BIOS S01KT56A 01/15/2018
RIP: 0010:inactive_task_timer+0x5d6/0x720
RSP: :91c89f203eb0 EFLAGS: 00010086
RAX: RBX: 91c891cc5810 RCX:
RDX: 91c89b572a80 RSI: RDI: a213319d
RBP: 91c89f3e1f40 R08: 0001 R09: 0001
R10: 91c89f203df0 R11: R12: 91c09ec0e000
R13: 91c89f3e0800 R14: 001e1f40 R15: a27c2bb0
FS: () GS:91c89f20() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 7f84659eb710 CR3: 000439212003 CR4: 001606e0
Call Trace:
? task_woken_dl+0x70/0x70
__hrtimer_run_queues+0x10f/0x530
hrtimer_interrupt+0xe5/0x240
smp_apic_timer_interrupt+0x79/0x2b0
apic_timer_interrupt+0xf/0x20
RIP: 0010:cpuidle_enter_state+0xa5/0x360
RSP: :a1f046387e98 EFLAGS: 0206 ORIG_RAX: ff13
RAX: 91c89b572a80 RBX: 0080584fb70b RCX:
RDX: 91c89b572a80 RSI: 0001 RDI: 91c89b572a80
RBP: 0002 R08: 15ec R09:
R10: R11: R12: 91c89f3ebe40
R13: a32dc4f8 R14: R15: 0080584f5890
? cpuidle_enter_state+0x9e/0x360
do_idle+0x203/0x280
cpu_startup_entry+0x6f/0x80
start_secondary+0x1b0/0x200
secondary_startup_64+0xa5/0xb0
Code: 31 c7 fa ff 0f 0b e9 e6 fd ff ff 80 3d 3c 61 24 01 00 0f 85 75 fd ff ff
48 c7 c7 b0 5b 0b a3 c6 05 28 61 24 01 01 e8 0a c7 fa ff <0f> 0b e9 5b fd ff ff
48 8b 7c 24 08 be ff ff ff ff e8 84 1b 01
irq event stamp: 793922
hardirqs last enabled at (793919): []
cpuidle_enter_state+0x9e/0x360
hardirqs last disabled at (793920): []
interrupt_entry+0xce/0xe0
softirqs last enabled at (793922): [] irq_enter+0x68/0x70
softirqs last disabled at (793921): [] irq_enter+0x4d/0x70
---[ end trace eda7418c80ca042c ]---
This happens because inactive_task_timer calls sub_running_bw (if
TASK_DEAD and non_contending) that might trigger a schedutil update,
which might access the clock. Clock is however currently updated only
later in inactive_task_timer function.
Fix the problem by updating the clock right after task_rq_lock().
Reported-by: kernel test robot
Signed-off-by: Juri Lelli
Cc: Ingo Molnar
Cc: Peter Zijlstra
Cc: Luca Abeni
Cc: Claudio Scordino
Cc: linux-kernel@vger.kernel.org
---
This was actually first spotted by lkp-robot[1], but the fix never made
it to the list as a proper patch. Apologies. :/
[1] https://www.spinics.net/lists/kernel/msg2706782.html
---
kernel/sched/deadline.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 1356afd1eeb6..fbfc3f1d368a 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1259,6 +1259,9 @@ static enum hrtimer_restart inactive_task_timer(struct
hrtimer *timer)
rq = task_rq_lock(p, );
+ sched_clock_tick();
+ update_rq_clock(rq);
+
if (!dl_task(p) || p->state == TASK_DEAD) {
struct dl_bw *dl_b = dl_bw_of(task_cpu(p));
@@ -1278,9 +1281,6 @@ static enum hrtimer_restart inactive_task_timer(struct
hrtimer *timer)
if (dl_se->dl_non_contending == 0)
goto unlock;
- sched_clock_tick();
- update_rq_clock(rq);
-
Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
On Wed, May 30, 2018 at 09:06:16AM -0700, Doug Anderson wrote: > On Wed, May 30, 2018 at 8:48 AM, Mark Brown wrote: > > Without the core doing something the regulator isn't going to get told > > that anything updated voltages anyway... > I was just suggesting that when the core tells the regulator driver to > disable itself that the regulator driver tell RPMh to not only disable > itself but also (temporarily) vote for a lower voltage. When the core > tells the regulator to re-enable itself then the regulator driver > restores the original voltage vote (or applies any vote that might > have been attempted while the regulator was disabled). That wouldn't > require any regulator core changes. It needs something to tell it what the new voltage to set is. signature.asc Description: PGP signature
Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
On Wed, May 30, 2018 at 09:12:25AM -0700, Doug Anderson wrote: > On Wed, May 30, 2018 at 8:50 AM, Mark Brown wrote: > > No, I'm saying that I don't know why that property exists at all. This > > sounds like it's intended to be the amount of current the regulator can > > deliver in each mode which is normally a design property of the silicon. > Ah, got it. So the whole point here is to be able to implement either > the function "set_load" or the function "get_optimum_mode". We need > some sort of table to convert from current to mode. That's what this > table does. We do need that table, my expectation would be that this table would be in the driver as it's not something I'd expect to vary between different systems but rather be a property of the silicon design. No sense in every single board having to copy it in. signature.asc Description: PGP signature
Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
Hi, On Wed, May 30, 2018 at 9:13 AM, Mark Brown wrote: > On Wed, May 30, 2018 at 09:09:02AM -0700, Doug Anderson wrote: >> On Wed, May 30, 2018 at 9:07 AM, Mark Brown wrote: > >> > It needs something to tell it what the new voltage to set is. > >> The regulator driver has its own cache of what voltage was most >> recently requested by Linux. It can use that, can't it? > > If we're just going to use the most recently set voltage then hopefully > the hardware already knew that, and it might not be the lowest available > voltage if the last consumer to get turned off was holding the voltage > higher. To circle back to the original point: the problem is that (IMHO) the hardware is doing the wrong thing by still counting Linux's vote for a voltage even though Linux also voted that the regulator should be disabled. So basically we're working around the hardware by pretending to vote for a dummy lower voltage whenever Linux wants the regulator disabled. From Linux's point of view everything works as normal--we just tell the hardware a falsehood so it doesn't count our vote for a voltage while the regulator is disabled. -Doug
Re: [PATCH 3.2 000/153] 3.2.102-rc1 review
On Wed, May 30, 2018 at 11:52:40AM +0100, Ben Hutchings wrote: > This is the start of the stable review cycle for the 3.2.102 release. > There are 153 patches in this series, which will be posted as responses > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Thu May 31 23:00:00 UTC 2018. > Anything received after that time might be too late. > Build results: total: 86 pass: 86 fail: 0 Qemu test results: total: 73 pass: 73 fail: 0 Details are available at http://kerneltests.org/builders/. Guenter
Re: [PATCH 08/13] blk-stat: export helpers for modifying blk_rq_stat
On Tue, May 29, 2018 at 05:17:19PM -0400, Josef Bacik wrote: > From: Josef Bacik > > We need to use blk_rq_stat in the blkcg qos stuff, so export some of > these helpers so they can be used by other things. > > Signed-off-by: Josef Bacik Acked-by: Tejun Heo Thanks. -- tejun
Re: [PATCH 0/2] PCI: Trivial quirks cleanup
On Thu, May 10, 2018 at 05:09:06PM -0500, Bjorn Helgaas wrote: > [Sorry for the repost; network hiccup here.] > > Trivial reorganization (move quirk infrastructure to the top) and > whitespace/comment style cleanup for consistency. No functional change > intended. > > --- > > Bjorn Helgaas (2): > PCI: Reorder quirks infrastructure code > PCI: Clean up whitespace in quirks.c > > > drivers/pci/quirks.c | 978 > +- > 1 file changed, 485 insertions(+), 493 deletions(-) I applied these to pci/trivial for v4.18.
Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
On Wed, May 30, 2018 at 09:41:55AM -0700, Doug Anderson wrote: > On Wed, May 30, 2018 at 9:36 AM, Mark Brown wrote: > > Yeah, and I don't think that's unreasonable for the core to do - just > > drop the voltage to the constraint minimum after it has turned off the > > regulator, then recheck and raise if needed before it enables again. > Would it do this for all regulators, though? Most regulators are > hooked up over a slow i2c bus, so that means that every regulator > disable would now do an extra i2c transfer even though for all > regulators (other than RPMh) the voltage of a regulator doesn't matter > when it's been "disabled' (from Linux's point of view). It'd only affect regulators that can vary in voltage and actually get disabled which is a pretty small subset. Most regulators are either fixed voltage or always on. > Hrmmm, I suppose maybe it'd be OK though because for most regulators > we'd use "regcache" and most regulators that we enabled/disable a lot > are not expected to change voltage in Linux (so the regcache write > would hopefully be a noop), so maybe it wouldn't be _that_ > inefficient... Even if the regulator lacks a cache we'd at least skip out on the write when we're not changing voltage as we'd do a read/modify/update cycle and stop at the modify. signature.asc Description: PGP signature
RE: [PATCH 1/1] pwm: fsl-ftm: Support the new version of FTM block on i.MX8x
Ping.
Shenwei
-Original Message-
From: Shenwei Wang
Sent: Thursday, May 24, 2018 1:09 PM
To: thierry.red...@gmail.com
Cc: linux-...@vger.kernel.org; dl-linux-imx ;
linux-kernel@vger.kernel.org; Shenwei Wang
Subject: [PATCH 1/1] pwm: fsl-ftm: Support the new version of FTM block on
i.MX8x
On the new i.MX8x SoC family, the following changes were made on the FTM
block:
1. Need to enable the IPG clock before accessing any FTM registers. Because the
IPG clock is not an option for FTM counter clock source, it can't be used as
the ftm_sys clock.
2. An additional PWM enable bit was added for each PWM channel in register
FTM_SC[16:23]. It supports 8 channels. Bit16 is for channel 0, and bit23 for
channel 7.
As the IP version information can not be obtained in any of the FTM registers,
a property of "fsl,has-pwmen-bits" is added in the ftm pwm device node. If it
has the property, the driver set the PWM enable bit when a PWM channel is
requested.
Signed-off-by: Shenwei Wang
---
drivers/pwm/pwm-fsl-ftm.c | 35 +--
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c index
557b4ea..0426458f 100644
--- a/drivers/pwm/pwm-fsl-ftm.c
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -86,7 +86,9 @@ struct fsl_pwm_chip {
struct regmap *regmap;
int period_ns;
+ bool has_pwmen;
+ struct clk *ipg_clk;
struct clk *clk[FSL_PWM_CLK_MAX];
};
@@ -97,16 +99,31 @@ static inline struct fsl_pwm_chip *to_fsl_chip(struct
pwm_chip *chip)
static int fsl_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm) {
+ int ret;
struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
- return clk_prepare_enable(fpc->clk[FSL_PWM_CLK_SYS]);
+ ret = clk_prepare_enable(fpc->ipg_clk);
+
+ if ((!ret) && (fpc->has_pwmen)) {
+ mutex_lock(>lock);
+ regmap_update_bits(fpc->regmap, FTM_SC,
+ BIT(pwm->hwpwm + 16), BIT(pwm->hwpwm + 16));
+ mutex_unlock(>lock);
+ }
+
+ return ret;
}
static void fsl_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) {
struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
- clk_disable_unprepare(fpc->clk[FSL_PWM_CLK_SYS]);
+ if (fpc->has_pwmen) {
+ mutex_lock(>lock);
+ regmap_update_bits(fpc->regmap, FTM_SC, BIT(pwm->hwpwm + 16),
0);
+ mutex_unlock(>lock);
+ }
+ clk_disable_unprepare(fpc->ipg_clk);
}
static int fsl_pwm_calculate_default_ps(struct fsl_pwm_chip *fpc, @@ -363,7
+380,7 @@ static int fsl_pwm_init(struct fsl_pwm_chip *fpc) {
int ret;
- ret = clk_prepare_enable(fpc->clk[FSL_PWM_CLK_SYS]);
+ ret = clk_prepare_enable(fpc->ipg_clk);
if (ret)
return ret;
@@ -371,7 +388,7 @@ static int fsl_pwm_init(struct fsl_pwm_chip *fpc)
regmap_write(fpc->regmap, FTM_OUTINIT, 0x00);
regmap_write(fpc->regmap, FTM_OUTMASK, 0xFF);
- clk_disable_unprepare(fpc->clk[FSL_PWM_CLK_SYS]);
+ clk_disable_unprepare(fpc->ipg_clk);
return 0;
}
@@ -428,6 +445,10 @@ static int fsl_pwm_probe(struct platform_device *pdev)
return PTR_ERR(fpc->clk[FSL_PWM_CLK_SYS]);
}
+ fpc->ipg_clk = devm_clk_get(>dev, "ipg");
+ if (IS_ERR(fpc->ipg_clk))
+ fpc->ipg_clk = fpc->clk[FSL_PWM_CLK_SYS];
+
fpc->clk[FSL_PWM_CLK_FIX] = devm_clk_get(fpc->chip.dev, "ftm_fix");
if (IS_ERR(fpc->clk[FSL_PWM_CLK_FIX]))
return PTR_ERR(fpc->clk[FSL_PWM_CLK_FIX]);
@@ -446,6 +467,8 @@ static int fsl_pwm_probe(struct platform_device *pdev)
fpc->chip.of_pwm_n_cells = 3;
fpc->chip.base = -1;
fpc->chip.npwm = 8;
+ fpc->has_pwmen = of_property_read_bool(pdev->dev.of_node,
+ "fsl,ftm-has-pwmen-bits");
ret = pwmchip_add(>chip);
if (ret < 0) {
@@ -480,7 +503,7 @@ static int fsl_pwm_suspend(struct device *dev)
if (!test_bit(PWMF_REQUESTED, >flags))
continue;
- clk_disable_unprepare(fpc->clk[FSL_PWM_CLK_SYS]);
+ clk_disable_unprepare(fpc->ipg_clk);
if (!pwm_is_enabled(pwm))
continue;
@@ -503,7 +526,7 @@ static int fsl_pwm_resume(struct device *dev)
if (!test_bit(PWMF_REQUESTED, >flags))
continue;
- clk_prepare_enable(fpc->clk[FSL_PWM_CLK_SYS]);
+ clk_prepare_enable(fpc->ipg_clk);
if (!pwm_is_enabled(pwm))
continue;
--
2.9.2
Re: [PATCH v2 1/2] ARM: debug: Add Iproc UART3 debug addresses
Hi Clément, Correct me if I'm wrong, but I thought the trend is to move to use earlycon that can be activated from kernel command line for early print before the serial driver is loaded. Have you tried earlcon? Thanks, Ray On 5/30/2018 6:19 AM, Clément Péron wrote: From: Clément Peron Broadcom Iproc SoCs typically use the UART3 for debug/console, provide a known good location for that. Signed-off-by: Clément Peron --- arch/arm/Kconfig.debug | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index 199ebc1c4538..4ea9d5793b91 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -207,6 +207,14 @@ choice depends on ARCH_BCM_HR2 select DEBUG_UART_8250 + config DEBUG_BCM_IPROC_UART3 + bool "Kernel low-level debugging on BCM IPROC UART3" + depends on ARCH_BCM_CYGNUS + select DEBUG_UART_8250 + help + Say Y here if you want the debug print routines to direct + their output to the third serial port on these devices. + config DEBUG_BCM_KONA_UART bool "Kernel low-level debugging messages via BCM KONA UART" depends on ARCH_BCM_MOBILE @@ -1557,6 +1565,7 @@ config DEBUG_UART_PHYS default 0x18000400 if DEBUG_BCM_HR2 default 0x1801 if DEBUG_SIRFATLAS7_UART0 default 0x1802 if DEBUG_SIRFATLAS7_UART1 + default 0x18023000 if DEBUG_BCM_IPROC_UART3 default 0x1c09 if DEBUG_VEXPRESS_UART0_RS1 default 0x20001000 if DEBUG_HIP01_UART default 0x2006 if DEBUG_RK29_UART0 @@ -1676,6 +1685,7 @@ config DEBUG_UART_VIRT default 0xf1002000 if DEBUG_MT8127_UART0 default 0xf1006000 if DEBUG_MT6589_UART0 default 0xf1009000 if DEBUG_MT8135_UART3 + default 0xf1023000 if DEBUG_BCM_IPROC_UART3 default 0xf11f1000 if DEBUG_VERSATILE default 0xf160 if DEBUG_INTEGRATOR default 0xf1c28000 if DEBUG_SUNXI_UART0 @@ -1791,7 +1801,7 @@ config DEBUG_UART_8250_WORD DEBUG_KEYSTONE_UART0 || DEBUG_KEYSTONE_UART1 || \ DEBUG_ALPINE_UART0 || \ DEBUG_DAVINCI_DMx_UART0 || DEBUG_DAVINCI_DA8XX_UART1 || \ - DEBUG_DAVINCI_DA8XX_UART2 || \ + DEBUG_DAVINCI_DA8XX_UART2 || DEBUG_BCM_IPROC_UART3 || \ DEBUG_BCM_KONA_UART || DEBUG_RK32_UART2 config DEBUG_UART_8250_PALMCHIP
Re: [PATCH 5/6 v2] mtd: rawnand: ams-delta: use GPIO lookup table
On Wed, 30 May 2018 19:43:09 +0200 Janusz Krzysztofik wrote: > On Wednesday, May 30, 2018 11:05:00 AM CEST Boris Brezillon wrote: > > Hi Janusz, > > Hi Boris, > > > On Sat, 26 May 2018 00:20:45 +0200 > > Janusz Krzysztofik wrote: > > > ... > > > Changes since v1: > > > - fix handling of devm_gpiod_get_optional() return values - thanks to > > > Andy Shevchenko. > > > > Can you put the changelog after the "---" separator so that it does not > > appear in the final commit message? > > Yes, sure, sorry for that. > > > > +err_gpiod: > > > + if (err == -ENODEV || err == -ENOENT) > > > + err = -EPROBE_DEFER; > > > > Hm, isn't it better to make gpiod_find() return ERR_PTR(-EPROBE_DEFER) > > here [1]? At least, ENOENT should not be turned into EPROBE_DEFER, > > because it's returned when there's no entry matching the requested gpio > > in the lookup table, and deferring the probe won't solve this problem. > > ENOENT is also returned when no matching lookup table is found. That may > happen if consumer dev_name stored in the table differs from dev_name > assigned > to the consumer by its bus, the platform bus in this case. For that reason I > think the consumer dev_name should be initialized in the table after the > device is registered, when its actual dev_name can be obtained. If that > device > registration happens after the driver is already registered, e.g., at > late_initcall, the device is probed before its lookup table is ready. For > that > reason returning EPROBE_DEFER seems better to me even in the ENOENT case. Sorry, I don't get it. Aren't GPIO lookup tables supposed to be declared in board files, especially if the GPIO is used by a platform device? When would you have a lookup table registered later in the init/boot process?
Re: [PATCH][next] mailbox: PCC: check for negative count for parse failure checking
On 05/30/2018 11:14 AM, Colin King wrote:
> From: Colin Ian King
>
> The function acpi_table_parse_enties_array can potentially return a
> negative value if parsing fails. Currently the check on the return
> is not checking for errors, so fix this by adding a -ve check too.
>
> Detected by CoverityScan, CID#1469477 ("Improper use of negative value")
>
> Fixes: 8f8027c5f935 ("mailbox: PCC: erroneous error message when parsing ACPI
> PCCT")
> Signed-off-by: Colin Ian King
> ---
> drivers/mailbox/pcc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index fc3c237daef2..87d67922020d 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -461,7 +461,7 @@ static int __init acpi_pcc_probe(void)
> count = acpi_table_parse_entries_array(ACPI_SIG_PCCT,
> sizeof(struct acpi_table_pcct), proc,
> ACPI_PCCT_TYPE_RESERVED, MAX_PCC_SUBSPACES);
> - if (count == 0 || count > MAX_PCC_SUBSPACES) {
> + if (count <= 0 || count > MAX_PCC_SUBSPACES) {
> pr_warn("Invalid PCCT: %d PCC subspaces\n", count);
> return -EINVAL;
> }
>
Yup, nice catch. A little paranoid, but we like that in a kernel :). Thanks.
Reviewed-by: Al Stone
--
ciao,
al
---
Al Stone
Software Engineer
Red Hat, Inc.
a...@redhat.com
---
Re: [alsa-devel][PATCH v1] ASoC: TSCS454: Add Support
Hi Steven,
I love your patch! Perhaps something to improve:
[auto build test WARNING on asoc/for-next]
[also build test WARNING on v4.17-rc7 next-20180530]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Steven-Eckhoff/ASoC-TSCS454-Add-Support/20180531-001905
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
for-next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> sound/soc/codecs/tscs454.c:2454:9: sparse: Using plain integer as NULL
>> pointer
>> sound/soc/codecs/tscs454.c:2629:5: sparse: symbol 'tscs454_set_sysclk' was
>> not declared. Should it be static?
sound/soc/codecs/tscs454.c:236:10: sparse: Initializer entry defined twice
sound/soc/codecs/tscs454.c:239:10: also defined here
Please review and possibly fold the followup patch.
vim +2454 sound/soc/codecs/tscs454.c
2348
2349 static struct snd_soc_dapm_widget const tscs454_dapm_widgets[] = {
2350 /* R_PLLCTL PG 0 ADDR 0x15 */
2351 SND_SOC_DAPM_SUPPLY("PLL 1 Power", R_PLLCTL, FB_PLLCTL_PU_PLL1,
0,
2352 pll_power_event,
2353 SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_PRE_PMD),
2354 SND_SOC_DAPM_SUPPLY("PLL 2 Power", R_PLLCTL, FB_PLLCTL_PU_PLL2,
0,
2355 pll_power_event,
2356 SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_PRE_PMD),
2357 /* R_I2SPINC0 PG 0 ADDR 0x22 */
2358 SND_SOC_DAPM_AIF_OUT("DAI 3 Out", "DAI 3 Capture", 0,
2359 R_I2SPINC0, FB_I2SPINC0_SDO3TRI, 1),
2360 SND_SOC_DAPM_AIF_OUT("DAI 2 Out", "DAI 2 Capture", 0,
2361 R_I2SPINC0, FB_I2SPINC0_SDO2TRI, 1),
2362 SND_SOC_DAPM_AIF_OUT("DAI 1 Out", "DAI 1 Capture", 0,
2363 R_I2SPINC0, FB_I2SPINC0_SDO1TRI, 1),
2364 /* R_PWRM0 PG 0 ADDR 0x33 */
2365 SND_SOC_DAPM_ADC("Input Processor Channel 3", NULL,
2366 R_PWRM0, FB_PWRM0_INPROC3PU, 0),
2367 SND_SOC_DAPM_ADC("Input Processor Channel 2", NULL,
2368 R_PWRM0, FB_PWRM0_INPROC2PU, 0),
2369 SND_SOC_DAPM_ADC("Input Processor Channel 1", NULL,
2370 R_PWRM0, FB_PWRM0_INPROC1PU, 0),
2371 SND_SOC_DAPM_ADC("Input Processor Channel 0", NULL,
2372 R_PWRM0, FB_PWRM0_INPROC0PU, 0),
2373 SND_SOC_DAPM_SUPPLY("Mic Bias 2",
2374 R_PWRM0, FB_PWRM0_MICB2PU, 0, NULL, 0),
2375 SND_SOC_DAPM_SUPPLY("Mic Bias 1", R_PWRM0,
2376 FB_PWRM0_MICB1PU, 0, NULL, 0),
2377 /* R_PWRM1 PG 0 ADDR 0x34 */
2378 SND_SOC_DAPM_SUPPLY("Sub Power", R_PWRM1, FB_PWRM1_SUBPU, 0,
NULL, 0),
2379 SND_SOC_DAPM_SUPPLY("Headphone Left Power",
2380 R_PWRM1, FB_PWRM1_HPLPU, 0, NULL, 0),
2381 SND_SOC_DAPM_SUPPLY("Headphone Right Power",
2382 R_PWRM1, FB_PWRM1_HPRPU, 0, NULL, 0),
2383 SND_SOC_DAPM_SUPPLY("Speaker Left Power",
2384 R_PWRM1, FB_PWRM1_SPKLPU, 0, NULL, 0),
2385 SND_SOC_DAPM_SUPPLY("Speaker Right Power",
2386 R_PWRM1, FB_PWRM1_SPKRPU, 0, NULL, 0),
2387 SND_SOC_DAPM_SUPPLY("Differential Input 2 Power",
2388 R_PWRM1, FB_PWRM1_D2S2PU, 0, NULL, 0),
2389 SND_SOC_DAPM_SUPPLY("Differential Input 1 Power",
2390 R_PWRM1, FB_PWRM1_D2S1PU, 0, NULL, 0),
2391 /* R_PWRM2 PG 0 ADDR 0x35 */
2392 SND_SOC_DAPM_SUPPLY("DAI 3 Out Power",
2393 R_PWRM2, FB_PWRM2_I2S3OPU, 0, NULL, 0),
2394 SND_SOC_DAPM_SUPPLY("DAI 2 Out Power",
2395 R_PWRM2, FB_PWRM2_I2S2OPU, 0, NULL, 0),
2396 SND_SOC_DAPM_SUPPLY("DAI 1 Out Power",
2397 R_PWRM2, FB_PWRM2_I2S1OPU, 0, NULL, 0),
2398 SND_SOC_DAPM_SUPPLY("DAI 3 In Power",
2399 R_PWRM2, FB_PWRM2_I2S3IPU, 0, NULL, 0),
2400 SND_SOC_DAPM_SUPPLY("DAI 2 In Power",
2401 R_PWRM2, FB_PWRM2_I2S2IPU, 0, NULL, 0),
2402 SND_SOC_DAPM_SUPPLY("DAI 1 In Power",
2403 R_PWRM2, FB_PWRM2_I2S1IPU, 0, NULL, 0),
2404 /* R_PWRM3 PG 0 ADDR 0x36 */
2405
[RFC PATCH] ASoC: TSCS454: tscs454_set_sysclk() can be static
Fixes: 3bed8970d09a ("ASoC: TSCS454: Add Support")
Signed-off-by: kbuild test robot
---
tscs454.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/tscs454.c b/sound/soc/codecs/tscs454.c
index 8d8cac3..d7e4f7c 100644
--- a/sound/soc/codecs/tscs454.c
+++ b/sound/soc/codecs/tscs454.c
@@ -2626,7 +2626,7 @@ static struct snd_soc_dapm_route const tscs454_intercon[]
= {
};
/* This is used when BCLK is sourcing the PLLs */
-int tscs454_set_sysclk(struct snd_soc_dai *dai, int clk_id, unsigned int freq,
+static int tscs454_set_sysclk(struct snd_soc_dai *dai, int clk_id, unsigned
int freq,
int dir)
{
struct snd_soc_component *component = dai->component;
Re: [git pull] Input updates for v4.17-rc7
On Tue, May 29, 2018 at 10:21:53PM -0500, Linus Torvalds wrote: > On Tue, May 29, 2018 at 6:38 PM Dmitry Torokhov > wrote: > > > We are switching a bunch of > > Lenovo devices with Synaptics touchpads from PS/2 emulation over to > > native RMI/SMbus. > > > Given that all commits are marked for stable there is no point delaying > > them till next release. > > Hmm. The elan driver stack overrun isn't mentioned, but seems more > fundamental fix than the other changes that just add a few new PnP ID's... I do not think that the Elan problem is actually observable on any hardware with 4.17 at the moment, as it only shows up when switching some Elan devices from PS/2 to SMBus and the patches that do that are slated for the next merge window. I included it in the pull as a precaution. Switching Synaptics to RMI is much more user-visible... Still should have mentioned Elan corruption fix though. Thanks. -- Dmitry
Re: [PATCH 04/11] dt-bindings: spi: Move and adjust the bindings for the fsl-qspi driver
Hi Boris,
On 30.05.2018 17:06, Boris Brezillon wrote:
On Wed, 30 May 2018 15:14:33 +0200
Frieder Schrempf wrote:
Move the documentation of the old SPI NOR driver to the place of the new
SPI memory interface based driver and adjust the content to reflect the
new drivers settings.
Maybe it's better to do that in 2 steps so that people can easily
identify what has changed in the bindings.
Ok, I can split this.
Thanks,
Frieder
Signed-off-by: Frieder Schrempf
---
.../devicetree/bindings/mtd/fsl-quadspi.txt | 65 --
.../devicetree/bindings/spi/spi-fsl-qspi.txt| 69
2 files changed, 69 insertions(+), 65 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
b/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
deleted file mode 100644
index 483e9cf..000
--- a/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
+++ /dev/null
@@ -1,65 +0,0 @@
-* Freescale Quad Serial Peripheral Interface(QuadSPI)
-
-Required properties:
- - compatible : Should be "fsl,vf610-qspi", "fsl,imx6sx-qspi",
-"fsl,imx7d-qspi", "fsl,imx6ul-qspi",
-"fsl,ls1021a-qspi"
-or
-"fsl,ls2080a-qspi" followed by "fsl,ls1021a-qspi",
-"fsl,ls1043a-qspi" followed by "fsl,ls1021a-qspi"
- - reg : the first contains the register location and length,
- the second contains the memory mapping address and length
- - reg-names: Should contain the reg names "QuadSPI" and "QuadSPI-memory"
- - interrupts : Should contain the interrupt for the device
- - clocks : The clocks needed by the QuadSPI controller
- - clock-names : Should contain the name of the clocks: "qspi_en" and "qspi".
-
-Optional properties:
- - fsl,qspi-has-second-chip: The controller has two buses, bus A and bus B.
- Each bus can be connected with two NOR flashes.
- Most of the time, each bus only has one NOR flash
- connected, this is the default case.
- But if there are two NOR flashes connected to the
- bus, you should enable this property.
- (Please check the board's schematic.)
- - big-endian : That means the IP register is big endian
-
-Example:
-
-qspi0: quadspi@40044000 {
- compatible = "fsl,vf610-qspi";
- reg = <0x40044000 0x1000>, <0x2000 0x1000>;
- reg-names = "QuadSPI", "QuadSPI-memory";
- interrupts = <0 24 IRQ_TYPE_LEVEL_HIGH>;
- clocks = < VF610_CLK_QSPI0_EN>,
- < VF610_CLK_QSPI0>;
- clock-names = "qspi_en", "qspi";
-
- flash0: s25fl128s@0 {
-
- };
-};
-
-Example showing the usage of two SPI NOR devices:
-
- {
- pinctrl-names = "default";
- pinctrl-0 = <_qspi2>;
- status = "okay";
-
- flash0: n25q256a@0 {
- #address-cells = <1>;
- #size-cells = <1>;
- compatible = "micron,n25q256a", "jedec,spi-nor";
- spi-max-frequency = <2900>;
- reg = <0>;
- };
-
- flash1: n25q256a@1 {
- #address-cells = <1>;
- #size-cells = <1>;
- compatible = "micron,n25q256a", "jedec,spi-nor";
- spi-max-frequency = <2900>;
- reg = <1>;
- };
-};
diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
new file mode 100644
index 000..0ee9cd8
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
@@ -0,0 +1,69 @@
+* Freescale Quad Serial Peripheral Interface(QuadSPI)
+
+Required properties:
+ - compatible : Should be "fsl,vf610-qspi", "fsl,imx6sx-qspi",
+"fsl,imx7d-qspi", "fsl,imx6ul-qspi",
+"fsl,ls1021a-qspi", "fsl,ls2080a-qspi"
+or
+"fsl,ls1043a-qspi" followed by "fsl,ls1021a-qspi"
+ - reg : the first contains the register location and length,
+ the second contains the memory mapping address and length
+ - reg-names: Should contain the reg names "QuadSPI" and "QuadSPI-memory"
+ - interrupts : Should contain the interrupt for the device
+ - clocks : The clocks needed by the QuadSPI controller
+ - clock-names : Should contain the name of the clocks: "qspi_en" and "qspi".
+
+Optional properties:
+ - big-endian : That means the IP registers format is big endian
+
+Required SPI slave node properties:
+ - reg: There are two buses (A and B) with two chip selects each.
+This encodes to which bus and CS the flash is connected:
+ <0>: Bus A, CS 0
+ <1>: Bus A, CS 1
+ <2>: Bus B, CS 0
+ <3>: Bus B, CS 1
+
+Example:
+
+qspi0: quadspi@40044000 {
+ compatible = "fsl,vf610-qspi";
+ reg = <0x40044000 0x1000>, <0x2000 0x1000>;
+
Re: [PATCH] ASoC: core: Fix return code shown on error for hw_params
On 30/05/18 16:12, Jon Hunter wrote: > The call to hw_params for a component fails the error code is held by > the variable '__ret' but the error message displays the value held by > the variable 'ret'. Fix the return code shown when hw_params fails for > a component. Sorry, just resent this as I realise the description does not read correctly! Jon -- nvpublic
Re: [PATCH v2 1/6] ARM: dra762: hwmod: Add MCAN support
On 30/05/18 17:50, Tony Lindgren wrote:
* Faiz Abbas [180530 14:12]:
From: Lokesh Vutla
Add MCAN hwmod data and register it for dra762 silicons.
Signed-off-by: Lokesh Vutla
Signed-off-by: Faiz Abbas
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 32 +++
1 file changed, 32 insertions(+)
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index 62352d1e6361..a2cd7f865a60 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1355,6 +1355,29 @@ static struct omap_hwmod dra7xx_mailbox13_hwmod = {
},
};
+/*
+ * 'mcan' class
+ *
+ */
+static struct omap_hwmod_class dra76x_mcan_hwmod_class = {
+ .name = "mcan",
+};
+
+/* mcan */
+static struct omap_hwmod dra76x_mcan_hwmod = {
+ .name = "mcan",
+ .class = _mcan_hwmod_class,
+ .clkdm_name = "wkupaon_clkdm",
+ .main_clk = "mcan_clk",
+ .prcm = {
+ .omap4 = {
+ .clkctrl_offs = DRA7XX_CM_WKUPAON_ADC_CLKCTRL_OFFSET,
+ .context_offs = DRA7XX_RM_WKUPAON_ADC_CONTEXT_OFFSET,
+ .modulemode = MODULEMODE_SWCTRL,
+ },
+ },
+};
You should be now able to leave out at least the clkctrl_offs and modulemode
here. Please also check if leaving out clkdm_name and main_clk now works :)
@@ -3818,6 +3841,14 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss2
= {
.user = OCP_USER_MPU,
};
+/* l3_main_1 -> mcan */
+static struct omap_hwmod_ocp_if dra76x_l3_main_1__mcan = {
+ .master = _l3_main_1_hwmod,
+ .slave = _mcan_hwmod,
+ .clk= "l3_iclk_div",
+ .user = OCP_USER_MPU | OCP_USER_SDMA,
+};
I think this we still need though for the clk. Tero, do you have any comments
on what all clocks can now be left out?
For the OCP if part, I think that is still needed until we switch over
to full sysc driver. clkctrl_offs you probably also need because that is
used for mapping the omap_hwmod against a specific clkctrl clock. Those
can be only removed once we are done with hwmod (or figure out some
other way to assign the clkctrl clock to a hwmod.)
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[PATCH RESEND] ASoC: core: Fix return code shown on error for hw_params
When the call to hw_params for a component fails, the error code is held
by the variable '__ret' but the error message displays the value held by
the variable 'ret'. Fix the return code shown when hw_params fails for
a component.
Fixes: b8135864d4d3 ("ASoC: snd_soc_component_driver has snd_pcm_ops")
Signed-off-by: Jon Hunter
---
sound/soc/soc-pcm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 2df4719a84db..9376eb476364 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -956,7 +956,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream
*substream,
if (__ret < 0) {
dev_err(component->dev,
"ASoC: %s hw params failed: %d\n",
- component->name, ret);
+ component->name, __ret);
ret = __ret;
}
}
--
2.7.4
Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd
On 29/05/18 11:04, Ulf Hansson wrote:
> To support devices being partitioned across multiple PM domains, let's
> begin with extending genpd to cope with these kind of configurations.
>
> Therefore, add a new exported function genpd_dev_pm_attach_by_id(), which
> is similar to the existing genpd_dev_pm_attach(), but with the difference
> that it allows its callers to provide an index to the PM domain that it
> wants to attach.
>
> Note that, genpd_dev_pm_attach_by_id() shall only be called by the driver
> core / PM core, similar to how the existing dev_pm_domain_attach() makes
> use of genpd_dev_pm_attach(). However, this is implemented by following
> changes on top.
>
> Because, only one PM domain can be attached per device, genpd needs to
> create a virtual device that it can attach/detach instead. More precisely,
> let the new function genpd_dev_pm_attach_by_id() register a virtual struct
> device via calling device_register(). Then let it attach this device to the
> corresponding PM domain, rather than the one that is provided by the
> caller. The actual attaching is done via re-using the existing genpd OF
> functions.
>
> At successful attachment, genpd_dev_pm_attach_by_id() returns the created
> virtual device, which allows the caller to operate on it to deal with power
> management. Following changes on top, provides more details in this
> regards.
>
> To deal with detaching of a PM domain for the multiple PM domains case,
> let's also extend the existing genpd_dev_pm_detach() function, to cover the
> cleanup of the created virtual device, via make it call device_unregister()
> on it. In this way, there is no need to introduce a new function to deal
> with detach for the multiple PM domain case, but instead the existing one
> is re-used.
>
> Signed-off-by: Ulf Hansson
> ---
>
> Changes in v2:
> - Fixed comments from Jon. Clarified function descriptions
> and changelog.
>
> ---
> drivers/base/power/domain.c | 80 +
> include/linux/pm_domain.h | 8
> 2 files changed, 88 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 2af99bfcbe3c..2b496d79159d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct
> device_node *np)
> }
> EXPORT_SYMBOL_GPL(of_genpd_remove_last);
>
> +static void genpd_release_dev(struct device *dev)
> +{
> + kfree(dev);
> +}
> +
> +static struct bus_type genpd_bus_type = {
> + .name = "genpd",
> +};
> +
> /**
> * genpd_dev_pm_detach - Detach a device from its PM domain.
> * @dev: Device to detach.
> @@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev,
> bool power_off)
>
> /* Check if PM domain can be powered off after removing this device. */
> genpd_queue_power_off_work(pd);
> +
> + /* Unregister the device if it was created by genpd. */
> + if (dev->bus == _bus_type)
> + device_unregister(dev);
> }
>
> static void genpd_dev_pm_sync(struct device *dev)
> @@ -2298,6 +2311,67 @@ int genpd_dev_pm_attach(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> +/**
> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
> + * @dev: Device to attach.
Can you update the description of the above as well?
Thanks
Jon
--
nvpublic
Re: [PATCH v1] MIPS: PCI: Use dev_printk()
On Tue, May 22, 2018 at 08:11:36AM -0500, Bjorn Helgaas wrote: > Use dev_printk() to follow style of other arches. > > I'll merge via the PCI tree unless there are objections. > > --- > > Bjorn Helgaas (1): > MIPS: PCI: Use dev_printk() when possible > > > arch/mips/pci/pci-legacy.c |7 ++- > 1 file changed, 2 insertions(+), 5 deletions(-) Applied with James' ack to pci/misc for v4.18.
[PATCH] ARM: dts: exynos: Add missing CPU clocks to secondary CPUs on Exynos542x
Secondary CPUs should have the same information in DeviceTree as booting
CPU from both correctness point of view and for possible hotplug
scenarios.
Suggested-by: Viresh Kumar
Signed-off-by: Krzysztof Kozlowski
---
arch/arm/boot/dts/exynos5420-cpus.dtsi | 6 ++
arch/arm/boot/dts/exynos5422-cpus.dtsi | 8 +++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/exynos5420-cpus.dtsi
b/arch/arm/boot/dts/exynos5420-cpus.dtsi
index a8e449471304..0ee6e92a3c29 100644
--- a/arch/arm/boot/dts/exynos5420-cpus.dtsi
+++ b/arch/arm/boot/dts/exynos5420-cpus.dtsi
@@ -38,6 +38,7 @@
device_type = "cpu";
compatible = "arm,cortex-a15";
reg = <0x1>;
+ clocks = < CLK_ARM_CLK>;
clock-frequency = <18>;
cci-control-port = <_control1>;
operating-points-v2 = <_a15_opp_table>;
@@ -49,6 +50,7 @@
device_type = "cpu";
compatible = "arm,cortex-a15";
reg = <0x2>;
+ clocks = < CLK_ARM_CLK>;
clock-frequency = <18>;
cci-control-port = <_control1>;
operating-points-v2 = <_a15_opp_table>;
@@ -60,6 +62,7 @@
device_type = "cpu";
compatible = "arm,cortex-a15";
reg = <0x3>;
+ clocks = < CLK_ARM_CLK>;
clock-frequency = <18>;
cci-control-port = <_control1>;
operating-points-v2 = <_a15_opp_table>;
@@ -83,6 +86,7 @@
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x101>;
+ clocks = < CLK_KFC_CLK>;
clock-frequency = <10>;
cci-control-port = <_control0>;
operating-points-v2 = <_a7_opp_table>;
@@ -94,6 +98,7 @@
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x102>;
+ clocks = < CLK_KFC_CLK>;
clock-frequency = <10>;
cci-control-port = <_control0>;
operating-points-v2 = <_a7_opp_table>;
@@ -105,6 +110,7 @@
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x103>;
+ clocks = < CLK_KFC_CLK>;
clock-frequency = <10>;
cci-control-port = <_control0>;
operating-points-v2 = <_a7_opp_table>;
diff --git a/arch/arm/boot/dts/exynos5422-cpus.dtsi
b/arch/arm/boot/dts/exynos5422-cpus.dtsi
index 7c130a00d1a8..e4a5857c135f 100644
--- a/arch/arm/boot/dts/exynos5422-cpus.dtsi
+++ b/arch/arm/boot/dts/exynos5422-cpus.dtsi
@@ -37,6 +37,7 @@
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x101>;
+ clocks = < CLK_KFC_CLK>;
clock-frequency = <10>;
cci-control-port = <_control0>;
operating-points-v2 = <_a7_opp_table>;
@@ -48,6 +49,7 @@
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x102>;
+ clocks = < CLK_KFC_CLK>;
clock-frequency = <10>;
cci-control-port = <_control0>;
operating-points-v2 = <_a7_opp_table>;
@@ -59,6 +61,7 @@
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x103>;
+ clocks = < CLK_KFC_CLK>;
clock-frequency = <10>;
cci-control-port = <_control0>;
operating-points-v2 = <_a7_opp_table>;
@@ -69,8 +72,8 @@
cpu4: cpu@0 {
device_type = "cpu";
compatible = "arm,cortex-a15";
- clocks = < CLK_ARM_CLK>;
reg = <0x0>;
+ clocks = < CLK_ARM_CLK>;
clock-frequency = <18>;
cci-control-port = <_control1>;
operating-points-v2 = <_a15_opp_table>;
@@ -82,6 +85,7 @@
device_type = "cpu";
compatible = "arm,cortex-a15";
reg = <0x1>;
+ clocks = < CLK_ARM_CLK>;
clock-frequency = <18>;
Re: [PATCH v1 0/3] xtensa/PCI: Trivial cleanup
On Tue, May 22, 2018 at 08:10:07AM -0500, Bjorn Helgaas wrote: > Remove some dead code, make things static, use dev_printk() to follow style > of other arches. > > I'll merge these via the PCI tree unless there are objections. > > --- > > Bjorn Helgaas (3): > xtensa/PCI: Remove dead code > xtensa/PCI: Make variables static > xtensa/PCI: Use dev_printk() when possible > > > arch/xtensa/include/asm/pci.h |2 - > arch/xtensa/kernel/pci.c | 69 > ++--- > 2 files changed, 4 insertions(+), 67 deletions(-) I applied these to pci/misc for v4.18.
[PATCH 1/1] pwm: fsl-ftm: Support the new version of FTM block on i.MX8x
On the new i.MX8x SoC family, the following changes were made on the FTM
block:
1. Need to enable the IPG clock before accessing any FTM registers. Because
the IPG clock is not an option for FTM counter clock source, it can't be
used as the ftm_sys clock.
2. An additional PWM enable bit was added for each PWM channel in register
FTM_SC[16:23]. It supports 8 channels. Bit16 is for channel 0, and bit23
for channel 7.
As the IP version information can not be obtained in any of the FTM
registers, a property of "fsl,has-pwmen-bits" is added in the ftm pwm
device node. If it has the property, the driver set the PWM enable bit
when a PWM channel is requested.
Signed-off-by: Shenwei Wang
---
drivers/pwm/pwm-fsl-ftm.c | 35 +--
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
index 557b4ea..0426458f 100644
--- a/drivers/pwm/pwm-fsl-ftm.c
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -86,7 +86,9 @@ struct fsl_pwm_chip {
struct regmap *regmap;
int period_ns;
+ bool has_pwmen;
+ struct clk *ipg_clk;
struct clk *clk[FSL_PWM_CLK_MAX];
};
@@ -97,16 +99,31 @@ static inline struct fsl_pwm_chip *to_fsl_chip(struct
pwm_chip *chip)
static int fsl_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
{
+ int ret;
struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
- return clk_prepare_enable(fpc->clk[FSL_PWM_CLK_SYS]);
+ ret = clk_prepare_enable(fpc->ipg_clk);
+
+ if ((!ret) && (fpc->has_pwmen)) {
+ mutex_lock(>lock);
+ regmap_update_bits(fpc->regmap, FTM_SC,
+ BIT(pwm->hwpwm + 16), BIT(pwm->hwpwm + 16));
+ mutex_unlock(>lock);
+ }
+
+ return ret;
}
static void fsl_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
- clk_disable_unprepare(fpc->clk[FSL_PWM_CLK_SYS]);
+ if (fpc->has_pwmen) {
+ mutex_lock(>lock);
+ regmap_update_bits(fpc->regmap, FTM_SC, BIT(pwm->hwpwm + 16),
0);
+ mutex_unlock(>lock);
+ }
+ clk_disable_unprepare(fpc->ipg_clk);
}
static int fsl_pwm_calculate_default_ps(struct fsl_pwm_chip *fpc,
@@ -363,7 +380,7 @@ static int fsl_pwm_init(struct fsl_pwm_chip *fpc)
{
int ret;
- ret = clk_prepare_enable(fpc->clk[FSL_PWM_CLK_SYS]);
+ ret = clk_prepare_enable(fpc->ipg_clk);
if (ret)
return ret;
@@ -371,7 +388,7 @@ static int fsl_pwm_init(struct fsl_pwm_chip *fpc)
regmap_write(fpc->regmap, FTM_OUTINIT, 0x00);
regmap_write(fpc->regmap, FTM_OUTMASK, 0xFF);
- clk_disable_unprepare(fpc->clk[FSL_PWM_CLK_SYS]);
+ clk_disable_unprepare(fpc->ipg_clk);
return 0;
}
@@ -428,6 +445,10 @@ static int fsl_pwm_probe(struct platform_device *pdev)
return PTR_ERR(fpc->clk[FSL_PWM_CLK_SYS]);
}
+ fpc->ipg_clk = devm_clk_get(>dev, "ipg");
+ if (IS_ERR(fpc->ipg_clk))
+ fpc->ipg_clk = fpc->clk[FSL_PWM_CLK_SYS];
+
fpc->clk[FSL_PWM_CLK_FIX] = devm_clk_get(fpc->chip.dev, "ftm_fix");
if (IS_ERR(fpc->clk[FSL_PWM_CLK_FIX]))
return PTR_ERR(fpc->clk[FSL_PWM_CLK_FIX]);
@@ -446,6 +467,8 @@ static int fsl_pwm_probe(struct platform_device *pdev)
fpc->chip.of_pwm_n_cells = 3;
fpc->chip.base = -1;
fpc->chip.npwm = 8;
+ fpc->has_pwmen = of_property_read_bool(pdev->dev.of_node,
+ "fsl,ftm-has-pwmen-bits");
ret = pwmchip_add(>chip);
if (ret < 0) {
@@ -480,7 +503,7 @@ static int fsl_pwm_suspend(struct device *dev)
if (!test_bit(PWMF_REQUESTED, >flags))
continue;
- clk_disable_unprepare(fpc->clk[FSL_PWM_CLK_SYS]);
+ clk_disable_unprepare(fpc->ipg_clk);
if (!pwm_is_enabled(pwm))
continue;
@@ -503,7 +526,7 @@ static int fsl_pwm_resume(struct device *dev)
if (!test_bit(PWMF_REQUESTED, >flags))
continue;
- clk_prepare_enable(fpc->clk[FSL_PWM_CLK_SYS]);
+ clk_prepare_enable(fpc->ipg_clk);
if (!pwm_is_enabled(pwm))
continue;
--
2.9.2
Re: [RFC/RFT] [PATCH 02/10] cpufreq: intel_pstate: Conditional frequency invariant accounting
Hi Peter, maybe you missed this previous my response: 20180518133353.GO30654@e110439-lin ? Would like to have your tought about the concept of "transient maximum capacity" I was describing... On 18-May 14:33, Patrick Bellasi wrote: > On 18-May 13:29, Peter Zijlstra wrote: > > On Fri, May 18, 2018 at 11:57:42AM +0100, Patrick Bellasi wrote: > > > Thus, my simple (maybe dumb) questions are: > > > - why can't we just fold turbo boost frequency into the existing concepts? > > > - what are the limitations of such a "simple" approach? > > > > Perhaps... but does this not further complicate the whole capacity vs > > util thing we already have in say the misfit patches? > > Not sure about that... > > > And the util_fits_capacity() thing from the EAS ones. > > In this case instead, if we can track somehow (not saying we can) > what is the currently available "transient maximum capacity"... > then a util_fits_capacity() should just look at that. > > If the transient capacity is already folded into cpu_capacity, as it > is now for RT and IRQ pressure, then likely we don't have to change > anything. > > > The thing is, we either need to dynamically scale the util or the > > capacity or both. I think for Thermal there are patches out there that > > drop the capacity. > > Not sure... but I would feel more comfortable by something which caps > the maximum capacity. Meaning, eventually you can fill up the maximum > possible capacity only "up to" a given value, because of thermal or other > reasons most of the scheduler maybe doesn't even have to know why? > > > But we'd then have to do the same for turbo/vector and all the other > > stuff as well. Otherwise we risk things like running at low U with 0% > > idle and not triggering the tipping point between eas and regular > > balancing. > > Interacting with the tipping point and/or OPP changes is indeed an > interesting side of the problem I was not considering so far... > > But again, the tipping point could not be defined as a delta > with respect to the "transient maximum capacity" ? > > > So either way around we need to know the 'true' max, either to fudge > > util or to fudge capacity. > > Right, but what I see from a concepts standpoint is something like: > > +--+--+ cpu_capacity_orig (CONSTANT at boot time) > | | | > | | | HW generated constraints > | v | > +-+ cpu_capacity_max (depending on thermal/turbo boost) > | | | > | | | SW generated constraints > | v | > +-+ cpu_capacity (depending on RT/IRQ pressure) > | | | > | | | tipping point delta > +--v--+ > | | Energy Aware mode available capacity > +-+ > > Where all the wkp/lb heuristics are updated to properly consider the > cpu_capacity_max metrics whenever it comes to know what is the max > speed we can reach now on a CPU. > > > And I'm not sure we can know in some of these cases :/ > > Right, this schema will eventually work only under the hypothesis that > "somehow" we can update cpu_capacity_max from HW events. > > Not entirely sure that's possible and/or at which time granularity on > all different platforms. > > > And while Vincent's patches might have been inspired by another problem, > > they do have the effect of always allowing util to go to 1, which is > > nice for this. > > Sure, that's a nice point, but still I have the feeling that always > reaching u=1 can defeat other interesting properties of a task, > For example, comparing task requirements in different CPUs and/or at > different times, which plays a big role for energy aware task > placement decisions. > > -- > #include > > Patrick Bellasi -- #include Patrick Bellasi
Re: LKMM litmus test for Roman Penyaev's rcu-rr
On Wed, May 30, 2018 at 9:29 AM Alan Stern wrote: > > > > Can't we simplify the whole sequence as basically > > > > A > > if (!B) > > D > > > > for that "not B" case, and just think about that. IOW, let's ignore the > > whole "not executed" code. > Your listing is slightly misleading. No. You're confused. You're confused because you're conflating two *entirely* different things. You're conflating the static source code with the dynamic execution. They are NOT THE SAME. It really should be: > A > if (!B) > ; // NOP > D No it really really shouldn't. There are two completely different situations: (1) there is the source code: A if (B) C D where C contains a barrier, and B depends on A and is not statically determinable. In the source code, 'D' looks unconditional BUT IT DAMN WELL IS NOT. It's not unconditional - it's just done in both conditions! That's a big big difference. > In other words, D should be beyond the end of the "if" statement, not > inside one of the branches. You're just completely confused. What you are stating makes no sense at all. Seriously, your reading of the code is entirely monsenscal, and seems to be about syntax, not about semantics. Which is crazy. Lookie here, you can change the syntactic model of that code to just be A if (B) C D else D and that code obviously has the EXACT SAME SEMANTICS. So if you get hung up on trivial syntactic issues, you are by definition confused, and your tool is garbage. You're doing memory ordering analysis, not syntax parsing, for chrissake! > At run time, of course, it doesn't matter; > CPUs don't try to detect where the two branches of an "if" recombine. > (Leaving aside issues like implementing an "if" as a move-conditional.) You cannot do it as a move-conditional, since that code generation would have been buggy shit, exactly because of C. But that's a code generation issue, not a run-time decision. So at run-time, the code ends up being A if (!B) D and D cannot be written before A has been read, because B depends on A, and you cannot expose specutive writes before the preconditions have been evaluated. > Remember, the original code was: > A > if (!B) > C > D > So the execution of D is _not_ conditional, and it doesn't depend on A > or B. (Again, CPUs don't make this distinction, but compilers do.) Again, the above is nothing but confused bullshit. D depends on B, which depends on A. Really. Really really. Anybody - or any tool - that doesn't see that is fundamentally wrong, and has been confused by syntax. A *compiler* will very much also make that distinction. If it doesn't make that distinction, it's not a compiler, it's a buggy piece of shit. Because semantics matter. Think about it. Linus
[PATCH v1 3/4] clk: tegra20: Turn EMC clock gate into divider
Kernel should never gate the EMC clock as it causes immediate lockup, so
removing clk-gate functionality doesn't affect anything. Turning EMC clk
gate into divider allows to implement glitch-less EMC scaling, avoiding
reparenting to a backup clock.
Signed-off-by: Dmitry Osipenko
---
drivers/clk/tegra/clk-tegra20.c | 39 -
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index cc857d4d4a86..189dbc86ab71 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -578,7 +578,6 @@ static struct tegra_clk tegra20_clks[tegra_clk_max]
__initdata = {
[tegra_clk_afi] = { .dt_id = TEGRA20_CLK_AFI, .present = true },
[tegra_clk_fuse] = { .dt_id = TEGRA20_CLK_FUSE, .present = true },
[tegra_clk_kfuse] = { .dt_id = TEGRA20_CLK_KFUSE, .present = true },
- [tegra_clk_emc] = { .dt_id = TEGRA20_CLK_EMC, .present = true },
};
static unsigned long tegra20_clk_measure_input_freq(void)
@@ -799,19 +798,18 @@ static struct tegra_periph_init_data
tegra_periph_nodiv_clk_list[] = {
TEGRA_INIT_DATA_NODIV("disp2", mux_pllpdc_clkm, CLK_SOURCE_DISP2, 30,
2, 26, 0, TEGRA20_CLK_DISP2),
};
-static void __init tegra20_periph_clk_init(void)
+static void __init tegra20_emc_clk_init(void)
{
- struct tegra_periph_init_data *data;
+ u32 emc_reg = readl_relaxed(clk_base + CLK_SOURCE_EMC);
+ u32 use_pllm_ud = BIT(29);
struct clk *clk;
- unsigned int i;
- /* ac97 */
- clk = tegra_clk_register_periph_gate("ac97", "pll_a_out0",
- TEGRA_PERIPH_ON_APB,
- clk_base, 0, 3, periph_clk_enb_refcnt);
- clks[TEGRA20_CLK_AC97] = clk;
+ if (emc_reg & use_pllm_ud) {
+ pr_err("%s: un-divided PllM_out0 used as clock source\n",
+ __func__);
+ return;
+ }
- /* emc */
clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm,
ARRAY_SIZE(mux_pllmcp_clkm),
CLK_SET_RATE_NO_REPARENT,
@@ -822,6 +820,27 @@ static void __init tegra20_periph_clk_init(void)
_lock);
clks[TEGRA20_CLK_MC] = clk;
+ clk = clk_register_divider(NULL, "emc", "emc_mux", CLK_IS_CRITICAL,
+ clk_base + CLK_SOURCE_EMC, 0, 7,
+ 0, _lock);
+ clks[TEGRA20_CLK_EMC] = clk;
+}
+
+static void __init tegra20_periph_clk_init(void)
+{
+ struct tegra_periph_init_data *data;
+ struct clk *clk;
+ unsigned int i;
+
+ /* ac97 */
+ clk = tegra_clk_register_periph_gate("ac97", "pll_a_out0",
+ TEGRA_PERIPH_ON_APB,
+ clk_base, 0, 3, periph_clk_enb_refcnt);
+ clks[TEGRA20_CLK_AC97] = clk;
+
+ /* emc */
+ tegra20_emc_clk_init();
+
/* dsi */
clk = tegra_clk_register_periph_gate("dsi", "pll_d", 0, clk_base, 0,
48, periph_clk_enb_refcnt);
--
2.17.0
Re: [PATCH 05/11] ARM: dts: Reflect change of FSL QSPI driver and remove unused properties
On Wed, 30 May 2018 15:14:34 +0200
Frieder Schrempf wrote:
> The FSL QSPI driver was moved to the SPI framework and it now
> acts as a SPI controller. Therefore the subnodes need to set
> spi-[rx/tx]-bus-width = <4>, so quad mode is used just as before.
We should try to keep the current behavior even when
spi-[rx/tx]-bus-width are not defined. How about considering
spi-[rx/tx]-bus-width as board constraints and then let the core pick
the best mode based on these constraints plus the SPI NOR chip
limitations.
>
> Also the properties 'bus-num', 'fsl,spi-num-chipselects' and
> 'fsl,spi-flash-chipselects' were never read by the driver and
> can be removed.
>
> The 'reg' properties are adjusted to reflect the what bus and
> chipselect the flash is connected to, as the new driver needs
> this information.
>
> The property 'fsl,qspi-has-second-chip' is not needed anymore
> and will be removed after the old driver was disabled to avoid
> breaking ls1021a-moxa-uc-8410a.dts.
>
> Signed-off-by: Frieder Schrempf
> ---
> arch/arm/boot/dts/imx6sx-sdb-reva.dts | 8 ++--
> arch/arm/boot/dts/imx6sx-sdb.dts| 8 ++--
> arch/arm/boot/dts/imx6ul-14x14-evk.dtsi | 2 ++
> arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts | 5 ++---
> 4 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6sx-sdb-reva.dts
> b/arch/arm/boot/dts/imx6sx-sdb-reva.dts
> index e3533e7..1a6f680 100644
> --- a/arch/arm/boot/dts/imx6sx-sdb-reva.dts
> +++ b/arch/arm/boot/dts/imx6sx-sdb-reva.dts
> @@ -131,13 +131,17 @@
> #size-cells = <1>;
> compatible = "spansion,s25fl128s", "jedec,spi-nor";
> spi-max-frequency = <6600>;
> + spi-rx-bus-width = <4>;
> + spi-tx-bus-width = <4>;
> };
>
> - flash1: s25fl128s@1 {
> - reg = <1>;
> + flash1: s25fl128s@2 {
> + reg = <2>;
Hm, you're breaking backward compat here. Can we try to re-use the
old numbering scheme instead of patching all DTs?
Re: [PATCH v2 0/6] mfd/regulator/clk: bd71837: ROHM BD71837 PMIC driver
Quoting Lee Jones (2018-05-30 04:16:49) > On Tue, 29 May 2018, Matti Vaittinen wrote: > > > Hello, > > > > On Tue, May 29, 2018 at 08:39:58AM +0100, Lee Jones wrote: > > > On Mon, 28 May 2018, Matti Vaittinen wrote: > > > > > > > Patch series adding support for ROHM BD71837 PMIC. > > > FYI, this patch-set is going to be difficult to manage since it was > > > not sent 'threaded'. > > > > > > If/when you send a subsequent version, could you please ensure you > > > send the set threaded so the patches keep in relation to one another > > > as they are reviewed? > > > > Thanks for the guidance. I have not sent so many patches to community so > > I am grateful also from all the practical tips =) Just one slight problem. > > I have only seen emails being threaded when one is replying to an email. > > So how should I send my patches in same thread? Just send first one and > > then send subsequent patches as replies? > > > > I just killed some unused definitions and one unused variable from the > > code so I am about to send new version. I'll try doing that as a threaded > > series and resend all the patches as v3. > > You don't need to do this manually. > > Just use `git send-email` with the correct arguments. > I usually send with 'git send-email *.patch' so that git can do the threading for me. Looks like these patches were sent with Mutt though, so perhaps 'git format-patch | git imap-send' was used without the --thread option on format-patch.
Re: [PATCH v7 3/7] drivers/i2c: Add port structure to FSI algorithm
On 05/29/2018 06:19 PM, Andy Shevchenko wrote:
On Wed, May 30, 2018 at 1:24 AM, Eddie James wrote:
From: "Edward A. James"
Add and initialize I2C adapters for each port on the FSI-attached I2C
master. Ports for each master are defined in the devicetree.
+#include
+static int fsi_i2c_set_port(struct fsi_i2c_port *port)
+{
+ int rc;
+ struct fsi_device *fsi = port->master->fsi;
+ u32 mode, dummy = 0;
+ u16 old_port;
+
+ rc = fsi_i2c_read_reg(fsi, I2C_FSI_MODE, );
+ if (rc)
+ return rc;
+
+ old_port = GETFIELD(I2C_MODE_PORT, mode);
+
+ if (old_port != port->port) {
Why not simple
if (port->port == GETFIELD())
return 0;
?
+ /* reset engine when port is changed */
+ rc = fsi_i2c_write_reg(fsi, I2C_FSI_RESET_ERR, );
+ if (rc)
+ return rc;
+ }
+ return rc;
It's hardly would be non-zero, right?
Sorry, misunderstood your comment here. You are correct, it can only be
zero.
Thanks,
Eddie
+}
static int fsi_i2c_probe(struct device *dev)
{
Isn't below somehow repeats of_i2c_register_devices() ?
Why not to use it?
+ /* Add adapter for each i2c port of the master. */
+ for_each_available_child_of_node(dev->of_node, np) {
+ rc = of_property_read_u32(np, "reg", _no);
+ if (rc || port_no > USHRT_MAX)
+ continue;
+
+ port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+ if (!port)
+ break;
+
+ port->master = i2c;
+ port->port = port_no;
+
+ port->adapter.owner = THIS_MODULE;
+ port->adapter.dev.of_node = np;
+ port->adapter.dev.parent = dev;
+ port->adapter.algo = _i2c_algorithm;
+ port->adapter.algo_data = port;
+
+ snprintf(port->adapter.name, sizeof(port->adapter.name),
+"i2c_bus-%u", port_no);
+
+ rc = i2c_add_adapter(>adapter);
+ if (rc < 0) {
+ dev_err(dev, "Failed to register adapter: %d\n", rc);
+ devm_kfree(dev, port);
This hurts my eyes. Why?!
+ continue;
+ }
+
+ list_add(>list, >ports);
+ }
+
dev_set_drvdata(dev, i2c);
return 0;
}
+ if (!list_empty(>ports)) {
My gosh, this is done already in list_for_each*()
+ list_for_each_entry(port, >ports, list) {
+ i2c_del_adapter(>adapter);
+ }
+ }
Re: [RFT v3 1/4] perf cs-etm: Generate branch sample for missed packets
On Wed, May 30, 2018 at 11:39:00PM +0800, Leo Yan wrote:
> Hi Mike,
>
> On Wed, May 30, 2018 at 04:04:34PM +0100, Mike Leach wrote:
>
> [...]
>
> > >>> + /* Generate sample for exception packet */
> > >>> + if (etmq->prev_packet->exc == true)
> > >>> + generate_sample = true;
> > >>
> > >>
> > >> Please don't do that. Exception packets have a type of their own and can
> > >> be
> > >> added to the decoder packet queue the same way INST_RANGE and TRACE_ON
> > >> packets
> > >> are. Moreover exception packet containt an address that, if I'm reading
> > >> the
> > >> documenation properly, can be used to keep track of instructions that
> > >> were
> > >> executed between the last address of the previous range packet and the
> > >> address
> > >> executed just before the exception occurred. Mike and Rob will have to
> > >> confirm
> > >> this as the decoder may be doing all that hard work for us.
> > >>
> >
> > clarification on the exception packets
> >
> > The Opencsd output exception packet gives you the exception number,
> > and optionally the preferred return address. If this address is
> > present does depend a lot on the underlying protocol - will normally
> > be there with ETMv4.
> > Exceptions are marked differently in the underlying protocol - the
> > OCSD packets abstract away these differences.
> >
> > consider the code:
> >
> > 0x1000:
> > 0x1100: BR 0x2000
> >
> > 0x2000:
> > 0x2020 BZ r4
> >
> > Without an exception this would result in the packets
> >
> > OCSD_RANGE(0x1000,0x1104, Last instr type=Br, taken) // recall that
> > range packets have start addr inclusive, end addr exclusive.
> > OCSD_RANGE(0x2000,0x2024, Last instr type=Br, > depends on condition>
> >
> > Now consider an exception occurring before the BR 0x2000
> >
> > this will result in:-
> > OCSD_RANGE(0x1000, 0x1100, Last instr type=Other)
> > OCSD_EXECEPTION(IRQ, ret-addr 0x1100)
> > OCSD_RANGE(IRQ_START, IRQ_END+4, Last instr type = BR, taken)//
> > this is more likely to have multiple ranges / branches before any
> > return, but simplified here.
> > OCSD_EXCEPTION_RETURN() // present if exception returns are
> > explicitly marked in underlying trace - may not always be depending on
> > circumstances.
> > OCSD_RANGE(0x1100,0x1104, Last=BR, taken) // continue on with short
> > range - just the branch
> > OCSD_RANGE(0x2000,0x2024, Last instr type=Br, > depends on condition>
> >
> > Now consider the exception occurring after the BR, but before any
> > other instructions are executed.
> >
> > OCSD_RANGE(0x1000,0x1104, Last instr type=Br, taken) // recall that
> > range packets have start addr inclusive, end addr exclusive.
> > OCSD_EXECEPTION(IRQ, ret-addr 0x2000) // here the preferred return
> > address is actually the target of the branch.
> > OCSD_RANGE(IRQ_START, IRQ_END+4, Last instr type = BR, taken)//
> > this is more likely to have multiple ranges / branches before any
> > return, but simplified here.
> > OCSD_RANGE(0x2000,0x2024, Last instr type=Br, > depends on condition>
> >
> > So in general it is possible to arrive in the IRQ_START range with the
> > previous packet having been either a taken branch, a not taken branch,
> > or not a branch.
> > Care must be taken - whether AutoFDO or normal trace disassembly not
> > to assume that having the last range packet as a taken branch means
> > that the next range packet is the target, if there is an intervening
> > exception packet.
>
> Thanks a lot for detailed explaination.
>
> IIUC, AutoFDO will not have such issue due every range packet will be
> handled for it. On the other hand, as you remind, the branch samples
> (and its consumer trace disassembler) is very dependent on the flag
> 'last_instr_taken_branch'.
>
> According to your explaination, I think we consider the branch is
> taken for below situations:
>
> - The new coming packet is exception packet (both for exception entry
> and exit packets);
> - The previous packet is expcetion packet;
> - The previous packet is normal range packet with
> 'last_instr_taken_branch' = true;
>
> So I'd like to use below function to demonstrate my understanding for
> exception packets handling. I also will send out one new patch for
> support exception packet for reviewing.
>
> If you have concern or I miss anything, please let me know.
>
> static bool cs_etm__is_taken_branch(struct cs_etm_packet *prev_packet,
> struct cs_etm_packet *packet,)
> {
> /* The branch is taken for normal range packet with taken branch flag
> */
> if (prev_packet->sample_type == CS_ETM_RANGE &&
> prev_packet->last_instr_taken_branch)
> return true;
>
> /* The branch is taken if previous packet is exception packet */
> if (prev_packet->sample_type == CS_ETM_EXCEPTION ||
> prev_packet->sample_type == CS_ETM_EXCEPTION_RET)
>
Re: [PATCH 01/13] block: add bi_blkg to the bio for cgroups
On Tue, May 29, 2018 at 05:17:12PM -0400, Josef Bacik wrote: > From: Josef Bacik > > Currently io.low uses a bi_cg_private to stash its private data for the > blkg, however other blkcg policies may want to use this as well. Since > we can get the private data out of the blkg, move this to bi_blkg in the > bio and make it generic, then we can use bio_associate_blkg() to attach > the blkg to the bio. > > Theoretically we could simply replace the bi_css with this since we can > get to all the same information from the blkg, however you have to > lookup the blkg, so for example wbc_init_bio() would have to lookup and > possibly allocate the blkg for the css it was trying to attach to the > bio. This could be problematic and result in us either not attaching > the css at all to the bio, or falling back to the root blkcg if we are > unable to allocate the corresponding blkg. > > So for now do this, and in the future if possible we could just replace > the bi_css with bi_blkg and update the helpers to do the correct > translation. > > Signed-off-by: Josef Bacik Acked-by: Tejun Heo Thanks. -- tejun
Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
On Wed, May 30, 2018 at 07:54:47AM -0700, Doug Anderson wrote: > On Wed, May 30, 2018 at 3:37 AM, Mark Brown wrote: > > I'm confused as to why we are specifying the maximum current the device > > can deliver in a given mode in the DT - surely that's a fixed property > > of the hardware? > Said another way: you're saying that you'd expect the "max-microamps" > table to have one fewer element than the listed modes? So in the > above example you'd have: No, I'm saying that I don't know why that property exists at all. This sounds like it's intended to be the amount of current the regulator can deliver in each mode which is normally a design property of the silicon. signature.asc Description: PGP signature
Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
Hi, On Wed, May 30, 2018 at 8:48 AM, Mark Brown wrote: > On Wed, May 30, 2018 at 08:34:50AM -0700, Doug Anderson wrote: >> On Wed, May 30, 2018 at 8:02 AM, Mark Brown wrote: > >> > What you're describing sounds like what we should be doing normally, if >> > we're not doing that we should probably be fixing the core. > >> I'm not convinced that this behavior makes sense to move to the core. >> On most regulators I'd expect that when the regulator driver says to >> turn them off that they will output no voltage. The reason RPMh is > > Oh, you mean while the regulator is off... TBH I don't see a huge > problem doing that anyway and just reverting to the bottom of the > constraints when everything gets turned off since we have to see if we > need to adjust the voltage every time we enabled anyway. > >> In any other system when Linux disabled the regulator it wouldn't >> matter that you left it set at 1.4V. Thus RPMh is special and IMO the >> workaround belongs there. > > Without the core doing something the regulator isn't going to get told > that anything updated voltages anyway... I was just suggesting that when the core tells the regulator driver to disable itself that the regulator driver tell RPMh to not only disable itself but also (temporarily) vote for a lower voltage. When the core tells the regulator to re-enable itself then the regulator driver restores the original voltage vote (or applies any vote that might have been attempted while the regulator was disabled). That wouldn't require any regulator core changes. -Doug
Re: [PATCH v2 6/9] PM / Domains: Don't attach devices in genpd with multi PM domains
On 29/05/18 11:04, Ulf Hansson wrote:
> The power-domain DT property may now contain a list of PM domain
> specifiers, which represents that a device are partitioned across multiple
> PM domains. This leads to a new situation in genpd_dev_pm_attach(), as only
> one PM domain can be attached per device.
>
> To remain things simple for the most common configuration, when a single PM
> domain is used, let's treat the multiple PM domain case as being specific.
>
> In other words, let's change genpd_dev_pm_attach() to check for multiple PM
> domains and prevent it from attach any PM domain for this case. Instead,
> leave this to be managed separately, from following changes to genpd.
>
> Cc: Rob Herring
> Cc: devicet...@vger.kernel.org
> Suggested-by: Jon Hunter
> Signed-off-by: Ulf Hansson
> ---
>
> Changes in v2:
> - Minor update to changelog to mention "PM domain specifiers" rather
> than a "list of phandles".
>
> ---
> drivers/base/power/domain.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 7ebf7993273a..12a20f21974d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2229,10 +2229,10 @@ static void genpd_dev_pm_sync(struct device *dev)
> * attaches the device to retrieved pm_domain ops.
> *
> * Returns 1 on successfully attached PM domain, 0 when the device don't
> need a
> - * PM domain or a negative error code in case of failures. Note that if a
> - * power-domain exists for the device, but it cannot be found or turned on,
> - * then return -EPROBE_DEFER to ensure that the device is not probed and to
> - * re-try again later.
> + * PM domain or when multiple power-domains exists for it, else a negative
> error
> + * code. Note that if a power-domain exists for the device, but it cannot be
> + * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> + * not probed and to re-try again later.
> */
> int genpd_dev_pm_attach(struct device *dev)
> {
> @@ -2243,10 +2243,18 @@ int genpd_dev_pm_attach(struct device *dev)
> if (!dev->of_node)
> return 0;
>
> + /*
> + * Devices with multiple PM domains must be attached separately, as we
> + * can only attach one PM domain per device.
> + */
> + if (of_count_phandle_with_args(dev->of_node, "power-domains",
> +"#power-domain-cells") != 1)
> + return 0;
> +
> ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
> "#power-domain-cells", 0, _args);
> if (ret < 0)
> - return 0;
> + return ret;
>
> mutex_lock(_list_lock);
> pd = genpd_get_from_provider(_args);
>
Acked-by: Jon Hunter
Tested-by: Jon Hunter
Cheers
Jon
--
nvpublic
Re: [PATCH v5 03/10] cpufreq/schedutil: add rt utilization tracking
Hi Vincent,
On Friday 25 May 2018 at 15:12:24 (+0200), Vincent Guittot wrote:
> Add both cfs and rt utilization when selecting an OPP for cfs tasks as rt
> can preempt and steal cfs's running time.
>
> Signed-off-by: Vincent Guittot
> ---
> kernel/sched/cpufreq_schedutil.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c
> b/kernel/sched/cpufreq_schedutil.c
> index 28592b6..a84b5a5 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -56,6 +56,7 @@ struct sugov_cpu {
> /* The fields below are only needed when sharing a policy: */
> unsigned long util_cfs;
> unsigned long util_dl;
> + unsigned long util_rt;
> unsigned long max;
>
> /* The field below is for single-CPU policies only: */
> @@ -178,14 +179,21 @@ static void sugov_get_util(struct sugov_cpu *sg_cpu)
> sg_cpu->max = arch_scale_cpu_capacity(NULL, sg_cpu->cpu);
> sg_cpu->util_cfs = cpu_util_cfs(rq);
> sg_cpu->util_dl = cpu_util_dl(rq);
> + sg_cpu->util_rt = cpu_util_rt(rq);
> }
>
> static unsigned long sugov_aggregate_util(struct sugov_cpu *sg_cpu)
> {
> struct rq *rq = cpu_rq(sg_cpu->cpu);
> + unsigned long util;
>
> - if (rq->rt.rt_nr_running)
> - return sg_cpu->max;
> + if (rq->rt.rt_nr_running) {
> + util = sg_cpu->max;
So I understand why we want to got to max freq when a RT task is running,
but I think there are use cases where we might want to be more conservative
and use the util_avg of the RT rq instead. The first use case is
battery-powered devices where going to max isn't really affordable from
an energy standpoint. Android, for example, has been using a RT
utilization signal to select OPPs for quite a while now, because going
to max blindly is _very_ expensive.
And the second use-case is thermal pressure. On some modern CPUs, going to
max freq can lead to stringent thermal capping very quickly, at the
point where your CPUs might not have enough capacity to serve your tasks
properly. And that can ultimately hurt the very RT tasks you originally
tried to run fast. In these systems, in the long term, you'd be better off
not asking for more than what you really need ...
So what about having a sched_feature to select between going to max and
using the RT util_avg ? Obviously the default should keep the current
behaviour.
Thanks,
Quentin
Re: [PATCH] FIXUP checkpatch: improve patch recognition
On Wed, 2018-05-30 at 09:51 -0700, Gwendal Grignou wrote: > Fix syntax error in patch. https://lkml.org/lkml/2018/5/29/932
Re: [PATCH INTERNAL 2/3] PCI: iproc: Fix up corrupted PAXC root complex config registers
On Thu, May 17, 2018 at 10:21:31AM -0700, Ray Jui wrote:
> On certain versions of Broadcom PAXC based root complexes, certain
> regions of the configuration space are corrupted. As a result, it
> prevents the Linux PCIe stack from traversing the linked list of the
> capability registers completely and therefore the root complex is
> not advertised as "PCIe capable". This prevents the correct PCIe RID
> from being parsed in the kernel PCIe stack. A correct RID is required
> for mapping to a stream ID from the SMMU or the device ID from the
> GICv3 ITS
>
> This patch fixes up the issue by manually populating the related
> PCIe capabilities based on readings from the PCIe capability structure
>
> Signed-off-by: Ray Jui
> Reviewed-by: Anup Patel
> Reviewed-by: Scott Branden
> ---
> drivers/pci/quirks.c | 95
>
> 1 file changed, 95 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 47dfea0..0cdbd0a 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -2198,6 +2198,101 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM,
> 0x16f0, quirk_paxc_bridge);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd750, quirk_paxc_bridge);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd802, quirk_paxc_bridge);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd804, quirk_paxc_bridge);
> +
> +/*
> + * The PCI capabilities list for certain revisions of Broadcom PAXC root
> + * complexes is incorrectly terminated due to corrupted configuration space
> + * registers in the range of 0x50 - 0x5f
> + *
> + * As a result, the capability list becomes broken and prevent standard PCI
> + * stack from being able to traverse to the PCIe capability structure
> + */
> +static void quirk_paxc_pcie_capability(struct pci_dev *pdev)
> +{
> + int pos, i = 0;
> + u8 next_cap;
> + u16 reg16, *cap;
> + struct pci_cap_saved_state *state;
> +
> + /* bail out if PCIe capability can be found */
> + if (pdev->pcie_cap || pci_find_capability(pdev, PCI_CAP_ID_EXP))
> + return;
> +
> + /* locate the power management capability */
> + pos = pci_find_capability(pdev, PCI_CAP_ID_PM);
> + if (!pos)
> + return;
> +
> + /* bail out if the next capability pointer is not 0x50/0x58 */
> + pci_read_config_byte(pdev, pos + 1, _cap);
> + if (next_cap != 0x50 && next_cap != 0x58)
> + return;
> +
> + /* bail out if we do not terminate at 0x50/0x58 */
> + pos = next_cap;
> + pci_read_config_byte(pdev, pos + 1, _cap);
> + if (next_cap != 0x00)
> + return;
> +
> + /*
> + * On these buggy HW, PCIe capability structure is expected to be at
> + * 0xac and should terminate the list
> + *
> + * Borrow the similar logic from theIntel DH895xCC VFs fixup to save
> + * the PCIe capability list
> + */
> + pos = 0xac;
> + pci_read_config_word(pdev, pos, );
> + if (reg16 == (0x | PCI_CAP_ID_EXP)) {
> + u32 status;
> +
> +#ifndef PCI_EXP_SAVE_REGS
> +#define PCI_EXP_SAVE_REGS 7
> +#endif
> + int size = PCI_EXP_SAVE_REGS * sizeof(u16);
> +
> + pdev->pcie_cap = pos;
> + pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, );
> + pdev->pcie_flags_reg = reg16;
> + pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, );
> + pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD;
Is there any way you can fix this in iproc_pcie_config_read() instead,
by making it notice when we're reading a corrupted part of config
space, and then returning the correct data instead? Is it just the
next capability pointer that's corrupted?
If you could fix it in the config accessor, lspci would automatically
show all the correct data (I think lspci will still show the wrong
data with this patch).
The quirk seems like a maintenance issue because anything that calls
pci_find_capability(pdev, PCI_CAP_ID_EXP)
will get the wrong answer.
> +
> + pdev->cfg_size = PCI_CFG_SPACE_EXP_SIZE;
> + if (pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, ) !=
> + PCIBIOS_SUCCESSFUL || (status == 0x))
> + pdev->cfg_size = PCI_CFG_SPACE_SIZE;
> +
> + if (pci_find_saved_cap(pdev, PCI_CAP_ID_EXP))
> + return;
> +
> + state = kzalloc(sizeof(*state) + size, GFP_KERNEL);
> + if (!state)
> + return;
> +
> + state->cap.cap_nr = PCI_CAP_ID_EXP;
> + state->cap.cap_extended = 0;
> + state->cap.size = size;
> + cap = (u16 *)>cap.data[0];
> + pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, [i++]);
> + pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, [i++]);
> + pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, [i++]);
> +
Re: [PATCH 2/6] PCI: iproc: Add INTx support with better modeling
Hi Andy,
On 5/29/2018 5:59 PM, Andy Shevchenko wrote:
On Wed, May 30, 2018 at 12:58 AM, Ray Jui wrote:
Add PCIe legacy interrupt INTx support to the iProc PCIe driver by
modeling it with its own IRQ domain. All 4 interrupts INTA, INTB, INTC,
INTD share the same interrupt line connected to the GIC in the system,
while the status of each INTx can be obtained through the INTX CSR
register
+ while ((status = iproc_pcie_read_reg(pcie, IPROC_PCIE_INTX_CSR) &
+ SYS_RC_INTX_MASK) != 0) {
+ for_each_set_bit(bit, , PCI_NUM_INTX) {
+ virq = irq_find_mapping(pcie->irq_domain, bit + 1);
+ if (virq)
+ generic_handle_irq(virq);
+ else
+ dev_err(dev, "unexpected INTx%u\n", bit);
+ }
+ }
do {
status = ...;
for_each_set_bit() {
...
}
} while (status);
would look slightly better for my opinion.
Indeed. I agree with you. I'll wait for comments before sending out v2
which will include this improvement.
Thanks,
Ray
[no subject]
unsubscribe linux-kernel
[PATCH v1 1/2] PCI/AER: Decode Error Source Requester ID
From: Bjorn Helgaas
Decode the Requester ID from the AER Error Source Register into domain/
bus/device/function format to match other logging. In cases where the ID
matches the device used for pci_err(), drop the extra ID completely so we
don't print it twice.
Signed-off-by: Bjorn Helgaas
---
drivers/pci/pcie/aer/aerdrv_errprint.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c
b/drivers/pci/pcie/aer/aerdrv_errprint.c
index 21ca5e1b0ded..d7fde8368d81 100644
--- a/drivers/pci/pcie/aer/aerdrv_errprint.c
+++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
@@ -163,17 +163,17 @@ void aer_print_error(struct pci_dev *dev, struct
aer_err_info *info)
int id = ((dev->bus->number << 8) | dev->devfn);
if (!info->status) {
- pci_err(dev, "PCIe Bus Error: severity=%s, type=Unaccessible,
id=%04x(Unregistered Agent ID)\n",
- aer_error_severity_string[info->severity], id);
+ pci_err(dev, "PCIe Bus Error: severity=%s, type=Inaccessible,
(Unregistered Agent ID)\n",
+ aer_error_severity_string[info->severity]);
goto out;
}
layer = AER_GET_LAYER_ERROR(info->severity, info->status);
agent = AER_GET_AGENT(info->severity, info->status);
- pci_err(dev, "PCIe Bus Error: severity=%s, type=%s, id=%04x(%s)\n",
+ pci_err(dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
aer_error_severity_string[info->severity],
- aer_error_layer[layer], id, aer_agent_string[agent]);
+ aer_error_layer[layer], aer_agent_string[agent]);
pci_err(dev, " device [%04x:%04x] error status/mask=%08x/%08x\n",
dev->vendor, dev->device,
@@ -186,7 +186,7 @@ void aer_print_error(struct pci_dev *dev, struct
aer_err_info *info)
out:
if (info->id && info->error_dev_num > 1 && info->id == id)
- pci_err(dev, " Error of this Agent(%04x) is reported first\n",
id);
+ pci_err(dev, " Error of this Agent is reported first\n");
trace_aer_event(dev_name(>dev), (info->status & ~info->mask),
info->severity, info->tlp_header_valid, >tlp);
@@ -194,9 +194,13 @@ void aer_print_error(struct pci_dev *dev, struct
aer_err_info *info)
void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info)
{
- pci_info(dev, "AER: %s%s error received: id=%04x\n",
+ u8 bus = info->id >> 8;
+ u8 devfn = info->id & 0xff;
+
+ pci_info(dev, "AER: %s%s error received: %04x:%02x:%02x.%d\n",
info->multi_error_valid ? "Multiple " : "",
- aer_error_severity_string[info->severity], info->id);
+ aer_error_severity_string[info->severity],
+ pci_domain_nr(dev->bus), bus, devfn >> 3, devfn & 0x7);
}
#ifdef CONFIG_ACPI_APEI_PCIEAER
[PATCH v1 0/2] PCI/AER: Clean up minor logging issues
AER logging printed the plain 16-bit Requester ID straight out of the TLP, which is hard to interpret, e.g., id=00e4 corresponds to what we normally see as 00:1c.4 in dmesg or lspci. Also, there's no need to print the vendor/device ID of the root port reporting an error; we can easily find that from dmesg or lspci. --- Bjorn Helgaas (2): PCI/AER: Decode Error Source Requester ID PCI/AER: Stop printing vendor/device ID drivers/pci/pcie/aer/aerdrv_errprint.c | 23 +-- 1 file changed, 13 insertions(+), 10 deletions(-)
[PATCH v1 2/2] PCI/AER: Stop printing vendor/device ID
From: Bjorn Helgaas The Vendor and Device ID of the root port that raised an AER interrupt is irrelevant and already available via normal enumeration dmesg logging or lspci. Remove the Vendor and Device ID from AER logging. Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aer/aerdrv_errprint.c |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c index d7fde8368d81..16116844531c 100644 --- a/drivers/pci/pcie/aer/aerdrv_errprint.c +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c @@ -175,9 +175,8 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info) aer_error_severity_string[info->severity], aer_error_layer[layer], aer_agent_string[agent]); - pci_err(dev, " device [%04x:%04x] error status/mask=%08x/%08x\n", - dev->vendor, dev->device, - info->status, info->mask); + pci_err(dev, " error status/mask=%08x/%08x\n", info->status, + info->mask); __aer_print_error(dev, info);
[PATCH][next] aio: fix missing break in switch statement
From: Colin Ian King
The addition of the IOCB_CMD_POLL command removed the break
statement for the IOCM_CMD_FDSYNC. From my understanding, this
should not have been removed as the fall-through does not seem
to make sense. Fix this by adding the break back again.
Detected by CoverityScan, CID#1469469 ("Missing break in switch")
Fixes: 2c14fa838cbe ("aio: implement IOCB_CMD_POLL")
Signed-off-by: Colin Ian King
---
fs/aio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/aio.c b/fs/aio.c
index 8274d09d44a2..e0b2f183fa1c 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1785,6 +1785,7 @@ static int io_submit_one(struct kioctx *ctx, struct iocb
__user *user_iocb,
break;
case IOCB_CMD_FDSYNC:
ret = aio_fsync(>fsync, iocb, true);
+ break;
case IOCB_CMD_POLL:
ret = aio_poll(req, iocb);
break;
--
2.17.0
Re: [PATCH] memcg: force charge kmem counter too
On Tue, May 29, 2018 at 1:31 AM, Michal Hocko wrote: > On Mon 28-05-18 10:23:07, Shakeel Butt wrote: >> On Mon, May 28, 2018 at 2:11 AM, Michal Hocko wrote: >> Though is there a precedence where the broken feature is not fixed >> because an alternative is available? > > Well, I can see how breaking GFP_NOFAIL semantic is problematic, on the > other hand we keep saying that kmem accounting in v1 is hard usable and > strongly discourage people from using it. Sure we can add the code which > handles _this_ particular case but that wouldn't make the whole thing > more usable I strongly suspect. Maybe I am wrong and you can provide > some specific examples. Is GFP_NOFAIL that common to matter? > > In any case we should balance between the code maintainability here. > Adding more cruft into the allocator path is not free. > We do not use kmem limits internally and this is something I found through code inspection. If this patch is increasing the cost of code maintainability I am fine with dropping it but at least there should a comment saying that kmem limits are broken and no need fix. Shakeel
Re: [PATCH v1 2/2] PCI/AER: Stop printing vendor/device ID
On Wed, May 30, 2018 at 10:54 AM Bjorn Helgaas wrote: > From: Bjorn Helgaas > The Vendor and Device ID of the root port that raised an AER interrupt is > irrelevant and already available via normal enumeration dmesg logging or > lspci. Er, what is getting printed is not the vendor/device id of the root port but that of the AER source device (the one that root port got an ERR_* message from). In case of fatal AERs, the end point device may become inaccessible so lspci will not be available, and enumeration logs (from boot) may have gotten rolled over. So I think it is still better to print this information here. Just my opinion :-) Thanks, Rajat > Remove the Vendor and Device ID from AER logging. > Signed-off-by: Bjorn Helgaas > --- > drivers/pci/pcie/aer/aerdrv_errprint.c |5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c > index d7fde8368d81..16116844531c 100644 > --- a/drivers/pci/pcie/aer/aerdrv_errprint.c > +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c > @@ -175,9 +175,8 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info) > aer_error_severity_string[info->severity], > aer_error_layer[layer], aer_agent_string[agent]); > - pci_err(dev, " device [%04x:%04x] error status/mask=%08x/%08x\n", > - dev->vendor, dev->device, > - info->status, info->mask); > + pci_err(dev, " error status/mask=%08x/%08x\n", info->status, > + info->mask); > __aer_print_error(dev, info);
Re: [PATCH bpf 1/2] bpf: fix alignment of netns_dev/netns_ino fields in bpf_{map,prog}_info
On Sun, May 27, 2018 at 01:28:42PM +0200, Eugene Syromiatnikov wrote:
> Recent introduction of netns_dev/netns_ino to bpf_map_info/bpf_prog info
> has broken compat, as offsets of these fields are different in 32-bit
> and 64-bit ABIs. One fix (other than implementing compat support in
> syscall in order to handle this discrepancy) is to use __aligned_u64
> instead of __u64 for these fields.
>
> Reported-by: Dmitry V. Levin
> Fixes: 52775b33bb507 ("bpf: offload: report device information about
> offloaded maps")
> Fixes: 675fc275a3a2d ("bpf: offload: report device information for
> offloaded programs")
Reviewed-by: "Dmitry V. Levin"
Cc: # v4.16+
Thanks,
--
ldv
signature.asc
Description: PGP signature
Re: [PATCH v2 5/6] ARM: dts: Add generic interconnect target module node for MCAN
* Faiz Abbas [180530 14:12]:
> The ti-sysc driver provides support for manipulating the idlemodes
> and interconnect level resets.
...
> --- a/arch/arm/boot/dts/dra76x.dtsi
> +++ b/arch/arm/boot/dts/dra76x.dtsi
> @@ -11,6 +11,25 @@
> / {
> compatible = "ti,dra762", "ti,dra7";
>
> + ocp {
> +
> + target-module@0x42c0 {
> + compatible = "ti,sysc-dra7-mcan";
> + ranges = <0x0 0x42c0 0x2000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0x42c01900 0x4>,
> + <0x42c01904 0x4>,
> + <0x42c01908 0x4>;
> + reg-names = "rev", "sysc", "syss";
> + ti,sysc-mask = <(SYSC_OMAP4_SOFTRESET |
> + SYSC_DRA7_MCAN_ENAWAKEUP)>;
> + ti,syss-mask = <1>;
> + clocks = <_clkctrl DRA7_ADC_CLKCTRL 0>;
> + clock-names = "fck";
> + };
> + };
> +
> };
Looks good to me except I think the reset won't do anything currently
with ti-sysc.c unless you specfify also "ti,hwmods" for the module?
Can you please check? It might be worth adding the reset function to
ti-sysc.c for non "ti,hwmods" case and that just might remove the
need for any hwmod code for this module.
Regards,
Tony
Re: [PATCH v2 1/6] ARM: dra762: hwmod: Add MCAN support
* Tero Kristo [180530 15:18]:
> For the OCP if part, I think that is still needed until we switch over to
> full sysc driver. clkctrl_offs you probably also need because that is used
> for mapping the omap_hwmod against a specific clkctrl clock. Those can be
> only removed once we are done with hwmod (or figure out some other way to
> assign the clkctrl clock to a hwmod.)
Hmm might be worth testing. I thought your commit 70f05be32133
("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
already parses the clkctrl from dts?
Regards,
Tony
checkpatch.pl: Perl regression in "improve patch recognition"
Since the commit 9dd0c31c6cc0 ("checkpatch: improve patch
recognition"), the checkpatch.pl in linux-next is only printing a lot
of error messages when started (with and without arguments):
Variable "$clean" is not imported at ./scripts/checkpatch.pl line 6496.
Variable "$clean" is not imported at ./scripts/checkpatch.pl line 6516.
Variable "$cnt_error" is not imported at ./scripts/checkpatch.pl line 6518.
Variable "$cnt_warn" is not imported at ./scripts/checkpatch.pl line 6518.
Variable "$cnt_chk" is not imported at ./scripts/checkpatch.pl line 6519.
Variable "$cnt_lines" is not imported at ./scripts/checkpatch.pl line 6520.
Variable "$clean" is not imported at ./scripts/checkpatch.pl line 6525.
Variable "$clean" is not imported at ./scripts/checkpatch.pl line 6544.
Variable "$clean" is not imported at ./scripts/checkpatch.pl line 6585.
Variable "$clean" is not imported at ./scripts/checkpatch.pl line 6591.
syntax error at ./scripts/checkpatch.pl line 2382, near ") {"
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2420.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2425.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2459.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2491.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2492.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2493.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2730.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2731.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2733.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2744.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2745.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2754.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2760.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2782.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2802.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2808.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2808.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2809.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2820.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2821.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2822.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2823.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2824.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2888.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2889.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2923.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2951.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2953.
Global symbol "$rawline" requires explicit package name (did you
forget to declare "my $rawline"?) at ./scripts/checkpatch.pl line
2966.
Global symbol "$rawline" requires
Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
On Wed, May 30, 2018 at 09:31:55AM -0700, Doug Anderson wrote: > On Wed, May 30, 2018 at 9:13 AM, Mark Brown wrote: > > If we're just going to use the most recently set voltage then hopefully > > the hardware already knew that, and it might not be the lowest available > > voltage if the last consumer to get turned off was holding the voltage > > higher. > To circle back to the original point: the problem is that (IMHO) the > hardware is doing the wrong thing by still counting Linux's vote for a > voltage even though Linux also voted that the regulator should be > disabled. So basically we're working around the hardware by > pretending to vote for a dummy lower voltage whenever Linux wants the > regulator disabled. From Linux's point of view everything works as > normal--we just tell the hardware a falsehood so it doesn't count our > vote for a voltage while the regulator is disabled. Yeah, and I don't think that's unreasonable for the core to do - just drop the voltage to the constraint minimum after it has turned off the regulator, then recheck and raise if needed before it enables again. signature.asc Description: PGP signature
Re: [PATCH v2] platform/chrome: Use to_cros_ec_dev more broadly
Hi Gwendal,
2018-05-30 18:04 GMT+02:00 Gwendal Grignou :
> Move to_cros_ec_dev macro to cros_ec.h and use it when the private ec
> object is needed from device object.
>
> Signed-off-by: Gwendal Grignou
> ---
> Change since v1:
>Remove changes in cros_ec_dev.c to avoid inter-dependencies.
>
> drivers/platform/chrome/cros_ec_lightbar.c | 21 +++--
> drivers/platform/chrome/cros_ec_sysfs.c| 2 --
> drivers/platform/chrome/cros_ec_vbc.c | 9 +++--
> include/linux/mfd/cros_ec.h| 2 ++
> 4 files changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_lightbar.c
> b/drivers/platform/chrome/cros_ec_lightbar.c
> index 6ea79d495aa2..68193bb53383 100644
> --- a/drivers/platform/chrome/cros_ec_lightbar.c
> +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> @@ -170,8 +170,7 @@ static ssize_t version_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> uint32_t version = 0, flags = 0;
> - struct cros_ec_dev *ec = container_of(dev,
> - struct cros_ec_dev, class_dev);
> + struct cros_ec_dev *ec = to_cros_ec_dev(dev);
> int ret;
>
> ret = lb_throttle();
> @@ -193,8 +192,7 @@ static ssize_t brightness_store(struct device *dev,
> struct cros_ec_command *msg;
> int ret;
> unsigned int val;
> - struct cros_ec_dev *ec = container_of(dev,
> - struct cros_ec_dev, class_dev);
> + struct cros_ec_dev *ec = to_cros_ec_dev(dev);
>
> if (kstrtouint(buf, 0, ))
> return -EINVAL;
> @@ -238,8 +236,7 @@ static ssize_t led_rgb_store(struct device *dev, struct
> device_attribute *attr,
> {
> struct ec_params_lightbar *param;
> struct cros_ec_command *msg;
> - struct cros_ec_dev *ec = container_of(dev,
> - struct cros_ec_dev, class_dev);
> + struct cros_ec_dev *ec = to_cros_ec_dev(dev);
> unsigned int val[4];
> int ret, i = 0, j = 0, ok = 0;
>
> @@ -311,8 +308,7 @@ static ssize_t sequence_show(struct device *dev,
> struct ec_response_lightbar *resp;
> struct cros_ec_command *msg;
> int ret;
> - struct cros_ec_dev *ec = container_of(dev,
> - struct cros_ec_dev, class_dev);
> + struct cros_ec_dev *ec = to_cros_ec_dev(dev);
>
> msg = alloc_lightbar_cmd_msg(ec);
> if (!msg)
> @@ -439,8 +435,7 @@ static ssize_t sequence_store(struct device *dev, struct
> device_attribute *attr,
> struct cros_ec_command *msg;
> unsigned int num;
> int ret, len;
> - struct cros_ec_dev *ec = container_of(dev,
> - struct cros_ec_dev, class_dev);
> + struct cros_ec_dev *ec = to_cros_ec_dev(dev);
>
> for (len = 0; len < count; len++)
> if (!isalnum(buf[len]))
> @@ -488,8 +483,7 @@ static ssize_t program_store(struct device *dev, struct
> device_attribute *attr,
> int extra_bytes, max_size, ret;
> struct ec_params_lightbar *param;
> struct cros_ec_command *msg;
> - struct cros_ec_dev *ec = container_of(dev, struct cros_ec_dev,
> - class_dev);
> + struct cros_ec_dev *ec = to_cros_ec_dev(dev);
>
> /*
> * We might need to reject the program for size reasons. The EC
> @@ -599,8 +593,7 @@ static umode_t cros_ec_lightbar_attrs_are_visible(struct
> kobject *kobj,
> struct attribute *a, int n)
> {
> struct device *dev = container_of(kobj, struct device, kobj);
> - struct cros_ec_dev *ec = container_of(dev,
> - struct cros_ec_dev, class_dev);
> + struct cros_ec_dev *ec = to_cros_ec_dev(dev);
> struct platform_device *pdev = to_platform_device(ec->dev);
> struct cros_ec_platform *pdata = pdev->dev.platform_data;
> int is_cros_ec;
> diff --git a/drivers/platform/chrome/cros_ec_sysfs.c
> b/drivers/platform/chrome/cros_ec_sysfs.c
> index 5a6db3fe213a..f34a50121064 100644
> --- a/drivers/platform/chrome/cros_ec_sysfs.c
> +++ b/drivers/platform/chrome/cros_ec_sysfs.c
> @@ -34,8 +34,6 @@
> #include
> #include
>
> -#define to_cros_ec_dev(dev) container_of(dev, struct cros_ec_dev, class_dev)
> -
> /* Accessor functions */
>
> static ssize_t reboot_show(struct device *dev,
> diff --git a/drivers/platform/chrome/cros_ec_vbc.c
> b/drivers/platform/chrome/cros_ec_vbc.c
> index 6d38e6b08334..5356f26bc022 100644
> --- a/drivers/platform/chrome/cros_ec_vbc.c
> +++ b/drivers/platform/chrome/cros_ec_vbc.c
> @@ -29,8 +29,7 @@ static ssize_t vboot_context_read(struct file *filp, struct
> kobject *kobj,
> loff_t pos,
Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
Hi, On Wed, May 30, 2018 at 9:36 AM, Mark Brown wrote: > On Wed, May 30, 2018 at 09:31:55AM -0700, Doug Anderson wrote: >> On Wed, May 30, 2018 at 9:13 AM, Mark Brown wrote: > >> > If we're just going to use the most recently set voltage then hopefully >> > the hardware already knew that, and it might not be the lowest available >> > voltage if the last consumer to get turned off was holding the voltage >> > higher. > >> To circle back to the original point: the problem is that (IMHO) the >> hardware is doing the wrong thing by still counting Linux's vote for a >> voltage even though Linux also voted that the regulator should be >> disabled. So basically we're working around the hardware by >> pretending to vote for a dummy lower voltage whenever Linux wants the >> regulator disabled. From Linux's point of view everything works as >> normal--we just tell the hardware a falsehood so it doesn't count our >> vote for a voltage while the regulator is disabled. > > Yeah, and I don't think that's unreasonable for the core to do - just > drop the voltage to the constraint minimum after it has turned off the > regulator, then recheck and raise if needed before it enables again. Would it do this for all regulators, though? Most regulators are hooked up over a slow i2c bus, so that means that every regulator disable would now do an extra i2c transfer even though for all regulators (other than RPMh) the voltage of a regulator doesn't matter when it's been "disabled' (from Linux's point of view). Hrmmm, I suppose maybe it'd be OK though because for most regulators we'd use "regcache" and most regulators that we enabled/disable a lot are not expected to change voltage in Linux (so the regcache write would hopefully be a noop), so maybe it wouldn't be _that_ inefficient... -Doug
Re: [PATCH v3 1/2] sched/cpufreq: always consider blocked FAIR utilization
On 27-May 11:50, Rafael J. Wysocki wrote:
> On Thu, May 24, 2018 at 4:10 PM, Patrick Bellasi
> wrote:
> > Since the refactoring introduced by:
> >
> >commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support")
> >
> > we aggregate FAIR utilization only if this class has runnable tasks.
> > This was mainly due to avoid the risk to stay on an high frequency just
> > because of the blocked utilization of a CPU not being properly decayed
> > while the CPU was idle.
> >
> > However, since:
> >
> >commit 31e77c93e432 ("sched/fair: Update blocked load when newly idle")
> >
> > the FAIR blocked utilization is properly decayed also for IDLE CPUs.
> >
> > This allows us to use the FAIR blocked utilization as a safe mechanism
> > to gracefully reduce the frequency only if no FAIR tasks show up on a
> > CPU for a reasonable period of time.
> >
> > Moreover, we also reduce the frequency drops of CPUs running periodic
> > tasks which, depending on the task periodicity and the time required
> > for a frequency switch, was increasing the chances to introduce some
> > undesirable performance variations.
> >
> > Reported-by: Vincent Guittot
> > Signed-off-by: Patrick Bellasi
> > Acked-by: Viresh Kumar
> > Acked-by: Vincent Guittot
> > Tested-by: Vincent Guittot
> > Cc: Ingo Molnar
> > Cc: Peter Zijlstra
> > Cc: Rafael J. Wysocki
> > Cc: Vincent Guittot
> > Cc: Viresh Kumar
> > Cc: Joel Fernandes
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux...@vger.kernel.org
>
> Reviewed-by: Rafael J. Wysocki
>
> Or please let me know if you want me to apply this one.
Hi Rafael, seems this patch has already been applied in tip/sched/core.
However is missing your tag above. :/
Dunno if I have / I can to do something about that.
> > ---
> > Changes in v3:
> > - add "Tested-by" and "Acked-by" Vincent tags
> >
> > Changes in v2:
> > - add "Acked-by" Viresh tag
> > ---
> > kernel/sched/cpufreq_schedutil.c | 17 -
> > 1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/kernel/sched/cpufreq_schedutil.c
> > b/kernel/sched/cpufreq_schedutil.c
> > index e13df951aca7..28592b62b1d5 100644
> > --- a/kernel/sched/cpufreq_schedutil.c
> > +++ b/kernel/sched/cpufreq_schedutil.c
> > @@ -183,22 +183,21 @@ static void sugov_get_util(struct sugov_cpu *sg_cpu)
> > static unsigned long sugov_aggregate_util(struct sugov_cpu *sg_cpu)
> > {
> > struct rq *rq = cpu_rq(sg_cpu->cpu);
> > - unsigned long util;
> >
> > - if (rq->rt.rt_nr_running) {
> > - util = sg_cpu->max;
> > - } else {
> > - util = sg_cpu->util_dl;
> > - if (rq->cfs.h_nr_running)
> > - util += sg_cpu->util_cfs;
> > - }
> > + if (rq->rt.rt_nr_running)
> > + return sg_cpu->max;
> >
> > /*
> > +* Utilization required by DEADLINE must always be granted while,
> > for
> > +* FAIR, we use blocked utilization of IDLE CPUs as a mechanism to
> > +* gracefully reduce the frequency when no tasks show up for longer
> > +* periods of time.
> > +*
> > * Ideally we would like to set util_dl as min/guaranteed freq and
> > * util_cfs + util_dl as requested freq. However, cpufreq is not yet
> > * ready for such an interface. So, we only do the latter for now.
> > */
> > - return min(util, sg_cpu->max);
> > + return min(sg_cpu->max, (sg_cpu->util_dl + sg_cpu->util_cfs));
> > }
> >
> > static void sugov_set_iowait_boost(struct sugov_cpu *sg_cpu, u64 time,
> > unsigned int flags)
> > --
> > 2.15.1
> >
--
#include
Patrick Bellasi
Re: [PATCH][next] mailbox: PCC: check for negative count for parse failure checking
On 30/05/18 18:59, Al Stone wrote:
> On 05/30/2018 11:14 AM, Colin King wrote:
>> From: Colin Ian King
>>
>> The function acpi_table_parse_enties_array can potentially return a
>> negative value if parsing fails. Currently the check on the return
>> is not checking for errors, so fix this by adding a -ve check too.
>>
>> Detected by CoverityScan, CID#1469477 ("Improper use of negative value")
>>
>> Fixes: 8f8027c5f935 ("mailbox: PCC: erroneous error message when parsing
>> ACPI PCCT")
>> Signed-off-by: Colin Ian King
>> ---
>> drivers/mailbox/pcc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
>> index fc3c237daef2..87d67922020d 100644
>> --- a/drivers/mailbox/pcc.c
>> +++ b/drivers/mailbox/pcc.c
>> @@ -461,7 +461,7 @@ static int __init acpi_pcc_probe(void)
>> count = acpi_table_parse_entries_array(ACPI_SIG_PCCT,
>> sizeof(struct acpi_table_pcct), proc,
>> ACPI_PCCT_TYPE_RESERVED, MAX_PCC_SUBSPACES);
>> -if (count == 0 || count > MAX_PCC_SUBSPACES) {
>> +if (count <= 0 || count > MAX_PCC_SUBSPACES) {
>> pr_warn("Invalid PCCT: %d PCC subspaces\n", count);
>> return -EINVAL;
>> }
>>
>
> Yup, nice catch. A little paranoid, but we like that in a kernel :). Thanks.
If it can go wrong, it will go wrong, especially with firmware :-)
>
> Reviewed-by: Al Stone
>
Re: [PATCH v1 1/2] PCI/AER: Decode Error Source Requester ID
On Wed, May 30, 2018 at 12:54:15PM -0500, Bjorn Helgaas wrote:
> void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info)
> {
> - pci_info(dev, "AER: %s%s error received: id=%04x\n",
> + u8 bus = info->id >> 8;
> + u8 devfn = info->id & 0xff;
> +
> + pci_info(dev, "AER: %s%s error received: %04x:%02x:%02x.%d\n",
> info->multi_error_valid ? "Multiple " : "",
> - aer_error_severity_string[info->severity], info->id);
> + aer_error_severity_string[info->severity],
> + pci_domain_nr(dev->bus), bus, devfn >> 3, devfn & 0x7);
I think PCI_SLOT(devfn), PCI_FUNC(devfn) is a bit more readable.
Re: [PATCH 13/13] Documentation: add a doc for blk-iolatency
On 05/29/2018 02:17 PM, Josef Bacik wrote: > From: Josef Bacik > > A basic documentation to describe the interface, statistics, and > behavior of io.latency. > > Signed-off-by: Josef Bacik > --- > Documentation/blk-iolatency.txt | 80 > + > 1 file changed, 80 insertions(+) > create mode 100644 Documentation/blk-iolatency.txt > > diff --git a/Documentation/blk-iolatency.txt b/Documentation/blk-iolatency.txt > new file mode 100644 > index ..9dd86f4f64b6 > --- /dev/null > +++ b/Documentation/blk-iolatency.txt > @@ -0,0 +1,80 @@ > +Block IO Latency Controller > + > +Overview > + > + > +This is a cgroup v2 controller for IO workload protection. You provide a > group > +with a latency target, and if the average latency exceeds that target the > +controller will throttle any peers that have a lower latency target than the > +protected workload. > + > +Interface > += > + > +- io.latency. This takes a similar format as the other controllers end above sentence with a period or a colon: > + > + "MAJOR:MINOR target= + > +- io.stat. If the controller is enabled you will see extra stats in io.stat > in > + addition to the normal ones end above sentence with a period. > + > + - depth=. This is the current queue depth for the group. > + - delay=. This is the current delay per task that > + does IO in this group. > + - use_delay=. This is how deep into the delay we currently > + are, the larger this number is the longer it'll take us to get back to > + queue depth > 1. > + - total_lat_avg=. The running average IO latency > + for this group. Running average is generally flawed, but will give an > + admistrator a general idea of the overall latency they can expect for administrator > + their workload on the given disk. > + > +HOWTO > += > + > +The limits are only applied at the peer level in the heirarchy. This means > that > +in the diagram below, only groups A, B, and C will influence eachother, and each other, and > +groups D and F will influence eachother. Group G will influence nobody. each other. > + > + [root] > + / |\ > + A BC > +/ \| > + DF G > + > + > +So the ideal way to configure this is to set io.latency in groups A, B, and > C. > +Generally you do not want to set a value lower than the latency your device > +supports. Experiment to find the value that works best for your workload, > start > +at higher than the expected latency for your device and watch the > total_lat_avg > +value in io.stat for your workload group to get an idea of the latency you > see > +during normal operation. Use this value as a basis for your real setting, > +setting at 10-15% higher than the value in io.stat. Experimentation is key > here > +because total_lat_avg is a running total, so is the "statistics" portion of > +"lies, damned lies, and statistics." > + > +How Throttling Works > + > + > +io.latency is work conserving, so as long as everybody is meeting their > latency > +target the controller doesn't do anything. Once a group starts missing it's its > +target it begins throttling any peer group that has a higher target than > itself. > +This throttling takes 2 forms 2 forms: > + > +- Queue depth throttling. This is the number of outstanding IO's a group is > + allowed to have. We will clamp down relatively quickly, starting at no > limit > + and going all the way down to 1 IO at a time. > + > +- Artificial delay induction. There are certain types of IO that cannot be > + throttled without possibly adversely affecting higher priority groups. > This > + includes swapping and metadata IO. These types of IO are allowed to occur > + normally, however they are "charged" to the originating group. If the > + originating group is being throttled you will see the use_delay and delay > + fields in io.stat increase. The delay value is how many microseconds that > are > + being added to any process that runs in this group. Because this number > can > + grow quite large if there is a lot of swapping or metadata IO occuring we occurring > + limit the individual delay events to 1 second at a time. > + > +Once the victimized group starts meeting it's latency target again it will > start its > +unthrottling any peer groups that were throttled previous. If the victimized previously. > +group simply stops doing
Re: [PATCH] mm: dmapool: Check the dma pool name
On Wed, May 30, 2018 at 08:14:09PM +0800, Baolin Wang wrote: > On 30 May 2018 at 20:01, Matthew Wilcox wrote: > > On Wed, May 30, 2018 at 07:28:43PM +0800, Baolin Wang wrote: > >> It will be crash if we pass one NULL name when creating one dma pool, > >> so we should check the passing name when copy it to dma pool. > > > > NAK. Crashing is the appropriate thing to do. Fix the caller to not > > pass NULL. > > > > If you permit NULL to be passed then you're inviting crashes or just > > bad reporting later when pool->name is printed. > > I think it just prints one NULL pool name. Sometimes the device > doesn't care the dma pool names, so I think we can make code more > solid to valid the passing parameters like other code does. > Or can we add check to return NULL when the passing name is NULL > instead of crashing the kernel? Thanks. No. Fix your driver.
Re: [PATCH 01/11] spi: spi-mem: Extend the SPI mem interface to set a custom memory name
Hi Boris,
On 30.05.2018 16:32, Boris Brezillon wrote:
Hi Frieder,
On Wed, 30 May 2018 15:14:30 +0200
Frieder Schrempf wrote:
When porting (Q)SPI controller drivers from the MTD layer to the SPI
layer, the naming scheme for the memory devices changes. To be able
to keep compatibility with the old drivers naming scheme, a function
is added to the SPI mem interface to let controller drivers set a
custom name for the memory.
Example for the FSL QSPI driver:
Name with the old driver: 21e.qspi,
or with multiple devices: 21e.qspi-0, 21e.qspi-1, ...
Name with the new driver without spi_mem_get_name: spi4.0
Signed-off-by: Frieder Schrempf
---
drivers/spi/spi-mem.c | 25 +
include/linux/spi/spi-mem.h | 3 +++
2 files changed, 28 insertions(+)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index 990770d..5e9af47 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -311,6 +311,31 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct
spi_mem_op *op)
EXPORT_SYMBOL_GPL(spi_mem_exec_op);
/**
+ * spi_mem_get_name() - Let drivers using the SPI mem interface specify a
+ * custom name for the memory to avoid compatibility
+ * issues with ported drivers.
+ * @mem: the SPI memory
+ *
+ * When porting (Q)SPI controller drivers from the MTD layer to the SPI
+ * layer, the naming scheme for the memory devices changes. To be able to
+ * keep compatibility with the old drivers naming scheme, this function can
+ * be used to get a custom name from the controller driver.
+ * If no custom name is available, the name of the SPI device is returned.
+ *
+ * Return: a char array that contains the name for the flash memory
+ */
+const char *spi_mem_get_name(struct spi_mem *mem)
+{
+ struct spi_controller *ctlr = mem->spi->controller;
+
+ if (ctlr->mem_ops && ctlr->mem_ops->get_name)
+ return ctlr->mem_ops->get_name(mem);
Looks like your implementation of ->get_name() in the fsl driver is
allocating a new string each time it's called. I guess other
implementations might be forced to do the same, so maybe it's better to
add a ->name field to struct spi_mem and only call ->get_name() once
when the device is probed. Then spi_mem_get_name() can just be
implemented like this:
const char *spi_mem_get_name(struct spi_mem *mem)
{
return mem->name;
}
Sounds like a good proposal. I will change that.
Thanks,
Frieder
Regards,
Boris
+
+ return dev_name(>spi->dev);
+}
+EXPORT_SYMBOL_GPL(spi_mem_get_name);
+
+/**
* spi_mem_adjust_op_size() - Adjust the data size of a SPI mem operation to
* match controller limitations
* @mem: the SPI memory
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index 4fa34a2..f1912d3 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -178,6 +178,7 @@ struct spi_controller_mem_ops {
const struct spi_mem_op *op);
int (*exec_op)(struct spi_mem *mem,
const struct spi_mem_op *op);
+ const char *(*get_name)(struct spi_mem *mem);
};
/**
@@ -236,6 +237,8 @@ bool spi_mem_supports_op(struct spi_mem *mem,
int spi_mem_exec_op(struct spi_mem *mem,
const struct spi_mem_op *op);
+const char *spi_mem_get_name(struct spi_mem *mem);
+
int spi_mem_driver_register_with_owner(struct spi_mem_driver *drv,
struct module *owner);
[PATCH] ASoC: core: Fix return code shown on error for hw_params
The call to hw_params for a component fails the error code is held by
the variable '__ret' but the error message displays the value held by
the variable 'ret'. Fix the return code shown when hw_params fails for
a component.
Fixes: b8135864d4d3 ("ASoC: snd_soc_component_driver has snd_pcm_ops")
Signed-off-by: Jon Hunter
---
sound/soc/soc-pcm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 2df4719a84db..9376eb476364 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -956,7 +956,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream
*substream,
if (__ret < 0) {
dev_err(component->dev,
"ASoC: %s hw params failed: %d\n",
- component->name, ret);
+ component->name, __ret);
ret = __ret;
}
}
--
2.7.4
[PATCH v1 2/4] ARM: dts: tegra20: Add interrupt to External Memory Controller
Add interrupt entry into the EMC DT node.
Signed-off-by: Dmitry Osipenko
---
arch/arm/boot/dts/tegra20.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 983dd5c14794..3cd3cb28cfd9 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -609,6 +609,7 @@
memory-controller@7000f400 {
compatible = "nvidia,tegra20-emc";
reg = <0x7000f400 0x200>;
+ interrupts = ;
#address-cells = <1>;
#size-cells = <0>;
};
--
2.17.0
Re: [PATCH] pci: mvebu: add MVEBU_MBUS dependency
On Tue, May 29, 2018 at 11:41:51AM +0200, Arnd Bergmann wrote:
> Enabling the PCI_MVEBU driver for compile testing caused a build failure
> on ARM randconfig builds:
>
> drivers/pci/host/pci-mvebu.c: In function 'mvebu_pcie_del_windows':
> drivers/pci/host/pci-mvebu.c:341:3: error: implicit declaration of function
> 'mvebu_mbus_del_window'; did you mean 'mvebu_pcie_del_windows'?
> [-Werror=implicit-function-declaration]
>mvebu_mbus_del_window(base, sz);
>^
>mvebu_pcie_del_windows
> drivers/pci/host/pci-mvebu.c: In function 'mvebu_pcie_add_windows':
> drivers/pci/host/pci-mvebu.c:364:9: error: implicit declaration of function
> 'mvebu_mbus_add_window_remap_by_id'; did you mean 'mvebu_pcie_add_windows'?
> [-Werror=implicit-function-declaration]
>ret = mvebu_mbus_add_window_remap_by_id(target, attribute, base,
> ^
> mvebu_pcie_add_windows
> drivers/pci/host/pci-mvebu.c: In function 'mvebu_pcie_probe':
> drivers/pci/host/pci-mvebu.c:1209:2: error: implicit declaration of function
> 'mvebu_mbus_get_pcie_mem_aperture'; did you mean
> 'mvebu_mbus_get_io_win_info'? [-Werror=implicit-function-declaration]
> drivers/pci/host/pci-mvebu.c:1215:2: error: implicit declaration of function
> 'mvebu_mbus_get_pcie_io_aperture'; did you mean 'mvebu_mbus_get_io_win_info'?
> [-Werror=implicit-function-declaration]
>
> The problem is that the mbus helper functions are not declared in the
> header. There are two ways of solving this: we can change the header
> to provide empty stub functions for CONFIG_MVEBU_MBUS=n, or we can
> add back the dependency here.
>
> I picked the second option since that is simpler, but it somewhat
> defeats the purpose of the original patch because we still cannot
> compile test on other platforms this way.
>
> Fixes: 51bc085d6454 ("PCI: Improve host drivers compile test coverage")
> Signed-off-by: Arnd Bergmann
Thanks, I added this when doing the merge.
> ---
> drivers/pci/host/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index fa51d0893384..5340950f4b98 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -6,6 +6,7 @@ menu "PCI host controller drivers"
> config PCI_MVEBU
> bool "Marvell EBU PCIe controller"
> depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> + depends on MVEBU_MBUS
> depends on ARM
> depends on OF
>
> --
> 2.9.0
>
Re: [PATCH v4 0/6] mfd/regulator/clk: bd71837: ROHM BD71837 PMIC driver
On Wed, May 30, 2018 at 03:56:34PM +0300, Matti Vaittinen wrote: > On Wed, May 30, 2018 at 12:00:00PM +0100, Mark Brown wrote: > > The tradeoff with forced PWM mode is that the quality of regulation will > > be a lot better, especially if the load changes suddenly (as things like > > CPUs often do). Most hardware that's at all current is able respond to > > changes in load and switch modes automatically when it's appropriate, > > except possibly in some very low power modes. > Yes. The mode switching is automatic. But there is this control bit for > disabling automatic mode switching and forcing the PWM. Problem with > these 4 last bucks is just that if regulator is in PFM (and it may be > if not forced to PWM - due to this automatic switching) then the voltage > change is not behaving well. That sounds like the mode switching just isn't very good and needs a bit of help so forcing the mode is probably going to do the right thing. signature.asc Description: PGP signature
Re: [PATCH v7 2/7] drivers/i2c: Add FSI-attached I2C master algorithm
On 05/29/2018 06:42 PM, Andy Shevchenko wrote:
On Wed, May 30, 2018 at 1:24 AM, Eddie James wrote:
From: "Edward A. James"
Add register definitions for FSI-attached I2C master and functions to
access those registers over FSI. Add an FSI driver so that our I2C bus
is probed up during an FSI scan.
This looks like reinventing a wheel in some ways.
See my comments below.
+/*
+ * Copyright 2017 IBM Corporation
+ *
+ * Eddie James
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
We are using SPDX identifiers. Can you?
Sure.
+/* Find left shift from first set bit in m */
+#define MASK_TO_LSH(m) (__builtin_ffsll(m) - 1ULL)
Oh. What about GENMASK()?
+/* Extract field m from v */
+#define GETFIELD(m, v) (((v) & (m)) >> MASK_TO_LSH(m))
+
+/* Set field m of v to val */
+#define SETFIELD(m, v, val)\
+ (((v) & ~(m)) | typeof(v))(val)) << MASK_TO_LSH(m)) & (m)))
Oh, what about
https://elixir.bootlin.com/linux/latest/source/include/linux/bitfield.h
?
Good idea, thanks.
+#define I2C_CMD_WITH_START 0x8000
+#define I2C_CMD_WITH_ADDR 0x4000
+#define I2C_CMD_RD_CONT0x2000
+#define I2C_CMD_WITH_STOP 0x1000
+#define I2C_CMD_FORCELAUNCH0x0800
BIT() ?
+#define I2C_CMD_ADDR 0x00fe
+#define I2C_CMD_READ 0x0001
GENMASK()? Though precisely here it might be good to leave explicit values.
+#define I2C_CMD_LEN0x
+#define I2C_MODE_CLKDIV0x
+#define I2C_MODE_PORT 0xfc00
+#define I2C_MODE_ENHANCED 0x0008
+#define I2C_MODE_DIAG 0x0004
+#define I2C_MODE_PACE_ALLOW0x0002
+#define I2C_MODE_WRAP 0x0001
What are they? Masks? Bit fields? Just plain numbers?
+#define I2C_WATERMARK_HI 0xf000
+#define I2C_WATERMARK_LO 0x00f0
GENMASK() ?
+#define I2C_INT_INV_CMD0x8000
+#define I2C_INT_PARITY 0x4000
+#define I2C_INT_BE_OVERRUN 0x2000
+#define I2C_INT_BE_ACCESS 0x1000
+#define I2C_INT_LOST_ARB 0x0800
+#define I2C_INT_NACK 0x0400
+#define I2C_INT_DAT_REQ0x0200
+#define I2C_INT_CMD_COMP 0x0100
+#define I2C_INT_STOP_ERR 0x0080
+#define I2C_INT_BUSY 0x0040
+#define I2C_INT_IDLE 0x0020
BIT()
+#define I2C_INT_ENABLE 0xff80
+#define I2C_INT_ERR0xfcc0
+#define I2C_STAT_INV_CMD 0x8000
+#define I2C_STAT_PARITY0x4000
+#define I2C_STAT_BE_OVERRUN0x2000
+#define I2C_STAT_BE_ACCESS 0x1000
+#define I2C_STAT_LOST_ARB 0x0800
+#define I2C_STAT_NACK 0x0400
+#define I2C_STAT_DAT_REQ 0x0200
+#define I2C_STAT_CMD_COMP 0x0100
+#define I2C_STAT_STOP_ERR 0x0080
+#define I2C_STAT_MAX_PORT 0x000f
+#define I2C_STAT_ANY_INT 0x8000
+#define I2C_STAT_SCL_IN0x0800
+#define I2C_STAT_SDA_IN0x0400
+#define I2C_STAT_PORT_BUSY 0x0200
+#define I2C_STAT_SELF_BUSY 0x0100
BIT()
+#define I2C_STAT_FIFO_COUNT0x00ff
GENMASK()
+
+#define I2C_STAT_ERR 0xfc80
+#define I2C_STAT_ANY_RESP 0xff80
+#define I2C_ESTAT_FIFO_SZ 0xff00
GENMASK()
+#define I2C_ESTAT_SCL_IN_SY0x8000
+#define I2C_ESTAT_SDA_IN_SY0x4000
+#define I2C_ESTAT_S_SCL0x2000
+#define I2C_ESTAT_S_SDA0x1000
+#define I2C_ESTAT_M_SCL0x0800
+#define I2C_ESTAT_M_SDA0x0400
+#define I2C_ESTAT_HI_WATER 0x0200
+#define I2C_ESTAT_LO_WATER 0x0100
+#define I2C_ESTAT_PORT_BUSY0x0080
+#define I2C_ESTAT_SELF_BUSY0x0040
BIT()
+#define I2C_ESTAT_VERSION 0x001f
GENMASK()
+ __be32 data_be;
No need to have a suffix. If anything can go wrong we have a tool,
it's called sparse. It will catch out inappropriate use of __bitwise
types.
I already have a variable called data...
+ __be32 data_be = cpu_to_be32(*data);
cpu_to_be32p() IIUC?
Sure.
+static int fsi_i2c_dev_init(struct fsi_i2c_master *i2c)
+{
+ int rc;
+ u32 mode = I2C_MODE_ENHANCED, extended_status, watermark = 0;
+ u32 interrupt = 0;
Redundant assignment.
No, I need to set the interrupt register to 0, so I must set this.
+
+ /* since we use polling, disable interrupts */
+ rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_INT_MASK, );
+ if (rc)
+ return rc;
+ return rc;
Would be non-zero?
No, fsi_i2c_write_reg returns non-zero on error, zero on success. That
is what I want.
Thanks,
Eddie
+}
Re: [PATCH] mm: dmapool: Check the dma pool name
On Wed, May 30, 2018 at 08:13:27AM -0700, Matthew Wilcox wrote: > On Wed, May 30, 2018 at 08:14:09PM +0800, Baolin Wang wrote: > > On 30 May 2018 at 20:01, Matthew Wilcox wrote: > > > On Wed, May 30, 2018 at 07:28:43PM +0800, Baolin Wang wrote: > > >> It will be crash if we pass one NULL name when creating one dma pool, > > >> so we should check the passing name when copy it to dma pool. > > > > > > NAK. Crashing is the appropriate thing to do. Fix the caller to not > > > pass NULL. > > > > > > If you permit NULL to be passed then you're inviting crashes or just > > > bad reporting later when pool->name is printed. > > > > I think it just prints one NULL pool name. Sometimes the device > > doesn't care the dma pool names, so I think we can make code more > > solid to valid the passing parameters like other code does. > > Or can we add check to return NULL when the passing name is NULL > > instead of crashing the kernel? Thanks. > > No. Fix your driver. Let me elaborate on this. Kernel code is supposed to be "reasonable". That means we don't check every argument to every function for sanity, unless it's going to cause trouble later. Crashing immediately with a bogus argument is fine; you can see the problem and fix it immediately. Returning NULL with a bad argument is actually worse; you won't know why the function returned NULL (maybe we're out of memory?) and you'll have a more complex debugging experience. Sometimes it makes sense to accept a NULL pointer and do something reasonable, like kfree(). In this case, we can eliminate checks in all the callers. But we don't, in general, put sanity checks on arguments without a good reason. Your reasons aren't good. "The driver doesn't care" -- well, just pass the driver's name, then.
Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
Hi, On Wed, May 30, 2018 at 9:07 AM, Mark Brown wrote: > On Wed, May 30, 2018 at 09:06:16AM -0700, Doug Anderson wrote: >> On Wed, May 30, 2018 at 8:48 AM, Mark Brown wrote: > >> > Without the core doing something the regulator isn't going to get told >> > that anything updated voltages anyway... > >> I was just suggesting that when the core tells the regulator driver to >> disable itself that the regulator driver tell RPMh to not only disable >> itself but also (temporarily) vote for a lower voltage. When the core >> tells the regulator to re-enable itself then the regulator driver >> restores the original voltage vote (or applies any vote that might >> have been attempted while the regulator was disabled). That wouldn't >> require any regulator core changes. > > It needs something to tell it what the new voltage to set is. The regulator driver has its own cache of what voltage was most recently requested by Linux. It can use that, can't it? -Doug
Re: [PATCH v2 7/9] PM / Domains: Split genpd_dev_pm_attach()
On 29/05/18 11:04, Ulf Hansson wrote:
> To extend genpd to deal with allowing multiple PM domains per device, some
> of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
> by moving some of the code into a sub-function.
>
> Signed-off-by: Ulf Hansson
> ---
> drivers/base/power/domain.c | 60 -
> 1 file changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 12a20f21974d..2af99bfcbe3c 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2221,38 +2221,15 @@ static void genpd_dev_pm_sync(struct device *dev)
> genpd_queue_power_off_work(pd);
> }
>
> -/**
> - * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> - * @dev: Device to attach.
> - *
> - * Parse device's OF node to find a PM domain specifier. If such is found,
> - * attaches the device to retrieved pm_domain ops.
> - *
> - * Returns 1 on successfully attached PM domain, 0 when the device don't
> need a
> - * PM domain or when multiple power-domains exists for it, else a negative
> error
> - * code. Note that if a power-domain exists for the device, but it cannot be
> - * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> - * not probed and to re-try again later.
> - */
> -int genpd_dev_pm_attach(struct device *dev)
> +static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
> + unsigned int index)
> {
> struct of_phandle_args pd_args;
> struct generic_pm_domain *pd;
> int ret;
>
> - if (!dev->of_node)
> - return 0;
> -
> - /*
> - * Devices with multiple PM domains must be attached separately, as we
> - * can only attach one PM domain per device.
> - */
> - if (of_count_phandle_with_args(dev->of_node, "power-domains",
> -"#power-domain-cells") != 1)
> - return 0;
> -
> - ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
> - "#power-domain-cells", 0, _args);
> + ret = of_parse_phandle_with_args(np, "power-domains",
> + "#power-domain-cells", index, _args);
> if (ret < 0)
> return ret;
>
> @@ -2290,6 +2267,35 @@ int genpd_dev_pm_attach(struct device *dev)
>
> return ret ? -EPROBE_DEFER : 1;
> }
> +
> +/**
> + * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> + * @dev: Device to attach.
> + *
> + * Parse device's OF node to find a PM domain specifier. If such is found,
> + * attaches the device to retrieved pm_domain ops.
> + *
> + * Returns 1 on successfully attached PM domain, 0 when the device don't
> need a
> + * PM domain or when multiple power-domains exists for it, else a negative
> error
> + * code. Note that if a power-domain exists for the device, but it cannot be
> + * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> + * not probed and to re-try again later.
> + */
> +int genpd_dev_pm_attach(struct device *dev)
> +{
> + if (!dev->of_node)
> + return 0;
> +
> + /*
> + * Devices with multiple PM domains must be attached separately, as we
> + * can only attach one PM domain per device.
> + */
> + if (of_count_phandle_with_args(dev->of_node, "power-domains",
> +"#power-domain-cells") != 1)
> + return 0;
> +
> + return __genpd_dev_pm_attach(dev, dev->of_node, 0);
> +}
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> static const struct of_device_id idle_state_match[] = {
Acked-by: Jon Hunter
Tested-by: Jon Hunter
Cheers
Jon
--
nvpublic
Re: [PATCH v1 0/4] sparc/PCI: VGA resource and other fixes
On Tue, May 22, 2018 at 08:02:17AM -0500, Bjorn Helgaas wrote: > [+cc sparclinux, sorry I missed this first time around] > > On Mon, May 21, 2018 at 07:30:29PM -0500, Bjorn Helgaas wrote: > > The main thing here is the first patch, a legacy VGA framebuffer fix for > > issues reported by Meelis. > > > > The others are to converge the pci_enable_device() path for LEON and PCIC > > with other arches (including sparc64), and to convert some logging to use > > dev_printk() and make it more consistent with the PCI core. > > > > I propose to merge these via the PCI tree. > > > > --- > > > > Bjorn Helgaas (4): > > sparc/PCI: Request legacy VGA framebuffer only for VGA devices > > sparc32/PCI/LEON: Converge device enable path > > sparc32/PCI/PCIC: Converge device enable path > > sparc/PCI: Use dev_printk() when possible > > > > > > arch/sparc/kernel/leon_pci.c | 62 ++ > > arch/sparc/kernel/pci.c| 136 > > +--- > > arch/sparc/kernel/pci_common.c | 31 ++--- > > arch/sparc/kernel/pci_msi.c| 10 +-- > > arch/sparc/kernel/pcic.c | 86 - > > 5 files changed, 159 insertions(+), 166 deletions(-) I applied these to my pci/sparc branch for v4.18 (with fixes for the typos reported by Meelis and the kbuild robot).
Re: [PATCH] PCI: Remove host driver Kconfig selection of CONFIG_PCIEPORTBUS
On Fri, May 18, 2018 at 05:31:07PM -0500, Bjorn Helgaas wrote: > From: Bjorn Helgaas > > Host bridge drivers do not use the portdrv interfaces (struct pcie_device, > struct pcie_port_service_driver, pcie_port_service_register(), etc), and > they should not select CONFIG_PCIEPORTBUS. > > If users need the portdrv services, they can select CONFIG_PCIEPORTBUS just > like all other PCI users. > > Signed-off-by: Bjorn Helgaas I applied this to pci/kconfig for v4.18. > --- > drivers/pci/dwc/Kconfig | 10 -- > drivers/pci/host/Kconfig |2 -- > 2 files changed, 12 deletions(-) > > diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig > index 2f3f5c50aa48..678390a77ba1 100644 > --- a/drivers/pci/dwc/Kconfig > +++ b/drivers/pci/dwc/Kconfig > @@ -68,7 +68,6 @@ config PCI_EXYNOS > depends on PCI > depends on SOC_EXYNOS5440 > depends on PCI_MSI_IRQ_DOMAIN > - select PCIEPORTBUS > select PCIE_DW_HOST > > config PCI_IMX6 > @@ -76,7 +75,6 @@ config PCI_IMX6 > depends on PCI > depends on SOC_IMX6Q > depends on PCI_MSI_IRQ_DOMAIN > - select PCIEPORTBUS > select PCIE_DW_HOST > > config PCIE_SPEAR13XX > @@ -84,7 +82,6 @@ config PCIE_SPEAR13XX > depends on PCI > depends on ARCH_SPEAR13XX > depends on PCI_MSI_IRQ_DOMAIN > - select PCIEPORTBUS > select PCIE_DW_HOST > help > Say Y here if you want PCIe support on SPEAr13XX SoCs. > @@ -94,7 +91,6 @@ config PCI_KEYSTONE > depends on PCI > depends on ARCH_KEYSTONE > depends on PCI_MSI_IRQ_DOMAIN > - select PCIEPORTBUS > select PCIE_DW_HOST > help > Say Y here if you want to enable PCI controller support on Keystone > @@ -117,7 +113,6 @@ config PCI_HISI > bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers" > depends on PCI > depends on PCI_MSI_IRQ_DOMAIN > - select PCIEPORTBUS > select PCIE_DW_HOST > select PCI_HOST_COMMON > help > @@ -129,7 +124,6 @@ config PCIE_QCOM > depends on PCI > depends on ARCH_QCOM && OF > depends on PCI_MSI_IRQ_DOMAIN > - select PCIEPORTBUS > select PCIE_DW_HOST > help > Say Y here to enable PCIe controller support on Qualcomm SoCs. The > @@ -141,7 +135,6 @@ config PCIE_ARMADA_8K > depends on PCI > depends on ARCH_MVEBU > depends on PCI_MSI_IRQ_DOMAIN > - select PCIEPORTBUS > select PCIE_DW_HOST > help > Say Y here if you want to enable PCIe controller support on > @@ -156,7 +149,6 @@ config PCIE_ARTPEC6_HOST > bool "Axis ARTPEC-6 PCIe controller Host Mode" > depends on MACH_ARTPEC6 > depends on PCI && PCI_MSI_IRQ_DOMAIN > - select PCIEPORTBUS > select PCIE_DW_HOST > select PCIE_ARTPEC6 > help > @@ -178,7 +170,6 @@ config PCIE_KIRIN > bool "HiSilicon Kirin series SoCs PCIe controllers" > depends on PCI_MSI_IRQ_DOMAIN > depends on PCI > - select PCIEPORTBUS > select PCIE_DW_HOST > help > Say Y here if you want PCIe controller support > @@ -189,7 +180,6 @@ config PCIE_HISI_STB > depends on ARCH_HISI > depends on PCI > depends on PCI_MSI_IRQ_DOMAIN > - select PCIEPORTBUS > select PCIE_DW_HOST > help >Say Y here if you want PCIe controller support on HiSilicon STB > SoCs > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > index 0d0177ce436c..90f0cc9dfd95 100644 > --- a/drivers/pci/host/Kconfig > +++ b/drivers/pci/host/Kconfig > @@ -83,7 +83,6 @@ config PCI_XGENE > bool "X-Gene PCIe controller" > depends on ARM64 > depends on OF || (ACPI && PCI_QUIRKS) > - select PCIEPORTBUS > help > Say Y here if you want internal PCI support on APM X-Gene SoC. > There are 5 internal PCIe ports available. Each port is GEN3 capable > @@ -194,7 +193,6 @@ config PCIE_MEDIATEK > depends on (ARM || ARM64) && (ARCH_MEDIATEK || COMPILE_TEST) > depends on OF > depends on PCI > - select PCIEPORTBUS > help > Say Y here if you want to enable PCIe controller support on > MediaTek SoCs. >
[PATCH] FIXUP checkpatch: improve patch recognition
Fix syntax error in patch.
Signed-off-by: Gwendal Grignou
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f1fecd8aa4d7..03dd7b6b0eab 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2378,7 +2378,7 @@ sub process {
# check if it's a mode change, rename or start of a patch
if (!$in_commit_log &&
($line =~ /^ mode change [0-7]+ => [0-7]+ \S+\s*$/ ||
- ($line =~ /^rename (?:from|to) \S+\s*$/ ||
+$line =~ /^rename (?:from|to) \S+\s*$/ ||
$line =~ /^diff --git a\/[\w\/\.\_\-]+ b\/\S+\s*$/)) {
$is_patch = 1;
}
--
2.17.0.921.gf22659ad46-goog
Re: Userland breakage from "Modify the device name as devfreq(X) for sysfs"
On Wed, May 30, 2018 at 12:31 AM, Greg KH wrote:
> On Tue, May 29, 2018 at 11:52:37PM -0700, John Stultz wrote:
>> On Tue, May 29, 2018 at 10:33 PM, Greg KH wrote:
>> > On Tue, May 29, 2018 at 10:14:35PM -0700, John Stultz wrote:
>> >> On Tue, May 8, 2018 at 7:28 PM, Chanwoo Choi
>> >> wrote:
>> >> > On 2018년 05월 09일 08:17, John Stultz wrote:
>> >> >> Hey folks,
>> >> >> I wanted to bring up an issue we've recently tripped over, which was
>> >> >> caused by 4585fbcb5331f ("PM / devfreq: Modify the device name as
>> >> >> devfreq(X) for sysfs").
>> >> >>
>> >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=4585fbcb5331fc910b7e553ad3efd0dd7b320d14
>> >> >>
>> >> >> That patch replaced paths like:
>> >> >>/sys/class/devfreq/ddr_devfreq/min_freq
>> >> >> and
>> >> >> /sys/class/devfreq/e82c.mali/min_freq
>> >> >>
>> >> >> With
>> >> >> /sys/class/devfreq/devfreq(0)/min_freq
>> >> >> and
>> >> >> /sys/class/devfreq/devfreq(1)/min_freq
>> >> >>
>> >> >>
>> >> >> This broke userspace we have that needs to work on 4.4, 4.9 and 4.14
>> >> >> (and on).
>> >> >
>> >> > Firstly, I'm sorry to make some problem on userland.
>> >> >
>> >> >>
>> >> >> I wanted to try to ask to understand more about the rational for this
>> >> >> patch, as it doesn't make much sense to me, particularly as now it is
>> >> >> less obvious as to which path is for which device - and more
>> >> >> worrisome it could change depending on initialization order.
>> >> >
>> >> > Some linux framework used the their own prefix under "/sys/class/"
>> >> > for device such as input/pwm/hwmon/regulator and so on.
>> >> > (But, some linux framework used the device name directly without any
>> >> > changes)
>> >> >
>> >> > I thought that devfreq better to use use the consistent name.
>> >> > If user wanted to access the specific device with device name,
>> >> > the user can access the path of '/sys/devices/platform/...'.
>> >> >
>> >> > [Example on Exynos5433-based TM2 board]
>> >> > root@localhost:~# ls -al /sys/class/devfreq
>> >> > total 0
>> >> > drwxr-xr-x 2 root root 0 Jul 26 04:49 .
>> >> > drwxr-xr-x 50 root root 0 Jan 1 1970 ..
>> >> > lrwxrwxrwx 1 root root 0 Jul 26 04:49 devfreq0 ->
>> >> > ../../devices/platform/soc/soc:bus0/devfreq/devfreq0
>> >> > lrwxrwxrwx 1 root root 0 Jul 26 04:49 devfreq1 ->
>> >> > ../../devices/platform/soc/soc:bus1/devfreq/devfreq1
>> >> > (skip)
>> >> >
>> >> > - User can access the devfreq device with specific device name.
>> >> > root@localhost:/sys/devices/platform/soc/soc:bus0/devfreq/devfreq0# pwd
>> >> > /sys/devices/platform/soc/soc:bus0/devfreq/devfreq0
>> >> >
>> >> > root@localhost:/sys/devices/platform/soc/soc:bus0/devfreq/devfreq0# ls
>> >> > available_frequencies devicemin_freq subsystemuevent
>> >> > available_governorsgovernor polling_interval target_freq
>> >> > cur_freq max_freq power trans_stat
>> >> >
>> >> > root@localhost:/sys/devices/platform/soc/soc:bus0/devfreq/devfreq0# cat
>> >> > min_freq
>> >> > 16000
>> >> >
>> >>
>> >> Sorry to not get back to you sooner on this. Was on vacation then this
>> >> discussion got buried in my inbox.
>> >>
>> >> I do agree that it the consistency with other subsystems is an
>> >> improvement, but it still doesn't help our situation that userspace
>> >> applications can't consistently work between kernel versions, as even
>> >> if we go with the /sys/devices/platform/soc/soc:bus0/devfreq/devfreq0
>> >> path rather then the /sys/class/devfreq/ path, in older kernels the
>> >> /sys/devices/platform/soc/soc:bus0/devfreq/devfreq0 doesn't exist.
>> >
>> > Ick, that's not ok. The sys/class/ path should always work for old and
>> > new. If not, it's broken and should be reverted.
>> >
>> > And it seems like it still works, just that the "names" are now
>> > different in the class path, of the device, right? And that should be
>> > fine, what is breaking when a device name changes?
>>
>> Well, code that is looking for:
>> /sys/class/devfreq/ddr_devfreq/min_freq
>>
>> won't work with newer kernels unless its changed to look for:
>> /sys/class/devfreq/devfreq0/min_freq
>>
>> (assuming the ddr_devfreq driver loaded before the mali one :)
>
> Ugh. In working with sysfs you should never care about the device name,
> you should be iterating over:
> /sys/class/devfreq/*/min_freq
>
> and just picking up the one you find that has a link back to whatever
> you want it to be. Device names are not deterministic, as you even
> claim here as you were somehow depending on the module load order :)
Well, previously it used the dev name, which was somewhat
deterministic (e82c.mali or ddr_devfreq) regardless of module load
order. Now, its just devfreqN, which obviously isn't deterministic.
Part of the trouble, as Chanwoo mentioned is there is no name entry,
so even iterating over the the devices, the devfreqN sysfs dirs are
indistinguishable.
>
RE: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client
Hi, > -Original Message- > From: Peter Ujfalusi [mailto:peter.ujfal...@ti.com] > Sent: Tuesday, May 29, 2018 8:35 PM > To: Radhey Shyam Pandey ; Vinod Koul > > Cc: Lars-Peter Clausen ; michal.si...@xilinx.com; linux- > ker...@vger.kernel.org; dmaeng...@vger.kernel.org; > dan.j.willi...@intel.com; Appana Durga Kedareswara Rao > ; linux-arm-ker...@lists.infradead.org > Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words > to netdev dma client > > Hi, > > On 2018-05-17 09:39, Radhey Shyam Pandey wrote: > >> Well, let's see where this is going to go when I can send the patches > >> for review. > > Thanks all. @Peter: If we have metadata patchset ready may be good > > to send an RFC? > > Sorry for the delay, I got distracted by this: > http://www.ti.com/lit/pdf/spruid7 Chapter 10. > > I have given some tough to the metadata attach patches. > In my case the 'metadata' is more like private data section within the > DMA descriptor (10.1.2.2.1) which is used by the remote peripheral and > the driver for the given peripheral and it is optional. > > I liked the idea of treating it as metadata as it gives more generic API > which can be adopted by other drivers if they need something similar. > > Another issue I have with the attach metadata way is that it would > require one memcpy to copy the data to the DMA descriptor and in high > throughput case it is not acceptable. I think memcpy is needed (alternative?) if dma engine doesn’t directly update metadata buffers i.e in RX, we need to copy metadata from dma descriptor fields to client allocated metadata buffer (sideband/ metadata info is part of Buffer descriptor fields) memcpy(app_w, hw->app, sizeof(u32) * XILINX_DMA_NUM_APP_WORDS); Descriptor Fields Address Space Offset Name Description 20h APP0 User Application Field 0 24h APP1 User Application Field 1 ... 30h APP5 User Application Field 5 > > For me probably a .get_private_area / .put_private_area like API would > be desirable where I can give the pointer of the 'metadata' are (and > size) to the user. > > But these can co-exist in my opinion and DMA drivers can opt to > implement none, either or both of the callbacks. > > In couple of days I can update the metadata patches I have atm and send > as RFC. > > Is there anything from your side I should take into account when doing that? I think a generic interface to attach/share metadata buffer b/w client and the dmaengine driver is good enough. Is metadata patchset (early version) available in TI external repos? Thanks, Radhey > > - Péter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH 5/6 v2] mtd: rawnand: ams-delta: use GPIO lookup table
On Wednesday, May 30, 2018 11:05:00 AM CEST Boris Brezillon wrote: > Hi Janusz, Hi Boris, > On Sat, 26 May 2018 00:20:45 +0200 > Janusz Krzysztofik wrote: > > ... > > Changes since v1: > > - fix handling of devm_gpiod_get_optional() return values - thanks to > > Andy Shevchenko. > > Can you put the changelog after the "---" separator so that it does not > appear in the final commit message? Yes, sure, sorry for that. > > +err_gpiod: > > + if (err == -ENODEV || err == -ENOENT) > > + err = -EPROBE_DEFER; > > Hm, isn't it better to make gpiod_find() return ERR_PTR(-EPROBE_DEFER) > here [1]? At least, ENOENT should not be turned into EPROBE_DEFER, > because it's returned when there's no entry matching the requested gpio > in the lookup table, and deferring the probe won't solve this problem. ENOENT is also returned when no matching lookup table is found. That may happen if consumer dev_name stored in the table differs from dev_name assigned to the consumer by its bus, the platform bus in this case. For that reason I think the consumer dev_name should be initialized in the table after the device is registered, when its actual dev_name can be obtained. If that device registration happens after the driver is already registered, e.g., at late_initcall, the device is probed before its lookup table is ready. For that reason returning EPROBE_DEFER seems better to me even in the ENOENT case. Thanks, Janusz
Re: LKMM litmus test for Roman Penyaev's rcu-rr
On Wed, 30 May 2018, Linus Torvalds wrote: > On Wed, May 30, 2018 at 9:29 AM Alan Stern > wrote: > > > > > > > Can't we simplify the whole sequence as basically > > > > > > A > > > if (!B) > > > D > > > > > > for that "not B" case, and just think about that. IOW, let's ignore the > > > whole "not executed" code. > > > Your listing is slightly misleading. > > No. You're confused. > > You're confused because you're conflating two *entirely* different things. > > You're conflating the static source code with the dynamic execution. They > are NOT THE SAME. I'll do my best not to conflate them. Your phrasing was not the greatest in this regard; it looked like you were trying to talk about a different static source program. >It really should be: > > > A > > if (!B) > > ; // NOP > > D > > No it really really shouldn't. > > There are two completely different situations: > > (1) there is the source code: > > A > if (B) > C > D > > where C contains a barrier, and B depends on A and is not statically > determinable. > > In the source code, 'D' looks unconditional BUT IT DAMN WELL IS NOT. > > It's not unconditional - it's just done in both conditions! That's a big > big difference. Can you explain in greater detail? A lot of people would say that something which occurs under either condition _is_ unconditional, by definition. > > In other words, D should be beyond the end of the "if" statement, not > > inside one of the branches. > > You're just completely confused. > > What you are stating makes no sense at all. > > Seriously, your reading of the code is entirely monsenscal, and seems to be > about syntax, not about semantics. Which is crazy. > > Lookie here, you can change the syntactic model of that code to just be > > A > if (B) > C > D > else > D > > and that code obviously has the EXACT SAME SEMANTICS. That is not entirely clear. The semantics of a source program are partly determined by the memory model, and the quality of the memory model is exactly what this email thread is about. Under a defective memory model, this code might indeed have different semantics from the original. Granted, it _shouldn't_ -- but our LKMM is not ideal. As an example, one of the known defects of the memory model (we have pointed it out repeatedly from the very beginning) is that when identical writes occur in the two branches of an "if" statement, just as in your code above, the model does not allow for the possibility that the compiler may combine the two writes into one and move it up before the "if". That's not the issue in this case, but it does show that the memory model we are working with isn't perfect. The subject of this discussion is another example in which the memory model fails to give the desired result. [It's also worth pointing out that semantics vary with context. Is "x = 1; y = 2;" semantically equivalent to "y = 2; x = 1;"? It is in a single-threaded setting. But in a multi-threaded setting it need not be.] > So if you get hung up on trivial syntactic issues, you are by definition > confused, and your tool is garbage. You're doing memory ordering analysis, > not syntax parsing, for chrissake! To be fair, we are (or rather, herd is) doing both. > > At run time, of course, it doesn't matter; > > CPUs don't try to detect where the two branches of an "if" recombine. > > (Leaving aside issues like implementing an "if" as a move-conditional.) > > You cannot do it as a move-conditional, since that code generation would > have been buggy shit, exactly because of C. But that's a code generation > issue, not a run-time decision. > > So at run-time, the code ends up being > > A > if (!B) > D Sorry, not clear enough. Do you mean that the CPU ends up executing essentially the same sequence of assembly instructions as you would get from a naive compilation of this source code? If so, then I agree. By the same reasoning, at run-time the code also ends up being A if (B) ; D (that is, the sequence of executed assembly instructions would be essentially the same). > and D cannot be written before A has been read, because B depends on A, and > you cannot expose specutive writes before the preconditions have been > evaluated. > > > Remember, the original code was: > > > A > > if (!B) > > C > > D > > > So the execution of D is _not_ conditional, and it doesn't depend on A > > or B. (Again, CPUs don't make this distinction, but compilers do.) > > Again, the above is nothing but confused bullshit. > > D depends on B, which depends on A. > > Really. Really really. Sure, in the code you wrote there is a control dependency from A to D. I would never say otherwise. But my earlier email concerned the original source code outline. Whether the
[PATCH][next] iio: tsl2x7x/tsl2772: avoid potential division by zero
From: Colin Ian King
It may be possible for tsl2772_get_lux to return a zero lux value
and hence a division by zero can occur when lux_val is zero. Check
for this case and return -ERANGE to avoid the division by zero.
Detected by CoverityScan, CID#1469484 ("Division or modulo by zero")
Signed-off-by: Colin Ian King
---
drivers/iio/light/tsl2772.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/iio/light/tsl2772.c b/drivers/iio/light/tsl2772.c
index 34d42a2504c9..df5b2a0da96c 100644
--- a/drivers/iio/light/tsl2772.c
+++ b/drivers/iio/light/tsl2772.c
@@ -582,6 +582,8 @@ static int tsl2772_als_calibrate(struct iio_dev *indio_dev)
"%s: failed to get lux\n", __func__);
return lux_val;
}
+ if (lux_val == 0)
+ return -ERANGE;
ret = (chip->settings.als_cal_target * chip->settings.als_gain_trim) /
lux_val;
--
2.17.0
Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
Hi, On Wed, May 30, 2018 at 3:37 AM, Mark Brown wrote: > On Tue, May 29, 2018 at 10:23:20PM -0700, Doug Anderson wrote: > >> > + qcom,drms-mode-max-microamps = <1 100>; > >> Things look pretty good to me now. I'm still hesitant about the whole >> need to list the modes twice (once using the unordered >> "regulator-allowed-modes" and once to match up against the ordered >> "qcom,drms-mode-max-microamps"). I'm also still of the opinion that >> the whole "drms-mode-max-microamps" ought to be a standard property >> (not a qcom specific one) and handled in the regulator core. > > I'm confused as to why we are specifying the maximum current the device > can deliver in a given mode in the DT - surely that's a fixed property > of the hardware? Said another way: you're saying that you'd expect the "max-microamps" table to have one fewer element than the listed modes? So in the above example you'd have: drms-modes: LPM, HPM max-microamps: 1 ...or in a more complicated case, you could have: drms-modes: RET, LPM, AUTO, HPM max-microamps: 1, 100, 1 Did I understand you correctly? I'm personally OK with that color for the shed. I kinda like the symmetry of having the same number of elements in both lists (and being able to print an error message if someone ends up asking for too much current), but it's not a big deal for me either way. -Doug
