All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
@ 2024-02-22 20:57 Luiz Capitulino
  2024-02-22 20:57 ` [PATCH 1/2] platform/mellanox: mlxbf-pmc: mlxbf_pmc_event_list(): make size ptr optional Luiz Capitulino
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Luiz Capitulino @ 2024-02-22 20:57 UTC (permalink / raw)
  To: ilpo.jarvinen, shravankr
  Cc: davthompson, ndalvi, linux-kernel, platform-driver-x86, Luiz Capitulino

Hi,

The mlxbf-pmc driver fails to load when the firmware reports a new but not
yet implemented performance block. I can reproduce this today with a
Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since this
reports the new clock_measure performance block.

This[1] patch from Shravan implements the clock_measure support and will
solve the issue. But this series avoids the situation by ignoring and
logging unsupported performance blocks.

NOTE: This series is based on latest linux-next which contains new changes
to mlxbf-pmc.

[1] https://lore.kernel.org/lkml/1c2f1b6da51523fe0f338f9ddce9e3903148f604.1707808180.git.shravankr@nvidia.com/

Luiz Capitulino (2):
  platform/mellanox: mlxbf-pmc: mlxbf_pmc_event_list(): make size ptr
    optional
  platform/mellanox: mlxbf-pmc: Ignore unsupported performance blocks

 drivers/platform/mellanox/mlxbf-pmc.c | 56 +++++++++++++++++----------
 1 file changed, 36 insertions(+), 20 deletions(-)

-- 
2.43.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] platform/mellanox: mlxbf-pmc: mlxbf_pmc_event_list(): make size ptr optional
  2024-02-22 20:57 [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading Luiz Capitulino
@ 2024-02-22 20:57 ` Luiz Capitulino
  2024-02-26 10:49   ` Hans de Goede
  2024-02-22 20:57 ` [PATCH 2/2] platform/mellanox: mlxbf-pmc: Ignore unsupported performance blocks Luiz Capitulino
  2024-02-26 13:27 ` [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading Ilpo Järvinen
  2 siblings, 1 reply; 16+ messages in thread
From: Luiz Capitulino @ 2024-02-22 20:57 UTC (permalink / raw)
  To: ilpo.jarvinen, shravankr
  Cc: davthompson, ndalvi, linux-kernel, platform-driver-x86, Luiz Capitulino

The mlxbf_pmc_event_list() function returns a pointer to an array of
supported events and the array size. The array size is returned via
a pointer passed as an argument, which is mandatory.

However, we want to be able to use mlxbf_pmc_event_list() just to check
if a block name is implemented/supported. For this usage passing the size
argument is not necessary so let's make it optional.

Signed-off-by: Luiz Capitulino <luizcap@redhat.com>
---
 drivers/platform/mellanox/mlxbf-pmc.c | 40 +++++++++++++++------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
index 250405bb59a7..b71636eb3db1 100644
--- a/drivers/platform/mellanox/mlxbf-pmc.c
+++ b/drivers/platform/mellanox/mlxbf-pmc.c
@@ -966,32 +966,33 @@ static bool mlxbf_pmc_valid_range(unsigned int blk_num, u32 offset)
 }
 
 /* Get the event list corresponding to a certain block */
-static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk, size_t *size)
+static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk, size_t *psize)
 {
 	const struct mlxbf_pmc_events *events;
+	size_t size;
 
 	if (strstr(blk, "tilenet")) {
 		events = mlxbf_pmc_hnfnet_events;
-		*size = ARRAY_SIZE(mlxbf_pmc_hnfnet_events);
+		size = ARRAY_SIZE(mlxbf_pmc_hnfnet_events);
 	} else if (strstr(blk, "tile")) {
 		events = mlxbf_pmc_hnf_events;
-		*size = ARRAY_SIZE(mlxbf_pmc_hnf_events);
+		size = ARRAY_SIZE(mlxbf_pmc_hnf_events);
 	} else if (strstr(blk, "triogen")) {
 		events = mlxbf_pmc_smgen_events;
-		*size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
+		size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
 	} else if (strstr(blk, "trio")) {
 		switch (pmc->event_set) {
 		case MLXBF_PMC_EVENT_SET_BF1:
 			events = mlxbf_pmc_trio_events_1;
-			*size = ARRAY_SIZE(mlxbf_pmc_trio_events_1);
+			size = ARRAY_SIZE(mlxbf_pmc_trio_events_1);
 			break;
 		case MLXBF_PMC_EVENT_SET_BF2:
 			events = mlxbf_pmc_trio_events_2;
-			*size = ARRAY_SIZE(mlxbf_pmc_trio_events_2);
+			size = ARRAY_SIZE(mlxbf_pmc_trio_events_2);
 			break;
 		default:
 			events = NULL;
-			*size = 0;
+			size = 0;
 			break;
 		}
 	} else if (strstr(blk, "mss")) {
@@ -999,43 +1000,46 @@ static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk, size
 		case MLXBF_PMC_EVENT_SET_BF1:
 		case MLXBF_PMC_EVENT_SET_BF2:
 			events = mlxbf_pmc_mss_events_1;
-			*size = ARRAY_SIZE(mlxbf_pmc_mss_events_1);
+			size = ARRAY_SIZE(mlxbf_pmc_mss_events_1);
 			break;
 		case MLXBF_PMC_EVENT_SET_BF3:
 			events = mlxbf_pmc_mss_events_3;
-			*size = ARRAY_SIZE(mlxbf_pmc_mss_events_3);
+			size = ARRAY_SIZE(mlxbf_pmc_mss_events_3);
 			break;
 		default:
 			events = NULL;
-			*size = 0;
+			size = 0;
 			break;
 		}
 	} else if (strstr(blk, "ecc")) {
 		events = mlxbf_pmc_ecc_events;
-		*size = ARRAY_SIZE(mlxbf_pmc_ecc_events);
+		size = ARRAY_SIZE(mlxbf_pmc_ecc_events);
 	} else if (strstr(blk, "pcie")) {
 		events = mlxbf_pmc_pcie_events;
-		*size = ARRAY_SIZE(mlxbf_pmc_pcie_events);
+		size = ARRAY_SIZE(mlxbf_pmc_pcie_events);
 	} else if (strstr(blk, "l3cache")) {
 		events = mlxbf_pmc_l3c_events;
-		*size = ARRAY_SIZE(mlxbf_pmc_l3c_events);
+		size = ARRAY_SIZE(mlxbf_pmc_l3c_events);
 	} else if (strstr(blk, "gic")) {
 		events = mlxbf_pmc_smgen_events;
-		*size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
+		size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
 	} else if (strstr(blk, "smmu")) {
 		events = mlxbf_pmc_smgen_events;
-		*size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
+		size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
 	} else if (strstr(blk, "llt_miss")) {
 		events = mlxbf_pmc_llt_miss_events;
-		*size = ARRAY_SIZE(mlxbf_pmc_llt_miss_events);
+		size = ARRAY_SIZE(mlxbf_pmc_llt_miss_events);
 	} else if (strstr(blk, "llt")) {
 		events = mlxbf_pmc_llt_events;
-		*size = ARRAY_SIZE(mlxbf_pmc_llt_events);
+		size = ARRAY_SIZE(mlxbf_pmc_llt_events);
 	} else {
 		events = NULL;
-		*size = 0;
+		size = 0;
 	}
 
+	if (psize)
+		*psize = size;
+
 	return events;
 }
 
-- 
2.43.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] platform/mellanox: mlxbf-pmc: Ignore unsupported performance blocks
  2024-02-22 20:57 [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading Luiz Capitulino
  2024-02-22 20:57 ` [PATCH 1/2] platform/mellanox: mlxbf-pmc: mlxbf_pmc_event_list(): make size ptr optional Luiz Capitulino
