* [PATCH 11/20] dt-bindings: usb: dwc3: Add synopsys, dwc3 compatible string
From: Serge Semin @ 2020-10-14 10:13 UTC (permalink / raw)
To: Mathias Nyman, Felipe Balbi, Greg Kroah-Hartman, Rob Herring
Cc: devicetree, linux-snps-arc, linux-mips, Neil Armstrong,
Kevin Hilman, Yoshihiro Shimoda, linux-usb, linux-kernel,
Lad Prabhakar, Serge Semin, Bjorn Andersson, Serge Semin,
Manu Gautam, Andy Gross, Pavel Parkhomenko, Alexey Malahov,
linuxppc-dev, linux-arm-kernel, Roger Quadros
In-Reply-To: <20201014101402.18271-1-Sergey.Semin@baikalelectronics.ru>
The DWC USB3 driver and some DTS files like Exynos 5250, Keystone k2e, etc
expects the DWC USB3 DT node to have the compatible string with the
"synopsys" vendor prefix. Let's add the corresponding compatible string to
the controller DT schema, but mark it as deprecated seeing the Synopsys,
Inc. is presented with just "snps" vendor prefix.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
Changelog v2:
- Drop quotes from around the compat string constant.
---
Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 12102a84faf6..2f746ac64e71 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -20,7 +20,10 @@ allOf:
properties:
compatible:
contains:
- const: snps,dwc3
+ oneOf:
+ - const: snps,dwc3
+ - const: synopsys,dwc3
+ deprecated: true
interrupts:
description: |
--
2.27.0
^ permalink raw reply related
* [PATCH 01/20] dt-bindings: usb: usb-hcd: Convert generic USB properties to DT schema
From: Serge Semin @ 2020-10-14 10:13 UTC (permalink / raw)
To: Mathias Nyman, Felipe Balbi, Greg Kroah-Hartman, Rob Herring
Cc: devicetree, linux-snps-arc, linux-mips, Neil Armstrong,
Kevin Hilman, Yoshihiro Shimoda, linux-usb, linux-kernel,
Lad Prabhakar, Serge Semin, Bjorn Andersson, Serge Semin,
Manu Gautam, Andy Gross, Pavel Parkhomenko, Alexey Malahov,
linuxppc-dev, linux-arm-kernel, Roger Quadros
In-Reply-To: <20201014101402.18271-1-Sergey.Semin@baikalelectronics.ru>
The generic USB HCD properties have been described in the legacy bindings
text file: Documentation/devicetree/bindings/usb/generic.txt . Let's
convert it' content into the USB HCD DT schema properties so all USB DT
nodes would be validated to have them properly utilized.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
Changelog v2:
- Discard '|' in all the new properties, since we don't need to preserve
the text formatting.
- Convert abbreviated form of the "maximum-speed" enum restriction into
the multi-lined version of the list.
- Drop quotes from around the string constants.
---
.../devicetree/bindings/usb/generic.txt | 57 ------------
.../devicetree/bindings/usb/usb-hcd.yaml | 88 +++++++++++++++++++
2 files changed, 88 insertions(+), 57 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/usb/generic.txt
diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
deleted file mode 100644
index ba472e7aefc9..000000000000
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ /dev/null
@@ -1,57 +0,0 @@
-Generic USB Properties
-
-Optional properties:
- - maximum-speed: tells USB controllers we want to work up to a certain
- speed. Valid arguments are "super-speed-plus",
- "super-speed", "high-speed", "full-speed" and
- "low-speed". In case this isn't passed via DT, USB
- controllers should default to their maximum HW
- capability.
- - dr_mode: tells Dual-Role USB controllers that we want to work on a
- particular mode. Valid arguments are "host",
- "peripheral" and "otg". In case this attribute isn't
- passed via DT, USB DRD controllers should default to
- OTG.
- - phy_type: tells USB controllers that we want to configure the core to support
- a UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is
- selected. Valid arguments are "utmi" and "utmi_wide".
- In case this isn't passed via DT, USB controllers should
- default to HW capability.
- - otg-rev: tells usb driver the release number of the OTG and EH supplement
- with which the device and its descriptors are compliant,
- in binary-coded decimal (i.e. 2.0 is 0200H). This
- property is used if any real OTG features(HNP/SRP/ADP)
- is enabled, if ADP is required, otg-rev should be
- 0x0200 or above.
- - companion: phandle of a companion
- - hnp-disable: tells OTG controllers we want to disable OTG HNP, normally HNP
- is the basic function of real OTG except you want it
- to be a srp-capable only B device.
- - srp-disable: tells OTG controllers we want to disable OTG SRP, SRP is
- optional for OTG device.
- - adp-disable: tells OTG controllers we want to disable OTG ADP, ADP is
- optional for OTG device.
- - usb-role-switch: boolean, indicates that the device is capable of assigning
- the USB data role (USB host or USB device) for a given
- USB connector, such as Type-C, Type-B(micro).
- see connector/usb-connector.yaml.
- - role-switch-default-mode: indicating if usb-role-switch is enabled, the
- device default operation mode of controller while usb
- role is USB_ROLE_NONE. Valid arguments are "host" and
- "peripheral". Defaults to "peripheral" if not
- specified.
-
-
-This is an attribute to a USB controller such as:
-
-dwc3@4a030000 {
- compatible = "synopsys,dwc3";
- reg = <0x4a030000 0xcfff>;
- interrupts = <0 92 4>
- usb-phy = <&usb2_phy>, <&usb3,phy>;
- maximum-speed = "super-speed";
- dr_mode = "otg";
- phy_type = "utmi_wide";
- otg-rev = <0x0200>;
- adp-disable;
-};
diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
index 7263b7f2b510..ee7ea205c71d 100644
--- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
@@ -22,9 +22,97 @@ properties:
description:
Name specifier for the USB PHY
+ maximum-speed:
+ description:
+ Tells USB controllers we want to work up to a certain speed. In case this
+ isn't passed via DT, USB controllers should default to their maximum HW
+ capability.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum:
+ - low-speed
+ - full-speed
+ - high-speed
+ - super-speed
+ - super-speed-plus
+
+ dr_mode:
+ description:
+ Tells Dual-Role USB controllers that we want to work on a particular
+ mode. In case this attribute isn't passed via DT, USB DRD controllers
+ should default to OTG.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [host, peripheral, otg]
+
+ phy_type:
+ description:
+ Tells USB controllers that we want to configure the core to support a
+ UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is selected. In case
+ this isn't passed via DT, USB controllers should default to HW
+ capability.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [utmi, utmi_wide]
+
+ otg-rev:
+ description:
+ Tells usb driver the release number of the OTG and EH supplement with
+ which the device and its descriptors are compliant, in binary-coded
+ decimal (i.e. 2.0 is 0200H). This property is used if any real OTG
+ features (HNP/SRP/ADP) is enabled. If ADP is required, otg-rev should be
+ 0x0200 or above.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ companion:
+ description: Phandle of a companion device
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ hnp-disable:
+ description:
+ Tells OTG controllers we want to disable OTG HNP. Normally HNP is the
+ basic function of real OTG except you want it to be a srp-capable only B
+ device.
+ type: boolean
+
+ srp-disable:
+ description:
+ Tells OTG controllers we want to disable OTG SRP. SRP is optional for OTG
+ device.
+ type: boolean
+
+ adp-disable:
+ description:
+ Tells OTG controllers we want to disable OTG ADP. ADP is optional for OTG
+ device.
+ type: boolean
+
+ usb-role-switch:
+ description:
+ Indicates that the device is capable of assigning the USB data role
+ (USB host or USB device) for a given USB connector, such as Type-C,
+ Type-B(micro). See connector/usb-connector.yaml.
+
+ role-switch-default-mode:
+ description:
+ Indicates if usb-role-switch is enabled, the device default operation
+ mode of controller while usb role is USB_ROLE_NONE.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [host, peripheral]
+ default: peripheral
+
examples:
- |
usb {
phys = <&usb2_phy1>, <&usb3_phy1>;
phy-names = "usb";
};
+ - |
+ usb@4a030000 {
+ compatible = "snps,dwc3";
+ reg = <0x4a030000 0xcfff>;
+ interrupts = <0 92 4>;
+ usb-phy = <&usb2_phy>, <&usb3_phy>;
+ maximum-speed = "super-speed";
+ dr_mode = "otg";
+ phy_type = "utmi_wide";
+ otg-rev = <0x0200>;
+ adp-disable;
+ };
--
2.27.0
^ permalink raw reply related
* Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
From: Serge Semin @ 2020-10-14 17:16 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andrew Lunn, linux-usb, Neil Armstrong, Tony Lindgren,
Bjorn Andersson, Pavel Parkhomenko,
linux-samsung-soc@vger.kernel.org, Kevin Hilman, Gregory Clement,
Wei Xu, Chen-Yu Tsai, Kukjin Kim, Andy Gross, linux-arm-msm,
linux-snps-arc, Sebastian Hesselbarth, devicetree, Jason Cooper,
Mathias Nyman, linux-kernel@vger.kernel.org, Lad Prabhakar,
Maxime Ripard, Alexey Malahov, Rob Herring, Santosh Shilimkar,
linux-omap, linux-arm-kernel, Roger Quadros, Felipe Balbi,
linux-mips, Greg Kroah-Hartman, Yoshihiro Shimoda, linuxppc-dev,
Patrice Chotard, Serge Semin, Li Yang, Manu Gautam,
Benoît Cousson, Shawn Guo
In-Reply-To: <CAJKOXPeErocR5-3xCDqBR3-k3w_2EQ_768d71n229cbzeo4TtQ@mail.gmail.com>
On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote:
> On Wed, 14 Oct 2020 at 12:23, Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > In accordance with the DWC USB3 bindings the corresponding node name is
> > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> >
> > Note we don't change the DWC USB3-compatible nodes names of
> > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > in-source comment says that the nodes name need to be preserved as
> > "^dwusb@.*" for some backward compatibility.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> >
> > ---
> >
> > Please, test the patch out to make sure it doesn't brake the dependent DTS
> > files. I did only a manual grepping of the possible nodes dependencies.
>
> 1. It is you who should compare the decompiled DTS, not us. For example:
> $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
>
> $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
> dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
> dts-new/${i#dts-old/}.fdt ; done
So basically you suggest first to compile the old and new dts files, then to
de-compile them, then diff old and new fdt's, and visually compare the results.
Personally it isn't that much better than what I did, since each old and new
dtbs will for sure differ due to the node names change suggested in this patch.
So it will lead to the visual debugging too, which isn't that effective. But
your approach is still more demonstrative to make sure that I didn't loose any
nodes redefinition, since in the occasion the old and new de-compiled nodes will
differ not only by the node names but with an additional old named node.
So to speak thanks for suggesting it. I'll try it to validate the proposed
changes.
Two questions:
1) Any advise of a good inliner/command to compile all dtbs at once? Of course I
can get all the updated dtsi'es, then find out all the dts'es which include
them, then directly use dtc to compile the found dts'es... On the other hand I
can just compile all dts'es, then compare old and new ones. The diff of the
non-modified dtb'es will be just empty...
2) What crosc64 is?
>
> 2. Split it per arm architectures (and proper subject prefix - not
> "arch") and subarchitectures so maintainers can pick it up.
Why? The changes are simple and can be formatted as a single patch. I've seen
tons of patches submitted like that, accepted and then merged. What you suggest
is just much more work, which I don't see quite required.
>
> 3. The subject title could be more accurate - there is no fix here
> because there was no errors in the first place. Requirement of DWC
> node names comes recently, so it is more alignment with dtschema.
> Otherwise automatic-pickup-stable-bot might want to pick up... and it
> should not go to stable.
Actually it is a fix, because the USB DT nodes should have been named with "usb"
prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes
naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm
DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was
wrong in the first place.
Regarding automatic-pickup-stable-bot if it exists I don't think it scans all the
emails, but most likely the stable@vger.kernel.org list only or the emails
having the "Fixes:" tag. If I am wrong please give me a link to the bot sources
or refer to a doc where I can read about the way it works, to take it into
account in future commits. Just to note I submitted patches which did some fixes,
had the word "fix" in the subject but weren't selected to be backported to the
stable kernel.
Anyway I don't really care that much about the subject text using the word "fix"
or some else. So if you suggest some better alternative, I'd be glad to consider
it.
-Sergey
>
> Best regards,
> Krzysztof
>
> > arch/arm/boot/dts/armada-375.dtsi | 2 +-
> > arch/arm/boot/dts/exynos5250.dtsi | 2 +-
> > arch/arm/boot/dts/exynos54xx.dtsi | 4 ++--
> > arch/arm/boot/dts/keystone-k2e.dtsi | 4 ++--
> > arch/arm/boot/dts/keystone.dtsi | 2 +-
> > arch/arm/boot/dts/ls1021a.dtsi | 2 +-
> > arch/arm/boot/dts/omap5-l4.dtsi | 2 +-
> > arch/arm/boot/dts/stih407-family.dtsi | 2 +-
> > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
> > arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 ++--
> > arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +-
> > arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 ++--
> > arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++---
> > arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++--
> > arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ++--
> > arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +-
> > arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 4 ++--
> > arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++--
> > arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
> > arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +-
> > arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 2 +-
> > arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 ++--
> > arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
> > arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++--
> > arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +-
> > 25 files changed, 38 insertions(+), 38 deletions(-)
> >
^ permalink raw reply
* Re: [PATCH 04/20] dt-bindings: usb: usb-hcd: Add "tpl-support" property
From: Serge Semin @ 2020-10-14 17:27 UTC (permalink / raw)
To: Rob Herring
Cc: Neil Armstrong, Bjorn Andersson, Pavel Parkhomenko, Kevin Hilman,
Andy Gross, linux-snps-arc, devicetree, Mathias Nyman,
Lad Prabhakar, Alexey Malahov, Rob Herring, linux-arm-kernel,
Roger Quadros, Felipe Balbi, Greg Kroah-Hartman,
Yoshihiro Shimoda, linux-usb, linux-mips, Serge Semin,
linux-kernel, Manu Gautam, linuxppc-dev
In-Reply-To: <20201014132756.GA1538723@bogus>
On Wed, Oct 14, 2020 at 08:27:56AM -0500, Rob Herring wrote:
> On Wed, 14 Oct 2020 13:13:46 +0300, Serge Semin wrote:
> > The host controller device might be designed to work for the particular
> > products or applications. In that case its DT node is supposed to be
> > equipped with the tpl-support property.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> >
> > ---
> >
> > Changelog v2:
> > - Grammar fix: "s/it'/its"
> > - Discard '|' from the property description, since we don't need to preserve
> > the text formatting.
> > ---
> > Documentation/devicetree/bindings/usb/usb-hcd.yaml | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
>
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> Traceback (most recent call last):
> File "/usr/local/bin/dt-extract-example", line 45, in <module>
> binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
> File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 343, in load
> return constructor.get_single_data()
> File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 111, in get_single_data
> node = self.composer.get_single_node()
> File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
> File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
> File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
> File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node
> File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
> ruamel.yaml.scanner.ScannerError: mapping values are not allowed in this context
> in "<unicode string>", line 27, column 14
> make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/usb/usb-hcd.example.dts] Error 1
> make[1]: *** Deleting file 'Documentation/devicetree/bindings/usb/usb-hcd.example.dts'
> make[1]: *** Waiting for unfinished jobs....
> ./Documentation/devicetree/bindings/usb/usb-hcd.yaml:27:14: [error] syntax error: mapping values are not allowed here (syntax)
> make[1]: *** [Documentation/devicetree/bindings/Makefile:59: Documentation/devicetree/bindings/processed-schema-examples.json] Error 123
> make: *** [Makefile:1366: dt_binding_check] Error 2
>
>
> See https://patchwork.ozlabs.org/patch/1382001
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure dt-schema is up to date:
>
> pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
>
> Please check and re-submit.
Hm, that's weird. Of course I did the dt_binding_check before submission, but
even after the dt-schema repo update I failed to see the error:
$ make -j8 ARCH=mips CROSS_COMPILE=mipsel-baikal-linux- dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/usb/usb-hcd.yaml
CHKDT Documentation/devicetree/bindings/usb/usb-hcd.yaml
SCHEMA Documentation/devicetree/bindings/processed-schema-examples.yaml
DTC Documentation/devicetree/bindings/usb/usb-hcd.example.dt.yaml
CHECK Documentation/devicetree/bindings/usb/usb-hcd.example.dt.yaml
Rob, any idea why has the bot got mad at me?
-Sergey
>
^ permalink raw reply
* Re: [PATCH 09/20] dt-bindings: usb: Convert DWC USB3 bindings to DT schema
From: Serge Semin @ 2020-10-14 17:37 UTC (permalink / raw)
To: Rob Herring
Cc: Neil Armstrong, linux-mips, Pavel Parkhomenko, Kevin Hilman,
Andy Gross, linux-snps-arc, devicetree, Mathias Nyman,
Lad Prabhakar, Alexey Malahov, Rob Herring, Bjorn Andersson,
linux-arm-kernel, Roger Quadros, Felipe Balbi, Greg Kroah-Hartman,
Yoshihiro Shimoda, linux-usb, linux-kernel, Serge Semin,
Manu Gautam, linuxppc-dev
In-Reply-To: <20201014133219.GA1545403@bogus>
On Wed, Oct 14, 2020 at 08:32:19AM -0500, Rob Herring wrote:
> On Wed, 14 Oct 2020 13:13:51 +0300, Serge Semin wrote:
> > DWC USB3 DT node is supposed to be compliant with the Generic xHCI
> > Controller schema, but with additional vendor-specific properties, the
> > controller-specific reference clocks and PHYs. So let's convert the
> > currently available legacy text-based DWC USB3 bindings to the DT schema
> > and make sure the DWC USB3 nodes are also validated against the
> > usb-xhci.yaml schema.
> >
> > Note we have to discard the nodename restriction of being prefixed with
> > "dwc3@" string, since in accordance with the usb-hcd.yaml schema USB nodes
> > are supposed to be named as "^usb(@.*)".
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> >
> > ---
> >
> > Changelog v2:
> > - Discard '|' from the descriptions, since we don't need to preserve
> > the text formatting in any of them.
> > - Drop quotes from around the string constants.
> > - Fix the "clock-names" prop description to be referring the enumerated
> > clock-names instead of the ones from the Databook.
> > ---
> > .../devicetree/bindings/usb/dwc3.txt | 125 --------
> > .../devicetree/bindings/usb/snps,dwc3.yaml | 295 ++++++++++++++++++
> > 2 files changed, 295 insertions(+), 125 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/usb/dwc3.txt
> > create mode 100644 Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >
>
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> ./Documentation/devicetree/bindings/usb/snps,dwc3.yaml:44:4: [warning] wrong indentation: expected 4 but found 3 (indentation)
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dt.yaml: dwc3@a600000: $nodename:0: 'dwc3@a600000' does not match '^usb(@.*)?'
> From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.example.dt.yaml: usb@ff500000: snps,quirk-frame-length-adjustment: True is not of type 'array'
> From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>
>
> See https://patchwork.ozlabs.org/patch/1382003
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure dt-schema is up to date:
>
> pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
>
> Please check and re-submit.
>
Both of these errors are fixed in the following patches of the series:
[PATCH 17/20] dt-bindings: usb: qcom,dwc3: Validate DWC3 sub-node
[PATCH 15/20] dt-bindings: usb: meson-g12a-usb: Fix FL-adj property value
This patch preserves the original legacy bindings and doesn't touch the
depended bogus DT schemas.
-Sergey
^ permalink raw reply
* Re: [PATCH 00/20] dt-bindings: usb: Add generic USB HCD, xHCI, DWC USB3 DT schema
From: Serge Semin @ 2020-10-14 17:39 UTC (permalink / raw)
To: Mathias Nyman, Felipe Balbi, Greg Kroah-Hartman, Rob Herring
Cc: devicetree, linux-snps-arc, linux-mips, Neil Armstrong,
Kevin Hilman, Yoshihiro Shimoda, linux-usb, linux-kernel,
Lad Prabhakar, Serge Semin, Bjorn Andersson, Manu Gautam,
Andy Gross, Pavel Parkhomenko, Alexey Malahov, linuxppc-dev,
linux-arm-kernel, Roger Quadros
In-Reply-To: <20201014101402.18271-1-Sergey.Semin@baikalelectronics.ru>
Ah, forgot to mark the series as v2. Sorry about that. The next one will be v3
then...
-Sergey
On Wed, Oct 14, 2020 at 01:13:42PM +0300, Serge Semin wrote:
> We've performed some work on the Generic USB HCD, xHCI and DWC USB3 DT
> bindings in the framework of the Baikal-T1 SoC support integration into
> the kernel. This patchset is a result of that work.
>
> First of all we moved the generic USB properties from the legacy text
> bindings into the USB HCD DT schema. So now the generic USB HCD-compatible
> DT nodes are validated taking into account the optional properties like:
> maximum-speed, dr_mode, otg-rev, usb-role-switch, etc. We've fixed these
> properties a bit so they would correspond to what functionality kernel
> currently supports.
>
> Secondly we converted generic USB xHCI text bindings file into the DT
> schema. It had to be split up into two bindings: DT schema with generic
> xHCI properties and a generic xHCI device DT schema. The later will be
> used to validate the pure xHCI-based nodes, while the former can be
> utilized by some vendor-specific versions of xHCI.
>
> Thirdly, what was primarily intended to be done for Baikal-T1 SoC USB we
> converted the legacy text-based DWC USB3 bindings to DT schema and altered
> the result a bit so it would be more coherent with what actually
> controller and its driver support. Since we've now got the DWC USB3 DT
> schema, we made it used to validate the sub-nodes of the Qualcom, TI and
> Amlogic DWC3 DT nodes.
>
> Finally we've also fixed all the OHCI/EHCI, xHCI and DW USB3 compatible DT
> nodes so they would comply with the nodes naming scema declared in the USB
> HCD DT bindings file.
>
> Link: https://lore.kernel.org/linux-usb/20201010224121.12672-1-Sergey.Semin@baikalelectronics.ru/
> Changelog v2:
> - Thanks to Sergei Shtylyov for suggesting the commit logs grammar fixes:
> [PATCH 04/18] dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types
> [PATCH 05/18] dt-bindings: usb: usb-hcd: Add "tpl-support" property
> [PATCH 11/18] dt-bindings: usb: dwc3: Add interrupt-names property support
> [PATCH 13/18] dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions
> [PATCH 17/18] dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node
> - Set FL-adj of the amlogiv,meson-g12a-usb controller with value 0x20 instead
> of completely removing the property.
> - Drop the patch:
> [PATCH 02/18] dt-bindings: usb: usb-hcd: Add "wireless" maximum-speed
> property value
> since "wireless" speed type is depracated due to lack of the device
> supporting it.
> - Drop quotes from around the compat string constant.
> - Discard '|' from the property descriptions, since we don't need to preserve
> the text formatting.
> - Convert abbreviated form of the "maximum-speed" enum constraint into
> the multi-lined version of the list.
> - Fix the DW USB3 "clock-names" prop description to be refererring to the
> enumerated clock-names instead of the ones from the Databook.
> - Add explicit "additionalProperties: true" to the usb-xhci.yaml schema,
> since additionalProperties/unevaluatedProperties are going to be mandary
> for each binding.
> - Use "oneOf: [dwc2.yaml#, snps,dwc3.yaml#]" instead of the bulky "if:
> properties: compatibe: ..." statement.
> - Discard the "^dwc3@[0-9a-f]+$" nodes from being acceptable as sub-nodes
> of the Qualcomm DWC3 DT nodes.
> - Add new patches:
> [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
> [PATCH 19/20] arch: dts: Fix xHCI DT nodes name
> [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Manu Gautam <mgautam@codeaurora.org>
> Cc: Roger Quadros <rogerq@ti.com>
> Cc: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-snps-arc@lists.infradead.org
> Cc: linux-mips@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-usb@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> Serge Semin (20):
> dt-bindings: usb: usb-hcd: Convert generic USB properties to DT schema
> dt-bindings: usb: usb-hcd: Add "otg-rev" property restriction
> dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types
> dt-bindings: usb: usb-hcd: Add "tpl-support" property
> dt-bindings: usb: usb-hcd: Add generic "usb-phy" property
> dt-bindings: usb: Convert xHCI bindings to DT schema
> dt-bindings: usb: xhci: Add Broadcom STB v2 compatible device
> dt-bindings: usb: renesas-xhci: Refer to the usb-xhci.yaml file
> dt-bindings: usb: Convert DWC USB3 bindings to DT schema
> dt-bindings: usb: dwc3: Add interrupt-names property support
> dt-bindings: usb: dwc3: Add synopsys,dwc3 compatible string
> dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions
> dt-bindings: usb: dwc3: Add Frame Length Adj restrictions
> dt-bindings: usb: meson-g12a-usb: Fix FL-adj property value
> dt-bindings: usb: meson-g12a-usb: Validate DWC2/DWC3 sub-nodes
> dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node
> dt-bindings: usb: qcom,dwc3: Validate DWC3 sub-node
> arch: dts: Fix EHCI/OHCI DT nodes name
> arch: dts: Fix xHCI DT nodes name
> arch: dts: Fix DWC USB3 DT nodes name
>
> .../usb/amlogic,meson-g12a-usb-ctrl.yaml | 6 +-
> .../devicetree/bindings/usb/dwc3.txt | 125 -------
> .../devicetree/bindings/usb/generic-xhci.yaml | 65 ++++
> .../devicetree/bindings/usb/generic.txt | 57 ----
> .../devicetree/bindings/usb/qcom,dwc3.yaml | 9 +-
> .../bindings/usb/renesas,usb-xhci.yaml | 4 +-
> .../devicetree/bindings/usb/snps,dwc3.yaml | 315 ++++++++++++++++++
> .../bindings/usb/ti,keystone-dwc3.yaml | 4 +-
> .../devicetree/bindings/usb/usb-hcd.yaml | 104 ++++++
> .../devicetree/bindings/usb/usb-xhci.txt | 41 ---
> .../devicetree/bindings/usb/usb-xhci.yaml | 42 +++
> arch/arc/boot/dts/axs10x_mb.dtsi | 4 +-
> arch/arc/boot/dts/hsdk.dts | 4 +-
> arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +-
> arch/arm/boot/dts/armada-375.dtsi | 2 +-
> arch/arm/boot/dts/bcm5301x.dtsi | 6 +-
> arch/arm/boot/dts/bcm53573.dtsi | 4 +-
> arch/arm/boot/dts/exynos5250.dtsi | 2 +-
> arch/arm/boot/dts/exynos54xx.dtsi | 4 +-
> arch/arm/boot/dts/hisi-x5hd2.dtsi | 4 +-
> arch/arm/boot/dts/keystone-k2e.dtsi | 4 +-
> arch/arm/boot/dts/keystone.dtsi | 2 +-
> arch/arm/boot/dts/lpc18xx.dtsi | 4 +-
> arch/arm/boot/dts/ls1021a.dtsi | 2 +-
> arch/arm/boot/dts/omap5-l4.dtsi | 2 +-
> arch/arm/boot/dts/stih407-family.dtsi | 2 +-
> arch/arm/boot/dts/stm32mp151.dtsi | 4 +-
> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
> arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 +-
> arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +-
> .../arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 +-
> .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +-
> .../arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 +-
> .../arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 +-
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +-
> .../arm64/boot/dts/hisilicon/hi3798cv200.dtsi | 4 +-
> arch/arm64/boot/dts/hisilicon/hip06.dtsi | 4 +-
> arch/arm64/boot/dts/hisilicon/hip07.dtsi | 4 +-
> arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 4 +-
> arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 4 +-
> arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 +-
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 +-
> arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 +-
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 +-
> arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +-
> arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 +-
> arch/mips/boot/dts/ingenic/jz4770.dtsi | 2 +-
> arch/mips/boot/dts/mti/sead3.dts | 2 +-
> arch/mips/boot/dts/ralink/mt7628a.dtsi | 2 +-
> arch/powerpc/boot/dts/akebono.dts | 6 +-
> 53 files changed, 605 insertions(+), 305 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/usb/dwc3.txt
> create mode 100644 Documentation/devicetree/bindings/usb/generic-xhci.yaml
> delete mode 100644 Documentation/devicetree/bindings/usb/generic.txt
> create mode 100644 Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> delete mode 100644 Documentation/devicetree/bindings/usb/usb-xhci.txt
> create mode 100644 Documentation/devicetree/bindings/usb/usb-xhci.yaml
>
> --
> 2.27.0
>
^ permalink raw reply
* Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
From: Serge Semin @ 2020-10-14 18:11 UTC (permalink / raw)
To: Florian Fainelli
Cc: Neil Armstrong, Bjorn Andersson, Paul Cercueil, Paul Mackerras,
Pavel Parkhomenko, linux-stm32, Rafał Miłecki,
Alexey Brodkin, Wei Xu, Andy Gross, bcm-kernel-feedback-list,
Kevin Hilman, linux-snps-arc, devicetree, Alexandre Torgue,
Mathias Nyman, Hauke Mehrtens, Lad Prabhakar, Vladimir Zapolskiy,
Rob Herring, linux-mediatek, Matthias Brugger, Alexey Malahov,
linux-arm-kernel, Roger Quadros, Felipe Balbi,
Thomas Bogendoerfer, Greg Kroah-Hartman, Yoshihiro Shimoda,
linux-usb, linux-mips, Serge Semin, linux-kernel, Manu Gautam,
Maxime Coquelin, Vineet Gupta, linuxppc-dev
In-Reply-To: <ed0221ee-c586-b8f8-a7d1-4a2a190dc41b@gmail.com>
On Wed, Oct 14, 2020 at 11:00:45AM -0700, Florian Fainelli wrote:
> On 10/14/20 3:14 AM, Serge Semin wrote:
> > In accordance with the Generic EHCI/OHCI bindings the corresponding node
> > name is suppose to comply with the Generic USB HCD DT schema, which
> > requires the USB nodes to have the name acceptable by the regexp:
> > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with
> > incompatible names.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> >
> > ---
> >
> > Please, test the patch out to make sure it doesn't brake the dependent DTS
> > files. I did only a manual grepping of the possible nodes dependencies.
>
> Not sure how you envisioned these change to be picked up, but you may
> need to split these changes between ARM/ARM64, MIPS and PowerPC at
> least. And within ARM/ARM64 you will most likely have to split according
> to the various SoC maintainers.
Hmm, I don't really know how it's going to be done in this case, but there must
be a way to get the cross-platform patches picked up in general. For
instance, see the patches like:
714acdbd1c94 arch: rename copy_thread_tls() back to copy_thread()
140c8180eb7c arch: remove HAVE_COPY_THREAD_TLS
They touched the files from different files, but still have been merged in.
Maybe I should have copied these three patches to the "linux-arch@vger.kernel.org"
list or some other mailing list...
-Sergey
> --
> Florian
^ permalink raw reply
* Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
From: Serge Semin @ 2020-10-14 21:21 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring
Cc: Neil Armstrong, Bjorn Andersson, Paul Cercueil, Paul Mackerras,
Pavel Parkhomenko, linux-stm32, Rafał Miłecki,
Alexey Brodkin, Wei Xu, Andy Gross, bcm-kernel-feedback-list,
Kevin Hilman, linux-snps-arc, devicetree, Alexandre Torgue,
Mathias Nyman, Hauke Mehrtens, Lad Prabhakar, Vladimir Zapolskiy,
linux-mediatek, Matthias Brugger, Alexey Malahov,
linux-arm-kernel, Roger Quadros, Felipe Balbi,
Thomas Bogendoerfer, Greg Kroah-Hartman, Yoshihiro Shimoda,
linux-usb, linux-mips, linux-kernel, Manu Gautam, Maxime Coquelin,
Vineet Gupta, linuxppc-dev
In-Reply-To: <932c52de-c2f1-ceea-e436-1471830700e9@gmail.com>
On Wed, Oct 14, 2020 at 11:41:17AM -0700, Florian Fainelli wrote:
> On 10/14/20 11:11 AM, Serge Semin wrote:
> > On Wed, Oct 14, 2020 at 11:00:45AM -0700, Florian Fainelli wrote:
> >> On 10/14/20 3:14 AM, Serge Semin wrote:
> >>> In accordance with the Generic EHCI/OHCI bindings the corresponding node
> >>> name is suppose to comply with the Generic USB HCD DT schema, which
> >>> requires the USB nodes to have the name acceptable by the regexp:
> >>> "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with
> >>> incompatible names.
> >>>
> >>> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> >>>
> >>> ---
> >>>
> >>> Please, test the patch out to make sure it doesn't brake the dependent DTS
> >>> files. I did only a manual grepping of the possible nodes dependencies.
> >>
> >
> >> Not sure how you envisioned these change to be picked up, but you may
> >> need to split these changes between ARM/ARM64, MIPS and PowerPC at
> >> least. And within ARM/ARM64 you will most likely have to split according
> >> to the various SoC maintainers.
> >
> > Hmm, I don't really know how it's going to be done in this case, but there must
> > be a way to get the cross-platform patches picked up in general. For
> > instance, see the patches like:
> > 714acdbd1c94 arch: rename copy_thread_tls() back to copy_thread()
> > 140c8180eb7c arch: remove HAVE_COPY_THREAD_TLS
> > They touched the files from different files, but still have been merged in.
>
> That situation is different, when a new facility is added and someone
> has gone through the work of adding support for all architectures (or
> nearly all of them), you want them to be merged in a way that limits
> merge conflicts with other architecture changes.
>
> Here you are fixing warnings, and each file you touch can clearly be
> independently change from other files in the series without causing
> merge conflicts. You are however creating the potential for merge
> conflicts with other changes that the various SoC maintainers have
> queued up.
>
> > Maybe I should have copied these three patches to the "linux-arch@vger.kernel.org"
> > list or some other mailing list...
>
> Maybe Rob can queue them through his device tree repository, with the
> ack of the various SoC maintainers...
That's what I hoped for in the first place. But AFAICS now Rob does the splitting
of his patches himself, while the repo is used either for the
Documentation/devicetree/ patches or for the Rob'es own work. So it seems to me
I'll have to split the series up and resubmit... (
Hope I am wrong. So, @Rob, will you be able to merge this and the next two patches
in via your repo or you'd rather suggest for me to split it up and resubmit?
-Sergey
> --
> Florian
^ permalink raw reply
* Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
From: Serge Semin @ 2020-10-14 21:22 UTC (permalink / raw)
To: Rob Herring
Cc: Andrew Lunn, Linux USB List, Neil Armstrong, Tony Lindgren,
Bjorn Andersson, Wei Xu, linux-samsung-soc, Kevin Hilman,
Gregory Clement, Krzysztof Kozlowski, Chen-Yu Tsai, Kukjin Kim,
Andy Gross, linux-arm-msm, arcml, Sebastian Hesselbarth,
devicetree, Jason Cooper, Mathias Nyman,
linux-kernel@vger.kernel.org, Lad Prabhakar, Maxime Ripard,
Alexey Malahov, Santosh Shilimkar, linux-omap, linux-arm-kernel,
Roger Quadros, Felipe Balbi, open list:MIPS, Greg Kroah-Hartman,
Yoshihiro Shimoda, linuxppc-dev, Patrice Chotard, Serge Semin,
Li Yang, Manu Gautam, Benoît Cousson, Shawn Guo,
Pavel Parkhomenko
In-Reply-To: <CAL_JsqKNuYS1ojJMCx1whLgynz+cTZ-Hvxn5pEFJc_PUgA1hsg@mail.gmail.com>
On Wed, Oct 14, 2020 at 01:35:16PM -0500, Rob Herring wrote:
> On Wed, Oct 14, 2020 at 9:37 AM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
> > >
> > > Hi Serge,
> > >
> > > Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> > > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > >
> >
> > > DWC3 is not a simple HDC, though.
> >
> > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
> > which are tuned by the DWC USB3 driver in the kernel. But after that the
> > controller is registered as xhci-hcd device so it's serviced by the xHCI driver,
> > which then registers the HCD device so the corresponding DT node is supposed
> > to be compatible with the next bindings: usb/usb-hcd.yaml, usb/usb-xhci.yaml
> > and usb/snps,dwc3,yaml. I've created the later one so to validate the denoted
> > compatibility.
> >
> > >
> > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > > >
> > > > Note we don't change the DWC USB3-compatible nodes names of
> > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > > in-source comment says that the nodes name need to be preserved as
> > > > "^dwusb@.*" for some backward compatibility.
> > >
> >
> > > interesting, compatibility with what? Some debugfs files, perhaps? :-)
> >
> > Don't really know.) In my experience the worst type of such compatibility is
> > connected with some bootloader magic, which may add/remove/modify properties
> > to nodes with pre-defined names.
>
> I seriously doubt anyone is using the APM machines with DT (even ACPI
> is somewhat doubtful). I say change them. Or remove the dts files and
> see what happens. Either way it can always be reverted.
Ok. I'll change them in v3.
-Sergey
>
> Rob
^ permalink raw reply
* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Khalid Aziz @ 2020-10-14 21:21 UTC (permalink / raw)
To: Catalin Marinas
Cc: Jann Horn, linuxppc-dev, linux-kernel, Christoph Hellwig,
linux-mm, Paul Mackerras, sparclinux, Anthony Yznaga,
Andrew Morton, Will Deacon, David S. Miller, linux-arm-kernel
In-Reply-To: <20201013091638.GA10778@gaia>
On 10/13/20 3:16 AM, Catalin Marinas wrote:
> On Mon, Oct 12, 2020 at 01:14:50PM -0600, Khalid Aziz wrote:
>> On 10/12/20 11:22 AM, Catalin Marinas wrote:
>>> On Mon, Oct 12, 2020 at 11:03:33AM -0600, Khalid Aziz wrote:
>>>> On 10/10/20 5:09 AM, Catalin Marinas wrote:
>>>>> On Wed, Oct 07, 2020 at 02:14:09PM -0600, Khalid Aziz wrote:
>>>>>> On 10/7/20 1:39 AM, Jann Horn wrote:
>>>>>>> arch_validate_prot() is a hook that can validate whether a given set of
>>>>>>> protection flags is valid in an mprotect() operation. It is given the set
>>>>>>> of protection flags and the address being modified.
>>>>>>>
>>>>>>> However, the address being modified can currently not actually be used in
>>>>>>> a meaningful way because:
>>>>>>>
>>>>>>> 1. Only the address is given, but not the length, and the operation can
>>>>>>> span multiple VMAs. Therefore, the callee can't actually tell which
>>>>>>> virtual address range, or which VMAs, are being targeted.
>>>>>>> 2. The mmap_lock is not held, meaning that if the callee were to check
>>>>>>> the VMA at @addr, that VMA would be unrelated to the one the
>>>>>>> operation is performed on.
>>>>>>>
>>>>>>> Currently, custom arch_validate_prot() handlers are defined by
>>>>>>> arm64, powerpc and sparc.
>>>>>>> arm64 and powerpc don't care about the address range, they just check the
>>>>>>> flags against CPU support masks.
>>>>>>> sparc's arch_validate_prot() attempts to look at the VMA, but doesn't take
>>>>>>> the mmap_lock.
>>>>>>>
>>>>>>> Change the function signature to also take a length, and move the
>>>>>>> arch_validate_prot() call in mm/mprotect.c down into the locked region.
>>>>> [...]
>>>>>> As Chris pointed out, the call to arch_validate_prot() from do_mmap2()
>>>>>> is made without holding mmap_lock. Lock is not acquired until
>>>>>> vm_mmap_pgoff(). This variance is uncomfortable but I am more
>>>>>> uncomfortable forcing all implementations of validate_prot to require
>>>>>> mmap_lock be held when non-sparc implementations do not have such need
>>>>>> yet. Since do_mmap2() is in powerpc specific code, for now this patch
>>>>>> solves a current problem.
>>>>>
>>>>> I still think sparc should avoid walking the vmas in
>>>>> arch_validate_prot(). The core code already has the vmas, though not
>>>>> when calling arch_validate_prot(). That's one of the reasons I added
>>>>> arch_validate_flags() with the MTE patches. For sparc, this could be
>>>>> (untested, just copied the arch_validate_prot() code):
>>>>
>>>> I am little uncomfortable with the idea of validating protection bits
>>>> inside the VMA walk loop in do_mprotect_pkey(). When ADI is being
>>>> enabled across multiple VMAs and arch_validate_flags() fails on a VMA
>>>> later, do_mprotect_pkey() will bail out with error leaving ADI enabled
>>>> on earlier VMAs. This will apply to protection bits other than ADI as
>>>> well of course. This becomes a partial failure of mprotect() call. I
>>>> think it should be all or nothing with mprotect() - when one calls
>>>> mprotect() from userspace, either the entire address range passed in
>>>> gets its protection bits updated or none of it does. That requires
>>>> validating protection bits upfront or undoing what earlier iterations of
>>>> VMA walk loop might have done.
>>>
>>> I thought the same initially but mprotect() already does this with the
>>> VM_MAY* flag checking. If you ask it for an mprotect() that crosses
>>> multiple vmas and one of them fails, it doesn't roll back the changes to
>>> the prior ones. I considered that a similar approach is fine for MTE
>>> (it's most likely a user error).
>>
>> You are right about the current behavior with VM_MAY* flags, but that is
>> not the right behavior. Adding more cases to this just perpetuates
>> incorrect behavior. It is not easy to roll back changes after VMAs have
>> potentially been split/merged which is probably why the current code
>> simply throws in the towel and returns with partially modified address
>> space. It is lot easier to do all the checks upfront and then proceed or
>> not proceed with modifying VMAs. One approach might be to call
>> arch_validate_flags() in a loop before modifying VMAs and walk all VMAs
>> with a read lock held. Current code also bails out with ENOMEM if it
>> finds a hole in the address range and leaves any modifications already
>> made in place. This is another case where a hole could have been
>> detected earlier.
>
> This should be ideal indeed though with the risk of breaking the current
> ABI (FWIW, FreeBSD seems to do a first pass to check for violations:
> https://github.com/freebsd/freebsd/blob/master/sys/vm/vm_map.c#L2630).
I am not sure I understand where the ABI breakage would be. Are we aware
of apps that intentionally modify address space partially using the
current code? What FreeBSD does seems like a reasonable thing to do. Any
way first thing to do is to update sparc to use arch_validate_flags()
and update sparc_validate_prot() to not peek into vma without lock. I
can do that unless Jann wants to rework this 2 patch series with these
changes.
>
> However, I'm not sure it's worth the hassle. Do we expect the user to
> call mprotect() across multiple mixed type mappings while relying on no
> change if an error is returned? We should probably at least document the
> current behaviour in the mprotect man page.
>
Yes, documenting current behavior is definitely a good thing to do.
--
Khalid
^ permalink raw reply
* Re: [PATCH 11/20] dt-bindings: usb: dwc3: Add synopsys,dwc3 compatible string
From: Serge Semin @ 2020-10-14 21:35 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring
Cc: Felipe Balbi, linux-snps-arc, linux-mips, Mathias Nyman,
Neil Armstrong, Greg Kroah-Hartman, Yoshihiro Shimoda, linux-usb,
linux-kernel, Lad Prabhakar, Serge Semin, Alexey Malahov,
Manu Gautam, devicetree, Andy Gross, Pavel Parkhomenko,
Kevin Hilman, Bjorn Andersson, linuxppc-dev, linux-arm-kernel,
Roger Quadros
In-Reply-To: <20201014201818.GA6926@kozik-lap>
On Wed, Oct 14, 2020 at 10:18:18PM +0200, Krzysztof Kozlowski wrote:
> On Wed, Oct 14, 2020 at 01:13:53PM +0300, Serge Semin wrote:
> > The DWC USB3 driver and some DTS files like Exynos 5250, Keystone k2e, etc
> > expects the DWC USB3 DT node to have the compatible string with the
> > "synopsys" vendor prefix. Let's add the corresponding compatible string to
> > the controller DT schema, but mark it as deprecated seeing the Synopsys,
> > Inc. is presented with just "snps" vendor prefix.
>
> Instead of adding deprecated schema just correct the DTSes to use snps.
> The "synopsys" is not even in vendor prefixes.
Yeah, it's not, but the driver and some dts'es use it this way. I am not sure
that the solution suggested by you is much better than mine. So let's hear the
Rob'es opinion out in this matter. @Rob, what do you think?
-Sergey
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Jann Horn @ 2020-10-14 22:29 UTC (permalink / raw)
To: Khalid Aziz
Cc: Catalin Marinas, linuxppc-dev, kernel list, Christoph Hellwig,
Linux-MM, Paul Mackerras, sparclinux, Anthony Yznaga,
Andrew Morton, Will Deacon, David S. Miller, Linux ARM
In-Reply-To: <e4c2c56b-3dbe-73dd-ea72-a5378de7de6a@oracle.com>
On Wed, Oct 14, 2020 at 11:24 PM Khalid Aziz <khalid.aziz@oracle.com> wrote:
[...]
> current code? What FreeBSD does seems like a reasonable thing to do. Any
> way first thing to do is to update sparc to use arch_validate_flags()
> and update sparc_validate_prot() to not peek into vma without lock. I
> can do that unless Jann wants to rework this 2 patch series with these
> changes.
Ah, if you're willing to take care of that, that'd be nice, please do. :)
^ permalink raw reply
* Re: [PATCH -next] Revert "powerpc/pci: unmap legacy INTx interrupts when a PHB is removed"
From: Oliver O'Halloran @ 2020-10-14 23:00 UTC (permalink / raw)
To: Qian Cai
Cc: Stephen Rothwell, Alexey Kardashevskiy, Linux Kernel Mailing List,
Linux-Next Mailing List, Cédric Le Goater, linuxppc-dev
In-Reply-To: <20201014182811.12027-1-cai@lca.pw>
On Thu, Oct 15, 2020 at 5:28 AM Qian Cai <cai@lca.pw> wrote:
>
> This reverts commit 3a3181e16fbde752007759f8759d25e0ff1fc425 which
> causes memory corruptions on POWER9 NV.
I was going to post this along with a fix for Cedric's original bug,
but I can do that separately so:
Acked-by: Oliver O'Halloran <oohall@gmail.com>
^ permalink raw reply
* Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
From: Serge Semin @ 2020-10-14 23:51 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andrew Lunn, linux-usb, Neil Armstrong, Tony Lindgren,
Bjorn Andersson, Pavel Parkhomenko,
linux-samsung-soc@vger.kernel.org, Kevin Hilman, Gregory Clement,
Wei Xu, Chen-Yu Tsai, Kukjin Kim, Andy Gross, linux-arm-msm,
linux-snps-arc, Sebastian Hesselbarth, devicetree, Jason Cooper,
Mathias Nyman, linux-kernel@vger.kernel.org, Lad Prabhakar,
Maxime Ripard, Alexey Malahov, Rob Herring, Santosh Shilimkar,
linux-omap, linux-arm-kernel, Roger Quadros, Felipe Balbi,
linux-mips, Greg Kroah-Hartman, Yoshihiro Shimoda, linuxppc-dev,
Patrice Chotard, Serge Semin, Li Yang, Manu Gautam,
Benoît Cousson, Shawn Guo
In-Reply-To: <CAJKOXPcHi_=jea=0YrPNo4dh6k03+63Tc2Uo+sd0u8+XPdQjOw@mail.gmail.com>
On Wed, Oct 14, 2020 at 10:04:32PM +0200, Krzysztof Kozlowski wrote:
> On Wed, 14 Oct 2020 at 19:16, Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote:
> > > On Wed, 14 Oct 2020 at 12:23, Serge Semin
> > > <Sergey.Semin@baikalelectronics.ru> wrote:
> > > >
> > > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > > >
> > > > Note we don't change the DWC USB3-compatible nodes names of
> > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > > in-source comment says that the nodes name need to be preserved as
> > > > "^dwusb@.*" for some backward compatibility.
> > > >
> > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > >
> > > > ---
> > > >
> > > > Please, test the patch out to make sure it doesn't brake the dependent DTS
> > > > files. I did only a manual grepping of the possible nodes dependencies.
> > >
> >
> > > 1. It is you who should compare the decompiled DTS, not us. For example:
> > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > > scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
> > >
> > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > > fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
> > > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
> > > dts-new/${i#dts-old/}.fdt ; done
> >
> > So basically you suggest first to compile the old and new dts files, then to
> > de-compile them, then diff old and new fdt's, and visually compare the results.
> > Personally it isn't that much better than what I did, since each old and new
> > dtbs will for sure differ due to the node names change suggested in this patch.
> > So it will lead to the visual debugging too, which isn't that effective. But
> > your approach is still more demonstrative to make sure that I didn't loose any
> > nodes redefinition, since in the occasion the old and new de-compiled nodes will
> > differ not only by the node names but with an additional old named node.
>
> My suggestion is to compare the entire, effective DTS after all
> inclusions. Maybe you did it already, I don't know.
Only by grepping the dts'es, which include the dtsi'es modified in this patch.
So your suggestion of compiling and de-compiling has been indeed relevant.
> The point is that
> when you change node names in DTSI but you miss one in DTS, you end up
> with two nodes.
Yep, that's exactly what I meant when I said that your approach was more
demonstrative, etc.
> This is much easier to spot with dtxdiff or with
> fdtdump (which behaves better for node moves).
>
> Indeed it is still a visual comparison - if you have any ideas how to
> automate it (e.g. ignore phandle changes), please share. It would
> solve my testings as well.
Alas I don't. That's why to save my time of coming up with an automated solution
I did a very thorough modification making sure that each affected node isn't
updated in the corresponding dts'es and asked to test the patches out.
Anyway the approach suggested by you will indeed give us a better understanding
of my patches correctness. So I'll use it before sending v3. Thanks.
> But asking others to test because you do
> not want to check it is not the best way to handle such changes.
>
> >
> > So to speak thanks for suggesting it. I'll try it to validate the proposed
> > changes.
> >
> > Two questions:
> > 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I
> > can get all the updated dtsi'es, then find out all the dts'es which include
> > them, then directly use dtc to compile the found dts'es... On the other hand I
> > can just compile all dts'es, then compare old and new ones. The diff of the
> > non-modified dtb'es will be just empty...
>
> make dtbs
It's not that easy.) "make dtbs" will build dtbs only for enabled boards, which
first need to be enabled in the kernel config. So I'll need to have a config
with all the affected dts. The later is the same as if I just found all the
affected dts and built them one-by-one by directly calling dtc.
> touch your dts or git stash pop
> make dtbs
> compare
> diff for all unchanged will be simply empty, so easy to spot
>
> > 2) What crosc64 is?
>
> Ah, just an alias for cross compiling + ccache + kbuild out. I just
> copied you my helpers, so you need to tweak them.
>
> >
> > >
> > > 2. Split it per arm architectures (and proper subject prefix - not
> > > "arch") and subarchitectures so maintainers can pick it up.
> >
> > Why? The changes are simple and can be formatted as a single patch. I've seen
> > tons of patches submitted like that, accepted and then merged. What you suggest
> > is just much more work, which I don't see quite required.
>
> DTS changes go separate between arm64 and arm. There is nothing
> unusual here - all changes are submitted like this.
> Second topic is to split by subarchitectures which is necessary if you
> want it to be picked up by maintainers. It also makes it easier to
> review.
The current patches are easy enough for review. The last three patches of the
series is a collection of the one-type changes concerning the same type of
nodes. So reviewing them won't cause any difficulty. But I assume that's not
the main point in this discussion.
> Sure, without split ber subarchitectures this could be picked
> up by SoC folks but you did not even CC them. So if you do not want to
> split it per subarchitectures for maintainers and you do not CC SoC,
> then how do you believe this should be picked up? Out of the regular
> patch submission way? That's not how the changes are handled.
AFAIU there are another ways of merging comprehensive patches. If they get to collect
all the Acked-by tags, they could be merged in, for instance, through Greg' or Rob'
(for dts) repos, if of course they get to agree with doing that. Am I wrong?
My hope was to ask Rob or Greg to get the patches merged in when they get
to collect all the ackes, since I thought it was an option in such cases. So if
they refuse to do so I'll have no choice but to split the series up into a
smaller patches as you say.
>
> >
> > >
> > > 3. The subject title could be more accurate - there is no fix here
> > > because there was no errors in the first place. Requirement of DWC
> > > node names comes recently, so it is more alignment with dtschema.
> > > Otherwise automatic-pickup-stable-bot might want to pick up... and it
> > > should not go to stable.
> >
> > Actually it is a fix, because the USB DT nodes should have been named with "usb"
> > prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes
> > naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm
> > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was
> > wrong in the first place.
>
> Not following the naming convention of DT spec which was loosely
> enforced is not an error which should be "fixed". Simply wrong title.
> This is an alignment with dtschema or correcting naming convention.
> Not fixing errors.
From your perspective it wasn't an error, from mine and most likely Rob' it
was.) Anyway as I said I don't care that much about preserving the subject
wording, so what about the next one:
<arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema
?
-Sergey
>
> >
> > Regarding automatic-pickup-stable-bot if it exists I don't think it scans all the
> > emails, but most likely the stable@vger.kernel.org list only or the emails
> > having the "Fixes:" tag. If I am wrong please give me a link to the bot sources
> > or refer to a doc where I can read about the way it works, to take it into
> > account in future commits. Just to note I submitted patches which did some fixes,
> > had the word "fix" in the subject but weren't selected to be backported to the
> > stable kernel.
>
> You mixed up bots. The regular stable bot picks commits with cc-stable
> or with "Fixes". The auto-pickup bot picks all commits (not emails...
> why would it look at emails?) looking like a fix. Wording could be one
> of the hints used in the heuristic. Anyway, this is not a fix,
> regardless of autosel, so the wording is not correct.
>
> Just Google for AUTOSEL. You can then ask Sasha for sources...
>
> > Anyway I don't really care that much about the subject text using the word "fix"
> > or some else. So if you suggest some better alternative, I'd be glad to consider
> > it.
>
> I already did. One example is: alignment with dtschema.
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH -next] Revert "powerpc/pci: unmap legacy INTx interrupts when a PHB is removed"
From: Stephen Rothwell @ 2020-10-15 0:18 UTC (permalink / raw)
To: Oliver O'Halloran
Cc: Alexey Kardashevskiy, Linux Kernel Mailing List,
Linux-Next Mailing List, Qian Cai, Cédric Le Goater,
linuxppc-dev
In-Reply-To: <CAOSf1CFT_Y67Q8caH2uFOYtwpRgFozh30ZWWZzzR-x18LBsG8g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 524 bytes --]
Hi Oliver,
On Thu, 15 Oct 2020 10:00:49 +1100 "Oliver O'Halloran" <oohall@gmail.com> wrote:
>
> On Thu, Oct 15, 2020 at 5:28 AM Qian Cai <cai@lca.pw> wrote:
> >
> > This reverts commit 3a3181e16fbde752007759f8759d25e0ff1fc425 which
> > causes memory corruptions on POWER9 NV.
>
> I was going to post this along with a fix for Cedric's original bug,
> but I can do that separately so:
>
> Acked-by: Oliver O'Halloran <oohall@gmail.com>
I added that to linux-next today.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 00/13] mm/debug_vm_pgtable fixes
From: Anshuman Khandual @ 2020-10-15 2:59 UTC (permalink / raw)
To: Andrew Morton, Aneesh Kumar K.V; +Cc: linux-mm, linuxppc-dev
In-Reply-To: <20201014133607.fbf63d060e331fcd6007b624@linux-foundation.org>
On 10/15/2020 02:06 AM, Andrew Morton wrote:
> On Wed, 14 Oct 2020 08:45:16 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
>
>>> Against mm-debug_vm_pgtable-avoid-none-pte-in-pte_clear_test.patch:
>>>
>>> https://lkml.kernel.org/r/87zh5wx51b.fsf@linux.ibm.com
>>
>>
>> yes this one we should get fixed. I was hoping someone familiar with
>> Riscv pte updates rules would pitch in. IIUC we need to update
>> RANDON_ORVALUE similar to how we updated it for s390 and ppc64.
>>
>>
>> Alternatively we can do this
>>
>> modified mm/debug_vm_pgtable.c
>> @@ -548,7 +548,7 @@ static void __init pte_clear_tests(struct mm_struct
>> *mm, pte_t *ptep,
>> pte_t pte = pfn_pte(pfn, prot);
>>
>> pr_debug("Validating PTE clear\n");
>> - pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>> +// pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>> set_pte_at(mm, vaddr, ptep, pte);
>> barrier();
>> pte_clear(mm, vaddr, ptep);
>>
>> till we get that feedback from RiscV team?
>
> Would it be better to do
>
> #ifdef CONFIG_S390
> pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
> #endif
I would suggest just dropping RANDOM_ORVALUE from pte_clear_tests()
possibly with a comment saying it needs to be fixed on RISCV before
being added back later.
pte = __pte(pte_val(pte));
OR
Disable RANDOM_ORVALUE only for RISCV.
#ifndef CONFIG_RISCV
pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
#endif
^ permalink raw reply
* [PATCH 1/2] ASoC: dt-bindings: fsl_spdif: Add new compatible string for i.MX8QM
From: Shengjiu Wang @ 2020-10-15 5:28 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood, robh+dt, devicetree
Cc: linuxppc-dev, linux-kernel
Add new compatible string "fsl,imx8qm-spdif" for supporting spdif
module on i.MX8QM.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
Documentation/devicetree/bindings/sound/fsl,spdif.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
index 2ac671f5cb9b..50449b6d1048 100644
--- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
@@ -20,6 +20,7 @@ properties:
- fsl,imx35-spdif
- fsl,vf610-spdif
- fsl,imx6sx-spdif
+ - fsl,imx8qm-spdif
reg:
maxItems: 1
--
2.27.0
^ permalink raw reply related
* [PATCH 2/2] ASoC: fsl_spdif: Add support for i.MX8QM platform
From: Shengjiu Wang @ 2020-10-15 5:28 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood, robh+dt, devicetree
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1602739728-4433-1-git-send-email-shengjiu.wang@nxp.com>
On i.MX8QM, there are separate interrupts for TX and RX.
As the EDMA can't be configured to swing back to first FIFO
after writing the second FIFO, so we need to force the burst
size to be 2 on i.MX8QM. And EDMA don't support to shift
the data from S24_LE to S16_LE, so the supported TX format
is also different on i.MX8QM.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_spdif.c | 57 ++++++++++++++++++++++++++++++++-------
1 file changed, 47 insertions(+), 10 deletions(-)
diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index f41496cf5b63..5fa178f3f497 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -49,10 +49,18 @@ static u8 srpc_dpll_locked[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0xa, 0xb };
* @imx: for imx platform
* @shared_root_clock: flag of sharing a clock source with others;
* so the driver shouldn't set root clock rate
+ * @interrupts: interrupt number
+ * @tx_burst: tx maxburst size
+ * @rx_burst: rx maxburst size
+ * @tx_formats: tx supported data format
*/
struct fsl_spdif_soc_data {
bool imx;
bool shared_root_clock;
+ u32 interrupts;
+ u32 tx_burst;
+ u32 rx_burst;
+ u64 tx_formats;
};
/*
@@ -128,16 +136,38 @@ struct fsl_spdif_priv {
static struct fsl_spdif_soc_data fsl_spdif_vf610 = {
.imx = false,
.shared_root_clock = false,
+ .interrupts = 1,
+ .tx_burst = FSL_SPDIF_TXFIFO_WML,
+ .rx_burst = FSL_SPDIF_RXFIFO_WML,
+ .tx_formats = FSL_SPDIF_FORMATS_PLAYBACK,
};
static struct fsl_spdif_soc_data fsl_spdif_imx35 = {
.imx = true,
.shared_root_clock = false,
+ .interrupts = 1,
+ .tx_burst = FSL_SPDIF_TXFIFO_WML,
+ .rx_burst = FSL_SPDIF_RXFIFO_WML,
+ .tx_formats = FSL_SPDIF_FORMATS_PLAYBACK,
};
static struct fsl_spdif_soc_data fsl_spdif_imx6sx = {
.imx = true,
.shared_root_clock = true,
+ .interrupts = 1,
+ .tx_burst = FSL_SPDIF_TXFIFO_WML,
+ .rx_burst = FSL_SPDIF_RXFIFO_WML,
+ .tx_formats = FSL_SPDIF_FORMATS_PLAYBACK,
+
+};
+
+static struct fsl_spdif_soc_data fsl_spdif_imx8qm = {
+ .imx = true,
+ .shared_root_clock = true,
+ .interrupts = 2,
+ .tx_burst = 2, /* Applied for EDMA */
+ .rx_burst = 2, /* Applied for EDMA */
+ .tx_formats = SNDRV_PCM_FMTBIT_S24_LE, /* Applied for EDMA */
};
/* Check if clk is a root clock that does not share clock source with others */
@@ -1283,6 +1313,8 @@ static int fsl_spdif_probe(struct platform_device *pdev)
/* Initialize this copy of the CPU DAI driver structure */
memcpy(&spdif_priv->cpu_dai_drv, &fsl_spdif_dai, sizeof(fsl_spdif_dai));
spdif_priv->cpu_dai_drv.name = dev_name(&pdev->dev);
+ spdif_priv->cpu_dai_drv.playback.formats =
+ spdif_priv->soc->tx_formats;
/* Get the addresses and IRQ */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1297,15 +1329,19 @@ static int fsl_spdif_probe(struct platform_device *pdev)
return PTR_ERR(spdif_priv->regmap);
}
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
+ for (i = 0; i < spdif_priv->soc->interrupts; i++) {
+ irq = platform_get_irq(pdev, i);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "no irq for node %s\n", pdev->name);
+ return irq;
+ }
- ret = devm_request_irq(&pdev->dev, irq, spdif_isr, 0,
- dev_name(&pdev->dev), spdif_priv);
- if (ret) {
- dev_err(&pdev->dev, "could not claim irq %u\n", irq);
- return ret;
+ ret = devm_request_irq(&pdev->dev, irq, spdif_isr, 0,
+ dev_name(&pdev->dev), spdif_priv);
+ if (ret) {
+ dev_err(&pdev->dev, "could not claim irq %u\n", irq);
+ return ret;
+ }
}
/* Get system clock for rx clock rate calculation */
@@ -1354,8 +1390,8 @@ static int fsl_spdif_probe(struct platform_device *pdev)
spdif_priv->dpll_locked = false;
- spdif_priv->dma_params_tx.maxburst = FSL_SPDIF_TXFIFO_WML;
- spdif_priv->dma_params_rx.maxburst = FSL_SPDIF_RXFIFO_WML;
+ spdif_priv->dma_params_tx.maxburst = spdif_priv->soc->tx_burst;
+ spdif_priv->dma_params_rx.maxburst = spdif_priv->soc->rx_burst;
spdif_priv->dma_params_tx.addr = res->start + REG_SPDIF_STL;
spdif_priv->dma_params_rx.addr = res->start + REG_SPDIF_SRL;
@@ -1468,6 +1504,7 @@ static const struct of_device_id fsl_spdif_dt_ids[] = {
{ .compatible = "fsl,imx35-spdif", .data = &fsl_spdif_imx35, },
{ .compatible = "fsl,vf610-spdif", .data = &fsl_spdif_vf610, },
{ .compatible = "fsl,imx6sx-spdif", .data = &fsl_spdif_imx6sx, },
+ { .compatible = "fsl,imx8qm-spdif", .data = &fsl_spdif_imx8qm, },
{}
};
MODULE_DEVICE_TABLE(of, fsl_spdif_dt_ids);
--
2.27.0
^ permalink raw reply related
* Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
From: Krzysztof Kozlowski @ 2020-10-15 6:14 UTC (permalink / raw)
To: Serge Semin
Cc: Andrew Lunn, linux-usb, Neil Armstrong, Tony Lindgren,
Bjorn Andersson, Pavel Parkhomenko,
linux-samsung-soc@vger.kernel.org, Kevin Hilman, Gregory Clement,
Wei Xu, Chen-Yu Tsai, Kukjin Kim, Andy Gross, linux-arm-msm,
linux-snps-arc, Sebastian Hesselbarth, devicetree, Jason Cooper,
Mathias Nyman, linux-kernel@vger.kernel.org, Lad Prabhakar,
Maxime Ripard, Alexey Malahov, Rob Herring, Santosh Shilimkar,
linux-omap, linux-arm-kernel, Roger Quadros, Felipe Balbi,
linux-mips, Greg Kroah-Hartman, Yoshihiro Shimoda, linuxppc-dev,
Patrice Chotard, Serge Semin, Li Yang, Manu Gautam,
Benoît Cousson, Shawn Guo
In-Reply-To: <20201014235105.kj4rtwiidph7gyen@mobilestation>
On Thu, Oct 15, 2020 at 02:51:05AM +0300, Serge Semin wrote:
> >
> > > So to speak thanks for suggesting it. I'll try it to validate the proposed
> > > changes.
> > >
> > > Two questions:
> > > 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I
> > > can get all the updated dtsi'es, then find out all the dts'es which include
> > > them, then directly use dtc to compile the found dts'es... On the other hand I
> > > can just compile all dts'es, then compare old and new ones. The diff of the
> > > non-modified dtb'es will be just empty...
> >
>
> > make dtbs
>
> It's not that easy.) "make dtbs" will build dtbs only for enabled boards, which
> first need to be enabled in the kernel config. So I'll need to have a config
> with all the affected dts. The later is the same as if I just found all the
> affected dts and built them one-by-one by directly calling dtc.
True. Sometimes allyesconfig for given arch might be helpful but not
always (e.g. for ARM it does not select all of ARMv4 and ARMv5 boards).
Most likely your approach is actually faster/more reliable.
>
> > touch your dts or git stash pop
> > make dtbs
> > compare
> > diff for all unchanged will be simply empty, so easy to spot
> >
> > > 2) What crosc64 is?
> >
> > Ah, just an alias for cross compiling + ccache + kbuild out. I just
> > copied you my helpers, so you need to tweak them.
> >
> > >
> > > >
> > > > 2. Split it per arm architectures (and proper subject prefix - not
> > > > "arch") and subarchitectures so maintainers can pick it up.
> > >
> > > Why? The changes are simple and can be formatted as a single patch. I've seen
> > > tons of patches submitted like that, accepted and then merged. What you suggest
> > > is just much more work, which I don't see quite required.
> >
>
> > DTS changes go separate between arm64 and arm. There is nothing
> > unusual here - all changes are submitted like this.
> > Second topic is to split by subarchitectures which is necessary if you
> > want it to be picked up by maintainers. It also makes it easier to
> > review.
>
> The current patches are easy enough for review. The last three patches of the
> series is a collection of the one-type changes concerning the same type of
> nodes. So reviewing them won't cause any difficulty. But I assume that's not
> the main point in this discussion.
>
> > Sure, without split ber subarchitectures this could be picked
> > up by SoC folks but you did not even CC them. So if you do not want to
> > split it per subarchitectures for maintainers and you do not CC SoC,
> > then how do you believe this should be picked up? Out of the regular
> > patch submission way? That's not how the changes are handled.
>
> AFAIU there are another ways of merging comprehensive patches. If they get to collect
> all the Acked-by tags, they could be merged in, for instance, through Greg' or Rob'
> (for dts) repos, if of course they get to agree with doing that. Am I wrong?
>
> My hope was to ask Rob or Greg to get the patches merged in when they get
> to collect all the ackes, since I thought it was an option in such cases. So if
> they refuse to do so I'll have no choice but to split the series up into a
> smaller patches as you say.
This is neither Rob's nor Greg's patch to pick up, but ARM SoC (which was
not CCed here). And most likely they won't pick it up because judging by
contents it is obvious it should go via ARM SoC.
Sure, if there are dependencies between some patches they can go with
acks through unrelated trees, but this not the usual way. This is an
exception in the process to solve particular dependency problem. It has
drawbacks - increases the chances of annoying conflicts.
The case here does not fall into this criteria - there is no dependency
of this patch on the others Therefore there is no reason to use the
unusual/exceptional way of handling patches. There is no reason why
this shouldn't go via either specific ARM subarchitecture maintainers or
via ARM SoC.
> > > > 3. The subject title could be more accurate - there is no fix here
> > > > because there was no errors in the first place. Requirement of DWC
> > > > node names comes recently, so it is more alignment with dtschema.
> > > > Otherwise automatic-pickup-stable-bot might want to pick up... and it
> > > > should not go to stable.
> > >
> > > Actually it is a fix, because the USB DT nodes should have been named with "usb"
> > > prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes
> > > naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm
> > > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was
> > > wrong in the first place.
> >
>
> > Not following the naming convention of DT spec which was loosely
> > enforced is not an error which should be "fixed". Simply wrong title.
> > This is an alignment with dtschema or correcting naming convention.
> > Not fixing errors.
>
> From your perspective it wasn't an error, from mine and most likely Rob' it
> was.) Anyway as I said I don't care that much about preserving the subject
> wording, so what about the next one:
> <arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema
> ?
Looks good.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Catalin Marinas @ 2020-10-15 9:05 UTC (permalink / raw)
To: Khalid Aziz
Cc: Jann Horn, linuxppc-dev, linux-kernel, Christoph Hellwig,
linux-mm, Paul Mackerras, sparclinux, Anthony Yznaga,
Andrew Morton, Will Deacon, David S. Miller, linux-arm-kernel
In-Reply-To: <e4c2c56b-3dbe-73dd-ea72-a5378de7de6a@oracle.com>
On Wed, Oct 14, 2020 at 03:21:16PM -0600, Khalid Aziz wrote:
> On 10/13/20 3:16 AM, Catalin Marinas wrote:
> > On Mon, Oct 12, 2020 at 01:14:50PM -0600, Khalid Aziz wrote:
> >> On 10/12/20 11:22 AM, Catalin Marinas wrote:
> >>> On Mon, Oct 12, 2020 at 11:03:33AM -0600, Khalid Aziz wrote:
> >>>> On 10/10/20 5:09 AM, Catalin Marinas wrote:
> >>>>> I still think sparc should avoid walking the vmas in
> >>>>> arch_validate_prot(). The core code already has the vmas, though not
> >>>>> when calling arch_validate_prot(). That's one of the reasons I added
> >>>>> arch_validate_flags() with the MTE patches. For sparc, this could be
> >>>>> (untested, just copied the arch_validate_prot() code):
> >>>>
> >>>> I am little uncomfortable with the idea of validating protection bits
> >>>> inside the VMA walk loop in do_mprotect_pkey(). When ADI is being
> >>>> enabled across multiple VMAs and arch_validate_flags() fails on a VMA
> >>>> later, do_mprotect_pkey() will bail out with error leaving ADI enabled
> >>>> on earlier VMAs. This will apply to protection bits other than ADI as
> >>>> well of course. This becomes a partial failure of mprotect() call. I
> >>>> think it should be all or nothing with mprotect() - when one calls
> >>>> mprotect() from userspace, either the entire address range passed in
> >>>> gets its protection bits updated or none of it does. That requires
> >>>> validating protection bits upfront or undoing what earlier iterations of
> >>>> VMA walk loop might have done.
> >>>
> >>> I thought the same initially but mprotect() already does this with the
> >>> VM_MAY* flag checking. If you ask it for an mprotect() that crosses
> >>> multiple vmas and one of them fails, it doesn't roll back the changes to
> >>> the prior ones. I considered that a similar approach is fine for MTE
> >>> (it's most likely a user error).
> >>
> >> You are right about the current behavior with VM_MAY* flags, but that is
> >> not the right behavior. Adding more cases to this just perpetuates
> >> incorrect behavior. It is not easy to roll back changes after VMAs have
> >> potentially been split/merged which is probably why the current code
> >> simply throws in the towel and returns with partially modified address
> >> space. It is lot easier to do all the checks upfront and then proceed or
> >> not proceed with modifying VMAs. One approach might be to call
> >> arch_validate_flags() in a loop before modifying VMAs and walk all VMAs
> >> with a read lock held. Current code also bails out with ENOMEM if it
> >> finds a hole in the address range and leaves any modifications already
> >> made in place. This is another case where a hole could have been
> >> detected earlier.
> >
> > This should be ideal indeed though with the risk of breaking the current
> > ABI (FWIW, FreeBSD seems to do a first pass to check for violations:
> > https://github.com/freebsd/freebsd/blob/master/sys/vm/vm_map.c#L2630).
>
> I am not sure I understand where the ABI breakage would be. Are we aware
> of apps that intentionally modify address space partially using the
> current code?
I hope there aren't any but you never know until you make the change and
someone complains. Arguably, such user code is already broken since
mprotect() doesn't even tell where the failure occurred.
> What FreeBSD does seems like a reasonable thing to do. Any way first
> thing to do is to update sparc to use arch_validate_flags() and update
> sparc_validate_prot() to not peek into vma without lock.
If you go for arch_validate_flags(), I think sparc_validate_prot()
doesn't need the vma at all.
BTW, on the ADI topic, I think you have a race in do_swap_page() since
set_pte_at() is called before arch_do_swap_page(). So a thread in the
same process would see the new mapping but the tags have not been
updated yet. Unless sparc relies on the new user pte to be set, I think
you can just swap the two calls.
--
Catalin
^ permalink raw reply
* Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
From: Amelie DELAUNAY @ 2020-10-15 8:05 UTC (permalink / raw)
To: Serge Semin, Mathias Nyman, Felipe Balbi, Greg Kroah-Hartman,
Rob Herring, Alexey Brodkin, Vineet Gupta, Hauke Mehrtens,
Rafał Miłecki, bcm-kernel-feedback-list, Wei Xu,
Vladimir Zapolskiy, Maxime Coquelin, Alexandre Torgue,
Paul Cercueil, Thomas Bogendoerfer, Matthias Brugger,
Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
Cc: devicetree, linux-snps-arc, linux-mips, Neil Armstrong,
Kevin Hilman, Yoshihiro Shimoda, linux-usb, linux-kernel,
Lad Prabhakar, Serge Semin, Bjorn Andersson, Manu Gautam,
Andy Gross, linux-mediatek, Pavel Parkhomenko, Alexey Malahov,
linuxppc-dev, linux-stm32, linux-arm-kernel, Roger Quadros
In-Reply-To: <20201014101402.18271-19-Sergey.Semin@baikalelectronics.ru>
Hi Serge,
On 10/14/20 12:14 PM, Serge Semin wrote:
> In accordance with the Generic EHCI/OHCI bindings the corresponding node
> name is suppose to comply with the Generic USB HCD DT schema, which
> requires the USB nodes to have the name acceptable by the regexp:
> "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with
> incompatible names.
>
> Signed-off-by: Serge Semin<Sergey.Semin@baikalelectronics.ru>
>
> diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi
> index bfe29023fbd5..576f7da564c5 100644
> --- a/arch/arm/boot/dts/stm32mp151.dtsi
> +++ b/arch/arm/boot/dts/stm32mp151.dtsi
> @@ -1404,7 +1404,7 @@ ethernet0: ethernet@5800a000 {
> status = "disabled";
> };
>
> - usbh_ohci: usbh-ohci@5800c000 {
> + usbh_ohci: usb@5800c000 {
> compatible = "generic-ohci";
> reg = <0x5800c000 0x1000>;
> clocks = <&rcc USBH>;
> @@ -1413,7 +1413,7 @@ usbh_ohci: usbh-ohci@5800c000 {
> status = "disabled";
> };
>
> - usbh_ehci: usbh-ehci@5800d000 {
> + usbh_ehci: usb@5800d000 {
> compatible = "generic-ehci";
> reg = <0x5800d000 0x1000>;
> clocks = <&rcc USBH>;
For STM32MP151:
Acked-by: Amelie Delaunay <amelie.delaunay@st.com>
Thanks,
Amelie
^ permalink raw reply
* Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
From: Felipe Balbi @ 2020-10-15 10:15 UTC (permalink / raw)
To: Serge Semin
Cc: Andrew Lunn, linux-usb, Neil Armstrong, Tony Lindgren,
Bjorn Andersson, Wei Xu, linux-samsung-soc, Kevin Hilman,
Gregory Clement, Krzysztof Kozlowski, Chen-Yu Tsai, Kukjin Kim,
Andy Gross, linux-arm-msm, linux-snps-arc, Sebastian Hesselbarth,
devicetree, Jason Cooper, Mathias Nyman, linux-kernel,
Lad Prabhakar, Maxime Ripard, Alexey Malahov, Rob Herring,
Santosh Shilimkar, linux-omap, linux-arm-kernel, Roger Quadros,
linux-mips, Greg Kroah-Hartman, Yoshihiro Shimoda, linuxppc-dev,
Patrice Chotard, Serge Semin, Li Yang, Manu Gautam,
Benoît Cousson, Shawn Guo, Pavel Parkhomenko
In-Reply-To: <20201014143720.yny3jco5pkb7dr4b@mobilestation>
[-- Attachment #1: Type: text/plain, Size: 831 bytes --]
Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
>>
>> Hi Serge,
>>
>> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
>> > In accordance with the DWC USB3 bindings the corresponding node name is
>> > suppose to comply with Generic USB HCD DT schema, which requires the USB
>>
>
>> DWC3 is not a simple HDC, though.
>
> Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
> which are tuned by the DWC USB3 driver in the kernel. But after that the
> controller is registered as xhci-hcd device so it's serviced by the xHCI driver,
in Dual-role or host-only builds, that's correct. We can also have
peripheral-only builds (both SW or HW versions) which means xhci isn't
even in the picture.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]
^ permalink raw reply
* [PATCH] powerpc: force inlining of csum_partial() to avoid multiple csum_partial() with GCC10
From: Christophe Leroy @ 2020-10-15 10:52 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
ppc-linux-objdump -d vmlinux | grep -e "<csum_partial>" -e "<__csum_partial>"
With gcc9 I get:
c0017ef8 <__csum_partial>:
c00182fc: 4b ff fb fd bl c0017ef8 <__csum_partial>
c0018478: 4b ff fa 80 b c0017ef8 <__csum_partial>
c03e8458: 4b c2 fa a0 b c0017ef8 <__csum_partial>
c03e8518: 4b c2 f9 e1 bl c0017ef8 <__csum_partial>
c03ef410: 4b c2 8a e9 bl c0017ef8 <__csum_partial>
c03f0b24: 4b c2 73 d5 bl c0017ef8 <__csum_partial>
c04279a4: 4b bf 05 55 bl c0017ef8 <__csum_partial>
c0429820: 4b be e6 d9 bl c0017ef8 <__csum_partial>
c0429944: 4b be e5 b5 bl c0017ef8 <__csum_partial>
c042b478: 4b be ca 81 bl c0017ef8 <__csum_partial>
c042b554: 4b be c9 a5 bl c0017ef8 <__csum_partial>
c045f15c: 4b bb 8d 9d bl c0017ef8 <__csum_partial>
c0492190: 4b b8 5d 69 bl c0017ef8 <__csum_partial>
c0492310: 4b b8 5b e9 bl c0017ef8 <__csum_partial>
c0495594: 4b b8 29 65 bl c0017ef8 <__csum_partial>
c049c420: 4b b7 ba d9 bl c0017ef8 <__csum_partial>
c049c870: 4b b7 b6 89 bl c0017ef8 <__csum_partial>
c049c930: 4b b7 b5 c9 bl c0017ef8 <__csum_partial>
c04a9ca0: 4b b6 e2 59 bl c0017ef8 <__csum_partial>
c04bdde4: 4b b5 a1 15 bl c0017ef8 <__csum_partial>
c04be480: 4b b5 9a 79 bl c0017ef8 <__csum_partial>
c04be710: 4b b5 97 e9 bl c0017ef8 <__csum_partial>
c04c969c: 4b b4 e8 5d bl c0017ef8 <__csum_partial>
c04ca2fc: 4b b4 db fd bl c0017ef8 <__csum_partial>
c04cf5bc: 4b b4 89 3d bl c0017ef8 <__csum_partial>
c04d0440: 4b b4 7a b9 bl c0017ef8 <__csum_partial>
With gcc10 I get:
c0018d08 <__csum_partial>:
c0019020 <csum_partial>:
c0019020: 4b ff fc e8 b c0018d08 <__csum_partial>
c001914c: 4b ff fe d4 b c0019020 <csum_partial>
c0019250: 4b ff fd d1 bl c0019020 <csum_partial>
c03e404c <csum_partial>:
c03e404c: 4b c3 4c bc b c0018d08 <__csum_partial>
c03e4050: 4b ff ff fc b c03e404c <csum_partial>
c03e40fc: 4b ff ff 51 bl c03e404c <csum_partial>
c03e6680: 4b ff d9 cd bl c03e404c <csum_partial>
c03e68c4: 4b ff d7 89 bl c03e404c <csum_partial>
c03e7934: 4b ff c7 19 bl c03e404c <csum_partial>
c03e7bf8: 4b ff c4 55 bl c03e404c <csum_partial>
c03eb148: 4b ff 8f 05 bl c03e404c <csum_partial>
c03ecf68: 4b c2 bd a1 bl c0018d08 <__csum_partial>
c04275b8 <csum_partial>:
c04275b8: 4b bf 17 50 b c0018d08 <__csum_partial>
c0427884: 4b ff fd 35 bl c04275b8 <csum_partial>
c0427b18: 4b ff fa a1 bl c04275b8 <csum_partial>
c0427bd8: 4b ff f9 e1 bl c04275b8 <csum_partial>
c0427cd4: 4b ff f8 e5 bl c04275b8 <csum_partial>
c0427e34: 4b ff f7 85 bl c04275b8 <csum_partial>
c045a1c0: 4b bb eb 49 bl c0018d08 <__csum_partial>
c0489464 <csum_partial>:
c0489464: 4b b8 f8 a4 b c0018d08 <__csum_partial>
c04896b0: 4b ff fd b5 bl c0489464 <csum_partial>
c048982c: 4b ff fc 39 bl c0489464 <csum_partial>
c0490158: 4b b8 8b b1 bl c0018d08 <__csum_partial>
c0492f0c <csum_partial>:
c0492f0c: 4b b8 5d fc b c0018d08 <__csum_partial>
c049326c: 4b ff fc a1 bl c0492f0c <csum_partial>
c049333c: 4b ff fb d1 bl c0492f0c <csum_partial>
c0493b18: 4b ff f3 f5 bl c0492f0c <csum_partial>
c0493f50: 4b ff ef bd bl c0492f0c <csum_partial>
c0493ffc: 4b ff ef 11 bl c0492f0c <csum_partial>
c04a0f78: 4b b7 7d 91 bl c0018d08 <__csum_partial>
c04b3e3c: 4b b6 4e cd bl c0018d08 <__csum_partial>
c04b40d0 <csum_partial>:
c04b40d0: 4b b6 4c 38 b c0018d08 <__csum_partial>
c04b4448: 4b ff fc 89 bl c04b40d0 <csum_partial>
c04b46f4: 4b ff f9 dd bl c04b40d0 <csum_partial>
c04bf448: 4b b5 98 c0 b c0018d08 <__csum_partial>
c04c5264: 4b b5 3a a5 bl c0018d08 <__csum_partial>
c04c61e4: 4b b5 2b 25 bl c0018d08 <__csum_partial>
gcc10 defines multiple versions of csum_partial() which are just
an unconditionnal branch to __csum_partial().
To enforce inlining of that branch to __csum_partial(),
mark csum_partial() as __always_inline.
With this patch with gcc10:
c0018d08 <__csum_partial>:
c0019148: 4b ff fb c0 b c0018d08 <__csum_partial>
c001924c: 4b ff fa bd bl c0018d08 <__csum_partial>
c03e40ec: 4b c3 4c 1d bl c0018d08 <__csum_partial>
c03e4120: 4b c3 4b e8 b c0018d08 <__csum_partial>
c03eb004: 4b c2 dd 05 bl c0018d08 <__csum_partial>
c03ecef4: 4b c2 be 15 bl c0018d08 <__csum_partial>
c0427558: 4b bf 17 b1 bl c0018d08 <__csum_partial>
c04286e4: 4b bf 06 25 bl c0018d08 <__csum_partial>
c0428cd8: 4b bf 00 31 bl c0018d08 <__csum_partial>
c0428d84: 4b be ff 85 bl c0018d08 <__csum_partial>
c045a17c: 4b bb eb 8d bl c0018d08 <__csum_partial>
c0489450: 4b b8 f8 b9 bl c0018d08 <__csum_partial>
c0491860: 4b b8 74 a9 bl c0018d08 <__csum_partial>
c0492eec: 4b b8 5e 1d bl c0018d08 <__csum_partial>
c04a0eac: 4b b7 7e 5d bl c0018d08 <__csum_partial>
c04b3e34: 4b b6 4e d5 bl c0018d08 <__csum_partial>
c04b426c: 4b b6 4a 9d bl c0018d08 <__csum_partial>
c04b463c: 4b b6 46 cd bl c0018d08 <__csum_partial>
c04c004c: 4b b5 8c bd bl c0018d08 <__csum_partial>
c04c0368: 4b b5 89 a1 bl c0018d08 <__csum_partial>
c04c5254: 4b b5 3a b5 bl c0018d08 <__csum_partial>
c04c60d4: 4b b5 2c 35 bl c0018d08 <__csum_partial>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/checksum.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/checksum.h b/arch/powerpc/include/asm/checksum.h
index 9cce06194dcc..cab8fec60005 100644
--- a/arch/powerpc/include/asm/checksum.h
+++ b/arch/powerpc/include/asm/checksum.h
@@ -164,7 +164,7 @@ static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
*/
__wsum __csum_partial(const void *buff, int len, __wsum sum);
-static inline __wsum csum_partial(const void *buff, int len, __wsum sum)
+static __always_inline __wsum csum_partial(const void *buff, int len, __wsum sum)
{
if (__builtin_constant_p(len) && len <= 16 && (len & 1) == 0) {
if (len == 2)
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v2] ima: defer arch_ima_get_secureboot() call to IMA init time
From: Chester Lin @ 2020-10-15 12:16 UTC (permalink / raw)
To: Mimi Zohar
Cc: clin, linux-efi, Dmitry Kasatkin, James Morris, jlee,
linux-security-module, linux-integrity,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Ard Biesheuvel,
Serge E. Hallyn
In-Reply-To: <98b04c130708893ebefdf81e127a66356b4a6129.camel@linux.ibm.com>
On Wed, Oct 14, 2020 at 07:38:50AM -0400, Mimi Zohar wrote:
> On Wed, 2020-10-14 at 17:35 +0800, Chester Lin wrote:
> > Hi Ard & Mimi,
> >
> > On Tue, Oct 13, 2020 at 06:59:21PM +0200, Ard Biesheuvel wrote:
> > > On Tue, 13 Oct 2020 at 18:46, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > >
> > > > [Cc'ing linuxppc-dev@lists.ozlabs.org]
> > > >
> > > > On Tue, 2020-10-13 at 10:18 +0200, Ard Biesheuvel wrote:
> > > > > Chester reports that it is necessary to introduce a new way to pass
> > > > > the EFI secure boot status between the EFI stub and the core kernel
> > > > > on ARM systems. The usual way of obtaining this information is by
> > > > > checking the SecureBoot and SetupMode EFI variables, but this can
> > > > > only be done after the EFI variable workqueue is created, which
> > > > > occurs in a subsys_initcall(), whereas arch_ima_get_secureboot()
> > > > > is called much earlier by the IMA framework.
> > > > >
> > > > > However, the IMA framework itself is started as a late_initcall,
> > > > > and the only reason the call to arch_ima_get_secureboot() occurs
> > > > > so early is because it happens in the context of a __setup()
> > > > > callback that parses the ima_appraise= command line parameter.
> > > > >
> > > > > So let's refactor this code a little bit, by using a core_param()
> > > > > callback to capture the command line argument, and deferring any
> > > > > reasoning based on its contents to the IMA init routine.
> > > > >
> > > > > Cc: Chester Lin <clin@suse.com>
> > > > > Cc: Mimi Zohar <zohar@linux.ibm.com>
> > > > > Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> > > > > Cc: James Morris <jmorris@namei.org>
> > > > > Cc: "Serge E. Hallyn" <serge@hallyn.com>
> > > > > Link: https://lore.kernel.org/linux-arm-kernel/20200904072905.25332-2-clin@suse.com/
> > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > ---
> > > > > v2: rebase onto series 'integrity: improve user feedback for invalid bootparams'
> > > >
> > > > Thanks, Ard. Based on my initial, limited testing on Power, it looks
> > > > good, but I'm hesistant to include it in the integrity 5.10 pull
> > > > request without it having been in linux-next and some additional
> > > > testing. It's now queued in the next-integrity-testing branch awaiting
> > > > some tags.
> > > >
> >
> > Tested-by: Chester Lin <clin@suse.com>
> >
> > I have tested this patch on x86 VM.
> >
> > * System configuration:
> > - Platform: QEMU/KVM
> > - Firmware: EDK2/OVMF + secure boot enabled.
> > - OS: SLE15-SP2 + SUSE's kernel-vanilla (=linux v5.9) + the follow commits
> > from linux-next and upstream:
> > * [PATCH v2] ima: defer arch_ima_get_secureboot() call to IMA init time
> > https://www.spinics.net/lists/linux-efi/msg20645.html
> > * e4d7e2df3a09 "ima: limit secure boot feedback scope for appraise"
> > * 7fe2bb7e7e5c "integrity: invalid kernel parameters feedback"
> > * 4afb28ab03d5 "ima: add check for enforced appraise option"
> >
> > * Logs with UEFI secure boot enabled:
> >
> > [ 0.000000] Linux version 5.9.0-858-g865c50e1d279-1.g8764d18-vanilla (geeko@b
> > uildhost) (gcc (SUSE Linux) 10.2.1 20200825 [revision c0746a1beb1ba073c7981eb09f
> > 55b3d993b32e5c], GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.34.0.20200325-1) #
> > 1 SMP Wed Oct 14 04:00:11 UTC 2020 (8764d18)
> > [ 0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.9.0-858-g865c50e1d279-1.
> > g8764d18-vanilla root=UUID=5304c03e-4d8a-4d67-873a-32a32e57cdeb console=ttyS0,11
> > 5200 resume=/dev/disk/by-path/pci-0000:04:00.0-part4 mitigations=auto ignore_log
> > level crashkernel=192M,high crashkernel=72M,low ima_appraise=off
> > [ 0.000000] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point regi
> > sters'
> > [ 0.000000] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
> > [ 0.000000] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'
> > ....
> > ....
> > [ 1.720309] ima: Secure boot enabled: ignoring ima_appraise=off option
> > [ 1.720314] ima: No TPM chip found, activating TPM-bypass!
> > [ 1.722129] ima: Allocated hash algorithm: sha256
>
>
> Thank you for testing the options aren't being set in secure boot mode.
> My main concern, however, is that IMA doesn't go into TPM-bypass mode.
> Does this system have a TPM?
>
> Mimi
>
Last time I didn't emulate a TPM device in KVM so that's why the kernel couldn't
find a TPM chip. I have tested this patch again with a virtual TPM 1.2 by
running swtpm and here are some logs:
-----
[ 0.000000] Linux version 5.9.0-858-g865c50e1d279-1.g8764d18-vanilla (geeko@b
uildhost) (gcc (SUSE Linux) 10.2.1 20200825 [revision c0746a1beb1ba073c7981eb09f
55b3d993b32e5c], GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.34.0.20200325-1) #
1 SMP Wed Oct 14 04:00:11 UTC 2020 (8764d18)
[ 0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.9.0-858-g865c50e1d279-1.
g8764d18-vanilla root=UUID=5304c03e-4d8a-4d67-873a-32a32e57cdeb console=ttyS0,11
5200 resume=/dev/disk/by-path/pci-0000:04:00.0-part4 mitigations=auto ignore_log
level crashkernel=192M,high crashkernel=72M,low ima_appraise=off
[ 0.000000] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point regi
sters'
.....
.....
[ 0.000000] efi: EFI v2.60 by EDK II
[ 0.000000] efi: SMBIOS=0x7fe82000 ACPI=0x7feb0000 ACPI 2.0=0x7feb0014 MEMATTT
R=0x7ef89698 RNG=0x7ea12e18
[ 0.000000] efi: seeding entropy pool
[ 0.000000] random: fast init done
[ 0.000000] SMBIOS 2.8 present.
[ 0.000000] DMI: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
[ 0.000000] Hypervisor detected: KVM
.....
.....
[ 1.957625] Linux agpgart interface v0.103
[ 1.961903] tpm_tis 00:04: 1.2 TPM (device-id 0x1, rev-id 1)
[ 1.966306] tpm tpm0: starting up the TPM manually
[ 1.981753] ahci 0000:00:1f.2: version 3.0
[ 1.984503] PCI Interrupt Link [GSIA] enabled at IRQ 16
.....
.....
[ 2.086645] ima: Secure boot enabled: ignoring ima_appraise=off option
[ 2.086650] ima: Allocated hash algorithm: sha256
[ 2.100662] evm: Initialising EVM extended attributes:
[ 2.100689] audit: type=1807 audit(1602760724.564:2): action=measure func=KEX
EC_KERNEL_CHECK res=1
[ 2.101849] evm: security.selinux
[ 2.101849] evm: security.apparmor
[ 2.101849] evm: security.ima
[ 2.101850] evm: security.capability
[ 2.101850] evm: HMAC attrs: 0x1
[ 2.102083] PM: Magic number: 8:588:327
[ 2.106723] audit: type=1807 audit(1602760724.564:3): action=measure func=MODD
ULE_CHECK res=1
[ 2.118761] RAS: Correctable Errors collector initialized.
.....
-----
In terms of TPM 2.0, swtpm seems to have issues while running the selftest so
I verified this patch on my laptop as well because it has a real TPM 2.0 chip:
-----
[ 0.000000] microcode: microcode updated early to revision 0xd6, date = 2020-
04-27
[ 0.000000] Linux version 5.9.0-858-g865c50e1d279-1.g8764d18-vanilla (geeko@b
uildhost) (gcc (SUSE Linux) 10.2.1 20200825 [revision c0746a1beb1ba073c7981eb09f
55b3d993b32e5c], GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.34.0.20200325-1) #
1 SMP Wed Oct 1404:00:11 UTC 2020 (8764d18)
[ 0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.9.0-858-g865c50e1d279-1.
g8764d18-vanilla root=UUID=2184bbb7-780e-48a2-98b8-3a8cd3366e5e splash=silent re
sume=/dev/disk/by-id/ata-INTEL_SSDSCKKF512G8_SATA_512GB_BTLA82850G3E512K-part3 i
gnore_loglevel crashkernel=222M,high crashkernel=72M,low ima_appraise=log
[ 0.000000] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point regi
sters'
.....
.....
[ 0.000000] efi: EFI v2.60 by American Megatrends
[ 0.000000] efi: ACPI=0x7a34e000 ACPI 2.0=0x7a34e000 SMBIOS=0xf0000 SMBIOS 3.
0=0xf0020 TPMFinalLog=0x7ac8b000 ESRT=0x7b0e0018 MEMATTR=0x7762d018 RNG=0x7b133f
18 TPMEventLog=0x6ab96018
[ 0.000000] efi: seeding entropy pool
[ 0.000000] random: fast init done
[ 0.000000] SMBIOS 3.1.0 present.
[ 0.000000] DMI: Dell Inc. Latitude 7490/0KP0FT, BIOS 1.14.0 01/22/2020
.....
.....
[ 7.182711] Non-volatile memory driver v1.3
[ 7.182748] Linux agpgart interface v0.103
[ 7.210826] tpm_tis MSFT0101:00: 2.0 TPM (device-id 0xFC, rev-id 1)
[ 7.234324] ahci 0000:00:17.0: version 3.0
.....
.....
[ 7.288573] integrity: Loading X.509 certificate: UEFI:MokListRT
[ 7.289299] integrity: Loaded X.509 cert 'openSUSE Secure Boot CA: 6842600de22c4c477e95be23dfea9513e5971762'
[ 7.289913] ima: Secure boot enabled: ignoring ima_appraise=log option
[ 7.289921] ima: Allocated hash algorithm: sha256
[ 7.314854] evm: Initialising EVM extended attributes:
[ 7.314878] audit: type=1807 audit(1602778176.572:2): action=measure func=KEXEC_KERNEL_CHECK res=1
[ 7.316656] evm: security.selinux
[ 7.316659] evm: security.apparmor
[ 7.318493] audit: type=1807 audit(1602778176.572:3): action=measure func=MODULE_CHECK res=1
[ 7.320092] evm: security.ima
[ 7.320093] evm: security.capability
[ 7.322554] evm: HMAC attrs: 0x1
^ permalink raw reply
* Re: [PATCH] powerpc: force inlining of csum_partial() to avoid multiple csum_partial() with GCC10
From: Segher Boessenkool @ 2020-10-15 13:25 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <a1d31f84ddb0926813b17fcd5cc7f3fa7b4deac2.1602759123.git.christophe.leroy@csgroup.eu>
Hi!
On Thu, Oct 15, 2020 at 10:52:20AM +0000, Christophe Leroy wrote:
> With gcc9 I get:
<snip>
> With gcc10 I get:
<snip>
> gcc10 defines multiple versions of csum_partial() which are just
> an unconditionnal branch to __csum_partial().
It doesn't inline it, yes.
Could you open a GCC PR for this please?
> To enforce inlining of that branch to __csum_partial(),
> mark csum_partial() as __always_inline.
That should be fine of course, but it would be good if we could fix this
in the compiler :-)
Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>
Segher
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox