* [PATCH 1/4] dts: rockchip: Add chasis-type for Powkiddy rk3566 devices
From: Chris Morgan @ 2024-03-25 17:51 UTC (permalink / raw)
To: linux-rockchip
Cc: devicetree, heiko, conor+dt, krzysztof.kozlowski+dt, robh,
Chris Morgan
In-Reply-To: <20240325175133.19393-1-macroalpha82@gmail.com>
From: Chris Morgan <macromorgan@hotmail.com>
Add the optional node of chasis-type for Powkiddy RK3566 based
devices.
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dtsi | 2 ++
arch/arm64/boot/dts/rockchip/rk3566-powkiddy-x55.dts | 1 +
2 files changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dtsi
index 3ab751a01cb2..bd332714a023 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dtsi
@@ -10,6 +10,8 @@
#include "rk3566.dtsi"
/ {
+ chassis-type = "handset";
+
aliases {
mmc1 = &sdmmc0;
mmc2 = &sdmmc1;
diff --git a/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-x55.dts b/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-x55.dts
index 4786b19fd017..5a648db41f35 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-x55.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-x55.dts
@@ -11,6 +11,7 @@
/ {
model = "Powkiddy x55";
+ chassis-type = "handset";
compatible = "powkiddy,x55", "rockchip,rk3566";
aliases {
--
2.34.1
^ permalink raw reply related
* Re: (subset) [PATCH 0/5] Add TCPM support for PM7250B and Fairphone 4
From: Mark Brown @ 2024-03-25 17:44 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Liam Girdwood, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Wesley Cheng,
Bryan O'Donoghue, Greg Kroah-Hartman, Luca Weiss
Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm,
linux-kernel, devicetree, linux-usb
In-Reply-To: <20240322-fp4-tcpm-v1-0-c5644099d57b@fairphone.com>
On Fri, 22 Mar 2024 09:01:31 +0100, Luca Weiss wrote:
> This series adds support for Type-C Port Management on the Fairphone 4
> which enables USB role switching and orientation switching.
>
> This enables a user for example to plug in a USB stick or a USB keyboard
> to the Type-C port.
>
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/5] dt-bindings: regulator: qcom,usb-vbus-regulator: Add PM7250B compatible
commit: 0c5f77f4eaef8ed9fe752d21f40ac471dd511cfc
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: (subset) [PATCH v5 0/3] add pmic pca9451a support
From: Mark Brown @ 2024-03-25 17:44 UTC (permalink / raw)
To: frank.li, ping.bai, lgirdwood, robh+dt, krzysztof.kozlowski+dt,
conor+dt, shawnguo, s.hauer, Joy Zou
Cc: kernel, festevam, linux-imx, devicetree, linux-arm-kernel,
linux-kernel, imx
In-Reply-To: <20240318095633.4079027-1-joy.zou@nxp.com>
On Mon, 18 Mar 2024 17:56:30 +0800, Joy Zou wrote:
> The patchset supports pmic pca9451a.
> For the details, please check the patch commit log.
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/3] regulator: dt-bindings: pca9450: add pca9451a support
commit: 0f3b73eb1440d18d2236ac660d4aa76fd726bec7
[2/3] regulator: pca9450: add pca9451a support
commit: 5edeb7d312628961046eec9b26a7e72f44baf846
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: (subset) [PATCH v4 0/3] add pmic pca9451a support
From: Mark Brown @ 2024-03-25 17:44 UTC (permalink / raw)
To: ping.bai, lgirdwood, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, Joy Zou
Cc: kernel, festevam, linux-imx, devicetree, linux-arm-kernel,
linux-kernel, imx
In-Reply-To: <20240314032923.2360248-1-joy.zou@nxp.com>
On Thu, 14 Mar 2024 11:29:20 +0800, Joy Zou wrote:
> The patchset supports pmic pca9451a.
> For the details, please check the patch commit log.
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/3] regulator: dt-bindings: pca9450: add pca9451a support
commit: 0f3b73eb1440d18d2236ac660d4aa76fd726bec7
[2/3] regulator: pca9450: add pca9451a support
commit: 5edeb7d312628961046eec9b26a7e72f44baf846
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: (subset) [PATCH v3 0/3] add pmic pca9451a support
From: Mark Brown @ 2024-03-25 17:44 UTC (permalink / raw)
To: ping.bai, lgirdwood, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, Joy Zou
Cc: kernel, festevam, linux-imx, devicetree, linux-arm-kernel,
linux-kernel, imx
In-Reply-To: <20240311113201.664252-1-joy.zou@nxp.com>
On Mon, 11 Mar 2024 19:31:58 +0800, Joy Zou wrote:
> The patchset supports pmic pca9451a.
> For the details, please check the patch commit log.
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/3] regulator: dt-bindings: pca9450: add pca9451a support
commit: 0f3b73eb1440d18d2236ac660d4aa76fd726bec7
[2/3] regulator: pca9450: add pca9451a support
commit: 5edeb7d312628961046eec9b26a7e72f44baf846
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: (subset) [PATCH v1 0/3] add pmic pca9451a support
From: Mark Brown @ 2024-03-25 17:44 UTC (permalink / raw)
To: ping.bai, lgirdwood, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, Joy Zou
Cc: kernel, festevam, linux-imx, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20230531065724.3671795-1-joy.zou@nxp.com>
On Wed, 31 May 2023 14:57:21 +0800, Joy Zou wrote:
> The patchset supports pmic pca9451a.
> For the details, please check the patch commit log.
>
> Joy Zou (3):
> dt-bindings: regulator: pca9450: add pca9451a support
> regulator: pca9450: add pca9451a support
> arm64: dts: imx93-11x11-evk: add pca9451a support
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/3] dt-bindings: regulator: pca9450: add pca9451a support
(no commit info)
[2/3] regulator: pca9450: add pca9451a support
commit: 5edeb7d312628961046eec9b26a7e72f44baf846
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH v2 1/1] ASoC: dt-bindings: fsl-audmix: Convert fsl,audmix.txt to yaml
From: Mark Brown @ 2024-03-25 17:44 UTC (permalink / raw)
To: Liam Girdwood, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shengjiu Wang, linux-sound, devicetree, linux-kernel, Frank Li
Cc: imx
In-Reply-To: <20240321150401.2464783-1-Frank.Li@nxp.com>
On Thu, 21 Mar 2024 11:04:00 -0400, Frank Li wrote:
> Convert fsl,audmix.txt to yaml to let dtb check tools check audmix part.
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: dt-bindings: fsl-audmix: Convert fsl,audmix.txt to yaml
commit: 33a499668eb6781a4518e4facf2d85cc8cd1a650
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH] ASoC: dt-bindings: wm8776: Convert to dtschema
From: Mark Brown @ 2024-03-25 17:44 UTC (permalink / raw)
To: lgirdwood, robh+dt, krzysztof.kozlowski+dt, conor+dt,
Kartik Agarwala
Cc: patches, linux-sound, devicetree, linux-kernel,
javier.carrasco.cruz
In-Reply-To: <20240317200201.119233-1-agarwala.kartik@gmail.com>
On Mon, 18 Mar 2024 01:32:02 +0530, Kartik Agarwala wrote:
> Convert WM8776 audio CODEC bindings from text to dtschema.
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: dt-bindings: wm8776: Convert to dtschema
commit: 1fdc23aa57913d7a0a521f71dcdefdde9364d4f9
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH v16 7/9] usb: dwc3: qcom: Refactor IRQ handling in glue driver
From: Krishna Kurapati PSSNV @ 2024-03-25 17:29 UTC (permalink / raw)
To: Johan Hovold
Cc: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, devicetree, linux-arm-msm, linux-usb, linux-kernel,
quic_ppratap, quic_jackp
In-Reply-To: <ZgF6zvaT2OkrbkHK@hovoldconsulting.com>
On 3/25/2024 6:53 PM, Johan Hovold wrote:
> On Mon, Mar 25, 2024 at 06:45:07PM +0530, Krishna Kurapati PSSNV wrote:
>>>> +static int dwc3_qcom_setup_irq(struct platform_device *pdev)
>>>> +{
>>>> + struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
>>>> + bool is_multiport;
>>>> + int ret;
>>>> + int i;
>>>> +
>>>> + qcom->num_ports = dwc3_qcom_find_num_ports(pdev);
>>>> + if (qcom->num_ports < 0)
>>>> + return -ENOMEM;
>>>
>>> Just return 'ret' directly.
>>
>> Sure, will init ret to -ENOMEM and return ret here. >
Hi Johan,
> I meant that you should return whatever error dwc3_qcom_find_num_ports()
> returns, so perhaps something like:
>
Got it. Any error that might come up in interrupt reading as well.
>
> ret = dwc3_qcom_find_num_ports(pdev);
> if (ret < 0)
> return ret;
>
> qcom->num_ports = ret;
>
> It looks like dwc3_qcom_find_num_ports() can also return 0 (e.g. on
> malformed DT), which also needs to be handled somehow. I missed that
> earlier.
>
From what I remember, Konrad mentioned that we might not need to
support incomplete or improper DT [1]. Also since this is close to
getting merged, can we take up any changes for Malformed DT handling
later given that only one or two devices are present and less likely to
be given a malformed DT.
[1]:
https://lore.kernel.org/all/c8d77d4f-6696-4dc9-8030-daf1d10b114b@linaro.org/
Regards,
Krishna,
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: rockchip: Correct RGxx3 SDMMC ordering
From: Chris Morgan @ 2024-03-25 17:16 UTC (permalink / raw)
To: Heiko Stübner
Cc: linux-rockchip, Chris Morgan, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh
In-Reply-To: <2963425.VdNmn5OnKV@diego>
On Mon, Mar 25, 2024 at 04:39:28PM +0100, Heiko Stübner wrote:
> Hmm,
>
> Am Montag, 25. März 2024, 15:37:28 CET schrieb Chris Morgan:
> > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Make the order of SDMMC predictable across the entire device lineup.
> > This allows userspace to always know that sdmmc0 is the eMMC (when
> > present), sdmmc1 is always the first sd card slot, sdmmc2 is always
> > the 2nd sd card slot (when present), and sdmmc3 is always the wifi
> > (when present).
> >
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>
> this is really really difficult.
>
> For example, you added the rg353p, rk503 nad rg353vs and their aliases
> two years ago for 6.1 , the rg353ps for 6.5. So someone might actually
> rely on those already in their userspace setup.
>
> So reorganizing the aliases to point to different devices might actually
> break userspace, which as you might know, is sort of a big deal ;-) .
>
> Userspace programs can determine the mmc mapping via the board-compatible
> though.
Okay, then this might need to be disregarded. Perhaps at a minimum we can
set the WiFi device to be mmc3 at least for a consistent basis. That was
the main driver of this request and shouldn't be a major concern for
userspace. Would that work?
Thank you.
>
>
> Heiko
>
> > ---
> > arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg-arc-d.dts | 7 -------
> > arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg-arc-s.dts | 6 ------
> > arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353p.dts | 7 -------
> > arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353ps.dts | 6 ------
> > arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353v.dts | 7 -------
> > arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353vs.dts | 6 ------
> > arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts | 6 ------
> > arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi | 7 +++++++
> > 8 files changed, 7 insertions(+), 45 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg-arc-d.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg-arc-d.dts
> > index ab83e8a61615..d239a8452957 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg-arc-d.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg-arc-d.dts
> > @@ -10,13 +10,6 @@
> > / {
> > model = "Anbernic RG ARC-D";
> > compatible = "anbernic,rg-arc-d", "rockchip,rk3566";
> > -
> > - aliases {
> > - mmc0 = &sdhci;
> > - mmc1 = &sdmmc0;
> > - mmc2 = &sdmmc1;
> > - mmc3 = &sdmmc2;
> > - };
> > };
> >
> > &i2c2 {
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg-arc-s.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg-arc-s.dts
> > index 6264a8c78d0b..4bb13d94a759 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg-arc-s.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg-arc-s.dts
> > @@ -10,10 +10,4 @@
> > / {
> > model = "Anbernic RG ARC-S";
> > compatible = "anbernic,rg-arc-s", "rockchip,rk3566";
> > -
> > - aliases {
> > - mmc1 = &sdmmc0;
> > - mmc2 = &sdmmc1;
> > - mmc3 = &sdmmc2;
> > - };
> > };
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353p.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353p.dts
> > index 8aa93c646bec..94c678c44d3a 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353p.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353p.dts
> > @@ -11,13 +11,6 @@ / {
> > model = "RG353P";
> > compatible = "anbernic,rg353p", "rockchip,rk3566";
> >
> > - aliases {
> > - mmc0 = &sdhci;
> > - mmc1 = &sdmmc0;
> > - mmc2 = &sdmmc1;
> > - mmc3 = &sdmmc2;
> > - };
> > -
> > battery: battery {
> > compatible = "simple-battery";
> > charge-full-design-microamp-hours = <3472000>;
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353ps.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353ps.dts
> > index b211973e36c2..25edd81ce26b 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353ps.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353ps.dts
> > @@ -11,12 +11,6 @@ / {
> > model = "RG353PS";
> > compatible = "anbernic,rg353ps", "rockchip,rk3566";
> >
> > - aliases {
> > - mmc0 = &sdmmc0;
> > - mmc1 = &sdmmc1;
> > - mmc2 = &sdmmc2;
> > - };
> > -
> > battery: battery {
> > compatible = "simple-battery";
> > charge-full-design-microamp-hours = <3472000>;
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353v.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353v.dts
> > index f49ce29ba597..5354c5958df2 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353v.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353v.dts
> > @@ -11,13 +11,6 @@ / {
> > model = "RG353V";
> > compatible = "anbernic,rg353v", "rockchip,rk3566";
> >
> > - aliases {
> > - mmc0 = &sdhci;
> > - mmc1 = &sdmmc0;
> > - mmc2 = &sdmmc1;
> > - mmc3 = &sdmmc2;
> > - };
> > -
> > battery: battery {
> > compatible = "simple-battery";
> > charge-full-design-microamp-hours = <3151000>;
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353vs.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353vs.dts
> > index a7dc462fe21f..02653b59f6c2 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353vs.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353vs.dts
> > @@ -11,12 +11,6 @@ / {
> > model = "RG353VS";
> > compatible = "anbernic,rg353vs", "rockchip,rk3566";
> >
> > - aliases {
> > - mmc0 = &sdmmc0;
> > - mmc1 = &sdmmc1;
> > - mmc2 = &sdmmc2;
> > - };
> > -
> > battery: battery {
> > compatible = "simple-battery";
> > charge-full-design-microamp-hours = <3151000>;
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
> > index 94e6dd61a2db..e7161a86a9f1 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
> > @@ -11,12 +11,6 @@ / {
> > model = "RG503";
> > compatible = "anbernic,rg503", "rockchip,rk3566";
> >
> > - aliases {
> > - mmc0 = &sdmmc0;
> > - mmc1 = &sdmmc1;
> > - mmc2 = &sdmmc2;
> > - };
> > -
> > adc-joystick {
> > compatible = "adc-joystick";
> > io-channels = <&adc_mux 0>,
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi
> > index 233eade30f21..47c8fdc7c843 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi
> > @@ -12,6 +12,13 @@
> > / {
> > chassis-type = "handset";
> >
> > + aliases {
> > + mmc0 = &sdhci;
> > + mmc1 = &sdmmc0;
> > + mmc2 = &sdmmc1;
> > + mmc3 = &sdmmc2;
> > + };
> > +
> > chosen: chosen {
> > stdout-path = "serial2:1500000n8";
> > };
> >
>
>
>
>
^ permalink raw reply
* Re: [PATCH 1/3] remoteproc: Add Arm remoteproc driver
From: Abdellatif El Khlifi @ 2024-03-25 17:13 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Sudeep Holla, Abdellatif El Khlifi, Bjorn Andersson, Rob Herring,
Liviu Dudau, Lorenzo Pieralisi, Krzysztof Kozlowski, Conor Dooley,
Drew.Reed, Adam.Johnston, linux-arm-kernel, devicetree,
linux-kernel, linux-remoteproc
In-Reply-To: <CANLsYkzdfP8Np-XwPDt=GBNLYiSypd8tNdb29KUwr+tyi7gJEA@mail.gmail.com>
Hi Mathieu,
> > > > > > > > This is an initial patchset for allowing to turn on and off the remote processor.
> > > > > > > > The FW is already loaded before the Corstone-1000 SoC is powered on and this
> > > > > > > > is done through the FPGA board bootloader in case of the FPGA target. Or by the Corstone-1000 FVP model
> > > > > > > > (emulator).
> > > > > > > >
> > > > > > > >From the above I take it that booting with a preloaded firmware is a
> > > > > > > scenario that needs to be supported and not just a temporary stage.
> > > > > >
> > > > > > The current status of the Corstone-1000 SoC requires that there is
> > > > > > a preloaded firmware for the external core. Preloading is done externally
> > > > > > either through the FPGA bootloader or the emulator (FVP) before powering
> > > > > > on the SoC.
> > > > > >
> > > > >
> > > > > Ok
> > > > >
> > > > > > Corstone-1000 will be upgraded in a way that the A core running Linux is able
> > > > > > to share memory with the remote core and also being able to access the remote
> > > > > > core memory so Linux can copy the firmware to. This HW changes are still
> > > > > > This is why this patchset is relying on a preloaded firmware. And it's the step 1
> > > > > > of adding remoteproc support for Corstone.
> > > > > >
> > > > >
> > > > > Ok, so there is a HW problem where A core and M core can't see each other's
> > > > > memory, preventing the A core from copying the firmware image to the proper
> > > > > location.
> > > > >
> > > > > When the HW is fixed, will there be a need to support scenarios where the
> > > > > firmware image has been preloaded into memory?
> > > >
> > > > No, this scenario won't apply when we get the HW upgrade. No need for an
> > > > external entity anymore. The firmware(s) will all be files in the linux filesystem.
> > > >
> > >
> > > Very well. I am willing to continue with this driver but it does so little that
> > > I wonder if it wouldn't simply be better to move forward with upstreaming when
> > > the HW is fixed. The choice is yours.
> > >
> >
> > I think Robin has raised few points that need clarification. I think it was
> > done as part of DT binding patch. I share those concerns and I wanted to
> > reaching to the same concerns by starting the questions I asked on corstone
> > device tree changes.
> >
>
> I also agree with Robin's point of view. Proceeding with an initial
> driver with minimal functionality doesn't preclude having complete
> bindings. But that said and as I pointed out, it might be better to
> wait for the HW to be fixed before moving forward.
We checked with the HW teams. The missing features will be implemented but
this will take time.
The foundation driver as it is right now is still valuable for people wanting to
know how to power control Corstone external systems in a future proof manner
(even in the incomplete state). We prefer to address all the review comments
made so it can be merged. This includes making the DT binding as complete as
possible as you advised. Then, once the HW is ready, I'll implement the comms
and the FW reload part. Is that OK please ?
Cheers,
Abdellatif
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: hwmon: Add NCT7363Y documentation
From: Guenter Roeck @ 2024-03-25 17:09 UTC (permalink / raw)
To: Rob Herring, baneric926
Cc: linux-hwmon, krzysztof.kozlowski+dt, robh+dt, conor+dt, corbet,
jdelvare, kwliu, kcfeng0, Paul Menzel, linux-kernel, devicetree,
Bonnie_Lo, linux-doc, DELPHINE_CHIU, openbmc
In-Reply-To: <171109961635.307786.7810067768607811171.robh@kernel.org>
On 3/22/24 02:26, Rob Herring wrote:
>
> On Fri, 22 Mar 2024 16:11:57 +0800, baneric926@gmail.com wrote:
>> From: Ban Feng <kcfeng0@nuvoton.com>
>>
>> Add bindings for the Nuvoton NCT7363Y Fan Controller
>>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> Signed-off-by: Ban Feng <kcfeng0@nuvoton.com>
>> ---
>> .../bindings/hwmon/nuvoton,nct7363.yaml | 66 +++++++++++++++++++
>> MAINTAINERS | 6 ++
>> 2 files changed, 72 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml:
> Error in referenced schema matching $id: http://devicetree.org/schemas/hwmon/fan-common.yaml
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.example.dtb: hwmon@22: fan-0: False schema does not allow {'pwms': [[1, 0, 50000]], 'tach-ch': ['']}
> from schema $id: http://devicetree.org/schemas/hwmon/nuvoton,nct7363.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.example.dtb: hwmon@22: fan-0: Unevaluated properties are not allowed ('pwms', 'tach-ch' were unexpected)
> from schema $id: http://devicetree.org/schemas/hwmon/nuvoton,nct7363.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.example.dtb: hwmon@22: fan-1: False schema does not allow {'pwms': [[1, 1, 50000]], 'tach-ch': b'\x01'}
> from schema $id: http://devicetree.org/schemas/hwmon/nuvoton,nct7363.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.example.dtb: hwmon@22: fan-1: Unevaluated properties are not allowed ('pwms', 'tach-ch' were unexpected)
> from schema $id: http://devicetree.org/schemas/hwmon/nuvoton,nct7363.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.example.dtb: fan-1: tach-ch: b'\x01' is not of type 'object', 'array', 'boolean', 'null'
> from schema $id: http://devicetree.org/schemas/dt-core.yaml#
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240322081158.4106326-2-kcfeng0@nuvoton.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>
I am a bit puzzled by this one. The patch has a Reviewed-by: tag from Rob,
but then Rob's bot complains about errors. hat am I missing ?
Thanks,
Guenter
^ permalink raw reply
* [PATCH] ARM: dts: stm32: add heartbeat led for stm32mp157c-ed1
From: patrice.chotard @ 2024-03-25 17:06 UTC (permalink / raw)
To: robh+dt, Krzysztof Kozlowski, alexandre.torgue
Cc: linux-stm32, linux-arm-kernel, linux-kernel, devicetree,
Patrice Chotard
From: Patrice Chotard <patrice.chotard@foss.st.com>
Add heartbeat led for stm32mp157c-ed1.
Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
arch/arm/boot/dts/st/stm32mp157c-ed1.dts | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/st/stm32mp157c-ed1.dts b/arch/arm/boot/dts/st/stm32mp157c-ed1.dts
index 66ed5f9921ba..bd727455a7e4 100644
--- a/arch/arm/boot/dts/st/stm32mp157c-ed1.dts
+++ b/arch/arm/boot/dts/st/stm32mp157c-ed1.dts
@@ -24,6 +24,15 @@ chosen {
stdout-path = "serial0:115200n8";
};
+ led {
+ compatible = "gpio-leds";
+ led-blue {
+ gpios = <&gpiod 9 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "heartbeat";
+ default-state = "off";
+ };
+ };
+
memory@c0000000 {
device_type = "memory";
reg = <0xC0000000 0x40000000>;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] drivers: watchdog: ast2600 support bootstatus
From: Guenter Roeck @ 2024-03-25 17:06 UTC (permalink / raw)
To: PeterYin, patrick, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Joel Stanley, Andrew Jeffery, Wim Van Sebroeck, devicetree,
linux-arm-kernel, linux-aspeed, linux-kernel, linux-watchdog
In-Reply-To: <924c4402-af14-4b7a-9a4e-4317c50482cd@gmail.com>
On 3/20/24 02:05, PeterYin wrote:
>
>
> Guenter Roeck 於 3/19/24 08:46 寫道:
>> On 3/17/24 22:52, Peter Yin wrote:
>>> Add WDIOF_EXTERN1 and WDIOF_CARDRESET bootstatus in ast2600
>>>
>>> Regarding the AST2600 specification, the WDTn Timeout Status Register
>>> (WDT10) has bit 1 reserved. To verify the second boot source,
>>> we need to check SEC14 bit 12 and bit 13.
>>> The bits 8-23 in the WDTn Timeout Status Register are the Watchdog
>>> Event Count, which we can use to verify WDIOF_EXTERN1.
>>>
>>> Signed-off-by: Peter Yin <peteryin.openbmc@gmail.com>
>>
>> You'll have to separate dts and yaml file changes from driver changes.
>>
>>> ---
>>> Change log:
>>>
>>> v1 -> v2
>>> - Add comment and support WDIOF_CARDRESET in ast2600
>>>
>>> v1
>>> - Patch 0001 - Add WDIOF_EXTERN1 bootstatus
>>> ---
>>> arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 8 ++---
>>> drivers/watchdog/aspeed_wdt.c | 45 ++++++++++++++++++++++---
>>> 2 files changed, 44 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
>>> index e0b44498269f..23ae7f0430e9 100644
>>> --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
>>> +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
>>> @@ -556,24 +556,24 @@ uart5: serial@1e784000 {
>>> wdt1: watchdog@1e785000 {
>>> compatible = "aspeed,ast2600-wdt";
>>> - reg = <0x1e785000 0x40>;
>>> + reg = <0x1e785000 0x40>, <0x1e6f2000 0x20>;
>>> };
>>> wdt2: watchdog@1e785040 {
>>> compatible = "aspeed,ast2600-wdt";
>>> - reg = <0x1e785040 0x40>;
>>> + reg = <0x1e785040 0x40>, <0x1e6f2000 0x020>;
>>> status = "disabled";
>>> };
>>> wdt3: watchdog@1e785080 {
>>> compatible = "aspeed,ast2600-wdt";
>>> - reg = <0x1e785080 0x40>;
>>> + reg = <0x1e785080 0x40>, <0x1e6f2000 0x020>;
>>> status = "disabled";
>>> };
>>> wdt4: watchdog@1e7850c0 {
>>> compatible = "aspeed,ast2600-wdt";
>>> - reg = <0x1e7850C0 0x40>;
>>> + reg = <0x1e7850C0 0x40>, <0x1e6f2000 0x020>;
>>> status = "disabled";
>>> };
>>> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
>>> index b4773a6aaf8c..65118e461130 100644
>>> --- a/drivers/watchdog/aspeed_wdt.c
>>> +++ b/drivers/watchdog/aspeed_wdt.c
>>> @@ -33,6 +33,7 @@ struct aspeed_wdt {
>>> void __iomem *base;
>>> u32 ctrl;
>>> const struct aspeed_wdt_config *cfg;
>>> + void __iomem *sec_base;
>>> };
>>> static const struct aspeed_wdt_config ast2400_config = {
>>> @@ -82,6 +83,15 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
>>> #define WDT_RESET_MASK1 0x1c
>>> #define WDT_RESET_MASK2 0x20
>>> +/*
>>> + * Only Ast2600 support
>>> + */
>>> +#define WDT_EVENT_COUNTER_MASK (0xFFF << 8)
>>> +#define WDT_SECURE_ENGINE_STATUS (0x14)
>>> +#define ABR_IMAGE_SOURCE BIT(12)
>>> +#define ABR_IMAGE_SOURCE_SPI BIT(13)
>>> +#define SECOND_BOOT_ENABLE BIT(14)
>>> +
>>> /*
>>> * WDT_RESET_WIDTH controls the characteristics of the external pulse (if
>>> * enabled), specifically:
>>> @@ -313,6 +323,7 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>>> const char *reset_type;
>>> u32 duration;
>>> u32 status;
>>> + u32 sec_st;
>>> int ret;
>>> wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
>>> @@ -330,6 +341,12 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>>> if (IS_ERR(wdt->base))
>>> return PTR_ERR(wdt->base);
>>> + if (of_device_is_compatible(np, "aspeed,ast2600-wdt")) {
>>> + wdt->sec_base = devm_platform_ioremap_resource(pdev, 1);
>>> + if (IS_ERR(wdt->sec_base))
>>> + return PTR_ERR(wdt->sec_base);
>>> + }
>>> +
>>> wdt->wdd.info = &aspeed_wdt_info;
>>> if (wdt->cfg->irq_mask) {
>>> @@ -459,12 +476,30 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>>> }
>>> status = readl(wdt->base + WDT_TIMEOUT_STATUS);
>>> - if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY) {
>>> - wdt->wdd.bootstatus = WDIOF_CARDRESET;
>>> - if (of_device_is_compatible(np, "aspeed,ast2400-wdt") ||
>>> - of_device_is_compatible(np, "aspeed,ast2500-wdt"))
>>> - wdt->wdd.groups = bswitch_groups;
>>> + if (of_device_is_compatible(np, "aspeed,ast2600-wdt")) {
>>> + /*
>>> + * The WDTn Timeout Status Register bit 1 is reserved.
>>> + * To verify the second boot source,
>>> + * we need to check SEC14 bit 12 and bit 13.
>>> + */
>>> + sec_st = readl(wdt->sec_base + WDT_SECURE_ENGINE_STATUS);
>>> + if( sec_st & SECOND_BOOT_ENABLE)
>>> + if (sec_st & ABR_IMAGE_SOURCE ||
>>> + sec_st & ABR_IMAGE_SOURCE_SPI)
>>
>> I am sure that checkpatch as something to say here. Either case, I would very
>> much prefer a single if() statement such as
>>
>> if (sec_st & SECOND_BOOT_ENABLE &&
>> sec_st & (ABR_IMAGE_SOURCE | ABR_IMAGE_SOURCE_SPI))
>>
>>> + wdt->wdd.bootstatus |= WDIOF_CARDRESET;
>>> +
>>> + /*
>>> + * To check Watchdog Event Count for WDIOF_EXTERN1
>>> + */
>>> + if (status & WDT_EVENT_COUNTER_MASK) {
>>> + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
>>> + }
>>
>> Unnecessary { }
>>
>> ... but does this really indicate that there was a reset due to some event ?
>> This reads three 8-bit counters. Wouldn't it make more sense to check bit 0
>> instead ?
>>
>> I am also not sure if reading the watchdog status from WDT_SECURE_ENGINE_STATUS
>> adds any value over the status reported in the watchdog status register.
>> You'll have to explain why the added complexity is necessary or even adds
>> value.
>>
>> Never mind, though ...
>>
>> Looking into the datasheets, the current code is quite completely wrong anyway.
>> Bit 1 of the status register indicates on ast2500 if the boot was from the second
>> boot source. It does not indicate that the most recent reset was triggered by
>> the watchdog. The code should just be changed to set WDIOF_CARDRESET if bit 0
>> of the status register is set. The boot source is out of scope for the watchdog
>> status bits.
>>
>> Thanks,
>> Guenter
>>
> Ast2600 has external reset flag on scu74 bit 1
> Can I modify the code like this?
>
> To set WDIOF_EXTERN1 if EXTERN_RESET_FLAG is set,
> To set WDIOF_CARDRESET if WDT_TIMEOUT_STATUS_EVENT(bit0) is set
>
>
> #define WDT_TIMEOUT_STATUS_EVENT BIT(0)
> #define EXTERN_RESET_FLAG BIT(1)
> #define ASPEED_SYSTEM_RESET_EVENT (0x74)
>
> status = readl(wdt->base + WDT_TIMEOUT_STATUS);
> if (status & WDT_TIMEOUT_STATUS_EVENT)
> wdt->wdd.bootstatus = WDIOF_CARDRESET;
>
> if (of_device_is_compatible(np, "aspeed,ast2600-wdt")) {
> status = readl(wdt->scu_base + ASPEED_SYSTEM_RESET_EVENT);
ASPEED_SYSTEM_RESET_EVENT is at offset 0x74, but the devicetree nodes
for "aspeed,ast2600-wdt" only request 0x20 bytes for scu. I don't really
understand, though, how this is supposed to work in the first place, since
the entire SCU address range is also requested by "aspeed,ast2600-sbc".
Granted, I don't see actual code in the upstream kernel listing itself
as compatible with "aspeed,ast2600-sbc" (it looks like attempts to upstream
that code were unsuccessful), but it seems wrong to use that memory space
for the watchdog driver.
Also, ast2500 has the "external reset" flag in SCU register 1e6e2000:0x3c.
That should be addressed at the same time, if at all.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: clock: qcom: Fix x1e80100 camcc power-domain declaration
From: Bryan O'Donoghue @ 2024-03-25 17:02 UTC (permalink / raw)
To: Rob Herring
Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
Krzysztof Kozlowski, Conor Dooley, Vladimir Zapolskiy, Abel Vesa,
Rajendra Nayak, linux-arm-msm, linux-clk, devicetree,
linux-kernel
In-Reply-To: <20240315151613.GA1256230-robh@kernel.org>
On 15/03/2024 15:16, Rob Herring wrote:
> Add new entries onto the end of existing ones. IOW, MMCX should always
> be 1st. Then you can move the descriptions to the top level and just put
> minItems or maxItems as appropriate here.
So for the CAMCC MXC should be switched on first per spec but, TBH I
don't think that's a real dependency that matters.
I can probably make this change with no functional impact.
---
bod
^ permalink raw reply
* Re: [PATCH v1 3/4] arm64: dts: rockchip: rk3588-rock5b: Enable GPU
From: Boris Brezillon @ 2024-03-25 17:01 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Heiko Stuebner, linux-rockchip, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-kernel, kernel
In-Reply-To: <a6uu7b3y7d4nirxbplc5cj4oeuyblx2grpvvldeovofhx3tnqc@dlse3vixhpws>
On Mon, 25 Mar 2024 16:58:47 +0100
Sebastian Reichel <sebastian.reichel@collabora.com> wrote:
> Hi,
>
> On Mon, Mar 25, 2024 at 04:44:41PM +0100, Boris Brezillon wrote:
> > On Mon, 25 Mar 2024 16:37:20 +0100
> > Sebastian Reichel <sebastian.reichel@collabora.com> wrote:
> >
> > > From: Boris Brezillon <boris.brezillon@collabora.com>
> > >
> > > Enable the Mali GPU in the Rock 5B.
> > >
> > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> >
> > I don't remember writing this patch ;-), maybe I screwed authorship at
> > some point, dunno.
>
> mh, I cherry-picked the DT patches from your branch before
> cleaning them up.
Yep, I probably wrote this patch and asked someone else to test. I
couldn't find any traces of this patch where I'm not flagged as the
author in any of my local branches.
>
> > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > > ---
> > > arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > > index 1fe8b2a0ed75..096ee7a98b89 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > > @@ -180,6 +180,11 @@ &cpu_l3 {
> > > cpu-supply = <&vdd_cpu_lit_s0>;
> > > };
> > >
> > > +&gpu {
> > > + mali-supply = <&vdd_gpu_s0>;
> > > + status = "okay";
> > > +};
> > > +
> > > &i2c0 {
> > > pinctrl-names = "default";
> > > pinctrl-0 = <&i2c0m2_xfer>;
> > > @@ -470,6 +475,7 @@ rk806_dvs3_null: dvs3-null-pins {
> > >
> > > regulators {
> > > vdd_gpu_s0: vdd_gpu_mem_s0: dcdc-reg1 {
> > > + regulator-always-on;
> >
> > Hm, should we mention why the regulator is always on here?
>
> In case of the EVB1 it's needed because the generic coupler driver
> cannot handle regulators that are not always on.
Ah, okay. I thought I added that for a different reason.
> I'm not sure why
> it was added for the Rock 5B. I will check if it works without that
> flag.
^ permalink raw reply
* Re: [PATCH v4 4/4] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
From: Mathieu Poirier @ 2024-03-25 16:51 UTC (permalink / raw)
To: Arnaud Pouliquen
Cc: Bjorn Andersson, Jens Wiklander, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-stm32, linux-arm-kernel, linux-remoteproc,
linux-kernel, op-tee, devicetree
In-Reply-To: <20240308144708.62362-5-arnaud.pouliquen@foss.st.com>
On Fri, Mar 08, 2024 at 03:47:08PM +0100, Arnaud Pouliquen wrote:
> The new TEE remoteproc device is used to manage remote firmware in a
> secure, trusted context. The 'st,stm32mp1-m4-tee' compatibility is
> introduced to delegate the loading of the firmware to the trusted
> execution context. In such cases, the firmware should be signed and
> adhere to the image format defined by the TEE.
>
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
> ---
> Updates from V3:
> - remove support of the attach use case. Will be addressed in a separate
> thread,
> - add st_rproc_tee_ops::parse_fw ops,
> - inverse call of devm_rproc_alloc()and tee_rproc_register() to manage cross
> reference between the rproc struct and the tee_rproc struct in tee_rproc.c.
> ---
> drivers/remoteproc/stm32_rproc.c | 60 +++++++++++++++++++++++++++++---
> 1 file changed, 56 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index 8cd838df4e92..13df33c78aa2 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -20,6 +20,7 @@
> #include <linux/remoteproc.h>
> #include <linux/reset.h>
> #include <linux/slab.h>
> +#include <linux/tee_remoteproc.h>
> #include <linux/workqueue.h>
>
> #include "remoteproc_internal.h"
> @@ -49,6 +50,9 @@
> #define M4_STATE_STANDBY 4
> #define M4_STATE_CRASH 5
>
> +/* Remote processor unique identifier aligned with the Trusted Execution Environment definitions */
Why is this the case? At least from the kernel side it is possible to call
tee_rproc_register() with any kind of value, why is there a need to be any
kind of alignment with the TEE?
> +#define STM32_MP1_M4_PROC_ID 0
> +
> struct stm32_syscon {
> struct regmap *map;
> u32 reg;
> @@ -257,6 +261,19 @@ static int stm32_rproc_release(struct rproc *rproc)
> return 0;
> }
>
> +static int stm32_rproc_tee_stop(struct rproc *rproc)
> +{
> + int err;
> +
> + stm32_rproc_request_shutdown(rproc);
> +
> + err = tee_rproc_stop(rproc);
> + if (err)
> + return err;
> +
> + return stm32_rproc_release(rproc);
> +}
> +
> static int stm32_rproc_prepare(struct rproc *rproc)
> {
> struct device *dev = rproc->dev.parent;
> @@ -693,8 +710,19 @@ static const struct rproc_ops st_rproc_ops = {
> .get_boot_addr = rproc_elf_get_boot_addr,
> };
>
> +static const struct rproc_ops st_rproc_tee_ops = {
> + .prepare = stm32_rproc_prepare,
> + .start = tee_rproc_start,
> + .stop = stm32_rproc_tee_stop,
> + .kick = stm32_rproc_kick,
> + .load = tee_rproc_load_fw,
> + .parse_fw = tee_rproc_parse_fw,
> + .find_loaded_rsc_table = tee_rproc_find_loaded_rsc_table,
> +};
> +
> static const struct of_device_id stm32_rproc_match[] = {
> - { .compatible = "st,stm32mp1-m4" },
> + {.compatible = "st,stm32mp1-m4",},
> + {.compatible = "st,stm32mp1-m4-tee",},
> {},
> };
> MODULE_DEVICE_TABLE(of, stm32_rproc_match);
> @@ -853,6 +881,7 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct stm32_rproc *ddata;
> struct device_node *np = dev->of_node;
> + struct tee_rproc *trproc = NULL;
> struct rproc *rproc;
> unsigned int state;
> int ret;
> @@ -861,9 +890,26 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - rproc = devm_rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
> - if (!rproc)
> - return -ENOMEM;
> + if (of_device_is_compatible(np, "st,stm32mp1-m4-tee")) {
> + /*
> + * Delegate the firmware management to the secure context.
> + * The firmware loaded has to be signed.
> + */
> + rproc = devm_rproc_alloc(dev, np->name, &st_rproc_tee_ops, NULL, sizeof(*ddata));
> + if (!rproc)
> + return -ENOMEM;
> +
> + trproc = tee_rproc_register(dev, rproc, STM32_MP1_M4_PROC_ID);
> + if (IS_ERR(trproc)) {
> + dev_err_probe(dev, PTR_ERR(trproc),
> + "signed firmware not supported by TEE\n");
> + return PTR_ERR(trproc);
> + }
> + } else {
> + rproc = devm_rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
> + if (!rproc)
> + return -ENOMEM;
> + }
>
> ddata = rproc->priv;
>
> @@ -915,6 +961,9 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> dev_pm_clear_wake_irq(dev);
> device_init_wakeup(dev, false);
> }
> + if (trproc)
if (rproc->tee_interface)
I am done reviewing this set.
Thanks,
Mathieu
> + tee_rproc_unregister(trproc);
> +
> return ret;
> }
>
> @@ -935,6 +984,9 @@ static void stm32_rproc_remove(struct platform_device *pdev)
> dev_pm_clear_wake_irq(dev);
> device_init_wakeup(dev, false);
> }
> + if (rproc->tee_interface)
> + tee_rproc_unregister(rproc->tee_interface);
> +
> }
>
> static int stm32_rproc_suspend(struct device *dev)
> --
> 2.25.1
>
^ permalink raw reply
* Re: [RFC PATCH 2/2] mfd: rohm-bd71828: Add power off functionality
From: Lee Jones @ 2024-03-25 16:51 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Andreas Kemnade, robh+dt, krzysztof.kozlowski+dt, conor+dt,
devicetree, linux-kernel, imx
In-Reply-To: <20240325141032.GO13211@google.com>
On Mon, 25 Mar 2024, Lee Jones wrote:
> On Mon, 25 Mar 2024, Matti Vaittinen wrote:
>
> > Hi Andreas,
> >
> > On 3/25/24 14:16, Andreas Kemnade wrote:
> > > On Mon, 25 Mar 2024 13:31:15 +0200
> > > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> > >
> > > > On 3/24/24 22:12, Andreas Kemnade wrote:
> > > > > Since the chip can power off the system, add the corresponding
> > > > > functionality.
> > > > > Based on https://github.com/kobolabs/Kobo-Reader/raw/master/hw/imx6sll-clara2e/kernel.tar.bz2
> > > > > No information source about the magic numbers found.
> > > >
> > > > Oh, interesting repository :) Thanks for linking to it! I didn't know
> > > > someone had reworked this driver...
> > > >
> > > which btw: contains this interesting snippet (output from fdtdump)
> > > bd71828-i2c@4b {
> > > reg = <0x0000004b>;
> > > compatible = "rohm,bd71828";
> > > gpio_int = <0x00000008 0x00000013 0x00000001>;
> > > gpio_wdogb = <0x00000039 0x00000018 0x00000001>;
> > > #address-cells = <0x00000001>;
> > > #size-cells = <0x00000000>;
> > > pmic@4b {
> > > compatible = "rohm,bd71828";
> > > regulators {
> > > BUCK1 {
> > > regulator-name = "buck1";
> > >
> > >
> > > and to make it work since basically no regulators are registered
> > > instead just some regmap_write()s are done to configure something
> > > in probe(). It is a pitfall to think that the information below pmic@4b
> > > is used, especially since it is not that obvious in the source.
>
> Odd! Not only did I not receive the original patch, I also did not
> receive your response Andreas. Spam is empty too.
>
> LORE too: https://lore.kernel.org/all/?q=%22mfd:%20rohm-bd71828:%20Add%20power%20off%20functionality%22
Super weird! They just all came through.
The LORE link above is now working too.
I suspect an issue with kernel.org.
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v4 3/4] remoteproc: stm32: Create sub-functions to request shutdown and release
From: Mathieu Poirier @ 2024-03-25 16:48 UTC (permalink / raw)
To: Arnaud Pouliquen
Cc: Bjorn Andersson, Jens Wiklander, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-stm32, linux-arm-kernel, linux-remoteproc,
linux-kernel, op-tee, devicetree
In-Reply-To: <20240308144708.62362-4-arnaud.pouliquen@foss.st.com>
On Fri, Mar 08, 2024 at 03:47:07PM +0100, Arnaud Pouliquen wrote:
> To prepare for the support of TEE remoteproc, create sub-functions
> that can be used in both cases, with and without TEE support.
>
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
> ---
> drivers/remoteproc/stm32_rproc.c | 84 +++++++++++++++++++-------------
> 1 file changed, 51 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index 88623df7d0c3..8cd838df4e92 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -209,6 +209,54 @@ static int stm32_rproc_mbox_idx(struct rproc *rproc, const unsigned char *name)
> return -EINVAL;
> }
>
> +static void stm32_rproc_request_shutdown(struct rproc *rproc)
> +{
> + struct stm32_rproc *ddata = rproc->priv;
> + int err, dummy_data, idx;
> +
> + /* Request shutdown of the remote processor */
> + if (rproc->state != RPROC_OFFLINE && rproc->state != RPROC_CRASHED) {
> + idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_SHUTDOWN);
> + if (idx >= 0 && ddata->mb[idx].chan) {
> + /* A dummy data is sent to allow to block on transmit. */
> + err = mbox_send_message(ddata->mb[idx].chan,
> + &dummy_data);
Why is this changed from the original implementation?
> + if (err < 0)
> + dev_warn(&rproc->dev, "warning: remote FW shutdown without ack\n");
> + }
> + }
> +}
> +
> +static int stm32_rproc_release(struct rproc *rproc)
> +{
> + struct stm32_rproc *ddata = rproc->priv;
> + unsigned int err = 0;
> +
> + /* To allow platform Standby power mode, set remote proc Deep Sleep. */
> + if (ddata->pdds.map) {
> + err = regmap_update_bits(ddata->pdds.map, ddata->pdds.reg,
> + ddata->pdds.mask, 1);
> + if (err) {
> + dev_err(&rproc->dev, "failed to set pdds\n");
> + return err;
> + }
> + }
> +
> + /* Update coprocessor state to OFF if available. */
> + if (ddata->m4_state.map) {
> + err = regmap_update_bits(ddata->m4_state.map,
> + ddata->m4_state.reg,
> + ddata->m4_state.mask,
> + M4_STATE_OFF);
> + if (err) {
> + dev_err(&rproc->dev, "failed to set copro state\n");
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int stm32_rproc_prepare(struct rproc *rproc)
> {
> struct device *dev = rproc->dev.parent;
> @@ -519,17 +567,9 @@ static int stm32_rproc_detach(struct rproc *rproc)
> static int stm32_rproc_stop(struct rproc *rproc)
> {
> struct stm32_rproc *ddata = rproc->priv;
> - int err, idx;
> + int err;
>
> - /* request shutdown of the remote processor */
> - if (rproc->state != RPROC_OFFLINE && rproc->state != RPROC_CRASHED) {
> - idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_SHUTDOWN);
> - if (idx >= 0 && ddata->mb[idx].chan) {
> - err = mbox_send_message(ddata->mb[idx].chan, "detach");
> - if (err < 0)
> - dev_warn(&rproc->dev, "warning: remote FW shutdown without ack\n");
> - }
> - }
> + stm32_rproc_request_shutdown(rproc);
>
> err = stm32_rproc_set_hold_boot(rproc, true);
> if (err)
> @@ -541,29 +581,7 @@ static int stm32_rproc_stop(struct rproc *rproc)
> return err;
> }
>
> - /* to allow platform Standby power mode, set remote proc Deep Sleep */
> - if (ddata->pdds.map) {
> - err = regmap_update_bits(ddata->pdds.map, ddata->pdds.reg,
> - ddata->pdds.mask, 1);
> - if (err) {
> - dev_err(&rproc->dev, "failed to set pdds\n");
> - return err;
> - }
> - }
> -
> - /* update coprocessor state to OFF if available */
> - if (ddata->m4_state.map) {
> - err = regmap_update_bits(ddata->m4_state.map,
> - ddata->m4_state.reg,
> - ddata->m4_state.mask,
> - M4_STATE_OFF);
> - if (err) {
> - dev_err(&rproc->dev, "failed to set copro state\n");
> - return err;
> - }
> - }
> -
> - return 0;
> + return stm32_rproc_release(rproc);
> }
>
> static void stm32_rproc_kick(struct rproc *rproc, int vqid)
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v4 1/4] remoteproc: Add TEE support
From: Mathieu Poirier @ 2024-03-25 16:46 UTC (permalink / raw)
To: Arnaud Pouliquen
Cc: Bjorn Andersson, Jens Wiklander, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-stm32, linux-arm-kernel, linux-remoteproc,
linux-kernel, op-tee, devicetree
In-Reply-To: <20240308144708.62362-2-arnaud.pouliquen@foss.st.com>
On Fri, Mar 08, 2024 at 03:47:05PM +0100, Arnaud Pouliquen wrote:
> Add a remoteproc TEE (Trusted Execution Environment) driver
> that will be probed by the TEE bus. If the associated Trusted
> application is supported on secure part this device offers a client
Device or driver? I thought I touched on that before.
> interface to load a firmware in the secure part.
> This firmware could be authenticated by the secure trusted application.
>
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
> ---
> Updates from V3:
> - rework TEE_REMOTEPROC description in Kconfig
> - fix some namings
> - add tee_rproc_parse_fw to support rproc_ops::parse_fw
> - add proc::tee_interface;
> - add rproc struct as parameter of the tee_rproc_register() function
> ---
> drivers/remoteproc/Kconfig | 10 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/tee_remoteproc.c | 434 ++++++++++++++++++++++++++++
> include/linux/remoteproc.h | 4 +
> include/linux/tee_remoteproc.h | 112 +++++++
> 5 files changed, 561 insertions(+)
> create mode 100644 drivers/remoteproc/tee_remoteproc.c
> create mode 100644 include/linux/tee_remoteproc.h
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 48845dc8fa85..2cf1431b2b59 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -365,6 +365,16 @@ config XLNX_R5_REMOTEPROC
>
> It's safe to say N if not interested in using RPU r5f cores.
>
> +
> +config TEE_REMOTEPROC
> + tristate "remoteproc support by a TEE application"
s/remoteproc/Remoteproc
> + depends on OPTEE
> + help
> + Support a remote processor with a TEE application. The Trusted
> + Execution Context is responsible for loading the trusted firmware
> + image and managing the remote processor's lifecycle.
> + This can be either built-in or a loadable module.
> +
> endif # REMOTEPROC
>
> endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 91314a9b43ce..fa8daebce277 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_RCAR_REMOTEPROC) += rcar_rproc.o
> obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
> obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
> obj-$(CONFIG_STM32_RPROC) += stm32_rproc.o
> +obj-$(CONFIG_TEE_REMOTEPROC) += tee_remoteproc.o
> obj-$(CONFIG_TI_K3_DSP_REMOTEPROC) += ti_k3_dsp_remoteproc.o
> obj-$(CONFIG_TI_K3_R5_REMOTEPROC) += ti_k3_r5_remoteproc.o
> obj-$(CONFIG_XLNX_R5_REMOTEPROC) += xlnx_r5_remoteproc.o
> diff --git a/drivers/remoteproc/tee_remoteproc.c b/drivers/remoteproc/tee_remoteproc.c
> new file mode 100644
> index 000000000000..c855210e52e3
> --- /dev/null
> +++ b/drivers/remoteproc/tee_remoteproc.c
> @@ -0,0 +1,434 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) STMicroelectronics 2024 - All Rights Reserved
> + * Author: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
> + */
> +
> +#include <linux/firmware.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/remoteproc.h>
> +#include <linux/slab.h>
> +#include <linux/tee_drv.h>
> +#include <linux/tee_remoteproc.h>
> +
> +#include "remoteproc_internal.h"
> +
> +#define MAX_TEE_PARAM_ARRY_MEMBER 4
> +
> +/*
> + * Authentication of the firmware and load in the remote processor memory
> + *
> + * [in] params[0].value.a: unique 32bit identifier of the remote processor
> + * [in] params[1].memref: buffer containing the image of the buffer
> + */
> +#define TA_RPROC_FW_CMD_LOAD_FW 1
> +
> +/*
> + * Start the remote processor
> + *
> + * [in] params[0].value.a: unique 32bit identifier of the remote processor
> + */
> +#define TA_RPROC_FW_CMD_START_FW 2
> +
> +/*
> + * Stop the remote processor
> + *
> + * [in] params[0].value.a: unique 32bit identifier of the remote processor
> + */
> +#define TA_RPROC_FW_CMD_STOP_FW 3
> +
> +/*
> + * Return the address of the resource table, or 0 if not found
> + * No check is done to verify that the address returned is accessible by
> + * the non secure context. If the resource table is loaded in a protected
> + * memory the access by the non secure context will lead to a data abort.
> + *
> + * [in] params[0].value.a: unique 32bit identifier of the remote processor
> + * [out] params[1].value.a: 32bit LSB resource table memory address
> + * [out] params[1].value.b: 32bit MSB resource table memory address
> + * [out] params[2].value.a: 32bit LSB resource table memory size
> + * [out] params[2].value.b: 32bit MSB resource table memory size
> + */
> +#define TA_RPROC_FW_CMD_GET_RSC_TABLE 4
> +
> +/*
> + * Return the address of the core dump
> + *
> + * [in] params[0].value.a: unique 32bit identifier of the remote processor
> + * [out] params[1].memref: address of the core dump image if exist,
> + * else return Null
> + */
> +#define TA_RPROC_FW_CMD_GET_COREDUMP 5
> +
> +struct tee_rproc_context {
> + struct list_head sessions;
> + struct tee_context *tee_ctx;
> + struct device *dev;
> +};
> +
> +static struct tee_rproc_context *tee_rproc_ctx;
> +
> +static void tee_rproc_prepare_args(struct tee_rproc *trproc, int cmd,
> + struct tee_ioctl_invoke_arg *arg,
> + struct tee_param *param,
> + unsigned int num_params)
> +{
> + memset(arg, 0, sizeof(*arg));
> + memset(param, 0, MAX_TEE_PARAM_ARRY_MEMBER * sizeof(*param));
> +
> + arg->func = cmd;
> + arg->session = trproc->session_id;
> + arg->num_params = num_params + 1;
> +
> + param[0] = (struct tee_param) {
> + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> + .u.value.a = trproc->rproc_id,
> + };
> +}
> +
> +int tee_rproc_load_fw(struct rproc *rproc, const struct firmware *fw)
> +{
> + struct tee_ioctl_invoke_arg arg;
> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
> + struct tee_rproc *trproc = rproc->tee_interface;
> + struct tee_shm *fw_shm;
> + int ret;
Declarations in reverse ascending order here and everywhere in the driver.
Sometimes it is done properly, sometimes it isn't.
> +
> + if (!trproc)
> + return -EINVAL;
> +
> + fw_shm = tee_shm_register_kernel_buf(tee_rproc_ctx->tee_ctx, (void *)fw->data, fw->size);
> + if (IS_ERR(fw_shm))
> + return PTR_ERR(fw_shm);
> +
> + tee_rproc_prepare_args(trproc, TA_RPROC_FW_CMD_LOAD_FW, &arg, param, 1);
> +
> + /* Provide the address of the firmware image */
> + param[1] = (struct tee_param) {
> + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT,
> + .u.memref = {
> + .shm = fw_shm,
> + .size = fw->size,
> + .shm_offs = 0,
> + },
> + };
> +
> + ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
> + if (ret < 0 || arg.ret != 0) {
> + dev_err(tee_rproc_ctx->dev,
> + "TA_RPROC_FW_CMD_LOAD_FW invoke failed TEE err: %x, ret:%x\n",
> + arg.ret, ret);
> + if (!ret)
> + ret = -EIO;
> + }
> +
> + tee_shm_free(fw_shm);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(tee_rproc_load_fw);
> +
> +struct resource_table *tee_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
> +{
> + struct tee_ioctl_invoke_arg arg;
> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
> + struct tee_rproc *trproc = rproc->tee_interface;
> + struct resource_table *rsc_table;
> + int ret;
> +
> + if (!trproc)
> + return ERR_PTR(-EINVAL);
> +
> + tee_rproc_prepare_args(trproc, TA_RPROC_FW_CMD_GET_RSC_TABLE, &arg, param, 2);
> +
> + param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
> + param[2].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
> +
> + ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
> + if (ret < 0 || arg.ret != 0) {
> + dev_err(tee_rproc_ctx->dev,
> + "TA_RPROC_FW_CMD_GET_RSC_TABLE invoke failed TEE err: %x, ret:%x\n",
> + arg.ret, ret);
> + return ERR_PTR(-EIO);
> + }
> +
> + *table_sz = param[2].u.value.a;
> +
> + /* If the size is null no resource table defined in the image */
> + if (!*table_sz)
> + return NULL;
> +
> + /* Store the resource table address that would be updated by the remote core. */
> + rsc_table = ioremap_wc(param[1].u.value.a, *table_sz);
> + if (IS_ERR_OR_NULL(rsc_table)) {
> + dev_err(tee_rproc_ctx->dev, "Unable to map memory region: %lld+%zx\n",
> + param[1].u.value.a, *table_sz);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + return rsc_table;
> +}
> +EXPORT_SYMBOL_GPL(tee_rproc_get_loaded_rsc_table);
> +
> +int tee_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
> +{
> + struct tee_rproc *trproc = rproc->tee_interface;
> + struct resource_table *rsc_table;
> + size_t table_sz;
> + int ret;
> +
> + ret = tee_rproc_load_fw(rproc, fw);
> + if (ret)
> + return ret;
> +
> + rsc_table = tee_rproc_get_loaded_rsc_table(rproc, &table_sz);
> + if (IS_ERR(rsc_table))
> + return PTR_ERR(rsc_table);
> +
> + /* Create a copy of the resource table to have same behaviour than the elf loader. */
> + rproc->cached_table = kmemdup(rsc_table, table_sz, GFP_KERNEL);
> + if (!rproc->cached_table)
> + return -ENOMEM;
Why not ->table_ptr and setting ->cached_table to NULL?
> +
> + rproc->table_ptr = rproc->cached_table;
> + rproc->table_sz = table_sz;
> + trproc->rsc_table = rsc_table;
I really don't see why this is needed, please remove and use rproc->table_ptr
instead.
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(tee_rproc_parse_fw);
> +
> +struct resource_table *tee_rproc_find_loaded_rsc_table(struct rproc *rproc,
> + const struct firmware *fw)
> +{
> + struct tee_rproc *trproc = rproc->tee_interface;
> + struct resource_table *rsc_table;
> + size_t table_sz;
> +
> + if (!trproc)
> + return ERR_PTR(-EINVAL);
> +
> + /* Check if the resourse table has already been obtained in tee_rproc_parse_fw() */
> + if (trproc->rsc_table)
> + return trproc->rsc_table;
Again, why not simply use rproc->rsc_table? This function should only return
the resource table that was set in tee_rproc_parse_fw().
> +
> + rsc_table = tee_rproc_get_loaded_rsc_table(rproc, &table_sz);
> + if (IS_ERR(rsc_table))
> + return rsc_table;
> +
> + rproc->table_sz = table_sz;
> + trproc->rsc_table = rsc_table;
> +
> + return rsc_table;
> +}
> +EXPORT_SYMBOL_GPL(tee_rproc_find_loaded_rsc_table);
> +
> +int tee_rproc_start(struct rproc *rproc)
> +{
> + struct tee_ioctl_invoke_arg arg;
> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
> + struct tee_rproc *trproc = rproc->tee_interface;
> + int ret;
> +
> + if (!trproc)
> + return -EINVAL;
> +
> + tee_rproc_prepare_args(trproc, TA_RPROC_FW_CMD_START_FW, &arg, param, 0);
> +
> + ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
> + if (ret < 0 || arg.ret != 0) {
> + dev_err(tee_rproc_ctx->dev,
> + "TA_RPROC_FW_CMD_START_FW invoke failed TEE err: %x, ret:%x\n",
> + arg.ret, ret);
> + if (!ret)
> + ret = -EIO;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(tee_rproc_start);
> +
> +int tee_rproc_stop(struct rproc *rproc)
> +{
> + struct tee_ioctl_invoke_arg arg;
> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
> + struct tee_rproc *trproc = rproc->tee_interface;
> + int ret;
> +
> + if (!trproc)
> + return -EINVAL;
> +
> + tee_rproc_prepare_args(trproc, TA_RPROC_FW_CMD_STOP_FW, &arg, param, 0);
> +
> + ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
> + if (ret < 0 || arg.ret != 0) {
> + dev_err(tee_rproc_ctx->dev,
> + "TA_RPROC_FW_CMD_STOP_FW invoke failed TEE err: %x, ret:%x\n",
> + arg.ret, ret);
> + if (!ret)
> + ret = -EIO;
> + }
> +
> + if (!rproc->table_ptr)
> + return ret;
> +
> + iounmap(trproc->rsc_table);
> + trproc->rsc_table = NULL;
> + rproc->table_ptr = NULL;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(tee_rproc_stop);
> +
> +static const struct tee_client_device_id stm32_tee_rproc_id_table[] = {
> + {UUID_INIT(0x80a4c275, 0x0a47, 0x4905,
> + 0x82, 0x85, 0x14, 0x86, 0xa9, 0x77, 0x1a, 0x08)},
> + {}
> +};
> +
> +struct tee_rproc *tee_rproc_register(struct device *dev, struct rproc *rproc, unsigned int rproc_id)
> +{
> + struct tee_client_device *tee_device;
> + struct tee_ioctl_open_session_arg sess_arg;
> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
> + struct tee_rproc *trproc;
> + int ret;
> +
> + /*
> + * Test if the device has been probed by the TEE bus. In case of failure, we ignore the
> + * reason. The bus could be not yet probed or the service not available in the secure
> + * firmware.The assumption in such a case is that the TEE remoteproc is not probed.
> + */
> + if (!tee_rproc_ctx)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + trproc = devm_kzalloc(dev, sizeof(*trproc), GFP_KERNEL);
> + if (!trproc)
> + return ERR_PTR(-ENOMEM);
> +
> + tee_device = to_tee_client_device(tee_rproc_ctx->dev);
> + memset(&sess_arg, 0, sizeof(sess_arg));
> +
> + memcpy(sess_arg.uuid, tee_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
> +
> + sess_arg.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL;
> + sess_arg.num_params = 1;
> +
> + param[0] = (struct tee_param) {
> + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> + .u.value.a = rproc_id,
> + };
> +
> + ret = tee_client_open_session(tee_rproc_ctx->tee_ctx, &sess_arg, param);
> + if (ret < 0 || sess_arg.ret != 0) {
> + dev_err(dev, "tee_client_open_session failed, err: %x\n", sess_arg.ret);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + trproc->parent = dev;
> + trproc->rproc_id = rproc_id;
> + trproc->session_id = sess_arg.session;
> +
> + trproc->rproc = rproc;
> + rproc->tee_interface = trproc;
> +
> + list_add_tail(&trproc->node, &tee_rproc_ctx->sessions);
> +
> + return trproc;
Once this function was called by a client, what prevents a user from unloading
the tee_remoteproc module and breaking everything?
> +}
> +EXPORT_SYMBOL_GPL(tee_rproc_register);
> +
> +int tee_rproc_unregister(struct tee_rproc *trproc)
> +{
If you pass a struct_rproc instead of a struct tee_rproc there is no need for
tee_rproc::rproc, which is only ever used in this function.
> + struct rproc *rproc = trproc->rproc;
> + int ret;
> +
> + ret = tee_client_close_session(tee_rproc_ctx->tee_ctx, trproc->session_id);
> + if (ret < 0)
> + dev_err(trproc->parent, "tee_client_close_session failed, err: %x\n", ret);
> +
> + list_del(&trproc->node);
> + rproc->tee_interface = NULL;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(tee_rproc_unregister);
> +
> +static int tee_rproc_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> +{
> + /* Today we support only the OP-TEE, could be extend to other tees */
> + return (ver->impl_id == TEE_IMPL_ID_OPTEE);
> +}
> +
> +static int tee_rproc_probe(struct device *dev)
> +{
> + struct tee_context *tee_ctx;
> + int ret;
> +
> + /* Open context with TEE driver */
> + tee_ctx = tee_client_open_context(NULL, tee_rproc_ctx_match, NULL, NULL);
> + if (IS_ERR(tee_ctx))
> + return PTR_ERR(tee_ctx);
> +
> + tee_rproc_ctx = devm_kzalloc(dev, sizeof(*tee_ctx), GFP_KERNEL);
> + if (!tee_rproc_ctx) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + tee_rproc_ctx->dev = dev;
> + tee_rproc_ctx->tee_ctx = tee_ctx;
> + INIT_LIST_HEAD(&tee_rproc_ctx->sessions);
> +
> + return 0;
> +err:
> + tee_client_close_context(tee_ctx);
> +
> + return ret;
> +}
> +
> +static int tee_rproc_remove(struct device *dev)
> +{
> + struct tee_rproc *entry, *tmp;
> +
> + list_for_each_entry_safe(entry, tmp, &tee_rproc_ctx->sessions, node) {
> + tee_client_close_session(tee_rproc_ctx->tee_ctx, entry->session_id);
> + list_del(&entry->node);
> + if (entry->rsc_table)
> + iounmap(entry->rsc_table);
> + kfree(entry);
> + }
> +
> + tee_client_close_context(tee_rproc_ctx->tee_ctx);
> +
> + return 0;
> +}
> +
> +MODULE_DEVICE_TABLE(tee, stm32_tee_rproc_id_table);
> +
> +static struct tee_client_driver tee_rproc_fw_driver = {
> + .id_table = stm32_tee_rproc_id_table,
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .bus = &tee_bus_type,
> + .probe = tee_rproc_probe,
> + .remove = tee_rproc_remove,
> + },
> +};
> +
> +static int __init tee_rproc_fw_mod_init(void)
> +{
> + return driver_register(&tee_rproc_fw_driver.driver);
> +}
> +
> +static void __exit tee_rproc_fw_mod_exit(void)
> +{
> + driver_unregister(&tee_rproc_fw_driver.driver);
> +}
> +
> +module_init(tee_rproc_fw_mod_init);
> +module_exit(tee_rproc_fw_mod_exit);
> +
> +MODULE_DESCRIPTION(" TEE remote processor control driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index b4795698d8c2..8b678009e481 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -503,6 +503,8 @@ enum rproc_features {
> RPROC_MAX_FEATURES,
> };
>
> +struct tee_rproc;
> +
> /**
> * struct rproc - represents a physical remote processor device
> * @node: list node of this rproc object
> @@ -545,6 +547,7 @@ enum rproc_features {
> * @cdev: character device of the rproc
> * @cdev_put_on_release: flag to indicate if remoteproc should be shutdown on @char_dev release
> * @features: indicate remoteproc features
> + * @tee_interface: pointer to the remoteproc tee context
> */
> struct rproc {
> struct list_head node;
> @@ -586,6 +589,7 @@ struct rproc {
> struct cdev cdev;
> bool cdev_put_on_release;
> DECLARE_BITMAP(features, RPROC_MAX_FEATURES);
> + struct tee_rproc *tee_interface;
> };
>
> /**
> diff --git a/include/linux/tee_remoteproc.h b/include/linux/tee_remoteproc.h
> new file mode 100644
> index 000000000000..571e47190d02
> --- /dev/null
> +++ b/include/linux/tee_remoteproc.h
> @@ -0,0 +1,112 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright(c) 2024 STMicroelectronics - All Rights Reserved
> + */
> +
> +#ifndef TEE_REMOTEPROC_H
> +#define TEE_REMOTEPROC_H
> +
> +#include <linux/tee_drv.h>
> +#include <linux/firmware.h>
> +#include <linux/remoteproc.h>
> +
> +struct rproc;
> +
> +/**
> + * struct tee_rproc - TEE remoteproc structure
> + * @node: Reference in list
> + * @rproc: Remoteproc reference
> + * @parent: Parent device
> + * @rproc_id: Identifier of the target firmware
> + * @session_id: TEE session identifier
> + * @rsc_table: Resource table virtual address.
> + */
> +struct tee_rproc {
> + struct list_head node;
> + struct rproc *rproc;
> + struct device *parent;
> + u32 rproc_id;
> + u32 session_id;
> + struct resource_table *rsc_table;
> +};
> +
> +#if IS_REACHABLE(CONFIG_TEE_REMOTEPROC)
> +
> +struct tee_rproc *tee_rproc_register(struct device *dev, struct rproc *rproc,
> + unsigned int rproc_id);
> +int tee_rproc_unregister(struct tee_rproc *trproc);
> +int tee_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw);
> +int tee_rproc_load_fw(struct rproc *rproc, const struct firmware *fw);
> +struct resource_table *tee_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz);
> +struct resource_table *tee_rproc_find_loaded_rsc_table(struct rproc *rproc,
> + const struct firmware *fw);
> +int tee_rproc_start(struct rproc *rproc);
> +int tee_rproc_stop(struct rproc *rproc);
> +
> +#else
> +
> +static inline struct tee_rproc *tee_rproc_register(struct device *dev, struct rproc *rproc,
> + unsigned int rproc_id)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> +static int tee_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
> +{
> + /* This shouldn't be possible */
> + WARN_ON(1);
> +
> + return 0;
> +}
> +
> +static inline int tee_rproc_unregister(struct tee_rproc *trproc)
> +{
> + /* This shouldn't be possible */
> + WARN_ON(1);
> +
> + return 0;
> +}
> +
> +static inline int tee_rproc_load_fw(struct rproc *rproc, const struct firmware *fw)
> +{
> + /* This shouldn't be possible */
> + WARN_ON(1);
> +
> + return 0;
> +}
> +
> +static inline int tee_rproc_start(struct rproc *rproc)
> +{
> + /* This shouldn't be possible */
> + WARN_ON(1);
> +
> + return 0;
> +}
> +
> +static inline int tee_rproc_stop(struct rproc *rproc)
> +{
> + /* This shouldn't be possible */
> + WARN_ON(1);
> +
> + return 0;
> +}
> +
> +static inline struct resource_table *
> +tee_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
> +{
> + /* This shouldn't be possible */
> + WARN_ON(1);
> +
> + return NULL;
> +}
> +
> +static inline struct resource_table *
> +tee_rproc_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw)
> +{
> + /* This shouldn't be possible */
> + WARN_ON(1);
> +
> + return NULL;
> +}
> +#endif /* CONFIG_TEE_REMOTEPROC */
> +#endif /* TEE_REMOTEPROC_H */
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v6 1/2] driver core: Introduce device_link_wait_removal()
From: Rob Herring @ 2024-03-25 16:44 UTC (permalink / raw)
To: Herve Codina
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Frank Rowand,
Saravana Kannan, Lizhi Hou, Max Zhen, Sonal Santan,
Stefano Stabellini, Jonathan Cameron, linux-kernel, devicetree,
Allan Nielsen, Horatiu Vultur, Steen Hegelund, Luca Ceresoli,
Nuno Sa, Thomas Petazzoni, stable
In-Reply-To: <20240325152140.198219-2-herve.codina@bootlin.com>
On Mon, Mar 25, 2024 at 04:21:25PM +0100, Herve Codina wrote:
> The commit 80dd33cf72d1 ("drivers: base: Fix device link removal")
> introduces a workqueue to release the consumer and supplier devices used
> in the devlink.
> In the job queued, devices are release and in turn, when all the
> references to these devices are dropped, the release function of the
> device itself is called.
>
> Nothing is present to provide some synchronisation with this workqueue
> in order to ensure that all ongoing releasing operations are done and
> so, some other operations can be started safely.
>
> For instance, in the following sequence:
> 1) of_platform_depopulate()
> 2) of_overlay_remove()
>
> During the step 1, devices are released and related devlinks are removed
> (jobs pushed in the workqueue).
> During the step 2, OF nodes are destroyed but, without any
> synchronisation with devlink removal jobs, of_overlay_remove() can raise
> warnings related to missing of_node_put():
> ERROR: memory leak, expected refcount 1 instead of 2
>
> Indeed, the missing of_node_put() call is going to be done, too late,
> from the workqueue job execution.
>
> Introduce device_link_wait_removal() to offer a way to synchronize
> operations waiting for the end of devlink removals (i.e. end of
> workqueue jobs).
> Also, as a flushing operation is done on the workqueue, the workqueue
> used is moved from a system-wide workqueue to a local one.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> Reviewed-by: Saravana Kannan <saravanak@google.com>
> ---
> drivers/base/core.c | 26 +++++++++++++++++++++++---
> include/linux/device.h | 1 +
> 2 files changed, 24 insertions(+), 3 deletions(-)
Greg, can you ack and I'll take this series.
Rob
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: mfd: twl: Convert trivial subdevices to json-schema
From: Guenter Roeck @ 2024-03-25 16:42 UTC (permalink / raw)
To: Andreas Kemnade, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, lee, alexandre.belloni, wim, linux-input, devicetree,
linux-kernel, linux-rtc, linux-watchdog, linux-omap, sre
In-Reply-To: <20240324183727.206384-1-andreas@kemnade.info>
On 3/24/24 11:37, Andreas Kemnade wrote:
> Convert subdevices with just an interrupt and compatbile to
> json-schema and wire up already converted subdevices.
> RTC is available in all variants, so allow it unconditionally.
> GPADC binding for TWL603X uses two different compatibles, so
> specify just the compatible and do not include it.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
For watchdog:
Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Changes in v2:
> - style cleanup
> - absolute paths
> - unevalutedProperties instead of additionalProperties
> due to not accepting things in if: clauses without it
>
> .../bindings/input/twl4030-pwrbutton.txt | 21 ------
> .../devicetree/bindings/mfd/ti,twl.yaml | 72 ++++++++++++++++++-
> .../devicetree/bindings/rtc/twl-rtc.txt | 11 ---
> .../bindings/watchdog/twl4030-wdt.txt | 10 ---
> 4 files changed, 71 insertions(+), 43 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> delete mode 100644 Documentation/devicetree/bindings/rtc/twl-rtc.txt
> delete mode 100644 Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt
>
> diff --git a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> deleted file mode 100644
> index 6c201a2ba8acf..0000000000000
> --- a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -Texas Instruments TWL family (twl4030) pwrbutton module
> -
> -This module is part of the TWL4030. For more details about the whole
> -chip see Documentation/devicetree/bindings/mfd/ti,twl.yaml.
> -
> -This module provides a simple power button event via an Interrupt.
> -
> -Required properties:
> -- compatible: should be one of the following
> - - "ti,twl4030-pwrbutton": For controllers compatible with twl4030
> -- interrupts: should be one of the following
> - - <8>: For controllers compatible with twl4030
> -
> -Example:
> -
> -&twl {
> - twl_pwrbutton: pwrbutton {
> - compatible = "ti,twl4030-pwrbutton";
> - interrupts = <8>;
> - };
> -};
> diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> index 52ed228fb1e7e..c2357fecb56cc 100644
> --- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> @@ -15,6 +15,67 @@ description: |
> USB transceiver or Audio amplifier.
> These chips are connected to an i2c bus.
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: ti,twl4030
> + then:
> + properties:
> + madc:
> + type: object
> + $ref: /schemas/iio/adc/ti,twl4030-madc.yaml
> + unevaluatedProperties: false
> +
> + bci:
> + type: object
> + $ref: /schemas/power/supply/twl4030-charger.yaml
> + unevaluatedProperties: false
> +
> + pwrbutton:
> + type: object
> + additionalProperties: false
> + properties:
> + compatible:
> + const: ti,twl4030-pwrbutton
> + interrupts:
> + items:
> + - items:
> + const: 8
> +
> + watchdog:
> + type: object
> + additionalProperties: false
> + properties:
> + compatible:
> + const: ti,twl4030-wdt
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: ti,twl6030
> + then:
> + properties:
> + gpadc:
> + type: object
> + properties:
> + compatible:
> + const: ti,twl6030-gpadc
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: ti,twl6032
> + then:
> + properties:
> + gpadc:
> + type: object
> + properties:
> + compatible:
> + const: ti,twl6032-gpadc
> +
> properties:
> compatible:
> description:
> @@ -42,7 +103,16 @@ properties:
> "#clock-cells":
> const: 1
>
> -additionalProperties: false
> + rtc:
> + type: object
> + additionalProperties: false
> + properties:
> + compatible:
> + const: ti,twl4030-rtc
> + interrupts:
> + maxItems: 1
> +
> +unevaluatedProperties: false
>
> required:
> - compatible
> diff --git a/Documentation/devicetree/bindings/rtc/twl-rtc.txt b/Documentation/devicetree/bindings/rtc/twl-rtc.txt
> deleted file mode 100644
> index 8f9a94f2f8969..0000000000000
> --- a/Documentation/devicetree/bindings/rtc/twl-rtc.txt
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -* Texas Instruments TWL4030/6030 RTC
> -
> -Required properties:
> -- compatible : Should be "ti,twl4030-rtc"
> -- interrupts : Should be the interrupt number.
> -
> -Example:
> - rtc {
> - compatible = "ti,twl4030-rtc";
> - interrupts = <11>;
> - };
> diff --git a/Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt b/Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt
> deleted file mode 100644
> index 80a37193c0b86..0000000000000
> --- a/Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt
> +++ /dev/null
> @@ -1,10 +0,0 @@
> -Device tree bindings for twl4030-wdt driver (TWL4030 watchdog)
> -
> -Required properties:
> - compatible = "ti,twl4030-wdt";
> -
> -Example:
> -
> -watchdog {
> - compatible = "ti,twl4030-wdt";
> -};
^ permalink raw reply
* Re: [PATCH v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
From: Bartosz Golaszewski @ 2024-03-25 16:23 UTC (permalink / raw)
To: Kalle Valo
Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Liam Girdwood, Mark Brown, Catalin Marinas, Will Deacon,
Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven, Arnd Bergmann,
Neil Armstrong, Marek Szyprowski, Alex Elder, Srini Kandagatla,
Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
Lukas Wunner, Dmitry Baryshkov, linux-bluetooth, netdev,
devicetree, linux-kernel, linux-wireless, linux-arm-msm,
linux-arm-kernel, linux-pci, linux-pm, Bartosz Golaszewski,
ath11k, Johan Hovold
In-Reply-To: <87frwe8jiu.fsf@kernel.org>
On Mon, Mar 25, 2024 at 3:37 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> > On Mon, Mar 25, 2024 at 2:57 PM Kalle Valo <kvalo@kernel.org> wrote:
> >
> >>
> >> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> >>
> >> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >> >
> >> > Add a PCI compatible for the ATH11K module on QCA6390 and describe the
> >> > power inputs from the PMU that it consumes.
> >> >
> >> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >>
> >> [...]
> >>
> >> > +allOf:
> >> > + - if:
> >> > + properties:
> >> > + compatible:
> >> > + contains:
> >> > + const: pci17cb,1101
> >> > + then:
> >> > + required:
> >> > + - vddrfacmn-supply
> >> > + - vddaon-supply
> >> > + - vddwlcx-supply
> >> > + - vddwlmx-supply
> >> > + - vddrfa0p8-supply
> >> > + - vddrfa1p2-supply
> >> > + - vddrfa1p7-supply
> >> > + - vddpcie0p9-supply
> >> > + - vddpcie1p8-supply
> >>
> >> I don't know DT well enough to know what the "required:" above means,
> >> but does this take into account that there are normal "plug&play" type
> >> of QCA6390 boards as well which don't need any DT settings?
> >
> > Do they require a DT node though for some reason?
>
> You can attach the device to any PCI slot, connect the WLAN antenna and
> it just works without DT nodes. I'm trying to make sure here that basic
> setup still works.
>
Sure, definitely. I there's no DT node, then the binding doesn't apply
and the driver (the platform part of it) will not probe.
> Adding also Johan and ath11k list. For example, I don't know what's the
> plan with Lenovo X13s, will it use this framework? I guess in theory we
> could have devices which use qcom,ath11k-calibration-variant from DT but
> not any of these supply properties?
>
Good point. I will receive the X13s in a month from now. I do plan on
upstreaming correct support for WLAN and BT for it as well.
I guess we can always relax the requirements once a valid use-case appears?
Bart
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH v3 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support
From: Rob Herring @ 2024-03-25 16:22 UTC (permalink / raw)
To: Tan Chun Hau
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Emil Renner Berthing, Krzysztof Kozlowski, Conor Dooley,
Maxime Coquelin, Alexandre Torgue, Simon Horman,
Bartosz Golaszewski, Andrew Halaney, Jisheng Zhang,
Uwe Kleine-König, Russell King, Ley Foon Tan, Jee Heng Sia,
netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel,
linux-riscv
In-Reply-To: <20240325085131.182657-2-chunhau.tan@starfivetech.com>
On Mon, Mar 25, 2024 at 01:51:31AM -0700, Tan Chun Hau wrote:
> Add StarFive JH8100 dwmac support.
> The JH8100 dwmac shares the same driver code as the JH7110 dwmac
> and has only one reset signal.
>
> Please refer to below:
>
> JH8100: reset-names = "stmmaceth";
> JH7110: reset-names = "stmmaceth", "ahb";
It's debatable whether JH8100 is compatible with JH7110 if the 2nd reset
was not optional. I guess if the Linux driver treated it that way, we
can get away with it. It would simplify the conditionals in the schema
if the schema also treated the 2nd entry as optional on JH7110 as well.
> JH7100: reset-names = "ahb";
>
> Example usage of JH8100 in the device tree:
>
> gmac0: ethernet@16030000 {
> compatible = "starfive,jh8100-dwmac",
> "starfive,jh7110-dwmac",
> "snps,dwmac-5.20";
> ...
> };
>
> Signed-off-by: Tan Chun Hau <chunhau.tan@starfivetech.com>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> .../bindings/net/starfive,jh7110-dwmac.yaml | 82 +++++++++++++------
> 2 files changed, 58 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 6b0341a8e0ea..a6d596b7dcf4 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -97,6 +97,7 @@ properties:
> - snps,dwxgmac-2.10
> - starfive,jh7100-dwmac
> - starfive,jh7110-dwmac
> + - starfive,jh8100-dwmac
>
> reg:
> minItems: 1
> diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> index 0d1962980f57..da3cc984fec9 100644
> --- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> @@ -18,6 +18,7 @@ select:
> enum:
> - starfive,jh7100-dwmac
> - starfive,jh7110-dwmac
> + - starfive,jh8100-dwmac
> required:
> - compatible
>
> @@ -30,6 +31,10 @@ properties:
> - items:
> - const: starfive,jh7110-dwmac
> - const: snps,dwmac-5.20
> + - items:
> + - const: starfive,jh8100-dwmac
> + - const: starfive,jh7110-dwmac
> + - const: snps,dwmac-5.20
>
> reg:
> maxItems: 1
> @@ -83,29 +88,13 @@ allOf:
> - if:
> properties:
> compatible:
> - contains:
> - const: starfive,jh7100-dwmac
> - then:
> - properties:
> - interrupts:
> - minItems: 2
> - maxItems: 2
> -
> - interrupt-names:
> - minItems: 2
> - maxItems: 2
> -
> - resets:
> - maxItems: 1
> -
> - reset-names:
> - const: ahb
> -
> - - if:
> - properties:
> - compatible:
> - contains:
> - const: starfive,jh7110-dwmac
> + allOf:
> + - contains:
> + enum:
> + - starfive,jh8100-dwmac
> + - contains:
> + enum:
> + - starfive,jh7110-dwmac
There's no need for the 2nd entry. You just need to check
I would something like this structure:
- if:
properties:
compatible:
contains:
const: starfive,jh7100-dwmac
then:
if:
properties:
compatible:
contains:
const: starfive,jh8100-dwmac
then:
...
else:
...
> then:
> properties:
> interrupts:
> @@ -117,10 +106,53 @@ allOf:
> maxItems: 3
>
> resets:
> - minItems: 2
> + maxItems: 1
>
> reset-names:
> - minItems: 2
> + const: stmmaceth
> +
> + else:
I don't think you need the else. Just do another 'if' entry.
> + if:
> + properties:
> + compatible:
> + contains:
> + const: starfive,jh7100-dwmac
> + then:
> + properties:
> + interrupts:
> + minItems: 2
> + maxItems: 2
> +
> + interrupt-names:
> + minItems: 2
> + maxItems: 2
> +
> + resets:
> + maxItems: 1
> +
> + reset-names:
> + const: ahb
> +
> + if:
> + properties:
> + compatible:
> + contains:
> + const: starfive,jh7110-dwmac
> + then:
> + properties:
> + interrupts:
> + minItems: 3
> + maxItems: 3
> +
> + interrupt-names:
> + minItems: 3
> + maxItems: 3
> +
> + resets:
> + minItems: 2
> +
> + reset-names:
> + minItems: 2
>
> unevaluatedProperties: false
>
> --
> 2.25.1
>
^ permalink raw reply
* RE: [PATCH v4 2/5] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'
From: Biju Das @ 2024-03-25 16:21 UTC (permalink / raw)
To: Prabhakar, Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Fabrizio Castro, Prabhakar Mahadev Lad
In-Reply-To: <20240322144355.878930-3-prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi Prabhakar,
> -----Original Message-----
> From: Prabhakar <prabhakar.csengg@gmail.com>
> Sent: Friday, March 22, 2024 2:44 PM
> Subject: [PATCH v4 2/5] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-
> names'
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> This commit adds support to validate the 'interrupts' and 'interrupt-names'
> properties for every supported SoC. This ensures proper handling and configuration of interrupt-
> related properties across supported platforms.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v3->v4
> - Reverted back to v2 version of the patch.
> - Used suggestion from Krzysztof for interrupts
> - Restored RB tag from Geert
>
> v2->v3
> - Listed interrupts and interrupt-names for every SoC in if check
> ---
> .../bindings/serial/renesas,scif.yaml | 73 ++++++++++++++-----
> 1 file changed, 55 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> index af72c3420453..eb2aa5e75e02 100644
> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> @@ -86,11 +86,6 @@ properties:
> oneOf:
> - items:
> - description: A combined interrupt
> - - items:
> - - description: Error interrupt
> - - description: Receive buffer full interrupt
> - - description: Transmit buffer empty interrupt
> - - description: Break interrupt
> - items:
> - description: Error interrupt
> - description: Receive buffer full interrupt @@ -98,21 +93,17 @@ properties:
> - description: Break interrupt
> - description: Data Ready interrupt
> - description: Transmit End interrupt
> + minItems: 4
I think here minItems is 1 as it is either 1 or 4 or 6
Cheers,
Biju
>
> interrupt-names:
> - oneOf:
> - - items:
> - - const: eri
> - - const: rxi
> - - const: txi
> - - const: bri
> - - items:
> - - const: eri
> - - const: rxi
> - - const: txi
> - - const: bri
> - - const: dri
> - - const: tei
> + minItems: 4
> + items:
> + - const: eri
> + - const: rxi
> + - const: txi
> + - const: bri
> + - const: dri
> + - const: tei
>
> clocks:
> minItems: 1
> @@ -173,6 +164,52 @@ allOf:
> required:
> - resets
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - renesas,rcar-gen1-scif
> + - renesas,rcar-gen2-scif
> + - renesas,rcar-gen3-scif
> + - renesas,rcar-gen4-scif
> + then:
> + properties:
> + interrupts:
> + maxItems: 1
> +
> + interrupt-names: false
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - renesas,scif-r7s72100
> + then:
> + properties:
> + interrupts:
> + minItems: 4
> + maxItems: 4
> +
> + interrupt-names:
> + maxItems: 4
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - renesas,scif-r7s9210
> + - renesas,scif-r9a07g044
> + then:
> + properties:
> + interrupts:
> + minItems: 6
> +
> + interrupt-names:
> + minItems: 6
> +
> unevaluatedProperties: false
>
> examples:
> --
> 2.34.1
>
^ 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