> -----Original Message----- > From: Stephen Boyd <sboyd@xxxxxxxxxx> > Sent: 2019年8月20日 2:30 > To: devicetree@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx; > linux-devel@xxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Mark Rutland > <mark.rutland@xxxxxxx>; Michael Turquette <mturquette@xxxxxxxxxxxx>; > Rob Herring <robh+dt@xxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; Wen > He <wen.he_1@xxxxxxx> > Cc: Leo Li <leoyang.li@xxxxxxx>; liviu.dudau@xxxxxxx > Subject: RE: [EXT] Re: [v2 2/3] clk: ls1028a: Add clock driver for Display output > interface > > Caution: EXT Email > > Quoting Wen He (2019-08-19 00:30:49) > > > Quoting Wen He (2019-08-15 03:16:12) > > > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index > > > > 801fa1cd0321..3c95d8ec31d4 100644 > > > > --- a/drivers/clk/Kconfig > > > > +++ b/drivers/clk/Kconfig > > > > @@ -223,6 +223,16 @@ config CLK_QORIQ > > > > This adds the clock driver support for Freescale QorIQ > platforms > > > > using common clock framework. > > > > > > > > +config CLK_LS1028A_PLLDIG > > > > + bool "Clock driver for LS1028A Display output" > > > > + depends on (ARCH_LAYERSCAPE || COMPILE_TEST) && OF > > > > > > Where is the OF dependency to build anything? Doesn't this still > > > compile without CONFIG_OF set? > > > > Yes, current included some APIs of the OF, like of_get_parent_name() > > And there isn't a stub API for of_get_parent_name when OF isn't defined? You are right, should be remove OF dependency. > > > > > + > > > > +static int plldig_clk_probe(struct platform_device *pdev) { > > > > + struct clk_plldig *data; > > > > + struct resource *mem; > > > > + const char *parent_name; > > > > + struct clk_init_data init = {}; > > > > + struct device *dev = &pdev->dev; > > > > + int ret; > > > > + > > > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > > > + if (!data) > > > > + return -ENOMEM; > > > > + > > > > + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > > + data->regs = devm_ioremap_resource(dev, mem); > > > > + if (IS_ERR(data->regs)) > > > > + return PTR_ERR(data->regs); > > > > + > > > > + init.name = dev->of_node->name; > > > > + init.ops = &plldig_clk_ops; > > > > + parent_name = of_clk_get_parent_name(dev->of_node, 0); > > > > + init.parent_names = &parent_name; > > > > > > Can you use the new way of specifying clk parents with the > > > parent_data member of clk_init? > > > > Of course, but I don't understand why need recommend to use this member? > > Is that the member parent_names will be discard in future? > > > > Here are definition of the clk-provider.h > > /* Only one of the following three should be assigned */ > > const char * const *parent_names; > > const struct clk_parent_data *parent_data; > > const struct clk_hw **parent_hws; > > > > For PLLDIG, it only has one parent. > > Yes. Can you use clk_parent_data array and specify a DT index of 0 and some > name that would go into "clock-names" in the .fw_name member? OK, but .fw_name used for to registering clk, current it registered with fixed clk in dts . I think should be specify a DT index of 0 and specify the unique name for .name member. I found have two ways to specify: 1. declare clk_parent_data variable parent_data, and initialization with clk_init_data, like this: parent_data.name = of_clk_get_parent_name(dev->of_node, 0); parent_data.index = 0; init.name = dev->of_node->name; init.ops = &plldig_clk_ops; init.parent_data = &parent_data; init.num_parents = 1; 2. Or use a static const array for here? And put the unique name and index like this. static const struct clk_parent_data parent_data[] = { {.name = "phy_27m", .index = 0}, }; After then initialization with macro CLK_HW_INIT_PARENTS_DATA? Best Regards, Wen