Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v5 05/27] iommu/arm-smmu-v3: Make CD programming use arm_smmu_write_entry()
From: Jason Gunthorpe @ 2024-03-26 18:30 UTC (permalink / raw)
  To: Mostafa Saleh
  Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
	Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
	Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <Zf7S109OKJlRVXiR@google.com>

On Sat, Mar 23, 2024 at 01:02:15PM +0000, Mostafa Saleh wrote:
> > +static void arm_smmu_get_cd_used(const __le64 *ent, __le64 *used_bits)
> > +{
> > +	used_bits[0] = cpu_to_le64(CTXDESC_CD_0_V);
> > +	if (!(ent[0] & cpu_to_le64(CTXDESC_CD_0_V)))
> > +		return;
> > +	memset(used_bits, 0xFF, sizeof(struct arm_smmu_cd));
> 
> This is a slightly different approach than what the driver does for STEs,
> where it explicitly sets the used bits. Is there a reason for that?

It is just more compact this way

> > +	/* EPD0 means T0SZ/TG0/IR0/OR0/SH0/TTB0 are IGNORED */
> > +	if (ent[0] & cpu_to_le64(CTXDESC_CD_0_TCR_EPD0)) {
> > +		used_bits[0] &= ~cpu_to_le64(
> > +			CTXDESC_CD_0_TCR_T0SZ | CTXDESC_CD_0_TCR_TG0 |
> > +			CTXDESC_CD_0_TCR_IRGN0 | CTXDESC_CD_0_TCR_ORGN0 |
> > +			CTXDESC_CD_0_TCR_SH0);
> > +		used_bits[1] &= ~cpu_to_le64(CTXDESC_CD_1_TTB0_MASK);
> > +	}
> > +}
> 
> We should add a comment about EPD1 maybe?

Driver doesn't use TTB1?

Jason

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 1/4] dt-bindings: rtc: armada-380-rtc: convert to dtschema
From: Rob Herring @ 2024-03-26 18:47 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: linux-arm-kernel, linux-kernel, Alexandre Belloni,
	Krzysztof Kozlowski, devicetree, linux-rtc, Conor Dooley,
	Baruch Siach
In-Reply-To: <20240325-rtc-yaml-v2-1-ff9f68f43dbc@gmail.com>


On Mon, 25 Mar 2024 22:10:08 +0100, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
> 
> This is a direct conversion with no additions.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  .../devicetree/bindings/rtc/armada-380-rtc.txt     | 24 ----------
>  .../bindings/rtc/marvell,armada-380-rtc.yaml       | 51 ++++++++++++++++++++++
>  2 files changed, 51 insertions(+), 24 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 2/4] dt-bindings: rtc: alphascale,asm9260-rtc: convert to dtschema
From: Rob Herring @ 2024-03-26 18:47 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: linux-rtc, devicetree, Baruch Siach, Conor Dooley,
	Krzysztof Kozlowski, linux-kernel, linux-arm-kernel,
	Alexandre Belloni
In-Reply-To: <20240325-rtc-yaml-v2-2-ff9f68f43dbc@gmail.com>


On Mon, 25 Mar 2024 22:10:09 +0100, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
> 
> This is a direct conversion with no additions.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  .../bindings/rtc/alphascale,asm9260-rtc.txt        | 19 --------
>  .../bindings/rtc/alphascale,asm9260-rtc.yaml       | 52 ++++++++++++++++++++++
>  2 files changed, 52 insertions(+), 19 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 3/4] dt-bindings: rtc: digicolor-rtc: move to trivial-rtc
From: Rob Herring @ 2024-03-26 18:47 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: linux-arm-kernel, Conor Dooley, devicetree, Baruch Siach,
	linux-kernel, Krzysztof Kozlowski, Alexandre Belloni, linux-rtc
In-Reply-To: <20240325-rtc-yaml-v2-3-ff9f68f43dbc@gmail.com>


On Mon, 25 Mar 2024 22:10:10 +0100, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
> 
> This device meets the requirements to be moved to trivial-rtc
> (compatible, reg and a single interrupt).
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  Documentation/devicetree/bindings/rtc/digicolor-rtc.txt | 17 -----------------
>  Documentation/devicetree/bindings/rtc/trivial-rtc.yaml  |  2 ++
>  2 files changed, 2 insertions(+), 17 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 4/4] dt-bindings: rtc: nxp,lpc1788-rtc: convert to dtschema
From: Rob Herring @ 2024-03-26 18:47 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: Conor Dooley, Baruch Siach, Krzysztof Kozlowski, linux-arm-kernel,
	devicetree, linux-kernel, linux-rtc, Alexandre Belloni
In-Reply-To: <20240325-rtc-yaml-v2-4-ff9f68f43dbc@gmail.com>


On Mon, 25 Mar 2024 22:10:11 +0100, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
> 
> This is a direct conversion with no additions.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  .../devicetree/bindings/rtc/nxp,lpc1788-rtc.txt    | 21 --------
>  .../devicetree/bindings/rtc/nxp,lpc1788-rtc.yaml   | 60 ++++++++++++++++++++++
>  2 files changed, 60 insertions(+), 21 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Mark Brown @ 2024-03-26 18:49 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <ZgMXe4EMbJ44W1tr@smile.fi.intel.com>


[-- Attachment #1.1: Type: text/plain, Size: 844 bytes --]

On Tue, Mar 26, 2024 at 08:44:11PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 26, 2024 at 06:25:16PM +0000, Mark Brown wrote:
> > On Tue, Mar 26, 2024 at 08:22:59PM +0200, Andy Shevchenko wrote:
> > > On Tue, Mar 26, 2024 at 06:16:28PM +0000, Mark Brown wrote:

> > > > I think the ACPI dependency there is as much about hiding the device on
> > > > irrelevant platforms as anything else, might be better replaced with an
> > > > x86 dependency though.

...

> > That's nice but I'm not sure what that has to do with the dependency on
> > ACPI?

> ACPI_PTR() makes ID no-op only if ACPI is defined. That also satisfies
> the ugly ifdeffery that is removed by this patch as well. If there is
> no dependency we will have compiler warning for defined but not used
> variable.

Again I don't think that's the main purpose of the dependency here.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 07/10] spi: pxa2xx: Provide num-cs for Sharp PDAs via device properties
From: Andy Shevchenko @ 2024-03-26 18:50 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <dcdf8c46-acdc-466d-afc6-caf0e0fa39e8@sirena.org.uk>

On Tue, Mar 26, 2024 at 06:21:48PM +0000, Mark Brown wrote:
> On Tue, Mar 26, 2024 at 08:07:57PM +0200, Andy Shevchenko wrote:
> 
> > Since driver can parse num-cs device property, replace platform data
> > with this new approach.
> 
> But why?

To be able to hide the header's contents from public.
Should I update the commit message?

...

> > +static const struct property_entry spitz_spi_properties[] = {
> > +	PROPERTY_ENTRY_U32("num-cs", 3),
> > +	{ }
> > +};
> 
> This is just platform data with less validation AFAICT.

I'm not sure what validation you are expecting here. It should be done via
DT schema ideally when the platform gets converted to DT. This change is
an interim to that (at least it makes kernel side better). After the platform
code may be gone completely or converted. If the latter happens, we got
the validation back.

In any case it's not worse than plain DT property handling in the kernel.
The validation in that case is done elsewhere. Since the property is defined
in board files the assumed validation is done during development/review
stages. But OTOH for the legacy code we need not to touch the property
provider more than once. We are _not_ expecting this to be spread.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Andy Shevchenko @ 2024-03-26 18:52 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <c3066481-bac6-48fd-8b38-6797975d83c2@sirena.org.uk>

On Tue, Mar 26, 2024 at 06:49:58PM +0000, Mark Brown wrote:
> On Tue, Mar 26, 2024 at 08:44:11PM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 26, 2024 at 06:25:16PM +0000, Mark Brown wrote:
> > > On Tue, Mar 26, 2024 at 08:22:59PM +0200, Andy Shevchenko wrote:
> > > > On Tue, Mar 26, 2024 at 06:16:28PM +0000, Mark Brown wrote:
> 
> > > > > I think the ACPI dependency there is as much about hiding the device on
> > > > > irrelevant platforms as anything else, might be better replaced with an
> > > > > x86 dependency though.

...

> > > That's nice but I'm not sure what that has to do with the dependency on
> > > ACPI?
> 
> > ACPI_PTR() makes ID no-op only if ACPI is defined. That also satisfies
> > the ugly ifdeffery that is removed by this patch as well. If there is
> > no dependency we will have compiler warning for defined but not used
> > variable.
> 
> Again I don't think that's the main purpose of the dependency here.

Oh, oh, my bad I missed acpi_dev_uid_to_integer() call.
Okay, with that in mind it's functional dependency for the ACPI-based
platforms. Do you want to keep it untouched?

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 2/6] arm64: dts: ti: k3-am64: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
  To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
In-Reply-To: <20240326185441.29656-1-afd@ti.com>

As described in the binding document for the "current-speed" property:

"This should only be present in case a driver has no chance to know the
baud rate of the slave device."

This is not the case for the UART used in K3 devices, the current
baud-rate can be calculated from the registers. Having this property
has the effect of actually skipping the baud-rate setup in some drivers
as it assumes it will already be set to this rate, which may not always
be the case.

It seems this property's purpose was mistaken as selecting the desired
baud-rate, which it does not. It would have been wrong to select that
here anyway as DT is not the place for configuration, especially when
there are already more standard ways to set serial baud-rates.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am642-evm.dts                  | 1 -
 arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts | 2 --
 arch/arm64/boot/dts/ti/k3-am642-sk.dts                   | 1 -
 3 files changed, 4 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
index 53fe1d065ddbb..e20e4ffd0f1fa 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
@@ -473,7 +473,6 @@ &main_uart0 {
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&main_uart0_pins_default>;
-	current-speed = <115200>;
 };
 
 /* main_uart1 is reserved for firmware usage */
diff --git a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
index 8237b8c815b84..dc774f9c75417 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
@@ -275,7 +275,6 @@ &main_uart0 {
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&main_uart0_pins_default>;
-	current-speed = <115200>;
 };
 
 &main_uart1 {
@@ -283,7 +282,6 @@ &main_uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&main_uart1_pins_default>;
 	uart-has-rtscts;
-	current-speed = <115200>;
 };
 
 &sdhci1 {
diff --git a/arch/arm64/boot/dts/ti/k3-am642-sk.dts b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
index 67cd41bf806ea..5b028b3a3192f 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
@@ -381,7 +381,6 @@ &main_uart0 {
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&main_uart0_pins_default>;
-	current-speed = <115200>;
 };
 
 &main_uart1 {
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 1/6] arm64: dts: ti: k3-am65: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
  To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis

As described in the binding document for the "current-speed" property:

"This should only be present in case a driver has no chance to know the
baud rate of the slave device."

This is not the case for the UART used in K3 devices, the current
baud-rate can be calculated from the registers. Having this property
has the effect of actually skipping the baud-rate setup in some drivers
as it assumes it will already be set to this rate, which may not always
be the case.

It seems this property's purpose was mistaken as selecting the desired
baud-rate, which it does not. It would have been wrong to select that
here anyway as DT is not the place for configuration, especially when
there are already more standard ways to set serial baud-rates.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi   | 1 -
 arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi    | 1 -
 arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 1 -
 3 files changed, 3 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index ff857117d7193..670557c89f756 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -89,7 +89,6 @@ main_uart0: serial@2800000 {
 		reg = <0x00 0x02800000 0x00 0x100>;
 		interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
 		status = "disabled";
 	};
diff --git a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
index 6ff3ccc39fb44..4f808e5089755 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
@@ -43,7 +43,6 @@ mcu_uart0: serial@40a00000 {
 		reg = <0x00 0x40a00000 0x00 0x100>;
 		interrupts = <GIC_SPI 565 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <96000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
 		status = "disabled";
 	};
diff --git a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
index 37527890ddeaf..eee072e44a42f 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
@@ -59,7 +59,6 @@ wkup_uart0: serial@42300000 {
 		reg = <0x42300000 0x100>;
 		interrupts = <GIC_SPI 697 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 150 TI_SCI_PD_EXCLUSIVE>;
 		status = "disabled";
 	};
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 6/6] arm64: dts: ti: k3-j784s4: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
  To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
In-Reply-To: <20240326185441.29656-1-afd@ti.com>

As described in the binding document for the "current-speed" property:

"This should only be present in case a driver has no chance to know the
baud rate of the slave device."

This is not the case for the UART used in K3 devices, the current
baud-rate can be calculated from the registers. Having this property
has the effect of actually skipping the baud-rate setup in some drivers
as it assumes it will already be set to this rate, which may not always
be the case.

It seems this property's purpose was mistaken as selecting the desired
baud-rate, which it does not. It would have been wrong to select that
here anyway as DT is not the place for configuration, especially when
there are already more standard ways to set serial baud-rates.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi       | 10 ----------
 arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi |  2 --
 2 files changed, 12 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
index b67c37460a73d..d42f25cacf23d 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
@@ -404,7 +404,6 @@ main_uart0: serial@2800000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02800000 0x00 0x200>;
 		interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 146 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
@@ -415,7 +414,6 @@ main_uart1: serial@2810000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02810000 0x00 0x200>;
 		interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 388 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 388 TI_SCI_PD_EXCLUSIVE>;
@@ -426,7 +424,6 @@ main_uart2: serial@2820000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02820000 0x00 0x200>;
 		interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 389 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 389 TI_SCI_PD_EXCLUSIVE>;
@@ -437,7 +434,6 @@ main_uart3: serial@2830000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02830000 0x00 0x200>;
 		interrupts = <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 390 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 390 TI_SCI_PD_EXCLUSIVE>;
@@ -448,7 +444,6 @@ main_uart4: serial@2840000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02840000 0x00 0x200>;
 		interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 391 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 391 TI_SCI_PD_EXCLUSIVE>;
@@ -459,7 +454,6 @@ main_uart5: serial@2850000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02850000 0x00 0x200>;
 		interrupts = <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 392 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 392 TI_SCI_PD_EXCLUSIVE>;
@@ -470,7 +464,6 @@ main_uart6: serial@2860000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02860000 0x00 0x200>;
 		interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 393 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 393 TI_SCI_PD_EXCLUSIVE>;
@@ -481,7 +474,6 @@ main_uart7: serial@2870000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02870000 0x00 0x200>;
 		interrupts = <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 394 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 394 TI_SCI_PD_EXCLUSIVE>;
@@ -492,7 +484,6 @@ main_uart8: serial@2880000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02880000 0x00 0x200>;
 		interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 395 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 395 TI_SCI_PD_EXCLUSIVE>;
@@ -503,7 +494,6 @@ main_uart9: serial@2890000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02890000 0x00 0x200>;
 		interrupts = <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 396 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 396 TI_SCI_PD_EXCLUSIVE>;
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
index 77a8d99139ec1..b47338e0f4812 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
@@ -304,7 +304,6 @@ wkup_uart0: serial@42300000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x42300000 0x00 0x200>;
 		interrupts = <GIC_SPI 897 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 397 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 397 TI_SCI_PD_EXCLUSIVE>;
@@ -315,7 +314,6 @@ mcu_uart0: serial@40a00000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x40a00000 0x00 0x200>;
 		interrupts = <GIC_SPI 846 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 149 0>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 4/6] arm64: dts: ti: k3-j721e: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
  To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
In-Reply-To: <20240326185441.29656-1-afd@ti.com>

As described in the binding document for the "current-speed" property:

"This should only be present in case a driver has no chance to know the
baud rate of the slave device."

This is not the case for the UART used in K3 devices, the current
baud-rate can be calculated from the registers. Having this property
has the effect of actually skipping the baud-rate setup in some drivers
as it assumes it will already be set to this rate, which may not always
be the case.

It seems this property's purpose was mistaken as selecting the desired
baud-rate, which it does not. It would have been wrong to select that
here anyway as DT is not the place for configuration, especially when
there are already more standard ways to set serial baud-rates.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j721e-main.dtsi       | 10 ----------
 arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi |  2 --
 2 files changed, 12 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
