Re: [PATCH net-next] net: mvpp2: Improve data types and use min()

[Russell King (Oracle)]

On Thu, Jul 11, 2024 at 05:47:43PM +0200, Thorsten Blum wrote:
> Change the data type of the variable freq in mvpp2_rx_time_coal_set()
> and mvpp2_tx_time_coal_set() to u32 because port->priv->tclk also has
> the data type u32.
> 
> Change the data type of the function parameter clk_hz in
> mvpp2_usec_to_cycles() and mvpp2_cycles_to_usec() to u32 accordingly
> and remove the following Coccinelle/coccicheck warning reported by
> do_div.cocci:
> 
>   WARNING: do_div() does a 64-by-32 division, please consider using div64_ul instead
> 
> Use min() to simplify the code and improve its readability.
> 
> Compile-tested only.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@xxxxxxxxxx>

I'm still on holiday, but it's a wet day today. Don't expect replies
from me to be regular.

I don't think this is a good idea.

priv->tclk comes from clk_get_rate() which returns an unsigned long.
tclk should _also_ be an unsigned long, not a u32, so that the range
of values clk_get_rate() returns can be represented without being
truncted.

Thus the use of unsigned long elsewhere where tclk is passed into is
actually correct.

If we need to limit tclk to values that u32 can represent, then that
needs to be done here:

                priv->tclk = clk_get_rate(priv->pp_clk);

by assigning the return value to an unsigned long local variable,
then checking its upper liit before assigning it to priv->tclk.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!