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

From: Andy Shevchenko
Date: Wed Nov 22 2023 - 07:26:02 EST


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