From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oi1-f170.google.com (mail-oi1-f170.google.com [209.85.167.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 03A5A195808 for ; Thu, 19 Dec 2024 17:01:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734627676; cv=none; b=lQy038UTrNVSntGcrYDEWervcQxF4Hoy0nv5sC8wuqzPvMyLyIxMQyJMd7+KkF0cFeyvZa7dbEJGg8gxSopu6I6mnhKgwUsKSYVn7s6/OzB1lF5IsDn3nWXcUm3d2/6OF1ipDWFM0ohzgDIbC8vs+zjWeISwrK67cGB2wU6e+4k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734627676; c=relaxed/simple; bh=AjD83FY3s3Rg+H6CFlU1E099C6Oj3p5cQP9EQB4oZqw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=k2siIwfez0SWXHvj0WO4IAJ2FZZy6LzRtx+nLO0fpzZ3f/0pcNZhfAKOs3mTWJN/np5cm1t5T8DlzvSNwj1O1BhnlZXQxkx876dxT6xkBzzZ73yhbTflTK21OHC+yscDleN6+wlLAeQg9J1zqZt9QjxTZXUgXhMnzvlaeKjpB1A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b=OEsMnqxt; arc=none smtp.client-ip=209.85.167.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b="OEsMnqxt" Received: by mail-oi1-f170.google.com with SMTP id 5614622812f47-3eb8db8ae9aso404052b6e.1 for ; Thu, 19 Dec 2024 09:01:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1734627672; x=1735232472; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=seYCbwhuufp+j/ujr+9eXs0TRXFJTNhViK8/tEkc+8o=; b=OEsMnqxtXhfznZZr+79B7S3mLu6S5rttIS0Ikxnoanfp3Txo+05Cl82CpZJRkztQ5m JvJEK/F/Ihv1KZa80iTc31K166WUsa3G6IJvbd3rkDE5oVR6VJE8avdyAB5ATsEHTaNi YeqVcnKv9j4KeEXFItthk22BQ2jZyZLqDdqkRtSZL0LOcvPoSO/hGjasKSY6eGHugsPZ jIDAP+4+LdpNy/gSS0Mef3KHZOKwfDnuwoY5YP7TcBy94IVWmZ/0B/cGm/Iljre6Mqix MsB11n3aZR4xCS6mNoRBAeZsv8D5QieRyhivO9UGFTGVVEoGcxPzx0x3g7k2GdVCOi5s lS6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734627672; x=1735232472; h=content-transfer-encoding:in-reply-to:content-language:from :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=seYCbwhuufp+j/ujr+9eXs0TRXFJTNhViK8/tEkc+8o=; b=Igm+m8u4q1rfCXbO9oUtM7DGoz/X1Aboquo4wR1IygbsFcuvZEUDudcv5nYbhBJD3c q89A3xGpnC6jvubqV7KUEYfrQ7wPLpnmazMHn4riESBBIyk/OwLIZJKB0KhS1ktZBlWj qyw3vwvahdfXtpUFJV431x9Q86g0vcX7fR1f9MJR1xxmKq9fkBqqduFhFEtbdFJ3Jnbp rwCIWa0lxQOKW4slxZqR+xUL3A/XYAYqTzPKgoJAaTr5y2fG3vTDIbNMroSUjFhM/8SX IZa51idKrq2xE7nx4vYsrEjlcoCB9Q04jTwftZWWt4zARKShwYuyQJWs2bvQxAOucMZf Xb6A== X-Forwarded-Encrypted: i=1; AJvYcCXCwX02CladNfiizpeS24qewVnoMRRcOEbxnSlttt3NOHjfQy/LmvsKKcqowny8UDsC34MW0Kx1WmF2sj4=@vger.kernel.org X-Gm-Message-State: AOJu0YzTHofZ3yKKU+hSARNQ/63zW2RUlNJ11HFToLRIDRYH5pYcu50f IKynGQMVYI5c5gzjS7XJQLiOIKyDE+NBay3x+/dXdou5WSfTOwECH4LweqODTZQ= X-Gm-Gg: ASbGncuglPpzmfwNO3SJXAPLC8l2Tv4BkHfIJtEY1nzLM99LbmS6RsZcCBAxwQ0gXki ukwYr/eg1nYTidVdt758VukoIDPwTn3y5XgiuvIxc03sk6JhG4tYUgZhIi6w+ZP8PmYgPpBAEl6 yW75XQp3FT9x48MyeWl51YWbFaNtwv13W3hdcqcHDPYu9uSxDhzCB1rPlx3D49SD7azzkyUkYtf XlkPZwhFEa/UUn8czAH8xf6ER8FSWEftN5Nop5TQAJaKoop4/jk0Jq3b7HHc97iLCdKNQgRx989 hoMLsGWE0DTkM+R++w== X-Google-Smtp-Source: AGHT+IHi7ROK/EahnwH9KRaMB4O8IDicD5PQVxVMLATyDKK3nBcXD13N27MeWFm2M+6hvaxA3e43pg== X-Received: by 2002:a05:6870:4786:b0:29f:c5f3:a827 with SMTP id 586e51a60fabf-2a7b32d1f65mr4039034fac.35.1734627671925; Thu, 19 Dec 2024 09:01:11 -0800 (PST) Received: from [192.168.0.142] (ip98-183-112-25.ok.ok.cox.net. [98.183.112.25]) by smtp.gmail.com with ESMTPSA id 586e51a60fabf-2a7d74c68e5sm391020fac.20.2024.12.19.09.01.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 19 Dec 2024 09:01:11 -0800 (PST) Message-ID: <94efa413-5fa9-4014-86c2-331442e9d42e@baylibre.com> Date: Thu, 19 Dec 2024 11:01:09 -0600 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 4/8] iio: backend: add API for interface configuration To: Jonathan Cameron , =?UTF-8?Q?Nuno_S=C3=A1?= Cc: Angelo Dureghello , Lars-Peter Clausen , Michael Hennerich , Mihail Chindris , Nuno Sa , Olivier Moysan , Jonathan Cameron , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, Antoniu Miclaus References: <20241216-wip-bl-ad3552r-axi-v0-iio-testing-carlos-v1-0-856ff71fc930@baylibre.com> <20241216-wip-bl-ad3552r-axi-v0-iio-testing-carlos-v1-4-856ff71fc930@baylibre.com> <20241219164233.087ff9cb@jic23-huawei> <20241219165115.23717a71@jic23-huawei> From: David Lechner Content-Language: en-US In-Reply-To: <20241219165115.23717a71@jic23-huawei> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 12/19/24 10:51 AM, Jonathan Cameron wrote: > On Thu, 19 Dec 2024 16:42:33 +0000 > Jonathan Cameron wrote: > >> On Tue, 17 Dec 2024 11:13:59 +0100 >> Nuno Sá wrote: >> >>> On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote: >>>> From: Antoniu Miclaus >>>> >>>> Add backend support for setting and getting the interface type >>>> in use. >>>> >>>> Link: >>>> https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b >>>> Signed-off-by: Antoniu Miclaus >>>> Co-developed-by: Angelo Dureghello >>>> Signed-off-by: Angelo Dureghello >>>> --- >>>> This patch has been picked up from the Antoniu patchset >>>> still not accepted, and extended with the interface setter, >>>> fixing also namespace names to be between quotation marks. >>>> --- >>>>  drivers/iio/industrialio-backend.c | 42 >>>> ++++++++++++++++++++++++++++++++++++++ >>>>  include/linux/iio/backend.h        | 19 +++++++++++++++++ >>>>  2 files changed, 61 insertions(+) >>>> >>>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- >>>> backend.c >>>> index 363281272035..6edc3e685f6a 100644 >>>> --- a/drivers/iio/industrialio-backend.c >>>> +++ b/drivers/iio/industrialio-backend.c >>>> @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev >>>> *indio_dev, uintptr_t private, >>>>  } >>>>  EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND"); >>>>   >>>> +/** >>>> + * iio_backend_interface_type_get - get the interface type used. >>>> + * @back: Backend device >>>> + * @type: Interface type >>>> + * >>>> + * RETURNS: >>>> + * 0 on success, negative error number on failure. >>>> + */ >>>> +int iio_backend_interface_type_get(struct iio_backend *back, >>>> +    enum iio_backend_interface_type *type) >>>> +{ >>>> + int ret; >>>> + >>>> + ret = iio_backend_op_call(back, interface_type_get, type); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (*type >= IIO_BACKEND_INTERFACE_MAX) >>>> + return -EINVAL; >>>> + >>>> + return 0; >>>> +} >>>> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND"); >>>> + >>>> +/** >>>> + * iio_backend_interface_type_set - set the interface type used. >>>> + * @back: Backend device >>>> + * @type: Interface type >>>> + * >>>> + * RETURNS: >>>> + * 0 on success, negative error number on failure. >>>> + */ >>>> +int iio_backend_interface_type_set(struct iio_backend *back, >>>> +    enum iio_backend_interface_type type) >>>> +{ >>>> + if (type >= IIO_BACKEND_INTERFACE_MAX) >>>> + return -EINVAL; >>>> + >>>> + return  iio_backend_op_call(back, interface_type_set, type); >>>> +} >>>> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND"); >>>> + >>>>  /** >>>>   * iio_backend_extend_chan_spec - Extend an IIO channel >>>>   * @back: Backend device >>>> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h >>>> index 10be00f3b120..2b7221099d8c 100644 >>>> --- a/include/linux/iio/backend.h >>>> +++ b/include/linux/iio/backend.h >>>> @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger { >>>>   IIO_BACKEND_SAMPLE_TRIGGER_MAX >>>>  }; >>>>   >>>> +enum iio_backend_interface_type { >>>> + IIO_BACKEND_INTERFACE_SERIAL_LVDS, >>>> + IIO_BACKEND_INTERFACE_SERIAL_CMOS, >>> >>> The above are apparently not used in the next patch so I would not add them now. >>>> + IIO_BACKEND_INTERFACE_SERIAL_SPI, >>>> + IIO_BACKEND_INTERFACE_SERIAL_DSPI, >>>> + IIO_BACKEND_INTERFACE_SERIAL_QSPI, >>> >>> I'll throw my 2 cents but it would be nice to have more feedback on this. I'm >>> not completely sure about the xSPI stuff in here. We treated the QSPI as a bus >>> both for control and data in which we also add child devices. And we've been >>> adding specific bus operations/configurations through the 'struct >>> ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not >>> be set through that interface? >> >> Maybe - kind of hard to tell until we actually have code. >> I'd go for kicking them into the long grass for now if they aren't used and >> just dropping them from this patch. If we ever need them,easy to bring back >> and then we should have a justification for why! > > oops. Misread. Obviously Nuno was saying the ones above aren't used, not the > SPI ones... I don't feel strongly either way on setting these via > this generic interface, or via the other path. > > Jonathan > >> >> J >> >> >>> >>> LVDS/CMOS still looks slightly different to me... >>> >>> - Nuno Sá >>> >>> >>> >> >> > I'm the one that suggested doing it this way to Angelo, so I'll chime in with my thinking. In Angelo's previous series where we added IIO backend support for this family of chips, in the devicetree discussion, we considered the possibility of two SPI controllers, one for register access and one for the high-speed streaming provided by the backend. Since the dual and quad SPI here is for IIO backend (the high-speed sample data interface) is what made me think we should put the control here rather than putting it in the platform data interface. Since this is new HDL, maybe we could avoid this issue altogether and tweak the implementation in the FPGA a bit. Clearly we had the AD3552R working without needing to poke this registers, so why can't we do the same for the new chip? In other words, make these things a compile time option in the HDL rather than a runtime option?