| Message ID | 1465283488-30830-2-git-send-email-wenyou.yang@atmel.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Wenyou, On 7 June 2016 at 01:11, Wenyou Yang <wenyou.yang@atmel.com> wrote: > Add post binding support for CLK uclass to recursively bind > its children as clk devices. > > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> > --- > > Changes in v2: None > > drivers/clk/clk-uclass.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) Can you please explain what this is for? We would normally call dm_scan_fdt_node() for this, but it seems that you are specifically to create drivers for things with no compatible strings. > > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c > index b483c1e..ee568e4 100644 > --- a/drivers/clk/clk-uclass.c > +++ b/drivers/clk/clk-uclass.c > @@ -106,7 +106,58 @@ int clk_get_by_index(struct udevice *dev, int index, struct udevice **clk_devp) > } > #endif > > +/** > + * clk_post-bind() - post binding for CLK uclass > + * Recursively bind its children as clk devices. > + * > + * @dev: clk device > + * @return: 0 on success, or negative error code on failure > + */ > +static int clk_post_bind(struct udevice *dev) > +{ > + const void *fdt = gd->fdt_blob; > + int offset = dev->of_offset; > + bool pre_reloc_only = !(gd->flags & GD_FLG_RELOC); > + const char *name; > + int ret; > + > + for (offset = fdt_first_subnode(fdt, offset); > + offset > 0; > + offset = fdt_next_subnode(fdt, offset)) { > + if (pre_reloc_only && > + !fdt_getprop(fdt, offset, "u-boot,dm-pre-reloc", NULL)) > + continue; > + /* > + * If this node has "compatible" property, this is not a clk > + * node, but a normal device. skip. > + */ > + fdt_get_property(fdt, offset, "compatible", &ret); > + if (ret >= 0) > + continue; > + > + if (ret != -FDT_ERR_NOTFOUND) > + return ret; > + > + name = fdt_get_name(fdt, offset, NULL); > + if (!name) > + return -EINVAL; > + > + ret = device_bind_driver_to_node(dev, "clk", name, > + offset, NULL); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > UCLASS_DRIVER(clk) = { > .id = UCLASS_CLK, > + .post_bind = clk_post_bind, > .name = "clk", > }; > + > +U_BOOT_DRIVER(clk_generic) = { > + .id = UCLASS_CLK, > + .name = "clk", > +}; > -- > 2.7.4 > Regards, Simon
> -----Original Message----- > From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass > Sent: 2016年6月10日 8:34 > To: Yang, Wenyou <Wenyou.Yang@atmel.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Andreas Bießmann > <andreas@biessmann.org> > Subject: Re: [PATCH v2 1/2] clk: clk-uclass: Add post binding for CLK uclass > > Hi Wenyou, > > On 7 June 2016 at 01:11, Wenyou Yang <wenyou.yang@atmel.com> wrote: > > Add post binding support for CLK uclass to recursively bind its > > children as clk devices. > > > > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> > > --- > > > > Changes in v2: None > > > > drivers/clk/clk-uclass.c | 51 > > ++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > > Can you please explain what this is for? We would normally call > dm_scan_fdt_node() for this, but it seems that you are specifically to create > drivers for things with no compatible strings. Yes, for pheriph32ck node have a lot of child nodes with no compatible strings. This patch purpose is to bind these children nodes. Otherwise these nodes will not be bound. For example, ---8<---- ahb { apb { pmc: pmc@f0014000 { [snip] periph32ck { compatible = "atmel,at91sam9x5-clk-peripheral"; #address-cells = <1>; #size-cells = <0>; clocks = <&h32ck>; macb0_clk: macb0_clk { #clock-cells = <0>; reg = <5>; atmel,clk-output-range = <0 83000000>; }; [snip] pioA_clk: pioA_clk { #clock-cells = <0>; reg = <18>; atmel,clk-output-range = <0 83000000>; }; [snip] spi0_clk: spi0_clk { #clock-cells = <0>; reg = <33>; atmel,clk-output-range = <0 83000000>; }; [snip] }; }; spi0: spi@f8000000 { compatible = "atmel,at91rm9200-spi"; reg = <0xf8000000 0x100>; clocks = <&spi0_clk>; clock-names = "spi_clk"; #address-cells = <1>; #size-cells = <0>; status = "disabled"; }; macb0: ethernet@f8008000 { compatible = "cdns,macb"; reg = <0xf8008000 0x1000>; #address-cells = <1>; #size-cells = <0>; clocks = <&macb0_clk>, <&macb0_clk>; clock-names = "hclk", "pclk"; status = "disabled"; }; [snip] pioA: gpio@fc038000 { compatible = "atmel,sama5d2-gpio"; reg = <0xfc038000 0x600>; clocks = <&pioA_clk>; gpio-controller; #gpio-cells = <2>; pinctrl { compatible = "atmel,sama5d2-pinctrl"; }; }; }; }; ---->8----- > > > > > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index > > b483c1e..ee568e4 100644 > > --- a/drivers/clk/clk-uclass.c > > +++ b/drivers/clk/clk-uclass.c > > @@ -106,7 +106,58 @@ int clk_get_by_index(struct udevice *dev, int > > index, struct udevice **clk_devp) } #endif > > > > +/** > > + * clk_post-bind() - post binding for CLK uclass > > + * Recursively bind its children as clk devices. > > + * > > + * @dev: clk device > > + * @return: 0 on success, or negative error code on failure */ > > +static int clk_post_bind(struct udevice *dev) { > > + const void *fdt = gd->fdt_blob; > > + int offset = dev->of_offset; > > + bool pre_reloc_only = !(gd->flags & GD_FLG_RELOC); > > + const char *name; > > + int ret; > > + > > + for (offset = fdt_first_subnode(fdt, offset); > > + offset > 0; > > + offset = fdt_next_subnode(fdt, offset)) { > > + if (pre_reloc_only && > > + !fdt_getprop(fdt, offset, "u-boot,dm-pre-reloc", NULL)) > > + continue; > > + /* > > + * If this node has "compatible" property, this is not a clk > > + * node, but a normal device. skip. > > + */ > > + fdt_get_property(fdt, offset, "compatible", &ret); > > + if (ret >= 0) > > + continue; > > + > > + if (ret != -FDT_ERR_NOTFOUND) > > + return ret; > > + > > + name = fdt_get_name(fdt, offset, NULL); > > + if (!name) > > + return -EINVAL; > > + > > + ret = device_bind_driver_to_node(dev, "clk", name, > > + offset, NULL); > > + if (ret) > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > UCLASS_DRIVER(clk) = { > > .id = UCLASS_CLK, > > + .post_bind = clk_post_bind, > > .name = "clk", > > }; > > + > > +U_BOOT_DRIVER(clk_generic) = { > > + .id = UCLASS_CLK, > > + .name = "clk", > > +}; > > -- > > 2.7.4 > > > > Regards, > Simon
Hi Wenyou, On 15 June 2016 at 19:08, Yang, Wenyou <Wenyou.Yang@atmel.com> wrote: > > >> -----Original Message----- >> From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass >> Sent: 2016年6月10日 8:34 >> To: Yang, Wenyou <Wenyou.Yang@atmel.com> >> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Andreas Bießmann >> <andreas@biessmann.org> >> Subject: Re: [PATCH v2 1/2] clk: clk-uclass: Add post binding for CLK uclass >> >> Hi Wenyou, >> >> On 7 June 2016 at 01:11, Wenyou Yang <wenyou.yang@atmel.com> wrote: >> > Add post binding support for CLK uclass to recursively bind its >> > children as clk devices. >> > >> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> >> > --- >> > >> > Changes in v2: None >> > >> > drivers/clk/clk-uclass.c | 51 >> > ++++++++++++++++++++++++++++++++++++++++++++++++ >> > 1 file changed, 51 insertions(+) >> >> >> Can you please explain what this is for? We would normally call >> dm_scan_fdt_node() for this, but it seems that you are specifically to create >> drivers for things with no compatible strings. > > Yes, for pheriph32ck node have a lot of child nodes with no compatible strings. > This patch purpose is to bind these children nodes. Otherwise these nodes will not be bound. > > For example, > > ---8<---- > ahb { > apb { > pmc: pmc@f0014000 { > > [snip] > > periph32ck { > compatible = "atmel,at91sam9x5-clk-peripheral"; > #address-cells = <1>; > #size-cells = <0>; > clocks = <&h32ck>; > > macb0_clk: macb0_clk { > #clock-cells = <0>; > reg = <5>; > atmel,clk-output-range = <0 83000000>; > }; > > [snip] > > pioA_clk: pioA_clk { > #clock-cells = <0>; > reg = <18>; > atmel,clk-output-range = <0 83000000>; > }; > > [snip] > > spi0_clk: spi0_clk { > #clock-cells = <0>; > reg = <33>; > atmel,clk-output-range = <0 83000000>; > }; > > [snip] > }; > }; > > spi0: spi@f8000000 { > compatible = "atmel,at91rm9200-spi"; > reg = <0xf8000000 0x100>; > clocks = <&spi0_clk>; > clock-names = "spi_clk"; > #address-cells = <1>; > #size-cells = <0>; > status = "disabled"; > }; > > macb0: ethernet@f8008000 { > compatible = "cdns,macb"; > reg = <0xf8008000 0x1000>; > #address-cells = <1>; > #size-cells = <0>; > clocks = <&macb0_clk>, <&macb0_clk>; > clock-names = "hclk", "pclk"; > status = "disabled"; > }; > > [snip] > > pioA: gpio@fc038000 { > compatible = "atmel,sama5d2-gpio"; > reg = <0xfc038000 0x600>; > clocks = <&pioA_clk>; > gpio-controller; > #gpio-cells = <2>; > > pinctrl { > compatible = "atmel,sama5d2-pinctrl"; > }; > }; > }; > }; > > ---->8----- > In that case I think your clock driver should handle this in its bind() method. It can scan the device tree and create the devices as needed. I don't think this feature should go into generic code, at least not yet. Also please see Stephen Warren's clock patch, which supports multiple clock IDs per clock device. Assuming he respins this soon I will be applying it to dm/master. http://patchwork.ozlabs.org/patch/625342/ >> >> > >> > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index >> > b483c1e..ee568e4 100644 >> > --- a/drivers/clk/clk-uclass.c >> > +++ b/drivers/clk/clk-uclass.c >> > @@ -106,7 +106,58 @@ int clk_get_by_index(struct udevice *dev, int >> > index, struct udevice **clk_devp) } #endif >> > >> > +/** >> > + * clk_post-bind() - post binding for CLK uclass >> > + * Recursively bind its children as clk devices. >> > + * >> > + * @dev: clk device >> > + * @return: 0 on success, or negative error code on failure */ >> > +static int clk_post_bind(struct udevice *dev) { >> > + const void *fdt = gd->fdt_blob; >> > + int offset = dev->of_offset; >> > + bool pre_reloc_only = !(gd->flags & GD_FLG_RELOC); >> > + const char *name; >> > + int ret; >> > + >> > + for (offset = fdt_first_subnode(fdt, offset); >> > + offset > 0; >> > + offset = fdt_next_subnode(fdt, offset)) { >> > + if (pre_reloc_only && >> > + !fdt_getprop(fdt, offset, "u-boot,dm-pre-reloc", NULL)) >> > + continue; >> > + /* >> > + * If this node has "compatible" property, this is not a clk >> > + * node, but a normal device. skip. >> > + */ >> > + fdt_get_property(fdt, offset, "compatible", &ret); >> > + if (ret >= 0) >> > + continue; >> > + >> > + if (ret != -FDT_ERR_NOTFOUND) >> > + return ret; >> > + >> > + name = fdt_get_name(fdt, offset, NULL); >> > + if (!name) >> > + return -EINVAL; >> > + >> > + ret = device_bind_driver_to_node(dev, "clk", name, >> > + offset, NULL); >> > + if (ret) >> > + return ret; >> > + } >> > + >> > + return 0; >> > +} >> > + >> > UCLASS_DRIVER(clk) = { >> > .id = UCLASS_CLK, >> > + .post_bind = clk_post_bind, >> > .name = "clk", >> > }; >> > + >> > +U_BOOT_DRIVER(clk_generic) = { >> > + .id = UCLASS_CLK, >> > + .name = "clk", >> > +}; >> > -- >> > 2.7.4 >> > Regards, Simon
HI Simon, > -----Original Message----- > From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass > Sent: 2016年6月16日 10:44 > To: Yang, Wenyou <Wenyou.Yang@atmel.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Andreas Bießmann > <andreas@biessmann.org> > Subject: Re: [PATCH v2 1/2] clk: clk-uclass: Add post binding for CLK uclass > > Hi Wenyou, > > On 15 June 2016 at 19:08, Yang, Wenyou <Wenyou.Yang@atmel.com> wrote: > > > > > >> -----Original Message----- > >> From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass > >> Sent: 2016年6月10日 8:34 > >> To: Yang, Wenyou <Wenyou.Yang@atmel.com> > >> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Andreas Bießmann > >> <andreas@biessmann.org> > >> Subject: Re: [PATCH v2 1/2] clk: clk-uclass: Add post binding for CLK > >> uclass > >> > >> Hi Wenyou, > >> > >> On 7 June 2016 at 01:11, Wenyou Yang <wenyou.yang@atmel.com> wrote: > >> > Add post binding support for CLK uclass to recursively bind its > >> > children as clk devices. > >> > > >> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> > >> > --- > >> > > >> > Changes in v2: None > >> > > >> > drivers/clk/clk-uclass.c | 51 > >> > ++++++++++++++++++++++++++++++++++++++++++++++++ > >> > 1 file changed, 51 insertions(+) > >> > >> > >> Can you please explain what this is for? We would normally call > >> dm_scan_fdt_node() for this, but it seems that you are specifically > >> to create drivers for things with no compatible strings. > > > > Yes, for pheriph32ck node have a lot of child nodes with no compatible strings. > > This patch purpose is to bind these children nodes. Otherwise these nodes will > not be bound. > > > > For example, > > > > ---8<---- > > ahb { > > apb { > > pmc: pmc@f0014000 { > > > > [snip] > > > > periph32ck { > > compatible = "atmel,at91sam9x5-clk-peripheral"; > > #address-cells = <1>; > > #size-cells = <0>; > > clocks = <&h32ck>; > > > > macb0_clk: macb0_clk { > > #clock-cells = <0>; > > reg = <5>; > > atmel,clk-output-range = <0 83000000>; > > }; > > > > [snip] > > > > pioA_clk: pioA_clk { > > #clock-cells = <0>; > > reg = <18>; > > atmel,clk-output-range = <0 83000000>; > > }; > > > > [snip] > > > > spi0_clk: spi0_clk { > > #clock-cells = <0>; > > reg = <33>; > > atmel,clk-output-range = <0 83000000>; > > }; > > > > [snip] > > }; > > }; > > > > spi0: spi@f8000000 { > > compatible = "atmel,at91rm9200-spi"; > > reg = <0xf8000000 0x100>; > > clocks = <&spi0_clk>; > > clock-names = "spi_clk"; > > #address-cells = <1>; > > #size-cells = <0>; > > status = "disabled"; > > }; > > > > macb0: ethernet@f8008000 { > > compatible = "cdns,macb"; > > reg = <0xf8008000 0x1000>; > > #address-cells = <1>; > > #size-cells = <0>; > > clocks = <&macb0_clk>, <&macb0_clk>; > > clock-names = "hclk", "pclk"; > > status = "disabled"; > > }; > > > > [snip] > > > > pioA: gpio@fc038000 { > > compatible = "atmel,sama5d2-gpio"; > > reg = <0xfc038000 0x600>; > > clocks = <&pioA_clk>; > > gpio-controller; > > #gpio-cells = <2>; > > > > pinctrl { > > compatible = "atmel,sama5d2-pinctrl"; > > }; > > }; > > }; > > }; > > > > ---->8----- > > > > In that case I think your clock driver should handle this in its > bind() method. It can scan the device tree and create the devices as needed. I > don't think this feature should go into generic code, at least not yet. Okay, I will move it to our clock driver. > > Also please see Stephen Warren's clock patch, which supports multiple clock IDs > per clock device. Assuming he respins this soon I will be applying it to dm/master. > > http://patchwork.ozlabs.org/patch/625342/ Thank you for your reminder. > > >> > >> > > >> > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c > >> > index > >> > b483c1e..ee568e4 100644 > >> > --- a/drivers/clk/clk-uclass.c > >> > +++ b/drivers/clk/clk-uclass.c > >> > @@ -106,7 +106,58 @@ int clk_get_by_index(struct udevice *dev, int > >> > index, struct udevice **clk_devp) } #endif > >> > > >> > +/** > >> > + * clk_post-bind() - post binding for CLK uclass > >> > + * Recursively bind its children as clk devices. > >> > + * > >> > + * @dev: clk device > >> > + * @return: 0 on success, or negative error code on failure */ > >> > +static int clk_post_bind(struct udevice *dev) { > >> > + const void *fdt = gd->fdt_blob; > >> > + int offset = dev->of_offset; > >> > + bool pre_reloc_only = !(gd->flags & GD_FLG_RELOC); > >> > + const char *name; > >> > + int ret; > >> > + > >> > + for (offset = fdt_first_subnode(fdt, offset); > >> > + offset > 0; > >> > + offset = fdt_next_subnode(fdt, offset)) { > >> > + if (pre_reloc_only && > >> > + !fdt_getprop(fdt, offset, "u-boot,dm-pre-reloc", NULL)) > >> > + continue; > >> > + /* > >> > + * If this node has "compatible" property, this is not a clk > >> > + * node, but a normal device. skip. > >> > + */ > >> > + fdt_get_property(fdt, offset, "compatible", &ret); > >> > + if (ret >= 0) > >> > + continue; > >> > + > >> > + if (ret != -FDT_ERR_NOTFOUND) > >> > + return ret; > >> > + > >> > + name = fdt_get_name(fdt, offset, NULL); > >> > + if (!name) > >> > + return -EINVAL; > >> > + > >> > + ret = device_bind_driver_to_node(dev, "clk", name, > >> > + offset, NULL); > >> > + if (ret) > >> > + return ret; > >> > + } > >> > + > >> > + return 0; > >> > +} > >> > + > >> > UCLASS_DRIVER(clk) = { > >> > .id = UCLASS_CLK, > >> > + .post_bind = clk_post_bind, > >> > .name = "clk", > >> > }; > >> > + > >> > +U_BOOT_DRIVER(clk_generic) = { > >> > + .id = UCLASS_CLK, > >> > + .name = "clk", > >> > +}; > >> > -- > >> > 2.7.4 > >> > > > Regards, > Simon Best Regards, Wenyou Yang
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index b483c1e..ee568e4 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -106,7 +106,58 @@ int clk_get_by_index(struct udevice *dev, int index, struct udevice **clk_devp) } #endif +/** + * clk_post-bind() - post binding for CLK uclass + * Recursively bind its children as clk devices. + * + * @dev: clk device + * @return: 0 on success, or negative error code on failure + */ +static int clk_post_bind(struct udevice *dev) +{ + const void *fdt = gd->fdt_blob; + int offset = dev->of_offset; + bool pre_reloc_only = !(gd->flags & GD_FLG_RELOC); + const char *name; + int ret; + + for (offset = fdt_first_subnode(fdt, offset); + offset > 0; + offset = fdt_next_subnode(fdt, offset)) { + if (pre_reloc_only && + !fdt_getprop(fdt, offset, "u-boot,dm-pre-reloc", NULL)) + continue; + /* + * If this node has "compatible" property, this is not a clk + * node, but a normal device. skip. + */ + fdt_get_property(fdt, offset, "compatible", &ret); + if (ret >= 0) + continue; + + if (ret != -FDT_ERR_NOTFOUND) + return ret; + + name = fdt_get_name(fdt, offset, NULL); + if (!name) + return -EINVAL; + + ret = device_bind_driver_to_node(dev, "clk", name, + offset, NULL); + if (ret) + return ret; + } + + return 0; +} + UCLASS_DRIVER(clk) = { .id = UCLASS_CLK, + .post_bind = clk_post_bind, .name = "clk", }; + +U_BOOT_DRIVER(clk_generic) = { + .id = UCLASS_CLK, + .name = "clk", +};
Add post binding support for CLK uclass to recursively bind its children as clk devices. Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> --- Changes in v2: None drivers/clk/clk-uclass.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)