Re: [PATCH net v2] s390/ism: ism driver implies smc protocol

Randy Dunlap
2023-11-15T20:43:31Z

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);

--
~Randy