Hi Chris,
On Thu, Dec 15, 2016 at 3:58 AM, Chris Brandt <Chris.Brandt@xxxxxxxxxxx> wrote:
> On Dec 14, 2016, Kuninori Morimoto wrote:
>> I don't think using global variable is a good idea.
>> For example, how about add reg_width_8bit into mstp_clock_group, and
>> cpg_mstp_read/write requests it, or something like that ?
>
> If I make a separate CLK_OF_DECLARE like this:
>
> static void __init cpg_mstp_clocks_init8(struct device_node *np) {
> reg_width_8bit = true;
> cpg_mstp_clocks_init(np);
> }
> CLK_OF_DECLARE(cpg_mstp_clks8, "renesas,r7s72100-mstp-clocks",
> cpg_mstp_clocks_init8);
>
>
> The only way I can pass the 8-bit/32-bit choice into cpg_mstp_clocks_init()
> is using a global variable.
>
>
> Unless, I change the API to cpg_mstp_clocks_init(np, reg_width_8bit)
>
> But, that means adding another function:
>
> CLK_OF_DECLARE(cpg_mstp_clks32, "renesas,cpg-mstp-clocks",
> cpg_mstp_clocks_init32);
>
> CLK_OF_DECLARE(cpg_mstp_clks8, "renesas,r7s72100-mstp-clocks",
> cpg_mstp_clocks_init8);
>
> static void __init cpg_mstp_clocks_init32(struct device_node *np)
> {
> cpg_mstp_clocks_init(np, false);
> }
> static void __init cpg_mstp_clocks_init8(struct device_node *np)
> {
> cpg_mstp_clocks_init(np, true);
> }
Or you can add an of_device_is_compatible(np, "renesas,r7s72100-mstp-clocks")
check to cpg_mstp_clocks_init(), keeping the single CLK_OF_DECLARE().
I think that would also performs slightly better, as only nodes compatible
with "renesas,cpg-mstp-clocks" would be subject to the additional check.
> A global variable is much easier, but if no one likes it, I can change to
> mstp_clock_group.reg_width_8bit instead.
Personally, I have no problem with the global variable: either you're running
on RZ/A1, and all MSTP clocks need 8-bit accesses, or you're not.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
