Re: [PATCH v5 07/39] soc: Add SoC driver for Cirrus ep93xx

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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