* Re: [PATCH v5 0/8] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support
From: Serge Semin @ 2020-05-22 15:41 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Serge Semin, Thomas Gleixner, Thomas Bogendoerfer, Alexey Malahov,
Maxim Kaurkin, Pavel Parkhomenko, Ramil Zaripov,
Ekaterina Skachko, Vadim Vlasov, Alexey Kolotnikov, Paul Burton,
Ralf Baechle, Arnd Bergmann, Alessandro Zummo, Alexandre Belloni,
Rob Herring, linux-mips, linux-rtc, devicetree, linux-kernel
In-Reply-To: <211ab91d-6085-3073-1cbc-2300abade1b7@linaro.org>
On Fri, May 22, 2020 at 05:28:42PM +0200, Daniel Lezcano wrote:
> On 21/05/2020 22:48, Serge Semin wrote:
> > As for all Baikal-T1 SoC related patchsets, which need this, we replaced
> > the DW APB Timer legacy plain text-based dt-binding file with DT schema.
> > Similarly the MIPS GIC bindings file is also converted to DT schema seeing
> > it also defines the MIPS GIC Timer binding.
> >
> > Aside from MIPS-specific r4k timer Baikal-T1 chip also provides a
> > functionality of two another timers: embedded into the MIPS GIC timer and
> > three external DW timers available over APB bus. But we can't use them
> > before the corresponding drivers are properly fixed. First of all DW APB
> > Timer shouldn't be bound to a single CPU, since as being accessible over
> > APB they are external with respect to all possible CPUs. Secondly there
> > might be more than just two DW APB Timers in the system (Baikal-T1 has
> > three of them), so permit the driver to use one of them as a clocksource
> > and the rest - for clockevents. Thirdly it's possible to use MIPS GIC
> > timer as a clocksource so register it in the corresponding subsystem
> > (the patch has been found in the Paul Burton MIPS repo so I left the
> > original Signed-off-by attribute). Finally in the same way as r4k timer
> > the MIPS GIC timer should be used with care when CPUFREQ config is enabled
> > since in case of CM2 the timer counting depends on the CPU reference clock
> > frequency while the clocksource subsystem currently doesn't support the
> > timers with non-stable clock.
> >
> > This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
> > base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
> > tag: v5.7-rc4
>
> Applied patch 1,2,4,5,6,7,8
>
> Thanks!
Great! Thanks. Am I right to expect the series in: git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
at the branch timers/core?
-Sergey
>
> -- Daniel
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH V5 5/5] iio: adc: Clean up ADC code common to PMIC5 and PMIC7
From: Andy Shevchenko @ 2020-05-22 15:41 UTC (permalink / raw)
To: Jishnu Prakash
Cc: agross, Bjorn Andersson, devicetree, Linux Kernel Mailing List,
Matthias Kaehlcke, Linus Walleij, Jonathan Cameron, Amit Kucheria,
smohanad, kgunda, aghayal, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-arm-msm,
linux-iio, linux-arm-msm-owner
In-Reply-To: <1590157452-27179-6-git-send-email-jprakash@codeaurora.org>
On Fri, May 22, 2020 at 5:25 PM Jishnu Prakash <jprakash@codeaurora.org> wrote:
>
> This commit includes the following changes:
>
> Add a common function used for read_raw callback for both PMIC5
> and PMIC7 ADCs.
>
> Add exit function for ADC.
>
> Add info_property under adc_data to more efficiently distinguish
> PMIC5 and PMIC7 ADCs.
...
> - if (of_device_is_compatible(node, "qcom,spmi-adc7"))
> - indio_dev->info = &adc7_info;
> - else
> - indio_dev->info = &adc5_info;
> + platform_set_drvdata(pdev, adc);
Ping-pong style of series. Can we convert to .info before we introduce
above check for compatibility?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH V5 3/5] iio: adc: Add support for PMIC7 ADC
From: Andy Shevchenko @ 2020-05-22 15:41 UTC (permalink / raw)
To: Jishnu Prakash
Cc: agross, Bjorn Andersson, devicetree, Linux Kernel Mailing List,
Matthias Kaehlcke, Linus Walleij, Jonathan Cameron, Amit Kucheria,
smohanad, kgunda, aghayal, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-arm-msm,
linux-iio, linux-arm-msm-owner
In-Reply-To: <CAHp75Vfgk0-Rye2We1A6_WTWMCK3D-WW4_T3CGPHc=-tB=6M9g@mail.gmail.com>
On Fri, May 22, 2020 at 6:37 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, May 22, 2020 at 5:25 PM Jishnu Prakash <jprakash@codeaurora.org> wrote:
> >
> > The ADC architecture on PMIC7 is changed as compared to PMIC5. The
> > major change from PMIC5 is that all SW communication to ADC goes through
> > PMK8350, which communicates with other PMICs through PBS when the ADC
> > on PMK8350 works in master mode. The SID register is used to identify the
> > PMICs with which the PBS needs to communicate. Add support for the same.
>
> Below should be in a separate patch, but it's a bikeshedding. So, I
> left it to maintainers to decide.
> Fine with me
Fine with me *after* addressing some issues below (besides ' < 0' part).
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml
From: Jonathan Albrieux @ 2020-05-22 15:44 UTC (permalink / raw)
To: Daniel Baluta
Cc: Jonathan Cameron, Jonathan Cameron, linux-kernel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Rob Herring
In-Reply-To: <2cb64245-a3ec-b6c0-5f64-b2fdf6cd3689@nxp.com>
On Fri, May 22, 2020 at 05:59:11PM +0300, Daniel Baluta wrote:
>
> On 22.05.2020 17:26, Jonathan Albrieux wrote:
> > On Fri, May 22, 2020 at 01:47:21PM +0300, Daniel Baluta wrote:
> > > > > > > > +
> > > > > > > > +maintainers:
> > > > > > > > + - can't find a mantainer, author is Daniel Baluta <daniel.baluta@intel.com>
> > > > > > > Daniel is still active in the kernel, just not at Intel any more. +CC
> > > > > > Oh ok thank you! Daniel are you still maintaining this driver?
> > > I can do reviews if requested but I'm not actively maintaining this driver.
> > > If anyone wants
> > >
> > > to take this over, will be more than happy.
> > >
> > >
> > > Other than that we can add my gmail address: Daniel Baluta
> > > <daniel.baluta@gmail.com>
> > >
> > >
> > >
> > Well if you'd like to review this patch I'd really appreciate :-)
> > Forgive me for not having understood your answer regarding the maintainer
> > field, can I add you to this binding as maintainer or are you saying to
> > not add you? Thank you and sorry for the repeated question,
> >
> >
>
> OK, so I think would be better not to add me as a maintainer because
>
> this would set some expecation from people, and I most likely won't
>
> have time to met them.
>
>
> Can you instead add the linux-iio mailing list as a maintainer, not sure
>
> if this is a common practice though.
>
>
Ok thank you! I don't know, maybe someone else could point out
the right direction to take, it would be sad to drop yaml documentation
format just for this.
Best regards,
Jonathan Albrieux
^ permalink raw reply
* Re: [PATCH v5 0/8] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support
From: Daniel Lezcano @ 2020-05-22 15:44 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Thomas Gleixner, Thomas Bogendoerfer, Alexey Malahov,
Maxim Kaurkin, Pavel Parkhomenko, Ramil Zaripov,
Ekaterina Skachko, Vadim Vlasov, Alexey Kolotnikov, Paul Burton,
Ralf Baechle, Arnd Bergmann, Alessandro Zummo, Alexandre Belloni,
Rob Herring, linux-mips, linux-rtc, devicetree, linux-kernel
In-Reply-To: <20200522154105.6gkutlgi2hxqjjzc@mobilestation>
On 22/05/2020 17:41, Serge Semin wrote:
> On Fri, May 22, 2020 at 05:28:42PM +0200, Daniel Lezcano wrote:
>> On 21/05/2020 22:48, Serge Semin wrote:
>>> As for all Baikal-T1 SoC related patchsets, which need this, we replaced
>>> the DW APB Timer legacy plain text-based dt-binding file with DT schema.
>>> Similarly the MIPS GIC bindings file is also converted to DT schema seeing
>>> it also defines the MIPS GIC Timer binding.
>>>
>>> Aside from MIPS-specific r4k timer Baikal-T1 chip also provides a
>>> functionality of two another timers: embedded into the MIPS GIC timer and
>>> three external DW timers available over APB bus. But we can't use them
>>> before the corresponding drivers are properly fixed. First of all DW APB
>>> Timer shouldn't be bound to a single CPU, since as being accessible over
>>> APB they are external with respect to all possible CPUs. Secondly there
>>> might be more than just two DW APB Timers in the system (Baikal-T1 has
>>> three of them), so permit the driver to use one of them as a clocksource
>>> and the rest - for clockevents. Thirdly it's possible to use MIPS GIC
>>> timer as a clocksource so register it in the corresponding subsystem
>>> (the patch has been found in the Paul Burton MIPS repo so I left the
>>> original Signed-off-by attribute). Finally in the same way as r4k timer
>>> the MIPS GIC timer should be used with care when CPUFREQ config is enabled
>>> since in case of CM2 the timer counting depends on the CPU reference clock
>>> frequency while the clocksource subsystem currently doesn't support the
>>> timers with non-stable clock.
>>>
>>> This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
>>> base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
>>> tag: v5.7-rc4
>>
>> Applied patch 1,2,4,5,6,7,8
>>
>> Thanks!
>
> Great! Thanks. Am I right to expect the series in: git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
> at the branch timers/core?
The series first goes to:
https://git.linaro.org/people/daniel.lezcano/linux.git/log/?h=timers/drivers/next
, then I send the PR to Thomas, who send in turn a PR at the merge
windows to Linus for the entire tip tree.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [PATCH] arm64: dts: rockchip: rename and label gpio-led subnodes part 2
From: Johan Jonker @ 2020-05-22 15:46 UTC (permalink / raw)
To: heiko; +Cc: robh+dt, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
Current dts files with 'gpio-led' nodes were manually verified.
In order to automate this process leds-gpio.txt
has been converted to yaml. With this conversion a check
for pattern properties was added. In part 2 rename and label
gpio-led subnodes that passed the regex, but still don't have
the preferred form. Any pin subnode that ends with '-gpio'
in the pinctrl node generates a warning.
Fix with help of the following rules:
1: Add nodename in the preferred form.
2: Always add a label that ends with '_led' to prevent conflicts
with other labels such as 'power' and 'mmc'
3: If leds need pinctrl add a label that ends with '_led_pin'
also to prevent conflicts with other labels.
patternProperties:
# The first form is preferred, but fall back to just 'led'
# anywhere in the node name to at least catch some child nodes.
"(^led-[0-9a-f]$|led)":
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/leds/
leds-gpio.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/
schemas/gpio/gpio.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3368-lion-haikou.dts | 6 +++---
arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi | 8 ++++----
arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi | 10 +++++-----
arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 6 +++---
arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts | 6 +++---
arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi | 6 +++---
arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 14 +++++++-------
arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi | 10 +++++-----
8 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3368-lion-haikou.dts b/arch/arm64/boot/dts/rockchip/rk3368-lion-haikou.dts
index cbde279ae..e8774347b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368-lion-haikou.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3368-lion-haikou.dts
@@ -25,9 +25,9 @@
};
leds {
- pinctrl-0 = <&led_pins_module>, <&led_sd_haikou>;
+ pinctrl-0 = <&module_led_pins>, <&sd_card_led_pin>;
- sd-card-led {
+ sd_card_led: led-3 {
label = "sd_card_led";
gpios = <&gpio0 RK_PD2 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "mmc0";
@@ -118,7 +118,7 @@
};
leds {
- led_sd_haikou: led-sd-gpio {
+ sd_card_led_pin: sd-card-led-pin {
rockchip,pins =
<0 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi b/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi
index e17311e09..eeef64e35 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi
@@ -76,16 +76,16 @@
leds {
compatible = "gpio-leds";
pinctrl-names = "default";
- pinctrl-0 = <&led_pins_module>;
+ pinctrl-0 = <&module_led_pins>;
- module_led1 {
+ module_led1: led-1 {
label = "module_led1";
gpios = <&gpio2 RK_PB5 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "heartbeat";
panic-indicator;
};
- module_led2 {
+ module_led2: led-2 {
label = "module_led2";
gpios = <&gpio3 RK_PA3 GPIO_ACTIVE_HIGH>;
default-state = "off";
@@ -270,7 +270,7 @@
&pinctrl {
leds {
- led_pins_module: led-module-gpio {
+ module_led_pins: module-led-pins {
rockchip,pins =
<2 RK_PB5 RK_FUNC_GPIO &pcfg_pull_none>,
<3 RK_PA3 RK_FUNC_GPIO &pcfg_pull_none>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
index e87a04477..e36837c04 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
@@ -141,15 +141,15 @@
leds {
compatible = "gpio-leds";
pinctrl-names = "default";
- pinctrl-0 = <&sys_led_gpio>, <&user_led_gpio>;
+ pinctrl-0 = <&sys_led_pin>, <&user_led_pin>;
- sys-led {
+ sys_led: led-0 {
label = "sys_led";
linux,default-trigger = "heartbeat";
gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
};
- user-led {
+ user_led: led-1 {
label = "user_led";
default-state = "off";
gpios = <&gpio4 RK_PD0 GPIO_ACTIVE_HIGH>;
@@ -586,11 +586,11 @@
};
leds {
- sys_led_gpio: sys_led-gpio {
+ sys_led_pin: sys-led-pin {
rockchip,pins = <0 RK_PA6 RK_FUNC_GPIO &pcfg_pull_none>;
};
- user_led_gpio: user_led-gpio {
+ user_led_pin: user-led-pin {
rockchip,pins = <4 RK_PD0 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
index c88018a0e..b24d54570 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
@@ -117,9 +117,9 @@
leds: gpio-leds {
compatible = "gpio-leds";
pinctrl-names = "default";
- pinctrl-0 = <&leds_gpio>;
+ pinctrl-0 = <&status_led_pin>;
- status {
+ status_led: led-0 {
gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
label = "status_led";
linux,default-trigger = "heartbeat";
@@ -520,7 +520,7 @@
};
gpio-leds {
- leds_gpio: leds-gpio {
+ status_led_pin: status-led-pin {
rockchip,pins = <0 RK_PB5 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts b/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts
index d80d6b726..a8d363568 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts
@@ -15,9 +15,9 @@
};
leds {
- pinctrl-0 = <&led_pin_module>, <&led_sd_haikou>;
+ pinctrl-0 = <&module_led_pin>, <&sd_card_led_pin>;
- sd-card-led {
+ sd_card_led: led-1 {
label = "sd_card_led";
gpios = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "mmc0";
@@ -179,7 +179,7 @@
};
leds {
- led_sd_haikou: led-sd-gpio {
+ sd_card_led_pin: sd-card-led-pin {
rockchip,pins =
<1 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>;
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
index 07694b196..ae31299cb 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
@@ -11,9 +11,9 @@
leds {
compatible = "gpio-leds";
pinctrl-names = "default";
- pinctrl-0 = <&led_pin_module>;
+ pinctrl-0 = <&module_led_pin>;
- module-led {
+ module_led: led-0 {
label = "module_led";
gpios = <&gpio2 RK_PD1 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "heartbeat";
@@ -450,7 +450,7 @@
};
leds {
- led_pin_module: led-module-gpio {
+ module_led_pin: module-led-pin {
rockchip,pins =
<2 RK_PD1 RK_FUNC_GPIO &pcfg_pull_none>;
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
index 9f225e9c3..cec70f2bf 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
@@ -61,23 +61,23 @@
leds {
compatible = "gpio-leds";
pinctrl-names = "default";
- pinctrl-0 = <&work_led_gpio>, <&diy_led_gpio>, <&yellow_led_gpio>;
+ pinctrl-0 = <&work_led_pin>, <&diy_led_pin>, <&yellow_led_pin>;
- work-led {
+ work_led: led-0 {
label = "green:work";
gpios = <&gpio2 RK_PD3 GPIO_ACTIVE_HIGH>;
default-state = "on";
linux,default-trigger = "heartbeat";
};
- diy-led {
+ diy_led: led-1 {
label = "red:diy";
gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
default-state = "off";
linux,default-trigger = "mmc1";
};
- yellow-led {
+ yellow_led: led-2 {
label = "yellow:yellow-led";
gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
default-state = "off";
@@ -595,15 +595,15 @@
};
leds {
- diy_led_gpio: diy_led-gpio {
+ diy_led_pin: diy-led-pin {
rockchip,pins = <0 RK_PB5 RK_FUNC_GPIO &pcfg_pull_none>;
};
- work_led_gpio: work_led-gpio {
+ work_led_pin: work-led-pin {
rockchip,pins = <2 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>;
};
- yellow_led_gpio: yellow_led-gpio {
+ yellow_led_pin: yellow-led-pin {
rockchip,pins = <0 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 6788ab28f..c39d0f411 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -39,15 +39,15 @@
leds {
compatible = "gpio-leds";
pinctrl-names = "default";
- pinctrl-0 = <&work_led_gpio>, <&diy_led_gpio>;
+ pinctrl-0 = <&work_led_pin>, <&diy_led_pin>;
- work-led {
+ work_led: led-0 {
label = "work";
default-state = "on";
gpios = <&gpio0 RK_PB3 GPIO_ACTIVE_HIGH>;
};
- diy-led {
+ diy_led: led-1 {
label = "diy";
default-state = "off";
gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
@@ -588,11 +588,11 @@
};
leds {
- work_led_gpio: work_led-gpio {
+ work_led_pin: work-led-pin {
rockchip,pins = <0 RK_PB3 RK_FUNC_GPIO &pcfg_pull_none>;
};
- diy_led_gpio: diy_led-gpio {
+ diy_led_pin: diy-led-pin {
rockchip,pins = <0 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v5 03/14] PCI: cadence: Convert all r/w accessors to perform only 32-bit accesses
From: Rob Herring @ 2020-05-22 15:54 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Tom Joseph, Lorenzo Pieralisi, Bjorn Helgaas, PCI,
linux-kernel@vger.kernel.org, Arnd Bergmann, Greg Kroah-Hartman,
devicetree, linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20200522033631.32574-4-kishon@ti.com>
On Thu, May 21, 2020 at 9:37 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Certain platforms like TI's J721E using Cadence PCIe IP can perform only
> 32-bit accesses for reading or writing to Cadence registers. Convert all
> read and write accesses to 32-bit in Cadence PCIe driver in preparation
> for adding PCIe support in TI's J721E SoC.
Looking more closely I don't think cdns_pcie_ep_assert_intx is okay
with this and never can be given the PCI_COMMAND and PCI_STATUS
registers are in the same word (IIRC, that's the main reason 32-bit
config space accesses are broken). So this isn't going to work at
least for EP accesses. And maybe you need a custom .raise_irq() hook
to minimize any problems (such as making the RMW atomic at least from
the endpoint's perspective).
Rob
^ permalink raw reply
* Re: [PATCH v5 0/3] Convert thermal bindings to yaml
From: Daniel Lezcano @ 2020-05-22 15:59 UTC (permalink / raw)
To: Amit Kucheria, linux-kernel, linux-arm-msm, swboyd, lukasz.luba,
mka, Amit Kucheria, Zhang Rui
Cc: devicetree, linux-pm
In-Reply-To: <cover.1585748882.git.amit.kucheria@linaro.org>
Hi Amit,
On 03/04/2020 09:01, Amit Kucheria wrote:
> Hi all,
>
> Here is a series splitting up the thermal bindings into 3 separate bindings
> in YAML, one each of the sensor, cooling-device and the thermal zones.
>
> A series to remove thermal.txt and change over all references to it will
> follow shortly. Another series to fixup problems found by enforcing this
> yaml definition across dts files will also follow.
Applied. Thanks for taking the time to convert it to the yaml schema.
Thanks Rob and Lukasz for the review.
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH v4 3/8] spi: stm32: Add 'SPI_SIMPLEX_RX', 'SPI_3WIRE_RX' support for stm32f4
From: dillon min @ 2020-05-22 15:59 UTC (permalink / raw)
To: Mark Brown
Cc: Rob Herring, p.zabel, Maxime Coquelin,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Alexandre Torgue, thierry.reding, Sam Ravnborg, Dave Airlie,
Daniel Vetter, Michael Turquette, Stephen Boyd, Linux ARM,
linux-kernel, linux-spi, linux-stm32, open list:DRM PANEL DRIVERS,
linux-clk
In-Reply-To: <CAL9mu0LAnT+AfjpGs0O-MD2HYrpnQRmrj6qXtJQrJi9kbQLPUw@mail.gmail.com>
On Fri, May 22, 2020 at 10:57 PM dillon min <dillon.minfei@gmail.com> wrote:
>
> hi Mark,
>
> Thanks for reviewing.
>
> On Fri, May 22, 2020 at 7:36 PM Mark Brown <broonie@kernel.org> wrote:
> >
> > On Mon, May 18, 2020 at 07:09:20PM +0800, dillon.minfei@gmail.com wrote:
> >
> > > 2, use stm32 spi's "In full-duplex (BIDIMODE=0 and RXONLY=0)", as tx_buf is
> > > null, we must add dummy data sent out before read data.
> > > so, add stm32f4_spi_tx_dummy() to handle this situation.
> >
> > There are flags SPI_CONTROLLER_MUST_TX and SPI_CONTROLLER_MUST_RX flags
> > that the driver can set if it needs to, no need to open code this in the
> > driver.
>
> Yes, after check SPI_CONTROLLER_MUST_TX in drivers/spi/spi.c , it's
> indeed to meet
> this situation, i will try it and sumbmit a new patch.
>
> thanks.
>
> best regards
>
> Dillon
Hi Mark,
There might be a conflict with 'SPI_CONTROLLER_MUST_TX' and 'SPI_3WIRE' mode,
i need to know the SPI_3WIRE direction, currently i get this
information from 'struct spi_device'
and 'struct spi_transfer'
if ((spi_device->mode & SPI_3WIRE) && (spi_transfer->tx_buf == NULL)
&& (spi_transfer->rx_buf != NULL))
this is a SPI_3WIRE_RX transfer
if ((spi_device->mode & SPI_3WIRE) && (spi_transfer->tx_buf != NULL)
&& (spi_transfer->rx_buf == NULL))
this is a SPI_3WIRE_TX transfer
but, after spi-core create a dummy tx_buf or rx_buf, then i can't get
the correct spi_3wire direction.
actually, this dummy tx_buf is useless for SPI_3WIRE. it's has meaning
for SPI_SIMPLE_RX mode,
simulate SPI_FULL_DUMPLEX
how do you think?
thanks
best regards
Dillon
^ permalink raw reply
* Re: [PATCH v5 0/8] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support
From: Serge Semin @ 2020-05-22 16:02 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Serge Semin, Thomas Gleixner, Thomas Bogendoerfer, Alexey Malahov,
Maxim Kaurkin, Pavel Parkhomenko, Ramil Zaripov,
Ekaterina Skachko, Vadim Vlasov, Alexey Kolotnikov, Paul Burton,
Ralf Baechle, Arnd Bergmann, Alessandro Zummo, Alexandre Belloni,
Rob Herring, linux-mips, linux-rtc, devicetree, linux-kernel
In-Reply-To: <0bbed067-f588-ef3e-a98a-c4b291810e4f@linaro.org>
On Fri, May 22, 2020 at 05:44:55PM +0200, Daniel Lezcano wrote:
> On 22/05/2020 17:41, Serge Semin wrote:
> > On Fri, May 22, 2020 at 05:28:42PM +0200, Daniel Lezcano wrote:
> >> On 21/05/2020 22:48, Serge Semin wrote:
> >>> As for all Baikal-T1 SoC related patchsets, which need this, we replaced
> >>> the DW APB Timer legacy plain text-based dt-binding file with DT schema.
> >>> Similarly the MIPS GIC bindings file is also converted to DT schema seeing
> >>> it also defines the MIPS GIC Timer binding.
> >>>
> >>> Aside from MIPS-specific r4k timer Baikal-T1 chip also provides a
> >>> functionality of two another timers: embedded into the MIPS GIC timer and
> >>> three external DW timers available over APB bus. But we can't use them
> >>> before the corresponding drivers are properly fixed. First of all DW APB
> >>> Timer shouldn't be bound to a single CPU, since as being accessible over
> >>> APB they are external with respect to all possible CPUs. Secondly there
> >>> might be more than just two DW APB Timers in the system (Baikal-T1 has
> >>> three of them), so permit the driver to use one of them as a clocksource
> >>> and the rest - for clockevents. Thirdly it's possible to use MIPS GIC
> >>> timer as a clocksource so register it in the corresponding subsystem
> >>> (the patch has been found in the Paul Burton MIPS repo so I left the
> >>> original Signed-off-by attribute). Finally in the same way as r4k timer
> >>> the MIPS GIC timer should be used with care when CPUFREQ config is enabled
> >>> since in case of CM2 the timer counting depends on the CPU reference clock
> >>> frequency while the clocksource subsystem currently doesn't support the
> >>> timers with non-stable clock.
> >>>
> >>> This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
> >>> base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
> >>> tag: v5.7-rc4
> >>
> >> Applied patch 1,2,4,5,6,7,8
> >>
> >> Thanks!
> >
> > Great! Thanks. Am I right to expect the series in: git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
> > at the branch timers/core?
>
> The series first goes to:
>
> https://git.linaro.org/people/daniel.lezcano/linux.git/log/?h=timers/drivers/next
>
> , then I send the PR to Thomas, who send in turn a PR at the merge
> windows to Linus for the entire tip tree.
Ok. Thanks for clarification.
-Sergey
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] net: phy: Add a helper to return the index for of the internal delay
From: Florian Fainelli @ 2020-05-22 16:11 UTC (permalink / raw)
To: Dan Murphy, andrew, hkallweit1, davem, robh
Cc: netdev, linux-kernel, devicetree
In-Reply-To: <20200522122534.3353-3-dmurphy@ti.com>
On 5/22/2020 5:25 AM, Dan Murphy wrote:
> Add a helper function that will return the index in the array for the
> passed in internal delay value. The helper requires the array, size and
> delay value.
>
> The helper will then return the index for the exact match or return the
> index for the index to the closest smaller value.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> drivers/net/phy/phy_device.c | 45 ++++++++++++++++++++++++++++++++++++
> include/linux/phy.h | 2 ++
> 2 files changed, 47 insertions(+)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 7481135d27ab..40f53b379d2b 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -2661,6 +2661,51 @@ void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause)
> }
> EXPORT_SYMBOL(phy_get_pause);
>
> +/**
> + * phy_get_delay_index - returns the index of the internal delay
> + * @phydev: phy_device struct
> + * @delay_values: array of delays the PHY supports
> + * @size: the size of the delay array
> + * @delay: the delay to be looked up
> + *
> + * Returns the index within the array of internal delay passed in.
Can we consider using s32 for storage that way the various
of_read_property_read_u32() are a natural fit (int works too, but I
would prefer being explicit).
> + */
> +int phy_get_delay_index(struct phy_device *phydev, int *delay_values, int size,
> + int delay)
> +{
> + int i;
> +
> + if (size <= 0)
> + return -EINVAL;
> +
> + if (delay <= delay_values[0])
> + return 0;
> +
> + if (delay > delay_values[size - 1])
> + return size - 1;
Does not that assume that the delays are sorted by ascending order, if
so, can you make it clear in the kernel doc?
> +
> + for (i = 0; i < size; i++) {
> + if (delay == delay_values[i])
> + return i;
> +
> + /* Find an approximate index by looking up the table */
> + if (delay > delay_values[i - 1] &&
&& i > 0 so you do not accidentally under-run the array?
> + delay < delay_values[i]) {
> + if (delay - delay_values[i - 1] < delay_values[i] - delay)
> + return i - 1;
> + else
> + return i;
> + }
> +
> + }
> +
> + phydev_err(phydev, "error finding internal delay index for %d\n",
> + delay);
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(phy_get_delay_index);
> +
> static bool phy_drv_supports_irq(struct phy_driver *phydrv)
> {
> return phydrv->config_intr && phydrv->ack_interrupt;
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 2bcdf19ed3b4..73552612c189 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1408,6 +1408,8 @@ void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx);
> bool phy_validate_pause(struct phy_device *phydev,
> struct ethtool_pauseparam *pp);
> void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause);
> +int phy_get_delay_index(struct phy_device *phydev, int *delay_values,
> + int size, int delay);
> void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv,
> bool *tx_pause, bool *rx_pause);
>
>
--
Florian
^ permalink raw reply
* Re: [PATCH 00/19] Implement NTB Controller using multiple PCI EP
From: Rob Herring @ 2020-05-22 16:11 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Lorenzo Pieralisi, Arnd Bergmann, Jon Mason, Dave Jiang,
Allen Hubbe, Tom Joseph, Bjorn Helgaas, Greg Kroah-Hartman,
Jonathan Corbet, PCI, Linux Doc Mailing List,
linux-kernel@vger.kernel.org, devicetree, linux-ntb
In-Reply-To: <20200514145927.17555-1-kishon@ti.com>
On Thu, May 14, 2020 at 8:59 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> This series is about implementing SW defined NTB using
> multiple endpoint instances. This series has been tested using
> 2 endpoint instances in J7 connected to two DRA7 boards. However there
> is nothing platform specific for the NTB functionality.
>
> This was presented in Linux Plumbers Conference. The presentation
> can be found @ [1]
I'd like to know why putting this into DT is better than configfs.
Does it solve some problem? Doing things in userspace is so much
easier and more flexible than modifying and updating a DT.
I don't really think the PCI endpoint stuff is mature enough to be
putting into DT either.
Rob
^ permalink raw reply
* Re: [PATCH net-next v2 4/4] net: dp83869: Add RGMII internal delay configuration
From: Florian Fainelli @ 2020-05-22 16:13 UTC (permalink / raw)
To: Dan Murphy, andrew, hkallweit1, davem, robh
Cc: netdev, linux-kernel, devicetree
In-Reply-To: <20200522122534.3353-5-dmurphy@ti.com>
On 5/22/2020 5:25 AM, Dan Murphy wrote:
> Add RGMII internal delay configuration for Rx and Tx.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> drivers/net/phy/dp83869.c | 101 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 101 insertions(+)
>
> diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
> index cfb22a21a2e6..a9008d32e2b6 100644
> --- a/drivers/net/phy/dp83869.c
> +++ b/drivers/net/phy/dp83869.c
> @@ -99,6 +99,14 @@
> #define DP83869_OP_MODE_MII BIT(5)
> #define DP83869_SGMII_RGMII_BRIDGE BIT(6)
>
> +/* RGMIIDCTL bits */
> +#define DP83869_RGMII_TX_CLK_DELAY_SHIFT 4
> +#define DP83869_RGMII_CLK_DELAY_INV 0
> +
> +static int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500, 1750,
> + 2000, 2250, 2500, 2750, 3000, 3250,
> + 3500, 3750, 4000};
> +
> enum {
> DP83869_PORT_MIRRORING_KEEP,
> DP83869_PORT_MIRRORING_EN,
> @@ -108,6 +116,8 @@ enum {
> struct dp83869_private {
> int tx_fifo_depth;
> int rx_fifo_depth;
> + u32 rx_id_delay;
> + u32 tx_id_delay;
> int io_impedance;
> int port_mirroring;
> bool rxctrl_strap_quirk;
> @@ -182,6 +192,7 @@ static int dp83869_of_init(struct phy_device *phydev)
> struct dp83869_private *dp83869 = phydev->priv;
> struct device *dev = &phydev->mdio.dev;
> struct device_node *of_node = dev->of_node;
> + int delay_size = ARRAY_SIZE(dp83869_internal_delay);
> int ret;
>
> if (!of_node)
> @@ -232,6 +243,26 @@ static int dp83869_of_init(struct phy_device *phydev)
> &dp83869->tx_fifo_depth))
> dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB;
>
> + dp83869->rx_id_delay = DP83869_RGMII_CLK_DELAY_INV;
> + ret = of_property_read_u32(of_node, "rx-internal-delay-ps",
> + &dp83869->rx_id_delay);
> + if (!ret && dp83869->rx_id_delay > dp83869_internal_delay[delay_size]) {
> + phydev_err(phydev,
> + "rx-internal-delay value of %u out of range\n",
> + dp83869->rx_id_delay);
> + return -EINVAL;
> + }
> +
> + dp83869->tx_id_delay = DP83869_RGMII_CLK_DELAY_INV;
> + ret = of_property_read_u32(of_node, "tx-internal-delay-ps",
> + &dp83869->tx_id_delay);
> + if (!ret && dp83869->tx_id_delay > dp83869_internal_delay[delay_size]) {
> + phydev_err(phydev,
> + "tx-internal-delay value of %u out of range\n",
> + dp83869->tx_id_delay);
> + return -EINVAL;
> + }
This is the kind of validation that I would be expecting from the PHY
library to do, in fact, since you use Device Tree standard property, I
would expect you only need to pass the maximum delay value and some
storage for your array of delays.
> +
> return ret;
> }
> #else
> @@ -270,6 +301,29 @@ static int dp83869_configure_rgmii(struct phy_device *phydev,
> return ret;
> }
>
> +static int dp83869_verify_rgmii_cfg(struct phy_device *phydev)
> +{
> + struct dp83869_private *dp83869 = phydev->priv;
> +
> + /* RX delay *must* be specified if internal delay of RX is used. */
> + if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) &&
> + dp83869->rx_id_delay == DP83869_RGMII_CLK_DELAY_INV) {
> + phydev_err(phydev, "ti,rx-internal-delay must be specified\n");
> + return -EINVAL;
> + }
> +
> + /* TX delay *must* be specified if internal delay of TX is used. */
> + if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) &&
> + dp83869->tx_id_delay == DP83869_RGMII_CLK_DELAY_INV) {
> + phydev_err(phydev, "ti,tx-internal-delay must be specified\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int dp83869_configure_mode(struct phy_device *phydev,
> struct dp83869_private *dp83869)
> {
> @@ -371,6 +425,12 @@ static int dp83869_config_init(struct phy_device *phydev)
> {
> struct dp83869_private *dp83869 = phydev->priv;
> int ret, val;
> + int delay_size = ARRAY_SIZE(dp83869_internal_delay);
> + int delay = 0;
> +
> + ret = dp83869_verify_rgmii_cfg(phydev);
> + if (ret)
> + return ret;
>
> ret = dp83869_configure_mode(phydev, dp83869);
> if (ret)
> @@ -394,6 +454,47 @@ static int dp83869_config_init(struct phy_device *phydev)
> dp83869->clk_output_sel <<
> DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT);
>
> + if (phy_interface_is_rgmii(phydev)) {
> + val = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIICTL);
> +
> + val &= ~(DP83869_RGMII_TX_CLK_DELAY_EN | DP83869_RGMII_RX_CLK_DELAY_EN);
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> + val |= (DP83869_RGMII_TX_CLK_DELAY_EN | DP83869_RGMII_RX_CLK_DELAY_EN);
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> + val |= DP83869_RGMII_TX_CLK_DELAY_EN;
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> + val |= DP83869_RGMII_RX_CLK_DELAY_EN;
> +
> + phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIICTL, val);
> +
> + if (dp83869->rx_id_delay) {
> + val = phy_get_delay_index(phydev,
> + &dp83869_internal_delay[0],
> + delay_size,
> + dp83869->rx_id_delay);
> + if (val < 0)
> + return val;
> +
> + delay |= val;
Don't you need to do a bitwise AND with the maximum delay value
supported by the range since you do a Read/Modify/Write operation here?
> + }
> +
> + if (dp83869->tx_id_delay) {
> + val = phy_get_delay_index(phydev,
> + &dp83869_internal_delay[0],
> + delay_size,
> + dp83869->tx_id_delay);
> + if (val < 0)
> + return val;
> +
> + delay |= val << DP83869_RGMII_TX_CLK_DELAY_SHIFT;
Likewise.
> + }
> +
> + phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIIDCTL,
> + delay);
> + }
> +
> return ret;
> }
>
>
--
Florian
^ permalink raw reply
* Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason
From: Petr Mladek @ 2020-05-22 16:21 UTC (permalink / raw)
To: Kees Cook
Cc: Pavel Tatashin, Anton Vorontsov, Colin Cross, Tony Luck,
Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
devicetree, linuxppc-dev
In-Reply-To: <20200515184434.8470-2-keescook@chromium.org>
On Fri 2020-05-15 11:44:29, Kees Cook wrote:
> To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
> the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
> KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
> distinguish between them, so there's no need to, as discussed here:
> https://lore.kernel.org/lkml/CA+CK2bAPv5u1ih5y9t5FUnTyximtFCtDYXJCpuyjOyHNOkRdqw@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Looks good to me:
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v4 3/8] spi: stm32: Add 'SPI_SIMPLEX_RX', 'SPI_3WIRE_RX' support for stm32f4
From: Mark Brown @ 2020-05-22 16:29 UTC (permalink / raw)
To: dillon min
Cc: Rob Herring, p.zabel, Maxime Coquelin,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Alexandre Torgue, thierry.reding, Sam Ravnborg, Dave Airlie,
Daniel Vetter, Michael Turquette, Stephen Boyd, Linux ARM,
linux-kernel, linux-spi, linux-stm32, open list:DRM PANEL DRIVERS,
linux-clk
In-Reply-To: <CAL9mu0JZ4Qy+m2oF9TSTRqA_mM0J89huCt3t_Gs7qHa=3LxhBw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 446 bytes --]
On Fri, May 22, 2020 at 11:59:25PM +0800, dillon min wrote:
> but, after spi-core create a dummy tx_buf or rx_buf, then i can't get
> the correct spi_3wire direction.
> actually, this dummy tx_buf is useless for SPI_3WIRE. it's has meaning
> for SPI_SIMPLE_RX mode,
> simulate SPI_FULL_DUMPLEX
Oh, that's annoying. I think the fix here is in the core, it should
ignore MUST_TX and MUST_RX in 3WIRE mode since they clearly make no
sense there.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 07/14] PCI: cadence: Add new *ops* for CPU addr fixup
From: Rob Herring @ 2020-05-22 16:45 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Arnd Bergmann, Tom Joseph,
Greg Kroah-Hartman, PCI, devicetree, linux-kernel@vger.kernel.org,
linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <3f9cf6e5-94f8-4c54-aaee-c181b0e79f1f@ti.com>
On Thu, May 21, 2020 at 5:35 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Hi Rob,
>
> On 5/21/2020 3:04 AM, Rob Herring wrote:
> > On Wed, May 06, 2020 at 08:44:22PM +0530, Kishon Vijay Abraham I wrote:
> >> Cadence driver uses "mem" memory resource to obtain the offset of
> >> configuration space address region, memory space address region and
> >> message space address region. The obtained offset is used to program
> >> the Address Translation Unit (ATU). However certain platforms like TI's
> >> J721E SoC require the absolute address to be programmed in the ATU and not
> >> just the offset.
> >
> > Once again, Cadence host binding is broken (or at least the example is).
> > The 'mem' region shouldn't even exist. It is overlapping the config
> > space and 'ranges':
> >
> > reg = <0x0 0xfb000000 0x0 0x01000000>,
> > <0x0 0x41000000 0x0 0x00001000>,
> > <0x0 0x40000000 0x0 0x04000000>;
> > reg-names = "reg", "cfg", "mem";
> >
> > ranges = <0x02000000 0x0 0x42000000 0x0 0x42000000 0x0 0x1000000>,
> > <0x01000000 0x0 0x43000000 0x0 0x43000000 0x0 0x0010000>;
> >
> >
> > 16M of registers looks a bit odd. I guess it doesn't matter
> > unless you have a 32-bit platform and care about your virtual
> > space. Probably should have been 3 regions for LM, RP, and AT looking
> > at the driver.
>
> The "mem" region in never ioremapped. However $patch removes requiring to add
> "mem" memory resource.
I was referring to ioremapping 'reg' region.
> >
> > Whatever outbound address translation you need should be based on
> > 'ranges'.
>
> You mean we don't need to add "new *ops* for CPU addr fixup"?. The issue is
> ranges provides CPU address and PCI address. The CPU will access whatever is
> populated in ranges to access the PCI bus. However while programming the ATU,
> we cannot use the CPU address provided in ranges directly (in some platforms)
> because the controller does not see the full address and only the lower 28bits.
Okay, that is clearer as to what the difference is. I think this
should be 2 patches. One dropping 'mem' usage and using a mask and the
2nd making the mask per platform.
Really, the parent node of the PCI controller should probably have
'ranges' and you could extract a mask from that. Looks like that is
what you had for DRA7... I'm not sure if ABI stability is important
for the Cadence platform. I'd assume that's just some IP eval system
and probably not?
Why do you need an ops here? All you need is a mask value.
> This similar restriction was there with Designware (mostly an integration
> issue) and we used *ops* to fixup the address that has to be programmed in ATU.
> The Designware initially used a wrapper so that ranges property can be directly
> used [1]. However this approach was later removed in [2]
>
> [1] -> https://lore.kernel.org/patchwork/patch/468523/
> [2] -> https://lkml.org/lkml/2015/10/16/232
So while you had the data for a mask in DT, the driver now hardcodes it?
Rob
^ permalink raw reply
* Re: [PATCH v7 3/5] remoteproc: Add support for runtime PM
From: Suman Anna @ 2020-05-22 16:47 UTC (permalink / raw)
To: Paul Cercueil, Bjorn Andersson, Ohad Ben-Cohen, Arnaud Pouliquen
Cc: od, linux-remoteproc, devicetree, linux-kernel, Tero Kristo
In-Reply-To: <20200515104340.10473-3-paul@crapouillou.net>
Hi Paul,
On 5/15/20 5:43 AM, Paul Cercueil wrote:
> Call pm_runtime_get_sync() before the firmware is loaded, and
> pm_runtime_put() after the remote processor has been stopped.
>
> Even though the remoteproc device has no PM callbacks, this allows the
> parent device's PM callbacks to be properly called.
I see this patch staged now for 5.8, and the latest -next branch has
broken the pm-runtime autosuspend feature we have in the OMAP remoteproc
driver. See commit 5f31b232c674 ("remoteproc/omap: Add support for
runtime auto-suspend/resume").
What was the original purpose of this patch, because there can be
differing backends across different SoCs.
regards
Suman
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>
> Notes:
> v2-v4: No change
> v5: Move calls to prepare/unprepare to rproc_fw_boot/rproc_shutdown
> v6: Instead of prepare/unprepare callbacks, use PM runtime callbacks
> v7: Check return value of pm_runtime_get_sync()
>
> drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index a7f96bc98406..e33d1ef27981 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -29,6 +29,7 @@
> #include <linux/devcoredump.h>
> #include <linux/rculist.h>
> #include <linux/remoteproc.h>
> +#include <linux/pm_runtime.h>
> #include <linux/iommu.h>
> #include <linux/idr.h>
> #include <linux/elf.h>
> @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
> if (ret)
> return ret;
>
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret);
> + return ret;
> + }
> +
> dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size);
>
> /*
> @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
> ret = rproc_enable_iommu(rproc);
> if (ret) {
> dev_err(dev, "can't enable iommu: %d\n", ret);
> - return ret;
> + goto put_pm_runtime;
> }
>
> rproc->bootaddr = rproc_get_boot_addr(rproc, fw);
> @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
> rproc->table_ptr = NULL;
> disable_iommu:
> rproc_disable_iommu(rproc);
> +put_pm_runtime:
> + pm_runtime_put(dev);
> return ret;
> }
>
> @@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc)
>
> rproc_disable_iommu(rproc);
>
> + pm_runtime_put(dev);
> +
> /* Free the copy of the resource table */
> kfree(rproc->cached_table);
> rproc->cached_table = NULL;
> @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
>
> rproc->state = RPROC_OFFLINE;
>
> + pm_runtime_no_callbacks(&rproc->dev);
> + pm_runtime_enable(&rproc->dev);
> +
> return rproc;
> }
> EXPORT_SYMBOL(rproc_alloc);
> @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc);
> */
> void rproc_free(struct rproc *rproc)
> {
> + pm_runtime_disable(&rproc->dev);
> put_device(&rproc->dev);
> }
> EXPORT_SYMBOL(rproc_free);
>
^ permalink raw reply
* Re: [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper
From: Petr Mladek @ 2020-05-22 16:51 UTC (permalink / raw)
To: Kees Cook
Cc: Pavel Tatashin, Anton Vorontsov, Colin Cross, Tony Luck,
Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
devicetree, linuxppc-dev
In-Reply-To: <20200515184434.8470-3-keescook@chromium.org>
On Fri 2020-05-15 11:44:30, Kees Cook wrote:
> From: Pavel Tatashin <pasha.tatashin@soleen.com>
>
> kmsg_dump() allows to dump kmesg buffer for various system events: oops,
> panic, reboot, etc. It provides an interface to register a callback call
> for clients, and in that callback interface there is a field "max_reason"
> which gets ignored unless always_kmsg_dump is passed as kernel parameter.
Strictly speaking, this is not fully true. "max_reason" field is not
ignored when set to KMSG_DUMP_PANIC even when always_kmsg_dump was not set.
It should be something like:
"which gets ignored for reason higher than KMSG_DUMP_OOPS unless
always_kmsg_dump is passed as kernel parameter".
Heh, I wonder if anyone will be able to parse this ;-)
Otherwise, it looks good to me. With the updated commit message:
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 3/4] remoteproc: add support for a new 64-bit trace version
From: Suman Anna @ 2020-05-22 16:54 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Rob Herring, Mathieu Poirier, Clement Leger, Loic Pallardy,
Arnaud Pouliquen, Lokesh Vutla, linux-remoteproc, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <997d6f9a-64ba-7a89-e909-9a5a474120b0@ti.com>
On 5/21/20 2:42 PM, Suman Anna wrote:
> Hi Bjorn,
>
> On 5/21/20 1:04 PM, Bjorn Andersson wrote:
>> On Wed 25 Mar 13:47 PDT 2020, Suman Anna wrote:
>>
>>> Introduce a new trace entry resource structure that accommodates
>>> a 64-bit device address to support 64-bit processors. This is to
>>> be used using an overloaded version value of 1 in the upper 32-bits
>>> of the previous resource type field. The new resource still uses
>>> 32-bits for the length field (followed by a 32-bit reserved field,
>>> so can be updated in the future), which is a sufficiently large
>>> trace buffer size. A 32-bit padding field also had to be added
>>> to align the device address on a 64-bit boundary, and match the
>>> usage on the firmware side.
>>>
>>> The remoteproc debugfs logic also has been adjusted accordingly.
>>>
>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>> ---
>>> drivers/remoteproc/remoteproc_core.c | 40 ++++++++++++++++++++-----
>>> drivers/remoteproc/remoteproc_debugfs.c | 37 ++++++++++++++++++-----
>>> include/linux/remoteproc.h | 26 ++++++++++++++++
>>> 3 files changed, 87 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/remoteproc/remoteproc_core.c
>>> b/drivers/remoteproc/remoteproc_core.c
>>> index 53bc37c508c6..b9a097990862 100644
>>> --- a/drivers/remoteproc/remoteproc_core.c
>>> +++ b/drivers/remoteproc/remoteproc_core.c
>>> @@ -609,21 +609,45 @@ void rproc_vdev_release(struct kref *ref)
>>> *
>>> * Returns 0 on success, or an appropriate error code otherwise
>>> */
>>> -static int rproc_handle_trace(struct rproc *rproc, struct
>>> fw_rsc_trace *rsc,
>>> +static int rproc_handle_trace(struct rproc *rproc, void *rsc,
>>> int offset, int avail, u16 ver)
>>> {
>>> struct rproc_debug_trace *trace;
>>> struct device *dev = &rproc->dev;
>>> + struct fw_rsc_trace *rsc1;
>>> + struct fw_rsc_trace2 *rsc2;
>>> char name[15];
>>> + size_t rsc_size;
>>> + u32 reserved;
>>> + u64 da;
>>> + u32 len;
>>> +
>>> + if (!ver) {
>>
>> This looks like a switch to me, but I also do think this looks rather
>> crude, if you spin off the tail of this function and call it from a
>> rproc_handle_trace() and rproc_handle_trace64() I believe this would be
>> cleaner.
>
> Yeah, ok. Will refactor for this in v2.
>
>>
>>> + rsc1 = (struct fw_rsc_trace *)rsc;
>>> + rsc_size = sizeof(*rsc1);
>>> + reserved = rsc1->reserved;
>>> + da = rsc1->da;
>>> + len = rsc1->len;
>>> + } else if (ver == 1) {
>>> + rsc2 = (struct fw_rsc_trace2 *)rsc;
>>> + rsc_size = sizeof(*rsc2);
>>> + reserved = rsc2->reserved;
>>> + da = rsc2->da;
>>> + len = rsc2->len;
>>> + } else {
>>> + dev_err(dev, "unsupported trace rsc version %d\n", ver);
>>
>> If we use "type" to describe your 64-bit-da-trace then this sanity check
>> would have been taken care of by the core.
>>
>>> + return -EINVAL;
>>> + }
>>> - if (sizeof(*rsc) > avail) {
>>> + if (rsc_size > avail) {
>>> dev_err(dev, "trace rsc is truncated\n");
>>> return -EINVAL;
>>> }
>>> /* make sure reserved bytes are zeroes */
>>> - if (rsc->reserved) {
>>> - dev_err(dev, "trace rsc has non zero reserved bytes\n");
>>> + if (reserved) {
>>> + dev_err(dev, "trace rsc has non zero reserved bytes, value =
>>> 0x%x\n",
>>> + reserved);
>>> return -EINVAL;
>>> }
>>> @@ -632,8 +656,8 @@ static int rproc_handle_trace(struct rproc
>>> *rproc, struct fw_rsc_trace *rsc,
>>> return -ENOMEM;
>>> /* set the trace buffer dma properties */
>>> - trace->trace_mem.len = rsc->len;
>>> - trace->trace_mem.da = rsc->da;
>>> + trace->trace_mem.len = len;
>>> + trace->trace_mem.da = da;
>>> /* set pointer on rproc device */
>>> trace->rproc = rproc;
>>> @@ -652,8 +676,8 @@ static int rproc_handle_trace(struct rproc
>>> *rproc, struct fw_rsc_trace *rsc,
>>> rproc->num_traces++;
>>> - dev_dbg(dev, "%s added: da 0x%x, len 0x%x\n",
>>> - name, rsc->da, rsc->len);
>>> + dev_dbg(dev, "%s added: da 0x%llx, len 0x%x\n",
>>> + name, da, len);
>>> return 0;
>>> }
>>> diff --git a/drivers/remoteproc/remoteproc_debugfs.c
>>> b/drivers/remoteproc/remoteproc_debugfs.c
>>> index 3560eed7a360..ff43736db45a 100644
>>> --- a/drivers/remoteproc/remoteproc_debugfs.c
>>> +++ b/drivers/remoteproc/remoteproc_debugfs.c
>>> @@ -192,7 +192,8 @@ static int rproc_rsc_table_show(struct seq_file
>>> *seq, void *p)
>>> struct resource_table *table = rproc->table_ptr;
>>> struct fw_rsc_carveout *c;
>>> struct fw_rsc_devmem *d;
>>> - struct fw_rsc_trace *t;
>>> + struct fw_rsc_trace *t1;
>>> + struct fw_rsc_trace2 *t2;
>>> struct fw_rsc_vdev *v;
>>> int i, j;
>>> @@ -205,6 +206,7 @@ static int rproc_rsc_table_show(struct seq_file
>>> *seq, void *p)
>>> int offset = table->offset[i];
>>> struct fw_rsc_hdr *hdr = (void *)table + offset;
>>> void *rsc = (void *)hdr + sizeof(*hdr);
>>> + u16 ver = hdr->st.v;
>>> switch (hdr->st.t) {
>>> case RSC_CARVEOUT:
>>> @@ -230,13 +232,32 @@ static int rproc_rsc_table_show(struct seq_file
>>> *seq, void *p)
>>> seq_printf(seq, " Name %s\n\n", d->name);
>>> break;
>>> case RSC_TRACE:
>>> - t = rsc;
>>> - seq_printf(seq, "Entry %d is of type %s\n",
>>> - i, types[hdr->st.t]);
>>> - seq_printf(seq, " Device Address 0x%x\n", t->da);
>>> - seq_printf(seq, " Length 0x%x Bytes\n", t->len);
>>> - seq_printf(seq, " Reserved (should be zero) [%d]\n",
>>> t->reserved);
>>> - seq_printf(seq, " Name %s\n\n", t->name);
>>> + if (ver == 0) {
>>
>> Again, this is a switch, here in a switch. Just defining a new
>> RSC_TRACE64 type would reduce the amount of code here...
>
> OK.
>
>>
>>> + t1 = rsc;
>>> + seq_printf(seq, "Entry %d is version %d of type %s\n",
>>> + i, ver, types[hdr->st.t]);
>>> + seq_printf(seq, " Device Address 0x%x\n",
>>> + t1->da);
>>> + seq_printf(seq, " Length 0x%x Bytes\n",
>>> + t1->len);
>>> + seq_printf(seq, " Reserved (should be zero) [%d]\n",
>>> + t1->reserved);
>>> + seq_printf(seq, " Name %s\n\n", t1->name);
>>> + } else if (ver == 1) {
>>> + t2 = rsc;
>>> + seq_printf(seq, "Entry %d is version %d of type %s\n",
>>> + i, ver, types[hdr->st.t]);
>>> + seq_printf(seq, " Device Address 0x%llx\n",
>>> + t2->da);
>>> + seq_printf(seq, " Length 0x%x Bytes\n",
>>> + t2->len);
>>> + seq_printf(seq, " Reserved (should be zero) [%d]\n",
>>> + t2->reserved);
>>> + seq_printf(seq, " Name %s\n\n", t2->name);
>>> + } else {
>>> + seq_printf(seq, "Entry %d is an unsupported version
>>> %d of type %s\n",
>>> + i, ver, types[hdr->st.t]);
>>> + }
>>> break;
>>> case RSC_VDEV:
>>> v = rsc;
>>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>>> index 526d3cb45e37..3b3bea42f8b1 100644
>>> --- a/include/linux/remoteproc.h
>>> +++ b/include/linux/remoteproc.h
>>> @@ -243,6 +243,32 @@ struct fw_rsc_trace {
>>> u8 name[32];
>>> } __packed;
>>> +/**
>>> + * struct fw_rsc_trace2 - trace buffer declaration supporting 64-bits
>>> + * @padding: initial padding after type field for aligned 64-bit access
>>> + * @da: device address (64-bit)
>>> + * @len: length (in bytes)
>>> + * @reserved: reserved (must be zero)
>>> + * @name: human-readable name of the trace buffer
>>> + *
>>> + * This resource entry is an enhanced version of the fw_rsc_trace
>>> resourec entry
>>> + * and the provides equivalent functionality but designed for 64-bit
>>> remote
>>> + * processors.
>>> + *
>>> + * @da specifies the device address of the buffer, @len specifies
>>> + * its size, and @name may contain a human readable name of the
>>> trace buffer.
>>> + *
>>> + * After booting the remote processor, the trace buffers are exposed
>>> to the
>>> + * user via debugfs entries (called trace0, trace1, etc..).
>>> + */
>>> +struct fw_rsc_trace2 {
>>
>> Sounds more like fw_rsc_trace64 to me - in particular since the version
>> of trace2 is 1...
>
> Yeah, will rename this.
>
>>
>>> + u32 padding;
>>> + u64 da;
>>> + u32 len;
>>> + u32 reserved;
>>
>> What's the purpose of this reserved field?
>
> Partly to make sure the entire resource is aligned on an 8-byte, and
> partly copied over from fw_rsc_trace entry. I guess 32-bits is already
> large enough of a size for trace entries irrespective of 32-bit or
> 64-bit traces, so I doubt if we want to make the len field also a u64.
Looking at this again, I can drop both padding and reserved fields, if I
move the len field before da. Any preferences/comments?
regards
Suman
>
> regards
> Suman
>
>>
>> Regards,
>> Bjorn
>>
>>> + u8 name[32];
>>> +} __packed;
>>> +
>>> /**
>>> * struct fw_rsc_vdev_vring - vring descriptor entry
>>> * @da: device address
>>> --
>>> 2.23.0
>>>
>
^ permalink raw reply
* Re: [PATCH V2 2/3] mmc: sdhci-msm: Use internal voltage control
From: Bjorn Andersson @ 2020-05-22 17:04 UTC (permalink / raw)
To: Veerabhadrarao Badiganti
Cc: adrian.hunter, ulf.hansson, robh+dt, linux-mmc, linux-kernel,
linux-arm-msm, devicetree, Asutosh Das, Vijay Viswanath,
Andy Gross
In-Reply-To: <08d11687-7aee-2c62-9435-670be1afb21e@codeaurora.org>
On Fri 22 May 06:27 PDT 2020, Veerabhadrarao Badiganti wrote:
> Hi Bjorn,
>
> On 5/22/2020 12:37 AM, Bjorn Andersson wrote:
> > On Thu 21 May 08:23 PDT 2020, Veerabhadrarao Badiganti wrote:
> >
> > > On qcom SD host controllers voltage switching be done after the HW
> > > is ready for it. The HW informs its readiness through power irq.
> > > The voltage switching should happen only then.
> > >
> > > Use the internal voltage switching and then control the voltage
> > > switching using power irq.
> > >
> > > Set the regulator load as well so that regulator can be configured
> > > in LPM mode when in is not being used.
> > >
> > > Co-developed-by: Asutosh Das <asutoshd@codeaurora.org>
> > > Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> > > Co-developed-by: Vijay Viswanath <vviswana@codeaurora.org>
> > > Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
> > > Co-developed-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> > > Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> > Looks better, thanks.
> >
> > > ---
> > > drivers/mmc/host/sdhci-msm.c | 207 +++++++++++++++++++++++++++++++++++++++++--
> > > 1 file changed, 198 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> > [..]
> > > static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
> > > @@ -1298,6 +1302,71 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
> > > sdhci_msm_hs400(host, &mmc->ios);
> > > }
> > > +static int sdhci_msm_set_vmmc(struct mmc_host *mmc)
> > > +{
> > > + int ret;
> > > +
> > > + if (IS_ERR(mmc->supply.vmmc))
> > > + return 0;
> > > +
> > > + ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, mmc->ios.vdd);
> > > + if (ret)
> > > + dev_err(mmc_dev(mmc), "%s: vmmc set ocr with vdd=%d failed: %d\n",
> > > + mmc_hostname(mmc), mmc->ios.vdd, ret);
> > Missed this one on v1, in the event that mmc_regulator_set_ocr() return
> > a non-zero value it has already printed an error message. So please
> > replace the tail with just:
> >
> > return mmc_regulator_set_ocr(...);
> >
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int sdhci_msm_set_vqmmc(struct sdhci_msm_host *msm_host,
> > > + struct mmc_host *mmc, bool level)
> > > +{
> > > + int load, ret;
> > > + struct mmc_ios ios;
> > > +
> > > + if (IS_ERR(mmc->supply.vqmmc) ||
> > > + (mmc->ios.power_mode == MMC_POWER_UNDEFINED) ||
> > > + (msm_host->vqmmc_enabled == level))
> > > + return 0;
> > > +
> > > + if (msm_host->vqmmc_load) {
> > > + load = level ? msm_host->vqmmc_load : 0;
> > > + ret = regulator_set_load(mmc->supply.vqmmc, load);
> > Sorry for the late reply on v1, but please see my explanation regarding
> > load and always-on regulators there.
>
> <Merging your comment from V1 here>
>
> >> You should still call regulator_enable()/regulator_disable() on your
> >> consumer regulator in this driver. When you do this the regulator core
> >> will conclude that the regulator_dev (i.e. the part that represents the
> >> hardware) is marked always_on and will not enable/disable the regulator.
>
> >> But it will still invoke _regulator_handle_consumer_enable() and
> >> _regulator_handle_consumer_disable(), which will aggregate the "load" of
> >> all client regulators and update the regulator's load.
>
> >> So this will apply the load as you expect regardless of it being
> >> supplied by a regulator marked as always_on.
>
> Since I'm not turning off this regulator for eMMC, I wanted to keep it in
> LPM mode
> to save some power.
> When the regulator configured in auto mode (RPMH_REGULATOR_MODE_AUTO) it
> switches to LPM/HPM mode based on the active load.
> So i have to minimize my driver load requirement so that I can let this
> regulator
> in LPM mode.
> So i need to set load every-time I disable/enable the regulator.
>
You call regulator_enable(vqmmc) and regulator_disable() below, so you
are telling the regulator framework that your struct regulator should be
"on" or "off".
This will cause the sum of all struct regulator's for the underlying
struct regulator_dev to be recalculated. So after calling
regulator_disable() below your effective addition to the load
calculation is 0, regardless of which load you have specified.
Independent of this the property regulator-always-on (always_on in
struct regulator_dev) will determine if the enable/disable request will
actually be sent to the RPMh.
So, if you where to not call regulator_disable() for eMMC your argument
is correct, but as far as I can see you are and you're relying on the
regulator core to keep it always-on - and then the load logic is in
effect still.
Regards,
Bjorn
> > > + if (ret) {
> > > + dev_err(mmc_dev(mmc), "%s: vqmmc set load failed: %d\n",
> > > + mmc_hostname(mmc), ret);
> > > + goto out;
> > > + }
> > > + }
> > > +
> > > + if (level) {
> > > + /* Set the IO voltage regulator to default voltage level */
> > > + if (msm_host->caps_0 & CORE_3_0V_SUPPORT)
> > > + ios.signal_voltage = MMC_SIGNAL_VOLTAGE_330;
> > > + else if (msm_host->caps_0 & CORE_1_8V_SUPPORT)
> > > + ios.signal_voltage = MMC_SIGNAL_VOLTAGE_180;
> > > +
> > > + if (msm_host->caps_0 & CORE_VOLT_SUPPORT) {
> > > + ret = mmc_regulator_set_vqmmc(mmc, &ios);
> > > + if (ret < 0) {
> > > + dev_err(mmc_dev(mmc), "%s: vqmmc set volgate failed: %d\n",
> > > + mmc_hostname(mmc), ret);
> > > + goto out;
> > > + }
> > > + }
> > > + ret = regulator_enable(mmc->supply.vqmmc);
> > > + } else {
> > > + ret = regulator_disable(mmc->supply.vqmmc);
> > > + }
> > > +
> > > + if (ret)
> > > + dev_err(mmc_dev(mmc), "%s: vqmm %sable failed: %d\n",
> > > + mmc_hostname(mmc), level ? "en":"dis", ret);
> > > + else
> > > + msm_host->vqmmc_enabled = level;
> > > +out:
> > > + return ret;
> > > +}
^ permalink raw reply
* Re: [PATCH v7 3/5] remoteproc: Add support for runtime PM
From: Paul Cercueil @ 2020-05-22 17:11 UTC (permalink / raw)
To: Suman Anna
Cc: Bjorn Andersson, Ohad Ben-Cohen, Arnaud Pouliquen, od,
linux-remoteproc, devicetree, linux-kernel, Tero Kristo
In-Reply-To: <035bf8ad-3ef0-8314-ae5c-a94a24c230c8@ti.com>
Hi Suman,
Le ven. 22 mai 2020 à 11:47, Suman Anna <s-anna@ti.com> a écrit :
> Hi Paul,
>
> On 5/15/20 5:43 AM, Paul Cercueil wrote:
>> Call pm_runtime_get_sync() before the firmware is loaded, and
>> pm_runtime_put() after the remote processor has been stopped.
>>
>> Even though the remoteproc device has no PM callbacks, this allows
>> the
>> parent device's PM callbacks to be properly called.
>
> I see this patch staged now for 5.8, and the latest -next branch has
> broken the pm-runtime autosuspend feature we have in the OMAP
> remoteproc driver. See commit 5f31b232c674 ("remoteproc/omap: Add
> support for runtime auto-suspend/resume").
>
> What was the original purpose of this patch, because there can be
> differing backends across different SoCs.
Did you try pm_suspend_ignore_children()? It looks like it was made for
your use-case.
Cheers,
-Paul
>
> regards
> Suman
>
>>
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>>
>> Notes:
>> v2-v4: No change
>> v5: Move calls to prepare/unprepare to
>> rproc_fw_boot/rproc_shutdown
>> v6: Instead of prepare/unprepare callbacks, use PM runtime
>> callbacks
>> v7: Check return value of pm_runtime_get_sync()
>>
>> drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_core.c
>> b/drivers/remoteproc/remoteproc_core.c
>> index a7f96bc98406..e33d1ef27981 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -29,6 +29,7 @@
>> #include <linux/devcoredump.h>
>> #include <linux/rculist.h>
>> #include <linux/remoteproc.h>
>> +#include <linux/pm_runtime.h>
>> #include <linux/iommu.h>
>> #include <linux/idr.h>
>> #include <linux/elf.h>
>> @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc *rproc,
>> const struct firmware *fw)
>> if (ret)
>> return ret;
>> \x7f+ ret = pm_runtime_get_sync(dev);
>> + if (ret < 0) {
>> + dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret);
>> + return ret;
>> + }
>> +
>> dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size);
>> \x7f /*
>> @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc *rproc,
>> const struct firmware *fw)
>> ret = rproc_enable_iommu(rproc);
>> if (ret) {
>> dev_err(dev, "can't enable iommu: %d\n", ret);
>> - return ret;
>> + goto put_pm_runtime;
>> }
>> \x7f rproc->bootaddr = rproc_get_boot_addr(rproc, fw);
>> @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc *rproc,
>> const struct firmware *fw)
>> rproc->table_ptr = NULL;
>> disable_iommu:
>> rproc_disable_iommu(rproc);
>> +put_pm_runtime:
>> + pm_runtime_put(dev);
>> return ret;
>> }
>> \x7f@@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc)
>> \x7f rproc_disable_iommu(rproc);
>> \x7f+ pm_runtime_put(dev);
>> +
>> /* Free the copy of the resource table */
>> kfree(rproc->cached_table);
>> rproc->cached_table = NULL;
>> @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device *dev,
>> const char *name,
>> \x7f rproc->state = RPROC_OFFLINE;
>> \x7f+ pm_runtime_no_callbacks(&rproc->dev);
>> + pm_runtime_enable(&rproc->dev);
>> +
>> return rproc;
>> }
>> EXPORT_SYMBOL(rproc_alloc);
>> @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc);
>> */
>> void rproc_free(struct rproc *rproc)
>> {
>> + pm_runtime_disable(&rproc->dev);
>> put_device(&rproc->dev);
>> }
>> EXPORT_SYMBOL(rproc_free);
>>
>
^ permalink raw reply
* Re: [PATCH 3/4] remoteproc: add support for a new 64-bit trace version
From: Bjorn Andersson @ 2020-05-22 17:33 UTC (permalink / raw)
To: Suman Anna
Cc: Rob Herring, Mathieu Poirier, Clement Leger, Loic Pallardy,
Arnaud Pouliquen, Lokesh Vutla, linux-remoteproc, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <f15b0c6d-eee8-b839-0c79-a5316dbbfa7b@ti.com>
On Fri 22 May 09:54 PDT 2020, Suman Anna wrote:
> On 5/21/20 2:42 PM, Suman Anna wrote:
> > Hi Bjorn,
> >
> > On 5/21/20 1:04 PM, Bjorn Andersson wrote:
> > > On Wed 25 Mar 13:47 PDT 2020, Suman Anna wrote:
[..]
> > > > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
[..]
> > > > +struct fw_rsc_trace2 {
> > >
> > > Sounds more like fw_rsc_trace64 to me - in particular since the version
> > > of trace2 is 1...
> >
> > Yeah, will rename this.
> >
> > >
> > > > + u32 padding;
> > > > + u64 da;
> > > > + u32 len;
> > > > + u32 reserved;
> > >
> > > What's the purpose of this reserved field?
> >
> > Partly to make sure the entire resource is aligned on an 8-byte, and
> > partly copied over from fw_rsc_trace entry. I guess 32-bits is already
> > large enough of a size for trace entries irrespective of 32-bit or
> > 64-bit traces, so I doubt if we want to make the len field also a u64.
>
> Looking at this again, I can drop both padding and reserved fields, if I
> move the len field before da. Any preferences/comments?
>
Sounds good to me.
Thanks,
Bjorn
^ permalink raw reply
* Re: [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper
From: Kees Cook @ 2020-05-22 17:34 UTC (permalink / raw)
To: Petr Mladek
Cc: Pavel Tatashin, Anton Vorontsov, Colin Cross, Tony Luck,
Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
devicetree, linuxppc-dev
In-Reply-To: <20200522165120.GL3464@linux-b0ei>
On Fri, May 22, 2020 at 06:51:20PM +0200, Petr Mladek wrote:
> On Fri 2020-05-15 11:44:30, Kees Cook wrote:
> > From: Pavel Tatashin <pasha.tatashin@soleen.com>
> >
> > kmsg_dump() allows to dump kmesg buffer for various system events: oops,
> > panic, reboot, etc. It provides an interface to register a callback call
> > for clients, and in that callback interface there is a field "max_reason"
> > which gets ignored unless always_kmsg_dump is passed as kernel parameter.
>
> Strictly speaking, this is not fully true. "max_reason" field is not
> ignored when set to KMSG_DUMP_PANIC even when always_kmsg_dump was not set.
>
> It should be something like:
>
> "which gets ignored for reason higher than KMSG_DUMP_OOPS unless
> always_kmsg_dump is passed as kernel parameter".
>
> Heh, I wonder if anyone will be able to parse this ;-)
Ah yeah, good point. I've reworded things like this:
kmsg_dump() allows to dump kmesg buffer for various system events: oops,
panic, reboot, etc. It provides an interface to register a callback
call for clients, and in that callback interface there is a field
"max_reason", but it was getting ignored when set to any "reason"
higher than KMSG_DUMP_OOPS unless "always_kmsg_dump" was passed as
kernel parameter.
Allow clients to actually control their "max_reason", and keep the
current behavior when "max_reason" is not set.
> Otherwise, it looks good to me. With the updated commit message:
>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
Thanks!
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v2 0/3] Re-introduce TX FIFO resize for larger EP bursting
From: Wesley Cheng @ 2020-05-22 17:49 UTC (permalink / raw)
To: Felipe Balbi, agross, bjorn.andersson, robh+dt, gregkh
Cc: linux-arm-msm, devicetree, linux-kernel, linux-usb, jackp
In-Reply-To: <87o8qgwazy.fsf@kernel.org>
On 5/22/2020 2:54 AM, Felipe Balbi wrote:
> Wesley Cheng <wcheng@codeaurora.org> writes:
>
>> Changes in V2:
>> - Modified TXFIFO resizing logic to ensure that each EP is reserved a
>> FIFO.
>> - Removed dev_dbg() prints and fixed typos from patches
>> - Added some more description on the dt-bindings commit message
>>
>> Reviewed-by: Felipe Balbi <balbi@kernel.org>
>
> I don't remember giving you a Reviewed-by, did I?
>
Hi Felipe,
Sorry, I put the Reviewed-by tag by mistake, I sent a follow up email to
disregard the tags. If you need me to resubmit the patch series
version, please let me know. Thanks!
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter
From: Grygorii Strashko @ 2020-05-22 17:59 UTC (permalink / raw)
To: Bibby Hsieh, Wolfram Sang, Bartosz Golaszewski, linux-i2c
Cc: tfiga, drinkcat, srv_heupstream, robh+dt, mark.rutland,
devicetree, Rafael J . Wysocki
In-Reply-To: <20200519072729.7268-3-bibby.hsieh@mediatek.com>
On 19/05/2020 10:27, Bibby Hsieh wrote:
> Although in the most platforms, the bus power of i2c
> are alway on, some platforms disable the i2c bus power
> in order to meet low power request.
>
> We get and enable bulk regulator in i2c adapter device.
>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> ---
> drivers/i2c/i2c-core-base.c | 84 +++++++++++++++++++++++++++++++++++++
> include/linux/i2c.h | 2 +
> 2 files changed, 86 insertions(+)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 5cc0b0ec5570..e1cc8d46bc51 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -313,12 +313,14 @@ static int i2c_smbus_host_notify_to_irq(const struct i2c_client *client)
> static int i2c_device_probe(struct device *dev)
> {
> struct i2c_client *client = i2c_verify_client(dev);
> + struct i2c_adapter *adap;
> struct i2c_driver *driver;
> int status;
>
> if (!client)
> return 0;
>
> + adap = client->adapter;
> driver = to_i2c_driver(dev->driver);
>
> client->irq = client->init_irq;
> @@ -378,6 +380,12 @@ static int i2c_device_probe(struct device *dev)
>
> dev_dbg(dev, "probe\n");
>
> + status = regulator_enable(adap->bus_regulator);
> + if (status < 0) {
> + dev_err(&adap->dev, "Failed to enable power regulator\n");
> + goto err_clear_wakeup_irq;
> + }
> +
> status = of_clk_set_defaults(dev->of_node, false);
> if (status < 0)
> goto err_clear_wakeup_irq;
> @@ -414,12 +422,14 @@ static int i2c_device_probe(struct device *dev)
> static int i2c_device_remove(struct device *dev)
> {
> struct i2c_client *client = i2c_verify_client(dev);
> + struct i2c_adapter *adap;
> struct i2c_driver *driver;
> int status = 0;
>
> if (!client || !dev->driver)
> return 0;
>
> + adap = client->adapter;
> driver = to_i2c_driver(dev->driver);
> if (driver->remove) {
> dev_dbg(dev, "remove\n");
> @@ -427,6 +437,8 @@ static int i2c_device_remove(struct device *dev)
> }
>
> dev_pm_domain_detach(&client->dev, true);
> + if (!pm_runtime_status_suspended(&client->dev))
> + regulator_disable(adap->bus_regulator);
Not sure this check is correct.
i2c_device_probe()
- regulator_enable - 1
pm_runtime_get()
- regulator_enable - 2
i2c_device_remove()
- pm_runtime_status_suspended() flase
- regulator_disable() - 1 --> still active?
Sorry, I probably missing smth.
>
> dev_pm_clear_wake_irq(&client->dev);
> device_init_wakeup(&client->dev, false);
> @@ -438,6 +450,72 @@ static int i2c_device_remove(struct device *dev)
> return status;
> }
>
[...]
--
Best regards,
grygorii
^ 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