Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mmc: sdhci-*: Don't emit error msg if sdhci_add_host() fails
From: Adrian Hunter @ 2018-05-25  9:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525151509.0270dbe1@xhacker.debian>

On 25/05/18 10:15, Jisheng Zhang wrote:
> I noticed below error msg with sdhci-pxav3 on some berlin platforms:
> 
> [.....] sdhci-pxav3 f7ab0000.sdhci failed to add host
> 
> It is due to getting related vmmc or vqmmc regulator returns
> -EPROBE_DEFER. It doesn't matter at all but it's confusing.
> 
>>From another side, if driver probing fails and the error number isn't
> -EPROBE_DEFER, the core will tell us something as below:
> 
> [.....] sdhci-pxav3: probe of f7ab0000.sdhci failed with error -EXX
> 
> So it's not necessary to emit error msg if sdhci_add_host() fails. And
> some other sdhci host drivers also have this issue, let's fix them
> together.
> 
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-bcm-kona.c | 4 +---
>  drivers/mmc/host/sdhci-pic32.c    | 4 +---
>  drivers/mmc/host/sdhci-pxav2.c    | 4 +---
>  drivers/mmc/host/sdhci-pxav3.c    | 4 +---
>  drivers/mmc/host/sdhci-s3c.c      | 4 +---
>  drivers/mmc/host/sdhci-spear.c    | 4 +---
>  drivers/mmc/host/sdhci-st.c       | 4 +---
>  7 files changed, 7 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
> index 11ca95c60bcf..bdbd4897c0f7 100644
> --- a/drivers/mmc/host/sdhci-bcm-kona.c
> +++ b/drivers/mmc/host/sdhci-bcm-kona.c
> @@ -284,10 +284,8 @@ static int sdhci_bcm_kona_probe(struct platform_device *pdev)
>  	sdhci_bcm_kona_sd_init(host);
>  
>  	ret = sdhci_add_host(host);
> -	if (ret) {
> -		dev_err(dev, "Failed sdhci_add_host\n");
> +	if (ret)
>  		goto err_reset;
> -	}
>  
>  	/* if device is eMMC, emulate card insert right here */
>  	if (!mmc_card_is_removable(host->mmc)) {
> diff --git a/drivers/mmc/host/sdhci-pic32.c b/drivers/mmc/host/sdhci-pic32.c
> index a6caa49ca25a..a11e6397d4ff 100644
> --- a/drivers/mmc/host/sdhci-pic32.c
> +++ b/drivers/mmc/host/sdhci-pic32.c
> @@ -200,10 +200,8 @@ static int pic32_sdhci_probe(struct platform_device *pdev)
>  	}
>  
>  	ret = sdhci_add_host(host);
> -	if (ret) {
> -		dev_err(&pdev->dev, "error adding host\n");
> +	if (ret)
>  		goto err_base_clk;
> -	}
>  
>  	dev_info(&pdev->dev, "Successfully added sdhci host\n");
>  	return 0;
> diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
> index 8986f9d9cf98..2c3827f54927 100644
> --- a/drivers/mmc/host/sdhci-pxav2.c
> +++ b/drivers/mmc/host/sdhci-pxav2.c
> @@ -221,10 +221,8 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
>  	host->ops = &pxav2_sdhci_ops;
>  
>  	ret = sdhci_add_host(host);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to add host\n");
> +	if (ret)
>  		goto disable_clk;
> -	}
>  
>  	return 0;
>  
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index a34434166ca7..b8e96f392428 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -472,10 +472,8 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>  	pm_suspend_ignore_children(&pdev->dev, 1);
>  
>  	ret = sdhci_add_host(host);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to add host\n");
> +	if (ret)
>  		goto err_add_host;
> -	}
>  
>  	if (host->mmc->pm_caps & MMC_PM_WAKE_SDIO_IRQ)
>  		device_init_wakeup(&pdev->dev, 1);
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index cda83ccb2702..9ef89d00970e 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -655,10 +655,8 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
>  		goto err_req_regs;
>  
>  	ret = sdhci_add_host(host);
> -	if (ret) {
> -		dev_err(dev, "sdhci_add_host() failed\n");
> +	if (ret)
>  		goto err_req_regs;
> -	}
>  
>  #ifdef CONFIG_PM
>  	if (pdata->cd_type != S3C_SDHCI_CD_INTERNAL)
> diff --git a/drivers/mmc/host/sdhci-spear.c b/drivers/mmc/host/sdhci-spear.c
> index 14511526a3a8..9247d51f2eed 100644
> --- a/drivers/mmc/host/sdhci-spear.c
> +++ b/drivers/mmc/host/sdhci-spear.c
> @@ -126,10 +126,8 @@ static int sdhci_probe(struct platform_device *pdev)
>  	}
>  
>  	ret = sdhci_add_host(host);
> -	if (ret) {
> -		dev_dbg(&pdev->dev, "error adding host\n");
> +	if (ret)
>  		goto disable_clk;
> -	}
>  
>  	platform_set_drvdata(pdev, host);
>  
> diff --git a/drivers/mmc/host/sdhci-st.c b/drivers/mmc/host/sdhci-st.c
> index c32daed0d418..8f95647195d9 100644
> --- a/drivers/mmc/host/sdhci-st.c
> +++ b/drivers/mmc/host/sdhci-st.c
> @@ -422,10 +422,8 @@ static int sdhci_st_probe(struct platform_device *pdev)
>  	st_mmcss_cconfig(np, host);
>  
>  	ret = sdhci_add_host(host);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Failed sdhci_add_host\n");
> +	if (ret)
>  		goto err_out;
> -	}
>  
>  	host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION));
>  
> 

^ permalink raw reply

* [PATCH v11 08/19] arm64: fpsimd: Eliminate task->mm checks
From: Christoffer Dall @ 2018-05-25  9:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527181008-13549-9-git-send-email-Dave.Martin@arm.com>

On Thu, May 24, 2018 at 05:56:37PM +0100, Dave Martin wrote:
> Currently the FPSIMD handling code uses the condition task->mm ==
> NULL as a hint that task has no FPSIMD register context.
> 
> The ->mm check is only there to filter out tasks that cannot
> possibly have FPSIMD context loaded, for optimisation purposes.
> Also, TIF_FOREIGN_FPSTATE must always be checked anyway before
> saving FPSIMD context back to memory.  For these reasons, the ->mm
> checks are not useful, providing that TIF_FOREIGN_FPSTATE is
> maintained in a consistent way for all threads.
> 
> The context switch logic is already deliberately optimised to defer
> reloads of the regs until ret_to_user (or sigreturn as a special
> case), and save them only if they have been previously loaded.
> These paths are the only places where the wrong_task and wrong_cpu
> conditions can be made false, by calling fpsimd_bind_task_to_cpu().
> Kernel threads by definition never reach these paths.  As a result,
> the wrong_task and wrong_cpu tests in fpsimd_thread_switch() will
> always yield true for kernel threads.
> 
> This patch removes the redundant checks and special-case code,                  ensuring that TIF_FOREIGN_FPSTATE is set whenever a kernel thread               is scheduled in, and ensures that this flag is set for the init
> task.  The fpsimd_flush_task_state() call already present in
> copy_thread() ensures the same for any new task.

nit: formatting still funny, but you shouldn't respin just for that.

> 
> With TIF_FOREIGN_FPSTATE always set for kernel threads, this patch
> ensures that no extra context save work is added for kernel
> threads, and eliminates the redundant context saving that may
> currently occur for kernel threads that have acquired an mm via
> use_mm().

Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>

> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Reviewed-by: Alex Benn?e <alex.bennee@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> ---
> 
> Changes since v10:
> 
>  * The INIT_THREAD flag change is split out into the prior
>    patch, since it is in principle a fix rather than simply a
>    tidy-up.
> 
> Requested by Christoffer Dall / Catalin Marinas:
> 
>  * Reworded commit message to explain the change more clearly,
>    and remove confusing claims about things being true by
>    construction.
> 
>  * Added a comment to the code explaining that wrong_cpu and
>    wrong_task will always be true for kernel threads.
> 
>  * Ensure .fpsimd_cpu = NR_CPUS for the init task.
> 
>    This does not seem to be a bug, because the wrong_task check in
>    fpsimd_thread_switch() should still always be true for the init
>    task; but it is nonetheless an inconsistency compared with what
>    copy_thread() does.
> 
>    So fix it to avoid future surprises.
> ---
>  arch/arm64/include/asm/processor.h |  4 +++-
>  arch/arm64/kernel/fpsimd.c         | 40 +++++++++++++++-----------------------
>  2 files changed, 19 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index 7675989..36d64f8 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -156,7 +156,9 @@ static inline void arch_thread_struct_whitelist(unsigned long *offset,
>  /* Sync TPIDR_EL0 back to thread_struct for current */
>  void tls_preserve_current_state(void);
>  
> -#define INIT_THREAD  {	}
> +#define INIT_THREAD {				\
> +	.fpsimd_cpu = NR_CPUS,			\
> +}
>  
>  static inline void start_thread_common(struct pt_regs *regs, unsigned long pc)
>  {
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 2d9a9e8..d736b6c 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -892,31 +892,25 @@ asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs)
>  
>  void fpsimd_thread_switch(struct task_struct *next)
>  {
> +	bool wrong_task, wrong_cpu;
> +
>  	if (!system_supports_fpsimd())
>  		return;
> +
> +	/* Save unsaved fpsimd state, if any: */
> +	fpsimd_save();
> +
>  	/*
> -	 * Save the current FPSIMD state to memory, but only if whatever is in
> -	 * the registers is in fact the most recent userland FPSIMD state of
> -	 * 'current'.
> +	 * Fix up TIF_FOREIGN_FPSTATE to correctly describe next's
> +	 * state.  For kernel threads, FPSIMD registers are never loaded
> +	 * and wrong_task and wrong_cpu will always be true.
>  	 */
> -	if (current->mm)
> -		fpsimd_save();
> -
> -	if (next->mm) {
> -		/*
> -		 * If we are switching to a task whose most recent userland
> -		 * FPSIMD state is already in the registers of *this* cpu,
> -		 * we can skip loading the state from memory. Otherwise, set
> -		 * the TIF_FOREIGN_FPSTATE flag so the state will be loaded
> -		 * upon the next return to userland.
> -		 */
> -		bool wrong_task = __this_cpu_read(fpsimd_last_state.st) !=
> +	wrong_task = __this_cpu_read(fpsimd_last_state.st) !=
>  					&next->thread.uw.fpsimd_state;
> -		bool wrong_cpu = next->thread.fpsimd_cpu != smp_processor_id();
> +	wrong_cpu = next->thread.fpsimd_cpu != smp_processor_id();
>  
> -		update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
> -				       wrong_task || wrong_cpu);
> -	}
> +	update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
> +			       wrong_task || wrong_cpu);
>  }
>  
>  void fpsimd_flush_thread(void)
> @@ -1121,9 +1115,8 @@ void kernel_neon_begin(void)
>  
>  	__this_cpu_write(kernel_neon_busy, true);
>  
> -	/* Save unsaved task fpsimd state, if any: */
> -	if (current->mm)
> -		fpsimd_save();
> +	/* Save unsaved fpsimd state, if any: */
> +	fpsimd_save();
>  
>  	/* Invalidate any task state remaining in the fpsimd regs: */
>  	fpsimd_flush_cpu_state();
> @@ -1245,8 +1238,7 @@ static int fpsimd_cpu_pm_notifier(struct notifier_block *self,
>  {
>  	switch (cmd) {
>  	case CPU_PM_ENTER:
> -		if (current->mm)
> -			fpsimd_save();
> +		fpsimd_save();
>  		fpsimd_flush_cpu_state();
>  		break;
>  	case CPU_PM_EXIT:
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v10 07/18] arm64: fpsimd: Eliminate task->mm checks
From: Christoffer Dall @ 2018-05-25  9:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524143714.GV13470@e103592.cambridge.arm.com>

On Thu, May 24, 2018 at 03:37:15PM +0100, Dave Martin wrote:
> On Thu, May 24, 2018 at 12:06:59PM +0200, Christoffer Dall wrote:
> > On Thu, May 24, 2018 at 10:50:56AM +0100, Dave Martin wrote:
> > > On Thu, May 24, 2018 at 10:33:50AM +0200, Christoffer Dall wrote:
> 
> [...]
> 
> > > > ...with a risk of being a bit over-pedantic and annoying, may I suggest
> > > > the following complete commit text:
> > > > 
> > > > ------8<------
> > > > Currently the FPSIMD handling code uses the condition task->mm ==
> > > > NULL as a hint that task has no FPSIMD register context.
> > > > 
> > > > The ->mm check is only there to filter out tasks that cannot
> > > > possibly have FPSIMD context loaded, for optimisation purposes.
> > > > However, TIF_FOREIGN_FPSTATE must always be checked anyway before
> > > > saving FPSIMD context back to memory.  For this reason, the ->mm
> > > > checks are not useful, providing that that TIF_FOREIGN_FPSTATE is
> > > > maintained properly for kernel threads.
> > > > 
> > > > FPSIMD context is never preserved for kernel threads across a context
> > > > switch and therefore TIF_FOREIGN_FPSTATE should always be true for
> > > 
> > > (This refactoring opens up the interesting possibility of making
> > > kernel-mode NEON in task context preemptible for kernel threads so
> > > that we actually do preserve state... but that's a discussion for
> > > another day.  There may be code around that relies on
> > > kernel_neon_begin() disabling preemption for real.)
> > > 
> > > > kernel threads.  This is indeed the case, as the wrong_task and
> > > 
> > > This suggests that TIF_FOREIGN_FPSTATE is always true for kernel
> > > threads today.  This is not quite because use_mm() can make mm non-
> > > NULL.
> > > 
> > 
> > I was suggesting that it's always true after this patch.
> 
> I tend to read the present tense as describing the situation before the
> patch, but this convention isn't followed universally.
> 
> This was part of the problem with my "true by construction" weasel
> words: the described property wasn't true by construction prior to the
> patch, and there wasn't sufficient explanation to convince people it's
> true afterwards.  If people are bring rigorous, it takes a _lot_ of
> explanation...
> 
> > 
> > > > wrong_cpu tests in fpsimd_thread_switch() will always yield false for
> > > > kernel threads.
> > > 
> > > ("false" -> "true".  My bad.)
> > > 
> > > > Further, the context switch logic is already deliberately optimised to
> > > > defer reloads of the FPSIMD context until ret_to_user (or sigreturn as a
> > > > special case), which kernel threads by definition never reach, and
> > > > therefore this change introduces no additional work in the critical
> > > > path.
> > > > 
> > > > This patch removes the redundant checks and special-case code.
> > > > ------8<------
> > > 
> > > Looking at my existing text, I rather reworded it like this.
> > > Does this work any better for you?
> > > 
> > > --8<--
> > > 
> > > Currently the FPSIMD handling code uses the condition task->mm ==
> > > NULL as a hint that task has no FPSIMD register context.
> > > 
> > > The ->mm check is only there to filter out tasks that cannot
> > > possibly have FPSIMD context loaded, for optimisation purposes.
> > > Also, TIF_FOREIGN_FPSTATE must always be checked anyway before
> > > saving FPSIMD context back to memory.  For these reasons, the ->mm
> > > checks are not useful, providing that TIF_FOREIGN_FPSTATE is
> > > maintained in a consistent way for kernel threads.
> > 
> > Consistent with what?  Without more context or explanation,
> 
> Consistent with the handling of user threads (though I admit it's not
> explicit in the text.)
> 
> > I'm not sure what the reader is to make of that.  Do you not mean the
> > TIF_FOREIGN_FPSTATE is always true for kernel threads?
> 
> Again, this is probably a red herring.  TIF_FOREIGN_FPSTATE is always
> true for kernel threads prior to the patch, except (randomly) for the
> init task.

That was really what my initial question was about, and what I thought
the commit message should make abundantly clear, because that ties the
message together with the code.

> 
> This change is not really about TIF_FOREIGN_FPSTATE at all, rather
> that there is nothing to justify handling kernel threads differently,
> or even distinguishing kernel threads from user threads at all in this
> code.

Understood.

> 
> Part of the confusion (and I had confused myself) comes from the fact
> that TIF_FOREIGN_FPSTATE is really a per-cpu property and doesn't make
> sense as a per-task property -- i.e., the flag is meaningless for
> scheduled-out tasks and we must explicitly "repair" it when scheduling
> a task in anyway.  I think it's a thread flag primarily so that it's
> convenient to check alongside other thread flags in the ret_to_user
> work loop.  This is somewhat less of a justification now that loop was
> ported to C.
> 
> > > 
> > > The context switch logic is already deliberately optimised to defer
> > > reloads of the regs until ret_to_user (or sigreturn as a special
> > > case), and save them only if they have been previously loaded.
> 
> Does it help to insert the following here?
> 
> "These paths are the only places where the wrong_task and wrong_cpu
> conditions can be made false, by calling fpsimd_bind_task_to_cpu()."
> 

yes it does.

> > > Kernel threads by definition never reach these paths.  As a result,
> > 
> > I'm struggling with the "As a result," here.  Is this because reloads of
> > regs in ret_to_user (or sigreturn) are the only places that can make
> > wrong_cpu or wrong_task be false?
> 
> See the proposed clarification above.  Is that sufficient?
> 

yes.

> > (I'm actually wanting to understand this, not just bikeshedding the
> > commit message, as new corner cases keep coming up on this logic.)
> 
> That's a good thing, and I would really like to explain it in a
> concise manner.  See [*] below for the "concise" explanation -- it may
> demonstrate why I've been evasive...
> 

I don't think you've been evasive at all, I just think we reason about
this in slightly different ways, and I was trying to convince myself why
this change is safe and summarize that concisely.  I think we've
accomplished both :)

> > > the wrong_task and wrong_cpu tests in fpsimd_thread_switch() will
> > > always yield true for kernel threads.
> > > 
> > > This patch removes the redundant checks and special-case code,                  ensuring that TIF_FOREIGN_FPSTATE is set whenever a kernel thread               is scheduled in, and ensures that this flag is set for the init
> > > task.  The fpsimd_flush_task_state() call already present in                    copy_thread() ensures the same for any new task.
> > 
> > nit: funny formatting
> 
> Dang, I was repeatedly pasing between Mutt and git commit terminals,
> which doesn't always work as I'd like...
> 
> > nit: ensuring that TIF_FOREIGN_FPSTATE *remains* set whenever a kernel
> > thread is scheduled in?
> 
> Er, yes.
> 
> > > With TIF_FOREIGN_FPSTATE always set for kernel threads, this patch
> > > ensures that no extra context save work is added for kernel
> > > threads, and eliminates the redundant context saving that may
> > > currently occur for kernel threads that have acquired an mm via
> > > use_mm().
> > > 
> > > -->8--
> > 
> > If you can slightly connect the dots with the "As a result" above, I'm
> > fine with your version of the text.
> 
> 
> As an aside, the big wall of text before the definition of struct
> fpsimd_last_state_struct is looking out of date and could use an
> update to cover at least some of what is explained in [*] better.
> 
> I'm currently considering that out of scope for this series, but I will
> keep it in mind to refresh it in the not too distant future.
> 

Fine with me.

> 
> Cheers
> ---Dave
> 
> --8<--
> 
> [*] The bigger picture:
> 
> * Consider a relation (C,T) between cpus C and tasks T, such that
>   (C,T) means "T's FPSIMD regs are loaded on cpu C".
> 
>   At a given point of execution of some cpu C, there is at most one task
>   T for which (C,T) holds.
>  
>   At a given point of execution of some task T, there is at most one
>   cpu C for which (C,T) holds.
> 
> * (C,T) becomes true whenever T's registers are loaded into cpu C.
> 
> * At sched-out, we must ensure that the registers of current are
>   loaded before writing them to current's thread_struct.  Thus, we
>   must save the registers if and only if (smp_processor_id(), current)
>   holds at this time.
> 
> * Before entering userspace, we must ensure that current's regs
>   are loaded, and we must only load the regs if they are not loaded
>   already (since if so, they might have been dirtied by current in
>   userspace since last loaded).
> 
>   Thus, when entering userspace, we must load the regs from memory
>   if and only if (smp_processor_id(), current) does not hold.
> 
> * Checking this relation involves per-CPU access and inspection of
>   current->thread, and was presumably considered too cumbersome for
>   implemenation an entry.S, particluarly in the ret_to_user work
>   pending loop (which is where the FPSIMD regs are finally loaded
>   before entering userspace, if they weren't loaded already).
> 
>   To mitigate this, the status of the check is cached in a thread flag
>   TIF_FOREIGN_FPSTATE: with softirqs disabled, (smp_processor_id(),
>   current) holds if and only if TIF_FOREIGN_FPSTATE is false.
>   TIF_FOREIGN_FPSTATE is corrected on sched-in by the code in
>   fpsimd_thread_switch().
> 
> [2] Anything that changes the state of the relation for current
>   requires its TIF_FOREIGN_FPSTATE to be changed to match.
> 
> * (smp_processor_id(), current) is established in
>   fpsimd_bind_task_to_cpu().  This is the only way the relation can be
>   made to hold between a task and a CPU.
> 
> * (C,T) is broken whenever
> 
> [1] T is created;
> 
>   * T's regs are loaded onto a different cpu C2, so (C2,T) becomes
>     true and (C,T) necessarily becomes false;
> 
>   * another task's regs are loaded into C, so (C,T2) becomes true
>     and (C,T) necessarily becomes false;
> 
>   * the kernel clobbers the regs on C for its own purposes, so
>     (C,T) becomes false but there is no T2 for which (C,T2) becomes
>     true as a result.  Examples are kernel-mode NEON and loading
>     the regs for a KVM vcpu;
> 
>   * T's register context changes via a thread_struct update instead
>     of running instructions in userspace, requiring the contents of
>     the hardware regs to be thrown away.  Examples are exec() (which
>     requires the registers to be zeroed), sigreturn (which populates the
>     regs from the user signal frame) and modification of the registers
>     via PTRACE_SETREGSET;
> 
>     As a (probably unnecesary) optimisation, sigreturn immediately
>     loads the registers and reestablishes (smp_processor_id(), current)
>     in anticipation of the return to userspace which is likely to
>     occur soon.  This allows the relation breaking logic to be omitted
>     in fpsimd_update_current_state() which does the work.
> 
> * In general, these relation breakings involve an unknown: knowing
>   either C or T but *not* both, we want to break (C,T).  If the
>   relation were recorded in task_struct only, we would need to scan all
>   tasks in the "T unknown" case.  If the relation were recorded in a
>   percpu variable only, we would need to scan all CPUs in the "C
>   unknown" case.  As well as having gnarly synchronisation
>   requirements, these would get expensive in many-tasks or many-cpus
>   situations.
> 
>   This is why the relation is recorded in both places, and is only
>   deemed to hold if the two records match up.  This is what
>   fpsimd_thread_switch() is checking for the task being scheduled in.
> 
>   The invalidation (breaking) operations are now factored as
> 
>   fpsimd_flush_task_state(): falsify (C,current) for every cpu C.
>   This is done by zapping current->thread.fpsimd_cpu with NR_CPUS
>   (chosen because it cannot match smp_processor_id()).
> 
>   fpsumd_flush_cpu_state(): falsify (smp_processor_id(),T) for every
>   task T.  This is done by zapping this_cpu(fpsimd_last_state.st)
>   with NULL (chosen because it cannot match &T->thread.uw.fpsimd_state
>   for any task).
> 
>   By [2] above, it is necessary to ensure that TIF_FOREIGN_FPSTATE is
>   set after calling either of the above functions.  Of the two,
>   fpsimd_flush_cpu_state() now does this implicitly but
>   fpsimd_flush_task_state() does not: but the caller must do it
>   instead.  I have a vague memory of some refactoring obstacle that
>   dissuaded me from pulling the set_thread_flag in, but I can't
>   remember it now.  I may review this later.
> 
> * Because the (C,T) relation may need to be manipulated by
>   kernel_neon_{begin,end}() in softirq context, examining or
>   manipulating for current or the running CPU must be done under
>   local_bh_disable().  The same goes for TIF_FOREIGN_FPSTATE which is
>   supposed to represent the same condition but may spontaneously become
>   stale if softirqs are not masked.  (The rule is not quite as strict
>   as this, but in order to make the code easier to reason about, I skip
>   the local_bh_disable() only where absolutely necessary --
>   restore_sve_fpsimd_context() is the only example today.)
> 
> Now, imagine that T is a kernel thread, and consider what needs to
> be done differently.  The observation of this patch is that nothing
> needs to be done differently at all.
> 
> There is a single anomaly relating to [1] above, in the form of a task
> that can run without ever being scheduled in: the init task.  Beyond
> that, kernel_neon_begin() before the first reschedule would spuriously
> save the FPSIMD regs into the init_task's thread struct, even though it
> is pointless to do so.  This patch fixes those anomalies by updating
> INIT_THREAD and INIT_THREAD_INFO to set up the init task so that it
> looks the same as some other kernel thread that has been scheduled in.
> 
> There is a strong design motivation to avoid unnecessary loads and
> saves of the state, so if removing the special-casing of kernel threads
> were to add cost it would imply that the code were _already_ suboptimal
> for user tasks.  This patch does not attempt to address that at all,
> but by assuming that the code is already well-optimised, "unnecessary"
> save/restore work will not be added.  If this were not the case, it
> could in any case be fixed independently.
> 
> The observation of this _series_ is that we don't need to do very
> much in order to be able to generalise the logic to accept KVM vcpus
> in place of T.
> 

Thanks for the explanation.
-Christoffer

^ permalink raw reply

* [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Jonathan Cameron @ 2018-05-25  8:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525063311.GA11605@apalos>

+CC Kenneth Lee

On Fri, 25 May 2018 09:33:11 +0300
Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:

> On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > On 24/05/18 12:50, Ilias Apalodimas wrote:  
> > >> Interesting, I hadn't thought about this use-case before. At first I
> > >> thought you were talking about mdev devices assigned to VMs, but I think
> > >> you're referring to mdevs assigned to userspace drivers instead? Out of
> > >> curiosity, is it only theoretical or does someone actually need this?  
> > > 
> > > There has been some non upstreamed efforts to have mdev and produce userspace
> > > drivers. Huawei is using it on what they call "wrapdrive" for crypto devices and
> > > we did a proof of concept for ethernet interfaces. At the time we choose not to
> > > involve the IOMMU for the reason you mentioned, but having it there would be
> > > good.  
> > 
> > I'm guessing there were good reasons to do it that way but I wonder, is
> > it not simpler to just have the kernel driver create a /dev/foo, with a
> > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > indirection, and since the mediating driver has to implement these
> > operations already, what is gained?  
> The best reason i can come up with is "common code". You already have one API
> doing that for you so we replicate it in a /dev file?
> The mdev approach still needs extentions to support what we tried to do (i.e
> mdev bus might need yo have access on iommu_ops), but as far as i undestand it's
> a possible case.
> > 
> > Thanks,
> > Jean  

^ permalink raw reply

* [PATCH v2 08/13] ASoC: pxa: remove the dmaengine compat need
From: Daniel Mack @ 2018-05-25  8:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524070703.11901-9-robert.jarzmik@free.fr>

On Thursday, May 24, 2018 09:06 AM, Robert Jarzmik wrote:
> As the pxa architecture switched towards the dmaengine slave map, the
> old compatibility mechanism to acquire the dma requestor line number and
> priority are not needed anymore.
> 
> This patch simplifies the dma resource acquisition, using the more
> generic function dma_request_slave_channel().
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

Reviewed-by: Daniel Mack <daniel@zonque.org>

> ---
>   sound/arm/pxa2xx-ac97.c     | 14 ++------------
>   sound/arm/pxa2xx-pcm-lib.c  |  6 +++---
>   sound/soc/pxa/pxa2xx-ac97.c | 32 +++++---------------------------
>   sound/soc/pxa/pxa2xx-i2s.c  |  6 ++----
>   4 files changed, 12 insertions(+), 46 deletions(-)
> 
> diff --git a/sound/arm/pxa2xx-ac97.c b/sound/arm/pxa2xx-ac97.c
> index 4bc244c40f80..236a63cdaf9f 100644
> --- a/sound/arm/pxa2xx-ac97.c
> +++ b/sound/arm/pxa2xx-ac97.c
> @@ -63,28 +63,18 @@ static struct snd_ac97_bus_ops pxa2xx_ac97_ops = {
>   	.reset	= pxa2xx_ac97_legacy_reset,
>   };
>   
> -static struct pxad_param pxa2xx_ac97_pcm_out_req = {
> -	.prio = PXAD_PRIO_LOWEST,
> -	.drcmr = 12,
> -};
> -
>   static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_out = {
>   	.addr		= __PREG(PCDR),
>   	.addr_width	= DMA_SLAVE_BUSWIDTH_4_BYTES,
> +	.chan_name	= "pcm_pcm_stereo_out",
>   	.maxburst	= 32,
> -	.filter_data	= &pxa2xx_ac97_pcm_out_req,
> -};
> -
> -static struct pxad_param pxa2xx_ac97_pcm_in_req = {
> -	.prio = PXAD_PRIO_LOWEST,
> -	.drcmr = 11,
>   };
>   
>   static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_in = {
>   	.addr		= __PREG(PCDR),
>   	.addr_width	= DMA_SLAVE_BUSWIDTH_4_BYTES,
> +	.chan_name	= "pcm_pcm_stereo_in",
>   	.maxburst	= 32,
> -	.filter_data	= &pxa2xx_ac97_pcm_in_req,
>   };
>   
>   static struct snd_pcm *pxa2xx_ac97_pcm;
> diff --git a/sound/arm/pxa2xx-pcm-lib.c b/sound/arm/pxa2xx-pcm-lib.c
> index e8da3b8ee721..dcbe7ecc1835 100644
> --- a/sound/arm/pxa2xx-pcm-lib.c
> +++ b/sound/arm/pxa2xx-pcm-lib.c
> @@ -125,9 +125,9 @@ int __pxa2xx_pcm_open(struct snd_pcm_substream *substream)
>   	if (ret < 0)
>   		return ret;
>   
> -	return snd_dmaengine_pcm_open_request_chan(substream,
> -					pxad_filter_fn,
> -					dma_params->filter_data);
> +	return snd_dmaengine_pcm_open(
> +		substream, dma_request_slave_channel(rtd->cpu_dai->dev,
> +						     dma_params->chan_name));
>   }
>   EXPORT_SYMBOL(__pxa2xx_pcm_open);
>   
> diff --git a/sound/soc/pxa/pxa2xx-ac97.c b/sound/soc/pxa/pxa2xx-ac97.c
> index 803818aabee9..1b41c0f2a8fb 100644
> --- a/sound/soc/pxa/pxa2xx-ac97.c
> +++ b/sound/soc/pxa/pxa2xx-ac97.c
> @@ -68,61 +68,39 @@ static struct snd_ac97_bus_ops pxa2xx_ac97_ops = {
>   	.reset	= pxa2xx_ac97_cold_reset,
>   };
>   
> -static struct pxad_param pxa2xx_ac97_pcm_stereo_in_req = {
> -	.prio = PXAD_PRIO_LOWEST,
> -	.drcmr = 11,
> -};
> -
>   static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_stereo_in = {
>   	.addr		= __PREG(PCDR),
>   	.addr_width	= DMA_SLAVE_BUSWIDTH_4_BYTES,
> +	.chan_name	= "pcm_pcm_stereo_in",
>   	.maxburst	= 32,
> -	.filter_data	= &pxa2xx_ac97_pcm_stereo_in_req,
> -};
> -
> -static struct pxad_param pxa2xx_ac97_pcm_stereo_out_req = {
> -	.prio = PXAD_PRIO_LOWEST,
> -	.drcmr = 12,
>   };
>   
>   static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_stereo_out = {
>   	.addr		= __PREG(PCDR),
>   	.addr_width	= DMA_SLAVE_BUSWIDTH_4_BYTES,
> +	.chan_name	= "pcm_pcm_stereo_out",
>   	.maxburst	= 32,
> -	.filter_data	= &pxa2xx_ac97_pcm_stereo_out_req,
>   };
>   
> -static struct pxad_param pxa2xx_ac97_pcm_aux_mono_out_req = {
> -	.prio = PXAD_PRIO_LOWEST,
> -	.drcmr = 10,
> -};
>   static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_aux_mono_out = {
>   	.addr		= __PREG(MODR),
>   	.addr_width	= DMA_SLAVE_BUSWIDTH_2_BYTES,
> +	.chan_name	= "pcm_aux_mono_out",
>   	.maxburst	= 16,
> -	.filter_data	= &pxa2xx_ac97_pcm_aux_mono_out_req,
>   };
>   
> -static struct pxad_param pxa2xx_ac97_pcm_aux_mono_in_req = {
> -	.prio = PXAD_PRIO_LOWEST,
> -	.drcmr = 9,
> -};
>   static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_aux_mono_in = {
>   	.addr		= __PREG(MODR),
>   	.addr_width	= DMA_SLAVE_BUSWIDTH_2_BYTES,
> +	.chan_name	= "pcm_aux_mono_in",
>   	.maxburst	= 16,
> -	.filter_data	= &pxa2xx_ac97_pcm_aux_mono_in_req,
>   };
>   
> -static struct pxad_param pxa2xx_ac97_pcm_aux_mic_mono_req = {
> -	.prio = PXAD_PRIO_LOWEST,
> -	.drcmr = 8,
> -};
>   static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_mic_mono_in = {
>   	.addr		= __PREG(MCDR),
>   	.addr_width	= DMA_SLAVE_BUSWIDTH_2_BYTES,
> +	.chan_name	= "pcm_aux_mic_mono",
>   	.maxburst	= 16,
> -	.filter_data	= &pxa2xx_ac97_pcm_aux_mic_mono_req,
>   };
>   
>   static int pxa2xx_ac97_hifi_startup(struct snd_pcm_substream *substream,
> diff --git a/sound/soc/pxa/pxa2xx-i2s.c b/sound/soc/pxa/pxa2xx-i2s.c
> index 3fb60baf6eab..e7184de0de04 100644
> --- a/sound/soc/pxa/pxa2xx-i2s.c
> +++ b/sound/soc/pxa/pxa2xx-i2s.c
> @@ -82,20 +82,18 @@ static struct pxa_i2s_port pxa_i2s;
>   static struct clk *clk_i2s;
>   static int clk_ena = 0;
>   
> -static unsigned long pxa2xx_i2s_pcm_stereo_out_req = 3;
>   static struct snd_dmaengine_dai_dma_data pxa2xx_i2s_pcm_stereo_out = {
>   	.addr		= __PREG(SADR),
>   	.addr_width	= DMA_SLAVE_BUSWIDTH_4_BYTES,
> +	.chan_name	= "tx",
>   	.maxburst	= 32,
> -	.filter_data	= &pxa2xx_i2s_pcm_stereo_out_req,
>   };
>   
> -static unsigned long pxa2xx_i2s_pcm_stereo_in_req = 2;
>   static struct snd_dmaengine_dai_dma_data pxa2xx_i2s_pcm_stereo_in = {
>   	.addr		= __PREG(SADR),
>   	.addr_width	= DMA_SLAVE_BUSWIDTH_4_BYTES,
> +	.chan_name	= "rx",
>   	.maxburst	= 32,
> -	.filter_data	= &pxa2xx_i2s_pcm_stereo_in_req,
>   };
>   
>   static int pxa2xx_i2s_startup(struct snd_pcm_substream *substream,
> 

^ permalink raw reply

* [PATCH] perf: hisi: fix uncore PMU index ID
From: Zhangshaokun @ 2018-05-25  8:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180523164219.GG2983@arm.com>

Hi Will,

On 2018/5/24 0:42, Will Deacon wrote:
> On Tue, May 22, 2018 at 05:18:51PM +0800, Zhangshaokun wrote:
>> On 2018/5/22 1:05, Will Deacon wrote:
>>> Whilst I'd normally just accept PMU driver submissions for vendor PMUs,
>>> this part rang my alarm bells:
>>>
>>>> diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
>>>> index 443906e..dcd8e77 100644
>>>> --- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
>>>> +++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
>>>> @@ -238,19 +238,10 @@ MODULE_DEVICE_TABLE(acpi, hisi_hha_pmu_acpi_match);
>>>>  static int hisi_hha_pmu_init_data(struct platform_device *pdev,
>>>>  				  struct hisi_pmu *hha_pmu)
>>>>  {
>>>> -	unsigned long long id;
>>>>  	struct resource *res;
>>>> -	acpi_status status;
>>>> -
>>>> -	status = acpi_evaluate_integer(ACPI_HANDLE(&pdev->dev),
>>>> -				       "_UID", NULL, &id);
>>>> -	if (ACPI_FAILURE(status))
>>>> -		return -EINVAL;
>>>> -
>>>> -	hha_pmu->index_id = id;
>>>>  
>>>>  	/*
>>>> -	 * Use SCCL_ID and UID to identify the HHA PMU, while
>>>> +	 * Use SCCL_ID and HHA index ID to identify the HHA PMU, while
>>>>  	 * SCCL_ID is in MPIDR[aff2].
>>>>  	 */
>>>>  	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
>>>> @@ -258,6 +249,13 @@ static int hisi_hha_pmu_init_data(struct platform_device *pdev,
>>>>  		dev_err(&pdev->dev, "Can not read hha sccl-id!\n");
>>>>  		return -EINVAL;
>>>>  	}
>>>> +
>>>> +	if (device_property_read_u32(&pdev->dev, "hisilicon,idx-id",
>>>> +				     &hha_pmu->index_id)) {
>>>> +		dev_err(&pdev->dev, "Can not read hha index-id!\n");
>>>> +		return -EINVAL;
>>>> +	}
>>>
>>> Is this a new DT property? If so, please can you update the binding
>>> documentation and get an Ack from a DT maintainer? It's not clear to me
>>
>> No, it is not a DT property. We don't support DT mode for this platform and
>> only support ACPI mode.
> 
> Hmm, but by using the firmware-agnostic "device_property_read_u32"
> interface, aren't you implicitly supporting it via DT as well? In fact,
> don't you now fail the probe if this new property isn't present? Isn't
> that a regression?
> 

Even though we don't support DT, using unified device interface seems a better practice
in general. We could use acpi_dev_get_property(), but not much gain.
As for regression, this never worked anyway, right? And this is for a new platform with
little distribution, so not much pain to upgrade the FW.

>>> what a "hisilicon,idx-id" is, nor how I would generate on from firmware.
>>>
>>
>> For HiSilicon this platform, it supports multi-sccl. each sccl has more than one uncore
>> PMUs. Like HHA uncore PMUs, each sccl has 2-HHA PMUs and idx-id is in _DSD package and
>> used to distinguish different HHA PMUs with the same sccl, as follow:
>>     Name (_DSD, Package () {
>>       ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>       Package () {
>>         Package () {"hisilicon,scl-id", 0x03},
>>         Package () {"hisilicon,idx-id", 0x00},
>>       }
>>     })
> 
> I'm still none the wiser about what this actually is. How is new _DSD crud
> supposed to be documented?
> 

For instance:
when run perf list | grep hisi_sccl3_hha
hisi_sccl3_hha0/rx_outer/ [kernel PMU event]
------------------------------------------
hisi_sccl3_hha1/rx_outer/ [kernel PMU event]
0 and 1 are the hha index in the same sccl that sccl-id is 3.
We document as hisi_sccl{X}_<l3c{Y}/hha{Y}/ddrc{Y}> in
Documentation/perf/hisi-pmu.txt

Thanks,
Shaokun

> Will
> 
> .
> 

^ permalink raw reply

* [PATCH 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains
From: Jon Hunter @ 2018-05-25  8:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAPDyKFozm88OM8cDFXXwvgdMirrt2=HhZtBdyhNjo2bgjHV2Mg@mail.gmail.com>


On 24/05/18 22:11, Ulf Hansson wrote:
> On 24 May 2018 at 17:48, Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>> On 18/05/18 11:31, Ulf Hansson wrote:
>>>
>>> The existing dev_pm_domain_attach() function, allows a single PM domain to
>>> be attached per device. To be able to support devices that are partitioned
>>> across multiple PM domains, let's introduce a new interface,
>>> dev_pm_domain_attach_by_id().
>>>
>>> The dev_pm_domain_attach_by_id() returns a new allocated struct device
>>> with
>>> the corresponding attached PM domain. This enables for example a driver to
>>> operate on the new device from a power management point of view. The
>>> driver
>>> may then also benefit from using the received device, to set up so called
>>> device-links towards its original device. Depending on the situation,
>>> these
>>> links may then be dynamically changed.
>>>
>>> The new interface is typically called by drivers during their probe phase,
>>> in case they manages devices which uses multiple PM domains. If that is
>>> the
>>> case, the driver also becomes responsible of managing the detaching of the
>>> PM domains, which typically should be done at the remove phase. Detaching
>>> is done by calling the existing dev_pm_domain_detach() function and for
>>> each of the received devices from dev_pm_domain_attach_by_id().
>>>
>>> Note, currently its only genpd that supports multiple PM domains per
>>> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
>>> other PM domain types, if/when needed.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>    drivers/base/power/common.c | 33 ++++++++++++++++++++++++++++++++-
>>>    include/linux/pm_domain.h   |  7 +++++++
>>>    2 files changed, 39 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
>>> index 7ae62b6..d3db974 100644
>>> --- a/drivers/base/power/common.c
>>> +++ b/drivers/base/power/common.c
>>> @@ -117,13 +117,44 @@ int dev_pm_domain_attach(struct device *dev, bool
>>> power_on)
>>>    EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>>>      /**
>>> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
>>
>>
>> Isn't this more of a 'get'?
> 
> I don't think so, at least according to the common understanding of
> how we use get and put APIs.
> 
> For example, clk_get() returns a cookie to a clk, which you then can
> do a hole bunch of different clk specific operations on.
> 
> This is different, there are no specific PM domain operations the
> caller can or should do. Instead the idea is to keep drivers more or
> less transparent, still using runtime PM as before. The only care the
> caller need to take is to use device links, which BTW isn't a PM
> domain specific thing.

Yes, but a user can still call pm_runtime_get/put with the device 
returned if they wish to, right?

>>
>>> + * @index: The index of the PM domain.
>>> + * @dev: Device to attach.
>>
>>
>> Isn't this just the device associated with the PM domain we are getting?
> 
> Correct, but please note, as stated above, I don't think it's a good
> idea to return a special PM domain cookie, because we don't want the
> caller to run special PM domain operations.
> 
>>
>>> + *
>>> + * As @dev may only be attached to a single PM domain, the backend PM
>>> domain
>>> + * provider should create a virtual device to attach instead. As
>>> attachment
>>> + * succeeds, the ->detach() callback in the struct dev_pm_domain should
>>> be
>>> + * assigned by the corresponding backend attach function.
>>> + *
>>> + * This function should typically be invoked from drivers during probe
>>> phase.
>>> + * Especially for those that manages devices which requires power
>>> management
>>> + * through more than one PM domain.
>>> + *
>>> + * Callers must ensure proper synchronization of this function with power
>>> + * management callbacks.
>>> + *
>>> + * Returns the virtual attached device in case successfully attached PM
>>> domain,
>>> + * NULL in case @dev don't need a PM domain, else a PTR_ERR().
>>
>>
>> Should this be 'NULL in the case where the @dev already has a power-domain'?
> 
> Yes, I think so. The intent is to be consistent with how
> dev_pm_domain_attach() works and according to the latest changes I did
> for it. It's queued for 4.18, please have a look in Rafael's tree and
> you will see.

Ah, I see your change now.

>>
>>> + */
>>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>>> +                                         unsigned int index)
>>> +{
>>> +       if (dev->pm_domain)
>>
>>
>> I wonder if this is worthy of a ...
>>
>>          if (WARN_ON(dev->pm_domain))
>>
>>> +               return NULL;
>>
>>
>> Don't we consider this an error case? I wonder why not return PTR_ERR here
>> as well? This would be consistent with dev_pm_domain_attach().
> 
> Please see above comment.

Right, but this case still seems like an error. My understanding is that 
only drivers will use this API directly and it will not be used by the 
device driver core (unlike dev_pm_domain_attach), so if anyone calls 
this attempting to attach another PM domain when one is already 
attached, they are doing something wrong.

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH v4 5/7] interconnect: qcom: Add msm8916 interconnect provider driver
From: Amit Kucheria @ 2018-05-25  8:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180309210958.16672-6-georgi.djakov@linaro.org>

On Fri, Mar 9, 2018 at 11:09 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
> Add driver for the Qualcomm interconnect buses found in msm8916 based
> platforms.
>
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
>  drivers/interconnect/Kconfig        |   5 +
>  drivers/interconnect/Makefile       |   1 +
>  drivers/interconnect/qcom/Kconfig   |  11 +
>  drivers/interconnect/qcom/Makefile  |   2 +
>  drivers/interconnect/qcom/msm8916.c | 482 ++++++++++++++++++++++++++++++++++++
>  include/linux/interconnect/qcom.h   | 350 ++++++++++++++++++++++++++
>  6 files changed, 851 insertions(+)
>  create mode 100644 drivers/interconnect/qcom/Kconfig
>  create mode 100644 drivers/interconnect/qcom/msm8916.c
>  create mode 100644 include/linux/interconnect/qcom.h
>
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> index a261c7d41deb..07a8276fa35a 100644
> --- a/drivers/interconnect/Kconfig
> +++ b/drivers/interconnect/Kconfig
> @@ -8,3 +8,8 @@ menuconfig INTERCONNECT
>
>           If unsure, say no.
>
> +if INTERCONNECT
> +
> +source "drivers/interconnect/qcom/Kconfig"
> +
> +endif
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> index 5edf0ae80818..5971b811c2d7 100644
> --- a/drivers/interconnect/Makefile
> +++ b/drivers/interconnect/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_INTERCONNECT)             += core.o
> +obj-$(CONFIG_INTERCONNECT_QCOM)                += qcom/
> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
> new file mode 100644
> index 000000000000..86465dc37bd4
> --- /dev/null
> +++ b/drivers/interconnect/qcom/Kconfig
> @@ -0,0 +1,11 @@
> +config INTERCONNECT_QCOM
> +       bool "Qualcomm Network-on-Chip interconnect drivers"
> +       depends on INTERCONNECT
> +       depends on ARCH_QCOM || COMPILE_TEST
> +       default y
> +
> +config INTERCONNECT_QCOM_MSM8916
> +       tristate "Qualcomm MSM8916 interconnect driver"
> +       depends on INTERCONNECT_QCOM
> +       help
> +         This is a driver for the Qualcomm Network-on-Chip on msm8916-based platforms.
> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
> index 095bdef1ee6e..a0c13a25e8db 100644
> --- a/drivers/interconnect/qcom/Makefile
> +++ b/drivers/interconnect/qcom/Makefile
> @@ -1 +1,3 @@
>  obj-y += smd-rpm.o
> +
> +obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += msm8916.o
> diff --git a/drivers/interconnect/qcom/msm8916.c b/drivers/interconnect/qcom/msm8916.c
> new file mode 100644
> index 000000000000..d5b54f8261c8
> --- /dev/null
> +++ b/drivers/interconnect/qcom/msm8916.c
> @@ -0,0 +1,482 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Linaro Ltd
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/interconnect/qcom.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "smd-rpm.h"
> +
> +#define RPM_MASTER_FIELD_BW    0x00007762
> +#define RPM_BUS_MASTER_REQ      0x73616d62
> +#define RPM_BUS_SLAVE_REQ       0x766c7362
> +
> +#define to_qcom_provider(_provider) \
> +       container_of(_provider, struct qcom_icc_provider, provider)
> +
> +#define DEFINE_QNODE(_name, _id, _port, _buswidth, _ap_owned,          \
> +                       _mas_rpm_id, _slv_rpm_id, _qos_mode,            \
> +                       _numlinks, ...)                                 \
> +               static struct qcom_icc_node _name = {                   \
> +               .id = _id,                                              \
> +               .name = #_name,                                         \
> +               .port = _port,                                          \
> +               .buswidth = _buswidth,                                  \
> +               .qos_mode = _qos_mode,                                  \
> +               .ap_owned = _ap_owned,                                  \
> +               .mas_rpm_id = _mas_rpm_id,                              \
> +               .slv_rpm_id = _slv_rpm_id,                              \
> +               .num_links = _numlinks,                                 \
> +               .links = { __VA_ARGS__ },                               \
> +       }

Move this macro definition just above its use below.

> +enum qcom_qos_mode {
> +       QCOM_QOS_MODE_BYPASS = 0,
> +       QCOM_QOS_MODE_FIXED,
> +       QCOM_QOS_MODE_MAX,
> +};
> +
> +struct qcom_icc_provider {
> +       struct icc_provider     provider;
> +       void __iomem            *base;
> +       struct clk              *bus_clk;
> +       struct clk              *bus_a_clk;
> +};
> +
> +#define MSM8916_MAX_LINKS      8
> +
> +/**
> + * struct qcom_icc_node - Qualcomm specific interconnect nodes
> + * @name: the node name used in debugfs
> + * @links: an array of nodes where we can go next while traversing
> + * @id: a unique node identifier
> + * @num_links: the total number of @links
> + * @port: the offset index into the masters QoS register space
> + * @buswidth: width of the interconnect between a node and the bus

units?

> + * @ap_owned: the AP CPU does the writing to QoS registers
> + * @rpm: reference to the RPM SMD driver
> + * @qos_mode: QoS mode for ap_owned resources
> + * @mas_rpm_id:        RPM id for devices that are bus masters
> + * @slv_rpm_id:        RPM id for devices that are bus slaves
> + * @rate: current bus clock rate in Hz
> + */
> +struct qcom_icc_node {
> +       unsigned char *name;
> +       u16 links[MSM8916_MAX_LINKS];
> +       u16 id;
> +       u16 num_links;
> +       u16 port;
> +       u16 buswidth;
> +       bool ap_owned;
> +       struct qcom_smd_rpm *rpm;
> +       enum qcom_qos_mode qos_mode;
> +       int mas_rpm_id;
> +       int slv_rpm_id;
> +       u64 rate;
> +};
> +
> +struct qcom_icc_desc {
> +       struct qcom_icc_node **nodes;
> +       size_t num_nodes;
> +};
> +
> +DEFINE_QNODE(mas_video, 63, 8, 16, 1, -1, -1, QCOM_QOS_MODE_BYPASS, 2, 10000, 10002);
> +DEFINE_QNODE(mas_jpeg, 62, 6, 16, 1, -1, -1, QCOM_QOS_MODE_BYPASS, 2, 10000, 10002);
> +DEFINE_QNODE(mas_vfe, 29, 9, 16, 1, -1, -1, QCOM_QOS_MODE_BYPASS, 2, 10001, 10002);
> +DEFINE_QNODE(mas_mdp, 22, 7, 16, 1, -1, -1, QCOM_QOS_MODE_BYPASS, 2, 10000, 10002);
> +DEFINE_QNODE(mas_qdss_bam, 53, 11, 16, 1, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10009);
> +DEFINE_QNODE(mas_snoc_cfg, 54, 0, 16, 0, 20, -1, QCOM_QOS_MODE_BYPASS, 1, 10009);
> +DEFINE_QNODE(mas_qdss_etr, 60, 10, 16, 1, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10009);
> +DEFINE_QNODE(mm_int_0, 10000, 0, 16, 1, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10003);
> +DEFINE_QNODE(mm_int_1, 10001, 0, 16, 1, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10003);
> +DEFINE_QNODE(mm_int_2, 10002, 0, 16, 1, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10004);
> +DEFINE_QNODE(mm_int_bimc, 10003, 0, 16, 1, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10008);
> +DEFINE_QNODE(snoc_int_0, 10004, 0, 8, 0, 99, 130, QCOM_QOS_MODE_FIXED, 3, 588, 519, 10027);
> +DEFINE_QNODE(snoc_int_1, 10005, 0, 8, 0, 100, 131, QCOM_QOS_MODE_FIXED, 3, 517, 663, 664);
> +DEFINE_QNODE(snoc_int_bimc, 10006, 0, 8, 0, 101, 132, QCOM_QOS_MODE_FIXED, 1, 10007);
> +DEFINE_QNODE(snoc_bimc_0_mas, 10007, 0, 8, 0, 3, -1, QCOM_QOS_MODE_FIXED, 1, 10025);
> +DEFINE_QNODE(snoc_bimc_1_mas, 10008, 0, 16, 1, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10026);
> +DEFINE_QNODE(qdss_int, 10009, 0, 8, 1, -1, -1, QCOM_QOS_MODE_FIXED, 2, 10004, 10006);
> +DEFINE_QNODE(bimc_snoc_slv, 10017, 0, 8, 1, -1, -1, QCOM_QOS_MODE_FIXED, 2, 10004, 10005);
> +DEFINE_QNODE(snoc_pnoc_mas, 10027, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10028);
> +DEFINE_QNODE(pnoc_snoc_slv, 10011, 0, 8, 0, -1, 45, QCOM_QOS_MODE_FIXED, 3, 10004, 10006, 10005);
> +DEFINE_QNODE(slv_srvc_snoc, 587, 0, 8, 0, -1, 29, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_qdss_stm, 588, 0, 4, 0, -1, 30, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_imem, 519, 0, 8, 0, -1, 26, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_apss, 517, 0, 4, 0, -1, 20, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_cats_0, 663, 0, 16, 0, -1, 106, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_cats_1, 664, 0, 8, 0, -1, 107, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(mas_apss, 1, 0, 8, 1, -1, -1, QCOM_QOS_MODE_FIXED, 3, 512, 10016, 514);
> +DEFINE_QNODE(mas_tcu0, 104, 5, 8, 1, -1, -1, QCOM_QOS_MODE_FIXED, 3, 512, 10016, 514);
> +DEFINE_QNODE(mas_tcu1, 105, 6, 8, 1, -1, -1, QCOM_QOS_MODE_FIXED, 3, 512, 10016, 514);
> +DEFINE_QNODE(mas_gfx, 26, 2, 8, 1, -1, -1, QCOM_QOS_MODE_FIXED, 3, 512, 10016, 514);
> +DEFINE_QNODE(bimc_snoc_mas, 10016, 0, 8, 1, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10017);
> +DEFINE_QNODE(snoc_bimc_0_slv, 10025, 0, 8, 0, -1, 24, QCOM_QOS_MODE_FIXED, 1, 512);
> +DEFINE_QNODE(snoc_bimc_1_slv, 10026, 0, 8, 1, -1, -1, QCOM_QOS_MODE_FIXED, 1, 512);
> +DEFINE_QNODE(slv_ebi_ch0, 512, 0, 8, 0, -1, 0, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_apps_l2, 514, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(snoc_pnoc_slv, 10028, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10012);
> +DEFINE_QNODE(pnoc_int_0, 10012, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 8, 10010, 10018, 10019, 10020, 10021, 10022, 10023, 10024);
> +DEFINE_QNODE(pnoc_int_1, 10013, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10010);
> +DEFINE_QNODE(pnoc_m_0, 10014, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10012);
> +DEFINE_QNODE(pnoc_m_1, 10015, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10010);
> +DEFINE_QNODE(pnoc_s_0, 10018, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 5, 620, 624, 579, 622, 521);
> +DEFINE_QNODE(pnoc_s_1, 10019, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 5, 627, 625, 535, 577, 618);
> +DEFINE_QNODE(pnoc_s_2, 10020, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 5, 533, 630, 629, 641, 632);
> +DEFINE_QNODE(pnoc_s_3, 10021, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 5, 536, 647, 636, 635, 634);
> +DEFINE_QNODE(pnoc_s_4, 10022, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 3, 596, 589, 590);
> +DEFINE_QNODE(pnoc_s_8, 10023, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 3, 614, 606, 613);
> +DEFINE_QNODE(pnoc_s_9, 10024, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 3, 609, 522, 598);
> +DEFINE_QNODE(slv_imem_cfg, 627, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_crypto_0_cfg, 625, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_msg_ram, 535, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_pdm, 577, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_prng, 618, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_clk_ctl, 620, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_mss, 521, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_tlmm, 624, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_tcsr, 579, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_security, 622, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_spdm, 533, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_pnoc_cfg, 641, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_pmic_arb, 632, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_bimc_cfg, 629, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_boot_rom, 630, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_mpm, 536, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_qdss_cfg, 635, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_rbcpr_cfg, 636, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_snoc_cfg, 647, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_dehr_cfg, 634, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_venus_cfg, 596, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_display_cfg, 590, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_camera_cfg, 589, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_usb_hs, 614, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_sdcc_1, 606, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_blsp_1, 613, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_sdcc_2, 609, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_gfx_cfg, 598, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(slv_audio, 522, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 0, 0);
> +DEFINE_QNODE(mas_blsp_1, 86, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10015);
> +DEFINE_QNODE(mas_spdm, 36, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10014);
> +DEFINE_QNODE(mas_dehr, 75, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10014);
> +DEFINE_QNODE(mas_audio, 15, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10014);
> +DEFINE_QNODE(mas_usb_hs, 87, 0, 4, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10015);
> +DEFINE_QNODE(mas_pnoc_crypto_0, 55, 0, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10013);
> +DEFINE_QNODE(mas_pnoc_sdcc_1, 78, 7, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10013);
> +DEFINE_QNODE(mas_pnoc_sdcc_2, 81, 8, 8, 0, -1, -1, QCOM_QOS_MODE_FIXED, 1, 10013);
> +DEFINE_QNODE(pnoc_snoc_mas, 10010, 0, 8, 0, 29, -1, QCOM_QOS_MODE_FIXED, 1, 10011);
> +
> +static struct qcom_icc_node *msm8916_snoc_nodes[] = {
> +       &mas_video,
> +       &mas_jpeg,
> +       &mas_vfe,
> +       &mas_mdp,
> +       &mas_qdss_bam,
> +       &mas_snoc_cfg,
> +       &mas_qdss_etr,
> +       &mm_int_0,
> +       &mm_int_1,
> +       &mm_int_2,
> +       &mm_int_bimc,
> +       &snoc_int_0,
> +       &snoc_int_1,
> +       &snoc_int_bimc,
> +       &snoc_bimc_0_mas,
> +       &snoc_bimc_1_mas,
> +       &qdss_int,
> +       &bimc_snoc_slv,
> +       &snoc_pnoc_mas,
> +       &pnoc_snoc_slv,
> +       &slv_srvc_snoc,
> +       &slv_qdss_stm,
> +       &slv_imem,
> +       &slv_apss,
> +       &slv_cats_0,
> +       &slv_cats_1,
> +};
> +
> +static struct qcom_icc_desc msm8916_snoc = {
> +       .nodes = msm8916_snoc_nodes,
> +       .num_nodes = ARRAY_SIZE(msm8916_snoc_nodes),
> +};
> +
> +static struct qcom_icc_node *msm8916_bimc_nodes[] = {
> +       &mas_apss,
> +       &mas_tcu0,
> +       &mas_tcu1,
> +       &mas_gfx,
> +       &bimc_snoc_mas,
> +       &snoc_bimc_0_slv,
> +       &snoc_bimc_1_slv,
> +       &slv_ebi_ch0,
> +       &slv_apps_l2,
> +};
> +
> +static struct qcom_icc_desc msm8916_bimc = {
> +       .nodes = msm8916_bimc_nodes,
> +       .num_nodes = ARRAY_SIZE(msm8916_bimc_nodes),
> +};
> +
> +static struct qcom_icc_node *msm8916_pnoc_nodes[] = {
> +       &snoc_pnoc_slv,
> +       &pnoc_int_0,
> +       &pnoc_int_1,
> +       &pnoc_m_0,
> +       &pnoc_m_1,
> +       &pnoc_s_0,
> +       &pnoc_s_1,
> +       &pnoc_s_2,
> +       &pnoc_s_3,
> +       &pnoc_s_4,
> +       &pnoc_s_8,
> +       &pnoc_s_9,
> +       &slv_imem_cfg,
> +       &slv_crypto_0_cfg,
> +       &slv_msg_ram,
> +       &slv_pdm,
> +       &slv_prng,
> +       &slv_clk_ctl,
> +       &slv_mss,
> +       &slv_tlmm,
> +       &slv_tcsr,
> +       &slv_security,
> +       &slv_spdm,
> +       &slv_pnoc_cfg,
> +       &slv_pmic_arb,
> +       &slv_bimc_cfg,
> +       &slv_boot_rom,
> +       &slv_mpm,
> +       &slv_qdss_cfg,
> +       &slv_rbcpr_cfg,
> +       &slv_snoc_cfg,
> +       &slv_dehr_cfg,
> +       &slv_venus_cfg,
> +       &slv_display_cfg,
> +       &slv_camera_cfg,
> +       &slv_usb_hs,
> +       &slv_sdcc_1,
> +       &slv_blsp_1,
> +       &slv_sdcc_2,
> +       &slv_gfx_cfg,
> +       &slv_audio,
> +       &mas_blsp_1,
> +       &mas_spdm,
> +       &mas_dehr,
> +       &mas_audio,
> +       &mas_usb_hs,
> +       &mas_pnoc_crypto_0,
> +       &mas_pnoc_sdcc_1,
> +       &mas_pnoc_sdcc_2,
> +       &pnoc_snoc_mas,
> +};
> +
> +static struct qcom_icc_desc msm8916_pnoc = {
> +       .nodes = msm8916_pnoc_nodes,
> +       .num_nodes = ARRAY_SIZE(msm8916_pnoc_nodes),
> +};
> +
> +static int qcom_icc_init(struct icc_node *node)
> +{
> +       struct qcom_icc_provider *qp = to_qcom_provider(node->provider);
> +       /* TODO: init qos and priority */
> +

No need to set_rate here before enabling the clock?

> +       clk_prepare_enable(qp->bus_clk);
> +       clk_prepare_enable(qp->bus_a_clk);
> +
> +       return 0;
> +}
> +
> +static int qcom_icc_set(struct icc_node *src, struct icc_node *dst,
> +                       u32 avg, u32 peak)
> +{
> +       struct qcom_icc_provider *qp;
> +       struct qcom_icc_node *qn;
> +       struct icc_node *node;
> +       struct icc_provider *provider;
> +       u64 avg_bw;
> +       u64 peak_bw;
> +       u64 rate = 0;
> +       int ret = 0;
> +
> +       if (!src)
> +               node = dst;
> +       else
> +               node = src;
> +
> +       qn = node->data;
> +       provider = node->provider;
> +       qp = to_qcom_provider(node->provider);
> +
> +       /* convert from kbps to bps */
> +       avg_bw = avg * 1000ULL;
> +       peak_bw = peak * 1000ULL;
> +

Since the core uses kbps and various SoC HW might use bps (or other
units), perhaps consider providing a macro in the core header such as:

#define icc_units_to_bps(bw)  (bw * 1000ULL)

and then move this conversion to the top of the function where you
define the variable

u64 avg_bw = icc_units_to_bps(avg);
u64 peak_bw = icc_units_to_bps(peak);

Since other drivers will end up copying this driver, it might help
prevent silly errors in the drivers and has a side-effect of allowing
the core to change internal units w/o any driver changes down the
line, if needed.

> +       /* set bandwidth */
> +       if (qn->ap_owned) {
> +               /* TODO: set QoS */
> +       } else {
> +               /* send message to the RPM processor */
> +               if (qn->mas_rpm_id != -1) {
> +                       ret = qcom_icc_rpm_smd_send(QCOM_SMD_RPM_ACTIVE_STATE,
> +                                                   RPM_BUS_MASTER_REQ,
> +                                                   qn->mas_rpm_id,
> +                                                   avg_bw);
> +               }
> +
> +               if (qn->slv_rpm_id != -1) {
> +                       ret = qcom_icc_rpm_smd_send(QCOM_SMD_RPM_ACTIVE_STATE,
> +                                                   RPM_BUS_SLAVE_REQ,
> +                                                   qn->slv_rpm_id,
> +                                                   avg_bw);
> +               }
> +       }
> +
> +       rate = max(avg_bw, peak_bw);
> +
> +       do_div(rate, qn->buswidth);
> +
> +       if (qn->rate != rate) {
> +               ret = clk_set_rate(qp->bus_clk, rate);
> +               if (ret) {
> +                       pr_err("set clk rate %lld error %d\n", rate, ret);
> +                       return ret;
> +               }
> +
> +               ret = clk_set_rate(qp->bus_a_clk, rate);
> +               if (ret) {
> +                       pr_err("set clk rate %lld error %d\n", rate, ret);
> +                       return ret;
> +               }
> +
> +               qn->rate = rate;
> +       }
> +
> +       return ret;
> +}
> +
> +static int qnoc_probe(struct platform_device *pdev)
> +{
> +       const struct qcom_icc_desc *desc;
> +       struct qcom_icc_node **qnodes;
> +       struct qcom_icc_provider *qp;
> +       struct resource *res;
> +       struct icc_provider *provider;
> +       size_t num_nodes, i;
> +       int ret;
> +
> +       /* wait for RPM */
> +       if (!qcom_icc_rpm_smd_available())
> +               return -EPROBE_DEFER;
> +
> +       desc = of_device_get_match_data(&pdev->dev);
> +       if (!desc)
> +               return -EINVAL;
> +
> +       qnodes = desc->nodes;
> +       num_nodes = desc->num_nodes;
> +
> +       qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
> +       if (!qp)
> +               return -ENOMEM;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       qp->base = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(qp->base))
> +               return PTR_ERR(qp->base);
> +
> +       qp->bus_clk = devm_clk_get(&pdev->dev, "bus_clk");
> +       if (IS_ERR(qp->bus_clk))
> +               return PTR_ERR(qp->bus_clk);
> +
> +       qp->bus_a_clk = devm_clk_get(&pdev->dev, "bus_a_clk");
> +       if (IS_ERR(qp->bus_a_clk))
> +               return PTR_ERR(qp->bus_a_clk);
> +
> +       provider = &qp->provider;
> +       provider->dev = &pdev->dev;
> +       provider->set = &qcom_icc_set;
> +       INIT_LIST_HEAD(&provider->nodes);
> +       provider->data = qp;
> +
> +       ret = icc_add_provider(provider);
> +       if (ret) {
> +               dev_err(&pdev->dev, "error adding interconnect provider\n");
> +               return ret;
> +       }
> +
> +       for (i = 0; i < num_nodes; i++) {
> +               struct icc_node *node;
> +               int ret;
> +               size_t j;
> +
> +               node = icc_node_create(qnodes[i]->id);
> +               if (IS_ERR(node)) {
> +                       ret = PTR_ERR(node);
> +                       goto err;
> +               }
> +
> +               node->name = qnodes[i]->name;
> +               node->data = qnodes[i];
> +               icc_node_add(node, provider);
> +
> +               dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
> +                       qnodes[i]->name, node->id);
> +
> +               /* populate links */
> +               for (j = 0; j < qnodes[i]->num_links; j++)
> +                       if (qnodes[i]->links[j])
> +                               icc_link_create(node, qnodes[i]->links[j]);
> +
> +               ret = qcom_icc_init(node);
> +               if (ret)
> +                       dev_err(&pdev->dev, "%s init error (%d)\n", node->name,
> +                               ret);
> +       }
> +
> +       platform_set_drvdata(pdev, provider);
> +
> +       return ret;
> +err:
> +       icc_del_provider(provider);
> +       return ret;
> +}
> +
> +static int qnoc_remove(struct platform_device *pdev)
> +{
> +       struct icc_provider *provider = platform_get_drvdata(pdev);
> +
> +       icc_del_provider(provider);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id qnoc_of_match[] = {
> +       { .compatible = "qcom,msm8916-pnoc", .data = &msm8916_pnoc },
> +       { .compatible = "qcom,msm8916-snoc", .data = &msm8916_snoc },
> +       { .compatible = "qcom,msm8916-bimc", .data = &msm8916_bimc },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, qnoc_of_match);
> +
> +static struct platform_driver qnoc_driver = {
> +       .probe = qnoc_probe,
> +       .remove = qnoc_remove,
> +       .driver = {
> +               .name = "qnoc-msm8916",
> +               .of_match_table = qnoc_of_match,
> +       },
> +};
> +module_platform_driver(qnoc_driver);
> +MODULE_AUTHOR("Georgi Djakov <georgi.djakov@linaro.org>");
> +MODULE_DESCRIPTION("Qualcomm msm8916 NoC driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/interconnect/qcom.h b/include/linux/interconnect/qcom.h
> new file mode 100644
> index 000000000000..2cd378d2f575
> --- /dev/null
> +++ b/include/linux/interconnect/qcom.h
> @@ -0,0 +1,350 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Qualcomm interconnect IDs
> + *
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef __QCOM_INTERCONNECT_IDS_H
> +#define __QCOM_INTERCONNECT_IDS_H
> +
> +#define FAB_BIMC 0
> +#define FAB_SYS_NOC 1024
> +#define FAB_MMSS_NOC 2048
> +#define FAB_OCMEM_NOC 3072
> +#define FAB_PERIPH_NOC 4096
> +#define FAB_CONFIG_NOC 5120
> +#define FAB_OCMEM_VNOC 6144
> +
> +#define MASTER_AMPSS_M0 1
> +#define MASTER_AMPSS_M1 2
> +#define APPSS_MASTER_FAB_MMSS 3
> +#define APPSS_MASTER_FAB_SYSTEM 4
> +#define SYSTEM_MASTER_FAB_APPSS 5
> +#define MASTER_SPS 6
> +#define MASTER_ADM_PORT0 7
> +#define MASTER_ADM_PORT1 8
> +#define SYSTEM_MASTER_ADM1_PORT0 9
> +#define MASTER_ADM1_PORT1 10
> +#define MASTER_LPASS_PROC 11
> +#define MASTER_MSS_PROCI 12
> +#define MASTER_MSS_PROCD 13
> +#define MASTER_MSS_MDM_PORT0 14
> +#define MASTER_LPASS 15
> +#define SYSTEM_MASTER_CPSS_FPB 16
> +#define SYSTEM_MASTER_SYSTEM_FPB 17
> +#define SYSTEM_MASTER_MMSS_FPB 18
> +#define MASTER_ADM1_CI 19
> +#define MASTER_ADM0_CI 20
> +#define MASTER_MSS_MDM_PORT1 21
> +#define MASTER_MDP_PORT0 22
> +#define MASTER_MDP_PORT1 23
> +#define MMSS_MASTER_ADM1_PORT0 24
> +#define MASTER_ROTATOR 25
> +#define MASTER_GRAPHICS_3D 26
> +#define MASTER_JPEG_DEC 27
> +#define MASTER_GRAPHICS_2D_CORE0 28
> +#define MASTER_VFE 29
> +#define MASTER_VPE 30
> +#define MASTER_JPEG_ENC 31
> +#define MASTER_GRAPHICS_2D_CORE1 32
> +#define MMSS_MASTER_APPS_FAB 33
> +#define MASTER_HD_CODEC_PORT0 34
> +#define MASTER_HD_CODEC_PORT1 35
> +#define MASTER_SPDM 36
> +#define MASTER_RPM 37
> +#define MASTER_MSS 38
> +#define MASTER_RIVA 39
> +#define SYSTEM_MASTER_UNUSED_6 40
> +#define MASTER_MSS_SW_PROC 41
> +#define MASTER_MSS_FW_PROC 42
> +#define MMSS_MASTER_UNUSED_2 43
> +#define MASTER_GSS_NAV 44
> +#define MASTER_PCIE 45
> +#define MASTER_SATA 46
> +#define MASTER_CRYPTO 47
> +#define MASTER_VIDEO_CAP 48
> +#define MASTER_GRAPHICS_3D_PORT1 49
> +#define MASTER_VIDEO_ENC 50
> +#define MASTER_VIDEO_DEC 51
> +#define MASTER_LPASS_AHB 52
> +#define MASTER_QDSS_BAM 53
> +#define MASTER_SNOC_CFG 54
> +#define MASTER_CRYPTO_CORE0 55
> +#define MASTER_CRYPTO_CORE1 56
> +#define MASTER_MSS_NAV 57
> +#define MASTER_OCMEM_DMA 58
> +#define MASTER_WCSS 59
> +#define MASTER_QDSS_ETR 60
> +#define MASTER_USB3 61
> +#define MASTER_JPEG 62
> +#define MASTER_VIDEO_P0 63
> +#define MASTER_VIDEO_P1 64
> +#define MASTER_MSS_PROC 65
> +#define MASTER_JPEG_OCMEM 66
> +#define MASTER_MDP_OCMEM 67
> +#define MASTER_VIDEO_P0_OCMEM 68
> +#define MASTER_VIDEO_P1_OCMEM 69
> +#define MASTER_VFE_OCMEM 70
> +#define MASTER_CNOC_ONOC_CFG 71
> +#define MASTER_RPM_INST 72
> +#define MASTER_RPM_DATA 73
> +#define MASTER_RPM_SYS 74
> +#define MASTER_DEHR 75
> +#define MASTER_QDSS_DAP 76
> +#define MASTER_TIC 77
> +#define MASTER_SDCC_1 78
> +#define MASTER_SDCC_3 79
> +#define MASTER_SDCC_4 80
> +#define MASTER_SDCC_2 81
> +#define MASTER_TSIF 82
> +#define MASTER_BAM_DMA 83
> +#define MASTER_BLSP_2 84
> +#define MASTER_USB_HSIC 85
> +#define MASTER_BLSP_1 86
> +#define MASTER_USB_HS 87
> +#define MASTER_PNOC_CFG 88
> +#define MASTER_V_OCMEM_GFX3D 89
> +#define MASTER_IPA 90
> +#define MASTER_QPIC 91
> +#define MASTER_MDPE 92
> +#define MASTER_USB_HS2 93
> +#define MASTER_VPU 94
> +#define MASTER_UFS 95
> +#define MASTER_BCAST 96
> +#define MASTER_CRYPTO_CORE2 97
> +#define MASTER_EMAC 98
> +#define MASTER_VPU_1 99
> +#define MASTER_PCIE_1 100
> +#define MASTER_USB3_1 101
> +#define MASTER_CNOC_MNOC_MMSS_CFG 102
> +#define MASTER_CNOC_MNOC_CFG 103
> +#define MASTER_TCU_0 104
> +#define MASTER_TCU_1 105
> +#define MASTER_CPP 106
> +#define MASTER_AUDIO 107
> +
> +#define SNOC_MM_INT_0 10000
> +#define SNOC_MM_INT_1 10001
> +#define SNOC_MM_INT_2 10002
> +#define SNOC_MM_INT_BIMC 10003
> +#define SNOC_INT_0 10004
> +#define SNOC_INT_1 10005
> +#define SNOC_INT_BIMC 10006
> +#define SNOC_BIMC_0_MAS 10007
> +#define SNOC_BIMC_1_MAS 10008
> +#define SNOC_QDSS_INT 10009
> +#define PNOC_SNOC_MAS 10010
> +#define PNOC_SNOC_SLV 10011
> +#define PNOC_INT_0 10012
> +#define PNOC_INT_1 10013
> +#define PNOC_M_0 10014
> +#define PNOC_M_1 10015
> +#define BIMC_SNOC_MAS 10016
> +#define BIMC_SNOC_SLV 10017
> +#define PNOC_SLV_0 10018
> +#define PNOC_SLV_1 10019
> +#define PNOC_SLV_2 10020
> +#define PNOC_SLV_3 10021
> +#define PNOC_SLV_4 10022
> +#define PNOC_SLV_8 10023
> +#define PNOC_SLV_9 10024
> +#define SNOC_BIMC_0_SLV 10025
> +#define SNOC_BIMC_1_SLV 10026
> +#define MNOC_BIMC_MAS 10027
> +#define MNOC_BIMC_SLV 10028
> +#define BIMC_MNOC_MAS 10029
> +#define BIMC_MNOC_SLV 10030
> +#define SNOC_BIMC_MAS 10031
> +#define SNOC_BIMC_SLV 10032
> +#define CNOC_SNOC_MAS 10033
> +#define CNOC_SNOC_SLV 10034
> +#define SNOC_CNOC_MAS 10035
> +#define SNOC_CNOC_SLV 10036
> +#define OVNOC_SNOC_MAS 10037
> +#define OVNOC_SNOC_SLV 10038
> +#define SNOC_OVNOC_MAS 10039
> +#define SNOC_OVNOC_SLV 10040
> +#define SNOC_PNOC_MAS 10041
> +#define SNOC_PNOC_SLV 10042
> +#define BIMC_INT_APPS_EBI 10043
> +#define BIMC_INT_APPS_SNOC 10044
> +#define SNOC_BIMC_2_MAS 10045
> +#define SNOC_BIMC_2_SLV 10046
> +#define PNOC_SLV_5 10047
> +#define PNOC_SLV_7 10048
> +#define PNOC_INT_2 10049
> +#define PNOC_INT_3 10050
> +#define PNOC_INT_4 10051
> +#define PNOC_INT_5 10052
> +#define PNOC_INT_6 10053
> +#define PNOC_INT_7 10054
> +
> +#define SLAVE_EBI_CH0 512
> +#define SLAVE_EBI_CH1 513
> +#define SLAVE_AMPSS_L2 514
> +#define APPSS_SLAVE_FAB_MMSS 515
> +#define APPSS_SLAVE_FAB_SYSTEM 516
> +#define SYSTEM_SLAVE_FAB_APPS 517
> +#define SLAVE_SPS 518
> +#define SLAVE_SYSTEM_IMEM 519
> +#define SLAVE_AMPSS 520
> +#define SLAVE_MSS 521
> +#define SLAVE_LPASS 522
> +#define SYSTEM_SLAVE_CPSS_FPB 523
> +#define SYSTEM_SLAVE_SYSTEM_FPB 524
> +#define SYSTEM_SLAVE_MMSS_FPB 525
> +#define SLAVE_CORESIGHT 526
> +#define SLAVE_RIVA 527
> +#define SLAVE_SMI 528
> +#define MMSS_SLAVE_FAB_APPS 529
> +#define MMSS_SLAVE_FAB_APPS_1 530
> +#define SLAVE_MM_IMEM 531
> +#define SLAVE_CRYPTO 532
> +#define SLAVE_SPDM 533
> +#define SLAVE_RPM 534
> +#define SLAVE_RPM_MSG_RAM 535
> +#define SLAVE_MPM 536
> +#define SLAVE_PMIC1_SSBI1_A 537
> +#define SLAVE_PMIC1_SSBI1_B 538
> +#define SLAVE_PMIC1_SSBI1_C 539
> +#define SLAVE_PMIC2_SSBI2_A 540
> +#define SLAVE_PMIC2_SSBI2_B 541
> +#define SLAVE_GSBI1_UART 542
> +#define SLAVE_GSBI2_UART 543
> +#define SLAVE_GSBI3_UART 544
> +#define SLAVE_GSBI4_UART 545
> +#define SLAVE_GSBI5_UART 546
> +#define SLAVE_GSBI6_UART 547
> +#define SLAVE_GSBI7_UART 548
> +#define SLAVE_GSBI8_UART 549
> +#define SLAVE_GSBI9_UART 550
> +#define SLAVE_GSBI10_UART 551
> +#define SLAVE_GSBI11_UART 552
> +#define SLAVE_GSBI12_UART 553
> +#define SLAVE_GSBI1_QUP 554
> +#define SLAVE_GSBI2_QUP 555
> +#define SLAVE_GSBI3_QUP 556
> +#define SLAVE_GSBI4_QUP 557
> +#define SLAVE_GSBI5_QUP 558
> +#define SLAVE_GSBI6_QUP 559
> +#define SLAVE_GSBI7_QUP 560
> +#define SLAVE_GSBI8_QUP 561
> +#define SLAVE_GSBI9_QUP 562
> +#define SLAVE_GSBI10_QUP 563
> +#define SLAVE_GSBI11_QUP 564
> +#define SLAVE_GSBI12_QUP 565
> +#define SLAVE_EBI2_NAND 566
> +#define SLAVE_EBI2_CS0 567
> +#define SLAVE_EBI2_CS1 568
> +#define SLAVE_EBI2_CS2 569
> +#define SLAVE_EBI2_CS3 570
> +#define SLAVE_EBI2_CS4 571
> +#define SLAVE_EBI2_CS5 572
> +#define SLAVE_USB_FS1 573
> +#define SLAVE_USB_FS2 574
> +#define SLAVE_TSIF 575
> +#define SLAVE_MSM_TSSC 576
> +#define SLAVE_MSM_PDM 577
> +#define SLAVE_MSM_DIMEM 578
> +#define SLAVE_MSM_TCSR 579
> +#define SLAVE_MSM_PRNG 580
> +#define SLAVE_GSS 581
> +#define SLAVE_SATA 582
> +#define SLAVE_USB3 583
> +#define SLAVE_WCSS 584
> +#define SLAVE_OCIMEM 585
> +#define SLAVE_SNOC_OCMEM 586
> +#define SLAVE_SERVICE_SNOC 587
> +#define SLAVE_QDSS_STM 588
> +#define SLAVE_CAMERA_CFG 589
> +#define SLAVE_DISPLAY_CFG 590
> +#define SLAVE_OCMEM_CFG 591
> +#define SLAVE_CPR_CFG 592
> +#define SLAVE_CPR_XPU_CFG 593
> +#define SLAVE_MISC_CFG 594
> +#define SLAVE_MISC_XPU_CFG 595
> +#define SLAVE_VENUS_CFG 596
> +#define SLAVE_MISC_VENUS_CFG 597
> +#define SLAVE_GRAPHICS_3D_CFG 598
> +#define SLAVE_MMSS_CLK_CFG 599
> +#define SLAVE_MMSS_CLK_XPU_CFG 600
> +#define SLAVE_MNOC_MPU_CFG 601
> +#define SLAVE_ONOC_MPU_CFG 602
> +#define SLAVE_SERVICE_MNOC 603
> +#define SLAVE_OCMEM 604
> +#define SLAVE_SERVICE_ONOC 605
> +#define SLAVE_SDCC_1 606
> +#define SLAVE_SDCC_3 607
> +#define SLAVE_SDCC_2 608
> +#define SLAVE_SDCC_4 609
> +#define SLAVE_BAM_DMA 610
> +#define SLAVE_BLSP_2 611
> +#define SLAVE_USB_HSIC 612
> +#define SLAVE_BLSP_1 613
> +#define SLAVE_USB_HS 614
> +#define SLAVE_PDM 615
> +#define SLAVE_PERIPH_APU_CFG 616
> +#define SLAVE_PNOC_MPU_CFG 617
> +#define SLAVE_PRNG 618
> +#define SLAVE_SERVICE_PNOC 619
> +#define SLAVE_CLK_CTL 620
> +#define SLAVE_CNOC_MSS 621
> +#define SLAVE_SECURITY 622
> +#define SLAVE_TCSR 623
> +#define SLAVE_TLMM 624
> +#define SLAVE_CRYPTO_0_CFG 625
> +#define SLAVE_CRYPTO_1_CFG 626
> +#define SLAVE_IMEM_CFG 627
> +#define SLAVE_MESSAGE_RAM 628
> +#define SLAVE_BIMC_CFG 629
> +#define SLAVE_BOOT_ROM 630
> +#define SLAVE_CNOC_MNOC_MMSS_CFG 631
> +#define SLAVE_PMIC_ARB 632
> +#define SLAVE_SPDM_WRAPPER 633
> +#define SLAVE_DEHR_CFG 634
> +#define SLAVE_QDSS_CFG 635
> +#define SLAVE_RBCPR_CFG 636
> +#define SLAVE_RBCPR_QDSS_APU_CFG 637
> +#define SLAVE_SNOC_MPU_CFG 638
> +#define SLAVE_CNOC_ONOC_CFG 639
> +#define SLAVE_CNOC_MNOC_CFG 640
> +#define SLAVE_PNOC_CFG 641
> +#define SLAVE_SNOC_CFG 642
> +#define SLAVE_EBI1_DLL_CFG 643
> +#define SLAVE_PHY_APU_CFG 644
> +#define SLAVE_EBI1_PHY_CFG 645
> +#define SLAVE_SERVICE_CNOC 646
> +#define SLAVE_IPS_CFG 647
> +#define SLAVE_QPIC 648
> +#define SLAVE_DSI_CFG 649
> +#define SLAVE_UFS_CFG 650
> +#define SLAVE_RBCPR_CX_CFG 651
> +#define SLAVE_RBCPR_MX_CFG 652
> +#define SLAVE_PCIE_CFG 653
> +#define SLAVE_USB_PHYS_CFG 654
> +#define SLAVE_VIDEO_CAP_CFG 655
> +#define SLAVE_AVSYNC_CFG 656
> +#define SLAVE_CRYPTO_2_CFG 657
> +#define SLAVE_VPU_CFG 658
> +#define SLAVE_BCAST_CFG 659
> +#define SLAVE_KLM_CFG 660
> +#define SLAVE_GENI_IR_CFG 661
> +#define SLAVE_OCMEM_GFX 662
> +#define SLAVE_CATS_128 663
> +#define SLAVE_OCMEM_64 664
> +#define SLAVE_PCIE_0 665
> +#define SLAVE_PCIE_1 666
> +#define SLAVE_PCIE_0_CFG 667
> +#define SLAVE_PCIE_1_CFG 668
> +#define SLAVE_SRVC_MNOC 669
> +#define SLAVE_USB_HS2 670
> +#define SLAVE_AUDIO 671
> +#define SLAVE_TCU 672
> +#define SLAVE_APPSS 673
> +#define SLAVE_PCIE_PARF 674
> +#define SLAVE_USB3_PHY_CFG 675
> +#define SLAVE_IPA_CFG 676
> +
> +#endif

^ permalink raw reply

* [PATCH v4 1/7] interconnect: Add generic on-chip interconnect API
From: Amit Kucheria @ 2018-05-25  8:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180309210958.16672-2-georgi.djakov@linaro.org>

On Fri, Mar 9, 2018 at 11:09 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
> This patch introduce a new API to get requirements and configure the
> interconnect buses across the entire chipset to fit with the current
> demand.
>
> The API is using a consumer/provider-based model, where the providers are
> the interconnect buses and the consumers could be various drivers.
> The consumers request interconnect resources (path) between endpoints and
> set the desired constraints on this data flow path. The providers receive
> requests from consumers and aggregate these requests for all master-slave
> pairs on that path. Then the providers configure each participating in the
> topology node according to the requested data flow path, physical links and
> constraints. The topology could be complicated and multi-tiered and is SoC
> specific.
>
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
>  Documentation/interconnect/interconnect.rst |  96 ++++++
>  drivers/Kconfig                             |   2 +
>  drivers/Makefile                            |   1 +
>  drivers/interconnect/Kconfig                |  10 +
>  drivers/interconnect/Makefile               |   1 +
>  drivers/interconnect/core.c                 | 489 ++++++++++++++++++++++++++++
>  include/linux/interconnect-provider.h       | 109 +++++++
>  include/linux/interconnect.h                |  40 +++
>  8 files changed, 748 insertions(+)
>  create mode 100644 Documentation/interconnect/interconnect.rst
>  create mode 100644 drivers/interconnect/Kconfig
>  create mode 100644 drivers/interconnect/Makefile
>  create mode 100644 drivers/interconnect/core.c
>  create mode 100644 include/linux/interconnect-provider.h
>  create mode 100644 include/linux/interconnect.h
>
> diff --git a/Documentation/interconnect/interconnect.rst b/Documentation/interconnect/interconnect.rst
> new file mode 100644
> index 000000000000..23eba68e8424
> --- /dev/null
> +++ b/Documentation/interconnect/interconnect.rst
> @@ -0,0 +1,96 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=====================================
> +GENERIC SYSTEM INTERCONNECT SUBSYSTEM
> +=====================================
> +
> +Introduction
> +------------
> +
> +This framework is designed to provide a standard kernel interface to control
> +the settings of the interconnects on a SoC. These settings can be throughput,
> +latency and priority between multiple interconnected devices or functional
> +blocks. This can be controlled dynamically in order to save power or provide
> +maximum performance.
> +
> +The interconnect bus is a hardware with configurable parameters, which can be
> +set on a data path according to the requests received from various drivers.
> +An example of interconnect buses are the interconnects between various
> +components or functional blocks in chipsets. There can be multiple interconnects
> +on a SoC that can be multi-tiered.
> +
> +Below is a simplified diagram of a real-world SoC interconnect bus topology.
> +
> +::
> +
> + +----------------+    +----------------+
> + | HW Accelerator |--->|      M NoC     |<---------------+
> + +----------------+    +----------------+                |
> +                         |      |                    +------------+
> +  +-----+  +-------------+      V       +------+     |            |
> +  | DDR |  |                +--------+  | PCIe |     |            |
> +  +-----+  |                | Slaves |  +------+     |            |
> +    ^ ^    |                +--------+     |         |   C NoC    |
> +    | |    V                               V         |            |
> + +------------------+   +------------------------+   |            |   +-----+
> + |                  |-->|                        |-->|            |-->| CPU |
> + |                  |-->|                        |<--|            |   +-----+
> + |     Mem NoC      |   |         S NoC          |   +------------+
> + |                  |<--|                        |---------+    |
> + |                  |<--|                        |<------+ |    |   +--------+
> + +------------------+   +------------------------+       | |    +-->| Slaves |
> +   ^  ^    ^    ^          ^                             | |        +--------+
> +   |  |    |    |          |                             | V
> + +------+  |  +-----+   +-----+  +---------+   +----------------+   +--------+
> + | CPUs |  |  | GPU |   | DSP |  | Masters |-->|       P NoC    |-->| Slaves |
> + +------+  |  +-----+   +-----+  +---------+   +----------------+   +--------+
> +           |
> +       +-------+
> +       | Modem |
> +       +-------+
> +
> +Terminology
> +-----------
> +
> +Interconnect provider is the software definition of the interconnect hardware.
> +The interconnect providers on the above diagram are M NoC, S NoC, C NoC and Mem
> +NoC.
> +
> +Interconnect node is the software definition of the interconnect hardware
> +port. Each interconnect provider consists of multiple interconnect nodes,
> +which are connected to other SoC components including other interconnect
> +providers. The point on the diagram where the CPUs connects to the memory is
> +called an interconnect node, which belongs to the Mem NoC interconnect provider.
> +
> +Interconnect endpoints are the first or the last element of the path. Every
> +endpoint is a node, but not every node is an endpoint.
> +
> +Interconnect path is everything between two endpoints including all the nodes
> +that have to be traversed to reach from a source to destination node. It may
> +include multiple master-slave pairs across several interconnect providers.
> +
> +Interconnect consumers are the entities which make use of the data paths exposed
> +by the providers. The consumers send requests to providers requesting various
> +throughput, latency and priority. Usually the consumers are device drivers, that
> +send request based on their needs. An example for a consumer is a video decoder
> +that supports various formats and image sizes.
> +
> +Interconnect providers
> +----------------------
> +
> +Interconnect provider is an entity that implements methods to initialize and
> +configure a interconnect bus hardware. The interconnect provider drivers should
> +be registered with the interconnect provider core.
> +
> +The interconnect framework provider API functions are documented in
> +.. kernel-doc:: include/linux/interconnect-provider.h
> +
> +Interconnect consumers
> +----------------------
> +
> +Interconnect consumers are the clients which use the interconnect APIs to
> +get paths between endpoints and set their bandwidth/latency/QoS requirements
> +for these interconnect paths.
> +

This document is missing a section on the locking semantics of the
framework. Does the core ensure that the entire path is locked for
set() to propagate?

> +The interconnect framework consumer API functions are documented in
> +.. kernel-doc:: include/linux/interconnect.h
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 879dc0604cba..96a1db022cee 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -219,4 +219,6 @@ source "drivers/siox/Kconfig"
>
>  source "drivers/slimbus/Kconfig"
>
> +source "drivers/interconnect/Kconfig"
> +
>  endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 24cd47014657..0cca95740d9b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -185,3 +185,4 @@ obj-$(CONFIG_TEE)           += tee/
>  obj-$(CONFIG_MULTIPLEXER)      += mux/
>  obj-$(CONFIG_UNISYS_VISORBUS)  += visorbus/
>  obj-$(CONFIG_SIOX)             += siox/
> +obj-$(CONFIG_INTERCONNECT)     += interconnect/
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> new file mode 100644
> index 000000000000..a261c7d41deb
> --- /dev/null
> +++ b/drivers/interconnect/Kconfig
> @@ -0,0 +1,10 @@
> +menuconfig INTERCONNECT
> +       tristate "On-Chip Interconnect management support"
> +       help
> +         Support for management of the on-chip interconnects.
> +
> +         This framework is designed to provide a generic interface for
> +         managing the interconnects in a SoC.
> +
> +         If unsure, say no.
> +
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> new file mode 100644
> index 000000000000..5edf0ae80818
> --- /dev/null
> +++ b/drivers/interconnect/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_INTERCONNECT)             += core.o
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> new file mode 100644
> index 000000000000..6306e258b9b9
> --- /dev/null
> +++ b/drivers/interconnect/core.c
> @@ -0,0 +1,489 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Interconnect framework core driver
> + *
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/idr.h>
> +#include <linux/init.h>
> +#include <linux/interconnect.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +
> +static DEFINE_IDR(icc_idr);
> +static LIST_HEAD(icc_provider_list);
> +static DEFINE_MUTEX(icc_provider_list_mutex);
> +static DEFINE_MUTEX(icc_path_mutex);
> +
> +/**
> + * struct icc_req - constraints that are attached to each node
> + *
> + * @req_node: entry in list of requests for the particular @node
> + * @node: the interconnect node to which this constraint applies
> + * @avg_bw: an integer describing the average bandwidth in kbps
> + * @peak_bw: an integer describing the peak bandwidth in kbps
> + */
> +struct icc_req {
> +       struct hlist_node req_node;
> +       struct icc_node *node;
> +       u32 avg_bw;
> +       u32 peak_bw;
> +};
> +
> +/**
> + * struct icc_path - interconnect path structure
> + * @num_nodes: number of hops (nodes)
> + * @reqs: array of the requests applicable to this path of nodes
> + */
> +struct icc_path {
> +       size_t num_nodes;
> +       struct icc_req reqs[0];
> +};
> +
> +static struct icc_node *node_find(const int id)
> +{
> +       struct icc_node *node;
> +
> +       node = idr_find(&icc_idr, id);
> +
> +       return node;
> +}
> +
> +static struct icc_path *path_allocate(struct icc_node *node, ssize_t num_nodes)
> +{
> +       struct icc_path *path;
> +       size_t i;
> +
> +       path = kzalloc(sizeof(*path) + num_nodes * sizeof(*path->reqs),
> +                      GFP_KERNEL);
> +       if (!path)
> +               return ERR_PTR(-ENOMEM);
> +
> +       path->num_nodes = num_nodes;
> +
> +       for (i = 0; i < num_nodes; i++) {
> +               hlist_add_head(&path->reqs[i].req_node, &node->req_list);
> +
> +               path->reqs[i].node = node;
> +               /* reference to previous node was saved during path traversal */
> +               node = node->reverse;
> +       }
> +
> +       return path;
> +}
> +
> +static struct icc_path *path_find(struct icc_node *src, struct icc_node *dst)
> +{
> +       struct icc_node *node = NULL;
> +       struct list_head traverse_list;
> +       struct list_head edge_list;
> +       struct list_head tmp_list;
> +       size_t i, number = 0;
> +       bool found = false;
> +
> +       INIT_LIST_HEAD(&traverse_list);
> +       INIT_LIST_HEAD(&edge_list);
> +       INIT_LIST_HEAD(&tmp_list);
> +
> +       list_add_tail(&src->search_list, &traverse_list);
> +
> +       do {
> +               list_for_each_entry(node, &traverse_list, search_list) {
> +                       if (node == dst) {
> +                               found = true;
> +                               list_add(&node->search_list, &tmp_list);
> +                               break;
> +                       }
> +                       for (i = 0; i < node->num_links; i++) {
> +                               struct icc_node *tmp = node->links[i];
> +
> +                               if (!tmp)
> +                                       return ERR_PTR(-ENOENT);
> +
> +                               if (tmp->is_traversed)
> +                                       continue;
> +
> +                               tmp->is_traversed = true;
> +                               tmp->reverse = node;
> +                               list_add_tail(&tmp->search_list, &edge_list);
> +                       }
> +               }
> +               if (found)
> +                       break;
> +
> +               list_splice_init(&traverse_list, &tmp_list);
> +               list_splice_init(&edge_list, &traverse_list);
> +
> +               /* count the number of nodes */
> +               number++;
> +
> +       } while (!list_empty(&traverse_list));
> +
> +       /* reset the traversed state */
> +       list_for_each_entry(node, &tmp_list, search_list)
> +               node->is_traversed = false;
> +
> +       if (found)
> +               return path_allocate(dst, number);
> +
> +       return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int path_init(struct icc_path *path)
> +{
> +       struct icc_node *node;
> +       size_t i;
> +
> +       for (i = 0; i < path->num_nodes; i++) {
> +               node = path->reqs[i].node;
> +
> +               mutex_lock(&node->provider->lock);
> +               node->provider->users++;
> +               mutex_unlock(&node->provider->lock);
> +       }
> +
> +       return 0;
> +}
> +

Consider adding some comments for node_aggregate and
provider_aggregate's aggregation algorithm

"We want the path to honor all bandwidth requests, so the average
bandwidth requirements from each consumer are aggregated at each node
and provider level. The peak bandwidth requirements will then be the
highest of all the peak bw requests"

or something to the effect that.

> +static void node_aggregate(struct icc_node *node)
> +{
> +       struct icc_req *r;
> +       u32 agg_avg = 0;
> +       u32 agg_peak = 0;
> +
> +       hlist_for_each_entry(r, &node->req_list, req_node) {
> +               /* sum(averages) and max(peaks) */
> +               agg_avg += r->avg_bw;
> +               agg_peak = max(agg_peak, r->peak_bw);
> +       }
> +
> +       node->avg_bw = agg_avg;
> +       node->peak_bw = agg_peak;
> +}
> +
> +static void provider_aggregate(struct icc_provider *provider, u32 *avg_bw,
> +                              u32 *peak_bw)
> +{
> +       struct icc_node *n;
> +       u32 agg_avg = 0;
> +       u32 agg_peak = 0;
> +
> +       /* aggregate for the interconnect provider */

You could get rid of this, the function name says as much.

> +       list_for_each_entry(n, &provider->nodes, node_list) {
> +               /* sum the average and max the peak */
> +               agg_avg += n->avg_bw;
> +               agg_peak = max(agg_peak, n->peak_bw);
> +       }
> +
> +       *avg_bw = agg_avg;
> +       *peak_bw = agg_peak;
> +}
> +
> +static int constraints_apply(struct icc_path *path)
> +{
> +       struct icc_node *next, *prev = NULL;
> +       int i;
> +
> +       for (i = 0; i < path->num_nodes; i++, prev = next) {
> +               struct icc_provider *provider;
> +               u32 avg_bw = 0;
> +               u32 peak_bw = 0;
> +               int ret;
> +
> +               next = path->reqs[i].node;
> +               /*
> +                * Both endpoints should be valid master-slave pairs of the
> +                * same interconnect provider that will be configured.
> +                */
> +               if (!next || !prev)
> +                       continue;
> +
> +               if (next->provider != prev->provider)
> +                       continue;
> +
> +               provider = next->provider;
> +               mutex_lock(&provider->lock);
> +
> +               /* aggregate requests for the provider */

Get rid of comment.

> +               provider_aggregate(provider, &avg_bw, &peak_bw);
> +
> +               if (provider->set) {
> +                       /* set the constraints */
> +                       ret = provider->set(prev, next, avg_bw, peak_bw);
> +               }
> +
> +               mutex_unlock(&provider->lock);
> +
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * icc_set() - set constraints on an interconnect path between two endpoints
> + * @path: reference to the path returned by icc_get()
> + * @avg_bw: average bandwidth in kbps
> + * @peak_bw: peak bandwidth in kbps
> + *
> + * This function is used by an interconnect consumer to express its own needs
> + * in term of bandwidth and QoS for a previously requested path between two
> + * endpoints. The requests are aggregated and each node is updated accordingly.
> + *
> + * Returns 0 on success, or an approproate error code otherwise.

*appropriate*

> + */
> +int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> +{
> +       struct icc_node *node;
> +       size_t i;
> +       int ret;
> +
> +       if (!path)
> +               return 0;
> +
> +       for (i = 0; i < path->num_nodes; i++) {
> +               node = path->reqs[i].node;
> +
> +               mutex_lock(&icc_path_mutex);
> +
> +               /* update the consumer request for this path */
> +               path->reqs[i].avg_bw = avg_bw;
> +               path->reqs[i].peak_bw = peak_bw;
> +
> +               /* aggregate requests for this node */
> +               node_aggregate(node);
> +
> +               mutex_unlock(&icc_path_mutex);
> +       }
> +
> +       ret = constraints_apply(path);
> +       if (ret)
> +               pr_err("interconnect: error applying constraints (%d)", ret);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(icc_set);
> +
> +/**
> + * icc_get() - return a handle for path between two endpoints
> + * @src_id: source device port id
> + * @dst_id: destination device port id
> + *
> + * This function will search for a path between two endpoints and return an
> + * icc_path handle on success. Use icc_put() to release
> + * constraints when the they are not needed anymore.
> + *
> + * Return: icc_path pointer on success, or ERR_PTR() on error
> + */
> +struct icc_path *icc_get(const int src_id, const int dst_id)
> +{
> +       struct icc_node *src, *dst;
> +       struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
> +
> +       src = node_find(src_id);
> +       if (!src)
> +               goto out;
> +
> +       dst = node_find(dst_id);
> +       if (!dst)
> +               goto out;
> +
> +       mutex_lock(&icc_path_mutex);
> +       path = path_find(src, dst);
> +       mutex_unlock(&icc_path_mutex);
> +       if (IS_ERR(path))
> +               goto out;
> +
> +       path_init(path);
> +
> +out:
> +       return path;
> +}
> +EXPORT_SYMBOL_GPL(icc_get);
> +
> +/**
> + * icc_put() - release the reference to the icc_path
> + * @path: interconnect path
> + *
> + * Use this function to release the constraints on a path when the path is
> + * no longer needed. The constraints will be re-aggregated.
> + */
> +void icc_put(struct icc_path *path)
> +{
> +       struct icc_node *node;
> +       size_t i;
> +       int ret;
> +
> +       if (!path || WARN_ON_ONCE(IS_ERR(path)))
> +               return;
> +
> +       ret = icc_set(path, 0, 0);
> +       if (ret)
> +               pr_err("%s: error (%d)\n", __func__, ret);
> +
> +       for (i = 0; i < path->num_nodes; i++) {
> +               node = path->reqs[i].node;
> +               hlist_del(&path->reqs[i].req_node);
> +
> +               mutex_lock(&node->provider->lock);
> +               node->provider->users--;
> +               mutex_unlock(&node->provider->lock);
> +       }
> +
> +       kfree(path);
> +}
> +EXPORT_SYMBOL_GPL(icc_put);
> +
> +/**
> + * icc_node_create() - create a node
> + * @id: node id
> + *
> + * Return: icc_node pointer on success, or ERR_PTR() on error
> + */
> +struct icc_node *icc_node_create(int id)
> +{
> +       struct icc_node *node;
> +
> +       /* check if node already exists */
> +       node = node_find(id);
> +       if (node)
> +               return node;
> +
> +       node = kzalloc(sizeof(*node), GFP_KERNEL);
> +       if (!node)
> +               return ERR_PTR(-ENOMEM);
> +
> +       id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
> +       if (WARN(id < 0, "couldn't get idr"))
> +               return ERR_PTR(id);
> +
> +       node->id = id;
> +
> +       return node;
> +}
> +EXPORT_SYMBOL_GPL(icc_node_create);
> +
> +/**
> + * icc_link_create() - create a link between two nodes
> + * @src_id: source node id
> + * @dst_id: destination node id
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_link_create(struct icc_node *node, const int dst_id)
> +{
> +       struct icc_node *dst;
> +       struct icc_node **new;
> +       int ret = 0;
> +
> +       if (IS_ERR_OR_NULL(node))
> +               return PTR_ERR(node);
> +
> +       mutex_lock(&node->provider->lock);
> +
> +       dst = node_find(dst_id);
> +       if (!dst)
> +               dst = icc_node_create(dst_id);
> +
> +       new = krealloc(node->links,
> +                      (node->num_links + 1) * sizeof(*node->links),
> +                      GFP_KERNEL);
> +       if (!new) {
> +               ret = -ENOMEM;
> +               goto out;
> +       }
> +
> +       node->links = new;
> +       node->links[node->num_links++] = dst;
> +
> +out:
> +       mutex_unlock(&node->provider->lock);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_link_create);
> +
> +/**
> + * icc_add_node() - add an interconnect node to interconnect provider
> + * @node: pointer to the interconnect node
> + * @provider: pointer to the interconnect provider
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider)
> +{
> +       if (WARN_ON(!node))
> +               return -EINVAL;
> +
> +       if (WARN_ON(!provider))
> +               return -EINVAL;
> +
> +       node->provider = provider;
> +
> +       mutex_lock(&provider->lock);
> +       list_add_tail(&node->node_list, &provider->nodes);
> +       mutex_unlock(&provider->lock);
> +
> +       return 0;
> +}
> +
> +/**
> + * icc_add_provider() - add a new interconnect provider
> + * @icc_provider: the interconnect provider that will be added into topology
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_add_provider(struct icc_provider *provider)
> +{
> +       if (WARN_ON(!provider))
> +               return -EINVAL;
> +
> +       if (WARN_ON(!provider->set))
> +               return -EINVAL;
> +
> +       mutex_init(&provider->lock);
> +       INIT_LIST_HEAD(&provider->nodes);
> +
> +       mutex_lock(&icc_provider_list_mutex);
> +       list_add(&provider->provider_list, &icc_provider_list);
> +       mutex_unlock(&icc_provider_list_mutex);
> +
> +       dev_dbg(provider->dev, "interconnect provider added to topology\n");
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_add_provider);
> +
> +/**
> + * icc_del_provider() - delete previously added interconnect provider
> + * @icc_provider: the interconnect provider that will be removed from topology
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_del_provider(struct icc_provider *provider)
> +{
> +       mutex_lock(&provider->lock);
> +       if (provider->users) {
> +               pr_warn("interconnect provider still has %d users\n",
> +                       provider->users);
> +       }
> +       mutex_unlock(&provider->lock);
> +
> +       mutex_lock(&icc_provider_list_mutex);
> +       list_del(&provider->provider_list);
> +       mutex_unlock(&icc_provider_list_mutex);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_del_provider);
> +
> +MODULE_AUTHOR("Georgi Djakov <georgi.djakov@linaro.org");
> +MODULE_DESCRIPTION("Interconnect Driver Core");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h
> new file mode 100644
> index 000000000000..779b5b5b1306
> --- /dev/null
> +++ b/include/linux/interconnect-provider.h
> @@ -0,0 +1,109 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef _LINUX_INTERCONNECT_PROVIDER_H
> +#define _LINUX_INTERCONNECT_PROVIDER_H
> +
> +#include <linux/interconnect.h>
> +
> +struct icc_node;
> +
> +/**
> + * struct icc_provider - interconnect provider (controller) entity that might
> + * provide multiple interconnect controls
> + *
> + * @provider_list: list of the registered interconnect providers
> + * @nodes: internal list of the interconnect provider nodes
> + * @set: pointer to device specific set operation function
> + * @dev: the device this interconnect provider belongs to
> + * @lock: lock to provide consistency during aggregation/update of constraints
> + * @users: count of active users
> + * @data: pointer to private data
> + */
> +struct icc_provider {
> +       struct list_head        provider_list;
> +       struct list_head        nodes;
> +       int (*set)(struct icc_node *src, struct icc_node *dst,
> +                  u32 avg_bw, u32 peak_bw);
> +       struct device           *dev;
> +       struct mutex            lock;
> +       int                     users;
> +       void                    *data;
> +};
> +
> +/**
> + * struct icc_node - entity that is part of the interconnect topology
> + *
> + * @id: platform specific node id
> + * @name: node name used in debugfs
> + * @links: a list of targets where we can go next when traversing
> + * @num_links: number of links to other interconnect nodes
> + * @provider: points to the interconnect provider of this node
> + * @node_list: list of interconnect nodes associated with @provider
> + * @search_list: list used when walking the nodes graph
> + * @reverse: pointer to previous node when walking the nodes graph
> + * @is_traversed: flag that is used when walking the nodes graph
> + * @req_list: a list of QoS constraint requests associated with this node


> + * @avg_bw: aggregated value of average bandwidth
> + * @peak_bw: aggregated value of peak bandwidth

Consider changing to "aggregated value of {average|peak} bandwidth
requests from all consumers"

> + * @data: pointer to private data
> + */
> +struct icc_node {
> +       int                     id;
> +       const char              *name;
> +       struct icc_node         **links;
> +       size_t                  num_links;
> +
> +       struct icc_provider     *provider;
> +       struct list_head        node_list;
> +       struct list_head        orphan_list;
> +       struct list_head        search_list;
> +       struct icc_node         *reverse;
> +       bool                    is_traversed;
> +       struct hlist_head       req_list;
> +       u32                     avg_bw;
> +       u32                     peak_bw;
> +       void                    *data;
> +};
> +
> +#if IS_ENABLED(CONFIG_INTERCONNECT)
> +
> +struct icc_node *icc_node_create(int id);
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider);
> +int icc_link_create(struct icc_node *node, const int dst_id);
> +int icc_add_provider(struct icc_provider *provider);
> +int icc_del_provider(struct icc_provider *provider);
> +
> +#else
> +
> +static inline struct icc_node *icc_node_create(int id)
> +{
> +       return ERR_PTR(-ENOTSUPP);
> +}
> +
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider)
> +{
> +       return -ENOTSUPP;
> +}
> +
> +static inline int icc_link_create(struct icc_node *node, const int dst_id)
> +{
> +       return -ENOTSUPP;
> +}
> +
> +static inline int icc_add_provider(struct icc_provider *provider)
> +{
> +       return -ENOTSUPP;
> +}
> +
> +static inline int icc_del_provider(struct icc_provider *provider)
> +{
> +       return -ENOTSUPP;
> +}
> +
> +#endif /* CONFIG_INTERCONNECT */
> +
> +#endif /* _LINUX_INTERCONNECT_PROVIDER_H */
> diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
> new file mode 100644
> index 000000000000..5a7cf72b76a5
> --- /dev/null
> +++ b/include/linux/interconnect.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef _LINUX_INTERCONNECT_H
> +#define _LINUX_INTERCONNECT_H
> +
> +#include <linux/types.h>
> +#include <linux/mutex.h>
> +
> +struct icc_path;
> +struct device;
> +
> +#if IS_ENABLED(CONFIG_INTERCONNECT)
> +
> +struct icc_path *icc_get(const int src_id, const int dst_id);
> +void icc_put(struct icc_path *path);
> +int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw);
> +
> +#else
> +
> +static inline struct icc_path *icc_get(const int src_id, const int dst_id)
> +{
> +       return NULL;
> +}
> +
> +static inline void icc_put(struct icc_path *path)
> +{
> +}
> +
> +static inline int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> +{
> +       return 0;
> +}
> +
> +#endif /* CONFIG_INTERCONNECT */
> +
> +#endif /* _LINUX_INTERCONNECT_H */
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
From: Marc Zyngier @ 2018-05-25  8:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524171238.ni2hclcmhcfv7dpb@lakrids.cambridge.arm.com>

On 24/05/18 18:12, Mark Rutland wrote:
> On Thu, May 17, 2018 at 11:35:47AM +0100, Marc Zyngier wrote:
>> There is no need to perform cache maintenance operations when
>> creating the HYP page tables if we have the multiprocessing
>> extensions. ARMv7 mandates them with the virtualization support,
>> and ARMv8 just mandates them unconditionally.
>>
>> Let's remove these operations.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  virt/kvm/arm/mmu.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index ba66bf7ae299..acbfea09578c 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
>>  		pte = pte_offset_kernel(pmd, addr);
>>  		kvm_set_pte(pte, pfn_pte(pfn, prot));
>>  		get_page(virt_to_page(pte));
>> -		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
>>  		pfn++;
>>  	} while (addr += PAGE_SIZE, addr != end);
>>  }
>> @@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
>>  			}
>>  			pmd_populate_kernel(NULL, pmd, pte);
>>  			get_page(virt_to_page(pmd));
>> -			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
>>  		}
>>  
>>  		next = pmd_addr_end(addr, end);
>> @@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
>>  			}
>>  			pud_populate(NULL, pud, pmd);
>>  			get_page(virt_to_page(pud));
>> -			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
>>  		}
>>  
>>  		next = pud_addr_end(addr, end);
>> @@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  			}
>>  			pgd_populate(NULL, pgd, pud);
>>  			get_page(virt_to_page(pgd));
>> -			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
>>  		}
>>  
>>  		next = pgd_addr_end(addr, end);
>> @@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  		pfn += (next - addr) >> PAGE_SHIFT;
>>  	} while (addr = next, addr != end);
>>  out:
>> +	dsb(ishst);
> 
> I think you need a dsb(ishst) wherever you had a
> kvm_flush_dcache_to_poc() previously.
> 
> Otherwise, the page table walker could see stale values. e.g. after you
> update a bunch of PTE entries and point a PMD entry at those, the PTW
> could see the updated PMD entry, but see stale PTE entries, which could
> be garbage.

