On 2023/1/30 18:07, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]
On Mon 30 Jan 2023 at 17:59, Yu Tu <yu.tu@xxxxxxxxxxx> wrote:
On 2023/1/30 17:47, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]
On Mon 30 Jan 2023 at 17:41, Yu Tu <yu.tu@xxxxxxxxxxx> wrote:
On 2023/1/30 17:06, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]
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
If I remove it, the "vid_pll_sel" clock will be missing a parent
(vid_pll_div). I will use the table method and give the above reasons. Do
you accept this method?
Either the clock exists or it does not.
If the HW actually exist, it is expected to be properly described.
If it does not, it obviously cannot be an input to another clock.
Please sort this out and make the necessary changes.
The CLKCTRL_VID_PLL_CLK_DIV register is actually described, but it is not
used in the actual board. According to your reply just now, description is
required, but I want to know how to describe it to meet your requirements.
Please give me some suggestions.
Implementing things is NOT about usage, it is about correctness.
Either there is actually a clock in the silicon you are producing at the
Amlogic factory, or there is not.
If the clock is there in the actual HW should be properly
described/implemented, as it "might" be used as an input to other clocks
- even if you personnaly don't.
If clock does not exists (nothing behind the registers, or broken, etc
...) then, yes you'll need to use parent tables and document this.
According to your suggestion, we need to describe the clock
(vid_pll_div).So it seems like we need to implement
"meson_vid_pll_div_ops" and do a commit first. Then submit the S4 SOC
clock driver. So change whether you take it or not?
Or if you have a better idea, let me know.
