Received-SPF: pass (google.com: domain of linux-kernel+bounces-194381-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: "Nechita, Ramona" <Ramona.Nechita@analog.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
CC: "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
        Jonathan Cameron
	<jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        "Hennerich,
 Michael" <Michael.Hennerich@analog.com>,
        Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>,
        "Sa, Nuno" <Nuno.Sa@analog.com>,
        Marius
 Cristea <marius.cristea@microchip.com>,
        "Schmitt, Marcelo"
	<Marcelo.Schmitt@analog.com>,
        Maksim Kiselev <bigunclemax@gmail.com>,
        Ivan
 Mikhaylov <fr0st61te@gmail.com>,
        Marcus Folkesson
	<marcus.folkesson@gmail.com>,
        Liam Beguin <liambeguin@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] drivers: iio: adc: add support for ad777x family
Thread-Topic: [PATCH] drivers: iio: adc: add support for ad777x family
Thread-Index: AQHarD+43ERipWM1gkKh9ZW8mJCjp7GjTUqAgAsEBnCAAAsrgIAACZWw
Date: Wed, 29 May 2024 15:45:58 +0000
Message-ID: 
 <SN6PR03MB432024DCE5978841F016A71FF3F22@SN6PR03MB4320.namprd03.prod.outlook.com>
References: <20240522120005.18197-1-ramona.nechita@analog.com>
 <Zk3-qxCb0zfR440Q@smile.fi.intel.com>
 <SN6PR03MB432071701D6A19C3B4C0C33FF3F22@SN6PR03MB4320.namprd03.prod.outlook.com>
 <ZldFpSQPAmAhbW2I@smile.fi.intel.com>
In-Reply-To: <ZldFpSQPAmAhbW2I@smile.fi.intel.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?DiHu07DSsNfmXV1wgBX3KUlpfh9VIaqQMUhM2432RGiKLWqkw0yJ6N9CBPMo?=
 =?us-ascii?Q?UBTS8Gn8cu6Ic0iyx6i5hnoNIxnWD51CkPAbiqaP2Zw1lDrF6wiQmTZ29CWu?=
 =?us-ascii?Q?RVzol/qG4OneoMojI/Xv3j7Ka55ElTq1VThjHPXlPS0jPSCrN5mgz6WBHwcW?=
 =?us-ascii?Q?lbb0gJV3Hxpo0MK+rtIZIai/pql+TTFXo38nCe9v3i06hivSC5mxBySDy2ca?=
 =?us-ascii?Q?Sa+ryKqV4wJUupr0Ln5ssnatCv4JwC6XDkPbGQhyjdtvxXBPO8I54Y5PMDmu?=
 =?us-ascii?Q?S6YnmTfy3k3Eeum4bj8/2g4aCAZo2/5jlnvPNqqAUhfHEpnGbmgyoHp7XRSd?=
 =?us-ascii?Q?J+clgP9lNOcbTQT9nhkNXYkszCHB+LRoaUStim3FAeDw9VeE+mHai148IUPM?=
 =?us-ascii?Q?RFAtsXf5Vg3Xjj5kPN0uHoDtt6/PE0cm0RnAX1aRDweMGaWUORcdwkh4wgjX?=
 =?us-ascii?Q?EnielgMJn3Q6630eA10S7sUnC9KE+cuOElYqSh0LJonYH7Yd1BNQZXJ0JB8u?=
 =?us-ascii?Q?4cbqPC2xtyX2P6a4zsZh3qwQIw+r8X2l248H/H8MpcV1VG8CujSyVoFO+QgR?=
 =?us-ascii?Q?ZplFui7Qi959xCaF28kZYfYJP0rzPTtXWDi9vCFuX5nfmFmLW12O199Vo1Y9?=
 =?us-ascii?Q?OxAChxGNlD06lTIspku44bW5BBw7bQviLgOSuGxcuu41LawcDCvSbTAMtO/2?=
 =?us-ascii?Q?9Gd9aKq5iqEWLD0NMRkDPrZd6TCEGo9dZzbZi+6TnmJyLNPxYRdSJrqq3hDj?=
 =?us-ascii?Q?u17dIIz0Xz699W3h3SY7H6AtxwAb0ivsGHDawt0dkc+5NGcqyhlSU5T1MeiV?=
 =?us-ascii?Q?kvIT9fITyCF7il6zkt8McJTLCjvv7X0ckaqf7887vlAJCyfgf3+5j1/nNwpI?=
 =?us-ascii?Q?ipi9Oi8FI78HPjj49BlVff5YYSMvOLQCVg7q7UBwtE4db2YooO78q9TJqPHk?=
 =?us-ascii?Q?/y/xCCpPUmWmcA9+vLdgIcsqxdqZ4O7WqWOcI0Fbd6lSWwaDXiwvdWzEywq9?=
 =?us-ascii?Q?YUsRoahZDtUe1FvD79bkPQ6ooxeGupUYpFnrYAMVEfSlixNYKqeVqQpz6mFx?=
 =?us-ascii?Q?14tZTmwQxMG7D0VlaX/C38dYLg/I9iYiatDHiukjwjNLuzGeVpwJpAILdzb4?=
 =?us-ascii?Q?Sp3VrqqUcwY8gjELziCf8m83j8NLuzzVMzJfBb5Dij7tOCCYICzbAxpO1PJI?=
 =?us-ascii?Q?YjK91OAkwUFpghgCjwI9rNSr/7mHdA9vHpC3cwHT4MtSywuEp6txIjKvas13?=
 =?us-ascii?Q?A9dSNkbjNKrJCX5PDK3SxIor+Ak9ZKGkO4HuwyBFhygxZOl+Lnuyt9QWJbCy?=
 =?us-ascii?Q?IScA+EJPCUZj3h5ir5v1TQa4yUjGyDk3rNCYWVGNkyst3n4IyRWCd1lfnK4V?=
 =?us-ascii?Q?pYZDYWMgil/JH69Uu11Yjw9ixGIQeDy6G+8jxVwBN0fiOA743hvYRqax7hdB?=
 =?us-ascii?Q?OXuaVQ65RYWxTwmjsVcYgmRWZQfhS64/scJCY77ABrmMkZBaY9K5ydEKtl5/?=
 =?us-ascii?Q?vNKeVhLdySLND16xmjwsm+WN7UYDz6zqJYjVyi78Ydja0sZxU5u7t+yJuIei?=
 =?us-ascii?Q?1YpOMrk+ZNfMRXi3yo5nmf6oGa08ip2THhFCkvaW?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: analog.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR03MB4320.namprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3497c1dc-d63e-4960-1063-08dc7ff66f1d
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 May 2024 15:45:58.2161
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: eaa689b4-8f87-40e0-9c6f-7228de4d754a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: EjYNm0b1pJ7XZPGBwSTpnigrscQhOqkeSttVRVJrfbgoVfDvy7/6iDkhkJog77srMieMkxdqP7qlcni8Y861he8VWmWQUPBxagd7xMNZ428=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR03MB6063
X-Proofpoint-ORIG-GUID: UqLxxliRahX-iDgPhqKGBNa8oQIpa4Pj
X-Proofpoint-GUID: UqLxxliRahX-iDgPhqKGBNa8oQIpa4Pj
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.12.28.16
 definitions=2024-05-29_12,2024-05-28_01,2024-05-17_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 malwarescore=0
 suspectscore=0 mlxlogscore=857 bulkscore=0 phishscore=0 lowpriorityscore=0
 spamscore=0 clxscore=1015 mlxscore=0 impostorscore=0 priorityscore=1501
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2405170001
 definitions=main-2405290107

>On Wed, May 29, 2024 at 03:01:06PM +0000, Nechita, Ramona wrote:
>
>...
>
>> >> +	/*
>> >> +	 * DMA (thus cache coherency maintenance) requires the
>> >> +	 * transfer buffers to live in their own cache lines.
>> >> +	 */
>> >> +	u8			reg_rx_buf[3] ____cacheline_aligned;
>> >> +	u8			reg_tx_buf[3];
>> >
>> >> +	u8			spidata_rx[32];
>> >> +	u8			spidata_tx[32];
>> >
>> >These will not be cache aligned. Is it a problem?
>>=20
>> No, it should be fine without the alignment.
>
>I.o.w. it means that only reg_*x_buf are supposed to be in the different c=
ache lines, correct?

Yes, that was how I intended.

>
>...
>
>> >Btw, can't you use regmap for IO?
>>=20
>> Unfortunately, I don't think regmap could be used, because of the crc=20
>> and the fact that data is shifted out on the SPI SDO line in the=20
>> interrupt. I consider perhaps adding regmap to the mix might complicate =
things a bit.
>
>Can you add this into the comment area of the patch?

Sure thing, I will wait a little to see if there are any new comments on th=
e v2 of the patch and I will make sure to add this as well in the next vers=
ion.

>
>...
>
>> >> +	ret =3D ad777x_spi_write(st, AD777X_REG_SRC_N_LSB, lsb);
>> >> +	if (ret)
>> >> +		return ret;
>> >> +	ret =3D ad777x_spi_write(st, AD777X_REG_SRC_N_MSB, msb);
>> >> +	if (ret)
>> >> +		return ret;
>> >
>> >Can you use 16-bit writes?
>> >Same Q to all similar LSB/MSB write groups.
>>=20
>> I cannot do 16-bit writes due to how the spi functions on the chip and=20
>> because the registers for MSB/LSB are at different addresses.
>
>They are supposed to be on the different addresses.
>You mean the distance between them > than stride?

I may be misunderstanding this, but the spi read/write expects HeaderBit+Ad=
dr+/-Data+CRC. Writing two=20
consecutive registers would mean creating a buffer with this format for eac=
h address and
sending all of them at once, correct? I feel like that would overcomplicate=
 the code a bit, but
I can do it if it seems appropriate.

>
>...
>
>> >> +	ret =3D devm_add_action_or_reset(&spi->dev,
>> >> +				       ad777x_clk_disable,
>> >> +				       st->mclk);
>> >> +	if (ret)
>> >> +		return ret;
>> >
>> >So, what's wrong with the _enabled() API?
>>=20
>> Sorry, I am not sure what you mean here by _enabled() API, is there a=20
>> different mechanism that can be used for this type of operations?
>
>devm_clk_get_enabled()

I will look into this and update it if it is the case with the review from =
the v2 patch as well.

>
--
Best Regards,
Ramona Nechita