| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <71f646cb-7114-0c78-493c-99bdb5217801@gmail.com>
Date: Sat, 15 Apr 2023 11:58:57 +0800
From: Jacky Huang <ychuang570808@...il.com>
To: Stephen Boyd <sboyd@...nel.org>, gregkh@...uxfoundation.org,
jirislaby@...nel.org, krzysztof.kozlowski+dt@...aro.org,
lee@...nel.org, mturquette@...libre.com, p.zabel@...gutronix.de,
robh+dt@...nel.org
Cc: devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
arnd@...db.de, schung@...oton.com, mjchen@...oton.com,
Jacky Huang <ychuang3@...oton.com>
Subject: Re: [PATCH v7 09/12] clk: nuvoton: Add clock driver for ma35d1 clock
controller
Dear Stephen,
On 2023/4/14 上午 04:27, Stephen Boyd wrote:
> Quoting Jacky Huang (2023-04-11 22:38:21)
>> diff --git a/drivers/clk/nuvoton/clk-ma35d1-divider.c b/drivers/clk/nuvoton/clk-ma35d1-divider.c
>> new file mode 100644
>> index 000000000000..8d573ba3dfd3
>> --- /dev/null
>> +++ b/drivers/clk/nuvoton/clk-ma35d1-divider.c
>> @@ -0,0 +1,134 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2023 Nuvoton Technology Corp.
> [...]
>> +struct clk_hw *ma35d1_reg_adc_clkdiv(struct device *dev, const char *name,
>> + const char *parent_name,
>> + spinlock_t *lock,
>> + unsigned long flags, void __iomem *reg,
>> + u8 shift, u8 width, u32 mask_bit)
>> +{
>> + struct ma35d1_adc_clk_div *div;
>> + struct clk_init_data init;
>> + struct clk_div_table *table;
>> + u32 max_div, min_div;
>> + struct clk_hw *hw;
>> + int ret;
>> + int i;
>> +
>> + div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
>> + if (!div)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + max_div = clk_div_mask(width) + 1;
>> + min_div = 1;
>> +
>> + table = devm_kcalloc(dev, max_div + 1, sizeof(*table), GFP_KERNEL);
>> + if (!table)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + for (i = 0; i < max_div; i++) {
>> + table[i].val = min_div + i;
>> + table[i].div = 2 * table[i].val;
>> + }
>> + table[max_div].val = 0;
>> + table[max_div].div = 0;
>> +
>> + init.name = name;
>> + init.ops = &ma35d1_adc_clkdiv_ops;
>> + init.flags |= flags;
>> + init.parent_names = parent_name ? &parent_name : NULL;
> Can you use parent_data instead?
Sure, I will modify it.
>
>> + init.num_parents = parent_name ? 1 : 0;
>> +
>> + div->reg = reg;
>> + div->shift = shift;
>> + div->width = width;
>> + div->mask = mask_bit ? BIT(mask_bit) : 0;
>> + div->lock = lock;
>> + div->hw.init = &init;
>> + div->table = table;
>> +
>> + hw = &div->hw;
>> + ret = devm_clk_hw_register(dev, hw);
>> + if (ret)
>> + return ERR_PTR(ret);
>> + return hw;
>> +}
>> diff --git a/drivers/clk/nuvoton/clk-ma35d1-pll.c b/drivers/clk/nuvoton/clk-ma35d1-pll.c
>> new file mode 100644
>> index 000000000000..6de67c964a2d
>> --- /dev/null
>> +++ b/drivers/clk/nuvoton/clk-ma35d1-pll.c
>> @@ -0,0 +1,315 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2023 Nuvoton Technology Corp.
>> + * Author: Chi-Fang Li <cfli0@...oton.com>
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/container_of.h>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +
>> +#include "clk-ma35d1.h"
>> +
>> +struct ma35d1_clk_pll {
>> + struct clk_hw hw;
>> + u8 type;
>> + u8 mode;
>> + void __iomem *ctl0_base;
>> + void __iomem *ctl1_base;
>> + void __iomem *ctl2_base;
>> +};
>> +
> [..]
>> +struct clk_hw *ma35d1_reg_clk_pll(enum ma35d1_pll_type type,
>> + struct device *dev,
>> + u8 u8mode, const char *name,
>> + const char *parent,
>> + void __iomem *base)
>> +{
>> + struct ma35d1_clk_pll *pll;
>> + struct clk_hw *hw;
>> + struct clk_init_data init = {};
>> + int ret;
>> +
>> + pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
>> + if (!pll)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + pll->type = type;
>> + pll->mode = u8mode;
>> + pll->ctl0_base = base + REG_PLL_CTL0_OFFSET;
>> + pll->ctl1_base = base + REG_PLL_CTL1_OFFSET;
>> + pll->ctl2_base = base + REG_PLL_CTL2_OFFSET;
>> +
>> + init.name = name;
>> + init.flags = 0;
>> + init.parent_names = &parent;
> Can you use parent_data instead?
I will fix it.
>
>> + init.num_parents = 1;
>> +
>> + if (type == MA35D1_CAPLL || type == MA35D1_DDRPLL)
>> + init.ops = &ma35d1_clk_fixed_pll_ops;
>> + else
>> + init.ops = &ma35d1_clk_pll_ops;
>> +
>> + pll->hw.init = &init;
>> + hw = &pll->hw;
>> +
>> + ret = devm_clk_hw_register(dev, hw);
>> + if (ret)
>> + return ERR_PTR(ret);
>> + return hw;
>> +}
>> diff --git a/drivers/clk/nuvoton/clk-ma35d1.c b/drivers/clk/nuvoton/clk-ma35d1.c
>> new file mode 100644
>> index 000000000000..066e1c6f2d35
>> --- /dev/null
>> +++ b/drivers/clk/nuvoton/clk-ma35d1.c
>> @@ -0,0 +1,897 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2023 Nuvoton Technology Corp.
>> + * Author: Chi-Fang Li <cfli0@...oton.com>
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/spinlock.h>
>> +#include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
>> +
>> +#include "clk-ma35d1.h"
>> +
>> +static DEFINE_SPINLOCK(ma35d1_lock);
>> +
>> +static const struct clk_parent_data ca35clk_sel_clks[] = {
>> + { .fw_name = "hxt", .name = "hxt" },
>> + { .fw_name = "capll", .name = "capll" },
>> + { .fw_name = "ddrpll", .name = "ddrpll" },
>> + { .fw_name = "dummy", .name = "dummy" }
> What is 'dummy'? Is that in the binding? Note, don't put both .fw_name
> and .name in the binding. For new drivers, prefer to use .index or .hw
> and never use .name to describe parents.
I will remove "dummy" and use .index or .hw instead.
>> +};
>> +
>> +static const char *const sysclk0_sel_clks[] = {
>> + "epll_div2", "syspll"
> [...]
>> diff --git a/drivers/clk/nuvoton/clk-ma35d1.h b/drivers/clk/nuvoton/clk-ma35d1.h
>> new file mode 100644
>> index 000000000000..28c60f081788
>> --- /dev/null
>> +++ b/drivers/clk/nuvoton/clk-ma35d1.h
>> @@ -0,0 +1,123 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (C) 2023 Nuvoton Technology Corp.
>> + * Author: Chi-Fang Li <cfli0@...oton.com>
>> + */
>> +
>> +#ifndef __DRV_CLK_NUVOTON_MA35D1_H
>> +#define __DRV_CLK_NUVOTON_MA35D1_H
> Is this header included in one C file? If so, remove the header file and
> put the contents in the C file.
Both clk-ma35d1.c and clk-ma35d1-pll.c include this header file.
Best regards,
Jacky Huang
Powered by blists - more mailing lists