| Message ID | 20240701151231.29425-5-kyarlagadda@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce Tegra register config settings | expand |
On Mon, 01 Jul 2024 20:42:22 +0530, Krishna Yarlagadda wrote: > I2C interface timing registers are configured using config setting > framework. List available field properties for Tegra I2C controllers. > > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com> > --- > .../misc/nvidia,tegra-config-settings.yaml | 83 +++++++++++++++++-- > 1 file changed, 74 insertions(+), 9 deletions(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dtb: /example-0/remoteproc@cd00000: failed to match any schema with compatible: ['qcom,ipq8074-wcss-pil'] Documentation/devicetree/bindings/clock/sprd,sc9863a-clk.example.dtb: /example-1/syscon@20e00000: failed to match any schema with compatible: ['sprd,sc9863a-glbregs', 'syscon', 'simple-mfd'] Documentation/devicetree/bindings/clock/milbeaut-clock.example.dtb: /example-2/serial@1e700010: failed to match any schema with compatible: ['socionext,milbeaut-usio-uart'] Documentation/devicetree/bindings/arm/hisilicon/controller/hi3798cv200-perictrl.example.dtb: /example-0/peripheral-controller@8a20000/phy@850: failed to match any schema with compatible: ['hisilicon,hi3798cv200-combphy'] /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/stm32/st,mlahb.example.dtb: ahb@38000000: Unevaluated properties are not allowed ('reg' was unexpected) from schema $id: http://devicetree.org/schemas/arm/stm32/st,mlahb.yaml# Documentation/devicetree/bindings/sound/st,stm32-sai.example.dtb: /example-0/sai@4400b000/audio-controller@4400b004: failed to match any schema with compatible: ['st,stm32-sai-sub-a'] Documentation/devicetree/bindings/thermal/brcm,avs-ro-thermal.example.dtb: /example-0/avs-monitor@7d5d2000: failed to match any schema with compatible: ['brcm,bcm2711-avs-monitor', 'syscon', 'simple-mfd'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240701151231.29425-5-kyarlagadda@nvidia.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Mon, Jul 01, 2024 at 08:42:22PM +0530, Krishna Yarlagadda wrote: > I2C interface timing registers are configured using config setting > framework. List available field properties for Tegra I2C controllers. How is I2C bus timing parameters specific to NVIDIA? Just because you have more controls? No. That's no reason to invent a whole new way to specify parameters. Extend what's already there and make it work for anyone. Rob
On Mon, Jul 01, 2024 at 11:42:27AM GMT, Rob Herring wrote: > On Mon, Jul 01, 2024 at 08:42:22PM +0530, Krishna Yarlagadda wrote: > > I2C interface timing registers are configured using config setting > > framework. List available field properties for Tegra I2C controllers. > > How is I2C bus timing parameters specific to NVIDIA? Just because you > have more controls? No. That's no reason to invent a whole new way to > specify parameters. Extend what's already there and make it work for > anyone. This may be applicable to a subset of this, and yes, maybe we can find generalizations for some of these parameters. However, we're also looking for feedback specifically on these config nodes that go beyond individual timing parameters. For example in the case of I2C, how should parameters for different operating modes be described? Would you agree with something along the lines provided in this series? Do you have any better suggestions? Thanks, Thierry
On Tue, Jul 2, 2024 at 4:29 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > On Mon, Jul 01, 2024 at 11:42:27AM GMT, Rob Herring wrote: > > On Mon, Jul 01, 2024 at 08:42:22PM +0530, Krishna Yarlagadda wrote: > > > I2C interface timing registers are configured using config setting > > > framework. List available field properties for Tegra I2C controllers. > > > > How is I2C bus timing parameters specific to NVIDIA? Just because you > > have more controls? No. That's no reason to invent a whole new way to > > specify parameters. Extend what's already there and make it work for > > anyone. > > This may be applicable to a subset of this, and yes, maybe we can find > generalizations for some of these parameters. > > However, we're also looking for feedback specifically on these config > nodes that go beyond individual timing parameters. For example in the > case of I2C, how should parameters for different operating modes be > described? Like what? It all looks like timing to me. > Would you agree with something along the lines provided in this series? When there are multiple users/vendors of it, maybe. In general, it goes against the DT design of properties for foo go in foo's node. This looks more like how ACPI does things where it's add another table for this new thing we need. Rob
On Wed, Jul 03, 2024 at 02:21:04PM GMT, Rob Herring wrote: > On Tue, Jul 2, 2024 at 4:29 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > On Mon, Jul 01, 2024 at 11:42:27AM GMT, Rob Herring wrote: > > > On Mon, Jul 01, 2024 at 08:42:22PM +0530, Krishna Yarlagadda wrote: > > > > I2C interface timing registers are configured using config setting > > > > framework. List available field properties for Tegra I2C controllers. > > > > > > How is I2C bus timing parameters specific to NVIDIA? Just because you > > > have more controls? No. That's no reason to invent a whole new way to > > > specify parameters. Extend what's already there and make it work for > > > anyone. > > > > This may be applicable to a subset of this, and yes, maybe we can find > > generalizations for some of these parameters. > > > > However, we're also looking for feedback specifically on these config > > nodes that go beyond individual timing parameters. For example in the > > case of I2C, how should parameters for different operating modes be > > described? > > Like what? It all looks like timing to me. The problem here isn't the individual properties but rather how to group them. More generally the problem is that we have a set of settings that need to be applied in different variants. Yes, they are all timings, but the values differ based on what mode a given controller operates at. Take for example I2C where we have things like start-hold time or stop- setup time, which we could describe in a more generic way (i.e. leave out the vendor prefix). However, depending on the mode that the I2C controller runs at (could be standard mode, fast mode or fastplus mode) these values need to be adjusted. So it's the same set of properties but with different values for each different operating mode. As far as I can tell there's no good construct to describe this in DT currently. > > Would you agree with something along the lines provided in this series? > > When there are multiple users/vendors of it, maybe. > > In general, it goes against the DT design of properties for foo go in > foo's node. This looks more like how ACPI does things where it's add > another table for this new thing we need. Well, that's what Krishna had proposed in the first version of the series, which you guys rejected. The problem with that is that we cannot easily group these settings using nodes because subnodes of I2C controllers are considered to be clients by default. This applies to SPI and other busses as well. This approach avoids these issues and can be more easily optimized since settings could be shared between multiple instances of the controllers. I have a slight preference of putting this into the controllers' device tree nodes, but I can't think of a good way of avoiding the above child node problem other than what we had in v1. Thierry
diff --git a/Documentation/devicetree/bindings/misc/nvidia,tegra-config-settings.yaml b/Documentation/devicetree/bindings/misc/nvidia,tegra-config-settings.yaml index 4e5d52504c01..5f4da633e69b 100644 --- a/Documentation/devicetree/bindings/misc/nvidia,tegra-config-settings.yaml +++ b/Documentation/devicetree/bindings/misc/nvidia,tegra-config-settings.yaml @@ -38,17 +38,74 @@ patternProperties: additionalProperties: false patternProperties: - "^[a-z0-9_]+-cfg$": - description: - Config profiles applied conditionally. + "^i2c-[a-z0-9_]+-cfg$": + description: Config settings for I2C devices. type: object - patternProperties: - "nvidia,[a-z0-9_]+$": - description: - Register field configuration. - $ref: /schemas/types.yaml#/definitions/uint32 + additionalProperties: false -additionalProperties: true + properties: + nvidia,i2c-clk-divisor-hs-mode: + description: I2C clock divisor for HS mode. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-clk-divisor-fs-mode: + description: I2C clock divisor for FS mode. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-hs-sclk-high-period: + description: I2C high speed sclk high period. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-hs-sclk-low-period: + description: I2C high speed sclk low period. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-hs-stop-setup-time: + description: I2C high speed stop setup time. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-hs-start-hold-time: + description: I2C high speed start hold time. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-hs-start-setup-time: + description: I2C high speed start setup time. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-sclk-high-period: + description: I2C sclk high period. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-sclk-low-period: + description: I2C sclk low period. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-bus-free-time: + description: I2C bus free time. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-stop-setup-time: + description: I2C stop setup time. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + nvidia,i2c-start-hold-time: + description: I2C start hold time. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 0xffff + +additionalProperties: false examples: - | @@ -58,5 +115,13 @@ examples: nvidia,i2c-hs-sclk-high-period = <0x03>; nvidia,i2c-hs-sclk-low-period = <0x08>; }; + i2c-fast-cfg { + nvidia,i2c-clk-divisor-fs-mode = <0x3c>; + nvidia,i2c-sclk-high-period = <0x02>; + }; + i2c-fastplus-cfg { + nvidia,i2c-clk-divisor-fs-mode = <0x4f>; + nvidia,i2c-sclk-high-period = <0x07>; + }; }; };
I2C interface timing registers are configured using config setting framework. List available field properties for Tegra I2C controllers. Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com> --- .../misc/nvidia,tegra-config-settings.yaml | 83 +++++++++++++++++-- 1 file changed, 74 insertions(+), 9 deletions(-)