From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6BE1AC77B7A for ; Sat, 3 Jun 2023 18:46:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229529AbjFCSfW (ORCPT ); Sat, 3 Jun 2023 14:35:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33580 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229481AbjFCSfU (ORCPT ); Sat, 3 Jun 2023 14:35:20 -0400 Received: from fgw20-7.mail.saunalahti.fi (fgw20-7.mail.saunalahti.fi [62.142.5.81]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 69420B1 for ; Sat, 3 Jun 2023 11:35:19 -0700 (PDT) Received: from localhost (88-113-26-95.elisa-laajakaista.fi [88.113.26.95]) by fgw20.mail.saunalahti.fi (Halon) with ESMTP id 60973bd3-023d-11ee-b3cf-005056bd6ce9; Sat, 03 Jun 2023 21:35:17 +0300 (EEST) From: andy.shevchenko@gmail.com Date: Sat, 3 Jun 2023 21:35:13 +0300 To: Nikita Shubin Cc: Alexander Sverdlin , Arnd Bergmann , Linus Walleij , Joel Stanley , Conor Dooley , Heiko Stuebner , Yinbo Zhu , Hitomi Hasegawa , Jonathan =?iso-8859-1?Q?Neusch=E4fer?= , Walker Chen , Paul Menzel , Emil Renner Berthing , Alexander Gordeev , Vasily Gorbik , Michael Peters , Kris Bahnsen , linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 03/43] soc: Add SoC driver for Cirrus ep93xx Message-ID: References: <20230424123522.18302-1-nikita.shubin@maquefel.me> <20230601053546.9574-4-nikita.shubin@maquefel.me> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230601053546.9574-4-nikita.shubin@maquefel.me> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > + * > + * Based on a rewrite of arch/arm/mach-ep93xx/core.c > + * Copyright (C) 2006 Lennert Buytenhek > + * Copyright (C) 2007 Herbert Valerio Riedel > + * > + * Thanks go to Michael Burian and Ray Lehtiniemi for their key > + * role in the ep93xx linux community Linux community. > + */ ... > +#include > +#include > +#include > +#include > +#include > +#include 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