Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <10276e15-8528-41a9-4b90-7fdf6b890206@redhat.com>
Date:   Mon, 14 Aug 2023 10:41:27 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH] hp-bioscfg: Update steps how order list elements are
 evaluated
Content-Language: en-US, nl
To:     Jorge Lopez <jorgealtxwork@gmail.com>,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org,
        thomas@t-8ch.de, ilpo.jarvinen@linux.intel.com,
        dan.carpenter@linaro.org
References: <20230809210740.18392-1-jorge.lopez2@hp.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <20230809210740.18392-1-jorge.lopez2@hp.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Jorge,

On 8/9/23 23:07, Jorge Lopez wrote:
> Update steps how order list elements data and elements size are
> evaluated
> 
> Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
> 
> ---
> Based on the latest platform-drivers-x86.git/for-next
> ---
>  .../x86/hp/hp-bioscfg/order-list-attributes.c    | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> index b19644ed12e0..d2b61ab950d4 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> @@ -152,7 +152,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
>  
>  		switch (order_obj[elem].type) {
>  		case ACPI_TYPE_STRING:
> -			if (elem != PREREQUISITES && elem != ORD_LIST_ELEMENTS) {
> +			if (elem != PREREQUISITES) {
>  				ret = hp_convert_hexstr_to_str(order_obj[elem].string.pointer,
>  							       order_obj[elem].string.length,
>  							       &str_value, &value_len);
> @@ -266,6 +266,15 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
>  			if (ret)
>  				goto exit_list;
>  
> +			/*
> +			 * It is expected for the element size value
> +			 * to be 1 and not to represent the actual
> +			 * number of elements stored in comma
> +			 * separated format. element size value is
> +			 * recalculated to report the correct number
> +			 * of data elements found.
> +			 */
> +
>  			part_tmp = tmpstr;
>  			part = strsep(&part_tmp, COMMA_SEP);
>  			if (!part)
> @@ -273,11 +282,14 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
>  					tmpstr,
>  					sizeof(ordered_list_data->elements[0]));
>  
> -			for (olist_elem = 1; olist_elem < MAX_ELEMENTS_SIZE && part; olist_elem++) {
> +			for (olist_elem = 0; olist_elem < MAX_ELEMENTS_SIZE && part; olist_elem++) {
>  				strscpy(ordered_list_data->elements[olist_elem],
>  					part,
>  					sizeof(ordered_list_data->elements[olist_elem]));
> +
>  				part = strsep(&part_tmp, COMMA_SEP);
> +				if (part && ordered_list_data->elements_size < MAX_ELEMENTS_SIZE)
> +					ordered_list_data->elements_size++;
>  			}

I believe that you can replace the:

				if (part && ordered_list_data->elements_size < MAX_ELEMENTS_SIZE)
					ordered_list_data->elements_size++;
			}

Lines with simply (after the loop has finished) doing:

			}
			ordered_list_data->elements_size = olist_elem'

Or am I missing something ?

Regards,

Hans