public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Piyush Patle <piyushpatle228@gmail.com>
Cc: "Andreas Klinger" <ak@it-klinger.de>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Andy Shevchenko" <andriy.shevchenko@intel.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/3] iio: adc: hx711: refactor to per-chip hx711_chip_info structure
Date: Fri, 24 Apr 2026 13:14:01 +0100	[thread overview]
Message-ID: <20260424131401.707ba1cd@jic23-huawei> (raw)
In-Reply-To: <20260422175910.1258579-3-piyushpatle228@gmail.com>

On Wed, 22 Apr 2026 23:29:09 +0530
Piyush Patle <piyushpatle228@gmail.com> wrote:

> Introduce hx711_chip_info to hold per-variant static configuration:
> device name, IIO channel spec, channel count, and iio_info pointer.
> Store a chip_info pointer in hx711_data and populate indio_dev fields
> from it at probe instead of hardcoding them.
> 
> Pass trailing pulse count directly to hx711_read() instead of
> computing it inside the function, and change hx711_reset_read() to
> take a const struct iio_chan_spec * instead of an integer channel
> index so callers can pass the full channel descriptor.
> 
> Use device_get_match_data() to look up the chip_info from the
> of_device_id table. No functional change for existing HX711 users.
> 
> Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
Couple of things sashiko raised on this...

One about hx711_gain_to_scale() is an existing driver bug.
Need to take a per instance copy of that so as to support
more than one sensor.  Ideally fix that whilst you are touching the
driver (as a precursor patch - maybe we'll backport it) but if you
want to leave it for another day then just say that in the cover letter.

The other one about matching via non DT routes is an interesting corner
that i suspect applies to other drivers.  The module alias in this
driver is also rather odd.


>  
> @@ -571,7 +602,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 and compatible bitbanging ADC driver");
>  MODULE_LICENSE("GPL");
>  MODULE_ALIAS("platform:hx711-gpio");
So Sashiko thinks (and it's plausible) that this module alias is a problem
as if the driver match is against it might be possible to probe the
driver without the match data getting set.  I think it's referring to
the final driver match fallback path
https://elixir.bootlin.com/linux/v7.0/source/drivers/base/platform.c#L1370

Whilst unlikely anyone would get to this, I guess the fix is to ensure
we get a match by having a platform_device_id table for legacy matching
and making sure that covers such a match and provides data.

We don't (I think) currently have an equivalent of the catch all matching
functions that exist for i2c and spi.
Bit ugly for platform data as I think you need to get it via
platform_get_device_id()

Maybe we just check if device_get_match_data() returns NULL and fail instead.

> -


  parent reply	other threads:[~2026-04-24 12:14 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-22 17:59 [PATCH v3 0/3] iio: adc: hx711: add HX710B support Piyush Patle
2026-04-22 17:59 ` [PATCH v3 1/3] dt-bindings: iio: adc: avia-hx711: add avia,hx710b compatible Piyush Patle
2026-04-23  8:05   ` Krzysztof Kozlowski
2026-04-24 12:00   ` Jonathan Cameron
2026-04-26 16:28   ` David Lechner
2026-04-26 16:43     ` Piyush Patle
2026-04-22 17:59 ` [PATCH v3 2/3] iio: adc: hx711: refactor to per-chip hx711_chip_info structure Piyush Patle
2026-04-22 20:16   ` Andy Shevchenko
2026-04-24 11:53   ` Jonathan Cameron
2026-04-24 12:14   ` Jonathan Cameron [this message]
2026-04-22 17:59 ` [PATCH v3 3/3] iio: adc: hx711: add support for HX710B Piyush Patle
2026-04-22 20:20   ` Andy Shevchenko
2026-04-24 12:19   ` Jonathan Cameron

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=20260424131401.707ba1cd@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=ak@it-klinger.de \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --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