* [PATCH v3 2/5] clk: qcom: clk-alpha-pll: Add HUAYRA_2290 support
From: Konrad Dybcio @ 2024-03-26 21:08 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Marijn Suijten, Konrad Dybcio, linux-arm-msm, linux-clk,
devicetree, linux-kernel, Konrad Dybcio, Dmitry Baryshkov
In-Reply-To: <20240219-topic-rb1_gpu-v3-0-86f67786539a@linaro.org>
Commit 134b55b7e19f ("clk: qcom: support Huayra type Alpha PLL")
introduced an entry to the alpha offsets array, but diving into QCM2290
downstream and some documentation, it turned out that the name Huayra
apparently has been used quite liberally across many chips, even with
noticeably different hardware.
Introduce another set of offsets and a new configure function for the
Huayra PLL found on QCM2290. This is required e.g. for the consumers
of GPUCC_PLL0 to properly start.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/clk/qcom/clk-alpha-pll.c | 47 ++++++++++++++++++++++++++++++++++++++++
drivers/clk/qcom/clk-alpha-pll.h | 3 +++
2 files changed, 50 insertions(+)
diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index 8a412ef47e16..27ba8aa3e577 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -83,6 +83,19 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = {
[PLL_OFF_TEST_CTL_U] = 0x20,
[PLL_OFF_STATUS] = 0x24,
},
+ [CLK_ALPHA_PLL_TYPE_HUAYRA_2290] = {
+ [PLL_OFF_L_VAL] = 0x04,
+ [PLL_OFF_ALPHA_VAL] = 0x08,
+ [PLL_OFF_USER_CTL] = 0x0c,
+ [PLL_OFF_CONFIG_CTL] = 0x10,
+ [PLL_OFF_CONFIG_CTL_U] = 0x14,
+ [PLL_OFF_CONFIG_CTL_U1] = 0x18,
+ [PLL_OFF_TEST_CTL] = 0x1c,
+ [PLL_OFF_TEST_CTL_U] = 0x20,
+ [PLL_OFF_TEST_CTL_U1] = 0x24,
+ [PLL_OFF_OPMODE] = 0x28,
+ [PLL_OFF_STATUS] = 0x38,
+ },
[CLK_ALPHA_PLL_TYPE_BRAMMO] = {
[PLL_OFF_L_VAL] = 0x04,
[PLL_OFF_ALPHA_VAL] = 0x08,
@@ -779,6 +792,40 @@ static long clk_alpha_pll_round_rate(struct clk_hw *hw, unsigned long rate,
return clamp(rate, min_freq, max_freq);
}
+void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
+ const struct alpha_pll_config *config)
+{
+ u32 val;
+
+ clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL(pll), config->config_ctl_val);
+ clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U(pll), config->config_ctl_hi_val);
+ clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U1(pll), config->config_ctl_hi1_val);
+ clk_alpha_pll_write_config(regmap, PLL_TEST_CTL(pll), config->test_ctl_val);
+ clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U(pll), config->test_ctl_hi_val);
+ clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U1(pll), config->test_ctl_hi1_val);
+ clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l);
+ clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha);
+ clk_alpha_pll_write_config(regmap, PLL_USER_CTL(pll), config->user_ctl_val);
+
+ /* Set PLL_BYPASSNL */
+ regmap_update_bits(regmap, PLL_MODE(pll), PLL_BYPASSNL, PLL_BYPASSNL);
+ regmap_read(regmap, PLL_MODE(pll), &val);
+
+ /* Wait 5 us between setting BYPASS and deasserting reset */
+ udelay(5);
+
+ /* Take PLL out from reset state */
+ regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N);
+ regmap_read(regmap, PLL_MODE(pll), &val);
+
+ /* Wait 50us for PLL_LOCK_DET bit to go high */
+ usleep_range(50, 55);
+
+ /* Enable PLL output */
+ regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, PLL_OUTCTRL);
+}
+EXPORT_SYMBOL_GPL(clk_huayra_2290_pll_configure);
+
static unsigned long
alpha_huayra_pll_calc_rate(u64 prate, u32 l, u32 a)
{
diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index fb6d50263bb9..d1cd52158c17 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -15,6 +15,7 @@
enum {
CLK_ALPHA_PLL_TYPE_DEFAULT,
CLK_ALPHA_PLL_TYPE_HUAYRA,
+ CLK_ALPHA_PLL_TYPE_HUAYRA_2290,
CLK_ALPHA_PLL_TYPE_BRAMMO,
CLK_ALPHA_PLL_TYPE_FABIA,
CLK_ALPHA_PLL_TYPE_TRION,
@@ -191,6 +192,8 @@ extern const struct clk_ops clk_alpha_pll_rivian_evo_ops;
void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
const struct alpha_pll_config *config);
+void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
+ const struct alpha_pll_config *config);
void clk_fabia_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
const struct alpha_pll_config *config);
void clk_trion_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
--
2.44.0
^ permalink raw reply related
* [PATCH v3 1/5] dt-bindings: clock: Add Qcom QCM2290 GPUCC
From: Konrad Dybcio @ 2024-03-26 21:08 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Marijn Suijten, Konrad Dybcio, linux-arm-msm, linux-clk,
devicetree, linux-kernel, Konrad Dybcio, Krzysztof Kozlowski
In-Reply-To: <20240219-topic-rb1_gpu-v3-0-86f67786539a@linaro.org>
Add device tree bindings for graphics clock controller for Qualcomm
Technology Inc's QCM2290 SoCs.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
.../bindings/clock/qcom,qcm2290-gpucc.yaml | 77 ++++++++++++++++++++++
include/dt-bindings/clock/qcom,qcm2290-gpucc.h | 32 +++++++++
2 files changed, 109 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
new file mode 100644
index 000000000000..734880805c1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,qcm2290-gpucc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Graphics Clock & Reset Controller on QCM2290
+
+maintainers:
+ - Konrad Dybcio <konradybcio@kernel.org>
+
+description: |
+ Qualcomm graphics clock control module provides the clocks, resets and power
+ domains on Qualcomm SoCs.
+
+ See also::
+ include/dt-bindings/clock/qcom,qcm2290-gpucc.h
+
+properties:
+ compatible:
+ const: qcom,qcm2290-gpucc
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: AHB interface clock,
+ - description: SoC CXO clock
+ - description: GPLL0 main branch source
+ - description: GPLL0 div branch source
+
+ power-domains:
+ description:
+ A phandle and PM domain specifier for the CX power domain.
+ maxItems: 1
+
+ required-opps:
+ description:
+ A phandle to an OPP node describing required CX performance point.
+ maxItems: 1
+
+required:
+ - compatible
+ - clocks
+ - power-domains
+
+allOf:
+ - $ref: qcom,gcc.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,gcc-qcm2290.h>
+ #include <dt-bindings/clock/qcom,rpmcc.h>
+ #include <dt-bindings/power/qcom-rpmpd.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ clock-controller@5990000 {
+ compatible = "qcom,qcm2290-gpucc";
+ reg = <0x0 0x05990000 0x0 0x9000>;
+ clocks = <&gcc GCC_GPU_CFG_AHB_CLK>,
+ <&rpmcc RPM_SMD_XO_CLK_SRC>,
+ <&gcc GCC_GPU_GPLL0_CLK_SRC>,
+ <&gcc GCC_GPU_GPLL0_DIV_CLK_SRC>;
+ power-domains = <&rpmpd QCM2290_VDDCX>;
+ required-opps = <&rpmpd_opp_low_svs>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ #power-domain-cells = <1>;
+ };
+ };
+...
diff --git a/include/dt-bindings/clock/qcom,qcm2290-gpucc.h b/include/dt-bindings/clock/qcom,qcm2290-gpucc.h
new file mode 100644
index 000000000000..7c76dd05278f
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,qcm2290-gpucc.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Linaro Limited
+ */
+
+#ifndef _DT_BINDINGS_CLK_QCOM_GPU_CC_QCM2290_H
+#define _DT_BINDINGS_CLK_QCOM_GPU_CC_QCM2290_H
+
+/* GPU_CC clocks */
+#define GPU_CC_AHB_CLK 0
+#define GPU_CC_CRC_AHB_CLK 1
+#define GPU_CC_CX_GFX3D_CLK 2
+#define GPU_CC_CX_GMU_CLK 3
+#define GPU_CC_CX_SNOC_DVM_CLK 4
+#define GPU_CC_CXO_AON_CLK 5
+#define GPU_CC_CXO_CLK 6
+#define GPU_CC_GMU_CLK_SRC 7
+#define GPU_CC_GX_GFX3D_CLK 8
+#define GPU_CC_GX_GFX3D_CLK_SRC 9
+#define GPU_CC_PLL0 10
+#define GPU_CC_SLEEP_CLK 11
+#define GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK 12
+
+/* Resets */
+#define GPU_GX_BCR 0
+
+/* GDSCs */
+#define GPU_CX_GDSC 0
+#define GPU_GX_GDSC 1
+
+#endif
--
2.44.0
^ permalink raw reply related
* [PATCH v3 0/5] A702 support
From: Konrad Dybcio @ 2024-03-26 21:08 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Marijn Suijten, Konrad Dybcio, linux-arm-msm, linux-clk,
devicetree, linux-kernel, Konrad Dybcio, Krzysztof Kozlowski,
Dmitry Baryshkov
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Changes in v3:
- Use EXPORT_SYMBOL_GPL in the alpha pll change
- Drop applied patches
- Pick up tags
- Link to v2: https://lore.kernel.org/r/20240219-topic-rb1_gpu-v2-0-2d3d6a0db040@linaro.org
Changes in v2:
- Drop applied smmu-bindings patch
- Fix the gpucc bindings patch to be even better
- Reorder HUAYRA_2290 definitions near HUAYRA (..Add HUAYRA_2290
support..)
- Replace weird memory barriers copypasted from msm-5.4 with readback to
ensure timely write completion (..Add HUAYRA_2290 support..)
- Keep my super amazing commit message referencing the 3D accelerator
official naming (dts)
- Pick up tags
- Link to v1: https://lore.kernel.org/r/20240219-topic-rb1_gpu-v1-0-d260fa854707@linaro.org
---
Konrad Dybcio (5):
dt-bindings: clock: Add Qcom QCM2290 GPUCC
clk: qcom: clk-alpha-pll: Add HUAYRA_2290 support
clk: qcom: Add QCM2290 GPU clock controller driver
arm64: dts: qcom: qcm2290: Add GPU nodes
arm64: dts: qcom: qrb2210-rb1: Enable the GPU
.../bindings/clock/qcom,qcm2290-gpucc.yaml | 77 ++++
arch/arm64/boot/dts/qcom/qcm2290.dtsi | 154 ++++++++
arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 8 +
drivers/clk/qcom/Kconfig | 9 +
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/clk-alpha-pll.c | 47 +++
drivers/clk/qcom/clk-alpha-pll.h | 3 +
drivers/clk/qcom/gpucc-qcm2290.c | 423 +++++++++++++++++++++
include/dt-bindings/clock/qcom,qcm2290-gpucc.h | 32 ++
9 files changed, 754 insertions(+)
---
base-commit: 084c8e315db34b59d38d06e684b1a0dd07d30287
change-id: 20240219-topic-rb1_gpu-3ec8c6830384
Best regards,
--
Konrad Dybcio <konrad.dybcio@linaro.org>
^ permalink raw reply
* Re: [PATCH RFT 0/7] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
From: Konrad Dybcio @ 2024-03-26 21:02 UTC (permalink / raw)
To: Bjorn Andersson, Neil Armstrong
Cc: Luca Weiss, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Abhinav Kumar, linux-arm-msm,
linux-phy, devicetree, linux-kernel
In-Reply-To: <liah4xvkfattlen7s2zi3vt2bl5pbbxqgig3k5ljqpveoao656@iacnommxkjkt>
On 16.03.2024 5:01 PM, Bjorn Andersson wrote:
> On Fri, Mar 15, 2024 at 06:35:15PM +0100, Neil Armstrong wrote:
>> On 15/03/2024 18:19, Luca Weiss wrote:
>>> On Thu Feb 29, 2024 at 2:07 PM CET, Neil Armstrong wrote:
>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>
>>>> The DisplayPort phy should be left enabled if is still powered on
>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>> PHY is not powered off.
>>>>
>>>> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
>>>> will be set in between of USB-Only, Combo and DisplayPort Only so
>>>> this will leave enough time to the DRM DisplayPort controller to
>>>> turn of the DisplayPort PHY.
>>>>
>>>> The patchset also includes bindings changes and DT changes.
>>>>
>>>> This has been successfully tested on an SM8550 board, but the
>>>> Thinkpad X13s deserved testing between non-PD USB, non-PD DisplayPort,
>>>> PD USB Hubs and PD Altmode Dongles to make sure the switch works
>>>> as expected.
>>>>
>>>> The DisplayPort 4 lanes setup can be check with:
>>>> $ cat /sys/kernel/debug/dri/ae01000.display-controller/DP-1/dp_debug
>>>> name = msm_dp
>>>> drm_dp_link
>>>> rate = 540000
>>>> num_lanes = 4
>>>
>>> Hi Neil,
>>>
>>> I tried this on QCM6490/SC7280 which should also support 4-lane DP but I
>>> haven't had any success so far.
>>>
> [..]
>>> [ 1775.563969] [drm:dp_ctrl_link_train] *ERROR* max v_level reached
>>> [ 1775.564031] [drm:dp_ctrl_link_train] *ERROR* link training #1 failed. ret=-11
>>
>> Interesting #1 means the 4 lanes are not physically connected to the other side,
>> perhaps QCM6490/SC7280 requires a specific way to enable the 4 lanes in the PHY,
>> or some fixups in the init tables.
>>
>
> I tested the same on rb3gen2 (qcs6490) a couple of weeks ago, with the
> same outcome. Looking at the AUX reads, after switching to 4-lane the
> link training is failing on all 4 lanes, in contrast to succeeding only
> on the first 2 if you e.g. forget to mux the other two.
>
> As such, my expectation is that there's something wrong in the QMP PHY
> (or possibly redriver) for this platform.
Do we have any downstream tag where 4lane dp works? I'm willing to believe
the PHY story..
Konrad
^ permalink raw reply
* Re: [PATCH v4 1/7] iio: accel: adxl345: Make data_range obsolete
From: Lothar Rubusch @ 2024-03-26 20:59 UTC (permalink / raw)
To: Jonathan Cameron
Cc: lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt,
conor+dt, linux-iio, devicetree, linux-kernel, eraretuya
In-Reply-To: <20240325203155.23ddfe3d@jic23-huawei>
On Mon, Mar 25, 2024 at 9:32 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 25 Mar 2024 15:33:50 +0000
> Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> > Replace write() data_format by regmap_update_bits(), because
> > bus specific pre-configuration may have happened before on
> > the same register. Changes then need to be masked.
> >
> > Remove the data_range field from the struct adxl345_data,
> > because it is not used anymore.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > ---
> > drivers/iio/accel/adxl345_core.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > index 8bd30a23e..be6758015 100644
> > --- a/drivers/iio/accel/adxl345_core.c
> > +++ b/drivers/iio/accel/adxl345_core.c
> > @@ -42,13 +42,13 @@
> > #define ADXL345_DATA_FORMAT_4G 1
> > #define ADXL345_DATA_FORMAT_8G 2
> > #define ADXL345_DATA_FORMAT_16G 3
> > +#define ADXL345_DATA_FORMAT_MSK ~((u8) BIT(6)) /* ignore spi-3wire */
>
> I'm not keen on seeing masking of a bit we don't yet
> handle done by value. Can we instead build this up by what we 'want' to
> write rather than don't. Will need a few more defines perhaps to cover
> the masks of SELF_TEST, INT_INVERT, FULL_RES, Justify and Range.
>
Good point. Anyway, there is also an input driver implementation for
the adxl345, mainly dealing with the interrupt feature as input
device. Thus, for the iio implementation I would suggest to reduce the
mask just to cover SELF_TEST and FULL_RES and leave INT_INVERT out. Is
this ok?
> >
> > #define ADXL345_DEVID 0xE5
> >
> > struct adxl345_data {
> > const struct adxl345_chip_info *info;
> > struct regmap *regmap;
> > - u8 data_range;
> > };
> >
> > #define ADXL345_CHANNEL(index, axis) { \
> > @@ -219,14 +219,13 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap)
> > data = iio_priv(indio_dev);
> > data->regmap = regmap;
> > /* Enable full-resolution mode */
> > - data->data_range = ADXL345_DATA_FORMAT_FULL_RES;
> > data->info = device_get_match_data(dev);
> > if (!data->info)
> > return -ENODEV;
> >
> > - ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT,
> > - data->data_range);
> > - if (ret < 0)
> > + ret = regmap_update_bits(regmap, ADXL345_REG_DATA_FORMAT,
> > + ADXL345_DATA_FORMAT_MSK, ADXL345_DATA_FORMAT_FULL_RES);
> > + if (ret)
> > return dev_err_probe(dev, ret, "Failed to set data range\n");
> >
> > indio_dev->name = data->info->name;
>
^ permalink raw reply
* [PATCH 3/4] arm64: dts: ti: k3-j721e: Use exact ranges for FSS node
From: Andrew Davis @ 2024-03-26 20:59 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: <20240326205920.40147-1-afd@ti.com>
The FSS bus contains several register ranges. Using an empty
ranges property works but causes a DT warning when we give
this node an address. Fix this by explicitly defining the
memory ranges in use.
Signed-off-by: Andrew Davis <afd@ti.com>
---
arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
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..b33a1ca2236ff 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
@@ -346,10 +346,14 @@ wkup_i2c0: i2c@42120000 {
fss: bus@47000000 {
compatible = "simple-bus";
- reg = <0x0 0x47000000 0x0 0x100>;
#address-cells = <2>;
#size-cells = <2>;
- ranges;
+ ranges = <0x0 0x47000000 0x0 0x47000000 0x0 0x100>, /* FSS Control */
+ <0x0 0x47034000 0x0 0x47034000 0x0 0x100>, /* HBMC Control */
+ <0x0 0x47040000 0x0 0x47040000 0x0 0x100>, /* OSPI0 Control */
+ <0x0 0x47050000 0x0 0x47050000 0x0 0x100>, /* OSPI1 Control */
+ <0x5 0x00000000 0x5 0x00000000 0x1 0x0000000>, /* HBMC/OSPI0 Memory */
+ <0x7 0x00000000 0x7 0x00000000 0x1 0x0000000>; /* OSPI1 Memory */
hbmc_mux: mux-controller@47000004 {
compatible = "reg-mux";
--
2.39.2
^ permalink raw reply related
* [PATCH 4/4] arm64: dts: ti: k3-j784s4: Use exact ranges for FSS node
From: Andrew Davis @ 2024-03-26 20:59 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: <20240326205920.40147-1-afd@ti.com>
The FSS bus contains several register ranges. Using an empty
ranges property works but causes a DT warning when we give
this node an address. Fix this by explicitly defining the
memory ranges in use.
Signed-off-by: Andrew Davis <afd@ti.com>
---
arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
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..c6d4bf79c1423 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
@@ -674,10 +674,13 @@ adc {
fss: bus@47000000 {
compatible = "simple-bus";
- reg = <0x00 0x47000000 0x00 0x100>;
#address-cells = <2>;
#size-cells = <2>;
- ranges;
+ ranges = <0x0 0x47000000 0x0 0x47000000 0x0 0x100>, /* FSS Control */
+ <0x0 0x47040000 0x0 0x47040000 0x0 0x100>, /* OSPI0 Control */
+ <0x0 0x47050000 0x0 0x47050000 0x0 0x100>, /* OSPI1 Control */
+ <0x5 0x00000000 0x5 0x00000000 0x1 0x0000000>, /* OSPI0 Memory */
+ <0x7 0x00000000 0x7 0x00000000 0x1 0x0000000>; /* OSPI1 Memory */
ospi0: spi@47040000 {
compatible = "ti,am654-ospi", "cdns,qspi-nor";
--
2.39.2
^ permalink raw reply related
* [PATCH 2/4] arm64: dts: ti: k3-j7200: Use exact ranges for FSS node
From: Andrew Davis @ 2024-03-26 20:59 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: <20240326205920.40147-1-afd@ti.com>
The FSS bus contains several register ranges. Using an empty
ranges property works but causes a DT warning when we give
this node an address. Fix this by explicitly defining the
memory ranges in use.
Signed-off-by: Andrew Davis <afd@ti.com>
---
arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
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..4fe39764ded40 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -520,10 +520,12 @@ mcu_spi2: spi@40320000 {
fss: bus@47000000 {
compatible = "simple-bus";
- reg = <0x00 0x47000000 0x00 0x100>;
#address-cells = <2>;
#size-cells = <2>;
- ranges;
+ ranges = <0x0 0x47000000 0x0 0x47000000 0x0 0x100>, /* FSS Control */
+ <0x0 0x47034000 0x0 0x47040000 0x0 0x100>, /* HBMC Control */
+ <0x0 0x47040000 0x0 0x47040000 0x0 0x100>, /* OSPI0 Control */
+ <0x5 0x00000000 0x5 0x00000000 0x1 0x0000000>; /* HBMC/OSPI0 Memory */
hbmc_mux: mux-controller@47000004 {
compatible = "reg-mux";
--
2.39.2
^ permalink raw reply related
* [PATCH 1/4] arm64: dts: ti: k3-am65: Use exact ranges for FSS node
From: Andrew Davis @ 2024-03-26 20:59 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
The FSS bus contains several register ranges. Using an empty
ranges property works but causes a DT warning when we give
this node an address. Fix this by explicitly defining the
memory ranges in use.
Signed-off-by: Andrew Davis <afd@ti.com>
---
arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
index 6ff3ccc39fb44..babfcbb9ecf56 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
@@ -286,7 +286,11 @@ fss: bus@47000000 {
compatible = "simple-bus";
#address-cells = <2>;
#size-cells = <2>;
- ranges;
+ ranges = <0x0 0x47000000 0x0 0x47000000 0x0 0x100>, /* FSS Control */
+ <0x0 0x47040000 0x0 0x47040000 0x0 0x100>, /* OSPI0 Control */
+ <0x0 0x47050000 0x0 0x47050000 0x0 0x100>, /* OSPI1 Control */
+ <0x5 0x00000000 0x5 0x00000000 0x1 0x0000000>, /* OSPI0 Memory */
+ <0x7 0x00000000 0x7 0x00000000 0x1 0x0000000>; /* OSPI1 Memory */
ospi0: spi@47040000 {
compatible = "ti,am654-ospi", "cdns,qspi-nor";
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v4 1/4] interconnect: qcom: icc-rpmh: Add QoS configuration support
From: Konrad Dybcio @ 2024-03-26 20:56 UTC (permalink / raw)
To: Odelu Kukatla, Bjorn Andersson, Georgi Djakov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Kees Cook, cros-qcom-dts-watchers, Gustavo A . R . Silva,
linux-arm-msm, linux-pm, devicetree, linux-kernel,
linux-hardening, quic_rlaggysh, quic_mdtipton
In-Reply-To: <20240325181628.9407-2-quic_okukatla@quicinc.com>
On 25.03.2024 7:16 PM, Odelu Kukatla wrote:
> It adds QoS support for QNOC device and includes support for
> configuring priority, priority forward disable, urgency forwarding.
> This helps in priortizing the traffic originating from different
> interconnect masters at NoC(Network On Chip).
>
> Signed-off-by: Odelu Kukatla <quic_okukatla@quicinc.com>
> ---
[...]
>
> + if (desc->config) {
> + struct resource *res;
> + void __iomem *base;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + goto skip_qos_config;
> +
> + base = devm_ioremap_resource(dev, res);
You were asked to substitute this call like 3 times already..
devm_platform_get_and_ioremap_resource
or even better, devm_platform_ioremap_resource
[...]
> @@ -70,6 +102,7 @@ struct qcom_icc_node {
> u64 max_peak[QCOM_ICC_NUM_BUCKETS];
> struct qcom_icc_bcm *bcms[MAX_BCM_PER_NODE];
> size_t num_bcms;
> + const struct qcom_icc_qosbox *qosbox;
I believe I came up with a better approach for storing this.. see [1]
Konrad
[1] https://lore.kernel.org/linux-arm-msm/20240326-topic-rpm_icc_qos_cleanup-v1-4-357e736792be@linaro.org/
^ permalink raw reply
* Re: [PATCH 1/6] arm64: dts: ti: k3-am65: Remove UART baud rate selection
From: Andrew Davis @ 2024-03-26 20:55 UTC (permalink / raw)
To: Nishanth Menon
Cc: Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <20240326202105.duim7cooxk4dwlz2@makeshift>
On 3/26/24 3:21 PM, Nishanth Menon wrote:
> On 13:54-20240326, Andrew Davis wrote:
>> 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
>
> I do not understand the explanation above -> how does one do this?
> If you are talking of the 8250 divider registers, someone has to program
> those - how do you compute the baud rate from registers that aren't
> programmed? Note: I am not commenting on the rationale of removing the
> property, just trying to understand the assertion above.
>
This divider register, when "unprogrammed", has a default value. When one
wants to use a TTY/serial device they pick the baud-rate(stty, termios, etc.),
that then programs the register with the value they selected.
As stated below, this property does not program that register, it only
tells the driver what it should have already been set to. Some drivers
need this as they do not have a way to know what the divider is actually
set to.
Setting this in our case is wrong for two reasons:
1) Our driver can and does just check the divider itself.
2) We do not setup most of these UARTs before the kernel starts, so
if our driver *did* care about this property it would not reflect
the actual divider value (which is unprogrammed at this point).
Andrew
>> 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
>>
>
^ permalink raw reply
* Re: [PATCH v2 5/6] PCI: dwc: rcar-gen4: Add support for other R-Car Gen4 PCIe controller
From: Bjorn Helgaas @ 2024-03-26 20:48 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: lpieralisi, kw, robh, bhelgaas, krzysztof.kozlowski+dt, conor+dt,
jingoohan1, gustavo.pimentel, mani, marek.vasut+renesas,
linux-pci, devicetree, linux-renesas-soc
In-Reply-To: <20240326024540.2336155-6-yoshihiro.shimoda.uh@renesas.com>
The subject line should specify which controller(s) this adds support
for.
On Tue, Mar 26, 2024 at 11:45:39AM +0900, Yoshihiro Shimoda wrote:
> The PCIe controllers of R-Car V4H (r8a779g0) and one more SoC require
> different initializing settings than R-Car S4-8 (r8a779f0). So, add
> specific functions for them as "renesas,rcar-gen4-pcie{-ep}" compatible.
I can't tell from this what's being added. This should say something
like "this driver previously supported r8.... Add support for r8...."
so it's clear what was existing and what is new.
Hmm... the first use of request_firmware() in drivers/pci/. That
warrants a mention here as it's a pretty significant change.
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 188 +++++++++++++++++++-
> 1 file changed, 187 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index a37613dd9ff4..7f3b5e9ca405 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -5,8 +5,10 @@
> */
>
> #include <linux/delay.h>
> +#include <linux/firmware.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> +#include <linux/iopoll.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/pci.h>
> @@ -20,9 +22,10 @@
> /* Renesas-specific */
> /* PCIe Mode Setting Register 0 */
> #define PCIEMSR0 0x0000
> -#define BIFUR_MOD_SET_ON BIT(0)
> +#define APP_SRIS_MODE BIT(6)
> #define DEVICE_TYPE_EP 0
> #define DEVICE_TYPE_RC BIT(4)
> +#define BIFUR_MOD_SET_ON BIT(0)
>
> /* PCIe Interrupt Status 0 */
> #define PCIEINTSTS0 0x0084
> @@ -37,33 +40,179 @@
> #define PCIEDMAINTSTSEN 0x0314
> #define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)
>
> +/* Port Logic Registers 89 */
> +#define PRTLGC89 0x0b70
> +
> +/* Port Logic Registers 90 */
> +#define PRTLGC90 0x0b74
> +
> /* PCIe Reset Control Register 1 */
> #define PCIERSTCTRL1 0x0014
> #define APP_HOLD_PHY_RST BIT(16)
> #define APP_LTSSM_ENABLE BIT(0)
>
> +/* PCIe Power Management Control */
> +#define PCIEPWRMNGCTRL 0x0070
> +#define APP_CLK_REQ_N BIT(11)
> +#define APP_CLK_PM_EN BIT(10)
> +
> #define RCAR_NUM_SPEED_CHANGE_RETRIES 10
> #define RCAR_MAX_LINK_SPEED 4
>
> #define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET 0x1000
> #define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET 0x800
>
> +#define RCAR_GEN4_PCIE_FIRMEARE_NAME "rcar_gen4_pcie.bin"
> +#define RCAR_GEN4_PCIE_FIRMEARE_BASE_ADDR 0xc000
s/FIRMEARE/FIRMWARE/
> struct rcar_gen4_pcie {
> struct dw_pcie dw;
> void __iomem *base;
> + /*
> + * The R-Car Gen4 documents don't describe the PHY registers' name.
> + * But, the initialization procedure describes these offsets. So,
> + * this driver has "phy_base + magical offset number" for it.
Make up your own #defines for the offsets. That would be better than
magic hex offsets below.
> + void __iomem *phy_base;
> struct platform_device *pdev;
> enum dw_pcie_device_mode mode;
>
> int (*start_link_enable)(struct rcar_gen4_pcie *rcar);
> + void (*additional_common_init)(struct rcar_gen4_pcie *rcar);
> };
> #define to_rcar_gen4_pcie(_dw) container_of(_dw, struct rcar_gen4_pcie, dw)
>
> struct rcar_gen4_pcie_platdata {
> enum dw_pcie_device_mode mode;
> int (*start_link_enable)(struct rcar_gen4_pcie *rcar);
> + void (*additional_common_init)(struct rcar_gen4_pcie *rcar);
> };
>
> /* Common */
> +static void rcar_gen4_pcie_phy_reg_update_bits(struct rcar_gen4_pcie *rcar,
> + u32 offset, u32 mask, u32 val)
> +{
> + u32 tmp;
> +
> + tmp = readl(rcar->phy_base + offset);
> + tmp &= ~mask;
> + tmp |= val;
> + writel(tmp, rcar->phy_base + offset);
> +}
> +
> +static int rcar_gen4_pcie_reg_check_bit(struct rcar_gen4_pcie *rcar,
> + u32 offset, u32 mask)
> +{
> + struct dw_pcie *dw = &rcar->dw;
> +
> + if (dw_pcie_readl_dbi(dw, offset) & mask)
> + return -EAGAIN;
> +
> + return 0;
> +}
> +
> +static int rcar_gen4_pcie_update_phy_firmware(struct rcar_gen4_pcie *rcar)
> +{
> + const u32 check_addr[] = { 0x00101018, 0x00101118, 0x00101021, 0x00101121};
> + struct dw_pcie *dw = &rcar->dw;
> + const struct firmware *fw;
> + unsigned int i, timeout;
> + u32 data;
> + int ret;
> +
> + ret = request_firmware(&fw, RCAR_GEN4_PCIE_FIRMEARE_NAME, dw->dev);
> + if (ret)
> + return ret;
It looks like a failure here leads to a probe failure, so I think this
needs a diagnostic message so the user has a hint about what went
wrong.
> + for (i = 0; i < (fw->size / 2); i++) {
> + data = fw->data[i * 2] | fw->data[(i * 2) + 1] << 8;
> + timeout = 100;
> +retry_data:
> + dw_pcie_writel_dbi(dw, PRTLGC89, RCAR_GEN4_PCIE_FIRMEARE_BASE_ADDR + i);
> + dw_pcie_writel_dbi(dw, PRTLGC90, data);
> + if (rcar_gen4_pcie_reg_check_bit(rcar, PRTLGC89, BIT(30)) < 0) {
> + if (!(--timeout)) {
> + ret = -ETIMEDOUT;
> + goto exit;
> + }
> + usleep_range(100, 200);
> + goto retry_data;
> + }
> + }
> +
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x0f8, BIT(17), BIT(17));
> +
> + for (i = 0; i < ARRAY_SIZE(check_addr); i++) {
> + timeout = 100;
> +retry_check:
> + dw_pcie_writel_dbi(dw, PRTLGC89, check_addr[i]);
> + ret = rcar_gen4_pcie_reg_check_bit(rcar, PRTLGC89, BIT(30));
> + ret |= rcar_gen4_pcie_reg_check_bit(rcar, PRTLGC90, BIT(0));
> + if (ret < 0) {
> + if (!(--timeout)) {
> + ret = -ETIMEDOUT;
> + goto exit;
> + }
> + usleep_range(100, 200);
> + goto retry_check;
> + }
> + }
> +
> + ret = 0;
> +exit:
> + release_firmware(fw);
> +
> + return ret;
> +}
> +
> +static int rcar_gen4_pcie_enable_phy(struct rcar_gen4_pcie *rcar)
> +{
> + struct dw_pcie *dw = &rcar->dw;
> + u32 val;
> + int ret;
> +
> + val = dw_pcie_readl_dbi(dw, PCIE_PORT_FORCE);
> + val |= PORT_FORCE_DO_DESKEW_FOR_SRIS;
> + dw_pcie_writel_dbi(dw, PCIE_PORT_FORCE, val);
> +
> + val = readl(rcar->base + PCIEMSR0);
> + val |= APP_SRIS_MODE;
> + writel(val, rcar->base + PCIEMSR0);
> +
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x700, BIT(28), 0);
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x700, BIT(20), 0);
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x700, BIT(12), 0);
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x700, BIT(4), 0);
> +
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x148, GENMASK(23, 22), BIT(22));
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x148, GENMASK(18, 16), GENMASK(17, 16));
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x148, GENMASK(7, 6), BIT(6));
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x148, GENMASK(2, 0), GENMASK(11, 0));
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x1d4, GENMASK(16, 15), GENMASK(16, 15));
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x514, BIT(26), BIT(26));
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x0f8, BIT(16), 0);
> + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x0f8, BIT(19), BIT(19));
> +
> + val = readl(rcar->base + PCIERSTCTRL1);
> + val &= ~APP_HOLD_PHY_RST;
> + writel(val, rcar->base + PCIERSTCTRL1);
> +
> + ret = readl_poll_timeout(rcar->phy_base + 0x0f8, val, !(val & BIT(18)),
> + 100, 10000);
> + if (ret < 0)
> + return ret;
> +
> + ret = rcar_gen4_pcie_update_phy_firmware(rcar);
> + if (ret)
> + return ret;
> +
> + val = readl(rcar->base + PCIERSTCTRL1);
> + val |= APP_LTSSM_ENABLE;
> + writel(val, rcar->base + PCIERSTCTRL1);
> +
> + return 0;
> +}
> +
> static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> bool enable)
> {
> @@ -201,6 +350,9 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar)
> if (ret)
> goto err_unprepare;
>
> + if (rcar->additional_common_init)
> + rcar->additional_common_init(rcar);
>
> return 0;
>
> err_unprepare:
> @@ -242,6 +394,10 @@ static void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
>
> static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar)
> {
> + rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy");
> + if (IS_ERR(rcar->phy_base))
> + return PTR_ERR(rcar->base);
I don't get it. This imposes a new requirement (presence of "phy"
resource) on the existing SoCs. That doesn't sound right.
> /* Renesas-specific registers */
> rcar->base = devm_platform_ioremap_resource_byname(rcar->pdev, "app");
>
> @@ -452,6 +608,7 @@ static int rcar_gen4_add_dw_pcie(struct rcar_gen4_pcie *rcar)
>
> rcar->mode = pd->mode;
> rcar->start_link_enable = pd->start_link_enable;
> + rcar->additional_common_init = pd->additional_common_init;
>
> switch (rcar->mode) {
> case DW_PCIE_RC_TYPE:
> @@ -521,6 +678,31 @@ static int r8a779f0_pcie_start_link_enable(struct rcar_gen4_pcie *rcar)
> return 0;
> }
>
> +static int rcar_gen4_pcie_start_link_enable(struct rcar_gen4_pcie *rcar)
> +{
> + return rcar_gen4_pcie_enable_phy(rcar);
> +}
> +
> +static void rcar_gen4_pcie_additional_common_init(struct rcar_gen4_pcie *rcar)
> +{
> + struct dw_pcie *dw = &rcar->dw;
> + u32 val;
> +
> + /*
> + * The SoC manual said the register setting is required. Otherwise,
> + * linkup failed.
> + */
> + val = dw_pcie_readl_dbi(dw, PCIE_PORT_LANE_SKEW);
> + val &= ~PORT_LANE_SKEW_INSERT_MASK;
> + if (dw->num_lanes < 4)
> + val |= BIT(6);
> + dw_pcie_writel_dbi(dw, PCIE_PORT_LANE_SKEW, val);
> +
> + val = readl(rcar->base + PCIEPWRMNGCTRL);
> + val |= APP_CLK_REQ_N | APP_CLK_PM_EN;
> + writel(val, rcar->base + PCIEPWRMNGCTRL);
I don't get this either. You do this "additional_common_init" part
only for the existing "renesas,rcar-gen4-pcie" and
"renesas,rcar-gen4-pcie-ep", but PCIE_PORT_LANE_SKEW and
PCIEPWRMNGCTRL do not appear in the driver prior to these patches. I
must be missing something. Or this is backwards and you meant to do
this for the *new* SoC?
If you need to limit some functionality to existing SoCs and add new
functionality for new SoCs, do those in separate patches if you can.
> +}
> +
> static struct rcar_gen4_pcie_platdata platdata_r8a779f0_pcie = {
> .mode = DW_PCIE_RC_TYPE,
> .start_link_enable = r8a779f0_pcie_start_link_enable,
> @@ -533,10 +715,14 @@ static struct rcar_gen4_pcie_platdata platdata_r8a779f0_pcie_ep = {
>
> static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie = {
> .mode = DW_PCIE_RC_TYPE,
> + .start_link_enable = rcar_gen4_pcie_start_link_enable,
> + .additional_common_init = rcar_gen4_pcie_additional_common_init,
> };
>
> static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie_ep = {
> .mode = DW_PCIE_EP_TYPE,
> + .start_link_enable = rcar_gen4_pcie_start_link_enable,
> + .additional_common_init = rcar_gen4_pcie_additional_common_init,
> };
>
> static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: qcs6490-rb3gen2: Enable UFS
From: Konrad Dybcio @ 2024-03-26 20:47 UTC (permalink / raw)
To: Bjorn Andersson, Bjorn Andersson, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240326-rb3gen2-ufs-v1-1-8c5c2dae1e64@quicinc.com>
On 26.03.2024 7:02 PM, Bjorn Andersson wrote:
> The rb3gen2 has UFS memory, adjust the necessary supply voltage and add
> the controller and phy nodes to enable this.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 63ebe0774f1d..0177d93ecd1d 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -221,8 +221,8 @@ vreg_l6b_1p2: ldo6 {
>
> vreg_l7b_2p952: ldo7 {
> regulator-name = "vreg_l7b_2p952";
> - regulator-min-microvolt = <2400000>;
> - regulator-max-microvolt = <3544000>;
> + regulator-min-microvolt = <2952000>;
> + regulator-max-microvolt = <2952000>;
> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> };
>
> @@ -508,6 +508,25 @@ &usb_1_qmpphy {
> status = "okay";
> };
>
> +&ufs_mem_hc {
> + reset-gpios = <&tlmm 175 GPIO_ACTIVE_LOW>;
> + vcc-supply = <&vreg_l7b_2p952>;
> + vcc-max-microamp = <800000>;
> + vccq-supply = <&vreg_l9b_1p2>;
> + vccq-max-microamp = <900000>;
> + vccq2-supply = <&vreg_l9b_1p2>;
> + vccq2-max-microamp = <900000>;
This probably requires adding supported modes and allow-set-mode to
these regs
Konrad
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: qcom: sc8180x: Drop flags for mdss irqs
From: Konrad Dybcio @ 2024-03-26 20:43 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Vinod Koul
Cc: linux-arm-msm, dri-devel, freedreno, devicetree
In-Reply-To: <20240326-fd-fix-schema-v1-3-4475d6d6d633@linaro.org>
On 26.03.2024 9:02 PM, Dmitry Baryshkov wrote:
> The number of interrupt cells for the mdss interrupt controller is 1,
> meaning there should only be one cell for the interrupt number, not two.
> Drop the second cell containing (unused) irq flags.
>
> Fixes: 494dec9b6f54 ("arm64: dts: qcom: sc8180x: Add display and gpu nodes")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [PATCH v6 0/2] Synchronize DT overlay removal with devlink removals
From: Rob Herring @ 2024-03-26 20:43 UTC (permalink / raw)
To: Herve Codina
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Frank Rowand,
Saravana Kannan, Lizhi Hou, Max Zhen, Sonal Santan,
Stefano Stabellini, Jonathan Cameron, linux-kernel, devicetree,
Allan Nielsen, Horatiu Vultur, Steen Hegelund, Luca Ceresoli,
Nuno Sa, Thomas Petazzoni
In-Reply-To: <20240325152140.198219-1-herve.codina@bootlin.com>
On Mon, Mar 25, 2024 at 04:21:24PM +0100, Herve Codina wrote:
> Hi,
>
> In the following sequence:
> of_platform_depopulate(); /* Remove devices from a DT overlay node */
> of_overlay_remove(); /* Remove the DT overlay node itself */
>
> Some warnings are raised by __of_changeset_entry_destroy() which was
> called from of_overlay_remove():
> ERROR: memory leak, expected refcount 1 instead of 2 ...
>
> The issue is that, during the device devlink removals triggered from the
> of_platform_depopulate(), jobs are put in a workqueue.
> These jobs drop the reference to the devices. When a device is no more
> referenced (refcount == 0), it is released and the reference to its
> of_node is dropped by a call to of_node_put().
> These operations are fully correct except that, because of the
> workqueue, they are done asynchronously with respect to function calls.
>
> In the sequence provided, the jobs are run too late, after the call to
> __of_changeset_entry_destroy() and so a missing of_node_put() call is
> detected by __of_changeset_entry_destroy().
>
> This series fixes this issue introducing device_link_wait_removal() in
> order to wait for the end of jobs execution (patch 1) and using this
> function to synchronize the overlay removal with the end of jobs
> execution (patch 2).
>
> Compared to the previous iteration:
> https://lore.kernel.org/linux-kernel/20240307111036.225007-1-herve.codina@bootlin.com/
> this v6 series:
> - Add Saravana's 'Reviewed-by' tag
>
> This series handles cases reported by Luca [1] and Nuno [2].
> [1]: https://lore.kernel.org/all/20231220181627.341e8789@booty/
> [2]: https://lore.kernel.org/all/20240205-fix-device-links-overlays-v2-2-5344f8c79d57@analog.com/
>
> Best regards,
> Hervé
>
> Changes v5 -> v6
> - Patch 1
> Add 'Reviewed-by: Saravana Kannan <saravanak@google.com>'
>
> - Patch 2
> No changes
>
> Changes v4 -> v5
> - Patch 1
> Remove the 'Fixes' tag
> Add 'Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>'
> Add 'Reviewed-by: Nuno Sa <nuno.sa@analog.com>'
>
> - Patch 2
> Update comment as suggested
> Add 'Reviewed-by: Saravana Kannan <saravanak@google.com>'
> Add 'Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>'
> Add 'Reviewed-by: Nuno Sa <nuno.sa@analog.com>'
>
> Changes v3 -> v4
> - Patch 1
> Uses flush_workqueue() instead of drain_workqueue().
>
> - Patch 2
> Remove unlock/re-lock when calling device_link_wait_removal()
> Move device_link_wait_removal() call to of_changeset_destroy()
> Update commit log
>
> Changes v2 -> v3
> - Patch 1
> No changes
>
> - Patch 2
> Add missing device.h
>
> Changes v1 -> v2
> - Patch 1
> Rename the workqueue to 'device_link_wq'
> Add 'Fixes' tag and Cc stable
>
> - Patch 2
> Add device.h inclusion.
> Call device_link_wait_removal() later in the overlay removal
> sequence (i.e. in free_overlay_changeset() function).
> Drop of_mutex lock while calling device_link_wait_removal().
> Add 'Fixes' tag and Cc stable
>
> Herve Codina (2):
> driver core: Introduce device_link_wait_removal()
> of: dynamic: Synchronize of_changeset_destroy() with the devlink
> removals
>
> drivers/base/core.c | 26 +++++++++++++++++++++++---
> drivers/of/dynamic.c | 12 ++++++++++++
> include/linux/device.h | 1 +
> 3 files changed, 36 insertions(+), 3 deletions(-)
Applied, thanks!
^ permalink raw reply
* Re: [PATCH 2/4] arm64: dts: qcom: sc8180x: drop legacy property #stream-id-cells
From: Konrad Dybcio @ 2024-03-26 20:43 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Vinod Koul
Cc: linux-arm-msm, dri-devel, freedreno, devicetree
In-Reply-To: <20240326-fd-fix-schema-v1-2-4475d6d6d633@linaro.org>
On 26.03.2024 9:02 PM, Dmitry Baryshkov wrote:
> The property #stream-id-cells is legacy, it is not documented as valid
> for the GPU. Drop it now.
>
> Fixes: 494dec9b6f54 ("arm64: dts: qcom: sc8180x: Add display and gpu nodes")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [PATCH v3 net-next 1/2] dt-bindings: net: airoha,en8811h: Add en8811h
From: Eric Woudstra @ 2024-03-26 20:41 UTC (permalink / raw)
To: Rob Herring
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Krzysztof Kozlowski, Conor Dooley, Andrew Lunn, Heiner Kallweit,
Russell King, Matthias Brugger, AngeloGioacchino Del Regno,
Frank Wunderlich, Daniel Golle, Lucien Jheng, Zhi-Jun You, netdev,
devicetree
In-Reply-To: <20240326192939.GA3250777-robh@kernel.org>
Hi Rob,
On 3/26/24 20:29, Rob Herring wrote:
> On Tue, Mar 26, 2024 at 05:23:04PM +0100, Eric Woudstra wrote:
>> Add the Airoha EN8811H 2.5 Gigabit PHY.
>>
>> The en8811h phy can be set with serdes polarity reversed on rx and/or tx.
>>
>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>
> Did you change something or forget to add Krzysztof's Reviewed-by?
Nothing has changed in this commit. I was wondering if I should do this,
so I should have added the Reviewed-by Krzysztof.
Best regards,
Eric Woudstra
^ permalink raw reply
* Re: [PATCH v2] riscv: dts: starfive: Remove PMIC interrupt info for Visionfive 2 board
From: Conor Dooley @ 2024-03-26 20:37 UTC (permalink / raw)
To: ganboing, kernel, robh, krzysztof.kozlowski+dt, conor+dt,
paul.walmsley, palmer, aou, devicetree, linux-riscv, linux-kernel,
Shengyu Qu
Cc: conor, Conor Dooley, stable
In-Reply-To: <TY3P286MB26116B828A34D614C09F4E8898202@TY3P286MB2611.JPNP286.PROD.OUTLOOK.COM>
From: Conor Dooley <conor.dooley@microchip.com>
On Thu, 07 Mar 2024 20:21:12 +0800, Shengyu Qu wrote:
> Interrupt line number of the AXP15060 PMIC is not a necessary part of
> its device tree. And this would cause kernel to try to enable interrupt
> line 0, which is not expected. So delete this part from device tree.
>
>
Applied to riscv-dt-fixes, thanks! And I didn't forget, so I re-wrote
the commit message to add some more information as promised.
[1/1] riscv: dts: starfive: Remove PMIC interrupt info for Visionfive 2 board
https://git.kernel.org/conor/c/0b163f43920d
Thanks,
Conor.
^ permalink raw reply
* Re: [PATCH 2/5] dt-bindings: display: Add GameForce Chi Panel
From: Rob Herring @ 2024-03-26 20:30 UTC (permalink / raw)
To: Chris Morgan
Cc: krzysztof.kozlowski+dt, airlied, devicetree, sam, neil.armstrong,
agx, megi, conor+dt, kernel, Chris Morgan, heiko, dri-devel,
daniel, quic_jesszhan, linux-rockchip
In-Reply-To: <20240325134959.11807-3-macroalpha82@gmail.com>
On Mon, 25 Mar 2024 08:49:56 -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> The GameForce Chi panel is a panel specific to the GameForce Chi
> handheld device that measures 3.5" diagonally with a resolution of
> 640x480.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
> .../devicetree/bindings/display/panel/rocktech,jh057n00900.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/5] dt-bindings: vendor-prefix: Add prefix for GameForce
From: Rob Herring @ 2024-03-26 20:30 UTC (permalink / raw)
To: Chris Morgan
Cc: sam, conor+dt, megi, airlied, krzysztof.kozlowski+dt,
neil.armstrong, devicetree, kernel, Chris Morgan, dri-devel, agx,
linux-rockchip, quic_jesszhan, heiko, daniel
In-Reply-To: <20240325134959.11807-2-macroalpha82@gmail.com>
On Mon, 25 Mar 2024 08:49:55 -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> GameForce is a company that produces handheld game consoles.
>
> https://gameforce.fun/
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 0/8] block: implement NVMEM provider
From: Rob Herring @ 2024-03-26 20:24 UTC (permalink / raw)
To: Daniel Golle
Cc: Krzysztof Kozlowski, Conor Dooley, Ulf Hansson, Jens Axboe,
Dave Chinner, Jan Kara, Thomas Weißschuh, Damien Le Moal,
Li Lingfeng, Christian Brauner, Christian Heusel, Min Li,
Adrian Hunter, Avri Altman, Hannes Reinecke, Christian Loehle,
Bean Huo, Yeqi Fu, Victor Shih, Christophe JAILLET,
Dominique Martinet, Ricardo B. Marliere, devicetree, linux-kernel,
linux-mmc, linux-block
In-Reply-To: <ZgGaay6bLFAcCo2E@makrotopia.org>
+boot-architecture list
On Mon, Mar 25, 2024 at 03:38:19PM +0000, Daniel Golle wrote:
> On Mon, Mar 25, 2024 at 10:10:46AM -0500, Rob Herring wrote:
> > On Thu, Mar 21, 2024 at 07:31:48PM +0000, Daniel Golle wrote:
> > > On embedded devices using an eMMC it is common that one or more (hw/sw)
> > > partitions on the eMMC are used to store MAC addresses and Wi-Fi
> > > calibration EEPROM data.
> > >
> > > Implement an NVMEM provider backed by a block device as typically the
> > > NVMEM framework is used to have kernel drivers read and use binary data
> > > from EEPROMs, efuses, flash memory (MTD), ...
> > >
> > > In order to be able to reference hardware partitions on an eMMC, add code
> > > to bind each hardware partition to a specific firmware subnode.
> > >
> > > Overall, this enables uniform handling across practially all flash
> > > storage types used for this purpose (MTD, UBI, and now also MMC).
> > >
> > > As part of this series it was necessary to define a device tree schema
> > > for block devices and partitions on them, which (similar to how it now
> > > works also for UBI volumes) can be matched by one or more properties.
> > >
> > > ---
> > > This series has previously been submitted as RFC on July 19th 2023[1]
> > > and most of the basic idea did not change since. Another round of RFC
> > > was submitted on March 5th 2024[2] which has received overall positive
> > > feedback and only minor corrections have been done since (see
> > > changelog below).
> >
> > I don't recall giving positive feedback.
> >
> > I still think this should use offsets rather than partition specific
> > information. Not wanting to have to update the offsets if they change is
> > not reason enough to not use them.
>
> Using raw offsets on the block device (rather than the partition)
> won't work for most existing devices and boot firmware out there. They
> always reference the partition, usually by the name of a GPT
> partition (but sometimes also PARTUUID or even PARTNO) which is then
> used in the exact same way as an MTD partition or UBI volume would be
> on devices with NOR or NAND flash.
MTD normally uses offsets hence why I'd like some alignment. UBI is
special because raw NAND is, well, special.
> Just on eMMC we usually use a GPT
> or MBR partition table rather than defining partitions in DT or cmdline,
> which is rather rare (for historic reasons, I suppose, but it is what it
> is now).
Yes, I understand how eMMC works. I don't understand why if you have
part #, uuid, or name you can't get to the offset or vice-versa. You
need only 1 piece of identification to map partition table entries to DT
nodes. Sure, offsets can change, but surely the firmware can handle
adjusting the DT?
An offset would also work for the case of random firmware data on the
disk that may or may not have a partition associated with it. There are
certainly cases of that. I don't think we have much of a solution for
that other than trying to educate vendors to not do that or OS
installers only supporting installing to something other than eMMC. This
is something EBBR[1] is trying to address.
> Depending on the eMMC chip used, that partition may not even be at the
> same offset for different batches of the same device and hence I'd
> like to just do it in the same way vendor firmware does it as well.
Often vendor firmware is not a model to follow...
> Chad of Adtran has previously confirmed that [1], which was the
> positive feedback I was refering to. Other vendors like GL-iNet or
> Netgear are doing the exact same thing.
>
> As of now, we support this in OpenWrt by adding a lot of
> board-specific knowledge to userland, which is ugly and also prevents
> using things like PXE-initiated nfsroot on those devices.
>
> The purpose of this series is to be able to properly support such devices
> (ie. practially all consumer-grade routers out there using an eMMC for
> storing firmware).
>
> Also, those devices have enough resources to run a general purpose
> distribution like Debian instead of OpenWrt, and all the userland
> hacks to set MAC addresses and extract WiFi-EEPROM-data in a
> board-specific ways will most certainly never find their way into
> Debian. It's just not how embedded Linux works, unless you are looking
> only at the RaspberryPi which got that data stored in a textfile
> which is shipped by the distribution -- something very weird and very
> different from literally all of-the-shelf routers, access-points or
> switches I have ever seen (and I've seen many). Maybe Felix who has
> seen even more of them can tell us more about that.
General purpose distros want to partition the disk themselves. Adding
anything to the DT for disk partitions would require the installer to be
aware of it. There's various distro folks on the boot-arch list, so
maybe one of them can comment.
Rob
[1] https://arm-software.github.io/ebbr/index.html#document-chapter4-firmware-media
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
From: Jeff Johnson @ 2024-03-26 20:21 UTC (permalink / raw)
To: Dmitry Baryshkov, Marc Gonzalez
Cc: Kalle Valo, ath10k, wireless, DT, Pierre-Hugues Husson,
Arnaud Vrac, Jami Kettunen, Jeffrey Hugo, Bjorn Andersson,
Konrad Dybcio
In-Reply-To: <CAA8EJppn6M9dpzyu9d283AUtaeN-i-L=-aM5P9BEELXPLFS8YQ@mail.gmail.com>
On 3/26/2024 10:51 AM, Dmitry Baryshkov wrote:
> On Tue, 26 Mar 2024 at 19:45, Marc Gonzalez <mgonzalez@freebox.fr> wrote:
>>
>> [ It has been pointed out to me that the previous message was unclear. ]
>> [ Below is my 2nd attempt at a clearer message. ]
>>
>> Problem: firmware-5.bin has not been parsed yet when we have to handle
>> the ATH10K_QMI_EVENT_SERVER_ARRIVE case, so we can't rely on feature bits
>> to work around the lack of MSA_READY indicator.
>
> Then, I'd say, we have to resort to the DT property, unless Kalle or
> Jeff have other proposals.
Another option is to follow the downstream driver model and only expect this
based upon static configuration within the driver.
Downstream driver has:
if (priv->device_id == ADRASTEA_DEVICE_ID) {
ret = wlfw_msa_mem_info_send_sync_msg(priv);
ret = wlfw_msa_ready_send_sync_msg(priv);
}
https://git.codelinaro.org/clo/la/platform/vendor/qcom-opensource/wlan/platform/-/blob/wlan-platform.lnx.1.0.r4-rel/icnss2/main.c?ref_type=heads#L968
The downstream MSA logic (including some other code that populates MSA-related
fields in the QMI messages) is only invoked for ADRASTEA_DEVICE_ID.
We could introduce a new hw_params parameter to have the same semantics.
But I'm OK with the DT option as well.
Kalle?
/jeff
^ permalink raw reply
* Re: [PATCH 1/6] arm64: dts: ti: k3-am65: Remove UART baud rate selection
From: Nishanth Menon @ 2024-03-26 20:21 UTC (permalink / raw)
To: Andrew Davis
Cc: Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <20240326185441.29656-1-afd@ti.com>
On 13:54-20240326, Andrew Davis wrote:
> 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
I do not understand the explanation above -> how does one do this?
If you are talking of the 8250 divider registers, someone has to program
those - how do you compute the baud rate from registers that aren't
programmed? Note: I am not commenting on the rationale of removing the
property, just trying to understand the assertion above.
> 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
>
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
^ permalink raw reply
* Re: [PATCH v2 4/6] PCI: dwc: rcar-gen4: Add a new function pointer for other SoC support
From: Bjorn Helgaas @ 2024-03-26 20:21 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: lpieralisi, kw, robh, bhelgaas, krzysztof.kozlowski+dt, conor+dt,
jingoohan1, gustavo.pimentel, mani, marek.vasut+renesas,
linux-pci, devicetree, linux-renesas-soc
In-Reply-To: <20240326024540.2336155-5-yoshihiro.shimoda.uh@renesas.com>
Include the function pointer name in the subject so it's a little more
specific.
On Tue, Mar 26, 2024 at 11:45:38AM +0900, Yoshihiro Shimoda wrote:
> This driver can reuse other R-Car Gen4 SoC support. However, some
> initializing settings differs between r8a779f0 and others. So, add
> a new function pointer start_link_enable() to support other R-Car
> Gen4 SoC in the future. No behavior changes.
Make it clear here what the new SoC is. I think it's r8a779f0, but
you have to read the patch and look for the new .compatible string to
figure that out.
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 57 +++++++++++++++++++--
> 1 file changed, 52 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index 0be760ed420b..a37613dd9ff4 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -53,9 +53,16 @@ struct rcar_gen4_pcie {
> void __iomem *base;
> struct platform_device *pdev;
> enum dw_pcie_device_mode mode;
> +
> + int (*start_link_enable)(struct rcar_gen4_pcie *rcar);
> };
> #define to_rcar_gen4_pcie(_dw) container_of(_dw, struct rcar_gen4_pcie, dw)
>
> +struct rcar_gen4_pcie_platdata {
> + enum dw_pcie_device_mode mode;
> + int (*start_link_enable)(struct rcar_gen4_pcie *rcar);
I think it's confusing to repeat "mode" and "start_link_enable" in
both rcar_gen4_pcie and rcar_gen4_pcie_platdata. I know several other
drivers use this pattern, but I think it is simpler overall to just
save the pointer directly, e.g.,
imx6_pcie_probe
imx6_pcie->drvdata = of_device_get_match_data(dev);
ls_pcie_probe
pcie->drvdata = of_device_get_match_data(dev);
tegra_pcie_dw_probe
data = of_device_get_match_data(dev);
pcie->of_data = (struct tegra_pcie_dw_of_data *)data;
So I think the best thing would be to add struct
rcar_gen4_pcie_platdata, *move* rcar_gen4_pcie.mode there, and save a
pointer to the rcar_gen4_pcie_platdata in struct rcar_gen4_pcie.
That could be its own separate patch, which is nice on its own because
it gets rid of the (void *) casts in rcar_gen4_pcie_of_match[].
Then add .start_link_enable() (or .ltssm_enable(), see below) and the
r8a779f0 bits in another patch.
> +};
> +
> /* Common */
> static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> bool enable)
> @@ -123,9 +130,13 @@ static int rcar_gen4_pcie_speed_change(struct dw_pcie *dw)
> static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> {
> struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> - int i, changes;
> + int i, changes, ret;
>
> - rcar_gen4_pcie_ltssm_enable(rcar, true);
> + if (rcar->start_link_enable) {
> + ret = rcar->start_link_enable(rcar);
This looks basically like what qcom does:
qcom_pcie_start_link
if (pcie->cfg->ops->ltssm_enable)
pcie->cfg->ops->ltssm_enable(pcie)
Can you copy that and use the same name for the pointer and function
name (.ltssm_enable, .*_ltssm_enable())?
> + if (ret)
> + return ret;
> + }
>
> /*
> * Require direct speed change with retrying here if the link_gen is
> @@ -437,7 +448,10 @@ static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> /* Common */
> static int rcar_gen4_add_dw_pcie(struct rcar_gen4_pcie *rcar)
> {
> - rcar->mode = (uintptr_t)of_device_get_match_data(&rcar->pdev->dev);
> + const struct rcar_gen4_pcie_platdata *pd = of_device_get_match_data(&rcar->pdev->dev);
> +
> + rcar->mode = pd->mode;
> + rcar->start_link_enable = pd->start_link_enable;
>
> switch (rcar->mode) {
> case DW_PCIE_RC_TYPE:
> @@ -500,14 +514,47 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev)
> rcar_gen4_pcie_unprepare(rcar);
> }
>
> +static int r8a779f0_pcie_start_link_enable(struct rcar_gen4_pcie *rcar)
> +{
> + rcar_gen4_pcie_ltssm_enable(rcar, true);
Previously we called rcar_gen4_pcie_ltssm_enable() for
"renesas,rcar-gen4-pcie" and "renesas,rcar-gen4-pcie-ep". But after
this patch, it looks like we only call it for "renesas,r8a779f0-pcie"
and "renesas,r8a779f0-pcie-ep"?
> +
> + return 0;
> +}
> +
> +static struct rcar_gen4_pcie_platdata platdata_r8a779f0_pcie = {
> + .mode = DW_PCIE_RC_TYPE,
> + .start_link_enable = r8a779f0_pcie_start_link_enable,
> +};
> +
> +static struct rcar_gen4_pcie_platdata platdata_r8a779f0_pcie_ep = {
> + .mode = DW_PCIE_EP_TYPE,
> + .start_link_enable = r8a779f0_pcie_start_link_enable,
> +};
> +
> +static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie = {
> + .mode = DW_PCIE_RC_TYPE,
> +};
> +
> +static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie_ep = {
> + .mode = DW_PCIE_EP_TYPE,
> +};
> +
> static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> + {
> + .compatible = "renesas,r8a779f0-pcie",
> + .data = &platdata_r8a779f0_pcie,
> + },
> + {
> + .compatible = "renesas,r8a779f0-pcie-ep",
> + .data = &platdata_r8a779f0_pcie_ep,
> + },
> {
> .compatible = "renesas,rcar-gen4-pcie",
> - .data = (void *)DW_PCIE_RC_TYPE,
> + .data = &platdata_rcar_gen4_pcie,
> },
> {
> .compatible = "renesas,rcar-gen4-pcie-ep",
> - .data = (void *)DW_PCIE_EP_TYPE,
> + .data = &platdata_rcar_gen4_pcie_ep,
> },
> {},
> };
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v4 4/7] dt-bindings: iio: accel: adxl345: Add spi-3wire
From: Lothar Rubusch @ 2024-03-26 20:17 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
conor+dt, linux-iio, devicetree, linux-kernel, eraretuya
In-Reply-To: <b13ca51c-db57-4a09-b689-cf27265d348f@linaro.org>
On Tue, Mar 26, 2024 at 7:30 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 25/03/2024 23:09, Lothar Rubusch wrote:
> >>
> >>
> >>>
> >>>> the tags. The upstream maintainer will do that for tags received on the
> >>>> version they apply.
> >>>>
> >>>
> >>> I'm pretty sure we will still see further iterations. So, I apply the
> >>> tags in the next version, already scheduled. Ok?
> >>>
> >>>> https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
> >>>>
> >>>
> >>> Going over the books I feel it does not make sense to still mention
> >>> feedback ("Reveiewed-by") for the v1 or v2 of the patch here in a v5,
> >>> does it? Your link mentiones "However if the patch has changed
> >>
> >> I don't understand. When did you receive the tag? v3, right? So what do
> >> you mean by v1 and v2?
> >>
> >
> > V1: The first version of the 3wire patch. I have split the single
> > patch upon some feedback (yours?!) - V2... So, my current
> > interpretation is, that every feedback I need to mention as
> > Reviewed-by tag, no?
>
> What? Feedback is not review. It's clearly explained in submitting
> patches. Please read it.
>
Exactly. My missunderstanding here is this: Why did you send me a
reminder that I forgot to add "Reviewed-by" tag in your last mail?
Could you please clarify your last mail? You wrote:
"(...)
This is a friendly reminder during the review process.
It looks like you received a tag and forgot to add it.
If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, (...)"
AFAIK noone literally had told me: "please add a Reviewed-by me tag",
or did I miss something? I'm a bit lost here, sorry.
> Best regards,
> Krzysztof
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox