Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
From: Klymenko, Anatoliy @ 2024-03-22 19:12 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Laurent Pinchart, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
	Simek, Michal, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Jonas Karlman, Jernej Skrabec, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Mauro Carvalho Chehab
  Cc: Tomi Valkeinen, dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-media@vger.kernel.org
In-Reply-To: <a82d525c-737a-4ac4-9d71-e88f4ba69ea1@linaro.org>

Hi Krzysztof,

Thanks a lot for the review.

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Thursday, March 21, 2024 10:59 PM
> To: Klymenko, Anatoliy <Anatoliy.Klymenko@amd.com>; Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>;
> Thomas Zimmermann <tzimmermann@suse.de>; David Airlie
> <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Simek, Michal
> <michal.simek@amd.com>; Andrzej Hajda <andrzej.hajda@intel.com>; Neil
> Armstrong <neil.armstrong@linaro.org>; Robert Foss <rfoss@kernel.org>; Jonas
> Karlman <jonas@kwiboo.se>; Jernej Skrabec <jernej.skrabec@gmail.com>; Rob
> Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>;
> Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>; dri-
> devel@lists.freedesktop.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-
> media@vger.kernel.org
> Subject: Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> > diff --git a/include/dt-bindings/media/media-bus-format.h b/include/dt-
> bindings/media/media-bus-format.h
> > new file mode 100644
> > index 000000000000..60fc6e11dabc
> > --- /dev/null
> > +++ b/include/dt-bindings/media/media-bus-format.h
> > @@ -0,0 +1,177 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> > +/*
> > + * Media Bus API header
> > + *
> > + * Copyright (C) 2009, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> 
> That's not true. Your SPDX tells something entirely different.
> 

Thank you - I'll see how to fix it.

> Anyway, you did not explain why you need to copy anything anywhere.
> 
> Specifically, random hex values *are not bindings*.
> 

The same media bus format values are being used by the reference driver in patch #9. And, as far as I know, we cannot use headers from Linux API headers directly (at least I noticed the same pattern in ../dt-bindings/sdtv-standarts.h for instance). What would be the best approach to reusing the same defines on DT and driver sides from your point of view? Symlink maybe?

> Best regards,
> Krzysztof

Thank you,
Anatoliy
_______________________________________________
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 v5 08/27] iommu/arm-smmu-v3: Move allocation of the cdtable into arm_smmu_get_cd_ptr()
From: Mostafa Saleh @ 2024-03-22 19:07 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
	Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
	Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <8-v5-9a37e0c884ce+31e3-smmuv3_newapi_p2_jgg@nvidia.com>

Hi Jason,

On Mon, Mar 04, 2024 at 07:43:56PM -0400, Jason Gunthorpe wrote:
> No reason to force callers to do two steps. Make arm_smmu_get_cd_ptr()
> able to return an entry in all cases except OOM

I believe the current code is more clear, as it is explicit about which path
is expected to allocate.

As there are many callers for arm_smmu_get_cd_ptr() directly and indirectly,
and it read-modify-writes the cdtable, it would be a pain to debug not
knowing which one could allocate, and this patch only abstracts one
allocating call, so it is not much code less.

For example, (again I don’t know much about SVA) I think there might be a
race condition as follows:
arm_smmu_attach_dev
	arm_smmu_domain_finalise() => set domain stage
	[....]
	arm_smmu_get_cd_ptr() => RMW master->cd_table

arm_smmu_sva_set_dev_pasid
	__arm_smmu_sva_bind
		Check stage is valid
		[...]
		arm_smmu_write_ctx_desc
			arm_smmu_get_cd_ptr => RMW master->cd_table

If this path is true though, I guess the in current code, we would need some
barriers in arm_smmu_get_cd_ptr(), arm_smmu_get_cd_ptr()

> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index e25dbb982feeee..2dd6cb17112e98 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -106,6 +106,7 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
>  
>  static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
>  				    struct arm_smmu_device *smmu);
> +static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master);
>  
>  static void parse_driver_options(struct arm_smmu_device *smmu)
>  {
> @@ -1231,6 +1232,11 @@ struct arm_smmu_cd *arm_smmu_get_cd_ptr(struct arm_smmu_master *master,
>  	struct arm_smmu_device *smmu = master->smmu;
>  	struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table;
>  
> +	if (!master->cd_table.cdtab) {
> +		if (arm_smmu_alloc_cd_tables(master))
> +			return NULL;
> +	}
> +
>  	if (cd_table->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
>  		return (struct arm_smmu_cd *)(cd_table->cdtab +
>  					      ssid * CTXDESC_CD_DWORDS);
> @@ -2719,12 +2725,6 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>  		struct arm_smmu_cd target_cd;
>  		struct arm_smmu_cd *cdptr;
>  
> -		if (!master->cd_table.cdtab) {
> -			ret = arm_smmu_alloc_cd_tables(master);
> -			if (ret)
> -				goto out_list_del;
> -		}
> -
>  		cdptr = arm_smmu_get_cd_ptr(master, IOMMU_NO_PASID);
>  		if (!cdptr) {
>  			ret = -ENOMEM;
> -- 
> 2.43.2
>
Thanks,
Mostafa

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

^ permalink raw reply

* Re: [PATCH RFC v4 02/15] ACPI: processor: Register all CPUs from acpi_processor_get_info()
From: Jonathan Cameron @ 2024-03-22 18:53 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Russell King, linux-pm, loongarch, linux-acpi, linux-arch,
	linux-kernel, linux-arm-kernel, linux-riscv, kvmarm, x86,
	acpica-devel, linux-csky, linux-doc, linux-ia64, linux-parisc,
	Salil Mehta, Jean-Philippe Brucker, jianyong.wu, justin.he,
	James Morse
In-Reply-To: <CAJZ5v0iiJpUWq5GMSnKFWQTzn_bdwoQz9m=hDaXNg4Lj_ePF4g@mail.gmail.com>

On Thu, 15 Feb 2024 20:22:29 +0100
"Rafael J. Wysocki" <rafael@kernel.org> wrote:

> On Wed, Jan 31, 2024 at 5:50 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > From: James Morse <james.morse@arm.com>
> >
> > To allow ACPI to skip the call to arch_register_cpu() when the _STA
> > value indicates the CPU can't be brought online right now, move the
> > arch_register_cpu() call into acpi_processor_get_info().
> >
> > Systems can still be booted with 'acpi=off', or not include an
> > ACPI description at all. For these, the CPUs continue to be
> > registered by cpu_dev_register_generic().
> >
> > This moves the CPU register logic back to a subsys_initcall(),
> > while the memory nodes will have been registered earlier.
> >
> > Signed-off-by: James Morse <james.morse@arm.com>
> > Reviewed-by: Gavin Shan <gshan@redhat.com>
> > Tested-by: Miguel Luis <miguel.luis@oracle.com>
> > Tested-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
> > Tested-by: Jianyong Wu <jianyong.wu@arm.com>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> > Changes since RFC v2:
> >  * Fixup comment in acpi_processor_get_info() (Gavin Shan)
> >  * Add comment in cpu_dev_register_generic() (Gavin Shan)
> > ---
> >  drivers/acpi/acpi_processor.c | 12 ++++++++++++
> >  drivers/base/cpu.c            |  6 +++++-
> >  2 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> > index cf7c1cca69dd..a68c475cdea5 100644
> > --- a/drivers/acpi/acpi_processor.c
> > +++ b/drivers/acpi/acpi_processor.c
> > @@ -314,6 +314,18 @@ static int acpi_processor_get_info(struct acpi_device *device)
> >                         cpufreq_add_device("acpi-cpufreq");
> >         }
> >
> > +       /*
> > +        * Register CPUs that are present. get_cpu_device() is used to skip
> > +        * duplicate CPU descriptions from firmware.
> > +        */
> > +       if (!invalid_logical_cpuid(pr->id) && cpu_present(pr->id) &&
> > +           !get_cpu_device(pr->id)) {
> > +               int ret = arch_register_cpu(pr->id);
> > +
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> >         /*
> >          *  Extra Processor objects may be enumerated on MP systems with
> >          *  less than the max # of CPUs. They should be ignored _iff  
> 
> This is interesting, because right below there is the following code:
> 
>     if (invalid_logical_cpuid(pr->id) || !cpu_present(pr->id)) {
>         int ret = acpi_processor_hotadd_init(pr);
> 
>         if (ret)
>             return ret;
>     }
> 
> and acpi_processor_hotadd_init() essentially calls arch_register_cpu()
> with some extra things around it (more about that below).
> 
> I do realize that acpi_processor_hotadd_init() is defined under
> CONFIG_ACPI_HOTPLUG_CPU, so for the sake of the argument let's
> consider an architecture where CONFIG_ACPI_HOTPLUG_CPU is set.
> 
> So why are the two conditionals that almost contradict each other both
> needed?  It looks like the new code could be combined with
> acpi_processor_hotadd_init() to do the right thing in all cases.

I jumped on to the end of this series to look at this as the two legs
look more similar at that point. I'll figure out how to drive
any changes through the series once the end goal is clear.

To make testing easy I made the acpi_process_make_enabled() look as
much like acpi_process_make_present() as possible.

> 
> Now, acpi_processor_hotadd_init() does some extra things that look
> like they should be done by the new code too.
> 
> 1. It checks invalid_phys_cpuid() which appears to be a good idea to me.

Indeed that is sensible. Not sure there is a path to here where it fails,
but defense in depth is good.

> 
> 2. It uses locking around arch_register_cpu() which doesn't seem
> unreasonable either.

Seems reasonable, though exactly what this protecting is unclear to me
- is the arch_register_cpu() and/or the acpi_map_cpu().
Whilst it would be nice to be sure, appears harmless, so let us
take it for consistency if nothing else.

The cpu_maps_update_begin()/end() calls though aren't necessary as
we aren't touching the cpu_present or cpu_online masks.


> 
> 3. It calls acpi_map_cpu() and I'm not sure why this is not done by
> the new code.

Doesn't exist except on x86 and longarch as Russell mentioned. So let's
see what it does (on x86)  So we are into the realm of interfaces that
look generic but really aren't :(  I particularly like the
generic_processor_info() which isn't particularly generic.

1. cpu = acpi_register_lapic()

Docs say: Register a local apic and generates a logic cpu number

2. generic_processor_info() in arch/x86/kernel/acpi/acpi.c

Checks against nr_cpus_ids - maybe that bit is useful

Allocate_logical_cpuid().
Digging in, it seems to do similar to setting __cpu_logical_map on arm64.
That's done in acpi_map_gic_cpu_interface, which happens when MADT is
parsed and I believe it's one of the the things we need to do whether
or not the CPU is enabled at boot. So already done.

acpi_processor_set_pdc() -- configure _PDC support (which I'd never heard
of before now).  Deprecated in ACPI 3.0. Given we are using stuff only added
in 6.5 we can probably skip that even if it would be harmless.

acpi_map_cpu2node() -- evalulate _PXM and set __apicid_to_node[]
entry. That is only used from x86 code. Not sure what equivalent would be.
Also numa_set_node(cpu, nid);  Which again sounds a lot more generic than
it is. Load of x86 specific stuff + set_cpu_numa_node() which is generic
and for ARM64 (and anything using CONFIG_GENERIC_ARCH_NUMA) is called
by numa_store_cpu_info() either from early_map_cpu_to_node() or smp_prepare_cpus()
which is called for_each_possible_cpu() and hence has already been done.

So conclusion on this one is there doesn't seem to be anything to do.
We could provide a __weak function or an ARM64 specific one that does
nothing or gate it on an appropriate config variable.  However, given
I presume 'future' ARM64 support for CPU hotplug will want to do something
in these calls, perhaps a better bet is to pass a bool into the function
to indicate these should be skipped if present is not changing.

Having done that, we end up with code that is messy enough we are
better off keeping them as separate functions, though they may
look a little more similar than in this version.

There is a final thing in here you didn't mention
setting pr->flags.need_hotplug_init
which causes extra stuff to occur in processor_driver.c
The extra stuff doesn't seem to be necessary for the enable case
despite being needed for change of present status.
I haven't figured this bit out yet (I need to mess around on x86
to understand what goes wrong if you don't use that flag).


> 
> The only thing that can be dropped from it is the _STA check AFAICS,
> because acpi_processor_add() won't even be called if the CPU is not
> present (and not enabled after the first patch).
> 
> So why does the code not do 1 - 3 above?
I agree with 1 and 2, reasoning for 3 given above.

> 
> > diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> > index 47de0f140ba6..13d052bf13f4 100644
> > --- a/drivers/base/cpu.c
> > +++ b/drivers/base/cpu.c
> > @@ -553,7 +553,11 @@ static void __init cpu_dev_register_generic(void)
> >  {
> >         int i, ret;
> >
> > -       if (!IS_ENABLED(CONFIG_GENERIC_CPU_DEVICES))
> > +       /*
> > +        * When ACPI is enabled, CPUs are registered via
> > +        * acpi_processor_get_info().
> > +        */
> > +       if (!IS_ENABLED(CONFIG_GENERIC_CPU_DEVICES) || !acpi_disabled)
> >                 return;  
> 
> Honestly, this looks like a quick hack to me and it absolutely
> requires an ACK from the x86 maintainers to go anywhere.
Will address this separately.

> 
> >
> >         for_each_present_cpu(i) {
> > --  


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

^ permalink raw reply

* Re: [PATCH v4 1/5] dt-bindings: misc: Add mikrobus-connector
From: Andrew Lunn @ 2024-03-22 18:51 UTC (permalink / raw)
  To: Ayush Singh
  Cc: open list, jkridner, robertcnelson, lorforlinux, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, Derek Kiernan, Dragan Cvetic,
	Arnd Bergmann, Greg Kroah-Hartman, Vaishnav M A, Mark Brown,
	Johan Hovold, Alex Elder,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/TEXAS INSTRUMENTS K3 ARCHITECTURE,
	open list:SPI SUBSYSTEM, moderated list:GREYBUS SUBSYSTEM,
	Vaishnav M A, Michael Walle
In-Reply-To: <711ff5ea-244d-4b64-a39c-3f2da63e30c0@gmail.com>

> After going through all the discussions here, I have a few questions:
> 
> 1. Is the old `*_register_device(controller, board_info)` style discouraged
> in favor of using device tree, at least for drivers using multiple
> fundamental buses (i2c, spi, etc)?

Historically, they were used in board files, where you needed to write
C code for every single board. That did not scale, which is why we
swapped to DT.

board_info is still useful, e.g. for platforms which don't have DT. I
support a few amd64 boards where i need to use a platform driver to
instantiate some I2C and MDIO devices. But in general DT is much
easier to use.

> 2. Is the preferred way to handle virtual devices (like those created by
> greybus subsystem) now device tree? Is that one of the blockers for greybus
> i2c, spi etc to still be in staging?

I would not say they are virtual. They do exist. They are just not
memory mapped like most devices, but in another address space, one
which you access via RPCs.

> 
> 3. How are virtual devices created in device tree? If I register an i2c
> adapter using `i2c_add_adapter`, is the device tree entry is dynamically
> created, which can then be used by a device tree overlay?

As far as i'm aware, there are no examples today. You are doing
something different, something new. Adding these dynamic devices to DT
is just a suggestion from me, as a good way to solve your problem. You
will need to look into the DT core and figure out how to do it.

     Andrew

_______________________________________________
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 v5 06/27] iommu/arm-smmu-v3: Consolidate clearing a CD table entry
From: Mostafa Saleh @ 2024-03-22 18:36 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
	Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
	Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <6-v5-9a37e0c884ce+31e3-smmuv3_newapi_p2_jgg@nvidia.com>

Hi Jason,

On Mon, Mar 04, 2024 at 07:43:54PM -0400, Jason Gunthorpe wrote:
> A cleared entry is all 0's. Make arm_smmu_clear_cd() do this sequence.
> 
> If we are clearing an entry and for some reason it is not already
> allocated in the CD table then something has gone wrong.
> 
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  2 +-
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 20 ++++++++++++++-----
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  2 ++
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> index 347c2fdd865c1a..bb9bb6fd7914ce 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> @@ -558,7 +558,7 @@ void arm_smmu_sva_remove_dev_pasid(struct iommu_domain *domain,
>  
>  	mutex_lock(&sva_lock);
>  
> -	arm_smmu_write_ctx_desc(master, id, NULL);
> +	arm_smmu_clear_cd(master, id);
>  
>  	list_for_each_entry(t, &master->bonds, list) {
>  		if (t->mm == mm) {
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 237fd6d92c880b..3fb4a1523d1d3f 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1303,6 +1303,19 @@ static void arm_smmu_write_cd_entry(struct arm_smmu_master *master, int ssid,
>  	arm_smmu_write_entry(&cd_writer.writer, cdptr->data, target->data);
>  }
>  
> +void arm_smmu_clear_cd(struct arm_smmu_master *master, ioasid_t ssid)
> +{
> +	struct arm_smmu_cd target = {};
> +	struct arm_smmu_cd *cdptr;
> +
> +	if (!master->cd_table.cdtab)
> +		return;
> +	cdptr = arm_smmu_get_cd_ptr(master, ssid);
> +	if (WARN_ON(!cdptr))
> +		return;

I don’t understand the SVA code enough, but AFAICT, arm_smmu_sva_set_dev_pasid
can allocate the L2 CD table through arm_smmu_write_ctx_desc. And if it failed
before allocating the CD table, then remove_dev_pasid would be called, which
warns here, the previous code would tolerate that, but that might regress on
systems with panic_on_warn, so I am not sure if that is necessary.

Otherwise, Reviewed-by: Mostafa Saleh <smostafa@google.com>

> +	arm_smmu_write_cd_entry(master, ssid, cdptr, &target);
> +}
> +
>  int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
>  			    struct arm_smmu_ctx_desc *cd)
>  {
> @@ -2702,9 +2715,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>  	case ARM_SMMU_DOMAIN_S2:
>  		arm_smmu_make_s2_domain_ste(&target, master, smmu_domain);
>  		arm_smmu_install_ste_for_dev(master, &target);
> -		if (master->cd_table.cdtab)
> -			arm_smmu_write_ctx_desc(master, IOMMU_NO_PASID,
> -						      NULL);
> +		arm_smmu_clear_cd(master, IOMMU_NO_PASID);
>  		break;
>  	}
>  
> @@ -2752,8 +2763,7 @@ static int arm_smmu_attach_dev_ste(struct device *dev,
>  	 * arm_smmu_domain->devices to avoid races updating the same context
>  	 * descriptor from arm_smmu_share_asid().
>  	 */
> -	if (master->cd_table.cdtab)
> -		arm_smmu_write_ctx_desc(master, IOMMU_NO_PASID, NULL);
> +	arm_smmu_clear_cd(master, IOMMU_NO_PASID);
>  	return 0;
>  }
>  
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 7078ed569fd4d3..87a7b57f566fbc 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -749,6 +749,8 @@ extern struct xarray arm_smmu_asid_xa;
>  extern struct mutex arm_smmu_asid_lock;
>  extern struct arm_smmu_ctx_desc quiet_cd;
>  
> +void arm_smmu_clear_cd(struct arm_smmu_master *master, ioasid_t ssid);
> +
>  int arm_smmu_write_ctx_desc(struct arm_smmu_master *smmu_master, int ssid,
>  			    struct arm_smmu_ctx_desc *cd);
>  void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid);
> -- 
> 2.43.2
>
Thanks,
Mostafa

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

^ permalink raw reply

* Re: [PATCH 2/3] dt-bindings: remoteproc: add Versal-NET platform
From: Tanmay Shah @ 2024-03-22 18:28 UTC (permalink / raw)
  To: Krzysztof Kozlowski, andersson, mathieu.poirier, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, michal.simek, ben.levinsky
  Cc: linux-remoteproc, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <b224cd7e-ebd0-41ef-b1e2-64f76d246307@linaro.org>



On 3/22/24 12:44 AM, Krzysztof Kozlowski wrote:
> On 21/03/2024 16:13, Tanmay Shah wrote:
>> 
>> 
>> On 3/21/24 2:39 AM, Krzysztof Kozlowski wrote:
>>> On 20/03/2024 16:14, Tanmay Shah wrote:
>>>>
>>>>
>>>> On 3/20/24 2:40 AM, Krzysztof Kozlowski wrote:
>>>>> On 19/03/2024 15:42, Tanmay Shah wrote:
>>>>>>
>>>>>>
>>>>>> On 3/19/24 12:30 AM, Krzysztof Kozlowski wrote:
>>>>>>> On 19/03/2024 01:51, Tanmay Shah wrote:
>>>>>>>> Hello Krzysztof,
>>>>>>>>
>>>>>>>> Thanks for reviews. Please find my comments below.
>>>>>>>>
>>>>>>>> On 3/17/24 1:53 PM, Krzysztof Kozlowski wrote:
>>>>>>>>> On 15/03/2024 22:15, Tanmay Shah wrote:
>>>>>>>>>> AMD-Xilinx Versal-NET platform is successor of Versal platform. It
>>>>>>>>>> contains multiple clusters of cortex-R52 real-time processing units.
>>>>>>>>>> Each cluster contains two cores of cortex-R52 processors. Each cluster
>>>>>>>>>> can be configured in lockstep mode or split mode.
>>>>>>>>>>
>>>>>>>>>> Each R52 core is assigned 128KB of TCM memory. ATCM memory is 64KB, BTCM
>>>>>>>>>> and CTCM memoreis are 32KB each. Each TCM memory has its own dedicated
>>>>>>>>>> power-domain that needs to be requested before using it.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
>>>>>>>>>> ---
>>>>>>>>>>  .../remoteproc/xlnx,zynqmp-r5fss.yaml         | 220 +++++++++++++++---
>>>>>>>>>>  1 file changed, 184 insertions(+), 36 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>>>>>>>>>> index 711da0272250..55654ee02eef 100644
>>>>>>>>>> --- a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>>>>>>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>>>>>>>>>> @@ -18,7 +18,9 @@ description: |
>>>>>>>>>>  
>>>>>>>>>>  properties:
>>>>>>>>>>    compatible:
>>>>>>>>>> -    const: xlnx,zynqmp-r5fss
>>>>>>>>>> +    enum:
>>>>>>>>>> +      - xlnx,zynqmp-r5fss
>>>>>>>>>> +      - xlnx,versal-net-r52fss
>>>>>>>>>>  
>>>>>>>>>>    "#address-cells":
>>>>>>>>>>      const: 2
>>>>>>>>>> @@ -64,7 +66,9 @@ patternProperties:
>>>>>>>>>>  
>>>>>>>>>>      properties:
>>>>>>>>>>        compatible:
>>>>>>>>>> -        const: xlnx,zynqmp-r5f
>>>>>>>>>> +        enum:
>>>>>>>>>> +          - xlnx,zynqmp-r5f
>>>>>>>>>> +          - xlnx,versal-net-r52f
>>>>>>>>>>  
>>>>>>>>>>        reg:
>>>>>>>>>>          minItems: 1
>>>>>>>>>> @@ -135,9 +139,11 @@ required:
>>>>>>>>>>  allOf:
>>>>>>>>>>    - if:
>>>>>>>>>>        properties:
>>>>>>>>>> -        xlnx,cluster-mode:
>>>>>>>>>> -          enum:
>>>>>>>>>> -            - 1
>>>>>>>>>> +        compatible:
>>>>>>>>>> +          contains:
>>>>>>>>>> +            enum:
>>>>>>>>>> +              - xlnx,versal-net-r52fss
>>>>>>>>>
>>>>>>>>> Why do you touch these lines?
>>>>>>>>>
>>>>>>>>>> +
>>>>>>>>>>      then:
>>>>>>>>>>        patternProperties:
>>>>>>>>>>          "^r5f@[0-9a-f]+$":
>>>>>>>>>> @@ -149,16 +155,14 @@ allOf:
>>>>>>>>>>                items:
>>>>>>>>>>                  - description: ATCM internal memory
>>>>>>>>>>                  - description: BTCM internal memory
>>>>>>>>>> -                - description: extra ATCM memory in lockstep mode
>>>>>>>>>> -                - description: extra BTCM memory in lockstep mode
>>>>>>>>>> +                - description: CTCM internal memory
>>>>>>>>>>  
>>>>>>>>>>              reg-names:
>>>>>>>>>>                minItems: 1
>>>>>>>>>>                items:
>>>>>>>>>> -                - const: atcm0
>>>>>>>>>> -                - const: btcm0
>>>>>>>>>> -                - const: atcm1
>>>>>>>>>> -                - const: btcm1
>>>>>>>>>> +                - const: atcm
>>>>>>>>>> +                - const: btcm
>>>>>>>>>> +                - const: ctcm
>>>>>>>>>>  
>>>>>>>>>>              power-domains:
>>>>>>>>>>                minItems: 2
>>>>>>>>>> @@ -166,33 +170,70 @@ allOf:
>>>>>>>>>>                  - description: RPU core power domain
>>>>>>>>>>                  - description: ATCM power domain
>>>>>>>>>>                  - description: BTCM power domain
>>>>>>>>>> -                - description: second ATCM power domain
>>>>>>>>>> -                - description: second BTCM power domain
>>>>>>>>>> +                - description: CTCM power domain
>>>>>>>>>>  
>>>>>>>>>>      else:
>>>>>>>>>> -      patternProperties:
>>>>>>>>>> -        "^r5f@[0-9a-f]+$":
>>>>>>>>>> -          type: object
>>>>>>>>>> -
>>>>>>>>>> -          properties:
>>>>>>>>>> -            reg:
>>>>>>>>>> -              minItems: 1
>>>>>>>>>> -              items:
>>>>>>>>>> -                - description: ATCM internal memory
>>>>>>>>>> -                - description: BTCM internal memory
>>>>>>>>>> -
>>>>>>>>>> -            reg-names:
>>>>>>>>>> -              minItems: 1
>>>>>>>>>> -              items:
>>>>>>>>>> -                - const: atcm0
>>>>>>>>>> -                - const: btcm0
>>>>>>>>>> -
>>>>>>>>>> -            power-domains:
>>>>>>>>>> -              minItems: 2
>>>>>>>>>> -              items:
>>>>>>>>>> -                - description: RPU core power domain
>>>>>>>>>> -                - description: ATCM power domain
>>>>>>>>>> -                - description: BTCM power domain
>>>>>>>>>> +      allOf:
>>>>>>>>>> +        - if:
>>>>>>>>>> +            properties:
>>>>>>>>>> +              xlnx,cluster-mode:
>>>>>>>>>> +                enum:
>>>>>>>>>> +                  - 1
>>>>>>>>>
>>>>>>>>> Whatever you did here, is not really readable. You have now multiple
>>>>>>>>> if:then:if:then embedded.
>>>>>>>>
>>>>>>>> For ZynqMP platform, TCM can be configured differently in lockstep mode
>>>>>>>> and split mode.
>>>>>>>>
>>>>>>>> For Versal-NET no such configuration is available, but new CTCM memory
>>>>>>>> is added.
>>>>>>>>
>>>>>>>> So, I am trying to achieve following representation of TCM for both:
>>>>>>>>
>>>>>>>> if: versal-net compatible
>>>>>>>> then:
>>>>>>>>   ATCM - 64KB
>>>>>>>>   BTCM - 32KB
>>>>>>>>   CTCM - 32KB
>>>>>>>>
>>>>>>>> else: (ZynqMP compatible)
>>>>>>>>   if:
>>>>>>>>     xlnx,cluster-mode (lockstep mode)
>>>>>>>>   then:
>>>>>>>>     ATCM0 - 64KB
>>>>>>>>     BTCM0 - 64KB
>>>>>>>>     ATCM1 - 64KB
>>>>>>>>     BTCM1 - 64KB
>>>>>>>>   else: (split mode)
>>>>>>>>     ATCM0 - 64KB
>>>>>>>>     BTCM0 - 64KB
>>>>>>>>
>>>>>>>>
>>>>>>>> If bindings are getting complicated, does it make sense to introduce
>>>>>>>> new file for Versal-NET bindings? Let me know how you would like me
>>>>>>>> to proceed.
>>>>>>>
>>>>>>> All this is broken in your previous patchset, but now we nicely see.
>>>>>>>
>>>>>>> No, this does not work like this. You do not have entirely different
>>>>>>> programming models in one device, don't you?
>>>>>>>
>>>>>>
>>>>>> I don't understand what do you mean? Programming model is same. Only number
>>>>>> of TCMs are changing based on configuration and platform. I can certainly
>>>>>> list different compatible for different platforms as requested. But other than
>>>>>> that not sure what needs to be fixed.
>>>>>
>>>>> You cannot have same programming model with different memory mappings.
>>>>> Anyway, please follow writing bindings rules: all of your different
>>>>> devices must have dedicated compatible. I really though we talked about
>>>>> two IPs on same SoC...
>>>>
>>>> I agree that Versal compatible should be added, I will do that in next revision.
>>>>
>>>> For ZynqMP case, it is two IPs on same SOC. In lockstep mode and split mode,
>>>> same SOC is configuring TCM differently.
>>>>
>>>> How this should be resolved for Versal-NET ? Driver avoids such TCM configuration
>>>> for Versal-NET.
>>>
>>> Binding should describe the hardware, not what driver is doing
>>> currently, so the question is: does your device have such properties or
>>> not? Anyway, you need compatible per each variant and each SoC
>>> implementation.
>> 
>> Thanks for reviews.
>> 
>> Okay in that case I believe I should add one more property to current bindings for TCM
>> configuration.
>> 
> 
> I am not sure if you understand how IRC works... You sent me message on
> IRC about this topic and shortly after you quit. So how am I supposed to
> send reply? IRC does not work like that...
> 

Yeah, I am referring related documentation on IRC.

>> From our discussion I conclude to following next steps:
>> 
>> 1) I will send Versal and Versal-NET support as part of previous series (v14) so we get
>> bigger picture in the first place.
>> 
>> 2) Add separate compatible for versal platform.
>> Use device compatible string to maintain
>> backward compatibility and not machine (root node) compatible string.
>> 
>> 3) Add tcm,mode property in bindings and each device must configure TCM based on that
>> property only and not based on compatible string.
>> 
>> 4) Versal-NET will disallow tcm,mode property in bindings as no such configuration is
>> possible for that platform.
> 
> I really don't know your SoCs. What about Zynq? You keep using here
> names all over the place, but I am not Xilinx maintainer.
> 

Zynq doesn't have Cortex-R IP so this driver isn't needed on that.



> 
> Best regards,
> Krzysztof
> 


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

^ permalink raw reply

* Re: [PATCH 2/2] arm64: dts: add description for solidrun cn9130 som and clearfog boards
From: Andrew Lunn @ 2024-03-22 18:27 UTC (permalink / raw)
  To: Josua Mayer
  Cc: Gregory Clement, Sebastian Hesselbarth, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Yazan Shhady,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <8ad64d2e-2be9-4d59-907b-669fdedc3695@solid-run.com>

On Fri, Mar 22, 2024 at 06:14:38PM +0000, Josua Mayer wrote:
> Am 22.03.24 um 16:38 schrieb Josua Mayer:
> > For the discrete PHYs, the generic LED code can make use of the
> > hardware offload support to read back the hardware configuration and
> > configure itself to match. The switch code is missing hardware offload
> > at the moment. So it cannot read back the current
> > configuration. However, it is simple code to add, and the discrete
> > code is a good example to follow.
> I have prototyped this on top of your patch-set, supporting offload
> for a single mode.
> It works as explained by you - first after boot-up the LEDs
> are executing their default function autonomously.
> 
> When I set trigger netdev, I can see offloaded property is 1,
> and when I enable extra bits offload turns off.
> 
> 
> For Clearfog Base I have added the requested LED descriptions,
> it should be ready now.
> 
> The Pro version I could
> 1) submit new version with only phy leds
> 2) wait (not preferred)
> 3) submit new version with an separate patch adding switch leds
> (can hold of ack on it till the fait of your patch-set becomes clear)

You probably need to wait whatever. We are in the merge window. Many
maintainers don't accept patches during these two weeks. They want you
to submit against -rc1 once it is released. And there is no rush. The
next merge window is not for another 7 weeks or so. Gregory will
accept patches for mvebu for around 5 of those weeks.

       Andrew

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

^ permalink raw reply

* Re: [PATCH v4 1/5] dt-bindings: misc: Add mikrobus-connector
From: Ayush Singh @ 2024-03-22 18:15 UTC (permalink / raw)
  To: open list
  Cc: jkridner, robertcnelson, lorforlinux, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, Derek Kiernan, Dragan Cvetic,
	Arnd Bergmann, Greg Kroah-Hartman, Vaishnav M A, Mark Brown,
	Johan Hovold, Alex Elder,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/TEXAS INSTRUMENTS K3 ARCHITECTURE,
	open list:SPI SUBSYSTEM, moderated list:GREYBUS SUBSYSTEM,
	Vaishnav M A, Michael Walle, Andrew Lunn
In-Reply-To: <20240317193714.403132-2-ayushdevel1325@gmail.com>

On 3/18/24 01:07, Ayush Singh wrote:

> Add DT bindings for mikroBUS interface. MikroBUS is an open standard
> developed by MikroElektronika for connecting add-on boards to
> microcontrollers or microprocessors.
>
> mikroBUS is a connector and does not have a controller. Instead the
> software is responsible for identification of board and setting up /
> registering uart, spi, i2c, pwm and other buses. Thus it needs a way to
> get uart, spi, i2c, pwm and gpio controllers / adapters.
>
> A mikroBUS addon board is free to leave some of the pins unused which
> are marked as NC or Not Connected.
>
> Some of the pins might need to be configured as GPIOs deviating from their
> reserved purposes Eg: SHT15 Click where the SCL and SDA Pins need to be
> configured as GPIOs for the driver (drivers/hwmon/sht15.c) to work.
>
> For some add-on boards the driver may not take care of some additional
> signals like reset/wake-up/other. Eg: ENC28J60 click where the reset line
> (RST pin on the mikrobus port) needs to be pulled high.
>
> Here's the list of pins in mikroBUS connector:
> Analog - AN
> Reset - RST
> SPI Chip Select - CS
> SPI Clock - SCK
> SPI Master Input Slave Output - MISO
> SPI Master Output Slave Input - MOSI
> VCC-3.3V power - +3.3V
> Reference Ground - GND
> PWM - PWM output
> INT - Hardware Interrupt
> RX - UART Receive
> TX - UART Transmit
> SCL - I2C Clock
> SDA - I2C Data
> +5V - VCC-5V power
> GND - Reference Ground
>
> Additionally, some new mikroBUS boards contain 1-wire EEPROM that contains
> a manifest to describe the addon board to provide plug and play
> capabilities.
>
> Link: https://www.mikroe.com/mikrobus
> Link:
> https://download.mikroe.com/documents/standards/mikrobus/mikrobus-standard-specification-v200.pdf
> mikroBUS specification
> Link: https://www.mikroe.com/sht1x-click SHT15 Click
> Link: https://www.mikroe.com/eth-click ENC28J60 Click
> Link: https://www.mikroe.com/clickid ClickID
>
> Co-developed-by: Vaishnav M A <vaishnav@beagleboard.org>
> Signed-off-by: Vaishnav M A <vaishnav@beagleboard.org>
> Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
> ---
>   .../connector/mikrobus-connector.yaml         | 113 ++++++++++++++++++
>   MAINTAINERS                                   |   6 +
>   2 files changed, 119 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
>
> diff --git a/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
> new file mode 100644
> index 000000000000..ee3736add41c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
> @@ -0,0 +1,113 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/connector/mikrobus-connector.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: mikroBUS add-on board socket
> +
> +maintainers:
> +  - Ayush Singh <ayushdevel1325@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: mikrobus-connector
> +
> +  pinctrl-0: true
> +  pinctrl-1: true
> +  pinctrl-2: true
> +  pinctrl-3: true
> +  pinctrl-4: true
> +  pinctrl-5: true
> +  pinctrl-6: true
> +  pinctrl-7: true
> +  pinctrl-8: true
> +
> +  pinctrl-names:
> +    items:
> +      - const: default
> +      - const: pwm_default
> +      - const: pwm_gpio
> +      - const: uart_default
> +      - const: uart_gpio
> +      - const: i2c_default
> +      - const: i2c_gpio
> +      - const: spi_default
> +      - const: spi_gpio
> +
> +  mikrobus-gpios:
> +    minItems: 11
> +    maxItems: 12
> +
> +  i2c-adapter:
> +    description: i2c adapter attached to the mikrobus socket.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  spi-controller:
> +    description: spi bus number of the spi-master attached to the mikrobus socket.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  uart:
> +    description: uart port attached to the mikrobus socket
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  pwms:
> +    description: the pwm-controller corresponding to the mikroBUS PWM pin.
> +    maxItems: 1
> +
> +  spi-cs:
> +    description: spi chip-select numbers corresponding to the chip-selects on the mikrobus socket.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    items:
> +      - description: chip select corresponding to CS pin
> +      - description: chip select corresponding to RST pin
> +
> +required:
> +  - compatible
> +  - pinctrl-0
> +  - pinctrl-1
> +  - pinctrl-2
> +  - pinctrl-3
> +  - pinctrl-4
> +  - pinctrl-5
> +  - pinctrl-6
> +  - pinctrl-7
> +  - pinctrl-8
> +  - i2c-adapter
> +  - spi-controller
> +  - spi-cs
> +  - uart
> +  - pwms
> +  - mikrobus-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    mikrobus {
> +      compatible = "mikrobus-connector";
> +      pinctrl-names = "default", "pwm_default", "pwm_gpio","uart_default", "uart_gpio", "i2c_default",
> +                      "i2c_gpio", "spi_default", "spi_gpio";
> +      pinctrl-0 = <&P2_03_gpio_input_pin &P1_04_gpio_pin &P1_02_gpio_pin>;
> +      pinctrl-1 = <&P2_01_pwm_pin>;
> +      pinctrl-2 = <&P2_01_gpio_pin>;
> +      pinctrl-3 = <&P2_05_uart_pin &P2_07_uart_pin>;
> +      pinctrl-4 = <&P2_05_gpio_pin &P2_07_gpio_pin>;
> +      pinctrl-5 = <&P2_09_i2c_pin &P2_11_i2c_pin>;
> +      pinctrl-6 = <&P2_09_gpio_pin &P2_11_gpio_pin>;
> +      pinctrl-7 = <&P1_12_spi_pin &P1_10_spi_pin &P1_08_spi_sclk_pin &P1_06_spi_cs_pin>;
> +      pinctrl-8 = <&P1_12_gpio_pin &P1_10_gpio_pin &P1_08_gpio_pin &P1_06_gpio_pin>;
> +      pwms = <&ehrpwm1 0 500000 0>;
> +      i2c-adapter = <&i2c1>;
> +      spi-controller = <&spi1>;
> +      spi-cs = <0 1>;
> +      uart = <&uart1>;
> +      mikrobus-gpios = <&gpio1 18 GPIO_ACTIVE_HIGH>, <&gpio0 23 GPIO_ACTIVE_HIGH>,
> +                       <&gpio0 30 GPIO_ACTIVE_HIGH>, <&gpio0 31 GPIO_ACTIVE_HIGH>,
> +                       <&gpio0 15 GPIO_ACTIVE_HIGH>, <&gpio0 14 GPIO_ACTIVE_HIGH>,
> +                       <&gpio0 4 GPIO_ACTIVE_HIGH>, <&gpio0 3 GPIO_ACTIVE_HIGH>,
> +                       <&gpio0 2 GPIO_ACTIVE_HIGH>, <&gpio0 5 GPIO_ACTIVE_HIGH>,
> +                       <&gpio2 25 GPIO_ACTIVE_HIGH>, <&gpio2 3 GPIO_ACTIVE_HIGH>;
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 375d34363777..69418a058c6b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14767,6 +14767,12 @@ M:	Oliver Neukum <oliver@neukum.org>
>   S:	Maintained
>   F:	drivers/usb/image/microtek.*
>   
> +MIKROBUS
> +M:	Ayush Singh <ayushdevel1325@gmail.com>
> +M:	Vaishnav M A <vaishnav@beagleboard.org>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
> +
>   MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
>   M:	Luka Kovacic <luka.kovacic@sartura.hr>
>   M:	Luka Perkov <luka.perkov@sartura.hr>


After going through all the discussions here, I have a few questions:

1. Is the old `*_register_device(controller, board_info)` style 
discouraged in favor of using device tree, at least for drivers using 
multiple fundamental buses (i2c, spi, etc)? Or is the problem just that 
these bindings do not leave open the possibility of using device tree 
overlays? Will it be fine if the dt bindings allow for dt overlays, but 
the driver still uses imperative registering of board?

2. Is the preferred way to handle virtual devices (like those created by 
greybus subsystem) now device tree? Is that one of the blockers for 
greybus i2c, spi etc to still be in staging?

3. How are virtual devices created in device tree? If I register an i2c 
adapter using `i2c_add_adapter`, is the device tree entry is dynamically 
created, which can then be used by a device tree overlay?


Ayush Singh


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

^ permalink raw reply

* Re: [PATCH 2/2] arm64: dts: add description for solidrun cn9130 som and clearfog boards
From: Josua Mayer @ 2024-03-22 18:14 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Gregory Clement, Sebastian Hesselbarth, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Yazan Shhady,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <748753a6-9fde-4d4f-8fee-5b93dbb532f0@solid-run.com>

Am 22.03.24 um 16:38 schrieb Josua Mayer:
> For the discrete PHYs, the generic LED code can make use of the
> hardware offload support to read back the hardware configuration and
> configure itself to match. The switch code is missing hardware offload
> at the moment. So it cannot read back the current
> configuration. However, it is simple code to add, and the discrete
> code is a good example to follow.
I have prototyped this on top of your patch-set, supporting offload
for a single mode.
It works as explained by you - first after boot-up the LEDs
are executing their default function autonomously.

When I set trigger netdev, I can see offloaded property is 1,
and when I enable extra bits offload turns off.


For Clearfog Base I have added the requested LED descriptions,
it should be ready now.

The Pro version I could
1) submit new version with only phy leds
2) wait (not preferred)
3) submit new version with an separate patch adding switch leds
(can hold of ack on it till the fait of your patch-set becomes clear)

_______________________________________________
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 v5 04/27] iommu/arm-smmu-v3: Add an ops indirection to the STE code
From: Mostafa Saleh @ 2024-03-22 18:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
	Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
	Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <4-v5-9a37e0c884ce+31e3-smmuv3_newapi_p2_jgg@nvidia.com>

Hi Jason,

On Mon, Mar 04, 2024 at 07:43:52PM -0400, Jason Gunthorpe wrote:
> Prepare to put the CD code into the same mechanism. Add an ops indirection
> around all the STE specific code and make the worker functions independent
> of the entry content being processed.
> 
> get_used and sync ops are provided to hook the correct code.
> 
> Signed-off-by: Michael Shavit <mshavit@google.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 172 ++++++++++++--------
>  1 file changed, 104 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index c60b067c1f553e..b7f947e36f596f 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -48,8 +48,20 @@ enum arm_smmu_msi_index {
>  	ARM_SMMU_MAX_MSIS,
>  };
>  
> -static void arm_smmu_sync_ste_for_sid(struct arm_smmu_device *smmu,
> -				      ioasid_t sid);
> +struct arm_smmu_entry_writer_ops;
> +struct arm_smmu_entry_writer {
> +	const struct arm_smmu_entry_writer_ops *ops;
> +	struct arm_smmu_master *master;
> +};
> +
> +struct arm_smmu_entry_writer_ops {
> +	unsigned int num_entry_qwords;
> +	__le64 v_bit;
> +	void (*get_used)(const __le64 *entry, __le64 *used);
> +	void (*sync)(struct arm_smmu_entry_writer *writer);
> +};
> +
> +#define NUM_ENTRY_QWORDS (sizeof(struct arm_smmu_ste) / sizeof(u64))
>
>  static phys_addr_t arm_smmu_msi_cfg[ARM_SMMU_MAX_MSIS][3] = {
>  	[EVTQ_MSI_INDEX] = {
> @@ -982,43 +994,42 @@ void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid)
>   * would be nice if this was complete according to the spec, but minimally it
>   * has to capture the bits this driver uses.
>   */
> -static void arm_smmu_get_ste_used(const struct arm_smmu_ste *ent,
> -				  struct arm_smmu_ste *used_bits)
> +static void arm_smmu_get_ste_used(const __le64 *ent, __le64 *used_bits)
>  {
> -	unsigned int cfg = FIELD_GET(STRTAB_STE_0_CFG, le64_to_cpu(ent->data[0]));
> +	unsigned int cfg = FIELD_GET(STRTAB_STE_0_CFG, le64_to_cpu(ent[0]));
>  
> -	used_bits->data[0] = cpu_to_le64(STRTAB_STE_0_V);
> -	if (!(ent->data[0] & cpu_to_le64(STRTAB_STE_0_V)))
> +	used_bits[0] = cpu_to_le64(STRTAB_STE_0_V);
> +	if (!(ent[0] & cpu_to_le64(STRTAB_STE_0_V)))
>  		return;
>  
> -	used_bits->data[0] |= cpu_to_le64(STRTAB_STE_0_CFG);
> +	used_bits[0] |= cpu_to_le64(STRTAB_STE_0_CFG);
>  
>  	/* S1 translates */
>  	if (cfg & BIT(0)) {
> -		used_bits->data[0] |= cpu_to_le64(STRTAB_STE_0_S1FMT |
> -						  STRTAB_STE_0_S1CTXPTR_MASK |
> -						  STRTAB_STE_0_S1CDMAX);
> -		used_bits->data[1] |=
> +		used_bits[0] |= cpu_to_le64(STRTAB_STE_0_S1FMT |
> +					    STRTAB_STE_0_S1CTXPTR_MASK |
> +					    STRTAB_STE_0_S1CDMAX);
> +		used_bits[1] |=
>  			cpu_to_le64(STRTAB_STE_1_S1DSS | STRTAB_STE_1_S1CIR |
>  				    STRTAB_STE_1_S1COR | STRTAB_STE_1_S1CSH |
>  				    STRTAB_STE_1_S1STALLD | STRTAB_STE_1_STRW |
>  				    STRTAB_STE_1_EATS);
> -		used_bits->data[2] |= cpu_to_le64(STRTAB_STE_2_S2VMID);
> +		used_bits[2] |= cpu_to_le64(STRTAB_STE_2_S2VMID);
>  	}
>  
>  	/* S2 translates */
>  	if (cfg & BIT(1)) {
> -		used_bits->data[1] |=
> +		used_bits[1] |=
>  			cpu_to_le64(STRTAB_STE_1_EATS | STRTAB_STE_1_SHCFG);
> -		used_bits->data[2] |=
> +		used_bits[2] |=
>  			cpu_to_le64(STRTAB_STE_2_S2VMID | STRTAB_STE_2_VTCR |
>  				    STRTAB_STE_2_S2AA64 | STRTAB_STE_2_S2ENDI |
>  				    STRTAB_STE_2_S2PTW | STRTAB_STE_2_S2R);
> -		used_bits->data[3] |= cpu_to_le64(STRTAB_STE_3_S2TTB_MASK);
> +		used_bits[3] |= cpu_to_le64(STRTAB_STE_3_S2TTB_MASK);
>  	}
>  
>  	if (cfg == STRTAB_STE_0_CFG_BYPASS)
> -		used_bits->data[1] |= cpu_to_le64(STRTAB_STE_1_SHCFG);
> +		used_bits[1] |= cpu_to_le64(STRTAB_STE_1_SHCFG);
>  }
>  
>  /*
> @@ -1027,57 +1038,55 @@ static void arm_smmu_get_ste_used(const struct arm_smmu_ste *ent,
>   * unused_update is an intermediate value of entry that has unused bits set to
>   * their new values.
>   */
> -static u8 arm_smmu_entry_qword_diff(const struct arm_smmu_ste *entry,
> -				    const struct arm_smmu_ste *target,
> -				    struct arm_smmu_ste *unused_update)
> +static u8 arm_smmu_entry_qword_diff(struct arm_smmu_entry_writer *writer,
> +				    const __le64 *entry, const __le64 *target,
> +				    __le64 *unused_update)
>  {
> -	struct arm_smmu_ste target_used = {};
> -	struct arm_smmu_ste cur_used = {};
> +	__le64 target_used[NUM_ENTRY_QWORDS] = {};
> +	__le64 cur_used[NUM_ENTRY_QWORDS] = {};
This is confusing to me, the function was modified to be generic, so its has
args are __le64 * instead of struct arm_smmu_ste *.
But NUM_ENTRY_QWORDS is defined as “(sizeof(struct arm_smmu_ste) / sizeof(u64))”
and in the same function writer->ops->num_entry_qwords is used nterchangeably,
I understand that this not a constant and the compiler would complain.
But since for any other num_entry_qwords larger than NUM_ENTRY_QWORDS it fails,
and we know STEs and CDs both have the same size, we simplify the code and make
it a constant everywhere.

I see in the next patch, that this is redefined to be the max between STE and
CD, but again, this hardware and it never changes, so my opinion is to simplify
the code, as there is no need to generalize this part.

>  	u8 used_qword_diff = 0;
>  	unsigned int i;
>  
> -	arm_smmu_get_ste_used(entry, &cur_used);
> -	arm_smmu_get_ste_used(target, &target_used);
> +	writer->ops->get_used(entry, cur_used);
> +	writer->ops->get_used(target, target_used);
>  
> -	for (i = 0; i != ARRAY_SIZE(target_used.data); i++) {
> +	for (i = 0; i != writer->ops->num_entry_qwords; i++) {
>  		/*
>  		 * Check that masks are up to date, the make functions are not
>  		 * allowed to set a bit to 1 if the used function doesn't say it
>  		 * is used.
>  		 */
> -		WARN_ON_ONCE(target->data[i] & ~target_used.data[i]);
> +		WARN_ON_ONCE(target[i] & ~target_used[i]);
>  
>  		/* Bits can change because they are not currently being used */
> -		unused_update->data[i] = (entry->data[i] & cur_used.data[i]) |
> -					 (target->data[i] & ~cur_used.data[i]);
> +		unused_update[i] = (entry[i] & cur_used[i]) |
> +				   (target[i] & ~cur_used[i]);
>  		/*
>  		 * Each bit indicates that a used bit in a qword needs to be
>  		 * changed after unused_update is applied.
>  		 */
> -		if ((unused_update->data[i] & target_used.data[i]) !=
> -		    target->data[i])
> +		if ((unused_update[i] & target_used[i]) != target[i])
>  			used_qword_diff |= 1 << i;
>  	}
>  	return used_qword_diff;
>  }
>  
> -static bool entry_set(struct arm_smmu_device *smmu, ioasid_t sid,
> -		      struct arm_smmu_ste *entry,
> -		      const struct arm_smmu_ste *target, unsigned int start,
> +static bool entry_set(struct arm_smmu_entry_writer *writer, __le64 *entry,
> +		      const __le64 *target, unsigned int start,
>  		      unsigned int len)
>  {
>  	bool changed = false;
>  	unsigned int i;
>  
>  	for (i = start; len != 0; len--, i++) {
> -		if (entry->data[i] != target->data[i]) {
> -			WRITE_ONCE(entry->data[i], target->data[i]);
> +		if (entry[i] != target[i]) {
> +			WRITE_ONCE(entry[i], target[i]);
>  			changed = true;
>  		}
>  	}
>  
>  	if (changed)
> -		arm_smmu_sync_ste_for_sid(smmu, sid);
> +		writer->ops->sync(writer);
>  	return changed;
>  }
>  
> @@ -1107,17 +1116,15 @@ static bool entry_set(struct arm_smmu_device *smmu, ioasid_t sid,
>   * V=0 process. This relies on the IGNORED behavior described in the
>   * specification.
>   */
> -static void arm_smmu_write_ste(struct arm_smmu_master *master, u32 sid,
> -			       struct arm_smmu_ste *entry,
> -			       const struct arm_smmu_ste *target)
> +static void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer,
> +				 __le64 *entry, const __le64 *target)
>  {
> -	unsigned int num_entry_qwords = ARRAY_SIZE(target->data);
> -	struct arm_smmu_device *smmu = master->smmu;
> -	struct arm_smmu_ste unused_update;
> +	unsigned int num_entry_qwords = writer->ops->num_entry_qwords;
> +	__le64 unused_update[NUM_ENTRY_QWORDS];
>  	u8 used_qword_diff;
>  
>  	used_qword_diff =
> -		arm_smmu_entry_qword_diff(entry, target, &unused_update);
> +		arm_smmu_entry_qword_diff(writer, entry, target, unused_update);
>  	if (hweight8(used_qword_diff) == 1) {
>  		/*
>  		 * Only one qword needs its used bits to be changed. This is a
> @@ -1133,22 +1140,21 @@ static void arm_smmu_write_ste(struct arm_smmu_master *master, u32 sid,
>  		 * writing it in the next step anyways. This can save a sync
>  		 * when the only change is in that qword.
>  		 */
> -		unused_update.data[critical_qword_index] =
> -			entry->data[critical_qword_index];
> -		entry_set(smmu, sid, entry, &unused_update, 0, num_entry_qwords);
> -		entry_set(smmu, sid, entry, target, critical_qword_index, 1);
> -		entry_set(smmu, sid, entry, target, 0, num_entry_qwords);
> +		unused_update[critical_qword_index] =
> +			entry[critical_qword_index];
> +		entry_set(writer, entry, unused_update, 0, num_entry_qwords);
> +		entry_set(writer, entry, target, critical_qword_index, 1);
> +		entry_set(writer, entry, target, 0, num_entry_qwords);
>  	} else if (used_qword_diff) {
>  		/*
>  		 * At least two qwords need their inuse bits to be changed. This
>  		 * requires a breaking update, zero the V bit, write all qwords
>  		 * but 0, then set qword 0
>  		 */
> -		unused_update.data[0] = entry->data[0] &
> -					cpu_to_le64(~STRTAB_STE_0_V);
> -		entry_set(smmu, sid, entry, &unused_update, 0, 1);
> -		entry_set(smmu, sid, entry, target, 1, num_entry_qwords - 1);
> -		entry_set(smmu, sid, entry, target, 0, 1);
> +		unused_update[0] = entry[0] & (~writer->ops->v_bit);
> +		entry_set(writer, entry, unused_update, 0, 1);
> +		entry_set(writer, entry, target, 1, num_entry_qwords - 1);
> +		entry_set(writer, entry, target, 0, 1);
>  	} else {
>  		/*
>  		 * No inuse bit changed. Sanity check that all unused bits are 0
> @@ -1156,18 +1162,7 @@ static void arm_smmu_write_ste(struct arm_smmu_master *master, u32 sid,
>  		 * compute_qword_diff().
>  		 */
>  		WARN_ON_ONCE(
> -			entry_set(smmu, sid, entry, target, 0, num_entry_qwords));
> -	}
> -
> -	/* It's likely that we'll want to use the new STE soon */
> -	if (!(smmu->options & ARM_SMMU_OPT_SKIP_PREFETCH)) {
> -		struct arm_smmu_cmdq_ent
> -			prefetch_cmd = { .opcode = CMDQ_OP_PREFETCH_CFG,
> -					 .prefetch = {
> -						 .sid = sid,
> -					 } };
> -
> -		arm_smmu_cmdq_issue_cmd(smmu, &prefetch_cmd);
> +			entry_set(writer, entry, target, 0, num_entry_qwords));
>  	}
>  }
>  
> @@ -1440,17 +1435,58 @@ arm_smmu_write_strtab_l1_desc(__le64 *dst, struct arm_smmu_strtab_l1_desc *desc)
>  	WRITE_ONCE(*dst, cpu_to_le64(val));
>  }
>  
> -static void arm_smmu_sync_ste_for_sid(struct arm_smmu_device *smmu, u32 sid)
> +struct arm_smmu_ste_writer {
> +	struct arm_smmu_entry_writer writer;
> +	u32 sid;
> +};
> +
> +static void arm_smmu_ste_writer_sync_entry(struct arm_smmu_entry_writer *writer)
>  {
> +	struct arm_smmu_ste_writer *ste_writer =
> +		container_of(writer, struct arm_smmu_ste_writer, writer);
>  	struct arm_smmu_cmdq_ent cmd = {
>  		.opcode	= CMDQ_OP_CFGI_STE,
>  		.cfgi	= {
> -			.sid	= sid,
> +			.sid	= ste_writer->sid,
>  			.leaf	= true,
>  		},
>  	};
>  
> -	arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
> +	arm_smmu_cmdq_issue_cmd_with_sync(writer->master->smmu, &cmd);
> +}
> +
> +static const struct arm_smmu_entry_writer_ops arm_smmu_ste_writer_ops = {
> +	.sync = arm_smmu_ste_writer_sync_entry,
> +	.get_used = arm_smmu_get_ste_used,
> +	.v_bit = cpu_to_le64(STRTAB_STE_0_V),
> +	.num_entry_qwords = sizeof(struct arm_smmu_ste) / sizeof(u64),
> +};
> +
> +static void arm_smmu_write_ste(struct arm_smmu_master *master, u32 sid,
> +			       struct arm_smmu_ste *ste,
> +			       const struct arm_smmu_ste *target)
> +{
> +	struct arm_smmu_device *smmu = master->smmu;
> +	struct arm_smmu_ste_writer ste_writer = {
> +		.writer = {
> +			.ops = &arm_smmu_ste_writer_ops,
> +			.master = master,
> +		},
> +		.sid = sid,
> +	};
> +
> +	arm_smmu_write_entry(&ste_writer.writer, ste->data, target->data);
> +
> +	/* It's likely that we'll want to use the new STE soon */
> +	if (!(smmu->options & ARM_SMMU_OPT_SKIP_PREFETCH)) {
> +		struct arm_smmu_cmdq_ent
> +			prefetch_cmd = { .opcode = CMDQ_OP_PREFETCH_CFG,
> +					 .prefetch = {
> +						 .sid = sid,
> +					 } };
> +
> +		arm_smmu_cmdq_issue_cmd(smmu, &prefetch_cmd);
> +	}
>  }
>  
>  static void arm_smmu_make_abort_ste(struct arm_smmu_ste *target)
> -- 
> 2.43.2
>
Thanks,
Mostafa

_______________________________________________
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 5/8] drm: zynqmp_dp: Don't retrain the link in our IRQ
From: Tomi Valkeinen @ 2024-03-22 18:09 UTC (permalink / raw)
  To: Sean Anderson
  Cc: Michal Simek, David Airlie, linux-kernel, Daniel Vetter,
	linux-arm-kernel, Laurent Pinchart, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, dri-devel
In-Reply-To: <2dbf138f-5112-48e1-85a6-9e3ad84ec4a6@linux.dev>

On 22/03/2024 18:18, Sean Anderson wrote:
> On 3/22/24 01:32, Tomi Valkeinen wrote:
>> On 21/03/2024 21:17, Sean Anderson wrote:
>>> On 3/21/24 15:08, Tomi Valkeinen wrote:
>>>> On 21/03/2024 20:01, Sean Anderson wrote:
>>>>> On 3/21/24 13:25, Tomi Valkeinen wrote:
>>>>>> On 21/03/2024 17:52, Sean Anderson wrote:
>>>>>>> On 3/20/24 02:53, Tomi Valkeinen wrote:
>>>>>>>> On 20/03/2024 00:51, Sean Anderson wrote:
>>>>>>>> Do we need to handle interrupts while either delayed work is being done?
>>>>>>>
>>>>>>> Probably not.
>>>>>>>
>>>>>>>> If we do need a delayed work, would just one work be enough which
>>>>>>>> handles both HPD_EVENT and HPD_IRQ, instead of two?
>>>>>>>
>>>>>>> Maybe, but then we need to determine which pending events we need to
>>>>>>> handle. I think since we have only two events it will be easier to just
>>>>>>> have separate workqueues.
>>>>>>
>>>>>> The less concurrency, the better...Which is why it would be nice to do it all in the threaded irq.
>>>>>
>>>>> Yeah, but we can use a mutex for this which means there is not too much
>>>>> interesting going on.
>>>>
>>>> Ok. Yep, if we get (hopefully) a single mutex with clearly defined fields that it protects, I'm ok with workqueues.
>>>>
>>>> I'd still prefer just one workqueue, though...
>>>
>>> Yeah, but then we need a spinlock or something to tell the workqueue what it should do.
>>
>> Yep. We could also always look at the HPD (if we drop the big sleeps) in the wq, and have a flag for the HPD IRQ, which would reduce the state to a single bit.
> 
> How about something like
> 
> zynqmp_dp_irq_handler(...)
> {
> 	/* Read status and handle underflow/overflow/vblank */
> 
> 	status &= ZYNQMP_DP_INT_HPD_EVENT | ZYNQMP_DP_INT_HPD_IRQ;
> 	if (status) {
> 		atomic_or(status, &dp->status);
> 		return IRQ_WAKE_THREAD;
> 	}
> 
> 	return IRQ_HANDLED;
> }
> 
> zynqmp_dp_thread_handler(...)
> {
> 	status = atomic_xchg(&dp->status, 0);
> 	/* process HPD stuff */
> }
> 
> which gets rid of the workqueue too.

I like it. We can't use IRQF_ONESHOT, as that would keep the irq masked 
while the threaded handler is being ran. I don't think that's a problem, 
but just something to keep in mind that both handlers can run concurrently.

  Tomi


_______________________________________________
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 4/4] clk: samsung: exynos850: fix propagation of SPI IPCLK rate
From: Sam Protsenko @ 2024-03-22 18:09 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: 김재원/JAEWON KIM, krzysztof.kozlowski,
	s.nawrocki, cw00.choi, alim.akhtar, mturquette, sboyd,
	peter.griffin, andre.draszik, linux-samsung-soc, linux-clk,
	linux-arm-kernel, linux-kernel, willmcvicker, kernel-team
In-Reply-To: <867158d7-0d98-4058-9c2e-6b573ec68516@linaro.org>

On Fri, Mar 22, 2024 at 4:39 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>
> Hi, Sam!
>
> On 3/1/24 00:13, Sam Protsenko wrote:
> > I fail to see how this patch fixes anything. Instead it looks to me it
> > replaces the (already) correctly implemented logic with incorrect one.
>
> I opened another thread asking for feedback on whether it's safe to
> re-parent the USI MUX to OSCCLK at run-time, find it here:
> https://lore.kernel.org/linux-samsung-soc/71df1d6b-f40b-4896-a672-c5f0f526fb1f@linaro.org/T/#m588abb87eb5fd8817d71d06b94c91eb84928e06b
>
> Jaewon came up with the idea on verifying what the downstream clock
> driver does. I added some prints in the driver, and indeed the USI MUX
> re-parents to OSCCLK on low SPI clock rates in the GS101 case.
>
> Thus I'll respin this patch set fixing GS101 on low USI clock rates by
> re-parenting the USI MUX to OSCCLK. I'll leave exynos850 out if I don't
> hear back from you, but I think it deserves the same fix. Allowing SPI
> to modify the clock rate of HSI2C/I3C at run-time is bad IMO.
> Re-parenting the USI MUX to OSCCLK fixes this problem, HSI2C/I3C will no
> longer be affected on low SPI clock rates.
>