@ 2024-02-22 20:57 ` Luiz Capitulino
  2024-02-26 10:49   ` Hans de Goede
  2024-02-26 13:27 ` [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading Ilpo Järvinen
  2 siblings, 1 reply; 16+ messages in thread
From: Luiz Capitulino @ 2024-02-22 20:57 UTC (permalink / raw)
  To: ilpo.jarvinen, shravankr
  Cc: davthompson, ndalvi, linux-kernel, platform-driver-x86, Luiz Capitulino

Currently, the driver has two behaviors to deal with new & unsupported
performance blocks reported by the firmware:

 1. For register and unknown block types, the driver will fail to load
    with the following error message:

    [ 4510.956369] mlxbf-pmc: probe of MLNXBFD2:00 failed with error -22

 2. For counter and crspace blocks, the driver will load and sysfs files
    will be created but getting the contents of event_list or trying to
    setup the counter will fail

Instead, let's ignore and log unsupported blocks. This means the driver
will always load and unsupported blocks will never show up in sysfs.

Signed-off-by: Luiz Capitulino <luizcap@redhat.com>
---
 drivers/platform/mellanox/mlxbf-pmc.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
index b71636eb3db1..746567767e5b 100644
--- a/drivers/platform/mellanox/mlxbf-pmc.c
+++ b/drivers/platform/mellanox/mlxbf-pmc.c
@@ -1043,6 +1043,11 @@ static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk, size
 	return events;
 }
 
+static bool mlxbf_pmc_event_supported(const char *blk)
+{
+	return !!mlxbf_pmc_event_list(blk, NULL);
+}
+
 /* Get the event number given the name */
 static int mlxbf_pmc_get_event_num(const char *blk, const char *evt)
 {
@@ -1761,6 +1766,9 @@ static int mlxbf_pmc_init_perftype_counter(struct device *dev, unsigned int blk_
 	struct mlxbf_pmc_attribute *attr;
 	unsigned int i = 0, j = 0;
 
+	if (!mlxbf_pmc_event_supported(pmc->block_name[blk_num]))
+		return -ENOENT;
+
 	/* "event_list" sysfs to list events supported by the block */
 	attr = &pmc->block[blk_num].attr_event_list;
 	attr->dev_attr.attr.mode = 0444;
@@ -1840,7 +1848,7 @@ static int mlxbf_pmc_init_perftype_reg(struct device *dev, unsigned int blk_num)
 
 	events = mlxbf_pmc_event_list(pmc->block_name[blk_num], &count);
 	if (!events)
-		return -EINVAL;
+		return -ENOENT;
 
 	pmc->block[blk_num].attr_event = devm_kcalloc(
 		dev, count, sizeof(struct mlxbf_pmc_attribute), GFP_KERNEL);
@@ -1878,7 +1886,7 @@ static int mlxbf_pmc_create_groups(struct device *dev, unsigned int blk_num)
 	else if (pmc->block[blk_num].type == MLXBF_PMC_TYPE_REGISTER)
 		err = mlxbf_pmc_init_perftype_reg(dev, blk_num);
 	else
-		err = -EINVAL;
+		err = -ENOENT;
 
 	if (err)
 		return err;
@@ -1983,6 +1991,10 @@ static int mlxbf_pmc_map_counters(struct device *dev)
 			return -ENOMEM;
 
 		ret = mlxbf_pmc_create_groups(dev, i);
+		if (ret == -ENOENT) {
+			dev_warn(dev, "ignoring unsupported block: '%s'\n", pmc->block_name[i]);
+			continue;
+		}
 		if (ret)
 			return ret;
 	}
-- 
2.43.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] platform/mellanox: mlxbf-pmc: mlxbf_pmc_event_list(): make size ptr optional
  2024-02-22 20:57 ` [PATCH 1/2] platform/mellanox: mlxbf-pmc: mlxbf_pmc_event_list(): make size ptr optional Luiz Capitulino
