* Re: [PATCH 1/5] media: i2c: vd55g1: Fix media bus code initialization
From: Jacopo Mondi @ 2026-06-22 9:28 UTC (permalink / raw)
To: Benjamin Mugnier
Cc: Sylvain Petinot, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, linux-media,
linux-kernel, devicetree
In-Reply-To: <20260428-vd55g4_and_fixes-v1-1-4f745a83b87e@foss.st.com>
Hi Benjamin
On Tue, Apr 28, 2026 at 10:40:55AM +0200, Benjamin Mugnier wrote:
> In the driver initialization, the index of the default media bus code
> from the supported media bus code array is passed directly to the
> vd55g1_get_fmt_code() function instead of the proper media bus code.
>
> This works correctly as a proper media bus code is set after
> initialization but could not have been the case. This also resulted in
> mutliple "Unsupported mbus format" error messages.
>
> Retrieve the media bus code from the media bus code array, and pass this
> media bus code to vd55g1_get_fmt_code() instead of the code index.
>
> Rename VD55G1_MBUS_CODE_DEF to VD55G1_MBUS_CODE_IDX_DEF and
> VD55G1_MODE_DEF to VD55G1_MODE_IDX_DEF while at it to avoid future
> confusions. Display the guilty error code in warning message.
>
> Fixes: e138e7f00042 ("media: i2c: vd55g1: Add support for vd65g4 RGB variant")
>
You should cc stable for fixes
Cc: stable@vger.kernel.org
The CI should have flagged that, but for some reason it didn't run
properly on your series
https://gitlab.freedesktop.org/linux-media/users/patchwork/-/pipelines/1655147
> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
> ---
> drivers/media/i2c/vd55g1.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
> index 78d18c028154..1e9db21322e3 100644
> --- a/drivers/media/i2c/vd55g1.c
> +++ b/drivers/media/i2c/vd55g1.c
> @@ -114,9 +114,9 @@
>
> #define VD55G1_WIDTH 804
> #define VD55G1_HEIGHT 704
> -#define VD55G1_MODE_DEF 0
> +#define VD55G1_MODE_IDX_DEF 0
> #define VD55G1_NB_GPIOS 4
> -#define VD55G1_MBUS_CODE_DEF 0
> +#define VD55G1_MBUS_CODE_IDX_DEF 0
> #define VD55G1_DGAIN_DEF 256
> #define VD55G1_AGAIN_DEF 19
> #define VD55G1_EXPO_MAX_TERM 64
> @@ -634,7 +634,7 @@ static u32 vd55g1_get_fmt_code(struct vd55g1 *sensor, u32 code)
Unrelated, but it seems you now have 2 codes for MONO. Does
if (sensor->id == VD55G1_MODEL_ID_VD55G1)
return code;
need an update ?
> goto adapt_bayer_pattern;
> }
> }
> - dev_warn(sensor->dev, "Unsupported mbus format\n");
> + dev_warn(sensor->dev, "Unsupported mbus format: 0x%x\n", code);
>
> return code;
>
> @@ -1347,6 +1347,7 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
> {
> struct vd55g1 *sensor = to_vd55g1(sd);
> struct v4l2_subdev_format fmt = { 0 };
> + int code;
> struct v4l2_subdev_route routes[] = {
> { .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE }
> };
> @@ -1361,9 +1362,13 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
> if (ret)
> return ret;
>
> - vd55g1_update_pad_fmt(sensor, &vd55g1_supported_modes[VD55G1_MODE_DEF],
> - vd55g1_get_fmt_code(sensor, VD55G1_MBUS_CODE_DEF),
> - &fmt.format);
> + if (sensor->id == VD55G1_MODEL_ID_VD55G1)
> + code = vd55g1_mbus_formats_mono[VD55G1_MBUS_CODE_IDX_DEF];
> + else
> + code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF][0];
Being this a multi-dimensional array, I don't seem much value in
defining VD55G1_MBUS_CODE_IDX_DEF if this is the only place where it
is used. What's the meaning of VD55G1_MBUS_CODE_IDX_DEF for
vd55g1_mbus_formats_bayer ? Does it represent the bitwidth or does it
represent the bayer pattern ?
I would rather define a
VD55G1_DEF_MBUS_CODE_MONO MEDIA_BUS_FMT_Y8_1X8
VD55G1_DEF_MBUS_CODE_BAYER MEDIA_BUS_FMT_SRGGB8_1X8
Or maybe do
code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF]
[VD55G1_MBUS_CODE_IDX_DEF];
if easier.
I understand it's a minor, so up to you.
> + vd55g1_update_pad_fmt(sensor,
> + &vd55g1_supported_modes[VD55G1_MODE_IDX_DEF],
> + vd55g1_get_fmt_code(sensor, code), &fmt.format);
>
> return vd55g1_set_pad_fmt(sd, sd_state, &fmt);
> }
>
> --
> 2.43.0
>
>
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Jonathan Cameron @ 2026-06-22 9:27 UTC (permalink / raw)
To: Nuno Sá
Cc: Conor Dooley, Janani Sunil, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <ajj6nEb4tATM3C7b@nsa>
On Mon, 22 Jun 2026 10:07:01 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Sun, Jun 21, 2026 at 07:35:42PM +0100, Conor Dooley wrote:
> > On Sun, Jun 21, 2026 at 03:33:40PM +0100, Jonathan Cameron wrote:
> > > On Fri, 19 Jun 2026 16:54:11 +0100
> > > Nuno Sá <noname.nuno@gmail.com> wrote:
> > >
> > > > On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:
> > > > > On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:
> > > > > > On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
> > > > > > > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> > > > > > > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> > > > > > > > >
> > > > > > > > > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > > > > > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > > > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > > > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > > > > > > integrated precision reference.
> > > > > > > > > > > > ...
> > > > > > > > > > > > Probably others may comment on that, but...
> > > > > > > > > > > >
> > > > > > > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > > > > > > >
> > > > > > > > > > > > That way I suppose that an example would look like...
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +patternProperties:
> > > > > > > > > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > > > > > > > > + type: object
> > > > > > > > > > > > > + description: Child nodes for individual channel configuration
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + properties:
> > > > > > > > > > > > > + reg:
> > > > > > > > > > > > > + description: Channel number.
> > > > > > > > > > > > > + minimum: 0
> > > > > > > > > > > > > + maximum: 15
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + adi,output-range-microvolt:
> > > > > > > > > > > > > + description: |
> > > > > > > > > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > > > > > > > > + oneOf:
> > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > + - const: 0
> > > > > > > > > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > + - const: -5000000
> > > > > > > > > > > > > + - const: 5000000
> > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > + - const: -10000000
> > > > > > > > > > > > > + - const: 10000000
> > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > + - const: -15000000
> > > > > > > > > > > > > + - const: 15000000
> > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > + - const: -20000000
> > > > > > > > > > > > > + - const: 20000000
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + required:
> > > > > > > > > > > > > + - reg
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + additionalProperties: false
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +required:
> > > > > > > > > > > > > + - compatible
> > > > > > > > > > > > > + - reg
> > > > > > > > > > > > > + - vdd-supply
> > > > > > > > > > > > > + - avdd-supply
> > > > > > > > > > > > > + - hvdd-supply
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +dependencies:
> > > > > > > > > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > > > > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +allOf:
> > > > > > > > > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +unevaluatedProperties: false
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +examples:
> > > > > > > > > > > > > + - |
> > > > > > > > > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + spi {
> > > > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + dac@0 {
> > > > > > > > > > > > > + compatible = "adi,ad5529r-16";
> > > > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > > > + spi-max-frequency = <25000000>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + channel@0 {
> > > > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > > + };
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + channel@1 {
> > > > > > > > > > > > > + reg = <1>;
> > > > > > > > > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > > + };
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + channel@2 {
> > > > > > > > > > > > > + reg = <2>;
> > > > > > > > > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > > + };
> > > > > > > > > > > > > + };
> > > > > > > > > > > > > + };
> > > > > > > > > > > > ...
> > > > > > > > > > > >
> > > > > > > > > > > > spi {
> > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > >
> > > > > > > > > > > > multi-dac@0 {
> > > > > > > > > > > > compatible = "adi,ad5529r-16";
> > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > spi-max-frequency = <25000000>;
> > > > > > > > > > > >
> > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > >
> > > > > > > > > > > > dac@0 {
> > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > >
> > > > > > > > > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > >
> > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > >
> > > > > > > > > > > > channel@0 {
> > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > };
> > > > > > > > > > > >
> > > > > > > > > > > > channel@1 {
> > > > > > > > > > > > reg = <1>;
> > > > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > };
> > > > > > > > > > > >
> > > > > > > > > > > > channel@2 {
> > > > > > > > > > > > reg = <2>;
> > > > > > > > > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > };
> > > > > > > > > > > > }
> > > > > > > > > > > >
> > > > > > > > > > > > dac@1 {
> > > > > > > > > > > > reg = <1>;
> > > > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > >
> > > > > > > > > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > >
> > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > >
> > > > > > > > > > > > channel@0 {
> > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > };
> > > > > > > > > > > >
> > > > > > > > > > > > channel@1 {
> > > > > > > > > > > > reg = <1>;
> > > > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > };
> > > > > > > > > > > > }
> > > > > > > > > > > > };
> > > > > > > > > > > > };
> > > > > > > > > > > >
> > > > > > > > > > > > then you might need something like:
> > > > > > > > > > > >
> > > > > > > > > > > > patternProperties:
> > > > > > > > > > > > "^dac@[0-3]$":
> > > > > > > > > > > >
> > > > > > > > > > > > and put most of the things under this node pattern.
> > > > > > > > > > > >
> > > > > > > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > > > > > > >
> > > > > > > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > > > > > > > > Hi Rodrigo,
> > > > > > > > > > >
> > > > > > > > > > > Thank you for looking at this.
> > > > > > > > > > >
> > > > > > > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > > > > > > speculatively without a validating use case.
> > > > > > > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > > > > > > looking at it another way a kind of distributed SPI mux.
> > > > > > > > > >
> > > > > > > > > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > > > > > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > > > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > > > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > > > > > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > > > > > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > > > > > > in all devices in the chain as one big SPI message.
> > > > > > > > > >
> > > > > > > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > > > > > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > > > > > > > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > > > > > > > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > > > > > > > > longer term how to support it cleanly in SPI.
> > > > > > > >
> > > > > > > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > > > > > > see if I can find what I am thinking of...
> > > > > > >
> > > > > > >
> > > > > > > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > > > > > > slightly different properties.
> > > > > > >
> > > > > > > microchip,device-addr:
> > > > > > > description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > > > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > enum: [0, 1, 2, 3]
> > > > > > > default: 0
> > > > > > >
> > > > > > > and
> > > > > > >
> > > > > > >
> > > > > > > microchip,hw-device-address:
> > > > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > minimum: 0
> > > > > > > maximum: 3
> > > > > > > description:
> > > > > > > The address is set on a per-device basis by fuses in the factory,
> > > > > > > configured on request. If not requested, the fuses are set for 0x1.
> > > > > > > The device address is part of the device markings to avoid
> > > > > > > potential confusion. This address is coded on two bits, so four possible
> > > > > > > addresses are available when multiple devices are present on the same
> > > > > > > SPI bus with only one Chip Select line for all devices.
> > > > > > > Each device communication starts by a CS falling edge, followed by the
> > > > > > > clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > > > > > > which is first one on the wire).
> > > > > > >
> > > > > > > This sounds exactly like the sort of feature that you're dealing with
> > > > > > > here?
> > > > > > >
> > > > > >
> > > > > > The core idea yes but for this chip, things are a bit more annoying (but
> > > > > > Janani can correct me if I'm wrong). Here, each device can, in theory,
> > > > > > have it's own supplies, pins and at the very least, channels with maybe
> > > > > > different scales. That is why Janani is proposing dac nodes. Given I
> > > > > > honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> > > > > > about solving this at the spi level.
> > > > > >
> > > > > > Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> > > > > > together in the same bus.
> > > > >
> > > > > I'm definitely missing something, because that property for the
> > > > > microchip devices is not impacted what else is on the bus. AFAICT, you
> > > > > could have an mcp3911 and an mcp3564 on the same bus even though both
> > > > > are completely different devices with different drivers. They have
> > > > > individual device nodes and their own supplies etc etc. These aren't
> > > > > per-channel properties on an adc or dac, they're per child device on a
> > > > > spi bus.
> > > >
> > > > Maybe I'm the one missing something :). IIRC, spi would not allow two
> > > > devices on the same CS right? Because for this chip we would need
> > > > something like:
> > > >
> > > > spi {
> > > > dac@0 {
> > > > reg = <0>;
> > > > adi,pin-id = <0>;
> > > > };
> > > >
> > > > dac@1 {
> > > > reg = <0>; // which seems already problematic?
> > > > adi,pin-id <1>;
> > > > };
> > > >
> > > > ...
> > > >
> > > > //up to 4
> > > > };
> > > Yeah. It's not clear to me how that works for the microchip devices
> > > (I suspect it doesn't!)
> > >
> > > Just thinking as I type, but could we do something a bit nasty with
> > > a gpio mux that doesn't actually switch but represents the GPIO being
> > > shared? Given this is all tied to the spi bus that should all happen
> > > under serializing locks.
> > >
> > > Agreed though that this would be nicer as an SPI thing that let
> > > us specify that a single CS is share by multiple devices and their
> > > is some other signal acting to select which one we are talking to.
> >
> > Whether it works or not, I think it is the more correct approach. Messing
> > with gpio muxes seems completely wrong, given the chip select may not be
> > a gpio at all.
> >
> > Why do you think the microchip devices won't work? Does the spi core
> > reject multiple devices with the same chip select being registered or
> > something like that?
>
> Not sure how things work atm. But I'm fairly sure it used to be like
> that. SPI would reject devices on the same controller and CS. Now that
> we support more than one CS per controller, not sure how things work.
We always supported more than one per CS per controller. I guess you mean
per device.
>
> Janani, maybe you can give it a try?
I think we'd need to get it to work with shared gpio proxy which maybe
will just get set up under the hood. This used to be opt in, but seems
that changed fairly recently so maybe some of us are working with out
of date knowledge! I haven't played with it yet, so might not be
that simple.
Jonathan
>
> - Nuno Sá
>
^ permalink raw reply
* Re: [PATCH v3] spi: dt-bindings: octeon: Convert to DT schema
From: Krzysztof Kozlowski @ 2026-06-22 9:24 UTC (permalink / raw)
To: Ninad Naik
Cc: broonie, robh, krzk+dt, conor+dt, david.daney, linux-spi,
devicetree, linux-kernel, me, linux-kernel-mentees, skhan
In-Reply-To: <20260618180149.475658-1-ninadnaik07@gmail.com>
On Thu, Jun 18, 2026 at 11:31:49PM +0530, Ninad Naik wrote:
> Convert octeon-3010 to DT schema
>
> Signed-off-by: Ninad Naik <ninadnaik07@gmail.com>
> ---
> Changes in v3:
> - Change the maintainer from David Daney to Rob Herring
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Rodrigo Alencar @ 2026-06-22 9:24 UTC (permalink / raw)
To: Jonathan Cameron, Nuno Sá
Cc: Conor Dooley, Janani Sunil, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <20260621153330.79b6600c@jic23-huawei>
On 21/06/26 15:33, Jonathan Cameron wrote:
> On Fri, 19 Jun 2026 16:54:11 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
>
> > On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:
> > > On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:
> > > > On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
> > > > > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> > > > > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> > > > > > >
> > > > > > > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > > > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > > > > >
> > > > > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > > > > integrated precision reference.
> > > > > > > > > > ...
> > > > > > > > > > Probably others may comment on that, but...
> > > > > > > > > >
> > > > > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > > > > >
> > > > > > > > > > That way I suppose that an example would look like...
> > > > > > > > > > > +
> > > > > > > > > > > +patternProperties:
> > > > > > > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > > > > > > + type: object
> > > > > > > > > > > + description: Child nodes for individual channel configuration
> > > > > > > > > > > +
> > > > > > > > > > > + properties:
> > > > > > > > > > > + reg:
> > > > > > > > > > > + description: Channel number.
> > > > > > > > > > > + minimum: 0
> > > > > > > > > > > + maximum: 15
> > > > > > > > > > > +
> > > > > > > > > > > + adi,output-range-microvolt:
> > > > > > > > > > > + description: |
> > > > > > > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > > > > > > + oneOf:
> > > > > > > > > > > + - items:
> > > > > > > > > > > + - const: 0
> > > > > > > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > > > > + - items:
> > > > > > > > > > > + - const: -5000000
> > > > > > > > > > > + - const: 5000000
> > > > > > > > > > > + - items:
> > > > > > > > > > > + - const: -10000000
> > > > > > > > > > > + - const: 10000000
> > > > > > > > > > > + - items:
> > > > > > > > > > > + - const: -15000000
> > > > > > > > > > > + - const: 15000000
> > > > > > > > > > > + - items:
> > > > > > > > > > > + - const: -20000000
> > > > > > > > > > > + - const: 20000000
> > > > > > > > > > > +
> > > > > > > > > > > + required:
> > > > > > > > > > > + - reg
> > > > > > > > > > > +
> > > > > > > > > > > + additionalProperties: false
> > > > > > > > > > > +
> > > > > > > > > > > +required:
> > > > > > > > > > > + - compatible
> > > > > > > > > > > + - reg
> > > > > > > > > > > + - vdd-supply
> > > > > > > > > > > + - avdd-supply
> > > > > > > > > > > + - hvdd-supply
> > > > > > > > > > > +
> > > > > > > > > > > +dependencies:
> > > > > > > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > > > > > > +
> > > > > > > > > > > +allOf:
> > > > > > > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > > > > +
> > > > > > > > > > > +unevaluatedProperties: false
> > > > > > > > > > > +
> > > > > > > > > > > +examples:
> > > > > > > > > > > + - |
> > > > > > > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > > > +
> > > > > > > > > > > + spi {
> > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > +
> > > > > > > > > > > + dac@0 {
> > > > > > > > > > > + compatible = "adi,ad5529r-16";
> > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > + spi-max-frequency = <25000000>;
> > > > > > > > > > > +
> > > > > > > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > +
> > > > > > > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > +
> > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > +
> > > > > > > > > > > + channel@0 {
> > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > + };
> > > > > > > > > > > +
> > > > > > > > > > > + channel@1 {
> > > > > > > > > > > + reg = <1>;
> > > > > > > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > + };
> > > > > > > > > > > +
> > > > > > > > > > > + channel@2 {
> > > > > > > > > > > + reg = <2>;
> > > > > > > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > + };
> > > > > > > > > > > + };
> > > > > > > > > > > + };
> > > > > > > > > > ...
> > > > > > > > > >
> > > > > > > > > > spi {
> > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > #size-cells = <0>;
> > > > > > > > > >
> > > > > > > > > > multi-dac@0 {
> > > > > > > > > > compatible = "adi,ad5529r-16";
> > > > > > > > > > reg = <0>;
> > > > > > > > > > spi-max-frequency = <25000000>;
> > > > > > > > > >
> > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > #size-cells = <0>;
> > > > > > > > > >
> > > > > > > > > > dac@0 {
> > > > > > > > > > reg = <0>;
> > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > >
> > > > > > > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > >
> > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > #size-cells = <0>;
> > > > > > > > > >
> > > > > > > > > > channel@0 {
> > > > > > > > > > reg = <0>;
> > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > channel@1 {
> > > > > > > > > > reg = <1>;
> > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > channel@2 {
> > > > > > > > > > reg = <2>;
> > > > > > > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > };
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > dac@1 {
> > > > > > > > > > reg = <1>;
> > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > >
> > > > > > > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > > > > >
> > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > #size-cells = <0>;
> > > > > > > > > >
> > > > > > > > > > channel@0 {
> > > > > > > > > > reg = <0>;
> > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > channel@1 {
> > > > > > > > > > reg = <1>;
> > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > };
> > > > > > > > > > }
> > > > > > > > > > };
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > then you might need something like:
> > > > > > > > > >
> > > > > > > > > > patternProperties:
> > > > > > > > > > "^dac@[0-3]$":
> > > > > > > > > >
> > > > > > > > > > and put most of the things under this node pattern.
> > > > > > > > > >
> > > > > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > > > > >
> > > > > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > > > > > > Hi Rodrigo,
> > > > > > > > >
> > > > > > > > > Thank you for looking at this.
> > > > > > > > >
> > > > > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > > > > speculatively without a validating use case.
> > > > > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > > > > looking at it another way a kind of distributed SPI mux.
> > > > > > > >
> > > > > > > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > > > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > > > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > > > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > > > > in all devices in the chain as one big SPI message.
> > > > > > > >
> > > > > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > > > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > > > > > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > > > > > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > > > > > > longer term how to support it cleanly in SPI.
> > > > > >
> > > > > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > > > > see if I can find what I am thinking of...
> > > > >
> > > > >
> > > > > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > > > > slightly different properties.
> > > > >
> > > > > microchip,device-addr:
> > > > > description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > enum: [0, 1, 2, 3]
> > > > > default: 0
> > > > >
> > > > > and
> > > > >
> > > > >
> > > > > microchip,hw-device-address:
> > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > minimum: 0
> > > > > maximum: 3
> > > > > description:
> > > > > The address is set on a per-device basis by fuses in the factory,
> > > > > configured on request. If not requested, the fuses are set for 0x1.
> > > > > The device address is part of the device markings to avoid
> > > > > potential confusion. This address is coded on two bits, so four possible
> > > > > addresses are available when multiple devices are present on the same
> > > > > SPI bus with only one Chip Select line for all devices.
> > > > > Each device communication starts by a CS falling edge, followed by the
> > > > > clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > > > > which is first one on the wire).
> > > > >
> > > > > This sounds exactly like the sort of feature that you're dealing with
> > > > > here?
> > > > >
> > > >
> > > > The core idea yes but for this chip, things are a bit more annoying (but
> > > > Janani can correct me if I'm wrong). Here, each device can, in theory,
> > > > have it's own supplies, pins and at the very least, channels with maybe
> > > > different scales. That is why Janani is proposing dac nodes. Given I
> > > > honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> > > > about solving this at the spi level.
> > > >
> > > > Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> > > > together in the same bus.
> > >
> > > I'm definitely missing something, because that property for the
> > > microchip devices is not impacted what else is on the bus. AFAICT, you
> > > could have an mcp3911 and an mcp3564 on the same bus even though both
> > > are completely different devices with different drivers. They have
> > > individual device nodes and their own supplies etc etc. These aren't
> > > per-channel properties on an adc or dac, they're per child device on a
> > > spi bus.
> >
> > Maybe I'm the one missing something :). IIRC, spi would not allow two
> > devices on the same CS right? Because for this chip we would need
> > something like:
> >
> > spi {
> > dac@0 {
> > reg = <0>;
> > adi,pin-id = <0>;
> > };
> >
> > dac@1 {
> > reg = <0>; // which seems already problematic?
> > adi,pin-id <1>;
> > };
> >
> > ...
> >
> > //up to 4
> > };
> Yeah. It's not clear to me how that works for the microchip devices
> (I suspect it doesn't!)
>
> Just thinking as I type, but could we do something a bit nasty with
> a gpio mux that doesn't actually switch but represents the GPIO being
> shared? Given this is all tied to the spi bus that should all happen
> under serializing locks.
>
> Agreed though that this would be nicer as an SPI thing that let
> us specify that a single CS is share by multiple devices and their
> is some other signal acting to select which one we are talking to.
>
If the device-addressing on the same chip-select is to be handled
by the spi framework, wouldn't we lose device-specific features?
I understand that this multi-device feature is there mostly to extend the
channel count from 16 to 32, 48 or 64. I suppose the command:
"MULTI DEVICE SW LDAC MODE"
exists so that software can update channel values accross multiple devices.
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply
* Re: [PATCH v3 1/3] dt-bindings: media: qcom,qcm2290-venus: document shikra Iris compatible
From: Krzysztof Kozlowski @ 2026-06-22 9:24 UTC (permalink / raw)
To: Vikash Garodia
Cc: Bryan O'Donoghue, Dikshita Agarwal, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jorge Ramirez-Ortiz, Bjorn Andersson, Konrad Dybcio,
linux-arm-msm, linux-media, devicetree, linux-kernel
In-Reply-To: <20260618-shikra_vpu-v3-1-1a32e26a35a1@oss.qualcomm.com>
On Thu, Jun 18, 2026 at 04:09:54PM +0530, Vikash Garodia wrote:
> Document the iris video accelerator used on shikra platforms by adding
> the qcom,shikra-iris compatible.
>
> Although QCM2290 and shikra share the same video hardware and overall
> integration, their SMMU programming differs. QCM2290 exposes separate
> stream IDs for the video hardware and the Xtensa path, requiring two
> explicit IOMMU entries, whereas shikra uses a masked SMR to collapse
> equivalent stream IDs into a single mapping. Due to QCM2290’s SID layout
> and Xtensa isolation requirements, such SMR masking is not applicable on
> QCM2290 platforms.
> Since shikra uses the same video hardware as QCM2290 and shares the same
> programming model and capabilities, it is added as a fallback compatible
> to qcom,qcm2290-venus, with conditional handling to allow either one or
> two IOMMU entries.
>
> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
> ---
> .../bindings/media/qcom,qcm2290-venus.yaml | 26 ++++++++++++++++------
> 1 file changed, 19 insertions(+), 7 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/3] arm64: dts: qcom: Add HP EliteBook X G2q 14 AI
From: Konrad Dybcio @ 2026-06-22 9:23 UTC (permalink / raw)
To: Abel Vesa, Jason Pettit
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel,
Akhil P Oommen, Mahadevan P, Sibi Sankar, Jingyi Wang,
Ananthu C V
In-Reply-To: <erlfxo4gcvuaakuggrgroniiwofdrocgtje32idibknj7kb42g@pdh7fo4x6ief>
On 6/22/26 10:18 AM, Abel Vesa wrote:
> On 26-06-20 21:50:42, Jason Pettit wrote:
>> Add board support for the HP EliteBook X G2q 14" Next Gen AI PC
>> (product SKU C4JG0AV, board 8E91), a Snapdragon X2 Elite (Glymur)
>> laptop, using the "hp,elitebook-x-g2q" / "qcom,glymur" compatible.
>>
>> Enabled by this device tree:
>>
>> - internal eDP panel (samsung,atna33xc20)
>> - 2x USB Type-C with DisplayPort alt-mode and USB
>> - chassis HDMI output
>> - chassis USB-A host port (usb_mp multiport controller)
>> - internal eUSB2 host with the Elan fingerprint reader
>> - NVMe SSD on PCIe5
>> - Wi-Fi and Bluetooth
>> - HID-over-I2C keyboard, touchpad, touchscreen; lid switch
>> - Adreno GPU and GMU (Freedreno GL on Mesa)
>> - audio playback and capture
>>
>> The HDMI jack is driven by a power-only DisplayPort-to-HDMI LSPCON on
>> the usb_2 combo-PHY DP lanes rather than being a third USB-C port; HPD
>> is on gpio126. The LSPCON is on an I/O sub-board with no I2C/AUX control
>> path, so it is modelled with the generic simple-bridge "parade,ps185hdm"
>> compatible used by the in-tree x1e80100 HDMI-bridge boards (the exact
>> bridge part is unconfirmed) and it needs CONFIG_DRM_SIMPLE_BRIDGE.
>>
>> The &gpu/&gmu enable, the audio nodes and &remoteproc_soccp opt into
>> glymur.dtsi SoC nodes that are still in-flight; those series are
>> declared as prerequisites in the cover letter.
>>
>> Signed-off-by: Jason Pettit <jason.pettit@oss.qualcomm.com>
>> Assisted-by: Claude:claude-opus-4-8
>> ---
[...]
>> +&usb_0_hsphy {
>> + vdd-supply = <&vreg_l3f_e0_0p91>;
>> + vdda12-supply = <&vreg_l4h_e0_1p2>;
>
> No redriver ?
Right, this must be bound to one of smb2370_[jkl]_e2_eusb2_repeater,
most likely in the natural order (0->j, 1->k)
Konrad
^ permalink raw reply
* Re: [PATCH v3] spi: dt-bindings: microchip,pic32mzda-spi: Convert to DT schema
From: Krzysztof Kozlowski @ 2026-06-22 9:22 UTC (permalink / raw)
To: Udaya Kiran Challa
Cc: tsbogend, robh, krzk+dt, conor+dt, skhan, me, linux-rtc,
devicetree, linux-kernel
In-Reply-To: <20260617101009.148851-1-challauday369@gmail.com>
On Wed, Jun 17, 2026 at 03:40:09PM +0530, Udaya Kiran Challa wrote:
> Convert Microchip PIC32 SPI controller devicetree binding
> from legacy text format to DT schema.
>
> Signed-off-by: Udaya Kiran Challa <challauday369@gmail.com>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: phy: qcom,usb-hs-phy: add qcom,hs-drv-slope
From: Krzysztof Kozlowski @ 2026-06-22 9:21 UTC (permalink / raw)
To: Herman van Hazendonk
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Philipp Zabel, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-arm-msm,
linux-phy, devicetree, linux-kernel, llvm, konrad.dybcio,
dmitry.baryshkov
In-Reply-To: <20260616-submit-phy-usb-hs-vendor-init-seq-v3-1-7d21fb1d1484@herrie.org>
On Tue, Jun 16, 2026 at 03:26:53PM +0200, Herman van Hazendonk wrote:
> The MSM8x60 / APQ8060 PHY needs three vendor ULPI register tweaks for
> stable USB operation: pre-emphasis level, CDR auto-reset and SE1
> gating in registers 0x32 and 0x36. A survey of MSM8x60-class
> downstream board files (Qualcomm SURF/FFA/Fluid/Dragon, Samsung
> Galaxy S2 family, Sony Xperia, HTC and HP TouchPad) shows that those
> three values are identical across every reference board and can be
> hardcoded in the driver behind the existing
> qcom,usb-hs-phy-msm8660 compatible.
>
> The only board-specific value is the 4-bit HS driver slope in bits
> [3:0] of register 0x32:
>
> HP TouchPad 5
> HTC MSM8660 ports 1
> Qualcomm / Samsung / Sony reference boards 0 (silicon default)
>
> Add a qcom,hs-drv-slope property carrying that 4-bit value, valid
> only on the qcom,usb-hs-phy-msm8660 variant. When the property is
> absent the driver leaves the silicon default in place, matching the
> behaviour of the Qualcomm reference platform.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/3] media: i2c: add imx576 image sensor driver
From: Himanshu Bhavani @ 2026-06-22 9:18 UTC (permalink / raw)
To: Laurent Pinchart
Cc: sakari.ailus@linux.intel.com, luca.weiss@fairphone.com,
Hardevsinh Palaniya, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Hans Verkuil, Hans de Goede, Vladimir Zapolskiy, Elgin Perumbilly,
Walter Werner Schneider, Kate Hsuan, Svyatoslav Ryhel,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
In-Reply-To: <20260622090432.GL3552167@killaraus.ideasonboard.com>
>On Mon, Jun 22, 2026 at 06:02:24AM +0000, Himanshu Bhavani wrote:
>> > On Fri, Jun 19, 2026 at 06:24:32PM +0530, Himanshu Bhavani wrote:
>> >> Add a v4l2 subdevice driver for the Sony imx576 sensor.
>> >>
>> >> The Sony IMX576 image sensor with an active
>> >> array size of 5760 x 4312
>> >>
>> >> The following features are supported:
>> >> - Manual exposure an gain control support
>> >> - vblank/hblank control support
>> >> - Supported resolution: 2880 x 2156 30fps (SRGGB10)
>> >
>> >You've been asked in v1 to make this driver dynamically compute
>> >registers instead of hardcoding modes. Please do so in v3. Nack on v2.
>>
>> As I mentioned earlier, I don't have the full datasheet yet, so I
>> can't implement this now.
>
>You have been given a link to the datasheet in a private reply.
Sorry, I missed that e-mail, Just checked.
However, the datasheet is the same that I have, but still I will check the reference's as suggested by Dave.
Thanks & BR,
Himanshu Bhavani
^ permalink raw reply
* Re: [PATCH v4 02/16] spi: dt-bindings: add spi-phy-pattern-partition property
From: Krzysztof Kozlowski @ 2026-06-22 9:17 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, pratyush, mwalle, takahiro.kuwano, linux-spi,
devicetree, linux-kernel, linux-mtd, praneeth, u-kumar1, a-dutta
In-Reply-To: <20260618073725.84733-3-s-k6@ti.com>
On Thu, Jun 18, 2026 at 01:07:11PM +0530, Santhosh Kumar K wrote:
> Add spi-phy-pattern-partition, a per-device phandle property on the
> flash sub-node that allows the DT author to directly reference the
> partition holding the PHY tuning pattern. Used to locate the pattern
> data during PHY tuning when the device cannot load the pattern
> dynamically.
>
> "Read PHY tuning" works by reading a known data pattern from the device
> repeatedly while sweeping controller delay parameters until the
> capture window is stable. For SPI NAND, the driver loads the pattern
> into the page cache once using write-to-cache opcodes, then reads it
> during the sweep. SPI NOR devices have no equivalent opcode, so the
> pattern must be pre-programmed in a dedicated flash partition. One
> partition per device is required to keep the procedure unambiguous
> when multiple devices share a bus.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
> ---
> .../bindings/spi/cdns,qspi-nor.yaml | 19 +++++++++++++++++++
> .../bindings/spi/spi-peripheral-props.yaml | 7 +++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> index 891f578b5ac4..c6f1b1d1251d 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> @@ -204,10 +204,29 @@ examples:
> flash@0 {
> compatible = "jedec,spi-nor";
> reg = <0x0>;
> + #address-cells = <1>;
> + #size-cells = <1>;
This is neither needed, nor correct.
> cdns,read-delay = <4>;
> cdns,tshsl-ns = <60>;
> cdns,tsd2d-ns = <60>;
> cdns,tchsh-ns = <60>;
> cdns,tslch-ns = <60>;
> + spi-phy-pattern-partition = <&phy_pattern>;
> +
> + partitions {
> + compatible = "fixed-partitions";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition@0 {
> + label = "data";
> + reg = <0x0 0x3fc0000>;
> + };
> +
> + phy_pattern: partition@3fc0000 {
> + label = "phy-pattern";
> + reg = <0x3fc0000 0x40000>;
> + };
> + };
> };
> };
> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> index ece86f65930f..38708f8197f9 100644
> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> @@ -123,6 +123,13 @@ properties:
> description:
> Delay, in microseconds, after a write transfer.
>
> + spi-phy-pattern-partition:
Is this specific to SPI-based MTD/NAND or rather broader - specific to
MTD/NAND memories, regardless of interface? Feels like the second, thus
maybe should be placed into the NAND bindings.
If the first, then in below description:
s/PHY/SPI PHY/ to be clear that this is about SPI, not the memory
itself.
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the flash partition holding the pre-programmed PHY tuning
> + pattern. Used when the device cannot load the pattern dynamically during
> + PHY tuning.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 01/16] spi: dt-bindings: add spi-max-post-config-frequency property
From: Krzysztof Kozlowski @ 2026-06-22 9:14 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, pratyush, mwalle, takahiro.kuwano, linux-spi,
devicetree, linux-kernel, linux-mtd, praneeth, u-kumar1, a-dutta
In-Reply-To: <20260618073725.84733-2-s-k6@ti.com>
On Thu, Jun 18, 2026 at 01:07:10PM +0530, Santhosh Kumar K wrote:
> Add spi-max-post-config-frequency, a generic uint32 property for SPI
> peripherals that support two distinct clock rates: a conservative rate
> always reachable without controller configuration, and a higher rate
> reachable only after controller-side configuration such as PHY tuning.
>
> When both properties are present, spi-max-frequency gives the
> conservative pre-configuration rate and spi-max-post-config-frequency
> gives the higher post-configuration target.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
> ---
> .../devicetree/bindings/spi/spi-peripheral-props.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> index 880a9f624566..ece86f65930f 100644
> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> @@ -45,6 +45,12 @@ properties:
> description:
> Maximum SPI clocking speed of the device in Hz.
>
> + spi-max-post-config-frequency:
-hz
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
and you need maxItems: 1.
Now, please take time and think if this should not be an array instead
(maxItems: ...) to cover other possible cases, e.g. different tuning
levels? IOW, having single spi-max-frequency turned out to be
insufficient. You address that insufficiency with one more frequency,
but what if this is going to be insufficient next month as well?
I don't know, answer is rather for domain experts.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Maximum SPI clock frequency in Hz achievable post controller-side
> + configuration.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support
From: Philipp Zabel @ 2026-06-22 9:12 UTC (permalink / raw)
To: Janani Sunil, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil
In-Reply-To: <20260519-ad5529r-driver-v3-2-267c0731aa68@analog.com>
On Di, 2026-05-19 at 17:42 +0200, Janani Sunil wrote:
> Add support for AD5529R 16-channel, 12/16 bit Digital to Analog Converter
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> ---
> MAINTAINERS | 1 +
> drivers/iio/dac/Kconfig | 17 ++
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/ad5529r.c | 527 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 546 insertions(+)
>
[...]
> diff --git a/drivers/iio/dac/ad5529r.c b/drivers/iio/dac/ad5529r.c
> new file mode 100644
> index 000000000000..9bb63030db95
> --- /dev/null
> +++ b/drivers/iio/dac/ad5529r.c
> @@ -0,0 +1,527 @@
[...]
> +static int ad5529r_reset(struct ad5529r_state *st)
> +{
> + struct reset_control *rst;
> + int ret;
> +
> + rst = devm_reset_control_get_optional_exclusive(&st->spi->dev, NULL);
Consider using devm_reset_control_get_optional_exclusive_deasserted()
to save a few lines, and to make sure the reset line is asserted again
when the driver is unbound.
> + if (IS_ERR(rst))
> + return PTR_ERR(rst);
> +
> + if (rst) {
> + ret = reset_control_deassert(rst);
> + if (ret)
> + return ret;
This branch could then be removed.
> + } else {
> + ret = regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
> + AD5529R_INTERFACE_CONFIG_A_SW_RESET);
> + if (ret)
> + return ret;
> + }
regards
Philipp
^ permalink raw reply
* Re: [PATCH v4 1/3] dt-bindings: net: add Realtek RTL8125 PCIe Ethernet
From: Krzysztof Kozlowski @ 2026-06-22 9:08 UTC (permalink / raw)
To: Heiner Kallweit
Cc: ricardo, nic_swsd, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Sebastian Reichel, netdev,
devicetree, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <876a38f8-75ea-4b32-bb65-216cb3adb436@gmail.com>
On Wed, Jun 17, 2026 at 06:43:42PM +0200, Heiner Kallweit wrote:
> On 17.06.2026 14:58, Ricardo Pardini via B4 Relay wrote:
> > From: Ricardo Pardini <ricardo@pardini.net>
> >
> > Add a binding for fixed/soldered Realtek RTL8125 PCIe Ethernet
> > controller.
> >
> > The "pciVVVV,DDDD" compatibles are the Open Firmware PCI Bus Binding
> > spelling, auto-derived from PCI-SIG vendor/device IDs, but they still
> > need a binding when used in a board DT - analogous to "usbVVVV,PPPP"
Ricardo,
No, they do not need. They are already documented, they already have a
binding, see: dtschema/schemas/pci/pci-device.yaml
> > compatibles documented in their own bindings (e.g. microchip,lan95xx)
> > so board DTs attaching properties (fixed MAC, nvmem cell, ...) to
> > these PCI function nodes can be validated.
> >
> > Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > Signed-off-by: Ricardo Pardini <ricardo@pardini.net>
> > ---
> > .../devicetree/bindings/net/realtek,rtl8125.yaml | 43 ++++++++++++++++++++++
> > MAINTAINERS | 1 +
> > 2 files changed, 44 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/realtek,rtl8125.yaml b/Documentation/devicetree/bindings/net/realtek,rtl8125.yaml
> > new file mode 100644
> > index 0000000000000..eee13fbc1e6a6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/realtek,rtl8125.yaml
> > @@ -0,0 +1,43 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/realtek,rtl8125.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Realtek RTL8125 2.5 Gigabit PCIe Ethernet Controller
> > +
> > +maintainers:
> > + - Heiner Kallweit <hkallweit1@gmail.com>
> > +
> > +description:
> > + The Realtek RTL8125 is a 2.5GBASE-T Ethernet controller with a PCIe host
> > + interface.
> > +
> > +allOf:
> > + - $ref: ethernet-controller.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: pci10ec,8125
>
> IIRC we came to the conclusion that the compatible string isn't used in the
> relevant code path. Then why add it here? Is there an alignment on this?
Heiner, it is used - in the DTS.
> If it should be added here, then an explaining comment would be helpful.
Commit msg should explain that. The compatible is used, so it
must be documented and in fact already is, so you need to specify them
ONLY if device nodes have some other properties, like being an ethernet
controller.
I assume that this is the case here, although that should be mentioned
in the commit msg.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Nuno Sá @ 2026-06-22 9:07 UTC (permalink / raw)
To: Conor Dooley
Cc: Jonathan Cameron, Janani Sunil, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <20260621-nutmeg-coauthor-715189372230@spud>
On Sun, Jun 21, 2026 at 07:35:42PM +0100, Conor Dooley wrote:
> On Sun, Jun 21, 2026 at 03:33:40PM +0100, Jonathan Cameron wrote:
> > On Fri, 19 Jun 2026 16:54:11 +0100
> > Nuno Sá <noname.nuno@gmail.com> wrote:
> >
> > > On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:
> > > > On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:
> > > > > On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
> > > > > > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> > > > > > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> > > > > > > >
> > > > > > > > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > > > > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > > > > > integrated precision reference.
> > > > > > > > > > > ...
> > > > > > > > > > > Probably others may comment on that, but...
> > > > > > > > > > >
> > > > > > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > > > > > >
> > > > > > > > > > > That way I suppose that an example would look like...
> > > > > > > > > > > > +
> > > > > > > > > > > > +patternProperties:
> > > > > > > > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > > > > > > > + type: object
> > > > > > > > > > > > + description: Child nodes for individual channel configuration
> > > > > > > > > > > > +
> > > > > > > > > > > > + properties:
> > > > > > > > > > > > + reg:
> > > > > > > > > > > > + description: Channel number.
> > > > > > > > > > > > + minimum: 0
> > > > > > > > > > > > + maximum: 15
> > > > > > > > > > > > +
> > > > > > > > > > > > + adi,output-range-microvolt:
> > > > > > > > > > > > + description: |
> > > > > > > > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > > > > > > > + oneOf:
> > > > > > > > > > > > + - items:
> > > > > > > > > > > > + - const: 0
> > > > > > > > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > > > > > + - items:
> > > > > > > > > > > > + - const: -5000000
> > > > > > > > > > > > + - const: 5000000
> > > > > > > > > > > > + - items:
> > > > > > > > > > > > + - const: -10000000
> > > > > > > > > > > > + - const: 10000000
> > > > > > > > > > > > + - items:
> > > > > > > > > > > > + - const: -15000000
> > > > > > > > > > > > + - const: 15000000
> > > > > > > > > > > > + - items:
> > > > > > > > > > > > + - const: -20000000
> > > > > > > > > > > > + - const: 20000000
> > > > > > > > > > > > +
> > > > > > > > > > > > + required:
> > > > > > > > > > > > + - reg
> > > > > > > > > > > > +
> > > > > > > > > > > > + additionalProperties: false
> > > > > > > > > > > > +
> > > > > > > > > > > > +required:
> > > > > > > > > > > > + - compatible
> > > > > > > > > > > > + - reg
> > > > > > > > > > > > + - vdd-supply
> > > > > > > > > > > > + - avdd-supply
> > > > > > > > > > > > + - hvdd-supply
> > > > > > > > > > > > +
> > > > > > > > > > > > +dependencies:
> > > > > > > > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > > > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > > > > > > > +
> > > > > > > > > > > > +allOf:
> > > > > > > > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > > > > > +
> > > > > > > > > > > > +unevaluatedProperties: false
> > > > > > > > > > > > +
> > > > > > > > > > > > +examples:
> > > > > > > > > > > > + - |
> > > > > > > > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > > > > +
> > > > > > > > > > > > + spi {
> > > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > > +
> > > > > > > > > > > > + dac@0 {
> > > > > > > > > > > > + compatible = "adi,ad5529r-16";
> > > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > > + spi-max-frequency = <25000000>;
> > > > > > > > > > > > +
> > > > > > > > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > +
> > > > > > > > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > +
> > > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > > +
> > > > > > > > > > > > + channel@0 {
> > > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > + };
> > > > > > > > > > > > +
> > > > > > > > > > > > + channel@1 {
> > > > > > > > > > > > + reg = <1>;
> > > > > > > > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > + };
> > > > > > > > > > > > +
> > > > > > > > > > > > + channel@2 {
> > > > > > > > > > > > + reg = <2>;
> > > > > > > > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > + };
> > > > > > > > > > > > + };
> > > > > > > > > > > > + };
> > > > > > > > > > > ...
> > > > > > > > > > >
> > > > > > > > > > > spi {
> > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > >
> > > > > > > > > > > multi-dac@0 {
> > > > > > > > > > > compatible = "adi,ad5529r-16";
> > > > > > > > > > > reg = <0>;
> > > > > > > > > > > spi-max-frequency = <25000000>;
> > > > > > > > > > >
> > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > >
> > > > > > > > > > > dac@0 {
> > > > > > > > > > > reg = <0>;
> > > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > > >
> > > > > > > > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > >
> > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > >
> > > > > > > > > > > channel@0 {
> > > > > > > > > > > reg = <0>;
> > > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > };
> > > > > > > > > > >
> > > > > > > > > > > channel@1 {
> > > > > > > > > > > reg = <1>;
> > > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > };
> > > > > > > > > > >
> > > > > > > > > > > channel@2 {
> > > > > > > > > > > reg = <2>;
> > > > > > > > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > };
> > > > > > > > > > > }
> > > > > > > > > > >
> > > > > > > > > > > dac@1 {
> > > > > > > > > > > reg = <1>;
> > > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > > >
> > > > > > > > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > > > > > >
> > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > >
> > > > > > > > > > > channel@0 {
> > > > > > > > > > > reg = <0>;
> > > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > };
> > > > > > > > > > >
> > > > > > > > > > > channel@1 {
> > > > > > > > > > > reg = <1>;
> > > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > };
> > > > > > > > > > > }
> > > > > > > > > > > };
> > > > > > > > > > > };
> > > > > > > > > > >
> > > > > > > > > > > then you might need something like:
> > > > > > > > > > >
> > > > > > > > > > > patternProperties:
> > > > > > > > > > > "^dac@[0-3]$":
> > > > > > > > > > >
> > > > > > > > > > > and put most of the things under this node pattern.
> > > > > > > > > > >
> > > > > > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > > > > > >
> > > > > > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > > > > > > > Hi Rodrigo,
> > > > > > > > > >
> > > > > > > > > > Thank you for looking at this.
> > > > > > > > > >
> > > > > > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > > > > > speculatively without a validating use case.
> > > > > > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > > > > > looking at it another way a kind of distributed SPI mux.
> > > > > > > > >
> > > > > > > > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > > > > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > > > > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > > > > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > > > > > in all devices in the chain as one big SPI message.
> > > > > > > > >
> > > > > > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > > > > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > > > > > > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > > > > > > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > > > > > > > longer term how to support it cleanly in SPI.
> > > > > > >
> > > > > > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > > > > > see if I can find what I am thinking of...
> > > > > >
> > > > > >
> > > > > > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > > > > > slightly different properties.
> > > > > >
> > > > > > microchip,device-addr:
> > > > > > description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > enum: [0, 1, 2, 3]
> > > > > > default: 0
> > > > > >
> > > > > > and
> > > > > >
> > > > > >
> > > > > > microchip,hw-device-address:
> > > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > minimum: 0
> > > > > > maximum: 3
> > > > > > description:
> > > > > > The address is set on a per-device basis by fuses in the factory,
> > > > > > configured on request. If not requested, the fuses are set for 0x1.
> > > > > > The device address is part of the device markings to avoid
> > > > > > potential confusion. This address is coded on two bits, so four possible
> > > > > > addresses are available when multiple devices are present on the same
> > > > > > SPI bus with only one Chip Select line for all devices.
> > > > > > Each device communication starts by a CS falling edge, followed by the
> > > > > > clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > > > > > which is first one on the wire).
> > > > > >
> > > > > > This sounds exactly like the sort of feature that you're dealing with
> > > > > > here?
> > > > > >
> > > > >
> > > > > The core idea yes but for this chip, things are a bit more annoying (but
> > > > > Janani can correct me if I'm wrong). Here, each device can, in theory,
> > > > > have it's own supplies, pins and at the very least, channels with maybe
> > > > > different scales. That is why Janani is proposing dac nodes. Given I
> > > > > honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> > > > > about solving this at the spi level.
> > > > >
> > > > > Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> > > > > together in the same bus.
> > > >
> > > > I'm definitely missing something, because that property for the
> > > > microchip devices is not impacted what else is on the bus. AFAICT, you
> > > > could have an mcp3911 and an mcp3564 on the same bus even though both
> > > > are completely different devices with different drivers. They have
> > > > individual device nodes and their own supplies etc etc. These aren't
> > > > per-channel properties on an adc or dac, they're per child device on a
> > > > spi bus.
> > >
> > > Maybe I'm the one missing something :). IIRC, spi would not allow two
> > > devices on the same CS right? Because for this chip we would need
> > > something like:
> > >
> > > spi {
> > > dac@0 {
> > > reg = <0>;
> > > adi,pin-id = <0>;
> > > };
> > >
> > > dac@1 {
> > > reg = <0>; // which seems already problematic?
> > > adi,pin-id <1>;
> > > };
> > >
> > > ...
> > >
> > > //up to 4
> > > };
> > Yeah. It's not clear to me how that works for the microchip devices
> > (I suspect it doesn't!)
> >
> > Just thinking as I type, but could we do something a bit nasty with
> > a gpio mux that doesn't actually switch but represents the GPIO being
> > shared? Given this is all tied to the spi bus that should all happen
> > under serializing locks.
> >
> > Agreed though that this would be nicer as an SPI thing that let
> > us specify that a single CS is share by multiple devices and their
> > is some other signal acting to select which one we are talking to.
>
> Whether it works or not, I think it is the more correct approach. Messing
> with gpio muxes seems completely wrong, given the chip select may not be
> a gpio at all.
>
> Why do you think the microchip devices won't work? Does the spi core
> reject multiple devices with the same chip select being registered or
> something like that?
Not sure how things work atm. But I'm fairly sure it used to be like
that. SPI would reject devices on the same controller and CS. Now that
we support more than one CS per controller, not sure how things work.
Janani, maybe you can give it a try?
- Nuno Sá
^ permalink raw reply
* Re: [PATCH v4 2/2] media: i2c: ov5640: Add reset controller support with GPIO fallback
From: Philipp Zabel @ 2026-06-22 9:05 UTC (permalink / raw)
To: Frank Li, robby.cai
Cc: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, festevam,
sebastian.krzyszkowiak, slongerbeam, sakari.ailus, mchehab,
kieran.bingham, kernel, devicetree, imx, linux-arm-kernel,
linux-kernel
In-Reply-To: <ajVPzoWVBi1vsqRQ@SMW015318>
On Fr, 2026-06-19 at 09:18 -0500, Frank Li wrote:
> On Fri, Jun 19, 2026 at 06:05:32PM +0800, robby.cai@oss.nxp.com wrote:
> > [You don't often get email from robby.cai@oss.nxp.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > From: Robby Cai <robby.cai@nxp.com>
> >
> > Add support for the reset controller framework by acquiring the reset
> > line using devm_reset_control_get_optional_shared_deasserted(). This
> > allows the driver to handle reset lines provided by a reset controller,
> > including shared ones, while avoiding unbalanced deassert counts.
> >
> > Retain support for legacy reset-gpios as a fallback when no reset
> > controller is defined. In that case, request the GPIO and keep it in the
> > deasserted state as the initial configuration.
> >
> > This enables the driver to support both reset-controller-backed reset
> > lines and older GPIO-based descriptions while preserving the existing
> > power-up sequencing behavior.
> >
> > Signed-off-by: Robby Cai <robby.cai@nxp.com>
> > ---
> > drivers/media/i2c/ov5640.c | 80 +++++++++++++++++++++++++++++++++-----
> > 1 file changed, 70 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index 85ecc23b3587..5e6db8aacb11 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -17,6 +17,7 @@
> > #include <linux/module.h>
> > #include <linux/pm_runtime.h>
> > #include <linux/regulator/consumer.h>
> > +#include <linux/reset.h>
> > #include <linux/slab.h>
> > #include <linux/types.h>
> > #include <media/v4l2-async.h>
> > @@ -442,6 +443,7 @@ struct ov5640_dev {
> > u32 xclk_freq;
> >
> > struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
> > + struct reset_control *reset;
> > struct gpio_desc *reset_gpio;
> > struct gpio_desc *pwdn_gpio;
> > bool upside_down;
> > @@ -2431,6 +2433,48 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
> > return ov5640_set_framefmt(sensor, &sensor->fmt);
> > }
> >
> > +static int ov5640_get_reset(struct device *dev, struct ov5640_dev *sensor)
> > +{
> > + /* use deasserted version to avoid unbalanced deassert counts */
> > + sensor->reset =
> > + devm_reset_control_get_optional_shared_deasserted(dev, NULL);
> > + if (IS_ERR(sensor->reset))
> > + return dev_err_probe(dev, PTR_ERR(sensor->reset),
> > + "Failed to get reset\n");
> > + else if (sensor->reset)
> > + return 0;
> > +
> > + /*
> > + * fallback to legacy reset-gpios
> > + * GPIOD_OUT_HIGH ensures deasserted state for ACTIVE_LOW reset
> > + */
> > + sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > + GPIOD_OUT_HIGH);
> > + if (IS_ERR(sensor->reset_gpio))
> > + return dev_err_probe(dev, PTR_ERR(sensor->reset_gpio),
> > + "Failed to get reset gpio");
>
> I think needn't fallback here, NO ABI change, just change to use reset-gpio
> driver.
Please keep the gpiod fallback, the reset-gpio driver may not be
available on all platforms using ov5640.
> > +
> > + return 0;
> > +}
> > +
> > +static int ov5640_reset_assert(struct ov5640_dev *sensor)
> > +{
> > + if (sensor->reset)
> > + return reset_control_assert(sensor->reset);
>
> needn't check sensor->reset, reset_control_assert() is no ops if NULL.
>
> > +
> > + gpiod_set_value_cansleep(sensor->reset_gpio, 1);
>
> Needn't fallback, directly replace.
See above.
regards
Philipp
^ permalink raw reply
* Re: [PATCH v2 2/3] media: i2c: add imx576 image sensor driver
From: Laurent Pinchart @ 2026-06-22 9:04 UTC (permalink / raw)
To: Himanshu Bhavani
Cc: sakari.ailus@linux.intel.com, luca.weiss@fairphone.com,
Hardevsinh Palaniya, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Hans Verkuil, Hans de Goede, Vladimir Zapolskiy, Elgin Perumbilly,
Walter Werner Schneider, Kate Hsuan, Svyatoslav Ryhel,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
In-Reply-To: <PN0P287MB20199A3EF8F3183176AC1E559AEF2@PN0P287MB2019.INDP287.PROD.OUTLOOK.COM>
On Mon, Jun 22, 2026 at 06:02:24AM +0000, Himanshu Bhavani wrote:
> > On Fri, Jun 19, 2026 at 06:24:32PM +0530, Himanshu Bhavani wrote:
> >> Add a v4l2 subdevice driver for the Sony imx576 sensor.
> >>
> >> The Sony IMX576 image sensor with an active
> >> array size of 5760 x 4312
> >>
> >> The following features are supported:
> >> - Manual exposure an gain control support
> >> - vblank/hblank control support
> >> - Supported resolution: 2880 x 2156 30fps (SRGGB10)
> >
> >You've been asked in v1 to make this driver dynamically compute
> >registers instead of hardcoding modes. Please do so in v3. Nack on v2.
>
> As I mentioned earlier, I don't have the full datasheet yet, so I
> can't implement this now.
You have been given a link to the datasheet in a private reply.
> Link to discussion:
> https://lore.kernel.org/linux-media/PN0P287MB2019AFCBDF0E24BFEF8E0E399A0F2@PN0P287MB2019.INDP287.PROD.OUTLOOK.COM/
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v5 4/4] arm64: dts: cix: sky1: add audss cru
From: Krzysztof Kozlowski @ 2026-06-22 9:02 UTC (permalink / raw)
To: joakim.zhang
Cc: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang, cix-kernel-upstream, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <20260622022520.3127103-5-joakim.zhang@cixtech.com>
On Mon, Jun 22, 2026 at 10:25:20AM +0800, joakim.zhang@cixtech.com wrote:
>
> + audss_cru: clock-controller@7110000 {
> + compatible = "cix,sky1-audss-cru";
> + reg = <0x0 0x07110000 0x0 0x10000>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + clocks = <&scmi_clk CLK_TREE_AUDIO_CLK0>,
> + <&scmi_clk CLK_TREE_AUDIO_CLK2>,
> + <&scmi_clk CLK_TREE_AUDIO_CLK4>,
> + <&scmi_clk CLK_TREE_AUDIO_CLK5>;
> + clock-names = "x8k", "x11k", "sys", "48m";
> + power-domains = <&smc_devpd SKY1_PD_AUDIO>;
> + resets = <&s5_syscon SKY1_AUDIO_HIFI5_NOC_RESET_N>;
> + status = "okay";
Drop.
> + };
> +
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v5 1/4] dt-bindings: soc: cix: add sky1 audss cru controller
From: Krzysztof Kozlowski @ 2026-06-22 9:01 UTC (permalink / raw)
To: joakim.zhang
Cc: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang, cix-kernel-upstream, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <20260622022520.3127103-2-joakim.zhang@cixtech.com>
On Mon, Jun 22, 2026 at 10:25:17AM +0800, joakim.zhang@cixtech.com wrote:
> From: Joakim Zhang <joakim.zhang@cixtech.com>
>
> The Cix Sky1 Audio Subsystem (AUDSS) Clock and Reset Unit (CRU)
> groups clock muxing, gating and block-level software reset control
> in a single register block.
>
> Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
> ---
> .../bindings/soc/cix/cix,sky1-audss-cru.yaml | 92 +++++++++++++++++++
> .../dt-bindings/clock/cix,sky1-audss-clock.h | 60 ++++++++++++
> .../dt-bindings/reset/cix,sky1-audss-reset.h | 25 +++++
> 3 files changed, 177 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/cix/cix,sky1-audss-cru.yaml
> create mode 100644 include/dt-bindings/clock/cix,sky1-audss-clock.h
> create mode 100644 include/dt-bindings/reset/cix,sky1-audss-reset.h
Both headers should have the same name as the compatible. I already
requested this some time ago, I think.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v5 8/8] arm64: dts: imx8qxp-mek: add parallel ov5640 camera support
From: guoniu.zhou @ 2026-06-22 9:01 UTC (permalink / raw)
To: Frank.Li
Cc: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
imx, devicetree, linux-arm-kernel
In-Reply-To: <20260617-imx8qxp_pcam-v5-8-7fa6c8e7fba7@nxp.com>
> Add parallel ov5640 nodes in imx8qxp-mek and create overlay file to enable
> it because it can work at two mode: MIPI CSI and parallel mode.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index 711e36cc2c99..f54fd4cdd926 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -434,6 +434,9 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek-pcie-ep.dtb
> imx8qxp-mek-ov5640-csi-dtbs := imx8qxp-mek.dtb imx8qxp-mek-ov5640-csi.dtbo
> dtb-${CONFIG_ARCH_MXC} += imx8qxp-mek-ov5640-csi.dtb
>
> +imx8qxp-mek-ov5640-cpi-dtbs := imx8qxp-mek.dtb imx8qxp-mek-ov5640-cpi.dtbo
> +dtb-${CONFIG_ARCH_MXC} += imx8qxp-mek-ov5640-cpi.dtb
> +
> dtb-$(CONFIG_ARCH_MXC) += imx8qxp-tqma8xqp-mba8xx.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8qxp-tqma8xqps-mb-smarc-2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8ulp-9x9-evk.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso b/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso
> new file mode 100644
> index 000000000000..9fbdd798f17d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2025 NXP
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/clock/imx8-lpcg.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/media/video-interfaces.h>
> +#include <dt-bindings/pinctrl/pads-imx8qxp.h>
> +
> +&cm40_i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov5640_pi: camera@3c {
> + compatible = "ovti,ov5640";
> + reg = <0x3c>;
> + clocks = <&pi0_misc_lpcg IMX_LPCG_CLK_0>;
> + clock-names = "xclk";
> + assigned-clocks = <&pi0_misc_lpcg IMX_LPCG_CLK_0>;
> + assigned-clock-rates = <24000000>;
> + AVDD-supply = <®_2v8>;
> + DOVDD-supply = <®_1v8>;
> + DVDD-supply = <®_1v5>;
> + pinctrl-0 = <&pinctrl_parallel_cpi>;
> + pinctrl-names = "default";
> + powerdown-gpios = <&lsio_gpio3 2 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&lsio_gpio3 3 GPIO_ACTIVE_LOW>;
> +
> + port {
> + ov5640_pi_ep: endpoint {
> + bus-type = <MEDIA_BUS_TYPE_PARALLEL>;
> + bus-width = <8>;
> + hsync-active = <1>;
> + pclk-sample = <1>;
> + remote-endpoint = <¶llel_cpi_in>;
> + vsync-active = <0>;
> + };
> + };
> + };
> +};
> +
> +&iomuxc {
> + pinctrl_parallel_cpi: parallelcpigrp {
> + fsl,pins = <
> + IMX8QXP_CSI_D00_CI_PI_D02 0xc0000041
> + IMX8QXP_CSI_D01_CI_PI_D03 0xc0000041
> + IMX8QXP_CSI_D02_CI_PI_D04 0xc0000041
> + IMX8QXP_CSI_D03_CI_PI_D05 0xc0000041
> + IMX8QXP_CSI_D04_CI_PI_D06 0xc0000041
> + IMX8QXP_CSI_D05_CI_PI_D07 0xc0000041
> + IMX8QXP_CSI_D06_CI_PI_D08 0xc0000041
> + IMX8QXP_CSI_D07_CI_PI_D09 0xc0000041
> +
> + IMX8QXP_CSI_MCLK_CI_PI_MCLK 0xc0000041
> + IMX8QXP_CSI_PCLK_CI_PI_PCLK 0xc0000041
> + IMX8QXP_CSI_HSYNC_CI_PI_HSYNC 0xc0000041
> + IMX8QXP_CSI_VSYNC_CI_PI_VSYNC 0xc0000041
> + IMX8QXP_CSI_EN_LSIO_GPIO3_IO02 0xc0000041
> + IMX8QXP_CSI_RESET_LSIO_GPIO3_IO03 0xc0000041
> + >;
> + };
> +};
> +
> +&isi {
> + status = "okay";
> +};
> +
> +¶llel_cpi {
> + status = "okay";
> +
> + ports {
> + port@0 {
> + parallel_cpi_in: endpoint {
> + hsync-active = <1>;
> + remote-endpoint = <&ov5640_pi_ep>;
> + };
> + };
> + };
> +};
Reviewed-by: Guoniu Zhou <guoniu.zhou@nxp.com>
--
Guoniu Zhou <guoniu.zhou@oss.nxp.com>
^ permalink raw reply
* Re: [PATCH v5 7/8] arm64: dts: imx8: add camera parallel interface (CPI) node
From: guoniu.zhou @ 2026-06-22 9:01 UTC (permalink / raw)
To: Frank.Li
Cc: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
imx, devicetree, linux-arm-kernel
In-Reply-To: <20260617-imx8qxp_pcam-v5-7-7fa6c8e7fba7@nxp.com>
> Add camera parallel interface (CPI) node.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> index a72b2f1c4a1b..b504f99f6acd 100644
> --- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> @@ -222,6 +222,19 @@ irqsteer_parallel: irqsteer@58260000 {
> status = "disabled";
> };
>
> + parallel_cpi: cpi@58261000 {
> + compatible = "fsl,imx8qxp-pcif";
> + reg = <0x58261000 0x1000>;
> + clocks = <&pi0_pxl_lpcg IMX_LPCG_CLK_0>,
> + <&pi0_ipg_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "pixel", "ipg";
> + assigned-clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_PER>;
> + assigned-clock-parents = <&clk IMX_SC_R_PI_0_PLL IMX_SC_PM_CLK_PLL>;
> + assigned-clock-rates = <160000000>;
> + power-domains = <&pd IMX_SC_R_PI_0>;
> + status = "disabled";
> + };
> +
> pi0_ipg_lpcg: clock-controller@58263004 {
> compatible = "fsl,imx8qxp-lpcg";
> reg = <0x58263004 0x4>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> index 232cf25dadfc..5aae15540d6c 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> @@ -62,6 +62,14 @@ isi_in_2: endpoint {
> remote-endpoint = <&mipi_csi0_out>;
> };
> };
> +
> + port@4 {
> + reg = <4>;
> +
> + isi_in_4: endpoint {
> + remote-endpoint = <¶llel_cpi_out>;
> + };
> + };
> };
> };
>
> @@ -95,3 +103,22 @@ &jpegenc {
> &mipi_csi_1 {
> status = "disabled";
> };
> +
> +¶llel_cpi {
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + parallel_cpi_out: endpoint {
> + remote-endpoint = <&isi_in_4>;
> + };
> + };
> + };
> +};
Reviewed-by: Guoniu Zhou <guoniu.zhou@nxp.com>
--
Guoniu Zhou <guoniu.zhou@oss.nxp.com>
^ permalink raw reply
* Re: [PATCH v5 6/8] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
From: guoniu.zhou @ 2026-06-22 9:01 UTC (permalink / raw)
To: Frank.Li
Cc: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
imx, devicetree, linux-arm-kernel, Alice Yuan, Robert Chiras,
Zhipeng Wang
In-Reply-To: <20260617-imx8qxp_pcam-v5-6-7fa6c8e7fba7@nxp.com>
On Wed, 17 Jun 2026 15:50:16 -0400, Frank.Li@oss.nxp.com <Frank.Li@oss.nxp.com> wrote:
> diff --git a/drivers/media/platform/nxp/imx-parallel-cpi.c b/drivers/media/platform/nxp/imx-parallel-cpi.c
> new file mode 100644
> index 000000000000..00f5d5f47644
> --- /dev/null
> +++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
> @@ -0,0 +1,614 @@
> [ ... skip 245 lines ... ]
> + }
> +
> + val = CPI_CTRL_REG1_PIXEL_WIDTH(pixel_width) |
> + CPI_CTRL_REG1_VSYNC_PULSE(vsync_pulse);
> + writel(val, pcpidev->regs + pdata->interface_ctrl_reg1);
> +}
The switch statement result is overwritten.
--
Guoniu Zhou <guoniu.zhou@oss.nxp.com>
^ permalink raw reply
* Re: [PATCH v5 3/8] media: synopsys: Use v4l2_subdev_get_frame_desc_passthrough()
From: guoniu.zhou @ 2026-06-22 9:01 UTC (permalink / raw)
To: Frank.Li
Cc: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
imx, devicetree, linux-arm-kernel
In-Reply-To: <20260617-imx8qxp_pcam-v5-3-7fa6c8e7fba7@nxp.com>
> Replace the local frame descriptor callback implementation with
> v4l2_subdev_get_frame_desc_passthrough().
>
> This helper provides the same functionality while avoiding duplicate
> code and simplifying the driver implementation.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>
> diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> index 41e48365167e..f51367409ff4 100644
> --- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> +++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> @@ -630,31 +630,11 @@ static int dw_mipi_csi2rx_disable_streams(struct v4l2_subdev *sd,
> return ret;
> }
>
> -static int
> -dw_mipi_csi2rx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> - struct v4l2_mbus_frame_desc *fd)
> -{
> - struct dw_mipi_csi2rx_device *csi2 = to_csi2(sd);
> - struct v4l2_subdev *remote_sd;
> - struct media_pad *remote_pad;
> -
> - remote_pad = media_pad_remote_pad_unique(&csi2->pads[DW_MIPI_CSI2RX_PAD_SINK]);
> - if (IS_ERR(remote_pad)) {
> - dev_err(csi2->dev, "can't get remote source pad\n");
> - return PTR_ERR(remote_pad);
> - }
> -
> - remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
> -
> - return v4l2_subdev_call(remote_sd, pad, get_frame_desc,
> - remote_pad->index, fd);
> -}
> -
> static const struct v4l2_subdev_pad_ops dw_mipi_csi2rx_pad_ops = {
> .enum_mbus_code = dw_mipi_csi2rx_enum_mbus_code,
> .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = dw_mipi_csi2rx_set_fmt,
> - .get_frame_desc = dw_mipi_csi2rx_get_frame_desc,
> + .get_frame_desc = v4l2_subdev_get_frame_desc_passthrough,
> .set_routing = dw_mipi_csi2rx_set_routing,
> .enable_streams = dw_mipi_csi2rx_enable_streams,
> .disable_streams = dw_mipi_csi2rx_disable_streams,
Reviewed-by: Guoniu Zhou <guoniu.zhou@nxp.com>
--
Guoniu Zhou <guoniu.zhou@oss.nxp.com>
^ permalink raw reply
* Re: [PATCH v3 09/12] iio: dac: ad5686: implement new sync() op for the spi bus
From: Rodrigo Alencar @ 2026-06-22 8:50 UTC (permalink / raw)
To: David Lechner, rodrigo.alencar, Michael Auchter, linux, linux-iio,
devicetree, linux-kernel, linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Kees Cook,
Gustavo A. R. Silva
In-Reply-To: <50a7e765-ba20-45c0-8a13-5e672413b600@baylibre.com>
On 20/06/26 11:33, David Lechner wrote:
> On 6/16/26 3:21 AM, Rodrigo Alencar via B4 Relay wrote:
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> >
> > Use of local SPI bus data to manage a collection of SPI transfers and
> > flush them to the SPI platform driver with the sync() operation. This
> > allows for faster handling of multiple channel DAC writes, avoiding kernel
> > overhead per spi_sync() call, which will be helpful when enabling
> > triggered buffer support.
...
> > +/**
> > + * struct ad5686_spi_data - SPI bus specific data
> > + * @msg: SPI message used for transfers
> > + * @size: number of transfers currently in the message
> > + * @capacity: maximum number of transfers that can be added to the message
> > + * @xfers: array of SPI transfers, allocated with the provided capacity
> > + */
> > +struct ad5686_spi_data {
> > + struct spi_message msg;
> > + unsigned int size;
> > + unsigned int capacity;
> > + struct spi_transfer xfers[] __counted_by(capacity);
> > +};
> > +
> > static int ad5686_spi_write(struct ad5686_state *st,
> > u8 cmd, u8 addr, u16 val)
> > {
> > - struct spi_device *spi = to_spi_device(st->dev);
> > - u8 tx_len, *buf;
> > + struct ad5686_spi_data *bus_data = st->bus_data;
> > + struct spi_transfer *xfer;
> >
> > + if (bus_data->size >= bus_data->capacity)
> > + return -E2BIG;
> > +
> > + if (bus_data->size)
> > + bus_data->xfers[bus_data->size - 1].cs_change = 1;
> > + else
> > + spi_message_init(&bus_data->msg);
>
> Seems odd that spi_message_init() is called conditionally. What
> prevents spi_message_add_tail() from growing the message unbounded
> on repeated calls?
The "size >= capacity" check right above this.
>
> > +
> > + xfer = &bus_data->xfers[bus_data->size];
> > switch (st->chip_info->regmap_type) {
> > case AD5310_REGMAP:
> > - st->data[0].d16 = cpu_to_be16(AD5310_CMD(cmd) |
> > - val);
> > - buf = &st->data[0].d8[0];
> > - tx_len = 2;
> > + st->data[bus_data->size].d16 =
> > + cpu_to_be16(AD5310_CMD(cmd) | val);
> > + *xfer = (struct spi_transfer) {
> > + .tx_buf = &st->data[bus_data->size].d16,
> > + .len = sizeof(st->data[bus_data->size].d16),
> > + };
> > break;
> > case AD5683_REGMAP:
> > - st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
> > - AD5683_DATA(val));
> > - buf = &st->data[0].d8[1];
> > - tx_len = 3;
> > + st->data[bus_data->size].d32 =
> > + cpu_to_be32(AD5686_CMD(cmd) | AD5683_DATA(val));
> > + *xfer = (struct spi_transfer) {
> > + .tx_buf = &st->data[bus_data->size].d8[1],
> > + .len = sizeof(st->data[bus_data->size].d32) - 1,
> > + };
> > break;
> > case AD5686_REGMAP:
> > - st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
> > - AD5686_ADDR(addr) |
> > - val);
> > - buf = &st->data[0].d8[1];
> > - tx_len = 3;
> > + st->data[bus_data->size].d32 =
> > + cpu_to_be32(AD5686_CMD(cmd) | AD5686_ADDR(addr) | val);
> > + *xfer = (struct spi_transfer) {
> > + .tx_buf = &st->data[bus_data->size].d8[1],
> > + .len = sizeof(st->data[bus_data->size].d32) - 1,
> > + };
> > break;
> > default:
> > return -EINVAL;
> > }
> >
> > - return spi_write(spi, buf, tx_len);
>
> If this function no longer writes, should we change the name of
> the function to something like ad5686_spi_write_prepare_msg()?
>
> > + spi_message_add_tail(xfer, &bus_data->msg);
> > + bus_data->size++;
> > +
> > + return 0;
> > +}
> > +
> > +static int ad5686_spi_sync(struct ad5686_state *st)
> > +{
> > + struct spi_device *spi = to_spi_device(st->dev);
> > + struct ad5686_spi_data *bus_data = st->bus_data;
> > +
> > + bus_data->size = 0; /* always reset, even on sync failure */
> > + return spi_sync(spi, &bus_data->msg);
> > }
> >
> > static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
> > {
> > - struct spi_transfer t[] = {
> > - {
> > - .tx_buf = &st->data[0].d8[1],
> > - .len = 3,
> > - .cs_change = 1,
> > - }, {
> > - .tx_buf = &st->data[1].d8[1],
> > - .rx_buf = &st->data[2].d8[1],
> > - .len = 3,
> > - },
> > - };
> > struct spi_device *spi = to_spi_device(st->dev);
> > + struct ad5686_spi_data *bus_data = st->bus_data;
> > + struct spi_transfer *xfer = &bus_data->xfers[0];
> > u8 cmd = 0;
> > int ret;
> >
> > @@ -85,8 +117,21 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
> > AD5686_ADDR(addr));
> > st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
> >
> > - ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t));
> > - if (ret < 0)
> > + xfer[0] = (struct spi_transfer) {
> > + .tx_buf = &st->data[0].d8[1],
> > + .len = sizeof(st->data[0].d32) - 1,
>
> Would make more sense to say `sizeof(st->data[0].d8) - 1` since
> the buffer is &st->data[0].d8[1].
the buffer is d8[1], d8[2] and d8[3], skipping d8[0], for a len = 3.
> > + .cs_change = 1,
> > + };
> > + xfer[1] = (struct spi_transfer) {
> > + .tx_buf = &st->data[1].d8[1],
> > + .rx_buf = &st->data[2].d8[1],
> > + .len = sizeof(st->data[1].d32) - 1,
>
> And here.
>
> > + };
> > +
> > + spi_message_init_with_transfers(&bus_data->msg, xfer, 2);
> > +
> > + ret = spi_sync(spi, &bus_data->msg);
> > + if (ret)
> > return ret;
> >
> > return be32_to_cpu(st->data[2].d32);
> > @@ -95,12 +140,26 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
> > static const struct ad5686_bus_ops ad5686_spi_ops = {
> > .write = ad5686_spi_write,
> > .read = ad5686_spi_read,
> > + .sync = ad5686_spi_sync,
> > };
> >
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply
* Re: [PATCH v2 5/5] PCI: spacemit-k1: Add Spacemit K3 PCIe host controller support
From: Inochi Amaoto @ 2026-06-22 8:50 UTC (permalink / raw)
To: Alex Elder, Inochi Amaoto, Jingoo Han, Manivannan Sadhasivam,
Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yixun Lan,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Christian Bruel, Vincent Guittot, Senchuan Zhang, Nam Cao,
Siddharth Vadapalli, Randolph Lin, Andy Shevchenko, Vidya Sagar,
Neil Armstrong, Gustavo Pimentel
Cc: linux-pci, devicetree, linux-kernel, linux-riscv, spacemit,
Yixun Lan, Longbin Li
In-Reply-To: <2dec4fe6-30d8-4949-bdc1-e32508340b87@riscstar.com>
On Tue, Jun 09, 2026 at 11:11:24AM -0500, Alex Elder wrote:
> On 5/16/26 8:48 PM, Inochi Amaoto wrote:
> > The PCIe controller on Spacemit K3 is almost a standard Synopsys
> > DesignWare PCIe IP with extra link and reset control. Unlike
> > the PCIe controller on K1, this controller supports external MSI
> > interrupt controller and can use multiple PHYs at the same time.
> >
> > Add driver to support PCIe controller on Spacemit K3 PCIe.
>
> It seems like you're creating a lot of new code for K3. In
> some cases it's very similar to K1, and it's not clear why it
> needs to be different.
>
> I'd much rather see a patch that prepares for K3 support by
> doing minor refactoring of the existing code to support K1
> in a way that will make adding K3 support more natural.
> Then a patch to enable K3 should be simpler (and can focus
> on what is truly different).
>
I can not understand what you see, the init logic of K3 is
completely different from the K1, so a new function is need.
For other logic, I have reused all things as I can.
> >
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> > drivers/pci/controller/dwc/Kconfig | 4 +-
> > drivers/pci/controller/dwc/pcie-spacemit-k1.c | 169 ++++++++++++++++++
> > 2 files changed, 171 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index f2fde13107f2..fae971ecd876 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -439,7 +439,7 @@ config PCIE_SOPHGO_DW
> > Sophgo SoCs.
> > config PCIE_SPACEMIT_K1
> > - tristate "SpacemiT K1 PCIe controller (host mode)"
> > + tristate "SpacemiT K1/K3 PCIe controller (host mode)"
> > depends on ARCH_SPACEMIT || COMPILE_TEST
> > depends on HAS_IOMEM
> > select PCIE_DW_HOST
> > @@ -447,7 +447,7 @@ config PCIE_SPACEMIT_K1
> > default ARCH_SPACEMIT
> > help
> > Enables support for the DesignWare based PCIe controller in
> > - the SpacemiT K1 SoC operating in host mode. Three controllers
> > + the SpacemiT K1/K3 SoC operating in host mode. Three controllers
> > are available on the K1 SoC; the first of these shares a PHY
> > with a USB 3.0 host controller (one or the other can be used).
> > diff --git a/drivers/pci/controller/dwc/pcie-spacemit-k1.c b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> > index 7f6f1df31cd8..7854d26220a9 100644
> > --- a/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> > +++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> > @@ -23,6 +23,7 @@
> > #define PCI_VENDOR_ID_SPACEMIT 0x201f
> > #define PCI_DEVICE_ID_SPACEMIT_K1 0x0001
> > +#define PCI_DEVICE_ID_SPACEMIT_K3 0x0002
> > /* Offsets and field definitions for link management registers */
> > #define K1_PHY_AHB_IRQ_EN 0x0000
> > @@ -32,8 +33,20 @@
> > #define SMLH_LINK_UP BIT(1)
> > #define RDLH_LINK_UP BIT(12)
> > +#define INTR_STATUS 0x0010
> > +
> > #define INTR_ENABLE 0x0014
> > #define MSI_CTRL_INT BIT(11)
> > +#define RDLH_LINK_UP_INT BIT(20)
> > +
> > +#define K3_PHY_AHB_IRQSTATUS_INTX 0x0008
>
> Can you add INTX support for K1 as well (perhaps in a separate
> patch)?
>
I have no knowledge about INTx support, and the vendor refuse
to provide any information about this. So no support for this.
> > +#define K3_ADDR_INTR_STATUS1 0x0018
> > +
> > +#define K3_CACHE_MSTR_AWCACHE_MODE GENMASK(14, 11)
> > +#define K3_CACHE_MSTR_AWCACHE_BEHAVIOR 0xf
> > +
> > +#define K3_MAX_PHY_NUMBER 6
>
> You used "count" in patch 2 as the field name.
>
> > /* Some controls require APMU regmap access */
> > #define SYSCON_APMU "spacemit,apmu"
> > @@ -48,6 +61,9 @@
> > #define PCIE_CONTROL_LOGIC 0x0004
> > #define PCIE_SOFT_RESET BIT(0)
> > +#define PCIE_PERSTN_OE BIT(24)
> > +#define PCIE_PERSTN_OUT BIT(25)
> > +#define PCIE_IGNORE_PERSTN BIT(31)
> > struct k1_pcie {
> > struct dw_pcie pci;
> > @@ -262,6 +278,152 @@ static const struct dw_pcie_ops k1_pcie_ops = {
> > .stop_link = k1_pcie_stop_link,
> > };
> > +static int k3_pcie_enable_phy(struct k1_pcie *pcie)
>
> Can you just make K1's single PHY be a special case of
> having "N" PHYs? I.e., just set the phy_count for
> K1 to be 1, so this loop would work for both K1 and K3?
>
This is reasonable.
> > +{
> > + int i, ret;
> > +
> > + for (i = 0; i < pcie->phy_count; i++) {
> > + ret = phy_init(pcie->phy[i]);
> > + if (ret)
> > + goto err_phy;
> > + }
> > +
> > + return 0;
> > +
> > +err_phy:
> > + while (--i >= 0)
> > + phy_exit(pcie->phy[i]);
> > +
> > + return ret;
> > +}
> > +
> > +static int k3_pcie_init(struct dw_pcie_rp *pp)
> > +{
> > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > + struct k1_pcie *k1 = to_k1_pcie(pci);
> > + u32 reset_ctrl = k1->pmu_off + PCIE_CLK_RESET_CONTROL;
> > + u32 val;
> > + int ret;
> > +
> > + regmap_clear_bits(k1->pmu, reset_ctrl, LTSSM_EN);
>
> Should the above be done for K1? Would it hurt? Handle
> both K1 and K3 the same way if possible.
>
> The next two things are identical to k1_pcie_init(). Make
> the code common if possible, so it's very obvious what
> really needs to be different between the two.
>
No, I have diff the code and find only about 40% of logic
are similar. However, these logic is fragmented, so I decide
to add a new function, which is easier to maintain.
> > +
> > + k1_pcie_toggle_soft_reset(k1);
>
> The "k1" prefix is fine for now, but if this driver gets
> used for more devices in the future, it might be worth
> renaming things to emphasize that it's not K1-specific.
>
Currently I have no knowledge on the new device and do not
think it is a good idea to change the name. Anyway, it is
possible to change to kx, but I do not think it is clear.
> > + ret = k1_pcie_enable_resources(k1);
> > + if (ret)
> > + return ret;
> > +
> > + regmap_set_bits(k1->pmu, reset_ctrl, PCIE_AUX_PWR_DET);
> > + regmap_clear_bits(k1->pmu, reset_ctrl, APP_HOLD_PHY_RST);
> > +
>
> You enable the PHY here much earlier than what's done in
> the K1 code. Should the K1 PHY be enabled earlier?
> Also, I don't really think there needs to be separate
> versions of the code that enables PHYs for K1 and K3.
>
IIRC this is a specific logic for K3.
> > + ret = k3_pcie_enable_phy(k1);
> > + if (ret) {
> > + k1_pcie_disable_resources(k1);
> > + return ret;
> > + }
> > +
> > + /* K3: Set IGNORE_PERSTN and drive PERSTN_OE high (assert reset) */
> > + regmap_set_bits(k1->pmu, k1->pmu_off + PCIE_CONTROL_LOGIC,
> > + PCIE_IGNORE_PERSTN | PCIE_PERSTN_OE | PCIE_PERSTN_OUT);
> > + usleep_range(1000, 2000);
> > + regmap_clear_bits(k1->pmu, k1->pmu_off + PCIE_CONTROL_LOGIC, PCIE_PERSTN_OUT);
> > +
> > + msleep(PCIE_T_PVPERL_MS);
> > +
> > + /*
> > + * Put the controller in root complex mode, and indicate that
> > + * Vaux (3.3v) is present.
> > + */
> > + regmap_set_bits(k1->pmu, k1->pmu_off + PCIE_CONTROL_LOGIC,
> > + PCIE_PERSTN_OUT | PCIE_PERSTN_OE);
> > +
> > + val = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
> > + val = u32_replace_bits(val, GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE,
> > + GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC);
> > + dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, val);
> > +
>
> The following block of code (roughly) is done right after
> enabling resources in the K1 version of this function.
>
> Maybe the order you do it is better, but in that case,
> change the (existing, and soon, common) code to do it
> however is best if that's the case.
>
> You should try to factor out the common parts and minimize
> what's actually different between the two.
>
> I would also expect that the device ID would be stored in the
> platform data rather than having both init functions hard-code
> the value here.
>
> I'm going to leave it at that for now.
>
> -Alex
This first thing you should know is the init logic of K3
is totally different from K1, I have asked for the vendor
and they told me it is better to treat them differently,
even if they are using the same ip.
For the request of the device ID setup, I think I can add
a function for it.
>
> > + dw_pcie_dbi_ro_wr_en(pci);
> > + dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, PCI_VENDOR_ID_SPACEMIT);
> > + dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, PCI_DEVICE_ID_SPACEMIT_K3);
> > + dw_pcie_dbi_ro_wr_dis(pci);
> > +
> > + /* Finally, as a workaround, disable ASPM L1 */
> > + k1_pcie_disable_aspm_l1(k1);
> > +
> > + return 0;
> > +}
> > +
> > +static int k3_pcie_msi_host_init(struct dw_pcie_rp *pp)
> > +{
> > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > + u32 val;
> > +
> > + dw_pcie_dbi_ro_wr_en(pci);
> > +
> > + val = dw_pcie_readl_dbi(pci, COHERENCY_CONTROL_3_OFF);
> > + val |= u32_replace_bits(val, K3_CACHE_MSTR_AWCACHE_BEHAVIOR,
> > + K3_CACHE_MSTR_AWCACHE_MODE);
> > + dw_pcie_writel_dbi(pci, COHERENCY_CONTROL_3_OFF, val);
> > +
> > + dw_pcie_dbi_ro_wr_dis(pci);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct dw_pcie_host_ops k3_pcie_host_ops = {
> > + .init = k3_pcie_init,
> > + .deinit = k1_pcie_deinit,
> > + .msi_init = k3_pcie_msi_host_init,
> > +};
> > +
> > +static const struct dw_pcie_ops k3_pcie_ops = {
> > + .link_up = k1_pcie_link_up,
> > + .start_link = k1_pcie_start_link,
> > + .stop_link = k1_pcie_stop_link,
> > +};
> > +
> > +static void k3_pcie_clear_irq_status(struct k1_pcie *k1,
> > + u32 *status0, u32 *status1, u32 *status2)
> > +{
> > + *status0 = readl_relaxed(k1->link + K3_PHY_AHB_IRQSTATUS_INTX);
> > + *status1 = readl_relaxed(k1->link + INTR_STATUS);
> > + *status2 = readl_relaxed(k1->link + K3_ADDR_INTR_STATUS1);
> > +
> > + writel_relaxed(*status0, k1->link + K3_PHY_AHB_IRQSTATUS_INTX);
> > + writel_relaxed(*status1, k1->link + INTR_STATUS);
> > + writel_relaxed(*status2, k1->link + K3_ADDR_INTR_STATUS1);
> > +}
> > +
> > +static int k3_pcie_parse_port(struct k1_pcie *k1)
> > +{
> > + struct device *dev = k1->pci.dev;
> > + u32 status0, status1, status2;
> > + int i;
> > +
> > + k1->phy = devm_kmalloc_array(dev, K3_MAX_PHY_NUMBER, sizeof(*k1->phy),
> > + GFP_KERNEL);
> > + if (!k1->phy)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < K3_MAX_PHY_NUMBER; i++) {
> > + k1->phy[i] = devm_of_phy_get_by_index(dev, dev->of_node, i);
> > + if (IS_ERR(k1->phy[i])) {
> > + if (PTR_ERR(k1->phy[i]) == -ENODEV)
> > + break;
> > +
> > + return PTR_ERR(k1->phy[i]);
> > + }
> > + }
> > +
> > + k1->phy_count = i;
> > + if (k1->phy_count == 0)
> > + return -EINVAL;
> > +
> > + k3_pcie_clear_irq_status(k1, &status0, &status1, &status2);
> > +
> > + return 0;
> > +}
> > +
> > static int k1_pcie_parse_port(struct k1_pcie *k1)
> > {
> > struct device *dev = k1->pci.dev;
> > @@ -363,8 +525,15 @@ static const struct k1_pcie_device_data k1_pcie_device_data = {
> > .parse_port = k1_pcie_parse_port,
> > };
> > +static const struct k1_pcie_device_data k3_pcie_device_data = {
> > + .host_ops = &k3_pcie_host_ops,
> > + .ops = &k3_pcie_ops,
> > + .parse_port = k3_pcie_parse_port,
> > +};
> > +
> > static const struct of_device_id k1_pcie_of_match_table[] = {
> > { .compatible = "spacemit,k1-pcie", .data = &k1_pcie_device_data},
> > + { .compatible = "spacemit,k3-pcie", .data = &k3_pcie_device_data},
> > { }
> > };
>
^ permalink raw reply
* Re: [PATCH v2 2/2] software node: Fix software_node_get_reference_args() with index -1
From: Alban Bedel @ 2026-06-22 8:41 UTC (permalink / raw)
To: Andy Shevchenko
Cc: driver-core, devicetree, Daniel Scally, Heikki Krogerus,
Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Saravana Kannan, Zijun Hu,
linux-kernel, Alban Bedel
In-Reply-To: <ajQ6j5roSZ16Yb-M@black.igk.intel.com>
On Thu, 18 Jun 2026 20:35:59 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Thu, Jun 18, 2026 at 05:20:35PM +0200, Alban Bedel wrote:
> > The bounds check for the index passed to
> > software_node_get_reference_args() was failing when passed UINT_MAX,
> > this in turn would lead to an out of bound access in the property
> > array. Fix the bound check to also cover the UINT_MAX case.
>
> ...
>
> > - if ((index + 1) * sizeof(*ref) > prop->length)
> > + if (index >= prop->length / sizeof(*ref))
>
> It trades multiplication for division (which might be not always
> power-of-two).
This code is not really performance relevant and using a non trivial
expression lead to the currently buggy code. I find that easy to
understand and obviously correct code is better suited here.
What alternative would you suggest?
Alban
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox