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

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] [day] [month] [year] [list]

Message-ID: <8f319458-96fd-42dd-9580-cee3f2b00341@quicinc.com>
Date: Tue, 12 Nov 2024 20:48:33 +0800
From: Lei Wei <quic_leiwei@...cinc.com>
To: Andrew Lunn <andrew@...n.ch>
CC: "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni
	<pabeni@...hat.com>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Heiner Kallweit
	<hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, <netdev@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <quic_kkumarcs@...cinc.com>, <quic_suruchia@...cinc.com>,
        <quic_pavir@...cinc.com>, <quic_linchen@...cinc.com>,
        <quic_luoj@...cinc.com>, <srinivas.kandagatla@...aro.org>,
        <bartosz.golaszewski@...aro.org>, <vsmuthu@....qualcomm.com>,
        <john@...ozen.org>
Subject: Re: [PATCH net-next 3/5] net: pcs: qcom-ipq: Add PCS create and
 phylink operations for IPQ9574



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