Re: [PATCH net 1/2] net: stmmac: Limit FIFO size by hardware feature value

[Kunihiko Hayashi]

Hi Andrew,

On 2025/01/21 1:29, Andrew Lunn wrote:
> On Mon, Jan 20, 2025 at 02:20:23PM +0900, Kunihiko Hayashi wrote:
> > Hi Andrew,
> >
> > On 2025/01/17 5:16, Andrew Lunn wrote:
> > > On Thu, Jan 16, 2025 at 11:08:52AM +0900, Kunihiko Hayashi wrote:
> > > > Tx/Rx FIFO size is specified by the parameter "{tx,rx}-fifo-depth"
> from
> > > > the platform layer.
> > > >
> > > > However, these values are constrained by upper limits determined by
> the
> > > > capabilities of each hardware feature. There is a risk that the
> upper
> > > > bits will be truncated due to the calculation, so it's appropriate
> to
> > > > limit them to the upper limit values.
> > >
> > > Are these values hard coded in the platform layer? Or can they come
> > > from userspace?
> >
> > My explanation is insufficient and misleading.
> > "From the platform layer" means the common layer of stmmac described in
> > "stmmac_platform.c".
> >
> > > If they are hard coded, we should also fix them. So maybe add a
> > > netdev_warn(), and encourage the platform maintainers to fix their
> > > platform. If they are coming from userspace, we should consider
> > > failing the ethtool call with an -EINVAL, and maybe an extack with the
> > > valid range?
> >
> > These values are derived from the devicetree and stored in the stmmac
> > private structure. They are hardware-specific values, so I think this
> > fix is sufficient.
>
> But if they are coming from device tree, the device tree developer has
> made an error, which has been silently ignored. Do we want to leave
> the device tree broken? Or should we encourage developers to fix them?
> Printing a warning would facilitate that.

I think that developers should fix the devicetree, so I'll add a warning
message if the specified value exceeds the hardware capability.

Thank you,

---
Best Regards
Kunihiko Hayashi