| Message ID | 89c95d0084da6ad7132caa54ca66eb619d8a5090.1484154449.git-series.gregory.clement@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 11, 2017 at 06:19:08PM +0100, Gregory CLEMENT wrote: > From: Hu Ziji <huziji@marvell.com> > > Marvell Xenon SDHC can support eMMC/SD/SDIO. > Add Xenon-specific properties. > Also add properties for Xenon PHY setting. > > Signed-off-by: Hu Ziji <huziji@marvell.com> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > --- > Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 197 +++++++- > 1 file changed, 197 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt > > diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt > new file mode 100644 > index 000000000000..a3876d2cc616 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt > @@ -0,0 +1,197 @@ > +Marvell Xenon SDHCI Controller device tree bindings > +This file documents differences between the core mmc properties > +described by mmc.txt and the properties used by the Xenon implementation. > + > +Multiple SDHCs might be put into a single Xenon IP, to save size and cost. > +Each SDHC is independent and owns independent resources, such as register sets, > +clock and PHY. > +Each SDHC should have an independent device tree node. > + > +Required Properties: > +- compatible: should be one of the following > + - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SoC. > + Must provide a second register area and marvell,pad-type. > + - "marvell,armada-8k-sdhci": For controllers on Armada 7K/8K SoC > + > +- clocks: > + Array of clocks required for SDHC. > + Require at least input clock for Xenon IP core. > + > +- clock-names: > + Array of names corresponding to clocks property. > + The input clock for Xenon IP core should be named as "core". > + > +- reg: > + * For "marvell,armada-3700-sdhci", two register areas. > + The first one for Xenon IP register. The second one for the Armada 3700 SoC > + PHY PAD Voltage Control register. > + Please follow the examples with compatible "marvell,armada-3700-sdhci" > + in below. > + Please also check property marvell,pad-type in below. > + > + * For other compatible strings, one register area for Xenon IP. > + > +Optional Properties: > +- mmccard: > + mmccard child node must be provided when current SDHC is for eMMC. > + Xenon SDHC often can support both SD and eMMC. This child node indicates that > + current SDHC is for eMMC card. Thus Xenon eMMC specific configuration and > + operations can be enabled prior to eMMC init sequence. > + Please refer to Documentation/devicetree/bindings/mmc/mmc-card.txt. > + This child node should not be set if current Xenon SDHC is for SD/SDIO. > + > +- bus-width: > + When 8-bit data bus width is in use for eMMC, this property should be > + explicitly provided and set as 8. > + It is optional when data bus width is 4-bit or 1-bit. > + > +- mmc-ddr-1_8v: > + Select this property when eMMC HS DDR is supported on SDHC side. > + > +- mmc-hs400-1_8v: > + Select this property when eMMC HS400 is supported on SDHC side. > + > +- no-1-8-v: > + Select this property when 1.8V signaling voltage supply is unavailable. > + When this property is enabled, both mmc-ddr-1_8v and mmc-hs400-1_8v should be > + cleared. > + > +- marvell,xenon-sdhc-id: > + Indicate the corresponding bit index of current SDHC in > + SDHC System Operation Control Register Bit[7:0]. > + Set/clear the corresponding bit to enable/disable current SDHC. > + If Xenon IP contains only one SDHC, this property is optional. > + > +- marvell,xenon-phy-type: > + Xenon support mutilple types of PHYs. > + To select eMMC 5.1 PHY, set: > + marvell,xenon-phy-type = "emmc 5.1 phy" > + eMMC 5.1 PHY is the default choice if this property is not provided. > + To select eMMC 5.0 PHY, set: > + marvell,xenon-phy-type = "emmc 5.0 phy" > + > + All those types of PHYs can support eMMC, SD and SDIO. > + Please note that this property only presents the type of PHY. > + It doesn't stand for the entire SDHC type or property. > + For example, "emmc 5.1 phy" doesn't mean that this Xenon SDHC only supports > + eMMC 5.1. > + > +- marvell,xenon-phy-znr: > + Set PHY ZNR value. > + Only available for eMMC PHY 5.1 and eMMC PHY 5.0. > + Valid range = [0:0x1F]. > + ZNR is set as 0xF by default if this property is not provided. > + > +- marvell,xenon-phy-zpr: > + Set PHY ZPR value. > + Only available for eMMC PHY 5.1 and eMMC PHY 5.0. > + Valid range = [0:0x1F]. > + ZPR is set as 0xF by default if this property is not provided. > + > +- marvell,xenon-phy-nr-success-tun: > + Set the number of required consecutive successful sampling points used to > + identify a valid sampling window, in tuning process. > + Valid range = [1:7]. > + Set as 0x4 by default if this property is not provided. > + > +- marvell,xenon-phy-tun-step-divider: > + Set the divider for calculating TUN_STEP. > + Set as 64 by default if this property is not provided. > + > +- marvell,xenon-phy-slow-mode: > + If this property is selected, transfers will bypass PHY. > + Only available when bus frequency lower than 55MHz in SDR mde. > + Disabled by default. Please only try this property if timing issues always > + occur with PHY enabled in eMMC HS SDR, SD SDR12, SD SDR25, SD SDR50 mode. > + > +- marvell,xenon-tun-count: > + Xenon SDHC SoC usually doesn't provide re-tuning counter in > + Capabilities Register 3 Bit[11:8]. > + This property provides the re-tuning counter. > + If this property is not set, default re-tuning counter will > + be set as 0x9 in driver. > + > +- marvell,pad-type: > + Type of Armada 3700 SoC PHY PAD Voltage Controller register. > + Only valid when "marvell,armada-3700-sdhci" is selected. > + Two types: "sd" and "fixed-1-8v". > + If "sd" is slected, SoC PHY PAD is set as 3.3V at the beginning and is > + switched to 1.8V when SD in UHS-I. > + If "fixed-1-8v" is slected, SoC PHY PAD is fixed 1.8V, such as for eMMC. > + Please follow the examples with compatible "marvell,armada-3700-sdhci" > + in below. > + > +Example: > +- For eMMC: > + > + sdhci@aa0000 { > + compatible = "marvell,armada-8k-sdhci"; > + reg = <0xaa0000 0x1000>; > + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH> > + clocks = <&emmc_clk>; > + clock-names = "core"; > + bus-width = <8>; > + mmc-ddr-1_8v; > + mmc-hs400-1_8v; > + marvell,xenon-sdhc-id = <0>; Should be dropped? With that, Acked-by: Rob Herring <robh@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11 January 2017 at 18:19, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > From: Hu Ziji <huziji@marvell.com> > > Marvell Xenon SDHC can support eMMC/SD/SDIO. > Add Xenon-specific properties. > Also add properties for Xenon PHY setting. > > Signed-off-by: Hu Ziji <huziji@marvell.com> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> An overall comment. Please run a spell-checking as I noticed some simple errors that needs to be fixed. Optional Properties: > +- mmccard: > + mmccard child node must be provided when current SDHC is for eMMC. > + Xenon SDHC often can support both SD and eMMC. This child node indicates that > + current SDHC is for eMMC card. Thus Xenon eMMC specific configuration and > + operations can be enabled prior to eMMC init sequence. > + Please refer to Documentation/devicetree/bindings/mmc/mmc-card.txt. > + This child node should not be set if current Xenon SDHC is for SD/SDIO. > + > +- bus-width: > + When 8-bit data bus width is in use for eMMC, this property should be > + explicitly provided and set as 8. > + It is optional when data bus width is 4-bit or 1-bit. > + > +- mmc-ddr-1_8v: > + Select this property when eMMC HS DDR is supported on SDHC side. > + > +- mmc-hs400-1_8v: > + Select this property when eMMC HS400 is supported on SDHC side. All the above properties is already specified as common mmc bindings. Let's not do that again, but instead just refer to mmc.txt as how other documentations looks like. > + > +- no-1-8-v: > + Select this property when 1.8V signaling voltage supply is unavailable. > + When this property is enabled, both mmc-ddr-1_8v and mmc-hs400-1_8v should be > + cleared. Please don't use this sdhci property, unless really needed. Perhaps you can elaborate exactly why it makes sense for your case. Or perhaps we already discussed this, in either case, please re-fresh my mind. [...] Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On jeu., janv. 26 2017, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 11 January 2017 at 18:19, Gregory CLEMENT > <gregory.clement@free-electrons.com> wrote: >> From: Hu Ziji <huziji@marvell.com> >> >> Marvell Xenon SDHC can support eMMC/SD/SDIO. >> Add Xenon-specific properties. >> Also add properties for Xenon PHY setting. >> >> Signed-off-by: Hu Ziji <huziji@marvell.com> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > An overall comment. Please run a spell-checking as I noticed some > simple errors that needs to be fixed. I though I already run a spell-checking before submitting the patch, but I will do it again. > > Optional Properties: >> +- mmccard: >> + mmccard child node must be provided when current SDHC is for eMMC. >> + Xenon SDHC often can support both SD and eMMC. This child node indicates that >> + current SDHC is for eMMC card. Thus Xenon eMMC specific configuration and >> + operations can be enabled prior to eMMC init sequence. >> + Please refer to Documentation/devicetree/bindings/mmc/mmc-card.txt. >> + This child node should not be set if current Xenon SDHC is for SD/SDIO. >> + >> +- bus-width: >> + When 8-bit data bus width is in use for eMMC, this property should be >> + explicitly provided and set as 8. >> + It is optional when data bus width is 4-bit or 1-bit. >> + >> +- mmc-ddr-1_8v: >> + Select this property when eMMC HS DDR is supported on SDHC side. >> + >> +- mmc-hs400-1_8v: >> + Select this property when eMMC HS400 is supported on SDHC side. > > All the above properties is already specified as common mmc bindings. > Let's not do that again, but instead just refer to mmc.txt as how > other documentations looks like. OK, but maybe we could keep the part about the mmcard for the eMMC to emphasize that we need to setup this option for the controller. >> + >> +- no-1-8-v: >> + Select this property when 1.8V signaling voltage supply is unavailable. >> + When this property is enabled, both mmc-ddr-1_8v and mmc-hs400-1_8v should be >> + cleared. > > Please don't use this sdhci property, unless really needed. Actually this is a mmc property, at least it is described in this the mmc binding: Documentation/devicetree/bindings/mmc/mmc.txt The description of this optional property is: "no-1-8-v: when present, denotes that 1.8v card voltage is not supported on this system, even if the controller claims it is." So as it is also part of the mmc binding we can remove it from our bindings as we are going to do it for the other mmc property. But I am curious to know why do you think we could use it without any need. Are you aware of some abuse of this property? > > Perhaps you can elaborate exactly why it makes sense for your case. Or > perhaps we already discussed this, in either case, please re-fresh my > mind. For instance we need it now on the 7040DB board which does not have the 1.8V signal voltage available. Gregory
> > OK, but maybe we could keep the part about the mmcard for the eMMC to > emphasize that we need to setup this option for the controller. Right! > >>> + >>> +- no-1-8-v: >>> + Select this property when 1.8V signaling voltage supply is unavailable. >>> + When this property is enabled, both mmc-ddr-1_8v and mmc-hs400-1_8v should be >>> + cleared. >> >> Please don't use this sdhci property, unless really needed. > > Actually this is a mmc property, at least it is described in this the > mmc binding: > Documentation/devicetree/bindings/mmc/mmc.txt Yes, the documentation is wrong. It should we be moved into the docs for sdhci. I will send a patch updating it. Apologize for the misleading information. > > The description of this optional property is: > "no-1-8-v: when present, denotes that 1.8v card voltage is not supported on > this system, even if the controller claims it is." > > So as it is also part of the mmc binding we can remove it from our > bindings as we are going to do it for the other mmc property. Nope. > > But I am curious to know why do you think we could use it without any > need. Are you aware of some abuse of this property? Yes. > >> >> Perhaps you can elaborate exactly why it makes sense for your case. Or >> perhaps we already discussed this, in either case, please re-fresh my >> mind. > > For instance we need it now on the 7040DB board which does not have the > 1.8V signal voltage available. It it's better to describe what the controller/board support, instead of what it doesn't. I think you should be able to use other existing mmc DT bindings (perhaps also you need the brand new "mmc-ddr-3_3v"), instead of the "no-1-8-v". Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On ven., janv. 27 2017, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> >> But I am curious to know why do you think we could use it without any >> need. Are you aware of some abuse of this property? > > Yes. > Actually I was hopping that you point some examples :) But as you point mmc-ddr-3_3v I found my answers here: http://www.spinics.net/lists/linux-mmc/msg38602.html >> >>> >>> Perhaps you can elaborate exactly why it makes sense for your case. Or >>> perhaps we already discussed this, in either case, please re-fresh my >>> mind. >> >> For instance we need it now on the 7040DB board which does not have the >> 1.8V signal voltage available. > > It it's better to describe what the controller/board support, instead > of what it doesn't. > > I think you should be able to use other existing mmc DT bindings > (perhaps also you need the brand new "mmc-ddr-3_3v"), instead of the > "no-1-8-v". OK I will try with this one. And if it works, I wonder if it worth mentioning it as it is also part of the mmc binding. Gregory > > Kind regards > Uffe
diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt new file mode 100644 index 000000000000..a3876d2cc616 --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt @@ -0,0 +1,197 @@ +Marvell Xenon SDHCI Controller device tree bindings +This file documents differences between the core mmc properties +described by mmc.txt and the properties used by the Xenon implementation. + +Multiple SDHCs might be put into a single Xenon IP, to save size and cost. +Each SDHC is independent and owns independent resources, such as register sets, +clock and PHY. +Each SDHC should have an independent device tree node. + +Required Properties: +- compatible: should be one of the following + - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SoC. + Must provide a second register area and marvell,pad-type. + - "marvell,armada-8k-sdhci": For controllers on Armada 7K/8K SoC + +- clocks: + Array of clocks required for SDHC. + Require at least input clock for Xenon IP core. + +- clock-names: + Array of names corresponding to clocks property. + The input clock for Xenon IP core should be named as "core". + +- reg: + * For "marvell,armada-3700-sdhci", two register areas. + The first one for Xenon IP register. The second one for the Armada 3700 SoC + PHY PAD Voltage Control register. + Please follow the examples with compatible "marvell,armada-3700-sdhci" + in below. + Please also check property marvell,pad-type in below. + + * For other compatible strings, one register area for Xenon IP. + +Optional Properties: +- mmccard: + mmccard child node must be provided when current SDHC is for eMMC. + Xenon SDHC often can support both SD and eMMC. This child node indicates that + current SDHC is for eMMC card. Thus Xenon eMMC specific configuration and + operations can be enabled prior to eMMC init sequence. + Please refer to Documentation/devicetree/bindings/mmc/mmc-card.txt. + This child node should not be set if current Xenon SDHC is for SD/SDIO. + +- bus-width: + When 8-bit data bus width is in use for eMMC, this property should be + explicitly provided and set as 8. + It is optional when data bus width is 4-bit or 1-bit. + +- mmc-ddr-1_8v: + Select this property when eMMC HS DDR is supported on SDHC side. + +- mmc-hs400-1_8v: + Select this property when eMMC HS400 is supported on SDHC side. + +- no-1-8-v: + Select this property when 1.8V signaling voltage supply is unavailable. + When this property is enabled, both mmc-ddr-1_8v and mmc-hs400-1_8v should be + cleared. + +- marvell,xenon-sdhc-id: + Indicate the corresponding bit index of current SDHC in + SDHC System Operation Control Register Bit[7:0]. + Set/clear the corresponding bit to enable/disable current SDHC. + If Xenon IP contains only one SDHC, this property is optional. + +- marvell,xenon-phy-type: + Xenon support mutilple types of PHYs. + To select eMMC 5.1 PHY, set: + marvell,xenon-phy-type = "emmc 5.1 phy" + eMMC 5.1 PHY is the default choice if this property is not provided. + To select eMMC 5.0 PHY, set: + marvell,xenon-phy-type = "emmc 5.0 phy" + + All those types of PHYs can support eMMC, SD and SDIO. + Please note that this property only presents the type of PHY. + It doesn't stand for the entire SDHC type or property. + For example, "emmc 5.1 phy" doesn't mean that this Xenon SDHC only supports + eMMC 5.1. + +- marvell,xenon-phy-znr: + Set PHY ZNR value. + Only available for eMMC PHY 5.1 and eMMC PHY 5.0. + Valid range = [0:0x1F]. + ZNR is set as 0xF by default if this property is not provided. + +- marvell,xenon-phy-zpr: + Set PHY ZPR value. + Only available for eMMC PHY 5.1 and eMMC PHY 5.0. + Valid range = [0:0x1F]. + ZPR is set as 0xF by default if this property is not provided. + +- marvell,xenon-phy-nr-success-tun: + Set the number of required consecutive successful sampling points used to + identify a valid sampling window, in tuning process. + Valid range = [1:7]. + Set as 0x4 by default if this property is not provided. + +- marvell,xenon-phy-tun-step-divider: + Set the divider for calculating TUN_STEP. + Set as 64 by default if this property is not provided. + +- marvell,xenon-phy-slow-mode: + If this property is selected, transfers will bypass PHY. + Only available when bus frequency lower than 55MHz in SDR mde. + Disabled by default. Please only try this property if timing issues always + occur with PHY enabled in eMMC HS SDR, SD SDR12, SD SDR25, SD SDR50 mode. + +- marvell,xenon-tun-count: + Xenon SDHC SoC usually doesn't provide re-tuning counter in + Capabilities Register 3 Bit[11:8]. + This property provides the re-tuning counter. + If this property is not set, default re-tuning counter will + be set as 0x9 in driver. + +- marvell,pad-type: + Type of Armada 3700 SoC PHY PAD Voltage Controller register. + Only valid when "marvell,armada-3700-sdhci" is selected. + Two types: "sd" and "fixed-1-8v". + If "sd" is slected, SoC PHY PAD is set as 3.3V at the beginning and is + switched to 1.8V when SD in UHS-I. + If "fixed-1-8v" is slected, SoC PHY PAD is fixed 1.8V, such as for eMMC. + Please follow the examples with compatible "marvell,armada-3700-sdhci" + in below. + +Example: +- For eMMC: + + sdhci@aa0000 { + compatible = "marvell,armada-8k-sdhci"; + reg = <0xaa0000 0x1000>; + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH> + clocks = <&emmc_clk>; + clock-names = "core"; + bus-width = <8>; + mmc-ddr-1_8v; + mmc-hs400-1_8v; + marvell,xenon-sdhc-id = <0>; + marvell,xenon-phy-type = "emmc 5.1 phy"; + marvell,xenon-tun-count = <11>; + + #address-cells = <1>; + #size-cells = <0>; + mmccard: mmccard@0 { + compatible = "mmc-card"; + reg = <0>; + }; + }; + +- For SD/SDIO: + + sdhci@ab0000 { + compatible = "marvell,armada-8k-sdhci"; + reg = <0xab0000 0x1000>; + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH> + vqmmc-supply = <&sd_regulator>; + clocks = <&sdclk>; + clock-names = "core"; + bus-width = <4>; + marvell,xenon-tun-count = <9>; + }; + +- For eMMC with compatible "marvell,armada-3700-sdhci": + + sdhci@aa0000 { + compatible = "marvell,armada-3700-sdhci"; + reg = <0xaa0000 0x1000>, + <phy_addr 0x4>; + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH> + clocks = <&emmcclk>; + clock-names = "core"; + bus-width = <8>; + mmc-ddr-1_8v; + mmc-hs400-1_8v; + + marvell,pad-type = "fixed-1-8v"; + + #address-cells = <1>; + #size-cells = <0>; + mmccard: mmccard@0 { + compatible = "mmc-card"; + reg = <0>; + }; + }; + +- For SD/SDIO with compatible "marvell,armada-3700-sdhci": + + sdhci@ab0000 { + compatible = "marvell,armada-3700-sdhci"; + reg = <0xab0000 0x1000>, + <phy_addr 0x4>; + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH> + vqmmc-supply = <&sd_regulator>; + clocks = <&sdclk>; + clock-names = "core"; + bus-width = <4>; + + marvell,pad-type = "sd"; + };