Re: [PATCH 05/14] staging: clocking-wizard: Implement CCF clock provider

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux