Re: [RFT PATCH v3 13/27] arm64: Add Apple vendor-specific system registers

[Will Deacon <will@kernel.org>]

On Fri, Mar 05, 2021 at 06:38:48AM +0900, Hector Martin wrote:
> Apple ARM64 SoCs have a ton of vendor-specific registers we're going to
> have to deal with, and those don't really belong in sysreg.h with all
> the architectural registers. Make a new home for them, and add some
> registers which are useful for early bring-up.
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  MAINTAINERS                           |  1 +
>  arch/arm64/include/asm/sysreg_apple.h | 69 +++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+)
>  create mode 100644 arch/arm64/include/asm/sysreg_apple.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aec14fbd61b8..3a352c687d4b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1646,6 +1646,7 @@ B:	https://github.com/AsahiLinux/linux/issues
>  C:	irc://chat.freenode.net/asahi-dev
>  T:	git https://github.com/AsahiLinux/linux.git
>  F:	Documentation/devicetree/bindings/arm/apple.yaml
> +F:	arch/arm64/include/asm/sysreg_apple.h

(this isn't needed with my suggestion below).

>  ARM/ARTPEC MACHINE SUPPORT
>  M:	Jesper Nilsson <jesper.nilsson@axis.com>
> diff --git a/arch/arm64/include/asm/sysreg_apple.h b/arch/arm64/include/asm/sysreg_apple.h
> new file mode 100644
> index 000000000000..48347a51d564
> --- /dev/null
> +++ b/arch/arm64/include/asm/sysreg_apple.h

I doubt apple are the only folks doing this, so can we instead have
sysreg-impdef.h please, and then have an Apple section in there for these
registers? That way, we could also have an imp_sys_reg() macro to limit
CRn to 11 or 15, which is the reserved encoding space for these registers.

We'll cc you for any patches touching the Apple parts, as we don't have
the first clue about what's hiding in there.

> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Apple SoC vendor-defined system register definitions
> + *
> + * Copyright The Asahi Linux Contributors
> +
> + * This file contains only well-understood registers that are useful to
> + * Linux. If you are looking for things to add here, you should visit:
> + *
> + * https://github.com/AsahiLinux/docs/wiki/HW:ARM-System-Registers
> + */
> +
> +#ifndef __ASM_SYSREG_APPLE_H
> +#define __ASM_SYSREG_APPLE_H
> +
> +#include <asm/sysreg.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +
> +/*
> + * Keep these registers in encoding order, except for register arrays;
> + * those should be listed in array order starting from the position of
> + * the encoding of the first register.
> + */
> +
> +#define SYS_APL_PMCR0_EL1		sys_reg(3, 1, 15, 0, 0)
> +#define PMCR0_IMODE			GENMASK(10, 8)
> +#define PMCR0_IMODE_OFF			0
> +#define PMCR0_IMODE_PMI			1
> +#define PMCR0_IMODE_AIC			2
> +#define PMCR0_IMODE_HALT		3
> +#define PMCR0_IMODE_FIQ			4
> +#define PMCR0_IACT			BIT(11)

The Arm ARM says this about imp-def sysregs:

  | The Arm architecture guarantees not to define any register name
  | prefixed with IMP_ as part of the standard Arm architecture.
  |
  | Note
  | Arm strongly recommends that any register names created in the
  | IMPLEMENTATION DEFINED register spaces be prefixed with IMP_ and
  | postfixed with _ELx, where appropriate.

and it seems like we could follow that here without much effort, if you
don't mind.

Will