[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3815d07a-ae29-404f-a624-4b18c84aa02e@kernel.org>
Date: Mon, 16 Sep 2024 08:42:50 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: arturs.artamonovs@...log.com, Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Greg Malysa <greg.malysa@...esys.com>,
Philipp Zabel <p.zabel@...gutronix.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Utsav Agarwal <Utsav.Agarwal@...log.com>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <brgl@...ev.pl>, Thomas Gleixner <tglx@...utronix.de>,
Andi Shyti <andi.shyti@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jirislaby@...nel.org>, Arnd Bergmann <arnd@...db.de>,
Olof Johansson <olof@...om.net>, soc@...nel.org
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
linux-gpio@...r.kernel.org, linux-i2c@...r.kernel.org,
linux-serial@...r.kernel.org, adsp-linux@...log.com,
Nathan Barrett-Morrison <nathan.morrison@...esys.com>
Subject: Re: [PATCH 01/21] arm64: Add ADI ADSP-SC598 SoC
On 12/09/2024 20:24, Arturs Artamonovs via B4 Relay wrote:
> From: Arturs Artamonovs <arturs.artamonovs@...log.com>
>
> Add ADSP-SC598 platform.
>
> Signed-off-by: Arturs Artamonovs <Arturs.Artamonovs@...log.com>
> Co-developed-by: Utsav Agarwal <Utsav.Agarwal@...log.com>
> Signed-off-by: Utsav Agarwal <Utsav.Agarwal@...log.com>
> Co-developed-by: Nathan Barrett-Morrison <nathan.morrison@...esys.com>
> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@...esys.com>
> Co-developed-by: Greg Malysa <greg.malysa@...esys.com>
> Signed-off-by: Greg Malysa <greg.malysa@...esys.com>
> ---
> arch/arm64/Kconfig.platforms | 13 ++
> drivers/soc/Makefile | 1 +
> drivers/soc/adi/Makefile | 5 +
> drivers/soc/adi/system.c | 257 +++++++++++++++++++++++++++++++++
> include/linux/soc/adi/adsp-gpio-port.h | 85 +++++++++++
> include/linux/soc/adi/cpu.h | 107 ++++++++++++++
> include/linux/soc/adi/rcu.h | 55 +++++++
> include/linux/soc/adi/sc59x.h | 147 +++++++++++++++++++
> include/linux/soc/adi/system_config.h | 65 +++++++++
> 9 files changed, 735 insertions(+)
>
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 6c6d11536b42ec6e878db8d355c17994c2500d7b..b9ea22ecddfcbff98486a314143e52934f26df44 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -292,6 +292,19 @@ config ARCH_ROCKCHIP
> This enables support for the ARMv8 based Rockchip chipsets,
> like the RK3368.
>
> +config ARCH_SC59X_64
> + bool "ADI 64-bit SC59X Platforms"
> + select TIMER_OF
> + select GPIOLIB
> + select PINCTRL
> + select COMMON_CLK_ADI_SC598
> + select PINCTRL_ADSP
> + select ADI_ADSP_IRQ
> + select COUNTER
> + help
> + This enables support for Analog Devices Incorporated's
> + Family of ARM64 DSP processors
Messed indentation.
> +
> +
> +static int regmap_system_write(void *context, unsigned int reg,
> + unsigned int val)
> +{
> + struct adi_system_context *ctx = context;
> + struct adi_system_register *sreg = radix_tree_lookup(&ctx->tree, reg);
> +
> + if (!sreg)
> + return -EIO;
> +
> + if (sreg->is_bits) {
> + return regmap_update_bits(ctx->regmap, sreg->offset,
> + sreg->mask,
> + (val << sreg->shift) & sreg->mask);
> + }
> +
> + return regmap_write(ctx->regmap, sreg->offset, val);
> +}
> +
> +static struct adi_system_context *create_context
> +(struct adi_system_config *config)
That's wrong wrapping. Wrapping happens after return type in such cases.
> +{
> + struct regmap *regmap = config->mmio_regmap;
> + struct adi_system_context *ctx;
> + size_t i;
> + int ret;
> +
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return ERR_PTR(-ENOMEM);
> +
> + ctx->regmap = regmap;
> + INIT_RADIX_TREE(&ctx->tree, GFP_KERNEL);
> +
> + for (i = 0; i < config->len; ++i) {
> + struct adi_system_register *sreg = &config->registers[i];
> +
> + ret = radix_tree_insert(&ctx->tree, sreg->id, sreg);
> + if (ret)
> + return ERR_PTR(ret);
> + }
> +
> + config->config.max_register = config->max_register;
> + config->config.reg_bits = 8 * sizeof(uint32_t);
> + config->config.val_bits = 8 * sizeof(uint32_t);
> + config->config.reg_stride = 1;
> +
> + return ctx;
> +}
> +
> +static void regmap_system_free_context(void *context)
> +{
> + struct adi_system_context *ctx = context;
> + unsigned int i;
> +
> + for (i = 0; i < ctx->config->len; ++i)
> + radix_tree_delete(&ctx->tree,
> + ctx->config->registers[i].id);
> +
> + kfree(ctx);
> +}
> +
> +static const struct regmap_bus regmap_system_bus = {
> + .fast_io = true,
> + .reg_write = regmap_system_write,
> + .reg_read = regmap_system_read,
> + .free_context = regmap_system_free_context,
> + .val_format_endian_default = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +static struct regmap *__devm_regmap_init_adi_system_config(struct device *dev,
> + struct adi_system_config *config,
> + struct lock_class_key *lock_key, const char *lock_name)
> +{
> + struct adi_system_context *ctx = create_context(config);
> +
> + if (IS_ERR(ctx))
> + return ERR_PTR(PTR_ERR(ctx));
> +
> + return __devm_regmap_init(dev, ®map_system_bus, ctx,
> + &config->config,
> + lock_key, lock_name);
> +}
> +
> +static DEFINE_SPINLOCK(adi_system_config_lock);
> +static LIST_HEAD(adi_system_config_list);
> +
> +static int adi_system_config_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct adi_system_config *config = &adi_pads_config;
> + struct device_node *np = dev->of_node;
> + struct regmap *regmap_mmio;
> + struct regmap *regmap_system;
> + struct resource *res;
> + void __iomem *base;
> + unsigned long flags;
> +
> + struct regmap_config mmio_config = {
> + .reg_bits = 8 * sizeof(uint32_t),
> + .val_bits = 8 * sizeof(uint32_t),
> + .reg_stride = sizeof(uint32_t),
> + };
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + base = devm_ioremap(dev, res->start, resource_size(res));
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + mmio_config.name = dev_name(dev);
> + mmio_config.max_register = resource_size(res) - sizeof(uint32_t);
> +
> + regmap_mmio = devm_regmap_init_mmio(dev, base, &mmio_config);
> + if (IS_ERR(regmap_mmio)) {
> + dev_err(dev, "mmio regmap initialization failed\n");
> + return PTR_ERR(regmap_mmio);
> + }
> +
> + config->mmio_regmap = regmap_mmio;
> + regmap_system = devm_regmap_init_adi_system_config(dev, config);
> + if (IS_ERR(regmap_system)) {
> + dev_err(dev, "system config regmap initialization failed\n");
> + return PTR_ERR(regmap_system);
> + }
> +
> + config->np = np;
> + config->system_regmap = regmap_system;
> + platform_set_drvdata(pdev, config);
> +
> + spin_lock_irqsave(&adi_system_config_lock, flags);
> + list_add_tail(&config->list, &adi_system_config_list);
> + spin_unlock_irqrestore(&adi_system_config_lock, flags);
> + return 0;
> +}
> +
> +static void adi_system_config_remove(struct platform_device *pdev)
> +{
> + struct adi_system_config *config = platform_get_drvdata(pdev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&adi_system_config_lock, flags);
> + list_del(&config->list);
> + spin_unlock_irqrestore(&adi_system_config_lock, flags);
> +}
> +
> +/*
> + * PADs configuration registers are required to configure peripherals,
> + * and by extension the system. Hence the driver focuses on driving them while
> + * also setting up the remaining system.
> + */
> +static const struct of_device_id pads_dt_ids[] = {
> + { .compatible = "adi,pads-peripheral-config", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, pads_dt_ids);
> +
> +static struct platform_driver pads_driver = {
> + .driver = {
> + .name = "adi-system-config",
> + .of_match_table = pads_dt_ids,
> + },
> + .probe = adi_system_config_probe,
> + .remove = adi_system_config_remove,
> +};
> +module_platform_driver(pads_driver);
> +
> +MODULE_AUTHOR("Greg Malysa <greg.malysa@...esys.com>");
> +MODULE_DESCRIPTION("ADI ADSP PADS CFG-based System Configuration Driver");
> +MODULE_LICENSE("GPL v2");
> \ No newline at end of file
You need to fix such errors.
> diff --git a/include/linux/soc/adi/adsp-gpio-port.h b/include/linux/soc/adi/adsp-gpio-port.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..6466ded03ec6092149a2abfc56a305f9124ac695
> --- /dev/null
> +++ b/include/linux/soc/adi/adsp-gpio-port.h
> @@ -0,0 +1,85 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright 2022-2024 - Analog Devices Inc.
> + */
> +
> +#ifndef GPIO_ADI_ADSP_PORT_H
> +#define GPIO_ADI_ADSP_PORT_H
> +
> +#include <linux/gpio/driver.h>
> +
> +/* Number of GPIOs per port instance */
> +#define ADSP_PORT_NGPIO 16
Hm? No, you cannot have GPIO driver in linux/soc...
> +
> +/* PORT memory layout */
> +#define ADSP_PORT_REG_FER 0x00
> +#define ADSP_PORT_REG_FER_SET 0x04
> +#define ADSP_PORT_REG_FER_CLEAR 0x08
> +#define ADSP_PORT_REG_DATA 0x0c
> +#define ADSP_PORT_REG_DATA_SET 0x10
> +#define ADSP_PORT_REG_DATA_CLEAR 0x14
> +#define ADSP_PORT_REG_DIR 0x18
> +#define ADSP_PORT_REG_DIR_SET 0x1c
> +#define ADSP_PORT_REG_DIR_CLEAR 0x20
> +#define ADSP_PORT_REG_INEN 0x24
> +#define ADSP_PORT_REG_INEN_SET 0x28
> +#define ADSP_PORT_REG_INEN_CLEAR 0x2c
> +#define ADSP_PORT_REG_PORT_MUX 0x30
> +#define ADSP_PORT_REG_DATA_TGL 0x34
> +#define ADSP_PORT_REG_POLAR 0x38
> +#define ADSP_PORT_REG_POLAR_SET 0x3c
> +#define ADSP_PORT_REG_POLAR_CLEAR 0x40
> +#define ADSP_PORT_REG_LOCK 0x44
> +#define ADSP_PORT_REG_TRIG_TGL 0x48
> +
> +/*
> + * One gpio instance per PORT instance in the hardware, provides the per-PORT
> + * interface to the hardware. Referenced in GPIO and PINCTRL drivers
> + */
> +struct adsp_gpio_port {
> + struct device *dev;
> + void __iomem *regs;
> + struct gpio_chip gpio;
> + struct irq_domain *irq_domain;
> + uint32_t irq_offset;
> + uint32_t open_drain;
> + spinlock_t lock;
> +};
> +
> +/* may need lock depending on register */
> +static inline uint32_t __adsp_gpio_readl(struct adsp_gpio_port *port,
> + size_t offset)
> +{
> + return readl(port->regs + offset);
> +}
> +
> +/* may need lock depending on register */
What does it mean?
> +static inline void __adsp_gpio_writel(struct adsp_gpio_port *port, uint32_t val,
> + size_t offset)
> +{
> + writel(val, port->regs + offset);
Not useful wrapper. Makes code bigger.
> +}
> +
> +/* may need lock depending on register */
> +static inline u16 __adsp_gpio_readw(struct adsp_gpio_port *port,
> + size_t offset)
> +{
> + return readw(port->regs + offset);
> +}
> +
> +/* may need lock depending on register */
> +static inline void __adsp_gpio_writew(struct adsp_gpio_port *port, u16 val,
> + size_t offset)
> +{
> + writew(val, port->regs + offset);
> +}
> +
> +static inline struct adsp_gpio_port *to_adsp_gpio_port(struct gpio_chip
> + *chip)
> +{
> + return container_of(chip, struct adsp_gpio_port, gpio);
> +}
> +
> +int adsp_attach_pint_to_gpio(struct adsp_gpio_port *port);
> +
> +#endif
> diff --git a/include/linux/soc/adi/cpu.h b/include/linux/soc/adi/cpu.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..0ddb30619b423ce70e9b8018ed6404cfd4ef6039
> --- /dev/null
> +++ b/include/linux/soc/adi/cpu.h
> @@ -0,0 +1,107 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright 2022-2024 - Analog Devices Inc.
> + */
> +
> +#ifndef __MACH_CPU_H
> +#define __MACH_CPU_H
That's not an accurate header guard.
> +
> +#define SYS_L2_START 0x20080000
> +#define SYS_SRAM_BASE (0x20080000 + SZ_16K)
> +
> +#define SYS_SRAM_SIZE (SZ_16K + SZ_32K * 3)
> +#define SYS_SRAM_ICC_SIZE SZ_4K
> +#define SYS_MMR_BASE 0x31000000
> +#define SYS_MMR_SIZE SZ_1M
> +#define SYS_SMC_BANK1 0x44000000
> +
> +#define SC59X_GIC_PORT0 0x310B2000
> +#define SC59X_GIC_PORT1 0x310B4000
> +
> +/*
> + * Timer Configuration Register Bits
> + */
> +#define TIMER_EMU_RUN 0x8000
> +#define TIMER_BPER_EN 0x4000
> +#define TIMER_BWID_EN 0x2000
> +#define TIMER_BDLY_EN 0x1000
> +#define TIMER_OUT_DIS 0x0800
> +#define TIMER_TIN_SEL 0x0400
> +#define TIMER_CLK_SEL 0x0300
> +#define TIMER_CLK_SCLK 0x0000
> +#define TIMER_CLK_ALT_CLK0 0x0100
> +#define TIMER_CLK_ALT_CLK1 0x0300
> +#define TIMER_PULSE_HI 0x0080
> +#define TIMER_SLAVE_TRIG 0x0040
> +#define TIMER_IRQ_MODE 0x0030
> +#define TIMER_IRQ_ACT_EDGE 0x0000
> +#define TIMER_IRQ_DLY 0x0010
> +#define TIMER_IRQ_WID_DLY 0x0020
> +#define TIMER_IRQ_PER 0x0030
> +#define TIMER_MODE 0x000f
> +#define TIMER_MODE_WDOG_P 0x0008
> +#define TIMER_MODE_WDOG_W 0x0009
> +#define TIMER_MODE_PWM_CONT 0x000c
> +#define TIMER_MODE_PWM 0x000d
> +#define TIMER_MODE_WDTH 0x000a
> +#define TIMER_MODE_WDTH_D 0x000b
> +#define TIMER_MODE_EXT_CLK 0x000e
> +#define TIMER_MODE_PININT 0x000f
> +
> +#define __BFP(m) u16 m; u16 __pad_##m
> +
> +struct gptimer3 {
> + __BFP(config);
> + uint32_t counter;
> + uint32_t period;
> + uint32_t width;
> + uint32_t delay;
> +};
> +
> +struct sc5xx_gptimer {
> + int id;
> + int irq;
> + int reserved;
> + int int_enable;
> + void __iomem *io_base;
> + void __iomem *cgu0_ctl;
> + unsigned long isr_count;
> + struct platform_device *pdev;
> + struct list_head node;
> +};
> +
> +struct gptimer3_group_regs {
> + __BFP(run);
> + __BFP(enable);
> + __BFP(disable);
> + __BFP(stop_cfg);
> + __BFP(stop_cfg_set);
> + __BFP(stop_cfg_clr);
> + __BFP(data_imsk);
> + __BFP(stat_imsk);
> + __BFP(tr_msk);
> + __BFP(tr_ie);
> + __BFP(data_ilat);
> + __BFP(stat_ilat);
> + __BFP(err_status);
> + __BFP(bcast_per);
> + __BFP(bcast_wid);
> + __BFP(bcast_dly);
> +};
> +
> +/* The actual gptimer API */
> +struct sc5xx_gptimer *gptimer_request(int id);
> +int gptimer_free(struct sc5xx_gptimer *timer);
> +void set_gptimer_pwidth(struct sc5xx_gptimer *timer, uint32_t width);
> +void set_gptimer_period(struct sc5xx_gptimer *timer, uint32_t period);
> +uint32_t get_gptimer_count(struct sc5xx_gptimer *timer);
> +void set_gptimer_config(struct sc5xx_gptimer *timer, uint16_t config);
> +void enable_gptimers(uint16_t mask);
> +void disable_gptimers(uint16_t mask);
> +void map_gptimers(void);
> +uint16_t get_gptimer_status(void);
> +void set_gptimer_status(uint16_t value);
> +void set_spu_securep_msec(uint16_t n, bool msec);
> +void platform_ipi_init(void);
All these do not exist.
Best regards,
Krzysztof
Powered by blists - more mailing lists