Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 5 May 2023 16:29:48 -0500
From:   Rob Herring <robh@kernel.org>
To:     Judith Mendez <jm@ti.com>
Cc:     Chandrasekar Ramakrishnan <rcsekar@samsung.com>,
        Wolfgang Grandegger <wg@grandegger.com>,
        Marc Kleine-Budde <mkl@pengutronix.de>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, linux-can@vger.kernel.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Schuyler Patton <spatton@ti.com>, Nishanth Menon <nm@ti.com>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Tero Kristo <kristo@kernel.org>,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        Oliver Hartkopp <socketcan@hartkopp.net>,
        Simon Horman <simon.horman@corigine.com>
Subject: Re: [PATCH v4 1/4] dt-bindings: net: can: Add poll-interval for MCAN
Message-ID: <20230505212948.GA3590042-robh@kernel.org>
References: <20230501224624.13866-1-jm@ti.com>
 <20230501224624.13866-2-jm@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230501224624.13866-2-jm@ti.com>
Precedence: bulk

On Mon, May 01, 2023 at 05:46:21PM -0500, Judith Mendez wrote:
> On AM62x SoC, MCANs on MCU domain do not have hardware interrupt
> routed to A53 Linux, instead they will use software interrupt by
> hrtimer. To enable timer method, interrupts should be optional so
> remove interrupts property from required section and introduce
> poll-interval property.
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
> Changelog:
> v3:
>  1. Move binding patch to first in series
>  2. Update description for poll-interval
>  3. Add oneOf to specify using interrupts/interrupt-names or poll-interval
>  4. Fix example property: add comment below 'example'
> 
> v2:
>   1. Add poll-interval property to enable timer polling method
>   2. Add example using poll-interval property
>   
>  .../bindings/net/can/bosch,m_can.yaml         | 36 +++++++++++++++++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
> index 67879aab623b..c024ee49962c 100644
> --- a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
> +++ b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
> @@ -14,6 +14,13 @@ maintainers:
>  allOf:
>    - $ref: can-controller.yaml#
>  
> +oneOf:
> +  - required:
> +      - interrupts
> +      - interrupt-names
> +  - required:
> +      - poll-interval

Move this next to 'required'.

> +
>  properties:
>    compatible:
>      const: bosch,m_can
> @@ -40,6 +47,14 @@ properties:
>        - const: int1
>      minItems: 1
>  
> +  poll-interval:
> +    $ref: /schemas/types.yaml#/definitions/flag

This is a common property already defined as a uint32. You shouldn't 
define a new type.

A flag doesn't even make sense. If that's all you need, then just enable 
polling if no interrupt is present.

> +    description: Enable hrtimer polling method for an M_CAN device.
> +      If this property is defined in MCAN node, it tells the driver to
> +      enable polling method for an MCAN device. If for an MCAN device,
> +      hardware interrupt is found and hrtimer polling method is enabled,

What's hrtimer? (Don't put Linuxisms in bindings)

> +      the driver will use hardware interrupt method.
> +
>    clocks:
>      items:
>        - description: peripheral clock
> @@ -122,8 +137,6 @@ required:
>    - compatible
>    - reg
>    - reg-names
> -  - interrupts
> -  - interrupt-names
>    - clocks
>    - clock-names
>    - bosch,mram-cfg
> @@ -132,6 +145,7 @@ additionalProperties: false
>  
>  examples:
>    - |
> +    // Example with interrupts
>      #include <dt-bindings/clock/imx6sx-clock.h>
>      can@20e8000 {
>        compatible = "bosch,m_can";
> @@ -149,4 +163,22 @@ examples:
>        };
>      };
>  
> +  - |
> +    // Example with timer polling
> +    #include <dt-bindings/clock/imx6sx-clock.h>
> +    can@20e8000 {
> +      compatible = "bosch,m_can";
> +      reg = <0x020e8000 0x4000>, <0x02298000 0x4000>;
> +      reg-names = "m_can", "message_ram";
> +      poll-interval;
> +      clocks = <&clks IMX6SX_CLK_CANFD>,
> +               <&clks IMX6SX_CLK_CANFD>;
> +      clock-names = "hclk", "cclk";
> +      bosch,mram-cfg = <0x0 0 0 32 0 0 0 1>;
> +
> +      can-transceiver {
> +        max-bitrate = <5000000>;
> +      };
> +    };
> +
>  ...
> -- 
> 2.17.1
>