Re: [PATCH v8 07/12] dt-bindings: ufs: Add NXP S32N79 UFS host controller

[Larisa Ileana Grigore]

On 10/14/2026 2:10 PM, Krzysztof Kozlowski wrote:
> On Tue, Oct 13, 2026 at 12:29:58PM +0200, Larisa Grigore wrote:
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: core
>
> I don't get why you added the clock-names.
Hello Krzysztof,

The properties, `clock-names` and `operating-points-v2`, are needed to 
let the generic UFS platform code discover and manage the controller clock.

`ufshcd_parse_operating_points()` uses `clock-names` to create the 
corresponding `struct ufs_clk_info` entry and populate 
`hba->clk_list_head`. The UFS core then uses this list to manage the 
clock, including enabling and disabling it during suspend and resume. 
Without this information, the clock would need to be handled separately 
by the S32N79 variant driver.

The OPP table contains one operating point with `opp-hz = <0>`. This is
intentional and follows the semantics documented in `ufs-common.yaml`:

  If clock frequency in the array is "0" then it is assumed that the
  frequency is set by the parent clock or a fixed rate clock source.

On S32N79, the clock rate is configured by an entity outside Linux. 
Linux does not program or scale this rate.

A similar pattern is used by the `amd,versal2-ufs` binding. It describes 
a single clock named `core` together with `freq-table-hz = <0 0>`.

Best regards,
Larisa

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - clocks
> > +  - clock-names
> > +  - operating-points-v2
>
> Your opp-table is empty, so requiring such property looks like wrongly
> written driver.
>
> > +
> > +allOf:
> > +  - $ref: ufs-common.yaml
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    ufshc@4ac80000 {
> > +        compatible = "nxp,s32n79-ufshc";
> > +        reg = <0x4ac80000 0x1000>, <0x4ac40000 0x1000>;
> > +        reg-names = "ufshc", "scm";
> > +        interrupt-parent = <&irqsteer_coss>;
> > +        interrupts = <211>;
> > +        clocks = <&clks 0x92>;
> > +        clock-names = "core";
> > +        operating-points-v2 = <&ufs_opp_table>;
> > +
> > +        ufs_opp_table: opp-table {
> > +            compatible = "operating-points-v2";
> > +
> > +            opp-0 {
> > +                opp-hz = /bits/ 64 <0>;
>
> Best regards,
> Krzysztof