Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v1] scsi: ufs: skip shutdown if hba is not powered
From: Martin K. Petersen @ 2019-09-24  2:24 UTC (permalink / raw)
  To: Stanley Chu
  Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
	kuohong.wang, avri.altman, linux-mediatek, peter.wang,
	alim.akhtar, matthias.bgg, pedrom.sousa, linux-arm-kernel,
	beanhuo
In-Reply-To: <1568780438-28753-1-git-send-email-stanley.chu@mediatek.com>


Stanley,

> In some cases, hba may go through shutdown flow without successful
> initialization and then make system hang.

Applied to 5.4/scsi-fixes, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

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

^ permalink raw reply

* RE: [PATCH v8 2/3] arm64: mm: implement arch_faults_on_old_pte() on arm64
From: Justin He (Arm Technology China) @ 2019-09-24  2:17 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Mark Rutland, Kaly Xin (Arm Technology China), Ralph Campbell,
	Andrew Morton, Suzuki Poulose, Marc Zyngier, Anshuman Khandual,
	linux-kernel@vger.kernel.org, Matthew Wilcox, linux-mm@kvack.org,
	Jérôme Glisse, James Morse,
	linux-arm-kernel@lists.infradead.org, Punit Agrawal,
	hejianet@gmail.com, Thomas Gleixner, nd, Will Deacon,
	Alex Van Brunt, Kirill A. Shutemov, Robin Murphy
In-Reply-To: <20190923161824.GD10192@arrakis.emea.arm.com>



> -----Original Message-----
> From: Catalin Marinas <catalin.marinas@arm.com>
> Sent: 2019年9月24日 0:18
> To: Justin He (Arm Technology China) <Justin.He@arm.com>
> Cc: Will Deacon <will@kernel.org>; Mark Rutland
> <Mark.Rutland@arm.com>; James Morse <James.Morse@arm.com>; Marc
> Zyngier <maz@kernel.org>; Matthew Wilcox <willy@infradead.org>; Kirill A.
> Shutemov <kirill.shutemov@linux.intel.com>; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> mm@kvack.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Punit
> Agrawal <punitagrawal@gmail.com>; Anshuman Khandual
> <Anshuman.Khandual@arm.com>; Alex Van Brunt
> <avanbrunt@nvidia.com>; Robin Murphy <Robin.Murphy@arm.com>;
> Thomas Gleixner <tglx@linutronix.de>; Andrew Morton <akpm@linux-
> foundation.org>; Jérôme Glisse <jglisse@redhat.com>; Ralph Campbell
> <rcampbell@nvidia.com>; hejianet@gmail.com; Kaly Xin (Arm Technology
> China) <Kaly.Xin@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v8 2/3] arm64: mm: implement
> arch_faults_on_old_pte() on arm64
> 
> On Sat, Sep 21, 2019 at 09:50:53PM +0800, Jia He wrote:
> > diff --git a/arch/arm64/include/asm/pgtable.h
> b/arch/arm64/include/asm/pgtable.h
> > index e09760ece844..4a9939615e41 100644
> > --- a/arch/arm64/include/asm/pgtable.h
> > +++ b/arch/arm64/include/asm/pgtable.h
> > @@ -868,6 +868,18 @@ static inline void update_mmu_cache(struct
> vm_area_struct *vma,
> >  #define phys_to_ttbr(addr)	(addr)
> >  #endif
> >
> > +/*
> > + * On arm64 without hardware Access Flag, copying fromuser will fail
> because
>                                                      ^^^^^^^^
> 						     from user
> 

Ok
> > + * the pte is old and cannot be marked young. So we always end up with
> zeroed
> > + * page after fork() + CoW for pfn mappings. we don't always have a
>                                                 ^^
> 						We
> 

Ok
> > + * hardware-managed access flag on arm64.
> > + */
> > +static inline bool arch_faults_on_old_pte(void)
> > +{
> > +	return !cpu_has_hw_af();
> 
> I saw an early incarnation of your patch having a
> WARN_ON(preemptible()). I think we need this back just in case this
> function will be used elsewhere in the future.

Okay

--
Cheers,
Justin (Jia He)


> 
> > +}
> > +#define arch_faults_on_old_pte arch_faults_on_old_pte
> 
> Otherwise,
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] PM / devfreq: Lock devfreq in trans_stat_show
From: Chanwoo Choi @ 2019-09-24  2:11 UTC (permalink / raw)
  To: Leonard Crestez, Matthias Kaehlcke, MyungJoo Ham, Kyungmin Park
  Cc: Artur Świgoń, linux-pm, Krzysztof Kozlowski,
	Lukasz Luba, NXP Linux Team, Georgi Djakov, linux-arm-kernel
In-Reply-To: <714675448e7fbf3c930b0dca6fbe54fa5f87211b.1569256001.git.leonard.crestez@nxp.com>

Hi,

On 19. 9. 24. 오전 1:27, Leonard Crestez wrote:
> There is no locking in this sysfs show function so stats printing can
> race with a devfreq_update_status called as part of freq switching or
> with initialization.
> 
> Also add an assert in devfreq_update_status to make it clear that lock
> must be held by caller.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/devfreq/devfreq.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> Changes since v1:
> * Split from series: low-priority bugfix not strictly required for PM QoS
> * Only keep lock during update, release before sprintf
> Link to v1: https://patchwork.kernel.org/patch/11149493/
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 4c58fbf7d4e4..00fc23fea5b2 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -206,10 +206,11 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
>  {
>  	int lev, prev_lev, ret = 0;
>  	unsigned long cur_time;
>  
>  	cur_time = jiffies;
> +	lockdep_assert_held(&devfreq->lock);

It better to move lock checking before 'cur_time = jiffies'
in order to reduce the redundant code execution.

>  
>  	/* Immediately exit if previous_freq is not initialized yet. */
>  	if (!devfreq->previous_freq)
>  		goto out;
>  
> @@ -1507,16 +1508,22 @@ static ssize_t trans_stat_show(struct device *dev,
>  	struct devfreq *devfreq = to_devfreq(dev);
>  	ssize_t len;
>  	int i, j;
>  	unsigned int max_state = devfreq->profile->max_state;
>  
> -	if (!devfreq->stop_polling &&
> -			devfreq_update_status(devfreq, devfreq->previous_freq))
> -		return 0;
>  	if (max_state == 0)
>  		return sprintf(buf, "Not Supported.\n");
>  
> +	/* lock and update */

It is not necessary. Anyone can know that this code is related to mutex lock/unlock.

> +	mutex_lock(&devfreq->lock);
> +	if (!devfreq->stop_polling &&
> +			devfreq_update_status(devfreq, devfreq->previous_freq)) {
> +		mutex_unlock(&devfreq->lock);
> +		return 0;
> +	}
> +	mutex_unlock(&devfreq->lock);
> +
>  	len = sprintf(buf, "     From  :   To\n");
>  	len += sprintf(buf + len, "           :");
>  	for (i = 0; i < max_state; i++)
>  		len += sprintf(buf + len, "%10lu",
>  				devfreq->profile->freq_table[i]);
> 

Basically, it is necessary. Please edit it by my comments.
Also, you have to add the following 'fixes' as following:
and send it stable mailing list.

Fixes: 39688ce6facd ("PM / devfreq: account suspend/resume for stats")

If you edit it by my comments, feel free to add my tag:
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

^ permalink raw reply

* RE: [PATCH v8 1/3] arm64: cpufeature: introduce helper cpu_has_hw_af()
From: Justin He (Arm Technology China) @ 2019-09-24  1:50 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Mark Rutland, Kaly Xin (Arm Technology China), Ralph Campbell,
	Andrew Morton, Suzuki Poulose, Marc Zyngier, Anshuman Khandual,
	linux-kernel@vger.kernel.org, Matthew Wilcox, linux-mm@kvack.org,
	Jérôme Glisse, James Morse,
	linux-arm-kernel@lists.infradead.org, Punit Agrawal,
	hejianet@gmail.com, Thomas Gleixner, nd, Will Deacon,
	Alex Van Brunt, Kirill A. Shutemov, Robin Murphy
In-Reply-To: <20190923160710.GC10192@arrakis.emea.arm.com>

Hi Catalin

> -----Original Message-----
> From: Catalin Marinas <catalin.marinas@arm.com>
> Sent: 2019年9月24日 0:07
> To: Justin He (Arm Technology China) <Justin.He@arm.com>
> Cc: Will Deacon <will@kernel.org>; Mark Rutland
> <Mark.Rutland@arm.com>; James Morse <James.Morse@arm.com>; Marc
> Zyngier <maz@kernel.org>; Matthew Wilcox <willy@infradead.org>; Kirill A.
> Shutemov <kirill.shutemov@linux.intel.com>; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> mm@kvack.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Punit
> Agrawal <punitagrawal@gmail.com>; Anshuman Khandual
> <Anshuman.Khandual@arm.com>; Alex Van Brunt
> <avanbrunt@nvidia.com>; Robin Murphy <Robin.Murphy@arm.com>;
> Thomas Gleixner <tglx@linutronix.de>; Andrew Morton <akpm@linux-
> foundation.org>; Jérôme Glisse <jglisse@redhat.com>; Ralph Campbell
> <rcampbell@nvidia.com>; hejianet@gmail.com; Kaly Xin (Arm Technology
> China) <Kaly.Xin@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v8 1/3] arm64: cpufeature: introduce helper
> cpu_has_hw_af()
> 
> On Sat, Sep 21, 2019 at 09:50:52PM +0800, Jia He wrote:
> > We unconditionally set the HW_AFDBM capability and only enable it on
> > CPUs which really have the feature. But sometimes we need to know
> > whether this cpu has the capability of HW AF. So decouple AF from
> > DBM by new helper cpu_has_hw_af().
> >
> > Reported-by: kbuild test robot <lkp@intel.com>
> > Suggested-by: Suzuki Poulose <Suzuki.Poulose@arm.com>
> > Signed-off-by: Jia He <justin.he@arm.com>
> > ---
> >  arch/arm64/include/asm/cpufeature.h | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/cpufeature.h
> b/arch/arm64/include/asm/cpufeature.h
> > index c96ffa4722d3..46caf934ba4e 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -667,6 +667,16 @@ static inline u32
> id_aa64mmfr0_parange_to_phys_shift(int parange)
> >  	default: return CONFIG_ARM64_PA_BITS;
> >  	}
> >  }
> > +
> > +/* Decouple AF from AFDBM. */
> 
> We could do with a better comment here or just remove it altogether. The
> aim of the patch was to decouple AF check from the AF+DBM but the
> comment here should describe what the function does. Maybe something
> like: "Check whether hardware update of the Access flag is supported".
> 

Okay, I will update it

--
Cheers,
Justin (Jia He)


> > +static inline bool cpu_has_hw_af(void)
> > +{
> > +	if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM))
> > +		return read_cpuid(ID_AA64MMFR1_EL1) & 0xf;
> > +
> > +	return false;
> > +}
> 
> Other than the comment above,
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] PM / devfreq: Check NULL governor in available_governors_show
From: Chanwoo Choi @ 2019-09-24  1:52 UTC (permalink / raw)
  To: Leonard Crestez, Matthias Kaehlcke, MyungJoo Ham, Kyungmin Park
  Cc: Artur Świgoń, linux-pm, linux-arm-kernel,
	Krzysztof Kozlowski, Lukasz Luba
