openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH linux dev-4.17 0/7] drivers/mmc/host: Add Aspeed SDIO driver
@ 2018-07-11  5:17 Ryan Chen
  2018-07-11  5:17 ` [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence Ryan Chen
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Ryan Chen @ 2018-07-11  5:17 UTC (permalink / raw)
  To: openbmc; +Cc: Ryan Chen, joel, andrew, ryan_chen, benh, mine260309

This series implements Aspeed SDIO driver. In Aspeed SDIO controller
with two slots and have a global register for interrupt status and other
general information.

For implements this, it will have have irq-aspeed-sdhci-ic.c for sdhci
each slot irq dispatch, and then go to sdhci driver handle (sdhci.c).

Another is sdhci-of-aspeed.c for specific function call.

Ryan Chen

Ryan Chen (7):
  clk: Aspeed: Modify clk-aspeed.c driver probe sequence
  clk: Aspeed: Add sdhci reset and clock
  irqchip: Aspeed: Add Aspeed sdhci irq driver
  dts: Aspeed: Add Aspeed sdhci dts document
  mmc: Aspeed: Add driver for Aspeed sdhci
  dts: Aspeed: Add devicetree for Aspeed sdhci
  configs: Aspeed: enable mmc host in defconfig

 .../aspeed,aspeed-sdhci-ic.txt                     |  25 +++
 .../bindings/mmc/aspeed,aspeed-sdhci.txt           |  42 +++++
 arch/arm/boot/dts/aspeed-ast2500-evb.dts           |  14 ++
 arch/arm/boot/dts/aspeed-g4.dtsi                   |  40 +++++
 arch/arm/boot/dts/aspeed-g5.dtsi                   |  40 +++++
 arch/arm/configs/aspeed_g5_defconfig               |   6 +
 drivers/clk/clk-aspeed.c                           |  15 +-
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-aspeed-sdhci-ic.c              | 147 +++++++++++++++++
 drivers/mmc/host/Kconfig                           |  12 ++
 drivers/mmc/host/Makefile                          |   1 +
 drivers/mmc/host/sdhci-of-aspeed.c                 | 178 +++++++++++++++++++++
 include/dt-bindings/clock/aspeed-clock.h           |   2 +-
 include/linux/mmc/sdhci-aspeed-data.h              |  28 ++++
 14 files changed, 548 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,aspeed-sdhci-ic.txt
 create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,aspeed-sdhci.txt
 create mode 100644 drivers/irqchip/irq-aspeed-sdhci-ic.c
 create mode 100644 drivers/mmc/host/sdhci-of-aspeed.c
 create mode 100644 include/linux/mmc/sdhci-aspeed-data.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence
  2018-07-11  5:17 [PATCH linux dev-4.17 0/7] drivers/mmc/host: Add Aspeed SDIO driver Ryan Chen
@ 2018-07-11  5:17 ` Ryan Chen
  2018-07-11  5:47   ` Benjamin Herrenschmidt
  2018-07-11  5:17 ` [PATCH linux dev-4.17 2/7] clk: Aspeed: Add sdhci reset and clock Ryan Chen
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Ryan Chen @ 2018-07-11  5:17 UTC (permalink / raw)
  To: openbmc; +Cc: Ryan Chen, joel, andrew, ryan_chen, benh, mine260309

In Aspeed's SoC, all IP clk gating and pll parameter is in scu
controller, before IP driver probe, scu driver need prepare for it.
So buildin_platform_driver to core_initcall.

Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
---
 drivers/clk/clk-aspeed.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 8796b8a..9e55743 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -573,7 +573,12 @@ static struct platform_driver aspeed_clk_driver = {
 		.suppress_bind_attrs = true,
 	},
 };
