Linux IIO development
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Piyush Patle <piyushpatle228@gmail.com>,
	ak@it-klinger.de, dlechner@baylibre.com, nuno.sa@analog.com,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 11/11] iio: adc: hx711: add support for HX710B
Date: Mon, 11 May 2026 15:26:24 +0100	[thread overview]
Message-ID: <20260511152624.352058d6@jic23-huawei> (raw)
In-Reply-To: <agG9FAXXiTKDIEDf@ashevche-desk.local>

On Mon, 11 May 2026 14:27:16 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, May 11, 2026 at 01:56:55AM +0530, Piyush Patle wrote:
> > Add support for the AVIA HX710B ADC, which shares the HX711 GPIO
> > interface but uses trailing PD_SCK pulses to select the active mode.
> > 
> > Model the HX710B with variant-specific channel tables and IIO info,
> > track the active channel across conversions, and use the fixed gain
> > value when computing scale.
> > 
> > Also update the adjacent Kconfig text, file header, and module
> > description so the driver text matches the newly supported variant.  
> 
> ...
> 
> >  #include <linux/slab.h>
> >  #include <linux/sched.h>  
> 
> >  #include <linux/delay.h>
> > +#include <linux/types.h>  
> 
> Seems wrong order.
> 
> And here + blank line to make linux/iio/* to be a separate group.
> 
> >  #include <linux/iio/iio.h>
> >  #include <linux/iio/sysfs.h>
> >  #include <linux/iio/buffer.h>  
> 
> ...
> 
> >  	/*
> >  	 * triggered buffer
> > -	 * 2x32-bit channel + 64-bit naturally aligned timestamp
> > +	 * up to 3x32-bit channels + 64-bit naturally aligned timestamp
> > +	 *
> > +	 * aligned_s64 satisfies the 8-byte alignment requirement for the
> > +	 * timestamp. For HX711 (at most 2 active channels), iio_push_to_
> > +	 * buffers_with_timestamp() places the timestamp at offset 8
> > +	 * (scan_bytes=8, already 8-byte aligned), identical to the original
> > +	 * 2-channel layout. The extra channel slot for HX710B does not affect
> > +	 * the HX711 ABI.
> >  	 */
> >  	struct {
> > -		u32 channel[2];
> > +		u32 channel[3];
> >  		aligned_s64 timestamp;
> >  	} buffer;  
> 
> Why can't we used a recently introduced macro for this?
> IIO_DECLARE_BUFFER_WITH_TS().
Putting it more strongly - we should be using that macro.
The timestamp is only there if all 3 channels are enabled
and present on a given device.  So this structure is actively missleading
and hence the need for the complicated comment.

Just remove that implication by using the more flexible buffer declaration
macro that Andy is suggesting.  We have that macro for precisely this case!

Jonathan


> 


      parent reply	other threads:[~2026-05-11 14:26 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-10 19:49 [PATCH v7 00/11] iio: adc: hx711: add HX710B support Piyush Patle
2026-05-10 19:49 ` [PATCH v7 01/11] dt-bindings: iio: adc: hx711: clean up existing binding text Piyush Patle
2026-05-11 16:08   ` Conor Dooley
2026-05-10 19:49 ` [PATCH v7 02/11] dt-bindings: iio: adc: hx711: add VSUP supply property Piyush Patle
2026-05-11 16:09   ` Conor Dooley
2026-05-10 19:49 ` [PATCH v7 03/11] dt-bindings: iio: adc: hx711: add RATE GPIO property Piyush Patle
2026-05-11 16:09   ` Conor Dooley
2026-05-10 19:49 ` [PATCH v7 04/11] dt-bindings: iio: adc: hx711: add HX710B support Piyush Patle
2026-05-11 16:09   ` Conor Dooley
2026-05-10 19:49 ` [PATCH v7 05/11] iio: adc: hx711: move scale computation to per-device storage Piyush Patle
2026-05-11 11:19   ` Andy Shevchenko
2026-05-11 13:45     ` Piyush Patle
2026-05-10 19:49 ` [PATCH v7 06/11] iio: adc: hx711: introduce hx711_chip_info structure Piyush Patle
2026-05-11 14:33   ` Jonathan Cameron
2026-05-10 19:49 ` [PATCH v7 07/11] iio: adc: hx711: pass trailing pulse count into hx711_read Piyush Patle
2026-05-10 19:49 ` [PATCH v7 08/11] iio: adc: hx711: split variable assignments in hx711_read and hx711_reset Piyush Patle
2026-05-11 11:22   ` Andy Shevchenko
2026-05-11 14:34     ` Jonathan Cameron
2026-05-10 19:49 ` [PATCH v7 09/11] iio: adc: hx711: localize loop iterators in hx711_read Piyush Patle
2026-05-10 19:49 ` [PATCH v7 10/11] iio: adc: hx711: pass iio_chan_spec to hx711_reset_read Piyush Patle
2026-05-10 20:26 ` [PATCH v7 11/11] iio: adc: hx711: add support for HX710B Piyush Patle
2026-05-11 11:27   ` Andy Shevchenko
2026-05-11 13:49     ` Piyush Patle
2026-05-11 14:59       ` Jonathan Cameron
2026-05-11 14:26     ` Jonathan Cameron [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260511152624.352058d6@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=ak@it-klinger.de \
    --cc=andriy.shevchenko@intel.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=piyushpatle228@gmail.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox