Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: debug: mark a function as __init to save some memory
From: Christophe JAILLET @ 2020-05-31 11:00 UTC (permalink / raw)
  To: catalin.marinas, will, tglx, paulmck, gregkh, mhiramat, dianders
  Cc: Christophe JAILLET, kernel-janitors, linux-kernel,
	linux-arm-kernel

'debug_monitors_init()' is only called via 'postcore_initcall'.
It can be marked as __init to save a few bytes of memory.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 arch/arm64/kernel/debug-monitors.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 15e80c876d46..5df49366e9ab 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -130,7 +130,7 @@ static int clear_os_lock(unsigned int cpu)
 	return 0;
 }
 
-static int debug_monitors_init(void)
+static int __init debug_monitors_init(void)
 {
 	return cpuhp_setup_state(CPUHP_AP_ARM64_DEBUG_MONITORS_STARTING,
 				 "arm64/debug_monitors:starting",
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [arm64, debug] PTRACE_SINGLESTEP does not single-step a valid instruction
From: Will Deacon @ 2020-05-31  9:52 UTC (permalink / raw)
  To: Mark Rutland; +Cc: Luis Machado, linux-arm-kernel
In-Reply-To: <20200221111652.GB45022@lakrids.cambridge.arm.com>

Hi folks,

Sorry, I wrote a reply to this on a plane (so you can tell how long ago that
was!) and then forgot about it.

On Fri, Feb 21, 2020 at 11:16:53AM +0000, Mark Rutland wrote:
> On Thu, Feb 20, 2020 at 01:29:42PM +0000, Will Deacon wrote:
> > On Thu, Feb 20, 2020 at 01:02:22PM +0000, Mark Rutland wrote:
> > > On Thu, Feb 13, 2020 at 12:01:16PM +0000, Will Deacon wrote:
> > > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> > > > index cd6e5fa48b9c..d479fbcbd0d2 100644
> > > > --- a/arch/arm64/kernel/ptrace.c
> > > > +++ b/arch/arm64/kernel/ptrace.c
> > > > @@ -1934,8 +1934,8 @@ static int valid_native_regs(struct user_pt_regs *regs)
> > > >   */
> > > >  int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task)
> > > >  {
> > > > -	if (!test_tsk_thread_flag(task, TIF_SINGLESTEP))
> > > > -		regs->pstate &= ~DBG_SPSR_SS;
> > > > +	/* https://lore.kernel.org/lkml/20191118131525.GA4180@willie-the-truck */
> > > > +	user_regs_reset_single_step(regs, task);
> > > 
> > > I think this change means we do the right thing for signal entry/return
> > > and ptrace messing with the regs. Instruction emulation seems to do the
> > > right thing via skip_faulting_instruction().
> > > 
> > > I think there are a few more single-step edge cases lying around (e.g.
> > > uprobes, rseq), but it looks like those have to be fixed separately. I
> > > fear fixing uprobes might require a largler structural change to single
> > > step, but ignoring uprobes the changes above seem to be sound.
> > 
> > Rseq should just abort when delivering the step signal and I'm not sure I
> > see the issue with uprobes. Can you elaborate on your concerns a bit,
> > please?
> 
> For rseq I wasn't sure what state PSTATE.SS should be when we head to
> the abort handler -- I think the sensible thing would be that it
> immediately triggers a single-step exception, but I don't see where we'd
> clear PSTATE.SS to ensure that.
> 
> For uprobes I fear that the uprobes xol single-stepping might end up
> conflicting with the regular ptrace single-stepping, and that the
> uprobes emulation might not always advance the state machine correctly.
> 
> > > If userspace doesn't consume the SS value today, I wonder if we should
> > > hide it when dumping the SPSR to userspace, so that userspace has a
> > > consistent view regardless of whether it's being stepped.
> > 
> > You can't really hide it though, because '0' has a meaning so I don't think
> > it gains us a lot other than increasing the scope of the change.
> 
> I think that it reduces the likelihood that single-stepping a program
> changes its behaviour unexpectedly. This patch makes the kernel
> disregard the PSTATE.SS value provided by userspace, so what is gained
> by exposing PSTATE.SS to userspace at all?
> 
> I do agree that there are potentially subtle landmines here; I just
> can't see a legitimate reason for userspace to need the value.
> 
> > > I'll try to dig into the uprobes stuff this afternoon, just in case
> > > that
> > > needs us to do something substantially different.
> > 
> > Thanks.
> 
> I didn't get the chance to do this yesterday, but I did think of another
> potential problem.
> 
> I *think* that when attempting to single-step a syscall, if prior to
> return from the syscall the tracer messed with the tracee's regs (e.g.
> to mess with arguments or the retun value) then valid_user_regs() will
> set the SS bit, and upon return from the syscall the next instruction
> would be executed rather than first raising a single-step exception.

I don't actually think that's a problem: if the tracer has taken control by
e.g. PTRACE_SYSCALL and modified the registers on the syscall return path,
then it has to resume execution of the tracee somehow. There's nothing like
a "PTRACE_RESUME_SINGLESTEP" request, so it would need to issue something
like PTRACE_CONT (which disables stepping altogether) or PTRACE_SINGLESTEP,
which would step over the first instruction after the SVC. That's the same
as the behaviour today.

> This patch relies on valid_user_regs() being a signal that PSTATE.SS is
> stale, but that's not always the case. To handle that generally I
> suspect we need two bits of state rather than just TIF_SINGLESTEP.

Added another bit of state feels like we'll open up another can of worms.
Given that I don't think we need it for ptrace, I'll write this up as a
proper patch.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] ARM: smp_twd: mark a function as __init to save some memory
From: Christophe JAILLET @ 2020-05-31  9:51 UTC (permalink / raw)
  To: linux, tglx, allison, kstewart, info, gregkh
  Cc: Christophe JAILLET, kernel-janitors, linux-kernel,
	linux-arm-kernel

'twd_clk_init()' is only called via 'core_initcall'.
It can be marked as __init to save a few bytes of memory.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
For the records, this function has been introduced in commit 4fd7f9b12810
("ARM: 7212/1: smp_twd: reconfigure clockevents after cpufreq change")
---
 arch/arm/kernel/smp_twd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 9a14f721a2b0..e0a0b0d820bc 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -129,7 +129,7 @@ static struct notifier_block twd_clk_nb = {
 	.notifier_call = twd_rate_change,
 };
 
-static int twd_clk_init(void)
+static int __init twd_clk_init(void)
 {
 	if (twd_evt && raw_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
 		return clk_notifier_register(twd_clk, &twd_clk_nb);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] ARM: riscpc: mark a function as __init to save some memory
From: Christophe JAILLET @ 2020-05-31  9:34 UTC (permalink / raw)
  To: linux; +Cc: Christophe JAILLET, kernel-janitors, linux-kernel,
	linux-arm-kernel

'ecard_bus_init()' is only called via 'postcore_initcall'.
It can be marked as __init to save a few bytes of memory.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 arch/arm/mach-rpc/ecard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-rpc/ecard.c b/arch/arm/mach-rpc/ecard.c
index 75cfad2cb143..9f4e7106efb9 100644
--- a/arch/arm/mach-rpc/ecard.c
+++ b/arch/arm/mach-rpc/ecard.c
@@ -1135,7 +1135,7 @@ struct bus_type ecard_bus_type = {
 	.shutdown	= ecard_drv_shutdown,
 };
 
-static int ecard_bus_init(void)
+static int __init ecard_bus_init(void)
 {
 	return bus_register(&ecard_bus_type);
 }
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: arm64: Register modification during syscall entry/exit stop
From: Will Deacon @ 2020-05-31  9:33 UTC (permalink / raw)
  To: Dave Martin
  Cc: Kyle Huey, Catalin Marinas, Linux Kernel Mailing List,
	Oleg Nesterov, Keno Fischer, linux-arm-kernel
In-Reply-To: <20200527101929.GT5031@arm.com>

On Wed, May 27, 2020 at 11:19:29AM +0100, Dave Martin wrote:
> On Wed, May 27, 2020 at 10:55:29AM +0100, Will Deacon wrote:
> > On Sun, May 24, 2020 at 02:56:35AM -0400, Keno Fischer wrote:
> > > Just ran into this issue again, with what I think may be most compelling
> > > example yet why this is problematic:
> > > 
> > > The tracee incurred a signal, we PTRACE_SYSEMU'd to the rt_sigreturn,
> > > which the tracer tried to emulate by applying the state from the signal frame.
> > > However, the PTRACE_SYSEMU stop is a syscall-stop, so the tracer's write
> > > to x7 was ignored and x7 retained the value it had in the signal handler,
> > > which broke the tracee.
> > 
> > Yeah, that sounds like a good justification to add a way to stop this. Could
> > you send a patch, please?
> > 
> > Interestingly, I *thought* the current behaviour was needed by strace, but I
> > can't find anything there that seems to require it. Oh well, we're stuck
> > with it anyway.
> 
> The fact that PTRACE_SYSEMU is only implemented for a few arches makes
> we wonder whether it was a misguided addition that should not be ported
> to new arches... i.e., why does hardly anyone need it?  But I haven't
> attempted to understand the history.
> 
> Can't PTRACE_SYSEMU be emulated by using PTRACE_SYSCALL, cancelling the
> syscall at the syscall enter stop, then modifying the regs at the
> syscall exit stop?
> 
> 
> If SYSEMU was obviously always broken, perhaps we can withdraw support
> for it.  Assuming nobody is crazy enough to try to emulate execve() I
> can't see anything other than sigreturn that would be affected by this
> issue though.  So maybe SYSEMU isn't broken enough to justify
> withdrawal.

Indeed, my preference on another thread [1] was to remove it, but it would
need agreement from Arm, since it was added by them (Sudeep).

But setting that aside, Keno has convinced me that the clobbering of x7
on syscall enter/exit can cause some problems for userspace, so I think
that having a way to disable that seems like a good idea. We can't change
the current default behaviour, but having an explicit opt-in seems
worthwhile.

Keno -- are you planning to send out a patch? You previously spoke about
implementing this using PTRACE_SETOPTIONS.

Will

[1] https://lore.kernel.org/r/20200515121346.GA22919@willie-the-truck

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH RFC] KVM: arm64: Sidestep stage2_unmap_vm() on vcpu reset when S2FWB is supported
From: Alexandru Elisei @ 2020-05-31  9:12 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Zenghui Yu, kvmarm, linux-arm-kernel, linux-kernel
In-Reply-To: <13db879dff56d091f98f7c5416ab1535@kernel.org>

