diff mbox series

ARM: dts: sync Moxa SDIO 'compatible' name with moxart-mmc.c

Message ID 20220905104317.2740661-1-saproj@gmail.com
State Changes Requested, archived
Headers show
Series ARM: dts: sync Moxa SDIO 'compatible' name with moxart-mmc.c | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 1 warnings, 16 lines checked
robh/patch-applied success

Commit Message

Sergei Antonov Sept. 5, 2022, 10:43 a.m. UTC
Driver moxart-mmc.c has .compatible = "moxa,moxart-mmc".

But moxart.dtsi and the documentation file moxa,moxart-dma.txt
contain another name: compatible = "moxa,moxart-sdhci".

Change name in moxart.dtsi and moxa,moxart-dma.txt to that from the driver.

Signed-off-by: Sergei Antonov <saproj@gmail.com>
Cc: Jonas Jensen <jonas.jensen@gmail.com>
---
 Documentation/devicetree/bindings/dma/moxa,moxart-dma.txt | 2 +-
 arch/arm/boot/dts/moxart.dtsi                             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Arnd Bergmann Sept. 5, 2022, 11:12 a.m. UTC | #1
On Mon, Sep 5, 2022, at 12:43 PM, Sergei Antonov wrote:
> Driver moxart-mmc.c has .compatible = "moxa,moxart-mmc".
>
> But moxart.dtsi and the documentation file moxa,moxart-dma.txt
> contain another name: compatible = "moxa,moxart-sdhci".
>
> Change name in moxart.dtsi and moxa,moxart-dma.txt to that from the driver.
>
> Signed-off-by: Sergei Antonov <saproj@gmail.com>
> Cc: Jonas Jensen <jonas.jensen@gmail.com>

Something is clearly wrong here, as the moxart-mmc device is not
an sdhci at all, but are you sure that this is actually the
correct device?

> diff --git a/arch/arm/boot/dts/moxart.dtsi b/arch/arm/boot/dts/moxart.dtsi
> index f5f070a87482..d69d73930ebe 100644
> --- a/arch/arm/boot/dts/moxart.dtsi
> +++ b/arch/arm/boot/dts/moxart.dtsi
> @@ -94,7 +94,7 @@ watchdog: watchdog@98500000 {
>  		};
> 
>  		sdhci: sdhci@98e00000 {
> -			compatible = "moxa,moxart-sdhci";
> +			compatible = "moxa,moxart-mmc";
>  			reg = <0x98e00000 0x5C>;
>  			interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
>  			clocks = <&clk_apb>;

Both the label and the device name still point to "sdhci",
so it would be possible that the SoC actually has both
an SDHCI compatible device and ftsdc010. In this case the
correct fix would be to add a second node for the
moxa,moxart-mmc with the correct reg and interrupts
properties but to leave this one alone.

      Arnd
Sergei Antonov Sept. 5, 2022, 11:24 a.m. UTC | #2
On Mon, 5 Sept 2022 at 14:13, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Sep 5, 2022, at 12:43 PM, Sergei Antonov wrote:
> > Driver moxart-mmc.c has .compatible = "moxa,moxart-mmc".
> >
> > But moxart.dtsi and the documentation file moxa,moxart-dma.txt
> > contain another name: compatible = "moxa,moxart-sdhci".
> >
> > Change name in moxart.dtsi and moxa,moxart-dma.txt to that from the driver.
> >
> > Signed-off-by: Sergei Antonov <saproj@gmail.com>
> > Cc: Jonas Jensen <jonas.jensen@gmail.com>
>
> Something is clearly wrong here, as the moxart-mmc device is not
> an sdhci at all, but are you sure that this is actually the
> correct device?

I am sure. Witnessing right now that it works with SDHC cards with
capacity 16 GB and 8 GB.
Datasheet quote:
"supports both MMC and SD interface protocol."
See https://bitbucket.org/Kasreyn/mkrom-uc7112lx/src/master/documents/FIC8120_DS_v1.2.pdf
, page 367.

> > diff --git a/arch/arm/boot/dts/moxart.dtsi b/arch/arm/boot/dts/moxart.dtsi
> > index f5f070a87482..d69d73930ebe 100644
> > --- a/arch/arm/boot/dts/moxart.dtsi
> > +++ b/arch/arm/boot/dts/moxart.dtsi
> > @@ -94,7 +94,7 @@ watchdog: watchdog@98500000 {
> >               };
> >
> >               sdhci: sdhci@98e00000 {
> > -                     compatible = "moxa,moxart-sdhci";
> > +                     compatible = "moxa,moxart-mmc";
> >                       reg = <0x98e00000 0x5C>;
> >                       interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> >                       clocks = <&clk_apb>;
>
> Both the label and the device name still point to "sdhci",
> so it would be possible that the SoC actually has both
> an SDHCI compatible device and ftsdc010. In this case the
> correct fix would be to add a second node for the
> moxa,moxart-mmc with the correct reg and interrupts
> properties but to leave this one alone.

SoC contains ftsdc010 which supports SDHC cards. Should be compatible
with MMC cards too. I will test it to be sure.
Arnd Bergmann Sept. 5, 2022, 12:13 p.m. UTC | #3
On Mon, Sep 5, 2022, at 1:24 PM, Sergei Antonov wrote:
> On Mon, 5 Sept 2022 at 14:13, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Mon, Sep 5, 2022, at 12:43 PM, Sergei Antonov wrote:
> I am sure. Witnessing right now that it works with SDHC cards with
> capacity 16 GB and 8 GB.
> Datasheet quote:
> "supports both MMC and SD interface protocol."
> See 
> https://bitbucket.org/Kasreyn/mkrom-uc7112lx/src/master/documents/FIC8120_DS_v1.2.pdf
> , page 367.

Ok, that does make it pretty clear that there is only one
controller.

>> > diff --git a/arch/arm/boot/dts/moxart.dtsi b/arch/arm/boot/dts/moxart.dtsi
>> > index f5f070a87482..d69d73930ebe 100644
>> > --- a/arch/arm/boot/dts/moxart.dtsi
>> > +++ b/arch/arm/boot/dts/moxart.dtsi
>> > @@ -94,7 +94,7 @@ watchdog: watchdog@98500000 {
>> >               };
>> >
>> >               sdhci: sdhci@98e00000 {
>> > -                     compatible = "moxa,moxart-sdhci";
>> > +                     compatible = "moxa,moxart-mmc";
>> >                       reg = <0x98e00000 0x5C>;
>> >                       interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
>> >                       clocks = <&clk_apb>;
>>
>> Both the label and the device name still point to "sdhci",
>> so it would be possible that the SoC actually has both
>> an SDHCI compatible device and ftsdc010. In this case the
>> correct fix would be to add a second node for the
>> moxa,moxart-mmc with the correct reg and interrupts
>> properties but to leave this one alone.
>
> SoC contains ftsdc010 which supports SDHC cards. Should be compatible
> with MMC cards too. I will test it to be sure.

Don't worry about MMC cards, that is not the point. Both sdhci
and ftsdc010 can support SD, SDHC, MMC and eMMC devices, probably
other related standards as well.

The only question was whether the ftsdc010 was incorrectly labeled
as an sdhci type controller rather than ftsdc010, or whether the
chip has both sdhci and ftsdc010 controllers that can be used
simultaneously as is common on some other chips.

Please respin the patch though and change the node name to mmc@,
and the change the label to something other than sdhci (e.g.
"mmc", or "ftsdc010", doesn't matter) to make this less
confusing.

      Arnd
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/dma/moxa,moxart-dma.txt b/Documentation/devicetree/bindings/dma/moxa,moxart-dma.txt
index 8a9f3559335b..20247827f35a 100644
--- a/Documentation/devicetree/bindings/dma/moxa,moxart-dma.txt
+++ b/Documentation/devicetree/bindings/dma/moxa,moxart-dma.txt
@@ -35,7 +35,7 @@  Use specific request line passing from dma
 For example, MMC request line is 5
 
 	sdhci: sdhci@98e00000 {
-		compatible = "moxa,moxart-sdhci";
+		compatible = "moxa,moxart-mmc";
 		reg = <0x98e00000 0x5C>;
 		interrupts = <5 0>;
 		clocks = <&clk_apb>;
diff --git a/arch/arm/boot/dts/moxart.dtsi b/arch/arm/boot/dts/moxart.dtsi
index f5f070a87482..d69d73930ebe 100644
--- a/arch/arm/boot/dts/moxart.dtsi
+++ b/arch/arm/boot/dts/moxart.dtsi
@@ -94,7 +94,7 @@  watchdog: watchdog@98500000 {
 		};
 
 		sdhci: sdhci@98e00000 {
-			compatible = "moxa,moxart-sdhci";
+			compatible = "moxa,moxart-mmc";
 			reg = <0x98e00000 0x5C>;
 			interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clk_apb>;