* Re: [PATCH 2/4] dt-bindings: mfd: x-powers,axp152: add boost regulator
From: Krzysztof Kozlowski @ 2024-03-30 9:30 UTC (permalink / raw)
To: Andre Przywara, Chen-Yu Tsai, Lee Jones, Liam Girdwood,
Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: devicetree, linux-kernel, linux-sunxi, Jernej Skrabec,
Samuel Holland, Ryan Walklin, Chris Morgan
In-Reply-To: <20240329235033.25309-3-andre.przywara@arm.com>
On 30/03/2024 00:50, Andre Przywara wrote:
> The X-Powers AXP717 contains a boost regulator, that it meant to provide
> the 5V USB VBUS voltage when the devices operates on battery.
>
> Add the name "boost" to the regexp describing the allowed node names,
> to allow the regulator to be described in the devicetree.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> index b8e8db0d58e9c..14ab367fc8871 100644
> --- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> +++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> @@ -274,7 +274,7 @@ properties:
> Defines the work frequency of DC-DC in kHz.
>
> patternProperties:
> - "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo)$":
> + "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo|boost)$":
That's not an easy to read regex...
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
If driver does not depend on _, please consider dropping (_|-).
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
From: Krzysztof Kozlowski @ 2024-03-30 9:27 UTC (permalink / raw)
To: Klymenko, Anatoliy, Conor Dooley
Cc: Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Simek, Michal,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Tomi Valkeinen,
dri-devel@lists.freedesktop.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-media@vger.kernel.org
In-Reply-To: <MW4PR12MB7165E47719C72CD5CEB68218E6392@MW4PR12MB7165.namprd12.prod.outlook.com>
On 30/03/2024 03:02, Klymenko, Anatoliy wrote:
>>>>
>>>
>>> Sorry, I didn't explain properly what I'm trying to achieve. I need to
>>> create a DT node property that represents video signal format, one of
>>> MEDIA_BUS_FMT_* from include/uapi/linux/media-bus-format.h. It would
>> be
>>> nice to reuse the same symbolic values in the device tree. What is the
>>> best approach here? Should I create a separate header in
There is no user of this new header, so I don't agree. Please send
either full work or link your other upstreamed patchset. Anything sent
as "DO NOT MERGE" does not count because it is not an user.
Without the DTS user I claim that you do not bind here anything...
>>> include/dt-bindings with the same or similar (to avoid multiple
>>> definition errors) defines, or is it better to create a symlink to
>>> media-bus-format.h like include/dt-bindings/linux-event-codes.h?
Copying or symlinking entire header into bindings does not help us to
understand what is exactly a binding here.
For example, maybe you encode runtime information into DT (don't do
this) and that's why you need these defines... Or maybe your block has
some capabilities. Dunno, patch was not tested, is defined as do not
merge and is not explaining any of these.
Therefore, please provide complete set of users ready to be merged, test
your patches, provide rationale why this is supposed to be a binding and
why do you think it represents hardware configuration, not OS policy or
runtime configuration.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
From: Krzysztof Kozlowski @ 2024-03-30 9:19 UTC (permalink / raw)
To: Dmitry Baryshkov, Sumit Semwal, Caleb Connolly, Neil Armstrong,
Jessica Zhang, Sam Ravnborg, David Airlie, Daniel Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: dri-devel, devicetree, linux-kernel, linux-arm-msm, Vinod Koul,
Caleb Connolly
In-Reply-To: <20240330-lg-sw43408-panel-v1-1-f5580fc9f2da@linaro.org>
On 30/03/2024 04:59, Dmitry Baryshkov wrote:
> From: Sumit Semwal <sumit.semwal@linaro.org>
>
> LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel present on Google Pixel 3
> phones.
... Few nits, which could be ignored, and one thing which is blocking -
missing example.
> +
> +maintainers:
> + - Caleb Connolly <caleb.connolly@linaro.org>
> +
> +description: |
Do not need '|' unless you need to preserve formatting.
> + This panel is used on the Pixel 3, it is a 60hz OLED panel which
> + required DSC (Display Stream Compression) and has rounded corners.
> +
> +allOf:
> + - $ref: panel-common.yaml#
> +
> +properties:
> + compatible:
> + items:
> + - const: lg,sw43408
> +
> + reg: true
> + port: true
> + vddi-supply: true
> + vpnl-supply: true
> + reset-gpios: true
> +
> +additionalProperties: false
This goes after required:
> +
> +required:
> + - compatible
> + - vddi-supply
> + - vpnl-supply
> + - reset-gpios
Please add example DTS.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 17/18] dt-bindings: pci: rockchip,rk3399-pcie-ep: Add ep-gpios property
From: Krzysztof Kozlowski @ 2024-03-30 9:16 UTC (permalink / raw)
To: Damien Le Moal, Manivannan Sadhasivam, Lorenzo Pieralisi,
Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-18-dlemoal@kernel.org>
On 30/03/2024 05:19, Damien Le Moal wrote:
> From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>
> Describe the `ep-gpios` property which is used to map the PERST# input
> signal for endpoint mode.
>
> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> .../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> index 6b62f6f58efe..9331d44d6963 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> @@ -30,6 +30,9 @@ properties:
> maximum: 32
> default: 32
>
> + ep-gpios:
> + description: Input GPIO configured for the PERST# signal.
Missing maxItems. But more important: why existing property perst-gpios,
which you already have there in common schema, is not correct for this case?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RFC PATCH 0/2] Add gpio-usb-c-connector compatible
From: Krishna Kurapati PSSNV @ 2024-03-30 9:13 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Herring, Krzysztof Kozlowski, Greg Kroah-Hartman,
Conor Dooley, Miquel Raynal, Guenter Roeck, Bjorn Helgaas,
Kyle Tso, Fabrice Gasnier, Heikki Krogerus, u.kleine-koenig,
AngeloGioacchino Del Regno, devicetree, linux-usb, linux-kernel,
quic_ppratap, quic_jackp
In-Reply-To: <CAA8EJpqx+VFW8z6oG=+pnhPN97Q3R6z+ygf85Uspve-9syQsUw@mail.gmail.com>
On 3/29/2024 6:23 PM, Dmitry Baryshkov wrote:
> On Fri, 29 Mar 2024 at 09:20, Krishna Kurapati
> <quic_kriskura@quicinc.com> wrote:
>>
>> QDU1000 IDP [1] has a Type-c connector and supports USB 3.0.
>> However it relies on usb-conn-gpio driver to read the vbus and id
>> gpio's and provide role switch. However the driver currently has
>> only gpio-b-connector compatible present in ID table. Adding that
>> in DT would mean that the device supports Type-B connector and not
>> Type-c connector. Thanks to Dmitry Baryshkov for pointing it out [2].
>
> USB-B connector is pretty simple, it really has just an ID pin and
> VBUS input, which translates to two GPIOs being routed from the
> _connector_ itself.
>
> USB-C is much more complicated, it has two CC pins and a VBus power
> pin. It is not enough just to measure CC pin levels. Moreover,
> properly handling USB 3.0 inside a USB-C connector requires a separate
> 'orientation' signal to tell the host which two lanes must be used for
> the USB SS signals. Thus it is no longer possible to route just two
> pins from the connector to the SoC.
>
> Having all that in mind, I suspect that you are not describing your
> hardware properly. I suppose that you have a Type-C port controller /
> redriver / switch, which handles CC lines communication and then
> provides ID / VBUS signals to the host. In such a case, please
> describe this TCPC in the DT file and use its compatible string
> instead of "gpio-c-connector".
>
Hi Dmitry,
My bad. I must have provided more details of the HW.
I presume you are referring to addition of a connector node, type-c
switch, pmic-glink and other remote endpoints like in other SoC's like
SM8450/ SM8550/ SM8650.
This HW is slightly different. It has a Uni Phy for Super speed and
hence no DP.
For orientation switching, on mobile SoC's, there is a provision for
orientation gpio given in pmic-glink node and is handled in ucsi_glink
driver. But on this version of HW, there is a USB-C Switch with its own
firmware taking care of orientation switching. It takes 8 SS Lines and 2
CC lines coming from connector as input and gives out 4 SS Lines (SS
TX1/TX2 RX1/RX2) as output which go to the SoC. So orientation switch is
done by the USB-C-switch in between and it automatically routes
appropriate active SS Lane from connector to the SoC.
As usual like in other targets, the DP and DM lines from type-c
connector go to the SoC directly.
To handle role switch, the VBUS and ID Pin connections are given to
SoC as well. There is a vbus controller regulator present to provide
vbus to connected peripherals in host mode.
There is no PPM entity (ADSP in mobile SoC's) and no UCSI involved
here. Hence we rely on usb-conn-gpio to read the vbus/id and switch
roles accordingly.
Hope this answers the query as to why we wanted to use usb-conn-gpio
and why we were trying to add a new compatible.
Regards,
Krishna,
>>
>> This series intends to add that compatible in driver and bindings
>> so that it can be used in QDU1000 IDP DT.
>>
>> [1]: https://lore.kernel.org/all/20240319091020.15137-3-quic_kbajaj@quicinc.com/
>> [2]: https://lore.kernel.org/all/CAA8EJprXPvji8TgZu1idH7y4GtHtD4VmQABFBcRt-9BQaCberg@mail.gmail.com/
>>
>> Krishna Kurapati (2):
>> dt-bindings: connector: Add gpio-usb-c-connector compatible
>> usb: common: usb-conn-gpio: Update ID table to add usb-c connector
>>
>> Documentation/devicetree/bindings/connector/usb-connector.yaml | 3 +++
>> drivers/usb/common/usb-conn-gpio.c | 1 +
>> 2 files changed, 4 insertions(+)
>>
>> --
>> 2.34.1
>>
>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply
* [PATCH v2 2/2] arm64: dts: qcom: pm6150: correct USB VBUS regulator compatible
From: Krzysztof Kozlowski @ 2024-03-30 9:13 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Dmitry Baryshkov, Greg Kroah-Hartman,
Danila Tikhonov, linux-arm-msm, devicetree, linux-kernel
Cc: Krzysztof Kozlowski
In-Reply-To: <20240330091311.6224-1-krzysztof.kozlowski@linaro.org>
The first part of the compatible of USB VBUS node misses ending quote,
thus we have one long compatible consisting of two compatible strings
leading to dtbs_check warnings:
sc7180-idp.dtb: usb-vbus-regulator@1100: compatible:0: 'qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
sc7180-idp.dtb: /soc@0/spmi@c440000/pmic@0/usb-vbus-regulator@1100: failed to match any schema with compatible: ['qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg']
Reported-by: Rob Herring <robh@kernel.org>
Fixes: f81c2f01cad6 ("arm64: dts: qcom: pm6150: define USB-C related blocks")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. New patch, pointed out by: Gabor Juhos
---
arch/arm64/boot/dts/qcom/pm6150.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/pm6150.dtsi b/arch/arm64/boot/dts/qcom/pm6150.dtsi
index b20a639cddf3..6de6ed562d97 100644
--- a/arch/arm64/boot/dts/qcom/pm6150.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm6150.dtsi
@@ -64,8 +64,8 @@ pm6150_resin: resin {
};
pm6150_vbus: usb-vbus-regulator@1100 {
- compatible = "qcom,pm6150-vbus-reg,
- qcom,pm8150b-vbus-reg";
+ compatible = "qcom,pm6150-vbus-reg",
+ "qcom,pm8150b-vbus-reg";
reg = <0x1100>;
status = "disabled";
};
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/2] arm64: dts: qcom: pm6150: correct Type-C compatible
From: Krzysztof Kozlowski @ 2024-03-30 9:13 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Dmitry Baryshkov, Greg Kroah-Hartman,
Danila Tikhonov, linux-arm-msm, devicetree, linux-kernel
Cc: Krzysztof Kozlowski
The first part of the compatible of Type-C node misses ending quote,
thus we have one long compatible consisting of two compatible strings
leading to dtbs_check warnings:
sc7180-acer-aspire1.dtb: pmic@0: typec@1500:compatible: 'oneOf' conditional failed, one must be fixed
sc7180-acer-aspire1.dtb: typec@1500: compatible:0: 'qcom,pm6150-typec,\n qcom,pm8150b-typec' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
Reported-by: Rob Herring <robh@kernel.org>
Fixes: f81c2f01cad6 ("arm64: dts: qcom: pm6150: define USB-C related blocks")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. Correct warning msg
---
arch/arm64/boot/dts/qcom/pm6150.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/pm6150.dtsi b/arch/arm64/boot/dts/qcom/pm6150.dtsi
index 11158c2bd524..b20a639cddf3 100644
--- a/arch/arm64/boot/dts/qcom/pm6150.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm6150.dtsi
@@ -71,8 +71,8 @@ pm6150_vbus: usb-vbus-regulator@1100 {
};
pm6150_typec: typec@1500 {
- compatible = "qcom,pm6150-typec,
- qcom,pm8150b-typec";
+ compatible = "qcom,pm6150-typec",
+ "qcom,pm8150b-typec";
reg = <0x1500>, <0x1700>;
interrupts = <0x0 0x15 0x00 IRQ_TYPE_EDGE_RISING>,
<0x0 0x15 0x01 IRQ_TYPE_EDGE_BOTH>,
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] arm64: dts: qcom: pm6150: correct Type-C compatible
From: Krzysztof Kozlowski @ 2024-03-30 9:05 UTC (permalink / raw)
To: Gabor Juhos, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
Dmitry Baryshkov, Danila Tikhonov, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <7f28584c-0e9a-44bf-b0c1-7f7a2acfb847@gmail.com>
On 30/03/2024 08:07, Gabor Juhos wrote:
> 2024. 03. 28. 8:45 keltezéssel, Krzysztof Kozlowski írta:
>> The first part of the compatible of Type-C node misses ending quote,
>> thus we have one long compatible consisting of two compatible strings
>> leading to dtbs_check warnings:
>>
>> sc7180-idp.dtb: usb-vbus-regulator@1100: compatible:0: 'qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
>> sc7180-idp.dtb: /soc@0/spmi@c440000/pmic@0/usb-vbus-regulator@1100: failed to match any schema with compatible: ['qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg']
>>
>> Reported-by: Rob Herring <robh@kernel.org>
>> Fixes: f81c2f01cad6 ("arm64: dts: qcom: pm6150: define USB-C related blocks")
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>> arch/arm64/boot/dts/qcom/pm6150.dtsi | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm6150.dtsi b/arch/arm64/boot/dts/qcom/pm6150.dtsi
>> index 11158c2bd524..b20a639cddf3 100644
>> --- a/arch/arm64/boot/dts/qcom/pm6150.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/pm6150.dtsi
>> @@ -71,8 +71,8 @@ pm6150_vbus: usb-vbus-regulator@1100 {
>> };
>>
>> pm6150_typec: typec@1500 {
>> - compatible = "qcom,pm6150-typec,
>> - qcom,pm8150b-typec";
>> + compatible = "qcom,pm6150-typec",
>> + "qcom,pm8150b-typec";
>
> The warning in the commit description is related to the
> 'usb-vbus-regulator@1100' node, whereas this fixes the compatible of a different
> one.
>
> Did you want to fix both?
D'oh! There are two of them! Thanks for the catch. Yes, I want to fix
both of them :)
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: pm6150: correct Type-C compatible
From: Gabor Juhos @ 2024-03-30 7:07 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
Dmitry Baryshkov, Danila Tikhonov, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <20240328074544.5076-1-krzysztof.kozlowski@linaro.org>
2024. 03. 28. 8:45 keltezéssel, Krzysztof Kozlowski írta:
> The first part of the compatible of Type-C node misses ending quote,
> thus we have one long compatible consisting of two compatible strings
> leading to dtbs_check warnings:
>
> sc7180-idp.dtb: usb-vbus-regulator@1100: compatible:0: 'qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
> sc7180-idp.dtb: /soc@0/spmi@c440000/pmic@0/usb-vbus-regulator@1100: failed to match any schema with compatible: ['qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg']
>
> Reported-by: Rob Herring <robh@kernel.org>
> Fixes: f81c2f01cad6 ("arm64: dts: qcom: pm6150: define USB-C related blocks")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/pm6150.dtsi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/pm6150.dtsi b/arch/arm64/boot/dts/qcom/pm6150.dtsi
> index 11158c2bd524..b20a639cddf3 100644
> --- a/arch/arm64/boot/dts/qcom/pm6150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm6150.dtsi
> @@ -71,8 +71,8 @@ pm6150_vbus: usb-vbus-regulator@1100 {
> };
>
> pm6150_typec: typec@1500 {
> - compatible = "qcom,pm6150-typec,
> - qcom,pm8150b-typec";
> + compatible = "qcom,pm6150-typec",
> + "qcom,pm8150b-typec";
The warning in the commit description is related to the
'usb-vbus-regulator@1100' node, whereas this fixes the compatible of a different
one.
Did you want to fix both?
Regards,
Gabor
^ permalink raw reply
* Re: [PATCH 00/23] v2: imx258 improvement series
From: Luigi311 @ 2024-03-30 6:37 UTC (permalink / raw)
To: Dang Huynh, linux-media
Cc: dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel
In-Reply-To: <4599295.LvFx2qVVIh@melttower>
On 3/29/24 21:51, Dang Huynh wrote:
> On Wednesday, March 27, 2024 11:16:46 PM UTC git@luigi311.com wrote:
>> From: Luigi311 <git@luigi311.com>
>
> The Linux kernel does not allow anonymous (or pseudonymous) contributions. You
> should use your real first and last name.
>
> See section 1.5 in "A guide to the Kernel Development Process":
> https://www.kernel.org/doc/html/v6.8/process/1.Intro.html#licensing
>
>
Ok I've changed my sign off to my real first and last name for the
next revision
^ permalink raw reply
* Re: [PATCH RFC v2 0/4] wifi: ath10k: support board-specific firmware overrides
From: Dmitry Baryshkov @ 2024-03-30 4:47 UTC (permalink / raw)
To: Kalle Valo, Jeff Johnson, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: ath10k, linux-wireless, netdev, devicetree, linux-arm-msm,
Krzysztof Kozlowski
In-Reply-To: <20240306-wcn3990-firmware-path-v2-0-f89e98e71a57@linaro.org>
On Wed, 6 Mar 2024 at 10:16, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
> modem DSP via the TQFTPserv. These MBN files are signed by the device
> vendor, can only be used with the particular SoC or device.
>
> Unfortunately different firmware versions come with different features.
> For example firmware for SDM845 doesn't use single-chan-info-per-channel
> feature, while firmware for QRB2210 / QRB4210 requires that feature.
>
> Allow board DT files to override the subdir of the fw dir used to lookup
> the firmware-N.bin file decribing corresponding WiFi firmware.
> For example, adding firmware-name = "qrb4210" property will make the
> driver look for the firmware-N.bin first in ath10k/WCN3990/hw1.0/qrb4210
> directory and then fallback to the default ath10k/WCN3990/hw1.0 dir.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> Changes in v2:
> - Fixed the comment about the default board name being NULL (Kalle)
> - Expanded commit message to provide examples for firmware paths (Kalle)
> - Added a note regarding board-2.bin to the commit message (Kalle)
> - Link to v1: https://lore.kernel.org/r/20240130-wcn3990-firmware-path-v1-0-826b93202964@linaro.org
>
> ---
> Dmitry Baryshkov (4):
> dt-bindings: net: wireless: ath10k: describe firmware-name property
> wifi: ath10k: support board-specific firmware overrides
> arm64: dts: qcom: qrb2210-rb1: add firmware-name qualifier to WiFi node
> arm64: dts: qcom: qrb4210-rb1: add firmware-name qualifier to WiFi node
Kalle, Jeff, is there anything pending on me on this series?
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH] sh: j2: drop incorrect SPI controller max frequency property
From: Rob Landley @ 2024-03-30 4:44 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, devicetree, linux-sh, linux-kernel
Cc: Kousik Sanagavarapu
In-Reply-To: <20240322064221.25776-1-krzysztof.kozlowski@linaro.org>
On 3/22/24 01:42, Krzysztof Kozlowski wrote:
> The J2 SPI controller bindings never allowed spi-max-frequency property
> in the controller node. Neither old spi-bus.txt bindings, nor new DT
> schema allows it. Linux driver does not parse that property from
> controller node, thus drop it from DTS as incorrect hardware
> description. The SPI child device has already the same property with
> the same value, so functionality should not be affected.
>
> Cc: Kousik Sanagavarapu <five231003@gmail.com>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Tested-by: Rob Landley <rob@landley.net>
(By which I mean a 6.8 kernel with this patch applied booted to a shell prompt
on my J2 turtle board.)
Rob
^ permalink raw reply
* [PATCH v2 18/18] PCI: rockchip-ep: Handle PERST# signal in endpoint mode
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
Currently, the Rockchip PCIe endpoint controller driver does not handle
PERST# signal, which prevents detecting when link training should
actually be started or if the host reset the device. This however can
be supported using the controller ep_gpio, set as an input GPIO for
endpoint mode.
Modify the endpoint rockchip driver to get the ep_gpio and its
associated interrupt which is serviced using a threaded IRQ with the
function rockchip_pcie_ep_perst_irq_thread() as handler.
This handler function notifies a link down event corresponding to the RC
side asserting the PERST# signal using pci_epc_linkdown() when the gpio
is high. Once the gpio value goes down, corresponding to the RC
de-asserting the PERST# signal, link training is started. The polarity
of the gpio interrupt trigger is changed from high to low after the RC
asserted PERST#, and conversely changed from low to high after the RC
de-asserts PERST#.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 118 +++++++++++++++++++++-
drivers/pci/controller/pcie-rockchip.c | 12 +--
2 files changed, 122 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 4006e7dee71a..e098c5fb6d59 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -18,6 +18,7 @@
#include <linux/sizes.h>
#include <linux/workqueue.h>
#include <linux/iopoll.h>
+#include <linux/gpio/consumer.h>
#include "pcie-rockchip.h"
@@ -50,6 +51,9 @@ struct rockchip_pcie_ep {
u64 irq_pci_addr;
u8 irq_pci_fn;
u8 irq_pending;
+ int perst_irq;
+ bool perst_asserted;
+ bool link_up;
struct delayed_work link_training;
};
@@ -464,13 +468,17 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc)
rockchip_pcie_write(rockchip, cfg, PCIE_CORE_PHY_FUNC_CFG);
+ if (rockchip->ep_gpio)
+ enable_irq(ep->perst_irq);
+
/* Enable configuration and start link training */
rockchip_pcie_write(rockchip,
PCIE_CLIENT_LINK_TRAIN_ENABLE |
PCIE_CLIENT_CONF_ENABLE,
PCIE_CLIENT_CONFIG);
- schedule_delayed_work(&ep->link_training, 0);
+ if (!rockchip->ep_gpio)
+ schedule_delayed_work(&ep->link_training, 0);
return 0;
}
@@ -480,6 +488,11 @@ static void rockchip_pcie_ep_stop(struct pci_epc *epc)
struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
struct rockchip_pcie *rockchip = &ep->rockchip;
+ if (rockchip->ep_gpio) {
+ ep->perst_asserted = true;
+ disable_irq(ep->perst_irq);
+ }
+
cancel_delayed_work_sync(&ep->link_training);
/* Stop link training and disable configuration */
@@ -542,6 +555,13 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work)
if (!rockchip_pcie_ep_link_up(rockchip))
goto again;
+ /*
+ * If PERST was asserted while polling the link, do not notify
+ * the function.
+ */
+ if (ep->perst_asserted)
+ return;
+
val = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS0);
dev_info(dev,
"Link UP (Negociated speed: %sGT/s, width: x%lu)\n",
@@ -551,6 +571,7 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work)
/* Notify the function */
pci_epc_linkup(ep->epc);
+ ep->link_up = true;
return;
@@ -558,6 +579,94 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work)
schedule_delayed_work(&ep->link_training, msecs_to_jiffies(5));
}
+static void rockchip_pcie_ep_perst_assert(struct rockchip_pcie_ep *ep)
+{
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+ struct device *dev = rockchip->dev;
+
+ dev_dbg(dev, "PERST asserted, link down\n");
+
+ if (ep->perst_asserted)
+ return;
+
+ ep->perst_asserted = true;
+
+ cancel_delayed_work_sync(&ep->link_training);
+
+ if (ep->link_up) {
+ pci_epc_linkdown(ep->epc);
+ ep->link_up = false;
+ }
+}
+
+static void rockchip_pcie_ep_perst_deassert(struct rockchip_pcie_ep *ep)
+{
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+ struct device *dev = rockchip->dev;
+
+ dev_dbg(dev, "PERST de-asserted, starting link training\n");
+
+ if (!ep->perst_asserted)
+ return;
+
+ ep->perst_asserted = false;
+
+ /* Enable link re-training */
+ rockchip_pcie_ep_retrain_link(rockchip);
+
+ /* Start link training */
+ schedule_delayed_work(&ep->link_training, 0);
+}
+
+static irqreturn_t rockchip_pcie_ep_perst_irq_thread(int irq, void *data)
+{
+ struct pci_epc *epc = data;
+ struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+ u32 perst = gpiod_get_value(rockchip->ep_gpio);
+
+ if (perst)
+ rockchip_pcie_ep_perst_assert(ep);
+ else
+ rockchip_pcie_ep_perst_deassert(ep);
+
+ irq_set_irq_type(ep->perst_irq,
+ (perst ? IRQF_TRIGGER_HIGH : IRQF_TRIGGER_LOW));
+
+ return IRQ_HANDLED;
+}
+
+static int rockchip_pcie_ep_setup_irq(struct pci_epc *epc)
+{
+ struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+ struct device *dev = rockchip->dev;
+ int ret;
+
+ if (!rockchip->ep_gpio)
+ return 0;
+
+ /* PCIe reset interrupt */
+ ep->perst_irq = gpiod_to_irq(rockchip->ep_gpio);
+ if (ep->perst_irq < 0) {
+ dev_err(dev, "No corresponding IRQ for PERST GPIO\n");
+ return ep->perst_irq;
+ }
+
+ ep->perst_asserted = true;
+ irq_set_status_flags(ep->perst_irq, IRQ_NOAUTOEN);
+ ret = devm_request_threaded_irq(dev, ep->perst_irq, NULL,
+ rockchip_pcie_ep_perst_irq_thread,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ "pcie-ep-perst", epc);
+ if (ret) {
+ dev_err(dev, "Request PERST GPIO IRQ failed %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
static const struct pci_epc_features rockchip_pcie_epc_features = {
.linkup_notifier = true,
.msi_capable = true,
@@ -721,6 +830,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
rockchip->is_rc = false;
rockchip->dev = dev;
INIT_DELAYED_WORK(&ep->link_training, rockchip_pcie_ep_link_training);
+ ep->link_up = false;
epc = devm_pci_epc_create(dev, &rockchip_pcie_epc_ops);
if (IS_ERR(epc)) {
@@ -748,7 +858,13 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
/* Only enable function 0 by default */
rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG);
+ err = rockchip_pcie_ep_setup_irq(epc);
+ if (err < 0)
+ goto err_uninit_port;
+
return 0;
+err_uninit_port:
+ rockchip_pcie_deinit_phys(rockchip);
err_release_resources:
rockchip_pcie_ep_release_resources(ep);
err_disable_clocks:
diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
index dbec700ba9f9..3938c0b6b5a9 100644
--- a/drivers/pci/controller/pcie-rockchip.c
+++ b/drivers/pci/controller/pcie-rockchip.c
@@ -119,13 +119,11 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
return PTR_ERR(rockchip->aclk_rst);
}
- if (rockchip->is_rc) {
- rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep",
- GPIOD_OUT_HIGH);
- if (IS_ERR(rockchip->ep_gpio))
- return dev_err_probe(dev, PTR_ERR(rockchip->ep_gpio),
- "failed to get ep GPIO\n");
- }
+ rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep",
+ rockchip->is_rc ? GPIOD_OUT_HIGH : GPIOD_IN);
+ if (IS_ERR(rockchip->ep_gpio))
+ return dev_err_probe(dev, PTR_ERR(rockchip->ep_gpio),
+ "failed to get ep GPIO\n");
rockchip->aclk_pcie = devm_clk_get(dev, "aclk");
if (IS_ERR(rockchip->aclk_pcie)) {
--
2.44.0
^ permalink raw reply related
* [PATCH v2 17/18] dt-bindings: pci: rockchip,rk3399-pcie-ep: Add ep-gpios property
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Describe the `ep-gpios` property which is used to map the PERST# input
signal for endpoint mode.
Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
.../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
index 6b62f6f58efe..9331d44d6963 100644
--- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
@@ -30,6 +30,9 @@ properties:
maximum: 32
default: 32
+ ep-gpios:
+ description: Input GPIO configured for the PERST# signal.
+
required:
- rockchip,max-outbound-regions
--
2.44.0
^ permalink raw reply related
* [PATCH v2 16/18] PCI: rockchip-ep: Improve link training
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
The Rockchip rk339 technical reference manual describe the endpoint mode
link training process clearly and states that:
Insure link training completion and success by observing link_st field
in PCIe Client BASIC_STATUS1 register change to 2'b11. If both side
support PCIe Gen2 speed, re-train can be Initiated by asserting the
Retrain Link field in Link Control and Status Register. The software
should insure the BASIC_STATUS0[negotiated_speed] changes to "1", that
indicates re-train to Gen2 successfully.
This procedure is very similar to what is done for the root-port mode in
rockchip_pcie_host_init_port().
Implement this link training procedure for the endpoint mode as well.
Given that the rk3399 SoC does not have an interrupt signaling link
status changes, training is implemented as a delayed work which is
rescheduled until the link training completes or the endpoint controller
is stopped. The link training work is first scheduled in
rockchip_pcie_ep_start() when the endpoint function is started. Link
training completion is signaled to the function using pci_epc_linkup().
Accordingly, the linkup_notifier field of the rockchip pci_epc_features
structure is changed to true.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 79 ++++++++++++++++++++++-
drivers/pci/controller/pcie-rockchip.h | 11 ++++
2 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 2767e8f1771d..4006e7dee71a 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -16,6 +16,8 @@
#include <linux/platform_device.h>
#include <linux/pci-epf.h>
#include <linux/sizes.h>
+#include <linux/workqueue.h>
+#include <linux/iopoll.h>
#include "pcie-rockchip.h"
@@ -48,6 +50,7 @@ struct rockchip_pcie_ep {
u64 irq_pci_addr;
u8 irq_pci_fn;
u8 irq_pending;
+ struct delayed_work link_training;
};
static void rockchip_pcie_clear_ep_ob_atu(struct rockchip_pcie *rockchip,
@@ -467,6 +470,8 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc)
PCIE_CLIENT_CONF_ENABLE,
PCIE_CLIENT_CONFIG);
+ schedule_delayed_work(&ep->link_training, 0);
+
return 0;
}
@@ -475,6 +480,8 @@ static void rockchip_pcie_ep_stop(struct pci_epc *epc)
struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
struct rockchip_pcie *rockchip = &ep->rockchip;
+ cancel_delayed_work_sync(&ep->link_training);
+
/* Stop link training and disable configuration */
rockchip_pcie_write(rockchip,
PCIE_CLIENT_CONF_DISABLE |
@@ -482,8 +489,77 @@ static void rockchip_pcie_ep_stop(struct pci_epc *epc)
PCIE_CLIENT_CONFIG);
}
+static void rockchip_pcie_ep_retrain_link(struct rockchip_pcie *rockchip)
+{
+ u32 status;
+
+ status = rockchip_pcie_read(rockchip, PCIE_EP_CONFIG_LCS);
+ status |= PCI_EXP_LNKCTL_RL;
+ rockchip_pcie_write(rockchip, status, PCIE_EP_CONFIG_LCS);
+}
+
+static bool rockchip_pcie_ep_link_up(struct rockchip_pcie *rockchip)
+{
+ u32 val = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS1);
+
+ return PCIE_LINK_UP(val);
+}
+
+static void rockchip_pcie_ep_link_training(struct work_struct *work)
+{
+ struct rockchip_pcie_ep *ep =
+ container_of(work, struct rockchip_pcie_ep, link_training.work);
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+ struct device *dev = rockchip->dev;
+ u32 val;
+ int ret;
+
+ /* Enable Gen1 training and wait for its completion */
+ ret = readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
+ val, PCIE_LINK_TRAINING_DONE(val), 50,
+ LINK_TRAIN_TIMEOUT);
+ if (ret)
+ goto again;
+
+ /* Make sure that the link is up */
+ ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_BASIC_STATUS1,
+ val, PCIE_LINK_UP(val), 50,
+ LINK_TRAIN_TIMEOUT);
+ if (ret)
+ goto again;
+
+ /* Check the current speed */
+ val = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL);
+ if (!PCIE_LINK_IS_GEN2(val) && rockchip->link_gen == 2) {
+ /* Enable retrain for gen2 */
+ rockchip_pcie_ep_retrain_link(rockchip);
+ readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
+ val, PCIE_LINK_IS_GEN2(val), 50,
+ LINK_TRAIN_TIMEOUT);
+ }
+
+ /* Check again that the link is up */
+ if (!rockchip_pcie_ep_link_up(rockchip))
+ goto again;
+
+ val = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS0);
+ dev_info(dev,
+ "Link UP (Negociated speed: %sGT/s, width: x%lu)\n",
+ (val & PCIE_CLIENT_NEG_LINK_SPEED) ? "5" : "2.5",
+ ((val & PCIE_CLIENT_NEG_LINK_WIDTH_MASK) >>
+ PCIE_CLIENT_NEG_LINK_WIDTH_SHIFT) << 1);
+
+ /* Notify the function */
+ pci_epc_linkup(ep->epc);
+
+ return;
+
+again:
+ schedule_delayed_work(&ep->link_training, msecs_to_jiffies(5));
+}
+
static const struct pci_epc_features rockchip_pcie_epc_features = {
- .linkup_notifier = false,
+ .linkup_notifier = true,
.msi_capable = true,
.msix_capable = false,
.align = ROCKCHIP_PCIE_AT_SIZE_ALIGN,
@@ -644,6 +720,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
rockchip = &ep->rockchip;
rockchip->is_rc = false;
rockchip->dev = dev;
+ INIT_DELAYED_WORK(&ep->link_training, rockchip_pcie_ep_link_training);
epc = devm_pci_epc_create(dev, &rockchip_pcie_epc_ops);
if (IS_ERR(epc)) {
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 0263f158ee8d..3963b7097a91 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -26,6 +26,7 @@
#define MAX_LANE_NUM 4
#define MAX_REGION_LIMIT 32
#define MIN_EP_APERTURE 28
+#define LINK_TRAIN_TIMEOUT (5000 * USEC_PER_MSEC)
#define PCIE_CLIENT_BASE 0x0
#define PCIE_CLIENT_CONFIG (PCIE_CLIENT_BASE + 0x00)
@@ -50,6 +51,10 @@
#define PCIE_CLIENT_DEBUG_LTSSM_MASK GENMASK(5, 0)
#define PCIE_CLIENT_DEBUG_LTSSM_L1 0x18
#define PCIE_CLIENT_DEBUG_LTSSM_L2 0x19
+#define PCIE_CLIENT_BASIC_STATUS0 (PCIE_CLIENT_BASE + 0x44)
+#define PCIE_CLIENT_NEG_LINK_WIDTH_MASK GENMASK(7, 6)
+#define PCIE_CLIENT_NEG_LINK_WIDTH_SHIFT 6
+#define PCIE_CLIENT_NEG_LINK_SPEED BIT(5)
#define PCIE_CLIENT_BASIC_STATUS1 (PCIE_CLIENT_BASE + 0x48)
#define PCIE_CLIENT_LINK_STATUS_UP 0x00300000
#define PCIE_CLIENT_LINK_STATUS_MASK 0x00300000
@@ -87,6 +92,8 @@
#define PCIE_CORE_CTRL_MGMT_BASE 0x900000
#define PCIE_CORE_CTRL (PCIE_CORE_CTRL_MGMT_BASE + 0x000)
+#define PCIE_CORE_PL_CONF_LS_MASK 0x00000001
+#define PCIE_CORE_PL_CONF_LS_READY 0x00000001
#define PCIE_CORE_PL_CONF_SPEED_5G 0x00000008
#define PCIE_CORE_PL_CONF_SPEED_MASK 0x00000018
#define PCIE_CORE_PL_CONF_LANE_MASK 0x00000006
@@ -144,6 +151,7 @@
#define PCIE_RC_CONFIG_BASE 0xa00000
#define PCIE_EP_CONFIG_BASE 0xa00000
#define PCIE_EP_CONFIG_DID_VID (PCIE_EP_CONFIG_BASE + 0x00)
+#define PCIE_EP_CONFIG_LCS (PCIE_EP_CONFIG_BASE + 0xd0)
#define PCIE_RC_CONFIG_RID_CCR (PCIE_RC_CONFIG_BASE + 0x08)
#define PCIE_RC_CONFIG_DCR (PCIE_RC_CONFIG_BASE + 0xc4)
#define PCIE_RC_CONFIG_DCR_CSPL_SHIFT 18
@@ -155,6 +163,7 @@
#define PCIE_RC_CONFIG_LINK_CAP (PCIE_RC_CONFIG_BASE + 0xcc)
#define PCIE_RC_CONFIG_LINK_CAP_L0S BIT(10)
#define PCIE_RC_CONFIG_LCS (PCIE_RC_CONFIG_BASE + 0xd0)
+#define PCIE_EP_CONFIG_LCS (PCIE_EP_CONFIG_BASE + 0xd0)
#define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
#define PCIE_RC_CONFIG_THP_CAP (PCIE_RC_CONFIG_BASE + 0x274)
#define PCIE_RC_CONFIG_THP_CAP_NEXT_MASK GENMASK(31, 20)
@@ -192,6 +201,8 @@
#define ROCKCHIP_VENDOR_ID 0x1d87
#define PCIE_LINK_IS_L2(x) \
(((x) & PCIE_CLIENT_DEBUG_LTSSM_MASK) == PCIE_CLIENT_DEBUG_LTSSM_L2)
+#define PCIE_LINK_TRAINING_DONE(x) \
+ (((x) & PCIE_CORE_PL_CONF_LS_MASK) == PCIE_CORE_PL_CONF_LS_READY)
#define PCIE_LINK_UP(x) \
(((x) & PCIE_CLIENT_LINK_STATUS_MASK) == PCIE_CLIENT_LINK_STATUS_UP)
#define PCIE_LINK_IS_GEN2(x) \
--
2.44.0
^ permalink raw reply related
* [PATCH v2 15/18] PCI: rockship-ep: Introduce rockchip_pcie_ep_stop()
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
Define the EPC operation ->stop for the rockchip endpoint driver with
the function rockchip_pcie_ep_stop(). This function disables link
training and the controller configuration, as the reverse to what
the start operation defined with rockchip_pcie_ep_start() does.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 13 +++++++++++++
drivers/pci/controller/pcie-rockchip.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 9215cac91f61..2767e8f1771d 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -470,6 +470,18 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc)
return 0;
}
+static void rockchip_pcie_ep_stop(struct pci_epc *epc)
+{
+ struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+
+ /* Stop link training and disable configuration */
+ rockchip_pcie_write(rockchip,
+ PCIE_CLIENT_CONF_DISABLE |
+ PCIE_CLIENT_LINK_TRAIN_DISABLE,
+ PCIE_CLIENT_CONFIG);
+}
+
static const struct pci_epc_features rockchip_pcie_epc_features = {
.linkup_notifier = false,
.msi_capable = true,
@@ -494,6 +506,7 @@ static const struct pci_epc_ops rockchip_pcie_epc_ops = {
.get_msi = rockchip_pcie_ep_get_msi,
.raise_irq = rockchip_pcie_ep_raise_irq,
.start = rockchip_pcie_ep_start,
+ .stop = rockchip_pcie_ep_stop,
.get_features = rockchip_pcie_ep_get_features,
};
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 30398156095f..0263f158ee8d 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -32,6 +32,7 @@
#define PCIE_CLIENT_CONF_ENABLE HIWORD_UPDATE_BIT(0x0001)
#define PCIE_CLIENT_CONF_DISABLE HIWORD_UPDATE(0x0001, 0)
#define PCIE_CLIENT_LINK_TRAIN_ENABLE HIWORD_UPDATE_BIT(0x0002)
+#define PCIE_CLIENT_LINK_TRAIN_DISABLE HIWORD_UPDATE(0x0002, 0)
#define PCIE_CLIENT_ARI_ENABLE HIWORD_UPDATE_BIT(0x0008)
#define PCIE_CLIENT_CONF_LANE_NUM(x) HIWORD_UPDATE(0x0030, ENCODE_LANES(x))
#define PCIE_CLIENT_MODE_RC HIWORD_UPDATE_BIT(0x0040)
--
2.44.0
^ permalink raw reply related
* [PATCH v2 14/18] PCI: rockchip-ep: Refactor endpoint link training enable
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
The function rockchip_pcie_init_port() enables link training for a
controller configured in EP mode. Enabling link training is again done
in rockchip_pcie_ep_probe() after that function executed
rockchip_pcie_init_port(). Enabling link training only needs to be done
once, and doing so at the probe stage before the controller is actually
started by the user serves no purpose.
Refactor this by removing the link training enablement from both
rockchip_pcie_init_port() and rockchip_pcie_ep_probe() and moving it to
the endpoint start operation defined with rockchip_pcie_ep_start().
Enabling the controller configuration using the PCIE_CLIENT_CONF_ENABLE
bit in the same PCIE_CLIENT_CONFIG register is also move to
rockchip_pcie_ep_start() and both the controller configuration and link
training enable bits are set with a single call to
rockchip_pcie_write().
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 14 ++++++--------
drivers/pci/controller/pcie-rockchip.c | 5 +++--
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index a7d008d95a8a..9215cac91f61 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -461,6 +461,12 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc)
rockchip_pcie_write(rockchip, cfg, PCIE_CORE_PHY_FUNC_CFG);
+ /* Enable configuration and start link training */
+ rockchip_pcie_write(rockchip,
+ PCIE_CLIENT_LINK_TRAIN_ENABLE |
+ PCIE_CLIENT_CONF_ENABLE,
+ PCIE_CLIENT_CONFIG);
+
return 0;
}
@@ -539,7 +545,6 @@ static int rockchip_pcie_ep_get_resources(struct rockchip_pcie_ep *ep)
ep->ob_addr = devm_kcalloc(dev, ep->max_regions, sizeof(*ep->ob_addr),
GFP_KERNEL);
-
if (!ep->ob_addr)
return -ENOMEM;
@@ -650,16 +655,9 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
rockchip_pcie_ep_hide_msix_cap(rockchip);
- /* Establish the link automatically */
- rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE,
- PCIE_CLIENT_CONFIG);
-
/* Only enable function 0 by default */
rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG);
- rockchip_pcie_write(rockchip, PCIE_CLIENT_CONF_ENABLE,
- PCIE_CLIENT_CONFIG);
-
return 0;
err_release_resources:
rockchip_pcie_ep_release_resources(ep);
diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
index 0ef2e622d36e..dbec700ba9f9 100644
--- a/drivers/pci/controller/pcie-rockchip.c
+++ b/drivers/pci/controller/pcie-rockchip.c
@@ -244,11 +244,12 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1,
PCIE_CLIENT_CONFIG);
- regs = PCIE_CLIENT_LINK_TRAIN_ENABLE | PCIE_CLIENT_ARI_ENABLE |
+ regs = PCIE_CLIENT_ARI_ENABLE |
PCIE_CLIENT_CONF_LANE_NUM(rockchip->lanes);
if (rockchip->is_rc)
- regs |= PCIE_CLIENT_CONF_ENABLE | PCIE_CLIENT_MODE_RC;
+ regs |= PCIE_CLIENT_LINK_TRAIN_ENABLE |
+ PCIE_CLIENT_CONF_ENABLE | PCIE_CLIENT_MODE_RC;
else
regs |= PCIE_CLIENT_CONF_DISABLE | PCIE_CLIENT_MODE_EP;
--
2.44.0
^ permalink raw reply related
* [PATCH v2 13/18] PCI: rockchip-ep: Refactor rockchip_pcie_ep_probe() MSI-X hiding
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
Move the code in rockchip_pcie_ep_probe() to hide the MSI-X capability
to its own function, rockchip_pcie_ep_hide_msix_cap(). No functional
changes.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 54 +++++++++++++----------
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 17d9fe48c621..a7d008d95a8a 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -583,6 +583,34 @@ static void rockchip_pcie_ep_release_resources(struct rockchip_pcie_ep *ep)
pci_epc_mem_exit(ep->epc);
}
+static void rockchip_pcie_ep_hide_msix_cap(struct rockchip_pcie *rockchip)
+{
+ u32 cfg_msi, cfg_msix_cp;
+
+ /*
+ * MSI-X is not supported but the controller still advertises the MSI-X
+ * capability by default, which can lead to the Root Complex side
+ * allocating MSI-X vectors which cannot be used. Avoid this by skipping
+ * the MSI-X capability entry in the PCIe capabilities linked-list: get
+ * the next pointer from the MSI-X entry and set that in the MSI
+ * capability entry (which is the previous entry). This way the MSI-X
+ * entry is skipped (left out of the linked-list) and not advertised.
+ */
+ cfg_msi = rockchip_pcie_read(rockchip, PCIE_EP_CONFIG_BASE +
+ ROCKCHIP_PCIE_EP_MSI_CTRL_REG);
+
+ cfg_msi &= ~ROCKCHIP_PCIE_EP_MSI_CP1_MASK;
+
+ cfg_msix_cp = rockchip_pcie_read(rockchip, PCIE_EP_CONFIG_BASE +
+ ROCKCHIP_PCIE_EP_MSIX_CAP_REG) &
+ ROCKCHIP_PCIE_EP_MSIX_CAP_CP_MASK;
+
+ cfg_msi |= cfg_msix_cp;
+
+ rockchip_pcie_write(rockchip, cfg_msi,
+ PCIE_EP_CONFIG_BASE + ROCKCHIP_PCIE_EP_MSI_CTRL_REG);
+}
+
static int rockchip_pcie_ep_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -590,7 +618,6 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
struct rockchip_pcie *rockchip;
struct pci_epc *epc;
int err;
- u32 cfg_msi, cfg_msix_cp;
ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
if (!ep)
@@ -621,6 +648,8 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
if (err)
goto err_disable_clocks;
+ rockchip_pcie_ep_hide_msix_cap(rockchip);
+
/* Establish the link automatically */
rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE,
PCIE_CLIENT_CONFIG);
@@ -628,29 +657,6 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
/* Only enable function 0 by default */
rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG);
- /*
- * MSI-X is not supported but the controller still advertises the MSI-X
- * capability by default, which can lead to the Root Complex side
- * allocating MSI-X vectors which cannot be used. Avoid this by skipping
- * the MSI-X capability entry in the PCIe capabilities linked-list: get
- * the next pointer from the MSI-X entry and set that in the MSI
- * capability entry (which is the previous entry). This way the MSI-X
- * entry is skipped (left out of the linked-list) and not advertised.
- */
- cfg_msi = rockchip_pcie_read(rockchip, PCIE_EP_CONFIG_BASE +
- ROCKCHIP_PCIE_EP_MSI_CTRL_REG);
-
- cfg_msi &= ~ROCKCHIP_PCIE_EP_MSI_CP1_MASK;
-
- cfg_msix_cp = rockchip_pcie_read(rockchip, PCIE_EP_CONFIG_BASE +
- ROCKCHIP_PCIE_EP_MSIX_CAP_REG) &
- ROCKCHIP_PCIE_EP_MSIX_CAP_CP_MASK;
-
- cfg_msi |= cfg_msix_cp;
-
- rockchip_pcie_write(rockchip, cfg_msi,
- PCIE_EP_CONFIG_BASE + ROCKCHIP_PCIE_EP_MSI_CTRL_REG);
-
rockchip_pcie_write(rockchip, PCIE_CLIENT_CONF_ENABLE,
PCIE_CLIENT_CONFIG);
--
2.44.0
^ permalink raw reply related
* [PATCH v2 12/18] PCI: rockchip-ep: Refactor rockchip_pcie_ep_probe() memory allocations
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
Introduce the function rockchip_pcie_ep_get_resources() to parse the DT
node of a rockchip PCIe endpoint controller and allocate the outbound
memory region and memory needed for IRQ handling. This function tidies
up rockchip_pcie_ep_probe(). No functional change.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 109 ++++++++++++----------
1 file changed, 62 insertions(+), 47 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 10fff395a13f..17d9fe48c621 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -526,15 +526,70 @@ static const struct of_device_id rockchip_pcie_ep_of_match[] = {
{},
};
+static int rockchip_pcie_ep_get_resources(struct rockchip_pcie_ep *ep)
+{
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+ struct device *dev = rockchip->dev;
+ struct pci_epc_mem_window *windows = NULL;
+ int err, i;
+
+ err = rockchip_pcie_parse_ep_dt(rockchip, ep);
+ if (err)
+ return err;
+
+ ep->ob_addr = devm_kcalloc(dev, ep->max_regions, sizeof(*ep->ob_addr),
+ GFP_KERNEL);
+
+ if (!ep->ob_addr)
+ return -ENOMEM;
+
+ windows = devm_kcalloc(dev, ep->max_regions,
+ sizeof(struct pci_epc_mem_window), GFP_KERNEL);
+ if (!windows)
+ return -ENOMEM;
+
+ for (i = 0; i < ep->max_regions; i++) {
+ windows[i].phys_base = rockchip->mem_res->start + (SZ_1M * i);
+ windows[i].size = SZ_1M;
+ windows[i].page_size = SZ_1M;
+ }
+ err = pci_epc_multi_mem_init(ep->epc, windows, ep->max_regions);
+ devm_kfree(dev, windows);
+
+ if (err < 0) {
+ dev_err(dev, "failed to initialize the memory space\n");
+ return err;
+ }
+
+ ep->irq_cpu_addr = pci_epc_mem_alloc_addr(ep->epc, &ep->irq_phys_addr,
+ SZ_1M);
+ if (!ep->irq_cpu_addr) {
+ dev_err(dev, "failed to reserve memory space for MSI\n");
+ goto err_epc_mem_exit;
+ }
+
+ ep->irq_pci_addr = ROCKCHIP_PCIE_EP_DUMMY_IRQ_ADDR;
+
+ return 0;
+
+err_epc_mem_exit:
+ pci_epc_mem_exit(ep->epc);
+
+ return err;
+}
+
+static void rockchip_pcie_ep_release_resources(struct rockchip_pcie_ep *ep)
+{
+ pci_epc_mem_exit(ep->epc);
+}
+
static int rockchip_pcie_ep_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct rockchip_pcie_ep *ep;
struct rockchip_pcie *rockchip;
struct pci_epc *epc;
- size_t max_regions;
- struct pci_epc_mem_window *windows = NULL;
- int err, i;
+ int err;
u32 cfg_msi, cfg_msix_cp;
ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
@@ -554,13 +609,13 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
ep->epc = epc;
epc_set_drvdata(epc, ep);
- err = rockchip_pcie_parse_ep_dt(rockchip, ep);
+ err = rockchip_pcie_ep_get_resources(ep);
if (err)
return err;
err = rockchip_pcie_enable_clocks(rockchip);
if (err)
- return err;
+ goto err_release_resources;
err = rockchip_pcie_init_port(rockchip);
if (err)
@@ -570,47 +625,9 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE,
PCIE_CLIENT_CONFIG);
- max_regions = ep->max_regions;
- ep->ob_addr = devm_kcalloc(dev, max_regions, sizeof(*ep->ob_addr),
- GFP_KERNEL);
-
- if (!ep->ob_addr) {
- err = -ENOMEM;
- goto err_uninit_port;
- }
-
/* Only enable function 0 by default */
rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG);
- windows = devm_kcalloc(dev, ep->max_regions,
- sizeof(struct pci_epc_mem_window), GFP_KERNEL);
- if (!windows) {
- err = -ENOMEM;
- goto err_uninit_port;
- }
- for (i = 0; i < ep->max_regions; i++) {
- windows[i].phys_base = rockchip->mem_res->start + (SZ_1M * i);
- windows[i].size = SZ_1M;
- windows[i].page_size = SZ_1M;
- }
- err = pci_epc_multi_mem_init(epc, windows, ep->max_regions);
- devm_kfree(dev, windows);
-
- if (err < 0) {
- dev_err(dev, "failed to initialize the memory space\n");
- goto err_uninit_port;
- }
-
- ep->irq_cpu_addr = pci_epc_mem_alloc_addr(epc, &ep->irq_phys_addr,
- SZ_1M);
- if (!ep->irq_cpu_addr) {
- dev_err(dev, "failed to reserve memory space for MSI\n");
- err = -ENOMEM;
- goto err_epc_mem_exit;
- }
-
- ep->irq_pci_addr = ROCKCHIP_PCIE_EP_DUMMY_IRQ_ADDR;
-
/*
* MSI-X is not supported but the controller still advertises the MSI-X
* capability by default, which can lead to the Root Complex side
@@ -638,10 +655,8 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
PCIE_CLIENT_CONFIG);
return 0;
-err_epc_mem_exit:
- pci_epc_mem_exit(epc);
-err_uninit_port:
- rockchip_pcie_deinit_phys(rockchip);
+err_release_resources:
+ rockchip_pcie_ep_release_resources(ep);
err_disable_clocks:
rockchip_pcie_disable_clocks(rockchip);
return err;
--
2.44.0
^ permalink raw reply related
* [PATCH v2 11/18] PCI: rockchip-ep: Implement the map_align endpoint controller operation
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
The rockchip PCIe endpoint controller handles PCIe transfers addresses
by masking the lower bits of the programmed PCI address and using the
same number of lower bits from the CPU address space used for the
mapping. For a PCI mapping of size bytes starting from pci_addr, the
number of bits masked is the number of address bits changing in the
address range [pci_addr..pci_addr + size - 1], up to 20 bits, that is,
up to 1MB mappings.
This means that when preparing a PCI address mapping, an endpoint
function driver must use an offset into the allocated controller
memory region that is equal to the mask of the starting PCI address
over rockchip_pcie_ep_ob_atu_num_bits() bits. This offset also
determines the maximum size of the mapping given the starting PCI
address and the fixed 1MB controller memory window size.
Implement the ->map_align() endpoint controller operation to allow the
mapping alignment to be transparently handled by endpoint function
drivers through the function pci_epc_map_align().
This commit contains contributions from Rick Wertenbroek
<rick.wertenbroek@gmail.com>.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 22 ++++++++++++++++++++++
drivers/pci/controller/pcie-rockchip.h | 5 +++++
2 files changed, 27 insertions(+)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 36692e34ca31..10fff395a13f 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -237,6 +237,27 @@ static inline u32 rockchip_ob_region(phys_addr_t addr)
return (addr >> ilog2(SZ_1M)) & 0x1f;
}
+static int rockchip_pcie_ep_map_align(struct pci_epc *epc, u8 fn, u8 vfn,
+ struct pci_epc_map *map)
+{
+ struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
+ int num_bits;
+
+ num_bits = rockchip_pcie_ep_ob_atu_num_bits(&ep->rockchip,
+ map->pci_addr, map->pci_size);
+
+ map->map_pci_addr = map->pci_addr & ~((1ULL << num_bits) - 1);
+ map->map_ofst = map->pci_addr - map->map_pci_addr;
+
+ if (map->map_ofst + map->pci_size > SZ_1M)
+ map->pci_size = SZ_1M - map->map_ofst;
+
+ map->map_size = ALIGN(map->map_ofst + map->pci_size,
+ ROCKCHIP_PCIE_AT_SIZE_ALIGN);
+
+ return 0;
+}
+
static int rockchip_pcie_ep_map_addr(struct pci_epc *epc, u8 fn, u8 vfn,
phys_addr_t addr, u64 pci_addr,
size_t size)
@@ -460,6 +481,7 @@ static const struct pci_epc_ops rockchip_pcie_epc_ops = {
.write_header = rockchip_pcie_ep_write_header,
.set_bar = rockchip_pcie_ep_set_bar,
.clear_bar = rockchip_pcie_ep_clear_bar,
+ .map_align = rockchip_pcie_ep_map_align,
.map_addr = rockchip_pcie_ep_map_addr,
.unmap_addr = rockchip_pcie_ep_unmap_addr,
.set_msi = rockchip_pcie_ep_set_msi,
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 02368ce9bd54..30398156095f 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -241,6 +241,11 @@
#define ROCKCHIP_PCIE_EP_MSIX_CAP_CP_MASK GENMASK(15, 8)
#define ROCKCHIP_PCIE_EP_DUMMY_IRQ_ADDR 0x1
#define ROCKCHIP_PCIE_EP_PCI_LEGACY_IRQ_ADDR 0x3
+
+#define ROCKCHIP_PCIE_AT_MIN_NUM_BITS 8
+#define ROCKCHIP_PCIE_AT_MAX_NUM_BITS 20
+#define ROCKCHIP_PCIE_AT_SIZE_ALIGN (1UL << ROCKCHIP_PCIE_AT_MIN_NUM_BITS)
+
#define ROCKCHIP_PCIE_EP_FUNC_BASE(fn) \
(PCIE_EP_PF_CONFIG_REGS_BASE + (((fn) << 12) & GENMASK(19, 12)))
#define ROCKCHIP_PCIE_EP_VIRT_FUNC_BASE(fn) \
--
2.44.0
^ permalink raw reply related
* [PATCH v2 10/18] PCI: rockchip-ep: Improve rockchip_pcie_ep_map_addr()
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
Add a check to verify that the outbound region to be used for mapping an
address is not already in use.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 93c2466d6fef..36692e34ca31 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -245,6 +245,9 @@ static int rockchip_pcie_ep_map_addr(struct pci_epc *epc, u8 fn, u8 vfn,
struct rockchip_pcie *pcie = &ep->rockchip;
u32 r = rockchip_ob_region(addr);
+ if (test_bit(r, &ep->ob_region_map))
+ return -EBUSY;
+
rockchip_pcie_prog_ep_ob_atu(pcie, fn, r, addr, pci_addr, size);
set_bit(r, &ep->ob_region_map);
--
2.44.0
^ permalink raw reply related
* [PATCH v2 09/18] PCI: rockchip-ep: Improve rockchip_pcie_ep_unmap_addr()
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
There is no need to loop over all regions to find the memory window used
to map an address. We can use rockchip_ob_region() to determine the
region index, together with a check that the address passed as argument
is the address used to create the mapping. Furthermore, the
ob_region_map bitmap should also be checked to ensure that we are not
attempting to unmap an address that is not mapped.
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
drivers/pci/controller/pcie-rockchip-ep.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index f8c26606df58..93c2466d6fef 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -258,13 +258,9 @@ static void rockchip_pcie_ep_unmap_addr(struct pci_epc *epc, u8 fn, u8 vfn,
{
struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
struct rockchip_pcie *rockchip = &ep->rockchip;
- u32 r;
-
- for (r = 0; r < ep->max_regions; r++)
- if (ep->ob_addr[r] == addr)
- break;
+ u32 r = rockchip_ob_region(addr);
- if (r == ep->max_regions)
+ if (addr != ep->ob_addr[r] || !test_bit(r, &ep->ob_region_map))
return;
rockchip_pcie_clear_ep_ob_atu(rockchip, r);
--
2.44.0
^ permalink raw reply related
* [PATCH v2 08/18] PCI: rockchip-ep: Use a macro to define EP controller .align feature
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
Introduce the macro ROCKCHIP_PCIE_AT_SIZE_ALIGN defined using
ROCKCHIP_PCIE_AT_MIN_NUM_BITS to initialize the .align field of the
controller epc_features structure, avoiding using the "magic" value 8
directly.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
drivers/pci/controller/pcie-rockchip.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 786efd918b3f..f8c26606df58 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -448,7 +448,7 @@ static const struct pci_epc_features rockchip_pcie_epc_features = {
.linkup_notifier = false,
.msi_capable = true,
.msix_capable = false,
- .align = 256,
+ .align = ROCKCHIP_PCIE_AT_SIZE_ALIGN,
};
static const struct pci_epc_features*
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 15ee949f2485..02368ce9bd54 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -248,6 +248,7 @@
#define ROCKCHIP_PCIE_AT_MIN_NUM_BITS 8
#define ROCKCHIP_PCIE_AT_MAX_NUM_BITS 20
+#define ROCKCHIP_PCIE_AT_SIZE_ALIGN (1UL << ROCKCHIP_PCIE_AT_MIN_NUM_BITS)
#define ROCKCHIP_PCIE_AT_IB_EP_FUNC_BAR_ADDR0(fn, bar) \
(PCIE_CORE_AXI_CONF_BASE + 0x0828 + (fn) * 0x0040 + (bar) * 0x0008)
--
2.44.0
^ permalink raw reply related
* [PATCH v2 07/18] PCI: rockchip-ep: Fix address translation unit programming
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
The rockchip PCIe endpoint controller handles PCIe transfers addresses
by masking the lower bits of the programmed PCI address and using the
same number of lower bits masked from the CPU address space used for the
mapping. For a PCI mapping of <size> bytes starting from <pci_addr>,
the number of bits masked is the number of address bits changing in the
address range [pci_addr..pci_addr + size - 1].
However, rockchip_pcie_prog_ep_ob_atu() calculates num_pass_bits only
using the size of the mapping, resulting in an incorrect number of mask
bits depending on the value of the PCI address to map.
Fix this by introducing the helper function
rockchip_pcie_ep_ob_atu_num_bits() to correctly calculate the number of
mask bits to use to program the address translation unit. The number of
mask bits iscalculated depending on both the PCI address and size of the
mapping, and clamped between 8 and 20 using the macros
ROCKCHIP_PCIE_AT_MIN_NUM_BITS and ROCKCHIP_PCIE_AT_MAX_NUM_BITS.
Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
Cc: stable@vger.kernel.org
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 15 +++++++++++----
drivers/pci/controller/pcie-rockchip.h | 4 ++++
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index c9046e97a1d2..786efd918b3f 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -63,16 +63,23 @@ static void rockchip_pcie_clear_ep_ob_atu(struct rockchip_pcie *rockchip,
ROCKCHIP_PCIE_AT_OB_REGION_DESC1(region));
}
+static int rockchip_pcie_ep_ob_atu_num_bits(struct rockchip_pcie *rockchip,
+ u64 pci_addr, size_t size)
+{
+ int num_pass_bits = fls64(pci_addr ^ (pci_addr + size - 1));
+
+ return clamp(num_pass_bits, ROCKCHIP_PCIE_AT_MIN_NUM_BITS,
+ ROCKCHIP_PCIE_AT_MAX_NUM_BITS);
+}
+
static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
u32 r, u64 cpu_addr, u64 pci_addr,
size_t size)
{
- int num_pass_bits = fls64(size - 1);
+ int num_pass_bits =
+ rockchip_pcie_ep_ob_atu_num_bits(rockchip, pci_addr, size);
u32 addr0, addr1, desc0;
- if (num_pass_bits < 8)
- num_pass_bits = 8;
-
addr0 = ((num_pass_bits - 1) & PCIE_CORE_OB_REGION_ADDR0_NUM_BITS) |
(lower_32_bits(pci_addr) & PCIE_CORE_OB_REGION_ADDR0_LO_ADDR);
addr1 = upper_32_bits(pci_addr);
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 6111de35f84c..15ee949f2485 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -245,6 +245,10 @@
(PCIE_EP_PF_CONFIG_REGS_BASE + (((fn) << 12) & GENMASK(19, 12)))
#define ROCKCHIP_PCIE_EP_VIRT_FUNC_BASE(fn) \
(PCIE_EP_PF_CONFIG_REGS_BASE + 0x10000 + (((fn) << 12) & GENMASK(19, 12)))
+
+#define ROCKCHIP_PCIE_AT_MIN_NUM_BITS 8
+#define ROCKCHIP_PCIE_AT_MAX_NUM_BITS 20
+
#define ROCKCHIP_PCIE_AT_IB_EP_FUNC_BAR_ADDR0(fn, bar) \
(PCIE_CORE_AXI_CONF_BASE + 0x0828 + (fn) * 0x0040 + (bar) * 0x0008)
#define ROCKCHIP_PCIE_AT_IB_EP_FUNC_BAR_ADDR1(fn, bar) \
--
2.44.0
^ permalink raw reply related
* [PATCH v2 06/18] PCI: endpoint: test: Implement link_down event operation
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
Implement the link_down event operation to stop the command execution
delayed work when the endpoint controller notifies a link down event.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
drivers/pci/endpoint/functions/pci-epf-test.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index ab40c3182677..e6d4e1747c9f 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -824,9 +824,19 @@ static int pci_epf_test_link_up(struct pci_epf *epf)
return 0;
}
+static int pci_epf_test_link_down(struct pci_epf *epf)
+{
+ struct pci_epf_test *epf_test = epf_get_drvdata(epf);
+
+ cancel_delayed_work_sync(&epf_test->cmd_handler);
+
+ return 0;
+}
+
static const struct pci_epc_event_ops pci_epf_test_event_ops = {
.core_init = pci_epf_test_core_init,
.link_up = pci_epf_test_link_up,
+ .link_down = pci_epf_test_link_down,
};
static int pci_epf_test_alloc_space(struct pci_epf *epf)
--
2.44.0
^ permalink raw reply related
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