That's a very good point. I initially only considered the EL2
initialisation (the EL2 MMU is not live yet), but failed to consider the
additional mappings at runtime (each time we instantiate a VM).

> That said, I think for ensuring the *order* those become visible in, you
> only need a dmb(ishst), and the ensure they *are visible* you need a
> DSB(ISHST) at the end.
And we definitely want the latter.

I'll respin this shortly.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Jon Hunter @ 2018-05-25  8:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAPDyKFp3nMoUoM0LfxcSvJKthwrAKK7yCLAkZMGW-oapUsK7GA@mail.gmail.com>


On 24/05/18 22:21, Ulf Hansson wrote:

...

>> OK, so this bit is a to-do as that is not yet exposed AFAICT. I see that you
>> said 'although we need to extend it to cover cleanup of the earlier
>> registered device, via calling device_unregister().' So if we do this then
>> that would be fine.
> 
> Let me clarify the changelog. It's not a to-do, as it's already done
> as part of $subject patch.

Yes I see it now. OK, then that's fine.

Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH v7 2/2] ARM: dts: imx: Add basic dts support for imx6sll EVK board
From: A.s. Dong @ 2018-05-25  7:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234127-11991-2-git-send-email-ping.bai@nxp.com>

Hi Shawn,

> -----Original Message-----
> From: Jacky Bai
> Sent: Friday, May 25, 2018 3:42 PM
> To: shawnguo at kernel.org; robh+dt at kernel.org; kernel at pengutronix.de
> Cc: devicetree at vger.kernel.org; linux-arm-kernel at lists.infradead.org; dl-
> linux-imx <linux-imx@nxp.com>; A.s. Dong <aisheng.dong@nxp.com>;
> jacky.baip at gmail.com
> Subject: [PATCH v7 2/2] ARM: dts: imx: Add basic dts support for imx6sll EVK
> board
> 
> Add dts file support for imx6sll EVK board.
> 
> Signed-off-by: Bai Ping <ping.bai@nxp.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Dong Aisheng <Aisheng.dong@nxp.com>

Would you help pick this patch series?
It seems there's only DTS part left for MX6SLL support.

Regards
Dong Aisheng

^ permalink raw reply

* [PATCH v2 13/13] ARM: pxa: change SSP DMA channels allocation
From: Daniel Mack @ 2018-05-25  7:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524070703.11901-14-robert.jarzmik@free.fr>

On Thursday, May 24, 2018 09:07 AM, Robert Jarzmik wrote:
> Now the dma_slave_map is available for PXA architecture, switch the SSP
> device to it.
> 
> This specifically means that :
> - for platform data based machines, the DMA requestor channels are
>    extracted from the slave map, where pxa-ssp-dai.<N> is a 1-1 match to
>    ssp.<N>, and the channels are either "rx" or "tx".
> 
> - for device tree platforms, the dma node should be hooked into the
>    pxa2xx-ac97 or pxa-ssp-dai node.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

Acked-by: Daniel Mack <daniel@zonque.org>


We should, however, merge what's left of this management glue code into 
the users of it, so the dma related properties can be put in the right 
devicetree node.

I'll prepare a patch for that for 4.18. This is a good preparation for 
this round though.


Thanks,
Daniel


> ---
> Since v1: Removed channel names from platform_data
> ---
>   arch/arm/plat-pxa/ssp.c    | 47 ----------------------------------------------
>   include/linux/pxa2xx_ssp.h |  2 --
>   sound/soc/pxa/pxa-ssp.c    |  5 ++---
>   3 files changed, 2 insertions(+), 52 deletions(-)
> 
> diff --git a/arch/arm/plat-pxa/ssp.c b/arch/arm/plat-pxa/ssp.c
> index ba13f793fbce..ed36dcab80f1 100644
> --- a/arch/arm/plat-pxa/ssp.c
> +++ b/arch/arm/plat-pxa/ssp.c
> @@ -127,53 +127,6 @@ static int pxa_ssp_probe(struct platform_device *pdev)
>   	if (IS_ERR(ssp->clk))
>   		return PTR_ERR(ssp->clk);
>   
> -	if (dev->of_node) {
> -		struct of_phandle_args dma_spec;
> -		struct device_node *np = dev->of_node;
> -		int ret;
> -
> -		/*
> -		 * FIXME: we should allocate the DMA channel from this
> -		 * context and pass the channel down to the ssp users.
> -		 * For now, we lookup the rx and tx indices manually
> -		 */
> -
> -		/* rx */
> -		ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells",
> -						 0, &dma_spec);
> -
> -		if (ret) {
> -			dev_err(dev, "Can't parse dmas property\n");
> -			return -ENODEV;
> -		}
> -		ssp->drcmr_rx = dma_spec.args[0];
> -		of_node_put(dma_spec.np);
> -
> -		/* tx */
> -		ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells",
> -						 1, &dma_spec);
> -		if (ret) {
> -			dev_err(dev, "Can't parse dmas property\n");
> -			return -ENODEV;
> -		}
> -		ssp->drcmr_tx = dma_spec.args[0];
> -		of_node_put(dma_spec.np);
> -	} else {
> -		res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> -		if (res == NULL) {
> -			dev_err(dev, "no SSP RX DRCMR defined\n");
> -			return -ENODEV;
> -		}
> -		ssp->drcmr_rx = res->start;
> -
> -		res = platform_get_resource(pdev, IORESOURCE_DMA, 1);
> -		if (res == NULL) {
> -			dev_err(dev, "no SSP TX DRCMR defined\n");
> -			return -ENODEV;
> -		}
> -		ssp->drcmr_tx = res->start;
> -	}
> -
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	if (res == NULL) {
>   		dev_err(dev, "no memory resource defined\n");
> diff --git a/include/linux/pxa2xx_ssp.h b/include/linux/pxa2xx_ssp.h
> index 8461b18e4608..03a7ca46735b 100644
> --- a/include/linux/pxa2xx_ssp.h
> +++ b/include/linux/pxa2xx_ssp.h
> @@ -212,8 +212,6 @@ struct ssp_device {
>   	int		type;
>   	int		use_count;
>   	int		irq;
> -	int		drcmr_rx;
> -	int		drcmr_tx;
>   
>   	struct device_node	*of_node;
>   };
> diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
> index 0291c7cb64eb..e09368d89bbc 100644
> --- a/sound/soc/pxa/pxa-ssp.c
> +++ b/sound/soc/pxa/pxa-ssp.c
> @@ -104,9 +104,8 @@ static int pxa_ssp_startup(struct snd_pcm_substream *substream,
>   	dma = kzalloc(sizeof(struct snd_dmaengine_dai_dma_data), GFP_KERNEL);
>   	if (!dma)
>   		return -ENOMEM;
> -
> -	dma->filter_data = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ?
> -				&ssp->drcmr_tx : &ssp->drcmr_rx;
> +	dma->chan_name = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ?
> +		"tx" : "rx";
>   
>   	snd_soc_dai_set_dma_data(cpu_dai, substream, dma);
>   
> 

^ permalink raw reply

* [PATCH RESEND V4 9/9] clk: imx: add imx7ulp clk driver
From: Dong Aisheng @ 2018-05-25  7:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234671-31755-1-git-send-email-aisheng.dong@nxp.com>

i.MX7ULP Clock functions are under joint control of the System
Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
modules, and Core Mode Controller (CMC)1 blocks

The clocking scheme provides clear separation between M4 domain
and A7 domain. Except for a few clock sources shared between two
domains, such as the System Oscillator clock, the Slow IRC (SIRC),
and and the Fast IRC clock (FIRCLK), clock sources and clock
management are separated and contained within each domain.

M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.

This driver only adds clock support in A7 domain.

Note that most clocks required to be operated when gated, e.g. pll,
pfd, pcc. And more special cases that scs/ddr/nic mux selecting
different clock source requires that clock to be enabled first,
then we need set CLK_OPS_PARENT_ENABLE flag for them properly.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

---
ChangeLog:
v2->v3:
 * no changes
v1->v2:
 * use of_clk_add_hw_provider instead
 * split the clocks register process into two parts: early part for possible
   timers clocks registered by CLK_OF_DECLARE_DRIVER and the later part for
   the left normal peripheral clocks registered by a platform driver.
---
 drivers/clk/imx/Makefile      |   1 +
 drivers/clk/imx/clk-imx7ulp.c | 227 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 228 insertions(+)
 create mode 100644 drivers/clk/imx/clk-imx7ulp.c

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index f4da12c..983c0a5 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -29,4 +29,5 @@ obj-$(CONFIG_SOC_IMX6SLL) += clk-imx6sll.o
 obj-$(CONFIG_SOC_IMX6SX) += clk-imx6sx.o
 obj-$(CONFIG_SOC_IMX6UL) += clk-imx6ul.o
 obj-$(CONFIG_SOC_IMX7D)  += clk-imx7d.o
+obj-$(CONFIG_SOC_IMX7ULP) += clk-imx7ulp.o
 obj-$(CONFIG_SOC_VF610)  += clk-vf610.o
diff --git a/drivers/clk/imx/clk-imx7ulp.c b/drivers/clk/imx/clk-imx7ulp.c
new file mode 100644
index 0000000..bbeaa0f
--- /dev/null
+++ b/drivers/clk/imx/clk-imx7ulp.c
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017~2018 NXP
+ *
+ * Author: Dong Aisheng <aisheng.dong@nxp.com>
+ *
+ */
+
+#include <dt-bindings/clock/imx7ulp-clock.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "clk.h"
+
+static const char * const pll_pre_sels[]	= { "sosc", "firc", };
+static const char * const spll_pfd_sels[]	= { "spll_pfd0", "spll_pfd1", "spll_pfd2", "spll_pfd3", };
+static const char * const spll_sels[]		= { "spll", "spll_pfd_sel", };
+static const char * const apll_pfd_sels[]	= { "apll_pfd0", "apll_pfd1", "apll_pfd2", "apll_pfd3", };
+static const char * const apll_sels[]		= { "apll", "apll_pfd_sel", };
+static const char * const scs_sels[]		= { "dummy", "sosc", "sirc", "firc", "dummy", "apll_sel", "spll_sel", "upll", };
+static const char * const ddr_sels[]		= { "apll_pfd_sel", "upll", };
+static const char * const nic_sels[]		= { "firc", "ddr_clk", };
+static const char * const periph_plat_sels[]	= { "dummy", "nic1_bus_clk", "nic1_clk", "ddr_clk", "apll_pfd2", "apll_pfd1", "apll_pfd0", "upll", };
+static const char * const periph_bus_sels[]	= { "dummy", "sosc_bus_clk", "mpll", "firc_bus_clk", "rosc", "nic1_bus_clk", "nic1_clk", "spll_bus_clk", };
+
+static struct clk_hw_onecell_data *clk_data;
+
+static void __init imx7ulp_clocks_early_init(struct device_node *scg_node)
+{
+	struct device_node *np = scg_node;
+	void __iomem *base;
+	struct clk_hw **clks;
+
+	clk_data = kzalloc(sizeof(*clk_data) +
+			   sizeof(*clk_data->hws) * IMX7ULP_CLK_END,
+			   GFP_KERNEL);
+	if (!clk_data)
+		return;
+
+	clk_data->num = IMX7ULP_CLK_END;
+	clks = clk_data->hws;
+
+	clks[IMX7ULP_CLK_DUMMY]		= imx_clk_hw_fixed("dummy", 0);
+
+	clks[IMX7ULP_CLK_ROSC]		= imx_obtain_fixed_clk_hw(np, "rosc");
+	clks[IMX7ULP_CLK_SOSC]		= imx_obtain_fixed_clk_hw(np, "sosc");
+	clks[IMX7ULP_CLK_SIRC]		= imx_obtain_fixed_clk_hw(np, "sirc");
+	clks[IMX7ULP_CLK_FIRC]		= imx_obtain_fixed_clk_hw(np, "firc");
+	clks[IMX7ULP_CLK_MIPI_PLL]	= imx_obtain_fixed_clk_hw(np, "mpll");
+	clks[IMX7ULP_CLK_UPLL]		= imx_obtain_fixed_clk_hw(np, "upll");
+
+	/* SCG1 */
+	base = of_iomap(np, 0);
+	WARN_ON(!base);
+
+	/* NOTE: xPLL config can't be changed when xPLL is enabled */
+	clks[IMX7ULP_CLK_APLL_PRE_SEL]	= imx_clk_hw_mux_flags("apll_pre_sel", base + 0x508, 0, 1, pll_pre_sels, ARRAY_SIZE(pll_pre_sels), CLK_SET_PARENT_GATE);
+	clks[IMX7ULP_CLK_SPLL_PRE_SEL]	= imx_clk_hw_mux_flags("spll_pre_sel", base + 0x608, 0, 1, pll_pre_sels, ARRAY_SIZE(pll_pre_sels), CLK_SET_PARENT_GATE);
+
+	/*							   name		    parent_name	   reg			shift	width	flags */
+	clks[IMX7ULP_CLK_APLL_PRE_DIV]	= imx_clk_hw_divider_flags("apll_pre_div", "apll_pre_sel", base + 0x508,	8,	3,	CLK_SET_RATE_GATE);
+	clks[IMX7ULP_CLK_SPLL_PRE_DIV]	= imx_clk_hw_divider_flags("spll_pre_div", "spll_pre_sel", base + 0x608,	8,	3,	CLK_SET_RATE_GATE);
+
+	/*						name	 parent_name	 base */
+	clks[IMX7ULP_CLK_APLL]		= imx_clk_pllv4("apll",  "apll_pre_div", base + 0x500);
+	clks[IMX7ULP_CLK_SPLL]		= imx_clk_pllv4("spll",  "spll_pre_div", base + 0x600);
+
+	/* APLL PFDs */
+	clks[IMX7ULP_CLK_APLL_PFD0]	= imx_clk_pfdv2("apll_pfd0", "apll", base + 0x50c, 0);
+	clks[IMX7ULP_CLK_APLL_PFD1]	= imx_clk_pfdv2("apll_pfd1", "apll", base + 0x50c, 1);
+	clks[IMX7ULP_CLK_APLL_PFD2]	= imx_clk_pfdv2("apll_pfd2", "apll", base + 0x50c, 2);
+	clks[IMX7ULP_CLK_APLL_PFD3]	= imx_clk_pfdv2("apll_pfd3", "apll", base + 0x50c, 3);
+
+	/* SPLL PFDs */
+	clks[IMX7ULP_CLK_SPLL_PFD0]	= imx_clk_pfdv2("spll_pfd0", "spll", base + 0x60C, 0);
+	clks[IMX7ULP_CLK_SPLL_PFD1]	= imx_clk_pfdv2("spll_pfd1", "spll", base + 0x60C, 1);
+	clks[IMX7ULP_CLK_SPLL_PFD2]	= imx_clk_pfdv2("spll_pfd2", "spll", base + 0x60C, 2);
+	clks[IMX7ULP_CLK_SPLL_PFD3]	= imx_clk_pfdv2("spll_pfd3", "spll", base + 0x60C, 3);
+
+	/* PLL Mux */
+	clks[IMX7ULP_CLK_APLL_PFD_SEL]	= imx_clk_hw_mux_flags("apll_pfd_sel", base + 0x508, 14, 2, apll_pfd_sels, ARRAY_SIZE(apll_pfd_sels), CLK_SET_RATE_PARENT | CLK_SET_PARENT_GATE);
+	clks[IMX7ULP_CLK_SPLL_PFD_SEL]	= imx_clk_hw_mux_flags("spll_pfd_sel", base + 0x608, 14, 2, spll_pfd_sels, ARRAY_SIZE(spll_pfd_sels), CLK_SET_RATE_PARENT | CLK_SET_PARENT_GATE);
+	clks[IMX7ULP_CLK_APLL_SEL]	= imx_clk_hw_mux_flags("apll_sel", base + 0x508, 1, 1, apll_sels, ARRAY_SIZE(apll_sels), CLK_SET_RATE_PARENT | CLK_SET_PARENT_GATE);
+	clks[IMX7ULP_CLK_SPLL_SEL]	= imx_clk_hw_mux_flags("spll_sel", base + 0x608, 1, 1, spll_sels, ARRAY_SIZE(spll_sels), CLK_SET_RATE_PARENT | CLK_SET_PARENT_GATE);
+
+	clks[IMX7ULP_CLK_SPLL_BUS_CLK]	= clk_hw_register_divider(NULL, "spll_bus_clk", "spll_sel", CLK_SET_RATE_GATE, base + 0x604, 8, 3, CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ZERO_GATE, &imx_ccm_lock);
+
+	/* scs/ddr/nic select different clock source requires that clock to be enabled first */
+	clks[IMX7ULP_CLK_SYS_SEL]	= imx_clk_hw_mux2("scs_sel", base + 0x14, 24, 4, scs_sels, ARRAY_SIZE(scs_sels));
+	clks[IMX7ULP_CLK_NIC_SEL]	= imx_clk_hw_mux2("nic_sel", base + 0x40, 28, 1, nic_sels, ARRAY_SIZE(nic_sels));
+	clks[IMX7ULP_CLK_DDR_SEL]	= imx_clk_hw_mux_flags("ddr_sel", base + 0x30, 24, 1, ddr_sels, ARRAY_SIZE(ddr_sels), CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE);
+
+	clks[IMX7ULP_CLK_CORE_DIV]	= imx_clk_hw_divider_flags("divcore",	"scs_sel",  base + 0x14, 16, 4, CLK_SET_RATE_PARENT | CLK_IS_CRITICAL);
+
+	clks[IMX7ULP_CLK_DDR_DIV]	= clk_hw_register_divider(NULL, "ddr_clk", "ddr_sel", CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, base + 0x30, 0, 3,
+								  CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ZERO_GATE, &imx_ccm_lock);
+
+	clks[IMX7ULP_CLK_NIC0_DIV]	= imx_clk_hw_divider_flags("nic0_clk",		"nic_sel",  base + 0x40, 24, 4, CLK_SET_RATE_PARENT | CLK_IS_CRITICAL);
+	clks[IMX7ULP_CLK_NIC1_DIV]	= imx_clk_hw_divider_flags("nic1_clk",		"nic0_clk", base + 0x40, 16, 4, CLK_SET_RATE_PARENT | CLK_IS_CRITICAL);
+	clks[IMX7ULP_CLK_NIC1_BUS_DIV]	= imx_clk_hw_divider_flags("nic1_bus_clk",	"nic1_clk", base + 0x40, 4,  4, CLK_SET_RATE_PARENT | CLK_IS_CRITICAL);
+
+	clks[IMX7ULP_CLK_SOSC_BUS_CLK]	= clk_hw_register_divider(NULL, "sosc_bus_clk", "sosc", 0, base + 0x104, 8, 3,
+								  CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ZERO_GATE, &imx_ccm_lock);
+	clks[IMX7ULP_CLK_FIRC_BUS_CLK]	= clk_hw_register_divider(NULL, "firc_bus_clk", "firc", 0, base + 0x304, 8, 3,
+								  CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ZERO_GATE, &imx_ccm_lock);
+
+	/* PCC2 */
+	base = of_iomap(np, 1);
+	WARN_ON(!base);
+
+	clks[IMX7ULP_CLK_LPTPM4]	= imx_clk_composite("lptpm4",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x94);
+	clks[IMX7ULP_CLK_LPTPM5]	= imx_clk_composite("lptpm5",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x98);
+	clks[IMX7ULP_CLK_LPIT1]		= imx_clk_composite("lpit1",   periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x9c);
+
+	/* PCC3 */
+	base = of_iomap(np, 2);
+	WARN_ON(!base);
+
+	clks[IMX7ULP_CLK_LPTPM6]	= imx_clk_composite("lptpm6",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x84);
+	clks[IMX7ULP_CLK_LPTPM7]	= imx_clk_composite("lptpm7",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x88);
+
+	clks[IMX7ULP_CLK_MMDC]		= clk_hw_register_gate(NULL, "mmdc", "nic1_clk", CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+							       base + 0xac, 30, 0, &imx_ccm_lock);
+
+	imx_check_clk_hws(clks, clk_data->num);
+
+	of_clk_add_hw_provider(scg_node, of_clk_hw_onecell_get, clk_data);
+}
+CLK_OF_DECLARE_DRIVER(imx7ulp, "fsl,imx7ulp-clock", imx7ulp_clocks_early_init);
+
+static const struct of_device_id imx7ulp_clk_dt_ids[] = {
+	{ .compatible = "fsl,imx7ulp-clock" },
+	{ /* sentinel */ }
+};
+
+static int imx7ulp_clk_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct clk_hw **clks = clk_data->hws;
+	struct resource *res;
+	void __iomem *base;
+
+	/* PCC2 */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "scg1");
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	clks[IMX7ULP_CLK_GPU_DIV]	= imx_clk_hw_divider("gpu_clk", "nic0_clk", base + 0x40, 20, 4);
+
+	/* PCC2 */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pcc2");
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	clks[IMX7ULP_CLK_DMA1]		= imx_clk_hw_gate("dma1", "nic1_clk", base + 0x20, 30);
+	clks[IMX7ULP_CLK_RGPIO2P1]	= imx_clk_hw_gate("rgpio2p1", "nic1_bus_clk", base + 0x3c, 30);
+	clks[IMX7ULP_CLK_DMA_MUX1]	= imx_clk_hw_gate("dma_mux1", "nic1_bus_clk", base + 0x84, 30);
+	clks[IMX7ULP_CLK_SNVS]		= imx_clk_hw_gate("snvs", "nic1_bus_clk", base + 0x8c, 30);
+	clks[IMX7ULP_CLK_CAAM]		= imx_clk_hw_gate("caam", "nic1_clk", base + 0x90, 30);
+	clks[IMX7ULP_CLK_LPSPI2]	= imx_clk_composite("lpspi2",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xa4);
+	clks[IMX7ULP_CLK_LPSPI3]	= imx_clk_composite("lpspi3",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xa8);
+	clks[IMX7ULP_CLK_LPI2C4]	= imx_clk_composite("lpi2c4",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xac);
+	clks[IMX7ULP_CLK_LPI2C5]	= imx_clk_composite("lpi2c5",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xb0);
+	clks[IMX7ULP_CLK_LPUART4]	= imx_clk_composite("lpuart4", periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xb4);
+	clks[IMX7ULP_CLK_LPUART5]	= imx_clk_composite("lpuart5", periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xb8);
+	clks[IMX7ULP_CLK_FLEXIO1]	= imx_clk_composite("flexio1", periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xc4);
+	clks[IMX7ULP_CLK_USB0]		= imx_clk_composite("usb0",    periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, true,  true, base + 0xcc);
+	clks[IMX7ULP_CLK_USB1]		= imx_clk_composite("usb1",    periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, true,  true, base + 0xd0);
+	clks[IMX7ULP_CLK_USB_PHY]	= imx_clk_hw_gate("usb_phy", "nic1_bus_clk", base + 0xD4, 30);
+	clks[IMX7ULP_CLK_USDHC0]	= imx_clk_composite("usdhc0",  periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, true,  true, base + 0xdc);
+	clks[IMX7ULP_CLK_USDHC1]	= imx_clk_composite("usdhc1",  periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, true,  true, base + 0xe0);
+	clks[IMX7ULP_CLK_WDG1]		= imx_clk_composite("wdg1",    periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, true,  true, base + 0xf4);
+	clks[IMX7ULP_CLK_WDG2]		= imx_clk_composite("sdg2",    periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, true,  true, base + 0x10c);
+
+	/* PCC3 */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pcc3");
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	clks[IMX7ULP_CLK_LPI2C6]	= imx_clk_composite("lpi2c6",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x90);
+	clks[IMX7ULP_CLK_LPI2C7]	= imx_clk_composite("lpi2c7",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x94);
+	clks[IMX7ULP_CLK_LPUART6]	= imx_clk_composite("lpuart6", periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x98);
+	clks[IMX7ULP_CLK_LPUART7]	= imx_clk_composite("lpuart7", periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x9c);
+	clks[IMX7ULP_CLK_DSI]		= imx_clk_composite("dsi",     periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, true,  true, base + 0xa4);
+	clks[IMX7ULP_CLK_LCDIF]		= imx_clk_composite("lcdif",   periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, true,  true, base + 0xa8);
+
+	clks[IMX7ULP_CLK_VIU]		= imx_clk_hw_gate("viu",   "nic1_clk",	   base + 0xa0, 30);
+	clks[IMX7ULP_CLK_PCTLC]		= imx_clk_hw_gate("pctlc", "nic1_bus_clk", base + 0xb8, 30);
+	clks[IMX7ULP_CLK_PCTLD]		= imx_clk_hw_gate("pctld", "nic1_bus_clk", base + 0xbc, 30);
+	clks[IMX7ULP_CLK_PCTLE]		= imx_clk_hw_gate("pctle", "nic1_bus_clk", base + 0xc0, 30);
+	clks[IMX7ULP_CLK_PCTLF]		= imx_clk_hw_gate("pctlf", "nic1_bus_clk", base + 0xc4, 30);
+
+	clks[IMX7ULP_CLK_GPU3D]		= imx_clk_composite("gpu3d",   periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x140);
+	clks[IMX7ULP_CLK_GPU2D]		= imx_clk_composite("gpu2d",   periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x144);
+
+	imx_check_clk_hws(clks, clk_data->num);
+
+	of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_data);
+
+	pr_debug("i.MX7ULP clock tree init done.\n");
+
+	return 0;
+}
+
+static struct platform_driver imx7ulp_clk_driver = {
+	.driver = {
+		.name = "imx7ulp-clock",
+		.of_match_table = imx7ulp_clk_dt_ids,
+	},
+	.probe = imx7ulp_clk_probe,
+};
+
+static int __init imx7ulp_clk_init(void)
+{
+	return platform_driver_register(&imx7ulp_clk_driver);
+}
+core_initcall(imx7ulp_clk_init);
-- 
2.7.4

^ permalink raw reply related

* [PATCH RESEND V4 8/9] clk: imx: implement new clk_hw based APIs
From: Dong Aisheng @ 2018-05-25  7:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234671-31755-1-git-send-email-aisheng.dong@nxp.com>

Clock providers are recommended to use the new struct clk_hw based API,
so implement IMX clk_hw based provider helpers functions to the new
approach.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

---
ChangeLog:
v2->v3:
 * no changes
v1->v2:  new patches
---
 drivers/clk/imx/clk.c | 22 ++++++++++++++++++
 drivers/clk/imx/clk.h | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c
index 9074e69..1efed86 100644
--- a/drivers/clk/imx/clk.c
+++ b/drivers/clk/imx/clk.c
@@ -18,6 +18,16 @@ void __init imx_check_clocks(struct clk *clks[], unsigned int count)
 			       i, PTR_ERR(clks[i]));
 }
 
+void imx_check_clk_hws(struct clk_hw *clks[], unsigned int count)
+{
+	unsigned int i;
+
+	for (i = 0; i < count; i++)
+		if (IS_ERR(clks[i]))
+			pr_err("i.MX clk %u: register failed with %ld\n",
+			       i, PTR_ERR(clks[i]));
+}
+
 static struct clk * __init imx_obtain_fixed_clock_from_dt(const char *name)
 {
 	struct of_phandle_args phandle;
@@ -49,6 +59,18 @@ struct clk * __init imx_obtain_fixed_clock(
 	return clk;
 }
 
+struct clk_hw * __init imx_obtain_fixed_clk_hw(struct device_node *np,
+					       const char *name)
+{
+	struct clk *clk;
+
+	clk = of_clk_get_by_name(np, name);
+	if (IS_ERR(clk))
+		return ERR_PTR(-ENOENT);
+
+	return __clk_get_hw(clk);
+}
+
 /*
  * This fixups the register CCM_CSCMR1 write value.
  * The write/read/divider values of the aclk_podf field
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 7fca912..d3fcaa5 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -8,6 +8,7 @@
 extern spinlock_t imx_ccm_lock;
 
 void imx_check_clocks(struct clk *clks[], unsigned int count);
+void imx_check_clk_hws(struct clk_hw *clks[], unsigned int count);
 void imx_register_uart_clocks(struct clk ** const clks[]);
 
 extern void imx_cscmr1_fixup(u32 *val);
@@ -54,6 +55,9 @@ struct clk *clk_register_gate2(struct device *dev, const char *name,
 struct clk * imx_obtain_fixed_clock(
 			const char *name, unsigned long rate);
 
+struct clk_hw *imx_obtain_fixed_clk_hw(struct device_node *np,
+				       const char *name);
+
 struct clk *imx_clk_gate_exclusive(const char *name, const char *parent,
 	 void __iomem *reg, u8 shift, u32 exclusive_mask);
 
@@ -90,6 +94,16 @@ static inline struct clk *imx_clk_fixed(const char *name, int rate)
 	return clk_register_fixed_rate(NULL, name, NULL, 0, rate);
 }
 
+static inline struct clk_hw *imx_clk_hw_fixed(const char *name, int rate)
+{
+	return clk_hw_register_fixed_rate(NULL, name, NULL, 0, rate);
+}
+
+static inline struct clk_hw *imx_get_clk_hw_fixed(const char *name, int rate)
+{
+	return clk_hw_register_fixed_rate(NULL, name, NULL, 0, rate);
+}
+
 static inline struct clk *imx_clk_mux_ldb(const char *name, void __iomem *reg,
 			u8 shift, u8 width, const char * const *parents,
 			int num_parents)
@@ -113,6 +127,15 @@ static inline struct clk *imx_clk_divider(const char *name, const char *parent,
 			reg, shift, width, 0, &imx_ccm_lock);
 }
 
+static inline struct clk_hw *imx_clk_hw_divider(const char *name,
+						const char *parent,
+						void __iomem *reg, u8 shift,
+						u8 width)
+{
+	return clk_hw_register_divider(NULL, name, parent, CLK_SET_RATE_PARENT,
+				       reg, shift, width, 0, &imx_ccm_lock);
+}
+
 static inline struct clk *imx_clk_divider_flags(const char *name,
 		const char *parent, void __iomem *reg, u8 shift, u8 width,
 		unsigned long flags)
@@ -121,6 +144,15 @@ static inline struct clk *imx_clk_divider_flags(const char *name,
 			reg, shift, width, 0, &imx_ccm_lock);
 }
 
+static inline struct clk_hw *imx_clk_hw_divider_flags(const char *name,
+						   const char *parent,
+						   void __iomem *reg, u8 shift,
+						   u8 width, unsigned long flags)
+{
+	return clk_hw_register_divider(NULL, name, parent, flags,
+				       reg, shift, width, 0, &imx_ccm_lock);
+}
+
 static inline struct clk *imx_clk_divider2(const char *name, const char *parent,
 		void __iomem *reg, u8 shift, u8 width)
 {
@@ -143,6 +175,13 @@ static inline struct clk *imx_clk_gate_flags(const char *name, const char *paren
 			shift, 0, &imx_ccm_lock);
 }
 
+static inline struct clk_hw *imx_clk_hw_gate(const char *name, const char *parent,
+					     void __iomem *reg, u8 shift)
+{
+	return clk_hw_register_gate(NULL, name, parent, CLK_SET_RATE_PARENT, reg,
+				    shift, 0, &imx_ccm_lock);
+}
+
 static inline struct clk *imx_clk_gate_dis(const char *name, const char *parent,
 		void __iomem *reg, u8 shift)
 {
@@ -222,6 +261,17 @@ static inline struct clk *imx_clk_mux2(const char *name, void __iomem *reg,
 			reg, shift, width, 0, &imx_ccm_lock);
 }
 
+static inline struct clk_hw *imx_clk_hw_mux2(const char *name, void __iomem *reg,
+					     u8 shift, u8 width,
+					     const char * const *parents,
+					     int num_parents)
+{
+	return clk_hw_register_mux(NULL, name, parents, num_parents,
+				   CLK_SET_RATE_NO_REPARENT |
+				   CLK_OPS_PARENT_ENABLE,
+				   reg, shift, width, 0, &imx_ccm_lock);
+}
+
 static inline struct clk *imx_clk_mux_flags(const char *name,
 			void __iomem *reg, u8 shift, u8 width,
 			const char * const *parents, int num_parents,
@@ -232,6 +282,18 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
 			&imx_ccm_lock);
 }
 
+static inline struct clk_hw *imx_clk_hw_mux_flags(const char *name,
+						  void __iomem *reg, u8 shift,
+						  u8 width,
+						  const char * const *parents,
+						  int num_parents,
+						  unsigned long flags)
+{
+	return clk_hw_register_mux(NULL, name, parents, num_parents,
+				   flags | CLK_SET_RATE_NO_REPARENT,
+				   reg, shift, width, 0, &imx_ccm_lock);
+}
+
 struct clk *imx_clk_cpu(const char *name, const char *parent_name,
 		struct clk *div, struct clk *mux, struct clk *pll,
 		struct clk *step);
-- 
2.7.4

^ permalink raw reply related

* [PATCH RESEND V4 7/9] clk: imx: make mux parent strings const
From: Dong Aisheng @ 2018-05-25  7:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234671-31755-1-git-send-email-aisheng.dong@nxp.com>

As the commit 2893c379461a ("clk: make strings in parent name arrays
const"), let's make the parent strings const, otherwise we may meet
the following warning when compiling:

drivers/clk/imx/clk-imx7ulp.c: In function 'imx7ulp_clocks_init':
drivers/clk/imx/clk-imx7ulp.c:73:35: warning: passing argument 5 of
	'imx_clk_mux_flags' discards 'const' qualifier from pointer target type

  clks[IMX7ULP_CLK_APLL_PRE_SEL] = imx_clk_mux_flags("apll_pre_sel", base + 0x508, 0,
	1, pll_pre_sels, ARRAY_SIZE(pll_pre_sels), CLK_SET_PARENT_GATE);
                                   ^
In file included from drivers/clk/imx/clk-imx7ulp.c:23:0:
drivers/clk/imx/clk.h:200:27: note: expected 'const char **' but argument is
 of type 'const char * const*'
...

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

---
ChangeLog:
v1->v3: no changes
---
 drivers/clk/imx/clk-busy.c      |  2 +-
 drivers/clk/imx/clk-fixup-mux.c |  2 +-
 drivers/clk/imx/clk.h           | 18 +++++++++++-------
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/imx/clk-busy.c b/drivers/clk/imx/clk-busy.c
index 9903652..e695622 100644
--- a/drivers/clk/imx/clk-busy.c
+++ b/drivers/clk/imx/clk-busy.c
@@ -154,7 +154,7 @@ static const struct clk_ops clk_busy_mux_ops = {
 
 struct clk *imx_clk_busy_mux(const char *name, void __iomem *reg, u8 shift,
 			     u8 width, void __iomem *busy_reg, u8 busy_shift,
-			     const char **parent_names, int num_parents)
+			     const char * const *parent_names, int num_parents)
 {
 	struct clk_busy_mux *busy;
 	struct clk *clk;
diff --git a/drivers/clk/imx/clk-fixup-mux.c b/drivers/clk/imx/clk-fixup-mux.c
index c9b327e..44817c1 100644
--- a/drivers/clk/imx/clk-fixup-mux.c
+++ b/drivers/clk/imx/clk-fixup-mux.c
@@ -70,7 +70,7 @@ static const struct clk_ops clk_fixup_mux_ops = {
 };
 
 struct clk *imx_clk_fixup_mux(const char *name, void __iomem *reg,
-			      u8 shift, u8 width, const char **parents,
+			      u8 shift, u8 width, const char * const *parents,
 			      int num_parents, void (*fixup)(u32 *val))
 {
 	struct clk_fixup_mux *fixup_mux;
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index bc43f68..7fca912 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -69,7 +69,7 @@ struct clk *imx_clk_busy_divider(const char *name, const char *parent_name,
 
 struct clk *imx_clk_busy_mux(const char *name, void __iomem *reg, u8 shift,
 			     u8 width, void __iomem *busy_reg, u8 busy_shift,
-			     const char **parent_names, int num_parents);
+			     const char * const *parent_names, int num_parents);
 
 struct clk_hw *imx_clk_composite(const char *name,
 				 const char * const *parent_names,
@@ -82,7 +82,7 @@ struct clk *imx_clk_fixup_divider(const char *name, const char *parent,
 				  void (*fixup)(u32 *val));
 
 struct clk *imx_clk_fixup_mux(const char *name, void __iomem *reg,
-			      u8 shift, u8 width, const char **parents,
+			      u8 shift, u8 width, const char * const *parents,
 			      int num_parents, void (*fixup)(u32 *val));
 
 static inline struct clk *imx_clk_fixed(const char *name, int rate)
@@ -91,7 +91,8 @@ static inline struct clk *imx_clk_fixed(const char *name, int rate)
 }
 
 static inline struct clk *imx_clk_mux_ldb(const char *name, void __iomem *reg,
-		u8 shift, u8 width, const char **parents, int num_parents)
+			u8 shift, u8 width, const char * const *parents,
+			int num_parents)
 {
 	return clk_register_mux(NULL, name, parents, num_parents,
 			CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT, reg,
@@ -204,7 +205,8 @@ static inline struct clk *imx_clk_gate4(const char *name, const char *parent,
 }
 
 static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg,
-		u8 shift, u8 width, const char **parents, int num_parents)
+			u8 shift, u8 width, const char * const *parents,
+			int num_parents)
 {
 	return clk_register_mux(NULL, name, parents, num_parents,
 			CLK_SET_RATE_NO_REPARENT, reg, shift,
@@ -212,7 +214,8 @@ static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg,
 }
 
 static inline struct clk *imx_clk_mux2(const char *name, void __iomem *reg,
-		u8 shift, u8 width, const char **parents, int num_parents)
+			u8 shift, u8 width, const char * const *parents,
+			int num_parents)
 {
 	return clk_register_mux(NULL, name, parents, num_parents,
 			CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
@@ -220,8 +223,9 @@ static inline struct clk *imx_clk_mux2(const char *name, void __iomem *reg,
 }
 
 static inline struct clk *imx_clk_mux_flags(const char *name,
-		void __iomem *reg, u8 shift, u8 width, const char **parents,
-		int num_parents, unsigned long flags)
+			void __iomem *reg, u8 shift, u8 width,
+			const char * const *parents, int num_parents,
+			unsigned long flags)
 {
 	return clk_register_mux(NULL, name, parents, num_parents,
 			flags | CLK_SET_RATE_NO_REPARENT, reg, shift, width, 0,
-- 
2.7.4

^ permalink raw reply related

* [PATCH RESEND V4 6/9] dt-bindings: clock: add imx7ulp clock binding doc
From: Dong Aisheng @ 2018-05-25  7:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234671-31755-1-git-send-email-aisheng.dong@nxp.com>

i.MX7ULP Clock functions are under joint control of the System
Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
modules, and Core Mode Controller (CMC)1 blocks

Note IMX7ULP has two clock domains: M4 and A7. This binding doc
is only for A7 clock domain.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Cc: devicetree at vger.kernel.org
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

---
ChangeLog:
v2->v3:
 * no changes
v1->v2: no changes
---
 .../devicetree/bindings/clock/imx7ulp-clock.txt    |  62 ++++++++++++
 include/dt-bindings/clock/imx7ulp-clock.h          | 105 +++++++++++++++++++++
 2 files changed, 167 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
 create mode 100644 include/dt-bindings/clock/imx7ulp-clock.h

diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
new file mode 100644
index 0000000..76ea3c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
@@ -0,0 +1,62 @@
+* Clock bindings for Freescale i.MX7ULP
+
+i.MX7ULP Clock functions are under joint control of the System
+Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
+modules, and Core Mode Controller (CMC)1 blocks
+
+The clocking scheme provides clear separation between M4 domain
+and A7 domain. Except for a few clock sources shared between two
+domains, such as the System Oscillator clock, the Slow IRC (SIRC),
+and and the Fast IRC clock (FIRCLK), clock sources and clock
+management are separated and contained within each domain.
+
+M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
+A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.
+
+Note: this binding doc is only for A7 clock domain.
+
+Required properties:
+
+- compatible:	Should be "fsl,imx7ulp-clock".
+- reg : 	Should contain registers location and length for scg1,
+		pcc2 and pcc3.
+- reg-names:	Should contain the according reg names "scg1", "pcc2"
+		and "pcc3".
+- #clock-cells:	Should be <1>.
+- clocks:	Should contain the fixed input clocks.
+- clock-name:   Should contain the following clock names:"rsoc", "sosc",
+		"sirc", "firc", "upll", "mpll".
+
+The clock consumer should specify the desired clock by having the clock
+ID in its "clocks" phandle cell.
+See include/dt-bindings/clock/imx7ulp-clock.h
+for the full list of i.MX7ULP clock IDs.
+
+Examples:
+
+#include <dt-bindings/clock/imx7ulp-clock.h>
+
+clks: scg1 at 403e0000 {
+	compatible = "fsl,imx7ulp-clock";
+	reg = <0x403e0000 0x10000>
+	      <0x403f0000 0x10000>
+	      <0x40b30000 0x10000>;
+	reg-names = "scg1", "pcc2", "pcc3";
+	clocks = <&rsoc>, <&sosc>, <&sirc>,
+		 <&firc>, <&upll>, <&mpll>;
+	clock-names = "rsoc", "sosc", "sirc",
+		      "firc", "upll", "mpll";
+	#clock-cells = <1>;
+};
+
+usdhc1: usdhc at 40380000 {
+	compatible = "fsl,imx7ulp-usdhc";
+	reg = <0x40380000 0x10000>;
+	interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&clks IMX7ULP_CLK_NIC1_BUS_DIV>,
+		 <&clks IMX7ULP_CLK_NIC1_DIV>,
+		 <&clks IMX7ULP_CLK_USDHC1>;
+	clock-names ="ipg", "ahb", "per";
+	bus-width = <4>;
+	status = "disabled";
+};
diff --git a/include/dt-bindings/clock/imx7ulp-clock.h b/include/dt-bindings/clock/imx7ulp-clock.h
new file mode 100644
index 0000000..2b6a29d
--- /dev/null
+++ b/include/dt-bindings/clock/imx7ulp-clock.h
@@ -0,0 +1,105 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017~2018 NXP
+ *
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_IMX7ULP_H
+#define __DT_BINDINGS_CLOCK_IMX7ULP_H
+
+#define IMX7ULP_CLK_DUMMY		0
+#define IMX7ULP_CLK_ROSC		1
+#define IMX7ULP_CLK_SOSC		2
+#define IMX7ULP_CLK_FIRC		3
+
+/* SCG1 */
+#define IMX7ULP_CLK_SPLL_PRE_SEL	4
+#define IMX7ULP_CLK_SPLL_PRE_DIV	5
+#define IMX7ULP_CLK_SPLL		6
+#define IMX7ULP_CLK_SPLL_POST_DIV1	7
+#define IMX7ULP_CLK_SPLL_POST_DIV2	8
+#define IMX7ULP_CLK_SPLL_PFD0		9
+#define IMX7ULP_CLK_SPLL_PFD1		10
+#define IMX7ULP_CLK_SPLL_PFD2		11
+#define IMX7ULP_CLK_SPLL_PFD3		12
+#define IMX7ULP_CLK_SPLL_PFD_SEL	13
+#define IMX7ULP_CLK_SPLL_SEL		14
+#define IMX7ULP_CLK_APLL_PRE_SEL	15
+#define IMX7ULP_CLK_APLL_PRE_DIV	16
+#define IMX7ULP_CLK_APLL		17
+#define IMX7ULP_CLK_APLL_POST_DIV1	18
+#define IMX7ULP_CLK_APLL_POST_DIV2	19
+#define IMX7ULP_CLK_APLL_PFD0		20
+#define IMX7ULP_CLK_APLL_PFD1		21
+#define IMX7ULP_CLK_APLL_PFD2		22
+#define IMX7ULP_CLK_APLL_PFD3		23
+#define IMX7ULP_CLK_APLL_PFD_SEL	24
+#define IMX7ULP_CLK_APLL_SEL		25
+#define IMX7ULP_CLK_UPLL		26
+#define IMX7ULP_CLK_SYS_SEL		27
+#define IMX7ULP_CLK_CORE_DIV		28
+#define IMX7ULP_CLK_BUS_DIV		29
+#define IMX7ULP_CLK_PLAT_DIV		30
+#define IMX7ULP_CLK_DDR_SEL		31
+#define IMX7ULP_CLK_DDR_DIV		32
+#define IMX7ULP_CLK_NIC_SEL		33
+#define IMX7ULP_CLK_NIC0_DIV		34
+#define IMX7ULP_CLK_GPU_DIV		35
+#define IMX7ULP_CLK_NIC1_DIV		36
+#define IMX7ULP_CLK_NIC1_BUS_DIV	37
+#define IMX7ULP_CLK_NIC1_EXT_DIV	38
+
+/* PCG2 */
+#define IMX7ULP_CLK_DMA1		39
+#define IMX7ULP_CLK_RGPIO2P1		40
+#define IMX7ULP_CLK_FLEXBUS		41
+#define IMX7ULP_CLK_SEMA42_1		42
+#define IMX7ULP_CLK_DMA_MUX1		43
+#define IMX7ULP_CLK_SNVS		44
+#define IMX7ULP_CLK_CAAM		45
+#define IMX7ULP_CLK_LPTPM4		46
+#define IMX7ULP_CLK_LPTPM5		47
+#define IMX7ULP_CLK_LPIT1		48
+#define IMX7ULP_CLK_LPSPI2		49
+#define IMX7ULP_CLK_LPSPI3		50
+#define IMX7ULP_CLK_LPI2C4		51
+#define IMX7ULP_CLK_LPI2C5		52
+#define IMX7ULP_CLK_LPUART4		53
+#define IMX7ULP_CLK_LPUART5		54
+#define IMX7ULP_CLK_FLEXIO1		55
+#define IMX7ULP_CLK_USB0		56
+#define IMX7ULP_CLK_USB1		57
+#define IMX7ULP_CLK_USB_PHY		58
+#define IMX7ULP_CLK_USB_PL301		59
+#define IMX7ULP_CLK_USDHC0		60
+#define IMX7ULP_CLK_USDHC1		61
+#define IMX7ULP_CLK_WDG1		62
+#define IMX7ULP_CLK_WDG2		63
+
+/* PCG3 */
+#define IMX7ULP_CLK_LPTPM6		64
+#define IMX7ULP_CLK_LPTPM7		65
+#define IMX7ULP_CLK_LPI2C6		66
+#define IMX7ULP_CLK_LPI2C7		67
+#define IMX7ULP_CLK_LPUART6		68
+#define IMX7ULP_CLK_LPUART7		69
+#define IMX7ULP_CLK_VIU			70
+#define IMX7ULP_CLK_DSI			71
+#define IMX7ULP_CLK_LCDIF		72
+#define IMX7ULP_CLK_MMDC		73
+#define IMX7ULP_CLK_PCTLC		74
+#define IMX7ULP_CLK_PCTLD		75
+#define IMX7ULP_CLK_PCTLE		76
+#define IMX7ULP_CLK_PCTLF		77
+#define IMX7ULP_CLK_GPU3D		78
+#define IMX7ULP_CLK_GPU2D		79
+#define IMX7ULP_CLK_MIPI_PLL		80
+#define IMX7ULP_CLK_SIRC		81
+#define IMX7ULP_CLK_SOSC_BUS_CLK	82
+#define IMX7ULP_CLK_FIRC_BUS_CLK	83
+#define IMX7ULP_CLK_SPLL_BUS_CLK	84
+
+#define IMX7ULP_CLK_END			85
+
+#endif /* __DT_BINDINGS_CLOCK_IMX7ULP_H */
-- 
2.7.4

^ permalink raw reply related

* [PATCH RESEND V4 5/9] clk: imx: add composite clk support
From: Dong Aisheng @ 2018-05-25  7:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234671-31755-1-git-send-email-aisheng.dong@nxp.com>

The imx composite clk is designed for Peripheral Clock Control (PCC)
module observed in IMX ULP SoC series. e.g. i.MX7ULP.

NOTE pcc can only be operated when clk is gated.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

---
ChangeLog:
v2->v3:
 * no changes
v1->v2:
 * remove an unneeded blank line change
 * use clk_hw_register
---
 drivers/clk/imx/Makefile        |  1 +
 drivers/clk/imx/clk-composite.c | 85 +++++++++++++++++++++++++++++++++++++++++
 drivers/clk/imx/clk.h           |  6 +++
 3 files changed, 92 insertions(+)
 create mode 100644 drivers/clk/imx/clk-composite.c

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index e5b0d42..f4da12c 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -4,6 +4,7 @@ obj-y += \
 	clk.o \
 	clk-busy.o \
 	clk-cpu.o \
+	clk-composite.o \
 	clk-fixup-div.o \
 	clk-fixup-mux.o \
 	clk-gate-exclusive.o \
diff --git a/drivers/clk/imx/clk-composite.c b/drivers/clk/imx/clk-composite.c
new file mode 100644
index 0000000..297974b
--- /dev/null
+++ b/drivers/clk/imx/clk-composite.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017~2018 NXP
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+
+#define PCG_PCS_SHIFT	24
+#define PCG_PCS_MASK	0x7
+#define PCG_CGC_SHIFT	30
+#define PCG_FRAC_SHIFT	3
+#define PCG_FRAC_WIDTH	1
+#define PCG_FRAC_MASK	BIT(3)
+#define PCG_PCD_SHIFT	0
+#define PCG_PCD_WIDTH	3
+#define PCG_PCD_MASK	0x7
+
+struct clk_hw *imx_clk_composite(const char *name,
+			      const char * const *parent_names,
+			      int num_parents, bool mux_present,
+			      bool rate_present, bool gate_present,
+			      void __iomem *reg)
+{
+	struct clk_hw *mux_hw = NULL, *fd_hw = NULL, *gate_hw = NULL;
+	struct clk_fractional_divider *fd = NULL;
+	struct clk_gate *gate = NULL;
+	struct clk_mux *mux = NULL;
+	struct clk_hw *hw;
+
+	if (mux_present) {
+		mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+		if (!mux)
+			return ERR_PTR(-ENOMEM);
+		mux_hw = &mux->hw;
+		mux->reg = reg;
+		mux->shift = PCG_PCS_SHIFT;
+		mux->mask = PCG_PCS_MASK;
+	}
+
+	if (rate_present) {
+		fd = kzalloc(sizeof(*fd), GFP_KERNEL);
+		if (!fd) {
+			kfree(mux);
+			return ERR_PTR(-ENOMEM);
+		}
+		fd_hw = &fd->hw;
+		fd->reg = reg;
+		fd->mshift = PCG_FRAC_SHIFT;
+		fd->mwidth = PCG_FRAC_WIDTH;
+		fd->mmask  = PCG_FRAC_MASK;
+		fd->nshift = PCG_PCD_SHIFT;
+		fd->nwidth = PCG_PCD_WIDTH;
+		fd->nmask = PCG_PCD_MASK;
+		fd->flags = CLK_FRAC_DIVIDER_ZERO_BASED;
+	}
+
+	if (gate_present) {
+		gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+		if (!gate) {
+			kfree(mux);
+			kfree(fd);
+			return ERR_PTR(-ENOMEM);
+		}
+		gate_hw = &gate->hw;
+		gate->reg = reg;
+		gate->bit_idx = PCG_CGC_SHIFT;
+	}
+
+	hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
+				       mux_hw, &clk_mux_ops, fd_hw,
+				       &clk_fractional_divider_ops, gate_hw,
+				       &clk_gate_ops, CLK_SET_RATE_GATE |
+				       CLK_SET_PARENT_GATE);
+	if (IS_ERR(hw)) {
+		kfree(mux);
+		kfree(fd);
+		kfree(gate);
+	}
+
+	return hw;
+}
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index a5a9374..bc43f68 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -71,6 +71,12 @@ struct clk *imx_clk_busy_mux(const char *name, void __iomem *reg, u8 shift,
 			     u8 width, void __iomem *busy_reg, u8 busy_shift,
 			     const char **parent_names, int num_parents);
 
