* Re: [PATCH v4 2/4] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs
From: sashiko-bot @ 2026-06-25 14:47 UTC (permalink / raw)
To: Marcelo Schmitt; +Cc: robh, conor+dt, devicetree
In-Reply-To: <0abf862eeb277ae82262d09b11b1446f04c47be7.1782397418.git.marcelo.schmitt@analog.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The voltage from the 'refin' regulator is not multiplied by the internal buffer gain, resulting in incorrect ADC scale.
- [Medium] External 'ref' regulator is completely ignored for chips with an internal reference, breaking the ability to overdrive the internal reference.
--
commit 06f09452b5298cd69acd94f1c33c2c91342a505d
Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs
This commit adds a new driver for the Analog Devices LTC2378-20 and
similar analog-to-digital converters.
> diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> new file mode 100644
> index 0000000000000..9a9f32e4989ba
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2378.c
[ ... ]
> +static int ltc2378_regulator_setup(struct device *dev, struct ltc2378_state *st)
> +{
> + int ret;
> +
> + ret = devm_regulator_get_enable_read_voltage(dev, "refin");
> + if (ret < 0 && ret != -ENODEV) {
> + return dev_err_probe(dev, ret, "failed to read refin regulator\n");
> + } else if (ret > 0) {
> + st->ref_uV = ret;
[Severity: High]
Does this assignment account for the internal buffer's amplification gain?
For example, on chips like the LTC2338-18, the internal reference buffer
amplifies the REFIN voltage by a factor of 2. If the 'refin' regulator provides
2.048V, the internal REF pin sees 4.096V. But this code appears to assign the
direct REFIN voltage to st->ref_uV, which would result in an incorrect ADC
scale calculation.
> + return 0;
> + }
> +
> + if (st->info->internal_ref_uv) {
> + st->ref_uV = st->info->internal_ref_uv;
> + return 0;
[Severity: Medium]
Does this early return prevent a board from overdriving the internal reference
with an external 'ref' supply?
If the hardware supports overdriving the internal reference and a 'ref-supply'
is provided in the device tree, it looks like this code will silently ignore
it and use the internal reference value instead.
> + }
> +
> + ret = devm_regulator_get_enable_read_voltage(dev, "ref");
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to read ref regulator\n");
> +
> + st->ref_uV = ret;
> +
> + return 0;
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782397418.git.marcelo.schmitt@analog.com?part=2
^ permalink raw reply
* Re: [PATCH v4 3/4] iio: adc: ltc2378: Enable high-speed data capture
From: sashiko-bot @ 2026-06-25 14:52 UTC (permalink / raw)
To: Marcelo Schmitt; +Cc: devicetree, robh, conor+dt
In-Reply-To: <7d91f48bb03a16e6876e7aeb05fe259e33ae625f.1782397418.git.marcelo.schmitt@analog.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing verification of SPI offload trigger offset after validation loop in `ltc2378_update_conversion_rate()` allows silent data corruption.
- [Medium] In-out parameters accumulate drift across iterative rounding loops in `ltc2378_update_conversion_rate()`.
- [Medium] Torn state on error path in `ltc2378_update_conversion_rate()` due to partially updated `st->offload_trigger_config`.
- [Low] Data race on `st->cnv_Hz` between sysfs readers and writers.
--
commit 57c4b9b3af36aafb3888623014ea397ad6cf0cd6
Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
iio: adc: ltc2378: Enable high-speed data capture
Make use of SPI transfer offloading to speed up data capture, enabling data
acquisition at faster sample rates (up to 2 MSPS).
> diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> --- a/drivers/iio/adc/ltc2378.c
> +++ b/drivers/iio/adc/ltc2378.c
[ ... ]
> @@ -254,7 +345,126 @@ static int ltc2378_read_raw(struct iio_dev *indio_dev,
> *val2 = chan->scan_type.realbits;
>
> return IIO_VAL_FRACTIONAL_LOG2;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = st->cnv_Hz;
[Severity: Low]
When reading the frequency via ltc2378_read_raw(), st->cnv_Hz is read without
acquiring the direct mode lock. Since it can be concurrently modified by
ltc2378_update_conversion_rate() during a write, does this cause a data race?
Should this be protected or accessed via READ_ONCE()?
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
[ ... ]
> +static int ltc2378_update_conversion_rate(struct ltc2378_state *st, int freq_Hz)
> +{
[ ... ]
> + /* Configure CNV PWM waveform */
> + cnv_wf.period_length_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, freq_Hz);
>
> + /*
> + * Ensure CNV high time meets minimum requirement (20ns). The PWM
> + * hardware may round the duty cycle, so iterate until we get at least
> + * the minimum required high time.
> + */
> + do {
> + cnv_wf.duty_length_ns = target;
> + ret = pwm_round_waveform_might_sleep(st->cnv_trigger, &cnv_wf);
[Severity: Medium]
The pwm_round_waveform_might_sleep() function can modify the passed cnv_wf
structure in place. Since the base period (cnv_wf.period_length_ns) is set
before the loop and not reset inside, does this cause the period to drift
across the iterations if the hardware constraints cause it to be repeatedly
rounded?
> + if (ret)
> + return ret;
> + target += 10; /* Increment by PWM duty cycle period */
> + } while (count++ < 100 && cnv_wf.duty_length_ns < LTC2378_TCNV_HIGH_NS);
> +
> + /* Double check the minimum CNV high time is met */
> + if (cnv_wf.duty_length_ns < LTC2378_TCNV_HIGH_NS)
> + return -EIO;
> +
> + /*
> + * Configure SPI offload PWM trigger.
> + * The trigger should fire after tBUSYLH + tCONV + tDSDOBUSYL.
> + * Minimum time needed: TBUSYLH (13ns) + TCONV (part-specific) + TDSDOBUSYL (5ns)
> + *
> + * Use the same period as CNV PWM to avoid timing issues.
> + * Convert back from period to frequency for the SPI offload API.
> + */
> + offload_period_ns = cnv_wf.period_length_ns;
> + config->periodic.frequency_hz = DIV_ROUND_UP(HZ_PER_GHZ, offload_period_ns);
> + min_read_offset = LTC2378_TBUSYLH_NS + st->info->tconv_ns + LTC2378_TDSDOBUSYL_NS;
> + offload_offset_ns = min_read_offset;
> + count = 0;
> + do {
> + config->periodic.offset_ns = offload_offset_ns;
> + ret = spi_offload_trigger_validate(st->offload_trigger, config);
[Severity: Medium]
The config pointer aliases st->offload_trigger_config. If this validation
fails, the function returns the error directly below without rolling back
the st->offload_trigger_config state. Could this leave the device in an
inconsistent state where the old st->cnv_wf configuration is paired with
a newly modified but rejected SPI trigger configuration upon the next
postenable?
Additionally, similar to the PWM loop above, spi_offload_trigger_validate()
modifies config in place. Does config->periodic.frequency_hz drift across
iterations since it is not reset inside the loop?
> + if (ret)
> + return ret;
> + offload_offset_ns += 10;
> + } while (count++ < 100 && config->periodic.offset_ns < min_read_offset);
[Severity: High]
If the loop exhausts its 100 retries without meeting the minimum offset
requirement, won't it silently proceed here without returning an error?
Unlike the preceding PWM loop which explicitly checks the bounds afterward,
could this missing check cause silent data corruption if the SPI controller
ends up reading while the ADC is still converting?
> +
> + st->cnv_wf = cnv_wf;
> + st->cnv_Hz = DIV_ROUND_CLOSEST_ULL(HZ_PER_GHZ, cnv_wf.period_length_ns);
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782397418.git.marcelo.schmitt@analog.com?part=3
^ permalink raw reply
* Re: [PATCH v6 2/2] drm/bridge: Add Lontium LT9611C(EX/UXD) MIPI DSI to HDMI driver
From: Dmitry Baryshkov @ 2026-06-25 14:57 UTC (permalink / raw)
To: Sunyun Yang
Cc: Krzysztof Kozlowski, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, maarten.lankhorst, rfoss, mripard,
Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
dri-devel, linux-kernel, xmzhu, xmzhu, rlyu, xbpeng
In-Reply-To: <CAFQXuNZVE6cZJGwrGKGtWnB-seSJLHFh8zW3jjAs6U4JLZFWng@mail.gmail.com>
On Thu, Jun 25, 2026 at 09:26:47PM +0800, Sunyun Yang wrote:
> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 21:17写道：
> >
> > On 25/06/2026 15:14, Sunyun Yang wrote:
> > > Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 20:54写道：
> > >>
> > >> On 08/05/2026 15:40, syyang@lontium.com wrote:
> > >>> +
> > >>> +static void lt9611c_reset(struct lt9611c *lt9611c)
> > >>> +{
> > >>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> > >>> + msleep(20);
> > >>> +
> > >>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 0);
> > >>> + msleep(20);
> > >>> +
> > >>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> > >>
> > >> This is just plain wrong. Why do you assert, then de-assert and then
> > >> finally assert AGAIN the reset leaving the device in powerdown stage?
> > >>
> > > I am using software to emulate the hardware RESET button on our EVB.
> > > When the hardware RESET button is pressed while our chip is running,
> > > the signal level changes from HIGH to LOW and then back to HIGH.
> > >
> > > Of course, we can also use the following:
> > > static void lt9611c_reset(struct lt9611c *lt9611c)
> > > {
> > > gpiod_set_value_cansleep(lt9611c->reset_gpio, 0);
> > > msleep(50);
> > > gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> > > msleep(20);
> > > }
> >
> > Makes no sense either and you just did not get the point and did not
> > answer my question. I asked WHY you leave asserted. Answer "we emulate"
> > is just plain wrong.
> >
> > So again please answer:
> >
> > Why do you leave device with reset asserted?
> >
>
> devicetree: reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;
It should be GPIO_ACTIVE_LOW, if the pin as active-low.
>
> GPIO_ACTIVE_HIGH:
>
> gpiod_set_value_cansleep(lt9611c->reset_gpio, 0); ------ reset pin
> is Low level : Clear the register configuration in the chip to stop
> the chip from working.
>
> gpiod_set_value_cansleep(lt9611c->reset_gpio, 1); ------ reset pin
> is high level: The chip resumes operation.
>
>
>
> > Best regards,
> > Krzysztof
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v4 4/4] iio: adc: ltc2378: Enable triggered buffer data capture
From: sashiko-bot @ 2026-06-25 15:00 UTC (permalink / raw)
To: Marcelo Schmitt; +Cc: robh, conor+dt, devicetree
In-Reply-To: <a98804f122b7c1d1c159ba20060a250ebe2caaed.1782397418.git.marcelo.schmitt@analog.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Single reads via sysfs (`in_voltage0_raw`) return scrambled/garbage data when the SPI offload engine is configured, because they incorrectly use the offload channel's endianness and shift properties.
--
commit 5a2926870e368170637e86d31e6346617e37a6b1
Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
iio: adc: ltc2378: Enable triggered buffer data capture
Enable users to run triggered data captures with LTC2378 and similar ADCs.
> diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> index 5b28630003b18..fcccd27745490 100644
> --- a/drivers/iio/adc/ltc2378.c
> +++ b/drivers/iio/adc/ltc2378.c
[ ... ]
> @@ -640,8 +662,16 @@ static int ltc2378_probe(struct spi_device *spi)
> /* Fall back to low speed usage when no SPI offload is available. */
> if (ret == -ENODEV) {
> indio_dev->info = <c2378_iio_info;
> - indio_dev->channels = &st->info->chan;
> - indio_dev->num_channels = 1;
> + indio_dev->channels = st->info->chan;
> + indio_dev->num_channels = ARRAY_SIZE(st->info->chan);
> +
> + ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> + iio_pollfunc_store_time,
> + ltc2378_trigger_handler,
> + NULL);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to setup triggered buffer\n");
> } else if (ret) {
> return dev_err_probe(dev, ret, "failed to get offload\n");
> } else {
[Severity: High]
This is a pre-existing issue, but do single reads via sysfs
(in_voltage0_raw) return scrambled data when the SPI offload engine is
configured?
When offload is configured, indio_dev->channels is assigned to the
offload channel just after this else block in ltc2378_probe():
indio_dev->channels = &st->info->offload_chan;
This channel specification is later passed to ltc2378_channel_single_read().
However, the single read function relies on the channel's endianness and
shift properties to decode the raw SPI data:
static int ltc2378_channel_single_read(const struct iio_chan_spec *chan, ...)
{
...
if (chan->scan_type.endianness == IIO_BE) {
if (chan->scan_type.realbits > 16)
sample = be32_to_cpu(st->scan.data.sample_buf32_be);
else
sample = be16_to_cpu(st->scan.data.sample_buf16_be);
} else {
if (chan->scan_type.realbits > 16)
sample = st->scan.data.sample_buf32;
else
sample = st->scan.data.sample_buf16;
}
sample >>= chan->scan_type.shift;
...
}
Because the offload_chan specification defines data as CPU-endian with
a 0 shift, the else branch is taken. This treats the natively big-endian,
MSB-aligned SPI data as unshifted CPU-endian data, scrambling the values on
little-endian architectures.
Could this be updated to properly handle manual sysfs reads when offload
is enabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782397418.git.marcelo.schmitt@analog.com?part=4
^ permalink raw reply
* Re: [PATCH v2 2/3] arm64: dts: qcom: sm8250: Add JPEG encoder node
From: Bryan O'Donoghue @ 2026-06-25 15:02 UTC (permalink / raw)
To: Atanas Filipov, linux-media
Cc: mchehab, robh, krzk+dt, conor+dt, andersson, konradybcio,
linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260625133828.3221781-3-atanas.filipov@oss.qualcomm.com>
On 25/06/2026 14:38, Atanas Filipov wrote:
> + qcom_jpeg_enc: jpeg-encoder@ac53000 {
> + compatible = "qcom,sm8250-jenc";
> +
> + reg = <0 0xac53000 0 0x1000>;
> +
> + interrupts = <GIC_SPI 474 IRQ_TYPE_EDGE_RISING>;
> + power-domains = <&camcc TITAN_TOP_GDSC>;
MXC is still missing from this list.
It cannot simultaneously be the case that jpeg-enc is a standalone block
but also relies on other things to scale MXC.
Take the example of MDSS and Iris here in sm8850.dtsi.
> +
> camss: camss@ac6a000 {
I'm sympathetic to using jpeg-enc without having to load/enable CAMSS
however, I keep coming back to
- CamNoc
jpeng-enc relies on a dedicted camera NoC exclusive to the rest of the
SoC
- jpeg-enc lives in the TITAN_TOP_GDSC power-island which wrappers
the whole camera block that jpeg lives in
To me those are just two examples of why things within that block should
be grouped together in DT and effectively look like a bus. There are
probably more examples. That is a function of the fact all of these
blocks live within the one power-island, have their own NoC with their
own input/output ports in that NoC.
They are designed to work together. Yes jpeg-enc _can_ be used outside
of Camera but, it is designed as part of that block and we will continue
to find dependencies as we go.
So again please structure this as a sub-node.
---
bod
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: qcs6490-rubikpi3: Enable cameras
From: Hongyang Zhao @ 2026-06-25 15:08 UTC (permalink / raw)
To: konrad.dybcio
Cc: andersson, conor+dt, devicetree, hongyang.zhao, konradybcio,
krzk+dt, linux-arm-msm, linux-kernel, robh, rosh
In-Reply-To: <a4073f5a-1965-4a6c-ba39-c236429e2db7@oss.qualcomm.com>
Hi Konrad,
Thanks for the review.
On 6/16/26 9:29 AM, Hongyang Zhao wrote:
> > Enable the two Raspberry Pi camera connectors on RubikPi 3 with
> > IMX219 sensors. CAM1 is connected to CCI1 and CSIPHY1, while CAM2
> > is connected to CCI0 and CSIPHY4.
> >
> > Add the shared 24 MHz camera oscillator, camera power enable
> > regulators, CAMSS endpoints, CCI sensor nodes and privacy LED GPIOs.
> >
> > Signed-off-by: Hongyang Zhao <hongyang.zhao@thundersoft.com>
> > ---
>
> [...]
>
> > + vreg_cam1_pwr: regulator-camera1-pwr {
> > + compatible = "regulator-fixed";
> > +
> > + regulator-name = "vreg_camera1_pwr";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > +
> > + gpio = <&tlmm 57 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > +
> > + pinctrl-0 = <&cam1_pwr_en>;
> > + pinctrl-names = "default";
> > +
> > + vin-supply = <&vreg_vcc3v3_output>;
> > + };
> > +
> > + vreg_cam2_pwr: regulator-camera2-pwr {
> > + compatible = "regulator-fixed";
> > +
> > + regulator-name = "vreg_camera2_pwr";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > +
> > + gpio = <&tlmm 58 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > +
> > + pinctrl-0 = <&cam2_pwr_en>;
> > + pinctrl-names = "default";
> > +
> > + vin-supply = <&vreg_vcc3v3_output>;
> > + };
> Are these supplies part of the baseboard?
Yes. These are board-level camera power switches on RUBIK Pi 3.
The public schematic shows this on page 13, in the Camera1&2 sheet:
https://thundercomm.s3-accelerate.amazonaws.com/uploads/web/rubik-pi-3/RUBIKPI3-IOB-V02-RELEASE.pdf
CAMERA1_PWR_EN and CAMERA2_PWR_EN are driven from the SOM side and gate
the camera connector power rails derived from VCC3V3_OUTPUT.
> [...]
>
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + /* CAM1 is routed to CSI1 / CSIPHY1. */
> I believe these comments are self-explanatory given the names of the labels
Ack, I will drop these comments in v2.
> [...]
>
> > + cam1_privacy_led_state: cam1-privacy-led-state {
> > + pins = "gpio18";
> > + function = "gpio";
> > + drive-strength = <8>;
> > + output-low;
> Please drop the output-foo properties from TLMM pins, the GPIO
> state will be controlled by the driver
Ack, I will drop the output-low/output-high properties from the TLMM
pinctrl states and let the GPIO consumers drive the state.
Thanks,
Hongyang
^ permalink raw reply
* RE: [PATCH net v5 1/4] net: ethernet: oa_tc6: Interrupt is active low, level triggered.
From: Selvamani Rajagopal @ 2026-06-25 15:21 UTC (permalink / raw)
To: Parthiban.Veerasooran@microchip.com, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, Piergiorgio Beruto
Cc: andrew@lunn.ch, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Conor.Dooley@microchip.com,
devicetree@vger.kernel.org
In-Reply-To: <f127837f-e08f-48e0-a3a9-906e1d61d6bb@microchip.com>
> -----Original Message-----
> From: Parthiban.Veerasooran@microchip.com <Parthiban.Veerasooran@microchip.com>
> Subject: Re: [PATCH net v5 1/4] net: ethernet: oa_tc6: Interrupt is active low, level triggered.
>
>
> With your above patches, I did a quick test (Test case 2) with two
> Microchip MAC-PHYs and faced a similar issue reported before. Sharing
> the dmesg crash log for your reference.
Root cause seems to be same. When oa_tc6_update_rx_skb function is called, tc6->rx_skb
seems to be NULL, which may mean, controller seems to be not getting start
I have a theory. Look at line #933. We have the following comment. I am sure this could be true
for the call to oa_tc6_prcs_rx_frame_end at line #926 or oa_tc6_prcs_ongoing_rx_frame at line #950.
/* After rx buffer overflow error received, there might be a
* possibility of getting an end valid of a previously
* incomplete rx frame along with the new rx frame start valid.
*/
Either we change the following line in the function oa_tc6_update_rx_skb
if ((tc6->rx_skb->tail + length) > tc6->rx_skb->end) {
to
if (tc6->rx_skb == NULL || (tc6->rx_skb->tail + length) > tc6->rx_skb->end) {
Or add a check
If (tc6->rx_skb) before calling above mentioned two functions from the callee function.
I could do. But I have no way of verifying this. I am sure it will fix the crash. I would like to confirm
whether traffic recovers.
>
> [ 2863.182105] eth1: Receive buffer overflow error
> [ 2863.199905] eth1: Receive buffer overflow error
> [ 2867.669312] Unable to handle kernel NULL pointer dereference at
> virtual address 00000000000000b8
^ permalink raw reply
* [PATCH v2] ASoC: dt-bindings: sound: nvidia,tegra30-ahub: Convert to DT schema
From: Charan Pedumuru @ 2026-06-25 15:28 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thierry Reding, Jonathan Hunter
Cc: linux-sound, devicetree, linux-tegra, linux-kernel,
Charan Pedumuru
Convert NVIDIA Tegra Audio Hub (AHUB) binding to DT schema.
Per-SoC differences in reg, reset-names, and dma-names are enforced
via allOf conditionals.
Add patternProperties for i2s child nodes referencing
nvidia,tegra30-i2s.yaml, reflecting actual DTSI usage where i2s
controllers are placed directly under ahub. nvidia,ahub-cif-ids is
not redeclared here as it is defined in the child i2s schema.
Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
---
Changes in v2:
- Modify the subject line to match with upstreaming standards and change
commit message to exaplin the changes.
- compatible: collapsed three standalone const entries into an enum,
sorted alphanumerically (tegra114 < tegra124 < tegra30)
- reset-names: moved per-SoC ordered items lists from a top-level oneOf
into the allOf if/then blocks; top-level now has only minItems/maxItems
- dma-names: dropped oneOf, kept single 20-item items list with minItems: 8;
per-SoC min/maxItems constraints moved into allOf if/then blocks
- "#address-cells": replaced $ref: types with enum: [1, 2] at top level;
added const: 1 in tegra30 and tegra114 if/then blocks, const: 2 in
tegra124 if/then block to match actual dtsi values
- "#size-cells": same treatment as #address-cells
- patternProperties i2s child: replaced allOf + $ref wrapper with direct
$ref; removed properties/required block (nvidia,ahub-cif-ids belongs in
the i2s schema).
- Link to v1: https://patch.msgid.link/20260613-nvidia-ahub-v1-1-5b7e85301736@gmail.com
---
.../bindings/sound/nvidia,tegra30-ahub.txt | 88 ------
.../bindings/sound/nvidia,tegra30-ahub.yaml | 297 +++++++++++++++++++++
2 files changed, 297 insertions(+), 88 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra30-ahub.txt b/Documentation/devicetree/bindings/sound/nvidia,tegra30-ahub.txt
deleted file mode 100644
index 0e9a1895d7fb..000000000000
--- a/Documentation/devicetree/bindings/sound/nvidia,tegra30-ahub.txt
+++ /dev/null
@@ -1,88 +0,0 @@
-NVIDIA Tegra30 AHUB (Audio Hub)
-
-Required properties:
-- compatible : For Tegra30, must contain "nvidia,tegra30-ahub". For Tegra114,
- must contain "nvidia,tegra114-ahub". For Tegra124, must contain
- "nvidia,tegra124-ahub". Otherwise, must contain "nvidia,<chip>-ahub",
- plus at least one of the above, where <chip> is tegra132.
-- reg : Should contain the register physical address and length for each of
- the AHUB's register blocks.
- - Tegra30 requires 2 entries, for the APBIF and AHUB/AUDIO register blocks.
- - Tegra114 requires an additional entry, for the APBIF2 register block.
-- interrupts : Should contain AHUB interrupt
-- clocks : Must contain an entry for each entry in clock-names.
- See ../clocks/clock-bindings.txt for details.
-- clock-names : Must include the following entries:
- - d_audio
- - apbif
-- resets : Must contain an entry for each entry in reset-names.
- See ../reset/reset.txt for details.
-- reset-names : Must include the following entries:
- Tegra30 and later:
- - d_audio
- - apbif
- - i2s0
- - i2s1
- - i2s2
- - i2s3
- - i2s4
- - dam0
- - dam1
- - dam2
- - spdif
- Tegra114 and later additionally require:
- - amx
- - adx
- Tegra124 and later additionally require:
- - amx1
- - adx1
- - afc0
- - afc1
- - afc2
- - afc3
- - afc4
- - afc5
-- ranges : The bus address mapping for the configlink register bus.
- Can be empty since the mapping is 1:1.
-- dmas : Must contain an entry for each entry in clock-names.
- See ../dma/dma.txt for details.
-- dma-names : Must include the following entries:
- - rx0 .. rx<n>
- - tx0 .. tx<n>
- ... where n is:
- Tegra30: 3
- Tegra114, Tegra124: 9
-- #address-cells : For the configlink bus. Should be <1>;
-- #size-cells : For the configlink bus. Should be <1>.
-
-AHUB client modules need to specify the IDs of their CIFs (Client InterFaces).
-For RX CIFs, the numbers indicate the register number within AHUB routing
-register space (APBIF 0..3 RX, I2S 0..5 RX, DAM 0..2 RX 0..1, SPDIF RX 0..1).
-For TX CIFs, the numbers indicate the bit position within the AHUB routing
-registers (APBIF 0..3 TX, I2S 0..5 TX, DAM 0..2 TX, SPDIF TX 0..1).
-
-Example:
-
-ahub@70080000 {
- compatible = "nvidia,tegra30-ahub";
- reg = <0x70080000 0x200 0x70080200 0x100>;
- interrupts = < 0 103 0x04 >;
- nvidia,dma-request-selector = <&apbdma 1>;
- clocks = <&tegra_car 106>, <&tegra_car 107>;
- clock-names = "d_audio", "apbif";
- resets = <&tegra_car 106>, <&tegra_car 107>, <&tegra_car 30>,
- <&tegra_car 11>, <&tegra_car 18>, <&tegra_car 101>,
- <&tegra_car 102>, <&tegra_car 108>, <&tegra_car 109>,
- <&tegra_car 110>, <&tegra_car 10>;
- reset-names = "d_audio", "apbif", "i2s0", "i2s1", "i2s2",
- "i2s3", "i2s4", "dam0", "dam1", "dam2",
- "spdif";
- dmas = <&apbdma 1>, <&apbdma 1>;
- <&apbdma 2>, <&apbdma 2>;
- <&apbdma 3>, <&apbdma 3>;
- <&apbdma 4>, <&apbdma 4>;
- dma-names = "rx0", "tx0", "rx1", "tx1", "rx2", "tx2", "rx3", "tx3";
- ranges;
- #address-cells = <1>;
- #size-cells = <1>;
-};
diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra30-ahub.yaml b/Documentation/devicetree/bindings/sound/nvidia,tegra30-ahub.yaml
new file mode 100644
index 000000000000..1c3fc7a97465
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/nvidia,tegra30-ahub.yaml
@@ -0,0 +1,297 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/nvidia,tegra30-ahub.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra Audio Hub (AHUB)
+
+maintainers:
+ - Jonathan Hunter <jonathanh@nvidia.com>
+
+description:
+ The NVIDIA Tegra AHUB (Audio Hub) is an audio interconnect block used to
+ route data between various audio clients such as I2S, DAM, SPDIF, and
+ APBIF. It exposes multiple register regions and supports different
+ configurations depending on the Tegra SoC generation. The AHUB also
+ provides a configlink bus for child audio components, which use CIF
+ (Client Interface) IDs to identify their data paths. The number of DMA
+ channels, reset lines, and additional modules varies across Tegra30,
+ Tegra114, and Tegra124 platforms.
+
+properties:
+ compatible:
+ oneOf:
+ - enum:
+ - nvidia,tegra114-ahub
+ - nvidia,tegra124-ahub
+ - nvidia,tegra30-ahub
+ - items:
+ - const: nvidia,tegra132-ahub
+ - const: nvidia,tegra124-ahub
+
+ reg:
+ minItems: 2
+ maxItems: 3
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Clock for the D_AUDIO domain
+ - description: Clock for the APBIF interface
+
+ clock-names:
+ items:
+ - const: d_audio
+ - const: apbif
+
+ resets:
+ minItems: 11
+ maxItems: 21
+
+ reset-names:
+ minItems: 11
+ maxItems: 21
+
+ dmas:
+ minItems: 8
+ maxItems: 20
+
+ dma-names:
+ minItems: 8
+ items:
+ - const: rx0
+ - const: tx0
+ - const: rx1
+ - const: tx1
+ - const: rx2
+ - const: tx2
+ - const: rx3
+ - const: tx3
+ - const: rx4
+ - const: tx4
+ - const: rx5
+ - const: tx5
+ - const: rx6
+ - const: tx6
+ - const: rx7
+ - const: tx7
+ - const: rx8
+ - const: tx8
+ - const: rx9
+ - const: tx9
+
+ "#address-cells":
+ enum: [1, 2]
+
+ "#size-cells":
+ enum: [1, 2]
+
+ ranges: true
+
+patternProperties:
+ "^i2s@[0-9a-f]+$":
+ $ref: /schemas/sound/nvidia,tegra30-i2s.yaml#
+ unevaluatedProperties: false
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: nvidia,tegra30-ahub
+ then:
+ properties:
+ reg:
+ minItems: 2
+ maxItems: 2
+
+ resets:
+ minItems: 11
+ maxItems: 11
+
+ reset-names:
+ items:
+ - const: d_audio
+ - const: apbif
+ - const: i2s0
+ - const: i2s1
+ - const: i2s2
+ - const: i2s3
+ - const: i2s4
+ - const: dam0
+ - const: dam1
+ - const: dam2
+ - const: spdif
+
+ dmas:
+ minItems: 8
+ maxItems: 8
+
+ dma-names:
+ minItems: 8
+ maxItems: 8
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 1
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: nvidia,tegra114-ahub
+ then:
+ properties:
+ reg:
+ minItems: 3
+ maxItems: 3
+
+ resets:
+ minItems: 13
+ maxItems: 13
+
+ reset-names:
+ items:
+ - const: d_audio
+ - const: apbif
+ - const: i2s0
+ - const: i2s1
+ - const: i2s2
+ - const: i2s3
+ - const: i2s4
+ - const: dam0
+ - const: dam1
+ - const: dam2
+ - const: spdif
+ - const: amx
+ - const: adx
+
+ dmas:
+ minItems: 20
+ maxItems: 20
+
+ dma-names:
+ minItems: 20
+ maxItems: 20
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 1
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: nvidia,tegra124-ahub
+ then:
+ properties:
+ reg:
+ minItems: 3
+ maxItems: 3
+
+ resets:
+ minItems: 21
+ maxItems: 21
+
+ reset-names:
+ items:
+ - const: d_audio
+ - const: apbif
+ - const: i2s0
+ - const: i2s1
+ - const: i2s2
+ - const: i2s3
+ - const: i2s4
+ - const: dam0
+ - const: dam1
+ - const: dam2
+ - const: spdif
+ - const: amx
+ - const: amx1
+ - const: adx
+ - const: adx1
+ - const: afc0
+ - const: afc1
+ - const: afc2
+ - const: afc3
+ - const: afc4
+ - const: afc5
+
+ dmas:
+ minItems: 20
+ maxItems: 20
+
+ dma-names:
+ minItems: 20
+ maxItems: 20
+
+ "#address-cells":
+ const: 2
+
+ "#size-cells":
+ const: 2
+
+required:
+ - reg
+ - compatible
+ - interrupts
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - dmas
+ - dma-names
+ - "#address-cells"
+ - "#size-cells"
+ - ranges
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/tegra30-car.h>
+ ahub@70080000 {
+ compatible = "nvidia,tegra30-ahub";
+ reg = <0x70080000 0x200>, <0x70080200 0x100>;
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&tegra_car TEGRA30_CLK_D_AUDIO>,
+ <&tegra_car TEGRA30_CLK_APBIF>;
+ clock-names = "d_audio", "apbif";
+ resets = <&tegra_car 106>, <&tegra_car 107>,
+ <&tegra_car 30>, <&tegra_car 11>,
+ <&tegra_car 18>, <&tegra_car 101>,
+ <&tegra_car 102>, <&tegra_car 108>,
+ <&tegra_car 109>, <&tegra_car 110>,
+ <&tegra_car 10>;
+ reset-names = "d_audio", "apbif",
+ "i2s0", "i2s1", "i2s2", "i2s3", "i2s4",
+ "dam0", "dam1", "dam2",
+ "spdif";
+ dmas = <&apbdma 1>, <&apbdma 1>,
+ <&apbdma 2>, <&apbdma 2>,
+ <&apbdma 3>, <&apbdma 3>,
+ <&apbdma 4>, <&apbdma 4>;
+ dma-names = "rx0", "tx0", "rx1", "tx1",
+ "rx2", "tx2", "rx3", "tx3";
+ ranges;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ i2s@70080300 {
+ compatible = "nvidia,tegra30-i2s";
+ reg = <0x70080300 0x100>;
+ nvidia,ahub-cif-ids = <4 4>;
+ clocks = <&tegra_car TEGRA30_CLK_I2S0>;
+ resets = <&tegra_car 30>;
+ reset-names = "i2s";
+ };
+ };
+...
---
base-commit: db7efce4ae23ad5e42f5f55428f529ff62b86fab
change-id: 20260422-nvidia-ahub-d2ceb0372744
Best regards,
--
Charan Pedumuru <charan.pedumuru@gmail.com>
^ permalink raw reply related
* RE: [PATCH net v5 1/4] net: ethernet: oa_tc6: Interrupt is active low, level triggered.
From: Selvamani Rajagopal @ 2026-06-25 15:31 UTC (permalink / raw)
To: Parthiban.Veerasooran@microchip.com, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, Piergiorgio Beruto
Cc: andrew@lunn.ch, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Conor.Dooley@microchip.com,
devicetree@vger.kernel.org
In-Reply-To: <CYYPR02MB982836BC273D09FD3BDE623583EC2@CYYPR02MB9828.namprd02.prod.outlook.com>
Parthiban,
Let me know if you prefer updating the patchset. I certainly prefer adding a NULL check
In oa_tc6_update_rx_skb function.
>
> Root cause seems to be same. When oa_tc6_update_rx_skb function is called, tc6-
> >rx_skb
> seems to be NULL, which may mean, controller seems to be not getting start
>
> I have a theory. Look at line #933. We have the following comment. I am sure this could
> be true
> for the call to oa_tc6_prcs_rx_frame_end at line #926 or oa_tc6_prcs_ongoing_rx_frame
> at line #950.
> /* After rx buffer overflow error received, there might be a
> * possibility of getting an end valid of a previously
> * incomplete rx frame along with the new rx frame start valid.
> */
>
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: qcom: Add Shikra CQM SoM platform
From: Kamal Wadhwa @ 2026-06-25 15:41 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rakesh Kota, linux-arm-msm, sashiko-reviews, Komal Bajaj, robh,
linux-phy, neil.armstrong, vkoul, olteanv, krzk+dt, conor+dt,
devicetree
In-Reply-To: <CAO9ioeVMieo8HtpRw+n4ZU9uV-_RQM061q5XfAA1Dtk2fnp4+g@mail.gmail.com>
On Wed, Jun 17, 2026 at 03:48:14PM +0300, Dmitry Baryshkov wrote:
> On Mon, 18 May 2026 at 14:49, Kamal Wadhwa
> <kamal.wadhwa@oss.qualcomm.com> wrote:
> >
> > On Sun, May 17, 2026 at 08:18:15PM +0300, Dmitry Baryshkov wrote:
> > > On Thu, May 14, 2026 at 04:09:18PM +0530, Kamal Wadhwa wrote:
> > > > On Wed, May 13, 2026 at 06:14:20PM +0300, Dmitry Baryshkov wrote:
> > > > > On 13/05/2026 17:29, Rakesh Kota wrote:
> > > > > > On Wed, May 13, 2026 at 03:01:47PM +0300, Dmitry Baryshkov wrote:
> > > > > > > On Wed, May 13, 2026 at 04:28:35AM +0000, sashiko-bot@kernel.org wrote:
> > > > > > > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > > > > > > > - [High] The PMIC regulator definitions omit their required input supply dependencies (e.g., `vdd_s2-supply`, `vdd_l3-supply`), breaking the power hierarchy.
> > > > > > > > - [Medium] The device tree inaccurately hardcodes the `compatible` string to a different PMIC model (`qcom,rpm-pm2250-regulators`) instead of explicitly identifying the actual hardware (PM4125).
> > > > > > > > --
> > > > > > > >
> > > > > > > > commit 457abf6e7ac410430c866842e08e602ec3daaa51
> > > > > > > > Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
> > > > > > > >
> > > > > > > > arm64: dts: qcom: Add Shikra CQM SoM platform
> > > > > > > >
> > > > > > > > Add device tree include for the CQM variant of the Shikra System-on-Module.
> > > > > > > > This variant integrates the Shikra SoC and PM4125 PMIC. The DTSI includes
> > > > > > > > the common shikra.dtsi and adds PM4125 PMIC regulator definitions.
> > > > > > > >
> > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > > > > > > > > new file mode 100644
> > > > > > > > > index 0000000000000..401e71720519d
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > > > > > > > [ ... ]
> > > > > > > > > +&rpm_requests {
> > > > > > > > > + regulators {
> > > > > > > > > + compatible = "qcom,rpm-pm2250-regulators";
> > > > > > > >
> > > > > > > > Since the commit message indicates this is a PM4125 PMIC, should the
> > > > > > > > compatible string include a specific identifier for the actual hardware?
> > > > > > > >
> > > > > > > > Using a specific identifier like "qcom,rpm-pm4125-regulators" alongside
> > > > > > > > the fallback allows the kernel to apply specific quirks if PM4125 errata
> > > > > > > > are discovered later.
> > > > > > >
> > > > > > > This can be ignored. The compat is a leftover from the historically
> > > > > > > incorrect naming.
> > > > > > >
> > > > > > > >
> > > > > > > > > +
> > > > > > > > > + pm4125_s2: s2 {
> > > > > > > > > + regulator-min-microvolt = <1000000>;
> > > > > > > > > + regulator-max-microvolt = <1200000>;
> > > > > > > > > + };
> > > > > > > >
> > > > > > > > Do these regulators need to explicitly define their input supply dependencies
> > > > > > > > such as vdd_s2-supply?
> > > > > > > >
> > > > > > > > Without these properties, the regulator framework might be unaware that the
> > > > > > > > PMIC regulators draw power from upstream supplies.
> > > > > > > >
> > > > > > > > If the kernel dynamically manages the upstream supply and its reference count
> > > > > > > > drops to zero, could it be disabled, causing an unexpected power loss for
> > > > > > > > downstream components?
> > > > > > >
> > > > > > > And this is a correct comment. Please provide missing supplies.
> > > > > > >
> > > > > > As per the Qualcomm system design, the parent-child supply relationship
> > > > > > is managed by the RPM firmware, not the Linux regulator framework. The
> > > > > > RPM ensures the parent supply is never disabled until all subsystem
> > > > > > votes are cleared.
> > > > >
> > > > > How is this different from other, previous platforms?
> > > >
> > > > This is not different. In the previous platforms too this is taken care from the
> > > > RPM/RPMH firmware side, the only case where we may need explicit vote to parent
> > > > is for non-rpmh/rpm regulator rails (like i2c based regulator pm8008), which
> > > > may have a RPM/RPMH regulator as a parent.
> > > >
> > > > Even on those previous targets the parent rail of all RPM/RPMH regulators are
> > > > internally voted by RPM/RPMH FW at proper voltage with required headroom
> > > > calculated based on the active child rails. This was done for all the
> > > > subsystems (including APPS) regulators.
> > > >
> > > > So no explicit handling from the APPS is required for parent supply.
> > >
> > > You are explaining the driver behaviour. But the question is about the
> > > hardware description. If there is no difference, please add necessary
> > > supplies back.
> >
> > I understand your concern about descibing the parent-child relation in the
> > devicetree, and given that we have been almost always followed this for all
> > the previous targets, it will expected of us to add them.
>
> Yes.
>
> >
> > However, we want to avoid the unnecessary access to the parent from APPS.
>
> Why? What is the reason? Do we want to do the same for all the
> platforms? Only for Shikra? Something else?
>
> > At the moment, I do not see a way to avoid that, if we add the parent
> > regulators.
>
> That depend on the answer to the previous question. In the end, we can
> make the driver ignore the parents by removing them from the regulator
> desc.
Ok, this seems like a good suggestion, so you mean its ok if we define the
regulator desc's supply column with NULL? And only keep that in the DT?
you mean like this?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/regulator/qcom-rpmh-regulator.c?h=v7.1#n1453
(please let me know if i got that right. thanks)
>
> >
> > @Bjorn, @Konrad - can you please also share your suggestion, how we can add
> > parent-child desciption, but avoid accessing parent supply from APPS, as its
> > Qualcomm's system design to handle this on RPM/RPMH firmware side (you may
> > recall we had a verbal/offline discussion about same concern in context of
> > RPMH regulators earlier).
>
> That's why offline discussions are bad - you can't include other
> participants in them.
Yes, i see your point. I will take care of that in future.
>
> --
> With best wishes
> Dmitry
^ permalink raw reply
* [PATCH v14 0/5] Enable Remote GPIO over RPMSG on i.MX Platform
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn
From: Shenwei Wang <shenwei.wang@nxp.com>
Support the remote devices on the remote processor via the RPMSG bus on
i.MX platform.
Changes in v14:
- Update gpio-rpmsg.rst per Mathieu’s feedback.
- Align the rpmsg-gpio driver with the revised gpio-rpmsg.rst.
- Modify rpmsg-core to enable prefix-based matching of RPMSG device IDs.
Changes in v13:
- drop the support for legacy NXP firmware.
- remove the fixed_up hooks from the rpmsg gpio driver.
- code cleanup.
Changes in v12:
- Fixed the "underline" warning reported by Randy.
Changes in v11:
- Expand RPMSG for the first time per Shuah's review comment.
Changes in v10:
- Update gpio-rpmsg.rst according to Daniel Baluta's review comments.
- Add a kernel CONFIG for fixed up handlers and only enable it on
i.MX products.
- Fixed bugs reported by kernel test robot.
Changes in v9:
- Reuse the gpio-virtio design for command and IRQ type definitions.
- Remove msg_id, version, and vendor fields from the generic protocol.
- Add fixed-up handlers to support legacy firmware.
Changes in v8:
- Add "depends on REMOTEPROC" in Kconfig to fix the build error reported
by the kernel test robot.
- Move the .rst patch before the .yaml patch.
- Handle the "ngpios" DT property based on Andrew's feedback.
Changes in v7:
- Reworked the driver to use the rpmsg_driver framework instead of
platform_driver, based on feedback from Bjorn and Arnaud.
- Updated gpio-rpmsg.yaml and imx_rproc.yaml according to comments from
Rob and Arnaud.
- Further refinements to gpio-rpmsg.yaml per Arnaud's feedback.
Changes in v6:
- make the driver more generic with the actions below:
rename the driver file to gpio-rpmsg.c
remove the imx related info in the function and variable names
rename the imx_rpmsg.h to rpdev_info.h
create a gpio-rpmsg.yaml and refer it in imx_rproc.yaml
- update the gpio-rpmsg.rst according to the feedback from Andrew and
move the source file to driver-api/gpio
- fix the bug reported by Zhongqiu Han
- remove the I2C related info
Changes in v5:
- move the gpio-rpmsg.rst from admin-guide to staging directory after
discussion with Randy Dunlap.
- add include files with some code improvements per Bartosz's comments.
Changes in v4:
- add a documentation to describe the transport protocol per Andrew's
comments.
- add a new handler to get the gpio direction.
Changes in v3:
- fix various format issue and return value check per Peng 's review
comments.
- add the logic to also populate the subnodes which are not in the
device map per Arnaud's request. (in imx_rproc.c)
- update the yaml per Frank's review comments.
Changes in v2:
- re-implemented the gpio driver per Linus Walleij's feedback by using
GPIOLIB_IRQCHIP helper library.
- fix various format issue per Mathieu/Peng 's review comments.
- update the yaml doc per Rob's feedback
Shenwei Wang (5):
docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
rpmsg: core: match rpmsg device IDs by prefix
gpio: rpmsg: add generic rpmsg GPIO driver
arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
.../devicetree/bindings/gpio/gpio-rpmsg.yaml | 55 ++
.../bindings/remoteproc/fsl,imx-rproc.yaml | 53 ++
Documentation/driver-api/gpio/gpio-rpmsg.rst | 271 +++++++++
Documentation/driver-api/gpio/index.rst | 1 +
arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 25 +
drivers/gpio/Kconfig | 17 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-rpmsg.c | 568 ++++++++++++++++++
drivers/rpmsg/rpmsg_core.c | 4 +-
9 files changed, 994 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
create mode 100644 Documentation/driver-api/gpio/gpio-rpmsg.rst
create mode 100644 drivers/gpio/gpio-rpmsg.c
--
2.43.0
^ permalink raw reply
* [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn
In-Reply-To: <20260625155432.815185-1-shenwei.wang@oss.nxp.com>
From: Shenwei Wang <shenwei.wang@nxp.com>
Describes the gpio rpmsg transport protocol over the rpmsg bus between
the remote system and Linux.
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
Documentation/driver-api/gpio/gpio-rpmsg.rst | 271 +++++++++++++++++++
Documentation/driver-api/gpio/index.rst | 1 +
2 files changed, 272 insertions(+)
create mode 100644 Documentation/driver-api/gpio/gpio-rpmsg.rst
diff --git a/Documentation/driver-api/gpio/gpio-rpmsg.rst b/Documentation/driver-api/gpio/gpio-rpmsg.rst
new file mode 100644
index 000000000000..7d351ff0adb0
--- /dev/null
+++ b/Documentation/driver-api/gpio/gpio-rpmsg.rst
@@ -0,0 +1,271 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+GPIO RPMSG (Remote Processor Messaging) Protocol
+================================================
+
+The GPIO RPMSG transport protocol is used for communication and interaction
+with GPIO controllers on remote processors via the RPMSG bus.
+
+Message Format
+--------------
+
+The RPMSG message consists of a 8-byte packet with the following layout:
+
+.. code-block:: none
+
+ +------+------+------+------+------+------+------+------+
+ | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+ | cmd | line | value |
+ +------+------+------+------+------+------+------+------+
+
+- **cmd**: Command code, used for GPIO_RPMSG_SEND messages.
+
+- **line**: The GPIO line (pin) index of the port.
+
+- **value**: See details in the command description below.
+
+
+GPIO Commands
+-------------
+
+Commands are specified in the **Cmd** field.
+
+The SEND message is always sent from Linux to the remote firmware. Each
+SEND corresponds to a single REPLY message. The GPIO driver should
+serialize messages and determine whether a REPLY message is required. If a
+REPLY message is expected but not received within the specified timeout
+period (currently 1 second in the Linux driver), the driver should return
+-ETIMEOUT.
+
+GET_DIRECTION (Cmd=2)
+~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+ +------+------+------+------+------+------+------+------+
+ | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+ | 2 | line | 0 |
+ +------+------+------+------+------+------+------+------+
+
+**Reply:**
+
+.. code-block:: none
+
+ +------+--------+--------+
+ | 0x00 | 0x01 | 0x02 |
+ | 1 | status | value |
+ +------+--------+--------+
+
+- **status**:
+
+ - 0: Ok
+ - 1: Error
+
+- **value**: Direction.
+
+ - 0: None
+ - 1: Output
+ - 2: Input
+
+
+SET_DIRECTION (Cmd=3)
+~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+ +------+------+------+------+------+------+------+------+
+ | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+ | 3 | line | value |
+ +------+------+------+------+------+------+------+------+
+
+- **value**: Direction.
+
+ - 0: None
+ - 1: Output
+ - 2: Input
+
+**Reply:**
+
+.. code-block:: none
+
+ +------+--------+--------+
+ | 0x00 | 0x01 | 0x02 |
+ | 1 | status | 0 |
+ +------+--------+--------+
+
+- **status**:
+
+ - 0: Ok
+ - 1: Error
+
+
+GET_VALUE (Cmd=4)
+~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+ +------+------+------+------+------+------+------+------+
+ | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+ | 4 | line | 0 |
+ +------+------+------+------+------+------+------+------+
+
+**Reply:**
+
+.. code-block:: none
+
+ +------+--------+--------+
+ | 0x00 | 0x01 | 0x02 |
+ | 1 | status | value |
+ +------+--------+--------+
+
+- **status**:
+
+ - 0: Ok
+ - 1: Error
+
+- **value**: Level.
+
+ - 0: Low
+ - 1: High
+
+
+SET_VALUE (Cmd=5)
+~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+ +------+------+------+------+------+------+------+------+
+ | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+ | 5 | line | value |
+ +------+------+------+------+------+------+------+------+
+
+- **value**: Output level.
+
+ - 0: Low
+ - 1: High
+
+**Reply:**
+
+.. code-block:: none
+
+ +------+--------+--------+
+ | 0x00 | 0x01 | 0x02 |
+ | 1 | status | 0 |
+ +------+--------+--------+
+
+- **status**:
+
+ - 0: Ok
+ - 1: Error
+
+
+SET_IRQ_TYPE (Cmd=6)
+~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+ +------+------+------+------+------+------+------+------+
+ | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+ | 6 | line | value |
+ +------+------+------+------+------+------+------+------+
+
+- **value**: IRQ types.
+
+ - 0: Interrupt disabled
+ - 1: Rising edge trigger
+ - 2: Falling edge trigger
+ - 3: Both edge trigger
+ - 4: High level trigger
+ - 8: Low level trigger
+
+**Reply:**
+
+.. code-block:: none
+
+ +------+--------+--------+
+ | 0x00 | 0x01 | 0x02 |
+ | 1 | status | 0 |
+ +------+--------+--------+
+
+- **status**:
+
+ - 0: Ok
+ - 1: Error
+
+SET_WAKEUP (Cmd=16)
+~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+ +------+------+------+------+------+------+------+------+
+ | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+ | 1 | line | value |
+ +------+------+------+------+------+------+------+------+
+
+- **value**: Wakeup enable.
+
+ The remote system should always aim to stay in a power-efficient state by
+ shutting down or clock-gating the GPIO blocks that aren't in use. Since
+ the remoteproc driver is responsible for managing the power states of the
+ remote firmware, the GPIO driver does not require to know the firmware's
+ running states.
+
+ When the wakeup bit is set, the remote firmware should configure the line
+ as a wakeup source. The firmware should send the notification message to
+ Linux after it is woken from the GPIO line.
+
+ - 0: Disable wakeup from GPIO
+ - 1: Enable wakeup from GPIO
+
+**Reply:**
+
+.. code-block:: none
+
+ +------+--------+--------+
+ | 0x00 | 0x01 | 0x02 |
+ | 1 | status | 0 |
+ +------+--------+--------+
+
+- **status**:
+
+ - 0: Ok
+ - 1: Error
+
+Notification Message
+--------------------
+
+Notifications are sent by the remote core and they have
+**Type=2 (GPIO_RPMSG_NOTIFY)**:
+
+When a GPIO line asserts an interrupt on the remote processor, the firmware
+should immediately mask the corresponding interrupt source and send a
+notification message to the Linux. Upon completion of the interrupt
+handling on the Linux side, the driver should issue a
+command **SET_IRQ_TYPE** to the firmware to unmask the interrupt.
+
+A Notification message can arrive between a SEND and its REPLY message,
+and the driver is expected to handle this scenario.
+
+.. code-block:: none
+
+ +------+------+--------+
+ | 0x00 | 0x01 | 0x02 |
+ | 2 | line | trigger|
+ +------+------+--------+
+
+- **line**: The GPIO line (pin) index of the port.
+
+- **trigger**: Optional parameter to indicate the trigger event type.
+
diff --git a/Documentation/driver-api/gpio/index.rst b/Documentation/driver-api/gpio/index.rst
index bee58f709b9a..e5eb1f82f01f 100644
--- a/Documentation/driver-api/gpio/index.rst
+++ b/Documentation/driver-api/gpio/index.rst
@@ -16,6 +16,7 @@ Contents:
drivers-on-gpio
bt8xxgpio
pca953x
+ gpio-rpmsg
Core
====
--
2.43.0
^ permalink raw reply related
* [PATCH v14 2/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn
In-Reply-To: <20260625155432.815185-1-shenwei.wang@oss.nxp.com>
From: Shenwei Wang <shenwei.wang@nxp.com>
Remote processors may announce multiple GPIO controllers over an RPMSG
channel. These GPIO controllers may require corresponding device tree
nodes, especially when acting as providers, to supply phandles for their
consumers.
Define an RPMSG node to work as a container for a group of RPMSG channels
under the imx_rproc node. Each subnode within "rpmsg" represents an
individual RPMSG channel. The name of each subnode corresponds to the
channel name as defined by the remote processor.
All remote devices associated with a given channel are defined as child
nodes under the corresponding channel node.
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
.../devicetree/bindings/gpio/gpio-rpmsg.yaml | 55 +++++++++++++++++++
.../bindings/remoteproc/fsl,imx-rproc.yaml | 53 ++++++++++++++++++
2 files changed, 108 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
new file mode 100644
index 000000000000..6c78b6850321
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-rpmsg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic RPMSG GPIO Controller
+
+maintainers:
+ - Shenwei Wang <shenwei.wang@nxp.com>
+
+description:
+ On an AMP platform, some GPIO controllers are exposed by the remote processor
+ through the RPMSG bus. The RPMSG GPIO transport protocol defines the packet
+ structure and communication flow between Linux and the remote firmware. Those
+ controllers are managed via this transport protocol. For more details of the
+ protocol, check the document below.
+ Documentation/driver-api/gpio/gpio-rpmsg.rst
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - fsl,rpmsg-gpio
+ - const: rpmsg-gpio
+ - const: rpmsg-gpio
+
+ reg:
+ description:
+ The reg property represents the index of the GPIO controllers. Since
+ the driver manages controllers on a remote system, this index tells
+ the remote system which controller to operate.
+ maxItems: 1
+
+ "#gpio-cells":
+ const: 2
+
+ gpio-controller: true
+
+ interrupt-controller: true
+
+ "#interrupt-cells":
+ const: 2
+
+required:
+ - compatible
+ - reg
+ - "#gpio-cells"
+ - "#interrupt-cells"
+
+allOf:
+ - $ref: /schemas/gpio/gpio.yaml#
+
+unevaluatedProperties: false
diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
index ce8ec0119469..aea33205a881 100644
--- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
@@ -85,6 +85,34 @@ properties:
This property is to specify the resource id of the remote processor in SoC
which supports SCFW
+ rpmsg:
+ type: object
+ additionalProperties: false
+ description:
+ Represents the RPMSG bus between Linux and the remote system. Contains
+ a group of RPMSG channel devices running on the bus.
+
+ properties:
+ rpmsg-io:
+ type: object
+ additionalProperties: false
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ patternProperties:
+ "gpio@[0-9a-f]+$":
+ type: object
+ $ref: /schemas/gpio/gpio-rpmsg.yaml#
+ unevaluatedProperties: false
+
+ required:
+ - '#address-cells'
+ - '#size-cells'
+
required:
- compatible
@@ -147,5 +175,30 @@ examples:
&mu 3 1>;
memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
syscon = <&src>;
+
+ rpmsg {
+ rpmsg-io {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ gpio@0 {
+ compatible = "rpmsg-gpio";
+ reg = <0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ };
+
+ gpio@1 {
+ compatible = "rpmsg-gpio";
+ reg = <1>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ };
+ };
+ };
};
...
--
2.43.0
^ permalink raw reply related
* [PATCH v14 3/5] rpmsg: core: match rpmsg device IDs by prefix
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn
In-Reply-To: <20260625155432.815185-1-shenwei.wang@oss.nxp.com>
From: Shenwei Wang <shenwei.wang@nxp.com>
The current rpmsg_id_match() implementation requires an exact
string match between the driver id_table entry and the rpmsg
device name using strncmp() with RPMSG_NAME_SIZE.
This makes it impossible for a driver to match a group of
rpmsg devices sharing a common prefix (e.g. dynamically
suffixed channel names).
Update the matching logic to compare only the length of the
id->name string, allowing id_table entries to act as prefixes.
This enables drivers to bind to devices whose names start with
the specified id->name.
The implementation is copied from a reply by Mathieu.
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
drivers/rpmsg/rpmsg_core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index e7f7831d37f8..f95bfc9965d4 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -414,7 +414,9 @@ ATTRIBUTE_GROUPS(rpmsg_dev);
static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
const struct rpmsg_device_id *id)
{
- return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
+ size_t len = strnlen(id->name, RPMSG_NAME_SIZE);
+
+ return strncmp(id->name, rpdev->id.name, len) == 0;
}
/* match rpmsg channel and rpmsg driver */
--
2.43.0
^ permalink raw reply related
* [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn, Bartosz Golaszewski
In-Reply-To: <20260625155432.815185-1-shenwei.wang@oss.nxp.com>
From: Shenwei Wang <shenwei.wang@nxp.com>
On an AMP platform, the system may include multiple processors:
- MCUs running an RTOS
- An MPU running Linux
These processors communicate via the RPMSG protocol.
The driver implements the standard GPIO interface, allowing
the Linux side to control GPIO controllers which reside in
the remote processor via RPMSG protocol.
Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
drivers/gpio/Kconfig | 17 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-rpmsg.c | 568 ++++++++++++++++++++++++++++++++++++++
3 files changed, 586 insertions(+)
create mode 100644 drivers/gpio/gpio-rpmsg.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 020e51e30317..4ad299fe3c6f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1917,6 +1917,23 @@ config GPIO_SODAVILLE
endmenu
+menu "RPMSG GPIO drivers"
+ depends on RPMSG
+
+config GPIO_RPMSG
+ tristate "Generic RPMSG GPIO support"
+ depends on OF && REMOTEPROC
+ select GPIOLIB_IRQCHIP
+ default REMOTEPROC
+ help
+ Say yes here to support the generic GPIO functions over the RPMSG
+ bus. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x, and
+ i.MX9x.
+
+ If unsure, say N.
+
+endmenu
+
menu "SPI GPIO expanders"
depends on SPI_MASTER
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index b267598b517d..ee75c0e65b8b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -157,6 +157,7 @@ obj-$(CONFIG_GPIO_RDC321X) += gpio-rdc321x.o
obj-$(CONFIG_GPIO_REALTEK_OTTO) += gpio-realtek-otto.o
obj-$(CONFIG_GPIO_REG) += gpio-reg.o
obj-$(CONFIG_GPIO_ROCKCHIP) += gpio-rockchip.o
+obj-$(CONFIG_GPIO_RPMSG) += gpio-rpmsg.o
obj-$(CONFIG_GPIO_RTD) += gpio-rtd.o
obj-$(CONFIG_ARCH_SA1100) += gpio-sa1100.o
obj-$(CONFIG_GPIO_SAMA5D2_PIOBU) += gpio-sama5d2-piobu.o
diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
new file mode 100644
index 000000000000..332e2925a830
--- /dev/null
+++ b/drivers/gpio/gpio-rpmsg.c
@@ -0,0 +1,568 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2026 NXP
+ *
+ * The driver exports a standard gpiochip interface to control
+ * the GPIO controllers via RPMSG on a remote processor.
+ */
+
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/irqdomain.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+#include <linux/rpmsg.h>
+#include <linux/virtio_gpio.h>
+
+#define GPIOS_PER_PORT_DEFAULT 32
+#define RPMSG_TIMEOUT 1000
+
+/* Additional commands beyond virtio-gpio */
+#define VIRTIO_GPIO_MSG_SET_WAKEUP 0x0010
+
+/* GPIO Receive MSG Type */
+#define GPIO_RPMSG_REPLY 1
+#define GPIO_RPMSG_NOTIFY 2
+
+#define CHAN_NAME_PREFIX "rpmsg-io-"
+#define GPIO_COMPAT_STR "rpmsg-gpio"
+
+struct rpmsg_gpio_response {
+ __u8 type;
+ union {
+ /* command reply */
+ struct {
+ __u8 status;
+ __u8 value;
+ };
+
+ /* interrupt notification */
+ struct {
+ __u8 line;
+ __u8 trigger; /* rising/falling/high/low */
+ };
+ };
+};
+
+struct rpmsg_gpio_line {
+ u8 irq_shutdown;
+ u8 irq_unmask;
+ u8 irq_mask;
+ u32 irq_wake_enable;
+ u32 irq_type;
+};
+
+struct rpmsg_gpio_port {
+ struct gpio_chip gc;
+ struct rpmsg_device *rpdev;
+ struct virtio_gpio_request *send_msg;
+ struct rpmsg_gpio_response *recv_msg;
+ struct completion cmd_complete;
+ struct mutex lock;
+ u32 ngpios;
+ u32 idx;
+ struct rpmsg_gpio_line lines[GPIOS_PER_PORT_DEFAULT];
+};
+
+static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port)
+{
+ int ret;
+
+ reinit_completion(&port->cmd_complete);
+
+ ret = rpmsg_send(port->rpdev->ept, port->send_msg, sizeof(*port->send_msg));
+ if (ret) {
+ dev_err(&port->rpdev->dev, "rpmsg_send failed: cmd=%d ret=%d\n",
+ port->send_msg->type, ret);
+ return ret;
+ }
+
+ ret = wait_for_completion_timeout(&port->cmd_complete,
+ msecs_to_jiffies(RPMSG_TIMEOUT));
+ if (ret == 0) {
+ dev_err(&port->rpdev->dev, "rpmsg_send timeout! cmd=%d\n",
+ port->send_msg->type);
+ return -ETIMEDOUT;
+ }
+
+ if (unlikely(port->recv_msg->status != VIRTIO_GPIO_STATUS_OK)) {
+ dev_err(&port->rpdev->dev, "remote core replies an error: cmd=%d!\n",
+ port->send_msg->type);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static struct virtio_gpio_request *
+rpmsg_gpio_msg_prepare(struct rpmsg_gpio_port *port, u16 line, u16 cmd, u32 val)
+{
+ struct virtio_gpio_request *msg = port->send_msg;
+
+ msg->type = cmd;
+ msg->gpio = line;
+ msg->value = val;
+
+ return msg;
+}
+
+static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int line)
+{
+ struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+ int ret;
+
+ guard(mutex)(&port->lock);
+
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_GET_VALUE, 0);
+
+ ret = rpmsg_gpio_send_message(port);
+ return ret ? ret : port->recv_msg->value;
+}
+
+static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned int line)
+{
+ struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+ int ret;
+
+ guard(mutex)(&port->lock);
+
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_GET_DIRECTION, 0);
+
+ ret = rpmsg_gpio_send_message(port);
+ if (ret)
+ return ret;
+
+ switch (port->recv_msg->value) {
+ case VIRTIO_GPIO_DIRECTION_IN:
+ return GPIO_LINE_DIRECTION_IN;
+ case VIRTIO_GPIO_DIRECTION_OUT:
+ return GPIO_LINE_DIRECTION_OUT;
+ default:
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static int rpmsg_gpio_direction_input(struct gpio_chip *gc, unsigned int line)
+{
+ struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+
+ guard(mutex)(&port->lock);
+
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
+ VIRTIO_GPIO_DIRECTION_IN);
+
+ return rpmsg_gpio_send_message(port);
+}
+
+static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int line, int val)
+{
+ struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+
+ guard(mutex)(&port->lock);
+
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
+
+ return rpmsg_gpio_send_message(port);
+}
+
+static int rpmsg_gpio_direction_output(struct gpio_chip *gc, unsigned int line, int val)
+{
+ struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+ int ret;
+
+ guard(mutex)(&port->lock);
+
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
+ VIRTIO_GPIO_DIRECTION_OUT);
+
+ ret = rpmsg_gpio_send_message(port);
+ if (ret)
+ return ret;
+
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
+
+ return rpmsg_gpio_send_message(port);
+}
+
+static int gpio_rpmsg_irq_set_type(struct irq_data *d, u32 type)
+{
+ struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 line = d->hwirq;
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ type = VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING;
+ irq_set_handler_locked(d, handle_simple_irq);
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ type = VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING;
+ irq_set_handler_locked(d, handle_simple_irq);
+ break;
+ case IRQ_TYPE_EDGE_BOTH:
+ type = VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH;
+ irq_set_handler_locked(d, handle_simple_irq);
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
+ irq_set_handler_locked(d, handle_level_irq);
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH;
+ irq_set_handler_locked(d, handle_level_irq);
+ break;
+ default:
+ dev_err(&port->rpdev->dev, "unsupported irq type: %u\n", type);
+ return -EINVAL;
+ }
+
+ port->lines[line].irq_type = type;
+
+ return 0;
+}
+
+static int gpio_rpmsg_irq_set_wake(struct irq_data *d, u32 enable)
+{
+ struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 line = d->hwirq;
+
+ port->lines[line].irq_wake_enable = enable;
+
+ return 0;
+}
+
+/*
+ * This unmask/mask function is invoked in two situations:
+ * - when an interrupt is being set up, and
+ * - after an interrupt has occurred.
+ *
+ * The GPIO driver does not access hardware registers directly.
+ * Instead, it caches all relevant information locally, and then sends
+ * the accumulated state to the remote system at this stage.
+ */
+static void gpio_rpmsg_unmask_irq(struct irq_data *d)
+{
+ struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 line = d->hwirq;
+
+ port->lines[line].irq_unmask = 1;
+}
+
+static void gpio_rpmsg_mask_irq(struct irq_data *d)
+{
+ struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 line = d->hwirq;
+
+ /*
+ * When an interrupt occurs, the remote system masks the interrupt
+ * and then sends a notification to Linux. After Linux processes
+ * that notification, it sends an RPMsg command back to the remote
+ * system to unmask the interrupt again.
+ */
+ port->lines[line].irq_mask = 1;
+}
+
+static void gpio_rpmsg_irq_shutdown(struct irq_data *d)
+{
+ struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 line = d->hwirq;
+
+ port->lines[line].irq_shutdown = 1;
+}
+
+static void gpio_rpmsg_irq_bus_lock(struct irq_data *d)
+{
+ struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+
+ mutex_lock(&port->lock);
+}
+
+static void gpio_rpmsg_irq_bus_sync_unlock(struct irq_data *d)
+{
+ struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 line = d->hwirq;
+
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_WAKEUP,
+ port->lines[line].irq_wake_enable);
+ rpmsg_gpio_send_message(port);
+
+ /*
+ * For mask irq, do nothing here.
+ * The remote system will mask interrupt after an interrupt occurs,
+ * and then send a notification to Linux system. After Linux system
+ * handles the notification, it sends an rpmsg back to the remote
+ * system to unmask this interrupt again.
+ */
+ if (port->lines[line].irq_mask && !port->lines[line].irq_unmask) {
+ port->lines[line].irq_mask = 0;
+ mutex_unlock(&port->lock);
+ return;
+ }
+
+ if (port->lines[line].irq_shutdown) {
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE,
+ VIRTIO_GPIO_IRQ_TYPE_NONE);
+ port->lines[line].irq_shutdown = 0;
+ } else {
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE,
+ port->lines[line].irq_type);
+
+ if (port->lines[line].irq_unmask)
+ port->lines[line].irq_unmask = 0;
+ }
+
+ rpmsg_gpio_send_message(port);
+ mutex_unlock(&port->lock);
+}
+
+static const struct irq_chip gpio_rpmsg_irq_chip = {
+ .irq_mask = gpio_rpmsg_mask_irq,
+ .irq_unmask = gpio_rpmsg_unmask_irq,
+ .irq_set_wake = gpio_rpmsg_irq_set_wake,
+ .irq_set_type = gpio_rpmsg_irq_set_type,
+ .irq_shutdown = gpio_rpmsg_irq_shutdown,
+ .irq_bus_lock = gpio_rpmsg_irq_bus_lock,
+ .irq_bus_sync_unlock = gpio_rpmsg_irq_bus_sync_unlock,
+ .flags = IRQCHIP_IMMUTABLE,
+};
+
+static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev,
+ struct device_node *np, const char *name)
+{
+ struct rpmsg_gpio_port *port;
+ struct gpio_irq_chip *girq;
+ struct gpio_chip *gc;
+ int ret;
+
+ port = devm_kzalloc(&rpdev->dev, sizeof(*port), GFP_KERNEL);
+ if (!port)
+ return -ENOMEM;
+
+ ret = of_property_read_u32(np, "reg", &port->idx);
+ if (ret)
+ return ret;
+
+ ret = devm_mutex_init(&rpdev->dev, &port->lock);
+ if (ret)
+ return ret;
+
+ ret = of_property_read_u32(np, "ngpios", &port->ngpios);
+ if (ret || port->ngpios > GPIOS_PER_PORT_DEFAULT)
+ port->ngpios = GPIOS_PER_PORT_DEFAULT;
+
+ port->send_msg = devm_kzalloc(&rpdev->dev,
+ sizeof(*port->send_msg),
+ GFP_KERNEL);
+
+ port->recv_msg = devm_kzalloc(&rpdev->dev,
+ sizeof(*port->recv_msg),
+ GFP_KERNEL);
+ if (!port->send_msg || !port->recv_msg)
+ return -ENOMEM;
+
+ init_completion(&port->cmd_complete);
+ port->rpdev = rpdev;
+
+ gc = &port->gc;
+ gc->owner = THIS_MODULE;
+ gc->parent = &rpdev->dev;
+ gc->fwnode = of_fwnode_handle(np);
+ gc->ngpio = port->ngpios;
+ gc->base = -1;
+ gc->label = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
+ name, port->idx);
+
+ gc->direction_input = rpmsg_gpio_direction_input;
+ gc->direction_output = rpmsg_gpio_direction_output;
+ gc->get_direction = rpmsg_gpio_get_direction;
+ gc->get = rpmsg_gpio_get;
+ gc->set = rpmsg_gpio_set;
+
+ girq = &gc->irq;
+ gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
+ girq->parent_handler = NULL;
+ girq->num_parents = 0;
+ girq->parents = NULL;
+ girq->chip->name = devm_kstrdup(&rpdev->dev, gc->label, GFP_KERNEL);
+
+ dev_set_drvdata(&rpdev->dev, port);
+
+ return devm_gpiochip_add_data(&rpdev->dev, gc, port);
+}
+
+static const char *rpmsg_get_rproc_node_name(struct rpmsg_device *rpdev)
+{
+ const char *name = NULL;
+ struct device_node *np;
+ struct rproc *rproc;
+
+ rproc = rproc_get_by_child(&rpdev->dev);
+ if (!rproc)
+ return NULL;
+
+ np = of_node_get(rproc->dev.of_node);
+ if (!np && rproc->dev.parent)
+ np = of_node_get(rproc->dev.parent->of_node);
+
+ if (np) {
+ name = devm_kstrdup(&rpdev->dev, np->name, GFP_KERNEL);
+ of_node_put(np);
+ }
+
+ return name;
+}
+
+static struct device_node *
+rpmsg_find_child_by_compat_reg(struct device_node *parent, const char *compat, u32 idx)
+{
+ struct device_node *child;
+ u32 reg;
+
+ for_each_available_child_of_node(parent, child) {
+ if (!of_device_is_compatible(child, compat))
+ continue;
+
+ if (of_property_read_u32(child, "reg", ®))
+ continue;
+
+ if (reg == idx)
+ return child;
+ }
+
+ return NULL;
+}
+
+static struct device_node *
+rpmsg_get_channel_ofnode(struct rpmsg_device *rpdev, const char *compat, u32 idx)
+{
+ struct device_node *np_chan = NULL, *np;
+ struct rproc *rproc;
+
+ rproc = rproc_get_by_child(&rpdev->dev);
+ if (!rproc)
+ return NULL;
+
+ np = of_node_get(rproc->dev.of_node);
+ if (!np && rproc->dev.parent)
+ np = of_node_get(rproc->dev.parent->of_node);
+
+ if (np)
+ np_chan = rpmsg_find_child_by_compat_reg(np, compat, idx);
+
+ return np_chan;
+}
+
+static int rpmsg_get_gpio_index(const char *name, const char *prefix)
+{
+ const char *p;
+ int base = 10;
+ int val;
+
+ if (!name)
+ return -EINVAL;
+
+ /* Ensure correct prefix */
+ if (!str_has_prefix(name, prefix))
+ return -EINVAL;
+
+ /* Find last '-' */
+ p = strrchr(name, '-');
+
+ if (!p || *(p + 1) == '\0')
+ return -EINVAL;
+
+ if (p[1] == '0' && (p[2] == 'x' || p[2] == 'X'))
+ base = 16;
+
+ if (kstrtoint(p + 1, base, &val))
+ return -EINVAL;
+
+ return val;
+}
+
+static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
+ int len, void *priv, u32 src)
+{
+ struct rpmsg_gpio_response *msg = data;
+ struct rpmsg_gpio_port *port = NULL;
+
+ port = dev_get_drvdata(&rpdev->dev);
+
+ if (!port) {
+ dev_err(&rpdev->dev, "port is null\n");
+ return -EINVAL;
+ }
+
+ if (msg->type == GPIO_RPMSG_REPLY) {
+ *port->recv_msg = *msg;
+ complete(&port->cmd_complete);
+ } else if (msg->type == GPIO_RPMSG_NOTIFY) {
+ generic_handle_domain_irq_safe(port->gc.irq.domain, msg->line);
+ } else {
+ dev_err(&rpdev->dev, "wrong message type (0x%x)\n", msg->type);
+ }
+
+ return 0;
+}
+
+static int rpmsg_gpio_channel_probe(struct rpmsg_device *rpdev)
+{
+ struct device *dev = &rpdev->dev;
+ struct device_node *np;
+ const char *rproc_name;
+ int idx;
+
+ idx = rpmsg_get_gpio_index(rpdev->id.name, CHAN_NAME_PREFIX);
+ if (idx < 0)
+ return -EINVAL;
+
+ if (!dev->of_node) {
+ np = rpmsg_get_channel_ofnode(rpdev, GPIO_COMPAT_STR, idx);
+ if (!np)
+ return -ENODEV;
+
+ dev->of_node = np;
+ set_primary_fwnode(dev, of_fwnode_handle(np));
+ return -EPROBE_DEFER;
+ }
+
+ rproc_name = rpmsg_get_rproc_node_name(rpdev);
+
+ return rpmsg_gpiochip_register(rpdev, dev->of_node, rproc_name);
+}
+
+static const struct of_device_id rpmsg_gpio_dt_ids[] = {
+ { .compatible = GPIO_COMPAT_STR },
+ { /* sentinel */ }
+};
+
+static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
+ { .name = CHAN_NAME_PREFIX },
+ { },
+};
+MODULE_DEVICE_TABLE(rpmsg, rpmsg_gpio_channel_id_table);
+
+static struct rpmsg_driver rpmsg_gpio_channel_client = {
+ .callback = rpmsg_gpio_channel_callback,
+ .id_table = rpmsg_gpio_channel_id_table,
+ .probe = rpmsg_gpio_channel_probe,
+ .drv = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = rpmsg_gpio_dt_ids,
+ },
+};
+module_rpmsg_driver(rpmsg_gpio_channel_client);
+
+MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
+MODULE_DESCRIPTION("generic rpmsg gpio driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related
* [PATCH v14 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn
In-Reply-To: <20260625155432.815185-1-shenwei.wang@oss.nxp.com>
From: Shenwei Wang <shenwei.wang@nxp.com>
Add the RPMSG bus node along with its GPIO subnodes to the device
tree.
Enable remote device communication and GPIO control via RPMSG on
the i.MX platform.
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index 1de3ad60c6aa..f1b984eb1203 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -190,6 +190,31 @@ scmi_sensor: protocol@15 {
cm33: remoteproc-cm33 {
compatible = "fsl,imx8ulp-cm33";
status = "disabled";
+
+ rpmsg {
+ rpmsg-io {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rpmsg_gpioa: gpio@0 {
+ compatible = "rpmsg-gpio";
+ reg = <0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ };
+
+ rpmsg_gpiob: gpio@1 {
+ compatible = "rpmsg-gpio";
+ reg = <1>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ };
+ };
+ };
};
soc: soc@0 {
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v14 2/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
From: sashiko-bot @ 2026-06-25 16:00 UTC (permalink / raw)
To: Shenwei Wang; +Cc: conor+dt, robh, Frank.Li, imx, devicetree
In-Reply-To: <20260625155432.815185-3-shenwei.wang@oss.nxp.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The `gpio-rpmsg.yaml` binding adds GPIO and interrupt controller provider capabilities but fails to list `gpio-controller` and `interrupt-controller` in its `required` properties array.
- [Medium] The `rpmsg` node schema hardcodes `rpmsg-io` as the only allowed subnode, contradicting the commit message's description of flexible channel names.
- [Low] The `patternProperties` regex used for matching GPIO subnodes in `fsl,imx-rproc.yaml` is missing a start-of-string anchor (`^`).
--
commit e02180a974397446bfafe8bc43641010b7365078
Author: Shenwei Wang <shenwei.wang@nxp.com>
dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
This commit adds device tree binding support for RPMSG GPIO controllers
exposed by a remote processor. It introduces a generic RPMSG GPIO controller
schema and updates the fsl,imx-rproc binding to support a new rpmsg subnode
container for these channels.
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> new file mode 100644
> index 0000000000000..6c78b6850321c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
[ ... ]
> + "#interrupt-cells":
> + const: 2
> +
> +required:
> + - compatible
> + - reg
> + - "#gpio-cells"
> + - "#interrupt-cells"
[Severity: Medium]
Should gpio-controller and interrupt-controller be included in this required
list? The binding adds GPIO and interrupt controller provider capabilities,
so omitting them might allow incomplete device tree nodes to pass validation.
> +
> +allOf:
> + - $ref: /schemas/gpio/gpio.yaml#
> +
> +unevaluatedProperties: false
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index ce8ec0119469c..aea33205a881a 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
[ ... ]
> @@ -85,6 +85,34 @@ properties:
> This property is to specify the resource id of the remote processor in SoC
> which supports SCFW
>
> + rpmsg:
> + type: object
> + additionalProperties: false
> + description:
> + Represents the RPMSG bus between Linux and the remote system. Contains
> + a group of RPMSG channel devices running on the bus.
> +
> + properties:
> + rpmsg-io:
> + type: object
> + additionalProperties: false
[Severity: Medium]
The commit message states "The name of each subnode corresponds to the
channel name as defined by the remote processor".
Does hardcoding rpmsg-io as a property and setting additionalProperties to
false unintentionally restrict the schema from allowing flexible channel
names?
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + patternProperties:
> + "gpio@[0-9a-f]+$":
> + type: object
> + $ref: /schemas/gpio/gpio-rpmsg.yaml#
> + unevaluatedProperties: false
[Severity: Low]
Does this regex need a start-of-string anchor (^)? Without it, could this
unintentionally match any node name that ends in gpio@[0-9a-f]+?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=2
^ permalink raw reply
* [PATCH 1/2] dt-bindings: arm: ti: Add bindings for PHYTEC AM67x based hardware
From: Nathan Morrisson @ 2026-06-25 16:02 UTC (permalink / raw)
To: nm, vigneshr, kristo, robh, krzk+dt, conor+dt
Cc: linux-arm-kernel, devicetree, linux-kernel, upstream
Add device tree bindings for the AM67x based phyCORE-AM67x SoM and
phyBOARD-Rigel.
Signed-off-by: Nathan Morrisson <nmorrisson@phytec.com>
---
Documentation/devicetree/bindings/arm/ti/k3.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/ti/k3.yaml b/Documentation/devicetree/bindings/arm/ti/k3.yaml
index 69b5441cbf1a..2dbe461f4520 100644
--- a/Documentation/devicetree/bindings/arm/ti/k3.yaml
+++ b/Documentation/devicetree/bindings/arm/ti/k3.yaml
@@ -222,6 +222,13 @@ properties:
- ti,j722s-evm
- const: ti,j722s
+ - description: K3 AM67 SoC PHYTEC phyBOARD-Rigel
+ items:
+ - enum:
+ - phytec,am6754-phyboard-rigel
+ - const: phytec,am67-phycore-som
+ - const: ti,j722s
+
- description: K3 J742S2 SoC
items:
- enum:
--
2.43.0
^ permalink raw reply related
* [PATCH 2/2] arm64: dts: ti: Add support for the phyCORE-AM67x
From: Nathan Morrisson @ 2026-06-25 16:02 UTC (permalink / raw)
To: nm, vigneshr, kristo, robh, krzk+dt, conor+dt
Cc: linux-arm-kernel, devicetree, linux-kernel, upstream
In-Reply-To: <20260625160214.4001298-1-nmorrisson@phytec.com>
Add support for the PHYTEC phyCORE-AM67x SoM [1] and the
corresponding phyBOARD-Rigel carrier board [2]. The phyCORE-AM67x SoM
uses the TI AM67x SoC and can come with different sizes and models of
DDR, eMMC, and SPI NOR Flash.
Supported features:
* Audio playback and recording
* CAN
* Debug UART
* eMMC
* Ethernet
* GPIO buttons
* Heartbeat LED
* I2C Current sensor
* I2C EEPROM
* I2C Light sensor
* I2C RTC
* Micro SD card
* PCIe
* SPI NOR flash
* USB
[1] https://www.phytec.com/product/phycore-am67x/
[2] https://www.phytec.com/product/phyboard-am67x-development-kit/
Signed-off-by: Nathan Morrisson <nmorrisson@phytec.com>
---
arch/arm64/boot/dts/ti/Makefile | 1 +
.../boot/dts/ti/k3-am67-phycore-som.dtsi | 328 ++++++++++++
.../boot/dts/ti/k3-am6754-phyboard-rigel.dts | 502 ++++++++++++++++++
3 files changed, 831 insertions(+)
create mode 100644 arch/arm64/boot/dts/ti/k3-am67-phycore-som.dtsi
create mode 100644 arch/arm64/boot/dts/ti/k3-am6754-phyboard-rigel.dts
diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index 371f9a043fe5..623ee2369132 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -184,6 +184,7 @@ dtb-$(CONFIG_ARCH_K3) += k3-j721s2-evm-pcie1-ep.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-j721s2-evm-usb0-type-a.dtbo
# Boards with J722s SoC
+dtb-$(CONFIG_ARCH_K3) += k3-am6754-phyboard-rigel.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am67a-beagley-ai.dtb
dtb-$(CONFIG_ARCH_K3) += k3-j722s-evm.dtb
dtb-$(CONFIG_ARCH_K3) += k3-j722s-evm-csi2-quad-rpi-cam-imx219.dtbo
diff --git a/arch/arm64/boot/dts/ti/k3-am67-phycore-som.dtsi b/arch/arm64/boot/dts/ti/k3-am67-phycore-som.dtsi
new file mode 100644
index 000000000000..8a40f648098e
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-am67-phycore-som.dtsi
@@ -0,0 +1,328 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Copyright (C) 2026 PHYTEC America LLC
+ * Author: Nathan Morrisson <nmorrisson@phytec.com>
+ */
+
+#include <dt-bindings/net/ti-dp83867.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+ compatible = "phytec,am67-phycore-som", "ti,j722s";
+ model = "PHYTEC phyCORE-AM67";
+
+ aliases {
+ ethernet0 = &cpsw_port1;
+ gpio0 = &main_gpio0;
+ mmc0 = &sdhci0;
+ rtc0 = &i2c_som_rtc;
+ rtc1 = &wkup_rtc0;
+ spi0 = &ospi0;
+ };
+
+ memory@80000000 {
+ /* 4G RAM */
+ reg = <0x00000000 0x80000000 0x00000000 0x80000000>,
+ <0x00000008 0x80000000 0x00000000 0x80000000>;
+ device_type = "memory";
+ bootph-all;
+ };
+
+ reserved_memory: reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ secure_tfa_ddr: tfa@9e780000 {
+ reg = <0x00 0x9e780000 0x00 0x80000>;
+ no-map;
+ };
+
+ secure_ddr: optee@9e800000 {
+ reg = <0x00 0x9e800000 0x00 0x01800000>;
+ no-map;
+ };
+
+ wkup_r5fss0_core0_dma_memory_region: memory@a0000000 {
+ compatible = "shared-dma-pool";
+ reg = <0x00 0xa0000000 0x00 0x100000>;
+ no-map;
+ };
+
+ wkup_r5fss0_core0_memory_region: memory@a0100000 {
+ compatible = "shared-dma-pool";
+ reg = <0x00 0xa0100000 0x00 0xf00000>;
+ no-map;
+ };
+ };
+
+ vcc_5v0_som: regulator-vcc-5v0-som {
+ compatible = "regulator-fixed";
+ regulator-name = "VCC_5V0_SOM";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <&leds_pins_default>;
+
+ led-0 {
+ color = <LED_COLOR_ID_GREEN>;
+ gpios = <&main_gpio0 13 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "heartbeat";
+ function = LED_FUNCTION_HEARTBEAT;
+ };
+ };
+};
+
+&main_pmx0 {
+ leds_pins_default: leds-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x034, PIN_OUTPUT, 7) /* (K22) OSPI0_CSN2.GPIO0_13 */
+ >;
+ };
+
+ mdio_pins_default: mdio-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x0160, PIN_OUTPUT, 0) /* (AC24) MDIO0_MDC */
+ J722S_IOPAD(0x015c, PIN_INPUT, 0) /* (AD25) MDIO0_MDIO */
+ >;
+ bootph-all;
+ };
+
+ ospi0_pins_default: ospi0-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x000, PIN_OUTPUT, 0) /* (L24) OSPI0_CLK */
+ J722S_IOPAD(0x02c, PIN_OUTPUT, 0) /* (K26) OSPI0_CSn0 */
+ J722S_IOPAD(0x00c, PIN_INPUT, 0) /* (K27) OSPI0_D0 */
+ J722S_IOPAD(0x010, PIN_INPUT, 0) /* (L27) OSPI0_D1 */
+ J722S_IOPAD(0x014, PIN_INPUT, 0) /* (L26) OSPI0_D2 */
+ J722S_IOPAD(0x018, PIN_INPUT, 0) /* (L25) OSPI0_D3 */
+ J722S_IOPAD(0x01c, PIN_INPUT, 0) /* (L21) OSPI0_D4 */
+ J722S_IOPAD(0x020, PIN_INPUT, 0) /* (M26) OSPI0_D5 */
+ J722S_IOPAD(0x024, PIN_INPUT, 0) /* (N27) OSPI0_D6 */
+ J722S_IOPAD(0x028, PIN_INPUT, 0) /* (M27) OSPI0_D7 */
+ J722S_IOPAD(0x008, PIN_INPUT, 0) /* (L22) OSPI0_DQS */
+ J722S_IOPAD(0x038, PIN_INPUT, 7) /* (J22) OSPI0_CSn3.GPIO0_14 */
+ >;
+ bootph-all;
+ };
+
+ pmic_irq_pins_default: pmic-irq-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x030, PIN_INPUT, 7) /* (K23) OSPI0_CSN1.GPIO0_12 */
+ >;
+ };
+
+ rgmii1_pins_default: rgmii1-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x014c, PIN_INPUT, 0) /* (AC25) RGMII1_RD0 */
+ J722S_IOPAD(0x0150, PIN_INPUT, 0) /* (AD27) RGMII1_RD1 */
+ J722S_IOPAD(0x0154, PIN_INPUT, 0) /* (AE24) RGMII1_RD2 */
+ J722S_IOPAD(0x0158, PIN_INPUT, 0) /* (AE26) RGMII1_RD3 */
+ J722S_IOPAD(0x0148, PIN_INPUT, 0) /* (AE27) RGMII1_RXC */
+ J722S_IOPAD(0x0144, PIN_INPUT, 0) /* (AD23) RGMII1_RX_CTL */
+ J722S_IOPAD(0x0134, PIN_OUTPUT, 0) /* (AF27) RGMII1_TD0 */
+ J722S_IOPAD(0x0138, PIN_OUTPUT, 0) /* (AE23) RGMII1_TD1 */
+ J722S_IOPAD(0x013c, PIN_OUTPUT, 0) /* (AG25) RGMII1_TD2 */
+ J722S_IOPAD(0x0140, PIN_OUTPUT, 0) /* (AF24) RGMII1_TD3 */
+ J722S_IOPAD(0x0130, PIN_OUTPUT, 0) /* (AG26) RGMII1_TXC */
+ J722S_IOPAD(0x012c, PIN_OUTPUT, 0) /* (AF25) RGMII1_TX_CTL */
+ >;
+ bootph-all;
+ };
+};
+
+&mcu_pmx0 {
+ wkup_i2c0_pins_default: wkup-i2c0-default-pins {
+ pinctrl-single,pins = <
+ J722S_MCU_IOPAD(0x04c, PIN_INPUT_PULLUP, 0) /* (B9) WKUP_I2C0_SCL */
+ J722S_MCU_IOPAD(0x050, PIN_INPUT_PULLUP, 0) /* (D11) WKUP_I2C0_SDA */
+ >;
+ bootph-all;
+ };
+};
+
+&cpsw3g {
+ pinctrl-names = "default";
+ pinctrl-0 = <&rgmii1_pins_default>;
+ bootph-all;
+ status = "okay";
+};
+
+&cpsw3g_mdio {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mdio_pins_default>;
+ status = "okay";
+
+ cpsw3g_phy1: ethernet-phy@1 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
+ tx-fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
+ ti,min-output-impedance;
+ };
+};
+
+&cpsw_port1 {
+ phy-mode = "rgmii-id";
+ phy-handle = <&cpsw3g_phy1>;
+ status = "okay";
+};
+
+&cpsw_port2 {
+ status = "disabled";
+};
+
+&ospi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&ospi0_pins_default>;
+ bootph-all;
+ status = "okay";
+
+ serial_flash: flash@0 {
+ compatible = "jedec,spi-nor";
+ reg = <0x0>;
+ spi-tx-bus-width = <8>;
+ spi-rx-bus-width = <8>;
+ spi-max-frequency = <25000000>;
+ vcc-supply = <&vdd_1v8>;
+ cdns,tshsl-ns = <60>;
+ cdns,tsd2d-ns = <60>;
+ cdns,tchsh-ns = <60>;
+ cdns,tslch-ns = <60>;
+ cdns,read-delay = <0>;
+ };
+};
+
+&sdhci0 {
+ non-removable;
+ bootph-all;
+ ti,driver-strength-ohm = <50>;
+ status = "okay";
+};
+
+&wkup_i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&wkup_i2c0_pins_default>;
+ clock-frequency = <400000>;
+ bootph-all;
+ status = "okay";
+
+ pmic@30 {
+ compatible = "ti,tps65219";
+ reg = <0x30>;
+ buck1-supply = <&vcc_5v0_som>;
+ buck2-supply = <&vcc_5v0_som>;
+ buck3-supply = <&vcc_5v0_som>;
+ ldo1-supply = <&vdd_3v3>;
+ ldo2-supply = <&vdd_1v8>;
+ ldo3-supply = <&vdd_3v3>;
+ ldo4-supply = <&vdd_3v3>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pmic_irq_pins_default>;
+ interrupt-parent = <&main_gpio0>;
+ interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ system-power-controller;
+ ti,power-button;
+
+ regulators {
+ vdd_3v3: buck1 {
+ regulator-name = "VDD_3V3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ vdd_1v8: buck2 {
+ regulator-name = "VDD_1V8";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ vdd_lpddr4: buck3 {
+ regulator-name = "VDD_LPDDR4";
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ vddshv_sdio: ldo1 {
+ regulator-name = "VDDSHV_SDIO";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-allow-bypass;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ vdd_1v2: ldo2 {
+ regulator-name = "VDD_1V2";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ vdda_1v8_phy: ldo3 {
+ regulator-name = "VDDA_1V8_PHY";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ vdd_1v8_pll: ldo4 {
+ regulator-name = "VDD_1V8_PLL";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+ };
+ };
+
+ vdd_core: regulator-vdd-core@44 {
+ compatible = "ti,tps62873";
+ reg = <0x44>;
+ bootph-pre-ram;
+ regulator-name = "VDD_CORE";
+ regulator-min-microvolt = <850000>;
+ regulator-max-microvolt = <850000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ eeprom@50 {
+ compatible = "atmel,24c32";
+ reg = <0x50>;
+ pagesize = <32>;
+ };
+
+ som_eeprom_opt: eeprom@51 {
+ compatible = "atmel,24c32";
+ reg = <0x51>;
+ pagesize = <32>;
+ };
+
+ i2c_som_rtc: rtc@52 {
+ compatible = "microcrystal,rv3028";
+ reg = <0x52>;
+ };
+};
+
+#include "k3-j722s-ti-ipc-firmware.dtsi"
diff --git a/arch/arm64/boot/dts/ti/k3-am6754-phyboard-rigel.dts b/arch/arm64/boot/dts/ti/k3-am6754-phyboard-rigel.dts
new file mode 100644
index 000000000000..7853d4f5d3b9
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-am6754-phyboard-rigel.dts
@@ -0,0 +1,502 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Copyright (C) 2026 PHYTEC America LLC
+ * Author: Nathan Morrisson <nmorrisson@phytec.com>
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/phy/phy.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include "k3-serdes.h"
+#include "k3-j722s.dtsi"
+#include "k3-am67-phycore-som.dtsi"
+
+/ {
+ compatible = "phytec,am6754-phyboard-rigel",
+ "phytec,am67-phycore-som", "ti,j722s";
+ model = "PHYTEC phyBOARD-Rigel AM67";
+
+ aliases {
+ gpio1 = &main_gpio1;
+ mmc1 = &sdhci1;
+ serial2 = &main_uart0;
+ usb0 = &usb0;
+ usb1 = &usb1;
+ };
+
+ can_tc0: can-phy0 {
+ compatible = "ti,tcan1042";
+ #phy-cells = <0>;
+ max-bitrate = <8000000>;
+ standby-gpios = <&gpio_exp1 1 GPIO_ACTIVE_HIGH>;
+ };
+
+ usb0_connector: connector {
+ compatible = "gpio-usb-b-connector", "usb-b-connector";
+ label = "USB-C";
+ data-role = "dual";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_usbc_power_pins_default>;
+
+ id-gpios = <&main_gpio1 15 GPIO_ACTIVE_HIGH>;
+
+ port {
+ usb0_con: endpoint {
+ remote-endpoint = <&usb0_ep>;
+ };
+ };
+ };
+
+ keys {
+ compatible = "gpio-keys";
+ autorepeat;
+ pinctrl-names = "default";
+ pinctrl-0 = <&gpio_keys_pins_default>;
+
+ key-home {
+ label = "home";
+ linux,code = <KEY_HOME>;
+ gpios = <&main_gpio1 23 GPIO_ACTIVE_HIGH>;
+ };
+
+ key-menu {
+ label = "menu";
+ linux,code = <KEY_MENU>;
+ gpios = <&gpio_exp1 4 GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+ pcie_refclk0: pcie-refclk0 {
+ compatible = "gpio-gate-clock";
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_pcie_usb_sel_pins_default>;
+ clocks = <&serdes_refclk>;
+ #clock-cells = <0>;
+ enable-gpios = <&main_gpio0 22 GPIO_ACTIVE_LOW>;
+ };
+
+ vcc_1v8: regulator-vcc-1v8 {
+ compatible = "regulator-fixed";
+ regulator-name = "VCC_1V8";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ vcc_3v3_aud: regulator-vcc-3v3-aud {
+ compatible = "regulator-fixed";
+ regulator-name = "VCC_3V3_AUD";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ vcc_3v3_mmc: regulator-vcc-3v3-mmc {
+ /* TPS22963C OUTPUT */
+ compatible = "regulator-fixed";
+ regulator-name = "VCC_3V3_MMC";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ vcc_3v3_sw: regulator-vcc-3v3-sw {
+ compatible = "regulator-fixed";
+ regulator-name = "VCC_3V3_SW";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ vcc_speaker: regulator-vcc-speaker {
+ compatible = "regulator-fixed";
+ regulator-name = "VCC_SPEAKER";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,widgets =
+ "Microphone", "Mic Jack",
+ "Headphone", "Headphone Jack",
+ "Line", "Stereo Jack",
+ "Speaker", "L SPKR",
+ "Speaker", "R SPKR";
+ simple-audio-card,routing =
+ "MIC1RP", "Mic Jack",
+ "Mic Jack", "MICBIAS",
+ "Headphone Jack", "HPL",
+ "Headphone Jack", "HPR",
+ "MIC1LM", "Stereo Jack",
+ "MIC1LP", "Stereo Jack",
+ "SPL", "L SPKR",
+ "SPR", "R SPKR";
+ simple-audio-card,name = "phyBOARD-Rigel";
+ simple-audio-card,format = "dsp_b";
+ simple-audio-card,bitclock-master = <&sound_master>;
+ simple-audio-card,frame-master = <&sound_master>;
+ simple-audio-card,bitclock-inversion;
+
+ simple-audio-card,cpu {
+ sound-dai = <&mcasp0>;
+ };
+
+ sound_master: simple-audio-card,codec {
+ sound-dai = <&audio_codec>;
+ clocks = <&audio_refclk1>;
+ };
+ };
+};
+
+&main_pmx0 {
+ audio_ext_refclk1_pins_default: audio-ext-refclk1-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x0a0, PIN_OUTPUT, 1) /* (N24) GPMC0_WPn.AUDIO_EXT_REFCLK1 */
+ >;
+ };
+
+ gpio_exp0_int_pins_default: gpio-exp0-int-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x0054, PIN_INPUT, 7) /* (T21) GPMC0_AD6.GPIO0_21 */
+ >;
+ };
+
+ gpio_exp1_int_pins_default: gpio-exp1-int-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x0244, PIN_INPUT, 7) /* (A24) MMC1_SDWP.GPIO1_49 */
+ >;
+ };
+
+ gpio_exp2_int_pins_default: gpio-exp2-int-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x0050, PIN_INPUT, 7) /* (T24) GPMC0_AD5.GPIO0_20 */
+ >;
+ };
+
+ gpio_keys_pins_default: gpio-keys-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x01d4, PIN_INPUT, 7) /* (B21) UART0_RTSn.GPIO1_23 */
+ >;
+ };
+
+ main_i2c0_pins_default: main-i2c0-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x01e0, PIN_INPUT_PULLUP, 0) /* (D23) I2C0_SCL */
+ J722S_IOPAD(0x01e4, PIN_INPUT_PULLUP, 0) /* (B22) I2C0_SDA */
+ >;
+ bootph-all;
+ };
+
+ main_i2c1_pins_default: main-i2c1-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x01e8, PIN_INPUT_PULLUP, 0) /* (C24) I2C1_SCL */
+ J722S_IOPAD(0x01ec, PIN_INPUT_PULLUP, 0) /* (A22) I2C1_SDA */
+ >;
+ bootph-all;
+ };
+
+ main_mcan0_pins_default: main-mcan0-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x1dc, PIN_INPUT, 0) /* (C22) MCAN0_RX */
+ J722S_IOPAD(0x1d8, PIN_OUTPUT, 0) /* (D22) MCAN0_TX */
+ >;
+ };
+
+ main_mcasp0_pins_default: main-mcasp0-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x1a8, PIN_INPUT, 0) /* (C26) MCASP0_AFSX */
+ J722S_IOPAD(0x1a4, PIN_INPUT, 0) /* (D25) MCASP0_ACLKX */
+ J722S_IOPAD(0x198, PIN_OUTPUT, 0) /* (A26) MCASP0_AXR2 */
+ J722S_IOPAD(0x194, PIN_INPUT, 0) /* (A25) MCASP0_AXR3 */
+ >;
+ };
+
+ main_mcasp1_pins_default: main-mcasp1-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x0090, PIN_INPUT, 2) /* (P27) GPMC0_BE0n_CLE.MCASP1_ACLKX */
+ J722S_IOPAD(0x0098, PIN_INPUT, 2) /* (V21) GPMC0_WAIT0.MCASP1_AFSX */
+ J722S_IOPAD(0x008c, PIN_OUTPUT, 2) /* (N23) GPMC0_WEn.MCASP1_AXR0 */
+ >;
+ };
+
+ main_mmc1_pins_default: main-mmc1-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x023c, PIN_INPUT, 0) /* (H22) MMC1_CMD */
+ J722S_IOPAD(0x0234, PIN_INPUT, 0) /* (H24) MMC1_CLK */
+ J722S_IOPAD(0x0230, PIN_INPUT, 0) /* (H23) MMC1_DAT0 */
+ J722S_IOPAD(0x022c, PIN_INPUT, 0) /* (H20) MMC1_DAT1 */
+ J722S_IOPAD(0x0228, PIN_INPUT, 0) /* (J23) MMC1_DAT2 */
+ J722S_IOPAD(0x0224, PIN_INPUT, 0) /* (H25) MMC1_DAT3 */
+ J722S_IOPAD(0x0240, PIN_INPUT, 0) /* (B24) MMC1_SDCD */
+ >;
+ bootph-all;
+ };
+
+ main_pcie_pins_default: main-pcie-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x07c, PIN_INPUT, 7) /* (T23) GPMC0_CLK.GPIO0_31 */
+ >;
+ };
+
+ main_pcie_usb_sel_pins_default: main-pcie-usb-sel-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x058, PIN_INPUT, 7) /* (T22) GPMC0_AD7.GPIO0_22 */
+ >;
+ };
+
+ main_uart0_pins_default: main-uart0-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x01c8, PIN_INPUT, 0) /* (F19) UART0_RXD */
+ J722S_IOPAD(0x01cc, PIN_OUTPUT, 0) /* (F20) UART0_TXD */
+ >;
+ bootph-all;
+ };
+
+ main_usbc_power_pins_default: main-usbc-power-default-pins {
+ pinctrl-single,pins = <
+ J722S_IOPAD(0x1b4, PIN_INPUT, 7) /* (B20) SPI0_CS0.GPIO1_15 */
+ >;
+ };
+};
+
+&audio_refclk1 {
+ assigned-clock-rates = <25000000>;
+};
+
+&main_i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_i2c0_pins_default>;
+ clock-frequency = <400000>;
+ status = "okay";
+
+ veml6030: light-sensor@10 {
+ compatible = "vishay,veml6030";
+ reg = <0x10>;
+ vdd-supply = <&vcc_3v3_sw>;
+ };
+};
+
+&main_i2c1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_i2c1_pins_default>;
+ clock-frequency = <100000>;
+ status = "okay";
+
+ audio_codec: audio-codec@18 {
+ compatible = "ti,tlv320aic3110";
+ reg = <0x18>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&audio_ext_refclk1_pins_default>;
+ #sound-dai-cells = <0>;
+ ai3xx-micbias-vg = <2>;
+ reset-gpios = <&gpio_exp1 7 GPIO_ACTIVE_LOW>;
+
+ HPVDD-supply = <&vcc_3v3_aud>;
+ SPRVDD-supply = <&vcc_speaker>;
+ SPLVDD-supply = <&vcc_speaker>;
+ AVDD-supply = <&vcc_3v3_aud>;
+ IOVDD-supply = <&vcc_3v3_aud>;
+ DVDD-supply = <&vcc_1v8>;
+ };
+
+ gpio_exp0: gpio@20 {
+ compatible = "nxp,pcf8574";
+ reg = <0x20>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&gpio_exp0_int_pins_default>;
+ interrupt-parent = <&main_gpio0>;
+ interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
+ gpio-line-names = "CSI3_STROBE", "CSI3_TRIGGER",
+ "CSI3_SHUTTER", "CSI3_OE",
+ "CSI2_STROBE", "CSI2_TRIGGER",
+ "CSI2_SHUTTER", "CSI2_OE";
+ };
+
+ gpio_exp1: gpio@21 {
+ compatible = "nxp,pcf8574";
+ reg = <0x21>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&gpio_exp1_int_pins_default>;
+ interrupt-parent = <&main_gpio1>;
+ interrupts = <49 IRQ_TYPE_LEVEL_LOW>;
+ gpio-line-names = "GPIO0_HDMI_RST", "GPIO1_CAN_nEN",
+ "GPIO2_LED", "GPIO3_MCU_CAN0_nEN",
+ "GPIO4_BUT2", "GPIO5_MCU_CAN1_nEN",
+ "GPIO6_AUDIO_GPIO", "GPIO7_AUDIO_USER_RESET";
+ };
+
+ gpio_exp2: gpio@23 {
+ compatible = "nxp,pcf8574";
+ reg = <0x23>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&gpio_exp2_int_pins_default>;
+ interrupt-parent = <&main_gpio0>;
+ interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
+ gpio-line-names = "CSI1_STROBE", "CSI1_TRIGGER",
+ "CSI1_SHUTTER", "CSI1_OE",
+ "CSI0_STROBE", "CSI0_TRIGGER",
+ "CSI0_SHUTTER", "CSI0_OE";
+ };
+
+ current-sensor@40 {
+ compatible = "ti,ina233";
+ reg = <0x40>;
+ shunt-resistor = <18000>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c02";
+ reg = <0x51>;
+ pagesize = <16>;
+ };
+};
+
+&main_mcan0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_mcan0_pins_default>;
+ phys = <&can_tc0>;
+ status = "okay";
+};
+
+&main_uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_uart0_pins_default>;
+ bootph-all;
+ status = "okay";
+};
+
+&mcasp0 {
+ #sound-dai-cells = <0>;
+ op-mode = <0>; /* MCASP_IIS_MODE */
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_mcasp0_pins_default>;
+ tdm-slots = <2>;
+ serial-dir = < /* 0: INACTIVE, 1: TX, 2: RX */
+ 0 0 1 2
+ 0 0 0 0
+ 0 0 0 0
+ 0 0 0 0
+ >;
+ status = "okay";
+};
+
+&mcasp1 {
+ #sound-dai-cells = <0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_mcasp1_pins_default>;
+ op-mode = <0>; /* MCASP_IIS_MODE */
+ tdm-slots = <2>;
+ serial-dir = < /* 0: INACTIVE, 1: TX, 2: RX */
+ 1 0 2 0
+ 0 0 0 0
+ 0 0 0 0
+ 0 0 0 0
+ >;
+ status = "okay";
+};
+
+&pcie0_rc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_pcie_pins_default>;
+ num-lanes = <1>;
+ phys = <&serdes1_pcie_link>;
+ phy-names = "pcie-phy";
+ reset-gpios = <&main_gpio0 31 GPIO_ACTIVE_HIGH>;
+ status = "okay";
+};
+
+&sdhci1 {
+ /* SD/MMC */
+ vmmc-supply = <&vcc_3v3_mmc>;
+ vqmmc-supply = <&vddshv_sdio>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_mmc1_pins_default>;
+ disable-wp;
+ no-1-8-v;
+ bootph-all;
+ status = "okay";
+};
+
+&serdes_ln_ctrl {
+ idle-states = <J722S_SERDES0_LANE0_USB>,
+ <J722S_SERDES1_LANE0_PCIE0_LANE0>;
+};
+
+&serdes0 {
+ status = "okay";
+
+ serdes0_usb_link: phy@0 {
+ reg = <0>;
+ cdns,num-lanes = <1>;
+ #phy-cells = <0>;
+ cdns,phy-type = <PHY_TYPE_USB3>;
+ resets = <&serdes_wiz0 1>;
+ };
+};
+
+&serdes_wiz0 {
+ status = "okay";
+};
+
+&serdes1 {
+ status = "okay";
+
+ serdes1_pcie_link: phy@0 {
+ reg = <0>;
+ cdns,num-lanes = <1>;
+ #phy-cells = <0>;
+ cdns,phy-type = <PHY_TYPE_PCIE>;
+ resets = <&serdes_wiz1 1>;
+ };
+};
+
+&serdes_wiz1 {
+ clocks = <&k3_clks 280 0>, <&k3_clks 280 1>, <&pcie_refclk0>;
+ status = "okay";
+};
+
+&usbss0 {
+ ti,vbus-divider;
+ status = "okay";
+};
+
+&usb0 {
+ dr_mode = "otg";
+ usb-role-switch;
+ maximum-speed = "high-speed";
+
+ port {
+ usb0_ep: endpoint {
+ remote-endpoint = <&usb0_con>;
+ };
+ };
+};
+
+&usbss1 {
+ ti,vbus-divider;
+ status = "okay";
+};
+
+&usb1 {
+ dr_mode = "host";
+ phys = <&serdes0_usb_link>;
+ phy-names = "cdns3,usb3-phy";
+ maximum-speed = "super-speed";
+};
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
From: sashiko-bot @ 2026-06-25 16:03 UTC (permalink / raw)
To: Shenwei Wang; +Cc: robh, conor+dt, devicetree, Frank.Li, imx
In-Reply-To: <20260625155432.815185-2-shenwei.wang@oss.nxp.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Missing endianness specification and implementation for multi-byte protocol fields.
- [High] The driver's direction_output callback causes hardware glitches by setting the direction before the initial value.
- [Medium] Inconsistent field sizes for REPLY and NOTIFICATION messages truncate the 'line' field.
- [Low] Incorrect command code in the SET_WAKEUP request diagram.
- [Low] Incorrect Linux error code usage (-ETIMEOUT).
- [Low] Grammatical error in SET_WAKEUP description.
--
commit 7712498eeadf5080c07364375a209ec4b75b63fd
Author: Shenwei Wang <shenwei.wang@nxp.com>
docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
This commit introduces the documentation for the GPIO RPMSG transport
protocol used over the RPMSG bus. It details the message formats,
command codes, and notification structures for communicating with
remote processors.
> diff --git a/Documentation/driver-api/gpio/gpio-rpmsg.rst b/Documentation/driver-api/gpio/gpio-rpmsg.rst
> new file mode 100644
> index 0000000000000..7d351ff0adb0e
> --- /dev/null
> +++ b/Documentation/driver-api/gpio/gpio-rpmsg.rst
> @@ -0,0 +1,271 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +GPIO RPMSG (Remote Processor Messaging) Protocol
> +================================================
[ ... ]
> +Message Format
> +--------------
> +
> +The RPMSG message consists of a 8-byte packet with the following layout:
> +
> +.. code-block:: none
> +
> + +------+------+------+------+------+------+------+------+
> + | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
> + | cmd | line | value |
> + +------+------+------+------+------+------+------+------+
[Severity: High]
Does this message format lack an explicit endianness specification for the
multi-byte fields (cmd, line, value)?
Without defining the endianness here, the driver implementation in
drivers/gpio/gpio-rpmsg.c:rpmsg_gpio_msg_prepare() assigns native integer types
directly:
msg->type = cmd;
msg->gpio = line;
msg->value = val;
Could this cause malformed packets when communicating between a big-endian host
and the remote processor?
> +
> +- **cmd**: Command code, used for GPIO_RPMSG_SEND messages.
[ ... ]
> +GPIO Commands
> +-------------
> +
> +Commands are specified in the **Cmd** field.
> +
> +The SEND message is always sent from Linux to the remote firmware. Each
> +SEND corresponds to a single REPLY message. The GPIO driver should
> +serialize messages and determine whether a REPLY message is required. If a
> +REPLY message is expected but not received within the specified timeout
> +period (currently 1 second in the Linux driver), the driver should return
> +-ETIMEOUT.
[Severity: Low]
Should this use the standard Linux kernel error code -ETIMEDOUT instead of
-ETIMEOUT?
> +
> +GET_DIRECTION (Cmd=2)
[ ... ]
> +SET_DIRECTION (Cmd=3)
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +**Request:**
> +
> +.. code-block:: none
> +
> + +------+------+------+------+------+------+------+------+
> + | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
> + | 3 | line | value |
> + +------+------+------+------+------+------+------+------+
> +
> +- **value**: Direction.
> +
> + - 0: None
> + - 1: Output
> + - 2: Input
[Severity: High]
Does this command lack a parameter to specify the initial output level,
requiring a separate SET_VALUE call?
In drivers/gpio/gpio-rpmsg.c:rpmsg_gpio_direction_output(), the driver sends
the SET_DIRECTION command before the SET_VALUE command:
rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
VIRTIO_GPIO_DIRECTION_OUT);
ret = rpmsg_gpio_send_message(port);
if (ret)
return ret;
rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
return rpmsg_gpio_send_message(port);
Could this sequence cause a hardware glitch by driving an unintended default
state on the physical GPIO line during the IPC transit time, before the
SET_VALUE command is processed?
> +
> +**Reply:**
[ ... ]
> +SET_WAKEUP (Cmd=16)
> +~~~~~~~~~~~~~~~~~~~
> +
> +**Request:**
> +
> +.. code-block:: none
> +
> + +------+------+------+------+------+------+------+------+
> + | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
> + | 1 | line | value |
> + +------+------+------+------+------+------+------+------+
[Severity: Low]
Does the command field in this diagram say '1' when it should say '16' or
'0x10' to match the SET_WAKEUP (Cmd=16) heading?
> +
> +- **value**: Wakeup enable.
> +
> + The remote system should always aim to stay in a power-efficient state by
> + shutting down or clock-gating the GPIO blocks that aren't in use. Since
> + the remoteproc driver is responsible for managing the power states of the
> + remote firmware, the GPIO driver does not require to know the firmware's
> + running states.
[Severity: Low]
Should "does not require to know" be rephrased to "does not need to know" for
better grammar?
> +
> + When the wakeup bit is set, the remote firmware should configure the line
[ ... ]
> +Notification Message
> +--------------------
> +
> +Notifications are sent by the remote core and they have
> +**Type=2 (GPIO_RPMSG_NOTIFY)**:
> +
> +When a GPIO line asserts an interrupt on the remote processor, the firmware
> +should immediately mask the corresponding interrupt source and send a
> +notification message to the Linux. Upon completion of the interrupt
> +handling on the Linux side, the driver should issue a
> +command **SET_IRQ_TYPE** to the firmware to unmask the interrupt.
> +
> +A Notification message can arrive between a SEND and its REPLY message,
> +and the driver is expected to handle this scenario.
> +
> +.. code-block:: none
> +
> + +------+------+--------+
> + | 0x00 | 0x01 | 0x02 |
> + | 2 | line | trigger|
> + +------+------+--------+
[Severity: Medium]
Does this notification message truncate the line index to 8 bits?
The SEND message diagram earlier explicitly defines an 8-byte layout with a
16-bit line field (occupying bytes 0x02 and 0x03). However, this NOTIFICATION
diagram and the driver's struct rpmsg_gpio_response in
drivers/gpio/gpio-rpmsg.c restrict the line index to an 8-bit __u8:
struct rpmsg_gpio_response {
...
__u8 line;
...
};
If an interrupt occurs on a line index greater than 255, could this cause the
driver to mishandle the IRQ due to truncation?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=1
^ permalink raw reply
* Re: [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver
From: sashiko-bot @ 2026-06-25 16:04 UTC (permalink / raw)
To: Shenwei Wang; +Cc: devicetree, conor+dt, Frank.Li, imx, robh
In-Reply-To: <20260625155432.815185-5-shenwei.wang@oss.nxp.com>
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [Critical] Writing to the `.name` field of the immutable `gpio_rpmsg_irq_chip` structure attempts to modify read-only memory, causing a kernel panic.
- [High] Host-endian integers are directly assigned to little-endian protocol fields, breaking communication on big-endian architectures.
- [High] An RPMSG timeout permanently desynchronizes the request/response state machine because the driver lacks transaction IDs to discard late replies.
- [High] The RPMSG callback blindly accesses the incoming message without validating that the payload length is sufficient.
- [High] The driver's GPIO callbacks can sleep, but it fails to set `gc->can_sleep = true`, incorrectly allowing them to be called from atomic contexts.
- [High] Interrupts will be permanently masked after firing once because the driver incorrectly relies on `irq_bus_sync_unlock` to send the unmask command during normal IRQ flow.
- [Medium] The driver leaks a reference to the `rproc` parent device node.
- [Medium] `rpmsg_gpio_direction_output()` configures the GPIO line as an output before setting the desired logical value, potentially causing a signal glitch.
--
commit e50fe4c0d14d5e1abdf1ce315d5d9511d01ee66e
Author: Shenwei Wang <shenwei.wang@nxp.com>
Subject: gpio: rpmsg: add generic rpmsg GPIO driver
This commit adds a generic GPIO driver to support controlling GPIO
controllers that reside on a remote processor. It translates standard
Linux GPIO calls into RPMSG protocol messages sent to the remote side.
> diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
> new file mode 100644
> index 0000000000000..332e2925a8309
> --- /dev/null
> +++ b/drivers/gpio/gpio-rpmsg.c
[ ... ]
> +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port)
> +{
> + int ret;
> +
> + reinit_completion(&port->cmd_complete);
> +
> + ret = rpmsg_send(port->rpdev->ept, port->send_msg, sizeof(*port->send_msg));
> + if (ret) {
> + dev_err(&port->rpdev->dev, "rpmsg_send failed: cmd=%d ret=%d\n",
> + port->send_msg->type, ret);
> + return ret;
> + }
> +
> + ret = wait_for_completion_timeout(&port->cmd_complete,
> + msecs_to_jiffies(RPMSG_TIMEOUT));
> + if (ret == 0) {
> + dev_err(&port->rpdev->dev, "rpmsg_send timeout! cmd=%d\n",
> + port->send_msg->type);
> + return -ETIMEDOUT;
[Severity: High]
If rpmsg_send times out, can this permanently desynchronize the request and
response state machine?
If a timeout occurs, rpmsg_gpio_send_message() returns -ETIMEDOUT but leaves
the channel active. Since the rpmsg_gpio_response doesn't appear to contain a
transaction ID, if the remote processor sends a delayed reply to this timed-out
command, will rpmsg_gpio_channel_callback() mistakenly accept it as the
completion for the next unrelated command sent by the host?
> + }
> +
> + if (unlikely(port->recv_msg->status != VIRTIO_GPIO_STATUS_OK)) {
> + dev_err(&port->rpdev->dev, "remote core replies an error: cmd=%d!\n",
> + port->send_msg->type);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static struct virtio_gpio_request *
> +rpmsg_gpio_msg_prepare(struct rpmsg_gpio_port *port, u16 line, u16 cmd, u32 val)
> +{
> + struct virtio_gpio_request *msg = port->send_msg;
> +
> + msg->type = cmd;
> + msg->gpio = line;
> + msg->value = val;
[Severity: High]
Are these values being assigned with the correct endianness?
The virtio_gpio_request structure defines these fields as __le16 and __le32,
but this assigns native host-endian integers to them directly. On big-endian
architectures, could this result in the remote processor misinterpreting the
commands?
> +
> + return msg;
> +}
[ ... ]
> +static int rpmsg_gpio_direction_output(struct gpio_chip *gc, unsigned int line, int val)
> +{
> + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> + int ret;
> +
> + guard(mutex)(&port->lock);
> +
> + rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
> + VIRTIO_GPIO_DIRECTION_OUT);
> +
> + ret = rpmsg_gpio_send_message(port);
> + if (ret)
> + return ret;
> +
> + rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
[Severity: Medium]
Could configuring the direction before setting the value cause a signal
glitch?
rpmsg_gpio_direction_output() sends VIRTIO_GPIO_MSG_SET_DIRECTION to the
remote processor before VIRTIO_GPIO_MSG_SET_VALUE. If the remote hardware
applies the direction immediately, could this drive the line to its default
or previous state for a short window before the intended new value is applied?
> +
> + return rpmsg_gpio_send_message(port);
> +}
[ ... ]
> +static void gpio_rpmsg_unmask_irq(struct irq_data *d)
> +{
> + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> + u32 line = d->hwirq;
> +
> + port->lines[line].irq_unmask = 1;
> +}
[Severity: High]
Will interrupts remain permanently masked on the remote processor after they
fire once?
gpio_rpmsg_unmask_irq() updates local memory but does not send an RPMSG
command to unmask the interrupt remotely. While gpio_rpmsg_irq_bus_sync_unlock()
does send the command, the core IRQ subsystem (like handle_level_irq) masks
and unmasks interrupts without invoking bus lock/unlock callbacks during normal
interrupt dispatch. Does this mean the unmask command is never actually sent
after an interrupt is handled?
[ ... ]
> +static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev,
> + struct device_node *np, const char *name)
> +{
> + struct rpmsg_gpio_port *port;
> + struct gpio_irq_chip *girq;
> + struct gpio_chip *gc;
> + int ret;
> +
> + port = devm_kzalloc(&rpdev->dev, sizeof(*port), GFP_KERNEL);
> + if (!port)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32(np, "reg", &port->idx);
> + if (ret)
> + return ret;
> +
> + ret = devm_mutex_init(&rpdev->dev, &port->lock);
> + if (ret)
> + return ret;
> +
> + ret = of_property_read_u32(np, "ngpios", &port->ngpios);
> + if (ret || port->ngpios > GPIOS_PER_PORT_DEFAULT)
> + port->ngpios = GPIOS_PER_PORT_DEFAULT;
> +
> + port->send_msg = devm_kzalloc(&rpdev->dev,
> + sizeof(*port->send_msg),
> + GFP_KERNEL);
> +
> + port->recv_msg = devm_kzalloc(&rpdev->dev,
> + sizeof(*port->recv_msg),
> + GFP_KERNEL);
> + if (!port->send_msg || !port->recv_msg)
> + return -ENOMEM;
> +
> + init_completion(&port->cmd_complete);
> + port->rpdev = rpdev;
> +
> + gc = &port->gc;
> + gc->owner = THIS_MODULE;
> + gc->parent = &rpdev->dev;
> + gc->fwnode = of_fwnode_handle(np);
> + gc->ngpio = port->ngpios;
> + gc->base = -1;
> + gc->label = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
> + name, port->idx);
[Severity: High]
Should gc->can_sleep be explicitly set to true for this gpio_chip?
The .set and .get callbacks in this driver acquire a mutex and wait for
completion, which will sleep. Without setting gc->can_sleep = true, the
gpiolib framework treats these GPIOs as atomic. If consumer drivers call
these GPIO operations from within an interrupt handler or a spinlock,
could this trigger a "scheduling while atomic" panic?
> +
> + gc->direction_input = rpmsg_gpio_direction_input;
> + gc->direction_output = rpmsg_gpio_direction_output;
> + gc->get_direction = rpmsg_gpio_get_direction;
> + gc->get = rpmsg_gpio_get;
> + gc->set = rpmsg_gpio_set;
> +
> + girq = &gc->irq;
> + gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
> + girq->parent_handler = NULL;
> + girq->num_parents = 0;
> + girq->parents = NULL;
> + girq->chip->name = devm_kstrdup(&rpdev->dev, gc->label, GFP_KERNEL);
[Severity: Critical]
Does this attempt to write to read-only memory?
gpio_irq_chip_set_chip() assigns the const structure gpio_rpmsg_irq_chip to
girq->chip. Modifying the .name field of a const structure placed in the
.rodata section will trigger a write-protection page fault and crash the
kernel. Should the irq_chip be dynamically allocated, or should the name be
set differently?
> +
> + dev_set_drvdata(&rpdev->dev, port);
> +
> + return devm_gpiochip_add_data(&rpdev->dev, gc, port);
> +}
[ ... ]
> +static struct device_node *
> +rpmsg_get_channel_ofnode(struct rpmsg_device *rpdev, const char *compat, u32 idx)
> +{
> + struct device_node *np_chan = NULL, *np;
> + struct rproc *rproc;
> +
> + rproc = rproc_get_by_child(&rpdev->dev);
> + if (!rproc)
> + return NULL;
> +
> + np = of_node_get(rproc->dev.of_node);
> + if (!np && rproc->dev.parent)
> + np = of_node_get(rproc->dev.parent->of_node);
> +
> + if (np)
> + np_chan = rpmsg_find_child_by_compat_reg(np, compat, idx);
> +
> + return np_chan;
[Severity: Medium]
Does this leak the reference to the parent device node?
rpmsg_get_channel_ofnode() acquires a reference to np using of_node_get(),
but it appears to return without calling of_node_put(np) to release it.
> +}
[ ... ]
> +static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
> + int len, void *priv, u32 src)
> +{
> + struct rpmsg_gpio_response *msg = data;
> + struct rpmsg_gpio_port *port = NULL;
> +
> + port = dev_get_drvdata(&rpdev->dev);
> +
> + if (!port) {
> + dev_err(&rpdev->dev, "port is null\n");
> + return -EINVAL;
> + }
> +
> + if (msg->type == GPIO_RPMSG_REPLY) {
[Severity: High]
Is there a risk of an out-of-bounds read here if the message is truncated?
rpmsg_gpio_channel_callback() receives a payload of size len, but blindly
dereferences the data pointer as struct rpmsg_gpio_response without
verifying that len >= sizeof(struct rpmsg_gpio_response). Could this read
out-of-bounds memory if the remote processor sends a malformed or small
message?
> + *port->recv_msg = *msg;
> + complete(&port->cmd_complete);
> + } else if (msg->type == GPIO_RPMSG_NOTIFY) {
> + generic_handle_domain_irq_safe(port->gc.irq.domain, msg->line);
> + } else {
> + dev_err(&rpdev->dev, "wrong message type (0x%x)\n", msg->type);
> + }
> +
> + return 0;
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=4
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: hwmon: chipcap2: Add label property
From: Flaviu Nistor @ 2026-06-25 16:04 UTC (permalink / raw)
To: Guenter Roeck, Javier Carrasco, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Shuah Khan
Cc: Flaviu Nistor, linux-hwmon, linux-kernel, devicetree, linux-doc
Add support for an optional label property similar to other hwmon devices.
This allows, in case of boards with multiple CHIPCAP2 sensors, to assign
distinct names to each instance.
Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
---
Changes in v2:
- Implement suggestion from Javier Carrasco as proposed by Krzysztof Kozlowski.
- Link to v1: https://lore.kernel.org/all/20260622122200.14245-1-flaviu.nistor@gmail.com/
.../devicetree/bindings/hwmon/amphenol,chipcap2.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
index 17351fdbefce..56b0cecfca5f 100644
--- a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
+++ b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
@@ -45,6 +45,8 @@ properties:
- const: low
- const: high
+ label: true
+
vdd-supply:
description:
Dedicated, controllable supply-regulator to reset the device and
@@ -55,6 +57,9 @@ required:
- reg
- vdd-supply
+allOf:
+ - $ref: hwmon-common.yaml#
+
additionalProperties: false
examples:
@@ -72,6 +77,7 @@ examples:
<5 IRQ_TYPE_EDGE_RISING>,
<6 IRQ_TYPE_EDGE_RISING>;
interrupt-names = "ready", "low", "high";
+ label = "Room";
vdd-supply = <®_vdd>;
};
};
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/2] hwmon: (chipcap2) Add support for label
From: Flaviu Nistor @ 2026-06-25 16:04 UTC (permalink / raw)
To: Guenter Roeck, Javier Carrasco, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Shuah Khan
Cc: Flaviu Nistor, linux-hwmon, linux-kernel, devicetree, linux-doc
In-Reply-To: <20260625160423.17882-1-flaviu.nistor@gmail.com>
Add support for label sysfs attribute similar to other hwmon devices.
This is particularly useful for systems with multiple sensors on the
same board, where identifying individual sensors is much easier since
labels can be defined via device tree.
Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
---
Changes in v2:
- No change for this patch in the patch series.
- Link to v1: https://lore.kernel.org/all/20260622122200.14245-1-flaviu.nistor@gmail.com/
Documentation/hwmon/chipcap2.rst | 2 ++
drivers/hwmon/chipcap2.c | 25 +++++++++++++++++++++++--
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/Documentation/hwmon/chipcap2.rst b/Documentation/hwmon/chipcap2.rst
index dc165becc64c..c38d87b91b69 100644
--- a/Documentation/hwmon/chipcap2.rst
+++ b/Documentation/hwmon/chipcap2.rst
@@ -70,4 +70,6 @@ humidity1_min_hyst: RW humidity low hystersis
humidity1_max_hyst: RW humidity high hystersis
humidity1_min_alarm: RO humidity low alarm indicator
humidity1_max_alarm: RO humidity high alarm indicator
+humidity1_label: RO descriptive name for the sensor
+temp1_label: RO descriptive name for the sensor
=============================== ======= ========================================
diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
index 4aecf463180f..086571d556b7 100644
--- a/drivers/hwmon/chipcap2.c
+++ b/drivers/hwmon/chipcap2.c
@@ -22,6 +22,8 @@
#include <linux/irq.h>
#include <linux/module.h>
#include <linux/regulator/consumer.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
#define CC2_START_CM 0xA0
#define CC2_START_NOM 0x80
@@ -83,6 +85,7 @@ struct cc2_data {
struct i2c_client *client;
struct regulator *regulator;
const char *name;
+ const char *label;
int irq_ready;
int irq_low;
int irq_high;
@@ -449,6 +452,8 @@ static umode_t cc2_is_visible(const void *data, enum hwmon_sensor_types type,
switch (attr) {
case hwmon_humidity_input:
return 0444;
+ case hwmon_humidity_label:
+ return cc2->label ? 0444 : 0;
case hwmon_humidity_min_alarm:
return cc2->rh_alarm.low_alarm_visible ? 0444 : 0;
case hwmon_humidity_max_alarm:
@@ -466,6 +471,8 @@ static umode_t cc2_is_visible(const void *data, enum hwmon_sensor_types type,
switch (attr) {
case hwmon_temp_input:
return 0444;
+ case hwmon_temp_label:
+ return cc2->label ? 0444 : 0;
default:
return 0;
}
@@ -552,6 +559,16 @@ static int cc2_humidity_max_alarm_status(struct cc2_data *data, long *val)
return 0;
}
+static int cc2_read_string(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, const char **str)
+{
+ struct cc2_data *data = dev_get_drvdata(dev);
+
+ *str = data->label;
+
+ return 0;
+}
+
static int cc2_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
int channel, long *val)
{
@@ -670,8 +687,9 @@ static int cc2_request_alarm_irqs(struct cc2_data *data, struct device *dev)
}
static const struct hwmon_channel_info *cc2_info[] = {
- HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
- HWMON_CHANNEL_INFO(humidity, HWMON_H_INPUT | HWMON_H_MIN | HWMON_H_MAX |
+ HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL),
+ HWMON_CHANNEL_INFO(humidity, HWMON_H_INPUT | HWMON_H_LABEL |
+ HWMON_H_MIN | HWMON_H_MAX |
HWMON_H_MIN_HYST | HWMON_H_MAX_HYST |
HWMON_H_MIN_ALARM | HWMON_H_MAX_ALARM),
NULL
@@ -680,6 +698,7 @@ static const struct hwmon_channel_info *cc2_info[] = {
static const struct hwmon_ops cc2_hwmon_ops = {
.is_visible = cc2_is_visible,
.read = cc2_read,
+ .read_string = cc2_read_string,
.write = cc2_write,
};
@@ -710,6 +729,8 @@ static int cc2_probe(struct i2c_client *client)
return dev_err_probe(dev, PTR_ERR(data->regulator),
"Failed to get regulator\n");
+ device_property_read_string(dev, "label", &data->label);
+
ret = cc2_request_ready_irq(data, dev);
if (ret)
return dev_err_probe(dev, ret, "Failed to request ready irq\n");
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v14 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
From: sashiko-bot @ 2026-06-25 16:06 UTC (permalink / raw)
To: Shenwei Wang; +Cc: devicetree, robh, Frank.Li, conor+dt, imx
In-Reply-To: <20260625155432.815185-6-shenwei.wang@oss.nxp.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `rpmsg-gpio` nodes are nested too deeply in the device tree for the `gpio-rpmsg` driver to discover them, causing an unconditional probe failure.
--
commit 9072997f8f4ba10a42ddca328c65744333555ac3
Author: Shenwei Wang <shenwei.wang@nxp.com>
arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
This patch adds an RPMSG bus node and its GPIO subnodes to the device tree
for the i.MX platform. It aims to enable remote device communication and
GPIO control via RPMSG.
> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index 1de3ad60c6aa7..f1b984eb1203a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> @@ -190,6 +190,31 @@ scmi_sensor: protocol@15 {
> cm33: remoteproc-cm33 {
> compatible = "fsl,imx8ulp-cm33";
> status = "disabled";
> +
> + rpmsg {
> + rpmsg-io {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rpmsg_gpioa: gpio@0 {
[Severity: Medium]
Does nesting these nodes cause the gpio-rpmsg driver to fail to probe?
Looking at rpmsg_find_child_by_compat_reg() in drivers/gpio/gpio-rpmsg.c,
the driver only searches the direct children of the remoteproc node:
for_each_available_child_of_node(parent, child) {
if (!of_device_is_compatible(child, compat))
continue;
Since for_each_available_child_of_node() is not recursive, the driver
fails to find the nested gpio@0 and gpio@1 nodes under rpmsg/rpmsg-io/.
This causes an -ENODEV return, preventing the driver from probing.
> + compatible = "rpmsg-gpio";
> + reg = <0>;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=5
^ permalink raw reply
* Re: [PATCH v14 3/5] rpmsg: core: match rpmsg device IDs by prefix
From: sashiko-bot @ 2026-06-25 16:06 UTC (permalink / raw)
To: Shenwei Wang; +Cc: imx, conor+dt, robh, Frank.Li, devicetree
In-Reply-To: <20260625155432.815185-4-shenwei.wang@oss.nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Forcing global prefix matching on all rpmsg drivers without a 'longest-prefix match' rule can cause unintended device binding and collisions.
- [Medium] Module auto-loading via udev is broken for dynamically suffixed devices because module alias generation remains exact and lacks a trailing wildcard.
--
commit 7cf465d01393f57a212484ddaf8a931c40ec4179
Author: Shenwei Wang <shenwei.wang@nxp.com>
rpmsg: core: match rpmsg device IDs by prefix
This commit updates the rpmsg device matching logic to compare only the
length of the id->name string. This allows id_table entries to act as
prefixes, enabling drivers to bind to dynamically suffixed devices.
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index e7f7831d37f89..f95bfc9965d4c 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -414,7 +414,9 @@ ATTRIBUTE_GROUPS(rpmsg_dev);
> static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
> const struct rpmsg_device_id *id)
> {
> - return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
> + size_t len = strnlen(id->name, RPMSG_NAME_SIZE);
> +
> + return strncmp(id->name, rpdev->id.name, len) == 0;
> }
[Severity: High]
Will this change cause unintended driver binding for existing drivers that
expect an exact match?
Because this modifies the global match behavior for all rpmsg drivers without
an opt-in flag or longest-prefix match logic, a shorter existing prefix could
hijack a new device.
For example, if a remote processor creates a channel named rpmsg-tty-control,
would it be incorrectly matched by an existing driver registered as rpmsg-tty?
Looking at rpmsg_dev_match(), the iteration returns on the first successful
match:
drivers/rpmsg/rpmsg_core.c:rpmsg_dev_match() {
...
for (i = 0; ids[i].name[0]; i++)
if (rpmsg_id_match(rpdev, &ids[i])) {
rpdev->id.driver_data = ids[i].driver_data;
return 1;
}
...
}
Could this result in a collision if multiple IDs share the same prefix?
[Severity: Medium]
Does this change break module auto-loading via udev for dynamically suffixed
devices?
When a dynamically suffixed device is registered, rpmsg_uevent() emits the
exact name:
drivers/rpmsg/rpmsg_core.c:rpmsg_uevent() {
...
return add_uevent_var(env, "MODALIAS=" RPMSG_DEVICE_MODALIAS_FMT,
rpdev->id.name);
}
However, the module alias generated by file2alias.c does not append a
wildcard for prefix matching:
scripts/mod/file2alias.c:do_rpmsg_entry() {
...
module_alias_printf(mod, false, RPMSG_DEVICE_MODALIAS_FMT, *name);
}
Since module_alias_printf() is called with append_wildcard set to false, the
generated alias will be an exact match (e.g., rpmsg:rpmsg-io-) which won't
match the uevent string (e.g., MODALIAS=rpmsg:rpmsg-io-1).
Will this prevent the module from loading automatically?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=3
^ 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