Yes, please leave Exynos850 out of it, if possible. It's fine with me
if you send it for gs101, as it's you who is going to maintain that
platform further, so it's for the maintainers to decide. I'll refrain
from reviewing that particular patch.

For Exynos850 driver, I'm convinced the SPI clock derivation is
already implemented in the correct way (exactly as it was designed in
HW), and doing anything else would be a hack, and frankly this sole
fact is already enough of argumentation for me. There is also the
whole bunch of use-cases which I think could be affected by using
OSCCLK, e.g.: clock signal integrity, runtime PM concerns, possible
interference in case of automatic clock control enablement, etc. I
don't even want to think about all possible pitfalls which
implementation of this non-standard and undocumented behavior could
create. So as the only person who currently supports Exynos850 drivers
(apart from the maintainers, of course), I would strictly oppose this
particular OSCCLK change.

> Cheers,
> ta

_______________________________________________
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 v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
From: Conor Dooley @ 2024-03-22 18:05 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Anatoliy Klymenko, Laurent Pinchart, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
	Michal Simek, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Jonas Karlman, Jernej Skrabec, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Mauro Carvalho Chehab, Tomi Valkeinen, dri-devel,
	linux-arm-kernel, linux-kernel, devicetree, linux-media
In-Reply-To: <a82d525c-737a-4ac4-9d71-e88f4ba69ea1@linaro.org>


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

On Fri, Mar 22, 2024 at 06:59:18AM +0100, Krzysztof Kozlowski wrote:
> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> > diff --git a/include/dt-bindings/media/media-bus-format.h b/include/dt-bindings/media/media-bus-format.h
> > new file mode 100644
> > index 000000000000..60fc6e11dabc
> > --- /dev/null
> > +++ b/include/dt-bindings/media/media-bus-format.h
> > @@ -0,0 +1,177 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> > +/*
> > + * Media Bus API header
> > + *
> > + * Copyright (C) 2009, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> 
> That's not true. Your SPDX tells something entirely different.
> 
> Anyway, you did not explain why you need to copy anything anywhere.

I assume by "copy anything anywhere" you mean "why did you copy a linux
uapi header into the bindings?

> Specifically, random hex values *are not bindings*.
> 
> Best regards,
> Krzysztof
> 

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

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

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

^ permalink raw reply

* Re: [PATCH v1 1/2] dt-bindings: media: rockchip-rga: add rockchip,rk3588-rga
From: Conor Dooley @ 2024-03-22 18:02 UTC (permalink / raw)
  To: Jianfeng Liu
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	linux-media, robh, krzysztof.kozlowski+dt, conor+dt, heiko,
	jacob-chen, ezequiel, mchehab, sfr
In-Reply-To: <20240322052915.3507937-2-liujianfeng1994@gmail.com>


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

On Fri, Mar 22, 2024 at 01:29:14PM +0800, Jianfeng Liu wrote:
> Add a new compatible for the rk3588 Rockchip SoC, which also features an
> RGA, which is called RGA2 in the TRM Part2. It is the same core as used
> on the rk3288 and rk3568, which documents the same RGA2.
> 
> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

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

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

^ permalink raw reply

* Re: [PATCH 2/5] mfd: rk8xx: Add RK816 support
From: kernel test robot @ 2024-03-22 17:58 UTC (permalink / raw)
  To: Alex Bee, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heiko Stuebner, Linus Walleij, Liam Girdwood,
	Mark Brown
  Cc: oe-kbuild-all, Chris Zhong, Zhang Qing, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel, linux-gpio,
	Alex Bee
In-Reply-To: <20240321143911.90210-5-knaerzche@gmail.com>

Hi Alex,

kernel test robot noticed the following build warnings:

[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on lee-mfd/for-mfd-fixes broonie-regulator/for-next robh/for-next linus/master v6.8 next-20240322]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Alex-Bee/dt-bindings-mfd-Add-rk816-binding/20240321-224318
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link:    https://lore.kernel.org/r/20240321143911.90210-5-knaerzche%40gmail.com
patch subject: [PATCH 2/5] mfd: rk8xx: Add RK816 support
config: i386-randconfig-062-20240322 (https://download.01.org/0day-ci/archive/20240323/202403230131.AhHTZiEx-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240323/202403230131.AhHTZiEx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403230131.AhHTZiEx-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/mfd/rk8xx-core.c:573:24: sparse: sparse: incorrect type in initializer (different modifiers) @@     expected unsigned int ( *get_irq_reg )( ... ) @@     got unsigned int ( const * )( ... ) @@
   drivers/mfd/rk8xx-core.c:573:24: sparse:     expected unsigned int ( *get_irq_reg )( ... )
   drivers/mfd/rk8xx-core.c:573:24: sparse:     got unsigned int ( const * )( ... )

vim +573 drivers/mfd/rk8xx-core.c

   567	
   568	static const struct regmap_irq_chip rk816_irq_chip = {
   569		.name = "rk816",
   570		.irqs = rk816_irqs,
   571		.num_irqs = ARRAY_SIZE(rk816_irqs),
   572		.num_regs = 3,
 > 573		.get_irq_reg = rk816_get_irq_reg,
   574		.status_base = RK816_INT_STS_REG1,
   575		.mask_base = RK816_INT_STS_MSK_REG1,
   576		.ack_base = RK816_INT_STS_REG1,
   577		.init_ack_masked = true,
   578	};
   579	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
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 5/5] KVM: arm64: Exclude FP ownership from kvm_vcpu_arch
From: Mark Brown @ 2024-03-22 17:52 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: kvmarm, linux-arm-kernel, kvm, James Morse, Suzuki K Poulose,
	Oliver Upton, Zenghui Yu, James Clark, Anshuman Khandual,
	Dongli Zhang
In-Reply-To: <20240322170945.3292593-6-maz@kernel.org>


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

On Fri, Mar 22, 2024 at 05:09:45PM +0000, Marc Zyngier wrote:
> In retrospect, it is fairly obvious that the FP state ownership
> is only meaningful for a given CPU, and that locating this
> information in the vcpu was just a mistake.
> 
> Move the ownership tracking into the host data structure, and
> rename it from fp_state to fp_owner, which is a better description
> (name suggested by Mark Brown).

There's still the thing with the interaction with SME support - to
summarise what I think you're asking for the userspace ABI there:

 - Create a requirement for userspace to set SVCR prior to setting any
   vector impacted register to ensure the correct format and that data
   isn't zeroed when SVCR is set.
 - Use the value of SVCR.SM and the guest maximum SVE and SME VLs to
   select the currently visible vector length for the Z, P and FFR
   registers, and if FFR can be accessed if not available in streaming
   mode.
 - Changes to SVCR.SM zero register data in the same way writes to the
   physical register do.
 - This also implies discarding or failing all writes to ZA and ZT0
   unless SVCR.ZA is set for consistency.
 - Add support for the V registers in the sysreg interface when SVE is
   enabled.

then the implementation can do what it likes to achieve that, the most
obvious thing being to store in native format for the current hardware
mode based on SVCR.{SM,ZA}.  Does that sound about right?

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

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

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

^ permalink raw reply

* Re: [PATCH v5 03/27] iommu/arm-smmu-v3: Add a type for the CD entry
From: Mostafa Saleh @ 2024-03-22 17:52 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
	Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
	Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <3-v5-9a37e0c884ce+31e3-smmuv3_newapi_p2_jgg@nvidia.com>

On Mon, Mar 04, 2024 at 07:43:51PM -0400, Jason Gunthorpe wrote:
> Instead of passing a naked __le16 * around to represent a CD table entry
> wrap it in a "struct arm_smmu_cd" with an array of the correct size. This
> makes it much clearer which functions will comprise the "CD API".
> 
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 20 +++++++++++---------
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  7 ++++++-
>  2 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 9e9233331c4636..c60b067c1f553e 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1219,7 +1219,8 @@ static void arm_smmu_write_cd_l1_desc(__le64 *dst,
>  	WRITE_ONCE(*dst, cpu_to_le64(val));
>  }
>  
> -static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
> +static struct arm_smmu_cd *arm_smmu_get_cd_ptr(struct arm_smmu_master *master,
> +					       u32 ssid)
>  {
>  	__le64 *l1ptr;
>  	unsigned int idx;
> @@ -1228,7 +1229,8 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
>  	struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table;
>  
>  	if (cd_table->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
> -		return cd_table->cdtab + ssid * CTXDESC_CD_DWORDS;
> +		return (struct arm_smmu_cd *)(cd_table->cdtab +
> +					      ssid * CTXDESC_CD_DWORDS);
>  
>  	idx = ssid >> CTXDESC_SPLIT;
>  	l1_desc = &cd_table->l1_desc[idx];
> @@ -1242,7 +1244,7 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
>  		arm_smmu_sync_cd(master, ssid, false);
>  	}
>  	idx = ssid & (CTXDESC_L2_ENTRIES - 1);
> -	return l1_desc->l2ptr + idx * CTXDESC_CD_DWORDS;
> +	return &l1_desc->l2ptr[idx];
>  }
>  
>  int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
> @@ -1261,7 +1263,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
>  	 */
>  	u64 val;
>  	bool cd_live;
> -	__le64 *cdptr;
> +	struct arm_smmu_cd *cdptr;
>  	struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table;
>  	struct arm_smmu_device *smmu = master->smmu;
>  
> @@ -1272,7 +1274,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
>  	if (!cdptr)
>  		return -ENOMEM;
>  
> -	val = le64_to_cpu(cdptr[0]);
> +	val = le64_to_cpu(cdptr->data[0]);
>  	cd_live = !!(val & CTXDESC_CD_0_V);
>  
>  	if (!cd) { /* (5) */
> @@ -1289,9 +1291,9 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
>  		 * this substream's traffic
>  		 */
>  	} else { /* (1) and (2) */
> -		cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
> -		cdptr[2] = 0;
> -		cdptr[3] = cpu_to_le64(cd->mair);
> +		cdptr->data[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
> +		cdptr->data[2] = 0;
> +		cdptr->data[3] = cpu_to_le64(cd->mair);
>  
>  		/*
>  		 * STE may be live, and the SMMU might read dwords of this CD in any
> @@ -1323,7 +1325,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
>  	 *   field within an aligned 64-bit span of a structure can be altered
>  	 *   without first making the structure invalid.
>  	 */
> -	WRITE_ONCE(cdptr[0], cpu_to_le64(val));
> +	WRITE_ONCE(cdptr->data[0], cpu_to_le64(val));
>  	arm_smmu_sync_cd(master, ssid, true);
>  	return 0;
>  }
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 23baf117e7e4b5..7078ed569fd4d3 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -282,6 +282,11 @@ struct arm_smmu_ste {
>  #define CTXDESC_L1_DESC_L2PTR_MASK	GENMASK_ULL(51, 12)
>  
>  #define CTXDESC_CD_DWORDS		8
> +
> +struct arm_smmu_cd {
> +	__le64 data[CTXDESC_CD_DWORDS];
> +};
> +
>  #define CTXDESC_CD_0_TCR_T0SZ		GENMASK_ULL(5, 0)
>  #define CTXDESC_CD_0_TCR_TG0		GENMASK_ULL(7, 6)
>  #define CTXDESC_CD_0_TCR_IRGN0		GENMASK_ULL(9, 8)
> @@ -591,7 +596,7 @@ struct arm_smmu_ctx_desc {
>  };
>  
>  struct arm_smmu_l1_ctx_desc {
> -	__le64				*l2ptr;
> +	struct arm_smmu_cd		*l2ptr;
>  	dma_addr_t			l2ptr_dma;
>  };
>  
> -- 
> 2.43.2
>

Reviewed-by: Mostafa Saleh <smostafa@google.com>

Thanks,
Mostafa

_______________________________________________
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 v5 02/27] iommu/arm-smmu-v3: Do not ATC invalidate the entire domain
From: Mostafa Saleh @ 2024-03-22 17:51 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
	Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
	Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <2-v5-9a37e0c884ce+31e3-smmuv3_newapi_p2_jgg@nvidia.com>

Hi Jason,

On Mon, Mar 04, 2024 at 07:43:50PM -0400, Jason Gunthorpe wrote:
> At this point we know which master we are going to change the PCI config
> on, this is the only device we need to invalidate. Switch
> arm_smmu_atc_inv_domain() for arm_smmu_atc_inv_master().
> 
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index d1bc151a5dff8c..9e9233331c4636 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2447,7 +2447,10 @@ static void arm_smmu_enable_ats(struct arm_smmu_master *master,
>  	pdev = to_pci_dev(master->dev);
>  
>  	atomic_inc(&smmu_domain->nr_ats_masters);
> -	arm_smmu_atc_inv_domain(smmu_domain, IOMMU_NO_PASID, 0, 0);
> +	/*
> +	 * ATC invalidation of PASID 0 causes the entire ATC to be flushed.
> +	 */
> +	arm_smmu_atc_inv_master(master);
>  	if (pci_enable_ats(pdev, stu))
>  		dev_err(master->dev, "Failed to enable ATS (STU %zu)\n", stu);
>  }
> -- 
> 2.43.2

Reviewed-by: Mostafa Saleh <smostafa@google.com>

Thanks,
Mostafa

_______________________________________________
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 v5 01/27] iommu/arm-smmu-v3: Do not allow a SVA domain to be set on the wrong PASID
From: Mostafa Saleh @ 2024-03-22 17:48 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
	Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
	Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <1-v5-9a37e0c884ce+31e3-smmuv3_newapi_p2_jgg@nvidia.com>

Hi Jason,

On Mon, Mar 04, 2024 at 07:43:49PM -0400, Jason Gunthorpe wrote:
> The SVA code is wired to assume that the SVA is programmed onto the
> mm->pasid. The current core code always does this, so it is fine.
> 
> Add a check for clarity.
> 
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> index 2610e82c0ecd0d..347c2fdd865c1a 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> @@ -581,6 +581,9 @@ static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
>  	int ret = 0;
>  	struct mm_struct *mm = domain->mm;
>  
> +	if (mm_get_enqcmd_pasid(mm) != id)
> +		return -EINVAL;
> +
I am not sure if that is needed, the only caller in the tree is the IOMMU code
and it does the right thing, as that check is removed later anyway, I don’t
think this patch adds much.

>  	mutex_lock(&sva_lock);
>  	ret = __arm_smmu_sva_bind(dev, id, mm);
>  	mutex_unlock(&sva_lock);
> -- 
> 2.43.2

Thanks,
Mostafa

_______________________________________________
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 1/4] dt-bindings: soc: mediatek: Add support for MT8188 VPPSYS
From: Conor Dooley @ 2024-03-22 17:42 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: linux-media, mchehab, robh, krzysztof.kozlowski+dt, conor+dt,
	matthias.bgg, amergnat, moudy.ho, hverkuil-cisco,
	sebastian.fricke, u.kleine-koenig, chunkuang.hu, p.zabel,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek,
	kernel
In-Reply-To: <20240322092845.381313-2-angelogioacchino.delregno@collabora.com>


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

On Fri, Mar 22, 2024 at 10:28:42AM +0100, AngeloGioacchino Del Regno wrote:
> Add compatible for MT8188 VPP mutex.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

You should at least mention the difference between this any anything
else.
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> ---
>  .../devicetree/bindings/soc/mediatek/mediatek,mutex.yaml         | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
> index ba2014a8725c..a10326a9683d 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
> @@ -33,6 +33,7 @@ properties:
>        - mediatek,mt8186-disp-mutex
>        - mediatek,mt8186-mdp3-mutex
>        - mediatek,mt8188-disp-mutex
> +      - mediatek,mt8188-vpp-mutex
>        - mediatek,mt8192-disp-mutex
>        - mediatek,mt8195-disp-mutex
>        - mediatek,mt8195-vpp-mutex
> -- 
> 2.44.0
> 

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

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

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

^ permalink raw reply

* Re: [PATCH 3/4] dt-bindings: media: mediatek: mdp3: Add support for MT8188 RDMA
From: Conor Dooley @ 2024-03-22 17:40 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: linux-media, mchehab, robh, krzysztof.kozlowski+dt, conor+dt,
	matthias.bgg, amergnat, moudy.ho, hverkuil-cisco,
	sebastian.fricke, u.kleine-koenig, chunkuang.hu, p.zabel,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek,
	kernel
In-Reply-To: <20240322092845.381313-4-angelogioacchino.delregno@collabora.com>


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

On Fri, Mar 22, 2024 at 10:28:44AM +0100, AngeloGioacchino Del Regno wrote:
> Add a compatible for MediaTek MT8188 RDMA, which supports only a
> subset of the MDP3 components of its similar MT8195 counterpart.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> ---
>  Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> index 59db8306485b..18603f6c5e06 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> @@ -23,6 +23,7 @@ properties:
>      oneOf:
>        - enum:
>            - mediatek,mt8183-mdp3-rdma
> +          - mediatek,mt8188-mdp3-rdma
>            - mediatek,mt8195-mdp3-rdma
>            - mediatek,mt8195-vdo1-rdma
>        - items:
> -- 
> 2.44.0
> 

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

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

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

^ permalink raw reply

* Re: [RFC PATCH v3 0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation
From: David Woodhouse @ 2024-03-22 17:33 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Oliver Upton, linux-arm-kernel, kvm, Paolo Bonzini,
	Jonathan Corbet, James Morse, Suzuki K Poulose, Zenghui Yu,
	Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
	Rafael J. Wysocki, Len Brown, Pavel Machek, Mostafa Saleh,
	Jean-Philippe Brucker, linux-doc, linux-kernel, kvmarm, linux-pm
In-Reply-To: <86frwiz1rs.wl-maz@kernel.org>


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

On Fri, 2024-03-22 at 16:09 +0000, Marc Zyngier wrote:
> 
> > Marc, I think I've also addressed your feedback? Is there anything else
> > to do other than wait for the spec to be published?
> 
> Other than the couple of minor nits I mentioned in replies to the
> individual patches, this looks good to me.

I believe I've handled all that. And also Sudeep's implicit nudge to
use BIT() instead of manually shifting (1<<PSCI_1_3_HIBERNATE_TYPE_OFF).

Rebased onto 6.8 and pushed to
https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/psci-hibernate-6.8

> > Shall I post a v4 with PSCI v1.3 as default and the self-test? Would
> > you apply that into a branch ready for merging when the spec is ready,
> > or should I just wait and repost it all then?
> 
> I think this can wait for the final spec. I assume that you are
> directly tracking this anyway, so we don't need to poll for the spec
> update.

Indeed, will post again when the spec is published. Thanks.

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

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

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

^ permalink raw reply

* Re: [PATCH v3 03/12] mm: Make HPAGE_PXD_* macros even if !THP
From: SeongJae Park @ 2024-03-22 17:14 UTC (permalink / raw)
  To: peterx
  Cc: linux-mm, linux-kernel, linuxppc-dev, Michael Ellerman,
	Christophe Leroy, Matthew Wilcox, Rik van Riel, Lorenzo Stoakes,
	Axel Rasmussen, Yang Shi, John Hubbard, linux-arm-kernel,
	Kirill A . Shutemov, Andrew Jones, Vlastimil Babka, Mike Rapoport,
	Andrew Morton, Muchun Song, Christoph Hellwig, linux-riscv,
	James Houghton, David Hildenbrand, Jason Gunthorpe,
	Andrea Arcangeli, Aneesh Kumar K . V, Mike Kravetz
In-Reply-To: <20240321220802.679544-4-peterx@redhat.com>

Hi Peter,

On Thu, 21 Mar 2024 18:07:53 -0400 peterx@redhat.com wrote:

> From: Peter Xu <peterx@redhat.com>
> 
> These macros can be helpful when we plan to merge hugetlb code into generic
> code.  Move them out and define them even if !THP.
> 
> We actually already defined HPAGE_PMD_NR for other reasons even if !THP.
> Reorganize these macros.
> 
> Reviewed-by: Christoph Hellwig <hch@infradead.org>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  include/linux/huge_mm.h | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index de0c89105076..3bcdfc7e5d57 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -64,9 +64,6 @@ ssize_t single_hugepage_flag_show(struct kobject *kobj,
>  				  enum transparent_hugepage_flag flag);
>  extern struct kobj_attribute shmem_enabled_attr;
>  
> -#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
> -#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
> -
>  /*
>   * Mask of all large folio orders supported for anonymous THP; all orders up to
>   * and including PMD_ORDER, except order-0 (which is not "huge") and order-1
> @@ -87,14 +84,19 @@ extern struct kobj_attribute shmem_enabled_attr;
>  #define thp_vma_allowable_order(vma, vm_flags, smaps, in_pf, enforce_sysfs, order) \
>  	(!!thp_vma_allowable_orders(vma, vm_flags, smaps, in_pf, enforce_sysfs, BIT(order)))
>  
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  #define HPAGE_PMD_SHIFT PMD_SHIFT
>  #define HPAGE_PMD_SIZE	((1UL) << HPAGE_PMD_SHIFT)
>  #define HPAGE_PMD_MASK	(~(HPAGE_PMD_SIZE - 1))
> +#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
> +#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
>  
>  #define HPAGE_PUD_SHIFT PUD_SHIFT
>  #define HPAGE_PUD_SIZE	((1UL) << HPAGE_PUD_SHIFT)
>  #define HPAGE_PUD_MASK	(~(HPAGE_PUD_SIZE - 1))
> +#define HPAGE_PUD_ORDER (HPAGE_PUD_SHIFT-PAGE_SHIFT)
> +#define HPAGE_PUD_NR (1<<HPAGE_PUD_ORDER)

I just found latest mm-unstable fails one of my build configurations[1] with
below error.  'git bisect' says this is the first patch set started the
failure.  I haven't looked in deep, but just reporting first.

    In file included from .../include/linux/mm.h:1115,
                     from .../mm/vmstat.c:14:
    .../mm/vmstat.c: In function 'zoneinfo_show_print':
    .../include/linux/huge_mm.h:87:25: error: 'PMD_SHIFT' undeclared (first use in this function); did you mean 'PUD_SHIFT'?
       87 | #define HPAGE_PMD_SHIFT PMD_SHIFT
          |                         ^~~~~~~~~

[1] https://github.com/awslabs/damon-tests/blob/next/corr/tests/build_m68k.sh


Thanks,
SJ

[...]

_______________________________________________
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 53/64] i2c: st: reword according to newest specification
From: Easwar Hariharan @ 2024-03-22 17:11 UTC (permalink / raw)
  To: Wolfram Sang, linux-i2c
  Cc: Patrice Chotard, Andi Shyti, linux-arm-kernel, linux-kernel
In-Reply-To: <20240322132619.6389-54-wsa+renesas@sang-engineering.com>

On 3/22/2024 6:25 AM, Wolfram Sang wrote:
> Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C
> specifications and replace "master/slave" with more appropriate terms.
> They are also more specific because we distinguish now between a remote
> entity ("client") and a local one ("target").
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/i2c/busses/i2c-st.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-st.c b/drivers/i2c/busses/i2c-st.c
> index ce2333408904..9bd45ae83c0c 100644
> --- a/drivers/i2c/busses/i2c-st.c
> +++ b/drivers/i2c/busses/i2c-st.c
> @@ -2,7 +2,7 @@
>  /*
>   * Copyright (C) 2013 STMicroelectronics
>   *
> - * I2C master mode controller driver, used in STMicroelectronics devices.
> + * I2C host controller driver, used in STMicroelectronics devices.
>   *
>   * Author: Maxime Coquelin <maxime.coquelin@st.com>
>   */
> @@ -150,7 +150,7 @@ struct st_i2c_timings {
>  
>  /**
>   * struct st_i2c_client - client specific data
> - * @addr: 8-bit slave addr, including r/w bit
> + * @addr: 8-bit client addr, including r/w bit
>   * @count: number of bytes to be transfered
>   * @xfered: number of bytes already transferred
>   * @buf: data buffer
> @@ -647,7 +647,7 @@ static int st_i2c_xfer_msg(struct st_i2c_dev *i2c_dev, struct i2c_msg *msg,
>  {
>  	struct st_i2c_client *c = &i2c_dev->client;
>  	u32 ctl, i2c, it;
> -	unsigned long timeout;
> +	unsigned long time_left;

Thanks for doing this. Is the timeout v/s time_left language also due to the specification change?
A link to the specification(s) in the commit message would be nice to have

>  	int ret;
>  
>  	c->addr		= i2c_8bit_addr_from_msg(msg);
> @@ -667,7 +667,7 @@ static int st_i2c_xfer_msg(struct st_i2c_dev *i2c_dev, struct i2c_msg *msg,
>  		i2c |= SSC_I2C_ACKG;
>  	st_i2c_set_bits(i2c_dev->base + SSC_I2C, i2c);
>  
> -	/* Write slave address */
> +	/* Write client address */
>  	st_i2c_write_tx_fifo(i2c_dev, c->addr);
>  
>  	/* Pre-fill Tx fifo with data in case of write */
> @@ -685,15 +685,12 @@ static int st_i2c_xfer_msg(struct st_i2c_dev *i2c_dev, struct i2c_msg *msg,
>  		st_i2c_set_bits(i2c_dev->base + SSC_I2C, SSC_I2C_STRTG);
>  	}
>  
> -	timeout = wait_for_completion_timeout(&i2c_dev->complete,
> +	time_left = wait_for_completion_timeout(&i2c_dev->complete,
>  			i2c_dev->adap.timeout);
>  	ret = c->result;
>  
> -	if (!timeout) {
> -		dev_err(i2c_dev->dev, "Write to slave 0x%x timed out\n",
> -				c->addr);
> +	if (!time_left)
>  		ret = -ETIMEDOUT;
> -	}

Why did we lost the dev_err() here?

>  
>  	i2c = SSC_I2C_STOPG | SSC_I2C_REPSTRTG;
>  	st_i2c_clr_bits(i2c_dev->base + SSC_I2C, i2c);
> @@ -769,7 +766,7 @@ static u32 st_i2c_func(struct i2c_adapter *adap)
>  }
>  
>  static const struct i2c_algorithm st_i2c_algo = {
> -	.master_xfer = st_i2c_xfer,
> +	.xfer = st_i2c_xfer,
>  	.functionality = st_i2c_func,
>  };
>  


_______________________________________________
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 5/5] KVM: arm64: Exclude FP ownership from kvm_vcpu_arch
From: Marc Zyngier @ 2024-03-22 17:09 UTC (permalink / raw)
  To: kvmarm, linux-arm-kernel, kvm
  Cc: James Morse, Suzuki K Poulose, Oliver Upton, Zenghui Yu,
	James Clark, Anshuman Khandual, Mark Brown, Dongli Zhang
In-Reply-To: <20240322170945.3292593-1-maz@kernel.org>

In retrospect, it is fairly obvious that the FP state ownership
is only meaningful for a given CPU, and that locating this
information in the vcpu was just a mistake.

Move the ownership tracking into the host data structure, and
rename it from fp_state to fp_owner, which is a better description
(name suggested by Mark Brown).

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/kvm_emulate.h    |  4 ++--
 arch/arm64/include/asm/kvm_host.h       | 14 +++++++-------
 arch/arm64/kvm/arm.c                    |  6 ------
 arch/arm64/kvm/fpsimd.c                 | 10 +++++-----
 arch/arm64/kvm/hyp/include/hyp/switch.h |  6 +++---
 arch/arm64/kvm/hyp/nvhe/hyp-main.c      |  2 --
 arch/arm64/kvm/hyp/nvhe/switch.c        |  2 +-
 arch/arm64/kvm/hyp/vhe/switch.c         |  2 +-
 8 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index debc3753d2ef..b17f2269fb81 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -594,7 +594,7 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu)
 		val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN);
 
 		if (!vcpu_has_sve(vcpu) ||
-		    (vcpu->arch.fp_state != FP_STATE_GUEST_OWNED))
+		    (*host_data_ptr(fp_owner) != FP_STATE_GUEST_OWNED))
 			val |= CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN;
 		if (cpus_have_final_cap(ARM64_SME))
 			val |= CPACR_EL1_SMEN_EL1EN | CPACR_EL1_SMEN_EL0EN;
@@ -602,7 +602,7 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu)
 		val = CPTR_NVHE_EL2_RES1;
 
 		if (vcpu_has_sve(vcpu) &&
-		    (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED))
+		    (*host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED))
 			val |= CPTR_EL2_TZ;
 		if (cpus_have_final_cap(ARM64_SME))
 			val &= ~CPTR_EL2_TSM;
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 838cdee2ecf7..951303e976de 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -545,6 +545,13 @@ struct kvm_host_data {
 	struct kvm_cpu_context host_ctxt;
 	struct user_fpsimd_state *fpsimd_state;	/* hyp VA */
 
+	/* Ownership of the FP regs */
+	enum {
+		FP_STATE_FREE,
+		FP_STATE_HOST_OWNED,
+		FP_STATE_GUEST_OWNED,
+	} fp_owner;
+
 	/*
 	 * host_debug_state contains the host registers which are
 	 * saved and restored during world switches.
@@ -621,13 +628,6 @@ struct kvm_vcpu_arch {
 	/* Exception Information */
 	struct kvm_vcpu_fault_info fault;
 
-	/* Ownership of the FP regs */
-	enum {
-		FP_STATE_FREE,
-		FP_STATE_HOST_OWNED,
-		FP_STATE_GUEST_OWNED,
-	} fp_state;
-
 	/* Configuration flags, set once and for all before the vcpu can run */
 	u8 cflags;
 
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index a24287c3ba99..66d8112da268 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -378,12 +378,6 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 
 	vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;
 
-	/*
-	 * Default value for the FP state, will be overloaded at load
-	 * time if we support FP (pretty likely)
-	 */
-	vcpu->arch.fp_state = FP_STATE_FREE;
-
 	/* Set up the timer */
 	kvm_timer_vcpu_init(vcpu);
 
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index e6bd99358615..98f8b9272e24 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -84,7 +84,7 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
 	 * guest in kvm_arch_vcpu_ctxflush_fp() and override this to
 	 * FP_STATE_FREE if the flag set.
 	 */
-	vcpu->arch.fp_state = FP_STATE_HOST_OWNED;
+	*host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED;
 	*host_data_ptr(fpsimd_state) = kern_hyp_va(&current->thread.uw.fpsimd_state);
 
 	vcpu_clear_flag(vcpu, HOST_SVE_ENABLED);
@@ -109,7 +109,7 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
 		 * been saved, this is very unlikely to happen.
 		 */
 		if (read_sysreg_s(SYS_SVCR) & (SVCR_SM_MASK | SVCR_ZA_MASK)) {
-			vcpu->arch.fp_state = FP_STATE_FREE;
+			*host_data_ptr(fp_owner) = FP_STATE_FREE;
 			fpsimd_save_and_flush_cpu_state();
 		}
 	}
@@ -125,7 +125,7 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
 void kvm_arch_vcpu_ctxflush_fp(struct kvm_vcpu *vcpu)
 {
 	if (test_thread_flag(TIF_FOREIGN_FPSTATE))
-		vcpu->arch.fp_state = FP_STATE_FREE;
+		*host_data_ptr(fp_owner) = FP_STATE_FREE;
 }
 
 /*
@@ -141,7 +141,7 @@ void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu)
 
 	WARN_ON_ONCE(!irqs_disabled());
 
-	if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED) {
+	if (*host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED) {
 
 		/*
 		 * Currently we do not support SME guests so SVCR is
@@ -194,7 +194,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 		isb();
 	}
 
-	if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED) {
+	if (*host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED) {
 		if (vcpu_has_sve(vcpu)) {
 			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
 
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 6def6ad8dd48..2629420d0659 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -42,7 +42,7 @@ extern struct kvm_exception_table_entry __stop___kvm_ex_table;
 /* Check whether the FP regs are owned by the guest */
 static inline bool guest_owns_fp_regs(struct kvm_vcpu *vcpu)
 {
-	return vcpu->arch.fp_state == FP_STATE_GUEST_OWNED;
+	return *host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED;
 }
 
 /* Save the 32-bit only FPSIMD system register state */
@@ -376,7 +376,7 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
 	isb();
 
 	/* Write out the host state if it's in the registers */
-	if (vcpu->arch.fp_state == FP_STATE_HOST_OWNED)
+	if (*host_data_ptr(fp_owner) == FP_STATE_HOST_OWNED)
 		__fpsimd_save_state(*host_data_ptr(fpsimd_state));
 
 	/* Restore the guest state */
@@ -389,7 +389,7 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
 	if (!(read_sysreg(hcr_el2) & HCR_RW))
 		write_sysreg(__vcpu_sys_reg(vcpu, FPEXC32_EL2), fpexc32_el2);
 
-	vcpu->arch.fp_state = FP_STATE_GUEST_OWNED;
+	*host_data_ptr(fp_owner) = FP_STATE_GUEST_OWNED;
 
 	return true;
 }
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index c5f625dc1f07..26561c562f7a 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -39,7 +39,6 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 	hyp_vcpu->vcpu.arch.cptr_el2	= host_vcpu->arch.cptr_el2;
 
 	hyp_vcpu->vcpu.arch.iflags	= host_vcpu->arch.iflags;
-	hyp_vcpu->vcpu.arch.fp_state	= host_vcpu->arch.fp_state;
 
 	hyp_vcpu->vcpu.arch.debug_ptr	= kern_hyp_va(host_vcpu->arch.debug_ptr);
 
@@ -63,7 +62,6 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 	host_vcpu->arch.fault		= hyp_vcpu->vcpu.arch.fault;
 
 	host_vcpu->arch.iflags		= hyp_vcpu->vcpu.arch.iflags;
-	host_vcpu->arch.fp_state	= hyp_vcpu->vcpu.arch.fp_state;
 
 	host_cpu_if->vgic_hcr		= hyp_cpu_if->vgic_hcr;
 	for (i = 0; i < hyp_cpu_if->used_lrs; ++i)
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 544a419b9a39..1f82d531a494 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -337,7 +337,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 
 	__sysreg_restore_state_nvhe(host_ctxt);
 
-	if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED)
+	if (*host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED)
 		__fpsimd_save_fpexc32(vcpu);
 
 	__debug_switch_to_host(vcpu);
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 14b7a6bc5909..b92f9fe2d50e 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -258,7 +258,7 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
 	sysreg_restore_host_state_vhe(host_ctxt);
 
-	if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED)
+	if (*host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED)
 		__fpsimd_save_fpexc32(vcpu);
 
 	__debug_switch_to_host(vcpu);
-- 
2.39.2


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

^ permalink raw reply related


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