Messages in this thread | | | From | Wei Fang <> | Subject | RE: [PATCH V2 net-next] net: fec: add CBS offload support | Date | Fri, 17 Feb 2023 02:18:10 +0000 |
| |
> -----Original Message----- > From: Alexander Lobakin <aleksander.lobakin@intel.com> > Sent: 2023年2月16日 23:28 > To: Wei Fang <wei.fang@nxp.com> > Cc: Shenwei Wang <shenwei.wang@nxp.com>; Clark Wang > <xiaoning.wang@nxp.com>; davem@davemloft.net; edumazet@google.com; > kuba@kernel.org; pabeni@redhat.com; simon.horman@corigine.com; > andrew@lunn.ch; netdev@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH V2 net-next] net: fec: add CBS offload support > > From: Wei Fang <wei.fang@nxp.com> > Date: Thu, 16 Feb 2023 13:03:37 +0000 > > > > >> -----Original Message----- > >> From: Alexander Lobakin <alexandr.lobakin@intel.com> > >> Sent: 2023年2月15日 0:49 > >> To: Wei Fang <wei.fang@nxp.com> > >> Cc: Shenwei Wang <shenwei.wang@nxp.com>; Clark Wang > >> <xiaoning.wang@nxp.com>; davem@davemloft.net; > edumazet@google.com; > >> kuba@kernel.org; pabeni@redhat.com; simon.horman@corigine.com; > >> andrew@lunn.ch; netdev@vger.kernel.org; dl-linux-imx > >> <linux-imx@nxp.com>; linux-kernel@vger.kernel.org > >> Subject: Re: [PATCH V2 net-next] net: fec: add CBS offload support > >> > >> From: Wei Fang <wei.fang@nxp.com> > >> Date: Tue, 14 Feb 2023 09:34:09 +0000 > > [...] > >>>> Oh okay. Then rounddown_pow_of_two() is what you're looking for. > >>>> > >>>> power = rounddown_pow_of_two(idle_slope); > >>>> > >>>> Or even just use one variable, @idle_slope. > >>>> > >>> Thanks for the reminder, I think I should use roundup_pow_of_two(). > >> > >> But your code does what rounddown_pow_of_two() does, not roundup. > >> Imagine that you have 0b1111, then your code will turn it into > >> 0b1000, not 0b10000. Or am I missing something? > >> > > 0b1111 is nearest to 0b10000, so it should be turned into 0x10000. > > fls() + BIT() won't give you the *nearest* pow-2, have you checked what your > code does return? Check with 0xff and then 0x101 and you'll be surprised, it > doesn't work that way. > My real intention is, if x >= 1.5 * 2 ^ (fls(x) - 1), then x = 2 ^ fls(x), otherwise, x = 2 ^ (fls(x) - 1). Anyway, I'll check the final implementation can meet my expectation.
> I'd highly suggest you introducing not only round_closest(), but also > round_closest_pow_of_two(), as your driver might not be the sole user of such > generic functionality. > Yes, I agree with you, it's better to use the generic functionality.
> > > >>> > >>>>> + idle_slope = DIV_ROUND_CLOSEST(idle_slope, power) * power; > >>>>> + > >>>>> + return idle_slope; > >>>> > >>>> You can return DIV_ROUND_ ... right away, without assignning first. > >>>> Also, I'm thinking of that this might be a generic helper. We have > >>>> roundup() and rounddown(), this could be something like > "round_closest()"? > [...] > > Thanks, > Olek
| |