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

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@maquefel.me>
> + *
> + * Based on a rewrite of arch/arm/mach-ep93xx/core.c
> + * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
> + * Copyright (C) 2007 Herbert Valerio Riedel <hvr@gnu.org>
> + *
> + * 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