| Message ID | 37949d69a2b35018dd418f5ee138abf217a82550.1714046812.git.siyanteng@loongson.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | stmmac: Add Loongson platform support | expand |
On Thu, Apr 25, 2024 at 09:04:35PM +0800, Yanteng Si wrote: > The multicast_filter_bins is initialized twice, it should > be 256, let's drop the first useless assignment. Please drop the second plat_stmmacenet_data::multicast_filter_bins init statement and just change the first one to initializing the correct value - 256. Thus you'll have 1. the multicast and unicast filters size inits done in the same place; 2. the in-situ comments preserved (it's not like they're that much helpful, but seeing the rest of the lines have a comment above it would be nice to have the comment preserved here too); 3. dropped the statement closely attached to the return statement (in kernel it's a widespread practice to separate the return statement with an empty line). The unit 1. is the main reason of course. A bit more readable commit log would be: "The plat_stmmacenet_data::multicast_filter_bins field is twice initialized in the loongson_default_data() method. Drop the redundant initialization, but for the readability sake keep the filters init statements defined in the same place of the method." -Serge(y) > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > --- > drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > index 9e40c28d453a..19906ea67636 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > @@ -15,9 +15,6 @@ static int loongson_default_data(struct plat_stmmacenet_data *plat) > plat->has_gmac = 1; > plat->force_sf_dma_mode = 1; > > - /* Set default value for multicast hash bins */ > - plat->multicast_filter_bins = HASH_TABLE_SIZE; > - > /* Set default value for unicast filter entries */ > plat->unicast_filter_entries = 1; > > -- > 2.31.4 >
On Fri, May 03, 2024 at 01:55:38PM +0300, Serge Semin wrote: > On Thu, Apr 25, 2024 at 09:04:35PM +0800, Yanteng Si wrote: > > The multicast_filter_bins is initialized twice, it should > > be 256, let's drop the first useless assignment. > > Please drop the second plat_stmmacenet_data::multicast_filter_bins > init statement and just change the first one to initializing the > correct value - 256. Thus you'll have > 1. the multicast and unicast filters size inits done in the same place; > 2. the in-situ comments preserved (it's not like they're that much > helpful, but seeing the rest of the lines have a comment above it > would be nice to have the comment preserved here too); > 3. dropped the statement closely attached to the return statement > (in kernel it's a widespread practice to separate the return > statement with an empty line). > > The unit 1. is the main reason of course. > > A bit more readable commit log would be: > > "The plat_stmmacenet_data::multicast_filter_bins field is twice > initialized in the loongson_default_data() method. Drop the redundant > initialization, but for the readability sake keep the filters init > statements defined in the same place of the method." [PATCH net-next v12 04/15] net: stmmac: dwmac-loongson: Drop useless platform data The patch subject is too generic. Just make it: [PATCH net-next v12 04/15] net: stmmac: dwmac-loongson: Drop duplicated hash-based filter size init -Serge(y) > > -Serge(y) > > > > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > > --- > > drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > > index 9e40c28d453a..19906ea67636 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > > @@ -15,9 +15,6 @@ static int loongson_default_data(struct plat_stmmacenet_data *plat) > > plat->has_gmac = 1; > > plat->force_sf_dma_mode = 1; > > > > - /* Set default value for multicast hash bins */ > > - plat->multicast_filter_bins = HASH_TABLE_SIZE; > > - > > /* Set default value for unicast filter entries */ > > plat->unicast_filter_entries = 1; > > > > -- > > 2.31.4 > >
在 2024/5/3 18:55, Serge Semin 写道: > On Thu, Apr 25, 2024 at 09:04:35PM +0800, Yanteng Si wrote: >> The multicast_filter_bins is initialized twice, it should >> be 256, let's drop the first useless assignment. > Please drop the second plat_stmmacenet_data::multicast_filter_bins > init statement and just change the first one to initializing the > correct value - 256. Thus you'll have > 1. the multicast and unicast filters size inits done in the same place; > 2. the in-situ comments preserved (it's not like they're that much > helpful, but seeing the rest of the lines have a comment above it > would be nice to have the comment preserved here too); > 3. dropped the statement closely attached to the return statement > (in kernel it's a widespread practice to separate the return > statement with an empty line). > > The unit 1. is the main reason of course. > > A bit more readable commit log would be: > > "The plat_stmmacenet_data::multicast_filter_bins field is twice > initialized in the loongson_default_data() method. Drop the redundant > initialization, but for the readability sake keep the filters init > statements defined in the same place of the method." OK! Thanks, Yanteng
在 2024/5/3 22:47, Serge Semin 写道: > On Fri, May 03, 2024 at 01:55:38PM +0300, Serge Semin wrote: >> On Thu, Apr 25, 2024 at 09:04:35PM +0800, Yanteng Si wrote: >>> The multicast_filter_bins is initialized twice, it should >>> be 256, let's drop the first useless assignment. >> Please drop the second plat_stmmacenet_data::multicast_filter_bins >> init statement and just change the first one to initializing the >> correct value - 256. Thus you'll have >> 1. the multicast and unicast filters size inits done in the same place; >> 2. the in-situ comments preserved (it's not like they're that much >> helpful, but seeing the rest of the lines have a comment above it >> would be nice to have the comment preserved here too); >> 3. dropped the statement closely attached to the return statement >> (in kernel it's a widespread practice to separate the return >> statement with an empty line). >> >> The unit 1. is the main reason of course. >> >> A bit more readable commit log would be: >> >> "The plat_stmmacenet_data::multicast_filter_bins field is twice >> initialized in the loongson_default_data() method. Drop the redundant >> initialization, but for the readability sake keep the filters init >> statements defined in the same place of the method." > [PATCH net-next v12 04/15] net: stmmac: dwmac-loongson: Drop useless platform data > > The patch subject is too generic. Just make it: > > [PATCH net-next v12 04/15] net: stmmac: dwmac-loongson: Drop duplicated hash-based filter size init OK, Thanks! Thanks, Yanteng
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index 9e40c28d453a..19906ea67636 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -15,9 +15,6 @@ static int loongson_default_data(struct plat_stmmacenet_data *plat) plat->has_gmac = 1; plat->force_sf_dma_mode = 1; - /* Set default value for multicast hash bins */ - plat->multicast_filter_bins = HASH_TABLE_SIZE; - /* Set default value for unicast filter entries */ plat->unicast_filter_entries = 1;