Re: [PATCH v2 6/6] of: of_cpu_phandle_to_id to support SMT threads

[Alireza Sanaee <alireza.sanaee@huawei.com>]

On Tue, 6 May 2025 15:36:05 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 06/05/2025 15:31, Alireza Sanaee wrote:
> >>>>    
> >>>
> >>> Hi Krzysztof,
> >>>
> >>> There are some existing bindings in which this pattern has been
> >>> used, so I don't think I am changing binding really.
> >>>
> >>> https://www.kernel.org/doc/Documentation/devicetree/bindings/thermal/thermal-zones.yaml#:~:text=cooling%2Ddevice%20%3D%20%3C%26CPU0%203%203%3E%2C%20%3C%26CPU1%203%203%3E%2C    
> >> I do not understand this - it is not cpus phandle. Please respond
> >> to specific comment: how many arguments are allowed by dtschema
> >> for cpus?  
> > 
> > Hi Krzysztof,
> > 
> > If you mean checking
> > here? https://github.com/devicetree-org/dt-schema/blob/e6ea659d2baa30df1ec0fcc4f8354208692489eb/dtschema/schemas/cpu-map.yaml#L110
> > 
> > There is no parameters allowed at this point for cpu phandles in the
> > cpu-map tree. Of course, this is different than what's been
> > implemented in the patchset.  
> Hm, ok, I thought you are adding this for cpu-map, but if not, then
> where are the bindings for this ABI?
> 
> BTW, share your DTS so we can be sure that it is properly validated
> against bindings.

No wait, I am adding this to cpu-map indeed, I meant the code is
different from what's available in the dt-schema, meaning that there is
a mismatch like what you pointed.

Based on your comments, my conjecture is that I will need to include dt
binding anyways.

SMT threads should be represented in the reg array of CPU nodes, and
will need to be addressed via an extra parameter specifying an index in
the reg array. 

There are various places in the kernel where we point to
CPU node using those phandles. Now the first place that we are trying to change is cpu-map for
enabling SMT resource sharing, and that probably means I should update
the binding related to that.

Hope that clarifies.

Thanks,
Alireza

> 
> Best regards,
> Krzysztof