linux-kernel - Re: [PATCH V4 04/10] dt-bindings: timestamp: Add nvidia,gpio-controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 4 Apr 2023 12:30:38 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Dipen Patel <dipenp@...dia.com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        jonathanh@...dia.com, linux-kernel@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-gpio@...r.kernel.org,
        linus.walleij@...aro.org, devicetree@...r.kernel.org,
        linux-doc@...r.kernel.org, robh+dt@...nel.org,
        timestamp@...ts.linux.dev, krzysztof.kozlowski+dt@...aro.org,
        brgl@...ev.pl, corbet@....net, gregkh@...uxfoundation.org
Subject: Re: [PATCH V4 04/10] dt-bindings: timestamp: Add
 nvidia,gpio-controller

On Mon, Mar 27, 2023 at 09:58:19AM -0700, Dipen Patel wrote:
> On 3/25/23 4:07 AM, Krzysztof Kozlowski wrote:
> > On 23/03/2023 02:29, Dipen Patel wrote:
> >> Introducing nvidia,gpio-controller property from Tegra234 SoCs onwards.
> >> This is done to help below case.
> >>
> >> Without this property code would look like:
> >> if (of_device_is_compatible(dev->of_node, "nvidia,tegra194-gte-aon"))
> >> 	hte_dev->c = gpiochip_find("tegra194-gpio-aon",
> >> 				   tegra_get_gpiochip_from_name);
> >> else if (of_device_is_compatible(dev->of_node, "nvidia,tegra234-gte-aon"))
> >> 	hte_dev->c = gpiochip_find("tegra234-gpio-aon",
> >> 				   tegra_get_gpiochip_from_name);
> >> else
> >> 	return -ENODEV;
> >>
> >> This means for every future addition of the compatible string, if else
> >> condition statements have to be expanded.
> >>
> >> With the property:
> >> gpio_ctrl = of_parse_phandle(dev->of_node, "nvidia,gpio-controller", 0);
> >> ....
> >> hte_dev->c = gpiochip_find(gpio_ctrl, tegra_get_gpiochip_from_of_node);
> >>
> >> This simplifies the code significantly. The introdunction of this
> >> property/binding does not break existing Tegra194 provider driver.
> >>
> >> Signed-off-by: Dipen Patel <dipenp@...dia.com>
> >> ---
> >>  .../timestamp/nvidia,tegra194-hte.yaml        | 31 +++++++++++++++++--
> >>  1 file changed, 29 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml b/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml
> >> index eafc33e9ae2e..841273a3d8ae 100644
> >> --- a/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml
> >> +++ b/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml
> >> @@ -51,6 +51,12 @@ properties:
> >>        LIC instance has 11 slices and Tegra234 LIC has 17 slices.
> >>      enum: [3, 11, 17]
> >>  
> >> +  nvidia,gpio-controller:
> >> +    $ref: /schemas/types.yaml#/definitions/phandle
> >> +    description:
> >> +      The phandle to AON gpio controller instance. This is required to handle
> >> +      namespace conversion between GPIO and GTE.
> >> +
> >>    '#timestamp-cells':
> >>      description:
> >>        This represents number of line id arguments as specified by the
> >> @@ -65,22 +71,43 @@ required:
> >>    - interrupts
> >>    - "#timestamp-cells"
> >>  
> >> +allOf:
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            enum:
> >> +              - nvidia,tegra234-gte-aon
> >> +    then:
> >> +      required:
> >> +        - nvidia,gpio-controller
> >> +
> >>  additionalProperties: false
> >>  
> >>  examples:
> >>    - |
> >>      tegra_hte_aon: timestamp@...0000 {
> >>                compatible = "nvidia,tegra194-gte-aon";
> >> -              reg = <0xc1e0000 0x10000>;
> >> +              reg = <0x0 0xc1e0000 0x0 0x10000>;
> > 
> > This is not really explained in commit msg... are you sure you tested it?
> I have to revert this part back in next patch as when I upgraded dtsschema it gave me errors.

We need the 0x0 in the DTS files because we have #address-cells = <2>
and #size-tells = <2>. For the examples, those default to just 1 cell,
so this can't be an exact copy of what we have in the DTS files.

Please make sure to always validate the bindings and examples.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)