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 X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AE9EBC388F7 for ; Sun, 25 Oct 2020 12:58:19 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 407E3208B3 for ; Sun, 25 Oct 2020 12:58:19 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="hS6iOhrm"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="qI254pdS" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 407E3208B3 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=I++ZijD3OhqoVclzF1Glkm2MBiDkqHwJJ+tHnh/umCQ=; b=hS6iOhrmiqdrPe4JuzOTcttXI DHq9VbdqLPVZvT4laPAHLF6bw4rickYo4KgIPyhXZ9kMmyh5GylKh0Jcpa8TDKKcIZdzjGDzN/KU8 Q1eBn9w7uhYjYYC7VC5+nZ5VU17Taue6GrPdAELOyB1sQ3CrLe9Y7Cj0SFqEOsxK4kbL3HtEQMgHz m+eFZDJG+mgYmEBBzEew7QX/TBGULrFXSyTsu0dNeWoCz40SEcDLvH6SEYSldKQV4ZZSYVjZji2ds WEr4QG7aas6vSualUkS/xBv6ITfbOKY8bbwI4rT8iR7PTmvXW0BCbE6fG8Y/1ZFwM7HQAJScmdPJv NdmKfOKqw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kWfZS-0006Y6-D0; Sun, 25 Oct 2020 12:56:26 +0000 Received: from mail-qk1-x742.google.com ([2607:f8b0:4864:20::742]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kWfZP-0006XY-Dm for linux-arm-kernel@lists.infradead.org; Sun, 25 Oct 2020 12:56:24 +0000 Received: by mail-qk1-x742.google.com with SMTP id j129so2090882qke.5 for ; Sun, 25 Oct 2020 05:56:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=dW6wQtx+CemqlF8Wph1tzg0pja08Z9O004Vy7mx2JnA=; b=qI254pdSKPJ5rsI3UGK2qp/UfOvOsA+m62PqibOeP3UNFzbeGlY5CoCnZYqylATVlh NTuF9Jkoj5E4oNI2L1nIlFrVAHcHSEPueQj9KWc/8CBU3d6oL1zT+YtV8XrwpMqF8nEA l+j+3+BnjJZyHwPP+yui8ddYSM6tHDx70MzOFNvK99KZS7qxtZ9lCxzYzpg+of5b1s6i o0LOWC0qQcEos7RDkvP1C89ZSGwWuk7Y/22GekU4MjgxMpuyfuphjHQbIkuqWRgR5kXc 5W5eB0eK0wU5hRsBz8LFFzj3w805PGfWxjHdvM4H9jcfq3ngNXqR4sUfpbv7shYIP6mX TUzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=dW6wQtx+CemqlF8Wph1tzg0pja08Z9O004Vy7mx2JnA=; b=T5llMxCJ+0Z19A5y24CenItgZxNn2Tvqf2rFsXjkrsvQlF78Vx1hHbMnW3MZ2o+Mlh UBiXy/SqMctd21Bv2+cDethhnaGggJspD/KO2sfK9zNYjBneQOumJL/0TqwzLP9RviYv aNDPIvEXo4mIK5VwK8mcvlf+5pEhKXLrXrCeGKkel95NkePLWjoar4g7cju+E9b3Qvdj EpHoktS/QvSp1ok3Urol0q+kdCluG0HlXpoV54bFvlhq+cTzfZMSNvRKQ1qG6V1jVYGA mO8gmEsBMzeegsAw3Jwf/N0jvxGhCZZAAPcqDB+2GJkApC8dYNCvqgI1eqZeG9QwOMK1 89cA== X-Gm-Message-State: AOAM530hcwDhhxjjaiSSZIYS0Byq+785itQx1KXeNftDixkqLjfWCO6V 2vTazcivU6VgRWpHIwpxnQOs8VbS9WNDYQ== X-Google-Smtp-Source: ABdhPJz84hctLYUYMFmsNSoipZYjD3+Zg11EwoNyVCHyT7SC4OtHb1h5TbW9oMPHx9yRFkAw5hdvVQ== X-Received: by 2002:a05:620a:15c7:: with SMTP id o7mr12116691qkm.262.1603630573927; Sun, 25 Oct 2020 05:56:13 -0700 (PDT) Received: from shinobu (072-189-064-225.res.spectrum.com. [72.189.64.225]) by smtp.gmail.com with ESMTPSA id j25sm4583718qkk.124.2020.10.25.05.56.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 25 Oct 2020 05:56:12 -0700 (PDT) Date: Sun, 25 Oct 2020 08:55:57 -0400 From: William Breathitt Gray To: David Lechner Subject: Re: [PATCH v5 3/5] counter: Add character device interface Message-ID: <20201025125557.GA3458@shinobu> References: <00be1fccc672c5207f3b04fe4cc09c29e22641f4.1601170670.git.vilhelm.gray@gmail.com> <181eb08a-be0a-f7cc-259d-b2a0f279950b@lechnology.com> <20201018164905.GD231549@shinobu> MIME-Version: 1.0 In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201025_085623_594464_48E790E8 X-CRM114-Status: GOOD ( 34.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kamel.bouhara@bootlin.com, gwendal@chromium.org, alexandre.torgue@st.com, linux-iio@vger.kernel.org, patrick.havelange@essensium.com, alexandre.belloni@bootlin.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, mcoquelin.stm32@gmail.com, syednwaris@gmail.com, linux-stm32@st-md-mailman.stormreply.com, jic23@kernel.org Content-Type: multipart/mixed; boundary="===============3989246506170569520==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============3989246506170569520== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="6TrnltStXW4iwmi0" Content-Disposition: inline --6TrnltStXW4iwmi0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 20, 2020 at 10:53:32AM -0500, David Lechner wrote: > >>> + > >>> +static int counter_chrdev_release(struct inode *inode, struct file *= filp) > >>> +{ > >>> + struct counter_device *const counter =3D filp->private_data; > >>> + unsigned long flags; > >>> + > >>> + put_device(&counter->dev); > >> > >> put_device() should be at the end of the function in case it is the la= st > >> reference. > >=20 > > put_device() shouldn't affect the counter_device events members, so I > > don't think there's a difference in this case if it's called at the > > beginning or end of the counter_chrdev_release function. > >=20 >=20 > It isn't possible the some memory allocated with devm_kalloc() could be > be referenced after calling put_device() now or in the future? You're right, if put_device() is called before then we could end up in a garbage state where the device memory is released but events list has not yet been freed. I'll move put_device() to after the events list is freed then. > >>> diff --git a/drivers/counter/counter-sysfs.c b/drivers/counter/counte= r-sysfs.c > >>> index e66ed99dd5ea..cefef61f170d 100644 > >>> --- a/drivers/counter/counter-sysfs.c > >>> +++ b/drivers/counter/counter-sysfs.c > >> > >> > >> Not sure why sysfs changes are in the chrdev patch. Are these > >> changes related somehow? > >=20 > > Sorry, I forgot to explain this in the cover letter. The changes here > > are only useful for the character device interface. These changes > > introduce the extensionZ_name and extensionZ_width sysfs attributes. > >=20 > > In the character device interface, extensions are selected by their id > > number, and the value returned depends on the type of data. The new > > sysfs attributes introduced here allow users to match the id of an > > extension with its name, as well as the bit width of the value returned > > so that the user knows whether to use the value_u8 or value_u64 union > > member in struct counter_event. > >=20 >=20 > Are we sure that all value types will always be CPU-endian unsigned > integers? Or should we make an enum to describe the data type instead > of just the width? It should be safe to assume that the character device interface will only ever return CPU-endian unsigned integers. The device driver should handle the conversion of any strange endianness from the device before the character device interface, while userspace is the one responsible for interpreting the meaning of count in the context of the application. Let's create a scenario for the sake of example. Suppose we want to use a counter device to track the vertical position of a component moved by a linear actuator. The operator considers some vertical position as the horizon, where anything above would be a positive position and anything below a negative position. The counter device stores its count in big-endian format; but the system CPU expects little-endian. The flow of data for this scenario would look like the following (where BE =3D big-endian, LE =3D little-endian): +----------+ +---------------+ +--------+ | Raw data | - BE -> | Device driver | -> LE -> | chrdev | - u64 -> +----------+ +---------------+ +--------+ At this point, the userspace application would read the unsigned integer =66rom the character device and determine how to interpret the position -- whether the count be converted to a signed value to represent a negative physical position. Whether or not a position should be considered negative is dependent on the user application and context. Because the character device does not know the context of the user application, it should only provide unsigned integers in order to ensure a standard interface for counter devices; userspace will be responsible for interpreting those counts to something meaningful for the context of their applications. William Breathitt Gray --6TrnltStXW4iwmi0 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEk5I4PDJ2w1cDf/bghvpINdm7VJIFAl+VddMACgkQhvpINdm7 VJL5EQ//cg58+imqX6jyWtytVNlM4xXCNghBQJ0b1YWtd+kNxWijJUuiCh6XQtS7 hIkDD6iXipyHCy6NAm2TQmLpMiiZGCZzBdeAEWpXU3ZeyTa0gmxKjuLLDg3ZoqS4 dt6zEHZVPvDExSPsISqIx+Uez4wypRgh2ExF/lMB9XDBWblvLgS7jt7Aqvqh41sa XscbJjxqTFskoh2D/ka8WX4UXGuXh9kxmwuiXwDnMcu6ct3+H1lq1UZ6jbeWMcIQ z+CteMRjZ4b0kZnz5XCxtEZ7HBc79bxkE3bN+LF7fdpSOa9DcoShWdWwqZB/UuOx dhAiIiIo2sUyrHNJkfkOfh6yjGaQ7LXv2Q6aseoHS1h6119aflBUefJGvE6AhyG7 8MNdfuWf4V5H+ete24BTPduUynHfIdV5D9lA2V42p0CkLnVlfQ75SFutqt5aUlUV mukprRWXGySjRg8D6WrKAqr6sY5WEqe8hxfxzFzbjXYrRo6ZgkSBWJx82b+KZOlt xwWL+UiyezTiYe2Updyl37yKyJjzQ5Mfrpt44Z6GPzXmVecuJJC/gIBJW1o6bn39 /JgcFw0DObC7DQuqnf6sCIGT+6V3L5v6yr5pR83WtueeZ4jZAc2pDh+svn+rUv4x BcTTdAs2HR9OYOD8QVDEAkonmtikk35m/G8j+mcAi91TmAE2pdo= =NtZ8 -----END PGP SIGNATURE----- --6TrnltStXW4iwmi0-- --===============3989246506170569520== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============3989246506170569520==--