Hi Lokesh,
> Il 03/09/2020 08:31 Lokesh Vutla <[email protected]> ha scritto:
>
>
> On 25/08/20 2:51 pm, Dario Binacchi wrote:
> > The implementation of this driver was needed to bind the device tree
> > sub-nodes of the 'clocks' node. In fact, the lack of the compatible
> > property in the 'clocks' node does not allow the generic 'syscon' or
> > 'simple-bus' drivers linked to the 'scm_conf@0' node to bind the
> > 'clocks' node and in turn its sub-nodes.
> > The 'scm@210000' node is therefore the node closest to the 'clocks' node
> > whose driver can bind all the 'clocks' sub-nodes.
> >
> > scm: scm@210000 {
> > compatible = "ti,am3-scm", "simple-bus";
> > ...
> >
> > scm_conf: scm_conf@0 {
> > compatible = "syscon", "simple-bus";
> > #address-cells = <1>;
> > #size-cells = <1>;
> > ranges = <0 0 0x800>;
> >
> > scm_clocks: clocks {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > };
> > };
> > };
> >
> > Signed-off-by: Dario Binacchi <[email protected]>
> > ---
> >
> > doc/device-tree-bindings/arm/omap,ctrl.txt | 82 ++++++
> > .../pinctrl/pinctrl-single.txt | 255 ++++++++++++++++++
>
> This documentation does not belong to this patch.
The omap,ctrl.txt file references to the pinctrl-single.txt document.
I thought it was better to add it rather than remove that
reference from the omap,ctrl.txt file.
>
> > drivers/misc/Kconfig | 7 +
> > drivers/misc/Makefile | 1 +
> > drivers/misc/ti-am3-scm.c | 90 +++++++
> > 5 files changed, 435 insertions(+)
> > create mode 100644 doc/device-tree-bindings/arm/omap,ctrl.txt
> > create mode 100644 doc/device-tree-bindings/pinctrl/pinctrl-single.txt
> > create mode 100644 drivers/misc/ti-am3-scm.c
> >
> > diff --git a/doc/device-tree-bindings/arm/omap,ctrl.txt
> > b/doc/device-tree-bindings/arm/omap,ctrl.txt
> > new file mode 100644
> > index 0000000000..8efd321cfa
> > --- /dev/null
> > +++ b/doc/device-tree-bindings/arm/omap,ctrl.txt
> > @@ -0,0 +1,82 @@
> > +OMAP Control Module bindings
> > +
> > +Control Module contains miscellaneous features under it based on SoC type.
> > +Pincontrol is one common feature, and it has a specialized support
> > +described in [1]. Typically some clock nodes are also under control module.
> > +Syscon is used to share register level access to drivers external to
> > +control module driver itself.
> > +
> > +See [2] for documentation about clock/clockdomain nodes.
> > +
> > +[1] doc/device-tree-bindings/pinctrl/pinctrl-single.txt
> > +[2] doc/device-tree-bindings/clock/ti,*.txt
> > +
> > +Required properties:
> > +- compatible: Must be one of:
> > + "ti,am3-scm"
> > + "ti,am4-scm"
> > + "ti,dm814-scrm"
> > + "ti,dm816-scrm"
> > + "ti,omap2-scm"
> > + "ti,omap3-scm"
> > + "ti,omap4-scm-core"
> > + "ti,omap4-scm-padconf-core"
> > + "ti,omap4-scm-wkup"
> > + "ti,omap4-scm-padconf-wkup"
> > + "ti,omap5-scm-core"
> > + "ti,omap5-scm-padconf-core"
> > + "ti,omap5-scm-wkup-pad-conf"
> > + "ti,dra7-scm-core"
> > +- reg: Contains Control Module register address range
> > + (base address and length)
> > +
> > +Optional properties:
> > +- clocks: clocks for this module
> > +- clockdomains: clockdomains for this module
> > +
> > +Examples:
> > +
> > +scm: scm@2000 {
> > + compatible = "ti,omap3-scm", "simple-bus";
> > + reg = <0x2000 0x2000>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges = <0 0x2000 0x2000>;
> > +
> > + omap3_pmx_core: pinmux@30 {
> > + compatible = "ti,omap3-padconf",
> > + "pinctrl-single";
> > + reg = <0x30 0x230>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + #interrupt-cells = <1>;
> > + interrupt-controller;
> > + pinctrl-single,register-width = <16>;
> > + pinctrl-single,function-mask = <0xff1f>;
> > + };
> > +
> > + scm_conf: scm_conf@270 {
> > + compatible = "syscon";
> > + reg = <0x270 0x330>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + scm_clocks: clocks {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + };
> > +
> > + scm_clockdomains: clockdomains {
> > + };
> > +}
> > +
> > +&scm_clocks {
> > + mcbsp5_mux_fck: mcbsp5_mux_fck {
> > + #clock-cells = <0>;
> > + compatible = "ti,composite-mux-clock";
> > + clocks = <&core_96m_fck>, <&mcbsp_clks>;
> > + ti,bit-shift = <4>;
> > + reg = <0x02d8>;
> > + };
> > +};
>
> [...snip...]
>
>
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > index b67e906a76..9e8b676637 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -500,4 +500,11 @@ config ESM_PMIC
> > Support ESM (Error Signal Monitor) on PMIC devices. ESM is used
> > typically to reboot the board in error condition.
> >
> > +config TI_AM3_SCM
> > + bool "AM33XX specific control module support (SCM)"
> > + depends on ARCH_OMAP2PLUS
> > + help
> > + The control module includes status and control logic not addressed
> > + within the peripherals or the rest of the device infrastructure.
> > +
> > endmenu
> > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > index 947bd3a647..056fb3b522 100644
> > --- a/drivers/misc/Makefile
> > +++ b/drivers/misc/Makefile
> > @@ -75,3 +75,4 @@ obj-$(CONFIG_MICROCHIP_FLEXCOM) += microchip_flexcom.o
> > obj-$(CONFIG_K3_AVS0) += k3_avs.o
> > obj-$(CONFIG_ESM_K3) += k3_esm.o
> > obj-$(CONFIG_ESM_PMIC) += esm_pmic.o
> > +obj-$(CONFIG_TI_AM3_SCM) += ti-am3-scm.o
> > diff --git a/drivers/misc/ti-am3-scm.c b/drivers/misc/ti-am3-scm.c
> > new file mode 100644
> > index 0000000000..e5f4f09261
> > --- /dev/null
> > +++ b/drivers/misc/ti-am3-scm.c
> > @@ -0,0 +1,90 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * AM335x specific control module (scm)
> > + *
> > + * Copyright (C) 2020 Dario Binacchi <[email protected]>
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <dm/lists.h>
> > +#include <linux/err.h>
> > +
> > +static int ti_am3_scm_bind(struct udevice *dev)
> > +{
> > + int err;
> > + struct udevice *conf_dev;
> > + ofnode clocks_node, conf_node;
> > +
> > + err = dm_scan_fdt_dev(dev);
> > + if (err) {
> > + dev_err(dev, "%s: dm_scan_fdt, err=%d\n", __func__, err);
> > + return err;
> > + }
> > +
> > + conf_node = dev_read_subnode(dev, "scm_conf@0");
> > + if (!ofnode_valid(conf_node)) {
> > + dev_err(dev, "%s: failed to get conf sub-node\n", __func__);
> > + return -ENODEV;
> > + }
> > +
> > + if (uclass_get_device_by_ofnode(UCLASS_SYSCON, conf_node, &conf_dev)) {
> > + if (uclass_get_device_by_ofnode(UCLASS_SIMPLE_BUS, conf_node,
> > + &conf_dev)) {
> > + dev_err(dev, "%s: failed to get conf device\n",
> > + __func__);
> > + return -ENODEV;
> > + }
> > + }
> > +
> > + clocks_node = dev_read_subnode(conf_dev, "clocks");
> > + if (!ofnode_valid(clocks_node)) {
> > + dev_err(dev, "%s: failed to get clocks sub-node\n", __func__);
> > + return -ENODEV;
> > + }
>
> Isn't clock node an optional property?
>
> Also do we really need a separate UBOOT_DRIVER for scm_clocks? Can it be
> handled
> in the same driver?
>
In the next version of the series, scm_clocks will be handled
by the same driver.
Regards,
Dario
> Thanks and regards,
> Lokesh
>
> > +
> > + err = device_bind_driver_to_node(conf_dev, "ti_am3_scm_clocks",
> > + "scm_clocks", clocks_node, NULL);
> > + if (err) {
> > + dev_err(dev, "%s: failed to bind scm_clocks\n", __func__);
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct udevice_id ti_am3_scm_ids[] = {
> > + {.compatible = "ti,am3-scm"},
> > + {}
> > +};
> > +
> > +U_BOOT_DRIVER(ti_am3_scm) = {
> > + .name = "ti_am3_scm",
> > + .id = UCLASS_SIMPLE_BUS,
> > + .of_match = ti_am3_scm_ids,
> > + .bind = ti_am3_scm_bind,
> > +};
> > +
> > +static int ti_am3_scm_clocks_bind(struct udevice *dev)
> > +{
> > + ofnode node;
> > + int err;
> > +
> > + dev_dbg(dev, "%s: dev=%p\n", __func__, dev);
> > + ofnode_for_each_subnode(node, dev_ofnode(dev)) {
> > + err = lists_bind_fdt(dev, node, NULL, false);
> > + if (err) {
> > + dev_err(dev, "%s: lists_bind_fdt, err=%d\n",
> > + __func__, err);
> > + return err;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +U_BOOT_DRIVER(ti_am3_scm_clocks) = {
> > + .name = "ti_am3_scm_clocks",
> > + .id = UCLASS_SIMPLE_BUS,
> > + .bind = ti_am3_scm_clocks_bind,
> > +};
> >
