linux-kernel - Re: [PATCH v5 07/39] 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: <ZV3tsg7qE5IVbNYb@smile.fi.intel.com>
Date:   Wed, 22 Nov 2023 14:01:54 +0200
From:   Andy Shevchenko <andriy.shevchenko@...el.com>
To:     Nikita Shubin <nikita.shubin@...uefel.me>
Cc:     Conor Dooley <conor.dooley@...rochip.com>,
        Joel Stanley <joel@....id.au>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Huisong Li <lihuisong@...wei.com>,
        Jonathan Neuschäfer <j.neuschaefer@....net>,
        Walker Chen <walker.chen@...rfivetech.com>,
        Arnd Bergmann <arnd@...db.de>,
        Emil Renner Berthing <kernel@...il.dk>,
        Alexander Sverdlin <alexander.sverdlin@...il.com>,
        Hal Feng <hal.feng@...rfivetech.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 07/39] soc: Add SoC driver for Cirrus ep93xx

On Wed, Nov 22, 2023 at 11:59:45AM +0300, Nikita Shubin wrote:
> Add an SoC driver for the ep93xx. Currently there is only one thing
> not fitting into any other framework, and that is the swlock setting.
> 
> Used for clock settings, pinctrl and restart.

...

> +# SPDX-License-Identifier: GPL-2.0

only

> +# SPDX-License-Identifier: GPL-2.0

only

> +// SPDX-License-Identifier: GPL-2.0+

or any new

Hmm...

...

> +#include <linux/kernel.h>

JFYI (in case you are using this as a proxy) we have new headers:

array_size,h
sprintf.h

Same applies to all your patches where it's being used.

...

> +#include <linux/soc/cirrus/ep93xx.h>
> +
> +

One blank line is enough.

...

> +static const char __init *ep93xx_get_soc_rev(struct regmap *map)
> +{
> +	switch (ep93xx_soc_revision(map)) {
> +	case EP93XX_CHIP_REV_D0:
> +		return "D0";
> +	case EP93XX_CHIP_REV_D1:
> +		return "D1";
> +	case EP93XX_CHIP_REV_E0:
> +		return "E0";
> +	case EP93XX_CHIP_REV_E1:
> +		return "E1";
> +	case EP93XX_CHIP_REV_E2:
> +		return "E2";
> +	default:
> +		return "unknown";
> +	}
> +}

+ Blank line.

> +const char *pinctrl_names[] = {
> +	"pinctrl-ep9301",	/* EP93XX_9301_SOC */
> +	"pinctrl-ep9307",	/* EP93XX_9307_SOC */
> +	"pinctrl-ep9312"	/* EP93XX_9312_SOC */

I would leave a trailing comma.

> +};

...

> +	enum ep93xx_soc_model model = (int)of_device_get_match_data(&pdev->dev);

Same comment about uintptr_t casting.

-- 
With Best Regards,
Andy Shevchenko