Re: [PATCH net-next 2/2] mlxsw: spectrum_buffers: Calculate the size of the main pool

[Ido Schimmel <idosch@idosch.org>]

On Tue, Oct 29, 2019 at 08:31:54PM -0700, Nathan Chancellor wrote:
> On Wed, Oct 23, 2019 at 09:05:00AM +0300, Ido Schimmel wrote:
> >  static int mlxsw_sp_sb_prs_init(struct mlxsw_sp *mlxsw_sp,
> >  				const struct mlxsw_sp_sb_pr *prs,
> > +				const struct mlxsw_sp_sb_pool_des *pool_dess,
> >  				size_t prs_len)
> >  {
> > +	/* Round down, unlike mlxsw_sp_bytes_cells(). */
> > +	u32 sb_cells = mlxsw_sp->sb->sb_size / mlxsw_sp->sb->cell_size;
> 
> This causes a link time error on arm32. It can be simply reproduced with
> the following configs + multi_v7_defconfig:
> 
> CONFIG_MLXSW_CORE=y
> CONFIG_MLXSW_PCI=y
> CONFIG_NET_SWITCHDEV=y
> CONFIG_VLAN_8021Q=y
> CONFIG_MLXSW_SPECTRUM=y
> 
> arm-linux-gnueabi-ld: drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.o: in function `mlxsw_sp_buffers_init':
> spectrum_buffers.c:(.text+0x1c88): undefined reference to `__aeabi_uldivmod'
> 
> It can be solved by something like this but I am not sure if that is
> proper or not since div_u64 returns a u64, which would implicitly get
> converted to u32. I can submit it as a formal patch if needed but I
> figured I would reach out first in case you want to go in a different
> direction.

Yes, this looks fine to me. The value can fit in a u32. Tested on my
system without issues.

Thanks for reaching out

> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
> index 33a978af80d6..968f0902e4fe 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
> @@ -470,7 +470,7 @@ static int mlxsw_sp_sb_prs_init(struct mlxsw_sp *mlxsw_sp,
>  				size_t prs_len)
>  {
>  	/* Round down, unlike mlxsw_sp_bytes_cells(). */
> -	u32 sb_cells = mlxsw_sp->sb->sb_size / mlxsw_sp->sb->cell_size;
> +	u32 sb_cells = div_u64(mlxsw_sp->sb->sb_size, mlxsw_sp->sb->cell_size);
>  	u32 rest_cells[2] = {sb_cells, sb_cells};
>  	int i;
>  	int err;