[PATCH] iio: temperature: maxim_thermocouple: use IIO_DECLARE_DMA_BUFFER_WITH_TS()

David Lechner posted 1 patch 2 months, 3 weeks ago
drivers/iio/temperature/maxim_thermocouple.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[PATCH] iio: temperature: maxim_thermocouple: use IIO_DECLARE_DMA_BUFFER_WITH_TS()
Posted by David Lechner 2 months, 3 weeks ago
Use IIO_DECLARE_DMA_BUFFER_WITH_TS() to declare the buffer used for
scan data. There was not any documentation explaining the buffer layout
previously, and this makes it self-documenting. The element type is
also changed to __be16 to match the format of the data read from the
device. This way, the count argument to IIO_DECLARE_DMA_BUFFER_WITH_TS()
is just the number of channels rather than needing to calculate the
number of bytes required.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/temperature/maxim_thermocouple.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
index cae8e84821d7fd521d59432580d51def939fa4d1..60eaa8140d3e98e10c073c8d18cf01524b1c1816 100644
--- a/drivers/iio/temperature/maxim_thermocouple.c
+++ b/drivers/iio/temperature/maxim_thermocouple.c
@@ -18,6 +18,8 @@
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
 
+#include <asm/byteorder.h>
+
 #define MAXIM_THERMOCOUPLE_DRV_NAME	"maxim_thermocouple"
 
 enum {
@@ -121,8 +123,8 @@ struct maxim_thermocouple_data {
 	struct spi_device *spi;
 	const struct maxim_thermocouple_chip *chip;
 	char tc_type;
-
-	u8 buffer[16] __aligned(IIO_DMA_MINALIGN);
+	/* Buffer for reading up to 2 hardware channels. */
+	IIO_DECLARE_DMA_BUFFER_WITH_TS(__be16, buffer, 2);
 };
 
 static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,

---
base-commit: f8f559752d573a051a984adda8d2d1464f92f954
change-id: 20250711-iio-use-more-iio_declare_buffer_with_ts-3-2cc387a66bdc

Best regards,
-- 
David Lechner <dlechner@baylibre.com>
Re: [PATCH] iio: temperature: maxim_thermocouple: use IIO_DECLARE_DMA_BUFFER_WITH_TS()
Posted by Andy Shevchenko 2 months, 3 weeks ago
On Fri, Jul 11, 2025 at 10:33:55AM -0500, David Lechner wrote:
> Use IIO_DECLARE_DMA_BUFFER_WITH_TS() to declare the buffer used for
> scan data. There was not any documentation explaining the buffer layout
> previously, and this makes it self-documenting. The element type is
> also changed to __be16 to match the format of the data read from the
> device. This way, the count argument to IIO_DECLARE_DMA_BUFFER_WITH_TS()
> is just the number of channels rather than needing to calculate the
> number of bytes required.

...

> +#include <asm/byteorder.h>

Hmm... I see nothing about this change in the commit message.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] iio: temperature: maxim_thermocouple: use IIO_DECLARE_DMA_BUFFER_WITH_TS()
Posted by David Lechner 2 months, 3 weeks ago
On 7/11/25 11:41 AM, Andy Shevchenko wrote:
> On Fri, Jul 11, 2025 at 10:33:55AM -0500, David Lechner wrote:
>> Use IIO_DECLARE_DMA_BUFFER_WITH_TS() to declare the buffer used for
>> scan data. There was not any documentation explaining the buffer layout
>> previously, and this makes it self-documenting. The element type is
>> also changed to __be16 to match the format of the data read from the
>> device. This way, the count argument to IIO_DECLARE_DMA_BUFFER_WITH_TS()
>> is just the number of channels rather than needing to calculate the
>> number of bytes required.
> 
> ...
> 
>> +#include <asm/byteorder.h>
> 
> Hmm... I see nothing about this change in the commit message.
> 

