Re: [PATCH net-next 3/5] net: pcs: qcom-ipq: Add PCS create and phylink operations for IPQ9574

[Lei Wei <quic_leiwei@quicinc.com>]

On 11/8/2024 9:24 PM, Andrew Lunn wrote:
>>> Another artefact of not have a child-parent relationship. I wounder if
>>> it makes sense to change the architecture. Have the PCS driver
>>> instantiate the PCS devices as its children. They then have a device
>>> structure for calls like clk_bulk_get(), and a more normal
>>> consumer/provider setup.
>>>
>>
>> I think you may be suggesting to drop the child node usage in the DTS, so
>> that we can attach all the MII clocks to the single PCS node, to facilitate
>> usage of bulk get() API to retrieve the MII clocks for that PCS.
> 
> I would keep the child nodes. They describe the cookie-cutter nature
> of the hardware. The problem is with the clk_bulk API, not allowing
> you to pass a device_node. of_clk_bulk_get() appears to do what you
> want, but it is not exported. What we do have is:
> 
> /**
>   * devm_get_clk_from_child - lookup and obtain a managed reference to a
>   *                           clock producer from child node.
>   * @dev: device for clock "consumer"
>   * @np: pointer to clock consumer node
>   * @con_id: clock consumer ID
>   *
>   * This function parses the clocks, and uses them to look up the
>   * struct clk from the registered list of clock providers by using
>   * @np and @con_id
>   *
>   * The clock will automatically be freed when the device is unbound
>   * from the bus.
>   */
> struct clk *devm_get_clk_from_child(struct device *dev,
>                                      struct device_node *np, const char *con_id);
> 
> So maybe a devm_get_clk_bulk_from_child() would be accepted?
> 
> However, it might not be worth the effort. Using the bulk API was just
> a suggestion to make the code simpler, not a strong requirement.
> 

OK, I agree.

For the PCS instantiation for child nodes, I would like to summarize the 
two options we have, and mention our chosen approach. 1.) Instantiate 
the PCS during the create API call, and export create/destroy API to the 
network driver similar to existing drivers (OR) 2.) Instantiate the 
child nodes during PCS probe and let the MAC driver access the 
'phylink_pcs' object using a ipq_pcs_get()/ipq_pcs_put() API instead of 
ipq_pcs_create()/ipq_pcs_destroy().

The other PCS drivers are following the create/destroy usage pattern 
(option 1). However we are leaning towards option 2, since it is a 
simpler design. Hope this approach is ok.

> 	Andrew