Devicetree
 help / color / mirror / Atom feed
* [PATCH 5/5] dt-bindings: dma: sun50i-a64-dma: Update device tree bindings documentation for A733
From: Yuanshen Cao @ 2026-06-19  4:53 UTC (permalink / raw)
  To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Maxime Ripard
  Cc: dmaengine, linux-arm-kernel, linux-sunxi, linux-kernel,
	devicetree, Yuanshen Cao
In-Reply-To: <20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com>

To complete the support for the A733 DMA controller, added
`allwinner,sun60i-a733-dma` to the list of compatible strings for
`allwinner,sun50i-a64-dma` dt-binding documentations..

Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
 Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
index c3e14eb6cfff..1cc3304b7414 100644
--- a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
+++ b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
@@ -25,6 +25,7 @@ properties:
           - allwinner,sun50i-a64-dma
           - allwinner,sun50i-a100-dma
           - allwinner,sun50i-h6-dma
+          - allwinner,sun60i-a733-dma
       - items:
           - const: allwinner,sun8i-r40-dma
           - const: allwinner,sun50i-a64-dma
@@ -70,6 +71,7 @@ if:
           - allwinner,sun20i-d1-dma
           - allwinner,sun50i-a100-dma
           - allwinner,sun50i-h6-dma
+          - allwinner,sun60i-a733-dma
 
 then:
   properties:

-- 
2.54.0


^ permalink raw reply related

* Re: [PATCH 2/5] dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths
From: sashiko-bot @ 2026-06-19  5:03 UTC (permalink / raw)
  To: Yuanshen Cao
  Cc: robh, vkoul, Frank.Li, devicetree, dmaengine, conor+dt,
	linux-sunxi
In-Reply-To: <20260619-sun60i-a733-dma-v1-2-da4b649fc72a@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-after-free vulnerability in the error handling paths of `sun6i_dma_prep_slave_sg` and `sun6i_dma_prep_dma_cyclic` when freeing DMA pool items.
--

commit ea99a7147f9100709dbaea854d96eccdbc89f235
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths

The A733 DMA controller supports higher address (up to 32G) compared to
previous generations. By moving set_addr into the sun6i_dma_config
structure, we can provide specialized implementations for different hardware.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index d92e702320d9f..059455425e19c 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
[ ... ]
> @@ -787,7 +796,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
>  				sg_dma_len(sg), flags);
>  
>  		} else {
> -			sun6i_dma_set_addr(sdev, v_lli,
> +			sdev->cfg->set_addr(sdev, v_lli,
>  					   sconfig->src_addr,
>  					   sg_dma_address(sg));
>  			v_lli->cfg = lli_cfg;

[Severity: High]
This isn't a bug introduced by this patch, but looking at the error handling
paths for sun6i_dma_prep_slave_sg() and sun6i_dma_prep_dma_cyclic(), is there
a use-after-free when freeing DMA pool items?

In the err_lli_free path, the code iterates over the linked list of items:

err_lli_free:
	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
		dma_pool_free(sdev->pool, v_lli, p_lli);

When dma_pool_free() executes, the memory associated with v_lli is returned
to the pool. The loop's update step then accesses v_lli->p_lli_next and
v_lli->v_lli_next to advance the pointers.

Under memory pressure, another thread might allocate this newly freed item
and overwrite its contents, potentially causing an invalid pointer
dereference. Should this loop use a safe iteration method that reads the
next pointers before calling dma_pool_free()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com?part=2

^ permalink raw reply

* Re: [PATCH 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping
From: sashiko-bot @ 2026-06-19  5:08 UTC (permalink / raw)
  To: Yuanshen Cao
  Cc: robh, conor+dt, devicetree, linux-sunxi, Frank.Li, dmaengine,
	vkoul
In-Reply-To: <20260619-sun60i-a733-dma-v1-3-da4b649fc72a@gmail.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Incorrect physical channel indexing in `sun6i_dma_interrupt` ignores the register index, applying status to the wrong physical channel.
- [High] Integer division truncation in the `sun6i_dma_interrupt` loop bound skips polling for trailing channels.
- [High] Time-of-Check to Time-of-Use (TOCTOU) race condition in `sun6i_dma_interrupt` risks a NULL pointer dereference.
--

commit 3bfcb64969bffa1792e80eaa282fc15450f3870b
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping

This commit introduces num_channels_per_reg to the sun6i_dma_config to
handle SoCs with varying channels per interrupt register. It modifies
the interrupt logic to use this new configuration value and bounds checking.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 059455425e19c..fb1c1a28744bb 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c

[ ... ]

> @@ -574,7 +575,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
>  	int i, j, ret = IRQ_NONE;
>  	u32 status;
>  
> -	for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
> +	for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {

[Severity: High]
This is a pre-existing issue, but does this loop condition truncate the
division and miss trailing channels?

If sdev->num_pchans is not perfectly divisible by num_channels_per_reg
(for example, 12 channels with 8 channels per register), the integer
division truncates the remainder. This means the loop will skip the second
register entirely, permanently missing interrupts for the trailing channels.

>  		status = sdev->cfg->read_irq_stat(sdev, i);
>  		if (!status)
>  			continue;
> @@ -584,7 +585,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
>  
>  		sdev->cfg->write_irq_stat(sdev, i, status);
>  
> -		for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
> +		for (j = 0; (j < sdev->cfg->num_channels_per_reg) && status; j++) {
>  			pchan = sdev->pchans + j;

[Severity: Critical]
This is a pre-existing issue, but does this apply the status to the wrong
physical channel?

The array lookup uses only the local offset j, discarding the register
index i. For any register i > 0, it accesses sdev->pchans[j] instead of
sdev->pchans[i * num_channels_per_reg + j].

With the newly added hardware where num_channels_per_reg is 1, j is always
0, which seems like it would cause all channels to incorrectly map to
channel 0.

>  			vchan = pchan->vchan;
>  			if (vchan && (status & vchan->irq_type)) {

[Severity: High]
This is a pre-existing issue, but is there a race condition here that could
lead to a NULL pointer dereference?

The interrupt handler reads vchan = pchan->vchan locklessly, and eventually
blocks to acquire vchan->vc.lock. Concurrently, sun6i_dma_terminate_all()
could acquire the same lock, nullify pchan->desc, and free the descriptors.

When the interrupt handler successfully acquires the lock:

spin_lock(&vchan->vc.lock);
vchan_cookie_complete(&pchan->desc->vd);

it does not re-verify pchan->desc before passing &pchan->desc->vd to
vchan_cookie_complete(), which would crash if pchan->desc was concurrently
set to NULL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com?part=3

^ permalink raw reply

* Re: [PATCH 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling
From: sashiko-bot @ 2026-06-19  5:13 UTC (permalink / raw)
  To: Yuanshen Cao
  Cc: dmaengine, linux-sunxi, robh, conor+dt, Frank.Li, devicetree,
	vkoul
In-Reply-To: <20260619-sun60i-a733-dma-v1-1-da4b649fc72a@gmail.com>

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Low] The parameter `chan_num` in the newly introduced `read_irq_en` and `write_irq_en` callbacks is highly misleading because it expects a register index, not a channel number.

Pre-existing issues:
- [Critical] `sun6i_dma_interrupt` incorrectly maps interrupts for higher channels to the wrong physical channel structs because it ignores the outer register index.
- [High] Integer division truncation in the `sun6i_dma_interrupt` loop bound completely ignores interrupts for higher channels on SoCs where the channel count is not a multiple of the channels per register.
- [High] `sun6i_dma_terminate_all` and `sun6i_dma_pause`/`resume` acquire `sdev->lock` without disabling softirqs, creating a deadlock with the DMA tasklet.
- [High] `sun6i_dma_interrupt` accesses `vchan->cyclic` and dereferences `pchan->desc` without holding `vc.lock`, racing with channel teardown.
--

commit 47f7d45b0543fc62a4eb37d06b46eeffe4dd06c6
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling

This patch refactors the sun6i-dma driver to support the Allwinner A733 DMA
controller. It moves interrupt handling and register dumping functions into
the sun6i_dma_config structure to handle different register layouts.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index a9a254dbf8cb9..d92e702320d9f 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -138,6 +138,11 @@ struct sun6i_dma_config {
>  	void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
>  	void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
>  	void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
> +	void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
> +	u32 (*read_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num);
> +	void (*write_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num, u32 irq_val);

[Severity: Low]
Is the chan_num parameter name misleading here?

Looking at sun6i_dma_start_desc(), the value passed to these callbacks is
actually the register index (irq_reg), not the physical channel number.

Could this cause confusion for future implementers who might try to use it
as a literal channel index?

[ ... ]

> @@ -549,14 +573,14 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
>  	u32 status;
>  
>  	for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {

[Severity: High]
This is a pre-existing issue, but does this loop bound properly handle
platforms where the channel count is not a multiple of the channels per
register?

Because integer division truncates, a platform like sun8i-h3 (which defines
nr_max_channels = 12) will evaluate 12 / 8 as 1.

Does this mean the loop only runs for i = 0 (handling channels 0-7) and
completely drops interrupts for channels 8-11, potentially leading to an
unhandled hardware interrupt storm? Should this use DIV_ROUND_UP?

> -		status = readl(sdev->base + DMA_IRQ_STAT(i));
> +		status = sdev->cfg->read_irq_stat(sdev, i);
>  		if (!status)
>  			continue;
>  
>  		dev_dbg(sdev->slave.dev, "DMA irq status %s: 0x%x\n",
>  			str_high_low(i), status);
>  
> -		writel(status, sdev->base + DMA_IRQ_STAT(i));
> +		sdev->cfg->write_irq_stat(sdev, i, status);
>  
>  		for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
>  			pchan = sdev->pchans + j;

[Severity: Critical]
This is a pre-existing issue, but does this correctly calculate the physical
channel pointer for higher channels?

Inside this interrupt loop, i represents the interrupt register index and j
is the local bit offset within that register. By calculating the physical
channel as sdev->pchans + j, it completely ignores the outer register index
offset.

If an interrupt triggers on physical channel 8 (i=1, j=0), won't the code
incorrectly assign it to sdev->pchans + 0?

Could this incorrectly complete the active transfer for channel 0, causing
use-after-free issues and DMA state corruption? Should the assignment be
sdev->pchans + j + i * sdev->cfg->num_channels_per_reg?


[Severity: High]
This is a pre-existing issue, but does sun6i_dma_interrupt() have a race
condition with channel teardown?

In the hardirq handler sun6i_dma_interrupt(), vchan->cyclic is checked
locklessly:

drivers/dma/sun6i-dma.c:sun6i_dma_interrupt() {
	...
	if (vchan->cyclic) {
		vchan_cyclic_callback(&pchan->desc->vd);
	}
	...
}

Concurrently, sun6i_dma_terminate_all() can run on another CPU, acquire
vchan->vc.lock, set vchan->cyclic = false, set pchan->desc = NULL, and free
the descriptors:

drivers/dma/sun6i-dma.c:sun6i_dma_terminate_all() {
	...
	vchan->cyclic = false;
	...
}

If the IRQ handler checks vchan->cyclic just before it is modified, could it
proceed to dereference pchan->desc after it has been nulled or freed?


[Severity: High]
This is a pre-existing issue, but do sun6i_dma_terminate_all(),
sun6i_dma_pause(), and sun6i_dma_resume() have a deadlock window?

These functions acquire sdev->lock using a plain spin_lock() from process
context:

drivers/dma/sun6i-dma.c:sun6i_dma_terminate_all() {
	...
	spin_lock(&sdev->lock);
	...
}

However, sdev->lock is also acquired by sun6i_dma_tasklet(), which runs in
softirq context.

If a hard interrupt occurs on the same CPU while this lock is held, and the
interrupt handler returns and processes pending softirqs (including the
scheduled sun6i_dma_tasklet()), won't the tasklet attempt to acquire
sdev->lock using spin_lock_irq(), resulting in a same-CPU ABBA deadlock?

Should process context use spin_lock_bh() or spin_lock_irqsave() when locking
resources shared with softirqs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com?part=1

^ permalink raw reply

* [PATCH] of: Fix RST inline emphasis warnings in of_map_id() kernel-doc
From: Vijayanand Jitta @ 2026-06-19  5:46 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan
  Cc: linux-arm-msm, devicetree, linux-kernel, kernel test robot,
	Vijayanand Jitta

The @filter_np parameter descriptions in of_map_id() and of_map_msi_id()
contained the text '*filter_np' in prose. Docutils interprets a leading
'*' as the start of RST emphasis (italic), but finds no closing '*',
triggering:

  Documentation/devicetree/kernel-api:11: ./drivers/of/base.c:2134:
  WARNING: Inline emphasis start-string without end-string. [docutils]

  Documentation/devicetree/kernel-api:11: ./drivers/of/base.c:2260:
  WARNING: Inline emphasis start-string without end-string. [docutils]

Fix by wrapping '*filter_np' in double backticks (*filter_np) to
render it as an RST inline code literal, which is also the correct
kernel-doc convention for pointer expressions.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202606130111.ldC96rqf-lkp@intel.com/
Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
---
 drivers/of/base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 7cb0d7e88247b..6e7a42dedad3f 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2160,7 +2160,7 @@ static bool of_check_bad_map(const __be32 *map, int len)
  * @map_mask_name: optional property name of the mask to use.
  * @filter_np: pointer to an optional filter node, or NULL to allow bypass.
  *	If non-NULL, the map property must exist (-ENODEV if absent). If
- *	*filter_np is also non-NULL, only entries targeting that node match.
+ *	``*filter_np`` is also non-NULL, only entries targeting that node match.
  * @arg: pointer to a &struct of_phandle_args for the result. On success,
  *	@arg->args_count will be set to the number of output specifier cells
  *	as defined by @cells_name in the target node, and
@@ -2350,7 +2350,7 @@ EXPORT_SYMBOL_GPL(of_map_iommu_id);
  *      stream/device ID) used as the lookup key in the msi-map table.
  * @filter_np: pointer to an optional filter node, or NULL to allow bypass.
  *	If non-NULL, the map property must exist (-ENODEV if absent). If
- *	*filter_np is also non-NULL, only entries targeting that node match.
+ *	``*filter_np`` is also non-NULL, only entries targeting that node match.
  * @arg: pointer to a &struct of_phandle_args for the result. On success,
  *	@arg->args_count will be set to the number of output specifier cells
  *	and @arg->args[0..args_count-1] will contain the translated output

---
base-commit: 598c7067dd8b65b93f3ccada47e9014a13137f1b
change-id: 20260619-iommu_map_kdoc_fix-906ad8a33f1d

Best regards,
--  
Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>


^ permalink raw reply related

* Re: [PATCH 1/2] dt-bindings: thermal: Add Qualcomm MBG thermal monitor support
From: Sachin Gupta @ 2026-06-19  5:59 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260601113009.38FC81F00893@smtp.kernel.org>



On 6/1/2026 5:00 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The PMIC schema restricts the standard 'temperature-sensor@' node name exclusively to the new MBG TM binding, causing false positive validation failures for standard-compliant legacy nodes.
> --
> 
> commit 1c6b45c4d32f87b04fb868a977041acc92be3854
> Author: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> 
> dt-bindings: thermal: Add Qualcomm MBG thermal monitor support
>      
> This commit adds device tree bindings for the Qualcomm Master Bandgap (MBG)
> temperature alarm peripheral found on the PM8775 PMIC. It introduces a new
> schema for the MBG thermal monitor and maps it in the SPMI PMIC bindings.
> 
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> @@ -193,6 +193,10 @@ patternProperties:
>>       type: object
>>       $ref: /schemas/thermal/qcom,spmi-temp-alarm.yaml#
>>   
>> +  "^temperature-sensor@[0-9a-f]+$":
>> +    type: object
>> +    $ref: /schemas/thermal/qcom-spmi-mbg-tm.yaml#
> 
> [Severity: Low]
> Does mapping this generic pattern exclusively to the MBG thermal monitor
> schema cause validation conflicts with other thermal sensors?
> 
> Since temperature-sensor is the standard generic node name, other thermal
> sensors under the PMIC (like qcom,spmi-temp-alarm) might use it. If a node
> uses temperature-sensor@a00 for a standard SPMI temp alarm, wouldn't it be
> evaluated solely against this MBG-specific schema and fail with a compatible
> string mismatch error?
> 
> Would it be better to structure this using a oneOf block to allow multiple
> applicable thermal bindings to share the standard node name?
> 

The reported case does not seem valid for this binding update.

temperature-sensor@... is intended for the MBG TM block 
(qcom-spmi-mbg-tm), while the existing SPMI temp-alarm block uses 
temp-alarm@... and its own binding (qcom,spmi-temp-alarm). So in 
practice they are distinct node names for distinct peripherals, and a 
normal temp-alarm node is
not expected to be described as temperature-sensor@....

Because of that, this should not cause a real conflict for standard PMIC 
DTS usage. A qcom,spmi-temp-alarm node named temperature-sensor@... 
would be a misnamed node rather than a valid legacy form we need to 
preserve.

Thanks,
Sachin

^ permalink raw reply

* Re: [PATCH 2/2] thermal: qcom: Add support for Qualcomm MBG thermal monitoring
From: Sachin Gupta @ 2026-06-19  6:00 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260601114035.2D9A81F00893@smtp.kernel.org>



On 6/1/2026 5:10 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
> - [High] Data race and TOCTOU window on `last_thres_crossed` leading to missed thermal violations.
> - [High] Potential NULL pointer dereference accessing `node->name` on non-OF platforms.
> - [High] Incorrect bitwise AND used to check a 4-bit fault status enumeration.
> - [High] Silent disablement of supported thermal trips.
> - [Medium] Hardware threshold interrupt is enabled before configuring the threshold value.
> - [Low] Incompatible pointer type passed to `regmap_read`.
> --
> 
> commit 59e6da95c48d5200129439570e254b1f9180acf4
> Author: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> Subject: thermal: qcom: Add support for Qualcomm MBG thermal monitoring
> 
> This patch adds a driver for the Qualcomm MBG thermal monitoring device.
> It monitors the die temperature and notifies the thermal framework when a
> level 1 upper threshold violation occurs via an SPMI interrupt.
> 
>> diff --git a/drivers/thermal/qcom/qcom-spmi-mbg-tm.c b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
>> new file mode 100644
>> index 0000000000000..60190b341fc7c
>> --- /dev/null
>> +++ b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
> 
> [ ... ]
> 
>> +static int mbg_tm_get_temp(struct thermal_zone_device *tz, int *temp)
>> +{
>> +	struct mbg_tm_chip *chip = thermal_zone_device_priv(tz);
>> +	int ret, milli_celsius;
>> +
>> +	if (chip->last_thres_crossed) {
>> +		dev_dbg(chip->dev, "last_temp: %d\n", chip->last_temp);
>> +		chip->last_thres_crossed = false;
>> +		*temp = chip->last_temp;
>> +		return 0;
>> +	}
> 
> [Severity: High]
> Could this lockless read and clear of chip->last_thres_crossed cause missed
> thermal violations?
> 
> If a user-space sysfs read occurs concurrently with the thermal worker
> execution (which is triggered by the ISR), the sysfs read might consume the
> flag by setting it to false.
> 
> When the thermal worker subsequently calls mbg_tm_get_temp(), it would see
> false and read the raw ADC value. Because ADC readings can fluctuate and
> lag, it might read a value below the threshold, entirely missing the
> violation.
> 

I will address this by adding proper locking for this state in the next 
patch series.

> [ ... ]
> 
>> +static int mbg_tm_set_trip_temp(struct thermal_zone_device *tz, int low_temp,
>> +				int temp)
>> +{
>> +	struct mbg_tm_chip *chip = thermal_zone_device_priv(tz);
>> +	int ret = 0;
>> +
>> +	guard(mutex)(&chip->lock);
>> +
>> +	/* The HW has a limitation that the trip set must be above 25C */
>> +	if (temp > MBG_MIN_TRIP_TEMP && temp < MBG_MAX_SUPPORTED_TEMP) {
>> +		ret = regmap_set_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
>> +				      MON2_UP_THRESH_EN);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		ret = regmap_write(chip->map, chip->base + MON2_LVL1_UP_THRESH,
>> +				   temp_to_vtemp_mv(temp));
> 
> [Severity: Medium]
> Is it safe to enable the threshold comparator before programming the new
> threshold value?
> 
> If the hardware retains a stale threshold from a prior state that is lower
> than the current temperature, enabling the comparator first could
> immediately trigger a spurious interrupt.
> 
> Would it be better to write the threshold value before enabling the
> interrupt?
> 

You are right that enabling the comparator before programming 
MON2_LVL1_UP_THRESH can allow a stale threshold to trigger a spurious 
interrupt. I will update the programming sequence to write the threshold 
first and enable the comparator afterward in the next patch series.

>> +		if (ret < 0)
>> +			return ret;
>> +	} else {
>> +		dev_dbg(chip->dev, "Set trip b/w 25C and 160C\n");
>> +		ret = regmap_clear_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
>> +					MON2_UP_THRESH_EN);
>> +		return ret;
>> +	}
> 
> [Severity: High]
> When the requested temperature falls outside the hardware's supported bounds,
> the driver disables the threshold interrupt and returns 0.
> 
> By returning 0, does the driver deceptively signal to the thermal core that
> the trip has been successfully configured?
> 
> Because the interrupt is disabled, the core will never be notified when the
> temperature violates the trip point. Should this return an error code like
> -ERANGE instead?
> 

I agree returning 0 here is not correct for an out-of-range request, 
since the trip is not actually configured. I will return -ERANGE for 
unsupported trip temperatures and update this in the next patch series.

> [ ... ]
> 
>> +static irqreturn_t mbg_tm_isr(int irq, void *data)
>> +{
>> +	struct mbg_tm_chip *chip = data;
>> +	int ret, val;
> 
> [Severity: Low]
> The regmap_read() function expects an unsigned int pointer for the value
> parameter, but val is declared as a signed int here.
> 

We can keep this as-is for now to stay consistent with the surrounding 
driver style, where int is commonly used for temporary readback 
variables with regmap_read(). Since the value is only used for bit 
checks and not sign-sensitive arithmetic, this does not cause a 
functional issue. I plan to keep it unchanged in this series for 
consistency.

>> +
>> +	scoped_guard(mutex, &chip->lock) {
>> +		ret = regmap_read(chip->map, chip->base + MBG_TEMP_MON2_FAULT_STATUS, &val);
>> +		if (ret < 0)
>> +			return IRQ_HANDLED;
>> +	}
>> +
>> +	if (FIELD_GET(MON_FAULT_STATUS_MASK, val) & MON_FAULT_LVL1_UPR) {
> 
> [Severity: High]
> Does this bitwise AND correctly check the 4-bit fault status enumeration?
> 
> MON_FAULT_LVL1_UPR is defined as 0x5 (binary 0101). A bitwise AND against
> 0x5 will evaluate to true if either bit 0 or bit 2 is set, matching
> unrelated fault states like 0x1, 0x4, or 0x7.
> 
> Should this use an exact equality check (==) instead?
> 

I’ll correct this to use == in the next patch series.

>> +		chip->last_thres_crossed = true;
> 
> [Severity: High]
> This sets chip->last_thres_crossed to true outside of the mutex. As
> mentioned earlier in mbg_tm_get_temp(), this flag is read and cleared
> locklessly.
> 
> Could this lead to a data race if mbg_tm_get_temp() executes concurrently?
> 

Agree updating last_thres_crossed outside the lock can race with 
concurrent reads in mbg_tm_get_temp(). I will move this update under the 
lock in the next patch series.

>> +		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
>> +		thermal_zone_device_update(chip->tz_dev, THERMAL_TRIP_VIOLATED);
>> +	} else {
>> +		dev_dbg(chip->dev, "Lvl1 upper threshold not violated, ignoring interrupt\n");
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int mbg_tm_probe(struct platform_device *pdev)
>> +{
>> +	struct mbg_tm_chip *chip;
>> +	struct device_node *node = pdev->dev.of_node;
> 
> [ ... ]
> 
>> +	chip->tz_dev = devm_thermal_of_zone_register(chip->dev, 0, chip, &mbg_tm_ops);
>> +	if (IS_ERR(chip->tz_dev))
>> +		return dev_err_probe(chip->dev, PTR_ERR(chip->tz_dev),
>> +				     "Failed to register sensor\n");
>> +
>> +	return devm_request_threaded_irq(&pdev->dev, chip->irq, NULL, mbg_tm_isr, IRQF_ONESHOT,
>> +					 node->name, chip);
>> +}
> 
> [Severity: High]
> Will node->name cause a NULL pointer dereference on non-OF platforms?
> 
> Because the driver uses device_property_read_u32(), it can be instantiated
> via ACPI or MFD without a device tree node, meaning pdev->dev.of_node
> could be NULL.
> 
> Would it be safer to use dev_name(&pdev->dev) instead?
> 

This driver is intended to support DT-only instantiation, so of_node is 
expected to be present for valid platforms. In that context, using 
node->name is consistent with the intended usage model.

Thanks,
Sachin Gupta


^ permalink raw reply

* [PATCH v6 0/2] Enable audio support for J721S2 EVM
From: Moteen Shah @ 2026-06-19  6:27 UTC (permalink / raw)
  To: krzk+dt, robh, conor+dt, nm, vigneshr, kristo
  Cc: devicetree, linux-arm-kernel, linux-kernel, u-kumar1,
	gehariprasath, y-abhilashchandra, m-shah, sen

Earlier version of the patchset sent upstream[0] was rejected as
its dependency[1], which resolves the DTBS check errors introduced
by [0] also got rejected on the grounds of ABI breakage.

Another solution to fix the DTBS check errors introduced by [0] is
to modify the ti,j721e-system-controller.yaml binding to allow
audio-refclk as clock-controller child. This is done in the first
patch of this series.

Changes since v5:
Link to v5:
https://lore.kernel.org/all/20260520115603.2662930-1-m-shah@ti.com/
- Move audio_refclk1 clock node from the overlay (k3-j721s2-evm-audio.dtso)
  to the main DTSI (k3-j721s2-main.dtsi) so it is always available
  independent of the overlay being applied
- Drop the &scm_conf block from the overlay since it is now in main.dtsi

Changes since v4:
Link to v4:
https://lore.kernel.org/all/20260519142341.2531948-1-m-shah@ti.com/
- Explicitly disable main_mcan3 and main_mcan5 as they will silently
  break mcasp audio routing

Link to v3:
https://lore.kernel.org/all/20260330094459.128648-1-m-shah@ti.com/
Link to v2:
https://lore.kernel.org/all/20260205130707.2033197-1-m-shah@ti.com/
Link to v1:
https://lore.kernel.org/all/20260112104536.83309-1-m-shah@ti.com/

Bootlogs:
https://gist.github.com/Jamm02/8ee551c2c8db3a58a9aa7976e049fa28

[0]: https://lore.kernel.org/linux-arm-kernel/20250604104656.38752-1-j-choudhary@ti.com/
[1]: https://lore.kernel.org/all/20250603095609.33569-4-j-choudhary@ti.com/

Jayesh Choudhary (1):
  arm64: dts: ti: Add audio overlay for k3-j721s2-evm

Moteen Shah (1):
  dt-bindings: ti: Update audio-refclk binding and j721e system
    controller

 .../bindings/clock/ti,am62-audio-refclk.yaml  |   8 +-
 .../soc/ti/ti,j721e-system-controller.yaml    |   6 +-
 arch/arm64/boot/dts/ti/Makefile               |   4 +
 .../boot/dts/ti/k3-j721s2-evm-audio.dtso      | 157 ++++++++++++++++++
 arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi    |   9 +
 5 files changed, 180 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm64/boot/dts/ti/k3-j721s2-evm-audio.dtso

-- 
2.34.1


^ permalink raw reply

* [PATCH v6 1/2] dt-bindings: ti: Update audio-refclk binding and j721e system controller
From: Moteen Shah @ 2026-06-19  6:27 UTC (permalink / raw)
  To: krzk+dt, robh, conor+dt, nm, vigneshr, kristo
  Cc: devicetree, linux-arm-kernel, linux-kernel, u-kumar1,
	gehariprasath, y-abhilashchandra, m-shah, sen
In-Reply-To: <20260619062749.1575066-1-m-shah@ti.com>

Add ti,j721s2-audio-refclk as a supported compatible string in the
ti,am62-audio-refclk binding. J721S2 uses the same audio reference
clock IP block first introduced on AM62. Per writing-bindings
guidelines, the J721S2-specific compatible is added as the primary
string with ti,am62-audio-refclk as the fallback.

Also extend the ti,j721e-system-controller clock-controller@ child
pattern to accept audio-refclk schemas alongside ehrpwm-tbclk via a
oneOf constraint, fixing the alphanumerical ordering of $refs.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Moteen Shah <m-shah@ti.com>
---
 .../devicetree/bindings/clock/ti,am62-audio-refclk.yaml   | 8 ++++++--
 .../bindings/soc/ti/ti,j721e-system-controller.yaml       | 6 ++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/ti,am62-audio-refclk.yaml b/Documentation/devicetree/bindings/clock/ti,am62-audio-refclk.yaml
index b2e40bd39a3af..6c8fb0793070d 100644
--- a/Documentation/devicetree/bindings/clock/ti,am62-audio-refclk.yaml
+++ b/Documentation/devicetree/bindings/clock/ti,am62-audio-refclk.yaml
@@ -11,8 +11,12 @@ maintainers:
 
 properties:
   compatible:
-    items:
-      - const: ti,am62-audio-refclk
+    oneOf:
+      - items:
+          - const: ti,am62-audio-refclk
+      - items:
+          - const: ti,j721s2-audio-refclk
+          - const: ti,am62-audio-refclk
 
   reg:
     maxItems: 1
diff --git a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
index f3bd0be3b279f..364be49f8c7c7 100644
--- a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
+++ b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
@@ -53,9 +53,11 @@ patternProperties:
 
   "^clock-controller@[0-9a-f]+$":
     type: object
-    $ref: /schemas/clock/ti,am654-ehrpwm-tbclk.yaml#
+    oneOf:
+      - $ref: /schemas/clock/ti,am62-audio-refclk.yaml#
+      - $ref: /schemas/clock/ti,am654-ehrpwm-tbclk.yaml#
     description:
-      Clock provider for TI EHRPWM nodes.
+      Clock provider for TI EHRPWM or Audio Reference Clock nodes.
 
   "phy@[0-9a-f]+$":
     type: object
-- 
2.34.1


^ permalink raw reply related

* [PATCH v6 2/2] arm64: dts: ti: Add audio overlay for k3-j721s2-evm
From: Moteen Shah @ 2026-06-19  6:27 UTC (permalink / raw)
  To: krzk+dt, robh, conor+dt, nm, vigneshr, kristo
  Cc: devicetree, linux-arm-kernel, linux-kernel, u-kumar1,
	gehariprasath, y-abhilashchandra, m-shah, sen
In-Reply-To: <20260619062749.1575066-1-m-shah@ti.com>

From: Jayesh Choudhary <j-choudhary@ti.com>

Add device tree overlay to enable analog audio support on J721S2-EVM
using PCM3168A codec connected to McASP4 serializers.

- Add audio_refclk1 clock node to k3-j721s2-main.dtsi
- Add nodes for sound-card, audio codec, I2C3 and McASP4
- Add pinmux for I2C3, McASP4, AUDIO_EXT_REFCLK1 and WKUP_GPIO_0
- Add GPIO expander (TCA6408) for codec control
- Add GPIO hogs to route I2C3 lines and McASP serializers
- Set idle-state to 0 in mux0 and mux1 for McASP signal routing

Reviewed-by: Hari Prasath Gujulan Elango <gehariprasath@ti.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Co-developed-by: Moteen Shah <m-shah@ti.com>
Signed-off-by: Moteen Shah <m-shah@ti.com>
---
 arch/arm64/boot/dts/ti/Makefile               |   4 +
 .../boot/dts/ti/k3-j721s2-evm-audio.dtso      | 157 ++++++++++++++++++
 arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi    |   9 +
 3 files changed, 170 insertions(+)
 create mode 100644 arch/arm64/boot/dts/ti/k3-j721s2-evm-audio.dtso

diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index 371f9a043fe52..d9824e17085f4 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -180,6 +180,7 @@ dtb-$(CONFIG_ARCH_K3) += k3-j721s2-common-proc-board.dtb
 dtb-$(CONFIG_ARCH_K3) += k3-j721s2-evm-gesi-exp-board.dtbo
 k3-j721s2-evm-dtbs := k3-j721s2-common-proc-board.dtb k3-j721s2-evm-gesi-exp-board.dtbo
 dtb-$(CONFIG_ARCH_K3) += k3-j721s2-evm.dtb
+dtb-$(CONFIG_ARCH_K3) += k3-j721s2-evm-audio.dtbo
 dtb-$(CONFIG_ARCH_K3) += k3-j721s2-evm-pcie1-ep.dtbo
 dtb-$(CONFIG_ARCH_K3) += k3-j721s2-evm-usb0-type-a.dtbo
 
@@ -320,6 +321,8 @@ k3-j721e-evm-pcie1-ep-dtbs := k3-j721e-common-proc-board.dtb \
 	k3-j721e-evm-pcie1-ep.dtbo
 k3-j721e-sk-csi2-dual-imx219-dtbs := k3-j721e-sk.dtb \
 	k3-j721e-sk-csi2-dual-imx219.dtbo
+k3-j721s2-evm-audio-dtbs := k3-j721s2-common-proc-board.dtb \
+	k3-j721s2-evm-audio.dtbo
 k3-j721s2-evm-pcie1-ep-dtbs := k3-j721s2-common-proc-board.dtb \
 	k3-j721s2-evm-pcie1-ep.dtbo
 k3-j721s2-evm-usb0-type-a-dtbs := k3-j721s2-common-proc-board.dtb \
@@ -396,6 +399,7 @@ dtb- += k3-am625-beagleplay-csi2-ov5640.dtb \
 	k3-j721e-evm-pcie0-ep.dtb \
 	k3-j721e-evm-pcie1-ep.dtb \
 	k3-j721e-sk-csi2-dual-imx219.dtb \
+	k3-j721s2-evm-audio.dtb \
 	k3-j721s2-evm-pcie1-ep.dtb \
 	k3-j721s2-evm-usb0-type-a.dtb \
 	k3-j722s-evm-csi2-quad-rpi-cam-imx219.dtb \
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-evm-audio.dtso b/arch/arm64/boot/dts/ti/k3-j721s2-evm-audio.dtso
new file mode 100644
index 0000000000000..ac5a827e1b750
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-evm-audio.dtso
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Device Tree Overlay for J721S2 Audio Support
+ *
+ * Copyright (C) 2026 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+
+#include "k3-pinctrl.h"
+
+&{/} {
+	codec_audio: sound {
+		compatible = "ti,j7200-cpb-audio";
+		model = "j721s2-cpb";
+
+		ti,cpb-mcasp = <&mcasp4>;
+		ti,cpb-codec = <&pcm3168a_1>;
+
+		clocks = <&k3_clks 213 0>, <&k3_clks 213 1>,
+			 <&k3_clks 157 299>, <&k3_clks 157 328>;
+		clock-names = "cpb-mcasp-auxclk", "cpb-mcasp-auxclk-48000",
+			      "cpb-codec-scki", "cpb-codec-scki-48000";
+	};
+
+	i2c_mux: mux-controller-2 {
+		compatible = "gpio-mux";
+		#mux-state-cells = <1>;
+		mux-gpios = <&wkup_gpio0 54 GPIO_ACTIVE_HIGH>;
+		idle-state = <1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&main_i2c3_mux_pins_default>;
+	};
+};
+
+&main_pmx0 {
+	mcasp4_pins_default: mcasp4-default-pins {
+		pinctrl-single,pins = <
+			J721S2_IOPAD(0x0c8, PIN_OUTPUT_PULLDOWN, 1) /* (AD28) MCASP4_ACLKX */
+			J721S2_IOPAD(0x06c, PIN_OUTPUT_PULLDOWN, 1) /* (V26) MCASP4_AFSX */
+			J721S2_IOPAD(0x068, PIN_INPUT_PULLDOWN, 1) /* (U28) MCASP4_AXR1 */
+			J721S2_IOPAD(0x0c4, PIN_OUTPUT_PULLDOWN, 1) /* (AB26) MCASP4_AXR2 */
+			J721S2_IOPAD(0x070, PIN_OUTPUT_PULLDOWN, 1) /* (R27) MCASP4_AXR3 */
+		>;
+	};
+
+	audio_ext_refclk1_pins_default: audio-ext-refclk1-default-pins {
+		pinctrl-single,pins = <
+			J721S2_IOPAD(0x078, PIN_OUTPUT, 1) /* (Y25) MCAN2_RX.AUDIO_EXT_REFCLK1 */
+		>;
+	};
+};
+
+&wkup_pmx2 {
+	main_i2c3_mux_pins_default: main-i2c3-mux-default-pins {
+		pinctrl-single,pins = <
+			J721S2_WKUP_IOPAD(0x038, PIN_OUTPUT, 7) /* (B27) WKUP_GPIO0_54 */
+		>;
+	};
+};
+
+&exp2 {
+	p09-hog {
+		/* P09 - MCASP/TRACE_MUX_S0 */
+		gpio-hog;
+		gpios = <9 GPIO_ACTIVE_HIGH>;
+		output-low;
+		line-name = "MCASP/TRACE_MUX_S0";
+	};
+
+	p10-hog {
+		/* P10 - MCASP/TRACE_MUX_S1 */
+		gpio-hog;
+		gpios = <10 GPIO_ACTIVE_HIGH>;
+		output-high;
+		line-name = "MCASP/TRACE_MUX_S1";
+	};
+};
+
+&mux0 {
+	idle-state = <0>;
+};
+
+&mux1 {
+	idle-state = <0>;
+};
+
+&main_mcan3 {
+	/* Conflicts with McASP4 signal routing via mux0 */
+	status = "disabled";
+};
+
+&main_mcan5 {
+	/* Conflicts with McASP4 signal routing via mux1 */
+	status = "disabled";
+};
+
+&k3_clks {
+	/* Configure AUDIO_EXT_REFCLK1 pin as output */
+	pinctrl-names = "default";
+	pinctrl-0 = <&audio_ext_refclk1_pins_default>;
+};
+
+&main_i2c3 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&main_i2c3_pins_default>;
+	clock-frequency = <400000>;
+	mux-states = <&i2c_mux 1>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	exp3: gpio@20 {
+		compatible = "ti,tca6408";
+		reg = <0x20>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		gpio-line-names = "CODEC_RSTZ", "CODEC_SPARE1",
+				  "UB926_RESETN", "UB926_LOCK",
+				  "UB926_PWR_SW_CNTRL", "UB926_TUNER_RESET",
+				  "UB926_GPIO_SPARE";
+	};
+
+	pcm3168a_1: audio-codec@44 {
+		compatible = "ti,pcm3168a";
+		reg = <0x44>;
+		#sound-dai-cells = <1>;
+		reset-gpios = <&exp3 0 GPIO_ACTIVE_LOW>;
+		clocks = <&audio_refclk1>;
+		clock-names = "scki";
+		VDD1-supply = <&vsys_3v3>;
+		VDD2-supply = <&vsys_3v3>;
+		VCCAD1-supply = <&vsys_5v0>;
+		VCCAD2-supply = <&vsys_5v0>;
+		VCCDA1-supply = <&vsys_5v0>;
+		VCCDA2-supply = <&vsys_5v0>;
+	};
+};
+
+&mcasp4 {
+	status = "okay";
+	#sound-dai-cells = <0>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&mcasp4_pins_default>;
+	op-mode = <0>;          /* MCASP_IIS_MODE */
+	tdm-slots = <2>;
+	auxclk-fs-ratio = <256>;
+	serial-dir = <	/* 0: INACTIVE, 1: TX, 2: RX */
+		0 2 1 1
+		0 0 0 0
+		0 0 0 0
+		0 0 0 0
+	>;
+};
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
index 1228ac5711bf0..36a01a06dd254 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
@@ -74,6 +74,15 @@ ehrpwm_tbclk: clock-controller@140 {
 			reg = <0x140 0x18>;
 			#clock-cells = <1>;
 		};
+
+		audio_refclk1: clock-controller@42e4 {
+			compatible = "ti,j721s2-audio-refclk", "ti,am62-audio-refclk";
+			reg = <0x42e4 0x4>;
+			clocks = <&k3_clks 157 299>;
+			assigned-clocks = <&k3_clks 157 299>;
+			assigned-clock-parents = <&k3_clks 157 328>;
+			#clock-cells = <0>;
+		};
 	};
 
 	main_ehrpwm0: pwm@3000000 {
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393
From: Andy Shevchenko @ 2026-06-19  6:40 UTC (permalink / raw)
  To: Nikhil Gautam
  Cc: linux-iio, jic23, dlechner, nuno.sa, andy, robh, krzk+dt,
	conor+dt, devicetree, linux-kernel
In-Reply-To: <98a08d17-5b70-4f4d-bf52-fe1073fde2b6@gmail.com>

On Fri, Jun 19, 2026 at 02:36:00AM +0530, Nikhil Gautam wrote:
> On 19-06-2026 12:56 am, Andy Shevchenko wrote:
> > On Thu, Jun 18, 2026 at 09:31:41PM +0530, Nikhil Gautam wrote:
> 
> Thank you very much for taking the time to do such a thorough review of this
> patch series.
> 
> Your detailed comments and suggestions are very helpful.
> I'll address the issues you've pointed out, update the cover letter to
> better explain the design decisions,
> and incorporate the requested coding style and API changes in the next
> revision.
> 
> I appreciate your review and feedback.

You're welcome!

...

> > > +config MLX90393
> > > +	tristate "MELEXIS MLX90393 3-axis magnetometer sensor"
> > > +	depends on I2C
> > Why not a regmap?
> The MLX90393 uses both register-based and command-based transactions.
> Since regmap does not naturally model the command-based interface,
> using it would require workarounds such as virtual registers or bypass
> paths.
> A custom transport abstraction is therefore simpler and better suited for
> this device.
> 
> I already discussed this on thread, Link :
> https://lore.kernel.org/linux-iio/20260423121834.4244-1-nikhilgtr@gmail.com/

Right, but please, put a summary in the commit message as it's  important
detail of implementation choice.

...

> > > +	for (unsigned int i = 0; i < MLX90393_OSR_MAX;  i++)
> > > +		if (mlx90393_osr_avail[i] == val) {
> > > +			*osr = i;
> > > +			return 0;
> > > +		}
> > Missing {}.
> Agreed, removed intentionally as single statement, will add as per
> guidelines all the places
> where needed

It's also better to have them when it might be ambiguous. And also when even
a single statement takes a few lines. TL;DR: Also apply a common sense.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: thermal: Add Qualcomm MBG thermal monitor support
From: Sachin Gupta @ 2026-06-19  6:44 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	Stephen Boyd, Jishnu Prakash, Kamal Wadhwa, Amit Kucheria,
	Thara Gopinath, linux-arm-msm, devicetree, linux-kernel, linux-pm,
	Satya Priya Kakitapalli, Ajit Pandey, Imran Shaik, Taniya Das,
	Jagadeesh Kona
In-Reply-To: <20260608-sweet-powerful-ibis-8b2adb@quoll>



On 6/8/2026 3:37 PM, Krzysztof Kozlowski wrote:
> On Mon, Jun 01, 2026 at 04:31:18PM +0530, Sachin Gupta wrote:
>> From: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>>
>> Add bindings for the Qualcomm MBG (Master Bandgap) temperature alarm peripheral
> 
> Feels unwrapped.
> 
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
> 

I will re-wrap the commit message in the next revision.

> 
>> found on the PM8775 PMIC. Unlike the existing SPMI temp alarm peripheral,
>> the MBG peripheral supports both hot and cold threshold monitoring across
>> two programmable levels (LVL1 and LVL2), with interrupt status reported via
>> a fault status register over SPMI.
>>
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> Co-developed-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
>> Signed-off-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
>> ---
>>   .../devicetree/bindings/mfd/qcom,spmi-pmic.yaml    |  4 ++
>>   .../bindings/thermal/qcom-spmi-mbg-tm.yaml         | 72 ++++++++++++++++++++++
>>   2 files changed, 76 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> index 644c42b5e2e5..5f409fe700b2 100644
>> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> @@ -193,6 +193,10 @@ patternProperties:
>>       type: object
>>       $ref: /schemas/thermal/qcom,spmi-temp-alarm.yaml#
>>   
>> +  "^temperature-sensor@[0-9a-f]+$":
>> +    type: object
>> +    $ref: /schemas/thermal/qcom-spmi-mbg-tm.yaml#
>> +
>>     "^typec@[0-9a-f]+$":
>>       type: object
>>       $ref: /schemas/usb/qcom,pmic-typec.yaml#
>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.yaml
>> new file mode 100644
>> index 000000000000..a0ecc9f35cf6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.yaml
> 
> Filename must match compatible.
> 

Thanks for the review. I can rename this to match the compatible naming, 
but wanted your preference on scope:

Should I use a generic naming scheme (qcom,spmi-mbg-tm.yaml with 
matching compatible), or make it PMIC-specific (qcom,pm8775-mbg-tm.yaml).

Thanks,
Sachin

>> @@ -0,0 +1,72 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/thermal/qcom-spmi-mbg-tm.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm's SPMI PMIC MBG Thermal Monitoring
> 
> Best regards,
> Krzysztof
> 


^ permalink raw reply

* Re: [PATCH 2/2] thermal: qcom: Add support for Qualcomm MBG thermal monitoring
From: Sachin Gupta @ 2026-06-19  6:45 UTC (permalink / raw)
  To: Konrad Dybcio, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
	Lukasz Luba, Stephen Boyd, Jishnu Prakash, Kamal Wadhwa,
	Amit Kucheria, Thara Gopinath
  Cc: linux-arm-msm, devicetree, linux-kernel, linux-pm,
	Satya Priya Kakitapalli, Ajit Pandey, Imran Shaik, Taniya Das,
	Jagadeesh Kona
In-Reply-To: <7478c540-a5fc-4238-bba0-5b04547f57c7@oss.qualcomm.com>



On 6/16/2026 3:40 PM, Konrad Dybcio wrote:
> On 6/1/26 1:01 PM, Sachin Gupta wrote:
>> From: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>>
>> Add driver for the Qualcomm MBG thermal monitoring device. It monitors
>> the die temperature, and when there is a level 1 upper threshold
>> violation, it receives an interrupt over spmi. The driver reads
>> the fault status register and notifies thermal accordingly.
>>
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> Co-developed-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
>> Signed-off-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +static const struct mbg_map_table map_table[] = {
>> +	/* minT	vtemp0	tc */
> 
> The struct is defined 2 lines above, the reader can tell the names
> of the fields

I will remove this in the next patch series.

>> +	{ -60000, 4337, 1967 },
>> +	{ -40000, 4731, 1964 },
>> +	{ -20000, 5124, 1957 },
>> +	{ 0,      5515, 1949 },
>> +	{ 20000,  5905, 1940 },
>> +	{ 40000,  6293, 1930 },
>> +	{ 60000,  6679, 1921 },
>> +	{ 80000,  7064, 1910 },
>> +	{ 100000, 7446, 1896 },
>> +	{ 120000, 7825, 1878 },
>> +	{ 140000, 8201, 1859 },
>> +};
> 
> Please add a comment stating this map is not PMIC-specific
> 

This table is PMIC-specific and applies only to the PM8775 MBG block.
MBG is used only on PM8775 PMIC.

> [...]
> 
>> +	/* The HW has a limitation that the trip set must be above 25C */
>> +	if (temp > MBG_MIN_TRIP_TEMP && temp < MBG_MAX_SUPPORTED_TEMP) {
>> +		ret = regmap_set_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
>> +				      MON2_UP_THRESH_EN);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		ret = regmap_write(chip->map, chip->base + MON2_LVL1_UP_THRESH,
>> +				   temp_to_vtemp_mv(temp));
>> +		if (ret < 0)
>> +			return ret;
>> +	} else {
>> +		dev_dbg(chip->dev, "Set trip b/w 25C and 160C\n");
> 
> Should this be an error print, returning an error condition?
> 

Yes, this should be treated as an error path. For out-of-range trip 
requests, I will return -ERANGE and update the log to an error-level 
message in the next patch series.

>> +		ret = regmap_clear_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
>> +					MON2_UP_THRESH_EN);
>> +		return ret;
>> +	}
>> +
>> +	/*
>> +	 * Configure the last_temp one degree higher, to ensure the
>> +	 * violated temp is returned to thermal framework when it reads
>> +	 * temperature for the first time after the violation happens.
>> +	 * This is needed to account for the inaccuracy in the conversion
>> +	 * formula used which leads to the thermal framework setting back
>> +	 * the same thresholds in case the temperature it reads does not
>> +	 * show violation.
>> +	 */
>> +	chip->last_temp = temp + MBG_TEMP_CONSTANT;
> 
> Will this work fine if the user tries to set the max temp supported
> by the hardware (i.e. is there headroom for max+1)?
> 

In the current implementation, temp == MBG_MAX_SUPPORTED_TEMP is not 
accepted (temp < MBG_MAX_SUPPORTED_TEMP), so the last_temp = temp + 
MBG_TEMP_CONSTANT path is never taken at absolute max. For accepted 
trips (strictly below max), there is headroom for the +1C adjustment.

>> +
>> +	return ret;
>> +}
>> +
>> +static const struct thermal_zone_device_ops mbg_tm_ops = {
>> +	.get_temp = mbg_tm_get_temp,
>> +	.set_trips = mbg_tm_set_trip_temp,
>> +};
>> +
>> +static irqreturn_t mbg_tm_isr(int irq, void *data)
>> +{
>> +	struct mbg_tm_chip *chip = data;
>> +	int ret, val;
>> +
>> +	scoped_guard(mutex, &chip->lock) {
>> +		ret = regmap_read(chip->map, chip->base + MBG_TEMP_MON2_FAULT_STATUS, &val);
>> +		if (ret < 0)
>> +			return IRQ_HANDLED;
>> +	}
>> +
>> +	if (FIELD_GET(MON_FAULT_STATUS_MASK, val) & MON_FAULT_LVL1_UPR) {
>> +		chip->last_thres_crossed = true;
>> +		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
>> +		thermal_zone_device_update(chip->tz_dev, THERMAL_TRIP_VIOLATED);
> 
> Should the assignment and this call also be guarded by the mutex?
> 
> Konrad

Yes, agreed. I will update this locking in the next patch series.

Thanks,
Sachin

^ permalink raw reply

* Re: [PATCH 4/5] dmaengine: sun6i-dma: Implement support for Allwinner A733 DMA controller
From: sashiko-bot @ 2026-06-19  7:12 UTC (permalink / raw)
  To: Yuanshen Cao
  Cc: linux-sunxi, conor+dt, devicetree, vkoul, dmaengine, robh,
	Frank.Li
In-Reply-To: <20260619-sun60i-a733-dma-v1-4-da4b649fc72a@gmail.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The DMA interrupt handler incorrectly resolves the physical channel (`pchan`) from the interrupt status bit by ignoring the outer loop register index `i`.
- [High] Integer division truncation in loop bounds causes upper hardware channels to be completely ignored, and introduces out-of-bounds MMIO accesses.
--

commit e328a2b422946f5a9b0337a1058273c7077c1ac4
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Implement support for Allwinner A733 DMA controller

This patch implements the actual support for the Allwinner A733 DMA
controller. It defines the new register offsets and bitfield mappings
required for the A733, which slightly differs from the older sun6i
series.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index fb1c1a28744bb..9585b4a9e00de 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c

[ ... ]

> @@ -1352,6 +1412,33 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
>  	.has_mbus_clk = true,
>  };
>  
> +/*
> + * The A733 binding uses the number of dma channels from the
> + * device tree node.
> + */
> +static struct sun6i_dma_config sun60i_a733_dma_cfg = {
> +	.clock_autogate_enable = sun6i_enable_clock_autogate_h3,
> +	.set_burst_length = sun6i_set_burst_length_h3,
> +	.set_drq          = sun6i_set_drq_h6,
> +	.set_mode         = sun6i_set_mode_h6,
> +	.set_addr         = sun6i_dma_set_addr_a733,
> +	.dump_com_regs    = sun6i_dma_dump_com_regs_a733,
> +	.read_irq_en      = sun6i_read_irq_en_a733,
> +	.write_irq_en     = sun6i_write_irq_en_a733,
> +	.read_irq_stat    = sun6i_read_irq_stat_a733,
> +	.write_irq_stat   = sun6i_write_irq_stat_a733,
> +	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
> +	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
> +	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> +			     BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> +			     BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
> +	.dst_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> +			     BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> +			     BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
> +	.num_channels_per_reg = DMA_IRQ_CHAN_NR_A733,

[Severity: Critical]
This is a pre-existing issue, but does setting num_channels_per_reg to 1
(DMA_IRQ_CHAN_NR_A733) break interrupt handling for all A733 channels > 0?

In sun6i_dma_interrupt(), the physical channel is calculated without the outer
loop index i:

    for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
        ...
        for (j = 0; (j < sdev->cfg->num_channels_per_reg) && status; j++) {
            pchan = sdev->pchans + j;

Since num_channels_per_reg is 1, j is always 0. As a result, every interrupt
on every channel will incorrectly resolve to sdev->pchans[0]. Doesn't this
completely break DMA functionality for all A733 channels except channel 0 by
causing spurious completions on channel 0 and hanging all others?

[Severity: High]
This is a pre-existing issue, but does using integer division truncation for
loop bounds ignore remainder channels and cause out-of-bounds MMIO accesses?

In sun6i_dma_interrupt(), the loop bound is:

    i < sdev->num_pchans / sdev->cfg->num_channels_per_reg

For SoCs where the number of channels is not a perfect multiple of
num_channels_per_reg (like H3 with 12 channels and 8 channels per register),
this division truncates (12 / 8 = 1), causing the remainder channels to be
entirely ignored so their interrupts are never serviced.

Additionally, this patch series introduces a similar loop in
sun6i_kill_tasklet():

    for (i = 0; i < DMA_MAX_CHANNELS / sdev->cfg->num_channels_per_reg; i++)

Since DMA_MAX_CHANNELS is 16 and num_channels_per_reg is 1 for A733, the loop
evaluates to 16. If the A733 hardware possesses fewer than 16 channels, won't
this cause out-of-bounds MMIO writes which can trigger bus aborts?

> +	.has_mbus_clk = true,
> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com?part=4

^ permalink raw reply

* RE: [PATCH v2 01/10] dt-bindings: reset: renesas,rzg2l-usbphy-ctrl: Document RZ/G3L support
From: Biju Das @ 2026-06-19  7:17 UTC (permalink / raw)
  To: Conor Dooley, biju.das.au
  Cc: Philipp Zabel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, magnus.damm, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	Prabhakar Mahadev Lad
In-Reply-To: <20260616-velcro-collar-3e764c6851da@spud>

Hi Conor,

Thanks for feedback.

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: 16 June 2026 16:45
> Subject: Re: [PATCH v2 01/10] dt-bindings: reset: renesas,rzg2l-usbphy-ctrl: Document RZ/G3L support
> 
> On Tue, Jun 16, 2026 at 11:44:43AM +0100, Biju wrote:
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Document Renesas RZ/G3L (r9a08g046) USB PHY controller bindings.
> > The RZ/G3L USB PHY block is similar to RZ/G3S but differs in that each
> > port has its own OTG controller, whereas RZ/G3S only has one on port 1.
> > To reflect this, RZ/G3L uses a regulators sub-node with per-port vbus0
> > and vbus1 entries instead of the single regulator-vbus property used
> > by other compatible SoCs.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v1->v2:
> >  * Updated commit description.
> >  * Added enum instead of const in the compatible section.
> >  * Dropped regulator1-vbus and added a regulators group node.
> >  * Updated schema check.
> > ---
> >  .../reset/renesas,rzg2l-usbphy-ctrl.yaml      | 49 +++++++++++++++++--
> >  1 file changed, 46 insertions(+), 3 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/reset/renesas,rzg2l-usbphy-ctrl.ya
> > ml
> > b/Documentation/devicetree/bindings/reset/renesas,rzg2l-usbphy-ctrl.ya
> > ml index c83469a1b379..12da48d069e5 100644
> > ---
> > a/Documentation/devicetree/bindings/reset/renesas,rzg2l-usbphy-ctrl.ya
> > ml
> > +++ b/Documentation/devicetree/bindings/reset/renesas,rzg2l-usbphy-ctr
> > +++ l.yaml
> > @@ -16,13 +16,17 @@ description:
> >  properties:
> >    compatible:
> >      oneOf:
> > +      - items:
> > +          - enum:
> > +              - renesas,r9a08g045-usbphy-ctrl # RZ/G3S
> > +              - renesas,r9a08g046-usbphy-ctrl # RZ/G3L
> 
> Looks fine, other than the fact that the "- items: - enum:" construct is the same as just having "-
> enum".
> pw-bot: changes-requested
> 
> Fix that problem, and

OK, will fix this in next version.

Cheers,
Biju

> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> 
> Cheers,
> Conor.

^ permalink raw reply

* Re: [PATCH v4 2/2] clk: amlogic: Add A9 AO clock controller driver
From: Jerome Brunet @ 2026-06-19  7:29 UTC (permalink / raw)
  To: Julian Braha
  Cc: jian.hu, Neil Armstrong, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao,
	Kevin Hilman, Martin Blumenstingl, linux-amlogic, linux-clk,
	devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <79b1a519-5723-4e0c-904c-b7fdf9564ee1@gmail.com>

On jeu. 18 juin 2026 at 19:56, Julian Braha <julianbraha@gmail.com> wrote:

> Hi Jian,
>
> On 6/18/26 10:49, Jian Hu via B4 Relay wrote:
>
>> +config COMMON_CLK_A9_AO
>> +	tristate "Amlogic A9 SoC AO clock controller support"
>> +	depends on ARM64 || COMPILE_TEST
>> +	default ARCH_MESON
>> +	select COMMON_CLK_MESON_REGMAP
>> +	select COMMON_CLK_MESON_CLKC_UTILS
>> +	select COMMON_CLK_MESON_DUALDIV
>
> Selecting COMMON_CLK_MESON_REGMAP is unnecessary since you're already
> selecting COMMON_CLK_MESON_DUALDIV here.

No, regmap clock are directly used so this is necessary.
Relying on other module dependencies is not enough

>
> - Julian Braha

-- 
Jerome

^ permalink raw reply

* Re: [PATCH v4 2/2] clk: amlogic: Add A9 AO clock controller driver
From: Jerome Brunet @ 2026-06-19  7:34 UTC (permalink / raw)
  To: sashiko-bot
  Cc: Jian Hu via B4 Relay, sashiko-reviews, linux-amlogic,
	neil.armstrong, robh, devicetree, conor+dt
In-Reply-To: <20260618100341.0A42F1F00A3A@smtp.kernel.org>

On jeu. 18 juin 2026 at 10:03, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] The AO PWM clocks are registered without an `ao_` or `a9_ao_`
> prefix, which will cause global namespace collisions with the main clock
> controller.
> - [Low] Typographical error in the clock name string literal for the RTC
> dualdiv input gate: `ao_rtc_duandiv_in` instead of `ao_rtc_dualdiv_in`.
> - [Low] Missing `#include <linux/module.h>` for `MODULE_*` macros.
> --
>
> commit ef5998ff087b6faed09d9eed4f59078d2b2f0a60
> Author: Jian Hu <jian.hu@amlogic.com>
>
> clk: amlogic: Add A9 AO clock controller driver
>     
> Add the Always-on clock controller driver for the Amlogic A9 SoC family.
>
>> diff --git a/drivers/clk/meson/a9-aoclk.c b/drivers/clk/meson/a9-aoclk.c
>> new file mode 100644
>> index 0000000000000..88aa8cf3f5d00
>> --- /dev/null
>> +++ b/drivers/clk/meson/a9-aoclk.c
>> @@ -0,0 +1,488 @@
>> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
>> +/*
>> + * Copyright (C) 2026 Amlogic, Inc. All rights reserved
>> + */
>> +
>> +#include <dt-bindings/clock/amlogic,a9-aoclkc.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>
> [Severity: Low]
> Should this file explicitly include <linux/module.h>?
>
> The file relies on several module macros at the end (such as MODULE_DESCRIPTION,
> MODULE_AUTHOR, and MODULE_LICENSE). Although it currently compiles successfully
> due to transitive includes from <linux/platform_device.h>, standard kernel
> practice is to include the header explicitly.

Yes please fix this

>
> [ ... ]
>
>> +static const struct clk_parent_data a9_ao_pwm_parents[] = {
>> +	{ .hw = &a9_ao_xtal.hw },
>> +	{ .fw_name = "fdiv5", },
>> +	{ .fw_name = "fdiv4", },
>> +	{ .fw_name = "fdiv3", }
>> +};
>> +
>> +static A9_COMP_SEL(pwm_a, AO_PWM_CLK_A_CTRL, 9, 0x7, a9_ao_pwm_parents);
>
> [Severity: Medium]
> Will this cause a clock name collision with the main clock controller?
>
> Because the A9_COMP_SEL macro uses the #_name stringification operator inside
> MESON_COMP_SEL, this will generate the clock name as exactly "pwm_a_sel"
> (and similarly "pwm_a_div" and "pwm_a" for the others), without any "ao_" or
> "a9_ao_" prefix.

I think AI got it wrong and missed the prefix addition. Unless I missed
something my self, I think you are fine here

>
> If the main clock controller conventionally registers a "pwm_a" clock, failing
> to prefix the Always-On clocks will lead to global namespace collisions in the
> Common Clock Framework and debugfs.
>
>> +static A9_COMP_DIV(pwm_a, AO_PWM_CLK_A_CTRL, 0, 8);
>> +static A9_COMP_GATE(pwm_a, AO_PWM_CLK_A_CTRL, 8);
>> +
>> +static A9_COMP_SEL(pwm_b, AO_PWM_CLK_B_CTRL, 9, 0x7, a9_ao_pwm_parents);
>
> [ ... ]
>
>> +static struct clk_regmap a9_ao_rtc_dualdiv_in = {
>> +	.data = &(struct clk_regmap_gate_data){
>> +		.offset = AO_RTC_BY_OSCIN_CTRL0,
>> +		.bit_idx = 31,
>> +	},
>> +	.hw.init = &(struct clk_init_data) {
>> +		.name = "ao_rtc_duandiv_in",
>
> [Severity: Low]
> This isn't a bug, but there's a typo in the clock name literal. Should
> "ao_rtc_duandiv_in" be "ao_rtc_dualdiv_in"?

Yes

>
>> +		.ops = &clk_regmap_gate_ops,
>> +		.parent_hws = (const struct clk_hw *[]) {
>> +			&a9_ao_xtal.hw
>> +		},

-- 
Jerome

^ permalink raw reply

* Re: [PATCH 0/8] Add PCIe M.2 Key E connector support for NXP i.MX boards
From: Bartosz Golaszewski @ 2026-06-19  7:37 UTC (permalink / raw)
  To: Sherry Sun (OSS)
  Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
	linux-bluetooth, linux-pm, sherry.sun, robh, krzk+dt, conor+dt,
	Frank.Li, s.hauer, kernel, festevam, amitkumar.karwar,
	neeraj.sanjaykale, marcel, luiz.dentz, hongxing.zhu, l.stach,
	lpieralisi, kwilczynski, mani, bhelgaas, brgl
In-Reply-To: <20260618101047.4185497-1-sherry.sun@oss.nxp.com>

On Thu, 18 Jun 2026 12:10:39 +0200, "Sherry Sun (OSS)"
<sherry.sun@oss.nxp.com> said:
> From: Sherry Sun <sherry.sun@nxp.com>
>
> This series adds support for NXP Wi-Fi/BT combo chips (88W9098, AW693)
> inserted into PCIe M.2 Key E connectors on several i.MX EVK/MEK boards.
>
> For M.2 cards that rely on PCIe L2 link state and wake-up mechanisms, the
> card must remain powered during suspend. Patch 1 uses the existing
> dw_pcie_rp::skip_pwrctrl_off flag to skip power-off during suspend and skip
> power-on during the init path.
>
> Alsp the btnxpuart driver is extended to obtain a pwrseq descriptor via the
> OF graph on the UART controller device in patch 2.
>
> Sherry Sun (8):
>   PCI: imx6: Add skip_pwrctrl_off flag support
>   power: sequencing: pcie-m2: Add PCI ID for NXP 88W9098 and AW693
>     Bluetooth

Can this be applied independently without build-time issues?

Bart

>   Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
>   arm64: dts: imx8mq-evk: Describe the PCIe M.2 Key E connector
>   arm64: dts: imx95-19x19-evk: Describe the PCIe M.2 Key E connector
>   arm64: dts: imx8dxl-evk: Describe the PCIe M.2 Key E connector
>   arm64: dts: imx8qm-mek: Describe the PCIe M.2 Key E connector
>   arm64: dts: imx8qxp-mek: Describe the PCIe M.2 Key E connector
>
>  arch/arm64/boot/dts/freescale/imx8dxl-evk.dts | 56 +++++++++++++-----
>  arch/arm64/boot/dts/freescale/imx8mq-evk.dts  | 44 ++++++++++++--
>  arch/arm64/boot/dts/freescale/imx8qm-mek.dts  | 58 ++++++++++++++-----
>  arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 54 ++++++++++++-----
>  .../boot/dts/freescale/imx95-19x19-evk.dts    | 55 +++++++++++++-----
>  drivers/bluetooth/btnxpuart.c                 | 33 ++++++++++-
>  drivers/pci/controller/dwc/pci-imx6.c         | 36 +++++++-----
>  drivers/power/sequencing/pwrseq-pcie-m2.c     |  4 ++
>  8 files changed, 264 insertions(+), 76 deletions(-)
>
> --
> 2.50.1
>
>

^ permalink raw reply

* Re: [PATCH v4 0/2] clk: amlogic: Add A9 AO clock controller
From: Jerome Brunet @ 2026-06-19  7:40 UTC (permalink / raw)
  To: Jian Hu via B4 Relay
  Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao, Kevin Hilman,
	Martin Blumenstingl, jian.hu, linux-amlogic, linux-clk,
	devicetree, linux-kernel, linux-arm-kernel, Conor Dooley
In-Reply-To: <20260618-a9_aoclk-v4-0-569d0425e50c@amlogic.com>

On jeu. 18 juin 2026 at 17:49, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@kernel.org> wrote:

> This series adds Amlogic A9 AO clock support, including dt-binding and AO clock driver.
>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>

A part from the 2 minor problems found by sashiko, This looks good.

> ---
> Changes in v4:
> - Drop CLK_IS_CRITICAL for ao_xtal_in clock.
> - Drop CLK_HW_INIT* and revert to explicit clock declarations.
> - Link to v3: https://lore.kernel.org/r/20260610-a9_aoclk-v3-0-b7592d6c31e2@amlogic.com
>
> Changes in v3:
> - Move COMPILE_TEST after 'depends on ARM64' reported by sashiko-bot.
> - Rename i2c3 to i3c reported by sashiko-bot.
> - Reword the comment describing ao_xtal_in's flags.
> - Use struct clk_init_data to describe ao_xtal_in's hw.init.
> - Link to v2: https://lore.kernel.org/r/20260603-a9_aoclk-v2-0-f47ea616ee78@amlogic.com
>
> Changes in v2:
> - Split the A9 clock driver and send the AO clock separately.
> - Rename aobus to soc.
> - Use CLK_HW_INIT_FW_NAME to describe clk_init_data.
> - Use CLK_HW_INIT_PARENTS_DATA to describe clk_init_data.
> - Use a9_ao prefix for MESON_COMP_SEL.
> - Correct duandiv name.
> - Fix pwm b reg.
> - Link to v1: https://lore.kernel.org/all/20260511-b4-a9_clk-v1-0-41cb4071b7c9@amlogic.com/
>
> ---
> Jian Hu (2):
>       dt-bindings: clock: Add Amlogic A9 AO clock controller
>       clk: amlogic: Add A9 AO clock controller driver
>
>  .../bindings/clock/amlogic,a9-aoclkc.yaml          |  76 ++++
>  drivers/clk/meson/Kconfig                          |  13 +
>  drivers/clk/meson/Makefile                         |   1 +
>  drivers/clk/meson/a9-aoclk.c                       | 488 +++++++++++++++++++++
>  include/dt-bindings/clock/amlogic,a9-aoclkc.h      |  76 ++++
>  5 files changed, 654 insertions(+)
> ---
> base-commit: ca89c88bcf69daca829044c638a8163d5ce47af0
> change-id: 20260603-a9_aoclk-bbf531badc63
>
> Best regards,

-- 
Jerome

^ permalink raw reply

* Re: [PATCH v6 06/16] iio: core: create local __iio_chan_prefix_emit() for reuse
From: Rodrigo Alencar @ 2026-06-19  7:43 UTC (permalink / raw)
  To: Andy Shevchenko, Rodrigo Alencar
  Cc: Nuno Sá, rodrigo.alencar, linux-iio, devicetree,
	linux-kernel, linux-doc, linux-hardening, Lars-Peter Clausen,
	Michael Hennerich, Jonathan Cameron, David Lechner,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
	Gustavo A. R. Silva
In-Reply-To: <ajQ1bZSNHQ96pyJx@ashevche-desk.local>

On 18/06/26 21:14, Andy Shevchenko wrote:
> On Thu, Jun 18, 2026 at 05:14:19PM +0100, Rodrigo Alencar wrote:
> > On 18/06/26 16:06, Nuno Sá wrote:
> > > On Thu, Jun 18, 2026 at 02:27:22PM +0100, Rodrigo Alencar via B4 Relay wrote:
> 
> ...
> 
> > > > +	dev_attr->attr.name = kasprintf(GFP_KERNEL, "%s%s", prefix, postfix);
> > > > +	if (!dev_attr->attr.name)
> > > >  		return -ENOMEM;
> > > 
> > > I don't oppose the change. Looks like a nice cleanup.
> 
> May I oppose it? I found use scnprintf() is harder to follow in comparison to
> nice kasprintf() that takes care for the dynamically allocated buffer.

In the next patch the function is reused in a sysfs attribute read handler,
a context wich would not be nice to have dynamic allocation. vscnprintf() is
the main building block of sysfs_emit() which limits the buffer length to
a page size, so I used scnprintf() trying not to deviate much from that. 

kasprintf() it is still used in the caller, where the logic was a bit confusing
as it tried to avoid multiple allocations.
 
> Also there is a chance to get a name silently cut due to insufficient space.
> Besides that this function can't be used (again due to 'c') in kasprintf()-like
> wrapper. I do not consider this as a good approach. Have you looked at seq_buf
> instead?

NAME_MAX is not the maximum length a filename can have? I suppose there should be
enough space for the channel-prefix. Indeed, seq_buf can be used and it cleans up
things a bit as it tracks the the position in the buffer.

> 
> > > But bear in mind this very sensible as any subtle mistake means ABI breakage.
> 
> Which immediately raises a question of test coverage. Do we have one? If not,
> this code must be accompanied with one.

Agreed. Will see to have tests for v7.

> > Yes! I tried to be careful... this is dangerous stuff!

-- 
Kind regards,

Rodrigo Alencar

^ permalink raw reply

* Re: [PATCH v7 2/2] drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels
From: Neil Armstrong @ 2026-06-19  7:47 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, devicetree, linux-kernel,
	linux-renesas-soc, Dmitry Baryshkov, KancyJoe
In-Reply-To: <CAD=FV=W5FgXGznH1+SA74WOfAUXaAggaJi=ds8udqZ69K3i=Ow@mail.gmail.com>

On 6/18/26 19:00, Doug Anderson wrote:
> Hi,
> 
> On Fri, Jun 5, 2026 at 7:51 AM Neil Armstrong <neil.armstrong@linaro.org> wrote:
>>
>> From: KancyJoe <kancy2333@outlook.com>
>>
>> Implement support for the Renesas 63419 based dual-DSI video mode
>> Display Panels found in the Ayaneo gaming handled devices.
>>
>> Signed-off-by: KancyJoe <kancy2333@outlook.com>
> 
> I notice "Kancy Joe" has a space in the source files, but not in the
> signoff. I guess Signed-off-by isn't necessarily required to be real
> names these days, but still seems odd...

Yeah I kept all the signoff and names as in the the source files,
it wasn't an issue for other patches, so I left it....

> 
> 
>> +/*
>> + * Helper to switch between DSI links, so we share a single dsi_ctx
>> + * for both links, so in case of an error all writes & sleep for
>> + * both links are ignored.
>> + */
>> +static inline void dsi_link_switch(struct renesas_r63419_panel *ctx,
>> +                                  struct mipi_dsi_multi_context *dsi_ctx,
>> +                                  unsigned int link)
>> +{
>> +       dsi_ctx->dsi = ctx->dsi[link];
>> +}
>> +
>> +static int renesas_r63419_on(struct renesas_r63419_panel *ctx)
>> +{
>> +       struct mipi_dsi_multi_context dsi_ctx = { 0 };
>> +
>> +       /* Panel registers are loaded from DDIC Non Volatile Memory */
>> +
>> +       dsi_link_switch(ctx, &dsi_ctx, 0);
>> +       mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>> +       dsi_link_switch(ctx, &dsi_ctx, 1);
>> +       mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> 
> Instead of dsi_link_switch(), can't you use the mipi_dsi_dual()
> function? I think it would be:
> 
> mipi_dsi_dual(mipi_dsi_dcs_exit_sleep_mode_multi, dsi_ctx,
> ctx->dsi[0], ctx->dsi[1]);

Right, indeed will switch

> 
> 
>> +static int renesas_r63419_disable(struct drm_panel *panel)
>> +{
>> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
>> +       struct mipi_dsi_multi_context dsi_ctx = { 0 };
>> +
>> +       dsi_link_switch(ctx, &dsi_ctx, 0);
>> +       mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>> +       dsi_link_switch(ctx, &dsi_ctx, 1);
>> +       mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>> +       mipi_dsi_msleep(&dsi_ctx, 50);
>> +
>> +       dsi_link_switch(ctx, &dsi_ctx, 0);
>> +       mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>> +       dsi_link_switch(ctx, &dsi_ctx, 1);
>> +       mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>> +       mipi_dsi_msleep(&dsi_ctx, 120);
>> +
>> +       return dsi_ctx.accum_err;
> 
> I'm not sure we've been terribly consistent, but should the above be
> "return 0"? I'm not actually sure there's any benefit to a panel's
> disable() function returning an error to begin with.
> drm_panel_disable() doesn't return an error, so all this does is skip
> setting "panel->enabled" to false and make it harder for the system to
> recover.

Yep you're right, there's no sense to return the error here.

> 
> 
>> +static int renesas_r63419_prepare(struct drm_panel *panel)
>> +{
>> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
>> +       int ret;
>> +
>> +       ret = regulator_bulk_enable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
>> +                                   ctx->vdd_supplies);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       usleep_range(1000, 2000);
>> +
>> +       ret = regulator_bulk_enable(ARRAY_SIZE(renesas_r63419_vcc_supplies),
>> +                                   ctx->vcc_supplies);
>> +       if (ret < 0) {
>> +               regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
>> +                                      ctx->vdd_supplies);
>> +               return ret;
>> +       }
>> +
>> +       usleep_range(1000, 2000);
>> +
>> +       gpiod_set_value_cansleep(ctx->reset_gpio, 0);
>> +
>> +       usleep_range(3000, 4000);
>> +
>> +       ret = renesas_r63419_on(ctx);
>> +       if (ret < 0) {
>> +               dev_err(panel->dev, "Failed to initialize panel: %d\n", ret);
>> +
>> +               /* Power off sequence from the r63419 datasheet */
>> +               regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vcc_supplies),
>> +                                      ctx->vcc_supplies);
>> +               regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
>> +                                      ctx->vdd_supplies);
>> +
>> +               gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> 
> To make de-init opposite to init, shouldn't the reset come before you
> turn the regulators off? Depending on the design of the panel, I'd
> imagine this could prevent back-powering some logic?
> 
> I'd also expect vdd supplies to be turned off first?

I did follow the DDIC spec here, and yeah Sashiko already
pointed it to me but reset needs to be switched to low _after_
the vdd supplies goes low. Perhaps indeed the vcc one should be
turned off after the reset. I'll do that.

> 
> 
>> +static int renesas_r63419_unprepare(struct drm_panel *panel)
>> +{
>> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
>> +
>> +       /* Power off sequence from the r63419 datasheet */
>> +       regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vcc_supplies), ctx->vcc_supplies);
>> +       regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies), ctx->vdd_supplies);
>> +
>> +       gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> 
> Similar: shouldn't the reset come before the regulators to make
> power-off the opposite of init.

Yep I'll move vcc disable after the reset to match the spec more closely.

> 
> 
>> +static int renesas_r63419_get_modes(struct drm_panel *panel,
>> +                                   struct drm_connector *connector)
>> +{
>> +       struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
>> +       const struct drm_display_mode *mode = ctx->desc->mode;
>> +
>> +       drm_connector_set_panel_orientation(connector, ctx->orientation);
> 
> IIRC, the above was a workaround that caused a warning splat. Is your
> panel used on a system that actually needs it? Could your DRM driver
> be fixed rather than persisting this hack? For context, see commit
> 47bef230225b ("drm/panel: panel-edp: Implement .get_orientation
> callback")

No need, I'll drop it

> 
> 
>> +static int renesas_r63419_probe(struct mipi_dsi_device *dsi)
>> +{
>> +       struct mipi_dsi_device_info info = { };
>> +       struct device *dev = &dsi->dev;
>> +       struct renesas_r63419_panel *ctx;
>> +       struct device_node *dsi1_node;
>> +       struct mipi_dsi_host *dsi1_host;
>> +       int ret, i;
>> +
>> +       ctx = devm_drm_panel_alloc(dev, struct renesas_r63419_panel, panel,
>> +                                  &renesas_r63419_panel_funcs, DRM_MODE_CONNECTOR_DSI);
>> +       if (IS_ERR(ctx))
>> +               return PTR_ERR(ctx);
>> +
>> +       ctx->desc = of_device_get_match_data(dev);
>> +       if (!ctx->desc)
>> +               return dev_err_probe(dev, -ENODEV,
>> +                                    "Failed to get panel description\n");
>> +
>> +       ret = devm_regulator_bulk_get_const(&dsi->dev,
>> +                                           ARRAY_SIZE(renesas_r63419_vdd_supplies),
>> +                                           renesas_r63419_vdd_supplies, &ctx->vdd_supplies);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       ret = devm_regulator_bulk_get_const(&dsi->dev,
>> +                                           ARRAY_SIZE(renesas_r63419_vcc_supplies),
>> +                                           renesas_r63419_vcc_supplies, &ctx->vcc_supplies);
>> +       if (ret < 0)
>> +               return ret;
> 
> It seems like both sets of supplies are always enabled / disabled
> together with no delay between them. Do you truly need two lists, or
> can this be combined to one list of regulators. That would simplify a
> bunch of logic.

You need a delay between enabling vcc and vdd according to the spec,
this is why I did a split.

> 
> 
>> +       ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>> +       if (IS_ERR(ctx->reset_gpio))
>> +               return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
>> +                                    "Failed to get reset gpio\n");
>> +
>> +       /* Get second DSI host */
>> +       dsi1_node = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
>> +       if (!dsi1_node)
>> +               return dev_err_probe(dev, -ENODEV,
>> +                                    "Failed to get remote node for second DSI\n");
>> +
>> +       dsi1_host = of_find_mipi_dsi_host_by_node(dsi1_node);
>> +       of_node_put(dsi1_node);
>> +       if (!dsi1_host)
>> +               return dev_err_probe(dev, -EPROBE_DEFER,
>> +                                    "Failed to find second DSI host\n");
>> +
>> +       /* Copy current DSI info, do not provide OF node since no driver needs to be attached */
>> +       strscpy(info.type, dsi->name, sizeof(info.type));
> 
> Can't you use the two-argument form of strscpy()?

Yeah sure

> 
> FWIW, I also notice that the Sashiko AI bot had some comments. Did you
> already look all of those over and decide they don't need fixing? I
> have a vague recollection that there's no need to worry about someone
> calling disable() and then enable() without going through the
> unprepare() / prepare(). If my memory is correct, I guess that would
> be nice to document... I didn't analyze some of the other claims that
> the AI bot had.

Yep I fixed the real issues, the remaining issues are about the init
sequence and some impossible init sequence between drm and dsi.

Thanks,
Neil

> 
> 
> -Doug


^ permalink raw reply

* Re: [PATCH v4 2/2] clk: amlogic: Add A9 AO clock controller driver
From: Julian Braha @ 2026-06-19  7:48 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: jian.hu, Neil Armstrong, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao,
	Kevin Hilman, Martin Blumenstingl, linux-amlogic, linux-clk,
	devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <1jbjd7c7ip.fsf@starbuckisacylon.baylibre.com>

Hi Jerome,

On 6/19/26 08:29, Jerome Brunet wrote:
> No, regmap clock are directly used so this is necessary.
> Relying on other module dependencies is not enough

What do you mean it's "not enough"?

Functionally, any user of COMMON_CLK_MESON_DUALDIV can also use
COMMON_CLK_MESON_REGMAP.

Unless you mean for documentation purposes?

- Julian Braha

^ permalink raw reply

* [PATCH] arm64: dts: renesas: r9a08g045: Move max-frequency to SoC dtsi
From: Biju @ 2026-06-19  7:56 UTC (permalink / raw)
  To: Geert Uytterhoeven, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Biju Das, linux-renesas-soc, devicetree, linux-kernel,
	Prabhakar Mahadev Lad, Biju Das

From: Biju Das <biju.das.jz@bp.renesas.com>

Move the max-frequency property for SDHI0/1/2 from the board-level
SMARC dtsi files into the r9a08g045.dtsi SoC file, since these
values reflect controller/SoC limitations rather than board-specific.

This removes the duplicated max-frequency = <125000000> entries for
SDHI0 (both SD and eMMC variants) and SDHI1 in rzg3s-smarc-som.dtsi
and rzg3s-smarc.dtsi, and the max-frequency = <50000000> entry for
SDHI2, consolidating them as defaults in r9a08g045.dtsi instead.

Boards needing a different limit can still override max-frequency
locally.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a08g045.dtsi       | 3 +++
 arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi | 3 ---
 arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi     | 1 -
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
index 3a69bb246bab..ae92d45ede38 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
@@ -655,6 +655,7 @@ sdhi0: mmc@11c00000  {
 				 <&cpg CPG_MOD R9A08G045_SDHI0_IMCLK2>,
 				 <&cpg CPG_MOD R9A08G045_SDHI0_ACLK>;
 			clock-names = "core", "clkh", "cd", "aclk";
+			max-frequency = <125000000>;
 			resets = <&cpg R9A08G045_SDHI0_IXRST>;
 			power-domains = <&cpg>;
 			status = "disabled";
@@ -670,6 +671,7 @@ sdhi1: mmc@11c10000 {
 				 <&cpg CPG_MOD R9A08G045_SDHI1_IMCLK2>,
 				 <&cpg CPG_MOD R9A08G045_SDHI1_ACLK>;
 			clock-names = "core", "clkh", "cd", "aclk";
+			max-frequency = <125000000>;
 			resets = <&cpg R9A08G045_SDHI1_IXRST>;
 			power-domains = <&cpg>;
 			status = "disabled";
@@ -685,6 +687,7 @@ sdhi2: mmc@11c20000 {
 				 <&cpg CPG_MOD R9A08G045_SDHI2_IMCLK2>,
 				 <&cpg CPG_MOD R9A08G045_SDHI2_ACLK>;
 			clock-names = "core", "clkh", "cd", "aclk";
+			max-frequency = <50000000>;
 			resets = <&cpg R9A08G045_SDHI2_IXRST>;
 			power-domains = <&cpg>;
 			status = "disabled";
diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
index b45acfe6288a..9039a927bc46 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
@@ -184,7 +184,6 @@ &sdhi0 {
 	bus-width = <4>;
 	sd-uhs-sdr50;
 	sd-uhs-sdr104;
-	max-frequency = <125000000>;
 	status = "okay";
 };
 #else
@@ -199,7 +198,6 @@ &sdhi0 {
 	mmc-hs200-1_8v;
 	non-removable;
 	fixed-emmc-driver-type = <1>;
-	max-frequency = <125000000>;
 	status = "okay";
 };
 #endif
@@ -210,7 +208,6 @@ &sdhi2 {
 	pinctrl-names = "default";
 	vmmc-supply = <&vcc_sdhi2>;
 	bus-width = <4>;
-	max-frequency = <50000000>;
 	status = "okay";
 };
 #endif
diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
index 70af605168b0..e3821d8c01e3 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
@@ -285,7 +285,6 @@ &sdhi1 {
 	bus-width = <4>;
 	sd-uhs-sdr50;
 	sd-uhs-sdr104;
-	max-frequency = <125000000>;
 	status = "okay";
 };
 
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v4 2/2] clk: amlogic: Add A9 AO clock controller driver
From: Jerome Brunet @ 2026-06-19  8:08 UTC (permalink / raw)
  To: Julian Braha
  Cc: jian.hu, Neil Armstrong, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao,
	Kevin Hilman, Martin Blumenstingl, linux-amlogic, linux-clk,
	devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <5af78bca-7c49-48b1-af8a-cfbe55ae26ba@gmail.com>

On ven. 19 juin 2026 at 08:48, Julian Braha <julianbraha@gmail.com> wrote:

> Hi Jerome,
>
> On 6/19/26 08:29, Jerome Brunet wrote:
>> No, regmap clock are directly used so this is necessary.
>> Relying on other module dependencies is not enough
>
> What do you mean it's "not enough"?
>
> Functionally, any user of COMMON_CLK_MESON_DUALDIV can also use
> COMMON_CLK_MESON_REGMAP.

This driver directly use regmap clocks. This is a direct dependency,
required for the build and stated as such. If the module needs it, it
depends on it, simply.

Relying on indirect dependencies is incorrect and fragile.
If those dependencies ever change, the build breaks for no obvious reason.

>
> Unless you mean for documentation purposes?
>
> - Julian Braha

-- 
Jerome

^ permalink raw reply

* Re: [PATCH v3 2/2] interconnect: qcom: add MSM8x60 NoC driver
From: me @ 2026-06-19  8:10 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Herman van Hazendonk, Georgi Djakov, Bjorn Andersson,
	Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-pm, devicetree, linux-kernel
In-Reply-To: <ff72dd25-926b-4a51-99ad-4535f6025dcc@oss.qualcomm.com>

On 2026-06-17 12:36, Konrad Dybcio wrote:
> On 6/6/26 2:34 PM, Herman van Hazendonk wrote:
>> Add a Qualcomm interconnect driver for the MSM8x60 family modelling 
>> the
>> four NoC fabrics (APPSS, System, MMSS, Daytona) that connect masters
>> and slaves on these Scorpion-class SoCs.  The driver implements the
>> interconnect-provider API to manage bandwidth between specific masters
>> and slaves via the RPM arbitration tables.
> 
> [...]
> 
> 
>> +/*
>> + * Minimum fabric clock rate to prevent bus starvation.
>> + *
>> + * When no consumers request bandwidth, the rate calculation yields 
>> 0,
>> + * causing fabric clocks to drop to minimum. This creates bimodal
>> + * performance: fast when other subsystems (like display) happen to
>> + * request bandwidth, slow otherwise.
>> + *
>> + * 384 MHz keeps fabric fast during concurrent MDP display scanout
>> + * and USB gadget traffic. legacy vendor kernel docs: "AXI bus 
>> frequency needs to be
>> + * kept at maximum value while USB data transfers are happening."
>> + * 266 MHz was insufficient - USB crashed during display activity.
>> + */
>> +#define MSM8660_FABRIC_MIN_RATE		384000000UL	/* 384 MHz */
> 
> Can you ensure that through a vote in the USB driver?
> 
> Konrad
Hi Konrad,

I'm reworking the interconnect driver based on earlier feedback, but 
running
into some regressions I need to tackle first. The driver I had was 
stable,
the reworked one has some issues that I'm trying to tackle before 
submitting
another version. The minimum rate will probably disappear as a result of 
that
anyway.

Thanks,
Herman

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox