* Re: [PATCH v5 04/12] i3c: master: Add support for devices using SETAASA
From: Alexandre Belloni @ 2026-07-20 16:07 UTC (permalink / raw)
To: Akhil R
Cc: frank.li, Frank.Li, conor+dt, devicetree, linux-hwmon, linux-i3c,
robh, sashiko-reviews
In-Reply-To: <20260703090855.1519255-1-akhilrajeev@nvidia.com>
On 03/07/2026 09:08:55+0000, Akhil R wrote:
> On Thu, 25 Jun 2026 07:42:03 -0500 Frank Li wrote:
> > On Thu, Jun 25, 2026 at 09:38:15AM +0000, Akhil R wrote:
> >> On Wed, 24 Jun 2026 13:57:46 -0400, Frank Li wrote:
> >> ...
> >> ...
> >> >> [Severity: High]
> >> >> Is it possible that sending the SETAASA broadcast before direct SETDASA
> >> >> assignments breaks initialization for devices that natively support SETAASA
> >> >> but are configured for SETDASA?
> >> >>
> >> >> According to the I3C specification, any device on the bus natively supporting
> >> >> SETAASA will respond to this broadcast by adopting its static address as its
> >> >> dynamic address.
> >> >>
> >> >> After this broadcast, the driver iterates through devices and attempts to
> >> >> assign custom dynamic addresses via direct SETDASA commands:
> >> >>
> >> >> drivers/i3c/master.c:i3c_master_early_i3c_dev_add() {
> >> >> ...
> >> >> ret = i3c_master_setdasa_locked(master, i3cdev->info.static_addr,
> >> >> i3cdev->boardinfo->init_dyn_addr);
> >> >> ...
> >> >> }
> >> >>
> >> >> Since the target device already adopted its dynamic address during the
> >> >> SETAASA broadcast, it is no longer in the unassigned state and will NACK
> >> >> the subsequent SETDASA command.
> >> >
> >> > Look like correct, but I am not sure if target will NACK SETDASA. Or should
> >> > use SETNEWDA for SETAASA method.
> >>
> >> Yes, this looks valid for mixed device buses. I can move
> >> i3c_master_setaasa_locked() after the SETDASA handling and before
> >> i3c_master_do_daa() in the same function, so SETDASA-assigned devices will
> >> ignore the later SETAASA broadcast. Does that sound good to you?
> >
> > yes, try it to follow spec.
>
> I just noticed that the specification says: if both bits 0 and 1 are set,
> meaning both SETDASA and SETAASA are supported, the I3C Bus Controller should
> use SETDASA first.
>
> So moving i3c_master_setaasa_locked() after SETDASA handling follows the spec.
>
> I will wait a few more days before sending v6, to see if there are any other
> concerns.
No other concerns on my side.
>
> Best Regards,
> Akhil
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply
* Re: [PATCH v4 0/4] ASoC: qcom and pinctrl: add LPASS LPR voting and Hawi LPASS LPI TLMM
From: Bartosz Golaszewski @ 2026-07-20 16:04 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Srinivas Kandagatla, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, linux-sound, Bjorn Andersson, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Srinivas Kandagatla, Jaroslav Kysela, Takashi Iwai,
Bartosz Golaszewski, Prasad Kumpatla
In-Reply-To: <44ee60ff-c7f8-4ca7-ad65-7b610ad13d24@kernel.org>
On Mon, 20 Jul 2026 15:54:41 +0200, Krzysztof Kozlowski <krzk@kernel.org> said:
> On 20/07/2026 15:42, Bartosz Golaszewski wrote:
>> On Mon, 20 Jul 2026 15:05:07 +0200, Prasad Kumpatla
>> <prasad.kumpatla@oss.qualcomm.com> said:
>>> This series adds support for LPASS low-power resource (LPR) voting
>>> through PRM and introduces LPASS LPI TLMM pinctrl support for newer
>>> platforms such as Hawi.
>>>
>>> On such platforms, LPASS requires LPR resource voting via PRM to keep
>>> the subsystem active. This is handled by adding a new clock ID and
>>> support for PARAM_ID_RSC_CPU_LPR in q6prm.
>>>
>>> Additionally, a new LPASS LPI TLMM block is introduced, requiring a
>>> dedicated DT binding and pinctrl driver.
>>
>> What is the merging strategy? Is an Ack from Mark required and the series
>> should go together through the pinctrl tree?
>
> Why would an ack be needed? Cover letter is very explicit on that
> through complete silence that there is no dependency (except that it is
> not true) so why would you take ASoC bits?
>
I don't think the cover letter is very explicit on there being no build-time
dependencies, hence my question.
Bart
^ permalink raw reply
* [PATCH v6 3/3] ASoC: qcom: sc8280xp: enhance machine driver for board-specific config
From: Mohammad Rafi Shaik @ 2026-07-20 16:02 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai
Cc: Krzysztof Kozlowski, linux-sound, linux-arm-msm, devicetree,
linux-kernel, Neil Armstrong
In-Reply-To: <20260720160226.1026440-1-mohammad.rafi.shaik@oss.qualcomm.com>
The sc8280xp machine driver is currently written with a largely
SoC-centric view and assumes a uniform audio topology across all boards.
In practice, multiple products based on the same SoC use different board
designs and external audio components, which require board-specific
configuration to function correctly.
Several Qualcomm platforms like talos integrate third-party audio codecs
or use different external audio paths. These designs often require
additional configuration such as explicit MI2S MCLK settings for audio
to work.
This change enhances the sc8280xp machine driver to support board-specific
configuration such as allowing each board variant to provide its own DAPM
widgets and routes, reflecting the actual audio components and connectors
present and enabling MI2S MCLK programming for boards that use external
codecs requiring a stable master clock.
Tested-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
---
sound/soc/qcom/sc8280xp.c | 251 +++++++++++++++++++++++++++++++++++---
1 file changed, 231 insertions(+), 20 deletions(-)
diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
index 98b15a527e37..2ecba74d736e 100644
--- a/sound/soc/qcom/sc8280xp.c
+++ b/sound/soc/qcom/sc8280xp.c
@@ -12,17 +12,78 @@
#include <sound/jack.h>
#include <linux/input-event-codes.h>
#include "qdsp6/q6afe.h"
+#include "qdsp6/q6apm.h"
+#include "qdsp6/q6prm.h"
#include "common.h"
#include "sdw.h"
+#define I2S_MCLKFS 256
+
+#define I2S_MCLK_RATE(rate) \
+ ((rate) * (I2S_MCLKFS))
+#define I2S_BIT_RATE(rate, channels, format) \
+ ((rate) * (channels) * (format))
+
+static struct snd_soc_dapm_widget sc8280xp_dapm_widgets[] = {
+ SND_SOC_DAPM_HP("Headphone Jack", NULL),
+ SND_SOC_DAPM_MIC("Mic Jack", NULL),
+ SND_SOC_DAPM_SPK("DP0 Jack", NULL),
+ SND_SOC_DAPM_SPK("DP1 Jack", NULL),
+ SND_SOC_DAPM_SPK("DP2 Jack", NULL),
+ SND_SOC_DAPM_SPK("DP3 Jack", NULL),
+ SND_SOC_DAPM_SPK("DP4 Jack", NULL),
+ SND_SOC_DAPM_SPK("DP5 Jack", NULL),
+ SND_SOC_DAPM_SPK("DP6 Jack", NULL),
+ SND_SOC_DAPM_SPK("DP7 Jack", NULL),
+};
+
+struct snd_soc_common {
+ const char *driver_name;
+ const struct snd_soc_dapm_widget *dapm_widgets;
+ int num_dapm_widgets;
+ const struct snd_soc_dapm_route *dapm_routes;
+ int num_dapm_routes;
+ const struct snd_kcontrol_new *controls;
+ int num_controls;
+ unsigned int codec_dai_fmt;
+ bool codec_sysclk_set;
+ bool mi2s_mclk_enable;
+ bool mi2s_bclk_enable;
+ bool wcd_jack;
+};
+
struct sc8280xp_snd_data {
bool stream_prepared[AFE_PORT_MAX];
struct snd_soc_card *card;
struct snd_soc_jack jack;
struct snd_soc_jack dp_jack[8];
+ const struct snd_soc_common *snd_soc_common_priv;
bool jack_setup;
};
+static inline int sc8280xp_get_mclk_freq(struct snd_pcm_hw_params *params)
+{
+ int rate = params_rate(params);
+
+ switch (rate) {
+ case 11025:
+ case 44100:
+ case 88200:
+ return I2S_MCLK_RATE(44100);
+ default:
+ break;
+ }
+
+ return I2S_MCLK_RATE(rate);
+}
+
+static inline int sc8280xp_get_bclk_freq(struct snd_pcm_hw_params *params)
+{
+ return I2S_BIT_RATE(params_rate(params),
+ params_channels(params),
+ snd_pcm_format_width(params_format(params)));
+}
+
static int sc8280xp_snd_init(struct snd_soc_pcm_runtime *rtd)
{
struct sc8280xp_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
@@ -32,10 +93,6 @@ static int sc8280xp_snd_init(struct snd_soc_pcm_runtime *rtd)
int dp_pcm_id = 0;
switch (cpu_dai->id) {
- case PRIMARY_MI2S_RX...QUATERNARY_MI2S_TX:
- case QUINARY_MI2S_RX...QUINARY_MI2S_TX:
- snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_BP_FP);
- break;
case WSA_CODEC_DMA_RX_0:
case WSA_CODEC_DMA_RX_1:
/*
@@ -64,7 +121,10 @@ static int sc8280xp_snd_init(struct snd_soc_pcm_runtime *rtd)
if (dp_jack)
return qcom_snd_dp_jack_setup(rtd, dp_jack, dp_pcm_id);
- return qcom_snd_wcd_jack_setup(rtd, &data->jack, &data->jack_setup);
+ if (data->snd_soc_common_priv->wcd_jack)
+ return qcom_snd_wcd_jack_setup(rtd, &data->jack, &data->jack_setup);
+
+ return 0;
}
static int sc8280xp_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
@@ -96,6 +156,63 @@ static int sc8280xp_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
return 0;
}
+static int sc8280xp_snd_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params)
+{
+ struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+ struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
+ struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
+ struct sc8280xp_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
+ int mclk_freq = sc8280xp_get_mclk_freq(params);
+ int bclk_freq = sc8280xp_get_bclk_freq(params);
+ int ret;
+
+ switch (cpu_dai->id) {
+ case PRIMARY_MI2S_RX ... QUATERNARY_MI2S_TX:
+ case QUINARY_MI2S_RX ... QUINARY_MI2S_TX:
+ case SENARY_MI2S_RX ... SENARY_MI2S_TX:
+ ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_BP_FP);
+ if (ret && ret != -ENOTSUPP)
+ return ret;
+
+ if (data->snd_soc_common_priv->codec_dai_fmt) {
+ ret = snd_soc_dai_set_fmt(codec_dai,
+ data->snd_soc_common_priv->codec_dai_fmt);
+ if (ret && ret != -ENOTSUPP)
+ return ret;
+ }
+
+ if (data->snd_soc_common_priv->mi2s_mclk_enable) {
+ ret = snd_soc_dai_set_sysclk(cpu_dai,
+ LPAIF_MI2S_MCLK, mclk_freq,
+ SND_SOC_CLOCK_OUT);
+ if (ret)
+ return ret;
+ }
+
+ if (data->snd_soc_common_priv->mi2s_bclk_enable) {
+ ret = snd_soc_dai_set_sysclk(cpu_dai,
+ LPAIF_MI2S_BCLK, bclk_freq,
+ SND_SOC_CLOCK_OUT);
+ if (ret)
+ return ret;
+ }
+
+ if (data->snd_soc_common_priv->codec_sysclk_set) {
+ ret = snd_soc_dai_set_sysclk(codec_dai,
+ 0, mclk_freq,
+ SND_SOC_CLOCK_IN);
+ if (ret)
+ return ret;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
static int sc8280xp_snd_prepare(struct snd_pcm_substream *substream)
{
struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
@@ -117,6 +234,7 @@ static int sc8280xp_snd_hw_free(struct snd_pcm_substream *substream)
static const struct snd_soc_ops sc8280xp_be_ops = {
.startup = qcom_snd_sdw_startup,
.shutdown = qcom_snd_sdw_shutdown,
+ .hw_params = sc8280xp_snd_hw_params,
.hw_free = sc8280xp_snd_hw_free,
.prepare = sc8280xp_snd_prepare,
};
@@ -145,38 +263,131 @@ static int sc8280xp_platform_probe(struct platform_device *pdev)
card = devm_kzalloc(dev, sizeof(*card), GFP_KERNEL);
if (!card)
return -ENOMEM;
- card->owner = THIS_MODULE;
+
/* Allocate the private data */
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
+ data->snd_soc_common_priv = of_device_get_match_data(dev);
+ if (!data->snd_soc_common_priv)
+ return -ENODEV;
+
+ card->owner = THIS_MODULE;
card->dev = dev;
dev_set_drvdata(dev, card);
snd_soc_card_set_drvdata(card, data);
+ card->dapm_widgets = data->snd_soc_common_priv->dapm_widgets;
+ card->num_dapm_widgets = data->snd_soc_common_priv->num_dapm_widgets;
+ card->dapm_routes = data->snd_soc_common_priv->dapm_routes;
+ card->num_dapm_routes = data->snd_soc_common_priv->num_dapm_routes;
+ card->controls = data->snd_soc_common_priv->controls;
+ card->num_controls = data->snd_soc_common_priv->num_controls;
+
ret = qcom_snd_parse_of(card);
if (ret)
return ret;
- card->driver_name = of_device_get_match_data(dev);
+ card->driver_name = data->snd_soc_common_priv->driver_name;
sc8280xp_add_be_ops(card);
return devm_snd_soc_register_card(dev, card);
}
+static const struct snd_soc_common eliza_priv_data = {
+ .driver_name = "eliza",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+ .wcd_jack = true,
+};
+
+static const struct snd_soc_common kaanapali_priv_data = {
+ .driver_name = "kaanapali",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+ .wcd_jack = true,
+};
+
+static const struct snd_soc_common qcs9100_priv_data = {
+ .driver_name = "sa8775p",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+};
+
+static const struct snd_soc_common qcs615_priv_data = {
+ .driver_name = "qcs615",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+};
+
+static const struct snd_soc_common qcm6490_priv_data = {
+ .driver_name = "qcm6490",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+ .wcd_jack = true,
+};
+
+static const struct snd_soc_common qcs6490_priv_data = {
+ .driver_name = "qcs6490",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+ .wcd_jack = true,
+};
+
+static const struct snd_soc_common qcs8275_priv_data = {
+ .driver_name = "qcs8300",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+};
+
+static const struct snd_soc_common sc8280xp_priv_data = {
+ .driver_name = "sc8280xp",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+ .wcd_jack = true,
+};
+
+static const struct snd_soc_common sm8450_priv_data = {
+ .driver_name = "sm8450",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+ .wcd_jack = true,
+};
+
+static const struct snd_soc_common sm8550_priv_data = {
+ .driver_name = "sm8550",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+ .wcd_jack = true,
+};
+
+static const struct snd_soc_common sm8650_priv_data = {
+ .driver_name = "sm8650",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+ .wcd_jack = true,
+};
+
+static const struct snd_soc_common sm8750_priv_data = {
+ .driver_name = "sm8750",
+ .dapm_widgets = sc8280xp_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
+ .wcd_jack = true,
+};
+
static const struct of_device_id snd_sc8280xp_dt_match[] = {
- {.compatible = "qcom,eliza-sndcard", "eliza"},
- {.compatible = "qcom,kaanapali-sndcard", "kaanapali"},
- {.compatible = "qcom,qcm6490-idp-sndcard", "qcm6490"},
- {.compatible = "qcom,qcs615-sndcard", "qcs615"},
- {.compatible = "qcom,qcs6490-rb3gen2-sndcard", "qcs6490"},
- {.compatible = "qcom,qcs8275-sndcard", "qcs8300"},
- {.compatible = "qcom,qcs9075-sndcard", "sa8775p"},
- {.compatible = "qcom,qcs9100-sndcard", "sa8775p"},
- {.compatible = "qcom,sc8280xp-sndcard", "sc8280xp"},
- {.compatible = "qcom,sm8450-sndcard", "sm8450"},
- {.compatible = "qcom,sm8550-sndcard", "sm8550"},
- {.compatible = "qcom,sm8650-sndcard", "sm8650"},
- {.compatible = "qcom,sm8750-sndcard", "sm8750"},
+ { .compatible = "qcom,eliza-sndcard", .data = &eliza_priv_data },
+ { .compatible = "qcom,kaanapali-sndcard", .data = &kaanapali_priv_data },
+ { .compatible = "qcom,qcm6490-idp-sndcard", .data = &qcm6490_priv_data },
+ { .compatible = "qcom,qcs615-sndcard", .data = &qcs615_priv_data },
+ { .compatible = "qcom,qcs6490-rb3gen2-sndcard", .data = &qcs6490_priv_data },
+ { .compatible = "qcom,qcs8275-sndcard", .data = &qcs8275_priv_data },
+ { .compatible = "qcom,qcs9075-sndcard", .data = &qcs9100_priv_data },
+ { .compatible = "qcom,qcs9100-sndcard", .data = &qcs9100_priv_data },
+ { .compatible = "qcom,sc8280xp-sndcard", .data = &sc8280xp_priv_data },
+ { .compatible = "qcom,sm8450-sndcard", .data = &sm8450_priv_data },
+ { .compatible = "qcom,sm8550-sndcard", .data = &sm8550_priv_data },
+ { .compatible = "qcom,sm8650-sndcard", .data = &sm8650_priv_data },
+ { .compatible = "qcom,sm8750-sndcard", .data = &sm8750_priv_data },
{}
};
--
2.34.1
^ permalink raw reply related
* [PATCH v6 2/3] ASoC: qcom: q6apm-lpass-dais: Add MI2S clock control
From: Mohammad Rafi Shaik @ 2026-07-20 16:02 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai
Cc: Krzysztof Kozlowski, linux-sound, linux-arm-msm, devicetree,
linux-kernel, Srinivas Kandagatla, Neil Armstrong
In-Reply-To: <20260720160226.1026440-1-mohammad.rafi.shaik@oss.qualcomm.com>
Add support for MI2S clock control within q6apm-lpass DAIs, including
handling of MCLK, BCLK, and ECLK via the DAI .set_sysclk callback.
Each MI2S port now retrieves its clock handles from the device tree,
allowing per-port clock configuration and proper enable/disable during
startup and shutdown.
Co-developed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
---
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 194 +++++++++++++++++++++++-
sound/soc/qcom/qdsp6/q6prm.h | 4 +
2 files changed, 196 insertions(+), 2 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index 006b283484d9..040b97f03897 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -2,10 +2,12 @@
// Copyright (c) 2021, Linaro Limited
#include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h>
+#include <linux/clk.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/device.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <sound/pcm.h>
@@ -15,15 +17,64 @@
#include "q6dsp-common.h"
#include "audioreach.h"
#include "q6apm.h"
+#include "q6prm.h"
#define AUDIOREACH_BE_PCM_BASE 16
+struct q6apm_dai_priv_data {
+ struct clk *mclk;
+ struct clk *bclk;
+ struct clk *eclk;
+ bool mclk_enabled, bclk_enabled, eclk_enabled;
+};
+
struct q6apm_lpass_dai_data {
struct q6apm_graph *graph[APM_PORT_MAX];
bool is_port_started[APM_PORT_MAX];
struct audioreach_module_config module_config[APM_PORT_MAX];
+ struct q6apm_dai_priv_data priv[APM_PORT_MAX];
};
+static void q6apm_lpass_dai_disable_clocks(struct q6apm_lpass_dai_data *dai_data, int id)
+{
+ if (dai_data->priv[id].mclk_enabled) {
+ clk_disable_unprepare(dai_data->priv[id].mclk);
+ dai_data->priv[id].mclk_enabled = false;
+ }
+
+ if (dai_data->priv[id].bclk_enabled) {
+ clk_disable_unprepare(dai_data->priv[id].bclk);
+ dai_data->priv[id].bclk_enabled = false;
+ }
+
+ if (dai_data->priv[id].eclk_enabled) {
+ clk_disable_unprepare(dai_data->priv[id].eclk);
+ dai_data->priv[id].eclk_enabled = false;
+ }
+}
+
+static void q6apm_lpass_dai_put_clocks(struct q6apm_lpass_dai_data *dai_data)
+{
+ int i;
+
+ for (i = 0; i < APM_PORT_MAX; i++) {
+ q6apm_lpass_dai_disable_clocks(dai_data, i);
+
+ if (dai_data->priv[i].mclk) {
+ clk_put(dai_data->priv[i].mclk);
+ dai_data->priv[i].mclk = NULL;
+ }
+ if (dai_data->priv[i].bclk) {
+ clk_put(dai_data->priv[i].bclk);
+ dai_data->priv[i].bclk = NULL;
+ }
+ if (dai_data->priv[i].eclk) {
+ clk_put(dai_data->priv[i].eclk);
+ dai_data->priv[i].eclk = NULL;
+ }
+ }
+}
+
static int q6dma_set_channel_map(struct snd_soc_dai *dai,
unsigned int tx_num,
const unsigned int *tx_ch_mask,
@@ -251,6 +302,66 @@ static int q6apm_lpass_dai_startup(struct snd_pcm_substream *substream, struct s
return 0;
}
+static int q6i2s_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+{
+ return q6apm_lpass_dai_startup(substream, dai);
+}
+
+static void q6i2s_lpass_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+{
+ struct q6apm_lpass_dai_data *dai_data = dev_get_drvdata(dai->dev);
+
+ q6apm_lpass_dai_shutdown(substream, dai);
+ q6apm_lpass_dai_disable_clocks(dai_data, dai->id);
+}
+
+static int q6i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id, unsigned int freq, int dir)
+{
+ struct q6apm_lpass_dai_data *dai_data = dev_get_drvdata(dai->dev);
+ struct clk *sysclk = NULL;
+ bool *enabled = NULL;
+ int ret = 0;
+
+ switch (clk_id) {
+ case LPAIF_MI2S_MCLK:
+ sysclk = dai_data->priv[dai->id].mclk;
+ enabled = &dai_data->priv[dai->id].mclk_enabled;
+ break;
+ case LPAIF_MI2S_BCLK:
+ sysclk = dai_data->priv[dai->id].bclk;
+ enabled = &dai_data->priv[dai->id].bclk_enabled;
+ break;
+ case LPAIF_MI2S_ECLK:
+ sysclk = dai_data->priv[dai->id].eclk;
+ enabled = &dai_data->priv[dai->id].eclk_enabled;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (sysclk) {
+ ret = clk_set_rate(sysclk, freq);
+ if (ret) {
+ dev_err(dai->dev, "Error, Unable to set rate (%d) for sysclk %d\n",
+ freq, clk_id);
+ return ret;
+ }
+
+ if (*enabled)
+ return 0;
+
+ ret = clk_prepare_enable(sysclk);
+ if (ret) {
+ dev_err(dai->dev, "Error, Unable to prepare (%d) sysclk\n", clk_id);
+ return ret;
+ }
+
+ *enabled = true;
+ }
+
+ return ret;
+}
+
static int q6i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
{
struct q6apm_lpass_dai_data *dai_data = dev_get_drvdata(dai->dev);
@@ -272,11 +383,12 @@ static const struct snd_soc_dai_ops q6dma_ops = {
static const struct snd_soc_dai_ops q6i2s_ops = {
.prepare = q6apm_lpass_dai_prepare,
- .startup = q6apm_lpass_dai_startup,
- .shutdown = q6apm_lpass_dai_shutdown,
+ .startup = q6i2s_dai_startup,
+ .shutdown = q6i2s_lpass_dai_shutdown,
.set_channel_map = q6dma_set_channel_map,
.hw_params = q6dma_hw_params,
.set_fmt = q6i2s_set_fmt,
+ .set_sysclk = q6i2s_set_sysclk,
.trigger = q6apm_lpass_dai_trigger,
};
@@ -297,6 +409,76 @@ static const struct snd_soc_component_driver q6apm_lpass_dai_component = {
.remove_order = SND_SOC_COMP_ORDER_FIRST,
};
+static int of_q6apm_parse_dai_data(struct device *dev,
+ struct q6apm_lpass_dai_data *data)
+{
+ int ret;
+
+ for_each_child_of_node_scoped(dev->of_node, node) {
+ struct q6apm_dai_priv_data *priv;
+ int id;
+
+ ret = of_property_read_u32(node, "reg", &id);
+ if (ret || id < 0 || id >= APM_PORT_MAX) {
+ dev_err(dev, "valid dai id not found:%d\n", ret);
+ continue;
+ }
+
+ switch (id) {
+ /* MI2S specific properties */
+ case PRIMARY_MI2S_RX ... QUATERNARY_MI2S_TX:
+ case QUINARY_MI2S_RX ... QUINARY_MI2S_TX:
+ case SENARY_MI2S_RX ... SENARY_MI2S_TX:
+ priv = &data->priv[id];
+ priv->mclk = of_clk_get_by_name(node, "mclk");
+ if (IS_ERR(priv->mclk)) {
+ int err = PTR_ERR(priv->mclk);
+
+ priv->mclk = NULL;
+ if (err == -EPROBE_DEFER) {
+ q6apm_lpass_dai_put_clocks(data);
+ return dev_err_probe(dev, err,
+ "unable to get mi2s mclk\n");
+ }
+ }
+
+ priv->bclk = of_clk_get_by_name(node, "bclk");
+ if (IS_ERR(priv->bclk)) {
+ int err = PTR_ERR(priv->bclk);
+
+ priv->bclk = NULL;
+ if (err == -EPROBE_DEFER) {
+ q6apm_lpass_dai_put_clocks(data);
+ return dev_err_probe(dev, err,
+ "unable to get mi2s bclk\n");
+ }
+ }
+
+ priv->eclk = of_clk_get_by_name(node, "eclk");
+ if (IS_ERR(priv->eclk)) {
+ int err = PTR_ERR(priv->eclk);
+
+ priv->eclk = NULL;
+ if (err == -EPROBE_DEFER) {
+ q6apm_lpass_dai_put_clocks(data);
+ return dev_err_probe(dev, err,
+ "unable to get mi2s eclk\n");
+ }
+ }
+ break;
+ default:
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static void q6apm_lpass_dai_clocks_action(void *data)
+{
+ q6apm_lpass_dai_put_clocks(data);
+}
+
static int q6apm_lpass_dai_dev_probe(struct platform_device *pdev)
{
struct q6dsp_audio_port_dai_driver_config cfg;
@@ -304,12 +486,20 @@ static int q6apm_lpass_dai_dev_probe(struct platform_device *pdev)
struct snd_soc_dai_driver *dais;
struct device *dev = &pdev->dev;
int num_dais;
+ int ret;
dai_data = devm_kzalloc(dev, sizeof(*dai_data), GFP_KERNEL);
if (!dai_data)
return -ENOMEM;
dev_set_drvdata(dev, dai_data);
+ ret = of_q6apm_parse_dai_data(dev, dai_data);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(dev, q6apm_lpass_dai_clocks_action, dai_data);
+ if (ret)
+ return ret;
memset(&cfg, 0, sizeof(cfg));
cfg.q6i2s_ops = &q6i2s_ops;
diff --git a/sound/soc/qcom/qdsp6/q6prm.h b/sound/soc/qcom/qdsp6/q6prm.h
index a988a32086fe..bbbe6d368a41 100644
--- a/sound/soc/qcom/qdsp6/q6prm.h
+++ b/sound/soc/qcom/qdsp6/q6prm.h
@@ -3,6 +3,10 @@
#ifndef __Q6PRM_H__
#define __Q6PRM_H__
+#define LPAIF_MI2S_MCLK 1
+#define LPAIF_MI2S_BCLK 2
+#define LPAIF_MI2S_ECLK 3
+
/* Clock ID for Primary I2S IBIT */
#define Q6PRM_LPASS_CLK_ID_PRI_MI2S_IBIT 0x100
/* Clock ID for Primary I2S EBIT */
--
2.34.1
^ permalink raw reply related
* [PATCH v6 1/3] ASoC: dt-bindings: qcom,q6apm-lpass-dais: Document DAI subnode
From: Mohammad Rafi Shaik @ 2026-07-20 16:02 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai
Cc: Krzysztof Kozlowski, linux-sound, linux-arm-msm, devicetree,
linux-kernel, Srinivas Kandagatla
In-Reply-To: <20260720160226.1026440-1-mohammad.rafi.shaik@oss.qualcomm.com>
Extend the qcom,q6apm-lpass-dais device tree binding to explicitly
describe Digital Audio Interface (DAI) child nodes.
Add #address-cells and #size-cells to allow representation of multiple
DAI instances as child nodes, and define a dai@<id> pattern to document
per-DAI properties such as the interface ID and associated clocks.
On platforms such as Monaco and Lemans, third-party codecs are hardware
wired to the SoC and do not always have an in-tree codec driver to manage
their clocks. For these designs, clock line enablement must be driven
from the platform side, and this series provides the necessary support
for that.
On QAIF-based platforms such as Shikra and Hawi, responsibility for voting
I2S MCLK and BCLK has moved from the DSP to the kernel. This series
introduces the required device tree binding support to represent and
vote for these clocks from the kernel.
Co-developed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
---
.../bindings/sound/qcom,q6apm-lpass-dais.yaml | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-lpass-dais.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-lpass-dais.yaml
index 2fb95544db8b..cde52c6f6761 100644
--- a/Documentation/devicetree/bindings/sound/qcom,q6apm-lpass-dais.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-lpass-dais.yaml
@@ -21,6 +21,46 @@ properties:
'#sound-dai-cells':
const: 1
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+# Digital Audio Interfaces
+patternProperties:
+ '^dai@[0-9a-f]+$':
+ type: object
+ description:
+ Q6DSP Digital Audio Interfaces.
+
+ properties:
+ reg:
+ maxItems: 1
+ description:
+ Digital Audio Interface ID
+
+ clocks:
+ minItems: 1
+ items:
+ - description: MI2S bit clock
+ - description: MI2S external bit clock
+ - description: MI2S master clock
+
+ clock-names:
+ minItems: 1
+ items:
+ - enum: [bclk, eclk, mclk]
+ - const: mclk
+
+ dependencies:
+ clocks: [clock-names]
+
+ required:
+ - reg
+
+ additionalProperties: false
+
required:
- compatible
- '#sound-dai-cells'
@@ -29,7 +69,20 @@ unevaluatedProperties: false
examples:
- |
+ #include <dt-bindings/sound/qcom,q6afe.h>
+
dais {
compatible = "qcom,q6apm-lpass-dais";
#sound-dai-cells = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dai@10 {
+ reg = <PRIMARY_MI2S_RX>;
+ clocks = <&q6prmcc LPASS_CLK_ID_PRI_MI2S_IBIT
+ LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+ <&q6prmcc LPASS_CLK_ID_MCLK_1
+ LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
+ clock-names = "bclk", "mclk";
+ };
};
--
2.34.1
^ permalink raw reply related
* [PATCH v6 0/3] ASoC: qcom: qdsp6: Add MI2S clock control
From: Mohammad Rafi Shaik @ 2026-07-20 16:02 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai
Cc: Krzysztof Kozlowski, linux-sound, linux-arm-msm, devicetree,
linux-kernel
Add support for MI2S clock control within q6apm-lpass DAIs, including
handling of MCLK, BCLK, and ECLK via the DAI .set_sysclk callback.
Each MI2S port now retrieves its clock handles from the device tree,
allowing per-port clock configuration and proper enable/disable during
startup and shutdown.
On platforms such as Monaco and Lemans, third-party codecs are
hardware-wired to the SoC and do not always have an in-tree codec
driver to manage their clocks. For these designs, clock line
enablement must be driven from the platform side, and this
series provides the necessary support for that.
On QAIF-based platforms such as Shikra and Hawi, responsibility
for voting I2S MCLK and bit-clock has moved from the DSP to the
kernel. This series introduces the required device tree binding
support to represent and vote for these clocks from the kernel.
Enhances the sc8280xp machine driver to set the boards spacific
configurations.
---
Changes in v6:
- Addressed review comments, reported by Krzysztof.
- Resolved all sashiko comments.
- Link to v5: https://lore.kernel.org/all/20260714145250.2473461-1-mohammad.rafi.shaik@oss.qualcomm.com/
Changes in v5:
- Addressed review comments, reported by Krzysztof.
- Resolved all sashiko comments.
- Link to v4: https://lore.kernel.org/all/20260711125740.3083236-1-mohammad.rafi.shaik@oss.qualcomm.com/
Changes in v4:
- Addressed review comments from Mark Brown.
- Resolved all sashiko comments.
- Link to v3: https://lore.kernel.org/all/20260706132009.1496321-1-mohammad.rafi.shaik@oss.qualcomm.com/
Changes in v3:
- Addressed all review comments from Mark Brown.
- Fixed OF node reference handling, clock configuration, and sample-rate
handling issues as suggested by Mark Brown.
- Added proper error checking for DAI configuration APIs as suggested by Mark Brown.
- Added SENARY DAI support alongside MI2S DAIs as suggested by Val Packett.
- Link to v2: https://lore.kernel.org/all/20260608023011.942228-1-mohammad.rafi.shaik@oss.qualcomm.com/
Changes in v2:
- Added a detailed commit description to clearly explain the need for this change.
- Improved the machine driver based on Neil’s feedback.
- Link to v1: https://lore.kernel.org/all/20260309111300.2484262-1-mohammad.rafi.shaik@oss.qualcomm.com/
---
Mohammad Rafi Shaik (3):
ASoC: dt-bindings: qcom,q6apm-lpass-dais: Document DAI subnode
ASoC: qcom: q6apm-lpass-dais: Add MI2S clock control
ASoC: qcom: sc8280xp: enhance machine driver for board-specific config
.../bindings/sound/qcom,q6apm-lpass-dais.yaml | 53 ++++
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 194 +++++++++++++-
sound/soc/qcom/qdsp6/q6prm.h | 4 +
sound/soc/qcom/sc8280xp.c | 251 ++++++++++++++++--
4 files changed, 480 insertions(+), 22 deletions(-)
base-commit: 0718283ab28bc3907e10b61a6b4be6fefa1cbb2f
--
2.34.1
^ permalink raw reply
* [PATCH] Documentation/devicetree/bindings: update Sudeep Holla's email address
From: Laszlo Ersek @ 2026-07-20 16:02 UTC (permalink / raw)
To: laszlo.ersek
Cc: Conor Dooley, Cristian Marussi, Jassi Brar, Krzysztof Kozlowski,
Linus Walleij, Liviu Dudau, Lorenzo Pieralisi, Rob Herring,
Sudeep Holla, arm-scmi, devicetree, linux-arm-kernel,
linux-kernel
MAINTAINERS disagrees with Documentation/devicetree/bindings on Sudeep's
email address; fix the latter.
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Cristian Marussi <cristian.marussi@arm.com>
Cc: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Linus Walleij <linusw@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Sudeep Holla <sudeep.holla@kernel.org>
Cc: arm-scmi@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Fixes: 59e82a4237cf ("MAINTAINERS: Change Sudeep Holla's email address")
Signed-off-by: Laszlo Ersek <laszlo.ersek@arm.com>
---
Documentation/devicetree/bindings/arm/arm,juno-fpga-apb-regs.yaml | 2 +-
Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml | 2 +-
Documentation/devicetree/bindings/arm/arm,vexpress-scc.yaml | 2 +-
Documentation/devicetree/bindings/dvfs/performance-domain.yaml | 2 +-
Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 2 +-
Documentation/devicetree/bindings/firmware/arm,scpi.yaml | 2 +-
Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/arm,juno-fpga-apb-regs.yaml b/Documentation/devicetree/bindings/arm/arm,juno-fpga-apb-regs.yaml
index ce5f2e1ec1ea..4ae7dc706b84 100644
--- a/Documentation/devicetree/bindings/arm/arm,juno-fpga-apb-regs.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,juno-fpga-apb-regs.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: ARM Juno FPGA APB Registers
maintainers:
- - Sudeep Holla <sudeep.holla@arm.com>
+ - Sudeep Holla <sudeep.holla@kernel.org>
properties:
compatible:
diff --git a/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml b/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml
index 95d4baa85506..30cc534a91ee 100644
--- a/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,vexpress-juno.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: ARM Versatile Express and Juno Boards
maintainers:
- - Sudeep Holla <sudeep.holla@arm.com>
+ - Sudeep Holla <sudeep.holla@kernel.org>
- Linus Walleij <linusw@kernel.org>
description: |+
diff --git a/Documentation/devicetree/bindings/arm/arm,vexpress-scc.yaml b/Documentation/devicetree/bindings/arm/arm,vexpress-scc.yaml
index 9b8f7e0c4ea0..060c0bc58a15 100644
--- a/Documentation/devicetree/bindings/arm/arm,vexpress-scc.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,vexpress-scc.yaml
@@ -8,7 +8,7 @@ title: ARM Versatile Express Serial Configuration Controller
maintainers:
- Liviu Dudau <liviu.dudau@arm.com>
- - Sudeep Holla <sudeep.holla@arm.com>
+ - Sudeep Holla <sudeep.holla@kernel.org>
description: |
Test chips for ARM Versatile Express platform implement SCC (Serial
diff --git a/Documentation/devicetree/bindings/dvfs/performance-domain.yaml b/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
index cc930660b794..09bbd2e33786 100644
--- a/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
+++ b/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: Generic performance domains
maintainers:
- - Sudeep Holla <sudeep.holla@arm.com>
+ - Sudeep Holla <sudeep.holla@kernel.org>
description: |+
This binding is intended for performance management of groups of devices or
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index d06cca9273c4..6fc44105f1c3 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -8,7 +8,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: System Control and Management Interface (SCMI) Message Protocol
maintainers:
- - Sudeep Holla <sudeep.holla@arm.com>
+ - Sudeep Holla <sudeep.holla@kernel.org>
description: |
The SCMI is intended to allow agents such as OSPM to manage various functions
diff --git a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
index 241317239ffc..08c7e05c577c 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
@@ -8,7 +8,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: System Control and Power Interface (SCPI) Message Protocol
maintainers:
- - Sudeep Holla <sudeep.holla@arm.com>
+ - Sudeep Holla <sudeep.holla@kernel.org>
description: |
Firmware implementing the SCPI described in ARM document number ARM DUI
diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml b/Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml
index 449b55afeb7d..025a5c55b284 100644
--- a/Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml
+++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: ARM MHUv3 Mailbox Controller
maintainers:
- - Sudeep Holla <sudeep.holla@arm.com>
+ - Sudeep Holla <sudeep.holla@kernel.org>
- Cristian Marussi <cristian.marussi@arm.com>
description: |
^ permalink raw reply related
* Re: [PATCH net-next v9 12/12] net: airoha: add phylink support
From: Christian Marangi @ 2026-07-20 15:43 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Simon Horman, Jonathan Corbet, Shuah Khan, Heiner Kallweit,
Russell King, Saravana Kannan, Philipp Zabel, netdev, devicetree,
linux-kernel, linux-doc, linux-arm-kernel, linux-mediatek,
Maxime Chevallier
In-Reply-To: <al5AVLxWRhnpGzJ1@lore-desk>
On Mon, Jul 20, 2026 at 05:35:48PM +0200, Lorenzo Bianconi wrote:
> > Add phylink support for each GDM port. For GDM1 add the internal interface
> > mode as the only supported mode. For GDM2/3/4 add the required
> > configuration of the PCS to make the external PHY or attached SFP cage
> > work.
> >
> > These needs to be defined in the GDM port node using the pcs-handle
> > property.
> >
> > Update and provide a .get/set_link_ksettings function that use phylink
> > for ethtool OPs now that we fully support phylink.
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> > drivers/net/ethernet/airoha/Kconfig | 1 +
> > drivers/net/ethernet/airoha/airoha_eth.c | 194 +++++++++++++++++++++-
> > drivers/net/ethernet/airoha/airoha_eth.h | 7 +-
> > drivers/net/ethernet/airoha/airoha_regs.h | 12 ++
> > 4 files changed, 207 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/airoha/Kconfig b/drivers/net/ethernet/airoha/Kconfig
> > index 1f6640a15fc9..789906516bf8 100644
> > --- a/drivers/net/ethernet/airoha/Kconfig
> > +++ b/drivers/net/ethernet/airoha/Kconfig
> > @@ -20,6 +20,7 @@ config NET_AIROHA
> > depends on NET_DSA || !NET_DSA
> > select NET_AIROHA_NPU
> > select PAGE_POOL
> > + select PHYLINK
> > help
> > This driver supports the gigabit ethernet MACs in the
> > Airoha SoC family.
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > index 59001fd4b6f7..ed1ac032f337 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> > @@ -8,6 +8,7 @@
> > #include <linux/of_reserved_mem.h>
> > #include <linux/platform_device.h>
> > #include <linux/tcp.h>
> > +#include <linux/pcs/pcs.h>
> > #include <linux/u64_stats_sync.h>
> > #include <net/dst_metadata.h>
> > #include <net/page_pool/helpers.h>
> > @@ -1837,7 +1838,7 @@ static void airoha_update_hw_stats(struct airoha_gdm_dev *dev)
> > struct airoha_gdm_port *port = dev->port;
> > int i;
> >
> > - spin_lock(&port->stats_lock);
> > + spin_lock(&port->lock);
>
> Hi Christian,
>
> as pointed out in a previous email, I do not like the approach of reusing this
> spin_lock for airoha_mac_link_up(). Can we use rtl_lock() (when necessary) as
> pointed out before?
>
For context, quoting from the previous series, the suggestion is to use
rtnl_is_locked() and then lock accordingly but I didn't find other usage of
that in other driver (aside from core net) and I don't like the use of
is_locked. I can already see the BOT saying that in the timeframe of
is_locked and writing the register another interface goes up causing a
race.
Guess I will add a simple mutex for this case and the other.
> >
> > for (i = 0; i < ARRAY_SIZE(port->devs); i++) {
> > if (port->devs[i])
> > @@ -1848,7 +1849,7 @@ static void airoha_update_hw_stats(struct airoha_gdm_dev *dev)
> > airoha_fe_set(dev->eth, REG_FE_GDM_MIB_CLEAR(port->id),
> > FE_GDM_MIB_RX_CLEAR_MASK | FE_GDM_MIB_TX_CLEAR_MASK);
> >
> > - spin_unlock(&port->stats_lock);
> > + spin_unlock(&port->lock);
> > }
> >
> > static void airoha_dev_set_xmit_frame_size(struct net_device *netdev)
> > @@ -1870,6 +1871,14 @@ static int airoha_dev_open(struct net_device *netdev)
> > u32 pse_port = FE_PSE_PORT_PPE1;
> > int err;
> >
> > + err = phylink_of_phy_connect(dev->phylink, netdev->dev.of_node, 0);
> > + if (err) {
> > + netdev_err(netdev, "could not attach PHY: %d\n", err);
> > + return err;
> > + }
> > +
> > + phylink_start(dev->phylink);
> > +
> > netif_tx_start_all_queues(netdev);
> > err = airoha_set_vip_for_gdm_port(dev, true);
> > if (err)
> > @@ -1909,6 +1918,10 @@ static int airoha_dev_stop(struct net_device *netdev)
> > airoha_set_gdm_port_fwd_cfg(qdma->eth,
> > REG_GDM_FWD_CFG(port->id),
> > FE_PSE_PORT_DROP);
> > +
> > + phylink_stop(dev->phylink);
> > + phylink_disconnect_phy(dev->phylink);
> > +
> > return 0;
> > }
> >
> > @@ -2389,6 +2402,24 @@ airoha_ethtool_get_rmon_stats(struct net_device *netdev,
> > } while (u64_stats_fetch_retry(&dev->stats.syncp, start));
> > }
> >
> > +static int
> > +airoha_ethtool_get_link_ksettings(struct net_device *netdev,
> > + struct ethtool_link_ksettings *cmd)
> > +{
> > + struct airoha_gdm_dev *dev = netdev_priv(netdev);
> > +
> > + return phylink_ethtool_ksettings_get(dev->phylink, cmd);
> > +}
> > +
> > +static int
> > +airoha_ethtool_set_link_ksettings(struct net_device *netdev,
> > + const struct ethtool_link_ksettings *cmd)
> > +{
> > + struct airoha_gdm_dev *dev = netdev_priv(netdev);
> > +
> > + return phylink_ethtool_ksettings_set(dev->phylink, cmd);
> > +}
> > +
> > static int airoha_qdma_set_chan_tx_sched(struct net_device *netdev,
> > int channel, enum tx_sched_mode mode,
> > const u16 *weights, u8 n_weights)
> > @@ -3120,7 +3151,8 @@ static const struct ethtool_ops airoha_ethtool_ops = {
> > .get_drvinfo = airoha_ethtool_get_drvinfo,
> > .get_eth_mac_stats = airoha_ethtool_get_mac_stats,
> > .get_rmon_stats = airoha_ethtool_get_rmon_stats,
> > - .get_link_ksettings = phy_ethtool_get_link_ksettings,
> > + .get_link_ksettings = airoha_ethtool_get_link_ksettings,
> > + .set_link_ksettings = airoha_ethtool_set_link_ksettings,
> > .get_link = ethtool_op_get_link,
> > };
> >
> > @@ -3176,6 +3208,155 @@ bool airoha_is_valid_gdm_dev(struct airoha_eth *eth,
> > return false;
> > }
> >
> > +/* Nothing to do in MAC, everything is handled in PCS */
> > +static void airoha_mac_config(struct phylink_config *config, unsigned int mode,
> > + const struct phylink_link_state *state)
> > +{
> > +}
> > +
> > +static void airoha_mac_link_up(struct phylink_config *config, struct phy_device *phy,
> > + unsigned int mode, phy_interface_t interface,
> > + int speed, int duplex, bool tx_pause, bool rx_pause)
> > +{
> > + struct airoha_gdm_dev *dev = container_of(config, struct airoha_gdm_dev,
> > + phylink_config);
> > + struct airoha_gdm_port *port = dev->port;
> > + struct airoha_eth *eth = dev->eth;
> > + u32 frag_size_tx, frag_size_rx;
> > + u32 mask, val;
> > +
> > + /* TX/RX frag is configured only for GDM4 */
> > + if (port->id != AIROHA_GDM4_IDX)
> > + return;
> > +
> > + switch (speed) {
> > + case SPEED_10000:
> > + case SPEED_5000:
> > + frag_size_tx = 8;
> > + frag_size_rx = 8;
> > + break;
> > + case SPEED_2500:
> > + frag_size_tx = 2;
> > + frag_size_rx = 1;
> > + break;
> > + default:
> > + frag_size_tx = 1;
> > + frag_size_rx = 0;
> > + }
> > +
> > + spin_lock(&port->lock);
> > +
> > + /* Configure TX/RX frag based on speed */
> > + if (dev->nbq == 1) {
> > + mask = GDM4_SGMII1_TX_FRAG_SIZE_MASK;
> > + val = FIELD_PREP(GDM4_SGMII1_TX_FRAG_SIZE_MASK,
> > + frag_size_tx);
> > + } else {
> > + mask = GDM4_SGMII0_TX_FRAG_SIZE_MASK;
> > + val = FIELD_PREP(GDM4_SGMII0_TX_FRAG_SIZE_MASK,
> > + frag_size_tx);
> > + }
> > + airoha_fe_rmw(eth, REG_FE_GDM4_TMBI_FRAG, mask, val);
> > +
> > + if (dev->nbq == 1) {
> > + mask = GDM4_SGMII1_RX_FRAG_SIZE_MASK;
> > + val = FIELD_PREP(GDM4_SGMII1_RX_FRAG_SIZE_MASK,
> > + frag_size_rx);
> > + } else {
> > + mask = GDM4_SGMII0_RX_FRAG_SIZE_MASK;
> > + val = FIELD_PREP(GDM4_SGMII0_RX_FRAG_SIZE_MASK,
> > + frag_size_rx);
> > + }
> > + airoha_fe_rmw(eth, REG_FE_GDM4_RMBI_FRAG, mask, val);
> > +
> > + spin_unlock(&port->lock);
> > +}
> > +
> > +/* Nothing to do in MAC, everything is handled in PCS */
> > +static void airoha_mac_link_down(struct phylink_config *config, unsigned int mode,
> > + phy_interface_t interface)
> > +{
> > +}
> > +
> > +static const struct phylink_mac_ops airoha_phylink_ops = {
> > + .mac_config = airoha_mac_config,
> > + .mac_link_up = airoha_mac_link_up,
> > + .mac_link_down = airoha_mac_link_down,
> > +};
> > +
> > +static int airoha_fill_available_pcs(struct phylink_config *config,
> > + struct phylink_pcs **available_pcs,
> > + unsigned int num_possible_pcs)
> > +{
> > + struct device *dev = config->dev;
> > +
> > + return fwnode_phylink_pcs_parse(dev_fwnode(dev), available_pcs,
> > + num_possible_pcs);
> > +}
> > +
> > +static int airoha_setup_phylink(struct net_device *netdev)
> > +{
> > + struct airoha_gdm_dev *dev = netdev_priv(netdev);
> > + struct device_node *np = netdev->dev.of_node;
> > + struct airoha_gdm_port *port = dev->port;
> > + struct phylink_config *config;
> > + phy_interface_t phy_mode;
> > + struct phylink *phylink;
> > + int err;
> > +
> > + err = of_get_phy_mode(np, &phy_mode);
> > + if (err) {
> > + dev_err(&netdev->dev, "incorrect phy-mode\n");
> > + return err;
> > + }
> > +
> > + config = &dev->phylink_config;
> > + config->dev = &netdev->dev;
> > + config->type = PHYLINK_NETDEV;
> > +
> > + /*
> > + * GDM1 only supports internal for Embedded Switch
> > + * and doesn't require a PCS.
> > + */
> > + if (port->id == AIROHA_GDM1_IDX) {
> > + config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> > + MAC_10000FD;
> > +
> > + __set_bit(PHY_INTERFACE_MODE_INTERNAL,
> > + config->supported_interfaces);
> > + } else {
> > + config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> > + MAC_10 | MAC_100 | MAC_1000 |
> > + MAC_2500FD | MAC_5000FD | MAC_10000FD;
> > +
> > + config->num_possible_pcs = fwnode_phylink_pcs_count(dev_fwnode(config->dev));
> > + config->fill_available_pcs = airoha_fill_available_pcs;
> > +
> > + __set_bit(PHY_INTERFACE_MODE_SGMII,
> > + config->supported_interfaces);
> > + __set_bit(PHY_INTERFACE_MODE_1000BASEX,
> > + config->supported_interfaces);
> > + __set_bit(PHY_INTERFACE_MODE_2500BASEX,
> > + config->supported_interfaces);
> > + __set_bit(PHY_INTERFACE_MODE_10GBASER,
> > + config->supported_interfaces);
> > + __set_bit(PHY_INTERFACE_MODE_USXGMII,
> > + config->supported_interfaces);
> > +
> > + phy_interface_copy(config->pcs_interfaces,
> > + config->supported_interfaces);
> > + }
> > +
> > + phylink = phylink_create(config, of_fwnode_handle(np),
> > + phy_mode, &airoha_phylink_ops);
> > + if (IS_ERR(phylink))
> > + return PTR_ERR(phylink);
> > +
> > + dev->phylink = phylink;
> > +
> > + return 0;
> > +}
> > +
> > static int airoha_alloc_gdm_device(struct airoha_eth *eth,
> > struct airoha_gdm_port *port,
> > int nbq, struct device_node *np)
> > @@ -3239,7 +3420,7 @@ static int airoha_alloc_gdm_device(struct airoha_eth *eth,
> > dev->nbq = nbq;
> > port->devs[index] = dev;
> >
> > - return 0;
> > + return airoha_setup_phylink(netdev);
> > }
> >
> > static int airoha_alloc_gdm_port(struct airoha_eth *eth,
> > @@ -3274,7 +3455,7 @@ static int airoha_alloc_gdm_port(struct airoha_eth *eth,
> > return -ENOMEM;
> >
> > port->id = id;
> > - spin_lock_init(&port->stats_lock);
> > + spin_lock_init(&port->lock);
> > eth->ports[p] = port;
> >
> > err = airoha_metadata_dst_alloc(port);
> > @@ -3471,6 +3652,8 @@ static int airoha_probe(struct platform_device *pdev)
> > netdev = netdev_from_priv(dev);
> > if (netdev->reg_state == NETREG_REGISTERED)
> > unregister_netdev(netdev);
> > + if (dev->phylink)
> > + phylink_destroy(dev->phylink);
> > of_node_put(netdev->dev.of_node);
> > }
> > airoha_metadata_dst_free(port);
> > @@ -3509,6 +3692,7 @@ static void airoha_remove(struct platform_device *pdev)
> >
> > netdev = netdev_from_priv(dev);
> > unregister_netdev(netdev);
> > + phylink_destroy(dev->phylink);
> > of_node_put(netdev->dev.of_node);
> > }
> > airoha_metadata_dst_free(port);
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
> > index f6d01a8e8da1..b49fc5304b3a 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.h
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.h
> > @@ -561,6 +561,9 @@ struct airoha_gdm_dev {
> > int nbq;
> >
> > struct airoha_hw_stats stats;
> > +
> > + struct phylink *phylink;
> > + struct phylink_config phylink_config;
> > };
> >
> > struct airoha_gdm_port {
> > @@ -568,8 +571,8 @@ struct airoha_gdm_port {
> > int id;
> > int users;
> >
> > - /* protect concurrent hw_stats accesses */
> > - spinlock_t stats_lock;
> > + /* protect concurrent hw_stats and frag register accesses */
> > + spinlock_t lock;
> >
> > struct metadata_dst *dsa_meta[AIROHA_MAX_DSA_PORTS];
> > };
> > diff --git a/drivers/net/ethernet/airoha/airoha_regs.h b/drivers/net/ethernet/airoha/airoha_regs.h
> > index 6fed63d013b4..8df02f51211c 100644
> > --- a/drivers/net/ethernet/airoha/airoha_regs.h
> > +++ b/drivers/net/ethernet/airoha/airoha_regs.h
> > @@ -357,6 +357,18 @@
> > #define IP_FRAGMENT_PORT_MASK GENMASK(8, 5)
> > #define IP_FRAGMENT_NBQ_MASK GENMASK(4, 0)
> >
> > +#define REG_FE_GDM4_TMBI_FRAG 0x2028
> > +#define GDM4_SGMII1_TX_WEIGHT_MASK GENMASK(31, 26)
> > +#define GDM4_SGMII1_TX_FRAG_SIZE_MASK GENMASK(25, 16)
> > +#define GDM4_SGMII0_TX_WEIGHT_MASK GENMASK(15, 10)
> > +#define GDM4_SGMII0_TX_FRAG_SIZE_MASK GENMASK(9, 0)
> > +
> > +#define REG_FE_GDM4_RMBI_FRAG 0x202c
> > +#define GDM4_SGMII1_RX_WEIGHT_MASK GENMASK(31, 26)
> > +#define GDM4_SGMII1_RX_FRAG_SIZE_MASK GENMASK(25, 16)
> > +#define GDM4_SGMII0_RX_WEIGHT_MASK GENMASK(15, 10)
> > +#define GDM4_SGMII0_RX_FRAG_SIZE_MASK GENMASK(9, 0)
> > +
> > #define REG_MC_VLAN_EN 0x2100
> > #define MC_VLAN_EN_MASK BIT(0)
> >
> > --
> > 2.53.0
> >
--
Ansuel
^ permalink raw reply
* Re: [PATCH v2 2/2] i3c: master: dw: Drop redundant core reset name
From: Frank Li @ 2026-07-20 15:43 UTC (permalink / raw)
To: Shubham Patil
Cc: alexandre.belloni, Frank.Li, robh, krzk+dt, conor+dt, linux-i3c,
devicetree, linux-kernel, michal.simek, radhey.shyam.pandey
In-Reply-To: <20260720073510.1869623-3-shubhamsanjay.patil@amd.com>
On Mon, Jul 20, 2026 at 01:05:10PM +0530, Shubham Patil wrote:
> The DesignWare I3C master has a single reset line, so a dedicated
> reset name is redundant. Look up the reset by index by passing NULL
> instead of the "core_rst" name.
>
> Signed-off-by: Shubham Patil <shubhamsanjay.patil@amd.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/i3c/master/dw-i3c-master.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 2f8c0c4683e0..488fb455d3cc 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -1584,7 +1584,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
> return PTR_ERR(master->pclk);
>
> master->core_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
> - "core_rst");
> + NULL);
> if (IS_ERR(master->core_rst))
> return PTR_ERR(master->core_rst);
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH v3 2/4] clocksource/drivers: Add support for the Renesas RZ/N1 timers
From: Herve Codina @ 2026-07-20 15:42 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, wsa+renesas, devicetree, conor+dt, robh
In-Reply-To: <20260717100827.592531F000E9@smtp.kernel.org>
Hi,
On Fri, 17 Jul 2026 10:08:26 +0000
sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
> - [High] Off-by-one error in CPU bounds check allows out-of-bounds array access in the hotplug callback.
> - [High] A 16-bit high-frequency clocksource is incorrectly flagged with CLOCK_SOURCE_SUSPEND_NONSTOP, breaking system suspend timekeeping.
> - [High] Probe incorrectly returns a positive IRQ number on devm_request_irq failure.
> - [High] Missing teardown callback in cpuhp_setup_state causes a Use-After-Free of clockevent devices.
> - [Medium] Down-counting timer values are potentially returned directly, breaking the requirement for monotonic increasing counters.
> - [Medium] Failing CPU bringup by returning an error from a CPU hotplug starting callback.
> --
>
> --- Patch [2]: [PATCH v3 2/4] clocksource/drivers: Add support for the Renesas RZ/N1 timers ---
> commit 67eeb3e62e1eb246f2738740b9b5319a484b8d50
> Author: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>
>
> clocksource/drivers: Add support for the Renesas RZ/N1 timers
>
> The Renesas RZ/N1 timer block controller is the controller in charge of
> timers available in the Renesas RZ/N1 SoCs family. This controller handles
> 8 timers: 6 16-bit timers and 2 32-bit timers.
>
> > diff --git a/drivers/clocksource/timer-rzn1.c b/drivers/clocksource/timer-rzn1.c
> > new file mode 100644
> > index 0000000000000..024f01bb0227b
> > --- /dev/null
> > +++ b/drivers/clocksource/timer-rzn1.c
>
> [ ... ]
>
> > +static u64 notrace rzn1_sched_read(void)
> > +{
> > + return readl_relaxed(rzn1_sched_clock->base + RZN1_TIMER_CURRENT_COUNT_REG);
> > +}
> > +
> > +static u64 rzn1_clksrc_read(struct clocksource *cs)
> > +{
> > + return readl_relaxed(rzn1_sched_clock->base + RZN1_TIMER_CURRENT_COUNT_REG);
> > +}
>
> [Severity: Medium]
> Since RZN1_TIMER_LOAD_COUNT_REG is used earlier to set the timer period,
> does this hardware act as a down-counter?
>
> If the hardware counts downward, will returning the raw value here break the
> timekeeping requirement for monotonically increasing values?
The timer counter is an incrementing counter.
It is compared to the LOAD_COUNT register value to trig an interrupt.
>
> > +static struct clocksource rzn1_clocksource = {
> > + .name = "rzn1_timer_clocksource",
> > + .rating = 200,
> > + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
>
> [Severity: High]
> Is it safe to flag a 16-bit clocksource with CLOCK_SOURCE_SUSPEND_NONSTOP?
>
> Since a 16-bit counter fed by a high-frequency clock will wrap around very
> quickly, will this cause massive wall time jumps if the system suspends for
> longer than the wrap interval?
This flag shouldn't be present. Will be removed in the next iteration.
>
> > + .read = rzn1_clksrc_read,
> > +};
> > +
> > +/* Timers table where 32-bit timers will be assigned per-cpu */
> > +static struct rzn1_timer *rzn1_tab_timers;
> > +
> > +static int rzn1_local_timer_starting_cpu(unsigned int cpu)
> > +{
> > + struct rzn1_timer *timer;
> > +
> > + if (cpu > RZN1_TIMER_NB_32BIT_TIMERS)
> > + return -EINVAL;
>
> [Severity: Medium]
> Will returning an error like -EINVAL completely abort the hotplug process
> for any CPU beyond the first two?
>
> Should this return 0 instead, allowing additional CPUs to still boot online
> and rely on a global broadcast timer?
Indeed, returning 0 is better even if we should reach this case on RZ/N1 SoCs.
Will update in the next iteration.
>
> [Severity: High]
> Also, does the greater-than condition here have an off-by-one error?
>
> If RZN1_TIMER_NB_32BIT_TIMERS is 2, a cpu value of 2 bypasses this check.
Will update in the next iteration.
>
> > +
> > + timer = &rzn1_tab_timers[cpu + RZN1_TIMER_BASE_INDEX_32BIT_TIMERS];
>
> [Severity: High]
> If a cpu value of 2 bypassed the check above, will this access out of bounds?
>
> The array is only sized for 8 elements total. Adding a cpu value of 2 to
> RZN1_TIMER_BASE_INDEX_32BIT_TIMERS (which is 6) computes an index of 8.
The cpu value check against RZN1_TIMER_NB_32BIT_TIMERS will be update in the
next iteration. This possible out of bound access will not exist anymore.
>
> [ ... ]
>
> > +static int rzn1_timer_setup_timers(struct platform_device *pdev,
> > + struct rzn1_timer *tab_timers,
> > + unsigned int first_timer_index,
> > + unsigned int last_timer_index,
> > + void __iomem *base, unsigned long clock_rate)
> > +{
>
> [ ... ]
>
> > + ret = devm_request_irq(dev, timer->ced.irq, rzn1_timer_interrupt,
> > + IRQF_TIMER, timer->ced.name, timer);
> > + if (ret < 0)
> > + return dev_err_probe(dev, irq, "timer%u: Failed to request IRQ\n", i);
>
> [Severity: High]
> Will calling dev_err_probe() with irq return the positive IRQ number back to
> the caller instead of the negative ret code?
>
> If the driver probe returns a positive integer, won't the driver core assume
> the probe succeeded despite the failure?
Indeed, will be update in the next iteration.
>
> [ ... ]
>
> > +static int rzn1_timer_probe_first(struct platform_device *pdev,
> > + struct rzn1_timer *tab_timers,
> > + void __iomem *base, unsigned long clock_rate)
> > +{
>
> [ ... ]
>
> > + /*
> > + * Second step, almost all operations that can fail have been called.
> > + * Timers are ready to work. Start with the last operation that can fail,
> > + * installing and invoking hotplug callbacks
> > + */
> > + rzn1_tab_timers = tab_timers;
> > + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> > + "clockevents/rzn1/global_timer:starting",
> > + rzn1_local_timer_starting_cpu, NULL);
>
> [Severity: High]
> Does passing a NULL teardown callback here leave registered clockevent
> devices dangling if a subsequent step fails during probe?
>
> If the driver fails and frees its timer structures via devres, or if a CPU
> is dynamically offlined later, will this lack of teardown cause a
> use-after-free in the clockevents core?
The probe cannot fail after the call to cpuhp_setup_state(). This has been taken
into account. The only case to handle is offlining a CPU (which I am not sure
it could happen in embedded system using a RZ/N1 SoC).
Anyway, will add the teardown function and:
- Adapt the probe to set IRQ_NOAUTOEN flag on interrupt related to timers
handled by hotplug.
- Enable irq hotplug startup function.
- Disable irq in teardown function.
Best regards,
Hervé
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: i3c: dw: Document missing optional core reset
From: Frank Li @ 2026-07-20 15:42 UTC (permalink / raw)
To: Shubham Patil
Cc: alexandre.belloni, Frank.Li, robh, krzk+dt, conor+dt, linux-i3c,
devicetree, linux-kernel, michal.simek, radhey.shyam.pandey
In-Reply-To: <20260720073510.1869623-2-shubhamsanjay.patil@amd.com>
On Mon, Jul 20, 2026 at 01:05:09PM +0530, Shubham Patil wrote:
> The DesignWare I3C master IP supports a core reset input, but the
> binding is missing the corresponding reset description.
>
> Document the optional reset property. Keep it optional because the
> reset line is integration-specific.
>
> Signed-off-by: Shubham Patil <shubhamsanjay.patil@amd.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Changes in v2:
> - Updated commit description
> - Dropped reset-name from the binding
> ---
> Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> index e803457d3f55..dc7ee38001d6 100644
> --- a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> +++ b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> @@ -38,6 +38,9 @@ properties:
> interrupts:
> maxItems: 1
>
> + resets:
> + maxItems: 1
> +
> power-domains:
> maxItems: 1
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH 2/5] arm64: dts: qcom: Add Kalambo SoC
From: Konrad Dybcio @ 2026-07-20 15:41 UTC (permalink / raw)
To: Gopikrishna Garmidi, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sibi Sankar, Pankaj Patil
Cc: Rajendra Nayak, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <782d626c-2545-4719-9833-efbc9ba82871@oss.qualcomm.com>
On 7/20/26 5:04 PM, Gopikrishna Garmidi wrote:
>
> Hi Konrad,
>
> Thanks a lot for the review and for catching these
>> On 7/18/26 2:06 PM, Gopikrishna Garmidi wrote:
>>> Introduce support for the Qualcomm Kalambo SoC. It is derived from Mahua
>>> with CPU cluster 0 removed, leaving it with just cluster 1. As a result,
>>> the PDP mailbox and SCP low-priority register windows are updated to
>>> match the reduced CPU complex. Everything else should work as-is.
[...]
> Does this look right to you, or would you prefer re-pointing cluster0's cores in place rather than delete + re-add?
Let's just delete the whole map and re-define it, I think
that's the least painful way to do it
>
>>
>> [...]
>>
>>> +&pdp0_mbox {
>>> + reg = <0 0x17610000 0 0x8000>, <0 0x1a980000 0 0x8000>;
>>> +};
>>
>> Are you sure about this change?
>
> Yes — confirmed against the SoC memory map. The Tx window is the NCC0/NCC1 mailbox instance; dropping the former cluster0 moves the Tx base from NCC0 (0x19980000) to NCC1 (0x1a980000), while reg[0] (Rx) is shared and unchanged. I'll note this in the commit message.
Got it, I overlooked the 0/1 difference in the register name when
comparing
Konrad
^ permalink raw reply
* Re: [RFC PATCH 1/3] dt-bindings: display: Add GlandaGPU binding
From: Conor Dooley @ 2026-07-20 15:37 UTC (permalink / raw)
To: Leander Kieweg
Cc: dri-devel, devicetree, airlied, simona, maarten.lankhorst,
mripard, tzimmermann, robh, krzk+dt, conor+dt
In-Reply-To: <CAMwcS0hBc_ruk_r+F6cQHQzO+jZagi3M9_AHQ3fpT7cbSUqeOg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]
On Sat, Jul 18, 2026 at 12:39:19AM +0200, Leander Kieweg wrote:
> Hi Conor,
>
> Thanks for the review.
>
> > Sadly googling this device only shows up Phoronix and other copy-paste
> > reporting sites. I think it'd probably be good to include a link to your
> > project either here or in the commit message.
>
> Good point either way. The link was in the cover letter, but I agree
> it makes sense to have it directly in the description too, so it's
> not something you have to go dig up separately when looking at just
> this patch.
>
> > This clock sounds like it is actually required? Doubt the device works
> > without it!
> > pw-bot: changes-requested
>
> You're right, I'll move clocks into the required list.
>
> > Usually I would say do not include things if you don't have to, but I
> > would much rather you did if the alternative is an interrupts property
> > like this that doesn't resemble anything used in devicetrees.
>
> The example was actually taken from my DE10-Standard DTS, just
> shortened, since I understood examples are generally expected to be
> minimal. For v2 I'll add interrupt-parent back in and use the
> GIC_SPI/IRQ_TYPE_LEVEL_HIGH macros from
> dt-bindings/interrupt-controller/arm-gic.h instead of raw numbers:
>
All of this sounds good, thanks.
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> gpu@c0000000 {
> compatible = "glanda,gpu-1.0";
> reg = <0xc0000000 0x1000000>;
> interrupt-parent = <&intc>;
> interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&osc1>;
> };
>
> Thanks again,
> Leander
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY
From: sashiko-bot @ 2026-07-20 15:37 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: conor+dt, neil.armstrong, olteanv, linux-phy, vkoul, devicetree,
robh
In-Reply-To: <20260720152654.92216-3-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The driver emits a false-positive kernel error log when NVMEM is disabled or unsupported.
--
--- Patch [2]: [PATCH v3 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY ---
commit 93ce035a5e3a24b4c6f63697869bbcb5d8e50db4
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Subject: phy: mediatek: Add support for PCI-Express Gen4 S-PHY
This commit adds support for the PCI-Express Gen4 S-PHY found in the latest
MediaTek SoCs, including MT8196, MT8894 and similar.
[ ... ]
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie-sphy.c b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
> new file mode 100644
> index 0000000000000..853aafa182597
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
[ ... ]
> +static int mtk_pcie_sphy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
> +{
> + u16 buf;
> + u8 tmp;
> + int ret;
> +
> + /*
> + * All of the calibrations are always max 8 bits long, but some may
> + * be split between two different 8-bits cells: handle this corner
> + * case by retrying reading as u16.
> + */
> + ret = nvmem_cell_read_u8(dev, name, &tmp);
> + if (ret == 0)
> + buf = tmp;
> + else
> + ret = nvmem_cell_read_u16(dev, name, &buf);
> +
> + if (ret == -ENOENT) {
> + dev_info(dev, "No calibration for %s. Using defaults\n", name);
> + return -ENOENT;
> + } else if (ret)
> + return dev_err_probe(dev, ret,
> + "Cannot get calibration data for %s\n", name);
[Severity: Low]
Does this emit a false-positive kernel error log when NVMEM is disabled or
unsupported?
If CONFIG_NVMEM=n, the NVMEM APIs return -EOPNOTSUPP, which will cause this
path to print an error with dev_err_probe(). However, the caller in
mtk_pcie_sphy_get_calibration_data() gracefully handles -EOPNOTSUPP as a
non-error condition:
if (ret < 0) {
/*
* If any of the calibration values is missing, or if there is
* no calibration at all in the eFuses, this is not a problem,
* as the PHY doesn't require one to actually work.
*/
if (ret == -ENOENT || ret == -EOPNOTSUPP) {
cal->supported = false;
return 0;
}
Could -EOPNOTSUPP be handled silently here in mtk_pcie_sphy_get_one_cal_para()
to avoid spurious error messages during boot?
> +
> + if (buf > max_val)
> + return dev_err_probe(dev, -ERANGE,
> + "Bad value %u retrieved for %s.\n", buf, name);
> +
> + return buf;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720152654.92216-1-angelogioacchino.delregno@collabora.com?part=2
^ permalink raw reply
* Re: [PATCH net-next v9 12/12] net: airoha: add phylink support
From: Lorenzo Bianconi @ 2026-07-20 15:35 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Simon Horman, Jonathan Corbet, Shuah Khan, Heiner Kallweit,
Russell King, Saravana Kannan, Philipp Zabel, netdev, devicetree,
linux-kernel, linux-doc, linux-arm-kernel, linux-mediatek,
Maxime Chevallier
In-Reply-To: <20260717065448.1498335-13-ansuelsmth@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 12669 bytes --]
> Add phylink support for each GDM port. For GDM1 add the internal interface
> mode as the only supported mode. For GDM2/3/4 add the required
> configuration of the PCS to make the external PHY or attached SFP cage
> work.
>
> These needs to be defined in the GDM port node using the pcs-handle
> property.
>
> Update and provide a .get/set_link_ksettings function that use phylink
> for ethtool OPs now that we fully support phylink.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> drivers/net/ethernet/airoha/Kconfig | 1 +
> drivers/net/ethernet/airoha/airoha_eth.c | 194 +++++++++++++++++++++-
> drivers/net/ethernet/airoha/airoha_eth.h | 7 +-
> drivers/net/ethernet/airoha/airoha_regs.h | 12 ++
> 4 files changed, 207 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/airoha/Kconfig b/drivers/net/ethernet/airoha/Kconfig
> index 1f6640a15fc9..789906516bf8 100644
> --- a/drivers/net/ethernet/airoha/Kconfig
> +++ b/drivers/net/ethernet/airoha/Kconfig
> @@ -20,6 +20,7 @@ config NET_AIROHA
> depends on NET_DSA || !NET_DSA
> select NET_AIROHA_NPU
> select PAGE_POOL
> + select PHYLINK
> help
> This driver supports the gigabit ethernet MACs in the
> Airoha SoC family.
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 59001fd4b6f7..ed1ac032f337 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -8,6 +8,7 @@
> #include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
> #include <linux/tcp.h>
> +#include <linux/pcs/pcs.h>
> #include <linux/u64_stats_sync.h>
> #include <net/dst_metadata.h>
> #include <net/page_pool/helpers.h>
> @@ -1837,7 +1838,7 @@ static void airoha_update_hw_stats(struct airoha_gdm_dev *dev)
> struct airoha_gdm_port *port = dev->port;
> int i;
>
> - spin_lock(&port->stats_lock);
> + spin_lock(&port->lock);
Hi Christian,
as pointed out in a previous email, I do not like the approach of reusing this
spin_lock for airoha_mac_link_up(). Can we use rtl_lock() (when necessary) as
pointed out before?
Regards,
Lorenzo
>
> for (i = 0; i < ARRAY_SIZE(port->devs); i++) {
> if (port->devs[i])
> @@ -1848,7 +1849,7 @@ static void airoha_update_hw_stats(struct airoha_gdm_dev *dev)
> airoha_fe_set(dev->eth, REG_FE_GDM_MIB_CLEAR(port->id),
> FE_GDM_MIB_RX_CLEAR_MASK | FE_GDM_MIB_TX_CLEAR_MASK);
>
> - spin_unlock(&port->stats_lock);
> + spin_unlock(&port->lock);
> }
>
> static void airoha_dev_set_xmit_frame_size(struct net_device *netdev)
> @@ -1870,6 +1871,14 @@ static int airoha_dev_open(struct net_device *netdev)
> u32 pse_port = FE_PSE_PORT_PPE1;
> int err;
>
> + err = phylink_of_phy_connect(dev->phylink, netdev->dev.of_node, 0);
> + if (err) {
> + netdev_err(netdev, "could not attach PHY: %d\n", err);
> + return err;
> + }
> +
> + phylink_start(dev->phylink);
> +
> netif_tx_start_all_queues(netdev);
> err = airoha_set_vip_for_gdm_port(dev, true);
> if (err)
> @@ -1909,6 +1918,10 @@ static int airoha_dev_stop(struct net_device *netdev)
> airoha_set_gdm_port_fwd_cfg(qdma->eth,
> REG_GDM_FWD_CFG(port->id),
> FE_PSE_PORT_DROP);
> +
> + phylink_stop(dev->phylink);
> + phylink_disconnect_phy(dev->phylink);
> +
> return 0;
> }
>
> @@ -2389,6 +2402,24 @@ airoha_ethtool_get_rmon_stats(struct net_device *netdev,
> } while (u64_stats_fetch_retry(&dev->stats.syncp, start));
> }
>
> +static int
> +airoha_ethtool_get_link_ksettings(struct net_device *netdev,
> + struct ethtool_link_ksettings *cmd)
> +{
> + struct airoha_gdm_dev *dev = netdev_priv(netdev);
> +
> + return phylink_ethtool_ksettings_get(dev->phylink, cmd);
> +}
> +
> +static int
> +airoha_ethtool_set_link_ksettings(struct net_device *netdev,
> + const struct ethtool_link_ksettings *cmd)
> +{
> + struct airoha_gdm_dev *dev = netdev_priv(netdev);
> +
> + return phylink_ethtool_ksettings_set(dev->phylink, cmd);
> +}
> +
> static int airoha_qdma_set_chan_tx_sched(struct net_device *netdev,
> int channel, enum tx_sched_mode mode,
> const u16 *weights, u8 n_weights)
> @@ -3120,7 +3151,8 @@ static const struct ethtool_ops airoha_ethtool_ops = {
> .get_drvinfo = airoha_ethtool_get_drvinfo,
> .get_eth_mac_stats = airoha_ethtool_get_mac_stats,
> .get_rmon_stats = airoha_ethtool_get_rmon_stats,
> - .get_link_ksettings = phy_ethtool_get_link_ksettings,
> + .get_link_ksettings = airoha_ethtool_get_link_ksettings,
> + .set_link_ksettings = airoha_ethtool_set_link_ksettings,
> .get_link = ethtool_op_get_link,
> };
>
> @@ -3176,6 +3208,155 @@ bool airoha_is_valid_gdm_dev(struct airoha_eth *eth,
> return false;
> }
>
> +/* Nothing to do in MAC, everything is handled in PCS */
> +static void airoha_mac_config(struct phylink_config *config, unsigned int mode,
> + const struct phylink_link_state *state)
> +{
> +}
> +
> +static void airoha_mac_link_up(struct phylink_config *config, struct phy_device *phy,
> + unsigned int mode, phy_interface_t interface,
> + int speed, int duplex, bool tx_pause, bool rx_pause)
> +{
> + struct airoha_gdm_dev *dev = container_of(config, struct airoha_gdm_dev,
> + phylink_config);
> + struct airoha_gdm_port *port = dev->port;
> + struct airoha_eth *eth = dev->eth;
> + u32 frag_size_tx, frag_size_rx;
> + u32 mask, val;
> +
> + /* TX/RX frag is configured only for GDM4 */
> + if (port->id != AIROHA_GDM4_IDX)
> + return;
> +
> + switch (speed) {
> + case SPEED_10000:
> + case SPEED_5000:
> + frag_size_tx = 8;
> + frag_size_rx = 8;
> + break;
> + case SPEED_2500:
> + frag_size_tx = 2;
> + frag_size_rx = 1;
> + break;
> + default:
> + frag_size_tx = 1;
> + frag_size_rx = 0;
> + }
> +
> + spin_lock(&port->lock);
> +
> + /* Configure TX/RX frag based on speed */
> + if (dev->nbq == 1) {
> + mask = GDM4_SGMII1_TX_FRAG_SIZE_MASK;
> + val = FIELD_PREP(GDM4_SGMII1_TX_FRAG_SIZE_MASK,
> + frag_size_tx);
> + } else {
> + mask = GDM4_SGMII0_TX_FRAG_SIZE_MASK;
> + val = FIELD_PREP(GDM4_SGMII0_TX_FRAG_SIZE_MASK,
> + frag_size_tx);
> + }
> + airoha_fe_rmw(eth, REG_FE_GDM4_TMBI_FRAG, mask, val);
> +
> + if (dev->nbq == 1) {
> + mask = GDM4_SGMII1_RX_FRAG_SIZE_MASK;
> + val = FIELD_PREP(GDM4_SGMII1_RX_FRAG_SIZE_MASK,
> + frag_size_rx);
> + } else {
> + mask = GDM4_SGMII0_RX_FRAG_SIZE_MASK;
> + val = FIELD_PREP(GDM4_SGMII0_RX_FRAG_SIZE_MASK,
> + frag_size_rx);
> + }
> + airoha_fe_rmw(eth, REG_FE_GDM4_RMBI_FRAG, mask, val);
> +
> + spin_unlock(&port->lock);
> +}
> +
> +/* Nothing to do in MAC, everything is handled in PCS */
> +static void airoha_mac_link_down(struct phylink_config *config, unsigned int mode,
> + phy_interface_t interface)
> +{
> +}
> +
> +static const struct phylink_mac_ops airoha_phylink_ops = {
> + .mac_config = airoha_mac_config,
> + .mac_link_up = airoha_mac_link_up,
> + .mac_link_down = airoha_mac_link_down,
> +};
> +
> +static int airoha_fill_available_pcs(struct phylink_config *config,
> + struct phylink_pcs **available_pcs,
> + unsigned int num_possible_pcs)
> +{
> + struct device *dev = config->dev;
> +
> + return fwnode_phylink_pcs_parse(dev_fwnode(dev), available_pcs,
> + num_possible_pcs);
> +}
> +
> +static int airoha_setup_phylink(struct net_device *netdev)
> +{
> + struct airoha_gdm_dev *dev = netdev_priv(netdev);
> + struct device_node *np = netdev->dev.of_node;
> + struct airoha_gdm_port *port = dev->port;
> + struct phylink_config *config;
> + phy_interface_t phy_mode;
> + struct phylink *phylink;
> + int err;
> +
> + err = of_get_phy_mode(np, &phy_mode);
> + if (err) {
> + dev_err(&netdev->dev, "incorrect phy-mode\n");
> + return err;
> + }
> +
> + config = &dev->phylink_config;
> + config->dev = &netdev->dev;
> + config->type = PHYLINK_NETDEV;
> +
> + /*
> + * GDM1 only supports internal for Embedded Switch
> + * and doesn't require a PCS.
> + */
> + if (port->id == AIROHA_GDM1_IDX) {
> + config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> + MAC_10000FD;
> +
> + __set_bit(PHY_INTERFACE_MODE_INTERNAL,
> + config->supported_interfaces);
> + } else {
> + config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> + MAC_10 | MAC_100 | MAC_1000 |
> + MAC_2500FD | MAC_5000FD | MAC_10000FD;
> +
> + config->num_possible_pcs = fwnode_phylink_pcs_count(dev_fwnode(config->dev));
> + config->fill_available_pcs = airoha_fill_available_pcs;
> +
> + __set_bit(PHY_INTERFACE_MODE_SGMII,
> + config->supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_1000BASEX,
> + config->supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_2500BASEX,
> + config->supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_10GBASER,
> + config->supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_USXGMII,
> + config->supported_interfaces);
> +
> + phy_interface_copy(config->pcs_interfaces,
> + config->supported_interfaces);
> + }
> +
> + phylink = phylink_create(config, of_fwnode_handle(np),
> + phy_mode, &airoha_phylink_ops);
> + if (IS_ERR(phylink))
> + return PTR_ERR(phylink);
> +
> + dev->phylink = phylink;
> +
> + return 0;
> +}
> +
> static int airoha_alloc_gdm_device(struct airoha_eth *eth,
> struct airoha_gdm_port *port,
> int nbq, struct device_node *np)
> @@ -3239,7 +3420,7 @@ static int airoha_alloc_gdm_device(struct airoha_eth *eth,
> dev->nbq = nbq;
> port->devs[index] = dev;
>
> - return 0;
> + return airoha_setup_phylink(netdev);
> }
>
> static int airoha_alloc_gdm_port(struct airoha_eth *eth,
> @@ -3274,7 +3455,7 @@ static int airoha_alloc_gdm_port(struct airoha_eth *eth,
> return -ENOMEM;
>
> port->id = id;
> - spin_lock_init(&port->stats_lock);
> + spin_lock_init(&port->lock);
> eth->ports[p] = port;
>
> err = airoha_metadata_dst_alloc(port);
> @@ -3471,6 +3652,8 @@ static int airoha_probe(struct platform_device *pdev)
> netdev = netdev_from_priv(dev);
> if (netdev->reg_state == NETREG_REGISTERED)
> unregister_netdev(netdev);
> + if (dev->phylink)
> + phylink_destroy(dev->phylink);
> of_node_put(netdev->dev.of_node);
> }
> airoha_metadata_dst_free(port);
> @@ -3509,6 +3692,7 @@ static void airoha_remove(struct platform_device *pdev)
>
> netdev = netdev_from_priv(dev);
> unregister_netdev(netdev);
> + phylink_destroy(dev->phylink);
> of_node_put(netdev->dev.of_node);
> }
> airoha_metadata_dst_free(port);
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
> index f6d01a8e8da1..b49fc5304b3a 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.h
> +++ b/drivers/net/ethernet/airoha/airoha_eth.h
> @@ -561,6 +561,9 @@ struct airoha_gdm_dev {
> int nbq;
>
> struct airoha_hw_stats stats;
> +
> + struct phylink *phylink;
> + struct phylink_config phylink_config;
> };
>
> struct airoha_gdm_port {
> @@ -568,8 +571,8 @@ struct airoha_gdm_port {
> int id;
> int users;
>
> - /* protect concurrent hw_stats accesses */
> - spinlock_t stats_lock;
> + /* protect concurrent hw_stats and frag register accesses */
> + spinlock_t lock;
>
> struct metadata_dst *dsa_meta[AIROHA_MAX_DSA_PORTS];
> };
> diff --git a/drivers/net/ethernet/airoha/airoha_regs.h b/drivers/net/ethernet/airoha/airoha_regs.h
> index 6fed63d013b4..8df02f51211c 100644
> --- a/drivers/net/ethernet/airoha/airoha_regs.h
> +++ b/drivers/net/ethernet/airoha/airoha_regs.h
> @@ -357,6 +357,18 @@
> #define IP_FRAGMENT_PORT_MASK GENMASK(8, 5)
> #define IP_FRAGMENT_NBQ_MASK GENMASK(4, 0)
>
> +#define REG_FE_GDM4_TMBI_FRAG 0x2028
> +#define GDM4_SGMII1_TX_WEIGHT_MASK GENMASK(31, 26)
> +#define GDM4_SGMII1_TX_FRAG_SIZE_MASK GENMASK(25, 16)
> +#define GDM4_SGMII0_TX_WEIGHT_MASK GENMASK(15, 10)
> +#define GDM4_SGMII0_TX_FRAG_SIZE_MASK GENMASK(9, 0)
> +
> +#define REG_FE_GDM4_RMBI_FRAG 0x202c
> +#define GDM4_SGMII1_RX_WEIGHT_MASK GENMASK(31, 26)
> +#define GDM4_SGMII1_RX_FRAG_SIZE_MASK GENMASK(25, 16)
> +#define GDM4_SGMII0_RX_WEIGHT_MASK GENMASK(15, 10)
> +#define GDM4_SGMII0_RX_FRAG_SIZE_MASK GENMASK(9, 0)
> +
> #define REG_MC_VLAN_EN 0x2100
> #define MC_VLAN_EN_MASK BIT(0)
>
> --
> 2.53.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v3 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY
From: AngeloGioacchino Del Regno @ 2026-07-20 15:26 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, linux-arm-kernel, linux-mediatek,
linux-phy, devicetree, linux-kernel, kernel
In-Reply-To: <20260720152654.92216-1-angelogioacchino.delregno@collabora.com>
Add support for the PCI-Express Gen4 S-PHY found in the latest
MediaTek SoCs, including MT8196, MT8894 and similar.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/phy/mediatek/Kconfig | 9 +
drivers/phy/mediatek/Makefile | 1 +
drivers/phy/mediatek/phy-mtk-pcie-sphy.c | 556 +++++++++++++++++++++++
3 files changed, 566 insertions(+)
create mode 100644 drivers/phy/mediatek/phy-mtk-pcie-sphy.c
diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index ba6461350951..77236f3084c8 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -13,6 +13,15 @@ config PHY_MTK_PCIE
callback for PCIe GEN3 port, it supports software efuse
initialization.
+config PHY_MTK_PCIE_SPHY
+ tristate "MediaTek PCIe S-PHY Driver"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ depends on OF
+ select GENERIC_PHY
+ help
+ Say 'Y' here to add support for MediaTek PCIe S-PHY driver for
+ PCI-Express Gen4 controllers as found in MT6991, MT8196 and others.
+
config PHY_MTK_XFI_TPHY
tristate "MediaTek 10GE SerDes XFI T-PHY driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index ed0da708759b..7e984c7cfea3 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -5,6 +5,7 @@
obj-$(CONFIG_PHY_MTK_DP) += phy-mtk-dp.o
obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
+obj-$(CONFIG_PHY_MTK_PCIE_SPHY) += phy-mtk-pcie-sphy.o
obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
diff --git a/drivers/phy/mediatek/phy-mtk-pcie-sphy.c b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
new file mode 100644
index 000000000000..853aafa18259
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
@@ -0,0 +1,556 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 MediaTek Inc.
+ * Copyright (c) 2026 Collabora Ltd.
+ * AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+
+#include "phy-mtk-io.h"
+
+/* PHY System Interface (SIF) registers */
+#define PEXTP_DIG_GLB_TOP 0x20
+# define RG_XTP_BYPASS_PIPE_RST_RC BIT(17)
+#define PEXTP_DIG_GLB_CKBG0 0x30
+# define RG_XTP_CKBG_XTAL_STABLE_TIME_SEL GENMASK(25, 16)
+#define PEXTP_DIG_GLB_TPLL_CTL0 0x38
+# define RG_XTP_TPLL_SET_STABLE_TIME_SEL GENMASK(7, 2)
+# define RG_XTP_TPLL_PWE_ON_STABLE_TIME_SEL GENMASK(9, 8)
+#define PEXTP_DIG_GLB_CLKREQ_CTL 0x50
+# define RG_XTP_CKM_EN_L1S0 BIT(13)
+# define RG_XTP_CKM_EN_L1S1 BIT(14)
+#define PEXTP_DIG_GLB_TPLL_CTL2 0xf4
+# define RG_XTP_TPLL_ISO_EN_STABLE_TIME_SEL GENMASK(13, 12)
+
+/* PHY System Interface Digital registers */
+#define PEXTP_DIG_LN_TRX_PIPE_IF_17 0x30e8
+# define RG_XTP_LN_RX_LF_CTLE_CSEL_GEN4 GENMASK(14, 12)
+#define PEXTP_DIG_LN_RX_F0 0x50f0
+# define RG_XTP_LN_RX_GEN1_CTLE1_CSEL GENMASK(3, 0)
+# define RG_XTP_LN_RX_GEN2_CTLE1_CSEL GENMASK(7, 4)
+# define RG_XTP_LN_RX_GEN3_CTLE1_CSEL GENMASK(11, 8)
+# define RG_XTP_LN_RX_GEN4_CTLE1_CSEL GENMASK(15, 12)
+#define PEXTP_DIG_LN_RX2_AEQ_EDGE_0 0x6004
+# define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3 GENMASK(21, 16)
+# define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4 GENMASK(29, 24)
+
+/* PHY System Interface Analog registers */
+#define PEXTP_ANA_GLB_TPLL1_RSVD 0x902c
+# define RG_XTP_GLB_TPLL1_P_PATH_GAIN GENMASK(2, 0)
+#define PEXTP_ANA_GLB_BIAS_0 0x9060
+# define RG_XTP_GLB_BIAS_INTR_CTRL GENMASK(5, 0)
+#define PEXTP_ANA_GLB_BIAS_1 0x90c0
+# define RG_XTP_GLB_BIAS_V2V_VTRIM GENMASK(9, 6)
+#define PEXTP_ANA_LN_TRX_0C 0xa00c
+# define RG_XTP_LN_TX_RSWN_IMPSEL GENMASK(20, 16)
+#define PEXTP_ANA_LN_TRX_34 0xa034
+# define RG_XTP_LN_RX_FE BIT(15)
+#define PEXTP_ANA_LN_TRX_6C 0xa06c
+# define RG_XTP_LN_RX_AEQ_CTLE_ERR_TYPE GENMASK(14, 13)
+# define AEQ_CTLE_SEARCH_ERR_TYPE_H1P5 0
+# define AEQ_CTLE_SEARCH_ERR_TYPE_H1P5_H2P5 1
+# define AEQ_CTLE_SEARCH_ERR_TYPE_P1P5_H2P5_H3P5 2
+#define PEXTP_ANA_LN_TRX_A0 0xa0a0
+# define RG_XTP_LN_TX_IMPSEL_PMOS GENMASK(4, 0)
+# define RG_XTP_LN_TX_IMPSEL_NMOS GENMASK(11, 7)
+# define RG_XTP_LN_RX_IMPSEL GENMASK(15, 12)
+#define PEXTP_ANA_LN_TRX_A8 0xa0a8
+# define RG_XTP_LN_RX_LEQ_RL_CTLE_CAL GENMASK(6, 2)
+# define RG_XTP_LN_RX_LEQ_RL_VGA_CAL GENMASK(11, 7)
+# define RG_XTP_LN_RX_LEQ_RL_DFE_CAL GENMASK(23, 19)
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_4 0xb004
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_8 0xb008
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_C 0xb00c
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_10 0xb010
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_14 0xb014
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_18 0xb018
+# define RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1 GENMASK(5, 0)
+# define RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0 GENMASK(13, 8)
+# define RG_XTP_LN_TX_LC_PRESET_MGx_Px_CP1 GENMASK(21, 16)
+#define PEXTP_REG_LANE(x) ((x) * 0x100)
+
+/* PHY Clock Management (CKM) registers */
+#define XTP_CKM_FORCE_6 0x38
+# define RG_CKM_BIAS_WAIT_PRD_US GENMASK(21, 16)
+#define XTP_CKM_REG_SPLL_FBKDIV_5 0xd4
+# define RG_CKM_CKTX_IMPSEL_PMOS GENMASK(19, 16)
+# define RG_CKM_CKTX_IMPSEL_NMOS GENMASK(23, 20)
+# define RG_CKM_CKTX_IMPSEL_SW GENMASK(27, 24)
+
+/* Calibration data from eFuses */
+#define MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES 2
+#define MTK_PCIE_SPHY_CALIBRATION_LAST_QUIRK_VER 4
+
+/**
+ * struct mtk_pcie_sphy_imp_sel - Impedance Selection parameters
+ * @pmos: Impedance selection for P-Channel MOSFET
+ * @nmos: Impedance selection for N-Channel MOSFET
+ */
+struct mtk_pcie_sphy_imp_sel {
+ u8 pmos;
+ u8 nmos;
+};
+
+/**
+ * struct mtk_pcie_sphy_efuse - eFuse calibration data for S-PHY
+ * @int_r_ctrl: Internal resistor selection of TX Bias Current
+ * @xtp_vtrim: XTP Bias V2V voltage calibration
+ * @cktx_impsel: SPLL CKTX Impedance Selection (P and N MOSFET)
+ * @cktx_r_mid: SPLL CKTX Intermediate Transition Impedance (Rmid)
+ * @rx_leq_rl_ctle: RX Front-End Return Loss Continuous Time Linear Equalization value
+ * @rx_leq_rl_vga: RX Front-End Return Loss Variable Gain Amplifier value
+ * @rx_leq_rl_dfe: RX Front-End Return Loss Decision Feedback Equalization value
+ * @rx_impsel: RX Impedance Selection
+ * @tx_impsel: TX Impedance Selection (P and N MOSFET)
+ * @tx_rswn_impsel: TX RSWn (Switch Resistance) impedance selection
+ * @supported: eFuse calibration data is supported
+ */
+struct mtk_pcie_sphy_efuse {
+ u8 int_r_ctrl;
+ u8 xtp_vtrim;
+ struct mtk_pcie_sphy_imp_sel cktx_impsel;
+ u8 cktx_r_mid;
+ u8 rx_leq_rl_ctle;
+ u8 rx_leq_rl_vga;
+ u8 rx_leq_rl_dfe;
+ u8 rx_impsel;
+ struct mtk_pcie_sphy_imp_sel tx_impsel;
+ u8 tx_rswn_impsel[MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES];
+ bool supported;
+};
+
+/**
+ * struct mtk_pcie_sphy - PCI-Express S-PHY driver main structure
+ * @dev: Pointer to device structure
+ * @phy: Pointer to generic phy structure
+ * @sif_base: IO mapped register base address of system interface
+ * @ckm_base: IO mapped register base address of clock management interface
+ * @num_lanes: Number of lanes
+ * @calibration: eFuse calibration data for S-PHY
+ */
+struct mtk_pcie_sphy {
+ struct device *dev;
+ struct phy *phy;
+ void __iomem *sif_base;
+ void __iomem *ckm_base;
+ u8 num_lanes;
+ struct mtk_pcie_sphy_efuse calibration;
+};
+
+static void mtk_pcie_sphy_apply_calibration(struct mtk_pcie_sphy *pcie_sphy)
+{
+ struct mtk_pcie_sphy_efuse *cal = &pcie_sphy->calibration;
+ int i;
+
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_ANA_GLB_BIAS_0,
+ RG_XTP_GLB_BIAS_INTR_CTRL, cal->int_r_ctrl);
+
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_ANA_GLB_BIAS_1,
+ RG_XTP_GLB_BIAS_V2V_VTRIM, cal->xtp_vtrim);
+
+ mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_REG_SPLL_FBKDIV_5,
+ RG_CKM_CKTX_IMPSEL_PMOS, cal->cktx_impsel.pmos);
+
+ mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_REG_SPLL_FBKDIV_5,
+ RG_CKM_CKTX_IMPSEL_NMOS, cal->cktx_impsel.nmos);
+
+ mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_REG_SPLL_FBKDIV_5,
+ RG_CKM_CKTX_IMPSEL_SW, cal->cktx_r_mid);
+
+ for (i = 0; i < pcie_sphy->num_lanes; i++) {
+ void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_0C,
+ RG_XTP_LN_TX_RSWN_IMPSEL, cal->tx_rswn_impsel[i]);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A8,
+ RG_XTP_LN_RX_LEQ_RL_CTLE_CAL, cal->rx_leq_rl_ctle);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A8,
+ RG_XTP_LN_RX_LEQ_RL_VGA_CAL, cal->rx_leq_rl_vga);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A8,
+ RG_XTP_LN_RX_LEQ_RL_DFE_CAL, cal->rx_leq_rl_dfe);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+ RG_XTP_LN_RX_IMPSEL, cal->rx_impsel);
+ }
+}
+
+/**
+ * mtk_pcie_sphy_init() - Initialize the PCI-Express S-PHY
+ * @phy: the phy to be initialized
+ *
+ * The hardware settings will be reset during suspend, it should be
+ * reinitialized when the consumer calls phy_init() again on resume.
+ */
+static int mtk_pcie_sphy_init(struct phy *phy)
+{
+ struct mtk_pcie_sphy *pcie_sphy = phy_get_drvdata(phy);
+ struct mtk_pcie_sphy_imp_sel tx_impsel;
+ int i;
+
+ /* Set CKM Bias wait time to 4 microseconds */
+ mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_FORCE_6,
+ RG_CKM_BIAS_WAIT_PRD_US, 4);
+
+ /* TPLL needs 63 ref_ck ticks to stabilize when setting frequency */
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_TPLL_CTL0,
+ RG_XTP_TPLL_SET_STABLE_TIME_SEL, 63);
+
+ /* TPLL needs 3 ref_ck ticks to stabilize when powering on... */
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_TPLL_CTL0,
+ RG_XTP_TPLL_PWE_ON_STABLE_TIME_SEL, 3);
+
+ /* ...and the same goes for setting isolation */
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_TPLL_CTL2,
+ RG_XTP_TPLL_ISO_EN_STABLE_TIME_SEL, 3);
+
+ /* XTAL doesn't need any stabilization time */
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_CKBG0,
+ RG_XTP_CKBG_XTAL_STABLE_TIME_SEL, 0);
+
+ /* Keep pextp_ckm enabled when in L1SS_L1S1 state */
+ mtk_phy_clear_bits(pcie_sphy->sif_base + PEXTP_DIG_GLB_CLKREQ_CTL, RG_XTP_CKM_EN_L1S1);
+
+ /* Set PIPE to reset TPLL */
+ mtk_phy_clear_bits(pcie_sphy->sif_base + PEXTP_DIG_GLB_TOP, RG_XTP_BYPASS_PIPE_RST_RC);
+
+ /* Set TPLL P-Path gain compensation to 1 */
+ mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_ANA_GLB_TPLL1_RSVD,
+ RG_XTP_GLB_TPLL1_P_PATH_GAIN, 1);
+
+ for (i = 0; i < pcie_sphy->num_lanes; i++) {
+ void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
+
+ /* Set RX Lane AEQ CTRL-E Search Error type to h1.5 + h2.5 */
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_6C,
+ RG_XTP_LN_RX_AEQ_CTLE_ERR_TYPE,
+ AEQ_CTLE_SEARCH_ERR_TYPE_H1P5_H2P5);
+
+ mtk_phy_set_bits(sif_lane_base + PEXTP_ANA_LN_TRX_34, RG_XTP_LN_RX_FE);
+
+ /* TRX: Select CTLE1 for RX Lane AutoEQ CTRL-E Setting on Gen4 */
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TRX_PIPE_IF_17,
+ RG_XTP_LN_RX_LF_CTLE_CSEL_GEN4, 1);
+
+ /* Set RX Lane AutoEQ CTRL-E for PCI-Express Gen1 to Gen 4 */
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+ RG_XTP_LN_RX_GEN1_CTLE1_CSEL, 13);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+ RG_XTP_LN_RX_GEN2_CTLE1_CSEL, 13);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+ RG_XTP_LN_RX_GEN3_CTLE1_CSEL, 13);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+ RG_XTP_LN_RX_GEN4_CTLE1_CSEL, 0);
+
+ /* Set RX Lane AutoEQ's Edge EQ Ratio to 22 * 0.0625 = 1.375 */
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
+ RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3, 22);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
+ RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4, 22);
+
+ /* Setup Digital lane TX Link Characteristics Table */
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_4,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 10);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_4,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_CP1, 2);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_8,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 11);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_8,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_CP1, 1);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_C,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 12);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_10,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 13);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_10,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1, 1);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_14,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 11);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_14,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1, 1);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_18,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 10);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_18,
+ RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1, 2);
+ }
+
+ if (pcie_sphy->calibration.supported) {
+ mtk_pcie_sphy_apply_calibration(pcie_sphy);
+
+ tx_impsel.pmos = pcie_sphy->calibration.tx_impsel.pmos;
+ tx_impsel.nmos = pcie_sphy->calibration.tx_impsel.nmos;
+ } else {
+ /* Set P=10, N=9 to prevent EMI if no calibration present */
+ tx_impsel.pmos = 10;
+ tx_impsel.nmos = 9;
+ }
+
+ /* Select TX Impedance on N and P MOSFETs */
+ for (i = 0; i < pcie_sphy->num_lanes; i++) {
+ void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+ RG_XTP_LN_TX_IMPSEL_PMOS, tx_impsel.pmos);
+
+ mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+ RG_XTP_LN_TX_IMPSEL_NMOS, tx_impsel.nmos);
+ }
+
+ return 0;
+}
+
+static const struct phy_ops mtk_pcie_sphy_ops = {
+ .init = mtk_pcie_sphy_init,
+ .owner = THIS_MODULE,
+};
+
+static int mtk_pcie_sphy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
+{
+ u16 buf;
+ u8 tmp;
+ int ret;
+
+ /*
+ * All of the calibrations are always max 8 bits long, but some may
+ * be split between two different 8-bits cells: handle this corner
+ * case by retrying reading as u16.
+ */
+ ret = nvmem_cell_read_u8(dev, name, &tmp);
+ if (ret == 0)
+ buf = tmp;
+ else
+ ret = nvmem_cell_read_u16(dev, name, &buf);
+
+ if (ret == -ENOENT) {
+ dev_info(dev, "No calibration for %s. Using defaults\n", name);
+ return -ENOENT;
+ } else if (ret)
+ return dev_err_probe(dev, ret,
+ "Cannot get calibration data for %s\n", name);
+
+ if (buf > max_val)
+ return dev_err_probe(dev, -ERANGE,
+ "Bad value %u retrieved for %s.\n", buf, name);
+
+ return buf;
+}
+
+static int mtk_pcie_sphy_get_calibration_data(struct mtk_pcie_sphy *pcie_sphy)
+{
+ struct mtk_pcie_sphy_efuse *cal = &pcie_sphy->calibration;
+ struct device *dev = pcie_sphy->dev;
+ u8 version;
+ int ret;
+
+ if (pcie_sphy->num_lanes > MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES) {
+ dev_info(dev, "Skipping PHY calibration for more than %u lanes.\n",
+ MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES);
+ ret = -EOPNOTSUPP;
+ goto end;
+ }
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "int-r",
+ FIELD_MAX(RG_XTP_GLB_BIAS_INTR_CTRL));
+ if (ret < 0)
+ goto end;
+ cal->int_r_ctrl = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "xtp-vtrim",
+ FIELD_MAX(RG_XTP_GLB_BIAS_V2V_VTRIM));
+ if (ret < 0)
+ goto end;
+ cal->xtp_vtrim = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "cktx-pmos",
+ FIELD_MAX(RG_CKM_CKTX_IMPSEL_PMOS));
+ if (ret < 0)
+ goto end;
+ cal->cktx_impsel.pmos = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "cktx-nmos",
+ FIELD_MAX(RG_CKM_CKTX_IMPSEL_NMOS));
+ if (ret < 0)
+ goto end;
+ cal->cktx_impsel.nmos = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "cktx-r-mid",
+ FIELD_MAX(RG_CKM_CKTX_IMPSEL_SW));
+ if (ret < 0)
+ goto end;
+ cal->cktx_r_mid = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "rxfe-lanes-rl-ctle",
+ FIELD_MAX(RG_XTP_LN_RX_LEQ_RL_CTLE_CAL));
+ if (ret < 0)
+ goto end;
+ cal->rx_leq_rl_ctle = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "rxfe-lanes-rl-vga",
+ FIELD_MAX(RG_XTP_LN_RX_LEQ_RL_VGA_CAL));
+ if (ret < 0)
+ goto end;
+ cal->rx_leq_rl_vga = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "rxfe-lanes-rl-dfe",
+ FIELD_MAX(RG_XTP_LN_RX_LEQ_RL_DFE_CAL));
+ if (ret < 0)
+ goto end;
+ cal->rx_leq_rl_dfe = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "rx-lanes-imp",
+ FIELD_MAX(RG_XTP_LN_RX_IMPSEL));
+ if (ret < 0)
+ goto end;
+ cal->rx_impsel = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-lanes-pmos",
+ FIELD_MAX(RG_XTP_LN_TX_IMPSEL_PMOS));
+ if (ret < 0)
+ goto end;
+ cal->tx_impsel.pmos = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-lanes-nmos",
+ FIELD_MAX(RG_XTP_LN_TX_IMPSEL_NMOS));
+ if (ret < 0)
+ goto end;
+ cal->tx_impsel.nmos = ret;
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-ln0-rswn",
+ FIELD_MAX(RG_XTP_LN_TX_RSWN_IMPSEL));
+ if (ret < 0)
+ goto end;
+ cal->tx_rswn_impsel[0] = ret;
+
+ if (pcie_sphy->num_lanes == 2) {
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-ln1-rswn",
+ FIELD_MAX(RG_XTP_LN_TX_RSWN_IMPSEL));
+ if (ret < 0)
+ goto end;
+ cal->tx_rswn_impsel[1] = ret;
+ }
+
+ ret = mtk_pcie_sphy_get_one_cal_para(dev, "cal-version", 15);
+ if (ret < 0)
+ goto end;
+ version = ret;
+
+ /* Quirk for eFuse calibration table versions 0 to 4 */
+ if ((version <= MTK_PCIE_SPHY_CALIBRATION_LAST_QUIRK_VER) &&
+ cal->rx_leq_rl_ctle == 10) {
+ cal->rx_leq_rl_vga = cal->rx_leq_rl_ctle;
+ cal->rx_leq_rl_dfe = cal->rx_leq_rl_ctle;
+ }
+
+end:
+ if (ret < 0) {
+ /*
+ * If any of the calibration values is missing, or if there is
+ * no calibration at all in the eFuses, this is not a problem,
+ * as the PHY doesn't require one to actually work.
+ */
+ if (ret == -ENOENT || ret == -EOPNOTSUPP) {
+ cal->supported = false;
+ return 0;
+ }
+ return ret;
+ };
+ cal->supported = true;
+
+ return 0;
+}
+
+static int mtk_pcie_sphy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct phy_provider *provider;
+ struct mtk_pcie_sphy *pcie_sphy;
+ u32 num_lanes;
+ int ret;
+
+ pcie_sphy = devm_kzalloc(dev, sizeof(*pcie_sphy), GFP_KERNEL);
+ if (!pcie_sphy)
+ return -ENOMEM;
+
+ pcie_sphy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
+ if (IS_ERR(pcie_sphy->sif_base))
+ return dev_err_probe(dev, PTR_ERR(pcie_sphy->sif_base),
+ "Failed to map phy-sif base\n");
+
+ pcie_sphy->ckm_base = devm_platform_ioremap_resource_byname(pdev, "ckm");
+ if (IS_ERR(pcie_sphy->ckm_base))
+ return dev_err_probe(dev, PTR_ERR(pcie_sphy->ckm_base),
+ "Failed to map phy-ckm base\n");
+
+ pcie_sphy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_sphy_ops);
+ if (IS_ERR(pcie_sphy->phy))
+ return dev_err_probe(dev, PTR_ERR(pcie_sphy->phy),
+ "Failed to create PCIe phy\n");
+
+ ret = of_property_read_u32(dev->of_node, "num-lanes", &num_lanes);
+ if (ret)
+ num_lanes = 1;
+ else if (num_lanes > 4)
+ return dev_err_probe(dev, -EINVAL, "Invalid number of lanes.\n");
+
+ pcie_sphy->num_lanes = num_lanes;
+ pcie_sphy->dev = dev;
+
+ ret = mtk_pcie_sphy_get_calibration_data(pcie_sphy);
+ if (ret)
+ return ret;
+
+ phy_set_drvdata(pcie_sphy->phy, pcie_sphy);
+
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
+
+ provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(provider))
+ return dev_err_probe(dev, PTR_ERR(provider),
+ "Could not register PCI-Express S-PHY\n");
+
+ return 0;
+}
+
+static const struct of_device_id mtk_pcie_sphy_of_match[] = {
+ { .compatible = "mediatek,mt8196-pcie-sphy" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_pcie_sphy_of_match);
+
+static struct platform_driver mtk_pcie_sphy_driver = {
+ .probe = mtk_pcie_sphy_probe,
+ .driver = {
+ .name = "mtk-pcie-sphy",
+ .of_match_table = mtk_pcie_sphy_of_match,
+ },
+};
+module_platform_driver(mtk_pcie_sphy_driver);
+
+MODULE_DESCRIPTION("MediaTek PCIe SPHY driver");
+MODULE_AUTHOR("AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>");
+MODULE_LICENSE("GPL");
--
2.55.0
^ permalink raw reply related
* [PATCH v3 1/2] dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY
From: AngeloGioacchino Del Regno @ 2026-07-20 15:26 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, linux-arm-kernel, linux-mediatek,
linux-phy, devicetree, linux-kernel, kernel
In-Reply-To: <20260720152654.92216-1-angelogioacchino.delregno@collabora.com>
This adds bindings for the PCI-Express Gen4 S-PHY found in newer
MediaTek SoCs, such as MT8196 and its variants.
In the current "revision 3", depending on the specific port, this
S-PHY supports up to two lanes of PCI-Express Gen 4 and both EP
and RC modes.
It is not clear whether revisions/versions earlier than 3 have
ever been shipped in any other MediaTek SoC.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
.../phy/mediatek,mt8196-pcie-sphy.yaml | 93 +++++++++++++++++++
1 file changed, 93 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
new file mode 100644
index 000000000000..3fd1bfbc3822
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mediatek,mt8196-pcie-sphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek PCI-Express Gen4 S-PHY
+
+maintainers:
+ - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+
+description:
+ The MediaTek PCI-Express Gen4 S-PHY is capable of data serialization
+ and de-serialization with 8, 10, 128 or 130 bits (16GT/s) encoding
+ and decoding, has analog buffers, elastic buffers and mechanisms for
+ receiver detection.
+ This PHY supports link rates of 2.5, 5.0, 8.0 and 16.0 GT/s per-lane
+ in both Root Complex (RC) and End Point (EP) modes, depending on the
+ chosen port.
+ The Digital PHY (PHYD) part adheres to the Intel PIPE (PHY Interface
+ for the PCIe) specification.
+
+properties:
+ compatible:
+ const: mediatek,mt8196-pcie-sphy
+
+ reg:
+ items:
+ - description: System Interface (SIF) registers
+ - description: Clock Management (CKM) registers
+
+ reg-names:
+ items:
+ - const: sif
+ - const: ckm
+
+ "#phy-cells":
+ const: 0
+
+ nvmem-cells:
+ items:
+ - description: Internal resistor selection of TX Bias Current
+ - description: XTP Bias V2V voltage calibration
+ - description: S-PLL CKTX Impedance Selection (P-MOSFET side)
+ - description: S-PLL CKTX Impedance Selection (N-MOSFET side)
+ - description: S-PLL CKTX Intermediate Transition Impedance
+ - description: RX Front-End Return Loss Continuous Time Linear EQ
+ - description: RX Front-End Return Loss Variable Gain Amplifier
+ - description: RX Front-End Return Loss Decision Feedback EQ
+ - description: RX Impedance Selection
+ - description: TX Impedance Selection (P-MOSFET side)
+ - description: TX Impedance Selection (N-MOSFET side)
+ - description: TX Lane 0 Switch Resistance (RSWn)
+ - description: TX Lane 1 Switch Resistance (RSWn)
+ - description: eFuse calibration table version
+
+ nvmem-cell-names:
+ items:
+ - const: int-r
+ - const: xtp-vtrim
+ - const: cktx-pmos
+ - const: cktx-nmos
+ - const: cktx-r-mid
+ - const: rxfe-lanes-rl-ctle
+ - const: rxfe-lanes-rl-vga
+ - const: rxfe-lanes-rl-dfe
+ - const: rx-lanes-imp
+ - const: tx-lanes-pmos
+ - const: tx-lanes-nmos
+ - const: tx-ln0-rswn
+ - const: tx-ln1-rswn
+ - const: cal-version
+
+ power-domains:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ phy@16900000 {
+ compatible = "mediatek,mt8196-pcie-sphy";
+ reg = <0x16900000 0x10000>, <0x16920000 0x10000>;
+ reg-names = "sif", "ckm";
+ power-domains = <&spm 10>;
+ #phy-cells = <0>;
+ };
--
2.55.0
^ permalink raw reply related
* [PATCH v3 0/2] PHY: Add MediaTek PCI-Express Gen4 S-PHY Driver
From: AngeloGioacchino Del Regno @ 2026-07-20 15:26 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, linux-arm-kernel, linux-mediatek,
linux-phy, devicetree, linux-kernel, kernel
Changes in v3:
- Allowed value 0 in calibration fields (as it's a legal value)
- Fixed duplicated LN_TX_IMPSEL_{P,N}MOS writes
- Allowed -ENOENT for nvmem_cell_read_u{8,16}() calls, as the
calibration data is effectively optional
- Removed two development stage leftover unused definitions
- Moved num_lanes check in mtk_pcie_sphy_get_calibration_data()
to reduce indent in probe function.
Changes in v2:
- Added items/description to reg in binding
- Added missing pm_runtime.h inclusion
- Moved pm_runtime_enable call to before registering PHY
- Fixed pmos/nmos variable size for 5 bits calibration values
- Fixed calibration for single-lane PCIe, as in, the ln1 rswn
calibration nvmem value is ignored in code if it's single
lane and will not return an error; this is due to the fact
that, effectively, single-lane may have a zero calibration
in tx-ln1-rswn which is fine, as that'd be anyway unused
- Changed calibration data errors to dev_err_probe and changed
the "no calibration for ..." message to dev_info instead
This adds a driver for the PCI-Express Gen4 "S-PHY" found in the
Genio MT8894, Kompanio MT8196, Dimensity MT6991 SoCs (which are
all variants of the same chip).
This was successfully tested on MT8894 and MT8196.
AngeloGioacchino Del Regno (2):
dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY
phy: mediatek: Add support for PCI-Express Gen4 S-PHY
.../phy/mediatek,mt8196-pcie-sphy.yaml | 93 +++
drivers/phy/mediatek/Kconfig | 9 +
drivers/phy/mediatek/Makefile | 1 +
drivers/phy/mediatek/phy-mtk-pcie-sphy.c | 556 ++++++++++++++++++
4 files changed, 659 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
create mode 100644 drivers/phy/mediatek/phy-mtk-pcie-sphy.c
--
2.55.0
^ permalink raw reply
* Re: [PATCH v2 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY
From: AngeloGioacchino Del Regno @ 2026-07-20 15:19 UTC (permalink / raw)
To: sashiko-reviews
Cc: neil.armstrong, olteanv, devicetree, vkoul, linux-phy, conor+dt,
robh
In-Reply-To: <20260720141301.482441F00A3A@smtp.kernel.org>
On 7/20/26 16:13, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] Improper handling of missing or unsupported NVMEM calibration data causes spurious error logs and hard probe failures.
> - [Medium] A valid calibration value of 0 is incorrectly treated as missing data, discarding all calibrations.
> - [Low] The TX_IMPSEL registers are updated twice with identical values during initialization.
> - [Low] Macros for AEQ_EGEQ_RATIO are defined but unused in favor of literal magic numbers.
> --
>
> --- Patch [2]: [PATCH v2 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY ---
> commit d91c6c0718440c44adc7e6a83f5aeca706a6c3d5
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Subject: phy: mediatek: Add support for PCI-Express Gen4 S-PHY
>
> This patch adds support for the PCI-Express Gen4 S-PHY found in the latest
> MediaTek SoCs, including MT8196 and MT8894.
>
>> --- a/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
>> +++ b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
> [ ... ]
>> +#define PEXTP_DIG_LN_RX2_AEQ_EDGE_0 0x6004
>> +# define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3 GENMASK(21, 16)
>> +# define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4 GENMASK(29, 24)
>> +# define AEQ_EGEQ_RATIO_GEN3_TO_22 0x16
>> +# define AEQ_EGEQ_RATIO_GEN4_TO_22 0x16
> [ ... ]
>> + mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
>> + RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3, 22);
>> +
>> + mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
>> + RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4, 22);
>
> [Severity: Low]
> Is there a reason the literal 22 is used here instead of the
> AEQ_EGEQ_RATIO_GEN3_TO_22 and AEQ_EGEQ_RATIO_GEN4_TO_22 macros defined
> earlier?
>
I forgot to remove those definitions. Will do for a v3 tomorrow.
> [ ... ]
>> +static void mtk_pcie_sphy_apply_calibration(struct mtk_pcie_sphy *pcie_sphy)
>> +{
> [ ... ]
>> + mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
>> + RG_XTP_LN_TX_IMPSEL_PMOS, cal->tx_impsel.pmos);
>> +
>> + mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
>> + RG_XTP_LN_TX_IMPSEL_NMOS, cal->tx_impsel.nmos);
>> + }
>> +}
>> +
> [ ... ]
>> +static int mtk_pcie_sphy_init(struct phy *phy)
>> +{
> [ ... ]
>> + if (pcie_sphy->calibration.supported) {
>> + mtk_pcie_sphy_apply_calibration(pcie_sphy);
>> +
>> + tx_impsel.pmos = pcie_sphy->calibration.tx_impsel.pmos;
>> + tx_impsel.nmos = pcie_sphy->calibration.tx_impsel.nmos;
>> + } else {
>> + /* Set P=10, N=9 to prevent EMI if no calibration present */
>> + tx_impsel.pmos = 10;
>> + tx_impsel.nmos = 9;
>> + }
>> +
>> + /* Select TX Impedance on N and P MOSFETs */
>> + for (i = 0; i < pcie_sphy->num_lanes; i++) {
>> + void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
>> +
>> + mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
>> + RG_XTP_LN_TX_IMPSEL_PMOS, tx_impsel.pmos);
>> +
>> + mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
>> + RG_XTP_LN_TX_IMPSEL_NMOS, tx_impsel.nmos);
>> + }
>
> [Severity: Low]
> Are these register updates redundant?
>
> When calibration is supported, mtk_pcie_sphy_apply_calibration() already
> writes cal->tx_impsel.pmos and cal->tx_impsel.nmos to PEXTP_ANA_LN_TRX_A0.
>
> Then this loop immediately overwrites the same registers with the exact same
> values.
>
Eh yes, fixed for v3.
> [ ... ]
>> +static int mtk_pcie_sphy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
>> +{
> [ ... ]
>> + ret = nvmem_cell_read_u8(dev, name, &tmp);
>> + if (ret == 0)
>> + buf = tmp;
>> + else
>> + ret = nvmem_cell_read_u16(dev, name, &buf);
>> +
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "Cannot get calibration data for %s\n", name);
>
> [Severity: High]
> Will this cause spurious error logs or fail the probe when calibration data is
> missing or unsupported?
>
> If the system does not have CONFIG_NVMEM enabled, nvmem_cell_read_u8() returns
> -EOPNOTSUPP, which gets logged as an error here.
>
> This error is then passed back up to mtk_pcie_sphy_get_calibration_data(),
> where it is not caught. This will cause the entire driver probe to fail.
>
> Similarly, if the cell is simply missing from the Device Tree, it returns
> -ENOENT. This also triggers an error log via dev_err_probe(), even though
> missing calibration data is explicitly documented as non-fatal and optional.
>
>> +
>> + if (buf == 0) {
>> + dev_info(dev, "No calibration for %s. Using defaults\n", name);
>> + return -ENOENT;
>> + }
>
> [Severity: Medium]
> Is a value of 0 always indicative of missing calibration?
>
> If a valid tuned calibration value is 0, this check will return -ENOENT and
> cause mtk_pcie_sphy_get_calibration_data() to discard all calibrations entirely.
>
> This also seems to prevent the quirk for cal-version == 0 from ever being
> applied:
>
Fixed for v3.
> [ ... ]
>> +static int mtk_pcie_sphy_get_calibration_data(struct mtk_pcie_sphy *pcie_sphy)
>> +{
> [ ... ]
>> + ret = mtk_pcie_sphy_get_one_cal_para(dev, "cal-version", 15);
>> + if (ret < 0)
>> + goto end;
>> + version = ret;
>> +
>> + /* Quirk for eFuse calibration table versions 0 to 4 */
>> + if ((version <= MTK_PCIE_SPHY_CALIBRATION_LAST_QUIRK_VER) &&
>> + cal->rx_leq_rl_ctle == 10) {
>
> Because mtk_pcie_sphy_get_one_cal_para() returns -ENOENT when it reads a 0,
> the version == 0 condition will never be reached.
>
Fixed for v3.
^ permalink raw reply
* Re: [EXT] Re: [PATCH 2/2] arm64: dts: freescale: add i.MX952 FRDM Board support device tree
From: Frank Li @ 2026-07-20 15:07 UTC (permalink / raw)
To: Joseph Guo; +Cc: sashiko-reviews, robh, Frank.Li, conor+dt, imx, devicetree
In-Reply-To: <56975936-9db1-431d-91a5-c038dc16c196@nxp.com>
On Mon, Jul 20, 2026 at 03:03:20PM +0800, Joseph Guo wrote:
> On 7/20/2026 2:55 PM, sashiko-bot@kernel.org wrote:
> > [You don't often get email from sashiko-bot@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
> >
> >
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [High] Shared GPIO requested exclusively by two CAN transceivers causes probe failure for the second transceiver.
> > - [Low] Use of undocumented, downstream-only device tree property `fsl,cd-gpio-wakeup-disable`.
> > --
> >
> > --- Patch [2]: [PATCH 2/2] arm64: dts: freescale: add i.MX952 FRDM Board support device tree ---
> > commit 8bbcb28d08a05e0f4c57023333492b4e023f7caf
> > Author: Joseph Guo <qijian.guo@nxp.com>
> >
> > arm64: dts: freescale: add i.MX952 FRDM Board support device tree
> >
> > Add basic device tree support for i.MX952 FRDM Board. The board-common hardware
> > description is placed in imx952-frdm.dtsi so that it can be shared with future
> > i.MX952-based FRDM derivative boards.
> >
> >> diff --git a/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi b/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi
> >> new file mode 100644
> >> index 0000000000000..b9ef6bec2bd5a
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi
> >> @@ -0,0 +1,727 @@
> > [ ... ]
> >> + flexcan2_phy: can-phy0 {
> >> + compatible = "nxp,tja1051";
> >> + #phy-cells = <0>;
> >> + max-bitrate = <8000000>;
> >> + silent-gpios = <&pcal6416 6 GPIO_ACTIVE_HIGH>;
> >> + };
> >> +
> >> + flexcan3_phy: can-phy1 {
> >> + compatible = "nxp,tja1051";
> >> + #phy-cells = <0>;
> >> + max-bitrate = <8000000>;
> >> + silent-gpios = <&pcal6416 6 GPIO_ACTIVE_HIGH>;
> >
> > [Severity: High]
> > Does this cause a probe failure for the second transceiver?
> >
> > The first PHY node successfully requests this GPIO line, taking exclusive
> > ownership. Because the phy-can-transceiver driver uses standard exclusive
> > requests (without GPIOD_FLAGS_BIT_NONEXCLUSIVE), the second request for the
> > exact same GPIO line fails with -EBUSY.
> >
> > This prevents the flexcan3_phy and the flexcan3 controller from probing and
> > functioning.
> >
>
> This two CAN PHY did share same pin as their silent-gpio on hardware.
Hardware share gpio, but driver may not handle this case. Are you sure
flexcan3_phy probed if can-phy0 already probed?
Frank
>
> The same design on i.MX95 15x15 FRDM. This should be fine.
>
> Regards,
> Joseph
>
> >> + };
> > [ ... ]
> >> +&usdhc2 {
> >> + bus-width = <4>;
> >> + cd-gpios = <&gpio3 0 GPIO_ACTIVE_LOW>;
> >> + pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> >> + pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
> >> + pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
> >> + pinctrl-3 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> >> + pinctrl-names = "default", "state_100mhz", "state_200mhz", "sleep";
> >> + vmmc-supply = <®_usdhc2_vmmc>;
> >> + fsl,cd-gpio-wakeup-disable;
> >
> > [Severity: Low]
> > Is this out-of-tree vendor property invalid in the upstream kernel?
> >
> > The property fsl,cd-gpio-wakeup-disable does not exist in any upstream device
> > tree bindings (such as fsl-imx-esdhc.yaml) and is not handled by any driver
> > code in the upstream Linux tree. This acts as dead code and will trigger
> > `make dtbs_check` validation warnings.
> >
> >> + status = "okay";
> >> +};
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260720-imx952_frdm-v1-0-6d145eaaa5c4@nxp.com?part=2
>
> --
> Best Regards,
> Joseph
^ permalink raw reply
* Re: [PATCH v13 2/3] dt-bindings: clock: imx95-blk-ctl: Define formatter child node schema
From: Laurent Pinchart @ 2026-07-20 15:06 UTC (permalink / raw)
To: guoniu.zhou
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Frank Li, Loic Poulain, Bryan O'Donoghue,
Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, imx,
linux-media, devicetree, linux-arm-kernel, linux-kernel,
linux-clk, Guoniu Zhou, Krzysztof Kozlowski
In-Reply-To: <20260720-csi_formatter-v13-2-4dc9a80e4cfd@oss.nxp.com>
Hello Guoniu,
Thank you for the patch.
On Mon, Jul 20, 2026 at 05:59:47PM +0800, guoniu.zhou@oss.nxp.com wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
>
> The Camera CSR contains control registers for multiple CSI formatter IPs
> at different register offsets. Each formatter is an independent hardware
> block with its own clock input and media pipeline connection.
>
> Define schema to allow formatter child nodes under nxp,imx95-camera-csr,
> with 'reg' property specifying the formatter's register offset within the
> CSR address space.
>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes in v13:
> - Add Reviewed-by tag from Frank and Krzysztof
>
> Changes in v11:
> - Move properties to top-level and use if:then:else (Krzysztof/Frank)
>
> Changes in v10:
> - Use single quotes for regex pattern to be consistent (Krzysztof Kozlowski)
> - Add formatter subnode binding and camera-csr syscon example
> - Update commit title and message
>
> Changes in v9:
> - New patch to address the issue of formatter acting as a child node of syscon
> ---
> .../bindings/clock/nxp,imx95-blk-ctl.yaml | 71 ++++++++++++++++++++++
> 1 file changed, 71 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> index 27403b4c52d6..fbbf1b3f1790 100644
> --- a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> +++ b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> @@ -39,6 +39,18 @@ properties:
> ID in its "clocks" phandle cell. See
> include/dt-bindings/clock/nxp,imx95-clock.h
>
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 1
> +
> +patternProperties:
> + '^formatter@[0-9a-f]+$':
> + type: object
> + $ref: /schemas/media/fsl,imx95-csi-formatter.yaml#
> + unevaluatedProperties: false
> +
> required:
> - compatible
> - reg
> @@ -46,6 +58,23 @@ required:
> - power-domains
> - clocks
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: nxp,imx95-camera-csr
> + then:
> + required:
> + - '#address-cells'
> + - '#size-cells'
> + else:
> + properties:
> + '#address-cells': false
> + '#size-cells': false
> + patternProperties:
> + '^formatter@[0-9a-f]+$': false
> +
> additionalProperties: false
>
> examples:
> @@ -57,4 +86,46 @@ examples:
> clocks = <&scmi_clk 114>;
> power-domains = <&scmi_devpd 21>;
> };
> +
> + - |
> + #include <dt-bindings/clock/nxp,imx95-clock.h>
> +
> + syscon@4ac10000 {
> + compatible = "nxp,imx95-camera-csr", "syscon";
> + reg = <0x4ac10000 0x10000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + #clock-cells = <1>;
> + clocks = <&scmi_clk 62>;
> + power-domains = <&scmi_devpd 3>;
> +
> + formatter@20 {
> + compatible = "fsl,imx95-csi-formatter";
> + reg = <0x20 0x100>;
> + clocks = <&cameramix_csr IMX95_CLK_CAMBLK_CSI2_FOR0>;
> + power-domains = <&scmi_devpd 3>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + endpoint {
> + remote-endpoint = <&mipi_csi_0_out>;
> + };
> +
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + endpoint {
> + remote-endpoint = <&isi_in_2>;
> + };
> + };
> + };
> + };
> + };
> ...
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v13 1/3] media: dt-bindings: Add CSI Pixel Formatter DT bindings
From: Laurent Pinchart @ 2026-07-20 15:06 UTC (permalink / raw)
To: guoniu.zhou
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Frank Li, Loic Poulain, Bryan O'Donoghue,
Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, imx,
linux-media, devicetree, linux-arm-kernel, linux-kernel,
linux-clk, Guoniu Zhou, Krzysztof Kozlowski
In-Reply-To: <20260720-csi_formatter-v13-1-4dc9a80e4cfd@oss.nxp.com>
Hi Guoniu,
Thank you for the patch.
On Mon, Jul 20, 2026 at 05:59:46PM +0800, guoniu.zhou@oss.nxp.com wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
>
> The i.MX95 CSI pixel formatting module uses packet info, pixel and
> non-pixel data from the CSI-2 host controller and reformat them to
> match Pixel Link(PL) definition.
>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes in v11:
> - Add Rb tags from Frank Li and Krzysztof Kozlowski
>
> Changes in v10:
> - Drop syscon parent node from example
> - Drop Reviewed-by tags from Frank and Krzysztof due to binding changes
> - Add description for reg property
> - Add space after formatter@20 before opening brace in example
> - Enhance the port description with more detailed information
> - Delete the blank line immediately following the endpoint in example
>
> Changes in v9:
> - Use direct node instead of syscon wrapper in example
>
> Changes in v8:
> - Use standard port reference instead of video-interfaces.yaml
> - Add parent syscon node in example to show device integration
> - Add required constraints for port@0 and port@1 in ports node
>
> Changes in v7:
> - Change compatible to imx95-csi-formatter as IP is i.MX95 specific per Marco's suggestion
> Link: https://lore.kernel.org/linux-media/20260511-csi_formatter-v6-0-01028e312e2b@oss.nxp.com/T/#mcd135b3de179b3cb69daa1fd6e0e8e27c85b3332
> ---
> .../bindings/media/fsl,imx95-csi-formatter.yaml | 88 ++++++++++++++++++++++
> 1 file changed, 88 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/fsl,imx95-csi-formatter.yaml b/Documentation/devicetree/bindings/media/fsl,imx95-csi-formatter.yaml
> new file mode 100644
> index 000000000000..58c4e1cc056b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/fsl,imx95-csi-formatter.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/fsl,imx95-csi-formatter.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: i.MX95 CSI Pixel Formatter
> +
> +maintainers:
> + - Guoniu Zhou <guoniu.zhou@nxp.com>
> +
> +description:
> + The CSI pixel formatting module found on i.MX95 uses packet info, pixel
> + and non-pixel data from the CSI-2 host controller and reformat them to
> + match Pixel Link(PL) definition.
> +
> +properties:
> + compatible:
> + const: fsl,imx95-csi-formatter
> +
> + reg:
> + maxItems: 1
> + description: Register offset and size within the parent syscon
> +
> + clocks:
> + maxItems: 1
> +
> + power-domains:
> + maxItems: 1
> +
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> +
> + properties:
> + port@0:
> + $ref: /schemas/graph.yaml#/properties/port
> + description:
> + Input port, connects to MIPI CSI-2 receiver output (IDI interface)
> +
> + port@1:
> + $ref: /schemas/graph.yaml#/properties/port
> + description:
> + Output port, connects to ISI input via Pixel Link (PL)
> +
> + required:
> + - port@0
> + - port@1
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - power-domains
> + - ports
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/nxp,imx95-clock.h>
> +
> + formatter@20 {
> + compatible = "fsl,imx95-csi-formatter";
> + reg = <0x20 0x100>;
> + clocks = <&cameramix_csr IMX95_CLK_CAMBLK_CSI2_FOR0>;
> + power-domains = <&scmi_devpd 3>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + endpoint {
> + remote-endpoint = <&mipi_csi_0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + endpoint {
> + remote-endpoint = <&isi_in_2>;
> + };
> + };
> + };
> + };
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 2/5] arm64: dts: qcom: Add Kalambo SoC
From: Gopikrishna Garmidi @ 2026-07-20 15:04 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sibi Sankar, Pankaj Patil
Cc: Rajendra Nayak, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <581659e3-9168-42b2-b715-943808d9ae2f@oss.qualcomm.com>
Hi Konrad,
Thanks a lot for the review and for catching these
> On 7/18/26 2:06 PM, Gopikrishna Garmidi wrote:
>> Introduce support for the Qualcomm Kalambo SoC. It is derived from Mahua
>> with CPU cluster 0 removed, leaving it with just cluster 1. As a result,
>> the PDP mailbox and SCP low-priority register windows are updated to
>> match the reduced CPU complex. Everything else should work as-is.
>>
>> Add a label to the Glymur cluster0 cpu-map node to allow its removal on
>> derived SoCs.
>>
>> Signed-off-by: Gopikrishna Garmidi <gopikrishna.garmidi@oss.qualcomm.com>
>> ---
>> arch/arm64/boot/dts/qcom/glymur.dtsi | 2 +-
>> arch/arm64/boot/dts/qcom/kalambo.dtsi | 29 +++++++++++++++++++++++++++++
>> 2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
>> index 55d91c696a3a..f2feef362be5 100644
>> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
>> @@ -275,7 +275,7 @@ cpu17: cpu@20500 {
>> };
>>
>> cpu-map {
>> - cluster0 {
>> + cpu_map_cluster0: cluster0 {
>
> I doubt the code that parses this will be happy about not
> being able to start from 'cluster0'
>
Good catch, you're right. parse_cluster() in
drivers/base/arch_topology.c walks "cluster%d" starting at 0 and bails
out on the first missing index, so deleting cluster0 and leaving only
cluster1 means the topology never gets parsed at all.
For v2 I'll delete both cluster0 and cluster1 and re-add the surviving
cores as cluster0 in the Kalambo SoC file, so the scan starts at
cluster0 as expected:
/delete-node/ &cpu_map_cluster0;
/delete-node/ &cpu_map_cluster1;
&{/cpus/cpu-map} {
cluster0 {
core0 {
cpu = <&cpu6>;
};
...
core5 {
cpu = <&cpu11>;
};
};
};
Does this look right to you, or would you prefer re-pointing cluster0's
cores in place rather than delete + re-add?
>
> [...]
>
>> +&pdp0_mbox {
>> + reg = <0 0x17610000 0 0x8000>, <0 0x1a980000 0 0x8000>;
>> +};
>
> Are you sure about this change?
Yes — confirmed against the SoC memory map. The Tx window is the
NCC0/NCC1 mailbox instance; dropping the former cluster0 moves the Tx
base from NCC0 (0x19980000) to NCC1 (0x1a980000), while reg[0] (Rx) is
shared and unchanged. I'll note this in the commit message.
>> +&cpu_scp_lpri1 {
>> + reg = <0x180 0x60>;
>> +};
>
> This is specific to old firmware, at least it was on Glymur, I believe..
Right, the size was bumped permanently on the firmware side, so this
override isn't needed anymore. I'll drop it in v2.
> Konrad
^ permalink raw reply
* Re: [PATCH RFC] arm64: dts: allwinner: a523: Add SPDIF to x96qproplus device
From: Chen-Yu Tsai @ 2026-07-20 14:59 UTC (permalink / raw)
To: Per Larsson
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jernej Skrabec,
Samuel Holland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/Allwinner sunXi SoC support,
open list:ARM/Allwinner sunXi SoC support, open list
In-Reply-To: <20260719030927.2417-1-per@palvencia.se>
On Sun, Jul 19, 2026 at 11:10 AM Per Larsson <per@palvencia.se> wrote:
>
> When spdif support was added for the Allwinner A523 family of SoCs, only
> two of the three possible pins were added to the dtsi, since the third
> would clash with the first ethernet port (emac0)
> However, some devices don't use emac0 and instead use emac1 for the only
> available ethernet port, leaving the pin free to use for spdif.
>
> Add it to the dtsi, and to such a device where this pin is used for spdif
>
> Signed-off-by: Per Larsson <per@palvencia.se>
> ---
> Marking this as RFC for a few reasons
> 1. This is my first submission, hoping everything is properly organized.
This is pretty good. The commit message is also well written. I would
like to see the patch split into two patches though. The first adds the
pinmux (and you can mention in the commit message that a subsequent
patch will reference it). The second patch enables SPDIF on the device
you have.
> 2. My testing setup is not the best: I get sound with this patch, but
> the pulseaudio daemon needs to be restarted far too often.
> Hopefully it's just something on this minirootfs. Testing welcome.
> 3. Not sure where to get the hash for a fixes tag or if that's even OK
This is a new addition, not a fix, so no fixes tag is warranted.
Thanks
ChenYu
> ---
> .../arm64/boot/dts/allwinner/sun55i-a523.dtsi | 7 ++++++
> .../dts/allwinner/sun55i-h728-x96qpro+.dts | 24 +++++++++++++++++++
> 2 files changed, 31 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> index ca6a16807049..7370e8ed24ec 100644
> --- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> @@ -216,6 +216,13 @@ spdif_out_pb_pin: spdif-pb-pin {
> allwinner,pinmux = <2>;
> };
>
> + /omit-if-no-ref/
> + spdif_out_ph_pin: spdif-ph-pin {
> + pins = "PH7";
> + function = "spdif";
> + allwinner,pinmux = <4>;
> + };
> +
> /omit-if-no-ref/
> spdif_out_pi_pin: spdif-pi-pin {
> pins = "PI10";
> diff --git a/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts b/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts
> index a96927fbdadd..b172985583f8 100644
> --- a/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts
> @@ -44,6 +44,24 @@ reg_vcc3v3: vcc3v3 {
> vin-supply = <®_vcc5v>;
> regulator-always-on;
> };
> +
> + sound-spdif {
> + compatible = "simple-audio-card";
> + simple-audio-card,name = "spdif-out";
> +
> + simple-audio-card,cpu {
> + sound-dai = <&spdif>;
> + };
> +
> + simple-audio-card,codec {
> + sound-dai = <&spdif_out>;
> + };
> + };
> +
> + spdif_out: spdif-out {
> + #sound-dai-cells = <0>;
> + compatible = "linux,spdif-dit";
> + };
> };
>
> &ehci0 {
> @@ -275,6 +293,12 @@ &r_pio {
> vcc-pm-supply = <®_aldo3>;
> };
>
> +&spdif {
> + pinctrl-names = "default";
> + pinctrl-0 = <&spdif_out_ph_pin>;
> + status = "okay";
> +};
> +
> &uart0 {
> pinctrl-names = "default";
> pinctrl-0 = <&uart0_pb_pins>;
> --
> 2.48.1
>
^ permalink raw reply
* Re: [PATCH v3 phy-next 2/8] soc: fsl: guts: add a global structure to hold state
From: Vladimir Oltean @ 2026-07-20 14:57 UTC (permalink / raw)
To: Michael Walle
Cc: devicetree, robh, conor+dt, vkoul, neil.armstrong, olteanv,
linux-phy
In-Reply-To: <20260720140139.dmgysphpse2jf3fi@skbuf>
Hi Michael,
On Mon, Jul 20, 2026 at 05:01:39PM +0300, Vladimir Oltean wrote:
> On Mon, Jul 20, 2026 at 01:50:49PM +0000, sashiko-bot@kernel.org wrote:
> > [Severity: High]
> > Will jumping to err_nomem cause a NULL pointer dereference?
> >
> > If the allocation of soc_dev_attr fails, the code now jumps to the error
> > handling path which dereferences soc_dev_attr to free its members:
> >
> > > @@ -269,6 +271,8 @@ static int __init fsl_guts_init(void)
> > > kfree(soc_dev_attr->revision);
> > > kfree(soc_dev_attr->serial_number);
> >
> > Since soc_dev_attr is NULL when this allocation fails, won't evaluating
> > soc_dev_attr->revision and other members result in a kernel panic?
>
> yeah.
>
> I tried to be nice and play with the existing "err_nomem" and "err"
> error handling labels, but the truth is that a single error cleanup path
> is just an antipattern that doesn't scale as code becomes more complex.
>
> Instead of having a single error handling procedure which needs to
> carefully fend off from various invalid contexts from all the goto
> sites, the standard convention is to have an incremental set of labels
> which undo just the setup that was done up to the goto jump site.
>
> I'll refactor fsl_guts_init() to use that convention for v4.
While I was doing this refactoring, I noticed that this code path:
if (soc_data)
soc_uid = fsl_guts_get_soc_uid(soc_data->sfp_compat,
soc_data->uid_offset);
if (soc_uid) {
soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX",
soc_uid);
// no kasprintf() NULL return code check here, unlike
// the rest of the code
}
soc_dev = soc_device_register(soc_dev_attr);
if (IS_ERR(soc_dev)) {
ret = PTR_ERR(soc_dev);
goto err;
}
proceeds with soc_device_register() even if fsl_guts_get_soc_uid() was
able to get a soc unique id from the security fuse processor, but we
fail to print that string to the soc_dev_attr->serial_number variable.
I don't see anywhere mentioned in commit 786dde1e59d7 ("soc: fsl: guts:
add serial_number support") that this would be intentional, so it can
just as well be an omission.
Is it OK if I replace the silent failure with a loud failure of the
entire fsl_guts_init()? The code executes only if soc_uid is non-zero
anyway.
kasprintf() fails only for memory related reasons.
^ 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