Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] dt-bindings: net: ti: icssg_prueth: Add documentation for PA_STATS support
From: MD Danish Anwar @ 2024-04-30 12:24 UTC (permalink / raw)
  To: Conor Dooley, Krzysztof Kozlowski, Rob Herring, Paolo Abeni,
	Jakub Kicinski, Eric Dumazet, David S. Miller
  Cc: linux-kernel, devicetree, netdev, linux-arm-kernel, srk,
	Vignesh Raghavendra, r-gunasekaran, Roger Quadros,
	MD Danish Anwar

Add documentation for ti,pa-stats property which is syscon regmap for
PA_STATS register. This will be used to dump statistics maintained by
ICSSG firmware.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
index e253fa786092..abf372f7191b 100644
--- a/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
+++ b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
@@ -55,6 +55,11 @@ properties:
     description:
       phandle to MII_RT module's syscon regmap
 
+  ti,pa-stats:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to PA_STATS module's syscon regmap
+
   ti,iep:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 2
@@ -194,6 +199,7 @@ examples:
                     "tx1-0", "tx1-1", "tx1-2", "tx1-3",
                     "rx0", "rx1";
         ti,mii-g-rt = <&icssg2_mii_g_rt>;
+        ti,pa-stats = <&icssg2_pa_stats>;
         ti,iep = <&icssg2_iep0>, <&icssg2_iep1>;
         interrupt-parent = <&icssg2_intc>;
         interrupts = <24 0 2>, <25 1 3>;
-- 
2.34.1


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

^ permalink raw reply related

* Re: [PATCH v4 6/7] iommu/dma: Centralise iommu_setup_dma_ops()
From: Konrad Dybcio @ 2024-04-30 12:23 UTC (permalink / raw)
  To: Dmitry Baryshkov, Robin Murphy,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Bjorn Andersson
  Cc: Joerg Roedel, Christoph Hellwig, Vineet Gupta, Russell King,
	Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, K. Y. Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Suravee Suthikulpanit,
	David Woodhouse, Lu Baolu, Niklas Schnelle, Matthew Rosato,
	Gerald Schaefer, Jean-Philippe Brucker, Rob Herring, Frank Rowand,
	Marek Szyprowski, Jason Gunthorpe, linux-kernel, linux-arm-kernel,
	linux-acpi, iommu, devicetree, Jason Gunthorpe
In-Reply-To: <CAA8EJprL8NbNfOvp17hrHoVNkKBpD39xfeu+STm6m9VObF2n9Q@mail.gmail.com>

On 29.04.2024 11:26 PM, Dmitry Baryshkov wrote:
> On Mon, 29 Apr 2024 at 19:31, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> On Fri, Apr 19, 2024 at 05:54:45PM +0100, Robin Murphy wrote:
>>> It's somewhat hard to see, but arm64's arch_setup_dma_ops() should only
>>> ever call iommu_setup_dma_ops() after a successful iommu_probe_device(),
>>> which means there should be no harm in achieving the same order of
>>> operations by running it off the back of iommu_probe_device() itself.
>>> This then puts it in line with the x86 and s390 .probe_finalize bodges,
>>> letting us pull it all into the main flow properly. As a bonus this lets
>>> us fold in and de-scope the PCI workaround setup as well.
>>>
>>> At this point we can also then pull the call up inside the group mutex,
>>> and avoid having to think about whether iommu_group_store_type() could
>>> theoretically race and free the domain if iommu_setup_dma_ops() ran just
>>> *before* iommu_device_use_default_domain() claims it... Furthermore we
>>> replace one .probe_finalize call completely, since the only remaining
>>> implementations are now one which only needs to run once for the initial
>>> boot-time probe, and two which themselves render that path unreachable.
>>>
>>> This leaves us a big step closer to realistically being able to unpick
>>> the variety of different things that iommu_setup_dma_ops() has been
>>> muddling together, and further streamline iommu-dma into core API flows
>>> in future.
>>>
>>> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com> # For Intel IOMMU
>>> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>>> Tested-by: Hanjun Guo <guohanjun@huawei.com>
>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>> ---
>>> v2: Shuffle around to make sure the iommu_group_do_probe_finalize() case
>>>     is covered as well, with bonus side-effects as above.
>>> v3: *Really* do that, remembering the other two probe_finalize sites too.
>>> ---
>>>  arch/arm64/mm/dma-mapping.c  |  2 --
>>>  drivers/iommu/amd/iommu.c    |  8 --------
>>>  drivers/iommu/dma-iommu.c    | 18 ++++++------------
>>>  drivers/iommu/dma-iommu.h    | 14 ++++++--------
>>>  drivers/iommu/intel/iommu.c  |  7 -------
>>>  drivers/iommu/iommu.c        | 20 +++++++-------------
>>>  drivers/iommu/s390-iommu.c   |  6 ------
>>>  drivers/iommu/virtio-iommu.c | 10 ----------
>>>  include/linux/iommu.h        |  7 -------
>>>  9 files changed, 19 insertions(+), 73 deletions(-)
>>
>> This patch breaks UFS on Qualcomm SC8180X Primus platform:
>>
>>
>> [    3.846856] arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402, iova=0x1032db3e0, fsynr=0x130000, cbfrsynra=0x300, cb=4
>> [    3.846880] ufshcd-qcom 1d84000.ufshc: ufshcd_check_errors: saved_err 0x20000 saved_uic_err 0x0
>> [    3.846929] host_regs: 00000000: 1587031f 00000000 00000300 00000000
>> [    3.846935] host_regs: 00000010: 01000000 00010217 00000000 00000000
>> [    3.846941] host_regs: 00000020: 00000000 00070ef5 00000000 00000000
>> [    3.846946] host_regs: 00000030: 0000000f 00000001 00000000 00000000
>> [    3.846951] host_regs: 00000040: 00000000 00000000 00000000 00000000
>> [    3.846956] host_regs: 00000050: 032db000 00000001 00000000 00000000
>> [    3.846962] host_regs: 00000060: 00000000 80000000 00000000 00000000
>> [    3.846967] host_regs: 00000070: 032dd000 00000001 00000000 00000000
>> [    3.846972] host_regs: 00000080: 00000000 00000000 00000000 00000000
>> [    3.846977] host_regs: 00000090: 00000016 00000000 00000000 0000000c
>> [    3.847074] ufshcd-qcom 1d84000.ufshc: ufshcd_err_handler started; HBA state eh_fatal; powered 1; shutting down 0; saved_err = 131072; saved_uic_err = 0; force_reset = 0
>> [    4.406550] ufshcd-qcom 1d84000.ufshc: ufshcd_verify_dev_init: NOP OUT failed -11
>> [    4.417953] ufshcd-qcom 1d84000.ufshc: ufshcd_async_scan failed: -11
> 
> Just to confirm: reverting f091e93306e0 ("dma-mapping: Simplify
> arch_setup_dma_ops()") and b67483b3c44e ("iommu/dma: Centralise
> iommu_setup_dma_ops()" fixes the issue for me. Please ping me if you'd
> like me to test a fix.

This also triggers a different issue (that also comes down to "ufs bad") on
another QC platform (SM8550):

[    4.282098] scsi host0: ufshcd
[    4.315970] ufshcd-qcom 1d84000.ufs: ufshcd_check_errors: saved_err 0x20000 saved_uic_err 0x0
[    4.330155] host_regs: 00000000: 3587031f 00000000 00000400 00000000
[    4.343955] host_regs: 00000010: 01000000 00010217 00000000 00000000
[    4.356027] host_regs: 00000020: 00000000 00070ef5 00000000 00000000
[    4.370136] host_regs: 00000030: 0000000f 00000003 00000000 00000000
[    4.376662] host_regs: 00000040: 00000000 00000000 00000000 00000000
[    4.383192] host_regs: 00000050: 85109000 00000008 00000000 00000000
[    4.389719] host_regs: 00000060: 00000000 80000000 00000000 00000000
[    4.396245] host_regs: 00000070: 8510a000 00000008 00000000 00000000
[    4.402773] host_regs: 00000080: 00000000 00000000 00000000 00000000
[    4.409298] host_regs: 00000090: 00000016 00000000 00000000 0000000c
[    4.415900] arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402, iova=0x8851093e0, fsynr=0x3b0001, cbfrsynra=0x60, cb=2
[    4.416135] ufshcd-qcom 1d84000.ufs: ufshcd_err_handler started; HBA state eh_fatal; powered 1; shutting down 0; saved_err = 131072; saved_uic_err = 0; force_reset = 0
[    4.951750] ufshcd-qcom 1d84000.ufs: ufshcd_verify_dev_init: NOP OUT failed -11
[    4.960644] ufshcd-qcom 1d84000.ufs: ufshcd_async_scan failed: -11

Reverting the commits Dmitry mentioned also fixes this.

Konrad


_______________________________________________
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] selftests:arm64: Test PR_SVE_VL_INHERIT after a double fork
From: Will Deacon @ 2024-04-30 12:20 UTC (permalink / raw)
  To: Dorine Tipo
  Cc: Catalin Marinas, Shuah Khan, linux-arm-kernel, linux-kselftest,
	linux-kernel, linux-kernel-mentees, Javier Carrasco
In-Reply-To: <20240429044012.5018-1-dorine.a.tipo@gmail.com>

On Mon, Apr 29, 2024 at 04:40:12AM +0000, Dorine Tipo wrote:
> Add a new test, double_fork_test() to check the inheritance of the SVE
> vector length after a double fork.
> The `EXPECTED_TESTS` macro has been updated to account for this additional
> test.
> This patch addresses task 7 on the TODO list.
> 
> Signed-off-by: Dorine Tipo <dorine.a.tipo@gmail.com>
> ---
>  tools/testing/selftests/arm64/fp/za-fork.c | 95 +++++++++++++++++++++-
>  1 file changed, 94 insertions(+), 1 deletion(-)

I haven't tried compiling this, but some of the code looks a little off:

> diff --git a/tools/testing/selftests/arm64/fp/za-fork.c b/tools/testing/selftests/arm64/fp/za-fork.c
> index 587b94648222..35229e570dcf 100644
> --- a/tools/testing/selftests/arm64/fp/za-fork.c
> +++ b/tools/testing/selftests/arm64/fp/za-fork.c
> @@ -11,7 +11,7 @@
> 
>  #include "kselftest.h"
> 
> -#define EXPECTED_TESTS 1
> +#define EXPECTED_TESTS 2
> 
>  int fork_test(void);
>  int verify_fork(void);
> @@ -69,6 +69,97 @@ int fork_test_c(void)
> 	}
>  }
> 
> +int double_fork_test(void)
> +{
> +	pid_t newpid, grandchild_pid, waiting;
> +	int ret, child_status, parent_result;
> +
> +	ret = prctl(PR_SVE_SET_VL, vl | PR_SVE_VL_INHERIT);
> +	if (ret < 0)
> +		ksft_exit_fail_msg("Failed to set SVE VL %d\n", vl);
> +
> +	newpid = fork();
> +	if (newpid == 0) {
> +		/* In child */
> +		if (!verify_fork()) {
> +			ksft_print_msg("ZA state invalid in child\n");
> +			exit(0);
> +		}
> +
> +		grandchild_pid = fork();
> +		if (grandchild_pid == 0) {
> +			/* in grandchild */
> +			if (!verfy_fork()) {
> +				ksft_print_msg("ZA state invalid in grandchild\n");
> +				exit(0);
> +			}
> +
> +			ret = prctl(PR_SVE_GET_VL);
> +			if (ret & PR_SVE_VL_INHERIT) {
> +				ksft_print_msg("prctl() reports _INHERIT\n");
> +				return;

Missing return value?

> +			}
> +			 ksft_print_msg("prctl() does not report _INHERIT\n");

Indentation.

> +
> +		} else if (grandchild_pid < 0) {
> +			ksft_print_msg("fork() failed in first child: %d\n", grandchild_pid);
> +			return 0;
> +		}
> +
> +		/*  Wait for the grandchild process to exit */
> +		waiting = waitpid(grandchild_pid, &child_status, 0);
> +		if (waiting < 0) {
> +			if (errno == EINTR)
> +				continue;

'continue' outside of a loop?

> +			ksft_print_msg("waitpid() failed: %d\n", errno);
> +			return 0;
> +		}
> +		if (waiting != grandchild_pid) {
> +			ksft_print_msg("waitpid() returned wrong PID\n");
> +			return 0;
> +		}
> +
> +		if (!WIFEXITED(child_status)) {
> +			ksft_print_msg("grandchild did not exit\n");
> +			return 0;
> +		}
> +
> +		exit(1);
> +		}

Stray '}' ?

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: [PATCH v8 11/16] irqchip/gic-v3: Add support for ACPI's disabled but 'online capable' CPUs
From: Jonathan Cameron @ 2024-04-30 12:15 UTC (permalink / raw)
  To: Marc Zyngier, linuxarm, linuxarm
  Cc: Thomas Gleixner, Peter Zijlstra, linux-pm, loongarch, linux-acpi,
	linux-arch, linux-kernel, linux-arm-kernel, kvmarm, x86,
	Russell King, Rafael J . Wysocki, Miguel Luis, James Morse,
	Salil Mehta, Jean-Philippe Brucker, Catalin Marinas, Will Deacon,
	Hanjun Guo, Ingo Molnar, Borislav Petkov, Dave Hansen, justin.he,
	jianyong.wu, Lorenzo Pieralisi, Sudeep Holla
In-Reply-To: <20240429101938.000027b2@huawei.com>

On Mon, 29 Apr 2024 10:21:31 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:

> On Sun, 28 Apr 2024 12:28:03 +0100
> Marc Zyngier <maz@kernel.org> wrote:
> 
> > On Fri, 26 Apr 2024 19:28:58 +0100,
> > Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:  
> > > 
> > > 
> > > I'll not send a formal v9 until early next week, so here is the current state
> > > if you have time to take another look before then.    
> > 
> > Don't bother resending this on my account -- you only sent it on
> > Friday and there hasn't been much response to it yet. There is still a
> > problem (see below), but looks otherwise OK.
> > 
> > [...]
> >   
> > > @@ -2363,11 +2381,25 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header,
> > >  				(struct acpi_madt_generic_interrupt *)header;
> > >  	u32 reg = readl_relaxed(acpi_data.dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> > >  	u32 size = reg == GIC_PIDR2_ARCH_GICv4 ? SZ_64K * 4 : SZ_64K * 2;
> > > +	int cpu = get_cpu_for_acpi_id(gicc->uid);    
> > 
> > I already commented that get_cpu_for_acpi_id() can...  
> 
> Indeed sorry - I blame Friday syndrome for me failing to address that.
> 
> >   
> > >  	void __iomem *redist_base;
> > >  
> > > -	if (!acpi_gicc_is_usable(gicc))
> > > +	/* Neither enabled or online capable means it doesn't exist, skip it */
> > > +	if (!(gicc->flags & (ACPI_MADT_ENABLED | ACPI_MADT_GICC_ONLINE_CAPABLE)))
> > >  		return 0;
> > >  
> > > +	/*
> > > +	 * Capable but disabled CPUs can be brought online later. What about
> > > +	 * the redistributor? ACPI doesn't want to say!
> > > +	 * Virtual hotplug systems can use the MADT's "always-on" GICR entries.
> > > +	 * Otherwise, prevent such CPUs from being brought online.
> > > +	 */
> > > +	if (!(gicc->flags & ACPI_MADT_ENABLED)) {
> > > +		pr_warn("CPU %u's redistributor is inaccessible: this CPU can't be brought online\n", cpu);
> > > +		cpumask_set_cpu(cpu, &broken_rdists);    
> > 
> > ... return -EINVAL, and then be passed to cpumask_set_cpu(), with
> > interesting effects. It shouldn't happen, but I trust anything that
> > comes from firmware tables as much as I trust a campaigning
> > politician's promises. This should really result in the RD being
> > considered unusable, but without affecting any CPU (there is no valid
> > CPU the first place).
> > 
> > Another question is what get_cpu_for acpi_id() returns for a disabled
> > CPU. A valid CPU number? Or -EINVAL?  
> It's a match function that works by iterating over 0 to nr_cpu_ids and
> 
> if (uid == get_acpi_id_for_cpu(cpu))
> 
> So the question become does get_acpi_id_for_cpu() return a valid CPU
> number for a disabled CPU.
> 
> That uses acpi_cpu_get_madt_gicc(cpu)->uid so this all gets a bit circular.
> That looks it up via cpu_madt_gicc[cpu] which after the proposed updated
> patch is set if enabled or online capable.  There are however a few other
> error checks in acpi_map_gic_cpu_interface() that could lead to it
> not being set (MPIDR validity checks). I suspect all of these end up being
> fatal elsewhere which is why this hasn't blown up before.
> 
> If any of those cases are possible we could get a null pointer
> dereference.
> 
> Easy to harden this case via the following (which will leave us with
> -EINVAL.  There are other call sites that might trip over this.
> I'm inclined to harden them as a separate issue though so as not
> to get in the way of this patch set.
> 
> 
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index bc9a6656fc0c..a407f9cd549e 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -124,7 +124,8 @@ static inline int get_cpu_for_acpi_id(u32 uid)
>         int cpu;
> 
>         for (cpu = 0; cpu < nr_cpu_ids; cpu++)
> -               if (uid == get_acpi_id_for_cpu(cpu))
> +               if (acpi_cpu_get_madt_gicc(cpu) &&
> +                   uid == get_acpi_id_for_cpu(cpu))
>                         return cpu;
> 
>         return -EINVAL;
> 
> I'll spin an additional patch to make that change after testing I haven't
> messed it up.
> 
> At the call site in gic_acpi_parse_madt_gicc() I'm not sure we can do better
> than just skipping setting broken_rdists. I'll also pull the declaration of
> that cpu variable down into this condition so it's more obvious we only
> care about it in this error path.

Just for the record, for my deliberately broken test case it seems that it returns
a valid CPU ID anyway. That's what I'd expect given acpi_parse_and_init_cpus()
doesn't check if the gicc entrees are enabled or not.

Jonathan

> 
> Jonathan
> 
> 
> 
> 
> 
> > 
> > Thanks,
> > 
> > 	M.
> >   
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


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

^ permalink raw reply

* Re: [PATCH v6 4/5] sched: Rename arch_update_thermal_pressure into arch_update_hw_pressure
From: Konrad Dybcio @ 2024-04-30 12:15 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: linux, catalin.marinas, will, sudeep.holla, rafael, viresh.kumar,
	agross, andersson, mingo, peterz, juri.lelli, dietmar.eggemann,
	rostedt, bsegall, mgorman, bristot, vschneid, lukasz.luba,
	rui.zhang, mhiramat, daniel.lezcano, amit.kachhap, corbet, gregkh,
	linux-arm-kernel, linux-kernel, linux-pm, linux-arm-msm,
	linux-trace-kernel, linux-doc, Qais Yousef, Dmitry Baryshkov
In-Reply-To: <CAKfTPtDhVfpvO46YWmMnVhJmiKUbNJt7d2cvmyXfPJ4g1YZkXg@mail.gmail.com>

On 30.04.2024 2:00 PM, Vincent Guittot wrote:
> H Konrad,
> 
> On Tue, 30 Apr 2024 at 13:23, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> On 26.03.2024 10:16 AM, Vincent Guittot wrote:
>>> Now that cpufreq provides a pressure value to the scheduler, rename
>>> arch_update_thermal_pressure into HW pressure to reflect that it returns
>>> a pressure applied by HW (i.e. with a high frequency change) and not
>>> always related to thermal mitigation but also generated by max current
>>> limitation as an example. Such high frequency signal needs filtering to be
>>> smoothed and provide an value that reflects the average available capacity
>>> into the scheduler time scale.
>>>
>>> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
>>> Reviewed-by: Qais Yousef <qyousef@layalina.io>
>>> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
>>> Tested-by: Lukasz Luba <lukasz.luba@arm.com>
>>> ---
>>
>> Hi, I'm not quite sure how, but this commit specifically breaks booting
>> on Qualcomm platforms with EAS..
> 
> This is the fix:
> https://lore.kernel.org/lkml/20240425073709.379016-1-vincent.guittot@linaro.org/

Yep, works now!

> 
>>
>> https://pastebin.com/raw/1Uh7u81x
> 
> Which platform is it ?
> I tested it on dragonboard rb3 and it booted and run tests  even w/o the fix

As the log suggests, SM8550 QRD

Thanks for your prompt response!

Konrad

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

^ permalink raw reply

* [PATCH net-next v3] net: ti: icssg_prueth: Add SW TX / RX Coalescing based on hrtimers
From: MD Danish Anwar @ 2024-04-30 12:06 UTC (permalink / raw)
  To: Dan Carpenter, Heiner Kallweit, Andrew Lunn, Jan Kiszka,
	Diogo Ivo, Paolo Abeni, Jakub Kicinski, Eric Dumazet,
	David S. Miller
  Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
	r-gunasekaran, Roger Quadros, Simon Horman, MD Danish Anwar

Add SW IRQ coalescing based on hrtimers for RX and TX data path for ICSSG
driver, which can be enabled by ethtool commands:

- RX coalescing
  ethtool -C eth1 rx-usecs 50

- TX coalescing can be enabled per TX queue

  - by default enables coalescing for TX0
  ethtool -C eth1 tx-usecs 50
  - configure TX0
  ethtool -Q eth0 queue_mask 1 --coalesce tx-usecs 100
  - configure TX1
  ethtool -Q eth0 queue_mask 2 --coalesce tx-usecs 100
  - configure TX0 and TX1
  ethtool -Q eth0 queue_mask 3 --coalesce tx-usecs 100 --coalesce
tx-usecs 100

Minimum value for both rx-usecs and tx-usecs is 20us.

Compared to gro_flush_timeout and napi_defer_hard_irqs this patch allows
to enable IRQ coalescing for RX path separately.

Benchmarking numbers:
 ===============================================================
| Method                  | Tput_TX | CPU_TX | Tput_RX | CPU_RX |
| ==============================================================
| Default Driver           943 Mbps    31%      517 Mbps  38%   |
| IRQ Coalescing (Patch)   943 Mbps    28%      518 Mbps  25%   |
 ===============================================================

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
Changes from v2 [2] to v3:
*) Collected RB tag from Andrew Lunn <andrew@lunn.ch>
*) Fixed warning caused by {} missing for outer if conditions in
   emac_napi_tx_poll() and emac_napi_rx_poll() APIs.
*) Fixed spelling mistake.

Changes from v1 [1] to v2:
*) Added Benchmarking numbers in the commit message as suggested by
   Andrew Lunn <andrew@lunn.ch>. Full logs [3]
*) Addressed comments given by Simon Horman <horms@kernel.org> in v1.

[1] https://lore.kernel.org/all/20240424091823.1814136-1-danishanwar@ti.com/
[2] https://lore.kernel.org/all/20240429071501.547680-1-danishanwar@ti.com/
[3] https://gist.githubusercontent.com/danish-ti/47855631be9f3635cee994693662a988/raw/94b4eb86b42fe243ab03186a88a314e0cb272fd0/gistfile1.txt

 drivers/net/ethernet/ti/icssg/icssg_common.c  | 41 ++++++--
 drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 93 +++++++++++++++++++
 drivers/net/ethernet/ti/icssg/icssg_prueth.c  | 18 +++-
 drivers/net/ethernet/ti/icssg/icssg_prueth.h  | 11 ++-
 4 files changed, 155 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index 284f97876054..088ab8076db4 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -122,7 +122,7 @@ void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
 }
 
 int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
-			     int budget)
+			     int budget, bool *tdown)
 {
 	struct net_device *ndev = emac->ndev;
 	struct cppi5_host_desc_t *desc_tx;
@@ -145,6 +145,7 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
 		if (cppi5_desc_is_tdcm(desc_dma)) {
 			if (atomic_dec_and_test(&emac->tdown_cnt))
 				complete(&emac->tdown_complete);
+			*tdown = true;
 			break;
 		}
 
@@ -190,19 +191,37 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
 	return num_tx;
 }
 
+static enum hrtimer_restart emac_tx_timer_callback(struct hrtimer *timer)
+{
+	struct prueth_tx_chn *tx_chns =
+			container_of(timer, struct prueth_tx_chn, tx_hrtimer);
+
+	enable_irq(tx_chns->irq);
+	return HRTIMER_NORESTART;
+}
+
 static int emac_napi_tx_poll(struct napi_struct *napi_tx, int budget)
 {
 	struct prueth_tx_chn *tx_chn = prueth_napi_to_tx_chn(napi_tx);
 	struct prueth_emac *emac = tx_chn->emac;
+	bool tdown = false;
 	int num_tx_packets;
 
-	num_tx_packets = emac_tx_complete_packets(emac, tx_chn->id, budget);
+	num_tx_packets = emac_tx_complete_packets(emac, tx_chn->id, budget,
+						  &tdown);
 
 	if (num_tx_packets >= budget)
 		return budget;
 
-	if (napi_complete_done(napi_tx, num_tx_packets))
-		enable_irq(tx_chn->irq);
+	if (napi_complete_done(napi_tx, num_tx_packets)) {
+		if (unlikely(tx_chn->tx_pace_timeout_ns && !tdown)) {
+			hrtimer_start(&tx_chn->tx_hrtimer,
+				      ns_to_ktime(tx_chn->tx_pace_timeout_ns),
+				      HRTIMER_MODE_REL_PINNED);
+		} else {
+			enable_irq(tx_chn->irq);
+		}
+	}
 
 	return num_tx_packets;
 }
@@ -226,6 +245,9 @@ int prueth_ndev_add_tx_napi(struct prueth_emac *emac)
 		struct prueth_tx_chn *tx_chn = &emac->tx_chns[i];
 
 		netif_napi_add_tx(emac->ndev, &tx_chn->napi_tx, emac_napi_tx_poll);
+		hrtimer_init(&tx_chn->tx_hrtimer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_PINNED);
+		tx_chn->tx_hrtimer.function = &emac_tx_timer_callback;
 		ret = request_irq(tx_chn->irq, prueth_tx_irq,
 				  IRQF_TRIGGER_HIGH, tx_chn->name,
 				  tx_chn);
@@ -871,8 +893,15 @@ int emac_napi_rx_poll(struct napi_struct *napi_rx, int budget)
 			break;
 	}
 
-	if (num_rx < budget && napi_complete_done(napi_rx, num_rx))
-		enable_irq(emac->rx_chns.irq[rx_flow]);
+	if (num_rx < budget && napi_complete_done(napi_rx, num_rx)) {
+		if (unlikely(emac->rx_pace_timeout_ns)) {
+			hrtimer_start(&emac->rx_hrtimer,
+				      ns_to_ktime(emac->rx_pace_timeout_ns),
+				      HRTIMER_MODE_REL_PINNED);
+		} else {
+			enable_irq(emac->rx_chns.irq[rx_flow]);
+		}
+	}
 
 	return num_rx;
 }