Hi Marc,

On 5/30/20 5:31 PM, Marc Zyngier wrote:
> Hi Alex,
>
> On 2020-05-30 11:46, Alexandru Elisei wrote:
>> Hi,
>
> [...]
>
>>>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>>>> index 48d0ec44ad77..e6378162cdef 100644
>>>> --- a/virt/kvm/arm/arm.c
>>>> +++ b/virt/kvm/arm/arm.c
>>>> @@ -983,8 +983,11 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu
>>>> *vcpu,
>>>>      /*
>>>>       * Ensure a rebooted VM will fault in RAM pages and detect if the
>>>>       * guest MMU is turned off and flush the caches as needed.
>>>> +     *
>>>> +     * S2FWB enforces all memory accesses to RAM being cacheable, we
>>>> +     * ensure that the cache is always coherent.
>>>>       */
>>>> -    if (vcpu->arch.has_run_once)
>>>> +    if (vcpu->arch.has_run_once && !cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
>>> I think userspace does not invalidate the icache when loading a new kernel image,
>>> and if the guest patched instructions, they could potentially still be in the
>>> icache. Should the icache be invalidated if FWB is present?
>>
>> I noticed that this was included in the current pull request and I
>> remembered that
>> I wasn't sure about this part. Did some more digging and it turns out that FWB
>> implies no cache maintenance needed for *data to instruction*
>> coherence. From ARM
>> DDI 0487F.b, page D5-2635:
>>
>> "When ARMv8.4-S2FWB is implemented, the architecture requires that
>> CLIDR_EL1.{LOUU, LOIUS} are zero so that no levels of data cache need to be
>> cleaned in order to manage coherency with instruction fetches".
>>
>> However, there's no mention that I found for instruction to data coherence,
>> meaning that the icache would still need to be invalidated on each vcpu in order
>> to prevent fetching of patched instructions from the icache. Am I
>> missing something?
>
> I think you are right, and this definitely matches the way we deal with
> the icache on the fault path. For some bizarre reason, I always assume
> that FWB implies DIC, which isn't true at all.
>
> I'm planning to address it as follows. Please let me know what you think.
>
> Thanks,
>
>         M.
>
> From f7860d1d284f41afea176cc17e5c9d895ae665e9 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Sat, 30 May 2020 17:22:19 +0100
> Subject: [PATCH] KVM: arm64: Flush the instruction cache if not unmapping the
>  VM on reboot
>
> On a system with FWB, we don't need to unmap Stage-2 on reboot,
> as even if userspace takes this opportunity to repaint the whole
> of memory, FWB ensures that the data side stays consistent even
> if the guest uses non-cacheable mappings.
>
> However, the I-side is not necessarily coherent with the D-side
> if CTR_EL0.DIC is 0. In this case, invalidate the i-cache to
> preserve coherency.
>
> Reported-by: Alexandru Elisei <alexandru.elisei@arm.com>
> Fixes: 892713e97ca1 ("KVM: arm64: Sidestep stage2_unmap_vm() on vcpu reset when
> S2FWB is supported")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/arm.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index b0b569f2cdd0..d6988401c22a 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -989,11 +989,17 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu
> *vcpu,
>       * Ensure a rebooted VM will fault in RAM pages and detect if the
>       * guest MMU is turned off and flush the caches as needed.
>       *
> -     * S2FWB enforces all memory accesses to RAM being cacheable, we
> -     * ensure that the cache is always coherent.
> +     * S2FWB enforces all memory accesses to RAM being cacheable,
> +     * ensuring that the data side is always coherent. We still
> +     * need to invalidate the I-cache though, as FWB does *not*
> +     * imply CTR_EL0.DIC.
>       */
> -    if (vcpu->arch.has_run_once && !cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
> -        stage2_unmap_vm(vcpu->kvm);
> +    if (vcpu->arch.has_run_once) {
> +        if (!cpus_have_final_cap(ARM64_HAS_STAGE2_FWB))
> +            stage2_unmap_vm(vcpu->kvm);
> +        else
> +            __flush_icache_all();
> +    }
>
>      vcpu_reset_hcr(vcpu);
>
>
Looks good, __flush_icache_all checks CTR_EL0.DIC before doing icache maintenance:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] pinctrl: pxa: pxa2xx: Remove 'pxa2xx_pinctrl_exit()' which is unused and broken
From: Christophe JAILLET @ 2020-05-31  7:37 UTC (permalink / raw)
  To: daniel, haojian.zhuang, robert.jarzmik, linus.walleij
  Cc: linux-gpio, Christophe JAILLET, kernel-janitors, linux-kernel,
	linux-arm-kernel

Commit 6d33ee7a0534 ("pinctrl: pxa: Use devm_pinctrl_register() for pinctrl registration")
has turned a 'pinctrl_register()' into 'devm_pinctrl_register()' in
'pxa2xx_pinctrl_init()'.
However, the corresponding 'pinctrl_unregister()' call in
'pxa2xx_pinctrl_exit()' has not been removed.

This is not an issue, because 'pxa2xx_pinctrl_exit()' is unused.
Remove it now to avoid some wondering in the future and save a few LoC.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
If some some reason the function should be kept, at least it should be
only 'return 0;'
---
 drivers/pinctrl/pxa/pinctrl-pxa2xx.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/pinctrl/pxa/pinctrl-pxa2xx.c b/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
index bddf2c5dd3bf..eab029a21643 100644
--- a/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
+++ b/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
@@ -425,15 +425,6 @@ int pxa2xx_pinctrl_init(struct platform_device *pdev,
 }
 EXPORT_SYMBOL_GPL(pxa2xx_pinctrl_init);
 
-int pxa2xx_pinctrl_exit(struct platform_device *pdev)
-{
-	struct pxa_pinctrl *pctl = platform_get_drvdata(pdev);
-
-	pinctrl_unregister(pctl->pctl_dev);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(pxa2xx_pinctrl_exit);
-
 MODULE_AUTHOR("Robert Jarzmik <robert.jarzmik@free.fr>");
 MODULE_DESCRIPTION("Marvell PXA2xx pinctrl driver");
 MODULE_LICENSE("GPL v2");
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: Re: [PATCH v2] i2c: imx-lpi2c: Fix runtime PM imbalance on error in lpi2c_imx_master_enable()
From: dinghao.liu @ 2020-05-31  7:19 UTC (permalink / raw)
  To: Markus Elfring
  Cc: Dong Aisheng, Kangjie Lu, Sascha Hauer, kernel-janitors,
	linux-kernel, linux-imx, kernel, Shawn Guo, Fabio Estevam,
	linux-arm-kernel, linux-i2c
In-Reply-To: <845a11a9-51ed-cc7c-350f-b319111f32ee@web.de>


Hi, Markus, 

> * How do you think about to replace the word “pairing” by “corresponding”?
> 
> * Will it be helpful to add an imperative wording?
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=ffeb595d84811dde16a28b33d8a7cf26d51d51b3#n151
> 
> * Would you like to add the tag “Fixes” to the commit message?
> 
> * Are you going to take such possibilities into account for any more patches?
> 

Thank you for your advice! I will fix them soon in the next version of patch.

Regards,
Dinghao
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* RE: [PATCH v2 1/5] scsi: ufs-mediatek: Fix imprecise waiting time for ref-clk control
From: Avri Altman @ 2020-05-31  7:10 UTC (permalink / raw)
  To: Stanley Chu, linux-scsi@vger.kernel.org,
	martin.petersen@oracle.com, alim.akhtar@samsung.com,
	jejb@linux.ibm.com
  Cc: pengshun.zhao@mediatek.com, bvanassche@acm.org,
	andy.teng@mediatek.com, cc.chou@mediatek.com,
	chun-hung.wu@mediatek.com, kuohong.wang@mediatek.com,
	linux-kernel@vger.kernel.org, cang@codeaurora.org,
	linux-mediatek@lists.infradead.org, peter.wang@mediatek.com,
	matthias.bgg@gmail.com, beanhuo@micron.com,
	chaotian.jing@mediatek.com, linux-arm-kernel@lists.infradead.org,
	asutoshd@codeaurora.org
In-Reply-To: <20200529092310.1106-2-stanley.chu@mediatek.com>

 
> 
> Currently ref-clk control timeout is implemented by Jiffies. However
> jiffies is not accurate enough thus "false timeout" may happen.
> 
> Use more accurate delay mechanism instead, for example, ktime.
> 
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> Reviewed-by: Andy Teng <andy.teng@mediatek.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

> ---
>  drivers/scsi/ufs/ufs-mediatek.c | 7 ++++---
>  drivers/scsi/ufs/ufs-mediatek.h | 2 +-
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
> index d56ce8d97d4e..523ee5573921 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.c
> +++ b/drivers/scsi/ufs/ufs-mediatek.c
> @@ -120,7 +120,7 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba,
> bool on)
>  {
>         struct ufs_mtk_host *host = ufshcd_get_variant(hba);
>         struct arm_smccc_res res;
> -       unsigned long timeout;
> +       ktime_t timeout, time_checked;
>         u32 value;
> 
>         if (host->ref_clk_enabled == on)
> @@ -135,8 +135,9 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba,
> bool on)
>         }
> 
>         /* Wait for ack */
> -       timeout = jiffies + msecs_to_jiffies(REFCLK_REQ_TIMEOUT_MS);
> +       timeout = ktime_add_us(ktime_get(), REFCLK_REQ_TIMEOUT_US);
>         do {
> +               time_checked = ktime_get();
>                 value = ufshcd_readl(hba, REG_UFS_REFCLK_CTRL);
> 
>                 /* Wait until ack bit equals to req bit */
> @@ -144,7 +145,7 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba,
> bool on)
>                         goto out;
> 
>                 usleep_range(100, 200);
> -       } while (time_before(jiffies, timeout));
> +       } while (ktime_before(time_checked, timeout));
Nit: you could get rid of time_checked if you would use ktime_compare(ktime_get(), timeout) > 0