@ 2024-02-26 10:49   ` Hans de Goede
  0 siblings, 0 replies; 16+ messages in thread
From: Hans de Goede @ 2024-02-26 10:49 UTC (permalink / raw)
  To: Luiz Capitulino, ilpo.jarvinen, shravankr
  Cc: davthompson, ndalvi, linux-kernel, platform-driver-x86

Hi,

On 2/22/24 21:57, Luiz Capitulino wrote:
> The mlxbf_pmc_event_list() function returns a pointer to an array of
> supported events and the array size. The array size is returned via
> a pointer passed as an argument, which is mandatory.
> 
> However, we want to be able to use mlxbf_pmc_event_list() just to check
> if a block name is implemented/supported. For this usage passing the size
> argument is not necessary so let's make it optional.
> 
> Signed-off-by: Luiz Capitulino <luizcap@redhat.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>  drivers/platform/mellanox/mlxbf-pmc.c | 40 +++++++++++++++------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
> index 250405bb59a7..b71636eb3db1 100644
> --- a/drivers/platform/mellanox/mlxbf-pmc.c
> +++ b/drivers/platform/mellanox/mlxbf-pmc.c
> @@ -966,32 +966,33 @@ static bool mlxbf_pmc_valid_range(unsigned int blk_num, u32 offset)
>  }
>  
>  /* Get the event list corresponding to a certain block */
> -static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk, size_t *size)
> +static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk, size_t *psize)
>  {
>  	const struct mlxbf_pmc_events *events;
> +	size_t size;
>  
>  	if (strstr(blk, "tilenet")) {
>  		events = mlxbf_pmc_hnfnet_events;
> -		*size = ARRAY_SIZE(mlxbf_pmc_hnfnet_events);
> +		size = ARRAY_SIZE(mlxbf_pmc_hnfnet_events);
>  	} else if (strstr(blk, "tile")) {
>  		events = mlxbf_pmc_hnf_events;
> -		*size = ARRAY_SIZE(mlxbf_pmc_hnf_events);
> +		size = ARRAY_SIZE(mlxbf_pmc_hnf_events);
>  	} else if (strstr(blk, "triogen")) {
>  		events = mlxbf_pmc_smgen_events;
> -		*size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
> +		size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
>  	} else if (strstr(blk, "trio")) {
>  		switch (pmc->event_set) {
>  		case MLXBF_PMC_EVENT_SET_BF1:
>  			events = mlxbf_pmc_trio_events_1;
> -			*size = ARRAY_SIZE(mlxbf_pmc_trio_events_1);
> +			size = ARRAY_SIZE(mlxbf_pmc_trio_events_1);
>  			break;
>  		case MLXBF_PMC_EVENT_SET_BF2:
>  			events = mlxbf_pmc_trio_events_2;
> -			*size = ARRAY_SIZE(mlxbf_pmc_trio_events_2);
> +			size = ARRAY_SIZE(mlxbf_pmc_trio_events_2);
>  			break;
>  		default:
>  			events = NULL;
> -			*size = 0;
> +			size = 0;
>  			break;
>  		}
>  	} else if (strstr(blk, "mss")) {
> @@ -999,43 +1000,46 @@ static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk, size
>  		case MLXBF_PMC_EVENT_SET_BF1:
>  		case MLXBF_PMC_EVENT_SET_BF2:
>  			events = mlxbf_pmc_mss_events_1;
> -			*size = ARRAY_SIZE(mlxbf_pmc_mss_events_1);
> +			size = ARRAY_SIZE(mlxbf_pmc_mss_events_1);
>  			break;
>  		case MLXBF_PMC_EVENT_SET_BF3:
>  			events = mlxbf_pmc_mss_events_3;
> -			*size = ARRAY_SIZE(mlxbf_pmc_mss_events_3);
> +			size = ARRAY_SIZE(mlxbf_pmc_mss_events_3);
>  			break;
>  		default:
>  			events = NULL;
> -			*size = 0;
> +			size = 0;
>  			break;
>  		}
>  	} else if (strstr(blk, "ecc")) {
>  		events = mlxbf_pmc_ecc_events;
> -		*size = ARRAY_SIZE(mlxbf_pmc_ecc_events);
> +		size = ARRAY_SIZE(mlxbf_pmc_ecc_events);
>  	} else if (strstr(blk, "pcie")) {
>  		events = mlxbf_pmc_pcie_events;
> -		*size = ARRAY_SIZE(mlxbf_pmc_pcie_events);
> +		size = ARRAY_SIZE(mlxbf_pmc_pcie_events);
>  	} else if (strstr(blk, "l3cache")) {
>  		events = mlxbf_pmc_l3c_events;
> -		*size = ARRAY_SIZE(mlxbf_pmc_l3c_events);
> +		size = ARRAY_SIZE(mlxbf_pmc_l3c_events);
>  	} else if (strstr(blk, "gic")) {
>  		events = mlxbf_pmc_smgen_events;
> -		*size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
> +		size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
>  	} else if (strstr(blk, "smmu")) {
>  		events = mlxbf_pmc_smgen_events;
> -		*size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
> +		size = ARRAY_SIZE(mlxbf_pmc_smgen_events);
>  	} else if (strstr(blk, "llt_miss")) {
>  		events = mlxbf_pmc_llt_miss_events;
> -		*size = ARRAY_SIZE(mlxbf_pmc_llt_miss_events);
> +		size = ARRAY_SIZE(mlxbf_pmc_llt_miss_events);
>  	} else if (strstr(blk, "llt")) {
>  		events = mlxbf_pmc_llt_events;
> -		*size = ARRAY_SIZE(mlxbf_pmc_llt_events);
> +		size = ARRAY_SIZE(mlxbf_pmc_llt_events);
>  	} else {
>  		events = NULL;
> -		*size = 0;
> +		size = 0;
>  	}
>  
> +	if (psize)
> +		*psize = size;
> +
>  	return events;
>  }
>  


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] platform/mellanox: mlxbf-pmc: Ignore unsupported performance blocks
  2024-02-22 20:57 ` [PATCH 2/2] platform/mellanox: mlxbf-pmc: Ignore unsupported performance blocks Luiz Capitulino
@ 2024-02-26 10:49   ` Hans de Goede
  0 siblings, 0 replies; 16+ messages in thread
From: Hans de Goede @ 2024-02-26 10:49 UTC (permalink / raw)
  To: Luiz Capitulino, ilpo.jarvinen, shravankr
  Cc: davthompson, ndalvi, linux-kernel, platform-driver-x86

Hi,

On 2/22/24 21:57, Luiz Capitulino wrote:
> Currently, the driver has two behaviors to deal with new & unsupported
> performance blocks reported by the firmware:
> 
>  1. For register and unknown block types, the driver will fail to load
>     with the following error message:
> 
>     [ 4510.956369] mlxbf-pmc: probe of MLNXBFD2:00 failed with error -22
> 
>  2. For counter and crspace blocks, the driver will load and sysfs files
>     will be created but getting the contents of event_list or trying to
>     setup the counter will fail
> 
> Instead, let's ignore and log unsupported blocks. This means the driver
> will always load and unsupported blocks will never show up in sysfs.
> 
> Signed-off-by: Luiz Capitulino <luizcap@redhat.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>  drivers/platform/mellanox/mlxbf-pmc.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
> index b71636eb3db1..746567767e5b 100644
> --- a/drivers/platform/mellanox/mlxbf-pmc.c
> +++ b/drivers/platform/mellanox/mlxbf-pmc.c
> @@ -1043,6 +1043,11 @@ static const struct mlxbf_pmc_events *mlxbf_pmc_event_list(const char *blk, size
>  	return events;
>  }
>  
> +static bool mlxbf_pmc_event_supported(const char *blk)
> +{
> +	return !!mlxbf_pmc_event_list(blk, NULL);
> +}
> +
>  /* Get the event number given the name */
>  static int mlxbf_pmc_get_event_num(const char *blk, const char *evt)
>  {
> @@ -1761,6 +1766,9 @@ static int mlxbf_pmc_init_perftype_counter(struct device *dev, unsigned int blk_
>  	struct mlxbf_pmc_attribute *attr;
>  	unsigned int i = 0, j = 0;
>  
> +	if (!mlxbf_pmc_event_supported(pmc->block_name[blk_num]))
> +		return -ENOENT;
> +
>  	/* "event_list" sysfs to list events supported by the block */
>  	attr = &pmc->block[blk_num].attr_event_list;
>  	attr->dev_attr.attr.mode = 0444;
> @@ -1840,7 +1848,7 @@ static int mlxbf_pmc_init_perftype_reg(struct device *dev, unsigned int blk_num)
>  
>  	events = mlxbf_pmc_event_list(pmc->block_name[blk_num], &count);
>  	if (!events)
> -		return -EINVAL;
> +		return -ENOENT;
>  
>  	pmc->block[blk_num].attr_event = devm_kcalloc(
>  		dev, count, sizeof(struct mlxbf_pmc_attribute), GFP_KERNEL);
> @@ -1878,7 +1886,7 @@ static int mlxbf_pmc_create_groups(struct device *dev, unsigned int blk_num)
>  	else if (pmc->block[blk_num].type == MLXBF_PMC_TYPE_REGISTER)
>  		err = mlxbf_pmc_init_perftype_reg(dev, blk_num);
>  	else
> -		err = -EINVAL;
> +		err = -ENOENT;
>  
>  	if (err)
>  		return err;
> @@ -1983,6 +1991,10 @@ static int mlxbf_pmc_map_counters(struct device *dev)
>  			return -ENOMEM;
>  
>  		ret = mlxbf_pmc_create_groups(dev, i);
> +		if (ret == -ENOENT) {
> +			dev_warn(dev, "ignoring unsupported block: '%s'\n", pmc->block_name[i]);
> +			continue;
> +		}
>  		if (ret)
>  			return ret;
>  	}


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-22 20:57 [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading Luiz Capitulino
  2024-02-22 20:57 ` [PATCH 1/2] platform/mellanox: mlxbf-pmc: mlxbf_pmc_event_list(): make size ptr optional Luiz Capitulino
  2024-02-22 20:57 ` [PATCH 2/2] platform/mellanox: mlxbf-pmc: Ignore unsupported performance blocks Luiz Capitulino
@ 2024-02-26 13:27 ` Ilpo Järvinen
  2024-02-26 15:49   ` Luiz Capitulino
  2 siblings, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2024-02-26 13:27 UTC (permalink / raw)
  To: shravankr, Luiz Capitulino
  Cc: davthompson, ndalvi, linux-kernel, platform-driver-x86

On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:

> The mlxbf-pmc driver fails to load when the firmware reports a new but not
> yet implemented performance block. I can reproduce this today with a
> Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since this
> reports the new clock_measure performance block.
> 
> This[1] patch from Shravan implements the clock_measure support and will
> solve the issue. But this series avoids the situation by ignoring and
> logging unsupported performance blocks.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo branch. Note it will show up in the public
platform-drivers-x86/review-ilpo branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/2] platform/mellanox: mlxbf-pmc: mlxbf_pmc_event_list(): make size ptr optional
      commit: c5b649996ac63d43f1d4185de177c90d664b2230
[2/2] platform/mellanox: mlxbf-pmc: Ignore unsupported performance blocks
      commit: 4e39d7be4123f65adf78b0a466cbaf1169d7cedb

--
 i.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-26 13:27 ` [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading Ilpo Järvinen
@ 2024-02-26 15:49   ` Luiz Capitulino
  2024-02-26 16:04     ` Ilpo Järvinen
  0 siblings, 1 reply; 16+ messages in thread
From: Luiz Capitulino @ 2024-02-26 15:49 UTC (permalink / raw)
  To: Ilpo Järvinen, shravankr
  Cc: davthompson, ndalvi, linux-kernel, platform-driver-x86, hdegoede

On 2024-02-26 08:27, Ilpo Järvinen wrote:
> On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:
> 
>> The mlxbf-pmc driver fails to load when the firmware reports a new but not
>> yet implemented performance block. I can reproduce this today with a
>> Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since this
>> reports the new clock_measure performance block.
>>
>> This[1] patch from Shravan implements the clock_measure support and will
>> solve the issue. But this series avoids the situation by ignoring and
>> logging unsupported performance blocks.
>>
>> [...]
> 
> 
> Thank you for your contribution, it has been applied to my local
> review-ilpo branch. Note it will show up in the public
> platform-drivers-x86/review-ilpo branch only once I've pushed my
> local branch there, which might take a while.

Thank you Ilpo and thanks Hans for the review.

The only detail is that we probably want this merged for 6.8 since
the driver doesn't currently load with the configuration mentioned above.

- Luiz

> 
> The list of commits applied:
> [1/2] platform/mellanox: mlxbf-pmc: mlxbf_pmc_event_list(): make size ptr optional
>        commit: c5b649996ac63d43f1d4185de177c90d664b2230
> [2/2] platform/mellanox: mlxbf-pmc: Ignore unsupported performance blocks
>        commit: 4e39d7be4123f65adf78b0a466cbaf1169d7cedb
> 
> --
>   i.
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-26 15:49   ` Luiz Capitulino
@ 2024-02-26 16:04     ` Ilpo Järvinen
  2024-02-26 16:10       ` Luiz Capitulino
  0 siblings, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2024-02-26 16:04 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: shravankr, davthompson, ndalvi, LKML, platform-driver-x86, Hans de Goede

[-- Attachment #1: Type: text/plain, Size: 1406 bytes --]

On Mon, 26 Feb 2024, Luiz Capitulino wrote:

> On 2024-02-26 08:27, Ilpo Järvinen wrote:
> > On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:
> > 
> > > The mlxbf-pmc driver fails to load when the firmware reports a new but not
> > > yet implemented performance block. I can reproduce this today with a
> > > Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since this
> > > reports the new clock_measure performance block.
> > > 
> > > This[1] patch from Shravan implements the clock_measure support and will
> > > solve the issue. But this series avoids the situation by ignoring and
> > > logging unsupported performance blocks.
> > > 
> > > [...]
> > 
> > 
> > Thank you for your contribution, it has been applied to my local
> > review-ilpo branch. Note it will show up in the public
> > platform-drivers-x86/review-ilpo branch only once I've pushed my
> > local branch there, which might take a while.
> 
> Thank you Ilpo and thanks Hans for the review.
> 
> The only detail is that we probably want this merged for 6.8 since
> the driver doesn't currently load with the configuration mentioned above.

Oh, sorry, I missed the mention in the coverletter.

So you'd want I drop these from review-ilpo branch as there they end
up into for-next branch, and they should go through Hans instead who 
handles fixes branch for this cycle?

-- 
 i.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-26 16:04     ` Ilpo Järvinen
@ 2024-02-26 16:10       ` Luiz Capitulino
  2024-02-26 16:57         ` Hans de Goede
  2024-02-26 16:59         ` Ilpo Järvinen
  0 siblings, 2 replies; 16+ messages in thread
From: Luiz Capitulino @ 2024-02-26 16:10 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: shravankr, davthompson, ndalvi, LKML, platform-driver-x86, Hans de Goede

On 2024-02-26 11:04, Ilpo Järvinen wrote:
> On Mon, 26 Feb 2024, Luiz Capitulino wrote:
> 
>> On 2024-02-26 08:27, Ilpo Järvinen wrote:
>>> On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:
>>>
>>>> The mlxbf-pmc driver fails to load when the firmware reports a new but not
>>>> yet implemented performance block. I can reproduce this today with a
>>>> Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since this
>>>> reports the new clock_measure performance block.
>>>>
>>>> This[1] patch from Shravan implements the clock_measure support and will
>>>> solve the issue. But this series avoids the situation by ignoring and
>>>> logging unsupported performance blocks.
>>>>
>>>> [...]
>>>
>>>
>>> Thank you for your contribution, it has been applied to my local
>>> review-ilpo branch. Note it will show up in the public
>>> platform-drivers-x86/review-ilpo branch only once I've pushed my
>>> local branch there, which might take a while.
>>
>> Thank you Ilpo and thanks Hans for the review.
>>
>> The only detail is that we probably want this merged for 6.8 since
>> the driver doesn't currently load with the configuration mentioned above.
> 
> Oh, sorry, I missed the mention in the coverletter.
> 
> So you'd want I drop these from review-ilpo branch as there they end
> up into for-next branch, and they should go through Hans instead who
> handles fixes branch for this cycle?

If that's the path to get this series merged for this cycle then yes,
but let's see if Hans agrees (sorry that I didn't know this before
posting).

One additional detail is that this series is on top of linux-next, which
has two additional mlxbf-pmc changes:

* 
https://lore.kernel.org/lkml/39be055af3506ce6f843d11e45d71620f2a96e26.1707808180.git.shravankr@nvidia.com/
* 
https://lore.kernel.org/lkml/d8548c70339a29258a906b2b518e5c48f669795c.1707808180.git.shravankr@nvidia.com/

Maybe those two should be included for 6.8 as well?

- Luiz


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-26 16:10       ` Luiz Capitulino
@ 2024-02-26 16:57         ` Hans de Goede
  2024-02-26 17:05           ` Luiz Capitulino
  2024-02-27 13:18           ` Ilpo Järvinen
  2024-02-26 16:59         ` Ilpo Järvinen
  1 sibling, 2 replies; 16+ messages in thread
From: Hans de Goede @ 2024-02-26 16:57 UTC (permalink / raw)
  To: Luiz Capitulino, Ilpo Järvinen
  Cc: shravankr, davthompson, ndalvi, LKML, platform-driver-x86

Hi Luiz,

On 2/26/24 17:10, Luiz Capitulino wrote:
> On 2024-02-26 11:04, Ilpo Järvinen wrote:
>> On Mon, 26 Feb 2024, Luiz Capitulino wrote:
>>
>>> On 2024-02-26 08:27, Ilpo Järvinen wrote:
>>>> On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:
>>>>
>>>>> The mlxbf-pmc driver fails to load when the firmware reports a new but not
>>>>> yet implemented performance block. I can reproduce this today with a
>>>>> Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since this
>>>>> reports the new clock_measure performance block.
>>>>>
>>>>> This[1] patch from Shravan implements the clock_measure support and will
>>>>> solve the issue. But this series avoids the situation by ignoring and
>>>>> logging unsupported performance blocks.
>>>>>
>>>>> [...]
>>>>
>>>>
>>>> Thank you for your contribution, it has been applied to my local
>>>> review-ilpo branch. Note it will show up in the public
>>>> platform-drivers-x86/review-ilpo branch only once I've pushed my
>>>> local branch there, which might take a while.
>>>
>>> Thank you Ilpo and thanks Hans for the review.
>>>
>>> The only detail is that we probably want this merged for 6.8 since
>>> the driver doesn't currently load with the configuration mentioned above.
>>
>> Oh, sorry, I missed the mention in the coverletter.
>>
>> So you'd want I drop these from review-ilpo branch as there they end
>> up into for-next branch, and they should go through Hans instead who
>> handles fixes branch for this cycle?
> 
> If that's the path to get this series merged for this cycle then yes,
> but let's see if Hans agrees (sorry that I didn't know this before
> posting).

