| Message ID | 20240825-02-k1-pinctrl-v2-0-ddd38a345d12@gentoo.org |
|---|---|
| Headers | show |
| Series | riscv: spacemit: add pinctrl support to K1 SoC | expand |
On 25/08/2024 15:10, Yixun Lan wrote: > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC. Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters It's "dt-bindings:" > > Two vendor specific properties are introduced here, As the pinctrl > has dedicated slew rate enable control - bit[7], so we have > spacemit,slew-rate-{enable,disable} for this. For the same reason, > creating spacemit,strong-pull-up for the strong pull up control. Huh, no, use generic properties. More on that below > > Signed-off-by: Yixun Lan <dlan@gentoo.org> > --- > .../bindings/pinctrl/spacemit,k1-pinctrl.yaml | 134 +++++++++++++++++ > include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h | 161 +++++++++++++++++++++ > 2 files changed, 295 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml > new file mode 100644 > index 0000000000000..8adfc5ebbce37 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml > @@ -0,0 +1,134 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: SpacemiT K1 SoC Pin Controller > + > +maintainers: > + - Yixun Lan <dlan@gentoo.org> > + > +properties: > + compatible: > + const: spacemit,k1-pinctrl > + > + reg: > + items: > + - description: pinctrl io memory base > + > +patternProperties: > + '-cfg$': > + type: object > + description: | Do not need '|' unless you need to preserve formatting. > + A pinctrl node should contain at least one subnode representing the > + pinctrl groups available on the machine. > + > + additionalProperties: false Keep it before description. > + > + patternProperties: > + '-pins$': > + type: object > + description: | > + Each subnode will list the pins it needs, and how they should > + be configured, with regard to muxer configuration, bias, input > + enable/disable, input schmitt trigger, slew-rate enable/disable, > + slew-rate, drive strength, power source. > + $ref: /schemas/pinctrl/pincfg-node.yaml > + > + allOf: > + - $ref: pincfg-node.yaml# > + - $ref: pinmux-node.yaml# You are duplicating refs. > + > + properties: > + pinmux: > + description: | > + The list of GPIOs and their mux settings that properties in the > + node apply to. This should be set using the K1_PADCONF macro to > + construct the value. > + $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux Hm why you need the ref? > + > + bias-disable: true > + > + bias-pull-up: true > + > + bias-pull-down: true > + > + drive-strength-microamp: > + description: | > + typical current when output high level, but in mA. > + 1.8V output: 11, 21, 32, 42 (mA) > + 3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA) > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + input-schmitt: > + description: | > + typical threshold for schmitt trigger. > + 0: buffer mode > + 1: trigger mode > + 2, 3: trigger mode > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3] > + > + power-source: > + description: external power supplies at 1.8v or 3.3v. > + enum: [ 1800, 3300 ] > + > + slew-rate: > + description: | > + slew rate for output buffer > + 0, 1: Slow speed Hm? Surprising, 0 is slow speed? > + 2: Medium speed > + 3: Fast speed > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3] > + > + spacemit,slew-rate-enable: > + description: enable slew rate. The presence of slew-rate enables it, doesn't it? > + type: boolean > + > + spacemit,slew-rate-disable: > + description: disable slew rate. > + type: boolean Just use slew-rate, 0 disable, some value to match real slew-rate. > + > + spacemit,strong-pull-up: > + description: enable strong pull up. Do not duplicate the property name in description. You did not say anything useful here. What is "strong"? bias-pull-up takes also an argument. > + type: boolean > + > + required: > + - pinmux > + > + additionalProperties: false This goes up, before description. > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + pinctrl@d401e000 { > + compatible = "spacemit,k1-pinctrl"; > + reg = <0x0 0xd401e000 0x0 0x400>; > + #pinctrl-cells = <2>; > + #gpio-range-cells = <3>; This wasn't ever tested... :( ... > diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h > new file mode 100644 > index 0000000000000..13ef4aa6c53a3 > --- /dev/null > +++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h > @@ -0,0 +1,161 @@ > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */ > +/* > + * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd > + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> > + * > + */ > + > +#ifndef _DT_BINDINGS_PINCTRL_K1_H > +#define _DT_BINDINGS_PINCTRL_K1_H > + > +#define PINMUX(pin, mux) \ > + (((pin) & 0xffff) | (((mux) & 0xff) << 16)) > + > +/* pin offset */ > +#define PINID(x) ((x) + 1) > + > +#define GPIO_INVAL 0 > +#define GPIO_00 PINID(0) Not really, pin numbers are not bindings. Drop entire header. ... > + > +#define SLEW_RATE_SLOW0 0 > +#define SLEW_RATE_SLOW1 1 > +#define SLEW_RATE_MEDIUM 2 > +#define SLEW_RATE_FAST 3 Not a binding, either. No usage in the driver. > + > +#define K1_PADCONF(pin, func) (((pin) << 16) | (func)) Not a binding. Best regards, Krzysztof
On Sun, 25 Aug 2024 13:10:02 +0000, Yixun Lan wrote: > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC. > > Two vendor specific properties are introduced here, As the pinctrl > has dedicated slew rate enable control - bit[7], so we have > spacemit,slew-rate-{enable,disable} for this. For the same reason, > creating spacemit,strong-pull-up for the strong pull up control. > > Signed-off-by: Yixun Lan <dlan@gentoo.org> > --- > .../bindings/pinctrl/spacemit,k1-pinctrl.yaml | 134 +++++++++++++++++ > include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h | 161 +++++++++++++++++++++ > 2 files changed, 295 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml: patternProperties:-cfg$:patternProperties:-pins$:properties:drive-strength-microamp: '$ref' should not be valid under {'const': '$ref'} hint: Standard unit suffix properties don't need a type $ref from schema $id: http://devicetree.org/meta-schemas/core.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.example.dtb: dsi-phy@10215000: drive-strength-microamp: 4000 is not of type 'array' from schema $id: http://devicetree.org/schemas/phy/mediatek,dsi-phy.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.example.dtb: dsi-phy@10215000: drive-strength-microamp: 4000 is not of type 'array' from schema $id: http://devicetree.org/schemas/property-units.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8183-pinctrl.example.dtb: pinctrl@10005000: i2c0-pins:pins1:drive-strength-microamp: 1000 is not of type 'array' from schema $id: http://devicetree.org/schemas/pinctrl/mediatek,mt8183-pinctrl.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8183-pinctrl.example.dtb: pins1: drive-strength-microamp: 1000 is not of type 'array' from schema $id: http://devicetree.org/schemas/property-units.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8186-pinctrl.example.dtb: pinctrl@10005000: i2c0-pins:pins:drive-strength-microamp: 1000 is not of type 'array' from schema $id: http://devicetree.org/schemas/pinctrl/mediatek,mt8186-pinctrl.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8186-pinctrl.example.dtb: pins: drive-strength-microamp: 1000 is not of type 'array' from schema $id: http://devicetree.org/schemas/property-units.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8195-pinctrl.example.dtb: pinctrl@10005000: i2c0-pins:pins:drive-strength-microamp: 1000 is not of type 'array' from schema $id: http://devicetree.org/schemas/pinctrl/mediatek,mt8195-pinctrl.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8195-pinctrl.example.dtb: pins: drive-strength-microamp: 1000 is not of type 'array' from schema $id: http://devicetree.org/schemas/property-units.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8188-pinctrl.example.dtb: pinctrl@10005000: i2c0-pins:pins:drive-strength-microamp: 1000 is not of type 'array' from schema $id: http://devicetree.org/schemas/pinctrl/mediatek,mt8188-pinctrl.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8188-pinctrl.example.dtb: pins: drive-strength-microamp: 1000 is not of type 'array' from schema $id: http://devicetree.org/schemas/property-units.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.example.dtb: pinctrl@d401e000: '#gpio-range-cells', '#pinctrl-cells' do not match any of the regexes: '-cfg$', 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.example.dtb: uart0-2-pins: drive-strength-microamp: 32 is not of type 'array' from schema $id: http://devicetree.org/schemas/property-units.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240825-02-k1-pinctrl-v2-1-ddd38a345d12@gentoo.org The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Hi Krzysztof: On 15:48 Sun 25 Aug , Krzysztof Kozlowski wrote: > On 25/08/2024 15:10, Yixun Lan wrote: > > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC. > > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. For bindings, the preferred subjects are > explained here: > https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters > > It's "dt-bindings:" Ok, will fix in next version > > > > > Two vendor specific properties are introduced here, As the pinctrl > > has dedicated slew rate enable control - bit[7], so we have > > spacemit,slew-rate-{enable,disable} for this. For the same reason, > > creating spacemit,strong-pull-up for the strong pull up control. > > Huh, no, use generic properties. More on that below > see my reply below > > > > > > Signed-off-by: Yixun Lan <dlan@gentoo.org> > > --- > > .../bindings/pinctrl/spacemit,k1-pinctrl.yaml | 134 +++++++++++++++++ > > include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h | 161 +++++++++++++++++++++ > > 2 files changed, 295 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml > > new file mode 100644 > > index 0000000000000..8adfc5ebbce37 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml > > @@ -0,0 +1,134 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: SpacemiT K1 SoC Pin Controller > > + > > +maintainers: > > + - Yixun Lan <dlan@gentoo.org> > > + > > +properties: > > + compatible: > > + const: spacemit,k1-pinctrl > > + > > + reg: > > + items: > > + - description: pinctrl io memory base > > + > > +patternProperties: > > + '-cfg$': > > + type: object > > + description: | > > Do not need '|' unless you need to preserve formatting. > Ok > > + A pinctrl node should contain at least one subnode representing the > > + pinctrl groups available on the machine. > > + > > + additionalProperties: false > > Keep it before description. Ok > > > + > > + patternProperties: > > + '-pins$': > > + type: object > > + description: | > > + Each subnode will list the pins it needs, and how they should > > + be configured, with regard to muxer configuration, bias, input > > + enable/disable, input schmitt trigger, slew-rate enable/disable, > > + slew-rate, drive strength, power source. > > + $ref: /schemas/pinctrl/pincfg-node.yaml > > + > > + allOf: > > + - $ref: pincfg-node.yaml# > > + - $ref: pinmux-node.yaml# > > You are duplicating refs. ok, will fix it > > > + > > + properties: > > + pinmux: > > + description: | > > + The list of GPIOs and their mux settings that properties in the > > + node apply to. This should be set using the K1_PADCONF macro to > > + construct the value. > > + $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux > > Hm why you need the ref? > will drop it > > + > > + bias-disable: true > > + > > + bias-pull-up: true > > + > > + bias-pull-down: true > > + > > + drive-strength-microamp: > > + description: | > > + typical current when output high level, but in mA. > > + 1.8V output: 11, 21, 32, 42 (mA) > > + 3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA) > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + > > + input-schmitt: > > + description: | > > + typical threshold for schmitt trigger. > > + 0: buffer mode > > + 1: trigger mode > > + 2, 3: trigger mode > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1, 2, 3] > > + > > + power-source: > > + description: external power supplies at 1.8v or 3.3v. > > + enum: [ 1800, 3300 ] > > + > > + slew-rate: > > + description: | > > + slew rate for output buffer > > + 0, 1: Slow speed > > Hm? Surprising, 0 is slow speed? > from docs, section 3.3.2.2 MFPR Register Description 0, 1 are same, both for slow speed https://developer.spacemit.com/documentation?token=An1vwTwKaigaXRkYfwmcznTXned > > + 2: Medium speed > > + 3: Fast speed > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1, 2, 3] > > + > > + spacemit,slew-rate-enable: > > + description: enable slew rate. > > The presence of slew-rate enables it, doesn't it? > yes, this should work, I will take this approach, thanks > > + type: boolean > > + > > + spacemit,slew-rate-disable: > > + description: disable slew rate. > > + type: boolean > > Just use slew-rate, 0 disable, some value to match real slew-rate. > sounds good to me, since 0, 1 indicate same meaning, can re-use 0 for disabling slew rate. > > + > > + spacemit,strong-pull-up: > > + description: enable strong pull up. > > Do not duplicate the property name in description. You did not say > anything useful here. What is "strong"? bias-pull-up takes also an argument. > there is a dedicated strong pull bit[3] for I2C, SD card kinds of pad I don't know how 'strong' it is if in ohms, will see if can get more info on this (may expand the description) I think using 'bias-pull-up' property with argument should also work, but it occur to me it's more intuitive to introduce a property here, which reflect the underlying hardware functionality. this is similar to starfive's jh7100 bindings/pinctrl/starfive,jh7100-pinctrl.yaml:154 (refer to exist code doesn't mean always correct, so I need advice here) I will keep this property unless there is objection, please let me know > > + type: boolean > > + > > + required: > > + - pinmux > > + > > + additionalProperties: false > > This goes up, before description. > Ok > > + > > +required: > > + - compatible > > + - reg > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h> > > + > > + soc { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + pinctrl@d401e000 { > > + compatible = "spacemit,k1-pinctrl"; > > + reg = <0x0 0xd401e000 0x0 0x400>; > > + #pinctrl-cells = <2>; > > + #gpio-range-cells = <3>; > > This wasn't ever tested... :( > ... will drop it > > > diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h > > new file mode 100644 > > index 0000000000000..13ef4aa6c53a3 > > --- /dev/null > > +++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h > > @@ -0,0 +1,161 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */ > > +/* > > + * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd > > + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> > > + * > > + */ > > + > > +#ifndef _DT_BINDINGS_PINCTRL_K1_H > > +#define _DT_BINDINGS_PINCTRL_K1_H > > + > > +#define PINMUX(pin, mux) \ > > + (((pin) & 0xffff) | (((mux) & 0xff) << 16)) > > + > > +/* pin offset */ > > +#define PINID(x) ((x) + 1) > > + > > +#define GPIO_INVAL 0 > > +#define GPIO_00 PINID(0) > > Not really, pin numbers are not bindings. Drop entire header. > Ok, I will move them to dts folder, which should be file arch/riscv/boot/dts/spacemit/k1-pinctrl.h > ... > > > + > > +#define SLEW_RATE_SLOW0 0 > > +#define SLEW_RATE_SLOW1 1 > > +#define SLEW_RATE_MEDIUM 2 > > +#define SLEW_RATE_FAST 3 > > Not a binding, either. No usage in the driver. Ok, will drop it > > > + > > +#define K1_PADCONF(pin, func) (((pin) << 16) | (func)) > > Not a binding. > same, move to dts > > > Best regards, > Krzysztof
On 13:10 Sun 25 Aug , Yixun Lan wrote: > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC. > > Two vendor specific properties are introduced here, As the pinctrl > has dedicated slew rate enable control - bit[7], so we have > spacemit,slew-rate-{enable,disable} for this. For the same reason, > creating spacemit,strong-pull-up for the strong pull up control. > > Signed-off-by: Yixun Lan <dlan@gentoo.org> > --- > .../bindings/pinctrl/spacemit,k1-pinctrl.yaml | 134 +++++++++++++++++ > include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h | 161 +++++++++++++++++++++ > 2 files changed, 295 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml > new file mode 100644 > index 0000000000000..8adfc5ebbce37 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml > @@ -0,0 +1,134 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: SpacemiT K1 SoC Pin Controller > + > +maintainers: > + - Yixun Lan <dlan@gentoo.org> > + > +properties: > + compatible: > + const: spacemit,k1-pinctrl > + > + reg: > + items: > + - description: pinctrl io memory base > + > +patternProperties: > + '-cfg$': > + type: object > + description: | > + A pinctrl node should contain at least one subnode representing the > + pinctrl groups available on the machine. > + > + additionalProperties: false > + > + patternProperties: > + '-pins$': > + type: object > + description: | > + Each subnode will list the pins it needs, and how they should > + be configured, with regard to muxer configuration, bias, input > + enable/disable, input schmitt trigger, slew-rate enable/disable, > + slew-rate, drive strength, power source. > + $ref: /schemas/pinctrl/pincfg-node.yaml > + > + allOf: > + - $ref: pincfg-node.yaml# > + - $ref: pinmux-node.yaml# > + > + properties: > + pinmux: > + description: | > + The list of GPIOs and their mux settings that properties in the > + node apply to. This should be set using the K1_PADCONF macro to > + construct the value. > + $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux > + > + bias-disable: true > + > + bias-pull-up: true > + > + bias-pull-down: true > + > + drive-strength-microamp: just realise here should use 'drive-strength' which will comply to hw will fix in next version > + description: | > + typical current when output high level, but in mA. > + 1.8V output: 11, 21, 32, 42 (mA) > + 3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA) > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + input-schmitt: > + description: | > + typical threshold for schmitt trigger. > + 0: buffer mode > + 1: trigger mode > + 2, 3: trigger mode > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3] > + > + power-source: > + description: external power supplies at 1.8v or 3.3v. > + enum: [ 1800, 3300 ] > + > + slew-rate: > + description: | > + slew rate for output buffer > + 0, 1: Slow speed > + 2: Medium speed > + 3: Fast speed > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3] > + > + spacemit,slew-rate-enable: > + description: enable slew rate. > + type: boolean > + > + spacemit,slew-rate-disable: > + description: disable slew rate. > + type: boolean > + > + spacemit,strong-pull-up: > + description: enable strong pull up. > + type: boolean > + > + required: > + - pinmux > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + pinctrl@d401e000 { > + compatible = "spacemit,k1-pinctrl"; > + reg = <0x0 0xd401e000 0x0 0x400>; > + #pinctrl-cells = <2>; > + #gpio-range-cells = <3>; > + > + uart0_2_cfg: uart0-2-cfg { > + uart0-2-pins { > + pinmux = <K1_PADCONF(GPIO_68, 2)>, > + <K1_PADCONF(GPIO_69, 2)>; > + bias-pull-up; > + drive-strength-microamp = <32>; > + }; > + }; > + }; > + }; > + > +... > diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h > new file mode 100644 > index 0000000000000..13ef4aa6c53a3 > --- /dev/null > +++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h > @@ -0,0 +1,161 @@ > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */ > +/* > + * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd > + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> > + * > + */ > + > +#ifndef _DT_BINDINGS_PINCTRL_K1_H > +#define _DT_BINDINGS_PINCTRL_K1_H > + > +#define PINMUX(pin, mux) \ > + (((pin) & 0xffff) | (((mux) & 0xff) << 16)) > + > +/* pin offset */ > +#define PINID(x) ((x) + 1) > + > +#define GPIO_INVAL 0 > +#define GPIO_00 PINID(0) > +#define GPIO_01 PINID(1) > +#define GPIO_02 PINID(2) > +#define GPIO_03 PINID(3) > +#define GPIO_04 PINID(4) > +#define GPIO_05 PINID(5) > +#define GPIO_06 PINID(6) > +#define GPIO_07 PINID(7) > +#define GPIO_08 PINID(8) > +#define GPIO_09 PINID(9) > +#define GPIO_10 PINID(10) > +#define GPIO_11 PINID(11) > +#define GPIO_12 PINID(12) > +#define GPIO_13 PINID(13) > +#define GPIO_14 PINID(14) > +#define GPIO_15 PINID(15) > +#define GPIO_16 PINID(16) > +#define GPIO_17 PINID(17) > +#define GPIO_18 PINID(18) > +#define GPIO_19 PINID(19) > +#define GPIO_20 PINID(20) > +#define GPIO_21 PINID(21) > +#define GPIO_22 PINID(22) > +#define GPIO_23 PINID(23) > +#define GPIO_24 PINID(24) > +#define GPIO_25 PINID(25) > +#define GPIO_26 PINID(26) > +#define GPIO_27 PINID(27) > +#define GPIO_28 PINID(28) > +#define GPIO_29 PINID(29) > +#define GPIO_30 PINID(30) > +#define GPIO_31 PINID(31) > + > +#define GPIO_32 PINID(32) > +#define GPIO_33 PINID(33) > +#define GPIO_34 PINID(34) > +#define GPIO_35 PINID(35) > +#define GPIO_36 PINID(36) > +#define GPIO_37 PINID(37) > +#define GPIO_38 PINID(38) > +#define GPIO_39 PINID(39) > +#define GPIO_40 PINID(40) > +#define GPIO_41 PINID(41) > +#define GPIO_42 PINID(42) > +#define GPIO_43 PINID(43) > +#define GPIO_44 PINID(44) > +#define GPIO_45 PINID(45) > +#define GPIO_46 PINID(46) > +#define GPIO_47 PINID(47) > +#define GPIO_48 PINID(48) > +#define GPIO_49 PINID(49) > +#define GPIO_50 PINID(50) > +#define GPIO_51 PINID(51) > +#define GPIO_52 PINID(52) > +#define GPIO_53 PINID(53) > +#define GPIO_54 PINID(54) > +#define GPIO_55 PINID(55) > +#define GPIO_56 PINID(56) > +#define GPIO_57 PINID(57) > +#define GPIO_58 PINID(58) > +#define GPIO_59 PINID(59) > +#define GPIO_60 PINID(60) > +#define GPIO_61 PINID(61) > +#define GPIO_62 PINID(62) > +#define GPIO_63 PINID(63) > + > +#define GPIO_64 PINID(64) > +#define GPIO_65 PINID(65) > +#define GPIO_66 PINID(66) > +#define GPIO_67 PINID(67) > +#define GPIO_68 PINID(68) > +#define GPIO_69 PINID(69) > +#define GPIO_70 PINID(70) > +#define GPIO_71 PINID(71) > +#define GPIO_72 PINID(72) > +#define GPIO_73 PINID(73) > +#define GPIO_74 PINID(74) > +#define GPIO_75 PINID(75) > +#define GPIO_76 PINID(76) > +#define GPIO_77 PINID(77) > +#define GPIO_78 PINID(78) > +#define GPIO_79 PINID(79) > +#define GPIO_80 PINID(80) > +#define GPIO_81 PINID(81) > +#define GPIO_82 PINID(82) > +#define GPIO_83 PINID(83) > +#define GPIO_84 PINID(84) > +#define GPIO_85 PINID(85) > + > +#define GPIO_101 PINID(89) > +#define GPIO_100 PINID(90) > +#define GPIO_99 PINID(91) > +#define GPIO_98 PINID(92) > +#define GPIO_103 PINID(93) > +#define GPIO_102 PINID(94) > + > +#define GPIO_104 PINID(109) > +#define GPIO_105 PINID(110) > +#define GPIO_106 PINID(111) > +#define GPIO_107 PINID(112) > +#define GPIO_108 PINID(113) > +#define GPIO_109 PINID(114) > +#define GPIO_110 PINID(115) > + > +#define GPIO_93 PINID(116) > +#define GPIO_94 PINID(117) > +#define GPIO_95 PINID(118) > +#define GPIO_96 PINID(119) > +#define GPIO_97 PINID(120) > + > +#define GPIO_86 PINID(122) > +#define GPIO_87 PINID(123) > +#define GPIO_88 PINID(124) > +#define GPIO_89 PINID(125) > +#define GPIO_90 PINID(126) > +#define GPIO_91 PINID(127) > +#define GPIO_92 PINID(128) > + > +#define GPIO_111 PINID(130) > +#define GPIO_112 PINID(131) > +#define GPIO_113 PINID(132) > +#define GPIO_114 PINID(133) > +#define GPIO_115 PINID(134) > +#define GPIO_116 PINID(135) > +#define GPIO_117 PINID(136) > +#define GPIO_118 PINID(137) > +#define GPIO_119 PINID(138) > +#define GPIO_120 PINID(139) > +#define GPIO_121 PINID(140) > +#define GPIO_122 PINID(141) > +#define GPIO_123 PINID(142) > +#define GPIO_124 PINID(143) > +#define GPIO_125 PINID(144) > +#define GPIO_126 PINID(145) > +#define GPIO_127 PINID(146) > + > +#define SLEW_RATE_SLOW0 0 > +#define SLEW_RATE_SLOW1 1 > +#define SLEW_RATE_MEDIUM 2 > +#define SLEW_RATE_FAST 3 > + > +#define K1_PADCONF(pin, func) (((pin) << 16) | (func)) > + > +#endif /* _DT_BINDINGS_PINCTRL_K1_H */ > > -- > 2.45.2
On Mon, Aug 26, 2024 at 01:36:35AM GMT, Yixun Lan wrote: > Hi Krzysztof: > > On 15:48 Sun 25 Aug , Krzysztof Kozlowski wrote: > > On 25/08/2024 15:10, Yixun Lan wrote: > > > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC. > > > > > > Please use subject prefixes matching the subsystem. You can get them for > > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > > your patch is touching. For bindings, the preferred subjects are > > explained here: > > https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters > > > > It's "dt-bindings:" > Ok, will fix in next version > > > > > > > > > Two vendor specific properties are introduced here, As the pinctrl > > > has dedicated slew rate enable control - bit[7], so we have > > > spacemit,slew-rate-{enable,disable} for this. For the same reason, > > > creating spacemit,strong-pull-up for the strong pull up control. > > > > Huh, no, use generic properties. More on that below > > > see my reply below > > > > > > > > > > > Signed-off-by: Yixun Lan <dlan@gentoo.org> > > > --- > > > .../bindings/pinctrl/spacemit,k1-pinctrl.yaml | 134 +++++++++++++++++ > > > include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h | 161 +++++++++++++++++++++ > > > 2 files changed, 295 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml > > > new file mode 100644 > > > index 0000000000000..8adfc5ebbce37 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml > > > @@ -0,0 +1,134 @@ > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: SpacemiT K1 SoC Pin Controller > > > + > > > +maintainers: > > > + - Yixun Lan <dlan@gentoo.org> > > > + > > > +properties: > > > + compatible: > > > + const: spacemit,k1-pinctrl > > > + > > > + reg: > > > + items: > > > + - description: pinctrl io memory base > > > + > > > +patternProperties: > > > + '-cfg$': > > > + type: object > > > + description: | > > > > Do not need '|' unless you need to preserve formatting. > > > Ok > > > + A pinctrl node should contain at least one subnode representing the > > > + pinctrl groups available on the machine. > > > + > > > + additionalProperties: false > > > > Keep it before description. > Ok > > > > > + > > > + patternProperties: > > > + '-pins$': > > > + type: object > > > + description: | > > > + Each subnode will list the pins it needs, and how they should > > > + be configured, with regard to muxer configuration, bias, input > > > + enable/disable, input schmitt trigger, slew-rate enable/disable, > > > + slew-rate, drive strength, power source. > > > + $ref: /schemas/pinctrl/pincfg-node.yaml > > > + > > > + allOf: > > > + - $ref: pincfg-node.yaml# > > > + - $ref: pinmux-node.yaml# > > > > You are duplicating refs. > ok, will fix it > > > > > + > > > + properties: > > > + pinmux: > > > + description: | > > > + The list of GPIOs and their mux settings that properties in the > > > + node apply to. This should be set using the K1_PADCONF macro to > > > + construct the value. > > > + $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux > > > > Hm why you need the ref? > > > will drop it > > > + > > > + bias-disable: true > > > + > > > + bias-pull-up: true > > > + > > > + bias-pull-down: true > > > + > > > + drive-strength-microamp: > > > + description: | > > > + typical current when output high level, but in mA. > > > + 1.8V output: 11, 21, 32, 42 (mA) > > > + 3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA) > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + > > > + input-schmitt: > > > + description: | > > > + typical threshold for schmitt trigger. > > > + 0: buffer mode > > > + 1: trigger mode > > > + 2, 3: trigger mode > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + enum: [0, 1, 2, 3] > > > + > > > + power-source: > > > + description: external power supplies at 1.8v or 3.3v. > > > + enum: [ 1800, 3300 ] > > > + > > > + slew-rate: > > > + description: | > > > + slew rate for output buffer > > > + 0, 1: Slow speed > > > > Hm? Surprising, 0 is slow speed? > > > from docs, section 3.3.2.2 MFPR Register Description > 0, 1 are same, both for slow speed > https://developer.spacemit.com/documentation?token=An1vwTwKaigaXRkYfwmcznTXned > I suspect this should not be set separately, but with the drive-strength. The document shows that the DS field sets both drive strength and slew rate. This at least tell the value 0 and 1 may be different. > > > + 2: Medium speed > > > + 3: Fast speed > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + enum: [0, 1, 2, 3] > > > + > > > + spacemit,slew-rate-enable: > > > + description: enable slew rate. > > > > The presence of slew-rate enables it, doesn't it? > > > yes, this should work, I will take this approach, thanks > > > > + type: boolean > > > + > > > + spacemit,slew-rate-disable: > > > + description: disable slew rate. > > > + type: boolean > > > > Just use slew-rate, 0 disable, some value to match real slew-rate. > > > sounds good to me, since 0, 1 indicate same meaning, can re-use 0 for > disabling slew rate. > > > > + > > > + spacemit,strong-pull-up: > > > + description: enable strong pull up. > > > > Do not duplicate the property name in description. You did not say > > anything useful here. What is "strong"? bias-pull-up takes also an argument. > > > there is a dedicated strong pull bit[3] for I2C, SD card kinds of pad > I don't know how 'strong' it is if in ohms, will see if can get > more info on this (may expand the description) > > I think using 'bias-pull-up' property with argument should also work, > but it occur to me it's more intuitive to introduce a property here, which > reflect the underlying hardware functionality. this is similar to starfive's jh7100 > bindings/pinctrl/starfive,jh7100-pinctrl.yaml:154 > (refer to exist code doesn't mean always correct, so I need advice here) > > I will keep this property unless there is objection, please let me know > > > > + type: boolean > > > + > > > + required: > > > + - pinmux > > > + > > > + additionalProperties: false > > > > This goes up, before description. > > > Ok > > > + > > > +required: > > > + - compatible > > > + - reg > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h> > > > + > > > + soc { > > > + #address-cells = <2>; > > > + #size-cells = <2>; > > > + > > > + pinctrl@d401e000 { > > > + compatible = "spacemit,k1-pinctrl"; > > > + reg = <0x0 0xd401e000 0x0 0x400>; > > > + #pinctrl-cells = <2>; > > > + #gpio-range-cells = <3>; > > > > This wasn't ever tested... :( > > ... > will drop it > > > > > diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h > > > new file mode 100644 > > > index 0000000000000..13ef4aa6c53a3 > > > --- /dev/null > > > +++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h > > > @@ -0,0 +1,161 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */ > > > +/* > > > + * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd > > > + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> > > > + * > > > + */ > > > + > > > +#ifndef _DT_BINDINGS_PINCTRL_K1_H > > > +#define _DT_BINDINGS_PINCTRL_K1_H > > > + > > > +#define PINMUX(pin, mux) \ > > > + (((pin) & 0xffff) | (((mux) & 0xff) << 16)) > > > + > > > +/* pin offset */ > > > +#define PINID(x) ((x) + 1) > > > + > > > +#define GPIO_INVAL 0 > > > +#define GPIO_00 PINID(0) > > > > Not really, pin numbers are not bindings. Drop entire header. > > > Ok, I will move them to dts folder, which should be file > arch/riscv/boot/dts/spacemit/k1-pinctrl.h > > > ... > > > > > + > > > +#define SLEW_RATE_SLOW0 0 > > > +#define SLEW_RATE_SLOW1 1 > > > +#define SLEW_RATE_MEDIUM 2 > > > +#define SLEW_RATE_FAST 3 > > > > Not a binding, either. No usage in the driver. > Ok, will drop it > > > > > > + > > > +#define K1_PADCONF(pin, func) (((pin) << 16) | (func)) > > > > Not a binding. > > > same, move to dts > > > > > > > Best regards, > > Krzysztof > > -- > Yixun Lan (dlan) > Gentoo Linux Developer > GPG Key ID AABEFD55
On Sun, Aug 25, 2024 at 01:10:02PM GMT, Yixun Lan wrote: > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC. > > Two vendor specific properties are introduced here, As the pinctrl > has dedicated slew rate enable control - bit[7], so we have > spacemit,slew-rate-{enable,disable} for this. For the same reason, > creating spacemit,strong-pull-up for the strong pull up control. > > Signed-off-by: Yixun Lan <dlan@gentoo.org> > --- > .../bindings/pinctrl/spacemit,k1-pinctrl.yaml | 134 +++++++++++++++++ > include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h | 161 +++++++++++++++++++++ > 2 files changed, 295 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml > new file mode 100644 > index 0000000000000..8adfc5ebbce37 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml > @@ -0,0 +1,134 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: SpacemiT K1 SoC Pin Controller > + > +maintainers: > + - Yixun Lan <dlan@gentoo.org> > + > +properties: > + compatible: > + const: spacemit,k1-pinctrl > + > + reg: > + items: > + - description: pinctrl io memory base > + > +patternProperties: > + '-cfg$': > + type: object > + description: | > + A pinctrl node should contain at least one subnode representing the > + pinctrl groups available on the machine. > + > + additionalProperties: false > + > + patternProperties: > + '-pins$': > + type: object > + description: | > + Each subnode will list the pins it needs, and how they should > + be configured, with regard to muxer configuration, bias, input > + enable/disable, input schmitt trigger, slew-rate enable/disable, > + slew-rate, drive strength, power source. > + $ref: /schemas/pinctrl/pincfg-node.yaml > + > + allOf: > + - $ref: pincfg-node.yaml# > + - $ref: pinmux-node.yaml# > + > + properties: > + pinmux: > + description: | > + The list of GPIOs and their mux settings that properties in the > + node apply to. This should be set using the K1_PADCONF macro to > + construct the value. > + $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux > + > + bias-disable: true > + > + bias-pull-up: true > + > + bias-pull-down: true > + > + drive-strength-microamp: > + description: | > + typical current when output high level, but in mA. > + 1.8V output: 11, 21, 32, 42 (mA) > + 3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA) > + $ref: /schemas/types.yaml#/definitions/uint32 > + If there are no other runtime restrictions, you should add extra check with the power-source, then you can always get vaild settings. Regards, Inochi > + input-schmitt: > + description: | > + typical threshold for schmitt trigger. > + 0: buffer mode > + 1: trigger mode > + 2, 3: trigger mode > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3] > + > + power-source: > + description: external power supplies at 1.8v or 3.3v. > + enum: [ 1800, 3300 ] > + > + slew-rate: > + description: | > + slew rate for output buffer > + 0, 1: Slow speed > + 2: Medium speed > + 3: Fast speed > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3] > + > + spacemit,slew-rate-enable: > + description: enable slew rate. > + type: boolean > + > + spacemit,slew-rate-disable: > + description: disable slew rate. > + type: boolean > + > + spacemit,strong-pull-up: > + description: enable strong pull up. > + type: boolean > + > + required: > + - pinmux > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + pinctrl@d401e000 { > + compatible = "spacemit,k1-pinctrl"; > + reg = <0x0 0xd401e000 0x0 0x400>; > + #pinctrl-cells = <2>; > + #gpio-range-cells = <3>; > + > + uart0_2_cfg: uart0-2-cfg { > + uart0-2-pins { > + pinmux = <K1_PADCONF(GPIO_68, 2)>, > + <K1_PADCONF(GPIO_69, 2)>; > + bias-pull-up; > + drive-strength-microamp = <32>; > + }; > + }; > + }; > + }; > + > +... > diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h > new file mode 100644 > index 0000000000000..13ef4aa6c53a3 > --- /dev/null > +++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h > @@ -0,0 +1,161 @@ > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */ > +/* > + * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd > + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> > + * > + */ > + > +#ifndef _DT_BINDINGS_PINCTRL_K1_H > +#define _DT_BINDINGS_PINCTRL_K1_H > + > +#define PINMUX(pin, mux) \ > + (((pin) & 0xffff) | (((mux) & 0xff) << 16)) > + > +/* pin offset */ > +#define PINID(x) ((x) + 1) > + > +#define GPIO_INVAL 0 > +#define GPIO_00 PINID(0) > +#define GPIO_01 PINID(1) > +#define GPIO_02 PINID(2) > +#define GPIO_03 PINID(3) > +#define GPIO_04 PINID(4) > +#define GPIO_05 PINID(5) > +#define GPIO_06 PINID(6) > +#define GPIO_07 PINID(7) > +#define GPIO_08 PINID(8) > +#define GPIO_09 PINID(9) > +#define GPIO_10 PINID(10) > +#define GPIO_11 PINID(11) > +#define GPIO_12 PINID(12) > +#define GPIO_13 PINID(13) > +#define GPIO_14 PINID(14) > +#define GPIO_15 PINID(15) > +#define GPIO_16 PINID(16) > +#define GPIO_17 PINID(17) > +#define GPIO_18 PINID(18) > +#define GPIO_19 PINID(19) > +#define GPIO_20 PINID(20) > +#define GPIO_21 PINID(21) > +#define GPIO_22 PINID(22) > +#define GPIO_23 PINID(23) > +#define GPIO_24 PINID(24) > +#define GPIO_25 PINID(25) > +#define GPIO_26 PINID(26) > +#define GPIO_27 PINID(27) > +#define GPIO_28 PINID(28) > +#define GPIO_29 PINID(29) > +#define GPIO_30 PINID(30) > +#define GPIO_31 PINID(31) > + > +#define GPIO_32 PINID(32) > +#define GPIO_33 PINID(33) > +#define GPIO_34 PINID(34) > +#define GPIO_35 PINID(35) > +#define GPIO_36 PINID(36) > +#define GPIO_37 PINID(37) > +#define GPIO_38 PINID(38) > +#define GPIO_39 PINID(39) > +#define GPIO_40 PINID(40) > +#define GPIO_41 PINID(41) > +#define GPIO_42 PINID(42) > +#define GPIO_43 PINID(43) > +#define GPIO_44 PINID(44) > +#define GPIO_45 PINID(45) > +#define GPIO_46 PINID(46) > +#define GPIO_47 PINID(47) > +#define GPIO_48 PINID(48) > +#define GPIO_49 PINID(49) > +#define GPIO_50 PINID(50) > +#define GPIO_51 PINID(51) > +#define GPIO_52 PINID(52) > +#define GPIO_53 PINID(53) > +#define GPIO_54 PINID(54) > +#define GPIO_55 PINID(55) > +#define GPIO_56 PINID(56) > +#define GPIO_57 PINID(57) > +#define GPIO_58 PINID(58) > +#define GPIO_59 PINID(59) > +#define GPIO_60 PINID(60) > +#define GPIO_61 PINID(61) > +#define GPIO_62 PINID(62) > +#define GPIO_63 PINID(63) > + > +#define GPIO_64 PINID(64) > +#define GPIO_65 PINID(65) > +#define GPIO_66 PINID(66) > +#define GPIO_67 PINID(67) > +#define GPIO_68 PINID(68) > +#define GPIO_69 PINID(69) > +#define GPIO_70 PINID(70) > +#define GPIO_71 PINID(71) > +#define GPIO_72 PINID(72) > +#define GPIO_73 PINID(73) > +#define GPIO_74 PINID(74) > +#define GPIO_75 PINID(75) > +#define GPIO_76 PINID(76) > +#define GPIO_77 PINID(77) > +#define GPIO_78 PINID(78) > +#define GPIO_79 PINID(79) > +#define GPIO_80 PINID(80) > +#define GPIO_81 PINID(81) > +#define GPIO_82 PINID(82) > +#define GPIO_83 PINID(83) > +#define GPIO_84 PINID(84) > +#define GPIO_85 PINID(85) > + > +#define GPIO_101 PINID(89) > +#define GPIO_100 PINID(90) > +#define GPIO_99 PINID(91) > +#define GPIO_98 PINID(92) > +#define GPIO_103 PINID(93) > +#define GPIO_102 PINID(94) > + > +#define GPIO_104 PINID(109) > +#define GPIO_105 PINID(110) > +#define GPIO_106 PINID(111) > +#define GPIO_107 PINID(112) > +#define GPIO_108 PINID(113) > +#define GPIO_109 PINID(114) > +#define GPIO_110 PINID(115) > + > +#define GPIO_93 PINID(116) > +#define GPIO_94 PINID(117) > +#define GPIO_95 PINID(118) > +#define GPIO_96 PINID(119) > +#define GPIO_97 PINID(120) > + > +#define GPIO_86 PINID(122) > +#define GPIO_87 PINID(123) > +#define GPIO_88 PINID(124) > +#define GPIO_89 PINID(125) > +#define GPIO_90 PINID(126) > +#define GPIO_91 PINID(127) > +#define GPIO_92 PINID(128) > + > +#define GPIO_111 PINID(130) > +#define GPIO_112 PINID(131) > +#define GPIO_113 PINID(132) > +#define GPIO_114 PINID(133) > +#define GPIO_115 PINID(134) > +#define GPIO_116 PINID(135) > +#define GPIO_117 PINID(136) > +#define GPIO_118 PINID(137) > +#define GPIO_119 PINID(138) > +#define GPIO_120 PINID(139) > +#define GPIO_121 PINID(140) > +#define GPIO_122 PINID(141) > +#define GPIO_123 PINID(142) > +#define GPIO_124 PINID(143) > +#define GPIO_125 PINID(144) > +#define GPIO_126 PINID(145) > +#define GPIO_127 PINID(146) > + > +#define SLEW_RATE_SLOW0 0 > +#define SLEW_RATE_SLOW1 1 > +#define SLEW_RATE_MEDIUM 2 > +#define SLEW_RATE_FAST 3 > + > +#define K1_PADCONF(pin, func) (((pin) << 16) | (func)) > + > +#endif /* _DT_BINDINGS_PINCTRL_K1_H */ > > -- > 2.45.2 >
On 26/08/2024 03:36, Yixun Lan wrote: > Hi Krzysztof: > > On 15:48 Sun 25 Aug , Krzysztof Kozlowski wrote: >> On 25/08/2024 15:10, Yixun Lan wrote: >>> Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC. >> >> >> Please use subject prefixes matching the subsystem. You can get them for >> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory >> your patch is touching. For bindings, the preferred subjects are >> explained here: >> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters >> >> It's "dt-bindings:" > Ok, will fix in next version > >> >>> >>> Two vendor specific properties are introduced here, As the pinctrl >>> has dedicated slew rate enable control - bit[7], so we have >>> spacemit,slew-rate-{enable,disable} for this. For the same reason, >>> creating spacemit,strong-pull-up for the strong pull up control. >> >> Huh, no, use generic properties. More on that below >> > see my reply below > >> >> >>> >>> Signed-off-by: Yixun Lan <dlan@gentoo.org> >>> --- >>> .../bindings/pinctrl/spacemit,k1-pinctrl.yaml | 134 +++++++++++++++++ >>> include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h | 161 +++++++++++++++++++++ >>> 2 files changed, 295 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml >>> new file mode 100644 >>> index 0000000000000..8adfc5ebbce37 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml >>> @@ -0,0 +1,134 @@ >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: SpacemiT K1 SoC Pin Controller >>> + >>> +maintainers: >>> + - Yixun Lan <dlan@gentoo.org> >>> + >>> +properties: >>> + compatible: >>> + const: spacemit,k1-pinctrl >>> + >>> + reg: >>> + items: >>> + - description: pinctrl io memory base >>> + >>> +patternProperties: >>> + '-cfg$': >>> + type: object >>> + description: | >> >> Do not need '|' unless you need to preserve formatting. >> > Ok >>> + A pinctrl node should contain at least one subnode representing the >>> + pinctrl groups available on the machine. >>> + >>> + additionalProperties: false >> >> Keep it before description. > Ok >> >>> + >>> + patternProperties: >>> + '-pins$': >>> + type: object >>> + description: | >>> + Each subnode will list the pins it needs, and how they should >>> + be configured, with regard to muxer configuration, bias, input >>> + enable/disable, input schmitt trigger, slew-rate enable/disable, >>> + slew-rate, drive strength, power source. >>> + $ref: /schemas/pinctrl/pincfg-node.yaml >>> + >>> + allOf: >>> + - $ref: pincfg-node.yaml# >>> + - $ref: pinmux-node.yaml# >> >> You are duplicating refs. > ok, will fix it >> >>> + >>> + properties: >>> + pinmux: >>> + description: | >>> + The list of GPIOs and their mux settings that properties in the >>> + node apply to. This should be set using the K1_PADCONF macro to >>> + construct the value. >>> + $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux >> >> Hm why you need the ref? >> > will drop it >>> + >>> + bias-disable: true >>> + >>> + bias-pull-up: true >>> + >>> + bias-pull-down: true >>> + >>> + drive-strength-microamp: >>> + description: | >>> + typical current when output high level, but in mA. >>> + 1.8V output: 11, 21, 32, 42 (mA) >>> + 3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA) >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + >>> + input-schmitt: >>> + description: | >>> + typical threshold for schmitt trigger. >>> + 0: buffer mode >>> + 1: trigger mode >>> + 2, 3: trigger mode >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: [0, 1, 2, 3] >>> + >>> + power-source: >>> + description: external power supplies at 1.8v or 3.3v. >>> + enum: [ 1800, 3300 ] >>> + >>> + slew-rate: >>> + description: | >>> + slew rate for output buffer >>> + 0, 1: Slow speed >> >> Hm? Surprising, 0 is slow speed? >> > from docs, section 3.3.2.2 MFPR Register Description > 0, 1 are same, both for slow speed > https://developer.spacemit.com/documentation?token=An1vwTwKaigaXRkYfwmcznTXned Don't store here register value. > >>> + 2: Medium speed >>> + 3: Fast speed >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: [0, 1, 2, 3] >>> + >>> + spacemit,slew-rate-enable: >>> + description: enable slew rate. >> >> The presence of slew-rate enables it, doesn't it? >> > yes, this should work, I will take this approach, thanks > >>> + type: boolean >>> + >>> + spacemit,slew-rate-disable: >>> + description: disable slew rate. >>> + type: boolean >> >> Just use slew-rate, 0 disable, some value to match real slew-rate. >> > sounds good to me, since 0, 1 indicate same meaning, can re-use 0 for > disabling slew rate. > >>> + >>> + spacemit,strong-pull-up: >>> + description: enable strong pull up. >> >> Do not duplicate the property name in description. You did not say >> anything useful here. What is "strong"? bias-pull-up takes also an argument. >> > there is a dedicated strong pull bit[3] for I2C, SD card kinds of pad > I don't know how 'strong' it is if in ohms, will see if can get > more info on this (may expand the description) > > I think using 'bias-pull-up' property with argument should also work, > but it occur to me it's more intuitive to introduce a property here, which > reflect the underlying hardware functionality. this is similar to starfive's jh7100 > bindings/pinctrl/starfive,jh7100-pinctrl.yaml:154 > (refer to exist code doesn't mean always correct, so I need advice here) No, avoid introducing properties which duplicate existing generic ones. Same story was with qualcomm and it was possible to use specific value. > > I will keep this property unless there is objection, please let me know > >>> + type: boolean >>> + >>> + required: >>> + - pinmux >>> + >>> + additionalProperties: false >> >> This goes up, before description. >> > Ok >>> + >>> +required: >>> + - compatible >>> + - reg >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h> >>> + >>> + soc { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + >>> + pinctrl@d401e000 { >>> + compatible = "spacemit,k1-pinctrl"; >>> + reg = <0x0 0xd401e000 0x0 0x400>; >>> + #pinctrl-cells = <2>; >>> + #gpio-range-cells = <3>; >> >> This wasn't ever tested... :( >> ... > will drop it Test your patches instead. Best regards, Krzysztof
On Sun, Aug 25, 2024 at 01:10:04PM GMT, Yixun Lan wrote: > Add pinctrl device tree data to SpacemiT's K1 SoC. > > Signed-off-by: Yixun Lan <dlan@gentoo.org> > --- > Note, only minimal device tree data added in this series, > which just try to demonstrate this pinctrl driver, but > more dt data can be added later, in separate patches. > --- > arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 19 +++++++++++++++++++ > arch/riscv/boot/dts/spacemit/k1.dtsi | 5 +++++ > 2 files changed, 24 insertions(+) > > diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi > new file mode 100644 > index 0000000000000..38ccaad1209f5 > --- /dev/null > +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi > @@ -0,0 +1,19 @@ > +// SPDX-License-Identifier: GPL-2.0 OR MIT > +/* > + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> > + */ > + > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h> > + > +&pinctrl { > + uart0_2_cfg: uart0-2-cfg { > + uart0-2-pins { > + pinmux = <K1_PADCONF(GPIO_68, 2)>, > + <K1_PADCONF(GPIO_69, 2)>; > + > + bias-pull-up; > + drive-strength-microamp = <32>; > + }; > + }; > +}; No common file is needed at least for now. You can put it in the board dts. Also, squash this into the next patch as it is more related to the uart. > diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi > index 0777bf9e01183..a2d5f7d4a942a 100644 > --- a/arch/riscv/boot/dts/spacemit/k1.dtsi > +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi > @@ -416,6 +416,11 @@ uart9: serial@d4017800 { > status = "disabled"; > }; > > + pinctrl: pinctrl@d401e000 { > + compatible = "spacemit,k1-pinctrl"; > + reg = <0x0 0xd401e000 0x0 0x400>; > + }; > + > plic: interrupt-controller@e0000000 { > compatible = "spacemit,k1-plic", "sifive,plic-1.0.0"; > reg = <0x0 0xe0000000 0x0 0x4000000>; > > -- > 2.45.2 >
On Mon, Aug 26, 2024 at 03:09:39AM +0000, Yixun Lan wrote: > > On 13:10 Sun 25 Aug , Yixun Lan wrote: > > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC. > > > > Two vendor specific properties are introduced here, As the pinctrl > > has dedicated slew rate enable control - bit[7], so we have > > spacemit,slew-rate-{enable,disable} for this. For the same reason, > > creating spacemit,strong-pull-up for the strong pull up control. > > > > Signed-off-by: Yixun Lan <dlan@gentoo.org> I got this mail, and one of your other ones, 5 times. What's going wrong with your mail setup? | 250 T Aug 25 Yixun Lan (6.0K) ┌─>[PATCH v2 4/4] riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3 | 251 N T Aug 26 Inochi Amaoto ( 12K) │ ┌─> | 252 T Aug 25 Yixun Lan (6.8K) ├─>[PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for K1 SoC | 253 N T Aug 26 Inochi Amaoto ( 46K) │ ┌─> | 254 T Aug 25 Yixun Lan ( 38K) ├─>[PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT K1 SoC | 255 N T Aug 26 Inochi Amaoto ( 22K) │ ┌─> | 256 T Aug 26 Yixun Lan ( 333) │ ├─> | 257 T Aug 26 Yixun Lan ( 338) │ ├─> | 258 T Aug 26 Yixun Lan ( 334) │ ├─> | 259 T Aug 26 Yixun Lan ( 334) │ ├─> | 260 T Aug 26 Yixun Lan ( 333) │ ├─> | 261 C Aug 25 Rob Herring (Ar (9.0K) │ ├─> | 262 N C Aug 26 Krzysztof Kozlo ( 14K) │ │ ┌─> | 263 N C Aug 26 Inochi Amaoto ( 19K) │ │ ├─> | 264 C Aug 26 Yixun Lan ( 285) │ │ ├─> | 265 C Aug 26 Yixun Lan ( 281) │ │ ├─> | 266 N C Aug 26 Yixun Lan ( 14K) │ │ ├─> | 267 N C Aug 26 Yixun Lan ( 12K) │ │ ├─> | 268 N C Aug 26 Yixun Lan ( 12K) │ │ ├─> | 269 T Aug 25 Krzysztof Kozlo ( 13K) │ ├─> | 270 T Aug 25 Yixun Lan ( 14K) ├─>[PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC | 271 T Aug 25 Yixun Lan (8.5K) [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC
On Tue, Aug 27, 2024 at 02:23:05AM +0000, Yixun Lan wrote: > Hi Conor: > > On 17:22 Mon 26 Aug , Conor Dooley wrote: > > On Mon, Aug 26, 2024 at 03:09:39AM +0000, Yixun Lan wrote: > > > > > > On 13:10 Sun 25 Aug , Yixun Lan wrote: > > > > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC. > > > > > > > > Two vendor specific properties are introduced here, As the pinctrl > > > > has dedicated slew rate enable control - bit[7], so we have > > > > spacemit,slew-rate-{enable,disable} for this. For the same reason, > > > > creating spacemit,strong-pull-up for the strong pull up control. > > > > > > > > Signed-off-by: Yixun Lan <dlan@gentoo.org> > > > > I got this mail, and one of your other ones, 5 times. What's going wrong > > with your mail setup? > > > Oops, sorry for this, it's the second time you complain this.. > TBO, I have no idea what's happened, asked Yangyu, he didn't have this problem > > I'm using mutt+msmtp to reply, while using b4 to send the patch series, The patches only appear once, it's the replies that have the problem. > for all the mails you received, do they all have same message-id? > I have a local filter for duplicated mails myself, could this help? > > (I leave only one address of your mail in this reply, see if still have problem?) It's nothing to do with me having multiple addresses, the mails have different message ids, and as you can see below, even different sizes too. For example, two copies of the current reply came in as: Message-ID: <66cd38b0.050a0220.113bce.d5acSMTPIN_ADDED_BROKEN@mx.google.com> Message-ID: <66cd3abe.050a0220.bf184.b4feSMTPIN_ADDED_BROKEN@mx.google.com> Your mails also don't properly appear on lore, you can see some [not found] mails there: https://lore.kernel.org/linux-devicetree/20240827033057.GYB208832.dlan.gentoo/ The SMPTIN_ADDED_BROKEN might be a hint as to what is wrong with your setup? > > > | 250 T Aug 25 Yixun Lan (6.0K) ┌─>[PATCH v2 4/4] riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3 > > | 251 N T Aug 26 Inochi Amaoto ( 12K) │ ┌─> > > | 252 T Aug 25 Yixun Lan (6.8K) ├─>[PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for K1 SoC > > | 253 N T Aug 26 Inochi Amaoto ( 46K) │ ┌─> > > | 254 T Aug 25 Yixun Lan ( 38K) ├─>[PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT K1 SoC > > | 255 N T Aug 26 Inochi Amaoto ( 22K) │ ┌─> > > | 256 T Aug 26 Yixun Lan ( 333) │ ├─> > > | 257 T Aug 26 Yixun Lan ( 338) │ ├─> > > | 258 T Aug 26 Yixun Lan ( 334) │ ├─> > > | 259 T Aug 26 Yixun Lan ( 334) │ ├─> > > | 260 T Aug 26 Yixun Lan ( 333) │ ├─> > > | 261 C Aug 25 Rob Herring (Ar (9.0K) │ ├─> > > | 262 N C Aug 26 Krzysztof Kozlo ( 14K) │ │ ┌─> > > | 263 N C Aug 26 Inochi Amaoto ( 19K) │ │ ├─> > > | 264 C Aug 26 Yixun Lan ( 285) │ │ ├─> > > | 265 C Aug 26 Yixun Lan ( 281) │ │ ├─> > > | 266 N C Aug 26 Yixun Lan ( 14K) │ │ ├─> > > | 267 N C Aug 26 Yixun Lan ( 12K) │ │ ├─> > > | 268 N C Aug 26 Yixun Lan ( 12K) │ │ ├─> > > | 269 T Aug 25 Krzysztof Kozlo ( 13K) │ ├─> > > | 270 T Aug 25 Yixun Lan ( 14K) ├─>[PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC > > | 271 T Aug 25 Yixun Lan (8.5K) [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC > > > > > -- > Yixun Lan (dlan) > Gentoo Linux Developer > GPG Key ID AABEFD55
This series adds pinctrl support to SpacemiT's K1 SoC, the controller uses a single register to describe all pin functions, including bias pull up/down, drive strength, schmitter trigger, slew rate, strong pull-up, mux mode. Later, we complete the pinctrl property of uart device for the Bananapi-F3 board. The pinctrl docs of K1 can be found here[1], and dts data of this series are largely converted from vendor's code[2]. Note, we rewrite this series as an independent pinctrl driver for K1 SoC, which mean it does not use pinctrl-single driver as the model anymore, see the suggestion from Krzysztof at [3]. Link: https://developer.spacemit.com/documentation?token=An1vwTwKaigaXRkYfwmcznTXned [1] Link: https://gitee.com/bianbu-linux/linux-6.1/blob/bl-v1.0.y/arch/riscv/boot/dts/spacemit/k1-x_pinctrl.dtsi [2] Link: https://lore.kernel.org/all/b7a01cba-9f68-4a6f-9795-b9103ee81d8b@kernel.org/ [3] Signed-off-by: Yixun Lan <dlan@gentoo.org> --- Changes in v2: - drop using pinctrl-single driver for K1 - rewite as independent pinctrl driver - rebase to v6.11-rc5 - Link to v1: https://lore.kernel.org/r/20240719-02-k1-pinctrl-v1-0-239ac5b77dd6@gentoo.org --- Yixun Lan (4): dt-binding: pinctrl: spacemit: add documents for K1 SoC pinctrl: spacemit: add support for SpacemiT K1 SoC riscv: dts: spacemit: add pinctrl support for K1 SoC riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3 .../bindings/pinctrl/spacemit,k1-pinctrl.yaml | 134 +++ arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts | 3 + arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 19 + arch/riscv/boot/dts/spacemit/k1.dtsi | 5 + drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/spacemit/Kconfig | 17 + drivers/pinctrl/spacemit/Makefile | 3 + drivers/pinctrl/spacemit/pinctrl-k1.c | 1012 ++++++++++++++++++++ drivers/pinctrl/spacemit/pinctrl-k1.h | 36 + include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h | 161 ++++ 11 files changed, 1392 insertions(+) --- base-commit: 5be63fc19fcaa4c236b307420483578a56986a37 change-id: 20240708-02-k1-pinctrl-3a2b0ec13101 prerequisite-change-id: 20240626-k1-01-basic-dt-1aa31eeebcd2:v5 prerequisite-patch-id: 47dcf6861f7d434d25855b379e6d7ef4ce369c9c prerequisite-patch-id: 77787fe82911923aff15ccf565e8fa451538c3a6 prerequisite-patch-id: b0bdb1742d96c5738f05262c3b0059102761390b prerequisite-patch-id: 3927d39d8d77e35d5bfe53d9950da574ff8f2054 prerequisite-patch-id: a98039136a4796252a6029e474f03906f2541643 prerequisite-patch-id: c95f6dc0547a2a63a76e3cba0cf5c623b212b4e6 prerequisite-patch-id: 66e750e438ee959ddc2a6f0650814a2d8c989139 prerequisite-patch-id: 29a0fd8c36c1a4340f0d0b68a4c34d2b8abfb1ab prerequisite-patch-id: 0bdfff661c33c380d1cf00a6c68688e05f88c0b3 prerequisite-patch-id: 99f15718e0bfbb7ed1a96dfa19f35841b004dae9 Best regards,