Re: [PATCH net v2 0/3] Limit devicetree parameters to hardware capability

From: Kunihiko Hayashi
Date: Thu Jan 23 2025 - 23:20:10 EST


Hi Russell,

On 2025/01/24 1:31, Russell King (Oracle) wrote:
On Thu, Jan 23, 2025 at 02:25:15PM +0900, Kunihiko Hayashi wrote:
Hi Russell,

Thank you for your comment.

On 2025/01/21 19:25, Russell King (Oracle) wrote:
On Tue, Jan 21, 2025 at 01:41:35PM +0900, Kunihiko Hayashi wrote:
This series includes patches that checks the devicetree properties,
the number of MTL queues and FIFO size values, and if these
specified
values exceed the value contained in the hardware capabilities,
limit to
the values from the capabilities.

And this sets hardware capability values if FIFO sizes are not
specified
and removes redundant lines.

I think you also indeed to explain why (and possibly understand) - if
there are hardware capabilities that describe these parameters - it
has
been necessary to have them in firmware as well.

There are two scenarios I can think of why these would be duplicated:

1. firmware/platform capabilities are there to correct wrong values
provided by the hardware.
2. firmware/platform capabilities are there to reduce the parameters
below hardware maximums.

Which it is affects whether your patch is correct or not, and thus
needs
to be mentioned.

I think scenario 2 applies in this case.

In light of my other reply
(https://lore.kernel.org/r/Z4_ZilVFKacuAUE8@xxxxxxxxxxxxxxxxxxxxx) I
don't think either of my two above applies, and the driver is designed
to allow platform code to override the hardware value, or to provide
the value if there is no hardware value.

I understand. Especially I realized that I had to care about some hardwares
not having these values.

My suggestion, therefore, would be (e.g.):

if (priv->dma_cap.rx_fifo_size &&
priv->plat->rx_fifo_size > priv->dma_cap.rx_fifo_size) {
dev_warn(priv->device,
"Rx FIFO size exceeds dma capability (%d)\n",
priv->plat->rx_fifo_size);
priv->plat->rx_fifo_size = priv->dma_cap.rx_fifo_size;
}

if we still want to limit it to the hardware provided capability, where
that is provided.

Thank you for your suggestion. I also came up with the same code in:
https://lore.kernel.org/all/c2aa354d-1bd5-4fb0-aa8b8-48fcce3c1628@xxxxxxxxxxxxx/#t

I'll reflect this code to the next.

Thank you,

---
Best Regards
Kunihiko Hayashi