+struct clk_hw *imx_clk_composite(const char *name,
+				 const char * const *parent_names,
+				 int num_parents, bool mux_present,
+				 bool rate_present, bool gate_present,
+				 void __iomem *reg);
+
 struct clk *imx_clk_fixup_divider(const char *name, const char *parent,
 				  void __iomem *reg, u8 shift, u8 width,
 				  void (*fixup)(u32 *val));
-- 
2.7.4

^ permalink raw reply related

* [PATCH RESEND V4 4/9] clk: imx: add pfdv2 support
From: Dong Aisheng @ 2018-05-25  7:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234671-31755-1-git-send-email-aisheng.dong@nxp.com>

The pfdv2 is designed for PLL Fractional Divide (PFD) observed in System
Clock Generation (SCG) module in IMX ULP SoC series. e.g. i.MX7ULP.

NOTE pfdv2 can only be operated when clk is gated.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

---
ChangeLog:
v2->v3:
 * no changes
v1->v2:
 * change to readl_poll_timeout
 * add pfd lock to protect share reg access between rate and enable/disable
   operations and multiple pfd instances.
 * use clk_hw_register
---
 drivers/clk/imx/Makefile    |   3 +-
 drivers/clk/imx/clk-pfdv2.c | 201 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/imx/clk.h       |   3 +
 3 files changed, 206 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/imx/clk-pfdv2.c

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index bfe31bf..e5b0d42 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -12,7 +12,8 @@ obj-y += \
 	clk-pllv2.o \
 	clk-pllv3.o \
 	clk-pllv4.o \
