Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v10 5/6] PCI: Bring the PCIe speed to MBps logic to new pcie_link_speed_to_mbps()
From: Bjorn Helgaas @ 2024-04-09 15:07 UTC (permalink / raw)
  To: Krishna chaitanya chundru
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Bjorn Helgaas, johan+linaro, bmasney,
	djakov, linux-arm-msm, devicetree, linux-kernel, linux-pci,
	vireshk, quic_vbadigan, quic_skananth, quic_nitegupt, quic_parass,
	krzysztof.kozlowski
In-Reply-To: <20240409-opp_support-v10-5-1956e6be343f@quicinc.com>

On Tue, Apr 09, 2024 at 03:43:23PM +0530, Krishna chaitanya chundru wrote:
> Bring the switch case in pcie_link_speed_mbps() to new function to
> the header file so that it can be used in other places like
> in controller driver.
> 
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>

Unnecessary.  Not every code review comment needs to be acknowledged
in the commit log :)

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/pci.c | 19 +------------------
>  drivers/pci/pci.h | 22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e5f243dd4288..40487b86a75e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5922,24 +5922,7 @@ int pcie_link_speed_mbps(struct pci_dev *pdev)
>  	if (err)
>  		return err;
>  
> -	switch (to_pcie_link_speed(lnksta)) {
> -	case PCIE_SPEED_2_5GT:
> -		return 2500;
> -	case PCIE_SPEED_5_0GT:
> -		return 5000;
> -	case PCIE_SPEED_8_0GT:
> -		return 8000;
> -	case PCIE_SPEED_16_0GT:
> -		return 16000;
> -	case PCIE_SPEED_32_0GT:
> -		return 32000;
> -	case PCIE_SPEED_64_0GT:
> -		return 64000;
> -	default:
> -		break;
> -	}
> -
> -	return -EINVAL;
> +	return pcie_link_speed_to_mbps(to_pcie_link_speed(lnksta));
>  }
>  EXPORT_SYMBOL(pcie_link_speed_mbps);
>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 17fed1846847..4de10087523e 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -290,6 +290,28 @@ void pci_bus_put(struct pci_bus *bus);
>  	 (speed) == PCIE_SPEED_2_5GT  ?  2500*8/10 : \
>  	 0)
>  
> +static inline int pcie_link_speed_to_mbps(enum pci_bus_speed speed)
> +{
> +	switch (speed) {
> +	case PCIE_SPEED_2_5GT:
> +		return 2500;
> +	case PCIE_SPEED_5_0GT:
> +		return 5000;
> +	case PCIE_SPEED_8_0GT:
> +		return 8000;
> +	case PCIE_SPEED_16_0GT:
> +		return 16000;
> +	case PCIE_SPEED_32_0GT:
> +		return 32000;
> +	case PCIE_SPEED_64_0GT:
> +		return 64000;
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  const char *pci_speed_string(enum pci_bus_speed speed);
>  enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev);
>  enum pcie_link_width pcie_get_width_cap(struct pci_dev *dev);
> 
> -- 
> 2.42.0
> 

^ permalink raw reply

* Re: [PATCH 0/4] DONOTMERGE: ep93xx-clk from ep93xx device tree conversion
From: Alexander Sverdlin @ 2024-04-09 15:09 UTC (permalink / raw)
  To: Nikita Shubin, Conor Dooley, Arnd Bergmann
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-kernel, linux-clk, devicetree, Linus Walleij,
	Krzysztof Kozlowski
In-Reply-To: <42f9da044fdc11e2495f6845c061afefa796f7cf.camel@maquefel.me>

Hi Nikita,

On Tue, 2024-04-09 at 14:48 +0300, Nikita Shubin wrote:
> On Mon, 2024-04-08 at 18:03 +0100, Conor Dooley wrote:
> > On Mon, Apr 08, 2024 at 11:09:52AM +0300, Nikita Shubin via B4 Relay
> > wrote:
> > > The goal is to recieve ACKs.
> > 
> > I dont see a maintainers entry in -rc1 for the drivers/soc/cirrus
> > portion. Who is gonna give you an Ack for that portion? If you
> > intended
> > maintaining that driver, should you not add a MAINTAINERS entry for
> > it?
> 
> drivers/soc/cirrus got it's ACK from ep93xx MAINTAINER - Alexander
> Sverdlin.
> 
> Arnd, Alexander - should we add it now ?

seems that we have couple of things to fix:

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#51: 
new file mode 100644

WARNING: please write a help paragraph that fully describes the config symbol
#60: FILE: drivers/soc/cirrus/Kconfig:5:
+config EP93XX_SOC
+	bool "Cirrus EP93xx chips SoC"
+	select SOC_BUS
+	select AUXILIARY_BUS
+	default y if !EP93XX_SOC_COMMON
+	help
+	  Support SoC for Cirrus EP93xx chips.
+

total: 0 errors, 2 warnings, 269 lines checked

-- 
Alexander Sverdlin.


^ permalink raw reply

* Re: [RFC PATCH v2 0/2] fw_devlink overlay fix
From: Geert Uytterhoeven @ 2024-04-09 15:10 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Herve Codina, Rob Herring, kernel-team, linux-kernel, imx,
	linux-arm-kernel, linux-i2c, devicetree, linux-spi, linux-acpi
In-Reply-To: <20240409053704.428336-1-saravanak@google.com>

Hi Saravana,

On Tue, Apr 9, 2024 at 7:37 AM Saravana Kannan <saravanak@google.com> wrote:
> Don't bother reviewing this patch. It needs to be tested and possibly
> refactored first.
>
> Geert and Herve,
>
> This patch serious should hopefully fix both of your use cases
> [1][2][3]. Can you please check to make sure the device links created
> to/from the overlay devices are to/from the right ones?

Thanks for your series!

After applying the first patch (the revert), the issue reported in
[1] is back, as expected.
After applying both patches, applying[A]/unapplying[B]/reapplying[C]
overlay [4] works as without this series, so
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Note that the state of /sys/class/devlink/ after [C] is still not the
same as after [A], as reported before in [5]:
  - platform:e6060000.pinctrl--platform:keys link is not recreated in [B],
  - nothing changes in /sys/class/devlink in [C].
But that issue is not introduced in this series.

> [1] - https://lore.kernel.org/lkml/CAMuHMdXEnSD4rRJ-o90x4OprUacN_rJgyo8x6=9F9rZ+-KzjOg@mail.gmail.com/

[4] "arm64: dts: renesas: ebisu: cn41: Add overlay for MSIOF0 and 25LC040"
    https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/renesas-overlays&id=222a4936b0d3dabd43bdffb3a578423bff97b02d
[5] https://lore.kernel.org/lkml/CAMuHMdXNoYH8PJE1xb4PK-vzjXtOzrxNJoZhsHT-H4Ucm=7_ig@mail.gmail.com/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 0/3] Fix up qcom,halt-regs definition in various schemas
From: Rob Herring @ 2024-04-09 15:10 UTC (permalink / raw)
  To: Luca Weiss
  Cc: ~postmarketos/upstreaming, phone-devel, Bjorn Andersson,
	Mathieu Poirier, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
	linux-remoteproc, devicetree, linux-kernel
In-Reply-To: <20240407-qcom-halt-regs-fixup-v1-0-a0ea4e2c178e@z3ntu.xyz>

On Sun, Apr 07, 2024 at 11:58:29AM +0200, Luca Weiss wrote:
> The original motivation is that a bunch of other schemas fail to
> validate qcom,halt-regs, for example like in the following examples:
> 
> arch/arm64/boot/dts/qcom/apq8016-sbc.dtb: remoteproc@4080000: qcom,halt-regs:0: [20] is too short
>         from schema $id: http://devicetree.org/schemas/remoteproc/qcom,msm8916-mss-pil.yaml#
> arch/arm64/boot/dts/qcom/apq8096-ifc6640.dtb: remoteproc@2080000: qcom,halt-regs:0: [82] is too short
>         from schema $id: http://devicetree.org/schemas/remoteproc/qcom,msm8996-mss-pil.yaml#
> arch/arm64/boot/dts/qcom/apq8039-t2.dtb: remoteproc@4080000: qcom,halt-regs:0: [32] is too short
>         from schema $id: http://devicetree.org/schemas/remoteproc/qcom,msm8916-mss-pil.yaml#
> 
> While I'm actually not quite sure why these patches fix this in
> the other schemas - feels like a bug/limitation in dt-schema maybe? -

Was this with v2024.02? It should be a bit better there. Though it 
may just have different errors. The limitation is that property 
types and in the case of matrix's (which phandle-array actually is) 
range for dimensions are global. So if there's not correct dimensions 
for a property, the tools aren't going to decode it properly.

Rob

^ permalink raw reply

* Re: [PATCH 3/6] drm/msm/adreno: Allow specifying default speedbin value
From: Konrad Dybcio @ 2024-04-09 15:12 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
	dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <pncr7ecf4eir36skul3iwt2nf5bpuwd5zjfzzfwwnxjwe4hoes@6z2xe54crijp>



On 4/6/24 04:56, Dmitry Baryshkov wrote:
> On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
>> From: Neil Armstrong <neil.armstrong@linaro.org>
>>
>> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
>> the highest. Falling back to it when things go wrong is largely
>> suboptimal, as more often than not, the top frequencies are not
>> supposed to work on other bins.
> 
> Isn't it better to just return an error here instead of trying to guess
> which speedbin to use?

Not sure. I'd rather better compatibility for e.g. booting up a new
laptop with just dt.

> 
> If that's not the case, I think the commit should be expanded with
> actually setting default_speedbin for the existing GPUs.

I think that should be addressed, although separately.

Konrad

^ permalink raw reply

* Re: [PATCH 5/6] drm/msm/adreno: Add speedbin data for SM8550 / A740
From: Konrad Dybcio @ 2024-04-09 15:13 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
	dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <nek4paofg3hturvjwpa2bnsvmicwmvwixzr6e6iuqstemgrqyo@cagcrnzjsne2>



On 4/6/24 05:25, Dmitry Baryshkov wrote:
> On Fri, Apr 05, 2024 at 10:41:33AM +0200, Konrad Dybcio wrote:
>> Add speebin data for A740, as found on SM8550 and derivative SoCs.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>   drivers/gpu/drm/msm/adreno/adreno_device.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> index 901ef767e491..c976a485aef2 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> @@ -570,6 +570,20 @@ static const struct adreno_info gpulist[] = {
>>   		.zapfw = "a740_zap.mdt",
>>   		.hwcg = a740_hwcg,
>>   		.address_space_size = SZ_16G,
>> +		.speedbins = ADRENO_SPEEDBINS(
> 
> I think this deserves either a comment or some info in the commit
> message.

"this" = ?

Konrad

^ permalink raw reply

* Re: [PATCH RESEND] arm64: dts: qcom: qcm6490-idp: Name the regulators
From: Bjorn Andersson @ 2024-04-09 15:16 UTC (permalink / raw)
  To: Umang Chheda
  Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240403132945.2117890-1-quic_uchheda@quicinc.com>

On Wed, Apr 03, 2024 at 06:59:45PM +0530, Umang Chheda wrote:
> Without explicitly specifying names for the regulators they are named
> based on the DeviceTree node name. This results in multiple regulators
> with the same name, making it impossible to reason debug prints and
> regulator_summary.
> 

Why is this marked "RESEND"? I can only find [1].

But you received review feedback on that one, which you have addressed,
which means that this is a new version of the patch - as such this
should be "[PATCH v2] ...".

[1] https://lore.kernel.org/all/20240329122940.3649730-1-quic_uchheda@quicinc.com/

> Signed-off-by: Umang Chheda <quic_uchheda@quicinc.com>
> ---

And here you can write things that won't be picked up in the git
history, such as the reason for sending the patch, or what changed since
v1.


Please look at go/upstream, adopt b4 for preparing your patches, use
--force-revision to send me v3 - where you clarify the changes between
v1 and v2 (this resend).

You can specify "Resubmit as v3 to clarify history of patch" or
something like that for the v3 changes. 


Change itself looks good, thank you.

Regards,
Bjorn

>  arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 41 ++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> index f8f8a43f638d..ac6d741868ca 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> @@ -195,129 +195,151 @@ regulators-0 {
>  		vdd-l14-l16-supply = <&vreg_s8b_1p272>;
>  
>  		vreg_s1b_1p872: smps1 {
> +			regulator-name = "vreg_s1b_1p872";
>  			regulator-min-microvolt = <1840000>;
>  			regulator-max-microvolt = <2040000>;
>  		};
>  
>  		vreg_s2b_0p876: smps2 {
> +			regulator-name = "vreg_s2b_0p876";
>  			regulator-min-microvolt = <570070>;
>  			regulator-max-microvolt = <1050000>;
>  		};
>  
>  		vreg_s7b_0p972: smps7 {
> +			regulator-name = "vreg_s7b_0p972";
>  			regulator-min-microvolt = <535000>;
>  			regulator-max-microvolt = <1120000>;
>  		};
>  
>  		vreg_s8b_1p272: smps8 {
> +			regulator-name = "vreg_s8b_1p272";
>  			regulator-min-microvolt = <1200000>;
>  			regulator-max-microvolt = <1500000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_RET>;
>  		};
>  
>  		vreg_l1b_0p912: ldo1 {
> +			regulator-name = "vreg_l1b_0p912";
>  			regulator-min-microvolt = <825000>;
>  			regulator-max-microvolt = <925000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l2b_3p072: ldo2 {
> +			regulator-name = "vreg_l2b_3p072";
>  			regulator-min-microvolt = <2700000>;
>  			regulator-max-microvolt = <3544000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l3b_0p504: ldo3 {
> +			regulator-name = "vreg_l3b_0p504";
>  			regulator-min-microvolt = <312000>;
>  			regulator-max-microvolt = <910000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l4b_0p752: ldo4 {
> +			regulator-name = "vreg_l4b_0p752";
>  			regulator-min-microvolt = <752000>;
>  			regulator-max-microvolt = <820000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		reg_l5b_0p752: ldo5 {
> +			regulator-name = "reg_l5b_0p752";
>  			regulator-min-microvolt = <552000>;
>  			regulator-max-microvolt = <832000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l6b_1p2: ldo6 {
> +			regulator-name = "vreg_l6b_1p2";
>  			regulator-min-microvolt = <1140000>;
>  			regulator-max-microvolt = <1260000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l7b_2p952: ldo7 {
> +			regulator-name = "vreg_l7b_2p952";
>  			regulator-min-microvolt = <2400000>;
>  			regulator-max-microvolt = <3544000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l8b_0p904: ldo8 {
> +			regulator-name = "vreg_l8b_0p904";
>  			regulator-min-microvolt = <870000>;
>  			regulator-max-microvolt = <970000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l9b_1p2: ldo9 {
> +			regulator-name = "vreg_l9b_1p2";
>  			regulator-min-microvolt = <1200000>;
>  			regulator-max-microvolt = <1304000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l11b_1p504: ldo11 {
> +			regulator-name = "vreg_l11b_1p504";
>  			regulator-min-microvolt = <1504000>;
>  			regulator-max-microvolt = <2000000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l12b_0p751: ldo12 {
> +			regulator-name = "vreg_l12b_0p751";
>  			regulator-min-microvolt = <751000>;
>  			regulator-max-microvolt = <824000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l13b_0p53: ldo13 {
> +			regulator-name = "vreg_l13b_0p53";
>  			regulator-min-microvolt = <530000>;
>  			regulator-max-microvolt = <824000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l14b_1p08: ldo14 {
> +			regulator-name = "vreg_l14b_1p08";
>  			regulator-min-microvolt = <1080000>;
>  			regulator-max-microvolt = <1304000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l15b_0p765: ldo15 {
> +			regulator-name = "vreg_l15b_0p765";
>  			regulator-min-microvolt = <765000>;
>  			regulator-max-microvolt = <1020000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l16b_1p1: ldo16 {
> +			regulator-name = "vreg_l16b_1p1";
>  			regulator-min-microvolt = <1100000>;
>  			regulator-max-microvolt = <1300000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l17b_1p7: ldo17 {
> +			regulator-name = "vreg_l17b_1p7";
>  			regulator-min-microvolt = <1700000>;
>  			regulator-max-microvolt = <1900000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l18b_1p8: ldo18 {
> +			regulator-name = "vreg_l18b_1p8";
>  			regulator-min-microvolt = <1800000>;
>  			regulator-max-microvolt = <2000000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l19b_1p8: ldo19 {
> +			regulator-name = "vreg_l19b_1p8";
>  			regulator-min-microvolt = <1800000>;
>  			regulator-max-microvolt = <2000000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> @@ -349,109 +371,128 @@ regulators-1 {
>  		vdd-bob-supply = <&vph_pwr>;
>  
>  		vreg_s1c_2p19: smps1 {
> +			regulator-name = "vreg_s1c_2p19";
>  			regulator-min-microvolt = <2190000>;
>  			regulator-max-microvolt = <2210000>;
>  		};
>  
>  		vreg_s2c_0p752: smps2 {
> +			regulator-name = "vreg_s2c_0p752";
>  			regulator-min-microvolt = <750000>;
>  			regulator-max-microvolt = <800000>;
>  		};
>  
>  		vreg_s5c_0p752: smps5 {
> +			regulator-name = "vreg_s5c_0p752";
>  			regulator-min-microvolt = <465000>;
>  			regulator-max-microvolt = <1050000>;
>  		};
>  
>  		vreg_s7c_0p752: smps7 {
> +			regulator-name = "vreg_s7c_0p752";
>  			regulator-min-microvolt = <465000>;
>  			regulator-max-microvolt = <800000>;
>  		};
>  
>  		vreg_s9c_1p084: smps9 {
> +			regulator-name = "vreg_s9c_1p084";
>  			regulator-min-microvolt = <1010000>;
>  			regulator-max-microvolt = <1170000>;
>  		};
>  
>  		vreg_l1c_1p8: ldo1 {
> +			regulator-name = "vreg_l1c_1p8";
>  			regulator-min-microvolt = <1800000>;
>  			regulator-max-microvolt = <1980000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l2c_1p62: ldo2 {
> +			regulator-name = "vreg_l2c_1p62";
>  			regulator-min-microvolt = <1620000>;
>  			regulator-max-microvolt = <1980000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l3c_2p8: ldo3 {
> +			regulator-name = "vreg_l3c_2p8";
>  			regulator-min-microvolt = <2800000>;
>  			regulator-max-microvolt = <3540000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l4c_1p62: ldo4 {
> +			regulator-name = "vreg_l4c_1p62";
>  			regulator-min-microvolt = <1620000>;
>  			regulator-max-microvolt = <3300000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l5c_1p62: ldo5 {
> +			regulator-name = "vreg_l5c_1p62";
>  			regulator-min-microvolt = <1620000>;
>  			regulator-max-microvolt = <3300000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l6c_2p96: ldo6 {
> +			regulator-name = "vreg_l6c_2p96";
>  			regulator-min-microvolt = <1650000>;
>  			regulator-max-microvolt = <3544000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l7c_3p0: ldo7 {
> +			regulator-name = "vreg_l7c_3p0";
>  			regulator-min-microvolt = <3000000>;
>  			regulator-max-microvolt = <3544000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l8c_1p62: ldo8 {
> +			regulator-name = "vreg_l8c_1p62";
>  			regulator-min-microvolt = <1620000>;
>  			regulator-max-microvolt = <2000000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l9c_2p96: ldo9 {
> +			regulator-name = "vreg_l9c_2p96";
>  			regulator-min-microvolt = <2700000>;
>  			regulator-max-microvolt = <35440000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l10c_0p88: ldo10 {
> +			regulator-name = "vreg_l10c_0p88";
>  			regulator-min-microvolt = <720000>;
>  			regulator-max-microvolt = <1050000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l11c_2p8: ldo11 {
> +			regulator-name = "vreg_l11c_2p8";
>  			regulator-min-microvolt = <2800000>;
>  			regulator-max-microvolt = <3544000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l12c_1p65: ldo12 {
> +			regulator-name = "vreg_l12c_1p65";
>  			regulator-min-microvolt = <1650000>;
>  			regulator-max-microvolt = <2000000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_l13c_2p7: ldo13 {
> +			regulator-name = "vreg_l13c_2p7";
>  			regulator-min-microvolt = <2700000>;
>  			regulator-max-microvolt = <3544000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
>  		vreg_bob_3p296: bob {
> +			regulator-name = "vreg_bob_3p296";
>  			regulator-min-microvolt = <3008000>;
>  			regulator-max-microvolt = <3960000>;
>  		};
> -- 
> 2.25.1
> 

^ permalink raw reply

* Re: [PATCH RESEND] arm64: dts: qcom: qcs6490-rb3gen2: enable PMIC Volume and Power buttons
From: Bjorn Andersson @ 2024-04-09 15:17 UTC (permalink / raw)
  To: Umang Chheda
  Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240403132839.2117675-1-quic_uchheda@quicinc.com>

On Wed, Apr 03, 2024 at 06:58:39PM +0530, Umang Chheda wrote:
> The Volume Down & Power buttons are controlled by the PMIC via
> the PON hardware, and the Volume Up is connected to a PMIC gpio.
> 
> Enable the necessary hardware and setup the GPIO state for the
> Volume Up gpio key.
> 
> Signed-off-by: Umang Chheda <quic_uchheda@quicinc.com>
> ---

I suppose this isn't a "resend" either, so please send me a new version
of this as well.

Regards,
Bjorn

>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 37 ++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 63ebe0774f1d..73f6d18d2331 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -9,6 +9,8 @@
>  #define PM7250B_SID 8
>  #define PM7250B_SID1 9
>  
> +#include <dt-bindings/input/linux-event-codes.h>
> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>  #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>  #include "sc7280.dtsi"
>  #include "pm7250b.dtsi"
> @@ -39,6 +41,22 @@ chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
>  
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		pinctrl-0 = <&key_vol_up_default>;
> +		pinctrl-names = "default";
> +
> +		key-volume-up {
> +			label = "Volume_up";
> +			gpios = <&pm7325_gpios 6 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_VOLUMEUP>;
> +			wakeup-source;
> +			debounce-interval = <15>;
> +			linux,can-disable;
> +		};
> +	};
> +
>  	reserved-memory {
>  		xbl_mem: xbl@80700000 {
>  			reg = <0x0 0x80700000 0x0 0x100000>;
> @@ -471,6 +489,25 @@ &gcc {
>  			   <GCC_WPSS_RSCP_CLK>;
>  };
>  
> +&pm7325_gpios {
> +	key_vol_up_default: key-vol-up-state {
> +		pins = "gpio6";
> +		function = "normal";
> +		input-enable;
> +		bias-pull-up;
> +		qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
> +	};
> +};
> +
> +&pon_pwrkey {
> +	status = "okay";
> +};
> +
> +&pon_resin {
> +	linux,code = <KEY_VOLUMEDOWN>;
> +	status = "okay";
> +};
> +
>  &qupv3_id_0 {
>  	status = "okay";
>  };
> -- 
> 2.25.1
> 

^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: display: mediatek: Add OF graph support for board path
From: Dmitry Baryshkov @ 2024-04-09 15:20 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: chunkuang.hu, robh, krzysztof.kozlowski+dt, conor+dt, p.zabel,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	matthias.bgg, shawn.sung, yu-chang.lee, ck.hu, jitao.shi,
	devicetree, linux-kernel, dri-devel, linux-mediatek,
	linux-arm-kernel, wenst, kernel
In-Reply-To: <20240409120211.321153-2-angelogioacchino.delregno@collabora.com>

On Tue, Apr 09, 2024 at 02:02:09PM +0200, AngeloGioacchino Del Regno wrote:
> The display IPs in MediaTek SoCs support being interconnected with
> different instances of DDP IPs (for example, merge0 or merge1) and/or
> with different DDP IPs (for example, rdma can be connected with either
> color, dpi, dsi, merge, etc), forming a full Display Data Path that
> ends with an actual display.
> 
> The final display pipeline is effectively board specific, as it does
> depend on the display that is attached to it, and eventually on the
> sensors supported by the board (for example, Adaptive Ambient Light
> would need an Ambient Light Sensor, otherwise it's pointless!), other
> than the output type.

With the color and gamma being in play, should the configuration be
board-driver or rather use-case driven with the driver being able to
reroute some of the blocks at runtime?

> 
> Add support for OF graphs to most of the MediaTek DDP (display) bindings
> to add flexibility to build custom hardware paths, hence enabling board
> specific configuration of the display pipeline and allowing to finally
> migrate away from using hardcoded paths.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH 2/6] soc: qcom: smem: Add pcode/fcode getters
From: Bjorn Andersson @ 2024-04-09 15:20 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
	dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-2-ce2b864251b1@linaro.org>

On Fri, Apr 05, 2024 at 10:41:30AM +0200, Konrad Dybcio wrote:
> Introduce getters for SoC product and feature codes and export them.
> 

Thought I commented on this already, but I don't see my reply...

Can you please elaborate on what this stuff is, such that we have a
track record in the history of this driver as well, for those of us that
don't know what "feature/product codes" contain or are good for (or have
forgotten next week).

Regards,
Bjorn

> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/soc/qcom/smem.c       | 66 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/soc/qcom/smem.h |  2 ++
>  2 files changed, 68 insertions(+)
> 
> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index 7191fa0c087f..e89b4d26877a 100644
> --- a/drivers/soc/qcom/smem.c
> +++ b/drivers/soc/qcom/smem.c
> @@ -795,6 +795,72 @@ int qcom_smem_get_soc_id(u32 *id)
>  }
>  EXPORT_SYMBOL_GPL(qcom_smem_get_soc_id);
>  
> +/**
> + * qcom_smem_get_feature_code() - return the feature code
> + * @id:	On success, we return the feature code here.
> + *
> + * Look up the feature code identifier from SMEM and return it.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_smem_get_feature_code(u32 *code)
> +{
> +	struct socinfo *info;
> +	u32 raw_code;
> +
> +	info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
> +	if (IS_ERR(info))
> +		return PTR_ERR(info);
> +
> +	/* This only makes sense for socinfo >= 16 */
> +	if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
> +		return -EINVAL;
> +
> +	raw_code = __le32_to_cpu(info->feature_code);
> +
> +	/* Ensure the value makes sense */
> +	if (raw_code >= SOCINFO_FC_INT_RESERVE)
> +		raw_code = SOCINFO_FC_UNKNOWN;
> +
> +	*code = raw_code;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_smem_get_feature_code);
> +
> +/**
> + * qcom_smem_get_product_code() - return the product code
> + * @id:	On success, we return the product code here.
> + *
> + * Look up feature code identifier from SMEM and return it.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_smem_get_product_code(u32 *code)
> +{
> +	struct socinfo *info;
> +	u32 raw_code;
> +
> +	info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
> +	if (IS_ERR(info))
> +		return PTR_ERR(info);
> +
> +	/* This only makes sense for socinfo >= 16 */
> +	if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
> +		return -EINVAL;
> +
> +	raw_code = __le32_to_cpu(info->pcode);
> +
> +	/* Ensure the value makes sense */
> +	if (raw_code >= SOCINFO_FC_INT_RESERVE)
> +		raw_code = SOCINFO_FC_UNKNOWN;
> +
> +	*code = raw_code;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_smem_get_product_code);
> +
>  static int qcom_smem_get_sbl_version(struct qcom_smem *smem)
>  {
>  	struct smem_header *header;
> diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h
> index a36a3b9d4929..aef8c9fc6c08 100644
> --- a/include/linux/soc/qcom/smem.h
> +++ b/include/linux/soc/qcom/smem.h
> @@ -13,5 +13,7 @@ int qcom_smem_get_free_space(unsigned host);
>  phys_addr_t qcom_smem_virt_to_phys(void *p);
>  
>  int qcom_smem_get_soc_id(u32 *id);
> +int qcom_smem_get_feature_code(u32 *code);
> +int qcom_smem_get_product_code(u32 *code);
>  
>  #endif
> 
> -- 
> 2.40.1
> 

^ permalink raw reply

* Re: [PATCH 3/6] drm/msm/adreno: Allow specifying default speedbin value
From: Dmitry Baryshkov @ 2024-04-09 15:23 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
	dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <d8a2ef87-f29e-4bdb-a9b8-591b8bd5d2b2@linaro.org>

On Tue, Apr 09, 2024 at 05:12:46PM +0200, Konrad Dybcio wrote:
> 
> 
> On 4/6/24 04:56, Dmitry Baryshkov wrote:
> > On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> > > From: Neil Armstrong <neil.armstrong@linaro.org>
> > > 
> > > Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> > > the highest. Falling back to it when things go wrong is largely
> > > suboptimal, as more often than not, the top frequencies are not
> > > supposed to work on other bins.
> > 
> > Isn't it better to just return an error here instead of trying to guess
> > which speedbin to use?
> 
> Not sure. I'd rather better compatibility for e.g. booting up a new
> laptop with just dt.

New speedbin can have lower max speed, so by attempting to run it at
higher freq you might be breaking it.

> 
> > 
> > If that's not the case, I think the commit should be expanded with
> > actually setting default_speedbin for the existing GPUs.
> 
> I think that should be addressed, although separately.

I'd prefer to have it as a part of this patch, but I'd not NAK it just
for this reason.

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH 5/6] drm/msm/adreno: Add speedbin data for SM8550 / A740
From: Dmitry Baryshkov @ 2024-04-09 15:24 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
	dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <0955cabc-fc4e-4790-a82c-7f6f807fe36b@linaro.org>

On Tue, Apr 09, 2024 at 05:13:15PM +0200, Konrad Dybcio wrote:
> 
> 
> On 4/6/24 05:25, Dmitry Baryshkov wrote:
> > On Fri, Apr 05, 2024 at 10:41:33AM +0200, Konrad Dybcio wrote:
> > > Add speebin data for A740, as found on SM8550 and derivative SoCs.
> > > 
> > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > > ---
> > >   drivers/gpu/drm/msm/adreno/adreno_device.c | 14 ++++++++++++++
> > >   1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > index 901ef767e491..c976a485aef2 100644
> > > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > @@ -570,6 +570,20 @@ static const struct adreno_info gpulist[] = {
> > >   		.zapfw = "a740_zap.mdt",
> > >   		.hwcg = a740_hwcg,
> > >   		.address_space_size = SZ_16G,
> > > +		.speedbins = ADRENO_SPEEDBINS(
> > 
> > I think this deserves either a comment or some info in the commit
> > message.
> 
> "this" = ?

I see two types of speedbins here, it would be nice to understand at
least some reason or some defailts for that (if you know them).

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v2 2/4] arm64: dts: qcom: msm8976: Add MDSS nodes
From: Bjorn Andersson @ 2024-04-09 15:24 UTC (permalink / raw)
  To: Adam Skladowski
  Cc: phone-devel, ~postmarketos/upstreaming, Andy Gross, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
	devicetree, linux-kernel
In-Reply-To: <20240401172153.9231-3-a39.skl@gmail.com>

On Mon, Apr 01, 2024 at 07:21:51PM +0200, Adam Skladowski wrote:
> diff --git a/arch/arm64/boot/dts/qcom/msm8976.dtsi b/arch/arm64/boot/dts/qcom/msm8976.dtsi
[..]
> +		mdss: display-subsystem@1a00000 {
[..]
> +			mdss_dsi0: dsi@1a94000 {
> +				compatible = "qcom,msm8976-dsi-ctrl", "qcom,mdss-dsi-ctrl";
> +				reg = <0x01a94000 0x25c>;
> +				reg-names = "dsi_ctrl";
> +
> +				interrupt-parent = <&mdss>;
> +				interrupts = <4>;
> +
> +				clocks = <&gcc GCC_MDSS_MDP_CLK>,
> +					 <&gcc GCC_MDSS_AHB_CLK>,
> +					 <&gcc GCC_MDSS_AXI_CLK>,
> +					 <&gcc GCC_MDSS_BYTE0_CLK>,
> +					 <&gcc GCC_MDSS_PCLK0_CLK>,
> +					 <&gcc GCC_MDSS_ESC0_CLK>;
> +				clock-names = "mdp_core",
> +					      "iface",
> +					      "bus",
> +					      "byte",
> +					      "pixel",
> +					      "core";
> +
> +				assigned-clocks = <&gcc GCC_MDSS_BYTE0_CLK_SRC>,
> +						  <&gcc GCC_MDSS_PCLK0_CLK_SRC>;
> +				assigned-clock-parents = <&mdss_dsi0_phy 0>,
> +							 <&mdss_dsi0_phy 1>;
> +
> +				phys = <&mdss_dsi0_phy>;
> +
> +				operating-points-v2 = <&dsi0_opp_table>;
> +				power-domains = <&gcc MDSS_GDSC>;
> +
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +

Seems reasonable to keep this disabled as well. Further more &mdss_dsi0
depends on &mdss_dsi0_phy which is disabled.

> +				ports {
[..]
> +			mdss_dsi0_phy: phy@1a94a00 {
> +				compatible = "qcom,dsi-phy-28nm-hpm-fam-b";
> +				reg = <0x01a94a00 0xd4>,
> +				      <0x01a94400 0x280>,
> +				      <0x01a94b80 0x30>;
> +				reg-names = "dsi_pll",
> +					    "dsi_phy",
> +					    "dsi_phy_regulator";
> +
> +				#clock-cells = <1>;
> +				#phy-cells = <0>;
> +
> +				clocks = <&gcc GCC_MDSS_AHB_CLK>,
> +					 <&rpmcc RPM_SMD_XO_CLK_SRC>;
> +				clock-names = "iface", "ref";
> +
> +				status = "disabled";
> +			};

PS. Leave &mdss_mdp enabled...

Regards,
Bjorn

^ permalink raw reply

* Re: [PATCH v2 3/4] arm64: dts: qcom: msm8976: Add Adreno GPU
From: Bjorn Andersson @ 2024-04-09 15:25 UTC (permalink / raw)
  To: Adam Skladowski
  Cc: phone-devel, ~postmarketos/upstreaming, Andy Gross, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
	devicetree, linux-kernel
In-Reply-To: <20240401172153.9231-4-a39.skl@gmail.com>

On Mon, Apr 01, 2024 at 07:21:52PM +0200, Adam Skladowski wrote:
> Add Adreno GPU node.
> 
> Signed-off-by: Adam Skladowski <a39.skl@gmail.com>
> ---
>  arch/arm64/boot/dts/qcom/msm8976.dtsi | 65 +++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8976.dtsi b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> index 6be310079f5b..77670fce9b8f 100644
> --- a/arch/arm64/boot/dts/qcom/msm8976.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> @@ -1074,6 +1074,71 @@ mdss_dsi1_phy: phy@1a96a00 {
>  			};
>  		};
>  
> +		adreno_gpu: gpu@1c00000 {
> +			compatible = "qcom,adreno-510.0", "qcom,adreno";
> +
> +			reg = <0x01c00000 0x40000>;
> +			reg-names = "kgsl_3d0_reg_memory";
> +
> +			interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "kgsl_3d0_irq";
> +
> +			clocks = <&gcc GCC_GFX3D_OXILI_CLK>,
> +				 <&gcc GCC_GFX3D_OXILI_AHB_CLK>,
> +				 <&gcc GCC_GFX3D_OXILI_GMEM_CLK>,
> +				 <&gcc GCC_GFX3D_BIMC_CLK>,
> +				 <&gcc GCC_GFX3D_OXILI_TIMER_CLK>,
> +				 <&gcc GCC_GFX3D_OXILI_AON_CLK>;
> +			clock-names = "core",
> +				      "iface",
> +				      "mem",
> +				      "mem_iface",
> +				      "rbbmtimer",
> +				      "alwayson";
> +
> +			power-domains = <&gcc OXILI_GX_GDSC>;
> +
> +			iommus = <&gpu_iommu 0>;
> +
> +			status = "disabled";

Make status the last property of the node, please.

Regards,
Bjorn

> +
> +			operating-points-v2 = <&gpu_opp_table>;
> +
> +			gpu_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +

^ permalink raw reply

* Re: [PATCH v8 3/8] perf: imx_perf: let the driver manage the counter usage rather the user
From: Will Deacon @ 2024-04-09 15:26 UTC (permalink / raw)
  To: Xu Yang
  Cc: frank.li, mark.rutland, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	shawnguo, s.hauer, kernel, festevam, john.g.garry, jolsa,
	namhyung, irogers, mike.leach, peterz, mingo, acme,
	alexander.shishkin, adrian.hunter, linux-arm-kernel, devicetree,
	linux-kernel, linux-perf-users, imx
In-Reply-To: <20240322063930.749126-3-xu.yang_2@nxp.com>

On Fri, Mar 22, 2024 at 02:39:25PM +0800, Xu Yang wrote:
> In current design, the user of perf app needs to input counter ID to count
> events. However, this is not user-friendly since the user needs to lookup
> the map table to find the counter. Instead of letting the user to input
> the counter, let this driver to manage the counters in this patch.

I think we still have to support the old interface so that we don't break
those existing users (even if the driver just ignores whatever counter ID
is provided in a backwards-compatible way).

> This will be implemented by:
>  1. allocate counter 0 for cycle event.
>  2. find unused counter from 1-10 for reference events.
>  3. allocate specific counter for counter-specific events.
> 
> In this patch, counter attribute is removed too. To mark counter-specific
> events, counter ID will be encoded into perf_pmu_events_attr.id.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v6:
>  - new patch
> Changes in v7:
>  - no changes
> Changes in v8:
>  - add Rb tag
> ---
>  drivers/perf/fsl_imx9_ddr_perf.c | 168 ++++++++++++++++++-------------
>  1 file changed, 99 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/perf/fsl_imx9_ddr_perf.c b/drivers/perf/fsl_imx9_ddr_perf.c
> index 0017f2c9ef91..b728719b494c 100644
> --- a/drivers/perf/fsl_imx9_ddr_perf.c
> +++ b/drivers/perf/fsl_imx9_ddr_perf.c
> @@ -245,14 +249,12 @@ static const struct attribute_group ddr_perf_events_attr_group = {
>  	.attrs = ddr_perf_events_attrs,
>  };
>  
> -PMU_FORMAT_ATTR(event, "config:0-7");
> -PMU_FORMAT_ATTR(counter, "config:8-15");
> +PMU_FORMAT_ATTR(event, "config:0-15");

Sadly, this is a user-visible change so I think it will break old tools,
won't it?

Will

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: usb: mtk-xhci: add compatible for MT7988
From: Rob Herring @ 2024-04-09 15:26 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Krzysztof Kozlowski,
	Conor Dooley, Chunfeng Yun, Greg Kroah-Hartman, Daniel Golle,
	linux-usb, linux-arm-kernel, linux-mediatek, devicetree,
	linux-kernel, Rafał Miłecki
In-Reply-To: <20240213130044.1976-1-zajec5@gmail.com>

On Tue, Feb 13, 2024 at 02:00:43PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> MT7988 SoC contains two on-SoC XHCI controllers. Add proper binding.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 1 +
>  1 file changed, 1 insertion(+)

Seems like this got missed. Applied now.

Rob


^ permalink raw reply

* Re: [PATCH net-next v2 4/5] net: stmmac: add support for RZ/N1 GMAC
From: Geert Uytterhoeven @ 2024-04-09 15:27 UTC (permalink / raw)
  To: Romain Gantois
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, Alexandre Torgue, Jose Abreu,
	Maxime Coquelin, Russell King, Clément Léger,
	Thomas Petazzoni, netdev, devicetree, linux-kernel,
	linux-renesas-soc, linux-stm32, linux-arm-kernel
In-Reply-To: <20240409-rzn1-gmac1-v2-4-79ca45f2fc79@bootlin.com>

Hi Romain,

On Tue, Apr 9, 2024 at 11:21 AM Romain Gantois
<romain.gantois@bootlin.com> wrote:
> From: Clément Léger <clement.leger@bootlin.com>
>
> Add support for the Renesas RZ/N1 GMAC. This support can make use of a
> custom RZ/N1 PCS which is fetched by parsing the pcs-handle device tree
> property.
>
> Signed-off-by: "Clément Léger" <clement.leger@bootlin.com>
> Co-developed-by: Romain Gantois <romain.gantois@bootlin.com>
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>

Thanks for your patch!

> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -142,6 +142,18 @@ config DWMAC_ROCKCHIP
>           This selects the Rockchip RK3288 SoC glue layer support for
>           the stmmac device driver.
>
> +config DWMAC_RZN1
> +       tristate "Renesas RZ/N1 dwmac support"
> +       default ARCH_RZN1

Why default to enabled?

> +       depends on OF && (ARCH_RZN1 || COMPILE_TEST)
> +       select PCS_RZN1_MIIC
> +       help
> +         Support for Ethernet controller on Renesas RZ/N1 SoC family.
> +
> +         This selects the Renesas RZ/N1 SoC glue layer support for
> +         the stmmac device driver. This support can make use of a custom MII
> +         converter PCS device.
> +
>  config DWMAC_SOCFPGA
>         tristate "SOCFPGA dwmac support"
>         default ARCH_INTEL_SOCFPGA

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: iio: adc: Add AD4000
From: Marcelo Schmitt @ 2024-04-09 15:30 UTC (permalink / raw)
  To: David Lechner
  Cc: Marcelo Schmitt, lars, Michael.Hennerich, jic23, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-iio, devicetree,
	linux-kernel
In-Reply-To: <CAMknhBGKNZhGbD7pQ0Z7SMCWqxqGux0LcO_wW0XGP4hLTOwNBg@mail.gmail.com>

On 04/08, David Lechner wrote:
> On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> <marcelo.schmitt@analog.com> wrote:
> >
> > Add device tree documentation for AD4000 family of ADC devices.
> >
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
> >
> 
> Suggested-by: David Lechner <dlechner@baylibre.com>
> 
> (if you still use mostly my suggestions in the end)

Yes, it's been of great help. Will include the tag in future ad4000 DT patches.

> 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> >  .../bindings/iio/adc/adi,ad4000.yaml          | 201 ++++++++++++++++++
> >  MAINTAINERS                                   |   7 +
> >  2 files changed, 208 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > new file mode 100644
> > index 000000000000..ca06afb5149e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > @@ -0,0 +1,201 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices AD4000 and similar Analog to Digital Converters
> > +
> > +maintainers:
> > +  - Marcelo Schmitt <marcelo.schmitt@analog.com>
> > +
> > +description: |
> > +  Analog Devices AD4000 family of Analog to Digital Converters with SPI support.
> > +  Specifications can be found at:
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
> > +
> > +$ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ad4000
> > +      - adi,ad4001
> > +      - adi,ad4002
> > +      - adi,ad4003
> > +      - adi,ad4004
> > +      - adi,ad4005
> > +      - adi,ad4006
> > +      - adi,ad4007
> > +      - adi,ad4008
> > +      - adi,ad4010
> > +      - adi,ad4011
> > +      - adi,ad4020
> > +      - adi,ad4021
> > +      - adi,ad4022
> > +      - adi,adaq4001
> > +      - adi,adaq4003
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 102040816 # for VIO > 2.7 V, 81300813 for VIO > 1.7 V
> > +
> > +  spi-cpha: true
> > +
> > +  adi,spi-mode:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    enum: [ single, chain ]
> 
> It sounds like there are more possible wiring configurations for these
> chips that I thought when suggesting reusing this binding from AD7944
> so we probably need more options here. (see my reply to the cover
> letter for the complete context of these remarks)
> 
> We identified A) an additional wiring configuration where SDI of the
> ADC chip is wired to SDO of the SPI controller and B) a potential need
> to pin mux between wiring modes to work around SPI controller
> limitations perhaps we could omit the adi,spi-mode property and just
> use the standard pinctrl properties.
> 
>   pinctrl-names:
>     description: |
>       Names for possible ways the SDI line of the controller is wired.
> 
>       * default: The SDI line of the ADC is connected to the SDO line of the
>         SPI controller.  CNV line of the ADC is connected to CS of the SPI
>         controller.
Not sure if should be DT, but maybe also point out that in default mode the
SPI controller must be capable of keeping ADC SDI (controller SDO) line high
during ADC conversions.

>       * single: The datasheet calls this "3-wire mode".  (NOTE: The datasheet's
>         definition of 3-wire mode is NOT at all related to the standard
>         spi-3wire property!)  In this mode, SDI is tied to VIO, and the CNV line
>         can be connected to the CS line of the SPI controller (typical) or to a
>         GPIO, in which case the CS line of the controller is unused.  The SDO
>         line of the SPI controller is not connected.
>       * multi: The datasheet calls this "4-wire mode" and is used when multiple
>         chips are connected in parallel.  In this mode, the ADC SDI line is tied
>         to the CS line on the SPI controller and the CNV line is connected to
>         a GPIO.  The SDO line of the SPI controller is not connected.
>       * chain: The datasheet calls this "chain mode".  This mode is used to save
>         on wiring when multiple ADCs are used.  In this mode, the SDI line of
>         one chip is tied to the SDO of the next chip in the chain and the SDI of
>         the last chip in the chain is tied to GND.  Only the first chip in the
>         chain is connected to the SPI bus.  The CNV line of all chips are tied
>         together.  The CS line of the SPI controller can be used as the CNV line
>         only if it is active high.
> 
>       If one name is specified, it is assumed the chip is hard-wired in this
>       configuration.
> 
>       If two names are specified, it is assumed that a pinmux can switch between
>       the two wiring configurations.  The first is the default mode for reading
>       and writing registers on the chip and the second is the mode for reading
>       the conversion data from the chip.
>     oneOf:
>       - items:
>           - enum:
>             - default
>             - single
>             - multi
>             - chain
>       - items:
>           - const: default
>           - enum:
>             - single
>             - multi
>             - chain
> 
>   pinctrl-0:
>     maxItems: 1
> 
>   pinctrl-1:
>     maxItems: 1
> 
> 
> > +    description: |
> > +      This property indicates the SPI wiring configuration.
> > +
> > +      When this property is omitted, it is assumed that the device is using what
> > +      the datasheet calls "4-wire mode". This is the conventional SPI mode used
> > +      when there are multiple devices on the same bus. In this mode, the CNV
> > +      line is used to initiate the conversion and the SDI line is connected to
> > +      CS on the SPI controller.
> > +
> > +      When this property is present, it indicates that the device is using one
> > +      of the following alternative wiring configurations:
> > +
> > +      * single: The datasheet calls this "3-wire mode". (NOTE: The datasheet's
> > +        definition of 3-wire mode is NOT at all related to the standard
> > +        spi-3wire property!) This mode is often used when the ADC is the only
> > +        device on the bus. In this mode, SDI is tied to VIO, and the CNV line
> > +        can be connected to the CS line of the SPI controller or to a GPIO, in
> > +        which case the CS line of the controller is unused.
> > +      * chain: The datasheet calls this "chain mode". This mode is used to save
> > +        on wiring when multiple ADCs are used. In this mode, the SDI line of
> > +        one chip is tied to the SDO of the next chip in the chain and the SDI of
> > +        the last chip in the chain is tied to GND. Only the first chip in the
> > +        chain is connected to the SPI bus. The CNV line of all chips are tied
> > +        together. The CS line of the SPI controller can be used as the CNV line
> > +        only if it is active high.
> > +
> > +  '#daisy-chained-devices': true
> > +
> > +  vdd-supply:
> > +    description: A 1.8V supply that powers the chip (VDD).
> > +
> > +  vio-supply:
> > +    description:
> > +      A 1.8V to 5.5V supply for the digital inputs and outputs (VIO).
> > +
> > +  ref-supply:
> > +    description:
> > +      A 2.5 to 5V supply for the external reference voltage (REF).
> > +
> > +  cnv-gpios:
> > +    description:
> > +      The Convert Input (CNV). This input has multiple functions. It initiates
> > +      the conversions and selects the SPI mode of the device (chain or CS). In
> > +      'single' mode, this property is omitted if the CNV pin is connected to the
> > +      CS line of the SPI controller. If 'single' mode is selected and this GPIO
> > +      is provided, it must be active low.
> 
> Since the conversion is triggered on the low to high transition of
> CNV, I think it only makes sense to have it active high and not active
> low.

The idea was to use the GPIO as a replacement for the controller CS when
in "3-wire"/single mode so we could have simpler handling of SPI transfers.
But if changing transfer to avoid latency then this might not simplify anything
anymore. Will probably drop this last line.

> 
> > +    maxItems: 1
> > +
> > +  adi,high-z-input:
> > +    type: boolean
> > +    description:
> > +      High-Z mode allows the amplifier and RC filter in front of the ADC to be
> > +      chosen based on the signal bandwidth of interest, rather than the settling
> > +      requirements of the switched capacitor SAR ADC inputs.
> > +
> > +  adi,gain-milli:
> > +    description: |
> > +      The hardware gain applied to the ADC input (in milli units).
> > +      The gain provided by the ADC input scaler is defined by the hardware
> > +      connections between chip pins OUT+, R1K-, R1K1-, R1K+, R1K1+, and OUT-.
> > +      If not present, default to 1000 (no actual gain applied).
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [454, 909, 1000, 1900]
> > +    default: 1000
> 
> Same suggestion as in V1 - we should make it clear that this property
> only applies to ADAQ chips (in the description and also a -if: for the
> bindings validator). Also, looking at the datasheet, it looks like
> there are a lot more pins on the ADAQ chips, so I think there are more
> properties missing here.
> 
> Some trivial ones:
> 
> vs-pos-supply (VS+ pin, 0 to 11V supply) and vs-neg-supply (VS- pin,
> -11 to 0V supply)
> 
> pd-amp-gpios (active low) and pd-ref-gpios (active low) for optional
> runtime power management.

Ok, will have closer look to these and other pins described in the datasheet and
include them here too.

> 
> Also the datasheet says the ADAQ chips supports "Single-ended to
> differential conversion". So it seems like we might need some extra
> properties to describe that case (a flag for indicating single-ended
> wiring and an optional voltage supply to describe what is connected to
> the negative input if it isn't tied to GND)

Yes, the differential ADCs also support "Single-ended to differential conversion".
Will provide support those too.

^ permalink raw reply

* Re: [PATCH v6 00/16] power: sequencing: implement the subsystem and add first users
From: Bartosz Golaszewski @ 2024-04-09 15:35 UTC (permalink / raw)
  To: Xilin Wu
  Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Kalle Valo, Bjorn Andersson,
	Konrad Dybcio, Liam Girdwood, Mark Brown, Catalin Marinas,
	Will Deacon, Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven,
	Arnd Bergmann, Neil Armstrong, Marek Szyprowski, Alex Elder,
	Srini Kandagatla, Greg Kroah-Hartman, Abel Vesa,
	Manivannan Sadhasivam, Lukas Wunner, Dmitry Baryshkov,
	linux-bluetooth, netdev, devicetree, linux-kernel, linux-wireless,
	linux-arm-msm, linux-arm-kernel, linux-pci, linux-pm,
	Bartosz Golaszewski
In-Reply-To: <6b63d5d2-5f30-4fbd-a872-91f32dc32c87@gmail.com>

On Sat, Apr 6, 2024 at 5:03 AM Xilin Wu <wuxilin123@gmail.com> wrote:
>
> I tested the patchset on SM8550 and it does give me working WiFi. However I
> seethe following warnings during boot.
>
> [    5.973011] mhi mhi0: Requested to power ON
> [    6.597591] mhi mhi0: Power on setup success
> [    6.597631] sysfs: cannot create duplicate filename '/devices/platform/soc@0/1c00000.pcie/pci0000:00/0000:00:00.0/resource0'
> [    6.597634] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S                 6.9.0-rc1-next-20240328-g955237c9980c #1
> [    6.597635] Hardware name: AYN Odin 2 (DT)
> [    6.597637] Workqueue: async async_run_entry_fn
> [    6.597645] Call trace:
> [    6.597646]  dump_backtrace+0xa0/0x128
> [    6.597649]  show_stack+0x20/0x38
> [    6.597650]  dump_stack_lvl+0x74/0x90
> [    6.597653]  dump_stack+0x18/0x28
> [    6.597654]  sysfs_warn_dup+0x6c/0x90
> [    6.597658]  sysfs_add_bin_file_mode_ns+0xdc/0x100
> [    6.597660]  sysfs_create_bin_file+0x7c/0xb8
> [    6.597662]  pci_create_attr+0xb4/0x1a8
> [    6.597665]  pci_create_resource_files+0x64/0xd0
> [    6.597667]  pci_create_sysfs_dev_files+0x24/0x40
> [    6.597669]  pci_bus_add_device+0x54/0x138
> [    6.597670]  pci_bus_add_devices+0x40/0x98
> [    6.597672]  pci_host_probe+0x70/0xf0
> [    6.597673]  dw_pcie_host_init+0x248/0x658
> [    6.597676]  qcom_pcie_probe+0x234/0x330
> [    6.597677]  platform_probe+0x70/0xd8
> [    6.597680]  really_probe+0xc8/0x3a0
> [    6.597681]  __driver_probe_device+0x84/0x170
> [    6.597682]  driver_probe_device+0x44/0x120
> [    6.597683]  __device_attach_driver+0xc4/0x168
> [    6.597684]  bus_for_each_drv+0x8c/0xf0
> [    6.597686]  __device_attach_async_helper+0xb4/0x118
> [    6.597687]  async_run_entry_fn+0x40/0x178
> [    6.597689]  process_one_work+0x16c/0x410
> [    6.597691]  worker_thread+0x284/0x3a0
> [    6.597693]  kthread+0x118/0x128
> [    6.597693]  ret_from_fork+0x10/0x20
> [    6.597698] ------------[ cut here ]------------
> [    6.597698] proc_dir_entry '0000:00/00.0' already registered
> [    6.597710] WARNING: CPU: 7 PID: 154 at fs/proc/generic.c:375 proc_register+0x138/0x1d0
> [    6.597713] Modules linked in:
> [    6.597714] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S                 6.9.0-rc1-next-20240328-g955237c9980c #1
> [    6.597715] Hardware name: AYN Odin 2 (DT)
> [    6.597716] Workqueue: async async_run_entry_fn
> [    6.597718] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> [    6.597719] pc : proc_register+0x138/0x1d0
> [    6.597721] lr : proc_register+0x138/0x1d0
> [    6.597723] sp : ffff800081e3b9a0
> [    6.597723] x29: ffff800081e3b9a0 x28: 0000000000000000 x27: ffffddb2a28eabe0
> [    6.597725] x26: ffff3425c9ada5c0 x25: ffffddb2a2d4eef0 x24: ffff3425c9ada540
> [    6.597726] x23: 0000000000000004 x22: ffff3425c7b1822c x21: 0000000000000004
> [    6.597727] x20: ffff3425c7b18180 x19: ffff3425c9adaec8 x18: ffffffffffffffff
> [    6.597729] x17: 3040636f732f6d72 x16: 6f6674616c702f73 x15: ffff800081e3b910
> [    6.597730] x14: 0000000000000000 x13: 0a64657265747369 x12: 6765722079646165
> [    6.597731] x11: fffffffffff00000 x10: ffffddb2a27c4fb0 x9 : ffffddb29f5d7528
> [    6.597733] x8 : 00000000ffff7fff x7 : ffffddb2a27c4fb0 x6 : 80000000ffff8000
> [    6.597734] x5 : 0000000000000358 x4 : 0000000000000000 x3 : 00000000ffffffff
> [    6.597736] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff3425c5ce0000
> [    6.597737] Call trace:
> [    6.597737]  proc_register+0x138/0x1d0
> [    6.597739]  proc_create_data+0x48/0x78
> [    6.597741]  pci_proc_attach_device+0x84/0x118
> [    6.597743]  pci_bus_add_device+0x5c/0x138
> [    6.597744]  pci_bus_add_devices+0x40/0x98
> [    6.597745]  pci_host_probe+0x70/0xf0
> [    6.597746]  dw_pcie_host_init+0x248/0x658
> [    6.597748]  qcom_pcie_probe+0x234/0x330
> [    6.597749]  platform_probe+0x70/0xd8
> [    6.597750]  really_probe+0xc8/0x3a0
> [    6.597751]  __driver_probe_device+0x84/0x170
> [    6.597752]  driver_probe_device+0x44/0x120
> [    6.597753]  __device_attach_driver+0xc4/0x168
> [    6.597754]  bus_for_each_drv+0x8c/0xf0
> [    6.597756]  __device_attach_async_helper+0xb4/0x118
> [    6.597757]  async_run_entry_fn+0x40/0x178
> [    6.597759]  process_one_work+0x16c/0x410
> [    6.597760]  worker_thread+0x284/0x3a0
> [    6.597761]  kthread+0x118/0x128
> [    6.597762]  ret_from_fork+0x10/0x20
> [    6.597763] ---[ end trace 0000000000000000 ]---
>
> This probably only occurs when the relevant drivers on compiled as built-in.
> Similar behavior has been noticed before as well:
>
> https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/#mdeeca9bc8e19458787d53738298abcfff443068a
>
> Thanks,
> Xilin
>

Thanks for the report. The reason for this was populating the platform
devices before the bridge device was fully added. In case of loadable
modules this meant the pwrctl probe would be deferred long enough for
that to complete so I didn't see it but with pwrctl built-in this
would trigger the problem. I fixed it locally and will resend with
that addressed.

Bart

^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: display: mediatek: Add OF graph support for board path
From: AngeloGioacchino Del Regno @ 2024-04-09 15:41 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: chunkuang.hu, robh, krzysztof.kozlowski+dt, conor+dt, p.zabel,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	matthias.bgg, shawn.sung, yu-chang.lee, ck.hu, jitao.shi,
	devicetree, linux-kernel, dri-devel, linux-mediatek,
	linux-arm-kernel, wenst, kernel
In-Reply-To: <oe75tx35rd27r2a24ofdxfaqwr53tylfp5fwz3nrwc2uz6nmrs@vwc2krbpy3fh>

Il 09/04/24 17:20, Dmitry Baryshkov ha scritto:
> On Tue, Apr 09, 2024 at 02:02:09PM +0200, AngeloGioacchino Del Regno wrote:
>> The display IPs in MediaTek SoCs support being interconnected with
>> different instances of DDP IPs (for example, merge0 or merge1) and/or
>> with different DDP IPs (for example, rdma can be connected with either
>> color, dpi, dsi, merge, etc), forming a full Display Data Path that
>> ends with an actual display.
>>
>> The final display pipeline is effectively board specific, as it does
>> depend on the display that is attached to it, and eventually on the
>> sensors supported by the board (for example, Adaptive Ambient Light
>> would need an Ambient Light Sensor, otherwise it's pointless!), other
>> than the output type.
> 
> With the color and gamma being in play, should the configuration be
> board-driver or rather use-case driven with the driver being able to
> reroute some of the blocks at runtime?
> 

The driver can already set some blocks to "BYPASS MODE" at runtime, meaning
that those will work as simple pass-through, performing *no* processing at
all, so that's addressed from the very beginning.

This doesn't mean that a specific pipeline must always support the "DISP_GAMMA"
or the "DISP_CCOLOR" block(s) alone, or together, or in combination with another
specific block.

For any other question, clarification, etc, I'm here :-)

Cheers!

>>
>> Add support for OF graphs to most of the MediaTek DDP (display) bindings
>> to add flexibility to build custom hardware paths, hence enabling board
>> specific configuration of the display pipeline and allowing to finally
>> migrate away from using hardcoded paths.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 


^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: display: mediatek: Add OF graph support for board path
From: Dmitry Baryshkov @ 2024-04-09 15:45 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: chunkuang.hu, robh, krzysztof.kozlowski+dt, conor+dt, p.zabel,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	matthias.bgg, shawn.sung, yu-chang.lee, ck.hu, jitao.shi,
	devicetree, linux-kernel, dri-devel, linux-mediatek,
	linux-arm-kernel, wenst, kernel
In-Reply-To: <8600acf8-7b51-456b-8a81-4233cfd6f121@collabora.com>

On Tue, 9 Apr 2024 at 18:41, AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 09/04/24 17:20, Dmitry Baryshkov ha scritto:
> > On Tue, Apr 09, 2024 at 02:02:09PM +0200, AngeloGioacchino Del Regno wrote:
> >> The display IPs in MediaTek SoCs support being interconnected with
> >> different instances of DDP IPs (for example, merge0 or merge1) and/or
> >> with different DDP IPs (for example, rdma can be connected with either
> >> color, dpi, dsi, merge, etc), forming a full Display Data Path that
> >> ends with an actual display.
> >>
> >> The final display pipeline is effectively board specific, as it does
> >> depend on the display that is attached to it, and eventually on the
> >> sensors supported by the board (for example, Adaptive Ambient Light
> >> would need an Ambient Light Sensor, otherwise it's pointless!), other
> >> than the output type.
> >
> > With the color and gamma being in play, should the configuration be
> > board-driver or rather use-case driven with the driver being able to
> > reroute some of the blocks at runtime?
> >
>
> The driver can already set some blocks to "BYPASS MODE" at runtime, meaning
> that those will work as simple pass-through, performing *no* processing at
> all, so that's addressed from the very beginning.
>
> This doesn't mean that a specific pipeline must always support the "DISP_GAMMA"
> or the "DISP_CCOLOR" block(s) alone, or together, or in combination with another
> specific block.

I was thinking about slightly different case: do you have enough
colour blocks to drive all outputs or do you have to select them for
the particular output only?

(excuse me, I didn't check the platform details).

> For any other question, clarification, etc, I'm here :-)
>
> Cheers!
>
> >>
> >> Add support for OF graphs to most of the MediaTek DDP (display) bindings
> >> to add flexibility to build custom hardware paths, hence enabling board
> >> specific configuration of the display pipeline and allowing to finally
> >> migrate away from using hardcoded paths.
> >>
> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >
>


-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v3 01/18] ASoC: dt-bindings: mediatek,mt8365-afe: Add audio afe document
From: Krzysztof Kozlowski @ 2024-04-09 15:46 UTC (permalink / raw)
  To: Alexandre Mergnat, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Lee Jones, Flora Fu, Jaroslav Kysela,
	Takashi Iwai, Sumit Semwal, Christian König, Catalin Marinas,
	Will Deacon, Rob Herring
  Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, linux-media, dri-devel, linaro-mm-sig
In-Reply-To: <20240226-audio-i350-v3-1-16bb2c974c55@baylibre.com>

On 09/04/2024 15:41, Alexandre Mergnat wrote:
> Add MT8365 audio front-end bindings
> 
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---

> +properties:
> +  compatible:
> +    const: mediatek,mt8365-afe-pcm
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#sound-dai-cells":
> +    const: 0
> +
> +  clocks:
> +    items:
> +      - description: 26M clock
> +      - description: mux for audio clock
> +      - description: audio i2s0 mck
> +      - description: audio i2s1 mck
> +      - description: audio i2s2 mck
> +      - description: audio i2s3 mck
> +      - description: engen 1 clock
> +      - description: engen 2 clock
> +      - description: audio 1 clock
> +      - description: audio 2 clock
> +      - description: mux for i2s0
> +      - description: mux for i2s1
> +      - description: mux for i2s2
> +      - description: mux for i2s3
> +
> +  clock-names:
> +    items:
> +      - const: top_clk26m_clk
> +      - const: top_audio_sel
> +      - const: audio_i2s0_m
> +      - const: audio_i2s1_m
> +      - const: audio_i2s2_m
> +      - const: audio_i2s3_m
> +      - const: engen1
> +      - const: engen2
> +      - const: aud1
> +      - const: aud2
> +      - const: i2s0_m_sel
> +      - const: i2s1_m_sel
> +      - const: i2s2_m_sel
> +      - const: i2s3_m_sel
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  mediatek,dmic-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Indicates how many data pins are used to transmit two channels of PDM
> +      signal. 1 means two wires, 0 means one wire. Default value is 0.
> +    enum:
> +      - 0 # one wire
> +      - 1 # two wires
> +
> +  mediatek,topckgen:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: The phandle of the mediatek topckgen controller

Nothing improved, so again, so something which is not obvious. What is
it used for? Why AFE needs topckgen for example?

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - power-domains
> +  - mediatek,topckgen
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/mediatek,mt8365-clk.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/power/mediatek,mt8365-power.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        afe@11220000 {

Did you implement the comment or decided to keep afe?

BTW, whatever "consistency" you have in mind, it does not really matter
that much for that example. And for sure do not add incorrect code
intentionally just to fix it in next patch.



Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH RFC 02/11] dt-bindings: riscv: Add Sdtrig optional CSRs existence on DT
From: Conor Dooley @ 2024-04-09 15:49 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Conor Dooley, Max Hsu, Rob Herring, Krzysztof Kozlowski,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Rafael J. Wysocki,
	Pavel Machek, Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan,
	Palmer Dabbelt, linux-riscv, devicetree, linux-kernel, linux-pm,
	kvm, kvm-riscv, linux-kselftest
In-Reply-To: <20240405-ebdb2943657ab08d2d563c03@orel>

[-- Attachment #1: Type: text/plain, Size: 2087 bytes --]

On Fri, Apr 05, 2024 at 05:59:41PM +0200, Andrew Jones wrote:
> On Fri, Mar 29, 2024 at 10:31:10AM +0000, Conor Dooley wrote:
> > On Fri, Mar 29, 2024 at 05:26:18PM +0800, Max Hsu wrote:
> > > The mcontext/hcontext/scontext CSRs are optional in the Sdtrig extension,
> > > to prevent RW operations to the missing CSRs, which will cause
> > > illegal instructions.
> > > 
> > > As a solution, we have proposed the dt format for these CSRs.
> > 
> > As I mentioned in your other patch, I amn't sure what the actual value
> > is in being told about "sdtrig" itself if so many of the CSRs are
> > optional. I think we should define pseudo extensions that represent
> > usable subsets that are allowed by riscv,isa-extensions, such as
> > those you describe here: sdtrig + mcontext, sdtrig + scontext and
> > sdtrig + hcontext. Probably also for strig + mscontext. What
> > additional value does having a debug child node give us that makes
> > it worth having over something like the above?
> 
> Yeah, Sdtrig, which doesn't tell you what you get, isn't nice at all.
> I wonder if we can start with requiring Sdtrig to be accompanied by
> Ssstrict in order to enable the context CSRs, i.e.
> 
>  Sdtrig          - support without optional CSRs
>  Sdtrig+Ssstrict - probe for optional CSRs, support what's found
> 
> If there are platforms with Sdtrig and optional CSRs, but not Ssstrict,
> then maybe the optional CSRs can be detected in some vendor-specific way,
> where the decision as to whether or not that vendor-specific way is
> acceptable is handled case-by-case.

I think it's pretty reasonable to make sstrict a requirement for the
kernel's use of sdtrig. If we have some non-sstrict systems that do
implement these particular CSRs, then I guess we can add some psuedo
instructions then (and nothing would stop the sstrict systems also
specifying directly). If they're using some non-standard CSRs then
case-by-case I guess.

I'm just specifically not keen on adding extra dt properties that do
things we can already do with the ones we have!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v3 02/18] ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document
From: Krzysztof Kozlowski @ 2024-04-09 15:51 UTC (permalink / raw)
  To: Alexandre Mergnat, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Lee Jones, Flora Fu, Jaroslav Kysela,
	Takashi Iwai, Sumit Semwal, Christian König, Catalin Marinas,
	Will Deacon, Rob Herring
  Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, linux-media, dri-devel, linaro-mm-sig
In-Reply-To: <20240226-audio-i350-v3-2-16bb2c974c55@baylibre.com>

On 09/04/2024 15:42, Alexandre Mergnat wrote:
> Add soundcard bindings for the MT8365 SoC with the MT6357 audio codec.
> 
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>


> +patternProperties:
> +  "^dai-link-[0-9]+$":
> +    type: object
> +    description:
> +      Container for dai-link level properties and CODEC sub-nodes.
> +
> +    properties:
> +      codec:
> +        type: object
> +        description: Holds subnode which indicates codec dai.
> +
> +        properties:
> +          sound-dai:
> +            maxItems: 1
> +            description: phandle of the codec DAI
> +
> +        additionalProperties: false
> +
> +      link-name:
> +        description:
> +          This property corresponds to the name of the BE dai-link to which
> +          we are going to update parameters in this node.
> +        items:
> +          const: 2ND_I2S_BE

What is the type of link-name? Why is it fixed? How can you have here
multiple dai links if all of them must have the same name?

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 05/11] spi: cadence-qspi: add FIFO depth detection quirk
From: Mark Brown @ 2024-04-09 15:51 UTC (permalink / raw)
  To: Théo Lebrun
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vaishnav Achath,
	Thomas Bogendoerfer, Rob Herring, linux-spi, devicetree,
	linux-kernel, linux-mips, Vladimir Kondratiev, Gregory CLEMENT,
	Thomas Petazzoni, Tawfik Bayouk
In-Reply-To: <D0FIC34Z35BV.1RT6NNGWA85SL@bootlin.com>

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

On Tue, Apr 09, 2024 at 12:07:56PM +0200, Théo Lebrun wrote:

>  - (3) Make DT property optional for all compatibles.
>     - (3a) If provided, warn if runtime detect value is different.
>     - (3b) If provided, do not detect+warn.

I think either of these is fine.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply


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