[U-Boot] [PATCH 1/6 v3] include: configs: ls1028a: set SYS_RX_ETH_BUFFER to 8

[Bin Meng]

Hi Alex,

On Wed, Jul 3, 2019 at 1:46 AM Alex Marginean <alexm.osslist at gmail.com> wrote:
>
> Hi Bin,
>
> On 7/1/2019 11:08 AM, Bin Meng wrote:
> > Hi Alex,
> >
> > On Thu, Jun 20, 2019 at 10:48 PM Alex Marginean
> > <alexandru.marginean at nxp.com> wrote:
> >>
> >> LS1028A ethernet interfaces work with at least 8 BDs, set number of buffers
> >> to match that.
> >>
> >> Signed-off-by: Alex Marginean <alexm.osslist at gmail.com>
> >> ---
> >>   include/configs/ls1028a_common.h | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/include/configs/ls1028a_common.h b/include/configs/ls1028a_common.h
> >> index 0db86396e9..f9d2602afd 100644
> >> --- a/include/configs/ls1028a_common.h
> >> +++ b/include/configs/ls1028a_common.h
> >> @@ -197,4 +197,8 @@
> >>   #define CONFIG_SYS_EEPROM_PAGE_WRITE_BITS      3
> >>   #define CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS  5
> >>
> >> +/* Ethernet */
> >> +/* smallest ENETC BD ring has 8 entries */
> >> +#define CONFIG_SYS_RX_ETH_BUFFER               8
> >> +
> >
> > We should avoid adding more ad-hoc CONFIG_XXX macros. We should either
> > make this a Kconfig option, or move this macro to the driver header
> > file (eg: fsl_enetc.h) and do not prefix it with CONFIG_.
> > If we move to driver header, this patch can be simply dropped as I see
> > there are already macro defined in patch [2/6].
>
> This is used in net.h to define PKTBUFSRX which is further used in net.c
> to define net_pkt_buf.  I was actually planning to use net_pkt_buf
> instead of allocation memory in the driver, that's why I used it.
> I can use an enetc specific define, but then I either have to allocate
> packet data in the driver or put more code in to only use up to 4
> (default CONFIG_SYS_RX_ETH_BUFFER value) BDs in an 8 entry BDR.

Ah, thanks for the clarification. I missed that! Then this looks good to me.

Reviewed-by: Bin Meng <bmeng.cn at gmail.com>

Regards,
Bin