* [PATCH v2 5/5] ARM: dts: aspeed: drop unused ref_voltage ADC property
From: Krzysztof Kozlowski @ 2024-04-05 6:46 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
Andrew Jeffery, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel
Cc: Krzysztof Kozlowski
In-Reply-To: <20240405064624.18997-1-krzysztof.kozlowski@linaro.org>
Aspeed ADC "ref_voltage" property is neither documented nor used.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. None
---
arch/arm/boot/dts/aspeed/aspeed-bmc-ampere-mtmitchell.dts | 1 -
arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-greatlakes.dts | 2 --
arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts | 2 --
3 files changed, 5 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-ampere-mtmitchell.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-ampere-mtmitchell.dts
index 7b540880cef9..3c8925034a8c 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-ampere-mtmitchell.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-ampere-mtmitchell.dts
@@ -813,7 +813,6 @@ bmc_ast2600_cpu: temperature-sensor@35 {
};
&adc0 {
- ref_voltage = <2500>;
status = "okay";
pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-greatlakes.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-greatlakes.dts
index 9a6757dd203f..998598c15fd0 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-greatlakes.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-greatlakes.dts
@@ -211,7 +211,6 @@ &i2c13 {
};
&adc0 {
- ref_voltage = <2500>;
status = "okay";
pinctrl-0 = <&pinctrl_adc0_default &pinctrl_adc1_default
&pinctrl_adc2_default &pinctrl_adc3_default
@@ -220,7 +219,6 @@ &pinctrl_adc4_default &pinctrl_adc5_default
};
&adc1 {
- ref_voltage = <2500>;
status = "okay";
pinctrl-0 = <&pinctrl_adc8_default &pinctrl_adc10_default
&pinctrl_adc11_default &pinctrl_adc12_default
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
index 1bd555a6c3ba..b4d003cbae8b 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
@@ -596,7 +596,6 @@ i2c-mux@72 {
};
&adc0 {
- ref_voltage = <2500>;
status = "okay";
pinctrl-0 = <&pinctrl_adc0_default &pinctrl_adc1_default
&pinctrl_adc2_default &pinctrl_adc3_default
@@ -605,7 +604,6 @@ &pinctrl_adc4_default &pinctrl_adc5_default
};
&adc1 {
- ref_voltage = <2500>;
status = "okay";
pinctrl-0 = <&pinctrl_adc8_default &pinctrl_adc9_default>;
};
--
2.34.1
^ permalink raw reply related
* [PATCH v2 4/5] ARM: dts: aspeed: harma: correct Mellanox multi-host property
From: Krzysztof Kozlowski @ 2024-04-05 6:46 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
Andrew Jeffery, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel
Cc: Krzysztof Kozlowski
In-Reply-To: <20240405064624.18997-1-krzysztof.kozlowski@linaro.org>
"mlx,multi-host" is using incorrect vendor prefix and is not documented.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. New patch
---
arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-harma.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-harma.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-harma.dts
index bbbab8023cd6..b2834399f3d1 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-harma.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-harma.dts
@@ -137,7 +137,7 @@ &mac3 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_rmii4_default>;
use-ncsi;
- mlx,multi-host;
+ mellanox,multi-host;
};
&rtc {
--
2.34.1
^ permalink raw reply related
* [PATCH v2 3/5] ARM: dts: aspeed: yosemitev2: correct Mellanox multi-host property
From: Krzysztof Kozlowski @ 2024-04-05 6:46 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
Andrew Jeffery, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel
Cc: Krzysztof Kozlowski
In-Reply-To: <20240405064624.18997-1-krzysztof.kozlowski@linaro.org>
"mlx,multi-host" is using incorrect vendor prefix and is not documented.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. None
---
arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemitev2.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemitev2.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemitev2.dts
index 6bf2ff85a40e..5143f85fbd70 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemitev2.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemitev2.dts
@@ -95,7 +95,7 @@ &mac0 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_rmii1_default>;
use-ncsi;
- mlx,multi-host;
+ mellanox,multi-host;
};
&adc {
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/5] ARM: dts: aspeed: yosemite4: correct Mellanox multi-host property
From: Krzysztof Kozlowski @ 2024-04-05 6:46 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
Andrew Jeffery, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel
Cc: Krzysztof Kozlowski
In-Reply-To: <20240405064624.18997-1-krzysztof.kozlowski@linaro.org>
"mlx,multi-host" is using incorrect vendor prefix and is not documented.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. None
---
arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
index 64075cc41d92..1bd555a6c3ba 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
@@ -88,7 +88,7 @@ &mac2 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_rmii3_default>;
use-ncsi;
- mlx,multi-host;
+ mellanox,multi-host;
};
&mac3 {
@@ -96,7 +96,7 @@ &mac3 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_rmii4_default>;
use-ncsi;
- mlx,multi-host;
+ mellanox,multi-host;
};
&fmc {
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/5] ARM: dts: aspeed: greatlakes: correct Mellanox multi-host property
From: Krzysztof Kozlowski @ 2024-04-05 6:46 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
Andrew Jeffery, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel
Cc: Krzysztof Kozlowski
"mlx,multi-host" is using incorrect vendor prefix and is not documented.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. None
---
arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-greatlakes.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-greatlakes.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-greatlakes.dts
index 7a53f54833a0..9a6757dd203f 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-greatlakes.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-greatlakes.dts
@@ -66,7 +66,7 @@ &mac3 {
pinctrl-0 = <&pinctrl_rmii4_default>;
no-hw-checksum;
use-ncsi;
- mlx,multi-host;
+ mellanox,multi-host;
ncsi-ctrl,start-redo-probe;
ncsi-ctrl,no-channel-monitor;
ncsi-package = <1>;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]
From: Krzysztof Kozlowski @ 2024-04-05 6:41 UTC (permalink / raw)
To: Frank Li, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list
In-Reply-To: <20240404161914.1655305-1-Frank.Li@nxp.com>
On 04/04/2024 18:19, Frank Li wrote:
> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
> i2c bus mux to connect both i2c devices. One will probe failure and other
> will probe success when devices driver check whoami. So one dtb can cover
> both board configuration.
I don't understand it. Either you add real device or not. If one board
has two devices, then why do you need to check for failures?
Anyway, don't add fake stuff to DTS.
NAK.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 210 ++++++++++++++++++
> 1 file changed, 210 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> index 8360bb851ac03..adff87c7cf305 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> @@ -30,6 +30,13 @@ reg_usdhc2_vmmc: usdhc2-vmmc {
> enable-active-high;
> };
>
> + reg_audio: regulator-wm8962 {
> + compatible = "regulator-fixed";
> + regulator-name = "3v3_aud";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> gpio-sbu-mux {
> compatible = "nxp,cbdtu02043", "gpio-sbu-mux";
> pinctrl-names = "default";
> @@ -44,6 +51,105 @@ usb3_data_ss: endpoint {
> };
> };
> };
> +
> + sound-wm8960 {
> + compatible = "fsl,imx-audio-wm8960";
> + model = "wm8960-audio";
> + audio-cpu = <&sai1>;
> + audio-codec = <&wm8960>;
> + hp-det-gpio = <&lsio_gpio1 0 GPIO_ACTIVE_HIGH>;
> + audio-routing =
> + "Headphone Jack", "HP_L",
> + "Headphone Jack", "HP_R",
> + "Ext Spk", "SPK_LP",
> + "Ext Spk", "SPK_LN",
> + "Ext Spk", "SPK_RP",
> + "Ext Spk", "SPK_RN",
> + "LINPUT1", "Mic Jack",
> + "Mic Jack", "MICB";
> + };
> +
> + sound-wm8962 {
> + compatible = "fsl,imx-audio-wm8962";
> + model = "wm8962-audio";
> + audio-cpu = <&sai1>;
> + audio-codec = <&wm8962>;
> + hp-det-gpio = <&lsio_gpio1 0 GPIO_ACTIVE_HIGH>;
> + audio-routing =
> + "Headphone Jack", "HPOUTL",
> + "Headphone Jack", "HPOUTR",
> + "Ext Spk", "SPKOUTL",
> + "Ext Spk", "SPKOUTR",
> + "AMIC", "MICBIAS",
> + "IN3R", "AMIC",
> + "IN1R", "AMIC";
> + };
> +
> + /*
> + * This dummy i2c mux. GPIO actually will not impact selection. At actual boards, only 1
> + * device connectted. I2C client driver will check ID when probe. Only matched ID's driver
> + * probe successfully.
NAK
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]
From: Francesco Dolcini @ 2024-04-05 6:36 UTC (permalink / raw)
To: Frank Li
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list
In-Reply-To: <20240404161914.1655305-1-Frank.Li@nxp.com>
Hello Frank,
On Thu, Apr 04, 2024 at 12:19:13PM -0400, Frank Li wrote:
> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
> i2c bus mux to connect both i2c devices. One will probe failure and other
> will probe success when devices driver check whoami. So one dtb can cover
> both board configuration.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 210 ++++++++++++++++++
> 1 file changed, 210 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> index 8360bb851ac03..adff87c7cf305 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> @@ -44,6 +51,105 @@ usb3_data_ss: endpoint {
[...]
> + /*
> + * This dummy i2c mux. GPIO actually will not impact selection. At actual boards, only 1
> + * device connectted. I2C client driver will check ID when probe. Only matched ID's driver
> + * probe successfully.
> + */
> + i2cvmux: i2cmux {
> + compatible = "i2c-mux-gpio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + mux-gpios = <&lsio_gpio5 0 GPIO_ACTIVE_HIGH>; /* use an unused gpio */
There is for sure people that have more experience and competency that
me and it would be interesting to hear their feedback, but this
looks like a bad hack, and you are just playing with the driver
behavior to ensure that you get what you need.
Francesco
^ permalink raw reply
* Re: [PATCH 2/2] mfd: rohm-bd71828: Add software-compatible variant BD71879
From: Lee Jones @ 2024-04-05 6:35 UTC (permalink / raw)
To: Andreas Kemnade
Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, mazziesaccount,
devicetree, linux-kernel
In-Reply-To: <20240402193515.513713-3-andreas@kemnade.info>
On Tue, 02 Apr 2024, Andreas Kemnade wrote:
> Add the BD71879 PMIC which is software-compatible to the BD71828, so reuse
> the same device_type enum.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> Suggested-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
> drivers/mfd/rohm-bd71828.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> index 4a1fa8a0d76a..f0b444690d4d 100644
> --- a/drivers/mfd/rohm-bd71828.c
> +++ b/drivers/mfd/rohm-bd71828.c
> @@ -585,6 +585,10 @@ static const struct of_device_id bd71828_of_match[] = {
> {
> .compatible = "rohm,bd71828",
> .data = (void *)ROHM_CHIP_TYPE_BD71828,
> + }, {
> + .compatible = "rohm,bd71879",
> + /* equivalent from a software point of view */
Nit: Sentences start with capital letters.
> + .data = (void *)ROHM_CHIP_TYPE_BD71828,
> }, {
> .compatible = "rohm,bd71815",
> .data = (void *)ROHM_CHIP_TYPE_BD71815,
> --
> 2.39.2
>
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879
From: Krzysztof Kozlowski @ 2024-04-05 6:35 UTC (permalink / raw)
To: Matti Vaittinen, Andreas Kemnade
Cc: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-kernel
In-Reply-To: <0295c8db-b01e-45e7-b44e-79d36d81cd48@gmail.com>
On 05/04/2024 08:01, Matti Vaittinen wrote:
> On 4/4/24 15:04, Krzysztof Kozlowski wrote:
>> On 04/04/2024 12:30, Andreas Kemnade wrote:
>>> On Thu, 4 Apr 2024 08:59:54 +0200
>>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>>> On 02/04/2024 21:35, Andreas Kemnade wrote:
>>>>> As this chip was seen in several devices in the wild, add it.
>>>>>
>>>>> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
>>>>> Suggested-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>>>> ---
>>>>> Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>>> index 0b62f854bf6b..e4df09e8961c 100644
>>>>> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>>> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>>> @@ -17,7 +17,9 @@ description: |
>>>>>
>>>>> properties:
>>>>> compatible:
>>>>> - const: rohm,bd71828
>>>>> + enum:
>>>>> + - rohm,bd71828
>>>>> + - rohm,bd71879
>>>>
>>>> In your second commit you claim they are compatible, so why they are not
>>>> marked as such?
>>>>
>>> so you mean allowing
>>>
>>> compatible = "rohm,bd71828"
>>> and
>>> compatible = "rohm,bd71879", "rohm,bd71828"
>
>
> This makes me slightly nervous. It wouldn't be the first time when I've
> been told "they are similar", and later the reality has turned out to be
> "they are similar, except...". Furthermore, even if these devices seem
> similar to software (which is what the comment in the MFD driver is
> referring to), it does not mean these devices are 100% electrically
> compatible so that they could be used as a "drop-in" replacement to each
> others. I wouldn't guarantee that.
compatibility does not mean that. It is only about software interface.
>
> Furthermore, my current understanding is that the BD71828 was a model
> that was used for a limited purposes. So, maybe creating an dt-entry like:
> compatible = "rohm,bd71879", "rohm,bd71828"
>
> might not prove to be too useful. (But I'm not 100% certain on this).
>
>> Yes. If there are reasons against, please briefly mention them in commit
>> msg.
>
> I would like to understand the rationale for allowing:
> compatible = "rohm,bd71879", "rohm,bd71828".
>
> Is the intention to:
> 1) allow boards which tell the software that "the hardware may be
> bd71828 or bd71879", or
> 2) to tell a binding reader that these ICs are likely to be usable as
> replacements to each others?
> (Or, is there some other rationale beyond these?)
None of these. Compatibility means that software can use fallback
compatible as matching mechanism and everything will work fine or with
reduced set of features, but still working.
>
> If it's 1), then I see limited sense in doing so, while I expect that
> not so many bd71828 variants will be seen out there - and at least not
> in that many different products. If it's the 2), then I wouldn't say we
> have the facts to do this.
> And, as always, if there is 3), 4), ... - I am keen to learn :)
>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879
From: Andreas Kemnade @ 2024-04-05 6:31 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Krzysztof Kozlowski, lee, robh+dt, krzysztof.kozlowski+dt,
conor+dt, devicetree, linux-kernel
In-Reply-To: <0295c8db-b01e-45e7-b44e-79d36d81cd48@gmail.com>
On Fri, 5 Apr 2024 09:01:25 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> > Yes. If there are reasons against, please briefly mention them in commit
> > msg.
>
> I would like to understand the rationale for allowing:
> compatible = "rohm,bd71879", "rohm,bd71828".
>
> Is the intention to:
> 1) allow boards which tell the software that "the hardware may be
> bd71828 or bd71879", or
> 2) to tell a binding reader that these ICs are likely to be usable as
> replacements to each others?
> (Or, is there some other rationale beyond these?)
As far as I understand the second compatible can be a fallback for
software which does not know the bd71879 (remember the devicetree is
not only for linux). Especially if that software does not use the full
functionality of the device, like drivers in bootloaders do. E.g.
mmc drivers in bootloaders might not use high speed modes at all, so
differences in them do not matter.
Regards,
Andreas
^ permalink raw reply
* Re: [PATCH 4/5] clk: qcom: Add camera clock controller driver for SM8150
From: Satya Priya Kakitapalli (Temp) @ 2024-04-05 6:27 UTC (permalink / raw)
To: Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio,
Michael Turquette, Stephen Boyd, Abhishek Sahu, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Stephen Boyd, linux-arm-msm, linux-clk, linux-kernel, devicetree,
Ajit Pandey, Imran Shaik, Taniya Das, Jagadeesh Kona
In-Reply-To: <18567989-fb60-49ae-92e6-94e1bc2fa1c7@linaro.org>
On 3/2/2024 9:43 PM, Bryan O'Donoghue wrote:
> On 29/02/2024 5:38 a.m., Satya Priya Kakitapalli wrote:
>> Add support for the camera clock controller for camera clients
>> to be able to request for camcc clocks on SM8150 platform.
>>
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> ---
>
>> +static int cam_cc_sm8150_probe(struct platform_device *pdev)
>> +{
>> + struct regmap *regmap;
>> + int ret;
>> +
>> + ret = devm_pm_runtime_enable(&pdev->dev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = pm_runtime_resume_and_get(&pdev->dev);
>> + if (ret)
>> + return ret;
>> +
>> + regmap = qcom_cc_map(pdev, &cam_cc_sm8150_desc);
>> + if (IS_ERR(regmap)) {
>> + pm_runtime_put(&pdev->dev);
>> + return PTR_ERR(regmap);
>> + }
>> +
>> + clk_trion_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
>> + clk_trion_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config);
>> + clk_regera_pll_configure(&cam_cc_pll2, regmap,
>> &cam_cc_pll2_config);
>> + clk_trion_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config);
>> + clk_trion_pll_configure(&cam_cc_pll4, regmap, &cam_cc_pll4_config);
>> +
>> + /* Keep the critical clock always-on */
>> + qcom_branch_set_clk_en(regmap, 0xc1e4); /* cam_cc_gdsc_clk */
>
> Does this clock need to be specified this way ?
>
> drivers/clk/qcom/camcc-sc8280xp.c::camcc_gdsc_clk specifies the gdsc
> clock as a shared op clock.
>
> Actually it looks to be register compatible, please try defining
> titan_top_gdsc as per the example in 8280xp.
>
>> +
>> + ret = qcom_cc_really_probe(pdev, &cam_cc_sm8150_desc, regmap);
>> +
>> + pm_runtime_put(&pdev->dev);
>> +
>> + return ret;
>> +}
>
> So this is a pattern we keep repeating in the clock probe() functions
> which I am writing a series to address. There's no need to continue to
> replicate the bug in new code though.
>
> Only switch on always-on clocks if probe succeeds.
>
> ret = qcom_cc_really_probe(pdev, &cam_cc_sm8150_desc, regmap);
> if (ret)
> goto probe_err;
>
> qcom_branch_set_clk_en(regmap, 0xc1e4); /* cam_cc_gdsc_clk */
>
> pm_runtime_put(&pdev->dev);
>
> return 0;
>
> probe_err:
> pm_runtime_put_sync(&pdev->dev);
>
> Alternatively switch on the always-on clocks before the really_probe()
> but then roll back in a probe_err: goto
>
> probe_err:
> remap_bits_update(regmap, 0xc1e4, BIT(0), 0);
> pm_runtime_put_sync(&pdev->dev);
>
> There may be corner cases where always-on has to happen before
> really_probe() I suppose but as a general pattern the above should be
> how we go.
>
I have rechecked this and see that this clock is PoR ON (i.e BIT(0) is
set upon power ON) and it should be kept always ON as per HW
recommendation. So even if the probe fails we shouldn't be clearing it
against the hw recommendation. We are setting the bit here again to make
sure it is set when the driver probes.
> Anyway I suspect the right thing to do is to define a
> titan_top_gdsc_clk with shared ops to "park" the GDSC clock to 19.2
> MHz instead of turning it off.
>
> You can get rid of the hard-coded always-on and indeed represent the
> clock in /sysfs - which is preferable IMO to just whacking registers
> to keep clocks always-on in probe anyway.
>
> Please try to define the titan_top_gdsc_clk as a shared_ops clock
> instead of hard coding to always on.
>
> If that doesn't work for some reason, then please fix your always-on
> logic in probe() to only make the clock fixed on, if really_probe()
> succeeds.
>
> ---
> bod
^ permalink raw reply
* Re: [PATCH v5 5/5] PCI: dwc: Add common send PME_Turn_Off message method
From: Manivannan Sadhasivam @ 2024-04-05 6:24 UTC (permalink / raw)
To: Frank Li
Cc: Bjorn Helgaas, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, imx, linux-pci, linux-kernel, devicetree
In-Reply-To: <20240319-pme_msg-v5-5-af9ffe57f432@nxp.com>
On Tue, Mar 19, 2024 at 12:07:15PM -0400, Frank Li wrote:
PCI: dwc: Add generic MSG TLP support for sending PME_Turn_Off during system suspend
> Reserve space at end of first IORESOURCE_MEM window as message TLP MMIO
> window. This space's size is 'region_align'.
>
> Set outbound ATU map memory write to send PCI message. So one MMIO write
> can trigger a PCI message, such as PME_Turn_Off.
>
> Add common dwc_pme_turn_off() function.
>
> Call dwc_pme_turn_off() to send out PME_Turn_Off message in general
> dw_pcie_suspend_noirq() if there are not platform callback pme_turn_off()
> exist.
>
How about:
"Instead of relying on the vendor specific implementations to send the
PME_Turn_Off message, let's introduce a generic way of sending the message using
the MSG TLP.
This is achieved by reserving a region for MSG TLP of size 'pci->region_align',
at the end of the first IORESOURCE_MEM window of the host bridge. And then
sending the PME_Turn_Off message during system suspend with the help of iATU.
It should be noted that this generic implementation is optional for the glue
drivers and can be overridden by a custom 'pme_turn_off' callback."
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 94 ++++++++++++++++++++++-
> drivers/pci/controller/dwc/pcie-designware.h | 3 +
> 2 files changed, 93 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 267687ab33cbc..d5723fce7a894 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -393,6 +393,31 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> return 0;
> }
>
> +static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct resource_entry *win;
> + struct resource *res;
> +
> + win = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> + if (win) {
> + res = devm_kzalloc(pci->dev, sizeof(*res), GFP_KERNEL);
> + if (!res)
> + return;
> +
> + /* Reserve last region_align block as message TLP space */
> + res->start = win->res->end - pci->region_align + 1;
> + res->end = win->res->end;
Don't you need to adjust the host bridge window size and end address?
> + res->name = "msg";
> + res->flags = win->res->flags | IORESOURCE_BUSY;
> +
Shouldn't this resource be added back to the host bridge?
> + if (!request_resource(win->res, res))
Why can't you use devm_ helper to manage the resource, since the lifetime of the
resource is till dw_pcie_host_deinit()?
> + pp->msg_res = res;
> + else
> + devm_kfree(pci->dev, res);
> + }
> +}
> +
> int dw_pcie_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -479,6 +504,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>
> dw_pcie_iatu_detect(pci);
>
> + /* Need call after dw_pcie_iatu_detect() before dw_pcie_setup_rc() */
It'd be better to add the reason also i.,e
/*
* Allocate the resource for MSG TLP before programming the iATU
* outbound window in dw_pcie_setup_rc(). Since the allocation depends
* on the value of 'region_align', this has to be done after
* dw_pcie_iatu_detect().
*/
> + if (pp->use_atu_msg)
Who is setting this flag?
> + dw_pcie_host_request_msg_tlp_res(pp);
> +
> ret = dw_pcie_edma_detect(pci);
> if (ret)
> goto err_free_msi;
> @@ -536,6 +565,11 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
>
> dw_pcie_edma_remove(pci);
>
> + if (pp->msg_res) {
> + release_resource(pp->msg_res);
> + devm_kfree(pci->dev, pp->msg_res);
> + }
> +
> if (pp->has_msi_ctrl)
> dw_pcie_free_msi(pp);
>
> @@ -697,6 +731,10 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> atu.pci_addr = entry->res->start - entry->offset;
> atu.size = resource_size(entry->res);
>
> + /* MSG TLB window resource reserve at one of end of IORESOURCE_MEM resource */
> + if (pp->msg_res && pp->msg_res->parent == entry->res)
> + atu.size -= resource_size(pp->msg_res);
If you adjust the bridge window above, then this won't be needed.
> +
> ret = dw_pcie_prog_outbound_atu(pci, &atu);
> if (ret) {
> dev_err(pci->dev, "Failed to set MEM range %pr\n",
> @@ -728,6 +766,8 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
> pci->num_ob_windows);
>
> + pp->msg_atu_index = i;
> +
> i = 0;
> resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {
> if (resource_type(entry->res) != IORESOURCE_MEM)
> @@ -833,11 +873,54 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_setup_rc);
>
> +/* Using message outbound ATU to send out PME_Turn_Off message for dwc PCIe controller */
> +static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
> +{
> + struct dw_pcie_ob_atu_cfg atu = { 0 };
> + void __iomem *m;
*mem
> + int ret;
> +
> + if (pci->num_ob_windows <= pci->pp.msg_atu_index)
> + return -EINVAL;
> +
> + if (!pci->pp.msg_res)
> + return -EINVAL;
> +
> + atu.code = PCIE_MSG_CODE_PME_TURN_OFF;
> + atu.routing = PCIE_MSG_TYPE_R_BC;
> + atu.type = PCIE_ATU_TYPE_MSG;
> + atu.size = resource_size(pci->pp.msg_res);
> + atu.index = pci->pp.msg_atu_index;
> +
> + if (!atu.size) {
> + dev_dbg(pci->dev,
> + "atu memory map windows is zero, please check 'msg' reg in dts\n");
You are already checking the existence of the 'pci->pp.msg_res' region above. So
shouldn't that be sufficient enough? Can the size be 0, if the region exist?
- Mani
> + return -ENOMEM;
> + }
> +
> + atu.cpu_addr = pci->pp.msg_res->start;
> +
> + ret = dw_pcie_prog_outbound_atu(pci, &atu);
> + if (ret)
> + return ret;
> +
> + m = ioremap(atu.cpu_addr, pci->region_align);
> + if (!m)
> + return -ENOMEM;
> +
> + /* A dummy write is converted to a Msg TLP */
> + writel(0, m);
> +
> + iounmap(m);
> +
> + return 0;
> +}
> +
> int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> {
> u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> u32 val;
> - int ret;
> + int ret = 0;
>
> /*
> * If L1SS is supported, then do not put the link into L2 as some
> @@ -849,10 +932,13 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> if (dw_pcie_get_ltssm(pci) <= DW_PCIE_LTSSM_DETECT_ACT)
> return 0;
>
> - if (!pci->pp.ops->pme_turn_off)
> - return 0;
> + if (pci->pp.ops->pme_turn_off)
> + pci->pp.ops->pme_turn_off(&pci->pp);
> + else
> + ret = dw_pcie_pme_turn_off(pci);
>
> - pci->pp.ops->pme_turn_off(&pci->pp);
> + if (ret)
> + return ret;
>
> ret = read_poll_timeout(dw_pcie_get_ltssm, val, val == DW_PCIE_LTSSM_L2_IDLE,
> PCIE_PME_TO_L2_TIMEOUT_US/10,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 703b50bc5e0f1..dca5de4c6e877 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -341,6 +341,9 @@ struct dw_pcie_rp {
> struct pci_host_bridge *bridge;
> raw_spinlock_t lock;
> DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
> + bool use_atu_msg;
> + int msg_atu_index;
> + struct resource *msg_res;
> };
>
> struct dw_pcie_ep_ops {
>
> --
> 2.34.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879
From: Matti Vaittinen @ 2024-04-05 6:01 UTC (permalink / raw)
To: Krzysztof Kozlowski, Andreas Kemnade
Cc: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-kernel
In-Reply-To: <8f37211a-57ed-48ab-8de8-cd5a0d4c6609@linaro.org>
On 4/4/24 15:04, Krzysztof Kozlowski wrote:
> On 04/04/2024 12:30, Andreas Kemnade wrote:
>> On Thu, 4 Apr 2024 08:59:54 +0200
>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>
>>> On 02/04/2024 21:35, Andreas Kemnade wrote:
>>>> As this chip was seen in several devices in the wild, add it.
>>>>
>>>> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
>>>> Suggested-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>>> ---
>>>> Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>> index 0b62f854bf6b..e4df09e8961c 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>> @@ -17,7 +17,9 @@ description: |
>>>>
>>>> properties:
>>>> compatible:
>>>> - const: rohm,bd71828
>>>> + enum:
>>>> + - rohm,bd71828
>>>> + - rohm,bd71879
>>>
>>> In your second commit you claim they are compatible, so why they are not
>>> marked as such?
>>>
>> so you mean allowing
>>
>> compatible = "rohm,bd71828"
>> and
>> compatible = "rohm,bd71879", "rohm,bd71828"
This makes me slightly nervous. It wouldn't be the first time when I've
been told "they are similar", and later the reality has turned out to be
"they are similar, except...". Furthermore, even if these devices seem
similar to software (which is what the comment in the MFD driver is
referring to), it does not mean these devices are 100% electrically
compatible so that they could be used as a "drop-in" replacement to each
others. I wouldn't guarantee that.
Furthermore, my current understanding is that the BD71828 was a model
that was used for a limited purposes. So, maybe creating an dt-entry like:
compatible = "rohm,bd71879", "rohm,bd71828"
might not prove to be too useful. (But I'm not 100% certain on this).
> Yes. If there are reasons against, please briefly mention them in commit
> msg.
I would like to understand the rationale for allowing:
compatible = "rohm,bd71879", "rohm,bd71828".
Is the intention to:
1) allow boards which tell the software that "the hardware may be
bd71828 or bd71879", or
2) to tell a binding reader that these ICs are likely to be usable as
replacements to each others?
(Or, is there some other rationale beyond these?)
If it's 1), then I see limited sense in doing so, while I expect that
not so many bd71828 variants will be seen out there - and at least not
in that many different products. If it's the 2), then I wouldn't say we
have the facts to do this.
And, as always, if there is 3), 4), ... - I am keen to learn :)
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH V2 RESEND 6/6] arm64: dts: qcom: sm8650: Add video and camera clock controllers
From: Jagadeesh Kona @ 2024-04-05 6:00 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vladimir Zapolskiy, linux-arm-msm, linux-clk, devicetree,
linux-kernel, Taniya Das, Satya Priya Kakitapalli, Ajit Pandey,
Imran Shaik
In-Reply-To: <CAA8EJpoW8MQQ3OPfOVYRJtgsn1JgKd5Ew7vqgWx3xWE-xJ=R-g@mail.gmail.com>
On 4/4/2024 9:35 PM, Dmitry Baryshkov wrote:
> On Thu, 4 Apr 2024 at 13:06, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>
>>
>>
>> On 4/4/2024 11:00 AM, Dmitry Baryshkov wrote:
>>> On Thu, 4 Apr 2024 at 08:13, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 4/3/2024 9:24 PM, Dmitry Baryshkov wrote:
>>>>> On Wed, 3 Apr 2024 at 10:16, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 3/25/2024 11:38 AM, Jagadeesh Kona wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 3/21/2024 6:43 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Thu, 21 Mar 2024 at 11:27, Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Add device nodes for video and camera clock controllers on Qualcomm
>>>>>>>>> SM8650 platform.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>>>> ---
>>>>>>>>> arch/arm64/boot/dts/qcom/sm8650.dtsi | 28 ++++++++++++++++++++++++++++
>>>>>>>>> 1 file changed, 28 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>>>> b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>>>> index 32c0a7b9aded..d862aa6be824 100644
>>>>>>>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>>>> @@ -4,6 +4,8 @@
>>>>>>>>> */
>>>>>>>>>
>>>>>>>>> #include <dt-bindings/clock/qcom,rpmh.h>
>>>>>>>>> +#include <dt-bindings/clock/qcom,sm8450-videocc.h>
>>>>>>>>> +#include <dt-bindings/clock/qcom,sm8650-camcc.h>
>>>>>>>>> #include <dt-bindings/clock/qcom,sm8650-dispcc.h>
>>>>>>>>> #include <dt-bindings/clock/qcom,sm8650-gcc.h>
>>>>>>>>> #include <dt-bindings/clock/qcom,sm8650-gpucc.h>
>>>>>>>>> @@ -3110,6 +3112,32 @@ opp-202000000 {
>>>>>>>>> };
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> + videocc: clock-controller@aaf0000 {
>>>>>>>>> + compatible = "qcom,sm8650-videocc";
>>>>>>>>> + reg = <0 0x0aaf0000 0 0x10000>;
>>>>>>>>> + clocks = <&bi_tcxo_div2>,
>>>>>>>>> + <&gcc GCC_VIDEO_AHB_CLK>;
>>>>>>>>> + power-domains = <&rpmhpd RPMHPD_MMCX>;
>>>>>>>>> + required-opps = <&rpmhpd_opp_low_svs>;
>>>>>>>>
>>>>>>>> The required-opps should no longer be necessary.
>>>>>>>>
>>>>>>>
>>>>>>> Sure, will check and remove this if not required.
>>>>>>
>>>>>>
>>>>>> I checked further on this and without required-opps, if there is no vote
>>>>>> on the power-domain & its peer from any other consumers, when runtime
>>>>>> get is called on device, it enables the power domain just at the minimum
>>>>>> non-zero level. But in some cases, the minimum non-zero level of
>>>>>> power-domain could be just retention and is not sufficient for clock
>>>>>> controller to operate, hence required-opps property is needed to specify
>>>>>> the minimum level required on power-domain for this clock controller.
>>>>>
>>>>> In which cases? If it ends up with the retention vote, it is a bug
>>>>> which must be fixed.
>>>>>
>>>>
>>>> The minimum non-zero level(configured from bootloaders) of MMCX is
>>>> retention on few chipsets but it can vary across the chipsets. Hence to
>>>> be on safer side from our end, it is good to have required-opps in DT to
>>>> specify the minimum level required for this clock controller.
>>>
>>> We are discussing sm8650, not some abstract chipset. Does it list
>>> retention or low_svs as a minimal level for MMCX?
>>>
>>
>> Actually, the minimum level for MMCX is external to the clock
>> controllers.
>
> Yes, it comes from cmd-db
>
>> But the clock controller requires MMCX to be atleast at
>> lowsvs for it to be functional.
>
> Correct
>
>> Hence we need to keep required-opps to
>> ensure the same without relying on the actual minimum level for MMCX.
>
> And this is not correct. There is no need for the DT to be redundant.
> I plan to send patches removing the existing required-opps when they
> are not required.
>
I agree this is not required if cmd-db minimum level is already at
lowsvs. But since MMCX running at lowsvs is a mandatory requirement for
clock controller to operate, I believe it is good to have required-opps
to ensure we meet this requirement in all cases, rather than relying on
the cmd-db minimum level which we have no control over.
Thanks,
Jagadeesh
^ permalink raw reply
* Re: [PATCH v2 1/2] ASoC: dt-bindings: qcom,sm8250: Add QCM6490 snd QCS6490 sound card
From: Mohammad Rafi Shaik @ 2024-04-05 5:55 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Srinivas Kandagatla, Banajit Goswami, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela,
Takashi Iwai, linux-arm-msm, alsa-devel, linux-sound, devicetree,
linux-kernel, quic_rohkumar
In-Reply-To: <CAA8EJpqWaYhzPKgTREtJnfdNZ4oSFZaRFM7Jhg+qd3AqadGOkA@mail.gmail.com>
On 4/4/2024 2:23 PM, Dmitry Baryshkov wrote:
> On Thu, 4 Apr 2024 at 11:48, Mohammad Rafi Shaik <quic_mohs@quicinc.com> wrote:
>>
>> Document the bindings for the Qualcomm QCM6490 IDP and QCS6490 RB3Gen2
>> soc platforms sound card.
>>
>> The bindings are the same as for other newer Qualcomm ADSP sound cards,
>> thus keep them in existing qcom,sm8250.yaml file, even though Linux driver
>> is separate.
>>
>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>> ---
>> Documentation/devicetree/bindings/sound/qcom,sm8250.yaml | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>> index 2ab6871e89e5..ff1a27f26bc2 100644
>> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>> @@ -29,6 +29,8 @@ properties:
>> - enum:
>> - qcom,apq8016-sbc-sndcard
>> - qcom,msm8916-qdsp6-sndcard
>> + - qcom,qcm6490-sndcard
>> + - qcom,qcs6490-rb3gen2-sndcard
>
> My 2c: you are adding one soundcard for the SoC family (qcm6490) and
> another one for the particular board kind (qcs6490-rb3gen2). That
> doesn't seem logical.
The qcm6490-sndcard compatible for enabling soundcard on
qcm6490 IDP boards.
Will change compatible name as qcom,qcm6490-idp-sndcard.
Thanks,
Rafi.
>
>> - qcom,qrb5165-rb5-sndcard
>> - qcom,sc7180-qdsp6-sndcard
>> - qcom,sc8280xp-sndcard
>> --
>> 2.25.1
>>
>>
>
>
^ permalink raw reply
* Re: [RFC PATCH 2/2] PCI: Add Qualcomm PCIe ECAM root complex driver
From: Manivannan Sadhasivam @ 2024-04-05 5:30 UTC (permalink / raw)
To: Mayank Rana
Cc: linux-pci, lpieralisi, kw, robh, bhelgaas, andersson,
krzysztof.kozlowski+dt, conor+dt, devicetree, linux-arm-msm,
quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
In-Reply-To: <1712257884-23841-3-git-send-email-quic_mrana@quicinc.com>
On Thu, Apr 04, 2024 at 12:11:24PM -0700, Mayank Rana wrote:
> On some of Qualcomm platform, firmware configures PCIe controller into
> ECAM mode allowing static memory allocation for configuration space of
> supported bus range. Firmware also takes care of bringing up PCIe PHY
> and performing required operation to bring PCIe link into D0. Firmware
> also manages system resources (e.g. clocks/regulators/resets/ bus voting).
> Hence add Qualcomm PCIe ECAM root complex driver which enumerates PCIe
> root complex and connected PCIe devices. Firmware won't be enumerating
> or powering up PCIe root complex until this driver invokes power domain
> based notification to bring PCIe link into D0/D3cold mode.
>
Is this an in-house PCIe IP of Qualcomm or the same DWC IP that is used in other
SoCs?
- Mani
> This driver also support MSI functionality using PCIe controller based
> MSI controller as GIC ITS based MSI functionality is not available on
> some of platform.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> drivers/pci/controller/Kconfig | 12 +
> drivers/pci/controller/Makefile | 1 +
> drivers/pci/controller/pcie-qcom-ecam.c | 575 ++++++++++++++++++++++++++++++++
> 3 files changed, 588 insertions(+)
> create mode 100644 drivers/pci/controller/pcie-qcom-ecam.c
>
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index e534c02..abbd9f2 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -353,6 +353,18 @@ config PCIE_XILINX_CPM
> Say 'Y' here if you want kernel support for the
> Xilinx Versal CPM host bridge.
>
> +config PCIE_QCOM_ECAM
> + tristate "QCOM PCIe ECAM host controller"
> + depends on ARCH_QCOM && PCI
> + depends on OF
> + select PCI_MSI
> + select PCI_HOST_COMMON
> + select IRQ_DOMAIN
> + help
> + Say 'Y' here if you want to use ECAM shift mode compatible Qualcomm
> + PCIe root host controller. The controller is programmed using firmware
> + to support ECAM compatible memory address space.
> +
> source "drivers/pci/controller/cadence/Kconfig"
> source "drivers/pci/controller/dwc/Kconfig"
> source "drivers/pci/controller/mobiveil/Kconfig"
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index f2b19e6..2f1ee1e 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
> obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
> obj-$(CONFIG_PCIE_APPLE) += pcie-apple.o
> obj-$(CONFIG_PCIE_MT7621) += pcie-mt7621.o
> +obj-$(CONFIG_PCIE_QCOM_ECAM) += pcie-qcom-ecam.o
>
> # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
> obj-y += dwc/
> diff --git a/drivers/pci/controller/pcie-qcom-ecam.c b/drivers/pci/controller/pcie-qcom-ecam.c
> new file mode 100644
> index 00000000..5b4c68b
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-qcom-ecam.c
> @@ -0,0 +1,575 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Qualcomm PCIe ECAM root host controller driver
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/irq.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/msi.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_pci.h>
> +#include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#define PCIE_MSI_CTRL_BASE (0x820)
> +#define PCIE_MSI_CTRL_SIZE (0x68)
> +#define PCIE_MSI_CTRL_ADDR_OFFS (0x0)
> +#define PCIE_MSI_CTRL_UPPER_ADDR_OFFS (0x4)
> +#define PCIE_MSI_CTRL_INT_N_EN_OFFS(n) (0x8 + 0xc * (n))
> +#define PCIE_MSI_CTRL_INT_N_MASK_OFFS(n) (0xc + 0xc * (n))
> +#define PCIE_MSI_CTRL_INT_N_STATUS_OFFS(n) (0x10 + 0xc * (n))
> +
> +#define MSI_DB_ADDR 0xa0000000
> +#define MSI_IRQ_PER_GRP (32)
> +
> +/**
> + * struct qcom_msi_irq - MSI IRQ information
> + * @client: pointer to MSI client struct
> + * @grp: group the irq belongs to
> + * @grp_index: index in group
> + * @hwirq: hwirq number
> + * @virq: virq number
> + * @pos: position in MSI bitmap
> + */
> +struct qcom_msi_irq {
> + struct qcom_msi_client *client;
> + struct qcom_msi_grp *grp;
> + unsigned int grp_index;
> + unsigned int hwirq;
> + unsigned int virq;
> + u32 pos;
> +};
> +
> +/**
> + * struct qcom_msi_grp - MSI group information
> + * @int_en_reg: memory-mapped interrupt enable register address
> + * @int_mask_reg: memory-mapped interrupt mask register address
> + * @int_status_reg: memory-mapped interrupt status register address
> + * @mask: tracks masked/unmasked MSI
> + * @irqs: structure to MSI IRQ information
> + */
> +struct qcom_msi_grp {
> + void __iomem *int_en_reg;
> + void __iomem *int_mask_reg;
> + void __iomem *int_status_reg;
> + u32 mask;
> + struct qcom_msi_irq irqs[MSI_IRQ_PER_GRP];
> +};
> +
> +/**
> + * struct qcom_msi - PCIe controller based MSI controller information
> + * @clients: list for tracking clients
> + * @dev: platform device node
> + * @nr_hwirqs: total number of hardware IRQs
> + * @nr_virqs: total number of virqs
> + * @nr_grps: total number of groups
> + * @grps: pointer to all groups information
> + * @bitmap: tracks used/unused MSI
> + * @mutex: for modifying MSI client list and bitmap
> + * @inner_domain: parent domain; gen irq related
> + * @msi_domain: child domain; pcie related
> + * @msi_db_addr: MSI doorbell address
> + * @cfg_lock: lock for configuring MSI controller registers
> + * @pcie_msi_cfg: memory-mapped MSI controller register space
> + */
> +struct qcom_msi {
> + struct list_head clients;
> + struct device *dev;
> + int nr_hwirqs;
> + int nr_virqs;
> + int nr_grps;
> + struct qcom_msi_grp *grps;
> + unsigned long *bitmap;
> + struct mutex mutex;
> + struct irq_domain *inner_domain;
> + struct irq_domain *msi_domain;
> + phys_addr_t msi_db_addr;
> + spinlock_t cfg_lock;
> + void __iomem *pcie_msi_cfg;
> +};
> +
> +/**
> + * struct qcom_msi_client - structure for each client of MSI controller
> + * @node: list to track number of MSI clients
> + * @msi: client specific MSI controller based resource pointer
> + * @dev: client's dev of pci_dev
> + * @nr_irqs: number of irqs allocated for client
> + * @msi_addr: MSI doorbell address
> + */
> +struct qcom_msi_client {
> + struct list_head node;
> + struct qcom_msi *msi;
> + struct device *dev;
> + unsigned int nr_irqs;
> + phys_addr_t msi_addr;
> +};
> +
> +static void qcom_msi_handler(struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct qcom_msi_grp *msi_grp;
> + u32 status;
> + int i;
> +
> + chained_irq_enter(chip, desc);
> +
> + msi_grp = irq_desc_get_handler_data(desc);
> + status = readl_relaxed(msi_grp->int_status_reg);
> + status ^= (msi_grp->mask & status);
> + writel(status, msi_grp->int_status_reg);
> +
> + for (i = 0; status; i++, status >>= 1)
> + if (status & 0x1)
> + generic_handle_irq(msi_grp->irqs[i].virq);
> +
> + chained_irq_exit(chip, desc);
> +}
> +
> +static void qcom_msi_mask_irq(struct irq_data *data)
> +{
> + struct irq_data *parent_data;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi *msi;
> + unsigned long flags;
> +
> + parent_data = data->parent_data;
> + if (!parent_data)
> + return;
> +
> + msi_irq = irq_data_get_irq_chip_data(parent_data);
> + msi = msi_irq->client->msi;
> + msi_grp = msi_irq->grp;
> +
> + spin_lock_irqsave(&msi->cfg_lock, flags);
> + pci_msi_mask_irq(data);
> + msi_grp->mask |= BIT(msi_irq->grp_index);
> + writel(msi_grp->mask, msi_grp->int_mask_reg);
> + spin_unlock_irqrestore(&msi->cfg_lock, flags);
> +}
> +
> +static void qcom_msi_unmask_irq(struct irq_data *data)
> +{
> + struct irq_data *parent_data;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi *msi;
> + unsigned long flags;
> +
> + parent_data = data->parent_data;
> + if (!parent_data)
> + return;
> +
> + msi_irq = irq_data_get_irq_chip_data(parent_data);
> + msi = msi_irq->client->msi;
> + msi_grp = msi_irq->grp;
> +
> + spin_lock_irqsave(&msi->cfg_lock, flags);
> + msi_grp->mask &= ~BIT(msi_irq->grp_index);
> + writel(msi_grp->mask, msi_grp->int_mask_reg);
> + pci_msi_unmask_irq(data);
> + spin_unlock_irqrestore(&msi->cfg_lock, flags);
> +}
> +
> +static struct irq_chip qcom_msi_irq_chip = {
> + .name = "qcom_pci_msi",
> + .irq_enable = qcom_msi_unmask_irq,
> + .irq_disable = qcom_msi_mask_irq,
> + .irq_mask = qcom_msi_mask_irq,
> + .irq_unmask = qcom_msi_unmask_irq,
> +};
> +
> +static int qcom_msi_domain_prepare(struct irq_domain *domain, struct device *dev,
> + int nvec, msi_alloc_info_t *arg)
> +{
> + struct qcom_msi *msi = domain->parent->host_data;
> + struct qcom_msi_client *client;
> +
> + client = kzalloc(sizeof(*client), GFP_KERNEL);
> + if (!client)
> + return -ENOMEM;
> +
> + client->msi = msi;
> + client->dev = dev;
> + client->msi_addr = msi->msi_db_addr;
> + mutex_lock(&msi->mutex);
> + list_add_tail(&client->node, &msi->clients);
> + mutex_unlock(&msi->mutex);
> +
> + /* zero out struct for pcie msi framework */
> + memset(arg, 0, sizeof(*arg));
> + return 0;
> +}
> +
> +static struct msi_domain_ops qcom_msi_domain_ops = {
> + .msi_prepare = qcom_msi_domain_prepare,
> +};
> +
> +static struct msi_domain_info qcom_msi_domain_info = {
> + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> + MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX),
> + .ops = &qcom_msi_domain_ops,
> + .chip = &qcom_msi_irq_chip,
> +};
> +
> +static int qcom_msi_irq_set_affinity(struct irq_data *data,
> + const struct cpumask *mask, bool force)
> +{
> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
> + int ret = 0;
> +
> + if (!parent_data)
> + return -ENODEV;
> +
> + /* set affinity for MSI HW IRQ */
> + if (parent_data->chip->irq_set_affinity)
> + ret = parent_data->chip->irq_set_affinity(parent_data, mask, force);
> +
> + return ret;
> +}
> +
> +static void qcom_msi_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> +{
> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
> + struct qcom_msi_irq *msi_irq = irq_data_get_irq_chip_data(data);
> + struct qcom_msi_client *client = msi_irq->client;
> +
> + if (!parent_data)
> + return;
> +
> + msg->address_lo = lower_32_bits(client->msi_addr);
> + msg->address_hi = upper_32_bits(client->msi_addr);
> + msg->data = msi_irq->pos;
> +}
> +
> +static struct irq_chip qcom_msi_bottom_irq_chip = {
> + .name = "qcom_msi",
> + .irq_set_affinity = qcom_msi_irq_set_affinity,
> + .irq_compose_msi_msg = qcom_msi_irq_compose_msi_msg,
> +};
> +
> +static int qcom_msi_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs, void *args)
> +{
> + struct device *dev = ((msi_alloc_info_t *)args)->desc->dev;
> + struct qcom_msi_client *tmp, *client = NULL;
> + struct qcom_msi *msi = domain->host_data;
> + int i, ret = 0;
> + int pos;
> +
> + mutex_lock(&msi->mutex);
> + list_for_each_entry(tmp, &msi->clients, node) {
> + if (tmp->dev == dev) {
> + client = tmp;
> + break;
> + }
> + }
> +
> + if (!client) {
> + dev_err(msi->dev, "failed to find MSI client dev\n");
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + pos = bitmap_find_next_zero_area(msi->bitmap, msi->nr_virqs, 0,
> + nr_irqs, nr_irqs - 1);
> + if (pos > msi->nr_virqs) {
> + ret = -ENOSPC;
> + goto out;
> + }
> +
> + bitmap_set(msi->bitmap, pos, nr_irqs);
> + for (i = 0; i < nr_irqs; i++) {
> + u32 grp = pos / MSI_IRQ_PER_GRP;
> + u32 index = pos % MSI_IRQ_PER_GRP;
> + struct qcom_msi_irq *msi_irq = &msi->grps[grp].irqs[index];
> +
> + msi_irq->virq = virq + i;
> + msi_irq->client = client;
> + irq_domain_set_info(domain, msi_irq->virq,
> + msi_irq->hwirq,
> + &qcom_msi_bottom_irq_chip, msi_irq,
> + handle_simple_irq, NULL, NULL);
> + client->nr_irqs++;
> + pos++;
> + }
> +out:
> + mutex_unlock(&msi->mutex);
> + return ret;
> +}
> +
> +static void qcom_msi_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs)
> +{
> + struct irq_data *data = irq_domain_get_irq_data(domain, virq);
> + struct qcom_msi_client *client;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi *msi;
> +
> + if (!data)
> + return;
> +
> + msi_irq = irq_data_get_irq_chip_data(data);
> + client = msi_irq->client;
> + msi = client->msi;
> +
> + mutex_lock(&msi->mutex);
> + bitmap_clear(msi->bitmap, msi_irq->pos, nr_irqs);
> +
> + client->nr_irqs -= nr_irqs;
> + if (!client->nr_irqs) {
> + list_del(&client->node);
> + kfree(client);
> + }
> + mutex_unlock(&msi->mutex);
> +
> + irq_domain_free_irqs_parent(domain, virq, nr_irqs);
> +}
> +
> +static const struct irq_domain_ops msi_domain_ops = {
> + .alloc = qcom_msi_irq_domain_alloc,
> + .free = qcom_msi_irq_domain_free,
> +};
> +
> +static int qcom_msi_alloc_domains(struct qcom_msi *msi)
> +{
> + msi->inner_domain = irq_domain_add_linear(NULL, msi->nr_virqs,
> + &msi_domain_ops, msi);
> + if (!msi->inner_domain) {
> + dev_err(msi->dev, "failed to create IRQ inner domain\n");
> + return -ENOMEM;
> + }
> +
> + msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->dev->of_node),
> + &qcom_msi_domain_info, msi->inner_domain);
> + if (!msi->msi_domain) {
> + dev_err(msi->dev, "failed to create MSI domain\n");
> + irq_domain_remove(msi->inner_domain);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_msi_irq_setup(struct qcom_msi *msi)
> +{
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi_irq *msi_irq;
> + int i, index, ret;
> + unsigned int irq;
> +
> + /* setup each MSI group. nr_hwirqs == nr_grps */
> + for (i = 0; i < msi->nr_hwirqs; i++) {
> + irq = irq_of_parse_and_map(msi->dev->of_node, i);
> + if (!irq) {
> + dev_err(msi->dev,
> + "MSI: failed to parse/map interrupt\n");
> + ret = -ENODEV;
> + goto free_irqs;
> + }
> +
> + msi_grp = &msi->grps[i];
> + msi_grp->int_en_reg = msi->pcie_msi_cfg +
> + PCIE_MSI_CTRL_INT_N_EN_OFFS(i);
> + msi_grp->int_mask_reg = msi->pcie_msi_cfg +
> + PCIE_MSI_CTRL_INT_N_MASK_OFFS(i);
> + msi_grp->int_status_reg = msi->pcie_msi_cfg +
> + PCIE_MSI_CTRL_INT_N_STATUS_OFFS(i);
> +
> + for (index = 0; index < MSI_IRQ_PER_GRP; index++) {
> + msi_irq = &msi_grp->irqs[index];
> +
> + msi_irq->grp = msi_grp;
> + msi_irq->grp_index = index;
> + msi_irq->pos = (i * MSI_IRQ_PER_GRP) + index;
> + msi_irq->hwirq = irq;
> + }
> +
> + irq_set_chained_handler_and_data(irq, qcom_msi_handler, msi_grp);
> + }
> +
> + return 0;
> +
> +free_irqs:
> + for (--i; i >= 0; i--) {
> + irq = msi->grps[i].irqs[0].hwirq;
> +
> + irq_set_chained_handler_and_data(irq, NULL, NULL);
> + irq_dispose_mapping(irq);
> + }
> +
> + return ret;
> +}
> +
> +static void qcom_msi_config(struct irq_domain *domain)
> +{
> + struct qcom_msi *msi;
> + int i;
> +
> + msi = domain->parent->host_data;
> +
> + /* program termination address */
> + writel(msi->msi_db_addr, msi->pcie_msi_cfg + PCIE_MSI_CTRL_ADDR_OFFS);
> + writel(0, msi->pcie_msi_cfg + PCIE_MSI_CTRL_UPPER_ADDR_OFFS);
> +
> + /* restore mask and enable all interrupts for each group */
> + for (i = 0; i < msi->nr_grps; i++) {
> + struct qcom_msi_grp *msi_grp = &msi->grps[i];
> +
> + writel(msi_grp->mask, msi_grp->int_mask_reg);
> + writel(~0, msi_grp->int_en_reg);
> + }
> +}
> +
> +static void qcom_msi_deinit(struct qcom_msi *msi)
> +{
> + irq_domain_remove(msi->msi_domain);
> + irq_domain_remove(msi->inner_domain);
> +}
> +
> +static struct qcom_msi *qcom_msi_init(struct device *dev)
> +{
> + struct qcom_msi *msi;
> + u64 addr;
> + int ret;
> +
> + msi = devm_kzalloc(dev, sizeof(*msi), GFP_KERNEL);
> + if (!msi)
> + return ERR_PTR(-ENOMEM);
> +
> + msi->dev = dev;
> + mutex_init(&msi->mutex);
> + spin_lock_init(&msi->cfg_lock);
> + INIT_LIST_HEAD(&msi->clients);
> +
> + msi->msi_db_addr = MSI_DB_ADDR;
> + msi->nr_hwirqs = of_irq_count(dev->of_node);
> + if (!msi->nr_hwirqs) {
> + dev_err(msi->dev, "no hwirqs found\n");
> + return ERR_PTR(-ENODEV);
> + }
> +
> + if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
> + dev_err(msi->dev, "failed to get reg address\n");
> + return ERR_PTR(-ENODEV);
> + }
> +
> + dev_dbg(msi->dev, "hwirq:%d pcie_msi_cfg:%llx\n", msi->nr_hwirqs, addr);
> + msi->pcie_msi_cfg = devm_ioremap(dev, addr + PCIE_MSI_CTRL_BASE, PCIE_MSI_CTRL_SIZE);
> + if (!msi->pcie_msi_cfg)
> + return ERR_PTR(-ENOMEM);
> +
> + msi->nr_virqs = msi->nr_hwirqs * MSI_IRQ_PER_GRP;
> + msi->nr_grps = msi->nr_hwirqs;
> + msi->grps = devm_kcalloc(dev, msi->nr_grps, sizeof(*msi->grps), GFP_KERNEL);
> + if (!msi->grps)
> + return ERR_PTR(-ENOMEM);
> +
> + msi->bitmap = devm_kcalloc(dev, BITS_TO_LONGS(msi->nr_virqs),
> + sizeof(*msi->bitmap), GFP_KERNEL);
> + if (!msi->bitmap)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = qcom_msi_alloc_domains(msi);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + ret = qcom_msi_irq_setup(msi);
> + if (ret) {
> + qcom_msi_deinit(msi);
> + return ERR_PTR(ret);
> + }
> +
> + qcom_msi_config(msi->msi_domain);
> + return msi;
> +}
> +
> +static int qcom_pcie_ecam_suspend_noirq(struct device *dev)
> +{
> + return pm_runtime_put_sync(dev);
> +}
> +
> +static int qcom_pcie_ecam_resume_noirq(struct device *dev)
> +{
> + return pm_runtime_get_sync(dev);
> +}
> +
> +static int qcom_pcie_ecam_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct qcom_msi *msi;
> + int ret;
> +
> + ret = devm_pm_runtime_enable(dev);
> + if (ret)
> + return ret;
> +
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret < 0) {
> + dev_err(dev, "fail to enable pcie controller: %d\n", ret);
> + return ret;
> + }
> +
> + msi = qcom_msi_init(dev);
> + if (IS_ERR(msi)) {
> + pm_runtime_put_sync(dev);
> + return PTR_ERR(msi);
> + }
> +
> + ret = pci_host_common_probe(pdev);
> + if (ret) {
> + dev_err(dev, "pci_host_common_probe() failed:%d\n", ret);
> + qcom_msi_deinit(msi);
> + pm_runtime_put_sync(dev);
> + }
> +
> + return ret;
> +}
> +
> +static const struct dev_pm_ops qcom_pcie_ecam_pm_ops = {
> + NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_ecam_suspend_noirq,
> + qcom_pcie_ecam_resume_noirq)
> +};
> +
> +static const struct pci_ecam_ops qcom_pcie_ecam_ops = {
> + .pci_ops = {
> + .map_bus = pci_ecam_map_bus,
> + .read = pci_generic_config_read,
> + .write = pci_generic_config_write,
> + }
> +};
> +
> +static const struct of_device_id qcom_pcie_ecam_of_match[] = {
> + {
> + .compatible = "qcom,pcie-ecam-rc",
> + .data = &qcom_pcie_ecam_ops,
> + },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, qcom_pcie_ecam_of_match);
> +
> +static struct platform_driver qcom_pcie_ecam_driver = {
> + .probe = qcom_pcie_ecam_probe,
> + .driver = {
> + .name = "qcom-pcie-ecam-rc",
> + .suppress_bind_attrs = true,
> + .of_match_table = qcom_pcie_ecam_of_match,
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + .pm = &qcom_pcie_ecam_pm_ops,
> + },
> +};
> +module_platform_driver(qcom_pcie_ecam_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm PCIe ECAM root complex driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH] dt-bindings: mfd: syscon: Add ti,am62p-cpsw-mac-efuse compatible
From: Siddharth Vadapalli @ 2024-04-05 5:21 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Siddharth Vadapalli, lee, robh, krzk+dt, conor+dt, devicetree,
linux-kernel, linux-arm-kernel, srk
In-Reply-To: <a895ddc8-5c18-49d7-86c4-b995bb946914@ti.com>
On Thu, Apr 04, 2024 at 02:02:21PM +0530, Siddharth Vadapalli wrote:
> On Wed, Apr 03, 2024 at 12:18:10PM +0530, Siddharth Vadapalli wrote:
> > On Wed, Apr 03, 2024 at 08:40:19AM +0200, Krzysztof Kozlowski wrote:
> > > On 03/04/2024 08:32, Siddharth Vadapalli wrote:
> > > > On Wed, Apr 03, 2024 at 08:27:06AM +0200, Krzysztof Kozlowski wrote:
> > > >> On 03/04/2024 07:35, Siddharth Vadapalli wrote:
> > > >>> On Tue, Apr 02, 2024 at 08:06:27PM +0200, Krzysztof Kozlowski wrote:
> > > >>>> On 02/04/2024 14:30, Siddharth Vadapalli wrote:
> > > >>>>> On Tue, Apr 02, 2024 at 02:08:32PM +0200, Krzysztof Kozlowski wrote:
> > > >>>>>> On 02/04/2024 12:57, Siddharth Vadapalli wrote:
> > > >>>>>>> The CTRLMMR_MAC_IDx registers within the CTRL_MMR space of TI's AM62p SoC
> > > >>>>>>> contain the MAC Address programmed in the eFuse. Add compatible for
> > > >>>>>>> allowing the CPSW driver to obtain a regmap for the CTRLMMR_MAC_IDx
> > > >>>>>>> registers within the System Controller device-tree node. The default MAC
> > > >>>>>>> Address for the interface corresponding to the first MAC port will be set
> > > >>>>>>> to the value programmed in the eFuse.
> > > >>>>>>>
> > > >>>>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> > > >>>>>>> ---
> > > >>>>>>>
> > > >>>>>>> This patch is based on linux-next tagged next-20240402.
> > > >>>>>>
> > > >>>>>> Where is the DTS using it?
> > > >>>>>
> > > >>>>> The current implementation in the device-tree for older TI K3 SoCs is as
> > > >>>>> follows:
> > > >>>>>
> > > >>>>> cpsw_port1: port@1 {
> > > >>>>> reg = <1>;
> > > >>>>> ti,mac-only;
> > > >>>>> label = "port1";
> > > >>>>> phys = <&phy_gmii_sel 1>;
> > > >>>>> mac-address = [00 00 00 00 00 00];
> > > >>>>> ti,syscon-efuse = <&wkup_conf 0x200>;
> > > >>>>> };
> > > >>>>>
> > > >>>>> The "ti,syscon-efuse" property passes the reference to the System
> > > >>>>> Controller node as well as the offset to the CTRLMMR_MAC_IDx registers
> > > >>>>> within the CTRL_MMR space.
> > > >>>>
> > > >>>> Please reference upstream DTS or lore link to patch under review.
> > > >>>
> > > >>> An example of the existing implementation in the device-tree for AM64x
> > > >>> is:
> > > >>> https://github.com/torvalds/linux/blob/d4e8c8ad5d14ad51ed8813442d81c43019fd669d/arch/arm64/boot/dts/ti/k3-am64-main.dtsi#L697
> > > >>> It uses:
> > > >>> ti,syscon-efuse = <&main_conf 0x200>;
> > > >>>
> > > >>> and "main_conf" node is defined at:
> > > >>> https://github.com/torvalds/linux/blob/d4e8c8ad5d14ad51ed8813442d81c43019fd669d/arch/arm64/boot/dts/ti/k3-am64-main.dtsi#L40
> > > >>
> > > >> It is quite different than your bindings, so your bindings are incorrect.
> > > >
> > > > Sorry I didn't understand what you mean. The references I have provided
> > > > are for existing DTS where "main_conf"/"wkup_conf" (System Controller
> > > > nodes) have the compatible "syscon", unlike in AM62p at:
> > > > https://github.com/torvalds/linux/blob/20f8173afaac90dd9dca11be4aa602a47776077f/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi#L8
> > > > which has the "simple-bus" compatible for the "wkup_conf" node.
> > > >
> > > > Also, shouldn't the device-tree bindings patches be posted first and get
> > > > merged before I post the device-tree patches that utilize the
> > > > compatible/properties that have been added in the bindings? That is the
> > > > reason why I had shared the "DIFF" for the DTS changes that I will be
> > > > posting once this patch for the new compatible is accepted.
> > > >
> > >
> > > That's not the process. I will be NAKing bindings which do not have any
> > > users, because I do not trust you test them.
> > >
> > > The process is almost always:
> > > 1. Send bindings,
> > > 2. Send driver changes (if applicable) in the same patchset.
> > > 3. Send DTS, usually in separate patches and provide lore link to the
> > > bindings in the changelog or cover letter.
> >
> > Thank you for clarifying. I will post the DTS patches corresponding to
> > this patch and reference this patch in the DTS patch series.
>
> I have posted the DTS patch at:
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240404081845.622707-1-s-vadapalli@ti.com/
> indicating the dependency on this bindings patch.
Hello Krzysztof,
Do I have to post a v2 for this patch? You had Acked it initially but I
am not sure if the discussion so far will make it unclear to readers
regarding the acceptance of this patch. Thank you for Acking the v3 DTS
patch at:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240404124614.891416-1-s-vadapalli@ti.com/
Since the v3 DTS patch mentions this bindings patch as a dependency, I
wanted to be sure whether I have to post a v2 for this or that won't be
required.
Regards,
Siddharth.
^ permalink raw reply
* Re: [PATCH v19 2/9] usb: dwc3: core: Access XHCI address space temporarily to read port info
From: Greg Kroah-Hartman @ 2024-04-05 4:43 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Krzysztof Kozlowski, Johan Hovold, Krishna Kurapati,
Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Conor Dooley, Thinh Nguyen, Felipe Balbi,
devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Johan Hovold
In-Reply-To: <Zg9THGBRuppfw4y+@hu-bjorande-lv.qualcomm.com>
On Thu, Apr 04, 2024 at 06:25:48PM -0700, Bjorn Andersson wrote:
> On Thu, Apr 04, 2024 at 02:58:29PM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Apr 04, 2024 at 10:07:27AM +0200, Krzysztof Kozlowski wrote:
> > > On 04/04/2024 09:21, Johan Hovold wrote:
> > > > On Thu, Apr 04, 2024 at 10:42:22AM +0530, Krishna Kurapati wrote:
> > > >
> > > >> +static int dwc3_get_num_ports(struct dwc3 *dwc)
> > > >> +{
> > > >> + void __iomem *base;
> > > >> + u8 major_revision;
> > > >> + u32 offset;
> > > >> + u32 val;
> > > >> +
> > > >> + /*
> > > >> + * Remap xHCI address space to access XHCI ext cap regs since it is
> > > >> + * needed to get information on number of ports present.
> > > >> + */
> > > >> + base = ioremap(dwc->xhci_resources[0].start,
> > > >> + resource_size(&dwc->xhci_resources[0]));
> > > >> + if (!base)
> > > >> + return PTR_ERR(base);
> > > >
> > > > This is obviously still broken. You need to update the return value as
> > > > well.
> > > >
> > > > Fix in v20.
> > >
> > > If one patchset reaches 20 versions, I think it is time to stop and
> > > really think from the beginning, why issues keep appearing and reviewers
> > > are still not happy.
> > >
> > > Maybe you did not perform extensive internal review, which you are
> > > encouraged to by your own internal policies, AFAIR. Before posting next
> > > version, please really get some internal review first.
> >
> > Also get those internal reviewers to sign-off on the commits and have
> > that show up when you post them next. That way they are also
> > responsible for this patchset, it's not fair that they are making you do
> > all the work here :)
> >
>
> I like this idea and I'm open to us changing our way of handling this.
>
> But unless such internal review brings significant input to the
> development I'd say a s-o-b would take the credit from the actual
> author.
It does not do that at all. It provides proof that someone else has
reviewed it and agrees with it. Think of it as a "path of blame" for
when things go bad (i.e. there is a bug in the submission.) Putting
your name on it makes you take responsibility if that happens.
> We've discussed a few times about carrying Reviewed-by et al from the
> internal reviews, but as maintainer I dislike this because I'd have no
> way to know if a r-b on vN means the patch was reviewed, or if it was
> just "accidentally" carried from v(N-1).
> But it might be worth this risk, is this something you think would be
> appropriate?
For some companies we REQUIRE this to happen due to low-quality
submissions and waste of reviewer's time. Based on the track record
here for some of these patchsets, hopefully it doesn't become a
requirement for this company as well :)
thanks,
greg k-h
^ permalink raw reply
* [PATCH v5 4/4] ARM: configs: at91: Enable LVDS serializer support
From: Dharma Balasubiramani @ 2024-04-05 4:35 UTC (permalink / raw)
To: andrzej . hajda @ intel . com, neil . armstrong @ linaro . org,
rfoss @ kernel . org, Laurent . pinchart @ ideasonboard . com,
jonas @ kwiboo . se, jernej . skrabec @ gmail . com,
maarten . lankhorst @ linux . intel . com, mripard @ kernel . org,
tzimmermann @ suse . de, airlied @ gmail . com,
daniel @ ffwll . ch, robh+dt @ kernel . org,
krzysztof . kozlowski+dt @ linaro . org, conor+dt @ kernel . org,
linux @ armlinux . org . uk, Nicolas . Ferre @ microchip . com,
alexandre . belloni @ bootlin . com,
claudiu . beznea @ tuxon . dev, Manikandan . M @ microchip . com,
Dharma . B @ microchip . com, arnd @ arndb . de,
geert+renesas @ glider . be, Jason @ zx2c4 . com,
mpe @ ellerman . id . au, gerg @ linux-m68k . org,
rdunlap @ infradead . org, vbabka @ suse . cz,
dri-devel @ lists . freedesktop . org,
devicetree @ vger . kernel . org,
linux-kernel @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
Hari . PrasathGE @ microchip . com, akpm @ linux-foundation . org,
deller @ gmx . de
Cc: Dharma Balasubiramani, Hari Prasath Gujulan Elango, Nicolas Ferre
In-Reply-To: <20240405043536.274220-1-dharma.b@microchip.com>
Enable LVDS serializer support for display pipeline.
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Acked-by: Hari Prasath Gujulan Elango <hari.prasathge@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
Changelog
v4 -> v5
v3 -> v4
v2 -> v3
- No Changes.
---
arch/arm/configs/at91_dt_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/at91_dt_defconfig b/arch/arm/configs/at91_dt_defconfig
index 1d53aec4c836..6eabe2313c9a 100644
--- a/arch/arm/configs/at91_dt_defconfig
+++ b/arch/arm/configs/at91_dt_defconfig
@@ -143,6 +143,7 @@ CONFIG_VIDEO_OV2640=m
CONFIG_VIDEO_OV7740=m
CONFIG_DRM=y
CONFIG_DRM_ATMEL_HLCDC=y
+CONFIG_DRM_MICROCHIP_LVDS_SERIALIZER=y
CONFIG_DRM_PANEL_SIMPLE=y
CONFIG_DRM_PANEL_EDP=y
CONFIG_FB_ATMEL=y
--
2.25.1
^ permalink raw reply related
* [PATCH v5 3/4] MAINTAINERS: add SAM9X7 SoC's LVDS controller
From: Dharma Balasubiramani @ 2024-04-05 4:35 UTC (permalink / raw)
To: andrzej . hajda @ intel . com, neil . armstrong @ linaro . org,
rfoss @ kernel . org, Laurent . pinchart @ ideasonboard . com,
jonas @ kwiboo . se, jernej . skrabec @ gmail . com,
maarten . lankhorst @ linux . intel . com, mripard @ kernel . org,
tzimmermann @ suse . de, airlied @ gmail . com,
daniel @ ffwll . ch, robh+dt @ kernel . org,
krzysztof . kozlowski+dt @ linaro . org, conor+dt @ kernel . org,
linux @ armlinux . org . uk, Nicolas . Ferre @ microchip . com,
alexandre . belloni @ bootlin . com,
claudiu . beznea @ tuxon . dev, Manikandan . M @ microchip . com,
Dharma . B @ microchip . com, arnd @ arndb . de,
geert+renesas @ glider . be, Jason @ zx2c4 . com,
mpe @ ellerman . id . au, gerg @ linux-m68k . org,
rdunlap @ infradead . org, vbabka @ suse . cz,
dri-devel @ lists . freedesktop . org,
devicetree @ vger . kernel . org,
linux-kernel @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
Hari . PrasathGE @ microchip . com, akpm @ linux-foundation . org,
deller @ gmx . de
Cc: Dharma Balasubiramani, Nicolas Ferre
In-Reply-To: <20240405043536.274220-1-dharma.b@microchip.com>
Add the newly added LVDS controller for the SAM9X7 SoC to the existing
MAINTAINERS entry.
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
Changelog
v4 -> v5
v3 -> v4
- No changes.
v2 -> v3
- Move the entry before "MICROCHIP SAMA5D2-COMPATIBLE ADC DRIVER".
v1 -> v2
- No Changes.
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index aa3b947fb080..3dd93dbe9542 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14562,6 +14562,14 @@ S: Supported
F: Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml
F: drivers/pwm/pwm-atmel.c
+MICROCHIP SAM9x7-COMPATIBLE LVDS CONTROLLER
+M: Manikandan Muralidharan <manikandan.m@microchip.com>
+M: Dharma Balasubiramani <dharma.b@microchip.com>
+L: dri-devel@lists.freedesktop.org
+S: Supported
+F: Documentation/devicetree/bindings/display/bridge/microchip,sam9x7-lvds.yaml
+F: drivers/gpu/drm/bridge/microchip-lvds.c
+
MICROCHIP SAMA5D2-COMPATIBLE ADC DRIVER
M: Eugen Hristev <eugen.hristev@microchip.com>
L: linux-iio@vger.kernel.org
--
2.25.1
^ permalink raw reply related
* [PATCH v5 2/4] drm/bridge: add lvds controller support for sam9x7
From: Dharma Balasubiramani @ 2024-04-05 4:35 UTC (permalink / raw)
To: andrzej . hajda @ intel . com, neil . armstrong @ linaro . org,
rfoss @ kernel . org, Laurent . pinchart @ ideasonboard . com,
jonas @ kwiboo . se, jernej . skrabec @ gmail . com,
maarten . lankhorst @ linux . intel . com, mripard @ kernel . org,
tzimmermann @ suse . de, airlied @ gmail . com,
daniel @ ffwll . ch, robh+dt @ kernel . org,
krzysztof . kozlowski+dt @ linaro . org, conor+dt @ kernel . org,
linux @ armlinux . org . uk, Nicolas . Ferre @ microchip . com,
alexandre . belloni @ bootlin . com,
claudiu . beznea @ tuxon . dev, Manikandan . M @ microchip . com,
Dharma . B @ microchip . com, arnd @ arndb . de,
geert+renesas @ glider . be, Jason @ zx2c4 . com,
mpe @ ellerman . id . au, gerg @ linux-m68k . org,
rdunlap @ infradead . org, vbabka @ suse . cz,
dri-devel @ lists . freedesktop . org,
devicetree @ vger . kernel . org,
linux-kernel @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
Hari . PrasathGE @ microchip . com, akpm @ linux-foundation . org,
deller @ gmx . de
Cc: Dharma Balasubiramani, Manikandan Muralidharan
In-Reply-To: <20240405043536.274220-1-dharma.b@microchip.com>
Add a new LVDS controller driver for sam9x7 which does the following:
- Prepares and enables the LVDS Peripheral clock
- Defines its connector type as DRM_MODE_CONNECTOR_LVDS and adds itself
to the global bridge list.
- Identifies its output endpoint as panel and adds it to the encoder
display pipeline
- Enables the LVDS serializer
Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
Changelog
v4 -> v5
- Drop the unused variable 'format'.
- Use DRM wrapper for dev_err() to maintain uniformity.
- return -ENODEV instead of -EINVAL to maintain consistency with other DRM
bridge drivers.
v3 -> v4
- No changes.
v2 ->v3
- Correct Typo error "serializer".
- Consolidate get() and prepare() functions and use devm_clk_get_prepared().
- Remove unused variable 'ret' in probe().
- Use devm_pm_runtime_enable() and drop the mchp_lvds_remove().
v1 -> v2
- Drop 'res' variable and combine two lines into one.
- Handle deferred probe properly, use dev_err_probe().
- Don't print anything on deferred probe. Dropped print.
- Remove the MODULE_ALIAS and add MODULE_DEVICE_TABLE().
- symbol 'mchp_lvds_driver' was not declared. It should be static.
---
drivers/gpu/drm/bridge/Kconfig | 7 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/microchip-lvds.c | 228 ++++++++++++++++++++++++
3 files changed, 236 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/microchip-lvds.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index efd996f6c138..889098e2d65f 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -190,6 +190,13 @@ config DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW
to DP++. This is used with the i.MX6 imx-ldb
driver. You are likely to say N here.
+config DRM_MICROCHIP_LVDS_SERIALIZER
+ tristate "Microchip LVDS serializer support"
+ depends on OF
+ depends on DRM_ATMEL_HLCDC
+ help
+ Support for Microchip's LVDS serializer.
+
config DRM_NWL_MIPI_DSI
tristate "Northwest Logic MIPI DSI Host controller"
depends on DRM
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 017b5832733b..7df87b582dca 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o
obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o
obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o
+obj-$(CONFIG_DRM_MICROCHIP_LVDS_SERIALIZER) += microchip-lvds.o
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
new file mode 100644
index 000000000000..149704f498a6
--- /dev/null
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 Microchip Technology Inc. and its subsidiaries
+ *
+ * Author: Manikandan Muralidharan <manikandan.m@microchip.com>
+ * Author: Dharma Balasubiramani <dharma.b@microchip.com>
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/delay.h>
+#include <linux/jiffies.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_graph.h>
+#include <linux/pinctrl/devinfo.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+
+#define LVDS_POLL_TIMEOUT_MS 1000
+
+/* LVDSC register offsets */
+#define LVDSC_CR 0x00
+#define LVDSC_CFGR 0x04
+#define LVDSC_SR 0x0C
+#define LVDSC_WPMR 0xE4
+
+/* Bitfields in LVDSC_CR (Control Register) */
+#define LVDSC_CR_SER_EN BIT(0)
+
+/* Bitfields in LVDSC_CFGR (Configuration Register) */
+#define LVDSC_CFGR_PIXSIZE_24BITS 0
+#define LVDSC_CFGR_DEN_POL_HIGH 0
+#define LVDSC_CFGR_DC_UNBALANCED 0
+#define LVDSC_CFGR_MAPPING_JEIDA BIT(6)
+
+/*Bitfields in LVDSC_SR */
+#define LVDSC_SR_CS BIT(0)
+
+/* Bitfields in LVDSC_WPMR (Write Protection Mode Register) */
+#define LVDSC_WPMR_WPKEY_MASK GENMASK(31, 8)
+#define LVDSC_WPMR_WPKEY_PSSWD 0x4C5644
+
+struct mchp_lvds {
+ struct device *dev;
+ void __iomem *regs;
+ struct clk *pclk;
+ struct drm_panel *panel;
+ struct drm_bridge bridge;
+ struct drm_bridge *panel_bridge;
+};
+
+static inline struct mchp_lvds *bridge_to_lvds(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct mchp_lvds, bridge);
+}
+
+static inline u32 lvds_readl(struct mchp_lvds *lvds, u32 offset)
+{
+ return readl_relaxed(lvds->regs + offset);
+}
+
+static inline void lvds_writel(struct mchp_lvds *lvds, u32 offset, u32 val)
+{
+ writel_relaxed(val, lvds->regs + offset);
+}
+
+static void lvds_serialiser_on(struct mchp_lvds *lvds)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(LVDS_POLL_TIMEOUT_MS);
+
+ /* The LVDSC registers can only be written if WPEN is cleared */
+ lvds_writel(lvds, LVDSC_WPMR, (LVDSC_WPMR_WPKEY_PSSWD &
+ LVDSC_WPMR_WPKEY_MASK));
+
+ /* Wait for the status of configuration registers to be changed */
+ while (lvds_readl(lvds, LVDSC_SR) & LVDSC_SR_CS) {
+ if (time_after(jiffies, timeout)) {
+ DRM_DEV_ERROR(lvds->dev, "%s: timeout error\n",
+ __func__);
+ return;
+ }
+ usleep_range(1000, 2000);
+ }
+
+ /* Configure the LVDSC */
+ lvds_writel(lvds, LVDSC_CFGR, (LVDSC_CFGR_MAPPING_JEIDA |
+ LVDSC_CFGR_DC_UNBALANCED |
+ LVDSC_CFGR_DEN_POL_HIGH |
+ LVDSC_CFGR_PIXSIZE_24BITS));
+
+ /* Enable the LVDS serializer */
+ lvds_writel(lvds, LVDSC_CR, LVDSC_CR_SER_EN);
+}
+
+static int mchp_lvds_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct mchp_lvds *lvds = bridge_to_lvds(bridge);
+
+ bridge->encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
+
+ return drm_bridge_attach(bridge->encoder, lvds->panel_bridge,
+ bridge, flags);
+}
+
+static void mchp_lvds_enable(struct drm_bridge *bridge)
+{
+ struct mchp_lvds *lvds = bridge_to_lvds(bridge);
+ int ret;
+
+ ret = clk_enable(lvds->pclk);
+ if (ret < 0) {
+ DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
+ return;
+ }
+
+ ret = pm_runtime_get_sync(lvds->dev);
+ if (ret < 0) {
+ DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
+ clk_disable(lvds->pclk);
+ return;
+ }
+
+ lvds_serialiser_on(lvds);
+}
+
+static void mchp_lvds_disable(struct drm_bridge *bridge)
+{
+ struct mchp_lvds *lvds = bridge_to_lvds(bridge);
+
+ pm_runtime_put(lvds->dev);
+ clk_disable(lvds->pclk);
+}
+
+static const struct drm_bridge_funcs mchp_lvds_bridge_funcs = {
+ .attach = mchp_lvds_attach,
+ .enable = mchp_lvds_enable,
+ .disable = mchp_lvds_disable,
+};
+
+static int mchp_lvds_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mchp_lvds *lvds;
+ struct device_node *port;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL);
+ if (!lvds)
+ return -ENOMEM;
+
+ lvds->dev = dev;
+
+ lvds->regs = devm_ioremap_resource(lvds->dev,
+ platform_get_resource(pdev, IORESOURCE_MEM, 0));
+ if (IS_ERR(lvds->regs))
+ return PTR_ERR(lvds->regs);
+
+ lvds->pclk = devm_clk_get_prepared(lvds->dev, "pclk");
+ if (IS_ERR(lvds->pclk))
+ return dev_err_probe(lvds->dev, PTR_ERR(lvds->pclk),
+ "could not get pclk_lvds prepared\n");
+
+ port = of_graph_get_remote_node(dev->of_node, 1, 0);
+ if (!port) {
+ DRM_DEV_ERROR(dev,
+ "can't find port point, please init lvds panel port!\n");
+ return -ENODEV;
+ }
+
+ lvds->panel = of_drm_find_panel(port);
+ of_node_put(port);
+
+ if (IS_ERR(lvds->panel))
+ return -EPROBE_DEFER;
+
+ lvds->panel_bridge = devm_drm_panel_bridge_add(dev, lvds->panel);
+
+ if (IS_ERR(lvds->panel_bridge))
+ return PTR_ERR(lvds->panel_bridge);
+
+ lvds->bridge.of_node = dev->of_node;
+ lvds->bridge.type = DRM_MODE_CONNECTOR_LVDS;
+ lvds->bridge.funcs = &mchp_lvds_bridge_funcs;
+
+ dev_set_drvdata(dev, lvds);
+ devm_pm_runtime_enable(dev);
+
+ drm_bridge_add(&lvds->bridge);
+
+ return 0;
+}
+
+static const struct of_device_id mchp_lvds_dt_ids[] = {
+ {
+ .compatible = "microchip,sam9x75-lvds",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mchp_lvds_dt_ids);
+
+static struct platform_driver mchp_lvds_driver = {
+ .probe = mchp_lvds_probe,
+ .driver = {
+ .name = "microchip-lvds",
+ .of_match_table = mchp_lvds_dt_ids,
+ },
+};
+module_platform_driver(mchp_lvds_driver);
+
+MODULE_AUTHOR("Manikandan Muralidharan <manikandan.m@microchip.com>");
+MODULE_AUTHOR("Dharma Balasubiramani <dharma.b@microchip.com>");
+MODULE_DESCRIPTION("Low Voltage Differential Signaling Controller Driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related
* [PATCH v5 1/4] dt-bindings: display: bridge: add sam9x75-lvds binding
From: Dharma Balasubiramani @ 2024-04-05 4:35 UTC (permalink / raw)
To: andrzej . hajda @ intel . com, neil . armstrong @ linaro . org,
rfoss @ kernel . org, Laurent . pinchart @ ideasonboard . com,
jonas @ kwiboo . se, jernej . skrabec @ gmail . com,
maarten . lankhorst @ linux . intel . com, mripard @ kernel . org,
tzimmermann @ suse . de, airlied @ gmail . com,
daniel @ ffwll . ch, robh+dt @ kernel . org,
krzysztof . kozlowski+dt @ linaro . org, conor+dt @ kernel . org,
linux @ armlinux . org . uk, Nicolas . Ferre @ microchip . com,
alexandre . belloni @ bootlin . com,
claudiu . beznea @ tuxon . dev, Manikandan . M @ microchip . com,
Dharma . B @ microchip . com, arnd @ arndb . de,
geert+renesas @ glider . be, Jason @ zx2c4 . com,
mpe @ ellerman . id . au, gerg @ linux-m68k . org,
rdunlap @ infradead . org, vbabka @ suse . cz,
dri-devel @ lists . freedesktop . org,
devicetree @ vger . kernel . org,
linux-kernel @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
Hari . PrasathGE @ microchip . com, akpm @ linux-foundation . org,
deller @ gmx . de
Cc: Dharma Balasubiramani, Rob Herring
In-Reply-To: <20240405043536.274220-1-dharma.b@microchip.com>
Add the 'sam9x75-lvds' compatible binding, which describes the Low Voltage
Differential Signaling (LVDS) Controller found on some Microchip's sam9x7
series System-on-Chip (SoC) devices. This binding will be used to define
the properties and configuration for the LVDS Controller in DT.
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changelog
v4 -> v5
- No changes.
v3 -> v4
- Rephrase the commit subject.
v2 -> v3
- No changes.
v1 -> v2
- Remove '|' in description, as there is no formatting to preserve.
- Remove 'gclk' from clock-names as there is only one clock(pclk).
- Remove the unused headers and include only used ones.
- Change the compatible name specific to SoC (sam9x75) instead of entire series.
- Change file name to match the compatible name.
---
.../bridge/microchip,sam9x75-lvds.yaml | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml b/Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
new file mode 100644
index 000000000000..862ef441ac9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/microchip,sam9x75-lvds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip SAM9X75 LVDS Controller
+
+maintainers:
+ - Dharma Balasubiramani <dharma.b@microchip.com>
+
+description:
+ The Low Voltage Differential Signaling Controller (LVDSC) manages data
+ format conversion from the LCD Controller internal DPI bus to OpenLDI
+ LVDS output signals. LVDSC functions include bit mapping, balanced mode
+ management, and serializer.
+
+properties:
+ compatible:
+ const: microchip,sam9x75-lvds
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Peripheral Bus Clock
+
+ clock-names:
+ items:
+ - const: pclk
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/clock/at91.h>
+ lvds-controller@f8060000 {
+ compatible = "microchip,sam9x75-lvds";
+ reg = <0xf8060000 0x100>;
+ interrupts = <56 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&pmc PMC_TYPE_PERIPHERAL 56>;
+ clock-names = "pclk";
+ };
--
2.25.1
^ permalink raw reply related
* [PATCH v5 0/4] LVDS Controller Support for SAM9X75 SoC
From: Dharma Balasubiramani @ 2024-04-05 4:35 UTC (permalink / raw)
To: andrzej . hajda @ intel . com, neil . armstrong @ linaro . org,
rfoss @ kernel . org, Laurent . pinchart @ ideasonboard . com,
jonas @ kwiboo . se, jernej . skrabec @ gmail . com,
maarten . lankhorst @ linux . intel . com, mripard @ kernel . org,
tzimmermann @ suse . de, airlied @ gmail . com,
daniel @ ffwll . ch, robh+dt @ kernel . org,
krzysztof . kozlowski+dt @ linaro . org, conor+dt @ kernel . org,
linux @ armlinux . org . uk, Nicolas . Ferre @ microchip . com,
alexandre . belloni @ bootlin . com,
claudiu . beznea @ tuxon . dev, Manikandan . M @ microchip . com,
Dharma . B @ microchip . com, arnd @ arndb . de,
geert+renesas @ glider . be, Jason @ zx2c4 . com,
mpe @ ellerman . id . au, gerg @ linux-m68k . org,
rdunlap @ infradead . org, vbabka @ suse . cz,
dri-devel @ lists . freedesktop . org,
devicetree @ vger . kernel . org,
linux-kernel @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
Hari . PrasathGE @ microchip . com, akpm @ linux-foundation . org,
deller @ gmx . de
Cc: Dharma Balasubiramani
This patch series introduces LVDS controller support for the SAM9X75 SoC. The
LVDS controller is designed to work with Microchip's sam9x7 series
System-on-Chip (SoC) devices, providing Low Voltage Differential Signaling
capabilities.
Patch series Changelog:
- Include configs: at91: Enable LVDS serializer
- include all necessary To/Cc entries.
The Individual Changelogs are available on the respective patches.
Dharma Balasubiramani (4):
dt-bindings: display: bridge: add sam9x75-lvds binding
drm/bridge: add lvds controller support for sam9x7
MAINTAINERS: add SAM9X7 SoC's LVDS controller
ARM: configs: at91: Enable LVDS serializer support
.../bridge/microchip,sam9x75-lvds.yaml | 55 +++++
MAINTAINERS | 8 +
arch/arm/configs/at91_dt_defconfig | 1 +
drivers/gpu/drm/bridge/Kconfig | 7 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/microchip-lvds.c | 228 ++++++++++++++++++
6 files changed, 300 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
create mode 100644 drivers/gpu/drm/bridge/microchip-lvds.c
--
2.25.1
^ permalink raw reply
* Re: [PATCH v17 11/35] virt: gunyah: Translate gh_rm_hyp_resource into gunyah_resource
From: Pavan Kondeti @ 2024-04-05 3:10 UTC (permalink / raw)
To: Elliot Berman
Cc: Alex Elder, Srinivas Kandagatla, Murali Nalajal, Trilok Soni,
Srivatsa Vaddagiri, Carl van Schaik, Philip Derrin,
Prakruthi Deepak Heragu, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas, Will Deacon,
Konrad Dybcio, Bjorn Andersson, Dmitry Baryshkov, Fuad Tabba,
Sean Christopherson, Andrew Morton, linux-arm-msm, linux-doc,
linux-kernel, devicetree, linux-arm-kernel, linux-mm
In-Reply-To: <20240222-gunyah-v17-11-1e9da6763d38@quicinc.com>
On Thu, Feb 22, 2024 at 03:16:34PM -0800, Elliot Berman wrote:
> When booting a Gunyah virtual machine, the host VM may gain capabilities
> to interact with resources for the guest virtual machine. Examples of
> such resources are vCPUs or message queues. To use those resources, we
> need to translate the RM response into a gunyah_resource structure which
> are useful to Linux drivers. Presently, Linux drivers need only to know
> the type of resource, the capability ID, and an interrupt.
>
> On ARM64 systems, the interrupt reported by Gunyah is the GIC interrupt
> ID number and always a SPI or extended SPI.
>
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
> arch/arm64/include/asm/gunyah.h | 36 ++++++++++++++++++++++
> drivers/virt/gunyah/rsc_mgr.c | 67 +++++++++++++++++++++++++++++++++++++++++
> drivers/virt/gunyah/rsc_mgr.h | 5 +++
> include/linux/gunyah.h | 2 ++
> 4 files changed, 110 insertions(+)
>
> diff --git a/arch/arm64/include/asm/gunyah.h b/arch/arm64/include/asm/gunyah.h
> new file mode 100644
> index 0000000000000..0cd3debe22b64
> --- /dev/null
> +++ b/arch/arm64/include/asm/gunyah.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +#ifndef _ASM_GUNYAH_H
> +#define _ASM_GUNYAH_H
> +
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +
> +static inline int arch_gunyah_fill_irq_fwspec_params(u32 virq,
> + struct irq_fwspec *fwspec)
> +{
> + /* Assume that Gunyah gave us an SPI or ESPI; defensively check it */
> + if (WARN(virq < 32, "Unexpected virq: %d\n", virq)) {
> + return -EINVAL;
> + } else if (virq <= 1019) {
> + fwspec->param_count = 3;
> + fwspec->param[0] = 0; /* GIC_SPI */
> + fwspec->param[1] = virq - 32; /* virq 32 -> SPI 0 */
> + fwspec->param[2] = IRQ_TYPE_EDGE_RISING;
> + } else if (WARN(virq < 4096, "Unexpected virq: %d\n", virq)) {
> + return -EINVAL;
> + } else if (virq < 5120) {
> + fwspec->param_count = 3;
> + fwspec->param[0] = 2; /* GIC_ESPI */
> + fwspec->param[1] = virq - 4096; /* virq 4096 -> ESPI 0 */
> + fwspec->param[2] = IRQ_TYPE_EDGE_RISING;
> + } else {
> + WARN(1, "Unexpected virq: %d\n", virq);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
__get_intid_range() in gic-v3 driver looks more pleasing. Other than
that the logic for the translation looks good to me.
Thanks,
Pavan
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: qcs6490-rb3gen2: Enable various remoteprocs
From: Bjorn Andersson @ 2024-04-05 2:53 UTC (permalink / raw)
To: Komal Bajaj
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel, quic_tsoni
In-Reply-To: <20240402090349.30172-3-quic_kbajaj@quicinc.com>
On Tue, Apr 02, 2024 at 02:33:49PM +0530, Komal Bajaj wrote:
> Enable the ADSP, CDSP and WPSS that are found on qcs6490-rb3gen2.
>
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 97824c769ba3..a25431ddf922 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -434,6 +434,21 @@ &qupv3_id_0 {
> status = "okay";
> };
>
> +&remoteproc_adsp {
> + firmware-name = "qcom/qcm6490/adsp.mbn";
Should this be qcm6490?
I already proposed a patch to add adsp and cdsp, using qcs6490, and this
was merged earlier this week. Please pay attention and review patches
posted on the public list.
Either way, this will now have to be rebased on linux-next.
Thanks,
Bjorn
> + status = "okay";
> +};
> +
> +&remoteproc_cdsp {
> + firmware-name = "qcom/qcm6490/cdsp.mbn";
> + status = "okay";
> +};
> +
> +&remoteproc_wpss {
> + firmware-name = "qcom/qcm6490/wpss.mbn";
> + status = "okay";
> +};
> +
> &tlmm {
> gpio-reserved-ranges = <32 2>, /* ADSP */
> <48 4>; /* NFC */
> --
> 2.42.0
>
^ 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