Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 0/2] Introduce PCI_FIXUP_IOMMU
From: Zhangfei Gao @ 2020-05-28  6:46 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, Arnd Bergmann,
	linux-pci, Greg Kroah-Hartman, Joerg Roedel, Hanjun Guo,
	Rafael J. Wysocki, linux-kernel, iommu, linux-acpi, Wangzhou,
	linux-crypto, Sudeep Holla, Bjorn Helgaas, kenneth-lee-2012,
	linux-arm-kernel, Len Brown
In-Reply-To: <20200527181842.GA256680@bjorn-Precision-5520>

Hi, Bjorn

On 2020/5/28 上午2:18, Bjorn Helgaas wrote:
> On Tue, May 26, 2020 at 07:49:07PM +0800, Zhangfei Gao wrote:
>> Some platform devices appear as PCI but are actually on the AMBA bus,
>> and they need fixup in drivers/pci/quirks.c handling iommu_fwnode.
>> Here introducing PCI_FIXUP_IOMMU, which is called after iommu_fwnode
>> is allocated, instead of reusing PCI_FIXUP_FINAL since it will slow
>> down iommu probing as all devices in fixup final list will be
>> reprocessed, suggested by Joerg, [1]
> Is this slowdown significant?  We already iterate over every device
> when applying PCI_FIXUP_FINAL quirks, so if we used the existing
> PCI_FIXUP_FINAL, we wouldn't be adding a new loop.  We would only be
> adding two more iterations to the loop in pci_do_fixups() that tries
> to match quirks against the current device.  I doubt that would be a
> measurable slowdown.
I do not notice the difference when compared fixup_iommu and fixup_final 
via get_jiffies_64,
since in our platform no other pci fixup is registered.

Here the plan is adding pci_fixup_device in iommu_fwspec_init,
so if using fixup_final the iteration will be done again here.

>
>> For example:
>> Hisilicon platform device need fixup in
>> drivers/pci/quirks.c handling fwspec->can_stall, which is introduced in [2]
>>
>> +static void quirk_huawei_pcie_sva(struct pci_dev *pdev)
>> +{
>> +    struct iommu_fwspec *fwspec;
>> +
>> +    pdev->eetlp_prefix_path = 1;
>> +    fwspec = dev_iommu_fwspec_get(&pdev->dev);
>> +    if (fwspec)
>> +        fwspec->can_stall = 1;
>> +}
>> +
>> +DECLARE_PCI_FIXUP_IOMMU(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
>> +DECLARE_PCI_iFIXUP_IOMMU(PCI_VENDOR_ID_HUAWEI, 0xa251, quirk_huawei_pcie_sva);
>>
>> [1] https://www.spinics.net/lists/iommu/msg44591.html
>> [2] https://www.spinics.net/lists/linux-pci/msg94559.html
> If you reference these in the commit logs, please use lore.kernel.org
> links instead of spinics.
Got it, thanks Bjorn.




_______________________________________________
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 2/2] iommu: calling pci_fixup_iommu in iommu_fwspec_init
From: Zhangfei Gao @ 2020-05-28  6:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, Arnd Bergmann,
	linux-pci, Joerg Roedel, Hanjun Guo, Rafael J. Wysocki,
	linux-kernel, iommu, linux-acpi, Wangzhou, linux-crypto,
	Sudeep Holla, Bjorn Helgaas, kenneth-lee-2012, linux-arm-kernel,
	Len Brown
In-Reply-To: <20200527090115.GB179718@kroah.com>



On 2020/5/27 下午5:01, Greg Kroah-Hartman wrote:
> On Tue, May 26, 2020 at 07:49:09PM +0800, Zhangfei Gao wrote:
>> Calling pci_fixup_iommu in iommu_fwspec_init, which alloc
>> iommu_fwnode. Some platform devices appear as PCI but are
>> actually on the AMBA bus, and they need fixup in
>> drivers/pci/quirks.c handling iommu_fwnode.
>> So calling pci_fixup_iommu after iommu_fwnode is allocated.
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>> ---
>>   drivers/iommu/iommu.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 7b37542..fb84c42 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -2418,6 +2418,10 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
>>   	fwspec->iommu_fwnode = iommu_fwnode;
>>   	fwspec->ops = ops;
>>   	dev_iommu_fwspec_set(dev, fwspec);
>> +
>> +	if (dev_is_pci(dev))
>> +		pci_fixup_device(pci_fixup_iommu, to_pci_dev(dev));
> Why can't the caller do this as it "knows" it is a PCI device at that
> point in time, right?
Putting fixup here is because
1. iommu_fwspec has been allocated
2. iommu_fwspec_init will be called by of_pci_iommu_init and 
iort_pci_iommu_init, covering both acpi and dt

Thanks

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

^ permalink raw reply

* Re: [PATCH v2 3/6] prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls
From: Michael Kerrisk (man-pages) @ 2020-05-28  6:57 UTC (permalink / raw)
  To: Dave Martin
  Cc: linux-arch, linux-man, Thomas Gleixner, linux-arm-kernel,
	Waiman Long
In-Reply-To: <1590614258-24728-4-git-send-email-Dave.Martin@arm.com>

Hi Dave,

On Wed, 27 May 2020 at 23:18, Dave Martin <Dave.Martin@arm.com> wrote:
>
> Add the PR_SPEC_DISABLE_NOEXEC mode added in Linux 5.1
> for the PR_SPEC_STORE_BYPASS "misfeature" of
> PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Waiman Long <longman@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>

I had already applied your earlier send of this patch (in a private
branch). I'll push those changes shortly.

Cheers,

Michael

> ---
>  man2/prctl.2 | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index b6fb51c..cab9915 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1187,6 +1187,12 @@ The speculation feature is disabled, mitigation is enabled.
>  Same as
>  .B PR_SPEC_DISABLE
>  but cannot be undone.
> +.TP
> +.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
> +Same as
> +.BR PR_SPEC_DISABLE ,
> +but but the state will be cleared on
> +.BR execve (2).
>  .RE
>  .IP
>  If all bits are 0,
> @@ -1251,6 +1257,17 @@ with the same value for
>  .I arg2
>  will fail with the error
>  .BR EPERM .
> +.\" commit 71368af9027f18fe5d1c6f372cfdff7e4bde8b48
> +.TP
> +.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
> +Same as
> +.BR PR_SPEC_DISABLE ,
> +but but the state will be cleared on
> +.BR execve (2).
> +Currently only supported for
> +.I arg2
> +equal to
> +.B PR_SPEC_STORE_BYPASS.
>  .RE
>  .IP
>  Any unsupported value in
> @@ -1899,11 +1916,12 @@ was
>  .BR PR_SET_SPECULATION_CTRL
>  and
>  .IR arg3
> -is neither
> +is not
>  .BR PR_SPEC_ENABLE ,
>  .BR PR_SPEC_DISABLE ,
> +.BR PR_SPEC_FORCE_DISABLE ,
>  nor
> -.BR PR_SPEC_FORCE_DISABLE .
> +.BR PR_SPEC_DISABLE_NOEXEC .
>  .SH VERSIONS
>  The
>  .BR prctl ()
> --
> 2.1.4
>


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

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

^ permalink raw reply

* Re: [PATCH v2 2/6] prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls
From: Michael Kerrisk (man-pages) @ 2020-05-28  7:01 UTC (permalink / raw)
  To: Dave Martin
  Cc: linux-arch, linux-man, Thomas Gleixner, Tim Chen,
	linux-arm-kernel
In-Reply-To: <1590614258-24728-3-git-send-email-Dave.Martin@arm.com>

Hi Dave,

On Wed, 27 May 2020 at 23:18, Dave Martin <Dave.Martin@arm.com> wrote:
>
> Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
> for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>

I had also applied this patch from the email you sent earlier. I've
pushed those changes to master now.

Thanks,

Michael

> ---
>  man2/prctl.2 | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index dc99218..b6fb51c 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1213,11 +1213,20 @@ arguments must be specified as 0; otherwise the call fails with the error
>  .\" commit 356e4bfff2c5489e016fdb925adbf12a1e3950ee
>  Sets the state of the speculation misfeature specified in
>  .IR arg2 .
> -Currently, the only permitted value for this argument is
> +Currently, this argument must be one of:
> +.RS
> +.TP
>  .B PR_SPEC_STORE_BYPASS
> -(otherwise the call fails with the error
> +speculative store bypass control, or
> +.\" commit 9137bb27e60e554dab694eafa4cca241fa3a694f
> +.TP
> +.BR PR_SPEC_INDIRECT_BRANCH " (since Linux 4.20)"
> +indirect branch speculation control.
> +.RE
> +.IP
> +(Otherwise the call fails with the error
>  .BR ENODEV ).
> -This setting is a per-thread attribute.
> +These settings are per-thread attributes.
>  The
>  .IR arg3
>  argument is used to hand in the control value,
> @@ -1235,13 +1244,16 @@ Same as
>  .BR PR_SPEC_DISABLE ,
>  but cannot be undone.
>  A subsequent
> -.B
> -prctl(..., PR_SPEC_ENABLE)
> +.BR prctl (\c
> +.IR arg2 ,
> +.BR PR_SPEC_ENABLE )
> +with the same value for
> +.I arg2
>  will fail with the error
>  .BR EPERM .
>  .RE
>  .IP
> -Any other value in
> +Any unsupported value in
>  .IR arg3
>  will result in the call failing with the error
>  .BR ERANGE .
> --
> 2.1.4
>


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

_______________________________________________
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 RFCv2 9/9] arm64: Support async page fault
From: Marc Zyngier @ 2020-05-28  7:03 UTC (permalink / raw)
  To: Gavin Shan
  Cc: catalin.marinas, linux-kernel, shan.gavin, Paolo Bonzini, will,
	kvmarm, linux-arm-kernel
In-Reply-To: <a6addc25-29af-3690-8392-efa5e8381e98@redhat.com>

On 2020-05-28 07:14, Gavin Shan wrote:
> Hi Paolo,
> 
> On 5/27/20 4:48 PM, Paolo Bonzini wrote:
>> I definitely appreciate the work, but this is repeating most of the
>> mistakes done in the x86 implementation.  In particular:
>> 
>> - the page ready signal can be done as an interrupt, rather than an
>> exception.  This is because "page ready" can be handled 
>> asynchronously,
>> in contrast to "page not present" which must be done on the same
>> instruction that triggers it.  You can refer to the recent series from
>> Vitaly Kuznetsov that switched "page ready" to an interrupt.
>> 
> 
> Yeah, page ready can be handled asynchronously. I think it would be
> nice for x86/arm64 to share same design. x86 has 256 vectors and it
> seems 0xec is picked for the purpose. However, arm64 doesn't have so
> many (interrupt/exception) vectors and PPI might be appropriate for
> the purpose if I'm correct, because it has same INTD for all CPUs.
> From this point, it's similar to x86's vector. There are 16 PPIs, which
> are in range of 16 to 31, and we might reserve one for this. According
> to GICv3/v4 spec, 22 - 30 have been assigned.

The assignment of the PPIs is completely implementation defined,
and is not part of the architecture (and certainly not in the
GICv3/v4 spec). SBSA makes some statements as to the way they *could*
be assigned, but that's in no way enforced. This allocation is entirely
controlled by userspace, which would need to configure tell KVM
which PPI to use on a per-VM basis.

You would then need to describe the PPI assignment through firmware
(both DT and ACPI) so that the guest kernel can know what PPI the
hypervisor would be signalling on.

It is also not very future proof should we move to a different
interrupt architecture.

> 
>> - the page not present is reusing the memory abort exception, and
>> there's really no reason to do so.  I think it would be best if ARM
>> could reserve one ESR exception code for the hypervisor.  Mark, any
>> ideas how to proceed here?
>> 
> 
> Well, a subclass of ESR exception code, whose DFSC (Data Fault Status
> Code) is 0x34, was taken for the purpose in RFCv1. The code is IMPDEF
> one and Mark suggested not to do so. I agree the page not present needs 
> a
> separately subclass of exception. With that, there will be less 
> conflicts
> and complexity. However, the question is which subclass or DFSC code I 
> should
> used for the purpose.

The current state of the architecture doesn't seem to leave much leeway 
in
terms of SW creativity here. You just can't allocate your own ISS 
encoding
without risking a clash with future revisions of the architecture.
It isn't even clear whether the value you put would stick in ESR_EL1
if it isn't a valid value for this CPU (see the definition of 'Reserved'
in the ARM ARM).

Allocating such a syndrome would require from ARM:

- the guarantee that no existing implementation, irrespective of the
   implementer, can cope with the ISS encoding of your choice,

- the written promise in the architecture that some EC/ISS values
   are reserved for SW, and that promise to apply retrospectively.

This is somewhat unlikely to happen.

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

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

^ permalink raw reply

* Re: [PATCH 06/12] PM / devfreq: Add cpu based scaling support to passive_governor
From: Chanwoo Choi @ 2020-05-28  7:17 UTC (permalink / raw)
  To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
	Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
  Cc: devicetree, srv_heupstream, linux-pm, linux-kernel,
	linux-mediatek, Sibi Sankar, linux-arm-kernel
In-Reply-To: <64b81eea-641c-811d-9830-718b28db4188@samsung.com>

Hi Andrew-sh.Cheng,

The exynos-bus.c used the passive governor.
Even if don't make the problem because DEVFREQ_PARENT_DEV is zero,
you need to initialize the parent_type with DEVFREQ_PARENT_DEV as following:

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 8fa8eb541373..1c71c47bc2ac 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -369,6 +369,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
                return -ENOMEM;
 
        passive_data->parent = parent_devfreq;
