| Message ID | 20220719005726.8739-17-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: qca8k: code split for qca8k | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next, async |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 2 this patch: 0 |
| netdev/cc_maintainers | success | CCed 10 of 10 maintainers |
| netdev/build_clang | success | Errors and warnings before: 2 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 2 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 110 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Tue, Jul 19, 2022 at 02:57:26AM +0200, Christian Marangi wrote: > Some function were exposed to permit migration to common code. Drop them > and make them static now that the user are in the same common code. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- Hmm, ideally the last patch that removes a certain function from being called from qca8k-8xxx.c would also delete its prototype and make it static in qca8k-common.c. Would that be hard to change?
On Tue, Jul 19, 2022 at 04:29:31PM +0300, Vladimir Oltean wrote: > On Tue, Jul 19, 2022 at 02:57:26AM +0200, Christian Marangi wrote: > > Some function were exposed to permit migration to common code. Drop them > > and make them static now that the user are in the same common code. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > Hmm, ideally the last patch that removes a certain function from being > called from qca8k-8xxx.c would also delete its prototype and make it > static in qca8k-common.c. Would that be hard to change? Can be done, it's really to compile check the changes and fix them. Problem is that the patch number would explode. (ok explode is a big thing but i think that would add 2-3 more patch to this big series... this is why I did the static change as the last patch instead of in the middle of the series) But yes it's totally doable and not that hard honestly.
On Tue, Jul 19, 2022 at 03:35:18PM +0200, Christian Marangi wrote: > On Tue, Jul 19, 2022 at 04:29:31PM +0300, Vladimir Oltean wrote: > > On Tue, Jul 19, 2022 at 02:57:26AM +0200, Christian Marangi wrote: > > > Some function were exposed to permit migration to common code. Drop them > > > and make them static now that the user are in the same common code. > > > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > > --- > > > > Hmm, ideally the last patch that removes a certain function from being > > called from qca8k-8xxx.c would also delete its prototype and make it > > static in qca8k-common.c. Would that be hard to change? > > Can be done, it's really to compile check the changes and fix them. > Problem is that the patch number would explode. (ok explode is a big > thing but i think that would add 2-3 more patch to this big series... > this is why I did the static change as the last patch instead of in the > middle of the series) > > But yes it's totally doable and not that hard honestly. Why would it add to the patch count? I don't think you understood what I meant. Take for example qca8k_bulk_read(). You migrated it in patch 4, but there was still a user left in qca8k_fdb_read(), which you migrated in patch 5. After patch 5 and until the last one, the prototype of qca8k_bulk_read() was essentially dangling, but you only removed it in the last patch. My request is that you prune the dangling definitions after each patch that stops using something exported. That won't increase the number of changes, but eliminate the last one. Also, it would be great if you could create a dependency graph such that you could avoid temporarily exporting some functions that don't need to be.
On Tue, Jul 19, 2022 at 04:44:27PM +0300, Vladimir Oltean wrote: > My request is that you prune the dangling definitions after each patch > that stops using something exported. Not "after" each patch, but "as part" of each patch.
diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c index 6e6cdb173556..5369f7b10482 100644 --- a/drivers/net/dsa/qca/qca8k-common.c +++ b/drivers/net/dsa/qca/qca8k-common.c @@ -104,7 +104,7 @@ const struct regmap_access_table qca8k_readable_table = { }; /* TODO: remove these extra ops when we can support regmap bulk read/write */ -int qca8k_bulk_read(struct qca8k_priv *priv, u32 reg, u32 *val, int len) +static int qca8k_bulk_read(struct qca8k_priv *priv, u32 reg, u32 *val, int len) { int i, count = len / sizeof(u32), ret; const struct qca8k_match_data *data; @@ -125,7 +125,7 @@ int qca8k_bulk_read(struct qca8k_priv *priv, u32 reg, u32 *val, int len) } /* TODO: remove these extra ops when we can support regmap bulk read/write */ -int qca8k_bulk_write(struct qca8k_priv *priv, u32 reg, u32 *val, int len) +static int qca8k_bulk_write(struct qca8k_priv *priv, u32 reg, u32 *val, int len) { int i, count = len / sizeof(u32), ret; const struct qca8k_match_data *data; @@ -184,7 +184,7 @@ qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb) return 0; } -void +static void qca8k_fdb_write(struct qca8k_priv *priv, u16 vid, u8 port_mask, const u8 *mac, u8 aging) { @@ -208,7 +208,7 @@ qca8k_fdb_write(struct qca8k_priv *priv, u16 vid, u8 port_mask, const u8 *mac, qca8k_bulk_write(priv, QCA8K_REG_ATU_DATA0, reg, sizeof(reg)); } -int +static int qca8k_fdb_access(struct qca8k_priv *priv, enum qca8k_fdb_cmd cmd, int port) { u32 reg; @@ -244,7 +244,7 @@ qca8k_fdb_access(struct qca8k_priv *priv, enum qca8k_fdb_cmd cmd, int port) return 0; } -int +static int qca8k_fdb_next(struct qca8k_priv *priv, struct qca8k_fdb *fdb, int port) { int ret; @@ -257,7 +257,7 @@ qca8k_fdb_next(struct qca8k_priv *priv, struct qca8k_fdb *fdb, int port) return qca8k_fdb_read(priv, fdb); } -int +static int qca8k_fdb_add(struct qca8k_priv *priv, const u8 *mac, u16 port_mask, u16 vid, u8 aging) { @@ -271,7 +271,7 @@ qca8k_fdb_add(struct qca8k_priv *priv, const u8 *mac, u16 port_mask, return ret; } -int +static int qca8k_fdb_del(struct qca8k_priv *priv, const u8 *mac, u16 port_mask, u16 vid) { int ret; @@ -292,7 +292,7 @@ qca8k_fdb_flush(struct qca8k_priv *priv) mutex_unlock(&priv->reg_mutex); } -int +static int qca8k_fdb_search_and_insert(struct qca8k_priv *priv, u8 port_mask, const u8 *mac, u16 vid) { @@ -328,7 +328,7 @@ qca8k_fdb_search_and_insert(struct qca8k_priv *priv, u8 port_mask, return ret; } -int +static int qca8k_fdb_search_and_del(struct qca8k_priv *priv, u8 port_mask, const u8 *mac, u16 vid) { @@ -400,7 +400,7 @@ qca8k_vlan_access(struct qca8k_priv *priv, enum qca8k_vlan_cmd cmd, u16 vid) return 0; } -int +static int qca8k_vlan_add(struct qca8k_priv *priv, u8 port, u16 vid, bool untagged) { u32 reg; @@ -438,7 +438,7 @@ qca8k_vlan_add(struct qca8k_priv *priv, u8 port, u16 vid, bool untagged) return ret; } -int +static int qca8k_vlan_del(struct qca8k_priv *priv, u8 port, u16 vid) { u32 reg, mask; diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h index b74b9012462b..12d0b5b2fd5d 100644 --- a/drivers/net/dsa/qca/qca8k.h +++ b/drivers/net/dsa/qca/qca8k.h @@ -434,22 +434,8 @@ int qca8k_read(struct qca8k_priv *priv, u32 reg, u32 *val); int qca8k_write(struct qca8k_priv *priv, u32 reg, u32 val); int qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val); -int qca8k_bulk_read(struct qca8k_priv *priv, u32 reg, u32 *val, int len); -int qca8k_bulk_write(struct qca8k_priv *priv, u32 reg, u32 *val, int len); - /* Common ops function */ -int qca8k_fdb_access(struct qca8k_priv *priv, enum qca8k_fdb_cmd cmd, int port); -int qca8k_fdb_next(struct qca8k_priv *priv, struct qca8k_fdb *fdb, int port); -int qca8k_fdb_add(struct qca8k_priv *priv, const u8 *mac, u16 port_mask, - u16 vid, u8 aging); -int qca8k_fdb_del(struct qca8k_priv *priv, const u8 *mac, u16 port_mask, u16 vid); void qca8k_fdb_flush(struct qca8k_priv *priv); -int qca8k_fdb_search_and_insert(struct qca8k_priv *priv, u8 port_mask, - const u8 *mac, u16 vid); -int qca8k_fdb_search_and_del(struct qca8k_priv *priv, u8 port_mask, - const u8 *mac, u16 vid); -int qca8k_vlan_add(struct qca8k_priv *priv, u8 port, u16 vid, bool untagged); -int qca8k_vlan_del(struct qca8k_priv *priv, u8 port, u16 vid); /* Common ethtool stats function */ void qca8k_get_strings(struct dsa_switch *ds, int port, u32 stringset, uint8_t *data);
Some function were exposed to permit migration to common code. Drop them and make them static now that the user are in the same common code. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- drivers/net/dsa/qca/qca8k-common.c | 22 +++++++++++----------- drivers/net/dsa/qca/qca8k.h | 14 -------------- 2 files changed, 11 insertions(+), 25 deletions(-)