Hmm, new hw enablement typically goes through -next and not to
the current fixes branch. And AFAICT this is new hw enablement,
not a regression / bug-fix.

Is there any special reason why this needs to be in 6.8 ?

For RHEL kernels you can cherry-pick patches from -next
as necessary.

> One additional detail is that this series is on top of linux-next, which
> has two additional mlxbf-pmc changes:
> 
> * https://lore.kernel.org/lkml/39be055af3506ce6f843d11e45d71620f2a96e26.1707808180.git.shravankr@nvidia.com/
> * https://lore.kernel.org/lkml/d8548c70339a29258a906b2b518e5c48f669795c.1707808180.git.shravankr@nvidia.com/

Hmm, those are not small patches, any other reason
why this really should go to -next IMHO.

Regards,

Hans



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-26 16:10       ` Luiz Capitulino
  2024-02-26 16:57         ` Hans de Goede
@ 2024-02-26 16:59         ` Ilpo Järvinen
  2024-02-26 17:07           ` Luiz Capitulino
  1 sibling, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2024-02-26 16:59 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: shravankr, davthompson, ndalvi, LKML, platform-driver-x86, Hans de Goede

[-- Attachment #1: Type: text/plain, Size: 2741 bytes --]

On Mon, 26 Feb 2024, Luiz Capitulino wrote:

> On 2024-02-26 11:04, Ilpo Järvinen wrote:
> > On Mon, 26 Feb 2024, Luiz Capitulino wrote:
> > 
> > > On 2024-02-26 08:27, Ilpo Järvinen wrote:
> > > > On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:
> > > > 
> > > > > The mlxbf-pmc driver fails to load when the firmware reports a new but
> > > > > not
> > > > > yet implemented performance block. I can reproduce this today with a
> > > > > Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since
> > > > > this
> > > > > reports the new clock_measure performance block.
> > > > > 
> > > > > This[1] patch from Shravan implements the clock_measure support and
> > > > > will
> > > > > solve the issue. But this series avoids the situation by ignoring and
> > > > > logging unsupported performance blocks.
> > > > > 
> > > > > [...]
> > > > 
> > > > 
> > > > Thank you for your contribution, it has been applied to my local
> > > > review-ilpo branch. Note it will show up in the public
> > > > platform-drivers-x86/review-ilpo branch only once I've pushed my
> > > > local branch there, which might take a while.
> > > 
> > > Thank you Ilpo and thanks Hans for the review.
> > > 
> > > The only detail is that we probably want this merged for 6.8 since
> > > the driver doesn't currently load with the configuration mentioned above.
> > 
> > Oh, sorry, I missed the mention in the coverletter.
> > 
> > So you'd want I drop these from review-ilpo branch as there they end
> > up into for-next branch, and they should go through Hans instead who
> > handles fixes branch for this cycle?
> 
> If that's the path to get this series merged for this cycle then yes,
> but let's see if Hans agrees (sorry that I didn't know this before
> posting).
>
> One additional detail is that this series is on top of linux-next, which
> has two additional mlxbf-pmc changes:
>
> *
> https://lore.kernel.org/lkml/39be055af3506ce6f843d11e45d71620f2a96e26.1707808180.git.shravankr@nvidia.com/
> *
> https://lore.kernel.org/lkml/d8548c70339a29258a906b2b518e5c48f669795c.1707808180.git.shravankr@nvidia.com/
> 
> Maybe those two should be included for 6.8 as well?

Those look a new feature to me so they belong to for-next. So no, they 
will not end up into 6.8 (to fixes branch). If the 2 patches in this 
series do not apply without some for-next targetting dependencies, you 
should rebase on top of fixes branch and send a new version.

About those two patches, please also see my reply. I intentionally only 2 
patches of that series because I wanted to see sysfs documentation first 
so you should resend those two patches to for-next with sysfs 
documentation.

-- 
 i.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-26 16:57         ` Hans de Goede
@ 2024-02-26 17:05           ` Luiz Capitulino
  2024-02-27 13:18           ` Ilpo Järvinen
  1 sibling, 0 replies; 16+ messages in thread
From: Luiz Capitulino @ 2024-02-26 17:05 UTC (permalink / raw)
  To: Hans de Goede, Ilpo Järvinen
  Cc: shravankr, davthompson, ndalvi, LKML, platform-driver-x86

On 2024-02-26 11:57, Hans de Goede wrote:
> Hi Luiz,
> 
> On 2/26/24 17:10, Luiz Capitulino wrote:
>> On 2024-02-26 11:04, Ilpo Järvinen wrote:
>>> On Mon, 26 Feb 2024, Luiz Capitulino wrote:
>>>
>>>> On 2024-02-26 08:27, Ilpo Järvinen wrote:
>>>>> On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:
>>>>>
>>>>>> The mlxbf-pmc driver fails to load when the firmware reports a new but not
>>>>>> yet implemented performance block. I can reproduce this today with a
>>>>>> Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since this
>>>>>> reports the new clock_measure performance block.
>>>>>>
>>>>>> This[1] patch from Shravan implements the clock_measure support and will
>>>>>> solve the issue. But this series avoids the situation by ignoring and
>>>>>> logging unsupported performance blocks.
>>>>>>
>>>>>> [...]
>>>>>
>>>>>
>>>>> Thank you for your contribution, it has been applied to my local
>>>>> review-ilpo branch. Note it will show up in the public
>>>>> platform-drivers-x86/review-ilpo branch only once I've pushed my
>>>>> local branch there, which might take a while.
>>>>
>>>> Thank you Ilpo and thanks Hans for the review.
>>>>
>>>> The only detail is that we probably want this merged for 6.8 since
>>>> the driver doesn't currently load with the configuration mentioned above.
>>>
>>> Oh, sorry, I missed the mention in the coverletter.
>>>
>>> So you'd want I drop these from review-ilpo branch as there they end
>>> up into for-next branch, and they should go through Hans instead who
>>> handles fixes branch for this cycle?
>>
>> If that's the path to get this series merged for this cycle then yes,
>> but let's see if Hans agrees (sorry that I didn't know this before
>> posting).
> 
> Hmm, new hw enablement typically goes through -next and not to
> the current fixes branch. And AFAICT this is new hw enablement,
> not a regression / bug-fix.
> 
> Is there any special reason why this needs to be in 6.8 ?

Since the new firmware feature is causing the driver not to load,
I'm seeing this more as a bug than new enablement. But it's fine
with me if you decide on not having them on 6.8.

> For RHEL kernels you can cherry-pick patches from -next
> as necessary.

I know :)

>> One additional detail is that this series is on top of linux-next, which
>> has two additional mlxbf-pmc changes:
>>
>> * https://lore.kernel.org/lkml/39be055af3506ce6f843d11e45d71620f2a96e26.1707808180.git.shravankr@nvidia.com/
>> * https://lore.kernel.org/lkml/d8548c70339a29258a906b2b518e5c48f669795c.1707808180.git.shravankr@nvidia.com/
> 
> Hmm, those are not small patches, any other reason
> why this really should go to -next IMHO.

OK.

- Luiz


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-26 16:59         ` Ilpo Järvinen
@ 2024-02-26 17:07           ` Luiz Capitulino
  2024-02-27 13:20             ` Ilpo Järvinen
  0 siblings, 1 reply; 16+ messages in thread
From: Luiz Capitulino @ 2024-02-26 17:07 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: shravankr, davthompson, ndalvi, LKML, platform-driver-x86, Hans de Goede

On 2024-02-26 11:59, Ilpo Järvinen wrote:
> On Mon, 26 Feb 2024, Luiz Capitulino wrote:
> 
>> On 2024-02-26 11:04, Ilpo Järvinen wrote:
>>> On Mon, 26 Feb 2024, Luiz Capitulino wrote:
>>>
>>>> On 2024-02-26 08:27, Ilpo Järvinen wrote:
>>>>> On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:
>>>>>
>>>>>> The mlxbf-pmc driver fails to load when the firmware reports a new but
>>>>>> not
>>>>>> yet implemented performance block. I can reproduce this today with a
>>>>>> Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since
>>>>>> this
>>>>>> reports the new clock_measure performance block.
>>>>>>
>>>>>> This[1] patch from Shravan implements the clock_measure support and
>>>>>> will
>>>>>> solve the issue. But this series avoids the situation by ignoring and
>>>>>> logging unsupported performance blocks.
>>>>>>
>>>>>> [...]
>>>>>
>>>>>
>>>>> Thank you for your contribution, it has been applied to my local
>>>>> review-ilpo branch. Note it will show up in the public
>>>>> platform-drivers-x86/review-ilpo branch only once I've pushed my
>>>>> local branch there, which might take a while.
>>>>
>>>> Thank you Ilpo and thanks Hans for the review.
>>>>
>>>> The only detail is that we probably want this merged for 6.8 since
>>>> the driver doesn't currently load with the configuration mentioned above.
>>>
>>> Oh, sorry, I missed the mention in the coverletter.
>>>
>>> So you'd want I drop these from review-ilpo branch as there they end
>>> up into for-next branch, and they should go through Hans instead who
>>> handles fixes branch for this cycle?
>>
>> If that's the path to get this series merged for this cycle then yes,
>> but let's see if Hans agrees (sorry that I didn't know this before
>> posting).
>>
>> One additional detail is that this series is on top of linux-next, which
>> has two additional mlxbf-pmc changes:
>>
>> *
>> https://lore.kernel.org/lkml/39be055af3506ce6f843d11e45d71620f2a96e26.1707808180.git.shravankr@nvidia.com/
>> *
>> https://lore.kernel.org/lkml/d8548c70339a29258a906b2b518e5c48f669795c.1707808180.git.shravankr@nvidia.com/
>>
>> Maybe those two should be included for 6.8 as well?
> 
> Those look a new feature to me so they belong to for-next. So no, they
> will not end up into 6.8 (to fixes branch). If the 2 patches in this
> series do not apply without some for-next targetting dependencies, you
> should rebase on top of fixes branch and send a new version.

Understood.

> About those two patches, please also see my reply. I intentionally only 2
> patches of that series because I wanted to see sysfs documentation first
> so you should resend those two patches to for-next with sysfs
> documentation.

I'm actually not author of the other patches :)

- Luiz



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-26 16:57         ` Hans de Goede
  2024-02-26 17:05           ` Luiz Capitulino
@ 2024-02-27 13:18           ` Ilpo Järvinen
  2024-02-27 18:28             ` Luiz Capitulino
  1 sibling, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2024-02-27 13:18 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Luiz Capitulino, shravankr, davthompson, ndalvi, LKML,
	platform-driver-x86

[-- Attachment #1: Type: text/plain, Size: 3160 bytes --]

On Mon, 26 Feb 2024, Hans de Goede wrote:

> Hi Luiz,
> 
> On 2/26/24 17:10, Luiz Capitulino wrote:
> > On 2024-02-26 11:04, Ilpo Järvinen wrote:
> >> On Mon, 26 Feb 2024, Luiz Capitulino wrote:
> >>
> >>> On 2024-02-26 08:27, Ilpo Järvinen wrote:
> >>>> On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:
> >>>>
> >>>>> The mlxbf-pmc driver fails to load when the firmware reports a new but not
> >>>>> yet implemented performance block. I can reproduce this today with a
> >>>>> Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since this
> >>>>> reports the new clock_measure performance block.
> >>>>>
> >>>>> This[1] patch from Shravan implements the clock_measure support and will
> >>>>> solve the issue. But this series avoids the situation by ignoring and
> >>>>> logging unsupported performance blocks.
> >>>>>
> >>>>> [...]
> >>>>
> >>>>
> >>>> Thank you for your contribution, it has been applied to my local
> >>>> review-ilpo branch. Note it will show up in the public
> >>>> platform-drivers-x86/review-ilpo branch only once I've pushed my
> >>>> local branch there, which might take a while.
> >>>
> >>> Thank you Ilpo and thanks Hans for the review.
> >>>
> >>> The only detail is that we probably want this merged for 6.8 since
> >>> the driver doesn't currently load with the configuration mentioned above.
> >>
> >> Oh, sorry, I missed the mention in the coverletter.
> >>
> >> So you'd want I drop these from review-ilpo branch as there they end
> >> up into for-next branch, and they should go through Hans instead who
> >> handles fixes branch for this cycle?
> > 
> > If that's the path to get this series merged for this cycle then yes,
> > but let's see if Hans agrees (sorry that I didn't know this before
> > posting).
> 
> Hmm, new hw enablement typically goes through -next and not to
> the current fixes branch. And AFAICT this is new hw enablement,
> not a regression / bug-fix.
> 
> Is there any special reason why this needs to be in 6.8 ?