In-Reply-To: <96f459015e6418cee4fa20fdbdb80c4caf417c19.1569256298.git.leonard.crestez@nxp.com>

Hi,

On 19. 9. 24. 오전 1:34, Leonard Crestez wrote:
> The governor is initialized after sysfs attributes become visible so in
> theory the governor field can be NULL here.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/devfreq/devfreq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Found this by hacking device core to call attribute "show" functions
> from inside device_add.
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 00fc23fea5b2..896fb2312f2f 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1322,11 +1322,11 @@ static ssize_t available_governors_show(struct device *d,
>  
>  	/*
>  	 * The devfreq with immutable governor (e.g., passive) shows
>  	 * only own governor.
>  	 */
> -	if (df->governor->immutable) {
> +	if (df->governor && df->governor->immutable) {
>  		count = scnprintf(&buf[count], DEVFREQ_NAME_LEN,
>  				  "%s ", df->governor_name);
>  	/*
>  	 * The devfreq device shows the registered governor except for
>  	 * immutable governors such as passive governor .
> 

As you mentioned, it create sysfs and then initialize the governor instance
as following:

	device_register()
		device_add()
			device_add_attrs()
				creating sysfs entries.
	governor = try_then_request_governor(...)


Thanks for fix-up.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>


Additionally, you have to add the following 'fixes' tag
and then send it to stable mailing list(stable@vger.kernel.org).
- Fixes: bcf23c79c4e46 ("PM / devfreq: Fix available_governor sysfs")

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

^ permalink raw reply

* RE: [PATCH v4 2/3] dmaengine: imx-sdma: fix dma freezes
From: Robin Gong @ 2019-09-24  1:38 UTC (permalink / raw)
  To: Philipp Puschmann, linux-kernel@vger.kernel.org
  Cc: Andy Duan, festevam@gmail.com, s.hauer@pengutronix.de,
	vkoul@kernel.org, dl-linux-imx, kernel@pengutronix.de,
	dan.j.williams@intel.com, shawnguo@kernel.org,
	dmaengine@vger.kernel.or, linux-arm-kernel@lists.infradead.org,
	l.stach@pengutronix.de
In-Reply-To: <20190919142942.12469-3-philipp.puschmann@emlix.com>


On 2019-9-19 22:30 Philipp Puschmann <philipp.puschmann@emlix.com> wrote 
> For some years and since many kernel versions there are reports that the RX
> UART SDMA channel stops working at some point. The workaround was to
> disable DMA for RX. This commit tries to fix the problem itself.
> 
> Due to its license i wasn't able to debug the sdma script itself but it somehow
> leads to blocking the scheduling of the channel script when a running sdma
> script does not find any free descriptor in the ring to put its data into.
> 
> If we detect such a potential case we manually restart the channel.
> 
> As sdmac->desc is constant we can move desc out of the loop.
> 
> Fixes: 1ec1e82f2510 ("dmaengine: Add Freescale i.MX SDMA support")
In fact, it's a refine patch rather than bug fix, just restore cyclic transfer
back in the corner case. There are two causes for such 'corner case':
1. improper number of BD or length of BD setting for cyclic, so that BD could
be consumed very quickly, worst case is uart Aging timer which one byte
may consume one BD. So for such case, enlarge more BDs is the right way as
your UART patch.
2. High cpu loading so that SDMA interrupt handler can't run in time to set 
BD_DONE flag back again, at last all BDs consumed. In such case, this patch
may blind other coding issues such as long time window of disable irq(spin_lock_irq)
. So I think this patch is much like a refine/restore patch, and it's better to add
a clear print information to hint user channel is restoring and unexpected high cpu
loading is coming...

> Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
> Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> 
> Changelog v4:
>  - fixed the fixes tag
> 
> Changelog v3:
>  - use correct dma_wmb() instead of dma_wb()
>  - add fixes tag
> 
> Changelog v2:
>  - clarify comment and commit description
> 
>  drivers/dma/imx-sdma.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> e029a2443cfc..a32b5962630e 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -775,21 +775,23 @@ static void sdma_start_desc(struct sdma_channel
> *sdmac)  static void sdma_update_channel_loop(struct sdma_channel
> *sdmac)  {
>  	struct sdma_buffer_descriptor *bd;
> -	int error = 0;
> -	enum dma_status	old_status = sdmac->status;
> +	struct sdma_desc *desc = sdmac->desc;
> +	int error = 0, cnt = 0;
> +	enum dma_status old_status = sdmac->status;
> 
>  	/*
>  	 * loop mode. Iterate over descriptors, re-setup them and
>  	 * call callback function.
>  	 */
> -	while (sdmac->desc) {
> -		struct sdma_desc *desc = sdmac->desc;
> +	while (desc) {
> 
>  		bd = &desc->bd[desc->buf_tail];
> 
>  		if (bd->mode.status & BD_DONE)
>  			break;
> 
> +		cnt++;
> +
>  		if (bd->mode.status & BD_RROR) {
>  			bd->mode.status &= ~BD_RROR;
>  			sdmac->status = DMA_ERROR;
> @@ -822,6 +824,17 @@ static void sdma_update_channel_loop(struct
> sdma_channel *sdmac)
>  		if (error)
>  			sdmac->status = old_status;
>  	}
> +
> +	/* In some situations it may happen that the sdma does not found any
> +	 * usable descriptor in the ring to put data into. The channel is
> +	 * stopped then. While there is no specific error condition we can
> +	 * check for, a necessary condition is that all available buffers for
> +	 * the current channel have been written to by the sdma script. In
> +	 * this case and after we have made the buffers available again,
> +	 * we restart the channel.
> +	 */
> +	if (cnt >= desc->num_bd)
> +		sdma_enable_channel(sdmac->sdma, sdmac->channel);
>  }
> 
>  static void mxc_sdma_handle_channel_normal(struct sdma_channel *data)
> --
> 2.23.0


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

^ permalink raw reply

* Re: printk() + memory offline deadlock (WAS Re: page_alloc.shuffle=1 + CONFIG_PROVE_LOCKING=y = arm64 hang)
From: Sergey Senozhatsky @ 2019-09-24  1:28 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Theodore Ts'o, Sergey Senozhatsky, Arnd Bergmann, linux-mm,
	Peter Zijlstra, Catalin Marinas, linux-kernel, Steven Rostedt,
	Sergey Senozhatsky, Qian Cai, Greg Kroah-Hartman, Waiman Long,
	Dan Williams, Will Deacon, Thomas Gleixner, linux-arm-kernel
In-Reply-To: <20190923125851.cykw55jpqwlerjrz@pathway.suse.cz>

On (09/23/19 14:58), Petr Mladek wrote:
> 
> If I understand it correctly then this is the re-appearing problem.
> The only systematic solution with the current approach is to
> take port->lock in printk_safe/printk_deferred context.

It probably is.
We have a number of reverse paths. TTY invokes MM under port->lock,
UART invokes TTY under port->lock, MM invokes UART under zone->lock,
UART invokes sched under port->lock, shced invokes UART, UART invokes
UART under port->lock (!), and so on.

> But this is a massive change that almost nobody wants. Instead,
> we want the changes that were discussed on Plumbers.
>
> Now, the question is what to do with existing kernels. There were
> several lockdep reports. And I am a bit lost. Did anyone seen
> real deadlocks or just the lockdep reports?

I think so. Qian Cai mentioned "a hang" in one of his reports
(was it unseeded random()?). I'll send a formal patch maybe,
since there were no objections.

> To be clear. I would feel more comfortable when there are no
> deadlocks. But I also do not want to invest too much time
> into old kernels. All these problems were there for ages.
> We could finally see them because lockdep was enabled in printk()
> thanks to printk_safe.

True.
Everyone was so much happier when printk() used to do

       lockdep_off();
       call_console_drivers();
       lockdep_on();

Now we can have lockdep and RCU checks enabled, yet somehow
printk_safe is still "a terrible thing" :)

	-ss

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

^ permalink raw reply

* [PATCH V8 2/2] mailbox: introduce ARM SMC based mailbox
From: Peng Fan @ 2019-09-24  1:14 UTC (permalink / raw)
  To: robh+dt@kernel.org, mark.rutland@arm.com,
	jassisinghbrar@gmail.com, sudeep.holla@arm.com,
	andre.przywara@arm.com, f.fainelli@gmail.com
  Cc: devicetree@vger.kernel.org, Peng Fan,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <1569287538-10854-1-git-send-email-peng.fan@nxp.com>

From: Peng Fan <peng.fan@nxp.com>

This mailbox driver implements a mailbox which signals transmitted data
via an ARM smc (secure monitor call) instruction. The mailbox receiver
is implemented in firmware and can synchronously return data when it
returns execution to the non-secure world again.
An asynchronous receive path is not implemented.
This allows the usage of a mailbox to trigger firmware actions on SoCs
which either don't have a separate management processor or on which such
a core is not available. A user of this mailbox could be the SCP
interface.

Modified from Andre Przywara's v2 patch
https://lore.kernel.org/patchwork/patch/812999/

Cc: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mailbox/Kconfig                |   7 ++
 drivers/mailbox/Makefile               |   2 +
 drivers/mailbox/arm-smc-mailbox.c      | 168 +++++++++++++++++++++++++++++++++
 include/linux/mailbox/arm-smccc-mbox.h |  22 +++++
 4 files changed, 199 insertions(+)
 create mode 100644 drivers/mailbox/arm-smc-mailbox.c
 create mode 100644 include/linux/mailbox/arm-smccc-mbox.h

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ab4eb750bbdd..7707ee26251a 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -16,6 +16,13 @@ config ARM_MHU
 	  The controller has 3 mailbox channels, the last of which can be
 	  used in Secure mode only.
 
+config ARM_SMC_MBOX
+	tristate "Generic ARM smc mailbox"
+	depends on OF && HAVE_ARM_SMCCC
+	help
+	  Generic mailbox driver which uses ARM smc calls to call into
+	  firmware for triggering mailboxes.
+
 config IMX_MBOX
 	tristate "i.MX Mailbox"
 	depends on ARCH_MXC || COMPILE_TEST
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index c22fad6f696b..93918a84c91b 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -7,6 +7,8 @@ obj-$(CONFIG_MAILBOX_TEST)	+= mailbox-test.o
 
 obj-$(CONFIG_ARM_MHU)	+= arm_mhu.o
 
+obj-$(CONFIG_ARM_SMC_MBOX)	+= arm-smc-mailbox.o
+
 obj-$(CONFIG_IMX_MBOX)	+= imx-mailbox.o
 
 obj-$(CONFIG_ARMADA_37XX_RWTM_MBOX)	+= armada-37xx-rwtm-mailbox.o
diff --git a/drivers/mailbox/arm-smc-mailbox.c b/drivers/mailbox/arm-smc-mailbox.c
new file mode 100644
index 000000000000..664c8b4a0ed0
--- /dev/null
+++ b/drivers/mailbox/arm-smc-mailbox.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016,2017 ARM Ltd.
+ * Copyright 2019 NXP
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox/arm-smccc-mbox.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+struct arm_smc_chan_data {
+	unsigned int function_id;
+};
+
+typedef unsigned long (smc_mbox_fn)(unsigned int, unsigned long,
+				    unsigned long, unsigned long,
+				    unsigned long, unsigned long,
+				    unsigned long);
+static smc_mbox_fn *invoke_smc_mbox_fn;
+
+static int arm_smc_send_data(struct mbox_chan *link, void *data)
+{
+	struct arm_smc_chan_data *chan_data = link->con_priv;
+	struct arm_smccc_mbox_cmd *cmd = data;
+	unsigned long ret;
+	u32 function_id;
+
+	function_id = chan_data->function_id;
+	if (!function_id)
+		function_id = cmd->function_id;
+
+	if (ARM_SMCCC_IS_64(function_id)) {
+		ret = invoke_smc_mbox_fn(function_id, cmd->args_smccc64[0],
+					 cmd->args_smccc64[1],
+					 cmd->args_smccc64[2],
+					 cmd->args_smccc64[3],
+					 cmd->args_smccc64[4],
+					 cmd->args_smccc64[5]);
+	} else {
+		ret = invoke_smc_mbox_fn(function_id, cmd->args_smccc32[0],
+					 cmd->args_smccc32[1],
+					 cmd->args_smccc32[2],
+					 cmd->args_smccc32[3],
+					 cmd->args_smccc32[4],
+					 cmd->args_smccc32[5]);
+	}
+
+	mbox_chan_received_data(link, (void *)ret);
+
+	return 0;
+}
+
+static unsigned long __invoke_fn_hvc(unsigned int function_id,
+				     unsigned long arg0, unsigned long arg1,
+				     unsigned long arg2, unsigned long arg3,
+				     unsigned long arg4, unsigned long arg5)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_hvc(function_id, arg0, arg1, arg2, arg3, arg4,
+		      arg5, 0, &res);
+	return res.a0;
+}
+
+static unsigned long __invoke_fn_smc(unsigned int function_id,
+				     unsigned long arg0, unsigned long arg1,
+				     unsigned long arg2, unsigned long arg3,
+				     unsigned long arg4, unsigned long arg5)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(function_id, arg0, arg1, arg2, arg3, arg4,
+		      arg5, 0, &res);
+	return res.a0;
+}
+
+static const struct mbox_chan_ops arm_smc_mbox_chan_ops = {
+	.send_data	= arm_smc_send_data,
+};
+
+static struct mbox_chan *
+arm_smc_mbox_of_xlate(struct mbox_controller *mbox,
+		      const struct of_phandle_args *sp)
+{
+	return mbox->chans;
+}
+
+static int arm_smc_mbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mbox_controller *mbox;
+	struct arm_smc_chan_data *chan_data;
+	int ret;
+	u32 function_id = 0;
+
+	if (of_device_is_compatible(dev->of_node, "arm,smc-mbox"))
+		invoke_smc_mbox_fn = __invoke_fn_smc;
+	else
+		invoke_smc_mbox_fn = __invoke_fn_hvc;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+
+	mbox->of_xlate = arm_smc_mbox_of_xlate;
+	mbox->num_chans = 1;
+	mbox->chans = devm_kzalloc(dev, sizeof(*mbox->chans), GFP_KERNEL);
+	if (!mbox->chans)
+		return -ENOMEM;
+
+	chan_data = devm_kzalloc(dev, sizeof(*chan_data), GFP_KERNEL);
+	if (!chan_data)
+		return -ENOMEM;
+
+	of_property_read_u32(dev->of_node, "arm,func-id", &function_id);
+	chan_data->function_id = function_id;
+
+	mbox->chans->con_priv = chan_data;
+
+	mbox->txdone_poll = false;
+	mbox->txdone_irq = false;
+	mbox->ops = &arm_smc_mbox_chan_ops;
+	mbox->dev = dev;
+
+	platform_set_drvdata(pdev, mbox);
+
+	ret = devm_mbox_controller_register(dev, mbox);
+	if (ret)
+		return ret;
+
+	dev_info(dev, "ARM SMC mailbox enabled.\n");
+
+	return ret;
+}
+
+static int arm_smc_mbox_remove(struct platform_device *pdev)
+{
+	struct mbox_controller *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(mbox);
+	return 0;
+}
+
+static const struct of_device_id arm_smc_mbox_of_match[] = {
+	{ .compatible = "arm,smc-mbox", },
+	{ .compatible = "arm,hvc-mbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, arm_smc_mbox_of_match);
+
+static struct platform_driver arm_smc_mbox_driver = {
+	.driver = {
+		.name = "arm-smc-mbox",
+		.of_match_table = arm_smc_mbox_of_match,
+	},
+	.probe		= arm_smc_mbox_probe,
+	.remove		= arm_smc_mbox_remove,
+};
+module_platform_driver(arm_smc_mbox_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("Generic ARM smc mailbox driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mailbox/arm-smccc-mbox.h b/include/linux/mailbox/arm-smccc-mbox.h
new file mode 100644
index 000000000000..140045644d82
--- /dev/null
+++ b/include/linux/mailbox/arm-smccc-mbox.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_ARM_SMCCC_MBOX_H_
+#define _LINUX_ARM_SMCCC_MBOX_H_
+
+/**
+ * struct arm_smccc_mbox_cmd - ARM SMCCC message structure
+ * @function_id:	function id passed from client, If mbox
+ *			DT has arm,func-id property, the driver will use
+ *			that one.
+ * @args_smccc32/64:	actual usage of registers is up to the protocol
+ *			(within the SMCCC limits)
+ */
+struct arm_smccc_mbox_cmd {
+	unsigned int function_id;
+	union {
+		u32 args_smccc32[6];
+		u64 args_smccc64[6];
+	};
+};
+
+#endif /* _LINUX_ARM_SMCCC_MBOX_H_ */
-- 
2.16.4


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

^ permalink raw reply related

* [PATCH V8 1/2] dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
From: Peng Fan @ 2019-09-24  1:14 UTC (permalink / raw)
  To: robh+dt@kernel.org, mark.rutland@arm.com,
	jassisinghbrar@gmail.com, sudeep.holla@arm.com,
	andre.przywara@arm.com, f.fainelli@gmail.com
  Cc: devicetree@vger.kernel.org, Peng Fan,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <1569287538-10854-1-git-send-email-peng.fan@nxp.com>

From: Peng Fan <peng.fan@nxp.com>

The ARM SMC/HVC mailbox binding describes a firmware interface to trigger
actions in software layers running in the EL2 or EL3 exception levels.
The term "ARM" here relates to the SMC instruction as part of the ARM
instruction set, not as a standard endorsed by ARM Ltd.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/mailbox/arm-smc.yaml       | 95 ++++++++++++++++++++++
 1 file changed, 95 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.yaml b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
new file mode 100644
index 000000000000..8764ad2726b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/arm-smc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM SMC Mailbox Interface
+
+maintainers:
+  - Peng Fan <peng.fan@nxp.com>
+
+description: |
+  This mailbox uses the ARM smc (secure monitor call) or hvc (hypervisor
+  call) instruction to trigger a mailbox-connected activity in firmware,
+  executing on the very same core as the caller. The value of r0/w0/x0
+  the firmware returns after the smc call is delivered as a received
+  message to the mailbox framework, so synchronous communication can be
+  established. The exact meaning of the action the mailbox triggers as
+  well as the return value is defined by their users and is not subject
+  to this binding.
+
+  One example use case of this mailbox is the SCMI interface, which uses
+  shared memory to transfer commands and parameters, and a mailbox to
+  trigger a function call. This allows SoCs without a separate management
+  processor (or when such a processor is not available or used) to use
+  this standardized interface anyway.
+
+  This binding describes no hardware, but establishes a firmware interface.
+  Upon receiving an SMC using the described SMC function identifier, the
+  firmware is expected to trigger some mailbox connected functionality.
+  The communication follows the ARM SMC calling convention.
+  Firmware expects an SMC function identifier in r0 or w0. The supported
+  identifier are passed from consumers, or listed in the the arm,func-id
+  property as described below. The firmware can return one value in
+  the first SMC result register, it is expected to be an error value,
+  which shall be propagated to the mailbox client.
+
+  Any core which supports the SMC or HVC instruction can be used, as long
+  as a firmware component running in EL3 or EL2 is handling these calls.
+
+properties:
+  compatible:
+    oneOf:
+      - description:
+          For implementations using ARM SMC instruction.
+        const: arm,smc-mbox
+
+      - description:
+          For implementations using ARM HVC instruction.
+        const: arm,hvc-mbox
+
+  "#mbox-cells":
+    const: 0
+
+  arm,func-id:
+    description: |
+      An single 32-bit value specifying the function ID used by the mailbox.
+      The function ID follows the ARM SMC calling convention standard.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - "#mbox-cells"
+
+examples:
+  - |
+    sram@93f000 {
+      compatible = "mmio-sram";
+      reg = <0x0 0x93f000 0x0 0x1000>;
+      #address-cells = <1>;
+      #size-cells = <1>;
+      ranges = <0x0 0x93f000 0x1000>;
+
+      cpu_scp_lpri: scp-shmem@0 {
+        compatible = "arm,scmi-shmem";
+        reg = <0x0 0x200>;
+      };
+    };
+
+    smc_tx_mbox: tx_mbox {
+      #mbox-cells = <0>;
+      compatible = "arm,smc-mbox";
+      arm,func-id = <0xc20000fe>;
+    };
+
+    firmware {
+      scmi {
+        compatible = "arm,scmi";
+        mboxes = <&smc_tx_mbox>;
+        mbox-names = "tx";
+        shmem = <&cpu_scp_lpri>;
+      };
+    };
+
+...
-- 
2.16.4


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

^ permalink raw reply related

* [PATCH V8 0/2] mailbox: arm: introduce smc triggered mailbox
From: Peng Fan @ 2019-09-24  1:14 UTC (permalink / raw)
  To: robh+dt@kernel.org, mark.rutland@arm.com,
	jassisinghbrar@gmail.com, sudeep.holla@arm.com,
	andre.przywara@arm.com, f.fainelli@gmail.com
  Cc: devicetree@vger.kernel.org, Peng Fan,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, dl-linux-imx

From: Peng Fan <peng.fan@nxp.com>

V8:
Add missed arm-smccc-mbox.h

V7:
Typo fix
#mbox-cells changed to 0
Add a new header file arm-smccc-mbox.h
Use ARM_SMCCC_IS_64

Andre,
  The function_id is still kept in arm_smccc_mbox_cmd, because arm,func-id
property is optional, so clients could pass function_id to mbox driver.

V6:
Switch to per-channel a mbox controller
Drop arm,num-chans, transports, method
Add arm,hvc-mbox compatible
Fix smc/hvc args, drop client id and use correct type.
https://patchwork.kernel.org/cover/11146641/

V5:
yaml fix
https://patchwork.kernel.org/cover/11117741/

V4:
yaml fix for num-chans in patch 1/2.
https://patchwork.kernel.org/cover/11116521/

V3:
Drop interrupt
Introduce transports for mem/reg usage
Add chan-id for mem usage
Convert to yaml format
https://patchwork.kernel.org/cover/11043541/

V2:
This is a modified version from Andre Przywara's patch series
https://lore.kernel.org/patchwork/cover/812997/.
The modification are mostly:
Introduce arm,num-chans
Introduce arm_smccc_mbox_cmd
txdone_poll and txdone_irq are both set to false
arm,func-ids are kept, but as an optional property.
Rewords SCPI to SCMI, because I am trying SCMI over SMC, not SCPI.
Introduce interrupts notification.

[1] is a draft implementation of i.MX8MM SCMI ATF implementation that
use smc as mailbox, power/clk is included, but only part of clk has been
implemented to work with hardware, power domain only supports get name
for now.

The traditional Linux mailbox mechanism uses some kind of dedicated hardware
IP to signal a condition to some other processing unit, typically a dedicated
management processor.
This mailbox feature is used for instance by the SCMI protocol to signal a
request for some action to be taken by the management processor.
However some SoCs does not have a dedicated management core to provide
those services. In order to service TEE and to avoid linux shutdown
power and clock that used by TEE, need let firmware to handle power
and clock, the firmware here is ARM Trusted Firmware that could also
run SCMI service.

The existing SCMI implementation uses a rather flexible shared memory
region to communicate commands and their parameters, it still requires a
mailbox to actually trigger the action.

This patch series provides a Linux mailbox compatible service which uses
smc calls to invoke firmware code, for instance taking care of SCMI requests.
The actual requests are still communicated using the standard SCMI way of
shared memory regions, but a dedicated mailbox hardware IP can be replaced via
this new driver.

This simple driver uses the architected SMC calling convention to trigger
firmware services, also allows for using "HVC" calls to call into hypervisors
or firmware layers running in the EL2 exception level.

Patch 1 contains the device tree binding documentation, patch 2 introduces
the actual mailbox driver.

Please note that this driver just provides a generic mailbox mechanism,
It could support synchronous TX/RX, or synchronous TX with asynchronous
RX. And while providing SCMI services was the reason for this exercise,
this driver is in no way bound to this use case, but can be used generically
where the OS wants to signal a mailbox condition to firmware or a
hypervisor.
Also the driver is in no way meant to replace any existing firmware
interface, but actually to complement existing interfaces.

[1] https://github.com/MrVan/arm-trusted-firmware/tree/scmi


Peng Fan (2):
  dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
  mailbox: introduce ARM SMC based mailbox

 .../devicetree/bindings/mailbox/arm-smc.yaml       |  95 ++++++++++++
 drivers/mailbox/Kconfig                            |   7 +
 drivers/mailbox/Makefile                           |   2 +
 drivers/mailbox/arm-smc-mailbox.c                  | 168 +++++++++++++++++++++
 include/linux/mailbox/arm-smccc-mbox.h             |  22 +++
 5 files changed, 294 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.yaml
 create mode 100644 drivers/mailbox/arm-smc-mailbox.c
 create mode 100644 include/linux/mailbox/arm-smccc-mbox.h

-- 
2.16.4


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

^ permalink raw reply

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
From: Vasily Khoruzhick @ 2019-09-23 23:55 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux
In-Reply-To: <CA+E=qVegU8M09tmbxGUaBSoueGU6PRsAtr9XWrc8V8HnCPjULg@mail.gmail.com>

On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>
> On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > > >>
> > > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > >>>>
> > > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > > >>>>
> > > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > > >>>
> > > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > > >>
> > > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > > >>
> > > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > > >>
> > > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > > >
> > > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > > without interrupts?
> > > > > > > > > >
> > > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > > it'll still do the job.
> > > > > > > > >
> > > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > > stat'"
> > > > > > > >
> > > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > > 0x09010000 register?
> > > > > > >
> > > > > > > What register is that and how do I check it?
> > > > > >
> > > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > > Debug Reset.
> > > > > >
> > > > > > And if you have busybox, you can use devmem.
> > > > >
> > > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > > manual, and particular register you're interested in is at 0x01700080,
> > > > > its value is 0x1110110F.
> > > > >
> > > > > Bits 16-19 are not defined in user manual and are not set.
> > > >
> > > > Sorry, I somehow thought this was for the H6...
> > > >
> > > > I don't have any idea then :/
> > >
> > > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > > node is dropped, but they don't work if PMU node is present (even with
> > > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > > working instead of 'perf stat'
> >
> > Well, it doesn't work so we should just remove the node, and if
> > someone wants it back, they should figure it out.
>
> Hey Maxime,
>
> So can you merge this patch?

Added new Maxime's email to CC

> > Maxime
> >
> > --
> > Maxime Ripard, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com

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

^ permalink raw reply

* Re: [PATCH] seccomp: remove unused arg from secure_computing()
From: Kees Cook @ 2019-09-23 23:51 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-s390, Will Drewry, Parisc List, X86 ML, linux-um, LKML,
	Oleg Nesterov, Andy Lutomirski, Christian Brauner,
	Thomas Gleixner, linux-arm-kernel
In-Reply-To: <20190923193446.GL15355@zn.tnic>

On Mon, Sep 23, 2019 at 09:34:46PM +0200, Borislav Petkov wrote:
> On Mon, Sep 23, 2019 at 11:41:59AM -0700, Andy Lutomirski wrote:
> > On Mon, Sep 23, 2019 at 2:49 AM Borislav Petkov <bp@alien8.de> wrote:
> > >
> > > On Fri, Sep 20, 2019 at 03:19:09PM +0200, Christian Brauner wrote:
> > > > While touching seccomp code I realized that the struct seccomp_data
> > > > argument to secure_computing() seems to be unused by all current
> > > > callers. So let's remove it unless there is some subtlety I missed.
> > > > Note, I only tested this on x86.
> > >
> > > What was amluto thinking in
> > >
> > > 2f275de5d1ed ("seccomp: Add a seccomp_data parameter secure_computing()")
> > 
> > IIRC there was a period of time in which x86 used secure_computing()
> > for normal syscalls, and it was a good deal faster to have the arch
> > code supply seccomp_data.  x86 no longer works like this, and syscalls
> > aren't fast anymore ayway :(
> 
> Uhuh, thanks Andy.
> 
> Christian, pls add that piece of history to the commit message.

Yeah, this is just left-over from the "two phase" seccomp optimization
that was removed a while back. I'll take this clean up into the seccomp
tree. Thanks!

-- 
Kees Cook

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

^ permalink raw reply

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
From: Vasily Khoruzhick @ 2019-09-23 23:51 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux
In-Reply-To: <20190813053905.hu2hyi7fah2vujzz@flea>

On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > >
> > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > >
> > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > >>
> > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > >>>>
> > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > >>>>
> > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > >>>
> > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > >>
> > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > >>
> > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > >>
> > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > >
> > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > without interrupts?
> > > > > > > > >
> > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > it'll still do the job.
> > > > > > > >
> > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > stat'"
> > > > > > >
> > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > 0x09010000 register?
> > > > > >
> > > > > > What register is that and how do I check it?
> > > > >
> > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > Debug Reset.
> > > > >
> > > > > And if you have busybox, you can use devmem.
> > > >
> > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > manual, and particular register you're interested in is at 0x01700080,
> > > > its value is 0x1110110F.
> > > >
> > > > Bits 16-19 are not defined in user manual and are not set.
> > >
> > > Sorry, I somehow thought this was for the H6...
> > >
> > > I don't have any idea then :/
> >
> > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > node is dropped, but they don't work if PMU node is present (even with
> > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > working instead of 'perf stat'
>
> Well, it doesn't work so we should just remove the node, and if
> someone wants it back, they should figure it out.

Hey Maxime,

So can you merge this patch?

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

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

^ permalink raw reply

* Re: [PATCH] PM / devfreq: Lock devfreq in trans_stat_show
From: Matthias Kaehlcke @ 2019-09-23 23:34 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Artur Świgoń, linux-pm, Krzysztof Kozlowski,
	Lukasz Luba, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	NXP Linux Team, Georgi Djakov, linux-arm-kernel
In-Reply-To: <714675448e7fbf3c930b0dca6fbe54fa5f87211b.1569256001.git.leonard.crestez@nxp.com>

On Mon, Sep 23, 2019 at 07:27:27PM +0300, Leonard Crestez wrote:
> There is no locking in this sysfs show function so stats printing can
> race with a devfreq_update_status called as part of freq switching or
> with initialization.
> 
> Also add an assert in devfreq_update_status to make it clear that lock
> must be held by caller.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/devfreq/devfreq.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> Changes since v1:
> * Split from series: low-priority bugfix not strictly required for PM QoS
> * Only keep lock during update, release before sprintf
> Link to v1: https://patchwork.kernel.org/patch/11149493/
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 4c58fbf7d4e4..00fc23fea5b2 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -206,10 +206,11 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
>  {
>  	int lev, prev_lev, ret = 0;
>  	unsigned long cur_time;
>  
>  	cur_time = jiffies;
> +	lockdep_assert_held(&devfreq->lock);
>  
>  	/* Immediately exit if previous_freq is not initialized yet. */
>  	if (!devfreq->previous_freq)
>  		goto out;
>  
> @@ -1507,16 +1508,22 @@ static ssize_t trans_stat_show(struct device *dev,
>  	struct devfreq *devfreq = to_devfreq(dev);
>  	ssize_t len;
>  	int i, j;
>  	unsigned int max_state = devfreq->profile->max_state;
>  
> -	if (!devfreq->stop_polling &&
> -			devfreq_update_status(devfreq, devfreq->previous_freq))
> -		return 0;
>  	if (max_state == 0)
>  		return sprintf(buf, "Not Supported.\n");
>  
> +	/* lock and update */

nit: the comment doesn't add much value, I'd suggest to remove it.

Forgot to comment on this earlier. My Reviewed-by tag still stands, up
to you if you want to re-spin.

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

^ permalink raw reply

* i.MX6Q Etnaviv errors
From: Adam Ford @ 2019-09-23 22:49 UTC (permalink / raw)
  To: arm-soc, etnaviv, dri-devel

I tired to run some tests on 5.4 and I was getting errors.  I went all
the way back to 5.0 and I get errors. 4.19 seems to work just fine.

I was curious to know if anyone is seeing errors.  I'll bisect
tomorrow, but I thought I'd ask before I got too far.

# LIBGL_DEBUG=verbose glmark2-es2-drm

drmOpenDevice: node name is /dev/dri/card0
drmOpenDevice: open result is 3, (OK)
drmOpenDevice: node name is /dev/dri/card0
drmOpenDevice: open result is 3, (OK)
drmOpenDevice: node name is /dev/dri/card1
drmOpenDevice: open result is 3, (OK)
drmOpenDevice: node name is /dev/dri/card2
drmOpenDevice: node name is /dev/dri/card3
drmOpenDevice: node name is /dev/dri/card4
drmOpenDevice: node name is /dev/dri/card5
drmOpenDevice: node name is /dev/dri/card6
drmOpenDevice: node name is /dev/dri/card7
drmOpenDevice: node name is /dev/dri/card8
drmOpenDevice: node name is /dev/dri/card9
drmOpenDevice: node name is /dev/dri/card10
drmOpenDevice: node name is /dev/dri/card11
drmOpenDevice: node name is /dev/dri/card12
drmOpenDevice: node name is /dev/dri/card13
drmOpenDevice: node name is /dev/dri/card14
drmOpenDevice: node name is /dev/dri/card15
drmOpenDevice: node name is /dev/dri/card0
drmOpenDevice: open result is 3, (OK)
drmOpenDevice: node name is /dev/dri/card0
drmOpenDevice: open result is 3, (OK)
drmGetBusid returned ''
libGL: Can't open configuration file /etc/drirc: No such file or directory.
libGL: Can't open configuration file /root/.drirc: No such file or directory.
MESA-LOADER: failed to open imx-drm (search paths /usr/lib/dri)
failed to load driver: imx-drm
libGL: Can't open configuration file /etc/drirc: No such file or directory.
libGL: Can't open configuration file /root/.drirc: No such file or directory.
libGL: Can't open configuration file /etc/drirc: No such file or directory.
libGL: Can't open configuration file /root/.drirc: No such file or directory.
Error: eglCreateWindowSurface failed with error: 0x3009
Error: eglCreateWindowSurface failed with error: 0x3009
Error: CanvasGeneric: Invalid EGL state
Error: main: Could not initialize canvas

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

^ permalink raw reply

* Re: [PATCH] arm64: dts: renesas: Add /soc dma-ranges
From: Rob Herring @ 2019-09-23 22:33 UTC (permalink / raw)
  To: Marek Vašut
  Cc: devicetree, Geert Uytterhoeven, Wolfram Sang,
	open list:MEDIA DRIVERS FOR RENESAS - FCP,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Marek Vasut
In-Reply-To: <CAL_JsqL47dQT-P78j4Ph61fsgA45Ha0AJjDajiMk52yFj++s+g@mail.gmail.com>

On Fri, Sep 13, 2019 at 10:14 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sat, Sep 7, 2019 at 5:16 PM <marek.vasut@gmail.com> wrote:
> >
> > From: Marek Vasut <marek.vasut+renesas@gmail.com>
> >
> > Add dma-ranges property into /soc node to describe the DMA capabilities
> > of the bus. This is currently needed to translate PCI DMA ranges, which
> > are limited to 32bit addresses.
>
> FYI, I've started working on this problem and issues around
> dma-ranges/dma_mask. Hopefully I'll get some patches out next week.

I've pushed out a branch here:

git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dma-masks

Can you test it on Renesas. I don't have a real platform having the issue.

Rob

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

^ permalink raw reply

* Re: [PATCH V8 7/8] Docs: misc: xilinx_sdfec: Add documentation
From: Kees Cook @ 2019-09-23 21:51 UTC (permalink / raw)
  To: Dragan Cvetic
  Cc: arnd, gregkh, michal.simek, linux-kernel, Derek Kiernan,
	linux-arm-kernel
In-Reply-To: <1562458542-457448-8-git-send-email-dragan.cvetic@xilinx.com>

On Sun, Jul 07, 2019 at 01:15:41AM +0100, Dragan Cvetic wrote:
> Add SD-FEC driver documentation.
> 
> Signed-off-by: Derek Kiernan <derek.kiernan@xilinx.com>
> Signed-off-by: Dragan Cvetic <dragan.cvetic@xilinx.com>
> ---
>  Documentation/misc-devices/index.rst | 1 +
>  1 file changed, 1 insertion(+)

Hi! It looks like Documentation/misc-devices/xilinx_sdfec.rst went
missing in this revision (and when committed upstream). It looks like it
was present at least in the V7 of the series:
https://lore.kernel.org/lkml/1560274185-264438-11-git-send-email-dragan.cvetic@xilinx.com/

-Kees

> 
> diff --git a/Documentation/misc-devices/index.rst b/Documentation/misc-devices/index.rst
> index a57f92d..f11c5da 100644
> --- a/Documentation/misc-devices/index.rst
> +++ b/Documentation/misc-devices/index.rst
> @@ -20,3 +20,4 @@ fit into other categories.
>     isl29003
>     lis3lv02d
>     max6875
> +   xilinx_sdfec
> -- 
> 2.7.4
> 

-- 
Kees Cook

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

^ permalink raw reply

* Re: [PATCH v7 5/6] PM / devfreq: Add PM QoS support
From: Matthias Kaehlcke @ 2019-09-23 21:42 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
	Viresh Kumar, NXP Linux Team, Krzysztof Kozlowski, Lukasz Luba,
	Chanwoo Choi, Kyungmin Park, MyungJoo Ham, Alexandre Bailon,
	Martin Kepplinger, Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <e9868310f9543b4f4a6c7bbe5d4d015da9a0e71d.1569272883.git.leonard.crestez@nxp.com>

On Tue, Sep 24, 2019 at 12:10:33AM +0300, Leonard Crestez wrote:
> Register notifiers with the PM QoS framework in order to respond to
> requests for DEV_PM_QOS_MIN_FREQUENCY and DEV_PM_QOS_MAX_FREQUENCY.
> 
> No notifiers are added by this patch but PM QoS constraints can be
> imposed externally (for example from other devices).
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/devfreq/devfreq.c | 76 +++++++++++++++++++++++++++++++++++++++
>  include/linux/devfreq.h   |  5 +++
>  2 files changed, 81 insertions(+)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 7f152a582e78..9887408f23bb 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -22,17 +22,20 @@
>  #include <linux/platform_device.h>
>  #include <linux/list.h>
>  #include <linux/printk.h>
>  #include <linux/hrtimer.h>
>  #include <linux/of.h>
> +#include <linux/pm_qos.h>
>  #include "governor.h"
>  
>  #define HZ_PER_KHZ 1000
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/devfreq.h>
>  
> +#define HZ_PER_KHZ	1000
> +
>  static struct class *devfreq_class;
>  
>  /*
>   * devfreq core provides delayed work based load monitoring helper
>   * functions. Governors can use these or can implement their own
> @@ -123,10 +126,16 @@ static void devfreq_get_freq_range(struct devfreq *devfreq,
>  	} else {
>  		*min_freq = freq_table[devfreq->profile->max_state - 1];
>  		*max_freq = freq_table[0];
>  	}
>  
> +	/* constraints from PM QoS */
> +	*min_freq = max(*min_freq, HZ_PER_KHZ * (unsigned long)dev_pm_qos_read_value(
> +				devfreq->dev.parent, DEV_PM_QOS_MIN_FREQUENCY));
> +	*max_freq = min(*max_freq, HZ_PER_KHZ * (unsigned long)dev_pm_qos_read_value(
> +				devfreq->dev.parent, DEV_PM_QOS_MAX_FREQUENCY));
> +
>  	/* constraints from sysfs */
>  	*min_freq = max(*min_freq, devfreq->min_freq);
>  	*max_freq = min(*max_freq, devfreq->max_freq);
>  
>  	/* constraints from OPP interface */
> @@ -605,10 +614,53 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
>  	mutex_unlock(&devfreq->lock);
>  
>  	return ret;
>  }
>  
> +/**
> + * devfreq_qos_notifier_call() - Common handler for QoS constraints.
> + * @devfreq:    the devfreq instance.
> + */
> +static int devfreq_qos_notifier_call(struct devfreq *devfreq)
> +{
> +	int err;
> +
> +	mutex_lock(&devfreq->lock);
> +	err = update_devfreq(devfreq);
> +	mutex_unlock(&devfreq->lock);
> +	if (err)
> +		dev_err(&devfreq->dev, "dvfs for QoS constraints"
> +				" failed with (%d) error\n", err);

nit: DVFS. devfreq_monitor() also uses the lower-case acronym though, so
you can claim this is consistent :)

I'd prefer to spare you another trivial re-spin, but unfortunately
breaking the log message into multiple lines is a coding style
violation:

Documentation/process/coding-style.rst
  2) Breaking long lines and strings

  However, never break user-visible strings such as printk messages,
  because that breaks the ability to grep for them.


With that fixed:

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

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

^ permalink raw reply

* Re: [PATCH] clk: at91: avoid sleeping early
From: Alexandre Belloni @ 2019-09-23 21:34 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, linux-kernel, Uwe Kleine-König,
	Claudiu Beznea, linux-clk, linux-arm-kernel
In-Reply-To: <20190923165848.3108A20882@mail.kernel.org>

On 23/09/2019 09:58:47-0700, Stephen Boyd wrote:
> Quoting Alexandre Belloni (2019-09-20 08:39:06)
> > It is not allowed to sleep to early in the boot process and this may lead
> > to kernel issues if the bootloader didn't prepare the slow clock and main
> > clock.
> > 
> > This results in the following error and dump stack on the AriettaG25:
> >    bad: scheduling from the idle thread!
> > 
> > Ensure it is possible to sleep, else simply have a delay.
> > 
> > Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > ---
> > 
> > Note that this was already discussed a while ago and Arnd said this approach was
> > reasonable:
> >   https://lore.kernel.org/lkml/6120818.MyeJZ74hYa@wuerfel/
> 
> Does this need a Fixes: tag?
> 

I'm not sure how far this can get backported

Fixes: 80eded6ce8bb ("clk: at91: add slow clks driver")


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

^ permalink raw reply

* Re: [RFCv4 0/7] interconnect: Add imx support via devfreq
From: Leonard Crestez @ 2019-09-23 21:22 UTC (permalink / raw)
  To: Martin Kepplinger, Jacky Bai, Anson Huang
  Cc: Mark Rutland, Artur Świgoń, Peng Fan, Viresh Kumar,
	Michael Turquette, Alexandre Bailon, Saravana Kannan,
	Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham, dl-linux-imx,
	devicetree@vger.kernel.org, linux-pm@vger.kernel.org, Rob Herring,
	linux-arm-kernel@lists.infradead.org, Aisheng Dong, Stephen Boyd,
	Kyungmin Park, kernel@pengutronix.de, Fabio Estevam, Shawn Guo,
	Georgi Djakov
In-Reply-To: <4440b392-d968-4a54-2ea0-ffd5beba02d1@posteo.de>

On 21.09.2019 09:07, Martin Kepplinger wrote:
> On 23.08.19 16:36, Leonard Crestez wrote:
>> This series add imx support for interconnect via devfreq: the ICC
>> framework is used to aggregate requests from devices and then those are
>> converted to DEV_PM_QOS_MIN_FREQUENCY requests for devfreq.
>>
>> The devfreq parts are posted separately, this series only intersects in
>> devicetree.
>>
>> Since there is no single devicetree node that can represent the "interconnect"
>> new API is added to allow individual devfreq nodes to act as parsing proxies
>> all mapping to a single soc-level icc provider. This is still RFC
>> because this
>>
>> The rest of the changes are small and deal with review comments.

> on imx8mq, probe() fails:
> 
> [    1.082847] imx-ddrc-devfreq 3d400000.dram-controller: failed to init
> firmware freq info: -19
> [    1.091434] imx-ddrc-devfreq: probe of 3d400000.dram-controller
> rejects match -19
> 
> in imx_ddrc_init_freq_info()'s check:
> 
> if (priv->freq_count <= 0 || priv->freq_count > IMX_DDRC_MAX_FREQ_COUNT)
> 
> That would indicate that I'm missing something in ATF? I'm pretty sure
> I'm running your tree though.

What is your board and uboot version? I tested on imx8mq-evk (SOC Rev 
B1) with NXP uboot. For example this uboot release works:

https://source.codeaurora.org/external/imx/uboot-imx/log/?h=imx_v2019.04_4.19.35_1.0.0

It is uboot which trains DDR for multiple frequencies and then passes 
that info to ATF. I'm not sure about the steps required to enable this 
for 3rd-party boards, should be same as for busfreq from NXP tree.

Getting this to work on a 3rd-party board would be interesting.

> Does anything come to your mind what I might be doing wrong? Am I
> running your "correct" linux tree? Remember I'm on imx8mq, so I don't
> exactly test this RFC of yours.
> 
> Also, how are your plans to rebase / update your ATF tree?

The ATF changes will show up in a future release of NXP ATF branch, when 
that happens I will drop my branch. NXP ATF releases are on CAF:

     https://source.codeaurora.org/external/imx/imx-atf/

> And since there's a lot in there: what additions are necessary for this
> devfreq to work?

devfreq imx support here: https://patchwork.kernel.org/cover/11104113/
Interconnect support also needs PM QoS support for devfreq:

     https://patchwork.kernel.org/cover/11157649/

> Lastly, how do you test? Is /sys/class/devfreq supposted to get populated?

Yes, and only the devfreq patches are required for that.

# cat /sys/class/devfreq/devfreq0/available_frequencies
25000000 100000000 800000000
# cat /sys/class/devfreq/devfreq0/cur_freq
800000000

You should be able to control frequencies manually with the userspace 
governor:
# echo "userspace" > /sys/class/devfreq/devfreq0/governor
# echo "25000000" > /sys/class/devfreq/devfreq0/userspace/set_freq

This series allows devices to request guaranteed bandwidth for 
themselves through the interconnect subsystem, without it DRAM freq will 
still switch but you'll have problems like display corruption as it 
turns on before freq goes up. You can check that probe worked by doing

# cat /sys/kernel/debug/interconnect/interconnect_summary

--
Regards,
Leonard

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

^ permalink raw reply

* Applied "ASoC: wm8994: Do not register inapplicable controls for WM1811" to the asoc tree
From: Mark Brown @ 2019-09-23 21:22 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: devicetree, alsa-devel, linux-samsung-soc, ckeepax, b.zolnierkie,
	sbkim73, patches, lgirdwood, Krzysztof Kozlowski, robh+dt,
	Mark Brown, linux-arm-kernel, m.szyprowski
In-Reply-To: <20190920130218.32690-2-s.nawrocki@samsung.com>

The patch

   ASoC: wm8994: Do not register inapplicable controls for WM1811

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From ca2347190adb5e4eece73a2b16e96e651c46246b Mon Sep 17 00:00:00 2001
From: Sylwester Nawrocki <s.nawrocki@samsung.com>
Date: Fri, 20 Sep 2019 15:02:10 +0200
Subject: [PATCH] ASoC: wm8994: Do not register inapplicable controls for
 WM1811

In case of WM1811 device there are currently being registered controls
referring to registers not existing on that device.
It has been noticed when getting values of "AIF1ADC2 Volume", "AIF1DAC2
Volume" controls was failing during ALSA state restoring at boot time:
 "amixer: Mixer hw:0 load error: Device or resource busy"

Reading some registers through I2C was failing with EBUSY error and
indeed these registers were not available according to the datasheet.

To fix this controls not available on WM1811 are moved to a separate
array and registered only for WM8994 and WM8958.

There are some further differences between WM8994 and WM1811,
e.g. registers 603h, 604h, 605h, which are not covered in this patch.

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Link: https://lore.kernel.org/r/20190920130218.32690-2-s.nawrocki@samsung.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/wm8994.c | 43 +++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index c3d06e8bc54f..d5fb7f5dd551 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -533,13 +533,10 @@ static SOC_ENUM_SINGLE_DECL(dac_osr,
 static SOC_ENUM_SINGLE_DECL(adc_osr,
 			    WM8994_OVERSAMPLING, 1, osr_text);
 
-static const struct snd_kcontrol_new wm8994_snd_controls[] = {
+static const struct snd_kcontrol_new wm8994_common_snd_controls[] = {
 SOC_DOUBLE_R_TLV("AIF1ADC1 Volume", WM8994_AIF1_ADC1_LEFT_VOLUME,
 		 WM8994_AIF1_ADC1_RIGHT_VOLUME,
 		 1, 119, 0, digital_tlv),
-SOC_DOUBLE_R_TLV("AIF1ADC2 Volume", WM8994_AIF1_ADC2_LEFT_VOLUME,
-		 WM8994_AIF1_ADC2_RIGHT_VOLUME,
-		 1, 119, 0, digital_tlv),
 SOC_DOUBLE_R_TLV("AIF2ADC Volume", WM8994_AIF2_ADC_LEFT_VOLUME,
 		 WM8994_AIF2_ADC_RIGHT_VOLUME,
 		 1, 119, 0, digital_tlv),
@@ -556,8 +553,6 @@ SOC_ENUM("AIF2DACR Source", aif2dacr_src),
 
 SOC_DOUBLE_R_TLV("AIF1DAC1 Volume", WM8994_AIF1_DAC1_LEFT_VOLUME,
 		 WM8994_AIF1_DAC1_RIGHT_VOLUME, 1, 96, 0, digital_tlv),
-SOC_DOUBLE_R_TLV("AIF1DAC2 Volume", WM8994_AIF1_DAC2_LEFT_VOLUME,
-		 WM8994_AIF1_DAC2_RIGHT_VOLUME, 1, 96, 0, digital_tlv),
 SOC_DOUBLE_R_TLV("AIF2DAC Volume", WM8994_AIF2_DAC_LEFT_VOLUME,
 		 WM8994_AIF2_DAC_RIGHT_VOLUME, 1, 96, 0, digital_tlv),
 
@@ -565,17 +560,12 @@ SOC_SINGLE_TLV("AIF1 Boost Volume", WM8994_AIF1_CONTROL_2, 10, 3, 0, aif_tlv),
 SOC_SINGLE_TLV("AIF2 Boost Volume", WM8994_AIF2_CONTROL_2, 10, 3, 0, aif_tlv),
 
 SOC_SINGLE("AIF1DAC1 EQ Switch", WM8994_AIF1_DAC1_EQ_GAINS_1, 0, 1, 0),
-SOC_SINGLE("AIF1DAC2 EQ Switch", WM8994_AIF1_DAC2_EQ_GAINS_1, 0, 1, 0),
 SOC_SINGLE("AIF2 EQ Switch", WM8994_AIF2_EQ_GAINS_1, 0, 1, 0),
 
 WM8994_DRC_SWITCH("AIF1DAC1 DRC Switch", WM8994_AIF1_DRC1_1, 2),
 WM8994_DRC_SWITCH("AIF1ADC1L DRC Switch", WM8994_AIF1_DRC1_1, 1),
 WM8994_DRC_SWITCH("AIF1ADC1R DRC Switch", WM8994_AIF1_DRC1_1, 0),
 
-WM8994_DRC_SWITCH("AIF1DAC2 DRC Switch", WM8994_AIF1_DRC2_1, 2),
-WM8994_DRC_SWITCH("AIF1ADC2L DRC Switch", WM8994_AIF1_DRC2_1, 1),
-WM8994_DRC_SWITCH("AIF1ADC2R DRC Switch", WM8994_AIF1_DRC2_1, 0),
-
 WM8994_DRC_SWITCH("AIF2DAC DRC Switch", WM8994_AIF2_DRC_1, 2),
 WM8994_DRC_SWITCH("AIF2ADCL DRC Switch", WM8994_AIF2_DRC_1, 1),
 WM8994_DRC_SWITCH("AIF2ADCR DRC Switch", WM8994_AIF2_DRC_1, 0),
@@ -594,9 +584,6 @@ SOC_SINGLE("Sidetone HPF Switch", WM8994_SIDETONE, 6, 1, 0),
 SOC_ENUM("AIF1ADC1 HPF Mode", aif1adc1_hpf),
 SOC_DOUBLE("AIF1ADC1 HPF Switch", WM8994_AIF1_ADC1_FILTERS, 12, 11, 1, 0),
 
-SOC_ENUM("AIF1ADC2 HPF Mode", aif1adc2_hpf),
-SOC_DOUBLE("AIF1ADC2 HPF Switch", WM8994_AIF1_ADC2_FILTERS, 12, 11, 1, 0),
-
 SOC_ENUM("AIF2ADC HPF Mode", aif2adc_hpf),
 SOC_DOUBLE("AIF2ADC HPF Switch", WM8994_AIF2_ADC_FILTERS, 12, 11, 1, 0),
 
@@ -637,6 +624,24 @@ SOC_SINGLE("AIF2DAC 3D Stereo Switch", WM8994_AIF2_DAC_FILTERS_2,
 	   8, 1, 0),
 };
 
+/* Controls not available on WM1811 */
+static const struct snd_kcontrol_new wm8994_snd_controls[] = {
+SOC_DOUBLE_R_TLV("AIF1ADC2 Volume", WM8994_AIF1_ADC2_LEFT_VOLUME,
+		 WM8994_AIF1_ADC2_RIGHT_VOLUME,
+		 1, 119, 0, digital_tlv),
+SOC_DOUBLE_R_TLV("AIF1DAC2 Volume", WM8994_AIF1_DAC2_LEFT_VOLUME,
+		 WM8994_AIF1_DAC2_RIGHT_VOLUME, 1, 96, 0, digital_tlv),
+
+SOC_SINGLE("AIF1DAC2 EQ Switch", WM8994_AIF1_DAC2_EQ_GAINS_1, 0, 1, 0),
+
+WM8994_DRC_SWITCH("AIF1DAC2 DRC Switch", WM8994_AIF1_DRC2_1, 2),
+WM8994_DRC_SWITCH("AIF1ADC2L DRC Switch", WM8994_AIF1_DRC2_1, 1),
+WM8994_DRC_SWITCH("AIF1ADC2R DRC Switch", WM8994_AIF1_DRC2_1, 0),
+
+SOC_ENUM("AIF1ADC2 HPF Mode", aif1adc2_hpf),
+SOC_DOUBLE("AIF1ADC2 HPF Switch", WM8994_AIF1_ADC2_FILTERS, 12, 11, 1, 0),
+};
+
 static const struct snd_kcontrol_new wm8994_eq_controls[] = {
 SOC_SINGLE_TLV("AIF1DAC1 EQ1 Volume", WM8994_AIF1_DAC1_EQ_GAINS_1, 11, 31, 0,
 	       eq_tlv),
@@ -4258,13 +4263,15 @@ static int wm8994_component_probe(struct snd_soc_component *component)
 	wm8994_handle_pdata(wm8994);
 
 	wm_hubs_add_analogue_controls(component);
-	snd_soc_add_component_controls(component, wm8994_snd_controls,
-			     ARRAY_SIZE(wm8994_snd_controls));
+	snd_soc_add_component_controls(component, wm8994_common_snd_controls,
+				       ARRAY_SIZE(wm8994_common_snd_controls));
 	snd_soc_dapm_new_controls(dapm, wm8994_dapm_widgets,
 				  ARRAY_SIZE(wm8994_dapm_widgets));
 
 	switch (control->type) {
 	case WM8994:
+		snd_soc_add_component_controls(component, wm8994_snd_controls,
+					       ARRAY_SIZE(wm8994_snd_controls));
 		snd_soc_dapm_new_controls(dapm, wm8994_specific_dapm_widgets,
 					  ARRAY_SIZE(wm8994_specific_dapm_widgets));
 		if (control->revision < 4) {
@@ -4284,8 +4291,10 @@ static int wm8994_component_probe(struct snd_soc_component *component)
 		}
 		break;
 	case WM8958:
+		snd_soc_add_component_controls(component, wm8994_snd_controls,
+					       ARRAY_SIZE(wm8994_snd_controls));
 		snd_soc_add_component_controls(component, wm8958_snd_controls,
-				     ARRAY_SIZE(wm8958_snd_controls));
+					       ARRAY_SIZE(wm8958_snd_controls));
 		snd_soc_dapm_new_controls(dapm, wm8958_dapm_widgets,
 					  ARRAY_SIZE(wm8958_dapm_widgets));
 		if (control->revision < 1) {
-- 
2.20.1


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

^ permalink raw reply related

* Applied "ASoC: samsung: arndale: Add missing OF node dereferencing" to the asoc tree
From: Mark Brown @ 2019-09-23 21:22 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: devicetree, alsa-devel, linux-samsung-soc, ckeepax, b.zolnierkie,
	sbkim73, patches, lgirdwood, Krzysztof Kozlowski, robh+dt,
	Mark Brown, linux-arm-kernel, m.szyprowski
In-Reply-To: <20190920130218.32690-3-s.nawrocki@samsung.com>

The patch

   ASoC: samsung: arndale: Add missing OF node dereferencing

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From fb629fa2587d0c150792d87e3053664bfc8dc78c Mon Sep 17 00:00:00 2001
From: Sylwester Nawrocki <s.nawrocki@samsung.com>
Date: Fri, 20 Sep 2019 15:02:11 +0200
Subject: [PATCH] ASoC: samsung: arndale: Add missing OF node dereferencing

Ensure there is no OF node references kept when the driver
is removed/unbound.

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
Link: https://lore.kernel.org/r/20190920130218.32690-3-s.nawrocki@samsung.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/samsung/arndale_rt5631.c | 34 ++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/sound/soc/samsung/arndale_rt5631.c b/sound/soc/samsung/arndale_rt5631.c
index c213913eb984..fd8c6642fb0d 100644
--- a/sound/soc/samsung/arndale_rt5631.c
+++ b/sound/soc/samsung/arndale_rt5631.c
@@ -5,6 +5,7 @@
 //  Author: Claude <claude@insginal.co.kr>
 
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 
@@ -74,6 +75,17 @@ static struct snd_soc_card arndale_rt5631 = {
 	.num_links = ARRAY_SIZE(arndale_rt5631_dai),
 };
 
+static void arndale_put_of_nodes(struct snd_soc_card *card)
+{
+	struct snd_soc_dai_link *dai_link;
+	int i;
+
+	for_each_card_prelinks(card, i, dai_link) {
+		of_node_put(dai_link->cpus->of_node);
+		of_node_put(dai_link->codecs->of_node);
+	}
+}
+
 static int arndale_audio_probe(struct platform_device *pdev)
 {
 	int n, ret;
@@ -103,18 +115,31 @@ static int arndale_audio_probe(struct platform_device *pdev)
 		if (!arndale_rt5631_dai[0].codecs->of_node) {
 			dev_err(&pdev->dev,
 			"Property 'samsung,audio-codec' missing or invalid\n");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto err_put_of_nodes;
 		}
 	}
 
 	ret = devm_snd_soc_register_card(card->dev, card);
+	if (ret) {
+		dev_err(&pdev->dev, "snd_soc_register_card() failed: %d\n", ret);
+		goto err_put_of_nodes;
+	}
+	return 0;
 
-	if (ret)
-		dev_err(&pdev->dev, "snd_soc_register_card() failed:%d\n", ret);
-
+err_put_of_nodes:
+	arndale_put_of_nodes(card);
 	return ret;
 }
 
+static int arndale_audio_remove(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+
+	arndale_put_of_nodes(card);
+	return 0;
+}
+
 static const struct of_device_id samsung_arndale_rt5631_of_match[] __maybe_unused = {
 	{ .compatible = "samsung,arndale-rt5631", },
 	{ .compatible = "samsung,arndale-alc5631", },
@@ -129,6 +154,7 @@ static struct platform_driver arndale_audio_driver = {
 		.of_match_table = of_match_ptr(samsung_arndale_rt5631_of_match),
 	},
 	.probe = arndale_audio_probe,
+	.remove = arndale_audio_remove,
 };
 
 module_platform_driver(arndale_audio_driver);
-- 
2.20.1


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

^ permalink raw reply related

* Re: [PATCH 3/3] arm64: dts: mark lx2160a esdhc controllers dma coherent
From: Ulf Hansson @ 2019-09-23 21:26 UTC (permalink / raw)
  To: Russell King
  Cc: Mark Rutland, DTML, dann frazier, Will Deacon, Li Yang,
	Nicolin Chen, Rob Herring, Y.b. Lu, Shawn Guo, Robin Murphy,
	Christoph Hellwig, Linux ARM
In-Reply-To: <E1iBz55-0008Mj-CX@rmk-PC.armlinux.org.uk>

On Sun, 22 Sep 2019 at 12:29, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> The LX2160A esdhc controllers are setup by the driver to be DMA
> coherent, but without marking them as such in DT, Linux thinks they
> are not.  This can lead to random sporadic DMA errors, even to the
> extent of preventing boot, such as:
>
> mmc0: ADMA error
> mmc0: sdhci: ============ SDHCI REGISTER DUMP ===========
> mmc0: sdhci: Sys addr:  0x00000000 | Version:  0x00002202
> mmc0: sdhci: Blk size:  0x00000008 | Blk cnt:  0x00000001
> mmc0: sdhci: Argument:  0x00000000 | Trn mode: 0x00000013
> mmc0: sdhci: Present:   0x01f50008 | Host ctl: 0x00000038
> mmc0: sdhci: Power:     0x00000003 | Blk gap:  0x00000000
> mmc0: sdhci: Wake-up:   0x00000000 | Clock:    0x000040d8
> mmc0: sdhci: Timeout:   0x00000003 | Int stat: 0x00000001
> mmc0: sdhci: Int enab:  0x037f108f | Sig enab: 0x037f108b
> mmc0: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00002202
> mmc0: sdhci: Caps:      0x35fa0000 | Caps_1:   0x0000af00
> mmc0: sdhci: Cmd:       0x0000333a | Max curr: 0x00000000
> mmc0: sdhci: Resp[0]:   0x00000920 | Resp[1]:  0x001d8a33
> mmc0: sdhci: Resp[2]:   0x325b5900 | Resp[3]:  0x3f400e00
> mmc0: sdhci: Host ctl2: 0x00000000
> mmc0: sdhci: ADMA Err:  0x00000009 | ADMA Ptr: 0x000000236d43820c
> mmc0: sdhci: ============================================
> mmc0: error -5 whilst initialising SD card
>
> These are caused by the device's descriptor fetch hitting speculatively
> loaded CPU cache lines that the CPU does not see through the normal,
> non-cacheable DMA coherent mapping that it uses for non-coherent
> devices.
>
> DT and the device must agree wrt whether the device is DMA coherent or
> not.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

As I am picking up patch1 and patch2 from this series, I can also help
out and pick up this one, if that is okay by people?

Kind regards
Uffe

> ---
>  arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> index 36b153e3da47..508af23edef0 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> @@ -593,6 +593,7 @@
>                         reg = <0x0 0x2140000 0x0 0x10000>;
>                         interrupts = <0 28 0x4>; /* Level high type */
>                         clocks = <&clockgen 4 1>;
> +                       dma-coherent;
>                         voltage-ranges = <1800 1800 3300 3300>;
>                         sdhci,auto-cmd12;
>                         little-endian;
> @@ -605,6 +606,7 @@
>                         reg = <0x0 0x2150000 0x0 0x10000>;
>                         interrupts = <0 63 0x4>; /* Level high type */
>                         clocks = <&clockgen 4 1>;
> +                       dma-coherent;
>                         voltage-ranges = <1800 1800 3300 3300>;
>                         sdhci,auto-cmd12;
>                         broken-cd;
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

^ permalink raw reply

* Re: [PATCH v6 2/6] PM / devfreq: Move more initialization before registration
From: Matthias Kaehlcke @ 2019-09-23 21:17 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Artur Świgoń, Abel Vesa, Saravana Kannan,
	linux-pm@vger.kernel.org, Viresh Kumar, dl-linux-imx,
	Krzysztof Kozlowski, Lukasz Luba, Chanwoo Choi, Kyungmin Park,
	MyungJoo Ham, Alexandre Bailon, Georgi Djakov,
	linux-arm-kernel@lists.infradead.org, Jacky Bai
In-Reply-To: <VI1PR04MB7023E58A11DF398FC90F98C7EE850@VI1PR04MB7023.eurprd04.prod.outlook.com>

On Mon, Sep 23, 2019 at 07:56:51PM +0000, Leonard Crestez wrote:
> On 23.09.2019 22:11, Matthias Kaehlcke wrote:
> > On Mon, Sep 23, 2019 at 06:56:28PM +0000, Leonard Crestez wrote:
> >> On 23.09.2019 21:11, Matthias Kaehlcke wrote:
> >>> On Mon, Sep 23, 2019 at 06:51:05PM +0300, Leonard Crestez wrote:
> >>>> In general it is a better to initialize an object before making it
> >>>> accessible externally (through device_register).
> >>>>
> >>>> This makes it possible to avoid relying on locking a partially
> >>>> initialized object.
> >>>>
> >>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> >>>> ---
> >>>>    drivers/devfreq/devfreq.c | 43 +++++++++++++++++++++++----------------
> >>>>    1 file changed, 25 insertions(+), 18 deletions(-)
> >>>>
> >>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> >>>> index 323d43315d1e..b4d2bfebb140 100644
> >>>> --- a/drivers/devfreq/devfreq.c
> >>>> +++ b/drivers/devfreq/devfreq.c
> >>>> @@ -587,10 +587,12 @@ static void devfreq_dev_release(struct device *dev)
> >>>>    	mutex_unlock(&devfreq_list_lock);
> >>>>    
> >>>>    	if (devfreq->profile->exit)
> >>>>    		devfreq->profile->exit(devfreq->dev.parent);
> >>>>    
> >>>> +	kfree(devfreq->time_in_state);
> >>>> +	kfree(devfreq->trans_table);
> >>>>    	mutex_destroy(&devfreq->lock);
> >>>>    	kfree(devfreq);
> >>>>    }
> >>>>    
> >>>>    /**
> >>>> @@ -670,44 +672,43 @@ struct devfreq *devfreq_add_device(struct device *dev,
> >>>>    	devfreq->max_freq = devfreq->scaling_max_freq;
> >>>>    
> >>>>    	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> >>>>    	atomic_set(&devfreq->suspend_count, 0);
> >>>>    
> >>>> -	dev_set_name(&devfreq->dev, "devfreq%d",
> >>>> -				atomic_inc_return(&devfreq_no));
> >>>> -	err = device_register(&devfreq->dev);
> >>>> -	if (err) {
> >>>> -		mutex_unlock(&devfreq->lock);
> >>>> -		put_device(&devfreq->dev);
> >>>> -		goto err_out;
> >>>> -	}
> >>>> -
> >>>> -	devfreq->trans_table = devm_kzalloc(&devfreq->dev,
> >>>> +	devfreq->trans_table = kzalloc(
> >>>>    			array3_size(sizeof(unsigned int),
> >>>>    				    devfreq->profile->max_state,
> >>>>    				    devfreq->profile->max_state),
> >>>>    			GFP_KERNEL);
> >>>>    	if (!devfreq->trans_table) {
> >>>>    		mutex_unlock(&devfreq->lock);
> >>>>    		err = -ENOMEM;
> >>>> -		goto err_devfreq;
> >>>> +		goto err_dev;
> >>>>    	}
> >>>>    
> >>>> -	devfreq->time_in_state = devm_kcalloc(&devfreq->dev,
> >>>> -			devfreq->profile->max_state,
> >>>> -			sizeof(unsigned long),
> >>>> -			GFP_KERNEL);
> >>>> +	devfreq->time_in_state = kcalloc(devfreq->profile->max_state,
> >>>> +					 sizeof(unsigned long),
> >>>> +					 GFP_KERNEL);
> >>>>    	if (!devfreq->time_in_state) {
> >>>>    		mutex_unlock(&devfreq->lock);
> >>>>    		err = -ENOMEM;
> >>>> -		goto err_devfreq;
> >>>> +		goto err_dev;
> >>>>    	}
> >>>>    
> >>>>    	devfreq->last_stat_updated = jiffies;
> >>>>    
> >>>>    	srcu_init_notifier_head(&devfreq->transition_notifier_list);
> >>>>    
> >>>> +	dev_set_name(&devfreq->dev, "devfreq%d",
> >>>> +				atomic_inc_return(&devfreq_no));
> >>>> +	err = device_register(&devfreq->dev);
> >>>> +	if (err) {
> >>>> +		mutex_unlock(&devfreq->lock);
> >>>> +		put_device(&devfreq->dev);
> >>>> +		goto err_out;
> >>>
> >>> As per my comment on v5 I think the goto needs to go to 'err_dev'. The
> >>> device registration failed, hence devfreq_dev_release() won't be
> >>> called to free allocated memory.
> >>
> >> This code is not modified in the patch, it only shows up as +added
> >> because diff got confused but there is an identical -removed chunk
> >> higher up.
> >>
> >> The device_register documentation mentions the following:
> >>
> >>    * NOTE: _Never_ directly free @dev after calling this function, even
> >>    * if it returned an error! Always use put_device() to give up the
> >>    * reference initialized in this function instead.
> >>
> >> Cleanup path then goes like this (from a hacked error in device_add):
> >>    dump_stack+0xdc/0x144
> >>   
> >>
> >>    devfreq_dev_release+0x38/0xc0
> >>   
> >>
> >>    device_release+0x34/0x90
> >>   
> >>
> >>    kobject_put+0x8c/0x1f0
> >>   
> >>
> >>    put_device+0x24/0x30
> >>   
> >>
> >>    devfreq_add_device+0x540/0x570
> >>   
> >>
> >>    devm_devfreq_add_device+0x60/0xd0
> >>   
> >>
> >>    imx_ddrc_probe+0x35c/0x4c8
> > 
> > Good to know, thanks for the pointer!
> > 
> >> Can I add your "Reviewed-By" for the rest of the series if I fix the nits?
> > 
> > By now you should have it for most patches. For this one:
> > 
> > Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> > 
> > There is one doubt I have left on "PM / devfreq: Add PM QoS support" that I
> > posted on v5:
> > 
> > "IIUC you rely on the notifiers being removed by devfreq_dev_release().
> > Does dev_pm_qos_remove_notifier() behave gracefully if the notifier is
> > not initialized/added or do we need to use BLOCKING_NOTIFIER_INIT() or
> > similar?"
> 
> Sorry for missing that.
> > Could you clarify this replying to the thread? Besides that and the
> > nits (which are optional to fix) the patch looks good to me.
> 
> The blocking_notifier_head structs are managed by PM QoS inside 
> dev_pm_qos_constraints_allocate and dev_pm_qos_constraints_destroy. The 
> devfreq subsystem only registers a notifier_block, that's a 
> NULL-terminated singly-linked list for which zero-initialization from 
> kzalloc should be sufficient.

Right, thanks for the clarification!

> But now that I look at this again I should warn and return NOTIFY_DONE 
> from devfreq_qos_notifier_call instead of propagating a negative errno.

Ack

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

^ permalink raw reply

* [PATCH v7 6/6] PM / devfreq: Use PM QoS for sysfs min/max_freq
From: Leonard Crestez @ 2019-09-23 21:10 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Matthias Kaehlcke
  Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
	Viresh Kumar, NXP Linux Team, Krzysztof Kozlowski, Lukasz Luba,
	Chanwoo Choi, Alexandre Bailon, Martin Kepplinger, Georgi Djakov,
	linux-arm-kernel, Jacky Bai
In-Reply-To: <cover.1569272883.git.leonard.crestez@nxp.com>

Switch the handling of min_freq and max_freq from sysfs to use the
dev_pm_qos_request interface.

Since PM QoS handles frequencies as kHz this change reduces the
precision of min_freq and max_freq. This shouldn't introduce problems
because frequencies which are not an integer number of kHz are likely
not an integer number of Hz either.

Try to ensure compatibility by rounding min values down and rounding
max values up.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/devfreq/devfreq.c | 49 +++++++++++++++++++++++++--------------
 include/linux/devfreq.h   |  9 +++----
 2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 9887408f23bb..a00737e34d36 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -132,14 +132,10 @@ static void devfreq_get_freq_range(struct devfreq *devfreq,
 	*min_freq = max(*min_freq, HZ_PER_KHZ * (unsigned long)dev_pm_qos_read_value(
 				devfreq->dev.parent, DEV_PM_QOS_MIN_FREQUENCY));
 	*max_freq = min(*max_freq, HZ_PER_KHZ * (unsigned long)dev_pm_qos_read_value(
 				devfreq->dev.parent, DEV_PM_QOS_MAX_FREQUENCY));
 
-	/* constraints from sysfs */
-	*min_freq = max(*min_freq, devfreq->min_freq);
-	*max_freq = min(*max_freq, devfreq->max_freq);
-
 	/* constraints from OPP interface */
 	*min_freq = max(*min_freq, devfreq->scaling_min_freq);
 	/* scaling_max_freq can be zero on error */
 	if (devfreq->scaling_max_freq)
 		*max_freq = min(*max_freq, devfreq->scaling_max_freq);
@@ -679,10 +675,12 @@ static void devfreq_dev_release(struct device *dev)
 			DEV_PM_QOS_MIN_FREQUENCY);
 
 	if (devfreq->profile->exit)
 		devfreq->profile->exit(devfreq->dev.parent);
 
+	dev_pm_qos_remove_request(&devfreq->user_max_freq_req);
+	dev_pm_qos_remove_request(&devfreq->user_min_freq_req);
 	kfree(devfreq->time_in_state);
 	kfree(devfreq->trans_table);
 	mutex_destroy(&devfreq->lock);
 	kfree(devfreq);
 }
@@ -747,18 +745,26 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
 	if (!devfreq->scaling_min_freq) {
 		err = -EINVAL;
 		goto err_dev;
 	}
-	devfreq->min_freq = devfreq->scaling_min_freq;
 
 	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
 	if (!devfreq->scaling_max_freq) {
 		err = -EINVAL;
 		goto err_dev;
 	}
-	devfreq->max_freq = devfreq->scaling_max_freq;
+
+	/* PM QoS requests for min/max freq from sysfs */
+	err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req,
+				     DEV_PM_QOS_MIN_FREQUENCY, 0);
+	if (err < 0)
+		goto err_dev;
+	err = dev_pm_qos_add_request(dev, &devfreq->user_max_freq_req,
+				     DEV_PM_QOS_MAX_FREQUENCY, S32_MAX);
+	if (err < 0)
+		goto err_dev;
 
 	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
 	atomic_set(&devfreq->suspend_count, 0);
 
 	devfreq->trans_table = kzalloc(
@@ -843,10 +849,14 @@ struct devfreq *devfreq_add_device(struct device *dev,
 err_dev:
 	/*
 	 * Cleanup path for errors that happen before registration.
 	 * Otherwise we rely on devfreq_dev_release.
 	 */
+	if (dev_pm_qos_request_active(&devfreq->user_max_freq_req))
+		dev_pm_qos_remove_request(&devfreq->user_max_freq_req);
+	if (dev_pm_qos_request_active(&devfreq->user_min_freq_req))
+		dev_pm_qos_remove_request(&devfreq->user_min_freq_req);
 	kfree(devfreq->time_in_state);
 	kfree(devfreq->trans_table);
 	kfree(devfreq);
 err_out:
 	return ERR_PTR(err);
@@ -1387,14 +1397,17 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
 
 	ret = sscanf(buf, "%lu", &value);
 	if (ret != 1)
 		return -EINVAL;
 
-	mutex_lock(&df->lock);
-	df->min_freq = value;
-	update_devfreq(df);
-	mutex_unlock(&df->lock);
+	/* round down to kHz for dev_pm_qos */
+	if (value)
+		value = value / HZ_PER_KHZ;
+
+	ret = dev_pm_qos_update_request(&df->user_min_freq_req, value);
+	if (ret < 0)
+		return ret;
 
 	return count;
 }
 
 static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
@@ -1419,19 +1432,19 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
 
 	ret = sscanf(buf, "%lu", &value);
 	if (ret != 1)
 		return -EINVAL;
 
-	mutex_lock(&df->lock);
-
-	/* Interpret zero as "don't care" */
-	if (!value)
-		value = ULONG_MAX;
+	/* round up to kHz for dev_pm_qos and interpret zero as "don't care" */
+	if (value)
+		value = DIV_ROUND_UP(value, HZ_PER_KHZ);
+	else
+		value = S32_MAX;
 
-	df->max_freq = value;
-	update_devfreq(df);
-	mutex_unlock(&df->lock);
+	ret = dev_pm_qos_update_request(&df->user_max_freq_req, value);
+	if (ret < 0)
+		return ret;
 
 	return count;
 }
 static DEVICE_ATTR_RW(min_freq);
 
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 8b92ccbd1962..3162eb9b0954 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -11,10 +11,11 @@
 #define __LINUX_DEVFREQ_H__
 
 #include <linux/device.h>
 #include <linux/notifier.h>
 #include <linux/pm_opp.h>
+#include <linux/pm_qos.h>
 
 #define DEVFREQ_NAME_LEN 16
 
 /* DEVFREQ governor name */
 #define DEVFREQ_GOV_SIMPLE_ONDEMAND	"simple_ondemand"
@@ -121,12 +122,12 @@ struct devfreq_dev_profile {
  *		devfreq.nb to the corresponding register notifier call chain.
  * @work:	delayed work for load monitoring.
  * @previous_freq:	previously configured frequency value.
  * @data:	Private data of the governor. The devfreq framework does not
  *		touch this.
- * @min_freq:	Limit minimum frequency requested by user (0: none)
- * @max_freq:	Limit maximum frequency requested by user (0: none)
+ * @user_min_freq_req:	PM QoS min frequency request from user (via sysfs)
+ * @user_max_freq_req:	PM QoS max frequency request from user (via sysfs)
  * @scaling_min_freq:	Limit minimum frequency requested by OPP interface
  * @scaling_max_freq:	Limit maximum frequency requested by OPP interface
  * @stop_polling:	 devfreq polling status of a device.
  * @suspend_freq:	 frequency of a device set during suspend phase.
  * @resume_freq:	 frequency of a device set in resume phase.
@@ -161,12 +162,12 @@ struct devfreq {
 	unsigned long previous_freq;
 	struct devfreq_dev_status last_status;
 
 	void *data; /* private data for governors */
 
-	unsigned long min_freq;
-	unsigned long max_freq;
+	struct dev_pm_qos_request user_min_freq_req;
+	struct dev_pm_qos_request user_max_freq_req;
 	unsigned long scaling_min_freq;
 	unsigned long scaling_max_freq;
 	bool stop_polling;
 
 	unsigned long suspend_freq;
-- 
2.17.1


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

^ permalink raw reply related


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