diff --git a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
index ca20325d4d3e..c8d0f45cc5b1 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
@@ -201,6 +201,93 @@ static void emac_get_rmon_stats(struct net_device *ndev,
 	rmon_stats->hist_tx[4] = emac_get_stat_by_name(emac, "tx_bucket5_frames");
 }
 
+static int emac_get_coalesce(struct net_device *ndev,
+			     struct ethtool_coalesce *coal,
+			     struct kernel_ethtool_coalesce *kernel_coal,
+			     struct netlink_ext_ack *extack)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+	struct prueth_tx_chn *tx_chn;
+
+	tx_chn = &emac->tx_chns[0];
+
+	coal->rx_coalesce_usecs = emac->rx_pace_timeout_ns / 1000;
+	coal->tx_coalesce_usecs = tx_chn->tx_pace_timeout_ns / 1000;
+
+	return 0;
+}
+
+static int emac_get_per_queue_coalesce(struct net_device *ndev, u32 queue,
+				       struct ethtool_coalesce *coal)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+	struct prueth_tx_chn *tx_chn;
+
+	if (queue >= PRUETH_MAX_TX_QUEUES)
+		return -EINVAL;
+
+	tx_chn = &emac->tx_chns[queue];
+
+	coal->tx_coalesce_usecs = tx_chn->tx_pace_timeout_ns / 1000;
+
+	return 0;
+}
+
+static int emac_set_coalesce(struct net_device *ndev,
+			     struct ethtool_coalesce *coal,
+			     struct kernel_ethtool_coalesce *kernel_coal,
+			     struct netlink_ext_ack *extack)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+	struct prueth *prueth = emac->prueth;
+	struct prueth_tx_chn *tx_chn;
+
+	tx_chn = &emac->tx_chns[0];
+
+	if (coal->rx_coalesce_usecs &&
+	    coal->rx_coalesce_usecs < ICSSG_MIN_COALESCE_USECS) {
+		dev_info(prueth->dev, "defaulting to min value of %dus for rx-usecs\n",
+			 ICSSG_MIN_COALESCE_USECS);
+		coal->rx_coalesce_usecs = ICSSG_MIN_COALESCE_USECS;
+	}
+
+	if (coal->tx_coalesce_usecs &&
+	    coal->tx_coalesce_usecs < ICSSG_MIN_COALESCE_USECS) {
+		dev_info(prueth->dev, "defaulting to min value of %dus for tx-usecs\n",
+			 ICSSG_MIN_COALESCE_USECS);
+		coal->tx_coalesce_usecs = ICSSG_MIN_COALESCE_USECS;
+	}
+
+	emac->rx_pace_timeout_ns = coal->rx_coalesce_usecs * 1000;
+	tx_chn->tx_pace_timeout_ns = coal->tx_coalesce_usecs * 1000;
+
+	return 0;
+}
+
+static int emac_set_per_queue_coalesce(struct net_device *ndev, u32 queue,
+				       struct ethtool_coalesce *coal)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+	struct prueth *prueth = emac->prueth;
+	struct prueth_tx_chn *tx_chn;
+
+	if (queue >= PRUETH_MAX_TX_QUEUES)
+		return -EINVAL;
+
+	tx_chn = &emac->tx_chns[queue];
+
+	if (coal->tx_coalesce_usecs &&
+	    coal->tx_coalesce_usecs < ICSSG_MIN_COALESCE_USECS) {
+		dev_info(prueth->dev, "defaulting to min value of %dus for tx-usecs for tx-%u\n",
+			 ICSSG_MIN_COALESCE_USECS, queue);
+		coal->tx_coalesce_usecs = ICSSG_MIN_COALESCE_USECS;
+	}
+
+	tx_chn->tx_pace_timeout_ns = coal->tx_coalesce_usecs * 1000;
+
+	return 0;
+}
+
 const struct ethtool_ops icssg_ethtool_ops = {
 	.get_drvinfo = emac_get_drvinfo,
 	.get_msglevel = emac_get_msglevel,
@@ -209,6 +296,12 @@ const struct ethtool_ops icssg_ethtool_ops = {
 	.get_ethtool_stats = emac_get_ethtool_stats,
 	.get_strings = emac_get_strings,
 	.get_ts_info = emac_get_ts_info,
+	.supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS |
+				     ETHTOOL_COALESCE_TX_USECS,
+	.get_coalesce = emac_get_coalesce,
+	.set_coalesce = emac_set_coalesce,
+	.get_per_queue_coalesce = emac_get_per_queue_coalesce,
+	.set_per_queue_coalesce = emac_set_per_queue_coalesce,
 	.get_channels = emac_get_channels,
 	.set_channels = emac_set_channels,
 	.get_link_ksettings = emac_get_link_ksettings,
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 186b0365c2e5..7c9e9518f555 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -243,6 +243,16 @@ static void emac_adjust_link(struct net_device *ndev)
 	}
 }
 
