| Message ID | 20231115155958.3249645-1-gbayer@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d565fa4300d9ebd5ba3bbd259ce841f8dab609d6 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] s390/ism: ism driver implies smc protocol | expand |
On Wed, Nov 15, 2023 at 04:59:58PM +0100, Gerd Bayer wrote: > Since commit a72178cfe855 ("net/smc: Fix dependency of SMC on ISM") > you can build the ism code without selecting the SMC network protocol. > That leaves some ism functions be reported as unused. Move these > functions under the conditional compile with CONFIG_SMC. > > Also codify the suggestion to also configure the SMC protocol in ism's > Kconfig - but with an "imply" rather than a "select" as SMC depends on > other config options and allow for a deliberate decision not to build > SMC. Also, mention that in ISM's help. > > Fixes: a72178cfe855 ("net/smc: Fix dependency of SMC on ISM") > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Closes: https://lore.kernel.org/netdev/afd142a2-1fa0-46b9-8b2d-7652d41d3ab8@infradead.org/ > Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com> > Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> > --- > drivers/s390/net/Kconfig | 3 +- > drivers/s390/net/ism_drv.c | 93 +++++++++++++++++++------------------- > 2 files changed, 48 insertions(+), 48 deletions(-) > > Hi Simon, > > this is version 2, that removes the unused forward declaration that you > found in v1 per: > https://lore.kernel.org/netdev/20231115102304.GN74656@kernel.org/#t > Other than that the patch is unchanged. Thanks Gerd, this version looks good to me. Reviewed-by: Simon Horman <horms@kernel.org>
On 11/15/23 07:59, Gerd Bayer wrote: > Since commit a72178cfe855 ("net/smc: Fix dependency of SMC on ISM") > you can build the ism code without selecting the SMC network protocol. > That leaves some ism functions be reported as unused. Move these > functions under the conditional compile with CONFIG_SMC. > > Also codify the suggestion to also configure the SMC protocol in ism's > Kconfig - but with an "imply" rather than a "select" as SMC depends on > other config options and allow for a deliberate decision not to build > SMC. Also, mention that in ISM's help. > > Fixes: a72178cfe855 ("net/smc: Fix dependency of SMC on ISM") > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Closes: https://lore.kernel.org/netdev/afd142a2-1fa0-46b9-8b2d-7652d41d3ab8@infradead.org/ > Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com> > Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> Acked-by: Randy Dunlap <rdunlap@infradead.org> Tested-by: Randy Dunlap <rdunlap@infradead.org> # build-tested Thanks. > --- > drivers/s390/net/Kconfig | 3 +- > drivers/s390/net/ism_drv.c | 93 +++++++++++++++++++------------------- > 2 files changed, 48 insertions(+), 48 deletions(-) > > Hi Simon, > > this is version 2, that removes the unused forward declaration that you > found in v1 per: > https://lore.kernel.org/netdev/20231115102304.GN74656@kernel.org/#t > Other than that the patch is unchanged. > > Thanks, > Gerd > > > diff --git a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig > index 4902d45e929c..c61e6427384c 100644 > --- a/drivers/s390/net/Kconfig > +++ b/drivers/s390/net/Kconfig > @@ -103,10 +103,11 @@ config CCWGROUP > config ISM > tristate "Support for ISM vPCI Adapter" > depends on PCI > + imply SMC > default n > help > Select this option if you want to use the Internal Shared Memory > - vPCI Adapter. > + vPCI Adapter. The adapter can be used with the SMC network protocol. > > To compile as a module choose M. The module name is ism. > If unsure, choose N. > diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c > index 6df7f377d2f9..81aabbfbbe2c 100644 > --- a/drivers/s390/net/ism_drv.c > +++ b/drivers/s390/net/ism_drv.c > @@ -30,7 +30,6 @@ static const struct pci_device_id ism_device_table[] = { > MODULE_DEVICE_TABLE(pci, ism_device_table); > > static debug_info_t *ism_debug_info; > -static const struct smcd_ops ism_ops; > > #define NO_CLIENT 0xff /* must be >= MAX_CLIENTS */ > static struct ism_client *clients[MAX_CLIENTS]; /* use an array rather than */ > @@ -289,22 +288,6 @@ static int ism_read_local_gid(struct ism_dev *ism) > return ret; > } > > -static int ism_query_rgid(struct ism_dev *ism, u64 rgid, u32 vid_valid, > - u32 vid) > -{ > - union ism_query_rgid cmd; > - > - memset(&cmd, 0, sizeof(cmd)); > - cmd.request.hdr.cmd = ISM_QUERY_RGID; > - cmd.request.hdr.len = sizeof(cmd.request); > - > - cmd.request.rgid = rgid; > - cmd.request.vlan_valid = vid_valid; > - cmd.request.vlan_id = vid; > - > - return ism_cmd(ism, &cmd); > -} > - > static void ism_free_dmb(struct ism_dev *ism, struct ism_dmb *dmb) > { > clear_bit(dmb->sba_idx, ism->sba_bitmap); > @@ -429,23 +412,6 @@ static int ism_del_vlan_id(struct ism_dev *ism, u64 vlan_id) > return ism_cmd(ism, &cmd); > } > > -static int ism_signal_ieq(struct ism_dev *ism, u64 rgid, u32 trigger_irq, > - u32 event_code, u64 info) > -{ > - union ism_sig_ieq cmd; > - > - memset(&cmd, 0, sizeof(cmd)); > - cmd.request.hdr.cmd = ISM_SIGNAL_IEQ; > - cmd.request.hdr.len = sizeof(cmd.request); > - > - cmd.request.rgid = rgid; > - cmd.request.trigger_irq = trigger_irq; > - cmd.request.event_code = event_code; > - cmd.request.info = info; > - > - return ism_cmd(ism, &cmd); > -} > - > static unsigned int max_bytes(unsigned int start, unsigned int len, > unsigned int boundary) > { > @@ -503,14 +469,6 @@ u8 *ism_get_seid(void) > } > EXPORT_SYMBOL_GPL(ism_get_seid); > > -static u16 ism_get_chid(struct ism_dev *ism) > -{ > - if (!ism || !ism->pdev) > - return 0; > - > - return to_zpci(ism->pdev)->pchid; > -} > - > static void ism_handle_event(struct ism_dev *ism) > { > struct ism_event *entry; > @@ -569,11 +527,6 @@ static irqreturn_t ism_handle_irq(int irq, void *data) > return IRQ_HANDLED; > } > > -static u64 ism_get_local_gid(struct ism_dev *ism) > -{ > - return ism->local_gid; > -} > - > static int ism_dev_init(struct ism_dev *ism) > { > struct pci_dev *pdev = ism->pdev; > @@ -774,6 +727,22 @@ module_exit(ism_exit); > /*************************** SMC-D Implementation *****************************/ > > #if IS_ENABLED(CONFIG_SMC) > +static int ism_query_rgid(struct ism_dev *ism, u64 rgid, u32 vid_valid, > + u32 vid) > +{ > + union ism_query_rgid cmd; > + > + memset(&cmd, 0, sizeof(cmd)); > + cmd.request.hdr.cmd = ISM_QUERY_RGID; > + cmd.request.hdr.len = sizeof(cmd.request); > + > + cmd.request.rgid = rgid; > + cmd.request.vlan_valid = vid_valid; > + cmd.request.vlan_id = vid; > + > + return ism_cmd(ism, &cmd); > +} > + > static int smcd_query_rgid(struct smcd_dev *smcd, u64 rgid, u32 vid_valid, > u32 vid) > { > @@ -811,6 +780,23 @@ static int smcd_reset_vlan_required(struct smcd_dev *smcd) > return ism_cmd_simple(smcd->priv, ISM_RESET_VLAN); > } > > +static int ism_signal_ieq(struct ism_dev *ism, u64 rgid, u32 trigger_irq, > + u32 event_code, u64 info) > +{ > + union ism_sig_ieq cmd; > + > + memset(&cmd, 0, sizeof(cmd)); > + cmd.request.hdr.cmd = ISM_SIGNAL_IEQ; > + cmd.request.hdr.len = sizeof(cmd.request); > + > + cmd.request.rgid = rgid; > + cmd.request.trigger_irq = trigger_irq; > + cmd.request.event_code = event_code; > + cmd.request.info = info; > + > + return ism_cmd(ism, &cmd); > +} > + > static int smcd_signal_ieq(struct smcd_dev *smcd, u64 rgid, u32 trigger_irq, > u32 event_code, u64 info) > { > @@ -830,11 +816,24 @@ static int smcd_supports_v2(void) > SYSTEM_EID.type[0] != '0'; > } > > +static u64 ism_get_local_gid(struct ism_dev *ism) > +{ > + return ism->local_gid; > +} > + > static u64 smcd_get_local_gid(struct smcd_dev *smcd) > { > return ism_get_local_gid(smcd->priv); > } > > +static u16 ism_get_chid(struct ism_dev *ism) > +{ > + if (!ism || !ism->pdev) > + return 0; > + > + return to_zpci(ism->pdev)->pchid; > +} > + > static u16 smcd_get_chid(struct smcd_dev *smcd) > { > return ism_get_chid(smcd->priv);
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Wed, 15 Nov 2023 16:59:58 +0100 you wrote: > Since commit a72178cfe855 ("net/smc: Fix dependency of SMC on ISM") > you can build the ism code without selecting the SMC network protocol. > That leaves some ism functions be reported as unused. Move these > functions under the conditional compile with CONFIG_SMC. > > Also codify the suggestion to also configure the SMC protocol in ism's > Kconfig - but with an "imply" rather than a "select" as SMC depends on > other config options and allow for a deliberate decision not to build > SMC. Also, mention that in ISM's help. > > [...] Here is the summary with links: - [net,v2] s390/ism: ism driver implies smc protocol https://git.kernel.org/netdev/net/c/d565fa4300d9 You are awesome, thank you!
diff --git a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig index 4902d45e929c..c61e6427384c 100644 --- a/drivers/s390/net/Kconfig +++ b/drivers/s390/net/Kconfig @@ -103,10 +103,11 @@ config CCWGROUP config ISM tristate "Support for ISM vPCI Adapter" depends on PCI + imply SMC default n help Select this option if you want to use the Internal Shared Memory - vPCI Adapter. + vPCI Adapter. The adapter can be used with the SMC network protocol. To compile as a module choose M. The module name is ism. If unsure, choose N. diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c index 6df7f377d2f9..81aabbfbbe2c 100644 --- a/drivers/s390/net/ism_drv.c +++ b/drivers/s390/net/ism_drv.c @@ -30,7 +30,6 @@ static const struct pci_device_id ism_device_table[] = { MODULE_DEVICE_TABLE(pci, ism_device_table); static debug_info_t *ism_debug_info; -static const struct smcd_ops ism_ops; #define NO_CLIENT 0xff /* must be >= MAX_CLIENTS */ static struct ism_client *clients[MAX_CLIENTS]; /* use an array rather than */ @@ -289,22 +288,6 @@ static int ism_read_local_gid(struct ism_dev *ism) return ret; } -static int ism_query_rgid(struct ism_dev *ism, u64 rgid, u32 vid_valid, - u32 vid) -{ - union ism_query_rgid cmd; - - memset(&cmd, 0, sizeof(cmd)); - cmd.request.hdr.cmd = ISM_QUERY_RGID; - cmd.request.hdr.len = sizeof(cmd.request); - - cmd.request.rgid = rgid; - cmd.request.vlan_valid = vid_valid; - cmd.request.vlan_id = vid; - - return ism_cmd(ism, &cmd); -} - static void ism_free_dmb(struct ism_dev *ism, struct ism_dmb *dmb) { clear_bit(dmb->sba_idx, ism->sba_bitmap); @@ -429,23 +412,6 @@ static int ism_del_vlan_id(struct ism_dev *ism, u64 vlan_id) return ism_cmd(ism, &cmd); } -static int ism_signal_ieq(struct ism_dev *ism, u64 rgid, u32 trigger_irq, - u32 event_code, u64 info) -{ - union ism_sig_ieq cmd; - - memset(&cmd, 0, sizeof(cmd)); - cmd.request.hdr.cmd = ISM_SIGNAL_IEQ; - cmd.request.hdr.len = sizeof(cmd.request); - - cmd.request.rgid = rgid; - cmd.request.trigger_irq = trigger_irq; - cmd.request.event_code = event_code; - cmd.request.info = info; - - return ism_cmd(ism, &cmd); -} - static unsigned int max_bytes(unsigned int start, unsigned int len, unsigned int boundary) { @@ -503,14 +469,6 @@ u8 *ism_get_seid(void) } EXPORT_SYMBOL_GPL(ism_get_seid); -static u16 ism_get_chid(struct ism_dev *ism) -{ - if (!ism || !ism->pdev) - return 0; - - return to_zpci(ism->pdev)->pchid; -} - static void ism_handle_event(struct ism_dev *ism) { struct ism_event *entry; @@ -569,11 +527,6 @@ static irqreturn_t ism_handle_irq(int irq, void *data) return IRQ_HANDLED; } -static u64 ism_get_local_gid(struct ism_dev *ism) -{ - return ism->local_gid; -} - static int ism_dev_init(struct ism_dev *ism) { struct pci_dev *pdev = ism->pdev; @@ -774,6 +727,22 @@ module_exit(ism_exit); /*************************** SMC-D Implementation *****************************/ #if IS_ENABLED(CONFIG_SMC) +static int ism_query_rgid(struct ism_dev *ism, u64 rgid, u32 vid_valid, + u32 vid) +{ + union ism_query_rgid cmd; + + memset(&cmd, 0, sizeof(cmd)); + cmd.request.hdr.cmd = ISM_QUERY_RGID; + cmd.request.hdr.len = sizeof(cmd.request); + + cmd.request.rgid = rgid; + cmd.request.vlan_valid = vid_valid; + cmd.request.vlan_id = vid; + + return ism_cmd(ism, &cmd); +} + static int smcd_query_rgid(struct smcd_dev *smcd, u64 rgid, u32 vid_valid, u32 vid) { @@ -811,6 +780,23 @@ static int smcd_reset_vlan_required(struct smcd_dev *smcd) return ism_cmd_simple(smcd->priv, ISM_RESET_VLAN); } +static int ism_signal_ieq(struct ism_dev *ism, u64 rgid, u32 trigger_irq, + u32 event_code, u64 info) +{ + union ism_sig_ieq cmd; + + memset(&cmd, 0, sizeof(cmd)); + cmd.request.hdr.cmd = ISM_SIGNAL_IEQ; + cmd.request.hdr.len = sizeof(cmd.request); + + cmd.request.rgid = rgid; + cmd.request.trigger_irq = trigger_irq; + cmd.request.event_code = event_code; + cmd.request.info = info; + + return ism_cmd(ism, &cmd); +} + static int smcd_signal_ieq(struct smcd_dev *smcd, u64 rgid, u32 trigger_irq, u32 event_code, u64 info) { @@ -830,11 +816,24 @@ static int smcd_supports_v2(void) SYSTEM_EID.type[0] != '0'; } +static u64 ism_get_local_gid(struct ism_dev *ism) +{ + return ism->local_gid; +} + static u64 smcd_get_local_gid(struct smcd_dev *smcd) { return ism_get_local_gid(smcd->priv); } +static u16 ism_get_chid(struct ism_dev *ism) +{ + if (!ism || !ism->pdev) + return 0; + + return to_zpci(ism->pdev)->pchid; +} + static u16 smcd_get_chid(struct smcd_dev *smcd) { return ism_get_chid(smcd->priv);
Since commit a72178cfe855 ("net/smc: Fix dependency of SMC on ISM") you can build the ism code without selecting the SMC network protocol. That leaves some ism functions be reported as unused. Move these functions under the conditional compile with CONFIG_SMC. Also codify the suggestion to also configure the SMC protocol in ism's Kconfig - but with an "imply" rather than a "select" as SMC depends on other config options and allow for a deliberate decision not to build SMC. Also, mention that in ISM's help. Fixes: a72178cfe855 ("net/smc: Fix dependency of SMC on ISM") Reported-by: Randy Dunlap <rdunlap@infradead.org> Closes: https://lore.kernel.org/netdev/afd142a2-1fa0-46b9-8b2d-7652d41d3ab8@infradead.org/ Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com> Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> --- drivers/s390/net/Kconfig | 3 +- drivers/s390/net/ism_drv.c | 93 +++++++++++++++++++------------------- 2 files changed, 48 insertions(+), 48 deletions(-) Hi Simon, this is version 2, that removes the unused forward declaration that you found in v1 per: https://lore.kernel.org/netdev/20231115102304.GN74656@kernel.org/#t Other than that the patch is unchanged. Thanks, Gerd