Received-SPF: pass (google.com: domain of linux-kernel+bounces-55267-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Received-SPF: Pass (protection.outlook.com: domain of topic.nl designates
 20.93.157.195 as permitted sender) receiver=protection.outlook.com;
 client-ip=20.93.157.195; helo=westeu11-emailsignatures-cloud.codetwo.com;
 pr=C
Authentication-Results-Original: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=topic.nl;
Message-ID: <e04ca010-289c-4216-95ea-2f2418613378@topic.nl>
Date: Tue, 6 Feb 2024 16:44:03 +0100
User-Agent: Mozilla Thunderbird
From: Mike Looijmans <mike.looijmans@topic.nl>
Subject: Re: [PATCH v3 2/2] iio: adc: ti-ads1298: Add driver
Content-Language: en-US, nl
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
CC: devicetree@vger.kernel.org, linux-iio@vger.kernel.org,
 Jonathan Cameron <jic23@kernel.org>, Lars-Peter Clausen <lars@metafoo.de>,
 Liam Beguin <liambeguin@gmail.com>, Liam Girdwood <lgirdwood@gmail.com>,
 Maksim Kiselev <bigunclemax@gmail.com>,
 Marcus Folkesson <marcus.folkesson@gmail.com>,
 Marius Cristea <marius.cristea@microchip.com>,
 Mark Brown <broonie@kernel.org>, Niklas Schnelle <schnelle@linux.ibm.com>,
 Okan Sahin <okan.sahin@analog.com>, linux-kernel@vger.kernel.org
References: <20240206065818.2016910-1-mike.looijmans@topic.nl>
 <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.949ef384-8293-46b8-903f-40a477c056ae.fd628a1a-a926-426e-a239-bfd8c9858b94@emailsignatures365.codetwo.com>
 <20240206065818.2016910-2-mike.looijmans@topic.nl>
 <ZcIsuiuisQjTIxJv@smile.fi.intel.com>
 <4c6654f5-2d9e-4c1b-a5de-7bdeacf5e99f@topic.nl>
 <ZcI5PoWojKRrdpVl@smile.fi.intel.com>
 <67387cf4-1065-4313-b4c6-054128ba8f3a@topic.nl>
 <40a3a47b-1388-4ed0-a24b-2c0bcef3be3d@topic.nl>
 <ZcJLnOiFoaABami1@smile.fi.intel.com>
Organization: Topic
In-Reply-To: <ZcJLnOiFoaABami1@smile.fi.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0
X-Microsoft-Antispam-Message-Info-Original:
 Z+B86hJEX+XeOWVo5W9PxrbTQ3O3gK+RdQnNitoHmCfD8DAN5aQB61qpduqmPF8bsMZJrI5qaXuDMOUjYV+NW9+pmaZ9l9FGET4zQIGnBtZY42wPw7iNuJrMu3xX7p+fMs6bpz0weckwswz92bffGThXDcl62jvQgTq4q7vkj0xyXm1MfbuZj3vhzMeyjJBgJkPBx6xE2sXU2vtC+i2KcvT8xjAA9otm68lZv8uT45aA6U9Eca95Zti1YTkJ2BuJ37UohBCanF/Vuhn0RnoLzuxU5PmkGyiOfcY3oYFFzJyXPh2RPFT64pDOSx5PsT1MCjTdDGwUtCp2mvAY2M5b2G9FBI3dpplxViCkRTXZhDIN5S9tIAprutP7uITlnUMXPCEq1F+H/VkKehN6mlbhSe1VlNmyz4xVR+w75H48fQWy10tjs0agVw0qsT3Q9dKpxbyaLfN088YzHL7XvUu+LILsrNjlIuOMwR+/8fmVIeHZQL/vco+KcDdt3xRCQYY6sa8JaSxp7GCl9rc1McG+dcZGkUD9iRmddiCJ+DszLNJ1Xr0/R8xIG8blEJo89lRH09xnCV0ZksFyDzs1IsfnkTm0EFT2xFr1fFhTYQwzi3/b8u6ZSUvCYeR+xnOvIPMWMLemqVW/HqKaA+fjSWC9l+0rL5QOcyQyObJnGMwThbI=
X-Forefront-Antispam-Report-Untrusted:
 CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AS8PR04MB8644.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(396003)(39840400004)(376002)(136003)(366004)(346002)(230922051799003)(1800799012)(64100799003)(451199024)(186009)(7416002)(5660300002)(44832011)(2906002)(31686004)(86362001)(41300700001)(83380400001)(38100700002)(31696002)(26005)(2616005)(15974865002)(36916002)(6512007)(6506007)(52116002)(36756003)(66476007)(54906003)(6916009)(478600001)(6486002)(66946007)(53546011)(66556008)(4326008)(38350700005)(8676002)(8936002)(316002)(43740500002)(45980500001);DIR:OUT;SFP:1102;
X-MS-Exchange-Transport-CrossTenantHeadersStamped: GVXPR04MB9758
X-CodeTwo-MessageID: 1528df36-8057-48d1-8c11-da68809fd9f8.20240206154408@westeu11-emailsignatures-cloud.codetwo.com
X-CodeTwoProcessed: true
X-EOPAttributedMessage: 0
X-MS-Exchange-Transport-CrossTenantHeadersStripped:
 AMS0EPF000001A0.eurprd05.prod.outlook.com
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id-Prvs:
	72804338-807c-4406-4985-08dc272a728f
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info:
	YMbC4+Lu4rOMCzXUtX9izmNiY48uji5FlEIbKk3ToNd3kVaI78gypLxj+yfXGwPeue3pG0LLAhWfMwIWUbPgUH8I6A+9bX+4i78cq515CVFJUXUA135Dc3fj7pqGfefH7VS1bkhH/839Smzw1kF1xtR6Hf+Lx2i96RZTahR/l4d4aMVsY/d21dc1q4KtGM2qa2Ojwim5h7A6FI/uoORpny4xmCfQRed2nA+Z7XQppPlZfN7dCBSHoiYJowX9AkMT3rtWTcYEHkZFORhU4mqXUIAlKDZdDFMsIOPROzfbY0NMBNaC7N1HITyiq3esj/+Q8Y1yCUrJorPiVFjUPUlq6njVcRheH1QsA0NDDYAKf9MTvsQW0iOwXtHuzdK8Hk9cnI5GtBP6dyFKVWQN6bp0ZFH1OiG1/W2QCjsHENuet0rIwZlZIpAYMIGB9TGe/B2EGCO4uD1VDDFMUdyXe4DPuEEmKmZ6Joo5omYnn67Wd3iP2UGo8Xay3vOAt5b6jDaje+34S+iPMZZ0zP1D3llVOBiEO/o3tsB2wJAxHk2tpas/Ar/Ir4jxtKrVDOpkn5H7bcPh8Y3B6nTeATVhndQte8oseSrXnxUC9Ubp3E5vjmE9rJVsPuOLv1WhnFWw38t880OcB68+G8UZQYRTYI9bp8pATrnmLbvidlTA4+lwYSIBeTdJ9KGEbmThLlWaQuUIgInqGITINLwDzgLr9q4XzZGDG+OanFN5OnhMUmd5YSMgkHPh53bWVfej/iigItOXuQU43MUx2WQr44K2AH9NTA==
X-Forefront-Antispam-Report:
	CIP:20.93.157.195;CTRY:NL;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:westeu11-emailsignatures-cloud.codetwo.com;PTR:westeu11-emailsignatures-cloud.codetwo.com;CAT:NONE;SFS:(13230031)(4636009)(39840400004)(136003)(346002)(396003)(376002)(230922051799003)(64100799003)(186009)(82310400011)(1800799012)(451199024)(46966006)(36840700001)(41300700001)(31686004)(40480700001)(6506007)(53546011)(47076005)(336012)(36916002)(2616005)(6512007)(26005)(7636003)(31696002)(7596003)(36860700001)(36756003)(86362001)(15974865002)(356005)(7416002)(4326008)(2906002)(6486002)(83380400001)(44832011)(8936002)(8676002)(5660300002)(316002)(478600001)(54906003)(6916009)(70206006)(70586007)(43740500002)(18886075002);DIR:OUT;SFP:1102;
X-OriginatorOrg: topic.nl
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Feb 2024 15:44:09.8578
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: d1d2ef2a-1ef2-4a15-9994-08dc272a75db
X-MS-Exchange-CrossTenant-Id: 449607a5-3517-482d-8d16-41dd868cbda3
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=449607a5-3517-482d-8d16-41dd868cbda3;Ip=[20.93.157.195];Helo=[westeu11-emailsignatures-cloud.codetwo.com]
X-MS-Exchange-CrossTenant-AuthSource:
	AMS0EPF000001A0.eurprd05.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS4PR04MB9689

On 06-02-2024 16:09, Andy Shevchenko wrote:
> On Tue, Feb 06, 2024 at 03:47:45PM +0100, Mike Looijmans wrote:
>> On 06-02-2024 15:25, Mike Looijmans wrote:
>>> On 06-02-2024 14:50, Andy Shevchenko wrote:
>>>> On Tue, Feb 06, 2024 at 02:33:47PM +0100, Mike Looijmans wrote:
>>>>> On 06-02-2024 13:57, Andy Shevchenko wrote:
>>>>>> On Tue, Feb 06, 2024 at 07:58:18AM +0100, Mike Looijmans wrote:
> ...
>
>>>>>>> +=C2=A0=C2=A0=C2=A0 wasbusy =3D --priv->rdata_xfer_busy;
>>>>>> Why preincrement? How would it be different from postincrement?
>>>>> Maybe better write this as:
>>>>>
>>>>> --priv->rdata_xfer_busy;
>>>>>
>>>>> wasbusy =3D priv->rdata_xfer_busy;
>>>>>
>>>>> I want the value after decrementing it.
>>>> Yes, looks more obvious.
>>> Having done that, and looking at it again, it's better to just eliminat=
e
>>> the local "wasbusy" altogether. More concise.
>>
>> This removes the decrement operator, so the method now looks like this:
>>
>>
>> static void ads1298_rdata_release_busy_or_restart(struct ads1298_private
>> *priv)
>> {
>>  =C2=A0=C2=A0 =C2=A0guard(spinlock_irqsave)(&priv->irq_busy_lock);
>>
>>  =C2=A0=C2=A0 =C2=A0if (priv->rdata_xfer_busy > 1) {
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 /*
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0* DRDY interrupt occurred b=
efore SPI completion. Start a new
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0* SPI transaction now to re=
trieve the data that wasn't latched
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0* into the ADS1298 chip's t=
ransfer buffer yet.
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0*/
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 spi_async(priv->spi, &priv->rdata=
_msg);
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 /*
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0* If more than one DRDY too=
k place, there was an overrun. Since
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0* the sample is already los=
t, reset the counter to 1 so that
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0* we will wait for a DRDY i=
nterrupt after this SPI transaction.
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0*/
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 priv->rdata_xfer_busy =3D 1;
>>  =C2=A0=C2=A0 =C2=A0} else {
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 /* No pending data, wait for DRDY=
 */
>>  =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 priv->rdata_xfer_busy =3D 0;
>>  =C2=A0=C2=A0 =C2=A0}
>> }
> Yep and it looks like you reinvented atomics :-)
>
> 	atomic_t rdata_xfer_busy;
> 	...
>
> But it's up to you what to do with that.
> Maybe Jonathan can advice something different.
>
The spinlock also protects the call to spi_async().


--=20
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl