From patchwork Wed Sep 27 08:26:07 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matti Vaittinen X-Patchwork-Id: 13400358 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C9C86E810A2 for ; Wed, 27 Sep 2023 08:26:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230248AbjI0I05 (ORCPT ); Wed, 27 Sep 2023 04:26:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47828 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230380AbjI0I01 (ORCPT ); Wed, 27 Sep 2023 04:26:27 -0400 Received: from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com [IPv6:2a00:1450:4864:20::12b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0A732CE9; Wed, 27 Sep 2023 01:26:23 -0700 (PDT) Received: by mail-lf1-x12b.google.com with SMTP id 2adb3069b0e04-503397ee920so16761852e87.1; Wed, 27 Sep 2023 01:26:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695803182; x=1696407982; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=3M+fhkSU75ZwJTVy/9LEZES58n3Sr/w+Y0+8L0nomxI=; b=S6bnlg6iw3CoIgvx97RpA9I+w/dZH1ssIGt+8PXtX2ZW1ICm+Tcuh2HwsUN7rPaAWq a9DF0IFLKSw1QzovG0nPPrExbNyyqc4POBE+lZznDUIqb5HzHN6OstWvg7lfOp1EAuAT cc2xXpxvVv5E8sOGM9T/y/nepfh0lud54ANJwqe3ZN7KIVFpZXmQdwlKQq5EZrlkQv4c Mfe50mhAk58KE14xYi/gH1y8F+EY3H2dIYcZnC+xbgcorO9IDqouYinD9dXTLrXtg6Ce r//3j1lnMEXVDyAwisflX3gLMzLM2n4/0r/NC0kuenDmq70MHYbi+ZFHjptMb1G7yNQb kyOA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695803182; x=1696407982; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=3M+fhkSU75ZwJTVy/9LEZES58n3Sr/w+Y0+8L0nomxI=; b=MWQ0GEcHmkY+kdQ2UmT65ZkiaTpzPqOn1Z6e9Kec1srAWOgww6G6MiwZLOKinDqvGk 75uE7PnD6qzNQUH6SGdUcjkZbGxFdYEWoc13w4ZMUoOPf5StBf4Qnq1aAmimt+ATWzqu U9KY3xAXrC8ZrSpcqDyC3AwYH6IYAdlV9DPLj70jQ1A0KPC1NkA0ci0hAaKEf5cpv8hS kL5xz/sGuWqGKfjTlhHYqZ+WZAPuY/Boqp2UVe4a0rbVMPXzSXfqx5iDQjiB3k2kpzEq 4BPJy/VlogUdA3ZmNiMiuL4jJeKzuYUX7fBK1HInrUW9RH+7FVxfXCgvr+x006xkix3E SIgg== X-Gm-Message-State: AOJu0Yyevn2YS4lSq5rEj8Kh2G15OPuxzS7cTH5HjbiRB9VAfJjx9GAe M4qWfxuUySyLs9UHQuwBii/ngh/dJT4= X-Google-Smtp-Source: AGHT+IEvTKArd+9rhJz1P3ux20jG6esY8uIeRZvvlL70yqMnj+s8j9q6aFlo4F34eBNu31UMcyyttQ== X-Received: by 2002:a05:6512:11c6:b0:4fa:5e76:7ad4 with SMTP id h6-20020a05651211c600b004fa5e767ad4mr983405lfr.10.1695803181724; Wed, 27 Sep 2023 01:26:21 -0700 (PDT) Received: from fedora ([213.255.186.46]) by smtp.gmail.com with ESMTPSA id h18-20020a197012000000b0050309ea3a62sm2479646lfc.277.2023.09.27.01.26.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 27 Sep 2023 01:26:20 -0700 (PDT) Date: Wed, 27 Sep 2023 11:26:07 +0300 From: Matti Vaittinen To: Matti Vaittinen , Matti Vaittinen Cc: Jonathan Cameron , Lars-Peter Clausen , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Matti Vaittinen , Andy Shevchenko , Angel Iglesias , Andreas Klinger , Benjamin Bara , Christophe JAILLET , linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 1/5] tools: iio: iio_generic_buffer ensure alignment Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-iio@vger.kernel.org The iio_generic_buffer can return garbage values when the total size of scan data is not a multiple of the largest element in the scan. This can be demonstrated by reading a scan, consisting, for example of one 4-byte and one 2-byte element, where the 4-byte element is first in the buffer. The IIO generic buffer code does not take into account the last two padding bytes that are needed to ensure that the 4-byte data for next scan is correctly aligned. Add the padding bytes required to align the next sample with the scan size. Signed-off-by: Matti Vaittinen --- I think the whole alignment code could be revised here, but I am unsure what kind of alignment is expected, and if it actually depends on the architecture. Anyways, I'll quote myself from another mail to explain how this patch handles things: > For non power of2 sizes, the alignment code will result strange alignments. > For example, scan consisting of two 6-byte elements would be packed - > meaning the second element would probably break the alignment rules by > starting from address '6'. I think that on most architectures the proper > access would require 2 padding bytes to be added at the end of the first > sample. Current code wouldn't do that. > If we allow only power of 2 sizes - I would expect a scan consisting of a > 8 byte element followed by a 16 byte element to be tightly packed. I'd > assume that for the 16 byte data, it'd be enough to ensure 8 byte alignment. > Current code would however add 8 bytes of padding at the end of the first > 8 byte element to make the 16 byte scan element to be aligned at 16 byte > address. To my uneducated mind this is not needed - but maybe I just don't > know what I am writing about :) Revision history v3 => v4: - drop extra print and TODO coment - add comment clarifying alignment sizes --- tools/iio/iio_generic_buffer.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c index 44bbf80f0cfd..c07c49397b19 100644 --- a/tools/iio/iio_generic_buffer.c +++ b/tools/iio/iio_generic_buffer.c @@ -54,9 +54,12 @@ enum autochan { static unsigned int size_from_channelarray(struct iio_channel_info *channels, int num_channels) { unsigned int bytes = 0; - int i = 0; + int i = 0, max = 0; + unsigned int misalignment; while (i < num_channels) { + if (channels[i].bytes > max) + max = channels[i].bytes; if (bytes % channels[i].bytes == 0) channels[i].location = bytes; else @@ -66,6 +69,19 @@ static unsigned int size_from_channelarray(struct iio_channel_info *channels, in bytes = channels[i].location + channels[i].bytes; i++; } + /* + * We wan't the data in next sample to also be properly aligned so + * we'll add padding at the end if needed. + * + * Please note, this code does ensure alignment to maximum channel + * size. It works only as long as the channel sizes are 1, 2, 4 or 8 + * bytes. Also, on 32 bit platforms it might be enough to align also + * the 8 byte elements to 4 byte boundary - which this code is not + * doing. + */ + misalignment = bytes % max; + if (misalignment) + bytes += max - misalignment; return bytes; }