+       passive_data->parent_type = DEVFREQ_PARENT_DEV;
 
        /* Add devfreq device for exynos bus with passive governor */
        bus->devfreq = devm_devfreq_add_device(dev, profile, DEVFREQ_GOV_PASSIVE,


On 5/28/20 3:14 PM, Chanwoo Choi wrote:
> Hi Andrew-sh.Cheng,
> 
> Thanks for your posting. I like this approach absolutely.
> I think that it is necessary. When I developed the embedded product,
> I needed this feature always. 
> 
> I add the comments on below.
> 
> 
> And the following email is not valid. So, I dropped this email
> from Cc list.
> Saravana Kannan <skannan@codeaurora.org>
> 
> 
> On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
>> From: Saravana Kannan <skannan@codeaurora.org>
>>
>> Many CPU architectures have caches that can scale independent of the
>> CPUs. Frequency scaling of the caches is necessary to make sure that the
>> cache is not a performance bottleneck that leads to poor performance and
>> power. The same idea applies for RAM/DDR.
>>
>> To achieve this, this patch adds support for cpu based scaling to the
>> passive governor. This is accomplished by taking the current frequency
>> of each CPU frequency domain and then adjust the frequency of the cache
>> (or any devfreq device) based on the frequency of the CPUs. It listens
>> to CPU frequency transition notifiers to keep itself up to date on the
>> current CPU frequency.
>>
>> To decide the frequency of the device, the governor does one of the
>> following:
>> * Derives the optimal devfreq device opp from required-opps property of
>>   the parent cpu opp_table.
>>
>> * Scales the device frequency in proportion to the CPU frequency. So, if
>>   the CPUs are running at their max frequency, the device runs at its
>>   max frequency. If the CPUs are running at their min frequency, the
>>   device runs at its min frequency. It is interpolated for frequencies
>>   in between.
>>
>> Andrew-sh.Cheng change
>> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
>> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
>> for kernel-5.7
>>
>> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
>> [Sibi: Integrated cpu-freqmap governor into passive_governor]
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
>> ---
>>  drivers/devfreq/Kconfig            |   2 +
>>  drivers/devfreq/governor_passive.c | 278 ++++++++++++++++++++++++++++++++++---
>>  include/linux/devfreq.h            |  40 +++++-
>>  3 files changed, 299 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index 0b1df12e0f21..d9067950af6a 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE
>>  	  device. This governor does not change the frequency by itself
>>  	  through sysfs entries. The passive governor recommends that
>>  	  devfreq device uses the OPP table to get the frequency/voltage.
>> +	  Alternatively the governor can also be chosen to scale based on
>> +	  the online CPUs current frequency.
>>  
>>  comment "DEVFREQ Drivers"
>>  
>> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
>> index 2d67d6c12dce..7dcda02a5bb7 100644
>> --- a/drivers/devfreq/governor_passive.c
>> +++ b/drivers/devfreq/governor_passive.c
>> @@ -8,11 +8,89 @@
>>   */
>>  
>>  #include <linux/module.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpufreq.h>
>> +#include <linux/cpumask.h>
>>  #include <linux/device.h>
>>  #include <linux/devfreq.h>
>> +#include <linux/slab.h>
>>  #include "governor.h"
>>  
>> -static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
>> +static unsigned int xlate_cpufreq_to_devfreq(struct devfreq_passive_data *data,
> 
> Need to change 'unsigned int' to 'unsigned long'.
> 
>> +					     unsigned int cpu)
>> +{
>> +	unsigned int cpu_min, cpu_max, dev_min, dev_max, cpu_percent, max_state;
> 
> Better to define them separately as following and then need to rename
> the variable. Usually, use the 'min_freq' and 'max_freq' word for
> the minimum/maximum frequency.
> 
> 	unsigned int cpu_min_freq, cpu_max_freq, cpu_curr_freq, cpu_percent;
> 	unsigned long dev_min_freq, dev_max_freq, dev_max_state,
> 
> The devfreq used 'unsigned long'. The cpufreq used 'unsigned long'
> and 'unsigned int'. You need to handle them properly.
> 
> 
>> +	struct devfreq_cpu_state *cpu_state = data->cpu_state[cpu];
>> +	struct devfreq *devfreq = (struct devfreq *)data->this;
>> +	unsigned long *freq_table = devfreq->profile->freq_table;
> 
> In this function, use 'cpu' work for cpufreq and use 'dev' for devfreq.
> So, I think 'dev_freq_table' is proper name instead of 'freq_table'
> for the readability.
> 
> 	freq_table -> dev_freq_table
> 
>> +	struct dev_pm_opp *opp = NULL, *cpu_opp = NULL;
> 
> In the get_target_freq_with_devfreq(), use 'p_opp' indicating
> the OPP of parent device. For the consistency, I think that
> use 'p_opp' instead of 'cpu_opp'. 
> 
>> +	unsigned long cpu_freq, freq;
> 
> Define the 'cpu_freq' on above with cpu_min_freq/cpu_max_freq definition.
> 	cpu_freq -> cpu_curr_freq.
> 
>> +
>> +	if (!cpu_state || cpu_state->first_cpu != cpu ||
>> +	    !cpu_state->opp_table || !devfreq->opp_table)
>> +		return 0;
>> +
>> +	cpu_freq = cpu_state->freq * 1000;
>> +	cpu_opp = devfreq_recommended_opp(cpu_state->dev, &cpu_freq, 0);
>> +	if (IS_ERR(cpu_opp))
>> +		return 0;
>> +
>> +	opp = dev_pm_opp_xlate_required_opp(cpu_state->opp_table,
>> +					    devfreq->opp_table, cpu_opp);
>> +	dev_pm_opp_put(cpu_opp);
>> +
>> +	if (!IS_ERR(opp)) {
>> +		freq = dev_pm_opp_get_freq(opp);
>> +		dev_pm_opp_put(opp);
> 
> Better to add the 'out' goto statement.
> If you use 'goto out', you can reduce the one indentation
> without 'else' statement.
> 	
> 
>> +	} else {
> 
> As I commented, when dev_pm_opp_xlate_required_opp() return successfully
> , use 'goto out'. We can remove 'else' and then reduce the unneeded indentation.
> 
> 
>> +		/* Use Interpolation if required opps is not available */
>> +		cpu_min = cpu_state->min_freq;
>> +		cpu_max = cpu_state->max_freq;
>> +		cpu_freq = cpu_state->freq;
>> +
>> +		if (freq_table) {
>> +			/* Get minimum frequency according to sorting order */
>> +			max_state = freq_table[devfreq->profile->max_state - 1];
>> +			if (freq_table[0] < max_state) {
>> +				dev_min = freq_table[0];
>> +				dev_max = max_state;
>> +			} else {
>> +				dev_min = max_state;
>> +				dev_max = freq_table[0];
>> +			}
>> +		} else {
>> +			if (devfreq->user_max_freq_req.data.freq.qos->max_freq.target_value
>> +			    <= devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value)
>> +				return 0;
>> +			dev_min =
>> +			devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value;
>> +			dev_max =
>> +			devfreq->user_max_freq_req.data.freq.qos->max_freq.target_value;
> 
> I think it is not proper to access the variable of pm_qos structure directly.
> Instead of direct access, you have to use the exported PM QoS function such as
> - pm_qos_read_value(devfreq->dev.parent, DEV_PM_QOS_MIN_FREQUENCY);
> - pm_qos_read_value(devfreq->dev.parent, DEV_PM_QOS_MAX_FREQUENCY);
> 
>> +		}
>> +		cpu_percent = ((cpu_freq - cpu_min) * 100) / cpu_max - cpu_min;
>> +		freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
>> +	}
> 
> 
> I think that you better to add 'out' jump label as following:
> 
> out:
> 
>> +
>> +	return freq;
>> +}
>> +
>> +static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
>> +					unsigned long *freq)
>> +{
>> +	struct devfreq_passive_data *p_data =
>> +				(struct devfreq_passive_data *)devfreq->data;
>> +	unsigned int cpu, target_freq = 0;
> 
> Need to define 'target_freq' with 'unsigned long' type.
> 
>> +
>> +	for_each_online_cpu(cpu)
>> +		target_freq = max(target_freq,
>> +				  xlate_cpufreq_to_devfreq(p_data, cpu));
>> +
>> +	*freq = target_freq;
>> +
>> +	return 0;
>> +}
>> +
>> +static int get_target_freq_with_devfreq(struct devfreq *devfreq,
>>  					unsigned long *freq)
>>  {
>>  	struct devfreq_passive_data *p_data
>> @@ -23,16 +101,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
>>  	int i, count, ret = 0;
>>  
>>  	/*
>> -	 * If the devfreq device with passive governor has the specific method
>> -	 * to determine the next frequency, should use the get_target_freq()
>> -	 * of struct devfreq_passive_data.
>> -	 */
>> -	if (p_data->get_target_freq) {
>> -		ret = p_data->get_target_freq(devfreq, freq);
>> -		goto out;
>> -	}
>> -
>> -	/*
>>  	 * If the parent and passive devfreq device uses the OPP table,
>>  	 * get the next frequency by using the OPP table.
>>  	 */
>> @@ -102,6 +170,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
>>  	return ret;
>>  }
>>  
>> +static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
>> +					   unsigned long *freq)
>> +{
>> +	struct devfreq_passive_data *p_data =
>> +				(struct devfreq_passive_data *)devfreq->data;
>> +	int ret;
>> +
>> +	/*
>> +	 * If the devfreq device with passive governor has the specific method
>> +	 * to determine the next frequency, should use the get_target_freq()
>> +	 * of struct devfreq_passive_data.
>> +	 */
>> +	if (p_data->get_target_freq)
>> +		return p_data->get_target_freq(devfreq, freq);
>> +
>> +	switch (p_data->parent_type) {
>> +	case DEVFREQ_PARENT_DEV:
>> +		ret = get_target_freq_with_devfreq(devfreq, freq);
>> +		break;
>> +	case CPUFREQ_PARENT_DEV:
>> +		ret = get_target_freq_with_cpufreq(devfreq, freq);
>> +		break;
>> +	default:
>> +		ret = -EINVAL;
>> +		dev_err(&devfreq->dev, "Invalid parent type\n");
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>  static int update_devfreq_passive(struct devfreq *devfreq, unsigned long freq)
>>  {
>>  	int ret;
>> @@ -156,6 +255,140 @@ static int devfreq_passive_notifier_call(struct notifier_block *nb,
>>  	return NOTIFY_DONE;
>>  }
>>  
>> +static int cpufreq_passive_notifier_call(struct notifier_block *nb,
>> +					 unsigned long event, void *ptr)
>> +{
>> +	struct devfreq_passive_data *data =
>> +			container_of(nb, struct devfreq_passive_data, nb);
>> +	struct devfreq *devfreq = (struct devfreq *)data->this;
>> +	struct devfreq_cpu_state *cpu_state;
>> +	struct cpufreq_freqs *freq = ptr;
> 
> How about changing 'freq' to 'cpu_freqs'?
> 
> In the drivers/cpufreq/cpufreq.c, use 'freqs' name indicating
> the instance of 'struct cpufreq_freqs'. And in order to
> identfy, how about adding 'cpu_' prefix for variable name?
> 
>> +	unsigned int current_freq;
> 
> Need to define curr_freq with 'unsigned long' type
> and better to use 'curr_freq' variable name.
> 
>> +	int ret;
>> +
>> +	if (event != CPUFREQ_POSTCHANGE || !freq ||
>> +	    !data->cpu_state[freq->policy->cpu])
>> +		return 0;
>> +
>> +	cpu_state = data->cpu_state[freq->policy->cpu];
>> +	if (cpu_state->freq == freq->new)
>> +		return 0;
>> +
>> +	/* Backup current freq and pre-update cpu state freq*/
>> +	current_freq = cpu_state->freq;
>> +	cpu_state->freq = freq->new;
>> +
>> +	mutex_lock(&devfreq->lock);
>> +	ret = update_devfreq(devfreq);
>> +	mutex_unlock(&devfreq->lock);
>> +	if (ret) {
>> +		cpu_state->freq = current_freq;
>> +		dev_err(&devfreq->dev, "Couldn't update the frequency.\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int cpufreq_passive_register(struct devfreq_passive_data **p_data)
>> +{
>> +	struct devfreq_passive_data *data = *p_data;
>> +	struct devfreq *devfreq = (struct devfreq *)data->this;
>> +	struct device *dev = devfreq->dev.parent;
>> +	struct opp_table *opp_table = NULL;
>> +	struct devfreq_cpu_state *state;
> 
> For the readability, I thinkt 'cpu_state' is proper instead of 'state'.
> 
>> +	struct cpufreq_policy *policy;
>> +	struct device *cpu_dev;
>> +	unsigned int cpu;
>> +	int ret;
>> +
>> +	get_online_cpus();
> 
> Add blank line.
> 
>> +	data->nb.notifier_call = cpufreq_passive_notifier_call;
>> +	ret = cpufreq_register_notifier(&data->nb,
>> +					CPUFREQ_TRANSITION_NOTIFIER);
>> +	if (ret) {
>> +		dev_err(dev, "Couldn't register cpufreq notifier.\n");
>> +		data->nb.notifier_call = NULL;
>> +		goto out;
>> +	}
>> +
>> +	/* Populate devfreq_cpu_state */
>> +	for_each_online_cpu(cpu) {
>> +		if (data->cpu_state[cpu])
>> +			continue;
>> +
>> +		policy = cpufreq_cpu_get(cpu);
> 
> cpufreq_cpu_get() might return 'NULL'. I think you need to handle
> return value as following:
> 
> 		if (!policy) {
> 			ret = -EINVAL;
> 			goto out;
> 		} else if (PTR_ERR(policy) == -EPROBE_DEFER) {
> 			goto out;
> 		} else if (IS_ERR(policy) {
> 			ret = PTR_ERR(policy);
> 			dev_err(dev, "Couldn't get the cpufreq_poliy.\n");
> 			goto out;
> 		}
> 
> If cpufreq_cpu_get() return successfully, to do next.
> It reduces the one indentaion.
> 
> 
> 
>> +		if (policy) {
>> +			state = kzalloc(sizeof(*state), GFP_KERNEL);
>> +			if (!state) {
>> +				ret = -ENOMEM;
>> +				goto out;
>> +			}
>> +
>> +			cpu_dev = get_cpu_device(cpu);
>> +			if (!cpu_dev) {
>> +				dev_err(dev, "Couldn't get cpu device.\n");
>> +				ret = -ENODEV;
>> +				goto out;
>> +			}
>> +
>> +			opp_table = dev_pm_opp_get_opp_table(cpu_dev);
>> +			if (IS_ERR(devfreq->opp_table)) {
>> +				ret = PTR_ERR(opp_table);
>> +				goto out;
>> +			}
>> +
>> +			state->dev = cpu_dev;
>> +			state->opp_table = opp_table;
>> +			state->first_cpu = cpumask_first(policy->related_cpus);
>> +			state->freq = policy->cur;
>> +			state->min_freq = policy->cpuinfo.min_freq;
>> +			state->max_freq = policy->cpuinfo.max_freq;
>> +			data->cpu_state[cpu] = state;
> 
> Add blank line.
> 
>> +			cpufreq_cpu_put(policy);
>> +		} else {
>> +			ret = -EPROBE_DEFER;
>> +			goto out;
>> +		}
>> +	}
> 
> Add blank line.
> 
>> +out:
>> +	put_online_cpus();
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Update devfreq */
>> +	mutex_lock(&devfreq->lock);
>> +	ret = update_devfreq(devfreq);
>> +	mutex_unlock(&devfreq->lock);
>> +	if (ret)
>> +		dev_err(dev, "Couldn't update the frequency.\n");
>> +
>> +	return ret;
>> +}
>> +
>> +static int cpufreq_passive_unregister(struct devfreq_passive_data **p_data)
>> +{
>> +	struct devfreq_passive_data *data = *p_data;
>> +	struct devfreq_cpu_state *cpu_state;
>> +	int cpu;
>> +
>> +	if (data->nb.notifier_call)
>> +		cpufreq_unregister_notifier(&data->nb,
>> +					    CPUFREQ_TRANSITION_NOTIFIER);
>> +
>> +	for_each_possible_cpu(cpu) {
>> +		cpu_state = data->cpu_state[cpu];
>> +		if (cpu_state) {
>> +			if (cpu_state->opp_table)
>> +				dev_pm_opp_put_opp_table(cpu_state->opp_table);
>> +			kfree(cpu_state);
>> +			cpu_state = NULL;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int devfreq_passive_event_handler(struct devfreq *devfreq,
>>  				unsigned int event, void *data)
>>  {
>> @@ -165,7 +398,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
>>  	struct notifier_block *nb = &p_data->nb;
>>  	int ret = 0;
>>  
>> -	if (!parent)
>> +	if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
>>  		return -EPROBE_DEFER;
> 
> If you modify the devfreq_passive_event_handler() as following,
> you can move this condition for DEVFREQ_PARENT_DEV into 
> (register|unregister)_parent_dev_notifier.
> 
> 	switch (event) {                                                                                  
> 	case DEVFREQ_GOV_START:                                               
> 		ret = register_parent_dev_notifier(p_data);
> 		break;
> 	case DEVFREQ_GOV_STOP:                                             
> 		ret = unregister_parent_dev_notifier(p_data);
> 		break;
> 	default: 
> 		ret = -EINVAL;
> 		break;
> 	}
>                                                                                               
> 	return ret;
> 
>>  
>>  	switch (event) {
>> @@ -173,13 +406,24 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
>>  		if (!p_data->this)
>>  			p_data->this = devfreq;
>>  
>> -		nb->notifier_call = devfreq_passive_notifier_call;
>> -		ret = devfreq_register_notifier(parent, nb,
>> -					DEVFREQ_TRANSITION_NOTIFIER);
>> +		if (p_data->parent_type == DEVFREQ_PARENT_DEV) {
>> +			nb->notifier_call = devfreq_passive_notifier_call;
>> +			ret = devfreq_register_notifier(parent, nb,
>> +						DEVFREQ_TRANSITION_NOTIFIER);
>> +		} else if (p_data->parent_type == CPUFREQ_PARENT_DEV) {
>> +			ret = cpufreq_passive_register(&p_data);
> 
> I think that we better to collect the code related to notifier registration
> into one function like devfreq_pass_register_notifier() instead of
> cpufreq_passive_register() as following: I think it is more simple and readable.
> 
> If you have more proper function name of register_parent_dev_notifier,
> please give your opinion.
> 
> 
> 	int register_parent_dev_notifier(struct devfreq_passive_data **p_data)
> 		switch (p_data->parent_type) {
> 		case DEVFREQ_PARENT_DEV:
> 			nb->notifier_call = devfreq_passive_notifier_call;
> 			ret = devfreq_register_notifier(parent, nb,
> 			break;
> 		case CPUFREQ_PARENT_DEV:
> 			cpufreq_register_notifier(...)
> 			...
> 			break;
> 		}
> 		
> 
>> +		} else {
>> +			ret = -EINVAL;
>> +		}
>>  		break;
>>  	case DEVFREQ_GOV_STOP:
>> -		WARN_ON(devfreq_unregister_notifier(parent, nb,
>> -					DEVFREQ_TRANSITION_NOTIFIER));
>> +		if (p_data->parent_type == DEVFREQ_PARENT_DEV)
>> +			WARN_ON(devfreq_unregister_notifier(parent, nb,
>> +						DEVFREQ_TRANSITION_NOTIFIER));
>> +		else if (p_data->parent_type == CPUFREQ_PARENT_DEV)
>> +			cpufreq_passive_unregister(&p_data);
>> +		else
>> +			ret = -EINVAL;
> 
> ditto. unregister_parent_dev_notifier(struct devfreq_passive_data **p_data)
> 
>>  		break;
>>  	default:
>>  		break;
>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>> index a4b19d593151..04ce576fd6f1 100644
>> --- a/include/linux/devfreq.h
>> +++ b/include/linux/devfreq.h
>> @@ -278,6 +278,32 @@ struct devfreq_simple_ondemand_data {
>>  
>>  #if IS_ENABLED(CONFIG_DEVFREQ_GOV_PASSIVE)
>>  /**
>> + * struct devfreq_cpu_state - holds the per-cpu state
>> + * @freq:	the current frequency of the cpu.
>> + * @min_freq:	the min frequency of the cpu.
>> + * @max_freq:	the max frequency of the cpu.
>> + * @first_cpu:	the cpumask of the first cpu of a policy.
>> + * @dev:	reference to cpu device.
>> + * @opp_table:	reference to cpu opp table.
>> + *
>> + * This structure stores the required cpu_state of a cpu.
>> + * This is auto-populated by the governor.
>> + */
>> +struct devfreq_cpu_state {> +	unsigned int freq;
> 
> It is better to change from 'freq' to 'curr_freq'
> for more correct expression.
> 
>> +	unsigned int min_freq;
>> +	unsigned int max_freq;
>> +	unsigned int first_cpu;
>> +	struct device *dev;
> 
> How about changing the name 'dev' to 'cpu_dev'?
> 
> 
>> +	struct opp_table *opp_table;
>> +};
> 
> devfreq_cpu_state is only handled by within driver/devfreq/governor_passive.c.
> 
> So, you can move it into drivers/devfreq/governor_passive.c
> and just add the definition into include/linux/devfreq.h as following:
> It is able to prevent the access of variable of 'struct devfreq_cpu_state'
> outside.
> 
> 	struct devfreq_cpu_state;
> 
>> +
>> +enum devfreq_parent_dev_type {
>> +	DEVFREQ_PARENT_DEV,
>> +	CPUFREQ_PARENT_DEV,
>> +};
>> +
>> +/**
>>   * struct devfreq_passive_data - ``void *data`` fed to struct devfreq
>>   *	and devfreq_add_device
>>   * @parent:	the devfreq instance of parent device.
>> @@ -288,13 +314,15 @@ struct devfreq_simple_ondemand_data {
>>   *			using governors except for passive governor.
>>   *			If the devfreq device has the specific method to decide
>>   *			the next frequency, should use this callback.
>> - * @this:	the devfreq instance of own device.
>> - * @nb:		the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
>> + * @parent_type		parent type of the device
> 
> Need to add ':' at the end of word. -> "parent_type:".
> 
>> + * @this:		the devfreq instance of own device.
>> + * @nb:			the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
> 
> I knew that you make them with same indentation.
> But, actually, it is not related to this patch like clean-up code.
> Even if it is not pretty, you better to don't touch 'this' and 'nb' indentaion.
> 
>> + * @cpu_state:		the state min/max/current frequency of all online cpu's
>>   *
>>   * The devfreq_passive_data have to set the devfreq instance of parent
>>   * device with governors except for the passive governor. But, don't need to
>> - * initialize the 'this' and 'nb' field because the devfreq core will handle
>> - * them.
>> + * initialize the 'this', 'nb' and 'cpu_state' field because the devfreq core
>> + * will handle them.
>>   */
>>  struct devfreq_passive_data {
>>  	/* Should set the devfreq instance of parent device */
>> @@ -303,9 +331,13 @@ struct devfreq_passive_data {
>>  	/* Optional callback to decide the next frequency of passvice device */
>>  	int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
>>  
>> +	/* Should set the type of parent device */
>> +	enum devfreq_parent_dev_type parent_type;
>> +
>>  	/* For passive governor's internal use. Don't need to set them */
>>  	struct devfreq *this;
>>  	struct notifier_block nb;
>> +	struct devfreq_cpu_state *cpu_state[NR_CPUS];
>>  };
>>  #endif
>>  
>>
> 
> 


-- 
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 related

* Re: [PATCH v2 0/6] prctl.2 man page updates for Linux 5.6
From: Michael Kerrisk (man-pages) @ 2020-05-28  7:11 UTC (permalink / raw)
  To: Dave Martin
  Cc: linux-arch, linux-man, Will Deacon, Catalin Marinas, Dave Hansen,
	Amit Daniel Kachhap, Waiman Long, Mark Rutland, Vincenzo Frascino,
	Tim Chen, Thomas Gleixner, linux-arm-kernel
In-Reply-To: <1590614258-24728-1-git-send-email-Dave.Martin@arm.com>

Hi Dave

On Wed, 27 May 2020 at 23:17, Dave Martin <Dave.Martin@arm.com> wrote:
>
> A bunch of updates to the prctl(2) man page to fill in missing
> prctls (mostly) up to Linux 5.6 (along with a few other tweaks and
> fixes).
>
> Patches from the v1 series [1] that have been applied or rejected
> already have been dropped.
>
> People not Cc'd on the whole series can find the whole series at
> https://lore.kernel.org/linux-man/ .
>
> Patches:
>
>  * Patch 1 is a new (but trivial) formatting fix, unrelated to the new
>    prctls.

Applied.

>  * Patches 2-3 relate to the speculation control prctls.  These are
>    unmodified from v1, but need review.

Applied, and pushed (since there were no review comments from last version).

>  * Patches 4-5 relate to the arm64 prctls from v1, with reviewer
>    feedback incorporated.  (See notes in the patches.)

I'll hold off on these patches, to see if review comments come in.

>  * Patch 6 is *draft* wording for the arm64 address tagging prctls.
>    The semantics of address tagging is particularly slippery, so
>    this needs discussion before merging.

Okay -- I'll hold off with that patch too.

Cheers,

Michael

> [1] https://lore.kernel.org/linux-man/29a02b16-dd61-6186-1340-fcc7d5225ad0@gmail.com/T/#t
>
>
> Dave Martin (6):
>   prctl.2: ffix use literal hyphens when referencing kernel docs
>   prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls
>   prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls
>   prctl.2: Add SVE prctls (arm64)
>   prctl.2: Add PR_PAC_RESET_KEYS (arm64)
>   prctl.2: Add tagged address ABI control prctls (arm64)
>
>  man2/prctl.2 | 444 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 435 insertions(+), 9 deletions(-)
>
> --
> 2.1.4
>


--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

_______________________________________________
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 v10] Add matrix keypad driver support for Mediatek SoCs
From: Marco Felsch @ 2020-05-28  7:19 UTC (permalink / raw)
  To: Fengping Yu
  Cc: Dmitry Torokhov, linux-mediatek, linux-input, Yingjoe Chen,
	Andy Shevchenko, linux-arm-kernel
In-Reply-To: <20200528053344.25936-1-fengping.yu@mediatek.com>

Hi,

this version looks nice to me =) One last question.

On 20-05-28 13:33, Fengping Yu wrote:
> 
> Change since v9:
> - modify KEYBOARD_MTK_KPD config dependent item
> - remove wakeup member of mtk_keypad struct

You also removed the device wakeup capability completely, was this
intended? What I mean is that we don't need that member within the
driver state struct.

Regards,
  Marco

> - remove default pinctrl state set
> - modify request irq failed return value
> - add space of kepad matching table
> - modify align issue
> 
> fengping.yu (3):
>   dt-bindings: Add keypad devicetree documentation
>   drivers: input: keyboard: Add mtk keypad driver
>   configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
> 
>  .../devicetree/bindings/input/mtk-kpd.yaml    |  95 +++++++++
>  arch/arm64/configs/defconfig                  |   1 +
>  drivers/input/keyboard/Kconfig                |  11 +
>  drivers/input/keyboard/Makefile               |   1 +
>  drivers/input/keyboard/mtk-kpd.c              | 201 ++++++++++++++++++
>  5 files changed, 309 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.yaml
>  create mode 100644 drivers/input/keyboard/mtk-kpd.c
> 
> -- 
> 2.18.0
> 

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

^ permalink raw reply

* Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
From: Will Deacon @ 2020-05-28  7:20 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Naohiro Aota, Stephen Boyd, Catalin Marinas, Masahiro Yamada,
	LKML, Manoj Gupta, Luis Lozano, Nathan Chancellor,
	Vincenzo Frascino, Robin Murphy, Linux ARM
In-Reply-To: <CAKwvOdm5hGzJ0WY_GAywRZ1c8MkA=H7imY0rrVgB4MgtyJ+iRg@mail.gmail.com>

On Wed, May 27, 2020 at 11:17:33AM -0700, Nick Desaulniers wrote:
> On Wed, May 27, 2020 at 11:08 AM Will Deacon <will@kernel.org> wrote:
> >
> > On Wed, May 27, 2020 at 10:55:24AM -0700, Nick Desaulniers wrote:
> > > On Wed, May 27, 2020 at 6:45 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > >
> > > > On 2020-05-26 18:31, Nick Desaulniers wrote:
> > > > > Custom toolchains that modify the default target to -mthumb cannot
> > > > > compile the arm64 compat vdso32, as
> > > > > arch/arm64/include/asm/vdso/compat_gettimeofday.h
> > > > > contains assembly that's invalid in -mthumb.  Force the use of -marm,
> > > > > always.
> > > >
> > > > FWIW, this seems suspicious - the only assembly instructions I see there
> > > > are SWI(SVC), MRRC, and a MOV, all of which exist in Thumb for the
> > > > -march=armv7a baseline that we set.
> > > >
> > > > On a hunch, I've just bodged "VDSO_CFLAGS += -mthumb" into my tree and
> > > > built a Thumb VDSO quite happily with Ubuntu 19.04's
> > > > gcc-arm-linux-gnueabihf. What was the actual failure you saw?
> > >
> > > From the link in the commit message: `write to reserved register 'R7'`
> > > https://godbolt.org/z/zwr7iZ
> > > IIUC r7 is reserved for the frame pointer in THUMB?
> > >
> > > What is the implicit default of your gcc-arm-linux-gnueabihf at -O2?
> > > -mthumb, or -marm?
> >
> > Hmm, but this *is* weird because if I build a 32-bit kernel then I get
> > either an ARM or a Thumb-2 VDSO depending on CONFIG_THUMB2_KERNEL. I'm
> > not sure if that's deliberate, but both build and appear to work.
> 
> That's because there's 3 VDSO's when it comes to ARM:
> arm64's 64b vdso: arch/arm64/kernel/vdso
> arm64's 32b vdso: arch/arm64/kernel/vdso32/
> arm's 32b vdso: arch/arm/kernel/vdso.c

Yes, I know that :)

> When you build a 32b kernel, you're only making use of the last of
> those three; the arm64 vdso and vdso32 code is irrelevant.
> This patch is specific to the second case, which is the 32b compat
> vdso for a 64b kernel.

Sure, but if you can build a Thumb-2 vDSO object for arch/arm/ using then
we should be able to build a Thumb-2 compat vDSO for arch/arm64, and your
patch is papering over a deeper issue. Generally, having the compat vDSO
behave differently to the arch/arm/ vDSO is indicative of something being
broken.

In other words, if your patch was correct (not sure that it is) then I
would expect a corresponding change to arch/arm/ to pass -marm when
CONFIG_THUMB2_KERNEL=y. Make sense?

Will

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

^ permalink raw reply

* Re: linux-next: build warning after merge of the aspeed tree
From: Joel Stanley @ 2020-05-28  7:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Stephen Rothwell, devicetree, Andrew Jeffery,
	Linux Kernel Mailing List, Vijay Khemka, Linux Next Mailing List,
	Rob Herring, ARM, Olof Johansson, Manikandan Elumalai,
	Devicetree Compiler, David Gibson
In-Reply-To: <CACPK8Xdm91DwuKcm_d9xh_+8gPzxWpWWAzJzq8pAFVc79x-q1A@mail.gmail.com>

On Thu, 28 May 2020 at 06:09, Joel Stanley <joel@jms.id.au> wrote:
>
> On Fri, 22 May 2020 at 08:16, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Fri, May 22, 2020 at 2:16 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > On Wed, 20 May 2020 07:56:36 +0000 Joel Stanley <joel@jms.id.au> wrote:
> > > > I've sent the patch so it applies to the dtc tree. It would be good to
> > > > see that change propagate over to -next as others have reported this
> > > > warning.
> > >
> > > These warnings now appear in the arm-soc tree.
> >
> > Right, I also saw them earlier.
> >
> > Joel, have you sent your patch to David Gibson for integration into
> > upstream dtc?
> > I don't know who sent the other patch, but as long as one of them
> > gets merged, I'd hope we can pull that into kernel as well.
>
> David asked for some extra features (and a typo fix) before he would
> merge it. I'll take a look at that now.

Here we go:

https://lore.kernel.org/lkml/20200528072037.1402346-1-joel@jms.id.au/

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

^ permalink raw reply

* Re: [RFC PATCH] iommu/arm-smmu: Add module parameter to set msi iova address
From: Auger Eric @ 2020-05-28  7:21 UTC (permalink / raw)
  To: Robin Murphy, Srinath Mannam, Will Deacon, Joerg Roedel
  Cc: Jean-Philippe Brucker, iommu, linux-kernel, Alex Williamson,
	bcm-kernel-feedback-list, linux-arm-kernel
In-Reply-To: <f9b221cf-1c7f-9f95-133b-dca65197b6c2@arm.com>

Hi,

On 5/27/20 7:30 PM, Robin Murphy wrote:
> On 2020-05-27 17:03, Srinath Mannam wrote:
>> This patch gives the provision to change default value of MSI IOVA base
>> to platform's suitable IOVA using module parameter. The present
>> hardcoded MSI IOVA base may not be the accessible IOVA ranges of
>> platform.
> 
> That in itself doesn't seem entirely unreasonable; IIRC the current
> address is just an arbitrary choice to fit nicely into Qemu's memory
> map,
correct
 and there was always the possibility that it wouldn't suit everything.

Indeed I also recently had this case of PCI host bridge collision with
the SW MSI reserved window - maybe that's the same ;-) -.
> 
>> Since commit aadad097cd46 ("iommu/dma: Reserve IOVA for PCIe inaccessible
>> DMA address"), inaccessible IOVA address ranges parsed from dma-ranges
>> property are reserved.
> 
> That, however, doesn't seem to fit here; iommu-dma maps MSI doorbells
> dynamically, so they aren't affected by reserved regions any more than
> regular DMA pages are. In fact, it explicitly ignores the software MSI
> region, since as the comment says, it *is* the software that manages those.
> 
> The MSI_IOVA_BASE region exists for VFIO, precisely because in that case
> the kernel *doesn't* control the address space, but still needs some way
> to steal a bit of it for MSIs that the guest doesn't necessarily know
> about, and give userspace a fighting chance of knowing what it's taken.
> I think at the time we discussed the idea of adding something to the
> VFIO uapi such that userspace could move this around if it wanted or
> needed to, but decided we could live without that initially.

Yes indeed ;-)

 Perhaps now
> the time has come?

Do you mean you would welcome a VFIO based approach or would a driver
parameter be sufficient?

Thanks

Eric


> 
> Robin.
> 
>> If any platform has the limitaion to access default MSI IOVA, then it can
>> be changed using "arm-smmu.msi_iova_base=0xa0000000" command line
>> argument.
>>
>> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
>> ---
>>   drivers/iommu/arm-smmu.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 4f1a350..5e59c9d 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -72,6 +72,9 @@ static bool disable_bypass =
>>   module_param(disable_bypass, bool, S_IRUGO);
>>   MODULE_PARM_DESC(disable_bypass,
>>       "Disable bypass streams such that incoming transactions from
>> devices that are not attached to an iommu domain will report an abort
>> back to the device and will not be allowed to pass through the SMMU.");
>> +static unsigned long msi_iova_base = MSI_IOVA_BASE;
>> +module_param(msi_iova_base, ulong, S_IRUGO);
>> +MODULE_PARM_DESC(msi_iova_base, "msi iova base address.");
>>     struct arm_smmu_s2cr {
>>       struct iommu_group        *group;
>> @@ -1566,7 +1569,7 @@ static void arm_smmu_get_resv_regions(struct
>> device *dev,
>>       struct iommu_resv_region *region;
>>       int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
>>   -    region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
>> +    region = iommu_alloc_resv_region(msi_iova_base, MSI_IOVA_LENGTH,
>>                        prot, IOMMU_RESV_SW_MSI);
>>       if (!region)
>>           return;
>>
> 


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

^ permalink raw reply

* Re: [RFC PATCH] iommu/arm-smmu: Add module parameter to set msi iova address
From: Jean-Philippe Brucker @ 2020-05-28  7:23 UTC (permalink / raw)
  To: Srinath Mannam
  Cc: Alex Williamson, Robin Murphy, Joerg Roedel,
	Linux Kernel Mailing List, Eric Auger, iommu, BCM Kernel Feedback,
	Will Deacon, Linux ARM
In-Reply-To: <CABe79T7WwD2AyWp2e5pAi8TO2r5=-v5gPb2Gjtf8EhHOn3dogQ@mail.gmail.com>

On Thu, May 28, 2020 at 10:45:14AM +0530, Srinath Mannam wrote:
> On Wed, May 27, 2020 at 11:00 PM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> Thanks Robin for your quick response.
> > On 2020-05-27 17:03, Srinath Mannam wrote:
> > > This patch gives the provision to change default value of MSI IOVA base
> > > to platform's suitable IOVA using module parameter. The present
> > > hardcoded MSI IOVA base may not be the accessible IOVA ranges of platform.
> >
> > That in itself doesn't seem entirely unreasonable; IIRC the current
> > address is just an arbitrary choice to fit nicely into Qemu's memory
> > map, and there was always the possibility that it wouldn't suit everything.
> >
> > > Since commit aadad097cd46 ("iommu/dma: Reserve IOVA for PCIe inaccessible
> > > DMA address"), inaccessible IOVA address ranges parsed from dma-ranges
> > > property are reserved.

I don't understand why we only reserve the PCIe windows for DMA domains.
Shouldn't VFIO also prevent userspace from mapping them?  If they were
part of the common reserved regions then we could have VFIO choose a
SW_MSI region among the remaining free space. It would just need a
different way of asking the IOMMU driver if a SW_MSI is needed, for
example with a domain attribute.

Thanks,
Jean

> >
> > That, however, doesn't seem to fit here; iommu-dma maps MSI doorbells
> > dynamically, so they aren't affected by reserved regions any more than
> > regular DMA pages are. In fact, it explicitly ignores the software MSI
> > region, since as the comment says, it *is* the software that manages those.
> Yes you are right, we don't see any issues with kernel drivers(PCI EP) because
> MSI IOVA allocated dynamically by honouring reserved regions same as DMA pages.
> >
> > The MSI_IOVA_BASE region exists for VFIO, precisely because in that case
> > the kernel *doesn't* control the address space, but still needs some way
> > to steal a bit of it for MSIs that the guest doesn't necessarily know
> > about, and give userspace a fighting chance of knowing what it's taken.
> > I think at the time we discussed the idea of adding something to the
> > VFIO uapi such that userspace could move this around if it wanted or
> > needed to, but decided we could live without that initially. Perhaps now
> > the time has come?
> Yes, we see issues only with user-space drivers(DPDK) in which MSI_IOVA_BASE
> region is considered to map MSI registers. This patch helps us to fix the issue.
> 
> Thanks,
> Srinath.
> >
> > Robin.
> >
> > > If any platform has the limitaion to access default MSI IOVA, then it can
> > > be changed using "arm-smmu.msi_iova_base=0xa0000000" command line argument.
> > >
> > > Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> > > ---
> > >   drivers/iommu/arm-smmu.c | 5 ++++-
> > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > > index 4f1a350..5e59c9d 100644
> > > --- a/drivers/iommu/arm-smmu.c
> > > +++ b/drivers/iommu/arm-smmu.c
> > > @@ -72,6 +72,9 @@ static bool disable_bypass =
> > >   module_param(disable_bypass, bool, S_IRUGO);
> > >   MODULE_PARM_DESC(disable_bypass,
> > >       "Disable bypass streams such that incoming transactions from devices that are not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through the SMMU.");
> > > +static unsigned long msi_iova_base = MSI_IOVA_BASE;
> > > +module_param(msi_iova_base, ulong, S_IRUGO);
> > > +MODULE_PARM_DESC(msi_iova_base, "msi iova base address.");
> > >
> > >   struct arm_smmu_s2cr {
> > >       struct iommu_group              *group;
> > > @@ -1566,7 +1569,7 @@ static void arm_smmu_get_resv_regions(struct device *dev,
> > >       struct iommu_resv_region *region;
> > >       int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> > >
> > > -     region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
> > > +     region = iommu_alloc_resv_region(msi_iova_base, MSI_IOVA_LENGTH,
> > >                                        prot, IOMMU_RESV_SW_MSI);
> > >       if (!region)
> > >               return;
> > >

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

^ permalink raw reply

* Re: [V9, 1/2] media: dt-bindings: media: i2c: Document OV02A10 bindings
From: Sakari Ailus @ 2020-05-28  7:23 UTC (permalink / raw)
  To: Dongchun Zhu
  Cc: Mark Rutland, Rob Herring, Andy Shevchenko, srv_heupstream,
	devicetree, Linus Walleij,
	Shengnan Wang (王圣男), Tomasz Figa,
	Bartosz Golaszewski, Sj Huang, Nicolas Boichat,
	moderated list:ARM/Mediatek SoC support, Louis Kuo,
	Matthias Brugger, Cao Bing Bu, Mauro Carvalho Chehab,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Linux Media Mailing List
In-Reply-To: <1590636882.8804.474.camel@mhfsdcap03>

Hi Dongchun,

On Thu, May 28, 2020 at 11:34:42AM +0800, Dongchun Zhu wrote:
> Hi Sakari, Rob,
> 
> On Thu, 2020-05-28 at 00:16 +0300, Sakari Ailus wrote:
> > Hi Rob, Dongchun,
> > 
> > On Wed, May 27, 2020 at 09:27:22AM -0600, Rob Herring wrote:
> > > > > > +    properties:
> > > > > > +      endpoint:
> > > > > > +        type: object
> > > > > > +        additionalProperties: false
> > > > > > +
> > > > > > +        properties:
> > > >
> > > > Actually I wonder whether we need to declare 'clock-lanes' here?
> > > 
> > > Yes, if you are using it.
> > 
> > Dongchun, can you confirm the chip has a single data and a single clock
> > lane and that it does not support lane reordering?
> > 
> 
> From the datasheet, 'MIPI inside the OV02A10 provides one single
> uni-directional clock lane and one bi-directional data lane solution for
> communication links between components inside a mobile device.
> The data lane has full support for HS(uni-directional) and
> LP(bi-directional) data transfer mode.'
> 
> The sensor doesn't support lane reordering, so 'clock-lanes' property
> would not be added in next release.
> 
> > So if there's nothing to convey to the driver, also the data-lanes should
> > be removed IMO.
> > 
> 
> However, 'data-lanes' property may still be required.
> It is known that either data-lanes or clock-lanes is an array of
> physical data lane indexes. Position of an entry determines the logical
> lane number, while the value of an entry indicates physical lane, e.g.,
> for 1-lane MIPI CSI-2 bus we could have "data-lanes = <1>;", assuming
> the clock lane is on hardware lane 0.
> 
> As mentioned earlier, the OV02A10 sensor supports only 1C1D and does not
> support lane reordering, so here we shall use 'data-lanes = <1>' as
> there is only a clock lane for OV02A10.
> 
> Reminder:
> If 'data-lanes' property is not present, the driver would assume
> four-lane operation. This means for one-lane or two-lane operation, this
> property must be present and set to the right physical lane indexes.
> If the hardware does not support lane reordering, monotonically
> incremented values shall be used from 0 or 1 onwards, depending on
> whether or not there is also a clock lane.

How can the driver use four lanes, considering the device only supports a
single lane??

-- 
Sakari Ailus

_______________________________________________
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 v14 1/2] media: dt-bindings: media: xilinx: Add Xilinx MIPI CSI-2 Rx Subsystem
From: Vishal Sagar @ 2020-05-28  7:25 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, Jacopo Mondi,
	Dinesh Kumar, Hyun Kwon, Sandip Kothari,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org, Michal Simek,
	Luca Ceresoli, hans.verkuil@cisco.com, mchehab@kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <20200527161140.GF6171@pendragon.ideasonboard.com>

Hi Laurent,

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Wednesday, May 27, 2020 9:42 PM
> To: Vishal Sagar <vsagar@xilinx.com>
> Cc: Hyun Kwon <hyunk@xilinx.com>; mchehab@kernel.org;
> robh+dt@kernel.org; mark.rutland@arm.com; Michal Simek
> <michals@xilinx.com>; linux-media@vger.kernel.org;
> devicetree@vger.kernel.org; hans.verkuil@cisco.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Dinesh Kumar
> <dineshk@xilinx.com>; Sandip Kothari <sandipk@xilinx.com>; Luca Ceresoli
> <luca@lucaceresoli.net>; Jacopo Mondi <jacopo@jmondi.org>
> Subject: Re: [PATCH v14 1/2] media: dt-bindings: media: xilinx: Add Xilinx MIPI
> CSI-2 Rx Subsystem
> 
> Hi Vishal,
> 
> Thank you for the patch.
> 
> On Wed, May 27, 2020 at 07:27:18PM +0530, Vishal Sagar wrote:
> > Add bindings documentation for Xilinx MIPI CSI-2 Rx Subsystem.
> >
> > The Xilinx MIPI CSI-2 Rx Subsystem consists of a CSI-2 Rx controller,
> > a D-PHY in Rx mode and a Video Format Bridge.
> >
> > Signed-off-by: Vishal Sagar <vishal.sagar@xilinx.com>
> > Reviewed-by: Hyun Kwon <hyun.kwon@xilinx.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > v14
> > - Removed xlnx,csi-pxl-format from required properties
> > - Added dependency of xlnx,csi-pxl-format on xlnx,vfb
> > - End the yaml file with ...
> > - Added Reviewed by Laurent
> >
> > v13
> > - Based on Laurent's suggestions
> > - Fixed the datatypes values as minimum and maximum
> > - condition added for en-vcx property
> >
> > v12
> > - Moved to yaml format
> > - Update CSI-2 and D-PHY
> > - Mention that bindings for D-PHY not here
> > - reset -> video-reset
> >
> > v11
> > - Modify compatible string from 4.0 to 5.0
> >
> > v10
> > - No changes
> >
> > v9
> > - Fix xlnx,vfb description.
> > - s/Optional/Required endpoint property.
> > - Move data-lanes description from Ports to endpoint property section.
> >
> > v8
> > - Added reset-gpios optional property to assert video_aresetn
> >
> > v7
> > - Removed the control name from dt bindings
> > - Updated the example dt node name to csi2rx
> >
> > v6
> > - Added "control" after V4L2_CID_XILINX_MIPICSISS_ACT_LANES as
> > suggested by Luca
> > - Added reviewed by Rob Herring
> >
> > v5
> > - Incorporated comments by Luca Cersoli
> > - Removed DPHY clock from description and example
> > - Removed bayer pattern from device tree MIPI CSI IP
> >   doesn't deal with bayer pattern.
> >
> > v4
> > - Added reviewed by Hyun Kwon
> >
> > v3
> > - removed interrupt parent as suggested by Rob
> > - removed dphy clock
> > - moved vfb to optional properties
> > - Added required and optional port properties section
> > - Added endpoint property section
> >
> > v2
> > - updated the compatible string to latest version supported
> > - removed DPHY related parameters
> > - added CSI v2.0 related property (including VCX for supporting upto 16
> >   virtual channels).
> > - modified csi-pxl-format from string to unsigned int type where the value
> >   is as per the CSI specification
> > - Defined port 0 and port 1 as sink and source ports.
> > - Removed max-lanes property as suggested by Rob and Sakari
> >
> >  .../bindings/media/xilinx/xlnx,csi2rxss.yaml       | 237
> +++++++++++++++++++++
> >  1 file changed, 237 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > new file mode 100644
> > index 0000000..2282231
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yam
> > +++ l
> > @@ -0,0 +1,237 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/xilinx/xlnx,csi2rxss.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx MIPI CSI-2 Receiver Subsystem
> > +
> > +maintainers:
> > +  - Vishal Sagar <vishal.sagar@xilinx.com>
> > +
> > +description: |
> > +  The Xilinx MIPI CSI-2 Receiver Subsystem is used to capture MIPI
> > +CSI-2
> > +  traffic from compliant camera sensors and send the output as AXI4
> > +Stream
> > +  video data for image processing.
> > +  The subsystem consists of a MIPI D-PHY in slave mode which captures
> > +the
> > +  data packets. This is passed along the MIPI CSI-2 Rx IP which
> > +extracts the
> > +  packet data. The optional Video Format Bridge (VFB) converts this
> > +data to
> > +  AXI4 Stream video data.
> > +  For more details, please refer to PG232 Xilinx MIPI CSI-2 Receiver
> Subsystem.
> > +  Please note that this bindings includes only the MIPI CSI-2 Rx
> > +controller
> > +  and Video Format Bridge and not D-PHY.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +        - xlnx,mipi-csi2-rx-subsystem-5.0
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    description: List of clock specifiers
> > +    items:
> > +      - description: AXI Lite clock
> > +      - description: Video clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: lite_aclk
> > +      - const: video_aclk
> > +
> > +  xlnx,csi-pxl-format:
> > +    description: |
> > +      This denotes the CSI Data type selected in hw design.
> > +      Packets other than this data type (except for RAW8 and
> > +      User defined data types) will be filtered out.
> > +      Possible values are as below -
> > +      0x1e - YUV4228B
> > +      0x1f - YUV42210B
> > +      0x20 - RGB444
> > +      0x21 - RGB555
> > +      0x22 - RGB565
> > +      0x23 - RGB666
> > +      0x24 - RGB888
> > +      0x28 - RAW6
> > +      0x29 - RAW7
> > +      0x2a - RAW8
> > +      0x2b - RAW10
> > +      0x2c - RAW12
> > +      0x2d - RAW14
> > +      0x2e - RAW16
> > +      0x2f - RAW20
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint32
> > +      - anyOf:
> > +        - minimum: 0x1e
> > +        - maximum: 0x24
> > +        - minimum: 0x28
> > +        - maximum: 0x2f
> > +
> > +  xlnx,vfb:
> > +    type: boolean
> > +    description: Present when Video Format Bridge is enabled in IP
> > + configuration
> > +
> > +  xlnx,en-csi-v2-0:
> > +    type: boolean
> > +    description: Present if CSI v2 is enabled in IP configuration.
> > +
> > +  xlnx,en-vcx:
> > +    type: boolean
> > +    description: |
> > +      When present, there are maximum 16 virtual channels, else only 4.
> > +
> > +  xlnx,en-active-lanes:
> > +    type: boolean
> > +    description: |
> > +      Present if the number of active lanes can be re-configured at
> > +      runtime in the Protocol Configuration Register. Otherwise all lanes,
> > +      as set in IP configuration, are always active.
> > +
> > +  video-reset-gpios:
> > +    description: Optional specifier for a GPIO that asserts video_aresetn.
> > +    maxItems: 1
> > +
> > +  ports:
> > +    type: object
> > +
> > +    properties:
> > +      port@0:
> > +        type: object
> > +        description: |
> > +          Input / sink port node, single endpoint describing the
> > +          CSI-2 transmitter.
> > +
> > +        properties:
> > +          reg:
> > +            const: 0
> > +
> > +          endpoint:
> > +            type: object
> > +
> > +            properties:
> > +
> > +              data-lanes:
> > +                description: |
> > +                  This is required only in the sink port 0 endpoint which
> > +                  connects to MIPI CSI-2 source like sensor.
> > +                  The possible values are -
> > +                  1       - For 1 lane enabled in IP.
> > +                  1 2     - For 2 lanes enabled in IP.
> > +                  1 2 3   - For 3 lanes enabled in IP.
> > +                  1 2 3 4 - For 4 lanes enabled in IP.
> > +                items:
> > +                  - const: 1
> > +                  - const: 2
> > +                  - const: 3
> > +                  - const: 4
> > +
> > +              remote-endpoint: true
> > +
> > +            required:
> > +              - data-lanes
> > +              - remote-endpoint
> > +
> > +            additionalProperties: false
> > +
> > +        additionalProperties: false
> > +
> > +      port@1:
> > +        type: object
> > +        description: |
> > +          Output / source port node, endpoint describing modules
> > +          connected the CSI-2 receiver.
> > +
> > +        properties:
> > +
> > +          reg:
> > +            const: 1
> > +
> > +          endpoint:
> > +            type: object
> > +
> > +            properties:
> > +
> > +              remote-endpoint: true
> > +
> > +            required:
> > +              - remote-endpoint
> > +
> > +            additionalProperties: false
> > +
> > +        additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - ports
> > +
> > +allOf:
> > +  - if:
> > +    required:
> > +      - xlnx,vfb
> > +    then:
> > +      required:
> > +        - xlnx,csi-pxl-format
> > +    else:
> > +      properties:
> > +        xlnx,csi-pxl-format: false
> > +
> > +  - if:
> > +    not:
> > +      required:
> > +        - xlnx,en-csi-v2-0
> > +    then:
> > +      properties:
> > +        xlnx,en-vcx: false
> 
> There's an indentation problem here, it should be
> 
> allOf:
>   - if:
>       required:
>         - xlnx,vfb
>     then:
>       required:
>         - xlnx,csi-pxl-format
>     else:
>       properties:
>         xlnx,csi-pxl-format: false
> 
>   - if:
>       not:
>         required:
>           - xlnx,en-csi-v2-0
>     then:
>       properties:
>         xlnx,en-vcx: false
> 
> Have you run the bindings checks ?
> 
> make
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/xilinx/xlnx,csi
> 2rxss.yaml dt_binding_check
> 
> It would have caught the issue.
> 

By mistake the incorrect patch was sent. Apologies for this.

> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    xcsi2rxss_1: csi2rx@a0020000 {
> > +        compatible = "xlnx,mipi-csi2-rx-subsystem-5.0";
> > +        reg = <0x0 0xa0020000 0x0 0x10000>;
> 
> I think I mentioned in a previous review that this should be
> 
>         reg = <0xa0020000 0x10000>;
> 
> even if it doesn't match what the real values, as dt_binding_check compiles
> the examples in the context of a bus that has #address-cells = <1> and #size-
> cells = <1>.
> 
> I can fix these when applying the patches to my tree if that's OK with you, and
> send a pull request.
> 

Yes that is fine. Thanks!

> > +        interrupt-parent = <&gic>;
> > +        interrupts = <0 95 4>;
> > +        xlnx,csi-pxl-format = <0x2a>;
> > +        xlnx,vfb;
> > +        xlnx,en-active-lanes;
> > +        xlnx,en-csi-v2-0;
> > +        xlnx,en-vcx;
> > +        clock-names = "lite_aclk", "video_aclk";
> > +        clocks = <&misc_clk_0>, <&misc_clk_1>;
> > +        video-reset-gpios = <&gpio 86 GPIO_ACTIVE_LOW>;
> > +
> > +        ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            port@0 {
> > +                /* Sink port */
> > +                reg = <0>;
> > +                csiss_in: endpoint {
> > +                    data-lanes = <1 2 3 4>;
> > +                    /* MIPI CSI-2 Camera handle */
> > +                    remote-endpoint = <&camera_out>;
> > +                };
> > +            };
> > +            port@1 {
> > +                /* Source port */
> > +                reg = <1>;
> > +                csiss_out: endpoint {
> > +                    remote-endpoint = <&vproc_in>;
> > +                };
> > +            };
> > +        };
> > +    };
> > +...
> 
> --
> Regards,
> 
> Laurent Pinchart

Regards
Vishal Sagar

_______________________________________________
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 09/12] devfreq: add mediatek cci devfreq
From: Chanwoo Choi @ 2020-05-28  7:35 UTC (permalink / raw)
  To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
	Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
  Cc: devicetree, srv_heupstream, linux-pm, linux-kernel,
	linux-mediatek, linux-arm-kernel
In-Reply-To: <20200520034307.20435-10-andrew-sh.cheng@mediatek.com>

Hi Andrew-sh.Cheng,

On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
> This adds a devfreq driver for the Cache Coherent Interconnect (CCI)
> of the Mediatek MT8183.
> 
> On the MT8183 the CCI is supplied by the same regulator as the LITTLE
> cores. The driver is notified when the regulator voltage changes
> (driven by cpufreq) and adjusts the CCI frequency to the maximum
> possible value.
> 
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> ---
>  drivers/devfreq/Kconfig              |  10 ++
>  drivers/devfreq/Makefile             |   1 +
>  drivers/devfreq/mt8183-cci-devfreq.c | 206 +++++++++++++++++++++++++++++++++++

The mt8183-cci.c is enough for driver name.

>  3 files changed, 217 insertions(+)
>  create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index d9067950af6a..4ed7116271ee 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -103,6 +103,16 @@ config ARM_IMX8M_DDRC_DEVFREQ
>  	  This adds the DEVFREQ driver for the i.MX8M DDR Controller. It allows
>  	  adjusting DRAM frequency.
>  
> +config ARM_MT8183_CCI_DEVFREQ
> +	tristate "MT8183 CCI DEVFREQ Driver"
> +	depends on ARM_MEDIATEK_CPUFREQ
> +	help
> +		This adds a devfreq driver for Cache Coherent Interconnect
> +		of Mediatek MT8183, which is shared the same regulator
> +		with cpu cluster.
> +		It can track buck voltage and update a proper cci frequency.

s/cci/CCI

> +		Use notification to get regulator status.
> +
>  config ARM_TEGRA_DEVFREQ
>  	tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
>  	depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 3eb4d5e6635c..5b1b670c954d 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE)	+= governor_passive.o
>  # DEVFREQ Drivers
>  obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)	+= exynos-bus.o
>  obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
> +obj-$(CONFIG_ARM_MT8183_CCI_DEVFREQ)	+= mt8183-cci-devfreq.o
>  obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
>  obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
>  obj-$(CONFIG_ARM_TEGRA20_DEVFREQ)	+= tegra20-devfreq.o
> diff --git a/drivers/devfreq/mt8183-cci-devfreq.c b/drivers/devfreq/mt8183-cci-devfreq.c
> new file mode 100644
> index 000000000000..cd7929a83bf8
> --- /dev/null
> +++ b/drivers/devfreq/mt8183-cci-devfreq.c
> @@ -0,0 +1,206 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 MediaTek Inc.

s/2019/2020

> +
> + * Author: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/devfreq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/time.h>
> +
> +#include "governor.h"

It is not needed. Please remove it.

> +
> +#define MAX_VOLT_LIMIT		(1150000)
> +
> +struct cci_devfreq {
> +	struct devfreq *devfreq;
> +	struct regulator *proc_reg;

'proc' means the 'processor'?
Instead of 'proc_reg', you better to use 'cpu_reg'.

> +	struct clk *cci_clk;
> +	int old_vproc;
> +	unsigned long old_freq;
> +};
> +
> +static int mtk_cci_set_voltage(struct cci_devfreq *cci_df, int vproc)
> +{
> +	int ret;
> +
> +	ret = regulator_set_voltage(cci_df->proc_reg, vproc,
> +				    MAX_VOLT_LIMIT);
> +	if (!ret)
> +		cci_df->old_vproc = vproc;
> +	return ret;
> +}
> +
> +static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,
> +				  u32 flags)
> +{
> +	int ret;
> +	struct cci_devfreq *cci_df = dev_get_drvdata(dev);
> +	struct dev_pm_opp *opp;
> +	unsigned long opp_rate, opp_voltage, old_voltage;
> +
> +	if (!cci_df)
> +		return -EINVAL;
> +
> +	if (cci_df->old_freq == *freq)
> +		return 0;
> +
> +	opp_rate = *freq;
> +	opp = dev_pm_opp_find_freq_floor(dev, &opp_rate);
> +	opp_voltage = dev_pm_opp_get_voltage(opp);
> +	dev_pm_opp_put(opp);


You can use the helper function for getting the rate 
with devfreq_recommended_opp(). You can refer the following code
in drivers/devfreq/exynos-bus.c

	opp = devfreq_recommended_opp(dev, freq, flags);
	if (IS_ERR(opp)) {
		dev_err(dev, "failed to get recommended opp instance\n");
		return PTR_ERR(opp);
	}
	dev_pm_opp_put(opp);

> +
> +	old_voltage = cci_df->old_vproc;
> +	if (old_voltage == 0)
> +		old_voltage = regulator_get_voltage(cci_df->proc_reg);
> +
> +	// scale up: set voltage first then freq
> +	if (opp_voltage > old_voltage) {
> +		ret = mtk_cci_set_voltage(cci_df, opp_voltage);
> +		if (ret) {
> +			pr_err("cci: failed to scale up voltage\n");
> +			return ret;
> +		}
> +	}
> +
> +	ret = clk_set_rate(cci_df->cci_clk, *freq);
> +	if (ret) {
> +		pr_err("%s: failed cci to set rate: %d\n", __func__,
> +		       ret);
> +		mtk_cci_set_voltage(cci_df, old_voltage);
> +		return ret;
> +	}
> +
> +	// scale down: set freq first then voltage
> +	if (opp_voltage < old_voltage) {
> +		ret = mtk_cci_set_voltage(cci_df, opp_voltage);
> +		if (ret) {
> +			pr_err("cci: failed to scale down voltage\n");
> +			clk_set_rate(cci_df->cci_clk, cci_df->old_freq);
> +			return ret;
> +		}
> +	}


I recommend that dev_pm_opp_set_rate() and dev_pm_opp_set_regulator()
instead of 'clk_set_rate' and 'regulator_set_voltage'.
In the dev_pm_opp_set_rate(), handle the these sequence.
You can refer the merged patch[1].

[1] commit 4294a779bd8dff6c65e7e85ffe7a1ea236e92a68
- PM / devfreq: exynos-bus: Convert to use dev_pm_opp_set_rate()


> +
> +	cci_df->old_freq = *freq;
> +
> +	return 0;
> +}
> +
> +static struct devfreq_dev_profile cci_devfreq_profile = {
> +	.target = mtk_cci_devfreq_target,

Need to add '.exit' for calling dev_pm_opp_of_remove_table().
You can refer the merged devfreq patches like exynos_bus.c, imx-bus.c.

> +};
> +
> +static int mtk_cci_devfreq_probe(struct platform_device *pdev)
> +{
> +	struct device *cci_dev = &pdev->dev;
> +	struct cci_devfreq *cci_df;
> +	struct devfreq_passive_data *passive_data;
> +	int ret;
> +
> +	cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
> +	if (!cci_df)
> +		return -ENOMEM;
> +
> +	cci_df->cci_clk = devm_clk_get(cci_dev, "cci_clock");
> +	ret = PTR_ERR_OR_ZERO(cci_df->cci_clk);
> +	if (ret) {
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(cci_dev, "failed to get clock for CCI: %d\n",
> +				ret);
> +		return ret;
> +	}
> +	cci_df->proc_reg = devm_regulator_get_optional(cci_dev, "proc");
> +	ret = PTR_ERR_OR_ZERO(cci_df->proc_reg);
> +	if (ret) {
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(cci_dev, "failed to get regulator for CCI: %d\n",
> +				ret);
> +		return ret;
> +	}

I recommend that use dev_pm_opp_set_regulators.
You can refer the merged patch[1].

[1] commit 4294a779bd8dff6c65e7e85ffe7a1ea236e92a68
- PM / devfreq: exynos-bus: Convert to use dev_pm_opp_set_rate()


> +	ret = regulator_enable(cci_df->proc_reg);
> +	if (ret) {
> +		pr_warn("enable buck for cci fail\n");

Use dev_err instead of 'pr_warn'.

> +		return ret;
> +	}
> +
> +	ret = dev_pm_opp_of_add_table(cci_dev);
> +	if (ret) {
> +		dev_err(cci_dev, "Fail to init CCI OPP table: %d\n", ret);

How about changing the error log as following
because in this driver, use the 'failed to' sentence for error handling?

	failed to get OPP table for CCI:L %d

> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, cci_df);
> +
> +	passive_data = devm_kzalloc(cci_dev, sizeof(*passive_data), GFP_KERNEL);
> +	if (!passive_data)
> +		return -ENOMEM;

On this error case, you have to call dev_pm_opp_of_remove_table().
You better to make the 'err_opp' jump lable and then add 'goto err_opp'.

> +
> +	passive_data->parent_type = CPUFREQ_PARENT_DEV;
> +
> +	cci_df->devfreq = devm_devfreq_add_device(cci_dev,
> +						  &cci_devfreq_profile,
> +						  DEVFREQ_GOV_PASSIVE,
> +						  passive_data);
> +	if (IS_ERR(cci_df->devfreq)) {
> +		ret = PTR_ERR(cci_df->devfreq);
> +		dev_err(cci_dev, "cannot create cci devfreq device:%d\n", ret);
> +		dev_pm_opp_of_remove_table(cci_dev);

Instead of direct call, use 'goto err_opp'.

> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mtk_cci_devfreq_remove(struct platform_device *pdev)
> +{
> +	struct device *cci_dev = &pdev->dev;
> +	struct cci_devfreq *cci_df;
> +	struct notifier_block *opp_nb;
> +
> +	cci_df = platform_get_drvdata(pdev);
> +	opp_nb = &cci_df->opp_nb;
> +
> +	dev_pm_opp_unregister_notifier(cci_dev, opp_nb);

This patch doesn't call the dev_pm_opp_register_notifier.
Please remove it.

> +	devm_devfreq_remove_device(cci_dev, cci_df->devfreq);

Don't need to call this function because you used devm_devfreq_add_device().

> +	dev_pm_opp_of_remove_table(cci_dev)> +	regulator_disable(cci_df->proc_reg);
> +
> +	return 0;
> +}
> +
> +static const __maybe_unused struct of_device_id
> +	mediatek_cci_devfreq_of_match[] = {

Make it on one line and remove '__maybe_unused' keyword.
- mediatek_cci_devfreq_of_match-> mediatek_cci_of_match

> +	{ .compatible = "mediatek,mt8183-cci" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, mediatek_cci_devfreq_of_match);

ditto.

> +
> +static struct platform_driver cci_devfreq_driver = {
> +	.probe	= mtk_cci_devfreq_probe,
> +	.remove	= mtk_cci_devfreq_remove,
> +	.driver = {
> +		.name = "mediatek-cci-devfreq",
> +		.of_match_table = of_match_ptr(mediatek_cci_devfreq_of_match),

ditto.

> +	},
> +};
> +
> +static int __init mtk_cci_devfreq_init(void)
> +{
> +	return platform_driver_register(&cci_devfreq_driver);
> +}
> +module_init(mtk_cci_devfreq_init)
> +
> +static void __exit mtk_cci_devfreq_exit(void)
> +{
> +	platform_driver_unregister(&cci_devfreq_driver);
> +}
> +module_exit(mtk_cci_devfreq_exit)

Use 'module_platform_driver' instead of module_init and module_exit.

> +
> +MODULE_DESCRIPTION("Mediatek CCI devfreq driver");
> +MODULE_AUTHOR("Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>");
> +MODULE_LICENSE("GPL v2");
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

^ permalink raw reply

* Re: [PATCH v2 00/91] drm/vc4: Support BCM2711 Display Pipelin
From: Maxime Ripard @ 2020-05-28  7:30 UTC (permalink / raw)
  To: Daniel Drake
  Cc: linux-arm-kernel, devicetree, Linux Kernel, dri-devel,
	Eric Anholt, bcm-kernel-feedback-list, linux-rpi-kernel,
	Jian-Hong Pan, Linux Upstreaming Team, linux-clk,
	Nicolas Saenz Julienne, linux-i2c
In-Reply-To: <CAD8Lp45ucK-yZ5G_DrUVA7rnxo58UF1LPUy65w2PCOcSxKx_Sg@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 767 bytes --]

Hi Daniel,

On Wed, May 27, 2020 at 05:15:12PM +0800, Daniel Drake wrote:
> On Wed, May 27, 2020 at 5:13 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > I'm about to send a v3 today or tomorrow, I can Cc you (and Jian-Hong) if you
> > want.
> 
> That would be great, although given the potentially inconsistent
> results we've been seeing so far it would be great if you could
> additionally push a git branch somewhere.
> That way we can have higher confidence that we are applying exactly
> the same patches to the same base etc.

So I sent a new iteration yesterday, and of course forgot to cc you... Sorry for
that.

I've pushed my current branch here:
https://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git/log/?h=rpi4-kms

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

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

^ permalink raw reply

* Re: [PATCH 08/12] dt-bindings: devfreq: add compatible for mt8183 cci devfreq
From: Chanwoo Choi @ 2020-05-28  7:42 UTC (permalink / raw)
  To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
	Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
  Cc: devicetree, srv_heupstream, linux-pm, linux-kernel,
	linux-mediatek, linux-arm-kernel
In-Reply-To: <20200520034307.20435-9-andrew-sh.cheng@mediatek.com>

Hi,

On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
> This adds dt-binding documentation of cci devfreq
> for Mediatek MT8183 SoC platform.
> 
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> ---
>  .../devicetree/bindings/devfreq/mt8183-cci.yaml    | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml
> 
> diff --git a/Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml b/Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml
> new file mode 100644
> index 000000000000..a7341fd94097
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: https://protect2.fireeye.com/url?k=33f1f15d-6e23ea05-33f07a12-0cc47a31c8b4-91b3f8aeecce95dc&q=1&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fdevfreq%2Fmt8183-cci.yaml%23
> +$schema: https://protect2.fireeye.com/url?k=fc7d9089-a1af8bd1-fc7c1bc6-0cc47a31c8b4-b46f5afc59faf86d&q=1&u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> +
> +title: CCI_DEVFREQ driver for MT8183.
> +
> +maintainers:
> +  - Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> +
> +description: |
> +  This module is used to create CCI DEVFREQ.
> +  The performance will depend on both CCI frequency and CPU frequency.
> +  For MT8183, CCI co-buck with Little core.
> +  Contain CCI opp table for voltage and frequency scaling.
> +
> +properties:
> +  compatible:
> +    const: "mediatek,mt8183-cci"
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: "cci"
> +
> +  operating-points-v2: true
> +  opp-table: true
> +
> +  proc-supply:
> +    description:
> +      Phandle of the regulator that provides the supply voltage.
> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - proc-supply
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/mt8183-clk.h>
> +    cci: cci {
> +      compatible = "mediatek,mt8183-cci";
> +      clocks = <&apmixedsys CLK_APMIXED_CCIPLL>;
> +      clock-names = "cci";
> +      operating-points-v2 = <&cci_opp>;
> +      proc-supply = <&mt6358_vproc12_reg>;
> +    };
> +
> 

I recommend that add the more detailed example
with OPP table with CPU node.


-- 
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 0/2] Introduce PCI_FIXUP_IOMMU
From: Joerg Roedel @ 2020-05-28  7:33 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, Arnd Bergmann,
	linux-pci, Greg Kroah-Hartman, Hanjun Guo, Rafael J. Wysocki,
	linux-kernel, iommu, kenneth-lee-2012, linux-acpi, Wangzhou,
	linux-crypto, Sudeep Holla, Bjorn Helgaas, Zhangfei Gao,
	linux-arm-kernel, Len Brown
In-Reply-To: <20200527181842.GA256680@bjorn-Precision-5520>

On Wed, May 27, 2020 at 01:18:42PM -0500, Bjorn Helgaas wrote:
> Is this slowdown significant?  We already iterate over every device
> when applying PCI_FIXUP_FINAL quirks, so if we used the existing
> PCI_FIXUP_FINAL, we wouldn't be adding a new loop.  We would only be
> adding two more iterations to the loop in pci_do_fixups() that tries
> to match quirks against the current device.  I doubt that would be a
> measurable slowdown.

I don't know how significant it is, but I remember people complaining
about adding new PCI quirks because it takes too long for them to run
them all. That was in the discussion about the quirk disabling ATS on
AMD Stoney systems.

So it probably depends on how many PCI devices are in the system whether
it causes any measureable slowdown.

Regards,

	Joerg


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

^ permalink raw reply

* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
From: Herbert Xu @ 2020-05-28  7:33 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: ebiggers, smueller, linux-crypto, linux-arm-kernel, ardb
In-Reply-To: <20200519190211.76855-1-ardb@kernel.org>

Ard Biesheuvel <ardb@kernel.org> wrote:
> Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
> from the generic implementation in what it returns as the output IV. So
> fix this, and add some test vectors to catch other non-compliant
> implementations.
> 
> Stephan, could you provide a reference for the NIST validation tool and
> how it flags this behaviour as non-compliant? Thanks.

I think our CTS and XTS are both broken with respect to af_alg.

The reason we use output IVs in general is to support chaining
which is required by algif_skcipher to break up large requests
into smaller ones.

For CTS and XTS that simply doesn't work.  So we should fix this
by changing algif_skcipher to not do chaining (and hence drop
support for large requests like algif_aead) for algorithms like
CTS/XTS.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

^ permalink raw reply

* [PATCH 0/9] Convert i.MX legacy platforms clock binding to json-schema
From: Anson Huang @ 2020-05-28  7:27 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, shawnguo, s.hauer, kernel, festevam,
	shc_work, s.trumtrar, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel
  Cc: Linux-imx

The patch series converts i.MX legacy platforms clock binding to
json-schema, including i.MX1, i.MX21, i.MX23, i.MX25, i.MX27, i.MX28,
i.MX31, i.MX35 and i.MX5.

On i.MX21, the CCM has no interrupt at all, so remove the interrupts
property from original binding doc.

Anson Huang (9):
  dt-bindings: clock: Convert i.MX5 clock to json-schema
  dt-bindings: clock: Convert i.MX35 clock to json-schema
  dt-bindings: clock: Convert i.MX31 clock to json-schema
  dt-bindings: clock: Convert i.MX28 clock to json-schema
  dt-bindings: clock: Convert i.MX23 clock to json-schema
  dt-bindings: clock: Convert i.MX27 clock to json-schema
  dt-bindings: clock: Convert i.MX25 clock to json-schema
  dt-bindings: clock: Convert i.MX21 clock to json-schema
  dt-bindings: clock: Convert i.MX1 clock to json-schema

 .../devicetree/bindings/clock/imx1-clock.txt       |  26 ---
 .../devicetree/bindings/clock/imx1-clock.yaml      |  49 ++++++
 .../devicetree/bindings/clock/imx21-clock.txt      |  27 ---
 .../devicetree/bindings/clock/imx21-clock.yaml     |  49 ++++++
 .../devicetree/bindings/clock/imx23-clock.txt      |  70 --------
 .../devicetree/bindings/clock/imx23-clock.yaml     |  90 ++++++++++
 .../devicetree/bindings/clock/imx25-clock.txt      | 160 ------------------
 .../devicetree/bindings/clock/imx25-clock.yaml     | 184 +++++++++++++++++++++
 .../devicetree/bindings/clock/imx27-clock.txt      |  27 ---
 .../devicetree/bindings/clock/imx27-clock.yaml     |  53 ++++++
 .../devicetree/bindings/clock/imx28-clock.txt      |  93 -----------
 .../devicetree/bindings/clock/imx28-clock.yaml     | 113 +++++++++++++
 .../devicetree/bindings/clock/imx31-clock.txt      |  90 ----------
 .../devicetree/bindings/clock/imx31-clock.yaml     | 118 +++++++++++++
 .../devicetree/bindings/clock/imx35-clock.txt      | 114 -------------
 .../devicetree/bindings/clock/imx35-clock.yaml     | 137 +++++++++++++++
 .../devicetree/bindings/clock/imx5-clock.txt       |  28 ----
 .../devicetree/bindings/clock/imx5-clock.yaml      |  63 +++++++
 18 files changed, 856 insertions(+), 635 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/imx1-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx1-clock.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/imx21-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx21-clock.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/imx23-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx23-clock.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/imx25-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx25-clock.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/imx27-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx27-clock.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/imx28-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx28-clock.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/imx31-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx31-clock.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/imx35-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx35-clock.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/imx5-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx5-clock.yaml

-- 
2.7.4


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

^ permalink raw reply

* [PATCH 1/9] dt-bindings: clock: Convert i.MX5 clock to json-schema
From: Anson Huang @ 2020-05-28  7:27 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, shawnguo, s.hauer, kernel, festevam,
	shc_work, s.trumtrar, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel
  Cc: Linux-imx
In-Reply-To: <1590650879-18288-1-git-send-email-Anson.Huang@nxp.com>

Convert the i.MX5 clock binding to DT schema format using json-schema.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 .../devicetree/bindings/clock/imx5-clock.txt       | 28 ----------
 .../devicetree/bindings/clock/imx5-clock.yaml      | 63 ++++++++++++++++++++++
 2 files changed, 63 insertions(+), 28 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/imx5-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx5-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/imx5-clock.txt b/Documentation/devicetree/bindings/clock/imx5-clock.txt
deleted file mode 100644
index a24ca9e..0000000
--- a/Documentation/devicetree/bindings/clock/imx5-clock.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-* Clock bindings for Freescale i.MX5
-
-Required properties:
-- compatible: Should be "fsl,<soc>-ccm" , where <soc> can be imx51 or imx53
-- reg: Address and length of the register set
-- interrupts: Should contain CCM interrupt
-- #clock-cells: Should be <1>
-
-The clock consumer should specify the desired clock by having the clock
-ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx5-clock.h
-for the full list of i.MX5 clock IDs.
-
-Examples (for mx53):
-
-clks: ccm@53fd4000{
-	compatible = "fsl,imx53-ccm";
-	reg = <0x53fd4000 0x4000>;
-	interrupts = <0 71 0x04 0 72 0x04>;
-	#clock-cells = <1>;
-};
-
-can1: can@53fc8000 {
-	compatible = "fsl,imx53-flexcan", "fsl,p1010-flexcan";
-	reg = <0x53fc8000 0x4000>;
-	interrupts = <82>;
-	clocks = <&clks IMX5_CLK_CAN1_IPG_GATE>, <&clks IMX5_CLK_CAN1_SERIAL_GATE>;
-	clock-names = "ipg", "per";
-};
diff --git a/Documentation/devicetree/bindings/clock/imx5-clock.yaml b/Documentation/devicetree/bindings/clock/imx5-clock.yaml
new file mode 100644
index 0000000..e4c405c
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx5-clock.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/imx5-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock bindings for Freescale i.MX5
+
+maintainers:
+  - Fabio Estevam <fabio.estevam@freescale.com>
+
+description: |
+  The clock consumer should specify the desired clock by having the clock
+  ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx5-clock.h
+  for the full list of i.MX5 clock IDs.
+
+properties:
+  compatible:
+    enum:
+      - fsl,imx53-ccm
+      - fsl,imx51-ccm
+      - fsl,imx50-ccm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description: CCM provides 2 interrupt requests, request 1 is to generate
+      interrupt for frequency or mux change, request 2 is to generate
+      interrupt for oscillator read or PLL lock.
+    items:
+      - description: CCM interrupt request 1
+      - description: CCM interrupt request 2
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - '#clock-cells'
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx5-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    clock-controller@53fd4000{
+        compatible = "fsl,imx53-ccm";
+        reg = <0x53fd4000 0x4000>;
+        interrupts = <0 71 IRQ_TYPE_LEVEL_HIGH>,
+                     <0 72 IRQ_TYPE_LEVEL_HIGH>;
+        #clock-cells = <1>;
+    };
+
+    can@53fc8000 {
+        compatible = "fsl,imx53-flexcan", "fsl,p1010-flexcan";
+        reg = <0x53fc8000 0x4000>;
+        interrupts = <82>;
+        clocks = <&clks IMX5_CLK_CAN1_IPG_GATE>, <&clks IMX5_CLK_CAN1_SERIAL_GATE>;
+        clock-names = "ipg", "per";
+    };
-- 
2.7.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 2/9] dt-bindings: clock: Convert i.MX35 clock to json-schema
From: Anson Huang @ 2020-05-28  7:27 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, shawnguo, s.hauer, kernel, festevam,
	shc_work, s.trumtrar, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel
  Cc: Linux-imx
In-Reply-To: <1590650879-18288-1-git-send-email-Anson.Huang@nxp.com>

Convert the i.MX35 clock binding to DT schema format using json-schema.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 .../devicetree/bindings/clock/imx35-clock.txt      | 114 -----------------
 .../devicetree/bindings/clock/imx35-clock.yaml     | 137 +++++++++++++++++++++
 2 files changed, 137 insertions(+), 114 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/imx35-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx35-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/imx35-clock.txt b/Documentation/devicetree/bindings/clock/imx35-clock.txt
deleted file mode 100644
index f497832..0000000
--- a/Documentation/devicetree/bindings/clock/imx35-clock.txt
+++ /dev/null
@@ -1,114 +0,0 @@
-* Clock bindings for Freescale i.MX35
-
-Required properties:
-- compatible: Should be "fsl,imx35-ccm"
-- reg: Address and length of the register set
-- interrupts: Should contain CCM interrupt
-- #clock-cells: Should be <1>
-
-The clock consumer should specify the desired clock by having the clock
-ID in its "clocks" phandle cell.  The following is a full list of i.MX35
-clocks and IDs.
-
-	Clock			ID
-	---------------------------
-	ckih			0
-	mpll			1
-	ppll			2
-	mpll_075		3
-	arm			4
-	hsp			5
-	hsp_div			6
-	hsp_sel			7
-	ahb			8
-	ipg			9
-	arm_per_div		10
-	ahb_per_div		11
-	ipg_per			12
-	uart_sel		13
-	uart_div		14
-	esdhc_sel		15
-	esdhc1_div		16
-	esdhc2_div		17
-	esdhc3_div		18
-	spdif_sel		19
-	spdif_div_pre		20
-	spdif_div_post		21
-	ssi_sel			22
-	ssi1_div_pre		23
-	ssi1_div_post		24
-	ssi2_div_pre		25
-	ssi2_div_post		26
-	usb_sel			27
-	usb_div			28
-	nfc_div			29
-	asrc_gate		30
-	pata_gate		31
-	audmux_gate		32
-	can1_gate		33
-	can2_gate		34
-	cspi1_gate		35
-	cspi2_gate		36
-	ect_gate		37
-	edio_gate		38
-	emi_gate		39
-	epit1_gate		40
-	epit2_gate		41
-	esai_gate		42
-	esdhc1_gate		43
-	esdhc2_gate		44
-	esdhc3_gate		45
-	fec_gate		46
-	gpio1_gate		47
-	gpio2_gate		48
-	gpio3_gate		49
-	gpt_gate		50
-	i2c1_gate		51
-	i2c2_gate		52
-	i2c3_gate		53
-	iomuxc_gate		54
-	ipu_gate		55
-	kpp_gate		56
-	mlb_gate		57
-	mshc_gate		58
-	owire_gate		59
-	pwm_gate		60
-	rngc_gate		61
-	rtc_gate		62
-	rtic_gate		63
-	scc_gate		64
-	sdma_gate		65
-	spba_gate		66
-	spdif_gate		67
-	ssi1_gate		68
-	ssi2_gate		69
-	uart1_gate		70
-	uart2_gate		71
-	uart3_gate		72
-	usbotg_gate		73
-	wdog_gate		74
-	max_gate		75
-	admux_gate		76
-	csi_gate		77
-	csi_div			78
-	csi_sel			79
-	iim_gate		80
-	gpu2d_gate		81
-	ckli_gate		82
-
-Examples:
-
-clks: ccm@53f80000 {
-	compatible = "fsl,imx35-ccm";
-	reg = <0x53f80000 0x4000>;
-	interrupts = <31>;
-	#clock-cells = <1>;
-};
-
-esdhc1: esdhc@53fb4000 {
-	compatible = "fsl,imx35-esdhc";
-	reg = <0x53fb4000 0x4000>;
-	interrupts = <7>;
-	clocks = <&clks 9>, <&clks 8>, <&clks 43>;
-	clock-names = "ipg", "ahb", "per";
-};
diff --git a/Documentation/devicetree/bindings/clock/imx35-clock.yaml b/Documentation/devicetree/bindings/clock/imx35-clock.yaml
new file mode 100644
index 0000000..fecea84
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx35-clock.yaml
@@ -0,0 +1,137 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/imx35-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock bindings for Freescale i.MX35
+
+maintainers:
+  - Steffen Trumtrar <s.trumtrar@pengutronix.de>
+
+description: |
+  The clock consumer should specify the desired clock by having the clock
+  ID in its "clocks" phandle cell. The following is a full list of i.MX35
+  clocks and IDs.
+
+        Clock			ID
+        ---------------------------
+        ckih			0
+        mpll			1
+        ppll			2
+        mpll_075		3
+        arm			4
+        hsp			5
+        hsp_div			6
+        hsp_sel			7
+        ahb			8
+        ipg			9
+        arm_per_div		10
+        ahb_per_div		11
+        ipg_per			12
+        uart_sel		13
+        uart_div		14
+        esdhc_sel		15
+        esdhc1_div		16
+        esdhc2_div		17
+        esdhc3_div		18
+        spdif_sel		19
+        spdif_div_pre		20
+        spdif_div_post		21
+        ssi_sel			22
+        ssi1_div_pre		23
+        ssi1_div_post		24
+        ssi2_div_pre		25
+        ssi2_div_post		26
+        usb_sel			27
+        usb_div			28
+        nfc_div			29
+        asrc_gate		30
+        pata_gate		31
+        audmux_gate		32
+        can1_gate		33
+        can2_gate		34
+        cspi1_gate		35
+        cspi2_gate		36
+        ect_gate		37
+        edio_gate		38
+        emi_gate		39
+        epit1_gate		40
+        epit2_gate		41
+        esai_gate		42
+        esdhc1_gate		43
+        esdhc2_gate		44
+        esdhc3_gate		45
+        fec_gate		46
+        gpio1_gate		47
+        gpio2_gate		48
+        gpio3_gate		49
+        gpt_gate		50
+        i2c1_gate		51
+        i2c2_gate		52
+        i2c3_gate		53
+        iomuxc_gate		54
+        ipu_gate		55
+        kpp_gate		56
+        mlb_gate		57
+        mshc_gate		58
+        owire_gate		59
+        pwm_gate		60
+        rngc_gate		61
+        rtc_gate		62
+        rtic_gate		63
+        scc_gate		64
+        sdma_gate		65
+        spba_gate		66
+        spdif_gate		67
+        ssi1_gate		68
+        ssi2_gate		69
+        uart1_gate		70
+        uart2_gate		71
+        uart3_gate		72
+        usbotg_gate		73
+        wdog_gate		74
+        max_gate		75
+        admux_gate		76
+        csi_gate		77
+        csi_div			78
+        csi_sel			79
+        iim_gate		80
+        gpu2d_gate		81
+        ckli_gate		82
+
+properties:
+  compatible:
+    const: fsl,imx35-ccm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - '#clock-cells'
+
+examples:
+  - |
+    clock-controller@53f80000 {
+        compatible = "fsl,imx35-ccm";
+        reg = <0x53f80000 0x4000>;
+        interrupts = <31>;
+        #clock-cells = <1>;
+    };
+
+    esdhc@53fb4000 {
+        compatible = "fsl,imx35-esdhc";
+        reg = <0x53fb4000 0x4000>;
+        interrupts = <7>;
+        clocks = <&clks 9>, <&clks 8>, <&clks 43>;
+        clock-names = "ipg", "ahb", "per";
+    };
-- 
2.7.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 3/9] dt-bindings: clock: Convert i.MX31 clock to json-schema
From: Anson Huang @ 2020-05-28  7:27 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, shawnguo, s.hauer, kernel, festevam,
	shc_work, s.trumtrar, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel
  Cc: Linux-imx
In-Reply-To: <1590650879-18288-1-git-send-email-Anson.Huang@nxp.com>

Convert the i.MX31 clock binding to DT schema format using json-schema.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 .../devicetree/bindings/clock/imx31-clock.txt      |  90 ----------------
 .../devicetree/bindings/clock/imx31-clock.yaml     | 118 +++++++++++++++++++++
 2 files changed, 118 insertions(+), 90 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/imx31-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx31-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/imx31-clock.txt b/Documentation/devicetree/bindings/clock/imx31-clock.txt
deleted file mode 100644
index 0a29109..0000000
--- a/Documentation/devicetree/bindings/clock/imx31-clock.txt
+++ /dev/null
@@ -1,90 +0,0 @@
-* Clock bindings for Freescale i.MX31
-
-Required properties:
-- compatible: Should be "fsl,imx31-ccm"
-- reg: Address and length of the register set
-- interrupts: Should contain CCM interrupt
-- #clock-cells: Should be <1>
-
-The clock consumer should specify the desired clock by having the clock
-ID in its "clocks" phandle cell.  The following is a full list of i.MX31
-clocks and IDs.
-
-	Clock		    ID
-	-----------------------
-	dummy	             0
-	ckih                 1
-	ckil                 2
-	mpll                 3
-	spll                 4
-	upll                 5
-	mcu_main             6
-	hsp                  7
-	ahb                  8
-	nfc                  9
-	ipg                  10
-	per_div              11
-	per                  12
-	csi_sel              13
-	fir_sel              14
-	csi_div              15
-	usb_div_pre          16
-	usb_div_post         17
-	fir_div_pre          18
-	fir_div_post         19
-	sdhc1_gate           20
-	sdhc2_gate           21
-	gpt_gate             22
-	epit1_gate           23
-	epit2_gate           24
-	iim_gate             25
-	ata_gate             26
-	sdma_gate            27
-	cspi3_gate           28
-	rng_gate             29
-	uart1_gate           30
-	uart2_gate           31
-	ssi1_gate            32
-	i2c1_gate            33
-	i2c2_gate            34
-	i2c3_gate            35
-	hantro_gate          36
-	mstick1_gate         37
-	mstick2_gate         38
-	csi_gate             39
-	rtc_gate             40
-	wdog_gate            41
-	pwm_gate             42
-	sim_gate             43
-	ect_gate             44
-	usb_gate             45
-	kpp_gate             46
-	ipu_gate             47
-	uart3_gate           48
-	uart4_gate           49
-	uart5_gate           50
-	owire_gate           51
-	ssi2_gate            52
-	cspi1_gate           53
-	cspi2_gate           54
-	gacc_gate            55
-	emi_gate             56
-	rtic_gate            57
-	firi_gate            58
-
-Examples:
-
-clks: ccm@53f80000{
-	compatible = "fsl,imx31-ccm";
-	reg = <0x53f80000 0x4000>;
-	interrupts = <31>, <53>;
-	#clock-cells = <1>;
-};
-
-uart1: serial@43f90000 {
-	compatible = "fsl,imx31-uart", "fsl,imx21-uart";
-	reg = <0x43f90000 0x4000>;
-	interrupts = <45>;
-	clocks = <&clks 10>, <&clks 30>;
-	clock-names = "ipg", "per";
-};
diff --git a/Documentation/devicetree/bindings/clock/imx31-clock.yaml b/Documentation/devicetree/bindings/clock/imx31-clock.yaml
new file mode 100644
index 0000000..2d9220e
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx31-clock.yaml
@@ -0,0 +1,118 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/imx31-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock bindings for Freescale i.MX31
+
+maintainers:
+  - Fabio Estevam <fabio.estevam@freescale.com>
+
+description: |
+  The clock consumer should specify the desired clock by having the clock
+  ID in its "clocks" phandle cell. The following is a full list of i.MX31
+  clocks and IDs.
+
+        Clock		    ID
+        -----------------------
+        dummy	             0
+        ckih                 1
+        ckil                 2
+        mpll                 3
+        spll                 4
+        upll                 5
+        mcu_main             6
+        hsp                  7
+        ahb                  8
+        nfc                  9
+        ipg                  10
+        per_div              11
+        per                  12
+        csi_sel              13
+        fir_sel              14
+        csi_div              15
+        usb_div_pre          16
+        usb_div_post         17
+        fir_div_pre          18
+        fir_div_post         19
+        sdhc1_gate           20
+        sdhc2_gate           21
+        gpt_gate             22
+        epit1_gate           23
+        epit2_gate           24
+        iim_gate             25
+        ata_gate             26
+        sdma_gate            27
+        cspi3_gate           28
+        rng_gate             29
+        uart1_gate           30
+        uart2_gate           31
+        ssi1_gate            32
+        i2c1_gate            33
+        i2c2_gate            34
+        i2c3_gate            35
+        hantro_gate          36
+        mstick1_gate         37
+        mstick2_gate         38
+        csi_gate             39
+        rtc_gate             40
+        wdog_gate            41
+        pwm_gate             42
+        sim_gate             43
+        ect_gate             44
+        usb_gate             45
+        kpp_gate             46
+        ipu_gate             47
+        uart3_gate           48
+        uart4_gate           49
+        uart5_gate           50
+        owire_gate           51
+        ssi2_gate            52
+        cspi1_gate           53
+        cspi2_gate           54
+        gacc_gate            55
+        emi_gate             56
+        rtic_gate            57
+        firi_gate            58
+
+properties:
+  compatible:
+    const: fsl,imx31-ccm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description: CCM provides 2 interrupt requests, request 1 is to generate
+      interrupt for DVFS when a frequency change is requested, request 2 is
+      to generate interrupt for DPTC when a voltage change is requested.
+    items:
+      - description: CCM DVFS interrupt request 1
+      - description: CCM DPTC interrupt request 2
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - '#clock-cells'
+
+examples:
+  - |
+    clock-controller@53f80000 {
+        compatible = "fsl,imx31-ccm";
+        reg = <0x53f80000 0x4000>;
+        interrupts = <31>, <53>;
+        #clock-cells = <1>;
+    };
+
+    serial@43f90000 {
+        compatible = "fsl,imx31-uart", "fsl,imx21-uart";
+        reg = <0x43f90000 0x4000>;
+        interrupts = <45>;
+        clocks = <&clks 10>, <&clks 30>;
+        clock-names = "ipg", "per";
+    };
-- 
2.7.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 4/9] dt-bindings: clock: Convert i.MX28 clock to json-schema
From: Anson Huang @ 2020-05-28  7:27 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, shawnguo, s.hauer, kernel, festevam,
	shc_work, s.trumtrar, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel
  Cc: Linux-imx
In-Reply-To: <1590650879-18288-1-git-send-email-Anson.Huang@nxp.com>

Convert the i.MX28 clock binding to DT schema format using json-schema.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 .../devicetree/bindings/clock/imx28-clock.txt      |  93 -----------------
 .../devicetree/bindings/clock/imx28-clock.yaml     | 113 +++++++++++++++++++++
 2 files changed, 113 insertions(+), 93 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/imx28-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx28-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/imx28-clock.txt b/Documentation/devicetree/bindings/clock/imx28-clock.txt
deleted file mode 100644
index d84a37d..0000000
--- a/Documentation/devicetree/bindings/clock/imx28-clock.txt
+++ /dev/null
@@ -1,93 +0,0 @@
-* Clock bindings for Freescale i.MX28
-
-Required properties:
-- compatible: Should be "fsl,imx28-clkctrl"
-- reg: Address and length of the register set
-- #clock-cells: Should be <1>
-
-The clock consumer should specify the desired clock by having the clock
-ID in its "clocks" phandle cell.  The following is a full list of i.MX28
-clocks and IDs.
-
-	Clock		ID
-	------------------
-	ref_xtal	0
-	pll0		1
-	pll1		2
-	pll2		3
-	ref_cpu		4
-	ref_emi		5
-	ref_io0		6
-	ref_io1		7
-	ref_pix		8
-	ref_hsadc	9
-	ref_gpmi	10
-	saif0_sel	11
-	saif1_sel	12
-	gpmi_sel	13
-	ssp0_sel	14
-	ssp1_sel	15
-	ssp2_sel	16
-	ssp3_sel	17
-	emi_sel		18
-	etm_sel		19
-	lcdif_sel	20
-	cpu		21
-	ptp_sel		22
-	cpu_pll		23
-	cpu_xtal	24
-	hbus		25
-	xbus		26
-	ssp0_div	27
-	ssp1_div	28
-	ssp2_div	29
-	ssp3_div	30
-	gpmi_div	31
-	emi_pll		32
-	emi_xtal	33
-	lcdif_div	34
-	etm_div		35
-	ptp		36
-	saif0_div	37
-	saif1_div	38
-	clk32k_div	39
-	rtc		40
-	lradc		41
-	spdif_div	42
-	clk32k		43
-	pwm		44
-	uart		45
-	ssp0		46
-	ssp1		47
-	ssp2		48
-	ssp3		49
-	gpmi		50
-	spdif		51
-	emi		52
-	saif0		53
-	saif1		54
-	lcdif		55
-	etm		56
-	fec		57
-	can0		58
-	can1		59
-	usb0		60
-	usb1		61
-	usb0_phy	62
-	usb1_phy	63
-	enet_out	64
-
-Examples:
-
-clks: clkctrl@80040000 {
-	compatible = "fsl,imx28-clkctrl";
-	reg = <0x80040000 0x2000>;
-	#clock-cells = <1>;
-};
-
-auart0: serial@8006a000 {
-	compatible = "fsl,imx28-auart", "fsl,imx23-auart";
-	reg = <0x8006a000 0x2000>;
-	interrupts = <112 70 71>;
-	clocks = <&clks 45>;
-};
diff --git a/Documentation/devicetree/bindings/clock/imx28-clock.yaml b/Documentation/devicetree/bindings/clock/imx28-clock.yaml
new file mode 100644
index 0000000..e4a7038
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx28-clock.yaml
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/imx28-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock bindings for Freescale i.MX28
+
+maintainers:
+  - Shawn Guo <shawn.guo@linaro.org>
+
+description: |
+  The clock consumer should specify the desired clock by having the clock
+  ID in its "clocks" phandle cell. The following is a full list of i.MX28
+  clocks and IDs.
+
+        Clock		ID
+        ------------------
+        ref_xtal	0
+        pll0		1
+        pll1		2
+        pll2		3
+        ref_cpu		4
+        ref_emi		5
+        ref_io0		6
+        ref_io1		7
+        ref_pix		8
+        ref_hsadc	9
+        ref_gpmi	10
+        saif0_sel	11
+        saif1_sel	12
+        gpmi_sel	13
+        ssp0_sel	14
+        ssp1_sel	15
+        ssp2_sel	16
+        ssp3_sel	17
+        emi_sel		18
+        etm_sel		19
+        lcdif_sel	20
+        cpu		21
+        ptp_sel		22
+        cpu_pll		23
+        cpu_xtal	24
+        hbus		25
+        xbus		26
+        ssp0_div	27
+        ssp1_div	28
+        ssp2_div	29
+        ssp3_div	30
+        gpmi_div	31
+        emi_pll		32
+        emi_xtal	33
+        lcdif_div	34
+        etm_div		35
+        ptp		36
+        saif0_div	37
+        saif1_div	38
+        clk32k_div	39
+        rtc		40
+        lradc		41
+        spdif_div	42
+        clk32k		43
+        pwm		44
+        uart		45
+        ssp0		46
+        ssp1		47
+        ssp2		48
+        ssp3		49
+        gpmi		50
+        spdif		51
+        emi		52
+        saif0		53
+        saif1		54
+        lcdif		55
+        etm		56
+        fec		57
+        can0		58
+        can1		59
+        usb0		60
+        usb1		61
+        usb0_phy	62
+        usb1_phy	63
+        enet_out	64
+
+properties:
+  compatible:
+    const: fsl,imx28-clkctrl
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+
+examples:
+  - |
+    clock-controller@80040000 {
+        compatible = "fsl,imx28-clkctrl";
+        reg = <0x80040000 0x2000>;
+        #clock-cells = <1>;
+    };
+
+    serial@8006a000 {
+        compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+        reg = <0x8006a000 0x2000>;
+        interrupts = <112 70 71>;
+        clocks = <&clks 45>;
+    };
-- 
2.7.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 6/9] dt-bindings: clock: Convert i.MX27 clock to json-schema
From: Anson Huang @ 2020-05-28  7:27 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, shawnguo, s.hauer, kernel, festevam,
	shc_work, s.trumtrar, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel
  Cc: Linux-imx
In-Reply-To: <1590650879-18288-1-git-send-email-Anson.Huang@nxp.com>

Convert the i.MX27 clock binding to DT schema format using json-schema.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 .../devicetree/bindings/clock/imx27-clock.txt      | 27 -----------
 .../devicetree/bindings/clock/imx27-clock.yaml     | 53 ++++++++++++++++++++++
 2 files changed, 53 insertions(+), 27 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/imx27-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/imx27-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/imx27-clock.txt b/Documentation/devicetree/bindings/clock/imx27-clock.txt
deleted file mode 100644
index 4c95c04..0000000
--- a/Documentation/devicetree/bindings/clock/imx27-clock.txt
+++ /dev/null
@@ -1,27 +0,0 @@
-* Clock bindings for Freescale i.MX27
-
-Required properties:
-- compatible: Should be "fsl,imx27-ccm"
-- reg: Address and length of the register set
-- interrupts: Should contain CCM interrupt
-- #clock-cells: Should be <1>
-
-The clock consumer should specify the desired clock by having the clock
-ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx27-clock.h
-for the full list of i.MX27 clock IDs.
-
-Examples:
-	clks: ccm@10027000{
-		compatible = "fsl,imx27-ccm";
-		reg = <0x10027000 0x1000>;
-		#clock-cells = <1>;
-	};
-
-	uart1: serial@1000a000 {
-		compatible = "fsl,imx27-uart", "fsl,imx21-uart";
-		reg = <0x1000a000 0x1000>;
-		interrupts = <20>;
-		clocks = <&clks IMX27_CLK_UART1_IPG_GATE>,
-			 <&clks IMX27_CLK_PER1_GATE>;
-		clock-names = "ipg", "per";
-	};
diff --git a/Documentation/devicetree/bindings/clock/imx27-clock.yaml b/Documentation/devicetree/bindings/clock/imx27-clock.yaml
new file mode 100644
index 0000000..2d69807
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx27-clock.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/imx27-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock bindings for Freescale i.MX27
+
+maintainers:
+  - Fabio Estevam <fabio.estevam@freescale.com>
+
+description: |
+  The clock consumer should specify the desired clock by having the clock
+  ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx27-clock.h
+  for the full list of i.MX27 clock IDs.
+
+properties:
+  compatible:
+    const: fsl,imx27-ccm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx27-clock.h>
+
+    clock-controller@10027000 {
+        compatible = "fsl,imx27-ccm";
+        reg = <0x10027000 0x1000>;
+        interrupts = <31>;
+        #clock-cells = <1>;
+    };
+
+    serial@1000a000 {
+        compatible = "fsl,imx27-uart", "fsl,imx21-uart";
+        reg = <0x1000a000 0x1000>;
+        interrupts = <20>;
+        clocks = <&clks IMX27_CLK_UART1_IPG_GATE>,
+                 <&clks IMX27_CLK_PER1_GATE>;
+        clock-names = "ipg", "per";
+    };
-- 
2.7.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


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