+static enum hrtimer_restart emac_rx_timer_callback(struct hrtimer *timer)
+{
+	struct prueth_emac *emac =
+			container_of(timer, struct prueth_emac, rx_hrtimer);
+	int rx_flow = PRUETH_RX_FLOW_DATA;
+
+	enable_irq(emac->rx_chns.irq[rx_flow]);
+	return HRTIMER_NORESTART;
+}
+
 static int emac_phy_connect(struct prueth_emac *emac)
 {
 	struct prueth *prueth = emac->prueth;
@@ -582,8 +592,10 @@ static int emac_ndo_stop(struct net_device *ndev)
 		netdev_err(ndev, "tx teardown timeout\n");
 
 	prueth_reset_tx_chan(emac, emac->tx_ch_num, true);
-	for (i = 0; i < emac->tx_ch_num; i++)
+	for (i = 0; i < emac->tx_ch_num; i++) {
 		napi_disable(&emac->tx_chns[i].napi_tx);
+		hrtimer_cancel(&emac->tx_chns[i].tx_hrtimer);
+	}
 
 	max_rx_flows = PRUETH_MAX_RX_FLOWS;
 	k3_udma_glue_tdown_rx_chn(emac->rx_chns.rx_chn, true);
@@ -591,6 +603,7 @@ static int emac_ndo_stop(struct net_device *ndev)
 	prueth_reset_rx_chan(&emac->rx_chns, max_rx_flows, true);
 
 	napi_disable(&emac->napi_rx);
+	hrtimer_cancel(&emac->rx_hrtimer);
 
 	cancel_work_sync(&emac->rx_mode_work);
 
@@ -801,6 +814,9 @@ static int prueth_netdev_init(struct prueth *prueth,
 	ndev->features = ndev->hw_features;
 
 	netif_napi_add(ndev, &emac->napi_rx, emac_napi_rx_poll);
+	hrtimer_init(&emac->rx_hrtimer, CLOCK_MONOTONIC,
+		     HRTIMER_MODE_REL_PINNED);
+	emac->rx_hrtimer.function = &emac_rx_timer_callback;
 	prueth->emac[mac] = emac;
 
 	return 0;
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 82e38ef5635b..a78c5eb75fb8 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -108,6 +108,8 @@ struct prueth_tx_chn {
 	u32 descs_num;
 	unsigned int irq;
 	char name[32];
+	struct hrtimer tx_hrtimer;
+	unsigned long tx_pace_timeout_ns;
 };
 
 struct prueth_rx_chn {
@@ -127,6 +129,9 @@ struct prueth_rx_chn {
 
 #define PRUETH_MAX_TX_TS_REQUESTS	50 /* Max simultaneous TX_TS requests */
 
+/* Minimum coalesce time in usecs for both Tx and Rx */
+#define ICSSG_MIN_COALESCE_USECS 20
+
 /* data for each emac port */
 struct prueth_emac {
 	bool is_sr1;
@@ -183,6 +188,10 @@ struct prueth_emac {
 
 	struct delayed_work stats_work;
 	u64 stats[ICSSG_NUM_STATS];
+
+	/* RX IRQ Coalescing Related */
+	struct hrtimer rx_hrtimer;
+	unsigned long rx_pace_timeout_ns;
 };
 
 /**
@@ -320,7 +329,7 @@ void prueth_ndev_del_tx_napi(struct prueth_emac *emac, int num);
 void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
 		      struct cppi5_host_desc_t *desc);
 int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
-			     int budget);
+			     int budget, bool *tdown);
 int prueth_ndev_add_tx_napi(struct prueth_emac *emac);
 int prueth_init_tx_chns(struct prueth_emac *emac);
 int prueth_init_rx_chns(struct prueth_emac *emac,

base-commit: 9f02bb6d7a229058ffaba4f6dd78e0f7b06b799c
-- 
2.34.1


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

^ permalink raw reply related

* Re: [EXT] [PATCH v8 6/6] docs: trusted-encrypted: add DCP as new trust source
From: David Gstir @ 2024-04-30 12:03 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Mimi Zohar, James Bottomley, Herbert Xu, David S. Miller,
	Kshitiz Varshney, Shawn Guo, Jonathan Corbet, Sascha Hauer,
	kernel@pengutronix.de, Fabio Estevam, dl-linux-imx, Ahmad Fatoum,
	sigma star Kernel Team, David Howells, Li Yang, Paul Moore,
	James Morris, Serge E. Hallyn, Paul E. McKenney, Randy Dunlap,
	Catalin Marinas, Rafael J. Wysocki, Tejun Heo,
	Steven Rostedt (Google), linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org,
	keyrings@vger.kernel.org, linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org,
	linux-security-module@vger.kernel.org, Richard Weinberger,
	David Oberhollenzer, Varun Sethi, Gaurav Jain, Pankaj Gupta
In-Reply-To: <DB6PR04MB319062F2A19A250BA22C12D48F1A2@DB6PR04MB3190.eurprd04.prod.outlook.com>

Hi Jarkko,

> On 30.04.2024, at 13:48, Kshitiz Varshney <kshitiz.varshney@nxp.com> wrote:
> 
> Hi David,
> 
>> -----Original Message-----
>> From: David Gstir <david@sigma-star.at>
>> Sent: Monday, April 29, 2024 5:05 PM
>> To: Kshitiz Varshney <kshitiz.varshney@nxp.com>


>> 
>> Did you get around to testing this?
>> I’d greatly appreciate a Tested-by for this. :-)
>> 
>> Thanks!
>> BR, David
> 
> Currently, I am bit busy with other priority activities. It will take time to test this patch set.

How should we proceed here?
Do we have to miss another release cycle, because of a Tested-by?

If any bugs pop up I’ll happily fix them, but at the moment it appears to be more of a formality.
IMHO the patch set itself is rather small and has been thoroughly reviewed to ensure that any huge
issues would already have been caught by now.

Thanks!
BR, David
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v6 4/5] sched: Rename arch_update_thermal_pressure into arch_update_hw_pressure
From: Vincent Guittot @ 2024-04-30 12:00 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: linux, catalin.marinas, will, sudeep.holla, rafael, viresh.kumar,
	agross, andersson, mingo, peterz, juri.lelli, dietmar.eggemann,
	rostedt, bsegall, mgorman, bristot, vschneid, lukasz.luba,
	rui.zhang, mhiramat, daniel.lezcano, amit.kachhap, corbet, gregkh,
	linux-arm-kernel, linux-kernel, linux-pm, linux-arm-msm,
	linux-trace-kernel, linux-doc, Qais Yousef, Dmitry Baryshkov
In-Reply-To: <95760e2b-ec38-4f04-8f86-e4f935d24a83@linaro.org>

H Konrad,

On Tue, 30 Apr 2024 at 13:23, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 26.03.2024 10:16 AM, Vincent Guittot wrote:
> > Now that cpufreq provides a pressure value to the scheduler, rename
> > arch_update_thermal_pressure into HW pressure to reflect that it returns
> > a pressure applied by HW (i.e. with a high frequency change) and not
> > always related to thermal mitigation but also generated by max current
> > limitation as an example. Such high frequency signal needs filtering to be
> > smoothed and provide an value that reflects the average available capacity
> > into the scheduler time scale.
> >
> > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> > Reviewed-by: Qais Yousef <qyousef@layalina.io>
> > Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
> > Tested-by: Lukasz Luba <lukasz.luba@arm.com>
> > ---
>
> Hi, I'm not quite sure how, but this commit specifically breaks booting
> on Qualcomm platforms with EAS..

This is the fix:
https://lore.kernel.org/lkml/20240425073709.379016-1-vincent.guittot@linaro.org/

>
> https://pastebin.com/raw/1Uh7u81x

Which platform is it ?
I tested it on dragonboard rb3 and it booted and run tests  even w/o the fix

>
> Konrad

_______________________________________________
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 net-next] net: phy: Don't conditionally compile the phy_link_topology creation
From: Maxime Chevallier @ 2024-04-30 11:57 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn,
	Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Jesse Brandeburg, Marek Behún, Piergiorgio Beruto,
	Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
	Nathan Chancellor, Antoine Tenart
In-Reply-To: <1ed5b8cb-c79b-44b9-8dbe-f78d7505b3b4@gmail.com>

Hello Heiner,

On Tue, 30 Apr 2024 10:17:31 +0200
Heiner Kallweit <hkallweit1@gmail.com> wrote:

> On 29.04.2024 15:10, Maxime Chevallier wrote:
> > The core of the phy_link_topology isn't directly tied to phylib, and at
> > the moment it's initialized, phylib might not be loaded yet. Move the
> > initialization of the topology to the phy_link_topology_core header,
> > which contains the bare minimum so that we can initialize it at netdev
> > creation.
> >   
> 
> The change fixes the issue for me, but according to my personal taste
> the code isn't intuitive and still error-prone. Also there's no good
> reason to inline a function like phy_link_topo_create() and make it
> publicly available. Do you expect it to be ever used outside net core?
> In general it may make sense to add a config symbol for the topology
> extension, there seem to be very few, specialized use cases for it.

I think I'm missing the point here then. Do you mean adding a Kconfig
option to explicitely turn phy_link_topology on ? or build it as a
dedicated kernel module ?

Or do you see something such as "if phylib is M or Y, then build the
topology stuff and make sure it's allocated when a netdev gets created
?"

Thanks,

Maxime

> 
> > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > Closes: https://lore.kernel.org/netdev/2e11b89d-100f-49e7-9c9a-834cc0b82f97@gmail.com/
> > Closes: https://lore.kernel.org/netdev/20240409201553.GA4124869@dev-arch.thelio-3990X/
> > ---
> >  drivers/net/phy/phy_link_topology.c    | 23 --------------------
> >  include/linux/phy_link_topology.h      |  5 -----
> >  include/linux/phy_link_topology_core.h | 30 +++++++++++++++++---------
> >  3 files changed, 20 insertions(+), 38 deletions(-)
> > 
> > diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> > index 985941c5c558..960aedd73308 100644
> > --- a/drivers/net/phy/phy_link_topology.c
> > +++ b/drivers/net/phy/phy_link_topology.c
> > @@ -12,29 +12,6 @@
> >  #include <linux/rtnetlink.h>
> >  #include <linux/xarray.h>
> >  
> > -struct phy_link_topology *phy_link_topo_create(struct net_device *dev)
> > -{
> > -	struct phy_link_topology *topo;
> > -
> > -	topo = kzalloc(sizeof(*topo), GFP_KERNEL);
> > -	if (!topo)
> > -		return ERR_PTR(-ENOMEM);
> > -
> > -	xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1);
> > -	topo->next_phy_index = 1;
> > -
> > -	return topo;
> > -}
> > -
> > -void phy_link_topo_destroy(struct phy_link_topology *topo)
> > -{
> > -	if (!topo)
> > -		return;
> > -
> > -	xa_destroy(&topo->phys);
> > -	kfree(topo);
> > -}
> > -
> >  int phy_link_topo_add_phy(struct phy_link_topology *topo,
> >  			  struct phy_device *phy,
> >  			  enum phy_upstream upt, void *upstream)
> > diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h
> > index 6b79feb607e7..ad72d7881257 100644
> > --- a/include/linux/phy_link_topology.h
> > +++ b/include/linux/phy_link_topology.h
> > @@ -32,11 +32,6 @@ struct phy_device_node {
> >  	struct phy_device *phy;
> >  };
> >  
> > -struct phy_link_topology {
> > -	struct xarray phys;
> > -	u32 next_phy_index;
> > -};
> > -
> >  static inline struct phy_device *
> >  phy_link_topo_get_phy(struct phy_link_topology *topo, u32 phyindex)
> >  {
> > diff --git a/include/linux/phy_link_topology_core.h b/include/linux/phy_link_topology_core.h
> > index 0a6479055745..0116ec49cd1b 100644
> > --- a/include/linux/phy_link_topology_core.h
> > +++ b/include/linux/phy_link_topology_core.h
> > @@ -2,24 +2,34 @@
> >  #ifndef __PHY_LINK_TOPOLOGY_CORE_H
> >  #define __PHY_LINK_TOPOLOGY_CORE_H
> >  
> > -struct phy_link_topology;
> > +#include <linux/xarray.h>
> >  
> > -#if IS_REACHABLE(CONFIG_PHYLIB)
> > -
> > -struct phy_link_topology *phy_link_topo_create(struct net_device *dev);
> > -void phy_link_topo_destroy(struct phy_link_topology *topo);
> > -
> > -#else
> > +struct phy_link_topology {
> > +	struct xarray phys;
> > +	u32 next_phy_index;
> > +};
> >  
> >  static inline struct phy_link_topology *phy_link_topo_create(struct net_device *dev)
> >  {
> > -	return NULL;
> > +	struct phy_link_topology *topo;
> > +
> > +	topo = kzalloc(sizeof(*topo), GFP_KERNEL);
> > +	if (!topo)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1);
> > +	topo->next_phy_index = 1;
> > +
> > +	return topo;
> >  }
> >  
> >  static inline void phy_link_topo_destroy(struct phy_link_topology *topo)
> >  {
> > -}
> > +	if (!topo)
> > +		return;
> >  
> > -#endif
> > +	xa_destroy(&topo->phys);
> > +	kfree(topo);
> > +}
> >  
> >  #endif /* __PHY_LINK_TOPOLOGY_CORE_H */  
> 


_______________________________________________
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 net-next v2] net: dsa: mt7530: do not set MT7530_P5_DIS when PHY muxing is being used
From: patchwork-bot+netdevbpf @ 2024-04-30 11:50 UTC (permalink / raw)
  To: =?utf-8?b?QXLEsW7DpyDDnE5BTCB2aWEgQjQgUmVsYXkgPGRldm51bGwrYXJpbmMudW5hbC5h?=,
	=?utf-8?b?cmluYzkuY29tQGtlcm5lbC5vcmc+?=
  Cc: daniel, dqfext, sean.wang, andrew, f.fainelli, olteanv, davem,
	edumazet, kuba, pabeni, matthias.bgg, angelogioacchino.delregno,
	netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
	arinc.unal
In-Reply-To: <20240428-for-netnext-mt7530-do-not-disable-port5-when-phy-muxing-v2-1-bb7c37d293f8@arinc9.com>

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Sun, 28 Apr 2024 12:19:58 +0300 you wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> DSA initalises the ds->num_ports amount of ports in
> dsa_switch_touch_ports(). When the PHY muxing feature is in use, port 5
> won't be defined in the device tree. Because of this, the type member of
> the dsa_port structure for this port will be assigned DSA_PORT_TYPE_UNUSED.
> The dsa_port_setup() function calls ds->ops->port_disable() when the port
> type is DSA_PORT_TYPE_UNUSED.
> 
> [...]

Here is the summary with links:
  - [net-next,v2] net: dsa: mt7530: do not set MT7530_P5_DIS when PHY muxing is being used
    https://git.kernel.org/netdev/net-next/c/16e6592cd5c5

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

^ permalink raw reply

* RE: [EXT] [PATCH v8 6/6] docs: trusted-encrypted: add DCP as new trust source
From: Kshitiz Varshney @ 2024-04-30 11:48 UTC (permalink / raw)
  To: David Gstir
  Cc: Jarkko Sakkinen, Mimi Zohar, James Bottomley, Herbert Xu,
	David S. Miller, Shawn Guo, Jonathan Corbet, Sascha Hauer,
	kernel@pengutronix.de, Fabio Estevam, dl-linux-imx, Ahmad Fatoum,
	sigma star Kernel Team, David Howells, Li Yang, Paul Moore,
	James Morris, Serge E. Hallyn, Paul E. McKenney, Randy Dunlap,
	Catalin Marinas, Rafael J. Wysocki, Tejun Heo,
	Steven Rostedt (Google), linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org,
	keyrings@vger.kernel.org, linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org,
	linux-security-module@vger.kernel.org, Richard Weinberger,
	David Oberhollenzer, Varun Sethi, Gaurav Jain, Pankaj Gupta
In-Reply-To: <7783BAE9-87DA-4DD5-ADFA-15A9B55EEF39@sigma-star.at>

Hi David,

> -----Original Message-----
> From: David Gstir <david@sigma-star.at>
> Sent: Monday, April 29, 2024 5:05 PM
> To: Kshitiz Varshney <kshitiz.varshney@nxp.com>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>; Mimi Zohar
> <zohar@linux.ibm.com>; James Bottomley <jejb@linux.ibm.com>; Herbert
> Xu <herbert@gondor.apana.org.au>; David S. Miller
> <davem@davemloft.net>; Shawn Guo <shawnguo@kernel.org>; Jonathan
> Corbet <corbet@lwn.net>; Sascha Hauer <s.hauer@pengutronix.de>;
> kernel@pengutronix.de; Fabio Estevam <festevam@gmail.com>; dl-linux-imx
> <linux-imx@nxp.com>; Ahmad Fatoum <a.fatoum@pengutronix.de>; sigma
> star Kernel Team <upstream+dcp@sigma-star.at>; David Howells
> <dhowells@redhat.com>; Li Yang <leoyang.li@nxp.com>; Paul Moore
> <paul@paul-moore.com>; James Morris <jmorris@namei.org>; Serge E.
> Hallyn <serge@hallyn.com>; Paul E. McKenney <paulmck@kernel.org>;
> Randy Dunlap <rdunlap@infradead.org>; Catalin Marinas
> <catalin.marinas@arm.com>; Rafael J. Wysocki
> <rafael.j.wysocki@intel.com>; Tejun Heo <tj@kernel.org>; Steven Rostedt
> (Google) <rostedt@goodmis.org>; linux-doc@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-integrity@vger.kernel.org;
> keyrings@vger.kernel.org; linux-crypto@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org; linux-security-
> module@vger.kernel.org; Richard Weinberger <richard@nod.at>; David
> Oberhollenzer <david.oberhollenzer@sigma-star.at>; Varun Sethi
> <V.Sethi@nxp.com>; Gaurav Jain <gaurav.jain@nxp.com>; Pankaj Gupta
> <pankaj.gupta@nxp.com>
> Subject: Re: [EXT] [PATCH v8 6/6] docs: trusted-encrypted: add DCP as new
> trust source
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Hi Kshitiz,
> 
> > On 09.04.2024, at 11:48, Kshitiz Varshney <kshitiz.varshney@nxp.com>
> wrote:
> >
> > Hi Jarkko,
> >
> >
> >> -----Original Message-----
> >> From: Jarkko Sakkinen <jarkko@kernel.org>
> >> Sent: Wednesday, April 3, 2024 9:18 PM
> >> To: David Gstir <david@sigma-star.at>; Mimi Zohar
> >> <zohar@linux.ibm.com>; James Bottomley <jejb@linux.ibm.com>;
> Herbert
> >> Xu <herbert@gondor.apana.org.au>; David S. Miller
> >> <davem@davemloft.net>
> >> Cc: Shawn Guo <shawnguo@kernel.org>; Jonathan Corbet
> >> <corbet@lwn.net>; Sascha Hauer <s.hauer@pengutronix.de>;
> Pengutronix
> >> Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> >> <festevam@gmail.com>; dl-linux-imx <linux-imx@nxp.com>; Ahmad
> Fatoum
> >> <a.fatoum@pengutronix.de>; sigma star Kernel Team
> >> <upstream+dcp@sigma-star.at>; David Howells <dhowells@redhat.com>;
> Li
> >> Yang <leoyang.li@nxp.com>; Paul Moore <paul@paul-moore.com>;
> James
> >> Morris <jmorris@namei.org>; Serge E. Hallyn <serge@hallyn.com>; Paul
> E.
> >> McKenney <paulmck@kernel.org>; Randy Dunlap
> <rdunlap@infradead.org>;
> >> Catalin Marinas <catalin.marinas@arm.com>; Rafael J. Wysocki
> >> <rafael.j.wysocki@intel.com>; Tejun Heo <tj@kernel.org>; Steven
> >> Rostedt
> >> (Google) <rostedt@goodmis.org>; linux-doc@vger.kernel.org; linux-
> >> kernel@vger.kernel.org; linux-integrity@vger.kernel.org;
> >> keyrings@vger.kernel.org; linux-crypto@vger.kernel.org; linux-arm-
> >> kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org;
> >> linux-security- module@vger.kernel.org; Richard Weinberger
> >> <richard@nod.at>; David Oberhollenzer
> >> <david.oberhollenzer@sigma-star.at>
> >> Subject: [EXT] Re: [PATCH v8 6/6] docs: trusted-encrypted: add DCP as
> >> new trust source
> >>
> >> Caution: This is an external email. Please take care when clicking
> >> links or opening attachments. When in doubt, report the message using
> >> the 'Report this email' button
> >>
> >>
> >> On Wed Apr 3, 2024 at 10:21 AM EEST, David Gstir wrote:
> >>> Update the documentation for trusted and encrypted KEYS with DCP as
> >>> new trust source:
> >>>
> >>> - Describe security properties of DCP trust source
> >>> - Describe key usage
> >>> - Document blob format
> >>>
> >>> Co-developed-by: Richard Weinberger <richard@nod.at>
> >>> Signed-off-by: Richard Weinberger <richard@nod.at>
> >>> Co-developed-by: David Oberhollenzer
> >>> <david.oberhollenzer@sigma-star.at>
> >>> Signed-off-by: David Oberhollenzer
> >>> <david.oberhollenzer@sigma-star.at>
> >>> Signed-off-by: David Gstir <david@sigma-star.at>
> >>> ---
> >>> .../security/keys/trusted-encrypted.rst       | 53 +++++++++++++++++++
> >>> security/keys/trusted-keys/trusted_dcp.c      | 19 +++++++
> >>> 2 files changed, 72 insertions(+)
> >>>
> >>> diff --git a/Documentation/security/keys/trusted-encrypted.rst
> >>> b/Documentation/security/keys/trusted-encrypted.rst
> >>> index e989b9802f92..f4d7e162d5e4 100644
> >>> --- a/Documentation/security/keys/trusted-encrypted.rst
> >>> +++ b/Documentation/security/keys/trusted-encrypted.rst
> >>> @@ -42,6 +42,14 @@ safe.
> >>>          randomly generated and fused into each SoC at manufacturing
> time.
> >>>          Otherwise, a common fixed test key is used instead.
> >>>
> >>> +     (4) DCP (Data Co-Processor: crypto accelerator of various i.MX
> >>> + SoCs)
> >>> +
> >>> +         Rooted to a one-time programmable key (OTP) that is
> >>> + generally
> >> burnt
> >>> +         in the on-chip fuses and is accessible to the DCP
> >>> + encryption engine
> >> only.
> >>> +         DCP provides two keys that can be used as root of trust:
> >>> + the OTP
> >> key
> >>> +         and the UNIQUE key. Default is to use the UNIQUE key, but
> selecting
> >>> +         the OTP key can be done via a module parameter
> >> (dcp_use_otp_key).
> >>> +
> >>>   *  Execution isolation
> >>>
> >>>      (1) TPM
> >>> @@ -57,6 +65,12 @@ safe.
> >>>
> >>>          Fixed set of operations running in isolated execution environment.
> >>>
> >>> +     (4) DCP
> >>> +
> >>> +         Fixed set of cryptographic operations running in isolated
> execution
> >>> +         environment. Only basic blob key encryption is executed there.
> >>> +         The actual key sealing/unsealing is done on main
> >>> + processor/kernel
> >> space.
> >>> +
> >>>   * Optional binding to platform integrity state
> >>>
> >>>      (1) TPM
> >>> @@ -79,6 +93,11 @@ safe.
> >>>          Relies on the High Assurance Boot (HAB) mechanism of NXP SoCs
> >>>          for platform integrity.
> >>>
> >>> +     (4) DCP
> >>> +
> >>> +         Relies on Secure/Trusted boot process (called HAB by vendor) for
> >>> +         platform integrity.
> >>> +
> >>>   *  Interfaces and APIs
> >>>
> >>>      (1) TPM
> >>> @@ -94,6 +113,11 @@ safe.
> >>>
> >>>          Interface is specific to silicon vendor.
> >>>
> >>> +     (4) DCP
> >>> +
> >>> +         Vendor-specific API that is implemented as part of the DCP
> >>> + crypto
> >> driver in
> >>> +         ``drivers/crypto/mxs-dcp.c``.
> >>> +
> >>>   *  Threat model
> >>>
> >>>      The strength and appropriateness of a particular trust source
> >>> for a given @@ -129,6 +153,13 @@ selected trust source:
> >>>      CAAM HWRNG, enable CRYPTO_DEV_FSL_CAAM_RNG_API and
> ensure
> >> the device
> >>>      is probed.
> >>>
> >>> +  *  DCP (Data Co-Processor: crypto accelerator of various i.MX
> >>> + SoCs)
> >>> +
> >>> +     The DCP hardware device itself does not provide a dedicated
> >>> + RNG
> >> interface,
> >>> +     so the kernel default RNG is used. SoCs with DCP like the
> >>> + i.MX6ULL do
> >> have
> >>> +     a dedicated hardware RNG that is independent from DCP which
> >>> + can be
> >> enabled
> >>> +     to back the kernel RNG.
> >>> +
> >>> Users may override this by specifying ``trusted.rng=kernel`` on the
> >>> kernel  command-line to override the used RNG with the kernel's
> >>> random
> >> number pool.
> >>>
> >>> @@ -231,6 +262,19 @@ Usage::
> >>> CAAM-specific format.  The key length for new keys is always in bytes.
> >>> Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
> >>>
> >>> +Trusted Keys usage: DCP
> >>> +-----------------------
> >>> +
> >>> +Usage::
> >>> +
> >>> +    keyctl add trusted name "new keylen" ring
> >>> +    keyctl add trusted name "load hex_blob" ring
> >>> +    keyctl print keyid
> >>> +
> >>> +"keyctl print" returns an ASCII hex copy of the sealed key, which
> >>> +is in format specific to this DCP key-blob implementation.  The key
> >>> +length for new keys is always in bytes. Trusted Keys can be 32 -
> >>> +128 bytes
> >> (256 - 1024 bits).
> >>> +
> >>> Encrypted Keys usage
> >>> --------------------
> >>>
> >>> @@ -426,3 +470,12 @@ string length.
> >>> privkey is the binary representation of TPM2B_PUBLIC excluding the
> >>> initial TPM2B header which can be reconstructed from the ASN.1 octed
> >>> string length.
> >>> +
> >>> +DCP Blob Format
> >>> +---------------
> >>> +
> >>> +.. kernel-doc:: security/keys/trusted-keys/trusted_dcp.c
> >>> +   :doc: dcp blob format
> >>> +
> >>> +.. kernel-doc:: security/keys/trusted-keys/trusted_dcp.c
> >>> +   :identifiers: struct dcp_blob_fmt
> >>> diff --git a/security/keys/trusted-keys/trusted_dcp.c
> >>> b/security/keys/trusted-keys/trusted_dcp.c
> >>> index 16c44aafeab3..b5f81a05be36 100644
> >>> --- a/security/keys/trusted-keys/trusted_dcp.c
> >>> +++ b/security/keys/trusted-keys/trusted_dcp.c
> >>> @@ -19,6 +19,25 @@
> >>> #define DCP_BLOB_VERSION 1
> >>> #define DCP_BLOB_AUTHLEN 16
> >>>
> >>> +/**
> >>> + * DOC: dcp blob format
> >>> + *
> >>> + * The Data Co-Processor (DCP) provides hardware-bound AES keys
> >>> +using its
> >>> + * AES encryption engine only. It does not provide direct key
> >> sealing/unsealing.
> >>> + * To make DCP hardware encryption keys usable as trust source, we
> >>> +define
> >>> + * our own custom format that uses a hardware-bound key to secure
> >>> +the sealing
> >>> + * key stored in the key blob.
> >>> + *
> >>> + * Whenever a new trusted key using DCP is generated, we generate a
> >>> +random 128-bit
> >>> + * blob encryption key (BEK) and 128-bit nonce. The BEK and nonce
> >>> +are used to
> >>> + * encrypt the trusted key payload using AES-128-GCM.
> >>> + *
> >>> + * The BEK itself is encrypted using the hardware-bound key using
> >>> +the DCP's AES
> >>> + * encryption engine with AES-128-ECB. The encrypted BEK, generated
> >>> +nonce,
> >>> + * BEK-encrypted payload and authentication tag make up the blob
> >>> +format together
> >>> + * with a version number, payload length and authentication tag.
> >>> + */
> >>> +
> >>> /**
> >>>  * struct dcp_blob_fmt - DCP BLOB format.
> >>>  *
> >>
> >> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> >>
> >> I can only test that this does not break a machine without the
> >> hardware feature.
> >>
> >> Is there anyone who could possibly peer test these patches?
> > I am already working on testing this patchset on i.MX6 platform.
> 
> Did you get around to testing this?
> I’d greatly appreciate a Tested-by for this. :-)
> 
> Thanks!
> BR, David

Currently, I am bit busy with other priority activities. It will take time to test this patch set.

Regards,
Kshitiz

_______________________________________________
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] ARM: Use conditionals for CFI branches
From: Linus Walleij @ 2024-04-30 11:45 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Russell King, Sami Tolvanen, Kees Cook, Nathan Chancellor,
	Nick Desaulniers, Arnd Bergmann, linux-arm-kernel, llvm
In-Reply-To: <CAMj1kXEfQYMk-PHBDBJS-LxTcx72+6ZZVffGtnoiHB4KcdoaSQ@mail.gmail.com>

On Tue, Apr 30, 2024 at 11:19 AM Ard Biesheuvel <ardb@kernel.org> wrote:


> These functions are only called indirectly if MULTI_CACHE is enabled,
> right? If so, this could be
>
> #if defined(CONFIG_CFI_CLANG) && defined(MULTI_CACHE)

Hm Sami may know better here, but with !MULTI_CACHE the
functions are indeed called directly, but does that mean the
compiler will not emit some magic bytes in front of these
symbols then?

Yours,
Linus Walleij

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

^ permalink raw reply

* [PATCH v2 6/8] spi: sun4i: use 'time_left' variable with wait_for_completion_timeout()
From: Wolfram Sang @ 2024-04-30 11:41 UTC (permalink / raw)
  To: linux-spi
  Cc: Wolfram Sang, Jernej Skrabec, Mark Brown, Chen-Yu Tsai,
	Samuel Holland, linux-arm-kernel, linux-sunxi, linux-kernel
In-Reply-To: <20240430114142.28551-1-wsa+renesas@sang-engineering.com>

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_timeout() causing patterns like:

	timeout = wait_for_completion_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Fix to the proper variable type 'unsigned long' while here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/spi/spi-sun4i.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 11d8bd27b3e9..2ee6755b43f5 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -206,7 +206,8 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
 				  struct spi_transfer *tfr)
 {
 	struct sun4i_spi *sspi = spi_controller_get_devdata(host);
-	unsigned int mclk_rate, div, timeout;
+	unsigned int mclk_rate, div;
+	unsigned long time_left;
 	unsigned int start, end, tx_time;
 	unsigned int tx_len = 0;
 	int ret = 0;
@@ -327,10 +328,10 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
 
 	tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
 	start = jiffies;
-	timeout = wait_for_completion_timeout(&sspi->done,
-					      msecs_to_jiffies(tx_time));
+	time_left = wait_for_completion_timeout(&sspi->done,
+						msecs_to_jiffies(tx_time));
 	end = jiffies;
-	if (!timeout) {
+	if (!time_left) {
 		dev_warn(&host->dev,
 			 "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
 			 dev_name(&spi->dev), tfr->len, tfr->speed_hz,
-- 
2.43.0


_______________________________________________
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 v2 7/8] spi: sun6i: use 'time_left' variable with wait_for_completion_timeout()
From: Wolfram Sang @ 2024-04-30 11:41 UTC (permalink / raw)
  To: linux-spi
  Cc: Wolfram Sang, Jernej Skrabec, Andre Przywara, Mark Brown,
	Chen-Yu Tsai, Samuel Holland, linux-arm-kernel, linux-sunxi,
	linux-kernel
In-Reply-To: <20240430114142.28551-1-wsa+renesas@sang-engineering.com>

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_timeout() causing patterns like:

	timeout = wait_for_completion_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Fix to the proper variable type 'unsigned long' while here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/spi/spi-sun6i.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index cd018ea1abf1..5c26bf056293 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -277,7 +277,8 @@ static int sun6i_spi_transfer_one(struct spi_controller *host,
 				  struct spi_transfer *tfr)
 {
 	struct sun6i_spi *sspi = spi_controller_get_devdata(host);
-	unsigned int div, div_cdr1, div_cdr2, timeout;
+	unsigned int div, div_cdr1, div_cdr2;
+	unsigned long time_left;
 	unsigned int start, end, tx_time;
 	unsigned int trig_level;
 	unsigned int tx_len = 0, rx_len = 0, nbits = 0;
@@ -488,26 +489,26 @@ static int sun6i_spi_transfer_one(struct spi_controller *host,
 
 	tx_time = spi_controller_xfer_timeout(host, tfr);
 	start = jiffies;
-	timeout = wait_for_completion_timeout(&sspi->done,
-					      msecs_to_jiffies(tx_time));
+	time_left = wait_for_completion_timeout(&sspi->done,
+						msecs_to_jiffies(tx_time));
 
 	if (!use_dma) {
 		sun6i_spi_drain_fifo(sspi);
 	} else {
-		if (timeout && rx_len) {
+		if (time_left && rx_len) {
 			/*
 			 * Even though RX on the peripheral side has finished
 			 * RX DMA might still be in flight
 			 */
-			timeout = wait_for_completion_timeout(&sspi->dma_rx_done,
-							      timeout);
-			if (!timeout)
+			time_left = wait_for_completion_timeout(&sspi->dma_rx_done,
+								time_left);
+			if (!time_left)
 				dev_warn(&host->dev, "RX DMA timeout\n");
 		}
 	}
 
 	end = jiffies;
-	if (!timeout) {
+	if (!time_left) {
 		dev_warn(&host->dev,
 			 "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
 			 dev_name(&spi->dev), tfr->len, tfr->speed_hz,
-- 
2.43.0


_______________________________________________
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 v2 3/8] spi: imx: use 'time_left' variable with wait_for_completion_timeout()
From: Wolfram Sang @ 2024-04-30 11:41 UTC (permalink / raw)
  To: linux-spi
  Cc: Wolfram Sang, Peng Fan, Mark Brown, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, imx, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20240430114142.28551-1-wsa+renesas@sang-engineering.com>

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_timeout() causing patterns like:

	timeout = wait_for_completion_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/spi/spi-imx.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index c3e5cee18bea..f4006c82f867 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1405,7 +1405,7 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,
 {
 	struct dma_async_tx_descriptor *desc_tx, *desc_rx;
 	unsigned long transfer_timeout;
-	unsigned long timeout;
+	unsigned long time_left;
 	struct spi_controller *controller = spi_imx->controller;
 	struct sg_table *tx = &transfer->tx_sg, *rx = &transfer->rx_sg;
 	struct scatterlist *last_sg = sg_last(rx->sgl, rx->nents);
@@ -1471,18 +1471,18 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,
 	transfer_timeout = spi_imx_calculate_timeout(spi_imx, transfer->len);
 
 	/* Wait SDMA to finish the data transfer.*/
-	timeout = wait_for_completion_timeout(&spi_imx->dma_tx_completion,
+	time_left = wait_for_completion_timeout(&spi_imx->dma_tx_completion,
 						transfer_timeout);
-	if (!timeout) {
+	if (!time_left) {
 		dev_err(spi_imx->dev, "I/O Error in DMA TX\n");
 		dmaengine_terminate_all(controller->dma_tx);
 		dmaengine_terminate_all(controller->dma_rx);
 		return -ETIMEDOUT;
 	}
 
-	timeout = wait_for_completion_timeout(&spi_imx->dma_rx_completion,
-					      transfer_timeout);
-	if (!timeout) {
+	time_left = wait_for_completion_timeout(&spi_imx->dma_rx_completion,
+						transfer_timeout);
+	if (!time_left) {
 		dev_err(&controller->dev, "I/O Error in DMA RX\n");
 		spi_imx->devtype_data->reset(spi_imx);
 		dmaengine_terminate_all(controller->dma_rx);
@@ -1501,7 +1501,7 @@ static int spi_imx_pio_transfer(struct spi_device *spi,
 {
 	struct spi_imx_data *spi_imx = spi_controller_get_devdata(spi->controller);
 	unsigned long transfer_timeout;
-	unsigned long timeout;
+	unsigned long time_left;
 
 	spi_imx->tx_buf = transfer->tx_buf;
 	spi_imx->rx_buf = transfer->rx_buf;
@@ -1517,9 +1517,9 @@ static int spi_imx_pio_transfer(struct spi_device *spi,
 
 	transfer_timeout = spi_imx_calculate_timeout(spi_imx, transfer->len);
 
-	timeout = wait_for_completion_timeout(&spi_imx->xfer_done,
-					      transfer_timeout);
-	if (!timeout) {
+	time_left = wait_for_completion_timeout(&spi_imx->xfer_done,
+						transfer_timeout);
+	if (!time_left) {
 		dev_err(&spi->dev, "I/O Error in PIO\n");
 		spi_imx->devtype_data->reset(spi_imx);
 		return -ETIMEDOUT;
-- 
2.43.0


_______________________________________________
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 v2 0/8] spi: use 'time_left' instead of 'timeout' with wait_for_*() functions
From: Wolfram Sang @ 2024-04-30 11:41 UTC (permalink / raw)
  To: linux-spi
  Cc: Wolfram Sang, Chen-Yu Tsai, imx, Jernej Skrabec, linux-arm-kernel,
	linux-kernel, linux-sunxi, Mark Brown, Samuel Holland,
	Sascha Hauer, Shawn Guo

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_*() functions causing patterns like:

        timeout = wait_for_completion_timeout(...)
        if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
obvious and self explaining.

This is part of a tree-wide series. The rest of the patches can be found here
(some parts may still be WIP):

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/time_left

Because these patches are generated, I audit them before sending. This is why I
will send series step by step. Build bot is happy with these patches, though.
No functional changes intended.

Changes since v1:
* tags added (thanks!)
* white space issue in sun6i driver fixed
* add maintainers to coverletter

Wolfram Sang (8):
  spi: armada-3700: use 'time_left' variable with
    wait_for_completion_timeout()
  spi: fsl-lpspi: use 'time_left' variable with
    wait_for_completion_timeout()
  spi: imx: use 'time_left' variable with wait_for_completion_timeout()
  spi: pic32-sqi: use 'time_left' variable with
    wait_for_completion_timeout()
  spi: pic32: use 'time_left' variable with
    wait_for_completion_timeout()
  spi: sun4i: use 'time_left' variable with
    wait_for_completion_timeout()
  spi: sun6i: use 'time_left' variable with
    wait_for_completion_timeout()
  spi: xlp: use 'time_left' variable with wait_for_completion_timeout()

 drivers/spi/spi-armada-3700.c |  8 ++++----
 drivers/spi/spi-fsl-lpspi.c   | 14 +++++++-------
 drivers/spi/spi-imx.c         | 20 ++++++++++----------
 drivers/spi/spi-pic32-sqi.c   |  6 +++---
 drivers/spi/spi-pic32.c       |  6 +++---
 drivers/spi/spi-sun4i.c       |  9 +++++----
 drivers/spi/spi-sun6i.c       | 17 +++++++++--------
 drivers/spi/spi-xlp.c         |  8 ++++----
 8 files changed, 45 insertions(+), 43 deletions(-)

-- 
2.43.0


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

^ permalink raw reply

* Re: [PATCH v2 1/3] arm64/mm: Refactor PMD_PRESENT_INVALID and PTE_PROT_NONE bits
From: David Hildenbrand @ 2024-04-30 11:37 UTC (permalink / raw)
  To: Catalin Marinas, Ryan Roberts
  Cc: Will Deacon, Joey Gouly, Ard Biesheuvel, Mark Rutland,
	Anshuman Khandual, Peter Xu, Mike Rapoport, Shivansh Vij,
	linux-arm-kernel, linux-kernel
In-Reply-To: <ZjDR0EIjLr9F2dWn@arm.com>

On 30.04.24 13:11, Catalin Marinas wrote:
> On Mon, Apr 29, 2024 at 06:15:45PM +0100, Ryan Roberts wrote:
>> On 29/04/2024 17:20, Catalin Marinas wrote:
>>> On Mon, Apr 29, 2024 at 03:02:05PM +0100, Ryan Roberts wrote:
>>>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
>>>> index dd9ee67d1d87..de62e6881154 100644
>>>> --- a/arch/arm64/include/asm/pgtable-prot.h
>>>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>>>> @@ -18,14 +18,7 @@
>>>>   #define PTE_DIRTY		(_AT(pteval_t, 1) << 55)
>>>>   #define PTE_SPECIAL		(_AT(pteval_t, 1) << 56)
>>>>   #define PTE_DEVMAP		(_AT(pteval_t, 1) << 57)
>>>> -#define PTE_PROT_NONE		(_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>>>> -
>>>> -/*
>>>> - * This bit indicates that the entry is present i.e. pmd_page()
>>>> - * still points to a valid huge page in memory even if the pmd
>>>> - * has been invalidated.
>>>> - */
>>>> -#define PMD_PRESENT_INVALID	(_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
>>>> +#define PTE_INVALID		(_AT(pteval_t, 1) << 59) /* only when !PTE_VALID */
>>>
>>> Nitpick - I prefer the PTE_PRESENT_INVALID name as it makes it clearer
>>> it's a present pte. We already have PTE_VALID, calling it PTE_INVALID
>>> looks like a negation only.
>>
>> Meh, for me the pte can only be valid or invalid if it is present. So it's
>> implicit. And if you have PTE_PRESENT_INVALID you should also have
>> PTE_PRESENT_VALID.
>>
>> We also have pte_mkinvalid(), which is core-mm-defined. In your scheme, surely
>> it should be pte_mkpresent_invalid()?
>>
>> But you're the boss, I'll change this to PTE_PRESENT_INVALID. :-(
> 
> TBH, I don't have a strong opinion but best to avoid the bikeshedding.
> I'll leave the decision to you ;). It would match the pmd_mkinvalid()
> core code. But if you drop 'present' make sure you add a comment above
> that it's meant for present ptes.

FWIW, I was confused by

present = valid | invalid

Something like

present = present_valid | present_invalid

would be more obvious at least to me ;)

-- 
Cheers,

David / dhildenb


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

^ permalink raw reply

* Re: [PATCH] arm64: dts: mediatek: mt7981: add efuse block
From: AngeloGioacchino Del Regno @ 2024-04-30 11:37 UTC (permalink / raw)
  To: Rafał Miłecki, Matthias Brugger
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	linux-arm-kernel, linux-mediatek, Rafał Miłecki
In-Reply-To: <20240430112932.20475-1-zajec5@gmail.com>

Il 30/04/24 13:29, Rafał Miłecki ha scritto:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> MT7981 (Filogic 820) is a low cost version of MT7986 (Filogic 830) and
> has efuse compatible with the later one.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>   arch/arm64/boot/dts/mediatek/mt7981b.dtsi | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
> index 2d7f91196e64..a5ea168c8fa7 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
> @@ -133,6 +133,13 @@ pio: pinctrl@11d00000 {
>   			#interrupt-cells = <2>;
>   		};
>   
> +		efuse@11f20000 {
> +			compatible = "mediatek,mt7986-efuse", "mediatek,efuse";

I'll be honest here - I've been tempted to do "that" way too many times... :-P

..but no, you have to add a "mediatek,mt7981-efuse" compatible to the binding and
use that instead of the 7986 one (and I'm sure you understand the reasons too...)

Cheers,
Angelo

> +			reg = <0 0x11f20000 0 0x1000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +		};
> +
>   		clock-controller@15000000 {
>   			compatible = "mediatek,mt7981-ethsys", "syscon";
>   			reg = <0 0x15000000 0 0x1000>;



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

^ permalink raw reply

* Re: [PATCH v2 1/3] arm64/mm: Refactor PMD_PRESENT_INVALID and PTE_PROT_NONE bits
From: Ryan Roberts @ 2024-04-30 11:35 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Will Deacon, Joey Gouly, Ard Biesheuvel, Mark Rutland,
	Anshuman Khandual, David Hildenbrand, Peter Xu, Mike Rapoport,
	Shivansh Vij, linux-arm-kernel, linux-kernel
In-Reply-To: <ZjDR0EIjLr9F2dWn@arm.com>

On 30/04/2024 12:11, Catalin Marinas wrote:
> On Mon, Apr 29, 2024 at 06:15:45PM +0100, Ryan Roberts wrote:
>> On 29/04/2024 17:20, Catalin Marinas wrote:
>>> On Mon, Apr 29, 2024 at 03:02:05PM +0100, Ryan Roberts wrote:
>>>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
>>>> index dd9ee67d1d87..de62e6881154 100644
>>>> --- a/arch/arm64/include/asm/pgtable-prot.h
>>>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>>>> @@ -18,14 +18,7 @@
>>>>  #define PTE_DIRTY		(_AT(pteval_t, 1) << 55)
>>>>  #define PTE_SPECIAL		(_AT(pteval_t, 1) << 56)
>>>>  #define PTE_DEVMAP		(_AT(pteval_t, 1) << 57)
>>>> -#define PTE_PROT_NONE		(_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>>>> -
>>>> -/*
>>>> - * This bit indicates that the entry is present i.e. pmd_page()
>>>> - * still points to a valid huge page in memory even if the pmd
>>>> - * has been invalidated.
>>>> - */
>>>> -#define PMD_PRESENT_INVALID	(_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
>>>> +#define PTE_INVALID		(_AT(pteval_t, 1) << 59) /* only when !PTE_VALID */
>>>
>>> Nitpick - I prefer the PTE_PRESENT_INVALID name as it makes it clearer
>>> it's a present pte. We already have PTE_VALID, calling it PTE_INVALID
>>> looks like a negation only.
>>
>> Meh, for me the pte can only be valid or invalid if it is present. So it's
>> implicit. And if you have PTE_PRESENT_INVALID you should also have
>> PTE_PRESENT_VALID.
>>
>> We also have pte_mkinvalid(), which is core-mm-defined. In your scheme, surely
>> it should be pte_mkpresent_invalid()?
>>
>> But you're the boss, I'll change this to PTE_PRESENT_INVALID. :-(
> 
> TBH, I don't have a strong opinion but best to avoid the bikeshedding.
> I'll leave the decision to you ;). It would match the pmd_mkinvalid()
> core code. But if you drop 'present' make sure you add a comment above
> that it's meant for present ptes.

OK, thanks - I'll add a comment and leave it as pte_invalid().

> 
>>>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>>>> index afdd56d26ad7..8dd4637d6b56 100644
>>>> --- a/arch/arm64/include/asm/pgtable.h
>>>> +++ b/arch/arm64/include/asm/pgtable.h
>>>> @@ -105,7 +105,7 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
>>>>  /*
>>>>   * The following only work if pte_present(). Undefined behaviour otherwise.
>>>>   */
>>>> -#define pte_present(pte)	(!!(pte_val(pte) & (PTE_VALID | PTE_PROT_NONE)))
>>>> +#define pte_present(pte)	(pte_valid(pte) || pte_invalid(pte))
>>>>  #define pte_young(pte)		(!!(pte_val(pte) & PTE_AF))
>>>>  #define pte_special(pte)	(!!(pte_val(pte) & PTE_SPECIAL))
>>>>  #define pte_write(pte)		(!!(pte_val(pte) & PTE_WRITE))
>>>> @@ -132,6 +132,7 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
>>>>  #define pte_dirty(pte)		(pte_sw_dirty(pte) || pte_hw_dirty(pte))
>>>>  
>>>>  #define pte_valid(pte)		(!!(pte_val(pte) & PTE_VALID))
>>>> +#define pte_invalid(pte)	((pte_val(pte) & (PTE_VALID | PTE_INVALID)) == PTE_INVALID)
> [...]
>>> I think it's sufficient to check PTE_PRESENT_INVALID only. We'd never
>>> have both bits set, so no need for mask and compare.
>>
>> My rationale is that the INVALID bit may have some other HW meaning when
>> PTE_VALID is set, so its not correct to interpret it as INVALID unless VALID is
>> clear. Granted bit 59 is AttrIndex[3] or PBHA[0], neither of which we are using
>> currently so it will always be 0 when PTE_VALID=1 (and same argument when its
>> moved to NS in next patch). But it feels fragile to me. I'd rather leave it as
>> is unless you insist.
> 
> You are right. It currently works but it may overlap with some hardware
> bit on valid ptes.

OK, I'll leave it as is.

> 
>>>>  /*
>>>>   * Execute-only user mappings do not have the PTE_USER bit set. All valid
>>>>   * kernel mappings have the PTE_UXN bit set.
>>>> @@ -261,6 +262,13 @@ static inline pte_t pte_mkpresent(pte_t pte)
>>>>  	return set_pte_bit(pte, __pgprot(PTE_VALID));
>>>>  }
>>>>  
>>>> +static inline pte_t pte_mkinvalid(pte_t pte)
>>>> +{
>>>> +	pte = set_pte_bit(pte, __pgprot(PTE_INVALID));
>>>> +	pte = clear_pte_bit(pte, __pgprot(PTE_VALID));
>>>> +	return pte;
>>>> +}
>>>
>>> I wonder whether we need to define this. I guess it makes sense than
>>> having the pmd_mkinvalid() use the PTE_* definitions directly, though it
>>> won't be something we need to do on a pte.
>>
>> For me its much cleaner to do it as is because it makes it clear that the format
>> is the same across pte, pmd and pud. And we need pte_invalid() (or
>> pte_present_invalid()) for PROT_NONE so isn't it better to match it with a setter?
> 
> I agree. It was just a remark above.

ACK.

> 
>>>>  static inline bool pmd_user_accessible_page(pmd_t pmd)
>>>>  {
>>>> -	return pmd_leaf(pmd) && !pmd_present_invalid(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
>>>> +	return pmd_valid(pmd) && !pmd_table(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
>>>>  }
>>>
>>> Maybe our pmd_leaf() should actually check valid && !table instead of
>>> present and no need to change these.
>>
>> I'm not sure that would be a great approach; pmd_leaf() is core-mm-defined. And
>> I can't imagine core-mm would want pmd_leaf() to start returning false after
>> calling pmd_mkinvalid(). You probably won't find anywhere where it actually
>> matters right now, but it would be subtly broken and could be exposed in future.
> 
> True, I think it's fine currently but you never know. So after
> pmd_mkinvalid() on a leaf pmd, pmd_leaf() should still return true. It
> might be worth adding a test to pmd_thp_tests() in debug_vm_pgtable.c.

Good idea; I'll add tests for this.

> 
>>>>  static inline bool pud_user_accessible_page(pud_t pud)
>>>>  {
>>>> -	return pud_leaf(pud) && (pud_user(pud) || pud_user_exec(pud));
>>>> +	return pud_valid(pud) && !pud_table(pud) && (pud_user(pud) || pud_user_exec(pud));
>>>>  }
>>>>  #endif
>>>
>>> Same here.
>>>
>>> Otherwise I'm happy with the patch. Feel free to add:
>>>
>>> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> 
> My reviewed-by tag still stands even if you leave the patch as is.

Thanks.

There is still one problem I need to resolve; During this work I discovered that
core-mm can call pmd_mkinvalid() for swap pmds. On arm64 this will turn the swap
pmd into a present pmd, and BadThings can happen in GUP-fast (and any other
lockless SW table walkers). My original fix modified core-mm to only call
pmd_mkinvalid() for present pmds. But discussion over there has shown that arm64
is the only arch that cannot handle this. So I've been convinced that it's
probably more robust to make arm64 handle it gracefully and add tests to
debug_vm_pgtable.c to check for this. Patch incoming shortly, but it will cause
a conflict with this series. So I'll send a v2 of this once that fix is accepted.

> 
> 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 0/3] drm/mediatek: Add support for OF graphs
From: AngeloGioacchino Del Regno @ 2024-04-30 11:33 UTC (permalink / raw)
  To: Alexandre Mergnat, chunkuang.hu
  Cc: robh, krzysztof.kozlowski+dt, conor+dt, p.zabel, airlied, daniel,
	maarten.lankhorst, mripard, tzimmermann, matthias.bgg, shawn.sung,
	yu-chang.lee, ck.hu, jitao.shi, devicetree, linux-kernel,
	dri-devel, linux-mediatek, linux-arm-kernel, wenst, kernel
In-Reply-To: <1fc23530-89ba-4e36-9e9a-a1289f56a9bc@baylibre.com>

Il 30/04/24 12:17, Alexandre Mergnat ha scritto:
> Hi Angelo,
> 
> On 09/04/2024 14:02, AngeloGioacchino Del Regno wrote:
>> This series was tested on MT8195 Cherry Tomato and on MT8395 Radxa
>> NIO-12L with both hardcoded paths, OF graph support and partially
>> hardcoded paths (meaning main display through OF graph and external
>> display hardcoded, because of OVL_ADAPTOR).
> 
> Is that make sense for you to add the DTS changes of these boards into this serie ?
> I asked because, IMHO, that could help to understand the serie.
> 

Yes and no... but I imagine that you're asking this because you're trying to
prepare something with a different SoC+board(s) combination :-)

In that case, I'm preventively sorry because what follows here is not 100%
perfectly tidy yet as I didn't mean to send the devicetree commits upstream
before this series got picked....

... but there you go - I'm sure that you won't mind and that the example will
be more than good enough for you.

Please note that one of the reasons why I didn't want to add this to the series
is that the following changes show only a mere 50% of the reasons why we want OF
graph support on mediatek-drm (but mainly, it's because I didn't have time to
actually rebase etc :-P )


Cheers!
Angelo



  .../boot/dts/mediatek/mt8195-cherry.dtsi      |  65 +++++++-
  arch/arm64/boot/dts/mediatek/mt8195.dtsi      | 153 +++++++++++++++++-
  .../dts/mediatek/mt8395-radxa-nio-12l.dts     |  24 +++
  3 files changed, 231 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
index 86d283ffe807..b05b6bbd457c 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
@@ -267,12 +267,29 @@ &auxadc {
  	status = "okay";
  };

+&dither0_out {
+	remote-endpoint = <&dsc0_in>;
+};
+
  &dp_intf0 {
  	status = "okay";

-	port {
-		dp_intf0_out: endpoint {
-			remote-endpoint = <&edp_in>;
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+			dp_intf0_in: endpoint {
+				remote-endpoint = <&merge0_out>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+			dp_intf0_out: endpoint {
+				remote-endpoint = <&edp_in>;
+			};
  		};
  	};
  };
@@ -287,6 +304,27 @@ dp_intf1_out: endpoint {
  	};
  };

+&dsc0 {
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+			dsc0_in: endpoint {
+				remote-endpoint = <&dither0_out>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+			dsc0_out: endpoint {
+				remote-endpoint = <&merge0_in>;
+			};
+		};
+	};
+};
+
  &edp_tx {
  	status = "okay";

@@ -481,6 +519,27 @@ pmic@34 {
  	};
  };

+&merge0 {
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+			merge0_in: endpoint {
+				remote-endpoint = <&dsc0_out>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+			merge0_out: endpoint {
+				remote-endpoint = <&dp_intf0_in>;
+			};
+		};
+	};
+};
+
  &mfg0 {
  	domain-supply = <&mt6315_7_vbuck1>;
  };
diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 88a0035a31a5..982572d7bfd8 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -3077,14 +3077,6 @@ vencsys_core1: clock-controller@1b000000 {
  			#clock-cells = <1>;
  		};

-		vdosys0: syscon@1c01a000 {
-			compatible = "mediatek,mt8195-vdosys0", "mediatek,mt8195-mmsys", "syscon";
-			reg = <0 0x1c01a000 0 0x1000>;
-			mboxes = <&gce0 0 CMDQ_THR_PRIO_4>;
-			#clock-cells = <1>;
-		};
-
-
  		jpgenc-master {
  			compatible = "mediatek,mt8195-jpgenc";
  			power-domains = <&spm MT8195_POWER_DOMAIN_VENC_CORE1>;
@@ -3143,6 +3135,38 @@ ovl0: ovl@1c000000 {
  			clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
  			iommus = <&iommu_vdo M4U_PORT_L0_DISP_OVL0_RDMA0>;
  			mediatek,gce-client-reg = <&gce0 SUBSYS_1c00XXXX 0x0000 0x1000>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					ovl0_in: endpoint {
+						remote-endpoint = <&vdosys0_ep_main>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					ovl0_out: endpoint {
+						remote-endpoint = <&rdma0_in>;
+					};
+				};
+			};
+		};
+
+		vdosys0: syscon@1c01a000 {
+			compatible = "mediatek,mt8195-vdosys0", "mediatek,mt8195-mmsys", "syscon";
+			reg = <0 0x1c01a000 0 0x1000>;
+			mboxes = <&gce0 0 CMDQ_THR_PRIO_4>;
+			#clock-cells = <1>;
+
+			port {
+				vdosys0_ep_main: endpoint {
+					remote-endpoint = <&ovl0_in>;
+				};
+			};
  		};

  		rdma0: rdma@1c002000 {
@@ -3153,6 +3177,25 @@ rdma0: rdma@1c002000 {
  			clocks = <&vdosys0 CLK_VDO0_DISP_RDMA0>;
  			iommus = <&iommu_vdo M4U_PORT_L0_DISP_RDMA0>;
  			mediatek,gce-client-reg = <&gce0 SUBSYS_1c00XXXX 0x2000 0x1000>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					rdma0_in: endpoint {
+						remote-endpoint = <&ovl0_out>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					rdma0_out: endpoint {
+						remote-endpoint = <&color0_in>;
+					};
+				};
+			};
  		};

  		color0: color@1c003000 {
@@ -3162,6 +3205,25 @@ color0: color@1c003000 {
  			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
  			clocks = <&vdosys0 CLK_VDO0_DISP_COLOR0>;
  			mediatek,gce-client-reg = <&gce0 SUBSYS_1c00XXXX 0x3000 0x1000>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					color0_in: endpoint {
+						remote-endpoint = <&rdma0_out>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					color0_out: endpoint {
+						remote-endpoint = <&ccorr0_in>;
+					};
+				};
+			};
  		};

  		ccorr0: ccorr@1c004000 {
@@ -3171,6 +3233,25 @@ ccorr0: ccorr@1c004000 {
  			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
  			clocks = <&vdosys0 CLK_VDO0_DISP_CCORR0>;
  			mediatek,gce-client-reg = <&gce0 SUBSYS_1c00XXXX 0x4000 0x1000>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					ccorr0_in: endpoint {
+						remote-endpoint = <&color0_out>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					ccorr0_out: endpoint {
+						remote-endpoint = <&aal0_in>;
+					};
+				};
+			};
  		};

  		aal0: aal@1c005000 {
@@ -3180,6 +3261,25 @@ aal0: aal@1c005000 {
  			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
  			clocks = <&vdosys0 CLK_VDO0_DISP_AAL0>;
  			mediatek,gce-client-reg = <&gce0 SUBSYS_1c00XXXX 0x5000 0x1000>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					aal0_in: endpoint {
+						remote-endpoint = <&ccorr0_out>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					aal0_out: endpoint {
+						remote-endpoint = <&gamma0_in>;
+					};
+				};
+			};
  		};

  		gamma0: gamma@1c006000 {
@@ -3189,6 +3289,25 @@ gamma0: gamma@1c006000 {
  			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
  			clocks = <&vdosys0 CLK_VDO0_DISP_GAMMA0>;
  			mediatek,gce-client-reg = <&gce0 SUBSYS_1c00XXXX 0x6000 0x1000>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					gamma0_in: endpoint {
+						remote-endpoint = <&aal0_out>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					gamma0_out: endpoint {
+						remote-endpoint = <&dither0_in>;
+					};
+				};
+			};
  		};

  		dither0: dither@1c007000 {
@@ -3198,6 +3317,24 @@ dither0: dither@1c007000 {
  			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
  			clocks = <&vdosys0 CLK_VDO0_DISP_DITHER0>;
  			mediatek,gce-client-reg = <&gce0 SUBSYS_1c00XXXX 0x7000 0x1000>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					dither0_in: endpoint {
+						remote-endpoint = <&gamma0_out>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					dither0_out: endpoint {
+					};
+				};
+			};
  		};

  		dsi0: dsi@1c008000 {
diff --git a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts 
b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
index 7ab19b4e046f..b4a7dad2fea7 100644
--- a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
@@ -1063,6 +1063,10 @@ &disp_pwm0 {
  	status = "okay";
  };

+&dither0_out {
+	remote-endpoint = <&dsi0_in>;
+};
+
  &dsi0 {
  	status = "okay";
  	#address-cells = <1>;
@@ -1089,6 +1093,25 @@ dsi_panel_in: endpoint {
  		};
  	};

+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+			dsi0_in: endpoint {
+				remote-endpoint = <&dither0_out>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+			dsi_out: endpoint {
+				remote-endpoint = <&dsi_panel_in>;
+			};
+		};
+	};
+/* old hardcoded dsi stuff, disappear!
  	ports {
  		port {
  			dsi_out: endpoint {
@@ -1096,6 +1119,7 @@ dsi_out: endpoint {
  			};
  		};
  	};
+*/

  };

-- 
2.45.0


_______________________________________________
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] arm64: dts: mediatek: mt7981: add efuse block
From: Rafał Miłecki @ 2024-04-30 11:29 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno, Matthias Brugger
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	linux-arm-kernel, linux-mediatek, Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.pl>

MT7981 (Filogic 820) is a low cost version of MT7986 (Filogic 830) and
has efuse compatible with the later one.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/arm64/boot/dts/mediatek/mt7981b.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
index 2d7f91196e64..a5ea168c8fa7 100644
--- a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
@@ -133,6 +133,13 @@ pio: pinctrl@11d00000 {
 			#interrupt-cells = <2>;
 		};
 
+		efuse@11f20000 {
+			compatible = "mediatek,mt7986-efuse", "mediatek,efuse";
+			reg = <0 0x11f20000 0 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+		};
+
 		clock-controller@15000000 {
 			compatible = "mediatek,mt7981-ethsys", "syscon";
 			reg = <0 0x15000000 0 0x1000>;
-- 
2.35.3


_______________________________________________
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 v6 4/5] sched: Rename arch_update_thermal_pressure into arch_update_hw_pressure
From: Konrad Dybcio @ 2024-04-30 11:23 UTC (permalink / raw)
  To: Vincent Guittot, linux, catalin.marinas, will, sudeep.holla,
	rafael, viresh.kumar, agross, andersson, mingo, peterz,
	juri.lelli, dietmar.eggemann, rostedt, bsegall, mgorman, bristot,
	vschneid, lukasz.luba, rui.zhang, mhiramat, daniel.lezcano,
	amit.kachhap, corbet, gregkh, linux-arm-kernel, linux-kernel,
	linux-pm, linux-arm-msm, linux-trace-kernel, linux-doc
  Cc: Qais Yousef, Bjorn Andersson, Dmitry Baryshkov
In-Reply-To: <20240326091616.3696851-5-vincent.guittot@linaro.org>

On 26.03.2024 10:16 AM, Vincent Guittot wrote:
> Now that cpufreq provides a pressure value to the scheduler, rename
> arch_update_thermal_pressure into HW pressure to reflect that it returns
> a pressure applied by HW (i.e. with a high frequency change) and not
> always related to thermal mitigation but also generated by max current
> limitation as an example. Such high frequency signal needs filtering to be
> smoothed and provide an value that reflects the average available capacity
> into the scheduler time scale.
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> Reviewed-by: Qais Yousef <qyousef@layalina.io>
> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
> Tested-by: Lukasz Luba <lukasz.luba@arm.com>
> ---

Hi, I'm not quite sure how, but this commit specifically breaks booting
on Qualcomm platforms with EAS..

https://pastebin.com/raw/1Uh7u81x

Konrad

_______________________________________________
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: Properly clean up iommu-dma remnants
From: Catalin Marinas @ 2024-04-30 11:22 UTC (permalink / raw)
  To: Robin Murphy; +Cc: joro, will, linux-arm-kernel, iommu, Dmitry Baryshkov
In-Reply-To: <d4cc20cbb0c45175e98dd76bf187e2ad6421296d.1714472573.git.robin.murphy@arm.com>

On Tue, Apr 30, 2024 at 11:22:53AM +0100, Robin Murphy wrote:
> Thanks to the somewhat asymmetrical nature, while removing
> iommu_setup_dma_ops() from the arch_setup_dma_ops() flow, I managed to
> forget that arm64's teardown path was also specific to iommu-dma. Clean
> that up to match, otherwise probe deferral will lead to the arch code
> erroneously removing DMA ops set elsewhere.
> 
> Reported-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Link: https://lore.kernel.org/linux-iommu/Zi_LV28TR-P-PzXi@eriador.lumag.spb.ru/
> Fixes: b67483b3c44e ("iommu/dma: Centralise iommu_setup_dma_ops()")
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

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

^ permalink raw reply

* Re: [PATCH v2 1/3] arm64/mm: Refactor PMD_PRESENT_INVALID and PTE_PROT_NONE bits
From: Catalin Marinas @ 2024-04-30 11:11 UTC (permalink / raw)
  To: Ryan Roberts
  Cc: Will Deacon, Joey Gouly, Ard Biesheuvel, Mark Rutland,
	Anshuman Khandual, David Hildenbrand, Peter Xu, Mike Rapoport,
	Shivansh Vij, linux-arm-kernel, linux-kernel
In-Reply-To: <839d6975-ce12-4fc9-aa3b-8ec5787bf577@arm.com>

On Mon, Apr 29, 2024 at 06:15:45PM +0100, Ryan Roberts wrote:
> On 29/04/2024 17:20, Catalin Marinas wrote:
> > On Mon, Apr 29, 2024 at 03:02:05PM +0100, Ryan Roberts wrote:
> >> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
> >> index dd9ee67d1d87..de62e6881154 100644
> >> --- a/arch/arm64/include/asm/pgtable-prot.h
> >> +++ b/arch/arm64/include/asm/pgtable-prot.h
> >> @@ -18,14 +18,7 @@
> >>  #define PTE_DIRTY		(_AT(pteval_t, 1) << 55)
> >>  #define PTE_SPECIAL		(_AT(pteval_t, 1) << 56)
> >>  #define PTE_DEVMAP		(_AT(pteval_t, 1) << 57)
> >> -#define PTE_PROT_NONE		(_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
> >> -
> >> -/*
> >> - * This bit indicates that the entry is present i.e. pmd_page()
> >> - * still points to a valid huge page in memory even if the pmd
> >> - * has been invalidated.
> >> - */
> >> -#define PMD_PRESENT_INVALID	(_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
> >> +#define PTE_INVALID		(_AT(pteval_t, 1) << 59) /* only when !PTE_VALID */
> > 
> > Nitpick - I prefer the PTE_PRESENT_INVALID name as it makes it clearer
> > it's a present pte. We already have PTE_VALID, calling it PTE_INVALID
> > looks like a negation only.
> 
> Meh, for me the pte can only be valid or invalid if it is present. So it's
> implicit. And if you have PTE_PRESENT_INVALID you should also have
> PTE_PRESENT_VALID.
> 
> We also have pte_mkinvalid(), which is core-mm-defined. In your scheme, surely
> it should be pte_mkpresent_invalid()?
> 
> But you're the boss, I'll change this to PTE_PRESENT_INVALID. :-(

TBH, I don't have a strong opinion but best to avoid the bikeshedding.
I'll leave the decision to you ;). It would match the pmd_mkinvalid()
core code. But if you drop 'present' make sure you add a comment above
that it's meant for present ptes.

> >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> >> index afdd56d26ad7..8dd4637d6b56 100644
> >> --- a/arch/arm64/include/asm/pgtable.h
> >> +++ b/arch/arm64/include/asm/pgtable.h
> >> @@ -105,7 +105,7 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
> >>  /*
> >>   * The following only work if pte_present(). Undefined behaviour otherwise.
> >>   */
> >> -#define pte_present(pte)	(!!(pte_val(pte) & (PTE_VALID | PTE_PROT_NONE)))
> >> +#define pte_present(pte)	(pte_valid(pte) || pte_invalid(pte))
> >>  #define pte_young(pte)		(!!(pte_val(pte) & PTE_AF))
> >>  #define pte_special(pte)	(!!(pte_val(pte) & PTE_SPECIAL))
> >>  #define pte_write(pte)		(!!(pte_val(pte) & PTE_WRITE))
> >> @@ -132,6 +132,7 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
> >>  #define pte_dirty(pte)		(pte_sw_dirty(pte) || pte_hw_dirty(pte))
> >>  
> >>  #define pte_valid(pte)		(!!(pte_val(pte) & PTE_VALID))
> >> +#define pte_invalid(pte)	((pte_val(pte) & (PTE_VALID | PTE_INVALID)) == PTE_INVALID)
[...]
> > I think it's sufficient to check PTE_PRESENT_INVALID only. We'd never
> > have both bits set, so no need for mask and compare.
> 
> My rationale is that the INVALID bit may have some other HW meaning when
> PTE_VALID is set, so its not correct to interpret it as INVALID unless VALID is
> clear. Granted bit 59 is AttrIndex[3] or PBHA[0], neither of which we are using
> currently so it will always be 0 when PTE_VALID=1 (and same argument when its
> moved to NS in next patch). But it feels fragile to me. I'd rather leave it as
> is unless you insist.

You are right. It currently works but it may overlap with some hardware
bit on valid ptes.

> >>  /*
> >>   * Execute-only user mappings do not have the PTE_USER bit set. All valid
> >>   * kernel mappings have the PTE_UXN bit set.
> >> @@ -261,6 +262,13 @@ static inline pte_t pte_mkpresent(pte_t pte)
> >>  	return set_pte_bit(pte, __pgprot(PTE_VALID));
> >>  }
> >>  
> >> +static inline pte_t pte_mkinvalid(pte_t pte)
> >> +{
> >> +	pte = set_pte_bit(pte, __pgprot(PTE_INVALID));
> >> +	pte = clear_pte_bit(pte, __pgprot(PTE_VALID));
> >> +	return pte;
> >> +}
> > 
> > I wonder whether we need to define this. I guess it makes sense than
> > having the pmd_mkinvalid() use the PTE_* definitions directly, though it
> > won't be something we need to do on a pte.
> 
> For me its much cleaner to do it as is because it makes it clear that the format
> is the same across pte, pmd and pud. And we need pte_invalid() (or
> pte_present_invalid()) for PROT_NONE so isn't it better to match it with a setter?

I agree. It was just a remark above.

> >>  static inline bool pmd_user_accessible_page(pmd_t pmd)
> >>  {
> >> -	return pmd_leaf(pmd) && !pmd_present_invalid(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> >> +	return pmd_valid(pmd) && !pmd_table(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> >>  }
> > 
> > Maybe our pmd_leaf() should actually check valid && !table instead of
> > present and no need to change these.
> 
> I'm not sure that would be a great approach; pmd_leaf() is core-mm-defined. And
> I can't imagine core-mm would want pmd_leaf() to start returning false after
> calling pmd_mkinvalid(). You probably won't find anywhere where it actually
> matters right now, but it would be subtly broken and could be exposed in future.

True, I think it's fine currently but you never know. So after
pmd_mkinvalid() on a leaf pmd, pmd_leaf() should still return true. It
might be worth adding a test to pmd_thp_tests() in debug_vm_pgtable.c.

> >>  static inline bool pud_user_accessible_page(pud_t pud)
> >>  {
> >> -	return pud_leaf(pud) && (pud_user(pud) || pud_user_exec(pud));
> >> +	return pud_valid(pud) && !pud_table(pud) && (pud_user(pud) || pud_user_exec(pud));
> >>  }
> >>  #endif
> > 
> > Same here.
> > 
> > Otherwise I'm happy with the patch. Feel free to add:
> > 
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

My reviewed-by tag still stands even if you leave the patch as is.

Thanks.

-- 
Catalin

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

^ permalink raw reply

* Re: [PATCH] arm64: dts: allwinner: Add cache information to the SoC dtsi for H6
From: Dragan Simic @ 2024-04-30 11:10 UTC (permalink / raw)
  To: Andre Przywara
  Cc: linux-sunxi, wens, jernej.skrabec, samuel, linux-arm-kernel,
	devicetree, robh, krzk+dt, conor+dt, linux-kernel
In-Reply-To: <20240430114627.0cfcd14a@donnerap.manchester.arm.com>

Hello Andre,

On 2024-04-30 12:46, Andre Przywara wrote:
> On Tue, 30 Apr 2024 02:01:42 +0200
> Dragan Simic <dsimic@manjaro.org> wrote:
>> On 2024-04-30 01:10, Andre Przywara wrote:
>> > On Sun, 28 Apr 2024 13:40:36 +0200
>> > Dragan Simic <dsimic@manjaro.org> wrote:
>> >
>> >> Add missing cache information to the Allwinner H6 SoC dtsi, to allow
>> >> the userspace, which includes lscpu(1) that uses the virtual files
>> >> provided
>> >> by the kernel under the /sys/devices/system/cpu directory, to display
>> >> the
>> >> proper H6 cache information.
>> >>
>> >> Adding the cache information to the H6 SoC dtsi also makes the
>> >> following
>> >> warning message in the kernel log go away:
>> >>
>> >>   cacheinfo: Unable to detect cache hierarchy for CPU 0
>> >>
>> >> The cache parameters for the H6 dtsi were obtained and partially
>> >> derived
>> >> by hand from the cache size and layout specifications found in the
>> >> following
>> >> datasheets and technical reference manuals:
>> >>
>> >>   - Allwinner H6 V200 datasheet, version 1.1
>> >>   - ARM Cortex-A53 revision r0p3 TRM, version E
>> >>
>> >> For future reference, here's a brief summary of the documentation:
>> >>
>> >>   - All caches employ the 64-byte cache line length
>> >>   - Each Cortex-A53 core has 32 KB of L1 2-way, set-associative
>> >> instruction
>> >>     cache and 32 KB of L1 4-way, set-associative data cache
>> >>   - The entire SoC has 512 KB of unified L2 16-way, set-associative
>> >> cache
>> >>
>> >> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> >
>> > I can confirm that the data below matches the manuals, but also the
>> > decoding of the architectural cache type registers (CCSIDR_EL1):
>> >   L1D: 32 KB: 128 sets, 4 way associative, 64 bytes/line
>> >   L1I: 32 KB: 256 sets, 2 way associative, 64 bytes/line
>> >   L2: 512 KB: 512 sets, 16 way associative, 64 bytes/line
>> 
>> Thank you very much for reviewing my patch in such a detailed way!
>> It's good to know that the values in the Allwinner datasheets match
>> with the observed reality, so to speak. :)
> 
> YW, and yes, I like to double check things when it comes to Allwinner
> documentation ;-) And it was comparably easy for this problem.

Double checking is always good, IMHO. :)

> Out of curiosity: what triggered that patch? Trying to get rid of false
> warning/error messages?

Yes, one of the motivators was to get rid of the false kernel warning,
and the other was to have the cache information nicely available through
lscpu(1).  I already did the same for a few Rockchip SoCs, [1][2][3] so
a couple of Allwinner SoCs were the next on my mental TODO list. :)

> And do you plan to address the H616 as well? It's a bit more tricky 
> there,
> since there are two die revisions out: one with 256(?)KB of L2, one 
> with
> 1MB(!). We know how to tell them apart, so I could provide some TF-A 
> code
> to patch that up in the DT. The kernel DT copy could go with 256KB 
> then.

I have no boards based on the Allwinner H616, so it wasn't on my radar.
Though, I'd be happy to prepare and submit a similar kernel patch for
the H616, if you'd then take it further and submit a TF-A patch that
fixes the DT according to the detected die revision?  Did I understand
the plan right?

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=67a6a98575974416834c2294853b3814376a7ce7
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=8612169a05c5e979af033868b7a9b177e0f9fcdf
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=b72633ba5cfa932405832de25d0f0a11716903b4

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

^ permalink raw reply


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