linux-kernel - Re: [PATCH v1 03/43] soc: Add SoC driver for Cirrus ep93xx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fde8e0c8-2730-f679-d361-86fad007ee47@molgen.mpg.de>
Date:   Thu, 1 Jun 2023 07:53:38 +0200
From:   Paul Menzel <pmenzel@...gen.mpg.de>
To:     Nikita Shubin <nikita.shubin@...uefel.me>
Cc:     Alexander Sverdlin <alexander.sverdlin@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        Joel Stanley <joel@....id.au>,
        Conor Dooley <conor.dooley@...rochip.com>,
        Heiko Stuebner <heiko@...ech.de>,
        Yinbo Zhu <zhuyinbo@...ngson.cn>,
        Hitomi Hasegawa <hasegawa-hitomi@...itsu.com>,
        Jonathan Neuschäfer <j.neuschaefer@....net>,
        Walker Chen <walker.chen@...rfivetech.com>,
        Emil Renner Berthing <kernel@...il.dk>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Michael Peters <mpeters@...eddedTS.com>,
        Kris Bahnsen <kris@...eddedTS.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 03/43] soc: Add SoC driver for Cirrus ep93xx

Dear Nikita,


Thank you for your patch. Some small nit picks.

Am 01.06.23 um 07:33 schrieb Nikita Shubin:
> This adds an SoC driver for the ep93xx. Currently there
> is only one thing not fitting into any other framework,
> and that is the swlock setting.

If you reflow for 75 characters per line, this would fit into two lines.

> It's used for clock settings and restart.

What datasheet did you use, and on what board did you test this?

> Signed-off-by: Nikita Shubin <nikita.shubin@...uefel.me>
> Tested-by: Alexander Sverdlin <alexander.sverdlin@...il.com>
> Acked-by: Alexander Sverdlin <alexander.sverdlin@...il.com>
> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
> ---

[…]

>   drivers/soc/Kconfig               |   1 +
>   drivers/soc/Makefile              |   1 +
>   drivers/soc/cirrus/Kconfig        |  11 +++
>   drivers/soc/cirrus/Makefile       |   2 +
>   drivers/soc/cirrus/soc-ep93xx.c   | 134 ++++++++++++++++++++++++++++++
>   include/linux/soc/cirrus/ep93xx.h |  16 +++-
>   6 files changed, 161 insertions(+), 4 deletions(-)
>   create mode 100644 drivers/soc/cirrus/Kconfig
>   create mode 100644 drivers/soc/cirrus/Makefile
>   create mode 100644 drivers/soc/cirrus/soc-ep93xx.c
> 
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index 4e176280113a..6149f0447b61 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -30,5 +30,6 @@ source "drivers/soc/ti/Kconfig"
>   source "drivers/soc/ux500/Kconfig"
>   source "drivers/soc/versatile/Kconfig"
>   source "drivers/soc/xilinx/Kconfig"
> +source "drivers/soc/cirrus/Kconfig"
>   
>   endmenu
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 3b0f9fb3b5c8..7a8a154b8d96 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -36,3 +36,4 @@ obj-y				+= ti/
>   obj-$(CONFIG_ARCH_U8500)	+= ux500/
>   obj-$(CONFIG_PLAT_VERSATILE)	+= versatile/
>   obj-y				+= xilinx/
> +obj-$(CONFIG_EP93XX_SOC)	+= cirrus/
> diff --git a/drivers/soc/cirrus/Kconfig b/drivers/soc/cirrus/Kconfig
> new file mode 100644
> index 000000000000..bc82c0422325
> --- /dev/null
> +++ b/drivers/soc/cirrus/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +if ARCH_EP93XX
> +
> +config EP93XX_SOC
> +	bool "Cirrus EP93xx chips SoC"
> +	default y if !EP93XX_SOC_COMMON
> +	help
> +	  Support Soc for Cirrus EP93xx chips.

Above (and more common in the whole SOC directory) you spell it SoC.

> +
> +endif
> diff --git a/drivers/soc/cirrus/Makefile b/drivers/soc/cirrus/Makefile
> new file mode 100644
> index 000000000000..ed6752844c6f
> --- /dev/null
> +++ b/drivers/soc/cirrus/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-y	+= soc-ep93xx.o
> diff --git a/drivers/soc/cirrus/soc-ep93xx.c b/drivers/soc/cirrus/soc-ep93xx.c
> new file mode 100644
> index 000000000000..64842e9e2316
> --- /dev/null
> +++ b/drivers/soc/cirrus/soc-ep93xx.c
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Soc driver for Cirrus EP93xx chips.

Ditto.

> + * Copyright (C) 2022 Nikita Shubin <nikita.shubin@...uefel.me>
> + *
> + * Based on a rewrite of arch/arm/mach-ep93xx/core.c
> + * Copyright (C) 2006 Lennert Buytenhek <buytenh@...tstofly.org>
> + * Copyright (C) 2007 Herbert Valerio Riedel <hvr@....org>
> + *
> + * Thanks go to Michael Burian and Ray Lehtiniemi for their key
> + * role in the ep93xx linux community

If you know their email addresses, you could add those too. Maybe also 
add a dot/period at the end.

> + */
> +
> +#include <linux/soc/cirrus/ep93xx.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/of.h>
> +
> +#define EP93XX_SYSCON_DEVCFG		0x80
> +
> +#define EP93XX_SWLOCK_MAGICK		0xaa
> +#define EP93XX_SYSCON_SWLOCK		0xc0
> +#define EP93XX_SYSCON_SYSCFG		0x9c
> +#define EP93XX_SYSCON_SYSCFG_REV_MASK	(0xf0000000)
> +#define EP93XX_SYSCON_SYSCFG_REV_SHIFT	(28)

[…]


Kind regards,

Paul