lkml.org 
[lkml]   [2023]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v1 03/43] soc: Add SoC driver for Cirrus ep93xx
Thu, Jun 01, 2023 at 08:33:54AM +0300, Nikita Shubin kirjoitti:
> This adds an SoC driver for the ep93xx. Currently there
> is only one thing not fitting into any other framework,
> and that is the swlock setting.
>
> It's used for clock settings and restart.

...

> source "drivers/soc/ux500/Kconfig"
> source "drivers/soc/versatile/Kconfig"
> source "drivers/soc/xilinx/Kconfig"
> +source "drivers/soc/cirrus/Kconfig"

Why not ordered?

...

> obj-$(CONFIG_ARCH_U8500) += ux500/
> obj-$(CONFIG_PLAT_VERSATILE) += versatile/
> obj-y += xilinx/
> +obj-$(CONFIG_EP93XX_SOC) += cirrus/

Ditto.

...

> +/*
> + * Soc driver for Cirrus EP93xx chips.

SoC

> + * 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

Linux
community.

> + */

...

> +#include <linux/soc/cirrus/ep93xx.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/of.h>

Can this be ordered?

...

> +#define EP93XX_SYSCON_SYSCFG_REV_MASK (0xf0000000)

GENMASK() ?
(will need bits.h)

> +#define EP93XX_SYSCON_SYSCFG_REV_SHIFT (28)

Here and above, do you need parentheses?

...

> +static struct regmap *map;

Global?!

...

> +EXPORT_SYMBOL_GPL(ep93xx_syscon_swlocked_write);

Can it (and other exported symbols) be exported with a namespace?

...

> +/**
> + * ep93xx_chip_revision() - returns the EP93xx chip revision

> + *

Redundant (?) blank line, but...

kernel doc validation will complain here a lot. Either drop kernel doc style or
fill it correctly.

> + */

...

> +static int __init ep93xx_soc_init(void)
> +{
> + /* Multiplatform guard, only proceed on ep93xx */
> + if (!of_machine_is_compatible("cirrus,ep9301"))
> + return 0;
> +
> + map = syscon_regmap_lookup_by_compatible("cirrus,ep9301-syscon");
> + if (IS_ERR(map))
> + return PTR_ERR(map);
> +
> + pr_info("EP93xx SoC revision %s\n", ep93xx_get_soc_rev());
> +
> + return 0;
> +}

> +

Unneeded blank line.

> +core_initcall(ep93xx_soc_init);
> +

Trailing blank line.

--
With Best Regards,
Andy Shevchenko


\
 
 \ /
  Last update: 2023-06-03 20:35    [W:1.817 / U:3.712 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site