Thanks,
Avri

> 
>         dev_err(hba->dev, "missing ack of refclk req, reg: 0x%x\n", value);
> 
> diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h
> index 5bbd3e9cbae2..fc42dcbfd800 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.h
> +++ b/drivers/scsi/ufs/ufs-mediatek.h
> @@ -28,7 +28,7 @@
>  #define REFCLK_REQUEST              BIT(0)
>  #define REFCLK_ACK                  BIT(1)
> 
> -#define REFCLK_REQ_TIMEOUT_MS       3
> +#define REFCLK_REQ_TIMEOUT_US       3000
> 
>  /*
>   * Vendor specific pre-defined parameters
> --
> 2.18.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 2/3] media: rockchip: Introduce driver for Rockhip's camera interface
From: kbuild test robot @ 2020-05-31  4:39 UTC (permalink / raw)
  To: Maxime Chevallier, Mauro Carvalho Chehab, Robin Murphy,
	Rob Herring, Mark Rutland, Heiko Stuebner, Hans Verkuil
  Cc: devicetree, Maxime Chevallier, kbuild-all, linux-arm-kernel,
	linux-media
In-Reply-To: <20200529130405.929429-3-maxime.chevallier@bootlin.com>

[-- Attachment #1: Type: text/plain, Size: 3528 bytes --]

Hi Maxime,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on robh/for-next v5.7-rc7 next-20200529]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Maxime-Chevallier/media-rockchip-Introduce-driver-for-the-camera-interface-on-PX30/20200531-081952
base:   git://linuxtv.org/media_tree.git master
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

In file included from drivers/media/platform/rockchip/cif/dev.c:22:
>> drivers/media/platform/rockchip/cif/regs.h:113: warning: "FRAME_END" redefined
113 | #define FRAME_END   (0x01 << 0)
|
In file included from arch/x86/include/asm/paravirt.h:18,
from arch/x86/include/asm/spinlock.h:10,
from include/linux/spinlock.h:89,
from include/linux/rwsem.h:16,
from include/linux/notifier.h:15,
from include/linux/clk.h:14,
from drivers/media/platform/rockchip/cif/dev.c:8:
arch/x86/include/asm/frame.h:60: note: this is the location of the previous definition
60 | #define FRAME_END "pop %" _ASM_BP "n"
|
In file included from include/media/v4l2-subdev.h:15,
from include/media/v4l2-device.h:13,
from drivers/media/platform/rockchip/cif/dev.h:15,
from drivers/media/platform/rockchip/cif/dev.c:21:
drivers/media/platform/rockchip/cif/dev.c: In function 'rkcif_plat_probe':
include/media/v4l2-common.h:58:32: warning: 'v4l2_dev' may be used uninitialized in this function [-Wmaybe-uninitialized]
58 |  printk(level "%s: " fmt, (dev)->name , ## arg)
|                                ^~
drivers/media/platform/rockchip/cif/dev.c:216:22: note: 'v4l2_dev' was declared here
216 |  struct v4l2_device *v4l2_dev;
|                      ^~~~~~~~
--
In file included from drivers/media/platform/rockchip/cif/capture.c:22:
>> drivers/media/platform/rockchip/cif/regs.h:113: warning: "FRAME_END" redefined
113 | #define FRAME_END   (0x01 << 0)
|
In file included from arch/x86/include/asm/paravirt.h:18,
from arch/x86/include/asm/spinlock.h:10,
from include/linux/spinlock.h:89,
from include/linux/wait.h:9,
from include/linux/pid.h:6,
from include/linux/sched.h:14,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/pm_runtime.h:11,
from drivers/media/platform/rockchip/cif/capture.c:10:
arch/x86/include/asm/frame.h:60: note: this is the location of the previous definition
60 | #define FRAME_END "pop %" _ASM_BP "n"
|

vim +/FRAME_END +113 drivers/media/platform/rockchip/cif/regs.h

   110	
   111	/* CIF INTSTAT */
   112	#define INTSTAT_CLS			(0x3FF)
 > 113	#define FRAME_END			(0x01 << 0)
   114	#define LINE_ERR			(0x01 << 2)
   115	#define PST_INF_FRAME_END		(0x01 << 9)
   116	#define FRAME_END_CLR			(0x01 << 0)
   117	#define LINE_ERR_CLR			(0x01 << 2)
   118	#define PST_INF_FRAME_END_CLR		(0x01 << 9)
   119	#define INTSTAT_ERR			(0xFC)
   120	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 72524 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] [v2] i2c: imx-lpi2c: Fix runtime PM imbalance on error
From: Dinghao Liu @ 2020-05-31  3:17 UTC (permalink / raw)
  To: dinghao.liu, kjlu
  Cc: Dong Aisheng, Shawn Guo, Sascha Hauer, linux-kernel,
	NXP Linux Team, Pengutronix Kernel Team, Fabio Estevam,
	linux-arm-kernel, linux-i2c

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
---
Changelog:

v2: - Use pm_runtime_put_noidle() instead of
      pm_runtime_put_autosuspend().
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 94743ba581fe..bdee02dff284 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -260,8 +260,10 @@ static int lpi2c_imx_master_enable(struct lpi2c_imx_struct *lpi2c_imx)
 	int ret;
 
 	ret = pm_runtime_get_sync(lpi2c_imx->adapter.dev.parent);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_noidle(lpi2c_imx->adapter.dev.parent);
 		return ret;
+	}
 
 	temp = MCR_RST;
 	writel(temp, lpi2c_imx->base + LPI2C_MCR);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH] irqchip/gic-v3-its: Don't try to move a disabled irq
From: kbuild test robot @ 2020-05-31  2:53 UTC (permalink / raw)
  To: Ali Saidi, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	linux-kernel
  Cc: benh, kbuild-all, zorik, zeev, alisaidi, linux-arm-kernel, dwmw
