From mboxrd@z Thu Jan 1 00:00:00 1970 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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E0F5FC433EF for ; Thu, 5 May 2022 15:27:06 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 786C383B39; Thu, 5 May 2022 17:27:03 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=reject dis=none) header.from=seco.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=seco.com header.i=@seco.com header.b="amtIZ05g"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id C1A8B83AE2; Thu, 5 May 2022 17:27:01 +0200 (CEST) Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-ve1eur03on0614.outbound.protection.outlook.com [IPv6:2a01:111:f400:fe09::614]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 59A8483B39 for ; Thu, 5 May 2022 17:26:58 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=reject dis=none) header.from=seco.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sean.anderson@seco.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SFXT7cA75w1EUOJxhhHdrnZKw6BoLMeCIcJEhP/Mi0ohN8u49+De/J0JuNN689LGbEvfVUv8WLb8CuPe05peIR7DCV/spXnjGaDvw6i5QxIdY0xAz4qEfSiBPaf/OYT8DZLalyLPkpCAToW/uVKTrJnSV6sFV76k/5Jo26aDdZg/DpwM2D0Ypyr0CLTpIe2WgKvm8mRz5/8/Z2vOnfPBxcQFUE++L5pHdIjEUrGfmhSbKISbPjQSUKLMwdzsgd6EMc4O+rJApc1BOGtQuebwm3Wdz0xCcCCycWSXoAS+0V+1J5S95fuksMuO+wDwOQAH3vARhRet1aV/lh9S7LsFUQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=AnT3iWR8tT14ahePZBRj+tLJogcrQlTz5hlsKIadVxo=; b=BFPC+YXm9LO6kP5jYD2kDHAm9ToBQfCfrHGWP9yMgAKyZq6/v/fW+fRdQ6FlB8f5vTApgwTq0ZJbpPxevT44BQ8ZivdF40qbNmt6Q5xDvqWmmKw97HVjvJbYZshgMp0ixRQQGnOGCQXlGIsrc14VcGONhb8SmIQ6Ya1lgWi79kmNPN2IjHjvgU6BTTZWBcGkWsaI4Fy8efv9zBVHMclhMbZfoOwRI7nGfM1iSl6NUYrleVVn7GHCKXLTTwnG1EQA9xr/h3vJKZddy4yI0JUUWaFM70QD8JMvteUZM0cC12o1Sd/rDrT+SUb5jO5zkgOn4bmL2c6OQmWL7hLBKDpLNQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=seco.com; dmarc=pass action=none header.from=seco.com; dkim=pass header.d=seco.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=seco.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=AnT3iWR8tT14ahePZBRj+tLJogcrQlTz5hlsKIadVxo=; b=amtIZ05g9ER4CgUxTn2CK84LI70PePV9htDMgIgglmpyhpPMI4JcACYtOH/UEgX+pUCiORhybTjIigg2JfoBYtneQyosLbW9YrLn5I/QPKuUL4sEptogFoQixPVrlZKzwRtC20XDXT1rNE9BVIpTKKX950FoWnY4W5NurRByM0SlwbClAfmqLa4zms/Vl6/FG4/3UwWjra+TGnvEOErSiYeFRLYNSAUB7rj1yXsOlgF8mh/wXKplXZ0y0Y/rwcf+mDF7UpcchdLD+B7pXYJHUwkR6ZCoGKolS+3q7SepplgzckeZAwz5CdPoCxNDjBDkXMQMaJSKxiTy3ZJOrrAtsQ== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=seco.com; Received: from DB7PR03MB4972.eurprd03.prod.outlook.com (2603:10a6:10:7d::22) by VI1PR03MB3806.eurprd03.prod.outlook.com (2603:10a6:803:6c::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5206.25; Thu, 5 May 2022 15:26:56 +0000 Received: from DB7PR03MB4972.eurprd03.prod.outlook.com ([fe80::a9d3:8abd:3f5e:a0c]) by DB7PR03MB4972.eurprd03.prod.outlook.com ([fe80::a9d3:8abd:3f5e:a0c%5]) with mapi id 15.20.5206.025; Thu, 5 May 2022 15:26:56 +0000 Subject: Re: [PATCH v3 08/13] misc: Add support for nvmem cells To: Simon Glass Cc: U-Boot Mailing List , Mario Six , Ramon Fried , Heinrich Schuchardt , Tom Rini , Joe Hershberger References: <20220418193659.3677824-1-sean.anderson@seco.com> <20220418193659.3677824-9-sean.anderson@seco.com> <32a295f1-f2c8-080e-b3f6-30f0dee6e25d@seco.com> <86ab39e4-f463-418e-a337-3173d019f624@seco.com> From: Sean Anderson Message-ID: <9da29f16-714e-77f6-829d-96a1d32a06d4@seco.com> Date: Thu, 5 May 2022 11:26:51 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-ClientProxiedBy: BL1PR13CA0181.namprd13.prod.outlook.com (2603:10b6:208:2be::6) To DB7PR03MB4972.eurprd03.prod.outlook.com (2603:10a6:10:7d::22) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 6984c69f-9d46-408d-a72e-08da2eabb051 X-MS-TrafficTypeDiagnostic: VI1PR03MB3806:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: qCLpHsxMKdBAGWBK8isOqjr2zh8eheVOiQB4ab5RwOm3WeMXohe9F6ZeA2Peox9FBdYfh5XfovqMwUM+9V+i7mEhX3b+5vY/wFBlyDwu404+fCaW02uAPZmVaR/yp6U4yyXl/WKfB+w9rR+uIhLh4tzFoRVvDYZU2F4LjPU+ttY3um5LC/J5G1jekgXd+6zwAdcaCrUg0WnsnhFnAjc6I8sacDFPUhx3sDLZjcM1tdUxAWwr+fT3a6kqvVhmnM+eMpDY0aROhTJeUKR9E8LZOBLi+fxPPVg33xWMPkYK+5SjDFqO1GT+g1vWGM3Sobgo2LA9LkBcpWTiqnPEPWLnGdR75WvfB4jOeoDIxmXz57iJP+R+IIkzXejEGg4ioqkoOXWOk7jAwodPuNrPqvayY6i36/3/I/ntPSTIPdb+fcEWkjoBp2u2v1qm44FbFeWKSymDdRsnRV0tj4MqcvRKoO4H0HfoUMJQPTXc7AScDzLcSkXOlrn+eicc8v2qpYUgA8tZQHuS/1ahUhlBvw/naQ18j4VDPthjkoLYNAoTp/QDpJ3ZT6O+0Hs/1Ow2As0BBasTStno4mvaP9b23SSUADciInhEMIoHrZj80Ye/+pZ05sCw7pqyoo7XQXvKi4mHMPr7ilYUdyKHuuUVbinyf2SBNzBBTO5vLHwTTEIsJES7D9qxE/H89jN3BO2x3QCfsEJGu7+7iZXqwDVQkUlzo+kSwhGGJYp2I9KC9Br2rNmI2OTscvTgrPtE1+ZMbpRcd9cXpwO0hWg+T8MYS3AWWg== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DB7PR03MB4972.eurprd03.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(366004)(53546011)(6506007)(30864003)(54906003)(26005)(31696002)(86362001)(8676002)(4326008)(8936002)(52116002)(44832011)(508600001)(316002)(6512007)(6666004)(6486002)(6916009)(2906002)(31686004)(36756003)(186003)(5660300002)(66476007)(83380400001)(66946007)(66556008)(38350700002)(38100700002)(2616005)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bVRteHR1ODFSeFRqK005d01OQ01ERnNxemZZeE9XZWs2ZklzMkFPUm5DTnla?= =?utf-8?B?TUczdm94SnBqYUJIYmlzTTN5KyttckhnaVNGaGJ4eU9iclFQVGFVQzV2dDdI?= =?utf-8?B?MnJmYndFRGVrVG8vYWpZTDduWU52VUZZTVAzNFEySTJ2a3Q0bWRXZXdQbG5S?= =?utf-8?B?N1VFQ0VWeGF0NWxJRFkwSXdnNlUzYmxLWWV5NFhadDBweXZZSE14NTEweUFX?= =?utf-8?B?WTZIcFNEbjhGWUlseHFxaEwyc1ArNnZkYXVYS1I1dnhJUXBVdW9zaE51WnVS?= =?utf-8?B?aGI3Mm41anNmT0poU3RzdktXT3NndFExdjU5dU9BcTZET3h1Mi93L2plcnNE?= =?utf-8?B?MUxSTHdiZWpkZjdrbnNDMW8xcFE1M3dKVFJVMFQxNnFWZjkza0VhWis3bVVz?= =?utf-8?B?VzBJRlY4bXdmK3UwQmZzVEQ0dFUyR0JJMDdaZnhSdmVCYkQ5QmM1U0tXRlpD?= =?utf-8?B?UmE4YTREYkNERGxOWWhEVzl5TDllbUlXWE54LzNsU0pCZWg5ZVFnU3NqTjh0?= =?utf-8?B?V3NDb0hTWDlHclh5dUVNNXVmVlJia2tCUGN6d3p5bExkb2NrNEpJSWNFendk?= =?utf-8?B?NERybHV5MUZ0L3ErejFYbEpBRUk1TTJaSjMyLzg2Q0ppNThZNDlqcmpMM0F3?= =?utf-8?B?dldwOXhyUHZKOWE0UmFqaUNndE4zVjRxSGdFcVVadnNhNjVoVVB4enBodmNF?= =?utf-8?B?WnlNeDFnSXQ5RDlLcEFqeGRCdXQyTmZaNEhza2NVS3FFTmE1UlJ5dTNpOVNu?= =?utf-8?B?NTZGL0lUUmRuSkhiME8xbStJVGtjd2RjVEFYdWdqUEpRUXdHNU1zRnlZSDZD?= =?utf-8?B?OWd1ek53cDNtR253M2hOT2g1QldsMld0V3orQXR3dk5zTHExN3h3RHp5UERR?= =?utf-8?B?Vk1ucUVURUtiTTRvTGx0blZPZCtuTDNPQlhna0VqNGZaQlRySTJUSUJzMnlR?= =?utf-8?B?UUU3eXdmc284K2w3dWNvTTA1YVRZcWtJcDFkZGczeUZKdDY3SU41MUw5OVJY?= =?utf-8?B?cjN1UCtIWFJLVWNiNzZINmxKcUFlYTFyUjlnT0EwNFFsZ2FPSFRSa2EyRGFr?= =?utf-8?B?ZjVjL0g2NWQyTE5iaExPeFp3dVBUUENsQ21uOWxvbDgwS1FXbERwUEFTaU4w?= =?utf-8?B?NEtaVmhDSUpOOTh5ZFNralhBcklQN09CNEN1TjFjTmoxVUJBQytnbnQ1OHZT?= =?utf-8?B?Z1BLKzZDd0FXc2RIR05WdVBTMHpQZkFNanYxbVBuZFptZnZTU3l0KzQ1UjY1?= =?utf-8?B?U2djdm5xRmxnN1NhSmFLdjlZLzg4ZExNRjM3UVlkOTBHcXgvdHJhTGFIZm1m?= =?utf-8?B?QUNKSzVMSjNidmt4RGVPSGcyOXZoRHVPbk56SE5SbEVTbmFHeUNFTG1lYUNq?= =?utf-8?B?NE45Rlowc0lORis1dUFwMlpvY3p6Q04yVFpzREpSOFNLbWNmT3B1WCtmb2w5?= =?utf-8?B?VUhHTThzZ0YrSVRqOVR5ejZ4RlE4RXY1STBBczNwZVYySXp4aWF5dldqNktU?= =?utf-8?B?TDZTZEdvZUxybFk0UEUxQkNDUk91VzZ0bi9CZUxsSDNlMGp0TXROaTBUYzBs?= =?utf-8?B?VzdrTmtvU01va0pWSjRobUtJblp3ZGFVQzYyV1hVUFdjT3NrbWFMY0JoSjUw?= =?utf-8?B?OHJsbE4veGt2K1VGSDQ1c00yQUZpY1BTVWRNNWcyRVliR05ud0M4aXlDZGc1?= =?utf-8?B?Ritwemc0TnZ0U0RhMlRTc0VSc0QxcFRIeUJMdElnWElYdjVVRVhEaURRSSt5?= =?utf-8?B?a0cwTVN6WmhjWWg0aUtpRDdSdGNZRXp2dGZEQU5WMzY1UnI3YnBpeFVXQUFW?= =?utf-8?B?QzdLVDJBZjNKS3pNOTFiL2ZWQ2Z2ZDV2OXFBdHR1Zy9FaXQzSWhDZ1VEMjRB?= =?utf-8?B?MStaeUtjcVlRblRjb2RrcEl4WlppaWJxS2c0dU03Y0o1blE5eUVGL3ZmMU5q?= =?utf-8?B?dkJ2YkRKR1RmYzRwOWQvNjRXVzQrLzMzSGRYdGRXeXB6NGVKcDdqSnYySVBh?= =?utf-8?B?RG1zUEZoaHE4c21obmxNNlJDdm9CUWI2VVpvVElmUmpPcmJ4aG5JRDJBR0VP?= =?utf-8?B?UFlLUzhrSDl5Um1xYWQ5VWFMbzA4WnRPMHhYRkVKbTUvN0ovYkJvL1k4Q01F?= =?utf-8?B?WGQ3UXVkRHo0M0FNc2ZCZllHMDlIWE84eGRHMFliTFY0K1V6V01jVzNFbGkv?= =?utf-8?B?SUNRbE5RSzltM3Q0aWdjZXVwSTU4UmprQXVrb3Nyd1E2dDZ4cHEydVJvYlpK?= =?utf-8?B?OWZTVzBOckNjQkY3cWJCQzZzdktpOEFIUkpLam1TSzdsQlMrZGF6aDZjR042?= =?utf-8?B?MkxYY29XVWRGQWRnMU1YOXh4dUV1ajFITGNUU0pZenlaeVBEKzJTMElkVk0v?= =?utf-8?Q?IDcRrePQYXkDmNtI=3D?= X-OriginatorOrg: seco.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6984c69f-9d46-408d-a72e-08da2eabb051 X-MS-Exchange-CrossTenant-AuthSource: DB7PR03MB4972.eurprd03.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 May 2022 15:26:56.2522 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: bebe97c3-6438-442e-ade3-ff17aa50e733 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: csHjtDd4kTgVpF7uDoEINVXE8feH039Km5pL0rLaTdYDwlN4dapL4QK0pa+XMP9l4PbVPusVdeE1+rngi47eVA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR03MB3806 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean Hi Simon, On 5/3/22 4:50 AM, Simon Glass wrote: > Hi Sean, > > On Fri, 29 Apr 2022 at 13:40, Sean Anderson wrote: >> >> Hi Simon, >> >> On 4/25/22 11:24 AM, Sean Anderson wrote: >> > >> > >> > On 4/25/22 1:48 AM, Simon Glass wrote: >> >> Hi Sean, >> >> >> >> On Mon, 18 Apr 2022 at 13:37, Sean Anderson wrote: >> >>> >> >>> This adds support for "nvmem cells" as seen in Linux. The nvmem device >> >>> class in Linux is used for various assorted ROMs and EEPROMs. In this >> >>> sense, it is similar to UCLASS_MISC, but also includes >> >>> UCLASS_I2C_EEPROM, UCLASS_RTC, and UCLASS_MTD. New drivers corresponding >> >>> to a Linux-style nvmem device should be implemented as one of the >> >>> previously-mentioned uclasses. The nvmem API acts as a compatibility >> >>> layer to adapt the (slightly different) APIs of these uclasses. It also >> >>> handles the lookup of nvmem cells. >> >>> >> >>> While nvmem devices can be accessed directly, they are most often used >> >>> by reading/writing contiguous values called "cells". Cells typically >> >>> hold information like calibration, versions, or configuration (such as >> >>> mac addresses). >> >>> >> >>> nvmem devices can specify "cells" in their device tree: >> >>> >> >>> qfprom: eeprom@700000 { >> >>> #address-cells = <1>; >> >>> #size-cells = <1>; >> >>> reg = <0x00700000 0x100000>; >> >>> >> >>> /* ... */ >> >>> >> >>> tsens_calibration: calib@404 { >> >>> reg = <0x404 0x10>; >> >>> }; >> >>> }; >> >>> >> >>> which can then be referenced like: >> >>> >> >>> tsens { >> >>> /* ... */ >> >>> nvmem-cells = <&tsens_calibration>; >> >>> nvmem-cell-names = "calibration"; >> >>> }; >> >>> >> >>> The tsens driver could then read the calibration value like: >> >>> >> >>> struct nvmem_cell cal_cell; >> >>> u8 cal[16]; >> >>> nvmem_cell_get_by_name(dev, "calibration", &cal_cell); >> >>> nvmem_cell_read(&cal_cell, cal, sizeof(cal)); >> >>> >> >>> Because nvmem devices are not all of the same uclass, supported uclasses >> >>> must register a nvmem_interface struct. This allows CONFIG_NVMEM to be >> >>> enabled without depending on specific uclasses. At the moment, >> >>> nvmem_interface is very bare-bones, and assumes that no initialization >> >>> is necessary. However, this could be amended in the future. >> >>> >> >>> Although I2C_EEPROM and MISC are quite similar (and could likely be >> >>> unified), they present different read/write function signatures. To >> >>> abstract over this, NVMEM uses the same read/write signature as Linux. >> >>> In particular, short read/writes are not allowed, which is allowed by >> >>> MISC. >> >>> >> >>> The functionality implemented by nvmem cells is very similar to that >> >>> provided by i2c_eeprom_partition. "fixed-partition"s for eeproms does >> >>> not seem to have made its way into Linux or into any device tree other >> >>> than sandbox. It is possible that with the introduction of this API it >> >>> would be possible to remove it. >> >> >> >> I still think this would be better as a separate uclass, with child >> >> devices created at bind time in each of the respective uclasses, like >> >> mmc_bind() does. Then you will see the nvmem devices in the DM tree. >> >> Wouldn't we want to add a command to access the nvmem devices? >> > >> > We already do. E.g. the misc/rtc/eeprom commands. The problem is that >> > for software to access them, they would have to use misc_read/dm_rtc_read/ >> > i2c_eeprom_read. >> > >> >> This patch feels like a shortcut to me and I'm not sure of the >> >> benefit of that shortcut. >> > Well, I suppose it's because "nvmem" devices are strict subsets of >> > existing devices. There is no new functionality here (except adapting >> > between semantics like for misc). We should always be able to use the >> > existing API to implement support for a new underlying uclass. There >> > should never be device-specific read/write methods, because we can >> > use the existing read/write uclass methods. >> > >> > What I'm trying to get at is that we sort of already have an nvmem >> > uclass with nvmem devices, they're just not accessible in a uniform >> > way. This series is trying to address the uniformity aspect. But I >> > don't think we need new devices for each nvmem interface, because >> > all they would do would take up ram/rom. >> > >> > --Sean >> > >> > PS. In an ideal world we'd have something like >> > >> > struct nvmem_ops { >> > read(); >> > write(); >> > }; >> > >> > struct dm_rtc_ops { >> > nvmem_ops nvmem; >> > /* the other ops minus read/write */ >> > }; >> > >> > int nvmem_read (...) { >> > struct nvmem_ops *ops = cell->nvmem->ops; >> > /* ... */ >> > >> > return ops->read(...); >> > } >> > >> > but unfortunately, we already have fragmented implementations. > > I don't see that as ideal as it involves a 'nested' API, something we > have avoided so far. > >> > >> >> To follow up on this, I've conducted some size experiments. The >> following is the bloat caused by applying the current series on >> sandbox64_defconfig: >> >> add/remove: 8/0 grow/shrink: 7/2 up/down: 1069/-170 (899) >> Function old new delta >> nvmem_cell_get_by_index - 216 +216 >> dm_test_ethaddr - 192 +192 >> nvmem_cell_write - 125 +125 >> nvmem_cell_read - 125 +125 >> nvmem_cell_get_by_name - 65 +65 >> addr - 64 +64 >> sandbox_i2c_rtc_probe - 54 +54 >> sb_eth_write_hwaddr 14 57 +43 >> sandbox_i2c_eeprom_probe 70 112 +42 >> misc_sandbox_probe 21 61 +40 >> eth_post_probe 444 484 +40 >> _u_boot_list_2_ut_dm_test_2_dm_test_ethaddr - 32 +32 >> __func__ 15147 15163 +16 >> data_gz 18327 18338 +11 >> dsa_pre_probe 181 185 +4 >> sb_eth_of_to_plat 126 64 -62 >> default_environment 553 445 -108 >> Total: Before=1765267, After=1766166, chg +0.05% >> >> And here is the difference (from baseline) when using your >> suggested approach: >> >> add/remove: 26/0 grow/shrink: 8/2 up/down: 2030/-170 (1860) >> Function old new delta >> dm_test_ethaddr - 192 +192 >> nvmem_cell_get_by_index - 152 +152 >> nvmem_register - 137 +137 >> _u_boot_list_2_driver_2_rtc_nvmem - 128 +128 >> _u_boot_list_2_driver_2_misc_nvmem - 128 +128 >> _u_boot_list_2_driver_2_i2c_eeprom_nvmem - 128 +128 >> _u_boot_list_2_uclass_driver_2_nvmem - 120 +120 >> misc_nvmem_write - 68 +68 >> misc_nvmem_read - 68 +68 >> nvmem_cell_write - 66 +66 >> nvmem_cell_read - 65 +65 >> nvmem_cell_get_by_name - 65 +65 >> addr - 64 +64 >> sandbox_i2c_rtc_probe - 54 +54 >> rtc_post_bind - 48 +48 >> nvmem_rtc_write - 48 +48 >> nvmem_rtc_read - 48 +48 >> misc_post_bind - 48 +48 >> i2c_eeprom_nvmem_write - 48 +48 >> i2c_eeprom_nvmem_read - 48 +48 >> sb_eth_write_hwaddr 14 57 +43 >> sandbox_i2c_eeprom_probe 70 112 +42 >> misc_sandbox_probe 21 61 +40 >> eth_post_probe 444 484 +40 >> _u_boot_list_2_ut_dm_test_2_dm_test_ethaddr - 32 +32 >> rtc_nvmem_ops - 16 +16 >> misc_nvmem_ops - 16 +16 >> i2c_eeprom_post_bind - 16 +16 >> i2c_eeprom_nvmem_ops - 16 +16 >> __func__ 15147 15163 +16 >> data_gz 18327 18338 +11 >> fmt - 9 +9 >> version_string 68 74 +6 >> dsa_pre_probe 181 185 +4 >> sb_eth_of_to_plat 126 64 -62 >> default_environment 553 445 -108 >> Total: Before=1765267, After=1767127, chg +0.11% >> >> As you can see, adding a second driver for each nvmem device >> doubles the size of this feature. The patch I used for this follows >> (it does not apply cleanly to v3 because the base contains some >> changes fixing bugs pointed out by Tom). > > Thanks for the analysis and patch. This is what buildman reports for me: > > 01: test: Load mac address using misc device > sandbox: w+ sandbox > 02: misc: nvmem: Convert to using udevices > sandbox: (for 1/1 boards) all +1584.0 data +576.0 rodata +32.0 text +976.0 > [..] > > So we have text growth of about 1KB on 64-bit x86. The data size is > not that important IMO since this is most likely to be used in U-Boot > proper which doesn't have tight constraints. For that we should > instead focus on reducing the cost of driver model overall, e.g. with > the ideas mentioned at [1]. Yes, I agree. One of the big problems is that each driver struct takes 128 bytes each. With 3 drivers added (and a uclass driver), that's 512 bytes right from the start. I don't think this is covered by your series. The current design is very ergonomic, but I don't know if it's the best space-wise. Another problem is that each function has to move registers around to set up the parameters for its two calls: 00000000000a56c4 : a56c4: f3 0f 1e fa endbr64 a56c8: 53 push %rbx a56c9: 48 89 cb mov %rcx,%rbx a56cc: 48 83 ec 10 sub $0x10,%rsp a56d0: 89 74 24 0c mov %esi,0xc(%rsp) a56d4: 48 89 14 24 mov %rdx,(%rsp) a56d8: e8 49 00 fd ff callq 75726 a56dd: 48 8b 14 24 mov (%rsp),%rdx a56e1: 8b 74 24 0c mov 0xc(%rsp),%esi a56e5: 89 d9 mov %ebx,%ecx a56e7: 48 83 c4 10 add $0x10,%rsp a56eb: 48 89 c7 mov %rax,%rdi a56ee: 5b pop %rbx a56ef: e9 4c ff ff ff jmpq a5640 I suppose we could shave off ~120 bytes by moving the dev_get_parent calls to nvmem_cell_read. > I didn't try on Thumb2 but I suppose it would be about 0.5KB. add/remove: 20/0 grow/shrink: 0/3 up/down: 731/-100 (631) Function old new delta dm_rtc_write - 78 +78 nvmem_register - 72 +72 _u_boot_list_2_uclass_driver_2_nvmem - 72 +72 _u_boot_list_2_driver_2_rtc_nvmem - 68 +68 _u_boot_list_2_driver_2_misc_nvmem - 68 +68 _u_boot_list_2_driver_2_i2c_eeprom_nvmem - 68 +68 misc_nvmem_write - 38 +38 misc_nvmem_read - 38 +38 rtc_post_bind - 28 +28 misc_post_bind - 28 +28 nvmem_rtc_write - 26 +26 nvmem_rtc_read - 26 +26 i2c_eeprom_nvmem_write - 26 +26 i2c_eeprom_nvmem_read - 26 +26 misc_write - 24 +24 i2c_eeprom_post_bind - 12 +12 fmt - 9 +9 rtc_nvmem_ops - 8 +8 misc_nvmem_ops - 8 +8 i2c_eeprom_nvmem_ops - 8 +8 version_string 74 68 -6 nvmem_cell_read 96 50 -46 nvmem_cell_get_by_index 144 96 -48 Total: Before=362129, After=362760, chg +0.17% > It seems OK to pay this cost to keep things clean. It's not terribly large, but I would like to try and keep the size of new features down. I'd like to make it easy to choose to use NVMEM instead of e.g. mac_read_from_eeprom > If we do go ahead with this series (i.e. without the last patch), I > don't think it should be a model for how to add new APIs in future. > E.g. we could save space by making a special case for PMICs which > support GPIOs, so that GPIO operations could accept a PMIC device or a > GPIO device, but it would be quite confusing, We could make the > random-number device disappear and just 'know' that a TPM and a MISC > device can produce random numbers, but I have the same feeling about > that. I agree. I think this is a bit of a special case because of how we already have several APIs all implementing the same idea. > Given that you have done the analysis and you are still pretty keen on > this, I am OK with it going in. I don't like it very much. but we can > always review things later. Perhaps add a comment in the nvmem header > files about how it works and why it doesn't use driver model in the > normal way? Sure. I will also include the patch from before as an RFC on the end of the series. --Sean