-builtin_platform_driver(aspeed_clk_driver);
+
+static int __init aspeed_clk_init(void)
+{
+	return platform_driver_register(&aspeed_clk_driver);
+}
+core_initcall(aspeed_clk_init);
 
 static void __init aspeed_ast2400_cc(struct regmap *map)
 {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH linux dev-4.17 2/7] clk: Aspeed: Add sdhci reset and clock
  2018-07-11  5:17 [PATCH linux dev-4.17 0/7] drivers/mmc/host: Add Aspeed SDIO driver Ryan Chen
  2018-07-11  5:17 ` [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence Ryan Chen
@ 2018-07-11  5:17 ` Ryan Chen
  2018-07-11  5:26   ` Joel Stanley
  2018-07-11  5:49   ` Benjamin Herrenschmidt
  2018-07-11  5:17 ` [PATCH linux dev-4.17 3/7] irqchip: Aspeed: Add Aspeed sdhci irq driver Ryan Chen
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 18+ messages in thread
From: Ryan Chen @ 2018-07-11  5:17 UTC (permalink / raw)
  To: openbmc; +Cc: Ryan Chen, joel, andrew, ryan_chen, benh, mine260309

1.Add Aspeed sdhci reset for SCU04 bit 16
2.Aspeed sdhci have two clock one is for controller clock,
another is for SD card clock. so when enable sdhci need enable
both.

Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
---
 drivers/clk/clk-aspeed.c                 | 8 +++++++-
 include/dt-bindings/clock/aspeed-clock.h | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 9e55743..cd1f3b2 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -20,6 +20,7 @@
 
 #define ASPEED_RESET_CTRL	0x04
 #define ASPEED_CLK_SELECTION	0x08
+#define  ASPEED_SDIO_CLK_EN BIT(15)
 #define ASPEED_CLK_STOP_CTRL	0x0c
 #define ASPEED_MPLL_PARAM	0x20
 #define ASPEED_HPLL_PARAM	0x24
@@ -260,6 +261,11 @@ static int aspeed_clk_enable(struct clk_hw *hw)
 	enval = (gate->flags & CLK_GATE_SET_TO_DISABLE) ? 0 : clk;
 	regmap_update_bits(gate->map, ASPEED_CLK_STOP_CTRL, clk, enval);
 
+	/* sd ext clk */
+	if (gate->reset_idx == 16) {
+		regmap_update_bits(gate->map, ASPEED_CLK_SELECTION, ASPEED_SDIO_CLK_EN, ASPEED_SDIO_CLK_EN);
+	}
+
 	if (gate->reset_idx >= 0) {
 		/* A delay of 10ms is specified by the ASPEED docs */
 		mdelay(10);
@@ -317,7 +323,7 @@ static const u8 aspeed_resets[] = {
 	[ASPEED_RESET_PECI]	= 10,
 	[ASPEED_RESET_I2C]	=  2,
 	[ASPEED_RESET_AHB]	=  1,
-
+	[ASPEED_RESET_SDHCI]	= 16,
 	/*
 	 * SCUD4 resets start at an offset to separate them from
 	 * the SCU04 resets.
diff --git a/include/dt-bindings/clock/aspeed-clock.h b/include/dt-bindings/clock/aspeed-clock.h
index 4476184..088553f 100644
--- a/include/dt-bindings/clock/aspeed-clock.h
+++ b/include/dt-bindings/clock/aspeed-clock.h
@@ -50,5 +50,5 @@
 #define ASPEED_RESET_I2C		7
 #define ASPEED_RESET_AHB		8
 #define ASPEED_RESET_CRT1		9
-
+#define ASPEED_RESET_SDHCI		10
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH linux dev-4.17 3/7] irqchip: Aspeed: Add Aspeed sdhci irq driver
  2018-07-11  5:17 [PATCH linux dev-4.17 0/7] drivers/mmc/host: Add Aspeed SDIO driver Ryan Chen
  2018-07-11  5:17 ` [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence Ryan Chen
  2018-07-11  5:17 ` [PATCH linux dev-4.17 2/7] clk: Aspeed: Add sdhci reset and clock Ryan Chen
@ 2018-07-11  5:17 ` Ryan Chen
  2018-07-11  5:29   ` Joel Stanley
  2018-07-11  5:17 ` [PATCH linux dev-4.17 4/7] dts: Aspeed: Add Aspeed sdhci dts document Ryan Chen
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Ryan Chen @ 2018-07-11  5:17 UTC (permalink / raw)
  To: openbmc; +Cc: Ryan Chen, joel, andrew, ryan_chen, benh, mine260309

In Aspeed SoC's sdhci have two slots and have a interrupt status and
general information in top for register. so it need a sdhci irq driver
probe defore sdhci driver probe

Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
---
 drivers/irqchip/Makefile              |   2 +-
 drivers/irqchip/irq-aspeed-sdhci-ic.c | 147 ++++++++++++++++++++++++++++++++++
 include/linux/mmc/sdhci-aspeed-data.h |  28 +++++++
 3 files changed, 176 insertions(+), 1 deletion(-)
 create mode 100644 drivers/irqchip/irq-aspeed-sdhci-ic.c
 create mode 100644 include/linux/mmc/sdhci-aspeed-data.h

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 5ed465a..579c4fb 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -78,7 +78,7 @@ obj-$(CONFIG_MVEBU_ODMI)		+= irq-mvebu-odmi.o
 obj-$(CONFIG_MVEBU_PIC)			+= irq-mvebu-pic.o
 obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
 obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
-obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o irq-aspeed-i2c-ic.o
+obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o irq-aspeed-i2c-ic.o irq-aspeed-sdhci-ic.o
 obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
 obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
 obj-$(CONFIG_IRQ_UNIPHIER_AIDET)	+= irq-uniphier-aidet.o
diff --git a/drivers/irqchip/irq-aspeed-sdhci-ic.c b/drivers/irqchip/irq-aspeed-sdhci-ic.c
new file mode 100644
index 0000000..c3d9d45
--- /dev/null
+++ b/drivers/irqchip/irq-aspeed-sdhci-ic.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SDHCI IRQCHIP driver for the Aspeed SoC
+ *
+ * Copyright (C) ASPEED Technology Inc.
+ * Ryan Chen <ryan_chen@aspeedtech.com>
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/mmc/sdhci-aspeed-data.h>
+
+#define ASPEED_SDHCI_SLOT_NUM			2
+
+static void aspeed_sdhci_irq_handler(struct irq_desc *desc)
+{
+	struct aspeed_sdhci_irq *sdhci_irq = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned long bit, status;
+	unsigned int slot_irq;
+
+	chained_irq_enter(chip, desc);
+	status = readl(sdhci_irq->regs + ASPEED_SDHCI_ISR);
+	for_each_set_bit(bit, &status, ASPEED_SDHCI_SLOT_NUM) {
+		slot_irq = irq_find_mapping(sdhci_irq->irq_domain, bit);
+		generic_handle_irq(slot_irq);
+	}
+	chained_irq_exit(chip, desc);
+}
+
+static void noop(struct irq_data *data) { }
+
+static unsigned int noop_ret(struct irq_data *data)
+{
+	return 0;
+}
+
+struct irq_chip sdhci_irq_chip = {
+	.name		= "sdhci-ic",
+	.irq_startup	= noop_ret,
+	.irq_shutdown	= noop,
+	.irq_enable	= noop,
+	.irq_disable	= noop,
+	.irq_ack	= noop,
+	.irq_mask	= noop,
+	.irq_unmask	= noop,
+	.flags		= IRQCHIP_SKIP_SET_WAKE,
+};
+
+static int ast_sdhci_map_irq_domain(struct irq_domain *domain,
+				    unsigned int irq, irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &sdhci_irq_chip, handle_simple_irq);
+	irq_set_chip_data(irq, domain->host_data);
+
+	return 0;
+}
+
+static const struct irq_domain_ops aspeed_sdhci_irq_domain_ops = {
+	.map = ast_sdhci_map_irq_domain,
+};
+
+static int irq_aspeed_sdhci_probe(struct platform_device *pdev)
+{
+	struct aspeed_sdhci_irq *sdhci_irq;
+	struct clk *sdclk;
+
+	sdhci_irq = kzalloc(sizeof(*sdhci_irq), GFP_KERNEL);
+	if (!sdhci_irq)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, sdhci_irq);
+	//node->data = sdhci_irq;
+	pdev->dev.of_node->data = sdhci_irq;
+
+	sdhci_irq->regs = of_iomap(pdev->dev.of_node, 0);
+	if (IS_ERR(sdhci_irq->regs))
+		return PTR_ERR(sdhci_irq->regs);
+
+	sdclk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(sdclk)) {
+		dev_err(&pdev->dev, "no sdclk clock defined\n");
+		return PTR_ERR(sdclk);
+	}
+
+	clk_prepare_enable(sdclk);
+
+	sdhci_irq->parent_irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	if (sdhci_irq->parent_irq < 0)
+		return sdhci_irq->parent_irq;
+
+	sdhci_irq->irq_domain = irq_domain_add_linear(
+					pdev->dev.of_node, ASPEED_SDHCI_SLOT_NUM,
+					&aspeed_sdhci_irq_domain_ops, NULL);
+	if (!sdhci_irq->irq_domain)
+		return -ENOMEM;
+
+	sdhci_irq->irq_domain->name = "aspeed-sdhci-irq";
+
+	irq_set_chained_handler_and_data(sdhci_irq->parent_irq,
+					 aspeed_sdhci_irq_handler, sdhci_irq);
+
+	pr_info("sdhci irq controller registered, irq %d\n", sdhci_irq->parent_irq);
+
+	return 0;
+}
+
+static const struct of_device_id irq_aspeed_sdhci_dt_ids[] = {
+	{ .compatible = "aspeed,aspeed-sdhci-irq", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, irq_aspeed_sdhci_dt_ids);
+
+static struct platform_driver irq_aspeed_sdhci_device_driver = {
+	.probe		= irq_aspeed_sdhci_probe,
+	.driver		= {
+		.name   = KBUILD_MODNAME,
+		.of_match_table	= irq_aspeed_sdhci_dt_ids,
+	}
+};
+
+static int __init irq_aspeed_sdhci_init(void)
+{
+	return platform_driver_register(&irq_aspeed_sdhci_device_driver);
+}
+core_initcall(irq_aspeed_sdhci_init);
+
+MODULE_AUTHOR("Ryan Chen");
+MODULE_DESCRIPTION("ASPEED SOC SDHCI IRQ Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mmc/sdhci-aspeed-data.h b/include/linux/mmc/sdhci-aspeed-data.h
new file mode 100644
index 0000000..fba2bf2
--- /dev/null
+++ b/include/linux/mmc/sdhci-aspeed-data.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef LINUX_MMC_SDHCI_ASPEED_DATA_H
+#define LINUX_MMC_SDHCI_ASPEED_DATA_H
+
+#include <linux/io.h>
+
+#define ASPEED_SDHCI_INFO			0x00
+#define  ASPEED_SDHCI_S1MMC8			BIT(25)
+#define  ASPEED_SDHCI_S0MMC8			BIT(24)
+#define ASPEED_SDHCI_BLOCK			0x04
+#define ASPEED_SDHCI_CTRL			0xF0
+#define ASPEED_SDHCI_ISR			0xFC
+
+struct aspeed_sdhci_irq {
+	void __iomem *regs;
+	int parent_irq;
+	struct irq_domain *irq_domain;
+};
+
+static inline void aspeed_sdhci_set_8bit_mode(struct aspeed_sdhci_irq *sdhci_irq, int mode)
+{
+	if (mode)
+		writel(ASPEED_SDHCI_S0MMC8 | readl(sdhci_irq->regs), sdhci_irq->regs);
+	else
+		writel(~ASPEED_SDHCI_S0MMC8 & readl(sdhci_irq->regs), sdhci_irq->regs);
+}
+
+#endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH linux dev-4.17 4/7] dts: Aspeed: Add Aspeed sdhci dts document
  2018-07-11  5:17 [PATCH linux dev-4.17 0/7] drivers/mmc/host: Add Aspeed SDIO driver Ryan Chen
                   ` (2 preceding siblings ...)
  2018-07-11  5:17 ` [PATCH linux dev-4.17 3/7] irqchip: Aspeed: Add Aspeed sdhci irq driver Ryan Chen
@ 2018-07-11  5:17 ` Ryan Chen
  2018-07-11  5:30   ` Joel Stanley
  2018-07-11  5:17 ` [PATCH linux dev-4.17 5/7] mmc: Aspeed: Add driver for Aspeed sdhci Ryan Chen
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Ryan Chen @ 2018-07-11  5:17 UTC (permalink / raw)
  To: openbmc; +Cc: Ryan Chen, joel, andrew, ryan_chen, benh, mine260309

This add Aspeed sdhci irq driver's dts file and devicetree document

Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
---
 .../aspeed,aspeed-sdhci-ic.txt                     | 25 ++++++++++++++++++++++
 arch/arm/boot/dts/aspeed-g4.dtsi                   | 20 +++++++++++++++++
 arch/arm/boot/dts/aspeed-g5.dtsi                   | 20 +++++++++++++++++
 3 files changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,aspeed-sdhci-ic.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,aspeed-sdhci-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/aspeed,aspeed-sdhci-ic.txt
new file mode 100644
index 0000000..e3393c3
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,aspeed-sdhci-ic.txt
@@ -0,0 +1,25 @@
+Device tree configuration for the SDHCI Interrupt Controller on the AST24XX and
+AST25XX SoCs.
+
+Required Properties:
+- #address-cells	: should be 1
+- #size-cells 		: should be 1
+- #interrupt-cells 	: should be 1
+- compatible 		: should be "aspeed,aspeed-sdhci-ic"
+- reg			: address start and range of controller
+- interrupts		: interrupt number
+- interrupt-controller	: denotes that the controller receives and fires
+			  new interrupts for child busses
+
+Example:
+
+sdhci_ic: interrupt-controller@0 {
+	#interrupt-cells = <1>;
+	#size-cells = <1>;
+	#interrupt-cells = <1>;
+	compatible = "aspeed,aspeed-sdhci-irq";
+	reg = <0x0 0x100>;
+	interrupts = <26>;
+	interrupt-controller;
+	clocks = <&syscon ASPEED_CLK_GATE_SDCLKCLK>;
+};
\ No newline at end of file
diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index e86fa80..7ad646d 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -190,6 +190,13 @@
 				reg = <0x1e720000 0x8000>;	// 32K
 			};
 
+			sdhci: sdhci@1e740000 {
+				compatible = "simple-bus";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				ranges = <0 0x1e740000 0x1000>;
+			};
+
 			gpio: gpio@1e780000 {
 				#gpio-cells = <2>;
 				gpio-controller;
@@ -369,6 +376,19 @@
 	};
 };
 
+&sdhci {
+
+	sdhci_ic: interrupt-controller@0 {
+		#interrupt-cells = <1>;
+		compatible = "aspeed,aspeed-sdhci-irq";
+		reg = <0x0 0x100>;
+		interrupts = <26>;
+		interrupt-controller;
+		clocks = <&syscon ASPEED_CLK_GATE_SDCLKCLK>;
+	};
+
+};
+
 &i2c {
 	i2c_ic: interrupt-controller@0 {
 		#interrupt-cells = <1>;
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index d92f047..ba850ca 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -256,6 +256,13 @@
 				reg = <0x1e720000 0x9000>;	// 36K
 			};
 
+			sdhci: sdhci@1e740000 {
+				compatible = "simple-bus";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				ranges = <0 0x1e740000 0x1000>;
+			};
+
 			gpio: gpio@1e780000 {
 				#gpio-cells = <2>;
 				gpio-controller;
@@ -446,6 +453,19 @@
 	};
 };
 
+&sdhci {
+
+	sdhci_ic: interrupt-controller@0 {
+		#interrupt-cells = <1>;
+		compatible = "aspeed,aspeed-sdhci-irq";
+		reg = <0x0 0x100>;
+		interrupts = <26>;
+		interrupt-controller;
+		clocks = <&syscon ASPEED_CLK_GATE_SDCLKCLK>;
+	};
+
+};
+
 &i2c {
 	i2c_ic: interrupt-controller@0 {
 		#interrupt-cells = <1>;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH linux dev-4.17 5/7] mmc: Aspeed: Add driver for Aspeed sdhci
  2018-07-11  5:17 [PATCH linux dev-4.17 0/7] drivers/mmc/host: Add Aspeed SDIO driver Ryan Chen
                   ` (3 preceding siblings ...)
  2018-07-11  5:17 ` [PATCH linux dev-4.17 4/7] dts: Aspeed: Add Aspeed sdhci dts document Ryan Chen
@ 2018-07-11  5:17 ` Ryan Chen
  2018-07-11  5:32   ` Joel Stanley
  2018-07-11  5:17 ` [PATCH linux dev-4.17 6/7] dts: Aspeed: Add devicetree " Ryan Chen
  2018-07-11  5:17 ` [PATCH linux dev-4.17 7/7] configs: Aspeed: enable mmc host in defconfig Ryan Chen
  6 siblings, 1 reply; 18+ messages in thread
From: Ryan Chen @ 2018-07-11  5:17 UTC (permalink / raw)
  To: openbmc; +Cc: Ryan Chen, joel, andrew, ryan_chen, benh, mine260309

Add a driver for Aspeed's sdhci controller core.

Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
---
 drivers/mmc/host/Kconfig           |  12 +++
 drivers/mmc/host/Makefile          |   1 +
 drivers/mmc/host/sdhci-of-aspeed.c | 178 +++++++++++++++++++++++++++++++++++++
 3 files changed, 191 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-of-aspeed.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 9589f9c..5056e6e 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -142,6 +142,18 @@ config MMC_SDHCI_OF_ARASAN
 
 	  If unsure, say N.
 
+config MMC_SDHCI_OF_ASPEED
+	tristate "SDHCI OF support for the ASPEED SDHCI controller"
+	depends on MMC_SDHCI_PLTFM
+	depends on OF
+	help
+	  This selects the ASPEED Secure Digital Host Controller Interface.
+
+	  If you have a controller with this interface, say Y or M here. You
+	  also need to enable an appropriate bus interface.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_OF_AT91
 	tristate "SDHCI OF support for the Atmel SDMMC controller"
 	depends on MMC_SDHCI_PLTFM
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 6aead24..084193b 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)	+= sdhci-esdhc-imx.o
 obj-$(CONFIG_MMC_SDHCI_DOVE)		+= sdhci-dove.o
 obj-$(CONFIG_MMC_SDHCI_TEGRA)		+= sdhci-tegra.o
 obj-$(CONFIG_MMC_SDHCI_OF_ARASAN)	+= sdhci-of-arasan.o
+obj-$(CONFIG_MMC_SDHCI_OF_ASPEED)	+= sdhci-of-aspeed.o
 obj-$(CONFIG_MMC_SDHCI_OF_AT91)		+= sdhci-of-at91.o
 obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
 obj-$(CONFIG_MMC_SDHCI_OF_HLWD)		+= sdhci-of-hlwd.o
diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
new file mode 100644
index 0000000..8e609e1
--- /dev/null
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ASPEED Secure Digital Host Controller Interface.
+ * Copyright (C) ASPEED Technology Inc.
+ * Ryan Chen <ryan_chen@aspeedtech.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/mmc/host.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/mmc/sdhci-aspeed-data.h>
+#include <linux/reset.h>
+#include "sdhci-pltfm.h"
+
+static void sdhci_aspeed_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	int div;
+	u16 clk;
+	unsigned long timeout;
+
+	if (clock == host->clock)
+		return;
+
+	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
+
+	if (clock == 0)
+		goto out;
+
+	for (div = 1; div < 256; div *= 2) {
+		if ((host->max_clk / div) <= clock)
+			break;
+	}
+	div >>= 1;
+
+	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk |= SDHCI_CLOCK_INT_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+
+	/* Wait max 20 ms */
+	timeout = 20;
+	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
+		 & SDHCI_CLOCK_INT_STABLE)) {
+		if (timeout == 0) {
+			pr_err("%s: Internal clock never stabilised.\n",
+			       mmc_hostname(host->mmc));
+			return;
+		}
+		timeout--;
+		mdelay(1);
+	}
+
+	clk |= SDHCI_CLOCK_CARD_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+
+out:
+	host->clock = clock;
+}
+
+static void sdhci_aspeed_set_bus_width(struct sdhci_host *host, int width)
+{
+	struct sdhci_pltfm_host *pltfm_priv = sdhci_priv(host);
+	struct aspeed_sdhci_irq *sdhci_irq = sdhci_pltfm_priv(pltfm_priv);
+
+	u8 ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+
+	if (sdhci_irq->regs) {
+		if (width == MMC_BUS_WIDTH_8)
+			aspeed_sdhci_set_8bit_mode(sdhci_irq, 1);
+		else
+			aspeed_sdhci_set_8bit_mode(sdhci_irq, 0);
+	}
+	if (width == MMC_BUS_WIDTH_4)
+		ctrl |= SDHCI_CTRL_4BITBUS;
+	else
+		ctrl &= ~SDHCI_CTRL_4BITBUS;
+
+	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+
+}
+
+static struct sdhci_ops  sdhci_aspeed_ops = {
+	.set_clock = sdhci_aspeed_set_clock,
+	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
+	.set_bus_width = sdhci_aspeed_set_bus_width,
+	.get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
+	.reset = sdhci_reset,
+	.set_uhs_signaling = sdhci_set_uhs_signaling,
+};
+
+static struct sdhci_pltfm_data sdhci_aspeed_pdata = {
+	.ops = &sdhci_aspeed_ops,
+	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+	.quirks2 = SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
+};
+
+static int sdhci_aspeed_probe(struct platform_device *pdev)
+{
+	struct sdhci_host *host;
+	struct device_node *pnode;
+	struct device_node *np = pdev->dev.of_node;
+	struct sdhci_pltfm_host *pltfm_host;
+	struct aspeed_sdhci_irq *sdhci_irq;
+
+	int ret;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_aspeed_pdata, sizeof(struct aspeed_sdhci_irq));
+	if (IS_ERR(host))
+		return PTR_ERR(host);
+
+	pltfm_host = sdhci_priv(host);
+	sdhci_irq = sdhci_pltfm_priv(pltfm_host);
+
+	sdhci_get_of_property(pdev);
+
+	pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
+
+	pnode = of_parse_phandle(np, "interrupt-parent", 0);
+	if (pnode)
+		memcpy(sdhci_irq, pnode->data, sizeof(struct aspeed_sdhci_irq));
+
+	ret = mmc_of_parse(host->mmc);
+	if (ret)
+		goto err_sdhci_add;
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		goto err_sdhci_add;
+
+	return 0;
+
+err_sdhci_add:
+	sdhci_pltfm_free(pdev);
+	return ret;
+}
+
+static int sdhci_aspeed_remove(struct platform_device *pdev)
+{
+	struct sdhci_host	*host = platform_get_drvdata(pdev);
+	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
+
+	sdhci_remove_host(host, dead);
+	sdhci_pltfm_free(pdev);
+	return 0;
+}
+
+static const struct of_device_id sdhci_aspeed_of_match[] = {
+	{ .compatible = "aspeed,sdhci-ast2400", .data = &sdhci_aspeed_pdata },
+	{ .compatible = "aspeed,sdhci-ast2500", .data = &sdhci_aspeed_pdata },
+	{}
+};
+
+MODULE_DEVICE_TABLE(of, sdhci_aspeed_of_match);
+
+static struct platform_driver sdhci_aspeed_driver = {
+	.driver		= {
+		.name	= "sdhci-aspeed",
+		.pm	= &sdhci_pltfm_pmops,
+		.of_match_table = sdhci_aspeed_of_match,
+	},
+	.probe		= sdhci_aspeed_probe,
+	.remove		= sdhci_aspeed_remove,
+};
+
+module_platform_driver(sdhci_aspeed_driver);
+
+MODULE_DESCRIPTION("Driver for the ASPEED SDHCI Controller");
+MODULE_AUTHOR("Ryan Chen <ryan_chen@aspeedtech.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH linux dev-4.17 6/7] dts: Aspeed: Add devicetree for Aspeed sdhci
  2018-07-11  5:17 [PATCH linux dev-4.17 0/7] drivers/mmc/host: Add Aspeed SDIO driver Ryan Chen
                   ` (4 preceding siblings ...)
  2018-07-11  5:17 ` [PATCH linux dev-4.17 5/7] mmc: Aspeed: Add driver for Aspeed sdhci Ryan Chen
@ 2018-07-11  5:17 ` Ryan Chen
  2018-07-11  5:17 ` [PATCH linux dev-4.17 7/7] configs: Aspeed: enable mmc host in defconfig Ryan Chen
  6 siblings, 0 replies; 18+ messages in thread
From: Ryan Chen @ 2018-07-11  5:17 UTC (permalink / raw)
  To: openbmc; +Cc: Ryan Chen, joel, andrew, ryan_chen, benh, mine260309

Add devicetree for Aspeed's sdhci

Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
---
 .../bindings/mmc/aspeed,aspeed-sdhci.txt           | 42 ++++++++++++++++++++++
 arch/arm/boot/dts/aspeed-ast2500-evb.dts           | 14 ++++++++
 arch/arm/boot/dts/aspeed-g4.dtsi                   | 20 +++++++++++
 arch/arm/boot/dts/aspeed-g5.dtsi                   | 20 +++++++++++
 4 files changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,aspeed-sdhci.txt

diff --git a/Documentation/devicetree/bindings/mmc/aspeed,aspeed-sdhci.txt b/Documentation/devicetree/bindings/mmc/aspeed,aspeed-sdhci.txt
new file mode 100644
index 0000000..c7cb985
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/aspeed,aspeed-sdhci.txt
@@ -0,0 +1,42 @@
+Device Tree Bindings for the ASPEED SoC SDHCI Controller
+
+This file documents differences between the core properties in mmc.txt
+and the properties present in the ASPEED SDHCI
+
+Required properties:
+- compatible : should be "aspeed,ast2500-sdhci" or "aspeed,ast2400-sdhci"
+- clocks : phandle + clock specifier pair of the external clock
+- interrupts : interrupt number
+- interrupt-parent : interrupt controller for bus, should reference a
+                     aspeed,aspeed-sdhci-ic interrupt controller
+
+Example:
+
+sdhci {
+	compatible = "simple-bus";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0 0x1e740000 0x1000>;
+
+	sdhci_ic: interrupt-controller@0 {
+		#interrupt-cells = <1>;
+		compatible = "aspeed,aspeed-sdhci-irq";
+		reg = <0x0 0x100>;
+		interrupts = <26>;
+		interrupt-controller;
+		clocks = <&syscon ASPEED_CLK_GATE_SDCLKCLK>;
+		clock-names = "sdclk", "sd_extclk";
+	};
+
+	sdhci_slot0: sdhci_slot0@100 {
+		compatible = "aspeed,sdhci-ast2500";
+		reg = <0x100 0x100>;
+		interrupts = <0>;
+		interrupt-parent = <&sdhci_ic>;
+		slot = <0>;
+		sdhci,auto-cmd12;
+		clocks = <&syscon ASPEED_CLK_SDIO>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_sd1_default>;
+	};
+};
diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
index 5dbb33c..5578ae0 100644
--- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
@@ -97,6 +97,20 @@
 	};
 };
 
+&sdhci_slot0 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_sd1_default>;
+};
+
+&sdhci_slot1 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_sd2_default>;
+};
+
 /*
  * Enable port A as device (via the virtual hub) and port B as
  * host by default on the eval board. This can be easily changed
diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index 7ad646d..f4a8d7d 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -387,6 +387,26 @@
 		clocks = <&syscon ASPEED_CLK_GATE_SDCLKCLK>;
 	};
 
+	sdhci_slot0: sdhci_slot0@100 {
+		compatible = "aspeed,sdhci-ast2400";
+		reg = <0x100 0x100>;
+		interrupts = <0>;
+		interrupt-parent = <&sdhci_ic>;
+		sdhci,auto-cmd12;
+		clocks = <&syscon ASPEED_CLK_SDIO>;
+		status = "disabled";
+	};
+
+	sdhci_slot1: sdhci_slot1@200 {
+		compatible = "aspeed,sdhci-ast2400";
+		reg = <0x200 0x100>;
+		interrupts = <1>;
+		interrupt-parent = <&sdhci_ic>;
+		sdhci,auto-cmd12;
+		clocks = <&syscon ASPEED_CLK_SDIO>;
+		status = "disabled";
+	};
+
 };
 
 &i2c {
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index ba850ca..effe631 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -464,6 +464,26 @@
 		clocks = <&syscon ASPEED_CLK_GATE_SDCLKCLK>;
 	};
 
+	sdhci_slot0: sdhci_slot0@100 {
+		compatible = "aspeed,sdhci-ast2500";
+		reg = <0x100 0x100>;
+		interrupts = <0>;
+		interrupt-parent = <&sdhci_ic>;
+		sdhci,auto-cmd12;
+		clocks = <&syscon ASPEED_CLK_SDIO>;
+		status = "disabled";
+	};
+
+	sdhci_slot1: sdhci_slot1@200 {
+		compatible = "aspeed,sdhci-ast2500";
+		reg = <0x200 0x100>;
+		interrupts = <1>;
+		interrupt-parent = <&sdhci_ic>;
+		sdhci,auto-cmd12;
+		clocks = <&syscon ASPEED_CLK_SDIO>;
+		status = "disabled";
+	};
+
 };
 
 &i2c {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH linux dev-4.17 7/7] configs: Aspeed: enable mmc host in defconfig
  2018-07-11  5:17 [PATCH linux dev-4.17 0/7] drivers/mmc/host: Add Aspeed SDIO driver Ryan Chen
                   ` (5 preceding siblings ...)
  2018-07-11  5:17 ` [PATCH linux dev-4.17 6/7] dts: Aspeed: Add devicetree " Ryan Chen
@ 2018-07-11  5:17 ` Ryan Chen
  6 siblings, 0 replies; 18+ messages in thread
From: Ryan Chen @ 2018-07-11  5:17 UTC (permalink / raw)
  To: openbmc; +Cc: Ryan Chen, joel, andrew, ryan_chen, benh, mine260309

Enable mmc host in aspeed_g5_defconfig

Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
---
 arch/arm/configs/aspeed_g5_defconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index b7f8fa1..7b99aa9 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -197,6 +197,12 @@ CONFIG_USB_CONFIGFS_F_LB_SS=y
 CONFIG_USB_CONFIGFS_F_FS=y
 CONFIG_USB_CONFIGFS_F_HID=y
 CONFIG_USB_CONFIGFS_F_PRINTER=y
+CONFIG_MMC=y
+CONFIG_MMC_BLOCK=y
+CONFIG_MMC_BLOCK_MINORS=8
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_PLTFM=y
+CONFIG_MMC_SDHCI_OF_ASPEED=y
 CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
 CONFIG_LEDS_CLASS_FLASH=y
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH linux dev-4.17 2/7] clk: Aspeed: Add sdhci reset and clock
  2018-07-11  5:17 ` [PATCH linux dev-4.17 2/7] clk: Aspeed: Add sdhci reset and clock Ryan Chen
@ 2018-07-11  5:26   ` Joel Stanley
  2018-07-11  5:49   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 18+ messages in thread
From: Joel Stanley @ 2018-07-11  5:26 UTC (permalink / raw)
  To: Ryan Chen
  Cc: OpenBMC Maillist, Andrew Jeffery, Ryan Chen,
	Benjamin Herrenschmidt, Lei YU

On 11 July 2018 at 15:17, Ryan Chen <ryanchen.aspeed@gmail.com> wrote:
> 1.Add Aspeed sdhci reset for SCU04 bit 16
> 2.Aspeed sdhci have two clock one is for controller clock,
> another is for SD card clock. so when enable sdhci need enable
> both.
>
> Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
> ---
>  drivers/clk/clk-aspeed.c                 | 8 +++++++-
>  include/dt-bindings/clock/aspeed-clock.h | 2 +-
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index 9e55743..cd1f3b2 100644
> --- a/drivers/clk/clk-aspeed.c
> +++ b/drivers/clk/clk-aspeed.c
> @@ -20,6 +20,7 @@
>
>  #define ASPEED_RESET_CTRL      0x04
>  #define ASPEED_CLK_SELECTION   0x08
> +#define  ASPEED_SDIO_CLK_EN BIT(15)
>  #define ASPEED_CLK_STOP_CTRL   0x0c
>  #define ASPEED_MPLL_PARAM      0x20
>  #define ASPEED_HPLL_PARAM      0x24
> @@ -260,6 +261,11 @@ static int aspeed_clk_enable(struct clk_hw *hw)
>         enval = (gate->flags & CLK_GATE_SET_TO_DISABLE) ? 0 : clk;
>         regmap_update_bits(gate->map, ASPEED_CLK_STOP_CTRL, clk, enval);
>
> +       /* sd ext clk */
> +       if (gate->reset_idx == 16) {
> +               regmap_update_bits(gate->map, ASPEED_CLK_SELECTION, ASPEED_SDIO_CLK_EN, ASPEED_SDIO_CLK_EN);

We do not special case individual clocks. The reset should instead be
added to the aspeed_gates structure above.

In fact, it already is:

        [ASPEED_CLK_GATE_SDCLKCLK] =    { 27, 16, "sdclk-gate",
 NULL,   0 }, /* SDIO/SD */

Although in that table it is 16, and you specify bit 15. The data
sheet I have says SD/SDIO is bit 16.


> +       }
> +
>         if (gate->reset_idx >= 0) {
>                 /* A delay of 10ms is specified by the ASPEED docs */
>                 mdelay(10);
> @@ -317,7 +323,7 @@ static const u8 aspeed_resets[] = {
>         [ASPEED_RESET_PECI]     = 10,
>         [ASPEED_RESET_I2C]      =  2,
>         [ASPEED_RESET_AHB]      =  1,
> -
> +       [ASPEED_RESET_SDHCI]    = 16,
>         /*
>          * SCUD4 resets start at an offset to separate them from
>          * the SCU04 resets.
> diff --git a/include/dt-bindings/clock/aspeed-clock.h b/include/dt-bindings/clock/aspeed-clock.h
> index 4476184..088553f 100644
> --- a/include/dt-bindings/clock/aspeed-clock.h
> +++ b/include/dt-bindings/clock/aspeed-clock.h
> @@ -50,5 +50,5 @@
>  #define ASPEED_RESET_I2C               7
>  #define ASPEED_RESET_AHB               8
>  #define ASPEED_RESET_CRT1              9
> -
> +#define ASPEED_RESET_SDHCI             10
>  #endif
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH linux dev-4.17 3/7] irqchip: Aspeed: Add Aspeed sdhci irq driver
  2018-07-11  5:17 ` [PATCH linux dev-4.17 3/7] irqchip: Aspeed: Add Aspeed sdhci irq driver Ryan Chen
@ 2018-07-11  5:29   ` Joel Stanley
  0 siblings, 0 replies; 18+ messages in thread
From: Joel Stanley @ 2018-07-11  5:29 UTC (permalink / raw)
  To: Ryan Chen
  Cc: OpenBMC Maillist, Andrew Jeffery, Ryan Chen,
	Benjamin Herrenschmidt, Lei YU

On 11 July 2018 at 15:17, Ryan Chen <ryanchen.aspeed@gmail.com> wrote:
> In Aspeed SoC's sdhci have two slots and have a interrupt status and
> general information in top for register. so it need a sdhci irq driver
> probe defore sdhci driver probe

I suggest we keep the irq handing within in the sdhci driver instead
of creating a separate sdhci irqchip driver.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH linux dev-4.17 4/7] dts: Aspeed: Add Aspeed sdhci dts document
  2018-07-11  5:17 ` [PATCH linux dev-4.17 4/7] dts: Aspeed: Add Aspeed sdhci dts document Ryan Chen
@ 2018-07-11  5:30   ` Joel Stanley
  0 siblings, 0 replies; 18+ messages in thread
From: Joel Stanley @ 2018-07-11  5:30 UTC (permalink / raw)
  To: Ryan Chen
  Cc: OpenBMC Maillist, Andrew Jeffery, Ryan Chen,
	Benjamin Herrenschmidt, Lei YU

On 11 July 2018 at 15:17, Ryan Chen <ryanchen.aspeed@gmail.com> wrote:
> This add Aspeed sdhci irq driver's dts file and devicetree document

The bindings document (Documentation/devicetree) should be a separate
patch to the device tree changes (arch/arm/boot/dts). The bindings
will be merged with the code, while the device tree changes are merged
by the architecture maintainer.


>
> Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
> ---
>  .../aspeed,aspeed-sdhci-ic.txt                     | 25 ++++++++++++++++++++++
>  arch/arm/boot/dts/aspeed-g4.dtsi                   | 20 +++++++++++++++++
>  arch/arm/boot/dts/aspeed-g5.dtsi                   | 20 +++++++++++++++++
>  3 files changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,aspeed-sdhci-ic.txt
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,aspeed-sdhci-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/aspeed,aspeed-sdhci-ic.txt
> new file mode 100644
> index 0000000..e3393c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,aspeed-sdhci-ic.txt
> @@ -0,0 +1,25 @@
> +Device tree configuration for the SDHCI Interrupt Controller on the AST24XX and
> +AST25XX SoCs.
> +
> +Required Properties:
> +- #address-cells       : should be 1
> +- #size-cells          : should be 1
> +- #interrupt-cells     : should be 1
> +- compatible           : should be "aspeed,aspeed-sdhci-ic"
> +- reg                  : address start and range of controller
> +- interrupts           : interrupt number
> +- interrupt-controller : denotes that the controller receives and fires
> +                         new interrupts for child busses
> +
> +Example:
> +
> +sdhci_ic: interrupt-controller@0 {
> +       #interrupt-cells = <1>;
> +       #size-cells = <1>;
> +       #interrupt-cells = <1>;
> +       compatible = "aspeed,aspeed-sdhci-irq";
> +       reg = <0x0 0x100>;
> +       interrupts = <26>;
> +       interrupt-controller;
> +       clocks = <&syscon ASPEED_CLK_GATE_SDCLKCLK>;
> +};
> \ No newline at end of file
> diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
> index e86fa80..7ad646d 100644
> --- a/arch/arm/boot/dts/aspeed-g4.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
> @@ -190,6 +190,13 @@
>                                 reg = <0x1e720000 0x8000>;      // 32K
>                         };
>
> +                       sdhci: sdhci@1e740000 {
> +                               compatible = "simple-bus";
> +                               #address-cells = <1>;
> +                               #size-cells = <1>;
> +                               ranges = <0 0x1e740000 0x1000>;
> +                       };
> +
>                         gpio: gpio@1e780000 {
>                                 #gpio-cells = <2>;
>                                 gpio-controller;
> @@ -369,6 +376,19 @@
>         };
>  };
>
> +&sdhci {
> +
> +       sdhci_ic: interrupt-controller@0 {
> +               #interrupt-cells = <1>;
> +               compatible = "aspeed,aspeed-sdhci-irq";
> +               reg = <0x0 0x100>;
> +               interrupts = <26>;
> +               interrupt-controller;
> +               clocks = <&syscon ASPEED_CLK_GATE_SDCLKCLK>;
> +       };
> +
> +};
> +
>  &i2c {
>         i2c_ic: interrupt-controller@0 {
>                 #interrupt-cells = <1>;
> diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
> index d92f047..ba850ca 100644
> --- a/arch/arm/boot/dts/aspeed-g5.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
> @@ -256,6 +256,13 @@
>                                 reg = <0x1e720000 0x9000>;      // 36K
>                         };
>
> +                       sdhci: sdhci@1e740000 {
> +                               compatible = "simple-bus";
> +                               #address-cells = <1>;
> +                               #size-cells = <1>;
> +                               ranges = <0 0x1e740000 0x1000>;
> +                       };
> +
>                         gpio: gpio@1e780000 {
>                                 #gpio-cells = <2>;
>                                 gpio-controller;
> @@ -446,6 +453,19 @@
>         };
>  };
>
> +&sdhci {
> +
> +       sdhci_ic: interrupt-controller@0 {
> +               #interrupt-cells = <1>;
> +               compatible = "aspeed,aspeed-sdhci-irq";
> +               reg = <0x0 0x100>;
> +               interrupts = <26>;
> +               interrupt-controller;
> +               clocks = <&syscon ASPEED_CLK_GATE_SDCLKCLK>;
> +       };
> +
> +};
> +
>  &i2c {
>         i2c_ic: interrupt-controller@0 {
>                 #interrupt-cells = <1>;
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH linux dev-4.17 5/7] mmc: Aspeed: Add driver for Aspeed sdhci
  2018-07-11  5:17 ` [PATCH linux dev-4.17 5/7] mmc: Aspeed: Add driver for Aspeed sdhci Ryan Chen
@ 2018-07-11  5:32   ` Joel Stanley
  0 siblings, 0 replies; 18+ messages in thread
From: Joel Stanley @ 2018-07-11  5:32 UTC (permalink / raw)
  To: Ryan Chen
  Cc: OpenBMC Maillist, Andrew Jeffery, Ryan Chen,
	Benjamin Herrenschmidt, Lei YU

On 11 July 2018 at 15:17, Ryan Chen <ryanchen.aspeed@gmail.com> wrote:
> Add a driver for Aspeed's sdhci controller core.

This looks good. You should add some more description of the sdhci
core. eg. it supports two slots, the different modes it supports, etc.

>
> Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
> ---
>  drivers/mmc/host/Kconfig           |  12 +++
>  drivers/mmc/host/Makefile          |   1 +
>  drivers/mmc/host/sdhci-of-aspeed.c | 178 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 191 insertions(+)
>  create mode 100644 drivers/mmc/host/sdhci-of-aspeed.c
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 9589f9c..5056e6e 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -142,6 +142,18 @@ config MMC_SDHCI_OF_ARASAN
>
>           If unsure, say N.
>
> +config MMC_SDHCI_OF_ASPEED
> +       tristate "SDHCI OF support for the ASPEED SDHCI controller"
> +       depends on MMC_SDHCI_PLTFM
> +       depends on OF
> +       help
> +         This selects the ASPEED Secure Digital Host Controller Interface.
> +
> +         If you have a controller with this interface, say Y or M here. You
> +         also need to enable an appropriate bus interface.
> +
> +         If unsure, say N.
> +
>  config MMC_SDHCI_OF_AT91
>         tristate "SDHCI OF support for the Atmel SDMMC controller"
>         depends on MMC_SDHCI_PLTFM
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 6aead24..084193b 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -78,6 +78,7 @@ obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)     += sdhci-esdhc-imx.o
>  obj-$(CONFIG_MMC_SDHCI_DOVE)           += sdhci-dove.o
>  obj-$(CONFIG_MMC_SDHCI_TEGRA)          += sdhci-tegra.o
>  obj-$(CONFIG_MMC_SDHCI_OF_ARASAN)      += sdhci-of-arasan.o
> +obj-$(CONFIG_MMC_SDHCI_OF_ASPEED)      += sdhci-of-aspeed.o
>  obj-$(CONFIG_MMC_SDHCI_OF_AT91)                += sdhci-of-at91.o
>  obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)       += sdhci-of-esdhc.o
>  obj-$(CONFIG_MMC_SDHCI_OF_HLWD)                += sdhci-of-hlwd.o
> diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> new file mode 100644
> index 0000000..8e609e1
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: GPL-2.0

This text here replaces the text below.

You can have your headers look something like:

  // SPDX-License-Identifier: GPL-2.0
  // Copyright (C) Some Company Ltd

Some people chose to omit it, but you can also have:

  // Author's name <email>


> +/*
> + * ASPEED Secure Digital Host Controller Interface.
> + * Copyright (C) ASPEED Technology Inc.
> + * Ryan Chen <ryan_chen@aspeedtech.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or (at
> + * your option) any later version.
> + *
> + */
> +
> +#include <linux/dma-mapping.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/mmc/host.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/mmc/sdhci-aspeed-data.h>
> +#include <linux/reset.h>
> +#include "sdhci-pltfm.h"
> +
> +static void sdhci_aspeed_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +       int div;
> +       u16 clk;
> +       unsigned long timeout;
> +
> +       if (clock == host->clock)
> +               return;
> +
> +       sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> +
> +       if (clock == 0)
> +               goto out;
> +
> +       for (div = 1; div < 256; div *= 2) {
> +               if ((host->max_clk / div) <= clock)
> +                       break;
> +       }
> +       div >>= 1;
> +
> +       clk = div << SDHCI_DIVIDER_SHIFT;
> +       clk |= SDHCI_CLOCK_INT_EN;
> +       sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +
> +       /* Wait max 20 ms */
> +       timeout = 20;
> +       while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> +                & SDHCI_CLOCK_INT_STABLE)) {
> +               if (timeout == 0) {
> +                       pr_err("%s: Internal clock never stabilised.\n",
> +                              mmc_hostname(host->mmc));
> +                       return;
> +               }
> +               timeout--;
> +               mdelay(1);
> +       }
> +
> +       clk |= SDHCI_CLOCK_CARD_EN;
> +       sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +
> +out:
> +       host->clock = clock;
> +}
> +
> +static void sdhci_aspeed_set_bus_width(struct sdhci_host *host, int width)
> +{
> +       struct sdhci_pltfm_host *pltfm_priv = sdhci_priv(host);
> +       struct aspeed_sdhci_irq *sdhci_irq = sdhci_pltfm_priv(pltfm_priv);
> +
> +       u8 ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> +
> +       if (sdhci_irq->regs) {
> +               if (width == MMC_BUS_WIDTH_8)
> +                       aspeed_sdhci_set_8bit_mode(sdhci_irq, 1);
> +               else
> +                       aspeed_sdhci_set_8bit_mode(sdhci_irq, 0);
> +       }
> +       if (width == MMC_BUS_WIDTH_4)
> +               ctrl |= SDHCI_CTRL_4BITBUS;
> +       else
> +               ctrl &= ~SDHCI_CTRL_4BITBUS;
> +
> +       sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> +
> +}
> +
> +static struct sdhci_ops  sdhci_aspeed_ops = {
> +       .set_clock = sdhci_aspeed_set_clock,
> +       .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> +       .set_bus_width = sdhci_aspeed_set_bus_width,
> +       .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
> +       .reset = sdhci_reset,
> +       .set_uhs_signaling = sdhci_set_uhs_signaling,
> +};
> +
> +static struct sdhci_pltfm_data sdhci_aspeed_pdata = {
> +       .ops = &sdhci_aspeed_ops,
> +       .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +       .quirks2 = SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
> +};
> +
> +static int sdhci_aspeed_probe(struct platform_device *pdev)
> +{
> +       struct sdhci_host *host;
> +       struct device_node *pnode;
> +       struct device_node *np = pdev->dev.of_node;
> +       struct sdhci_pltfm_host *pltfm_host;
> +       struct aspeed_sdhci_irq *sdhci_irq;
> +
> +       int ret;
> +
> +       host = sdhci_pltfm_init(pdev, &sdhci_aspeed_pdata, sizeof(struct aspeed_sdhci_irq));
> +       if (IS_ERR(host))
> +               return PTR_ERR(host);
> +
> +       pltfm_host = sdhci_priv(host);
> +       sdhci_irq = sdhci_pltfm_priv(pltfm_host);
> +
> +       sdhci_get_of_property(pdev);
> +
> +       pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
> +
> +       pnode = of_parse_phandle(np, "interrupt-parent", 0);
> +       if (pnode)
> +               memcpy(sdhci_irq, pnode->data, sizeof(struct aspeed_sdhci_irq));
> +
> +       ret = mmc_of_parse(host->mmc);
> +       if (ret)
> +               goto err_sdhci_add;
> +
> +       ret = sdhci_add_host(host);
> +       if (ret)
> +               goto err_sdhci_add;
> +
> +       return 0;
> +
> +err_sdhci_add:
> +       sdhci_pltfm_free(pdev);
> +       return ret;
> +}
> +
> +static int sdhci_aspeed_remove(struct platform_device *pdev)
> +{
> +       struct sdhci_host       *host = platform_get_drvdata(pdev);
> +       int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
> +
> +       sdhci_remove_host(host, dead);
> +       sdhci_pltfm_free(pdev);
> +       return 0;
> +}
> +
> +static const struct of_device_id sdhci_aspeed_of_match[] = {
> +       { .compatible = "aspeed,sdhci-ast2400", .data = &sdhci_aspeed_pdata },
> +       { .compatible = "aspeed,sdhci-ast2500", .data = &sdhci_aspeed_pdata },
> +       {}
> +};
> +
> +MODULE_DEVICE_TABLE(of, sdhci_aspeed_of_match);
> +
> +static struct platform_driver sdhci_aspeed_driver = {
> +       .driver         = {
> +               .name   = "sdhci-aspeed",
> +               .pm     = &sdhci_pltfm_pmops,
> +               .of_match_table = sdhci_aspeed_of_match,
> +       },
> +       .probe          = sdhci_aspeed_probe,
> +       .remove         = sdhci_aspeed_remove,
> +};
> +
> +module_platform_driver(sdhci_aspeed_driver);
> +
> +MODULE_DESCRIPTION("Driver for the ASPEED SDHCI Controller");
> +MODULE_AUTHOR("Ryan Chen <ryan_chen@aspeedtech.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence
  2018-07-11  5:17 ` [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence Ryan Chen
@ 2018-07-11  5:47   ` Benjamin Herrenschmidt
  2018-07-17  6:04     ` Ryan Chen
  0 siblings, 1 reply; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2018-07-11  5:47 UTC (permalink / raw)
  To: Ryan Chen, openbmc; +Cc: joel, andrew, ryan_chen, mine260309

On Wed, 2018-07-11 at 13:17 +0800, Ryan Chen wrote:
> In Aspeed's SoC, all IP clk gating and pll parameter is in scu
> controller, before IP driver probe, scu driver need prepare for it.
> So buildin_platform_driver to core_initcall.
> 
> Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
> ---
>  drivers/clk/clk-aspeed.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index 8796b8a..9e55743 100644
> --- a/drivers/clk/clk-aspeed.c
> +++ b/drivers/clk/clk-aspeed.c
> @@ -573,7 +573,12 @@ static struct platform_driver aspeed_clk_driver = {
>  		.suppress_bind_attrs = true,
>  	},
>  };
> -builtin_platform_driver(aspeed_clk_driver);
> +
> +static int __init aspeed_clk_init(void)
> +{
> +	return platform_driver_register(&aspeed_clk_driver);
> +}
> +core_initcall(aspeed_clk_init);
>  
>  static void __init aspeed_ast2400_cc(struct regmap *map)
>  {

It's generally considered dangerous to register drivers at core
initcall time.

Any reason we don't use the generic clock driver registration mechanism
that runs at of_clk_init() time ?

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH linux dev-4.17 2/7] clk: Aspeed: Add sdhci reset and clock
  2018-07-11  5:17 ` [PATCH linux dev-4.17 2/7] clk: Aspeed: Add sdhci reset and clock Ryan Chen
  2018-07-11  5:26   ` Joel Stanley
@ 2018-07-11  5:49   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2018-07-11  5:49 UTC (permalink / raw)
  To: Ryan Chen, openbmc; +Cc: joel, andrew, ryan_chen, mine260309

On Wed, 2018-07-11 at 13:17 +0800, Ryan Chen wrote:
> 1.Add Aspeed sdhci reset for SCU04 bit 16
> 2.Aspeed sdhci have two clock one is for controller clock,
> another is for SD card clock. so when enable sdhci need enable
> both.

You should expose this as two different clocks instead of
putting a hack inside aspeed_clk_enable().

> Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
> ---
>  drivers/clk/clk-aspeed.c                 | 8 +++++++-
>  include/dt-bindings/clock/aspeed-clock.h | 2 +-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index 9e55743..cd1f3b2 100644
> --- a/drivers/clk/clk-aspeed.c
> +++ b/drivers/clk/clk-aspeed.c
> @@ -20,6 +20,7 @@
>  
>  #define ASPEED_RESET_CTRL	0x04
>  #define ASPEED_CLK_SELECTION	0x08
> +#define  ASPEED_SDIO_CLK_EN BIT(15)
>  #define ASPEED_CLK_STOP_CTRL	0x0c
>  #define ASPEED_MPLL_PARAM	0x20
>  #define ASPEED_HPLL_PARAM	0x24
> @@ -260,6 +261,11 @@ static int aspeed_clk_enable(struct clk_hw *hw)
>  	enval = (gate->flags & CLK_GATE_SET_TO_DISABLE) ? 0 : clk;
>  	regmap_update_bits(gate->map, ASPEED_CLK_STOP_CTRL, clk, enval);
>  
> +	/* sd ext clk */
> +	if (gate->reset_idx == 16) {
> +		regmap_update_bits(gate->map, ASPEED_CLK_SELECTION, ASPEED_SDIO_CLK_EN, ASPEED_SDIO_CLK_EN);
> +	}
> +
>  	if (gate->reset_idx >= 0) {
>  		/* A delay of 10ms is specified by the ASPEED docs */
>  		mdelay(10);
> @@ -317,7 +323,7 @@ static const u8 aspeed_resets[] = {
>  	[ASPEED_RESET_PECI]	= 10,
>  	[ASPEED_RESET_I2C]	=  2,
>  	[ASPEED_RESET_AHB]	=  1,
> -
> +	[ASPEED_RESET_SDHCI]	= 16,
>  	/*
>  	 * SCUD4 resets start at an offset to separate them from
>  	 * the SCU04 resets.
> diff --git a/include/dt-bindings/clock/aspeed-clock.h b/include/dt-bindings/clock/aspeed-clock.h
> index 4476184..088553f 100644
> --- a/include/dt-bindings/clock/aspeed-clock.h
> +++ b/include/dt-bindings/clock/aspeed-clock.h
> @@ -50,5 +50,5 @@
>  #define ASPEED_RESET_I2C		7
>  #define ASPEED_RESET_AHB		8
>  #define ASPEED_RESET_CRT1		9
> -
> +#define ASPEED_RESET_SDHCI		10
>  #endif

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence
  2018-07-11  5:47   ` Benjamin Herrenschmidt
@ 2018-07-17  6:04     ` Ryan Chen
  2018-07-17  9:45       ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 18+ messages in thread
From: Ryan Chen @ 2018-07-17  6:04 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: openbmc, joel, andrew, ryan_chen, mine260309

On Wed, Jul 11, 2018 at 03:47:56PM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2018-07-11 at 13:17 +0800, Ryan Chen wrote:
> > In Aspeed's SoC, all IP clk gating and pll parameter is in scu
> > controller, before IP driver probe, scu driver need prepare for it.
> > So buildin_platform_driver to core_initcall.
> > 
> > Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
> > ---
> >  drivers/clk/clk-aspeed.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> > index 8796b8a..9e55743 100644
> > --- a/drivers/clk/clk-aspeed.c
> > +++ b/drivers/clk/clk-aspeed.c
> > @@ -573,7 +573,12 @@ static struct platform_driver aspeed_clk_driver = {
> >  		.suppress_bind_attrs = true,
> >  	},
> >  };
> > -builtin_platform_driver(aspeed_clk_driver);
> > +
> > +static int __init aspeed_clk_init(void)
> > +{
> > +	return platform_driver_register(&aspeed_clk_driver);
> > +}
> > +core_initcall(aspeed_clk_init);
> >  
> >  static void __init aspeed_ast2400_cc(struct regmap *map)
> >  {
> 
> It's generally considered dangerous to register drivers at core
> initcall time.

Understand. 

But if interrupt controller have clk gating. 
the scu driver should be eraly than irq chip driver probe. 
Is this point is reasonable? 

> 
> Any reason we don't use the generic clock driver registration mechanism
> that runs at of_clk_init() time ?

I will use "if (gate->reset_idx == aspeed_resets[ASPEED_RESET_SDHCI])", 
is it suitable ?  


Ryan Chen

> 
> Cheers,
> Ben.
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence
  2018-07-17  6:04     ` Ryan Chen
@ 2018-07-17  9:45       ` Benjamin Herrenschmidt
  2018-07-18  6:37         ` Joel Stanley
  0 siblings, 1 reply; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2018-07-17  9:45 UTC (permalink / raw)
  To: Ryan Chen; +Cc: openbmc, joel, andrew, ryan_chen, mine260309

On Tue, 2018-07-17 at 14:04 +0800, Ryan Chen wrote:
> On Wed, Jul 11, 2018 at 03:47:56PM +1000, Benjamin Herrenschmidt wrote:
> > On Wed, 2018-07-11 at 13:17 +0800, Ryan Chen wrote:
> > > In Aspeed's SoC, all IP clk gating and pll parameter is in scu
> > > controller, before IP driver probe, scu driver need prepare for it.
> > > So buildin_platform_driver to core_initcall.
> > > 
> > > Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
> > > ---
> > >  drivers/clk/clk-aspeed.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> > > index 8796b8a..9e55743 100644
> > > --- a/drivers/clk/clk-aspeed.c
> > > +++ b/drivers/clk/clk-aspeed.c
> > > @@ -573,7 +573,12 @@ static struct platform_driver aspeed_clk_driver = {
> > >  		.suppress_bind_attrs = true,
> > >  	},
> > >  };
> > > -builtin_platform_driver(aspeed_clk_driver);
> > > +
> > > +static int __init aspeed_clk_init(void)
> > > +{
> > > +	return platform_driver_register(&aspeed_clk_driver);
> > > +}
> > > +core_initcall(aspeed_clk_init);
> > >  
> > >  static void __init aspeed_ast2400_cc(struct regmap *map)
> > >  {
> > 
> > It's generally considered dangerous to register drivers at core
> > initcall time.
> 
> Understand. 
> 
> But if interrupt controller have clk gating. 
> the scu driver should be eraly than irq chip driver probe. 
> Is this point is reasonable? 

I'm not sure I understand what you are trying to solve other than
making sure the clock driver is loaded before everything else. Joel, do
we have a way to ensure that ? I noticed all other clock drivers use
that macro to be initialized at of_clk_init, any reason we don't ?
> 
> > 
> > Any reason we don't use the generic clock driver registration mechanism
> > that runs at of_clk_init() time ?
> 
> I will use "if (gate->reset_idx == aspeed_resets[ASPEED_RESET_SDHCI])", 
> is it suitable ?  

I'm on holiday, I don't have the code at hand to check. Joel ? What do
you reckon ?

> 
> Ryan Chen
> 
> > 
> > Cheers,
> > Ben.
> > 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence
  2018-07-17  9:45       ` Benjamin Herrenschmidt
@ 2018-07-18  6:37         ` Joel Stanley
  2018-07-18  7:16           ` Ryan Chen
  0 siblings, 1 reply; 18+ messages in thread
From: Joel Stanley @ 2018-07-18  6:37 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Ryan Chen, OpenBMC Maillist, Andrew Jeffery, Ryan Chen, Lei YU

On 17 July 2018 at 19:15, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2018-07-17 at 14:04 +0800, Ryan Chen wrote:
>> On Wed, Jul 11, 2018 at 03:47:56PM +1000, Benjamin Herrenschmidt wrote:
>> > On Wed, 2018-07-11 at 13:17 +0800, Ryan Chen wrote:
>> > > In Aspeed's SoC, all IP clk gating and pll parameter is in scu
>> > > controller, before IP driver probe, scu driver need prepare for it.
>> > > So buildin_platform_driver to core_initcall.
>> > >
>> > > Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
>> > > ---
>> > >  drivers/clk/clk-aspeed.c | 7 ++++++-
>> > >  1 file changed, 6 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
>> > > index 8796b8a..9e55743 100644
>> > > --- a/drivers/clk/clk-aspeed.c
>> > > +++ b/drivers/clk/clk-aspeed.c
>> > > @@ -573,7 +573,12 @@ static struct platform_driver aspeed_clk_driver = {
>> > >           .suppress_bind_attrs = true,
>> > >   },
>> > >  };
>> > > -builtin_platform_driver(aspeed_clk_driver);
>> > > +
>> > > +static int __init aspeed_clk_init(void)
>> > > +{
>> > > + return platform_driver_register(&aspeed_clk_driver);
>> > > +}
>> > > +core_initcall(aspeed_clk_init);
>> > >
>> > >  static void __init aspeed_ast2400_cc(struct regmap *map)
>> > >  {
>> >
>> > It's generally considered dangerous to register drivers at core
>> > initcall time.
>>
>> Understand.
>>
>> But if interrupt controller have clk gating.
>> the scu driver should be eraly than irq chip driver probe.
>> Is this point is reasonable?
>
> I'm not sure I understand what you are trying to solve other than
> making sure the clock driver is loaded before everything else. Joel, do
> we have a way to ensure that ? I noticed all other clock drivers use
> that macro to be initialized at of_clk_init, any reason we don't ?
>>
>> >
>> > Any reason we don't use the generic clock driver registration mechanism
>> > that runs at of_clk_init() time ?
>>
>> I will use "if (gate->reset_idx == aspeed_resets[ASPEED_RESET_SDHCI])",
>> is it suitable ?
>
> I'm on holiday, I don't have the code at hand to check. Joel ? What do
> you reckon ?

We don't need to do this. I mentioned this in my review; we have the
reset added to the clk driver already. With this, reset can be
released via the normal reset controller call in the sdhci driver.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence
  2018-07-18  6:37         ` Joel Stanley
@ 2018-07-18  7:16           ` Ryan Chen
  0 siblings, 0 replies; 18+ messages in thread
From: Ryan Chen @ 2018-07-18  7:16 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Benjamin Herrenschmidt, OpenBMC Maillist, Andrew Jeffery,
	Ryan Chen, Lei YU

On Wed, Jul 18, 2018 at 04:07:14PM +0930, Joel Stanley wrote:
> On 17 July 2018 at 19:15, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > On Tue, 2018-07-17 at 14:04 +0800, Ryan Chen wrote:
> >> On Wed, Jul 11, 2018 at 03:47:56PM +1000, Benjamin Herrenschmidt wrote:
> >> > On Wed, 2018-07-11 at 13:17 +0800, Ryan Chen wrote:
> >> > > In Aspeed's SoC, all IP clk gating and pll parameter is in scu
> >> > > controller, before IP driver probe, scu driver need prepare for it.
> >> > > So buildin_platform_driver to core_initcall.
> >> > >
> >> > > Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
> >> > > ---
> >> > >  drivers/clk/clk-aspeed.c | 7 ++++++-
> >> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> >> > >
> >> > > diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> >> > > index 8796b8a..9e55743 100644
> >> > > --- a/drivers/clk/clk-aspeed.c
> >> > > +++ b/drivers/clk/clk-aspeed.c
> >> > > @@ -573,7 +573,12 @@ static struct platform_driver aspeed_clk_driver = {
> >> > >           .suppress_bind_attrs = true,
> >> > >   },
> >> > >  };
> >> > > -builtin_platform_driver(aspeed_clk_driver);
> >> > > +
> >> > > +static int __init aspeed_clk_init(void)
> >> > > +{
> >> > > + return platform_driver_register(&aspeed_clk_driver);
> >> > > +}
> >> > > +core_initcall(aspeed_clk_init);
> >> > >
> >> > >  static void __init aspeed_ast2400_cc(struct regmap *map)
> >> > >  {
> >> >
> >> > It's generally considered dangerous to register drivers at core
> >> > initcall time.
> >>
> >> Understand.
> >>
> >> But if interrupt controller have clk gating.
> >> the scu driver should be eraly than irq chip driver probe.
> >> Is this point is reasonable?
> >
> > I'm not sure I understand what you are trying to solve other than
> > making sure the clock driver is loaded before everything else. Joel, do
> > we have a way to ensure that ? I noticed all other clock drivers use
> > that macro to be initialized at of_clk_init, any reason we don't ?
> >>
> >> >
> >> > Any reason we don't use the generic clock driver registration mechanism
> >> > that runs at of_clk_init() time ?
> >>
> >> I will use "if (gate->reset_idx == aspeed_resets[ASPEED_RESET_SDHCI])",
> >> is it suitable ?
> >
> > I'm on holiday, I don't have the code at hand to check. Joel ? What do
> > you reckon ?
> 
> We don't need to do this. I mentioned this in my review; we have the
> reset added to the clk driver already. With this, reset can be
> released via the normal reset controller call in the sdhci driver.

In my new patch will use following in aspeed_clk_enable function
.....
	- Put in reset
	- enable sd clk
	/* sd ext clk */
	if (gate->reset_idx == aspeed_resets[ASPEED_RESET_SDHCI]) {
		regmap_update_bits(gate->map, ASPEED_CLK_SELECTION, 
				ASPEED_SDIO_CLK_EN, ASPEED_SDIO_CLK_EN);
	}
	- Out of reset
.....
Do you think is it suitable, Joel? 

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-07-18  7:18 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-11  5:17 [PATCH linux dev-4.17 0/7] drivers/mmc/host: Add Aspeed SDIO driver Ryan Chen
2018-07-11  5:17 ` [PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence Ryan Chen
2018-07-11  5:47   ` Benjamin Herrenschmidt
2018-07-17  6:04     ` Ryan Chen
2018-07-17  9:45       ` Benjamin Herrenschmidt
2018-07-18  6:37         ` Joel Stanley
2018-07-18  7:16           ` Ryan Chen
2018-07-11  5:17 ` [PATCH linux dev-4.17 2/7] clk: Aspeed: Add sdhci reset and clock Ryan Chen
2018-07-11  5:26   ` Joel Stanley
2018-07-11  5:49   ` Benjamin Herrenschmidt
2018-07-11  5:17 ` [PATCH linux dev-4.17 3/7] irqchip: Aspeed: Add Aspeed sdhci irq driver Ryan Chen
2018-07-11  5:29   ` Joel Stanley
2018-07-11  5:17 ` [PATCH linux dev-4.17 4/7] dts: Aspeed: Add Aspeed sdhci dts document Ryan Chen
2018-07-11  5:30   ` Joel Stanley
2018-07-11  5:17 ` [PATCH linux dev-4.17 5/7] mmc: Aspeed: Add driver for Aspeed sdhci Ryan Chen
2018-07-11  5:32   ` Joel Stanley
2018-07-11  5:17 ` [PATCH linux dev-4.17 6/7] dts: Aspeed: Add devicetree " Ryan Chen
2018-07-11  5:17 ` [PATCH linux dev-4.17 7/7] configs: Aspeed: enable mmc host in defconfig Ryan Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).