| Message ID | 20241212-speedy-v1-0-544ad7bcfb6a@gmail.com |
|---|---|
| Headers | show |
| Series | Add Samsung SPEEDY serial bus host controller driver | expand |
On Thu, 12 Dec 2024 23:09:01 +0200, Markuss Broks wrote: > Add the schema for the Samsung SPEEDY serial bus host controller. > The bus has 4 bit wide addresses for addressing devices > and 8 bit wide register addressing. Each register is also 8 > bit long, so the address can be 0-f (hexadecimal), node name > for child device follows the format: node_name@[0-f]. > > Co-developed-by: Maksym Holovach <nergzd@nergzd723.xyz> > Signed-off-by: Maksym Holovach <nergzd@nergzd723.xyz> > Signed-off-by: Markuss Broks <markuss.broks@gmail.com> > --- > .../bindings/soc/samsung/exynos-speedy.yaml | 78 ++++++++++++++++++++++ > 1 file changed, 78 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/soc/samsung/exynos-speedy.yaml: ignoring, error in schema: properties: clock-names /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml: properties:compatible: [{'items': [{'enum': ['samsung,exynos9810-speedy']}, {'const': 'samsung,exynos-speedy'}]}] is not of type 'object', 'boolean' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml: properties:clock-names: [{'const': 'pclk'}] is not of type 'object', 'boolean' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml: properties:compatible: [{'items': [{'enum': ['samsung,exynos9810-speedy']}, {'const': 'samsung,exynos-speedy'}]}] is not of type 'object', 'boolean' from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml: properties:clock-names: [{'const': 'pclk'}] is not of type 'object', 'boolean' from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# Documentation/devicetree/bindings/soc/samsung/exynos-speedy.example.dtb: /example-0/speedy@141c0000: failed to match any schema with compatible: ['samsung,exynos9810-speedy', 'samsung-exynos-speedy'] Documentation/devicetree/bindings/soc/samsung/exynos-speedy.example.dtb: /example-0/speedy@141c0000: failed to match any schema with compatible: ['samsung,exynos9810-speedy', 'samsung-exynos-speedy'] Documentation/devicetree/bindings/soc/samsung/exynos-speedy.example.dtb: /example-0/speedy@141c0000/pmic@0: failed to match any schema with compatible: ['samsung,s2mps18-pmic'] Documentation/devicetree/bindings/soc/samsung/exynos-speedy.example.dtb: /example-0/speedy@141c0000/regulator@1: failed to match any schema with compatible: ['samsung,s2mps18-regulator'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241212-speedy-v1-1-544ad7bcfb6a@gmail.com 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.
On 12/12/2024 22:09, Markuss Broks wrote: > Add the schema for the Samsung SPEEDY serial bus host controller. > The bus has 4 bit wide addresses for addressing devices > and 8 bit wide register addressing. Each register is also 8 > bit long, so the address can be 0-f (hexadecimal), node name > for child device follows the format: node_name@[0-f]. This wasn't tested so limited review. A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > Co-developed-by: Maksym Holovach <nergzd@nergzd723.xyz> > Signed-off-by: Maksym Holovach <nergzd@nergzd723.xyz> > Signed-off-by: Markuss Broks <markuss.broks@gmail.com> > --- > .../bindings/soc/samsung/exynos-speedy.yaml | 78 ++++++++++++++++++++++ Filename must match compatible. > 1 file changed, 78 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..304b322a74ea70f23d8c072b44b6ca86b7cc807f > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml > @@ -0,0 +1,78 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/soc/samsung/exynos-speedy.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Samsung Exynos SPEEDY serial bus host controller Speedy or SPEEDY? > + > +maintainers: > + - Markuss Broks <markuss.broks@gmail.com> > + > +description: > + Samsung SPEEDY is a proprietary Samsung serial 1-wire bus. 1-wire? But not compatible with w1 (onwire)? > + It is used on various Samsung Exynos chips. The bus can > + address at most 4 bit (16) devices. The devices on the bus > + have 8 bit long register line, and the registers are also > + 8 bit long each. It is typically used for communicating with > + Samsung PMICs (s2mps17, s2mps18, ...) and other Samsung chips, > + such as RF parts. > + > +properties: > + compatible: > + - items: > + - enum: > + - samsung,exynos9810-speedy > + - const: samsung,exynos-speedy Drop last compatible and use only SoC specific. > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + - const: pclk Drop clock-names, not needed for one entry. > + > + interrupts: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - "#address-cells" > + - "#size-cells" You do not have them in the properties, anyway required goes before additionalProperties > + > +patternProperties: > + "^[a-z][a-z0-9]*@[0-9a-f]$": That's odd regex. Look at other bus bindings. > + type: object > + additionalProperties: true > + > + properties: > + reg: > + maxItems: 1 maximum: 15 > + > + required: > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + speedy0: speedy@141c0000 { Drop unused label. > + compatible = "samsung,exynos9810-speedy", > + "samsung-exynos-speedy"; > + reg = <0x141c0000 0x2000>; > + #address-cells = <1>; > + #size-cells = <0>; > + No resources? No clocks? No interrupts? Best regards, Krzysztof
On 12/12/2024 22:09, Markuss Broks wrote: > Hey, > > This series adds support for the Samsung SPEEDY serial bus host > controller. Samsung SPEEDY (actually an acronym) is a proprietary > Samsung 1 wire serial bus, which is used on various Samsung devices. > It does not look like you tested the bindings, at least after quick look. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Maybe you need to update your dtschema and yamllint. It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). Please run standard kernel tools for static analysis, like coccinelle, smatch and sparse, and fix reported warnings. Also please check for warnings when building with W=1. Most of these commands (checks or W=1 build) can build specific targets, like some directory, to narrow the scope to only your code. The code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. Please run scripts/checkpatch.pl and fix reported warnings. Then please run `scripts/checkpatch.pl --strict` and (probably) fix more warnings. Some warnings can be ignored, especially from --strict run, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. Best regards, Krzysztof
Hi Krzysztof, On 12/13/24 9:40 AM, Krzysztof Kozlowski wrote: > On 12/12/2024 22:09, Markuss Broks wrote: >> Add the schema for the Samsung SPEEDY serial bus host controller. >> The bus has 4 bit wide addresses for addressing devices >> and 8 bit wide register addressing. Each register is also 8 >> bit long, so the address can be 0-f (hexadecimal), node name >> for child device follows the format: node_name@[0-f]. > > This wasn't tested so limited review. > > A nit, subject: drop second/last, redundant "bindings". The > "dt-bindings" prefix is already stating that these are bindings. > See also: > https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > >> Co-developed-by: Maksym Holovach <nergzd@nergzd723.xyz> >> Signed-off-by: Maksym Holovach <nergzd@nergzd723.xyz> >> Signed-off-by: Markuss Broks <markuss.broks@gmail.com> >> --- >> .../bindings/soc/samsung/exynos-speedy.yaml | 78 ++++++++++++++++++++++ > Filename must match compatible. > >> 1 file changed, 78 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml >> new file mode 100644 >> index 0000000000000000000000000000000000000000..304b322a74ea70f23d8c072b44b6ca86b7cc807f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml >> @@ -0,0 +1,78 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/soc/samsung/exynos-speedy.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Samsung Exynos SPEEDY serial bus host controller > Speedy or SPEEDY? Technically it's an acronym (Serial Protocol in an EffEctive Digital waY), but we could agree on if we use the capitalized or uncapitalised version and use it consistently throughout. > >> + >> +maintainers: >> + - Markuss Broks <markuss.broks@gmail.com> >> + >> +description: >> + Samsung SPEEDY is a proprietary Samsung serial 1-wire bus. > 1-wire? But not compatible with w1 (onwire)? Nope, I suppose this requires more clarification, as explained in the previous letter, there are several differences between the protocols, looking at the Samsung patent. [1] > >> + It is used on various Samsung Exynos chips. The bus can >> + address at most 4 bit (16) devices. The devices on the bus >> + have 8 bit long register line, and the registers are also >> + 8 bit long each. It is typically used for communicating with >> + Samsung PMICs (s2mps17, s2mps18, ...) and other Samsung chips, >> + such as RF parts. >> + >> +properties: >> + compatible: >> + - items: >> + - enum: >> + - samsung,exynos9810-speedy >> + - const: samsung,exynos-speedy > Drop last compatible and use only SoC specific. Makes sense, for some reason I didn't realise it doesn't make much sense. > >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + clock-names: >> + - const: pclk > Drop clock-names, not needed for one entry. > >> + >> + interrupts: >> + maxItems: 1 >> + >> +required: >> + - compatible >> + - reg >> + - "#address-cells" >> + - "#size-cells" > You do not have them in the properties, anyway required goes before > additionalProperties > >> + >> +patternProperties: >> + "^[a-z][a-z0-9]*@[0-9a-f]$": > That's odd regex. Look at other bus bindings. Okay, I'll look into it. > >> + type: object >> + additionalProperties: true >> + >> + properties: >> + reg: >> + maxItems: 1 > maximum: 15 > >> + >> + required: >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + speedy0: speedy@141c0000 { > Drop unused label. > >> + compatible = "samsung,exynos9810-speedy", >> + "samsung-exynos-speedy"; >> + reg = <0x141c0000 0x2000>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + > No resources? No clocks? No interrupts? Will extend the example. > > > > Best regards, > Krzysztof - Markuss [1] https://patents.google.com/patent/US9882711B1/en
Hey, This series adds support for the Samsung SPEEDY serial bus host controller. Samsung SPEEDY (actually an acronym) is a proprietary Samsung 1 wire serial bus, which is used on various Samsung devices. This driver adds support for the version of controller without the IP_BATCHER block. It appears that block is a small MCU attached to the SPEEDY controller to offload the SPEEDY I/O tasks from the AP. IP_BATCHER is found on Exynos7885, but not found on Exynos9810 and Exynos8895. This version of driver should still work on Exynos7885 though, but the IP_BATCHER is not supported at the moment. On Exynos9810, SPEEDY controllers are also mapped into MMIO space of other processors on the CPU. For example, APM also has a window to the SPEEDY IP, and it uses it for power-management related things. During testing however, it seems that if APM is not active the AP can access the SPEEDY controller freely and without interference from APM firmware. Things to improve: - SPEEDY host controller has an interrupt line to the AP, but current implementation uses polling instead, - add support for handling IP_BATCHER block, - add support for bulk transfers, - test on other SoCs (Exynos9820, 9830, 9840, ...). - runtime PM - Markuss To: Rob Herring <robh@kernel.org> To: Krzysztof Kozlowski <krzk+dt@kernel.org> To: Conor Dooley <conor+dt@kernel.org> To: Alim Akhtar <alim.akhtar@samsung.com> To: Krzysztof Kozlowski <krzk@kernel.org> Cc: linux-samsung-soc@vger.kernel.org Cc: devicetree@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Cc: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> Signed-off-by: Markuss Broks <markuss.broks@gmail.com> --- Markuss Broks (3): dt-bindings: soc: samsung: exynos-speedy: Document SPEEDY host controller bindings soc: samsung: Add a driver for Samsung SPEEDY host controller MAINTAINERS: Add entry for the Samsung Exynos SPEEDY host controller .../bindings/soc/samsung/exynos-speedy.yaml | 78 ++++ MAINTAINERS | 7 + drivers/soc/samsung/Kconfig | 13 + drivers/soc/samsung/Makefile | 2 + drivers/soc/samsung/exynos-speedy.c | 457 +++++++++++++++++++++ include/linux/soc/samsung/exynos-speedy.h | 56 +++ 6 files changed, 613 insertions(+) --- base-commit: 1b2ab8149928c1cea2d7eca30cd35bb7fe014053 change-id: 20241210-speedy-e43f5df2b1d6 Best regards,