Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Date:   Wed, 22 Nov 2023 14:01:54 +0200
From:   Andy Shevchenko <andriy.shevchenko@intel.com>
To:     Nikita Shubin <nikita.shubin@maquefel.me>
Cc:     Conor Dooley <conor.dooley@microchip.com>,
        Joel Stanley <joel@jms.id.au>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Huisong Li <lihuisong@huawei.com>,
        Jonathan =?iso-8859-1?Q?Neusch=E4fer?= <j.neuschaefer@gmx.net>,
        Walker Chen <walker.chen@starfivetech.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Emil Renner Berthing <kernel@esmil.dk>,
        Alexander Sverdlin <alexander.sverdlin@gmail.com>,
        Hal Feng <hal.feng@starfivetech.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 07/39] soc: Add SoC driver for Cirrus ep93xx
Message-ID: <ZV3tsg7qE5IVbNYb@smile.fi.intel.com>
References: <20231122-ep93xx-v5-0-d59a76d5df29@maquefel.me>
 <20231122-ep93xx-v5-7-d59a76d5df29@maquefel.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231122-ep93xx-v5-7-d59a76d5df29@maquefel.me>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

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