In-Reply-To: <20200529015501.15771-1-alisaidi@amazon.com>

[-- Attachment #1: Type: text/plain, Size: 5316 bytes --]

Hi Ali,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on linux/master v5.7-rc7]
[cannot apply to tip/irq/core arm-jcooper/irqchip/for-next next-20200529]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Ali-Saidi/irqchip-gic-v3-its-Don-t-try-to-move-a-disabled-irq/20200531-043957
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 86852175b016f0c6873dcbc24b93d12b7b246612
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

drivers/irqchip/irq-gic-v3-its.c: In function 'its_irq_domain_activate':
>> drivers/irqchip/irq-gic-v3-its.c:3449:14: warning: passing argument 1 of 'cpumask_and' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
3449 |  cpumask_and(cpu_mask, cpu_mask, cpu_online_mask);
|              ^~~~~~~~
In file included from include/linux/rcupdate.h:31,
from include/linux/radix-tree.h:15,
from include/linux/idr.h:15,
from include/linux/kernfs.h:13,
from include/linux/sysfs.h:16,
from include/linux/kobject.h:20,
from include/linux/of.h:17,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from drivers/irqchip/irq-gic-v3-its.c:7:
include/linux/cpumask.h:424:47: note: expected 'struct cpumask *' but argument is of type 'const struct cpumask *'
424 | static inline int cpumask_and(struct cpumask *dstp,
|                               ~~~~~~~~~~~~~~~~^~~~
In file included from include/linux/bits.h:23,
from include/linux/ioport.h:15,
from include/linux/acpi.h:12,
from drivers/irqchip/irq-gic-v3-its.c:7:
drivers/irqchip/irq-gic-v3-its.c: In function 'its_init_vpe_domain':
include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
26 |   __builtin_constant_p((l) > (h)), (l) > (h), 0)))
|                            ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
|                                                              ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|   ^~~~~~~~~~~~~~~~~~~
drivers/irqchip/irq-gic-v3-its.c:4765:10: note: in expansion of macro 'GENMASK'
4765 |  devid = GENMASK(device_ids(its) - 1, 0);
|          ^~~~~~~
include/linux/bits.h:26:40: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
26 |   __builtin_constant_p((l) > (h)), (l) > (h), 0)))
|                                        ^
include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
|                                                              ^
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|   ^~~~~~~~~~~~~~~~~~~
drivers/irqchip/irq-gic-v3-its.c:4765:10: note: in expansion of macro 'GENMASK'
4765 |  devid = GENMASK(device_ids(its) - 1, 0);
|          ^~~~~~~

vim +3449 drivers/irqchip/irq-gic-v3-its.c

  3433	
  3434	static int its_irq_domain_activate(struct irq_domain *domain,
  3435					   struct irq_data *d, bool reserve)
  3436	{
  3437		struct its_device *its_dev = irq_data_get_irq_chip_data(d);
  3438		u32 event = its_get_event_id(d);
  3439		const struct cpumask *cpu_mask = cpu_online_mask;
  3440		int cpu;
  3441	
  3442		/* get the cpu_mask of local node */
  3443		if (its_dev->its->numa_node >= 0)
  3444			cpu_mask = cpumask_of_node(its_dev->its->numa_node);
  3445	
  3446		/* If the cpu set to a different CPU that is still online use it */
  3447		cpu = its_dev->event_map.col_map[event];
  3448	
> 3449		cpumask_and(cpu_mask, cpu_mask, cpu_online_mask);
  3450	
  3451		if (!cpumask_test_cpu(cpu, cpu_mask)) {
  3452			/* Bind the LPI to the first possible CPU */
  3453			cpu = cpumask_first(cpu_mask);
  3454		}
  3455	
  3456		if (cpu >= nr_cpu_ids) {
  3457			if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144)
  3458				return -EINVAL;
  3459	
  3460			cpu = cpumask_first(cpu_online_mask);
  3461		}
  3462	
  3463		its_dev->event_map.col_map[event] = cpu;
  3464		irq_data_update_effective_affinity(d, cpumask_of(cpu));
  3465	
  3466		/* Map the GIC IRQ and event to the device */
  3467		its_send_mapti(its_dev, d->hwirq, event);
  3468		return 0;
  3469	}
  3470	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 71779 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v5 2/6] interconnect: Add generic interconnect driver for Exynos SoCs
From: Chanwoo Choi @ 2020-05-31  0:13 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, Seung-Woo Kim,
	Artur Świgoń, Krzysztof Kozlowski, linux-kernel,
	inki.dae, Chanwoo Choi, MyungJoo Ham, dri-devel, Georgi Djakov,
	linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20200529163200.18031-3-s.nawrocki@samsung.com>

Hi Sylwester,

On Sat, May 30, 2020 at 1:34 AM Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
>
> This patch adds a generic interconnect driver for Exynos SoCs in order
> to provide interconnect functionality for each "samsung,exynos-bus"
> compatible device.
>
> The SoC topology is a graph (or more specifically, a tree) and its
> edges are specified using the 'samsung,interconnect-parent' in the
> DT. Due to unspecified relative probing order, -EPROBE_DEFER may be
> propagated to ensure that the parent is probed before its children.
>
> Each bus is now an interconnect provider and an interconnect node as
> well (cf. Documentation/interconnect/interconnect.rst), i.e. every bus
> registers itself as a node. Node IDs are not hardcoded but rather
> assigned dynamically at runtime. This approach allows for using this
> driver with various Exynos SoCs.
>
> Frequencies requested via the interconnect API for a given node are
> propagated to devfreq using dev_pm_qos_update_request(). Please note
> that it is not an error when CONFIG_INTERCONNECT is 'n', in which
> case all interconnect API functions are no-op.
>
> Signed-off-by: Artur Świgoń <a.swigon@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>
> Changes for v5:
>  - adjust to renamed exynos,interconnect-parent-node property,
>  - use automatically generated platform device id as the interconect
>    node id instead of a now unavailable devfreq->id field,
>  - add icc_ prefix to some variables to make the code more self-commenting,
>  - use icc_nodes_remove() instead of icc_node_del() + icc_node_destroy(),
>  - adjust to exynos,interconnect-parent-node property rename to
>    samsung,interconnect-parent,
>  - converted to a separate platform driver in drivers/interconnect.
> ---
>  drivers/interconnect/Kconfig         |   1 +
>  drivers/interconnect/Makefile        |   1 +
>  drivers/interconnect/exynos/Kconfig  |   6 ++
>  drivers/interconnect/exynos/Makefile |   4 +
>  drivers/interconnect/exynos/exynos.c | 185 +++++++++++++++++++++++++++++++++++
>  5 files changed, 197 insertions(+)
>  create mode 100644 drivers/interconnect/exynos/Kconfig
>  create mode 100644 drivers/interconnect/exynos/Makefile
>  create mode 100644 drivers/interconnect/exynos/exynos.c
>
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> index 5b7204e..eca6eda 100644
> --- a/drivers/interconnect/Kconfig
> +++ b/drivers/interconnect/Kconfig
> @@ -11,6 +11,7 @@ menuconfig INTERCONNECT
>
>  if INTERCONNECT
>
> +source "drivers/interconnect/exynos/Kconfig"
>  source "drivers/interconnect/imx/Kconfig"
>  source "drivers/interconnect/qcom/Kconfig"
>
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> index 4825c28..2ba1de6 100644
> --- a/drivers/interconnect/Makefile
> +++ b/drivers/interconnect/Makefile
> @@ -4,5 +4,6 @@ CFLAGS_core.o                           := -I$(src)
>  icc-core-objs                          := core.o
>
>  obj-$(CONFIG_INTERCONNECT)             += icc-core.o
> +obj-$(CONFIG_INTERCONNECT_EXYNOS)      += exynos/
>  obj-$(CONFIG_INTERCONNECT_IMX)         += imx/
>  obj-$(CONFIG_INTERCONNECT_QCOM)                += qcom/
> diff --git a/drivers/interconnect/exynos/Kconfig b/drivers/interconnect/exynos/Kconfig
> new file mode 100644
> index 0000000..e51e52e
> --- /dev/null
> +++ b/drivers/interconnect/exynos/Kconfig
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config INTERCONNECT_EXYNOS
> +       tristate "Exynos generic interconnect driver"
> +       depends on ARCH_EXYNOS || COMPILE_TEST
> +       help
> +         Generic interconnect driver for Exynos SoCs.
> diff --git a/drivers/interconnect/exynos/Makefile b/drivers/interconnect/exynos/Makefile
> new file mode 100644
> index 0000000..e19d1df
> --- /dev/null
> +++ b/drivers/interconnect/exynos/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +exynos-interconnect-objs               := exynos.o
> +
> +obj-$(CONFIG_INTERCONNECT_EXYNOS)      += exynos-interconnect.o
> diff --git a/drivers/interconnect/exynos/exynos.c b/drivers/interconnect/exynos/exynos.c
> new file mode 100644
> index 0000000..8278194
> --- /dev/null
> +++ b/drivers/interconnect/exynos/exynos.c
> @@ -0,0 +1,185 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Exynos generic interconnect provider driver
> + *
> + * Copyright (c) 2020 Samsung Electronics Co., Ltd.
> + *
> + * Authors: Artur Świgoń <a.swigon@samsung.com>
> + *          Sylwester Nawrocki <s.nawrocki@samsung.com>
> + */
> +#include <linux/device.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_qos.h>
> +
> +#define kbps_to_khz(x) ((x) / 8)
> +
> +struct exynos_icc_priv {
> +       struct device *dev;
> +
> +       /* One interconnect node per provider */
> +       struct icc_provider provider;
> +       struct icc_node *node;
> +
> +       struct dev_pm_qos_request qos_req;
> +};
> +
> +static struct icc_node *exynos_icc_get_parent(struct device_node *np)
> +{
> +       struct of_phandle_args args;
> +       int num, ret;
> +
> +       num = of_count_phandle_with_args(np, "samsung,interconnect-parent",
> +                                       "#interconnect-cells");
> +       if (num != 1)
> +               return NULL; /* parent nodes are optional */
> +
> +       ret = of_parse_phandle_with_args(np, "samsung,interconnect-parent",
> +                                       "#interconnect-cells", 0, &args);
> +       if (ret < 0)
> +               return ERR_PTR(ret);
> +
> +       of_node_put(args.np);
> +
> +       return of_icc_get_from_provider(&args);
> +}
> +
> +
> +static int exynos_generic_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> +       struct exynos_icc_priv *src_priv = src->data, *dst_priv = dst->data;
> +       s32 src_freq = kbps_to_khz(max(src->avg_bw, src->peak_bw));
> +       s32 dst_freq = kbps_to_khz(max(dst->avg_bw, dst->peak_bw));
> +       int ret;
> +
> +       ret = dev_pm_qos_update_request(&src_priv->qos_req, src_freq);
> +       if (ret < 0) {
> +               dev_err(src_priv->dev, "failed to update PM QoS of %s\n",
> +                       src->name);
> +               return ret;
> +       }
> +
> +       ret = dev_pm_qos_update_request(&dst_priv->qos_req, dst_freq);
> +       if (ret < 0) {
> +               dev_err(dst_priv->dev, "failed to update PM QoS of %s\n",
> +                       dst->name);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static struct icc_node *exynos_generic_icc_xlate(struct of_phandle_args *spec,
> +                                                void *data)
> +{
> +       struct exynos_icc_priv *priv = data;
> +
> +       if (spec->np != priv->dev->parent->of_node)
> +               return ERR_PTR(-EINVAL);
> +
> +       return priv->node;
> +}
> +
> +static int exynos_generic_icc_remove(struct platform_device *pdev)
> +{
> +       struct exynos_icc_priv *priv = platform_get_drvdata(pdev);
> +       struct icc_node *parent_node, *node = priv->node;
> +
> +       parent_node = exynos_icc_get_parent(priv->dev->parent->of_node);
> +       if (parent_node && !IS_ERR(parent_node))
> +               icc_link_destroy(node, parent_node);
> +
> +       icc_nodes_remove(&priv->provider);
> +       icc_provider_del(&priv->provider);
> +
> +       return 0;
> +}
> +
> +static int exynos_generic_icc_probe(struct platform_device *pdev)
> +{
> +       struct device *bus_dev = pdev->dev.parent;
> +       struct exynos_icc_priv *priv;
> +       struct icc_provider *provider;
> +       struct icc_node *icc_node, *icc_parent_node;
> +       int ret;
> +
> +       priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->dev = &pdev->dev;
> +       platform_set_drvdata(pdev, priv);
> +
> +       provider = &priv->provider;
> +
> +       provider->set = exynos_generic_icc_set;
> +       provider->aggregate = icc_std_aggregate;
> +       provider->xlate = exynos_generic_icc_xlate;
> +       provider->dev = bus_dev;
> +       provider->inter_set = true;
> +       provider->data = priv;
> +
> +       ret = icc_provider_add(provider);
> +       if (ret < 0)
> +               return ret;
> +
> +       icc_node = icc_node_create(pdev->id);
> +       if (IS_ERR(icc_node)) {
> +               ret = PTR_ERR(icc_node);
> +               goto err_prov_del;
> +       }
> +
> +       priv->node = icc_node;
> +       icc_node->name = bus_dev->of_node->name;
> +       icc_node->data = priv;
> +       icc_node_add(icc_node, provider);
> +
> +       icc_parent_node = exynos_icc_get_parent(bus_dev->of_node);
> +       if (IS_ERR(icc_parent_node)) {
> +               ret = PTR_ERR(icc_parent_node);
> +               goto err_node_del;
> +       }
> +       if (icc_parent_node) {
> +               ret = icc_link_create(icc_node, icc_parent_node->id);
> +               if (ret < 0)
> +                       goto err_node_del;
> +       }
> +
> +       /*
> +        * Register a PM QoS request for the bus device for which also devfreq
> +        * functionality is registered.
> +        */
> +       ret = dev_pm_qos_add_request(bus_dev, &priv->qos_req,
> +                                    DEV_PM_QOS_MIN_FREQUENCY, 0);
> +       if (ret < 0)
> +               goto err_link_destroy;
> +
> +       return 0;
> +
> +err_link_destroy:
> +       if (icc_parent_node)
> +               icc_link_destroy(icc_node, icc_parent_node);
> +err_node_del:
> +       icc_nodes_remove(provider);
> +err_prov_del:
> +       icc_provider_del(provider);
> +
> +       return ret;
> +}
> +
> +static struct platform_driver exynos_generic_icc_driver = {
> +       .driver = {
> +               .name = "exynos-generic-icc",
> +       },
> +       .probe = exynos_generic_icc_probe,
> +       .remove = exynos_generic_icc_remove,
> +};
> +module_platform_driver(exynos_generic_icc_driver);
> +
> +MODULE_DESCRIPTION("Exynos generic interconnect driver");
> +MODULE_AUTHOR("Artur Świgoń <a.swigon@samsung.com>");
> +MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:exynos-generic-icc");
> --
> 2.7.4
>

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

As I commented, How about changing the compatible name 'exynos-icc'
without 'generic'?
The 'exynos-icc' doesn't have the any specific version of Exynos SoC,
it think that it already contain the 'generic' meaning for Exynos SoC.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v3 4/4] pinctrl: bcm2835: Add support for wake-up interrupts
From: Florian Fainelli @ 2020-05-31  0:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Stefan Wahren,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Florian Fainelli, Geert Uytterhoeven, Scott Branden, Ray Jui,
	Linus Walleij, Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM,
	Rob Herring,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Nicolas Saenz Julienne,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200531001101.24945-1-f.fainelli@gmail.com>

Leverage the IRQCHIP_MASK_ON_SUSPEND flag in order to avoid having to
specifically treat the GPIO interrupts during suspend and resume, and
simply implement an irq_set_wake() callback that is responsible for
enabling the parent wake-up interrupt as a wake-up interrupt.

To avoid allocating unnecessary resources for other chips, the wake-up
interrupts are only initialized if we have a brcm,bcm7211-gpio
compatibility string.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 76 ++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 1b00d93aa66e..1d21129f7751 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -19,6 +19,7 @@
 #include <linux/irq.h>
 #include <linux/irqdesc.h>
 #include <linux/init.h>
+#include <linux/interrupt.h>
 #include <linux/of_address.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
@@ -76,6 +77,7 @@
 struct bcm2835_pinctrl {
 	struct device *dev;
 	void __iomem *base;
+	int *wake_irq;
 
 	/* note: locking assumes each bank will have its own unsigned long */
 	unsigned long enabled_irq_map[BCM2835_NUM_BANKS];
@@ -435,6 +437,11 @@ static void bcm2835_gpio_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(host_chip, desc);
 }
 
+static irqreturn_t bcm2835_gpio_wake_irq_handler(int irq, void *dev_id)
+{
+	return IRQ_HANDLED;
+}
+
 static inline void __bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc,
 	unsigned reg, unsigned offset, bool enable)
 {
@@ -634,6 +641,34 @@ static void bcm2835_gpio_irq_ack(struct irq_data *data)
 	bcm2835_gpio_set_bit(pc, GPEDS0, gpio);
 }
 
+static int bcm2835_gpio_irq_set_wake(struct irq_data *data, unsigned int on)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
+	unsigned gpio = irqd_to_hwirq(data);
+	unsigned int irqgroup;
+	int ret = -EINVAL;
+
+	if (!pc->wake_irq)
+		return ret;
+
+	if (gpio <= 27)
+		irqgroup = 0;
+	else if (gpio >= 28 && gpio <= 45)
+		irqgroup = 1;
+	else if (gpio >= 46 && gpio <= 57)
+		irqgroup = 2;
+	else
+		return ret;
+
+	if (on)
+		ret = enable_irq_wake(pc->wake_irq[irqgroup]);
+	else
+		ret = disable_irq_wake(pc->wake_irq[irqgroup]);
+
+	return ret;
+}
+
 static struct irq_chip bcm2835_gpio_irq_chip = {
 	.name = MODULE_NAME,
 	.irq_enable = bcm2835_gpio_irq_enable,
@@ -642,6 +677,8 @@ static struct irq_chip bcm2835_gpio_irq_chip = {
 	.irq_ack = bcm2835_gpio_irq_ack,
 	.irq_mask = bcm2835_gpio_irq_disable,
 	.irq_unmask = bcm2835_gpio_irq_enable,
+	.irq_set_wake = bcm2835_gpio_irq_set_wake,
+	.flags = IRQCHIP_MASK_ON_SUSPEND,
 };
 
 static int bcm2835_pctl_get_groups_count(struct pinctrl_dev *pctldev)
@@ -1154,6 +1191,7 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 	struct resource iomem;
 	int err, i;
 	const struct of_device_id *match;
+	int is_7211 = 0;
 
 	BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_pins) != BCM2711_NUM_GPIOS);
 	BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_groups) != BCM2711_NUM_GPIOS);
@@ -1180,6 +1218,7 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	pdata = match->data;
+	is_7211 = of_device_is_compatible(np, "brcm,bcm7211-gpio");
 
 	pc->gpio_chip = *pdata->gpio_chip;
 	pc->gpio_chip.parent = dev;
@@ -1214,6 +1253,15 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 				     GFP_KERNEL);
 	if (!girq->parents)
 		return -ENOMEM;
+
+	if (is_7211) {
+		pc->wake_irq = devm_kcalloc(dev, BCM2835_NUM_IRQS,
+					    sizeof(*pc->wake_irq),
+					    GFP_KERNEL);
+		if (!pc->wake_irq)
+			return -ENOMEM;
+	}
+
 	/*
 	 * Use the same handler for all groups: this is necessary
 	 * since we use one gpiochip to cover all lines - the
@@ -1221,8 +1269,34 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 	 * bank that was firing the IRQ and look up the per-group
 	 * and bank data.
 	 */
-	for (i = 0; i < BCM2835_NUM_IRQS; i++)
+	for (i = 0; i < BCM2835_NUM_IRQS; i++) {
+		int len;
+		char *name;
+
 		girq->parents[i] = irq_of_parse_and_map(np, i);
+		if (!is_7211)
+			continue;
+
+		/* Skip over the all banks interrupts */
+		pc->wake_irq[i] = irq_of_parse_and_map(np, i +
+						       BCM2835_NUM_IRQS + 1);
+
+		len = strlen(dev_name(pc->dev)) + 16;
+		name = devm_kzalloc(pc->dev, len, GFP_KERNEL);
+		if (!name)
+			return -ENOMEM;
+
+		snprintf(name, len, "%s:bank%d", dev_name(pc->dev), i);
+
+		/* These are optional interrupts */
+		err = devm_request_irq(dev, pc->wake_irq[i],
+				       bcm2835_gpio_wake_irq_handler,
+				       IRQF_SHARED, name, pc);
+		if (err)
+			dev_warn(dev, "unable to request wake IRQ %d\n",
+				 pc->wake_irq[i]);
+	}
+
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_level_irq;
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v3 3/4] pinctrl: bcm2835: Match BCM7211 compatible string
From: Florian Fainelli @ 2020-05-31  0:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Stefan Wahren,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Florian Fainelli, Geert Uytterhoeven, Scott Branden, Ray Jui,
	Linus Walleij, Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM,
	Rob Herring,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Nicolas Saenz Julienne,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200531001101.24945-1-f.fainelli@gmail.com>

The BCM7211 SoC uses the same pinconf_ops as the ones defined for the
BCM2711 SoC, match the compatible string and use the correct set of
options.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 06bd2b70af3c..1b00d93aa66e 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -1137,6 +1137,10 @@ static const struct of_device_id bcm2835_pinctrl_match[] = {
 		.compatible = "brcm,bcm2711-gpio",
 		.data = &bcm2711_plat_data,
 	},
+	{
+		.compatible = "brcm,bcm7211-gpio",
+		.data = &bcm2711_plat_data,
+	},
 	{}
 };
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v3 2/4] dt-bindings: pinctrl: Document optional BCM7211 wake-up interrupts
From: Florian Fainelli @ 2020-05-31  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Stefan Wahren,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Florian Fainelli, Geert Uytterhoeven, Scott Branden, Ray Jui,
	Linus Walleij, Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM,
	Rob Herring,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Nicolas Saenz Julienne,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200531001101.24945-1-f.fainelli@gmail.com>

BCM7211 supports wake-up interrupts in the form of optional interrupt
lines, one per bank, plus the "all banks" interrupt line.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt         | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
index dfc67b90591c..5682b2010e50 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
@@ -16,7 +16,9 @@ Required properties:
   second cell is used to specify optional parameters:
   - bit 0 specifies polarity (0 for normal, 1 for inverted)
 - interrupts : The interrupt outputs from the controller. One interrupt per
-  individual bank followed by the "all banks" interrupt.
+  individual bank followed by the "all banks" interrupt. For BCM7211, an
+  additional set of per-bank interrupt line and an "all banks" wake-up
+  interrupt may be specified.
 - interrupt-controller: Marks the device node as an interrupt controller.
 - #interrupt-cells : Should be 2.
   The first cell is the GPIO number.
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v3 1/4] dt-bindings: pinctrl: Document 7211 compatible for brcm, bcm2835-gpio.txt
From: Florian Fainelli @ 2020-05-31  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Stefan Wahren,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Florian Fainelli, Geert Uytterhoeven, Scott Branden, Ray Jui,
	Linus Walleij, Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM,
	Rob Herring,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Nicolas Saenz Julienne,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200531001101.24945-1-f.fainelli@gmail.com>

Document the brcm,bcm7211-gpio compatible string in the
brcm,bcm2835-gpio.txt document.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
index 3cab7336a326..dfc67b90591c 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
@@ -9,6 +9,7 @@ Required properties:
   "brcm,bcm2835-gpio" - BCM2835 compatible pinctrl
   "brcm,bcm7211-gpio" - BCM7211 compatible pinctrl
   "brcm,bcm2711-gpio" - BCM2711 compatible pinctrl
+  "brcm,bcm7211-gpio" - BCM7211 compatible pinctrl
 - reg: Should contain the physical address of the GPIO module's registers.
 - gpio-controller: Marks the device node as a GPIO controller.
 - #gpio-cells : Should be two. The first cell is the pin number and the
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v3 0/4] pinctrl: bcm2835: Add support for wake-up interrupts
From: Florian Fainelli @ 2020-05-31  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Stefan Wahren,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Florian Fainelli, Geert Uytterhoeven, Scott Branden, Ray Jui,
	Linus Walleij, Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM,
	Rob Herring,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Nicolas Saenz Julienne,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE

Hi Linus,

This patch series updates the bcm2835 pinctrl driver to support
the BCM7211 SoC which is quite similar to 2711 (Raspberry Pi 4)
except that it also supports wake-up interrupts.

Thanks!

Changes in v3:

- added Rob's Acked-by for the binding patches (#1 and #2)
- correct check on the number of GPIOs in irq_set_irq_wake (Stefan)

Changes in v2:

- fixed patch #3 to reference the correct data structure (Stefan)
- fixed patch #4 to use conditional initialization and fetching of
  interrupt resources to limit the memory overhead for non-7211 chips

Florian Fainelli (4):
  dt-bindings: pinctrl: Document 7211 compatible for
    brcm,bcm2835-gpio.txt
  dt-bindings: pinctrl: Document optional BCM7211 wake-up interrupts
  pinctrl: bcm2835: Match BCM7211 compatible string
  pinctrl: bcm2835: Add support for wake-up interrupts

 .../bindings/pinctrl/brcm,bcm2835-gpio.txt    |  5 +-
 drivers/pinctrl/bcm/pinctrl-bcm2835.c         | 80 ++++++++++++++++++-
 2 files changed, 83 insertions(+), 2 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v5 5/6] ARM: dts: exynos: Add interconnects to Exynos4412 mixer