index c7eafbc862f96..0da785be80ff4 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
@@ -1337,7 +1337,6 @@ main_uart0: serial@2800000 {
 		reg = <0x00 0x02800000 0x00 0x100>;
 		interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 146 0>;
 		clock-names = "fclk";
@@ -1349,7 +1348,6 @@ main_uart1: serial@2810000 {
 		reg = <0x00 0x02810000 0x00 0x100>;
 		interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 278 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 278 0>;
 		clock-names = "fclk";
@@ -1361,7 +1359,6 @@ main_uart2: serial@2820000 {
 		reg = <0x00 0x02820000 0x00 0x100>;
 		interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 279 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 279 0>;
 		clock-names = "fclk";
@@ -1373,7 +1370,6 @@ main_uart3: serial@2830000 {
 		reg = <0x00 0x02830000 0x00 0x100>;
 		interrupts = <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 280 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 280 0>;
 		clock-names = "fclk";
@@ -1385,7 +1381,6 @@ main_uart4: serial@2840000 {
 		reg = <0x00 0x02840000 0x00 0x100>;
 		interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 281 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 281 0>;
 		clock-names = "fclk";
@@ -1397,7 +1392,6 @@ main_uart5: serial@2850000 {
 		reg = <0x00 0x02850000 0x00 0x100>;
 		interrupts = <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 282 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 282 0>;
 		clock-names = "fclk";
@@ -1409,7 +1403,6 @@ main_uart6: serial@2860000 {
 		reg = <0x00 0x02860000 0x00 0x100>;
 		interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 283 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 283 0>;
 		clock-names = "fclk";
@@ -1421,7 +1414,6 @@ main_uart7: serial@2870000 {
 		reg = <0x00 0x02870000 0x00 0x100>;
 		interrupts = <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 284 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 284 0>;
 		clock-names = "fclk";
@@ -1433,7 +1425,6 @@ main_uart8: serial@2880000 {
 		reg = <0x00 0x02880000 0x00 0x100>;
 		interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 285 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 285 0>;
 		clock-names = "fclk";
@@ -1445,7 +1436,6 @@ main_uart9: serial@2890000 {
 		reg = <0x00 0x02890000 0x00 0x100>;
 		interrupts = <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 286 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 286 0>;
 		clock-names = "fclk";
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
index 4618b697fbc47..8fc768d84b759 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
@@ -243,7 +243,6 @@ wkup_uart0: serial@42300000 {
 		reg = <0x00 0x42300000 0x00 0x100>;
 		interrupts = <GIC_SPI 897 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 287 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 287 0>;
 		clock-names = "fclk";
@@ -255,7 +254,6 @@ mcu_uart0: serial@40a00000 {
 		reg = <0x00 0x40a00000 0x00 0x100>;
 		interrupts = <GIC_SPI 846 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <96000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 149 0>;
 		clock-names = "fclk";
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 5/6] arm64: dts: ti: k3-j721s2: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
  To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
In-Reply-To: <20240326185441.29656-1-afd@ti.com>

As described in the binding document for the "current-speed" property:

"This should only be present in case a driver has no chance to know the
baud rate of the slave device."

This is not the case for the UART used in K3 devices, the current
baud-rate can be calculated from the registers. Having this property
has the effect of actually skipping the baud-rate setup in some drivers
as it assumes it will already be set to this rate, which may not always
be the case.

It seems this property's purpose was mistaken as selecting the desired
baud-rate, which it does not. It would have been wrong to select that
here anyway as DT is not the place for configuration, especially when
there are already more standard ways to set serial baud-rates.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi       | 10 ----------
 arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi |  2 --
 2 files changed, 12 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
index b70c8615e3c15..3cb06a7e4117f 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
@@ -459,7 +459,6 @@ main_uart0: serial@2800000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02800000 0x00 0x200>;
 		interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 146 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
@@ -470,7 +469,6 @@ main_uart1: serial@2810000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02810000 0x00 0x200>;
 		interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 350 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 350 TI_SCI_PD_EXCLUSIVE>;
@@ -481,7 +479,6 @@ main_uart2: serial@2820000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02820000 0x00 0x200>;
 		interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 351 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 351 TI_SCI_PD_EXCLUSIVE>;
@@ -492,7 +489,6 @@ main_uart3: serial@2830000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02830000 0x00 0x200>;
 		interrupts = <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 352 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 352 TI_SCI_PD_EXCLUSIVE>;
@@ -503,7 +499,6 @@ main_uart4: serial@2840000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02840000 0x00 0x200>;
 		interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 353 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 353 TI_SCI_PD_EXCLUSIVE>;
@@ -514,7 +509,6 @@ main_uart5: serial@2850000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02850000 0x00 0x200>;
 		interrupts = <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 354 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 354 TI_SCI_PD_EXCLUSIVE>;
@@ -525,7 +519,6 @@ main_uart6: serial@2860000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02860000 0x00 0x200>;
 		interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 355 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 355 TI_SCI_PD_EXCLUSIVE>;
@@ -536,7 +529,6 @@ main_uart7: serial@2870000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02870000 0x00 0x200>;
 		interrupts = <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 356 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 356 TI_SCI_PD_EXCLUSIVE>;
@@ -547,7 +539,6 @@ main_uart8: serial@2880000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02880000 0x00 0x200>;
 		interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 357 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 357 TI_SCI_PD_EXCLUSIVE>;
@@ -558,7 +549,6 @@ main_uart9: serial@2890000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x02890000 0x00 0x200>;
 		interrupts = <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 358 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 358 TI_SCI_PD_EXCLUSIVE>;
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
index eaf7f709440e6..5ccb04c7c4624 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
@@ -298,7 +298,6 @@ wkup_uart0: serial@42300000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x42300000 0x00 0x200>;
 		interrupts = <GIC_SPI 897 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 359 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 359 TI_SCI_PD_EXCLUSIVE>;
@@ -309,7 +308,6 @@ mcu_uart0: serial@40a00000 {
 		compatible = "ti,j721e-uart", "ti,am654-uart";
 		reg = <0x00 0x40a00000 0x00 0x200>;
 		interrupts = <GIC_SPI 846 IRQ_TYPE_LEVEL_HIGH>;
-		current-speed = <115200>;
 		clocks = <&k3_clks 149 3>;
 		clock-names = "fclk";
 		power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 3/6] arm64: dts: ti: k3-j7200: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 18:54 UTC (permalink / raw)
  To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
In-Reply-To: <20240326185441.29656-1-afd@ti.com>

As described in the binding document for the "current-speed" property:

"This should only be present in case a driver has no chance to know the
baud rate of the slave device."

This is not the case for the UART used in K3 devices, the current
baud-rate can be calculated from the registers. Having this property
has the effect of actually skipping the baud-rate setup in some drivers
as it assumes it will already be set to this rate, which may not always
be the case.

It seems this property's purpose was mistaken as selecting the desired
baud-rate, which it does not. It would have been wrong to select that
here anyway as DT is not the place for configuration, especially when
there are already more standard ways to set serial baud-rates.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j7200-main.dtsi       | 10 ----------
 arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi |  2 --
 2 files changed, 12 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
index 657f9cc9f4ea0..9386bf3ef9f68 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
@@ -440,7 +440,6 @@ main_uart0: serial@2800000 {
 		reg = <0x00 0x02800000 0x00 0x100>;
 		interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 146 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 146 2>;
 		clock-names = "fclk";
@@ -452,7 +451,6 @@ main_uart1: serial@2810000 {
 		reg = <0x00 0x02810000 0x00 0x100>;
 		interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 278 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 278 2>;
 		clock-names = "fclk";
@@ -464,7 +462,6 @@ main_uart2: serial@2820000 {
 		reg = <0x00 0x02820000 0x00 0x100>;
 		interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 279 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 279 2>;
 		clock-names = "fclk";
@@ -476,7 +473,6 @@ main_uart3: serial@2830000 {
 		reg = <0x00 0x02830000 0x00 0x100>;
 		interrupts = <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 280 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 280 2>;
 		clock-names = "fclk";
@@ -488,7 +484,6 @@ main_uart4: serial@2840000 {
 		reg = <0x00 0x02840000 0x00 0x100>;
 		interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 281 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 281 2>;
 		clock-names = "fclk";
@@ -500,7 +495,6 @@ main_uart5: serial@2850000 {
 		reg = <0x00 0x02850000 0x00 0x100>;
 		interrupts = <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 282 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 282 2>;
 		clock-names = "fclk";
@@ -512,7 +506,6 @@ main_uart6: serial@2860000 {
 		reg = <0x00 0x02860000 0x00 0x100>;
 		interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 283 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 283 2>;
 		clock-names = "fclk";
@@ -524,7 +517,6 @@ main_uart7: serial@2870000 {
 		reg = <0x00 0x02870000 0x00 0x100>;
 		interrupts = <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 284 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 284 2>;
 		clock-names = "fclk";
@@ -536,7 +528,6 @@ main_uart8: serial@2880000 {
 		reg = <0x00 0x02880000 0x00 0x100>;
 		interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 285 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 285 2>;
 		clock-names = "fclk";
@@ -548,7 +539,6 @@ main_uart9: serial@2890000 {
 		reg = <0x00 0x02890000 0x00 0x100>;
 		interrupts = <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 286 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 286 2>;
 		clock-names = "fclk";
diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
index 7cf21c99956e0..c547bc8fb6082 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -259,7 +259,6 @@ wkup_uart0: serial@42300000 {
 		reg = <0x00 0x42300000 0x00 0x100>;
 		interrupts = <GIC_SPI 897 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <48000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 287 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 287 2>;
 		clock-names = "fclk";
@@ -271,7 +270,6 @@ mcu_uart0: serial@40a00000 {
 		reg = <0x00 0x40a00000 0x00 0x100>;
 		interrupts = <GIC_SPI 846 IRQ_TYPE_LEVEL_HIGH>;
 		clock-frequency = <96000000>;
-		current-speed = <115200>;
 		power-domains = <&k3_pds 149 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 149 2>;
 		clock-names = "fclk";
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 1/2] arm64: dts: ti: k3-am65: Add full compatible to SerDes control nodes
From: Andrew Davis @ 2024-03-26 18:56 UTC (permalink / raw)
  To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis

This matches the binding for this register region which fixes a couple
DTS check warnings.

While here trim the leading 0s from the "reg" definition.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index ff857117d7193..738c5c4acbcd2 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -483,13 +483,13 @@ scm_conf: scm-conf@100000 {
 		ranges = <0x0 0x0 0x00100000 0x1c000>;
 
 		serdes0_clk: clock@4080 {
-			compatible = "syscon";
-			reg = <0x00004080 0x4>;
+			compatible = "ti,am654-serdes-ctrl", "syscon";
+			reg = <0x4080 0x4>;
 		};
 
 		serdes1_clk: clock@4090 {
-			compatible = "syscon";
-			reg = <0x00004090 0x4>;
+			compatible = "ti,am654-serdes-ctrl", "syscon";
+			reg = <0x4090 0x4>;
 		};
 
 		serdes_mux: mux-controller {
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 2/2] arm64: dts: ti: k3-am65: Move SerDes mux nodes under the control node
From: Andrew Davis @ 2024-03-26 18:56 UTC (permalink / raw)
  To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, devicetree, linux-kernel, Andrew Davis
In-Reply-To: <20240326185627.29852-1-afd@ti.com>

These SerDes lane select muxes use bits from the same register as
the SerDes clock select mux. Make the lane select mux a child
of the SerDes control node.

This removes one more requirement on scm-conf being a syscon node
which will later be converted to fix a couple DTS check warnings.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index 738c5c4acbcd2..5ce67e6a33600 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -66,7 +66,7 @@ serdes0: serdes@900000 {
 		assigned-clock-parents = <&k3_clks 153 8>, <&k3_clks 153 4>;
 		ti,serdes-clk = <&serdes0_clk>;
 		#clock-cells = <1>;
-		mux-controls = <&serdes_mux 0>;
+		mux-controls = <&serdes0_mux 0>;
 	};
 
 	serdes1: serdes@910000 {
@@ -81,7 +81,7 @@ serdes1: serdes@910000 {
 		assigned-clock-parents = <&k3_clks 154 9>, <&k3_clks 154 5>;
 		ti,serdes-clk = <&serdes1_clk>;
 		#clock-cells = <1>;
-		mux-controls = <&serdes_mux 1>;
+		mux-controls = <&serdes1_mux 0>;
 	};
 
 	main_uart0: serial@2800000 {
@@ -485,18 +485,23 @@ scm_conf: scm-conf@100000 {
 		serdes0_clk: clock@4080 {
 			compatible = "ti,am654-serdes-ctrl", "syscon";
 			reg = <0x4080 0x4>;
+
+			serdes0_mux: mux-controller {
+				compatible = "mmio-mux";
+				#mux-control-cells = <1>;
+				mux-reg-masks = <0x0 0x3>; /* lane select */
+			};
 		};
 
 		serdes1_clk: clock@4090 {
 			compatible = "ti,am654-serdes-ctrl", "syscon";
 			reg = <0x4090 0x4>;
-		};
 
-		serdes_mux: mux-controller {
-			compatible = "mmio-mux";
-			#mux-control-cells = <1>;
-			mux-reg-masks = <0x4080 0x3>, /* SERDES0 lane select */
-					<0x4090 0x3>; /* SERDES1 lane select */
+			serdes1_mux: mux-controller {
+				compatible = "mmio-mux";
+				#mux-control-cells = <1>;
+				mux-reg-masks = <0x0 0x3>; /* lane select */
+			};
 		};
 
 		dss_oldi_io_ctrl: dss-oldi-io-ctrl@41e0 {
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 2/3] devicetree: phy: rockchip-emmc: Document changed strobe-pulldown property
From: Folker Schwesinger via B4 Relay @ 2024-03-26 18:54 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner, Chris Ruehl,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Christopher Obbard, Alban Browaeys, Doug Anderson, Brian Norris,
	Jensen Huang, linux-phy, linux-arm-kernel, linux-rockchip,
	linux-kernel, devicetree, Folker Schwesinger
In-Reply-To: <20240326-rk-default-enable-strobe-pulldown-v1-0-f410c71605c0@folker-schwesinger.de>

From: Folker Schwesinger <dev@folker-schwesinger.de>

Document the changes regarding the optional strobe-pulldown property.
These changes are necessary as the default behavior of the driver was
restored to the Rockchip kernel behavior of enabling the internal
pulldown by default.

Fixes: f34e43f12382 ("devicetree: phy: rockchip-emmc: pulldown property")
Signed-off-by: Folker Schwesinger <dev@folker-schwesinger.de>
---
 Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
index 57d28c0d5696..10c05437f7ab 100644
--- a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
+++ b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
@@ -16,8 +16,8 @@ Optional properties:
  - drive-impedance-ohm: Specifies the drive impedance in Ohm.
                         Possible values are 33, 40, 50, 66 and 100.
                         If not set, the default value of 50 will be applied.
- - rockchip,enable-strobe-pulldown: Enable internal pull-down for the strobe
-                                    line.  If not set, pull-down is not used.
+ - rockchip,disable-strobe-pulldown: Disable internal pull-down for the strobe
+                                     line.  If not set, pull-down is used.
  - rockchip,output-tapdelay-select: Specifies the phyctrl_otapdlysec register.
                                     If not set, the register defaults to 0x4.
                                     Maximum value 0xf.

-- 
2.44.0



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 0/3] phy: rockchip: emmc: Enable internal strobe pull-down by default
From: Folker Schwesinger via B4 Relay @ 2024-03-26 18:54 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner, Chris Ruehl,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Christopher Obbard, Alban Browaeys, Doug Anderson, Brian Norris,
	Jensen Huang, linux-phy, linux-arm-kernel, linux-rockchip,
	linux-kernel, devicetree, Folker Schwesinger

Various RK3399 boards, including the Rock Pi 4 series, the NanoPi 4
series and possibly others showed instabilities with some eMMC modules
when operating in HS400 mode [1]. Later, a missing pull-down resistor on
the eMMC strobe line was identified as the root cause of these
instabilities [2].

This series changes the Rockchip eMMC PHY driver to enable the
internal pull-down for the strobe line by default, effectively
restoring the behavior of the vendor kernel [3].
Addressing the problem in the driver avoids fix-ups in individual
board DTs.

As this change makes the optional DT property enable-strobe-pulldown
obsolete, it is replaced by a new property disable-strobe-pulldown,
that, when used, allows to disable the internal pull-down.

While at it, the enable-strobe-pulldown property was removed from the
NanoPi4 series DTS (which was the only user of the property in the kernel).

[1]: https://lore.kernel.org/all/20230705144255.115299-1-chris.obbard@collabora.com/
[2]: https://lore.kernel.org/all/20220822074139.3810-1-jensenhuang@friendlyarm.com/
[3]: https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/phy/rockchip/phy-rockchip-emmc.c#L310

Signed-off-by: Folker Schwesinger <dev@folker-schwesinger.de>
---
Folker Schwesinger (3):
      phy: rockchip: emmc: Enable pulldown for strobe line
      devicetree: phy: rockchip-emmc: Document changed strobe-pulldown property
      arm64: dts: rockchip: Remove enable-strobe-pulldown for NanoPi4 boards

 Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt | 4 ++--
 arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi            | 1 -
 drivers/phy/rockchip/phy-rockchip-emmc.c                    | 6 +++---
 3 files changed, 5 insertions(+), 6 deletions(-)
---
base-commit: 4cece764965020c22cff7665b18a012006359095
change-id: 20240326-rk-default-enable-strobe-pulldown-3356c8cf1b4f

Best regards,
-- 
Folker Schwesinger <dev@folker-schwesinger.de>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 1/3] phy: rockchip: emmc: Enable pulldown for strobe line
From: Folker Schwesinger via B4 Relay @ 2024-03-26 18:54 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner, Chris Ruehl,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Christopher Obbard, Alban Browaeys, Doug Anderson, Brian Norris,
	Jensen Huang, linux-phy, linux-arm-kernel, linux-rockchip,
	linux-kernel, devicetree, Folker Schwesinger
In-Reply-To: <20240326-rk-default-enable-strobe-pulldown-v1-0-f410c71605c0@folker-schwesinger.de>

From: Folker Schwesinger <dev@folker-schwesinger.de>

Restore the behavior of the Rockchip kernel that undconditionally
enables the internal strobe pulldown.
As the DT property rockchip,enable-strobe-pulldown is obsolete now,
replace it with a property to disable the internal pulldown.

This fixes I/O errors observed on various Rock Pi 4 and NanoPi4 series
boards with some eMMC modules. Other boards may also be affected.

An example of these errors is as follows:

[  290.060817] mmc1: running CQE recovery
[  290.061337] blk_update_request: I/O error, dev mmcblk1, sector 1411072 op 0x1:(WRITE) flags 0x800 phys_seg 36 prio class 0
[  290.061370] EXT4-fs warning (device mmcblk1p1): ext4_end_bio:348: I/O error 10 writing to inode 29547 starting block 176466)
[  290.061484] Buffer I/O error on device mmcblk1p1, logical block 172288

Fixes: 8b5c2b45b8f0 ("phy: rockchip: set pulldown for strobe line in dts")
Signed-off-by: Folker Schwesinger <dev@folker-schwesinger.de>
---
 drivers/phy/rockchip/phy-rockchip-emmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-emmc.c b/drivers/phy/rockchip/phy-rockchip-emmc.c
index 20023f6eb994..6e637f3e1b19 100644
--- a/drivers/phy/rockchip/phy-rockchip-emmc.c
+++ b/drivers/phy/rockchip/phy-rockchip-emmc.c
@@ -376,14 +376,14 @@ static int rockchip_emmc_phy_probe(struct platform_device *pdev)
 	rk_phy->reg_offset = reg_offset;
 	rk_phy->reg_base = grf;
 	rk_phy->drive_impedance = PHYCTRL_DR_50OHM;
-	rk_phy->enable_strobe_pulldown = PHYCTRL_REN_STRB_DISABLE;
+	rk_phy->enable_strobe_pulldown = PHYCTRL_REN_STRB_ENABLE;
 	rk_phy->output_tapdelay_select = PHYCTRL_OTAPDLYSEL_DEFAULT;
 
 	if (!of_property_read_u32(dev->of_node, "drive-impedance-ohm", &val))
 		rk_phy->drive_impedance = convert_drive_impedance_ohm(pdev, val);
 
-	if (of_property_read_bool(dev->of_node, "rockchip,enable-strobe-pulldown"))
-		rk_phy->enable_strobe_pulldown = PHYCTRL_REN_STRB_ENABLE;
+	if (of_property_read_bool(dev->of_node, "rockchip,disable-strobe-pulldown"))
+		rk_phy->enable_strobe_pulldown = PHYCTRL_REN_STRB_DISABLE;
 
 	if (!of_property_read_u32(dev->of_node, "rockchip,output-tapdelay-select", &val)) {
 		if (val <= PHYCTRL_OTAPDLYSEL_MAXVALUE)

-- 
2.44.0



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 3/3] arm64: dts: rockchip: Remove enable-strobe-pulldown for NanoPi4 boards
From: Folker Schwesinger via B4 Relay @ 2024-03-26 18:54 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner, Chris Ruehl,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Christopher Obbard, Alban Browaeys, Doug Anderson, Brian Norris,
	Jensen Huang, linux-phy, linux-arm-kernel, linux-rockchip,
	linux-kernel, devicetree, Folker Schwesinger
In-Reply-To: <20240326-rk-default-enable-strobe-pulldown-v1-0-f410c71605c0@folker-schwesinger.de>

From: Folker Schwesinger <dev@folker-schwesinger.de>

The Rockchip eMMC PHY driver enables the internal strobe pulldown by
default. So, remove the enable-strobe-pulldown property from the
NanoPi4 series boards DTS.

Signed-off-by: Folker Schwesinger <dev@folker-schwesinger.de>
---
 arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
index b7f1e47978a6..38af91b9e756 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
@@ -168,7 +168,6 @@ &cpu_l3 {
 };
 
 &emmc_phy {
-	rockchip,enable-strobe-pulldown;
 	status = "okay";
 };
 

-- 
2.44.0



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH bpf-next v2 1/1] arm64/cfi,bpf: Support kCFI + BPF on arm64
From: Mark Rutland @ 2024-03-26 18:59 UTC (permalink / raw)
  To: Puranjay Mohan
  Cc: Catalin Marinas, Will Deacon, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Zi Shen Lim, Suzuki K Poulose, Mark Brown,
	linux-arm-kernel, open list,
	open list:BPF [GENERAL] (Safe Dynamic Programs and Tools),
	Josh Poimboeuf
In-Reply-To: <20240324211518.93892-2-puranjay12@gmail.com>

Hi Puranjay,

On Sun, Mar 24, 2024 at 09:15:18PM +0000, Puranjay Mohan wrote:
> Currently, bpf_dispatcher_*_func() is marked with `__nocfi` therefore
> calling BPF programs from this interface doesn't cause CFI warnings.
> 
> When BPF programs are called directly from C: from BPF helpers or
> struct_ops, CFI warnings are generated.
> 
> Implement proper CFI prologues for the BPF programs and callbacks and
> drop __nocfi for arm64. Fix the trampoline generation code to emit kCFI
> prologue when a struct_ops trampoline is being prepared.
> 
> Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>

Presumably this'll need a Cc stable and a Fixes tag?

> ---
>  arch/arm64/include/asm/cfi.h    | 23 ++++++++++++++
>  arch/arm64/kernel/alternative.c | 54 +++++++++++++++++++++++++++++++++
>  arch/arm64/net/bpf_jit_comp.c   | 28 +++++++++++++----
>  3 files changed, 99 insertions(+), 6 deletions(-)
>  create mode 100644 arch/arm64/include/asm/cfi.h
> 
> diff --git a/arch/arm64/include/asm/cfi.h b/arch/arm64/include/asm/cfi.h
> new file mode 100644
> index 000000000000..670e191f8628
> --- /dev/null
> +++ b/arch/arm64/include/asm/cfi.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_ARM64_CFI_H
> +#define _ASM_ARM64_CFI_H
> +
> +#ifdef CONFIG_CFI_CLANG
> +#define __bpfcall
> +static inline int cfi_get_offset(void)
> +{
> +	return 4;
> +}
> +#define cfi_get_offset cfi_get_offset
> +extern u32 cfi_bpf_hash;
> +extern u32 cfi_bpf_subprog_hash;
> +extern u32 cfi_get_func_hash(void *func);
> +#else
> +#define cfi_bpf_hash 0U
> +#define cfi_bpf_subprog_hash 0U
> +static inline u32 cfi_get_func_hash(void *func)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_CFI_CLANG */
> +#endif /* _ASM_ARM64_CFI_H */
> diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
> index 8ff6610af496..1715da7df137 100644
> --- a/arch/arm64/kernel/alternative.c
> +++ b/arch/arm64/kernel/alternative.c
> @@ -13,6 +13,7 @@
>  #include <linux/elf.h>
>  #include <asm/cacheflush.h>
>  #include <asm/alternative.h>
> +#include <asm/cfi.h>
>  #include <asm/cpufeature.h>
>  #include <asm/insn.h>
>  #include <asm/module.h>
> @@ -298,3 +299,56 @@ noinstr void alt_cb_patch_nops(struct alt_instr *alt, __le32 *origptr,
>  		updptr[i] = cpu_to_le32(aarch64_insn_gen_nop());
>  }
>  EXPORT_SYMBOL(alt_cb_patch_nops);
> +
> +#ifdef CONFIG_CFI_CLANG
> +struct bpf_insn;
> +
> +/* Must match bpf_func_t / DEFINE_BPF_PROG_RUN() */
> +extern unsigned int __bpf_prog_runX(const void *ctx,
> +				    const struct bpf_insn *insn);
> +
> +/*
> + * Force a reference to the external symbol so the compiler generates
> + * __kcfi_typid.
> + */
> +__ADDRESSABLE(__bpf_prog_runX);
> +
> +/* u32 __ro_after_init cfi_bpf_hash = __kcfi_typeid___bpf_prog_runX; */
> +asm (
> +"	.pushsection	.data..ro_after_init,\"aw\",@progbits	\n"
> +"	.type	cfi_bpf_hash,@object				\n"
> +"	.globl	cfi_bpf_hash					\n"
> +"	.p2align	2, 0x0					\n"
> +"cfi_bpf_hash:							\n"
> +"	.word	__kcfi_typeid___bpf_prog_runX			\n"
> +"	.size	cfi_bpf_hash, 4					\n"
> +"	.popsection						\n"
> +);
> +
> +/* Must match bpf_callback_t */
> +extern u64 __bpf_callback_fn(u64, u64, u64, u64, u64);
> +
> +__ADDRESSABLE(__bpf_callback_fn);
> +
> +/* u32 __ro_after_init cfi_bpf_subprog_hash = __kcfi_typeid___bpf_callback_fn; */
> +asm (
> +"	.pushsection	.data..ro_after_init,\"aw\",@progbits	\n"
> +"	.type	cfi_bpf_subprog_hash,@object			\n"
> +"	.globl	cfi_bpf_subprog_hash				\n"
> +"	.p2align	2, 0x0					\n"
> +"cfi_bpf_subprog_hash:						\n"
> +"	.word	__kcfi_typeid___bpf_callback_fn			\n"
> +"	.size	cfi_bpf_subprog_hash, 4				\n"
> +"	.popsection						\n"
> +);
> +
> +u32 cfi_get_func_hash(void *func)
> +{
> +	u32 hash;
> +
> +	if (get_kernel_nofault(hash, func - cfi_get_offset()))
> +		return 0;
> +
> +	return hash;
> +}
> +#endif

I realise this is following the example of x86, but this has nothing to do with
alternatives, so could we please place it elsewhere? e.g. add a new
arch/arm64/net/bpf_cfi.c?

Which functions is cfi_get_func_hash() used against? The comment in the code
below says:

	if (flags & BPF_TRAMP_F_INDIRECT) {
		/*
		 * Indirect call for bpf_struct_ops
		 */
		emit_kcfi(cfi_get_func_hash(func_addr), ctx);
	}

... but it's not clear to me which functions specifically would be in that
'func_addr', not why returning 0 is fine -- surely we should fail compilation
if the provided function pointer causes a fault and we don't have a valid
typeid?

> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index bc16eb694657..2372812bb47c 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -17,6 +17,7 @@
>  #include <asm/asm-extable.h>
>  #include <asm/byteorder.h>
>  #include <asm/cacheflush.h>
> +#include <asm/cfi.h>
>  #include <asm/debug-monitors.h>
>  #include <asm/insn.h>
>  #include <asm/patching.h>
> @@ -158,6 +159,12 @@ static inline void emit_bti(u32 insn, struct jit_ctx *ctx)
>  		emit(insn, ctx);
>  }
>  
> +static inline void emit_kcfi(u32 hash, struct jit_ctx *ctx)
> +{
> +	if (IS_ENABLED(CONFIG_CFI_CLANG))
> +		emit(hash, ctx);
> +}
> +
>  /*
>   * Kernel addresses in the vmalloc space use at most 48 bits, and the
>   * remaining bits are guaranteed to be 0x1. So we can compose the address
> @@ -295,7 +302,7 @@ static bool is_lsi_offset(int offset, int scale)
>  #define PROLOGUE_OFFSET (BTI_INSNS + 2 + PAC_INSNS + 8)
>  
>  static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf,
> -			  bool is_exception_cb)
> +			  bool is_exception_cb, bool is_subprog)
>  {
>  	const struct bpf_prog *prog = ctx->prog;
>  	const bool is_main_prog = !bpf_is_subprog(prog);
> @@ -306,7 +313,6 @@ static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf,
>  	const u8 fp = bpf2a64[BPF_REG_FP];
>  	const u8 tcc = bpf2a64[TCALL_CNT];
>  	const u8 fpb = bpf2a64[FP_BOTTOM];
> -	const int idx0 = ctx->idx;
>  	int cur_offset;
>  
>  	/*
> @@ -332,6 +338,8 @@ static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf,
>  	 *
>  	 */
>  
> +	emit_kcfi(is_subprog ? cfi_bpf_subprog_hash : cfi_bpf_hash, ctx);
> +	const int idx0 = ctx->idx;
>  	/* bpf function may be invoked by 3 instruction types:
>  	 * 1. bl, attached via freplace to bpf prog via short jump
>  	 * 2. br, attached via freplace to bpf prog via long jump
> @@ -1648,7 +1656,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>  	 * BPF line info needs ctx->offset[i] to be the offset of
>  	 * instruction[i] in jited image, so build prologue first.
>  	 */
> -	if (build_prologue(&ctx, was_classic, prog->aux->exception_cb)) {
> +	if (build_prologue(&ctx, was_classic, prog->aux->exception_cb,
> +			   bpf_is_subprog(prog))) {
>  		prog = orig_prog;
>  		goto out_off;
>  	}
> @@ -1696,7 +1705,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>  	ctx.idx = 0;
>  	ctx.exentry_idx = 0;
>  
> -	build_prologue(&ctx, was_classic, prog->aux->exception_cb);
> +	build_prologue(&ctx, was_classic, prog->aux->exception_cb,
> +		       bpf_is_subprog(prog));
>  
>  	if (build_body(&ctx, extra_pass)) {
>  		prog = orig_prog;
> @@ -1745,9 +1755,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>  		jit_data->ro_header = ro_header;
>  	}
>  
> -	prog->bpf_func = (void *)ctx.ro_image;
> +	prog->bpf_func = (void *)ctx.ro_image + cfi_get_offset();
>  	prog->jited = 1;
> -	prog->jited_len = prog_size;
> +	prog->jited_len = prog_size - cfi_get_offset();
>  
>  	if (!prog->is_func || extra_pass) {
>  		int i;
> @@ -2011,6 +2021,12 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
>  	/* return address locates above FP */
>  	retaddr_off = stack_size + 8;
>  
> +	if (flags & BPF_TRAMP_F_INDIRECT) {
> +		/*
> +		 * Indirect call for bpf_struct_ops
> +		 */
> +		emit_kcfi(cfi_get_func_hash(func_addr), ctx);
> +	}

I'm confused; why does the trampoline need this?

The code that branches to the trampoline doesn't check the type hash: either
the callsite branches directly (hence no check), or the common ftrace
trampoline does so indirectly, and the latter doesn't know the expected typeid,
so it cannot check.

If we wanted a tpyeid here, that should probably be distinct from the original
function tpyeid, since the trampoline calling convention is different.

IIUC this isn't reachable today regardless since CALL_OPS and KCFI are
currently mutually exclusive.

Either I've misunderstood what's going on here, or this can be removed.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 09/64] i2c: bcm2835: reword according to newest specification
From: Andi Shyti @ 2024-03-26 19:03 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, Florian Fainelli, Broadcom internal kernel review list,
	Ray Jui, Scott Branden, linux-rpi-kernel, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20240322132619.6389-10-wsa+renesas@sang-engineering.com>

Hi Wolfram,

> @@ -391,7 +391,7 @@ static u32 bcm2835_i2c_func(struct i2c_adapter *adap)
>  }
>  
>  static const struct i2c_algorithm bcm2835_i2c_algo = {
> -	.master_xfer	= bcm2835_i2c_xfer,
> +	.xfer	= bcm2835_i2c_xfer,

Here you are breaking the alignment (even though I think a "tab"
alignment is not needed).

Andi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 10/64] i2c: brcmstb: reword according to newest specification
From: Andi Shyti @ 2024-03-26 19:04 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, Kamal Dasu, Broadcom internal kernel review list,
	Florian Fainelli, linux-arm-kernel, linux-kernel
In-Reply-To: <20240322132619.6389-11-wsa+renesas@sang-engineering.com>

Hi Wolfram,

On Fri, Mar 22, 2024 at 02:25:03PM +0100, Wolfram Sang wrote:
> Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C
> specifications and replace "master/slave" with more appropriate terms.
> They are also more specific because we distinguish now between a remote
> entity ("client") and a local one ("target").
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Andi Shyti <andi.shyti@kernel.org>

Thanks,
Andi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 11/64] i2c: cadence: reword according to newest specification
From: Andi Shyti @ 2024-03-26 19:06 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-i2c, Michal Simek, linux-arm-kernel, linux-kernel
In-Reply-To: <20240322132619.6389-12-wsa+renesas@sang-engineering.com>

Hi Wolfram,

>  static const struct i2c_algorithm cdns_i2c_algo = {
> -	.master_xfer	= cdns_i2c_master_xfer,
> +	.xfer	= cdns_i2c_xfer,

alignment :-)

Andi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5 01/27] iommu/arm-smmu-v3: Do not allow a SVA domain to be set on the wrong PASID
From: Mostafa Saleh @ 2024-03-26 19:06 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
	Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
	Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <20240326183016.GK6245@nvidia.com>

On Tue, Mar 26, 2024 at 03:30:16PM -0300, Jason Gunthorpe wrote:
> On Fri, Mar 22, 2024 at 05:48:52PM +0000, Mostafa Saleh wrote:
> > Hi Jason,
> > 
> > On Mon, Mar 04, 2024 at 07:43:49PM -0400, Jason Gunthorpe wrote:
> > > The SVA code is wired to assume that the SVA is programmed onto the
> > > mm->pasid. The current core code always does this, so it is fine.
> > > 
> > > Add a check for clarity.
> > > 
> > > Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > > ---
> > >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > > index 2610e82c0ecd0d..347c2fdd865c1a 100644
> > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > > @@ -581,6 +581,9 @@ static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
> > >  	int ret = 0;
> > >  	struct mm_struct *mm = domain->mm;
> > >  
> > > +	if (mm_get_enqcmd_pasid(mm) != id)
> > > +		return -EINVAL;
> > > +
> > I am not sure if that is needed, the only caller in the tree is the IOMMU code
> > and it does the right thing, as that check is removed later anyway, I don’t
> > think this patch adds much.
> 
> It really should be backported, when we get drivers that do other
> things it creates a hazard. I've added a fixes line.

Maybe I am misunderstanding the case, but AFAIU, the only caller for this is
iommu_sva_bind_device() which is the function populating the pasid, so this
condition should never hit with the current code.
And Linux won’t backport new drivers, so there is no need to do that?

Thanks,
Mostafa

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


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