-	clk-pfd.o
+	clk-pfd.o \
+	clk-pfdv2.o
 
 obj-$(CONFIG_SOC_IMX1)   += clk-imx1.o
 obj-$(CONFIG_SOC_IMX21)  += clk-imx21.o
diff --git a/drivers/clk/imx/clk-pfdv2.c b/drivers/clk/imx/clk-pfdv2.c
new file mode 100644
index 0000000..afb2904
--- /dev/null
+++ b/drivers/clk/imx/clk-pfdv2.c
@@ -0,0 +1,201 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017~2018 NXP
+ *
+ * Author: Dong Aisheng <aisheng.dong@nxp.com>
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/iopoll.h>
+#include <linux/slab.h>
+
+/**
+ * struct clk_pfdv2 - IMX PFD clock
+ * @clk_hw:	clock source
+ * @reg:	PFD register address
+ * @gate_bit:	Gate bit offset
+ * @vld_bit:	Valid bit offset
+ * @frac_off:	PLL Fractional Divider offset
+ */
+
+struct clk_pfdv2 {
+	struct clk_hw	hw;
+	void __iomem	*reg;
+	u8		gate_bit;
+	u8		vld_bit;
+	u8		frac_off;
+};
+
+#define to_clk_pfdv2(_hw) container_of(_hw, struct clk_pfdv2, hw)
+
+#define CLK_PFDV2_FRAC_MASK 0x3f
+
+#define LOCK_TIMEOUT_US		USEC_PER_MSEC
+
+static DEFINE_SPINLOCK(pfd_lock);
+
+static int clk_pfdv2_wait(struct clk_pfdv2 *pfd)
+{
+	u32 val;
+
+	return readl_poll_timeout(pfd->reg, val, val & pfd->vld_bit,
+				  0, LOCK_TIMEOUT_US);
+}
+
+static int clk_pfdv2_enable(struct clk_hw *hw)
+{
+	struct clk_pfdv2 *pfd = to_clk_pfdv2(hw);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&pfd_lock, flags);
+	val = readl_relaxed(pfd->reg);
+	val &= ~pfd->gate_bit;
+	writel_relaxed(val, pfd->reg);
+	spin_unlock_irqrestore(&pfd_lock, flags);
+
+	return clk_pfdv2_wait(pfd);
+}
+
+static void clk_pfdv2_disable(struct clk_hw *hw)
+{
+	struct clk_pfdv2 *pfd = to_clk_pfdv2(hw);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&pfd_lock, flags);
+	val = readl_relaxed(pfd->reg);
+	val |= pfd->gate_bit;
+	writel_relaxed(val, pfd->reg);
+	spin_unlock_irqrestore(&pfd_lock, flags);
+}
+
+static unsigned long clk_pfdv2_recalc_rate(struct clk_hw *hw,
+					   unsigned long parent_rate)
+{
+	struct clk_pfdv2 *pfd = to_clk_pfdv2(hw);
+	u64 tmp = parent_rate;
+	u8 frac;
+
+	frac = (readl_relaxed(pfd->reg) >> pfd->frac_off)
+		& CLK_PFDV2_FRAC_MASK;
+
+	if (!frac) {
+		pr_debug("clk_pfdv2: %s invalid pfd frac value 0\n",
+			 clk_hw_get_name(hw));
+		return 0;
+	}
+
+	tmp *= 18;
+	do_div(tmp, frac);
+
+	return tmp;
+}
+
+static long clk_pfdv2_round_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *prate)
+{
+	u64 tmp = *prate;
+	u8 frac;
+
+	tmp = tmp * 18 + rate / 2;
+	do_div(tmp, rate);
+	frac = tmp;
+
+	if (frac < 12)
+		frac = 12;
+	else if (frac > 35)
+		frac = 35;
+
+	tmp = *prate;
+	tmp *= 18;
+	do_div(tmp, frac);
+
+	return tmp;
+}
+
+static int clk_pfdv2_is_enabled(struct clk_hw *hw)
+{
+	struct clk_pfdv2 *pfd = to_clk_pfdv2(hw);
+
+	if (readl_relaxed(pfd->reg) & pfd->gate_bit)
+		return 0;
+
+	return 1;
+}
+
+static int clk_pfdv2_set_rate(struct clk_hw *hw, unsigned long rate,
+			      unsigned long parent_rate)
+{
+	struct clk_pfdv2 *pfd = to_clk_pfdv2(hw);
+	unsigned long flags;
+	u64 tmp = parent_rate;
+	u32 val;
+	u8 frac;
+
+	tmp = tmp * 18 + rate / 2;
+	do_div(tmp, rate);
+	frac = tmp;
+	if (frac < 12)
+		frac = 12;
+	else if (frac > 35)
+		frac = 35;
+
+	spin_lock_irqsave(&pfd_lock, flags);
+	val = readl_relaxed(pfd->reg);
+	val &= ~(CLK_PFDV2_FRAC_MASK << pfd->frac_off);
+	val |= frac << pfd->frac_off;
+	writel_relaxed(val, pfd->reg);
+	spin_unlock_irqrestore(&pfd_lock, flags);
+
+	return 0;
+}
+
+static const struct clk_ops clk_pfdv2_ops = {
+	.enable		= clk_pfdv2_enable,
+	.disable	= clk_pfdv2_disable,
+	.recalc_rate	= clk_pfdv2_recalc_rate,
+	.round_rate	= clk_pfdv2_round_rate,
+	.set_rate	= clk_pfdv2_set_rate,
+	.is_enabled     = clk_pfdv2_is_enabled,
+};
+
+struct clk_hw *imx_clk_pfdv2(const char *name, const char *parent_name,
+			     void __iomem *reg, u8 idx)
+{
+	struct clk_init_data init;
+	struct clk_pfdv2 *pfd;
+	struct clk_hw *hw;
+	int ret;
+
+	WARN_ON(idx > 3);
+
+	pfd = kzalloc(sizeof(*pfd), GFP_KERNEL);
+	if (!pfd)
+		return ERR_PTR(-ENOMEM);
+
+	pfd->reg = reg;
+	pfd->gate_bit = 1 << ((idx + 1) * 8 - 1);
+	pfd->vld_bit = pfd->gate_bit - 1;
+	pfd->frac_off = idx * 8;
+
+	init.name = name;
+	init.ops = &clk_pfdv2_ops;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+	init.flags = CLK_SET_RATE_GATE;
+
+	pfd->hw.init = &init;
+
+	hw = &pfd->hw;
+	ret = clk_hw_register(NULL, hw);
+	if (ret) {
+		kfree(pfd);
+		hw = ERR_PTR(ret);
+	}
+
+	return hw;
+}
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 2fb4f1d..a5a9374 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -60,6 +60,9 @@ struct clk *imx_clk_gate_exclusive(const char *name, const char *parent,
 struct clk *imx_clk_pfd(const char *name, const char *parent_name,
 		void __iomem *reg, u8 idx);
 
+struct clk_hw *imx_clk_pfdv2(const char *name, const char *parent_name,
+			     void __iomem *reg, u8 idx);
+
 struct clk *imx_clk_busy_divider(const char *name, const char *parent_name,
 				 void __iomem *reg, u8 shift, u8 width,
 				 void __iomem *busy_reg, u8 busy_shift);
-- 
2.7.4

^ permalink raw reply related

* [PATCH RESEND V4 3/9] clk: imx: add pllv4 support
From: Dong Aisheng @ 2018-05-25  7:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234671-31755-1-git-send-email-aisheng.dong@nxp.com>

pllv4 is designed for System Clock Generation (SCG) module observed
in IMX ULP SoC series. e.g. i.MX7ULP.

The SCG modules generates clock used to derive processor, system,
peripheral bus and external memory interface clocks while this patch
intends to support the PLL part.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

---
ChangeLog:
v2->v3:
 * no changes
v1->v2:
 * remove clk_pllv4_is_enabled() check in set_rate, instead it will
   be handled by core later.
 * use readl_poll_timeout
 * use clk_hw_register instead of clk_register
 * other minor changes
---
 drivers/clk/imx/Makefile    |   1 +
 drivers/clk/imx/clk-pllv4.c | 182 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/imx/clk.h       |   3 +
 3 files changed, 186 insertions(+)
 create mode 100644 drivers/clk/imx/clk-pllv4.c

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 8c3baa7..bfe31bf 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -11,6 +11,7 @@ obj-y += \
 	clk-pllv1.o \
 	clk-pllv2.o \
 	clk-pllv3.o \
+	clk-pllv4.o \
 	clk-pfd.o
 
 obj-$(CONFIG_SOC_IMX1)   += clk-imx1.o
diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c
new file mode 100644
index 0000000..67c64c7
--- /dev/null
+++ b/drivers/clk/imx/clk-pllv4.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017~2018 NXP
+ *
+ * Author: Dong Aisheng <aisheng.dong@nxp.com>
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/iopoll.h>
+#include <linux/slab.h>
+
+/* PLL Control Status Register (xPLLCSR) */
+#define PLL_CSR_OFFSET		0x0
+#define PLL_VLD			BIT(24)
+#define PLL_EN			BIT(0)
+
+/* PLL Configuration Register (xPLLCFG) */
+#define PLL_CFG_OFFSET		0x08
+#define BP_PLL_MULT		16
+#define BM_PLL_MULT		(0x7f << 16)
+
+/* PLL Numerator Register (xPLLNUM) */
+#define PLL_NUM_OFFSET		0x10
+
+/* PLL Denominator Register (xPLLDENOM) */
+#define PLL_DENOM_OFFSET	0x14
+
+struct clk_pllv4 {
+	struct clk_hw	hw;
+	void __iomem	*base;
+};
+
+/* Valid PLL MULT Table */
+static const int pllv4_mult_table[] = {33, 27, 22, 20, 17, 16};
+
+#define to_clk_pllv4(__hw) container_of(__hw, struct clk_pllv4, hw)
+
+#define LOCK_TIMEOUT_US		USEC_PER_MSEC
+
+static inline int clk_pllv4_wait_lock(struct clk_pllv4 *pll)
+{
+	u32 csr;
+
+	return readl_poll_timeout(pll->base  + PLL_CSR_OFFSET,
+				  csr, csr & PLL_VLD, 0, LOCK_TIMEOUT_US);
+}
+
+static int clk_pllv4_is_enabled(struct clk_hw *hw)
+{
+	struct clk_pllv4 *pll = to_clk_pllv4(hw);
+
+	if (readl_relaxed(pll->base) & PLL_EN)
+		return 1;
+
+	return 0;
+}
+
+static unsigned long clk_pllv4_recalc_rate(struct clk_hw *hw,
+					   unsigned long parent_rate)
+{
+	struct clk_pllv4 *pll = to_clk_pllv4(hw);
+	u32 div;
+
+	div = readl_relaxed(pll->base + PLL_CFG_OFFSET);
+	div &= BM_PLL_MULT;
+	div >>= BP_PLL_MULT;
+
+	return parent_rate * div;
+}
+
+static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *prate)
+{
+	unsigned long parent_rate = *prate;
+	unsigned long round_rate, i;
+
+	for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
+		round_rate = parent_rate * pllv4_mult_table[i];
+		if (rate >= round_rate)
+			return round_rate;
+	}
+
+	return round_rate;
+}
+
+static bool clk_pllv4_is_valid_mult(unsigned int mult)
+{
+	int i;
+
+	/* check if mult is in valid MULT table */
+	for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
+		if (pllv4_mult_table[i] == mult)
+			return true;
+	}
+
+	return false;
+}
+
+static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long rate,
+			      unsigned long parent_rate)
+{
+	struct clk_pllv4 *pll = to_clk_pllv4(hw);
+	u32 val, mult;
+
+	mult = rate / parent_rate;
+
+	if (!clk_pllv4_is_valid_mult(mult))
+		return -EINVAL;
+
+	val = readl_relaxed(pll->base + PLL_CFG_OFFSET);
+	val &= ~BM_PLL_MULT;
+	val |= mult << BP_PLL_MULT;
+	writel_relaxed(val, pll->base + PLL_CFG_OFFSET);
+
+	return 0;
+}
+
+static int clk_pllv4_enable(struct clk_hw *hw)
+{
+	u32 val;
+	struct clk_pllv4 *pll = to_clk_pllv4(hw);
+
+	val = readl_relaxed(pll->base);
+	val |= PLL_EN;
+	writel_relaxed(val, pll->base);
+
+	return clk_pllv4_wait_lock(pll);
+}
+
+static void clk_pllv4_disable(struct clk_hw *hw)
+{
+	u32 val;
+	struct clk_pllv4 *pll = to_clk_pllv4(hw);
+
+	val = readl_relaxed(pll->base);
+	val &= ~PLL_EN;
+	writel_relaxed(val, pll->base);
+}
+
+static const struct clk_ops clk_pllv4_ops = {
+	.recalc_rate	= clk_pllv4_recalc_rate,
+	.round_rate	= clk_pllv4_round_rate,
+	.set_rate	= clk_pllv4_set_rate,
+	.enable		= clk_pllv4_enable,
+	.disable	= clk_pllv4_disable,
+	.is_enabled	= clk_pllv4_is_enabled,
+};
+
+struct clk_hw *imx_clk_pllv4(const char *name, const char *parent_name,
+			  void __iomem *base)
+{
+	struct clk_pllv4 *pll;
+	struct clk_hw *hw;
+	struct clk_init_data init;
+	int ret;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	pll->base = base;
+
+	init.name = name;
+	init.ops = &clk_pllv4_ops;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+	init.flags = CLK_SET_RATE_GATE;
+
+	pll->hw.init = &init;
+
+	hw = &pll->hw;
+	ret = clk_hw_register(NULL, hw);
+	if (ret) {
+		kfree(pll);
+		hw = ERR_PTR(ret);
+	}
+
+	return hw;
+}
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 8076ec0..2fb4f1d 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -42,6 +42,9 @@ enum imx_pllv3_type {
 struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 		const char *parent_name, void __iomem *base, u32 div_mask);
 
