On 2/18/22 23:15, Stephen Boyd wrote:

Hi,

quoted

quoted

quoted

@@ -68,6 +68,7 @@ obj-$(CONFIG_COMMON_CLK_STM32MP157)   += clk-stm32mp1.o
   obj-$(CONFIG_COMMON_CLK_TPS68470)      += clk-tps68470.o
   obj-$(CONFIG_CLK_TWL6040)              += clk-twl6040.o
   obj-$(CONFIG_ARCH_VT8500)              += clk-vt8500.o
+obj-$(CONFIG_COMMON_CLK_RS9_PCIE)      += clk-renesas-pcie.o

Is there a reason it doesn't go into drivers/clk/renesas?

The drivers/clk/renesas/ is for renesas SoC (R-Car/RZ/...),
this chip is different group (it's probably even IDT PLL IP).

Ah ok so it's not a renesas SoC but a renesas IP?

I suspect it is IDT IP, since the datasheet looks similar to what 
versaclock datasheets used to look like, except the register layout is 
much simpler and the registers are completely different. So it _might_ 
be some mutation of the IDT PLL, and it is now owned by renesas.

The renesas SoC clock IP is something entirely different from the IP 
used here.

quoted

quoted

quoted

+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>

Is this used? If not please remove.

This one is for of_device_get_match_data()

So it's going away?

Yes

[...]

quoted

quoted

Use clk_parent_data please.

This one line I don't understand -- can you expand on what you expect me
to do here ?

Use 'struct clk_parent_data' and set .index to 0 so that when
registering the clk you don't need to get the parent clk name.

quoted

quoted

quoted

+       if (!parent_clk)
+               return dev_err_probe(&client->dev, -EINVAL,
+                                    "Missing XTal input clock\n");
+
+       rs9->regmap = devm_regmap_init_i2c(client, &rs9_regmap_config);
+       if (IS_ERR(rs9->regmap))
+               return dev_err_probe(&client->dev, PTR_ERR(rs9->regmap),
+                                    "Failed to allocate register map\n");
+
+       /* Register clock */
+       for (i = 0; i < rs9->chip_info->num_clks; i++) {
+               name[3]++;
+               hw = clk_hw_register_fixed_factor(&client->dev, name,
+                                                 parent_clk, 0, 4, 1);

To do that it looks like maybe we'll need to export
__clk_hw_register_fixed_factor() and introduces some sort of
clk_hw_register_fixed_factor_parent_data() API.

Setting parent_clk to NULL should be enough.

[...]