Re: [PATCH v6 08/40] soc: Add SoC driver for Cirrus ep93xx

[Andy Shevchenko]

On Tue, Dec 12, 2023 at 11:20:25AM +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.

...

> +/*
> + * SoC driver for Cirrus EP93xx chips.
> + * Copyright (C) 2022 Nikita Shubin <nikita.shubin@xxxxxxxxxxx>
> + *
> + * Based on a rewrite of arch/arm/mach-ep93xx/core.c
> + * Copyright (C) 2006 Lennert Buytenhek <buytenh@xxxxxxxxxxxxxx>
> + * Copyright (C) 2007 Herbert Valerio Riedel <hvr@xxxxxxx>
> + *
> + * Thanks go to Michael Burian and Ray Lehtiniemi for their key
> + * role in the ep93xx Linux community

Missing period.

> + */

...

> +#include <linux/bits.h>
> +#include <linux/init.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/of_fdt.h>
> +#include <linux/platform_device.h>

Isn't this an incorrect header and should be auxiliary one?

> +#include <linux/regmap.h>
> +#include <linux/slab.h>

+ spinlock.h ?

But since it's a new code, why not cleanup.h?

> +#include <linux/sys_soc.h>

...

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

int?

Maybe

	strict device *dev = &pdev->dev;
	enum ep93xx_soc_model model;
	...
	model = (enum ep93xx_soc_model)(uintptr_t)device_get_match_data(dev);

?

...

> +	struct device *dev = &pdev->dev;

Ah you even have this already!

...

> +	dev_info(dev, "EP93xx SoC revision %s\n", attrs->revision);

Hmm... Is this message anyhow useful?

-- 
With Best Regards,
Andy Shevchenko