To me it sounded like fix to 1a218d312e65 ("platform/mellanox: mlxbf-pmc: 
Add Mellanox BlueField PMC driver") and 423c3361855c ("platform/mellanox: 
mlxbf-pmc: Add support for BlueField-3") although not explicitly marked as 
such.

But I'm fine with taking these through for-next, it's relatively late into 
the cycle already anyway.

> For RHEL kernels you can cherry-pick patches from -next
> as necessary.

It's also possible to send them later directly to stable folks once 
Linus' tree has them after the next merge window if you feel they're 
useful for stable inclusion.

> > One additional detail is that this series is on top of linux-next, which
> > has two additional mlxbf-pmc changes:
> > 
> > * https://lore.kernel.org/lkml/39be055af3506ce6f843d11e45d71620f2a96e26.1707808180.git.shravankr@nvidia.com/
> > * https://lore.kernel.org/lkml/d8548c70339a29258a906b2b518e5c48f669795c.1707808180.git.shravankr@nvidia.com/
> 
> Hmm, those are not small patches, any other reason
> why this really should go to -next IMHO.

Those two linked patches are totally unrelated.


-- 
 i.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-26 17:07           ` Luiz Capitulino
@ 2024-02-27 13:20             ` Ilpo Järvinen
  0 siblings, 0 replies; 16+ messages in thread
From: Ilpo Järvinen @ 2024-02-27 13:20 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: shravankr, davthompson, ndalvi, LKML, platform-driver-x86, Hans de Goede

[-- Attachment #1: Type: text/plain, Size: 3301 bytes --]

On Mon, 26 Feb 2024, Luiz Capitulino wrote:

> On 2024-02-26 11:59, Ilpo Järvinen wrote:
> > On Mon, 26 Feb 2024, Luiz Capitulino wrote:
> > 
> > > On 2024-02-26 11:04, Ilpo Järvinen wrote:
> > > > On Mon, 26 Feb 2024, Luiz Capitulino wrote:
> > > > 
> > > > > On 2024-02-26 08:27, Ilpo Järvinen wrote:
> > > > > > On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:
> > > > > > 
> > > > > > > The mlxbf-pmc driver fails to load when the firmware reports a new
> > > > > > > but
> > > > > > > not
> > > > > > > yet implemented performance block. I can reproduce this today with
> > > > > > > a
> > > > > > > Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035,
> > > > > > > since
> > > > > > > this
> > > > > > > reports the new clock_measure performance block.
> > > > > > > 
> > > > > > > This[1] patch from Shravan implements the clock_measure support
> > > > > > > and
> > > > > > > will
> > > > > > > solve the issue. But this series avoids the situation by ignoring
> > > > > > > and
> > > > > > > logging unsupported performance blocks.
> > > > > > > 
> > > > > > > [...]
> > > > > > 
> > > > > > 
> > > > > > Thank you for your contribution, it has been applied to my local
> > > > > > review-ilpo branch. Note it will show up in the public
> > > > > > platform-drivers-x86/review-ilpo branch only once I've pushed my
> > > > > > local branch there, which might take a while.
> > > > > 
> > > > > Thank you Ilpo and thanks Hans for the review.
> > > > > 
> > > > > The only detail is that we probably want this merged for 6.8 since
> > > > > the driver doesn't currently load with the configuration mentioned
> > > > > above.
> > > > 
> > > > Oh, sorry, I missed the mention in the coverletter.
> > > > 
> > > > So you'd want I drop these from review-ilpo branch as there they end
> > > > up into for-next branch, and they should go through Hans instead who
> > > > handles fixes branch for this cycle?
> > > 
> > > If that's the path to get this series merged for this cycle then yes,
> > > but let's see if Hans agrees (sorry that I didn't know this before
> > > posting).
> > > 
> > > One additional detail is that this series is on top of linux-next, which
> > > has two additional mlxbf-pmc changes:
> > > 
> > > *
> > > https://lore.kernel.org/lkml/
39be055af3506ce6f843d11e45d71620f2a96e26.1707808180.git.shravankr@nvidia.com/
> > > *
> > > https://lore.kernel.org/lkml/d8548c70339a29258a906b2b518e5c48f669795c.1707808180.git.shravankr@nvidia.com/
> > > 
> > > Maybe those two should be included for 6.8 as well?
> > 
> > Those look a new feature to me so they belong to for-next. So no, they
> > will not end up into 6.8 (to fixes branch). If the 2 patches in this
> > series do not apply without some for-next targetting dependencies, you
> > should rebase on top of fixes branch and send a new version.
> 
> Understood.
> 
> > About those two patches, please also see my reply. I intentionally only 2
> > patches of that series because I wanted to see sysfs documentation first
> > so you should resend those two patches to for-next with sysfs
> > documentation.
> 
> I'm actually not author of the other patches :)

Ah, sorry. I didn't pay enough attention to that. :-)


-- 
 i.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading
  2024-02-27 13:18           ` Ilpo Järvinen
@ 2024-02-27 18:28             ` Luiz Capitulino
  0 siblings, 0 replies; 16+ messages in thread
From: Luiz Capitulino @ 2024-02-27 18:28 UTC (permalink / raw)
  To: Ilpo Järvinen, Hans de Goede
  Cc: shravankr, davthompson, ndalvi, LKML, platform-driver-x86

On 2024-02-27 08:18, Ilpo Järvinen wrote:
> On Mon, 26 Feb 2024, Hans de Goede wrote:
> 
>> Hi Luiz,
>>
>> On 2/26/24 17:10, Luiz Capitulino wrote:
>>> On 2024-02-26 11:04, Ilpo Järvinen wrote:
>>>> On Mon, 26 Feb 2024, Luiz Capitulino wrote:
>>>>
>>>>> On 2024-02-26 08:27, Ilpo Järvinen wrote:
>>>>>> On Thu, 22 Feb 2024 15:57:28 -0500, Luiz Capitulino wrote:
>>>>>>
>>>>>>> The mlxbf-pmc driver fails to load when the firmware reports a new but not
>>>>>>> yet implemented performance block. I can reproduce this today with a
>>>>>>> Bluefield-3 card and UEFI version 4.6.0-18-g7d063bb-BId13035, since this
>>>>>>> reports the new clock_measure performance block.
>>>>>>>
>>>>>>> This[1] patch from Shravan implements the clock_measure support and will
>>>>>>> solve the issue. But this series avoids the situation by ignoring and
>>>>>>> logging unsupported performance blocks.
>>>>>>>
>>>>>>> [...]
>>>>>>
>>>>>>
>>>>>> Thank you for your contribution, it has been applied to my local
>>>>>> review-ilpo branch. Note it will show up in the public
>>>>>> platform-drivers-x86/review-ilpo branch only once I've pushed my
>>>>>> local branch there, which might take a while.
>>>>>
>>>>> Thank you Ilpo and thanks Hans for the review.
>>>>>
>>>>> The only detail is that we probably want this merged for 6.8 since
>>>>> the driver doesn't currently load with the configuration mentioned above.
>>>>
>>>> Oh, sorry, I missed the mention in the coverletter.
>>>>
>>>> So you'd want I drop these from review-ilpo branch as there they end
>>>> up into for-next branch, and they should go through Hans instead who
>>>> handles fixes branch for this cycle?
>>>
>>> If that's the path to get this series merged for this cycle then yes,
>>> but let's see if Hans agrees (sorry that I didn't know this before
>>> posting).
>>
>> Hmm, new hw enablement typically goes through -next and not to
>> the current fixes branch. And AFAICT this is new hw enablement,
>> not a regression / bug-fix.
>>
>> Is there any special reason why this needs to be in 6.8 ?
> 
> To me it sounded like fix to 1a218d312e65 ("platform/mellanox: mlxbf-pmc:
> Add Mellanox BlueField PMC driver") and 423c3361855c ("platform/mellanox:
> mlxbf-pmc: Add support for BlueField-3") although not explicitly marked as
> such.
> 
> But I'm fine with taking these through for-next, it's relatively late into
> the cycle already anyway.
> 
>> For RHEL kernels you can cherry-pick patches from -next
>> as necessary.
> 
> It's also possible to send them later directly to stable folks once
> Linus' tree has them after the next merge window if you feel they're
> useful for stable inclusion.

Fair enough. Let's proceed with the original plan of having them merged
in the for-next branch. Sorry for the noise this discussion may have
caused.

- Luiz

> 
>>> One additional detail is that this series is on top of linux-next, which
>>> has two additional mlxbf-pmc changes:
>>>
>>> * https://lore.kernel.org/lkml/39be055af3506ce6f843d11e45d71620f2a96e26.1707808180.git.shravankr@nvidia.com/
>>> * https://lore.kernel.org/lkml/d8548c70339a29258a906b2b518e5c48f669795c.1707808180.git.shravankr@nvidia.com/
>>
>> Hmm, those are not small patches, any other reason
>> why this really should go to -next IMHO.
> 
> Those two linked patches are totally unrelated.
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-02-27 18:28 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-22 20:57 [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading Luiz Capitulino
2024-02-22 20:57 ` [PATCH 1/2] platform/mellanox: mlxbf-pmc: mlxbf_pmc_event_list(): make size ptr optional Luiz Capitulino
2024-02-26 10:49   ` Hans de Goede
2024-02-22 20:57 ` [PATCH 2/2] platform/mellanox: mlxbf-pmc: Ignore unsupported performance blocks Luiz Capitulino
2024-02-26 10:49   ` Hans de Goede
2024-02-26 13:27 ` [PATCH 0/2] platform/mellanox: mlxbf-pmc: Fix module loading Ilpo Järvinen
2024-02-26 15:49   ` Luiz Capitulino
2024-02-26 16:04     ` Ilpo Järvinen
2024-02-26 16:10       ` Luiz Capitulino
2024-02-26 16:57         ` Hans de Goede
2024-02-26 17:05           ` Luiz Capitulino
2024-02-27 13:18           ` Ilpo Järvinen
2024-02-27 18:28             ` Luiz Capitulino
2024-02-26 16:59         ` Ilpo Järvinen
2024-02-26 17:07           ` Luiz Capitulino
2024-02-27 13:20             ` Ilpo Järvinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.