[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210305212042.GA658055@robh.at.kernel.org>
Date: Fri, 5 Mar 2021 15:20:42 -0600
From: Rob Herring <robh@...nel.org>
To: mgross@...ux.intel.com
Cc: markgross@...nel.org, arnd@...db.de, bp@...e.de,
damien.lemoal@....com, dragan.cvetic@...inx.com,
gregkh@...uxfoundation.org, corbet@....net,
palmerdabbelt@...gle.com, paul.walmsley@...ive.com,
peng.fan@....com, shawnguo@...nel.org, jassisinghbrar@...il.com,
linux-kernel@...r.kernel.org,
"C, Udhayakumar" <udhayakumar.c@...el.com>,
devicetree@...r.kernel.org
Subject: Re: [PATCH v6 32/34] dt-bindings: misc: hddl_dev: Add hddl device
management documentation
On Fri, Feb 12, 2021 at 02:23:02PM -0800, mgross@...ux.intel.com wrote:
> From: "C, Udhayakumar" <udhayakumar.c@...el.com>
>
> Add hddl device management documentation
>
> The HDDL client driver acts as an software RTC to sync with network time.
> It abstracts xlink protocol to communicate with remote IA host.
> This driver exports the details about sensors available in the platform
> to remote IA host as xlink packets.
> This driver also handles device connect/disconnect events and identifies
> board id and soc id using gpio's based on platform configuration.
Not sure any of this belongs in DT, but trivial comments below.
>
> Cc: Rob Herring <robh+dt@...nel.org>
> Cc: devicetree@...r.kernel.org
> Signed-off-by: C Udhayakumar <udhayakumar.c@...el.com>
> Signed-off-by: Mark Gross <mgross@...ux.intel.com>
> ---
> .../bindings/misc/intel,hddl-client.yaml | 117 ++++++++++++++++++
> 1 file changed, 117 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/misc/intel,hddl-client.yaml
>
> diff --git a/Documentation/devicetree/bindings/misc/intel,hddl-client.yaml b/Documentation/devicetree/bindings/misc/intel,hddl-client.yaml
> new file mode 100644
> index 000000000000..522b461663b5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/intel,hddl-client.yaml
> @@ -0,0 +1,117 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/misc/intel,hddl-client.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Intel hddl client device to handle platform management in Bay series
> +
> +maintainers:
> + - Udhayakumar C <udhayakumar.c@...el.com>
> +
> +description: |
> + The HDDL client driver acts as an software RTC to sync with network time.
> + It abstracts xlink protocol to communicate with remote host. This driver
> + exports the details about sensors available in the platform to remote
> + host as xlink packets.
> + This driver also handles device connect/disconnect events and identifies
> + board id and soc id using gpio's based on platform configuration.
> +
> +select: false
That's nice. If you never want to use this schema, why write it? One way
to avoid the errors it has I guess.
> +
> +properties:
> + compatible:
> + items:
> + - const: intel,hddl-client
> +
> + reg:
> + minItems: 4
> + maxItems: 4
Looks like only 1 in the example. If 4 entries, you need to say what
each one is.
> +
> + xlink_chan:
Vendor specific properties need vendor prefixes.
s/_/-/
> + minItems: 1
> + maxItems: 1
> + description: xlink channel number used for communication
> + with remote host for time sync and sharing sensor
> + details available in platform.
> +
> + i2c_xlink_chan:
> + minItems: 1
> + maxItems: 1
> + description: xlink channel number used for communication
> + with remote host for xlink i2c smbus.
> +
> + sensor_name:
> + type: object
> + description:
> + Details about sensors and its configuration on local host and remote
> + host.
> +
> + properties:
> + compatible:
> + items:
> + - const: intel_tsens
> +
> + reg:
> + description: i2c slave address for sensor.
> +
> + local-host:
> + minItems: 1
> + maxItems: 1
> + description: enable bit 0 to register sensor as i2c slave
> + in local host (normal i2c client)
> + enable bit 1 to mimic sensor as i2c slave
> + in local host (onchip sensors as i2c slave)
> + enable bit 2 to register i2c slave as xlink smbus slave
> + in local host.
> + remote-host:
> + minItems: 1
> + maxItems: 1
> + description: enable bit 0 to register sensor as i2c slave
> + in remote host (normal i2c client)
> + enable bit 1 to mimic sensor as i2c slave
> + in remote host (onchip sensors as i2c slave)
> + enable bit 2 to register i2c slave as xlink smbus slave
> + in remote host.
> +
> + bus:
> + minItems: 1
> + maxItems: 1
> + description: i2c bus number for the i2c client device.
> +
> + required:
> + - compatible
> + - reg
> + - local-host
> + - remote-host
> + - bus
> +
> +required:
> + - compatible
> + - reg
> + - xlink_chan
> + - i2c_xlink_chan
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + hddl_dev{
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + hddl@...20000 {
> + compatible = "intel,hddl-client";
> + status = "disabled";
Don't show status in examples.
> + reg = <0x0 0x20320000 0x0 0x800>;
> + xlink_chan = <1080>;
> + i2c_xlink_chan = <1081>;
> + kmb_xlink_tj {
> + status = "okay";
> + compatible = "intel_tsens";
> + local-host = <0x3>;
> + remote-host = <0x3>;
> + bus = <0x1>;
> + };
> + };
> + };
> --
> 2.17.1
>
Powered by blists - more mailing lists