From: Chanwoo Choi @ 2020-05-31  0:07 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, Seung-Woo Kim,
	Artur Świgoń, Krzysztof Kozlowski, linux-kernel,
	inki.dae, Chanwoo Choi, MyungJoo Ham, dri-devel, Georgi Djakov,
	linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20200529163200.18031-6-s.nawrocki@samsung.com>

Hi Sylwester,

On Sat, May 30, 2020 at 1:33 AM Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
>
> From: Artur Świgoń <a.swigon@samsung.com>
>
> This patch adds an 'interconnects' property to Exynos4412 DTS in order to
> declare the interconnect path used by the mixer. Please note that the
> 'interconnect-names' property is not needed when there is only one path in
> 'interconnects', in which case calling of_icc_get() with a NULL name simply
> returns the right path.
>
> Signed-off-by: Artur Świgoń <a.swigon@samsung.com>
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
> Changes for v5:
>  - none.
> ---
>  arch/arm/boot/dts/exynos4412.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
> index a7496d3..eee86d2 100644
> --- a/arch/arm/boot/dts/exynos4412.dtsi
> +++ b/arch/arm/boot/dts/exynos4412.dtsi
> @@ -776,6 +776,7 @@
>         clock-names = "mixer", "hdmi", "sclk_hdmi", "vp";
>         clocks = <&clock CLK_MIXER>, <&clock CLK_HDMI>,
>                  <&clock CLK_SCLK_HDMI>, <&clock CLK_VP>;
> +       interconnects = <&bus_display &bus_dmc>;

I think it is really good and necessary in order to support the
minimum bandwidth.
Until now, I had to add the additional code to support for this same purpose
into product code.

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v5 4/6] ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes
From: Chanwoo Choi @ 2020-05-31  0:02 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, Seung-Woo Kim,
	Artur Świgoń, Krzysztof Kozlowski, linux-kernel,
	inki.dae, Chanwoo Choi, MyungJoo Ham, dri-devel, Georgi Djakov,
	linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20200529163200.18031-5-s.nawrocki@samsung.com>

Hi Sylwester,

On Sat, May 30, 2020 at 1:33 AM Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
>
> This patch adds the following properties for Exynos4412 interconnect
> bus nodes:
>  - samsung,interconnect-parent: to declare connections between
>    nodes in order to guarantee PM QoS requirements between nodes;
>  - #interconnect-cells: required by the interconnect framework.
>
> Note that #interconnect-cells is always zero and node IDs are not
> hardcoded anywhere.
>
> Signed-off-by: Artur Świgoń <a.swigon@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes for v5:
>  - adjust to renamed exynos,interconnect-parent-node property,
>  - add properties in common exynos4412.dtsi file rather than
>    in Odroid specific odroid4412-odroid-common.dtsi.
> ---
>  arch/arm/boot/dts/exynos4412.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
> index 4886894..a7496d3 100644
> --- a/arch/arm/boot/dts/exynos4412.dtsi
> +++ b/arch/arm/boot/dts/exynos4412.dtsi
> @@ -381,6 +381,7 @@
>                         clocks = <&clock CLK_DIV_DMC>;
>                         clock-names = "bus";
>                         operating-points-v2 = <&bus_dmc_opp_table>;
> +                       #interconnect-cells = <0>;
>                         status = "disabled";
>                 };
>
> @@ -450,6 +451,8 @@
>                         clocks = <&clock CLK_DIV_GDL>;
>                         clock-names = "bus";
>                         operating-points-v2 = <&bus_leftbus_opp_table>;
> +                       samsung,interconnect-parent = <&bus_dmc>;
> +                       #interconnect-cells = <0>;
>                         status = "disabled";
>                 };
>
> @@ -466,6 +469,8 @@
>                         clocks = <&clock CLK_ACLK160>;
>                         clock-names = "bus";
>                         operating-points-v2 = <&bus_display_opp_table>;
> +                       samsung,interconnect-parent = <&bus_leftbus>;
> +                       #interconnect-cells = <0>;
>                         status = "disabled";
>                 };
>
> --
> 2.7.4
>

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v5 1/6] dt-bindings: exynos-bus: Add documentation for interconnect properties
From: Chanwoo Choi @ 2020-05-31  0:01 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, Seung-Woo Kim,
	Artur Świgoń, Krzysztof Kozlowski, linux-kernel,
	inki.dae, Chanwoo Choi, MyungJoo Ham, dri-devel, Georgi Djakov,
	linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20200529163200.18031-2-s.nawrocki@samsung.com>

Hi Sylwester,


On Sat, May 30, 2020 at 1:32 AM Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
>
> Add documentation for new optional properties in the exynos bus nodes:
> samsung,interconnect-parent, #interconnect-cells.
> These properties allow to specify the SoC interconnect structure which
> then allows the interconnect consumer devices to request specific
> bandwidth requirements.
>
> Signed-off-by: Artur Świgoń <a.swigon@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes for v5:
>  - exynos,interconnect-parent-node renamed to samsung,interconnect-parent
> ---
>  Documentation/devicetree/bindings/devfreq/exynos-bus.txt | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> index e71f752..e0d2daa 100644
> --- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> @@ -51,6 +51,11 @@ Optional properties only for parent bus device:
>  - exynos,saturation-ratio: the percentage value which is used to calibrate
>                         the performance count against total cycle count.
>
> +Optional properties for interconnect functionality (QoS frequency constraints):
> +- samsung,interconnect-parent: phandle to the parent interconnect node; for
> +  passive devices should point to same node as the exynos,parent-bus property.
> +- #interconnect-cells: should be 0
> +
>  Detailed correlation between sub-blocks and power line according to Exynos SoC:
>  - In case of Exynos3250, there are two power line as following:
>         VDD_MIF |--- DMC
> @@ -185,8 +190,9 @@ Example1:
>         ----------------------------------------------------------
>
>  Example2 :
> -       The bus of DMC (Dynamic Memory Controller) block in exynos3250.dtsi
> -       is listed below:
> +       The bus of DMC (Dynamic Memory Controller) block in exynos3250.dtsi is
> +       listed below. An interconnect path "bus_lcd0 -- bus_leftbus -- bus_dmc"
> +       is defined for demonstration purposes.
>
>         bus_dmc: bus_dmc {
>                 compatible = "samsung,exynos-bus";
> @@ -376,12 +382,15 @@ Example2 :
>         &bus_dmc {
>                 devfreq-events = <&ppmu_dmc0_3>, <&ppmu_dmc1_3>;
>                 vdd-supply = <&buck1_reg>;      /* VDD_MIF */
> +               #interconnect-cells = <0>;
>                 status = "okay";
>         };
>
>         &bus_leftbus {
>                 devfreq-events = <&ppmu_leftbus_3>, <&ppmu_rightbus_3>;
>                 vdd-supply = <&buck3_reg>;
> +               samsung,interconnect-parent = <&bus_dmc>;
> +               #interconnect-cells = <0>;
>                 status = "okay";
>         };
>
> @@ -392,6 +401,8 @@ Example2 :
>
>         &bus_lcd0 {
>                 devfreq = <&bus_leftbus>;
> +               samsung,interconnect-parent = <&bus_leftbus>;
> +               #interconnect-cells = <0>;
>                 status = "okay";
>         };
>
> --
> 2.7.4
>

If you add the usage example like the mixer device of patch5 to this
dt-binding document,
I think it is very beneficial and more helpful for user of
exynos-bus/exynos-generic-icc.

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v5 3/6] PM / devfreq: exynos-bus: Add registration of interconnect child device
From: Chanwoo Choi @ 2020-05-30 23:57 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, Seung-Woo Kim,
	Artur Świgoń, Krzysztof Kozlowski, linux-kernel,
	inki.dae, Chanwoo Choi, MyungJoo Ham, dri-devel, Georgi Djakov,
	linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20200529163200.18031-4-s.nawrocki@samsung.com>

Hi Sylwester,