+struct clk_hw *imx_clk_pllv4(const char *name, const char *parent_name,
+			     void __iomem *base);
+
 struct clk *clk_register_gate2(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 bit_idx, u8 cgr_val,
-- 
2.7.4

^ permalink raw reply related

* [PATCH RESEND V4 2/9] clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support
From: Dong Aisheng @ 2018-05-25  7:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234671-31755-1-git-send-email-aisheng.dong@nxp.com>

Adding CLK_FRAC_DIVIDER_ZERO_BASED flag to indicate the numerator and
denominator value in register are start from 0.

This can be used to support frac dividers like below:
Divider output clock = Divider input clock x [(frac +1) / (div +1)]
where frac/div in register is:
000b - Divide by 1.
001b - Divide by 2.
010b - Divide by 3.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

---
ChangeLog:
v2->v3:
 * no changes
v1->v2:
 * improve comments suggested by Stephen
---
 drivers/clk/clk-fractional-divider.c | 10 ++++++++++
 include/linux/clk-provider.h         |  8 ++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
index fdf625f..7ccde6b 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -40,6 +40,11 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
 	m = (val & fd->mmask) >> fd->mshift;
 	n = (val & fd->nmask) >> fd->nshift;
 
+	if (fd->flags & CLK_FRAC_DIVIDER_ZERO_BASED) {
+		m++;
+		n++;
+	}
+
 	if (!n || !m)
 		return parent_rate;
 
@@ -103,6 +108,11 @@ static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate,
 			GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0),
 			&m, &n);
 
+	if (fd->flags & CLK_FRAC_DIVIDER_ZERO_BASED) {
+		m--;
+		n--;
+	}
+
 	if (fd->lock)
 		spin_lock_irqsave(fd->lock, flags);
 	else
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 3cf522f6..8fad6c8 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -577,6 +577,12 @@ void clk_hw_unregister_fixed_factor(struct clk_hw *hw);
  * @lock:	register lock
  *
  * Clock with adjustable fractional divider affecting its output frequency.
+ *
+ * Flags:
+ * CLK_FRAC_DIVIDER_ZERO_BASED - by default the numerator and denominator
+ *	is the value read from the register. If CLK_FRAC_DIVIDER_ZERO_BASED
+ *	is set then the numerator and denominator are both the value read
+ *	plus one.
  */
 struct clk_fractional_divider {
 	struct clk_hw	hw;
@@ -596,6 +602,8 @@ struct clk_fractional_divider {
 
 #define to_clk_fd(_hw) container_of(_hw, struct clk_fractional_divider, hw)
 
+#define CLK_FRAC_DIVIDER_ZERO_BASED		BIT(0)
+
 extern const struct clk_ops clk_fractional_divider_ops;
 struct clk *clk_register_fractional_divider(struct device *dev,
 		const char *name, const char *parent_name, unsigned long flags,
-- 
2.7.4

^ permalink raw reply related

* [PATCH RESEND V4 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support
From: Dong Aisheng @ 2018-05-25  7:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234671-31755-1-git-send-email-aisheng.dong@nxp.com>

For dividers with zero indicating clock is disabled, instead of giving a
warning each time like "clkx: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not
set" in exist code, we'd like to introduce enable/disable function for it.
e.g.
000b - Clock disabled
001b - Divide by 1
010b - Divide by 2
...

Set rate when the clk is disabled will cache the rate request and only
when the clk is enabled will the driver actually program the hardware to
have the requested divider value. Similarly, when the clk is disabled we'll
write a 0 there, but when the clk is enabled we'll restore whatever rate
(divider) was chosen last.

It does mean that recalc rate will be sort of odd, because when the clk is
off it will return 0, and when the clk is on it will return the right rate.
So to make things work, we'll need to return the cached rate in recalc rate
when the clk is off and read the hardware when the clk is on.

NOTE for the default off divider, the recalc rate will still return 0 as
there's still no proper preset rate. Enable such divider will give user
a reminder error message.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

---
ChangeLog:
v2->v3:
 * split normal and gate ops
 * fix the possible racy
v1->v2:
 * add enable/disable for the type of CLK_DIVIDER_ZERO_GATE dividers
---
 drivers/clk/clk-divider.c    | 152 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h |   9 +++
 2 files changed, 161 insertions(+)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index b6234a5..b3566fd 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -122,6 +122,9 @@ unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
 
 	div = _get_div(table, val, flags, width);
 	if (!div) {
+		if (flags & CLK_DIVIDER_ZERO_GATE)
+			return 0;
+
 		WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
 			"%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n",
 			clk_hw_get_name(hw));
@@ -145,6 +148,34 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
 				   divider->flags, divider->width);
 }
 
+static unsigned long clk_divider_gate_recalc_rate(struct clk_hw *hw,
+						  unsigned long parent_rate)
+{
+	struct clk_divider *divider = to_clk_divider(hw);
+	unsigned long flags = 0;
+	unsigned int val;
+
+	if (divider->lock)
+		spin_lock_irqsave(divider->lock, flags);
+	else
+		__acquire(divider->lock);
+
+	if (!clk_hw_is_enabled(hw)) {
+		val = divider->cached_val;
+	} else {
+		val = clk_readl(divider->reg) >> divider->shift;
+		val &= clk_div_mask(divider->width);
+	}
+
+	if (divider->lock)
+		spin_unlock_irqrestore(divider->lock, flags);
+	else
+		__release(divider->lock);
+
+	return divider_recalc_rate(hw, parent_rate, val, divider->table,
+				   divider->flags, divider->width);
+}
+
 static bool _is_valid_table_div(const struct clk_div_table *table,
 							 unsigned int div)
 {
@@ -437,6 +468,108 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
 	return 0;
 }
 
+static int clk_divider_gate_set_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_rate)
+{
+	struct clk_divider *divider = to_clk_divider(hw);
+	unsigned long flags = 0;
+	int value;
+	u32 val;
+
+	value = divider_get_val(rate, parent_rate, divider->table,
+				divider->width, divider->flags);
+	if (value < 0)
+		return value;
+
+	if (divider->lock)
+		spin_lock_irqsave(divider->lock, flags);
+	else
+		__acquire(divider->lock);
+
+	if (clk_hw_is_enabled(hw)) {
+		if (divider->flags & CLK_DIVIDER_HIWORD_MASK) {
+			val = clk_div_mask(divider->width) << (divider->shift + 16);
+		} else {
+			val = clk_readl(divider->reg);
+			val &= ~(clk_div_mask(divider->width) << divider->shift);
+		}
+		val |= (u32)value << divider->shift;
+		clk_writel(val, divider->reg);
+	} else {
+		divider->cached_val = value;
+	}
+
+	if (divider->lock)
+		spin_unlock_irqrestore(divider->lock, flags);
+	else
+		__release(divider->lock);
+
+	return 0;
+}
+
+static int clk_divider_enable(struct clk_hw *hw)
+{
+	struct clk_divider *divider = to_clk_divider(hw);
+	unsigned long flags = 0;
+	u32 val;
+
+	if (!divider->cached_val) {
+		pr_err("%s: no valid preset rate\n", clk_hw_get_name(hw));
+		return -EINVAL;
+	}
+
+	if (divider->lock)
+		spin_lock_irqsave(divider->lock, flags);
+	else
+		__acquire(divider->lock);
+
+	/* restore div val */
+	val = clk_readl(divider->reg);
+	val |= divider->cached_val << divider->shift;
+	clk_writel(val, divider->reg);
+
+	if (divider->lock)
+		spin_unlock_irqrestore(divider->lock, flags);
+	else
+		__release(divider->lock);
+
+	return 0;
+}
+
+static void clk_divider_disable(struct clk_hw *hw)
+{
+	struct clk_divider *divider = to_clk_divider(hw);
+	unsigned long flags = 0;
+	u32 val;
+
+	if (divider->lock)
+		spin_lock_irqsave(divider->lock, flags);
+	else
+		__acquire(divider->lock);
+
+	/* store the current div val */
+	val = clk_readl(divider->reg) >> divider->shift;
+	val &= clk_div_mask(divider->width);
+	divider->cached_val = val;
+	clk_writel(0, divider->reg);
+
+	if (divider->lock)
+		spin_unlock_irqrestore(divider->lock, flags);
+	else
+		__release(divider->lock);
+}
+
+static int clk_divider_is_enabled(struct clk_hw *hw)
+{
+	struct clk_divider *divider = to_clk_divider(hw);
+	u32 val;
+
+	val = clk_readl(divider->reg) >> divider->shift;
+	val &= clk_div_mask(divider->width);
+
+	return val ? 1 : 0;
+}
+
 const struct clk_ops clk_divider_ops = {
 	.recalc_rate = clk_divider_recalc_rate,
 	.round_rate = clk_divider_round_rate,
@@ -444,6 +577,16 @@ const struct clk_ops clk_divider_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_divider_ops);
 
+const struct clk_ops clk_divider_gate_ops = {
+	.recalc_rate = clk_divider_gate_recalc_rate,
+	.round_rate = clk_divider_round_rate,
+	.set_rate = clk_divider_gate_set_rate,
+	.enable = clk_divider_enable,
+	.disable = clk_divider_disable,
+	.is_enabled = clk_divider_is_enabled,
+};
+EXPORT_SYMBOL_GPL(clk_divider_gate_ops);
+
 const struct clk_ops clk_divider_ro_ops = {
 	.recalc_rate = clk_divider_recalc_rate,
 	.round_rate = clk_divider_round_rate,
@@ -459,6 +602,7 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name,
 	struct clk_divider *div;
 	struct clk_hw *hw;
 	struct clk_init_data init;
+	u32 val;
 	int ret;
 
 	if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
@@ -476,6 +620,8 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name,
 	init.name = name;
 	if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
 		init.ops = &clk_divider_ro_ops;
+	else if (clk_divider_flags & CLK_DIVIDER_ZERO_GATE)
+		init.ops = &clk_divider_gate_ops;
 	else
 		init.ops = &clk_divider_ops;
 	init.flags = flags | CLK_IS_BASIC;
@@ -491,6 +637,12 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name,
 	div->hw.init = &init;
 	div->table = table;
 
+	if (div->flags & CLK_DIVIDER_ZERO_GATE) {
+		val = clk_readl(reg) >> shift;
+		val &= clk_div_mask(width);
+		div->cached_val = val;
+	}
+
 	/* register the clock */
 	hw = &div->hw;
 	ret = clk_hw_register(dev, hw);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 1053e2a..3cf522f6 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -361,6 +361,7 @@ struct clk_div_table {
  * @shift:	shift to the divider bit field
  * @width:	width of the divider bit field
  * @table:	array of value/divider pairs, last entry should have div = 0
+ * @cached_val: cached div hw value used for CLK_DIVIDER_ZERO_GATE
  * @lock:	register lock
  *
  * Clock with an adjustable divider affecting its output frequency.  Implements
@@ -389,6 +390,12 @@ struct clk_div_table {
  * CLK_DIVIDER_MAX_AT_ZERO - For dividers which are like CLK_DIVIDER_ONE_BASED
  *	except when the value read from the register is zero, the divisor is
  *	2^width of the field.
+ * CLK_DIVIDER_ZERO_GATE - For dividers which are like CLK_DIVIDER_ONE_BASED
+ *	when the value read from the register is zero, it means the divisor
+ *	is gated. For this case, the cached_val will be used to store the
+ *	intermediate div for the normal rate operation, like set_rate/get_rate/
+ *	recalc_rate. When the divider is ungated, the driver will actually
+ *	program the hardware to have the requested divider value.
  */
 struct clk_divider {
 	struct clk_hw	hw;
@@ -397,6 +404,7 @@ struct clk_divider {
 	u8		width;
 	u8		flags;
 	const struct clk_div_table	*table;
+	u32		cached_val;
 	spinlock_t	*lock;
 };
 
@@ -410,6 +418,7 @@ struct clk_divider {
 #define CLK_DIVIDER_ROUND_CLOSEST	BIT(4)
 #define CLK_DIVIDER_READ_ONLY		BIT(5)
 #define CLK_DIVIDER_MAX_AT_ZERO		BIT(6)
+#define CLK_DIVIDER_ZERO_GATE		BIT(7)
 
 extern const struct clk_ops clk_divider_ops;
 extern const struct clk_ops clk_divider_ro_ops;
-- 
2.7.4

^ permalink raw reply related

* [PATCH RESEND V4 0/9] clk: add imx7ulp clk support
From: Dong Aisheng @ 2018-05-25  7:51 UTC (permalink / raw)
  To: linux-arm-kernel

This is a rebased version of below patch series against latest clk tree.
[PATCH RESEND V3 0/9] clk: add imx7ulp clk support
https://lkml.org/lkml/2018/3/16/310

It only updates the license to SPDX format as well as a minor fix of
pllv4.

This patch series intends to add imx7ulp clk support.

i.MX7ULP Clock functions are under joint control of the System
Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
modules, and Core Mode Controller (CMC)1 blocks

The clocking scheme provides clear separation between M4 domain
and A7 domain. Except for a few clock sources shared between two
domains, such as the System Oscillator clock, the Slow IRC (SIRC),
and and the Fast IRC clock (FIRCLK), clock sources and clock
management are separated and contained within each domain.

M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.

Note: this series only adds A7 clock domain support as M4 clock
domain will be handled by M4 seperately.

Change Log:
v2->v3:
 * Patch 1 changed on: 1) split normal and gate ops 2) fix the possible racy
   Others no changes.

v1->v2:
 * add enable/disable for the type of CLK_DIVIDER_ZERO_GATE dividers
 * use clk_hw apis to register clocks
 * use of_clk_add_hw_provider
 * split the clocks register process into two parts: early part for possible
   timers clocks registered by CLK_OF_DECLARE_DRIVER and the later part for
   the left normal peripheral clocks registered by a platform driver.

Dong Aisheng (9):
  clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support
  clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support
  clk: imx: add pllv4 support
  clk: imx: add pfdv2 support
  clk: imx: add composite clk support
  dt-bindings: clock: add imx7ulp clock binding doc
  clk: imx: make mux parent strings const
  clk: imx: implement new clk_hw based APIs
  clk: imx: add imx7ulp clk driver

 .../devicetree/bindings/clock/imx7ulp-clock.txt    |  62 ++++++
 drivers/clk/clk-divider.c                          | 152 ++++++++++++++
 drivers/clk/clk-fractional-divider.c               |  10 +
 drivers/clk/imx/Makefile                           |   6 +-
 drivers/clk/imx/clk-busy.c                         |   2 +-
 drivers/clk/imx/clk-composite.c                    |  85 ++++++++
 drivers/clk/imx/clk-fixup-mux.c                    |   2 +-
 drivers/clk/imx/clk-imx7ulp.c                      | 227 +++++++++++++++++++++
 drivers/clk/imx/clk-pfdv2.c                        | 201 ++++++++++++++++++
 drivers/clk/imx/clk-pllv4.c                        | 182 +++++++++++++++++
 drivers/clk/imx/clk.c                              |  22 ++
 drivers/clk/imx/clk.h                              |  92 ++++++++-
 include/dt-bindings/clock/imx7ulp-clock.h          | 105 ++++++++++
 include/linux/clk-provider.h                       |  17 ++
 14 files changed, 1155 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
 create mode 100644 drivers/clk/imx/clk-composite.c
 create mode 100644 drivers/clk/imx/clk-imx7ulp.c
 create mode 100644 drivers/clk/imx/clk-pfdv2.c
 create mode 100644 drivers/clk/imx/clk-pllv4.c
 create mode 100644 include/dt-bindings/clock/imx7ulp-clock.h

-- 
2.7.4

^ permalink raw reply

* [PATCH V0:net-next 4/4] dt-bindings: stm32: add compatible for syscon
From: Christophe Roullier @ 2018-05-25  7:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234401-15812-1-git-send-email-christophe.roullier@st.com>

This patch describes syscon DT bindings.

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/arm/stm32/stm32-syscon.txt         | 14 ++++++++++++++
 .../devicetree/bindings/arm/{ => stm32}/stm32.txt          |  0
 2 files changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/stm32/stm32-syscon.txt
 rename Documentation/devicetree/bindings/arm/{ => stm32}/stm32.txt (100%)

diff --git a/Documentation/devicetree/bindings/arm/stm32/stm32-syscon.txt b/Documentation/devicetree/bindings/arm/stm32/stm32-syscon.txt
new file mode 100644
index 0000000..99980ae
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/stm32/stm32-syscon.txt
@@ -0,0 +1,14 @@
+STMicroelectronics STM32 Platforms System Controller
+
+Properties:
+   - compatible : should contain two values. First value must be :
+                 - " st,stm32mp157-syscfg " - for stm32mp157 based SoCs,
+                 second value must be always "syscon".
+   - reg : offset and length of the register set.
+
+ Example:
+         syscfg: syscon at 50020000 {
+                 compatible = "st,stm32mp157-syscfg", "syscon";
+                 reg = <0x50020000 0x400>;
+         };
+
diff --git a/Documentation/devicetree/bindings/arm/stm32.txt b/Documentation/devicetree/bindings/arm/stm32/stm32.txt
similarity index 100%
rename from Documentation/devicetree/bindings/arm/stm32.txt
rename to Documentation/devicetree/bindings/arm/stm32/stm32.txt
-- 
1.9.1

^ permalink raw reply related

* [PATCH V0:net-next 3/4] net: stmmac: add dwmac-4.20a compatible
From: Christophe Roullier @ 2018-05-25  7:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527234401-15812-1-git-send-email-christophe.roullier@st.com>

Manage dwmac-4.20a version from synopsys

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index ebd3e5f..6d141f3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -472,7 +472,8 @@ struct plat_stmmacenet_data *
 	}
 
 	if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
-	    of_device_is_compatible(np, "snps,dwmac-4.10a")) {
+	    of_device_is_compatible(np, "snps,dwmac-4.10a") ||
+	    of_device_is_compatible(np, "snps,dwmac-4.20a")) {
 		plat->has_gmac4 = 1;
 		plat->has_gmac = 0;
 		plat->pmt = 1;
-- 
1.9.1

^ permalink raw reply related


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