It is for __be16. I kind of assumed that would be obvious, but sure,
better to be explicit about it.
Re: [PATCH] iio: temperature: maxim_thermocouple: use IIO_DECLARE_DMA_BUFFER_WITH_TS()
Posted by Andy Shevchenko 2 months, 3 weeks ago
On Fri, Jul 11, 2025 at 12:04:03PM -0500, David Lechner wrote:
> On 7/11/25 11:41 AM, Andy Shevchenko wrote:
> > On Fri, Jul 11, 2025 at 10:33:55AM -0500, David Lechner wrote:

...

> >> +#include <asm/byteorder.h>
> > 
> > Hmm... I see nothing about this change in the commit message.
> 
> It is for __be16. I kind of assumed that would be obvious, but sure,
> better to be explicit about it.

Isn't it in types.h?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] iio: temperature: maxim_thermocouple: use IIO_DECLARE_DMA_BUFFER_WITH_TS()
Posted by David Lechner 2 months, 3 weeks ago
On 7/11/25 12:50 PM, Andy Shevchenko wrote:
> On Fri, Jul 11, 2025 at 12:04:03PM -0500, David Lechner wrote:
>> On 7/11/25 11:41 AM, Andy Shevchenko wrote:
>>> On Fri, Jul 11, 2025 at 10:33:55AM -0500, David Lechner wrote:
> 
> ...
> 
>>>> +#include <asm/byteorder.h>
>>>
>>> Hmm... I see nothing about this change in the commit message.
>>
>> It is for __be16. I kind of assumed that would be obvious, but sure,
>> better to be explicit about it.
> 
> Isn't it in types.h?
> 

No, only CPU-endian types are in types.h. The actual #define for
 __be16 is in include/uapi/linux/byteorder/big_endian.h. This is
included in one driver in staging, otherwise it is only included
in arch/*/include/uapi/asm/byteorder.h. And asm/byteorder.h is what
Jonathan used for similar in some of his recent IWYU patches as well,
so I assume that is the preferred way to do it.
Re: [PATCH] iio: temperature: maxim_thermocouple: use IIO_DECLARE_DMA_BUFFER_WITH_TS()
Posted by Jonathan Cameron 2 months, 3 weeks ago
On Fri, 11 Jul 2025 13:38:17 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 7/11/25 12:50 PM, Andy Shevchenko wrote:
> > On Fri, Jul 11, 2025 at 12:04:03PM -0500, David Lechner wrote:  
> >> On 7/11/25 11:41 AM, Andy Shevchenko wrote:  
> >>> On Fri, Jul 11, 2025 at 10:33:55AM -0500, David Lechner wrote:  
> > 
> > ...
> >   
> >>>> +#include <asm/byteorder.h>  
> >>>
> >>> Hmm... I see nothing about this change in the commit message.  
> >>
> >> It is for __be16. I kind of assumed that would be obvious, but sure,
> >> better to be explicit about it.  
> > 
> > Isn't it in types.h?
> >   
> 
> No, only CPU-endian types are in types.h. The actual #define for
>  __be16 is in include/uapi/linux/byteorder/big_endian.h. This is
> included in one driver in staging, otherwise it is only included
> in arch/*/include/uapi/asm/byteorder.h. And asm/byteorder.h is what
> Jonathan used for similar in some of his recent IWYU patches as well,
> so I assume that is the preferred way to do it.
> 
Never trust me :)  I may have been after be16_to_cpu() or similar
though rather than the type. Can't remember. When I get back to those
I'll take a look at the logs.
Re: [PATCH] iio: temperature: maxim_thermocouple: use IIO_DECLARE_DMA_BUFFER_WITH_TS()
Posted by David Lechner 2 months, 3 weeks ago
On 7/13/25 9:10 AM, Jonathan Cameron wrote:
> On Fri, 11 Jul 2025 13:38:17 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
>> On 7/11/25 12:50 PM, Andy Shevchenko wrote:
>>> On Fri, Jul 11, 2025 at 12:04:03PM -0500, David Lechner wrote:  
>>>> On 7/11/25 11:41 AM, Andy Shevchenko wrote:  
>>>>> On Fri, Jul 11, 2025 at 10:33:55AM -0500, David Lechner wrote:  
>>>
>>> ...
>>>   
>>>>>> +#include <asm/byteorder.h>  
>>>>>
>>>>> Hmm... I see nothing about this change in the commit message.  
>>>>
>>>> It is for __be16. I kind of assumed that would be obvious, but sure,
>>>> better to be explicit about it.  
>>>
>>> Isn't it in types.h?
>>>   
>>
>> No, only CPU-endian types are in types.h. The actual #define for
>>  __be16 is in include/uapi/linux/byteorder/big_endian.h. This is
>> included in one driver in staging, otherwise it is only included
>> in arch/*/include/uapi/asm/byteorder.h. And asm/byteorder.h is what
>> Jonathan used for similar in some of his recent IWYU patches as well,
>> so I assume that is the preferred way to do it.
>>
> Never trust me :)  I may have been after be16_to_cpu() or similar
> though rather than the type. Can't remember. When I get back to those
> I'll take a look at the logs.
> 
> 

Yes, you used asm/byteorder for le16_to_cpup() then I (incorrectly)
made the leap that e.g. __le16 would come from there as well.
Re: [PATCH] iio: temperature: maxim_thermocouple: use IIO_DECLARE_DMA_BUFFER_WITH_TS()
Posted by Andy Shevchenko 2 months, 3 weeks ago
On Fri, Jul 11, 2025 at 01:38:17PM -0500, David Lechner wrote:
> On 7/11/25 12:50 PM, Andy Shevchenko wrote:
> > On Fri, Jul 11, 2025 at 12:04:03PM -0500, David Lechner wrote:
> >> On 7/11/25 11:41 AM, Andy Shevchenko wrote:
> >>> On Fri, Jul 11, 2025 at 10:33:55AM -0500, David Lechner wrote:

...

> >>>> +#include <asm/byteorder.h>
> >>>
> >>> Hmm... I see nothing about this change in the commit message.
> >>
> >> It is for __be16. I kind of assumed that would be obvious, but sure,
> >> better to be explicit about it.
> > 
> > Isn't it in types.h?
> 
> No, only CPU-endian types are in types.h. The actual #define for
>  __be16 is in include/uapi/linux/byteorder/big_endian.h.

Huh?!

> This is
> included in one driver in staging, otherwise it is only included
> in arch/*/include/uapi/asm/byteorder.h. And asm/byteorder.h is what
> Jonathan used for similar in some of his recent IWYU patches as well,
> so I assume that is the preferred way to do it.

include/uapi/linux/types.h:37:typedef __u16 __bitwise __be16;

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] iio: temperature: maxim_thermocouple: use IIO_DECLARE_DMA_BUFFER_WITH_TS()
Posted by David Lechner 2 months, 3 weeks ago
On 7/11/25 1:50 PM, Andy Shevchenko wrote:
> On Fri, Jul 11, 2025 at 01:38:17PM -0500, David Lechner wrote:
>> On 7/11/25 12:50 PM, Andy Shevchenko wrote:
>>> On Fri, Jul 11, 2025 at 12:04:03PM -0500, David Lechner wrote:
>>>> On 7/11/25 11:41 AM, Andy Shevchenko wrote:
>>>>> On Fri, Jul 11, 2025 at 10:33:55AM -0500, David Lechner wrote:
> 
> ...
> 
>>>>>> +#include <asm/byteorder.h>
>>>>>
>>>>> Hmm... I see nothing about this change in the commit message.
>>>>
>>>> It is for __be16. I kind of assumed that would be obvious, but sure,
>>>> better to be explicit about it.
>>>
>>> Isn't it in types.h?
>>
>> No, only CPU-endian types are in types.h. The actual #define for
>>  __be16 is in include/uapi/linux/byteorder/big_endian.h.
> 
> Huh?!
> 
>> This is
>> included in one driver in staging, otherwise it is only included
>> in arch/*/include/uapi/asm/byteorder.h. And asm/byteorder.h is what
>> Jonathan used for similar in some of his recent IWYU patches as well,
>> so I assume that is the preferred way to do it.
> 
> include/uapi/linux/types.h:37:typedef __u16 __bitwise __be16;
> 

I see. I looked for #define first and found it so didn't go
looking for typedef. types.h it is.