* [PATCH v1 2/2] iio: adc: hx711: add support for HX710B @ 2026-04-18 17:06 Piyush Patle 2026-04-18 22:05 ` David Lechner 2026-04-19 12:00 ` Jonathan Cameron 0 siblings, 2 replies; 4+ messages in thread From: Piyush Patle @ 2026-04-18 17:06 UTC (permalink / raw) To: jic23, ak; +Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel Refactor the driver around per-chip configuration so HX711 and HX710B can share the same core. Add HX710B channel definitions, pulse-count based channel selection, a variant-specific iio_info, and fixed-scale handling at probe. Update the Kconfig text and trim comments so the added support stays focused on the actual driver behavior. Signed-off-by: Piyush Patle <piyushpatle228@gmail.com> --- drivers/iio/adc/Kconfig | 9 +- drivers/iio/adc/hx711.c | 222 ++++++++++++++++++++++++++++++++-------- 2 files changed, 184 insertions(+), 47 deletions(-) diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 60038ae8dfc4..ddf981fa72a2 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -784,18 +784,21 @@ config HI8435 called hi8435. config HX711 - tristate "AVIA HX711 ADC for weight cells" + tristate "AVIA HX711 and HX710B ADC" depends on GPIOLIB select IIO_BUFFER select IIO_TRIGGERED_BUFFER help - If you say yes here you get support for AVIA HX711 ADC which is used - for weigh cells + If you say yes here you get support for AVIA HX711 and HX710B ADCs + which are used for bridge sensors such as weigh cells. This driver uses two GPIOs, one acts as the clock and controls the channel selection and gain, the other one is used for the measurement data + The HX710B is a variant with fixed gain and a different channel + selection scheme. + Currently the raw value is read from the chip and delivered. To get an actual weight one needs to subtract the zero offset and multiply by a scale factor. diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c index 1db8b68a8f64..cd251fa9f6b7 100644 --- a/drivers/iio/adc/hx711.c +++ b/drivers/iio/adc/hx711.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-or-later /* - * HX711: analog to digital converter for weight sensor module + * HX711/HX710B ADC driver * * Copyright (c) 2016 Andreas Klinger <ak@it-klinger.de> */ @@ -76,13 +76,34 @@ static int hx711_get_scale_to_gain(int scale) return -EINVAL; } +/** + * struct hx711_chip_info - per-variant static configuration + * @name: IIO device name + * @channels: channel specification + * @num_channels: number of channels + * @chan_pulse_count: trailing pulse count for fixed-gain variants + * @num_chan_pulses: number of pulse-count entries + * @reset_channel: default channel after reset + */ +struct hx711_chip_info { + const char *name; + const struct iio_chan_spec *channels; + int num_channels; + const int *chan_pulse_count; + int num_chan_pulses; + int reset_channel; +}; + struct hx711_data { - struct device *dev; - struct gpio_desc *gpiod_pd_sck; - struct gpio_desc *gpiod_dout; - int gain_set; /* gain set on device */ - int gain_chan_a; /* gain for channel A */ - struct mutex lock; + struct device *dev; + struct gpio_desc *gpiod_pd_sck; + struct gpio_desc *gpiod_dout; + int gain_set; /* HX711 */ + int gain_chan_a; /* HX711 channel A gain */ + int channel_set; /* HX710B */ + int scale; /* HX710B scale */ + const struct hx711_chip_info *chip_info; + struct mutex lock; /* * triggered buffer * 2x32-bit channel + 64-bit naturally aligned timestamp @@ -92,13 +113,10 @@ struct hx711_data { aligned_s64 timestamp; } buffer; /* - * delay after a rising edge on SCK until the data is ready DOUT - * this is dependent on the hx711 where the datasheet tells a - * maximum value of 100 ns - * but also on potential parasitic capacities on the wiring + * Delay after SCK rising edge before sampling DOUT. */ - u32 data_ready_delay_ns; - u32 clock_frequency; + u32 data_ready_delay_ns; + u32 clock_frequency; }; static int hx711_cycle(struct hx711_data *hx711_data) @@ -139,7 +157,11 @@ static int hx711_cycle(struct hx711_data *hx711_data) return gpiod_get_value(hx711_data->gpiod_dout); } -static int hx711_read(struct hx711_data *hx711_data) +/* + * Clock out 24 data bits and then send trailing pulses to select the + * next channel/gain state. + */ +static int hx711_read(struct hx711_data *hx711_data, int trailing_pulses) { int i, ret; int value = 0; @@ -158,7 +180,7 @@ static int hx711_read(struct hx711_data *hx711_data) value ^= 0x800000; - for (i = 0; i < hx711_get_gain_to_pulse(hx711_data->gain_set); i++) + for (i = 0; i < trailing_pulses; i++) hx711_cycle(hx711_data); return value; @@ -188,6 +210,7 @@ static int hx711_wait_for_ready(struct hx711_data *hx711_data) static int hx711_reset(struct hx711_data *hx711_data) { + const struct hx711_chip_info *info = hx711_data->chip_info; int val = hx711_wait_for_ready(hx711_data); if (val) { @@ -206,13 +229,17 @@ static int hx711_reset(struct hx711_data *hx711_data) val = hx711_wait_for_ready(hx711_data); - /* after a reset the gain is 128 */ - hx711_data->gain_set = HX711_RESET_GAIN; + /* Restore variant default after reset. */ + if (info->chan_pulse_count) + hx711_data->channel_set = info->reset_channel; + else + hx711_data->gain_set = HX711_RESET_GAIN; } return val; } +/* Select HX711 channel/gain for the next conversion. */ static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) { int ret; @@ -221,7 +248,8 @@ static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) if (hx711_data->gain_set == 32) { hx711_data->gain_set = hx711_data->gain_chan_a; - ret = hx711_read(hx711_data); + ret = hx711_read(hx711_data, + hx711_get_gain_to_pulse(hx711_data->gain_set)); if (ret < 0) return ret; @@ -233,7 +261,8 @@ static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) if (hx711_data->gain_set != 32) { hx711_data->gain_set = 32; - ret = hx711_read(hx711_data); + ret = hx711_read(hx711_data, + hx711_get_gain_to_pulse(hx711_data->gain_set)); if (ret < 0) return ret; @@ -246,27 +275,52 @@ static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) return 0; } +/* Select HX710B channel for the next conversion. */ +static int hx710b_set_channel(struct hx711_data *hx711_data, int chan) +{ + const struct hx711_chip_info *info = hx711_data->chip_info; + int ret; + + if (chan >= info->num_chan_pulses) + return -EINVAL; + + if (hx711_data->channel_set == chan) + return 0; + + hx711_data->channel_set = chan; + + ret = hx711_read(hx711_data, info->chan_pulse_count[chan]); + if (ret < 0) + return ret; + + return hx711_wait_for_ready(hx711_data); +} + static int hx711_reset_read(struct hx711_data *hx711_data, int chan) { + const struct hx711_chip_info *info = hx711_data->chip_info; + int trailing_pulses; int ret; - int val; - /* - * hx711_reset() must be called from here - * because it could be calling hx711_read() by itself - */ + /* Reset first so the read starts from a known chip state. */ if (hx711_reset(hx711_data)) { dev_err(hx711_data->dev, "reset failed!"); return -EIO; } - ret = hx711_set_gain_for_channel(hx711_data, chan); - if (ret < 0) - return ret; - - val = hx711_read(hx711_data); + if (info->chan_pulse_count) { + ret = hx710b_set_channel(hx711_data, chan); + if (ret < 0) + return ret; + trailing_pulses = info->chan_pulse_count[chan]; + } else { + ret = hx711_set_gain_for_channel(hx711_data, chan); + if (ret < 0) + return ret; + trailing_pulses = hx711_get_gain_to_pulse(hx711_data->gain_set); + } - return val; + return hx711_read(hx711_data, trailing_pulses); } static int hx711_read_raw(struct iio_dev *indio_dev, @@ -274,6 +328,7 @@ static int hx711_read_raw(struct iio_dev *indio_dev, int *val, int *val2, long mask) { struct hx711_data *hx711_data = iio_priv(indio_dev); + const struct hx711_chip_info *info = hx711_data->chip_info; switch (mask) { case IIO_CHAN_INFO_RAW: @@ -290,7 +345,10 @@ static int hx711_read_raw(struct iio_dev *indio_dev, *val = 0; mutex_lock(&hx711_data->lock); - *val2 = hx711_get_gain_to_scale(hx711_data->gain_set); + if (info->chan_pulse_count) + *val2 = hx711_data->scale; + else + *val2 = hx711_get_gain_to_scale(hx711_data->gain_set); mutex_unlock(&hx711_data->lock); @@ -332,7 +390,8 @@ static int hx711_write_raw(struct iio_dev *indio_dev, if (gain != 32) hx711_data->gain_chan_a = gain; - ret = hx711_read(hx711_data); + ret = hx711_read(hx711_data, + hx711_get_gain_to_pulse(hx711_data->gain_set)); if (ret < 0) { mutex_unlock(&hx711_data->lock); return ret; @@ -423,6 +482,10 @@ static const struct iio_info hx711_iio_info = { .attrs = &hx711_attribute_group, }; +static const struct iio_info hx710b_iio_info = { + .read_raw = hx711_read_raw, +}; + static const struct iio_chan_spec hx711_chan_spec[] = { { .type = IIO_VOLTAGE, @@ -455,10 +518,69 @@ static const struct iio_chan_spec hx711_chan_spec[] = { IIO_CHAN_SOFT_TIMESTAMP(2), }; +/* HX710B channels: differential input and DVDD-AVDD measurement. */ +static const struct iio_chan_spec hx710b_chan_spec[] = { + { + .type = IIO_VOLTAGE, + .channel = 0, + .indexed = 1, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE), + .scan_index = 0, + .scan_type = { + .sign = 'u', + .realbits = 24, + .storagebits = 32, + .endianness = IIO_CPU, + }, + }, + { + .type = IIO_VOLTAGE, + .channel = 1, + .indexed = 1, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE), + .scan_index = 1, + .scan_type = { + .sign = 'u', + .realbits = 24, + .storagebits = 32, + .endianness = IIO_CPU, + }, + }, + IIO_CHAN_SOFT_TIMESTAMP(2), +}; + +/* + * HX710B trailing pulse counts. + * 25 selects differential input, 26 selects DVDD-AVDD. + */ +static const int hx710b_pulse_counts[] = { + 25, /* channel 0: differential input, 10 SPS */ + 26, /* channel 1: DVDD-AVDD voltage, 40 SPS */ +}; + +static const struct hx711_chip_info hx711_chip = { + .name = "hx711", + .channels = hx711_chan_spec, + .num_channels = ARRAY_SIZE(hx711_chan_spec), + .chan_pulse_count = NULL, +}; + +static const struct hx711_chip_info hx710b_chip = { + .name = "hx710b", + .channels = hx710b_chan_spec, + .num_channels = ARRAY_SIZE(hx710b_chan_spec), + .chan_pulse_count = hx710b_pulse_counts, + .num_chan_pulses = ARRAY_SIZE(hx710b_pulse_counts), + .reset_channel = 0, +}; + static int hx711_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct hx711_data *hx711_data; + const struct hx711_chip_info *chip_info; struct iio_dev *indio_dev; int ret; int i; @@ -472,6 +594,11 @@ static int hx711_probe(struct platform_device *pdev) mutex_init(&hx711_data->lock); + chip_info = device_get_match_data(dev); + if (!chip_info) + return -ENODEV; + hx711_data->chip_info = chip_info; + /* * PD_SCK stands for power down and serial clock input of HX711 * in the driver it is an output @@ -510,12 +637,20 @@ static int hx711_probe(struct platform_device *pdev) /* we need 10^-9 mV */ ret *= 100; - for (i = 0; i < HX711_GAIN_MAX; i++) - hx711_gain_to_scale[i].scale = - ret / hx711_gain_to_scale[i].gain / 1678; + if (chip_info->chan_pulse_count) { + /* HX710B uses a fixed gain, so compute scale once at probe. */ + hx711_data->scale = ret / 128 / 1678; + hx711_data->channel_set = chip_info->reset_channel; + indio_dev->info = &hx710b_iio_info; + } else { + for (i = 0; i < HX711_GAIN_MAX; i++) + hx711_gain_to_scale[i].scale = + ret / hx711_gain_to_scale[i].gain / 1678; - hx711_data->gain_set = 128; - hx711_data->gain_chan_a = 128; + hx711_data->gain_set = 128; + hx711_data->gain_chan_a = 128; + indio_dev->info = &hx711_iio_info; + } hx711_data->clock_frequency = 400000; ret = device_property_read_u32(&pdev->dev, "clock-frequency", @@ -533,11 +668,10 @@ static int hx711_probe(struct platform_device *pdev) hx711_data->data_ready_delay_ns = 1000000000 / hx711_data->clock_frequency; - indio_dev->name = "hx711"; - indio_dev->info = &hx711_iio_info; + indio_dev->name = chip_info->name; indio_dev->modes = INDIO_DIRECT_MODE; - indio_dev->channels = hx711_chan_spec; - indio_dev->num_channels = ARRAY_SIZE(hx711_chan_spec); + indio_dev->channels = chip_info->channels; + indio_dev->num_channels = chip_info->num_channels; ret = devm_iio_triggered_buffer_setup(dev, indio_dev, iio_pollfunc_store_time, @@ -554,7 +688,8 @@ static int hx711_probe(struct platform_device *pdev) } static const struct of_device_id of_hx711_match[] = { - { .compatible = "avia,hx711", }, + { .compatible = "avia,hx711", .data = &hx711_chip }, + { .compatible = "avia,hx710b", .data = &hx710b_chip }, { } }; @@ -571,7 +706,6 @@ static struct platform_driver hx711_driver = { module_platform_driver(hx711_driver); MODULE_AUTHOR("Andreas Klinger <ak@it-klinger.de>"); -MODULE_DESCRIPTION("HX711 bitbanging driver - ADC for weight cells"); +MODULE_DESCRIPTION("HX711/HX710B GPIO ADC driver"); MODULE_LICENSE("GPL"); MODULE_ALIAS("platform:hx711-gpio"); - -- 2.43.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1 2/2] iio: adc: hx711: add support for HX710B 2026-04-18 17:06 [PATCH v1 2/2] iio: adc: hx711: add support for HX710B Piyush Patle @ 2026-04-18 22:05 ` David Lechner 2026-04-19 9:14 ` Piyush Patle 2026-04-19 12:00 ` Jonathan Cameron 1 sibling, 1 reply; 4+ messages in thread From: David Lechner @ 2026-04-18 22:05 UTC (permalink / raw) To: Piyush Patle, jic23, ak; +Cc: nuno.sa, andy, linux-iio, linux-kernel When there is more than one patch in a series, add a cover letter to keep the patches together. On 4/18/26 12:06 PM, Piyush Patle wrote: > Refactor the driver around per-chip configuration so HX711 and HX710B > can share the same core. > > Add HX710B channel definitions, pulse-count based channel selection, a Can you explain this difference a bit more? It seems like the two chips are quite different in this regard. > variant-specific iio_info, and fixed-scale handling at probe. Update the > Kconfig text and trim comments so the added support stays focused on the > actual driver behavior. > > Signed-off-by: Piyush Patle <piyushpatle228@gmail.com> > --- > drivers/iio/adc/Kconfig | 9 +- > drivers/iio/adc/hx711.c | 222 ++++++++++++++++++++++++++++++++-------- > 2 files changed, 184 insertions(+), 47 deletions(-) > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 60038ae8dfc4..ddf981fa72a2 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -784,18 +784,21 @@ config HI8435 > called hi8435. > > config HX711 > - tristate "AVIA HX711 ADC for weight cells" > + tristate "AVIA HX711 and HX710B ADC" > depends on GPIOLIB > select IIO_BUFFER > select IIO_TRIGGERED_BUFFER > help > - If you say yes here you get support for AVIA HX711 ADC which is used > - for weigh cells > + If you say yes here you get support for AVIA HX711 and HX710B ADCs > + which are used for bridge sensors such as weigh cells. > > This driver uses two GPIOs, one acts as the clock and controls the > channel selection and gain, the other one is used for the measurement > data > > + The HX710B is a variant with fixed gain and a different channel > + selection scheme. > + > Currently the raw value is read from the chip and delivered. > To get an actual weight one needs to subtract the > zero offset and multiply by a scale factor. > diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c > index 1db8b68a8f64..cd251fa9f6b7 100644 > --- a/drivers/iio/adc/hx711.c > +++ b/drivers/iio/adc/hx711.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > - * HX711: analog to digital converter for weight sensor module > + * HX711/HX710B ADC driver > * > * Copyright (c) 2016 Andreas Klinger <ak@it-klinger.de> > */ > @@ -76,13 +76,34 @@ static int hx711_get_scale_to_gain(int scale) > return -EINVAL; > } > > +/** > + * struct hx711_chip_info - per-variant static configuration > + * @name: IIO device name > + * @channels: channel specification > + * @num_channels: number of channels > + * @chan_pulse_count: trailing pulse count for fixed-gain variants > + * @num_chan_pulses: number of pulse-count entries > + * @reset_channel: default channel after reset > + */ > +struct hx711_chip_info { > + const char *name; > + const struct iio_chan_spec *channels; > + int num_channels; > + const int *chan_pulse_count; > + int num_chan_pulses; > + int reset_channel; > +}; > + > struct hx711_data { > - struct device *dev; > - struct gpio_desc *gpiod_pd_sck; > - struct gpio_desc *gpiod_dout; > - int gain_set; /* gain set on device */ > - int gain_chan_a; /* gain for channel A */ > - struct mutex lock; > + struct device *dev; > + struct gpio_desc *gpiod_pd_sck; > + struct gpio_desc *gpiod_dout; > + int gain_set; /* HX711 */ > + int gain_chan_a; /* HX711 channel A gain */ > + int channel_set; /* HX710B */ > + int scale; /* HX710B scale */ > + const struct hx711_chip_info *chip_info; > + struct mutex lock; > /* > * triggered buffer > * 2x32-bit channel + 64-bit naturally aligned timestamp > @@ -92,13 +113,10 @@ struct hx711_data { > aligned_s64 timestamp; > } buffer; > /* > - * delay after a rising edge on SCK until the data is ready DOUT > - * this is dependent on the hx711 where the datasheet tells a > - * maximum value of 100 ns > - * but also on potential parasitic capacities on the wiring > + * Delay after SCK rising edge before sampling DOUT. > */ > - u32 data_ready_delay_ns; > - u32 clock_frequency; > + u32 data_ready_delay_ns; > + u32 clock_frequency; > }; > > static int hx711_cycle(struct hx711_data *hx711_data) > @@ -139,7 +157,11 @@ static int hx711_cycle(struct hx711_data *hx711_data) > return gpiod_get_value(hx711_data->gpiod_dout); > } > > -static int hx711_read(struct hx711_data *hx711_data) > +/* > + * Clock out 24 data bits and then send trailing pulses to select the > + * next channel/gain state. > + */ > +static int hx711_read(struct hx711_data *hx711_data, int trailing_pulses) > { > int i, ret; > int value = 0; > @@ -158,7 +180,7 @@ static int hx711_read(struct hx711_data *hx711_data) > > value ^= 0x800000; > > - for (i = 0; i < hx711_get_gain_to_pulse(hx711_data->gain_set); i++) > + for (i = 0; i < trailing_pulses; i++) > hx711_cycle(hx711_data); > > return value; > @@ -188,6 +210,7 @@ static int hx711_wait_for_ready(struct hx711_data *hx711_data) > > static int hx711_reset(struct hx711_data *hx711_data) > { > + const struct hx711_chip_info *info = hx711_data->chip_info; > int val = hx711_wait_for_ready(hx711_data); > > if (val) { > @@ -206,13 +229,17 @@ static int hx711_reset(struct hx711_data *hx711_data) > > val = hx711_wait_for_ready(hx711_data); > > - /* after a reset the gain is 128 */ > - hx711_data->gain_set = HX711_RESET_GAIN; > + /* Restore variant default after reset. */ > + if (info->chan_pulse_count) > + hx711_data->channel_set = info->reset_channel; > + else > + hx711_data->gain_set = HX711_RESET_GAIN; > } > > return val; > } > > +/* Select HX711 channel/gain for the next conversion. */ > static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) > { > int ret; > @@ -221,7 +248,8 @@ static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) > if (hx711_data->gain_set == 32) { > hx711_data->gain_set = hx711_data->gain_chan_a; > > - ret = hx711_read(hx711_data); > + ret = hx711_read(hx711_data, > + hx711_get_gain_to_pulse(hx711_data->gain_set)); > if (ret < 0) > return ret; > > @@ -233,7 +261,8 @@ static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) > if (hx711_data->gain_set != 32) { > hx711_data->gain_set = 32; > > - ret = hx711_read(hx711_data); > + ret = hx711_read(hx711_data, > + hx711_get_gain_to_pulse(hx711_data->gain_set)); > if (ret < 0) > return ret; > > @@ -246,27 +275,52 @@ static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) > return 0; > } > > +/* Select HX710B channel for the next conversion. */ > +static int hx710b_set_channel(struct hx711_data *hx711_data, int chan) > +{ > + const struct hx711_chip_info *info = hx711_data->chip_info; > + int ret; > + > + if (chan >= info->num_chan_pulses) > + return -EINVAL; > + > + if (hx711_data->channel_set == chan) > + return 0; > + > + hx711_data->channel_set = chan; > + > + ret = hx711_read(hx711_data, info->chan_pulse_count[chan]); > + if (ret < 0) > + return ret; > + > + return hx711_wait_for_ready(hx711_data); > +} > + > static int hx711_reset_read(struct hx711_data *hx711_data, int chan) > { > + const struct hx711_chip_info *info = hx711_data->chip_info; > + int trailing_pulses; > int ret; > - int val; > > - /* > - * hx711_reset() must be called from here > - * because it could be calling hx711_read() by itself > - */ > + /* Reset first so the read starts from a known chip state. */ > if (hx711_reset(hx711_data)) { > dev_err(hx711_data->dev, "reset failed!"); > return -EIO; > } > > - ret = hx711_set_gain_for_channel(hx711_data, chan); > - if (ret < 0) > - return ret; > - > - val = hx711_read(hx711_data); > + if (info->chan_pulse_count) { > + ret = hx710b_set_channel(hx711_data, chan); > + if (ret < 0) > + return ret; > + trailing_pulses = info->chan_pulse_count[chan]; > + } else { > + ret = hx711_set_gain_for_channel(hx711_data, chan); > + if (ret < 0) > + return ret; > + trailing_pulses = hx711_get_gain_to_pulse(hx711_data->gain_set); > + } > > - return val; > + return hx711_read(hx711_data, trailing_pulses); > } > > static int hx711_read_raw(struct iio_dev *indio_dev, > @@ -274,6 +328,7 @@ static int hx711_read_raw(struct iio_dev *indio_dev, > int *val, int *val2, long mask) > { > struct hx711_data *hx711_data = iio_priv(indio_dev); > + const struct hx711_chip_info *info = hx711_data->chip_info; > > switch (mask) { > case IIO_CHAN_INFO_RAW: > @@ -290,7 +345,10 @@ static int hx711_read_raw(struct iio_dev *indio_dev, > *val = 0; > mutex_lock(&hx711_data->lock); > > - *val2 = hx711_get_gain_to_scale(hx711_data->gain_set); > + if (info->chan_pulse_count) > + *val2 = hx711_data->scale; > + else > + *val2 = hx711_get_gain_to_scale(hx711_data->gain_set); > > mutex_unlock(&hx711_data->lock); > > @@ -332,7 +390,8 @@ static int hx711_write_raw(struct iio_dev *indio_dev, > if (gain != 32) > hx711_data->gain_chan_a = gain; > > - ret = hx711_read(hx711_data); > + ret = hx711_read(hx711_data, > + hx711_get_gain_to_pulse(hx711_data->gain_set)); > if (ret < 0) { > mutex_unlock(&hx711_data->lock); > return ret; > @@ -423,6 +482,10 @@ static const struct iio_info hx711_iio_info = { > .attrs = &hx711_attribute_group, > }; > > +static const struct iio_info hx710b_iio_info = { > + .read_raw = hx711_read_raw, > +}; > + > static const struct iio_chan_spec hx711_chan_spec[] = { > { > .type = IIO_VOLTAGE, > @@ -455,10 +518,69 @@ static const struct iio_chan_spec hx711_chan_spec[] = { > IIO_CHAN_SOFT_TIMESTAMP(2), > }; > > +/* HX710B channels: differential input and DVDD-AVDD measurement. */ > +static const struct iio_chan_spec hx710b_chan_spec[] = { > + { > + .type = IIO_VOLTAGE, > + .channel = 0, > + .indexed = 1, If it is differential, shouldn't we have .differential = 1 and .channel2 = 1? > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_SCALE), > + .scan_index = 0, > + .scan_type = { > + .sign = 'u', > + .realbits = 24, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > + }, > + { > + .type = IIO_VOLTAGE, > + .channel = 1, > + .indexed = 1, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_SCALE), > + .scan_index = 1, > + .scan_type = { > + .sign = 'u', > + .realbits = 24, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > + }, > + IIO_CHAN_SOFT_TIMESTAMP(2), > +}; > + > +/* > + * HX710B trailing pulse counts. > + * 25 selects differential input, 26 selects DVDD-AVDD. > + */ > +static const int hx710b_pulse_counts[] = { > + 25, /* channel 0: differential input, 10 SPS */ > + 26, /* channel 1: DVDD-AVDD voltage, 40 SPS */ We could store these numbers in the .address field of the channel instead. It is there for driver-specific things like this. > +}; > + > +static const struct hx711_chip_info hx711_chip = { > + .name = "hx711", > + .channels = hx711_chan_spec, > + .num_channels = ARRAY_SIZE(hx711_chan_spec), > + .chan_pulse_count = NULL, > +}; > + > +static const struct hx711_chip_info hx710b_chip = { > + .name = "hx710b", > + .channels = hx710b_chan_spec, > + .num_channels = ARRAY_SIZE(hx710b_chan_spec), > + .chan_pulse_count = hx710b_pulse_counts, > + .num_chan_pulses = ARRAY_SIZE(hx710b_pulse_counts), > + .reset_channel = 0, > +}; > + > static int hx711_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct hx711_data *hx711_data; > + const struct hx711_chip_info *chip_info; > struct iio_dev *indio_dev; > int ret; > int i; > @@ -472,6 +594,11 @@ static int hx711_probe(struct platform_device *pdev) > > mutex_init(&hx711_data->lock); > > + chip_info = device_get_match_data(dev); > + if (!chip_info) > + return -ENODEV; We recently decided to standardize on not checking for NULL on this. > + hx711_data->chip_info = chip_info; > + > /* > * PD_SCK stands for power down and serial clock input of HX711 > * in the driver it is an output > @@ -510,12 +637,20 @@ static int hx711_probe(struct platform_device *pdev) > /* we need 10^-9 mV */ > ret *= 100; > > - for (i = 0; i < HX711_GAIN_MAX; i++) > - hx711_gain_to_scale[i].scale = > - ret / hx711_gain_to_scale[i].gain / 1678; > + if (chip_info->chan_pulse_count) { Why does chan_pulse_count imply fixed gain? If it is not related, we should add a new flag to the chip info instead. We want to avoid assuming any branch of the if statement is a specific chip in case any more are added in the future. > + /* HX710B uses a fixed gain, so compute scale once at probe. */ > + hx711_data->scale = ret / 128 / 1678; > + hx711_data->channel_set = chip_info->reset_channel; > + indio_dev->info = &hx710b_iio_info; > + } else { > + for (i = 0; i < HX711_GAIN_MAX; i++) > + hx711_gain_to_scale[i].scale = > + ret / hx711_gain_to_scale[i].gain / 1678; > > - hx711_data->gain_set = 128; > - hx711_data->gain_chan_a = 128; > + hx711_data->gain_set = 128; > + hx711_data->gain_chan_a = 128; > + indio_dev->info = &hx711_iio_info; > + } > > hx711_data->clock_frequency = 400000; > ret = device_property_read_u32(&pdev->dev, "clock-frequency", > @@ -533,11 +668,10 @@ static int hx711_probe(struct platform_device *pdev) > hx711_data->data_ready_delay_ns = > 1000000000 / hx711_data->clock_frequency; > > - indio_dev->name = "hx711"; > - indio_dev->info = &hx711_iio_info; > + indio_dev->name = chip_info->name; > indio_dev->modes = INDIO_DIRECT_MODE; > - indio_dev->channels = hx711_chan_spec; > - indio_dev->num_channels = ARRAY_SIZE(hx711_chan_spec); > + indio_dev->channels = chip_info->channels; > + indio_dev->num_channels = chip_info->num_channels; > > ret = devm_iio_triggered_buffer_setup(dev, indio_dev, > iio_pollfunc_store_time, > @@ -554,7 +688,8 @@ static int hx711_probe(struct platform_device *pdev) > } > > static const struct of_device_id of_hx711_match[] = { > - { .compatible = "avia,hx711", }, > + { .compatible = "avia,hx711", .data = &hx711_chip }, > + { .compatible = "avia,hx710b", .data = &hx710b_chip }, > { } > }; > > @@ -571,7 +706,6 @@ static struct platform_driver hx711_driver = { > module_platform_driver(hx711_driver); > > MODULE_AUTHOR("Andreas Klinger <ak@it-klinger.de>"); > -MODULE_DESCRIPTION("HX711 bitbanging driver - ADC for weight cells"); > +MODULE_DESCRIPTION("HX711/HX710B GPIO ADC driver"); > MODULE_LICENSE("GPL"); > MODULE_ALIAS("platform:hx711-gpio"); > - ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1 2/2] iio: adc: hx711: add support for HX710B 2026-04-18 22:05 ` David Lechner @ 2026-04-19 9:14 ` Piyush Patle 0 siblings, 0 replies; 4+ messages in thread From: Piyush Patle @ 2026-04-19 9:14 UTC (permalink / raw) To: David Lechner; +Cc: jic23, ak, nuno.sa, andy, linux-iio, linux-kernel On Sun, Apr 19, 2026 at 3:35 AM David Lechner <dlechner@baylibre.com> wrote: > > When there is more than one patch in a series, add a cover letter to > keep the patches together. > A cover letter was included, but it seems it did not reach properly. I will resend the series with correct threading in v2. > On 4/18/26 12:06 PM, Piyush Patle wrote: > > Refactor the driver around per-chip configuration so HX711 and HX710B > > can share the same core. > > > > Add HX710B channel definitions, pulse-count based channel selection, a > > Can you explain this difference a bit more? It seems like the two chips > are quite different in this regard. > On HX711, the number of trailing SCK pulses (1–3) after a conversion selects both the channel and gain for the next measurement. On HX710B, the gain is fixed and the trailing pulses only select the channel: 1 pulse selects the differential input channel (10 SPS) & 2 pulses select the DVDD-AVDD measurement channel (40 SPS) I will make this distinction clearer in the commit message in v2. > > variant-specific iio_info, and fixed-scale handling at probe. Update the > > Kconfig text and trim comments so the added support stays focused on the > > actual driver behavior. > > > > Signed-off-by: Piyush Patle <piyushpatle228@gmail.com> > > --- > > drivers/iio/adc/Kconfig | 9 +- > > drivers/iio/adc/hx711.c | 222 ++++++++++++++++++++++++++++++++-------- > > 2 files changed, 184 insertions(+), 47 deletions(-) > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 60038ae8dfc4..ddf981fa72a2 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -784,18 +784,21 @@ config HI8435 > > called hi8435. > > > > config HX711 > > - tristate "AVIA HX711 ADC for weight cells" > > + tristate "AVIA HX711 and HX710B ADC" > > depends on GPIOLIB > > select IIO_BUFFER > > select IIO_TRIGGERED_BUFFER > > help > > - If you say yes here you get support for AVIA HX711 ADC which is used > > - for weigh cells > > + If you say yes here you get support for AVIA HX711 and HX710B ADCs > > + which are used for bridge sensors such as weigh cells. > > > > This driver uses two GPIOs, one acts as the clock and controls the > > channel selection and gain, the other one is used for the measurement > > data > > > > + The HX710B is a variant with fixed gain and a different channel > > + selection scheme. > > + > > Currently the raw value is read from the chip and delivered. > > To get an actual weight one needs to subtract the > > zero offset and multiply by a scale factor. > > diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c > > index 1db8b68a8f64..cd251fa9f6b7 100644 > > --- a/drivers/iio/adc/hx711.c > > +++ b/drivers/iio/adc/hx711.c > > @@ -1,6 +1,6 @@ > > // SPDX-License-Identifier: GPL-2.0-or-later > > /* > > - * HX711: analog to digital converter for weight sensor module > > + * HX711/HX710B ADC driver > > * > > * Copyright (c) 2016 Andreas Klinger <ak@it-klinger.de> > > */ > > @@ -76,13 +76,34 @@ static int hx711_get_scale_to_gain(int scale) > > return -EINVAL; > > } > > > > +/** > > + * struct hx711_chip_info - per-variant static configuration > > + * @name: IIO device name > > + * @channels: channel specification > > + * @num_channels: number of channels > > + * @chan_pulse_count: trailing pulse count for fixed-gain variants > > + * @num_chan_pulses: number of pulse-count entries > > + * @reset_channel: default channel after reset > > + */ > > +struct hx711_chip_info { > > + const char *name; > > + const struct iio_chan_spec *channels; > > + int num_channels; > > + const int *chan_pulse_count; > > + int num_chan_pulses; > > + int reset_channel; > > +}; > > + > > struct hx711_data { > > - struct device *dev; > > - struct gpio_desc *gpiod_pd_sck; > > - struct gpio_desc *gpiod_dout; > > - int gain_set; /* gain set on device */ > > - int gain_chan_a; /* gain for channel A */ > > - struct mutex lock; > > + struct device *dev; > > + struct gpio_desc *gpiod_pd_sck; > > + struct gpio_desc *gpiod_dout; > > + int gain_set; /* HX711 */ > > + int gain_chan_a; /* HX711 channel A gain */ > > + int channel_set; /* HX710B */ > > + int scale; /* HX710B scale */ > > + const struct hx711_chip_info *chip_info; > > + struct mutex lock; > > /* > > * triggered buffer > > * 2x32-bit channel + 64-bit naturally aligned timestamp > > @@ -92,13 +113,10 @@ struct hx711_data { > > aligned_s64 timestamp; > > } buffer; > > /* > > - * delay after a rising edge on SCK until the data is ready DOUT > > - * this is dependent on the hx711 where the datasheet tells a > > - * maximum value of 100 ns > > - * but also on potential parasitic capacities on the wiring > > + * Delay after SCK rising edge before sampling DOUT. > > */ > > - u32 data_ready_delay_ns; > > - u32 clock_frequency; > > + u32 data_ready_delay_ns; > > + u32 clock_frequency; > > }; > > > > static int hx711_cycle(struct hx711_data *hx711_data) > > @@ -139,7 +157,11 @@ static int hx711_cycle(struct hx711_data *hx711_data) > > return gpiod_get_value(hx711_data->gpiod_dout); > > } > > > > -static int hx711_read(struct hx711_data *hx711_data) > > +/* > > + * Clock out 24 data bits and then send trailing pulses to select the > > + * next channel/gain state. > > + */ > > +static int hx711_read(struct hx711_data *hx711_data, int trailing_pulses) > > { > > int i, ret; > > int value = 0; > > @@ -158,7 +180,7 @@ static int hx711_read(struct hx711_data *hx711_data) > > > > value ^= 0x800000; > > > > - for (i = 0; i < hx711_get_gain_to_pulse(hx711_data->gain_set); i++) > > + for (i = 0; i < trailing_pulses; i++) > > hx711_cycle(hx711_data); > > > > return value; > > @@ -188,6 +210,7 @@ static int hx711_wait_for_ready(struct hx711_data *hx711_data) > > > > static int hx711_reset(struct hx711_data *hx711_data) > > { > > + const struct hx711_chip_info *info = hx711_data->chip_info; > > int val = hx711_wait_for_ready(hx711_data); > > > > if (val) { > > @@ -206,13 +229,17 @@ static int hx711_reset(struct hx711_data *hx711_data) > > > > val = hx711_wait_for_ready(hx711_data); > > > > - /* after a reset the gain is 128 */ > > - hx711_data->gain_set = HX711_RESET_GAIN; > > + /* Restore variant default after reset. */ > > + if (info->chan_pulse_count) > > + hx711_data->channel_set = info->reset_channel; > > + else > > + hx711_data->gain_set = HX711_RESET_GAIN; > > } > > > > return val; > > } > > > > +/* Select HX711 channel/gain for the next conversion. */ > > static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) > > { > > int ret; > > @@ -221,7 +248,8 @@ static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) > > if (hx711_data->gain_set == 32) { > > hx711_data->gain_set = hx711_data->gain_chan_a; > > > > - ret = hx711_read(hx711_data); > > + ret = hx711_read(hx711_data, > > + hx711_get_gain_to_pulse(hx711_data->gain_set)); > > if (ret < 0) > > return ret; > > > > @@ -233,7 +261,8 @@ static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) > > if (hx711_data->gain_set != 32) { > > hx711_data->gain_set = 32; > > > > - ret = hx711_read(hx711_data); > > + ret = hx711_read(hx711_data, > > + hx711_get_gain_to_pulse(hx711_data->gain_set)); > > if (ret < 0) > > return ret; > > > > @@ -246,27 +275,52 @@ static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan) > > return 0; > > } > > > > +/* Select HX710B channel for the next conversion. */ > > +static int hx710b_set_channel(struct hx711_data *hx711_data, int chan) > > +{ > > + const struct hx711_chip_info *info = hx711_data->chip_info; > > + int ret; > > + > > + if (chan >= info->num_chan_pulses) > > + return -EINVAL; > > + > > + if (hx711_data->channel_set == chan) > > + return 0; > > + > > + hx711_data->channel_set = chan; > > + > > + ret = hx711_read(hx711_data, info->chan_pulse_count[chan]); > > + if (ret < 0) > > + return ret; > > + > > + return hx711_wait_for_ready(hx711_data); > > +} > > + > > static int hx711_reset_read(struct hx711_data *hx711_data, int chan) > > { > > + const struct hx711_chip_info *info = hx711_data->chip_info; > > + int trailing_pulses; > > int ret; > > - int val; > > > > - /* > > - * hx711_reset() must be called from here > > - * because it could be calling hx711_read() by itself > > - */ > > + /* Reset first so the read starts from a known chip state. */ > > if (hx711_reset(hx711_data)) { > > dev_err(hx711_data->dev, "reset failed!"); > > return -EIO; > > } > > > > - ret = hx711_set_gain_for_channel(hx711_data, chan); > > - if (ret < 0) > > - return ret; > > - > > - val = hx711_read(hx711_data); > > + if (info->chan_pulse_count) { > > + ret = hx710b_set_channel(hx711_data, chan); > > + if (ret < 0) > > + return ret; > > + trailing_pulses = info->chan_pulse_count[chan]; > > + } else { > > + ret = hx711_set_gain_for_channel(hx711_data, chan); > > + if (ret < 0) > > + return ret; > > + trailing_pulses = hx711_get_gain_to_pulse(hx711_data->gain_set); > > + } > > > > - return val; > > + return hx711_read(hx711_data, trailing_pulses); > > } > > > > static int hx711_read_raw(struct iio_dev *indio_dev, > > @@ -274,6 +328,7 @@ static int hx711_read_raw(struct iio_dev *indio_dev, > > int *val, int *val2, long mask) > > { > > struct hx711_data *hx711_data = iio_priv(indio_dev); > > + const struct hx711_chip_info *info = hx711_data->chip_info; > > > > switch (mask) { > > case IIO_CHAN_INFO_RAW: > > @@ -290,7 +345,10 @@ static int hx711_read_raw(struct iio_dev *indio_dev, > > *val = 0; > > mutex_lock(&hx711_data->lock); > > > > - *val2 = hx711_get_gain_to_scale(hx711_data->gain_set); > > + if (info->chan_pulse_count) > > + *val2 = hx711_data->scale; > > + else > > + *val2 = hx711_get_gain_to_scale(hx711_data->gain_set); > > > > mutex_unlock(&hx711_data->lock); > > > > @@ -332,7 +390,8 @@ static int hx711_write_raw(struct iio_dev *indio_dev, > > if (gain != 32) > > hx711_data->gain_chan_a = gain; > > > > - ret = hx711_read(hx711_data); > > + ret = hx711_read(hx711_data, > > + hx711_get_gain_to_pulse(hx711_data->gain_set)); > > if (ret < 0) { > > mutex_unlock(&hx711_data->lock); > > return ret; > > @@ -423,6 +482,10 @@ static const struct iio_info hx711_iio_info = { > > .attrs = &hx711_attribute_group, > > }; > > > > +static const struct iio_info hx710b_iio_info = { > > + .read_raw = hx711_read_raw, > > +}; > > + > > static const struct iio_chan_spec hx711_chan_spec[] = { > > { > > .type = IIO_VOLTAGE, > > @@ -455,10 +518,69 @@ static const struct iio_chan_spec hx711_chan_spec[] = { > > IIO_CHAN_SOFT_TIMESTAMP(2), > > }; > > > > +/* HX710B channels: differential input and DVDD-AVDD measurement. */ > > +static const struct iio_chan_spec hx710b_chan_spec[] = { > > + { > > + .type = IIO_VOLTAGE, > > + .channel = 0, > > + .indexed = 1, > > If it is differential, shouldn't we have .differential = 1 and .channel2 = 1? > Yes, good catch. I will add .differential = 1 and .channel2 = 1 for channel 0 in v2. > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > > + BIT(IIO_CHAN_INFO_SCALE), > > + .scan_index = 0, > > + .scan_type = { > > + .sign = 'u', > > + .realbits = 24, > > + .storagebits = 32, > > + .endianness = IIO_CPU, > > + }, > > + }, > > + { > > + .type = IIO_VOLTAGE, > > + .channel = 1, > > + .indexed = 1, > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > > + BIT(IIO_CHAN_INFO_SCALE), > > + .scan_index = 1, > > + .scan_type = { > > + .sign = 'u', > > + .realbits = 24, > > + .storagebits = 32, > > + .endianness = IIO_CPU, > > + }, > > + }, > > + IIO_CHAN_SOFT_TIMESTAMP(2), > > +}; > > + > > +/* > > + * HX710B trailing pulse counts. > > + * 25 selects differential input, 26 selects DVDD-AVDD. > > + */ > > +static const int hx710b_pulse_counts[] = { > > + 25, /* channel 0: differential input, 10 SPS */ > > + 26, /* channel 1: DVDD-AVDD voltage, 40 SPS */ > > We could store these numbers in the .address field of the channel > instead. It is there for driver-specific things like this. > Agreed. I will store the trailing pulse counts in chan->address and remove the separate array in v2. > > +}; > > + > > +static const struct hx711_chip_info hx711_chip = { > > + .name = "hx711", > > + .channels = hx711_chan_spec, > > + .num_channels = ARRAY_SIZE(hx711_chan_spec), > > + .chan_pulse_count = NULL, > > +}; > > + > > +static const struct hx711_chip_info hx710b_chip = { > > + .name = "hx710b", > > + .channels = hx710b_chan_spec, > > + .num_channels = ARRAY_SIZE(hx710b_chan_spec), > > + .chan_pulse_count = hx710b_pulse_counts, > > + .num_chan_pulses = ARRAY_SIZE(hx710b_pulse_counts), > > + .reset_channel = 0, > > +}; > > + > > static int hx711_probe(struct platform_device *pdev) > > { > > struct device *dev = &pdev->dev; > > struct hx711_data *hx711_data; > > + const struct hx711_chip_info *chip_info; > > struct iio_dev *indio_dev; > > int ret; > > int i; > > @@ -472,6 +594,11 @@ static int hx711_probe(struct platform_device *pdev) > > > > mutex_init(&hx711_data->lock); > > > > + chip_info = device_get_match_data(dev); > > + if (!chip_info) > > + return -ENODEV; > > We recently decided to standardize on not checking for NULL on this. > Got it, I will remove the NULL check in v2. > > + hx711_data->chip_info = chip_info; > > + > > /* > > * PD_SCK stands for power down and serial clock input of HX711 > > * in the driver it is an output > > @@ -510,12 +637,20 @@ static int hx711_probe(struct platform_device *pdev) > > /* we need 10^-9 mV */ > > ret *= 100; > > > > - for (i = 0; i < HX711_GAIN_MAX; i++) > > - hx711_gain_to_scale[i].scale = > > - ret / hx711_gain_to_scale[i].gain / 1678; > > + if (chip_info->chan_pulse_count) { > > Why does chan_pulse_count imply fixed gain? If it is not related, we should > add a new flag to the chip info instead. > My initial reasoning was that chan_pulse_count is only used for variants where channel selection is driven entirely by trailing pulses and no gain configuration is exposed to the driver, so it implicitly indicated a fixed-gain device. However, this conflates channel selection and gain handling, which are independent concerns and may not scale well for future variants. I will rework this in v2 by introducing a dedicated fixed gain flag in the chip info. > We want to avoid assuming any branch of the if statement is a specific chip > in case any more are added in the future. > Also, while reviewing this, I noticed that the pulse counts were incorrectly specified as 25/26. Since hx711_read() already clocks out 24 data bits, trailing_pulses should represent only the additional cycles after data, so the correct values are 1 and 2. This will be fixed in v2. > > + /* HX710B uses a fixed gain, so compute scale once at probe. */ > > + hx711_data->scale = ret / 128 / 1678; > > + hx711_data->channel_set = chip_info->reset_channel; > > + indio_dev->info = &hx710b_iio_info; > > + } else { > > + for (i = 0; i < HX711_GAIN_MAX; i++) > > + hx711_gain_to_scale[i].scale = > > + ret / hx711_gain_to_scale[i].gain / 1678; > > > > - hx711_data->gain_set = 128; > > - hx711_data->gain_chan_a = 128; > > + hx711_data->gain_set = 128; > > + hx711_data->gain_chan_a = 128; > > + indio_dev->info = &hx711_iio_info; > > + } > > > > hx711_data->clock_frequency = 400000; > > ret = device_property_read_u32(&pdev->dev, "clock-frequency", > > @@ -533,11 +668,10 @@ static int hx711_probe(struct platform_device *pdev) > > hx711_data->data_ready_delay_ns = > > 1000000000 / hx711_data->clock_frequency; > > > > - indio_dev->name = "hx711"; > > - indio_dev->info = &hx711_iio_info; > > + indio_dev->name = chip_info->name; > > indio_dev->modes = INDIO_DIRECT_MODE; > > - indio_dev->channels = hx711_chan_spec; > > - indio_dev->num_channels = ARRAY_SIZE(hx711_chan_spec); > > + indio_dev->channels = chip_info->channels; > > + indio_dev->num_channels = chip_info->num_channels; > > > > ret = devm_iio_triggered_buffer_setup(dev, indio_dev, > > iio_pollfunc_store_time, > > @@ -554,7 +688,8 @@ static int hx711_probe(struct platform_device *pdev) > > } > > > > static const struct of_device_id of_hx711_match[] = { > > - { .compatible = "avia,hx711", }, > > + { .compatible = "avia,hx711", .data = &hx711_chip }, > > + { .compatible = "avia,hx710b", .data = &hx710b_chip }, > > { } > > }; > > > > @@ -571,7 +706,6 @@ static struct platform_driver hx711_driver = { > > module_platform_driver(hx711_driver); > > > > MODULE_AUTHOR("Andreas Klinger <ak@it-klinger.de>"); > > -MODULE_DESCRIPTION("HX711 bitbanging driver - ADC for weight cells"); > > +MODULE_DESCRIPTION("HX711/HX710B GPIO ADC driver"); > > MODULE_LICENSE("GPL"); > > MODULE_ALIAS("platform:hx711-gpio"); > > - > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1 2/2] iio: adc: hx711: add support for HX710B 2026-04-18 17:06 [PATCH v1 2/2] iio: adc: hx711: add support for HX710B Piyush Patle 2026-04-18 22:05 ` David Lechner @ 2026-04-19 12:00 ` Jonathan Cameron 1 sibling, 0 replies; 4+ messages in thread From: Jonathan Cameron @ 2026-04-19 12:00 UTC (permalink / raw) To: Piyush Patle; +Cc: ak, dlechner, nuno.sa, andy, linux-iio, linux-kernel On Sat, 18 Apr 2026 22:36:00 +0530 Piyush Patle <piyushpatle228@gmail.com> wrote: > Refactor the driver around per-chip configuration so HX711 and HX710B > can share the same core. > > Add HX710B channel definitions, pulse-count based channel selection, a > variant-specific iio_info, and fixed-scale handling at probe. Update the > Kconfig text and trim comments so the added support stays focused on the > actual driver behavior. > > Signed-off-by: Piyush Patle <piyushpatle228@gmail.com> Various comments inline. Many of which probably overlap with David's review. Jonathan > --- > drivers/iio/adc/Kconfig | 9 +- > drivers/iio/adc/hx711.c | 222 ++++++++++++++++++++++++++++++++-------- > 2 files changed, 184 insertions(+), 47 deletions(-) > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 60038ae8dfc4..ddf981fa72a2 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -784,18 +784,21 @@ config HI8435 > called hi8435. > > config HX711 > - tristate "AVIA HX711 ADC for weight cells" > + tristate "AVIA HX711 and HX710B ADC" > depends on GPIOLIB > select IIO_BUFFER > select IIO_TRIGGERED_BUFFER > help > - If you say yes here you get support for AVIA HX711 ADC which is used > - for weigh cells > + If you say yes here you get support for AVIA HX711 and HX710B ADCs > + which are used for bridge sensors such as weigh cells. > > This driver uses two GPIOs, one acts as the clock and controls the > channel selection and gain, the other one is used for the measurement > data > > + The HX710B is a variant with fixed gain and a different channel > + selection scheme. > + > Currently the raw value is read from the chip and delivered. > To get an actual weight one needs to subtract the > zero offset and multiply by a scale factor. > diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c > index 1db8b68a8f64..cd251fa9f6b7 100644 > --- a/drivers/iio/adc/hx711.c > +++ b/drivers/iio/adc/hx711.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > - * HX711: analog to digital converter for weight sensor module > + * HX711/HX710B ADC driver I'd keep the extra info that it's for weight sensors. > * > * Copyright (c) 2016 Andreas Klinger <ak@it-klinger.de> > */ > @@ -76,13 +76,34 @@ static int hx711_get_scale_to_gain(int scale) > return -EINVAL; > } > > +/** > + * struct hx711_chip_info - per-variant static configuration > + * @name: IIO device name > + * @channels: channel specification > + * @num_channels: number of channels > + * @chan_pulse_count: trailing pulse count for fixed-gain variants > + * @num_chan_pulses: number of pulse-count entries > + * @reset_channel: default channel after reset > + */ > +struct hx711_chip_info { > + const char *name; > + const struct iio_chan_spec *channels; > + int num_channels; > + const int *chan_pulse_count; > + int num_chan_pulses; > + int reset_channel; Various comments inline about probably adding more field rather than relying on chan_pulse_count for things that aren't immediately obviously related to that. > +}; > + > struct hx711_data { > - struct device *dev; > - struct gpio_desc *gpiod_pd_sck; > - struct gpio_desc *gpiod_dout; > - int gain_set; /* gain set on device */ > - int gain_chan_a; /* gain for channel A */ > - struct mutex lock; > + struct device *dev; > + struct gpio_desc *gpiod_pd_sck; > + struct gpio_desc *gpiod_dout; > + int gain_set; /* HX711 */ > + int gain_chan_a; /* HX711 channel A gain */ > + int channel_set; /* HX710B */ > + int scale; /* HX710B scale */ > + const struct hx711_chip_info *chip_info; The indentation change is a lot of noise. I'd just not bother doing it. Just have chip_info as the one thing more indented. My personal preference is never do careful alignment for structure members anyway as it leads to churn like this when a future patch 'needs' to change it. > + struct mutex lock; > /* > * triggered buffer > * 2x32-bit channel + 64-bit naturally aligned timestamp > @@ -92,13 +113,10 @@ struct hx711_data { > aligned_s64 timestamp; > } buffer; > /* > - * delay after a rising edge on SCK until the data is ready DOUT > - * this is dependent on the hx711 where the datasheet tells a > - * maximum value of 100 ns > - * but also on potential parasitic capacities on the wiring > + * Delay after SCK rising edge before sampling DOUT. > */ > - u32 data_ready_delay_ns; > - u32 clock_frequency; > + u32 data_ready_delay_ns; > + u32 clock_frequency; > }; > > static int hx711_read_raw(struct iio_dev *indio_dev, > @@ -274,6 +328,7 @@ static int hx711_read_raw(struct iio_dev *indio_dev, > int *val, int *val2, long mask) > { > struct hx711_data *hx711_data = iio_priv(indio_dev); > + const struct hx711_chip_info *info = hx711_data->chip_info; > > switch (mask) { > case IIO_CHAN_INFO_RAW: > @@ -290,7 +345,10 @@ static int hx711_read_raw(struct iio_dev *indio_dev, > *val = 0; > mutex_lock(&hx711_data->lock); > > - *val2 = hx711_get_gain_to_scale(hx711_data->gain_set); > + if (info->chan_pulse_count) I'd prefer a more obvious bool to use for this. info->fixed_scale or something like that? > + *val2 = hx711_data->scale; > + else > + *val2 = hx711_get_gain_to_scale(hx711_data->gain_set); > > mutex_unlock(&hx711_data->lock); > > @@ -332,7 +390,8 @@ static int hx711_write_raw(struct iio_dev *indio_dev, > if (gain != 32) > hx711_data->gain_chan_a = gain; > > - ret = hx711_read(hx711_data); > + ret = hx711_read(hx711_data, > + hx711_get_gain_to_pulse(hx711_data->gain_set)); > if (ret < 0) { > mutex_unlock(&hx711_data->lock); > return ret; > @@ -423,6 +482,10 @@ static const struct iio_info hx711_iio_info = { > .attrs = &hx711_attribute_group, > }; > +/* > + * HX710B trailing pulse counts. > + * 25 selects differential input, 26 selects DVDD-AVDD. > + */ > +static const int hx710b_pulse_counts[] = { > + 25, /* channel 0: differential input, 10 SPS */ > + 26, /* channel 1: DVDD-AVDD voltage, 40 SPS */ Odd spacing in the comments. > +}; > + > +static const struct hx711_chip_info hx711_chip = { > + .name = "hx711", > + .channels = hx711_chan_spec, > + .num_channels = ARRAY_SIZE(hx711_chan_spec), > + .chan_pulse_count = NULL, That will be the 'natural' default if you don't set it. I'm not sure explicitly setting it to NULL is providing useful documentation so perhaps just let the C spec stuff ensure it is null. > +}; > + > +static const struct hx711_chip_info hx710b_chip = { > + .name = "hx710b", > + .channels = hx710b_chan_spec, > + .num_channels = ARRAY_SIZE(hx710b_chan_spec), > + .chan_pulse_count = hx710b_pulse_counts, > + .num_chan_pulses = ARRAY_SIZE(hx710b_pulse_counts), > + .reset_channel = 0, Given this is only set in this instance and is 0, what benefit is it bringing? > +}; > + > static int hx711_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct hx711_data *hx711_data; > + const struct hx711_chip_info *chip_info; > struct iio_dev *indio_dev; > int ret; > int i; > @@ -472,6 +594,11 @@ static int hx711_probe(struct platform_device *pdev) > > mutex_init(&hx711_data->lock); > > + chip_info = device_get_match_data(dev); We've had some recent discussions around concluded there is little point in sanity checking something that would be such an obvious bug in a driver. So for new code at least, don't add this check so we get slightly simpler code. > + if (!chip_info) > + return -ENODEV; > + hx711_data->chip_info = chip_info; > + > /* > * PD_SCK stands for power down and serial clock input of HX711 > * in the driver it is an output > @@ -510,12 +637,20 @@ static int hx711_probe(struct platform_device *pdev) > /* we need 10^-9 mV */ > ret *= 100; > > - for (i = 0; i < HX711_GAIN_MAX; i++) > - hx711_gain_to_scale[i].scale = > - ret / hx711_gain_to_scale[i].gain / 1678; > + if (chip_info->chan_pulse_count) { > + /* HX710B uses a fixed gain, so compute scale once at probe. */ Have a flag in chip_info for fixed gain and probably put these value in chip_info as well. > + hx711_data->scale = ret / 128 / 1678; > + hx711_data->channel_set = chip_info->reset_channel; > + indio_dev->info = &hx710b_iio_info; > + } else { > + for (i = 0; i < HX711_GAIN_MAX; i++) > + hx711_gain_to_scale[i].scale = > + ret / hx711_gain_to_scale[i].gain / 1678; > > - hx711_data->gain_set = 128; > - hx711_data->gain_chan_a = 128; > + hx711_data->gain_set = 128; > + hx711_data->gain_chan_a = 128; > + indio_dev->info = &hx711_iio_info; I'd not make this one dependent on else for chan_pulse_count. handle it as a separate thing in chip_info as the code will end up more readable. > + } > @@ -554,7 +688,8 @@ static int hx711_probe(struct platform_device *pdev) > } > > static const struct of_device_id of_hx711_match[] = { > - { .compatible = "avia,hx711", }, > + { .compatible = "avia,hx711", .data = &hx711_chip }, > + { .compatible = "avia,hx710b", .data = &hx710b_chip }, Even though it's a new part you are adding, put it above the old one to have alpha numeric ordering. Makes it easier to know where to add other new parts in future. > { } > }; > > @@ -571,7 +706,6 @@ static struct platform_driver hx711_driver = { > module_platform_driver(hx711_driver); > > MODULE_AUTHOR("Andreas Klinger <ak@it-klinger.de>"); > -MODULE_DESCRIPTION("HX711 bitbanging driver - ADC for weight cells"); > +MODULE_DESCRIPTION("HX711/HX710B GPIO ADC driver"); > MODULE_LICENSE("GPL"); > MODULE_ALIAS("platform:hx711-gpio"); > - Looks like an unrelated change. Always do a quick check for these before sending out patches. They slow things down a little and are easy to clean up! Thanks, Jonathan ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-19 12:00 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-18 17:06 [PATCH v1 2/2] iio: adc: hx711: add support for HX710B Piyush Patle 2026-04-18 22:05 ` David Lechner 2026-04-19 9:14 ` Piyush Patle 2026-04-19 12:00 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox