答复: [PATCH net-next] driver: cadence macb driver support acpi mode

[Xiaowu Ding]

Hi Andrew:
	Thank you very much for your advices.
 
	There will be some problems with the clk_hw_register_fixed_rate interface in the acpi mode.
	It seems that the kernel common clock framework can not support the acpi mode,just support the dt mode.
	Register the fixed clocks with the clk_hw_register_fixed_rate function ,and the clks hw should be used by some
	Drivers,but the driver can not get the clock hw with the devm_get_clk function,because this function can not support
	acpi mode, the driver cannot find the device correct parent clk hw node.
	
	So currently, we just get the clk rate from the acpi node with the device_property_read_u32 function.

Br Xiaowu

-----邮件原件-----
发件人: Andrew Lunn <andrew at lunn.ch> 
发送时间: 2022年8月24日 23:17
收件人: Xiaowu Ding <xiaowu.ding at jaguarmicro.com>
抄送: davem at davemloft.net; edumazet at google.com; kuba at kernel.org; pabeni at redhat.com; linux at armlinux.org.uk; nicolas.ferre at microchip.com; claudiu.beznea at microchip.com; palmer at dabbelt.com; paul.walmsley at sifive.com; netdev at vger.kernel.org; linux-kernel at vger.kernel.org; linux-riscv at lists.infradead.org
主题: Re: [PATCH net-next] driver: cadence macb driver support acpi mode

> +/* On ACPI platforms, clocks are controlled by firmware and/or
> + * ACPI, not by drivers.Need to store the clock value.
> + */
> +struct macb_acpi_config {
> +	u32 hclk_rate;          /* amba clock rate*/
> +	u32 pclk_rate;          /* amba apb clock rate*/
> +	u32 txclk_rate;         /* tx clock rate*/
> +	u32 rxclk_rate;         /* rx clock rate*/
> +	u32 tsuclk_rate;        /* tx clock rate*/
> +	bool acpi_enable;       /* is acpi or not */
> +};

> +static int macb_acpi_support(struct macb *bp) {
> +	struct device *dev = &bp->pdev->dev;
> +	struct macb_acpi_config *config = &bp->acpicfg;
> +	int ret;
> +	u32 property;
> +
> +	/*acpi must be report the pclk*/
> +	property = 0;
> +	ret = device_property_read_u32(dev, MACB_SYSPCLOCK, &property);
> +	if (ret) {
> +		dev_err(dev, "unable to obtain %s property\n", MACB_SYSPCLOCK);
> +		return ret;
> +	}
> +
> +	config->pclk_rate = property;

It seems like you could make this simpler by just calling

clk_hw_register_fixed_rate(dev, "pclk", NULL, 0, property);

You then don't need to modify any other code with respect to clocks.
The clock does exist, so model it in the common clock framework.

    Andrew