* Re: [PATCH v3 1/2] dt-bindings: exynos-dw-mshc-common: add exynos78xx variants [not found] <640e0b2d.c20a0220.63661.19a4@mx.google.com> @ 2023-03-12 17:29 ` Krzysztof Kozlowski 0 siblings, 0 replies; 6+ messages in thread From: Krzysztof Kozlowski @ 2023-03-12 17:29 UTC (permalink / raw) To: Sergey Lisov, Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Jaehoon Chung Cc: linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel On 12/03/2023 18:26, Sergey Lisov wrote: >> Thanks for letting me know. >> >> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 > > OK, at least its codified somewhere. You got feedback from Devicetree maintainer. It should have been enough. > Still, this results in the opposite > effect: DTBs written for one SoC, using compatibles from other SoCs just > because "they are the same anyway". And doing this properly, well, results > in essentially duplicate compatibles. > > And "fallback compatibles" won't solve this case anyway, as there is no > common compatible that denotes "Exynos7 DW-MMC that has the bug". Your explanation is not correct and we talked about this so many times. No wildcards in compatibles. That's the rule. You cannot find any argument good enough to break that rule. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <640e0136.c20a0220.2d5bf.1959@mx.google.com>]
* Re: [PATCH v3 1/2] dt-bindings: exynos-dw-mshc-common: add exynos78xx variants [not found] <640e0136.c20a0220.2d5bf.1959@mx.google.com> @ 2023-03-12 17:09 ` Krzysztof Kozlowski 2023-03-12 17:26 ` Sergey Lisov 0 siblings, 1 reply; 6+ messages in thread From: Krzysztof Kozlowski @ 2023-03-12 17:09 UTC (permalink / raw) To: Sergey Lisov, Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Jaehoon Chung Cc: linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel On 12/03/2023 17:43, Sergey Lisov wrote: >> Bindings and DTS (and driver) are always separate. > > Okay, will split the patch. > >> Compatibles must be specific. > > No, this way you'd have tons of identical compatibles that only differ in > the exynosXXXX digits, and are functionally equivalent. Thanks for letting me know. https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 >> That's non-bisectable change (also breaking other users of DTS), so you >> need to explain in commit msg rationale - devices were never compatible >> and using exynos7 does not work in certain cases. BTW, this rationale was only example - you need to come with something real. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: exynos-dw-mshc-common: add exynos78xx variants 2023-03-12 17:09 ` Krzysztof Kozlowski @ 2023-03-12 17:26 ` Sergey Lisov 0 siblings, 0 replies; 6+ messages in thread From: Sergey Lisov @ 2023-03-12 17:26 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Jaehoon Chung Cc: linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel > Thanks for letting me know. > > https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 OK, at least its codified somewhere. Still, this results in the opposite effect: DTBs written for one SoC, using compatibles from other SoCs just because "they are the same anyway". And doing this properly, well, results in essentially duplicate compatibles. And "fallback compatibles" won't solve this case anyway, as there is no common compatible that denotes "Exynos7 DW-MMC that has the bug". >>> That's non-bisectable change (also breaking other users of DTS), so you >>> need to explain in commit msg rationale - devices were never compatible >>> and using exynos7 does not work in certain cases. Probably it makes sense to put this patch after the actual implementation, so that git bisect always gives a working setup. > BTW, this rationale was only example - you need to come with something real. Pretty much the only thing that is broken are SDIO cards, because they run very short transfers (below the DMA threshold) over the data lines. That's exactly what I stated. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 0/2] mmc: dw_mmc: fix DW MMC cores with 32-bit bus on 64-bit Linux systems @ 2023-03-12 13:03 Sergey Lisov 2023-03-12 13:03 ` [PATCH v3 1/2] dt-bindings: exynos-dw-mshc-common: add exynos78xx variants Sergey Lisov 0 siblings, 1 reply; 6+ messages in thread From: Sergey Lisov @ 2023-03-12 13:03 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Jaehoon Chung Cc: linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel DesignWare MMC cores have a configurable data bus width of either 16, 32, or 64 bytes. It is possible, and some vendors actually do it, to ship a DW MMC core configured for 32-bit data bus within a 64-bit SoC. In this case the kernel will attempt 64-bit (readq) accesses to certain 64-bit MMIO registers, while the core will expect pairs of 32-bit accesses. It seems that currently the only register for which the kernel performs 64-bit accesses is the FIFO. The symptom is that the DW MMC core never receives a read on the second half of the register, does not register the datum as being read, and thus not advancing its internal FIFO pointer, breaking further reads. It also seems that this FIFO is only used for small (less than 16 bytes) transfers, which probably means that only some SDIO cards are affected. Changelog: v3: - removed "fifo-access-32bit" devicetree property - added "samsung,exynos78xx-dw-mshc" compatible string - added "samsung,exynos78xx-dw-mshc-smu" compatible string v2: - added commit messages v1: - added "fifo-access-32bit" devicetree property - added DW_MMC_QUIRK_FIFO64_32 - added new dw_mci_{pull,push}_data* variants (...-data64_32) Sergey Lisov (2): dt-bindings: exynos-dw-mshc-common: add exynos78xx variants mmc: dw_mmc: add an option to force 32-bit access to 64-bit FIFO .../bindings/mmc/samsung,exynos-dw-mshc.yaml | 2 + arch/arm64/boot/dts/exynos/exynos7885.dtsi | 2 +- drivers/mmc/host/dw_mmc-exynos.c | 41 +++++- drivers/mmc/host/dw_mmc.c | 122 +++++++++++++++++- drivers/mmc/host/dw_mmc.h | 2 + 5 files changed, 165 insertions(+), 4 deletions(-) -- 2.38.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] dt-bindings: exynos-dw-mshc-common: add exynos78xx variants 2023-03-12 13:03 [PATCH v3 0/2] mmc: dw_mmc: fix DW MMC cores with 32-bit bus on 64-bit Linux systems Sergey Lisov @ 2023-03-12 13:03 ` Sergey Lisov 2023-03-12 13:35 ` Krzysztof Kozlowski 0 siblings, 1 reply; 6+ messages in thread From: Sergey Lisov @ 2023-03-12 13:03 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Jaehoon Chung Cc: linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel Some Samsung Exynos boards using the arm64 architecture have DW MMC controllers configured for a 32-bit data bus but a 64-bit FIFO. On these systems the 64-bit FIFO registers must be accessed in two 32-bit halves. Add two new compatible strings, "samsung,exynos78xx-dw-mshc" and "samsung,exynos78xx-dw-mshc-smu" respectively, to denote exynos78xx boards that need this quirk. But it's very possible that all "samsung,exynos7-dw-mshc" boards are actually affected. --- .../devicetree/bindings/mmc/samsung,exynos-dw-mshc.yaml | 2 ++ arch/arm64/boot/dts/exynos/exynos7885.dtsi | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/mmc/samsung,exynos-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/samsung,exynos-dw-mshc.yaml index fdaa18481..a72a67792 100644 --- a/Documentation/devicetree/bindings/mmc/samsung,exynos-dw-mshc.yaml +++ b/Documentation/devicetree/bindings/mmc/samsung,exynos-dw-mshc.yaml @@ -22,6 +22,8 @@ properties: - samsung,exynos5420-dw-mshc-smu - samsung,exynos7-dw-mshc - samsung,exynos7-dw-mshc-smu + - samsung,exynos78xx-dw-mshc + - samsung,exynos78xx-dw-mshc-smu - axis,artpec8-dw-mshc reg: diff --git a/arch/arm64/boot/dts/exynos/exynos7885.dtsi b/arch/arm64/boot/dts/exynos/exynos7885.dtsi index 23c2e0bb0..4b94ac9da 100644 --- a/arch/arm64/boot/dts/exynos/exynos7885.dtsi +++ b/arch/arm64/boot/dts/exynos/exynos7885.dtsi @@ -294,7 +294,7 @@ pmu_system_controller: system-controller@11c80000 { }; mmc_0: mmc@13500000 { - compatible = "samsung,exynos7-dw-mshc-smu"; + compatible = "samsung,exynos78xx-dw-mshc-smu"; reg = <0x13500000 0x2000>; interrupts = <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>; #address-cells = <1>; -- 2.38.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: exynos-dw-mshc-common: add exynos78xx variants 2023-03-12 13:03 ` [PATCH v3 1/2] dt-bindings: exynos-dw-mshc-common: add exynos78xx variants Sergey Lisov @ 2023-03-12 13:35 ` Krzysztof Kozlowski 2023-03-12 16:43 ` Sergey Lisov 0 siblings, 1 reply; 6+ messages in thread From: Krzysztof Kozlowski @ 2023-03-12 13:35 UTC (permalink / raw) To: Sergey Lisov, Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Jaehoon Chung Cc: linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel On 12/03/2023 14:03, Sergey Lisov wrote: > Some Samsung Exynos boards using the arm64 architecture have DW MMC > controllers configured for a 32-bit data bus but a 64-bit FIFO. On these > systems the 64-bit FIFO registers must be accessed in two 32-bit halves. > > Add two new compatible strings, "samsung,exynos78xx-dw-mshc" and > "samsung,exynos78xx-dw-mshc-smu" respectively, to denote exynos78xx > boards that need this quirk. But it's very possible that all > "samsung,exynos7-dw-mshc" boards are actually affected. > --- > .../devicetree/bindings/mmc/samsung,exynos-dw-mshc.yaml | 2 ++ > arch/arm64/boot/dts/exynos/exynos7885.dtsi | 2 +- Bindings and DTS (and driver) are always separate. > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/mmc/samsung,exynos-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/samsung,exynos-dw-mshc.yaml > index fdaa18481..a72a67792 100644 > --- a/Documentation/devicetree/bindings/mmc/samsung,exynos-dw-mshc.yaml > +++ b/Documentation/devicetree/bindings/mmc/samsung,exynos-dw-mshc.yaml > @@ -22,6 +22,8 @@ properties: > - samsung,exynos5420-dw-mshc-smu > - samsung,exynos7-dw-mshc > - samsung,exynos7-dw-mshc-smu > + - samsung,exynos78xx-dw-mshc > + - samsung,exynos78xx-dw-mshc-smu Compatibles must be specific. There is also no fallback, but I assume that's intentional - you say these are not compatible? > - axis,artpec8-dw-mshc > > reg: > diff --git a/arch/arm64/boot/dts/exynos/exynos7885.dtsi b/arch/arm64/boot/dts/exynos/exynos7885.dtsi > index 23c2e0bb0..4b94ac9da 100644 > --- a/arch/arm64/boot/dts/exynos/exynos7885.dtsi > +++ b/arch/arm64/boot/dts/exynos/exynos7885.dtsi > @@ -294,7 +294,7 @@ pmu_system_controller: system-controller@11c80000 { > }; > > mmc_0: mmc@13500000 { > - compatible = "samsung,exynos7-dw-mshc-smu"; > + compatible = "samsung,exynos78xx-dw-mshc-smu"; That's non-bisectable change (also breaking other users of DTS), so you need to explain in commit msg rationale - devices were never compatible and using exynos7 does not work in certain cases. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: exynos-dw-mshc-common: add exynos78xx variants 2023-03-12 13:35 ` Krzysztof Kozlowski @ 2023-03-12 16:43 ` Sergey Lisov 0 siblings, 0 replies; 6+ messages in thread From: Sergey Lisov @ 2023-03-12 16:43 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Jaehoon Chung Cc: linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel > Bindings and DTS (and driver) are always separate. Okay, will split the patch. > Compatibles must be specific. No, this way you'd have tons of identical compatibles that only differ in the exynosXXXX digits, and are functionally equivalent. > That's non-bisectable change (also breaking other users of DTS), so you > need to explain in commit msg rationale - devices were never compatible > and using exynos7 does not work in certain cases. Valid point. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-12 17:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <640e0b2d.c20a0220.63661.19a4@mx.google.com> 2023-03-12 17:29 ` [PATCH v3 1/2] dt-bindings: exynos-dw-mshc-common: add exynos78xx variants Krzysztof Kozlowski [not found] <640e0136.c20a0220.2d5bf.1959@mx.google.com> 2023-03-12 17:09 ` Krzysztof Kozlowski 2023-03-12 17:26 ` Sergey Lisov 2023-03-12 13:03 [PATCH v3 0/2] mmc: dw_mmc: fix DW MMC cores with 32-bit bus on 64-bit Linux systems Sergey Lisov 2023-03-12 13:03 ` [PATCH v3 1/2] dt-bindings: exynos-dw-mshc-common: add exynos78xx variants Sergey Lisov 2023-03-12 13:35 ` Krzysztof Kozlowski 2023-03-12 16:43 ` Sergey Lisov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).