From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EC6CC21A431 for ; Fri, 20 Dec 2024 19:39:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734723564; cv=none; b=BmlEwX6zcy13VJyMSh0yNaeebk0+slBonAVzzzBGgzUmY7oP33+TMU2/UlGKvzJlAb5SduGB9sFHm7fN7W5ANIIzvRiuyTGlzHgtkuGg8FnkXHyCpQmDdDUFIRVPW52PnueP4BaNFvRyKxH19bCYmY48PRx3pT8zlMvmf8qhwFc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734723564; c=relaxed/simple; bh=WaTkqQd0gCJx5+ld/Hgl+wnVFSH38qph2AHIt3Pou80=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=C+ZyLx2j8S3n7Mfa8I5J+yfQ0zGz+JfDIzVv6vA+wswH5aIT4YlAHuRIkJ3ZdCQ/2Xwfk9C/ScSg2pQyTJGSCBM9kqok7uzBkIoqL0QUXLpSgDsea8TLHZRarHWIJTzsJ5pesIafJYj9Ld1/IC/vdgcrQf7lS0Bb/JuXmDYldAE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=ItoCk1AG; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="ItoCk1AG" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1734723562; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UvMoGPZXA/JCbWJzTQp5VTX9HgW6KoGFQy+Y1R5Z8oU=; b=ItoCk1AGLe/lcF8XgTUCKhJRa9gdw6BkfA3a7A2D5SBc94KiOv0zlFCYJIUxxv/RWXPBHl cRWODgEJuLsMiGYXKyHz7dju1VwwrYvfc3jEYoyT5UREAM5D7ZYlTeVrf9DsBOyndZ4xhv yLDo/xnF4oa3r19QbyTdDI+8Uo4+HkQ= Received: from mail-yb1-f198.google.com (mail-yb1-f198.google.com [209.85.219.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-671-slw04N7IN2qxPgLyN2TPaQ-1; Fri, 20 Dec 2024 14:39:20 -0500 X-MC-Unique: slw04N7IN2qxPgLyN2TPaQ-1 X-Mimecast-MFC-AGG-ID: slw04N7IN2qxPgLyN2TPaQ Received: by mail-yb1-f198.google.com with SMTP id 3f1490d57ef6-e3a0d165daeso2914485276.1 for ; Fri, 20 Dec 2024 11:39:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734723560; x=1735328360; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=UvMoGPZXA/JCbWJzTQp5VTX9HgW6KoGFQy+Y1R5Z8oU=; b=pbnc5GYGCBqPnGyWtpgxHeAgU88rg4XWrOGbc9/0HcxksZWsk7vSkGnJnjVRwJ2Rcx hj6aC1AkPsbViUuwA/GNHv+EPXnltNxaw8njBBhMU2iIZNG1Fpa6X68YS6/h2tpo1ICY 03t23wfs8U4Ia8Xm8ZY3zF5KyBNMedKRamaztb2vCpqhDw+ncFl+KWMabZ6mCa+rTo2m rNuH+6J7k2c8kDGAwkP710nkoFvPx+msU7K402G59gWk5IoQJgNWo+Qt5clyOT0KGaNi 2cMgsHVQC64w5mw/fEdSpjGh4l0Rn05gRuHy6JEVH46LHhxIc0pHIeM5zKqNJSPdUMzJ IHrw== X-Gm-Message-State: AOJu0Yz8VfNTeVh+D962sPCuugrx2eGtRBNrVLMB7F04Y1Q9VHRL/D3k Y4ZEhfBeRA/uxwqX9GsCSmoXutqSTiu3QnBpTudALBb28+N+ca7/mRxb2Y79PvskT+OipbHDXKL 6gmeTG2FACKV1UnXqkz0+nmpj16Q/eLdCQ+UdRA1QJBN3l+oqG7elzmb/MNaWjg== X-Gm-Gg: ASbGnctjqchn29YWySeQFWyUQM1rUSGnmOzE9HQlX9aDgR1ka8uMYnTkh7IaAyiUX3w G/ha3/fZ/ErDqTUCj3YosPy8uWs9TS0gO3+6wIhRSWSBkfl7Z2XcmO2PR0cUIkNhrV03Jj7IjK+ EuHM6/k1l3SgCW5jTpITaB552cxK0pEVN4JXm8Q1M1Ffe6Ivju71g7lfg/g7fJqcuV7zKZH5viB NXZ7JgCS8jmfb4HjlBy7w9i0Bp4tteuL+rWOSmgBqjmaMdBKj7rbSmM98+4eUWhbAmDMUErY1IF BS+RT10xpk3rLmdC4v6R9MWFkMd/zoUz/Q8J++ZI0Kfd+cOrLwCFK4hHz7R+Og== X-Received: by 2002:a25:1e85:0:b0:e39:7347:6d76 with SMTP id 3f1490d57ef6-e538c256b59mr2918258276.19.1734723560235; Fri, 20 Dec 2024 11:39:20 -0800 (PST) X-Google-Smtp-Source: AGHT+IHL5XtiuXrohZaf8df4Lv1IXHirXfqxMzPVe9I/pESLg1R0ZgUBtt3WSyl6Q3KDh7GtORHhcw== X-Received: by 2002:a25:1e85:0:b0:e39:7347:6d76 with SMTP id 3f1490d57ef6-e538c256b59mr2918243276.19.1734723559943; Fri, 20 Dec 2024 11:39:19 -0800 (PST) Received: from ?IPV6:2603:9001:3d00:5353::14c1? (2603-9001-3d00-5353-0000-0000-0000-14c1.inf6.spectrum.com. [2603:9001:3d00:5353::14c1]) by smtp.gmail.com with ESMTPSA id 3f1490d57ef6-e537cf46c9fsm959308276.40.2024.12.20.11.39.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 20 Dec 2024 11:39:19 -0800 (PST) Message-ID: <1ace6102-2e88-499a-a5fd-71951863b987@redhat.com> Date: Fri, 20 Dec 2024 14:39:18 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 2/3] nvmem: core: add nvmem_cell_write_variable_u32() To: Srinivas Kandagatla , Sebastian Reichel , Greg Kroah-Hartman , Maxime Ripard Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org References: <20241104152312.3813601-1-jberring@redhat.com> <20241104152312.3813601-3-jberring@redhat.com> <488ea6c2-0832-4409-902b-2b6b193daf8c@linaro.org> Content-Language: en-US From: Jennifer Berringer In-Reply-To: <488ea6c2-0832-4409-902b-2b6b193daf8c@linaro.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Thanks for the feedback and for accepting my first patch. On 12/14/24 10:07, Srinivas Kandagatla wrote: >> + * nvmem_cell_write_variable_u32() - Write up to 32-bits of data as a host-endian number > >> + * >> + * @cell: nvmem cell to be written. >> + * @val: Value to be written which may be truncated. >> + * >> + * Return: length of bytes written or negative on failure. >> + */ >> +int nvmem_cell_write_variable_u32(struct nvmem_cell *cell, u32 val) > > This new API is confusing, as writing to cell should in the same order of the data. Doing any data manipulation within the api is confusing to users. > > If the intention is to know the size of cell before writing, then best way to address this is to provide the cell size visibility to the consumer. > > --srini My intention was to mirror the existing functions in this file, hoping that would make it less confusing rather than more. nvmem_cell_read_variable_le_u32() already similarly has consumers accessing the contents of a cell without knowing its size, silently filling any bytes not read with zero. The function I add doesn't change the order of the bytes. The existing read_variable_le functions (in contrast) byte swap from little-endian to the CPU's endianness and indicate this in the function name. Otherwise, I believe the function I add here is what would be expected from a write equivalent. Its return value also gives the caller the actual cell size upon success. The only manipulation done to the data here is truncating to ignore the highest-order bytes. This behavior should match the below example unless the size is 3 bytes, which my patch should handle but the below example can't. if (entry->bytes == sizeof(u32)) { return __nvmem_cell_entry_write(entry, &val, sizeof(u32)); } else if (entry->bytes == sizeof(u16)) { u16 val_16 = (u16) val; return __nvmem_cell_entry_write(entry, &val_16, sizeof(u16)); } else if (entry->bytes == sizeof(u8)) { u8 val_8 = (u8) val; return __nvmem_cell_entry_write(entry, &val_8, sizeof(u8)); } else { return -EINVAL; } Still, if you prefer, I can send new patches that add a function to get the cell size and put any truncation behavior needed by nvmem-reboot-mode into that source file instead.