On 11.5.2018 09:58, James Kelly wrote: > Hi Michal > > On 11 May 2018 at 16:06, Michal Simek <michal.simek@xxxxxxxxxx> wrote: > >> On 7.5.2018 03:20, James Kelly wrote: >>> The CCF clock providers that are currently used by the driver are not >>> capable of supporting the Clocking Wizard IP register interface for >>> fractional ratios, nor are they able to enforce constraints require to >>> ensure the PLL will always lock. >>> >>> None of the common CCF clock providers seem to be a good fit so we >>> implement a new CCF clock provider within the driver that can be expanded >>> upon in subsequent patches. The initial implementation supports multiply >>> or divide by fixed integer ratios. >>> >>> The CCF clock provider uses: >>> - devm kernel APIs for clock registration to simplify error recovery and >>> module unloading. >>> - regmap kernel APIs for memory mapped I/O. Regmap is also able to manage >>> prepare/enable/disable/unprepare of the AXI clock for us. >>> >>> Note that use of the new CCF clock provider is deferred to a subsequent >>> patch. >>> >>> Signed-off-by: James Kelly <jamespeterkelly@xxxxxxxxx> >>> --- >>> .../clocking-wizard/clk-xlnx-clock-wizard.c | 164 >> +++++++++++++++++++++ >>> 1 file changed, 164 insertions(+) >>> >>> diff --git a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c >> b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c >>> index 3b66ac3b5ed0..ba9b1dc93d50 100644 >>> --- a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c >>> +++ b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c >>> @@ -3,6 +3,7 @@ >>> * Xilinx 'Clocking Wizard' driver >>> * >>> * Copyright (C) 2013 - 2014 Xilinx >>> + * Copyright (C) 2018 James Kelly >>> * >>> * Sören Brinkmann <soren.brinkmann@xxxxxxxxxx> >>> * >>> @@ -67,11 +68,13 @@ >>> #include <linux/of.h> >>> #include <linux/module.h> >>> #include <linux/err.h> >>> +#include <linux/regmap.h> >>> >>> #define WZRD_NUM_OUTPUTS 7 >>> #define WZRD_ACLK_MAX_FREQ 250000000UL >>> #define WZRD_CLKNAME_AXI "s_axi_aclk" >>> #define WZRD_CLKNAME_IN1 "clk_in1" >>> +#define WZRD_FLAG_MULTIPLY BIT(0) >>> >>> #define WZRD_CLK_CFG_REG(n) (0x200 + 4 * (n)) >>> >>> @@ -91,28 +94,90 @@ enum clk_wzrd_int_clks { >>> wzrd_clk_int_max >>> }; >>> >>> +enum clk_wzrd_clk { >>> + WZRD_CLK_DIV, >>> + WZRD_CLK_PLL, >>> + WZRD_CLK_OUT, >>> + WZRD_CLK_COUNT >>> +}; >>> + >>> +/* >>> + * Register map extracted from Xilinx document listed below. >>> + * >>> + * PG065 Clocking Wizard v6.0 LogiCORE IP Product Guide >>> + */ >>> +static const struct regmap_config clk_wzrd_regmap_config = { >>> + .reg_bits = 32, >>> + .reg_stride = 4, >>> + .val_bits = 32 >>> +}; >>> + >>> +static const struct reg_field clk_wzrd_status_locked = >> REG_FIELD(0x004, 0, 0); >>> +static const struct reg_field clk_wzrd_divclk_divide = >> REG_FIELD(0x200, 0, 7); >>> +static const struct reg_field clk_wzrd_clkfbout_mult = >> REG_FIELD(0x200, 8, 15); >>> +static const struct reg_field clk_wzrd_clkfbout_frac = REG_FIELD(0x200, >> 16, 25); >>> +static const struct reg_field clk_wzrd_clkout_divide[WZRD_NUM_OUTPUTS] >> = { >>> + REG_FIELD(0x208, 0, 7), >>> + REG_FIELD(0x214, 0, 7), >>> + REG_FIELD(0x220, 0, 7), >>> + REG_FIELD(0x22C, 0, 7), >>> + REG_FIELD(0x238, 0, 7), >>> + REG_FIELD(0x244, 0, 7), >>> + REG_FIELD(0x250, 0, 7) >>> +}; >>> + >>> +static const struct reg_field clk_wzrd_clkout0_frac = REG_FIELD(0x208, >> 8, 17); >>> +static const struct reg_field clk_wzrd_reconfig = REG_FIELD(0x25C, >> 0, 1); >>> + >>> +/** >>> + * struct clk_wzrd_clk_data - Clocking Wizard component clock provider >> data >>> + * >>> + * @hw: handle between common and >> hardware-specific interfaces >>> + * @flags: hardware specific flags >>> + * @int_field: pointer to regmap field for integer part >>> + * >>> + * Flags: >>> + * WZRD_FLAG_MULTIPLY Clock is a multiplier rather than a divider >>> + */ >>> +struct clk_wzrd_clk_data { >>> + struct clk_hw hw; >>> + unsigned long flags; >>> + struct regmap_field *int_field; >>> +}; >>> + >>> +#define to_clk_wzrd_clk_data(_hw) \ >>> + container_of(_hw, struct clk_wzrd_clk_data, hw) >>> + >>> /** >>> * struct clk_wzrd: >>> * @clk_data: Clock data >>> * @nb: Notifier block >>> * @base: Memory base >>> + * @regmap: Register map for hardware >>> * @clk_in1: Handle to input clock 'clk_in1' >>> * @axi_clk: Handle to input clock 's_axi_aclk' >>> * @clks_internal: Internal clocks >>> * @clkout: Output clocks >>> * @speed_grade: Speed grade of the device >>> * @suspended: Flag indicating power state of the device >>> + * @div: Divider internal clock provider data >>> + * @pll: Phase locked loop internal clock provider data >>> + * @clkout_data: Array of output clock provider data >>> */ >>> struct clk_wzrd { >>> struct clk_onecell_data clk_data; >>> struct notifier_block nb; >>> void __iomem *base; >>> + struct regmap *regmap; >>> struct clk *clk_in1; >>> struct clk *axi_clk; >>> struct clk *clks_internal[wzrd_clk_int_max]; >>> struct clk *clkout[WZRD_NUM_OUTPUTS]; >>> unsigned int speed_grade; >>> bool suspended; >>> + struct clk_wzrd_clk_data div_data; >>> + struct clk_wzrd_clk_data pll_data; >> >> Names don't fit with what it is added to kernel-doc above. >> >> drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c:152: info: >> Scanning doc for struct >> drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c:181: warning: >> Function parameter or member 'div_data' not described in 'clk_wzrd' >> drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c:181: warning: >> Function parameter or member 'pll_data' not described in 'clk_wzrd' >> >> Anyway. Shubhrajyoti is going to test this series that's why please give >> us some time for it. >> >> Thanks, >> Michal >> >> > Thanks for your feedback on this and the other patches in this series. > > I will incorporate your suggestions into version 2 which I plan to send > to the list early next week. It will be great if you can wait for my colleague's feedback from testing. Recently we have done similar changes in this driver to extend functionality that's why it shouldn't be hard to test/compare and comment. Thanks, Michal _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel