On Sat 28 Jan 2023 at 18:17, Yu Tu <yu.tu@xxxxxxxxxxx> wrote: > On 2023/1/20 17:47, Jerome Brunet wrote: >> [ EXTERNAL EMAIL ] >> On Fri 20 Jan 2023 at 11:33, Yu Tu <yu.tu@xxxxxxxxxxx> wrote: >> >>> Hi >>> On 2023/1/19 19:37, Jerome Brunet wrote: >>>> [ EXTERNAL EMAIL ] >>>> On Mon 16 Jan 2023 at 15:42, Yu Tu <yu.tu@xxxxxxxxxxx> wrote: >>>> >>>>> Add the peripherals clock controller driver in the s4 SoC family. >>>>> >>>>> Signed-off-by: Yu Tu <yu.tu@xxxxxxxxxxx> >>>> [...] >>>> >>>>> + >>>>> +/* Video Clocks */ >>>>> +static struct clk_regmap s4_vid_pll_div = { >>>>> + .data = &(struct meson_vid_pll_div_data){ >>>>> + .val = { >>>>> + .reg_off = CLKCTRL_VID_PLL_CLK_DIV, >>>>> + .shift = 0, >>>>> + .width = 15, >>>>> + }, >>>>> + .sel = { >>>>> + .reg_off = CLKCTRL_VID_PLL_CLK_DIV, >>>>> + .shift = 16, >>>>> + .width = 2, >>>>> + }, >>>>> + }, >>>>> + .hw.init = &(struct clk_init_data) { >>>>> + .name = "vid_pll_div", >>>>> + /* >>>>> + * The frequency division from the hdmi_pll clock to the vid_pll_div >>>>> + * clock is the default value of this register. When designing the >>>>> + * video module of the chip, a default value that can meet the >>>>> + * requirements of the video module will be solidified according >>>>> + * to the usage requirements of the chip, so as to facilitate chip >>>>> + * simulation. So this is ro_ops. >>>>> + * It is important to note that this clock is not used on this >>>>> + * chip and is described only for the integrity of the clock tree. >>>>> + */ >>>> If it is reset value and will be applicable to all the design, regarless >>>> of the use-case, then yes RO ops is OK >>>> >>>> >From what I understand here, the value will depend on the use-case requirements. >>>> This is a typical case where the DT prop "assigned-rate" should be used, not RO ops. >>> >>> Check the previous chip history, the actual scene is not used at all, >>> basically is used in simulation. So the previous SOC was "ro_ops" without >>> any problems. This S4 SOC is not actually useful either. >>> >>> So when you were upstream, you had no problem making "ro_ops". I wonder if >>> I could delete this useless clock, so you don't have to worry about it. >> I don't know what to make of this. What is the point of adding a useless >> clock ? > > As explained earlier this "vid_pll_div" is actually used in chip > emulation. So next I'd like to know what you suggest to do with the clock? > If it does not exist in the actual SoC, please remove it