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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAS=xmjsFoQg-HgWA5-e=cF3qF+PcVNc3i_3Z5EBPNJ2HyOZKQ@mail.gmail.com>
Date:	Sat, 16 May 2015 10:54:58 +0800
From:	Brent Wang <wangbintian@...il.com>
To:	Stephen Boyd <sboyd@...eaurora.org>
Cc:	Bintian <bintian.wang@...wei.com>,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Kevin Hilman <khilman@...aro.org>,
	Mike Turquette <mturquette@...aro.org>,
	Rob Herring <rob.herring@...aro.org>,
	Zhangfei Gao <zhangfei.gao@...aro.org>,
	Haojian Zhuang <haojian.zhuang@...aro.org>,
	Xu Wei <xuwei5@...ilicon.com>,
	Jaehoon Chung <jh80.chung@...sung.com>,
	Olof Johansson <olof@...om.net>,
	Haifeng Yan <yanhaifeng@...il.com>,
	"xuejiancheng@...wei.com" <xuejiancheng@...wei.com>,
	"sledge.yanwei@...wei.com" <sledge.yanwei@...wei.com>,
	Tomeu Vizoso <tomeu.vizoso@...labora.com>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	Guodong Xu <guodong.xu@...aro.org>,
	Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@...aro.org>,
	Tyler Baker <tyler.baker@...aro.org>,
	Kevin Hilman <khilman@...nel.org>, pebolle@...cali.nl,
	Arnd Bergmann <arnd@...db.de>,
	Marc Zyngier <marc.zyngier@....com>,
	"xuyiping@...ilicon.com" <xuyiping@...ilicon.com>,
	"wangbinghui@...ilicon.com" <wangbinghui@...ilicon.com>,
	"zhenwei.wang@...ilicon.com" <zhenwei.wang@...ilicon.com>,
	"victor.lixin@...ilicon.com" <victor.lixin@...ilicon.com>,
	"puck.chen@...ilicon.com" <puck.chen@...ilicon.com>,
	"dan.zhao@...ilicon.com" <dan.zhao@...ilicon.com>,
	"huxinwei@...wei.com" <huxinwei@...wei.com>,
	"z.liuxinliang@...wei.com" <z.liuxinliang@...wei.com>,
	"heyunlei@...wei.com" <heyunlei@...wei.com>,
	XinWei Kong <kong.kongxinwei@...ilicon.com>,
	"btw@...l.itp.ac.cn" <btw@...l.itp.ac.cn>,
	"w.f@...wei.com" <w.f@...wei.com>,
	"Liguozhu (Kenneth)" <liguozhu@...ilicon.com>
Subject: Re: [PATCH v4 4/5] clk: hi6220: Clock driver support for Hisilicon
 hi6220 SoC

Hello  Stephen,

2015-05-16 3:30 GMT+08:00 Stephen Boyd <sboyd@...eaurora.org>:
> On 05/15, Bintian wrote:
>> On 2015/5/15 8:25, Stephen Boyd wrote:
>> >On 05/05, Bintian Wang wrote:
>> >>diff --git a/drivers/clk/hisilicon/clkdivider-hi6220.c b/drivers/clk/hisilicon/clkdivider-hi6220.c
>> >>+
>> >>+/**
>> >>+ * struct hi6220_clk_divider - divider clock for hi6220
>> >>+ *
>> >>+ * @hw:            handle between common and hardware-specific interfaces
>> >>+ * @reg:   register containing divider
>> >>+ * @shift: shift to the divider bit field
>> >>+ * @width: width of the divider bit field
>> >>+ * @mask:  mask for setting divider rate
>> >>+ * @table: the div table that the divider supports
>> >>+ * @lock:  register lock
>> >>+ */
>> >>+struct hi6220_clk_divider {
>> >>+   struct clk_hw   hw;
>> >>+   void __iomem    *reg;
>> >>+   u8              shift;
>> >>+   u8              width;
>> >>+   u32             mask;
>> >>+   const struct clk_div_table *table;
>> >>+   spinlock_t      *lock;
>> >>+};
>> >
>> >The clk-divider.c code has been made "reusable". Can you please
>> >try to use the functions that it now exposes instead of
>> >copy/pasting it and modifying it to suit your needs? A lot of
>> >this code looks the same.
>> In fact, I discussed this problem with Rob Herring and Mike Turquette
>> in the 96boards internal mail list before.
>>
>> The divider in hi6220 has a mask bit to guarantee writing the correct
>> bits in register when setting rate, but the index of this mask bit has
>> no rules to get (e.g. by left shift some fixed bits), so I add this
>> divider clock to handle it, we can regard hi6220_clk_divider as a
>> special case of generic divider clock.
>>
>> If I don't add this divider clock for hi6220 chip, then I should change
>> the core APIs "clk_register_divider" and "clk_register_divider_table",
>> and then many other drivers will be updated.
>> So I think just add this divider clock is a good solution now.
>
> I think you missed my point. I didn't suggest using
> clk_register_divider or clk_register_divider_table(). I'm
> suggesting to use
>
> unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
>                 unsigned int val, const struct clk_div_table *table,
>                 unsigned long flags);
> long divider_round_rate(struct clk_hw *hw, unsigned long rate,
>                 unsigned long *prate, const struct clk_div_table *table,
>                 u8 width, unsigned long flags);
> int divider_get_val(unsigned long rate, unsigned long parent_rate,
>                 const struct clk_div_table *table, u8 width,
>                 unsigned long flags);
Got it and I will prepare next version soon.

>
>> >>+           return ERR_PTR(-ENOMEM);
>> >>+   }
>> >>+
>> >>+   for (i = 0; i < max_div; i++) {
>> >>+           table[i].div = min_div + i;
>> >>+           table[i].val = table[i].div - 1;
>> >>+   }
>> >>+
>> >>+   init.name = name;
>> >>+   init.ops = &hi6220_clkdiv_ops;
>> >>+   init.flags = flags | CLK_IS_BASIC;
>> >
>> >It's basic?
>> I rechecked this flag, it's really useless to us, so I can remove it.
>> But can you tell me which case I should use it?
>
> I think the basic flag is there for drivers that want to know what type
> of clock they're dealing with when all they have is the struct clk_hw
> pointer. I like to discourage use of this flag in hopes of deleting
> it someday.
>
>>
>> How about just send this patch for review not the whole patch set in
>> next version?
>>
>
> Yes a single patch is fine. I take it you want the patch to go
> through arm-soc with some Ack from us?
Yes, exactly.
The dts file includes the clock head file,  this patch goes through
arm-soc is a good choice.

Thanks,

Bintian
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