From: Damien Le Moal <dlemoal@kernel.org>
To: Geert Uytterhoeven <geert@linux-m68k.org>,
Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: linux-sh@vger.kernel.org, "Niklas Cassel" <cassel@kernel.org>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
"Conor Dooley" <conor+dt@kernel.org>,
"Michael Turquette" <mturquette@baylibre.com>,
"Stephen Boyd" <sboyd@kernel.org>,
"David Airlie" <airlied@gmail.com>,
"Daniel Vetter" <daniel@ffwll.ch>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
"Krzysztof Wilczyński" <kw@linux.com>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Jiri Slaby" <jirislaby@kernel.org>,
"Magnus Damm" <magnus.damm@gmail.com>,
"Daniel Lezcano" <daniel.lezcano@linaro.org>,
"Rich Felker" <dalias@libc.org>,
"John Paul Adrian Glaubitz" <glaubitz@physik.fu-berlin.de>,
"Lee Jones" <lee@kernel.org>, "Helge Deller" <deller@gmx.de>,
"Heiko Stuebner" <heiko.stuebner@cherry.de>,
"Shawn Guo" <shawnguo@kernel.org>,
"Sebastian Reichel" <sre@kernel.org>,
"Chris Morgan" <macromorgan@hotmail.com>,
"Linus Walleij" <linus.walleij@linaro.org>,
"Arnd Bergmann" <arnd@arndb.de>,
"David Rientjes" <rientjes@google.com>,
"Hyeonggon Yoo" <42.hyeyoo@gmail.com>,
"Vlastimil Babka" <vbabka@suse.cz>, "Baoquan He" <bhe@redhat.com>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Guenter Roeck" <linux@roeck-us.net>,
"Kefeng Wang" <wangkefeng.wang@huawei.com>,
"Stephen Rothwell" <sfr@canb.auug.org.au>,
"Javier Martinez Canillas" <javierm@redhat.com>,
"Guo Ren" <guoren@kernel.org>,
"Azeem Shaikh" <azeemshaikh38@gmail.com>,
"Max Filippov" <jcmvbkbc@gmail.com>,
"Jonathan Corbet" <corbet@lwn.net>,
"Jacky Huang" <ychuang3@nuvoton.com>,
"Herve Codina" <herve.codina@bootlin.com>,
"Manikanta Guntupalli" <manikanta.guntupalli@amd.com>,
"Anup Patel" <apatel@ventanamicro.com>,
"Biju Das" <biju.das.jz@bp.renesas.com>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Sam Ravnborg" <sam@ravnborg.org>,
"Sergey Shtylyov" <s.shtylyov@omp.ru>,
"Laurent Pinchart" <laurent.pinchart+renesas@ideasonboard.com>,
linux-ide@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
linux-clk@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-pci@vger.kernel.org, linux-serial@vger.kernel.org,
linux-fbdev@vger.kernel.org
Subject: Re: [RESEND v7 14/37] clk: Compatible with narrow registers
Date: Fri, 5 Apr 2024 22:07:04 +0900 [thread overview]
Message-ID: <8ce4c1d8-e379-4fe0-ae31-ba5bdf4c1e06@kernel.org> (raw)
In-Reply-To: <CAMuHMdVXvPW+3-sY2XPQ2aMcTZkK9zoMnxWeZ+PRB+VRgGszdQ@mail.gmail.com>
On 4/5/24 21:56, Geert Uytterhoeven wrote:
> Hi Sato-san,
>
> On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
> <ysato@users.sourceforge.jp> wrote:
>> divider and gate only support 32-bit registers.
>> Older hardware uses narrower registers, so I want to be able to handle
>> 8-bit and 16-bit wide registers.
>>
>> Seven clk_divider flags are used, and if I add flags for 8bit access and
>> 16bit access, 8bit will not be enough, so I expanded it to u16.
>>
>> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
>
> Thanks for the update!
>
>> --- a/drivers/clk/clk-divider.c
>> +++ b/drivers/clk/clk-divider.c
>> @@ -26,20 +26,38 @@
>> * parent - fixed parent. No clk_set_parent support
>> */
>>
>> -static inline u32 clk_div_readl(struct clk_divider *divider)
>> -{
>> - if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> - return ioread32be(divider->reg);
>> -
>> - return readl(divider->reg);
>> +static inline u32 clk_div_read(struct clk_divider *divider)
>> +{
>> + if (divider->flags & CLK_DIVIDER_REG_8BIT)
>
> When you need curly braces in one branch of an if/else statement,
> please use curly braces in all branches (everywhere).
>
>> + return readb(divider->reg);
>> + else if (divider->flags & CLK_DIVIDER_REG_16BIT) {
And no need for an else after a return...
>> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> + return ioread16be(divider->reg);
>> + else
and here.
>> + return readw(divider->reg);
>> + } else {
>> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> + return ioread32be(divider->reg);
>> + else
here too.
>> + return readl(divider->reg);
>> + }
>> }
>
>> --- a/drivers/clk/clk-gate.c
>> +++ b/drivers/clk/clk-gate.c
>
>> @@ -137,12 +155,30 @@ struct clk_hw *__clk_hw_register_gate(struct device *dev,
>> struct clk_init_data init = {};
>> int ret = -EINVAL;
>>
>> + /* validate register size option and bit_idx */
>> if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
>> if (bit_idx > 15) {
>> pr_err("gate bit exceeds LOWORD field\n");
>> return ERR_PTR(-EINVAL);
>> }
>> }
>> + if (clk_gate_flags & CLK_GATE_REG_16BIT) {
>> + if (bit_idx > 15) {
>> + pr_err("gate bit exceeds 16 bits\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> + }
>> + if (clk_gate_flags & CLK_GATE_REG_8BIT) {
>> + if (bit_idx > 7) {
>> + pr_err("gate bit exceeds 8 bits\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> + }
>> + if ((clk_gate_flags & CLK_GATE_HIWORD_MASK) &&
>
> If you use parentheses around "a & b" here...
>
>> + clk_gate_flags & (CLK_GATE_REG_8BIT | CLK_GATE_REG_16BIT)) {
>
> please add parentheses here, too.
>
>> + pr_err("HIWORD_MASK required 32-bit register\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>>
>> /* allocate the gate */
>> gate = kzalloc(sizeof(*gate), GFP_KERNEL);
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index 4a537260f655..eaa6ff1d0b2e 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -508,12 +508,16 @@ void of_fixed_clk_setup(struct device_node *np);
>> * CLK_GATE_BIG_ENDIAN - by default little endian register accesses are used for
>> * the gate register. Setting this flag makes the register accesses big
>> * endian.
>> + * CLK_GATE_REG_8BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses 8bit.
>> + * CLK_GATE_REG_16BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses 16bit.
>> */
>> struct clk_gate {
>> struct clk_hw hw;
>> void __iomem *reg;
>> u8 bit_idx;
>> - u8 flags;
>> + u32 flags;
>
> (from my comments on v6)
> There is no need to increase the size of the flags field for the gate clock.
>
>
>> spinlock_t *lock;
>> };
>>
>
>> @@ -675,13 +681,17 @@ struct clk_div_table {
>> * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
>> * for the divider register. Setting this flag makes the register accesses
>> * big endian.
>> + * CLK_DIVIDER_REG_8BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses 8bit.
>> + * CLK_DIVIDER_REG_16BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses 16bit.
>> */
>> struct clk_divider {
>> struct clk_hw hw;
>> void __iomem *reg;
>> u8 shift;
>> u8 width;
>> - u8 flags;
>> + u16 flags;
>> const struct clk_div_table *table;
>> spinlock_t *lock;
>> };
>
>> @@ -726,18 +738,18 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
>> struct device_node *np, const char *name,
>> const char *parent_name, const struct clk_hw *parent_hw,
>> const struct clk_parent_data *parent_data, unsigned long flags,
>> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
>> + void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
>
> "u16 clk_divider_flags", to match clk_divider.flags.
>
>> const struct clk_div_table *table, spinlock_t *lock);
>> struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
>> struct device_node *np, const char *name,
>> const char *parent_name, const struct clk_hw *parent_hw,
>> const struct clk_parent_data *parent_data, unsigned long flags,
>> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
>> + void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
>
> Likewise.
>
>> const struct clk_div_table *table, spinlock_t *lock);
>> struct clk *clk_register_divider_table(struct device *dev, const char *name,
>> const char *parent_name, unsigned long flags,
>> void __iomem *reg, u8 shift, u8 width,
>> - u8 clk_divider_flags, const struct clk_div_table *table,
>> + u32 clk_divider_flags, const struct clk_div_table *table,
>
> Likewise.
>
>> spinlock_t *lock);
>> /**
>> * clk_register_divider - register a divider clock with the clock framework
>
> Gr{oetje,eeting}s,
>
> Geert
>
--
Damien Le Moal
Western Digital Research
next prev parent reply other threads:[~2024-04-05 13:07 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1712207606.git.ysato@users.sourceforge.jp>
2024-04-04 5:14 ` [RESEND v7 01/37] sh: passing FDT address to kernel startup Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 02/37] sh: Kconfig unified OF supported targets Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 03/37] sh: Enable OF support for build and configuration Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 04/37] dt-bindings: interrupt-controller: Add header for Renesas SH3/4 INTC Yoshinori Sato
2024-04-04 6:08 ` Krzysztof Kozlowski
2024-04-04 5:14 ` [RESEND v7 05/37] sh: GENERIC_IRQ_CHIP support for CONFIG_OF=y Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 06/37] sh: kernel/setup Update DT support Yoshinori Sato
2024-04-04 16:48 ` Rob Herring
2024-04-04 5:14 ` [RESEND v7 07/37] sh: Fix COMMON_CLK support in CONFIG_OF=y Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 08/37] clocksource: sh_tmu: CLOCKSOURCE support Yoshinori Sato
2024-04-09 13:56 ` Geert Uytterhoeven
2024-04-04 5:14 ` [RESEND v7 09/37] dt-binding: Add compatible SH7750 SoC Yoshinori Sato
2024-04-05 12:31 ` Geert Uytterhoeven
2024-04-10 12:12 ` Rob Herring
2024-04-04 5:14 ` [RESEND v7 10/37] sh: Common PCI Framework driver support Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 11/37] pci: pci-sh7751: Add SH7751 PCI driver Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 12/37] dt-bindings: pci: pci-sh7751: Add SH7751 PCI Yoshinori Sato
2024-04-10 12:16 ` Rob Herring
2024-04-04 5:14 ` [RESEND v7 13/37] dt-bindings: clock: sh7750-cpg: Add renesas, sh7750-cpg header Yoshinori Sato
2024-04-05 12:40 ` [RESEND v7 13/37] dt-bindings: clock: sh7750-cpg: Add renesas,sh7750-cpg header Geert Uytterhoeven
2024-04-10 16:13 ` Rob Herring
2024-04-04 5:14 ` [RESEND v7 14/37] clk: Compatible with narrow registers Yoshinori Sato
2024-04-05 12:56 ` Geert Uytterhoeven
2024-04-05 13:07 ` Damien Le Moal [this message]
2024-04-04 5:14 ` [RESEND v7 15/37] clk: renesas: Add SH7750/7751 CPG Driver Yoshinori Sato
2024-05-21 7:41 ` Geert Uytterhoeven
2024-04-04 5:14 ` [RESEND v7 16/37] irqchip: Add SH7751 INTC driver Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 17/37] dt-bindings: interrupt-controller: renesas, sh7751-intc: Add json-schema Yoshinori Sato
2024-04-10 16:14 ` [RESEND v7 17/37] dt-bindings: interrupt-controller: renesas,sh7751-intc: " Rob Herring
2024-04-04 5:14 ` [RESEND v7 18/37] irqchip: SH7751 external interrupt encoder with enable gate Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 19/37] dt-bindings: interrupt-controller: renesas, sh7751-irl-ext: Add json-schema Yoshinori Sato
2024-04-04 7:45 ` [RESEND v7 19/37] dt-bindings: interrupt-controller: renesas,sh7751-irl-ext: " Rob Herring
2024-04-04 5:14 ` [RESEND v7 20/37] serial: sh-sci: fix SH4 OF support Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 21/37] dt-bindings: serial: renesas, scif: Add scif-sh7751 Yoshinori Sato
2024-04-05 14:02 ` Geert Uytterhoeven
2024-04-04 5:14 ` [RESEND v7 22/37] dt-bindings: display: smi, sm501: SMI SM501 binding json-schema Yoshinori Sato
2024-04-10 16:33 ` [RESEND v7 22/37] dt-bindings: display: smi,sm501: " Rob Herring
2024-04-04 5:14 ` [RESEND v7 23/37] dt-bindings: display: sm501 register definition helper Yoshinori Sato
2024-04-05 13:25 ` Geert Uytterhoeven
2024-04-04 5:14 ` [RESEND v7 24/37] mfd: sm501: Convert platform_data to OF property Yoshinori Sato
2024-05-02 16:02 ` Lee Jones
2024-04-04 5:14 ` [RESEND v7 25/37] dt-binding: sh: cpus: Add SH CPUs json-schema Yoshinori Sato
2024-04-10 16:37 ` Rob Herring
2024-04-04 5:14 ` [RESEND v7 26/37] dt-bindings: vendor-prefixes: Add iodata Yoshinori Sato
2024-04-04 6:57 ` Krzysztof Kozlowski
2024-04-04 5:14 ` [RESEND v7 27/37] dt-bindings: ata: ata-generic: Add new targets Yoshinori Sato
2024-04-04 6:57 ` Krzysztof Kozlowski
2024-04-04 5:14 ` [RESEND v7 28/37] dt-bindings: soc: renesas: sh: Add SH7751 based target Yoshinori Sato
2024-04-05 13:44 ` Geert Uytterhoeven
2024-04-10 16:38 ` Rob Herring
2024-04-04 5:14 ` [RESEND v7 29/37] sh: SH7751R SoC Internal peripheral definition dtsi Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 30/37] sh: add RTS7751R2D Plus DTS Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 31/37] sh: Add IO DATA LANDISK dts Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 32/37] sh: Add IO DATA USL-5P dts Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 33/37] sh: j2_mimas_v2.dts update Yoshinori Sato
2024-04-05 12:44 ` Geert Uytterhoeven
2024-04-04 5:14 ` [RESEND v7 34/37] sh: Add dtbs target support Yoshinori Sato
2024-04-05 12:45 ` Geert Uytterhoeven
2024-04-04 5:14 ` [RESEND v7 35/37] sh: RTS7751R2D Plus OF defconfig Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 36/37] sh: LANDISK " Yoshinori Sato
2024-04-04 5:14 ` [RESEND v7 37/37] sh: j2_defconfig: update Yoshinori Sato
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8ce4c1d8-e379-4fe0-ae31-ba5bdf4c1e06@kernel.org \
--to=dlemoal@kernel.org \
--cc=42.hyeyoo@gmail.com \
--cc=airlied@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=apatel@ventanamicro.com \
--cc=arnd@arndb.de \
--cc=azeemshaikh38@gmail.com \
--cc=bhe@redhat.com \
--cc=bhelgaas@google.com \
--cc=biju.das.jz@bp.renesas.com \
--cc=cassel@kernel.org \
--cc=conor+dt@kernel.org \
--cc=corbet@lwn.net \
--cc=dalias@libc.org \
--cc=daniel.lezcano@linaro.org \
--cc=daniel@ffwll.ch \
--cc=deller@gmx.de \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=geert@linux-m68k.org \
--cc=glaubitz@physik.fu-berlin.de \
--cc=gregkh@linuxfoundation.org \
--cc=guoren@kernel.org \
--cc=heiko.stuebner@cherry.de \
--cc=herve.codina@bootlin.com \
--cc=javierm@redhat.com \
--cc=jcmvbkbc@gmail.com \
--cc=jirislaby@kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=kw@linux.com \
--cc=laurent.pinchart+renesas@ideasonboard.com \
--cc=lee@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=lpieralisi@kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=macromorgan@hotmail.com \
--cc=magnus.damm@gmail.com \
--cc=manikanta.guntupalli@amd.com \
--cc=mripard@kernel.org \
--cc=mturquette@baylibre.com \
--cc=rientjes@google.com \
--cc=robh@kernel.org \
--cc=s.shtylyov@omp.ru \
--cc=sam@ravnborg.org \
--cc=sboyd@kernel.org \
--cc=sfr@canb.auug.org.au \
--cc=shawnguo@kernel.org \
--cc=sre@kernel.org \
--cc=tglx@linutronix.de \
--cc=tzimmermann@suse.de \
--cc=u.kleine-koenig@pengutronix.de \
--cc=vbabka@suse.cz \
--cc=wangkefeng.wang@huawei.com \
--cc=ychuang3@nuvoton.com \
--cc=ysato@users.sourceforge.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).