On Sat, May 30, 2020 at 1:33 AM Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
>
> This patch adds registration of a child platform device for the exynos
> interconnect driver. It is assumed that the interconnect provider will
> only be needed when #interconnect-cells property is present in the bus
> DT node, hence the child device will be created only when such a property
> is present.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>
> Changes for v5:
>  - new patch.
> ---
>  drivers/devfreq/exynos-bus.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 8fa8eb5..856e37d 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -24,6 +24,7 @@
>
>  struct exynos_bus {
>         struct device *dev;
> +       struct platform_device *icc_pdev;
>
>         struct devfreq *devfreq;
>         struct devfreq_event_dev **edev;
> @@ -156,6 +157,8 @@ static void exynos_bus_exit(struct device *dev)
>         if (ret < 0)
>                 dev_warn(dev, "failed to disable the devfreq-event devices\n");
>
> +       platform_device_unregister(bus->icc_pdev);
> +
>         dev_pm_opp_of_remove_table(dev);
>         clk_disable_unprepare(bus->clk);
>         if (bus->opp_table) {
> @@ -168,6 +171,8 @@ static void exynos_bus_passive_exit(struct device *dev)
>  {
>         struct exynos_bus *bus = dev_get_drvdata(dev);
>
> +       platform_device_unregister(bus->icc_pdev);
> +
>         dev_pm_opp_of_remove_table(dev);
>         clk_disable_unprepare(bus->clk);
>  }
> @@ -431,6 +436,18 @@ static int exynos_bus_probe(struct platform_device *pdev)
>         if (ret < 0)
>                 goto err;
>
> +       /* Create child platform device for the interconnect provider */
> +       if (of_get_property(dev->of_node, "#interconnect-cells", NULL)) {
> +                   bus->icc_pdev = platform_device_register_data(
> +                                               dev, "exynos-generic-icc",
> +                                               PLATFORM_DEVID_AUTO, NULL, 0);
> +
> +                   if (IS_ERR(bus->icc_pdev)) {
> +                           ret = PTR_ERR(bus->icc_pdev);
> +                           goto err;
> +                   }
> +       }
> +
>         max_state = bus->devfreq->profile->max_state;
>         min_freq = (bus->devfreq->profile->freq_table[0] / 1000);
>         max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000);
> --
> 2.7.4
>

It looks like very similar like the registering the interconnect
device of imx-bus.c
and I already reviewed and agreed this approach.

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

nitpick: IMHO, I think that 'exynos-icc' is proper and simple without
'generic' word.
If we need to add new icc compatible int the future, we will add
'exynosXXXX-icc' new compatible.
But, I'm not forcing it. just opinion. Anyway, I agree this approach.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 4/4] pinctrl: bcm2835: Add support for wake-up interrupts
From: Stefan Wahren @ 2020-05-30 23:30 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Geert Uytterhoeven, Scott Branden, Ray Jui, Linus Walleij,
	Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM, Rob Herring,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Nicolas Saenz Julienne,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <bf0ceef7-4015-69f1-336c-4d296f96adae@gmail.com>

Hi Florian,

Am 30.05.20 um 23:19 schrieb Florian Fainelli:
>
> On 5/30/2020 12:49 AM, Stefan Wahren wrote:
>> Hi Florian,
>>
>> Am 29.05.20 um 21:15 schrieb Florian Fainelli:
>>>  }
>>>  
>>> +static int bcm2835_gpio_irq_set_wake(struct irq_data *data, unsigned int on)
>>> +{
>>> +	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
>>> +	struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
>>> +	unsigned gpio = irqd_to_hwirq(data);
>>> +	unsigned int irqgroup;
>>> +	int ret = -EINVAL;
>>> +
>>> +	if (!pc->wake_irq)
>>> +		return ret;
>>> +
>>> +	if (gpio <= 27)
>>> +		irqgroup = 0;
>>> +	else if (gpio >= 28 && gpio <= 45)
>>> +		irqgroup = 1;
>>> +	else if (gpio >= 46 && gpio <= 53)
>>> +		irqgroup = 2;
>> in case the BCM7211 has 58 GPIOs, but the wake up interrupts are only
>> available for the first 54 this should deserve a comment.
> irqgroup 2 covers GPIOs 46 through 57, thanks for noticing. Do you have
> more comments before I spin a v3? Thank you for reviewing.

no, i don't.

Regards
Stefan


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [clk:clk-mediatek 5/6] drivers/clk/mediatek/clk-mt6765.c:537:35: warning: unused variable 'ifr0_cg_regs'
From: kbuild test robot @ 2020-05-30 22:25 UTC (permalink / raw)
  To: Owen, Chen,
  Cc: kbuild-all, Stephen Boyd, clang-built-linux, Mars Cheng,
	Macpaul Lin, linux-clk, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-mediatek
head:   571cfadcc628dd5591444f7289e27445ea732f4c
commit: 1aca9939bf72893887cb7e3455e44c864bada2f9 [5/6] clk: mediatek: Add MT6765 clock support
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 2388a096e7865c043e83ece4e26654bd3d1a20d5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        git checkout 1aca9939bf72893887cb7e3455e44c864bada2f9
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/clk/mediatek/clk-mt6765.c:537:35: warning: unused variable 'ifr0_cg_regs' [-Wunused-const-variable]
static const struct mtk_gate_regs ifr0_cg_regs = {
^
>> drivers/clk/mediatek/clk-mt6765.c:543:35: warning: unused variable 'ifr1_cg_regs' [-Wunused-const-variable]
static const struct mtk_gate_regs ifr1_cg_regs = {
^
2 warnings generated.

vim +/ifr0_cg_regs +537 drivers/clk/mediatek/clk-mt6765.c

   536	
 > 537	static const struct mtk_gate_regs ifr0_cg_regs = {
   538		.set_ofs = 0x200,
   539		.clr_ofs = 0x200,
   540		.sta_ofs = 0x200,
   541	};
   542	
 > 543	static const struct mtk_gate_regs ifr1_cg_regs = {
   544		.set_ofs = 0x74,
   545		.clr_ofs = 0x74,
   546		.sta_ofs = 0x74,
   547	};
   548	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 73488 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 4/4] pinctrl: bcm2835: Add support for wake-up interrupts
From: Florian Fainelli @ 2020-05-30 21:19 UTC (permalink / raw)
  To: Stefan Wahren, Florian Fainelli, linux-kernel
  Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Geert Uytterhoeven, Scott Branden, Ray Jui, Linus Walleij,
	Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM, Rob Herring,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Nicolas Saenz Julienne
In-Reply-To: <2677905e-a9ad-a44e-93bc-ad185aa269de@i2se.com>



On 5/30/2020 12:49 AM, Stefan Wahren wrote:
> Hi Florian,
> 
> Am 29.05.20 um 21:15 schrieb Florian Fainelli:
>> Leverage the IRQCHIP_MASK_ON_SUSPEND flag in order to avoid having to
>> specifically treat the GPIO interrupts during suspend and resume, and
>> simply implement an irq_set_wake() callback that is responsible for
>> enabling the parent wake-up interrupt as a wake-up interrupt.
>>
>> To avoid allocating unnecessary resources for other chips, the wake-up
>> interrupts are only initialized if we have a brcm,bcm7211-gpio
>> compatibility string.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  drivers/pinctrl/bcm/pinctrl-bcm2835.c | 76 ++++++++++++++++++++++++++-
>>  1 file changed, 75 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
>> index 1b00d93aa66e..1fbf067a3eed 100644
>> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
>> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/irq.h>
>>  #include <linux/irqdesc.h>
>>  #include <linux/init.h>
>> +#include <linux/interrupt.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of.h>
>>  #include <linux/of_irq.h>
>> @@ -76,6 +77,7 @@
>>  struct bcm2835_pinctrl {
>>  	struct device *dev;
>>  	void __iomem *base;
>> +	int *wake_irq;
>>  
>>  	/* note: locking assumes each bank will have its own unsigned long */
>>  	unsigned long enabled_irq_map[BCM2835_NUM_BANKS];
>> @@ -435,6 +437,11 @@ static void bcm2835_gpio_irq_handler(struct irq_desc *desc)
>>  	chained_irq_exit(host_chip, desc);
>>  }
>>  
>> +static irqreturn_t bcm2835_gpio_wake_irq_handler(int irq, void *dev_id)
>> +{
>> +	return IRQ_HANDLED;
>> +}
>> +
>>  static inline void __bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc,
>>  	unsigned reg, unsigned offset, bool enable)
>>  {
>> @@ -634,6 +641,34 @@ static void bcm2835_gpio_irq_ack(struct irq_data *data)
>>  	bcm2835_gpio_set_bit(pc, GPEDS0, gpio);
>>  }
>>  
>> +static int bcm2835_gpio_irq_set_wake(struct irq_data *data, unsigned int on)
>> +{
>> +	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
>> +	struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
>> +	unsigned gpio = irqd_to_hwirq(data);
>> +	unsigned int irqgroup;
>> +	int ret = -EINVAL;
>> +
>> +	if (!pc->wake_irq)
>> +		return ret;
>> +
>> +	if (gpio <= 27)
>> +		irqgroup = 0;
>> +	else if (gpio >= 28 && gpio <= 45)
>> +		irqgroup = 1;
>> +	else if (gpio >= 46 && gpio <= 53)
>> +		irqgroup = 2;
> in case the BCM7211 has 58 GPIOs, but the wake up interrupts are only
> available for the first 54 this should deserve a comment.

irqgroup 2 covers GPIOs 46 through 57, thanks for noticing. Do you have
more comments before I spin a v3? Thank you for reviewing.
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox