[{"id":3319267,"web_url":"http://patchwork.ozlabs.org/comment/3319267/","msgid":"<3280d9e3c7ba19f86b85a7fa89f5be25.sboyd@kernel.org>","list_archive_url":null,"date":"2024-05-29T23:15:19","subject":"Re: [DO NOT MERGE v8 14/36] clk: Compatible with narrow registers","submitter":{"id":73546,"url":"http://patchwork.ozlabs.org/api/people/73546/","name":"Stephen Boyd","email":"sboyd@kernel.org"},"content":"Quoting Yoshinori Sato (2024-05-29 01:01:00)\n> divider and gate only support 32-bit registers.\n> Older hardware uses narrower registers, so I want to be able to handle\n> 8-bit and 16-bit wide registers.\n> \n> Seven clk_divider flags are used, and if I add flags for 8bit access and\n> 16bit access, 8bit will not be enough, so I expanded it to u16.\n> \n> Signed-off-by: Yoshinori Sato \n> ---\n> drivers/clk/clk-divider.c | 41 +++++++++++++++++++++---------\n> drivers/clk/clk-gate.c | 49 ++++++++++++++++++++++++++++++++----\n> include/linux/clk-provider.h | 20 ++++++++++++---\n> 3 files changed, 89 insertions(+), 21 deletions(-)\n> \n> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c\n> index a2c2b5203b0a..abafcbbb6578 100644\n> --- a/drivers/clk/clk-divider.c\n> +++ b/drivers/clk/clk-divider.c\n> @@ -26,17 +26,34 @@\n> * parent - fixed parent. No clk_set_parent support\n> */\n> \n> -static inline u32 clk_div_readl(struct clk_divider *divider)\n> -{\n> +static inline u32 clk_div_read(struct clk_divider *divider)\n\nPlease don't change the name. The 'l' is for the return type, u32, which\nis not changed.\n\n> +{\n> + if (divider->flags & CLK_DIVIDER_REG_8BIT)\n> + return readb(divider->reg);\n> + if (divider->flags & CLK_DIVIDER_REG_16BIT) {\n> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN) {\n> + return ioread16be(divider->reg);\n> + } else {\n> + return readw(divider->reg);\n> + }\n> + }\n> if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)\n> return ioread32be(divider->reg);\n> \n> return readl(divider->reg);\n> }\n> \n> -static inline void clk_div_writel(struct clk_divider *divider, u32 val)\n> +static inline void clk_div_write(struct clk_divider *divider, u32 val)\n\nSame comment.\n\n> {\n> - if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)\n> + if (divider->flags & CLK_DIVIDER_REG_8BIT)\n> + writeb(val, divider->reg);\n> + else if (divider->flags & CLK_DIVIDER_REG_16BIT) {\n> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN) {\n> + iowrite16be(val, divider->reg);\n> + } else {\n> + writew(val, divider->reg);\n> + }\n> + } else if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)\n> iowrite32be(val, divider->reg);\n> else\n> writel(val, divider->reg);\n> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h\n> index 4a537260f655..25f61bd5b952 100644\n> --- a/include/linux/clk-provider.h\n> +++ b/include/linux/clk-provider.h\n> @@ -508,6 +508,10 @@ void of_fixed_clk_setup(struct device_node *np);\n> * CLK_GATE_BIG_ENDIAN - by default little endian register accesses are used for\n> * the gate register. Setting this flag makes the register accesses big\n> * endian.\n> + * CLK_GATE_REG_8BIT - by default 32bit register accesses are used for\n> + * the gate register. Setting this flag makes the register accesses 8bit.\n> + * CLK_GATE_REG_16BIT - by default 32bit register accesses are used for\n> + * the gate register. Setting this flag makes the register accesses 16bit.\n> */\n> struct clk_gate {\n> struct clk_hw hw;\n> @@ -522,6 +526,8 @@ struct clk_gate {\n> #define CLK_GATE_SET_TO_DISABLE BIT(0)\n> #define CLK_GATE_HIWORD_MASK BIT(1)\n> #define CLK_GATE_BIG_ENDIAN BIT(2)\n> +#define CLK_GATE_REG_8BIT BIT(3)\n> +#define CLK_GATE_REG_16BIT BIT(4)\n\nPlease add kunit tests for the gate at least.\n\n> \n> extern const struct clk_ops clk_gate_ops;\n> struct clk_hw *__clk_hw_register_gate(struct device *dev,\n> @@ -675,13 +681,17 @@ struct clk_div_table {\n> * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used\n> * for the divider register. Setting this flag makes the register accesses\n> * big endian.\n> + * CLK_DIVIDER_REG_8BIT - by default 32bit register accesses are used for\n> + * the gate register. Setting this flag makes the register accesses 8bit.\n> + * CLK_DIVIDER_REG_16BIT - by default 32bit register accesses are used for\n> + * the gate register. Setting this flag makes the register accesses 16bit.\n> */\n> struct clk_divider {\n> struct clk_hw hw;\n> void __iomem *reg;\n> u8 shift;\n> u8 width;\n> - u8 flags;\n> + u16 flags;\n> const struct clk_div_table *table;\n> spinlock_t *lock;\n> };\n> @@ -697,6 +707,8 @@ struct clk_divider {\n> #define CLK_DIVIDER_READ_ONLY BIT(5)\n> #define CLK_DIVIDER_MAX_AT_ZERO BIT(6)\n> #define CLK_DIVIDER_BIG_ENDIAN BIT(7)\n> +#define CLK_DIVIDER_REG_8BIT BIT(8)\n> +#define CLK_DIVIDER_REG_16BIT BIT(9)\n> \n> extern const struct clk_ops clk_divider_ops;\n> extern const struct clk_ops clk_divider_ro_ops;\n> @@ -726,18 +738,18 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,\n> struct device_node *np, const char *name,\n> const char *parent_name, const struct clk_hw *parent_hw,\n> const struct clk_parent_data *parent_data, unsigned long flags,\n> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,\n> + void __iomem *reg, u8 shift, u8 width, u16 clk_divider_flags,\n\nLet's just make this unsigned long for the flags. We don't need to\nspecify a strict size like this for the callers.\n\n> const struct clk_div_table *table, spinlock_t *lock);\n> struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,\n> struct device_node *np, const char *name,\n> const char *parent_name, const struct clk_hw *parent_hw,\n> const struct clk_parent_data *parent_data, unsigned long flags,\n> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,\n> + void __iomem *reg, u8 shift, u8 width, u16 clk_divider_flags,\n\nSame here.\n\n> const struct clk_div_table *table, spinlock_t *lock);\n> struct clk *clk_register_divider_table(struct device *dev, const char *name,\n> const char *parent_name, unsigned long flags,\n> void __iomem *reg, u8 shift, u8 width,\n> - u8 clk_divider_flags, const struct clk_div_table *table,\n> + u16 clk_divider_flags, const struct clk_div_table *table,\n\nSame here. Preferably do that in another patch too.","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=OabscRPI;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=139.178.88.99; helo=sv.mirrors.kernel.org;\n envelope-from=linux-pci+bounces-8065-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"OabscRPI\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sv.mirrors.kernel.org (sv.mirrors.kernel.org [139.178.88.99])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4VqQGT70ZQz20Pc\n\tfor ; Thu, 30 May 2024 09:15:29 +1000 (AEST)","from smtp.subspace.kernel.org (wormhole.subspace.kernel.org\n [52.25.139.140])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby sv.mirrors.kernel.org (Postfix) with ESMTPS id 529E52871F6\n\tfor ; Wed, 29 May 2024 23:15:27 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id D04C615B562;\n\tWed, 29 May 2024 23:15:22 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 845C729A0;\n\tWed, 29 May 2024 23:15:22 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id DDF6DC32781;\n\tWed, 29 May 2024 23:15:21 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1717024522; cv=none;\n b=gX6zQ596nUTRegTyPf8uIagGEszOJ3sY3Djjbflw+4ayjGhS9Wf2PbRln9OQlaGjRZLvI+L0MoyUvvyI2xwsNQCWlTq9Gr9D48ydRL4gKTZ6a+QwKShL6t6syM4w9gHNxVL/UL4ISm3s9gcDh77DSYKT6ATBy0l2OOWJAcpms20=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1717024522; c=relaxed/simple;\n\tbh=7HtHIBMnOSfXwlIN2FN4JThv0qqYOxoaXPTbFn/okcY=;\n\th=Message-ID:Content-Type:MIME-Version:In-Reply-To:References:\n\t Subject:From:Cc:To:Date;\n b=MFro4ks6n/KhLMlthy+Dh4FXMl2dtxt1qYkJa+RnrjEMR9S/cJl/UY5/1JCew3nJd9hCSEdL5vIfBOjad8KigGKBhNLxi5rMxtHww19v/aiohLdNNxyyBReU9L9AWLf8Q0Tdp3AtsufrktPoFjZAhFayg8tBE45zhZhDIctX9uM=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=OabscRPI; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1717024522;\n\tbh=7HtHIBMnOSfXwlIN2FN4JThv0qqYOxoaXPTbFn/okcY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=OabscRPIEp57y8+0XlGk2MoMOeUdg3DFch7rE9IX08FLlS4oK8rykJtvab3sIEtiA\n\t bRKPiWK35XtvcazFBhiVQFUOddAQgdYj+QVCvdrPBzcU9X3d/30Gviua7TUav5HcVH\n\t ifDThHZQalm9753WBOnV8YV5lSp4qJgxmXRDfyTMrk2+eTjTsr1Tx0gsrPMLu8yfMp\n\t f4K/Uum4draifjed4IA71oYuDtsQwdkKU4U3aojjuTNeYziub9jTJi54QezxYVXe7/\n\t K4QJdoFY4b00+gH/sYrGVsHbk4NdQ+Hv/vn/al2ck+PIe7Q0n1ndrOSIs8/MUlVFzB\n\t UmKhhc3IAcIoA==","Message-ID":"<3280d9e3c7ba19f86b85a7fa89f5be25.sboyd@kernel.org>","Content-Type":"text/plain; charset=\"utf-8\"","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"\n ","References":"\n ","Subject":"Re: [DO NOT MERGE v8 14/36] clk: Compatible with narrow registers","From":"Stephen Boyd ","Cc":"Yoshinori Sato ,\n Damien Le Moal , Niklas Cassel ,\n Rob Herring , Krzysztof Kozlowski ,\n Conor Dooley ,\n Geert Uytterhoeven ,\n Michael Turquette ,\n David Airlie , Daniel Vetter ,\n Maarten Lankhorst ,\n Maxime Ripard , Thomas Zimmermann ,\n Thomas Gleixner , Bjorn Helgaas ,\n Lorenzo Pieralisi , Krzysztof =?utf-8?q?Wilczy=C5=84?=\n\t=?utf-8?q?ski?= ,\n Greg Kroah-Hartman ,\n Jiri Slaby , Magnus Damm ,\n Daniel Lezcano , Rich Felker ,\n John Paul Adrian Glaubitz ,\n Lee Jones , Helge Deller ,\n Heiko Stuebner ,\n Neil Armstrong ,\n Chris Morgan , Sebastian Reichel ,\n Linus Walleij , Arnd Bergmann ,\n Masahiro Yamada , Baoquan He ,\n Andrew Morton ,\n Guenter Roeck , Kefeng Wang ,\n Stephen Rothwell ,\n Azeem Shaikh , Guo Ren ,\n Max Filippov , Jernej Skrabec ,\n Herve Codina ,\n Andy Shevchenko ,\n Anup Patel , Jacky Huang ,\n Hugo Villeneuve , Jonathan Corbet ,\n Wolfram Sang , Uwe =?utf-8?q?Kleine-K?=\n\t=?utf-8?q?=C3=B6nig?= ,\n Christophe JAI LLET ,\n Sam Ravnborg ,\n Javier Martinez Canillas ,\n Sergey Shtylyov ,\n Laurent Pinchart ,\n linux-ide@vger.kernel.org, devicetree@vger.kernel.org,\n linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org,\n linux-clk@vger.kernel.org, dri-devel@lists.freedesktop.org,\n linux-pci@vger.kernel.org, linux-serial@vger.kernel.org,\n linux-fbdev@vger.kernel.org","To":"Yoshinori Sato , linux-sh@vger.kernel.org","Date":"Wed, 29 May 2024 16:15:19 -0700","User-Agent":"alot/0.10"}}]