Re: [PATCH] ata: pata_rb532: Add OF support and make COMPILE_TESTable
Hello! On 4/20/21 5:04 PM, Thomas Bogendoerfer wrote: > Add OF support for switching RB532 do device tree possible. I couldnb't parse that. :-) > By removing > the not needed asm/mach-rc32434/rb.h include the driver could be > compile tested now. I think it's a separte issue worth its own patch. > > Signed-off-by: Thomas Bogendoerfer [...] MBR, Segrei
Re: [PATCH] net: ethernet: ravb: Fix release of refclk
Hello!
On 17.04.2021 16:23, Adam Ford wrote:
The call to clk_disable_unprepare() can happen before priv is
initialized.
Mhm, how's that? :-/
This means moving clk_disable_unprepare out of
out_release into a new label.
Fixes: 8ef7adc6beb2("net: ethernet: ravb: Enable optional refclk")
Signed-off-by: Adam Ford
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 8c84c40ab9a0..64a545c98ff2 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
@@ -2252,8 +2252,9 @@ static int ravb_probe(struct platform_device *pdev)
/* Stop PTP Clock driver */
if (chip_id != RCAR_GEN2)
ravb_ptp_stop(ndev);
-out_release:
+out_unprepare_refclk:
I'd really prefer out_disable_refclk.
clk_disable_unprepare(priv->refclk);
+out_release:
free_netdev(ndev);
pm_runtime_put(>dev);
MBR, Sergei
Re: [PATCH v2] MIPS: Makefile: Replace -pg with CC_FLAGS_FTRACE
Hello! On 19.04.2021 8:51, zhaoxiao wrote: In preparation for mips supporting ftrace built on other compiler options, let's have the mips Makefiles remove the $(CC_FLAGS_FTRACE) flags, whatever these may be, rather than assuming '-pg'. But your patch is adding 'em, not removing? Signed-off-by: zhaoxiao --- v2:add this same change be appropriate to all of the below Makefile. arch/mips/boot/compressed/Makefile | 2 +- arch/mips/kernel/Makefile | 8 arch/mips/vdso/Makefile| 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile index f93f72bcba97..e4b7839293e1 100644 --- a/arch/mips/boot/compressed/Makefile +++ b/arch/mips/boot/compressed/Makefile @@ -18,7 +18,7 @@ include $(srctree)/arch/mips/Kbuild.platforms BOOT_HEAP_SIZE := 0x40 # Disable Function Tracer -KBUILD_CFLAGS := $(filter-out -pg, $(KBUILD_CFLAGS)) +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE), $(KBUILD_CFLAGS)) KBUILD_CFLAGS := $(filter-out -fstack-protector, $(KBUILD_CFLAGS)) diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile index b4a57f1de772..814b3da30501 100644 --- a/arch/mips/kernel/Makefile +++ b/arch/mips/kernel/Makefile @@ -17,10 +17,10 @@ obj-y += cpu-probe.o endif ifdef CONFIG_FUNCTION_TRACER -CFLAGS_REMOVE_ftrace.o = -pg -CFLAGS_REMOVE_early_printk.o = -pg -CFLAGS_REMOVE_perf_event.o = -pg -CFLAGS_REMOVE_perf_event_mipsxx.o = -pg +CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE) +CFLAGS_REMOVE_early_printk.o = $(CC_FLAGS_FTRACE) +CFLAGS_REMOVE_perf_event.o = $(CC_FLAGS_FTRACE) +CFLAGS_REMOVE_perf_event_mipsxx.o = $(CC_FLAGS_FTRACE) endif obj-$(CONFIG_CEVT_BCM1480) += cevt-bcm1480.o diff --git a/arch/mips/vdso/Makefile b/arch/mips/vdso/Makefile index 2131d3fd7333..1b2ea34c3d3b 100644 --- a/arch/mips/vdso/Makefile +++ b/arch/mips/vdso/Makefile @@ -46,7 +46,7 @@ CFLAGS_vgettimeofday-o32.o = -include $(srctree)/$(src)/config-n32-o32-env.c -in CFLAGS_vgettimeofday-n32.o = -include $(srctree)/$(src)/config-n32-o32-env.c -include $(c-gettimeofday-y) endif -CFLAGS_REMOVE_vgettimeofday.o = -pg +CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) ifdef CONFIG_MIPS_DISABLE_VDSO ifndef CONFIG_MIPS_LD_CAN_LINK_VDSO @@ -60,7 +60,7 @@ ldflags-y := -Bsymbolic --no-undefined -soname=linux-vdso.so.1 \ $(filter -E%,$(KBUILD_CFLAGS)) -nostdlib -shared \ -G 0 --eh-frame-hdr --hash-style=sysv --build-id=sha1 -T -CFLAGS_REMOVE_vdso.o = -pg +CFLAGS_REMOVE_vdso.o = $(CC_FLAGS_FTRACE) GCOV_PROFILE := n UBSAN_SANITIZE := n MBR, Sergei
Re: [PATCH v5 net-next 10/10] dt-bindings: net: korina: Add DT bindings for IDT 79RC3243x SoCs
On 16.04.2021 16:35, Thomas Bogendoerfer wrote:
On Fri, Apr 16, 2021 at 12:29:46PM +0300, Sergei Shtylyov wrote:
On 16.04.2021 11:52, Thomas Bogendoerfer wrote:
Add device tree bindings for ethernet controller integrated into
IDT 79RC3243x SoCs.
Signed-off-by: Thomas Bogendoerfer
---
.../bindings/net/idt,3243x-emac.yaml | 74 +++
1 file changed, 74 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/idt,3243x-emac.yaml
diff --git a/Documentation/devicetree/bindings/net/idt,3243x-emac.yaml
b/Documentation/devicetree/bindings/net/idt,3243x-emac.yaml
new file mode 100644
index ..3697af5cb66f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/idt,3243x-emac.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/idt,3243x-emac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IDT 79rc3243x Ethernet controller
+
+description: Ethernet controller integrated into IDT 79RC3243x family SoCs
+
+maintainers:
+ - Thomas Bogendoerfer
+
+allOf:
+ - $ref: ethernet-controller.yaml#
+
+properties:
+ compatible:
+const: idt,3243x-emac
+
+ reg:
+maxItems: 3
+
+ reg-names:
+items:
+ - const: korina_regs
+ - const: korina_dma_rx
+ - const: korina_dma_tx
+
+ interrupts:
+items:
+ - description: RX interrupt
+ - description: TX interrupt
+
+ interrupt-names:
+items:
+ - const: korina_rx
+ - const: korina_tx
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+items:
+ - const: mdioclk
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - interrupt-names
+
+additionalProperties: false
+
+examples:
+ - |
+
+ethernet@6 {
+compatible = "idt,3243x-emac";
+
+reg = <0x6 0x1>,
+ <0x4 0x14>,
+ <0x40014 0x14>;
+reg-names = "korina_regs",
+"korina_dma_rx",
+"korina_dma_tx";
+
+interrupts-extended = < 0>, < 1>;
You use this prop, yet don't describe it?
that's just interrupt-parent and interrupts in one statement. And since
I know. :-) Yet you don't mention it, making the "interrupts" prop
mandatory instead. And as both interrupt parents are the same here, it does
not even seem justified...
make dt_binding_check didn't complained I thought that's good this way.
Rob, do I need to describe interrupts-extended as well ?
I could change that to interrupt-parent/interrupts as the driver no
longer uses dma under/overrun interrupts, which have a different
interrupt-parent.
In DT, we describe the hardware, not the driver's capabilities.
Thomas.
MBR, Sergei
Re: [PATCH] usb: misc: adutux: fix whitespace coding style issue
On 16.04.2021 11:08, Malte Deiseroth wrote: Correct missing space error ceckpatch.pl is complaining about. It's called checkpatch.pl. :-) Signed-off-by: Malte Deiseroth [...] MBR, Sergei
Re: [PATCH v5 net-next 10/10] dt-bindings: net: korina: Add DT bindings for IDT 79RC3243x SoCs
On 16.04.2021 11:52, Thomas Bogendoerfer wrote:
Add device tree bindings for ethernet controller integrated into
IDT 79RC3243x SoCs.
Signed-off-by: Thomas Bogendoerfer
---
.../bindings/net/idt,3243x-emac.yaml | 74 +++
1 file changed, 74 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/idt,3243x-emac.yaml
diff --git a/Documentation/devicetree/bindings/net/idt,3243x-emac.yaml
b/Documentation/devicetree/bindings/net/idt,3243x-emac.yaml
new file mode 100644
index ..3697af5cb66f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/idt,3243x-emac.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/idt,3243x-emac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IDT 79rc3243x Ethernet controller
+
+description: Ethernet controller integrated into IDT 79RC3243x family SoCs
+
+maintainers:
+ - Thomas Bogendoerfer
+
+allOf:
+ - $ref: ethernet-controller.yaml#
+
+properties:
+ compatible:
+const: idt,3243x-emac
+
+ reg:
+maxItems: 3
+
+ reg-names:
+items:
+ - const: korina_regs
+ - const: korina_dma_rx
+ - const: korina_dma_tx
+
+ interrupts:
+items:
+ - description: RX interrupt
+ - description: TX interrupt
+
+ interrupt-names:
+items:
+ - const: korina_rx
+ - const: korina_tx
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+items:
+ - const: mdioclk
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - interrupt-names
+
+additionalProperties: false
+
+examples:
+ - |
+
+ethernet@6 {
+compatible = "idt,3243x-emac";
+
+reg = <0x6 0x1>,
+ <0x4 0x14>,
+ <0x40014 0x14>;
+reg-names = "korina_regs",
+"korina_dma_rx",
+"korina_dma_tx";
+
+interrupts-extended = < 0>, < 1>;
You use this prop, yet don't describe it?
+interrupt-names = "korina_rx", "korina_tx";
+
+clocks = <>;
+clock-names = "mdioclk";
+};
MBR, Sergei
Re: [PATCH v4 net-next 07/10] net: korina: Add support for device tree
Hello!
On 16.04.2021 11:47, Thomas Bogendoerfer wrote:
If there is no mac address passed via platform data try to get it via > device
tree and fall back to a random mac address, if all fail.
Signed-off-by: Thomas Bogendoerfer
---
drivers/net/ethernet/korina.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/korina.c b/drivers/net/ethernet/korina.c
index d6dbbdd43d7c..cd078a5c679b 100644
--- a/drivers/net/ethernet/korina.c
+++ b/drivers/net/ethernet/korina.c
@@ -43,6 +43,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -1068,7 +1070,12 @@ static int korina_probe(struct platform_device *pdev)
SET_NETDEV_DEV(dev, >dev);
lp = netdev_priv(dev);
- memcpy(dev->dev_addr, mac_addr, ETH_ALEN);
+ if (mac_addr) {
+ ether_addr_copy(dev->dev_addr, mac_addr);
+ } else {
+ if (of_get_mac_address(pdev->dev.of_node, dev->dev_addr))
*else* *if* here, and no need for {} then? :-)
+ eth_hw_addr_random(dev);
+ }
lp->rx_irq = platform_get_irq_byname(pdev, "korina_rx");
lp->tx_irq = platform_get_irq_byname(pdev, "korina_tx");
[...]
MBR, Sergei
Re: [PATCH 13/15] usb: dwc2: Add exit hibernation mode before removing drive
On 15.04.2021 8:41, Artur Petrosyan wrote:
When dwc2 core is in hibernation mode loading
driver again causes driver fail. Because in
that mode registers are not accessible.
In order to exit from hibernation checking
dwc2 core power saving state in "dwc2_driver_remove()"
function. If core is in hibernation, then checking the
operational mode of the driver. To check whether dwc2 core
is operating in host mode or device mode there is one way
which is retrieving the backup value of "gotgctl" and compare
the "CurMod" value. If previously core entered hibernation
in host mode then the exit is performed for host if not then
exit is performed for device mode. The introduced checking
is because in hibernation state all registers are not
accessible.
Signed-off-by: Artur Petrosyan
---
drivers/usb/dwc2/platform.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index f8b819cfa80e..2ae4748ed5ec 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -316,8 +316,24 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
static int dwc2_driver_remove(struct platform_device *dev)
{
struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
+ struct dwc2_gregs_backup *gr;
int ret = 0;
+ /* Exit Hibernation when driver is removed. */
+ if (hsotg->hibernated) {
+ if (gr->gotgctl & GOTGCTL_CURMODE_HOST) {
+ ret = dwc2_exit_hibernation(hsotg, 0, 0, 1);
+ if (ret)
+ dev_err(hsotg->dev,
+ "exit hibernation failed.\n");
+ } else {
+ ret = dwc2_exit_hibernation(hsotg, 0, 0, 0);
+ if (ret)
+ dev_err(hsotg->dev,
+ "exit hibernation failed.\n");
Again, why duplicate the innermost *if*?
+ }
+ }
+
/* Exit Partial Power Down when driver is removed. */
if (hsotg->in_ppd) {
ret = dwc2_exit_partial_power_down(hsotg, 0, true);
MBR, Sergei
Re: [PATCH 10/15] usb: dwc2: Allow exit hibernation in urb enqueue
On 15.04.2021 8:40, Artur Petrosyan wrote:
When core is in hibernation state and an external
hub is connected, upper layer sends URB enqueue request,
which results in port reset issue.
- Added exit from hibernation state to avoid port
reset issue and process upper layer request properly.
Signed-off-by: Artur Petrosyan
---
drivers/usb/dwc2/hcd.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index cc9ad6cf02d9..3b03b2d73aaa 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4631,12 +4631,29 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd,
struct urb *urb,
struct dwc2_qh *qh;
bool qh_allocated = false;
struct dwc2_qtd *qtd;
+ struct dwc2_gregs_backup *gr;
+
+ gr = >gr_backup;
if (dbg_urb(urb)) {
dev_vdbg(hsotg->dev, "DWC OTG HCD URB Enqueue\n");
dwc2_dump_urb_info(hcd, urb, "urb_enqueue");
}
+ if (hsotg->hibernated) {
+ if (gr->gotgctl & GOTGCTL_CURMODE_HOST) {
+ retval = dwc2_exit_hibernation(hsotg, 0, 0, 1);
+ if (retval)
+ dev_err(hsotg->dev,
+ "exit hibernation failed.\n");
+ } else {
+ retval = dwc2_exit_hibernation(hsotg, 0, 0, 0);
+ if (retval)
+ dev_err(hsotg->dev,
+ "exit hibernation failed.\n");
Why not put these identical *if*s outside the the outer *if*?
+ }
+ }
+
if (hsotg->in_ppd) {
retval = dwc2_exit_partial_power_down(hsotg, 0, true);
if (retval)
MBR, Sergei
Re: [PATCH v2 11/12] usb: dwc2: Add clock gating exiting flow by system resume
On 13.04.2021 10:37, Artur Petrosyan wrote: If not hibernation nor partial power down are supported, s/not/neither/? port resume is done using the clock gating programming flow. Adds a new flow of exiting clock gating when PC is resumed. Signed-off-by: Artur Petrosyan --- Changes in v2: - None drivers/usb/dwc2/hcd.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 09dcd37b9ef8..04a1b53d65af 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -4445,6 +4445,28 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd) break; case DWC2_POWER_DOWN_PARAM_HIBERNATION: case DWC2_POWER_DOWN_PARAM_NONE: + /* +* If not hibernation nor partial power down are supported, s/not/neither/? [...] MBR, Sergei
Re: [PATCH v2 10/12] usb: dwc2: Add clock gating entering flow by system suspend
On 13.04.2021 10:37, Artur Petrosyan wrote: If not hibernation nor partial power down are supported, s/not/neither/? clock gating is used to save power. Adds a new flow of entering clock gating when PC is suspended. Signed-off-by: Artur Petrosyan --- Changes in v2: - None drivers/usb/dwc2/hcd.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 31d6a1b87228..09dcd37b9ef8 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -4372,6 +4372,15 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) break; case DWC2_POWER_DOWN_PARAM_HIBERNATION: case DWC2_POWER_DOWN_PARAM_NONE: + /* +* If not hibernation nor partial power down are supported, s/not/neither/? +* clock gating is used to save power. +*/ + dwc2_host_enter_clock_gating(hsotg); + + /* After entering suspend, hardware is not accessible */ + clear_bit(HCD_FLAG_HW_ACCESSIBLE, >flags); + break; default: goto skip_power_saving; } MBR, Sergei
Re: [PATCH 03/12] usb: dwc2: Allow entering clock gating from USB_SUSPEND interrupt
On 13.04.2021 10:16, Artur Petrosyan wrote:
If core doesn't support hibernation or partial power
down power saving options, power can still be saved
using clock gating on all the clocks.
- Added entering clock gating state from USB_SUSPEND
interrupt.
Signed-off-by: Artur Petrosyan
Acked-by: Minas Harutyunyan
---
drivers/usb/dwc2/core_intr.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index 8c0152b514be..ab7fe303c0f9 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -529,14 +529,18 @@ static void dwc2_handle_usb_suspend_intr(struct
dwc2_hsotg *hsotg)
/* Ask phy to be suspended */
if (!IS_ERR_OR_NULL(hsotg->uphy))
usb_phy_set_suspend(hsotg->uphy, true);
- }
-
- if (hsotg->hw_params.hibernation) {
+ } else if (hsotg->hw_params.hibernation) {
ret = dwc2_enter_hibernation(hsotg, 0);
if (ret && ret != -ENOTSUPP)
dev_err(hsotg->dev,
"%s: enter hibernation
failed\n",
__func__);
+ } else {
+ /*
+* If not hibernation nor partial power down
are supported,
s/not/neither/?
[...]
MBR, Sergei
Re: [PATCH 07/12] usb: dwc2: Update enter clock gating when port is suspended
Hello! On 13.04.2021 10:17, Artur Petrosyan wrote: Updates the implementation of entering clock gating mode when core receives port suspend. Instead of setting the required bit fields of the registers inline, called the "dwc2_host_enter_clock_gating()" function. Signed-off-by: Artur Petrosyan --- drivers/usb/dwc2/hcd.c | 19 --- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 27f030d5de54..e1225fe6c61a 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c [...] @@ -3323,22 +3322,12 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) break; case DWC2_POWER_DOWN_PARAM_HIBERNATION: case DWC2_POWER_DOWN_PARAM_NONE: - default: - hprt0 = dwc2_read_hprt0(hsotg); - hprt0 |= HPRT0_SUSP; - dwc2_writel(hsotg, hprt0, HPRT0); - hsotg->bus_suspended = true; /* -* If power_down is supported, Phy clock will be suspended -* after registers are backuped. +* If not hibernation nor partial power down are supported, s/not/neither/? MBR, Sergei
Re: [PATCH v1] pata_ipx4xx_cf: Fix unsigned comparison with less than zero
On 10.04.2021 2:42, angkery wrote: From: Junlin Yang The return from the call to platform_get_irq() is int, it can be a negative error code, however this is being assigned to an unsigned int variable 'irq', so making 'irq' an int, and change the position to keep the code format. Fixes coccicheck warnings: ./drivers/ata/pata_ixp4xx_cf.c:168:5-8: WARNING: Unsigned expression compared with zero: irq > 0 Signed-off-by: Junlin Yang --- changes in v1: update the commit information. Not seeing what's updated, and anwyways it should read irq < 0, else it doesn't make sense... [...] MBR, Sergei
Re: [PATCH] pata_ipx4xx_cf: Fix unsigned comparison with less than zero
On 10.04.2021 2:38, angkery wrote: On Fri, 9 Apr 2021 20:02:56 +0300 Sergei Shtylyov wrote: On 4/9/21 7:49 PM, Sergei Shtylyov wrote: From: Junlin Yang The return from the call to platform_get_irq() is int, it can be a negative error code, however this is being assigned to an unsigned int variable 'irq', so making 'irq' an int, and change the position to keep the code format. ./drivers/ata/pata_ixp4xx_cf.c:168:5-8: WARNING: Unsigned expression compared with zero: irq > 0 I'd understand < 0... but > 0? What tool warned about this issue? yes, by coccicheck, i will update commits information. Thanks! Yet, looking at your subject, the above should read irq < 0, not > 0. Signed-off-by: Junlin Yang
Re: [PATCH] pata_ipx4xx_cf: Fix unsigned comparison with less than zero
On 4/9/21 7:49 PM, Sergei Shtylyov wrote:
>> From: Junlin Yang
>>
>> The return from the call to platform_get_irq() is int, it can be
>> a negative error code, however this is being assigned to an unsigned
>> int variable 'irq', so making 'irq' an int, and change the position to
>> keep the code format.
>>
>> ./drivers/ata/pata_ixp4xx_cf.c:168:5-8:
>> WARNING: Unsigned expression compared with zero: irq > 0
>
>I'd understand < 0... but > 0? What tool warned about this issue?
>
>> Signed-off-by: Junlin Yang
>> ---
>> drivers/ata/pata_ixp4xx_cf.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
>> index abc0e87..43215a4 100644
>> --- a/drivers/ata/pata_ixp4xx_cf.c
>> +++ b/drivers/ata/pata_ixp4xx_cf.c
>> @@ -135,12 +135,12 @@ static void ixp4xx_setup_port(struct ata_port *ap,
>>
>> static int ixp4xx_pata_probe(struct platform_device *pdev)
>> {
>> - unsigned int irq;
>
>Oops, not sure how I missed that. :-/
>Thanks for fixing my overlook. :-)
Forgot to add my
Reviewed-by: Sergei Shtylyov
> [...]
MBR, Sergei
Re: [PATCH] pata_ipx4xx_cf: Fix unsigned comparison with less than zero
On 4/9/21 4:54 PM, angkery wrote:
> From: Junlin Yang
>
> The return from the call to platform_get_irq() is int, it can be
> a negative error code, however this is being assigned to an unsigned
> int variable 'irq', so making 'irq' an int, and change the position to
> keep the code format.
>
> ./drivers/ata/pata_ixp4xx_cf.c:168:5-8:
> WARNING: Unsigned expression compared with zero: irq > 0
I'd understand < 0... but > 0? What tool warned about this issue?
> Signed-off-by: Junlin Yang
> ---
> drivers/ata/pata_ixp4xx_cf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> index abc0e87..43215a4 100644
> --- a/drivers/ata/pata_ixp4xx_cf.c
> +++ b/drivers/ata/pata_ixp4xx_cf.c
> @@ -135,12 +135,12 @@ static void ixp4xx_setup_port(struct ata_port *ap,
>
> static int ixp4xx_pata_probe(struct platform_device *pdev)
> {
> - unsigned int irq;
Oops, not sure how I missed that. :-/
Thanks for fixing my overlook. :-)
[...]
MBR, Sergei
Re: [PATCH] MIPS: octeon: Add __raw_copy_[from|to|in]_user symbols
Hello!
On 09.04.2021 0:48, Thomas Bogendoerfer wrote:
Cavium Octeon has it's own memcpy implementation and also need the change
Its. :-)
done in commit 04324f44cb69 ("MIPS: Remove get_fs/set_fs").
Fixes: 04324f44cb69 ("MIPS: Remove get_fs/set_fs")
Reported-by: kernel test robot
Signed-off-by: Thomas Bogendoerfer
[...]
MBR, Sergei
Re: [PATCH 2/4] exec: remove compat_do_execve
On 3/26/21 5:38 PM, Christoph Hellwig wrote:
> Just call compat_do_execve instead.
compat_do_execveat(), maybe?
> Signed-off-by: Christoph Hellwig
> ---
> fs/exec.c | 17 +
> 1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index b63fb020909075..06e07278b456fa 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
[...]
> @@ -2072,7 +2057,7 @@ COMPAT_SYSCALL_DEFINE3(execve, const char __user *,
> filename,
> const compat_uptr_t __user *, argv,
> const compat_uptr_t __user *, envp)
> {
> - return compat_do_execve(getname(filename), argv, envp);
> + return compat_do_execveat(AT_FDCWD, getname(filename), argv, envp, 0);
> }
>
> COMPAT_SYSCALL_DEFINE5(execveat, int, fd,
MBR, Sergei
Re: [PATCH v1] usb: dwc3: core: Add shutdown callback for dwc3
Hello!
On 23.03.2021 22:27, Sandeep Maheswaram wrote:
This patch adds a shutdown callback to USB DWC core driver to ensure that
it is properly shutdown in reboot/shutdown path. This is required
where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after
SMMU translation is disabled as part of SMMU shutdown callback in
system reboot or shutdown path, then IOVAs(I/O virtual address)
Space before (, please.
which it was using will go on the bus as the physical addresses which
might result in unknown crashes (NoC/interconnect errors).
Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after
iommu shutdown. As we are adding iommu nodes in dwc core node
in device tree adding shutdown callback in core driver seems correct.
Signed-off-by: Sandeep Maheswaram
---
drivers/usb/dwc3/core.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 94fdbe5..777b2b5 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
[...]
@@ -1976,6 +1987,7 @@ MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match);
static struct platform_driver dwc3_driver = {
.probe = dwc3_probe,
.remove = dwc3_remove,
+ .shutdown = dwc3_shutdown,
Please indent = with tabs as above and below.
.driver = {
.name = "dwc3",
.of_match_table = of_match_ptr(of_dwc3_match),
MBR, Sergei
Re: [PATCH v1 4/6] usb: gadget: pch_udc: Move pch_udc_init() to satisfy kernel doc
Hi!
On 23.03.2021 0:11, Andy Shevchenko wrote:
Kernel doc and the content described by it shouldn't be teared apart.
s/teared/torn/?
Otherwise validator is not happy:
.../pch_udc.c:573: warning: expecting prototype for pch_udc_reconnect().
Prototype was for pch_udc_init() instead
Fixes: 1c575d2d2e3f ("usb: gadget: pch_udc: Fix usb/gadget/pch_udc: Fix ether gadget
connect/disconnect issue")
Signed-off-by: Andy Shevchenko
[...]
MBR, Sergei
Re: [PATCH v2 01/13] dt-bindings: usb: mtk-xhci: support property usb2-lpm-disable
Hello! On 23.03.2021 10:02, Chunfeng Yun wrote: Add support common property usb2-lpm-disable ^ for Signed-off-by: Chunfeng Yun [...] MBR, Sergei
Re: [PATCH 07/13] usb: xhci-mtk: add support ip-sleep wakeup for mT8192
On 22.03.2021 6:13, Chunfeng Yun wrote:
Add support ip-sleep wakeup for mT8192, it's a specific revision,
^ for
and not follow IPM rule.
Following?
Signed-off-by: Chunfeng Yun
---
drivers/usb/host/xhci-mtk.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 8ba1f914cb75..1bfa28c9b5a2 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -70,6 +70,10 @@
#define WC0_IS_P BIT(12) /* polarity */
#define WC0_IS_EN BIT(6)
+/* mt8192 */
+#define WC0_SSUSB0_CDENBIT(6)
+#define WC0_IS_SPM_EN BIT(1)
+
/* mt2712 etc */
#define PERI_SSUSB_SPM_CTRL 0x0
#define SSC_IP_SLEEP_EN BIT(4)
@@ -79,6 +83,7 @@ enum ssusb_uwk_vers {
SSUSB_UWK_V1 = 1,
SSUSB_UWK_V2,
SSUSB_UWK_V11 = 11, /* specific revision 1.1 */
+ SSUSB_UWK_V12, /* specific revision 1.2 */
SSUSB_UWK_V1_2, maybe?
[...]
MBR, Sergei
Re: [PATCH 10/13] usb: mtu3: support ip-sleep wakeup for MT8183
Same comments as to the patch #6. MBR, Sergei
Re: [PATCH 06/13] usb: xhci-mtk: support ip-sleep wakeup for MT8183
Hello!
On 22.03.2021 6:13, Chunfeng Yun wrote:
Add support ip-sleep wakeup for MT8183, it's similar to MT8173,
^ for
and it's also a specific one, but not follow IPM rule.
Following?
Due to the index 2 already used by many DTS, it's better to keep
it unchanged for backward compatible, treat specific ones without
Compatibility.
following IPM rule as revision 1.x, meanwhile reserve 3~10 for
later revision that follows the IPM rule.
Signed-off-by: Chunfeng Yun
---
drivers/usb/host/xhci-mtk.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 09f2ddbfe8b9..8ba1f914cb75 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -57,12 +57,19 @@
#define CTRL_U2_FORCE_PLL_STB BIT(28)
/* usb remote wakeup registers in syscon */
+
/* mt8173 etc */
#define PERI_WK_CTRL1 0x4
#define WC1_IS_C(x) (((x) & 0xf) << 26) /* cycle debounce */
#define WC1_IS_EN BIT(25)
#define WC1_IS_P BIT(6) /* polarity for ip sleep */
+/* mt8183 */
+#define PERI_WK_CTRL0 0x0
+#define WC0_IS_C(x)(((x) & 0xf) << 28) /* cycle debounce */
+#define WC0_IS_P BIT(12) /* polarity */
+#define WC0_IS_EN BIT(6)
+
/* mt2712 etc */
#define PERI_SSUSB_SPM_CTRL 0x0
#define SSC_IP_SLEEP_EN BIT(4)
@@ -71,6 +78,7 @@
enum ssusb_uwk_vers {
SSUSB_UWK_V1 = 1,
SSUSB_UWK_V2,
+ SSUSB_UWK_V11 = 11, /* specific revision 1.1 */
SSUSB_UWK_V1_1, maybe?
[...]
MBR, Sergei
Re: [PATCH V3 5/5] arm64: dts: renesas: beacon kits: Setup AVB refclk
On 18.03.2021 15:44, Adam Ford wrote: The AVB refererence clock assumes an external clock that runs reference automatically. Because the Versaclock is wired to provide the AVB refclock, the device tree needs to reference it in order for the driver to start the clock. Signed-off-by: Adam Ford Reviewed-by: Geert Uytterhoeven i.e. will queue in renesas-devel (with the typo fixed) once the DT bindings have been accepted. Who do I need to ping to get the DT bindings accepted? They have an acked-by from Rob. Normally, the bindings get picked up by a subsystem maintainer... or Rob :-) [...] MBR, Sergei
Re: [PATCH V3 1/5] dt-bindings: net: renesas,etheravb: Add additional clocks
Hi! On 2/24/21 2:51 PM, Adam Ford wrote: > The AVB driver assumes there is an external crystal, but it could > be clocked by other means. In order to enable a programmable > clock, it needs to be added to the clocks list and enabled in the > driver. Since there currently only one clock, there is no > clock-names list either. > > Update bindings to add the additional optional clock, and explicitly > name both of them. > > Signed-off-by: Adam Ford > Reviewed-by: Geert Uytterhoeven > Acked-by: Rob Herring Reviewed-by: Sergei Shtylyov [...] PS: Sorry for the dalay reviewing... MBR, Sergei
Re: [PATCH 08/10] MIPS: disable CONFIG_IDE in malta*_defconfig
On 3/18/21 7:57 AM, Christoph Hellwig wrote: > Various malta defconfigs enable CONFIG_IDE for the tc86c001 ide driver, > hich is a Toshiba plug in card that does not make much sense to use on ^ which is for > bigsur platforms. For all other ATA cards libata support is already ^ Malta. > enabled. > > Signed-off-by: Christoph Hellwig [...] MBR, Sergei
Re: [PATCH 07/10] MIPS: disable CONFIG_IDE in bigsur_defconfig
Hi! On 3/18/21 7:57 AM, Christoph Hellwig wrote: > bigsur_defconfig enables CONFIG_IDE for the tc86c001 ide driver, which > is a Toshiba plug in card that does not make much sense to use on bigsur ^ for Else that doesn't make much sense. :-) > platforms. For all other ATA cards libata support is already enabled. > > Signed-off-by: Christoph Hellwig [...] MBR, Sergei
Re: [PATCH] MIPS: ralink: define stubs for clk_set_parent to fix compile testing
On 17.03.2021 12:56, Krzysztof Kozlowski wrote:
[...]
The Ralink MIPS platform does not use Common Clock Framework and does
not define certain clock operations leading to compile test failures:
/usr/bin/mips-linux-gnu-ld: drivers/usb/phy/phy-tegra-usb.o: in function
`tegra_usb_phy_init':
phy-tegra-usb.c:(.text+0x1dd4): undefined reference to `clk_get_parent'
Reported-by: kernel test robot
Signed-off-by: Krzysztof Kozlowski
---
arch/mips/ralink/clk.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c
index 2f9d5acb38ea..8387177a47ef 100644
--- a/arch/mips/ralink/clk.c
+++ b/arch/mips/ralink/clk.c
@@ -70,6 +70,20 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
}
EXPORT_SYMBOL_GPL(clk_round_rate);
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+ WARN_ON(clk);
+ return -1;
Shouldn't this be a proepr error code (-1 corresponds to -EPRERM)?
Could be ENODEV or EINVAL but all other stubs here and in ar7/clock.c
use -1. Do you prefer it then to have it inconsistent with others?
No. :-)
Best regards,
Krzysztof
MBR, Sergei
Re: [PATCH] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again
On 17.03.2021 10:15, Tiezhu Yang wrote:
After commit 0ebeea8ca8a4 ("bpf: Restrict bpf_probe_read{, str}() only to
archs where they work"), bpf_probe_read{, str}() functions were not longer
No longer.
available on MIPS, so there exists some errors when running bpf program:
Exist.
root@linux:/home/loongson/bcc# python examples/tracing/task_switch.py
bpf: Failed to load program: Invalid argument
[...]
11: (85) call bpf_probe_read#4
unknown func bpf_probe_read#4
[...]
Exception: Failed to load BPF program count_sched: Invalid argument
So select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE in arch/mips/Kconfig,
otherwise the bpf old helper bpf_probe_read() will not be available.
This is similar with the commit d195b1d1d1196 ("powerpc/bpf: Enable
bpf_probe_read{, str}() on powerpc again").
Fixes: 0ebeea8ca8a4 ("bpf: Restrict bpf_probe_read{, str}() only to archs where they
work")
Signed-off-by: Tiezhu Yang
[...]
MBR, Sergei
Re: [PATCH] MIPS: ralink: define stubs for clk_set_parent to fix compile testing
Hello!
On 16.03.2021 20:57, Krzysztof Kozlowski wrote:
The Ralink MIPS platform does not use Common Clock Framework and does
not define certain clock operations leading to compile test failures:
/usr/bin/mips-linux-gnu-ld: drivers/usb/phy/phy-tegra-usb.o: in function
`tegra_usb_phy_init':
phy-tegra-usb.c:(.text+0x1dd4): undefined reference to `clk_get_parent'
Reported-by: kernel test robot
Signed-off-by: Krzysztof Kozlowski
---
arch/mips/ralink/clk.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c
index 2f9d5acb38ea..8387177a47ef 100644
--- a/arch/mips/ralink/clk.c
+++ b/arch/mips/ralink/clk.c
@@ -70,6 +70,20 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
}
EXPORT_SYMBOL_GPL(clk_round_rate);
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+ WARN_ON(clk);
+ return -1;
Shouldn't this be a proepr error code (-1 corresponds to -EPRERM)?
+}
+EXPORT_SYMBOL(clk_set_parent);
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+ WARN_ON(clk);
+ return NULL;
+}
+EXPORT_SYMBOL(clk_get_parent);
+
void __init plat_time_init(void)
{
struct clk *clk;
MBR, Sergei
Re: [PATCH V3] ata: sata_highbank: delete redundant print and fix return value
Hello!
On 15.03.2021 5:39, Wang Qing wrote:
platform_get_irq() has already checked and printed the return value,
the printing here is nothing special, and should corrected to < 0.
Also, thhe return value should return a real error.
The.
It's a bad idea to mix the fix and cleanup in one patch (and to do two
things in one patch as well); also describing a bug fix as an "appendage"
to the clean up. Anyway, I had already posted the fix here:
https://marc.info/?l=linux-ide=161575450917827
Signed-off-by: Wang Qing
---
drivers/ata/sata_highbank.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
index 64b2ef1..4438ee6
--- a/drivers/ata/sata_highbank.c
+++ b/drivers/ata/sata_highbank.c
@@ -469,10 +469,8 @@ static int ahci_highbank_probe(struct platform_device
*pdev)
}
irq = platform_get_irq(pdev, 0);
- if (irq <= 0) {
- dev_err(dev, "no irq\n");
- return -EINVAL;
- }
+ if (irq < 0)
+ return irq;
What about irq == 0?
[...]
MBR, Sergei
Re: [PATCH 1/2] ata: delete redundant printing of return value
Hello again. :-)
Now, 2 patches to the different files shouldn't have and identical subject!
And the patch
subject should reflect the exact patch locus, e.g. "sata_highbank: delete
redundant printing
of return value".
On 3/13/21 10:47 AM, Wang Qing wrote:
> platform_get_irq() has already checked and printed the return value,
> the printing here is nothing special, it is not necessary at all.
>
> Signed-off-by: Wang Qing
> ---
> drivers/ata/sata_highbank.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
> index 64b2ef1..a43d42a
> --- a/drivers/ata/sata_highbank.c
> +++ b/drivers/ata/sata_highbank.c
> @@ -469,10 +469,8 @@ static int ahci_highbank_probe(struct platform_device
> *pdev)
> }
>
> irq = platform_get_irq(pdev, 0);
> - if (irq <= 0) {
> - dev_err(dev, "no irq\n");
> + if (irq <= 0)
> return -EINVAL;
> - }
Again, this code has more serious issue: it breaks deferred probing by
overriding the result to -EINVAL...
MBR, Sergei
Re: [PATCH 1/2] ata: delete redundant printing of return value
Hello!
The patch subject should reflect the exact patch locus, e.g. "ata:
libahci_platform:
delete redundant printing of return value".
On 3/13/21 10:46 AM, Wang Qing wrote:
> platform_get_irq() has already checked and printed the return value,
> the printing here is nothing special, it is not necessary at all.
>
> Signed-off-by: Wang Qing
> ---
> drivers/ata/libahci_platform.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index de638da..2e7ea75
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -582,11 +582,8 @@ int ahci_platform_init_host(struct platform_device *pdev,
> int i, irq, n_ports, rc;
>
> irq = platform_get_irq(pdev, 0);
> - if (irq <= 0) {
> - if (irq != -EPROBE_DEFER)
> - dev_err(dev, "no irq\n");
> + if (irq <= 0)
> return irq;
> - }
This code has more serious problem: it returns 0 on error as if it was
success.
Should be changed to *if* (irq < 0) (in a separate patch).
MBR, Sergei
Re: [PATCH] ata: Trivial spelling fixes in the file pata_ns87415.c
Hello! For the future, the driver name should go instead (or after) the "ata: ", like this: "pata_ns87415: Trivial spelling fixes". MBR, Sergei
Re: [PATCH v4 3/7] MIPS: Loongson64: Add support for the Loongson-2K1000 to get cpu_clock_freq
Hello!
On 10.03.2021 10:56, Qing Zhang wrote:
Get the fixed-clock from the CPU0 node of the device tree.
Signed-off-by: Jiaxun Yang
Signed-off-by: Qing Zhang
Tested-by: Ming Wang
---
v3-v4: Standard submission of information
Add return after error
arch/mips/loongson64/time.c | 24
1 file changed, 24 insertions(+)
diff --git a/arch/mips/loongson64/time.c b/arch/mips/loongson64/time.c
index 91e842b58365..2d84f9b20a9b 100644
--- a/arch/mips/loongson64/time.c
+++ b/arch/mips/loongson64/time.c
@@ -11,9 +11,33 @@
#include
#include
+#include
+#include
void __init plat_time_init(void)
{
+ struct clk *clk = NULL;
You don't seem to need this initializer.
+ struct device_node *np;
+
+ if (loongson_sysconf.fw_interface == LOONGSON_DTB) {
+ of_clk_init(NULL);
+
+ np = of_get_cpu_node(0, NULL);
+ if (!np) {
+ pr_err("Failed to get CPU node\n");
+ return;
+ }
+
+ clk = of_clk_get(np, 0);
+ if (IS_ERR(clk)) {
+ pr_err("Failed to get CPU clock: %ld\n", PTR_ERR(clk));
+ return;
+ }
+
+ cpu_clock_freq = clk_get_rate(clk);
+ clk_put(clk);
+ }
+
/* setup mips r4k timer */
mips_hpt_frequency = cpu_clock_freq / 2;
Re: [PATCH v3 5/7] irqchip/loongson-liointc: irqchip add 2.0 version
Hello!
On 10.03.2021 5:26, zhangqing wrote:
[...]
+ if (of_device_is_compatible(node, "loongson,liointc-2.0")) {
+ base = liointc_get_reg_byname(node, "main");
+ if (!base) {
+ err = -ENODEV;
+ goto out_free_priv;
+ }
+ for (i = 0; i < LIOINTC_NUM_CORES; i++) {
+ priv->core_isr[i] =
+ liointc_get_reg_byname(node, core_reg_names[i]);
Please write assignments on a single line.
In addition, write assignments on a single line
for (i = 0; i priv->core_isr[i] = liointc_get_reg_byname(node,
core_reg_names[i]);
It is 92 characters, more than 80 characters...
The new line length limit is 100 chars. :-)
[...]
MBR, Sergei
Re: [PATCH] usb: typec: tcpm: turn tcpm_ams_finish into void function
Hello!
On 08.03.2021 9:21, Yang Li wrote:
This function always return '0' and no callers use the return value.
Returns.
So make it a void function.
This eliminates the following coccicheck warning:
./drivers/usb/typec/tcpm/tcpm.c:778:5-8: Unneeded variable: "ret".
Return "0" on line 794
Reported-by: Abaci Robot
Signed-off-by: Yang Li
---
drivers/usb/typec/tcpm/tcpm.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index be0b646..6ee984e 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -773,10 +773,8 @@ static enum typec_cc_status tcpm_rp_cc(struct tcpm_port
*port)
return TYPEC_CC_RP_DEF;
}
-static int tcpm_ams_finish(struct tcpm_port *port)
+static void tcpm_ams_finish(struct tcpm_port *port)
{
- int ret = 0;
-
tcpm_log(port, "AMS %s finished", tcpm_ams_str[port->ams]);
if (port->pd_capable && port->pwr_role == TYPEC_SOURCE) {
@@ -791,7 +789,7 @@ static int tcpm_ams_finish(struct tcpm_port *port)
port->in_ams = false;
port->ams = NONE_AMS;
- return ret;
+ return;
Just remove that *return*.
}
[...]
MBR, Sergei
Re: [PATCH 03/17] usb: xhci-mtk: get the microframe boundary for ESIT
Hello!
On 05.03.2021 12:02, Chunfeng Yun wrote:
Tune the boundary for FS/LS ESIT due to CS:
For ISOC out-ep, the controller starts transfer data after
the first SS; for others, the data is already transfered
Transferred.
before the last CS.
Signed-off-by: Chunfeng Yun
---
drivers/usb/host/xhci-mtk-sch.c | 24 +++-
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index 8950d1f10a7f..e3b18dfca874 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -513,22 +513,35 @@ static void update_sch_tt(struct usb_device *udev,
list_del(_ep->tt_endpoint);
}
+static u32 get_esit_boundary(struct mu3h_sch_ep_info *sch_ep)
+{
+ u32 boundary = sch_ep->esit;
+
+ if (sch_ep->sch_tt) { /* LS/FS with TT */
+ /* tune for CS */
+ if (sch_ep->ep_type != ISOC_OUT_EP)
+ boundary += 1;
Why not:
boundary++;
+ else if (boundary > 1) /* normally esit >= 8 for FS/LS */
+ boundary -= 1;
boundary--;
[...]
MBR, Sergei
Re: [PATCH] arch: mips: sibyte: Return -EFAULT if copy_to_user() fails
Hello!
On 01.03.2021 9:33, Wang Qing wrote:
The copy_to_user() function returns the number of bytes remaining to be
copied, but we want to return -EFAULT if the copy doesn't complete.
Then 'err' is hardly a good name for that variable. :-)
Signed-off-by: Wang Qing
---
arch/mips/sibyte/common/sb_tbprof.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/sibyte/common/sb_tbprof.c
b/arch/mips/sibyte/common/sb_tbprof.c
index f80d7a7..eac125f
--- a/arch/mips/sibyte/common/sb_tbprof.c
+++ b/arch/mips/sibyte/common/sb_tbprof.c
@@ -465,7 +465,7 @@ static ssize_t sbprof_tb_read(struct file *filp, char *buf,
if (err) {
*offp = cur_off + cur_count - err;
mutex_unlock();
- return err;
+ return -EFAULT;
}
pr_debug(DEVNAME ": read from sample %d, %d bytes\n",
cur_sample, cur_count);
MBR, Sergei
Re: [PATCH 19/20] usbip: usbip_host: Manual replacement of the deprecated strlcpy() with return values
Hello! On 22.02.2021 18:12, Romain Perier wrote: The strlcpy() reads the entire source buffer first, it is dangerous if the source buffer lenght is unbounded or possibility non NULL-terminated. Length. Possibly? It can lead to linear read overflows, crashes, etc... As recommended in the deprecated interfaces [1], it should be replaced by strscpy. This commit replaces all calls to strlcpy that handle the return values by the corresponding strscpy calls with new handling of the return values (as it is quite different between the two functions). [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy Signed-off-by: Romain Perier [...] MBR, Sergei
Re: [PATCH 1/2] list: Add list_is_null() to check if a list_head has been initialized
Hello!
On 27.02.2021 1:49, Laurent Pinchart wrote:
From: Laurent Pinchart
The new function checks if the list_head prev and next pointers are
NULL, in order to see if a list_head that has been zeroed when allocated
has been initialized with INIT_LIST_HEAD() or added to a list.
So zeroed or initialized/added? :-)
This can be used in cleanup functions that want to support being safely
called when an object has not been initialized, to return immediately.
In most cases other fields of the object can be checked for this
purpose, but in some cases a list_head field is the only option.
Signed-off-by: Laurent Pinchart
---
include/linux/list.h | 13 +
1 file changed, 13 insertions(+)
diff --git a/include/linux/list.h b/include/linux/list.h
index 85c92555e31f..e4fc6954de3b 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -29,6 +29,19 @@ static inline void INIT_LIST_HEAD(struct list_head *list)
list->prev = list;
}
+/**
+ * list_is_null - check if a list_head has been initialized
+ * @list: the list
+ *
+ * Check if the list_head prev and next pointers are NULL. This is useful to
+ * see if a list_head that has been zeroed when allocated has been initialized
+ * with INIT_LIST_HEAD() or added to a list.
So zeroed or initialized/added? :-)
+ */
+static inline bool list_is_null(struct list_head *list)
+{
+ return list->prev == NULL && list->next == NULL;
Maybe instead:
return !list->prev && !list->next;
[...]
MBR, Sergei
Re: [PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB
Hello! On 26.02.2021 1:51, Fabrizio Castro wrote: Document bindings for R-Car DAB hardware accelerator, currently found on the r8a77990 SoC (a.k.a. R-Car E3) and on the r8a77965 SoC (a.k.a. R-Car M3-N). Signed-off-by: Fabrizio Castro --- .../devicetree/bindings/misc/renesas,dab.yaml | 75 +++ 1 file changed, 75 insertions(+) create mode 100644 Documentation/devicetree/bindings/misc/renesas,dab.yaml diff --git a/Documentation/devicetree/bindings/misc/renesas,dab.yaml b/Documentation/devicetree/bindings/misc/renesas,dab.yaml new file mode 100644 index ..e9494add13d7 --- /dev/null +++ b/Documentation/devicetree/bindings/misc/renesas,dab.yaml @@ -0,0 +1,75 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright (C) 2021 Renesas Electronics Corporation +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/misc/renesas,dab.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Renesas R-Car DAB Hardware Accelerator + +maintainers: + - Fabrizio Castro + +description: + The DAB hardware accelerator found on some R-Car devices is a hardware + accelerator for software DAB demodulators. + It consists of one FFT (Fast Fourier Transform) module and one decoder module, + compatible with DAB specification (ETSI EN 300 401 and ETSI TS 102 563). + The decoder module can perform FIC decoding and MSC decoding processing from + de-puncture to final decoded result. + +properties: + compatible: +items: + - enum: + - renesas,dab-r8a77965 # R-Car M3-N + - renesas,dab-r8a77990 # R-Car E3 Why not renesas,-dab? + - const: renesas,rcar-gen3-dab # Generic fallback for R-Car Gen3 devices [...] MBR, Sergei
Re: usb: dwc3: gadget: Change runtime pm function for DWC3 runtime suspend
Hello!
On 15.02.2021 5:38, Daehwan Jung wrote:
It seems pm_runtime_put calls runtime_idle callback not runtime_suspend
callback.
It's better to use pm_runtime_put_sync_suspend to allow DWC3 runtime suspend.
Signed-off-by: Daehwan Jung
---
drivers/usb/dwc3/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index aebcf8e..4a4b93b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2229,7 +2229,7 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int
is_on)
*/
ret = pm_runtime_get_sync(dwc->dev);
if (!ret || ret < 0) {
BTW, this can be shortened to (ret <= 0)...
- pm_runtime_put(dwc->dev);
+ pm_runtime_put_sync_suspend(dwc->dev);
return 0;
}
MBR, Sergei
Re: [PATCH] usb: host: ehci: remove casting dma_alloc_coherent
Hello! On 04.02.2021 10:17, Xu Wang wrote: Remove casting the values returned by dma_alloc_coherent. I'm not seeing any casting, I'm seeing a trailing space removed. Signed-off-by: Xu Wang --- drivers/usb/host/ehci-mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/ehci-mem.c b/drivers/usb/host/ehci-mem.c index 6361d81272bc..769329032257 100644 --- a/drivers/usb/host/ehci-mem.c +++ b/drivers/usb/host/ehci-mem.c @@ -185,7 +185,7 @@ static int ehci_mem_init (struct ehci_hcd *ehci, gfp_t flags) } /* Hardware periodic table */ - ehci->periodic = + ehci->periodic = dma_alloc_coherent(ehci_to_hcd(ehci)->self.sysdev, ehci->periodic_size * sizeof(__le32), >periodic_dma, flags); MBR, Sergei
Re: [PATCH 02/27] x86/syscalls: fix -Wmissing-prototypes warnings from COND_SYSCALL()
Hello! On 28.01.2021 3:50, Masahiro Yamada wrote: Building kernel/sys_ni.c with W=1 omits tons of -Wmissing-prototypes Emits? warnings. $ make W=1 kernel/sys_ni.o [ snip ] CC kernel/sys_ni.o In file included from kernel/sys_ni.c:10: ./arch/x86/include/asm/syscall_wrapper.h:83:14: warning: no previous prototype for '__x64_sys_io_setup' [-Wmissing-prototypes] 83 | __weak long __##abi##_##name(const struct pt_regs *__unused) \ | ^~ ./arch/x86/include/asm/syscall_wrapper.h:100:2: note: in expansion of macro '__COND_SYSCALL' 100 | __COND_SYSCALL(x64, sys_##name) | ^~ ./arch/x86/include/asm/syscall_wrapper.h:256:2: note: in expansion of macro '__X64_COND_SYSCALL' 256 | __X64_COND_SYSCALL(name) \ | ^~ kernel/sys_ni.c:39:1: note: in expansion of macro 'COND_SYSCALL' 39 | COND_SYSCALL(io_setup); | ^~~~ ./arch/x86/include/asm/syscall_wrapper.h:83:14: warning: no previous prototype for '__ia32_sys_io_setup' [-Wmissing-prototypes] 83 | __weak long __##abi##_##name(const struct pt_regs *__unused) \ | ^~ ./arch/x86/include/asm/syscall_wrapper.h:120:2: note: in expansion of macro '__COND_SYSCALL' 120 | __COND_SYSCALL(ia32, sys_##name) | ^~ ./arch/x86/include/asm/syscall_wrapper.h:257:2: note: in expansion of macro '__IA32_COND_SYSCALL' 257 | __IA32_COND_SYSCALL(name) | ^~~ kernel/sys_ni.c:39:1: note: in expansion of macro 'COND_SYSCALL' 39 | COND_SYSCALL(io_setup); | ^~~~ ... __SYS_STUB0() and __SYS_STUBx() defined a few lines above have forward declarations. Let's do likewise for __COND_SYSCALL() to fix the warnings. Signed-off-by: Masahiro Yamada --- arch/x86/include/asm/syscall_wrapper.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h index a84333adeef2..80c08c7d5e72 100644 --- a/arch/x86/include/asm/syscall_wrapper.h +++ b/arch/x86/include/asm/syscall_wrapper.h @@ -80,6 +80,7 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs); } #define __COND_SYSCALL(abi, name) \ + __weak long __##abi##_##name(const struct pt_regs *__unused); \ __weak long __##abi##_##name(const struct pt_regs *__unused)\ Aren't these two lines identical? [...] MBR, Sergei
Re: [PATCH 02/27] x86/syscalls: fix -Wmissing-prototypes warnings from COND_SYSCALL()
On 28.01.2021 10:59, Sergei Shtylyov wrote: [...] Building kernel/sys_ni.c with W=1 omits tons of -Wmissing-prototypes Emits? warnings. $ make W=1 kernel/sys_ni.o [ snip ] CC kernel/sys_ni.o In file included from kernel/sys_ni.c:10: ./arch/x86/include/asm/syscall_wrapper.h:83:14: warning: no previous prototype for '__x64_sys_io_setup' [-Wmissing-prototypes] 83 | __weak long __##abi##_##name(const struct pt_regs *__unused) \ | ^~ ./arch/x86/include/asm/syscall_wrapper.h:100:2: note: in expansion of macro '__COND_SYSCALL' 100 | __COND_SYSCALL(x64, sys_##name) | ^~ ./arch/x86/include/asm/syscall_wrapper.h:256:2: note: in expansion of macro '__X64_COND_SYSCALL' 256 | __X64_COND_SYSCALL(name) \ | ^~ kernel/sys_ni.c:39:1: note: in expansion of macro 'COND_SYSCALL' 39 | COND_SYSCALL(io_setup); | ^~~~ ./arch/x86/include/asm/syscall_wrapper.h:83:14: warning: no previous prototype for '__ia32_sys_io_setup' [-Wmissing-prototypes] 83 | __weak long __##abi##_##name(const struct pt_regs *__unused) \ | ^~ ./arch/x86/include/asm/syscall_wrapper.h:120:2: note: in expansion of macro '__COND_SYSCALL' 120 | __COND_SYSCALL(ia32, sys_##name) | ^~ ./arch/x86/include/asm/syscall_wrapper.h:257:2: note: in expansion of macro '__IA32_COND_SYSCALL' 257 | __IA32_COND_SYSCALL(name) | ^~~ kernel/sys_ni.c:39:1: note: in expansion of macro 'COND_SYSCALL' 39 | COND_SYSCALL(io_setup); | ^~~~ ... __SYS_STUB0() and __SYS_STUBx() defined a few lines above have forward declarations. Let's do likewise for __COND_SYSCALL() to fix the warnings. Signed-off-by: Masahiro Yamada --- arch/x86/include/asm/syscall_wrapper.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h index a84333adeef2..80c08c7d5e72 100644 --- a/arch/x86/include/asm/syscall_wrapper.h +++ b/arch/x86/include/asm/syscall_wrapper.h @@ -80,6 +80,7 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs); } #define __COND_SYSCALL(abi, name) \ + __weak long __##abi##_##name(const struct pt_regs *__unused); \ __weak long __##abi##_##name(const struct pt_regs *__unused) \ Aren't these two lines identical? Ah, got it! :-) [...] MBR, Sergei
Re: [RE-RESEND PATCH 1/4] usb: musb: Fix runtime PM race in musb_queue_resume_work
On 1/23/21 5:24 PM, Paul Cercueil wrote:
> musb_queue_resume_work() would call the provided callback if the runtime
> PM status was 'active'. Otherwise, it would enqueue the request if the
> hardware was still suspended (musb->is_runtime_suspended is true).
>
> This causes a race with the runtime PM handlers, as it is possible to be
> in the case where the runtime PM status is not yet 'active', but the
> hardware has been awaken (PM resume function has been called).
Awakened. :-)
> When hitting the race, the resume work was not enqueued, which probably
> triggered other bugs further down the stack. For instance, a telnet
> connection on Ingenic SoCs would result in a 50/50 chance of a
> segmentation fault somewhere in the musb code.
>
> Rework the code so that either we call the callback directly if
> (musb->is_runtime_suspended == 0), or enqueue the query otherwise.
>
> Fixes: ea2f35c01d5e ("usb: musb: Fix sleeping function called from invalid
> context for hdrc glue")
> Cc: sta...@vger.kernel.org # v4.9+
> Signed-off-by: Paul Cercueil
> Reviewed-by: Tony Lindgren
> Tested-by: Tony Lindgren
[...]
MBR, Sergei
Re: [PATCH] sh_eth: Fix power down vs. is_opened flag ordering
On 1/18/21 6:08 PM, Geert Uytterhoeven wrote:
> sh_eth_close() does a synchronous power down of the device before
> marking it closed. Revert the order, to make sure the device is never
> marked opened while suspended.
>
> While at it, use pm_runtime_put() instead of pm_runtime_put_sync(), as
> there is no reason to do a synchronous power down.
>
> Fixes: 7fa2955ff70ce453 ("sh_eth: Fix sleeping function called from invalid
> context")
> Signed-off-by: Geert Uytterhoeven
Reviewed-by: Sergei Shtylyov
[...]
MBR, Sergei
Re: [PATCH V2 4/4] net: ethernet: ravb: Enable optional refclk
Hello!
On 15.01.2021 23:19, Adam Ford wrote:
For devices that use a programmable clock for the avb reference clock,
AVB.
the driver may need to enable them. Add code to find the optional clock
and enable it when available.
Signed-off-by: Adam Ford
[...]
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index bd30505fbc57..739e30f45daa 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2148,6 +2148,14 @@ static int ravb_probe(struct platform_device *pdev)
goto out_release;
}
+ priv->refclk = devm_clk_get_optional(>dev, "refclk");
+ if (IS_ERR(priv->refclk)) {
+ error = PTR_ERR(priv->refclk);
+ goto out_release;
+ } else {
No need for *else* after *goto*.
+ (void)clk_prepare_enable(priv->refclk);
You can really omit (void)...
Also, I'm not seeing where do you call clk_disable_unprepare()...
[...]
MBR, Sergei
Re: [PATCH next 12/15] arm64: dts: mediatek: mt8183: fix dtbs_check warning
On 16.01.2021 12:06, Chunfeng Yun wrote:
Harmonize node names, compatibles and properties.
Signed-off-by: Chunfeng Yun
---
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5b782a4769e7..a69a033a68ac 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
[...]
@@ -908,11 +908,11 @@
status = "disabled";
};
- mipi_tx0: mipi-dphy@11e5 {
+ mipi_tx0: dsi-phy@11e5 {
Again, why not just "phy@..."?
[...]
@@ -931,11 +931,10 @@
};
};
- u3phy: usb-phy@11f4 {
+ u3phy: t-phy@11f4 {
Here as well...
[...]
MBR, Sergei
Re: [PATCH next 11/15] arm64: dts: mediatek: mt7622: harmonize node names and compatibles
Hello!
On 16.01.2021 12:06, Chunfeng Yun wrote:
This is used to fix dtbs_check warning
Signed-off-by: Chunfeng Yun
---
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
index 5b9ec032ce8d..4cfa09b0ca3c 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
@@ -740,8 +740,8 @@
status = "disabled";
};
- u3phy: usb-phy@1a0c4000 {
- compatible = "mediatek,mt7622-u3phy",
+ u3phy: t-phy@1a0c4000 {
What is "t-phy"? Perhaps you should have used just "phy" here...
+ compatible = "mediatek,mt7622-tphy",
"mediatek,generic-tphy-v1";
reg = <0 0x1a0c4000 0 0x700>;
#address-cells = <2>;
@@ -875,8 +875,9 @@
status = "disabled";
};
- sata_phy: sata-phy@1a243000 {
- compatible = "mediatek,generic-tphy-v1";
+ sata_phy: t-phy@1a243000 {
Same here...
+ compatible = "mediatek,mt7622-tphy",
+"mediatek,generic-tphy-v1";
#address-cells = <2>;
#size-cells = <2>;
ranges;
MBR, Sergei
Re: [PATCH] ata: remove redundant error print in rb532_pata_driver_probe
On 1/12/21 2:15 PM, Sergei Shtylyov wrote:
[...]
>> From: Menglong Dong
>>
>> Coccinelle reports a redundant error print in rb532_pata_driver_probe.
>> As 'platform_get_irq' already prints the error message, error print
>> here is redundant and can be removed.
>>
>> Signed-off-by: Menglong Dong
>> ---
>> drivers/ata/pata_rb532_cf.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
>> index 479c4b29b856..dcde84f571c4 100644
>> --- a/drivers/ata/pata_rb532_cf.c
>> +++ b/drivers/ata/pata_rb532_cf.c
>> @@ -115,10 +115,8 @@ static int rb532_pata_driver_probe(struct
>> platform_device *pdev)
>> }
>>
>> irq = platform_get_irq(pdev, 0);
>> -if (irq <= 0) {
>> -dev_err(>dev, "no IRQ resource found\n");
>> +if (irq <= 0)
>> return -ENOENT;
>
>This still beaks the probe deferral. :-(
>But that's another problem...
BTW, your patch summary looks quite wrong -- we need the prefix to specify
the patch locus,
like this:
pata_rb532_cf: remove redundant error printing in the probe() method
I.e. "ata: " may even be omitted (your choise though).
> [...]
MBR, Sergei
Re: [PATCH 2/2] compiler.h: Include asm/rwonce.h under ARM64 and ALPHA to fix build errors
Hello! On 13.01.2021 13:57, Tiezhu Yang wrote: When make M=samples/bpf on the Loongson 3A3000 platform which belongs to MIPS arch, there exists many similar build errors about 'asm/rwonce.h' file not found, so include it only under CONFIG_ARM64 and CONFIG_ALPHA due to it exists only in arm64 and alpha arch. CLANG-bpf samples/bpf/xdpsock_kern.o In file included from samples/bpf/xdpsock_kern.c:2: In file included from ./include/linux/bpf.h:9: In file included from ./include/linux/workqueue.h:9: In file included from ./include/linux/timer.h:5: In file included from ./include/linux/list.h:9: In file included from ./include/linux/kernel.h:10: ./include/linux/compiler.h:246:10: fatal error: 'asm/rwonce.h' file not found ^~ 1 error generated. $ find . -name rwonce.h ./include/asm-generic/rwonce.h ./arch/arm64/include/asm/rwonce.h ./arch/alpha/include/asm/rwonce.h Signed-off-by: Tiezhu Yang --- include/linux/compiler.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/linux/compiler.h b/include/linux/compiler.h index b8fe0c2..bdbe759 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -243,6 +243,12 @@ static inline void *offset_to_ptr(const int *off) */ #define prevent_tail_call_optimization() mb() +#ifdef CONFIG_ARM64 Why not #if defined(CONFIG_ALPHA) || defined(CONFIG_ARM64)? #include +#endif + +#ifdef CONFIG_ALPHA +#include +#endif #endif /* __LINUX_COMPILER_H */ MBR, Sergei
Re: [PATCH] ata: remove redundant error print in rb532_pata_driver_probe
Hello!
On 13.01.2021 17:04, Menglong Dong wrote:
[...]
irq = platform_get_irq(pdev, 0);
- if (irq <= 0) {
- dev_err(>dev, "no IRQ resource found\n");
+ if (irq <= 0)
return -ENOENT;
This still beaks the probe deferral. :-(
But that's another problem...
[...]
MBR, Sergei
What does this 'MBR' mean? I am a novice~~~
Generally speaking, Master Boot Record. But I also use it to send you My
Best Regards. :-)
So, is it better to replace 'platform_get_irq' with
'platform_get_irq_optional' here?
No. You should stop overriding the result to -ENOENT and pass the result
up the call chain instead. In order to do it, you should only check for (irq <
0).
--
Best Regards
Menglong Dong
MBR, Sergei
Re: [PATCH v7 4/7] dt-bindings: media: max9286: Document 'maxim,reverse-channel-microvolt'
Hello! On 13.01.2021 21:55, Jacopo Mondi wrote: Document the 'reverse-channel-microvolt' vendor property in the Where is "maxim,"? bindings document of the max9286 driver. The newly introduced property allows to specifying the initial Specify? configuration of the GMSL reverse control channel to accommodate remote serializers pre-programmed with the high threshold power supply noise immunity enabled. Reviewed-by: Rob Herring Reviewed-by: Laurent Pinchart Reviewed-by: Kieran Bingham Signed-off-by: Jacopo Mondi [...] MBR, Sergei
Re: [PATCH] ata: remove redundant error print in rb532_pata_driver_probe
Hello!
On 1/12/21 5:36 AM, menglong8.d...@gmail.com wrote:
> From: Menglong Dong
>
> Coccinelle reports a redundant error print in rb532_pata_driver_probe.
> As 'platform_get_irq' already prints the error message, error print
> here is redundant and can be removed.
>
> Signed-off-by: Menglong Dong
> ---
> drivers/ata/pata_rb532_cf.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
> index 479c4b29b856..dcde84f571c4 100644
> --- a/drivers/ata/pata_rb532_cf.c
> +++ b/drivers/ata/pata_rb532_cf.c
> @@ -115,10 +115,8 @@ static int rb532_pata_driver_probe(struct
> platform_device *pdev)
> }
>
> irq = platform_get_irq(pdev, 0);
> - if (irq <= 0) {
> - dev_err(>dev, "no IRQ resource found\n");
> + if (irq <= 0)
> return -ENOENT;
This still beaks the probe deferral. :-(
But that's another problem...
[...]
MBR, Sergei
Re: [PATCH] usb/c67x00: Replace tasklet with work
Hello!
On 11.01.2021 7:40, Davidlohr Bueso wrote:
Tasklets have long been deprecated as being too heavy on the system
by running in irq context - and this is not a performance critical
path. If a higher priority process wants to run, it must wait for
the tasklet to finish before doing so.
c67x00_do_work() will now run in process context and have further
concurrency (tasklets being serialized among themselves), but this
is done holding the c67x00->lock, so it should be fine. Furthermore,
this patch fixes the usage of the lock in the callback as otherwise
it would need to be irq-safe.
Signed-off-by: Davidlohr Bueso
[...]
diff --git a/drivers/usb/c67x00/c67x00-sched.c
b/drivers/usb/c67x00/c67x00-sched.c
index e65f1a0ae80b..af60f4fdd340 100644
--- a/drivers/usb/c67x00/c67x00-sched.c
+++ b/drivers/usb/c67x00/c67x00-sched.c
@@ -1123,24 +1123,26 @@ static void c67x00_do_work(struct c67x00_hcd *c67x00)
/* -- */
-static void c67x00_sched_tasklet(struct tasklet_struct *t)
+static void c67x00_sched_work(struct work_struct *work)
{
- struct c67x00_hcd *c67x00 = from_tasklet(c67x00, t, tasklet);
+ struct c67x00_hcd *c67x00;
+
+ c67x00 = container_of(work, struct c67x00_hcd, work);
c67x00_do_work(c67x00);
}
void c67x00_sched_kick(struct c67x00_hcd *c67x00)
{
- tasklet_hi_schedule(>tasklet);
+queue_work(system_highpri_wq, >work);
Use a tab to indent, please.
}
int c67x00_sched_start_scheduler(struct c67x00_hcd *c67x00)
{
- tasklet_setup(>tasklet, c67x00_sched_tasklet);
+INIT_WORK(>work, c67x00_sched_work);
Here as well.
return 0;
}
void c67x00_sched_stop_scheduler(struct c67x00_hcd *c67x00)
{
- tasklet_kill(>tasklet);
+cancel_work_sync(>work);
And here.
[...]
MBR, Sergei
Re: [PATCH 1/2] spi: rpc-if: Avoid use of C++ style comments
On 12/30/20 5:57 PM, Lad Prabhakar wrote: > Replace C++ style comment with C style. Note that the switch to // was made following the SPI maintainer's request... > Signed-off-by: Lad Prabhakar [...] MBR, Sergei
Re: [PATCH 2/4] ARM: dts: renesas: Add fck to etheravb-rcar-gen2 clock-names list
On 29.12.2020 0:31, Adam Ford wrote: The bindings have been updated to support two clocks, but the original clock now requires the name fck. Add a clock-names list in the device tree with fck in it. Hopefully this won't break RPM... Signed-off-by: Adam Ford [...] MBR, Sergei
Re: [PATCH 4/4] net: ethernet: ravb: Name the AVB functional clock fck
On 29.12.2020 0:31, Adam Ford wrote: The bindings have been updated to support two clocks, but the original clock now requires the name fck to distinguish it from the other. Signed-off-by: Adam Ford Reviewed-by: Sergei Shtylyov MBR, Sergei
Re: [PATCH 1/4] dt-bindings: net: renesas,etheravb: Add additional clocks
Hello! On 29.12.2020 0:31, Adam Ford wrote: The AVB driver assumes there is an external clock, but it could be driven by an external clock. Driver can be driven by external clock? :-) In order to enable a programmable clock, it needs to be added to the clocks list and enabled in the driver. Since there currently only one clock, there is no ^ is clock-names list either. Update bindings to add the additional optional clock, and explicitly name both of them. Signed-off-by: Adam Ford [...] MBR, Sergei
Re: [PATCH v4 2/4] spi: ls7a: Add YAML schemas
On 12/25/20 1:35 PM, Qing Zhang wrote:
> Switch the DT binding to a YAML schema to enable the DT validation.
>
> Signed-off-by: Qing Zhang
> ---
>
> v4: fix warnings/errors about running 'make dt_binding_check'
>
> ---
> .../devicetree/bindings/spi/loongson,spi-ls7a.yaml | 46
> ++
> 1 file changed, 46 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
>
> diff --git a/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
> b/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
> new file mode 100644
> index 000..8cc9bc5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/loongson,spi-ls7a.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson LS7A PCH SPI Controller
> +
> +maintainers:
> + - Qing Zhang
> +
> +description: |
> + This controller can be found in Loongson-3 systems with LS7A PCH.
> +
> +properties:
> + compatible:
> +const: loongson,ls7a-spi
> +
> + reg:
> +maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - num-chipselects
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +pci {
> +#address-cells = <3>;
> +#size-cells = <2>;
> +
> +spi@16,0 {
> +compatible = "pci0014,7a0b.0",
> +"pci0014,7a0b",
> +"pciclass088000",
> +"pciclass0800";
> +
> +reg = <0xb000 0x0 0x0 0x0 0x0>;
> +num-chipselects = <0>;
The above lines after { need extra indentation level.
> +};
> +};
> +
> +...
MBR, Sergei
Re: [PATCH V2 2/9] arm64: dts: renesas: beacon kit: Fix Audio Clock sources
On 24.12.2020 20:04, Adam Ford wrote: The SoC was expecting two clock sources with different frequencies. One to support 44.1KHz and one to support 48KHz. With the newly added ability to configure the programmably clock, configure both clocks. Programmable. Assign the rcar-sound clocks to reference the versaclock instead of the fixed clock. Signed-off-by: Adam Ford [...] MBR, Sergei
Re: [PATCH V2 5/9] arm64: dts: renesas: beacon: Cleanup USB References
On 24.12.2020 20:04, Adam Ford wrote:
The programmable versaclock is used for the usb_extal reference clock for
the EHCI driver instead of a fixed-clock. Because the versaclock needs to
be enabled, the clock reference needs to be added to the clocks list.
For the USB3 Phy, the perferred clock reference is usb3s_clk, so
PHY. Preferred.
remove the usb_extal reference.
Signed-off-by: Adam Ford
[...]
diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
index ade2f58ad99b..a6f7193e4d97 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
[...]
@@ -315,16 +306,12 @@ {
vmmc-supply = <_3p3v>;
vqmmc-supply = <_1p8v>;
bus-width = <8>;
- mmc-hs200-1_8v;
+ mmc-hs400-1_8v;
Unrelated?
non-removable;
fixed-emmc-driver-type = <1>;
status = "okay";
};
[...]
MBR, Sergei
Re: [PATCH V2 8/9] arm64: dts: renesas: Introduce r8a774e1-beacon-rzg2h-kit
On 24.12.2020 20:05, Adam Ford wrote: eacon EmebeddedWorks is introducing a new kit based on the Beacon? RZ/G2H SoC from Renesas. The SOM supports eMMC, WiFi and Bluetooth, along with a Cat-M1 cellular radio. The Baseboard has Ethernet, USB, HDMI, stereo audio in and out, along with a variety of push buttons and LED's, and support for a parallel RGB and an LVDS display. It uses the same baseboard and SOM files as the RZ/G2M and RZ/G2N kits. Signed-off-by: Adam Ford [...] MBR, Sergei
Re: [PATCH v7 10/12] mfd: bd9571mwv: Use devm_regmap_add_irq_chip()
Hello!
On 24.12.2020 10:04, Yoshihiro Shimoda wrote:
Use dev_regmap_add_irq_chip() to simplify the code.
devm_?
Signed-off-by: Yoshihiro Shimoda
Acked-for-MFD-by: Lee Jones
Reviewed-by: Matti Vaittinen
Reviewed-by: Geert Uytterhoeven
---
drivers/mfd/bd9571mwv.c | 27 ++-
1 file changed, 6 insertions(+), 21 deletions(-)
diff --git a/drivers/mfd/bd9571mwv.c b/drivers/mfd/bd9571mwv.c
index e68c3fa..49e968e 100644
--- a/drivers/mfd/bd9571mwv.c
+++ b/drivers/mfd/bd9571mwv.c
@@ -170,31 +170,17 @@ static int bd9571mwv_probe(struct i2c_client *client,
if (ret)
return ret;
- ret = regmap_add_irq_chip(bd->regmap, bd->irq, IRQF_ONESHOT, 0,
- _irq_chip, >irq_data);
+ ret = devm_regmap_add_irq_chip(bd->dev, bd->regmap, bd->irq,
+ IRQF_ONESHOT, 0, _irq_chip,
+ >irq_data);
if (ret) {
dev_err(bd->dev, "Failed to register IRQ chip\n");
return ret;
}
[...]
MBR, Sergei
Re: [PATCH v5 05/12] regulator: bd9571mwv: Add BD9574MWF support
On 22.12.2020 14:22, Yoshihiro Shimoda wrote:
Add support for BD9574MWF which is silimar chip with BD9571MWV.
Similar (again)? :-)
Note that we don't support voltage rails VD{09,18,25,33} by this
driver on BD9574. The VD09 voltage could be read from PMIC but that
is not supported by this commit.
Signed-off-by: Yoshihiro Shimoda
Reviewed-by: Matti Vaittinen
[...]
MBR, Sergei
Re: [PATCH v5 08/12] gpio: bd9571mwv: Add BD9574MWF support
Hello! On 22.12.2020 14:22, Yoshihiro Shimoda wrote: Add support for BD9574MWF which is silimar chip with BD9571MWV. Similar? :-) Note that BD9574MWF has additional features "RECOV_GPOUT", "FREQSEL" and "RTC_IN", but supports GPIO function only. Signed-off-by: Yoshihiro Shimoda Reviewed-by: Matti Vaittinen [...] MBR, Sergei
Re: [PATCH v1 1/8] usb: phy: tegra: Add delay after power up
Hello! On 15.12.2020 23:21, Dmitry Osipenko wrote: The PHY hardware needs the delay of 2ms after power up, otherwise initial interrupt may be lost if USB controller is accessed before PHY is settled down. Previously this issue was masked by implicit delays, but now it pops up after squashing the older ehci-tegra driver into the ChipIdea driver. Tested-by: Matt Merhar Tested-by: Nicolas Chauvet Tested-by: Peter Geis Signed-off-by: Dmitry Osipenko --- drivers/usb/phy/phy-tegra-usb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c index 03a333797382..cee9c9dbb775 100644 --- a/drivers/usb/phy/phy-tegra-usb.c +++ b/drivers/usb/phy/phy-tegra-usb.c @@ -784,6 +784,9 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy) phy->powered_on = true; + /* let PHY to settle down */ Let the PHY settle down. + usleep_range(2000, 2500); + return 0; } MBR, Sergei
Re: [RFC] ravb: Add support for optional txc_refclk
Hello!
On 12.12.2020 19:56, Adam Ford wrote:
The SoC expects the txv_refclk is provided, but if it is provided
by a programmable clock, there needs to be a way to get and enable
this clock to operate. It needs to be optional since it's only
necessary for those with programmable clocks.
Signed-off-by: Adam Ford
[...]
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index bd30505fbc57..4c3f95923ef2 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2148,6 +2148,18 @@ static int ravb_probe(struct platform_device *pdev)
goto out_release;
}
+ priv->ref_clk = devm_clk_get(>dev, "txc_refclk");
Why not devm_clk_get_optional()?
+ if (IS_ERR(priv->ref_clk)) {
+ if (PTR_ERR(priv->ref_clk) == -EPROBE_DEFER) {
+ /* for Probe defer return error */
+ error = PTR_ERR(priv->ref_clk);
+ goto out_release;
+ }
+ /* Ignore other errors since it's optional */
+ } else {
+ (void)clk_prepare_enable(priv->ref_clk);
+ }
+
ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
ndev->min_mtu = ETH_MIN_MTU;
MBR, Sergei
Re: [PATCH v3] usb: musb: remove unused variable 'devctl'
On 12/10/20 6:01 PM, Greg Kroah-Hartman wrote: [...] >>> From: Min Guo >>> >>> Remove unused 'devctl' variable to fix compile warnings: >>> >>> drivers/usb/musb/musbhsdma.c: In function 'dma_controller_irq': >>> drivers/usb/musb/musbhsdma.c:324:8: warning: variable 'devctl' set >>> but not used [-Wunused-but-set-variable] >>> >>> Signed-off-by: Min Guo >>> --- >>> changes in v3 >>> suggested by Greg Kroah-Hartman: >>> Add a comment. >>> >>> changes in v2 >>> suggested by Alan Stern: >>> Add void before musb_read to indicate that the register MUSB_DEVCTL >>> was intended to be read and discarded. >>> --- >>> drivers/usb/musb/musbhsdma.c | 8 +--- >>> 1 file changed, 5 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c >>> index 0aacfc8be5a1..2a345b4ad015 100644 >>> --- a/drivers/usb/musb/musbhsdma.c >>> +++ b/drivers/usb/musb/musbhsdma.c >> [...] >>> @@ -336,7 +334,11 @@ irqreturn_t dma_controller_irq(int irq, void >>> *private_data) >>> < musb_channel->len) ? >>> "=> reconfig 0" : "=> complete"); >>> - devctl = musb_readb(mbase, MUSB_DEVCTL); >>> + /* >>> +* Some hardware may need to read the >>> +* MUSB_DEVCTL register once to take effect. >>> +*/ >>> + (void)musb_readb(mbase, MUSB_DEVCTL); >> >>Hm, forcibly reading DevCtl in the DMA driver... sounds quite >> nonsensically. Lemme take a look... > > What happened to your look? I thought I posted it the same day... Indeed, here it is, archived: https://marc.info/?l=linux-usb=160621841910477 > thanks, > > greg k-h MBR, Sergei
Re: [PATCH v2 2/4] spi: Add devicetree bindings documentation for Loongson SPI
On 12/8/20 1:47 PM, zhangqing wrote:
>>> Add spi-ls7a binding documentation.
>>>
>>> Signed-off-by: Qing Zhang
>>> ---
>>> Documentation/devicetree/bindings/spi/spi-ls7a.txt | 31
>>> ++
>>> 1 file changed, 31 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-ls7a.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-ls7a.txt
>>> b/Documentation/devicetree/bindings/spi/spi-ls7a.txt
>>> new file mode 100644
>>> index 000..56247b5
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/spi/spi-ls7a.txt
>>> @@ -0,0 +1,31 @@
>>> +Binding for LOONGSON LS7A SPI controller
>>> +
>>> +Required properties:
>>> +- compatible: should be
>>> "pci0014,7a0b.0","pci0014,7a0b","pciclass088000","pciclass0880".
>>> +- reg: reference IEEE Std 1275-1994.
>>> +- #address-cells: <1>, as required by generic SPI binding.
>>> +- #size-cells: <0>, also as required by generic SPI binding.
>>> +- #interrupts: No hardware interrupt.
>>
>> You say it's a required prop, yet yuoe example doesn't have it...
> I want to emphasize here that LS7A SPI has no hardware interrupts,
> and DT is not actually used.
The why document the property at all?
>>> +
>>> +Child nodes as per the generic SPI binding.
>>> +
>>> +Example:
>>> +
>>> + spi@16,0 {
>>> + compatible = "pci0014,7a0b.0",
>>> + "pci0014,7a0b",
>>> + "pciclass088000",
>>> + "pciclass0880";
>>> +
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + reg = <0xb000 0x0 0x0 0x0 0x0>;
>>> + num-chipselects = <0>;
>>> + spiflash: s25fl016k@0 {
>>> + #address-cells = <1>;
>>> + #size-cells = <1>;
>>
>> Once more?
>>
>>> + compatible ="spansion,s25fl016k","jedec,spi-nor";
>>
>> Once more?
>>
>>> + spi-max-frequency=<5000>;
>>> + reg=<0>;
>>
>> Once more? Did you mean this for a child node?
> Yes, these are child node attributes, the child node splash is not
> necessary.
You should indent the child nodes with 1 more tab...
>>
>>> + };
>>> + };
>>
> Thanks
>
> -Qing
MBR, Sergei
Re: [PATCH v2 2/4] spi: Add devicetree bindings documentation for Loongson SPI
Hello!
On 08.12.2020 10:44, Qing Zhang wrote:
Add spi-ls7a binding documentation.
Signed-off-by: Qing Zhang
---
Documentation/devicetree/bindings/spi/spi-ls7a.txt | 31 ++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-ls7a.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-ls7a.txt
b/Documentation/devicetree/bindings/spi/spi-ls7a.txt
new file mode 100644
index 000..56247b5
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-ls7a.txt
@@ -0,0 +1,31 @@
+Binding for LOONGSON LS7A SPI controller
+
+Required properties:
+- compatible: should be
"pci0014,7a0b.0","pci0014,7a0b","pciclass088000","pciclass0880".
+- reg: reference IEEE Std 1275-1994.
+- #address-cells: <1>, as required by generic SPI binding.
+- #size-cells: <0>, also as required by generic SPI binding.
+- #interrupts: No hardware interrupt.
You say it's a required prop, yet yuoe example doesn't have it...
+
+Child nodes as per the generic SPI binding.
+
+Example:
+
+ spi@16,0 {
+ compatible = "pci0014,7a0b.0",
+ "pci0014,7a0b",
+ "pciclass088000",
+ "pciclass0880";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0xb000 0x0 0x0 0x0 0x0>;
+ num-chipselects = <0>;
+ spiflash: s25fl016k@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
Once more?
+ compatible
="spansion,s25fl016k","jedec,spi-nor";
Once more?
+ spi-max-frequency=<5000>;
+ reg=<0>;
Once more? Did you mean this for a child node?
+ };
+ };
MBR, Sergei
Re: [PATCH v2 0/5] memory: renesas-rpc-if: Trivial fixes
On 11/26/20 10:11 PM, Lad Prabhakar wrote: > This patch series fixes trivial issues in RPC-IF driver. > > Changes for v2: > * Balanced PM in rpcif_disable_rpm > * Fixed typo in patch 4/5 > * Dropped C++ style fixes patch The part that fixed the comment style wasd good, you should\ve kept it... > * Included RB tags from Sergei > > Cheers, > Prabhakar [...] MBR, Sergei
Re: [PATCH v2 2/5] memory: renesas-rpc-if: Fix unbalanced pm_runtime_enable in rpcif_{enable,disable}_rpm
On 11/26/20 10:11 PM, Lad Prabhakar wrote:
> rpcif_enable_rpm calls pm_runtime_enable, so rpcif_disable_rpm needs to
> call pm_runtime_disable and not pm_runtime_put_sync.
>
> Fixes: ca7d8b980b67f ("memory: add Renesas RPC-IF driver")
> Reported-by: Reported-by: Geert Uytterhoeven
Reported by reported? :-)
> Signed-off-by: Lad Prabhakar
> Cc: sta...@vger.kernel.org
Reviewed-by: Sergei Shtylyov
[...]
MBR, Sergei
Re: [PATCH 5/5] memory: renesas-rpc-if: Fix a reference leak in rpcif_probe()
On 11/24/20 2:25 PM, Lad Prabhakar wrote:
> Release the node reference by calling of_node_put(flash) in the probe.
Sorry about missing this...
> Fixes: ca7d8b980b67f ("memory: add Renesas RPC-IF driver")
> Reported-by: Pavel Machek
> Signed-off-by: Lad Prabhakar
> Cc: sta...@vger.kernel.org
Reviewed-by: Sergei Shtylyov
[...]
MBR, Sergei
Re: [PATCH 3/5] memory: renesas-rpc-if: Export symbols as GPL
On 24.11.2020 14:25, Lad Prabhakar wrote: Renesas RPC-IF driver is licensed under GPL2.0, to be in sync export the symbols as GPL. Didn't know there's a connection... Suggested-by: Pavel Machek Signed-off-by: Lad Prabhakar Reviewed-by: Sergei Shtylyov [...] MBR, Sergei
Re: [PATCH 1/5] memory: renesas-rpc-if: Return correct value to the caller of rpcif_manual_xfer()
Hello!
On 24.11.2020 14:25, Lad Prabhakar wrote:
In the error path of rpcif_manual_xfer() the value of ret is overwritten
by value returned by reset_control_reset() function and thus returning
incorrect value to the caller.
This patch makes sure the correct value is returned to the caller of
rpcif_manual_xfer() by dropping the overwrite of ret in error path.
Also now we ignore the value returned by reset_control_reset() in the
error path and instead print a error message when it fails.
Fixes: ca7d8b980b67f ("memory: add Renesas RPC-IF driver")
Reported-by: Pavel Machek
Signed-off-by: Lad Prabhakar
Cc: sta...@vger.kernel.org
Reviewed-by: Sergei Shtylyov
MBR, Sergei
Re: [PATCH 4/5] memory: renesas-rpc-if: Avoid use of C++ style comments
On 11/24/20 2:25 PM, Lad Prabhakar wrote:
> Replace C++ style comment with C style.
Thanks, I've overlooked this, and the header files should use C style
comment,
not C++.
> While at it also replace the tab with a space between struct and
> struct name.
No connection between these 2 changes, so there should be 2 patches, not 1.
Also, I'd like to ask you that they're left intact (unless it causes problems
for you).
> Suggested-by: Pavel Machek
> Signed-off-by: Lad Prabhakar
[...]
> diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
> index b8c7cc63065f..30ea6bd969b4 100644
> --- a/include/memory/renesas-rpc-if.h
> +++ b/include/memory/renesas-rpc-if.h
> @@ -19,7 +19,7 @@ enum rpcif_data_dir {
> RPCIF_DATA_OUT,
> };
>
> -struct rpcif_op {
> +struct rpcif_op {
> struct {
> u8 buswidth;
> u8 opcode;
> @@ -57,7 +57,7 @@ struct rpcif_op {
> } data;
> };
>
> -struct rpcif {
> +struct rpcif {
> struct device *dev;
> void __iomem *dirmap;
> struct regmap *regmap;
> @@ -93,4 +93,4 @@ static inline void rpcif_disable_rpm(struct rpcif *rpc)
> pm_runtime_put_sync(rpc->dev);
> }
>
> -#endif // __RENESAS_RPC_IF_H
> +#endif /* __RENESAS_RPC_IF_H */
MBR, Sergei
Re: [PATCH 0/5] memory: renesas-rpc-if: Trivial fixes
On 11/24/20 2:34 PM, Lad, Prabhakar wrote: [...] >> This patch series fixes trivial issues in RPC-IF driver. >> >> Cheers, >> Prabhakar >> >> Lad Prabhakar (5): >> memory: renesas-rpc-if: Return correct value to the caller of >> rpcif_manual_xfer() >> memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static >> inline >> memory: renesas-rpc-if: Export symbols as GPL >> memory: renesas-rpc-if: Avoid use of C++ style comments >> memory: renesas-rpc-if: Fix a reference leak in rpcif_probe() >> > Patches sent to sergei.shtyl...@cogentembedded.com have bounced back > so including gmail address (patchwork [1]). Sorry, I got laid off by Cogent last May. Thanks for CCing my gmail address... > [1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=390163 > > Cheers, > Prabhakar [...] MBR, Sergei
Re: [PATCH 2/5] memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static inline
Hello!
On 11/24/20 2:25 PM, Lad Prabhakar wrote:
> Define rpcif_enable_rpm() and rpcif_disable_rpm() as static
Not sure why I didn't do it this way myself...
> inline in the header instead of exporting it.
s/it/them/.
> Suggested-by: Pavel Machek
> Signed-off-by: Lad Prabhakar
> ---
> drivers/memory/renesas-rpc-if.c | 13 -
> include/memory/renesas-rpc-if.h | 13 +++--
> 2 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
> index 69f2e2b4cd50..c5b5691503d7 100644
> --- a/drivers/memory/renesas-rpc-if.c
> +++ b/drivers/memory/renesas-rpc-if.c
[...]
> @@ -204,18 +203,6 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
> }
> EXPORT_SYMBOL(rpcif_sw_init);
>
> -void rpcif_enable_rpm(struct rpcif *rpc)
> -{
> - pm_runtime_enable(rpc->dev);
> -}
> -EXPORT_SYMBOL(rpcif_enable_rpm);
> -
> -void rpcif_disable_rpm(struct rpcif *rpc)
> -{
> - pm_runtime_put_sync(rpc->dev);
Ugh... sorry for this blunder (that went unnoticed till now). Mind fixing
it to pm_runtime_disable() (before this patch)?
> -}
> -EXPORT_SYMBOL(rpcif_disable_rpm);
> -
> void rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
> {
> u32 dummy;
[...]
MBR, Sergei
Re: [PATCH v3] usb: musb: remove unused variable 'devctl'
On 11/24/20 12:13 PM, Sergei Shtylyov wrote:
[...]
>> From: Min Guo
>>
>> Remove unused 'devctl' variable to fix compile warnings:
>>
>> drivers/usb/musb/musbhsdma.c: In function 'dma_controller_irq':
>> drivers/usb/musb/musbhsdma.c:324:8: warning: variable 'devctl' set
>> but not used [-Wunused-but-set-variable]
>>
>> Signed-off-by: Min Guo
>> ---
>> changes in v3
>> suggested by Greg Kroah-Hartman:
>> Add a comment.
>>
>> changes in v2
>> suggested by Alan Stern:
>> Add void before musb_read to indicate that the register MUSB_DEVCTL
>> was intended to be read and discarded.
>> ---
>> drivers/usb/musb/musbhsdma.c | 8 +---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
>> index 0aacfc8be5a1..2a345b4ad015 100644
>> --- a/drivers/usb/musb/musbhsdma.c
>> +++ b/drivers/usb/musb/musbhsdma.c
> [...]
>> @@ -336,7 +334,11 @@ irqreturn_t dma_controller_irq(int irq, void
>> *private_data)
>> < musb_channel->len) ?
>> "=> reconfig 0" : "=> complete");
>> - devctl = musb_readb(mbase, MUSB_DEVCTL);
>> + /*
>> + * Some hardware may need to read the
>> + * MUSB_DEVCTL register once to take effect.
>> + */
>> + (void)musb_readb(mbase, MUSB_DEVCTL);
>
> Hm, forcibly reading DevCtl in the DMA driver... sounds quite
> nonsensically. Lemme take a look...
Indeed, prior to commit c418fd6c01fbc5516a2cd1eaf1df1ec86869028a ("usb:
gadget: musb: fix short
isoc packets with inventra dma") the DevCtl read was done in order to check the
DevCtl.HostMode bit --
this test was removed but the DevCtl read was left intact... so my vote goes
for deleting both the
read and the variable...
> [...]
MBR, Sergei
Re: [PATCH v3] usb: musb: remove unused variable 'devctl'
Hello! On 24.11.2020 11:49, min@mediatek.com wrote: From: Min Guo Remove unused 'devctl' variable to fix compile warnings: drivers/usb/musb/musbhsdma.c: In function 'dma_controller_irq': drivers/usb/musb/musbhsdma.c:324:8: warning: variable 'devctl' set but not used [-Wunused-but-set-variable] Signed-off-by: Min Guo --- changes in v3 suggested by Greg Kroah-Hartman: Add a comment. changes in v2 suggested by Alan Stern: Add void before musb_read to indicate that the register MUSB_DEVCTL was intended to be read and discarded. --- drivers/usb/musb/musbhsdma.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c index 0aacfc8be5a1..2a345b4ad015 100644 --- a/drivers/usb/musb/musbhsdma.c +++ b/drivers/usb/musb/musbhsdma.c [...] @@ -336,7 +334,11 @@ irqreturn_t dma_controller_irq(int irq, void *private_data) < musb_channel->len) ? "=> reconfig 0" : "=> complete"); -devctl = musb_readb(mbase, MUSB_DEVCTL); + /* +* Some hardware may need to read the +* MUSB_DEVCTL register once to take effect. +*/ + (void)musb_readb(mbase, MUSB_DEVCTL); Hm, forcibly reading DevCtl in the DMA driver... sounds quite nonsensically. Lemme take a look... [...] MBR, Sergei
Re: [PATCH 1/2] media: max9271: Fix GPIO enable/disable
On 11/20/20 7:15 PM, Jacopo Mondi wrote:
> Fix GPIO enable/disable operations which wrongly read the 0x0f register
> to obtain the current mask of the enabled lines instead of using
> the correct 0x0e register.
>
> Also fix access to bit 0 of the register which is marked as reserved.
>
> Fixes: 34009bffc1c6 ("media: i2c: Add RDACM20 driver")
> Signed-off-by: Jacopo Mondi
> ---
> drivers/media/i2c/max9271.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/max9271.c b/drivers/media/i2c/max9271.c
> index 0f6f7a092a46..c247db569bab 100644
> --- a/drivers/media/i2c/max9271.c
> +++ b/drivers/media/i2c/max9271.c
> @@ -223,12 +223,12 @@ int max9271_enable_gpios(struct max9271_device *dev, u8
> gpio_mask)
> {
> int ret;
>
> - ret = max9271_read(dev, 0x0f);
> + ret = max9271_read(dev, 0x0e);
> if (ret < 0)
> return 0;
>
> /* BIT(0) reserved: GPO is always enabled. */
> - ret |= gpio_mask | BIT(0);
> + ret |= (gpio_mask & ~BIT(0));
() hardly needed here, = and = have very low prio...
[...]
MBR, Sergei
Re: [PATCH] usbnet: ipheth: fix connectivity with iOS 14
Hello! On 19.11.2020 20:24, Yves-Alexis Perez wrote: Starting with iOS 14 released in September 2020, connectivity using the personal hotspot USB tethering function of iOS devices is broken. Communication between the host and the device (for example ICMP traffic or DNS resolution using the DNS service running in the device itself) works fine, but communication to endpoints further away doesn't work. Investigation on the matter shows that UDP and ICMP traffic from the ^ "no" missing? tethered host is reaching the Internet at all. For TCP traffic there are exchanges between tethered host and server but packets are modified in transit leading to impossible communication. After some trials Matti Vuorela discovered that reducing the URB buffer size by two bytes restored the previous behavior. While a better solution might exist to fix the issue, since the protocol is not publicly documented and considering the small size of the fix, let's do that. Tested-by: Matti Vuorela Signed-off-by: Yves-Alexis Perez Link: https://lore.kernel.org/linux-usb/caan0qaxmysj9vx3zemkviv_b19ju-_exn8yn_usefxpjs6g...@mail.gmail.com/ Link: https://github.com/libimobiledevice/libimobiledevice/issues/1038 Cc: sta...@vger.kernel.org [...] MBR, Sergei
Re: [PATCH 3/3] MIPS: KASLR: Make relocation_address can be configured
Hello! On 19.11.2020 5:29, Jinyang He wrote: When CONFIG_RANDOMIZE_BASE is not set, determine_relocation_address() always returns a constant. It is not friendly to users if the address cannot be used. Make it can be configured at Kconfig. Make it configurable? Signed-off-by: Jinyang He [...] MBR, Sergei
Re: [PATCH] USB: quirks: Add USB_QUIRK_DISCONNECT_SUSPEND quirk for Lenovo A630Z TIO built-in usb-audio card
Hello!
On 18.11.2020 11:56, penghao wrote:
Add a USB_QUIRK_DISCONNECT_SUSPEND quirk for the Lenovo TIO built-in
usb-audio. when A630Z going into S3,the system immediately wakeup 7-8
seconds later by usb-audio disconnect interrupt to avoids the issue.
Seeking a better fix, we've tried a lot of things, including:
- Check that the device's power/wakeup is disabled
- Check that remote wakeup is off at the USB level
- All the quirks in drivers/usb/core/quirks.c
e.g. USB_QUIRK_RESET_RESUME,
USB_QUIRK_RESET,
USB_QUIRK_IGNORE_REMOTE_WAKEUP,
USB_QUIRK_NO_LPM.
but none of that makes any difference.
There are no errors in the logs showing any suspend/resume-related issues.
When the system wakes up due to the modem, log-wise it appears to be a
normal resume.
Introduce a quirk to disable the port during suspend when the modem is
detected.
Changes since v2
- Add Changes commit format
Changes since v1
- Change subject form "ALSA" to "USB:"
- Adjust to approoriate line
Appropriate.
Signed-off-by: penghao
---
drivers/usb/core/quirks.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 7c1198f80c23..25f655c2cd28 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
[...]
@@ -430,6 +434,7 @@ static const struct usb_device_id usb_quirk_list[] = {
USB_QUIRK_DELAY_CTRL_MSG },
/* Corsair Strafe RGB */
+
Why? :-/
{ USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT |
USB_QUIRK_DELAY_CTRL_MSG },
MBR, Sergei
Re: [RFC PATCH] MIPS: Kconfig: Select ARCH_WANT_FRAME_POINTERS
On 16.11.2020 12:47, Tiezhu Yang wrote:
[...]
Select ARCH_WANT_FRAME_POINTERS to fix the following build error under
CONFIG_DEBUG_ATOMIC_SLEEP:
CC arch/mips/kernel/signal.o
{standard input}: Assembler messages:
{standard input}:1775: Error: Unable to parse register name $fp
scripts/Makefile.build:283: recipe for target 'arch/mips/kernel/signal.o'
failed
make[2]: *** [arch/mips/kernel/signal.o] Error 1
scripts/Makefile.build:500: recipe for target 'arch/mips/kernel' failed
make[1]: *** [arch/mips/kernel] Error 2
Makefile:1799: recipe for target 'arch/mips' failed
make: *** [arch/mips] Error 2
I think it is useful to get debugging information as described in the
following texts:
lib/Kconfig.debug
config FRAME_POINTER
bool "Compile the kernel with frame pointers"
...
default y if (DEBUG_INFO && UML) || ARCH_WANT_FRAME_POINTERS
help
If you say Y here the resulting kernel image will be slightly
larger and slower, but it gives very useful debugging information
in case of kernel bugs. (precise oopses/stacktraces/warnings)
Documentation/dev-tools/kgdb.rst
This option inserts code to into the compiled executable which saves
So to or into? :-)
Hi Sergei,
Thanks for pointing that out，this may be a typo which should be "into".
Oops, I didn't realize it was a quote from the KGDB docs... :-)
I will update the commit message to correct it.
Thank you!
Thanks,
Tiezhu
[...]
MBR, Sergei
Re: [RFC PATCH] MIPS: Kconfig: Select ARCH_WANT_FRAME_POINTERS
Hello!
On 16.11.2020 8:03, Tiezhu Yang wrote:
Select ARCH_WANT_FRAME_POINTERS to fix the following build error under
CONFIG_DEBUG_ATOMIC_SLEEP:
CC arch/mips/kernel/signal.o
{standard input}: Assembler messages:
{standard input}:1775: Error: Unable to parse register name $fp
scripts/Makefile.build:283: recipe for target 'arch/mips/kernel/signal.o' failed
make[2]: *** [arch/mips/kernel/signal.o] Error 1
scripts/Makefile.build:500: recipe for target 'arch/mips/kernel' failed
make[1]: *** [arch/mips/kernel] Error 2
Makefile:1799: recipe for target 'arch/mips' failed
make: *** [arch/mips] Error 2
I think it is useful to get debugging information as described in the
following texts:
lib/Kconfig.debug
config FRAME_POINTER
bool "Compile the kernel with frame pointers"
...
default y if (DEBUG_INFO && UML) || ARCH_WANT_FRAME_POINTERS
help
If you say Y here the resulting kernel image will be slightly
larger and slower, but it gives very useful debugging information
in case of kernel bugs. (precise oopses/stacktraces/warnings)
Documentation/dev-tools/kgdb.rst
This option inserts code to into the compiled executable which saves
So to or into? :-)
the frame information in registers or on the stack at different points
which allows a debugger such as gdb to more accurately construct stack
back traces while debugging the kernel.
Signed-off-by: Tiezhu Yang
[...]
MBR, Sergei
Re: [PATCH 2/2] MAINTAINERS: Set myself as Goldfish RTC maintainer
Hello! On 11/14/20 4:09 PM, Jiaxun Yang wrote: > While Gildfish platform is dusted, the RTC driver remains Goldfish. :-) > valuable for us. > > I'm volunteering to maintain goldfish RTC driver onward. > > Signed-off-by: Jiaxun Yang > Cc: Miodrag Dinic [...] MBR, Sergei
Re: [PATCH v4 8/8] [DNI] arm64: dts: renesas: eagle: Specify channel amplitude
On 11/12/20 7:27 PM, Jacopo Mondi wrote:
> Use the newly introduced 'maxim,maxim,initial-reverse-channel-mV'
"maxim," repeated twice.
> property to specify the initial reverse channel amplitude when the
> remote serializers are not pre-programmed with noise immunity threshold
> enabled.
>
> Signed-off-by: Jacopo Mondi
> ---
> arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> index 45b947d44cee..75296865104c 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> @@ -308,6 +308,7 @@ gmsl: gmsl-deserializer@48 {
>
> /* eagle-pca9654-max9286-pwdn */
> enable-gpios = <_expander 0 GPIO_ACTIVE_HIGH>;
> + maxim,initial-reverse-channel-mV = <100>;
>
> /*
>* Workaround: Hog the CAMVDD line high as we can't establish a
MBR, Sergei
Re: [PATCH resend2] sh/intc: Restore devm_ioremap() alignment
Hello!
On 11/10/20 6:49 PM, Geert Uytterhoeven wrote:
> Restore alignment of the continuation of the devm_ioremap() call in
It's a plain ioremap() call below, isn't it, :-)
> register_intc_controller().
>
> Fixes: 4bdc0d676a643140 ("remove ioremap_nocache and devm_ioremap_nocache")
> Signed-off-by: Geert Uytterhoeven
> ---
> drivers/sh/intc/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/sh/intc/core.c b/drivers/sh/intc/core.c
> index f8e070d67fa3266d..a14684ffe4c1a8ef 100644
> --- a/drivers/sh/intc/core.c
> +++ b/drivers/sh/intc/core.c
> @@ -214,7 +214,7 @@ int __init register_intc_controller(struct intc_desc
> *desc)
> d->window[k].phys = res->start;
> d->window[k].size = resource_size(res);
> d->window[k].virt = ioremap(res->start,
> - resource_size(res));
> + resource_size(res));
> if (!d->window[k].virt)
> goto err2;
> }
MBR, Sergei
Re: [PATCH] MIPS: BMC47xx: fix kconfig dependency bug for BCM47XX_SSB
Hello! On 04.11.2020 19:41, Necip Fazil Yildiran wrote: When BCM47XX_SSB is enabled and SSB_PCIHOST is disabled, it results in the following Kbuild warning: WARNING: unmet direct dependencies detected for SSB_B43_PCI_BRIDGE Depends on [n]: SSB [=y] && SSB_PCIHOST [=n] Selected by [y]: - BCM47XX_SSB [=y] && BCM47XX [=y] && PCI [=y] The reason is that BCM47XX_SSB selects SSB_B43_PCI_BRIDGE without depending on or selecting SSB_PCIHOST while SSB_B43_PCI_BRIDGE depends on SSB_PCIHOST. This can also fail building the kernel as demonstrated in a bug report. Honor the kconfig dependency to remove unmet direct dependency warnings and avoid any potential build failures. Link: https://bugzilla.kernel.org/show_bug.cgi?id=210051 Isn't that tag supposed to be used for the patch mail itself? At least that's what I'm seeing in the USB patches where it points to the msg with a patch... Signed-off-by: Necip Fazil Yildiran [...] MBR, Sergei
Re: [PATCH AUTOSEL 4.19 08/60] MIPS: ftrace: Remove redundant #ifdef CONFIG_DYNAMIC_FTRACE
Hello! On 27.10.2020 3:03, Sasha Levin wrote: From: Zejiang Tang [ Upstream commit 39116103a7345927fa99644d08bc0cc9d45fea6f ] There exists redundant #ifdef CONFIG_DYNAMIC_FTRACE in ftrace.c, remove it. Signed-off-by: Zejiang Tang Reviewed-by: Steven Rostedt (VMware) Signed-off-by: Thomas Bogendoerfer Signed-off-by: Sasha Levin --- arch/mips/kernel/ftrace.c | 4 1 file changed, 4 deletions(-) diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c index b122cbb4aad18..7dd52da55907f 100644 --- a/arch/mips/kernel/ftrace.c +++ b/arch/mips/kernel/ftrace.c @@ -37,10 +37,6 @@ void arch_ftrace_update_code(int command) ftrace_modify_all_code(command); } -#endif - -#ifdef CONFIG_DYNAMIC_FTRACE - #define JAL 0x0c00/* jump & link: ip --> ra, jump to target */ #define ADDR_MASK 0x03ff /* op_code|addr : 31...26|25 0 */ #define JUMP_RANGE_MASK ((1UL << 28) - 1) Are you sure this is neccessary in -stable? What bug does it fix? MBR, Sergei
Re: [PATCH net] ravb: Fix bit fields checking in ravb_hwtstamp_get()
On 9/30/20 10:21 PM, Andrew Gabbasov wrote:
> In the function ravb_hwtstamp_get() in ravb_main.c with the existing
> values for RAVB_RXTSTAMP_TYPE_V2_L2_EVENT (0x2) and RAVB_RXTSTAMP_TYPE_ALL
> (0x6)
>
> if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_V2_L2_EVENT)
> config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
> else if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_ALL)
> config.rx_filter = HWTSTAMP_FILTER_ALL;
>
> if the test on RAVB_RXTSTAMP_TYPE_ALL should be true,
> it will never be reached.
>
> This issue can be verified with 'hwtstamp_config' testing program
> (tools/testing/selftests/net/hwtstamp_config.c). Setting filter type
> to ALL and subsequent retrieving it gives incorrect value:
>
> $ hwtstamp_config eth0 OFF ALL
> flags = 0
> tx_type = OFF
> rx_filter = ALL
> $ hwtstamp_config eth0
> flags = 0
> tx_type = OFF
> rx_filter = PTP_V2_L2_EVENT
>
> Correct this by converting if-else's to switch.
>
> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
> Reported-by: Julia Lawall
> Signed-off-by: Andrew Gabbasov
Reviewed-by: Sergei Shtylyov
[...]
MBR, Sergei
Re: [PATCH net] ravb: Fix bit fields checking in ravb_hwtstamp_get()
Hello! On 10/19/20 10:32 AM, Andrew Gabbasov wrote: Sorry for the delay again, I keep forgetting about the mails I' couldn't reply quickly. :-| [...] >>The patch was set to the "Changes Requested" state -- most probably >> because of this >> mail. Though unintentionally, it served to throttle actions on this patch. I >> did only >> remember about this patch yesterday... :-) >> >> [...] In the function ravb_hwtstamp_get() in ravb_main.c with the existing values for RAVB_RXTSTAMP_TYPE_V2_L2_EVENT (0x2) and RAVB_RXTSTAMP_TYPE_ALL (0x6) if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_V2_L2_EVENT) config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT; else if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_ALL) config.rx_filter = HWTSTAMP_FILTER_ALL; if the test on RAVB_RXTSTAMP_TYPE_ALL should be true, it will never be reached. This issue can be verified with 'hwtstamp_config' testing program (tools/testing/selftests/net/hwtstamp_config.c). Setting filter type to ALL and subsequent retrieving it gives incorrect value: $ hwtstamp_config eth0 OFF ALL flags = 0 tx_type = OFF rx_filter = ALL $ hwtstamp_config eth0 flags = 0 tx_type = OFF rx_filter = PTP_V2_L2_EVENT Correct this by converting if-else's to switch. >>> >>> Earlier you proposed to fix this issue by changing the value >>> of RAVB_RXTSTAMP_TYPE_ALL constant to 0x4. >>> Unfortunately, simple changing of the constant value will not >>> be enough, since the code in ravb_rx() (actually determining >>> if timestamp is needed) >>> >>> u32 get_ts = priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE; >>> [...] >>> get_ts &= (q == RAVB_NC) ? >>> RAVB_RXTSTAMP_TYPE_V2_L2_EVENT : >>> ~RAVB_RXTSTAMP_TYPE_V2_L2_EVENT; >>> >>> will work incorrectly and will need to be fixed too, making this >>> piece of code more complicated. Judging on the above code, we can only stamp RAVB_RXTSTAMP_TYPE_V2_L2_EVENT on the NC queue, and the rest only on the BE queue, right? >>> So, it's probably easier and safer to keep the constant value and >>> the code in ravb_rx() intact, and just fix the get ioctl code, >>> where the issue is actually located. >> >>We have one more issue with the current driver: bit 2 of >> priv->tstamp_rx_ctrl >> can only be set as a part of the ALL mask, not individually. I'm now >> thinking we >> should set RAVB_RXTSTAMP_TYPE[_ALL] to 2 (and probably just drop the ALL >> mask)... > > [skipped] > >>Yeah, that's better. But do we really need am anonymous bit 2 that can't >> be >> toggled other than via passing the ALL mask? > > The driver supports setting timestamps either for all packets or for some > particular kind of packets (events). Bit 1 in internal mask corresponds > to this selected kind. Bit 2 corresponds to all other packets, and ALL mask > combines both variants. Although bit 2 can't be controlled individually > (since there is no much sense to Request stamping of only packets, other than > events, moreover, there is no user-visible filter constant to represent it), > and that's why is anonymous, it provides a convenient way to handle stamping > logic in ravb_rx(), so I don't see an immediate need to get rid of it. OK, you convinced me. :-) I suggest that you repost the patch since it's now applying with a large offset. [...] > Best regards, > Andrew MBR, Sergei
Re: [PATCH v1] usb: dwc3: core: fix a issue about clear connect state
Hello!
On 10/18/20 4:47 PM, Dejin Zheng wrote:
> According to Synopsys Programming Guide chapter 2.2 Register Resets,
> it cannot reset the DCTL register by set DCTL.CSFTRST for Core Soft Reset,
s/set/setting/.
> if DWC3 controller as a slave device and stay connected with a usb host,
> then, reboot linux, it will fail to reinitialize dwc3 as a slave device
s/reboot/while rebooting/.
> when the DWC3 controller did not power off. because the connection status
> is incorrect, so we also need clear DCTL.RUN_STOP bit for disable connect
^ to ^ ing
> when do core soft reset.
^ ing
> Fixes: f59dcab176293b6 ("usb: dwc3: core: improve reset sequence")
> Signed-off-by: Dejin Zheng
[...]
MBR, Sergei
Re: [PATCH net] ravb: Fix bit fields checking in ravb_hwtstamp_get()
Hello!
On 10/1/20 10:13 AM, Andrew Gabbasov wrote:
The patch was set to the "Changes Requested" state -- most probably because
of this
mail. Though unintentionally, it served to throttle actions on this patch. I
did only
remember about this patch yesterday... :-)
[...]
>> In the function ravb_hwtstamp_get() in ravb_main.c with the existing
> values
>> for RAVB_RXTSTAMP_TYPE_V2_L2_EVENT (0x2) and RAVB_RXTSTAMP_TYPE_ALL
>> (0x6)
>>
>> if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_V2_L2_EVENT)
>> config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
>> else if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_ALL)
>> config.rx_filter = HWTSTAMP_FILTER_ALL;
>>
>> if the test on RAVB_RXTSTAMP_TYPE_ALL should be true, it will never be
>> reached.
>>
>> This issue can be verified with 'hwtstamp_config' testing program
>> (tools/testing/selftests/net/hwtstamp_config.c). Setting filter type to
> ALL
>> and subsequent retrieving it gives incorrect value:
>>
>> $ hwtstamp_config eth0 OFF ALL
>> flags = 0
>> tx_type = OFF
>> rx_filter = ALL
>> $ hwtstamp_config eth0
>> flags = 0
>> tx_type = OFF
>> rx_filter = PTP_V2_L2_EVENT
>>
>> Correct this by converting if-else's to switch.
>
> Earlier you proposed to fix this issue by changing the value
> of RAVB_RXTSTAMP_TYPE_ALL constant to 0x4.
> Unfortunately, simple changing of the constant value will not
> be enough, since the code in ravb_rx() (actually determining
> if timestamp is needed)
>
> u32 get_ts = priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE;
> [...]
> get_ts &= (q == RAVB_NC) ?
> RAVB_RXTSTAMP_TYPE_V2_L2_EVENT :
> ~RAVB_RXTSTAMP_TYPE_V2_L2_EVENT;
>
> will work incorrectly and will need to be fixed too, making this
> piece of code more complicated.
>
> So, it's probably easier and safer to keep the constant value and
> the code in ravb_rx() intact, and just fix the get ioctl code,
> where the issue is actually located.
We have one more issue with the current driver: bit 2 of priv->tstamp_rx_ctrl
can only be set as a part of the ALL mask, not individually. I'm now thinking we
should set RAVB_RXTSTAMP_TYPE[_ALL] to 2 (and probably just drop the ALL
mask)...
[...]
>> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
>> Reported-by: Julia Lawall
>> Signed-off-by: Andrew Gabbasov
>> ---
>> drivers/net/ethernet/renesas/ravb_main.c | 10 +++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
>> b/drivers/net/ethernet/renesas/ravb_main.c
>> index df89d09b253e..c0610b2d3b14 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -1802,12 +1802,16 @@ static int ravb_hwtstamp_get(struct net_device
>> *ndev, struct ifreq *req)
>> config.flags = 0;
>> config.tx_type = priv->tstamp_tx_ctrl ? HWTSTAMP_TX_ON :
>> HWTSTAMP_TX_OFF;
>> -if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_V2_L2_EVENT)
>> +switch (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE) {
>> +case RAVB_RXTSTAMP_TYPE_V2_L2_EVENT:
>> config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
>> -else if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_ALL)
>> +break;
>> +case RAVB_RXTSTAMP_TYPE_ALL:
>> config.rx_filter = HWTSTAMP_FILTER_ALL;
>> -else
>> +break;
>> +default:
>> config.rx_filter = HWTSTAMP_FILTER_NONE;
Yeah, that's better. But do we really need am anonymous bit 2 that can't be
toggled other than via passing the ALL mask?
[...]
MBR, Sergei
