ARM64: dts: meson-gxbb: add MMC support
Kevin Hilman
maintainerPATCH v2 1/2 ARM64: dts: meson-gxbb: add MMC support
Add binding and basic support for the SD/eMMC controller on Amlogic S905/GXBB devices.
Signed-off-by Kevin Hilman <[email protected]>
--- .../devicetree/bindings/mmc/amlogic,meson-gxbb.txt | 29 +++++++++ .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 74 ++++++++++++++++++++++ arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 71 +++++++++++++++++++++ arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 36 ++++++++++- 4 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt
Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt new file +29 -0
@@ -0,0 +1,29 @@ +Amlogic SD / eMMC controller for S905/GXBB family SoCs + +The MMC 5.1 compliant host controller on Amlogic provides the +interface for SD, eMMC and SDIO devices. + +This file documents the properties in addition to those available in +the MMC core bindings, documented by mmc.txt. + +Required properties: +- compatible : contains "amlogic,meson-gxbb" +- clocks : A list of phandle + clock-specifier pairs for the clocks listed in clock-names. +- clock-names: Should contain the following: + "core" - Main peripheral bus clock + "clkin0" - Parent clock of internal mux + "clkin1" - Other parent clock of internal mux + The driver has an interal mux clock which switches between clkin0 and clkin1 depending on the + clock rate requested by the MMC core. + +Example: + + sd_emmc_a: mmc at 70000 { + compatible = "amlogic,meson-gxbb-mmc"; + reg = <0x0 0x70000 0x0 0x2000>; + interrupts = < GIC_SPI 216 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_SD_EMMC_A>, <&xtal>, <&clkc CLKID_FCLK_DIV2>; + clock-names = "core", "clkin0", "clkin1"; + pinctrl-0 = <&emmc_pins>; + }; +
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts +74 -0
83 lines not on this page · raw message
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi +71 -0
80 lines not on this page · raw message
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +35 -1
@@ -247,7 +247,8 @@ mux { groups = "emmc_nand_d07", "emmc_cmd", - "emmc_clk"; + "emmc_clk", + "emmc_ds"; function = "emmc"; }; };
@@ -330,6 +331,39 @@ #address-cells = <2>; #size-cells = <2>; ranges = <0x0 0x0 0x0 0xd0000000 0x0 0x200000>; + + sd_emmc_a: mmc at 70000 { + compatible = "amlogic,meson-gxbb-mmc"; + reg = <0x0 0x70000 0x0 0x2000>; + interrupts = <GIC_SPI 216 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_SD_EMMC_A>, + <&xtal>, + <&clkc CLKID_FCLK_DIV2>; + clock-names = "core", "clkin0", "clkin1"; + status = "disabled"; + }; + + sd_emmc_b: mmc at 72000 { + compatible = "amlogic,meson-gxbb-mmc"; + reg = <0x0 0x72000 0x0 0x2000>; + interrupts = <GIC_SPI 217 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_SD_EMMC_B>, + <&xtal>, + <&clkc CLKID_FCLK_DIV2>; + clock-names = "core", "clkin0", "clkin1"; + status = "disabled"; + }; + + sd_emmc_c: mmc at 74000 { + compatible = "amlogic,meson-gxbb-mmc"; + reg = <0x0 0x74000 0x0 0x2000>; + interrupts = <GIC_SPI 218 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_SD_EMMC_C>, + <&xtal>, + <&clkc CLKID_FCLK_DIV2>; + clock-names = "core", "clkin0", "clkin1"; + status = "disabled"; + }; }; ethmac: ethernet at c9410000 {
Signature · 1 line
2.9.0
Kevin Hilman
maintainerPATCH v2 2/2 MMC: meson: initial support for GXBB platforms
Initial support for the SD/eMMC controller in the Amlogic S905/GXBB family of SoCs.
Currently working for the SD and eMMC interfaces, but not yet tested for SDIO.
Tested external SD card and internal eMMC on meson-gxbb-p200 and meson-gxbb-odroidc2.
Signed-off-by Kevin Hilman <[email protected]>
--- MAINTAINERS | 1 + drivers/mmc/host/Kconfig | 10 + drivers/mmc/host/Makefile | 1 + drivers/mmc/host/meson-gxbb.c | 918 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 930 insertions(+) create mode 100644 drivers/mmc/host/meson-gxbb.c
MAINTAINERS +1 -0
@@ -972,6 +972,7 @@ F: arch/arm/mach-meson/ F: arch/arm/boot/dts/meson* F: arch/arm64/boot/dts/amlogic/ F: drivers/pinctrl/meson/ +F: drivers/mmc/host/meson* N: meson ARM/Annapurna Labs ALPINE ARCHITECTURE
drivers/mmc/host/Kconfig +10 -0
@@ -332,6 +332,16 @@ config MMC_SDHCI_IPROC If unsure, say N. +config MMC_MESON_GXBB + tristate "Amlogic S905/GXBB SD/MMC Host Controller support" + depends on ARCH_MESON && MMC + help + This selects support for the Amlogic SD/MMC Host Controller + found on the S905/GXBB family of SoCs. This controller is + MMC 5.1 compliant and support SD, eMMC and SDIO interfaces. + + If you have a controller with this interface, say Y here. + config MMC_MOXART tristate "MOXART SD/MMC Host Controller support" depends on ARCH_MOXART && MMC
drivers/mmc/host/Makefile +1 -0
@@ -53,6 +53,7 @@ obj-$(CONFIG_MMC_JZ4740) += jz4740_mmc.o obj-$(CONFIG_MMC_VUB300) += vub300.o obj-$(CONFIG_MMC_USHC) += ushc.o obj-$(CONFIG_MMC_WMT) += wmt-sdmmc.o +obj-$(CONFIG_MMC_MESON_GXBB) += meson-gxbb.o obj-$(CONFIG_MMC_MOXART) += moxart-mmc.o obj-$(CONFIG_MMC_SUNXI) += sunxi-mmc.o obj-$(CONFIG_MMC_USDHI6ROL0) += usdhi6rol0.o
drivers/mmc/host/meson-gxbb.c new file +918 -0
918 lines not on this page · raw message
Signature · 1 line
2.9.0
PATCH v2 2/2 MMC: meson: initial support for GXBB platforms
MAINTAINERS F: arch/arm/mach-meson/ · line 975
Quoted 9 lines · Kevin Hilman
MAINTAINERS +1
-0@@ -972,6 +972,7 @@ F: arch/arm/mach-meson/ F: arch/arm/boot/dts/meson* F: arch/arm64/boot/dts/amlogic/ F: drivers/pinctrl/meson/ +F: drivers/mmc/host/meson*
Weird spacing here?
Quoted 8 lines
MAINTAINERS +0
-0N: meson ARM/Annapurna Labs ALPINE ARCHITECTUREdrivers/mmc/host/Kconfig +0
-0
drivers/mmc/host/Kconfig config MMC_SDHCI_IPROC · line 341
Quoted 10 lines
drivers/mmc/host/Kconfig +7
-0@@ -332,6 +332,16 @@ config MMC_SDHCI_IPROC If unsure, say N. +config MMC_MESON_GXBB + tristate "Amlogic S905/GXBB SD/MMC Host Controller support" + depends on ARCH_MESON && MMC + help + This selects support for the Amlogic SD/MMC Host Controller + found on the S905/GXBB family of SoCs. This controller is + MMC 5.1 compliant and support SD, eMMC and SDIO interfaces.
s/support/supports/
Quoted 59 lines
drivers/mmc/host/Kconfig +3
-0+ + If you have a controller with this interface, say Y here. + config MMC_MOXART tristate "MOXART SD/MMC Host Controller support" depends on ARCH_MOXART && MMCdrivers/mmc/host/meson-gxbb.c new file +47
-0@@ -0,0 +1,918 @@ +/* +* This file is provided under a dual BSD/GPLv2 license. When using or +* redistributing this file, you may do so under either license. +* +* GPL LICENSE SUMMARY +* +* Copyright (c) 2016 BayLibre, SAS. +* Author: Kevin Hilman <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of version 2 of the GNU General Public License as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +* The full GNU General Public License is included in this distribution +* in the file called COPYING. +* +* BSD LICENSE +* +* Copyright (c) 2016 BayLibre, SAS. +* Author: Kevin Hilman <[email protected]> +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions +* are met: +* +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in +* the documentation and/or other materials provided with the +* distribution. +* * Neither the name of Intel Corporation nor the names of its +* contributors may be used to endorse or promote products derived +* from this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
drivers/mmc/host/meson-gxbb.c line 57
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/moduleparam.h>
Unused?
drivers/mmc/host/meson-gxbb.c line 63
Quoted 6 lines
drivers/mmc/host/meson-gxbb.c new file +6
-0+#include <linux/init.h> +#include <linux/device.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/ioport.h> +#include <linux/regmap.h>
Unused?
Quoted 92 lines
drivers/mmc/host/meson-gxbb.c new file +92
-0+#include <linux/spinlock.h> +#include <linux/dma-mapping.h> +#include <linux/mmc/host.h> +#include <linux/mmc/mmc.h> +#include <linux/mmc/sdio.h> +#include <linux/mmc/slot-gpio.h> +#include <linux/io.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> + +#define DRIVER_NAME "meson-gxbb-mmc" + +#define SD_EMMC_CLOCK 0x0 +#define CLK_DIV_SHIFT 0 +#define CLK_DIV_WIDTH 6 +#define CLK_DIV_MASK 0x3f +#define CLK_DIV_MAX 63 +#define CLK_SRC_SHIFT 6 +#define CLK_SRC_WIDTH 2 +#define CLK_SRC_MASK 0x3 +#define CLK_SRC_XTAL 0/* external crystal */ +#define CLK_SRC_XTAL_RATE 24000000 +#define CLK_SRC_PLL 1/* FCLK_DIV2 */ +#define CLK_SRC_PLL_RATE 1000000000 +#define CLK_PHASE_SHIFT 8 +#define CLK_PHASE_MASK 0x3 +#define CLK_PHASE_0 0 +#define CLK_PHASE_90 1 +#define CLK_PHASE_180 2 +#define CLK_PHASE_270 3 +#define CLK_ALWAYS_ON BIT(24) + +#define SD_EMMC_DElAY 0x4 +#define SD_EMMC_ADJUST 0x8 +#define SD_EMMC_CALOUT 0x10 +#define SD_EMMC_START 0x40 +#define START_DESC_INIT BIT(0) +#define START_DESC_BUSY BIT(1) +#define START_DESC_ADDR_SHIFT 2 +#define START_DESC_ADDR_MASK (~0x3) + +#define SD_EMMC_CFG 0x44 +#define CFG_BUS_WIDTH_SHIFT 0 +#define CFG_BUS_WIDTH_MASK 0x3 +#define CFG_BUS_WIDTH_1 0x0 +#define CFG_BUS_WIDTH_4 0x1 +#define CFG_BUS_WIDTH_8 0x2 +#define CFG_DDR BIT(2) +#define CFG_BLK_LEN_SHIFT 4 +#define CFG_BLK_LEN_MASK 0xf +#define CFG_RESP_TIMEOUT_SHIFT 8 +#define CFG_RESP_TIMEOUT_MASK 0xf +#define CFG_RC_CC_SHIFT 12 +#define CFG_RC_CC_MASK 0xf +#define CFG_STOP_CLOCK BIT(22) +#define CFG_CLK_ALWAYS_ON BIT(18) +#define CFG_AUTO_CLK BIT(23) + +#define SD_EMMC_STATUS 0x48 +#define STATUS_BUSY BIT(31) + +#define SD_EMMC_IRQ_EN 0x4c +#define IRQ_EN_MASK 0x3fff +#define IRQ_RXD_ERR_SHIFT 0 +#define IRQ_RXD_ERR_MASK 0xff +#define IRQ_TXD_ERR BIT(8) +#define IRQ_DESC_ERR BIT(9) +#define IRQ_RESP_ERR BIT(10) +#define IRQ_RESP_TIMEOUT BIT(11) +#define IRQ_DESC_TIMEOUT BIT(12) +#define IRQ_END_OF_CHAIN BIT(13) +#define IRQ_RESP_STATUS BIT(14) +#define IRQ_SDIO BIT(15) + +#define SD_EMMC_CMD_CFG 0x50 +#define SD_EMMC_CMD_ARG 0x54 +#define SD_EMMC_CMD_DAT 0x58 +#define SD_EMMC_CMD_RSP 0x5c +#define SD_EMMC_CMD_RSP1 0x60 +#define SD_EMMC_CMD_RSP2 0x64 +#define SD_EMMC_CMD_RSP3 0x68 + +#define SD_EMMC_RXD 0x94 +#define SD_EMMC_TXD 0x94 +#define SD_EMMC_LAST_REG SD_EMMC_TXD + +#define SD_EMMC_CFG_BLK_SIZE 512/* internal buffer max: 512 bytes */ +#define SD_EMMC_CFG_RESP_TIMEOUT 256/* in clock cycles */ +#define SD_EMMC_CFG_CMD_GAP 16/* in clock cycles */ +#define MUX_CLK_NUM_PARENTS 2 + +struct meson_host {
drivers/mmc/host/meson-gxbb.c line 165
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+struct device *dev; +struct mmc_host *mmc; +struct mmc_request *mrq; +struct mmc_command *cmd; + +spinlock_t lock; +void __iomem *regs; +#ifdef USE_REGMAP +struct regmap *regmap; +#endif
Dead code?
Quoted 37 lines
drivers/mmc/host/meson-gxbb.c new file +37
-0+int irq; +u32 ocr_mask; +struct clk *core_clk; +struct clk_mux mux; +struct clk *mux_clk; +struct clk *mux_parent[MUX_CLK_NUM_PARENTS ]; +unsigned long mux_parent_rate[MUX_CLK_NUM_PARENTS ]; + +struct clk_divider cfg_div; +struct clk *cfg_div_clk; + +unsigned int bounce_buf_size; +void *bounce_buf; +dma_addr_t bounce_dma_addr; + +unsigned long clk_rate; +unsigned long clk_src_rate; +unsigned short clk_src_div; +}; + +#define reg_read (host, offset) readl(host->regs + offset) +#define reg_write (host, offset, val) writel(val, host->regs + offset) + +struct cmd_cfg { + union { + struct { + u32 length:9; + u32 block_mode:1; + u32 r1b:1; + u32 end_of_chain:1; + u32 timeout:4; /* 2^timeout msec */ + u32 no_resp:1; + u32 no_cmd:1; + u32 data_io:1; + u32 data_wr:1; + u32 resp_nocrc:1; + u32 resp_128:1;
drivers/mmc/host/meson-gxbb.c line 211
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +9
-0+ u32 resp_num:1; + u32 data_num:1; + u32 cmd_index:6; + u32 error:1; + u32 owner:1; +#define CFG_OWNER_CPU 1 +#define CFG_OWNER_MMC 0 + }; +u32 val;
Does this need to be __le32 to handle endian swaps? I thought bitfields weren't safe for things that are written to hardware, but I can't seem to find any definitive email about that anywhere.
drivers/mmc/host/meson-gxbb.c line 219
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c new file +8
-0+ }; +}; + +struct sd_emmc_desc { +struct cmd_cfg cmd_cfg; +u32 cmd_arg; +u32 cmd_data; +u32 cmd_resp;
__le32?
Quoted 51 lines
drivers/mmc/host/meson-gxbb.c new file +51
-0+}; +#define CMD_DATA_MASK (~0x3) +#define CMD_DATA_BIG_ENDIAN BIT(1) +#define CMD_DATA_SRAM BIT(0) +#define CMD_RESP_MASK (~0x1) +#define CMD_RESP_SRAM BIT(0) + +static int meson_mmc_clk_set (struct meson_host *host,unsigned long clk_rate) +{ +struct mmc_host *mmc = host->mmc; +int ret =0 ; +u32 cfg; + +if (clk_rate) { +if (WARN_ON (clk_rate > mmc->f_max)) + clk_rate = mmc->f_max; +else if (WARN_ON (clk_rate < mmc->f_min)) + clk_rate = mmc->f_min; + } + +if (clk_rate == host->clk_rate) +return 0 ; + +/* stop clock */ + cfg =reg_read (host,SD_EMMC_CFG ); +if (!(cfg &CFG_STOP_CLOCK )) { + cfg |=CFG_STOP_CLOCK ; +reg_write (host,SD_EMMC_CFG , cfg); + } + +dev_dbg (host->dev,"change clock rate %lu -> %lu\n" , + host->clk_rate, clk_rate); + ret =clk_set_rate (host->cfg_div_clk, clk_rate); +if (clk_rate && clk_rate !=clk_get_rate (host->cfg_div_clk)) +dev_warn (host->dev,"divider requested rate %lu != actual rate %lu: ret=%d\n" , + clk_rate,clk_get_rate (host->cfg_div_clk), ret); +else + host->clk_rate = clk_rate; + +/* (re)start clock, if non-zero */ +if (clk_rate) { + cfg =reg_read (host,SD_EMMC_CFG ); + cfg &= ~CFG_STOP_CLOCK ; +reg_write (host,SD_EMMC_CFG , cfg); + } + +return ret; +} + +static int meson_mmc_clk_init (struct meson_host *host) +{
drivers/mmc/host/meson-gxbb.c line 280
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+struct clk_init_data init; +char clk_name[32 ]; +int i, ret =0 ; +const char *mux_parent_names[MUX_CLK_NUM_PARENTS ]; +unsigned int mux_parent_count =0 ; +const char *clk_div_parents[1 ]; +unsigned int f_min =UINT_MAX ; +u32 clk_reg, cfg; + +/* get the mux parents from DT */
None of this looks DT specific (good).
Quoted 22 lines
drivers/mmc/host/meson-gxbb.c new file +22
-0+for (i =0 ; i <MUX_CLK_NUM_PARENTS ; i++) { +char name[16 ]; + +snprintf (name,sizeof (name),"clkin%d" , i); + host->mux_parent[i] =devm_clk_get (host->dev, name); +if (IS_ERR (host->mux_parent[i])) { + ret =PTR_ERR (host->mux_parent[i]); +if (PTR_ERR (host->mux_parent[i]) != -EPROBE_DEFER ) +dev_err (host->dev,"Missing clock %s\n" , name); + host->mux_parent[i] =NULL ; +return ret; + } + + host->mux_parent_rate[i] =clk_get_rate (host->mux_parent[i]); + mux_parent_names[i] =__clk_get_name (host->mux_parent[i]); + mux_parent_count++; +if (host->mux_parent_rate[i] < f_min) + f_min = host->mux_parent_rate[i]; + } + +/* cacluate f_min based on input clocks, and max divider value */ +if (f_min !=UINT_MAX )
drivers/mmc/host/meson-gxbb.c line 312
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ f_min =DIV_ROUND_UP (CLK_SRC_XTAL_RATE ,CLK_DIV_MAX ); + else + f_min =4000000 ;/* default min: 400 MHz */ + host->mmc->f_min = f_min; + +/* create the mux */ +snprintf (clk_name,sizeof (clk_name),"%s#mux" ,dev_name (host->dev)); + init.name = clk_name; + init.ops = &clk_mux_ops; + init.flags =CLK_IS_BASIC ;
Do you need CLK_IS_BASIC? If not please drop it.
drivers/mmc/host/meson-gxbb.c line 323
Quoted 11 lines
drivers/mmc/host/meson-gxbb.c new file +11
-0+ init.parent_names = mux_parent_names; + init.num_parents = mux_parent_count; + + host->mux.reg = host->regs +SD_EMMC_CLOCK ; + host->mux.shift =CLK_SRC_SHIFT ; + host->mux.mask =CLK_SRC_MASK ; + host->mux.flags =0 ; + host->mux.table =NULL ; + host->mux.hw.init = &init; + + host->mux_clk =devm_clk_register (host->dev, &host->mux.hw);
Hmm I was hoping to get rid of devm_clk_register() and replace it with devm_clk_hw_register() instead. All part of my grand plan to split providers from consumers, but this driver is different in the sense that it registers clks to provide to itself. Maybe we should make __clk_create_clk() into a real clk provider API so that we can use devm_clk_hw_register() here and then generate a clk for this device from the hw structure.
drivers/mmc/host/meson-gxbb.c line 324
Quoted 1 line
drivers/mmc/host/meson-gxbb.c new file +1
-0+if (WARN_ON (PTR_ERR_OR_ZERO (host->mux_clk)))
NULL is a valid clk so I would expect to only see
WARN_ON(IS_ERR(host->mux_clk)) here.
Quoted 21 lines
drivers/mmc/host/meson-gxbb.c new file +21
-0+return PTR_ERR (host->mux_clk); + +/* create the divider */ +snprintf (clk_name,sizeof (clk_name),"%s#div" ,dev_name (host->dev)); + init.name =devm_kstrdup (host->dev, clk_name,GFP_KERNEL ); + init.ops = &clk_divider_ops; + init.flags =CLK_IS_BASIC |CLK_SET_RATE_PARENT ; + clk_div_parents[0 ] =__clk_get_name (host->mux_clk); + init.parent_names = clk_div_parents; + init.num_parents =ARRAY_SIZE (clk_div_parents); + + host->cfg_div.reg = host->regs +SD_EMMC_CLOCK ; + host->cfg_div.shift =CLK_DIV_SHIFT ; + host->cfg_div.width =CLK_DIV_WIDTH ; + host->cfg_div.hw.init = &init; + host->cfg_div.flags =CLK_DIVIDER_ONE_BASED | +CLK_DIVIDER_ROUND_CLOSEST |CLK_DIVIDER_ALLOW_ZERO ; + + host->cfg_div_clk =devm_clk_register (host->dev, &host->cfg_div.hw); +if (WARN_ON (PTR_ERR_OR_ZERO (host->cfg_div_clk))) +return PTR_ERR (host->cfg_div_clk);
drivers/mmc/host/meson-gxbb.c line 355
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ +/* init SD_EMMC_CLOCK to sane defaults w/min clock rate */ + clk_reg =0 ; + clk_reg |=CLK_PHASE_180 <<CLK_PHASE_SHIFT ; + clk_reg |=CLK_SRC_XTAL <<CLK_SRC_SHIFT ; + clk_reg |=CLK_DIV_MAX <<CLK_DIV_SHIFT ; + clk_reg &= ~CLK_ALWAYS_ON ; +reg_write (host,SD_EMMC_CLOCK , clk_reg); + +clk_prepare_enable (host->cfg_div_clk);
This can fail...
drivers/mmc/host/meson-gxbb.c line 363
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c new file +8
-0+ +/* Ensure clock starts in "auto" mode, not "always on" */ + cfg =reg_read (host,SD_EMMC_CFG ); + cfg &= ~CFG_CLK_ALWAYS_ON ; + cfg |=CFG_AUTO_CLK ; +reg_write (host,SD_EMMC_CFG , cfg); + +meson_mmc_clk_set (host, f_min);
This can fail too but we don't handle the error?
Quoted 73 lines
drivers/mmc/host/meson-gxbb.c new file +73
-0+ +return ret; +} + +static void meson_mmc_set_ios (struct mmc_host *mmc,struct mmc_ios *ios) +{ +struct meson_host *host =mmc_priv (mmc); +u32 bus_width; +u32 val, orig; + +/* +* GPIO regulator, only controls switching between 1v8 and +* 3v3, doesn't support MMC_POWER_OFF, MMC_POWER_ON. +*/ +switch (ios->power_mode) { +case MMC_POWER_UP : +if (!IS_ERR (mmc->supply.vmmc)) +mmc_regulator_set_ocr (mmc, mmc->supply.vmmc, ios->vdd); + } + +meson_mmc_clk_set (host, ios->clock); + +/* Bus width */ + val =reg_read (host,SD_EMMC_CFG ); +switch (ios->bus_width) { +case MMC_BUS_WIDTH_1 : + bus_width =CFG_BUS_WIDTH_1 ; +break ; +case MMC_BUS_WIDTH_4 : + bus_width =CFG_BUS_WIDTH_4 ; +break ; +case MMC_BUS_WIDTH_8 : + bus_width =CFG_BUS_WIDTH_8 ; +break ; +default : +dev_err (host->dev,"Invalid ios->bus_width: %u. Setting to 4.\n" , + ios->bus_width); + bus_width =CFG_BUS_WIDTH_4 ; +return ; + } + + val =reg_read (host,SD_EMMC_CFG ); + orig = val; + + val &= ~(CFG_BUS_WIDTH_MASK <<CFG_BUS_WIDTH_SHIFT ); + val |= bus_width <<CFG_BUS_WIDTH_SHIFT ; + + val &= ~(CFG_BLK_LEN_MASK <<CFG_BLK_LEN_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_BLK_SIZE ) <<CFG_BLK_LEN_SHIFT ; + + val &= ~(CFG_RESP_TIMEOUT_MASK <<CFG_RESP_TIMEOUT_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_RESP_TIMEOUT ) <<CFG_RESP_TIMEOUT_SHIFT ; + + val &= ~(CFG_RC_CC_MASK <<CFG_RC_CC_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_CMD_GAP ) <<CFG_RC_CC_SHIFT ; + +reg_write (host,SD_EMMC_CFG , val); + +if (val != orig) +dev_dbg (host->dev,"%s: SD_EMMC_CFG: 0x%08x -> 0x%08x\n" , + __func__, orig, val); +} + +static int meson_mmc_wait_busy (struct mmc_host *mmc,unsigned int timeout) +{ +struct meson_host *host =mmc_priv (mmc); +u32 status; +int i; + +for (i = timeout; i >0 ; i--) { + status =reg_read (host,SD_EMMC_STATUS ); +if (!(status &STATUS_BUSY )) +break ;
drivers/mmc/host/meson-gxbb.c line 446
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ }; + +return (timeout ==0 ); +} + +static int meson_mmc_request_done (struct mmc_host *mmc,struct mmc_request *mrq) +{ +struct meson_host *host =mmc_priv (mmc); +struct mmc_command *cmd = host->cmd; +unsigned int loops =0xfffff ;
Perhaps this should be a value of time instead of raw loops? Who knows how fast the CPU can run all those loops?
Quoted 13 lines
drivers/mmc/host/meson-gxbb.c new file +13
-0+ +WARN_ON (host->mrq != mrq); +if (cmd && !cmd->error) +if (meson_mmc_wait_busy (mmc, loops)) +dev_warn (host->dev,"%s: timeout busy.\n" , __func__); + + host->mrq =NULL ; + host->cmd =NULL ; +mmc_request_done (host->mmc, mrq); + +return 0 ; +} +
[...]
Quoted 21 lines
drivers/mmc/host/meson-gxbb.c new file +21
-0+ +static void meson_mmc_request (struct mmc_host *mmc,struct mmc_request *mrq) +{ +struct meson_host *host =mmc_priv (mmc); + +WARN_ON (host->mrq !=NULL ); + +/* Stop execution */ +reg_write (host,SD_EMMC_START ,0 ); + +/* clear, ack, enable all interrupts */ +reg_write (host,SD_EMMC_IRQ_EN ,0 ); +reg_write (host,SD_EMMC_STATUS ,IRQ_EN_MASK ); +reg_write (host,SD_EMMC_IRQ_EN ,IRQ_EN_MASK ); + + host->mrq = mrq; + +if (meson_mmc_check_cmd (mmc, mrq->cmd)) +return ; + +if (mrq->sbc)
drivers/mmc/host/meson-gxbb.c line 609
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+meson_mmc_start_cmd (mmc, mrq->sbc); + else + meson_mmc_start_cmd(mmc, mrq->cmd); +} + +static int meson_mmc_read_resp (struct mmc_host *mmc,struct mmc_command *cmd) +{ +struct meson_host *host =mmc_priv (mmc); + +if (!cmd) {
This happens?
Quoted 22 lines
drivers/mmc/host/meson-gxbb.c new file +22
-0+dev_dbg (host->dev,"%s: NULL command.\n" , __func__); +return -EINVAL ; + } + +if (cmd->flags &MMC_RSP_136 ) { + cmd->resp[0 ] =reg_read (host,SD_EMMC_CMD_RSP3 ); + cmd->resp[1 ] =reg_read (host,SD_EMMC_CMD_RSP2 ); + cmd->resp[2 ] =reg_read (host,SD_EMMC_CMD_RSP1 ); + cmd->resp[3 ] =reg_read (host,SD_EMMC_CMD_RSP ); + }else if (cmd->flags &MMC_RSP_PRESENT ) { + cmd->resp[0 ] =reg_read (host,SD_EMMC_CMD_RSP ); + } + +return 0 ; +} + +static irqreturn_t meson_mmc_irq (int irq,void *dev_id) +{ +struct meson_host *host = dev_id; +struct mmc_request *mrq = host->mrq; +struct mmc_command *cmd = host->cmd; +u32 irq_en, status, raw_status;
drivers/mmc/host/meson-gxbb.c line 641
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+irqreturn_t ret =IRQ_HANDLED ; + +if (WARN_ON (!host)) +return IRQ_NONE ; + +if (WARN_ON (!mrq)) +return IRQ_NONE ; + +if (WARN_ON (!cmd)) +return IRQ_NONE ;
These can happen? Spurious interrupts abound? Ugh.
Quoted 65 lines
drivers/mmc/host/meson-gxbb.c new file +65
-0+ +spin_lock (&host->lock); + irq_en =reg_read (host,SD_EMMC_IRQ_EN ); + raw_status =reg_read (host,SD_EMMC_STATUS ); + status = raw_status & irq_en; + +if (!status) { +dev_warn (host->dev,"Spurious IRQ! status=0x%08x, irq_en=0x%08x\n" , + raw_status, irq_en); + ret =IRQ_NONE ; + goto out; + } + + cmd->error =0 ; +if (status &IRQ_RXD_ERR_MASK ) { +dev_dbg (host->dev,"Unhandled IRQ: RXD error\n" ); + cmd->error = -EILSEQ ; + } +if (status &IRQ_TXD_ERR ) { +dev_dbg (host->dev,"Unhandled IRQ: TXD error\n" ); + cmd->error = -EILSEQ ; + } +if (status &IRQ_DESC_ERR ) +dev_dbg (host->dev,"Unhandled IRQ: Descriptor error\n" ); +if (status &IRQ_RESP_ERR ) { +dev_dbg (host->dev,"Unhandled IRQ: Response error\n" ); + cmd->error = -EILSEQ ; + } +if (status &IRQ_RESP_TIMEOUT ) { +dev_dbg (host->dev,"Unhandled IRQ: Response timeout\n" ); + cmd->error = -ETIMEDOUT ; + } +if (status &IRQ_DESC_TIMEOUT ) { +dev_dbg (host->dev,"Unhandled IRQ: Descriptor timeout\n" ); + cmd->error = -ETIMEDOUT ; + } +if (status &IRQ_SDIO ) +dev_dbg (host->dev,"Unhandled IRQ: SDIO.\n" ); + +if (status & (IRQ_END_OF_CHAIN |IRQ_RESP_STATUS )) + ret =IRQ_WAKE_THREAD ; +else { +dev_warn (host->dev,"Unknown IRQ! status=0x%04x: MMC CMD%u arg=0x%08x flags=0x%08x stop=%d\n" , + status, cmd->opcode, cmd->arg, + cmd->flags, mrq->stop ?1 :0 ); +if (cmd->data) { +struct mmc_data *data = cmd->data; + +dev_warn (host->dev,"\tblksz %u blocks %u flags 0x%08x (%s%s)" , + data->blksz, data->blocks, data->flags, + data->flags &MMC_DATA_WRITE ?"write" :"" , + data->flags &MMC_DATA_READ ?"read" :"" ); + } + } + +out: +/* ack all (enabled) interrupts */ +reg_write (host,SD_EMMC_STATUS , status); + +if (ret ==IRQ_HANDLED ) { +meson_mmc_read_resp (host->mmc, cmd); +meson_mmc_request_done (host->mmc, cmd->mrq); + } + +spin_unlock (&host->lock);
drivers/mmc/host/meson-gxbb.c line 716
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+return ret; +} + +static irqreturn_t meson_mmc_irq_thread (int irq,void *dev_id) +{ +struct meson_host *host = dev_id; +struct mmc_request *mrq = host->mrq; +struct mmc_command *cmd = host->cmd; +struct mmc_data *data; +
Drop that newline?
Quoted 34 lines
drivers/mmc/host/meson-gxbb.c new file +34
-0+unsigned int xfer_bytes; +int ret =IRQ_HANDLED ; + +if (WARN_ON (!mrq)) + ret =IRQ_NONE ; + +if (WARN_ON (!cmd)) + ret =IRQ_NONE ; + + data = cmd->data; +if (data) { + xfer_bytes = data->blksz * data->blocks; +if (data->flags &MMC_DATA_READ ) { +WARN_ON (xfer_bytes > host->bounce_buf_size); +sg_copy_from_buffer (data->sg, data->sg_len, + host->bounce_buf, xfer_bytes); + data->bytes_xfered = xfer_bytes; + } + } + +meson_mmc_read_resp (host->mmc, cmd); +if (!data || !data->stop || mrq->sbc) +meson_mmc_request_done (host->mmc, mrq); +else +meson_mmc_start_cmd (host->mmc, data->stop); + +return ret; +} + +/* +* NOTE: we only need this until the GPIO/pinctrl driver can handle +* interrupts. For now, the MMC core will use this for polling. +*/ +static int meson_mmc_get_cd (struct mmc_host *mmc)
drivers/mmc/host/meson-gxbb.c line 760
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+{ +int status =mmc_gpio_get_cd (mmc); + +if (status == -ENOSYS ) +return 1 ;/* assume present */ + +return status; +} + +static struct mmc_host_ops meson_mmc_ops = {
const?
Quoted 47 lines
drivers/mmc/host/meson-gxbb.c new file +47
-0+ .request = meson_mmc_request, + .set_ios = meson_mmc_set_ios, + .get_cd = meson_mmc_get_cd, +}; + +static int meson_mmc_probe (struct platform_device *pdev) +{ +struct device_node *np = pdev->dev.of_node; +struct resource *res; +struct meson_host *host; +struct mmc_host *mmc; +int ret; + + mmc =mmc_alloc_host (sizeof (struct meson_host ), &pdev->dev); +if (!mmc) +return -ENOMEM ; + host =mmc_priv (mmc); + host->mmc = mmc; + host->dev = &pdev->dev; +dev_set_drvdata (&pdev->dev, host); + +spin_lock_init (&host->lock); + + host->core_clk =devm_clk_get (&pdev->dev,"core" ); +if (IS_ERR (host->core_clk)) { + ret =PTR_ERR (host->core_clk); +if (ret == -EPROBE_DEFER ) +dev_dbg (&pdev->dev, +"Missing core clock. EPROBE_DEFER\n" ); +else +dev_err (&pdev->dev, +"Unable to get core clk (ret=%d).\n" , ret); + goto free_host; + } + +/* Voltage supply */ +mmc_of_parse_voltage (np, &host->ocr_mask); + ret =mmc_regulator_get_supply (mmc); +if (ret == -EPROBE_DEFER ) { +dev_dbg (&pdev->dev,"Missing regulator: EPROBE_DEFER" ); + goto free_host; + } + +if (!mmc->ocr_avail) + mmc->ocr_avail = host->ocr_mask; + + ret =mmc_of_parse (mmc);
drivers/mmc/host/meson-gxbb.c line 817
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+if (ret) { +dev_warn (&pdev->dev,"error parsing DT: %d\n" , ret); + goto free_host; + } + + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); +if (!res) { + ret = -ENODEV ; + goto free_host; + }
This if can be dropped and go right into devm_ioremap_resource().
Quoted 21 lines
drivers/mmc/host/meson-gxbb.c new file +21
-0+ host->regs =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (host->regs)) { + ret =PTR_ERR (host->regs); + goto free_host; + } + + host->irq =platform_get_irq (pdev,0 ); +if (host->irq ==0 ) { +dev_err (&pdev->dev,"failed to get interrupt resource.\n" ); + ret = -EINVAL ; + goto free_host; + } + + ret =devm_request_threaded_irq (&pdev->dev, host->irq, + meson_mmc_irq, meson_mmc_irq_thread, +IRQF_SHARED ,DRIVER_NAME , host); +if (ret) + goto free_host; + +/* data bounce buffer */ + host->bounce_buf_size =SZ_512K ;
drivers/mmc/host/meson-gxbb.c line 848
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ host->bounce_buf = +dma_alloc_coherent (host->dev, host->bounce_buf_size, + &host->bounce_dma_addr,GFP_KERNEL ); +if (host->bounce_buf ==NULL ) { +dev_err (host->dev,"Unable to map allocate DMA bounce buffer.\n" ); + ret = -ENOMEM ; + goto free_host; + } + +clk_prepare_enable (host->core_clk);
This can fail.
drivers/mmc/host/meson-gxbb.c line 859
Quoted 11 lines
drivers/mmc/host/meson-gxbb.c new file +11
-0+ + ret =meson_mmc_clk_init (host); +if (ret) + goto free_host; + +/* Stop execution */ +reg_write (host,SD_EMMC_START ,0 ); + +/* clear, ack, enable all interrupts */ +reg_write (host,SD_EMMC_IRQ_EN ,0 ); +reg_write (host,SD_EMMC_STATUS ,IRQ_EN_MASK );
Shouldn't we do this before requesting the interrupt?
Quoted 14 lines
drivers/mmc/host/meson-gxbb.c new file +14
-0+ + mmc->ops = &meson_mmc_ops; +mmc_add_host (mmc); + +return 0 ; + +free_host: +dev_dbg (host->dev,"Failed to probe: ret=%d\n" , ret); +if (host->core_clk) +clk_disable_unprepare (host->core_clk); +mmc_free_host (mmc); +return ret; +} +
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Kevin Hilman
maintainerPATCH v2 2/2 MMC: meson: initial support for GXBB platforms
Quoted 263 lines · Stephen Boyd
MAINTAINERS F: arch/arm/mach-meson/ · line 975
Quoted 9 lines · Kevin Hilman
MAINTAINERS +1
-0@@ -972,6 +972,7 @@ F: arch/arm/mach-meson/ F: arch/arm/boot/dts/meson* F: arch/arm64/boot/dts/amlogic/ F: drivers/pinctrl/meson/ +F: drivers/mmc/host/meson*Weird spacing here?
Quoted 8 lines
MAINTAINERS +0
-0N: meson ARM/Annapurna Labs ALPINE ARCHITECTUREdrivers/mmc/host/Kconfig +0
-0drivers/mmc/host/Kconfig config MMC_SDHCI_IPROC · line 341
Quoted 10 lines
drivers/mmc/host/Kconfig +7
-0@@ -332,6 +332,16 @@ config MMC_SDHCI_IPROC If unsure, say N. +config MMC_MESON_GXBB + tristate "Amlogic S905/GXBB SD/MMC Host Controller support" + depends on ARCH_MESON && MMC + help + This selects support for the Amlogic SD/MMC Host Controller + found on the S905/GXBB family of SoCs. This controller is + MMC 5.1 compliant and support SD, eMMC and SDIO interfaces.s/support/supports/
Quoted 59 lines
drivers/mmc/host/Kconfig +3
-0+ + If you have a controller with this interface, say Y here. + config MMC_MOXART tristate "MOXART SD/MMC Host Controller support" depends on ARCH_MOXART && MMCdrivers/mmc/host/meson-gxbb.c new file +47
-0@@ -0,0 +1,918 @@ +/* +* This file is provided under a dual BSD/GPLv2 license. When using or +* redistributing this file, you may do so under either license. +* +* GPL LICENSE SUMMARY +* +* Copyright (c) 2016 BayLibre, SAS. +* Author: Kevin Hilman <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of version 2 of the GNU General Public License as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +* The full GNU General Public License is included in this distribution +* in the file called COPYING. +* +* BSD LICENSE +* +* Copyright (c) 2016 BayLibre, SAS. +* Author: Kevin Hilman <[email protected]> +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions +* are met: +* +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in +* the documentation and/or other materials provided with the +* distribution. +* * Neither the name of Intel Corporation nor the names of its +* contributors may be used to endorse or promote products derived +* from this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, drivers/mmc/host/meson-gxbb.c line 57
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/moduleparam.h> Unused?
drivers/mmc/host/meson-gxbb.c line 63
Quoted 6 lines
drivers/mmc/host/meson-gxbb.c new file +6
-0+#include <linux/init.h> +#include <linux/device.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/ioport.h> +#include <linux/regmap.h> Unused?
Quoted 92 lines
drivers/mmc/host/meson-gxbb.c new file +92
-0+#include <linux/spinlock.h> +#include <linux/dma-mapping.h> +#include <linux/mmc/host.h> +#include <linux/mmc/mmc.h> +#include <linux/mmc/sdio.h> +#include <linux/mmc/slot-gpio.h> +#include <linux/io.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> + +#define DRIVER_NAME "meson-gxbb-mmc" + +#define SD_EMMC_CLOCK 0x0 +#define CLK_DIV_SHIFT 0 +#define CLK_DIV_WIDTH 6 +#define CLK_DIV_MASK 0x3f +#define CLK_DIV_MAX 63 +#define CLK_SRC_SHIFT 6 +#define CLK_SRC_WIDTH 2 +#define CLK_SRC_MASK 0x3 +#define CLK_SRC_XTAL 0/* external crystal */ +#define CLK_SRC_XTAL_RATE 24000000 +#define CLK_SRC_PLL 1/* FCLK_DIV2 */ +#define CLK_SRC_PLL_RATE 1000000000 +#define CLK_PHASE_SHIFT 8 +#define CLK_PHASE_MASK 0x3 +#define CLK_PHASE_0 0 +#define CLK_PHASE_90 1 +#define CLK_PHASE_180 2 +#define CLK_PHASE_270 3 +#define CLK_ALWAYS_ON BIT(24) + +#define SD_EMMC_DElAY 0x4 +#define SD_EMMC_ADJUST 0x8 +#define SD_EMMC_CALOUT 0x10 +#define SD_EMMC_START 0x40 +#define START_DESC_INIT BIT(0) +#define START_DESC_BUSY BIT(1) +#define START_DESC_ADDR_SHIFT 2 +#define START_DESC_ADDR_MASK (~0x3) + +#define SD_EMMC_CFG 0x44 +#define CFG_BUS_WIDTH_SHIFT 0 +#define CFG_BUS_WIDTH_MASK 0x3 +#define CFG_BUS_WIDTH_1 0x0 +#define CFG_BUS_WIDTH_4 0x1 +#define CFG_BUS_WIDTH_8 0x2 +#define CFG_DDR BIT(2) +#define CFG_BLK_LEN_SHIFT 4 +#define CFG_BLK_LEN_MASK 0xf +#define CFG_RESP_TIMEOUT_SHIFT 8 +#define CFG_RESP_TIMEOUT_MASK 0xf +#define CFG_RC_CC_SHIFT 12 +#define CFG_RC_CC_MASK 0xf +#define CFG_STOP_CLOCK BIT(22) +#define CFG_CLK_ALWAYS_ON BIT(18) +#define CFG_AUTO_CLK BIT(23) + +#define SD_EMMC_STATUS 0x48 +#define STATUS_BUSY BIT(31) + +#define SD_EMMC_IRQ_EN 0x4c +#define IRQ_EN_MASK 0x3fff +#define IRQ_RXD_ERR_SHIFT 0 +#define IRQ_RXD_ERR_MASK 0xff +#define IRQ_TXD_ERR BIT(8) +#define IRQ_DESC_ERR BIT(9) +#define IRQ_RESP_ERR BIT(10) +#define IRQ_RESP_TIMEOUT BIT(11) +#define IRQ_DESC_TIMEOUT BIT(12) +#define IRQ_END_OF_CHAIN BIT(13) +#define IRQ_RESP_STATUS BIT(14) +#define IRQ_SDIO BIT(15) + +#define SD_EMMC_CMD_CFG 0x50 +#define SD_EMMC_CMD_ARG 0x54 +#define SD_EMMC_CMD_DAT 0x58 +#define SD_EMMC_CMD_RSP 0x5c +#define SD_EMMC_CMD_RSP1 0x60 +#define SD_EMMC_CMD_RSP2 0x64 +#define SD_EMMC_CMD_RSP3 0x68 + +#define SD_EMMC_RXD 0x94 +#define SD_EMMC_TXD 0x94 +#define SD_EMMC_LAST_REG SD_EMMC_TXD + +#define SD_EMMC_CFG_BLK_SIZE 512/* internal buffer max: 512 bytes */ +#define SD_EMMC_CFG_RESP_TIMEOUT 256/* in clock cycles */ +#define SD_EMMC_CFG_CMD_GAP 16/* in clock cycles */ +#define MUX_CLK_NUM_PARENTS 2 + +struct meson_host {drivers/mmc/host/meson-gxbb.c line 165
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+struct device *dev; +struct mmc_host *mmc; +struct mmc_request *mrq; +struct mmc_command *cmd; + +spinlock_t lock; +void __iomem *regs; +#ifdef USE_REGMAP +struct regmap *regmap; +#endif Dead code?
Quoted 42 lines
drivers/mmc/host/meson-gxbb.c new file +42
-0+int irq; +u32 ocr_mask; +struct clk *core_clk; +struct clk_mux mux; +struct clk *mux_clk; +struct clk *mux_parent[MUX_CLK_NUM_PARENTS ]; +unsigned long mux_parent_rate[MUX_CLK_NUM_PARENTS ]; + +struct clk_divider cfg_div; +struct clk *cfg_div_clk; + +unsigned int bounce_buf_size; +void *bounce_buf; +dma_addr_t bounce_dma_addr; + +unsigned long clk_rate; +unsigned long clk_src_rate; +unsigned short clk_src_div; +}; + +#define reg_read (host, offset) readl(host->regs + offset) +#define reg_write (host, offset, val) writel(val, host->regs + offset) + +struct cmd_cfg { + union { + struct { + u32 length:9; + u32 block_mode:1; + u32 r1b:1; + u32 end_of_chain:1; + u32 timeout:4; /* 2^timeout msec */ + u32 no_resp:1; + u32 no_cmd:1; + u32 data_io:1; + u32 data_wr:1; + u32 resp_nocrc:1; + u32 resp_128:1; + u32 resp_num:1; + u32 data_num:1; + u32 cmd_index:6; + u32 error:1; + u32 owner:1;
Quoted 9 lines
drivers/mmc/host/meson-gxbb.c line 211
Quoted 4 lines
drivers/mmc/host/meson-gxbb.c new file +4
-0+#define CFG_OWNER_CPU 1 +#define CFG_OWNER_MMC 0 + }; +u32 val;Does this need to be
__le32to handle endian swaps? I thought bitfields weren't safe for things that are written to hardware, but I can't seem to find any definitive email about that anywhere.
Hmm, yeah. I need to rework and test this for CONFIG_CPU_BIG_ENDIAN=y. I think I'll completely get rid of the bitfields.
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c line 219
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c new file +8
-0+ }; +}; + +struct sd_emmc_desc { +struct cmd_cfg cmd_cfg; +u32 cmd_arg; +u32 cmd_data; +u32 cmd_resp;
__le32?
Ack.
Quoted 55 lines
Quoted 55 lines
drivers/mmc/host/meson-gxbb.c new file +55
-0+}; +#define CMD_DATA_MASK (~0x3) +#define CMD_DATA_BIG_ENDIAN BIT(1) +#define CMD_DATA_SRAM BIT(0) +#define CMD_RESP_MASK (~0x1) +#define CMD_RESP_SRAM BIT(0) + +static int meson_mmc_clk_set (struct meson_host *host,unsigned long clk_rate) +{ +struct mmc_host *mmc = host->mmc; +int ret =0 ; +u32 cfg; + +if (clk_rate) { +if (WARN_ON (clk_rate > mmc->f_max)) + clk_rate = mmc->f_max; +else if (WARN_ON (clk_rate < mmc->f_min)) + clk_rate = mmc->f_min; + } + +if (clk_rate == host->clk_rate) +return 0 ; + +/* stop clock */ + cfg =reg_read (host,SD_EMMC_CFG ); +if (!(cfg &CFG_STOP_CLOCK )) { + cfg |=CFG_STOP_CLOCK ; +reg_write (host,SD_EMMC_CFG , cfg); + } + +dev_dbg (host->dev,"change clock rate %lu -> %lu\n" , + host->clk_rate, clk_rate); + ret =clk_set_rate (host->cfg_div_clk, clk_rate); +if (clk_rate && clk_rate !=clk_get_rate (host->cfg_div_clk)) +dev_warn (host->dev,"divider requested rate %lu != actual rate %lu: ret=%d\n" , + clk_rate,clk_get_rate (host->cfg_div_clk), ret); +else + host->clk_rate = clk_rate; + +/* (re)start clock, if non-zero */ +if (clk_rate) { + cfg =reg_read (host,SD_EMMC_CFG ); + cfg &= ~CFG_STOP_CLOCK ; +reg_write (host,SD_EMMC_CFG , cfg); + } + +return ret; +} + +static int meson_mmc_clk_init (struct meson_host *host) +{ +struct clk_init_data init; +char clk_name[32 ]; +int i, ret =0 ; +const char *mux_parent_names[MUX_CLK_NUM_PARENTS ];
Quoted 9 lines
drivers/mmc/host/meson-gxbb.c line 280
Quoted 6 lines
drivers/mmc/host/meson-gxbb.c new file +6
-0+unsigned int mux_parent_count =0 ; +const char *clk_div_parents[1 ]; +unsigned int f_min =UINT_MAX ; +u32 clk_reg, cfg; + +/* get the mux parents from DT */ None of this looks DT specific (good).
Ok, will drop the "from DT" part of the comment.
Quoted 26 lines
Quoted 26 lines
drivers/mmc/host/meson-gxbb.c new file +26
-0+for (i =0 ; i <MUX_CLK_NUM_PARENTS ; i++) { +char name[16 ]; + +snprintf (name,sizeof (name),"clkin%d" , i); + host->mux_parent[i] =devm_clk_get (host->dev, name); +if (IS_ERR (host->mux_parent[i])) { + ret =PTR_ERR (host->mux_parent[i]); +if (PTR_ERR (host->mux_parent[i]) != -EPROBE_DEFER ) +dev_err (host->dev,"Missing clock %s\n" , name); + host->mux_parent[i] =NULL ; +return ret; + } + + host->mux_parent_rate[i] =clk_get_rate (host->mux_parent[i]); + mux_parent_names[i] =__clk_get_name (host->mux_parent[i]); + mux_parent_count++; +if (host->mux_parent_rate[i] < f_min) + f_min = host->mux_parent_rate[i]; + } + +/* cacluate f_min based on input clocks, and max divider value */ +if (f_min !=UINT_MAX ) + f_min =DIV_ROUND_UP (CLK_SRC_XTAL_RATE ,CLK_DIV_MAX ); +else + f_min =4000000 ;/* default min: 400 MHz */ + host->mmc->f_min = f_min;
Quoted 9 lines
drivers/mmc/host/meson-gxbb.c line 312
Quoted 6 lines
drivers/mmc/host/meson-gxbb.c new file +6
-0+ +/* create the mux */ +snprintf (clk_name,sizeof (clk_name),"%s#mux" ,dev_name (host->dev)); + init.name = clk_name; + init.ops = &clk_mux_ops; + init.flags =CLK_IS_BASIC ;Do you need
CLK_IS_BASIC? If not please drop it.
Dropped.
Quoted 12 lines
drivers/mmc/host/meson-gxbb.c line 323
Quoted 11 lines
drivers/mmc/host/meson-gxbb.c new file +11
-0+ init.parent_names = mux_parent_names; + init.num_parents = mux_parent_count; + + host->mux.reg = host->regs +SD_EMMC_CLOCK ; + host->mux.shift =CLK_SRC_SHIFT ; + host->mux.mask =CLK_SRC_MASK ; + host->mux.flags =0 ; + host->mux.table =NULL ; + host->mux.hw.init = &init; + + host->mux_clk =devm_clk_register (host->dev, &host->mux.hw);
Quoted 7 lines
Hmm I was hoping to get rid of
devm_clk_register()and replace it withdevm_clk_hw_register()instead. All part of my grand plan to split providers from consumers, but this driver is different in the sense that it registers clks to provide to itself. Maybe we should make__clk_create_clk()into a real clk provider API so that we can usedevm_clk_hw_register()here and then generate a clk for this device from the hw structure.
So is there something else I should do here, or can we rework this later after you rework the framework a bit?
Quoted 4 lines
drivers/mmc/host/meson-gxbb.c line 324
Quoted 1 line
drivers/mmc/host/meson-gxbb.c new file +1
-0+if (WARN_ON (PTR_ERR_OR_ZERO (host->mux_clk)))NULL is a valid clk so I would expect to only see
WARN_ON(IS_ERR(host->mux_clk)) here.
OK.
Quoted 34 lines
Quoted 21 lines
drivers/mmc/host/meson-gxbb.c new file +21
-0+return PTR_ERR (host->mux_clk); + +/* create the divider */ +snprintf (clk_name,sizeof (clk_name),"%s#div" ,dev_name (host->dev)); + init.name =devm_kstrdup (host->dev, clk_name,GFP_KERNEL ); + init.ops = &clk_divider_ops; + init.flags =CLK_IS_BASIC |CLK_SET_RATE_PARENT ; + clk_div_parents[0 ] =__clk_get_name (host->mux_clk); + init.parent_names = clk_div_parents; + init.num_parents =ARRAY_SIZE (clk_div_parents); + + host->cfg_div.reg = host->regs +SD_EMMC_CLOCK ; + host->cfg_div.shift =CLK_DIV_SHIFT ; + host->cfg_div.width =CLK_DIV_WIDTH ; + host->cfg_div.hw.init = &init; + host->cfg_div.flags =CLK_DIVIDER_ONE_BASED | +CLK_DIVIDER_ROUND_CLOSEST |CLK_DIVIDER_ALLOW_ZERO ; + + host->cfg_div_clk =devm_clk_register (host->dev, &host->cfg_div.hw); +if (WARN_ON (PTR_ERR_OR_ZERO (host->cfg_div_clk))) +return PTR_ERR (host->cfg_div_clk);drivers/mmc/host/meson-gxbb.c line 355
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ +/* init SD_EMMC_CLOCK to sane defaults w/min clock rate */ + clk_reg =0 ; + clk_reg |=CLK_PHASE_180 <<CLK_PHASE_SHIFT ; + clk_reg |=CLK_SRC_XTAL <<CLK_SRC_SHIFT ; + clk_reg |=CLK_DIV_MAX <<CLK_DIV_SHIFT ; + clk_reg &= ~CLK_ALWAYS_ON ; +reg_write (host,SD_EMMC_CLOCK , clk_reg); + +clk_prepare_enable (host->cfg_div_clk);This can fail...
Quoted 11 lines
drivers/mmc/host/meson-gxbb.c line 363
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c new file +8
-0+ +/* Ensure clock starts in "auto" mode, not "always on" */ + cfg =reg_read (host,SD_EMMC_CFG ); + cfg &= ~CFG_CLK_ALWAYS_ON ; + cfg |=CFG_AUTO_CLK ; +reg_write (host,SD_EMMC_CFG , cfg); + +meson_mmc_clk_set (host, f_min);This can fail too but we don't handle the error?
Ok, fixing the error checking.
Quoted 78 lines
Quoted 78 lines
drivers/mmc/host/meson-gxbb.c new file +78
-0+ +return ret; +} + +static void meson_mmc_set_ios (struct mmc_host *mmc,struct mmc_ios *ios) +{ +struct meson_host *host =mmc_priv (mmc); +u32 bus_width; +u32 val, orig; + +/* +* GPIO regulator, only controls switching between 1v8 and +* 3v3, doesn't support MMC_POWER_OFF, MMC_POWER_ON. +*/ +switch (ios->power_mode) { +case MMC_POWER_UP : +if (!IS_ERR (mmc->supply.vmmc)) +mmc_regulator_set_ocr (mmc, mmc->supply.vmmc, ios->vdd); + } + +meson_mmc_clk_set (host, ios->clock); + +/* Bus width */ + val =reg_read (host,SD_EMMC_CFG ); +switch (ios->bus_width) { +case MMC_BUS_WIDTH_1 : + bus_width =CFG_BUS_WIDTH_1 ; +break ; +case MMC_BUS_WIDTH_4 : + bus_width =CFG_BUS_WIDTH_4 ; +break ; +case MMC_BUS_WIDTH_8 : + bus_width =CFG_BUS_WIDTH_8 ; +break ; +default : +dev_err (host->dev,"Invalid ios->bus_width: %u. Setting to 4.\n" , + ios->bus_width); + bus_width =CFG_BUS_WIDTH_4 ; +return ; + } + + val =reg_read (host,SD_EMMC_CFG ); + orig = val; + + val &= ~(CFG_BUS_WIDTH_MASK <<CFG_BUS_WIDTH_SHIFT ); + val |= bus_width <<CFG_BUS_WIDTH_SHIFT ; + + val &= ~(CFG_BLK_LEN_MASK <<CFG_BLK_LEN_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_BLK_SIZE ) <<CFG_BLK_LEN_SHIFT ; + + val &= ~(CFG_RESP_TIMEOUT_MASK <<CFG_RESP_TIMEOUT_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_RESP_TIMEOUT ) <<CFG_RESP_TIMEOUT_SHIFT ; + + val &= ~(CFG_RC_CC_MASK <<CFG_RC_CC_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_CMD_GAP ) <<CFG_RC_CC_SHIFT ; + +reg_write (host,SD_EMMC_CFG , val); + +if (val != orig) +dev_dbg (host->dev,"%s: SD_EMMC_CFG: 0x%08x -> 0x%08x\n" , + __func__, orig, val); +} + +static int meson_mmc_wait_busy (struct mmc_host *mmc,unsigned int timeout) +{ +struct meson_host *host =mmc_priv (mmc); +u32 status; +int i; + +for (i = timeout; i >0 ; i--) { + status =reg_read (host,SD_EMMC_STATUS ); +if (!(status &STATUS_BUSY )) +break ; + }; + +return (timeout ==0 ); +} +
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c line 446
Quoted 5 lines
drivers/mmc/host/meson-gxbb.c new file +5
-0+static int meson_mmc_request_done (struct mmc_host *mmc,struct mmc_request *mrq) +{ +struct meson_host *host =mmc_priv (mmc); +struct mmc_command *cmd = host->cmd; +unsigned int loops =0xfffff ;Perhaps this should be a value of time instead of raw loops? Who knows how fast the CPU can run all those loops?
Ack.
Quoted 39 lines
Quoted 13 lines
drivers/mmc/host/meson-gxbb.c new file +13
-0+ +WARN_ON (host->mrq != mrq); +if (cmd && !cmd->error) +if (meson_mmc_wait_busy (mmc, loops)) +dev_warn (host->dev,"%s: timeout busy.\n" , __func__); + + host->mrq =NULL ; + host->cmd =NULL ; +mmc_request_done (host->mmc, mrq); + +return 0 ; +} +[...]
Quoted 25 lines
drivers/mmc/host/meson-gxbb.c new file +25
-0+ +static void meson_mmc_request (struct mmc_host *mmc,struct mmc_request *mrq) +{ +struct meson_host *host =mmc_priv (mmc); + +WARN_ON (host->mrq !=NULL ); + +/* Stop execution */ +reg_write (host,SD_EMMC_START ,0 ); + +/* clear, ack, enable all interrupts */ +reg_write (host,SD_EMMC_IRQ_EN ,0 ); +reg_write (host,SD_EMMC_STATUS ,IRQ_EN_MASK ); +reg_write (host,SD_EMMC_IRQ_EN ,IRQ_EN_MASK ); + + host->mrq = mrq; + +if (meson_mmc_check_cmd (mmc, mrq->cmd)) +return ; + +if (mrq->sbc) +meson_mmc_start_cmd (mmc, mrq->sbc); +else +meson_mmc_start_cmd (mmc, mrq->cmd); +}
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c line 609
Quoted 6 lines
drivers/mmc/host/meson-gxbb.c new file +6
-0+ +static int meson_mmc_read_resp (struct mmc_host *mmc,struct mmc_command *cmd) +{ +struct meson_host *host =mmc_priv (mmc); + +if (!cmd) {This happens?
Hmm, nope, it's prevented elsewhere. I'll remove.
Quoted 26 lines
Quoted 26 lines
drivers/mmc/host/meson-gxbb.c new file +26
-0+dev_dbg (host->dev,"%s: NULL command.\n" , __func__); +return -EINVAL ; + } + +if (cmd->flags &MMC_RSP_136 ) { + cmd->resp[0 ] =reg_read (host,SD_EMMC_CMD_RSP3 ); + cmd->resp[1 ] =reg_read (host,SD_EMMC_CMD_RSP2 ); + cmd->resp[2 ] =reg_read (host,SD_EMMC_CMD_RSP1 ); + cmd->resp[3 ] =reg_read (host,SD_EMMC_CMD_RSP ); + }else if (cmd->flags &MMC_RSP_PRESENT ) { + cmd->resp[0 ] =reg_read (host,SD_EMMC_CMD_RSP ); + } + +return 0 ; +} + +static irqreturn_t meson_mmc_irq (int irq,void *dev_id) +{ +struct meson_host *host = dev_id; +struct mmc_request *mrq = host->mrq; +struct mmc_command *cmd = host->cmd; +u32 irq_en, status, raw_status; +irqreturn_t ret =IRQ_HANDLED ; + +if (WARN_ON (!host)) +return IRQ_NONE ;
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c line 641
Quoted 6 lines
drivers/mmc/host/meson-gxbb.c new file +6
-0+ +if (WARN_ON (!mrq)) +return IRQ_NONE ; + +if (WARN_ON (!cmd)) +return IRQ_NONE ;These can happen? Spurious interrupts abound? Ugh.
They do when you mis-program the hardware, so I want to catch that here.
Quoted 176 lines
Quoted 65 lines
drivers/mmc/host/meson-gxbb.c new file +65
-0+ +spin_lock (&host->lock); + irq_en =reg_read (host,SD_EMMC_IRQ_EN ); + raw_status =reg_read (host,SD_EMMC_STATUS ); + status = raw_status & irq_en; + +if (!status) { +dev_warn (host->dev,"Spurious IRQ! status=0x%08x, irq_en=0x%08x\n" , + raw_status, irq_en); + ret =IRQ_NONE ; + goto out; + } + + cmd->error =0 ; +if (status &IRQ_RXD_ERR_MASK ) { +dev_dbg (host->dev,"Unhandled IRQ: RXD error\n" ); + cmd->error = -EILSEQ ; + } +if (status &IRQ_TXD_ERR ) { +dev_dbg (host->dev,"Unhandled IRQ: TXD error\n" ); + cmd->error = -EILSEQ ; + } +if (status &IRQ_DESC_ERR ) +dev_dbg (host->dev,"Unhandled IRQ: Descriptor error\n" ); +if (status &IRQ_RESP_ERR ) { +dev_dbg (host->dev,"Unhandled IRQ: Response error\n" ); + cmd->error = -EILSEQ ; + } +if (status &IRQ_RESP_TIMEOUT ) { +dev_dbg (host->dev,"Unhandled IRQ: Response timeout\n" ); + cmd->error = -ETIMEDOUT ; + } +if (status &IRQ_DESC_TIMEOUT ) { +dev_dbg (host->dev,"Unhandled IRQ: Descriptor timeout\n" ); + cmd->error = -ETIMEDOUT ; + } +if (status &IRQ_SDIO ) +dev_dbg (host->dev,"Unhandled IRQ: SDIO.\n" ); + +if (status & (IRQ_END_OF_CHAIN |IRQ_RESP_STATUS )) + ret =IRQ_WAKE_THREAD ; +else { +dev_warn (host->dev,"Unknown IRQ! status=0x%04x: MMC CMD%u arg=0x%08x flags=0x%08x stop=%d\n" , + status, cmd->opcode, cmd->arg, + cmd->flags, mrq->stop ?1 :0 ); +if (cmd->data) { +struct mmc_data *data = cmd->data; + +dev_warn (host->dev,"\tblksz %u blocks %u flags 0x%08x (%s%s)" , + data->blksz, data->blocks, data->flags, + data->flags &MMC_DATA_WRITE ?"write" :"" , + data->flags &MMC_DATA_READ ?"read" :"" ); + } + } + +out: +/* ack all (enabled) interrupts */ +reg_write (host,SD_EMMC_STATUS , status); + +if (ret ==IRQ_HANDLED ) { +meson_mmc_read_resp (host->mmc, cmd); +meson_mmc_request_done (host->mmc, cmd->mrq); + } + +spin_unlock (&host->lock);drivers/mmc/host/meson-gxbb.c line 716
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+return ret; +} + +static irqreturn_t meson_mmc_irq_thread (int irq,void *dev_id) +{ +struct meson_host *host = dev_id; +struct mmc_request *mrq = host->mrq; +struct mmc_command *cmd = host->cmd; +struct mmc_data *data; +Drop that newline?
Quoted 34 lines
drivers/mmc/host/meson-gxbb.c new file +34
-0+unsigned int xfer_bytes; +int ret =IRQ_HANDLED ; + +if (WARN_ON (!mrq)) + ret =IRQ_NONE ; + +if (WARN_ON (!cmd)) + ret =IRQ_NONE ; + + data = cmd->data; +if (data) { + xfer_bytes = data->blksz * data->blocks; +if (data->flags &MMC_DATA_READ ) { +WARN_ON (xfer_bytes > host->bounce_buf_size); +sg_copy_from_buffer (data->sg, data->sg_len, + host->bounce_buf, xfer_bytes); + data->bytes_xfered = xfer_bytes; + } + } + +meson_mmc_read_resp (host->mmc, cmd); +if (!data || !data->stop || mrq->sbc) +meson_mmc_request_done (host->mmc, mrq); +else +meson_mmc_start_cmd (host->mmc, data->stop); + +return ret; +} + +/* +* NOTE: we only need this until the GPIO/pinctrl driver can handle +* interrupts. For now, the MMC core will use this for polling. +*/ +static int meson_mmc_get_cd (struct mmc_host *mmc)drivers/mmc/host/meson-gxbb.c line 760
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+{ +int status =mmc_gpio_get_cd (mmc); + +if (status == -ENOSYS ) +return 1 ;/* assume present */ + +return status; +} + +static struct mmc_host_ops meson_mmc_ops = {const?
Quoted 51 lines
drivers/mmc/host/meson-gxbb.c new file +51
-0+ .request = meson_mmc_request, + .set_ios = meson_mmc_set_ios, + .get_cd = meson_mmc_get_cd, +}; + +static int meson_mmc_probe (struct platform_device *pdev) +{ +struct device_node *np = pdev->dev.of_node; +struct resource *res; +struct meson_host *host; +struct mmc_host *mmc; +int ret; + + mmc =mmc_alloc_host (sizeof (struct meson_host ), &pdev->dev); +if (!mmc) +return -ENOMEM ; + host =mmc_priv (mmc); + host->mmc = mmc; + host->dev = &pdev->dev; +dev_set_drvdata (&pdev->dev, host); + +spin_lock_init (&host->lock); + + host->core_clk =devm_clk_get (&pdev->dev,"core" ); +if (IS_ERR (host->core_clk)) { + ret =PTR_ERR (host->core_clk); +if (ret == -EPROBE_DEFER ) +dev_dbg (&pdev->dev, +"Missing core clock. EPROBE_DEFER\n" ); +else +dev_err (&pdev->dev, +"Unable to get core clk (ret=%d).\n" , ret); + goto free_host; + } + +/* Voltage supply */ +mmc_of_parse_voltage (np, &host->ocr_mask); + ret =mmc_regulator_get_supply (mmc); +if (ret == -EPROBE_DEFER ) { +dev_dbg (&pdev->dev,"Missing regulator: EPROBE_DEFER" ); + goto free_host; + } + +if (!mmc->ocr_avail) + mmc->ocr_avail = host->ocr_mask; + + ret =mmc_of_parse (mmc); +if (ret) { +dev_warn (&pdev->dev,"error parsing DT: %d\n" , ret); + goto free_host; + }
Quoted 9 lines
drivers/mmc/host/meson-gxbb.c line 817
Quoted 6 lines
drivers/mmc/host/meson-gxbb.c new file +6
-0+ + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); +if (!res) { + ret = -ENODEV ; + goto free_host; + }This if can be dropped and go right into
devm_ioremap_resource().
Yup, somone already submitted a patch to remove that from linux-next.
Quoted 34 lines
Quoted 21 lines
drivers/mmc/host/meson-gxbb.c new file +21
-0+ host->regs =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (host->regs)) { + ret =PTR_ERR (host->regs); + goto free_host; + } + + host->irq =platform_get_irq (pdev,0 ); +if (host->irq ==0 ) { +dev_err (&pdev->dev,"failed to get interrupt resource.\n" ); + ret = -EINVAL ; + goto free_host; + } + + ret =devm_request_threaded_irq (&pdev->dev, host->irq, + meson_mmc_irq, meson_mmc_irq_thread, +IRQF_SHARED ,DRIVER_NAME , host); +if (ret) + goto free_host; + +/* data bounce buffer */ + host->bounce_buf_size =SZ_512K ;drivers/mmc/host/meson-gxbb.c line 848
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ host->bounce_buf = +dma_alloc_coherent (host->dev, host->bounce_buf_size, + &host->bounce_dma_addr,GFP_KERNEL ); +if (host->bounce_buf ==NULL ) { +dev_err (host->dev,"Unable to map allocate DMA bounce buffer.\n" ); + ret = -ENOMEM ; + goto free_host; + } + +clk_prepare_enable (host->core_clk);This can fail.
Quoted 14 lines
drivers/mmc/host/meson-gxbb.c line 859
Quoted 11 lines
drivers/mmc/host/meson-gxbb.c new file +11
-0+ + ret =meson_mmc_clk_init (host); +if (ret) + goto free_host; + +/* Stop execution */ +reg_write (host,SD_EMMC_START ,0 ); + +/* clear, ack, enable all interrupts */ +reg_write (host,SD_EMMC_IRQ_EN ,0 ); +reg_write (host,SD_EMMC_STATUS ,IRQ_EN_MASK );Shouldn't we do this before requesting the interrupt?
Yes.
Quoted 6 lines
Quoted 6 lines
drivers/mmc/host/meson-gxbb.c new file +6
-0+ + mmc->ops = &meson_mmc_ops; +mmc_add_host (mmc); + +return 0 ; +
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c line 873
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c new file +8
-0+free_host: +dev_dbg (host->dev,"Failed to probe: ret=%d\n" , ret); +if (host->core_clk) +clk_disable_unprepare (host->core_clk); +mmc_free_host (mmc); +return ret; +} +
I'll clean up all the other minor comments for v2 also.
Thanks for the review!
Kevin
PATCH v2 2/2 MMC: meson: initial support for GXBB platforms
Quoted 14 lines · Kevin Hilman
Quoted 12 lines · Stephen Boyd
drivers/mmc/host/meson-gxbb.c line 323
Quoted 3 lines · Kevin Hilman
drivers/mmc/host/meson-gxbb.c new file +3
-0+ host->mux.hw.init = &init; + + host->mux_clk =devm_clk_register (host->dev, &host->mux.hw);Hmm I was hoping to get rid of
devm_clk_register()and replace it withdevm_clk_hw_register()instead. All part of my grand plan to split providers from consumers, but this driver is different in the sense that it registers clks to provide to itself. Maybe we should make__clk_create_clk()into a real clk provider API so that we can usedevm_clk_hw_register()here and then generate a clk for this device from the hw structure.
Quoted 3 lines
So is there something else I should do here, or can we rework this later after you rework the framework a bit?
That can wait for later. If nobody takes care of it I'll do it when I get around to it.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 2/2 MMC: meson: initial support for GXBB platforms
In reply to Kevin HilmanQuoted 113 lines · Kevin Hilman
Initial support for the SD/eMMC controller in the Amlogic S905/GXBB family of SoCs.
Currently working for the SD and eMMC interfaces, but not yet tested for SDIO.
Tested external SD card and internal eMMC on meson-gxbb-p200 and meson-gxbb-odroidc2.
Signed-off-by Kevin Hilman <[email protected]>--- MAINTAINERS | 1 + drivers/mmc/host/Kconfig | 10 + drivers/mmc/host/Makefile | 1 + drivers/mmc/host/meson-gxbb.c | 918 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 930 insertions(+) create mode 100644 drivers/mmc/host/meson-gxbb.cMAINTAINERS +1
-0@@ -972,6 +972,7 @@ F: arch/arm/mach-meson/ F: arch/arm/boot/dts/meson* F: arch/arm64/boot/dts/amlogic/ F: drivers/pinctrl/meson/ +F: drivers/mmc/host/meson* N: meson ARM/Annapurna Labs ALPINE ARCHITECTUREdrivers/mmc/host/Kconfig +10
-0@@ -332,6 +332,16 @@ config MMC_SDHCI_IPROC If unsure, say N. +config MMC_MESON_GXBB + tristate "Amlogic S905/GXBB SD/MMC Host Controller support" + depends on ARCH_MESON && MMC + help + This selects support for the Amlogic SD/MMC Host Controller + found on the S905/GXBB family of SoCs. This controller is + MMC 5.1 compliant and support SD, eMMC and SDIO interfaces. + + If you have a controller with this interface, say Y here. + config MMC_MOXART tristate "MOXART SD/MMC Host Controller support" depends on ARCH_MOXART && MMCdrivers/mmc/host/Makefile +1
-0@@ -53,6 +53,7 @@ obj-$(CONFIG_MMC_JZ4740) += jz4740_mmc.o obj-$(CONFIG_MMC_VUB300) += vub300.o obj-$(CONFIG_MMC_USHC) += ushc.o obj-$(CONFIG_MMC_WMT) += wmt-sdmmc.o +obj-$(CONFIG_MMC_MESON_GXBB) += meson-gxbb.o obj-$(CONFIG_MMC_MOXART) += moxart-mmc.o obj-$(CONFIG_MMC_SUNXI) += sunxi-mmc.o obj-$(CONFIG_MMC_USDHI6ROL0) += usdhi6rol0.odrivers/mmc/host/meson-gxbb.c new file +44
-0@@ -0,0 +1,918 @@ +/* +* This file is provided under a dual BSD/GPLv2 license. When using or +* redistributing this file, you may do so under either license. +* +* GPL LICENSE SUMMARY +* +* Copyright (c) 2016 BayLibre, SAS. +* Author: Kevin Hilman <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of version 2 of the GNU General Public License as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +* The full GNU General Public License is included in this distribution +* in the file called COPYING. +* +* BSD LICENSE +* +* Copyright (c) 2016 BayLibre, SAS. +* Author: Kevin Hilman <[email protected]> +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions +* are met: +* +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in +* the documentation and/or other materials provided with the +* distribution. +* * Neither the name of Intel Corporation nor the names of its +* contributors may be used to endorse or promote products derived +* from this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
drivers/mmc/host/meson-gxbb.c line 54
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/
Lots of licence text. Isn't it enough to state dual BSD/GPLv2?
[...]
Quoted 24 lines
drivers/mmc/host/meson-gxbb.c new file +24
-0+ +struct meson_host { +struct device *dev; +struct mmc_host *mmc; +struct mmc_request *mrq; +struct mmc_command *cmd; + +spinlock_t lock; +void __iomem *regs; +#ifdef USE_REGMAP +struct regmap *regmap; +#endif +int irq; +u32 ocr_mask; +struct clk *core_clk; +struct clk_mux mux; +struct clk *mux_clk; +struct clk *mux_parent[MUX_CLK_NUM_PARENTS ]; +unsigned long mux_parent_rate[MUX_CLK_NUM_PARENTS ]; + +struct clk_divider cfg_div; +struct clk *cfg_div_clk; + +unsigned int bounce_buf_size;
drivers/mmc/host/meson-gxbb.c line 187
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+void *bounce_buf; +dma_addr_t bounce_dma_addr; + +unsigned long clk_rate; +unsigned long clk_src_rate; +unsigned short clk_src_div; +}; + +#define reg_read (host, offset) readl(host->regs + offset) +#define reg_write (host, offset, val) writel(val, host->regs + offset)
I am not a fan of macros, especially when I don't think they improves the code.
Could we just stick to use readl/writel() directly instead!?
[...]
Quoted 28 lines
drivers/mmc/host/meson-gxbb.c new file +28
-0+static int meson_mmc_clk_set (struct meson_host *host,unsigned long clk_rate) +{ +struct mmc_host *mmc = host->mmc; +int ret =0 ; +u32 cfg; + +if (clk_rate) { +if (WARN_ON (clk_rate > mmc->f_max)) + clk_rate = mmc->f_max; +else if (WARN_ON (clk_rate < mmc->f_min)) + clk_rate = mmc->f_min; + } + +if (clk_rate == host->clk_rate) +return 0 ; + +/* stop clock */ + cfg =reg_read (host,SD_EMMC_CFG ); +if (!(cfg &CFG_STOP_CLOCK )) { + cfg |=CFG_STOP_CLOCK ; +reg_write (host,SD_EMMC_CFG , cfg); + } + +dev_dbg (host->dev,"change clock rate %lu -> %lu\n" , + host->clk_rate, clk_rate); + ret =clk_set_rate (host->cfg_div_clk, clk_rate); +if (clk_rate && clk_rate !=clk_get_rate (host->cfg_div_clk)) + dev_warn(host->dev, "divider requested rate %lu != actual rate %lu: ret=%d\n",
drivers/mmc/host/meson-gxbb.c line 264
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ clk_rate, clk_get_rate(host->cfg_div_clk), ret); + else + host->clk_rate = clk_rate; + +/* (re)start clock, if non-zero */ +if (clk_rate) { + cfg =reg_read (host,SD_EMMC_CFG ); + cfg &= ~CFG_STOP_CLOCK ; +reg_write (host,SD_EMMC_CFG , cfg); + }
You probably want to update mmc->actual_clock, which is useful for debugging purpose.
Quoted 49 lines
drivers/mmc/host/meson-gxbb.c new file +49
-0+ +return ret; +} + +static int meson_mmc_clk_init (struct meson_host *host) +{ +struct clk_init_data init; +char clk_name[32 ]; +int i, ret =0 ; +const char *mux_parent_names[MUX_CLK_NUM_PARENTS ]; +unsigned int mux_parent_count =0 ; +const char *clk_div_parents[1 ]; +unsigned int f_min =UINT_MAX ; +u32 clk_reg, cfg; + +/* get the mux parents from DT */ +for (i =0 ; i <MUX_CLK_NUM_PARENTS ; i++) { +char name[16 ]; + +snprintf (name,sizeof (name),"clkin%d" , i); + host->mux_parent[i] =devm_clk_get (host->dev, name); +if (IS_ERR (host->mux_parent[i])) { + ret =PTR_ERR (host->mux_parent[i]); +if (PTR_ERR (host->mux_parent[i]) != -EPROBE_DEFER ) +dev_err (host->dev,"Missing clock %s\n" , name); + host->mux_parent[i] =NULL ; +return ret; + } + + host->mux_parent_rate[i] =clk_get_rate (host->mux_parent[i]); + mux_parent_names[i] =__clk_get_name (host->mux_parent[i]); + mux_parent_count++; +if (host->mux_parent_rate[i] < f_min) + f_min = host->mux_parent_rate[i]; + } + +/* cacluate f_min based on input clocks, and max divider value */ +if (f_min !=UINT_MAX ) + f_min =DIV_ROUND_UP (CLK_SRC_XTAL_RATE ,CLK_DIV_MAX ); +else + f_min =4000000 ;/* default min: 400 MHz */ + host->mmc->f_min = f_min; + +/* create the mux */ +snprintf (clk_name,sizeof (clk_name),"%s#mux" ,dev_name (host->dev)); + init.name = clk_name; + init.ops = &clk_mux_ops; + init.flags =CLK_IS_BASIC ; + init.parent_names = mux_parent_names;
drivers/mmc/host/meson-gxbb.c line 323
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ init.num_parents = mux_parent_count; + + host->mux.reg = host->regs +SD_EMMC_CLOCK ; + host->mux.shift =CLK_SRC_SHIFT ; + host->mux.mask =CLK_SRC_MASK ; + host->mux.flags =0 ; + host->mux.table =NULL ; + host->mux.hw.init = &init; + + host->mux_clk =devm_clk_register (host->dev, &host->mux.hw);
I think it would make sense to add a comment here to clarify why you register a clock like this.
I assume it's beacuse you benefit from the clock framwork about acheiving the best/closest reqeust clockrate, as it take into account muxes/parent clocks as well!?
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+if (WARN_ON (PTR_ERR_OR_ZERO (host->mux_clk))) +return PTR_ERR (host->mux_clk); + +/* create the divider */ +snprintf (clk_name,sizeof (clk_name),"%s#div" ,dev_name (host->dev)); + init.name =devm_kstrdup (host->dev, clk_name,GFP_KERNEL ); + init.ops = &clk_divider_ops; + init.flags =CLK_IS_BASIC |CLK_SET_RATE_PARENT ; + clk_div_parents[0 ] =__clk_get_name (host->mux_clk); + init.parent_names = clk_div_parents;
drivers/mmc/host/meson-gxbb.c line 343
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ init.num_parents =ARRAY_SIZE (clk_div_parents); + + host->cfg_div.reg = host->regs +SD_EMMC_CLOCK ; + host->cfg_div.shift =CLK_DIV_SHIFT ; + host->cfg_div.width =CLK_DIV_WIDTH ; + host->cfg_div.hw.init = &init; + host->cfg_div.flags =CLK_DIVIDER_ONE_BASED | +CLK_DIVIDER_ROUND_CLOSEST |CLK_DIVIDER_ALLOW_ZERO ; + + host->cfg_div_clk =devm_clk_register (host->dev, &host->cfg_div.hw);
Ditto.
Quoted 24 lines
drivers/mmc/host/meson-gxbb.c new file +24
-0+if (WARN_ON (PTR_ERR_OR_ZERO (host->cfg_div_clk))) +return PTR_ERR (host->cfg_div_clk); + +/* init SD_EMMC_CLOCK to sane defaults w/min clock rate */ + clk_reg =0 ; + clk_reg |=CLK_PHASE_180 <<CLK_PHASE_SHIFT ; + clk_reg |=CLK_SRC_XTAL <<CLK_SRC_SHIFT ; + clk_reg |=CLK_DIV_MAX <<CLK_DIV_SHIFT ; + clk_reg &= ~CLK_ALWAYS_ON ; +reg_write (host,SD_EMMC_CLOCK , clk_reg); + +clk_prepare_enable (host->cfg_div_clk); + +/* Ensure clock starts in "auto" mode, not "always on" */ + cfg =reg_read (host,SD_EMMC_CFG ); + cfg &= ~CFG_CLK_ALWAYS_ON ; + cfg |=CFG_AUTO_CLK ; +reg_write (host,SD_EMMC_CFG , cfg); + +meson_mmc_clk_set (host, f_min); + +return ret; +} +
drivers/mmc/host/meson-gxbb.c line 377
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+static void meson_mmc_set_ios (struct mmc_host *mmc,struct mmc_ios *ios) +{ +struct meson_host *host =mmc_priv (mmc); +u32 bus_width; +u32 val, orig; + +/* +* GPIO regulator, only controls switching between 1v8 and +* 3v3, doesn't support MMC_POWER_OFF, MMC_POWER_ON. +*/
I don't follow. Shouldn't you call regulator_enable|disable() for the above regulator? Why not?
drivers/mmc/host/meson-gxbb.c line 381
Quoted 4 lines
drivers/mmc/host/meson-gxbb.c new file +4
-0+switch (ios->power_mode) { +case MMC_POWER_UP : +if (!IS_ERR (mmc->supply.vmmc)) +mmc_regulator_set_ocr (mmc, mmc->supply.vmmc, ios->vdd);
The above comment talks about 1v8 and 3v3, which seems to me that you are changing the I/O voltage, but that's not what *vmmc* should be used for.
If this is about I/O voltage, you shall instead use *vqmmc* and the corresponding mmc_regulator_set_vqmmc() to change the voltage level.
This also leaves me to ask, then how do you control the power to the card? This is actually what the *vmmc* should be used for.
Quoted 45 lines
drivers/mmc/host/meson-gxbb.c new file +45
-0+ } + +meson_mmc_clk_set (host, ios->clock); + +/* Bus width */ + val =reg_read (host,SD_EMMC_CFG ); +switch (ios->bus_width) { +case MMC_BUS_WIDTH_1 : + bus_width =CFG_BUS_WIDTH_1 ; +break ; +case MMC_BUS_WIDTH_4 : + bus_width =CFG_BUS_WIDTH_4 ; +break ; +case MMC_BUS_WIDTH_8 : + bus_width =CFG_BUS_WIDTH_8 ; +break ; +default : +dev_err (host->dev,"Invalid ios->bus_width: %u. Setting to 4.\n" , + ios->bus_width); + bus_width =CFG_BUS_WIDTH_4 ; +return ; + } + + val =reg_read (host,SD_EMMC_CFG ); + orig = val; + + val &= ~(CFG_BUS_WIDTH_MASK <<CFG_BUS_WIDTH_SHIFT ); + val |= bus_width <<CFG_BUS_WIDTH_SHIFT ; + + val &= ~(CFG_BLK_LEN_MASK <<CFG_BLK_LEN_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_BLK_SIZE ) <<CFG_BLK_LEN_SHIFT ; + + val &= ~(CFG_RESP_TIMEOUT_MASK <<CFG_RESP_TIMEOUT_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_RESP_TIMEOUT ) <<CFG_RESP_TIMEOUT_SHIFT ; + + val &= ~(CFG_RC_CC_MASK <<CFG_RC_CC_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_CMD_GAP ) <<CFG_RC_CC_SHIFT ; + +reg_write (host,SD_EMMC_CFG , val); + +if (val != orig) +dev_dbg (host->dev,"%s: SD_EMMC_CFG: 0x%08x -> 0x%08x\n" , + __func__, orig, val); +} +
[...]
Quoted 18 lines
drivers/mmc/host/meson-gxbb.c new file +18
-0+ +static int meson_mmc_probe (struct platform_device *pdev) +{ +struct device_node *np = pdev->dev.of_node; +struct resource *res; +struct meson_host *host; +struct mmc_host *mmc; +int ret; + + mmc =mmc_alloc_host (sizeof (struct meson_host ), &pdev->dev); +if (!mmc) +return -ENOMEM ; + host =mmc_priv (mmc); + host->mmc = mmc; + host->dev = &pdev->dev; +dev_set_drvdata (&pdev->dev, host); + +spin_lock_init (&host->lock);
drivers/mmc/host/meson-gxbb.c line 792
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ + host->core_clk =devm_clk_get (&pdev->dev,"core" ); +if (IS_ERR (host->core_clk)) { + ret =PTR_ERR (host->core_clk); +if (ret == -EPROBE_DEFER ) +dev_dbg (&pdev->dev, +"Missing core clock. EPROBE_DEFER\n" ); +else +dev_err (&pdev->dev, +"Unable to get core clk (ret=%d).\n" , ret);
I think these prints are unessarry, they should be printed by other frameworks already, right!?
drivers/mmc/host/meson-gxbb.c line 797
Quoted 5 lines
drivers/mmc/host/meson-gxbb.c new file +5
-0+ goto free_host; + } + +/* Voltage supply */ +mmc_of_parse_voltage (np, &host->ocr_mask);
This looks odd.
Usally we get "ocr_avail" when asking the vmmc regulator about its supported voltage range, via calling the below mmc_regulator_get_supply() API. Can you explain why thay isn't work for you?
drivers/mmc/host/meson-gxbb.c line 800
Quoted 3 lines
drivers/mmc/host/meson-gxbb.c new file +3
-0+ ret =mmc_regulator_get_supply (mmc); +if (ret == -EPROBE_DEFER ) { +dev_dbg (&pdev->dev,"Missing regulator: EPROBE_DEFER" );
The print shouldn't be needed, please remove.
drivers/mmc/host/meson-gxbb.c line 805
Quoted 5 lines
drivers/mmc/host/meson-gxbb.c new file +5
-0+ goto free_host; + } + +if (!mmc->ocr_avail) + mmc->ocr_avail = host->ocr_mask;
mmc->ocr_avail needs to be assigned to a valid value. This fallback doesn't cover that as host->ocr_mask may very well be zero.
Quoted 68 lines
drivers/mmc/host/meson-gxbb.c new file +68
-0+ + ret =mmc_of_parse (mmc); +if (ret) { +dev_warn (&pdev->dev,"error parsing DT: %d\n" , ret); + goto free_host; + } + + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); +if (!res) { + ret = -ENODEV ; + goto free_host; + } + host->regs =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (host->regs)) { + ret =PTR_ERR (host->regs); + goto free_host; + } + + host->irq =platform_get_irq (pdev,0 ); +if (host->irq ==0 ) { +dev_err (&pdev->dev,"failed to get interrupt resource.\n" ); + ret = -EINVAL ; + goto free_host; + } + + ret =devm_request_threaded_irq (&pdev->dev, host->irq, + meson_mmc_irq, meson_mmc_irq_thread, +IRQF_SHARED ,DRIVER_NAME , host); +if (ret) + goto free_host; + +/* data bounce buffer */ + host->bounce_buf_size =SZ_512K ; + host->bounce_buf = +dma_alloc_coherent (host->dev, host->bounce_buf_size, + &host->bounce_dma_addr,GFP_KERNEL ); +if (host->bounce_buf ==NULL ) { +dev_err (host->dev,"Unable to map allocate DMA bounce buffer.\n" ); + ret = -ENOMEM ; + goto free_host; + } + +clk_prepare_enable (host->core_clk); + + ret =meson_mmc_clk_init (host); +if (ret) + goto free_host; + +/* Stop execution */ +reg_write (host,SD_EMMC_START ,0 ); + +/* clear, ack, enable all interrupts */ +reg_write (host,SD_EMMC_IRQ_EN ,0 ); +reg_write (host,SD_EMMC_STATUS ,IRQ_EN_MASK ); + + mmc->ops = &meson_mmc_ops; +mmc_add_host (mmc); + +return 0 ; + +free_host: +dev_dbg (host->dev,"Failed to probe: ret=%d\n" , ret); +if (host->core_clk) +clk_disable_unprepare (host->core_clk); +mmc_free_host (mmc); +return ret; +} +
[...]
drivers/mmc/host/meson-gxbb.c line 897
Quoted 4 lines
drivers/mmc/host/meson-gxbb.c new file +4
-0+ +static const struct of_device_id meson_mmc_of_match[] = { + { + .compatible ="amlogic,meson-gxbb-mmc" ,
You need to fold in a DT documentation patch, in this series as to describe all the required/optional resourses for the device. That of course also includes the compatible string.
drivers/mmc/host/meson-gxbb.c line 901
Quoted 4 lines
drivers/mmc/host/meson-gxbb.c new file +4
-0+ }, + {} +}; +MODULE_DEVICE_TABLE (of, meson_mmc_of_match);
Kind regards
Uffe
Re: [PATCH v2 2/2] MMC: meson: initial support for GXBB platforms
Quoted 11 lines · Ulf Hansson
Quoted 10 lines · Kevin Hilman
Initial support for the SD/eMMC controller in the Amlogic S905/GXBB family of SoCs.
Currently working for the SD and eMMC interfaces, but not yet tested for SDIO.
Tested external SD card and internal eMMC on meson-gxbb-p200 and meson-gxbb-odroidc2.
Signed-off-by Kevin Hilman <[email protected]>
[...]
Quoted 49 lines
Quoted 49 lines
drivers/mmc/host/meson-gxbb.c new file +48
-0@@ -0,0 +1,918 @@ +/* +* This file is provided under a dual BSD/GPLv2 license. When using or +* redistributing this file, you may do so under either license. +* +* GPL LICENSE SUMMARY +* +* Copyright (c) 2016 BayLibre, SAS. +* Author: Kevin Hilman <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of version 2 of the GNU General Public License as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +* The full GNU General Public License is included in this distribution +* in the file called COPYING. +* +* BSD LICENSE +* +* Copyright (c) 2016 BayLibre, SAS. +* Author: Kevin Hilman <[email protected]> +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions +* are met: +* +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in +* the documentation and/or other materials provided with the +* distribution. +* * Neither the name of Intel Corporation nor the names of its +* contributors may be used to endorse or promote products derived +* from this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c line 54
Quoted 6 lines
drivers/mmc/host/meson-gxbb.c new file +6
-0+* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ Lots of licence text. Isn't it enough to state dual BSD/GPLv2?
At least with regard to the BSD license part it probably isn't. There is only one canonical "GPLv2" (a copy of which is included in the kernel tree), but there are quite a number of different variants of the "BSD" license:
- 4-clause BSD license (with "advertising clause" and with "endorsement clause")
- 3-clause BSD license (without "advertising clause", but with "endorsement clause")
- 2-clause BSD license (without "advertising clause" and without "endorsement clause")
In addition to the these three major types, the exact terms of the "endorsement clause" can vary quite a bit.
While being at the topic of the "endorsement clause": Is there a particular reason for Intel being explicitly named here (e.g. because the code is based on some previously existing code which is licensed with this specific wording)?
Regards,
Karsten
Signature · 3 lines
Gem. Par. 28 Abs. 4 Bundesdatenschutzgesetz widerspreche ich der Nutzung sowie der Weitergabe meiner personenbezogenen Daten für Zwecke der Werbung sowie der Markt- oder Meinungsforschung.
Kevin Hilman
maintainerPATCH v2 2/2 MMC: meson: initial support for GXBB platforms
In reply to Ulf HanssonQuoted 166 lines · Ulf Hansson
Quoted 113 lines · Kevin Hilman
Initial support for the SD/eMMC controller in the Amlogic S905/GXBB family of SoCs.
Currently working for the SD and eMMC interfaces, but not yet tested for SDIO.
Tested external SD card and internal eMMC on meson-gxbb-p200 and meson-gxbb-odroidc2.
Signed-off-by Kevin Hilman <[email protected]>--- MAINTAINERS | 1 + drivers/mmc/host/Kconfig | 10 + drivers/mmc/host/Makefile | 1 + drivers/mmc/host/meson-gxbb.c | 918 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 930 insertions(+) create mode 100644 drivers/mmc/host/meson-gxbb.cMAINTAINERS +1
-0@@ -972,6 +972,7 @@ F: arch/arm/mach-meson/ F: arch/arm/boot/dts/meson* F: arch/arm64/boot/dts/amlogic/ F: drivers/pinctrl/meson/ +F: drivers/mmc/host/meson* N: meson ARM/Annapurna Labs ALPINE ARCHITECTUREdrivers/mmc/host/Kconfig +10
-0@@ -332,6 +332,16 @@ config MMC_SDHCI_IPROC If unsure, say N. +config MMC_MESON_GXBB + tristate "Amlogic S905/GXBB SD/MMC Host Controller support" + depends on ARCH_MESON && MMC + help + This selects support for the Amlogic SD/MMC Host Controller + found on the S905/GXBB family of SoCs. This controller is + MMC 5.1 compliant and support SD, eMMC and SDIO interfaces. + + If you have a controller with this interface, say Y here. + config MMC_MOXART tristate "MOXART SD/MMC Host Controller support" depends on ARCH_MOXART && MMCdrivers/mmc/host/Makefile +1
-0@@ -53,6 +53,7 @@ obj-$(CONFIG_MMC_JZ4740) += jz4740_mmc.o obj-$(CONFIG_MMC_VUB300) += vub300.o obj-$(CONFIG_MMC_USHC) += ushc.o obj-$(CONFIG_MMC_WMT) += wmt-sdmmc.o +obj-$(CONFIG_MMC_MESON_GXBB) += meson-gxbb.o obj-$(CONFIG_MMC_MOXART) += moxart-mmc.o obj-$(CONFIG_MMC_SUNXI) += sunxi-mmc.o obj-$(CONFIG_MMC_USDHI6ROL0) += usdhi6rol0.odrivers/mmc/host/meson-gxbb.c new file +44
-0@@ -0,0 +1,918 @@ +/* +* This file is provided under a dual BSD/GPLv2 license. When using or +* redistributing this file, you may do so under either license. +* +* GPL LICENSE SUMMARY +* +* Copyright (c) 2016 BayLibre, SAS. +* Author: Kevin Hilman <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of version 2 of the GNU General Public License as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +* The full GNU General Public License is included in this distribution +* in the file called COPYING. +* +* BSD LICENSE +* +* Copyright (c) 2016 BayLibre, SAS. +* Author: Kevin Hilman <[email protected]> +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions +* are met: +* +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in +* the documentation and/or other materials provided with the +* distribution. +* * Neither the name of Intel Corporation nor the names of its +* contributors may be used to endorse or promote products derived +* from this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT drivers/mmc/host/meson-gxbb.c line 54
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ Lots of licence text. Isn't it enough to state dual BSD/GPLv2?
[...]
Quoted 24 lines
drivers/mmc/host/meson-gxbb.c new file +24
-0+ +struct meson_host { +struct device *dev; +struct mmc_host *mmc; +struct mmc_request *mrq; +struct mmc_command *cmd; + +spinlock_t lock; +void __iomem *regs; +#ifdef USE_REGMAP +struct regmap *regmap; +#endif +int irq; +u32 ocr_mask; +struct clk *core_clk; +struct clk_mux mux; +struct clk *mux_clk; +struct clk *mux_parent[MUX_CLK_NUM_PARENTS ]; +unsigned long mux_parent_rate[MUX_CLK_NUM_PARENTS ]; + +struct clk_divider cfg_div; +struct clk *cfg_div_clk; + +unsigned int bounce_buf_size;drivers/mmc/host/meson-gxbb.c line 187
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+void *bounce_buf; +dma_addr_t bounce_dma_addr; + +unsigned long clk_rate; +unsigned long clk_src_rate; +unsigned short clk_src_div; +}; + +#define reg_read (host, offset) readl(host->regs + offset) +#define reg_write (host, offset, val) writel(val, host->regs + offset)I am not a fan of macros, especially when I don't think they improves the code.
Quoted 2 lines
Could we just stick to use readl/writel() directly instead!?
Yes, I'll remove these. They are leftover from when I was using macros to switch between readl/writel and regmap accessors.
Quoted 34 lines
Quoted 34 lines
drivers/mmc/host/meson-gxbb.c new file +33
-0+static int meson_mmc_clk_set (struct meson_host *host,unsigned long clk_rate) +{ +struct mmc_host *mmc = host->mmc; +int ret =0 ; +u32 cfg; + +if (clk_rate) { +if (WARN_ON (clk_rate > mmc->f_max)) + clk_rate = mmc->f_max; +else if (WARN_ON (clk_rate < mmc->f_min)) + clk_rate = mmc->f_min; + } + +if (clk_rate == host->clk_rate) +return 0 ; + +/* stop clock */ + cfg =reg_read (host,SD_EMMC_CFG ); +if (!(cfg &CFG_STOP_CLOCK )) { + cfg |=CFG_STOP_CLOCK ; +reg_write (host,SD_EMMC_CFG , cfg); + } + +dev_dbg (host->dev,"change clock rate %lu -> %lu\n" , + host->clk_rate, clk_rate); + ret =clk_set_rate (host->cfg_div_clk, clk_rate); +if (clk_rate && clk_rate !=clk_get_rate (host->cfg_div_clk)) +dev_warn (host->dev,"divider requested rate %lu != actual rate %lu: ret=%d\n" , + clk_rate,clk_get_rate (host->cfg_div_clk), ret); +else + host->clk_rate = clk_rate; + +/* (re)start clock, if non-zero */
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c line 264
Quoted 5 lines
drivers/mmc/host/meson-gxbb.c new file +5
-0+if (clk_rate) { + cfg =reg_read (host,SD_EMMC_CFG ); + cfg &= ~CFG_STOP_CLOCK ; +reg_write (host,SD_EMMC_CFG , cfg); + }You probably want to update
mmc->actual_clock, which is useful for debugging purpose.
OK, I hadn't noticed that field. I'll just drop my own host->clk_rate and use that.
Quoted 63 lines
Quoted 49 lines
drivers/mmc/host/meson-gxbb.c new file +49
-0+ +return ret; +} + +static int meson_mmc_clk_init (struct meson_host *host) +{ +struct clk_init_data init; +char clk_name[32 ]; +int i, ret =0 ; +const char *mux_parent_names[MUX_CLK_NUM_PARENTS ]; +unsigned int mux_parent_count =0 ; +const char *clk_div_parents[1 ]; +unsigned int f_min =UINT_MAX ; +u32 clk_reg, cfg; + +/* get the mux parents from DT */ +for (i =0 ; i <MUX_CLK_NUM_PARENTS ; i++) { +char name[16 ]; + +snprintf (name,sizeof (name),"clkin%d" , i); + host->mux_parent[i] =devm_clk_get (host->dev, name); +if (IS_ERR (host->mux_parent[i])) { + ret =PTR_ERR (host->mux_parent[i]); +if (PTR_ERR (host->mux_parent[i]) != -EPROBE_DEFER ) +dev_err (host->dev,"Missing clock %s\n" , name); + host->mux_parent[i] =NULL ; +return ret; + } + + host->mux_parent_rate[i] =clk_get_rate (host->mux_parent[i]); + mux_parent_names[i] =__clk_get_name (host->mux_parent[i]); + mux_parent_count++; +if (host->mux_parent_rate[i] < f_min) + f_min = host->mux_parent_rate[i]; + } + +/* cacluate f_min based on input clocks, and max divider value */ +if (f_min !=UINT_MAX ) + f_min =DIV_ROUND_UP (CLK_SRC_XTAL_RATE ,CLK_DIV_MAX ); +else + f_min =4000000 ;/* default min: 400 MHz */ + host->mmc->f_min = f_min; + +/* create the mux */ +snprintf (clk_name,sizeof (clk_name),"%s#mux" ,dev_name (host->dev)); + init.name = clk_name; + init.ops = &clk_mux_ops; + init.flags =CLK_IS_BASIC ; + init.parent_names = mux_parent_names;drivers/mmc/host/meson-gxbb.c line 323
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ init.num_parents = mux_parent_count; + + host->mux.reg = host->regs +SD_EMMC_CLOCK ; + host->mux.shift =CLK_SRC_SHIFT ; + host->mux.mask =CLK_SRC_MASK ; + host->mux.flags =0 ; + host->mux.table =NULL ; + host->mux.hw.init = &init; + + host->mux_clk =devm_clk_register (host->dev, &host->mux.hw);I think it would make sense to add a comment here to clarify why you register a clock like this.
Quoted 3 lines
I assume it's beacuse you benefit from the clock framwork about acheiving the best/closest reqeust clockrate, as it take into account muxes/parent clocks as well!?
Correct. This IP block has an internal mux and divider, so I want to take advanate of the clock framework features to model those clocks.
Quoted 70 lines
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+if (WARN_ON (PTR_ERR_OR_ZERO (host->mux_clk))) +return PTR_ERR (host->mux_clk); + +/* create the divider */ +snprintf (clk_name,sizeof (clk_name),"%s#div" ,dev_name (host->dev)); + init.name =devm_kstrdup (host->dev, clk_name,GFP_KERNEL ); + init.ops = &clk_divider_ops; + init.flags =CLK_IS_BASIC |CLK_SET_RATE_PARENT ; + clk_div_parents[0 ] =__clk_get_name (host->mux_clk); + init.parent_names = clk_div_parents;drivers/mmc/host/meson-gxbb.c line 343
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+ init.num_parents =ARRAY_SIZE (clk_div_parents); + + host->cfg_div.reg = host->regs +SD_EMMC_CLOCK ; + host->cfg_div.shift =CLK_DIV_SHIFT ; + host->cfg_div.width =CLK_DIV_WIDTH ; + host->cfg_div.hw.init = &init; + host->cfg_div.flags =CLK_DIVIDER_ONE_BASED | +CLK_DIVIDER_ROUND_CLOSEST |CLK_DIVIDER_ALLOW_ZERO ; + + host->cfg_div_clk =devm_clk_register (host->dev, &host->cfg_div.hw);Ditto.
Quoted 24 lines
drivers/mmc/host/meson-gxbb.c new file +24
-0+if (WARN_ON (PTR_ERR_OR_ZERO (host->cfg_div_clk))) +return PTR_ERR (host->cfg_div_clk); + +/* init SD_EMMC_CLOCK to sane defaults w/min clock rate */ + clk_reg =0 ; + clk_reg |=CLK_PHASE_180 <<CLK_PHASE_SHIFT ; + clk_reg |=CLK_SRC_XTAL <<CLK_SRC_SHIFT ; + clk_reg |=CLK_DIV_MAX <<CLK_DIV_SHIFT ; + clk_reg &= ~CLK_ALWAYS_ON ; +reg_write (host,SD_EMMC_CLOCK , clk_reg); + +clk_prepare_enable (host->cfg_div_clk); + +/* Ensure clock starts in "auto" mode, not "always on" */ + cfg =reg_read (host,SD_EMMC_CFG ); + cfg &= ~CFG_CLK_ALWAYS_ON ; + cfg |=CFG_AUTO_CLK ; +reg_write (host,SD_EMMC_CFG , cfg); + +meson_mmc_clk_set (host, f_min); + +return ret; +} +drivers/mmc/host/meson-gxbb.c line 377
Quoted 10 lines
drivers/mmc/host/meson-gxbb.c new file +10
-0+static void meson_mmc_set_ios (struct mmc_host *mmc,struct mmc_ios *ios) +{ +struct meson_host *host =mmc_priv (mmc); +u32 bus_width; +u32 val, orig; + +/* +* GPIO regulator, only controls switching between 1v8 and +* 3v3, doesn't support MMC_POWER_OFF, MMC_POWER_ON. +*/ I don't follow. Shouldn't you call regulator_enable|disable() for the above regulator? Why not?
drivers/mmc/host/meson-gxbb.c line 381
Quoted 4 lines
drivers/mmc/host/meson-gxbb.c new file +4
-0+switch (ios->power_mode) { +case MMC_POWER_UP : +if (!IS_ERR (mmc->supply.vmmc)) +mmc_regulator_set_ocr (mmc, mmc->supply.vmmc, ios->vdd);The above comment talks about 1v8 and 3v3, which seems to me that you are changing the I/O voltage, but that's not what *vmmc* should be used for.
Quoted 5 lines
If this is about I/O voltage, you shall instead use *vqmmc* and the corresponding
mmc_regulator_set_vqmmc()to change the voltage level.This also leaves me to ask, then how do you control the power to the card? This is actually what the *vmmc* should be used for.
I'll respin this whole section based on your clarifications. I was confused about how the various regulators are meant to be used. Thanks for clarifying.
Quoted 72 lines
Quoted 46 lines
drivers/mmc/host/meson-gxbb.c new file +45
-0+ } + +meson_mmc_clk_set (host, ios->clock); + +/* Bus width */ + val =reg_read (host,SD_EMMC_CFG ); +switch (ios->bus_width) { +case MMC_BUS_WIDTH_1 : + bus_width =CFG_BUS_WIDTH_1 ; +break ; +case MMC_BUS_WIDTH_4 : + bus_width =CFG_BUS_WIDTH_4 ; +break ; +case MMC_BUS_WIDTH_8 : + bus_width =CFG_BUS_WIDTH_8 ; +break ; +default : +dev_err (host->dev,"Invalid ios->bus_width: %u. Setting to 4.\n" , + ios->bus_width); + bus_width =CFG_BUS_WIDTH_4 ; +return ; + } + + val =reg_read (host,SD_EMMC_CFG ); + orig = val; + + val &= ~(CFG_BUS_WIDTH_MASK <<CFG_BUS_WIDTH_SHIFT ); + val |= bus_width <<CFG_BUS_WIDTH_SHIFT ; + + val &= ~(CFG_BLK_LEN_MASK <<CFG_BLK_LEN_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_BLK_SIZE ) <<CFG_BLK_LEN_SHIFT ; + + val &= ~(CFG_RESP_TIMEOUT_MASK <<CFG_RESP_TIMEOUT_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_RESP_TIMEOUT ) <<CFG_RESP_TIMEOUT_SHIFT ; + + val &= ~(CFG_RC_CC_MASK <<CFG_RC_CC_SHIFT ); + val |=ilog2 (SD_EMMC_CFG_CMD_GAP ) <<CFG_RC_CC_SHIFT ; + +reg_write (host,SD_EMMC_CFG , val); + +if (val != orig) +dev_dbg (host->dev,"%s: SD_EMMC_CFG: 0x%08x -> 0x%08x\n" , + __func__, orig, val); +} +[...]
Quoted 23 lines
drivers/mmc/host/meson-gxbb.c new file +23
-0+ +static int meson_mmc_probe (struct platform_device *pdev) +{ +struct device_node *np = pdev->dev.of_node; +struct resource *res; +struct meson_host *host; +struct mmc_host *mmc; +int ret; + + mmc =mmc_alloc_host (sizeof (struct meson_host ), &pdev->dev); +if (!mmc) +return -ENOMEM ; + host =mmc_priv (mmc); + host->mmc = mmc; + host->dev = &pdev->dev; +dev_set_drvdata (&pdev->dev, host); + +spin_lock_init (&host->lock); + + host->core_clk =devm_clk_get (&pdev->dev,"core" ); +if (IS_ERR (host->core_clk)) { + ret =PTR_ERR (host->core_clk); +if (ret == -EPROBE_DEFER )
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c line 792
Quoted 5 lines
drivers/mmc/host/meson-gxbb.c new file +5
-0+dev_dbg (&pdev->dev, +"Missing core clock. EPROBE_DEFER\n" ); + else + dev_err(&pdev->dev, + "Unable to get core clk (ret=%d).\n", ret);I think these prints are unessarry, they should be printed by other frameworks already, right!?
OK, I'll drop these.
Quoted 6 lines
drivers/mmc/host/meson-gxbb.c line 797
Quoted 5 lines
drivers/mmc/host/meson-gxbb.c new file +5
-0+ goto free_host; + } + +/* Voltage supply */ +mmc_of_parse_voltage (np, &host->ocr_mask);
Quoted 6 lines
This looks odd.
Usally we get "
ocr_avail" when asking the vmmc regulator about its supported voltage range, via calling the belowmmc_regulator_get_supply()API. Can you explain why thay isn't work for you?
You're right, that's working fine. I'll drop this.
Quoted 5 lines
drivers/mmc/host/meson-gxbb.c line 800
Quoted 3 lines
drivers/mmc/host/meson-gxbb.c new file +3
-0+ ret =mmc_regulator_get_supply (mmc); +if (ret == -EPROBE_DEFER ) { +dev_dbg (&pdev->dev,"Missing regulator: EPROBE_DEFER" );The print shouldn't be needed, please remove.
OK.
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c line 805
Quoted 5 lines
drivers/mmc/host/meson-gxbb.c new file +5
-0+ goto free_host; + } + +if (!mmc->ocr_avail) + mmc->ocr_avail = host->ocr_mask;
mmc->ocr_availneeds to be assigned to a valid value. This fallback doesn't cover that ashost->ocr_maskmay very well be zero.
Dropped this as it's now handled elsewhere.
Quoted 71 lines
Quoted 68 lines
drivers/mmc/host/meson-gxbb.c new file +68
-0+ + ret =mmc_of_parse (mmc); +if (ret) { +dev_warn (&pdev->dev,"error parsing DT: %d\n" , ret); + goto free_host; + } + + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); +if (!res) { + ret = -ENODEV ; + goto free_host; + } + host->regs =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (host->regs)) { + ret =PTR_ERR (host->regs); + goto free_host; + } + + host->irq =platform_get_irq (pdev,0 ); +if (host->irq ==0 ) { +dev_err (&pdev->dev,"failed to get interrupt resource.\n" ); + ret = -EINVAL ; + goto free_host; + } + + ret =devm_request_threaded_irq (&pdev->dev, host->irq, + meson_mmc_irq, meson_mmc_irq_thread, +IRQF_SHARED ,DRIVER_NAME , host); +if (ret) + goto free_host; + +/* data bounce buffer */ + host->bounce_buf_size =SZ_512K ; + host->bounce_buf = +dma_alloc_coherent (host->dev, host->bounce_buf_size, + &host->bounce_dma_addr,GFP_KERNEL ); +if (host->bounce_buf ==NULL ) { +dev_err (host->dev,"Unable to map allocate DMA bounce buffer.\n" ); + ret = -ENOMEM ; + goto free_host; + } + +clk_prepare_enable (host->core_clk); + + ret =meson_mmc_clk_init (host); +if (ret) + goto free_host; + +/* Stop execution */ +reg_write (host,SD_EMMC_START ,0 ); + +/* clear, ack, enable all interrupts */ +reg_write (host,SD_EMMC_IRQ_EN ,0 ); +reg_write (host,SD_EMMC_STATUS ,IRQ_EN_MASK ); + + mmc->ops = &meson_mmc_ops; +mmc_add_host (mmc); + +return 0 ; + +free_host: +dev_dbg (host->dev,"Failed to probe: ret=%d\n" , ret); +if (host->core_clk) +clk_disable_unprepare (host->core_clk); +mmc_free_host (mmc); +return ret; +} +[...]
Quoted 8 lines
drivers/mmc/host/meson-gxbb.c line 897
Quoted 4 lines
drivers/mmc/host/meson-gxbb.c new file +4
-0+ +static const struct of_device_id meson_mmc_of_match[] = { + { + .compatible ="amlogic,meson-gxbb-mmc" ,You need to fold in a DT documentation patch, in this series as to describe all the required/optional resourses for the device. That of course also includes the compatible string.
That was all part of PATCH 1/2 of this series, or was there something else missing from that patch?
Thanks for the review,
Kevin
Rob Herring
maintainerPATCH v2 1/2 ARM64: dts: meson-gxbb: add MMC support
In reply to Kevin HilmanQuoted 62 lines · Kevin Hilman
Add binding and basic support for the SD/eMMC controller on Amlogic S905/GXBB devices.
Signed-off-by Kevin Hilman <[email protected]>--- .../devicetree/bindings/mmc/amlogic,meson-gxbb.txt | 29 +++++++++ .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 74 ++++++++++++++++++++++ arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 71 +++++++++++++++++++++ arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 36 ++++++++++- 4 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txtDocumentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt new file +29
-0@@ -0,0 +1,29 @@ +Amlogic SD / eMMC controller for S905/GXBB family SoCs + +The MMC 5.1 compliant host controller on Amlogic provides the +interface for SD, eMMC and SDIO devices. + +This file documents the properties in addition to those available in +the MMC core bindings, documented by mmc.txt. + +Required properties: +- compatible : contains "amlogic,meson-gxbb" +- clocks : A list of phandle + clock-specifier pairs for the clocks listed in clock-names. +- clock-names: Should contain the following: + "core" - Main peripheral bus clock + "clkin0" - Parent clock of internal mux + "clkin1" - Other parent clock of internal mux + The driver has an interal mux clock which switches between clkin0 and clkin1 depending on the + clock rate requested by the MMC core. + +Example: + + sd_emmc_a: mmc at 70000 { + compatible = "amlogic,meson-gxbb-mmc"; + reg = <0x0 0x70000 0x0 0x2000>; + interrupts = < GIC_SPI 216 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_SD_EMMC_A>, <&xtal>, <&clkc CLKID_FCLK_DIV2>; + clock-names = "core", "clkin0", "clkin1"; + pinctrl-0 = <&emmc_pins>; + }; +arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts +7
-0@@ -73,6 +73,45 @@ default-state = "off"; }; }; + + mmc_iv: gpio-regulator { + compatible = "regulator-gpio"; + + regulator-name = "mmc-gpio-supply"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>;
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts line 92
Quoted 10 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts +10
-0+ + gpios = <&gpio_ao GPIOAO_3 GPIO_ACTIVE_HIGH>; + gpio-states = <0 1>; + + /* + * Based on ODROID-C2 schematics: + * signal name: IO_TF_3V3N_1V8, GPIOAO bit 3 + */ + states = <3300000 0 + 1800000 1>;
There are multiple things wrong in this node based on the regulator-gpio documentation.
Quoted 89 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts +57
-0+ }; + + vcc1v8: regulator { + compatible = "regulator-fixed"; + regulator-name = "VCC1V8"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + }; + + vcc3v3: regulator { + compatible = "regulator-fixed"; + regulator-name = "VCC3V3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + + emmc_pwrseq: emmc-pwrseq { + compatible = "mmc-pwrseq-emmc"; + reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>; + }; }; &uart_AO {@@ -87,3 +126,38 @@ pinctrl-names = "default"; }; +&sd_emmc_b { + status = "okay"; + pinctrl-0 = <&sdcard_pins>; + pinctrl-names = "default"; + + bus-width = <4>; + cap-sd-highspeed; + max-frequency = <100000000>; + + cd-gpios = <&gpio CARD_6 GPIO_ACTIVE_HIGH>; + cd-inverted; + + vmmc-supply = <&mmc_iv>; + voltage-ranges = <1800 3300>; +}; + +&sd_emmc_c { + status = "okay"; + pinctrl-0 = <&emmc_pins>; + pinctrl-names = "default"; + + bus-width = <8>; + cap-sd-highspeed; + max-frequency = <200000000>; + non-removable; + disable-wp; + cap-mmc-highspeed; + mmc-ddr-1_8v; + mmc-hs200-1_8v; + + mmc-pwrseq = <&emmc_pwrseq>; + voltage-ranges = <1800 3300>; + vmmc-supply = <&vcc3v3>; + vmmcq-sumpply = <&vcc1v8>; +};arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi +17
-0@@ -57,6 +57,42 @@ device_type = "memory"; reg = <0x0 0x0 0x0 0x40000000>; }; + + mmc_iv: gpio-regulator { + compatible = "regulator-gpio"; + + regulator-name = "mmc-gpio-supply"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + + gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>; + gpio-states = <0 1>; + + /* Based on P200 schematics, signal CARD_1.8V/3.3V_CTR */ + states = <1800000 0 + 3300000 1>; + }; + + vddio_boot: regulator {
[...]
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi line 84
Quoted 1 line
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi +1
-0+ vcc_3v3: regulator {
This won't even compile as you have the same node name twice.
Quoted 114 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi +46
-0+ compatible = "regulator-fixed"; + regulator-name = "VCC_3V3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + + emmc_pwrseq: emmc-pwrseq { + compatible = "mmc-pwrseq-emmc"; + reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>; + }; }; /* This UART is brought out to the DB9 connector */@@ -72,3 +108,38 @@ pinctrl-names = "default"; }; +&sd_emmc_b { + status = "okay"; + pinctrl-0 = <&sdcard_pins>; + pinctrl-names = "default"; + + bus-width = <4>; + cap-sd-highspeed; + max-frequency = <100000000>; + + cd-gpios = <&gpio CARD_6 GPIO_ACTIVE_HIGH>; + cd-inverted; + + voltage-ranges = <1800 3300>; + vmmc-supply = <&mmc_iv>; +}; + +&sd_emmc_c { + status = "okay"; + pinctrl-0 = <&emmc_pins>; + pinctrl-names = "default"; + + bus-width = <8>; + cap-sd-highspeed; + cap-mmc-highspeed; + max-frequency = <200000000>; + non-removable; + disable-wp; + mmc-ddr-1_8v; + mmc-hs200-1_8v; + + mmc-pwrseq = <&emmc_pwrseq>; + voltage-ranges = <1800 3300>; + vmmc-supply = <&vcc_3v3>; + vmmcq-sumpply = <&vddio_boot>; +};arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +35
-1@@ -247,7 +247,8 @@ mux { groups = "emmc_nand_d07", "emmc_cmd", - "emmc_clk"; + "emmc_clk", + "emmc_ds"; function = "emmc"; }; };@@ -330,6 +331,39 @@ #address-cells = <2>; #size-cells = <2>; ranges = <0x0 0x0 0x0 0xd0000000 0x0 0x200000>; + + sd_emmc_a: mmc at 70000 { + compatible = "amlogic,meson-gxbb-mmc"; + reg = <0x0 0x70000 0x0 0x2000>; + interrupts = <GIC_SPI 216 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_SD_EMMC_A>, + <&xtal>, + <&clkc CLKID_FCLK_DIV2>; + clock-names = "core", "clkin0", "clkin1"; + status = "disabled"; + }; + + sd_emmc_b: mmc at 72000 { + compatible = "amlogic,meson-gxbb-mmc"; + reg = <0x0 0x72000 0x0 0x2000>; + interrupts = <GIC_SPI 217 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_SD_EMMC_B>, + <&xtal>, + <&clkc CLKID_FCLK_DIV2>; + clock-names = "core", "clkin0", "clkin1"; + status = "disabled"; + }; + + sd_emmc_c: mmc at 74000 { + compatible = "amlogic,meson-gxbb-mmc"; + reg = <0x0 0x74000 0x0 0x2000>; + interrupts = <GIC_SPI 218 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_SD_EMMC_C>, + <&xtal>, + <&clkc CLKID_FCLK_DIV2>; + clock-names = "core", "clkin0", "clkin1"; + status = "disabled"; + }; }; ethmac: ethernet at c9410000 {Signature · 6 lines
2.9.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Kevin Hilman
maintainerPATCH v2 1/2 ARM64: dts: meson-gxbb: add MMC support
Hi Rob,
Quoted 5 lines · Rob Herring
Quoted 4 lines · Kevin Hilman
Add binding and basic support for the SD/eMMC controller on Amlogic S905/GXBB devices.
Signed-off-by Kevin Hilman <[email protected]>
[...]
Quoted 11 lines
Quoted 11 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts +11
-0+ mmc_iv: gpio-regulator { + compatible = "regulator-gpio"; + + regulator-name = "mmc-gpio-supply"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + + gpios = <&gpio_ao GPIOAO_3 GPIO_ACTIVE_HIGH>; + gpio-states = <0 1>; + + /*
Quoted 8 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts line 92
Quoted 5 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts +5
-0+ * Based on ODROID-C2 schematics: + * signal name: IO_TF_3V3N_1V8, GPIOAO bit 3 + */ + states = <3300000 0 + 1800000 1>;There are multiple things wrong in this node based on the regulator-gpio documentation.
I see I got the property name wrong, and the number of elements wrong, and this fixes those issues:
- gpio-states = <0 1>;
+ gpios-states = <0>;
By "multiple", did you mean more than those 2? If so, I'm not seeing what else is wrong.
[...]
Quoted 17 lines
Quoted 16 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi +16
-0+ mmc_iv: gpio-regulator { + compatible = "regulator-gpio"; + + regulator-name = "mmc-gpio-supply"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + + gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>; + gpio-states = <0 1>; + + /* Based on P200 schematics, signal CARD_1.8V/3.3V_CTR */ + states = <1800000 0 + 3300000 1>; + }; + + vddio_boot: regulator {[...]
Quoted 3 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi line 84
Quoted 1 line
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi +1
-0+ vcc_3v3: regulator {This won't even compile as you have the same node name twice.
Oops, it's wrong indeed, but in fact, it does compile, otherwise I would've caught that.. Even if I remove the labels, it still compiles. Interesting.
Anyways, I'll make those unique node-names in the next spin.
Thanks for the review,
Kevin
A message that is not in the archive (sent off-list or not yet received): m2oa58nntc.fsf-rdvid1DuHRBWk0Htik3J/[email protected]
+dtc list
Quoted 38 lines · Kevin Hilman
Hi Rob,
Quoted 5 lines · Rob Herring
Quoted 4 lines · Kevin Hilman
Add binding and basic support for the SD/eMMC controller on Amlogic S905/GXBB devices.
[...]
Quoted 11 lines
Quoted 11 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts +11
-0+ mmc_iv: gpio-regulator { + compatible = "regulator-gpio"; + + regulator-name = "mmc-gpio-supply"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + + gpios = <&gpio_ao GPIOAO_3 GPIO_ACTIVE_HIGH>; + gpio-states = <0 1>; + + /*Quoted 8 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts line 92
Quoted 5 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts +5
-0+ * Based on ODROID-C2 schematics: + * signal name: IO_TF_3V3N_1V8, GPIOAO bit 3 + */ + states = <3300000 0 + 1800000 1>;There are multiple things wrong in this node based on the regulator-gpio documentation.
I see I got the property name wrong, and the number of elements wrong, and this fixes those issues:
(quoted diff) +1
-1- gpio-states = <0 1>; + gpios-states = <0>;
Quoted 2 lines
By "multiple", did you mean more than those 2? If so, I'm not seeing what else is wrong.
I was thinking it needed an enable-gpio property, but now I see that is separate from "gpios".
Quoted 19 lines
[...]
Quoted 17 lines
Quoted 16 lines
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi +16
-0+ mmc_iv: gpio-regulator { + compatible = "regulator-gpio"; + + regulator-name = "mmc-gpio-supply"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + + gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>; + gpio-states = <0 1>; + + /* Based on P200 schematics, signal CARD_1.8V/3.3V_CTR */ + states = <1800000 0 + 3300000 1>; + }; + + vddio_boot: regulator {[...]
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
+
vcc_3v3: regulator {This won't even compile as you have the same node name twice.
Oops, it's wrong indeed, but in fact, it does compile, otherwise I would've caught that.. Even if I remove the labels, it still compiles. Interesting.
Humm, news to me. I will have to do some testing unless the dtc folks have any insight.
Rob
A message that is not in the archive (sent off-list or not yet received): 20160803231843.23012-1-khilman-rdvid1DuHRBWk0Htik3J/[email protected]
Quoted 18 lines · Kevin Hilman
Add binding and basic support for the SD/eMMC controller on Amlogic S905/GXBB devices.
--- .../devicetree/bindings/mmc/amlogic,meson-gxbb.txt | 29 +++++++++ .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 74 ++++++++++++++++++++++ arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 71 +++++++++++++++++++++ arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 36 ++++++++++- 4 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txtDocumentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt new file +0
-0
Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt line 10
Quoted 10 lines
Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt new file +10
-0@@ -0,0 +1,29 @@ +Amlogic SD / eMMC controller for S905/GXBB family SoCs + +The MMC 5.1 compliant host controller on Amlogic provides the +interface for SD, eMMC and SDIO devices. + +This file documents the properties in addition to those available in +the MMC core bindings, documented by mmc.txt. + +Required properties: +- compatible : contains "amlogic,meson-gxbb"
I assume that should be "amlogic,meson-gxbb-mmc" instead of "amlogic,meson-gxbb", shouldn't it?
Quoted 9 lines
Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt new file +9
-0+- clocks : A list of phandle + clock-specifier pairs for the clocks listed in clock-names. +- clock-names: Should contain the following: + "core" - Main peripheral bus clock + "clkin0" - Parent clock of internal mux + "clkin1" - Other parent clock of internal mux + The driver has an interal mux clock which switches between clkin0 and clkin1 depending on the + clock rate requested by the MMC core. + +Example:
Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt line 29
Quoted 10 lines
Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt new file +10
-0+ + sd_emmc_a: mmc@70000 { + compatible = "amlogic,meson-gxbb-mmc"; + reg = <0x0 0x70000 0x0 0x2000>; + interrupts = < GIC_SPI 216 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_SD_EMMC_A>, <&xtal>, <&clkc CLKID_FCLK_DIV2>; + clock-names = "core", "clkin0", "clkin1"; + pinctrl-0 = <&emmc_pins>; + }; +
Regards,
Karsten
Signature · 7 lines
Gem. Par. 28 Abs. 4 Bundesdatenschutzgesetz widerspreche ich der Nutzung sowie der Weitergabe meiner personenbezogenen Daten für Zwecke der Werbung sowie der Markt- oder Meinungsforschung. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Kevin Hilman
maintainerPATCH v2 1/2 ARM64: dts: meson-gxbb: add MMC support
Quoted 25 lines · Karsten Merker
Quoted 23 lines · Kevin Hilman
Add binding and basic support for the SD/eMMC controller on Amlogic S905/GXBB devices.
Signed-off-by Kevin Hilman <[email protected]>--- .../devicetree/bindings/mmc/amlogic,meson-gxbb.txt | 29 +++++++++ .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 74 ++++++++++++++++++++++ arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 71 +++++++++++++++++++++ arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 36 ++++++++++- 4 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txtDocumentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt new file +5
-0@@ -0,0 +1,29 @@ +Amlogic SD / eMMC controller for S905/GXBB family SoCs + +The MMC 5.1 compliant host controller on Amlogic provides the +interface for SD, eMMC and SDIO devices. +
Quoted 8 lines
Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt line 10
Quoted 5 lines
Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt new file +5
-0+This file documents the properties in addition to those available in +the MMC core bindings, documented by mmc.txt. + +Required properties: +- compatible : contains "amlogic,meson-gxbb"I assume that should be "amlogic,meson-gxbb-mmc" instead of "amlogic,meson-gxbb", shouldn't it?
Correct, thanks for the review.
Kevin
kbuild test robot
build boturl: https://github.com/0day-ci/linux/commits/Kevin-Hilman/ARM64-dts-meson-gxbb-add-MMC-support/20160804-072056 config: arm64-defconfig (attached as .config) compiler: aarch64-linux-gnu-gcc (Debian…
Hi Kevin,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc1 next-20160812]
[cannot apply to ulf.hansson-mmc/next robh/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Kevin-Hilman/ARM64-dts-meson-gxbb-add-MMC-support/20160804-072056
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
>> Error: arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi:338.21-22 syntax error >> FATAL ERROR: Unable to parse input tree
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 29377 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20160814/37296eba/attachment.obj>
Kevin Hilman
maintainerPATCH v2 1/2 ARM64: dts: meson-gxbb: add MMC support
Quoted 16 lines · kbuild test robot
Hi Kevin,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc1 next-20160812]
[cannot apply to ulf.hansson-mmc/next robh/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]url: https://github.com/0day-ci/linux/commits/Kevin-Hilman/ARM64-dts-meson-gxbb-add-MMC-support/20160804-072056
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
Quoted 4 lines
All errors (new ones prefixed by >>):
Quoted 2 lines
Quoted 2 lines
Error: arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi:338.21-22 syntax error FATAL ERROR: Unable to parse input tree
This is due to a clock dependency[1] which is in the process of being queued in the clock tree.
Kevin
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-August/446650.html