Hi Sylwester,

On 02/04/2015 10:02 PM, Sylwester Nawrocki wrote:
> Hi Chanwoo,
> 
> On 02/02/15 15:24, Chanwoo Choi wrote:
>> This patch adds the mux/divider/gate clocks for CMU_BUS{0|1|2} domains
>> which contain global data buses clocked at up the 400MHz. These blocks
>> transfer data between DRAM and various sub-blocks. These clock domains
>> also contain global peripheral buses clocked at 67/111/200/222/266/333/400
>> MHz and used for register accesses.
> 
>> +/*
>> + * Register offset definitions for CMU_BUS{0|1|2}
>> + */
>> +#define DIV_BUS                             0x0600
>> +#define DIV_STAT_BUS                        0x0700
>> +#define ENABLE_ACLK_BUS                     0x0800
>> +#define ENABLE_PCLK_BUS                     0x0900
>> +#define ENABLE_IP_BUS0                      0x0b00
>> +#define ENABLE_IP_BUS1                      0x0b04
>> +
>> +#define MUX_SEL_BUS2                        0x0200  /* Only for CMU_BUS2 */
>> +#define MUX_ENABLE_BUS2                     0x0300  /* Only for CMU_BUS2 */
>> +#define MUX_STAT_BUS2                       0x0400  /* Only for CMU_BUS2 */
>> +
>> +/* list of all parent clock list */
>> +PNAME(mout_aclk_bus2_400_p) = { "oscclk", "aclk_bus2_400", };
>> +
>> +#define CMU_BUS_COMMON_CLK_REGS     \
>> +    DIV_BUS,                \
>> +    DIV_STAT_BUS,           \
>> +    ENABLE_ACLK_BUS,        \
>> +    ENABLE_PCLK_BUS,        \
>> +    ENABLE_IP_BUS0,         \
>> +    ENABLE_IP_BUS1
>> +
>> +static unsigned long bus01_clk_regs[] __initdata = {
>> +    CMU_BUS_COMMON_CLK_REGS,
>> +};
>> +
>> +static unsigned long bus2_clk_regs[] __initdata = {
>> +    MUX_SEL_BUS2,
>> +    MUX_ENABLE_BUS2,
>> +    MUX_STAT_BUS2,
>> +    CMU_BUS_COMMON_CLK_REGS,
>> +};
>> +
>> +static struct samsung_div_clock bus0_div_clks[] __initdata = {
>> +    /* DIV_BUS0 */
>> +    DIV(CLK_DIV_PCLK_BUS_133, "div_pclk_bus0_133", "aclk_bus0_400",
>> +                    DIV_BUS, 0, 3),
>> +};
>> +
>> +/* CMU_BUS0 clocks */
>> +static struct samsung_gate_clock bus0_gate_clks[] __initdata = {
>> +    /* ENABLE_ACLK_BUS0 */
>> +    GATE(CLK_ACLK_AHB2APB_BUSP, "aclk_ahb2apb_bus0p", "div_pclk_bus0_133",
>> +                    ENABLE_ACLK_BUS, 4, CLK_IGNORE_UNUSED, 0),
>> +    GATE(CLK_ACLK_BUSNP_133, "aclk_bus0np_133", "div_pclk_bus0_133",
>> +                    ENABLE_ACLK_BUS, 2, CLK_IGNORE_UNUSED, 0),
>> +    GATE(CLK_ACLK_BUSND_400, "aclk_bus0nd_400", "aclk_bus0_400",
>> +                    ENABLE_ACLK_BUS, 0, CLK_IGNORE_UNUSED, 0),
>> +
>> +    /* ENABLE_PCLK_BUS0 */
>> +    GATE(CLK_PCLK_BUSSRVND_133, "pclk_bus0srvnd_133", "div_pclk_bus0_133",
>> +                    ENABLE_PCLK_BUS, 2, 0, 0),
>> +    GATE(CLK_PCLK_PMU_BUS, "pclk_pmu_bus0", "div_pclk_bus0_133",
>> +                    ENABLE_PCLK_BUS, 1, CLK_IGNORE_UNUSED, 0),
>> +    GATE(CLK_PCLK_SYSREG_BUS, "pclk_sysreg_bus0", "div_pclk_bus0_133",
>> +                    ENABLE_PCLK_BUS, 0, CLK_IGNORE_UNUSED, 0),
>> +};
>> +
>> +/* CMU_BUS1 clocks */
>> +static struct samsung_div_clock bus1_div_clks[] __initdata = {
>> +    /* DIV_BUS1 */
>> +    DIV(CLK_DIV_PCLK_BUS_133, "div_pclk_bus1_133", "aclk_bus1_400",
>> +                    DIV_BUS, 0, 3),
>> +};
>> +
>> +static struct samsung_gate_clock bus1_gate_clks[] __initdata = {
>> +    /* ENABLE_ACLK_BUS1 */
>> +    GATE(CLK_ACLK_AHB2APB_BUSP, "aclk_ahb2apb_bus1p", "div_pclk_bus1_133",
>> +                    ENABLE_ACLK_BUS, 4, CLK_IGNORE_UNUSED, 0),
>> +    GATE(CLK_ACLK_BUSNP_133, "aclk_bus1np_133", "div_pclk_bus1_133",
>> +                    ENABLE_ACLK_BUS, 2, CLK_IGNORE_UNUSED, 0),
>> +    GATE(CLK_ACLK_BUSND_400, "aclk_bus1nd_400", "aclk_bus1_400",
>> +                    ENABLE_ACLK_BUS, 0, CLK_IGNORE_UNUSED, 0),
>> +
>> +    /* ENABLE_PCLK_BUS1 */
>> +    GATE(CLK_PCLK_BUSSRVND_133, "pclk_bus1srvnd_133", "div_pclk_bus1_133",
>> +                    ENABLE_PCLK_BUS, 2, 0, 0),
>> +    GATE(CLK_PCLK_PMU_BUS, "pclk_pmu_bus1", "div_pclk_bus1_133",
>> +                    ENABLE_PCLK_BUS, 1, CLK_IGNORE_UNUSED, 0),
>> +    GATE(CLK_PCLK_SYSREG_BUS, "pclk_sysreg_bus1", "div_pclk_bus1_133",
>> +                    ENABLE_PCLK_BUS, 0, CLK_IGNORE_UNUSED, 0),
>> +};
>> +
>> +/* CMU_BUS2 clocks */
>> +static struct samsung_mux_clock bus2_mux_clks[] __initdata = {
>> +    /* MUX_SEL_BUS2 */
>> +    MUX(CLK_MOUT_ACLK_BUS2_400_USER, "mout_aclk_bus2_400_user",
>> +                    mout_aclk_bus2_400_p, MUX_SEL_BUS2, 0, 1),
>> +};
>> +
>> +static struct samsung_div_clock bus2_div_clks[] __initdata = {
>> +    /* DIV_BUS2 */
>> +    DIV(CLK_DIV_PCLK_BUS_133, "div_pclk_bus2_133",
>> +                    "mout_aclk_bus2_400_user", DIV_BUS, 0, 3),
>> +};
>> +
>> +static struct samsung_gate_clock bus2_gate_clks[] __initdata = {
>> +    /* ENABLE_ACLK_BUS2 */
>> +    GATE(CLK_ACLK_AHB2APB_BUSP, "aclk_ahb2apb_bus2p", "div_pclk_bus2_133",
>> +                    ENABLE_ACLK_BUS, 3, CLK_IGNORE_UNUSED, 0),
>> +    GATE(CLK_ACLK_BUSNP_133, "aclk_bus2np_133", "div_pclk_bus2_133",
>> +                    ENABLE_ACLK_BUS, 2, CLK_IGNORE_UNUSED, 0),
>> +    GATE(CLK_ACLK_BUS2BEND_400, "aclk_bus2bend_400",
>> +                    "mout_aclk_bus2_400_user", ENABLE_ACLK_BUS,
>> +                    1, CLK_IGNORE_UNUSED, 0),
>> +    GATE(CLK_ACLK_BUS2RTND_400, "aclk_bus2rtnd_400",
>> +                    "mout_aclk_bus2_400_user", ENABLE_ACLK_BUS,
>> +                    0, CLK_IGNORE_UNUSED, 0),
>> +
>> +    /* ENABLE_PCLK_BUS2 */
>> +    GATE(CLK_PCLK_BUSSRVND_133, "pclk_bus2srvnd_133", "div_pclk_bus2_133",
>> +                    ENABLE_PCLK_BUS, 2, 0, 0),
>> +    GATE(CLK_PCLK_PMU_BUS, "pclk_pmu_bus2", "div_pclk_bus2_133",
>> +                    ENABLE_PCLK_BUS, 1, CLK_IGNORE_UNUSED, 0),
>> +    GATE(CLK_PCLK_SYSREG_BUS, "pclk_sysreg_bus2", "div_pclk_bus2_133",
>> +                    ENABLE_PCLK_BUS, 0, CLK_IGNORE_UNUSED, 0),
>> +};
>> +
>> +#define CMU_BUS_INFO_CLKS(id)                                               
>> \
>> +    .div_clks               = bus##id##_div_clks,                   \
>> +    .nr_div_clks            = ARRAY_SIZE(bus##id##_div_clks),       \
>> +    .gate_clks              = bus##id##_gate_clks,                  \
>> +    .nr_gate_clks           = ARRAY_SIZE(bus##id##_gate_clks),      \
>> +    .nr_clk_ids             = BUSx_NR_CLK
>> +
>> +static struct samsung_cmu_info bus0_cmu_info __initdata = {
>> +    CMU_BUS_INFO_CLKS(0),
>> +    .clk_regs               = bus01_clk_regs,
>> +    .nr_clk_regs            = ARRAY_SIZE(bus01_clk_regs),
>> +};
>> +
>> +static struct samsung_cmu_info bus1_cmu_info __initdata = {
>> +    CMU_BUS_INFO_CLKS(1),
>> +    .clk_regs               = bus01_clk_regs,
>> +    .nr_clk_regs            = ARRAY_SIZE(bus01_clk_regs),
>> +};
>> +
>> +static struct samsung_cmu_info bus2_cmu_info __initdata = {
>> +    CMU_BUS_INFO_CLKS(2),
>> +    .mux_clks               = bus2_mux_clks,
>> +    .nr_mux_clks            = ARRAY_SIZE(bus2_mux_clks),
>> +    .clk_regs               = bus2_clk_regs,
>> +    .nr_clk_regs            = ARRAY_SIZE(bus2_clk_regs),
>> +};
>> +
>> +#define exynos5433_cmu_bus_init(id)                                 \
>> +static void __init exynos5433_cmu_bus##id##_init(struct device_node *np)\
>> +{                                                                   \
>> +    samsung_cmu_register_one(np, &bus##id##_cmu_info);              \
>> +}                                                                   \
>> +CLK_OF_DECLARE(exynos5433_cmu_bus##id,                                      
>> \
>> +            "samsung,exynos5433-cmu-bus"#id,                        \
>> +            exynos5433_cmu_bus##id##_init)
>> +
>> +exynos5433_cmu_bus_init(0);
>> +exynos5433_cmu_bus_init(1);
>> +exynos5433_cmu_bus_init(2);
> 
> I picked up all the exynos5433 patches, however I don't quite like
> how we re defining 3 almost identical sets of data for CMU_BUS0/1/2.

Thanks for your pickup. In case of CMU_BUS0/1/2, I think there is a update
to remain.

> 
> We already have the parent clocks to CMU_BUSn specified in DT, then
> we could list the CMU_BUSn output clock names using clock-output-names
> property.  We could also differentiate CMU_BUS instances with an OF
> alias id, which could be used to create the CMU_BUS internal divider
> clock names (div_pclk_bus{0,1,2}_133).
> 
> This would require new data structures to store the register bit fields
> for each clock, clk names would be filled from DT.  It likely wouldn't
> be much less code this way, if not more.  And we would need to handle
> various clock types somehow, thus this would imply changes in the common
> samsung clk code.  I guess it's worth to try something like this in
> future (for any new SoCs) though, because repeating data as is done
> above doesn't look like a pattern we'd like to stay with.

I agree your opinion completely. If you need to help or co-work,
I can help in a variety of ways (e.g., test, implementation, debugging or other 
).

Best Regards,
Chanwoo Choi


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to